From 7b8315d18e3ccda620e748289a455f0798dc9bf8 Mon Sep 17 00:00:00 2001 From: Michael Appel Date: Mon, 4 May 2026 13:39:00 -0400 Subject: [PATCH] fix: block SystemRoot/WINDIR in workspace .env and harden reg.exe path resolution [AI-assisted] (#74454) * fix: address issue * fix: address PR review feedback * Add changelog entry for PR #74454 --------- Co-authored-by: Devin Robison --- CHANGELOG.md | 1 + src/infra/dotenv.test.ts | 6 +++++ src/infra/windows-install-roots.test.ts | 36 ++++++++++++------------- src/infra/windows-install-roots.ts | 27 ++++--------------- 4 files changed, 30 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c328827d6a..1fa7c3b34af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -203,6 +203,7 @@ Docs: https://docs.openclaw.ai - Agents/subagents: detect prefix-only completion announce replies and fall back to the captured child result so requester chats no longer lose most of long sub-agent reports silently. Fixes #76412. Thanks @inxaos and @davemorin. - TUI: replace the stale-response watchdog notice with plain user-facing copy so stalled replies no longer surface backend or streaming internals. (#77120) Thanks @davemorin. - Security/Windows: validate `SystemRoot`/`WINDIR` env values through the Windows install-root validator and add them to the dangerous-host-env policy when resolving `icacls.exe`/`whoami.exe` for `openclaw security audit`, so workspace `.env` overrides and bare command names cannot redirect Windows ACL helpers to attacker-controlled binaries. (#74458) Thanks @mmaps. +- Security/Windows: pin Windows registry-probe `reg.exe` resolution to the canonical Windows install root in install-root probing, so `SystemRoot`/`WINDIR` env overrides cannot redirect registry queries during Windows host detection. (#74454) Thanks @mmaps. ## 2026.5.3-1 diff --git a/src/infra/dotenv.test.ts b/src/infra/dotenv.test.ts index a2f364bfd41..1beb8e9e19c 100644 --- a/src/infra/dotenv.test.ts +++ b/src/infra/dotenv.test.ts @@ -228,8 +228,10 @@ describe("loadDotEnv", () => { "HTTP_PROXY=http://evil-proxy:8080", "HOMEBREW_BREW_FILE=./evil-brew/bin/brew", "HOMEBREW_PREFIX=./evil-brew", + "SystemRoot=.\\fake-root", "UV_PYTHON=./attacker-python", "uv_python=./attacker-python-lower", + "WINDIR=.\\fake-windir", ].join("\n"), ); await writeEnvFile(path.join(stateDir, ".env"), "BAR=from-global\n"); @@ -245,8 +247,10 @@ describe("loadDotEnv", () => { delete process.env.HTTP_PROXY; delete process.env.HOMEBREW_BREW_FILE; delete process.env.HOMEBREW_PREFIX; + delete process.env.SystemRoot; delete process.env.UV_PYTHON; delete process.env.uv_python; + delete process.env.WINDIR; loadDotEnv({ quiet: true }); @@ -262,8 +266,10 @@ describe("loadDotEnv", () => { expect(process.env.HTTP_PROXY).toBeUndefined(); expect(process.env.HOMEBREW_BREW_FILE).toBeUndefined(); expect(process.env.HOMEBREW_PREFIX).toBeUndefined(); + expect(process.env.SystemRoot).toBeUndefined(); expect(process.env.UV_PYTHON).toBeUndefined(); expect(process.env.uv_python).toBeUndefined(); + expect(process.env.WINDIR).toBeUndefined(); }); }); }); diff --git a/src/infra/windows-install-roots.test.ts b/src/infra/windows-install-roots.test.ts index 55466bb0469..112cf1baca4 100644 --- a/src/infra/windows-install-roots.test.ts +++ b/src/infra/windows-install-roots.test.ts @@ -171,25 +171,25 @@ describe("getWindowsProgramFilesRoots", () => { }); describe("locateWindowsRegExe", () => { - it("prefers SystemRoot and WINDIR candidates over arbitrary drive scans", () => { - expect( - _private.getWindowsRegExeCandidates({ - SystemRoot: "D:\\Windows", - WINDIR: "E:\\Windows", - }), - ).toEqual([ - "D:\\Windows\\System32\\reg.exe", - "E:\\Windows\\System32\\reg.exe", - "C:\\Windows\\System32\\reg.exe", - ]); + it("uses the fixed Windows system reg.exe candidate", () => { + expect(_private.getWindowsRegExeCandidates()).toEqual(["C:\\Windows\\System32\\reg.exe"]); }); - it("dedupes equivalent roots case-insensitively", () => { - expect( - _private.getWindowsRegExeCandidates({ - SystemRoot: "D:\\Windows\\", - windir: "d:\\windows", - }), - ).toEqual(["D:\\Windows\\System32\\reg.exe", "C:\\Windows\\System32\\reg.exe"]); + it("does not resolve readable reg.exe files from env-derived roots", () => { + _resetWindowsInstallRootsForTests({ + isReadableFile: (filePath) => filePath === "D:\\Windows\\System32\\reg.exe", + }); + + const originalEnv = process.env; + try { + process.env = { + ...originalEnv, + SystemRoot: "D:\\Windows", + WINDIR: "E:\\Windows", + }; + expect(_private.locateWindowsRegExe()).toBeNull(); + } finally { + process.env = originalEnv; + } }); }); diff --git a/src/infra/windows-install-roots.ts b/src/infra/windows-install-roots.ts index 2c580d87028..ea3ba520c7a 100644 --- a/src/infra/windows-install-roots.ts +++ b/src/infra/windows-install-roots.ts @@ -92,29 +92,12 @@ function getEnvValueCaseInsensitive( return actualKey ? env[actualKey] : undefined; } -function getWindowsRegExeCandidates(env: Record): readonly string[] { - const seen = new Set(); - const candidates: string[] = []; - for (const root of [ - normalizeWindowsInstallRoot(getEnvValueCaseInsensitive(env, "SystemRoot")), - normalizeWindowsInstallRoot(getEnvValueCaseInsensitive(env, "WINDIR")), - DEFAULT_WINDOWS_SYSTEM_ROOT, - ]) { - if (!root) { - continue; - } - const key = normalizeLowercaseStringOrEmpty(root); - if (seen.has(key)) { - continue; - } - seen.add(key); - candidates.push(path.win32.join(root, "System32", "reg.exe")); - } - return candidates; +function getWindowsRegExeCandidates(): readonly string[] { + return [path.win32.join(DEFAULT_WINDOWS_SYSTEM_ROOT, "System32", "reg.exe")]; } -function locateWindowsRegExe(env: Record = process.env): string | null { - for (const candidate of getWindowsRegExeCandidates(env)) { +function locateWindowsRegExe(): string | null { + for (const candidate of getWindowsRegExeCandidates()) { if (isReadableFileFn(candidate)) { return candidate; } @@ -151,7 +134,7 @@ function runRegQuery( } function defaultQueryRegistryValue(key: string, valueName: string): string | null { - const regExe = locateWindowsRegExe(process.env); + const regExe = locateWindowsRegExe(); if (!regExe) { return null; }