diff --git a/app.py b/app.py index e740ad518..75a7da875 100644 --- a/app.py +++ b/app.py @@ -67,7 +67,11 @@ def register_static_mime_types() -> None: REQUEST_TIMEOUT, OPENAI_API_KEY, AUTH_FILE, ) from core.database import SessionLocal, ApiToken -from core.middleware import SecurityHeadersMiddleware, is_cors_preflight +from core.middleware import ( + CodexCookbookBoundaryMiddleware, + SecurityHeadersMiddleware, + is_cors_preflight, +) from core.auth import AuthManager, normalize_known_username from core.exceptions import ( SessionNotFoundError, InvalidFileUploadError, @@ -358,7 +362,7 @@ async def dispatch(self, request: Request, call_next): path = request.url.path # A genuine CORS preflight (OPTIONS + Access-Control-Request-Method) # carries no credentials by design and must reach CORSMiddleware to be - # answered. AuthMiddleware is the outermost middleware, so gating the + # answered. AuthMiddleware runs outside CORSMiddleware, so gating the # preflight on auth 401s it before CORS can respond -- which blocks # every cross-origin browser/WebView client before the real request # is sent. Let real preflights through (only OPTIONS w/ the ACRM @@ -473,6 +477,12 @@ def _do(): else: logger.info("Auth middleware disabled (set AUTH_ENABLED=true to enable)") +# Added after AuthMiddleware so Starlette places this boundary outermost. Raw +# Odysseus bearer/internal credentials are rejected before authentication can +# read or update token state and before FastAPI parses a Cookbook route body. +# The same boundary remains installed when authentication is disabled. +app.add_middleware(CodexCookbookBoundaryMiddleware) + # ========= STATIC FILES ========= os.makedirs(STATIC_DIR, exist_ok=True) diff --git a/core/middleware.py b/core/middleware.py index 0e164e35a..8c9eade97 100644 --- a/core/middleware.py +++ b/core/middleware.py @@ -6,7 +6,8 @@ from fastapi import HTTPException, Request from starlette.middleware.base import BaseHTTPMiddleware -from starlette.responses import Response +from starlette.responses import JSONResponse, Response +from starlette.routing import get_route_path # Per-process token that lets the in-app tool layer hit admin-gated @@ -17,6 +18,7 @@ INTERNAL_TOOL_HEADER = "X-Odysseus-Internal-Token" # Pseudo-username on in-process tool-loopback requests; require_admin trusts it and it is reserved. INTERNAL_TOOL_USER = "internal-tool" +CODEX_COOKBOOK_PREFIX = "/api/codex/cookbook" def is_cors_preflight(method: str, headers) -> bool: @@ -28,6 +30,100 @@ def is_cors_preflight(method: str, headers) -> bool: return method == "OPTIONS" and "access-control-request-method" in headers +def is_codex_cookbook_path(path: str) -> bool: + """Match only the duplicate Codex Cookbook route family.""" + return path == CODEX_COOKBOOK_PREFIX or path.startswith( + f"{CODEX_COOKBOOK_PREFIX}/" + ) + + +def is_odysseus_bearer_authorization(value: str | None) -> bool: + """Recognize an Odysseus Bearer value, including proxy-combined fields.""" + if not isinstance(value, str): + return False + for candidate in value.split(","): + parts = candidate.strip().split(None, 1) + if ( + len(parts) == 2 + and parts[0].casefold() == "bearer" + and parts[1].startswith("ody_") + ): + return True + return False + + +def _header_values(headers, name: str) -> list[str]: + """Return every field value, with a mapping fallback for direct callers.""" + getlist = getattr(headers, "getlist", None) + if callable(getlist): + values = getlist(name) + else: + value = headers.get(name) + values = value if isinstance(value, (list, tuple)) else [value] + return [value for value in values if isinstance(value, str)] + + +def _internal_header_matches(value: str) -> bool: + """Compare raw or proxy-combined values without obs-text type failures.""" + candidates = [value] + trimmed_value = value.strip(" \t") + if trimmed_value != value: + candidates.append(trimmed_value) + if "," in value: + for part in value.split(","): + candidates.append(part) + trimmed_part = part.strip(" \t") + if trimmed_part != part: + candidates.append(trimmed_part) + try: + expected = INTERNAL_TOOL_TOKEN.encode("utf-8") + except (AttributeError, UnicodeError): + return False + for candidate in candidates: + try: + if secrets.compare_digest(candidate.encode("utf-8"), expected): + return True + except (AttributeError, TypeError, UnicodeError): + continue + return False + + +def require_codex_cookbook_browser(request: Request) -> None: + """Reject bearer and internal-tool principals at the shared boundary.""" + current_user = getattr(request.state, "current_user", None) + if ( + getattr(request.state, "api_token", False) + or current_user == "api" + or current_user == INTERNAL_TOOL_USER + ): + raise HTTPException(403, "Forbidden") + if any( + is_odysseus_bearer_authorization(value) + for value in _header_values(request.headers, "authorization") + ): + raise HTTPException(403, "Forbidden") + if any( + _internal_header_matches(value) + for value in _header_values(request.headers, INTERNAL_TOOL_HEADER) + ): + raise HTTPException(403, "Forbidden") + + +class CodexCookbookBoundaryMiddleware(BaseHTTPMiddleware): + """Apply the Codex Cookbook principal gate before request-body parsing.""" + + async def dispatch(self, request: Request, call_next) -> Response: + if is_codex_cookbook_path(get_route_path(request.scope)): + try: + require_codex_cookbook_browser(request) + except HTTPException as exc: + return JSONResponse( + status_code=exc.status_code, + content={"detail": exc.detail}, + ) + return await call_next(request) + + def require_admin(request: Request): """Raise 403 if the current user isn't an admin. Allows access when auth is explicitly disabled, or when the request carries diff --git a/integrations/claude/skills/odysseus/SKILL.md b/integrations/claude/skills/odysseus/SKILL.md index 31b40ee01..10a2b933c 100644 --- a/integrations/claude/skills/odysseus/SKILL.md +++ b/integrations/claude/skills/odysseus/SKILL.md @@ -1,6 +1,6 @@ --- name: odysseus -description: Use when the user asks Claude Code to read or write Odysseus data (todos, email, calendar, memory, documents) or to launch/monitor/stop a Cookbook model-serve task through the scoped Claude Agent API. Requires ODYSSEUS_URL and ODYSSEUS_API_TOKEN. +description: Use when the user asks Claude Code to read or write Odysseus data (todos, email, calendar, memory, documents) through the scoped Claude Agent API. Requires ODYSSEUS_URL and ODYSSEUS_API_TOKEN. --- # Odysseus @@ -34,6 +34,7 @@ If the user says "reminder" + a time, default to TODO with due_date. Only switch - Do not call helpers like `do_manage_notes`, email MCP internals, or database sessions directly for user data, even if shell access exists. - Never send email directly unless the user explicitly asks to send and the token has a send-capable scope. - Keep actions scoped to the token owner. +- Cookbook/model deployment is intentionally operator-controlled in the Odysseus UI and unavailable to bearer tokens; agents should consume the user-configured inference endpoint rather than attempt `/api/codex/cookbook/*`. ## Todos @@ -106,49 +107,6 @@ python3 ~/.claude/skills/odysseus/scripts/odysseus_api.py POST /api/codex/memory - `POST /api/codex/emails/draft` — body matches `SendEmailRequest` (`to`, `cc`, `bcc`, `subject`, `body`, `body_html`, `attachments`, `account_id`, `in_reply_to`, `references`). Requires `email:draft` (or `email:send`). - `POST /api/codex/emails/send` — same body. Requires `email:send`. Never send without explicit user instruction. -## Cookbook serve (debug a failing model launch) - -The Cookbook surface lets you reproduce what a human would do in Odysseus → Cookbook: read which serves are running, tail their tmux output to see why they crashed, edit the launch command, relaunch, kill a stuck one. Use this when the user is debugging a model server that won't come up (compute-capability errors, OOM, missing kernels, wrong attention backend, etc.). - -- `GET /api/codex/cookbook/tasks` — list active serve/download/install tasks (sessionId, type, status, repo_id, remoteHost, payload._cmd). Requires `cookbook:read`. -- `GET /api/codex/cookbook/servers` — list configured servers (name, host, port, env type + path, model dirs). Requires `cookbook:read`. -- `GET /api/codex/cookbook/cached?host=` — list models already cached on the named server (HF cache + Ollama + extra modelDirs). Call BEFORE `serve` to see what's already on disk. Requires `cookbook:read`. -- `GET /api/codex/cookbook/presets` — list saved serve presets (model + host + port + cmd). The user's saved preset usually has a working cmd — try `preset NAME` before composing your own. Requires `cookbook:read`. -- `GET /api/codex/cookbook/output/{session_id}?tail=400` — read the last N lines of the task's persistent log file (preferred) or tmux pane (fallback). The log file persists across vllm crashes, so this returns the actual Python traceback even after the bash prompt + neofetch banner overwrites the pane. Default tail=400. Requires `cookbook:read`. -- `POST /api/codex/cookbook/serve` — launch a serve task. Body matches `ServeRequest`: `{ repo_id, cmd, remote_host?, ssh_port?, env_prefix?, gpus?, platform? }`. The `cmd` is validated: leading binary must be `vllm`/`python3`/`sglang`/`llama-server`/`ollama`/`node`/`npx`. NEVER prefix with `cd …`, `source …`, or chain with `&&`/`||`/`;`/`$(...)` — the validator rejects shell metacharacters. The venv activation (`env_prefix`) is added automatically from the host's saved settings, so pass the bare binary + args. Requires `cookbook:launch`. -- `POST /api/codex/cookbook/preset/{name}` — launch a saved preset by name. Reuses the working cmd + host the user already saved. Requires `cookbook:launch`. -- `POST /api/codex/cookbook/adopt` — register an externally-launched tmux session into cookbook tracking. Body: `{ tmux_session, model, host?, port? }`. Use this when serve_model rejected a cmd and you fell back to direct ssh+tmux — without adoption, the session is invisible to the UI. Requires `cookbook:launch`. -- `POST /api/codex/cookbook/stop/{session_id}` — kill the tmux session for that task. Requires `cookbook:launch`. - -```bash -# Survey what's running -python3 ~/.claude/skills/odysseus/scripts/odysseus_api.py cookbook tasks - -# Tail the failing one (sessionId from `cookbook tasks`) -python3 ~/.claude/skills/odysseus/scripts/odysseus_api.py cookbook output serve-abc12345 400 - -# Stop the previous attempt before you try a new flag set -python3 ~/.claude/skills/odysseus/scripts/odysseus_api.py cookbook stop serve-abc12345 - -# Relaunch with new flags. cmd MUST begin with one of the allowlisted binaries. -python3 ~/.claude/skills/odysseus/scripts/odysseus_api.py cookbook serve \ - /mnt/HADES/models/Qwen3.5-397B-A17B-AWQ \ - "vllm serve /mnt/HADES/models/Qwen3.5-397B-A17B-AWQ --host 0.0.0.0 --port 8001 --tensor-parallel-size 8 --max-model-len 262144 --gpu-memory-utilization 0.90 --dtype auto --max-num-seqs 8 --trust-remote-code --enable-expert-parallel --enable-auto-tool-choice --tool-call-parser qwen3_coder --reasoning-parser qwen3" \ - pewds@192.168.1.12 -``` - -**Debug loop pattern:** when a serve is failing, the productive sequence is - -1. `cookbook tasks` → find the failing sessionId. -2. `cookbook output SID 600` → read the last 600 lines, find the actual root-cause line (often above the visible tail because tmux scrollback rolled — request a larger `tail` if the error references "above"). -3. `cookbook stop SID` — kill the previous attempt before relaunching; two serves on the same `--port` collide. -4. `cookbook serve repo "new cmd"` — try the next variation. Wait ~20s, then `cookbook output` on the new sessionId. - -**Hard limits this surface enforces:** -- `cookbook serve` cmd allowlist + shell-metacharacter rejection — you cannot run arbitrary shell, only model-server binaries. -- `cookbook stop` only targets task sessionIds matching `[a-zA-Z0-9_-]+`. -- The agent CAN spawn GPU-pinning long-lived processes — always `cookbook stop` your previous attempt before relaunching, and check `cookbook tasks` for collisions on the same `--port` before launching. - ## Forbidden Bypass Pattern If you are about to reach the Odysseus host/container, import app internals, query the database, or call MCP helper modules directly, stop. Those paths bypass Odysseus Settings and token scopes. Ask the user to enable the relevant Claude Agent tool toggle instead. diff --git a/integrations/claude/skills/odysseus/scripts/odysseus_api.py b/integrations/claude/skills/odysseus/scripts/odysseus_api.py index 8a22eb494..6fa324011 100755 --- a/integrations/claude/skills/odysseus/scripts/odysseus_api.py +++ b/integrations/claude/skills/odysseus/scripts/odysseus_api.py @@ -22,15 +22,6 @@ def _usage() -> int: print(" odysseus_api.py documents read DOC_ID", file=sys.stderr) print(" odysseus_api.py documents create JSON_PAYLOAD", file=sys.stderr) print(" odysseus_api.py documents delete DOC_ID", file=sys.stderr) - print(" odysseus_api.py cookbook tasks", file=sys.stderr) - print(" odysseus_api.py cookbook servers", file=sys.stderr) - print(" odysseus_api.py cookbook cached [HOST]", file=sys.stderr) - print(" odysseus_api.py cookbook presets", file=sys.stderr) - print(" odysseus_api.py cookbook output SESSION_ID [tail]", file=sys.stderr) - print(" odysseus_api.py cookbook serve REPO_ID 'CMD' [REMOTE_HOST]", file=sys.stderr) - print(" odysseus_api.py cookbook preset NAME", file=sys.stderr) - print(" odysseus_api.py cookbook adopt SESSION_ID MODEL [HOST] [PORT]", file=sys.stderr) - print(" odysseus_api.py cookbook stop SESSION_ID", file=sys.stderr) print(" odysseus_api.py METHOD /api/codex/path [json-body]", file=sys.stderr) return 2 @@ -113,61 +104,6 @@ def main() -> int: body = None else: return _usage() - elif command == "cookbook": - if len(sys.argv) < 3: - return _usage() - action = sys.argv[2].lower() - if action == "tasks": - method = "GET" - path = "/api/codex/cookbook/tasks" - body = None - elif action == "servers": - method = "GET" - path = "/api/codex/cookbook/servers" - body = None - elif action == "output" and len(sys.argv) >= 4: - method = "GET" - sid = sys.argv[3] - tail = sys.argv[4] if len(sys.argv) >= 5 else "400" - path = f"/api/codex/cookbook/output/{sid}?tail={tail}" - body = None - elif action == "cached": - method = "GET" - if len(sys.argv) >= 4: - from urllib.parse import quote - path = f"/api/codex/cookbook/cached?host={quote(sys.argv[3])}" - else: - path = "/api/codex/cookbook/cached" - body = None - elif action == "presets": - method = "GET" - path = "/api/codex/cookbook/presets" - body = None - elif action == "preset" and len(sys.argv) >= 4: - from urllib.parse import quote - method = "POST" - path = f"/api/codex/cookbook/preset/{quote(sys.argv[3])}" - body = None - elif action == "adopt" and len(sys.argv) >= 5: - method = "POST" - path = "/api/codex/cookbook/adopt" - payload = {"tmux_session": sys.argv[3], "model": sys.argv[4]} - if len(sys.argv) >= 6: payload["host"] = sys.argv[5] - if len(sys.argv) >= 7: payload["port"] = int(sys.argv[6]) - body = json.dumps(payload) - elif action == "serve" and len(sys.argv) >= 5: - method = "POST" - path = "/api/codex/cookbook/serve" - payload = {"repo_id": sys.argv[3], "cmd": sys.argv[4]} - if len(sys.argv) >= 6: - payload["remote_host"] = sys.argv[5] - body = json.dumps(payload) - elif action == "stop" and len(sys.argv) >= 4: - method = "POST" - path = f"/api/codex/cookbook/stop/{sys.argv[3]}" - body = None - else: - return _usage() else: if len(sys.argv) < 3: return _usage() diff --git a/integrations/codex/scripts/odysseus_api.py b/integrations/codex/scripts/odysseus_api.py index 8a22eb494..6fa324011 100755 --- a/integrations/codex/scripts/odysseus_api.py +++ b/integrations/codex/scripts/odysseus_api.py @@ -22,15 +22,6 @@ def _usage() -> int: print(" odysseus_api.py documents read DOC_ID", file=sys.stderr) print(" odysseus_api.py documents create JSON_PAYLOAD", file=sys.stderr) print(" odysseus_api.py documents delete DOC_ID", file=sys.stderr) - print(" odysseus_api.py cookbook tasks", file=sys.stderr) - print(" odysseus_api.py cookbook servers", file=sys.stderr) - print(" odysseus_api.py cookbook cached [HOST]", file=sys.stderr) - print(" odysseus_api.py cookbook presets", file=sys.stderr) - print(" odysseus_api.py cookbook output SESSION_ID [tail]", file=sys.stderr) - print(" odysseus_api.py cookbook serve REPO_ID 'CMD' [REMOTE_HOST]", file=sys.stderr) - print(" odysseus_api.py cookbook preset NAME", file=sys.stderr) - print(" odysseus_api.py cookbook adopt SESSION_ID MODEL [HOST] [PORT]", file=sys.stderr) - print(" odysseus_api.py cookbook stop SESSION_ID", file=sys.stderr) print(" odysseus_api.py METHOD /api/codex/path [json-body]", file=sys.stderr) return 2 @@ -113,61 +104,6 @@ def main() -> int: body = None else: return _usage() - elif command == "cookbook": - if len(sys.argv) < 3: - return _usage() - action = sys.argv[2].lower() - if action == "tasks": - method = "GET" - path = "/api/codex/cookbook/tasks" - body = None - elif action == "servers": - method = "GET" - path = "/api/codex/cookbook/servers" - body = None - elif action == "output" and len(sys.argv) >= 4: - method = "GET" - sid = sys.argv[3] - tail = sys.argv[4] if len(sys.argv) >= 5 else "400" - path = f"/api/codex/cookbook/output/{sid}?tail={tail}" - body = None - elif action == "cached": - method = "GET" - if len(sys.argv) >= 4: - from urllib.parse import quote - path = f"/api/codex/cookbook/cached?host={quote(sys.argv[3])}" - else: - path = "/api/codex/cookbook/cached" - body = None - elif action == "presets": - method = "GET" - path = "/api/codex/cookbook/presets" - body = None - elif action == "preset" and len(sys.argv) >= 4: - from urllib.parse import quote - method = "POST" - path = f"/api/codex/cookbook/preset/{quote(sys.argv[3])}" - body = None - elif action == "adopt" and len(sys.argv) >= 5: - method = "POST" - path = "/api/codex/cookbook/adopt" - payload = {"tmux_session": sys.argv[3], "model": sys.argv[4]} - if len(sys.argv) >= 6: payload["host"] = sys.argv[5] - if len(sys.argv) >= 7: payload["port"] = int(sys.argv[6]) - body = json.dumps(payload) - elif action == "serve" and len(sys.argv) >= 5: - method = "POST" - path = "/api/codex/cookbook/serve" - payload = {"repo_id": sys.argv[3], "cmd": sys.argv[4]} - if len(sys.argv) >= 6: - payload["remote_host"] = sys.argv[5] - body = json.dumps(payload) - elif action == "stop" and len(sys.argv) >= 4: - method = "POST" - path = f"/api/codex/cookbook/stop/{sys.argv[3]}" - body = None - else: - return _usage() else: if len(sys.argv) < 3: return _usage() diff --git a/integrations/codex/skills/odysseus/SKILL.md b/integrations/codex/skills/odysseus/SKILL.md index d4cbdf726..cbb83815e 100644 --- a/integrations/codex/skills/odysseus/SKILL.md +++ b/integrations/codex/skills/odysseus/SKILL.md @@ -1,6 +1,6 @@ --- name: odysseus -description: Use when the user asks Codex to read or write Odysseus data (todos, email, calendar, memory, documents) or to launch/monitor/stop a Cookbook model-serve task through the scoped Codex Agent API. Requires ODYSSEUS_URL and ODYSSEUS_API_TOKEN. +description: Use when the user asks Codex to read or write Odysseus data (todos, email, calendar, memory, documents) through the scoped Codex Agent API. Requires ODYSSEUS_URL and ODYSSEUS_API_TOKEN. --- # Odysseus @@ -34,6 +34,7 @@ If the user says "reminder" + a time, default to TODO with due_date. Only switch - Do not call helpers like `do_manage_notes`, email MCP internals, or database sessions directly for user data, even if shell access exists. - Never send email directly unless the user explicitly asks to send and the token has a send-capable scope. - Keep actions scoped to the token owner. +- Cookbook/model deployment is intentionally operator-controlled in the Odysseus UI and unavailable to bearer tokens; agents should consume the user-configured inference endpoint rather than attempt `/api/codex/cookbook/*`. ## Todos @@ -106,37 +107,6 @@ python3 integrations/codex/scripts/odysseus_api.py POST /api/codex/memory '{"tex - `POST /api/codex/emails/draft` — body matches `SendEmailRequest` (`to`, `cc`, `bcc`, `subject`, `body`, `body_html`, `attachments`, `account_id`, `in_reply_to`, `references`). Requires `email:draft` (or `email:send`). - `POST /api/codex/emails/send` — same body. Requires `email:send`. Never send without explicit user instruction. -## Cookbook serve (debug a failing model launch) - -The Cookbook surface lets you reproduce what a human would do in Odysseus → Cookbook: read which serves are running, tail their tmux output to see why they crashed, edit the launch command, relaunch, kill a stuck one. Use this when the user is debugging a model server that won't come up (compute-capability errors, OOM, missing kernels, wrong attention backend, etc.). - -- `GET /api/codex/cookbook/tasks` — list active serve/download/install tasks (sessionId, type, status, repo_id, remoteHost, payload._cmd). Requires `cookbook:read`. -- `GET /api/codex/cookbook/servers` — list configured servers (name, host, port, env type + path, model dirs). Requires `cookbook:read`. -- `GET /api/codex/cookbook/cached?host=` — list models already cached on the named server (HF cache + Ollama + extra modelDirs). Call BEFORE `serve` to see what's already on disk. Requires `cookbook:read`. -- `GET /api/codex/cookbook/presets` — list saved serve presets (model + host + port + cmd). The user's saved preset usually has a working cmd — try `preset NAME` before composing your own. Requires `cookbook:read`. -- `GET /api/codex/cookbook/output/{session_id}?tail=400` — read the last N lines of the task's persistent log file (preferred) or tmux pane (fallback). The log file persists across vllm crashes, so this returns the actual Python traceback even after the bash prompt + neofetch banner overwrites the pane. Default tail=400. Requires `cookbook:read`. -- `POST /api/codex/cookbook/serve` — launch a serve task. Body matches `ServeRequest`: `{ repo_id, cmd, remote_host?, ssh_port?, env_prefix?, gpus?, platform? }`. The `cmd` is validated: leading binary must be `vllm`/`python3`/`sglang`/`llama-server`/`ollama`/`node`/`npx`. NEVER prefix with `cd …`, `source …`, or chain with `&&`/`||`/`;`/`$(...)` — the validator rejects shell metacharacters. The venv activation (`env_prefix`) is added automatically from the host's saved settings, so pass the bare binary + args. Requires `cookbook:launch`. -- `POST /api/codex/cookbook/preset/{name}` — launch a saved preset by name. Reuses the working cmd + host the user already saved. Requires `cookbook:launch`. -- `POST /api/codex/cookbook/adopt` — register an externally-launched tmux session into cookbook tracking. Body: `{ tmux_session, model, host?, port? }`. Use this when serve_model rejected a cmd and you fell back to direct ssh+tmux — without adoption, the session is invisible to the UI. Requires `cookbook:launch`. -- `POST /api/codex/cookbook/stop/{session_id}` — kill the tmux session. Requires `cookbook:launch`. - -```bash -python3 ~/plugins/odysseus/scripts/odysseus_api.py cookbook tasks -python3 ~/plugins/odysseus/scripts/odysseus_api.py cookbook output serve-abc12345 400 -python3 ~/plugins/odysseus/scripts/odysseus_api.py cookbook stop serve-abc12345 -python3 ~/plugins/odysseus/scripts/odysseus_api.py cookbook serve \ - /mnt/HADES/models/Qwen3.5-397B-A17B-AWQ \ - "vllm serve /mnt/HADES/models/Qwen3.5-397B-A17B-AWQ --host 0.0.0.0 --port 8001 --tensor-parallel-size 8 --max-model-len 262144 --gpu-memory-utilization 0.90 --dtype auto --max-num-seqs 8 --trust-remote-code --enable-expert-parallel --enable-auto-tool-choice --tool-call-parser qwen3_coder --reasoning-parser qwen3" \ - pewds@192.168.1.12 -``` - -**Debug loop pattern:** `tasks` → `output SID 600` (find root cause; request larger `tail` if it references "above") → `stop SID` → `serve repo "new cmd"` → wait ~20s → `output` on the new sessionId. - -**Hard limits this surface enforces:** -- `cookbook serve` cmd allowlist + shell-metacharacter rejection. -- `cookbook stop` requires sessionIds matching `[a-zA-Z0-9_-]+`. -- Agent CAN spawn GPU-pinning long-lived processes — always `cookbook stop` your previous attempt before relaunching. - ## Forbidden Bypass Pattern If you are about to reach the Odysseus host/container, import app internals, query the database, or call MCP helper modules directly, stop. Those paths bypass Odysseus Settings and token scopes. Ask the user to enable the relevant Codex Agent tool toggle instead. diff --git a/routes/api_token_routes.py b/routes/api_token_routes.py index cbc828731..9ea178d56 100644 --- a/routes/api_token_routes.py +++ b/routes/api_token_routes.py @@ -25,8 +25,6 @@ "calendar:write", "memory:read", "memory:write", - "cookbook:read", - "cookbook:launch", } TOKEN_PROFILES = { "chat": ["chat"], @@ -68,7 +66,6 @@ def ensure_before(write_scope: str, read_scope: str): ensure_before("calendar:write", "calendar:read") ensure_before("memory:write", "memory:read") ensure_before("email:draft", "email:read") - ensure_before("cookbook:launch", "cookbook:read") return normalized or [DEFAULT_SCOPES] diff --git a/routes/codex_routes.py b/routes/codex_routes.py index 9fe36a822..953d7f839 100644 --- a/routes/codex_routes.py +++ b/routes/codex_routes.py @@ -12,18 +12,16 @@ from pathlib import Path from typing import Any -from fastapi import APIRouter, BackgroundTasks, Body, HTTPException, Request +from fastapi import APIRouter, BackgroundTasks, Body, Depends, HTTPException, Request from fastapi.responses import StreamingResponse -from core.middleware import require_admin +from core.middleware import require_admin, require_codex_cookbook_browser from src.auth_helpers import require_authenticated_request, require_user from src.tool_implementations import do_manage_notes from src.constants import COOKBOOK_STATE_FILE from routes._validators import validate_remote_host, validate_ssh_port -COOKBOOK_READ_SCOPES = {"cookbook:read", "cookbook:launch"} -COOKBOOK_LAUNCH_SCOPES = {"cookbook:launch"} TODO_READ_SCOPES = {"todos:read", "todos:write"} TODO_WRITE_SCOPES = {"todos:write"} EMAIL_READ_SCOPES = {"email:read", "email:draft", "email:send"} @@ -35,6 +33,20 @@ CALENDAR_WRITE_SCOPES = {"calendar:write"} DOCS_READ_SCOPES = {"documents:read", "documents:write"} DOCS_WRITE_SCOPES = {"documents:write"} +CODEX_CAPABILITY_SCOPES = set().union( + {"chat"}, + TODO_READ_SCOPES, + TODO_WRITE_SCOPES, + EMAIL_READ_SCOPES, + EMAIL_DRAFT_SCOPES, + EMAIL_SEND_SCOPES, + MEMORY_READ_SCOPES, + MEMORY_WRITE_SCOPES, + CALENDAR_READ_SCOPES, + CALENDAR_WRITE_SCOPES, + DOCS_READ_SCOPES, + DOCS_WRITE_SCOPES, +) WRITE_ACTIONS = {"add", "create", "new", "save", "remind", "update", "delete", "toggle_item", "remove", "remove_item"} @@ -110,18 +122,19 @@ def _scope_owner_all(request: Request, required: set[str]) -> str: return require_user(request) -def _require_cookbook_scope(request: Request, allowed: set[str]) -> str: - """Authorize a Codex cookbook route. +def _require_cookbook_admin(request: Request) -> None: + """Keep the duplicate Codex Cookbook surface browser-admin only. - For API-token callers, enforce the given scope set. - For cookie-session callers, additionally require admin privileges - because cookbook surfaces expose host topology, task logs, tmux - commands, and model-serving controls. + API tokens and internal-tool loopback identities are deliberately denied + with the same response before a route body, Cookbook state, credentials, + filesystem, network, process, or endpoint operation is evaluated. Trusted + in-app Cookbook/model operations use their canonical route families. """ - owner = _scope_owner(request, allowed) - if not getattr(request.state, "api_token", False): - require_admin(request) - return owner + require_codex_cookbook_browser(request) + require_admin(request) + + +COOKBOOK_ROUTE_DEPENDENCIES = (Depends(_require_cookbook_admin),) def _find_endpoint(router: APIRouter | None, method: str, path: str): @@ -166,7 +179,10 @@ def setup_codex_routes( @router.get("/capabilities") def capabilities(request: Request): - token_scopes = set(getattr(request.state, "api_token_scopes", []) or []) + token_scopes = ( + set(getattr(request.state, "api_token_scopes", []) or []) + & CODEX_CAPABILITY_SCOPES + ) has_token = bool(getattr(request.state, "api_token", False)) def scoped(allowed): return bool(token_scopes.intersection(allowed)) if has_token else True @@ -203,11 +219,6 @@ def scoped(allowed): "actions": ["library", "read", "create", "delete"], "available": documents_library_endpoint is not None, }, - "cookbook": { - "read": scoped(COOKBOOK_READ_SCOPES), - "launch": scoped(COOKBOOK_LAUNCH_SCOPES), - "actions": ["tasks", "servers", "output", "serve", "stop"], - }, }, "safety": { "email_send_requires_confirmation": True, @@ -512,16 +523,12 @@ async def codex_documents_create(request: Request, body: dict[str, Any] = Body(d raise HTTPException(400, f"Invalid document payload: {exc}") return await _as_owner(request, owner, documents_create_endpoint, request, req) - # ── Cookbook surface ── - # Lets the agent run the same launch / monitor / kill loop the user - # would do by hand in the Cookbook UI: read the current task list + - # tmux output, launch a serve task, stop one. Two scopes: - # cookbook:read — list tasks + tail output + list servers - # cookbook:launch — also start/stop serves (host shell exec) - # `cookbook:launch` is genuinely powerful: /api/model/serve runs SSH'd - # commands on the user's hosts. The existing _validate_serve_cmd - # allowlist (vllm/python3/sglang/llama-server/etc., no shell metachars) - # keeps the agent inside the same sandbox the UI uses. + # ── Browser-admin compatibility surface ── + # This duplicate route family is retained for cookie-session compatibility. + # The outer middleware rejects API-token and internal-tool callers before + # body parsing; the router dependency and first handler call are backstops. + # Canonical in-app Cookbook and model operations continue to use + # /api/cookbook and /api/model. async def _run_shell(cmd: str, timeout: float = 15.0) -> dict: """Run a shell command, return {exit_code, stdout, stderr}.""" @@ -565,16 +572,22 @@ def _redact_task(t: dict) -> dict: if k not in ("hf_token", "_secrets")} return clean - @router.get("/cookbook/tasks") + @router.get( + "/cookbook/tasks", + dependencies=COOKBOOK_ROUTE_DEPENDENCIES, + ) async def codex_cookbook_tasks(request: Request): - _require_cookbook_scope(request, COOKBOOK_READ_SCOPES) + _require_cookbook_admin(request) state = _read_cookbook_state() tasks = state.get("tasks") or [] return {"tasks": [_redact_task(t) for t in tasks]} - @router.get("/cookbook/servers") + @router.get( + "/cookbook/servers", + dependencies=COOKBOOK_ROUTE_DEPENDENCIES, + ) async def codex_cookbook_servers(request: Request): - _require_cookbook_scope(request, COOKBOOK_READ_SCOPES) + _require_cookbook_admin(request) state = _read_cookbook_state() servers = state.get("env", {}).get("servers") or [] # Strip ssh creds / passwords; keep only what's needed to pick a host. @@ -591,9 +604,12 @@ async def codex_cookbook_servers(request: Request): }) return {"servers": cleaned} - @router.get("/cookbook/output/{session_id}") + @router.get( + "/cookbook/output/{session_id}", + dependencies=COOKBOOK_ROUTE_DEPENDENCIES, + ) async def codex_cookbook_output(request: Request, session_id: str, tail: int = 400): - _require_cookbook_scope(request, COOKBOOK_READ_SCOPES) + _require_cookbook_admin(request) # Defensive: session_id must be the tmux-style id we issue # (`serve-XXXX` / `cookbook-XXXX` / `queue-XXXX`); anything else # would let the agent run arbitrary `tmux capture-pane` targets. @@ -633,9 +649,12 @@ async def codex_cookbook_output(request: Request, session_id: str, tail: int = 4 "task": _redact_task(task), } - @router.post("/cookbook/serve") + @router.post( + "/cookbook/serve", + dependencies=COOKBOOK_ROUTE_DEPENDENCIES, + ) async def codex_cookbook_serve(request: Request, body: dict[str, Any] = Body(default_factory=dict)): - _require_cookbook_scope(request, COOKBOOK_LAUNCH_SCOPES) + _require_cookbook_admin(request) # Wraps /api/model/serve with the SAME validation the UI uses. # _validate_serve_cmd (called inside model_serve) rejects shell # metachars and requires the leading binary to be in the @@ -672,9 +691,12 @@ async def codex_cookbook_serve(request: Request, body: dict[str, Any] = Body(def raise HTTPException(503, "model serve endpoint unavailable") return await serve_endpoint(request, req) - @router.post("/cookbook/stop/{session_id}") + @router.post( + "/cookbook/stop/{session_id}", + dependencies=COOKBOOK_ROUTE_DEPENDENCIES, + ) async def codex_cookbook_stop(request: Request, session_id: str): - _require_cookbook_scope(request, COOKBOOK_LAUNCH_SCOPES) + _require_cookbook_admin(request) import re as _re if not _re.fullmatch(r"[a-zA-Z0-9_-]+", session_id): raise HTTPException(400, "Invalid session id") @@ -689,12 +711,15 @@ async def codex_cookbook_stop(request: Request, session_id: str): result = await _run_shell(cmd, timeout=10) return {"session_id": session_id, "exit_code": result.get("exit_code"), "host": host or "local"} - @router.get("/cookbook/cached") + @router.get( + "/cookbook/cached", + dependencies=COOKBOOK_ROUTE_DEPENDENCIES, + ) async def codex_cookbook_cached(request: Request, host: str | None = None): """List cached models on a configured server (or local if host is omitted). Mirrors `list_cached_models` from the chat agent so external agents have the same inventory view before deciding what to serve/download.""" - _require_cookbook_scope(request, COOKBOOK_READ_SCOPES) + _require_cookbook_admin(request) # Hit /api/model/cached internally, with the same modelDirs the chat # agent's list_cached_models would resolve from cookbook state. state = _read_cookbook_state() @@ -751,12 +776,15 @@ def _dirs_for(srv: dict) -> str: platform=params.get("platform") or None, ) - @router.get("/cookbook/presets") + @router.get( + "/cookbook/presets", + dependencies=COOKBOOK_ROUTE_DEPENDENCIES, + ) async def codex_cookbook_presets(request: Request): """List saved serve presets (model + host + port + launch cmd). Counterpart to `list_serve_presets`. Use BEFORE composing a `serve` body — the user's saved preset usually has the working cmd already.""" - _require_cookbook_scope(request, COOKBOOK_READ_SCOPES) + _require_cookbook_admin(request) state = _read_cookbook_state() presets = state.get("presets") or [] out = [] @@ -772,11 +800,14 @@ async def codex_cookbook_presets(request: Request): }) return {"presets": out, "default_host": (state.get("env") or {}).get("defaultServer", "")} - @router.post("/cookbook/preset/{name}") + @router.post( + "/cookbook/preset/{name}", + dependencies=COOKBOOK_ROUTE_DEPENDENCIES, + ) async def codex_cookbook_serve_preset(request: Request, name: str): """Launch a saved preset by name. Reuses the working cmd + host the user already saved, avoiding the cmd-allowlist trial-and-error loop.""" - _require_cookbook_scope(request, COOKBOOK_LAUNCH_SCOPES) + _require_cookbook_admin(request) import re as _re if not _re.fullmatch(r"[A-Za-z0-9 _.:@\-]+", name): raise HTTPException(400, "Invalid preset name") @@ -822,13 +853,16 @@ async def codex_cookbook_serve_preset(request: Request, name: str): raise HTTPException(503, "model serve endpoint unavailable") return await serve_endpoint(request, req) - @router.post("/cookbook/adopt") + @router.post( + "/cookbook/adopt", + dependencies=COOKBOOK_ROUTE_DEPENDENCIES, + ) async def codex_cookbook_adopt(request: Request, body: dict[str, Any] = Body(default_factory=dict)): """Adopt an existing tmux session (one started via raw ssh+tmux) into cookbook tracking. Needed when serve_model rejects a cmd and the agent falls back to direct ssh — without adoption the session is invisible to the UI. Body: {tmux_session, model, host?, port?}.""" - _require_cookbook_scope(request, COOKBOOK_LAUNCH_SCOPES) + _require_cookbook_admin(request) norm = dict(body or {}) sess = (norm.get("tmux_session") or norm.get("session_id") or "").strip() model = (norm.get("model") or norm.get("repo_id") or "").strip() diff --git a/static/js/admin.js b/static/js/admin.js index 6162708fd..c696788e3 100644 --- a/static/js/admin.js +++ b/static/js/admin.js @@ -2477,8 +2477,6 @@ const _TOKEN_SCOPES = [ { key: 'calendar:write', label: 'Calendar write', detail: 'Create and update calendar events' }, { key: 'memory:read', label: 'Memory read', detail: 'Read memory when enabled' }, { key: 'memory:write', label: 'Memory write', detail: 'Write memory when enabled' }, - { key: 'cookbook:read', label: 'Cookbook read', detail: 'List cookbook tasks + tail their tmux output' }, - { key: 'cookbook:launch', label: 'Cookbook launch', detail: 'Launch and stop cookbook serve tasks' }, ]; function _renderTokenScopeRows(t) { diff --git a/static/js/settings.js b/static/js/settings.js index 72936adee..a4c34792d 100644 --- a/static/js/settings.js +++ b/static/js/settings.js @@ -5242,8 +5242,6 @@ async function initUnifiedIntegrations() { { key: 'calendar:write', label: 'Calendar write', detail: 'Create and update calendar events' }, { key: 'memory:read', label: 'Memory', detail: 'Read memory when enabled' }, { key: 'memory:write', label: 'Memory write', detail: 'Write memory when enabled' }, - { key: 'cookbook:read', label: 'Cookbook', detail: 'List cookbook tasks + tail their tmux output (debug a model serve from outside the UI)' }, - { key: 'cookbook:launch', label: 'Cookbook launch', detail: 'Launch and stop cookbook serve tasks. Powerful: runs SSH commands on your configured servers, bounded by the same allowlist the UI uses (vllm/python3/sglang/llama-server/...)' }, ]; // Strict name-prefix match keeps Codex and Claude tokens in their own forms. const agentTokens = (Array.isArray(tokens) ? tokens : []).filter(tok => @@ -5256,7 +5254,6 @@ async function initUnifiedIntegrations() { email: '', calendar: '', memory: '', - cookbook: '', }; const _scopeNiceLabel = (label) => label.replace(/\s+(write|drafts?|send)$/i, ''); const _scopeAction = (key) => (key.split(':')[1] || '').toLowerCase(); diff --git a/tests/test_api_token_routes.py b/tests/test_api_token_routes.py index 40afc2226..4440160ce 100644 --- a/tests/test_api_token_routes.py +++ b/tests/test_api_token_routes.py @@ -192,7 +192,12 @@ def __init__(self, **kw): invalidator.assert_called_once() -def test_create_token_accepts_cookbook_read_scope(monkeypatch, token_routes_mod): +@pytest.mark.parametrize("retired_scope", ["cookbook:read", "cookbook:launch"]) +def test_create_token_rejects_retired_cookbook_scopes( + monkeypatch, + token_routes_mod, + retired_scope, +): monkeypatch.setenv("AUTH_ENABLED", "true") mod = token_routes_mod @@ -202,24 +207,13 @@ def test_create_token_accepts_cookbook_read_scope(monkeypatch, token_routes_mod) req = _req("alice", is_admin=True) create_token = _get_handler(mod, "POST", "/tokens") - resp = create_token(request=req, name="cookbook-reader", scopes="cookbook:read") - assert resp["scopes"] == ["cookbook:read"] - - -def test_cookbook_launch_scope_implies_read(monkeypatch, token_routes_mod): - monkeypatch.setenv("AUTH_ENABLED", "true") - mod = token_routes_mod - - fake_session = MagicMock() - monkeypatch.setattr(mod, "get_db_session", lambda: _db_ctx(fake_session)) - monkeypatch.setattr(mod, "get_current_user", lambda req: req.state.current_user) - - req = _req("alice", is_admin=True) - create_token = _get_handler(mod, "POST", "/tokens") - resp = create_token(request=req, name="cookbook-launcher", scopes="cookbook:launch") + with pytest.raises(HTTPException) as exc: + create_token(request=req, name="retired-scope", scopes=retired_scope) - assert resp["scopes"] == ["cookbook:read", "cookbook:launch"] + assert exc.value.status_code == 400 + assert exc.value.detail == f"Unknown token scope: {retired_scope}" + fake_session.add.assert_not_called() # --------------------------------------------------------------------------- @@ -276,6 +270,50 @@ def test_list_tokens_returns_safe_display_fields_only(monkeypatch, token_routes_ assert result[1]["scopes"] == ["chat"] +def test_legacy_cookbook_scope_rows_are_listed_without_being_rewritten( + monkeypatch, + token_routes_mod, +): + """Removing a scope must not migrate or silently edit existing DB rows.""" + monkeypatch.setenv("AUTH_ENABLED", "true") + mod = token_routes_mod + row = SimpleNamespace( + id="legacy1", + name="Legacy", + owner="alice", + token_prefix="ody_lega", + token_hash="$2b$12$NOTRETURNED", + scopes="cookbook:read,cookbook:launch", + is_active=True, + last_used_at=None, + created_at=None, + ) + fake_session = MagicMock() + fake_session.query.return_value.all.return_value = [row] + monkeypatch.setattr(mod, "get_db_session", lambda: _db_ctx(fake_session)) + + list_tokens = _get_handler(mod, "GET", "/tokens") + result = list_tokens(request=_req("alice", is_admin=True)) + + assert result[0]["scopes"] == ["cookbook:read", "cookbook:launch"] + assert row.scopes == "cookbook:read,cookbook:launch" + fake_session.add.assert_not_called() + + +def test_token_profiles_do_not_advertise_retired_cookbook_scopes(token_routes_mod): + mod = token_routes_mod + token_profiles = _get_handler(mod, "GET", "/tokens/profiles") + + result = token_profiles(request=_req("alice", is_admin=True)) + + assert "cookbook:read" not in result["allowed_scopes"] + assert "cookbook:launch" not in result["allowed_scopes"] + assert all( + "cookbook:read" not in scopes and "cookbook:launch" not in scopes + for scopes in result["profiles"].values() + ) + + # --------------------------------------------------------------------------- # 4. DELETE /api/tokens/{id} — found → deleted + cache invalidated # --------------------------------------------------------------------------- @@ -393,6 +431,37 @@ def test_update_token_applies_explicit_scopes(monkeypatch, token_routes_mod): assert resp["scopes"] == ["chat"] +@pytest.mark.parametrize("retired_scope", ["cookbook:read", "cookbook:launch"]) +def test_update_token_rejects_retired_cookbook_scopes_without_mutation( + monkeypatch, + token_routes_mod, + retired_scope, +): + monkeypatch.setenv("AUTH_ENABLED", "true") + mod = token_routes_mod + + token = SimpleNamespace( + id="tok123", name="original", owner="alice", + token_prefix="ody_orig", scopes="chat", is_active=True, + ) + fake_session = MagicMock() + fake_session.query.return_value.filter.return_value.first.return_value = token + monkeypatch.setattr(mod, "get_db_session", lambda: _db_ctx(fake_session)) + + invalidator = MagicMock() + req = _patch_request(invalidator, {"scopes": [retired_scope]}) + update_token = _get_handler(mod, "PATCH", "/tokens/{token_id}") + + with pytest.raises(HTTPException) as exc: + asyncio.run(update_token(request=req, token_id="tok123")) + + assert exc.value.status_code == 400 + assert exc.value.detail == f"Unknown token scope: {retired_scope}" + assert token.scopes == "chat" + fake_session.add.assert_not_called() + invalidator.assert_not_called() + + def test_update_missing_token_returns_404(monkeypatch, token_routes_mod): monkeypatch.setenv("AUTH_ENABLED", "true") mod = token_routes_mod diff --git a/tests/test_codex_cookbook_admin_gate.py b/tests/test_codex_cookbook_admin_gate.py index c267283f1..d2f9589b2 100644 --- a/tests/test_codex_cookbook_admin_gate.py +++ b/tests/test_codex_cookbook_admin_gate.py @@ -1,118 +1,609 @@ -"""Codex cookbook routes require admin for cookie-session callers. +"""Fail-closed boundary tests for the duplicate Codex Cookbook routes.""" -Regression test for issue #4542: non-admin users could reach cookbook -routes (tasks, servers, output, stop, adopt, presets, etc.) through -normal cookie sessions because _scope_owner only checked login status, -not admin privileges. +import ast +import asyncio +import json +from pathlib import Path +from types import SimpleNamespace -After the fix, cookie-session callers must be admin; API-token callers -are still governed by scope checks only. -""" import pytest -from types import SimpleNamespace -from fastapi import HTTPException +from fastapi import FastAPI, HTTPException, Request +from fastapi.middleware.cors import CORSMiddleware +from fastapi.testclient import TestClient +from starlette.middleware.base import BaseHTTPMiddleware + +import core.middleware as middleware +from core.middleware import ( + CodexCookbookBoundaryMiddleware, + INTERNAL_TOOL_HEADER, + INTERNAL_TOOL_TOKEN, + INTERNAL_TOOL_USER, + is_codex_cookbook_path, + is_odysseus_bearer_authorization, +) +import routes.codex_routes as codex_routes + + +COOKBOOK_ROUTES = [ + pytest.param("GET", "/api/codex/cookbook/tasks", None, id="tasks"), + pytest.param("GET", "/api/codex/cookbook/servers", None, id="servers"), + pytest.param( + "GET", + "/api/codex/cookbook/output/serve-test?tail=40", + None, + id="output", + ), + pytest.param("GET", "/api/codex/cookbook/cached?host=gpu", None, id="cached"), + pytest.param("GET", "/api/codex/cookbook/presets", None, id="presets"), + pytest.param( + "POST", + "/api/codex/cookbook/serve", + {"repo_id": "org/model", "cmd": "vllm serve org/model"}, + id="serve", + ), + pytest.param( + "POST", + "/api/codex/cookbook/preset/saved", + None, + id="preset", + ), + pytest.param( + "POST", + "/api/codex/cookbook/adopt", + {"tmux_session": "serve-test", "model": "org/model", "host": "gpu"}, + id="adopt", + ), + pytest.param( + "POST", + "/api/codex/cookbook/stop/serve-test", + None, + id="stop", + ), +] + +BLOCKED_IDENTITIES = [ + pytest.param({"x-test-identity": "api-token"}, id="api-token"), + pytest.param({"x-test-identity": "internal-user"}, id="internal-user"), + pytest.param( + {INTERNAL_TOOL_HEADER: INTERNAL_TOOL_TOKEN}, + id="internal-header", + ), +] + +PRE_BODY_CREDENTIALS = [ + pytest.param( + {"authorization": "Bearer ody_valid_format_token"}, + id="valid-format-bearer", + ), + pytest.param( + {"authorization": "bEaReR \t ody_x"}, + id="invalid-short-bearer", + ), + pytest.param( + {INTERNAL_TOOL_HEADER: INTERNAL_TOOL_TOKEN}, + id="internal-header", + ), +] -from routes.codex_routes import _require_cookbook_scope +_INTERNAL_HEADER_BYTES = INTERNAL_TOOL_HEADER.lower().encode("ascii") +_INTERNAL_TOKEN_BYTES = INTERNAL_TOOL_TOKEN.encode("utf-8") +DUPLICATE_PRE_BODY_HEADERS = [ + pytest.param( + [ + (b"authorization", b"Basic placeholder"), + (b"authorization", b"Bearer ody_second_value"), + ], + id="bearer-second", + ), + pytest.param( + [ + (b"authorization", b"Bearer ody_first_value"), + (b"authorization", b"Basic placeholder"), + ], + id="bearer-first", + ), + pytest.param( + [ + (_INTERNAL_HEADER_BYTES, b"invalid"), + (_INTERNAL_HEADER_BYTES, _INTERNAL_TOKEN_BYTES), + ], + id="internal-second", + ), + pytest.param( + [ + (_INTERNAL_HEADER_BYTES, _INTERNAL_TOKEN_BYTES), + (_INTERNAL_HEADER_BYTES, b"invalid"), + ], + id="internal-first", + ), + pytest.param( + [(b"authorization", b"Basic placeholder, Bearer ody_combined")], + id="bearer-proxy-combined", + ), + pytest.param( + [(_INTERNAL_HEADER_BYTES, b"invalid, " + _INTERNAL_TOKEN_BYTES)], + id="internal-proxy-combined", + ), + pytest.param( + [(_INTERNAL_HEADER_BYTES, b" " + _INTERNAL_TOKEN_BYTES)], + id="internal-leading-sp", + ), + pytest.param( + [(_INTERNAL_HEADER_BYTES, _INTERNAL_TOKEN_BYTES + b" ")], + id="internal-trailing-sp", + ), + pytest.param( + [(_INTERNAL_HEADER_BYTES, b" " + _INTERNAL_TOKEN_BYTES + b" ")], + id="internal-both-sp", + ), + pytest.param( + [(_INTERNAL_HEADER_BYTES, b"\t" + _INTERNAL_TOKEN_BYTES + b"\t")], + id="internal-both-htab", + ), + pytest.param( + [ + (_INTERNAL_HEADER_BYTES, b"\xff"), + (b"authorization", b"Bearer ody_after_obs_text"), + ], + id="non-ascii-before-bearer", + ), + pytest.param( + [ + (_INTERNAL_HEADER_BYTES, b"\xff"), + (_INTERNAL_HEADER_BYTES, _INTERNAL_TOKEN_BYTES), + ], + id="non-ascii-before-internal", + ), +] -COOKBOOK_READ_SCOPES = {"cookbook:read", "cookbook:launch"} -COOKBOOK_LAUNCH_SCOPES = {"cookbook:launch"} +class _PoisonPath: + def __fspath__(self): + raise AssertionError("Cookbook state must not be resolved before the gate") -def _cookie_request(*, current_user="bob", is_admin=False): - """Simulate a cookie-session request (no api_token).""" - auth_mgr = SimpleNamespace( +def _build_app(side_effects: list[str]) -> FastAPI: + app = FastAPI() + app.state.auth_manager = SimpleNamespace( is_configured=True, - is_admin=lambda user: is_admin and user == "bob", + is_admin=lambda username: username == "alice", ) - return SimpleNamespace( - state=SimpleNamespace( - current_user=current_user, - api_token=False, - ), - app=SimpleNamespace(state=SimpleNamespace(auth_manager=auth_mgr)), - headers={}, + + @app.middleware("http") + async def stamp_test_identity(request: Request, call_next): + identity = request.headers.get("x-test-identity") + if identity == "api-token": + request.state.current_user = "api" + request.state.api_token = True + request.state.api_token_owner = "alice" + # Legacy rows can still carry these retired strings. They grant no + # access and are intentionally not migrated by this change. + request.state.api_token_scopes = ["cookbook:read", "cookbook:launch"] + elif identity == "internal-user": + request.state.current_user = INTERNAL_TOOL_USER + request.state.api_token = False + else: + request.state.current_user = "alice" + request.state.api_token = False + return await call_next(request) + + # Registered last, matching app.py: this boundary is outermost and rejects + # raw Odysseus credentials before the inner auth/identity middleware. + app.add_middleware(CodexCookbookBoundaryMiddleware) + + @app.post("/api/model/serve") + async def model_serve_stub(request: Request, body: dict): + side_effects.append("model-serve") + return {"ok": True} + + @app.get("/api/model/cached") + async def model_cached_stub(request: Request): + side_effects.append("model-cached") + return {"models": []} + + app.include_router(codex_routes.setup_codex_routes()) + return app + + +@pytest.fixture +def blocked_client(monkeypatch): + side_effects: list[str] = [] + monkeypatch.setattr(codex_routes, "COOKBOOK_STATE_FILE", _PoisonPath()) + + async def unexpected_process(*args, **kwargs): + side_effects.append("process") + raise AssertionError("process launch must not happen before the gate") + + monkeypatch.setattr(asyncio, "create_subprocess_shell", unexpected_process) + + import core.atomic_io as atomic_io + import routes.cookbook_helpers as cookbook_helpers + + def unexpected_write(*args, **kwargs): + side_effects.append("state-write") + raise AssertionError("state write must not happen before the gate") + + def unexpected_serve_request(*args, **kwargs): + side_effects.append("serve-body") + raise AssertionError("serve body must not be evaluated before the gate") + + monkeypatch.setattr(atomic_io, "atomic_write_json", unexpected_write) + monkeypatch.setattr(cookbook_helpers, "ServeRequest", unexpected_serve_request) + + with TestClient(_build_app(side_effects)) as client: + yield client, side_effects + + +@pytest.mark.parametrize("headers", BLOCKED_IDENTITIES) +@pytest.mark.parametrize("method,path,body", COOKBOOK_ROUTES) +def test_duplicate_cookbook_routes_fail_closed_before_side_effects( + blocked_client, + headers, + method, + path, + body, +): + client, side_effects = blocked_client + response = client.request(method, path, headers=headers, json=body) + + assert response.status_code == 403 + assert response.json() == {"detail": "Forbidden"} + assert side_effects == [] + + +@pytest.mark.parametrize("headers", PRE_BODY_CREDENTIALS) +def test_gate_precedes_json_body_validation(blocked_client, headers): + client, side_effects = blocked_client + headers = {**headers, "content-type": "application/json"} + + response = client.post( + "/api/codex/cookbook/serve", + headers=headers, + content=b"{not-json", ) + assert response.status_code == 403 + assert response.json() == {"detail": "Forbidden"} + assert side_effects == [] -def _api_token_request(*, scopes=None, owner="alice"): - """Simulate an API-token request.""" - return SimpleNamespace( - state=SimpleNamespace( - current_user="api", - api_token=True, - api_token_scopes=scopes or [], - api_token_owner=owner, - ), - app=SimpleNamespace(state=SimpleNamespace(auth_manager=None)), + +@pytest.mark.parametrize("raw_headers", DUPLICATE_PRE_BODY_HEADERS) +def test_all_duplicate_and_obs_text_credentials_precede_body_validation( + blocked_client, + raw_headers, +): + client, side_effects = blocked_client + + response = client.post( + "/api/codex/cookbook/serve", + headers=[*raw_headers, (b"content-type", b"application/json")], + content=b"{not-json", + ) + + assert response.status_code == 403 + assert response.json() == {"detail": "Forbidden"} + assert side_effects == [] + + +@pytest.mark.parametrize( + "value", + [ + "Bearer ody_token", + "bearer ody_token", + "BEARER ody_token", + "BeArEr\tody_token", + " bearer \t ody_token ", + "Basic placeholder, Bearer ody_token", + ], +) +def test_odysseus_bearer_parser_accepts_scheme_case_and_sp_htab(value): + assert is_odysseus_bearer_authorization(value) is True + + +@pytest.mark.parametrize( + "value", + [None, "", "ody_token", "Basic ody_token", "Bearer", "Bearer other"], +) +def test_odysseus_bearer_parser_rejects_other_credentials(value): + assert is_odysseus_bearer_authorization(value) is False + + +def test_internal_header_match_preserves_an_exact_whitespace_token(monkeypatch): + configured_token = "\t configured token \t" + monkeypatch.setattr(middleware, "INTERNAL_TOOL_TOKEN", configured_token) + + assert middleware._internal_header_matches(configured_token) is True + + +def test_mounted_root_path_gate_precedes_json_validation(monkeypatch): + side_effects: list[str] = [] + monkeypatch.setattr(codex_routes, "COOKBOOK_STATE_FILE", _PoisonPath()) + child = _build_app(side_effects) + parent = FastAPI() + parent.mount("/odysseus", child) + + with TestClient(parent) as client: + response = client.post( + "/odysseus/api/codex/cookbook/serve", + headers={ + "authorization": "bEaReR ody_legacy", + "content-type": "application/json", + }, + content=b"{not-json", + ) + + assert response.status_code == 403 + assert response.json() == {"detail": "Forbidden"} + assert side_effects == [] + + +def test_auth_disabled_shape_rejects_raw_bearer_before_body_parsing(): + """The boundary remains installed when app.AuthMiddleware is absent.""" + app = FastAPI() + app.add_middleware(CodexCookbookBoundaryMiddleware) + + @app.post("/api/codex/cookbook/serve") + async def unreachable(body: dict): + return body + + with TestClient(app) as client: + response = client.post( + "/api/codex/cookbook/serve", + headers={ + "authorization": "BEARER \t ody_legacy", + "content-type": "application/json", + }, + content=b"{not-json", + ) + + assert response.status_code == 403 + assert response.json() == {"detail": "Forbidden"} + + +def test_localhost_bypass_stack_still_runs_outer_boundary_first(): + app = FastAPI() + + class LocalhostBypassMiddleware(BaseHTTPMiddleware): + async def dispatch(self, request, call_next): + # Matches app.AuthMiddleware's accepted loopback branch: continue + # without stamping an authenticated cookie/API principal. + return await call_next(request) + + app.add_middleware(LocalhostBypassMiddleware) + app.add_middleware(CodexCookbookBoundaryMiddleware) + + @app.post("/api/codex/cookbook/serve") + async def unreachable(body: dict): + return body + + with TestClient(app) as client: + response = client.post( + "/api/codex/cookbook/serve", + headers={ + "authorization": "bEaReR ody_legacy", + "content-type": "application/json", + }, + content=b"{not-json", + ) + + assert response.status_code == 403 + assert response.json() == {"detail": "Forbidden"} + + +@pytest.mark.parametrize( + "path,expected", + [ + ("/api/codex/cookbook", True), + ("/api/codex/cookbook/tasks", True), + ("/api/codex/cookbookish", False), + ("/api/codex/cookbooks/tasks", False), + ("/api/cookbook/state", False), + ("/api/model/serve", False), + ], +) +def test_boundary_path_match_is_exact(path, expected): + assert is_codex_cookbook_path(path) is expected + + +@pytest.mark.parametrize( + "path", + ["/api/cookbook/state", "/api/model/serve", "/api/codex/cookbookish"], +) +def test_boundary_does_not_intercept_canonical_or_neighbor_routes(path): + app = FastAPI() + app.add_middleware(CodexCookbookBoundaryMiddleware) + + @app.api_route(path, methods=["GET", "POST"]) + async def unaffected_route(): + return {"ok": True} + + with TestClient(app) as client: + response = client.post( + path, + headers={"authorization": "BeArEr\tody_token"}, + ) + + assert response.status_code == 200 + assert response.json() == {"ok": True} + + +def test_cors_preflight_reaches_cors_middleware(): + app = FastAPI() + app.add_middleware( + CORSMiddleware, + allow_origins=["https://client.example"], + allow_methods=["POST"], + allow_headers=["authorization", "content-type"], + ) + app.add_middleware(CodexCookbookBoundaryMiddleware) + + @app.post("/api/codex/cookbook/serve") + async def unused_route(body: dict): + return body + + with TestClient(app) as client: + response = client.options( + "/api/codex/cookbook/serve", + headers={ + "origin": "https://client.example", + "access-control-request-method": "POST", + "access-control-request-headers": "authorization,content-type", + }, + ) + + assert response.status_code == 200 + assert response.headers["access-control-allow-origin"] == "https://client.example" + + +def test_cookie_admin_retains_duplicate_read_access(monkeypatch, tmp_path): + state_path = tmp_path / "cookbook_state.json" + state_path.write_text( + json.dumps({"tasks": [{"sessionId": "serve-test", "status": "running"}]}), + encoding="utf-8", + ) + monkeypatch.setattr(codex_routes, "COOKBOOK_STATE_FILE", state_path) + side_effects: list[str] = [] + + with TestClient(_build_app(side_effects)) as client: + response = client.get("/api/codex/cookbook/tasks") + + assert response.status_code == 200 + assert response.json() == { + "tasks": [{"sessionId": "serve-test", "status": "running"}] + } + assert side_effects == [] + + +def test_cookie_admin_retains_duplicate_serve_compatibility(): + side_effects: list[str] = [] + + with TestClient(_build_app(side_effects)) as client: + response = client.post( + "/api/codex/cookbook/serve", + json={"repo_id": "org/model", "cmd": "vllm serve org/model"}, + ) + + assert response.status_code == 200 + assert response.json() == {"ok": True} + assert side_effects == ["model-serve"] + + +def test_non_admin_cookie_session_is_still_rejected(monkeypatch): + monkeypatch.setenv("AUTH_ENABLED", "true") + request = SimpleNamespace( + state=SimpleNamespace(current_user="bob", api_token=False), headers={}, + app=SimpleNamespace( + state=SimpleNamespace( + auth_manager=SimpleNamespace( + is_configured=True, + is_admin=lambda username: False, + ) + ) + ), ) + with pytest.raises(HTTPException) as exc_info: + codex_routes._require_cookbook_admin(request) -class TestCookieSessionAdminGate: - """Non-admin cookie sessions must be rejected; admin sessions allowed.""" - - def test_non_admin_rejected_read(self, monkeypatch): - monkeypatch.setenv("AUTH_ENABLED", "true") - req = _cookie_request(is_admin=False) - with pytest.raises(HTTPException) as exc: - _require_cookbook_scope(req, COOKBOOK_READ_SCOPES) - assert exc.value.status_code == 403 - - def test_non_admin_rejected_launch(self, monkeypatch): - monkeypatch.setenv("AUTH_ENABLED", "true") - req = _cookie_request(is_admin=False) - with pytest.raises(HTTPException) as exc: - _require_cookbook_scope(req, COOKBOOK_LAUNCH_SCOPES) - assert exc.value.status_code == 403 - - def test_admin_allowed_read(self, monkeypatch): - monkeypatch.setenv("AUTH_ENABLED", "true") - req = _cookie_request(is_admin=True) - owner = _require_cookbook_scope(req, COOKBOOK_READ_SCOPES) - assert owner == "bob" - - def test_admin_allowed_launch(self, monkeypatch): - monkeypatch.setenv("AUTH_ENABLED", "true") - req = _cookie_request(is_admin=True) - owner = _require_cookbook_scope(req, COOKBOOK_LAUNCH_SCOPES) - assert owner == "bob" - - -class TestApiTokenScopeGate: - """API-token callers are governed by scope, not admin status.""" - - def test_token_with_scope_allowed(self, monkeypatch): - monkeypatch.setenv("AUTH_ENABLED", "true") - req = _api_token_request(scopes=["cookbook:read"]) - owner = _require_cookbook_scope(req, COOKBOOK_READ_SCOPES) - assert owner == "alice" - - def test_token_missing_scope_rejected(self, monkeypatch): - monkeypatch.setenv("AUTH_ENABLED", "true") - req = _api_token_request(scopes=["unrelated:scope"]) - with pytest.raises(HTTPException) as exc: - _require_cookbook_scope(req, COOKBOOK_READ_SCOPES) - assert exc.value.status_code == 403 - - -class TestSourceCodeGate: - """Static checks: all cookbook routes use _require_cookbook_scope.""" - - def test_no_raw_scope_owner_in_cookbook_routes(self): - from pathlib import Path - source = Path("routes/codex_routes.py").read_text(encoding="utf-8") - # _scope_owner should NOT appear inside cookbook route handlers. - # Find lines between cookbook route defs that still call _scope_owner. - in_cookbook = False - violations = [] - for i, line in enumerate(source.splitlines(), 1): - if "@router." in line and "/cookbook/" in line: - in_cookbook = True - elif "@router." in line and "/cookbook/" not in line: - in_cookbook = False - if in_cookbook and "_scope_owner(request" in line: - violations.append((i, line.strip())) - assert violations == [], ( - f"Cookbook routes still use _scope_owner instead of _require_cookbook_scope: {violations}" + assert exc_info.value.status_code == 403 + + +def test_capabilities_hide_retired_cookbook_surface(): + side_effects: list[str] = [] + with TestClient(_build_app(side_effects)) as client: + response = client.get( + "/api/codex/capabilities", + headers={"x-test-identity": "api-token"}, ) + + assert response.status_code == 200 + payload = response.json() + assert "cookbook" not in payload["tools"] + assert "cookbook" not in json.dumps(payload).lower() + assert payload["tools"]["todos"]["read"] is False + + +def test_shipped_agent_surfaces_do_not_offer_cookbook_bearer_actions(): + root = Path(__file__).resolve().parents[1] + helper_paths = [ + root / "integrations/codex/scripts/odysseus_api.py", + root / "integrations/claude/skills/odysseus/scripts/odysseus_api.py", + ] + skill_paths = [ + root / "integrations/codex/skills/odysseus/SKILL.md", + root / "integrations/claude/skills/odysseus/SKILL.md", + ] + + for path in helper_paths: + source = path.read_text(encoding="utf-8") + assert 'command == "cookbook"' not in source + assert "/api/codex/cookbook" not in source + + for path in skill_paths: + source = path.read_text(encoding="utf-8").lower() + assert "## cookbook serve" not in source + assert "cookbook:read" not in source + assert "cookbook:launch" not in source + assert "cookbook/model deployment is intentionally operator-controlled" in source + assert "/api/codex/cookbook/*" in source + + for relative in ("static/js/admin.js", "static/js/settings.js"): + source = (root / relative).read_text(encoding="utf-8") + assert "cookbook:read" not in source + assert "cookbook:launch" not in source + + +def test_real_app_places_pre_body_boundary_outside_auth(): + """Starlette's last-added middleware is outermost in the request stack.""" + app_source = ( + Path(__file__).resolve().parents[1] / "app.py" + ).read_text(encoding="utf-8") + boundary_registration = app_source.index( + "app.add_middleware(CodexCookbookBoundaryMiddleware)" + ) + auth_registration = app_source.index("app.add_middleware(AuthMiddleware)") + + # Registering the boundary later makes it outermost, so raw Odysseus bearer + # and internal credentials are rejected before token-cache/last-used work. + # Handler/dependency gates remain the backstop for stamped identities. + assert boundary_registration > auth_registration + + +def test_every_duplicate_handler_starts_with_the_direct_call_backstop(): + source = ( + Path(__file__).resolve().parents[1] / "routes/codex_routes.py" + ).read_text(encoding="utf-8") + tree = ast.parse(source) + expected = { + "codex_cookbook_tasks", + "codex_cookbook_servers", + "codex_cookbook_output", + "codex_cookbook_serve", + "codex_cookbook_stop", + "codex_cookbook_cached", + "codex_cookbook_presets", + "codex_cookbook_serve_preset", + "codex_cookbook_adopt", + } + found = set() + + for node in ast.walk(tree): + if not isinstance(node, ast.AsyncFunctionDef) or node.name not in expected: + continue + found.add(node.name) + statements = list(node.body) + if ( + statements + and isinstance(statements[0], ast.Expr) + and isinstance(statements[0].value, ast.Constant) + and isinstance(statements[0].value.value, str) + ): + statements = statements[1:] + first = statements[0] + assert isinstance(first, ast.Expr), node.name + assert isinstance(first.value, ast.Call), node.name + assert isinstance(first.value.func, ast.Name), node.name + assert first.value.func.id == "_require_cookbook_admin", node.name + + assert found == expected diff --git a/tests/test_codex_ssh_host_validation.py b/tests/test_codex_ssh_host_validation.py index 80f918d47..23beac972 100644 --- a/tests/test_codex_ssh_host_validation.py +++ b/tests/test_codex_ssh_host_validation.py @@ -8,6 +8,7 @@ the validators the rest of the cookbook routes already apply. """ import asyncio +from types import SimpleNamespace import pytest from fastapi import APIRouter, HTTPException @@ -25,6 +26,30 @@ def _route_endpoint(path: str, method: str, router=None): def _launch_request() -> Request: + app = SimpleNamespace( + state=SimpleNamespace( + auth_manager=SimpleNamespace( + is_configured=True, + is_admin=lambda username: username == "alice", + ) + ) + ) + request = Request( + { + "type": "http", + "method": "POST", + "path": "/api/codex/cookbook/adopt", + "headers": [], + "state": {}, + "app": app, + } + ) + request.state.current_user = "alice" + request.state.api_token = False + return request + + +def _bearer_launch_request() -> Request: request = Request( { "type": "http", @@ -34,6 +59,7 @@ def _launch_request() -> Request: "state": {}, } ) + request.state.current_user = "api" request.state.api_token = True request.state.api_token_owner = "alice" request.state.api_token_scopes = ["cookbook:launch"] @@ -200,7 +226,7 @@ async def test_documents_pagination_out_of_range_offset_returns_empty_page(): @pytest.mark.parametrize("host_field", ["host", "remote_host"]) -def test_adopt_rejects_ssh_option_host_before_shell(monkeypatch, host_field): +def test_adopt_handler_rejects_ssh_option_host_before_shell(monkeypatch, host_field): calls = [] async def fail_if_shell_runs(*args, **kwargs): @@ -219,10 +245,39 @@ async def fail_if_shell_runs(*args, **kwargs): with pytest.raises(HTTPException) as exc: asyncio.run(endpoint(_launch_request(), body)) + # Direct endpoint extraction intentionally bypasses FastAPI's shared + # router dependency; request-level bearer denial is covered separately. assert exc.value.status_code == 400 assert calls == [] +def test_direct_adopt_endpoint_rejects_bearer_before_body_or_shell(monkeypatch): + calls = [] + + async def fail_if_shell_runs(*args, **kwargs): + calls.append((args, kwargs)) + raise RuntimeError("shell should not run for a bearer caller") + + monkeypatch.setattr(asyncio, "create_subprocess_shell", fail_if_shell_runs) + endpoint = _route_endpoint("/api/codex/cookbook/adopt", "POST") + + with pytest.raises(HTTPException) as exc: + asyncio.run( + endpoint( + _bearer_launch_request(), + { + "tmux_session": "serve_abc123", + "model": "org/model", + "host": "box", + }, + ) + ) + + assert exc.value.status_code == 403 + assert exc.value.detail == "Forbidden" + assert calls == [] + + @pytest.mark.asyncio async def test_email_draft_document_accepts_send_scope_with_document_write(): calls = [] diff --git a/tests/test_cors_preflight.py b/tests/test_cors_preflight.py index 24f69290b..0a834678b 100644 --- a/tests/test_cors_preflight.py +++ b/tests/test_cors_preflight.py @@ -1,10 +1,10 @@ """Regression test for the CORS-preflight auth bypass. -AuthMiddleware is the outermost middleware, so it used to 401 the credential-less -OPTIONS preflight before CORSMiddleware could answer it -- which blocks every -cross-origin browser/WebView client before the real request is ever sent. The -fix lets a genuine preflight through; `is_cors_preflight` is the pure predicate -it uses. Guard it so the bypass can't silently regress. +AuthMiddleware runs outside CORSMiddleware, so it used to 401 the credential-less +OPTIONS preflight before CORS could answer it -- which blocks every cross-origin +browser/WebView client before the real request is ever sent. The fix lets a +genuine preflight through; `is_cors_preflight` is the pure predicate it uses. +Guard it so the bypass can't silently regress. """ import os