diff --git a/app/(dashboard)/harnesses/[id]/page.tsx b/app/(dashboard)/harnesses/[id]/page.tsx index 1212e32..2338b96 100644 --- a/app/(dashboard)/harnesses/[id]/page.tsx +++ b/app/(dashboard)/harnesses/[id]/page.tsx @@ -1044,6 +1044,9 @@ function HermesHarnessDetail({ params }: { params: Promise<{ id: string }> }) { />

Admins can DM, add bot to groups, approve commands, and access global memory. + {platform === 'telegram' && ( + <> Entries may be numeric Telegram user IDs or @usernames — @usernames are resolved to IDs automatically. + )}

diff --git a/app/api/harnesses/[id]/settings/route.test.ts b/app/api/harnesses/[id]/settings/route.test.ts index 04e3260..fd4206a 100644 --- a/app/api/harnesses/[id]/settings/route.test.ts +++ b/app/api/harnesses/[id]/settings/route.test.ts @@ -14,18 +14,20 @@ vi.mock('@/lib/services', () => ({ services: { harness: { restart: vi.fn(), get: vi.fn(() => undefined) }, config: { getSettings: vi.fn(() => ({})) }, + surfaceAdmins: { syncFromAllowlist: vi.fn() }, }, })) vi.mock('@/lib/resolvers', () => ({ resolveIdentifier: vi.fn(async () => null), expandSignalAllowlist: vi.fn(async (_id: string, users: string[]) => users), + expandTelegramAllowlist: vi.fn(async (_id: string, users: string[]) => users), })) vi.mock('@/lib/services/harness-compose', () => ({ generateStandaloneCompose: vi.fn(() => '') })) vi.mock('@/lib/env-helpers', () => ({ buildSettingsEnvValue: vi.fn(() => '') })) import { GET, PUT } from './route' import { services } from '@/lib/services' -import { expandSignalAllowlist } from '@/lib/resolvers' +import { expandSignalAllowlist, expandTelegramAllowlist } from '@/lib/resolvers' function makeParams(id: string) { return { params: Promise.resolve({ id }) } @@ -112,6 +114,7 @@ describe('Settings API — PUT', () => { const env = writtenEnv() expect(env).toContain('SLACK_CHANNEL_POLICY=approved-only') expect(env).toContain('SIGNAL_GROUP_INVITE_POLICY=approved-only') + expect(env).toContain('TELEGRAM_GROUP_INVITE_POLICY=approved-only') }) it('writes SLACK_CHANNEL_POLICY=allow-all when group invite policy is allow-all', async () => { @@ -127,6 +130,7 @@ describe('Settings API — PUT', () => { const env = writtenEnv() expect(env).toContain('SLACK_CHANNEL_POLICY=allow-all') expect(env).toContain('SIGNAL_GROUP_INVITE_POLICY=allow-all') + expect(env).toContain('TELEGRAM_GROUP_INVITE_POLICY=allow-all') }) it('expands Signal allowed users to include resolved UUIDs before writing', async () => { @@ -150,6 +154,58 @@ describe('Settings API — PUT', () => { expect(expandSignalAllowlist).toHaveBeenCalledWith('h_test', ['+15550001234']) }) + + it('expands Telegram @usernames and syncs the policy-plane admin overlay', async () => { + // Two stores hold Telegram admins: the .env allowlist (bootstrap) and the + // SurfaceAdminService overlay (served live to the policy plugin). A settings + // write must keep them converged — and expand @handles to numeric IDs, since + // the gateway matches numeric sender IDs verbatim. + ;(expandTelegramAllowlist as ReturnType).mockResolvedValueOnce(['@juniper', '424242']) + const body = { + dmPolicy: 'approved-only', + groupInvitePolicy: 'approved-only', + mentionGating: true, + commandApprovalAdminOnly: true, + memoryScope: 'channel', + surfaces: { + telegram: { + allowedUsers: ['@juniper'], + adminUsers: ['@juniper'], + allowedGroups: [], + allowAll: false, + allowAllGroups: false, + }, + }, + } + await PUT(makeRequest(body), makeParams('h_test')) + + expect(expandTelegramAllowlist).toHaveBeenCalledWith('h_test', ['@juniper']) + expect(services.surfaceAdmins.syncFromAllowlist).toHaveBeenCalledWith( + 'h_test', 'telegram', ['@juniper', '424242'], + ) + }) + + it('does NOT sync the overlay on an allow-all/empty telegram allowlist (would wipe an explicit admin roster)', async () => { + const body = { + dmPolicy: 'allow-all', + groupInvitePolicy: 'approved-only', + mentionGating: true, + commandApprovalAdminOnly: true, + memoryScope: 'channel', + surfaces: { + telegram: { + allowedUsers: [], + adminUsers: [], + allowedGroups: [], + allowAll: true, + allowAllGroups: false, + }, + }, + } + await PUT(makeRequest(body), makeParams('h_test')) + + expect(services.surfaceAdmins.syncFromAllowlist).not.toHaveBeenCalled() + }) }) describe('Settings API — GET mention-gating reflects the runtime', () => { diff --git a/app/api/harnesses/[id]/settings/route.ts b/app/api/harnesses/[id]/settings/route.ts index ca91a42..5837b14 100644 --- a/app/api/harnesses/[id]/settings/route.ts +++ b/app/api/harnesses/[id]/settings/route.ts @@ -3,7 +3,7 @@ import fs from 'fs' import path from 'path' import os from 'os' import { buildSettingsEnvValue } from '@/lib/env-helpers' -import { resolveIdentifier, expandSignalAllowlist } from '@/lib/resolvers' +import { resolveIdentifier, expandSignalAllowlist, expandTelegramAllowlist } from '@/lib/resolvers' import { services } from '@/lib/services' import { adapterForRuntime } from '@/lib/services/harness' @@ -57,9 +57,12 @@ type SurfaceSettings = { // each connected surface honors the same approved-only/allow-all choice. Slack's // runtime reads SLACK_CHANNEL_POLICY (approved-only = empty SLACK_ALLOWED_CHANNELS // means NO channels approved; allow-all = empty means respond everywhere). +// Telegram's var is enforced by the group-approval POST endpoint +// (surfaces/[platform]/groups/[groupId]) that the swarm_map_policy plugin calls. const GROUP_INVITE_VARS: Record = { signal: 'SIGNAL_GROUP_INVITE_POLICY', slack: 'SLACK_CHANNEL_POLICY', + telegram: 'TELEGRAM_GROUP_INVITE_POLICY', } // Env var names for mention-gating per platform @@ -235,6 +238,10 @@ export async function PUT( let content = fs.readFileSync(envPath, 'utf-8') + // Telegram allowlist as written to the env — captured so the policy-plane + // admin overlay (SurfaceAdminService) can be synced after the env write. + let telegramAllowlist: string[] | undefined + for (const [platform, vars] of Object.entries(PLATFORM_VARS)) { const settings = body.surfaces[platform] if (!settings) continue @@ -247,10 +254,25 @@ export async function PUT( if (platform === 'signal' && allowedUsers.length > 0) { allowedUsers = await expandSignalAllowlist(id, allowedUsers) } + // For Telegram, expand @usernames to also include their resolved numeric + // ID — the gateway matches numeric sender IDs verbatim, so an + // @username-only entry never matches anyone (see expandTelegramAllowlist). + if (platform === 'telegram') { + if (allowedUsers.length > 0) { + allowedUsers = await expandTelegramAllowlist(id, allowedUsers) + } + // Only converge the admin overlay against a concrete allowlist. An + // allow-all/empty policy writes '*' or '' here — syncing that would + // silently wipe an explicitly-configured admin roster because of an + // unrelated DM-policy toggle. + if (allowedUsers.length > 0) { + telegramAllowlist = allowedUsers + } + } const usersValue = buildSettingsEnvValue(body.dmPolicy, settings.allowAll, allowedUsers) const usersRegex = new RegExp(`^${vars.users}=.*$`, 'm') if (usersRegex.test(content)) { - content = content.replace(usersRegex, `${vars.users}=${usersValue}`) + content = content.replace(usersRegex, () => `${vars.users}=${usersValue}`) } else { content = content.trimEnd() + `\n${vars.users}=${usersValue}\n` } @@ -363,6 +385,15 @@ export async function PUT( fs.writeFileSync(envPath, content, { mode: 0o600 }) + // Keep the policy-plane admin overlay converged with the Telegram allowlist + // just written — the two admin stores must never diverge. Non-numeric entries + // (raw @handles) are dropped by the sync's validation, never stored. + if (telegramAllowlist !== undefined) { + try { + services.surfaceAdmins.syncFromAllowlist(id, 'telegram', telegramAllowlist) + } catch {} + } + // Per-harness resource limits (compose deploy.resources.limits — NOT env vars). // Persist on the harness overlay and detect a change so the compose is only // regenerated when needed. diff --git a/app/api/harnesses/[id]/surfaces/[platform]/groups/[groupId]/route.test.ts b/app/api/harnesses/[id]/surfaces/[platform]/groups/[groupId]/route.test.ts index 1a58250..4a781b9 100644 --- a/app/api/harnesses/[id]/surfaces/[platform]/groups/[groupId]/route.test.ts +++ b/app/api/harnesses/[id]/surfaces/[platform]/groups/[groupId]/route.test.ts @@ -1,16 +1,22 @@ /** - * Tests for the is-group-allowed route the swarm_map_policy plugin calls. - * GET /api/harnesses/:id/surfaces/:platform/groups/:groupId → { allowed } + * Tests for the group routes the swarm_map_policy plugin calls. + * GET /api/harnesses/:id/surfaces/:platform/groups/:groupId → { allowed } + * POST /api/harnesses/:id/surfaces/:platform/groups/:groupId → group-invite approval */ import { describe, it, expect, beforeEach, vi } from 'vitest' -import { GET } from './route' +import { GET, POST } from './route' vi.mock('@/lib/services', () => ({ - services: { surfaceAdmins: { isGroupAllowed: vi.fn() } }, + services: { + surfaceAdmins: { isGroupAllowed: vi.fn(), approveGroupInvite: vi.fn() }, + harness: { restart: vi.fn() }, + }, })) import { services } from '@/lib/services' const isGroupAllowed = services.surfaceAdmins.isGroupAllowed as ReturnType +const approveGroupInvite = services.surfaceAdmins.approveGroupInvite as ReturnType +const restart = services.harness.restart as ReturnType function makeParams(id: string, platform: string, groupId: string) { return { params: Promise.resolve({ id, platform, groupId }) } @@ -34,3 +40,69 @@ describe('GET is-group-allowed', () => { expect(await res.json()).toEqual({ allowed: false }) }) }) + +describe('POST group-invite approval', () => { + function makePost(addedByUserId?: unknown, rawBody?: string) { + return new Request('http://x', { + method: 'POST', + body: rawBody ?? JSON.stringify({ addedByUserId }), + }) + } + + it('approves, recreates the container, and reports restarted when the allowlist was updated', async () => { + approveGroupInvite.mockReturnValue({ ok: true, approved: true, updated: true }) + const res = await POST(makePost('111'), makeParams('h_seraph', 'telegram', '-100777')) + expect(res.status).toBe(200) + expect(await res.json()).toEqual({ approved: true, restarted: true }) + expect(approveGroupInvite).toHaveBeenCalledWith('h_seraph', 'telegram', '-100777', '111') + // env_file is only read at container creation — a recreate must follow the write. + expect(restart).toHaveBeenCalledWith('h_seraph', 'recreate') + }) + + it('reports restarted:false (still approved) when the recreate fails', async () => { + approveGroupInvite.mockReturnValue({ ok: true, approved: true, updated: true }) + restart.mockImplementationOnce(() => { throw new Error('no compose file') }) + const res = await POST(makePost('111'), makeParams('h_seraph', 'telegram', '-100777')) + expect(await res.json()).toEqual({ approved: true, restarted: false }) + }) + + it('approves without a restart when the group is already allowed', async () => { + approveGroupInvite.mockReturnValue({ ok: true, approved: true, updated: false }) + const res = await POST(makePost('111'), makeParams('h_seraph', 'telegram', '-100777')) + expect(await res.json()).toEqual({ approved: true, already_allowed: true }) + expect(restart).not.toHaveBeenCalled() + }) + + it('returns { approved: false, reason } (200) when the adder is not an admin', async () => { + approveGroupInvite.mockReturnValue({ ok: true, approved: false, reason: 'not an admin' }) + const res = await POST(makePost('222'), makeParams('h_seraph', 'telegram', '-100777')) + expect(res.status).toBe(200) + expect(await res.json()).toEqual({ approved: false, reason: 'not an admin' }) + expect(restart).not.toHaveBeenCalled() + }) + + it('maps service validation failures to 400', async () => { + approveGroupInvite.mockReturnValue({ ok: false, status: 400, error: 'Invalid group id' }) + const res = await POST(makePost('111'), makeParams('h_seraph', 'telegram', 'bad,id')) + expect(res.status).toBe(400) + }) + + it('rejects a malformed JSON body with 400', async () => { + const res = await POST(makePost(undefined, 'not-json'), makeParams('h_seraph', 'telegram', '-100777')) + expect(res.status).toBe(400) + expect(approveGroupInvite).not.toHaveBeenCalled() + }) + + it('passes a non-string addedByUserId through as empty (service 400s it)', async () => { + approveGroupInvite.mockReturnValue({ ok: false, status: 400, error: 'addedByUserId is required' }) + const res = await POST(makePost(42), makeParams('h_seraph', 'telegram', '-100777')) + expect(res.status).toBe(400) + expect(approveGroupInvite).toHaveBeenCalledWith('h_seraph', 'telegram', '-100777', '') + }) + + it('URL-decodes the groupId (signal ids are base64 with url-encoded chars)', async () => { + approveGroupInvite.mockReturnValue({ ok: true, approved: true, updated: false }) + await POST(makePost('111'), makeParams('h_seraph', 'signal', 'abc%2Bdef%3D%3D')) + expect(approveGroupInvite).toHaveBeenCalledWith('h_seraph', 'signal', 'abc+def==', '111') + }) +}) diff --git a/app/api/harnesses/[id]/surfaces/[platform]/groups/[groupId]/route.ts b/app/api/harnesses/[id]/surfaces/[platform]/groups/[groupId]/route.ts index a1b171d..13865d6 100644 --- a/app/api/harnesses/[id]/surfaces/[platform]/groups/[groupId]/route.ts +++ b/app/api/harnesses/[id]/surfaces/[platform]/groups/[groupId]/route.ts @@ -17,3 +17,75 @@ export async function GET( const allowed = services.surfaceAdmins.isGroupAllowed(id, platform, decodeURIComponent(groupId)) return NextResponse.json({ allowed }) } + +/** + * POST /api/harnesses/:id/surfaces/:platform/groups/:groupId + * + * Group-invite approval for the swarm_map_policy agent plugin: the agent was + * just added to a group and asks whether the invite is approved. Consumed by a + * hermes-agent-mt change — this comment is the contract. + * + * Request body (JSON): + * { "addedByUserId": "" } + * + * Responses (always 200 unless the request itself is malformed): + * { "approved": true, "restarted": boolean } — group appended to the + * allowlist env var (e.g. TELEGRAM_GROUP_ALLOWED_CHATS); the container was + * recreated so the change takes effect (restarted:false = env written but + * recreate failed, e.g. no compose file yet). + * { "approved": true, "already_allowed": true } — allowlist is '*' or the + * group is already listed; nothing written, no restart. + * { "approved": false, "reason": "" } — invite policy is + * approved-only (or unset, the secure default) and addedByUserId is not an + * admin. The plugin should leave the group. + * 400 { "error": "" } — unsupported platform, + * structurally invalid groupId, missing addedByUserId, or no agent .env. + * + * Policy: allow-all → approve for anyone; approved-only/unset → approve only + * when addedByUserId is an admin (SurfaceAdminService.isAdmin, fail-closed). + * + * Auth: exempted from the operator-cookie gate in middleware.ts (agents cannot + * obtain the cookie) — see AGENT_CALLABLE_POST_PATHS there for the trust model. + */ +export async function POST( + request: Request, + { params }: { params: Promise<{ id: string; platform: string; groupId: string }> }, +) { + const { id, platform, groupId } = await params + + let body: { addedByUserId?: unknown } + try { + body = (await request.json()) as { addedByUserId?: unknown } + } catch { + return NextResponse.json({ error: 'Invalid JSON body' }, { status: 400 }) + } + const addedByUserId = typeof body.addedByUserId === 'string' ? body.addedByUserId : '' + + const result = services.surfaceAdmins.approveGroupInvite( + id, + platform, + decodeURIComponent(groupId), + addedByUserId, + ) + + if (!result.ok) { + return NextResponse.json({ error: result.error }, { status: result.status }) + } + if (!result.approved) { + return NextResponse.json({ approved: false, reason: result.reason }) + } + if (!result.updated) { + return NextResponse.json({ approved: true, already_allowed: true }) + } + + // Recreate the container so the updated allowlist actually loads (env_file is + // read at container creation, not on a plain restart) — same as every other + // env-writing route. Best-effort: the env is written either way. + let restarted = false + try { + services.harness.restart(id, 'recreate') + restarted = true + } catch {} + + return NextResponse.json({ approved: true, restarted }) +} diff --git a/app/api/harnesses/[id]/surfaces/connect/route.test.ts b/app/api/harnesses/[id]/surfaces/connect/route.test.ts index 6d057bb..4743b68 100644 --- a/app/api/harnesses/[id]/surfaces/connect/route.test.ts +++ b/app/api/harnesses/[id]/surfaces/connect/route.test.ts @@ -12,17 +12,27 @@ import os from 'os' // Mock the services module — we assert harness.restart is invoked. const restartMock = vi.hoisted(() => vi.fn()) +const syncFromAllowlistMock = vi.hoisted(() => vi.fn()) vi.mock('@/lib/services', () => ({ services: { harness: { restart: restartMock }, + surfaceAdmins: { syncFromAllowlist: syncFromAllowlistMock }, }, })) -vi.mock('@/lib/resolvers', () => ({ +// Use the real resolvers index (so resolveTelegramAdmins' strict logic is +// exercised through the route) but stub the per-platform network resolvers. +vi.mock('@/lib/resolvers', async (importOriginal) => ({ + ...(await importOriginal()), expandSignalAllowlist: vi.fn(async () => [ '+15550001234', 'aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee', ]), })) +const resolveTelegramUsernameMock = vi.hoisted(() => vi.fn()) +vi.mock('@/lib/resolvers/telegram', () => ({ + resolveTelegramUsername: resolveTelegramUsernameMock, + getTelegramDisplayName: vi.fn(async () => null), +})) import { POST } from './route' @@ -34,6 +44,11 @@ let readSpy: ReturnType let writeSpy: ReturnType beforeEach(() => { + // vi.restoreAllMocks() only restores spies — hoisted vi.fn() mocks keep their + // call history across tests unless cleared here. + restartMock.mockClear() + syncFromAllowlistMock.mockClear() + resolveTelegramUsernameMock.mockClear() vi.spyOn(os, 'homedir').mockReturnValue('/home/test') vi.spyOn(fs, 'existsSync').mockReturnValue(true) readSpy = vi.spyOn(fs, 'readFileSync') @@ -86,6 +101,61 @@ describe('surface connect — applies env by recreating the container', () => { vi.unstubAllGlobals() }) + it('resolves telegram @handles to numeric IDs and normalizes multiple admins', async () => { + resolveTelegramUsernameMock.mockResolvedValue({ + display: '@juniper', + nativeId: '424242', + profileName: 'Juniper', + }) + + const req = new Request('http://localhost/api/harnesses/h_seraph/surfaces/connect', { + method: 'POST', + body: JSON.stringify({ + platform: 'telegram', + config: { token: 'new-bot-token', adminUser: '123456789, @juniper' }, + }), + }) + + const res = await POST(req, makeParams('h_seraph')) + expect(res.status).toBe(200) + + // The token from the request payload is used (the .env may not have it yet). + expect(resolveTelegramUsernameMock).toHaveBeenCalledWith('h_seraph', '@juniper', 'new-bot-token') + + // Numeric IDs only in the env — never a raw @handle. + const envCall = writeSpy.mock.calls.find(c => String(c[0]).endsWith('.env')) + expect(envCall?.[1]).toContain('TELEGRAM_ALLOWED_USERS=123456789,424242') + + // Display names persisted the same way the settings path does. + const resolvedCall = writeSpy.mock.calls.find(c => String(c[0]).endsWith('resolved-identities.json')) + expect(resolvedCall).toBeTruthy() + expect(JSON.parse(resolvedCall![1] as string).telegram).toEqual([ + { display: '123456789', nativeId: '123456789' }, + { display: '@juniper', nativeId: '424242', profileName: 'Juniper' }, + ]) + + // Policy-plane overlay kept converged with the allowlist just written. + expect(syncFromAllowlistMock).toHaveBeenCalledWith('h_seraph', 'telegram', ['123456789', '424242']) + }) + + it('returns 400 (and writes nothing) when a telegram @handle fails to resolve', async () => { + resolveTelegramUsernameMock.mockResolvedValue(null) + + const req = new Request('http://localhost/api/harnesses/h_seraph/surfaces/connect', { + method: 'POST', + body: JSON.stringify({ + platform: 'telegram', + config: { token: 'new-bot-token', adminUser: '@ghosthandle' }, + }), + }) + + const res = await POST(req, makeParams('h_seraph')) + expect(res.status).toBe(400) + expect((await res.json()).error).toContain('@ghosthandle') + expect(writeSpy).not.toHaveBeenCalled() + expect(restartMock).not.toHaveBeenCalled() + }) + it('still succeeds (does not 500) when recreate fails', async () => { restartMock.mockImplementationOnce(() => { throw new Error('no compose file') }) diff --git a/app/api/harnesses/[id]/surfaces/connect/route.ts b/app/api/harnesses/[id]/surfaces/connect/route.ts index 186c57e..363f2da 100644 --- a/app/api/harnesses/[id]/surfaces/connect/route.ts +++ b/app/api/harnesses/[id]/surfaces/connect/route.ts @@ -4,7 +4,7 @@ import path from 'path' import os from 'os' import { buildConnectEnvVars, mergeEnvVars, ensurePolicyDefaults, getSignalDaemonUrl } from '@/lib/env-helpers' import { services } from '@/lib/services' -import { expandSignalAllowlist } from '@/lib/resolvers' +import { expandSignalAllowlist, resolveTelegramAdmins, type ResolvedIdentity } from '@/lib/resolvers' function agentDataDir(harnessId: string): string { const name = harnessId.replace(/^h_/, '').replace(/_/g, '-') @@ -65,6 +65,8 @@ export async function POST( } // If adminUser is provided, set the platform's ALLOWED_USERS var + // (comma-separated for multiple admins) + let telegramAdmins: { ids: string[]; resolved: ResolvedIdentity[] } | undefined if (config.adminUser) { const allowedUsersKey: Record = { signal: 'SIGNAL_ALLOWED_USERS', @@ -80,6 +82,19 @@ export async function POST( // only the UUID, so a phone-only allowlist silently rejects the admin. const expanded = await expandSignalAllowlist(id, config.adminUser.split(',')) envVars[key] = expanded.join(',') + } else if (platform === 'telegram') { + // Resolve @usernames to numeric user IDs — the gateway matches inbound + // sender IDs (always numeric) against TELEGRAM_ALLOWED_USERS verbatim, + // so a raw @handle silently locks the admin out. STRICT: a handle that + // fails to resolve is a 400 (surfaced inline in the connect dialog), + // never stored raw. The token comes from the request payload because + // TELEGRAM_BOT_TOKEN may not be in the agent .env yet at connect time. + const resolution = await resolveTelegramAdmins(id, config.adminUser.split(','), config.token) + if (!resolution.ok) { + return NextResponse.json({ error: resolution.error }, { status: 400 }) + } + envVars[key] = resolution.ids.join(',') + telegramAdmins = resolution } else { envVars[key] = config.adminUser } @@ -104,6 +119,27 @@ export async function POST( fs.writeFileSync(envPath, content, { mode: 0o600 }) + if (telegramAdmins) { + // Persist display names the same way the settings path does (merged, not + // clobbered — connect only knows about telegram), so the settings UI can + // show "@handle (Name)" next to the numeric ID. + const resolvedPath = path.join(dataDir, 'resolved-identities.json') + let resolvedMap: Record = {} + try { + resolvedMap = JSON.parse(fs.readFileSync(resolvedPath, 'utf-8')) + } catch {} + resolvedMap.telegram = telegramAdmins.resolved + try { + fs.writeFileSync(resolvedPath, JSON.stringify(resolvedMap, null, 2), { mode: 0o600 }) + } catch {} + + // Keep the policy-plane admin overlay (SurfaceAdminService) converged with + // the allowlist we just wrote — the two stores must never diverge. + try { + services.surfaceAdmins.syncFromAllowlist(id, 'telegram', telegramAdmins.ids) + } catch {} + } + // Recreate the container so the new env_file actually takes effect. Writing // .env alone leaves the running gateway on the OLD environment (a stale // surface account/token), which silently keeps a broken connection alive. diff --git a/components/surfaces/surface-connect-dialog.tsx b/components/surfaces/surface-connect-dialog.tsx index bcddc80..a5554dc 100644 --- a/components/surfaces/surface-connect-dialog.tsx +++ b/components/surfaces/surface-connect-dialog.tsx @@ -407,18 +407,37 @@ function TelegramBody({ s, onClose }: { s: Hook; onClose: () => void }) { )} + {/* Non-blocking: with Group Privacy ON the bot still connects, it just + won't see unaddressed group messages until privacy is disabled. */} + {step !== 'input' && s.privacyModeOn === true && ( +
+ +
+

Group Privacy is ON

+

+ The bot will not see regular group messages — only commands and @mentions. + To disable: message @BotFather/setprivacyDisable. +

+

+ After changing it, the bot must be removed and re-added to any groups it is already in. +

+
+
+ )} +
- + s.setAdminUser(e.target.value)} - placeholder="Your Telegram user ID or @username" + placeholder="123456789, @username" className="w-full px-3 py-2 rounded-md border border-[var(--border)] bg-[var(--surface)] text-sm" disabled={step === 'verifying' || step === 'connecting'} />

- Who can manage this bot. Numeric user ID or @username. + Who can manage this bot. One or more entries, comma-separated — numeric + Telegram user IDs or @usernames; @usernames are resolved to IDs automatically.

diff --git a/hooks/useSurfaceRegister.test.tsx b/hooks/useSurfaceRegister.test.tsx index 9ad5ffc..6314f45 100644 --- a/hooks/useSurfaceRegister.test.tsx +++ b/hooks/useSurfaceRegister.test.tsx @@ -200,7 +200,10 @@ describe('useSurfaceRegister — telegram', () => { onConnected, }) ) - act(() => { result.current.setToken('123456:ABCdef') }) + act(() => { + result.current.setToken('123456:ABCdef') + result.current.setAdminUser('123456789, @juniper') + }) await act(async () => { await result.current.verify() }) expect(result.current.step).toBe('verified') @@ -211,10 +214,51 @@ describe('useSurfaceRegister — telegram', () => { const connectCall = fetchFn.mock.calls.find((c) => String(c[0]).includes('/surfaces/connect')) const body = bodyOf(connectCall!) expect(body.platform).toBe('telegram') - expect(body.config).toMatchObject({ token: '123456:ABCdef' }) + // Multiple admins ride as a comma-separated adminUser; the connect route + // splits, resolves @handles, and normalizes to numeric IDs server-side. + expect(body.config).toMatchObject({ token: '123456:ABCdef', adminUser: '123456789, @juniper' }) expect(onConnected).toHaveBeenCalled() }) + it('flags privacy mode when getMe reports can_read_all_group_messages: false', async () => { + mockFetch((url) => { + if (url.includes('api.telegram.org')) + return { ok: true, result: { username: 'mybot', can_read_all_group_messages: false } } + return {} + }) + const { result } = renderHook(() => + useSurfaceRegister({ + platform: 'telegram', + target: { kind: 'harness', harnessId: 'h_tg' }, + onConnected: vi.fn(), + }) + ) + expect(result.current.privacyModeOn).toBeNull() // unknown before verify + act(() => { result.current.setToken('123456:ABCdef') }) + await act(async () => { await result.current.verify() }) + expect(result.current.step).toBe('verified') + // Non-blocking: verified is still reached; the dialog shows a warning. + expect(result.current.privacyModeOn).toBe(true) + }) + + it('does not flag privacy mode when getMe reports can_read_all_group_messages: true', async () => { + mockFetch((url) => { + if (url.includes('api.telegram.org')) + return { ok: true, result: { username: 'mybot', can_read_all_group_messages: true } } + return {} + }) + const { result } = renderHook(() => + useSurfaceRegister({ + platform: 'telegram', + target: { kind: 'harness', harnessId: 'h_tg' }, + onConnected: vi.fn(), + }) + ) + act(() => { result.current.setToken('123456:ABCdef') }) + await act(async () => { await result.current.verify() }) + expect(result.current.privacyModeOn).toBe(false) + }) + it('pending mode: getMe verifies then captures { token } without connect', async () => { mockFetch((url) => { if (url.includes('api.telegram.org')) return { ok: true, result: { username: 'mybot' } } diff --git a/hooks/useSurfaceRegister.ts b/hooks/useSurfaceRegister.ts index febe570..60c5bbc 100644 --- a/hooks/useSurfaceRegister.ts +++ b/hooks/useSurfaceRegister.ts @@ -113,6 +113,10 @@ export function useSurfaceRegister(opts: UseSurfaceRegisterOptions) { // Slack-specific: a second token (app-level xapp- token for Socket Mode) const [appToken, setAppToken] = useState('') const [botUsername, setBotUsername] = useState('') + // Telegram-specific: true when BotFather Group Privacy is ON (getMe reports + // can_read_all_group_messages: false) — the bot won't see regular group + // messages. Non-blocking warning in the dialog; null = unknown/not checked. + const [privacyModeOn, setPrivacyModeOn] = useState(null) // ── Signal: daemon health + deploy ──────────────────────────────────────── @@ -314,6 +318,7 @@ export function useSurfaceRegister(opts: UseSurfaceRegisterOptions) { const data = await res.json() if (data.ok) { setBotUsername(data.result.username || data.result.first_name || 'Bot') + setPrivacyModeOn(data.result.can_read_all_group_messages === false) setStep('verified') } else { setError(data.description || 'Invalid bot token') @@ -461,6 +466,7 @@ export function useSurfaceRegister(opts: UseSurfaceRegisterOptions) { setUrl('') setAppToken('') setBotUsername('') + setPrivacyModeOn(null) }, [platform, defaultDisplayName]) /** @@ -507,6 +513,7 @@ export function useSurfaceRegister(opts: UseSurfaceRegisterOptions) { appToken, setAppToken, botUsername, + privacyModeOn, // signal actions checkHealth, deploy, diff --git a/lib/resolvers/index.test.ts b/lib/resolvers/index.test.ts index 2fe6a62..8f7c4ee 100644 --- a/lib/resolvers/index.test.ts +++ b/lib/resolvers/index.test.ts @@ -1,11 +1,14 @@ // @vitest-environment node /** - * Tests for expandSignalAllowlist. + * Tests for expandSignalAllowlist, resolveTelegramAdmins, expandTelegramAllowlist. * * Sealed-sender Signal DMs identify the sender only by UUID, never phone number. * The gateway compares that inbound UUID against SIGNAL_ALLOWED_USERS verbatim, * so a phone-number-only allowlist silently rejects the very person it names. * expandSignalAllowlist resolves each phone to its UUID and stores BOTH forms. + * + * Telegram has the analogous failure: the gateway matches numeric sender IDs + * against TELEGRAM_ALLOWED_USERS verbatim, so a raw @username never matches. */ import { describe, it, expect, vi, beforeEach } from 'vitest' @@ -13,11 +16,17 @@ vi.mock('./signal', () => ({ resolveSignalPhone: vi.fn(), getSignalAccountUuid: vi.fn(), })) +vi.mock('./telegram', () => ({ + resolveTelegramUsername: vi.fn(), + getTelegramDisplayName: vi.fn(), +})) -import { expandSignalAllowlist } from './index' +import { expandSignalAllowlist, resolveTelegramAdmins, expandTelegramAllowlist } from './index' import { resolveSignalPhone } from './signal' +import { resolveTelegramUsername } from './telegram' const mockResolve = vi.mocked(resolveSignalPhone) +const mockTgResolve = vi.mocked(resolveTelegramUsername) describe('expandSignalAllowlist', () => { beforeEach(() => vi.clearAllMocks()) @@ -77,3 +86,76 @@ describe('expandSignalAllowlist', () => { ]) }) }) + +describe('resolveTelegramAdmins (strict — connect path)', () => { + beforeEach(() => vi.clearAllMocks()) + + it('passes numeric IDs through without resolving', async () => { + const result = await resolveTelegramAdmins('h_x', ['123456789', '-1001234']) + expect(result).toEqual({ + ok: true, + ids: ['123456789', '-1001234'], + resolved: [ + { display: '123456789', nativeId: '123456789' }, + { display: '-1001234', nativeId: '-1001234' }, + ], + }) + expect(mockTgResolve).not.toHaveBeenCalled() + }) + + it('resolves @usernames to numeric IDs, passing the explicit bot token through', async () => { + mockTgResolve.mockResolvedValue({ display: '@juniper', nativeId: '424242', profileName: 'Juniper' }) + const result = await resolveTelegramAdmins('h_x', ['111', ' @juniper '], 'tok:abc') + expect(result).toEqual({ + ok: true, + ids: ['111', '424242'], + resolved: [ + { display: '111', nativeId: '111' }, + { display: '@juniper', nativeId: '424242', profileName: 'Juniper' }, + ], + }) + expect(mockTgResolve).toHaveBeenCalledWith('h_x', '@juniper', 'tok:abc') + }) + + it('fails (never stores the raw handle) when a handle does not resolve', async () => { + mockTgResolve.mockResolvedValue(null) + const result = await resolveTelegramAdmins('h_x', ['@ghost'], 'tok:abc') + expect(result.ok).toBe(false) + if (!result.ok) expect(result.error).toContain('@ghost') + }) + + it('rejects the wildcard as an admin entry', async () => { + const result = await resolveTelegramAdmins('h_x', ['*']) + expect(result.ok).toBe(false) + }) + + it('dedupes entries that resolve to the same numeric ID and skips blanks', async () => { + mockTgResolve.mockResolvedValue({ display: '@dup', nativeId: '999' }) + const result = await resolveTelegramAdmins('h_x', ['999', '@dup', '', ' ']) + expect(result).toMatchObject({ ok: true, ids: ['999'] }) + }) +}) + +describe('expandTelegramAllowlist (best-effort — settings path)', () => { + beforeEach(() => vi.clearAllMocks()) + + it('expands an @username entry to include its resolved numeric ID', async () => { + mockTgResolve.mockResolvedValue({ display: '@juniper', nativeId: '424242' }) + expect(await expandTelegramAllowlist('h_x', ['@juniper'])).toEqual(['@juniper', '424242']) + }) + + it('passes numeric IDs and the "*" wildcard through without resolving', async () => { + expect(await expandTelegramAllowlist('h_x', ['123', '*', '-100999'])).toEqual(['123', '*', '-100999']) + expect(mockTgResolve).not.toHaveBeenCalled() + }) + + it('keeps the handle unchanged when resolution fails (no worse than before)', async () => { + mockTgResolve.mockResolvedValue(null) + expect(await expandTelegramAllowlist('h_x', ['@ghost'])).toEqual(['@ghost']) + }) + + it('does not duplicate an ID already present in the list', async () => { + mockTgResolve.mockResolvedValue({ display: '@juniper', nativeId: '424242' }) + expect(await expandTelegramAllowlist('h_x', ['@juniper', '424242'])).toEqual(['@juniper', '424242']) + }) +}) diff --git a/lib/resolvers/index.ts b/lib/resolvers/index.ts index c565a30..05884d3 100644 --- a/lib/resolvers/index.ts +++ b/lib/resolvers/index.ts @@ -56,6 +56,102 @@ export async function expandSignalAllowlist( return out } +const TELEGRAM_NUMERIC_RE = /^-?\d+$/ + +export type TelegramAdminResolution = + | { ok: true; ids: string[]; resolved: ResolvedIdentity[] } + | { ok: false; error: string } + +/** + * Resolve a list of Telegram admin entries (numeric IDs and/or @usernames) to + * numeric user IDs. STRICT: any @username that fails to resolve is an error — + * the runtime matches TELEGRAM_ALLOWED_USERS against numeric sender IDs only, + * so silently storing a raw handle produces an allowlist that never matches + * (the admin is locked out with no visible symptom). + * + * `botToken` is passed through to getChat so this works at connect time, before + * TELEGRAM_BOT_TOKEN has been written to the agent .env. + * + * Order is preserved; duplicates (same numeric ID) are removed. + */ +export async function resolveTelegramAdmins( + harnessId: string, + entries: string[], + botToken?: string +): Promise { + const ids: string[] = [] + const resolved: ResolvedIdentity[] = [] + const seen = new Set() + + for (const raw of entries) { + const entry = raw.trim() + if (!entry) continue + if (entry === '*') { + return { ok: false, error: 'Wildcard (*) is not a valid admin entry' } + } + if (TELEGRAM_NUMERIC_RE.test(entry)) { + if (!seen.has(entry)) { + seen.add(entry) + ids.push(entry) + resolved.push({ display: entry, nativeId: entry }) + } + continue + } + const result = await resolveTelegramUsername(harnessId, entry, botToken) + if (!result?.nativeId) { + const handle = entry.startsWith('@') ? entry : `@${entry}` + return { + ok: false, + error: `Could not resolve ${handle} to a Telegram user ID. Check the spelling, or use the numeric user ID instead (the account must have a public username).`, + } + } + if (!seen.has(result.nativeId)) { + seen.add(result.nativeId) + ids.push(result.nativeId) + resolved.push(result) + } + } + + return { ok: true, ids, resolved } +} + +/** + * Expand a Telegram allowlist so every @username entry is stored alongside its + * resolved numeric user ID (mirrors expandSignalAllowlist). + * + * The gateway matches inbound sender IDs — always numeric — against + * TELEGRAM_ALLOWED_USERS verbatim, so an @username-only entry never matches + * anyone. Best-effort: entries that fail to resolve pass through unchanged + * (no worse than before; resolution retries on the next settings write). + * + * Order is preserved and duplicates are removed. + */ +export async function expandTelegramAllowlist( + harnessId: string, + identifiers: string[] +): Promise { + const out: string[] = [] + const seen = new Set() + const push = (value: string) => { + const trimmed = value.trim() + if (trimmed && !seen.has(trimmed)) { + seen.add(trimmed) + out.push(trimmed) + } + } + + for (const raw of identifiers) { + const id = raw.trim() + if (!id) continue + push(id) + if (id === '*' || TELEGRAM_NUMERIC_RE.test(id)) continue + const resolved = await resolveTelegramUsername(harnessId, id) + if (resolved?.nativeId) push(resolved.nativeId) + } + + return out +} + /** * Resolve an identifier for a given platform. * Detects whether the input is already a native ID or needs resolution. diff --git a/lib/resolvers/telegram.ts b/lib/resolvers/telegram.ts index a14e89f..650330c 100644 --- a/lib/resolvers/telegram.ts +++ b/lib/resolvers/telegram.ts @@ -20,12 +20,17 @@ function getTelegramToken(harnessId: string): string | null { /** * Resolve a Telegram @username to numeric user ID via getChat. * Only works for public users/channels/groups. + * + * `botToken` overrides the token read from the agent .env — the connect route + * passes the token from the request payload because TELEGRAM_BOT_TOKEN may not + * be written to the .env yet at connect time. */ export async function resolveTelegramUsername( harnessId: string, - username: string + username: string, + botToken?: string ): Promise { - const token = getTelegramToken(harnessId) + const token = botToken || getTelegramToken(harnessId) if (!token) return null const handle = username.startsWith('@') ? username : `@${username}` diff --git a/lib/services/__tests__/surface-admins.test.ts b/lib/services/__tests__/surface-admins.test.ts index 50881dd..6c9df33 100644 --- a/lib/services/__tests__/surface-admins.test.ts +++ b/lib/services/__tests__/surface-admins.test.ts @@ -191,6 +191,172 @@ describe('SurfaceAdminService.setAdmins — authz + validation', () => { }) }) +describe('SurfaceAdminService.syncFromAllowlist — store convergence', () => { + it('is a no-op when no explicit list exists (bootstrap default already tracks the env)', () => { + writeAgentEnv('seraph', 'TELEGRAM_ALLOWED_USERS=111\n') + svc.syncFromAllowlist('h_seraph', 'telegram', ['111', '222']) + expect(storage.read('harnesses.json', [])).toEqual([]) + expect(audit.append).not.toHaveBeenCalled() + }) + + it('replaces a stale explicit list with the new allowlist', () => { + storage.write('harnesses.json', [ + { id: 'h_seraph', surfaceAdmins: { telegram: ['999'] } }, + ]) + svc.syncFromAllowlist('h_seraph', 'telegram', ['111', '222']) + const stored = storage.read }>>('harnesses.json', []) + expect(stored.find((h) => h.id === 'h_seraph')?.surfaceAdmins?.telegram).toEqual(['111', '222']) + // The policy plane now answers from the converged list. + expect(svc.isAdmin('h_seraph', 'telegram', '222')).toBe(true) + expect(svc.isAdmin('h_seraph', 'telegram', '999')).toBe(false) + expect(audit.append).toHaveBeenCalledWith( + expect.objectContaining({ what: 'surface-admins:sync:telegram', target: 'h_seraph' }), + ) + }) + + it('drops invalid entries (raw @handles, wildcards) and dedupes — never stores an unresolved handle', () => { + storage.write('harnesses.json', [ + { id: 'h_seraph', surfaceAdmins: { telegram: ['999'] } }, + ]) + svc.syncFromAllowlist('h_seraph', 'telegram', ['@juniper', '111', '*', '111']) + const stored = storage.read }>>('harnesses.json', []) + expect(stored.find((h) => h.id === 'h_seraph')?.surfaceAdmins?.telegram).toEqual(['111']) + }) + + it('does not touch other platforms on the same overlay', () => { + storage.write('harnesses.json', [ + { id: 'h_seraph', surfaceAdmins: { telegram: ['999'], signal: ['+64111'] } }, + ]) + svc.syncFromAllowlist('h_seraph', 'telegram', ['111']) + const stored = storage.read }>>('harnesses.json', []) + expect(stored.find((h) => h.id === 'h_seraph')?.surfaceAdmins?.signal).toEqual(['+64111']) + }) + + it('ignores unsupported platforms', () => { + storage.write('harnesses.json', [ + { id: 'h_seraph', surfaceAdmins: { telegram: ['999'] } }, + ]) + svc.syncFromAllowlist('h_seraph', 'whatsapp', ['111']) + const stored = storage.read }>>('harnesses.json', []) + expect(stored.find((h) => h.id === 'h_seraph')?.surfaceAdmins?.telegram).toEqual(['999']) + }) +}) + +describe('SurfaceAdminService.approveGroupInvite — policy × admin matrix', () => { + function agentEnvContent(harnessName: string): string { + return fs.readFileSync(path.join(home, `.hermes-${harnessName}`, '.env'), 'utf-8') + } + + it('approves and appends when policy is unset (approved-only default) and the adder is an admin', () => { + writeAgentEnv('seraph', 'TELEGRAM_ALLOWED_USERS=111\nTELEGRAM_GROUP_ALLOWED_CHATS=\n') + const res = svc.approveGroupInvite('h_seraph', 'telegram', '-100777', '111') + expect(res).toEqual({ ok: true, approved: true, updated: true }) + expect(agentEnvContent('seraph')).toContain('TELEGRAM_GROUP_ALLOWED_CHATS=-100777') + expect(svc.isGroupAllowed('h_seraph', 'telegram', '-100777')).toBe(true) + expect(audit.append).toHaveBeenCalledWith( + expect.objectContaining({ who: '111', what: 'surface-groups:approve:telegram', target: 'h_seraph' }), + ) + }) + + it('rejects (approved: false, no write) when the adder is not an admin', () => { + writeAgentEnv('seraph', 'TELEGRAM_ALLOWED_USERS=111\nTELEGRAM_GROUP_ALLOWED_CHATS=g1\n') + const res = svc.approveGroupInvite('h_seraph', 'telegram', '-100777', '222') + expect(res).toMatchObject({ ok: true, approved: false }) + if (res.ok && !res.approved) expect(res.reason).toContain('not an admin') + expect(agentEnvContent('seraph')).not.toContain('-100777') + }) + + it('approves for a non-admin when policy is allow-all, and appends', () => { + writeAgentEnv('seraph', 'TELEGRAM_ALLOWED_USERS=111\nTELEGRAM_GROUP_INVITE_POLICY=allow-all\n') + const res = svc.approveGroupInvite('h_seraph', 'telegram', '-100777', '222') + expect(res).toEqual({ ok: true, approved: true, updated: true }) + expect(agentEnvContent('seraph')).toContain('TELEGRAM_GROUP_ALLOWED_CHATS=-100777') + }) + + it('rejects a non-admin under an explicit approved-only policy', () => { + writeAgentEnv('seraph', 'TELEGRAM_ALLOWED_USERS=111\nTELEGRAM_GROUP_INVITE_POLICY=approved-only\n') + expect(svc.approveGroupInvite('h_seraph', 'telegram', '-100777', '222')).toMatchObject({ + ok: true, + approved: false, + }) + }) + + it('honors the explicit admin overlay for the admin check', () => { + writeAgentEnv('seraph', 'TELEGRAM_ALLOWED_USERS=111\n') + storage.write('harnesses.json', [ + { id: 'h_seraph', surfaceAdmins: { telegram: ['222'] } }, + ]) + expect(svc.approveGroupInvite('h_seraph', 'telegram', '-100777', '111')).toMatchObject({ approved: false }) + expect(svc.approveGroupInvite('h_seraph', 'telegram', '-100777', '222')).toMatchObject({ approved: true }) + }) + + it('is a no-write approval when the allowlist is the * wildcard', () => { + writeAgentEnv('seraph', 'TELEGRAM_ALLOWED_USERS=111\nTELEGRAM_GROUP_ALLOWED_CHATS=*\n') + const res = svc.approveGroupInvite('h_seraph', 'telegram', '-100777', '111') + expect(res).toEqual({ ok: true, approved: true, updated: false }) + expect(agentEnvContent('seraph')).toContain('TELEGRAM_GROUP_ALLOWED_CHATS=*') + }) + + it('is a no-write approval when the group is already listed', () => { + writeAgentEnv('seraph', 'TELEGRAM_ALLOWED_USERS=111\nTELEGRAM_GROUP_ALLOWED_CHATS=-100777\n') + const res = svc.approveGroupInvite('h_seraph', 'telegram', '-100777', '111') + expect(res).toEqual({ ok: true, approved: true, updated: false }) + }) + + it('appends without clobbering existing groups', () => { + writeAgentEnv('seraph', 'TELEGRAM_ALLOWED_USERS=111\nTELEGRAM_GROUP_ALLOWED_CHATS=-100111\n') + svc.approveGroupInvite('h_seraph', 'telegram', '-100777', '111') + expect(agentEnvContent('seraph')).toContain('TELEGRAM_GROUP_ALLOWED_CHATS=-100111,-100777') + }) + + it('rejects structurally invalid group ids with 400 (env injection guard)', () => { + writeAgentEnv('seraph', 'TELEGRAM_ALLOWED_USERS=111\n') + // '$'/backtick: String.replace replacement patterns ("x$`" splices the + // preceding file content — incl. the bot token line — into the allowlist). + for (const bad of ['*', '', 'g1,g2', 'g1\nKEY=val', 'a b', 'g#1', "g'1", 'x$`', 'x$&', 'g$1', 'g`id`']) { + expect(svc.approveGroupInvite('h_seraph', 'telegram', bad, '111')).toMatchObject({ ok: false, status: 400 }) + } + }) + + it('inserts the allowlist value literally even if a replacement-pattern char reached the write', () => { + // Belt-and-braces for the replacer-function fix: token line must survive + // verbatim and the value may not splice file content. + writeAgentEnv( + 'seraph', + 'TELEGRAM_BOT_TOKEN=secretAAA\nTELEGRAM_GROUP_ALLOWED_CHATS=-100111\nTELEGRAM_ALLOWED_USERS=111\n', + ) + svc.approveGroupInvite('h_seraph', 'telegram', '-100777', '111') + const content = agentEnvContent('seraph') + expect(content).toContain('TELEGRAM_BOT_TOKEN=secretAAA\n') + expect(content).toContain('TELEGRAM_GROUP_ALLOWED_CHATS=-100111,-100777') + expect(content.match(/TELEGRAM_BOT_TOKEN=/g)).toHaveLength(1) + }) + + it('rejects an oversized addedByUserId with 400 (audit-log spam guard)', () => { + writeAgentEnv('seraph', 'TELEGRAM_ALLOWED_USERS=111\n') + expect( + svc.approveGroupInvite('h_seraph', 'telegram', '-100777', '9'.repeat(129)), + ).toMatchObject({ ok: false, status: 400 }) + }) + + it('rejects a missing addedByUserId and an unsupported platform with 400', () => { + writeAgentEnv('seraph', 'TELEGRAM_ALLOWED_USERS=111\n') + expect(svc.approveGroupInvite('h_seraph', 'telegram', '-100777', '')).toMatchObject({ ok: false, status: 400 }) + expect(svc.approveGroupInvite('h_seraph', 'whatsapp', '-100777', '111')).toMatchObject({ ok: false, status: 400 }) + }) + + it('rejects when the agent .env does not exist', () => { + expect(svc.approveGroupInvite('h_ghost', 'telegram', '-100777', '111')).toMatchObject({ ok: false, status: 400 }) + }) + + it('works for signal too (shared invite-policy plumbing)', () => { + writeAgentEnv('seraph', 'SIGNAL_ALLOWED_USERS=+64111\nSIGNAL_GROUP_INVITE_POLICY=approved-only\n') + const res = svc.approveGroupInvite('h_seraph', 'signal', 'group.abc', '+64111') + expect(res).toMatchObject({ ok: true, approved: true, updated: true }) + expect(agentEnvContent('seraph')).toContain('SIGNAL_GROUP_ALLOWED_USERS=group.abc') + }) +}) + describe('SurfaceAdminService.isGroupAllowed', () => { it('is true for a listed group and for a wildcard', () => { writeAgentEnv('seraph', 'SIGNAL_GROUP_ALLOWED_USERS=g1,g2\n') diff --git a/lib/services/surface-admins.ts b/lib/services/surface-admins.ts index c215904..ec86ab2 100644 --- a/lib/services/surface-admins.ts +++ b/lib/services/surface-admins.ts @@ -20,6 +20,25 @@ const ALLOWED_USERS_VARS: Record = { slack: 'SLACK_ALLOWED_USERS', } +// Per-platform group/channel allowlist env var (which groups the agent responds +// in). Shared by isGroupAllowed and approveGroupInvite. +const GROUP_ALLOWED_VARS: Record = { + signal: 'SIGNAL_GROUP_ALLOWED_USERS', + telegram: 'TELEGRAM_GROUP_ALLOWED_CHATS', + mattermost: 'MATTERMOST_ALLOWED_CHANNELS', + discord: 'DISCORD_ALLOWED_CHANNELS', + slack: 'SLACK_ALLOWED_CHANNELS', +} + +// Per-platform group-invite policy env var (who may add the agent to groups). +// Must stay in step with GROUP_INVITE_VARS in the settings route, which writes +// these from the single UI toggle. Unset = approved-only (secure default). +const GROUP_INVITE_POLICY_VARS: Record = { + signal: 'SIGNAL_GROUP_INVITE_POLICY', + slack: 'SLACK_CHANNEL_POLICY', + telegram: 'TELEGRAM_GROUP_INVITE_POLICY', +} + export const SUPPORTED_SURFACES = Object.keys(ALLOWED_USERS_VARS) export function isSupportedSurface(platform: string): boolean { @@ -263,6 +282,57 @@ export class SurfaceAdminService { return { ok: true, admins: cleaned } } + /** + * Keep the explicit admin overlay converged with the surface's DM allowlist + * after an OPERATOR write to the ALLOWED_USERS env var (surface connect or + * settings PUT). + * + * Why: the two admin stores can diverge — the overlay is HSM-side policy + * served live, while ALLOWED_USERS is the bootstrap set. If an operator + * rewrites the allowlist while a stale explicit overlay exists, the policy + * plane keeps answering from the stale list. This replaces the explicit list + * with the valid identities from the new allowlist — but ONLY when an + * explicit list already exists. With no explicit list, listAdmins already + * reads the allowlist live (bootstrap default), so there is nothing to sync + * and writing one would change the setAdmins bootstrap-actor semantics. + * + * No actor check: this is not a surface-identity mutation — it runs inside + * operator-transport-gated routes (middleware.ts) as a side effect of the env + * write the operator just made. Invalid entries (wildcards, unresolved + * @handles) are dropped, never stored. + */ + syncFromAllowlist(harnessId: string, platform: string, allowlist: string[]): void { + if (!isSupportedSurface(platform)) return + const overlays = this.readOverlays() + const idx = overlays.findIndex((h) => h.id === harnessId) + const existing = idx !== -1 ? overlays[idx].surfaceAdmins?.[platform] : undefined + if (!Array.isArray(existing)) return // no explicit list → bootstrap default already tracks the env + + const cleaned: string[] = [] + const seen = new Set() + for (const entry of allowlist) { + if (!isValidIdentity(platform, entry)) continue + const id = entry.trim() + if (!seen.has(id)) { + seen.add(id) + cleaned.push(id) + } + } + + overlays[idx] = { + ...overlays[idx], + surfaceAdmins: { ...overlays[idx].surfaceAdmins, [platform]: cleaned }, + } + this.storage.write(HARNESSES_FILE, overlays) + + this.audit?.append({ + who: 'operator', + what: `surface-admins:sync:${platform}`, + target: harnessId, + meta: { count: cleaned.length }, + }) + } + /** * Is `groupId` allowed on `platform` for this harness? Mirrors the existing * /policy?action=group-check logic, exposed at the REST path the plugin's @@ -270,14 +340,7 @@ export class SurfaceAdminService { */ isGroupAllowed(harnessId: string, platform: string, groupId: string): boolean { if (!harnessId || !platform || !groupId) return false - const groupVars: Record = { - signal: 'SIGNAL_GROUP_ALLOWED_USERS', - telegram: 'TELEGRAM_GROUP_ALLOWED_CHATS', - mattermost: 'MATTERMOST_ALLOWED_CHANNELS', - discord: 'DISCORD_ALLOWED_CHANNELS', - slack: 'SLACK_ALLOWED_CHANNELS', - } - const varName = groupVars[platform] + const varName = GROUP_ALLOWED_VARS[platform] if (!varName) return false try { const env = parseEnvFile(path.join(agentDataDir(harnessId), '.env')) @@ -288,4 +351,96 @@ export class SurfaceAdminService { return false } } + + /** + * Group-invite enforcement for the swarm_map_policy plugin: the agent was + * just added to `groupId` by `addedByUserId` — approve or reject the invite. + * + * - Policy 'allow-all': anyone may add the agent → approved; the group is + * appended to the allowlist so the agent actually responds there. + * - Policy 'approved-only' (or unset — the secure default): approved only + * when `addedByUserId` is an admin (isAdmin, fail-closed), in which case + * the group is appended to the allowlist. + * - A '*' allowlist or an already-listed group approves without a write + * (updated: false — no container recreate needed). + * + * `groupId` is structurally validated before it touches the .env (same + * injection guard rationale as isValidIdentity — a comma/newline in the value + * would corrupt the allowlist or inject env lines). + * + * The caller (route) recreates the container when `updated` is true, matching + * every other env-writing route. + */ + approveGroupInvite( + harnessId: string, + platform: string, + groupId: string, + addedByUserId: string, + ): + | { ok: true; approved: true; updated: boolean } + | { ok: true; approved: false; reason: string } + | { ok: false; status: 400; error: string } { + const varName = GROUP_ALLOWED_VARS[platform] + if (!varName) { + return { ok: false, status: 400, error: `Unsupported platform: ${platform}` } + } + // Structural guard before the value touches the .env: no whitespace/control + // chars (line injection), no comma (list separator), no comment/quote chars, + // no '$'/backtick (String.replace replacement patterns + shell expansion). + // '=' is deliberately allowed — Signal group IDs are base64 ('=' padding), + // and only the FIRST '=' on a line splits key from value. + const group = typeof groupId === 'string' ? groupId.trim() : '' + if (!group || group.length > 128 || group === '*' || /[\s,;#'"$`]/.test(group)) { + return { ok: false, status: 400, error: 'Invalid group id' } + } + const actor = typeof addedByUserId === 'string' ? addedByUserId.trim() : '' + if (!actor || actor.length > 128) { + return { ok: false, status: 400, error: 'addedByUserId is required (max 128 chars)' } + } + + const envPath = path.join(agentDataDir(harnessId), '.env') + if (!fs.existsSync(envPath)) { + return { ok: false, status: 400, error: 'Agent .env not found' } + } + const env = parseEnvFile(envPath) + + // Unset defaults to approved-only — the secure default everywhere else. + const policyVar = GROUP_INVITE_POLICY_VARS[platform] + const policy = policyVar ? env[policyVar] : undefined + if (policy !== 'allow-all' && !this.isAdmin(harnessId, platform, actor)) { + return { + ok: true, + approved: false, + reason: 'group invite policy is approved-only and the inviting user is not an admin', + } + } + + const raw = env[varName] + if (raw === '*' || parseCommaList(raw).includes(group)) { + return { ok: true, approved: true, updated: false } + } + + // Append the group to the allowlist line (create it if absent). + const value = [...parseCommaList(raw), group].join(',') + let content = fs.readFileSync(envPath, 'utf-8') + const regex = new RegExp(`^${varName}=.*$`, 'm') + if (regex.test(content)) { + // Replacer function: the value must be inserted literally — a bare string + // here would expand `$&`/`` $` ``-style replacement patterns and splice + // surrounding .env content into the line. + content = content.replace(regex, () => `${varName}=${value}`) + } else { + content = content.trimEnd() + `\n${varName}=${value}\n` + } + fs.writeFileSync(envPath, content, { mode: 0o600 }) + + this.audit?.append({ + who: actor, + what: `surface-groups:approve:${platform}`, + target: harnessId, + meta: { groupId: group }, + }) + + return { ok: true, approved: true, updated: true } + } } diff --git a/middleware.test.ts b/middleware.test.ts index 979bd50..0038647 100644 --- a/middleware.test.ts +++ b/middleware.test.ts @@ -58,6 +58,19 @@ describe('middleware auth gate — token SET', () => { expect(passedThrough(await middleware(req('/api/harnesses', 'GET', good)))).toBe(true) }) + // --- agent-callable POST: group-invite approval (the ONLY ungated mutation) --- + it('POST agent groups path (group-invite approval) → passes ungated', async () => { + expect(passedThrough(await middleware(req(AGENT_GROUPS, 'POST')))).toBe(true) + }) + it('PUT / PATCH / DELETE on the groups path are still gated', async () => { + for (const m of ['PUT', 'PATCH', 'DELETE']) { + expect((await middleware(req(AGENT_GROUPS, m))).status).toBe(401) + } + }) + it('POST on the sibling admins path is still gated (only groups is agent-callable)', async () => { + expect((await middleware(req(AGENT_ADMIN, 'POST'))).status).toBe(401) + }) + // --- mutations always require the session --- it('POST with no cookie → 401', async () => { const res = await middleware(req('/api/harnesses/create', 'POST')) @@ -105,4 +118,7 @@ describe('middleware auth gate — token UNSET (fail-closed)', () => { expect(passedThrough(await middleware(req(AGENT_ADMIN, 'GET')))).toBe(true) expect(passedThrough(await middleware(req(AGENT_GROUPS, 'GET')))).toBe(true) }) + it('agent group-invite approval POST still passes (same fleet-keeps-working rationale)', async () => { + expect(passedThrough(await middleware(req(AGENT_GROUPS, 'POST')))).toBe(true) + }) }) diff --git a/middleware.ts b/middleware.ts index ca023cb..ff8a7fb 100644 --- a/middleware.ts +++ b/middleware.ts @@ -46,8 +46,29 @@ const AGENT_READABLE_GET_PATHS: RegExp[] = [ /^\/api\/harnesses\/[^/]+\/policy$/, // agent .env policy read ] +// The ONLY POST path agents call at runtime: group-invite approval ("user X +// just added me to group Y — approved?"). The plugin cannot obtain the operator +// cookie, so this rides the same exemption as the agent reads above. +// +// TRUST MODEL: the caller self-reports addedByUserId — the server cannot verify +// who really performed the platform-side invite, so any local process could +// claim an admin's ID. What bounds the blast radius: the handler can ONLY +// append one structurally-validated groupId to that surface's group allowlist +// (never admins, tokens, or any other env var), the invite policy + admin check +// still gate it, and every approval is audit-logged with the claimed actor. +// This mirrors the trust already extended to the agent's is_admin / +// is_group_allowed reads, which the same plugin acts on. +const AGENT_CALLABLE_POST_PATHS: RegExp[] = [ + /^\/api\/harnesses\/[^/]+\/surfaces\/[^/]+\/groups\/[^/]+$/, // group-invite approval +] + function requiresAuth(method: string, pathname: string): boolean { - if (MUTATING_METHODS.has(method)) return true + if (MUTATING_METHODS.has(method)) { + if (method === 'POST' && AGENT_CALLABLE_POST_PATHS.some((re) => re.test(pathname))) { + return false + } + return true + } if (READ_METHODS.has(method)) { return !AGENT_READABLE_GET_PATHS.some((re) => re.test(pathname)) }