From 5be3df1d6f3f71977f1e5d5425a0124561c2965c Mon Sep 17 00:00:00 2001
From: Kaloyan Tenchov <zayfod@gmail.com>
Date: Tue, 19 May 2026 08:59:17 -0400
Subject: [PATCH] fix(signal): consult pairing store in is_allowed
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

BaseChannel.is_allowed ORs is_approved (the pairing store) into the
allow decision; the signal override dropped that step and only looked
at config.allow_from. With the new DM-pairing flow in place, an
approved-via-pairing sender's next message would have failed the
allow check and triggered another pairing code in a loop.

OR in a normalized check against the pairing store: walk each part of
the pipe-joined sender_id through _normalize_signal_id and call
is_approved for each variant, so an approval stored under one form
(phone with/without "+", UUID/ACI) still matches when the next inbound
uses a different form. Mirrors how slack.py:643 handles it.

Also tightens the empty-allowlist warning to only fire when nothing
else granted access, since pairing-store hits are now a valid path.

Not part of the original review, but Comments 2 and 3 turn this latent
gap into a broken round-trip — included so the pairing UX actually
works.
---
 nanobot/channels/signal.py            | 25 +++++++++++++++++++++----
 tests/channels/test_signal_channel.py | 16 ++++++++++++++++
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/nanobot/channels/signal.py b/nanobot/channels/signal.py
index ecee7116d..66b7d2b40 100644
--- a/nanobot/channels/signal.py
+++ b/nanobot/channels/signal.py
@@ -22,6 +22,7 @@ from nanobot.bus.queue import MessageBus
 from nanobot.channels.base import BaseChannel
 from nanobot.config.paths import get_media_dir
 from nanobot.config.schema import Base
+from nanobot.pairing import is_approved
 from nanobot.utils.helpers import safe_filename, split_message
 
 
@@ -375,12 +376,28 @@ class SignalChannel(BaseChannel):
         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)
+        if self._sender_matches_allowlist(sender_id, allow_list):
+            return True
+        if self._sender_approved_via_pairing(sender_id):
+            return True
+        if not allow_list:
+            self.logger.warning("allow_from is empty — all access denied")
+        return False
+
+    def _sender_approved_via_pairing(self, sender_id: str) -> bool:
+        """Return True if any normalized variant of sender_id is in the pairing store.
+
+        Pairing approval may be recorded under any of the identifier forms
+        signal exposes (phone with/without ``+``, UUID, ACI), so we check
+        each part of the pipe-joined composite against ``is_approved``.
+        """
+        for part in str(sender_id).split("|"):
+            for variant in self._normalize_signal_id(part):
+                if is_approved(self.name, variant):
+                    return True
+        return False
 
     async def start(self) -> None:
         """Start the Signal channel and connect to signal-cli daemon."""
diff --git a/tests/channels/test_signal_channel.py b/tests/channels/test_signal_channel.py
index 3a0565570..9822ff3b6 100644
--- a/tests/channels/test_signal_channel.py
+++ b/tests/channels/test_signal_channel.py
@@ -681,6 +681,22 @@ class TestHandleDataMessageDM:
         assert handled[0]["is_dm"] is True
         assert handled[0]["chat_id"] == "+19995550002"
 
+    @pytest.mark.asyncio
+    async def test_dm_paired_sender_allowed_without_allowlist_entry(self, monkeypatch):
+        # Once a sender completes pairing they should pass is_allowed on every
+        # subsequent message — otherwise the pairing reply loops forever.
+        approved = {"+19995550002"}
+        monkeypatch.setattr(
+            "nanobot.channels.signal.is_approved",
+            lambda channel, sender_id: sender_id in approved,
+        )
+        ch = _make_channel(dm_enabled=True, dm_policy="allowlist", dm_allow_from=[])
+        assert ch.is_allowed("+19995550002") is True
+        # Variant forms (with/without "+") must still match a stored approval.
+        assert ch.is_allowed("19995550002") is True
+        # Unpaired sender stays denied.
+        assert ch.is_allowed("+19995559999") is False
+
     @pytest.mark.asyncio
     async def test_dm_allowlist_matches_without_plus_prefix(self):
         """An allowlist entry without '+' must match a sender that carries '+'."""