fix(gateway): fence multiplex profile attribution in session lookup and inheritance - #88387
69k4xmdfm2-blip wants to merge 1 commit into
Conversation
…nd inheritance A Telegram private chat reports the user's own id as chat.id, so in a multiplexed gateway the peer tuple (source, user_id, chat_id, chat_type, thread_id) is byte-identical for every bot. Two sites trusted it: find_latest_gateway_session_for_peer's conservative fallback matched on that tuple alone and returned a sibling profile's row, executing the wrong persona, credentials and filesystem scope (NousResearch#74285). create_session then made that crossing permanent: the reset records the adopted row as parent_session_id, and the unconditional COALESCE stamped profile_name from the parent. Since "default" persists as NULL, a default child is always eligible, so inheritance only ever flows default -> named profile and every later reset copies the wrong name forward again, while session_key stays correct (NousResearch#88381). Both fences derive from session_key, which already carries the namespace (agent:main: for default, agent:<profile>: otherwise) - no new column and no migration. The fallback now requires a shared namespace, comparing a keyless candidate on profile_name instead ('' == default); no match returns None and mints a fresh session rather than borrowing a sibling's. profile_name inheritance moves into its own statement gated on that same namespace, leaving cwd/git_repo_root/git_branch unconditional (NousResearch#64709) and keyless CLI/subagent lineage untouched, where the parent's profile is the only signal available. Both fences fail closed and are no-ops on a single-profile gateway.
Correct and well-tested fix for a serious multiplex-contamination bug: Telegram DMs make the peer tuple byte-identical across every bot on one gateway, so the conservative fallback happily adopted a sibling profile's session — executing another persona's credentials and filesystem scope. Fencing by the
No blocking issues found beyond item 1's caller-scope verification. |
|
Thanks for this PR. Merged via #101248 (ad8b9e0) on current main — per-profile /voice state, off-loop startup hydration, session-recovery owner fence, int route ids. #101248 won as the consolidated fix because it covers the whole multiplex-profile bug class in one change (with tests) rather than the single symptom addressed here; this PR is superseded by it. If anything from your original change is still missing on main >= ad8b9e0, please open a fresh PR/issue against main and tag it. Thanks again. |
What
Two profile fences in
hermes_state.pyso a multiplexed gateway can never attribute a session tothe wrong profile.
Fixes the durable-mislabel half of #88381; hardens the lookup half of #74285.
Why
A Telegram private chat reports the user's own id as
chat.id, so for every bot in amultiplexed gateway the peer tuple
(source, user_id, chat_id, chat_type, thread_id)isbyte-identical. Two places trusted that tuple:
1.
find_latest_gateway_session_for_peer— the conservative fallback (#74285). With thetuple alone it happily returns a sibling profile's row, so a DM to bot A can be answered by
profile B, with B's persona, credentials and filesystem scope.
2.
create_session— parent inheritance (#88381, this PR's new finding). Once (1) hascrossed profiles, the reset records the adopted row as
parent_session_id, and the unconditionalstamps the child with the parent's profile. Because
"default"persists asNULL(
run_agent.py::_ensure_db_session,conversation_compression.py), a default child is alwaysNULLat insert and therefore always eligible — inheritance can only flow default → namedprofile, never back. Every subsequent daily/idle reset copies the wrong name forward again, so
one crossing mislabels the lineage permanently while
session_keystays correct. The twodisagree, and
profile_nameis the one that session lists, per-profile aggregation and thedesktop sidebar read.
Real rows from my install (
default+medicina, two bot tokens, one gateway):The four open PRs for #74285 (#75043, #74153, #85205, #74593) all fence the lookup; none touches
the inheritance write, so already-crossed lineages stay mislabelled after any of them lands. This
PR is deliberately scoped to be compatible with whichever one you take: the lookup fence here is
a namespace comparison in the same predicate position, so it rebases cleanly or drops out
entirely if you prefer their shape.
How
session_keyalready carries the truth (agent:main:…for default,agent:<profile>:…otherwise), so both fences derive from it — no new column, no migration.
agent:<ns>:prefix. A keyless candidate (identity write lost the key — the case the fallback exists for) is
compared on
profile_nameinstead,''== default. No match now returnsNone, which mints afresh session: strictly safer than borrowing a sibling's.
profile_namemoves out of the shared parent-backfillUPDATEinto its ownstatement, gated on both rows sharing that namespace.
cwd/git_repo_root/git_branchkeep inheriting unconditionally (harmless across profiles; gating them would regress bug(state): compression-split child sessions lose cwd/git_repo_root, causing project sidebar entry to disappear every compression #64709).
Keyless lineage — CLI, subagents, delegation — still inherits unconditionally, since there the
parent's profile is the only signal available. Namespace extraction uses
substr/instrso noUDF is needed.
Both fences fail closed and are no-ops on a single-profile gateway (
agent:main:on both sides).Testing
New:
tests/gateway/test_multiplex_peer_fallback_profile_fence.py— 10 cases.RED verified against a pristine
hermes_state.py(same tests, module shadowed viaPYTHONPATH):3 fail without the patch —
test_fallback_ignores_sibling_profile_row,test_fallback_ignores_sibling_keyless_row_by_profile_name,test_child_does_not_inherit_sibling_profile_across_namespaces(
AssertionError: default child inherited sibling profile: 'medicina').Coverage includes the negative cases that keep the fences honest: own keyless row still recovers
(both from
defaultand from a named profile), exact-key matching unaffected with both rowspresent, a newer sibling row does not outrank this profile's older correct row, same-namespace
rotation still inherits, keyless subagent lineage still inherits, and an explicit child
profile_nameis never overwritten.Neighbouring suites (
test_session_continuity_82616,test_branch_routing_columns,test_hermes_state.py,test_session.py,test_multiplex_profile_authz,test_profile_resolution,test_profile_routing,test_multiplex_phase0) show no new failures:the handful that fail here reproduce identically against a pristine
hermes_state.pyon thismachine, so they are pre-existing and unrelated to this change.
Platform: Windows 11, Python 3.11 (venv) — verified end-to-end on a live 2-profile ×
2-Telegram-bot multiplexed gateway, which is where the mislabelled rows above came from.
Existing data
Deployments that already crossed can be repaired offline, since
session_keyis authoritative:plus clearing the cross-namespace
parent_session_idlinks that caused them. I did not add thatto
hermes sessions repair-routingin this PR to keep the diff reviewable — happy to follow upif you want it there.