Repository navigation
security(chat): export_session has no ownership check — any authenticated user can export another user's full transcript #14011
Description
Activity
Delivered by PR #14013, merged as
e03a8198e.Verified in the base branch
$ git log origin/Dev_new_gui --oneline --grep=14011 e03a8198e security(chat): require ownership on export_session and reset_chat (#14011) (#14013) export_session Depends(validate_session_ownership) -> present reset_chat await validate_session_ownership(...) -> presentAcceptance criteria — all four met
AC how export_sessionrefuses a session the caller does not ownDepends(validate_session_ownership), the pattern the other eight endpoints in the file usetest: user A cannot export user B's session; the owner still can — behavioural request-level tests via TestClienton a mounted router; a non-owner gets 403, the owner does notevery other endpoint in the file audited by reading the body 13 endpoints; results in the PR further gaps fixed in the same PR or filed reset_chatfixed here;create_sessionfiled as #14012 and since fixed in PR #14017What the audit turned up beyond the filed issue
reset_chat— a second gap of the same class, not in the original report:session_idfrom the body with only a format check, then the conversation is cleared. Fixed here.create_session— worse, and filed separately as security(chat): create_session accepts a client-supplied id with no collision guard — wipes another user's transcript and reassigns ownership #14012: a client-supplied id with no collision guard both wiped the transcript and reassigned ownership. Fixed in PR security(chat): refuse a colliding session id and a silent ownership takeover (#14012) #14017.list_sessionsandclear_session_checkpoints— checked and not gaps (caller-scoped; admin-gated). Recorded so they are not re-audited.
Corrections I made along the way
My first version of the export tests asserted on
inspect.signature, which does not meet this issue's own AC ("behavioural, notinspect.getsource") — and could not have caught the failure most worth catching, a dependency bindingsession_idfrom the query string instead of the path. Replaced with request-level tests.Before treating these endpoints as protected
Both checks route through the shared validator, which fails open (#14010) and whose ownership record expires after 24 hours and is then claimed by whoever accesses the session next (#14018). In a default deployment these calls are inert. They are correct and necessary; the enforcement posture is what decides whether they bite.
Found while verifying the endpoint audit in PR #14009 (#13982). Same vulnerability class, different router — and unlike the flag-gated checks, this one is simply never called.
Problem
export_session(autobot-backend/api/chat_sessions.py,GET /chat/sessions/{session_id}/export) takessession_idfrom the path and calls neithervalidate_session_ownershipnorvalidate_chat_ownershipanywhere in its body:The only guards are a session-id format check and a format check. Verified by reading the full function, not by grepping for the word "ownership".
Impact
Any authenticated user who knows or guesses another user's
session_idcan export that user's entire chat transcript. This is bulk disclosure of conversation content, which is a worse outcome than the injection gap #13982 covered, and it is not feature-flag dependent — there is no check to enable.Session ids are the same values that appear in URLs and are referenced across the chat APIs, so "knows or guesses" is not a high bar in practice.
Suggested fix
Add the ownership check the sibling endpoints use, before any data is read:
Then audit the rest of
api/chat_sessions.pythe same way — by reading each function that accepts asession_id, not by grepping. #14009's audit coveredapi/chat.pyonly; the assumption that a sibling router was equally covered turned out to be wrong, which is the reason to check the rest by hand.Acceptance criteria
export_sessionrefuses asession_idthe caller does not owninspect.getsource)api/chat_sessions.pythat accepts a session id is audited by reading the function body; the result is listed in the PR, including any found unprotectedNote on enforcement
Adding the call closes the missing-check gap, but the shared validator currently fails open — see #14010. Both are needed for this endpoint to actually refuse a non-owner in a default deployment.
Related
api/chat.py