mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-09-25 17:42:20 +02:00
fix(security): close API token agent authorization gaps
This commit is contained in:
+64
-14
@@ -4,12 +4,17 @@ import os
|
||||
from typing import Optional
|
||||
from fastapi import Request, HTTPException
|
||||
|
||||
from src.owner_identity import auth_disabled, effective_storage_owner
|
||||
from src.owner_identity import (
|
||||
auth_disabled,
|
||||
effective_storage_owner,
|
||||
is_request_sentinel_owner,
|
||||
)
|
||||
|
||||
|
||||
def get_current_user(request: Request) -> Optional[str]:
|
||||
"""Get current username from request state (set by auth middleware)."""
|
||||
return getattr(request.state, 'current_user', None)
|
||||
state = getattr(request, "state", None)
|
||||
return getattr(state, "current_user", None)
|
||||
|
||||
|
||||
def effective_user(request: Request) -> Optional[str]:
|
||||
@@ -29,29 +34,58 @@ def effective_user(request: Request) -> Optional[str]:
|
||||
owner falls back to :func:`get_current_user` (the "api" pseudo-user), so it
|
||||
never escalates.
|
||||
"""
|
||||
if getattr(request.state, "api_token", False):
|
||||
owner = getattr(request.state, "api_token_owner", None)
|
||||
if owner:
|
||||
return owner
|
||||
if _is_api_token_request(request):
|
||||
state = getattr(request, "state", None)
|
||||
owner = getattr(state, "api_token_owner", None)
|
||||
if isinstance(owner, str) and owner.strip():
|
||||
return owner.strip()
|
||||
return get_current_user(request)
|
||||
|
||||
|
||||
def _is_api_token_request(request: Request) -> bool:
|
||||
"""Return True when middleware authenticated a bearer API token."""
|
||||
return bool(getattr(request.state, "api_token", False))
|
||||
state = getattr(request, "state", None)
|
||||
return getattr(state, "api_token", False) is True
|
||||
|
||||
|
||||
def require_api_token_owner(request: Request) -> str:
|
||||
"""Return a real owner for a bearer request, failing closed otherwise.
|
||||
|
||||
The middleware normally resolves token owners against configured human
|
||||
accounts. Keep that invariant at route boundaries too: direct endpoint
|
||||
tests, alternate ASGI entry points, and future middleware changes must not
|
||||
turn a request sentinel or an ownerless token into a durable/executable
|
||||
owner.
|
||||
"""
|
||||
state = getattr(request, "state", None)
|
||||
owner = getattr(state, "api_token_owner", None)
|
||||
if (
|
||||
not isinstance(owner, str)
|
||||
or not owner.strip()
|
||||
or is_request_sentinel_owner(owner)
|
||||
):
|
||||
raise HTTPException(403, "API token has no owner")
|
||||
return owner.strip()
|
||||
|
||||
|
||||
def require_api_token_scope(request: Request, required_scope: str) -> Optional[str]:
|
||||
"""Require one declared scope for bearer callers; leave browser callers unchanged."""
|
||||
if not _is_api_token_request(request):
|
||||
return effective_user(request)
|
||||
scopes = set(getattr(request.state, "api_token_scopes", []) or [])
|
||||
if required_scope not in scopes:
|
||||
state = getattr(request, "state", None)
|
||||
raw_scopes = getattr(state, "api_token_scopes", None)
|
||||
if isinstance(raw_scopes, (list, tuple, set, frozenset)):
|
||||
scopes = {
|
||||
value.strip().casefold()
|
||||
for value in raw_scopes
|
||||
if isinstance(value, str) and value.strip()
|
||||
}
|
||||
else:
|
||||
scopes = set()
|
||||
normalized_scope = str(required_scope or "").strip().casefold()
|
||||
if not normalized_scope or normalized_scope not in scopes:
|
||||
raise HTTPException(403, f"API token missing required scope: {required_scope}")
|
||||
owner = getattr(request.state, "api_token_owner", None)
|
||||
if not owner:
|
||||
raise HTTPException(403, "API token has no owner")
|
||||
return owner
|
||||
return require_api_token_owner(request)
|
||||
|
||||
|
||||
def require_chat_scope(request: Request) -> Optional[str]:
|
||||
@@ -59,6 +93,22 @@ def require_chat_scope(request: Request) -> Optional[str]:
|
||||
return require_api_token_scope(request, "chat")
|
||||
|
||||
|
||||
def require_interactive_request(request: Request) -> Optional[str]:
|
||||
"""Reject bearer integrations from browser-only agent/control surfaces.
|
||||
|
||||
This is deliberately a bearer-principal gate rather than an authentication
|
||||
requirement. Cookie sessions and AUTH_ENABLED=false keep their existing
|
||||
route behavior, while API tokens cannot enter routes that start, resume,
|
||||
approve, or otherwise control interactive agent work.
|
||||
"""
|
||||
current_user = get_current_user(request)
|
||||
if _is_api_token_request(request) or (
|
||||
isinstance(current_user, str) and current_user.strip().casefold() == "api"
|
||||
):
|
||||
raise HTTPException(403, "API tokens cannot use this interactive surface")
|
||||
return current_user
|
||||
|
||||
|
||||
def enforce_api_token_chat_controls(
|
||||
request: Request,
|
||||
*,
|
||||
@@ -88,7 +138,7 @@ def require_authenticated_request(request: Request) -> str:
|
||||
sessions or their own API-token scope/owner gate.
|
||||
"""
|
||||
if _is_api_token_request(request):
|
||||
return effective_user(request) or ""
|
||||
return require_api_token_owner(request)
|
||||
return require_user(request)
|
||||
|
||||
|
||||
|
||||
@@ -274,6 +274,7 @@ class ChatProcessor:
|
||||
agent_mode: bool = False,
|
||||
incognito: bool = False,
|
||||
use_skills: bool = True,
|
||||
allow_tool_preprocessing: bool = True,
|
||||
) -> Tuple[List[Dict[str, str]], List[Dict[str, Any]], List[Dict[str, str]]]:
|
||||
"""Build the context preface for LLM calls.
|
||||
|
||||
@@ -457,7 +458,7 @@ class ChatProcessor:
|
||||
# hundreds of KB of duplicate page HTML and confuses the model) or for
|
||||
# link-heavy pastes (>3 URLs typically means it's a boilerplate-laden
|
||||
# blog post, not a "summarize this URL" request).
|
||||
urls = extract_urls(message)
|
||||
urls = extract_urls(message) if allow_tool_preprocessing else []
|
||||
non_yt_urls = [u for u in urls if not is_youtube_url(u)]
|
||||
skip_url_fetch = len(message) > 2000 or len(non_yt_urls) > 3
|
||||
if not skip_url_fetch:
|
||||
|
||||
+15
-5
@@ -1,6 +1,6 @@
|
||||
"""Trust-boundary helpers for client-supplied chat metadata."""
|
||||
|
||||
from typing import Any
|
||||
from typing import Any, Optional
|
||||
|
||||
from src.tool_approval_scopes import CHAT_SESSION_APPROVAL_CONTEXT_MARKER
|
||||
|
||||
@@ -11,12 +11,22 @@ _SERVER_OWNED_MESSAGE_METADATA = frozenset({
|
||||
})
|
||||
|
||||
|
||||
def sanitize_client_message_metadata(metadata: Any) -> Any:
|
||||
"""Drop fields that can only be produced by server-side tool execution."""
|
||||
def sanitize_client_message_metadata(metadata: Any) -> Optional[dict]:
|
||||
"""Normalize client metadata and drop server-owned fields.
|
||||
|
||||
Client metadata is only a JSON object. In particular, do not let a
|
||||
list-of-pairs value reach ``dict.update``: that mapping-compatible shape
|
||||
can smuggle protected approval fields through an otherwise safe merge.
|
||||
Malformed metadata is normalized away; server-generated metadata remains
|
||||
untouched because this helper is called only at client ingress points.
|
||||
"""
|
||||
if metadata is None:
|
||||
return None
|
||||
if not isinstance(metadata, dict):
|
||||
return metadata
|
||||
return {
|
||||
return None
|
||||
sanitized = {
|
||||
key: value
|
||||
for key, value in metadata.items()
|
||||
if key not in _SERVER_OWNED_MESSAGE_METADATA
|
||||
}
|
||||
return sanitized or None
|
||||
|
||||
Reference in New Issue
Block a user