diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e4681389d5..5abe53398a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -215,6 +215,7 @@ Docs: https://docs.openclaw.ai - QQBot: preserve the framework command authorization decision when converting framework command contexts into engine slash command contexts, so downstream slash handlers see `commandAuthorized` matching the channel's resolved `isAuthorizedSender` instead of a hardcoded `true`. (#77453) Thanks @drobison00. - Security/Windows: block `LOCALAPPDATA` from workspace `.env` and resolve Windows update-flow portable Git path prepends from the trusted process-local `LOCALAPPDATA` only, so workspace-supplied values cannot redirect `git` discovery during `openclaw update`. (#77470) Thanks @drobison00. - Browser/SSRF: enforce the existing current-tab URL navigation policy before tab-scoped debug, export, and read routes (console, page errors, network requests, trace start/stop, response body, screenshot, snapshot, storage, etc.) collect from an already-selected tab, so blocked tabs return a policy error instead of being read first and redacted only at response time. (#75731) Thanks @eleqtrizit. +- Security/Windows: route the `.cmd`/`.bat` process wrapper through the shared Windows install-root resolver instead of `process.env.ComSpec`, so workspace dotenv-blocked `SystemRoot`/`WINDIR` overrides and unsafe values like UNC paths or path-lists cannot redirect `cmd.exe` selection on Windows. (#77472) Thanks @drobison00. ## 2026.5.3-1 diff --git a/src/process/exec.ts b/src/process/exec.ts index 6907b63cc2e..d0bd312f113 100644 --- a/src/process/exec.ts +++ b/src/process/exec.ts @@ -9,6 +9,7 @@ import { decodeWindowsOutputBuffer, resolveWindowsConsoleEncoding, } from "../infra/windows-encoding.js"; +import { getWindowsInstallRoots } from "../infra/windows-install-roots.js"; import { logDebug, logError } from "../logger.js"; import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js"; import { resolveCommandStdio } from "./spawn-utils.js"; @@ -45,6 +46,13 @@ function buildCmdExeCommandLine(resolvedCommand: string, args: string[]): string return [escapeForCmdExe(resolvedCommand), ...args.map(escapeForCmdExe)].join(" "); } +function resolveTrustedWindowsCmdExe(): string { + if (process.platform !== "win32") { + return "cmd.exe"; + } + return path.win32.join(getWindowsInstallRoots().systemRoot, "System32", "cmd.exe"); +} + /** * On Windows, Node 18.20.2+ (CVE-2024-27980) rejects spawning .cmd/.bat directly * without shell, causing EINVAL. Resolve npm/npx to node + cli script so we @@ -107,7 +115,7 @@ function resolveChildProcessInvocation(params: { const useCmdWrapper = isWindowsBatchCommand(resolvedCommand); return { - command: useCmdWrapper ? (process.env.ComSpec ?? "cmd.exe") : resolvedCommand, + command: useCmdWrapper ? resolveTrustedWindowsCmdExe() : resolvedCommand, args: useCmdWrapper ? ["/d", "/s", "/c", buildCmdExeCommandLine(resolvedCommand, finalArgv.slice(1))] : finalArgv.slice(1), diff --git a/src/process/exec.windows.test.ts b/src/process/exec.windows.test.ts index 9be869c0bb8..7feccf18e81 100644 --- a/src/process/exec.windows.test.ts +++ b/src/process/exec.windows.test.ts @@ -3,6 +3,10 @@ import { EventEmitter } from "node:events"; import fs from "node:fs"; import path from "node:path"; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { + _resetWindowsInstallRootsForTests, + getWindowsInstallRoots, +} from "../infra/windows-install-roots.js"; const { spawnMock, spawnSyncMock, execFileMock, execFilePromisifyMock } = vi.hoisted(() => { const execFilePromisifyMock = vi.fn(); @@ -101,6 +105,10 @@ function expectCmdWrappedInvocation(params: { expect(params.captured[2].windowsVerbatimArguments).toBe(true); } +function expectedTrustedCmdExe(): string { + return path.win32.join(getWindowsInstallRoots().systemRoot, "System32", "cmd.exe"); +} + async function expectShimmedWindowsCommandWithoutExitCodeSucceeds(params?: { killed?: boolean }) { const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); const child = createMockChild({ @@ -127,6 +135,10 @@ describe("windows command wrapper behavior", () => { }); beforeEach(() => { + // Stub the registry probe so install-root resolution is fully driven by + // process.env in tests; on real Windows runners the registry returns the + // canonical SystemRoot and would shadow the test's env setup. + _resetWindowsInstallRootsForTests({ queryRegistryValue: () => null }); spawnMock.mockReset(); spawnSyncMock.mockReset(); spawnSyncMock.mockReturnValue({ stdout: "Active code page: 936", stderr: "" }); @@ -157,7 +169,7 @@ describe("windows command wrapper behavior", () => { it("wraps .cmd commands via cmd.exe in runCommandWithTimeout", async () => { const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); - const expectedComSpec = process.env.ComSpec ?? "cmd.exe"; + const expectedComSpec = expectedTrustedCmdExe(); spawnMock.mockImplementation( (_command: string, _args: string[], _options: Record) => createMockChild(), @@ -173,9 +185,91 @@ describe("windows command wrapper behavior", () => { } }); + it("ignores ComSpec when selecting the Windows command wrapper", async () => { + const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + const previousComSpec = process.env.ComSpec; + const previousSystemRoot = process.env.SystemRoot; + process.env.ComSpec = "C:\\workspace\\evil\\cmd.exe"; + process.env.SystemRoot = "C:\\Windows"; + + spawnMock.mockImplementation( + (_command: string, _args: string[], _options: Record) => createMockChild(), + ); + + try { + const result = await runCommandWithTimeout(["pnpm", "--version"], { timeoutMs: 1000 }); + expect(result.code).toBe(0); + const captured = spawnMock.mock.calls[0] as SpawnCall | undefined; + expectCmdWrappedInvocation({ + captured, + expectedComSpec: path.win32.join("C:\\Windows", "System32", "cmd.exe"), + }); + } finally { + if (previousComSpec === undefined) { + delete process.env.ComSpec; + } else { + process.env.ComSpec = previousComSpec; + } + if (previousSystemRoot === undefined) { + delete process.env.SystemRoot; + } else { + process.env.SystemRoot = previousSystemRoot; + } + platformSpy.mockRestore(); + } + }); + + it("rejects unsafe Windows root values when selecting the command wrapper", async () => { + const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + const previousSystemRoot = process.env.SystemRoot; + const previousWindir = process.env.WINDIR; + + spawnMock.mockImplementation( + (_command: string, _args: string[], _options: Record) => createMockChild(), + ); + + try { + for (const unsafeRoot of [ + "\\\\evil\\share", + "C:\\Windows;C:\\evil", + "\\Windows", + "relative\\path", + ]) { + _resetWindowsInstallRootsForTests({ queryRegistryValue: () => null }); + // Set every install-root env source to the unsafe value so the + // resolver rejects each one and falls through to the safe default. + // Deleting WINDIR here is unreliable on real Windows runners, so + // overwrite it with the same rejected payload. + process.env.SystemRoot = unsafeRoot; + process.env.WINDIR = unsafeRoot; + spawnMock.mockClear(); + + const result = await runCommandWithTimeout(["pnpm", "--version"], { timeoutMs: 1000 }); + expect(result.code).toBe(0); + const captured = spawnMock.mock.calls[0] as SpawnCall | undefined; + expectCmdWrappedInvocation({ + captured, + expectedComSpec: path.win32.join("C:\\Windows", "System32", "cmd.exe"), + }); + } + } finally { + if (previousSystemRoot === undefined) { + delete process.env.SystemRoot; + } else { + process.env.SystemRoot = previousSystemRoot; + } + if (previousWindir === undefined) { + delete process.env.WINDIR; + } else { + process.env.WINDIR = previousWindir; + } + platformSpy.mockRestore(); + } + }); + it("wraps corepack.cmd via cmd.exe in runCommandWithTimeout", async () => { const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); - const expectedComSpec = process.env.ComSpec ?? "cmd.exe"; + const expectedComSpec = expectedTrustedCmdExe(); spawnMock.mockImplementation( (_command: string, _args: string[], _options: Record) => createMockChild(), @@ -243,7 +337,7 @@ describe("windows command wrapper behavior", () => { it("falls back to npm.cmd when npm-cli.js is unavailable", async () => { const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); const existsSpy = vi.spyOn(fs, "existsSync").mockReturnValue(false); - const expectedComSpec = process.env.ComSpec ?? "cmd.exe"; + const expectedComSpec = expectedTrustedCmdExe(); spawnMock.mockImplementation( (_command: string, _args: string[], _options: Record) => createMockChild(), @@ -297,7 +391,7 @@ describe("windows command wrapper behavior", () => { it("uses cmd.exe wrapper with windowsVerbatimArguments in runExec for .cmd shims", async () => { const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); - const expectedComSpec = process.env.ComSpec ?? "cmd.exe"; + const expectedComSpec = expectedTrustedCmdExe(); execFileMock.mockImplementation( (