P3: engine PDP + one signed receipt chain (fail-closed authorization spine)#3
P3: engine PDP + one signed receipt chain (fail-closed authorization spine)#3bb-connor wants to merge 11 commits into
Conversation
… green) Phase 0 steps 1-TS and 3 (ADR-0004 app-lifetime signed sidecar): - issuer-key.ts: loadOrCreateIssuerKey persists a 32-byte Ed25519 seed 0o600 via writePrivateAtomic; derives pubKeyHex (seedHex never leaves main). - swarm-engine.ts: SwarmEngineSidecar spawns swarm_detect --serve :0, parses the listening sentinel, pins protocol_version fail-closed, mints a per-launch bearer, owns lifecycle via ProcessSupervisor, exposes authHeader/baseUrl/ signerPubKey + stopLiveService + replayRun. - Generalized the P2 ProcessSupervisor to src/main/process/ (no duplication). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Meta-audit found a latent fail-open in evaluateSentinel: token_required was parsed but never enforced, so an engine advertising token_required:false would still go ready. Since the supervisor always spawns with AMBUSH_ENGINE_TOKEN, an unauthenticated PDP is a misconfig/tamper — now surfaced as kind:'token-not-required' and handled fail-closed (stop supervisor, engine-down), mirroring protocol-mismatch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ppy/build green) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…r fail-closed, tail durability (hard-review fixes) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ail-closed, G5/G6) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…required + G6 stall test (hard-review fixes) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…/v1/authorize (typecheck/oxlint green) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ing + teardown/UTF8/tests (hard-review) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…view gate) Records the ratified rule (from meta-audit#2 + the P3 evidence): any commit touching the security-critical set gets an adversarial multi-lens hard-review as a merge gate; light review is non-authoritative there. Includes the P3 scorecard — 3/3 TCB commits had a real fail-open the implementing agent's own tests passed — and the corollaries (behaviorally-complete gate tests, fail-closed default, no fix regresses a prior fix, read-only automation). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
P3 — the engine PDP + single signed receipt chain (foundation → PDP → PEP), all fail-closed. Every TCB commit passed an adversarial 4-lens hard-review as a merge gate (3 caught + fixed real fail-opens; see docs/ambush/engineering/SECURITY-REVIEW-DISCIPLINE.md). Deferred follow-ups noted in the description. @codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f1512adef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…-leak, unsigned denials, prestop auth, PATH binary, win shell, +more - P1 TCB binary spoofing: sidecar resolves swarm_detect via resolveEngineBinary (app-owned/packaged in a packaged app, repo build dir in dev; never PATH/cwd). - P1 orphaned append allow-leak: /v1/authorize appends via append_unless_superseded; a timed-out request is superseded so the writer skips its commit (never lands a late allow the caller was denied). - P2 ephemeral chain: sidecar passes AMBUSH_SPINE_DIR (durable per-user path); the engine maps it over the memory-ephemeral spine dir so the chain persists. - P2 unsigned denials: missing-identity deny now chains a signed authorize.missing_identity mcp_tool_decision fact before returning (fail-closed). - P2 agent DoS: /prestop moved out of the always-open probe group into the master-bearer-guarded group; supervisor sends the master bearer on prestop. - P2 unchained runtime responses: IngestRuntimeRequestResponseRouter chains response_execution onto the spine. - P2 governed-without-a-gate: orchestrator holds a governed launch with no gateable MCP server (or proceeds honestly ungoverned under operator override). - P2 stale governed status: sidecar re-probes /healthz post-readiness and reflects a tripped PDP as not-running. - P2 win shell: gate proxy spawns .cmd/.bat inner MCP via cmd.exe /d /c. - P3 follow-up: terminal command governance gap made explicit + fail-safe (banner + log + AMBUSH_TERM_FAILCLOSED), documented in ADR-0004. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
All 10 review findings addressed in P1 — the timed-out Gates green: Rust fmt/clippy/build 0, authorize 17/17, spine 47/47, policy 19/19 (0 new failures); TS typecheck/oxlint clean, 128 tests. Full P3 CI already green. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45ebbd7d2d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // the original verdict never enters the signed chain. Checked here, after | ||
| // any stall and before the first durable write, so a timed-out append that | ||
| // is still queued/in-flight lands nothing. | ||
| if superseded.is_some_and(|flag| flag.load(Ordering::SeqCst)) { |
There was a problem hiding this comment.
When the writer spends longer than authorize_append_timeout inside the durable write_all/sync_all/persist_head work, the timeout handler sets superseded only after this single pre-write check has already passed. The writer can then still commit the original verdict even though /v1/authorize has returned a 503 deny, so a slow fsync can leave a signed allow receipt for a call the PEP did not execute. Recheck/rollback on supersession after the blocking durable steps, or avoid returning deny once the append is past this point.
Useful? React with 👍 / 👎.
| // Chain the executed response onto the spine ledger (best-effort, like the | ||
| // demo/replay path) so runtime agent responses are on one signed chain and | ||
| // not just published as an ephemeral ResponseExecution event. | ||
| record_response_execution(&self.spine_ledger, &audit).await; |
There was a problem hiding this comment.
The new response_execution append only covers route_request; the dispatcher path for GovernanceVeto calls the adjacent route_governance_veto, which builds an AuditTrail with AuditResponseRecord::Failure via audit_governance_veto and publishes it, but returns without calling record_response_execution. Tom/Pouncer vetoes are therefore missing from the signed chain while normal executions are present, breaking the one-chain audit story specifically for vetoed responses.
Useful? React with 👍 / 👎.
| if (isPackagedApp()) { | ||
| const rels = [ | ||
| ...relCandidates, | ||
| join('engine', 'bin'), | ||
| 'bin', | ||
| join('engine', 'target', 'release'), | ||
| join('engine', 'target', 'debug'), | ||
| '.', | ||
| ] |
There was a problem hiding this comment.
In packaged builds this resolver refuses PATH/cwd and searches only bundled resource/app paths under engine/bin, bin, engine/target/*, or the root. I checked config/electron-builder.config.cjs: the extraResources lists do not copy swarm_detect/swarmctl into any of those loose locations, so a packaged app will report the engine binary missing and block governed launches. Add an engine extraResource to a searched path, or update the resolver to the actual staged path, before enabling EngineGovernor in packaged builds.
Useful? React with 👍 / 👎.
| this.currentAuthorizeToken = authorizeToken | ||
| this.endpoint = null | ||
| this.lineBuffer = '' | ||
| const args = ['--serve', '--bind', `${LIVE_HOST}:0`] |
There was a problem hiding this comment.
When the sidecar is spawned from Electron main, this argv relies on swarm_detect's CLI default rulesets/default.yaml and does not set a cwd. In the repo that file is under engine/rulesets/default.yaml (and packaged builds need an explicit resource path), so a normal app launch from the repo root or a packaged install exits before the listening sentinel and governance never becomes ready. Resolve/package the ruleset and pass --config <path> or set the child cwd to the directory that actually contains rulesets/default.yaml.
Useful? React with 👍 / 👎.
| return ( | ||
| StatusCode::BAD_REQUEST, | ||
| ResponseJson(json!({ | ||
| "verdict": PolicyVerdict::Deny, | ||
| "rule_name": "authorize.invalid_request", | ||
| "reason": rejection.body_text(), | ||
| })), |
There was a problem hiding this comment.
This invalid-payload branch returns a deny response without going through record_authorization_decision, even though the route's contract says every /v1/authorize allow or deny is recorded on the spine before returning. An authenticated caller that sends malformed JSON or omits required fields gets an unsigned 400, leaving no non-repudiable authorize.invalid_request receipt. Chain an invalid-request fact before returning, and fail closed with 503 if that append cannot be recorded.
Useful? React with 👍 / 👎.
| "response_receipt_id": response_receipt_id, | ||
| "created_at_ms": audit.created_at_ms, | ||
| }); | ||
| let event_id = format!("response_execution:{trail_id}"); |
There was a problem hiding this comment.
The ledger dedup key is only trail_id, but audit_authorize_and_execute constructs that id as trail:{hunt_id}:{now_ms}. If two response actions for the same hunt are routed within the same millisecond, the second append is treated as a duplicate and the chain omits a distinct response_execution fact. Include the response receipt/action/event id (or another per-action nonce) in the event id, and reserve dedup for true replays of the same audit.
Useful? React with 👍 / 👎.
| if index == last_index { | ||
| torn = true; | ||
| break; |
There was a problem hiding this comment.
Any verification failure on the final line is treated as a torn append and the ledger is rewritten to the valid prefix. That is safe for an unparseable partial tail, but if the last line is a complete envelope with a bad signature or broken chain link, this silently drops tail tampering instead of failing closed; with a missing/stale head anchor it can erase the latest receipts on open. Only parse/truncation-style tail errors should be healed, while signed-but-invalid envelopes should return the integrity error.
Useful? React with 👍 / 👎.
| // Durable per-user spine dir so the signed receipt chain persists across | ||
| // restarts (else it lands in an ephemeral temp dir under the memory ruleset). | ||
| spineDir: engineSpineDir(userDataDir), | ||
| onLog: (level, message) => bus.log(level, 'engine', message), |
There was a problem hiding this comment.
The sidecar is started asynchronously, but this construction only wires onLog; nothing emits bus.governorUpdate when the sentinel/readiness probe later flips the EngineGovernor from unavailable to available. If the renderer bootstraps before that transition, the status bar stays ungoverned and refreshReceipts() is never triggered even though launches are now governed. Wire onState to publish the current governor status whenever the sidecar state changes.
Useful? React with 👍 / 👎.
1 participant
P3 — Delete Chio: one policy authority (PDP) + one signed receipt chain
Implements ADR-0004: collapse agent-tool governance to a single Policy Decision Point (the Rust engine's
--serveprocess) and a single signed evidence chain (swarm-spine). Every governed agent tool call is authorized byPOST /v1/authorizeand recorded as anmcp_tool_decisionfact on the same chain asresponse_execution. Fail-closed everywhere: engine-down / timeout / malformed-verdict / unrecordable-decision ⇒ deny.This PR delivers the complete authorization spine — foundation → PDP → PEP. Receipts UI, client-side verification, full credential capability-scoping, the TerminalGovernor→PDP repoint, and the actual Chio deletion are documented follow-ups (deletion is gated on a one-release shadow-run per ADR-0004).
What's in this PR
1. Foundation — the one signed chain (
engine/crates/swarm-spine,swarm-runtime)SWARM_EVIDENCE_SIGNING_KEY, hex seed →SigningKey::from_bytes) somcp_tool_decision,response_execution, and envelopes share one issuer across restarts (the TS sidecar pins this pubkey).SpineLedger— append-only, per-issuer ed25519-chained; cross-process advisory lock (a 2nd--serveon one vault refuses to start); one owning writer thread; atomic durable append with all-or-nothing rollback;open()binds the recovered chain to the writer's own issuer (refuses a foreign issuer); torn/newline-less-tail self-heal; interior corruption is fatal;event_iddedup.2. PDP — the decision point (
swarm-policy,swarm-runtime)POST /v1/authorize(bearer-guarded) →ToolGate(fail-closed: unknown server/empty-tool/deny-listed/empty-allowlist/not-in-allowlist ⇒ Deny; deny-list wins) → writes anmcp_tool_decisionfact (allow and deny, for non-repudiation) → returns{verdict, receipt_id, seq}. HushSpec lives in the attested binary (ToolAccessConfig), not an unsigned sidecar.spawn_blocking); a concurrency test proves contiguous seq + responsive/healthzunder load.3. PEP — the runtime enforcement point (
src/main/{governance,swarm-engine})EngineGovernor+ a fail-closed stdio MCP gate proxy: every agenttools/callPOSTs to/v1/authorizeand is denied on any error (non-200, refused, timeout, malformed verdict, non-allow); it forwards the gate's own re-serialized canonical message (no parser-differential); the launch re-asserts governance after worktree checkout (no TOCTOU silent-ungoverned); agents receive an authorize-scoped token only (the master bearer never enters.mcp.json).Security review
Every TCB commit went through an adversarial 4-lens hard-review (lock/concurrency, chain/crypto, fail-open, ADR-drift + adjudicator) as a merge gate. It caught a real fail-open on all three — a chain that silently adopted a foreign issuer, a G5 latch that leaked
allowafter tripping, and a launch TOCTOU that ran vectors unproxied — each of which the implementing agent's own tests had passed. All were fixed and confirmed by a focused delta re-review before this PR. The security-review discipline is documented indocs/ambush/engineering/.Verification
Engine:
cargo fmt --check/clippy -D warnings/buildclean;swarm-spine,swarm-policy, and theauthorize_gate_integrationsuite green (pre-existing experiment-fixture gaps unchanged, 0 regressions). TS:typecheck(node+cli+web) +oxlintclean,.d.tsguard clean, governance/swarm/swarm-engine suites green.Deferred (documented follow-ups)
Receipts UI + client-side Ed25519 verification · full per-operation credential capability-scoping (bind
operation_id/agent_idto the credential server-side) · TerminalGovernor→engine-PDP repoint · delete Chio (after a shadow-run with zero verdict divergence).🤖 Generated with Claude Code