mirrors/hermes-agent
1
0
Fork 0
mirror of https://github.com/NousResearch/hermes-agent.git synced 2026-07-08 13:12:08 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

security(gateway): prove chat/thread origin for persisted /resume; tighten DM scoping

Browse source 
Addresses the egilewski/CodeRabbit and teknium1 reviews on PR #52355.

1) Persisted-row chat scope (egilewski/CodeRabbit). The sessions table stored
   only source + user_id, so an identity-bearing caller could resume/list an
   INACTIVE persisted row that matched source+user_id but belonged to a
   DIFFERENT chat (probe: same user moves `same_user_chat_b` into chat-a).
   Persist the messaging origin and compare it:
   - schema: sessions gains origin_chat_id / origin_thread_id (declarative
     auto-migration via the existing column reconciler).
   - SessionDB._insert_session_row accepts + writes the two columns.
   - the gateway records them at every origin-bearing creation: both
     SessionStore create paths (get_or_create_session + reset/switch) and the
     /title path that materializes a store-only session into the DB.
   - _resume_target_allowed's identity branch now also requires
     origin_chat_id AND origin_thread_id to match the caller. Legacy rows with
     NULL origin (created before this change) cannot prove chat origin and
     fail closed — resume them via a live session or an admin --all override.
   The /sessions listing inherits the fix (non-Matrix rows route through the
   same helper).

2) DM key-contract mirror (teknium1). _same_origin_chat's DM branch only
   compared user_id and allowed when either side was missing, diverging from
   build_session_key (no-chat_id DM keys are built from user_id_alt or
   user_id). It now: treats an equal non-blank chat_id as sufficient (the DM
   key IS the chat_id when present), and otherwise compares the effective
   participant id (user_id_alt or user_id), failing closed on a
   missing/different participant so two no-chat_id DM origins are never
   conflated.

Tests: add same-user/different-chat (e2e + unit) and chat-scope unit cases;
add DM no-chat_id / user_id_alt / no-identity / same-chat_id cases; update
existing fixtures to record origin_chat_id like the gateway does; make the
cross-room `/resume --all` listing test run as admin (cross-room listing is
admin-gated) and give the boundary-state resume runner a live same-origin so
its post-resume clearing assertions exercise an authorized resume.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
claudlos 2026-06-28 19:27:02 -05:00 • committed by Teknium
parent 33a5090bf6
commit 5248877c61
5 changed files with 166 additions and 48 deletions
Download patch file Download diff file Expand all files Collapse all files

57
gateway/slash_commands.py
View file

