diff --git a/app.py b/app.py index 8363ba4e9..8e4fcfe9f 100644 --- a/app.py +++ b/app.py @@ -392,9 +392,23 @@ if AUTH_ENABLED: # Allow DIRECT localhost requests (internal service calls from # heartbeats etc.). Tunnel/proxy-forwarded requests are excluded by # _is_trusted_loopback so LOCALHOST_BYPASS can't be abused over a - # Cloudflare tunnel / reverse proxy. Keep LOCALHOST_BYPASS=false for - # network-exposed deployments regardless. - if LOCALHOST_BYPASS and _is_trusted_loopback(request): + # Cloudflare tunnel / reverse proxy. An explicitly presented ody_ + # bearer token still follows its token capability boundary; local + # requests without one keep the documented bypass behavior. + auth_header = request.headers.get("authorization", "") + auth_scheme, auth_separator, auth_credentials = auth_header.partition(" ") + raw_api_token = ( + auth_credentials.lstrip(" ") + if auth_separator and auth_scheme.casefold() == "bearer" + else "" + ) + if not raw_api_token.startswith("ody_"): + raw_api_token = "" + if ( + LOCALHOST_BYPASS + and _is_trusted_loopback(request) + and not raw_api_token + ): return await call_next(request) if not auth_manager.is_configured: # No users yet — redirect to login for first-time setup @@ -403,9 +417,8 @@ if AUTH_ENABLED: return JSONResponse(status_code=401, content={"error": "Setup required"}) # --- Bearer token auth (API tokens for external integrations) --- - auth_header = request.headers.get("authorization", "") - if auth_header.startswith("Bearer ody_"): - raw_token = auth_header[7:] + if raw_api_token: + raw_token = raw_api_token # Sanity check: tokens are "ody_" + 43 chars of base64 if len(raw_token) < 12 or len(raw_token) > 100: return JSONResponse(status_code=401, content={"error": "Invalid API token"}) @@ -426,6 +439,19 @@ if AUTH_ENABLED: matched_scopes = scopes or [] break if matched_id: + from src.api_token_capabilities import authorize_api_token_request + + route_decision = authorize_api_token_request( + request.method, + request.scope, + matched_scopes, + ) + if not route_decision.allowed: + return JSONResponse( + status_code=403, + content={"error": route_decision.error}, + ) + # Update last_used_at off the hot path. Doing it # inline used to keep the request open across an # extra commit; do it fire-and-forget instead. diff --git a/companion/README.md b/companion/README.md index 8f22ff256..c263be211 100644 --- a/companion/README.md +++ b/companion/README.md @@ -5,14 +5,15 @@ Odysseus server offers and pair to it, without duplicating any LLM logic. | Method | Path | Auth | Purpose | |---|---|---|---| -| GET | `/api/companion/ping` | session or token | cheap, auth-validated health check | -| GET | `/api/companion/info` | session or token | server identity + capability flags | -| GET | `/api/companion/models` | session or token | the **caller's own** model endpoints | +| GET | `/api/companion/ping` | session or chat-scoped token | cheap, auth-validated health check | +| GET | `/api/companion/info` | session or chat-scoped token | server identity + capability flags | +| GET | `/api/companion/models` | session or chat-scoped token | the **caller's own** model endpoints | | GET | `/api/companion/pair` | **admin cookie** | pairing page (a form; never mints) | | POST | `/api/companion/pair` | **admin cookie** | mint a one-time pairing token (`?format=json` for an in-app screen) | -`/models` scopes to the caller's real owner plus legacy null-owner shared rows -(same rule as `owner_filter`) and never returns API-key material. +Bearer reads use the existing chat scope. `/models` scopes to the caller's real +owner plus legacy null-owner shared rows (same rule as `owner_filter`) and never +returns API-key material. ## Pairing CSRF posture diff --git a/companion/routes.py b/companion/routes.py index 0191640ef..62c0ecedf 100644 --- a/companion/routes.py +++ b/companion/routes.py @@ -5,9 +5,8 @@ offers and pair to it, without duplicating any LLM logic. Auth is enforced globally by AuthMiddleware (app.py), so reaching a handler here means the caller is authenticated by either a cookie session or a Bearer `ody_` -API token. Ping/info accept either credential type, models requires a chat- -scoped API token for bearer callers, and the pairing endpoints are admin-cookie -only. +API token. Read endpoints accept cookie sessions or chat-scoped bearer tokens; +the pairing endpoints remain admin-cookie only. Pairing CSRF posture: minting happens ONLY on POST. The session cookie is SameSite=Lax (routes/auth_routes.py), which a browser does not send on a @@ -53,8 +52,8 @@ def owner_can_see(row_owner, owner) -> bool: return row_owner is None or row_owner == owner -def require_models_scope(request: Request) -> None: - """Require the companion chat scope for bearer-token model inventory.""" +def require_companion_scope(request: Request) -> None: + """Require the existing chat scope for companion bearer reads.""" if not getattr(request.state, "api_token", False): return scopes = getattr(request.state, "api_token_scopes", None) or [] @@ -86,6 +85,7 @@ def setup_companion_routes() -> APIRouter: def ping(request: Request): """Cheap, auth-validated health check. A 200 with ok=true confirms the host/port and credential are valid; middleware returns 401 otherwise.""" + require_companion_scope(request) from core.constants import APP_VERSION return { "ok": True, @@ -98,6 +98,7 @@ def setup_companion_routes() -> APIRouter: def info(request: Request): """Server identity + coarse capability flags. `owner` is the caller's own identity (the token's owner for bearer callers).""" + require_companion_scope(request) from core.constants import APP_VERSION return { "name": "odysseus", @@ -116,7 +117,7 @@ def setup_companion_routes() -> APIRouter: rows -- the same rule as owner_filter. Read-only; never returns api_key material. """ - require_models_scope(request) + require_companion_scope(request) import json as _json from core.database import SessionLocal, ModelEndpoint diff --git a/routes/api_token_routes.py b/routes/api_token_routes.py index cbc828731..8268f3495 100644 --- a/routes/api_token_routes.py +++ b/routes/api_token_routes.py @@ -8,26 +8,12 @@ from fastapi import APIRouter, HTTPException, Request, Form from core.database import get_db_session, ApiToken from core.middleware import require_admin +from src.api_token_capabilities import ALL_API_TOKEN_SCOPES from src.auth_helpers import get_current_user MAX_NAME_LEN = 100 DEFAULT_SCOPES = "chat" -ALLOWED_SCOPES = { - "chat", - "todos:read", - "todos:write", - "documents:read", - "documents:write", - "email:read", - "email:draft", - "email:send", - "calendar:read", - "calendar:write", - "memory:read", - "memory:write", - "cookbook:read", - "cookbook:launch", -} +ALLOWED_SCOPES = ALL_API_TOKEN_SCOPES TOKEN_PROFILES = { "chat": ["chat"], "codex_todos": ["todos:read", "todos:write"], @@ -68,7 +54,6 @@ def _normalize_scopes(scopes: str | list[str] | None = None, profile: str | None 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..ff15124bb 100644 --- a/routes/codex_routes.py +++ b/routes/codex_routes.py @@ -16,25 +16,27 @@ from fastapi import APIRouter, BackgroundTasks, Body, HTTPException, Request from fastapi.responses import StreamingResponse from core.middleware import require_admin +from src.api_token_capabilities import ( + CALENDAR_READ_SCOPES, + CALENDAR_WRITE_SCOPES, + COOKBOOK_LAUNCH_SCOPES, + COOKBOOK_READ_SCOPES, + DOCS_READ_SCOPES, + DOCS_WRITE_SCOPES, + EMAIL_DRAFT_SCOPES, + EMAIL_READ_SCOPES, + EMAIL_SEND_SCOPES, + MEMORY_READ_SCOPES, + MEMORY_WRITE_SCOPES, + TODO_READ_SCOPES, + TODO_WRITE_SCOPES, +) 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"} -EMAIL_DRAFT_SCOPES = {"email:draft", "email:send"} -EMAIL_SEND_SCOPES = {"email:send"} -MEMORY_READ_SCOPES = {"memory:read", "memory:write"} -MEMORY_WRITE_SCOPES = {"memory:write"} -CALENDAR_READ_SCOPES = {"calendar:read", "calendar:write"} -CALENDAR_WRITE_SCOPES = {"calendar:write"} -DOCS_READ_SCOPES = {"documents:read", "documents:write"} -DOCS_WRITE_SCOPES = {"documents:write"} WRITE_ACTIONS = {"add", "create", "new", "save", "remind", "update", "delete", "toggle_item", "remove", "remove_item"} diff --git a/routes/model_routes.py b/routes/model_routes.py index 600150a66..36bccad7d 100644 --- a/routes/model_routes.py +++ b/routes/model_routes.py @@ -1614,8 +1614,10 @@ def setup_model_routes(model_discovery): legacy/shared null-owner rows). Cached per-user for 30s.""" # Require auth; "" is the unconfigured single-user mode, treated as # "see everything" by _fetch_models. + is_api_token = False try: - if getattr(request.state, "api_token", False): + is_api_token = bool(getattr(request.state, "api_token", False)) + if is_api_token: scopes = set(getattr(request.state, "api_token_scopes", []) or []) if "chat" not in scopes: raise HTTPException(403, "API token is not scoped for chat") @@ -1633,15 +1635,17 @@ def setup_model_routes(model_discovery): except Exception as e: logger.error("Auth gate error in GET /api/models, failing closed: %s", e) raise HTTPException(status_code=500, detail="Internal error") - # Admins see every endpoint (they manage the global pool); regular - # users get the owner-scoped view. + # Browser admins see every endpoint because they manage the global + # pool. Bearer tokens are integrations, not browser-admin sessions: + # even an admin-owned token stays limited to owner + shared endpoints. _is_admin = False - try: - auth_mgr = getattr(request.app.state, "auth_manager", None) - if owner and auth_mgr is not None and getattr(auth_mgr, "is_admin", None): - _is_admin = bool(auth_mgr.is_admin(owner)) - except Exception: - _is_admin = False + if not is_api_token: + try: + auth_mgr = getattr(request.app.state, "auth_manager", None) + if owner and auth_mgr is not None and getattr(auth_mgr, "is_admin", None): + _is_admin = bool(auth_mgr.is_admin(owner)) + except Exception: + _is_admin = False now = _time.time() # Cache key includes the admin flag so a demotion / promotion doesn't # serve the wrong scoped view from cache. @@ -1654,7 +1658,7 @@ def setup_model_routes(model_discovery): # Kick off background refresh to update caches from live endpoints. # Page boot can opt out with background=false so opening Odysseus does # not start endpoint probes against slow/offline model servers. - if background or refresh: + if not is_api_token and (background or refresh): _refresh_caches_bg(force=refresh) return result diff --git a/src/api_token_capabilities.py b/src/api_token_capabilities.py new file mode 100644 index 000000000..21c5e97f7 --- /dev/null +++ b/src/api_token_capabilities.py @@ -0,0 +1,289 @@ +"""Default-deny route capabilities for ``ody_`` API tokens. + +The auth middleware consults this manifest only after it has validated a bearer +token. Browser sessions, internal-tool requests, auth exemptions, preflight +requests, and auth-disabled deployments stay on their existing auth paths. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from itertools import product +from typing import Iterable, Mapping, Sequence + + +API_TOKEN_FORBIDDEN_ERROR = "API token is not authorized for this endpoint" + +CHAT_SCOPES = frozenset({"chat"}) +TODO_READ_SCOPES = frozenset({"todos:read", "todos:write"}) +TODO_WRITE_SCOPES = frozenset({"todos:write"}) +EMAIL_READ_SCOPES = frozenset({"email:read", "email:draft", "email:send"}) +EMAIL_DRAFT_SCOPES = frozenset({"email:draft", "email:send"}) +EMAIL_SEND_SCOPES = frozenset({"email:send"}) +MEMORY_READ_SCOPES = frozenset({"memory:read", "memory:write"}) +MEMORY_WRITE_SCOPES = frozenset({"memory:write"}) +CALENDAR_READ_SCOPES = frozenset({"calendar:read", "calendar:write"}) +CALENDAR_WRITE_SCOPES = frozenset({"calendar:write"}) +DOCS_READ_SCOPES = frozenset({"documents:read", "documents:write"}) +DOCS_WRITE_SCOPES = frozenset({"documents:write"}) +COOKBOOK_READ_SCOPES = frozenset({"cookbook:read", "cookbook:launch"}) +COOKBOOK_LAUNCH_SCOPES = frozenset({"cookbook:launch"}) + +ALL_API_TOKEN_SCOPES = frozenset().union( + CHAT_SCOPES, + TODO_READ_SCOPES, + EMAIL_READ_SCOPES, + MEMORY_READ_SCOPES, + CALENDAR_READ_SCOPES, + DOCS_READ_SCOPES, +) + + +ScopeSet = frozenset[str] +ScopeOptions = tuple[ScopeSet, ...] + + +def _one_of(scopes: Iterable[str]) -> ScopeOptions: + """Return alternatives where any one accepted scope authorizes a route.""" + return tuple(frozenset({scope}) for scope in sorted(scopes)) + + +def _one_from_each(*groups: Iterable[str]) -> ScopeOptions: + """Return alternatives requiring one accepted scope from every group.""" + normalized = [tuple(sorted(group)) for group in groups] + return tuple(frozenset(option) for option in product(*normalized)) + + +@dataclass(frozen=True) +class ApiTokenRouteCapability: + methods: frozenset[str] + path: str + scope_options: ScopeOptions + + def matches(self, method: str, path: str) -> bool: + normalized_path = _normalize_route_path(path) + return ( + normalized_path is not None + and method.upper() in self.methods + and _path_template_matches(self.path, normalized_path) + ) + + def accepts(self, token_scopes: Iterable[str] | str | None) -> bool: + scopes = normalize_api_token_scopes(token_scopes) + return any(required.issubset(scopes) for required in self.scope_options) + + +@dataclass(frozen=True) +class ApiTokenRouteDecision: + allowed: bool + error: str | None = None + + +def _methods(*methods: str) -> frozenset[str]: + return frozenset(method.upper() for method in methods) + + +def _capability( + method: str, + path: str, + scopes: Iterable[str], +) -> ApiTokenRouteCapability: + return ApiTokenRouteCapability(_methods(method), path, _one_of(scopes)) + + +def _normalize_route_path(path: str) -> str | None: + """Normalize the single trailing slash FastAPI redirects by default. + + Everything else stays strict. ASGI ``scope['path']`` is already decoded, so + question marks and hashes can be legitimate path-segment data rather than + query or fragment delimiters. Repeated separators, dot segments, + backslashes, and control characters are treated as malformed instead of + being normalized into a capability match. + """ + if not isinstance(path, str) or not path.startswith("/"): + return None + if any(ord(char) < 32 or ord(char) == 127 for char in path): + return None + if "\\" in path: + return None + if path != "/" and path.endswith("/"): + path = path[:-1] + if path != "/" and (path.endswith("/") or "//" in path): + return None + parts = path.split("/")[1:] + if any(part in {"", ".", ".."} for part in parts): + return None + return path + + +def _path_template_matches(template: str, path: str) -> bool: + template_parts = template.split("/")[1:] + path_parts = path.split("/")[1:] + if len(template_parts) != len(path_parts): + return False + for expected, actual in zip(template_parts, path_parts): + if expected.startswith("{") and expected.endswith("}"): + if not actual: + return False + continue + if expected != actual: + return False + return True + + +def _route_path_from_scope(scope: Mapping[str, object]) -> str: + """Return the same application-relative path the ASGI router receives.""" + path = scope.get("path", "") + root_path = scope.get("root_path", "") + if not isinstance(path, str): + return "" + if not isinstance(root_path, str): + root_path = "" + if root_path: + if path == root_path: + path = "/" + elif ( + path.startswith(root_path) + and len(path) > len(root_path) + and path[len(root_path)] == "/" + ): + path = path[len(root_path):] + return path + + +def _has_encoded_path_separator(scope: Mapping[str, object]) -> bool: + """Reject encoded delimiters whose decoding can differ across HTTP layers.""" + raw_path = scope.get("raw_path") + if not isinstance(raw_path, (bytes, bytearray)): + return False + lowered = bytes(raw_path).split(b"?", 1)[0].lower() + return any(encoded in lowered for encoded in (b"%2f", b"%5c", b"%00")) + + +def normalize_api_token_scopes( + token_scopes: Iterable[str] | str | None, +) -> frozenset[str]: + if isinstance(token_scopes, str): + values: Iterable[object] = token_scopes.split(",") + else: + values = token_scopes or () + return frozenset( + normalized + for scope in values + if (normalized := str(scope).strip()) + ) + + +_BOOTSTRAP_SCOPES = _one_of(ALL_API_TOKEN_SCOPES) +_EMAIL_DOCUMENT_DRAFT_SCOPES = _one_from_each( + EMAIL_DRAFT_SCOPES, + DOCS_WRITE_SCOPES, +) + + +API_TOKEN_ROUTE_CAPABILITIES: tuple[ApiTokenRouteCapability, ...] = ( + _capability("POST", "/api/v1/chat", CHAT_SCOPES), + _capability("GET", "/api/models", CHAT_SCOPES), + _capability("GET", "/api/companion/ping", CHAT_SCOPES), + _capability("GET", "/api/companion/info", CHAT_SCOPES), + _capability("GET", "/api/companion/models", CHAT_SCOPES), + ApiTokenRouteCapability( + _methods("GET"), + "/api/codex/capabilities", + _BOOTSTRAP_SCOPES, + ), + ApiTokenRouteCapability( + _methods("GET"), + "/api/codex/plugin.zip", + _BOOTSTRAP_SCOPES, + ), + ApiTokenRouteCapability( + _methods("GET"), + "/api/claude/plugin.zip", + _BOOTSTRAP_SCOPES, + ), + _capability("GET", "/api/codex/todos", TODO_READ_SCOPES), + _capability("POST", "/api/codex/todos", TODO_READ_SCOPES), + _capability("GET", "/api/codex/emails", EMAIL_READ_SCOPES), + _capability("GET", "/api/codex/emails/{uid}", EMAIL_READ_SCOPES), + ApiTokenRouteCapability( + _methods("POST"), + "/api/codex/emails/draft-document", + _EMAIL_DOCUMENT_DRAFT_SCOPES, + ), + _capability("POST", "/api/codex/emails/draft", EMAIL_DRAFT_SCOPES), + _capability("POST", "/api/codex/emails/send", EMAIL_SEND_SCOPES), + _capability("GET", "/api/codex/memory", MEMORY_READ_SCOPES), + _capability("POST", "/api/codex/memory", MEMORY_WRITE_SCOPES), + _capability("DELETE", "/api/codex/memory/{memory_id}", MEMORY_WRITE_SCOPES), + _capability("GET", "/api/codex/calendar/events", CALENDAR_READ_SCOPES), + _capability("POST", "/api/codex/calendar/events", CALENDAR_WRITE_SCOPES), + _capability( + "DELETE", + "/api/codex/calendar/events/{uid}", + CALENDAR_WRITE_SCOPES, + ), + _capability("GET", "/api/codex/documents", DOCS_READ_SCOPES), + _capability("GET", "/api/codex/documents/{doc_id}", DOCS_READ_SCOPES), + _capability("POST", "/api/codex/documents", DOCS_WRITE_SCOPES), + _capability("DELETE", "/api/codex/documents/{doc_id}", DOCS_WRITE_SCOPES), +) + + +def _validate_manifest(capabilities: Sequence[ApiTokenRouteCapability]) -> None: + seen: set[tuple[str, str]] = set() + for capability in capabilities: + if _normalize_route_path(capability.path) != capability.path: + raise RuntimeError( + f"Malformed API-token capability path: {capability.path!r}" + ) + if not capability.methods or not capability.scope_options: + raise RuntimeError("API-token capabilities must declare methods and scopes") + for required in capability.scope_options: + if not required or not required.issubset(ALL_API_TOKEN_SCOPES): + raise RuntimeError("API-token capability contains an unknown scope") + for method in capability.methods: + key = (method, capability.path) + if key in seen: + raise RuntimeError( + f"Duplicate API-token capability: {method} {capability.path}" + ) + seen.add(key) + + +_validate_manifest(API_TOKEN_ROUTE_CAPABILITIES) + + +def find_api_token_route_capability( + method: str, + path: str, +) -> ApiTokenRouteCapability | None: + for capability in API_TOKEN_ROUTE_CAPABILITIES: + if capability.matches(method, path): + return capability + return None + + +def authorize_api_token_route( + method: str, + path: str, + token_scopes: Iterable[str] | str | None, +) -> ApiTokenRouteDecision: + capability = find_api_token_route_capability(method, path) + if capability is not None and capability.accepts(token_scopes): + return ApiTokenRouteDecision(allowed=True) + return ApiTokenRouteDecision(allowed=False, error=API_TOKEN_FORBIDDEN_ERROR) + + +def authorize_api_token_request( + method: str, + scope: Mapping[str, object], + token_scopes: Iterable[str] | str | None, +) -> ApiTokenRouteDecision: + if _has_encoded_path_separator(scope): + return ApiTokenRouteDecision(allowed=False, error=API_TOKEN_FORBIDDEN_ERROR) + return authorize_api_token_route( + method, + _route_path_from_scope(scope), + token_scopes, + ) diff --git a/tests/test_api_token_capabilities.py b/tests/test_api_token_capabilities.py new file mode 100644 index 000000000..5fb116288 --- /dev/null +++ b/tests/test_api_token_capabilities.py @@ -0,0 +1,405 @@ +import pytest + +from src.api_token_capabilities import ( + ALL_API_TOKEN_SCOPES, + API_TOKEN_FORBIDDEN_ERROR, + API_TOKEN_ROUTE_CAPABILITIES, + authorize_api_token_request, + authorize_api_token_route, + find_api_token_route_capability, +) + + +def _allowed(method, path, scopes): + return authorize_api_token_route(method, path, scopes).allowed + + +def test_retained_public_chat_and_model_inventory_require_chat_scope(): + for method, path in [ + ("POST", "/api/v1/chat"), + ("GET", "/api/models"), + ]: + assert _allowed(method, path, ["chat"]) is True + assert _allowed(method, path, ["documents:read"]) is False + assert _allowed(method, path, []) is False + + +def test_companion_bearer_reads_all_require_chat_scope(): + for path in [ + "/api/companion/ping", + "/api/companion/info", + "/api/companion/models", + ]: + assert _allowed("GET", path, ["chat"]) is True + assert _allowed("GET", path, ["todos:read"]) is False + assert _allowed("GET", path, []) is False + + +@pytest.mark.parametrize( + ("method", "path", "accepted_scope", "rejected_scope"), + [ + ("GET", "/api/codex/todos", "todos:read", "email:read"), + ("POST", "/api/codex/todos", "todos:write", "email:read"), + ("GET", "/api/codex/emails", "email:read", "todos:read"), + ("GET", "/api/codex/emails/abc123", "email:send", "chat"), + ("POST", "/api/codex/emails/draft", "email:draft", "email:read"), + ("POST", "/api/codex/emails/send", "email:send", "email:draft"), + ("GET", "/api/codex/memory", "memory:read", "calendar:read"), + ("POST", "/api/codex/memory", "memory:write", "memory:read"), + ("DELETE", "/api/codex/memory/mem-1", "memory:write", "memory:read"), + ("GET", "/api/codex/calendar/events", "calendar:read", "memory:read"), + ("POST", "/api/codex/calendar/events", "calendar:write", "calendar:read"), + ( + "DELETE", + "/api/codex/calendar/events/event-1", + "calendar:write", + "calendar:read", + ), + ("GET", "/api/codex/documents", "documents:read", "todos:read"), + ("GET", "/api/codex/documents/doc-1", "documents:write", "chat"), + ("POST", "/api/codex/documents", "documents:write", "documents:read"), + ( + "DELETE", + "/api/codex/documents/doc-1", + "documents:write", + "documents:read", + ), + ], +) +def test_codex_route_families_require_their_existing_scopes( + method, + path, + accepted_scope, + rejected_scope, +): + assert _allowed(method, path, [accepted_scope]) is True + assert _allowed(method, path, [rejected_scope]) is False + + +def test_email_draft_document_requires_email_draft_and_document_write(): + path = "/api/codex/emails/draft-document" + + assert _allowed("POST", path, ["email:draft", "documents:write"]) is True + assert _allowed("POST", path, ["email:send", "documents:write"]) is True + assert _allowed("POST", path, ["email:draft"]) is False + assert _allowed("POST", path, ["documents:write"]) is False + assert _allowed("POST", path, ["email:read", "documents:write"]) is False + + +def test_bootstrap_downloads_require_at_least_one_accepted_scope(): + for path in [ + "/api/codex/capabilities", + "/api/codex/plugin.zip", + "/api/claude/plugin.zip", + ]: + for scope in ALL_API_TOKEN_SCOPES: + assert _allowed("GET", path, [scope]) is True + assert _allowed("GET", path, []) is False + assert _allowed("GET", path, ["unknown:scope"]) is False + + +@pytest.mark.parametrize( + ("method", "path"), + [ + ("GET", "/api/tokens"), + ("POST", "/api/tokens"), + ("GET", "/api/tokens/profiles"), + ("PATCH", "/api/tokens/token-1"), + ("GET", "/api/companion/pair"), + ("POST", "/api/companion/pair"), + ("POST", "/api/shell/exec"), + ("POST", "/api/shell/stream"), + ("GET", "/api/workspace/browse"), + ("GET", "/api/tools"), + ("POST", "/api/tools"), + ("GET", "/api/users"), + ("GET", "/api/sessions"), + ("GET", "/api/history/session-1"), + ("POST", "/api/upload"), + ("POST", "/api/chat_stream"), + ("GET", "/api/calendar/events"), + ("GET", "/api/codex/todos/export"), + ], +) +def test_privileged_and_owner_attributing_ui_routes_remain_blocked(method, path): + decision = authorize_api_token_route(method, path, ALL_API_TOKEN_SCOPES) + + assert decision.allowed is False + assert decision.error == API_TOKEN_FORBIDDEN_ERROR + + +@pytest.mark.parametrize( + ("method", "path"), + [ + ("GET", "/api/codex/cookbook/tasks"), + ("GET", "/api/codex/cookbook/servers"), + ("GET", "/api/codex/cookbook/output/serve-1"), + ("GET", "/api/codex/cookbook/cached"), + ("GET", "/api/codex/cookbook/presets"), + ("POST", "/api/codex/cookbook/serve"), + ("POST", "/api/codex/cookbook/stop/serve-1"), + ("POST", "/api/codex/cookbook/preset/default"), + ("POST", "/api/codex/cookbook/adopt"), + ], +) +def test_legacy_cookbook_scopes_are_inert_at_the_bearer_boundary(method, path): + decision = authorize_api_token_route( + method, + path, + ["cookbook:read", "cookbook:launch"], + ) + + assert decision.allowed is False + assert decision.error == API_TOKEN_FORBIDDEN_ERROR + + +def test_method_matching_is_exact_and_case_insensitive(): + assert _allowed("post", "/api/v1/chat", ["chat"]) is True + assert _allowed("GET", "/api/v1/chat", ["chat"]) is False + assert _allowed("POST", "/api/models", ["chat"]) is False + assert _allowed("OPTIONS", "/api/models", ["chat"]) is False + + +def test_single_trailing_slash_matches_but_malformed_paths_fail_closed(): + assert _allowed("GET", "/api/models/", ["chat"]) is True + for path in [ + "/api/models//", + "//api/models", + "/api//models", + "/api/./models", + "/api/../models", + "/api/models?refresh=true", + "/api/models#fragment", + "/api/models\\extra", + "/api/models\x00", + "api/models", + "", + ]: + assert _allowed("GET", path, ["chat"]) is False + + +def test_path_templates_match_one_nonempty_segment_only(): + assert find_api_token_route_capability( + "GET", + "/api/codex/emails/abc123", + ) is not None + assert find_api_token_route_capability( + "DELETE", + "/api/codex/calendar/events/event-1", + ) is not None + assert find_api_token_route_capability( + "GET", + "/api/codex/emails/abc123/extra", + ) is None + assert find_api_token_route_capability("GET", "/api/codex/emails//") is None + + +def test_asgi_root_path_is_removed_before_matching(): + decision = authorize_api_token_request( + "GET", + { + "root_path": "/odysseus", + "path": "/odysseus/api/models", + "raw_path": b"/odysseus/api/models", + }, + ["chat"], + ) + + assert decision.allowed is True + + wrong_prefix = authorize_api_token_request( + "GET", + { + "root_path": "/odysseus", + "path": "/odyssey/api/models", + "raw_path": b"/odyssey/api/models", + }, + ["chat"], + ) + assert wrong_prefix.allowed is False + + +@pytest.mark.parametrize( + ("root_path", "path", "raw_path"), + [ + ("/odysseus/", "/odysseus//api/models", b"/odysseus//api/models"), + ("/", "//api/models", b"//api/models"), + ], +) +def test_asgi_root_path_with_trailing_slash_is_removed_before_matching( + root_path, + path, + raw_path, +): + decision = authorize_api_token_request( + "GET", + { + "root_path": root_path, + "path": path, + "raw_path": raw_path, + }, + ["chat"], + ) + + assert decision.allowed is True + + +@pytest.mark.parametrize("encoded", [b"%2f", b"%2F", b"%5c", b"%5C", b"%00"]) +def test_encoded_path_delimiters_fail_closed(encoded): + decision = authorize_api_token_request( + "GET", + { + "path": "/api/models", + "raw_path": b"/api" + encoded + b"models", + }, + ["chat"], + ) + + assert decision.allowed is False + + +@pytest.mark.parametrize( + ("decoded", "encoded"), + [("?", b"%3F"), ("#", b"%23")], +) +def test_encoded_calendar_uid_characters_follow_the_decoded_router_path( + decoded, + encoded, +): + decision = authorize_api_token_request( + "DELETE", + { + "path": f"/api/codex/calendar/events/team{decoded}primary", + "raw_path": b"/api/codex/calendar/events/team" + encoded + b"primary", + }, + ["calendar:write"], + ) + + assert decision.allowed is True + + +def test_encoded_static_letters_follow_the_decoded_router_path(): + decision = authorize_api_token_request( + "GET", + { + "path": "/api/models", + "raw_path": b"/api/%6dodels", + }, + ["chat"], + ) + + assert decision.allowed is True + + +def test_missing_scope_and_unknown_route_share_one_public_error(): + wrong_scope = authorize_api_token_route("GET", "/api/models", ["todos:read"]) + unknown_route = authorize_api_token_route( + "GET", + "/api/private-owner-data", + ["chat"], + ) + + assert wrong_scope == unknown_route + assert wrong_scope.error == API_TOKEN_FORBIDDEN_ERROR + + +def test_scope_string_normalization_is_not_character_based(): + assert _allowed("GET", "/api/models", "todos:read, chat") is True + assert _allowed("GET", "/api/models", "c,h,a,t") is False + + +def test_every_manifest_entry_has_known_nonempty_scopes_and_unique_methods(): + seen = set() + for capability in API_TOKEN_ROUTE_CAPABILITIES: + assert capability.scope_options + for option in capability.scope_options: + assert option + assert option <= ALL_API_TOKEN_SCOPES + for method in capability.methods: + key = (method, capability.path) + assert key not in seen + seen.add(key) + + +def _router_inventory(router): + return { + (method, route.path) + for route in router.routes + for method in getattr(route, "methods", set()) + } + + +def test_manifest_matches_runtime_scoped_router_inventory(): + from companion.routes import setup_companion_routes + from routes.codex_routes import setup_claude_routes, setup_codex_routes + + companion_inventory = _router_inventory(setup_companion_routes()) + companion_pairing = { + route + for route in companion_inventory + if route[1] == "/api/companion/pair" + } + assert companion_pairing == { + ("GET", "/api/companion/pair"), + ("POST", "/api/companion/pair"), + } + + expected = { + ("POST", "/api/v1/chat"), + ("GET", "/api/models"), + } + codex_inventory = _router_inventory(setup_codex_routes()) + cookbook_inventory = { + route + for route in codex_inventory + if route[1].startswith("/api/codex/cookbook/") + } + assert cookbook_inventory == { + ("GET", "/api/codex/cookbook/tasks"), + ("GET", "/api/codex/cookbook/servers"), + ("GET", "/api/codex/cookbook/output/{session_id}"), + ("GET", "/api/codex/cookbook/cached"), + ("GET", "/api/codex/cookbook/presets"), + ("POST", "/api/codex/cookbook/serve"), + ("POST", "/api/codex/cookbook/stop/{session_id}"), + ("POST", "/api/codex/cookbook/preset/{name}"), + ("POST", "/api/codex/cookbook/adopt"), + } + expected.update(codex_inventory - cookbook_inventory) + expected.update(_router_inventory(setup_claude_routes())) + expected.update(companion_inventory - companion_pairing) + actual = { + (method, capability.path) + for capability in API_TOKEN_ROUTE_CAPABILITIES + for method in capability.methods + } + + assert actual == expected + + +def test_accepted_scope_catalog_is_explicit_and_has_no_admin_scope(): + assert ALL_API_TOKEN_SCOPES == { + "chat", + "todos:read", + "todos:write", + "documents:read", + "documents:write", + "email:read", + "email:draft", + "email:send", + "calendar:read", + "calendar:write", + "memory:read", + "memory:write", + } + + +def test_token_minting_and_route_checks_share_the_scope_catalog(): + from routes.api_token_routes import ALLOWED_SCOPES + import routes.codex_routes as codex_routes + + assert ALLOWED_SCOPES is ALL_API_TOKEN_SCOPES + assert codex_routes.TODO_READ_SCOPES <= ALL_API_TOKEN_SCOPES + assert codex_routes.EMAIL_READ_SCOPES <= ALL_API_TOKEN_SCOPES + assert codex_routes.COOKBOOK_READ_SCOPES.isdisjoint(ALL_API_TOKEN_SCOPES) + assert codex_routes.COOKBOOK_LAUNCH_SCOPES.isdisjoint(ALL_API_TOKEN_SCOPES) diff --git a/tests/test_api_token_middleware_integration.py b/tests/test_api_token_middleware_integration.py new file mode 100644 index 000000000..019ad098f --- /dev/null +++ b/tests/test_api_token_middleware_integration.py @@ -0,0 +1,279 @@ +"""Behavior-level coverage for the production API-token middleware boundary.""" + +import json +import os +from pathlib import Path +import subprocess +import sys +import textwrap + + +ROOT = Path(__file__).resolve().parents[1] + + +def test_production_auth_middleware_enforces_api_token_capabilities(tmp_path): + """Drive the real ``app.AuthMiddleware`` in an isolated subprocess.""" + + env = os.environ.copy() + env.update({ + "AUTH_ENABLED": "true", + "CHROMADB_CONNECT_TIMEOUT": "0.01", + "CHROMADB_HOST": "127.0.0.1", + "CHROMADB_PORT": "9", + "DATABASE_URL": f"sqlite:///{tmp_path / 'app.db'}", + "LOCALHOST_BYPASS": "false", + "ODYSSEUS_DATA_DIR": str(tmp_path), + "ODYSSEUS_DISABLE_MCP": "1", + "OPENAI_API_KEY": "", + "PYTHONPATH": str(ROOT), + "PYTHON_DOTENV_DISABLED": "1", + }) + probe = textwrap.dedent( + """ + import asyncio + import json + + from starlette.requests import Request + from starlette.responses import JSONResponse + + import app as app_module + + + RAW_TOKEN = "ody_" + "a" * 43 + + + class _AuthManager: + is_configured = True + users = {"alice": {}} + + @staticmethod + def validate_token(_token): + return False + + @staticmethod + def get_username_for_token(_token): + return None + + + app_module.auth_manager = _AuthManager() + app_module.app.state.auth_manager = app_module.auth_manager + app_module.app.state._token_cache_dirty = False + app_module._bcrypt.checkpw = lambda *_args: True + + + def _request( + method, + path, + *, + loopback, + authorization=None, + raw_path=None, + ): + return Request({ + "type": "http", + "http_version": "1.1", + "method": method, + "scheme": "http", + "path": path, + "raw_path": raw_path or path.encode(), + "root_path": "", + "query_string": b"", + "headers": [ + ( + b"authorization", + (authorization or f"Bearer {RAW_TOKEN}").encode(), + ), + ], + "client": ( + "127.0.0.1" if loopback else "192.0.2.10", + 4321, + ), + "server": ("testserver", 80), + "app": app_module.app, + }) + + + async def _case( + path, + scopes, + *, + loopback=False, + localhost_bypass=False, + authorization=None, + method="GET", + raw_path=None, + ): + app_module.LOCALHOST_BYPASS = localhost_bypass + app_module._token_cache.clear() + app_module._token_cache[RAW_TOKEN[:8]] = [ + ("token-1", "unused-hash", "alice", scopes), + ] + request = _request( + method, + path, + loopback=loopback, + authorization=authorization, + raw_path=raw_path, + ) + calls = [] + + async def call_next(received): + calls.append(received) + return JSONResponse({"reached": True}) + + middleware = app_module.AuthMiddleware(app_module.app) + response = await middleware.dispatch(request, call_next) + return { + "status": response.status_code, + "body": json.loads(response.body), + "called": len(calls), + "api_token": getattr(request.state, "api_token", None), + "owner": getattr(request.state, "api_token_owner", None), + } + + + async def main(): + original_create_task = app_module._asyncio.create_task + app_module._asyncio.create_task = lambda coroutine: coroutine.close() + try: + forbidden = await _case("/api/unregistered", ["chat"]) + allowed = await _case("/api/models", ["chat"]) + encoded_calendar_uids = { + "question_mark": await _case( + "/api/codex/calendar/events/team?primary", + ["calendar:write"], + method="DELETE", + raw_path=( + b"/api/codex/calendar/events/team%3Fprimary" + ), + ), + "hash": await _case( + "/api/codex/calendar/events/team#primary", + ["calendar:write"], + method="DELETE", + raw_path=( + b"/api/codex/calendar/events/team%23primary" + ), + ), + } + cookbook_forbidden = {} + for label, (method, path) in { + "tasks": ("GET", "/api/codex/cookbook/tasks"), + "servers": ("GET", "/api/codex/cookbook/servers"), + "output": ( + "GET", + "/api/codex/cookbook/output/serve-1", + ), + "cached": ("GET", "/api/codex/cookbook/cached"), + "presets": ("GET", "/api/codex/cookbook/presets"), + "serve": ("POST", "/api/codex/cookbook/serve"), + "stop": ( + "POST", + "/api/codex/cookbook/stop/serve-1", + ), + "preset": ( + "POST", + "/api/codex/cookbook/preset/default", + ), + "adopt": ("POST", "/api/codex/cookbook/adopt"), + }.items(): + cookbook_forbidden[label] = await _case( + path, + ["cookbook:read", "cookbook:launch"], + method=method, + ) + local_wrong_scope = {} + for label, authorization in { + "canonical": f"Bearer {RAW_TOKEN}", + "lowercase": f"bearer {RAW_TOKEN}", + "uppercase": f"BEARER {RAW_TOKEN}", + "mixed_case": f"bEaReR {RAW_TOKEN}", + "multiple_spaces": f"Bearer {RAW_TOKEN}", + }.items(): + local_wrong_scope[label] = await _case( + "/api/models", + ["todos:read"], + loopback=True, + localhost_bypass=True, + authorization=authorization, + ) + finally: + app_module._asyncio.create_task = original_create_task + + print("RESULT=" + json.dumps({ + "forbidden": forbidden, + "allowed": allowed, + "cookbook_forbidden": cookbook_forbidden, + "encoded_calendar_uids": encoded_calendar_uids, + "local_wrong_scope": local_wrong_scope, + }, sort_keys=True)) + + + asyncio.run(main()) + """ + ) + result = subprocess.run( + [sys.executable, "-c", probe], + cwd=ROOT, + env=env, + capture_output=True, + text=True, + timeout=30, + check=False, + ) + + assert result.returncode == 0, result.stderr + result_line = next( + (line for line in result.stdout.splitlines() if line.startswith("RESULT=")), + None, + ) + assert result_line is not None, result.stdout + observed = json.loads(result_line.removeprefix("RESULT=")) + + generic_error = {"error": "API token is not authorized for this endpoint"} + assert observed["forbidden"] == { + "status": 403, + "body": generic_error, + "called": 0, + "api_token": None, + "owner": None, + } + assert observed["allowed"] == { + "status": 200, + "body": {"reached": True}, + "called": 1, + "api_token": True, + "owner": "alice", + } + assert observed["encoded_calendar_uids"] == { + "question_mark": observed["allowed"], + "hash": observed["allowed"], + } + expected_forbidden = { + "status": 403, + "body": generic_error, + "called": 0, + "api_token": None, + "owner": None, + } + assert observed["cookbook_forbidden"] == { + label: expected_forbidden + for label in ( + "tasks", + "servers", + "output", + "cached", + "presets", + "serve", + "stop", + "preset", + "adopt", + ) + } + assert observed["local_wrong_scope"] == { + "canonical": expected_forbidden, + "lowercase": expected_forbidden, + "uppercase": expected_forbidden, + "mixed_case": expected_forbidden, + "multiple_spaces": expected_forbidden, + } diff --git a/tests/test_api_token_routes.py b/tests/test_api_token_routes.py index 40afc2226..1f8d94398 100644 --- a/tests/test_api_token_routes.py +++ b/tests/test_api_token_routes.py @@ -192,7 +192,12 @@ def test_create_token_attributes_owner_hashes_secret_and_returns_raw_once(monkey invalidator.assert_called_once() -def test_create_token_accepts_cookbook_read_scope(monkeypatch, token_routes_mod): +@pytest.mark.parametrize("scope", ["cookbook:read", "cookbook:launch"]) +def test_create_token_rejects_retired_cookbook_scopes( + monkeypatch, + token_routes_mod, + scope, +): monkeypatch.setenv("AUTH_ENABLED", "true") mod = token_routes_mod @@ -202,24 +207,12 @@ 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") + with pytest.raises(HTTPException) as exc: + create_token(request=req, name="retired-cookbook", scopes=scope) - 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") - - assert resp["scopes"] == ["cookbook:read", "cookbook:launch"] + assert exc.value.status_code == 400 + assert exc.value.detail == f"Unknown token scope: {scope}" + fake_session.add.assert_not_called() # --------------------------------------------------------------------------- diff --git a/tests/test_companion_readonly.py b/tests/test_companion_readonly.py index 589621b66..8a3a623b3 100644 --- a/tests/test_companion_readonly.py +++ b/tests/test_companion_readonly.py @@ -117,12 +117,16 @@ def _ep( ) -def _models_route(): +def _companion_route(path): for route in setup_companion_routes().routes: - if getattr(route, "path", "") == "/api/companion/models": + if getattr(route, "path", "") == path: assert "GET" in getattr(route, "methods", set()) return route.endpoint - raise AssertionError("GET /api/companion/models route not found") + raise AssertionError(f"GET {path} route not found") + + +def _models_route(): + return _companion_route("/api/companion/models") def _call_models_route(monkeypatch, rows, request): @@ -256,6 +260,38 @@ def test_models_route_rejects_api_token_without_chat_scope(monkeypatch): assert "chat scope" in exc.value.detail +@pytest.mark.parametrize( + "path", + [ + "/api/companion/ping", + "/api/companion/info", + "/api/companion/models", + ], +) +def test_all_companion_bearer_reads_reject_tokens_without_chat_scope(path): + with pytest.raises(HTTPException) as exc: + _companion_route(path)( + _request( + api_token=True, + api_token_owner="alice", + api_token_scopes=["todos:read"], + current_user="api", + ) + ) + + assert exc.value.status_code == 403 + assert "chat scope" in exc.value.detail + + +@pytest.mark.parametrize("path", ["/api/companion/ping", "/api/companion/info"]) +def test_companion_identity_reads_still_allow_cookie_sessions(path): + result = _companion_route(path)( + _request(api_token=False, current_user="alice") + ) + + assert isinstance(result, dict) + + def test_models_route_unresolved_owner_returns_only_shared_rows(monkeypatch): rows = [ _ep(1, "alice-endpoint", "alice"), diff --git a/tests/test_model_routes.py b/tests/test_model_routes.py index a692c8e47..2c3ba0718 100644 --- a/tests/test_model_routes.py +++ b/tests/test_model_routes.py @@ -1684,7 +1684,83 @@ def test_api_models_scopes_api_token_to_token_owner(monkeypatch): result = _route_endpoint(router, "/api/models")(request) assert [item["endpoint_name"] for item in result["items"]] == ["alice", "shared"] - assert admin_checks == ["alice"] + assert admin_checks == [] + + +def test_api_models_admin_owned_token_does_not_reuse_cookie_admin_inventory(monkeypatch): + rows = [ + _route_ep("admin", "http://admin.example/v1", cached_models=["admin-model"], owner="admin"), + _route_ep("shared", "http://shared.example/v1", cached_models=["shared-model"], owner=None), + _route_ep("alice", "http://alice.example/v1", cached_models=["alice-model"], owner="alice"), + ] + db = _RouteDb(rows) + router = model_routes.setup_model_routes(model_discovery=None) + admin_checks = [] + + monkeypatch.setattr(model_routes, "ModelEndpoint", _RouteModelEndpoint) + monkeypatch.setattr(model_routes, "SessionLocal", lambda: db) + + auth_manager = SimpleNamespace( + is_configured=True, + is_admin=lambda user: admin_checks.append(user) or True, + ) + cookie_request = SimpleNamespace( + state=SimpleNamespace(current_user="admin", api_token=False), + app=SimpleNamespace(state=SimpleNamespace(auth_manager=auth_manager)), + ) + token_request = SimpleNamespace( + state=SimpleNamespace( + current_user="api", + api_token=True, + api_token_owner="admin", + api_token_scopes=["chat"], + ), + app=SimpleNamespace(state=SimpleNamespace(auth_manager=auth_manager)), + ) + + endpoint = _route_endpoint(router, "/api/models") + cookie_result = endpoint(cookie_request) + token_result = endpoint(token_request) + + assert [item["endpoint_name"] for item in cookie_result["items"]] == ["admin", "shared", "alice"] + assert [item["endpoint_name"] for item in token_result["items"]] == ["admin", "shared"] + assert admin_checks == ["admin"] + + +@pytest.mark.parametrize("refresh_kwargs", [{"refresh": True}, {"background": True}]) +def test_api_models_api_token_never_starts_global_refresh(monkeypatch, refresh_kwargs): + rows = [ + _route_ep("alice", "http://alice.example/v1", cached_models=["alice-model"], owner="alice"), + _route_ep("bob", "http://bob.example/v1", cached_models=["bob-model"], owner="bob"), + ] + db = _RouteDb(rows) + router = model_routes.setup_model_routes(model_discovery=None) + + monkeypatch.setattr(model_routes, "ModelEndpoint", _RouteModelEndpoint) + monkeypatch.setattr(model_routes, "SessionLocal", lambda: db) + + def fail_thread(*args, **kwargs): + raise AssertionError("bearer model inventory must not start the global refresher") + + monkeypatch.setattr(threading, "Thread", fail_thread) + + request = SimpleNamespace( + state=SimpleNamespace( + current_user="api", + api_token=True, + api_token_owner="alice", + api_token_scopes=["chat"], + ), + app=SimpleNamespace( + state=SimpleNamespace( + auth_manager=SimpleNamespace(is_configured=True, is_admin=lambda user: True), + ), + ), + ) + + result = _route_endpoint(router, "/api/models")(request, **refresh_kwargs) + + assert [item["endpoint_name"] for item in result["items"]] == ["alice"] def test_api_models_returns_only_pinned_proxy_models_without_refresh_probe(monkeypatch):