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

security(gateway): fail closed on no-provenance persisted /resume for non-DM callers

Browse source 
Addresses egilewski/CodeRabbit follow-up on PR #52355: the identity-bearing
persisted fallback compared row_chat == caller_chat, which SUCCEEDS when both
normalize to "" — so a legacy row with no stored chat provenance could still be
resumed by a caller that also has no chat_id (probe: a group caller with
chat_id=None resuming a NULL-chat telegram row on matching user_id).

A non-DM session (group/channel/forum/thread) is keyed by chat_id in
build_session_key, so a blank chat on either side is NOT proof of same-chat.
Require both row and caller chat_id to be non-blank and equal for non-DM
callers; a legacy NULL-chat row (or a caller missing its chat_id) now fails
closed. DMs are unchanged: they are keyed on user_id, so a no-chat_id DM row
stays resumable by the same user (and a mismatching chat_id, when present, is
still rejected).

Adds the blank-caller-chat group probe and a DM no-chat_id same-user/other-user
regression.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
claudlos 2026-06-29 08:32:14 -05:00 • committed by Teknium
parent 5248877c61
commit 599a6391d4
2 changed files with 68 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

18
gateway/slash_commands.py
View file

@ -808,6 +808,8 @@ class GatewaySlashCommandsMixin:
row_chat = str(row.get("chat_id") or "")
caller_thread = str(getattr(source, "thread_id", "") or "")
row_thread = str(row.get("thread_id") or "")
chat_type = (getattr(source, "chat_type", "") or "").lower()
caller_is_dm = chat_type in {"dm", "direct", "private", ""}
if caller_uid:
# Identity-bearing caller: allow only when the row PROVES the same
# owner AND the same platform/origin AND the same chat/thread. A row
@ -822,13 +824,25 @@ class GatewaySlashCommandsMixin:
# 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 (
base_ok = (
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
)
if not base_ok:
return False
if caller_is_dm:
# DMs are keyed on user_id; chat_id is legitimately absent on
# both sides for a no-chat_id DM (already scoped by user_id
# above). Still reject a mismatching chat_id when present.
return row_chat == caller_chat
# Non-DM (group/channel/forum/thread): build_session_key includes
# chat_id, so a row (or caller) with NO chat provenance cannot prove
# same-chat. Require both sides non-blank and equal — a legacy
# NULL-chat row (or a caller missing its chat_id) fails closed even
# when both normalize to "". (CWE-639)
return bool(row_chat) and bool(caller_chat) and row_chat == caller_chat
# 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
# to a DIFFERENT chat or user. Same-platform alone is therefore NOT

72
tests/gateway/test_resume_command.py
View file

