security(chat): get_session_owner cannot distinguish 'no owner' from 'unreadable', and callers now WRITE on that answer #14033

Found while reviewing PR #14025 (#14020). Pre-existing error contract, newly load-bearing.

Problem

ChatHistoryManager.get_session_owner (autobot-backend/chat_history/session.py) collapses three different situations into a single None:

• the session has no owner recorded (a genuine legacy session)
• the session file does not exist
• the file exists but is unreadable — decrypt failure, corruption, disk error

It swallows both OSError and Exception internally and returns None for all of them. _load_existing_chat_data does the same, returning {} on a decrypt failure.

Why it matters more now

Until recently that answer only drove reads. Two recent changes made it drive writes and authorization:

• security(chat): session ownership expires after 24h and is then claimed by whoever accesses it next #14018 — an absent Redis ownership record is resolved against this lookup. A None from an unreadable file reads as "genuinely unowned", so the session takes the claim path.
• security(chat): save_chat_by_id can create a session with no durable owner, leaving it permanently reclaimable #14020 / PR security(chat): write the durable owner when the save path creates a session (#14020) #14025 — the save path writes owner metadata based on the ownership outcome.

So a transient decrypt failure or disk hiccup on a session that does have an owner can be read as unowned, and the resulting claim can then be written to the file, permanently reassigning ownership on a momentary glitch. _build_session_chat_data merges with existing_metadata.update(metadata), so the owner/username keys are overwritten rather than preserved.

The same corrupted-read event also silently drops the rest of the metadata (org_id, team_id, thinking_preferences, title), because _load_existing_chat_data returns {}.

PR #14025 narrows the exposure — it now writes only when the validator reports legacy_migration, and never on the unchecked fast paths — but it does not remove it: the validator itself reaches legacy_migration via the same collapsed None.

Suggested approach

Give the lookup an honest error contract:

1. get_session_owner distinguishes absent (no file, or a file with no owner → None) from unreadable (decrypt/corruption/IO → raise, or return a distinct sentinel).
2. Callers treat "unreadable" as unknown, not as unowned: deny or defer rather than claim, and never write an owner on that path.
3. _load_existing_chat_data likewise should not report a corrupted file as an empty dict, since that silently discards metadata on save.

This is the natural companion to #14016 and #14021 — consolidating the ownership lookups is the moment to give them one error contract as well as one implementation.

Acceptance criteria

• A decrypt/IO failure is distinguishable from "no owner recorded" at the call site
• Test: an unreadable session file does not result in a claim, and does not result in an owner being written
• Test: a genuinely ownerless session still behaves as today
• A corrupted read no longer silently discards existing metadata on save
• The remaining ownership lookups share the contract (see tech-debt(security): two different validate_session_ownership functions — same name, different signature and semantics #14016, security(chat): shared-link endpoints check ownership against the expiring Redis key only — the real owner is locked out after 24h #14021)

Related

• security(chat): session ownership expires after 24h and is then claimed by whoever accesses it next #14018 — made the file record authoritative, which is what turned this into an authorization input
• security(chat): save_chat_by_id can create a session with no durable owner, leaving it permanently reclaimable #14020 / PR security(chat): write the durable owner when the save path creates a session (#14020) #14025 — the write path; narrowed but did not remove the exposure
• tech-debt(security): two different validate_session_ownership functions — same name, different signature and semantics #14016 — two functions named validate_session_ownership
• security(chat): shared-link endpoints check ownership against the expiring Redis key only — the real owner is locked out after 24h #14021 — shared-link endpoints use a third, raw lookup
• security(chat): ownership enforcement fails OPEN — an unavailable feature-flag service disables every check platform-wide #14010 — enforcement fails open