From 64b8ffffc1ea44ac6e275c00f74b5a0b58c23698 Mon Sep 17 00:00:00 2001
From: Xiangzhe <bakryun0718@proton.me>
Date: Fri, 21 Aug 2026 13:01:10 -0300
Subject: [PATCH] fix(security): reject eval-style version-probe args for
 custom ACP agents
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

POST /api/acp/agents lets a client register a custom agent controlling both
`binary` and `versionCommand`. The version probe runs execFileSync(binary,
args); the binary-match check alone still admits an eval argument on a
matching interpreter (`node -e …`, `python -c …`, `ruby -e …`), which is
arbitrary code execution with no shell metacharacter. `/api/acp/agents` is
already LOCAL_ONLY (#7948) so the remote/anonymous vector is closed, but a
loopback/LAN caller with requireLogin=false — or any authenticated caller —
could still reach the sink.

resolveVersionProbe() now restricts untrusted (requireBinaryMatch) probes to
a bare binary or a single recognized version flag, so no code-running argument
can pass. Built-in agents (requireBinaryMatch=false) are unaffected.

Reported by @c111mb3r via GHSA-jphr-2gw7-xrwp and GHSA-hf57-cqmx-p4gr.
---
 src/lib/acp/registry.ts             | 14 ++++++++++++
 tests/unit/acp-agents-route.test.ts | 29 ++++++++++++++++++++++++
 tests/unit/acp-registry.test.ts     | 35 +++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+)

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(