diff --git a/apps/desktop/src/hermes.ts b/apps/desktop/src/hermes.ts index 529fc935d5..9b85e04c27 100644 --- a/apps/desktop/src/hermes.ts +++ b/apps/desktop/src/hermes.ts @@ -1185,7 +1185,9 @@ export function approvePairing(platform: string, requestId: string): Promise<{ o ...profileScoped(), path: '/api/pairing/approve', method: 'POST', - body: { platform, request_id: requestId } + // These endpoints read the profile off the body, not the query string — + // `profileScoped()` alone would approve into the wrong profile's store. + body: { platform, request_id: requestId, ...profileScoped() } }) } @@ -1194,7 +1196,7 @@ export function revokePairing(platform: string, userId: string): Promise<{ ok: b ...profileScoped(), path: '/api/pairing/revoke', method: 'POST', - body: { platform, user_id: userId } + body: { platform, user_id: userId, ...profileScoped() } }) } diff --git a/apps/desktop/src/pairing-scope.test.ts b/apps/desktop/src/pairing-scope.test.ts new file mode 100644 index 0000000000..170efa13b0 --- /dev/null +++ b/apps/desktop/src/pairing-scope.test.ts @@ -0,0 +1,43 @@ +// Pairing writes must target the profile the user is actually looking at. +// The approve/revoke endpoints read `profile` off the BODY (a POST body is +// not touched by query-param scoping), so a request that only carried +// `profileScoped()` at the top level would approve into the default +// profile's whitelist while the operator was managing another one — a grant +// the running gateway for that profile never consults. +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const api = vi.fn().mockResolvedValue({ ok: true }) + +vi.stubGlobal('window', { hermesDesktop: { api } }) + +describe('pairing requests carry the active profile', () => { + beforeEach(() => api.mockClear()) + + it('scopes approve and revoke by body, and the listing by query', async () => { + const mod = await import('@/hermes') + mod.setApiRequestProfile('work') + + await mod.approvePairing('telegram', 'a'.repeat(16)) + await mod.revokePairing('telegram', 'U1') + await mod.getPairing() + + const [approve, revoke, list] = api.mock.calls.map(call => call[0]) + + expect(approve.body.profile).toBe('work') + expect(revoke.body.profile).toBe('work') + expect(list.profile).toBe('work') + }) + + it('omits the profile entirely for single-profile users', async () => { + const mod = await import('@/hermes') + mod.setApiRequestProfile(null) + + await mod.approvePairing('telegram', 'a'.repeat(16)) + await mod.getPairing() + + const [approve, list] = api.mock.calls.map(call => call[0]) + + expect(approve.body.profile).toBeUndefined() + expect(list.profile).toBeUndefined() + }) +}) diff --git a/hermes_cli/web_models.py b/hermes_cli/web_models.py index ad24fca6b8..3ff438c638 100644 --- a/hermes_cli/web_models.py +++ b/hermes_cli/web_models.py @@ -430,11 +430,13 @@ class PairingApprove(BaseModel): platform: str code: str = "" request_id: str = "" + profile: Optional[str] = None class PairingRevoke(BaseModel): platform: str user_id: str + profile: Optional[str] = None # --- from web_server.py (originally lines 13793-13804) --- diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index cf068da557..7ee26c138f 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -12008,15 +12008,33 @@ _ACTION_LOG_FILES.setdefault("computer-use-grant", "action-computer-use-grant.lo # --------------------------------------------------------------------------- -def _pairing_store(): +def _pairing_store(profile: Optional[str] = None): + """Pairing store for ``profile`` — the dashboard's own when unspecified. + + Every other admin endpoint scopes by profile, and the gateway already + keeps one store per served profile (``gateway/run.py``). Without this the + dashboard and desktop always read the global store, so an operator on a + named profile approves into a whitelist their gateway never consults. + + ``PairingStore`` resolves the profile's home itself (``default`` maps back + to the global store), so this only needs to validate the name — no + ``_profile_scope`` needed, and nothing process-global is swapped across + the ``await`` boundary. + """ from gateway.pairing import PairingStore - return PairingStore() + requested = (profile or "").strip() + if not requested or requested.lower() == "current": + return PairingStore() + + _resolve_profile_dir(requested) # 400/404 on an unknown profile + + return PairingStore(profile=requested) @app.get("/api/pairing") -async def list_pairing(): - store = _pairing_store() +async def list_pairing(profile: Optional[str] = None): + store = _pairing_store(profile) return { "pending": store.list_pending(), "approved": store.list_approved(), @@ -12025,7 +12043,7 @@ async def list_pairing(): @app.post("/api/pairing/approve") async def approve_pairing(body: PairingApprove): - store = _pairing_store() + store = _pairing_store(body.profile) platform = (body.platform or "").lower().strip() # `request_id` is what an admin surface sends after listing pending # requests; `code` is the one-time code the user relays from their DM. @@ -12061,7 +12079,7 @@ async def approve_pairing(body: PairingApprove): @app.post("/api/pairing/revoke") async def revoke_pairing(body: PairingRevoke): - store = _pairing_store() + store = _pairing_store(body.profile) platform = (body.platform or "").lower().strip() if not platform or not body.user_id: raise HTTPException(status_code=400, detail="platform and user_id are required") @@ -12074,8 +12092,8 @@ async def revoke_pairing(body: PairingRevoke): @app.post("/api/pairing/clear-pending") -async def clear_pending_pairing(): - store = _pairing_store() +async def clear_pending_pairing(profile: Optional[str] = None): + store = _pairing_store(profile) count = store.clear_pending() return {"ok": True, "cleared": count} diff --git a/tests/gateway/test_pairing.py b/tests/gateway/test_pairing.py index 8dfaeafebb..b377eb175e 100644 --- a/tests/gateway/test_pairing.py +++ b/tests/gateway/test_pairing.py @@ -58,19 +58,18 @@ class TestProfileScopedDiscovery: global_dir = tmp_path / "global-pairing" global_dir.mkdir(parents=True) - # PairingStore.__init__ resolves the profile dir via a *function-local* - # ``from hermes_constants import get_hermes_home``, so the patch must - # target ``hermes_constants.get_hermes_home`` (the source module) — not - # ``gateway.pairing.get_hermes_home`` (the module-global binding, which - # the local re-import bypasses). Patching the wrong target would leave - # ``self._dir`` rooted at the real HERMES_HOME. + # A profile's store anchors to the hermes ROOT, not the current + # HERMES_HOME — the current home may itself be a profile, and nesting + # profiles inside profiles is how a `-p work` CLI and its gateway end + # up reading different files. Patch that seam, not get_hermes_home. with patch("gateway.pairing.PAIRING_DIR", global_dir), patch( - "hermes_constants.get_hermes_home", return_value=home + "gateway.pairing.get_default_hermes_root", return_value=home ): store = PairingStore(profile="alice") - # Store lives under the mocked home's profile dir — provably scoped - # there, and distinct from the module-global PAIRING_DIR. - assert store._dir == home / "profiles" / "alice" / "pairing" + # Scoped under the mocked root's profile dir, using the same + # consolidated layout a standalone `hermes -p alice` resolves — + # and provably distinct from the module-global PAIRING_DIR. + assert store._dir == home / "profiles" / "alice" / "platforms" / "pairing" assert store._dir != global_dir with store._lock: store._approve_user("telegram", "tg-456", "Bob") diff --git a/tests/hermes_cli/test_dashboard_admin_endpoints.py b/tests/hermes_cli/test_dashboard_admin_endpoints.py index f155d58f28..42dc5e997b 100644 --- a/tests/hermes_cli/test_dashboard_admin_endpoints.py +++ b/tests/hermes_cli/test_dashboard_admin_endpoints.py @@ -273,6 +273,49 @@ class TestPairingEndpoints: assert r.json()["user"]["user_id"] == "user1" assert self.client.get("/api/pairing").json()["pending"] == [] + def test_pairing_is_isolated_per_profile(self): + """A named profile's approvals must land in the store its own gateway reads. + + The gateway keeps one PairingStore per served profile, so an approval + written to the global store grants access the running gateway never + consults — the user stays locked out with the dashboard showing them + as approved. + """ + from gateway.pairing import PairingStore + from hermes_constants import get_hermes_home + + (get_hermes_home() / "profiles" / "work").mkdir(parents=True, exist_ok=True) + PairingStore().generate_code("telegram", "global-1", "GlobalGuy") + PairingStore(profile="work").generate_code("telegram", "work-1", "WorkGal") + + listed = self.client.get("/api/pairing?profile=work").json()["pending"] + assert [row["user_id"] for row in listed] == ["work-1"] + + r = self.client.post( + "/api/pairing/approve", + json={ + "platform": "telegram", + "request_id": listed[0]["request_id"], + "profile": "work", + }, + ) + assert r.status_code == 200 + + # The grant is visible to the profile's own store — what the gateway reads. + assert PairingStore(profile="work").is_approved("telegram", "work-1") is True + + # ...and it never leaked into the global store, whose own pending row + # is still waiting. (Asserted against this user rather than an empty + # list: the module-level PAIRING_DIR is bound at import, so the global + # store carries whatever earlier cases in this class approved.) + global_view = self.client.get("/api/pairing").json() + assert PairingStore().is_approved("telegram", "work-1") is False + assert "work-1" not in [row["user_id"] for row in global_view["approved"]] + assert "global-1" in [row["user_id"] for row in global_view["pending"]] + + def test_unknown_profile_is_rejected(self): + assert self.client.get("/api/pairing?profile=ghost").status_code == 404 + class TestWebhookEndpoints: @pytest.fixture(autouse=True) diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 2bd7a4b179..3cc2c5a0ad 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -61,8 +61,8 @@ export function getManagementProfile(): string { // Endpoint families that honor ?profile= on the backend (web_server.py // _profile_scope or explicit per-profile DB opens). Anything else — ops, -// pairing, cron (which has its own per-job profile params), profiles -// themselves — is machine-global or self-scoped and must NOT be rewritten. +// cron (which has its own per-job profile params), profiles themselves — is +// machine-global or self-scoped and must NOT be rewritten. const PROFILE_SCOPED_PREFIXES = [ "/api/status", "/api/gateway", @@ -80,6 +80,10 @@ const PROFILE_SCOPED_PREFIXES = [ "/api/model/auxiliary", "/api/model/moa", "/api/model/options", + // A named profile keeps its own pairing whitelist, and its gateway only + // consults that one — approving into the global store would grant access + // the running gateway never sees. + "/api/pairing", ]; function withManagementProfile(url: string): string { @@ -1090,18 +1094,29 @@ export const api = { ), // ── Admin: Pairing ────────────────────────────────────────────────── + // The mutating endpoints read the profile off the BODY, so the query-param + // rewrite in withManagementProfile doesn't reach them — send it explicitly + // or an approval lands in the wrong profile's whitelist. getPairing: () => fetchJSON("/api/pairing"), approvePairing: (platform: string, request_id: string) => fetchJSON<{ ok: boolean; user: PairingUser }>("/api/pairing/approve", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ platform, request_id }), + body: JSON.stringify({ + platform, + request_id, + profile: getManagementProfile() || undefined, + }), }), revokePairing: (platform: string, user_id: string) => fetchJSON<{ ok: boolean }>("/api/pairing/revoke", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ platform, user_id }), + body: JSON.stringify({ + platform, + user_id, + profile: getManagementProfile() || undefined, + }), }), clearPendingPairing: () => fetchJSON<{ ok: boolean; cleared: number }>("/api/pairing/clear-pending", {