fix(security): close eval_scheme AI-residency bypass (#478) + doc hygiene pass - #519
Merged
Merged
Conversation
…iene pass ## #478: eval_scheme bypasses AI-residency filtering (security-relevant) check_kb_residency's classification gate only ever inspected the outer MCP tool call's name. eval_scheme is itself an MCP tool but not kb_*-prefixed, so classify_kb_tool never saw it and the gate unconditionally Allowed regardless of what the queued Scheme code did -- an AI agent could call (eval_scheme "(kb-graph-view-state)") (or 17 sibling KB-reading Scheme primitives across kb_graph_view.rs/kb_preview.rs/kb_queries.rs/ kb_primitives.rs/io_packages.rs) and read fully unfiltered content from a LocalModelsOnly-restricted KB, completely bypassing ADR-048's mechanism. mae-scheme's SharedState has no requester-identity field at all (used identically for AI and human evals), so the primitives themselves are structurally unable to self-gate. Fixed with a chokepoint inside check_kb_residency itself: a special case for tool_name == "eval_scheme", scanning the queued code string against a static list of the 18 sensitive primitive names. If the code references one AND any registered KB is currently residency-restricted, the WHOLE eval_scheme call is denied outright (not filtered) -- arbitrary Scheme code has no reliable post-hoc result-filtering point the way a typed MCP tool response does. Both real call sites (ai_event_handler.rs:172 embedded, :895 external-MCP) get the fix for free since both already funnel through check_kb_residency unchanged -- zero call-site changes. 7 new adversarial tests: the exact reported bypass denied; a second sensitive primitive with no MCP sibling (kb-get-block) denied; denial for a restricted registered instance (not just primary); no false-positive on unrelated eval_scheme code even with a restriction active; no false-positive when nothing is restricted even with sensitive code; local (Ollama) provider stays exempt; one real end-to-end test through handle_mcp_request's actual dispatch path. Verified the deny-path tests genuinely fail against the pre-fix code (temporarily disabled the new check, confirmed 4/7 tests failed with the exact leaked content visible in the end-to-end test's own failure output, restored the fix, confirmed all 7 pass). ADR-048 updated with an implementation note + Status corrected from stale "Proposed" to "Accepted" (the classify_kb_tool mechanism it describes has long been shipped and tested). ## Hygiene pass (bundled per repo preference for fewer PRs/CI cycles) - Closed 9 GitHub issues for already-shipped work with evidence comments: #416/#420/#421/#422 (ADR-061 epic+phases, doc already said "all six phases implemented"), #500 (macOS babel banner fix), #506 (macOS setsid fallback), #470 (nextest adoption), #389 (macOS CI standard workflow), #491 (sccache cross-job caching). - Verified #454 (ADR-067 Phase E) with fresh evidence (not just trusting a prior note) before closing -- confirmed the real adversarial timeline fixture (had_full_replication_window) exists and passes, surfaced via kb_sharing_status rather than literally kb_audit/kb_health as the issue text specified (noted as an honest caveat, not silently closed over). Closed #348 (ROADMAP.md/KB_SHARING.md docs-vs-reality drift) -- both citations already fixed in an earlier pass without the issue being closed. - Added 3 missing @ai-caution:[architecture-debt] cross-references to ROADMAP.md per CLAUDE.md's own tagging convention (schema.rs panic landmines, federation.rs Incremental-ingest staleness, db.rs pub(super) signature-stability note) -- these are invariant/landmine markers, not file-size debt, so they don't belong in mae-audit.md's size-ceiling-scoped "Known exceptions" list. - Refreshed epic #394's stale checklist and ADR-057's progress-note table -- both still said ADR-061 was "Not started," which had gone stale relative to the already-landed commits. Verified: cargo build --workspace --release --features gui clean, cargo clippy -p mae -p mae-core -p mae-ai --all-targets -D warnings clean, cargo nextest run -p mae --bin mae (443 tests) + mae-kb --lib --features storage-sqlite (356 tests) all pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fixes the "ADR KB staleness gate (ADR-059 Phase E)" CI failure this PR introduced: ADR-048's Status line flipped Proposed -> Accepted earlier in this branch, and `make verify-adr-kb-sync` correctly caught that assets/mae-adr.cozo(.sha256) hadn't been regenerated in the same range. Regenerated via `make adr-kb`, re-verified with `make verify-adr-kb-sync BASE=origin/main` locally (matches the exact CI invocation) -- now reports "Header field(s) changed in 1 file(s) and assets/mae-adr.cozo.sha256 was updated in the same range -- ADR KB is in sync." Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Prompted by hitting exactly this failure in CI on this same branch: an ADR-048 Status header change landed without assets/mae-adr.cozo(.sha256) being regenerated in the same commit, caught by CI's "ADR KB staleness gate" rather than locally. Mirrors the existing code-map-freshness check immediately above it -- same pattern, same rationale (catch it before it costs a full CI cycle, not just at the CI gate). Verified both directions, not just the trivial pass: temporarily recreated the exact failure (header change committed, KB assets reset to their pre-regeneration state) and confirmed the hook exits 1 with a clear message; restored the correct state and confirmed it passes again. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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.
#478:
eval_schemebypasses AI-residency filtering (security-relevant)check_kb_residency's classification gate only ever inspected the outer MCP tool call's name.eval_schemeis itself an MCP tool but notkb_*-prefixed, soclassify_kb_toolnever saw it and the gate unconditionallyAllowed regardless of what the queued Scheme code did — an AI agent could call(eval_scheme "(kb-graph-view-state)")(or 17 sibling KB-reading Scheme primitives acrosskb_graph_view.rs/kb_preview.rs/kb_queries.rs/kb_primitives.rs/io_packages.rs) and read fully unfiltered content from aLocalModelsOnly-restricted KB, completely bypassing ADR-048's mechanism.mae-scheme'sSharedStatehas no requester-identity field at all (used identically for AI and human evals), so the primitives themselves are structurally unable to self-gate.Fix: a chokepoint inside
check_kb_residencyitself — a special case fortool_name == "eval_scheme", scanning the queued code string against a static list of the 18 sensitive primitive names. If the code references one AND any registered KB is currently residency-restricted, the WHOLEeval_schemecall is denied outright (not filtered) — arbitrary Scheme code has no reliable post-hoc result-filtering point the way a typed MCP tool response does. Both real call sites (ai_event_handler.rs:172embedded,:895external-MCP) get the fix for free since both already funnel throughcheck_kb_residencyunchanged — zero call-site changes.7 new adversarial tests, including one real end-to-end test through
handle_mcp_request's actual dispatch path. Verified the deny-path tests genuinely fail against the pre-fix code (temporarily disabled the new check, confirmed 4/7 tests failed with the exact leaked content visible in the end-to-end test's own failure output, restored the fix, confirmed all 7 pass).ADR-048 updated with an implementation note + Status corrected from stale "Proposed" to "Accepted".
Hygiene pass (bundled per repo preference for fewer PRs/CI cycles)
@ai-caution:[architecture-debt]cross-references to ROADMAP.md.Test plan
cargo build --workspace --release --features guicleancargo clippy -p mae -p mae-core -p mae-ai --all-targets -- -D warningscleancargo nextest run -p mae --bin mae— 443 tests passcargo nextest run -p mae-kb --lib --features storage-sqlite— 356 tests pass🤖 Generated with Claude Code