diff --git a/gateway/pairing.py b/gateway/pairing.py index bf6212f97f3f..a096438d7775 100644 --- a/gateway/pairing.py +++ b/gateway/pairing.py @@ -536,6 +536,56 @@ def approve_code(self, platform: str, code: str) -> Optional[dict]: "user_name": matched_entry.get("user_name", ""), } + def approve_by_user_id(self, platform: str, user_id: str) -> Optional[dict]: + """ + Approve a pending pairing request by the sender's ``user_id``. + + This is the fallback path for platforms where the pairing code DM + may not reach the operator (e.g. WeChat/Weixin rate-limiting). + The ``user_id`` is already shown in ``list_pending`` output, so + the operator can copy it directly. + + Returns ``{user_id, user_name}`` on success, ``None`` if no + pending entry matches the user_id or the platform is locked out. + + Unlike :meth:`approve_code`, this does **not** count as a failed + attempt when no match is found — there is no secret to brute-force. + """ + with self._lock: + self._cleanup_expired(platform) + + if self._is_locked_out(platform): + return None + + pending = self._load_json(self._pending_path(platform)) + + matched_key = None + matched_entry = None + for entry_id, entry in pending.items(): + if not isinstance(entry, dict): + continue + entry_uid = entry.get("user_id", "") + if not entry_uid: + continue + if self._user_ids_match(platform, user_id, entry_uid): + matched_key = entry_id + matched_entry = entry + break + + if matched_key is None: + return None + + del pending[matched_key] + self._save_json(self._pending_path(platform), pending) + + self._approve_user(platform, matched_entry["user_id"], + matched_entry.get("user_name", "")) + + return { + "user_id": matched_entry["user_id"], + "user_name": matched_entry.get("user_name", ""), + } + def list_pending(self, platform: str = None) -> list: """List pending pairing requests, optionally filtered by platform. diff --git a/hermes_cli/pairing.py b/hermes_cli/pairing.py index 101a1d10bc77..cc0ed55ad43a 100644 --- a/hermes_cli/pairing.py +++ b/hermes_cli/pairing.py @@ -18,7 +18,8 @@ def pairing_command(args): if action == "list": _cmd_list(store) elif action == "approve": - _cmd_approve(store, args.platform, args.code) + by_user_id = getattr(args, "by_user_id", False) + _cmd_approve(store, args.platform, args.code, by_user_id=by_user_id) elif action == "revoke": _cmd_revoke(store, args.platform, args.user_id) elif action == "clear-pending": @@ -39,8 +40,8 @@ def _cmd_list(store): if pending: print(f"\n Pending Pairing Requests ({len(pending)}):") - print(f" {'Platform':<12} {'Code':<10} {'User ID':<20} {'Name':<20} {'Age'}") - print(f" {'--------':<12} {'----':<10} {'-------':<20} {'----':<20} {'---'}") + print(f" {'Platform':<12} {'Code(hash)':<12} {'User ID':<20} {'Name':<20} {'Age'}") + print(f" {'--------':<12} {'----------':<12} {'-------':<20} {'----':<20} {'---'}") for p in pending: print( f" {p['platform']:<12} {p['code']:<10} {p['user_id']:<20} " @@ -61,9 +62,38 @@ def _cmd_list(store): print() -def _cmd_approve(store, platform: str, code: str): - """Approve a pairing code.""" +def _cmd_approve(store, platform: str, code: str, *, by_user_id: bool = False): + """Approve a pairing code or a pending user by user_id.""" platform = platform.lower().strip() + + if by_user_id: + result = store.approve_by_user_id(platform, code) + if result: + uid = result["user_id"] + name = result.get("user_name") or "" + display = f"{name} ({uid})" if name else uid + print(f"\n Approved! User {display} on {platform} can now use the bot~") + print(" They'll be recognized automatically on their next message.\n") + elif store._is_locked_out(platform): + import time as _time + limits = store._load_json(store._rate_limit_path()) + lockout_until = limits.get(f"_lockout:{platform}", 0) + remaining = max(0, int(lockout_until - _time.time())) + mins = remaining // 60 + print( + f"\n Platform '{platform}' is locked out after too many failed " + f"approval attempts." + ) + print(f" Lockout clears in ~{mins} minute(s).") + print( + " To reset sooner, delete the '_lockout:{0}' entry from " + "~/.hermes/platforms/pairing/_rate_limits.json\n".format(platform) + ) + else: + print(f"\n No pending request for user_id '{code}' on platform '{platform}'.") + print(" Run 'hermes pairing list' to see pending requests.\n") + return + code = code.upper().strip() result = store.approve_code(platform, code) diff --git a/hermes_cli/subcommands/pairing.py b/hermes_cli/subcommands/pairing.py index 55b022ed6db9..cb90d4e3e399 100644 --- a/hermes_cli/subcommands/pairing.py +++ b/hermes_cli/subcommands/pairing.py @@ -21,12 +21,22 @@ def build_pairing_parser(subparsers, *, cmd_pairing: Callable) -> None: pairing_sub.add_parser("list", help="Show pending + approved users") pairing_approve_parser = pairing_sub.add_parser( - "approve", help="Approve a pairing code" + "approve", help="Approve a pairing code or pending user" ) pairing_approve_parser.add_argument( "platform", help="Platform name (telegram, discord, slack, whatsapp)" ) - pairing_approve_parser.add_argument("code", help="Pairing code to approve") + pairing_approve_parser.add_argument( + "code", help="Pairing code to approve (or user_id with --by-user-id)" + ) + pairing_approve_parser.add_argument( + "--by-user-id", + action="store_true", + default=False, + help="Treat the code argument as a user_id instead — useful when the " + "pairing code DM was not delivered (e.g. WeChat/Weixin). The user_id " + "is shown in 'hermes pairing list'.", + ) pairing_revoke_parser = pairing_sub.add_parser("revoke", help="Revoke user access") pairing_revoke_parser.add_argument("platform", help="Platform name") diff --git a/tests/gateway/test_pairing.py b/tests/gateway/test_pairing.py index ef1037692e6b..123d1123c96a 100644 --- a/tests/gateway/test_pairing.py +++ b/tests/gateway/test_pairing.py @@ -762,6 +762,72 @@ def fake_read_text(self, *a, **kw): # The warning must fire — otherwise this is the silent-failure bug. assert any(rec.levelno == logging.WARNING for rec in caplog.records), \ "PermissionError on approved.json must produce a WARNING log line" + + +class TestApproveByUserId: + """Tests for approve_by_user_id — the fallback approval path when + the pairing code DM was not delivered (#70651).""" + + def test_approve_by_user_id_succeeds(self, tmp_path): + with patch("gateway.pairing.PAIRING_DIR", tmp_path): + store = PairingStore() + store.generate_code("weixin", "o9cq_user1@im.wechat", "Alice") + result = store.approve_by_user_id("weixin", "o9cq_user1@im.wechat") + assert isinstance(result, dict) + assert result["user_id"] == "o9cq_user1@im.wechat" + assert result["user_name"] == "Alice" + + def test_approve_by_user_id_makes_user_approved(self, tmp_path): + with patch("gateway.pairing.PAIRING_DIR", tmp_path): + store = PairingStore() + store.generate_code("telegram", "user1", "Bob") + store.approve_by_user_id("telegram", "user1") + assert store.is_approved("telegram", "user1") is True + + def test_approve_by_user_id_removes_from_pending(self, tmp_path): + with patch("gateway.pairing.PAIRING_DIR", tmp_path): + store = PairingStore() + store.generate_code("telegram", "user1") + store.approve_by_user_id("telegram", "user1") + pending = store.list_pending("telegram") + assert len(pending) == 0 + + def test_approve_by_user_id_unknown_user_returns_none(self, tmp_path): + with patch("gateway.pairing.PAIRING_DIR", tmp_path): + store = PairingStore() + store.generate_code("telegram", "user1") + result = store.approve_by_user_id("telegram", "nonexistent") + assert result is None + + def test_approve_by_user_id_does_not_count_as_failed_attempt(self, tmp_path): + """Unlike approve_code, a missed user_id lookup must not count + toward the brute-force lockout — there is no secret to brute-force.""" + with patch("gateway.pairing.PAIRING_DIR", tmp_path): + store = PairingStore() + store.generate_code("telegram", "user1") + for _ in range(MAX_FAILED_ATTEMPTS + 2): + store.approve_by_user_id("telegram", "wrong_user") + # Platform must NOT be locked out + assert store._is_locked_out("telegram") is False + # The real code should still work via approve_code + code = store.list_pending("telegram")[0] + # ... but we don't have the raw code, so just verify not locked out + + def test_approve_by_user_id_works_without_code_delivery(self, tmp_path): + """The core #70651 scenario: code was never delivered to the operator, + but user_id is visible in list_pending.""" + with patch("gateway.pairing.PAIRING_DIR", tmp_path): + store = PairingStore() + # User sends a DM; gateway generates a code and tries to send it + store.generate_code("weixin", "wx_user_abc", "TestUser") + # Code DM was rate-limited / dropped — operator only has list output + pending = store.list_pending("weixin") + assert len(pending) == 1 + user_id = pending[0]["user_id"] + # Operator approves by user_id + result = store.approve_by_user_id("weixin", user_id) + assert result is not None + assert result["user_id"] == "wx_user_abc" # Profile-scoped storage (multiplexing gateway isolation) # ---------------------------------------------------------------------------