Harden secret provisioning and lifecycle controls

Assistant: codex
Assistant-Model: gpt-5.6-sol
Assistant-Session: 01a0217e-8c4c-7383-be6b-f50a6e485306

tests/test_auth_capability.py

@@ -1,6 +1,8 @@
 import copy
 import os
+from contextlib import contextmanager
 from pathlib import Path
+from types import SimpleNamespace
 
 import pytest
 
@@ -172,9 +174,17 @@ def test_apply_auth_capability_bypasses_kv_and_writes_ttl_options():
 
 
 class FakeVerifyClient:
-    def approle_login_token(self, role_name):
+    def __init__(self):
+        self.token = "test-token"
+        self.sessions_closed = 0
+
+    @contextmanager
+    def approle_session(self, role_name):
         assert role_name == "warden-sign"
-        return "test-token"
+        try:
+            yield SimpleNamespace(client=self)
+        finally:
+            self.sessions_closed += 1
 
     def token_capabilities(self, path, *, token):
         assert token == "test-token"
@@ -184,10 +194,12 @@ class FakeVerifyClient:
 
 
 def test_auth_capability_verification_uses_capability_probes():
+    client = FakeVerifyClient()
     results = run_verification(
-        FakeVerifyClient(), _auth_entry(), "", positive=True, negative=True
+        client, _auth_entry(), "", positive=True, negative=True
     )
     assert [result.passed for result in results] == [True, True]
+    assert client.sessions_closed == 2
 
 
 class FakeHandoffClient:

tests/test_exec_delivery.py

@@ -5,6 +5,7 @@ import pytest
 from secrets_engine.catalog import validate_entry
 from secrets_engine.errors import DeliveryError
 from secrets_engine.exec_delivery import (
+    _fetch_value,
     _npm_userconfig,
     _registry_authkey,
     exec_with_secret,
@@ -70,3 +71,53 @@ def test_exec_rejects_undeclared_field_before_fetch(monkeypatch):
     )
     with pytest.raises(DeliveryError):
         exec_with_secret(object(), entry, "other_field", ["probe"], mode="exec-env")
+
+
+def test_fetch_records_non_secret_session_cleanup_before_child(monkeypatch):
+    entry = validate_entry(VALID)
+
+    class Session:
+        def __init__(self):
+            self.client = self
+            self.closed = False
+
+        def _run(self, _args):
+            from types import SimpleNamespace
+            import json
+
+            return SimpleNamespace(
+                returncode=0,
+                stdout=json.dumps({"data": {"data": {"api_token": "test-value"}}}),
+            )
+
+        def evidence(self):
+            return {
+                "session_handle": "safe-handle",
+                "established": True,
+                "revocation_attempted": self.closed,
+                "revocation_succeeded": self.closed,
+            }
+
+    session = Session()
+
+    class Client:
+        from contextlib import contextmanager
+
+        @contextmanager
+        def approle_session(self, _role):
+            try:
+                yield session
+            finally:
+                session.closed = True
+
+    evidence = {}
+    value = _fetch_value(Client(), entry, "api_token", session_evidence=evidence)
+
+    assert value == "test-value"
+    assert evidence == {
+        "session_handle": "safe-handle",
+        "established": True,
+        "revocation_attempted": True,
+        "revocation_succeeded": True,
+    }
+    assert "test-value" not in repr(evidence)

tests/test_existing_lane_admission.py

@@ -36,13 +36,13 @@ class RecordingApplyClient:
 
 class RecordingProvisionClient:
     def __init__(self):
-        self.puts = []
+        self.patches = []
 
     def ensure_kv_mount(self, _mount):
         raise AssertionError("existing mount must not be created during provision")
 
-    def kv_put(self, mount, path, field, value):
-        self.puts.append((mount, path, field, value))
+    def kv_patch_fields(self, mount, path, values):
+        self.patches.append((mount, path, values))
 
 
 def _existing_mount_entry():
@@ -97,8 +97,43 @@ def test_provision_existing_mount_never_attempts_mount_creation(tmp_path):
     os.chmod(value_file, 0o600)
     client = RecordingProvisionClient()
     provision_from_file(client, entry, "api_token", value_file)
