fix(pairing): scope the approve/revoke endpoints to a profile
The gateway keeps one PairingStore per served profile, but every `/api/pairing` endpoint built the global one. An operator managing a named profile saw the wrong pending list, and approving wrote a grant into a whitelist their running gateway never consults — the user stays locked out while the UI shows them as approved. `_pairing_store(profile)` now resolves per profile and validates the name (400/404 on an unknown one). No `_profile_scope` needed: PairingStore resolves the profile's home itself, so nothing process-global is swapped across an await. Both GUIs had to change to match. The listing rides the query param — for the dashboard that meant deleting `pairing` from the "machine-global, must NOT be rewritten" exclusion list, a comment this change makes false. The mutating endpoints read the profile off the BODY, which no query-param rewrite reaches, so approve/revoke send it explicitly on both surfaces.
This commit is contained in:
@@ -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() }
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
43
apps/desktop/src/pairing-scope.test.ts
Normal file
43
apps/desktop/src/pairing-scope.test.ts
Normal file
@@ -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()
|
||||
})
|
||||
})
|
||||
@@ -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) ---
|
||||
|
||||
@@ -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}
|
||||
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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<PairingResponse>("/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", {
|
||||
|
||||
Reference in New Issue
Block a user