mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-09 13:21:42 +00:00
security(gateway): fail closed on persisted /resume for identity-less callers
Addresses egilewski (Codex/CodeRabbit) follow-up on PR #52355: the no-identity branch of _resume_target_allowed() returned True after only checking that the row's source didn't mismatch the caller platform. The sessions table has no chat_id, so same-platform alone is not ownership proof — a Telegram group caller in chat-a with user_id=None could resume (and /sessions could list) a persisted row owned by another chat/user (e.g. victim_chat_b_uid, source=telegram, user_id=victim). Fail closed: an identity-less caller can no longer bind to or enumerate a persisted session by id/title. A legitimate same-chat resume of an ACTIVE session still works via the live-origin branch (which compares chat_id), and an operator can use the admin --all override. The listing path inherits the fix because _resume_row_visible() routes non-Matrix rows through the same helper. Adds an end-to-end no-identity probe (resume blocked) and a unit-level persisted-fallback assertion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
bb6e216aab
commit
33a5090bf6
2 changed files with 46 additions and 3 deletions
|
|
@ -804,9 +804,15 @@ class GatewaySlashCommandsMixin:
|
|||
and bool(row_src) and bool(caller_src)
|
||||
and str(row_src) == str(caller_src)
|
||||
)
|
||||
# No caller identity (single-user / no-identity context): there is no
|
||||
# cross-user boundary to enforce beyond the same-platform check above.
|
||||
return True
|
||||
# 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
|
||||
# ownership proof — an identity-less caller must not bind to, or
|
||||
# enumerate, a persisted session by id/title. Fail closed. A legitimate
|
||||
# same-chat resume of an ACTIVE session still works through the
|
||||
# live-origin branch above (which compares chat_id), and an operator can
|
||||
# use the admin --all override. (CWE-639: IDOR on session routing.)
|
||||
return False
|
||||
|
||||
async def _resume_row_visible(
|
||||
self, source: SessionSource, row: dict, allow_all: bool
|
||||
|
|
|
|||
|
|
@ -500,6 +500,43 @@ class TestHandleSessionsCommand:
|
|||
assert "Resumed" not in result, name
|
||||
db.close()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resume_blocks_no_identity_caller_on_persisted_row(self, tmp_path):
|
||||
"""A caller with no user_id must not resume a persisted row on
|
||||
same-platform alone: the row has no chat_id to prove ownership, so a
|
||||
Telegram group caller in chat-a (user_id=None) cannot bind to a row
|
||||
owned by another chat/user (IDOR regression for the no-identity branch)."""
|
||||
from hermes_state import SessionDB
|
||||
db = SessionDB(db_path=tmp_path / "state.db")
|
||||
db.create_session("victim_chat_b_uid", "telegram", user_id="victim")
|
||||
db.set_session_title("victim_chat_b_uid", "Victim Chat B")
|
||||
db.create_session("current_session_001", "telegram")
|
||||
|
||||
for name in ("Victim Chat B", "victim_chat_b_uid"):
|
||||
event = _make_event(text=f"/resume {name}", user_id=None,
|
||||
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_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
|
||||
db = SessionDB(db_path=tmp_path / "state.db")
|
||||
db.create_session("victim_chat_b_uid", "telegram", user_id="victim")
|
||||
runner = _make_runner(session_db=db)
|
||||
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",
|
||||
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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue