mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(gateway): align multiplex pairing stores
Co-authored-by: x7peeps <x7peeps@users.noreply.github.com>
This commit is contained in:
parent
75657f89c4
commit
a6397c379b
6 changed files with 195 additions and 31 deletions
|
|
@ -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="<name>"``, storage lives under
|
||||
``<HERMES_HOME>/profiles/<name>/pairing/`` (per-profile, used by
|
||||
multiplexing gateways so each profile has its own whitelist).
|
||||
Without a profile, storage is the global ``<HERMES_HOME>/pairing/``
|
||||
directory (backward-compat for the ``hermes pairing`` CLI).
|
||||
When constructed with ``profile="<name>"``, storage resolves from that
|
||||
profile's own HERMES_HOME using the same legacy/consolidated layout rules
|
||||
as ``hermes -p <name> 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 <profile> 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.
|
||||
|
|
|
|||
|
|
@ -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/<name>/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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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}"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -557,16 +557,94 @@ class TestUnreadablePairingFile:
|
|||
|
||||
class TestProfileScopedStorage:
|
||||
"""PairingStore(profile="<name>") should isolate per-profile whitelists
|
||||
under <HERMES_HOME>/profiles/<name>/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"
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
),
|
||||
)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue