From 3834cd72b1f091313b5a4eeebb11b584a464008b Mon Sep 17 00:00:00 2001 From: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com> Date: Fri, 2 Oct 2026 23:39:13 +0100 Subject: [PATCH] fix(runtime): enforce local control across Cookbook wrappers --- routes/codex_routes.py | 11 ++++++++ routes/cookbook_routes.py | 9 +++++++ src/agent_runtime/owned_resources.py | 2 ++ tests/test_cookbook_docker_access.py | 10 ++++++-- tests/test_wave3_background_followup.py | 12 +++------ tests/test_wave3_local_control.py | 34 +++++++++++++++++++++++++ 6 files changed, 68 insertions(+), 10 deletions(-) diff --git a/routes/codex_routes.py b/routes/codex_routes.py index 9fe36a822..f42c6b632 100644 --- a/routes/codex_routes.py +++ b/routes/codex_routes.py @@ -118,6 +118,17 @@ def _require_cookbook_scope(request: Request, allowed: set[str]) -> str: because cookbook surfaces expose host topology, task logs, tmux commands, and model-serving controls. """ + # Internal transport/owner attribution is not a scoped external credential. + # In no-login mode, this wrapper must preserve the native local-operator + # boundary even though it invokes endpoint functions without dependencies. + from src.agent_runtime.authority import is_internal_tool_request + from src.auth_helpers import _auth_disabled + from core.middleware import INTERNAL_TOOL_HEADER + if is_internal_tool_request(request) or request.headers.get(INTERNAL_TOOL_HEADER): + raise HTTPException(403, "Internal Cookbook calls require a dedicated producer") + if _auth_disabled(): + from routes.shell_routes import _require_admin + _require_admin(request) owner = _scope_owner(request, allowed) if not getattr(request.state, "api_token", False): require_admin(request) diff --git a/routes/cookbook_routes.py b/routes/cookbook_routes.py index 995947d5e..02b419894 100644 --- a/routes/cookbook_routes.py +++ b/routes/cookbook_routes.py @@ -426,6 +426,13 @@ def setup_cookbook_routes() -> APIRouter: if not claimed: _require_admin(request) router = APIRouter(tags=["cookbook"], dependencies=[Depends(protect_native_control)]) + + def protect_local_model_producer(request, remote_host): + # Scoped wrappers can call endpoint functions directly, without FastAPI + # dependencies. Enforce native control at the actual producer as well. + if not remote_host and getattr(request.state, "local_model_authority", None) is None: + from routes.shell_routes import _require_admin + _require_admin(request) _cookbook_state_path = Path(COOKBOOK_STATE_FILE) _state_get_cache = {"ts": 0.0, "mtime": 0.0, "value": None} _tasks_status_cache = {"ts": 0.0, "value": None} @@ -1100,6 +1107,7 @@ def setup_cookbook_routes() -> APIRouter: """Download a HuggingFace model in a tmux session. Uses `hf download` CLI directly — runs in tmux via `script -qc` for real TTY progress, streams ANSI-stripped output via log file.""" + protect_local_model_producer(request, req.remote_host) require_admin(request) # Defence-in-depth: even though this endpoint is admin-gated, refuse # values that would land in shell contexts with metacharacters. @@ -2018,6 +2026,7 @@ def setup_cookbook_routes() -> APIRouter: keep strict validation, but serving local cached models must not require a fake org/name wrapper. """ + protect_local_model_producer(request, req.remote_host) require_admin(request) # Defence-in-depth: reject values that could break out of shell contexts. validate_remote_host(req.remote_host) diff --git a/src/agent_runtime/owned_resources.py b/src/agent_runtime/owned_resources.py index 7bb429288..a83c8180b 100644 --- a/src/agent_runtime/owned_resources.py +++ b/src/agent_runtime/owned_resources.py @@ -260,6 +260,8 @@ def needs_owned_binding(operation): "notes", "memory", "vault", "upload", "uploads", "attachments", "shell", "model", "cookbook"} segments = path.strip("/").split("/") + if len(segments) >= 3 and segments[:3] == ["api", "codex", "cookbook"]: + raise ResourceIdentityError("Cookbook wrappers require a dedicated resource-bound tool") if len(segments) >= 2 and segments[0] == "api" and segments[1].casefold() in private: raise ResourceIdentityError("Owned records require a dedicated resource-bound tool") return False diff --git a/tests/test_cookbook_docker_access.py b/tests/test_cookbook_docker_access.py index 5acf49e0a..d8fe8d404 100644 --- a/tests/test_cookbook_docker_access.py +++ b/tests/test_cookbook_docker_access.py @@ -1,4 +1,5 @@ from unittest.mock import AsyncMock +from types import SimpleNamespace import pytest @@ -11,6 +12,11 @@ from src.host_docker_access import HOST_DOCKER_ACCESS_HINT from tests.helpers.unix_sockets import bound_unix_socket +@pytest.fixture(autouse=True) +def authenticated_admin_mode(monkeypatch): + monkeypatch.setenv("AUTH_ENABLED", "true") + + def _model_serve_endpoint(): router = cookbook_routes.setup_cookbook_routes() for route in router.routes: @@ -27,6 +33,8 @@ def _admin_request() -> Request: "path": "/api/model/serve", "headers": [], "state": {}, + "app": SimpleNamespace(state=SimpleNamespace(auth_manager=SimpleNamespace( + is_configured=True, is_admin=lambda user: user == "admin"))), } ) request.state.current_user = "admin" @@ -139,7 +147,6 @@ async def test_local_container_serve_returns_host_docker_opt_in_hint( assert cookbook_routes.shutil.which(binary) == "/usr/bin/docker" return False - monkeypatch.setattr(cookbook_routes, "require_admin", lambda request: None) monkeypatch.setattr(cookbook_routes, "_binary_available", binary_available) monkeypatch.setattr(cookbook_routes, "running_in_container", lambda: True) monkeypatch.setattr( @@ -199,7 +206,6 @@ async def test_local_container_serve_allows_generated_docker_exec_when_enabled( launched_commands.append(command) return _Process() - monkeypatch.setattr(cookbook_routes, "require_admin", lambda request: None) monkeypatch.setattr(cookbook_routes, "_binary_available", binary_available) monkeypatch.setattr(cookbook_routes, "running_in_container", lambda: True) monkeypatch.setattr( diff --git a/tests/test_wave3_background_followup.py b/tests/test_wave3_background_followup.py index 4deeb0ef5..9c778fe96 100644 --- a/tests/test_wave3_background_followup.py +++ b/tests/test_wave3_background_followup.py @@ -1,13 +1,12 @@ """Permanent linkage loss suppresses continuation without granting authority.""" -import asyncio -import sys -from types import ModuleType, SimpleNamespace +from types import SimpleNamespace import time import pytest from src import bg_jobs, bg_monitor from src.agent_runtime import process_resources as resources from tests.test_background_resource_identity import store, seed +from src.agent_runtime.resources import ResourceIdentityError @pytest.fixture @@ -15,8 +14,8 @@ def monitor_session(monkeypatch): messages = [] sess = SimpleNamespace(id='thread', owner='alice', model='test-model', get_context_messages=lambda: []) sm = SimpleNamespace(get_session=lambda sid: sess, add_message=lambda *args: messages.append(args), save_sessions=lambda: None) - ai = ModuleType('src.ai_interaction'); ai.get_session_manager = lambda: sm - monkeypatch.setitem(sys.modules, 'src.ai_interaction', ai) + import src.ai_interaction as ai + monkeypatch.setattr(ai, 'get_session_manager', lambda: sm) import src.agent_runs monkeypatch.setattr(src.agent_runs, 'is_active', lambda sid: False) async def drain(*args, **kwargs): @@ -49,9 +48,6 @@ async def test_invalid_linkage_is_terminal_without_message(store, monkeypatch, m resources.validate_job(resource) -from src.agent_runtime.resources import ResourceIdentityError - - async def test_busy_session_retries_then_continues(store, monkeypatch, monitor_session): _, rec = seed(store, status='done') import src.agent_runs diff --git a/tests/test_wave3_local_control.py b/tests/test_wave3_local_control.py index ba3e77304..ba75f8e59 100644 --- a/tests/test_wave3_local_control.py +++ b/tests/test_wave3_local_control.py @@ -207,3 +207,37 @@ async def test_auth_disabled_local_route_usage(control_app, monkeypatch): async with httpx.AsyncClient(transport=httpx.ASGITransport(app=app, client=('192.0.2.1', 1)), base_url='http://127.0.0.1') as client: for path, body in [('/api/shell/exec', {'command': ''}), ('/api/cookbook/state', {'tasks': []}), ('/api/model/download', {'repo_id': 'org/model'})]: assert (await client.post(path, json=body)).status_code == 403 + + +@pytest.mark.parametrize('host,internal', [('127.0.0.1', True), ('192.0.2.1', False)]) +async def test_scoped_wrapper_cannot_bypass_native_control(control_app, monkeypatch, host, internal): + from routes.codex_routes import setup_codex_routes + app, spawned, _ = control_app + app.include_router(setup_codex_routes()) + monkeypatch.setenv('AUTH_ENABLED', 'false') + headers = {INTERNAL_TOOL_HEADER: INTERNAL_TOOL_TOKEN} if internal else {} + async with httpx.AsyncClient(transport=httpx.ASGITransport(app=app, client=(host, 123)), base_url='http://127.0.0.1') as client: + r = await client.post('/api/codex/cookbook/serve', json={'repo_id': 'samplepkg', 'cmd': 'python -m pip install samplepkg'}, headers=headers) + assert r.status_code == 403 and not spawned + + +@pytest.mark.parametrize('path', ['/api/codex/cookbook/serve', '/api/codex/cookbook/stop/job', '/api/codex/%63ookbook/serve']) +async def test_generic_app_api_cannot_substitute_scoped_wrapper(control_app, path): + _, spawned, work = control_app + authority = RequestAuthority('request', 'alice', 'thread', str(work), (OperationGrant('app_api'),)) + content = json.dumps({'action': 'call', 'method': 'POST', 'path': path, 'body': {'repo_id': 'samplepkg', 'cmd': 'python -m pip install samplepkg'}}) + _, result = await tool_execution.execute_tool_block(ToolBlock('app_api', content), owner='alice', + session_id='thread', workspace=str(work), request_authority=authority, security_context=ToolRunSecurityContext()) + assert result['failure_kind'] == 'resource_identity_denied' and not spawned + + +async def test_direct_endpoint_call_keeps_producer_gate(control_app, monkeypatch): + from routes.cookbook_helpers import ModelDownloadRequest + app, spawned, _ = control_app + router = cookbook_routes.setup_cookbook_routes() + endpoint = next(route.endpoint for route in router.routes if getattr(route, 'path', '') == '/api/model/download') + monkeypatch.setenv('AUTH_ENABLED', 'false') + req = request('192.0.2.1') + with pytest.raises(HTTPException) as exc: + await endpoint(req, ModelDownloadRequest(repo_id='org/model')) + assert exc.value.status_code == 403 and not spawned