mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-21 22:52:19 +03:00
fix(security): reject eval-style version-probe args for custom ACP agents
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.
This commit is contained in:
@@ -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 };
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user