From cd8ea6f0f4116e61c7edd303b20682dcc1f8172c Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 29 Jul 2026 18:18:17 -0500 Subject: [PATCH 1/7] feat(desktop): approve pairing requests from the messaging page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Desktop had no pairing surface at all. Someone DMs the bot, gets a code, and lands in pending — where only the dashboard or `hermes pairing approve` could let them in. That gap is why the dashboard's broken approve button went unnoticed for so long: desktop users never saw the flow. This puts it where the decision already lives. The messaging page is already master-detail by platform, already polls every 6s, and already owns "who can talk to this bot" — the allowlist env vars in the same detail pane are what an approval writes into. A pending block sits above the credential fields (approving is the hot path); a count badge on the platform row is the discovery mechanism, so you see "Telegram 2" whenever you open Messaging for any reason. Neither renders when nobody is waiting: approvals are rare, and a permanent empty state would be chrome on a page that is otherwise about credentials. Approval sends the row's request_id and never a code — the code is the requester's proof that the channel is theirs and is never returned by the API. Rows paint optimistically from a snapshot and roll back visibly on failure, and a 429 gets its own message since the code path's lockout is a condition the operator can only wait out. Pairing rides the existing platform refresh via allSettled, so a backend without the endpoint yields no rows instead of blanking the page. --- apps/desktop/src/app/messaging/index.test.tsx | 76 +++++++ apps/desktop/src/app/messaging/index.tsx | 211 +++++++++++++++++- apps/desktop/src/hermes.ts | 36 +++ apps/desktop/src/i18n/en.ts | 17 ++ apps/desktop/src/i18n/types.ts | 17 ++ apps/desktop/src/i18n/zh.ts | 17 ++ apps/desktop/src/types/hermes.ts | 15 ++ 7 files changed, 383 insertions(+), 6 deletions(-) diff --git a/apps/desktop/src/app/messaging/index.test.tsx b/apps/desktop/src/app/messaging/index.test.tsx index b078a2043b1..be451825de4 100644 --- a/apps/desktop/src/app/messaging/index.test.tsx +++ b/apps/desktop/src/app/messaging/index.test.tsx @@ -7,10 +7,16 @@ import type { MessagingPlatformInfo } from '@/types/hermes' const getMessagingPlatforms = vi.fn() const updateMessagingPlatform = vi.fn() +const getPairing = vi.fn() +const approvePairing = vi.fn() +const revokePairing = vi.fn() const openExternalLink = vi.fn() vi.mock('@/hermes', () => ({ + approvePairing: (platformId: string, requestId: string) => approvePairing(platformId, requestId), getMessagingPlatforms: () => getMessagingPlatforms(), + getPairing: () => getPairing(), + revokePairing: (platformId: string, userId: string) => revokePairing(platformId, userId), updateMessagingPlatform: (id: string, body: unknown) => updateMessagingPlatform(id, body) })) @@ -44,6 +50,7 @@ function platform(patch: Partial = {}): MessagingPlatform beforeEach(() => { updateMessagingPlatform.mockResolvedValue({ ok: true, platform: 'teams' }) + getPairing.mockResolvedValue({ approved: [], pending: [] }) }) afterEach(() => { @@ -93,3 +100,72 @@ describe('MessagingView setup-guide link', () => { await waitFor(() => expect(openExternalLink).toHaveBeenCalledWith(docsUrl)) }) }) + +describe('MessagingView pairing', () => { + const pendingUser = { + age_minutes: 3, + platform: 'teams', + request_id: 'a1b2c3d4e5f60718', + user_id: '7712345', + user_name: 'Bee' + } + + it('approves the listed request by its request id, never by a code', async () => { + // The whole point of the request-id grant path: the UI can only ever send + // the server-side row id, because the one-time code is never returned by + // the API. Posting anything derived from the code could not be approved. + getMessagingPlatforms.mockResolvedValue({ platforms: [platform()] }) + getPairing.mockResolvedValue({ approved: [], pending: [pendingUser] }) + approvePairing.mockResolvedValue({ ok: true, user: { user_id: '7712345', user_name: 'Bee' } }) + + await renderMessaging() + + const approve = await screen.findByRole('button', { name: 'Approve' }) + await act(async () => { + fireEvent.click(approve) + }) + + await waitFor(() => expect(approvePairing).toHaveBeenCalledWith('teams', 'a1b2c3d4e5f60718')) + }) + + it('restores the pending row when approval fails', async () => { + // Optimistic removal must not silently swallow the request: a failed + // approve has to leave the operator something to retry. + getMessagingPlatforms.mockResolvedValue({ platforms: [platform()] }) + getPairing.mockResolvedValue({ approved: [], pending: [pendingUser] }) + approvePairing.mockRejectedValue(new Error('500 boom')) + + await renderMessaging() + + await act(async () => { + fireEvent.click(await screen.findByRole('button', { name: 'Approve' })) + }) + + expect(await screen.findByRole('button', { name: 'Approve' })).toBeTruthy() + expect(screen.getByText('Bee')).toBeTruthy() + }) + + it('shows no pairing affordance when nobody is waiting', async () => { + // Approvals are rare; an always-present empty state would be permanent + // chrome on a page that is otherwise about credentials. + getMessagingPlatforms.mockResolvedValue({ platforms: [platform()] }) + getPairing.mockResolvedValue({ approved: [], pending: [] }) + + await renderMessaging() + + expect((await screen.findAllByText('Microsoft Teams')).length).toBeGreaterThan(0) + expect(screen.queryByRole('button', { name: 'Approve' })).toBeNull() + expect(screen.queryByText(/Pending requests/)).toBeNull() + }) + + it('still renders platforms when the pairing endpoint fails', async () => { + // An older backend without the endpoint must not blank the page. + getMessagingPlatforms.mockResolvedValue({ platforms: [platform()] }) + getPairing.mockRejectedValue(new Error('404 not found')) + + await renderMessaging() + + expect((await screen.findAllByText('Microsoft Teams')).length).toBeGreaterThan(0) + expect(screen.queryByRole('button', { name: 'Approve' })).toBeNull() + }) +}) diff --git a/apps/desktop/src/app/messaging/index.tsx b/apps/desktop/src/app/messaging/index.tsx index 08ae2a44404..a6828db62c3 100644 --- a/apps/desktop/src/app/messaging/index.tsx +++ b/apps/desktop/src/app/messaging/index.tsx @@ -6,14 +6,19 @@ import { PageLoader } from '@/components/page-loader' import { StatusDot, type StatusTone } from '@/components/status-dot' import { Button } from '@/components/ui/button' import { DisclosureCaret } from '@/components/ui/disclosure-caret' +import { ConfirmDialog } from '@/components/ui/confirm-dialog' import { ErrorBanner } from '@/components/ui/error-state' import { Input } from '@/components/ui/input' import { Switch } from '@/components/ui/switch' import { Tip } from '@/components/ui/tooltip' import { + approvePairing, getMessagingPlatforms, + getPairing, type MessagingEnvVarInfo, type MessagingPlatformInfo, + type PairingUser, + revokePairing, updateMessagingPlatform } from '@/hermes' import { type Translations, useI18n } from '@/i18n' @@ -74,6 +79,22 @@ const trimEdits = (edits: Record): Record => .filter(([, v]) => v) ) +/** Stable row identity: a user id is only unique within its platform. */ +const pairingKey = (user: PairingUser) => `${user.platform}:${user.user_id}` + +const pairingLabel = (user: PairingUser) => user.user_name || user.user_id + +/** Group pairing rows by platform id so a detail pane can slice its own. */ +function byPlatform(rows: PairingUser[]): Record { + const grouped: Record = {} + + for (const row of rows) { + ;(grouped[row.platform] ||= []).push(row) + } + + return grouped +} + const FIELD_COPY: Record = { TELEGRAM_PROXY: { advanced: true }, DISCORD_REPLY_TO_MODE: { advanced: true }, @@ -108,6 +129,12 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . // Both save/toggle toasts offer the same one-click restart. const restartGatewayAction = { label: t.commandCenter.restartGateway, onClick: () => void runGatewayRestart() } const [platforms, setPlatforms] = useState(null) + const [pairing, setPairing] = useState<{ approved: PairingUser[]; pending: PairingUser[] }>({ + approved: [], + pending: [] + }) + const [approving, setApproving] = useState(null) + const [pendingRevoke, setPendingRevoke] = useState(null) const [edits, setEdits] = useState({}) const [query, setQuery] = useState('') const [refreshing, setRefreshing] = useState(false) @@ -122,11 +149,22 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . } try { - const result = await getMessagingPlatforms() - setPlatforms(result.platforms) - } catch (err) { - if (!silent) { - notifyError(err, m.loadFailed) + // Pairing rides the same refresh as platform status: both feed this + // page and a lone failure shouldn't blank the other. A backend older + // than the request-id endpoint just yields no rows. + const [result, pairingResult] = await Promise.allSettled([getMessagingPlatforms(), getPairing()]) + + if (result.status === 'fulfilled') { + setPlatforms(result.value.platforms) + } else if (!silent) { + notifyError(result.reason, m.loadFailed) + } + + if (pairingResult.status === 'fulfilled') { + setPairing({ + approved: pairingResult.value.approved ?? [], + pending: pairingResult.value.pending ?? [] + }) } } finally { if (!silent) { @@ -189,6 +227,9 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . return platforms.find(platform => platform.id === selectedId) || platforms[0] || null }, [platforms, selectedId]) + const pendingByPlatform = useMemo(() => byPlatform(pairing.pending), [pairing.pending]) + const approvedByPlatform = useMemo(() => byPlatform(pairing.approved), [pairing.approved]) + const visiblePlatforms = useMemo(() => { if (!platforms) { return [] @@ -284,6 +325,57 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . } } + // Approve/revoke paint from a snapshot immediately, then let the + // authoritative refresh have the last word. A failed write restores the + // snapshot so the row never silently disappears on an error. + async function handleApprove(user: PairingUser) { + if (!user.request_id) { + return + } + + const key = pairingKey(user) + const snapshot = pairing + setApproving(key) + setPairing(current => ({ + approved: current.approved, + pending: current.pending.filter(row => pairingKey(row) !== key) + })) + + try { + await approvePairing(user.platform, user.request_id) + notify({ kind: 'success', title: m.approvedUser(pairingLabel(user)), message: m.approvedHint }) + await refreshPlatforms(true) + } catch (err) { + setPairing(snapshot) + // 429 is the code path's brute-force lockout — a distinct condition the + // operator can only wait out, so it gets its own message. + const lockedOut = err instanceof Error && err.message.includes('429') + notifyError(err, lockedOut ? m.pairingLockedOut : m.failedApprove(pairingLabel(user))) + } finally { + setApproving(null) + } + } + + // ConfirmDialog owns the pending → done → close beat and shows an inline + // error when onConfirm throws, so this rethrows instead of swallowing. + async function handleRevoke(user: PairingUser) { + const key = pairingKey(user) + const snapshot = pairing + setPairing(current => ({ + approved: current.approved.filter(row => pairingKey(row) !== key), + pending: current.pending + })) + + try { + await revokePairing(user.platform, user.user_id) + notify({ kind: 'success', title: m.revokedUser(pairingLabel(user)), message: user.platform }) + await refreshPlatforms(true) + } catch (err) { + setPairing(snapshot) + throw err + } + } + return ( setSelectedId(platform.id)} + pendingCount={pendingByPlatform[platform.id]?.length ?? 0} platform={platform} /> @@ -326,7 +419,10 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . > {selected && ( void handleApprove(user)} onClear={key => void handleClear(selected, key)} onEdit={(key, value) => setEdits(current => ({ @@ -337,6 +433,8 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . } })) } + onRevoke={setPendingRevoke} + pending={pendingByPlatform[selected.id] ?? []} platform={selected} saving={saving} /> @@ -344,6 +442,18 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . )} + + setPendingRevoke(null)} + onConfirm={() => (pendingRevoke ? handleRevoke(pendingRevoke) : undefined)} + open={Boolean(pendingRevoke)} + title={m.revokeTitle} + /> ) } @@ -351,12 +461,16 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . function PlatformRow({ active, onSelect, + pendingCount, platform }: { active: boolean onSelect: () => void + pendingCount: number platform: MessagingPlatformInfo }) { + const { t } = useI18n() + return ( ) } function PlatformDetail({ + approved, + approving, edits, + onApprove, onClear, onEdit, + onRevoke, + pending, platform, saving }: { + approved: PairingUser[] + approving: null | string edits: Record + onApprove: (user: PairingUser) => void onClear: (key: string) => void onEdit: (key: string, value: string) => void + onRevoke: (user: PairingUser) => void + pending: PairingUser[] platform: MessagingPlatformInfo saving: string | null }) { @@ -418,6 +557,66 @@ function PlatformDetail({ {platform.error_message && {platform.error_message}} + {/* Pending pairing requests. Rendered only when someone is actually + waiting — an empty-state card here would be permanent chrome on a + page that is usually about credentials, not approvals. */} + {pending.length > 0 && ( +
+ {m.pendingRequests(pending.length)} +
+ {pending.map(user => { + const busy = approving === pairingKey(user) + const waited = typeof user.age_minutes === 'number' ? m.waitingSince(user.age_minutes) : null + + return ( + onApprove(user)} + size="sm" + variant="secondary" + > + {busy ? m.approving : m.approve} + + } + // An unnamed requester is only a user id — showing it as + // both title and description just repeats itself. + description={[user.user_name ? user.user_id : null, waited].filter(Boolean).join(' · ')} + key={pairingKey(user)} + title={pairingLabel(user)} + /> + ) + })} +
+
+ )} + + {approved.length > 0 && ( +
+ {m.approvedUsers(approved.length)} +
+ {approved.map(user => ( + onRevoke(user)} + size="sm" + variant="ghost" + > + {m.revoke} + + } + description={user.user_name ? user.user_id : undefined} + key={pairingKey(user)} + title={pairingLabel(user)} + /> + ))} +
+
+ )} +
{m.getCredentials}

diff --git a/apps/desktop/src/hermes.ts b/apps/desktop/src/hermes.ts index f94f0d2d741..529fc935d5e 100644 --- a/apps/desktop/src/hermes.ts +++ b/apps/desktop/src/hermes.ts @@ -43,6 +43,8 @@ import type { OAuthStartResponse, OAuthSubmitResponse, PaginatedSessions, + PairingResponse, + PairingUser, ProfileCreatePayload, ProfileSetupCommand, ProfileSoul, @@ -180,6 +182,8 @@ export type { ModelOptionProvider, ModelOptionsResponse, PaginatedSessions, + PairingResponse, + PairingUser, ProfileCreatePayload, ProfileInfo, ProfileSetupCommand, @@ -1162,6 +1166,38 @@ export function testMessagingPlatform(platformId: string): Promise { + return window.hermesDesktop.api({ + ...profileScoped(), + path: '/api/pairing' + }) +} + +export function approvePairing(platform: string, requestId: string): Promise<{ ok: boolean; user: PairingUser }> { + return window.hermesDesktop.api<{ ok: boolean; user: PairingUser }>({ + ...profileScoped(), + path: '/api/pairing/approve', + method: 'POST', + body: { platform, request_id: requestId } + }) +} + +export function revokePairing(platform: string, userId: string): Promise<{ ok: boolean }> { + return window.hermesDesktop.api<{ ok: boolean }>({ + ...profileScoped(), + path: '/api/pairing/revoke', + method: 'POST', + body: { platform, user_id: userId } + }) +} + // -- Webhooks (subscription CRUD) -------------------------------------------- // The webhook receiver is its own gateway platform; subscriptions live in a // shared JSON store the CLI/dashboard also drive. Enable mutates config and diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index d55c494e070..3444c30a4b8 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -1367,6 +1367,23 @@ export const en: Translations = { failedUpdate: name => `Failed to update ${name}`, failedSave: name => `Failed to save ${name}`, failedClear: key => `Failed to clear ${key}`, + pendingRequests: count => `Pending requests (${count})`, + pendingAria: count => `${count} pending pairing ${count === 1 ? 'request' : 'requests'}`, + approvedUsers: count => `Approved users (${count})`, + approve: 'Approve', + approving: 'Approving...', + revoke: 'Revoke', + revoking: 'Revoking...', + revokeAria: name => `Revoke ${name}`, + revokeTitle: 'Revoke access', + revokeDesc: (name: string) => `${name} will lose access and stop being recognized on their next message.`, + approvedUser: name => `${name} approved`, + approvedHint: 'They are recognized automatically on their next message.', + revokedUser: name => `${name} revoked`, + failedApprove: name => `Failed to approve ${name}`, + failedRevoke: name => `Failed to revoke ${name}`, + pairingLockedOut: 'Too many failed approvals — this platform is locked out. Try again later.', + waitingSince: minutes => (minutes < 1 ? 'just now' : `${minutes}m ago`), fieldCopy: { TELEGRAM_BOT_TOKEN: { label: 'Bot token', diff --git a/apps/desktop/src/i18n/types.ts b/apps/desktop/src/i18n/types.ts index 8ccc8a034ca..822d22b3794 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -1204,6 +1204,23 @@ export interface Translations { failedUpdate: (name: string) => string failedSave: (name: string) => string failedClear: (key: string) => string + pendingRequests: (count: number) => string + pendingAria: (count: number) => string + approvedUsers: (count: number) => string + approve: string + approving: string + revoke: string + revoking: string + revokeAria: (name: string) => string + revokeTitle: string + revokeDesc: (name: string) => string + approvedUser: (name: string) => string + approvedHint: string + revokedUser: (name: string) => string + failedApprove: (name: string) => string + failedRevoke: (name: string) => string + pairingLockedOut: string + waitingSince: (minutes: number) => string fieldCopy: Record platformIntro: Record } diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts index 86049412b4c..599abfbf4b7 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -1563,6 +1563,23 @@ export const zh: Translations = { failedUpdate: name => `更新 ${name} 失败`, failedSave: name => `保存 ${name} 失败`, failedClear: key => `清除 ${key} 失败`, + pendingRequests: count => `待处理请求（${count}）`, + pendingAria: count => `${count} 条待处理配对请求`, + approvedUsers: count => `已批准用户（${count}）`, + approve: '批准', + approving: '批准中…', + revoke: '撤销', + revoking: '撤销中…', + revokeAria: name => `撤销 ${name}`, + revokeTitle: '撤销访问权限', + revokeDesc: name => `${name} 将失去访问权限，下次发送消息时不再被识别。`, + approvedUser: name => `已批准 ${name}`, + approvedHint: '对方下次发送消息时会被自动识别。', + revokedUser: name => `已撤销 ${name}`, + failedApprove: name => `批准 ${name} 失败`, + failedRevoke: name => `撤销 ${name} 失败`, + pairingLockedOut: '批准失败次数过多，该平台已被暂时锁定，请稍后再试。', + waitingSince: minutes => (minutes < 1 ? '刚刚' : `${minutes} 分钟前`), fieldCopy: { TELEGRAM_BOT_TOKEN: { label: 'Bot 令牌', diff --git a/apps/desktop/src/types/hermes.ts b/apps/desktop/src/types/hermes.ts index bfc75cf6b3f..46536226153 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/apps/desktop/src/types/hermes.ts @@ -238,6 +238,21 @@ export interface MessagingPlatformsResponse { platforms: MessagingPlatformInfo[] } +/** A pending pairing request, or an already-approved user, for one platform. */ +export interface PairingUser { + age_minutes?: number + platform: string + /** Present on pending rows only — the id `approvePairing` grants on. */ + request_id?: string + user_id: string + user_name?: string +} + +export interface PairingResponse { + approved: PairingUser[] + pending: PairingUser[] +} + export interface MessagingPlatformUpdate { clear_env?: string[] enabled?: boolean From d063684b3fe2b177b859997329cdd2b6230b3069 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 29 Jul 2026 18:25:02 -0500 Subject: [PATCH 2/7] style(desktop): sort the confirm-dialog import (eslint perfectionist) --- apps/desktop/src/app/messaging/index.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/app/messaging/index.tsx b/apps/desktop/src/app/messaging/index.tsx index a6828db62c3..737eb8e4d25 100644 --- a/apps/desktop/src/app/messaging/index.tsx +++ b/apps/desktop/src/app/messaging/index.tsx @@ -5,8 +5,8 @@ import { useCallback, useEffect, useMemo, useState } from 'react' import { PageLoader } from '@/components/page-loader' import { StatusDot, type StatusTone } from '@/components/status-dot' import { Button } from '@/components/ui/button' -import { DisclosureCaret } from '@/components/ui/disclosure-caret' import { ConfirmDialog } from '@/components/ui/confirm-dialog' +import { DisclosureCaret } from '@/components/ui/disclosure-caret' import { ErrorBanner } from '@/components/ui/error-state' import { Input } from '@/components/ui/input' import { Switch } from '@/components/ui/switch' @@ -129,10 +129,12 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . // Both save/toggle toasts offer the same one-click restart. const restartGatewayAction = { label: t.commandCenter.restartGateway, onClick: () => void runGatewayRestart() } const [platforms, setPlatforms] = useState(null) + const [pairing, setPairing] = useState<{ approved: PairingUser[]; pending: PairingUser[] }>({ approved: [], pending: [] }) + const [approving, setApproving] = useState(null) const [pendingRevoke, setPendingRevoke] = useState(null) const [edits, setEdits] = useState({}) From 4cbd46545e285e47fcb9f67d5848d19842aff960 Mon Sep 17 00:00:00 2001 From: briandevans <252620095+briandevans@users.noreply.github.com> Date: Tue, 7 Jul 2026 16:44:43 -0700 Subject: [PATCH 3/7] fix(gateway): scope pairing platform discovery to the profile dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-profile pairing isolation added self._dir and scoped every per-file path helper (_pending_path, _approved_path) to it, but _all_platforms still enumerated the module-global PAIRING_DIR. For a profile-scoped PairingStore, list_approved/list_pending/clear_pending therefore operated on the GLOBAL platform set while loading each platform's file from the PROFILE dir — so list_approved() returned [] for a user that is_approved() confirmed as approved, a silent divergence between the authz surface and the list/inspect/clear surface. Route discovery through self._dir. Byte-identical for the global store (self._dir == PAIRING_DIR when no profile is set); only the buggy profile-scoped case changes. self._dir is guaranteed to exist (__init__ mkdirs it). --- gateway/pairing.py | 2 +- tests/gateway/test_pairing.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/gateway/pairing.py b/gateway/pairing.py index de8a3c8c929..8e568aad2e6 100644 --- a/gateway/pairing.py +++ b/gateway/pairing.py @@ -727,7 +727,7 @@ class PairingStore: def _all_platforms(self, suffix: str) -> list: """List all platforms that have data files of a given suffix.""" platforms = [] - for f in PAIRING_DIR.iterdir(): + for f in self._dir.iterdir(): if f.name.endswith(f"-{suffix}.json"): platform = f.name.replace(f"-{suffix}.json", "") if not platform.startswith("_"): diff --git a/tests/gateway/test_pairing.py b/tests/gateway/test_pairing.py index 8034cacc6a0..9f1538a49c8 100644 --- a/tests/gateway/test_pairing.py +++ b/tests/gateway/test_pairing.py @@ -46,6 +46,34 @@ class TestSplitPairingDirMigration: assert "ou_user" in migrated +class TestProfileScopedDiscovery: + def test_list_approved_scopes_platform_discovery_to_profile_dir(self, tmp_path): + # A profile-scoped store must enumerate platforms from its own + # per-profile directory (self._dir), not the module-global PAIRING_DIR. + # Regression: _all_platforms iterated PAIRING_DIR while every per-file + # path helper routed through self._dir, so a profile store confirmed a + # user via is_approved() (reads self._dir) yet returned [] from + # list_approved() (scanned the empty global dir). + home = tmp_path / "home" + global_dir = tmp_path / "global-pairing" + global_dir.mkdir(parents=True) + + with patch("gateway.pairing.PAIRING_DIR", global_dir), patch( + "gateway.pairing.get_hermes_home", return_value=home + ): + store = PairingStore(profile="alice") + # Store lives under the profile dir, provably distinct from PAIRING_DIR. + assert store._dir != global_dir + with store._lock: + store._approve_user("telegram", "tg-456", "Bob") + + assert store.is_approved("telegram", "tg-456") is True + approved = store.list_approved() + + assert [r["user_id"] for r in approved] == ["tg-456"] + assert approved[0]["platform"] == "telegram" + + # --------------------------------------------------------------------------- # _secure_write # --------------------------------------------------------------------------- From 75657f89c4c5afbbde4aa6657cc33b07976e7dfe Mon Sep 17 00:00:00 2001 From: briandevans <252620095+briandevans@users.noreply.github.com> Date: Wed, 8 Jul 2026 17:43:09 -0700 Subject: [PATCH 4/7] test(gateway): patch hermes_constants.get_hermes_home so profile store scopes to the mocked home --- tests/gateway/test_pairing.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/gateway/test_pairing.py b/tests/gateway/test_pairing.py index 9f1538a49c8..484283a8cac 100644 --- a/tests/gateway/test_pairing.py +++ b/tests/gateway/test_pairing.py @@ -58,11 +58,19 @@ 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. with patch("gateway.pairing.PAIRING_DIR", global_dir), patch( - "gateway.pairing.get_hermes_home", return_value=home + "hermes_constants.get_hermes_home", return_value=home ): store = PairingStore(profile="alice") - # Store lives under the profile dir, provably distinct from PAIRING_DIR. + # 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" assert store._dir != global_dir with store._lock: store._approve_user("telegram", "tg-456", "Bob") 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 5/7] 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, ), ) From f174c0b6bb1611a773bf9e244301a9519094a1af Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 29 Jul 2026 18:37:20 -0500 Subject: [PATCH 6/7] fix(pairing): scope the approve/revoke endpoints to a profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/desktop/src/hermes.ts | 6 ++- apps/desktop/src/pairing-scope.test.ts | 43 +++++++++++++++++++ hermes_cli/web_models.py | 2 + hermes_cli/web_server.py | 34 +++++++++++---- tests/gateway/test_pairing.py | 19 ++++---- .../test_dashboard_admin_endpoints.py | 43 +++++++++++++++++++ web/src/lib/api.ts | 23 ++++++++-- 7 files changed, 146 insertions(+), 24 deletions(-) create mode 100644 apps/desktop/src/pairing-scope.test.ts diff --git a/apps/desktop/src/hermes.ts b/apps/desktop/src/hermes.ts index 529fc935d5e..9b85e04c277 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 00000000000..170efa13b0b --- /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 ad24fca6b84..3ff438c6380 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 cf068da5574..7ee26c138f8 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 8dfaeafebbc..b377eb175e2 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 f155d58f28d..42dc5e997b1 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 2bd7a4b1797..3cc2c5a0ad6 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", { From 3c5da5307c8d70951de29b8c27d8f68b0733f765 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 29 Jul 2026 18:50:09 -0500 Subject: [PATCH 7/7] feat(pairing): give pairing its own change signal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The poll this page's pairing block rode was retired hours earlier by f8e07a332, which moved Messaging onto platforms.changed. That signal is the mtime of gateway_state.json — where the gateway persists connect/disconnect health — and a new pairing request moves none of it. So on an event-capable backend a pending row stayed invisible until something unrelated reconnected, and the count badge with it. Adds pairing.changed to the change watcher, signed off the pending/approved ledgers across the global store and every profile's own. _rate_limits.json is deliberately excluded: it moves on every unauthorized DM, including ones that produce no new row, so signalling on it would refetch for nothing. The page now refreshes platforms and pairing on their own signals rather than one combined call, and the legacy visible-tab poll (older backends) covers both. --- apps/desktop/src/app/messaging/index.test.tsx | 35 ++++++++++ apps/desktop/src/app/messaging/index.tsx | 69 ++++++++++++------- .../hooks/use-message-stream/gateway-event.ts | 6 +- apps/desktop/src/store/live-sync.ts | 5 ++ tests/tui_gateway/test_change_watcher.py | 48 +++++++++++++ tui_gateway/server.py | 41 +++++++++++ 6 files changed, 179 insertions(+), 25 deletions(-) diff --git a/apps/desktop/src/app/messaging/index.test.tsx b/apps/desktop/src/app/messaging/index.test.tsx index be451825de4..836c5fc9384 100644 --- a/apps/desktop/src/app/messaging/index.test.tsx +++ b/apps/desktop/src/app/messaging/index.test.tsx @@ -168,4 +168,39 @@ describe('MessagingView pairing', () => { expect((await screen.findAllByText('Microsoft Teams')).length).toBeGreaterThan(0) expect(screen.queryByRole('button', { name: 'Approve' })).toBeNull() }) + + it('refetches pending rows on pairing.changed, not on platforms.changed', async () => { + // The two signals are not interchangeable: platforms.changed tracks + // connect/disconnect health via gateway_state.json, which a new pairing + // request never moves. Riding it would leave someone invisible in the + // pending list until an unrelated reconnect happened to fire. + const { $changeEventsAvailable, $pairingChangeTick, $platformsChangeTick } = await import( + '@/store/live-sync' + ) + + getMessagingPlatforms.mockResolvedValue({ platforms: [platform()] }) + getPairing.mockResolvedValue({ approved: [], pending: [] }) + + await renderMessaging() + await act(async () => { + $changeEventsAvailable.set(true) + }) + getPairing.mockClear() + + // Someone DMs the bot: the store moves, the watcher ticks pairing.changed. + getPairing.mockResolvedValue({ approved: [], pending: [pendingUser] }) + await act(async () => { + $pairingChangeTick.set($pairingChangeTick.get() + 1) + }) + + await waitFor(() => expect(getPairing).toHaveBeenCalled()) + expect(await screen.findByRole('button', { name: 'Approve' })).toBeTruthy() + + // A platform health tick alone must not be what fetches pairing. + getPairing.mockClear() + await act(async () => { + $platformsChangeTick.set($platformsChangeTick.get() + 1) + }) + expect(getPairing).not.toHaveBeenCalled() + }) }) diff --git a/apps/desktop/src/app/messaging/index.tsx b/apps/desktop/src/app/messaging/index.tsx index 737eb8e4d25..a921e8f97d3 100644 --- a/apps/desktop/src/app/messaging/index.tsx +++ b/apps/desktop/src/app/messaging/index.tsx @@ -26,7 +26,7 @@ import { openExternalLink } from '@/lib/external-link' import { ExternalLink, Save, Trash2 } from '@/lib/icons' import { normalize } from '@/lib/text' import { cn } from '@/lib/utils' -import { $changeEventsAvailable, $platformsChangeTick } from '@/store/live-sync' +import { $changeEventsAvailable, $pairingChangeTick, $platformsChangeTick } from '@/store/live-sync' import { notify, notifyError } from '@/store/notifications' import { runGatewayRestart } from '@/store/system-actions' @@ -151,22 +151,11 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . } try { - // Pairing rides the same refresh as platform status: both feed this - // page and a lone failure shouldn't blank the other. A backend older - // than the request-id endpoint just yields no rows. - const [result, pairingResult] = await Promise.allSettled([getMessagingPlatforms(), getPairing()]) - - if (result.status === 'fulfilled') { - setPlatforms(result.value.platforms) - } else if (!silent) { - notifyError(result.reason, m.loadFailed) - } - - if (pairingResult.status === 'fulfilled') { - setPairing({ - approved: pairingResult.value.approved ?? [], - pending: pairingResult.value.pending ?? [] - }) + const result = await getMessagingPlatforms() + setPlatforms(result.platforms) + } catch (err) { + if (!silent) { + notifyError(err, m.loadFailed) } } finally { if (!silent) { @@ -177,14 +166,46 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . [m] ) - useRefreshHotkey(() => void refreshPlatforms()) + // Pairing has its own signal. platforms.changed tracks connect/disconnect + // health via gateway_state.json, which a new pairing request never moves — + // riding it would leave a pending row invisible until something unrelated + // reconnected. Failures stay silent: an older backend without the endpoint + // should show no rows, not an error banner over a working page. + const refreshPairing = useCallback(async () => { + try { + const result = await getPairing() + setPairing({ approved: result.approved ?? [], pending: result.pending ?? [] }) + } catch { + // Leave the last known rows in place rather than blanking them. + } + }, []) + + const refreshAll = useCallback( + async (silent = false) => { + await Promise.all([refreshPlatforms(silent), refreshPairing()]) + }, + [refreshPairing, refreshPlatforms] + ) + + useRefreshHotkey(() => void refreshAll()) useEffect(() => { - void refreshPlatforms() - }, [refreshPlatforms]) + void refreshAll() + }, [refreshAll]) const changeEventsAvailable = useStore($changeEventsAvailable) const platformsChangeTick = useStore($platformsChangeTick) + const pairingChangeTick = useStore($pairingChangeTick) + + // A new pending request (or a grant from another surface) moves the pairing + // store on disk; the change watcher turns that into pairing.changed. + useEffect(() => { + if (!changeEventsAvailable || pairingChangeTick === 0 || document.hidden) { + return + } + + void refreshPairing() + }, [changeEventsAvailable, pairingChangeTick, refreshPairing]) // Connection status updates without a manual "check" click. platforms.changed // (the gateway persisting connect/disconnect/health to gateway_state.json) @@ -210,7 +231,7 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . return } - void refreshPlatforms(true) + void refreshAll(true) } const id = window.setInterval(tick, 6000) @@ -219,7 +240,7 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . cancelled = true window.clearInterval(id) } - }, [changeEventsAvailable, refreshPlatforms]) + }, [changeEventsAvailable, refreshAll]) const selected = useMemo(() => { if (!platforms) { @@ -346,7 +367,7 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . try { await approvePairing(user.platform, user.request_id) notify({ kind: 'success', title: m.approvedUser(pairingLabel(user)), message: m.approvedHint }) - await refreshPlatforms(true) + await refreshPairing() } catch (err) { setPairing(snapshot) // 429 is the code path's brute-force lockout — a distinct condition the @@ -371,7 +392,7 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . try { await revokePairing(user.platform, user.user_id) notify({ kind: 'success', title: m.revokedUser(pairingLabel(user)), message: user.platform }) - await refreshPlatforms(true) + await refreshPairing() } catch (err) { setPairing(snapshot) throw err diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts index f8b232a8831..9ee6a2feb9b 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts @@ -26,6 +26,7 @@ import { $gateway } from '@/store/gateway' import { applyGoalStatusText } from '@/store/goals' import { notifyCronChanged, + notifyPairingChanged, notifyPetChanged, notifyPlatformsChanged, notifySessionsChanged, @@ -303,7 +304,8 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) { event.type === 'pet.changed' || event.type === 'cron.changed' || event.type === 'sessions.changed' || - event.type === 'platforms.changed' + event.type === 'platforms.changed' || + event.type === 'pairing.changed' ) { // Change-watcher broadcasts (server._broadcast_watched_changes): the // backend's on-disk signature moved. Route to the live-sync ticks the @@ -319,6 +321,8 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) { notifyCronChanged() } else if (event.type === 'platforms.changed') { notifyPlatformsChanged() + } else if (event.type === 'pairing.changed') { + notifyPairingChanged() } else { notifySessionsChanged() } diff --git a/apps/desktop/src/store/live-sync.ts b/apps/desktop/src/store/live-sync.ts index cab1f5a8a99..61b2a55cf9f 100644 --- a/apps/desktop/src/store/live-sync.ts +++ b/apps/desktop/src/store/live-sync.ts @@ -19,6 +19,7 @@ export const $changeEventsAvailable = atom(false) export const $cronChangeTick = atom(0) export const $sessionsChangeTick = atom(0) export const $platformsChangeTick = atom(0) +export const $pairingChangeTick = atom(0) /** `pet.info.meta`-shaped payload carried on `pet.changed` — lets the pet skip * the heavy sprite refetch when the broadcast already says enabled=false. */ @@ -52,6 +53,10 @@ export function notifyPlatformsChanged(): void { $platformsChangeTick.set($platformsChangeTick.get() + 1) } +export function notifyPairingChanged(): void { + $pairingChangeTick.set($pairingChangeTick.get() + 1) +} + /** Reset on gateway wipe/reconnect — a new backend re-advertises capability on * its own gateway.ready, and stale ticks must not fire refreshes into stores * the wipe just cleared. */ diff --git a/tests/tui_gateway/test_change_watcher.py b/tests/tui_gateway/test_change_watcher.py index b4b6558418e..c0785283496 100644 --- a/tests/tui_gateway/test_change_watcher.py +++ b/tests/tui_gateway/test_change_watcher.py @@ -72,6 +72,54 @@ def test_gateway_state_move_broadcasts_platforms_changed(watcher_home): assert ("platforms.changed", {}) in events +def test_pending_pairing_request_broadcasts_pairing_changed(watcher_home): + """A new pending request must reach the Messaging page on its own signal. + + The messaging gateway writes the pending code from a different process, and + it moves nothing in gateway_state.json — so platforms.changed cannot stand + in for this. Without a dedicated signal the badge stays invisible until an + unrelated connect/disconnect happens to fire. + """ + home, events = watcher_home + store = home / "platforms" / "pairing" + store.mkdir(parents=True) + server._broadcast_watched_changes(now=0.0) + + (store / "telegram-pending.json").write_text('{"abc": {"user_id": "1"}}') + server._broadcast_watched_changes(now=10.0) + + assert ("pairing.changed", {}) in events + assert ("platforms.changed", {}) not in events + + +def test_pairing_signal_follows_a_profile_store(watcher_home): + """Each profile keeps its own whitelist, and the page can be scoped to any.""" + home, events = watcher_home + store = home / "profiles" / "work" / "platforms" / "pairing" + store.mkdir(parents=True) + server._broadcast_watched_changes(now=0.0) + + (store / "telegram-approved.json").write_text('{"u1": {"user_id": "u1"}}') + server._broadcast_watched_changes(now=10.0) + + assert ("pairing.changed", {}) in events + + +def test_rate_limit_churn_does_not_broadcast_pairing_changed(watcher_home): + """_rate_limits.json moves on every unauthorized DM, including ones that + produce no new row — signalling on it would refetch for nothing.""" + home, events = watcher_home + store = home / "platforms" / "pairing" + store.mkdir(parents=True) + (store / "telegram-pending.json").write_text("{}") + server._broadcast_watched_changes(now=0.0) + + (store / "_rate_limits.json").write_text('{"telegram:1": 123}') + server._broadcast_watched_changes(now=10.0) + + assert ("pairing.changed", {}) not in events + + def test_sessions_floor_coalesces_burst_but_keeps_trailing_edge(watcher_home): home, events = watcher_home server._broadcast_watched_changes(now=0.0) diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 5cc0d87742f..b3a0ffa6187 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -3082,6 +3082,46 @@ def _platforms_sig(): return None +def _pairing_sig(): + """Newest mtime across every profile's pairing store. + + An unknown DMer's pending code is written by the messaging gateway — a + DIFFERENT process that never touches this gateway's transports — so the + files are the only shared signal. ``platforms.changed`` cannot stand in + for this: it tracks connect/disconnect/health, and a pairing request + moves nothing in gateway_state.json. + """ + home = _watcher_home() + sig = None + # Global store (legacy `pairing/` and consolidated `platforms/pairing/`) + # plus every named profile's own — the Messaging page can be scoped to any + # of them, and a request landing in a profile store must still tick. + roots = [home / "pairing", home / "platforms" / "pairing"] + try: + for profile_dir in (home / "profiles").iterdir(): + roots.append(profile_dir / "pairing") + roots.append(profile_dir / "platforms" / "pairing") + except OSError: + pass + + for root in roots: + try: + entries = list(root.iterdir()) + except OSError: + continue + for entry in entries: + # Only the pending/approved ledgers — _rate_limits.json moves on + # every unauthorized DM, including ones that produce no new row. + if not entry.name.endswith(("-pending.json", "-approved.json")): + continue + try: + mtime = entry.stat().st_mtime_ns + except OSError: + continue + sig = mtime if sig is None else max(sig, mtime) + return sig + + # Watched change signals: event → (check interval, signature fn, payload fn). # Signatures are stat/dict-lookup cheap, same bar as the skin watcher; the # check interval keeps the pricier probes (pet resolves the active sheet off @@ -3091,6 +3131,7 @@ _CHANGE_WATCHES: dict[str, tuple[float, Any, Any]] = { "cron.changed": (1.0, _cron_sig, lambda: {}), "sessions.changed": (0.5, _sessions_sig, lambda: {}), "platforms.changed": (2.0, _platforms_sig, lambda: {}), + "pairing.changed": (2.0, _pairing_sig, lambda: {}), } # state.db moves on every message append during a streaming turn, and the