mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-09-10 10:12:20 +02:00
fix(agent): retire superseded approvals
This commit is contained in:
+16
-3
@@ -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,
|
||||
):
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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"})}]
|
||||
|
||||
Reference in New Issue
Block a user