diff --git a/sdk/typescript/README.md b/sdk/typescript/README.md index fb5e5968e..05856eff3 100644 --- a/sdk/typescript/README.md +++ b/sdk/typescript/README.md @@ -907,8 +907,54 @@ command manifest, `scan --schema --format json` for a command schema, and `--format toon|json|yaml|jsonl` and `--full-output`. `skills add` syncs agent skills; `mcp add` registers the CLI as an MCP server. -MCP exposes only the read-only `info` command because the transport cannot -cancel active scans. +Start the server with `codex-security --mcp` (or +`npx --yes @openai/codex-security --mcp`). It uses stdin/stdout and exposes +`info` for read-only metadata and `scan` for security scans. For example, an +MCP client can launch it with: + +```json +{ + "mcpServers": { + "codex-security": { + "command": "npx", + "args": ["--yes", "@openai/codex-security", "--mcp"] + } + } +} +``` + +The `scan` tool accepts the existing scan options using camelCase names: +`repository`, `path`, `mode`, `diff`, `workingTree`, `outputDir`, `maxCost`, +and so on. Defaults match the CLI. Relative paths resolve from the server's +working directory. First check local inputs without starting a model: + +```json +{ "repository": "/path/to/repository", "dryRun": true } +``` + +Then call `scan` with `dryRun` omitted or false to run the scan. Standard, +Deep, path, and Git diff scans are supported. MCP does not support `patch`, +`patchSeverity`, or `createPr`; patching and other commands remain CLI-only. + +Scans run noninteractively with the same local credentials and `auth` +selection as the CLI. Sign in with `codex-security login` before starting the +server, or supply `OPENAI_API_KEY`/`CODEX_API_KEY` in its environment. Scans +can incur model costs, write local artifacts, and run repository tools. Only +scan targets the user has authorized, and configure the MCP client's tool +call timeout to allow the scan to finish. + +Results are returned as JSON in both text content and `structuredContent`: +`{ "exitCode": 0, "data": { ... } }`. `data` is the same scan result or dry-run +preflight data as CLI JSON output. Exit code `1` means the requested severity +threshold was met; `2` means invalid inputs, incomplete results, or failure. +Nonzero outcomes set MCP `isError` while preserving any available `data` and +`error` message. Scan diagnostics go to stderr; stdout is reserved for MCP. + +MCP cancellation notifications stop the corresponding scan. Disconnecting +the client or stopping the server cancels active scans and waits for cleanup; +partial artifacts remain available at the output directory. Canceled MCP +requests do not receive a result. This server is separate from the bundled +security plugin's MCP server used internally during scans. ## Containerized bulk scans diff --git a/sdk/typescript/package.json b/sdk/typescript/package.json index 38f9a145a..b47288212 100644 --- a/sdk/typescript/package.json +++ b/sdk/typescript/package.json @@ -60,6 +60,7 @@ "dependencies": { "@inquirer/prompts": "8.3.0", "@linear/sdk": "89.0.0", + "@modelcontextprotocol/server": "2.0.0-beta.4", "@octokit/core": "7.0.6", "@openai/codex": "0.149.1", "@openai/codex-sdk": "0.149.1", diff --git a/sdk/typescript/pnpm-lock.yaml b/sdk/typescript/pnpm-lock.yaml index 9683918ef..6cf37bbb0 100644 --- a/sdk/typescript/pnpm-lock.yaml +++ b/sdk/typescript/pnpm-lock.yaml @@ -14,6 +14,9 @@ importers: '@linear/sdk': specifier: 89.0.0 version: 89.0.0(graphql@17.0.2) + '@modelcontextprotocol/server': + specifier: 2.0.0-beta.4 + version: 2.0.0-beta.4 '@octokit/core': specifier: 7.0.6 version: 7.0.6 diff --git a/sdk/typescript/scripts/smoke-package.mjs b/sdk/typescript/scripts/smoke-package.mjs index 80a442b47..a497ab28f 100644 --- a/sdk/typescript/scripts/smoke-package.mjs +++ b/sdk/typescript/scripts/smoke-package.mjs @@ -1,5 +1,6 @@ import assert from "node:assert/strict"; -import { spawnSync } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; +import { once } from "node:events"; import { chmod, cp, @@ -22,6 +23,7 @@ import { sep, } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; +import { createInterface } from "node:readline"; import { packageSmokeTimeouts } from "./package-smoke-timeouts.mjs"; const PACKAGE_SMOKE_TIMEOUT_MS = packageSmokeTimeouts().commandTimeoutMs; @@ -158,6 +160,74 @@ async function pluginFiles(directory) { return files.sort(); } +async function smokeCliMcp(launcher, consumer) { + const repository = join(consumer, "mcp-repository"); + await mkdir(repository); + await writeFile( + join(repository, "example.js"), + "export const example = 1;\n", + ); + const child = spawn(process.execPath, [launcher, "--mcp"], { + cwd: consumer, + env: { + ...process.env, + CODEX_SECURITY_STATE_DIR: join(consumer, "mcp-state"), + }, + stdio: "pipe", + timeout: PACKAGE_SMOKE_TIMEOUT_MS, + killSignal: "SIGKILL", + windowsHide: true, + }); + const closed = once(child, "close"); + const lines = createInterface({ input: child.stdout }); + const responses = lines[Symbol.asyncIterator](); + let stderr = ""; + child.stderr.setEncoding("utf8").on("data", (text) => (stderr += text)); + const send = (message) => child.stdin.write(JSON.stringify(message) + "\n"); + async function request(id, method, params) { + send({ jsonrpc: "2.0", id, method, params }); + for (;;) { + const line = await responses.next(); + assert.equal(line.done, false, `MCP closed before ${method}: ${stderr}`); + const response = JSON.parse(line.value); + if (response.id !== id) continue; + assert.equal(response.error, undefined, JSON.stringify(response)); + return response.result; + } + } + try { + await request(1, "initialize", { + protocolVersion: "2025-11-25", + capabilities: {}, + clientInfo: { name: "package-smoke", version: "1.0.0" }, + }); + send({ jsonrpc: "2.0", method: "notifications/initialized" }); + const tools = await request(2, "tools/list", {}); + assert.deepEqual(tools.tools.map((tool) => tool.name).sort(), [ + "info", + "scan", + ]); + const info = await request(3, "tools/call", { + name: "info", + arguments: {}, + }); + assert.equal(info.structuredContent.scanMcp, true); + const scan = await request(4, "tools/call", { + name: "scan", + arguments: { repository, dryRun: true }, + }); + assert.notEqual(scan.isError, true, JSON.stringify(scan)); + assert.equal(scan.structuredContent.exitCode, 0); + assert.equal(scan.structuredContent.data.dryRun, true); + child.stdin.end(); + assert.equal((await closed)[0], 0, stderr); + } finally { + lines.close(); + child.kill("SIGKILL"); + await closed; + } +} + async function smokeNestedDeepScanWorker(installedRoot, consumer) { const sdk = await import( pathToFileURL(join(installedRoot, "dist", "index.js")).href @@ -472,6 +542,8 @@ try { assert.match(help, /Usage: codex-security\b/u); assert.match(help, /\bpublish\b/u); + await smokeCliMcp(launcher, consumer); + const publicationScan = join(consumer, "publication-scan"); await cp( join(installedRoot, "_bundled_plugin", "examples", "completed-scan"), diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index 5656c5862..03f7fd4b0 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -46,6 +46,7 @@ import { pipeline } from "node:stream/promises"; import { fileURLToPath, pathToFileURL } from "node:url"; import { promisify, stripVTControlCharacters } from "node:util"; import { Cli, z } from "incur"; +import type { Transport } from "@modelcontextprotocol/server"; import { parse as parseToml } from "smol-toml"; import { classifyConnectionFailure, @@ -1105,6 +1106,7 @@ interface PatchRiskAssessment extends PatchRiskReport { } interface CliDependencies { + mcpInput?: Readable; createSecurity( config: CodexSecurityConfig, ): Pick; @@ -1575,6 +1577,7 @@ export async function main( "--llms", "--llms-full", "--schema", + "--mcp", "--dry-run", ].includes(argument), ) && @@ -2710,187 +2713,251 @@ export async function main( } }, }); + const scanArgsSchema = z.object({ + repository: z + .string() + .optional() + .describe("Repository root to scan (default: current directory)."), + }); + const scanOptionsSchema = z + .object({ + auth: z + .enum(SCAN_AUTH_MODES) + .default("auto") + .describe( + "Select ChatGPT, OPENAI_API_KEY/CODEX_API_KEY, or automatic authentication.", + ), + verbose: z + .boolean() + .default(false) + .describe("Print scan diagnostics to stderr."), + safetyIdentifier: optionValue("--safety-identifier") + .optional() + .describe( + "Stable hashed end-user ID for this scan's model requests (1–64 characters).", + ), + path: z + .array(optionValue("--path")) + .default([]) + .describe( + "Scan only PATH; repeat for multiple repository-relative paths.", + ), + knowledgeBase: z + .array(optionValue("--knowledge-base")) + .default([]) + .describe( + "Add security-context files or directories; repeat for multiple paths.", + ), + scanPromptFile: optionValue("--scan-prompt-file") + .optional() + .describe("Append scan instructions from FILE."), + validationPromptFile: optionValue("--validation-prompt-file") + .optional() + .describe( + "Replace final validation with the workflow in FILE (not Deep).", + ), + postScanPromptFile: optionValue("--post-scan-prompt-file") + .optional() + .describe("Run FILE after each scan, including failures."), + diff: optionValue("--diff") + .optional() + .describe("Scan committed Git changes from BASE to --head."), + workingTree: z + .boolean() + .default(false) + .describe("Scan staged and unstaged changes against --base."), + head: optionValue("--head") + .optional() + .describe("Git head ref for --diff (default: HEAD)."), + base: optionValue("--base") + .optional() + .describe("Git base ref for --working-tree (default: HEAD)."), + mode: z + .enum(["standard", "deep"]) + .default("standard") + .describe("Scan mode; deep supports repository and path targets."), + ...DEEP_SCAN_OPTION_SCHEMAS, + model: optionValue("--model") + .optional() + .describe( + `OpenAI model to use (default: ${DEFAULT_SCAN_MODEL_CONFIGURATION.model}).`, + ), + effort: effortOption(), + provider: PROVIDER_OPTION, + outputDir: optionValue("--output-dir") + .optional() + .describe( + "Artifact directory outside the repository (default: Codex Security state; CODEX_SECURITY_STATE_DIR).", + ), + archiveExisting: z + .boolean() + .default(false) + .describe("Archive existing results; requires --output-dir."), + pluginPath: optionValue("--plugin-path") + .optional() + .describe(PLUGIN_PATH_DESCRIPTION), + python: optionValue("--python") + .optional() + .describe(PYTHON_PATH_DESCRIPTION), + codex: z + .array(optionValue("--codex")) + .default([]) + .describe(CODEX_OVERRIDE_DESCRIPTION), + failOnSeverity: z + .enum(REPORTABLE_SEVERITIES) + .optional() + .describe("Exit 1 for findings at or above LEVEL."), + patch: z + .boolean() + .default(false) + .describe("Patch and verify confirmed findings after the scan."), + patchSeverity: z + .enum(REPORTABLE_SEVERITIES) + .optional() + .describe("Patch findings at or above LEVEL; requires --patch."), + createPr: CREATE_PR_OPTION, + maxCost: z + .number() + .positive() + .optional() + .describe("Stop the scan if estimated USD cost exceeds AMOUNT."), + headless: z + .boolean() + .default(false) + .describe( + "Use plain text progress instead of the interactive dashboard.", + ), + dryRun: z + .boolean() + .default(false) + .describe("Validate local scan inputs without starting a scan."), + }) + .refine( + (options) => + Number(options.path.length > 0) + + Number(options.diff !== undefined) + + Number(options.workingTree) <= + 1, + { + message: "--path, --diff, and --working-tree are mutually exclusive.", + }, + ) + .refine( + (options) => options.head === undefined || options.diff !== undefined, + { message: "--head requires --diff." }, + ) + .refine((options) => options.base === undefined || options.workingTree, { + message: "--base requires --working-tree.", + }) + .refine( + (options) => !options.archiveExisting || options.outputDir !== undefined, + { message: "--archive-existing requires --output-dir." }, + ) + .refine((options) => options.patchSeverity === undefined || options.patch, { + message: "--patch-severity requires --patch.", + }) + .refine((options) => !options.createPr || options.patch, { + message: "--create-pr requires --patch.", + }) + .refine((options) => !options.patch || !options.dryRun, { + message: "--patch cannot be combined with --dry-run.", + }) + .refine( + (options) => + options.mode === "deep" || + (options.workers === undefined && + options.subagents === undefined && + options.stopAfterNoNew === undefined && + options.maxDiscoveryRuns === undefined && + options.maxTimeHours === undefined), + { message: "Deep scan settings require --mode deep." }, + ); + const scanArguments = ( + repository: string | undefined, + options: z.infer, + ): ScanArguments => ({ + auth: options.auth, + safetyIdentifier: options.safetyIdentifier, + verbose: options.verbose, + repository: repository, + paths: options.path, + knowledgeBasePaths: options.knowledgeBase, + scanPromptFile: options.scanPromptFile, + validationPromptFile: options.validationPromptFile, + postScanPromptFile: options.postScanPromptFile, + diff: options.diff, + workingTree: options.workingTree, + head: options.head, + base: options.base, + mode: options.mode, + workers: options.workers, + subagents: options.subagents, + stopAfterNoNew: options.stopAfterNoNew, + maxDiscoveryRuns: options.maxDiscoveryRuns, + maxTimeHours: options.maxTimeHours, + model: options.model, + effort: options.effort, + provider: options.provider, + outputDir: options.outputDir, + archiveExisting: options.archiveExisting, + pluginPath: options.pluginPath, + pythonPath: options.python, + codex: options.codex, + failOnSeverity: options.failOnSeverity, + patch: options.patch, + patchSeverity: options.patchSeverity, + createPr: options.createPr, + maxCostUsd: options.maxCost, + headless: options.headless, + dryRun: options.dryRun, + }); + const infoSchema = z.object({ + sdkVersion: z.string(), + bundledPluginVersion: z.string(), + scanMcp: z.literal(true), + cancellationNote: z.string(), + cliVersion: z.string(), + codexVersion: z.string(), + codexSdkVersion: z.string(), + model: z.string(), + reasoningEffort: z.string(), + nextStep: z.string(), + }); + const metadata = () => ({ + sdkVersion: VERSION, + bundledPluginVersion: BUNDLED_PLUGIN_VERSION, + scanMcp: true as const, + cancellationNote: + "MCP request cancellation and client disconnects stop active scans and preserve partial output.", + cliVersion: VERSION, + codexVersion: CODEX_EXECUTABLE_VERSION, + codexSdkVersion: CODEX_SDK_VERSION, + ...scanModelConfiguration(DEFAULT_CODEX_CONFIG), + nextStep: "codex-security scan . --dry-run", + }); + const mcpInstructions = + "Use info for SDK metadata and scan to run security scans. Scans use local credentials, can make billable model calls, and write artifacts. Only scan repositories the user has authorized. Patching and other commands remain CLI-only."; + const scanMcpAnnotations = { + readOnlyHint: false, + destructiveHint: true, + idempotentHint: false, + openWorldHint: true, + }; const cli = Cli.create("codex-security", { description: "Run, import, validate, patch, verify fixes, export, and publish Codex Security findings.", version: VERSION, mcp: { command: "npx --yes @openai/codex-security --mcp", - instructions: - "Use info for read-only SDK metadata. Scans and other state-changing commands are CLI-only because the MCP transport cannot cancel active commands.", + instructions: mcpInstructions, }, }) .command("scan", { description: "Run a Codex Security scan.", destructive: true, - mcp: false, - args: z.object({ - repository: z - .string() - .optional() - .describe("Repository root to scan (default: current directory)."), - }), - options: z - .object({ - auth: z - .enum(SCAN_AUTH_MODES) - .default("auto") - .describe( - "Select ChatGPT, OPENAI_API_KEY/CODEX_API_KEY, or automatic authentication.", - ), - verbose: z - .boolean() - .default(false) - .describe("Print scan diagnostics to stderr."), - safetyIdentifier: optionValue("--safety-identifier") - .optional() - .describe( - "Stable hashed end-user ID for this scan's model requests (1–64 characters).", - ), - path: z - .array(optionValue("--path")) - .default([]) - .describe( - "Scan only PATH; repeat for multiple repository-relative paths.", - ), - knowledgeBase: z - .array(optionValue("--knowledge-base")) - .default([]) - .describe( - "Add security-context files or directories; repeat for multiple paths.", - ), - scanPromptFile: optionValue("--scan-prompt-file") - .optional() - .describe("Append scan instructions from FILE."), - validationPromptFile: optionValue("--validation-prompt-file") - .optional() - .describe( - "Replace final validation with the workflow in FILE (not Deep).", - ), - postScanPromptFile: optionValue("--post-scan-prompt-file") - .optional() - .describe("Run FILE after each scan, including failures."), - diff: optionValue("--diff") - .optional() - .describe("Scan committed Git changes from BASE to --head."), - workingTree: z - .boolean() - .default(false) - .describe("Scan staged and unstaged changes against --base."), - head: optionValue("--head") - .optional() - .describe("Git head ref for --diff (default: HEAD)."), - base: optionValue("--base") - .optional() - .describe("Git base ref for --working-tree (default: HEAD)."), - mode: z - .enum(["standard", "deep"]) - .default("standard") - .describe("Scan mode; deep supports repository and path targets."), - ...DEEP_SCAN_OPTION_SCHEMAS, - model: optionValue("--model") - .optional() - .describe( - `OpenAI model to use (default: ${DEFAULT_SCAN_MODEL_CONFIGURATION.model}).`, - ), - effort: effortOption(), - provider: PROVIDER_OPTION, - outputDir: optionValue("--output-dir") - .optional() - .describe( - "Artifact directory outside the repository (default: Codex Security state; CODEX_SECURITY_STATE_DIR).", - ), - archiveExisting: z - .boolean() - .default(false) - .describe("Archive existing results; requires --output-dir."), - pluginPath: optionValue("--plugin-path") - .optional() - .describe(PLUGIN_PATH_DESCRIPTION), - python: optionValue("--python") - .optional() - .describe(PYTHON_PATH_DESCRIPTION), - codex: z - .array(optionValue("--codex")) - .default([]) - .describe(CODEX_OVERRIDE_DESCRIPTION), - failOnSeverity: z - .enum(REPORTABLE_SEVERITIES) - .optional() - .describe("Exit 1 for findings at or above LEVEL."), - patch: z - .boolean() - .default(false) - .describe("Patch and verify confirmed findings after the scan."), - patchSeverity: z - .enum(REPORTABLE_SEVERITIES) - .optional() - .describe("Patch findings at or above LEVEL; requires --patch."), - createPr: CREATE_PR_OPTION, - maxCost: z - .number() - .positive() - .optional() - .describe("Stop the scan if estimated USD cost exceeds AMOUNT."), - headless: z - .boolean() - .default(false) - .describe( - "Use plain text progress instead of the interactive dashboard.", - ), - dryRun: z - .boolean() - .default(false) - .describe("Validate local scan inputs without starting a scan."), - }) - .refine( - (options) => - Number(options.path.length > 0) + - Number(options.diff !== undefined) + - Number(options.workingTree) <= - 1, - { - message: - "--path, --diff, and --working-tree are mutually exclusive.", - }, - ) - .refine( - (options) => options.head === undefined || options.diff !== undefined, - { message: "--head requires --diff." }, - ) - .refine( - (options) => options.base === undefined || options.workingTree, - { - message: "--base requires --working-tree.", - }, - ) - .refine( - (options) => - !options.archiveExisting || options.outputDir !== undefined, - { message: "--archive-existing requires --output-dir." }, - ) - .refine( - (options) => options.patchSeverity === undefined || options.patch, - { - message: "--patch-severity requires --patch.", - }, - ) - .refine((options) => !options.createPr || options.patch, { - message: "--create-pr requires --patch.", - }) - .refine((options) => !options.patch || !options.dryRun, { - message: "--patch cannot be combined with --dry-run.", - }) - .refine( - (options) => - options.mode === "deep" || - (options.workers === undefined && - options.subagents === undefined && - options.stopAfterNoNew === undefined && - options.maxDiscoveryRuns === undefined && - options.maxTimeHours === undefined), - { message: "Deep scan settings require --mode deep." }, - ), + mcp: { annotations: scanMcpAnnotations }, + args: scanArgsSchema, + options: scanOptionsSchema, examples: [ { args: { repository: "." } }, { args: { repository: "." }, options: { model: "gpt-5.6-terra" } }, @@ -2919,42 +2986,7 @@ export async function main( return; } const outcome = await runScan( - { - auth: options.auth, - safetyIdentifier: options.safetyIdentifier, - verbose: options.verbose, - repository: args.repository, - paths: options.path, - knowledgeBasePaths: options.knowledgeBase, - scanPromptFile: options.scanPromptFile, - validationPromptFile: options.validationPromptFile, - postScanPromptFile: options.postScanPromptFile, - diff: options.diff, - workingTree: options.workingTree, - head: options.head, - base: options.base, - mode: options.mode, - workers: options.workers, - subagents: options.subagents, - stopAfterNoNew: options.stopAfterNoNew, - maxDiscoveryRuns: options.maxDiscoveryRuns, - maxTimeHours: options.maxTimeHours, - model: options.model, - effort: options.effort, - provider: options.provider, - outputDir: options.outputDir, - archiveExisting: options.archiveExisting, - pluginPath: options.pluginPath, - pythonPath: options.python, - codex: options.codex, - failOnSeverity: options.failOnSeverity, - patch: options.patch, - patchSeverity: options.patchSeverity, - createPr: options.createPr, - maxCostUsd: options.maxCost, - headless: options.headless, - dryRun: options.dryRun, - }, + scanArguments(args.repository, options), errorOutput, dependencies, format !== "json" && format !== "jsonl", @@ -4207,33 +4239,195 @@ export async function main( openWorldHint: false, }, }, - output: z.object({ - sdkVersion: z.string(), - bundledPluginVersion: z.string(), - scanMcp: z.literal(false), - cancellationNote: z.string(), - cliVersion: z.string(), - codexVersion: z.string(), - codexSdkVersion: z.string(), - model: z.string(), - reasoningEffort: z.string(), - nextStep: z.string(), - }), - run() { + output: infoSchema, + run: metadata, + }); + + if (argv.includes("--mcp")) { + // incur's MCP adapter does not pass request cancellation to commands. + const [{ McpServer }, { StdioServerTransport }] = await Promise.all([ + import("@modelcontextprotocol/server"), + import("@modelcontextprotocol/server/stdio"), + ]); + const server = new McpServer( + { name: "codex-security", version: VERSION }, + { instructions: mcpInstructions }, + ); + const pending = new Set>(); + // The SDK ignores cancellation for request IDs 0 and "". Track those + // requests before async tool validation so immediate cancellation works too. + const scanCancellation = new Map(); + server.registerTool( + "info", + { + description: "Show read-only SDK and bundled-plugin metadata.", + outputSchema: infoSchema, + annotations: { + readOnlyHint: true, + idempotentHint: true, + destructiveHint: false, + openWorldHint: false, + }, + }, + () => { + const data = metadata(); return { - sdkVersion: VERSION, - bundledPluginVersion: BUNDLED_PLUGIN_VERSION, - scanMcp: false as const, - cancellationNote: - "Scans are CLI-only because the MCP transport cannot cancel active commands.", - cliVersion: VERSION, - codexVersion: CODEX_EXECUTABLE_VERSION, - codexSdkVersion: CODEX_SDK_VERSION, - ...scanModelConfiguration(DEFAULT_CODEX_CONFIG), - nextStep: "codex-security scan . --dry-run", + content: [{ type: "text", text: JSON.stringify(data) }], + structuredContent: data, + }; + }, + ); + server.registerTool( + "scan", + { + description: + "Run a Codex Security scan and return its exit code and results. Uses local credentials, can incur model costs, and writes scan artifacts. Does not patch findings.", + inputSchema: z + .object(scanOptionsSchema.shape) + .omit({ patch: true, patchSeverity: true, createPr: true }) + .extend(scanArgsSchema.shape) + .strict(), + outputSchema: z.object({ + exitCode: z.number(), + data: z.record(z.string(), z.unknown()).optional(), + error: z.string().optional(), + }), + annotations: scanMcpAnnotations, + }, + async (input, context) => { + // Parse the complete CLI schema too, preserving its cross-option refinements. + const parsed = scanOptionsSchema.safeParse(input); + let outcome: ScanOutcome; + if (!parsed.success) { + outcome = { + exitCode: 2, + error: parsed.error.issues.map((issue) => issue.message).join(" "), + }; + } else { + const operation = runScan( + scanArguments(input.repository, parsed.data), + errorOutput, + dependencies, + false, + scanCancellation.has(context.mcpReq.id) + ? AbortSignal.any([ + context.mcpReq.signal, + scanCancellation.get(context.mcpReq.id)!.signal, + ]) + : context.mcpReq.signal, + ); + pending.add(operation); + try { + outcome = await operation; + } finally { + pending.delete(operation); + } + } + return { + content: [{ type: "text", text: JSON.stringify(outcome) }], + structuredContent: { ...outcome }, + ...(outcome.exitCode === 0 ? {} : { isError: true }), + }; + }, + ); + const input = dependencies.mcpInput ?? process.stdin; + const protocolOutput = + output instanceof NodeWritable + ? output + : new NodeWritable({ + write(chunk, _encoding, callback) { + writeCliOutput(output, chunk).then(() => callback(), callback); + }, + }); + const stdio = new StdioServerTransport(input, protocolOutput); + const transport: Transport = { + async start() { + stdio.onmessage = (message) => { + if ( + "id" in message && + (message.id === 0 || message.id === "") && + "method" in message && + message.method === "tools/call" && + message.params?.["name"] === "scan" + ) { + scanCancellation.set(message.id, new AbortController()); + } else if ( + "method" in message && + message.method === "notifications/cancelled" + ) { + const requestId = message.params?.["requestId"]; + if (requestId === 0 || requestId === "") { + scanCancellation.get(requestId)?.abort(); + } + } + transport.onmessage?.(message); }; + stdio.onclose = () => transport.onclose?.(); + stdio.onerror = (error) => transport.onerror?.(error); + await stdio.start(); }, + async send(message) { + if ( + "id" in message && + !("method" in message) && + message.id !== undefined && + message.id !== null + ) { + const cancellation = scanCancellation.get(message.id); + scanCancellation.delete(message.id); + if (cancellation?.signal.aborted) return; + } + await stdio.send(message); + }, + close: () => stdio.close(), + }; + let resolveClosed!: () => void; + const closed = new Promise((resolve) => { + resolveClosed = resolve; }); + server.server.onclose = resolveClosed; + let closing: Promise | undefined; + const stop = (): void => { + closing ??= server.close(); + void closing.then(resolveClosed, resolveClosed); + }; + const onInterrupt = (): void => { + exitCode = 130; + stop(); + }; + const onTerminate = (): void => { + exitCode = 143; + stop(); + }; + // StdioServerTransport does not close itself when its input reaches EOF. + input.once("end", stop); + input.once("close", stop); + input.once("error", stop); + protocolOutput.once("close", () => { + protocolOutput.off("error", stop); + stop(); + }); + // Writes can fail after EOF and after main returns. Keep the error handler + // for the output stream's lifetime, without waiting on a blocked writer. + protocolOutput.on("error", stop); + dependencies.addSignalListener("SIGINT", onInterrupt); + dependencies.addSignalListener("SIGTERM", onTerminate); + try { + await server.connect(transport); + await closed; + await Promise.allSettled(pending); + await closing; + } finally { + input.off("end", stop); + input.off("close", stop); + input.off("error", stop); + dependencies.removeSignalListener("SIGINT", onInterrupt); + dependencies.removeSignalListener("SIGTERM", onTerminate); + await server.close(); + updateController.abort(); + } + return exitCode; + } let notice: UpdateNotice | undefined; try { @@ -6125,9 +6319,10 @@ async function runScan( errorOutput: Writable, dependencies: CliDependencies, interactive = true, + signal?: AbortSignal, ): Promise { return await withTerminalErrorsHandled(errorOutput, () => - executeScan(arguments_, errorOutput, dependencies, interactive), + executeScan(arguments_, errorOutput, dependencies, interactive, signal), ); } @@ -6162,6 +6357,7 @@ async function executeScan( errorOutput: Writable, dependencies: CliDependencies, interactive = true, + signal?: AbortSignal, ): Promise { let scanDir: string | null = null; let requestedSignal: SignalName | null = null; @@ -6207,6 +6403,8 @@ async function executeScan( }); }; const preparationAbortController = new AbortController(); + const scanSignal = + signal === undefined ? preparationAbortController.signal : signal; const stopPresentation = (): void => { try { dashboard?.stop(); @@ -6248,8 +6446,10 @@ async function executeScan( dependencies.removeSignalListener("SIGINT", onInterrupt); dependencies.removeSignalListener("SIGTERM", onTerminate); }; - dependencies.addSignalListener("SIGINT", onInterrupt); - dependencies.addSignalListener("SIGTERM", onTerminate); + if (signal === undefined) { + dependencies.addSignalListener("SIGINT", onInterrupt); + dependencies.addSignalListener("SIGTERM", onTerminate); + } let security: Pick | null = null; @@ -6264,6 +6464,7 @@ async function executeScan( let failed = false; let failure: unknown; try { + scanSignal.throwIfAborted(); const directory = dependencies.currentDirectory(); repository = arguments_.repository ?? directory; const target = targetFromArguments(arguments_); @@ -6301,7 +6502,7 @@ async function executeScan( auth: arguments_.auth, provider, command: "scan", - signal: preparationAbortController.signal, + signal: scanSignal, }, errorOutput, dependencies, @@ -6481,7 +6682,7 @@ async function executeScan( `Moved existing results to: ${errorMessage(archiveDir)}\n`, ); }, - signal: preparationAbortController.signal, + signal: scanSignal, onOutputDirReady: (path) => { scanDir = path; diagnostic("scan.output_ready", { scan_dir: path }); @@ -6689,6 +6890,15 @@ async function executeScan( removeSignalListeners(); } + if (signal?.aborted) { + errorOutput.write("Scan canceled.\n"); + if (scanDir !== null) { + errorOutput.write( + `Partial output was kept at ${errorMessage(scanDir)}.\n`, + ); + } + return { exitCode: 130, error: "Scan canceled." }; + } if (requestedSignal !== null) { diagnostic("scan.interrupted", { signal: requestedSignal, diff --git a/sdk/typescript/tests-ts/cli-mcp.test.ts b/sdk/typescript/tests-ts/cli-mcp.test.ts new file mode 100644 index 000000000..4e46b4639 --- /dev/null +++ b/sdk/typescript/tests-ts/cli-mcp.test.ts @@ -0,0 +1,568 @@ +import { PassThrough, Writable } from "node:stream"; +import { setImmediate } from "node:timers/promises"; +import { describe, expect, test } from "bun:test"; +import type { CallToolResult, Tool } from "@modelcontextprotocol/server"; +import { main } from "../src/cli.js"; +import { ConfigurationError } from "../src/errors.js"; +import type { ScanOptions } from "../src/api.js"; +import { + capture, + dependencies, + fakePreflight, + fakeResult, + FakeSignals, +} from "./cli-fixtures.js"; +import { BUNDLED_PLUGIN_VERSION, VERSION } from "../src/version.js"; + +async function connect( + deps = dependencies(), + finishWrite: (callback: (error?: Error | null) => void) => void = ( + callback, + ) => callback(), +) { + const input = new PassThrough(); + const stderr = capture(true); + const responses = new Map(); + const waiting = new Map void>(); + let partial = ""; + const output = new Writable({ + write(chunk, _encoding, callback) { + partial += chunk.toString(); + let newline: number; + while ((newline = partial.indexOf("\n")) !== -1) { + // Every stdout line must be protocol JSON, even while scans report progress. + const response = JSON.parse(partial.slice(0, newline)); + partial = partial.slice(newline + 1); + if ( + typeof response.id === "number" || + typeof response.id === "string" + ) { + responses.set(response.id, response.result ?? response.error); + waiting.get(response.id)?.(response.result ?? response.error); + waiting.delete(response.id); + } + } + finishWrite(callback); + }, + }); + const serving = main(["--mcp"], output, stderr.stream, { + ...deps, + mcpInput: input, + }); + let id = 0; + const send = (message: object) => input.write(JSON.stringify(message) + "\n"); + const request = ( + method: string, + params: object = {}, + requestId: string | number = ++id, + ) => { + const result = new Promise((resolve) => { + waiting.set(requestId, (value) => resolve(value as T)); + }); + send({ jsonrpc: "2.0", id: requestId, method, params }); + return { id: requestId, result }; + }; + await request("initialize", { + protocolVersion: "2025-11-25", + capabilities: {}, + clientInfo: { name: "codex-security-test", version: "1.0.0" }, + }).result; + send({ jsonrpc: "2.0", method: "notifications/initialized" }); + return { + input, + output, + stderr, + serving, + responses, + request, + call: (name: string, args: object = {}, requestId?: string | number) => + request( + "tools/call", + { name, arguments: args }, + requestId, + ), + cancel: (requestId: string | number) => + send({ + jsonrpc: "2.0", + method: "notifications/cancelled", + params: { requestId, reason: "test cancellation" }, + }), + close: async () => { + input.end(); + expect(await serving).toBe(0); + expect(partial).toBe(""); + }, + }; +} + +describe("CLI MCP scans", () => { + test("advertises scan-only inputs and read-only metadata", async () => { + const session = await connect(); + try { + const { tools } = await session.request<{ tools: Tool[] }>("tools/list") + .result; + expect(tools.map((tool) => tool.name).sort()).toEqual(["info", "scan"]); + const scan = tools.find((tool) => tool.name === "scan")!; + expect(scan.inputSchema.properties).toMatchObject({ + repository: { type: "string" }, + path: { type: "array", default: [] }, + auth: { default: "auto" }, + mode: { default: "standard" }, + dryRun: { type: "boolean", default: false }, + }); + for (const name of ["patch", "patchSeverity", "createPr"]) { + expect(scan.inputSchema.properties).not.toHaveProperty(name); + } + expect(scan.annotations).toMatchObject({ + readOnlyHint: false, + openWorldHint: true, + }); + expect( + tools.find((tool) => tool.name === "info")?.annotations, + ).toMatchObject({ + readOnlyHint: true, + idempotentHint: true, + destructiveHint: false, + openWorldHint: false, + }); + const info = await session.call("info").result; + expect(info.structuredContent).toMatchObject({ + sdkVersion: VERSION, + bundledPluginVersion: BUNDLED_PLUGIN_VERSION, + scanMcp: true, + }); + expect(JSON.parse((info.content[0] as { text: string }).text)).toEqual( + info.structuredContent, + ); + } finally { + await session.close(); + } + }); + + test("runs scans with shared options, noninteractive auth and protocol-safe progress", async () => { + const calls: unknown[] = []; + let closed = 0; + const deps = dependencies({ + onTurn: (repository, options) => calls.push({ repository, options }), + onClose: () => { + closed++; + }, + costUpdates: [ + { + model: "gpt-5.6-sol", + estimatedUsd: 1, + inputTokens: 1, + cachedInputTokens: 0, + cacheWriteInputTokens: 0, + outputTokens: 1, + }, + ], + }); + deps.scanAuthenticationPrompt = { + isInteractive: () => true, + select: async () => { + throw new Error("MCP must not prompt"); + }, + }; + deps.hasStoredChatGPTSignIn = async () => true; + const session = await connect(deps); + try { + const result = await session.call("scan", { + repository: "/synthetic/repo", + auth: "chatgpt", + path: ["src"], + mode: "deep", + workers: 2, + maxCost: 5, + outputDir: "/synthetic/results", + model: "gpt-5.6-terra", + effort: "high", + }).result; + expect(result.isError).not.toBe(true); + expect(result.structuredContent).toMatchObject({ + exitCode: 0, + data: { scanDir: "/tmp/scan" }, + }); + expect(calls).toEqual([ + expect.objectContaining({ + repository: "/synthetic/repo", + options: expect.objectContaining({ + auth: "chatgpt", + mode: "deep", + workers: 2, + maxCostUsd: 5, + outputDir: "/synthetic/results", + }), + }), + ]); + expect(closed).toBe(1); + expect(session.stderr.text()).toContain("Scan complete"); + expect(session.stderr.text()).not.toContain("\u001b["); + } finally { + await session.close(); + } + }); + + test("uses preflight for dry runs without starting a model", async () => { + const deps = dependencies({ + onRun: () => { + throw new Error("must not scan"); + }, + }); + const session = await connect(deps); + try { + const result = await session.call("scan", { + repository: "/synthetic/repo", + dryRun: true, + }).result; + expect(result.structuredContent).toMatchObject({ + exitCode: 0, + data: { dryRun: true, repository: "/synthetic/repo" }, + }); + } finally { + await session.close(); + } + }); + + test("rejects invalid option combinations and unsupported mutations before scanning", async () => { + let started = 0; + const session = await connect( + dependencies({ + onRun: () => { + started++; + }, + }), + ); + try { + for (const input of [ + { path: ["src"], diff: "main" }, + { workingTree: true, diff: "main" }, + { head: "main" }, + { base: "main" }, + { archiveExisting: true }, + { workers: 2 }, + { maxCost: -1 }, + { patch: true }, + { patchSeverity: "high" }, + { createPr: true }, + ]) { + expect((await session.call("scan", input).result).isError).toBe(true); + } + expect(started).toBe(0); + expect(await session.call("patch").result).toMatchObject({ + code: -32602, + }); + } finally { + await session.close(); + } + }); + + test("preserves findings and per-call failure status without stopping the server", async () => { + for (const [result, input, exitCode] of [ + [fakeResult(["high"]), { failOnSeverity: "high" }, 1], + [fakeResult([], "partial"), {}, 2], + ] as const) { + const session = await connect(dependencies({ result })); + try { + const response = await session.call("scan", input).result; + expect(response.isError).toBe(true); + expect(response.structuredContent).toMatchObject({ + exitCode, + data: JSON.parse(JSON.stringify(result.toJSON())), + }); + expect((await session.call("info").result).isError).not.toBe(true); + } finally { + await session.close(); + } + } + const deps = dependencies(); + deps.createSecurity = () => ({ + run: async () => { + throw new ConfigurationError("synthetic configuration error"); + }, + preflight: async () => fakePreflight(), + close: async () => {}, + }); + const session = await connect(deps); + try { + expect( + (await session.call("scan").result).structuredContent, + ).toMatchObject({ + exitCode: 2, + error: expect.stringContaining("synthetic configuration error"), + }); + } finally { + await session.close(); + } + }); + + test.each([0, "", "0", 42])( + "cancels only scan request %j and waits for its cleanup", + async (requestId) => { + const started = Promise.withResolvers(); + const healthyStarted = Promise.withResolvers(); + const finishHealthy = Promise.withResolvers(); + const stopped = Promise.withResolvers(); + const deps = dependencies(); + deps.createSecurity = () => { + let canceledScan = false; + return { + run: async (repository, options) => { + canceledScan = repository === "/synthetic/cancel"; + if (!canceledScan) { + healthyStarted.resolve(options!.signal!); + await finishHealthy.promise; + return fakeResult(); + } + started.resolve(); + await new Promise((resolve) => + options!.signal!.addEventListener("abort", () => resolve(), { + once: true, + }), + ); + throw new DOMException("Canceled", "AbortError"); + }, + preflight: async () => fakePreflight(), + close: async () => { + if (canceledScan) stopped.resolve(); + }, + }; + }; + const session = await connect(deps); + try { + const canceled = session.call( + "scan", + { + repository: "/synthetic/cancel", + }, + requestId, + ); + const healthy = session.call( + "scan", + { + repository: "/synthetic/complete", + }, + requestId === "0" ? 0 : "0", + ); + await started.promise; + const healthySignal = await healthyStarted.promise; + session.cancel(canceled.id); + await stopped.promise; + expect(healthySignal.aborted).toBe(false); + finishHealthy.resolve(); + const completed = await healthy.result; + expect(completed.structuredContent).toMatchObject({ exitCode: 0 }); + expect(session.responses.has(canceled.id)).toBe(false); + expect((await session.call("info").result).isError).not.toBe(true); + } finally { + finishHealthy.resolve(); + await session.close(); + } + }, + ); + + test.each([0, "", "0"])( + "honors immediate cancellation of request %j before starting a scan", + async (requestId) => { + let started = 0; + const session = await connect( + dependencies({ + onRun: () => { + started++; + }, + }), + ); + try { + session.call("scan", {}, requestId); + session.cancel(requestId); + await session.call("info").result; + await setImmediate(); + expect(started).toBe(0); + expect(session.responses.has(requestId)).toBe(false); + } finally { + await session.close(); + } + }, + ); + + test.each([0, ""])( + "ignores unknown and late cancellations of request %j", + async (requestId) => { + const session = await connect(); + try { + session.cancel(requestId); + expect( + (await session.call("scan", {}, requestId).result).structuredContent, + ).toMatchObject({ exitCode: 0 }); + session.cancel(requestId); + expect( + (await session.call("scan", {}, requestId).result).structuredContent, + ).toMatchObject({ exitCode: 0 }); + } finally { + await session.close(); + } + }, + ); + + test("handles a buffered stdout failure after EOF while scan cleanup is pending", async () => { + let bufferOutput = false; + const pendingWrite = + Promise.withResolvers<(error?: Error | null) => void>(); + const started = Promise.withResolvers(); + const cleanupStarted = Promise.withResolvers(); + const finishCleanup = Promise.withResolvers(); + const deps = dependencies(); + deps.createSecurity = () => ({ + run: async (_repository, options) => { + started.resolve(); + await new Promise((resolve) => + options!.signal!.addEventListener("abort", () => resolve(), { + once: true, + }), + ); + throw new DOMException("Canceled", "AbortError"); + }, + preflight: async () => fakePreflight(), + close: async () => { + cleanupStarted.resolve(); + await finishCleanup.promise; + }, + }); + const session = await connect(deps, (callback) => { + if (bufferOutput) pendingWrite.resolve(callback); + else callback(); + }); + try { + session.call("scan"); + await started.promise; + bufferOutput = true; + session.call("info"); + const finishWrite = await pendingWrite.promise; + session.input.end(); + await cleanupStarted.promise; + finishWrite(new Error("synthetic broken pipe")); + await setImmediate(); + } finally { + finishCleanup.resolve(); + expect(await session.serving).toBe(0); + } + expect(session.output.listenerCount("error")).toBe(0); + }); + + test.each(["info", "scan"])( + "handles a buffered %s response failure after main returns", + async (name) => { + let bufferOutput = false; + let scansClosed = 0; + const pendingWrite = + Promise.withResolvers<(error?: Error | null) => void>(); + const session = await connect( + dependencies({ + onClose: () => { + scansClosed++; + }, + }), + (callback) => { + if (bufferOutput) pendingWrite.resolve(callback); + else callback(); + }, + ); + bufferOutput = true; + const response = session.call(name); + const finishWrite = await pendingWrite.promise; + expect((await response.result).isError).not.toBe(true); + session.input.end(); + expect(await session.serving).toBe(0); + expect(scansClosed).toBe(name === "scan" ? 1 : 0); + expect(session.output.writableLength).toBeGreaterThan(0); + finishWrite(new Error("synthetic broken pipe after shutdown")); + await setImmediate(); + expect(session.output.closed).toBe(true); + expect(session.output.listenerCount("error")).toBe(0); + }, + ); + + test("disconnects abort preparation and active scans, preserve artifacts, and await cleanup", async () => { + for (const phase of [ + "preparation", + "scan", + "preflight", + "output-close", + "output-error", + ] as const) { + const started = Promise.withResolvers(); + const canceled = Promise.withResolvers(); + const finishCleanup = Promise.withResolvers(); + const deps = dependencies(); + const waitForCancellation = async (options: ScanOptions | undefined) => { + if (phase === "scan") options!.onOutputDirReady?.("/synthetic/partial"); + started.resolve(); + await new Promise((resolve) => + options!.signal!.addEventListener("abort", () => resolve(), { + once: true, + }), + ); + canceled.resolve(); + throw new DOMException("Canceled", "AbortError"); + }; + deps.createSecurity = () => ({ + run: async (_repository, options) => waitForCancellation(options), + preflight: async (_repository, options) => waitForCancellation(options), + close: async () => { + await finishCleanup.promise; + }, + }); + const session = await connect(deps); + session.call("scan", { dryRun: phase === "preflight" }); + await started.promise; + let finished = false; + void session.serving.then(() => { + finished = true; + }); + if (phase === "output-close") session.output.destroy(); + else if (phase === "output-error") + session.output.destroy(new Error("synthetic broken pipe")); + else if (phase === "scan") session.input.destroy(); + else session.input.end(); + await canceled.promise; + expect(finished).toBe(false); + finishCleanup.resolve(); + expect(await session.serving).toBe(0); + if (phase === "scan") + expect(session.stderr.text()).toContain( + "Partial output was kept at /synthetic/partial", + ); + } + }); + + test("server signals cancel scans and remove signal handlers", async () => { + for (const [signal, exitCode] of [ + ["SIGINT", 130], + ["SIGTERM", 143], + ] as const) { + const signals = new FakeSignals(); + const started = Promise.withResolvers(); + const deps = dependencies({ signals }); + deps.createSecurity = () => ({ + run: async (_repository, options) => { + started.resolve(); + await new Promise((resolve) => + options!.signal!.addEventListener("abort", () => resolve(), { + once: true, + }), + ); + return fakeResult(); + }, + preflight: async () => fakePreflight(), + close: async () => {}, + }); + const session = await connect(deps); + session.call("scan"); + await started.promise; + signals.emit(signal); + expect(await session.serving).toBe(exitCode); + expect(signals.listeners.get("SIGINT")?.size).toBe(0); + expect(signals.listeners.get("SIGTERM")?.size).toBe(0); + } + }); +}); diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index 2cda6d522..8e27dee17 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -40,7 +40,6 @@ import { resolveCliPath, } from "../src/cli.js"; import { scanPreflightCodexConfig } from "../src/api.js"; -import { CODEX_EXECUTABLE_VERSION, CODEX_SDK_VERSION } from "../src/version.js"; import { DEFAULT_CODEX_CONFIG, FIREWORKS_CODEX_PROVIDER, @@ -1065,60 +1064,6 @@ describe("CLI", () => { ); }); - test("exposes only typed, read-only SDK metadata over MCP", () => { - const child = spawnSync( - process.execPath, - [join(import.meta.dir, "../src/cli.ts"), "--mcp"], - { - encoding: "utf8", - input: [ - '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"codex-security-test","version":"1.0.0"}}}', - '{"jsonrpc":"2.0","method":"notifications/initialized","params":{}}', - '{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}', - '{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"info","arguments":{}}}', - "", - ].join("\n"), - timeout: 30_000, - }, - ); - expect(child.status).toBe(0); - const responses = child.stdout - .trim() - .split("\n") - .map((line) => JSON.parse(line)); - const tools = responses.find((response) => response.id === 2).result.tools; - expect(tools).toHaveLength(1); - expect(tools[0]).toMatchObject({ - name: "info", - annotations: { - readOnlyHint: true, - idempotentHint: true, - destructiveHint: false, - openWorldHint: false, - }, - outputSchema: { - properties: { - sdkVersion: { type: "string" }, - bundledPluginVersion: { type: "string" }, - scanMcp: { const: false }, - cancellationNote: { type: "string" }, - }, - }, - }); - const metadata = responses.find((response) => response.id === 3).result; - expect(metadata.structuredContent).toMatchObject({ - sdkVersion: VERSION, - bundledPluginVersion: BUNDLED_PLUGIN_VERSION, - scanMcp: false, - cliVersion: VERSION, - codexVersion: CODEX_EXECUTABLE_VERSION, - codexSdkVersion: CODEX_SDK_VERSION, - model: "gpt-5.6-sol", - reasoningEffort: "xhigh", - nextStep: "codex-security scan . --dry-run", - }); - }, 30_000); - test("presents interactive scan history and hides abandoned running scans", async () => { const stdout = capture(true); const scan = { @@ -1483,7 +1428,7 @@ describe("CLI", () => { expect(JSON.parse(stdout.text())).toMatchObject({ sdkVersion: VERSION, bundledPluginVersion: BUNDLED_PLUGIN_VERSION, - scanMcp: false, + scanMcp: true, }); expect(stderr.text()).toBe(""); expect(started).toBe(false);