fix(security): isolate bearer authorization paths

This commit is contained in:
RaresKeY
2026-08-28 21:25:23 +00:00
parent 9150a453b4
commit 50c8675a21
24 changed files with 1173 additions and 160 deletions
+295
View File
@@ -552,3 +552,298 @@ async def test_stream_bearer_chat_cannot_dispatch_image_generation(monkeypatch):
assert exc.value.status_code == 403
assert "image" in str(exc.value.detail).lower()
assert "chat" not in captured
def test_nested_legacy_approval_shapes_are_stripped_on_ingress_and_projection():
from src.message_metadata import sanitize_projected_message_metadata
metadata = {
"safe": {"label": "keep"},
CHAT_SESSION_APPROVAL_CONTEXT_MARKER: True,
"approval_id": "legacy-root",
"resolved": "approve",
"session_id": "session-1",
"tool_events": [
{
"ask_user": {
"kind": "tool_approval",
"approval_id": "legacy-nested",
"resolved": "approve",
"approved_by_interactive_session": True,
"session_id": "session-1",
"label": "Allow",
}
},
{
"kind": "tool_approval",
"approval_id": "legacy-direct",
"resolved": "approve",
"session_id": "session-1",
"label": "Allow",
},
["not-a-metadata-mapping"],
],
}
client = sanitize_client_message_metadata(metadata)
assert client == {"safe": {"label": "keep"}}
projected = sanitize_projected_message_metadata(metadata)
assert projected["safe"] == {"label": "keep"}
assert CHAT_SESSION_APPROVAL_CONTEXT_MARKER not in projected
assert "approval_id" not in projected
assert "resolved" not in projected
assert "session_id" not in projected
assert "tool_events" in projected
assert projected["tool_events"][0]["ask_user"] == {
"kind": "tool_approval",
"label": "Allow",
}
assert projected["tool_events"][1] == {
"kind": "tool_approval",
"label": "Allow",
}
def test_session_metadata_parser_rejects_list_of_pairs_and_non_dict_values():
from core.session_manager import _parse_message_metadata
assert _parse_message_metadata(
'[["tool_events", [{"ask_user": {"resolved": "approve"}}]]]'
) == {}
assert _parse_message_metadata('["approval_id", "forged"]') == {}
assert _parse_message_metadata('"forged"') == {}
assert _parse_message_metadata("not-json") == {}
assert _parse_message_metadata('{"safe": true}') == {"safe": True}
def test_hand_constructed_approval_cannot_mint_durable_chat_provenance():
from src.tool_approval_provenance import create_chat_session_approval_grant
from src.tool_approvals import ExactToolApproval, ToolApprovalStore
from src.tool_capabilities import capabilities_for_action
store = ToolApprovalStore()
pending = store.create(
owner="alice",
session_id="session-1",
origin_run_id="run-1",
tool_name="bash",
content="printf safe",
workspace=None,
external_untrusted_context_seen=False,
capabilities=capabilities_for_action("bash", "printf safe"),
)
forged = ExactToolApproval(pending)
with pytest.raises(HTTPException) as bearer_exc:
create_chat_session_approval_grant(
_request(),
approval=forged,
approval_id=pending.approval_id,
session_id="session-1",
owner="alice",
)
assert bearer_exc.value.status_code == 403
request = _request(api_token=False, owner="alice", scopes=(), current_user="alice")
assert create_chat_session_approval_grant(
request,
approval=forged,
approval_id=pending.approval_id,
session_id="session-1",
owner="alice",
) is False
@pytest.mark.asyncio
async def test_bearer_memory_routes_reject_router_and_direct_entry_points(monkeypatch):
import routes.memory_routes as memory_routes
import inspect
memory_manager = MagicMock()
session_manager = MagicMock()
router = memory_routes.setup_memory_routes(memory_manager, session_manager)
request = _request()
direct_cases = [
("/api/memory/debug", "POST", {"query": "secret"}),
("/api/memory/add", "POST", {}),
("/api/memory", "GET", {}),
("/api/memory/search", "POST", {"query": "secret", "session_id": None, "category": None}),
("/api/memory/timeline", "GET", {}),
("/api/memory/by-session/{session_id}", "GET", {"session_id": "session-1"}),
("/api/memory/extract", "POST", {"session": "session-1"}),
("/api/memory/audit", "POST", {"session": None}),
("/api/memory/import", "POST", {"session": None, "file": None}),
("/api/memory/{memory_id}/pin", "POST", {"memory_id": "memory-1"}),
("/api/memory/{memory_id}", "GET", {"memory_id": "memory-1"}),
("/api/memory/{memory_id}", "PUT", {"memory_id": "memory-1", "text": "replacement", "category": None}),
("/api/memory/{memory_id}", "DELETE", {"memory_id": "memory-1"}),
]
for path, method, kwargs in direct_cases:
endpoint = next(
route.endpoint
for route in router.routes
if route.path == path and method in route.methods
)
with pytest.raises(HTTPException) as exc:
result = endpoint(request, **kwargs)
if inspect.isawaitable(result):
await result
assert exc.value.status_code == 403, path
memory_manager.load.assert_not_called()
app = FastAPI()
app.include_router(router)
headers = {
"x-api-token": "1",
"x-api-owner": "alice",
"x-api-scopes": "chat",
}
async with _client(_PrincipalState(app)) as client:
response = await client.get("/api/memory", headers=headers)
assert response.status_code == 403
@pytest.mark.asyncio
async def test_bearer_capability_suppresses_deferred_callbacks_and_message_events(monkeypatch):
from routes import chat_helpers
from src.auth_helpers import request_capability
request = _request()
capability = request_capability(request)
assert capability.is_bearer is True
assert capability.allow_deferred_work is False
assert capability.allow_detached_execution is False
assert capability.allow_message_events is False
assert capability.allow_auto_naming is False
sess = SimpleNamespace(
history=[object()] * 8,
endpoint_url="https://selected.example/v1",
model="selected-model",
headers={"Authorization": "Bearer selected"},
name="New chat",
add_message=MagicMock(),
)
webhook_manager = MagicMock()
monkeypatch.setattr(
chat_helpers,
"_spawn_bg",
lambda *args, **kwargs: (_ for _ in ()).throw(AssertionError("bearer scheduled work")),
)
monkeypatch.setattr(
chat_helpers,
"accumulate_token_usage",
lambda *args, **kwargs: (_ for _ in ()).throw(AssertionError("bearer usage callback")),
)
chat_helpers.run_post_response_tasks(
sess,
SimpleNamespace(),
"session-1",
"hello",
"answer",
{"prompt_tokens": 1},
{"auto_memory": True, "auto_skills": True},
MagicMock(),
MagicMock(),
webhook_manager,
agent_rounds=3,
agent_tool_calls=3,
skills_manager=MagicMock(),
owner="alice",
capability=capability,
)
webhook_manager.fire_and_forget.assert_not_called()
with_marker = SimpleNamespace(
get_context_messages=lambda: [{
"role": "user",
"content": "prior",
"metadata": {CHAT_SESSION_APPROVAL_CONTEXT_MARKER: True, "safe": "yes"},
}]
)
projected = chat_helpers._history_for_request_capability(with_marker, capability)
assert projected == [{"role": "user", "content": "prior", "metadata": {"safe": "yes"}}]
chat_helpers.fire_message_event(
request,
webhook_manager,
"session-1",
sess,
"hello",
capability=capability,
)
webhook_manager.fire_and_forget.assert_not_called()
chat_handler = MagicMock()
chat_helpers.add_user_message(
sess,
chat_handler,
SimpleNamespace(
attachment_meta=[],
user_content="hello",
text_for_context="hello",
),
capability=capability,
)
chat_handler.update_session_name_if_needed.assert_not_called()
def test_bearer_cannot_reach_workspace_or_hwfit_direct_handlers(monkeypatch):
from routes import hwfit_routes, workspace_routes
bearer = _request()
workspace_router = workspace_routes.setup_workspace_routes()
browse = next(route.endpoint for route in workspace_router.routes if route.path == "/api/workspace/browse")
vet = next(route.endpoint for route in workspace_router.routes if route.path == "/api/workspace/vet")
with pytest.raises(HTTPException):
browse(bearer, path="/")
with pytest.raises(HTTPException):
vet(bearer, path="/")
hwfit_router = hwfit_routes.setup_hwfit_routes()
for path in ("/api/hwfit/system", "/api/hwfit/models", "/api/hwfit/profiles", "/api/hwfit/image-models"):
endpoint = next(route.endpoint for route in hwfit_router.routes if route.path == path)
with pytest.raises(HTTPException):
endpoint(request=bearer)
@pytest.mark.asyncio
async def test_codex_bearer_rejected_before_direct_and_router_host_control(monkeypatch):
import routes.codex_routes as codex_routes
router = codex_routes.setup_codex_routes()
bearer = _request(scopes=("chat", "cookbook:read", "cookbook:launch"))
direct_cases = [
("/api/codex/capabilities", "GET", (bearer,)),
("/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:
endpoint = next(
route.endpoint
for route in router.routes
if route.path == path and method in route.methods
)
with pytest.raises(HTTPException) as exc:
result = endpoint(*args)
if hasattr(result, "__await__"):
await result
assert exc.value.status_code == 403
app = FastAPI()
app.include_router(router)
headers = {
"x-api-token": "1",
"x-api-owner": "alice",
"x-api-scopes": "cookbook:read,cookbook:launch",
}
async with _client(_PrincipalState(app)) as client:
for method, path, kwargs in (
("GET", "/api/codex/capabilities", {}),
("GET", "/api/codex/cookbook/tasks", {}),
("POST", "/api/codex/cookbook/serve", {"json": {}}),
):
response = await client.request(method, path, headers=headers, **kwargs)
assert response.status_code == 403, (path, response.text)
+7 -6
View File
@@ -5,8 +5,8 @@ routes (tasks, servers, output, stop, adopt, presets, etc.) through
normal cookie sessions because _scope_owner only checked login status,
not admin privileges.
After the fix, cookie-session callers must be admin; API-token callers
are still governed by scope checks only.
After the fix, cookie-session callers must be admin and bearer callers are
rejected from the Codex host-control plane regardless of legacy scopes.
"""
import pytest
from types import SimpleNamespace
@@ -80,13 +80,14 @@ class TestCookieSessionAdminGate:
class TestApiTokenScopeGate:
"""API-token callers are governed by scope, not admin status."""
"""Bearer callers cannot enter Codex host-control routes."""
def test_token_with_scope_allowed(self, monkeypatch):
def test_token_with_legacy_scope_rejected(self, monkeypatch):
monkeypatch.setenv("AUTH_ENABLED", "true")
req = _api_token_request(scopes=["cookbook:read"])
owner = _require_cookbook_scope(req, COOKBOOK_READ_SCOPES)
assert owner == "alice"
with pytest.raises(HTTPException) as exc:
_require_cookbook_scope(req, COOKBOOK_READ_SCOPES)
assert exc.value.status_code == 403
def test_token_missing_scope_rejected(self, monkeypatch):
monkeypatch.setenv("AUTH_ENABLED", "true")
+33 -8
View File
@@ -8,6 +8,7 @@ These pin validation on the host/port before they reach the ssh string, matching
the validators the rest of the cookbook routes already apply.
"""
import asyncio
from types import SimpleNamespace
import pytest
from fastapi import APIRouter, HTTPException
@@ -56,6 +57,30 @@ def _codex_request(scopes) -> Request:
return request
def _interactive_request(path="/api/codex/documents") -> Request:
app = SimpleNamespace(
state=SimpleNamespace(
auth_manager=SimpleNamespace(
is_configured=True,
is_admin=lambda username: username == "alice",
)
)
)
request = Request(
{
"type": "http",
"method": "GET",
"path": path,
"headers": [],
"state": {},
"app": app,
}
)
request.state.current_user = "alice"
request.state.api_token = False
return request
def test_rejects_remote_host_with_shell_metacharacters():
task = {"remoteHost": "box; rm -rf ~", "sshPort": ""}
with pytest.raises(HTTPException) as exc:
@@ -135,7 +160,7 @@ def _documents_endpoint(total: int):
async def test_documents_pagination_clamps_offset_and_limit():
endpoint, calls = _documents_endpoint(total=99)
result = await endpoint(_codex_request(["documents:read"]), offset=-10, limit=500)
result = await endpoint(_interactive_request(), offset=-10, limit=500)
assert calls[-1]["owner"] == "alice"
assert calls[-1]["offset"] == 0
@@ -148,7 +173,7 @@ async def test_documents_pagination_clamps_offset_and_limit():
async def test_documents_pagination_clamps_zero_limit_to_one():
endpoint, calls = _documents_endpoint(total=3)
result = await endpoint(_codex_request(["documents:read"]), offset=0, limit=0)
result = await endpoint(_interactive_request(), offset=0, limit=0)
assert calls[-1]["limit"] == 1
assert len(result["documents"]) == 1
@@ -159,7 +184,7 @@ async def test_documents_pagination_clamps_zero_limit_to_one():
async def test_documents_pagination_returns_next_offset_when_truncated():
endpoint, _calls = _documents_endpoint(total=7)
result = await endpoint(_codex_request(["documents:read"]), offset=2, limit=3)
result = await endpoint(_interactive_request(), offset=2, limit=3)
assert [doc["id"] for doc in result["documents"]] == ["doc-2", "doc-3", "doc-4"]
assert result["next_offset"] == 5
@@ -170,7 +195,7 @@ async def test_documents_pagination_rejects_invalid_offset():
endpoint, _calls = _documents_endpoint(total=7)
with pytest.raises(HTTPException) as exc:
await endpoint(_codex_request(["documents:read"]), offset="soon", limit=3)
await endpoint(_interactive_request(), offset="soon", limit=3)
assert exc.value.status_code == 400
assert exc.value.detail == "Invalid offset"
@@ -181,7 +206,7 @@ async def test_documents_pagination_rejects_invalid_limit():
endpoint, _calls = _documents_endpoint(total=7)
with pytest.raises(HTTPException) as exc:
await endpoint(_codex_request(["documents:read"]), offset=0, limit="many")
await endpoint(_interactive_request(), offset=0, limit="many")
assert exc.value.status_code == 400
assert exc.value.detail == "Invalid limit"
@@ -191,7 +216,7 @@ async def test_documents_pagination_rejects_invalid_limit():
async def test_documents_pagination_out_of_range_offset_returns_empty_page():
endpoint, calls = _documents_endpoint(total=3)
result = await endpoint(_codex_request(["documents:read"]), offset=10, limit=2)
result = await endpoint(_interactive_request(), offset=10, limit=2)
assert calls[-1]["offset"] == 10
assert calls[-1]["limit"] == 2
@@ -217,7 +242,7 @@ def test_adopt_rejects_ssh_option_host_before_shell(monkeypatch, host_field):
}
with pytest.raises(HTTPException) as exc:
asyncio.run(endpoint(_launch_request(), body))
asyncio.run(endpoint(_interactive_request("/api/codex/cookbook/adopt"), body))
assert exc.value.status_code == 400
assert calls == []
@@ -237,7 +262,7 @@ async def test_email_draft_document_accepts_send_scope_with_document_write():
endpoint = _route_endpoint("/api/codex/emails/draft-document", "POST", router=router)
result = await endpoint(
_codex_request(["email:send", "documents:write"]),
_interactive_request("/api/codex/emails/draft-document"),
{"to": "recipient@example.com", "subject": "Subject", "body": "Body"},
)
+1 -1
View File
@@ -69,7 +69,7 @@ def _build_context_harness(monkeypatch, chat_helpers, history):
temperature=0.7, max_tokens=1024, system_prompt="You are Odysseus.", character_name=None,
)
def fake_add_user_message(sess, chat_handler, preprocessed, incognito=False):
def fake_add_user_message(sess, chat_handler, preprocessed, incognito=False, capability=None):
sess.messages.append({"role": "user", "content": preprocessed.user_content})
async def fake_maybe_compact(sess, endpoint_url, model, messages, headers, owner=None):
+57 -1
View File
@@ -1744,6 +1744,11 @@ def test_api_models_scopes_api_token_to_token_owner(monkeypatch):
monkeypatch.setattr(model_routes, "ModelEndpoint", _RouteModelEndpoint)
monkeypatch.setattr(model_routes, "SessionLocal", lambda: db)
monkeypatch.setattr(threading, "Thread", _NoopThread)
monkeypatch.setattr(
model_routes,
"_disable_stale_cookbook_local_endpoints",
lambda _db: (_ for _ in ()).throw(AssertionError("bearer read touched stale-row state")),
)
request = SimpleNamespace(
state=SimpleNamespace(
@@ -1765,7 +1770,58 @@ 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_bearer_model_refresh_and_background_flags_fail_before_management_work(monkeypatch):
router = model_routes.setup_model_routes(model_discovery=None)
def fail_session():
raise AssertionError("bearer refresh reached the model database")
monkeypatch.setattr(model_routes, "SessionLocal", fail_session)
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),
),
),
)
endpoint = _route_endpoint(router, "/api/models")
for flags in ({"refresh": True}, {"background": True}, {"refresh": True, "background": True}):
with pytest.raises(HTTPException) as exc:
endpoint(request, **flags)
assert exc.value.status_code == 403
def test_bearer_is_rejected_from_model_management_and_tool_inventory_routes():
from fastapi import Response
router = model_routes.setup_model_routes(model_discovery=None)
request = SimpleNamespace(
state=SimpleNamespace(
current_user="api",
api_token=True,
api_token_owner="alice",
api_token_scopes=["chat"],
),
app=SimpleNamespace(state=SimpleNamespace(auth_manager=None)),
)
with pytest.raises(HTTPException) as tools_exc:
_route_endpoint(router, "/api/tools")(request)
assert tools_exc.value.status_code == 403
with pytest.raises(HTTPException) as endpoint_exc:
_route_endpoint(router, "/api/model-endpoints/{ep_id}/models")(
"endpoint-1", request, Response()
)
assert endpoint_exc.value.status_code == 403
def test_api_models_returns_only_pinned_proxy_models_without_refresh_probe(monkeypatch):
+68 -1
View File
@@ -7,6 +7,8 @@ from pathlib import Path
from types import SimpleNamespace
from core.models import ChatMessage, Session
from core.database import Session as DbSession
from tests.helpers.sqlite_db import make_temp_sqlite
from src.tool_approval_scopes import (
CHAT_SESSION_APPROVAL_CONTEXT_MARKER,
ToolApprovalScope,
@@ -92,7 +94,7 @@ def test_allow_for_task_bypasses_only_the_resumed_run_gate():
assert fresh.decision_for("bash").allowed is False
def test_allow_for_chat_session_applies_to_later_turns_in_only_that_chat():
def test_allow_for_chat_session_applies_to_later_turns_in_only_that_chat(monkeypatch):
store = ToolApprovalStore()
pending = _pending(store, selected_tools=["bash", "manage_skills"])
grant = store.consume(
@@ -109,6 +111,45 @@ def test_allow_for_chat_session_applies_to_later_turns_in_only_that_chat():
assert grant.pending.selected_tools == ("bash", "manage_skills")
assert grant.pending.continuation_query.startswith("inspect the project")
# The browser-shaped resolution card is not enough to create authority.
# Persist the separate grant through the same helper used by the route,
# backed by a fresh database so reload behavior is real rather than a
# monkeypatched history predicate.
monkeypatch.delenv("AUTH_ENABLED", raising=False)
import core.database as database
from src.tool_approval_provenance import create_chat_session_approval_grant
db_factory, _engine, _tmpfile = make_temp_sqlite(database.Base.metadata)
monkeypatch.setattr(database, "SessionLocal", db_factory)
db = db_factory()
try:
db.add(
DbSession(
id="session-1",
name="Chat",
endpoint_url="http://example.invalid",
model="test",
owner="Alice",
)
)
db.commit()
finally:
db.close()
interactive_request = SimpleNamespace(
state=SimpleNamespace(
api_token=False,
current_user="alice",
),
headers={},
)
assert create_chat_session_approval_grant(
interactive_request,
approval=grant,
approval_id=pending.approval_id,
session_id="session-1",
owner="alice",
) is True
resolved_card = pending.public_payload()
resolved_card["resolved"] = "approve"
resolved_card["approved_by_interactive_session"] = True
@@ -125,6 +166,7 @@ def test_allow_for_chat_session_applies_to_later_turns_in_only_that_chat():
name="Chat",
endpoint_url="http://example.invalid",
model="test",
owner="Alice",
history=history,
)
@@ -137,6 +179,31 @@ def test_allow_for_chat_session_applies_to_later_turns_in_only_that_chat():
assert future_turn.approval_gate_bypassed is True
assert future_turn.decision_for("bash").allowed is True
# A fresh in-memory Session object with the same durable id/owner sees the
# grant after a simulated reload; the transcript card itself is still only
# display metadata.
reloaded = Session(
id="session-1",
name="Reloaded",
endpoint_url="http://example.invalid",
model="test",
owner="alice",
history=[ChatMessage("user", "after reload")],
)
assert reloaded.get_context_messages()[-1]["metadata"][CHAT_SESSION_APPROVAL_CONTEXT_MARKER] is True
wrong_owner = Session(
id="session-1",
name="Wrong owner",
endpoint_url="http://example.invalid",
model="test",
owner="bob",
history=[ChatMessage("user", "cross-owner")],
)
assert CHAT_SESSION_APPROVAL_CONTEXT_MARKER not in (
wrong_owner.get_context_messages()[-1].get("metadata") or {}
)
# The persisted card is bound to its original chat id, so a fork/copy does
# not inherit the grant merely by copying transcript metadata.
other_session = Session(