@ -710,9 +710,19 @@ class GatewaySlashCommandsMixin:
chat_type = (getattr(current, "chat_type", "") or "").lower()
# DM-like chats are always per-user.
if chat_type in {"dm", "direct", "private", ""}:
if origin.user_id and current.user_id:
return origin.user_id == current.user_id
return True
# chat_id was already required equal above and, when present, IS the
# DM session key — so an equal non-empty chat_id is sufficient.
# build_session_key only falls back to the participant id
# (``user_id_alt or user_id`` — Signal/Feishu key on user_id_alt)
# when there is NO chat_id; mirror that and fail closed on a
# missing/different participant so two no-chat_id DM origins are
# never conflated (was: compared user_id only and allowed when
# either side was missing).
if str(getattr(current, "chat_id", "") or ""):
return True
cur_pid = str(current.user_id_alt or current.user_id or "")
org_pid = str(origin.user_id_alt or origin.user_id or "")
return bool(cur_pid) and cur_pid == org_pid
# Non-DM: scope by participant whenever the session key for this source
# is per-user. is_shared_multi_user_session mirrors build_session_key's
# isolation rules exactly, so the guard stays in lock-step with the key.
@ -787,22 +797,37 @@ class GatewaySlashCommandsMixin:
return False # different platform / source
caller_uid = str(getattr(source, "user_id", "") or "")
row_uid = str(row.get("user_id") or "")
# Chat/thread origin recorded at session creation (see
# SessionDB._insert_session_row). The sessions table historically stored
# only source + user_id, so a same-user row could belong to a DIFFERENT
# chat; comparing the persisted origin closes that gap. Legacy rows
# created before origin capture have NULL here and therefore fail closed
# (they cannot prove the caller's chat) — resume them via a live session
# or an admin override.
caller_chat = str(getattr(source, "chat_id", "") or "")
row_chat = str(row.get("chat_id") or "")
caller_thread = str(getattr(source, "thread_id", "") or "")
row_thread = str(row.get("thread_id") or "")
if caller_uid:
# Identity-bearing caller: allow only when the row PROVES the same
# owner AND the same platform/origin. A row with no/blank user_id
# cannot be proven to belong to this caller; a row with no/blank
# source cannot be proven to share the caller's platform (the
# row_src check above only rejects a *mismatching* non-blank source,
# so a blank/legacy source would otherwise slip through on user_id
# equality alone). Either gap fails closed — an identified user must
# not bind to an unowned, other-owned, or unproven-origin persisted
# session by id/title. (Legacy NULL-owner or blank-source rows are
# intentionally not resumable this way; use a live session or an
# explicit admin override.)
# owner AND the same platform/origin AND the same chat/thread. A row
# with no/blank user_id cannot be proven to belong to this caller; a
# row with no/blank source cannot be proven to share the caller's
# platform (the row_src check above only rejects a *mismatching*
# non-blank source, so a blank/legacy source would otherwise slip
# through on user_id equality alone); and a row whose origin chat
# (or thread) differs from the caller's belongs to a different
# conversation. Any gap fails closed — an identified user must not
# bind to an unowned, other-owned, other-chat, or unproven-origin
# persisted session by id/title. (Legacy NULL-owner/blank-source/
# NULL-chat rows are intentionally not resumable this way; use a
# live session or an explicit admin override.)
return (
bool(row_uid) and row_uid == caller_uid
and bool(row_src) and bool(caller_src)
and str(row_src) == str(caller_src)
and row_chat == caller_chat
and row_thread == caller_thread
)
# No caller identity: the persisted row carries only source + user_id
# (the sessions table has no chat_id), so a same-platform row can belong
@ -3254,6 +3279,12 @@ class GatewaySlashCommandsMixin:
session_id=session_id,
source=source.platform.value if source.platform else "unknown",
user_id=source.user_id,
# Persist the messaging origin so a later /resume of this
# titled-but-now-inactive session can prove it belongs to the
# caller's chat/thread (IDOR scoping).
chat_id=source.chat_id,
chat_type=source.chat_type,
thread_id=source.thread_id,
)
except Exception:
pass # Session might already exist, ignore errors

6
hermes_state.py
View file

