From 3a1bd4f1c8af2a0efc4c0d62301726fb92dfd086 Mon Sep 17 00:00:00 2001 From: tegwick Date: Sun, 23 Aug 2026 12:05:58 +0200 Subject: [PATCH] Harden secret provisioning and lifecycle controls Assistant: codex Assistant-Model: gpt-5.6-sol Assistant-Session: 01a0217e-8c4c-7383-be6b-f50a6e485306 --- README.md | 3 +- SCOPE.md | 78 ++++--- docs/cli.md | 32 +++ docs/hardening-backlog.md | 7 +- policies/secrets-engine-build.hcl | 2 +- policies/secrets-engine-prod.hcl | 2 +- policies/secrets-engine-test.hcl | 2 +- src/secrets_engine/cli.py | 172 ++++++++++++--- src/secrets_engine/exec_delivery.py | 54 +++-- src/secrets_engine/lifecycle.py | 196 +++++++++++++++++ src/secrets_engine/openbao.py | 208 ++++++++++++++++-- src/secrets_engine/provision.py | 4 +- src/secrets_engine/roles.py | 2 +- src/secrets_engine/routing.py | 17 +- src/secrets_engine/verify.py | 80 ++++--- tests/test_auth_capability.py | 18 +- tests/test_exec_delivery.py | 51 +++++ tests/test_existing_lane_admission.py | 45 +++- tests/test_integration_bao.py | 31 +++ tests/test_lifecycle.py | 151 +++++++++++++ tests/test_multifield_readiness.py | 138 ++++++++++++ tests/test_openbao_safe_write.py | 171 ++++++++++++++ ...-WP-0007-production-lifecycle-hardening.md | 67 +++++- 23 files changed, 1369 insertions(+), 162 deletions(-) create mode 100644 src/secrets_engine/lifecycle.py create mode 100644 tests/test_lifecycle.py create mode 100644 tests/test_multifield_readiness.py create mode 100644 tests/test_openbao_safe_write.py diff --git a/README.md b/README.md index 5b247fc..f58e60a 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,8 @@ and production stages. OpenBao remains the custody and enforcement backend. `secrets-engine` owns the operator and agent interaction model: catalog, decision checks, plan/apply, -safe provisioning, verification, delivery, evidence, rotation, and deactivation. +guarded provisioning, verification, delivery, evidence, lifecycle metadata, and +native-access deactivation. ## Start Here diff --git a/SCOPE.md b/SCOPE.md index 45efdda..03562d9 100644 --- a/SCOPE.md +++ b/SCOPE.md @@ -48,25 +48,24 @@ provider, credential broker, or general secrets API. - Imports one declared field from a mode-0600 file outside Git worktrees. - Generates random values only for non-production lanes. - Keeps values out of CLI output and evidence records. - -This is a bootstrap/pilot primitive, not a safe general updater for existing -multi-field production paths. The current adapter invokes `bao kv put` with one -field and can replace sibling fields at that KV path; it also places the value -in the local `bao` subprocess argument vector. Do not use it for the admitted -shared multi-field lanes until merge-safe, non-argv provisioning is implemented. +- Uses a CAS-aware create/patch primitive. Secret data is passed through a + mode-0600 temporary JSON input reference that is removed in a `finally` path, + never as a raw subprocess argument. Existing-path updates preserve unmentioned + sibling fields and stale-version writes fail closed. ### Verification -- KV positive verification logs in through the lane AppRole and reports whether - one declared field is present, without printing the field value. +- KV positive verification logs in through the lane AppRole and reports field + presence without printing values. With no `--field`, it checks every declared + field; an explicit `--field` narrows the check. - KV negative verification checks that a fixed invalid token cannot read the path. - Auth-capability verification checks the AppRole token's capabilities on exact allowlisted and denial-probe paths. These are bounded policy/presence probes. They do not yet prove denial for a -real unrelated workload identity, validate every field by default, exercise a -provider operation, or produce OpenBao audit-request correlation. +real unrelated workload identity, exercise a provider operation, or produce +OpenBao audit-request correlation. ### Exec-time delivery @@ -82,8 +81,10 @@ provider operation, or produce OpenBao audit-request correlation. The OpenBao KV response is parsed in the parent process, so every field stored at the path crosses that process boundary even though only the selected field is -injected. Scoped AppRole tokens rely on configured TTL/use limits; exec and -verification do not explicitly revoke them after use. +injected. Exec and verification use scoped AppRole sessions that explicitly +self-revoke in a `finally` path. Non-secret evidence contains only an accessor +fingerprint and establishment/revocation outcome; TTL/use limits remain cleanup +backstops. ### Auth-capability handoff @@ -100,24 +101,34 @@ cannot be recovered through that AppRole. ### Routing and evidence - `route` returns a non-secret pointer containing lane ownership, decision - status, metadata/value-presence booleans, readiness, and a safe next command. + status, metadata/value-presence booleans, missing declared field names, + readiness, and a safe next command. Every declared field must be present. - Records scrubbed local JSONL evidence and posts a minimal State Hub progress event on a best-effort basis. - Keeps OpenBao audit logs as the backend source of truth. -Route readiness checks only the first declared KV field, and State Hub evidence -delivery is not durable or transactional. Treat route output as operational -guidance, not complete attestation for multi-field or high-risk lanes. +State Hub evidence delivery is not durable or transactional, and route does not +replace authorization or unrelated-identity denial evidence. Treat route output +as operational guidance, not complete attestation for high-risk lanes. ### Revocation currently available -- For `auth-capability` lanes, live `revoke` deletes the AppRole and policy. -- For KV lanes, live `revoke` deletes all KV metadata/versions at the path. +- For engine-managed native auth, live `revoke` deletes the AppRole and policy. +- For KV lanes, ordinary revoke explicitly preserves all KV metadata/versions. +- Externally managed delivery auth and workload delivery are reported as + preserved and are not mutated. +- `lifecycle suspend` removes only the managed AppRole and preserves its policy + for reviewed re-apply. +- `lifecycle deactivate` removes managed native AppRole/policy objects while + preserving KV custody. +- `lifecycle destroy --dry-run` renders managed-auth removal plus irreversible + KV metadata deletion. Live destroy is currently disabled even with exact + catalog-id confirmation; it will remain closed until the canonical + exact-action approval contract in `SECRETS-WP-0007-T04` is enforced. -KV revoke is destructive decommissioning, not soft deactivation: it currently -leaves the KV lane's consumer AppRole and policy in place. There is no general -lease/accessor revoke, rotation command, compromised state, or reversible lane -state machine. +These operations do not manage external workload delivery. There is currently +no general lease/accessor operator command, rotation command, compromised state, +or persistent/reversible lane state machine. ## CLI Surface @@ -133,6 +144,7 @@ secrets-engine exec secrets-engine policy publication secrets-engine route secrets-engine revoke [--dry-run] +secrets-engine lifecycle suspend|deactivate|destroy ``` The implemented exec adapters are `exec-env` and `npm-config`. `read-check` is @@ -156,9 +168,10 @@ verification, `approle-login` is auth-capability handoff metadata, and - A service API, daemon, UI, queue, scheduler, or remote multi-user service. - OpenBao OIDC/service-auth login for steady-state secrets-engine operation. - Native `exec-file` or response-wrapped delivery. -- Merge-safe multi-field KV updates or provider-side rotation. -- First-class rotate, compromise, suspend, reactivate, lease-status, or audit - report commands. +- Provider-side rotation or coordinated multi-consumer rollout. +- First-class rotate, compromise, reactivate, lease-status, or audit report + commands; lifecycle operations currently execute plans without persistent + lane state. - Dual-control enforcement beyond accepting the catalog label. - Direct flex-auth evaluation, claim validation, or identity authentication. - Runtime tenancy isolation; `org`, `repo`, consumers, and stages are catalog @@ -194,9 +207,10 @@ Canonical boundary: for a new production auth surface. - Never mutate an existing shared mount or replace workload delivery by implication. -- Never use current single-field provisioning on a shared multi-field path. -- Never call KV `revoke` unless destructive deletion of all path versions is the - explicitly approved intent. +- Never add KV destruction back to ordinary `revoke`; irreversible custody + destruction requires `lifecycle destroy`, exact confirmation, and a distinct + approved action. The live path remains disabled until that approval contract + exists. - Keep bootstrap and handoff material outside repositories with mode 0600 and explicit expiry/revocation handling. @@ -222,8 +236,9 @@ keywords: [secrets, openbao, policy, approle, catalog, decision, stage, least-pr type: security title: AppRole-scoped exec delivery description: Fetches one declared KV field through a lane AppRole and injects it into a child - process using exec-env or a temporary npm config, with output redaction and cleanup. This is - CLI-local delivery; exec-file, wrapping, service delivery, and explicit token revocation are absent. + process using exec-env or a temporary npm config, with output redaction, explicit scoped-token + self-revocation, and non-secret cleanup evidence. This is CLI-local delivery; exec-file, wrapping, + and service delivery are absent. keywords: [secrets, delivery, exec, npm, injection, redaction, openbao] ``` @@ -239,6 +254,7 @@ keywords: [openbao, approle, auth-capability, handoff, least-privilege] type: governance title: Non-secret routing and evidence pointers description: Reports decision/readiness metadata and records scrubbed local and best-effort State Hub - evidence without returning secret values. It is not a durable audit store or complete multi-field attestation. + evidence without returning secret values. Readiness covers every declared field, but this is not a + durable audit store or a real-unrelated-identity denial attestation. keywords: [routing, evidence, state-hub, audit, secrets] ``` diff --git a/docs/cli.md b/docs/cli.md index 7d86fc4..2f411fb 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -54,6 +54,9 @@ secrets-engine exec --catalog [--field NAME] [--mode auto|npm-confi secrets-engine policy publication secrets-engine route [--json] secrets-engine revoke [--dry-run] +secrets-engine lifecycle suspend [--dry-run] +secrets-engine lifecycle deactivate [--dry-run] +secrets-engine lifecycle destroy [--dry-run] [--confirm-destroy ] ``` `policy publication` resolves a lane's effective publication scope and the env @@ -70,6 +73,35 @@ fresh AppRole `secret_id` and writes `role_id` plus `secret_id` to caller-chosen mode-0600 files outside Git worktrees. It never prints the `secret_id`; use the resulting files only for attended out-of-band delivery. +`revoke` currently means **deactivate native secrets-engine access**. For +engine-managed delivery auth it deletes the lane AppRole and policy; it always +preserves KV metadata/values, existing externally managed auth, and workload +delivery. There is no general KV-destruction command. `provision` uses +CAS-aware create/patch behavior with a strict temporary input reference: values +are absent from argv, sibling fields are preserved, and stale writes fail. + +With no `--field`, `verify` checks every declared KV field positively and runs +one path-level negative probe. `route` likewise requires every declared field +and reports only missing field names. An explicit `--field` narrows positive +verification; production use will bind such subsets to the action approval. + +Explicit lifecycle commands separate intent: + +- `suspend` removes the engine-managed AppRole but preserves policy and KV; +- `deactivate` removes the engine-managed AppRole and policy but preserves KV; +- `destroy --dry-run` renders deactivation plus irreversible KV metadata + deletion. Live execution is fail-closed even with exact catalog-id + confirmation until `SECRETS-WP-0007-T04` supplies the canonical exact-action + approval contract. + +All three preserve externally managed auth and workload delivery. The legacy +`revoke` command is a compatibility alias for safe native deactivation, never +KV destruction. + +Exec and verification AppRole logins are scoped sessions. The issued token +self-revokes on every exit path before exec starts (or when verification ends), +and evidence stores only a short accessor fingerprint plus cleanup outcome. + ## Exit codes | Code | Meaning | diff --git a/docs/hardening-backlog.md b/docs/hardening-backlog.md index a7d36db..4a6526d 100644 --- a/docs/hardening-backlog.md +++ b/docs/hardening-backlog.md @@ -49,8 +49,11 @@ contents in this repo. ## H4 — Rotation & lifecycle states -- Implement `rotate`, and explicit `compromised` / `deactivated` lane states with - evidence, beyond the current `revoke` (metadata delete). +- Implement `rotate` and persistent `compromised` / `deactivated` lane states + with evidence. Explicit suspend/deactivate/destroy plans now exist, and + ordinary `revoke` safely aliases native AppRole/policy deactivation, but lane + state and coordinated provider/workload rotation remain outstanding. Live + destroy remains disabled until exact-action authorization is available. ## H5 — Audit report command diff --git a/policies/secrets-engine-build.hcl b/policies/secrets-engine-build.hcl index 1ca9c8d..94769b7 100644 --- a/policies/secrets-engine-build.hcl +++ b/policies/secrets-engine-build.hcl @@ -6,7 +6,7 @@ # KV v2 data + metadata under the build prefix. path "secret/data/build/*" { - capabilities = ["create", "read", "update", "delete"] + capabilities = ["create", "read", "update", "patch", "delete"] } path "secret/metadata/build/*" { capabilities = ["create", "read", "update", "delete", "list"] diff --git a/policies/secrets-engine-prod.hcl b/policies/secrets-engine-prod.hcl index ddaff5b..cf11be9 100644 --- a/policies/secrets-engine-prod.hcl +++ b/policies/secrets-engine-prod.hcl @@ -15,7 +15,7 @@ # Apply approved production lane values (provisioning) and metadata. # Owner-scoped prod lanes (e.g. whynot-design/...) — NOT under build/ or test/. path "secret/data/+/*" { - capabilities = ["create", "update"] + capabilities = ["create", "update", "patch"] } path "secret/metadata/+/*" { capabilities = ["create", "read", "update", "list"] diff --git a/policies/secrets-engine-test.hcl b/policies/secrets-engine-test.hcl index 7db51a3..8cc367a 100644 --- a/policies/secrets-engine-test.hcl +++ b/policies/secrets-engine-test.hcl @@ -5,7 +5,7 @@ # cannot administer sys/, auth/, or identity/, cannot act as root. path "secret/data/test/*" { - capabilities = ["create", "read", "update", "delete"] + capabilities = ["create", "read", "update", "patch", "delete"] } path "secret/metadata/test/*" { capabilities = ["create", "read", "update", "delete", "list"] diff --git a/src/secrets_engine/cli.py b/src/secrets_engine/cli.py index 0194cf1..3f4f3d6 100644 --- a/src/secrets_engine/cli.py +++ b/src/secrets_engine/cli.py @@ -12,6 +12,7 @@ Command surface (FR7): exec --catalog [--field NAME] [--mode auto|npm-config|exec-env] -- CMD... route [--json] revoke + lifecycle suspend|deactivate|destroy Every privileged action is decision-gated and writes non-secret evidence. `plan` and `apply --dry-run` never mutate OpenBao. @@ -214,13 +215,32 @@ def cmd_verify(cfg: Config, args) -> int: entry = get_entry(cfg.catalog_dir, args.catalog_id) decision = _require_lane_approval(cfg, entry) client = OpenBaoClient.resolve(cfg.bao_addr, bootstrap_token_file=args.bootstrap_token_file) - field = args.field or (entry.fields[0] if entry.fields else "") - if entry.stores_kv_value() and not field: + fields = [args.field] if args.field else list(entry.fields) + if entry.stores_kv_value() and not fields: from secrets_engine.errors import VerificationError raise VerificationError(f"lane '{entry.id}' has no field to verify") positive = args.positive or not args.negative negative = args.negative or not args.positive - results = run_verification(client, entry, field, positive=positive, negative=negative) + if entry.stores_kv_value(): + results = [] + if positive: + for field in fields: + results.extend( + run_verification( + client, entry, field, positive=True, negative=False + ) + ) + if negative: + # Denial is path-scoped, so one probe covers every field on this path. + results.extend( + run_verification( + client, entry, fields[0], positive=False, negative=True + ) + ) + else: + results = run_verification( + client, entry, "", positive=positive, negative=negative + ) rc = 0 for r in results: print(r.render()) @@ -287,10 +307,33 @@ def cmd_exec(cfg: Config, args) -> int: w.record("exec", result="attempt", catalog_id=entry.id, stage=entry.stage, decision_id=decision.id if decision else "", detail={"command": args.command[0], "mode": args.mode}) - rc = exec_with_secret(client, entry, field, args.command, mode=args.mode) + session_detail: dict[str, object] = {} + try: + rc = exec_with_secret( + client, + entry, + field, + args.command, + mode=args.mode, + session_evidence=session_detail, + ) + except SecretsEngineError as e: + w.record( + "exec", + result=f"failed-{type(e).__name__}", + catalog_id=entry.id, + stage=entry.stage, + decision_id=decision.id if decision else "", + detail={ + "command": args.command[0], + "mode": args.mode, + "session": session_detail, + }, + ) + raise w.record("exec", result=f"exit-{rc}", catalog_id=entry.id, stage=entry.stage, decision_id=decision.id if decision else "", - detail={"command": args.command[0]}) + detail={"command": args.command[0], "session": session_detail}) return rc @@ -338,36 +381,81 @@ def cmd_route(cfg: Config, args) -> int: def cmd_revoke(cfg: Config, args) -> int: + from secrets_engine.lifecycle import ( + apply_lifecycle_plan, + build_native_deactivation_plan, + ) + entry = get_entry(cfg.catalog_dir, args.catalog_id) decision = None if args.dry_run else _require_lane_approval(cfg, entry) - client = OpenBaoClient.resolve(cfg.bao_addr, bootstrap_token_file=args.bootstrap_token_file) - if entry.kind == "auth-capability": - if args.dry_run: - print( - f"(dry-run) would delete approle {entry.role_name} " - f"and policy {entry.policy_name}" - ) - return 0 - client.delete_approle(entry.role_name) - client.delete_policy(entry.policy_name) - print( - f"revoked lane '{entry.id}': deleted approle {entry.role_name} " - f"and policy {entry.policy_name}" - ) - _writer(cfg).record( - "revoke", result="auth-capability-deactivated", catalog_id=entry.id, - stage=entry.stage, decision_id=decision.id if decision else "" - ) - return 0 + plan = build_native_deactivation_plan(entry) if args.dry_run: - print(f"(dry-run) would delete KV metadata {entry.mount}/{entry.path} " - f"and approle {entry.role_name}") + print(plan.render()) + print("\n(dry-run: no OpenBao mutation performed)") return 0 - client.kv_delete_metadata(entry.mount, entry.path) - print(f"revoked lane '{entry.id}': KV metadata deleted at {entry.mount}/{entry.path}") + client = OpenBaoClient.resolve( + cfg.bao_addr, bootstrap_token_file=args.bootstrap_token_file + ) + result = apply_lifecycle_plan(client, plan) + print(plan.render()) + print(result.render()) _writer(cfg).record( - "revoke", result="deactivated", catalog_id=entry.id, stage=entry.stage, - decision_id=decision.id if decision else "" + "revoke", + result="native-access-deactivated", + catalog_id=entry.id, + stage=entry.stage, + decision_id=decision.id if decision else "", + detail={ + "operation": plan.operation, + "applied": list(result.applied), + "preserved": list(result.preserved), + }, + ) + return 0 + + +def cmd_lifecycle(cfg: Config, args) -> int: + from secrets_engine.lifecycle import ( + apply_lifecycle_plan, + build_lifecycle_plan, + require_destroy_confirmation, + ) + + entry = get_entry(cfg.catalog_dir, args.catalog_id) + if args.operation == "destroy" and not args.dry_run: + require_destroy_confirmation(entry, args.confirm_destroy) + # A lane-level approval is not an authorization to erase custody. Keep + # the destructive live path closed until T04 supplies a canonical, + # exact-action, expiring and dual-control-capable decision contract. + from secrets_engine.errors import PolicyGuardError + + raise PolicyGuardError( + "live destroy is disabled until an exact-action destruction " + "approval contract is available; use --dry-run to inspect targets" + ) + decision = None if args.dry_run else _require_lane_approval(cfg, entry) + plan = build_lifecycle_plan(entry, args.operation) + if args.dry_run: + print(plan.render()) + print("\n(dry-run: no OpenBao mutation performed)") + return 0 + client = OpenBaoClient.resolve( + cfg.bao_addr, bootstrap_token_file=args.bootstrap_token_file + ) + result = apply_lifecycle_plan(client, plan) + print(plan.render()) + print(result.render()) + _writer(cfg).record( + f"lifecycle-{args.operation}", + result="applied", + catalog_id=entry.id, + stage=entry.stage, + decision_id=decision.id if decision else "", + detail={ + "operation": plan.operation, + "applied": list(result.applied), + "preserved": list(result.preserved), + }, ) return 0 @@ -455,12 +543,34 @@ def build_parser() -> argparse.ArgumentParser: ro.add_argument("--json", action="store_true") ro.set_defaults(func=cmd_route) - rv = sub.add_parser("revoke", help="deactivate a lane (delete KV metadata)") + rv = sub.add_parser( + "revoke", + help="deactivate native policy/AppRole access (preserves KV custody)", + ) rv.add_argument("catalog_id") rv.add_argument("--dry-run", action="store_true") add_token_arg(rv) rv.set_defaults(func=cmd_revoke) + lc = sub.add_parser("lifecycle", help="explicit lane lifecycle operations") + lcsub = lc.add_subparsers(dest="operation", required=True) + for operation, help_text in ( + ("suspend", "stop new native logins; preserve policy and KV custody"), + ("deactivate", "remove native AppRole/policy; preserve KV custody"), + ("destroy", "irreversibly delete KV metadata after deactivation"), + ): + lp = lcsub.add_parser(operation, help=help_text) + lp.add_argument("catalog_id") + lp.add_argument("--dry-run", action="store_true") + if operation == "destroy": + lp.add_argument( + "--confirm-destroy", + default="", + help="exact catalog id required for live irreversible deletion", + ) + add_token_arg(lp) + lp.set_defaults(func=cmd_lifecycle) + return p diff --git a/src/secrets_engine/exec_delivery.py b/src/secrets_engine/exec_delivery.py index c202f44..3a601e7 100644 --- a/src/secrets_engine/exec_delivery.py +++ b/src/secrets_engine/exec_delivery.py @@ -50,27 +50,45 @@ def resolve_npm_token_env(entry: CatalogEntry, *, policy_dir=None) -> str: return res.token_env -def _fetch_value(client: OpenBaoClient, entry: CatalogEntry, field: str) -> str: +def _fetch_value( + client: OpenBaoClient, + entry: CatalogEntry, + field: str, + *, + session_evidence: dict[str, object] | None = None, +) -> str: """Read the field value via an approle-scoped token. Held in memory only.""" if entry.delivery_auth_method != "approle" or not entry.has_delivery_auth: raise DeliveryError( f"lane '{entry.id}' has no AppRole delivery auth for native exec" ) + session = None try: - token = client.approle_login_token(entry.role_name) + with client.approle_session(entry.role_name) as session: + proc = session.client._run( + ["kv", "get", "-format=json", f"{entry.mount}/{entry.path}"] + ) + if proc.returncode != 0: + raise DeliveryError( + f"scoped read failed for lane '{entry.id}' (denied or absent)" + ) + try: + data = json.loads(proc.stdout)["data"]["data"] + except (json.JSONDecodeError, KeyError) as e: + raise DeliveryError(f"malformed KV response for lane '{entry.id}'") from e + if field not in data: + raise DeliveryError(f"field '{field}' absent in lane '{entry.id}'") + return data[field] + except DeliveryError: + raise except Exception as e: - raise DeliveryError(f"could not obtain scoped token for delivery: {e}") from e - scoped = OpenBaoClient(addr=client.addr, token=token, bao_bin=client.bao_bin) - proc = scoped._run(["kv", "get", "-format=json", f"{entry.mount}/{entry.path}"]) - if proc.returncode != 0: - raise DeliveryError(f"scoped read failed for lane '{entry.id}' (denied or absent)") - try: - data = json.loads(proc.stdout)["data"]["data"] - except (json.JSONDecodeError, KeyError) as e: - raise DeliveryError(f"malformed KV response for lane '{entry.id}'") from e - if field not in data: - raise DeliveryError(f"field '{field}' absent in lane '{entry.id}'") - return data[field] + raise DeliveryError(f"scoped delivery session failed: {e}") from e + finally: + if session_evidence is not None: + if session is not None and hasattr(session, "evidence"): + session_evidence.update(session.evidence()) + else: + session_evidence.setdefault("established", session is not None) def _registry_authkey(registry: str) -> str: @@ -123,6 +141,7 @@ def exec_with_secret( *, mode: str = "auto", policy_dir=None, + session_evidence: dict[str, object] | None = None, ) -> int: """Run `command` with the lane's secret injected for the child only. @@ -151,7 +170,12 @@ def exec_with_secret( f"(allowed {sorted(declared)})" ) - value = _fetch_value(client, entry, field) + if session_evidence is None: + value = _fetch_value(client, entry, field) + else: + value = _fetch_value( + client, entry, field, session_evidence=session_evidence + ) child_env = dict(os.environ) if mode == "npm-config": diff --git a/src/secrets_engine/lifecycle.py b/src/secrets_engine/lifecycle.py new file mode 100644 index 0000000..7e2ffb5 --- /dev/null +++ b/src/secrets_engine/lifecycle.py @@ -0,0 +1,196 @@ +"""Non-destructive lifecycle plans for native secrets-engine access. + +The legacy KV revoke path deleted all KV metadata while leaving delivery auth +behind. That is not revocation and is not reversible. Ordinary revoke now +means *deactivate native access*: remove only policy/AppRole objects managed by +secrets-engine and preserve OpenBao custody plus externally managed workload +delivery. +""" +from __future__ import annotations + +from dataclasses import dataclass + +from secrets_engine.catalog import CatalogEntry +from secrets_engine.errors import PolicyGuardError +from secrets_engine.openbao import OpenBaoClient + +LIFECYCLE_OPERATIONS = ("suspend", "deactivate", "destroy") + + +@dataclass(frozen=True) +class LifecycleAction: + kind: str + target: str + mutation: bool + reason: str + mount: str = "" + path: str = "" + + def render(self) -> str: + mode = "mutation" if self.mutation else "no mutation" + return f" [{self.kind}] {self.target} ({mode}; {self.reason})" + + +@dataclass(frozen=True) +class LifecyclePlan: + catalog_id: str + operation: str + actions: tuple[LifecycleAction, ...] + + def render(self) -> str: + lines = [ + f"Lifecycle plan for lane '{self.catalog_id}'", + f" operation: {self.operation}", + " actions:", + ] + lines.extend(action.render() for action in self.actions) + return "\n".join(lines) + + +@dataclass(frozen=True) +class LifecycleResult: + applied: tuple[str, ...] + preserved: tuple[str, ...] + + def render(self) -> str: + lines = [f" applied: {target}" for target in self.applied] + lines.extend(f" preserved: {target}" for target in self.preserved) + return "\n".join(lines) or " (nothing to do)" + + +def build_lifecycle_plan(entry: CatalogEntry, operation: str) -> LifecyclePlan: + """Plan an explicit lifecycle operation with no hidden mutations.""" + if operation not in LIFECYCLE_OPERATIONS: + raise PolicyGuardError( + f"unknown lifecycle operation '{operation}'; allowed {LIFECYCLE_OPERATIONS}" + ) + if operation == "destroy" and not entry.stores_kv_value(): + raise PolicyGuardError( + f"lane '{entry.id}' is {entry.kind}; it has no KV custody to destroy" + ) + actions: list[LifecycleAction] = [] + if entry.manages_delivery_auth: + actions.append( + LifecycleAction( + "delete-approle", + entry.role_name, + True, + "stop new native delivery logins", + ) + ) + if operation in {"deactivate", "destroy"}: + actions.append( + LifecycleAction( + "delete-policy", + entry.policy_name, + True, + "remove native consumer policy", + ) + ) + else: + actions.append( + LifecycleAction( + "preserve-policy", + entry.policy_name, + False, + "suspension keeps policy for reviewed rollback", + ) + ) + elif entry.has_delivery_auth: + actions.extend( + ( + LifecycleAction( + "preserve-external-approle", + entry.role_name, + False, + "delivery auth is externally managed", + ), + LifecycleAction( + "preserve-external-policy", + entry.policy_name, + False, + "delivery auth is externally managed", + ), + ) + ) + else: + actions.append( + LifecycleAction( + "no-native-delivery-auth", + entry.id, + False, + "lane declares no native delivery auth", + ) + ) + + if entry.stores_kv_value(): + if operation == "destroy": + actions.append( + LifecycleAction( + "delete-kv-metadata", + f"{entry.mount}/{entry.path}", + True, + "irreversibly delete all KV versions and metadata", + mount=entry.mount, + path=entry.path, + ) + ) + else: + actions.append( + LifecycleAction( + "preserve-kv-custody", + f"{entry.mount}/{entry.path}", + False, + "suspend/deactivate never delete KV metadata or values", + ) + ) + if entry.workload_delivery: + actions.append( + LifecycleAction( + "preserve-workload-delivery", + entry.id, + False, + "workload delivery is externally managed", + ) + ) + return LifecyclePlan( + catalog_id=entry.id, + operation=operation, + actions=tuple(actions), + ) + + +def build_native_deactivation_plan(entry: CatalogEntry) -> LifecyclePlan: + """Compatibility plan for ordinary revoke: safe native deactivation.""" + return build_lifecycle_plan(entry, "deactivate") + + +def require_destroy_confirmation(entry: CatalogEntry, confirmation: str) -> None: + """Require an exact lane id before an irreversible custody operation.""" + if confirmation != entry.id: + raise PolicyGuardError( + "destroy requires --confirm-destroy with the exact catalog id; " + "no custody mutation performed" + ) + + +def apply_lifecycle_plan( + client: OpenBaoClient, plan: LifecyclePlan +) -> LifecycleResult: + """Apply exactly the mutations described by a lifecycle plan.""" + applied: list[str] = [] + preserved: list[str] = [] + for action in plan.actions: + if not action.mutation: + preserved.append(action.target) + continue + if action.kind == "delete-approle": + client.delete_approle(action.target) + elif action.kind == "delete-policy": + client.delete_policy(action.target) + elif action.kind == "delete-kv-metadata": + client.kv_delete_metadata(action.mount, action.path) + else: # defensive: a rendered plan must not grow hidden mutations + raise ValueError(f"unsupported lifecycle mutation '{action.kind}'") + applied.append(action.target) + return LifecycleResult(applied=tuple(applied), preserved=tuple(preserved)) diff --git a/src/secrets_engine/openbao.py b/src/secrets_engine/openbao.py index 38cea7b..7cf881a 100644 --- a/src/secrets_engine/openbao.py +++ b/src/secrets_engine/openbao.py @@ -19,6 +19,8 @@ import os import shutil import stat import subprocess +import tempfile +from contextlib import contextmanager from dataclasses import dataclass from pathlib import Path @@ -49,6 +51,39 @@ def _check_token_file(path: Path) -> str: return token +@dataclass +class ScopedTokenSession: + """One AppRole login token that revokes itself on close.""" + + client: "OpenBaoClient" + accessor_fingerprint: str + closed: bool = False + revocation_attempted: bool = False + revocation_succeeded: bool = False + + def evidence(self) -> dict[str, object]: + """Return non-secret lifecycle evidence for this issued session.""" + return { + "session_handle": self.accessor_fingerprint, + "established": True, + "revocation_attempted": self.revocation_attempted, + "revocation_succeeded": self.revocation_succeeded, + } + + def close(self) -> None: + if self.closed: + return + self.revocation_attempted = True + try: + self.client._run_ok(["token", "revoke", "-self"]) + self.revocation_succeeded = True + finally: + # Drop the credential from the reusable client object even when + # backend cleanup fails; TTL/use limits remain the final backstop. + self.client.token = "" + self.closed = True + + @dataclass class OpenBaoClient: addr: str @@ -104,6 +139,25 @@ class OpenBaoClient: ) return proc.stdout + def _run_ok_with_json_file(self, args: list[str], payload: dict) -> str: + """Run a CLI operation with JSON from a strict temporary file reference.""" + fd, temp_name = tempfile.mkstemp(prefix="se-bao-input-", suffix=".json") + temp_path = Path(temp_name) + try: + os.fchmod(fd, 0o600) + with os.fdopen(fd, "w", encoding="utf-8") as fh: + fd = -1 + json.dump(payload, fh) + fh.write("\n") + return self._run_ok([*args, f"@{temp_path}"]) + finally: + if fd >= 0: + os.close(fd) + try: + temp_path.unlink() + except FileNotFoundError: + pass + # -- health / capabilities -------------------------------------------- def is_reachable(self) -> bool: @@ -188,21 +242,43 @@ class OpenBaoClient: ["write", "-field=secret_id", "-f", f"auth/approle/role/{role_name}/secret-id"] ).strip() - def approle_login_token(self, role_name: str) -> str: - """Login as the approle and return a scoped child token. Used only for - verification / exec delivery; never logged.""" + def create_approle_session(self, role_name: str) -> ScopedTokenSession: + """Login through AppRole without putting role/secret ids in argv.""" role_id = self.read_approle_role_id(role_name) secret_id = self.create_approle_secret_id(role_name) - token = self._run_ok( - [ - "write", - "-field=token", - "auth/approle/login", - f"role_id={role_id}", - f"secret_id={secret_id}", - ] - ).strip() - return token + try: + output = self._run_ok_with_json_file( + ["write", "-format=json", "auth/approle/login"], + {"role_id": role_id, "secret_id": secret_id}, + ) + finally: + del secret_id + try: + auth = json.loads(output)["auth"] + token = str(auth["client_token"]) + accessor = str(auth.get("accessor", "")) + except (json.JSONDecodeError, KeyError, TypeError) as e: + raise BackendError("malformed AppRole login response") from e + if not token: + raise BackendError("AppRole login returned an empty token") + import hashlib + + fingerprint = hashlib.sha256(accessor.encode("utf-8")).hexdigest()[:12] + scoped = OpenBaoClient(addr=self.addr, token=token, bao_bin=self.bao_bin) + del token + return ScopedTokenSession( + client=scoped, + accessor_fingerprint=fingerprint, + ) + + @contextmanager + def approle_session(self, role_name: str): + """Yield a scoped client and revoke its token on every exit path.""" + session = self.create_approle_session(role_name) + try: + yield session + finally: + session.close() def token_capabilities(self, path: str, *, token: str) -> list[str]: """Return token capabilities for a path without returning any secret value.""" @@ -253,33 +329,117 @@ class OpenBaoClient: return raise BackendError(f"could not enable kv at {mount}: {stderr.strip()}") - def kv_put(self, mount: str, path: str, field: str, value: str) -> None: - """Write a single field. `value` is a secret and is passed via stdin-free - argv only as a key=value to the local CLI; it is never logged or returned.""" - self._run_ok(["kv", "put", f"{mount}/{path}", f"{field}={value}"]) + def kv_current_version(self, mount: str, path: str) -> int: + """Return current KV v2 version from metadata, or 0 when absent. + + Metadata contains no secret values. Permission/backend errors fail closed + rather than being misread as a new path. + """ + proc = self._run( + ["kv", "metadata", "get", "-format=json", f"-mount={mount}", path] + ) + if proc.returncode != 0: + message = f"{proc.stderr}\n{proc.stdout}".lower() + if "no value found" in message or "not found" in message or "404" in message: + return 0 + raise BackendError( + f"bao kv metadata get failed (exit {proc.returncode}): " + f"{proc.stderr.strip() or proc.stdout.strip()}" + ) + try: + version = json.loads(proc.stdout).get("data", {}).get("current_version", 0) + return int(version) + except (json.JSONDecodeError, TypeError, ValueError) as e: + raise BackendError("malformed KV metadata response") from e + + def kv_patch_fields( + self, + mount: str, + path: str, + values: dict[str, str], + *, + expected_version: int | None = None, + ) -> int: + """Atomically update declared fields without exposing values in argv. + + Existing paths use server-side HTTP PATCH with CAS, preserving every + unmentioned sibling. New paths use CAS=0 put. Values are written to a + mode-0600 temporary JSON input file referenced by path and removed in a + finally block. The returned integer is the version used as the CAS base. + """ + if not values: + raise BackendError("refusing empty KV field update") + if any(not isinstance(k, str) or not k for k in values): + raise BackendError("KV field update contains an invalid field name") + version = ( + self.kv_current_version(mount, path) + if expected_version is None + else expected_version + ) + if version < 0: + raise BackendError("expected KV version must be non-negative") + + if version == 0: + args = [ + "kv", + "put", + f"-mount={mount}", + "-cas=0", + path, + ] + else: + args = [ + "kv", + "patch", + f"-mount={mount}", + "-method=patch", + f"-cas={version}", + path, + ] + self._run_ok_with_json_file(args, values) + return version def kv_metadata_exists(self, mount: str, path: str) -> bool: proc = self._run(["kv", "metadata", "get", "-format=json", f"{mount}/{path}"]) return proc.returncode == 0 - def kv_field_present(self, mount: str, path: str, field: str, *, token: str | None = None) -> bool: - """Return whether `field` exists at the path — WITHOUT returning its value. + def kv_fields_present( + self, + mount: str, + path: str, + fields: list[str] | tuple[str, ...], + *, + token: str | None = None, + ) -> dict[str, bool]: + """Return declared-field presence after one read, never field values. - If `token` is given, the read is attempted as that (scoped) token, so a - True/False result doubles as a positive/negative access check. + If ``token`` is given, the read is attempted as that scoped token. A + denied or malformed read reports every requested field as absent; no + response data is returned to the caller. """ + requested = tuple(dict.fromkeys(fields)) + if not requested: + return {} client = self if token is not None: client = OpenBaoClient(addr=self.addr, token=token, bao_bin=self.bao_bin) proc = client._run(["kv", "get", "-format=json", f"{mount}/{path}"]) if proc.returncode != 0: - return False + return {field: False for field in requested} try: doc = json.loads(proc.stdout) except json.JSONDecodeError: - return False + return {field: False for field in requested} data = doc.get("data", {}).get("data", {}) - return field in data and bool(data[field]) + if not isinstance(data, dict): + return {field: False for field in requested} + return {field: field in data and bool(data[field]) for field in requested} + + def kv_field_present( + self, mount: str, path: str, field: str, *, token: str | None = None + ) -> bool: + """Compatibility wrapper for a single non-secret presence result.""" + return self.kv_fields_present(mount, path, [field], token=token)[field] def kv_can_read(self, mount: str, path: str, *, token: str) -> bool: """True iff `token` is permitted to read the path at all (no value used).""" diff --git a/src/secrets_engine/provision.py b/src/secrets_engine/provision.py index 255ce75..8d2b02f 100644 --- a/src/secrets_engine/provision.py +++ b/src/secrets_engine/provision.py @@ -57,7 +57,7 @@ def provision_from_file( value = _read_value_file(Path(file_path)) if entry.manages_mount: client.ensure_kv_mount(entry.mount) - client.kv_put(entry.mount, entry.path, field, value) + client.kv_patch_fields(entry.mount, entry.path, {field: value}) del value return field @@ -81,6 +81,6 @@ def provision_generated(client: OpenBaoClient, entry: CatalogEntry, field: str) value = "test-" + "".join(_secrets.choice(alphabet) for _ in range(32)) if entry.manages_mount: client.ensure_kv_mount(entry.mount) - client.kv_put(entry.mount, entry.path, field, value) + client.kv_patch_fields(entry.mount, entry.path, {field: value}) del value return field diff --git a/src/secrets_engine/roles.py b/src/secrets_engine/roles.py index e4a3a02..35d0500 100644 --- a/src/secrets_engine/roles.py +++ b/src/secrets_engine/roles.py @@ -24,7 +24,7 @@ STAGE_PREFIX = { } # Capabilities a stage role's *own* policy may carry. Anything else is broad. -ALLOWED_CAPABILITIES = {"create", "read", "update", "delete", "list"} +ALLOWED_CAPABILITIES = {"create", "read", "update", "patch", "delete", "list"} # Auth-capability lanes grant an operational action, not KV value access. Keep # this intentionally smaller than the stage-role capability set. diff --git a/src/secrets_engine/routing.py b/src/secrets_engine/routing.py index 8abbe8d..e47fc52 100644 --- a/src/secrets_engine/routing.py +++ b/src/secrets_engine/routing.py @@ -27,6 +27,7 @@ class RouteResult: review_url: str metadata_applied: bool value_present: bool + missing_fields: list[str] ready: bool next_command: str missing: str @@ -59,16 +60,17 @@ def route_lane( metadata_applied = False value_present = False + missing_fields: list[str] = list(entry.fields) if entry.stores_kv_value() else [] if client is not None and client.is_reachable(): if entry.has_delivery_auth: policy_applied = client.read_policy(entry.policy_name) is not None role_applied = client.approle_exists(entry.role_name) metadata_applied = policy_applied and role_applied if entry.stores_kv_value(): - # Presence check uses the engine's own token; reports boolean only. - field = entry.fields[0] if entry.fields else "" - if field: - value_present = client.kv_field_present(entry.mount, entry.path, field) + # One read produces booleans only; readiness requires every field. + presence = client.kv_fields_present(entry.mount, entry.path, entry.fields) + missing_fields = [field for field in entry.fields if not presence.get(field)] + value_present = bool(entry.fields) and not missing_fields else: # Auth-capability lanes have no stored value; a fresh secret_id is minted # on demand through the handoff command once metadata exists. @@ -99,10 +101,12 @@ def route_lane( "--role-id-file --secret-id-file " ) elif not value_present: - missing = "provisioned secret value" + names = ", ".join(missing_fields) + missing = f"provisioned secret fields: {names}" + next_field = missing_fields[0] if missing_fields else entry.fields[0] next_command = ( f"secrets-engine provision {entry.id} --stage {entry.stage} " - f"--field {entry.fields[0]} --from-file " + f"--field {next_field} --from-file " ) else: missing = "" @@ -123,6 +127,7 @@ def route_lane( review_url=review_url, metadata_applied=metadata_applied, value_present=value_present, + missing_fields=missing_fields, ready=ready, next_command=next_command, missing=missing, diff --git a/src/secrets_engine/verify.py b/src/secrets_engine/verify.py index 5bbd178..1f7f0f0 100644 --- a/src/secrets_engine/verify.py +++ b/src/secrets_engine/verify.py @@ -27,6 +27,16 @@ class VerifyResult: return f" {self.check} check: {status} ({self.detail.get('reason', '')})" +def _session_evidence(session: object | None) -> dict[str, Any]: + """Extract only the session's explicit non-secret lifecycle record.""" + if session is None: + return {"established": False} + evidence = getattr(session, "evidence", None) + if callable(evidence): + return evidence() + return {"established": True} + + def verify_positive(client: OpenBaoClient, entry: CatalogEntry, field: str) -> VerifyResult: """Approved consumer token must be able to read the field.""" if entry.delivery_auth_method != "approle" or not entry.has_delivery_auth: @@ -35,15 +45,20 @@ def verify_positive(client: OpenBaoClient, entry: CatalogEntry, field: str) -> V False, {"reason": "lane has no AppRole delivery auth", "path": entry.path}, ) + session = None try: - token = client.approle_login_token(entry.role_name) + with client.approle_session(entry.role_name) as session: + present = session.client.kv_field_present(entry.mount, entry.path, field) except Exception as e: # backend errors -> failed verification, not a value leak return VerifyResult( "positive", False, - {"reason": f"could not obtain approle token: {e}", "path": entry.path}, + { + "reason": f"scoped verification session failed: {e}", + "path": entry.path, + "session": _session_evidence(session), + }, ) - present = client.kv_field_present(entry.mount, entry.path, field, token=token) return VerifyResult( "positive", present, @@ -54,6 +69,7 @@ def verify_positive(client: OpenBaoClient, entry: CatalogEntry, field: str) -> V "path": entry.path, "field": field, "role": entry.role_name, + "session": _session_evidence(session), }, ) @@ -76,26 +92,26 @@ def verify_negative(client: OpenBaoClient, entry: CatalogEntry) -> VerifyResult: def verify_auth_capability_positive(client: OpenBaoClient, entry: CatalogEntry) -> VerifyResult: """Approved AppRole token must carry update on every exact allowed path.""" + missing: list[str] = [] + session = None try: - token = client.approle_login_token(entry.role_name) + with client.approle_session(entry.role_name) as session: + for path in entry.auth_allowed_paths: + caps = session.client.token_capabilities( + path, token=session.client.token + ) + if "update" not in caps: + missing.append(path) except Exception as e: return VerifyResult( "positive", False, - {"reason": f"could not obtain approle token: {e}", "role": entry.role_name}, + { + "reason": f"scoped verification session failed: {e}", + "role": entry.role_name, + "session": _session_evidence(session), + }, ) - missing: list[str] = [] - for path in entry.auth_allowed_paths: - try: - caps = client.token_capabilities(path, token=token) - except Exception as e: - return VerifyResult( - "positive", - False, - {"reason": f"could not inspect capabilities: {e}", "path": path}, - ) - if "update" not in caps: - missing.append(path) passed = not missing return VerifyResult( "positive", @@ -107,32 +123,33 @@ def verify_auth_capability_positive(client: OpenBaoClient, entry: CatalogEntry) "role": entry.role_name, "allowed_paths": sorted(entry.auth_allowed_paths), "missing_update": missing, + "session": _session_evidence(session), }, ) def verify_auth_capability_negative(client: OpenBaoClient, entry: CatalogEntry) -> VerifyResult: """Approved AppRole token must not gain update outside denial probes.""" + leaks: list[str] = [] + session = None try: - token = client.approle_login_token(entry.role_name) + with client.approle_session(entry.role_name) as session: + for path in entry.auth_denied_probe_paths: + caps = session.client.token_capabilities( + path, token=session.client.token + ) + if "update" in caps or "sudo" in caps or "root" in caps: + leaks.append(path) except Exception as e: return VerifyResult( "negative", False, - {"reason": f"could not obtain approle token: {e}", "role": entry.role_name}, + { + "reason": f"scoped verification session failed: {e}", + "role": entry.role_name, + "session": _session_evidence(session), + }, ) - leaks: list[str] = [] - for path in entry.auth_denied_probe_paths: - try: - caps = client.token_capabilities(path, token=token) - except Exception as e: - return VerifyResult( - "negative", - False, - {"reason": f"could not inspect capabilities: {e}", "path": path}, - ) - if "update" in caps or "sudo" in caps or "root" in caps: - leaks.append(path) passed = not leaks return VerifyResult( "negative", @@ -144,6 +161,7 @@ def verify_auth_capability_negative(client: OpenBaoClient, entry: CatalogEntry) "role": entry.role_name, "denied_probe_paths": entry.auth_denied_probe_paths, "leaks": leaks, + "session": _session_evidence(session), }, ) diff --git a/tests/test_auth_capability.py b/tests/test_auth_capability.py index 2b8481f..944dceb 100644 --- a/tests/test_auth_capability.py +++ b/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: diff --git a/tests/test_exec_delivery.py b/tests/test_exec_delivery.py index f575b6a..60899f6 100644 --- a/tests/test_exec_delivery.py +++ b/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) diff --git a/tests/test_existing_lane_admission.py b/tests/test_existing_lane_admission.py index 1d48782..67d6522 100644 --- a/tests/test_existing_lane_admission.py +++ b/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"}, + ) ] diff --git a/tests/test_integration_bao.py b/tests/test_integration_bao.py index ab8a02d..480f707 100644 --- a/tests/test_integration_bao.py +++ b/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") diff --git a/tests/test_lifecycle.py b/tests/test_lifecycle.py new file mode 100644 index 0000000..e3d47f4 --- /dev/null +++ b/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 diff --git a/tests/test_multifield_readiness.py b/tests/test_multifield_readiness.py new file mode 100644 index 0000000..8c9df52 --- /dev/null +++ b/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) diff --git a/tests/test_openbao_safe_write.py b/tests/test_openbao_safe_write.py new file mode 100644 index 0000000..f06e7be --- /dev/null +++ b/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 diff --git a/workplans/SECRETS-WP-0007-production-lifecycle-hardening.md b/workplans/SECRETS-WP-0007-production-lifecycle-hardening.md index 52a293b..e96c43e 100644 --- a/workplans/SECRETS-WP-0007-production-lifecycle-hardening.md +++ b/workplans/SECRETS-WP-0007-production-lifecycle-hardening.md @@ -4,7 +4,7 @@ type: workplan title: "Production-safe provisioning, authorization, and lifecycle hardening" domain: infotech repo: secrets-engine -status: ready +status: active owner: codex topic_slug: custodian created: "2026-08-23" @@ -79,10 +79,20 @@ must not expose the current bootstrap and lifecycle shortcuts as an API. ```task id: SECRETS-WP-0007-T01 -status: todo +status: done priority: high ``` +Completed 2026-08-23. During containment, existing multi-field provisioning +failed before file read or backend access. Ordinary `revoke` builds one +lifecycle plan used by both +dry-run and live execution, deletes only engine-managed native AppRole/policy +objects, and explicitly preserves KV custody, external delivery auth, and +workload delivery. No destructive KV command remains on the ordinary CLI. +Focused lifecycle, admission, auth-capability, and throwaway OpenBao integration +tests pass. The temporary multi-field rejection was then superseded by the +merge-safe primitive completed in T02. + Add immediate fail-closed guards before implementing replacement primitives: - reject `provision` on an existing multi-field lane; @@ -107,10 +117,18 @@ Acceptance: ```task id: SECRETS-WP-0007-T02 -status: todo +status: done priority: high ``` +Completed 2026-08-23. Replaced raw `field=value` argv writes with a CAS-aware +backend primitive: new paths use CAS-zero create; existing paths use server-side +HTTP patch with the observed metadata version. Values travel through a +mode-0600 temporary JSON reference removed in `finally`, and never appear in +argv. Unit tests inspect the full request/cleanup; throwaway OpenBao integration +proves sibling preservation and stale-CAS rejection. Stage policies now include +the narrow KV `patch` capability needed by this operation. + Replace the raw `field=value` subprocess call with a backend write primitive that never places the value in argv and safely updates one or more declared fields without removing unmentioned siblings. The design may use OpenBao CAS, @@ -135,10 +153,19 @@ Acceptance: ```task id: SECRETS-WP-0007-T03 -status: todo +status: progress priority: high ``` +Progress 2026-08-23. Added one lifecycle plan model shared by dry-run and live +execution. Ordinary `revoke` safely aliases native deactivation; suspend and +deactivate preserve KV custody and externally managed workload delivery. +Destroy has an unmistakable dry-run plan and exact-id confirmation, but its live +handler is deliberately fail-closed before coarse lane approval or backend +access until T04 supplies distinct action authorization. Scoped issued-session +self-revocation is implemented in T05; a general known-accessor operator command +remains outstanding. + Replace the overloaded `revoke` behavior with explicit lifecycle operations and plans. Define at least: @@ -171,10 +198,16 @@ Acceptance: ```task id: SECRETS-WP-0007-T04 -status: todo +status: wait priority: high ``` +Waiting 2026-08-23 on the canonical external contract rather than parsing +decision prose or inventing authorization ownership locally. Contract requests +were sent to State Hub (`24663321-0263-43fe-8d48-e9c7e06d7bb9`) and flex-auth +(`ef8ff95d-6b4e-46f8-b1a9-497d06cf7c9a`). Until resolved, local fixtures cannot +unlock the new destroy path and that live operation remains disabled. + Define and enforce the decision contract needed by production commands. A resolved approval must bind at least: @@ -209,10 +242,20 @@ Acceptance: ```task id: SECRETS-WP-0007-T05 -status: todo +status: progress priority: high ``` +Progress 2026-08-23. AppRole login material now travels through strict temporary +JSON input rather than argv. Exec and both verification kinds use a scoped +session that self-revokes in `finally`, drops the in-memory token even when +cleanup fails, and exposes only an accessor fingerprint plus cleanup booleans as +evidence. Tests cover success and exception cleanup, idempotence, failed revoke +visibility, and value/credential exclusion. Provider review requests were sent +to railiance-platform (`8f910aff-3a94-43c6-8805-eb9276e46fc0`) and key-cape +(`0627ca55-115e-43dc-b2f6-3195be3bc90d`); steady-state engine authentication +remains external-contract work. + Refactor AppRole login into a scoped session object that retains only the minimum non-secret lifecycle handle needed to revoke the issued token in a `finally` path. Apply it consistently to exec, KV verification, auth-capability @@ -240,10 +283,20 @@ Acceptance: ```task id: SECRETS-WP-0007-T06 -status: todo +status: progress priority: high ``` +Progress 2026-08-23. KV verification now checks every declared field by default, +with one path-level negative probe, and route readiness reads once and requires +all declared fields. Missing readiness evidence contains field names only. Tests +prove a missing sibling makes readiness false. The real unrelated-identity +negative contract, audit request correlation, durable evidence delivery, and +lane audit summary remain outstanding. + +The complete repository suite passes with 103 tests after these changes, +including throwaway OpenBao integration coverage. + Make verification and routing truthful for multi-field and high-risk lanes: - verify every declared field unless the exact approved subset is explicit;