diff --git a/README.md b/README.md index b7961695..155bb642 100644 --- a/README.md +++ b/README.md @@ -1177,6 +1177,8 @@ npm install -g context-mode Each `ctx_execute` call spawns an isolated subprocess with its own process boundary. Scripts can't access each other's memory or state. The subprocess runs your code, captures stdout, and only that stdout enters the conversation context. The raw data — log files, API responses, snapshots — never leaves the sandbox. +Host-driven cancellation is supported end to end: when the MCP client aborts a request, the abort signal reaches `ctx_execute`/`ctx_execute_file` and kills the entire spawned process tree (children and grandchildren — via the dedicated process group on Unix, `taskkill /T` on Windows), never a broad name/port sweep. The per-call `.ctx-mode-*` temp directory is removed afterward, including its `ownership.json` sidecar — a metadata-only manifest (nonce, script path/hash, PIDs, language) that deliberately never contains the executed code, command, cwd, or environment. + Twelve language runtimes are available: JavaScript, TypeScript, Python, Shell, Ruby, Go, Rust, PHP, Perl, R, Elixir, and C#. Bun is auto-detected for 3-5x faster JS/TS execution. Authenticated CLIs work through credential passthrough — `gh`, `aws`, `gcloud`, `kubectl`, `docker` inherit environment variables and config paths without exposing them to the conversation. diff --git a/src/executor.ts b/src/executor.ts index 62af2e6e..325b82f8 100644 --- a/src/executor.ts +++ b/src/executor.ts @@ -1,7 +1,8 @@ import { spawn, execSync, execFileSync } from "node:child_process"; -import { mkdtempSync, writeFileSync, rmSync, existsSync } from "node:fs"; +import { mkdtempSync, writeFileSync, rmSync, existsSync, realpathSync, readFileSync } from "node:fs"; import { join, resolve } from "node:path"; import { tmpdir } from "node:os"; +import { createHash, randomBytes } from "node:crypto"; import { detectRuntimes, buildCommand, @@ -110,6 +111,13 @@ const OS_TMPDIR = (() => { return "/tmp"; })(); +/** + * Grace window (ms) after an abort's tree kill before the execution settles + * deterministically — see the abort handler in #spawn. Bounds abort latency + * when `close` is held up by inherited stdio handles (Windows). + */ +const ABORT_SETTLE_GRACE_MS = 1500; + /** * Pure helper — exported for unit testing. Issue #782. * @@ -222,6 +230,8 @@ interface ExecuteOptions { * a non-project cwd (e.g. $HOME). */ cwd?: string; + /** Request-scoped cancellation. Never cancels other executions. */ + signal?: AbortSignal; } interface ExecuteFileOptions extends ExecuteOptions { @@ -280,11 +290,27 @@ export class PolyglotExecutor { } async execute(opts: ExecuteOptions): Promise { - const { language, code, timeout, background = false, cwd: cwdOverride } = opts; + const { language, code, timeout, background = false, cwd: cwdOverride, signal } = opts; + + // Deterministic pre-abort path: never spawn a process for a request that + // is already cancelled. Spawn-then-kill-immediately races on Windows + // (taskkill can miss a just-created process; even on a clean kill `close` + // can stall on inherited pipe handles), so resolve synchronously instead. + // No temp dir created → nothing to clean up, no listener to remove. + if (signal?.aborted) { + return { + stdout: "", + stderr: "Execution aborted before start", + exitCode: 1, + timedOut: false, + }; + } + const tmpDir = mkdtempSync(join(OS_TMPDIR, ".ctx-mode-")); try { const filePath = this.#writeScript(tmpDir, code, language); + this.#writeOwnershipManifest(tmpDir, filePath, language); const cmd = buildCommand(this.#runtimes, language, filePath); // Rust: compile then run @@ -304,7 +330,7 @@ export class PolyglotExecutor { // Issue #45 — `cwdOverride` lets per-call sites (Codex MCP handlers) pin // cwd without mutating process-wide state. const cwd = cwdOverride ?? this.#projectRoot; - const result = await this.#spawn(cmd, cwd, tmpDir, timeout, background); + const result = await this.#spawn(cmd, cwd, tmpDir, timeout, background, signal); // Skip tmpDir cleanup if process was backgrounded — it may still need files if (!result.backgrounded) { @@ -319,14 +345,29 @@ export class PolyglotExecutor { } async executeFile(opts: ExecuteFileOptions): Promise { - const { path: filePath, language, code, timeout } = opts; + const { path: filePath, language, code, timeout, signal } = opts; const absolutePath = resolve(this.#projectRoot, filePath); const wrappedCode = this.#wrapWithFileContent( absolutePath, language, code, ); - return this.execute({ language, code: wrappedCode, timeout }); + return this.execute({ language, code: wrappedCode, timeout, signal }); + } + + #writeOwnershipManifest(tmpDir: string, scriptPath: string, language: Language): void { + // Sidecar intentionally excludes code, command, cwd, and environment so it + // remains safe to retain for a future ownership-verified orphan reaper. + writeFileSync(join(tmpDir, "ownership.json"), JSON.stringify({ + version: 1, + nonce: randomBytes(16).toString("hex"), + scriptPath: realpathSync(scriptPath), + scriptSha256: createHash("sha256").update(readFileSync(scriptPath)).digest("hex"), + createdAt: new Date().toISOString(), + executorPid: process.pid, + parentPid: process.ppid, + language, + }) + "\n", { encoding: "utf-8", mode: 0o600 }); } #writeScript(tmpDir: string, code: string, language: Language): string { @@ -412,6 +453,7 @@ export class PolyglotExecutor { sandboxTmpDir: string, timeout: number | undefined, background = false, + signal?: AbortSignal, ): Promise { return new Promise((res) => { // Only .cmd/.bat shims need shell on Windows; real executables don't. @@ -467,6 +509,36 @@ export class PolyglotExecutor { let timedOut = false; let resolved = false; + let abortSettleTimer: NodeJS.Timeout | undefined; + const abort = () => { + // Exact spawned root only: killTree uses its dedicated process group or + // taskkill /T, never a broad name/port sweep. + if (resolved) return; + killTree(proc); + // Deterministic settle: after the tree kill we do NOT wait forever for + // `close`. Descendants that die slowly (or briefly survive) keep + // inherited stdio pipe handles open — on Windows that can delay the + // root's `close` well past the actual kill. Give real output a short + // grace window, then settle with whatever we captured. + abortSettleTimer = setTimeout(() => { + if (resolved) return; + resolved = true; + clearTimeout(timer); + signal?.removeEventListener("abort", abort); + proc.stdout?.removeAllListeners("data"); + proc.stderr?.removeAllListeners("data"); + try { proc.kill("SIGKILL"); } catch { /* already gone */ } + res({ + stdout: Buffer.concat(stdoutChunks).toString("utf-8"), + stderr: Buffer.concat(stderrChunks).toString("utf-8"), + exitCode: 1, + timedOut: false, + }); + }, ABORT_SETTLE_GRACE_MS); + abortSettleTimer.unref?.(); + }; + if (signal?.aborted) abort(); + else signal?.addEventListener("abort", abort, { once: true }); // Issue #406 — if the caller didn't pass a timeout we don't fire one. // Timeout policy belongs to the MCP host/client (Claude Code, VSCode, // JetBrains all enforce their own RPC timeouts); imposing a second @@ -538,6 +610,8 @@ export class PolyglotExecutor { proc.on("close", (exitCode) => { clearTimeout(timer); + if (abortSettleTimer) clearTimeout(abortSettleTimer); + signal?.removeEventListener("abort", abort); if (resolved) return; // Already resolved by background timeout const rawStdout = Buffer.concat(stdoutChunks).toString("utf-8"); let rawStderr = Buffer.concat(stderrChunks).toString("utf-8"); @@ -559,6 +633,8 @@ export class PolyglotExecutor { proc.on("error", (err) => { clearTimeout(timer); + if (abortSettleTimer) clearTimeout(abortSettleTimer); + signal?.removeEventListener("abort", abort); if (resolved) return; // Already resolved by background timeout res({ stdout: "", diff --git a/src/server.ts b/src/server.ts index 5a884c20..86cf763d 100644 --- a/src/server.ts +++ b/src/server.ts @@ -279,7 +279,7 @@ const originalRegisterTool = server.registerTool.bind(server); const [name, config, handler] = args as [ string, Record, - (toolArgs: Record) => Promise | unknown, + (toolArgs: Record, extra?: unknown) => Promise | unknown, ]; if (suppressMcpToolsForNativePluginHost) { emitSuppressionDiagnostic(); @@ -293,15 +293,19 @@ const originalRegisterTool = server.registerTool.bind(server); function wrapToolHandler( name: string, - handler: (toolArgs: Record) => Promise | unknown, -): (toolArgs: Record) => Promise { - return async (toolArgs: Record) => { + handler: (toolArgs: Record, extra?: unknown) => Promise | unknown, +): (toolArgs: Record, extra?: unknown) => Promise { + return async (toolArgs: Record, extra?: unknown) => { // #854: mark a tool call in-flight so the bridge-child idle reaper never // shuts the server down mid-execution during a long ctx_execute/batch that // emits no further inbound messages. Symmetric end in finally (success+error). noteRequestStart(); try { - return await handler(toolArgs); + // The MCP SDK invokes registered tool handlers with (args, extra), where + // extra.signal carries request cancellation from the host. Forward it — + // dropping it silently disabled the executor-level abort wiring (#83aefee): + // every handler saw extra === {} so no running execution could be cancelled. + return await handler(toolArgs, extra); } catch (err) { const result = storageErrorResult(err); if (result) { @@ -1740,7 +1744,7 @@ EXAMPLE: ctx_execute(language: "javascript", code: "const out = require('child_p ), }), }, - async ({ language, code, timeout, background, cwd, intent }) => { + async ({ language, code, timeout, background, cwd, intent }, extra: { signal?: AbortSignal } = {}) => { // Security: deny-only firewall if (language === "shell") { const denied = checkDenyPolicy(code, "execute"); @@ -1819,7 +1823,7 @@ __cm_main().catch(e=>{console.error(e);process.exitCode=1});${background ? '\nse })(typeof require!=='undefined'?require:null);`; } const effTimeout = resolveExecTimeout(timeout); - const result = await executor.execute({ language, code: instrumentedCode, timeout: effTimeout, background, cwd }); + const result = await executor.execute({ language, code: instrumentedCode, timeout: effTimeout, background, cwd, signal: extra.signal }); // Echo the executed source code before stdout so users can audit // and tooling can block command patterns (Issues #717 + #736). @@ -2111,7 +2115,7 @@ EXAMPLE: ctx_execute_file(path: "data.csv", language: "javascript", code: "const ), }), }, - async ({ path, language, code, timeout, intent }) => { + async ({ path, language, code, timeout, intent }, extra: { signal?: AbortSignal } = {}) => { // Security (#852): confine the processed file to the project root so // ctx_execute_file cannot be used to escape the host's sandbox/permission // controls. Runs before the deny-glob check — boundary first, then policy. @@ -2138,6 +2142,7 @@ EXAMPLE: ctx_execute_file(path: "data.csv", language: "javascript", code: "const language, code, timeout: effTimeout, + signal: extra.signal, }); // Echo path + executed source code before stdout for audit/debug diff --git a/tests/executor.test.ts b/tests/executor.test.ts index e8320f6d..0e139b96 100644 --- a/tests/executor.test.ts +++ b/tests/executor.test.ts @@ -1,6 +1,6 @@ import { describe, test, expect, afterAll } from "vitest"; import { strict as assert } from "node:assert"; -import { existsSync, writeFileSync, mkdirSync, rmSync } from "node:fs"; +import { existsSync, writeFileSync, mkdirSync, rmSync, mkdtempSync, readFileSync, readdirSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { @@ -2082,3 +2082,306 @@ describe("killSiblingMcpServers (#559)", () => { assert.equal(report.totalKilled, 0); }); }); + +// --------------------------------------------------------------------------- +// AbortSignal cancellation — regression tests for the #83aefee fix +// (request-scoped process-tree kill + temp dir / ownership sidecar cleanup). +// +// Windows/Git Bash notes that shape these tests: +// - bash $$ / $! are MSYS-internal PIDs, invisible to Node's process.kill and +// to taskkill — so tree descendants are native node processes, whose PIDs +// are real OS PIDs on both platforms. +// - Neither $TMPDIR (MSYS rewrites it to its own /tmp view) nor dirname $0 +// ($0 is /usr/bin/bash under the executor's spawn form) identifies the +// sandbox dir, so it is located by scanning OS temp for a .ctx-mode-* dir +// whose ownership.json manifest carries THIS process's pid (tests run +// sequentially, so at most one live sandbox matches). +// --------------------------------------------------------------------------- + +describe("AbortSignal cancellation (process tree + temp cleanup)", () => { + const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + const MARKER_ENV = "__CM_ABORT_TEST_MARKER"; + + function isAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } + } + + async function waitDead(pid: number, deadlineMs = 3000): Promise { + const start = Date.now(); + while (Date.now() - start < deadlineMs) { + if (!isAlive(pid)) return; + await sleep(50); + } + } + + async function waitFor( + predicate: () => boolean, + timeoutMs = 10_000, + what = "condition", + ): Promise { + const start = Date.now(); + while (Date.now() - start < timeoutMs) { + if (predicate()) return; + await sleep(50); + } + throw new Error(`Timed out after ${timeoutMs}ms waiting for ${what}`); + } + + // Locate the live per-call .ctx-mode-* sandbox dir for THIS process via its + // ownership manifest (executorPid filter; newest wins if a stale dir lingers + // from a crashed earlier test). Mirrors the executor's OS_TMPDIR choice. + function findSandboxDir(): string | null { + const base = process.platform === "win32" + ? (process.env.TEMP ?? process.env.TMP ?? tmpdir()) + : tmpdir(); + let best: { dir: string; createdAt: string } | null = null; + for (const entry of readdirSync(base)) { + if (!entry.startsWith(".ctx-mode-")) continue; + const dir = join(base, entry); + try { + const manifest = JSON.parse(readFileSync(join(dir, "ownership.json"), "utf-8")); + if (manifest.executorPid !== process.pid) continue; + if (!best || String(manifest.createdAt) > best.createdAt) best = { dir, createdAt: String(manifest.createdAt) }; + } catch { /* not ours, or mid-cleanup */ } + } + return best?.dir ?? null; + } + + test("abort kills the entire shell tree (child + grandchild) and cleans the .ctx-mode temp dir", async () => { + const scratch = mkdtempSync(join(tmpdir(), "ctx-abort-test-")); + const marker = join(scratch, "marker"); + process.env[MARKER_ENV] = marker; + process.env.__CM_ABORT_NODE = process.execPath; + // Child code is passed through the environment (eval'd inside node) to + // avoid shell-quoting collisions; the child spawns the grandchild and + // reports both REAL OS PIDs to the marker. + process.env.__CM_ABORT_CHILD_CODE = [ + "const cp = require('child_process');", + "const fs = require('fs');", + "const g = cp.spawn(process.execPath, ['-e', 'setTimeout(() => {}, 120000)']);", + "fs.writeFileSync(process.env.__CM_ABORT_TEST_MARKER, JSON.stringify({ child: process.pid, grand: g.pid }));", + "setTimeout(() => {}, 120000);", + ].join("\n"); + const pids: number[] = []; + let sandboxTmp = ""; + try { + const controller = new AbortController(); + const promise = executor.execute({ + language: "shell", + // Tree: bash (spawned root) -> node child -> node grandchild. + code: `"$__CM_ABORT_NODE" -e "eval(process.env.__CM_ABORT_CHILD_CODE)"`, + signal: controller.signal, + }); + + await waitFor(() => existsSync(marker), 10_000, "process tree to spawn"); + const info = JSON.parse(readFileSync(marker, "utf-8")); + pids.push(info.child, info.grand); + sandboxTmp = findSandboxDir() ?? ""; + + assert.ok( + sandboxTmp.includes(".ctx-mode-"), + `sandbox should be the per-call .ctx-mode temp dir, got: ${sandboxTmp}`, + ); + assert.ok( + isAlive(info.child) && isAlive(info.grand), + "child and grandchild must be alive before abort", + ); + + controller.abort(); + const abortAt = Date.now(); + const result = await promise; + assert.ok( + Date.now() - abortAt < 5_000, + "abort must settle the execution promptly, not wait on child close", + ); + + assert.equal(result.timedOut, false, "abort must not be reported as timeout"); + assert.notEqual(result.exitCode, 0, "aborted shell must exit non-zero"); + for (const pid of pids) { + await waitDead(pid); + assert.equal(isAlive(pid), false, `PID ${pid} survived abort — orphaned process`); + } + assert.equal( + existsSync(sandboxTmp), + false, + ".ctx-mode-* temp dir must be removed after abort (ownership.json included)", + ); + } finally { + delete process.env[MARKER_ENV]; + delete process.env.__CM_ABORT_NODE; + delete process.env.__CM_ABORT_CHILD_CODE; + for (const pid of pids) { + try { process.kill(pid, "SIGKILL"); } catch { /* already dead */ } + } + rmSync(scratch, { recursive: true, force: true }); + } + }, 20_000); + + test("ownership.json sidecar carries metadata only (no command/code/secrets) and is cleaned up", async () => { + const scratch = mkdtempSync(join(tmpdir(), "ctx-abort-test-")); + const marker = join(scratch, "marker"); + process.env[MARKER_ENV] = marker; + try { + const controller = new AbortController(); + const promise = executor.execute({ + language: "shell", + code: [ + `echo running > "$${MARKER_ENV}"`, + `# CANARY_SECRET_9f2e41 must never appear in the sidecar`, + `sleep 120`, + ].join("\n"), + signal: controller.signal, + }); + + await waitFor(() => existsSync(marker), 10_000, "marker file"); + const sandboxTmp = findSandboxDir() ?? ""; + assert.ok(sandboxTmp.includes(".ctx-mode-"), `expected sandbox dir, got: ${sandboxTmp}`); + + const manifestPath = join(sandboxTmp, "ownership.json"); + const raw = readFileSync(manifestPath, "utf-8"); + const manifest = JSON.parse(raw); + + // Fixed metadata-only schema: no code, command, cwd, or environment. + assert.deepEqual( + Object.keys(manifest).sort(), + ["createdAt", "executorPid", "language", "nonce", "parentPid", "scriptPath", "scriptSha256", "version"], + "ownership.json must expose exactly the metadata-only schema", + ); + assert.equal(manifest.version, 1); + assert.equal(manifest.language, "shell"); + assert.equal(manifest.executorPid, process.pid); + assert.match(manifest.nonce, /^[0-9a-f]{32}$/); + assert.match(manifest.scriptSha256, /^[0-9a-f]{64}$/); + assert.ok( + manifest.scriptPath.startsWith(sandboxTmp), + `scriptPath must stay inside the sandbox tmp dir: ${manifest.scriptPath}`, + ); + assert.equal(raw.includes("CANARY_SECRET_9f2e41"), false, "sidecar must not retain executed code text"); + assert.equal(raw.includes("sleep 120"), false, "sidecar must not retain executed command text"); + + controller.abort(); + const abortAt = Date.now(); + await promise; + assert.ok(Date.now() - abortAt < 5_000, "abort must settle promptly"); + assert.equal(existsSync(manifestPath), false, "ownership.json must be removed with the temp dir"); + assert.equal(existsSync(sandboxTmp), false, ".ctx-mode-* temp dir must be removed after abort"); + } finally { + delete process.env[MARKER_ENV]; + rmSync(scratch, { recursive: true, force: true }); + } + }, 20_000); + + test("pre-aborted signal settles immediately and never spawns the script", async () => { + const scratch = mkdtempSync(join(tmpdir(), "ctx-abort-test-")); + const marker = join(scratch, "marker"); + process.env[MARKER_ENV] = marker; + try { + const controller = new AbortController(); + controller.abort(); + const started = Date.now(); + const r = await executor.execute({ + language: "shell", + code: `echo started > "$${MARKER_ENV}"\nsleep 120`, + signal: controller.signal, + }); + assert.ok(Date.now() - started < 5_000, "pre-aborted execution must settle immediately"); + assert.equal(r.timedOut, false, "pre-aborted execution is a cancellation, not a timeout"); + assert.notEqual(r.exitCode, 0); + assert.equal(existsSync(marker), false, "pre-aborted execution must never run the script"); + } finally { + delete process.env[MARKER_ENV]; + rmSync(scratch, { recursive: true, force: true }); + } + }, 10_000); + + test("abort is request-scoped: concurrent and fresh executions are unaffected", async () => { + const scratch = mkdtempSync(join(tmpdir(), "ctx-abort-test-")); + const marker = join(scratch, "marker"); + process.env[MARKER_ENV] = marker; + let sandboxTmp = ""; + try { + const controller = new AbortController(); + const abortable = executor.execute({ + language: "shell", + code: `echo running > "$${MARKER_ENV}"\nsleep 120`, + signal: controller.signal, + }); + // Concurrent execution with NO signal — must run to completion untouched. + const concurrent = executor.execute({ + language: "shell", + code: "sleep 1 && echo done", + }); + + await waitFor(() => existsSync(marker), 10_000, "marker file"); + sandboxTmp = findSandboxDir() ?? ""; + assert.ok(sandboxTmp.includes(".ctx-mode-"), `expected sandbox dir, got: ${sandboxTmp}`); + + controller.abort(); + const abortAt = Date.now(); + const [aborted, kept] = await Promise.all([abortable, concurrent]); + assert.ok(Date.now() - abortAt < 5_000, "abort must settle promptly"); + + assert.equal(aborted.timedOut, false); + assert.notEqual(aborted.exitCode, 0); + assert.equal(kept.exitCode, 0, "concurrent execution must not be touched by another request's abort"); + assert.equal(kept.timedOut, false); + assert.equal(kept.stdout.trim(), "done"); + assert.equal(existsSync(sandboxTmp), false, "aborted execution's temp dir must be cleaned"); + + // Fresh execution AFTER the abort — a stale listener must never kill it. + const fresh = await executor.execute({ + language: "javascript", + code: "console.log('fresh');", + }); + assert.equal(fresh.exitCode, 0); + assert.equal(fresh.stdout.trim(), "fresh"); + } finally { + delete process.env[MARKER_ENV]; + rmSync(scratch, { recursive: true, force: true }); + } + }, 20_000); + + test("timeout path stays green when an inert signal is attached", async () => { + const controller = new AbortController(); + const r = await executor.execute({ + language: "javascript", + code: "process.stdout.write(String(process.pid)); setTimeout(() => {}, 30000);", + timeout: 600, + signal: controller.signal, + }); + assert.equal(r.timedOut, true, "timeout must still fire when a signal is attached but never aborts"); + const pid = parseInt(r.stdout.trim(), 10); + assert.ok(pid > 0, `expected PID in stdout, got "${r.stdout}"`); + await waitDead(pid); + assert.equal(isAlive(pid), false, "timed-out process survived"); + }, 10_000); + + test("executeFile forwards the signal and aborts the wrapped execution", async () => { + const scratch = mkdtempSync(join(tmpdir(), "ctx-abort-test-")); + const dataFile = join(scratch, "data.txt"); + writeFileSync(dataFile, "hello\nworld\n", "utf-8"); + try { + const controller = new AbortController(); + const promise = executor.executeFile({ + path: dataFile, + language: "javascript", + code: "setTimeout(() => {}, 30000);", + signal: controller.signal, + }); + await sleep(500); + controller.abort(); + const started = Date.now(); + const r = await promise; + assert.ok(Date.now() - started < 5_000, "executeFile abort must terminate promptly"); + assert.equal(r.timedOut, false); + assert.notEqual(r.exitCode, 0); + } finally { + rmSync(scratch, { recursive: true, force: true }); + } + }, 15_000); +});