From a6397c379b8a1c752676513a13adcb00c16cec4f Mon Sep 17 00:00:00 2001 From: wgu9 <145739220+wgu9@users.noreply.github.com> Date: Fri, 24 Jul 2026 11:40:35 -0700 Subject: [PATCH] fix(gateway): align multiplex pairing stores Co-authored-by: x7peeps --- gateway/pairing.py | 49 +++++++--- gateway/run.py | 34 +++++-- hermes_constants.py | 12 ++- .../gateway/test_multiplex_pairing_stores.py | 26 +++++ tests/gateway/test_pairing.py | 96 +++++++++++++++++-- .../gateway/test_unauthorized_dm_behavior.py | 9 +- 6 files changed, 195 insertions(+), 31 deletions(-) diff --git a/gateway/pairing.py b/gateway/pairing.py index 8e568aad2e6..a7c872f9514 100644 --- a/gateway/pairing.py +++ b/gateway/pairing.py @@ -33,7 +33,11 @@ from gateway.whatsapp_identity import ( expand_whatsapp_aliases, normalize_whatsapp_identifier, ) -from hermes_constants import get_hermes_dir, get_hermes_home +from hermes_constants import ( + get_default_hermes_root, + get_hermes_dir, + get_hermes_home, +) from utils import atomic_replace logger = logging.getLogger(__name__) @@ -197,11 +201,15 @@ def _merge_pairing_dir(active_dir: Path, alternate_dir: Path) -> None: _secure_write(dest, json.dumps(merged, indent=2, ensure_ascii=False)) -def _migrate_split_pairing_dirs() -> None: - home = get_hermes_home() +def _migrate_split_pairing_dirs( + *, + home: Optional[Path] = None, + active: Optional[Path] = None, +) -> None: + home = home or get_hermes_home() old_dir = home / "pairing" new_dir = home / "platforms" / "pairing" - active = PAIRING_DIR + active = active or PAIRING_DIR alternate = new_dir if active.resolve() == old_dir.resolve() else old_dir _merge_pairing_dir(active, alternate) @@ -241,26 +249,39 @@ class PairingStore: - {platform}-approved.json : approved (paired) users - _rate_limits.json : rate limit tracking - When constructed with ``profile=""``, storage lives under - ``/profiles//pairing/`` (per-profile, used by - multiplexing gateways so each profile has its own whitelist). - Without a profile, storage is the global ``/pairing/`` - directory (backward-compat for the ``hermes pairing`` CLI). + When constructed with ``profile=""``, storage resolves from that + profile's own HERMES_HOME using the same legacy/consolidated layout rules + as ``hermes -p pairing ...``. This keeps multiplex gateways and + profile-scoped CLI approvals on one whitelist. Without a profile, storage + is the global pairing directory for the current HERMES_HOME. """ def __init__(self, profile: Optional[str] = None): # Resolve storage directory lazily — tests use a temp HERMES_HOME # and PairingStore may be constructed before the env is set. if profile: - from hermes_constants import get_hermes_home - self._dir = get_hermes_home() / "profiles" / profile / "pairing" + root = get_default_hermes_root() + profile_home = ( + root + if profile == "default" + else root / "profiles" / profile + ) + self._dir = get_hermes_dir( + "platforms/pairing", + "pairing", + home=profile_home, + ) else: self._dir = PAIRING_DIR self._dir.mkdir(parents=True, exist_ok=True) - if not profile: + if profile: + # Explicit stores must resolve exactly as a standalone + # ``hermes -p pairing ...`` process does. Merge the + # alternate old/new layout so upgrades cannot split approvals. + _migrate_split_pairing_dirs(home=profile_home, active=self._dir) + else: # Heal installs whose global pairing data ended up split across - # the legacy and new directories (per-profile stores never had - # the legacy/new split). + # the legacy and new directories. _migrate_split_pairing_dirs() # Protects all read-modify-write cycles. The gateway runs multiple # platform adapters concurrently in threads sharing one PairingStore. diff --git a/gateway/run.py b/gateway/run.py index 57cf1755ad0..204b5357871 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -12487,11 +12487,15 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew served = [active] + sorted(self._profile_adapters.keys()) # Per-profile PairingStores so authz_mixin can route pairing # checks to the right whitelist. The active profile gets a store - # at its HERMES_HOME; additional served profiles get one under - # profiles//pairing/. See gateway.pairing.PairingStore. + # at its HERMES_HOME; additional served profiles resolve from + # their own profile homes. See gateway.pairing.PairingStore. for name in served: if name and name not in self.pairing_stores: - self.pairing_stores[name] = PairingStore(profile=name) + self.pairing_stores[name] = ( + self.pairing_store + if name == active + else PairingStore(profile=name) + ) write_runtime_status(served_profiles=served) except Exception: logger.debug("could not record served_profiles", exc_info=True) @@ -13678,23 +13682,39 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew == "pair" ): platform_name = source.platform.value if source.platform else "unknown" + pairing_store = self._pairing_store_for(source) + if pairing_store is None: + logger.error( + "Cannot offer pairing code on %s: no pairing store", + platform_name, + ) + return None # Rate-limit ALL pairing responses (code or rejection) to # prevent spamming the user with repeated messages when # multiple DMs arrive in quick succession. - if self.pairing_store._is_rate_limited(platform_name, source.user_id): + if pairing_store._is_rate_limited(platform_name, source.user_id): return None - code = self.pairing_store.generate_code( + code = pairing_store.generate_code( platform_name, source.user_id, source.user_name or "" ) if code: adapter = self._adapter_for_source(source) if adapter: + store_profile = getattr(pairing_store, "profile", None) + profile_arg = ( + f"-p {store_profile} " + if isinstance(store_profile, str) + and store_profile + and store_profile != "default" + else "" + ) await adapter.send( source.chat_id, f"Hi~ I don't recognize you yet!\n\n" f"Here's your pairing code: `{code}`\n\n" f"Ask the bot owner to run:\n" - f"`hermes pairing approve {platform_name} {code}`" + f"`hermes {profile_arg}pairing approve " + f"{platform_name} {code}`" ) else: adapter = self._adapter_for_source(source) @@ -13705,7 +13725,7 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew "Please try again later!" ) # Record rate limit so subsequent messages are silently ignored - self.pairing_store._record_rate_limit(platform_name, source.user_id) + pairing_store._record_rate_limit(platform_name, source.user_id) return None # Intercept messages that are responses to a pending /update prompt. diff --git a/hermes_constants.py b/hermes_constants.py index bbf45f4d268..e282dc2dcce 100644 --- a/hermes_constants.py +++ b/hermes_constants.py @@ -236,7 +236,12 @@ def get_bundled_skills_dir(default: Path | None = None) -> Path: return get_hermes_home() / "skills" -def get_hermes_dir(new_subpath: str, old_name: str) -> Path: +def get_hermes_dir( + new_subpath: str, + old_name: str, + *, + home: Path | None = None, +) -> Path: """Resolve a Hermes subdirectory with backward compatibility. New installs get the consolidated layout (e.g. ``cache/images``). @@ -254,12 +259,15 @@ def get_hermes_dir(new_subpath: str, old_name: str) -> Path: Args: new_subpath: Preferred path relative to HERMES_HOME (e.g. ``"cache/images"``). old_name: Legacy path relative to HERMES_HOME (e.g. ``"image_cache"``). + home: Optional explicit Hermes home. Profile-aware callers that manage + more than one home in the same process use this instead of + temporarily mutating the process or context-local HERMES_HOME. Returns: Absolute ``Path`` — legacy location if it exists with content, otherwise the new location. """ - home = get_hermes_home() + home = home or get_hermes_home() old_path = home / old_name if _legacy_path_has_content(old_path): return old_path diff --git a/tests/gateway/test_multiplex_pairing_stores.py b/tests/gateway/test_multiplex_pairing_stores.py index 1dbe5844e7b..63c4a9ea9a7 100644 --- a/tests/gateway/test_multiplex_pairing_stores.py +++ b/tests/gateway/test_multiplex_pairing_stores.py @@ -23,6 +23,7 @@ def _bare_runner(multiplex: bool = True): runner.config = MagicMock(multiplex_profiles=multiplex) runner.adapters = {} runner._profile_adapters = {} + runner.pairing_store = MagicMock() runner.pairing_stores = {} return runner @@ -54,8 +55,33 @@ def test_secondary_profile_pairing_stores_created(tmp_path, monkeypatch): assert "default" in runner.pairing_stores, ( "active profile PairingStore missing — the NameError swallow is back" ) + assert runner.pairing_stores["default"] is runner.pairing_store assert "coder" in runner.pairing_stores, ( "secondary profile PairingStore missing — the NameError swallow is back" ) +def test_pairing_store_scoped_to_profile_dir(tmp_path, monkeypatch): + """The created store must live under the profile's pairing directory.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes")) + (tmp_path / ".hermes").mkdir() + + runner = _bare_runner() + + async def _no_secondary(profile_name, profile_home, claimed): + return 0 + + runner._start_one_profile_adapters = _no_secondary + runner._adapter_credential_fingerprint = lambda adapter: None + + with patch("hermes_cli.profiles.profiles_to_serve", return_value=[ + ("ops", tmp_path / ".hermes" / "profiles" / "ops"), + ]), patch("hermes_cli.profiles.get_active_profile_name", return_value="default"): + runner._profile_adapters["ops"] = {} + asyncio.run(runner._start_secondary_profile_adapters()) + + store = runner.pairing_stores["ops"] + assert store.profile == "ops" + assert "profiles/ops/platforms/pairing" in str(store._dir).replace("\\", "/"), ( + f"store not profile-scoped: {store._dir}" + ) diff --git a/tests/gateway/test_pairing.py b/tests/gateway/test_pairing.py index 484283a8cac..8dfaeafebbc 100644 --- a/tests/gateway/test_pairing.py +++ b/tests/gateway/test_pairing.py @@ -557,16 +557,94 @@ class TestUnreadablePairingFile: class TestProfileScopedStorage: """PairingStore(profile="") should isolate per-profile whitelists - under /profiles//pairing/ so a multiplexing gateway - can keep each profile's allowlist separate. + under each profile's own Hermes home so a multiplexing gateway can keep + every profile's allowlist separate. """ + def test_default_store_uses_global_dir(self, tmp_path, monkeypatch): + """PairingStore() (no profile) keeps the legacy global path so the + ``hermes pairing`` CLI continues to work without a profile context.""" + from hermes_constants import get_hermes_home + monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: tmp_path) + # Re-import PAIRING_DIR (it's a module-level constant resolved at + # import time) so the test exercises the right path. We patch it + # rather than re-importing so the assertion is unambiguous. + with patch("gateway.pairing.PAIRING_DIR", tmp_path): + store = PairingStore() + assert store.profile is None + assert store._dir == tmp_path + assert store._approved_path("weixin") == tmp_path / "weixin-approved.json" + + def test_profile_store_uses_profiles_subdir(self, tmp_path, monkeypatch): + """Explicit profile stores use that profile's normal Hermes layout.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + store = PairingStore(profile="yangyang") + assert store.profile == "yangyang" + expected = tmp_path / "profiles" / "yangyang" / "platforms" / "pairing" + assert store._dir == expected + assert store._approved_path("weixin") == expected / "weixin-approved.json" + # Auto-creates the directory + assert expected.is_dir() + + def test_profile_store_matches_profile_cli_home(self, tmp_path, monkeypatch): + """Gateway and ``hermes -p`` must resolve the same pairing store.""" + from hermes_constants import get_hermes_dir + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + profile_home = tmp_path / "profiles" / "coder" + profile_home.mkdir(parents=True) + + gateway_store = PairingStore(profile="coder") + cli_dir = get_hermes_dir( + "platforms/pairing", + "pairing", + home=profile_home, + ) + + assert gateway_store._dir == cli_dir + + def test_default_profile_store_is_global_store(self, tmp_path, monkeypatch): + """Multiplexing must not invent a ``profiles/default`` store.""" + from hermes_constants import get_hermes_dir + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + expected = get_hermes_dir( + "platforms/pairing", + "pairing", + home=tmp_path, + ) + + with patch("gateway.pairing.PAIRING_DIR", expected): + assert PairingStore(profile="default")._dir == PairingStore()._dir + + def test_profile_store_merges_split_pairing_layouts( + self, tmp_path, monkeypatch + ): + """Existing approvals survive either profile directory layout.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + profile_home = tmp_path / "profiles" / "coder" + legacy_dir = profile_home / "pairing" + consolidated_dir = profile_home / "platforms" / "pairing" + legacy_dir.mkdir(parents=True) + consolidated_dir.mkdir(parents=True) + (legacy_dir / "telegram-approved.json").write_text( + '{"legacy-user": {"user_name": "Legacy"}}', + encoding="utf-8", + ) + (consolidated_dir / "telegram-approved.json").write_text( + '{"new-user": {"user_name": "New"}}', + encoding="utf-8", + ) + + store = PairingStore(profile="coder") + + assert store.is_approved("telegram", "legacy-user") + assert store.is_approved("telegram", "new-user") def test_profile_approval_does_not_leak_to_global(self, tmp_path, monkeypatch): """Approving in a profile-scoped store must not appear in the global store — and vice versa. This is the whole point of the fix.""" - from hermes_constants import get_hermes_home - monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) with patch("gateway.pairing.PAIRING_DIR", tmp_path): global_store = PairingStore() profile_store = PairingStore(profile="yangyang") @@ -585,15 +663,19 @@ class TestProfileScopedStorage: def test_profile_uses_distinct_rate_limit_file(self, tmp_path, monkeypatch): """Rate-limit state is per-profile, not shared globally — otherwise one profile's flood would lock out the other profile's users.""" - from hermes_constants import get_hermes_home - monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) with patch("gateway.pairing.PAIRING_DIR", tmp_path): global_store = PairingStore() profile_store = PairingStore(profile="yangyang") assert global_store._rate_limit_path() == tmp_path / "_rate_limits.json" assert profile_store._rate_limit_path() == ( - tmp_path / "profiles" / "yangyang" / "pairing" / "_rate_limits.json" + tmp_path + / "profiles" + / "yangyang" + / "platforms" + / "pairing" + / "_rate_limits.json" ) diff --git a/tests/gateway/test_unauthorized_dm_behavior.py b/tests/gateway/test_unauthorized_dm_behavior.py index f26577e2bfd..c0bd88879df 100644 --- a/tests/gateway/test_unauthorized_dm_behavior.py +++ b/tests/gateway/test_unauthorized_dm_behavior.py @@ -41,7 +41,13 @@ def _clear_auth_env(monkeypatch) -> None: monkeypatch.delenv(key, raising=False) -def _make_event(platform: Platform, user_id: str, chat_id: str) -> MessageEvent: +def _make_event( + platform: Platform, + user_id: str, + chat_id: str, + *, + profile: str | None = None, +) -> MessageEvent: return MessageEvent( text="hello", message_id="m1", @@ -51,6 +57,7 @@ def _make_event(platform: Platform, user_id: str, chat_id: str) -> MessageEvent: chat_id=chat_id, user_name="tester", chat_type="dm", + profile=profile, ), )