diff --git a/apps/desktop/src/app/messaging/index.test.tsx b/apps/desktop/src/app/messaging/index.test.tsx index b078a2043b1..836c5fc9384 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,107 @@ 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() + }) + + 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 08ae2a44404..a921e8f97d3 100644 --- a/apps/desktop/src/app/messaging/index.tsx +++ b/apps/desktop/src/app/messaging/index.tsx @@ -5,15 +5,20 @@ 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 { 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' 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' @@ -21,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' @@ -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,14 @@ 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) @@ -137,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) @@ -170,7 +231,7 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . return } - void refreshPlatforms(true) + void refreshAll(true) } const id = window.setInterval(tick, 6000) @@ -179,7 +240,7 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . cancelled = true window.clearInterval(id) } - }, [changeEventsAvailable, refreshPlatforms]) + }, [changeEventsAvailable, refreshAll]) const selected = useMemo(() => { if (!platforms) { @@ -189,6 +250,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 +348,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 refreshPairing() + } 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 refreshPairing() + } catch (err) { + setPairing(snapshot) + throw err + } + } + return ( setSelectedId(platform.id)} + pendingCount={pendingByPlatform[platform.id]?.length ?? 0} platform={platform} /> @@ -326,7 +442,10 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . > {selected && ( void handleApprove(user)} onClear={key => void handleClear(selected, key)} onEdit={(key, value) => setEdits(current => ({ @@ -337,6 +456,8 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . } })) } + onRevoke={setPendingRevoke} + pending={pendingByPlatform[selected.id] ?? []} platform={selected} saving={saving} /> @@ -344,6 +465,18 @@ export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, . )} + + setPendingRevoke(null)} + onConfirm={() => (pendingRevoke ? handleRevoke(pendingRevoke) : undefined)} + open={Boolean(pendingRevoke)} + title={m.revokeTitle} + /> ) } @@ -351,12 +484,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 +580,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/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/hermes.ts b/apps/desktop/src/hermes.ts index f94f0d2d741..9b85e04c277 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,40 @@ 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', + // 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() } + }) +} + +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, ...profileScoped() } + }) +} + // -- 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 8ada8b48b31..6426b61ac5d 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 3ec175fd002..7bd9c8cb1fc 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 1797214d098..171da340086 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/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/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/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 diff --git a/gateway/pairing.py b/gateway/pairing.py index de8a3c8c929..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. @@ -727,7 +748,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/gateway/run.py b/gateway/run.py index c69dac1eb16..97c5d4a6030 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -12486,11 +12486,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) @@ -13677,23 +13681,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) @@ -13704,7 +13724,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_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/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 8034cacc6a0..b377eb175e2 100644 --- a/tests/gateway/test_pairing.py +++ b/tests/gateway/test_pairing.py @@ -46,6 +46,41 @@ 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) + + # 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( + "gateway.pairing.get_default_hermes_root", return_value=home + ): + store = PairingStore(profile="alice") + # 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") + + 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 # --------------------------------------------------------------------------- @@ -521,16 +556,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") @@ -549,15 +662,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, ), ) 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/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 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", {