fix(discord): bind stops to approving admin

2026-07-13 03:36:18 -05:00
parent 6af68e76de
commit 318a7bdda8
3 changed files with 175 additions and 32 deletions
--- 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,
--- 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<typeof vi.fn> };
+  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<void> => {
+      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<DiscordIngressPayload> = {},
 ): ReturnType<typeof createDiscordIngressEnvelope> {
-  return createDiscordIngressEnvelope(createPayload({ content, messageId }), SERVICE_TOKEN);
+  return createDiscordIngressEnvelope(
+    createPayload({ content, messageId, ...overrides }),
+    SERVICE_TOKEN,
+  );
 }

 function createPayload(overrides: Partial<DiscordIngressPayload> = {}): DiscordIngressPayload {
@@ -129,6 +147,23 @@ function createPayload(overrides: Partial<DiscordIngressPayload> = {}): 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' } },
        },
      ],
    });