From 20e36006bf063890f1b402c8ff5d234c7e057cc7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 11:23:57 +0000 Subject: [PATCH] Sandboxed HTTP honors cancellation (LOU-U17) sandboxHttpFetch now accepts an optional AbortSignal: an already-aborted signal rejects with an AbortError without running anything, an abort mid-flight rejects promptly, and the signal is forwarded to SandboxRunOptions.signal (NoopSandbox passes it to execFile so the child is killed). http.ts's sandbox transport and slack.ts's execute and sandboxExecute paths now pass the tool's abortSignal through. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016bY29ibQgv3MDdjAbUT4YR --- src/security/sandboxCore.ts | 3 + src/tools/built-in/http.ts | 2 +- src/tools/built-in/sandboxFetch.test.ts | 118 ++++++++++++++++++++++++ src/tools/built-in/sandboxFetch.ts | 43 ++++++++- src/tools/built-in/slack.ts | 32 ++++--- 5 files changed, 181 insertions(+), 17 deletions(-) create mode 100644 src/tools/built-in/sandboxFetch.test.ts diff --git a/src/security/sandboxCore.ts b/src/security/sandboxCore.ts index c28d790ce..b17eeb26b 100644 --- a/src/security/sandboxCore.ts +++ b/src/security/sandboxCore.ts @@ -35,6 +35,8 @@ export interface SandboxRunOptions { env?: Record; /** Timeout in milliseconds after which the command is killed. */ timeoutMs?: number; + /** Aborts the command (LOU-U17); `run()` then rejects with an `AbortError`. Adapters may ignore it. */ + signal?: AbortSignal; } /** @@ -79,6 +81,7 @@ export const NoopSandbox: SandboxAdapter = { cwd: opts.cwd, env: opts.env ? { ...process.env, ...opts.env } : process.env, timeout: opts.timeoutMs, + signal: opts.signal, }); return { stdout: stdout.toString(), stderr: stderr.toString(), exitCode: 0 }; } catch (error: any) { diff --git a/src/tools/built-in/http.ts b/src/tools/built-in/http.ts index df3d4b8a4..8804cadb1 100644 --- a/src/tools/built-in/http.ts +++ b/src/tools/built-in/http.ts @@ -198,7 +198,7 @@ function createSandboxTransport(sandbox: SandboxAdapter, validateSSL: boolean, t sandboxHttpFetch( sandbox, { url, method: init.method, headers: init.headers, body: init.body, insecureTLS: !validateSSL }, - { timeoutMs } + { timeoutMs, signal: init.signal } ); } diff --git a/src/tools/built-in/sandboxFetch.test.ts b/src/tools/built-in/sandboxFetch.test.ts new file mode 100644 index 000000000..74c2cb88a --- /dev/null +++ b/src/tools/built-in/sandboxFetch.test.ts @@ -0,0 +1,118 @@ +import { describe, it, expect, vi } from 'vitest'; +import type { SandboxAdapter, SandboxRunOptions, SandboxResult } from '../../security/sandboxCore'; +import { NoopSandbox } from '../../security/sandboxCore'; +import { sandboxHttpFetch } from './sandboxFetch'; +import { createHttpTool } from './http'; +import { createSlackTool } from './slack'; + +const REQUEST = { url: 'https://example.test/hook', method: 'GET' }; +const OK_RESULT: SandboxResult = { + stdout: JSON.stringify({ status: 200, statusText: 'OK', headers: {}, body: 'hi' }), + stderr: '', + exitCode: 0, +}; + +type RunArgs = [string, string[], SandboxRunOptions]; + +/** The options object `run` was called with the first time. */ +function firstRunOptions(run: { mock: { calls: unknown[][] } }): SandboxRunOptions { + return (run.mock.calls[0] as unknown as RunArgs)[2]; +} + +/** A sandbox whose run() never settles on its own (no network involved). */ +function hangingSandbox() { + const run = vi.fn(() => new Promise(() => undefined)); + const sandbox: SandboxAdapter = { name: 'hang', run, writeFile: async () => undefined }; + return { sandbox, run }; +} + +describe('sandboxHttpFetch cancellation (LOU-U17)', () => { + it('(a) an already-aborted signal rejects with an AbortError without running anything', async () => { + const { sandbox, run } = hangingSandbox(); + const controller = new AbortController(); + controller.abort(); + + await expect(sandboxHttpFetch(sandbox, REQUEST, { signal: controller.signal })).rejects.toMatchObject({ + name: 'AbortError', + }); + expect(run).not.toHaveBeenCalled(); + }); + + it('(b) aborting mid-flight rejects promptly with an AbortError', async () => { + const { sandbox, run } = hangingSandbox(); + const controller = new AbortController(); + const pending = sandboxHttpFetch(sandbox, REQUEST, { signal: controller.signal, timeoutMs: 60_000 }); + expect(run).toHaveBeenCalledTimes(1); + + const start = Date.now(); + setTimeout(() => controller.abort(), 20); + await expect(pending).rejects.toMatchObject({ name: 'AbortError' }); + expect(Date.now() - start).toBeLessThan(2000); + }); + + it('(c) the signal reaches sandbox.run() options', async () => { + const run = vi.fn(async () => OK_RESULT); + const controller = new AbortController(); + + const response = await sandboxHttpFetch({ name: 'spy', run, writeFile: async () => undefined }, REQUEST, { + signal: controller.signal, + }); + + expect(await response.text()).toBe('hi'); + expect(firstRunOptions(run).signal).toBe(controller.signal); + }); + + it('without a signal behaves as before', async () => { + const run = vi.fn(async () => OK_RESULT); + const response = await sandboxHttpFetch({ name: 'spy', run, writeFile: async () => undefined }, REQUEST); + expect(response.status).toBe(200); + }); + + it('NoopSandbox kills the child process when the signal aborts', async () => { + const controller = new AbortController(); + const pending = NoopSandbox.run('node', ['-e', 'setTimeout(() => {}, 30000)'], { signal: controller.signal }); + setTimeout(() => controller.abort(), 50); + await expect(pending).rejects.toMatchObject({ name: 'AbortError' }); + }); + + it('http tool sandboxExecute forwards its abortSignal to the sandbox', async () => { + const { sandbox, run } = hangingSandbox(); + const controller = new AbortController(); + const pending = createHttpTool().sandboxExecute!({ url: 'http://93.184.216.34/', method: 'GET' }, sandbox, { + abortSignal: controller.signal, + }); + await vi.waitFor(() => expect(run).toHaveBeenCalled()); + expect(firstRunOptions(run).signal).toBeDefined(); + + controller.abort(); + await expect(pending).rejects.toMatchObject({ name: 'AbortError' }); + }); + + it('slack tool sandboxExecute forwards its abortSignal to the sandbox', async () => { + const { sandbox, run } = hangingSandbox(); + const controller = new AbortController(); + const descriptor = createSlackTool({ webhookUrl: 'https://hooks.slack.test/services/mock' }); + const pending = descriptor.sandboxExecute!({ channel: '#c', message: 'm', approvalId: 'a' }, sandbox, { + abortSignal: controller.signal, + }); + await vi.waitFor(() => expect(run).toHaveBeenCalled()); + expect(firstRunOptions(run).signal).toBe(controller.signal); + + controller.abort(); + await expect(pending).rejects.toMatchObject({ name: 'AbortError' }); + }); + + it('slack tool execute passes its abortSignal to the fetch implementation', async () => { + const fetchImpl = vi.fn(async () => ({ ok: true, status: 200, text: async () => '' }) as Response); + const controller = new AbortController(); + await createSlackTool({ + webhookUrl: 'https://hooks.slack.test/services/mock', + fetchImpl: fetchImpl as unknown as typeof fetch, + }).tool.execute!( + { channel: '#c', message: 'm', approvalId: 'a' }, + { toolCallId: 't', messages: [], abortSignal: controller.signal } + ); + const init = (fetchImpl.mock.calls[0] as unknown as [string, RequestInit])[1]; + expect(init.signal).toBe(controller.signal); + }); +}); diff --git a/src/tools/built-in/sandboxFetch.ts b/src/tools/built-in/sandboxFetch.ts index c484e444e..c2d604ebb 100644 --- a/src/tools/built-in/sandboxFetch.ts +++ b/src/tools/built-in/sandboxFetch.ts @@ -79,6 +79,32 @@ if (req.insecureTLS) { export interface SandboxFetchOptions { /** Milliseconds before the sandboxed request is killed. Defaults to 30s. */ timeoutMs?: number; + /** + * Cancels the request (LOU-U17). An already-aborted signal rejects without + * running anything in the sandbox; an abort mid-flight rejects promptly + * with an `AbortError` and is also handed to `sandbox.run()` so an adapter + * that supports it can kill the sandboxed process. + */ + signal?: AbortSignal; +} + +/** The `AbortError`-shaped rejection reported when the caller cancels. */ +function sandboxAbortError(url: string): Error { + const error = new Error(`Sandboxed HTTP request to ${url} was aborted`); + error.name = 'AbortError'; + return error; +} + +/** Races `pending` against `signal`, rejecting with an AbortError as soon as it aborts. */ +function rejectOnAbort(pending: Promise, signal: AbortSignal | undefined, url: string): Promise { + if (!signal) { + return pending; + } + return new Promise((resolve, reject) => { + const onAbort = () => reject(sandboxAbortError(url)); + signal.addEventListener('abort', onAbort, { once: true }); + pending.then(resolve, reject).finally(() => signal.removeEventListener('abort', onAbort)); + }); } /** @@ -92,13 +118,22 @@ export async function sandboxHttpFetch( request: SandboxFetchRequest, options: SandboxFetchOptions = {} ): Promise { + const { signal } = options; + if (signal?.aborted) { + throw sandboxAbortError(request.url); + } const encoded = Buffer.from(JSON.stringify(request), 'utf-8').toString('base64'); const timeoutMs = options.timeoutMs ?? 30000; - const result = await sandbox.run('node', ['-e', SANDBOX_FETCH_SCRIPT], { - env: { SANDBOX_FETCH_REQUEST: encoded }, - timeoutMs, - }); + const result = await rejectOnAbort( + sandbox.run('node', ['-e', SANDBOX_FETCH_SCRIPT], { + env: { SANDBOX_FETCH_REQUEST: encoded }, + timeoutMs, + signal, + }), + signal, + request.url + ); if (result.exitCode !== 0) { throw new Error( diff --git a/src/tools/built-in/slack.ts b/src/tools/built-in/slack.ts index 477d11a3f..158a37678 100644 --- a/src/tools/built-in/slack.ts +++ b/src/tools/built-in/slack.ts @@ -103,7 +103,8 @@ export async function postSlackAlert( channel: string, message: string, approvalId: string, - options: SlackToolOptions = {} + options: SlackToolOptions = {}, + signal?: AbortSignal ): Promise<{ ok: boolean }> { const webhookUrl = options.webhookUrl ?? process.env[SLACK_WEBHOOK_URL_ENV_KEY]; if (!webhookUrl) { @@ -119,6 +120,7 @@ export async function postSlackAlert( method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(payload), + signal, }); if (!response.ok) { @@ -139,7 +141,8 @@ export async function postSlackAlertViaSandbox( message: string, approvalId: string, sandbox: SandboxAdapter, - options: SlackToolOptions = {} + options: SlackToolOptions = {}, + signal?: AbortSignal ): Promise<{ ok: boolean }> { const webhookUrl = options.webhookUrl ?? process.env[SLACK_WEBHOOK_URL_ENV_KEY]; if (!webhookUrl) { @@ -150,12 +153,16 @@ export async function postSlackAlertViaSandbox( const payload = buildSlackAlertPayload(channel, message, approvalId); - const response = await sandboxHttpFetch(sandbox, { - url: webhookUrl, - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(payload), - }); + const response = await sandboxHttpFetch( + sandbox, + { + url: webhookUrl, + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(payload), + }, + { signal } + ); if (!response.ok) { const errorText = await response.text(); @@ -182,8 +189,9 @@ export function createSlackTool(options: SlackToolOptions = {}): ToolDescriptor message: z.string().describe('Alert message text'), approvalId: z.string().describe('The pending approval id the "Fix it" button will resolve'), }), - execute: async ({ channel, message, approvalId }) => { - return postSlackAlert(channel, message, approvalId, options); + execute: async ({ channel, message, approvalId }, executeOptions) => { + // `?.`: direct callers have historically passed no options object. + return postSlackAlert(channel, message, approvalId, options, executeOptions?.abortSignal); }, }), // LOU-K2: this tool POSTs to a webhook URL read from an env var / @@ -193,9 +201,9 @@ export function createSlackTool(options: SlackToolOptions = {}): ToolDescriptor // above is left unchanged for direct callers (e.g. // examples/ops-pipeline, which calls descriptor.tool.execute() itself). requiresSandbox: true, - sandboxExecute: async (args, sandbox) => { + sandboxExecute: async (args, sandbox, callOptions) => { const { channel, message, approvalId } = args as { channel: string; message: string; approvalId: string }; - return postSlackAlertViaSandbox(channel, message, approvalId, sandbox, options); + return postSlackAlertViaSandbox(channel, message, approvalId, sandbox, options, callOptions?.abortSignal); }, }; }