Skip to content

Commit 49d4bf4

Browse files
mmapsmmaps
committed
fix: address PR review feedback
1 parent bbaa0ba commit 49d4bf4

9 files changed

Lines changed: 27 additions & 13 deletions

‎apps/macos/Sources/OpenClaw/HostEnvSecurityPolicy.generated.swift‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,7 @@ enum HostEnvSecurityPolicy {
425425
"SSL_CERT_DIR",
426426
"SSL_CERT_FILE",
427427
"SUDO_EDITOR",
428+
"SYSTEMROOT",
428429
"TF_CLI_CONFIG_FILE",
429430
"TF_PLUGIN_CACHE_DIR",
430431
"UV_DEFAULT_INDEX",
@@ -435,6 +436,7 @@ enum HostEnvSecurityPolicy {
435436
"VIRTUAL_ENV",
436437
"VISUAL",
437438
"WGETRC",
439+
"WINDIR",
438440
"XDG_CONFIG_DIRS",
439441
"XDG_CONFIG_HOME",
440442
"YARN_RC_FILENAME",

‎src/infra/dotenv.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,7 @@ const BLOCKED_WORKSPACE_DOTENV_KEYS = new Set([
7272
"PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH",
7373
"SYNOLOGY_CHAT_INCOMING_URL",
7474
"SYNOLOGY_NAS_HOST",
75-
// Windows helper roots affect command resolution for audit/repair flows.
76-
"SYSTEMROOT",
7775
"UV_PYTHON",
78-
"WINDIR",
7976
]);
8077

8178
// Block endpoint redirection for any service without overfitting per-provider names.

‎src/infra/host-env-security-policy.json‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,9 @@
233233
"NODE_AUTH_TOKEN",
234234
"NPM_TOKEN",
235235
"REDIS_URL",
236-
"SSH_AUTH_SOCK"
236+
"SSH_AUTH_SOCK",
237+
"SYSTEMROOT",
238+
"WINDIR"
237239
],
238240
"allowedInheritedOverrideOnlyKeys": [
239241
"ALL_PROXY",
@@ -263,6 +265,8 @@
263265
"SSH_AUTH_SOCK",
264266
"SSL_CERT_DIR",
265267
"SSL_CERT_FILE",
268+
"SYSTEMROOT",
269+
"WINDIR",
266270
"ZDOTDIR"
267271
],
268272
"blockedOverridePrefixes": ["GIT_CONFIG_", "NPM_CONFIG_", "CARGO_REGISTRIES_", "TF_VAR_"],

‎src/infra/host-env-security.reported-baseline.json‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,7 @@
222222
"SSL_CERT_DIR",
223223
"SSL_CERT_FILE",
224224
"SUDO_EDITOR",
225+
"SYSTEMROOT",
225226
"TF_CLI_CONFIG_FILE",
226227
"TF_PLUGIN_CACHE_DIR",
227228
"UV_DEFAULT_INDEX",
@@ -232,10 +233,11 @@
232233
"VIRTUAL_ENV",
233234
"VISUAL",
234235
"WGETRC",
236+
"WINDIR",
235237
"XDG_CONFIG_DIRS",
236238
"XDG_CONFIG_HOME",
237239
"YARN_RC_FILENAME",
238240
"ZDOTDIR"
239241
],
240-
"expectedTotalReportedEntries": 232
242+
"expectedTotalReportedEntries": 234
241243
}

‎src/infra/host-env-security.reported-baseline.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ const INHERITED_ALLOWLIST_RATIONALE: Record<string, string> = {
4747
SSH_AUTH_SOCK: "Trusted inherited SSH agent socket from operator runtime.",
4848
SSL_CERT_DIR: "Trusted inherited OpenSSL certificate directory path.",
4949
SSL_CERT_FILE: "Trusted inherited OpenSSL certificate file path.",
50+
SYSTEMROOT: "Trusted inherited Windows system root selected by the host OS.",
51+
WINDIR: "Trusted inherited Windows directory selected by the host OS.",
5052
ZDOTDIR: "Trusted inherited shell startup directory boundary.",
5153
};
5254