@ -1590,6 +1590,12 @@ class SessionDB:
only filling columns that are still NULL, never overwriting values an
earlier writer already set (so a later bare call with source="unknown"
can't clobber a real source/model).
``chat_id``/``thread_id`` record the messaging origin (the chat/room and
thread the session was started in) so that gateway ``/resume`` can prove
a persisted, now-inactive row belongs to the caller's chat/thread before
switching to it (IDOR scoping — without them the ``sessions`` table has
no chat/thread to compare).
"""
def _do(conn):
conn.execute(

3
tests/gateway/test_matrix_project_context_isolation.py
View file

@ -506,6 +506,9 @@ async def test_matrix_resume_all_lists_room_names():
source_b,
[_entry(source_a, "session-a", "Project A Plan"), _entry(source_b, "session-b", "Project B Plan")],
)
# Cross-room `/resume --all` listing is admin-gated (IDOR scoping), so this
# cross-room listing test must run as a configured admin.
runner._resume_caller_is_admin = lambda _src: True
result = await runner._handle_resume_command(_event("/resume --all", source_b))

144
tests/gateway/test_resume_command.py
View file

@ -84,8 +84,8 @@ class TestHandleResumeCommand:
"""With no argument, lists recently titled sessions."""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("sess_001", "telegram", user_id="12345")
db.create_session("sess_002", "telegram", user_id="12345")
db.create_session("sess_001", "telegram", user_id="12345", chat_id="67890")
db.create_session("sess_002", "telegram", user_id="12345", chat_id="67890")
db.set_session_title("sess_001", "Research")
db.set_session_title("sess_002", "Coding")
@ -105,7 +105,7 @@ class TestHandleResumeCommand:
"""With no arg and no titled sessions, shows instructions."""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("sess_001", "telegram", user_id="12345") # No title
db.create_session("sess_001", "telegram", user_id="12345", chat_id="67890") # No title
event = _make_event(text="/resume")
runner = _make_runner(session_db=db, event=event)
@ -119,11 +119,11 @@ class TestHandleResumeCommand:
"""Numeric argument resumes the indexed titled session from the list."""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("sess_001", "telegram", user_id="12345")
db.create_session("sess_002", "telegram", user_id="12345")
db.create_session("sess_001", "telegram", user_id="12345", chat_id="67890")
db.create_session("sess_002", "telegram", user_id="12345", chat_id="67890")
db.set_session_title("sess_001", "Research")
db.set_session_title("sess_002", "Coding")
db.create_session("current_session_001", "telegram", user_id="12345")
db.create_session("current_session_001", "telegram", user_id="12345", chat_id="67890")
event = _make_event(text="/resume 2")
runner = _make_runner(session_db=db, current_session_id="current_session_001",
@ -141,9 +141,9 @@ class TestHandleResumeCommand:
"""Out-of-range numeric arguments show a helpful error."""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("sess_001", "telegram", user_id="12345")
db.create_session("sess_001", "telegram", user_id="12345", chat_id="67890")
db.set_session_title("sess_001", "Research")
db.create_session("current_session_001", "telegram", user_id="12345")
db.create_session("current_session_001", "telegram", user_id="12345", chat_id="67890")
event = _make_event(text="/resume 9")
runner = _make_runner(session_db=db, current_session_id="current_session_001",
@ -160,9 +160,9 @@ class TestHandleResumeCommand:
"""Resolves a title and switches to that session."""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("old_session_abc", "telegram", user_id="12345")
db.create_session("old_session_abc", "telegram", user_id="12345", chat_id="67890")
db.set_session_title("old_session_abc", "My Project")
db.create_session("current_session_001", "telegram", user_id="12345")
db.create_session("current_session_001", "telegram", user_id="12345", chat_id="67890")
event = _make_event(text="/resume My Project")
runner = _make_runner(session_db=db, current_session_id="current_session_001",
@ -216,7 +216,7 @@ class TestHandleResumeCommand:
"""Returns error for unknown session name."""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("current_session_001", "telegram", user_id="12345")
db.create_session("current_session_001", "telegram", user_id="12345", chat_id="67890")
event = _make_event(text="/resume Nonexistent Session")
runner = _make_runner(session_db=db, event=event)
@ -229,7 +229,7 @@ class TestHandleResumeCommand:
"""Returns friendly message when already on the requested session."""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("current_session_001", "telegram", user_id="12345")
db.create_session("current_session_001", "telegram", user_id="12345", chat_id="67890")
db.set_session_title("current_session_001", "Active Project")
event = _make_event(text="/resume Active Project")
@ -244,11 +244,11 @@ class TestHandleResumeCommand:
"""Asking for 'My Project' when 'My Project #2' exists gets the latest."""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("sess_v1", "telegram", user_id="12345")
db.create_session("sess_v1", "telegram", user_id="12345", chat_id="67890")
db.set_session_title("sess_v1", "My Project")
db.create_session("sess_v2", "telegram", user_id="12345")
db.create_session("sess_v2", "telegram", user_id="12345", chat_id="67890")
db.set_session_title("sess_v2", "My Project #2")
db.create_session("current_session_001", "telegram", user_id="12345")
db.create_session("current_session_001", "telegram", user_id="12345", chat_id="67890")
event = _make_event(text="/resume My Project")
runner = _make_runner(session_db=db, current_session_id="current_session_001",
@ -267,12 +267,12 @@ class TestHandleResumeCommand:
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("compressed_root", "telegram", user_id="12345")
db.create_session("compressed_root", "telegram", user_id="12345", chat_id="67890")
db.set_session_title("compressed_root", "Compressed Work")
db.end_session("compressed_root", "compression")
db.create_session("compressed_child", "telegram", user_id="12345", parent_session_id="compressed_root")
db.create_session("compressed_child", "telegram", user_id="12345", chat_id="67890", parent_session_id="compressed_root")
db.append_message("compressed_child", "user", "hello from continuation")
db.create_session("current_session_001", "telegram", user_id="12345")
db.create_session("current_session_001", "telegram", user_id="12345", chat_id="67890")
event = _make_event(text="/resume Compressed Work")
runner = _make_runner(
@ -300,9 +300,9 @@ class TestHandleResumeCommand:
"""Switching sessions clears any cached running agent."""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("old_session", "telegram", user_id="12345")
db.create_session("old_session", "telegram", user_id="12345", chat_id="67890")
db.set_session_title("old_session", "Old Work")
db.create_session("current_session_001", "telegram", user_id="12345")
db.create_session("current_session_001", "telegram", user_id="12345", chat_id="67890")
event = _make_event(text="/resume Old Work")
runner = _make_runner(session_db=db, current_session_id="current_session_001",
@ -326,9 +326,9 @@ class TestHandleResumeCommand:
import threading
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("old_session", "telegram", user_id="12345")
db.create_session("old_session", "telegram", user_id="12345", chat_id="67890")
db.set_session_title("old_session", "Old Work")
db.create_session("current_session_001", "telegram", user_id="12345")
db.create_session("current_session_001", "telegram", user_id="12345", chat_id="67890")
event = _make_event(text="/resume Old Work")
runner = _make_runner(session_db=db, current_session_id="current_session_001",
@ -353,9 +353,9 @@ class TestHandleResumeCommand:
"""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("abc123", "telegram", user_id="12345")
db.create_session("abc123", "telegram", user_id="12345", chat_id="67890")
db.set_session_title("abc123", "Bracketed")
db.create_session("current_session_001", "telegram", user_id="12345")
db.create_session("current_session_001", "telegram", user_id="12345", chat_id="67890")
for raw in ("<abc123>", "[abc123]", '"abc123"', "'abc123'"):
event = _make_event(text=f"/resume {raw}")
@ -382,9 +382,9 @@ class TestHandleResumeCommand:
"""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("unnamed_session_xyz", "telegram", user_id="12345")
db.create_session("unnamed_session_xyz", "telegram", user_id="12345", chat_id="67890")
# Deliberately no title set — this session can ONLY be resolved by ID.
db.create_session("current_session_001", "telegram", user_id="12345")
db.create_session("current_session_001", "telegram", user_id="12345", chat_id="67890")
event = _make_event(text="/resume unnamed_session_xyz")
runner = _make_runner(
@ -409,7 +409,7 @@ class TestHandleSessionsCommand:
async def test_sessions_command_lists_current_platform_sessions(self, tmp_path):
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("tg_session", "telegram", user_id="12345")
db.create_session("tg_session", "telegram", user_id="12345", chat_id="67890")
db.set_session_title("tg_session", "Telegram Work")
db.create_session("discord_session", "discord")
db.set_session_title("discord_session", "Discord Work")
@ -434,7 +434,7 @@ class TestHandleSessionsCommand:
config is not."""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("tg_named", "telegram", user_id="12345")
db.create_session("tg_named", "telegram", user_id="12345", chat_id="67890")
db.set_session_title("tg_named", "Telegram Work")
db.create_session("discord_unnamed", "discord") # other origin
db.append_message("discord_unnamed", "user", "discord first prompt")
@ -462,7 +462,7 @@ class TestHandleSessionsCommand:
db.set_session_title("victim_other_uid", "Other User")
db.create_session("victim_missing_uid", "telegram") # NULL owner
db.set_session_title("victim_missing_uid", "Unowned")
db.create_session("current_session_001", "telegram", user_id="12345")
db.create_session("current_session_001", "telegram", user_id="12345", chat_id="67890")
for name in ("Other User", "victim_other_uid", "Unowned", "victim_missing_uid"):
event = _make_event(text=f"/resume {name}")
@ -482,14 +482,14 @@ class TestHandleSessionsCommand:
unproven-origin transcript)."""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("blank_source_same_uid", "telegram", user_id="12345")
db.create_session("blank_source_same_uid", "telegram", user_id="12345", chat_id="67890")
db.set_session_title("blank_source_same_uid", "Blank Source Same UID")
# Simulate a malformed/legacy row that does not record its origin.
db._conn.execute(
"UPDATE sessions SET source = '' WHERE id = ?", ("blank_source_same_uid",)
)
db._conn.commit()
db.create_session("current_session_001", "telegram", user_id="12345")
db.create_session("current_session_001", "telegram", user_id="12345", chat_id="67890")
for name in ("Blank Source Same UID", "blank_source_same_uid"):
event = _make_event(text=f"/resume {name}")
@ -537,11 +537,56 @@ class TestHandleSessionsCommand:
allow_override=False) is False
db.close()
@pytest.mark.asyncio
async def test_resume_blocks_same_user_different_chat(self, tmp_path):
"""egilewski/CodeRabbit probe: the SAME user must not move a persisted
transcript from another chat into the current one. The row records its
records origin chat_id, so a chat-a caller cannot resume a chat-b row even with
a matching user_id (persisted-row chat-scope proof)."""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("same_user_chat_b", "telegram", user_id="12345",
chat_id="chat-b")
db.set_session_title("same_user_chat_b", "Same User Chat B")
db.create_session("current_session_001", "telegram", user_id="12345",
chat_id="chat-a")
for name in ("Same User Chat B", "same_user_chat_b"):
event = _make_event(text=f"/resume {name}", user_id="12345",
chat_id="chat-a")
event.source.chat_type = "group"
runner = _make_runner(session_db=db, current_session_id="current_session_001",
event=event)
result = await runner._handle_resume_command(event)
runner.session_store.switch_session.assert_not_called()
assert "Resumed" not in result, name
db.close()
def test_resume_target_allowed_chat_scope(self, tmp_path):
"""Unit-level: identity-bearing persisted fallback requires the row's
origin chat (and thread) to match the caller's."""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("row_chat_a", "telegram", user_id="12345",
chat_id="chat-a")
db.create_session("row_chat_b", "telegram", user_id="12345",
chat_id="chat-b")
db.create_session("row_legacy_nochat", "telegram", user_id="12345") # NULL chat
runner = _make_runner(session_db=db)
runner._gateway_session_origin_for_id = lambda sid: None # persisted-only
caller = SessionSource(platform=Platform.TELEGRAM, chat_id="chat-a",
chat_type="group", user_id="12345")
# Same chat → allowed; different chat → blocked; legacy NULL-chat → blocked.
assert runner._resume_target_allowed(caller, "row_chat_a", allow_override=False) is True
assert runner._resume_target_allowed(caller, "row_chat_b", allow_override=False) is False
assert runner._resume_target_allowed(caller, "row_legacy_nochat", allow_override=False) is False
db.close()
@pytest.mark.asyncio
async def test_gateway_dispatches_sessions_command(self, tmp_path):
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("tg_session", "telegram", user_id="12345")
db.create_session("tg_session", "telegram", user_id="12345", chat_id="67890")
db.set_session_title("tg_session", "Telegram Work")
event = _make_event(text="/sessions")
@ -580,11 +625,40 @@ class TestSameOriginChatGroupScoping:
runner.config.group_sessions_per_user = False
assert runner._same_origin_chat(self._src("alice"), self._src("bob")) is True
def test_dm_cross_user_still_blocked(self):
def test_dm_cross_user_blocked_without_chat_id(self):
# No-chat_id DM: build_session_key falls back to the participant id
# (user_id_alt or user_id), so two different participants are different
# origins and must not match. (With a chat_id present the DM key IS the
# chat_id — see test_dm_same_chat_id_is_same_origin.)
runner = _make_runner()
a = self._src("alice", chat_type="dm", chat_id=None)
b = self._src("bob", chat_type="dm", chat_id=None)
assert runner._same_origin_chat(a, b) is False
def test_dm_no_identity_no_chat_id_fails_closed(self):
# teknium1 review: an identity-less no-chat_id DM must fail closed rather
# than be treated as a shared origin.
runner = _make_runner()
a = self._src(None, chat_type="dm", chat_id=None)
b = self._src(None, chat_type="dm", chat_id=None)
assert runner._same_origin_chat(a, b) is False
def test_dm_user_id_alt_mismatch_without_chat_id_blocked(self):
# No-chat_id DM keyed on user_id_alt (Signal/Feishu): different alt ids
# are different sessions even if user_id is absent/equal.
runner = _make_runner()
a = self._src(None, chat_type="dm", chat_id=None, user_id_alt="alice-alt")
b = self._src(None, chat_type="dm", chat_id=None, user_id_alt="bob-alt")
assert runner._same_origin_chat(a, b) is False
def test_dm_same_chat_id_is_same_origin(self):
# With a chat_id present, the DM session key is chat_id-only (no
# participant), so an equal chat_id is a same-origin match — mirrors
# build_session_key.
runner = _make_runner()
a = self._src("alice", chat_type="dm", chat_id="dm-1")
b = self._src("bob", chat_type="dm", chat_id="dm-1")
assert runner._same_origin_chat(a, b) is False
b = self._src("alice", chat_type="dm", chat_id="dm-1")
assert runner._same_origin_chat(a, b) is True
def test_resume_target_allowed_blocks_cross_user_live_group(self):
"""End-to-end via the live-origin branch: Alice cannot resume Bob's

4
tests/gateway/test_session_boundary_security_state.py
View file

@ -90,6 +90,10 @@ def _make_resume_runner():
runner._session_db = AsyncSessionDB(MagicMock())
runner._session_db._db.resolve_session_by_title.return_value = "resumed-session"
runner._session_db._db.get_session_title.return_value = "Resumed Work"
# The resumed session is live and shares the caller's origin, so the
# /resume IDOR guard authorizes it (this test covers the post-resume
# security-state clearing, not the ownership check).
runner._gateway_session_origin_for_id = lambda sid: source
return runner, session_key