@ -183,9 +183,9 @@ class TestHandleResumeCommand:
restored conversation, while leaving other chats' overrides intact (#10702)."""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("old_session_abc", "telegram")
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")
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",
@ -523,7 +523,8 @@ class TestHandleSessionsCommand:
assert "Resumed" not in result, name
db.close()
def test_resume_target_allowed_blocks_no_identity_persisted(self, tmp_path):
@pytest.mark.asyncio
async def test_resume_target_allowed_blocks_no_identity_persisted(self, tmp_path):
"""Unit-level: the persisted-row fallback fails closed for an
identity-less caller (no live origin resolvable)."""
from hermes_state import SessionDB
@ -533,7 +534,7 @@ class TestHandleSessionsCommand:
runner._gateway_session_origin_for_id = lambda sid: None # inactive/persisted-only
caller = SessionSource(platform=Platform.TELEGRAM, chat_id="chat-a",
chat_type="group", user_id=None)
assert runner._resume_target_allowed(caller, "victim_chat_b_uid",
assert await runner._resume_target_allowed(caller, "victim_chat_b_uid",
allow_override=False) is False
db.close()
@ -562,7 +563,8 @@ class TestHandleSessionsCommand:
assert "Resumed" not in result, name
db.close()
def test_resume_target_allowed_chat_scope(self, tmp_path):
@pytest.mark.asyncio
async 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
@ -577,9 +579,33 @@ class TestHandleSessionsCommand:
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
assert await runner._resume_target_allowed(caller, "row_chat_a", allow_override=False) is True
assert await runner._resume_target_allowed(caller, "row_chat_b", allow_override=False) is False
assert await runner._resume_target_allowed(caller, "row_legacy_nochat", allow_override=False) is False
# egilewski/CodeRabbit probe: a GROUP caller that itself has no chat_id
# must NOT resume a legacy NULL-chat row just because both normalize to
# "" — a non-DM session is keyed by chat_id, so blank == no provenance.
blank_caller = SessionSource(platform=Platform.TELEGRAM, chat_id=None,
chat_type="group", user_id="12345")
assert await runner._resume_target_allowed(blank_caller, "row_legacy_nochat",
allow_override=False) is False
db.close()
@pytest.mark.asyncio
async def test_resume_target_allowed_dm_no_chat_id_scopes_by_user(self, tmp_path):
"""A DM is keyed on user_id; a no-chat_id DM row is resumable by the same
user (chat_id legitimately absent on both sides), unlike a group row."""
from hermes_state import SessionDB
db = SessionDB(db_path=tmp_path / "state.db")
db.create_session("dm_row", "telegram", user_id="12345") # DM, no chat_id
runner = _make_runner(session_db=db)
runner._gateway_session_origin_for_id = lambda sid: None # persisted-only
same = SessionSource(platform=Platform.TELEGRAM, chat_id=None,
chat_type="dm", user_id="12345")
other = SessionSource(platform=Platform.TELEGRAM, chat_id=None,
chat_type="dm", user_id="99999")
assert await runner._resume_target_allowed(same, "dm_row", allow_override=False) is True
assert await runner._resume_target_allowed(other, "dm_row", allow_override=False) is False
db.close()
@pytest.mark.asyncio
@ -660,13 +686,14 @@ class TestSameOriginChatGroupScoping:
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):
@pytest.mark.asyncio
async def test_resume_target_allowed_blocks_cross_user_live_group(self):
"""End-to-end via the live-origin branch: Alice cannot resume Bob's
active group session in the same chat."""
runner = _make_runner()
bob = self._src("bob")
runner._gateway_session_origin_for_id = lambda sid: bob
assert runner._resume_target_allowed(
assert await runner._resume_target_allowed(
self._src("alice"), "bobs_live_sid", allow_override=False
) is False
@ -717,7 +744,8 @@ class TestResumeRowVisibleMatrixAllScoping:
return SessionSource(platform=Platform.MATRIX, chat_id=chat_id,
chat_type="group", user_id=user_id)
def test_non_admin_all_does_not_expose_other_room(self):
@pytest.mark.asyncio
async def test_non_admin_all_does_not_expose_other_room(self):
runner = _make_runner()
runner._resume_caller_is_admin = lambda src: False
# Titled row whose live origin is a DIFFERENT Matrix room.
@ -725,32 +753,35 @@ class TestResumeRowVisibleMatrixAllScoping:
chat_type="group", user_id="@bob:hs")
runner._gateway_session_origin_for_id = lambda sid: other_room
row = {"id": "sid_other_room"}
assert runner._resume_row_visible(self._matrix_src(), row, allow_all=True) is False
assert await runner._resume_row_visible(self._matrix_src(), row, allow_all=True) is False
def test_non_admin_all_still_shows_same_room(self):
@pytest.mark.asyncio
async def test_non_admin_all_still_shows_same_room(self):
runner = _make_runner()
runner._resume_caller_is_admin = lambda src: False
same_room = SessionSource(platform=Platform.MATRIX, chat_id="!room-a:hs",
chat_type="group", user_id="@bob:hs")
runner._gateway_session_origin_for_id = lambda sid: same_room
row = {"id": "sid_same_room"}
assert runner._resume_row_visible(self._matrix_src(), row, allow_all=True) is True
assert await runner._resume_row_visible(self._matrix_src(), row, allow_all=True) is True
def test_admin_all_exposes_cross_room(self):
@pytest.mark.asyncio
async def test_admin_all_exposes_cross_room(self):
runner = _make_runner()
runner._resume_caller_is_admin = lambda src: True
other_room = SessionSource(platform=Platform.MATRIX, chat_id="!room-b:hs",
chat_type="group", user_id="@bob:hs")
runner._gateway_session_origin_for_id = lambda sid: other_room
row = {"id": "sid_other_room"}
assert runner._resume_row_visible(self._matrix_src(), row, allow_all=True) is True
assert await runner._resume_row_visible(self._matrix_src(), row, allow_all=True) is True
def test_non_admin_all_fails_closed_on_unknown_origin(self):
@pytest.mark.asyncio
async def test_non_admin_all_fails_closed_on_unknown_origin(self):
runner = _make_runner()
runner._resume_caller_is_admin = lambda src: False
runner._gateway_session_origin_for_id = lambda sid: None
row = {"id": "sid_unknown"}
assert runner._resume_row_visible(self._matrix_src(), row, allow_all=True) is False
assert await runner._resume_row_visible(self._matrix_src(), row, allow_all=True) is False
class TestSameMatrixRoomThreadScoping:
@ -792,7 +823,8 @@ class TestSameMatrixRoomThreadScoping:
assert runner._same_matrix_room(threaded, room_level) is False
assert runner._same_matrix_room(room_level, threaded) is False
def test_resume_row_visible_blocks_cross_thread(self):
@pytest.mark.asyncio
async def test_resume_row_visible_blocks_cross_thread(self):
"""End-to-end through the Matrix listing guard."""
runner = _make_runner()
runner._resume_caller_is_admin = lambda src: False
@ -800,4 +832,4 @@ class TestSameMatrixRoomThreadScoping:
runner._gateway_session_origin_for_id = lambda sid: origin_thread_b
row = {"id": "sid_thread_b"}
caller_thread_a = self._msrc(thread_id="thread-a")
assert runner._resume_row_visible(caller_thread_a, row, allow_all=False) is False
assert await runner._resume_row_visible(caller_thread_a, row, allow_all=False) is False