fix(research): gate /api/research/spinoff on session ownership (#878)

The spinoff endpoint authenticated the caller (_require_user) but never
verified the research session belonged to them before reading the
persisted report and seeding it into a new chat session owned by the
caller. Any authenticated user who knew or guessed another user's
research session ID could exfiltrate that user's full report into their
own session — a cross-user data disclosure (IDOR).

Every other endpoint in this router gates on _owns_in_memory /
_assert_owns_research right after validating the session ID; spinoff was
the lone exception. Add the same _owns_in_memory check (covers both the
in-memory task and the on-disk JSON) so a non-owner gets a 404 before any
data is read or a session is created.

Add regression tests pinning the anonymous (401) and wrong-owner (404)
cases.
This commit is contained in:
Mahdi Salmanzade
2026-06-02 07:26:12 +04:00
committed by GitHub
parent fca8d68aba
commit 66cd44b66d
2 changed files with 36 additions and 2 deletions

View File

@@ -492,8 +492,14 @@ def setup_research_routes(research_handler, session_manager=None) -> APIRouter:
injects a single system message containing the report and sources so
the user can ask follow-up questions in a clean conversation.
"""
_require_user(request)
user = _require_user(request)
_validate_session_id(session_id)
# SECURITY: gate on ownership before reading the persisted research —
# otherwise any authenticated user could spin off (and thereby read)
# another user's report by guessing its session ID. Mirrors every other
# endpoint in this file (see result_peek above).
if not _owns_in_memory(session_id, user):
raise HTTPException(404, "No research found for this session")
if session_manager is None:
raise HTTPException(500, "session_manager not configured")
@@ -574,7 +580,6 @@ def setup_research_routes(research_handler, session_manager=None) -> APIRouter:
# Create new session
new_sid = str(uuid.uuid4())
user = get_current_user(request)
title_query = (query or "research").strip()
if len(title_query) > 60:

View File

@@ -177,6 +177,35 @@ def test_research_delete_rejects_anonymous():
assert exc.value.status_code == 401
def test_research_spinoff_rejects_anonymous():
"""spinoff must 401 before reading any research data."""
from routes.research_routes import setup_research_routes
rh = MagicMock()
router = setup_research_routes(rh, session_manager=MagicMock())
target = next(r.endpoint for r in router.routes if getattr(r, "path", "") == "/api/research/spinoff/{session_id}")
with pytest.raises(HTTPException) as exc:
asyncio.run(target(session_id="x", request=_fake_request(user=None)))
assert exc.value.status_code == 401
def test_research_spinoff_rejects_wrong_owner():
"""A user must not be able to spin off (and thereby read) another user's
research report. The ownership gate must 404 before any data is read or a
new session is created. Regression for the cross-user disclosure IDOR."""
from routes.research_routes import setup_research_routes
sm = MagicMock()
rh = MagicMock()
rh._active_tasks = {"x": {"owner": "alice"}}
rh.get_result.return_value = "TOP SECRET REPORT"
router = setup_research_routes(rh, session_manager=sm)
target = next(r.endpoint for r in router.routes if getattr(r, "path", "") == "/api/research/spinoff/{session_id}")
with pytest.raises(HTTPException) as exc:
asyncio.run(target(session_id="x", request=_fake_request(user="bob")))
assert exc.value.status_code == 404
# The attacker must never get a session created on their behalf.
sm.create_session.assert_not_called()
# ---------------------------------------------------------------------------
# pop_notifications owner filter
# ---------------------------------------------------------------------------