From 3895eca2214e68aaa35ce614ff5ba2d3c0201727 Mon Sep 17 00:00:00 2001 From: RaresKeY <158580472+RaresKeY@users.noreply.github.com> Date: Tue, 21 Jul 2026 14:19:00 +0000 Subject: [PATCH] fix(auth): align token capabilities with current routes --- app.py | 19 +- companion/README.md | 11 +- companion/routes.py | 13 +- routes/api_token_routes.py | 18 +- routes/codex_routes.py | 28 +- src/api_token_capabilities.py | 353 ++++++++++++++-------- tests/test_api_token_capabilities.py | 427 ++++++++++++++++++++++----- tests/test_companion_readonly.py | 42 ++- 8 files changed, 655 insertions(+), 256 deletions(-) diff --git a/app.py b/app.py index c9a8e74dd..d4b798fd5 100644 --- a/app.py +++ b/app.py @@ -392,9 +392,15 @@ 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", "") + if ( + LOCALHOST_BYPASS + and _is_trusted_loopback(request) + and not auth_header.startswith("Bearer ody_") + ): return await call_next(request) if not auth_manager.is_configured: # No users yet — redirect to login for first-time setup @@ -403,7 +409,6 @@ 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:] # Sanity check: tokens are "ody_" + 43 chars of base64 @@ -426,11 +431,11 @@ if AUTH_ENABLED: matched_scopes = scopes or [] break if matched_id: - from src.api_token_capabilities import authorize_api_token_route + from src.api_token_capabilities import authorize_api_token_request - route_decision = authorize_api_token_route( + route_decision = authorize_api_token_request( request.method, - path, + request.scope, matched_scopes, ) if not route_decision.allowed: 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..ac51169b2 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"], 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/src/api_token_capabilities.py b/src/api_token_capabilities.py index ed135d5b5..be96ac260 100644 --- a/src/api_token_capabilities.py +++ b/src/api_token_capabilities.py @@ -1,42 +1,124 @@ -"""Default-deny route capabilities for ``ody_`` API tokens.""" +"""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, field -from typing import Iterable +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, + COOKBOOK_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 - scopes: frozenset[str] = field(default_factory=frozenset) + scope_options: ScopeOptions def matches(self, method: str, path: str) -> bool: + normalized_path = _normalize_route_path(path) return ( - method.upper() in self.methods - and _path_template_matches(self.path, path) + 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 - required_scopes: tuple[str, ...] = () def _methods(*methods: str) -> frozenset[str]: - return frozenset(m.upper() for m in methods) + return frozenset(method.upper() for method in methods) -def _scopes(*scopes: str) -> frozenset[str]: - return frozenset(scopes) +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. Query strings and fragments do not belong in + ASGI ``scope['path']``; 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 or "#" in path or "\\" 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.strip("/").split("/") - path_parts = path.strip("/").split("/") + 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): @@ -49,125 +131,144 @@ def _path_template_matches(template: str, path: str) -> bool: return True -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") -DOCUMENTS_READ = _scopes("documents:read", "documents:write") -DOCUMENTS_WRITE = _scopes("documents:write") -COOKBOOK_READ = _scopes("cookbook:read", "cookbook:launch") -COOKBOOK_LAUNCH = _scopes("cookbook:launch") +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 = "" + root_path = root_path.rstrip("/") + if root_path and (path == root_path or path.startswith(root_path + "/")): + path = path[len(root_path):] or "/" + 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, ...] = ( - ApiTokenRouteCapability(_methods("POST"), "/api/v1/chat", _scopes("chat")), - ApiTokenRouteCapability(_methods("GET"), "/api/companion/ping"), - ApiTokenRouteCapability(_methods("GET"), "/api/companion/info"), - ApiTokenRouteCapability(_methods("GET"), "/api/companion/models"), - ApiTokenRouteCapability(_methods("GET"), "/api/codex/capabilities"), - ApiTokenRouteCapability(_methods("GET"), "/api/codex/plugin.zip"), - ApiTokenRouteCapability(_methods("GET"), "/api/claude/plugin.zip"), - ApiTokenRouteCapability(_methods("GET"), "/api/codex/todos", TODO_READ), + _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("POST"), - "/api/codex/todos", - TODO_READ | TODO_WRITE, - ), - ApiTokenRouteCapability(_methods("GET"), "/api/codex/emails", EMAIL_READ), - ApiTokenRouteCapability(_methods("GET"), "/api/codex/emails/{uid}", EMAIL_READ), - ApiTokenRouteCapability(_methods("POST"), "/api/codex/emails/draft", EMAIL_DRAFT), - ApiTokenRouteCapability(_methods("POST"), "/api/codex/emails/send", EMAIL_SEND), - ApiTokenRouteCapability(_methods("GET"), "/api/codex/memory", MEMORY_READ), - ApiTokenRouteCapability(_methods("POST"), "/api/codex/memory", MEMORY_WRITE), - ApiTokenRouteCapability( - _methods("DELETE"), - "/api/codex/memory/{memory_id}", - MEMORY_WRITE, + _methods("GET"), + "/api/codex/capabilities", + _BOOTSTRAP_SCOPES, ), ApiTokenRouteCapability( _methods("GET"), - "/api/codex/calendar/events", - CALENDAR_READ, + "/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/calendar/events", - CALENDAR_WRITE, + "/api/codex/emails/draft-document", + _EMAIL_DOCUMENT_DRAFT_SCOPES, ), - ApiTokenRouteCapability( - _methods("DELETE"), + _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, + CALENDAR_WRITE_SCOPES, ), - ApiTokenRouteCapability(_methods("GET"), "/api/codex/documents", DOCUMENTS_READ), - ApiTokenRouteCapability( - _methods("GET"), - "/api/codex/documents/{doc_id}", - DOCUMENTS_READ, - ), - ApiTokenRouteCapability( - _methods("POST"), - "/api/codex/documents", - DOCUMENTS_WRITE, - ), - ApiTokenRouteCapability( - _methods("DELETE"), - "/api/codex/documents/{doc_id}", - DOCUMENTS_WRITE, - ), - ApiTokenRouteCapability( - _methods("GET"), - "/api/codex/cookbook/tasks", - COOKBOOK_READ, - ), - ApiTokenRouteCapability( - _methods("GET"), - "/api/codex/cookbook/servers", - COOKBOOK_READ, - ), - ApiTokenRouteCapability( - _methods("GET"), + _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), + _capability("GET", "/api/codex/cookbook/tasks", COOKBOOK_READ_SCOPES), + _capability("GET", "/api/codex/cookbook/servers", COOKBOOK_READ_SCOPES), + _capability( + "GET", "/api/codex/cookbook/output/{session_id}", - COOKBOOK_READ, + COOKBOOK_READ_SCOPES, ), - ApiTokenRouteCapability( - _methods("GET"), - "/api/codex/cookbook/cached", - COOKBOOK_READ, - ), - ApiTokenRouteCapability( - _methods("GET"), - "/api/codex/cookbook/presets", - COOKBOOK_READ, - ), - ApiTokenRouteCapability( - _methods("POST"), - "/api/codex/cookbook/serve", - COOKBOOK_LAUNCH, - ), - ApiTokenRouteCapability( - _methods("POST"), + _capability("GET", "/api/codex/cookbook/cached", COOKBOOK_READ_SCOPES), + _capability("GET", "/api/codex/cookbook/presets", COOKBOOK_READ_SCOPES), + _capability("POST", "/api/codex/cookbook/serve", COOKBOOK_LAUNCH_SCOPES), + _capability( + "POST", "/api/codex/cookbook/stop/{session_id}", - COOKBOOK_LAUNCH, + COOKBOOK_LAUNCH_SCOPES, ), - ApiTokenRouteCapability( - _methods("POST"), + _capability( + "POST", "/api/codex/cookbook/preset/{name}", - COOKBOOK_LAUNCH, - ), - ApiTokenRouteCapability( - _methods("POST"), - "/api/codex/cookbook/adopt", - COOKBOOK_LAUNCH, + COOKBOOK_LAUNCH_SCOPES, ), + _capability("POST", "/api/codex/cookbook/adopt", COOKBOOK_LAUNCH_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, @@ -181,29 +282,23 @@ def find_api_token_route_capability( def authorize_api_token_route( method: str, path: str, - token_scopes: Iterable[str] | None, + token_scopes: Iterable[str] | str | None, ) -> ApiTokenRouteDecision: capability = find_api_token_route_capability(method, path) - if capability is None: - return ApiTokenRouteDecision( - allowed=False, - error="API token is not allowed for this endpoint", - ) - - if not capability.scopes: + if capability is not None and capability.accepts(token_scopes): return ApiTokenRouteDecision(allowed=True) + return ApiTokenRouteDecision(allowed=False, error=API_TOKEN_FORBIDDEN_ERROR) - scopes = { - str(scope).strip() - for scope in (token_scopes or []) - if str(scope).strip() - } - if scopes.intersection(capability.scopes): - return ApiTokenRouteDecision(allowed=True) - required = tuple(sorted(capability.scopes)) - return ApiTokenRouteDecision( - allowed=False, - error=f"API token missing required scope: {' or '.join(required)}", - required_scopes=required, +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 index 55a0cd64f..1d799e52f 100644 --- a/tests/test_api_token_capabilities.py +++ b/tests/test_api_token_capabilities.py @@ -1,106 +1,379 @@ +from pathlib import Path + +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 test_api_token_route_capability_allows_declared_scope(): - decision = authorize_api_token_route("POST", "/api/v1/chat", ["chat"]) - - assert decision.allowed is True - assert decision.error is None +def _allowed(method, path, scopes): + return authorize_api_token_route(method, path, scopes).allowed -def test_api_token_route_capability_rejects_missing_scope(): - decision = authorize_api_token_route("POST", "/api/v1/chat", ["documents:read"]) - - assert decision.allowed is False - assert decision.error == "API token missing required scope: chat" - assert decision.required_scopes == ("chat",) - - -def test_api_token_route_capability_rejects_unregistered_routes(): +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", + ), + ("GET", "/api/codex/cookbook/tasks", "cookbook:read", "chat"), + ("GET", "/api/codex/cookbook/servers", "cookbook:launch", "chat"), + ( + "GET", + "/api/codex/cookbook/output/serve-1", + "cookbook:read", + "chat", + ), + ("GET", "/api/codex/cookbook/cached", "cookbook:read", "chat"), + ("GET", "/api/codex/cookbook/presets", "cookbook:read", "chat"), + ("POST", "/api/codex/cookbook/serve", "cookbook:launch", "chat"), + ( + "POST", + "/api/codex/cookbook/stop/serve-1", + "cookbook:launch", + "cookbook:read", + ), + ( + "POST", + "/api/codex/cookbook/preset/default", + "cookbook:launch", + "cookbook:read", + ), + ("POST", "/api/codex/cookbook/adopt", "cookbook:launch", "chat"), + ], +) +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 + + +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", + "", ]: - decision = authorize_api_token_route(method, path, ["chat", "todos:write"]) - assert decision.allowed is False - assert decision.error == "API token is not allowed for this endpoint" + assert _allowed("GET", path, ["chat"]) is False -def test_api_token_route_capability_allows_valid_token_only_bootstrap_routes(): - for method, path in [ +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("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 + + +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 test_manifest_contains_only_the_current_audited_bearer_routes(): + expected = { + ("POST", "/api/v1/chat"), + ("GET", "/api/models"), ("GET", "/api/companion/ping"), ("GET", "/api/companion/info"), ("GET", "/api/companion/models"), ("GET", "/api/codex/capabilities"), ("GET", "/api/codex/plugin.zip"), ("GET", "/api/claude/plugin.zip"), - ]: - decision = authorize_api_token_route(method, path, []) - assert decision.allowed is True + ("GET", "/api/codex/todos"), + ("POST", "/api/codex/todos"), + ("GET", "/api/codex/emails"), + ("GET", "/api/codex/emails/{uid}"), + ("POST", "/api/codex/emails/draft-document"), + ("POST", "/api/codex/emails/draft"), + ("POST", "/api/codex/emails/send"), + ("GET", "/api/codex/memory"), + ("POST", "/api/codex/memory"), + ("DELETE", "/api/codex/memory/{memory_id}"), + ("GET", "/api/codex/calendar/events"), + ("POST", "/api/codex/calendar/events"), + ("DELETE", "/api/codex/calendar/events/{uid}"), + ("GET", "/api/codex/documents"), + ("GET", "/api/codex/documents/{doc_id}"), + ("POST", "/api/codex/documents"), + ("DELETE", "/api/codex/documents/{doc_id}"), + ("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"), + } + actual = { + (method, capability.path) + for capability in API_TOKEN_ROUTE_CAPABILITIES + for method in capability.methods + } + + assert actual == expected -def test_api_token_route_capability_matches_path_templates(): - 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 - ) +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", + "cookbook:read", + "cookbook:launch", + } -def test_api_token_route_capability_preserves_codex_fine_grained_checks(): +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 <= ALL_API_TOKEN_SCOPES + + +def test_capability_gate_runs_only_after_a_valid_bearer_match(): + source = Path("app.py").read_text(encoding="utf-8") + cors_gate = source.index("if is_cors_preflight(") + exempt_gate = source.index("if _is_auth_exempt(path):") + internal_gate = source.index("# In-process internal-tool token bypass") + local_gate = source.index("# Allow DIRECT localhost requests") + bearer_gate = source.index('if auth_header.startswith("Bearer ody_"):') + token_match = source.index("if matched_id:", bearer_gate) + capability_gate = source.index("authorize_api_token_request(", token_match) + token_state = source.index("request.state.api_token = True", capability_gate) + cookie_gate = source.index("# --- Cookie-based session auth ---", token_state) + assert ( - authorize_api_token_route("GET", "/api/codex/todos", ["todos:write"]).allowed - is True - ) - assert ( - authorize_api_token_route("POST", "/api/codex/todos", ["todos:read"]).allowed - is True - ) - assert ( - authorize_api_token_route( - "POST", - "/api/codex/emails/send", - ["email:draft"], - ).allowed - is False - ) - assert ( - authorize_api_token_route( - "POST", - "/api/codex/emails/send", - ["email:send"], - ).allowed - is True - ) - assert ( - authorize_api_token_route( - "DELETE", - "/api/codex/documents/doc-1", - ["documents:read"], - ).allowed - is False - ) - assert ( - authorize_api_token_route( - "DELETE", - "/api/codex/documents/doc-1", - ["documents:write"], - ).allowed - is True + cors_gate + < exempt_gate + < internal_gate + < local_gate + < bearer_gate + < token_match + < capability_gate + < token_state + < cookie_gate ) + local_block = source[local_gate:bearer_gate] + assert 'not auth_header.startswith("Bearer ody_")' in local_block 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"),