diff --git a/routes/chat_routes.py b/routes/chat_routes.py index b388e505a..0b181796f 100644 --- a/routes/chat_routes.py +++ b/routes/chat_routes.py @@ -916,6 +916,7 @@ def setup_chat_routes( ) exact_tool_approval = None pending_tool_approval = None + retired_tool_approval_taint = False tool_approval_continuation = False # Workspace: confine the agent's file/shell tools to this folder. workspace, workspace_rejected = _resolve_request_workspace( @@ -1122,6 +1123,15 @@ def setup_chat_routes( f"Denied the {pending_tool_approval.tool_name} action shown above." ) chat_mode = "agent" + else: + # A normal user message supersedes the card that was waiting + # in this thread. Retire its opaque grant, but preserve the + # originating provenance for this turn so dismissing a card + # cannot make the same model-requested action authoritative. + retired_tool_approval_taint = tool_approval_store.retire_for_session( + owner=owner, + session_id=session, + ) _reconcile_selected_route_from_request(request, sess, session, form_data, owner=owner) if _clear_orphaned_session_endpoint(sess, owner=owner): raise HTTPException(400, "Selected model endpoint was removed. Pick another model in Settings.") @@ -2217,9 +2227,12 @@ def setup_chat_routes( uploaded_files=ctx.uploaded_files, defer_context_shaping=_foreground_policy.enabled, external_untrusted_context_seen=bool( - tool_approval_continuation - and pending_tool_approval - and pending_tool_approval.external_untrusted_context_seen + retired_tool_approval_taint + or ( + tool_approval_continuation + and pending_tool_approval + and pending_tool_approval.external_untrusted_context_seen + ) ), exact_approval=exact_tool_approval, ): diff --git a/src/tool_approvals.py b/src/tool_approvals.py index 8b7fd3d63..d3707c5f6 100644 --- a/src/tool_approvals.py +++ b/src/tool_approvals.py @@ -358,5 +358,35 @@ class ToolApprovalStore: self._purge_expired_locked(now) return self._pending.get(str(approval_id or "")) + def retire_for_session(self, *, owner: Any, session_id: Any) -> bool: + """Discard pending actions superseded by an ordinary user turn. + + Returns whether any retired action carried external provenance, so the + caller can preserve that security state without treating the new user + message as an approval continuation. + """ + now = time.time() + normalized_owner = _normalized_owner(owner) + normalized_session = str(session_id or "") + if not normalized_session: + return False + with self._lock: + self._purge_expired_locked(now) + retired_ids = [ + approval_id + for approval_id, pending in self._pending.items() + if ( + pending.owner == normalized_owner + and pending.session_id == normalized_session + ) + ] + carried_taint = any( + self._pending[approval_id].external_untrusted_context_seen + for approval_id in retired_ids + ) + for approval_id in retired_ids: + self._pending.pop(approval_id, None) + return carried_taint + tool_approval_store = ToolApprovalStore() diff --git a/tests/test_fenced_example_not_executed_for_native_models.py b/tests/test_fenced_example_not_executed_for_native_models.py index e6dccae77..1e61de017 100644 --- a/tests/test_fenced_example_not_executed_for_native_models.py +++ b/tests/test_fenced_example_not_executed_for_native_models.py @@ -50,6 +50,8 @@ def _patch_common(monkeypatch, exec_calls): monkeypatch.setattr(al, "get_setting", lambda key, default=None: default, raising=False) monkeypatch.setattr(al, "get_mcp_manager", lambda: None, raising=False) monkeypatch.setattr(al, "estimate_tokens", lambda *a, **k: 10, raising=False) + # These tests exercise tool-channel parsing, not owner authorization. + monkeypatch.setattr(al, "blocked_tools_for_owner", lambda owner: set(), raising=False) async def _fake_exec(block, *a, **k): exec_calls.append(block) diff --git a/tests/test_foreground_model_routing.py b/tests/test_foreground_model_routing.py index 90a7e6f5f..e2ceb8762 100644 --- a/tests/test_foreground_model_routing.py +++ b/tests/test_foreground_model_routing.py @@ -376,6 +376,34 @@ async def test_chat_stream_denial_keeps_originating_run_tainted(monkeypatch): assert chat_routes.tool_approval_store.peek(pending.approval_id) is None +@pytest.mark.asyncio +async def test_chat_stream_normal_reply_retires_pending_action_but_keeps_taint( + monkeypatch, +): + from src.tool_capabilities import capabilities_for_action + + captured = {} + endpoint = _chat_stream_endpoint(monkeypatch, "agent", captured) + pending = chat_routes.tool_approval_store.create( + owner="alice", + session_id="session-1", + origin_run_id="run-1", + tool_name="bash", + content="printf retry", + workspace=None, + external_untrusted_context_seen=True, + capabilities=capabilities_for_action("bash", "printf retry"), + ) + + response = await endpoint(_RouteRequest("agent")) + async for _ in response.body_iterator: + pass + + assert "exact_approval" not in captured + assert captured["agent_external_untrusted_context_seen"] is True + assert chat_routes.tool_approval_store.peek(pending.approval_id) is None + + @pytest.mark.asyncio async def test_chat_stream_approval_ignores_research_and_new_attachments(monkeypatch): from src.tool_capabilities import capabilities_for_action diff --git a/tests/test_tool_approvals.py b/tests/test_tool_approvals.py index 2e13ffa32..cd988c7ae 100644 --- a/tests/test_tool_approvals.py +++ b/tests/test_tool_approvals.py @@ -105,6 +105,17 @@ def test_new_session_approval_supersedes_prior_pending_action(): assert store.peek(second.approval_id) == second +def test_ordinary_session_turn_retires_pending_action_and_preserves_taint(): + store = ToolApprovalStore() + pending = _pending(store, owner="Alice", session_id="session-1") + + assert store.retire_for_session(owner="bob", session_id="session-1") is False + assert store.peek(pending.approval_id) == pending + assert store.retire_for_session(owner="alice", session_id="session-1") is True + assert store.peek(pending.approval_id) is None + assert store.retire_for_session(owner="alice", session_id=None) is False + + def test_independent_headless_runs_do_not_supersede_each_other(): store = ToolApprovalStore() first = _pending(store, session_id=None, origin_run_id="headless-1") diff --git a/tests/test_tool_task_cancelled_on_disconnect.py b/tests/test_tool_task_cancelled_on_disconnect.py index cb086dd20..f55df3f41 100644 --- a/tests/test_tool_task_cancelled_on_disconnect.py +++ b/tests/test_tool_task_cancelled_on_disconnect.py @@ -47,6 +47,8 @@ def test_tool_task_cancelled_on_generator_close(monkeypatch): monkeypatch.setattr(al, "get_setting", lambda key, default=None: default, raising=False) monkeypatch.setattr(al, "get_mcp_manager", lambda: None, raising=False) monkeypatch.setattr(al, "estimate_tokens", lambda *a, **k: 10, raising=False) + # This test exercises task cancellation, not owner authorization. + monkeypatch.setattr(al, "blocked_tools_for_owner", lambda owner: set(), raising=False) monkeypatch.setattr(al, "execute_tool_block", _slow_exec, raising=False) native_calls = [{"name": "bash", "arguments": json.dumps({"command": "sleep 60"})}]