-    assert client.puts == [
-        ("platform", "workloads/example/runtime", "api_token", "test-only-value")
+    assert client.patches == [
+        (
+            "platform",
+            "workloads/example/runtime",
+            {"api_token": "test-only-value"},
+        )
+    ]
+
+
+def test_provision_existing_multi_field_path_uses_merge_safe_backend(tmp_path):
+    data = copy.deepcopy(VALID)
+    data.update(
+        {
+            "stage": "prod",
+            "mount": "platform",
+            "path": "workloads/example/runtime",
+            "mount_management": "existing",
+            "fields": ["api_token", "webhook_secret"],
+            "workload_delivery": [
+                {"mode": "external-secrets", "owner": "rapp-example"}
+            ],
+        }
+    )
+    entry = validate_entry(data)
+    value_file = tmp_path / "value"
+    value_file.write_text("test-only-value", encoding="utf-8")
+    os.chmod(value_file, 0o600)
+    client = RecordingProvisionClient()
+
+    provision_from_file(client, entry, "api_token", value_file)
+
+    assert client.patches == [
+        (
+            "platform",
+            "workloads/example/runtime",
+            {"api_token": "test-only-value"},
+        )
     ]
 
 

tests/test_integration_bao.py

@@ -17,6 +17,7 @@ import pytest
 from secrets_engine.apply import apply_plan
 from secrets_engine.catalog import get_entry
 from secrets_engine.config import repo_root
+from secrets_engine.errors import BackendError
 from secrets_engine.exec_delivery import exec_with_secret
 from secrets_engine.openbao import OpenBaoClient
 from secrets_engine.plan import build_plan
@@ -75,6 +76,18 @@ def test_full_chain(bao_dev, tmp_path):
     os.chmod(tokenfile, 0o600)
     provision_from_file(client, entry, "npm_token", tokenfile)
 
+    # Server-side patch preserves an unmentioned sibling. Provisioning the
+    # declared field again must not replace that sibling.
+    client.kv_patch_fields(
+        entry.mount, entry.path, {"integration_sibling": "still-present"}
+    )
+    tokenfile.write_text("npm_integrationTESTvalue0987654321")
+    os.chmod(tokenfile, 0o600)
+    provision_from_file(client, entry, "npm_token", tokenfile)
+    assert client.kv_field_present(
+        entry.mount, entry.path, "integration_sibling", token=client.token
+    )
+
     pos = verify_positive(client, entry, "npm_token")
     assert pos.passed, pos.detail
     neg = verify_negative(client, entry)
@@ -93,6 +106,24 @@ def test_full_chain(bao_dev, tmp_path):
     assert "SE_NPM_TOKEN" not in os.environ
 
 
+def test_merge_safe_patch_rejects_stale_cas(bao_dev):
+    client = bao_dev
+    mount = "cas-test"
+    path = "build/example"
+    client.ensure_kv_mount(mount)
+    client.kv_patch_fields(mount, path, {"first": "one", "second": "two"})
+    stale = client.kv_current_version(mount, path)
+    client.kv_patch_fields(mount, path, {"first": "new"}, expected_version=stale)
+
+    with pytest.raises(BackendError):
+        client.kv_patch_fields(
+            mount, path, {"second": "stale-write"}, expected_version=stale
+        )
+
+    assert client.kv_field_present(mount, path, "first", token=client.token)
+    assert client.kv_field_present(mount, path, "second", token=client.token)
+
+
 def test_idempotent_apply(bao_dev):
     client = bao_dev
     entry = get_entry(repo_root() / "catalog", "whynot-design-npm-publish")

tests/test_lifecycle.py

@@ -0,0 +1,151 @@
+import copy
+
+import pytest
+
+from secrets_engine.catalog import get_entry, validate_entry
+from secrets_engine.config import repo_root
+from secrets_engine.errors import PolicyGuardError
+from secrets_engine.lifecycle import (
+    apply_lifecycle_plan,
+    build_lifecycle_plan,
+    build_native_deactivation_plan,
+    require_destroy_confirmation,
+)
+from tests.test_catalog import VALID
+
+
+class RecordingLifecycleClient:
+    def __init__(self):
+        self.calls = []
+
+    def delete_approle(self, role_name):
+        self.calls.append(("delete-approle", role_name))
+
+    def delete_policy(self, policy_name):
+        self.calls.append(("delete-policy", policy_name))
+
+    def kv_delete_metadata(self, mount, path):
+        self.calls.append(("delete-kv-metadata", f"{mount}/{path}"))
+
+
+def _existing_kv_entry(*, auth_management="engine"):
+    data = copy.deepcopy(VALID)
+    data.update(
+        {
+            "stage": "prod",
+            "mount": "platform",
+            "path": "workloads/example/runtime",
+            "mount_management": "existing",
+            "workload_delivery": [
+                {"mode": "external-secrets", "owner": "rapp-example"}
+            ],
+        }
+    )
+    if auth_management == "existing":
+        data["delivery_auth"] = {
+            "method": "approle",
+            "management": "existing",
+            "role_name": "external-example-role",
+            "policy_name": "external-example-policy",
+        }
+    return validate_entry(data)
+
+
+def test_kv_revoke_plan_deactivates_native_auth_and_preserves_custody():
+    entry = _existing_kv_entry()
+    plan = build_native_deactivation_plan(entry)
+
+    assert [(a.kind, a.target, a.mutation) for a in plan.actions] == [
+        ("delete-approle", entry.role_name, True),
+        ("delete-policy", entry.policy_name, True),
+        ("preserve-kv-custody", "platform/workloads/example/runtime", False),
+        ("preserve-workload-delivery", entry.id, False),
+    ]
+
+    client = RecordingLifecycleClient()
+    result = apply_lifecycle_plan(client, plan)
+    assert client.calls == [
+        ("delete-approle", entry.role_name),
+        ("delete-policy", entry.policy_name),
+    ]
+    assert "platform/workloads/example/runtime" in result.preserved
+
+
+def test_revoke_plan_never_mutates_externally_managed_delivery_auth():
+    entry = _existing_kv_entry(auth_management="existing")
+    plan = build_native_deactivation_plan(entry)
+    assert not any(action.mutation for action in plan.actions)
+
+    client = RecordingLifecycleClient()
+    result = apply_lifecycle_plan(client, plan)
+    assert client.calls == []
+    assert entry.role_name in result.preserved
+    assert entry.policy_name in result.preserved
+
+
+def test_suspend_removes_only_approle_and_preserves_policy_and_kv():
+    entry = _existing_kv_entry()
+    plan = build_lifecycle_plan(entry, "suspend")
+    client = RecordingLifecycleClient()
+
+    result = apply_lifecycle_plan(client, plan)
+
+    assert client.calls == [("delete-approle", entry.role_name)]
+    assert entry.policy_name in result.preserved
+    assert f"{entry.mount}/{entry.path}" in result.preserved
+
+
+def test_destroy_plan_is_explicit_and_deletes_auth_before_kv_metadata():
+    entry = _existing_kv_entry()
+    plan = build_lifecycle_plan(entry, "destroy")
+    client = RecordingLifecycleClient()
+
+    result = apply_lifecycle_plan(client, plan)
+
+    assert plan.operation == "destroy"
+    assert client.calls == [
+        ("delete-approle", entry.role_name),
+        ("delete-policy", entry.policy_name),
+        ("delete-kv-metadata", f"{entry.mount}/{entry.path}"),
+    ]
+    assert list(result.applied) == [
+        entry.role_name,
+        entry.policy_name,
+        f"{entry.mount}/{entry.path}",
+    ]
+
+
+def test_destroy_requires_exact_confirmation_and_kv_lane():
+    entry = _existing_kv_entry()
+    with pytest.raises(PolicyGuardError, match="exact catalog id"):
+        require_destroy_confirmation(entry, "wrong-lane")
+    require_destroy_confirmation(entry, entry.id)
+
+    auth_entry = get_entry(repo_root() / "catalog", "warden-sign")
+    with pytest.raises(PolicyGuardError, match="no KV custody"):
+        build_lifecycle_plan(auth_entry, "destroy")
+
+
+def test_auth_capability_revoke_deletes_only_role_and_policy():
+    entry = get_entry(repo_root() / "catalog", "warden-sign")
+    plan = build_native_deactivation_plan(entry)
+    client = RecordingLifecycleClient()
+
+    apply_lifecycle_plan(client, plan)
+
+    assert client.calls == [
+        ("delete-approle", entry.role_name),
+        ("delete-policy", entry.policy_name),
+    ]
+
+
+def test_rendered_and_applied_mutation_targets_are_identical():
+    entry = _existing_kv_entry()
+    plan = build_native_deactivation_plan(entry)
+    rendered = plan.render()
+    expected_targets = [action.target for action in plan.actions if action.mutation]
+    assert all(target in rendered for target in expected_targets)
+
+    client = RecordingLifecycleClient()
+    result = apply_lifecycle_plan(client, plan)
+    assert list(result.applied) == expected_targets

tests/test_multifield_readiness.py

@@ -0,0 +1,138 @@
+import copy
+from pathlib import Path
+from types import SimpleNamespace
+
+import pytest
+
+from secrets_engine import cli
+from secrets_engine.catalog import validate_entry
+from secrets_engine.config import Config
+from secrets_engine.errors import PolicyGuardError
+from secrets_engine.routing import route_lane
+from secrets_engine.verify import VerifyResult
+from tests.test_catalog import VALID
+
+
+def _entry():
+    data = copy.deepcopy(VALID)
+    data["fields"] = ["api_token", "webhook_secret"]
+    return validate_entry(data)
+
+
+class ReadinessClient:
+    def __init__(self, presence):
+        self.presence = presence
+        self.requested_fields = None
+
+    def is_reachable(self):
+        return True
+
+    def read_policy(self, _name):
+        return "path \"secret/data/test/team/thing\" {}"
+
+    def approle_exists(self, _name):
+        return True
+
+    def kv_fields_present(self, _mount, _path, fields):
+        self.requested_fields = list(fields)
+        return dict(self.presence)
+
+
+def test_route_requires_every_declared_field_and_names_only_missing_fields(tmp_path):
+    client = ReadinessClient({"api_token": True, "webhook_secret": False})
+
+    result = route_lane(_entry(), hub_url="", repo_root=tmp_path, client=client)
+
+    assert client.requested_fields == ["api_token", "webhook_secret"]
+    assert result.value_present is False
+    assert result.ready is False
+    assert result.missing_fields == ["webhook_secret"]
+    assert result.missing == "provisioned secret fields: webhook_secret"
+    assert "--field webhook_secret" in result.next_command
+
+
+def test_route_is_ready_only_when_every_declared_field_is_present(tmp_path):
+    client = ReadinessClient({"api_token": True, "webhook_secret": True})
+
+    result = route_lane(_entry(), hub_url="", repo_root=tmp_path, client=client)
+
+    assert result.value_present is True
+    assert result.missing_fields == []
+    assert result.ready is True
+
+
+def _config(tmp_path: Path) -> Config:
+    return Config(
+        catalog_dir=tmp_path,
+        policy_dir=tmp_path,
+        evidence_dir=tmp_path / "evidence",
+        hub_url="",
+        bao_addr="http://127.0.0.1:8200",
+        topic_id="test-topic",
+    )
+
+
+def test_verify_defaults_to_every_declared_field_and_one_path_denial(
+    tmp_path, monkeypatch
+):
+    entry = _entry()
+    calls = []
+    records = []
+
+    monkeypatch.setattr(cli, "get_entry", lambda *_args: entry)
+    monkeypatch.setattr(cli, "_require_lane_approval", lambda *_args: None)
+    monkeypatch.setattr(cli.OpenBaoClient, "resolve", lambda *_args, **_kwargs: object())
+
+    def fake_verify(_client, _entry, field, *, positive, negative):
+        calls.append((field, positive, negative))
+        check = "positive" if positive else "negative"
+        return [VerifyResult(check, True, {"field": field, "reason": "test"})]
+
+    monkeypatch.setattr(cli, "run_verification", fake_verify)
+    monkeypatch.setattr(
+        cli,
+        "_writer",
+        lambda _cfg: SimpleNamespace(record=lambda *args, **kwargs: records.append((args, kwargs))),
+    )
+    args = SimpleNamespace(
+        catalog_id=entry.id,
+        bootstrap_token_file=None,
+        field=None,
+        positive=False,
+        negative=False,
+    )
+
+    assert cli.cmd_verify(_config(tmp_path), args) == 0
+    assert calls == [
+        ("api_token", True, False),
+        ("webhook_secret", True, False),
+        ("api_token", False, True),
+    ]
+    assert len(records) == 3
+
+
+def test_live_destroy_fails_before_approval_or_backend_until_action_contract(
+    tmp_path, monkeypatch
+):
+    entry = _entry()
+    monkeypatch.setattr(cli, "get_entry", lambda *_args: entry)
+    monkeypatch.setattr(
+        cli,
+        "_require_lane_approval",
+        lambda *_args: pytest.fail("coarse lane approval must not authorize destroy"),
+    )
+    monkeypatch.setattr(
+        cli.OpenBaoClient,
+        "resolve",
+        lambda *_args, **_kwargs: pytest.fail("backend must not be reached"),
+    )
+    args = SimpleNamespace(
+        catalog_id=entry.id,
+        operation="destroy",
+        dry_run=False,
+        confirm_destroy=entry.id,
+        bootstrap_token_file=None,
+    )
+
+    with pytest.raises(PolicyGuardError, match="exact-action destruction approval"):
+        cli.cmd_lifecycle(_config(tmp_path), args)

tests/test_openbao_safe_write.py

@@ -0,0 +1,171 @@
+import json
+from pathlib import Path
+from types import SimpleNamespace
+
+import pytest
+
+from secrets_engine.errors import BackendError
+from secrets_engine.openbao import OpenBaoClient
+
+
+def test_kv_patch_keeps_secret_out_of_argv_and_cleans_input(monkeypatch):
+    secret = "fake-SUPER-SECRET-value"
+    client = OpenBaoClient(addr="http://example.invalid", token="", bao_bin="bao")
+    monkeypatch.setattr(client, "kv_current_version", lambda _mount, _path: 7)
+    captured = {}
+
+    def fake_run_ok(args, *, stdin=None):
+        captured["args"] = list(args)
+        captured["stdin"] = stdin
+        input_path = Path(args[-1][1:])
+        captured["input_path"] = input_path
+        captured["mode"] = input_path.stat().st_mode & 0o777
+        captured["payload"] = json.loads(input_path.read_text(encoding="utf-8"))
+        return ""
+
+    monkeypatch.setattr(client, "_run_ok", fake_run_ok)
+    base = client.kv_patch_fields("secret", "prod/example", {"TOKEN": secret})
+
+    assert base == 7
+    assert secret not in " ".join(captured["args"])
+    assert captured["stdin"] is None
+    assert captured["mode"] == 0o600
+    assert captured["payload"] == {"TOKEN": secret}
+    assert not captured["input_path"].exists()
+
+
+def test_new_path_uses_cas_zero_put_without_secret_in_argv(monkeypatch):
+    secret = "fake-new-path-secret"
+    client = OpenBaoClient(addr="http://example.invalid", token="", bao_bin="bao")
+    monkeypatch.setattr(client, "kv_current_version", lambda _mount, _path: 0)
+    captured = {}
+
+    def fake_run_ok(args, *, stdin=None):
+        captured["args"] = list(args)
+        return ""
+
+    monkeypatch.setattr(client, "_run_ok", fake_run_ok)
+    client.kv_patch_fields("secret", "build/example", {"TOKEN": secret})
+
+    assert captured["args"][:5] == [
+        "kv",
+        "put",
+        "-mount=secret",
+        "-cas=0",
+        "build/example",
+    ]
+    assert secret not in " ".join(captured["args"])
+    assert not Path(captured["args"][-1][1:]).exists()
+
+
+def test_kv_current_version_fails_closed_on_permission_error(monkeypatch):
+    client = OpenBaoClient(addr="http://example.invalid", token="", bao_bin="bao")
+    monkeypatch.setattr(
+        client,
+        "_run",
+        lambda _args: SimpleNamespace(
+            returncode=2, stdout="", stderr="permission denied"
+        ),
+    )
+    with pytest.raises(BackendError, match="metadata get failed"):
+        client.kv_current_version("secret", "prod/example")
+
+
+def test_kv_current_version_returns_zero_only_for_absent_path(monkeypatch):
+    client = OpenBaoClient(addr="http://example.invalid", token="", bao_bin="bao")
+    monkeypatch.setattr(
+        client,
+        "_run",
+        lambda _args: SimpleNamespace(
+            returncode=2, stdout="No value found at secret/metadata/example", stderr=""
+        ),
+    )
+    assert client.kv_current_version("secret", "example") == 0
+
+
+def test_approle_session_keeps_login_material_out_of_argv_and_revokes_self(monkeypatch):
+    client = OpenBaoClient(addr="http://example.invalid", token="parent", bao_bin="bao")
+    monkeypatch.setattr(client, "read_approle_role_id", lambda _role: "role-id-value")
+    monkeypatch.setattr(
+        client, "create_approle_secret_id", lambda _role: "secret-id-value"
+    )
+    captured = {}
+
+    def fake_json_call(args, payload):
+        captured["args"] = list(args)
+        captured["payload"] = dict(payload)
+        return json.dumps(
+            {"auth": {"client_token": "scoped-token", "accessor": "accessor-value"}}
+        )
+
+    monkeypatch.setattr(client, "_run_ok_with_json_file", fake_json_call)
+    session = client.create_approle_session("example-role")
+    revoke_calls = []
+    monkeypatch.setattr(
+        session.client,
+        "_run_ok",
+        lambda args, **_kwargs: revoke_calls.append(list(args)) or "",
+    )
+
+    assert "role-id-value" not in " ".join(captured["args"])
+    assert "secret-id-value" not in " ".join(captured["args"])
+    assert captured["payload"] == {
+        "role_id": "role-id-value",
+        "secret_id": "secret-id-value",
+    }
+    assert session.client.token == "scoped-token"
+    assert session.accessor_fingerprint
+
+    session.close()
+    session.close()
+    assert revoke_calls == [["token", "revoke", "-self"]]
+    assert session.client.token == ""
+    assert session.closed is True
+    assert session.evidence() == {
+        "session_handle": session.accessor_fingerprint,
+        "established": True,
+        "revocation_attempted": True,
+        "revocation_succeeded": True,
+    }
+    assert "accessor-value" not in json.dumps(session.evidence())
+    assert "scoped-token" not in json.dumps(session.evidence())
+
+
+def test_failed_session_revocation_is_visible_and_credential_is_dropped(monkeypatch):
+    client = OpenBaoClient(addr="http://example.invalid", token="issued", bao_bin="bao")
+    from secrets_engine.openbao import ScopedTokenSession
+
+    session = ScopedTokenSession(client=client, accessor_fingerprint="safe-handle")
+    monkeypatch.setattr(
+        client,
+        "_run_ok",
+        lambda *_args, **_kwargs: (_ for _ in ()).throw(BackendError("revoke failed")),
+    )
+
+    with pytest.raises(BackendError, match="revoke failed"):
+        session.close()
+
+    assert session.client.token == ""
+    assert session.closed is True
+    assert session.evidence() == {
+        "session_handle": "safe-handle",
+        "established": True,
+        "revocation_attempted": True,
+        "revocation_succeeded": False,
+    }
+
+
+def test_approle_session_context_revokes_on_failure(monkeypatch):
+    client = OpenBaoClient(addr="http://example.invalid", token="parent", bao_bin="bao")
+    session = SimpleNamespace(closed=False)
+
+    def close():
+        session.closed = True
+
+    session.close = close
+    monkeypatch.setattr(client, "create_approle_session", lambda _role: session)
+
+    with pytest.raises(RuntimeError, match="child failed"):
+        with client.approle_session("example-role"):
+            raise RuntimeError("child failed")
+    assert session.closed is True