fix: scope chat active-document lookup to the session owner (#569)

This commit is contained in:
Rasmus
2026-06-02 04:46:40 +02:00
committed by GitHub
parent f13d897093
commit e73f3edc06
2 changed files with 24 additions and 3 deletions
+7 -3
View File
@@ -23,6 +23,7 @@ from src.prompt_security import untrusted_context_message
from core.exceptions import SessionNotFoundError from core.exceptions import SessionNotFoundError
from src.auth_helpers import get_current_user from src.auth_helpers import get_current_user
from routes.session_routes import _verify_session_owner from routes.session_routes import _verify_session_owner
from routes.document_helpers import _owner_session_filter
from core.database import SessionLocal, get_session_mode, set_session_mode from core.database import SessionLocal, get_session_mode, set_session_mode
from core.database import Session as DBSession, ChatMessage as DBChatMessage from core.database import Session as DBSession, ChatMessage as DBChatMessage
from core.database import Document as DBDocument, ModelEndpoint from core.database import Document as DBDocument, ModelEndpoint
@@ -424,9 +425,12 @@ def setup_chat_routes(
try: try:
if active_doc_id: if active_doc_id:
logger.info(f"[doc-inject] active_doc_id from frontend: {active_doc_id}") logger.info(f"[doc-inject] active_doc_id from frontend: {active_doc_id}")
active_doc = _doc_db.query(DBDocument).filter( # Scope to the caller's documents. The session and in-memory
DBDocument.id == active_doc_id, # fallbacks below are already owner/session-bound; this
).first() # explicit-id path looked up by id alone, so a user could
# inject another user's document by passing its id.
_doc_q = _doc_db.query(DBDocument).filter(DBDocument.id == active_doc_id)
active_doc = _owner_session_filter(_doc_q, ctx.user).first()
if active_doc: if active_doc:
logger.info(f"[doc-inject] found by ID: title={active_doc.title!r}, lang={active_doc.language!r}, is_active={active_doc.is_active}, content_len={len(active_doc.current_content or '')}") logger.info(f"[doc-inject] found by ID: title={active_doc.title!r}, lang={active_doc.language!r}, is_active={active_doc.is_active}, content_len={len(active_doc.current_content or '')}")
else: else:
+17
View File
@@ -929,3 +929,20 @@ def test_mcp_oauth_page_escapes_reflected_values():
body = text.split("def _oauth_authorize_page(", 1)[1].split("return f", 1)[0] body = text.split("def _oauth_authorize_page(", 1)[1].split("return f", 1)[0]
for var in ("auth_url", "server_id", "host"): for var in ("auth_url", "server_id", "host"):
assert f"{var} = html.escape({var}" in body, var assert f"{var} = html.escape({var}" in body, var
def test_chat_active_document_lookup_is_owner_scoped():
"""The explicit `active_doc_id` path in /api/chat_stream must scope the
document lookup to the caller. Resolving by id alone let any user inject
another user's document into their own chat context (the session and
in-memory fallbacks were already owner/session-bound; this branch wasn't)."""
import re
src = Path(__file__).resolve().parents[1] / "routes" / "chat_routes.py"
text = src.read_text()
# The frontend-supplied id is resolved through the shared owner filter.
assert "_owner_session_filter(_doc_q, ctx.user)" in text
# And never by id alone (the previous IDOR shape, whitespace-insensitive).
flat = re.sub(r"\s+", " ", text)
assert "filter( DBDocument.id == active_doc_id, ).first()" not in flat
assert "filter(DBDocument.id == active_doc_id).first()" not in flat