@@ -83,7 +85,7 @@ describe("host env reported baseline coverage", () => {
8385
baseline.reportedDangerousEverywhereKeys.length +
8486
baseline.reportedDangerousOverrideOnlyKeys.length,
8587
).toBe(baseline.expectedTotalReportedEntries);
86-
expect(baseline.expectedTotalReportedEntries).toBe(232);
88+
expect(baseline.expectedTotalReportedEntries).toBe(234);
8789
expect(sortUniqueUpper(baseline.reportedDangerousEverywhereKeys)).toEqual(
8890
baseline.reportedDangerousEverywhereKeys,
8991
);

‎src/infra/host-env-security.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -837,6 +837,8 @@ describe("isDangerousHostEnvOverrideVarName", () => {
837837
expect(isDangerousHostEnvOverrideVarName("AWS_CONFIG_FILE")).toBe(true);
838838
expect(isDangerousHostEnvOverrideVarName("aws_config_file")).toBe(true);
839839
expect(isDangerousHostEnvOverrideVarName("yarn_rc_filename")).toBe(true);
840+
expect(isDangerousHostEnvOverrideVarName("SystemRoot")).toBe(true);
841+
expect(isDangerousHostEnvOverrideVarName("windir")).toBe(true);
840842
expect(isDangerousHostEnvOverrideVarName("BASH_ENV")).toBe(false);
841843
expect(isDangerousHostEnvOverrideVarName("FOO")).toBe(false);
842844
});

‎src/infra/windows-install-roots.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import fs from "node:fs";
33
import path from "node:path";
44
import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js";
55

6-
const DEFAULT_SYSTEM_ROOT = "C:\\Windows";
6+
export const DEFAULT_WINDOWS_SYSTEM_ROOT = "C:\\Windows";
77
const DEFAULT_PROGRAM_FILES = "C:\\Program Files";
88
const DEFAULT_PROGRAM_FILES_X86 = "C:\\Program Files (x86)";
99
const WINDOWS_NT_CURRENT_VERSION_KEY = "HKLM\\SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion";
@@ -98,7 +98,7 @@ function getWindowsRegExeCandidates(env: Record<string, string | undefined>): re
9898
for (const root of [
9999
normalizeWindowsInstallRoot(getEnvValueCaseInsensitive(env, "SystemRoot")),
100100
normalizeWindowsInstallRoot(getEnvValueCaseInsensitive(env, "WINDIR")),
101-
DEFAULT_SYSTEM_ROOT,
101+
DEFAULT_WINDOWS_SYSTEM_ROOT,
102102
]) {
103103
if (!root) {
104104
continue;
@@ -206,7 +206,7 @@ function buildWindowsInstallRoots(
206206
registryRoots.systemRoot ??
207207
normalizeWindowsInstallRoot(getEnvValueCaseInsensitive(env, "SystemRoot")) ??
208208
normalizeWindowsInstallRoot(getEnvValueCaseInsensitive(env, "WINDIR")) ??
209-
DEFAULT_SYSTEM_ROOT,
209+
DEFAULT_WINDOWS_SYSTEM_ROOT,
210210
programFiles:
211211
registryRoots.programFiles ??
212212
normalizeWindowsInstallRoot(getEnvValueCaseInsensitive(env, "ProgramFiles")) ??

‎src/security/windows-acl.test.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
2+
import { DEFAULT_WINDOWS_SYSTEM_ROOT } from "../infra/windows-install-roots.js";
23
import type { WindowsAclEntry, WindowsAclSummary } from "./windows-acl.js";
34

45
const MOCK_USERNAME = "MockUser";
56
const mockUserInfo = () => ({ username: MOCK_USERNAME });
67
const emptyUserInfo = () => ({ username: "" });
7-
const DEFAULT_ICACLS = "C:\\Windows\\System32\\icacls.exe";
8-
const DEFAULT_WHOAMI = "C:\\Windows\\System32\\whoami.exe";
8+
const DEFAULT_ICACLS = `${DEFAULT_WINDOWS_SYSTEM_ROOT}\\System32\\icacls.exe`;
9+
const DEFAULT_WHOAMI = `${DEFAULT_WINDOWS_SYSTEM_ROOT}\\System32\\whoami.exe`;
910

1011
let createIcaclsResetCommand: typeof import("./windows-acl.js").createIcaclsResetCommand;
1112
let formatIcaclsResetCommand: typeof import("./windows-acl.js").formatIcaclsResetCommand;

‎src/security/windows-acl.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import os from "node:os";
22
import path from "node:path";
3-
import { normalizeWindowsInstallRoot } from "../infra/windows-install-roots.js";
3+
import {
4+
DEFAULT_WINDOWS_SYSTEM_ROOT,
5+
normalizeWindowsInstallRoot,
6+
} from "../infra/windows-install-roots.js";
47
import { runExec } from "../process/exec.js";
58
import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js";
69

@@ -52,7 +55,6 @@ const TRUSTED_BASE = new Set([
5255
]);
5356
const WORLD_SUFFIXES = ["\\users", "\\authenticated users"];
5457
const TRUSTED_SUFFIXES = ["\\administrators", "\\system", "\\système"];
55-
const DEFAULT_WINDOWS_SYSTEM_ROOT = "C:\\Windows";
5658

5759
// Accept an optional leading * which icacls prefixes to SIDs when invoked with /sid
5860
// (e.g. *S-1-5-18 instead of S-1-5-18).
@@ -116,6 +118,8 @@ function buildTrustedPrincipals(env?: NodeJS.ProcessEnv): Set<string> {
116118
}
117119

118120
function resolveWindowsSystemCommand(command: string, env?: NodeJS.ProcessEnv): string {
121+
// Never fall back to a bare helper name here; Windows command search can
122+
// consult the current directory and PATH before the real System32 helper.
119123
const root =
120124
[
121125
normalizeWindowsInstallRoot(env?.SystemRoot),

0 commit comments

Comments
 (0)