diff --git a/ProductRequirementsDocument.md b/ProductRequirementsDocument.md index 2212ae0..caa0cfd 100644 --- a/ProductRequirementsDocument.md +++ b/ProductRequirementsDocument.md @@ -201,7 +201,7 @@ secrets-engine plan secrets-engine apply --stage secrets-engine provision --stage --from-file secrets-engine verify --positive -secrets-engine verify --negative +secrets-engine verify --negative --negative-token-file secrets-engine exec --catalog -- secrets-engine revoke ``` diff --git a/README.md b/README.md index f58e60a..9a82794 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,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, guarded provisioning, verification, delivery, evidence, lifecycle metadata, and -native-access deactivation. +native-access deactivation. Local evidence can be inspected through an +allowlisted per-lane `audit` summary without exposing record detail. ## Start Here diff --git a/SCOPE.md b/SCOPE.md index 03562d9..6ae7ca0 100644 --- a/SCOPE.md +++ b/SCOPE.md @@ -58,14 +58,17 @@ provider, credential broker, or general secrets API. - 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. +- KV negative verification requires a real unrelated token supplied through a + mode-0600 file outside Git and checks that it cannot read the path. Missing + unrelated identity material fails closed without a backend probe. - 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, exercise a provider operation, or produce -OpenBao audit-request correlation. +These are bounded policy/presence probes. A throwaway OpenBao integration test +proves an overlapping unrelated policy is detected. Production still needs a +reviewed owner/source for each unrelated identity; the engine does not mint or +select that identity itself. Verification does not yet exercise a provider +operation or produce OpenBao audit-request correlation. ### Exec-time delivery @@ -104,12 +107,19 @@ cannot be recovered through that AppRole. 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. + event on a best-effort basis. Each requested State Hub delivery receives an + append-only local `delivered`, `failed`, or `skipped-no-topic` companion + record. +- `audit` summarizes allowlisted local lane evidence: action/result counts, + canonical decision references, session cleanup outcomes, and State Hub + delivery outcomes. It never re-emits arbitrary evidence detail. - Keeps OpenBao audit logs as the backend source of truth. -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. +State Hub evidence delivery is not queued or transactional; the local receipt +makes failure visible but does not replay it. Route and audit do not replace +authorization, OpenBao audit logs, or unrelated-identity denial evidence. +Treat their output as operational guidance, not complete attestation for +high-risk lanes. ### Revocation currently available @@ -145,6 +155,7 @@ secrets-engine policy publication secrets-engine route secrets-engine revoke [--dry-run] secrets-engine lifecycle suspend|deactivate|destroy +secrets-engine audit [--json] ``` The implemented exec adapters are `exec-env` and `npm-config`. `read-check` is @@ -254,7 +265,9 @@ 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. Readiness covers every declared field, but this is not a - durable audit store or a real-unrelated-identity denial attestation. + evidence without returning secret values. Append-only delivery receipts expose State Hub failures, + and an allowlisted audit command summarizes lane operations and cleanup. KV verification can attest a + supplied real unrelated identity, but the engine does not own identity selection. This is not a durable + remote audit store. keywords: [routing, evidence, state-hub, audit, secrets] ``` diff --git a/docs/cli.md b/docs/cli.md index 2f411fb..5d6ca65 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -48,7 +48,7 @@ secrets-engine decision inspect secrets-engine plan --stage secrets-engine apply --stage [--dry-run] [--bootstrap-token-file F] secrets-engine provision --stage --field NAME (--from-file F | --generate) -secrets-engine verify [--field NAME] [--positive] [--negative] +secrets-engine verify [--field NAME] [--positive] [--negative] [--negative-token-file F] secrets-engine handoff --stage --role-id-file F --secret-id-file F secrets-engine exec --catalog [--field NAME] [--mode auto|npm-config|exec-env] -- CMD... secrets-engine policy publication @@ -57,6 +57,7 @@ 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 ] +secrets-engine audit [--json] ``` `policy publication` resolves a lane's effective publication scope and the env @@ -81,7 +82,9 @@ 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 +one path-level negative probe. KV denial requires `--negative-token-file` with a +real unrelated identity's mode-0600 token file outside Git; absence fails the +check without calling OpenBao. `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. @@ -102,6 +105,13 @@ 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. +`audit` is a read-only, local evidence summary. It reports action and result +counts, canonical decision references, session cleanup outcomes, and State Hub +delivery outcomes for one cataloged lane. Its parser allowlists those fields and +does not echo arbitrary JSONL detail. State Hub failures are recorded locally as +append-only companion receipts; they are visible but are not automatically +replayed. + ## Exit codes | Code | Meaning | @@ -177,7 +187,8 @@ secrets-engine apply whynot-design-npm-publish --stage prod secrets-engine provision whynot-design-npm-publish --stage prod \ --field npm_token --from-file ~/.secrets-engine/whynot.token # 5. prove access without printing the value -secrets-engine verify whynot-design-npm-publish --field npm_token --positive --negative +secrets-engine verify whynot-design-npm-publish --field npm_token --positive --negative \ + --negative-token-file /secure/path/unrelated.token # 6. publish with the token injected into the child only secrets-engine exec --catalog whynot-design-npm-publish -- npm publish ``` diff --git a/docs/hardening-backlog.md b/docs/hardening-backlog.md index 4a6526d..e40b76e 100644 --- a/docs/hardening-backlog.md +++ b/docs/hardening-backlog.md @@ -57,8 +57,10 @@ contents in this repo. ## H5 — Audit report command -- `secrets-engine audit ` summarizing non-secret evidence (decision, - applies, provisions, verifications, execs, revokes) for a lane. +- Implemented: `secrets-engine audit ` summarizes allowlisted local + action/result, decision, session-cleanup, and State Hub delivery evidence. +- Remaining: durable/queued State Hub delivery and OpenBao audit request-id + correlation. ## H6 — API service mode diff --git a/docs/whynot-design-real-publish-closeout.md b/docs/whynot-design-real-publish-closeout.md index 06ac506..ab0749a 100644 --- a/docs/whynot-design-real-publish-closeout.md +++ b/docs/whynot-design-real-publish-closeout.md @@ -70,9 +70,14 @@ BAO_ADDR=https://bao.coulomb.social \ BAO_ADDR=https://bao.coulomb.social \ secrets-engine verify whynot-design-npm-publish \ --field npm_token --positive --negative \ + --negative-token-file /secure/path/reviewed-unrelated.token \ --bootstrap-token-file ~/.secrets-engine/bootstrap/prod-whynot-design.token ``` +The unrelated token must represent a reviewed identity outside the lane policy; +it is not the bootstrap token and must be held in a separate mode-0600 file +outside Git. + 6. In the `whynot-design` repo, coordinate a real version bump. Run the publish through child-only injection: diff --git a/scripts/demo-e2e.sh b/scripts/demo-e2e.sh index afad8b9..dae5f5f 100755 --- a/scripts/demo-e2e.sh +++ b/scripts/demo-e2e.sh @@ -21,6 +21,7 @@ export BAO_ADDR="http://127.0.0.1:8270" export BAO_TOKEN="se-demo-root" # Keep secret material OUTSIDE the repo worktree (engine enforces this). TOKENFILE="$WORK/whynot.token" +NEGATIVE_TOKENFILE="$WORK/unrelated.token" cleanup() { set +e @@ -59,7 +60,15 @@ secrets-engine provision whynot-design-npm-publish --stage prod \ echo echo "### 4. verify positive (approved consumer can read) + negative (others denied)" -secrets-engine verify whynot-design-npm-publish --field npm_token --positive --negative | sed 's/^/ /' +"$BAO_BIN" policy write se-demo-unrelated - <<'EOF' >/dev/null +path "secret/data/prod/coulomb/whynot-design/npm" { + capabilities = ["deny"] +} +EOF +umask 077 +"$BAO_BIN" token create -field=token -policy=se-demo-unrelated > "$NEGATIVE_TOKENFILE" +secrets-engine verify whynot-design-npm-publish --field npm_token \ + --positive --negative --negative-token-file "$NEGATIVE_TOKENFILE" | sed 's/^/ /' echo echo "### 5. exec-time delivery: child sees the token via a temp npmrc; parent never does" diff --git a/src/secrets_engine/audit.py b/src/secrets_engine/audit.py new file mode 100644 index 0000000..b18ab82 --- /dev/null +++ b/src/secrets_engine/audit.py @@ -0,0 +1,167 @@ +"""Allowlisted, non-secret summaries over local append-only evidence.""" +from __future__ import annotations + +import json +import re +import uuid +from collections import Counter +from dataclasses import asdict, dataclass +from datetime import datetime, timezone +from pathlib import Path +from typing import Any + +_LABEL = re.compile(r"^[a-z][a-z0-9-]{0,79}$") +_DECISION_REF = re.compile(r"^[A-Z][A-Z0-9-]{2,80}$") +_DELIVERY_RESULTS = {"delivered", "failed", "skipped-no-topic"} +_VERIFY_RESULT = re.compile(r"^(positive|negative):(pass|fail)$") +_ERROR_RESULT = re.compile( + r"^failed-(Catalog|Decision|PolicyGuard|Backend|Provisioning|Verification|Delivery)Error$" +) + + +def _safe_label(value: object) -> str: + text = value if isinstance(value, str) else "" + return text if _LABEL.fullmatch(text) else "invalid-label" + + +def _safe_result(value: object) -> str: + text = value if isinstance(value, str) else "" + if ( + _LABEL.fullmatch(text) + or _VERIFY_RESULT.fullmatch(text) + or _ERROR_RESULT.fullmatch(text) + ): + return text + return "invalid-label" + + +def _safe_decision_ref(value: object) -> str: + text = value if isinstance(value, str) else "" + if not text: + return "" + try: + return str(uuid.UUID(text)) + except ValueError: + return text if _DECISION_REF.fullmatch(text) else "" + + +def _safe_timestamp(value: object) -> str: + if not isinstance(value, str): + return "" + try: + parsed = datetime.fromisoformat(value.replace("Z", "+00:00")) + if parsed.tzinfo is None: + return "" + return parsed.astimezone(timezone.utc).isoformat() + except ValueError: + return "" + + +@dataclass(frozen=True) +class LaneAuditSummary: + catalog_id: str + operation_records: int + malformed_records: int + first_ts: str + last_ts: str + actions: dict[str, int] + results: dict[str, int] + decision_refs: list[str] + session_cleanup: dict[str, int] + hub_delivery: dict[str, int] + + def to_json(self) -> dict[str, Any]: + return asdict(self) + + def render(self) -> str: + lines = [ + f"Lane audit summary for '{self.catalog_id}'", + f" operation records: {self.operation_records}", + f" malformed records: {self.malformed_records}", + f" first: {self.first_ts or 'n/a'}", + f" last: {self.last_ts or 'n/a'}", + " actions: " + _render_counts(self.actions), + " results: " + _render_counts(self.results), + " decisions: " + (", ".join(self.decision_refs) or "none"), + " session cleanup: " + _render_counts(self.session_cleanup), + " hub delivery: " + _render_counts(self.hub_delivery), + ] + return "\n".join(lines) + + +def _render_counts(counts: dict[str, int]) -> str: + return ", ".join(f"{key}={value}" for key, value in sorted(counts.items())) or "none" + + +def summarize_lane_evidence(evidence_dir: Path, catalog_id: str) -> LaneAuditSummary: + """Summarize one lane without returning arbitrary record fields or detail.""" + actions: Counter[str] = Counter() + results: Counter[str] = Counter() + cleanup: Counter[str] = Counter() + hub_delivery: Counter[str] = Counter() + decisions: set[str] = set() + timestamps: list[str] = [] + malformed = 0 + operation_records = 0 + + for path in sorted(Path(evidence_dir).glob("evidence-*.jsonl")): + try: + lines = path.read_text(encoding="utf-8").splitlines() + except OSError: + malformed += 1 + continue + for line in lines: + try: + record = json.loads(line) + except (json.JSONDecodeError, TypeError): + malformed += 1 + continue + if not isinstance(record, dict): + malformed += 1 + continue + if record.get("catalog_id") != catalog_id: + continue + action = _safe_label(record.get("action")) + result = _safe_result(record.get("result")) + if action == "evidence-delivery": + if result in _DELIVERY_RESULTS: + hub_delivery[result] += 1 + else: + hub_delivery["invalid"] += 1 + continue + + operation_records += 1 + actions[action] += 1 + results[result] += 1 + timestamp = _safe_timestamp(record.get("ts")) + if timestamp: + timestamps.append(timestamp) + decision = _safe_decision_ref(record.get("decision_id")) + if decision: + decisions.add(decision) + + detail = record.get("detail") + session = detail.get("session") if isinstance(detail, dict) else None + if isinstance(session, dict): + attempted = session.get("revocation_attempted") is True + succeeded = session.get("revocation_succeeded") is True + if succeeded: + cleanup["succeeded"] += 1 + elif attempted: + cleanup["failed"] += 1 + else: + cleanup["not-attempted"] += 1 + + timestamps.sort() + return LaneAuditSummary( + catalog_id=catalog_id, + operation_records=operation_records, + malformed_records=malformed, + first_ts=timestamps[0] if timestamps else "", + last_ts=timestamps[-1] if timestamps else "", + actions=dict(sorted(actions.items())), + results=dict(sorted(results.items())), + decision_refs=sorted(decisions), + session_cleanup=dict(sorted(cleanup.items())), + hub_delivery=dict(sorted(hub_delivery.items())), + ) diff --git a/src/secrets_engine/cli.py b/src/secrets_engine/cli.py index 3f4f3d6..08e2695 100644 --- a/src/secrets_engine/cli.py +++ b/src/secrets_engine/cli.py @@ -7,12 +7,13 @@ Command surface (FR7): plan --stage apply --stage [--dry-run] [--bootstrap-token-file F] provision --stage (--from-file F | --generate) --field NAME - verify [--positive] [--negative] [--field NAME] + verify [--positive] [--negative] [--field NAME] [--negative-token-file F] handoff --stage --role-id-file F --secret-id-file F exec --catalog [--field NAME] [--mode auto|npm-config|exec-env] -- CMD... route [--json] revoke lifecycle suspend|deactivate|destroy + audit [--json] Every privileged action is decision-gated and writes non-secret evidence. `plan` and `apply --dry-run` never mutate OpenBao. @@ -221,6 +222,13 @@ def cmd_verify(cfg: Config, args) -> int: 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 + unrelated_token = None + if entry.stores_kv_value() and negative and args.negative_token_file: + from secrets_engine.openbao import read_strict_token_file + + unrelated_token = read_strict_token_file( + Path(args.negative_token_file), purpose="negative verification token" + ) if entry.stores_kv_value(): results = [] if positive: @@ -234,7 +242,12 @@ def cmd_verify(cfg: Config, args) -> int: # 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 + client, + entry, + fields[0], + positive=False, + negative=True, + unrelated_token=unrelated_token, ) ) else: @@ -460,6 +473,21 @@ def cmd_lifecycle(cfg: Config, args) -> int: return 0 +def cmd_audit(cfg: Config, args) -> int: + """Summarize allowlisted non-secret evidence for one cataloged lane.""" + import json + + from secrets_engine.audit import summarize_lane_evidence + + entry = get_entry(cfg.catalog_dir, args.catalog_id) + summary = summarize_lane_evidence(cfg.evidence_dir, entry.id) + if args.json: + print(json.dumps(summary.to_json(), indent=2, sort_keys=True)) + else: + print(summary.render()) + return 0 + + # -- parser ---------------------------------------------------------------- @@ -512,6 +540,11 @@ def build_parser() -> argparse.ArgumentParser: ve.add_argument("--field", default=None) ve.add_argument("--positive", action="store_true") ve.add_argument("--negative", action="store_true") + ve.add_argument( + "--negative-token-file", + default=None, + help="mode-0600 out-of-repo token for a real unrelated identity", + ) add_token_arg(ve) ve.set_defaults(func=cmd_verify) @@ -571,6 +604,11 @@ def build_parser() -> argparse.ArgumentParser: add_token_arg(lp) lp.set_defaults(func=cmd_lifecycle) + au = sub.add_parser("audit", help="summarize non-secret local lane evidence") + au.add_argument("catalog_id") + au.add_argument("--json", action="store_true") + au.set_defaults(func=cmd_audit) + return p diff --git a/src/secrets_engine/evidence.py b/src/secrets_engine/evidence.py index d432d8a..eaf7882 100644 --- a/src/secrets_engine/evidence.py +++ b/src/secrets_engine/evidence.py @@ -10,6 +10,7 @@ import json import os import urllib.error import urllib.request +import uuid from dataclasses import dataclass, field from datetime import datetime, timezone from pathlib import Path @@ -55,6 +56,12 @@ class EvidenceWriter: day = datetime.now(timezone.utc).strftime("%Y-%m-%d") return self.evidence_dir / f"evidence-{day}.jsonl" + def _append_local(self, record: dict[str, Any]) -> None: + """Append one record before any best-effort external delivery.""" + path = self._log_path() + with path.open("a", encoding="utf-8") as fh: + fh.write(json.dumps(record, sort_keys=True) + "\n") + def record( self, action: str, @@ -67,7 +74,10 @@ class EvidenceWriter: hub: bool = True, ) -> dict[str, Any]: """Append one non-secret evidence record. Returns the stored record.""" + record_id = str(uuid.uuid4()) + hub_requested = bool(hub and self.hub_url) record = { + "record_id": record_id, "ts": datetime.now(timezone.utc).isoformat(), "action": action, "result": result, @@ -76,20 +86,38 @@ class EvidenceWriter: "stage": stage, "decision_id": decision_id, "detail": _scrub(detail or {}), + "hub_delivery_requested": hub_requested, } - path = self._log_path() - with path.open("a", encoding="utf-8") as fh: - fh.write(json.dumps(record, sort_keys=True) + "\n") - if hub and self.hub_url: - self._post_hub(action, result, catalog_id, stage, decision_id) + self._append_local(record) + if hub_requested: + delivery_result = self._post_hub( + action, result, catalog_id, stage, decision_id + ) + # Append-only companion evidence makes an unavailable State Hub + # visible without rewriting or delaying the primary local record. + self._append_local( + { + "record_id": str(uuid.uuid4()), + "related_record_id": record_id, + "ts": datetime.now(timezone.utc).isoformat(), + "action": "evidence-delivery", + "result": delivery_result, + "actor": self.actor, + "catalog_id": catalog_id, + "stage": stage, + "decision_id": decision_id, + "detail": {}, + "hub_delivery_requested": False, + } + ) return record def _post_hub( self, action: str, result: str, catalog_id: str, stage: str, decision_id: str - ) -> None: - """Best-effort progress note to State Hub. Never raises; never sends values.""" + ) -> str: + """Best-effort progress note; return a non-secret delivery outcome.""" if not self.topic_id: - return + return "skipped-no-topic" summary = f"secrets-engine {action}: {result}" if catalog_id: summary += f" [{catalog_id}{'/' + stage if stage else ''}]" @@ -111,6 +139,7 @@ class EvidenceWriter: method="POST", ) urllib.request.urlopen(req, timeout=3).read() + return "delivered" except (urllib.error.URLError, OSError, ValueError): # Hub being offline must never block secret work or leak anything. - pass + return "failed" diff --git a/src/secrets_engine/openbao.py b/src/secrets_engine/openbao.py index 7cf881a..0c43414 100644 --- a/src/secrets_engine/openbao.py +++ b/src/secrets_engine/openbao.py @@ -28,29 +28,34 @@ from secrets_engine.errors import BackendError, ProvisioningError from secrets_engine.safe_paths import containing_git_worktree -def _check_token_file(path: Path) -> str: - """Read a bootstrap token file after enforcing mode-0600 and out-of-repo.""" +def read_strict_token_file(path: Path, *, purpose: str = "token") -> str: + """Read token material only from a mode-0600 path outside Git worktrees.""" if not path.exists(): - raise ProvisioningError(f"bootstrap token file not found: {path}") + raise ProvisioningError(f"{purpose} file not found: {path}") st = path.stat() if st.st_mode & 0o077: raise ProvisioningError( - f"bootstrap token file {path} is group/other-accessible " + f"{purpose} file {path} is group/other-accessible " f"(mode {oct(st.st_mode & 0o777)}); must be 0600" ) # Refuse a token file living inside a Git worktree. worktree = containing_git_worktree(path) if worktree is not None: raise ProvisioningError( - f"bootstrap token file {path} is inside a Git worktree ({worktree}); " + f"{purpose} file {path} is inside a Git worktree ({worktree}); " "store it outside any repo" ) token = path.read_text(encoding="utf-8").strip() if not token: - raise ProvisioningError(f"bootstrap token file {path} is empty") + raise ProvisioningError(f"{purpose} file {path} is empty") return token +def _check_token_file(path: Path) -> str: + """Compatibility wrapper for bootstrap authentication input.""" + return read_strict_token_file(path, purpose="bootstrap token") + + @dataclass class ScopedTokenSession: """One AppRole login token that revokes itself on close.""" diff --git a/src/secrets_engine/routing.py b/src/secrets_engine/routing.py index e47fc52..c0d2b31 100644 --- a/src/secrets_engine/routing.py +++ b/src/secrets_engine/routing.py @@ -86,7 +86,8 @@ def route_lane( if entry.kind == "kv" and entry.delivery_auth_management == "existing": missing = "externally managed OpenBao policy/AppRole readiness" next_command = ( - f"secrets-engine verify {entry.id} --positive --negative" + f"secrets-engine verify {entry.id} --positive --negative " + "--negative-token-file " ) elif entry.kind == "kv" and not entry.has_delivery_auth: missing = "native delivery auth declaration" @@ -114,7 +115,8 @@ def route_lane( next_command = f"secrets-engine exec --catalog {entry.id} -- " else: next_command = ( - f"secrets-engine verify {entry.id} --positive --negative" + f"secrets-engine verify {entry.id} --positive --negative " + "--negative-token-file " ) return RouteResult( diff --git a/src/secrets_engine/verify.py b/src/secrets_engine/verify.py index 1f7f0f0..9fde90a 100644 --- a/src/secrets_engine/verify.py +++ b/src/secrets_engine/verify.py @@ -74,10 +74,25 @@ def verify_positive(client: OpenBaoClient, entry: CatalogEntry, field: str) -> V ) -def verify_negative(client: OpenBaoClient, entry: CatalogEntry) -> VerifyResult: - """An unrelated token must be denied. Uses an empty (invalid) token.""" - # An empty/garbage token stands in for an unrelated consumer. - denied = not client.kv_can_read(entry.mount, entry.path, token="se-unrelated-denied") +def verify_negative( + client: OpenBaoClient, + entry: CatalogEntry, + *, + unrelated_token: str | None, +) -> VerifyResult: + """A real unrelated token must be denied the cataloged path.""" + if not unrelated_token: + return VerifyResult( + "negative", + False, + { + "reason": "no real unrelated token supplied; denial not proven", + "path": entry.path, + }, + ) + denied = not client.kv_can_read( + entry.mount, entry.path, token=unrelated_token + ) return VerifyResult( "negative", denied, @@ -167,7 +182,13 @@ def verify_auth_capability_negative(client: OpenBaoClient, entry: CatalogEntry) def run_verification( - client: OpenBaoClient, entry: CatalogEntry, field: str, *, positive: bool, negative: bool + client: OpenBaoClient, + entry: CatalogEntry, + field: str, + *, + positive: bool, + negative: bool, + unrelated_token: str | None = None, ) -> list[VerifyResult]: if entry.kind == "kv" and field not in entry.fields: raise VerificationError( @@ -183,7 +204,9 @@ def run_verification( if positive: results.append(verify_positive(client, entry, field)) if negative: - results.append(verify_negative(client, entry)) + results.append( + verify_negative(client, entry, unrelated_token=unrelated_token) + ) if not results: raise VerificationError("no verification check selected (use --positive/--negative)") return results diff --git a/tests/test_audit.py b/tests/test_audit.py new file mode 100644 index 0000000..1dcd47e --- /dev/null +++ b/tests/test_audit.py @@ -0,0 +1,102 @@ +import json +import urllib.error +from pathlib import Path +from types import SimpleNamespace + +from secrets_engine.audit import summarize_lane_evidence +from secrets_engine.cli import build_parser +from secrets_engine.evidence import EvidenceWriter + + +DECISION_ID = "e6381a56-3e55-4fac-b22c-63ee1c152ce8" + + +def test_lane_audit_summarizes_allowlisted_fields_and_cleanup(tmp_path, monkeypatch): + writer = EvidenceWriter( + evidence_dir=tmp_path, + hub_url="http://hub.invalid", + topic_id="topic-id", + ) + monkeypatch.setattr( + "urllib.request.urlopen", lambda *_args, **_kwargs: SimpleNamespace(read=lambda: b"{}") + ) + writer.record( + "exec", + result="exit-0", + catalog_id="test-lane", + stage="test", + decision_id=DECISION_ID, + detail={ + "session": { + "session_handle": "safe-fingerprint", + "revocation_attempted": True, + "revocation_succeeded": True, + } + }, + ) + + def offline(*_args, **_kwargs): + raise urllib.error.URLError("offline") + + monkeypatch.setattr("urllib.request.urlopen", offline) + writer.record( + "verify", + result="positive-pass", + catalog_id="test-lane", + stage="test", + decision_id=DECISION_ID, + detail={ + "session": { + "session_handle": "another-safe-fingerprint", + "revocation_attempted": True, + "revocation_succeeded": False, + } + }, + ) + + path = next(tmp_path.glob("evidence-*.jsonl")) + with path.open("a", encoding="utf-8") as fh: + fh.write("not-json\n") + fh.write( + json.dumps( + { + "catalog_id": "test-lane", + "action": "fake-SUPER-SECRET-value", + "result": "fake-SUPER-SECRET-value", + "decision_id": "fake-SUPER-SECRET-value", + "ts": "not-a-time", + "detail": {"value": "fake-SUPER-SECRET-value"}, + } + ) + + "\n" + ) + + summary = summarize_lane_evidence(tmp_path, "test-lane") + rendered = json.dumps(summary.to_json()) + summary.render() + + assert summary.operation_records == 3 + assert summary.malformed_records == 1 + assert summary.actions == {"exec": 1, "invalid-label": 1, "verify": 1} + assert summary.results == { + "exit-0": 1, + "invalid-label": 1, + "positive-pass": 1, + } + assert summary.decision_refs == [DECISION_ID] + assert summary.session_cleanup == {"failed": 1, "succeeded": 1} + assert summary.hub_delivery == {"delivered": 1, "failed": 1} + assert "fake-SUPER-SECRET-value" not in rendered + assert "safe-fingerprint" not in rendered + + +def test_lane_audit_empty_directory_is_a_valid_empty_summary(tmp_path): + summary = summarize_lane_evidence(tmp_path, "test-lane") + assert summary.operation_records == 0 + assert summary.actions == {} + assert summary.render().startswith("Lane audit summary for 'test-lane'") + + +def test_cli_parser_exposes_audit_json_command(): + args = build_parser().parse_args(["audit", "test-lane", "--json"]) + assert args.catalog_id == "test-lane" + assert args.json is True diff --git a/tests/test_integration_bao.py b/tests/test_integration_bao.py index 480f707..41e3bcb 100644 --- a/tests/test_integration_bao.py +++ b/tests/test_integration_bao.py @@ -90,9 +90,32 @@ def test_full_chain(bao_dev, tmp_path): pos = verify_positive(client, entry, "npm_token") assert pos.passed, pos.detail - neg = verify_negative(client, entry) + client.write_policy( + "se-unrelated-denied", + f'path "{entry.mount}/data/{entry.path}" {{ capabilities = ["deny"] }}\n', + ) + unrelated = json.loads( + client._run_ok( + ["token", "create", "-format=json", "-policy=se-unrelated-denied"] + ) + )["auth"]["client_token"] + neg = verify_negative(client, entry, unrelated_token=unrelated) assert neg.passed, neg.detail + # A real unrelated identity with accidental policy overlap must make the + # negative check fail; an invalid/garbage token could not detect this. + client.write_policy( + "se-unrelated-overlap", + f'path "{entry.mount}/data/{entry.path}" {{ capabilities = ["read"] }}\n', + ) + overlapping = json.loads( + client._run_ok( + ["token", "create", "-format=json", "-policy=se-unrelated-overlap"] + ) + )["auth"]["client_token"] + leaked = verify_negative(client, entry, unrelated_token=overlapping) + assert not leaked.passed, leaked.detail + # exec delivery: child can resolve token via npmrc; assert via a probe script probe = tmp_path / "probe.sh" probe.write_text( diff --git a/tests/test_multifield_readiness.py b/tests/test_multifield_readiness.py index 8c9df52..2fa51dd 100644 --- a/tests/test_multifield_readiness.py +++ b/tests/test_multifield_readiness.py @@ -83,8 +83,11 @@ def test_verify_defaults_to_every_declared_field_and_one_path_denial( 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): + def fake_verify( + _client, _entry, field, *, positive, negative, unrelated_token=None + ): calls.append((field, positive, negative)) + assert unrelated_token is None check = "positive" if positive else "negative" return [VerifyResult(check, True, {"field": field, "reason": "test"})] @@ -100,6 +103,7 @@ def test_verify_defaults_to_every_declared_field_and_one_path_denial( field=None, positive=False, negative=False, + negative_token_file=None, ) assert cli.cmd_verify(_config(tmp_path), args) == 0 diff --git a/tests/test_redact_evidence.py b/tests/test_redact_evidence.py index ba17dcc..0150c22 100644 --- a/tests/test_redact_evidence.py +++ b/tests/test_redact_evidence.py @@ -1,4 +1,6 @@ import json +import urllib.error +from types import SimpleNamespace from secrets_engine.evidence import EvidenceWriter, _scrub from secrets_engine.redact import looks_secret, redact_text @@ -39,3 +41,48 @@ def test_evidence_record_has_no_value(tmp_path): # written to disk too files = list(tmp_path.glob("evidence-*.jsonl")) assert files and "npm_shouldnotappear123" not in files[0].read_text() + + +def test_evidence_records_append_only_hub_delivery_success(tmp_path, monkeypatch): + monkeypatch.setattr( + "urllib.request.urlopen", + lambda *_args, **_kwargs: SimpleNamespace(read=lambda: b"{}"), + ) + writer = EvidenceWriter( + evidence_dir=tmp_path, + hub_url="http://hub.invalid", + topic_id="topic-id", + ) + + primary = writer.record("verify", result="pass", catalog_id="lane") + + lines = [ + json.loads(line) + for line in next(tmp_path.glob("evidence-*.jsonl")).read_text().splitlines() + ] + assert len(lines) == 2 + assert lines[0] == primary + assert lines[1]["action"] == "evidence-delivery" + assert lines[1]["result"] == "delivered" + assert lines[1]["related_record_id"] == primary["record_id"] + + +def test_evidence_records_hub_failure_without_raising(tmp_path, monkeypatch): + def offline(*_args, **_kwargs): + raise urllib.error.URLError("offline") + + monkeypatch.setattr("urllib.request.urlopen", offline) + writer = EvidenceWriter( + evidence_dir=tmp_path, + hub_url="http://hub.invalid", + topic_id="topic-id", + ) + + writer.record("verify", result="pass", catalog_id="lane") + + lines = [ + json.loads(line) + for line in next(tmp_path.glob("evidence-*.jsonl")).read_text().splitlines() + ] + assert lines[-1]["action"] == "evidence-delivery" + assert lines[-1]["result"] == "failed" diff --git a/tests/test_verification_negative.py b/tests/test_verification_negative.py new file mode 100644 index 0000000..538a25f --- /dev/null +++ b/tests/test_verification_negative.py @@ -0,0 +1,61 @@ +import copy +import os + +import pytest + +from secrets_engine.catalog import validate_entry +from secrets_engine.errors import ProvisioningError +from secrets_engine.openbao import read_strict_token_file +from secrets_engine.verify import verify_negative +from tests.test_catalog import VALID + + +def _entry(): + return validate_entry(copy.deepcopy(VALID)) + + +class NegativeClient: + def __init__(self, can_read): + self.can_read = can_read + self.tokens = [] + + def kv_can_read(self, _mount, _path, *, token): + self.tokens.append(token) + return self.can_read + + +def test_negative_check_fails_closed_without_real_unrelated_token(): + client = NegativeClient(can_read=False) + result = verify_negative(client, _entry(), unrelated_token=None) + assert result.passed is False + assert "not proven" in result.detail["reason"] + assert client.tokens == [] + + +def test_negative_check_uses_real_token_and_detects_policy_overlap(): + denied = NegativeClient(can_read=False) + assert verify_negative( + denied, _entry(), unrelated_token="test-unrelated-token" + ).passed + assert denied.tokens == ["test-unrelated-token"] + + overlapping = NegativeClient(can_read=True) + result = verify_negative( + overlapping, _entry(), unrelated_token="test-overlapping-token" + ) + assert result.passed is False + assert "LEAK RISK" in result.detail["reason"] + + +def test_negative_token_file_requires_mode_0600(tmp_path): + path = tmp_path / "unrelated.token" + path.write_text("test-unrelated-token", encoding="utf-8") + os.chmod(path, 0o644) + with pytest.raises(ProvisioningError, match="must be 0600"): + read_strict_token_file(path, purpose="negative verification token") + + os.chmod(path, 0o600) + assert ( + read_strict_token_file(path, purpose="negative verification token") + == "test-unrelated-token" + ) diff --git a/workplans/SECRETS-WP-0007-production-lifecycle-hardening.md b/workplans/SECRETS-WP-0007-production-lifecycle-hardening.md index e96c43e..1dee317 100644 --- a/workplans/SECRETS-WP-0007-production-lifecycle-hardening.md +++ b/workplans/SECRETS-WP-0007-production-lifecycle-hardening.md @@ -290,11 +290,17 @@ 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. +prove a missing sibling makes readiness false. Evidence delivery attempts now +receive append-only local success/failure receipts, and `audit` produces an +allowlisted per-lane summary of actions, results, decisions, session cleanup, +and delivery outcomes without echoing arbitrary detail. The real +unrelated-identity input is now enforced as a strict mode-0600 out-of-repo token +file; absence fails closed, and throwaway OpenBao integration deliberately adds +an overlapping unrelated read policy and proves the check fails. Production +identity selection/ownership, audit request correlation, queued/replayed State +Hub delivery, and complete privileged failure-path evidence remain outstanding. -The complete repository suite passes with 103 tests after these changes, +The complete repository suite passes with 111 tests after these changes, including throwaway OpenBao integration coverage. Make verification and routing truthful for multi-field and high-risk lanes: