Merge pull request #2566 from Hinotoi-agent/fix/channel-approval-target-authz

[security] fix(permissions): scope channel approval targets
This commit is contained in:
gavrielc
2026-05-22 22:57:28 +03:00
committed by GitHub
3 changed files with 113 additions and 10 deletions
@@ -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', () => {
+23 -9
View File
@@ -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}`,
+9 -1
View File
@@ -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,