From 318a7bdda8c69841e0f548a0197f5a338327d03d Mon Sep 17 00:00:00 2001 From: Jarvis Date: Mon, 13 Jul 2026 03:36:18 -0500 Subject: [PATCH] fix(discord): bind stops to approving admin --- apps/gateway/src/chat/chat.gateway.ts | 73 +++++++++++++----- .../plugin/discord-ingress.security.spec.ts | 75 ++++++++++++++++--- plugins/discord/src/index.ts | 59 ++++++++++++++- 3 files changed, 175 insertions(+), 32 deletions(-) diff --git a/apps/gateway/src/chat/chat.gateway.ts b/apps/gateway/src/chat/chat.gateway.ts index edacf2d..1cde3a7 100644 --- a/apps/gateway/src/chat/chat.gateway.ts +++ b/apps/gateway/src/chat/chat.gateway.ts @@ -1,3 +1,4 @@ +import { createHash } from 'node:crypto'; import { Inject, Logger, Optional } from '@nestjs/common'; import { WebSocketGateway, @@ -14,6 +15,7 @@ import type { AgentSessionEvent } from '@mariozechner/pi-coding-agent'; import { verifyDiscordIngressEnvelope, parseDiscordInteractionBindings, + resolveDiscordInteractionActorId, resolveDiscordInteractionBinding, type DiscordIngressEnvelope, type DiscordIngressPayload, @@ -655,8 +657,8 @@ export class ChatGateway implements OnGatewayInit, OnGatewayConnection, OnGatewa if (!client.data.discordService) return; const ingress = this.resolveDiscordIngress(client, envelope, 'approve'); const actionParts = ingress?.content.match(/^\/approve\s+([^\s]+)\s+([^\s]+)$/i); - const serviceUserId = process.env['DISCORD_SERVICE_USER_ID']; - if (!ingress || !actionParts || !serviceUserId || !this.commandAuthorization) return; + const tenantId = process.env['DISCORD_SERVICE_TENANT_ID']?.trim(); + if (!ingress || !actionParts || !tenantId || !this.commandAuthorization) return; const binding = resolveDiscordInteractionBinding( parseDiscordInteractionBindings(process.env['DISCORD_INTERACTION_BINDINGS']), ingress.guildId, @@ -664,14 +666,22 @@ export class ChatGateway implements OnGatewayInit, OnGatewayConnection, OnGatewa ingress.userId, 'approve', ); - if (!binding) return; + const actorId = binding && resolveDiscordInteractionActorId(binding, ingress.userId); + if (!actorId) return; + const providerId = actionParts[1]!; + const sessionId = actionParts[2]!; const approval = await this.commandAuthorization.createRuntimeTerminationApproval({ - providerId: actionParts[1]!, - sessionId: actionParts[2]!, - actorId: serviceUserId, - tenantId: process.env['DISCORD_SERVICE_TENANT_ID'] ?? serviceUserId, + providerId, + sessionId, + actorId, + tenantId, channelId: ingress.channelId, - correlationId: ingress.correlationId, + correlationId: this.discordRuntimeActionCorrelation( + binding.instanceId, + ingress, + providerId, + sessionId, + ), agentName: binding.instanceId, }); client.emit('discord:approval', { @@ -690,8 +700,8 @@ export class ChatGateway implements OnGatewayInit, OnGatewayConnection, OnGatewa if (!client.data.discordService) return; const ingress = this.resolveDiscordIngress(client, envelope, 'stop'); const actionParts = ingress?.content.match(/^\/stop\s+([^\s]+)\s+([^\s]+)\s+([^\s]+)$/i); - const serviceUserId = process.env['DISCORD_SERVICE_USER_ID']; - if (!ingress || !actionParts || !serviceUserId || !this.runtimeRegistry) return; + const tenantId = process.env['DISCORD_SERVICE_TENANT_ID']?.trim(); + if (!ingress || !actionParts || !tenantId || !this.runtimeRegistry) return; const binding = resolveDiscordInteractionBinding( parseDiscordInteractionBindings(process.env['DISCORD_INTERACTION_BINDINGS']), ingress.guildId, @@ -699,18 +709,26 @@ export class ChatGateway implements OnGatewayInit, OnGatewayConnection, OnGatewa ingress.userId, 'stop', ); - if (!binding) return; + const actorId = binding && resolveDiscordInteractionActorId(binding, ingress.userId); + if (!actorId) return; + const providerId = actionParts[1]!; + const sessionId = actionParts[2]!; try { - // RuntimeProviderService consumes the durable approval exactly once using this - // server-derived context before it invokes the provider termination. - await this.runtimeRegistry.terminate(actionParts[1]!, actionParts[2]!, actionParts[3]!, { + // RuntimeProviderService consumes the durable approval exactly once using the + // provisioned approving-admin identity, never the Discord service account. + await this.runtimeRegistry.terminate(providerId, sessionId, actionParts[3]!, { actorScope: { - userId: serviceUserId, - tenantId: process.env['DISCORD_SERVICE_TENANT_ID'] ?? serviceUserId, + userId: actorId, + tenantId, }, channelId: ingress.channelId, - correlationId: ingress.correlationId, + correlationId: this.discordRuntimeActionCorrelation( + binding.instanceId, + ingress, + providerId, + sessionId, + ), }); client.emit('discord:stop', { correlationId: ingress.correlationId, success: true }); } catch { @@ -718,6 +736,27 @@ export class ChatGateway implements OnGatewayInit, OnGatewayConnection, OnGatewa } } + /** + * Correlates the immutable termination target rather than either Discord message. + * Approval and stop are distinct ingress events, but must consume the same seven-field action. + */ + private discordRuntimeActionCorrelation( + instanceId: string, + ingress: DiscordIngressPayload, + providerId: string, + sessionId: string, + ): string { + const target = [ + instanceId, + ingress.guildId, + ingress.channelId, + ingress.conversationId, + providerId, + sessionId, + ]; + return `discord-action:v1:${createHash('sha256').update(JSON.stringify(target)).digest('hex')}`; + } + private resolveDiscordIngress( client: Socket, envelope: DiscordIngressEnvelope, diff --git a/apps/gateway/src/plugin/discord-ingress.security.spec.ts b/apps/gateway/src/plugin/discord-ingress.security.spec.ts index 438c52c..87fed51 100644 --- a/apps/gateway/src/plugin/discord-ingress.security.spec.ts +++ b/apps/gateway/src/plugin/discord-ingress.security.spec.ts @@ -4,6 +4,8 @@ import { verifyDiscordIngressEnvelope, DiscordPlugin, type DiscordIngressPayload, + parseDiscordInteractionBindings, + resolveDiscordInteractionActorId, resolveDiscordInteractionBinding, } from '@mosaicstack/discord-plugin'; import { ChatGateway } from '../chat/chat.gateway.js'; @@ -38,7 +40,12 @@ function configureDiscordEnv(role: 'admin' | 'member' = 'admin'): void { instanceId: 'Nova', guildId: 'guild-001', channelId: 'channel-001', - pairedUsers: { 'user-001': role === 'admin' ? 'admin' : 'operator' }, + pairedUsers: { + 'user-001': { + role: role === 'admin' ? 'admin' : 'operator', + mosaicUserId: 'mosaic-admin-001', + }, + }, }, ]); } @@ -68,8 +75,10 @@ function commandAuthorization(role: 'admin' | 'member'): CommandAuthorizationSer function discordGateway(role: 'admin' | 'member'): { gateway: ChatGateway; client: { data: { discordService: boolean }; emit: ReturnType }; + consumedActions: Array<{ actorId: string; correlationId: string }>; } { const authorization = commandAuthorization(role); + const consumedActions: Array<{ actorId: string; correlationId: string }> = []; const runtimeRegistry = { terminate: async ( providerId: string, @@ -81,6 +90,10 @@ function discordGateway(role: 'admin' | 'member'): { correlationId: string; }, ): Promise => { + consumedActions.push({ + actorId: context.actorScope.userId, + correlationId: context.correlationId, + }); const approved = await authorization.consumeRuntimeTerminationApproval(approvalId, { providerId, sessionId, @@ -105,14 +118,19 @@ function discordGateway(role: 'admin' | 'member'): { runtimeRegistry as never, ), client: { data: { discordService: true }, emit: vi.fn() }, + consumedActions, }; } function ingressEnvelope( content: string, messageId: string, + overrides: Partial = {}, ): ReturnType { - return createDiscordIngressEnvelope(createPayload({ content, messageId }), SERVICE_TOKEN); + return createDiscordIngressEnvelope( + createPayload({ content, messageId, ...overrides }), + SERVICE_TOKEN, + ); } function createPayload(overrides: Partial = {}): DiscordIngressPayload { @@ -129,6 +147,23 @@ function createPayload(overrides: Partial = {}): DiscordI } describe('Discord ingress security', () => { + it('keeps legacy role-only bindings valid while withholding privileged actor identity', () => { + const [binding] = parseDiscordInteractionBindings( + JSON.stringify([ + { + instanceId: 'Nova', + guildId: 'guild-001', + channelId: 'channel-001', + pairedUsers: { 'user-001': 'admin' }, + }, + ]), + ); + expect( + resolveDiscordInteractionBinding([binding!], 'guild-001', 'channel-001', 'user-001', 'send'), + ).toEqual(binding); + expect(resolveDiscordInteractionActorId(binding!, 'user-001')).toBeNull(); + }); + it('binds a differently named configured interaction instance without code changes', () => { const binding = resolveDiscordInteractionBinding( [ @@ -136,7 +171,7 @@ describe('Discord ingress security', () => { instanceId: 'Nova', guildId: 'guild-001', channelId: 'channel-001', - pairedUsers: { 'user-001': 'operator' }, + pairedUsers: { 'user-001': { role: 'operator', mosaicUserId: 'mosaic-operator-001' } }, }, ], 'guild-001', @@ -212,12 +247,14 @@ describe('Discord ingress security', () => { expect(replayProtector.size).toBe(2); }); - it('mints a Discord approval and consumes it for the exact stop action', async () => { + it('consumes the exact target once when approval and stop are separate Discord messages', async () => { configureDiscordEnv(); - const { gateway, client } = discordGateway('admin'); + const { gateway, client, consumedActions } = discordGateway('admin'); await gateway.handleDiscordApproval( client as never, - ingressEnvelope('/approve fleet runtime-1', 'approve-message'), + ingressEnvelope('/approve fleet runtime-1', 'approve-message', { + correlationId: 'approval-ingress-correlation', + }), ); const approval = client.emit.mock.calls.find( ([event]) => event === 'discord:approval', @@ -229,12 +266,20 @@ describe('Discord ingress security', () => { await gateway.handleDiscordStop( client as never, - ingressEnvelope(`/stop fleet runtime-1 ${approval.approvalId}`, 'stop-message'), + ingressEnvelope(`/stop fleet runtime-1 ${approval.approvalId}`, 'stop-message', { + correlationId: 'stop-ingress-correlation', + }), ); expect(client.emit).toHaveBeenCalledWith('discord:stop', { - correlationId: 'correlation-001', + correlationId: 'stop-ingress-correlation', success: true, }); + expect(consumedActions).toEqual([ + { + actorId: 'mosaic-admin-001', + correlationId: expect.stringMatching(/^discord-action:v1:/), + }, + ]); }); it('rejects unpaired and non-admin Discord users for approval and stop', async () => { @@ -264,7 +309,9 @@ describe('Discord ingress security', () => { const { gateway, client } = discordGateway('admin'); await gateway.handleDiscordApproval( client as never, - ingressEnvelope('/approve fleet runtime-1', 'replay-approve'), + ingressEnvelope('/approve fleet runtime-1', 'replay-approve', { + correlationId: 'replay-approval-correlation', + }), ); const approval = client.emit.mock.calls.find( ([event]) => event === 'discord:approval', @@ -273,11 +320,15 @@ describe('Discord ingress security', () => { }; await gateway.handleDiscordStop( client as never, - ingressEnvelope(`/stop fleet runtime-1 ${approval.approvalId}`, 'replay-stop-one'), + ingressEnvelope(`/stop fleet runtime-1 ${approval.approvalId}`, 'replay-stop-one', { + correlationId: 'replay-stop-correlation-one', + }), ); await gateway.handleDiscordStop( client as never, - ingressEnvelope(`/stop fleet runtime-1 ${approval.approvalId}`, 'replay-stop-two'), + ingressEnvelope(`/stop fleet runtime-1 ${approval.approvalId}`, 'replay-stop-two', { + correlationId: 'replay-stop-correlation-two', + }), ); const stopResults = client.emit.mock.calls.filter(([event]) => event === 'discord:stop'); expect(stopResults.map(([, result]) => (result as { success: boolean }).success)).toEqual([ @@ -300,7 +351,7 @@ describe('Discord ingress security', () => { instanceId: 'Nova', guildId: 'guild-001', channelId: 'channel-001', - pairedUsers: { 'user-001': 'operator' }, + pairedUsers: { 'user-001': { role: 'operator', mosaicUserId: 'mosaic-operator-001' } }, }, ], }); diff --git a/plugins/discord/src/index.ts b/plugins/discord/src/index.ts index 28d835d..d111e87 100644 --- a/plugins/discord/src/index.ts +++ b/plugins/discord/src/index.ts @@ -19,12 +19,22 @@ export interface DiscordPluginConfig { export type DiscordInteractionOperation = 'bind' | 'attach' | 'send' | 'approve' | 'stop'; export type DiscordInteractionRole = 'viewer' | 'operator' | 'admin'; +/** A provisioned Discord-to-Mosaic identity pairing. */ +export interface DiscordInteractionUserBinding { + role: DiscordInteractionRole; + /** Required for privileged approval and stop operations. */ + mosaicUserId?: string; +} + +/** Legacy role-only pairings remain valid for non-privileged Discord ingress. */ +export type DiscordInteractionPairing = DiscordInteractionRole | DiscordInteractionUserBinding; + export interface DiscordInteractionBinding { instanceId: string; guildId: string; channelId: string; /** Pairing roster keyed by Discord user ID. */ - pairedUsers: Readonly>; + pairedUsers: Readonly>; } const operationRoles: Readonly< @@ -49,10 +59,22 @@ export function resolveDiscordInteractionBinding( (candidate) => candidate.guildId === guildId && candidate.channelId === channelId, ); if (!binding) return null; - const role = binding.pairedUsers[userId]; + const pairing = binding.pairedUsers[userId]; + const role = typeof pairing === 'string' ? pairing : pairing?.role; return role && operationRoles[operation].includes(role) ? binding : null; } +/** Resolves the provisioned Mosaic identity for an already-authorized Discord user. */ +export function resolveDiscordInteractionActorId( + binding: DiscordInteractionBinding, + discordUserId: string, +): string | null { + const pairing = binding.pairedUsers[discordUserId]; + if (typeof pairing === 'string') return null; + const mosaicUserId = pairing?.mosaicUserId?.trim(); + return mosaicUserId || null; +} + /** Parses provisioned binding roster JSON and rejects malformed or empty data. */ export function parseDiscordInteractionBindings( value: string | undefined, @@ -75,11 +97,42 @@ export function parseDiscordInteractionBindings( ) { throw new Error('Invalid Discord interaction binding'); } + const pairedUsers = Object.fromEntries( + Object.entries(candidate.pairedUsers).map(([discordUserId, pairing]: [string, unknown]) => { + if (!discordUserId.trim()) { + throw new Error('Invalid Discord interaction user binding'); + } + if (typeof pairing === 'string') { + if (!['viewer', 'operator', 'admin'].includes(pairing)) { + throw new Error('Invalid Discord interaction user binding'); + } + return [discordUserId, pairing]; + } + if (typeof pairing !== 'object' || pairing === null) { + throw new Error('Invalid Discord interaction user binding'); + } + const userBinding = pairing as Partial; + if ( + !userBinding.role || + !['viewer', 'operator', 'admin'].includes(userBinding.role) || + (userBinding.mosaicUserId !== undefined && !userBinding.mosaicUserId.trim()) + ) { + throw new Error('Invalid Discord interaction user binding'); + } + return [ + discordUserId, + { + role: userBinding.role, + ...(userBinding.mosaicUserId ? { mosaicUserId: userBinding.mosaicUserId } : {}), + }, + ]; + }), + ) as Record; return { instanceId: candidate.instanceId, guildId: candidate.guildId, channelId: candidate.channelId, - pairedUsers: candidate.pairedUsers, + pairedUsers, }; }); }