From a3784da1725823d9fc402da16c8a555a45db7d1d Mon Sep 17 00:00:00 2001 From: RaresKeY <158580472+RaresKeY@users.noreply.github.com> Date: Sun, 7 Jun 2026 16:19:08 +0300 Subject: [PATCH] fix: block app_api access to shell routes (#3225) --- src/agent_loop.py | 5 +- src/tool_implementations.py | 14 +++--- src/tool_index.py | 2 +- src/tool_schemas.py | 2 +- tests/test_review_regressions.py | 85 ++++++++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 10 deletions(-) diff --git a/src/agent_loop.py b/src/agent_loop.py index f936e759a..7a626fb7d 100644 --- a/src/agent_loop.py +++ b/src/agent_loop.py @@ -357,7 +357,7 @@ If the user asks for a reminder/alarm before the event, pass `reminder_minutes` ```app_api {"action": "call", "method": "GET", "path": "/api/cookbook/gpus"} ``` -GENERIC LOOPBACK to ANY Odysseus internal endpoint. Use this whenever the user wants something the UI can do but there's NO named tool for it. Every UI button hits some /api/* endpoint — you can hit the same one. Auth is handled automatically. +GENERIC LOOPBACK to allowed Odysseus internal endpoints. Use this whenever the user wants something the UI can do but there's NO named tool for it. Many UI buttons hit /api/* endpoints — you can hit allowed ones. Auth is handled automatically. **Discovery first.** If you're not sure of the path, call `{"action":"endpoints","filter":""}` (e.g. filter='calendar' or 'gallery' or 'theme') to list available endpoints with their methods + summaries. Then call with action='call'. @@ -376,12 +376,13 @@ GENERIC LOOPBACK to ANY Odysseus internal endpoint. Use this whenever the user w - Compare: `/api/compare/sessions`, `/api/compare/start` - Email: use named email tools (`list_email_accounts`, `list_emails`, `read_email`, `send_email`, `reply_to_email`). Do NOT use `/api/email/accounts`; it is owner-filtered in tool context and may falsely return empty. - Endpoints (model providers): `/api/endpoints`, `/api/endpoints/{id}` +- Shell: do NOT use `app_api` for `/api/shell/*`; use named command tooling instead. Body for POST/PUT/PATCH goes in `body` (object). Query params in `query` (object). Returns the parsed JSON of the response. **When to prefer named tools over app_api:** if a named wrapper exists (list_email_accounts, list_emails, read_email, manage_calendar, manage_notes, list_served_models, etc.) USE IT — it has nicer output formatting and clearer schema. Reach for `app_api` only when there's no wrapper for what you need. -Blocked paths (refused for safety): /api/auth/, /api/users/, /api/tokens/, /api/admin/, /api/backup/restore, /api/email/accounts.""", +Blocked paths (refused for safety): /api/auth/, /api/users/, /api/tokens/, /api/admin/, /api/shell/, /api/backup/restore, /api/email/accounts.""", } def get_builtin_overrides() -> dict: diff --git a/src/tool_implementations.py b/src/tool_implementations.py index e589652f0..316588b0b 100644 --- a/src/tool_implementations.py +++ b/src/tool_implementations.py @@ -2693,14 +2693,15 @@ async def _cookbook_register_task(session_id: str, model: str, host: str, # Paths the generic `app_api` tool will refuse to call. Auth/token/user -# administration is too risky to route through an agent surface even -# when the agent is admin-context — accidental "delete account" -# style mistakes have permanent blast radius. +# administration and host shell execution are too risky to route through an +# agent surface even when the agent is admin-context; accidental account or +# command mistakes have permanent blast radius. _APP_API_BLOCKLIST_PREFIXES = ( "/api/auth", # login/logout/password "/api/users", # user CRUD (bare /api/users list+create+delete must also block) "/api/tokens", # api token mgmt (bare /api/tokens list+create must also block) "/api/admin", # admin one-shots (wipe etc.) + "/api/shell", # host shell execution must stay behind named command tooling "/api/backup/restore", # destructive restore ) @@ -2737,7 +2738,7 @@ _APP_API_BLOCKLIST_METHOD_PATH = ( async def do_app_api(content: str, owner: Optional[str] = None) -> Dict: - """Generic loopback to any internal Odysseus API endpoint. Lets the + """Generic loopback to allowed internal Odysseus API endpoints. Lets the agent reach the full UI-button surface (cookbook, email, notes, calendar, skills, sessions, gallery, research, etc.) without us landing a named tool wrapper for every one. @@ -2751,7 +2752,8 @@ async def do_app_api(content: str, owner: Optional[str] = None) -> Dict: The `endpoints` action returns the OpenAPI surface (method + path + summary) so the agent can discover what's reachable. A blocklist - refuses auth/user/admin paths to keep blast radius bounded. + refuses sensitive auth/user/admin/shell paths to keep blast radius + bounded. """ import httpx try: @@ -2811,7 +2813,7 @@ async def do_app_api(content: str, owner: Optional[str] = None) -> Dict: if not path.startswith("/"): path = "/" + path if any(path.startswith(p) for p in _APP_API_BLOCKLIST_PREFIXES): - return {"error": f"Path blocked for safety: {path}. Auth/user/admin endpoints are off-limits via app_api.", "exit_code": 1} + return {"error": f"Path blocked for safety: {path}. Sensitive endpoints are off-limits via app_api.", "exit_code": 1} method = (args.get("method") or "GET").upper() if method not in ("GET", "POST", "PUT", "PATCH", "DELETE"): diff --git a/src/tool_index.py b/src/tool_index.py index b7a703571..2db125447 100644 --- a/src/tool_index.py +++ b/src/tool_index.py @@ -153,7 +153,7 @@ BUILTIN_TOOL_DESCRIPTIONS: Dict[str, str] = { "serve_preset": "Launch a saved Cookbook serve preset by name. Reuses the exact tmux command + host the user already saved. Use for 'run stable diffusion 3.5', 'serve vllm-qwen', 'start the inpaint model' — preset-name matches the user's UI labels.", "adopt_served_model": "Register an existing tmux model server (one started manually or outside the cookbook flow) into Cookbook tracking AND add it as a chat endpoint. Use when the user (or a previous turn) launched something via ssh+tmux and now wants it visible in the UI, stoppable via stop_served_model, and usable in the model picker.", "list_cookbook_servers": "List the cookbook's configured servers (remote GPU boxes + local) and which is the current default. Use this BEFORE download_model/serve_model when the user didn't name a host — to decide where to run, or to ask the user which server when ambiguous. Downloads/serves default to the cookbook's selected server, NOT localhost.", - "app_api": "Generic loopback to ANY Odysseus internal endpoint. Use this when the user wants something the UI can do but there's no named tool for it. Covers calendar, gallery, library/documents, memory, notes, tasks, settings, research, compare, cookbook GPUs/state — every UI button hits some /api/* endpoint and you can hit it too. action='endpoints' with filter= lists available endpoints. action='call' takes method+path+body. Hits same routes the UI uses — auth flows free. NOTE: themes are NOT an API endpoint — use the ui_control tool (create_theme / set_theme), not app_api. SESSIONS/CHATS: do NOT use app_api for these — GET /api/sessions returns EMPTY for tool calls (it's owner-filtered and tool calls authenticate as a different identity). EMAIL ACCOUNTS: do NOT use /api/email/accounts via app_api; use list_email_accounts, list_emails, and read_email instead. To list/rename/archive/delete/fork chats use the list_sessions and manage_session tools instead.", + "app_api": "Generic loopback to allowed Odysseus internal endpoints. Use this when the user wants something the UI can do but there's no named tool for it. Covers calendar, gallery, library/documents, memory, notes, tasks, settings, research, compare, cookbook GPUs/state — every allowed UI button hits some /api/* endpoint and you can hit it too. Sensitive auth/user/admin/shell paths are blocked; do NOT use app_api for shell commands, use named command tooling instead. action='endpoints' with filter= lists available endpoints. action='call' takes method+path+body. Hits same routes the UI uses — auth flows free. NOTE: themes are NOT an API endpoint — use the ui_control tool (create_theme / set_theme), not app_api. SESSIONS/CHATS: do NOT use app_api for these — GET /api/sessions returns EMPTY for tool calls (it's owner-filtered and tool calls authenticate as a different identity). EMAIL ACCOUNTS: do NOT use /api/email/accounts via app_api; use list_email_accounts, list_emails, and read_email instead. To list/rename/archive/delete/fork chats use the list_sessions and manage_session tools instead.", "edit_image": "Edit an image in the gallery: upscale (increase resolution), remove background (rembg), inpaint (fill selected area), or harmonize (blend edits). Specify image ID and action.", "trigger_research": "Start a deep research job on any topic — appears in the Deep Research sidebar, streams progress, produces a detailed report. Use for 'research X', 'look into Y', 'do deep research on Z', 'investigate'. NOT a scheduled task — it runs now and surfaces in the sidebar.", } diff --git a/src/tool_schemas.py b/src/tool_schemas.py index 307a3516a..6b8be74fd 100644 --- a/src/tool_schemas.py +++ b/src/tool_schemas.py @@ -950,7 +950,7 @@ FUNCTION_TOOL_SCHEMAS = [ "type": "function", "function": { "name": "app_api", - "description": "Generic loopback to ANY internal Odysseus endpoint. Use this when there's no named tool for what the user wants. Hits the same routes the UI buttons hit (cookbook, gallery, library/documents, memory, notes, calendar, tasks, settings, themes, research, compare, etc.). action='endpoints' returns the OpenAPI surface (use `filter` to narrow). action='call' (default) takes method+path+body. Auth/user/admin paths are blocked for safety. Do not use for email account discovery; use list_email_accounts instead because /api/email/accounts is owner-filtered in tool context.", + "description": "Generic loopback to allowed internal Odysseus endpoints. Use this when there's no named tool for what the user wants. Hits the same routes the UI buttons hit (cookbook, gallery, library/documents, memory, notes, calendar, tasks, settings, themes, research, compare, etc.). action='endpoints' returns the OpenAPI surface (use `filter` to narrow). action='call' (default) takes method+path+body. Sensitive auth/user/admin/shell paths are blocked for safety. Do not use for shell commands; use named command tooling instead. Do not use for email account discovery; use list_email_accounts instead because /api/email/accounts is owner-filtered in tool context.", "parameters": { "type": "object", "properties": { diff --git a/tests/test_review_regressions.py b/tests/test_review_regressions.py index cd451111b..a57000915 100644 --- a/tests/test_review_regressions.py +++ b/tests/test_review_regressions.py @@ -115,6 +115,19 @@ def _install_core_auth_stub(monkeypatch): return auth_mod +def _install_core_middleware_stub(monkeypatch): + """Install the narrow middleware surface needed by loopback tool tests.""" + core_mod = types.ModuleType("core") + core_mod.__path__ = [] + middleware_mod = types.ModuleType("core.middleware") + middleware_mod.INTERNAL_TOOL_HEADER = "X-Internal-Tool" + middleware_mod.INTERNAL_TOOL_TOKEN = "test-token" + core_mod.middleware = middleware_mod + monkeypatch.setitem(sys.modules, "core", core_mod) + monkeypatch.setitem(sys.modules, "core.middleware", middleware_mod) + return middleware_mod + + def test_providers_requires_admin_before_discovery_and_cache(monkeypatch): _install_model_route_import_stubs(monkeypatch) import routes.model_routes as model_routes @@ -428,6 +441,78 @@ async def test_admin_agent_tools_require_admin(monkeypatch): assert "requires an admin" in result["error"] +@pytest.mark.asyncio +async def test_app_api_blocks_shell_routes_before_loopback(monkeypatch): + import httpx + from src.tool_implementations import do_app_api + + class UnexpectedAsyncClient: + def __init__(self, *args, **kwargs): + raise AssertionError("app_api should block shell routes before loopback") + + monkeypatch.setattr(httpx, "AsyncClient", UnexpectedAsyncClient) + + for path in ("/api/shell/exec", "api/shell/stream"): + result = await do_app_api( + json.dumps( + { + "action": "call", + "method": "POST", + "path": path, + "body": {"command": "echo should-not-run"}, + } + ), + owner="admin", + ) + + assert result["exit_code"] == 1 + assert "Path blocked for safety" in result["error"] + assert "Sensitive endpoints" in result["error"] + + +@pytest.mark.asyncio +async def test_app_api_endpoint_discovery_hides_shell_routes(monkeypatch): + _install_core_middleware_stub(monkeypatch) + import httpx + from src.tool_implementations import do_app_api + + class FakeResponse: + def json(self): + return { + "paths": { + "/api/shell/exec": {"post": {"summary": "Execute Shell Command"}}, + "/api/shell/stream": {"post": {"summary": "Stream Shell Command"}}, + "/api/auth/settings": {"get": {"summary": "Auth Settings"}}, + "/api/cookbook/gpus": {"get": {"summary": "List GPUs"}}, + } + } + + class FakeAsyncClient: + def __init__(self, *args, **kwargs): + pass + + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + return False + + async def get(self, *args, **kwargs): + return FakeResponse() + + monkeypatch.setattr(httpx, "AsyncClient", FakeAsyncClient) + + result = await do_app_api(json.dumps({"action": "endpoints"}), owner="admin") + + assert result["exit_code"] == 0 + paths = {(endpoint["method"], endpoint["path"]) for endpoint in result["endpoints"]} + assert ("GET", "/api/cookbook/gpus") in paths + assert ("POST", "/api/shell/exec") not in paths + assert ("POST", "/api/shell/stream") not in paths + assert ("GET", "/api/auth/settings") not in paths + assert all(not endpoint["path"].startswith("/api/shell") for endpoint in result["endpoints"]) + + @pytest.mark.asyncio async def test_public_agent_policy_blocks_sensitive_tools(monkeypatch): auth_mod = _install_core_auth_stub(monkeypatch)