From 0b32ecea5087a197d42be4dc87729817b2aada7b Mon Sep 17 00:00:00 2001 From: RaresKeY <158580472+RaresKeY@users.noreply.github.com> Date: Sun, 30 Aug 2026 15:35:17 +0000 Subject: [PATCH] fix(security): restore scoped bearer compatibility --- routes/chat_helpers.py | 23 ++- routes/chat_routes.py | 79 +++++++-- routes/codex_routes.py | 33 ++-- routes/history/history_routes.py | 19 ++- routes/session_routes.py | 10 +- routes/webhook/webhook_routes.py | 65 +++++--- src/context_compactor.py | 16 +- src/llm_core.py | 28 +++- tests/test_api_token_forward_fixes.py | 179 +++++++++++++++++++-- tests/test_api_token_security_cycle4.py | 100 +++++++++++- tests/test_session_endpoint_owner_scope.py | 7 +- 11 files changed, 468 insertions(+), 91 deletions(-) diff --git a/routes/chat_helpers.py b/routes/chat_helpers.py index 8b7ebff5a..1e07ea691 100644 --- a/routes/chat_helpers.py +++ b/routes/chat_helpers.py @@ -514,8 +514,18 @@ def _has_auth_keys(headers) -> bool: ) -def resolve_session_auth(sess, session_id: str, owner: Optional[str] = None): +def resolve_session_auth( + sess, + session_id: str, + owner: Optional[str] = None, + *, + allow_live_probes: bool = True, +): """Ensure session has auth headers — resolve from endpoint DB if missing.""" + if not allow_live_probes: + # Bearer chat is cache-only and request-local. Do not resolve provider + # credentials or write recovered headers/session state in this mode. + return try: from src.chatgpt_subscription import is_chatgpt_subscription_base is_chatgpt_subscription = is_chatgpt_subscription_base(getattr(sess, "endpoint_url", "") or "") @@ -594,7 +604,7 @@ def _match_cached_model_id(requested: str, models) -> Optional[str]: def _normalize_model_id_from_cache(sess) -> Optional[str]: - """Use stored endpoint model IDs before falling back to a live /models probe.""" + """Use stored ``cached_models``/pinned IDs before a live /models probe.""" endpoint_url = getattr(sess, "endpoint_url", "") or "" requested = getattr(sess, "model", "") or "" if not endpoint_url or not requested: @@ -622,11 +632,12 @@ def _normalize_model_id_from_cache(sess) -> Optional[str]: except Exception: continue - raw_models = getattr(ep, "cached_models", None) - if not raw_models: - continue try: - models = json.loads(raw_models) if isinstance(raw_models, str) else raw_models + from routes.model_routes import _effective_endpoint_kind, _picker_models_for_endpoint + + base_url = getattr(ep, "base_url", "") or "" + kind = _effective_endpoint_kind(ep, base_url) + models, _ = _picker_models_for_endpoint(ep, base_url, kind) except Exception: continue diff --git a/routes/chat_routes.py b/routes/chat_routes.py index 403ec9539..6d2fdee6b 100644 --- a/routes/chat_routes.py +++ b/routes/chat_routes.py @@ -55,7 +55,11 @@ from core.database import Session as DBSession, ChatMessage as DBChatMessage from core.database import Document as DBDocument, ModelEndpoint from core.log_safety import redact_url from routes.research_routes import _resolve_research_endpoint -from routes.model_routes import _visible_models +from routes.model_routes import ( + _effective_endpoint_kind, + _picker_models_for_endpoint, + _visible_models, +) from routes.chat_helpers import ( resolve_session_auth, build_chat_context, @@ -414,8 +418,17 @@ def _session_url_matches_endpoint(session_url: str, endpoint_base: str) -> bool: return sess in variants or sess.startswith(base + "/") -def _clear_orphaned_session_endpoint(sess, owner: str | None = None) -> bool: +def _clear_orphaned_session_endpoint( + sess, + owner: str | None = None, + *, + allow_live_probes: bool = True, +) -> bool: """Clear a session model if its endpoint was deleted from ModelEndpoint.""" + if not allow_live_probes: + # Bearer chat must not turn an orphan check into a session/database + # mutation. Interactive callers retain the repair behavior below. + return False if not getattr(sess, "endpoint_url", ""): return False db = SessionLocal() @@ -581,13 +594,16 @@ def _recover_empty_session_model(sess, session_id: str, owner: str | None = None except Exception as e: logger.warning("Failed to parse cached_models for endpoint %r", getattr(ep, "id", "?"), exc_info=e) cached = [] - if not cached: - visible = [] - else: - try: - visible = _visible_models(cached, getattr(ep, "hidden_models", None)) - except Exception: - visible = cached + try: + base_url = getattr(ep, "base_url", "") or "" + kind = _effective_endpoint_kind(ep, base_url) + visible, _ = _picker_models_for_endpoint(ep, base_url, kind) + except Exception: + visible = _visible_models( + cached, + getattr(ep, "hidden_models", None), + getattr(ep, "pinned_models", None), + ) if current_model and current_model in {str(item).strip() for item in visible}: return False if is_chatgpt_subscription and allow_live_probes: @@ -611,9 +627,15 @@ def _recover_empty_session_model(sess, session_id: str, owner: str | None = None if not cached: return False try: - visible = _visible_models(cached, getattr(ep, "hidden_models", None)) + base_url = getattr(ep, "base_url", "") or "" + kind = _effective_endpoint_kind(ep, base_url) + visible, _ = _picker_models_for_endpoint(ep, base_url, kind) except Exception: - visible = cached + visible = _visible_models( + cached, + getattr(ep, "hidden_models", None), + getattr(ep, "pinned_models", None), + ) if current_model and current_model in {str(item).strip() for item in visible}: return False if not visible: @@ -661,6 +683,8 @@ def _reconcile_selected_route_from_request( session_id: str, form_data, owner: str | None = None, + *, + allow_live_probes: bool = True, ) -> bool: """Apply the model route the browser selected before streaming. @@ -669,6 +693,11 @@ def _reconcile_selected_route_from_request( stream request includes the route that was selected at click/send time. Trust only registered endpoint ids, or the session's existing endpoint URL. """ + if not allow_live_probes: + # The bearer path may consume the already-selected session route, but + # it must not resolve credentials or persist a browser-supplied route. + return False + selected_model = str(form_data.get("selected_model") or "").strip() selected_endpoint_id = str(form_data.get("selected_endpoint_id") or "").strip() selected_endpoint_url = str(form_data.get("selected_endpoint_url") or "").strip() @@ -795,7 +824,11 @@ def setup_chat_routes( raise HTTPException(404, f"Session '{session}' not found") owner = effective_user(request) request_capability = build_request_capability(request) - if _clear_orphaned_session_endpoint(sess, owner=owner): + if _clear_orphaned_session_endpoint( + sess, + owner=owner, + allow_live_probes=request_capability.allow_live_probes, + ): raise HTTPException(400, "Selected model endpoint was removed. Pick another model in Settings.") # Empty model + live endpoint = setup race (Issue #587). Repair from @@ -1322,8 +1355,19 @@ def setup_chat_routes( external_untrusted_context_seen = ( external_untrusted_context_seen or retired_tool_approval_taint ) - _reconcile_selected_route_from_request(request, sess, session, form_data, owner=owner) - if _clear_orphaned_session_endpoint(sess, owner=owner): + _reconcile_selected_route_from_request( + request, + sess, + session, + form_data, + owner=owner, + allow_live_probes=request_capability.allow_live_probes, + ) + if _clear_orphaned_session_endpoint( + sess, + owner=owner, + allow_live_probes=request_capability.allow_live_probes, + ): raise HTTPException(400, "Selected model endpoint was removed. Pick another model in Settings.") # Issue #587: picker shows a model from the endpoint cache but # s.model never made it onto the DB row (first-send race after @@ -1402,7 +1446,12 @@ def setup_chat_routes( _enforce_chat_privileges(request, sess) # Ensure session has auth headers - resolve_session_auth(sess, session, owner=effective_user(request)) + resolve_session_auth( + sess, + session, + owner=effective_user(request), + allow_live_probes=request_capability.allow_live_probes, + ) # Check for research_pending BEFORE mode persist overwrites it # An approval response resumes the sealed agent action. Do not let diff --git a/routes/codex_routes.py b/routes/codex_routes.py index 0ef711a76..3f84aea8b 100644 --- a/routes/codex_routes.py +++ b/routes/codex_routes.py @@ -1,9 +1,9 @@ """Codex integration routes. These are small HTTP surfaces intended for the Codex plugin/MCP bridge. They -reuse existing Odysseus helpers. The bridge is an interactive host-control -plane and is unavailable to bearer principals; cookie/admin callers retain the -documented operation path. +reuse existing Odysseus helpers. Owner-scoped data operations support bearer +principals with the matching token scope; the Cookbook/plugin host-control +plane remains interactive-only. """ import asyncio @@ -90,7 +90,6 @@ async def _as_owner(request: Request, owner: str, fn, *args, **kwargs): def _scope_owner(request: Request, allowed: set[str]) -> str: """Return the data owner if the caller is allowed for this Codex action.""" - require_non_bearer_request(request) if getattr(request.state, "api_token", False) is True: scopes = set(getattr(request.state, "api_token_scopes", []) or []) if not scopes.intersection(allowed): @@ -102,7 +101,6 @@ def _scope_owner(request: Request, allowed: set[str]) -> str: def _scope_owner_all(request: Request, required: set[str]) -> str: """Return owner only when an API token has every required scope.""" - require_non_bearer_request(request) if getattr(request.state, "api_token", False) is True: scopes = set(getattr(request.state, "api_token_scopes", []) or []) missing = required - scopes @@ -120,6 +118,7 @@ def _require_cookbook_scope(request: Request, allowed: set[str]) -> str: privileges because cookbook surfaces expose host topology, task logs, tmux commands, and model-serving controls. """ + require_non_bearer_request(request) owner = _scope_owner(request, allowed) if getattr(request.state, "api_token", False) is not True: require_admin(request) @@ -156,7 +155,6 @@ def setup_codex_routes( router = APIRouter( prefix="/api/codex", tags=["codex"], - dependencies=[Depends(require_non_bearer_request)], ) email_list_endpoint = _find_endpoint(email_router, "GET", "/api/email/list") email_read_endpoint = _find_endpoint(email_router, "GET", "/api/email/read/{uid}") @@ -172,7 +170,6 @@ def setup_codex_routes( @router.get("/capabilities") def capabilities(request: Request): - require_non_bearer_request(request) token_scopes = set(getattr(request.state, "api_token_scopes", []) or []) has_token = getattr(request.state, "api_token", False) is True def scoped(allowed): @@ -222,7 +219,7 @@ def setup_codex_routes( }, } - @router.get("/plugin.zip") + @router.get("/plugin.zip", dependencies=[Depends(require_non_bearer_request)]) def plugin_zip(request: Request): require_non_bearer_request(request) require_authenticated_request(request) @@ -522,7 +519,7 @@ def setup_codex_routes( # ── Cookbook surface ── # These handlers retain their legacy scope constants for compatibility - # with callers and tests, but the bridge is now an interactive-only + # with callers and tests, but the bridge is an interactive-only # host-control plane. Bearer principals are rejected before any task-list, # tmux-output, launch, stop, or model-serving operation. @@ -568,14 +565,14 @@ def setup_codex_routes( if k not in ("hf_token", "_secrets")} return clean - @router.get("/cookbook/tasks") + @router.get("/cookbook/tasks", dependencies=[Depends(require_non_bearer_request)]) async def codex_cookbook_tasks(request: Request): _require_cookbook_scope(request, COOKBOOK_READ_SCOPES) 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=[Depends(require_non_bearer_request)]) async def codex_cookbook_servers(request: Request): _require_cookbook_scope(request, COOKBOOK_READ_SCOPES) state = _read_cookbook_state() @@ -594,7 +591,7 @@ def setup_codex_routes( }) return {"servers": cleaned} - @router.get("/cookbook/output/{session_id}") + @router.get("/cookbook/output/{session_id}", dependencies=[Depends(require_non_bearer_request)]) async def codex_cookbook_output(request: Request, session_id: str, tail: int = 400): _require_cookbook_scope(request, COOKBOOK_READ_SCOPES) # Defensive: session_id must be the tmux-style id we issue @@ -636,7 +633,7 @@ def setup_codex_routes( "task": _redact_task(task), } - @router.post("/cookbook/serve") + @router.post("/cookbook/serve", dependencies=[Depends(require_non_bearer_request)]) async def codex_cookbook_serve(request: Request, body: dict[str, Any] = Body(default_factory=dict)): _require_cookbook_scope(request, COOKBOOK_LAUNCH_SCOPES) # Wraps /api/model/serve with the SAME validation the UI uses. @@ -675,7 +672,7 @@ def setup_codex_routes( 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=[Depends(require_non_bearer_request)]) async def codex_cookbook_stop(request: Request, session_id: str): _require_cookbook_scope(request, COOKBOOK_LAUNCH_SCOPES) import re as _re @@ -692,7 +689,7 @@ def setup_codex_routes( 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=[Depends(require_non_bearer_request)]) 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 @@ -754,7 +751,7 @@ def setup_codex_routes( platform=params.get("platform") or None, ) - @router.get("/cookbook/presets") + @router.get("/cookbook/presets", dependencies=[Depends(require_non_bearer_request)]) 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` @@ -775,7 +772,7 @@ def setup_codex_routes( }) return {"presets": out, "default_host": (state.get("env") or {}).get("defaultServer", "")} - @router.post("/cookbook/preset/{name}") + @router.post("/cookbook/preset/{name}", dependencies=[Depends(require_non_bearer_request)]) 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.""" @@ -825,7 +822,7 @@ def setup_codex_routes( raise HTTPException(503, "model serve endpoint unavailable") return await serve_endpoint(request, req) - @router.post("/cookbook/adopt") + @router.post("/cookbook/adopt", dependencies=[Depends(require_non_bearer_request)]) 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 diff --git a/routes/history/history_routes.py b/routes/history/history_routes.py index 2bfefc3c2..88dc53010 100644 --- a/routes/history/history_routes.py +++ b/routes/history/history_routes.py @@ -792,7 +792,6 @@ def setup_history_routes(session_manager, upload_handler=None) -> APIRouter: try: from src.model_context import estimate_tokens, get_context_length from src.llm_core import llm_call_async - from src.endpoint_resolver import resolve_endpoint if len(session.history) < 6: return {"status": "ok", "message": "Not enough messages to compact"} @@ -822,11 +821,19 @@ def setup_history_routes(session_manager, upload_handler=None) -> APIRouter: for m in older ) - # Use utility model if available - util_url, util_model, util_headers = resolve_endpoint("utility", owner=owner or None) - compact_url = util_url or session.endpoint_url - compact_model = util_model or session.model - compact_headers = util_headers if util_url else session.headers + # Use the utility model only for interactive/live-capable callers. + # Bearer compaction remains on the selected session route. + if capability.allow_live_probes: + from src.endpoint_resolver import resolve_endpoint + + util_url, util_model, util_headers = resolve_endpoint("utility", owner=owner or None) + compact_url = util_url or session.endpoint_url + compact_model = util_model or session.model + compact_headers = util_headers if util_url else session.headers + else: + compact_url = session.endpoint_url + compact_model = session.model + compact_headers = session.headers from src.context_compactor import SELF_SUMMARY_SYSTEM_PROMPT, normalize_compaction_summary compaction_count = sum(1 for m in session.history if isinstance(m, ChatMessage) and "[Conversation summary" in (m.content or "")) diff --git a/routes/session_routes.py b/routes/session_routes.py index 39d09058d..896fc6cf4 100644 --- a/routes/session_routes.py +++ b/routes/session_routes.py @@ -1043,12 +1043,16 @@ def setup_session_routes( raise HTTPException(400, "Nothing old enough to compact") from src.context_compactor import SELF_SUMMARY_SYSTEM_PROMPT - from src.endpoint_resolver import resolve_endpoint from src.llm_core import llm_call_async owner = getattr(session, "owner", None) or effective_user(request) - url, model, headers = resolve_endpoint("utility", owner=owner) - if not url or not model: + if capability.allow_live_probes: + from src.endpoint_resolver import resolve_endpoint + + url, model, headers = resolve_endpoint("utility", owner=owner) + if not url or not model: + url, model, headers = session.endpoint_url, session.model, session.headers + else: url, model, headers = session.endpoint_url, session.model, session.headers if not url or not model: raise HTTPException(400, "No model configured for compaction") diff --git a/routes/webhook/webhook_routes.py b/routes/webhook/webhook_routes.py index ec01a7c35..30d55e70c 100644 --- a/routes/webhook/webhook_routes.py +++ b/routes/webhook/webhook_routes.py @@ -75,26 +75,53 @@ def _cached_endpoint_model_ids(endpoint) -> list[str]: ``auto`` alias, but it must not turn an ordinary chat request into a remote catalog probe. Malformed/legacy cache shapes are treated as empty. """ - raw = getattr(endpoint, "cached_models", None) - if not raw: - return [] try: - value = json.loads(raw) if isinstance(raw, str) else raw - except (TypeError, ValueError): - return [] - if isinstance(value, dict): - value = value.get("data") or value.get("models") or [] - if not isinstance(value, list): - return [] - ids = [] - for item in value: - if isinstance(item, str) and item.strip(): - ids.append(item.strip()) - elif isinstance(item, dict): - model_id = item.get("id") or item.get("name") or item.get("model") - if isinstance(model_id, str) and model_id.strip(): - ids.append(model_id.strip()) - return ids + from routes.model_routes import _effective_endpoint_kind, _picker_models_for_endpoint + + base_url = getattr(endpoint, "base_url", "") or "" + kind = _effective_endpoint_kind(endpoint, base_url) + models, _ = _picker_models_for_endpoint(endpoint, base_url, kind) + return models + except Exception: + raw = getattr(endpoint, "cached_models", None) + pinned_raw = getattr(endpoint, "pinned_models", None) + hidden_raw = getattr(endpoint, "hidden_models", None) + if not raw and not pinned_raw: + return [] + try: + value = json.loads(raw) if isinstance(raw, str) else raw + pinned = json.loads(pinned_raw) if isinstance(pinned_raw, str) else pinned_raw + hidden = json.loads(hidden_raw) if isinstance(hidden_raw, str) else hidden_raw + except (TypeError, ValueError): + return [] + if isinstance(value, dict): + value = value.get("data") or value.get("models") or [] + if isinstance(pinned, dict): + pinned = pinned.get("data") or pinned.get("models") or [] + if isinstance(hidden, dict): + hidden = hidden.get("data") or hidden.get("models") or [] + if not isinstance(value, list): + value = [] + if not isinstance(pinned, list): + pinned = [] + if not isinstance(hidden, list): + hidden = [] + raw_ids = value + pinned + hidden_ids = {str(item).strip() for item in hidden if str(item).strip()} + ids = [] + for item in raw_ids: + if isinstance(item, str) and item.strip(): + model_id = item.strip() + elif isinstance(item, dict): + model_id = item.get("id") or item.get("name") or item.get("model") + if not isinstance(model_id, str) or not model_id.strip(): + continue + model_id = model_id.strip() + else: + continue + if model_id not in hidden_ids and model_id not in ids: + ids.append(model_id) + return ids def setup_webhook_routes( diff --git a/src/context_compactor.py b/src/context_compactor.py index 94e4b7638..68a303a2f 100644 --- a/src/context_compactor.py +++ b/src/context_compactor.py @@ -379,11 +379,17 @@ async def maybe_compact( if "[Conversation summary" in m.get("content", "") ) - # Use utility model if configured, otherwise fall back to session model - util_url, util_model, util_headers = resolve_endpoint("utility", owner=owner) - compact_url = util_url or endpoint_url - compact_model = util_model or model - compact_headers = util_headers if util_url else headers + # Use the utility model only for interactive/live-capable callers. Bearer + # chat must stay on the already-selected session route and headers. + if allow_live_probes: + util_url, util_model, util_headers = resolve_endpoint("utility", owner=owner) + compact_url = util_url or endpoint_url + compact_model = util_model or model + compact_headers = util_headers if util_url else headers + else: + compact_url = endpoint_url + compact_model = model + compact_headers = headers prompt = SELF_SUMMARY_SYSTEM_PROMPT.replace( "{count}", str(len(older)) diff --git a/src/llm_core.py b/src/llm_core.py index 699460e00..3c043daae 100644 --- a/src/llm_core.py +++ b/src/llm_core.py @@ -1881,11 +1881,29 @@ def _configured_cached_model_ids( for ep in rows: if _model_list_base(getattr(ep, "base_url", "")) != target: continue - models = _parse_model_cache(getattr(ep, "cached_models", None) or getattr(ep, "models", None)) - if not models: - continue - hidden = set(_parse_model_cache(getattr(ep, "hidden_models", None))) - return [m for m in models if m not in hidden] + cached = _parse_model_cache(getattr(ep, "cached_models", None) or getattr(ep, "models", None)) + pinned_raw = getattr(ep, "pinned_models", None) + pinned = _parse_model_cache(pinned_raw) + try: + # Keep cache-only validation aligned with the model picker: + # explicit API pins are an allow-list, legacy API rows use + # cached-visible models, and local endpoints merge cache+pins. + from routes.model_routes import _effective_endpoint_kind, _picker_models_for_endpoint + + base_url = getattr(ep, "base_url", "") or "" + kind = _effective_endpoint_kind(ep, base_url) + models, _ = _picker_models_for_endpoint(ep, base_url, kind) + except Exception: + # The model route module is not required for this low-level + # helper. Preserve the safe cache+pin behavior if it cannot be + # imported during an unusual bootstrap/test sequence. + hidden = set(_parse_model_cache(getattr(ep, "hidden_models", None))) + models = [] + for model in cached + pinned: + if model not in models and model not in hidden: + models.append(model) + if models or (pinned_raw is not None and str(pinned_raw).strip() != ""): + return models except Exception: return [] finally: diff --git a/tests/test_api_token_forward_fixes.py b/tests/test_api_token_forward_fixes.py index 5239cfccd..b8ec6430a 100644 --- a/tests/test_api_token_forward_fixes.py +++ b/tests/test_api_token_forward_fixes.py @@ -395,7 +395,7 @@ def test_bearer_context_preprocessing_does_not_fetch_embedded_urls(monkeypatch): async def test_sync_bearer_chat_cannot_use_research_memory_or_background_extraction(monkeypatch): from routes import chat_routes - calls = {"memory": 0, "research": 0, "post": [], "recovery": []} + calls = {"memory": 0, "research": 0, "post": [], "recovery": [], "orphan": []} class _ChatHandler: async def handle_memory_command(self, _session, _message): @@ -434,7 +434,11 @@ async def test_sync_bearer_chat_cannot_use_research_memory_or_background_extract return "answer", args[0][0], "selected-model" monkeypatch.setattr(chat_routes, "_verify_session_owner", lambda *args, **kwargs: None) - monkeypatch.setattr(chat_routes, "_clear_orphaned_session_endpoint", lambda *args, **kwargs: False) + def clear_orphan(*args, **kwargs): + calls["orphan"].append(kwargs) + return False + + monkeypatch.setattr(chat_routes, "_clear_orphaned_session_endpoint", clear_orphan) def recover(*args, **kwargs): calls["recovery"].append(kwargs) return False @@ -487,6 +491,7 @@ async def test_sync_bearer_chat_cannot_use_research_memory_or_background_extract assert calls["memory"] == 0 assert calls["research"] == 0 assert calls["post"] and calls["post"][0]["allow_background_extraction"] is False + assert calls["orphan"] == [{"owner": "alice", "allow_live_probes": False}] assert calls["recovery"] == [{"owner": "alice", "allow_live_probes": False}] @@ -503,12 +508,28 @@ async def test_stream_bearer_chat_disables_deferred_memory_extraction(monkeypatc capture_completion=True, ) recovery_calls = [] + boundary_calls = {"reconcile": [], "orphan": [], "auth": []} def recover(*args, **kwargs): recovery_calls.append(kwargs) return False monkeypatch.setattr(chat_routes, "_recover_empty_session_model", recover) + monkeypatch.setattr( + chat_routes, + "_reconcile_selected_route_from_request", + lambda *args, **kwargs: boundary_calls["reconcile"].append(kwargs) or False, + ) + monkeypatch.setattr( + chat_routes, + "_clear_orphaned_session_endpoint", + lambda *args, **kwargs: boundary_calls["orphan"].append(kwargs) or False, + ) + monkeypatch.setattr( + chat_routes, + "resolve_session_auth", + lambda *args, **kwargs: boundary_calls["auth"].append(kwargs), + ) request = SimpleNamespace( headers={}, app=SimpleNamespace(state=SimpleNamespace(auth_manager=None)), @@ -532,6 +553,9 @@ async def test_stream_bearer_chat_disables_deferred_memory_extraction(monkeypatc assert captured["post_processed"] assert captured["post_processed"][0][1]["allow_background_extraction"] is False + assert boundary_calls["reconcile"] == [{"owner": "alice", "allow_live_probes": False}] + assert boundary_calls["orphan"] == [{"owner": "alice", "allow_live_probes": False}] + assert boundary_calls["auth"] == [{"owner": "alice", "allow_live_probes": False}] assert recovery_calls == [{"owner": "alice", "allow_live_probes": False}] @@ -680,6 +704,114 @@ def test_bearer_model_recovery_uses_cache_without_endpoint_or_session_writes(mon assert db.rollbacks == 0 +def test_bearer_model_recovery_uses_pinned_only_cache_inventory(monkeypatch): + chat_routes, db, endpoint, session_row, sess = _recovery_harness( + monkeypatch, + ["stale-cached-model"], + ) + endpoint.pinned_models = json.dumps(["pinned-model"]) + endpoint.hidden_models = json.dumps(["stale-cached-model"]) + + assert chat_routes._recover_empty_session_model( + sess, + "session-1", + owner="alice", + allow_live_probes=False, + ) is True + assert sess.model == "pinned-model" + assert session_row.model == "" + assert db.commits == 0 + + +def test_bearer_no_live_recovery_boundaries_do_not_open_or_commit(monkeypatch): + from routes import chat_helpers, chat_routes + + session = SimpleNamespace( + id="session-1", + endpoint_url="https://api.example.test/v1/chat/completions", + model="selected-model", + headers={"Authorization": "Bearer selected"}, + ) + + def forbidden_db(*args, **kwargs): + raise AssertionError("bearer no-live boundary opened a database session") + + monkeypatch.setattr(chat_routes, "SessionLocal", forbidden_db) + assert chat_routes._clear_orphaned_session_endpoint( + session, + owner="alice", + allow_live_probes=False, + ) is False + assert chat_routes._reconcile_selected_route_from_request( + SimpleNamespace(), + session, + "session-1", + {"selected_model": "new-model", "selected_endpoint_id": "ep"}, + owner="alice", + allow_live_probes=False, + ) is False + + monkeypatch.setattr(chat_helpers, "SessionLocal", forbidden_db) + monkeypatch.setattr( + "src.endpoint_resolver.resolve_endpoint_runtime", + lambda *args, **kwargs: (_ for _ in ()).throw( + AssertionError("bearer no-live auth resolved provider credentials") + ), + ) + original_headers = dict(session.headers) + chat_helpers.resolve_session_auth( + session, + "session-1", + owner="alice", + allow_live_probes=False, + ) + assert session.headers == original_headers + + +@pytest.mark.asyncio +async def test_bearer_context_compaction_uses_session_route_without_utility_resolution(monkeypatch): + from src import context_compactor + + resolver_calls = [] + llm_calls = [] + monkeypatch.setattr( + context_compactor, + "resolve_endpoint", + lambda *args, **kwargs: resolver_calls.append((args, kwargs)) or ( + "https://utility.example/v1", + "utility-model", + {"Authorization": "Bearer utility"}, + ), + ) + monkeypatch.setattr(context_compactor, "get_context_length", lambda *args, **kwargs: 1) + + async def summarize(*args, **kwargs): + llm_calls.append((args, kwargs)) + return "summary" + + monkeypatch.setattr(context_compactor, "llm_call_async", summarize) + session = SimpleNamespace() + messages = [{"role": "user", "content": f"message {i}"} for i in range(6)] + + _result, _context, compacted = await context_compactor.maybe_compact( + session, + "https://selected.example/v1/chat/completions", + "selected-model", + messages, + {"Authorization": "Bearer selected"}, + owner="alice", + persist=False, + allow_live_probes=False, + ) + + assert compacted is True + assert resolver_calls == [] + assert llm_calls[0][0][:2] == ( + "https://selected.example/v1/chat/completions", + "selected-model", + ) + assert llm_calls[0][1]["headers"] == {"Authorization": "Bearer selected"} + assert llm_calls[0][1]["allow_live_probes"] is False def test_interactive_model_recovery_retains_live_catalog_and_persistence(monkeypatch): chat_routes, db, endpoint, session_row, sess = _recovery_harness(monkeypatch, []) from src import chatgpt_subscription, endpoint_resolver @@ -998,19 +1130,40 @@ def test_bearer_cannot_reach_workspace_or_hwfit_direct_handlers(monkeypatch): @pytest.mark.asyncio -async def test_codex_bearer_rejected_before_direct_and_router_host_control(monkeypatch): +async def test_codex_bearer_data_scope_is_allowed_but_host_control_is_denied(monkeypatch): import routes.codex_routes as codex_routes + async def manage_notes(*args, **kwargs): + return {"owner": kwargs["owner"], "ok": True} + + monkeypatch.setattr(codex_routes, "do_manage_notes", manage_notes) router = codex_routes.setup_codex_routes() - bearer = _request(scopes=("chat", "cookbook:read", "cookbook:launch")) - direct_cases = [ - ("/api/codex/capabilities", "GET", (bearer,)), + bearer = _request(scopes=("chat", "todos:read", "cookbook:read", "cookbook:launch")) + capabilities = next( + route.endpoint + for route in router.routes + if route.path == "/api/codex/capabilities" and "GET" in route.methods + ) + assert capabilities(bearer)["tools"]["todos"]["read"] is True + + todos = next( + route.endpoint + for route in router.routes + if route.path == "/api/codex/todos" and "GET" in route.methods + ) + assert await todos(bearer) == {"owner": "alice", "ok": True} + + with pytest.raises(HTTPException) as missing_scope: + await todos(_request(scopes=("chat",))) + assert missing_scope.value.status_code == 403 + + direct_host_cases = [ ("/api/codex/plugin.zip", "GET", (bearer,)), ("/api/codex/cookbook/tasks", "GET", (bearer,)), ("/api/codex/cookbook/serve", "POST", (bearer, {})), ("/api/codex/cookbook/output/{session_id}", "GET", (bearer, "serve-1")), ] - for path, method, args in direct_cases: + for path, method, args in direct_host_cases: endpoint = next( route.endpoint for route in router.routes @@ -1027,11 +1180,19 @@ async def test_codex_bearer_rejected_before_direct_and_router_host_control(monke headers = { "x-api-token": "1", "x-api-owner": "alice", - "x-api-scopes": "cookbook:read,cookbook:launch", + "x-api-scopes": "todos:read,cookbook:read,cookbook:launch", } async with _client(_PrincipalState(app)) as client: + capabilities_response = await client.get("/api/codex/capabilities", headers=headers) + assert capabilities_response.status_code == 200, capabilities_response.text + assert capabilities_response.json()["tools"]["todos"]["read"] is True + + todos_response = await client.get("/api/codex/todos", headers=headers) + assert todos_response.status_code == 200, todos_response.text + assert todos_response.json() == {"owner": "alice", "ok": True} + for method, path, kwargs in ( - ("GET", "/api/codex/capabilities", {}), + ("GET", "/api/codex/plugin.zip", {}), ("GET", "/api/codex/cookbook/tasks", {}), ("POST", "/api/codex/cookbook/serve", {"json": {}}), ): diff --git a/tests/test_api_token_security_cycle4.py b/tests/test_api_token_security_cycle4.py index b783671b4..55bedc316 100644 --- a/tests/test_api_token_security_cycle4.py +++ b/tests/test_api_token_security_cycle4.py @@ -300,6 +300,97 @@ def test_session_creation_passes_bearer_no_live_capability_to_model_validation(m assert seen["allow_live_probes"] is False +def test_bearer_session_creation_uses_pinned_only_cache_inventory(monkeypatch): + from routes import session_routes as sr + from src import database, llm_core + + endpoint = SimpleNamespace( + id="ep", + is_enabled=True, + base_url="https://api.example.test/v1", + api_key=None, + endpoint_kind="api", + cached_models=json.dumps(["stale-cached-model"]), + pinned_models=json.dumps(["server-pinned-model"]), + hidden_models=json.dumps(["stale-cached-model"]), + ) + db = _EndpointDb(endpoint) + monkeypatch.setattr(sr, "SessionLocal", lambda: db) + monkeypatch.setattr(database, "SessionLocal", lambda: db) + monkeypatch.setattr(sr, "_reject_raw_endpoint_url_for_non_admin", lambda *args, **kwargs: None) + monkeypatch.setattr( + llm_core, + "httpx_get_kimi_aware", + lambda *args, **kwargs: (_ for _ in ()).throw( + AssertionError("bearer setup attempted a live model probe") + ), + ) + manager = SimpleNamespace( + create_session=lambda **kwargs: SimpleNamespace( + id=kwargs["session_id"], + name=kwargs["name"], + model=kwargs["model"], + endpoint_url=kwargs["endpoint_url"], + rag=kwargs["rag"], + headers={}, + ), + ) + router = sr.setup_session_routes(manager, {}) + create_session = _endpoint(router, "/api/session", "POST") + + result = create_session( + request=_Request(), + name="chat", + endpoint_url="", + model="", + rag=None, + skip_validation=None, + api_key="", + endpoint_id="ep", + ) + + assert result.model == "server-pinned-model" + + +def test_bearer_cache_only_model_normalization_rejects_forbidden_fallback(monkeypatch): + from routes import chat_helpers + from src import database, llm_core + + endpoint = SimpleNamespace( + id="ep", + is_enabled=True, + base_url="https://api.example.test/v1", + endpoint_kind="api", + cached_models=json.dumps(["stale-cached-model"]), + pinned_models=json.dumps(["server-pinned-model"]), + hidden_models=json.dumps(["stale-cached-model"]), + ) + db = _EndpointDb(endpoint) + monkeypatch.setattr(chat_helpers, "SessionLocal", lambda: db) + monkeypatch.setattr(database, "SessionLocal", lambda: db) + monkeypatch.setattr( + llm_core, + "httpx_get_kimi_aware", + lambda *args, **kwargs: (_ for _ in ()).throw( + AssertionError("cache-only normalization attempted a live probe") + ), + ) + + allowed = SimpleNamespace( + endpoint_url="https://api.example.test/v1/chat/completions", + model="server-pinned-model", + owner="alice", + ) + forbidden = SimpleNamespace( + endpoint_url=allowed.endpoint_url, + model="stale-cached-model", + owner="alice", + ) + + assert chat_helpers._normalize_model_id_from_cache(allowed) == "server-pinned-model" + assert chat_helpers._normalize_model_id_from_cache(forbidden) is None + + def test_explicit_bearer_model_does_not_require_live_setup_probe(monkeypatch): from routes import session_routes as sr from src import llm_core @@ -530,6 +621,9 @@ class _EndpointDb: def first(self): return self.endpoint + def all(self): + return [self.endpoint] + def close(self): return None @@ -545,7 +639,9 @@ async def test_sync_chat_fallback_uses_cached_models_without_provider_probe(monk created_at=1, base_url="http://127.0.0.1:11434/v1", api_key="configured-key", - cached_models=json.dumps(["cached-model"]), + cached_models=json.dumps(["stale-cached-model"]), + pinned_models=json.dumps(["server-pinned-model"]), + hidden_models=json.dumps(["stale-cached-model"]), provider_auth_id=None, ) monkeypatch.setattr(wr, "SessionLocal", lambda: _EndpointDb(endpoint)) @@ -590,5 +686,5 @@ async def test_sync_chat_fallback_uses_cached_models_without_provider_probe(monk provider=None, ) result = await sync_chat(request=_Request(), body=body) - assert result["model"] == "cached-model" + assert result["model"] == "server-pinned-model" assert seen["allow_live_probes"] is False diff --git a/tests/test_session_endpoint_owner_scope.py b/tests/test_session_endpoint_owner_scope.py index e1ea50588..64de1c962 100644 --- a/tests/test_session_endpoint_owner_scope.py +++ b/tests/test_session_endpoint_owner_scope.py @@ -49,9 +49,10 @@ def test_chat_endpoint_recovery_paths_are_owner_scoped(): chat_routes = (root / "routes" / "chat_routes.py").read_text(encoding="utf-8") chat_helpers = (root / "routes" / "chat_helpers.py").read_text(encoding="utf-8") - assert "def _clear_orphaned_session_endpoint(sess, owner:" in chat_routes + assert "def _clear_orphaned_session_endpoint(" in chat_routes assert "def _recover_empty_session_model(sess, session_id: str, owner:" in chat_routes assert "q = owner_filter(q, ModelEndpoint, owner)" in chat_routes - assert "resolve_session_auth(sess, session, owner=effective_user(request))" in chat_routes - assert "def resolve_session_auth(sess, session_id: str, owner:" in chat_helpers + assert "allow_live_probes=request_capability.allow_live_probes" in chat_routes + assert "def resolve_session_auth(" in chat_helpers + assert "allow_live_probes: bool = True" in chat_helpers assert "update_q = update_q.filter(DBSession.owner == owner)" in chat_helpers