mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
* fix(gateway): per-profile pairing whitelist isolation for multiplex gateways Pairing approvals are stored per profile (profiles/<name>/pairing/) and authz routes pairing checks through the serving profile's store, so one profile's approved users no longer authorize against every other profile's whitelist in multiplex mode. The global store remains for the hermes pairing CLI and single-profile gateways; unregistered/unstamped sources fall back to it, preserving existing behavior. Salvaged from PR #53045 (pairing half). The SOUL.md half was dropped: the agent turn already runs inside _profile_runtime_scope on main, so load_soul_md() resolves per-profile without changes. Original work by @soddy022. * ci: redispatch after arm64 docker dashboard-slot flake (unrelated to this PR) --------- Co-authored-by: soddy022 <290613374+soddy022@users.noreply.github.com>
This commit is contained in:
Teknium
GitHub
parent
088b989442
commit
249c69b958
4 changed files with 166 additions and 8 deletions
|
|
@ -246,6 +246,21 @@ class GatewayAuthorizationMixin:
|
|||
return any(str(item).strip() for item in sender_allow)
|
||||
return False
|
||||
|
||||
def _pairing_store_for(self, source: "SessionSource"):
|
||||
"""Pick the per-profile PairingStore for a source, falling back to global.
|
||||
|
||||
In a multiplexing gateway, each profile owns its own pairing whitelist
|
||||
so isolation is preserved. When the source has no profile (single-
|
||||
profile gateway, or a path that hasn't stamped profile yet) or the
|
||||
profile isn't registered, fall back to ``self.pairing_store`` (the
|
||||
global default) so existing behavior is preserved.
|
||||
"""
|
||||
per_profile = getattr(self, "pairing_stores", None) or {}
|
||||
profile = getattr(source, "profile", None)
|
||||
if profile and profile in per_profile:
|
||||
return per_profile[profile]
|
||||
return getattr(self, "pairing_store", None)
|
||||
|
||||
def _is_user_authorized(self, source: SessionSource) -> bool:
|
||||
"""
|
||||
Check if a user is authorized to use the bot.
|
||||
|
|
@ -430,10 +445,14 @@ class GatewayAuthorizationMixin:
|
|||
# allowlist IS configured, operator approval also writes the user into
|
||||
# that allowlist (see PairingStore._approve_user), keeping a single
|
||||
# operator-visible source of truth. (#23778: the original bypass was the
|
||||
# inbound message/approval-button gate, not this grant; that gate is
|
||||
# inbound message/approval-button gate, not this gate; that gate is
|
||||
# fixed separately.)
|
||||
# In multiplex gateways, route to the per-profile PairingStore so each
|
||||
# profile's whitelist is isolated; falls back to the global store when
|
||||
# the source has no profile or the profile isn't registered.
|
||||
platform_name = source.platform.value if source.platform else ""
|
||||
if self.pairing_store.is_approved(platform_name, user_id):
|
||||
pairing_store = self._pairing_store_for(source)
|
||||
if pairing_store is not None and pairing_store.is_approved(platform_name, user_id):
|
||||
return True
|
||||
|
||||
# Check platform-specific and global allowlists
|
||||
|
|
|
|||
|
|
@ -195,22 +195,41 @@ class PairingStore:
|
|||
- {platform}-pending.json : pending pairing requests
|
||||
- {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).
|
||||
"""
|
||||
|
||||
def __init__(self):
|
||||
PAIRING_DIR.mkdir(parents=True, exist_ok=True)
|
||||
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"
|
||||
else:
|
||||
self._dir = PAIRING_DIR
|
||||
self._dir.mkdir(parents=True, exist_ok=True)
|
||||
# Protects all read-modify-write cycles. The gateway runs multiple
|
||||
# platform adapters concurrently in threads sharing one PairingStore.
|
||||
self._lock = threading.RLock()
|
||||
self._profile = profile # for diagnostics / log lines
|
||||
|
||||
@property
|
||||
def profile(self) -> Optional[str]:
|
||||
"""Profile name this store is scoped to, or None for the global store."""
|
||||
return self._profile
|
||||
|
||||
def _pending_path(self, platform: str) -> Path:
|
||||
return PAIRING_DIR / f"{platform}-pending.json"
|
||||
return self._dir / f"{platform}-pending.json"
|
||||
|
||||
def _approved_path(self, platform: str) -> Path:
|
||||
return PAIRING_DIR / f"{platform}-approved.json"
|
||||
return self._dir / f"{platform}-approved.json"
|
||||
|
||||
def _rate_limit_path(self) -> Path:
|
||||
return PAIRING_DIR / "_rate_limits.json"
|
||||
return self._dir / "_rate_limits.json"
|
||||
|
||||
def _load_json(self, path: Path) -> dict:
|
||||
if path.exists():
|
||||
|
|
|
|||
|
|
@ -3047,9 +3047,15 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
except Exception as exc:
|
||||
logger.debug("checkpoint auto-maintenance skipped: %s", exc)
|
||||
|
||||
# DM pairing store for code-based user authorization
|
||||
# DM pairing store for code-based user authorization.
|
||||
# ``pairing_store`` stays as the global/default store for the
|
||||
# ``hermes pairing`` CLI and any caller without a profile context.
|
||||
# ``pairing_stores`` is the per-profile map used by
|
||||
# ``authz_mixin._is_user_authorized`` to route checks to the right
|
||||
# whitelist (one per profile in multiplex mode).
|
||||
from gateway.pairing import PairingStore
|
||||
self.pairing_store = PairingStore()
|
||||
self.pairing_stores: Dict[str, "PairingStore"] = {}
|
||||
|
||||
# Event hook system
|
||||
from gateway.hooks import HookRegistry
|
||||
|
|
@ -8335,6 +8341,13 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
try:
|
||||
from gateway.status import write_runtime_status
|
||||
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.
|
||||
for name in served:
|
||||
if name and name not in self.pairing_stores:
|
||||
self.pairing_stores[name] = PairingStore(profile=name)
|
||||
write_runtime_status(served_profiles=served)
|
||||
except Exception:
|
||||
logger.debug("could not record served_profiles", exc_info=True)
|
||||
|
|
|
|||
|
|
@ -720,3 +720,110 @@ class TestUnreadablePairingFile:
|
|||
# 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"
|
||||
# Profile-scoped storage (multiplexing gateway isolation)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
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.
|
||||
"""
|
||||
|
||||
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):
|
||||
"""PairingStore(profile="yangyang") puts files under
|
||||
<HERMES_HOME>/profiles/yangyang/pairing/."""
|
||||
from hermes_constants import get_hermes_home
|
||||
monkeypatch.setattr("hermes_constants.get_hermes_home", lambda: tmp_path)
|
||||
store = PairingStore(profile="yangyang")
|
||||
assert store.profile == "yangyang"
|
||||
expected = tmp_path / "profiles" / "yangyang" / "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_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)
|
||||
with patch("gateway.pairing.PAIRING_DIR", tmp_path):
|
||||
global_store = PairingStore()
|
||||
profile_store = PairingStore(profile="yangyang")
|
||||
|
||||
# Approve in the profile store
|
||||
profile_store._approve_user("weixin", "yangyang_user", "杨洋")
|
||||
# And in the global store, a different user
|
||||
global_store._approve_user("weixin", "global_user", "Default")
|
||||
|
||||
# Cross-isolation: each store only sees its own user
|
||||
assert profile_store.is_approved("weixin", "yangyang_user") is True
|
||||
assert profile_store.is_approved("weixin", "global_user") is False
|
||||
assert global_store.is_approved("weixin", "global_user") is True
|
||||
assert global_store.is_approved("weixin", "yangyang_user") is False
|
||||
|
||||
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)
|
||||
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"
|
||||
)
|
||||
|
||||
def test_pairing_store_for_helper_routes_by_profile(self, tmp_path, monkeypatch):
|
||||
"""_pairing_store_for(source) on a gateway-like object picks the
|
||||
per-profile store when source.profile is set, and falls back to
|
||||
the global store when it isn't (defensive — single-profile
|
||||
gateways, or any code path that hasn't stamped source.profile)."""
|
||||
from gateway.session import SessionSource
|
||||
from gateway.config import Platform
|
||||
|
||||
class FakeGateway:
|
||||
def __init__(self):
|
||||
self.pairing_store = object() # sentinel
|
||||
self.pairing_stores = {
|
||||
"default": "default-store",
|
||||
"yangyang": "yangyang-store",
|
||||
}
|
||||
|
||||
# Method under test — copy of the real helper so this test
|
||||
# is self-contained even if the real one moves.
|
||||
def _pairing_store_for(self, source):
|
||||
per_profile = getattr(self, "pairing_stores", None) or {}
|
||||
profile = getattr(source, "profile", None)
|
||||
if profile and profile in per_profile:
|
||||
return per_profile[profile]
|
||||
return getattr(self, "pairing_store", None)
|
||||
|
||||
g = FakeGateway()
|
||||
# source with profile="yangyang" → per-profile store
|
||||
s_yy = SessionSource(platform=Platform.WEIXIN, chat_id="c", profile="yangyang")
|
||||
assert g._pairing_store_for(s_yy) == "yangyang-store"
|
||||
# source with no profile → fallback to global
|
||||
s_none = SessionSource(platform=Platform.WEIXIN, chat_id="c")
|
||||
assert g._pairing_store_for(s_none) is g.pairing_store
|
||||
# source with an unknown profile → fallback (defensive)
|
||||
s_unknown = SessionSource(platform=Platform.WEIXIN, chat_id="c", profile="ghost")
|
||||
assert g._pairing_store_for(s_unknown) is g.pairing_store
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue