fix(cli): harden RDE dashboard state + Windows auto-update re-exec

Address PR #1521 review:

- dev-env-state: validate localDashboard on read (mirroring the
  cliUpdateCheck cache). A hand-edited/cross-version state file with a
  non-string version reached parseVersionCore (version.trim()) via
  shouldRestartDashboard -> isVersionNewer inside startDashboardIfNeeded,
  which is outside the auto-update fail-open guard and would crash
  `stack dev`. Malformed records are now treated as 'no dashboard'.

- self-update: re-exec via npx.cmd on Windows now spawns with shell:true.
  After CVE-2024-27980 Node throws EINVAL spawning a .cmd directly without
  a shell, so the auto-update re-exec silently never ran on Windows. args
  stay a clean argv array; runReexec quotes them for the shell at spawn
  time so paths/args with spaces survive.
This commit is contained in:
Bilal Godil 2026-06-01 14:47:17 -07:00
parent 6967742524
commit 6aac337428
4 changed files with 97 additions and 6 deletions

View File

@ -158,6 +158,38 @@ describe("dev env state", () => {
expect(state.localDashboard?.version).toBeUndefined();
});
it("drops a localDashboard whose version is a non-string", () => {
useTempStateFile();
const statePath = process.env.STACK_DEV_ENVS_PATH;
if (statePath == null) {
throw new Error("STACK_DEV_ENVS_PATH should be set by useTempStateFile().");
}
// A hand-edited / cross-version file with a non-string version would
// otherwise reach parseVersionCore (version.trim()) and throw, crashing
// `stack dev` outside the auto-update fail-open guard. Treat as no record.
writeFileSync(statePath, JSON.stringify({
version: 1,
localDashboard: { port: 26700, secret: "s", pid: 999, startedAtMillis: 1, version: 2 },
projectsByConfigPath: {},
}), { mode: 0o600 });
expect(readDevEnvState().localDashboard).toBeUndefined();
});
it("drops a structurally malformed localDashboard on read", () => {
useTempStateFile();
const statePath = process.env.STACK_DEV_ENVS_PATH;
if (statePath == null) {
throw new Error("STACK_DEV_ENVS_PATH should be set by useTempStateFile().");
}
// Missing secret + non-numeric pid: not a usable dashboard record.
writeFileSync(statePath, JSON.stringify({
version: 1,
localDashboard: { port: 26700, pid: "nope", startedAtMillis: 1 },
projectsByConfigPath: {},
}), { mode: 0o600 });
expect(readDevEnvState().localDashboard).toBeUndefined();
});
it("writes state as owner-readable JSON", () => {
useTempStateFile();
writeDevEnvState({

View File

@ -54,6 +54,31 @@ function isCliUpdateCheckCache(value: unknown): value is CliUpdateCheckCache {
);
}
type LocalDashboardState = NonNullable<DevEnvState["localDashboard"]>;
// Validate the on-disk dashboard record, mirroring isCliUpdateCheckCache: a
// hand-edited or cross-version state file could carry wrong-typed fields. In
// particular a non-string `version` flows into shouldRestartDashboard ->
// isVersionNewer -> parseVersionCore (version.trim()) inside
// startDashboardIfNeeded, which is not behind the auto-update fail-open guard,
// so it would throw and crash `stack dev`. Treat anything malformed as "no
// dashboard recorded" (a fresh one is then started).
function isLocalDashboardState(value: unknown): value is LocalDashboardState {
if (value == null || typeof value !== "object") return false;
const candidate = value as Record<string, unknown>;
return (
typeof candidate.port === "number" &&
Number.isFinite(candidate.port) &&
typeof candidate.secret === "string" &&
typeof candidate.pid === "number" &&
Number.isFinite(candidate.pid) &&
typeof candidate.startedAtMillis === "number" &&
Number.isFinite(candidate.startedAtMillis) &&
(candidate.logPath === undefined || typeof candidate.logPath === "string") &&
(candidate.version === undefined || typeof candidate.version === "string")
);
}
export function readDevEnvState(): DevEnvState {
const path = devEnvStatePath();
if (!existsSync(path)) {
@ -70,7 +95,7 @@ export function readDevEnvState(): DevEnvState {
version: 1,
anonymousRefreshToken: typeof parsed.anonymousRefreshToken === "string" ? parsed.anonymousRefreshToken : undefined,
anonymousApiBaseUrl: typeof parsed.anonymousApiBaseUrl === "string" ? parsed.anonymousApiBaseUrl : undefined,
localDashboard: parsed.localDashboard,
localDashboard: isLocalDashboardState(parsed.localDashboard) ? parsed.localDashboard : undefined,
cliUpdateCheck: isCliUpdateCheckCache(parsed.cliUpdateCheck) ? parsed.cliUpdateCheck : undefined,
projectsByConfigPath: parsed.projectsByConfigPath ?? {},
};

View File

@ -156,12 +156,27 @@ describe("buildNpxInvocation", () => {
]);
});
it("uses npx.cmd on Windows", () => {
it("uses npx.cmd and requests a shell on Windows (needed to spawn a .cmd post-CVE-2024-27980)", () => {
const spy = vi.spyOn(process, "platform", "get").mockReturnValue("win32");
try {
expect(buildNpxInvocation({
const invocation = buildNpxInvocation({
packageName: "@hexclave/cli", version: "1.0.0", binName: "stack", forwardArgs: [],
}).command).toBe("npx.cmd");
});
expect(invocation.command).toBe("npx.cmd");
expect(invocation.shell).toBe(true);
} finally {
spy.mockRestore();
}
});
it("spawns npx directly without a shell off Windows", () => {
const spy = vi.spyOn(process, "platform", "get").mockReturnValue("linux");
try {
const invocation = buildNpxInvocation({
packageName: "@hexclave/cli", version: "1.0.0", binName: "stack", forwardArgs: [],
});
expect(invocation.command).toBe("npx");
expect(invocation.shell).toBe(false);
} finally {
spy.mockRestore();
}

View File

@ -142,6 +142,11 @@ export async function resolveLatestVersion(
export type NpxInvocation = {
command: string,
args: string[],
// Windows' launcher is `npx.cmd`; after CVE-2024-27980 Node refuses to spawn
// a .cmd/.bat directly (EINVAL) unless `shell` is set, so the re-exec has to
// go through the shell there. `args` stays a clean argv array — runReexec
// quotes it for the shell at spawn time.
shell: boolean,
};
export function buildNpxInvocation(opts: {
@ -150,9 +155,11 @@ export function buildNpxInvocation(opts: {
binName: string,
forwardArgs: string[],
}): NpxInvocation {
const command = process.platform === "win32" ? "npx.cmd" : "npx";
const isWindows = process.platform === "win32";
const command = isWindows ? "npx.cmd" : "npx";
return {
command,
shell: isWindows,
args: [
"--yes",
// Override any global npm "cooldown" for this call only — we always want
@ -201,11 +208,23 @@ type ReexecResult =
| { exited: true, code: number }
| { exited: false, error: string };
// Quote an argument for the single cmd.exe command line that Node builds when
// `spawn` runs with `shell: true` on Windows — it joins argv with spaces and
// does not quote, so an unquoted path/arg with a space would be split. Wrap
// anything that isn't a plain token (and the empty string) in double quotes,
// escaping embedded quotes. A no-op on the non-shell (POSIX) path.
function quoteShellArg(arg: string): string {
if (arg !== "" && !/[\s"&|<>^()]/.test(arg)) return arg;
return `"${arg.replace(/"/g, '\\"')}"`;
}
function runReexec(invocation: NpxInvocation): Promise<ReexecResult> {
return new Promise((resolvePromise) => {
const child = spawn(invocation.command, invocation.args, {
const args = invocation.shell ? invocation.args.map(quoteShellArg) : invocation.args;
const child = spawn(invocation.command, args, {
stdio: "inherit",
env: { ...process.env, [SKIP_AUTO_UPDATE_ENV]: "1" },
shell: invocation.shell,
});
const cleanup = forwardSignals(child);