From 7d15dbceeb78209e465e0f7010d1d9dcda8ec556 Mon Sep 17 00:00:00 2001 From: hinotoi-agent Date: Wed, 20 May 2026 10:12:26 +0800 Subject: [PATCH] fix(permissions): scope channel approval targets Filter channel registration target options to the approver's authorized agent groups and re-check target authorization before applying a pending approval. Add regression coverage for scoped admins attempting to connect channels to out-of-scope groups. --- .../permissions/channel-approval.test.ts | 81 +++++++++++++++++++ src/modules/permissions/channel-approval.ts | 32 +++++--- src/modules/permissions/index.ts | 10 ++- 3 files changed, 113 insertions(+), 10 deletions(-) diff --git a/src/modules/permissions/channel-approval.test.ts b/src/modules/permissions/channel-approval.test.ts index a2e66900b..02e87d373 100644 --- a/src/modules/permissions/channel-approval.test.ts +++ b/src/modules/permissions/channel-approval.test.ts @@ -357,6 +357,87 @@ describe('unknown-channel registration flow', () => { .c; expect(stillPending).toBe(1); }); + + it('does not let a scoped admin connect an unknown channel to another agent group', async () => { + const { routeInbound } = await import('../../router.js'); + const { getResponseHandlers } = await import('../../response-registry.js'); + const { getDb } = await import('../../db/connection.js'); + + createAgentGroup({ id: 'ag-2', name: 'Betty', folder: 'betty', agent_provider: null, created_at: now() }); + upsertUser({ id: 'telegram:scoped-admin', kind: 'telegram', display_name: 'Scoped Admin', created_at: now() }); + grantRole({ + user_id: 'telegram:scoped-admin', + role: 'admin', + agent_group_id: 'ag-1', + granted_by: 'telegram:owner', + granted_at: now(), + }); + createMessagingGroup({ + id: 'mg-dm-scoped-admin', + channel_type: 'telegram', + platform_id: 'dm-scoped-admin', + name: 'Scoped Admin DM', + is_group: 0, + unknown_sender_policy: 'public', + created_at: now(), + }); + getDb() + .prepare( + `INSERT INTO user_dms (user_id, channel_type, messaging_group_id, resolved_at) + VALUES (?, ?, ?, ?)`, + ) + .run('telegram:scoped-admin', 'telegram', 'mg-dm-scoped-admin', now()); + + await routeInbound(groupMention('chat-scoped-cross-group')); + await new Promise((r) => setTimeout(r, 10)); + + const pending = getDb().prepare('SELECT messaging_group_id FROM pending_channel_approvals').get() as { + messaging_group_id: string; + }; + expect(pending).toBeDefined(); + expect(deliverMock).toHaveBeenCalledTimes(1); + expect(deliverMock.mock.calls[0][1]).toBe('dm-scoped-admin'); + + for (const handler of getResponseHandlers()) { + const claimed = await handler({ + questionId: pending.messaging_group_id, + value: 'choose_existing', + userId: 'scoped-admin', + channelType: 'telegram', + platformId: 'dm-scoped-admin', + threadId: null, + }); + if (claimed) break; + } + + const followupPayload = JSON.parse(deliverMock.mock.calls[1][4] as string) as { + options: Array<{ label: string; value: string }>; + }; + expect(followupPayload.options.map((option) => option.value)).toContain('connect:ag-1'); + expect(followupPayload.options.map((option) => option.value)).not.toContain('connect:ag-2'); + + for (const handler of getResponseHandlers()) { + const claimed = await handler({ + questionId: pending.messaging_group_id, + value: 'connect:ag-2', + userId: 'scoped-admin', + channelType: 'telegram', + platformId: 'dm-scoped-admin', + threadId: null, + }); + if (claimed) break; + } + + const mgaCount = ( + getDb() + .prepare('SELECT COUNT(*) AS c FROM messaging_group_agents WHERE messaging_group_id = ?') + .get(pending.messaging_group_id) as { c: number } + ).c; + expect(mgaCount).toBe(0); + const stillPending = (getDb().prepare('SELECT COUNT(*) AS c FROM pending_channel_approvals').get() as { c: number }) + .c; + expect(stillPending).toBe(1); + }); }); describe('no-owner / no-agent failure modes', () => { diff --git a/src/modules/permissions/channel-approval.ts b/src/modules/permissions/channel-approval.ts index 6127cea2f..762fc5c4a 100644 --- a/src/modules/permissions/channel-approval.ts +++ b/src/modules/permissions/channel-approval.ts @@ -55,6 +55,7 @@ import type { InboundEvent } from '../../channels/adapter.js'; import type { AgentGroup } from '../../types.js'; import { pickApprovalDelivery, pickApprover } from '../approvals/primitive.js'; import { createPendingChannelApproval, hasInFlightChannelApproval } from './db/pending-channel-approvals.js'; +import { hasAdminPrivilege } from './db/user-roles.js'; // ── Value constants (response handler in index.ts parses these) ── @@ -76,15 +77,24 @@ function toFolder(name: string): string { // ── Card builders ── -function buildApprovalOptions(agentGroups: AgentGroup[]): RawOption[] { +function visibleAgentGroupsForApprover( + agentGroups: AgentGroup[], + approverUserId: string | null | undefined, +): AgentGroup[] { + if (!approverUserId) return agentGroups; + return agentGroups.filter((agentGroup) => hasAdminPrivilege(approverUserId, agentGroup.id)); +} + +function buildApprovalOptions(agentGroups: AgentGroup[], approverUserId?: string | null): RawOption[] { + const visibleAgentGroups = visibleAgentGroupsForApprover(agentGroups, approverUserId); const options: RawOption[] = []; - if (agentGroups.length === 1) { + if (visibleAgentGroups.length === 1) { options.push({ - label: `Connect to ${agentGroups[0].name}`, - selectedLabel: `✅ Connected to ${agentGroups[0].name}`, - value: `${CONNECT_PREFIX}${agentGroups[0].id}`, + label: `Connect to ${visibleAgentGroups[0].name}`, + selectedLabel: `✅ Connected to ${visibleAgentGroups[0].name}`, + value: `${CONNECT_PREFIX}${visibleAgentGroups[0].id}`, }); - } else { + } else if (visibleAgentGroups.length > 1) { options.push({ label: 'Choose existing agent', selectedLabel: '📋 Choosing…', @@ -194,7 +204,7 @@ export async function requestChannelApproval(input: RequestChannelApprovalInput) const channelName = originMg?.name ?? null; const title = isGroup ? '📣 Bot mentioned in new channel' : '💬 New direct message'; const question = buildQuestionText(isGroup, senderName, channelName, originChannelType); - const options = normalizeOptions(buildApprovalOptions(agentGroups)); + const options = normalizeOptions(buildApprovalOptions(agentGroups, delivery.userId)); createPendingChannelApproval({ messaging_group_id: messagingGroupId, @@ -241,8 +251,12 @@ export async function requestChannelApproval(input: RequestChannelApprovalInput) /** * Build normalized options for the agent-selection follow-up card. */ -export function buildAgentSelectionOptions(agentGroups: AgentGroup[]): NormalizedOption[] { - const options: RawOption[] = agentGroups.map((ag) => ({ +export function buildAgentSelectionOptions( + agentGroups: AgentGroup[], + approverUserId?: string | null, +): NormalizedOption[] { + const visibleAgentGroups = visibleAgentGroupsForApprover(agentGroups, approverUserId); + const options: RawOption[] = visibleAgentGroups.map((ag) => ({ label: ag.name, selectedLabel: `✅ Connected to ${ag.name}`, value: `${CONNECT_PREFIX}${ag.id}`, diff --git a/src/modules/permissions/index.ts b/src/modules/permissions/index.ts index 2b51d63dd..67adfe771 100644 --- a/src/modules/permissions/index.ts +++ b/src/modules/permissions/index.ts @@ -354,7 +354,7 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise< if (!adapter) return true; const agentGroups = getAllAgentGroups(); - const options = buildAgentSelectionOptions(agentGroups); + const options = buildAgentSelectionOptions(agentGroups, approverId); const title = '📋 Choose an agent'; updatePendingChannelApprovalCard(row.messaging_group_id, title, JSON.stringify(options)); @@ -438,6 +438,14 @@ async function handleChannelApprovalResponse(payload: ResponsePayload): Promise< deletePendingChannelApproval(row.messaging_group_id); return true; } + if (!hasAdminPrivilege(approverId, targetAgentGroupId)) { + log.warn('Channel registration: target agent group rejected for unauthorized approver', { + messagingGroupId: row.messaging_group_id, + targetAgentGroupId, + approverId, + }); + return true; + } } else { log.warn('Channel registration: unknown response value', { messagingGroupId: row.messaging_group_id,