mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-05-21 00:52:34 +00:00
fix(signal): normalize composite sender_ids in is_allowed too
The base BaseChannel.is_allowed() does a literal ``sender_id in allow_from`` check, but Signal's sender_id is a pipe-joined composite of phone/UUID parts. After splitting an allowlist entry like ``+phone|uuid`` into two separate entries, the per-DM gate accepted it but the base gate still denied because the composite sender string wasn't literally in the list. Override is_allowed on SignalChannel to delegate to _sender_matches_allowlist, which already splits both sides on ``|`` and normalizes each part. _sender_matches_allowlist itself now also splits allowlist entries on ``|`` so legacy composite entries keep working too. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
01725bab11
commit
3874b3acf4
@ -360,6 +360,23 @@ class SignalChannel(BaseChannel):
|
|||||||
# Each message is a dict with: sender_name, sender_number, content, timestamp
|
# Each message is a dict with: sender_name, sender_number, content, timestamp
|
||||||
self._group_buffers: dict[str, deque] = {}
|
self._group_buffers: dict[str, deque] = {}
|
||||||
|
|
||||||
|
def is_allowed(self, sender_id: str) -> bool:
|
||||||
|
"""Override base check to normalize and split pipe-joined identifiers.
|
||||||
|
|
||||||
|
``sender_id`` from Signal is the pipe-joined composite produced by
|
||||||
|
``_collect_sender_id_parts``; allow_from entries may be single
|
||||||
|
identifiers or composites and may use the ``+`` prefix variant or
|
||||||
|
not. Delegates to ``_sender_matches_allowlist`` so the base gate
|
||||||
|
matches the per-policy DM gate.
|
||||||
|
"""
|
||||||
|
allow_list = self.config.allow_from
|
||||||
|
if not allow_list:
|
||||||
|
self.logger.warning("allow_from is empty — all access denied")
|
||||||
|
return False
|
||||||
|
if "*" in allow_list:
|
||||||
|
return True
|
||||||
|
return self._sender_matches_allowlist(sender_id, allow_list)
|
||||||
|
|
||||||
async def start(self) -> None:
|
async def start(self) -> None:
|
||||||
"""Start the Signal channel and connect to signal-cli daemon."""
|
"""Start the Signal channel and connect to signal-cli daemon."""
|
||||||
if not self.config.phone_number:
|
if not self.config.phone_number:
|
||||||
@ -937,11 +954,12 @@ class SignalChannel(BaseChannel):
|
|||||||
def _sender_matches_allowlist(cls, sender_id: str, allow_list: list[str]) -> bool:
|
def _sender_matches_allowlist(cls, sender_id: str, allow_list: list[str]) -> bool:
|
||||||
"""Return True if any normalized variant of sender_id is on allow_list.
|
"""Return True if any normalized variant of sender_id is on allow_list.
|
||||||
|
|
||||||
sender_id is the pipe-joined identifier string built by
|
Both ``sender_id`` and each allow_list entry can be a single
|
||||||
_collect_sender_id_parts. Each part and each allow_list entry is run
|
identifier or a pipe-joined composite of several (e.g.
|
||||||
through _normalize_signal_id so an allowlist entry like ``1234567890``
|
``"+1234567890|uuid-abc"``); both sides are split on ``|`` and each
|
||||||
matches a sender ``+1234567890`` (and vice versa), and case-only
|
part is run through ``_normalize_signal_id`` so an allowlist entry
|
||||||
differences in UUIDs/ACIs match too.
|
like ``1234567890`` matches a sender ``+1234567890`` (and vice
|
||||||
|
versa), and case-only differences in UUIDs/ACIs match too.
|
||||||
"""
|
"""
|
||||||
if not allow_list:
|
if not allow_list:
|
||||||
return False
|
return False
|
||||||
@ -952,7 +970,8 @@ class SignalChannel(BaseChannel):
|
|||||||
return False
|
return False
|
||||||
allow_variants: set[str] = set()
|
allow_variants: set[str] = set()
|
||||||
for entry in allow_list:
|
for entry in allow_list:
|
||||||
allow_variants.update(cls._normalize_signal_id(entry))
|
for part in str(entry).split("|"):
|
||||||
|
allow_variants.update(cls._normalize_signal_id(part))
|
||||||
return bool(sender_variants & allow_variants)
|
return bool(sender_variants & allow_variants)
|
||||||
|
|
||||||
def _remember_account_id_alias(self, value: str | None) -> None:
|
def _remember_account_id_alias(self, value: str | None) -> None:
|
||||||
|
|||||||
@ -493,6 +493,51 @@ class TestGroupBuffer:
|
|||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
class TestIsAllowed:
|
||||||
|
"""The base-channel allowlist gate is overridden to understand Signal's
|
||||||
|
pipe-joined composite sender_ids and the +/no-+ phone variants.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def test_denies_when_allowlist_empty(self):
|
||||||
|
ch = _make_channel(dm_enabled=True, dm_policy="open") # open -> no entries
|
||||||
|
assert ch.is_allowed("+19995550001") is False
|
||||||
|
|
||||||
|
def test_allows_wildcard(self):
|
||||||
|
ch = _make_channel(dm_policy="allowlist", dm_allow_from=["*"])
|
||||||
|
assert ch.is_allowed("+19995550001|some-uuid") is True
|
||||||
|
|
||||||
|
def test_allows_composite_sender_against_split_allowlist(self):
|
||||||
|
"""Composite sender_id, single-id allow_from — must match either part."""
|
||||||
|
ch = _make_channel(
|
||||||
|
dm_policy="allowlist",
|
||||||
|
dm_allow_from=["+19995550001"],
|
||||||
|
)
|
||||||
|
assert ch.is_allowed("+19995550001|1872ba20-uuid") is True
|
||||||
|
|
||||||
|
def test_allows_composite_sender_against_composite_allowlist_entry(self):
|
||||||
|
"""Backward compat: pipe-joined composite allowlist entries still match."""
|
||||||
|
composite = "+19995550001|1872ba20-uuid"
|
||||||
|
ch = _make_channel(dm_policy="allowlist", dm_allow_from=[composite])
|
||||||
|
assert ch.is_allowed(composite) is True
|
||||||
|
|
||||||
|
def test_allows_when_only_uuid_part_is_listed(self):
|
||||||
|
ch = _make_channel(dm_policy="allowlist", dm_allow_from=["1872ba20-uuid"])
|
||||||
|
assert ch.is_allowed("+19995550001|1872ba20-uuid") is True
|
||||||
|
|
||||||
|
def test_denies_when_no_part_matches(self):
|
||||||
|
ch = _make_channel(dm_policy="allowlist", dm_allow_from=["+12223334444"])
|
||||||
|
assert ch.is_allowed("+19995550001|1872ba20-uuid") is False
|
||||||
|
|
||||||
|
def test_allowlist_union_includes_group_ids(self):
|
||||||
|
"""allow_from is the union of dm.allow_from and group.allow_from."""
|
||||||
|
ch = _make_channel(
|
||||||
|
group_enabled=True,
|
||||||
|
group_policy="allowlist",
|
||||||
|
group_allow_from=["group-id-base64=="],
|
||||||
|
)
|
||||||
|
assert "group-id-base64==" in ch.config.allow_from
|
||||||
|
|
||||||
|
|
||||||
class TestCheckInboundPolicy:
|
class TestCheckInboundPolicy:
|
||||||
"""Direct tests for the policy gate that _handle_data_message now delegates to."""
|
"""Direct tests for the policy gate that _handle_data_message now delegates to."""
|
||||||
|
|
||||||
@ -665,6 +710,22 @@ class TestHandleDataMessageDM:
|
|||||||
await ch._handle_receive_notification(params)
|
await ch._handle_receive_notification(params)
|
||||||
assert len(handled) == 1
|
assert len(handled) == 1
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_dm_allowlist_matches_pipe_joined_composite_entry(self):
|
||||||
|
"""Allowlist entries written as ``phone|uuid`` composites still work.
|
||||||
|
|
||||||
|
Some configs pre-date the per-part splitting and store the full
|
||||||
|
sender_id composite as a single allow_from entry. Keep matching it.
|
||||||
|
"""
|
||||||
|
composite = "+19995550001|1872ba20-f52a-4bad-b434-bf7f808c8b22"
|
||||||
|
ch, handled = self._make_dm_channel(policy="allowlist", allow_from=[composite])
|
||||||
|
params = _dm_envelope(
|
||||||
|
source_number="+19995550001",
|
||||||
|
source_uuid="1872ba20-f52a-4bad-b434-bf7f808c8b22",
|
||||||
|
)
|
||||||
|
await ch._handle_receive_notification(params)
|
||||||
|
assert len(handled) == 1
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_dm_disabled_rejected(self):
|
async def test_dm_disabled_rejected(self):
|
||||||
ch = _make_channel(dm_enabled=False)
|
ch = _make_channel(dm_enabled=False)
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user