security: STRIDE verification audit — fixes (E-5, T-5, T-4), truth-up, claim/evidence CI gate#39
Merged
Merged
Conversation
A line-by-line re-audit of every "✅ resolved" STRIDE finding (prompted by D-6 being marked done without an implementation) found more of the same pattern. This commit fixes the two small real gaps and corrects the docs so the threat model stops overstating coverage. Real fixes (with regression tests): - E-5: core/AgentRegistry.register() silently unregistered+overwrote a duplicate agent id — a malicious agent could seize a trusted agent's identity with no signal. Now rejects the collision (throws) and emits a safety.violation audit event. Legit re-register after unregister still works. - T-5: plugin-sandbox resolved msg.result raw — untrusted plugin output flowed into prompts unsanitized. Added bounded recursive sanitizePluginResult(): every string passes the injection pipeline; depth/node caps stop a pathological return; injection is audited. Docs truth-up (STRIDE.md table is now authoritative + audit note): - E-2 ❌ not implemented (IsolatedAgentRunner exists but unwired; agents run in-process — the architecture diagram was already correct). - T-4 ❌ not implemented (per-entry hash only; no cross-entry chain). - S-1 / S-3 / T-2⚠️ downgraded (oversold: external-replay-only; no caller auth; key falls back to EOS_AGENT_SECRET). - S-2/E-1 residual noted (dead unauthenticated server.ts approval endpoints emit an unconsumed event — re-introduction footgun). - README "single-process" limitation corrected: it previously repeated the false E-2 worker_thread claim. Full suite 121/121. https://claude.ai/code/session_01ArAvRMiZgCwF5oNj3r94Ap
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
After D-6 was found marked
✅ resolvedwithout an implementation, every✅STRIDE claim was re-audited line-by-line against the code. The pattern repeated. This PR fixes the genuine gaps (with attack-path regression tests), truth-ups the docs so they stop overstating coverage, and adds a CI gate that structurally prevents this class of drift from recurring.Audit result (24
✅claims)IsolatedAgentRunnerexists, zero callers; agents run in-process)EOS_AGENT_SECRET)server.tsstill exposes unauthenticated/api/approvals/:id/{approve,deny}emitting an unconsumed event — dead code, not a live bypass)Real fixes (each with a regression test that demonstrates the attack path)
core/AgentRegistry.register()silentlyunregister()d + overwrote a duplicate id (agent identity is the trust anchor for ApprovalGatetrustedAgents). Now rejects the collision and emits asafety.violationaudit event.msg.resultraw into prompts. Added bounded recursivesanitizePluginResult(): every string passes the injection pipeline; depth/node caps; injection audited.previousHashchained fromGENESIS; streamingverifyChain()fails closed on tamper/deletion/reorder; legacy pre-chain entries stay verifiable; chain head bootstraps from disk across restarts.Docs truth-up
docs/STRIDE.md: added an❌legend + dated Verification Audit Corrections note; the Finding Summary table is now authoritative (E-2 ❌; S-1/S-3/T-2README.md: the "single-process architecture" limitation previously repeated the false E-2 worker_thread claim (introduced in PR Claude/fix glasswally agent glitch #38) — corrected to state plainly all agents share one process/heap; only the plugin sandbox is real isolation.Structural fix — claim/evidence CI gate
scripts/check-stride-claims.mjs(npm run check:stride, wired intosecurity.yml): every STRIDE finding must have a manifest entry; a✅must be test-backed (an attack-path test that references the id) or explicitly audit-attested with a code anchor; a non-✅must declareresolved:false. Doc and manifest cannot silently diverge — the gate already caught one such drift on T-4 during development. This is the durable fix for the D-6 class of bug.Deliberately out of scope
IsolatedAgentRunner): a runtime rearchitecture with high destabilization risk and modest marginal security given the verified in-process controls + the real plugin sandbox. Left honestly marked ❌ as a roadmap decision (per maintainer: may be downgraded rather than built).server.tsapproval endpoints (touches external API surface) — flagged in docs, not removed here.Test plan
npm test— 126/126, 10 suitesnpm run check:stride— PASS (29 findings: 4 test-backed, 19 audit-attested, 6 not-resolved)agent-registry-collision(E-5),plugin-return-sanitization(T-5),decision-ledger-chain(T-4: tamper / deletion / reorder / legacy back-compat)tests/security/model-guard.tserrors remain, untouched)