fix: channel approval buttons are not authorized to a specific user (known issue #11)

apps/workers/src/jobs/claw-gateway-run/handler.ts

@@ -18,7 +18,7 @@ import { clawGatewayRunSchema } from './schema.js';
 const log = createLogger('claw-gateway-run');
 
 export async function handleClawGatewayRun(data: unknown): Promise<void> {
-  const { runId, tenantId, action, content } = clawGatewayRunSchema.parse(data);
+  const { runId, tenantId, action, content, approvedBy } = clawGatewayRunSchema.parse(data);
 
   try {
     const { executeRun, terminateRun, grantPendingToolsForRun } = await import('@chatbot/claw-studio');
@@ -35,15 +35,20 @@ export async function handleClawGatewayRun(data: unknown): Promise<void> {
 
     if (action === 'reject' || action === 'cancel') {
       log.info({ runId, tenantId, action }, 'Terminating run');
-      await terminateRun({ runId, deps, kind: action === 'reject' ? 'reject' : 'cancel' });
+      await terminateRun({
+        runId,
+        deps,
+        kind: action === 'reject' ? 'reject' : 'cancel',
+        approvedBy: action === 'reject' ? approvedBy : undefined,
+      });
       return;
     }
 
     log.info({ runId, tenantId, resume: action ?? null }, 'Executing gateway run');
     await executeRun({
       runId,
       deps,
-      resume: action ? { action, content } : undefined,
+      resume: action ? { action, content, approvedBy } : undefined,
     });
     log.info({ runId, tenantId }, 'Gateway run handler finished');
   } catch (error) {

apps/workers/src/jobs/claw-gateway-run/schema.ts

@@ -18,6 +18,8 @@ export const clawGatewayRunSchema = z.object({
   ]).optional(),
   /** The user's reply text, for clarification_response. */
   content: z.string().optional(),
+  /** The platform user id that made this decision, when it came from a channel. */
+  approvedBy: z.string().optional(),
 });
 
 export type ClawGatewayRunData = z.infer<typeof clawGatewayRunSchema>;

libs/claw-studio/src/connectors/types.ts

@@ -32,6 +32,18 @@ export interface HilCapabilities {
 
 export interface BaseConnectorConfig {
   enabled: boolean;
+  /**
+   * Platform user ids allowed to act on this channel's approval affordances
+   * (Approve / Reject / "Always allow for this task"). Not a secret — these
+   * are public platform identifiers, not credentials.
+   *
+   * Unset or empty means no allowlist is configured yet. See the gateway's
+   * `handleResume` for the resulting default-deny behavior on
+   * `approve_always`, which grants a standing permission and is too
+   * consequential to leave open to any workspace member by default
+   * (docs/superpowers/claw-studio-known-issues.md #11).
+   */
+  approverIds?: string[];
 }
 
 export interface SlackConnectorConfig extends BaseConnectorConfig {

libs/claw-studio/src/gateway/execute-run.ts

@@ -45,7 +45,7 @@ const logger = createLogger('claw-studio:gateway:execute');
 export interface ExecuteRunInput {
   runId: string;
   /** Present when this is a HIL resume rather than a first execution. */
-  resume?: { action: Exclude<ReplyAction, 'reject'>; content?: string };
+  resume?: { action: Exclude<ReplyAction, 'reject'>; content?: string; approvedBy?: string };
   deps: RouterDeps;
   /**
    * Per-run runtime shaping. Optional and defaulted, so channel runs are unchanged;
@@ -432,9 +432,12 @@ export async function executeRun(input: ExecuteRunInput): Promise<void> {
       graphInput = new Command({
         resume: { decisions: Array.from({ length: decisionCount }, () => ({ type: 'approve' as const })) },
       });
+      const approvedSuffix = resume.approvedBy ? ` by ${resume.approvedBy}` : '';
       await runs.appendEvent(run, {
         eventType: 'approval_decision',
-        content: resume.action === 'approve_always' ? 'approved (always, for this task)' : 'approved',
+        content:
+          (resume.action === 'approve_always' ? 'approved (always, for this task)' : 'approved') +
+          approvedSuffix,
       });
     } else {
       graphInput = { messages: [new HumanMessage(resume.content ?? '')] };
@@ -570,6 +573,8 @@ export async function terminateRun(input: {
   runId: string;
   deps: RouterDeps;
   kind: 'reject' | 'cancel';
+  /** Platform user id that rejected, when `kind` is `'reject'` and it came from a channel. */
+  approvedBy?: string;
 }): Promise<void> {
   const runs = getRunService();
   const bus = getRunEventBus();
@@ -595,7 +600,11 @@ export async function terminateRun(input: {
   const unsubscribe = adapter ? attachToRun(run, adapter, input.deps) : () => {};
   try {
     if (rejected) {
-      await runs.appendEvent(run, { eventType: 'approval_decision', content: 'rejected' });
+      const approvedSuffix = input.approvedBy ? ` by ${input.approvedBy}` : '';
+      await runs.appendEvent(run, {
+        eventType: 'approval_decision',
+        content: `rejected${approvedSuffix}`,
+      });
     } else {
       await runs.appendEvent(run, { eventType: 'status', content: 'cancelled' });
     }

libs/claw-studio/src/gateway/gateway-service.test.ts

@@ -1,6 +1,6 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 
-const { mockRegistry, mockRunService, adapter } = vi.hoisted(() => {
+const { mockRegistry, mockRunService, mockConnectorConfigService, adapter } = vi.hoisted(() => {
   const adapter = {
     channelType: 'slack',
     displayName: 'Slack',
@@ -26,6 +26,7 @@ const { mockRegistry, mockRunService, adapter } = vi.hoisted(() => {
       list: vi.fn(() => [adapter]),
     },
     mockRunService: { create: vi.fn(), get: vi.fn() },
+    mockConnectorConfigService: { getRaw: vi.fn() },
   };
 });
 
@@ -35,6 +36,12 @@ vi.mock('@chatbot/shared', () => ({
 
 vi.mock('../connectors/registry', () => ({ getConnectorRegistry: () => mockRegistry }));
 vi.mock('./run-service', () => ({ getRunService: () => mockRunService }));
+// Default (no allowlist configured) preserves the pre-existing permissive
+// behavior for plain approve/reject; approve_always is covered explicitly
+// below since it default-denies with no allowlist regardless.
+vi.mock('../connectors/config-service', () => ({
+  ClawConnectorConfigService: vi.fn(() => mockConnectorConfigService),
+}));
 
 import { handleInbound } from './gateway-service';
 
@@ -66,6 +73,8 @@ beforeEach(() => {
   adapter.getConfig.mockResolvedValue({ enabled: true });
   adapter.parseInbound.mockResolvedValue(newRunMessage);
   mockRunService.create.mockResolvedValue(createdRun);
+  // No allowlist configured by default.
+  mockConnectorConfigService.getRaw.mockResolvedValue(null);
 });
 
 describe('handleInbound', () => {
@@ -114,6 +123,8 @@ describe('handleInbound', () => {
     // Enqueue before ack: acking first would promise the user a run that might
     // never be queued.
     expect(order).toEqual(['create', 'enqueue', 'ack']);
+    // A fresh run has no approver decision to attribute — approvedBy is
+    // only threaded through on a HIL resume.
     expect(enqueue).toHaveBeenCalledWith({ runId: 'run_new', tenantId: 'tenant1' });
   });
 
@@ -194,6 +205,7 @@ describe('handleInbound — HIL resume', () => {
       tenantId: 'tenant1',
       action: 'approve',
       content: undefined,
+      approvedBy: 'U1',
     });
     // A resume must never create a second run.
     expect(mockRunService.create).not.toHaveBeenCalled();
@@ -270,6 +282,7 @@ describe('handleInbound — HIL resume', () => {
       tenantId: 'tenant1',
       action: 'clarification_response',
       content: 'the blue one',
+      approvedBy: 'U1',
     });
   });
 
@@ -295,3 +308,92 @@ describe('handleInbound — HIL resume', () => {
     expect(enqueue).not.toHaveBeenCalled();
   });
 });
+
+describe('handleInbound — HIL resume — approver allowlist', () => {
+  const replyMessage = {
+    ...newRunMessage,
+    taskDescription: '',
+    replyContext: { runId: 'run_existing', action: 'approve' as const, tenantId: 'tenant1' },
+  };
+
+  beforeEach(() => {
+    adapter.parseInbound.mockResolvedValue(replyMessage);
+    mockRunService.get.mockResolvedValue({
+      runId: 'run_existing',
+      tenantId: 'tenant1',
+      status: 'awaiting_approval',
+    });
+  });
+
+  it('ignores an approve from a user who is not on the configured allowlist', async () => {
+    mockConnectorConfigService.getRaw.mockResolvedValue({ approverIds: ['someone-else'] });
+
+    const res = await handleInbound('slack', req(), deps);
+
+    expect(res.status).toBe(200);
+    expect(enqueue).not.toHaveBeenCalled();
+  });
+
+  it('queues an approve from a user who is on the configured allowlist', async () => {
+    mockConnectorConfigService.getRaw.mockResolvedValue({ approverIds: ['U1', 'U2'] });
+
+    const res = await handleInbound('slack', req(), deps);
+
+    expect(res.status).toBe(200);
+    expect(enqueue).toHaveBeenCalledWith(
+      expect.objectContaining({ runId: 'run_existing', action: 'approve', approvedBy: 'U1' }),
+    );
+  });
+
+  it('queues a plain approve with no allowlist configured — unchanged back-compat behavior', async () => {
+    mockConnectorConfigService.getRaw.mockResolvedValue(null);
+
+    const res = await handleInbound('slack', req(), deps);
+
+    expect(res.status).toBe(200);
+    expect(enqueue).toHaveBeenCalledWith(
+      expect.objectContaining({ runId: 'run_existing', action: 'approve', approvedBy: 'U1' }),
+    );
+  });
+
+  it('ignores approve_always with no allowlist configured — a standing grant needs one', async () => {
+    mockConnectorConfigService.getRaw.mockResolvedValue(null);
+    adapter.parseInbound.mockResolvedValue({
+      ...replyMessage,
+      replyContext: { ...replyMessage.replyContext, action: 'approve_always' as const },
+    });
+
+    const res = await handleInbound('slack', req(), deps);
+
+    expect(res.status).toBe(200);
+    expect(enqueue).not.toHaveBeenCalled();
+  });
+
+  it('queues approve_always once an allowlist is configured and the user is on it', async () => {
+    mockConnectorConfigService.getRaw.mockResolvedValue({ approverIds: ['U1'] });
+    adapter.parseInbound.mockResolvedValue({
+      ...replyMessage,
+      replyContext: { ...replyMessage.replyContext, action: 'approve_always' as const },
+    });
+
+    const res = await handleInbound('slack', req(), deps);
+
+    expect(res.status).toBe(200);
+    expect(enqueue).toHaveBeenCalledWith(
+      expect.objectContaining({ action: 'approve_always', approvedBy: 'U1' }),
+    );
+  });
+
+  it('ignores a reject from a user who is not on the configured allowlist', async () => {
+    mockConnectorConfigService.getRaw.mockResolvedValue({ approverIds: ['someone-else'] });
+    adapter.parseInbound.mockResolvedValue({
+      ...replyMessage,
+      replyContext: { ...replyMessage.replyContext, action: 'reject' as const },
+    });
+
+    const res = await handleInbound('slack', req(), deps);
+
+    expect(res.status).toBe(200);
+    expect(enqueue).not.toHaveBeenCalled();
+  });
+});

libs/claw-studio/src/gateway/gateway-service.ts

@@ -12,6 +12,7 @@
 
 import { createLogger } from '@chatbot/shared';
 import { getConnectorRegistry } from '../connectors/registry';
+import { ClawConnectorConfigService } from '../connectors/config-service';
 import { getRunService } from './run-service';
 import { isTerminalStatus, type ChannelAdapter, type GatewayMessage, type ReplyAction } from './types';
 import type { ChannelType } from '../connectors/types';
@@ -40,6 +41,13 @@ export interface GatewayJobPayload {
   /** `cancel` is dashboard-only; channels only ever send the three ReplyActions. */
   action?: ReplyAction | 'cancel';
   content?: string;
+  /**
+   * The platform user id that made this decision, when it came from a
+   * channel. Threaded through to the run's `approval_decision` event so a
+   * standing grant (`approve_always`) is attributable to a person, not just
+   * to "someone in the workspace" (known-issues #11/#13).
+   */
+  approvedBy?: string;
 }
 
 export type EnqueueRunFn = (payload: GatewayJobPayload) => Promise<void>;
@@ -196,11 +204,46 @@ async function handleResume(
     return json({ error: 'Reply was empty.' }, 400);
   }
 
+  // Approve/reject/approve_always come from a channel button or thread reply —
+  // the signature check above proves the request came from the right
+  // *workspace*, not that the clicker is anyone in particular. Without this,
+  // any member who can see the approval message can press it.
+  // (docs/superpowers/claw-studio-known-issues.md #11)
+  if (reply.action === 'approve' || reply.action === 'approve_always' || reply.action === 'reject') {
+    const connectorConfig = await new ClawConnectorConfigService(run.tenantId).getRaw(
+      message.channelType,
+    );
+    const approverIds = connectorConfig?.approverIds ?? [];
+    const isAllowlisted =
+      approverIds.length > 0 && !!message.userId && approverIds.includes(message.userId);
+
+    if (approverIds.length > 0 && !isAllowlisted) {
+      logger.warn(
+        { runId: run.runId, channel: adapter.channelType, userId: message.userId },
+        'Resume action from a user not on the channel approver allowlist — ignoring',
+      );
+      return await adapter.sendAck(req, run.runId);
+    }
+
+    // approve_always grants a STANDING permission on the task (not a one-time
+    // action), so trusting "anyone in the workspace" by default is too
+    // consequential — require an explicit allowlist before it can be used at
+    // all, same as the known-issues register's own stopgap suggestion.
+    if (approverIds.length === 0 && reply.action === 'approve_always') {
+      logger.warn(
+        { runId: run.runId, channel: adapter.channelType },
+        'approve_always requested with no approver allowlist configured for this channel — ignoring',
+      );
+      return await adapter.sendAck(req, run.runId);
+    }
+  }
+
   await deps.enqueue({
     runId: run.runId,
     tenantId: run.tenantId,
     action: reply.action,
     content: reply.content,
+    approvedBy: message.userId,
   });
 
   logger.info(