mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
* fix(providers): spawn Auggie CLI with shell:true on win32 (#6304) * chore: sync CHANGELOG to release tip (#6510; bullet re-added at merge)
This commit is contained in:
committed by
GitHub
parent
3aeb87be7b
commit
fd4133df82
@@ -77,6 +77,32 @@ function buildAuggieArgs(model: string): string[] {
|
||||
return ["--print", "--quiet", "--model", model, "--"];
|
||||
}
|
||||
|
||||
/**
|
||||
* Spawn options shared by both auggie spawn sites.
|
||||
*
|
||||
* `shell: true` is required on win32: since Node's CVE-2024-27980 fix
|
||||
* (Node >=18.20.2/20.12.2/21.7.3), `spawn()` refuses to launch `.cmd`/`.bat`
|
||||
* targets (e.g. the global-npm `auggie.cmd` shim resolved by
|
||||
* resolveAuggieBin()'s PATH fallback) without shell interpretation, throwing
|
||||
* `spawn EINVAL`. The argv array (built by buildAuggieArgs()) is always a
|
||||
* fixed literal list plus an allowlist-validated `model` — never
|
||||
* interpolated into a shell string — so enabling `shell` here does not
|
||||
* reopen argument-injection: Node still passes argv as discrete array
|
||||
* elements to the shell, it does not concatenate them into a single
|
||||
* command line.
|
||||
*/
|
||||
export function buildAuggieSpawnOptions(stdio: ["pipe", "pipe", "pipe"]): {
|
||||
env: NodeJS.ProcessEnv;
|
||||
stdio: ["pipe", "pipe", "pipe"];
|
||||
shell: boolean;
|
||||
} {
|
||||
return {
|
||||
env: process.env,
|
||||
stdio,
|
||||
shell: process.platform === "win32",
|
||||
};
|
||||
}
|
||||
|
||||
// ─── Binary discovery ────────────────────────────────────────────────────────
|
||||
|
||||
export function resolveAuggieBin(): string {
|
||||
@@ -267,13 +293,15 @@ export class AuggieExecutor extends BaseExecutor {
|
||||
}
|
||||
|
||||
private spawnAuggie(auggieBin: string, model: string, promptText: string) {
|
||||
// No `shell` option: argv is passed directly to the OS loader, so no cmd.exe
|
||||
// metacharacter interpretation is possible even on Windows. `model` is already
|
||||
// allowlist-validated by resolveAuggieModel() before reaching here.
|
||||
const child = spawn(auggieBin, buildAuggieArgs(model), {
|
||||
env: process.env,
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
});
|
||||
// `shell: true` on win32 only (see buildAuggieSpawnOptions() for why) — argv
|
||||
// stays a fixed literal array; `model` is already allowlist-validated by
|
||||
// resolveAuggieModel() before reaching here, so no argument-injection surface
|
||||
// is reopened by shell interpretation.
|
||||
const child = spawn(
|
||||
auggieBin,
|
||||
buildAuggieArgs(model),
|
||||
buildAuggieSpawnOptions(["pipe", "pipe", "pipe"])
|
||||
);
|
||||
// EPIPE from a fast-exiting CLI arrives ASYNCHRONOUSLY as an 'error' event on
|
||||
// stdin (not a sync throw), so the try/catch below cannot swallow it — without
|
||||
// this handler the unhandled stream error crashes the process instead of
|
||||
@@ -368,12 +396,14 @@ export class AuggieExecutor extends BaseExecutor {
|
||||
|
||||
let child: ReturnType<typeof spawn>;
|
||||
try {
|
||||
// No `shell` option — argv goes straight to the OS loader (no cmd.exe
|
||||
// interpretation). `model` is already allowlist-validated upstream.
|
||||
child = spawn(auggieBin, buildAuggieArgs(model), {
|
||||
env: process.env,
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
});
|
||||
// `shell: true` on win32 only (see buildAuggieSpawnOptions() for why).
|
||||
// `model` is already allowlist-validated upstream, so shell interpretation
|
||||
// does not reopen argument-injection.
|
||||
child = spawn(
|
||||
auggieBin,
|
||||
buildAuggieArgs(model),
|
||||
buildAuggieSpawnOptions(["pipe", "pipe", "pipe"])
|
||||
);
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
emitError(
|
||||
|
||||
56
tests/unit/auggie-win32-spawn-6304.test.ts
Normal file
56
tests/unit/auggie-win32-spawn-6304.test.ts
Normal file
@@ -0,0 +1,56 @@
|
||||
/**
|
||||
* Regression test for #6304 — Auggie (Augment CLI) provider always fails on
|
||||
* Windows with `spawn EINVAL`.
|
||||
*
|
||||
* Root cause: resolveAuggieBin() falls back to "auggie.cmd" (the global-npm
|
||||
* bin shim) on win32. Since Node's CVE-2024-27980 fix (Node >=18.20.2/
|
||||
* 20.12.2/21.7.3), `spawn()` refuses `.cmd`/`.bat` targets without
|
||||
* `shell: true`, throwing `spawn EINVAL`.
|
||||
*
|
||||
* Both auggie spawn sites build their options via the shared
|
||||
* buildAuggieSpawnOptions() helper, so asserting on that helper's output
|
||||
* covers spawnAuggie() (non-streaming path) and the inline spawn in
|
||||
* runStreaming() alike.
|
||||
*/
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
const { buildAuggieSpawnOptions } = await import("@omniroute/open-sse/executors/auggie");
|
||||
|
||||
/** Temporarily override process.platform for the duration of `fn`. */
|
||||
function withPlatform<T>(platform: string, fn: () => T): T {
|
||||
const original = Object.getOwnPropertyDescriptor(process, "platform")!;
|
||||
Object.defineProperty(process, "platform", { value: platform, configurable: true });
|
||||
try {
|
||||
return fn();
|
||||
} finally {
|
||||
Object.defineProperty(process, "platform", original);
|
||||
}
|
||||
}
|
||||
|
||||
test("buildAuggieSpawnOptions sets shell:true on win32 (fixes spawn EINVAL)", () => {
|
||||
const options = withPlatform("win32", () => buildAuggieSpawnOptions(["pipe", "pipe", "pipe"]));
|
||||
assert.equal(
|
||||
options.shell,
|
||||
true,
|
||||
"spawn() must use shell:true on win32 or launching auggie.cmd throws EINVAL " +
|
||||
"(Node CVE-2024-27980 fix)"
|
||||
);
|
||||
});
|
||||
|
||||
test("buildAuggieSpawnOptions leaves shell falsy on posix platforms", () => {
|
||||
for (const platform of ["linux", "darwin"]) {
|
||||
const options = withPlatform(platform, () => buildAuggieSpawnOptions(["pipe", "pipe", "pipe"]));
|
||||
assert.ok(
|
||||
!options.shell,
|
||||
`spawn() should not need shell interpretation on ${platform}`
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test("buildAuggieSpawnOptions forwards the requested stdio and process.env", () => {
|
||||
const options = withPlatform("linux", () => buildAuggieSpawnOptions(["pipe", "pipe", "pipe"]));
|
||||
assert.deepEqual(options.stdio, ["pipe", "pipe", "pipe"]);
|
||||
assert.equal(options.env, process.env);
|
||||
});
|
||||
Reference in New Issue
Block a user