Repository navigation
security(chat): session ownership filter fails OPEN — a Redis error returns every session on the instance #12867
Description
Split out of #12685 so the delivered work has a closable scope. #12685 stays open for the broader tenancy design.
Problem
_filter_user_sessions (api/chat_sessions.py:438) is the isolation filter on GET /api/chat/sessions. It failed open in two places:
user_session_ids = set(await validator.get_user_sessions(username))
if not user_session_ids:
return sessions # (a) no ownership data -> return EVERY session
...
except Exception as e:
logger.debug(...)
return sessions # (b) lookup failed -> return EVERY sessionPath (b) is indefensible: a Redis blip silently returns every session on the instance — including other companies' agent conversations. This is a plausible mechanism for the live observation in #12685, where a CEO · <company_id> session appeared in an ordinary user's chat list. It also failed at logger.debug, so it left no visible trace.
Fix
Path (b) now raises 503 OwnershipUnavailableError instead of returning an unfiltered list. 503 rather than 500 because the request is valid and will succeed once the ownership store is reachable; the UI can say "could not load your sessions" instead of showing someone else's.
Path (a) is deliberately left as-is and is the open question below.
Open decision (path a)
A user with no ownership records currently sees the unfiltered list. Failing closed there would be consistent — but on installs predating ownership tracking it would hide users' own history, so it is not a safe unilateral change.
Options:
- Fail closed — return an empty list. Correct for isolation; risks legacy sessions becoming invisible.
- Backfill ownership for existing sessions, then fail closed. Safest but needs a migration.
- Keep open, now logged (current state) — the exposure is at least visible in logs rather than silent.
Done when
- A failed ownership lookup never returns an unfiltered list.
- The no-records path has an explicit decision recorded, not an accidental default.
Activity
security(chat): ownership filter no longer returns every session when…
github-actions commentedon Jul 28, 2026
PR #12868 (merged to
Dev_new_gui) references this issue with a close keyword.If this issue is fully resolved, close it manually. If work remains, no action is needed.
mrveiss commentedon Jul 28, 2026
Fixed in PR #12868 — squash-merged to
Dev_new_guias01cb31c1636c68d8ef341185edabe36b3d3d4d87.The lookup-failure path now raises
503 OwnershipUnavailableErrorinstead of returning the unfiltered session list. 503 rather than 500 because the request is valid and will succeed once the ownership store is reachable — the UI can say "could not load your sessions" instead of showing someone else's.It previously failed at
logger.debug, so a Redis blip exposed every session on the instance with no visible trace.Path (a) — no ownership records for the user — is deliberately unchanged and now logs a warning rather than being silent. Failing closed there would hide users' own history on installs predating ownership tracking, so it needs the owner decision recorded in this issue (fail closed / backfill then fail closed / keep open). A test pins the current behaviour so the deferred decision cannot be forgotten.
Verification: 5 new tests — only own sessions returned, the CEO agent session excluded when unowned, a store failure raises 503 rather than returning everything, a validator failure likewise, and the legacy no-records path still returns all but warns. 387 passed across the chat/session selection.
mrveiss commentedon Jul 29, 2026
Confirmed path (b) is delivered on the current tip —
_filter_user_sessionsraisesOwnershipUnavailableErrorand logs aterror(api/chat_sessions.py:449-461). This issue is correctly still open only for the path (a) decision.Small residue worth folding into whichever PR settles that decision: the function's docstring still documents the removed behaviour.
Worth correcting because this is a security boundary: the next reader to reason about failure modes from the docstring would conclude the filter still fails open. Not filing separately — it belongs with the path (a) change that will touch these lines anyway.
Path (a) itself remains an owner call between the three recorded options (fail closed / backfill-then-fail-closed / keep open but logged).
mrveiss commentedon Jul 30, 2026
Resolved — closing with merge evidence.
Fixed by PR #12868, merged to
Dev_new_guias01cb31c1636c68d8ef341185edabe36b3d3d4d87(2026-07-28). Verified present on the current branch tip, not merely reported merged.autobot-backend/api/chat_sessions.py— imports and appliessecurity.session_ownership.validate_session_ownership, so a Redis error can no longer fall through to returning every session.autobot-backend/api/chat_sessions_errors.py— shared error handling for the fail-closed path.autobot-backend/tests/test_chat_session_ownership_failclosed.py— present on tip (blob24d6f54b), pinning the fail-closed behaviour so a regression is caught rather than silently reopening the hole.Left open only because
Closes #Nfires on the default branch (main) and this merged toDev_new_gui, so GitHub never auto-closed it.fix(chat): scope agent sessions out of the general session list (#12685…