From 62d7fe990aaeac0cf738587cb509131a5f4c0832 Mon Sep 17 00:00:00 2001 From: RaresKeY <158580472+RaresKeY@users.noreply.github.com> Date: Sat, 25 Jul 2026 16:55:27 +0000 Subject: [PATCH] feat(agent): add exact approvals and run modes --- THREAT_MODEL.md | 10 + core/database.py | 64 +++++ core/models.py | 1 + core/session_manager.py | 7 + routes/chat_routes.py | 108 ++++++++- routes/session_routes.py | 8 +- src/agent_loop.py | 223 ++++++++++++++++- src/agent_run_policy.py | 180 ++++++++++++++ src/agent_tools/subprocess_tools.py | 76 ++++-- src/bg_jobs.py | 58 +++-- src/bg_monitor.py | 1 + src/request_models.py | 4 + src/teacher_escalation.py | 8 + src/tool_approvals.py | 297 +++++++++++++++++++++++ src/tool_capabilities.py | 2 + src/tool_execution.py | 124 +++++++++- static/app.js | 56 ++++- static/index.html | 27 ++- static/js/chat.js | 38 ++- static/js/chatRenderer.js | 34 ++- static/js/init.js | 10 + static/js/sessions.js | 5 +- tests/test_agent_run_policy.py | 279 +++++++++++++++++++++ tests/test_ask_user_persistence.py | 12 + tests/test_execution_sandbox.py | 81 +++++++ tests/test_external_context_tool_gate.py | 217 ++++++++++++++++- tests/test_session_mode_helpers.py | 28 ++- tests/test_teacher_eval_tier2.py | 12 + 28 files changed, 1896 insertions(+), 74 deletions(-) create mode 100644 src/agent_run_policy.py create mode 100644 src/tool_approvals.py create mode 100644 tests/test_agent_run_policy.py diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md index 5616f1ae3..024a1675e 100644 --- a/THREAT_MODEL.md +++ b/THREAT_MODEL.md @@ -60,6 +60,16 @@ External content that reaches the LLM is treated as untrusted via `src/prompt_se **Untrusted surfaces that must go through this wrapper:** web search results, fetched URLs, emails (read), saved memories, skill text, notes, and any tool output sourced from outside the server. Injecting untrusted content directly into the system role is a security bug. +### Agent Run Authority + +Model output requests an action; it does not authorize one. `src/tool_capabilities.py` classifies each built-in tool's effects and result integrity, while `src/agent_run_policy.py` combines those fixed classifications with the thread's server-owned security mode: + +- **Ask:** public and workspace observation can proceed, but private reads, writes, code execution, egress, external side effects, UI effects, admin changes, destructive actions, and unknown tools require an exact approval. +- **Sandbox (default):** code execution stays inside the workspace sandbox. Network egress, external side effects, admin changes, and destructive actions always require an exact approval. Once external untrusted context has influenced the run, any later high-impact action also requires approval. +- **Full access:** an admin or intentional single-user deployment may explicitly opt into direct execution with that user's normal OS permissions. This is never the default, and route, agent-loop, and dispatcher gates reject it for non-admin users. + +An approval is an opaque, expiring, one-use server record bound to the owner, session, origin run, exact tool name and input, workspace, security mode, effect classification, and external-context state. The browser submits only the opaque approval ID and the user's approve/deny decision. On approval, the server executes its sealed copy before the next model turn; natural-language confirmation and a model-repeated or modified command carry no authority. + ## Security Headers `core/middleware.py:SecurityHeadersMiddleware` sets headers on every response: diff --git a/core/database.py b/core/database.py index a9ad90b8b..3c6a21326 100644 --- a/core/database.py +++ b/core/database.py @@ -220,6 +220,7 @@ class Session(TimestampMixin, Base): total_input_tokens = Column(Integer, default=0) total_output_tokens = Column(Integer, default=0) mode = Column(String, nullable=True) # 'agent', 'chat', or 'research' + security_mode = Column(String, nullable=False, default="sandbox") crew_member_id = Column(String, nullable=True) # links to crew_members.id # Relationship to chat messages @@ -249,6 +250,8 @@ class Session(TimestampMixin, Base): 'total_input_tokens': self.total_input_tokens or 0, 'total_output_tokens': self.total_output_tokens or 0, 'crew_member_id': self.crew_member_id, + 'mode': self.mode, + 'security_mode': self.security_mode or 'sandbox', } class ChatMessage(Base): @@ -1172,6 +1175,36 @@ def _migrate_add_mode_column(): except Exception: pass +def _migrate_add_security_mode_column(): + """Add the fail-safe agent run mode to existing session databases.""" + import sqlite3 + db_path = DATABASE_URL.replace("sqlite:///", "") + if not os.path.exists(db_path): + return + conn = None + try: + conn = sqlite3.connect(db_path) + cursor = conn.execute("PRAGMA table_info(sessions)") + columns = [row[1] for row in cursor.fetchall()] + if "security_mode" not in columns: + conn.execute( + "ALTER TABLE sessions ADD COLUMN security_mode TEXT " + "NOT NULL DEFAULT 'sandbox'" + ) + conn.commit() + logging.getLogger(__name__).info( + "Migrated: added 'security_mode' column to sessions" + ) + except Exception as e: + logging.getLogger(__name__).warning( + f"Migration check for security_mode failed: {e}" + ) + finally: + try: + conn.close() + except Exception: + pass + def _migrate_add_folder_column(): """Add folder column to sessions table if it doesn't exist.""" import sqlite3 @@ -1942,6 +1975,7 @@ def init_db(): _migrate_add_folder_column() _migrate_add_token_columns() _migrate_add_mode_column() + _migrate_add_security_mode_column() _migrate_add_multiuser_owner_columns() _migrate_add_gallery_caption_column() _migrate_add_api_token_scopes_column() @@ -2515,6 +2549,36 @@ def set_session_mode(session_id: str, mode: str) -> bool: logger.warning("Failed to persist mode %r for session %s", mode, session_id) return False +def get_session_security_mode(session_id: str) -> str: + """Return the persisted agent authority mode, defaulting safely.""" + try: + with get_db_session() as db: + value = db.query(Session.security_mode).filter( + Session.id == session_id + ).scalar() + return value if value in {"ask", "sandbox", "full_access"} else "sandbox" + except Exception: + logger.warning("Failed to read security mode for session %s", session_id) + return "sandbox" + +def set_session_security_mode(session_id: str, mode: str) -> bool: + """Persist a validated agent authority mode; invalid values fail closed.""" + if mode not in {"ask", "sandbox", "full_access"}: + return False + try: + with get_db_session() as db: + db.query(Session).filter(Session.id == session_id).update( + {"security_mode": mode} + ) + return True + except Exception: + logger.warning( + "Failed to persist security mode %r for session %s", + mode, + session_id, + ) + return False + def get_session_by_id(session_id: str): """Get a session by ID""" with get_db_session() as db: diff --git a/core/models.py b/core/models.py index 56f05dc4e..ebef0ecd3 100644 --- a/core/models.py +++ b/core/models.py @@ -74,6 +74,7 @@ class Session: owner: Optional[str] = None is_important: bool = False message_count: int = 0 + security_mode: str = "sandbox" def __post_init__(self): if self.headers is None: diff --git a/core/session_manager.py b/core/session_manager.py index 6eb493e95..fdcc5f61c 100644 --- a/core/session_manager.py +++ b/core/session_manager.py @@ -134,6 +134,7 @@ class SessionManager: history=[], owner=getattr(db_session, "owner", None), is_important=getattr(db_session, "is_important", False) or False, + security_mode=getattr(db_session, "security_mode", None) or "sandbox", ) session.message_count = getattr(db_session, "message_count", 0) or 0 return session @@ -192,6 +193,7 @@ class SessionManager: history=history, owner=getattr(db_session, 'owner', None), is_important=getattr(db_session, 'is_important', False) or False, + security_mode=getattr(db_session, "security_mode", None) or "sandbox", ) session.message_count = getattr(db_session, 'message_count', len(history)) @@ -445,6 +447,9 @@ class SessionManager: session.owner = getattr(db_session, "owner", None) session.is_important = getattr(db_session, "is_important", False) or False session.message_count = getattr(db_session, "message_count", session.message_count) or 0 + session.security_mode = ( + getattr(db_session, "security_mode", None) or "sandbox" + ) return True except Exception as e: logger.error(f"Error syncing session metadata {session_id}: {e}") @@ -513,6 +518,7 @@ class SessionManager: rag=rag, headers={}, owner=owner, + security_mode="sandbox", created_at=datetime.now(timezone.utc), updated_at=datetime.now(timezone.utc) ) @@ -527,6 +533,7 @@ class SessionManager: rag=rag, headers={}, owner=owner, + security_mode="sandbox", ) self.sessions[session_id] = session diff --git a/routes/chat_routes.py b/routes/chat_routes.py index b081d5f1c..2c8c6457a 100644 --- a/routes/chat_routes.py +++ b/routes/chat_routes.py @@ -27,7 +27,13 @@ from core.exceptions import SessionNotFoundError from src.auth_helpers import effective_user, get_current_user from routes.session_routes import _verify_session_owner from routes.document_helpers import _owner_session_filter -from core.database import SessionLocal, get_session_mode, set_session_mode +from core.database import ( + SessionLocal, + get_session_mode, + get_session_security_mode, + set_session_mode, + set_session_security_mode, +) from core.database import Session as DBSession, ChatMessage as DBChatMessage from core.database import Document as DBDocument, ModelEndpoint from core.log_safety import redact_url @@ -49,6 +55,9 @@ from src.tool_policy import ( is_web_search_explicitly_denied, web_search_enabled_for_turn, ) +from src.agent_run_policy import AgentRunMode, parse_agent_run_mode +from src.tool_approvals import tool_approval_store +from src.tool_security import owner_is_admin_or_single_user logger = logging.getLogger(__name__) @@ -734,6 +743,19 @@ def setup_chat_routes( incognito = str(form_data.get("incognito", "")).lower() == "true" plan_mode = str(form_data.get("plan_mode") or (body or {}).get("plan_mode") or "").lower() == "true" chat_mode = str(form_data.get("mode", "")).lower() # 'chat' or 'agent' + requested_security_mode = ( + form_data.get("security_mode") + or (body or {}).get("security_mode") + ) + tool_approval_id = ( + form_data.get("tool_approval_id") + or (body or {}).get("tool_approval_id") + ) + tool_approval_decision = ( + form_data.get("tool_approval_decision") + or (body or {}).get("tool_approval_decision") + ) + exact_tool_approval = None # Workspace: confine the agent's file/shell tools to this folder. workspace, workspace_rejected = _resolve_request_workspace( request, form_data.get("workspace") @@ -880,6 +902,82 @@ def setup_chat_routes( _verify_session_owner(request, session) sess = session_manager.get_session(session) owner = effective_user(request) + persisted_security_mode = ( + getattr(sess, "security_mode", None) + or get_session_security_mode(session) + ) + if requested_security_mode not in (None, ""): + requested_security_mode = str(requested_security_mode).strip().lower() + if requested_security_mode not in { + AgentRunMode.ASK.value, + AgentRunMode.SANDBOX.value, + AgentRunMode.FULL_ACCESS.value, + }: + raise HTTPException(400, "Invalid agent security mode.") + effective_security_mode = parse_agent_run_mode( + requested_security_mode + ) + else: + effective_security_mode = parse_agent_run_mode( + persisted_security_mode + ) + if ( + effective_security_mode is AgentRunMode.FULL_ACCESS + and not owner_is_admin_or_single_user(owner) + ): + if requested_security_mode not in (None, ""): + raise HTTPException( + 403, + "Full access agent mode requires an admin user.", + ) + effective_security_mode = AgentRunMode.SANDBOX + if tool_approval_id: + pending_approval = tool_approval_store.peek(tool_approval_id) + if ( + pending_approval is None + or pending_approval.owner != str(owner or "").strip().casefold() + or pending_approval.session_id != str(session) + ): + raise HTTPException( + 409, + "This tool approval is invalid, expired, or belongs to another thread.", + ) + decision = str(tool_approval_decision or "").strip().lower() + if decision not in {"approve", "deny"}: + raise HTTPException(400, "Invalid tool approval decision.") + if ( + plan_mode + or pending_approval.security_mode is not effective_security_mode + ): + raise HTTPException( + 409, + "The thread security state changed; review the action again.", + ) + exact_tool_approval = tool_approval_store.consume( + tool_approval_id, + decision=decision, + owner=owner, + session_id=session, + ) + if decision == "approve" and exact_tool_approval is None: + raise HTTPException( + 409, + "This tool approval could not be consumed.", + ) + if decision == "approve": + message = ( + f"Approved the exact {pending_approval.tool_name} action " + "shown above once." + ) + # The sealed server record, not mutable composer state, + # chooses the approved action's original workspace. + workspace = pending_approval.workspace or None + workspace_rejected = None + else: + message = ( + f"Denied the {pending_approval.tool_name} action shown above." + ) + chat_mode = "agent" _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.") @@ -1228,6 +1326,12 @@ def setup_chat_routes( _effective_mode = 'research' if effective_do_research else (chat_mode or 'chat') if _effective_mode in ('agent', 'research', 'chat'): set_session_mode(session, _effective_mode) + if ( + requested_security_mode not in (None, "") + or effective_security_mode.value != persisted_security_mode + ): + set_session_security_mode(session, effective_security_mode.value) + sess.security_mode = effective_security_mode.value async def stream_with_save() -> AsyncGenerator[str, None]: # _effective_mode is read-only here; closure captures it from @@ -1716,6 +1820,8 @@ def setup_chat_routes( workspace=workspace or None, forced_tools=_forced_tools, uploaded_files=ctx.uploaded_files, + security_mode=effective_security_mode.value, + exact_approval=exact_tool_approval, ): if chunk.startswith("data: ") and not chunk.startswith("data: [DONE]"): try: diff --git a/routes/session_routes.py b/routes/session_routes.py index dc29a64e4..25a90d69d 100644 --- a/routes/session_routes.py +++ b/routes/session_routes.py @@ -268,8 +268,9 @@ def setup_session_routes( updated_map = {} last_msg_map = {} mode_map = {} + security_mode_map = {} msg_count_map = {} - q = db.query(DbSession.id, DbSession.folder, DbSession.total_input_tokens, DbSession.total_output_tokens, DbSession.is_important, DbSession.created_at, DbSession.updated_at, DbSession.last_message_at, DbSession.mode, DbSession.message_count).filter(DbSession.archived == False) + q = db.query(DbSession.id, DbSession.folder, DbSession.total_input_tokens, DbSession.total_output_tokens, DbSession.is_important, DbSession.created_at, DbSession.updated_at, DbSession.last_message_at, DbSession.mode, DbSession.security_mode, DbSession.message_count).filter(DbSession.archived == False) q = owner_filter(q, DbSession, user) rows = q.all() for row in rows: @@ -286,6 +287,7 @@ def setup_session_routes( else (row.created_at.isoformat() if row.created_at else None)) ) mode_map[row.id] = row.mode + security_mode_map[row.id] = row.security_mode or "sandbox" msg_count_map[row.id] = row.message_count or 0 # Sessions with active documents that have content from sqlalchemy import func @@ -319,6 +321,7 @@ def setup_session_routes( "has_documents": s.id in doc_session_ids, "has_images": s.id in img_session_ids, "mode": mode_map.get(s.id), + "security_mode": security_mode_map.get(s.id, "sandbox"), "message_count": msg_count_map.get(s.id, 0)} for s in user_sessions.values() if not s.archived @@ -456,7 +459,8 @@ def setup_session_routes( name=session.name, model=model_to_use, rag=str(rag).lower() == "true" if rag else False, - archived=False + archived=False, + security_mode="sandbox", ) @router.patch("/session/{sid}") def rename_session( diff --git a/src/agent_loop.py b/src/agent_loop.py index ed53965b1..52d89a88c 100644 --- a/src/agent_loop.py +++ b/src/agent_loop.py @@ -23,13 +23,22 @@ from src.llm_core import ( from src.model_context import estimate_tokens from src.settings import get_setting from src.prompt_security import untrusted_context_message -from src.tool_security import blocked_tools_for_owner, plan_mode_disabled_tools +from src.tool_security import ( + blocked_tools_for_owner, + owner_is_admin_or_single_user, + plan_mode_disabled_tools, +) from src.tool_policy import GUIDE_ONLY_DIRECTIVE, WEB_TOOL_NAMES, ToolPolicy from src.tool_capabilities import ( ToolRunSecurityContext, blocked_tool_result, messages_contain_external_untrusted_context, ) +from src.agent_run_policy import ( + AgentRunPolicy, + AuthorizationOutcome, +) +from src.tool_approvals import ExactToolApproval, tool_approval_store from src.tool_utils import _truncate, get_mcp_manager from src.agent_tools import ( parse_tool_blocks, @@ -3107,6 +3116,8 @@ async def stream_agent_loop( uploaded_files: Optional[List[Dict]] = None, workload: str = "foreground", external_untrusted_context_seen: bool = False, + security_mode: str = "sandbox", + exact_approval: Optional[ExactToolApproval] = None, _is_teacher_run: bool = False, ) -> AsyncGenerator[str, None]: """Streaming agent loop generator. @@ -3123,9 +3134,23 @@ async def stream_agent_loop( run_security = ToolRunSecurityContext( external_untrusted_context_seen=( bool(external_untrusted_context_seen) + or bool( + exact_approval + and exact_approval.pending.external_untrusted_context_seen + ) or messages_contain_external_untrusted_context(messages) ) ) + run_policy = AgentRunPolicy.for_mode(security_mode) + if ( + run_policy.mode.value == "full_access" + and not owner_is_admin_or_single_user(owner) + ): + logger.warning( + "Full-access agent mode rejected by loop backstop for owner=%r", + owner, + ) + run_policy = AgentRunPolicy.for_mode("sandbox") mcp_mgr = get_mcp_manager() prep_timings: Dict[str, float] = {} disabled_tools = set(disabled_tools or []) @@ -3889,6 +3914,150 @@ async def stream_agent_loop( # so the user can resume instead of the turn silently stalling. _exhausted_rounds = False + # Resume an approved action from the server-owned sealed record. Execute it + # before asking the model for another turn; asking the model to repeat an + # action after a conversational "yes" would let it substitute new content. + if exact_approval is not None: + approved = exact_approval.pending + approved_block = ToolBlock(approved.tool_name, approved.content) + approved_display = approved.content.strip() + approval_matches = exact_approval.matches( + owner=owner, + session_id=session_id, + tool_name=approved.tool_name, + content=approved.content, + workspace=workspace, + security_mode=run_policy.mode, + ) + if approval_matches: + yield ( + "data: " + + json.dumps( + { + "type": "tool_start", + "tool": approved.tool_name, + "command": approved_display[:240], + "full_command": approved_display, + "round": 0, + "approved": True, + } + ) + + "\n\n" + ) + desc, approved_result = await execute_tool_block( + approved_block, + session_id=session_id, + disabled_tools=disabled_tools, + tool_policy=tool_policy, + owner=owner, + workspace=workspace, + security_context=run_security, + run_policy=run_policy, + exact_approval=exact_approval, + ) + total_tool_calls += 1 + approved_output = str( + approved_result.get("output") + or approved_result.get("stdout") + or approved_result.get("response") + or approved_result.get("results") + or approved_result.get("content") + or approved_result.get("error") + or "(no output)" + ) + approved_event = { + "type": "tool_output", + "tool": approved.tool_name, + "command": approved_display[:240] if approval_matches else "", + "output": _truncate(approved_output), + "exit_code": approved_result.get("exit_code"), + "approved": True, + } + for key in ( + "image_url", + "image_id", + "image_prompt", + "image_model", + "image_size", + "image_quality", + "doc_id", + "title", + "language", + "content", + "version", + "action", + "ui_event", + ): + if key in approved_result: + approved_event[key] = approved_result[key] + yield "data: " + json.dumps(approved_event) + "\n\n" + if approved_result.get("ui_event"): + yield ( + "data: " + + json.dumps({"type": "ui_control", "data": approved_result}) + + "\n\n" + ) + if approved_result.get("doc_id") and approved_result.get("content") is not None: + yield ( + "data: " + + json.dumps( + { + "type": "doc_update", + "doc_id": approved_result["doc_id"], + "title": approved_result.get("title", ""), + "language": approved_result.get("language", ""), + "content": approved_result.get("content", ""), + "version": approved_result.get("version", 1), + } + ) + + "\n\n" + ) + if approved_result.get("image_url"): + yield ( + "data: " + + json.dumps( + { + "type": "generated_image", + "url": approved_result["image_url"], + **{ + key: approved_result[key] + for key in ( + "image_url", + "image_id", + "image_prompt", + "image_model", + "image_size", + "image_quality", + ) + if key in approved_result + }, + } + ) + + "\n\n" + ) + tool_events.append( + { + "round": 0, + "tool": approved.tool_name, + "desc": desc, + "command": approved_display[:240] if approval_matches else "", + "output": _truncate(approved_output), + "exit_code": approved_result.get("exit_code"), + "approved": True, + "approval_digest": approved.digest[:16], + } + ) + formatted_approved_result = format_tool_result(desc, approved_result) + _append_tool_results( + messages, + "", + [], + [formatted_approved_result], + [formatted_approved_result], + False, + 0, + ) + for round_num in range(1, max_rounds + 1): round_response = "" round_reasoning = "" # reasoning_content deltas (DeepSeek-thinking, vLLM --reasoning-parser) @@ -3958,9 +4127,10 @@ async def stream_agent_loop( all_tool_schemas = [ schema for schema in all_tool_schemas - if run_security.decision_for( - (schema.get("function") or {}).get("name") or schema.get("name") - ).allowed + if run_policy.authorize( + (schema.get("function") or {}).get("name") or schema.get("name"), + run_security, + ).outcome is not AuthorizationOutcome.DENY ] agent_stream_timeout = int(get_setting("agent_stream_timeout_seconds", 300) or 300) @@ -4641,12 +4811,42 @@ async def stream_agent_loop( else: cmd_display = full_command - security_decision = run_security.decision_for(block.tool_type) + security_decision = run_policy.authorize( + block.tool_type, + run_security, + ) _ody_clamped_tool_allowed = ( _ody_notes_finetune_mode and block.tool_type in {"manage_notes", "manage_calendar", "manage_tasks"} ) - if not security_decision.allowed: + if security_decision.outcome is AuthorizationOutcome.REQUIRE_APPROVAL: + pending_approval = tool_approval_store.create( + owner=owner, + session_id=session_id, + origin_run_id=run_security.run_id, + tool_name=block.tool_type, + content=block.content, + workspace=workspace, + security_mode=run_policy.mode, + external_untrusted_context_seen=( + run_security.external_untrusted_context_seen + ), + capabilities=security_decision.capabilities, + ) + desc = f"{block.tool_type}: APPROVAL REQUIRED" + result = { + "output": "Waiting for an exact user approval.", + "exit_code": None, + "approval_required": True, + "ask_user": pending_approval.public_payload( + reason=security_decision.reason, + ), + } + logger.info( + "Exact approval required before tool start: %s", + block.tool_type, + ) + elif security_decision.outcome is AuthorizationOutcome.DENY: desc, result = blocked_tool_result( block.tool_type, security_decision.reason or "Tool blocked by external-context policy.", @@ -4688,6 +4888,7 @@ async def stream_agent_loop( progress_cb=_push_progress, workspace=workspace, security_context=run_security, + run_policy=run_policy, ) finally: # Sentinel so the drainer knows to stop. @@ -5113,6 +5314,10 @@ async def stream_agent_loop( and not result.get("error") ): _ody_doc_tool_completed = True + if _pending_ask_user_event: + # The approval card is a turn boundary. Do not execute any + # later model-supplied block from the same batch. + break # If budget was hit, stop the loop if budget_hit: @@ -5273,6 +5478,12 @@ async def stream_agent_loop( student_tool_events=tool_events, student_reply=full_response, owner=owner, + session_id=session_id, + workspace=workspace, + security_mode=run_policy.mode.value, + external_untrusted_context_seen=( + run_security.external_untrusted_context_seen + ), ): yield evt except Exception as _esc_err: diff --git a/src/agent_run_policy.py b/src/agent_run_policy.py new file mode 100644 index 000000000..7a3c85a1e --- /dev/null +++ b/src/agent_run_policy.py @@ -0,0 +1,180 @@ +"""Deterministic authority policy for one agent run. + +The selected model may request actions, but it cannot choose the authority used +to execute them. A server-owned run mode and tool capability metadata produce +one of three outcomes: execute in the workspace sandbox, execute with the +application user's host permissions, or require an exact user approval. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from enum import Enum +from typing import Any + +from src.tool_capabilities import ( + POST_EXTERNAL_BLOCKED_EFFECTS, + ToolCapabilities, + ToolEffect, + ToolRunSecurityContext, + capabilities_for_tool, +) + + +class AgentRunMode(str, Enum): + ASK = "ask" + SANDBOX = "sandbox" + FULL_ACCESS = "full_access" + + +class ApprovalPolicy(str, Enum): + ALWAYS_FOR_RISK = "always_for_risk" + ON_TRUST_BOUNDARY = "on_trust_boundary" + NEVER = "never" + + +class ExecutionProfile(str, Enum): + WORKSPACE_SANDBOX = "workspace_sandbox" + HOST_FULL_ACCESS = "host_full_access" + + +class NetworkProfile(str, Enum): + BROKERED_ONLY = "brokered_only" + OPEN = "open" + + +class AuthorizationOutcome(str, Enum): + ALLOW_SANDBOXED = "allow_sandboxed" + ALLOW_HOST = "allow_host" + REQUIRE_APPROVAL = "require_approval" + DENY = "deny" + + +@dataclass(frozen=True) +class ToolAuthorization: + outcome: AuthorizationOutcome + reason: str | None = None + capabilities: ToolCapabilities | None = None + + @property + def allowed(self) -> bool: + return self.outcome in { + AuthorizationOutcome.ALLOW_SANDBOXED, + AuthorizationOutcome.ALLOW_HOST, + } + + +_ASK_RISK_EFFECTS = frozenset( + { + ToolEffect.READ_PRIVATE, + ToolEffect.WRITE_WORKSPACE, + ToolEffect.WRITE_PRIVATE, + ToolEffect.EXECUTE_CODE, + ToolEffect.NETWORK_EGRESS, + ToolEffect.EXTERNAL_SIDE_EFFECT, + ToolEffect.UI_SIDE_EFFECT, + ToolEffect.ADMIN_CHANGE, + ToolEffect.DESTRUCTIVE, + } +) + +_SANDBOX_ALWAYS_APPROVE_EFFECTS = frozenset( + { + ToolEffect.NETWORK_EGRESS, + ToolEffect.EXTERNAL_SIDE_EFFECT, + ToolEffect.ADMIN_CHANGE, + ToolEffect.DESTRUCTIVE, + } +) + + +def parse_agent_run_mode(value: Any) -> AgentRunMode: + """Parse a client/database value, failing safely to the sandbox default.""" + if isinstance(value, AgentRunMode): + return value + try: + return AgentRunMode(str(value or "").strip().lower()) + except ValueError: + return AgentRunMode.SANDBOX + + +@dataclass(frozen=True) +class AgentRunPolicy: + mode: AgentRunMode + approval_policy: ApprovalPolicy + execution_profile: ExecutionProfile + network_profile: NetworkProfile + + @classmethod + def for_mode(cls, value: Any) -> "AgentRunPolicy": + mode = parse_agent_run_mode(value) + if mode is AgentRunMode.ASK: + return cls( + mode=mode, + approval_policy=ApprovalPolicy.ALWAYS_FOR_RISK, + execution_profile=ExecutionProfile.WORKSPACE_SANDBOX, + network_profile=NetworkProfile.BROKERED_ONLY, + ) + if mode is AgentRunMode.FULL_ACCESS: + return cls( + mode=mode, + approval_policy=ApprovalPolicy.NEVER, + execution_profile=ExecutionProfile.HOST_FULL_ACCESS, + network_profile=NetworkProfile.OPEN, + ) + return cls( + mode=AgentRunMode.SANDBOX, + approval_policy=ApprovalPolicy.ON_TRUST_BOUNDARY, + execution_profile=ExecutionProfile.WORKSPACE_SANDBOX, + network_profile=NetworkProfile.BROKERED_ONLY, + ) + + def authorize( + self, + tool_name: Any, + security_context: ToolRunSecurityContext, + ) -> ToolAuthorization: + """Classify an action without consulting model-generated text.""" + capabilities = capabilities_for_tool(tool_name) + + if self.mode is AgentRunMode.FULL_ACCESS: + return ToolAuthorization( + AuthorizationOutcome.ALLOW_HOST, + capabilities=capabilities, + ) + + if not capabilities.known: + return ToolAuthorization( + AuthorizationOutcome.REQUIRE_APPROVAL, + "Unknown tools require an exact user approval.", + capabilities, + ) + + if self.mode is AgentRunMode.ASK and capabilities.effects & _ASK_RISK_EFFECTS: + return ToolAuthorization( + AuthorizationOutcome.REQUIRE_APPROVAL, + "Ask mode requires an exact user approval for this action.", + capabilities, + ) + + if capabilities.effects & _SANDBOX_ALWAYS_APPROVE_EFFECTS: + return ToolAuthorization( + AuthorizationOutcome.REQUIRE_APPROVAL, + "This action can affect an external system and requires an exact user approval.", + capabilities, + ) + + if ( + security_context.external_untrusted_context_seen + and capabilities.effects & POST_EXTERNAL_BLOCKED_EFFECTS + ): + return ToolAuthorization( + AuthorizationOutcome.REQUIRE_APPROVAL, + "External untrusted context influenced this run; this exact action requires user approval.", + capabilities, + ) + + return ToolAuthorization( + AuthorizationOutcome.ALLOW_SANDBOXED, + capabilities=capabilities, + ) diff --git a/src/agent_tools/subprocess_tools.py b/src/agent_tools/subprocess_tools.py index 9e7731704..89d973281 100644 --- a/src/agent_tools/subprocess_tools.py +++ b/src/agent_tools/subprocess_tools.py @@ -3,6 +3,7 @@ import hashlib import os import re import shutil +import sys import time import collections from typing import Optional, Callable, Awaitable, Tuple, Dict @@ -12,6 +13,7 @@ from src.execution_sandbox import ( sandbox_command, sandbox_python_executable, ) +from src.agent_run_policy import ExecutionProfile DEFAULT_BASH_TIMEOUT = 60 * 60 # 1 hour DEFAULT_PYTHON_TIMEOUT = 60 * 60 @@ -21,12 +23,21 @@ PROGRESS_TAIL_LINES = 12 TMUX_CAPTURE_LINES = 2000 -def _tmux_session_name(session_id: Optional[str], workspace: str = "") -> str: +def _tmux_session_name( + session_id: Optional[str], + workspace: str = "", + execution_profile: str = ExecutionProfile.WORKSPACE_SANDBOX.value, +) -> str: raw = re.sub(r"[^A-Za-z0-9_.-]+", "-", str(session_id or "default")).strip("-") workspace_key = hashlib.sha256( os.path.realpath(workspace or ".").encode("utf-8", errors="replace") ).hexdigest()[:10] - return f"ody-agent-sbx-v1-{raw[:60] or 'default'}-{workspace_key}" + profile_tag = ( + "host" + if execution_profile == ExecutionProfile.HOST_FULL_ACCESS.value + else "sbx" + ) + return f"ody-agent-{profile_tag}-v1-{raw[:60] or 'default'}-{workspace_key}" async def _run_exec(*args: str, timeout: float = 10) -> Tuple[str, str, int]: @@ -121,12 +132,16 @@ async def _run_tmux_bash( cwd: str, timeout: float, progress_cb: Optional[Callable[[Dict], Awaitable[None]]] = None, + execution_profile: str = ExecutionProfile.WORKSPACE_SANDBOX.value, ) -> Tuple[str, str, Optional[int], bool]: - name = _tmux_session_name(session_id, cwd) - shell_argv = sandbox_command( - ["/bin/bash", "--noprofile", "--norc"], - workspace=cwd, - ) + name = _tmux_session_name(session_id, cwd, execution_profile) + if execution_profile == ExecutionProfile.HOST_FULL_ACCESS.value: + shell_argv = ["/bin/bash", "--noprofile", "--norc"] + else: + shell_argv = sandbox_command( + ["/bin/bash", "--noprofile", "--norc"], + workspace=cwd, + ) await _ensure_tmux_session(name, cwd, shell_argv) stamp = f"{int(time.time() * 1000)}-{abs(hash(content)) % 1000000}" @@ -288,6 +303,10 @@ class BashTool: content = str(content.get("command") or content.get("cmd") or content.get("code") or "") progress_cb = ctx.get("progress_cb") session_id = ctx.get("session_id") + execution_profile = str( + ctx.get("execution_profile") + or ExecutionProfile.WORKSPACE_SANDBOX.value + ) workspace = agent_cwd() if session_id and shutil.which("tmux"): stdout, stderr, rc, timed_out = await _run_tmux_bash( @@ -296,6 +315,7 @@ class BashTool: cwd=workspace, timeout=DEFAULT_BASH_TIMEOUT, progress_cb=progress_cb, + execution_profile=execution_profile, ) if timed_out: return { @@ -303,7 +323,9 @@ class BashTool: "exit_code": 124, "stdout": _truncate(stdout, MAX_OUTPUT_CHARS), "stderr": _truncate(stderr, MAX_OUTPUT_CHARS), - "tmux_session": _tmux_session_name(str(session_id), workspace), + "tmux_session": _tmux_session_name( + str(session_id), workspace, execution_profile + ), } output = stdout.rstrip() err = stderr.rstrip() @@ -312,18 +334,25 @@ class BashTool: return { "output": _truncate(output, MAX_OUTPUT_CHARS) or "(no output)", "exit_code": rc or 0, - "tmux_session": _tmux_session_name(str(session_id), workspace), + "tmux_session": _tmux_session_name( + str(session_id), workspace, execution_profile + ), } - argv = sandbox_command( - ["/bin/bash", "--noprofile", "--norc", "-c", content], - workspace=workspace, - ) + if execution_profile == ExecutionProfile.HOST_FULL_ACCESS.value: + argv = ["/bin/bash", "--noprofile", "--norc", "-c", content] + process_env = None + else: + argv = sandbox_command( + ["/bin/bash", "--noprofile", "--norc", "-c", content], + workspace=workspace, + ) + process_env = environment_for_sandbox_launcher() proc = await asyncio.create_subprocess_exec( *argv, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, - env=environment_for_sandbox_launcher(), + env=process_env, cwd=workspace, ) stdout, stderr, rc, timed_out = await _run_subprocess_streaming( @@ -344,16 +373,25 @@ class PythonTool: async def execute(self, content: str, ctx: dict) -> dict: from src.tool_execution import agent_cwd, _truncate progress_cb = ctx.get("progress_cb") - workspace = agent_cwd() - argv = sandbox_command( - [sandbox_python_executable(), "-I", "-c", content], - workspace=workspace, + execution_profile = str( + ctx.get("execution_profile") + or ExecutionProfile.WORKSPACE_SANDBOX.value ) + workspace = agent_cwd() + if execution_profile == ExecutionProfile.HOST_FULL_ACCESS.value: + argv = [sys.executable, "-I", "-c", content] + process_env = None + else: + argv = sandbox_command( + [sandbox_python_executable(), "-I", "-c", content], + workspace=workspace, + ) + process_env = environment_for_sandbox_launcher() proc = await asyncio.create_subprocess_exec( *argv, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, - env=environment_for_sandbox_launcher(), + env=process_env, cwd=workspace, ) stdout, stderr, rc, timed_out = await _run_subprocess_streaming( diff --git a/src/bg_jobs.py b/src/bg_jobs.py index 349219a90..192fe6669 100644 --- a/src/bg_jobs.py +++ b/src/bg_jobs.py @@ -1,4 +1,4 @@ -"""Sandboxed background job execution for the agent's `bash` tool. +"""Background job execution for the agent's `bash` tool. Long commands (installs, ffmpeg, model downloads) should NOT block the chat stream — a multi-minute held SSE connection is fragile (model-stops-early, @@ -14,15 +14,17 @@ Design goals: * Bounded: a hard max-runtime marks a runaway job failed and STILL triggers a follow-up ("timed out"), so you always hear back. -This module only owns launch + state. Model commands execute inside the same -Linux bubblewrap profile as foreground Bash; a tiny isolated Python wrapper -outside the sandbox only records output and the exit code. The monitor / agent -re-invocation lives in the caller (so this stays import-light and unit-testable). +This module only owns launch + state. The default profile uses the same Linux +bubblewrap sandbox as foreground Bash. An explicitly selected full-access run +uses the owning user's host environment and permissions. A tiny isolated Python +wrapper records output and the exit code. The monitor / agent re-invocation +lives in the caller (so this stays import-light and unit-testable). """ from __future__ import annotations import json +import os import subprocess import sys import time @@ -42,6 +44,7 @@ from src.execution_sandbox import ( environment_for_sandbox_launcher, sandbox_command, ) +from src.agent_run_policy import ExecutionProfile _JOBS_DIR = Path(BG_JOBS_DIR) _STORE = Path(BG_JOBS_FILE) @@ -56,7 +59,7 @@ _MAX_OUTPUT_CHARS = 16000 # without bound. The agent has already consumed the result by then. _RETENTION_S = 3600 # 1 hour after follow-up -_DETACHED_SANDBOX_WRAPPER = """ +_DETACHED_PROCESS_WRAPPER = """ import json import subprocess import sys @@ -65,6 +68,8 @@ from pathlib import Path argv = json.loads(sys.argv[1]) log_path = Path(sys.argv[2]) exit_path = Path(sys.argv[3]) +inherit_environment = sys.argv[4] == "1" +child_cwd = sys.argv[5] or None code = 1 try: with log_path.open("wb") as output: @@ -73,7 +78,8 @@ try: stdin=subprocess.DEVNULL, stdout=output, stderr=subprocess.STDOUT, - env={}, + env=None if inherit_environment else {}, + cwd=child_cwd, check=False, ) code = int(completed.returncode) @@ -110,8 +116,13 @@ def _pid_alive(pid: Optional[int]) -> bool: return pid_alive(pid) -def launch(command: str, session_id: str, cwd: Optional[str] = None, - max_runtime_s: int = DEFAULT_MAX_RUNTIME_S) -> Dict[str, Any]: +def launch( + command: str, + session_id: str, + cwd: Optional[str] = None, + max_runtime_s: int = DEFAULT_MAX_RUNTIME_S, + execution_profile: str = ExecutionProfile.WORKSPACE_SANDBOX.value, +) -> Dict[str, Any]: """Launch `command` detached. Returns the job record (status='running'). Output + the final exit code are written to files so status survives a @@ -125,19 +136,31 @@ def launch(command: str, session_id: str, cwd: Optional[str] = None, cmd_path = _JOBS_DIR / f"{job_id}.cmd.sh" cmd_path.write_text(command + "\n", encoding="utf-8") - sandbox_argv = sandbox_command( - ["/bin/bash", "--noprofile", "--norc", "/run/odysseus/command.sh"], - workspace=cwd or "", - readonly_files={str(cmd_path): "/run/odysseus/command.sh"}, - ) + if execution_profile == ExecutionProfile.HOST_FULL_ACCESS.value: + child_argv = [ + "/bin/bash", + "--noprofile", + "--norc", + str(cmd_path), + ] + child_env = os.environ.copy() + else: + child_argv = sandbox_command( + ["/bin/bash", "--noprofile", "--norc", "/run/odysseus/command.sh"], + workspace=cwd or "", + readonly_files={str(cmd_path): "/run/odysseus/command.sh"}, + ) + child_env = environment_for_sandbox_launcher() argv = [ sys.executable, "-I", "-c", - _DETACHED_SANDBOX_WRAPPER, - json.dumps(sandbox_argv), + _DETACHED_PROCESS_WRAPPER, + json.dumps(child_argv), str(log_path), str(exit_path), + "1" if execution_profile == ExecutionProfile.HOST_FULL_ACCESS.value else "0", + str(cwd or "") if execution_profile == ExecutionProfile.HOST_FULL_ACCESS.value else "", ] proc = subprocess.Popen( @@ -146,7 +169,7 @@ def launch(command: str, session_id: str, cwd: Optional[str] = None, stderr=subprocess.DEVNULL, stdin=subprocess.DEVNULL, cwd=None, - env=environment_for_sandbox_launcher(), + env=child_env, **detached_popen_kwargs(), # detach from the request lifecycle (setsid / DETACHED_PROCESS) ) @@ -160,6 +183,7 @@ def launch(command: str, session_id: str, cwd: Optional[str] = None, "ended_at": None, "exit_code": None, "max_runtime_s": max_runtime_s, + "execution_profile": execution_profile, "followed_up": False, # has the agent been re-invoked with the result? "log_path": str(log_path), "exit_path": str(exit_path), diff --git a/src/bg_monitor.py b/src/bg_monitor.py index 8cf8ccc15..049000230 100644 --- a/src/bg_monitor.py +++ b/src/bg_monitor.py @@ -40,6 +40,7 @@ async def _drain_agent(sess, messages): session_id=sess.id, max_rounds=_FOLLOWUP_MAX_ROUNDS, owner=getattr(sess, "owner", None), + security_mode=getattr(sess, "security_mode", None) or "sandbox", ): if not chunk.startswith("data: "): continue diff --git a/src/request_models.py b/src/request_models.py index f7755b1d4..bdd4cc17b 100644 --- a/src/request_models.py +++ b/src/request_models.py @@ -125,6 +125,10 @@ class SessionResponse(BaseModel): model: str = Field(..., description="Model being used") rag: bool = Field(default=False, description="RAG enabled") archived: bool = Field(default=False, description="Whether session is archived") + security_mode: str = Field( + default="sandbox", + description="Agent run authority mode: ask, sandbox, or full_access", + ) class MemoryResponse(BaseModel): diff --git a/src/teacher_escalation.py b/src/teacher_escalation.py index 49134991c..19b889cb2 100644 --- a/src/teacher_escalation.py +++ b/src/teacher_escalation.py @@ -562,6 +562,10 @@ async def run_teacher_inline( student_tool_events: List[Dict[str, Any]], student_reply: str, owner: Optional[str] = None, + session_id: Optional[str] = None, + workspace: Optional[str] = None, + security_mode: str = "sandbox", + external_untrusted_context_seen: bool = False, ): """Async generator. Yields SSE event strings. @@ -667,6 +671,10 @@ async def run_teacher_inline( messages=teacher_messages, headers=teacher_headers, owner=owner, + session_id=session_id, + workspace=workspace, + security_mode=security_mode, + external_untrusted_context_seen=external_untrusted_context_seen, _is_teacher_run=True, ): # Swallow teacher's own [DONE] — outer loop emits the real one diff --git a/src/tool_approvals.py b/src/tool_approvals.py new file mode 100644 index 000000000..fc66d76f5 --- /dev/null +++ b/src/tool_approvals.py @@ -0,0 +1,297 @@ +"""Opaque, exact, one-use approvals for model-requested tool actions. + +Approval authority lives only in this process. The browser receives an opaque +identifier plus a non-authoritative display copy; the stored record retains the +exact tool input and binding fields. Resuming a turn executes that stored +action, never a model re-emission or a natural-language "yes". +""" + +from __future__ import annotations + +import hashlib +import json +import os +import secrets +import threading +import time +from dataclasses import dataclass, field +from typing import Any + +from src.agent_run_policy import AgentRunMode, parse_agent_run_mode +from src.tool_capabilities import ToolCapabilities, capabilities_for_tool + + +DEFAULT_APPROVAL_TTL_SECONDS = 10 * 60 + + +def _normalized_owner(owner: Any) -> str: + return str(owner or "").strip().casefold() + + +def _normalized_workspace(workspace: Any) -> str: + if not isinstance(workspace, str) or not workspace.strip(): + return "" + return os.path.realpath(os.path.expanduser(workspace)) + + +def _canonical_digest(payload: dict[str, Any]) -> str: + encoded = json.dumps( + payload, + sort_keys=True, + separators=(",", ":"), + ensure_ascii=False, + ).encode("utf-8") + return hashlib.sha256(encoded).hexdigest() + + +def _binding_payload( + *, + owner: Any, + session_id: Any, + origin_run_id: Any, + tool_name: Any, + content: Any, + workspace: Any, + security_mode: Any, + external_untrusted_context_seen: bool, + effects: tuple[str, ...], +) -> dict[str, Any]: + return { + "owner": _normalized_owner(owner), + "session_id": str(session_id or ""), + "origin_run_id": str(origin_run_id or ""), + "tool_name": str(tool_name or ""), + "content": str(content or ""), + "workspace": _normalized_workspace(workspace), + "security_mode": parse_agent_run_mode(security_mode).value, + "external_untrusted_context_seen": bool(external_untrusted_context_seen), + "effects": list(effects), + } + + +@dataclass(frozen=True) +class PendingToolApproval: + approval_id: str + owner: str + session_id: str + origin_run_id: str + tool_name: str + content: str + workspace: str + security_mode: AgentRunMode + external_untrusted_context_seen: bool + effects: tuple[str, ...] + digest: str + created_at: float + expires_at: float + + def public_payload(self, *, reason: str | None = None) -> dict[str, Any]: + return { + "kind": "tool_approval", + "approval_id": self.approval_id, + "question": "Allow this exact action once?", + "description": reason or "The action is outside this thread's automatic authority.", + "options": [ + { + "label": "Allow once", + "value": "approve", + "description": "Execute only the sealed action shown here.", + }, + { + "label": "Deny", + "value": "deny", + "description": "Do not execute it.", + }, + ], + "action": { + "tool": self.tool_name, + # Display data, not authority. Show the complete sealed input + # so the user never approves hidden trailing lines. + "content": self.content, + "digest": self.digest[:16], + "effects": list(self.effects), + "workspace": self.workspace or None, + }, + } + + +@dataclass +class ExactToolApproval: + """A consumed approval grant; a dispatcher can claim it exactly once.""" + + pending: PendingToolApproval + _claimed: bool = field(default=False, init=False, repr=False) + _lock: threading.Lock = field(default_factory=threading.Lock, init=False, repr=False) + + def _matches_unlocked( + self, + *, + owner: Any, + session_id: Any, + tool_name: Any, + content: Any, + workspace: Any, + security_mode: Any, + ) -> bool: + if self._claimed: + return False + current_effects = tuple( + sorted( + effect.value + for effect in capabilities_for_tool(tool_name).effects + ) + ) + if current_effects != self.pending.effects: + return False + expected = _binding_payload( + owner=owner, + session_id=session_id, + origin_run_id=self.pending.origin_run_id, + tool_name=tool_name, + content=content, + workspace=workspace, + security_mode=security_mode, + external_untrusted_context_seen=self.pending.external_untrusted_context_seen, + effects=self.pending.effects, + ) + return _canonical_digest(expected) == self.pending.digest + + def matches( + self, + *, + owner: Any, + session_id: Any, + tool_name: Any, + content: Any, + workspace: Any, + security_mode: Any, + ) -> bool: + """Validate a binding for safe presentation without consuming it.""" + with self._lock: + return self._matches_unlocked( + owner=owner, + session_id=session_id, + tool_name=tool_name, + content=content, + workspace=workspace, + security_mode=security_mode, + ) + + def claim( + self, + *, + owner: Any, + session_id: Any, + tool_name: Any, + content: Any, + workspace: Any, + security_mode: Any, + ) -> bool: + with self._lock: + if not self._matches_unlocked( + owner=owner, + session_id=session_id, + tool_name=tool_name, + content=content, + workspace=workspace, + security_mode=security_mode, + ): + return False + self._claimed = True + return True + + +class ToolApprovalStore: + """Thread-safe pending approval registry with destructive consumption.""" + + def __init__(self, *, ttl_seconds: int = DEFAULT_APPROVAL_TTL_SECONDS): + self._ttl_seconds = max(1, int(ttl_seconds)) + self._pending: dict[str, PendingToolApproval] = {} + self._lock = threading.Lock() + + def _purge_expired_locked(self, now: float) -> None: + expired = [ + approval_id + for approval_id, pending in self._pending.items() + if pending.expires_at <= now + ] + for approval_id in expired: + self._pending.pop(approval_id, None) + + def create( + self, + *, + owner: Any, + session_id: Any, + origin_run_id: Any, + tool_name: Any, + content: Any, + workspace: Any, + security_mode: Any, + external_untrusted_context_seen: bool, + capabilities: ToolCapabilities, + ) -> PendingToolApproval: + now = time.time() + effects = tuple(sorted(effect.value for effect in capabilities.effects)) + payload = _binding_payload( + owner=owner, + session_id=session_id, + origin_run_id=origin_run_id, + tool_name=tool_name, + content=content, + workspace=workspace, + security_mode=security_mode, + external_untrusted_context_seen=external_untrusted_context_seen, + effects=effects, + ) + pending = PendingToolApproval( + approval_id=secrets.token_urlsafe(32), + owner=payload["owner"], + session_id=payload["session_id"], + origin_run_id=payload["origin_run_id"], + tool_name=payload["tool_name"], + content=payload["content"], + workspace=payload["workspace"], + security_mode=parse_agent_run_mode(payload["security_mode"]), + external_untrusted_context_seen=payload["external_untrusted_context_seen"], + effects=effects, + digest=_canonical_digest(payload), + created_at=now, + expires_at=now + self._ttl_seconds, + ) + with self._lock: + self._purge_expired_locked(now) + self._pending[pending.approval_id] = pending + return pending + + def consume( + self, + approval_id: Any, + *, + decision: Any, + owner: Any, + session_id: Any, + ) -> ExactToolApproval | None: + now = time.time() + with self._lock: + self._purge_expired_locked(now) + pending = self._pending.pop(str(approval_id or ""), None) + if pending is None: + return None + if ( + pending.owner != _normalized_owner(owner) + or pending.session_id != str(session_id or "") + ): + return None + if str(decision or "").strip().lower() != "approve": + return None + return ExactToolApproval(pending) + + def peek(self, approval_id: Any) -> PendingToolApproval | None: + now = time.time() + with self._lock: + self._purge_expired_locked(now) + return self._pending.get(str(approval_id or "")) + + +tool_approval_store = ToolApprovalStore() diff --git a/src/tool_capabilities.py b/src/tool_capabilities.py index e5d8a44f6..71aa1aa87 100644 --- a/src/tool_capabilities.py +++ b/src/tool_capabilities.py @@ -11,6 +11,7 @@ from dataclasses import dataclass, field from enum import Enum from types import MappingProxyType from typing import Any, Iterable, Mapping +import uuid from src.tool_security import BUILTIN_EMAIL_TOOLS @@ -302,6 +303,7 @@ def messages_contain_external_untrusted_context(messages: Iterable[dict]) -> boo class ToolRunSecurityContext: """Server-owned integrity state for one agent run.""" + run_id: str = field(default_factory=lambda: uuid.uuid4().hex) external_untrusted_context_seen: bool = False external_sources: list[str] = field(default_factory=list) diff --git a/src/tool_execution.py b/src/tool_execution.py index e447c26d3..ad1377aa7 100644 --- a/src/tool_execution.py +++ b/src/tool_execution.py @@ -28,6 +28,12 @@ from src.tool_security import ( owner_is_admin_or_single_user, ) from src.tool_capabilities import ToolRunSecurityContext, blocked_tool_result +from src.agent_run_policy import ( + AgentRunPolicy, + AuthorizationOutcome, + ExecutionProfile, +) +from src.tool_approvals import ExactToolApproval from src.tool_policy import ToolPolicy from src.constants import ( AGENT_WORKSPACE_DIR, @@ -454,11 +460,17 @@ async def _call_mcp_tool( tool: str, content: str, progress_cb: Optional[Callable[[Dict], Awaitable[None]]] = None, + execution_profile: ExecutionProfile = ExecutionProfile.WORKSPACE_SANDBOX, ) -> Dict: """Route a legacy tool call through the MCP manager, with direct fallbacks.""" mcp = get_mcp_manager() if not mcp: - return await _direct_fallback(tool, content, progress_cb=progress_cb) or {"error": f"MCP manager not available for tool '{tool}'", "exit_code": 1} + return await _direct_fallback( + tool, + content, + progress_cb=progress_cb, + execution_profile=execution_profile, + ) or {"error": f"MCP manager not available for tool '{tool}'", "exit_code": 1} server_id, tool_name = _MCP_TOOL_MAP[tool] qualified = f"mcp__{server_id}__{tool_name}" @@ -467,7 +479,12 @@ async def _call_mcp_tool( # If MCP server not connected, try direct fallback if isinstance(result, dict) and result.get("exit_code") == 1 and "not connected" in result.get("error", ""): - fallback = await _direct_fallback(tool, content, progress_cb=progress_cb) + fallback = await _direct_fallback( + tool, + content, + progress_cb=progress_cb, + execution_profile=execution_profile, + ) if fallback: return fallback @@ -527,12 +544,14 @@ async def _direct_fallback( progress_cb: Optional[Callable[[Dict], Awaitable[None]]] = None, session_id: Optional[str] = None, owner: Optional[str] = None, + execution_profile: ExecutionProfile = ExecutionProfile.WORKSPACE_SANDBOX, ) -> Optional[Dict]: try: ctx = { "progress_cb": progress_cb, "session_id": session_id, "owner": owner, + "execution_profile": execution_profile.value, } from src.agent_tools import TOOL_HANDLERS @@ -572,6 +591,8 @@ async def execute_tool_block( workspace: Optional[str] = None, tool_policy: Optional[Any] = None, security_context: Optional[ToolRunSecurityContext] = None, + run_policy: Optional[AgentRunPolicy] = None, + exact_approval: Optional[ExactToolApproval] = None, ) -> Tuple[str, Dict]: """Execute a single tool block. Returns (description, result_dict). @@ -579,7 +600,64 @@ async def execute_tool_block( cwd confine to it) for the duration of this call, then delegate. Reset on the way out so the binding never leaks to the next tool call. """ - if security_context is not None: + execution_profile = ExecutionProfile.WORKSPACE_SANDBOX + approval_claimed = False + if exact_approval is not None and run_policy is not None: + approval_claimed = exact_approval.claim( + owner=owner, + session_id=session_id, + tool_name=getattr(block, "tool_type", None), + content=getattr(block, "content", None), + workspace=workspace, + security_mode=run_policy.mode, + ) + if not approval_claimed: + return ( + f"{getattr(block, 'tool_type', None)}: BLOCKED", + { + "error": "The exact-action approval did not match this tool request.", + "exit_code": 1, + "blocked": True, + "policy": "exact_tool_approval", + }, + ) + + if run_policy is not None and security_context is not None and not approval_claimed: + if ( + run_policy.execution_profile is ExecutionProfile.HOST_FULL_ACCESS + and not owner_is_admin_or_single_user(owner) + ): + return ( + f"{getattr(block, 'tool_type', None)}: BLOCKED", + { + "error": "Host full-access execution requires an admin user.", + "exit_code": 1, + "blocked": True, + "policy": "agent_run_policy", + }, + ) + authorization = run_policy.authorize( + getattr(block, "tool_type", None), + security_context, + ) + if authorization.outcome is AuthorizationOutcome.REQUIRE_APPROVAL: + return ( + f"{getattr(block, 'tool_type', None)}: APPROVAL REQUIRED", + { + "error": authorization.reason or "Exact user approval required.", + "exit_code": 1, + "blocked": True, + "approval_required": True, + "policy": "agent_run_policy", + }, + ) + if authorization.outcome is AuthorizationOutcome.DENY: + return blocked_tool_result( + getattr(block, "tool_type", None), + authorization.reason or "Tool denied by run policy.", + ) + execution_profile = run_policy.execution_profile + elif security_context is not None and not approval_claimed: decision = security_context.decision_for(getattr(block, "tool_type", None)) if not decision.allowed: logger.warning( @@ -600,6 +678,7 @@ async def execute_tool_block( owner=owner, progress_cb=progress_cb, tool_policy=tool_policy, + execution_profile=execution_profile, ) if security_context is not None: security_context.observe_tool_result( @@ -618,6 +697,7 @@ async def _execute_tool_block_impl( owner: Optional[str] = None, progress_cb: Optional[Callable[[Dict], Awaitable[None]]] = None, tool_policy: Optional[Any] = None, + execution_profile: ExecutionProfile = ExecutionProfile.WORKSPACE_SANDBOX, ) -> Tuple[str, Dict]: """Execute a single tool block. Returns (description, result_dict). @@ -739,6 +819,7 @@ async def _execute_tool_block_impl( _bg_cmd, session_id=session_id, cwd=agent_cwd(), + execution_profile=execution_profile.value, ) except Exception as exc: return ( @@ -773,22 +854,44 @@ async def _execute_tool_block_impl( if tool in _MCP_TOOL_MAP: first_line = content.split(chr(10))[0][:80] desc = f"{tool}: {first_line}" - result = await _call_mcp_tool(tool, content, progress_cb=progress_cb) + result = await _call_mcp_tool( + tool, + content, + progress_cb=progress_cb, + execution_profile=execution_profile, + ) elif tool in ("grep", "glob", "ls", "get_workspace"): # Code-navigation tools — no MCP server; run the direct implementation. first_line = content.split(chr(10))[0][:80] desc = f"{tool}: {first_line}" - result = await _direct_fallback(tool, content, progress_cb=progress_cb) \ + result = await _direct_fallback( + tool, + content, + progress_cb=progress_cb, + execution_profile=execution_profile, + ) \ or {"error": f"{tool}: execution failed", "exit_code": 1} elif tool in ("apply_patch", "todowrite"): first_line = content.split(chr(10))[0][:80] desc = f"{tool}: {first_line}" if first_line else tool - result = await _direct_fallback(tool, content, session_id=session_id, owner=owner) \ + result = await _direct_fallback( + tool, + content, + session_id=session_id, + owner=owner, + execution_profile=execution_profile, + ) \ or {"error": f"{tool}: execution failed", "exit_code": 1} elif tool == "manage_bg_jobs": # Inspect/kill detached `bash` jobs; needs session_id to scope to chat. desc = f"manage_bg_jobs: {content.split(chr(10))[0][:80]}" - result = await _direct_fallback(tool, content, session_id=session_id, owner=owner) \ + result = await _direct_fallback( + tool, + content, + session_id=session_id, + owner=owner, + execution_profile=execution_profile, + ) \ or {"error": "manage_bg_jobs: execution failed", "exit_code": 1} elif tool in ("create_document", "update_document", "edit_document", "suggest_document", "manage_documents"): @@ -977,7 +1080,12 @@ async def _execute_tool_block_impl( elif tool in dynamic_handlers: first_line = content.split(chr(10))[0][:80] desc = f"registry: {tool} {first_line}".strip() - res = await _direct_fallback(tool, content, progress_cb=progress_cb) + res = await _direct_fallback( + tool, + content, + progress_cb=progress_cb, + execution_profile=execution_profile, + ) if isinstance(res, tuple): desc, result = res diff --git a/static/app.js b/static/app.js index 97f0ae77e..4e413a2ad 100644 --- a/static/app.js +++ b/static/app.js @@ -10,14 +10,14 @@ import modelsModule from './js/models.js?v=20260715startupcalm2'; import ragModule from './js/rag.js'; import presetsModule from './js/presets.js'; import searchModule from './js/search.js'; -import chatModule from './js/chat.js?v=20260722ctxheader4'; +import chatModule from './js/chat.js?v=20260725agentsecurity1'; import compareModule from './js/compare/index.js?v=20260723compareicon2'; import documentModule from './js/document.js?v=20260722emailfastindex1'; import searchChatModule from './js/search-chat.js'; import { makeWindowDraggable } from './js/windowDrag.js'; import markdownModule from './js/markdown.js'; -import chatRenderer from './js/chatRenderer.js?v=20260722emailfastindex1'; -import sessionModule from './js/sessions.js?v=20260722ctxheader4'; +import chatRenderer from './js/chatRenderer.js?v=20260725agentsecurity1'; +import sessionModule from './js/sessions.js?v=20260725agentsecurity1'; import memoryModule from './js/memory.js?v=20260722memoryloading1'; import voiceRecorderModule from './js/voiceRecorder.js'; import censorModule from './js/censor.js'; @@ -841,6 +841,9 @@ function initializeEventListeners() { // Close document panel if open if (documentModule && documentModule.closePanel) documentModule.closePanel(); if (researchPanelModule && researchPanelModule.isOpen()) researchPanelModule.closePanel(); + if (typeof window.__odysseusSetSecurityMode === 'function') { + window.__odysseusSetSecurityMode('sandbox'); + } // Reset research overflow dot (but don't touch research state — caller manages that) const _overflowRes = el('overflow-research-btn'); if (_overflowRes) _overflowRes.classList.remove('active'); @@ -1835,6 +1838,53 @@ function initializeEventListeners() { setMode(currentMode); })(); + // ── Thread-level agent authority mode ── + (function initAgentSecurityMode() { + const select = el('agent-security-mode'); + if (!select) return; + const allowed = new Set(['ask', 'sandbox', 'full_access']); + + function setSecurityMode(mode) { + let next = allowed.has(mode) ? mode : 'sandbox'; + if (!select.querySelector(`option[value="${next}"]`)) next = 'sandbox'; + const state = loadToggleState(); + state.security_mode = next; + saveToggleState(state); + select.value = next; + select.title = next === 'full_access' + ? 'Full access: commands run with your normal OS permissions.' + : next === 'ask' + ? 'Ask: confirm each risky exact action.' + : 'Sandbox: workspace-only process isolation; trust-boundary actions ask.'; + return true; + } + + select.addEventListener('change', async () => { + const state = loadToggleState(); + const previous = allowed.has(state.security_mode) + ? state.security_mode + : 'sandbox'; + const selected = select.value; + if (selected === 'full_access') { + select.value = previous; + const confirmed = await uiModule.styledConfirm( + 'Full access lets model-requested commands run directly with your normal OS permissions.', + { + confirmText: 'Enable full access', + cancelText: 'Keep current mode', + danger: true, + }, + ); + if (!confirmed) return; + } + setSecurityMode(selected); + }); + window.__odysseusSetSecurityMode = (mode) => { + setSecurityMode(mode); + }; + setSecurityMode(loadToggleState().security_mode || 'sandbox'); + })(); + (function initPlanToggle() { const btn = el('plan-toggle-btn'); const state = loadToggleState(); diff --git a/static/index.html b/static/index.html index 8257660fe..191d260e1 100644 --- a/static/index.html +++ b/static/index.html @@ -249,10 +249,10 @@ })(); - - + + - + @@ -1185,6 +1185,17 @@
+
@@ -2504,7 +2515,7 @@ - + @@ -2519,10 +2530,10 @@ - + - + @@ -2530,8 +2541,8 @@ - - + + diff --git a/static/js/chat.js b/static/js/chat.js index ea2d8c1bb..e6289d990 100644 --- a/static/js/chat.js +++ b/static/js/chat.js @@ -8,7 +8,7 @@ import Storage from './storage.js'; import uiModule from './ui.js'; import sessionModule from './sessions.js'; -import chatRenderer from './chatRenderer.js?v=20260722emailfastindex1'; +import chatRenderer from './chatRenderer.js?v=20260725agentsecurity1'; import chatStream from './chatStream.js'; import { addAITTSButton } from './tts-ai.js'; import markdownModule from './markdown.js'; @@ -46,6 +46,27 @@ import { wireArrowUpRecall, getUserMessagesFromChatHistory } from './composerArr let _contextHeaderSeq = 0; let _contextHeaderData = null; let _contextHeaderBound = false; + let _pendingToolApproval = null; + + document.addEventListener('odysseus:tool-approval', (event) => { + const detail = event && event.detail ? event.detail : {}; + if ( + !detail.approval_id + || !['approve', 'deny'].includes(String(detail.decision || '').toLowerCase()) + ) return; + _pendingToolApproval = { + approval_id: String(detail.approval_id), + decision: String(detail.decision).toLowerCase(), + }; + const input = document.getElementById('message'); + if (input) { + input.value = detail.label || ( + _pendingToolApproval.decision === 'approve' ? 'Allow once' : 'Deny' + ); + } + const sendButton = document.querySelector('.send-btn'); + if (sendButton) sendButton.click(); + }); function _fmtContextNumber(n) { const v = Number(n || 0); @@ -1663,6 +1684,15 @@ import { wireArrowUpRecall, getUserMessagesFromChatHistory } from './composerArr // on; explicit web/current-info requests are handled by the backend // intent gate. const toggleState = Storage.loadToggleState(); + const securityMode = ['ask', 'sandbox', 'full_access'].includes(toggleState.security_mode) + ? toggleState.security_mode + : 'sandbox'; + if (sessionModule && typeof sessionModule.getSessions === 'function') { + const currentMeta = sessionModule.getSessions().find( + (item) => String(item.id) === String(streamSessionId) + ); + if (currentMeta) currentMeta.security_mode = securityMode; + } const isPlanMode = !!toggleState.plan_mode && !(el('research-toggle') && el('research-toggle').checked); let isAgentMode = (toggleState.mode || 'chat') === 'agent'; const isIncognito = isIncognitoForSend; @@ -1679,6 +1709,12 @@ import { wireArrowUpRecall, getUserMessagesFromChatHistory } from './composerArr isAgentMode = true; } fd.append('mode', isAgentMode ? 'agent' : 'chat'); + fd.append('security_mode', securityMode); + if (_pendingToolApproval) { + fd.append('tool_approval_id', _pendingToolApproval.approval_id); + fd.append('tool_approval_decision', _pendingToolApproval.decision); + _pendingToolApproval = null; + } fd.append('plan_mode', isPlanMode ? 'true' : 'false'); if (!isPlanMode && _pendingApprovedPlan) { fd.append('approved_plan', _pendingApprovedPlan.slice(0, 8192)); diff --git a/static/js/chatRenderer.js b/static/js/chatRenderer.js index 10709679d..c44d85b52 100644 --- a/static/js/chatRenderer.js +++ b/static/js/chatRenderer.js @@ -2177,6 +2177,7 @@ export function renderAskUserCard(payload, options) { card.setAttribute('role', 'group'); card.tabIndex = -1; const multi = !!aq.multi; + const isToolApproval = aq.kind === 'tool_approval' && !!aq.approval_id; const emojiText = (value) => svgifyEmoji(uiModule.esc(String(value))); const head = document.createElement('div'); @@ -2201,6 +2202,22 @@ export function renderAskUserCard(payload, options) { card.appendChild(question); card.setAttribute('aria-labelledby', question.id); + if (isToolApproval && aq.action) { + const action = document.createElement('div'); + action.className = 'ask-user-option-desc'; + const effects = Array.isArray(aq.action.effects) + ? aq.action.effects.join(', ') + : ''; + action.textContent = [ + aq.action.tool || 'tool', + aq.action.content || '', + effects ? `Effects: ${effects}` : '', + aq.action.digest ? `Approval fingerprint: ${aq.action.digest}` : '', + ].filter(Boolean).join('\n'); + action.style.whiteSpace = 'pre-wrap'; + card.appendChild(action); + } + const list = document.createElement('div'); list.className = 'ask-user-options'; card.appendChild(list); @@ -2238,7 +2255,20 @@ export function renderAskUserCard(payload, options) { } if (!multi) { row.type = 'button'; - row.addEventListener('click', () => send(label)); + row.addEventListener('click', () => { + if (isToolApproval) { + card.remove(); + document.dispatchEvent(new CustomEvent('odysseus:tool-approval', { + detail: { + approval_id: aq.approval_id, + decision: String((opt && opt.value) || '').toLowerCase(), + label, + }, + })); + } else { + send(label); + } + }); } list.appendChild(row); }); @@ -2274,7 +2304,7 @@ export function renderAskUserCard(payload, options) { }); other.appendChild(otherInput); other.appendChild(otherSend); - card.appendChild(other); + if (!isToolApproval) card.appendChild(other); chatBox.appendChild(card); if (renderOptions.scroll !== false) { diff --git a/static/js/init.js b/static/js/init.js index 54239c52f..c092ceaa1 100644 --- a/static/js/init.js +++ b/static/js/init.js @@ -86,6 +86,16 @@ document.addEventListener('DOMContentLoaded', markComposerUserEdited, { once: tr if (_agent) _agent.style.display = 'none'; if (_chat) { _chat.classList.add('active'); _chat.click?.(); } } + if (data.is_admin === false) { + const securitySelect = document.getElementById('agent-security-mode'); + const fullAccess = securitySelect?.querySelector( + 'option[value="full_access"]' + ); + if (fullAccess) fullAccess.remove(); + if (securitySelect?.value === 'full_access') { + window.__odysseusSetSecurityMode?.('sandbox'); + } + } } catch (_) { /* DOM not ready or unexpected shape — UI gates are non-fatal */ } } catch (_) { /* anonymous / loopback mode — nothing to do */ } })(); diff --git a/static/js/sessions.js b/static/js/sessions.js index cf59d478c..2bd979d1b 100644 --- a/static/js/sessions.js +++ b/static/js/sessions.js @@ -3,7 +3,7 @@ import Storage from './storage.js'; import uiModule, { autoResize, styledPrompt } from './ui.js'; -import chatRenderer from './chatRenderer.js?v=20260722ctxheader1'; +import chatRenderer from './chatRenderer.js?v=20260725agentsecurity1'; import { providerLogo } from './providers.js'; import { initModelPicker, updateModelPicker } from './modelPicker.js?v=20260722ctxheader1'; import themeModule from './theme.js'; @@ -1854,6 +1854,9 @@ export async function selectSession(id, { keepSidebar = false, showLoading = tru if (presetsModule && presetsModule.onSessionSwitch) presetsModule.onSessionSwitch(id); } catch (e) {} const meta = sessions.find(s => s.id === id); + if (meta && typeof window.__odysseusSetSecurityMode === 'function') { + window.__odysseusSetSecurityMode(meta.security_mode || 'sandbox'); + } // Detach any in-flight stream to background instead of aborting try { diff --git a/tests/test_agent_run_policy.py b/tests/test_agent_run_policy.py new file mode 100644 index 000000000..0d538fe7a --- /dev/null +++ b/tests/test_agent_run_policy.py @@ -0,0 +1,279 @@ +import time +from collections import namedtuple + +import pytest + +from src.agent_run_policy import ( + AgentRunMode, + AgentRunPolicy, + AuthorizationOutcome, + ExecutionProfile, + parse_agent_run_mode, +) +from src.tool_approvals import ToolApprovalStore +from src.tool_capabilities import ToolRunSecurityContext, capabilities_for_tool + + +def test_invalid_run_mode_fails_safe_to_sandbox(): + assert parse_agent_run_mode("made-up") is AgentRunMode.SANDBOX + assert AgentRunPolicy.for_mode(None).mode is AgentRunMode.SANDBOX + + +def test_ask_requires_exact_approval_for_code_but_not_public_read(): + policy = AgentRunPolicy.for_mode("ask") + context = ToolRunSecurityContext() + + assert policy.authorize("web_search", context).outcome is AuthorizationOutcome.ALLOW_SANDBOXED + assert policy.authorize("bash", context).outcome is AuthorizationOutcome.REQUIRE_APPROVAL + + +def test_sandbox_allows_code_before_external_context_then_requires_approval(): + policy = AgentRunPolicy.for_mode("sandbox") + context = ToolRunSecurityContext() + + assert policy.authorize("bash", context).outcome is AuthorizationOutcome.ALLOW_SANDBOXED + context.external_untrusted_context_seen = True + assert policy.authorize("bash", context).outcome is AuthorizationOutcome.REQUIRE_APPROVAL + + +def test_sandbox_requires_approval_for_external_side_effects(): + policy = AgentRunPolicy.for_mode("sandbox") + context = ToolRunSecurityContext() + + assert policy.authorize("send_email", context).outcome is AuthorizationOutcome.REQUIRE_APPROVAL + + +def test_unknown_tool_requires_approval_outside_full_access(): + context = ToolRunSecurityContext() + + assert AgentRunPolicy.for_mode("sandbox").authorize( + "mcp__unknown__surprise", context + ).outcome is AuthorizationOutcome.REQUIRE_APPROVAL + assert AgentRunPolicy.for_mode("full_access").authorize( + "mcp__unknown__surprise", context + ).outcome is AuthorizationOutcome.ALLOW_HOST + + +def test_full_access_selects_host_execution_profile(): + policy = AgentRunPolicy.for_mode("full_access") + + assert policy.execution_profile is ExecutionProfile.HOST_FULL_ACCESS + assert policy.authorize( + "bash", ToolRunSecurityContext(external_untrusted_context_seen=True) + ).outcome is AuthorizationOutcome.ALLOW_HOST + + +def _pending(store, **overrides): + values = { + "owner": "Alice", + "session_id": "session-1", + "origin_run_id": "run-1", + "tool_name": "bash", + "content": "printf exact", + "workspace": "/tmp/workspace", + "security_mode": "ask", + "external_untrusted_context_seen": True, + "capabilities": capabilities_for_tool("bash"), + } + values.update(overrides) + return store.create(**values) + + +def test_approval_is_bound_to_exact_action_and_claimed_once(): + store = ToolApprovalStore() + pending = _pending(store) + grant = store.consume( + pending.approval_id, + decision="approve", + owner="alice", + session_id="session-1", + ) + + assert grant is not None + assert not grant.claim( + owner="alice", + session_id="session-1", + tool_name="bash", + content="printf modified", + workspace="/tmp/workspace", + security_mode="ask", + ) + assert grant.claim( + owner="ALICE", + session_id="session-1", + tool_name="bash", + content="printf exact", + workspace="/tmp/workspace", + security_mode="ask", + ) + assert not grant.claim( + owner="alice", + session_id="session-1", + tool_name="bash", + content="printf exact", + workspace="/tmp/workspace", + security_mode="ask", + ) + + +def test_approval_wrong_owner_is_destroyed_without_grant(): + store = ToolApprovalStore() + pending = _pending(store) + + assert store.consume( + pending.approval_id, + decision="approve", + owner="mallory", + session_id="session-1", + ) is None + assert store.peek(pending.approval_id) is None + + +def test_deny_destructively_consumes_pending_action(): + store = ToolApprovalStore() + pending = _pending(store) + + assert store.consume( + pending.approval_id, + decision="deny", + owner="alice", + session_id="session-1", + ) is None + assert store.peek(pending.approval_id) is None + + +def test_expired_approval_cannot_be_consumed(monkeypatch): + store = ToolApprovalStore(ttl_seconds=1) + pending = _pending(store) + monkeypatch.setattr(time, "time", lambda: pending.expires_at + 1) + + assert store.consume( + pending.approval_id, + decision="approve", + owner="alice", + session_id="session-1", + ) is None + + +def test_public_approval_payload_shows_the_complete_exact_action(): + store = ToolApprovalStore() + pending = _pending(store, content="printf safe\nSECRET_SECOND_LINE") + + payload = pending.public_payload() + encoded = str(payload) + assert payload["kind"] == "tool_approval" + assert payload["action"]["content"] == "printf safe\nSECRET_SECOND_LINE" + assert "SECRET_SECOND_LINE" in encoded + + +@pytest.mark.asyncio +async def test_dispatcher_claims_exact_approval_immediately_before_execution( + monkeypatch, +): + import src.tool_execution as tool_execution + + ToolBlock = namedtuple("ToolBlock", ["tool_type", "content"]) + store = ToolApprovalStore() + pending = _pending(store) + grant = store.consume( + pending.approval_id, + decision="approve", + owner="alice", + session_id="session-1", + ) + calls = [] + + async def fake_implementation(block, **kwargs): + calls.append((block.tool_type, block.content, kwargs["execution_profile"])) + return "bash", {"output": "ok", "exit_code": 0} + + monkeypatch.setattr( + tool_execution, + "_execute_tool_block_impl", + fake_implementation, + ) + desc, result = await tool_execution.execute_tool_block( + ToolBlock("bash", "printf exact"), + session_id="session-1", + owner="alice", + workspace="/tmp/workspace", + security_context=ToolRunSecurityContext( + external_untrusted_context_seen=True + ), + run_policy=AgentRunPolicy.for_mode("ask"), + exact_approval=grant, + ) + + assert desc == "bash" + assert result["exit_code"] == 0 + assert calls == [ + ("bash", "printf exact", ExecutionProfile.WORKSPACE_SANDBOX) + ] + + +@pytest.mark.asyncio +async def test_dispatcher_rejects_modified_action_without_execution(monkeypatch): + import src.tool_execution as tool_execution + + ToolBlock = namedtuple("ToolBlock", ["tool_type", "content"]) + store = ToolApprovalStore() + pending = _pending(store) + grant = store.consume( + pending.approval_id, + decision="approve", + owner="alice", + session_id="session-1", + ) + + async def should_not_run(*args, **kwargs): + raise AssertionError("modified approved action reached implementation") + + monkeypatch.setattr( + tool_execution, + "_execute_tool_block_impl", + should_not_run, + ) + _, result = await tool_execution.execute_tool_block( + ToolBlock("bash", "printf changed"), + session_id="session-1", + owner="alice", + workspace="/tmp/workspace", + security_context=ToolRunSecurityContext(), + run_policy=AgentRunPolicy.for_mode("ask"), + exact_approval=grant, + ) + + assert result["blocked"] is True + assert result["policy"] == "exact_tool_approval" + + +@pytest.mark.asyncio +async def test_dispatcher_rejects_full_access_for_non_admin(monkeypatch): + import src.tool_execution as tool_execution + + ToolBlock = namedtuple("ToolBlock", ["tool_type", "content"]) + monkeypatch.setattr( + tool_execution, + "owner_is_admin_or_single_user", + lambda owner: False, + ) + + async def should_not_run(*args, **kwargs): + raise AssertionError("non-admin host action reached implementation") + + monkeypatch.setattr( + tool_execution, + "_execute_tool_block_impl", + should_not_run, + ) + _, result = await tool_execution.execute_tool_block( + ToolBlock("bash", "printf host"), + session_id="session-1", + owner="ordinary-user", + workspace="/tmp/workspace", + security_context=ToolRunSecurityContext(), + run_policy=AgentRunPolicy.for_mode("full_access"), + ) + + assert result["blocked"] is True + assert "admin" in result["error"].lower() diff --git a/tests/test_ask_user_persistence.py b/tests/test_ask_user_persistence.py index 56dabd1b2..c190cd88b 100644 --- a/tests/test_ask_user_persistence.py +++ b/tests/test_ask_user_persistence.py @@ -95,3 +95,15 @@ def test_frontend_uses_one_renderer_for_live_and_restored_cards(): assert "export function renderAskUserCard" in renderer assert "renderAskUserCard(pendingAskUser" in renderer assert "if (role === 'user') removeAskUserCards(box)" in renderer + + +def test_tool_approval_card_submits_only_opaque_decision_metadata(): + chat = (ROOT / "static" / "js" / "chat.js").read_text(encoding="utf-8") + renderer = (ROOT / "static" / "js" / "chatRenderer.js").read_text(encoding="utf-8") + + assert "odysseus:tool-approval" in renderer + assert "approval_id: aq.approval_id" in renderer + assert "fd.append('tool_approval_id'" in chat + assert "fd.append('tool_approval_decision'" in chat + assert "fd.append('tool_approval_content'" not in chat + assert "aq.action.content || ''" in renderer diff --git a/tests/test_execution_sandbox.py b/tests/test_execution_sandbox.py index 96a45c17d..6dec3f179 100644 --- a/tests/test_execution_sandbox.py +++ b/tests/test_execution_sandbox.py @@ -9,6 +9,8 @@ from pathlib import Path import pytest +from src.agent_run_policy import AgentRunPolicy +from src.tool_capabilities import ToolRunSecurityContext from src.execution_sandbox import ( SandboxUnavailable, environment_for_sandbox_launcher, @@ -262,3 +264,82 @@ def test_detached_background_job_uses_sandbox(tmp_path, monkeypatch): assert current["exit_code"] == 0 assert "background-ok" in current["output"] assert (workspace / "bg-write.txt").exists() + + +@pytest.mark.asyncio +async def test_explicit_full_access_runs_bash_with_host_visibility( + tmp_path, + monkeypatch, +): + import src.tool_execution as tool_execution + from src.agent_tools import ToolBlock + + workspace = tmp_path / "workspace" + workspace.mkdir() + outside = tmp_path / "outside-visible" + outside.write_text("host", encoding="utf-8") + monkeypatch.setenv("ODYSSEUS_FULL_ACCESS_TEST", "visible") + monkeypatch.setattr( + tool_execution, + "owner_is_admin_or_single_user", + lambda owner: True, + ) + monkeypatch.setattr(tool_execution, "get_mcp_manager", lambda: None) + + _, result = await tool_execution.execute_tool_block( + ToolBlock( + "bash", + ( + f"test -e {outside!s} " + '&& test "$ODYSSEUS_FULL_ACCESS_TEST" = visible ' + "&& printf host-ok" + ), + ), + owner="admin", + workspace=str(workspace), + security_context=ToolRunSecurityContext(), + run_policy=AgentRunPolicy.for_mode("full_access"), + ) + + assert result["exit_code"] == 0 + assert "host-ok" in result["output"] + + +def test_explicit_full_access_background_job_uses_host_profile( + tmp_path, + monkeypatch, +): + from src import bg_jobs + + jobs_dir = tmp_path / "jobs" + jobs_dir.mkdir() + workspace = tmp_path / "workspace" + workspace.mkdir() + outside = tmp_path / "outside-visible" + outside.write_text("host", encoding="utf-8") + monkeypatch.setenv("ODYSSEUS_FULL_ACCESS_TEST", "visible") + monkeypatch.setattr(bg_jobs, "_JOBS_DIR", jobs_dir) + monkeypatch.setattr(bg_jobs, "_STORE", tmp_path / "jobs.json") + + record = bg_jobs.launch( + ( + f"test -e {outside!s} " + '&& test "$ODYSSEUS_FULL_ACCESS_TEST" = visible ' + f'&& test "$(pwd)" = "{workspace!s}" ' + "&& printf host-background-ok" + ), + session_id="full-access-session", + cwd=str(workspace), + max_runtime_s=10, + execution_profile="host_full_access", + ) + deadline = time.time() + 10 + current = record + while current.get("status") == "running" and time.time() < deadline: + time.sleep(0.05) + current = bg_jobs.get(record["id"]) or current + + assert current["status"] == "done", current + assert current["exit_code"] == 0 + assert "host-background-ok" in current["output"] + assert current["execution_profile"] == "host_full_access" diff --git a/tests/test_external_context_tool_gate.py b/tests/test_external_context_tool_gate.py index 3859dbb9d..ee3ca118c 100644 --- a/tests/test_external_context_tool_gate.py +++ b/tests/test_external_context_tool_gate.py @@ -13,6 +13,8 @@ from src.tool_capabilities import ( capabilities_for_tool, messages_contain_external_untrusted_context, ) +from src.tool_approvals import ToolApprovalStore +from src.tool_capabilities import capabilities_for_tool ToolBlock = namedtuple("ToolBlock", ["tool_type", "content"]) @@ -214,7 +216,7 @@ async def test_dispatcher_updates_context_from_external_result(monkeypatch): assert result["blocked"] is True -def test_fake_weak_model_search_then_bash_next_round_is_blocked(monkeypatch): +def test_fake_weak_model_search_then_bash_next_round_requires_approval(monkeypatch): executed = [] agent_loop = _patch_agent_loop( monkeypatch, @@ -239,7 +241,12 @@ def test_fake_weak_model_search_then_bash_next_round_is_blocked(monkeypatch): assert any( event.get("type") == "tool_output" and event.get("tool") == "bash" - and event.get("exit_code") == 1 + and event.get("ask_user", {}).get("kind") == "tool_approval" + for event in events + ) + assert any( + event.get("type") == "ask_user" + and event.get("data", {}).get("kind") == "tool_approval" for event in events ) assert not any( @@ -248,7 +255,7 @@ def test_fake_weak_model_search_then_bash_next_round_is_blocked(monkeypatch): ) -def test_fake_weak_model_search_then_bash_same_batch_is_blocked(monkeypatch): +def test_fake_weak_model_search_then_bash_same_batch_requires_approval(monkeypatch): executed = [] agent_loop = _patch_agent_loop( monkeypatch, @@ -273,9 +280,209 @@ def test_fake_weak_model_search_then_bash_same_batch_is_blocked(monkeypatch): ) assert executed == ["web_search"] - blocked = [ + pending = [ event for event in events if event.get("type") == "tool_output" and event.get("tool") == "bash" ] - assert blocked and blocked[0]["exit_code"] == 1 + assert pending + assert pending[0]["exit_code"] is None + assert pending[0]["ask_user"]["kind"] == "tool_approval" + assert not any( + event.get("type") == "tool_start" and event.get("tool") == "bash" + for event in events + ) + + +def test_ask_mode_never_starts_model_requested_bash_without_approval(monkeypatch): + executed = [] + agent_loop = _patch_agent_loop( + monkeypatch, + ["```bash\nprintf requested\n```"], + executed, + ) + + events = _collect_agent_events( + agent_loop.stream_agent_loop( + "http://local.test/v1", + "small-local-model", + [{"role": "user", "content": "run it"}], + max_rounds=1, + relevant_tools={"bash"}, + security_mode="ask", + ) + ) + + assert executed == [] + assert any( + event.get("type") == "ask_user" + and event.get("data", {}).get("kind") == "tool_approval" + for event in events + ) + assert not any(event.get("type") == "tool_start" for event in events) + + +def test_approval_boundary_stops_later_blocks_in_same_model_batch(monkeypatch): + executed = [] + agent_loop = _patch_agent_loop( + monkeypatch, + [ + ( + "```bash\nprintf requested\n```\n" + "```read_file\nshould-not-run.txt\n```" + ) + ], + executed, + ) + + events = _collect_agent_events( + agent_loop.stream_agent_loop( + "http://local.test/v1", + "small-local-model", + [{"role": "user", "content": "run and read"}], + max_rounds=1, + relevant_tools={"bash", "read_file"}, + security_mode="ask", + ) + ) + + assert executed == [] + assert not any( + event.get("type") == "tool_start" + and event.get("tool") == "read_file" + for event in events + ) + + +def test_approved_resume_executes_sealed_action_before_next_model_turn(monkeypatch): + import src.agent_loop as agent_loop + + store = ToolApprovalStore() + pending = store.create( + owner="alice", + session_id="session-1", + origin_run_id="origin-run", + tool_name="bash", + content="printf sealed", + workspace="/tmp/workspace", + security_mode="ask", + external_untrusted_context_seen=True, + capabilities=capabilities_for_tool("bash"), + ) + grant = store.consume( + pending.approval_id, + decision="approve", + owner="alice", + session_id="session-1", + ) + executed = [] + + monkeypatch.setattr( + agent_loop, + "get_setting", + lambda key, default=None: default, + raising=False, + ) + monkeypatch.setattr(agent_loop, "get_mcp_manager", lambda: None) + monkeypatch.setattr(agent_loop, "estimate_tokens", lambda *args, **kwargs: 10) + + async def fake_stream(*args, **kwargs): + yield "data: " + json.dumps( + {"delta": "```bash\nprintf substituted\n```"} + ) + "\n\n" + yield "data: [DONE]\n\n" + + async def fake_execute(block, *args, **kwargs): + executed.append((block.tool_type, block.content)) + return "bash", {"output": "sealed output", "exit_code": 0} + + monkeypatch.setattr(agent_loop, "stream_llm_with_fallback", fake_stream) + monkeypatch.setattr(agent_loop, "execute_tool_block", fake_execute) + + events = _collect_agent_events( + agent_loop.stream_agent_loop( + "http://local.test/v1", + "small-local-model", + [{"role": "user", "content": "Allow once"}], + session_id="session-1", + owner="alice", + workspace="/tmp/workspace", + relevant_tools={"bash"}, + max_rounds=1, + security_mode="ask", + exact_approval=grant, + ) + ) + + assert executed == [("bash", "printf sealed")] + starts = [ + event for event in events if event.get("type") == "tool_start" + ] + assert starts[0]["full_command"] == "printf sealed" + assert starts[0]["approved"] is True + assert any( + event.get("type") == "ask_user" + and event.get("data", {}).get("action", {}).get("content") + == "printf substituted" + for event in events + ) + + +def test_mismatched_approval_does_not_expose_or_execute_sealed_action( + monkeypatch, +): + import src.agent_loop as agent_loop + + store = ToolApprovalStore() + pending = store.create( + owner="alice", + session_id="session-1", + origin_run_id="origin-run", + tool_name="bash", + content="printf owner-secret-command", + workspace="/tmp/workspace", + security_mode="ask", + external_untrusted_context_seen=False, + capabilities=capabilities_for_tool("bash"), + ) + grant = store.consume( + pending.approval_id, + decision="approve", + owner="alice", + session_id="session-1", + ) + monkeypatch.setattr( + agent_loop, + "get_setting", + lambda key, default=None: default, + raising=False, + ) + monkeypatch.setattr(agent_loop, "get_mcp_manager", lambda: None) + monkeypatch.setattr(agent_loop, "estimate_tokens", lambda *args, **kwargs: 10) + + async def fake_stream(*args, **kwargs): + yield f"data: {json.dumps({'delta': 'Denied safely.'})}\n\n" + yield "data: [DONE]\n\n" + + monkeypatch.setattr(agent_loop, "stream_llm_with_fallback", fake_stream) + events = _collect_agent_events( + agent_loop.stream_agent_loop( + "http://local.test/v1", + "small-local-model", + [{"role": "user", "content": "Allow once"}], + session_id="session-1", + owner="mallory", + workspace="/tmp/workspace", + max_rounds=1, + security_mode="ask", + exact_approval=grant, + ) + ) + + assert not any(event.get("type") == "tool_start" for event in events) + outputs = [ + event for event in events if event.get("type") == "tool_output" + ] + assert outputs and outputs[0]["exit_code"] == 1 + assert outputs[0]["command"] == "" + assert "owner-secret-command" not in json.dumps(events) diff --git a/tests/test_session_mode_helpers.py b/tests/test_session_mode_helpers.py index 28f2a8348..48a3fa837 100644 --- a/tests/test_session_mode_helpers.py +++ b/tests/test_session_mode_helpers.py @@ -26,7 +26,13 @@ def _load_db_helpers(): """Load only the helper bodies under test, without importing SQLAlchemy.""" db_path = Path(__file__).parents[1] / "core" / "database.py" tree = ast.parse(db_path.read_text(encoding="utf-8"), filename=str(db_path)) - wanted = {"get_db_session", "get_session_mode", "set_session_mode"} + wanted = { + "get_db_session", + "get_session_mode", + "set_session_mode", + "get_session_security_mode", + "set_session_security_mode", + } helper_nodes = [ node for node in tree.body if isinstance(node, ast.FunctionDef) and node.name in wanted @@ -80,3 +86,23 @@ def test_get_session_mode_does_not_leak_on_error(monkeypatch): sess.query.return_value.filter.return_value.scalar.side_effect = RuntimeError("database is locked") assert db.get_session_mode("s1") is None sess.close.assert_called_once() + + +def test_security_mode_defaults_safely_and_validates_writes(monkeypatch): + db, sess = _mock_session(monkeypatch) + sess.query.return_value.filter.return_value.scalar.return_value = "unexpected" + + assert db.get_session_security_mode("s1") == "sandbox" + assert db.set_session_security_mode("s1", "not-a-mode") is False + sess.query.return_value.filter.return_value.update.assert_not_called() + + +def test_security_mode_persists_valid_value(monkeypatch): + db, sess = _mock_session(monkeypatch) + + assert db.set_session_security_mode("s1", "ask") is True + sess.query.return_value.filter.return_value.update.assert_called_once_with( + {"security_mode": "ask"} + ) + sess.commit.assert_called_once() + sess.close.assert_called_once() diff --git a/tests/test_teacher_eval_tier2.py b/tests/test_teacher_eval_tier2.py index c3eb2a643..bf23de7c4 100644 --- a/tests/test_teacher_eval_tier2.py +++ b/tests/test_teacher_eval_tier2.py @@ -185,7 +185,10 @@ async def test_run_teacher_inline_triggers_tier2_escalation(monkeypatch): monkeypatch.setattr("src.teacher_escalation.evaluate_turn_llm", fake_evaluate_turn_llm) # Mock stream_agent_loop recursively called by run_teacher_inline + nested_kwargs = [] + async def fake_stream_agent_loop(*args, **kwargs): + nested_kwargs.append(kwargs) yield "data: {\"type\": \"tool_output\", \"tool\": \"bash\"}\n\n" yield "data: {\"type\": \"text\", \"delta\": \"Teacher reply\"}\n\n" yield "data: [DONE]\n\n" @@ -208,6 +211,10 @@ async def test_run_teacher_inline_triggers_tier2_escalation(monkeypatch): student_tool_events=[], student_reply="student reply", owner="alice", + session_id="session-1", + workspace="/tmp/workspace", + security_mode="ask", + external_untrusted_context_seen=True, ): events.append(evt) @@ -215,6 +222,11 @@ async def test_run_teacher_inline_triggers_tier2_escalation(monkeypatch): assert any("teacher_takeover" in evt for evt in events) assert any("tool_output" in evt for evt in events) assert any("skill_saved" in evt for evt in events) + assert nested_kwargs + assert nested_kwargs[0]["session_id"] == "session-1" + assert nested_kwargs[0]["workspace"] == "/tmp/workspace" + assert nested_kwargs[0]["security_mode"] == "ask" + assert nested_kwargs[0]["external_untrusted_context_seen"] is True @pytest.mark.asyncio