SEC-768: fix(agent): check stopWhen before running auto-approved siblings of an approval-gated call

.changeset/stop-when-before-approval.md

@@ -0,0 +1,5 @@
+---
+"@openrouter/agent": patch
+---
+
+Security hardening: when `stopWhen` is already met on the first response and `allowFinalResponse` is `false`, pausing for an approval-gated tool call no longer executes its auto-approved sibling calls. They are saved as skipped results instead.

packages/agent/src/lib/model-result.ts

@@ -3240,13 +3240,16 @@ export class ModelResult<
    * @param toolCalls - The tool calls to check
    * @param currentRound - The current execution round (1-indexed)
    * @param currentResponse - The current response to save if pausing
+   * @param skipAutoExecution - When true, auto-approved calls are not executed;
+   *   they are saved as skipped results so the paused turn stays well-formed
    * @returns True if execution should pause for approval, false to continue
    * @throws Error if approval is required but no state accessor is configured
    */
   private async handleApprovalCheck(
     suppliedToolCalls: ParsedToolCall<Tool>[],
     currentRound: number,
     currentResponse: models.OpenResponsesResult,
+    skipAutoExecution = false,
   ): Promise<boolean> {
     if (!this.options.tools) {
       return false;
@@ -3311,10 +3314,19 @@ export class ModelResult<
       ...initialSurvivors.filter((call) => !pendingIds.has(call.id)),
       ...blockedCalls,
     ];
-    const unsentResults = await this.executeAutoApproveTools(
-      executableNow as ParsedToolCall<TTools[number]>[],
-      turnContext,
-    );
+    const unsentResults: UnsentToolResult<TTools>[] = skipAutoExecution
+      ? executableNow.map(
+          (call) =>
+            createRejectedResult(
+              call.id,
+              String(call.name),
+              'Tool execution skipped: stop condition met',
+            ) as UnsentToolResult<TTools>,
+        )
+      : await this.executeAutoApproveTools(
+          executableNow as ParsedToolCall<TTools[number]>[],
+          turnContext,
+        );
 
     // Combine pre-denied results (from hook "deny") with executed results.
     const combinedResults: UnsentToolResult<TTools>[] = [
@@ -6335,8 +6347,16 @@ export class ModelResult<
         // Extract and check tool calls
         const toolCalls = extractToolCallsFromResponse(currentResponse);
 
+        // With allowFinalResponse: false, a met stopWhen means no tool from
+        // this response runs (the post-loop stop path skips them). Evaluate it
+        // before the approval check so pausing for an approval-gated call does
+        // not execute its auto-approved siblings. With allowFinalResponse on,
+        // the stop path runs these calls anyway, so the check is not needed.
+        const skipAutoExecution =
+          this.options.allowFinalResponse === false && (await this.shouldStopExecution());
+
         // Check for approval requirements
-        if (await this.handleApprovalCheck(toolCalls, 0, currentResponse)) {
+        if (await this.handleApprovalCheck(toolCalls, 0, currentResponse, skipAutoExecution)) {
           return; // Paused for approval
         }
 

packages/agent/tests/unit/stop-when-before-approval.test.ts

@@ -0,0 +1,180 @@
+import type { OpenRouterCore } from '@openrouter/sdk/core';
+import type * as models from '@openrouter/sdk/models';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { z } from 'zod/v4';
+import { tool } from '../../src/lib/tool.js';
+import type { ConversationState, StateAccessor, Tool } from '../../src/lib/tool-types.js';
+
+const mockBetaResponsesSend = vi.hoisted(() => vi.fn());
+
+vi.mock('@openrouter/sdk/funcs/betaResponsesSend', () => ({
+  betaResponsesSend: mockBetaResponsesSend,
+}));
+
+// Import ModelResult AFTER vi.mock so the transport mock is wired in.
+const { ModelResult } = await import('../../src/lib/model-result.js');
+
+function makeResponse(output: models.OpenResponsesResult['output']): models.OpenResponsesResult {
+  return {
+    id: 'resp_1',
+    object: 'response',
+    createdAt: 0,
+    model: 'test-model',
+    status: 'completed',
+    completedAt: 0,
+    output,
+    error: null,
+    incompleteDetails: null,
+    temperature: null,
+    topP: null,
+    presencePenalty: null,
+    frequencyPenalty: null,
+    metadata: null,
+    instructions: null,
+    tools: [],
+    toolChoice: 'auto',
+    parallelToolCalls: false,
+  };
+}
+
+function functionCall(callId: string, name: string): models.OutputFunctionCallItem {
+  return {
+    type: 'function_call',
+    id: `fc_${callId}`,
+    callId,
+    name,
+    arguments: '{"v":"x"}',
+    status: 'completed',
+  };
+}
+
+function createMemoryAccessor<TTools extends readonly Tool[]>() {
+  let state: ConversationState<TTools> | null = null;
+  const accessor: StateAccessor<TTools> = {
+    load: async () => state,
+    save: async (next) => {
+      state = next;
+    },
+  };
+  return {
+    accessor,
+    get: () => state,
+  };
+}
+
+function makeTools() {
+  const autoExecute = vi.fn(async () => ({
+    ok: true,
+  }));
+  const gatedExecute = vi.fn(async () => ({
+    ok: true,
+  }));
+  const auto = tool({
+    name: 'auto',
+    inputSchema: z.object({
+      v: z.string(),
+    }),
+    outputSchema: z.object({
+      ok: z.boolean(),
+    }),
+    execute: autoExecute,
+  });
+  const gated = tool({
+    name: 'gated',
+    inputSchema: z.object({
+      v: z.string(),
+    }),
+    outputSchema: z.object({
+      ok: z.boolean(),
+    }),
+    requireApproval: true,
+    execute: gatedExecute,
+  });
+  return {
+    tools: [
+      auto,
+      gated,
+    ] as const,
+    autoExecute,
+    gatedExecute,
+  };
+}
+
+describe('stopWhen is evaluated before the approval check on the first response', () => {
+  beforeEach(() => {
+    mockBetaResponsesSend.mockReset();
+    mockBetaResponsesSend.mockResolvedValue({
+      ok: true,
+      value: makeResponse([
+        functionCall('call_auto', 'auto'),
+        functionCall('call_gated', 'gated'),
+      ]),
+    });
+  });
+
+  it('does not run an auto-executable sibling of an approval-gated call when stopWhen is already met', async () => {
+    const { tools, autoExecute, gatedExecute } = makeTools();
+    const memory = createMemoryAccessor<typeof tools>();
+    const result = new ModelResult<typeof tools>({
+      request: {
+        model: 'test-model',
+        input: 'go',
+      },
+      client: {} as OpenRouterCore,
+      tools,
+      state: memory.accessor,
+      stopWhen: () => true,
+      allowFinalResponse: false,
+    });
+
+    await result.getResponse();
+
+    expect(autoExecute).not.toHaveBeenCalled();
+    expect(gatedExecute).not.toHaveBeenCalled();
+    expect(mockBetaResponsesSend).toHaveBeenCalledTimes(1);
+
+    const saved = memory.get();
+    expect(saved?.status).toBe('awaiting_approval');
+    expect(saved?.pendingToolCalls?.map((tc) => tc.id)).toEqual([
+      'call_gated',
+    ]);
+    // The skipped sibling still gets a matching output for the resume request.
+    expect(saved?.unsentToolResults).toEqual([
+      {
+        callId: 'call_auto',
+        name: 'auto',
+        output: null,
+        error: 'Tool execution skipped: stop condition met',
+      },
+    ]);
+    expect(await result.requiresApproval()).toBe(true);
+  });
+
+  it('still runs the auto-executable sibling when stopWhen is not met', async () => {
+    const { tools, autoExecute } = makeTools();
+    const memory = createMemoryAccessor<typeof tools>();
+    const result = new ModelResult<typeof tools>({
+      request: {
+        model: 'test-model',
+        input: 'go',
+      },
+      client: {} as OpenRouterCore,
+      tools,
+      state: memory.accessor,
+      stopWhen: () => false,
+      allowFinalResponse: false,
+    });
+
+    await result.getResponse();
+
+    expect(autoExecute).toHaveBeenCalledTimes(1);
+    const saved = memory.get();
+    expect(saved?.status).toBe('awaiting_approval');
+    expect(saved?.pendingToolCalls?.map((tc) => tc.id)).toEqual([
+      'call_gated',
+    ]);
+    expect(saved?.unsentToolResults?.map((r) => r.callId)).toEqual([
+      'call_auto',
+    ]);
+  });
+});