From e73f3edc06ada5c8fa15d0c1070e767003017ba8 Mon Sep 17 00:00:00 2001 From: Rasmus <113692639+RasmusLiltorp@users.noreply.github.com> Date: Tue, 2 Jun 2026 04:46:40 +0200 Subject: [PATCH] fix: scope chat active-document lookup to the session owner (#569) --- routes/chat_routes.py | 10 +++++++--- tests/test_security_regressions.py | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/routes/chat_routes.py b/routes/chat_routes.py index 84143d0..764c7e0 100644 --- a/routes/chat_routes.py +++ b/routes/chat_routes.py @@ -23,6 +23,7 @@ from src.prompt_security import untrusted_context_message from core.exceptions import SessionNotFoundError from src.auth_helpers import get_current_user 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 Session as DBSession, ChatMessage as DBChatMessage from core.database import Document as DBDocument, ModelEndpoint @@ -424,9 +425,12 @@ def setup_chat_routes( try: if active_doc_id: logger.info(f"[doc-inject] active_doc_id from frontend: {active_doc_id}") - active_doc = _doc_db.query(DBDocument).filter( - DBDocument.id == active_doc_id, - ).first() + # Scope to the caller's documents. The session and in-memory + # fallbacks below are already owner/session-bound; this + # 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: 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: diff --git a/tests/test_security_regressions.py b/tests/test_security_regressions.py index 4aa1443..38a1c52 100644 --- a/tests/test_security_regressions.py +++ b/tests/test_security_regressions.py @@ -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] for var in ("auth_url", "server_id", "host"): 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