diff --git a/src/lib/acp/registry.ts b/src/lib/acp/registry.ts index 93315a546d..07408f6236 100644 --- a/src/lib/acp/registry.ts +++ b/src/lib/acp/registry.ts @@ -200,6 +200,14 @@ let _customAgentDefs: CustomAgentDef[] = []; const DISALLOWED_VERSION_COMMAND_CHARS = /[;&|<>`$\r\n]/; +// A version probe only ever needs a version flag. For untrusted (client-registered) +// custom agents the binary-match check alone is not enough: the caller controls both +// `binary` and `versionCommand`, so a matching interpreter with an eval-style argument +// (`node -e …`, `python -c …`, `ruby -e …`) reaches execFileSync as arbitrary code +// execution without any shell metacharacter. Restricting the args to a recognized +// version flag closes that path — see GHSA-jphr-2gw7-xrwp / GHSA-hf57-cqmx-p4gr. +const SAFE_VERSION_PROBE_ARG = /^(-v|-V|--version|-version|version|--ver)$/; + /** * Set custom agent definitions from settings. */ @@ -300,6 +308,12 @@ export function resolveVersionProbe( if (!allowed.has(normalizedCommand)) { return null; } + + // Untrusted probe: allow only a bare binary or a single recognized version + // flag, so a matching interpreter cannot smuggle an eval/exec argument. + if (args.length > 1 || (args.length === 1 && !SAFE_VERSION_PROBE_ARG.test(args[0]))) { + return null; + } } return { command, args }; diff --git a/tests/unit/acp-agents-route.test.ts b/tests/unit/acp-agents-route.test.ts index 699cb607bd..ab2e118dc8 100644 --- a/tests/unit/acp-agents-route.test.ts +++ b/tests/unit/acp-agents-route.test.ts @@ -100,3 +100,32 @@ test("POST /api/acp/agents rejects unsafe version commands for authenticated ses assert.equal(response.status, 400); assert.match(body.error, /Invalid versionCommand/i); }); + +test("POST /api/acp/agents rejects an interpreter eval payload (GHSA-jphr-2gw7-xrwp)", async () => { + // Exact shape of the advisory PoC: binary + versionCommand both name `node`, + // so the binary-match check passes, but the `-e` eval argument must still be + // refused before it can reach execFileSync("node", ["-e", ...]). + process.env.JWT_SECRET = "acp-agents-jwt-secret"; + await localDb.updateSettings({ requireLogin: true, password: "hashed-password" }); + const token = await createSessionToken(); + + const response = await routeModule.POST( + makeRequest( + "POST", + { + id: "anonrce", + name: "anonrce", + binary: "node", + versionCommand: 'node -e "process.exit(1)"', + providerAlias: "anonrce", + spawnArgs: [], + protocol: "stdio", + }, + token + ) + ); + const body = (await response.json()) as any; + + assert.equal(response.status, 400); + assert.match(body.error, /Invalid versionCommand/i); +}); diff --git a/tests/unit/acp-registry.test.ts b/tests/unit/acp-registry.test.ts index deb14d709b..ed8dff978e 100644 --- a/tests/unit/acp-registry.test.ts +++ b/tests/unit/acp-registry.test.ts @@ -32,6 +32,41 @@ test("resolveVersionProbe rejects shell metacharacters in version commands", () assert.equal(probe, null); }); +// Regression guard — GHSA-jphr-2gw7-xrwp / GHSA-hf57-cqmx-p4gr (ACP custom-agent +// RCE). A client-registered custom agent controls both `binary` and +// `versionCommand`; the binary-match check alone still admits an eval-style +// argument on a matching interpreter, which reaches execFileSync as arbitrary +// code execution (no shell metacharacter required). A version *probe* only ever +// needs a version flag, so untrusted probes must reject non-version arguments. +test("resolveVersionProbe rejects interpreter eval arguments on a matching binary", () => { + assert.equal(resolveVersionProbe("node", 'node -e "process.exit(1)"', true), null); + assert.equal(resolveVersionProbe("node", "node --eval 1", true), null); + assert.equal(resolveVersionProbe("python3", 'python3 -c "import os"', true), null); + assert.equal(resolveVersionProbe("ruby", 'ruby -e "puts 1"', true), null); + // Any extra argument beyond a single version flag is refused for a probe. + assert.equal(resolveVersionProbe("node", "node --version --eval 1", true), null); +}); + +test("resolveVersionProbe still accepts legitimate version flags for custom agents", () => { + assert.deepEqual(resolveVersionProbe("node", "node --version", true), { + command: "node", + args: ["--version"], + }); + assert.deepEqual(resolveVersionProbe("my-agent", "my-agent -v", true), { + command: "my-agent", + args: ["-v"], + }); + assert.deepEqual(resolveVersionProbe("my-agent", "my-agent version", true), { + command: "my-agent", + args: ["version"], + }); + // Bare binary with no arguments is a valid probe too. + assert.deepEqual(resolveVersionProbe("my-agent", "my-agent", true), { + command: "my-agent", + args: [], + }); +}); + test("shouldUseShellForVersionProbe preserves Windows npm wrapper detection", () => { assert.equal(shouldUseShellForVersionProbe("codex", "win32"), true); assert.equal(