mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
Node 24 refuses execFile of npm.cmd without a shell (nodejs/node#52554), so embedded-service install (9Router/CLIProxy) failed with spawn EINVAL on Windows. runNpm now enables shell on win32 only; to stay Hard-Rule-#13 safe under a shell, the install --prefix is passed via npm_config_prefix (env) instead of an argv path (survives spaces), and the user-supplied version is constrained by SERVICE_VERSION_PATTERN at the route boundary.
This commit is contained in:
committed by
GitHub
parent
e9c739184e
commit
29bdb8dfde
@@ -6,6 +6,10 @@
|
||||
|
||||
## [3.8.42] — TBD
|
||||
|
||||
### 🔧 Bug Fixes
|
||||
|
||||
- **services (installer):** fix `spawn EINVAL` when installing an embedded service (9Router / CLIProxy) on **Windows + Node.js 24+**. Node 24 stopped letting `child_process.execFile()` run `.cmd` batch files without a shell (nodejs/node#52554), and npm on Windows is `npm.cmd`, so `runNpm()` threw `EINVAL` the moment a user clicked **Install**. `runNpm` now enables `shell` on win32 only. To keep Hard Rule #13 intact under a shell — where the shell, not `execFile`, parses argv — the install `--prefix` (a `DATA_DIR` path that can legitimately contain spaces, e.g. `C:\Users\John Doe\.omniroute\…`) is now passed via the `npm_config_prefix` **environment variable** instead of an argv path, and the user-supplied install `version` is constrained to a dist-tag/semver shape (`SERVICE_VERSION_PATTERN`) at the route boundary so it can never carry shell metacharacters. With the prefix in the environment and the version validated, every remaining argv entry is a static flag. Regression guards: `tests/unit/services/installers/runNpm-shell-5379.test.ts` (+ existing `ninerouter.test.ts` aligned to npm's `npm_config_prefix` env). ([#5379](https://github.com/diegosouzapw/OmniRoute/issues/5379))
|
||||
|
||||
---
|
||||
|
||||
## [3.8.41] — 2026-06-29
|
||||
|
||||
@@ -1,11 +1,15 @@
|
||||
import { z } from "zod";
|
||||
import { install, InstallResult } from "@/lib/services/installers/ninerouter";
|
||||
import { InstallError } from "@/lib/services/installers/utils";
|
||||
import { InstallError, SERVICE_VERSION_PATTERN } from "@/lib/services/installers/utils";
|
||||
import { createErrorResponse } from "@/lib/api/errorResponse";
|
||||
import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
|
||||
|
||||
const BodySchema = z.object({
|
||||
version: z.string().optional().default("latest"),
|
||||
version: z
|
||||
.string()
|
||||
.regex(SERVICE_VERSION_PATTERN, "Invalid version: only letters, digits and . _ + - are allowed")
|
||||
.optional()
|
||||
.default("latest"),
|
||||
});
|
||||
|
||||
export async function POST(request: Request): Promise<Response> {
|
||||
|
||||
@@ -1,11 +1,15 @@
|
||||
import { z } from "zod";
|
||||
import { install, InstallResult } from "@/lib/services/installers/cliproxy";
|
||||
import { InstallError } from "@/lib/services/installers/utils";
|
||||
import { InstallError, SERVICE_VERSION_PATTERN } from "@/lib/services/installers/utils";
|
||||
import { createErrorResponse } from "@/lib/api/errorResponse";
|
||||
import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
|
||||
|
||||
const BodySchema = z.object({
|
||||
version: z.string().optional().default("latest"),
|
||||
version: z
|
||||
.string()
|
||||
.regex(SERVICE_VERSION_PATTERN, "Invalid version: only letters, digits and . _ + - are allowed")
|
||||
.optional()
|
||||
.default("latest"),
|
||||
});
|
||||
|
||||
export async function POST(request: Request): Promise<Response> {
|
||||
|
||||
@@ -77,16 +77,10 @@ export async function install(version = "latest"): Promise<InstallResult> {
|
||||
}
|
||||
|
||||
await runNpm(
|
||||
[
|
||||
"install",
|
||||
`${NINEROUTER_PACKAGE}@${version}`,
|
||||
"--omit=dev",
|
||||
"--no-audit",
|
||||
"--no-fund",
|
||||
"--prefix",
|
||||
NINEROUTER_INSTALL_DIR,
|
||||
],
|
||||
{ cwd: NINEROUTER_INSTALL_DIR }
|
||||
["install", `${NINEROUTER_PACKAGE}@${version}`, "--omit=dev", "--no-audit", "--no-fund"],
|
||||
// `--prefix` is passed via `prefix` (→ npm_config_prefix env) instead of an
|
||||
// argv path so an install dir with spaces survives the Windows shell (#5379).
|
||||
{ cwd: NINEROUTER_INSTALL_DIR, prefix: NINEROUTER_INSTALL_DIR }
|
||||
);
|
||||
|
||||
const installedVersion = await getInstalledVersion();
|
||||
|
||||
@@ -73,10 +73,68 @@ function classifyError(
|
||||
return new InstallError(raw, `Falha na instalação: ${raw}`, 500);
|
||||
}
|
||||
|
||||
/** Runs npm with the given args array. Never uses shell interpolation. */
|
||||
/**
|
||||
* Validates a user-supplied service version (npm dist-tag or semver). Constrained
|
||||
* to letters, digits and `. _ + -`, with a leading alphanumeric, so the value can
|
||||
* never carry shell metacharacters once `runNpm` runs under a shell on Windows
|
||||
* (see `buildNpmExecOptions`). Accepts `latest`, `next`, `1.2.3`, `1.2.3-beta.1`,
|
||||
* `1.2.3+build.5`; rejects `latest && calc`, `$(id)`, spaces, leading `-`, etc.
|
||||
*/
|
||||
export const SERVICE_VERSION_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._+-]*$/;
|
||||
|
||||
export interface NpmExecOptions {
|
||||
cwd?: string;
|
||||
timeout: number;
|
||||
env: NodeJS.ProcessEnv;
|
||||
maxBuffer: number;
|
||||
shell?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds the `execFile` options for {@link runNpm}.
|
||||
*
|
||||
* On Windows, npm is `npm.cmd` (a batch wrapper). Node 24 refuses to `execFile`
|
||||
* a `.cmd` without a shell (nodejs/node#52554 — manifests as `spawn EINVAL`, see
|
||||
* issue #5379), so we enable `shell` on win32 only.
|
||||
*
|
||||
* Enabling the shell means the shell — not `execFile` — splits the command line,
|
||||
* so NO runtime value may be interpolated into argv (Hard Rule #13). The install
|
||||
* prefix (a DATA_DIR path that can legitimately contain spaces, e.g.
|
||||
* `C:\Users\John Doe\.omniroute\…`) is therefore exported as the
|
||||
* `npm_config_prefix` environment variable — npm's documented env form of
|
||||
* `--prefix` — never as an argv entry. With the prefix moved to the environment
|
||||
* and the version constrained by {@link SERVICE_VERSION_PATTERN}, every remaining
|
||||
* argv entry is a static, metacharacter-free flag.
|
||||
*/
|
||||
export function buildNpmExecOptions(
|
||||
platform: NodeJS.Platform,
|
||||
options: { cwd?: string; timeoutMs: number; prefix?: string }
|
||||
): NpmExecOptions {
|
||||
const env: NodeJS.ProcessEnv = { ...process.env };
|
||||
if (options.prefix) {
|
||||
env.npm_config_prefix = options.prefix;
|
||||
}
|
||||
const execOptions: NpmExecOptions = {
|
||||
cwd: options.cwd,
|
||||
timeout: options.timeoutMs,
|
||||
env,
|
||||
maxBuffer: 10 * 1024 * 1024, // 10 MB for npm output
|
||||
};
|
||||
if (platform === "win32") {
|
||||
execOptions.shell = true;
|
||||
}
|
||||
return execOptions;
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs npm with the given args array. Never uses shell interpolation: argv holds
|
||||
* only static flags, and any install prefix is passed via `options.prefix`
|
||||
* (exported as `npm_config_prefix`), not as an argv path. See
|
||||
* {@link buildNpmExecOptions} for the Windows/Node-24 shell handling.
|
||||
*/
|
||||
export function runNpm(
|
||||
args: string[],
|
||||
options: { cwd?: string; timeoutMs?: number } = {}
|
||||
options: { cwd?: string; timeoutMs?: number; prefix?: string } = {}
|
||||
): Promise<NpmRunResult> {
|
||||
const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS;
|
||||
// On Windows, npm is npm.cmd; on Unix it's npm.
|
||||
@@ -86,12 +144,11 @@ export function runNpm(
|
||||
execFile(
|
||||
npmBin,
|
||||
args,
|
||||
{
|
||||
buildNpmExecOptions(process.platform, {
|
||||
cwd: options.cwd,
|
||||
timeout: timeoutMs,
|
||||
env: process.env,
|
||||
maxBuffer: 10 * 1024 * 1024, // 10 MB for npm output
|
||||
},
|
||||
timeoutMs,
|
||||
prefix: options.prefix,
|
||||
}),
|
||||
(err, stdout, stderr) => {
|
||||
if (err) {
|
||||
const classified = classifyError(
|
||||
|
||||
@@ -24,11 +24,14 @@ set -e
|
||||
CMD="$1"
|
||||
shift
|
||||
if [ "$CMD" = "install" ]; then
|
||||
# Find --prefix arg value
|
||||
# Resolve the install prefix like real npm: an explicit --prefix arg wins,
|
||||
# otherwise fall back to the npm_config_prefix env var (#5379 passes the
|
||||
# prefix via env instead of argv so paths with spaces survive the win shell).
|
||||
PREFIX=""
|
||||
while [ $# -gt 0 ]; do
|
||||
if [ "$1" = "--prefix" ]; then PREFIX="$2"; shift 2; else shift; fi
|
||||
done
|
||||
if [ -z "$PREFIX" ]; then PREFIX="$npm_config_prefix"; fi
|
||||
PKG_DIR="$PREFIX/node_modules/9router"
|
||||
mkdir -p "$PKG_DIR/app"
|
||||
echo '{"name":"9router","version":"0.4.59"}' > "$PKG_DIR/package.json"
|
||||
|
||||
74
tests/unit/services/installers/runNpm-shell-5379.test.ts
Normal file
74
tests/unit/services/installers/runNpm-shell-5379.test.ts
Normal file
@@ -0,0 +1,74 @@
|
||||
/**
|
||||
* Regression tests for #5379 — `spawn EINVAL` installing embedded services
|
||||
* (9Router / CLIProxy) on Windows + Node.js 24+.
|
||||
*
|
||||
* Node 24 no longer lets `child_process.execFile()` run `.cmd` batch files on
|
||||
* Windows without a shell (nodejs/node#52554). npm on Windows is `npm.cmd`, so
|
||||
* `runNpm()` threw `EINVAL` immediately. The fix flips `shell` on win32.
|
||||
*
|
||||
* Because `shell: true` makes the shell — not execFile — parse the command line,
|
||||
* NO runtime value may be interpolated into argv (Hard Rule #13). The install
|
||||
* `--prefix` (a DATA_DIR path that can contain spaces, e.g.
|
||||
* `C:\Users\John Doe\.omniroute\…`) is therefore passed via the
|
||||
* `npm_config_prefix` environment variable instead of an argv entry, and the
|
||||
* user-supplied install `version` is constrained by SERVICE_VERSION_PATTERN.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import {
|
||||
buildNpmExecOptions,
|
||||
SERVICE_VERSION_PATTERN,
|
||||
} from "../../../../src/lib/services/installers/utils.ts";
|
||||
|
||||
test("buildNpmExecOptions: win32 enables shell so npm.cmd runs on Node 24 (#5379)", () => {
|
||||
const opts = buildNpmExecOptions("win32", { timeoutMs: 1000 });
|
||||
assert.equal(opts.shell, true);
|
||||
});
|
||||
|
||||
test("buildNpmExecOptions: non-win32 platforms never enable shell", () => {
|
||||
for (const platform of ["linux", "darwin", "freebsd"] as NodeJS.Platform[]) {
|
||||
const opts = buildNpmExecOptions(platform, { timeoutMs: 1000 });
|
||||
assert.equal(opts.shell, undefined, `${platform} must not use a shell`);
|
||||
}
|
||||
});
|
||||
|
||||
test("buildNpmExecOptions: prefix is passed via npm_config_prefix env, never argv (Hard Rule #13)", () => {
|
||||
const prefix = "C:\\Users\\John Doe\\.omniroute\\services\\9router";
|
||||
const opts = buildNpmExecOptions("win32", { timeoutMs: 1000, prefix });
|
||||
assert.equal(opts.env.npm_config_prefix, prefix);
|
||||
});
|
||||
|
||||
test("buildNpmExecOptions: without a prefix npm_config_prefix is left untouched", () => {
|
||||
const inherited = process.env.npm_config_prefix;
|
||||
const opts = buildNpmExecOptions("linux", { timeoutMs: 1000 });
|
||||
assert.equal(opts.env.npm_config_prefix, inherited);
|
||||
});
|
||||
|
||||
test("buildNpmExecOptions: carries cwd, timeout and maxBuffer through", () => {
|
||||
const opts = buildNpmExecOptions("linux", { cwd: "/tmp/install", timeoutMs: 4242 });
|
||||
assert.equal(opts.cwd, "/tmp/install");
|
||||
assert.equal(opts.timeout, 4242);
|
||||
assert.equal(opts.maxBuffer, 10 * 1024 * 1024);
|
||||
});
|
||||
|
||||
test("SERVICE_VERSION_PATTERN: accepts dist-tags and semver", () => {
|
||||
for (const v of ["latest", "next", "1.2.3", "1.2.3-beta.1", "1.2.3+build.5", "0.4.59"]) {
|
||||
assert.ok(SERVICE_VERSION_PATTERN.test(v), `${v} should be valid`);
|
||||
}
|
||||
});
|
||||
|
||||
test("SERVICE_VERSION_PATTERN: rejects shell metacharacters (injection guard)", () => {
|
||||
for (const v of [
|
||||
"latest && calc",
|
||||
"1.2.3; rm -rf /",
|
||||
"$(whoami)",
|
||||
"`id`",
|
||||
"a|b",
|
||||
"a b",
|
||||
"",
|
||||
"-flag",
|
||||
]) {
|
||||
assert.equal(SERVICE_VERSION_PATTERN.test(v), false, `${JSON.stringify(v)} must be rejected`);
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user