Skip to content

Commit 3811001

Browse files
eleqtriziteleqtrizit
authored
fix(exec): bind Windows allowlist execution path (#98260)
* fix(exec): bind windows allowlist execution path * fix(exec): add windows shadow execution proof * fix(exec): preserve wildcard allowlist behavior * fix(exec): correct blocked plan test fixture
1 parent 44ec758 commit 3811001

3 files changed

Lines changed: 204 additions & 7 deletions

File tree

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1926,7 +1926,7 @@
19261926
"test:unit:fast:audit": "node scripts/test-unit-fast-audit.mjs",
19271927
"test:voicecall:closedloop": "node scripts/test-voicecall-closedloop.mjs",
19281928
"test:watch": "node scripts/test-projects.mjs --watch",
1929-
"test:windows:ci": "node scripts/test-projects.mjs src/shared/runtime-import.test.ts src/process/exec.windows.test.ts src/process/windows-command.test.ts src/infra/windows-install-roots.test.ts extensions/lobster/src/lobster-runner.test.ts test/scripts/format-generated-module.test.ts test/scripts/npm-runner.test.ts test/scripts/pnpm-runner.test.ts test/scripts/run-with-env.test.ts test/scripts/ui.test.ts test/scripts/vitest-process-group.test.ts",
1929+
"test:windows:ci": "node scripts/test-projects.mjs src/shared/runtime-import.test.ts src/process/exec.windows.test.ts src/process/windows-command.test.ts src/infra/windows-install-roots.test.ts src/node-host/invoke-system-run-allowlist.test.ts extensions/lobster/src/lobster-runner.test.ts test/scripts/format-generated-module.test.ts test/scripts/npm-runner.test.ts test/scripts/pnpm-runner.test.ts test/scripts/run-with-env.test.ts test/scripts/ui.test.ts test/scripts/vitest-process-group.test.ts",
19301930
"tool-display:check": "node --import tsx scripts/tool-display.ts --check",
19311931
"tool-display:write": "node --import tsx scripts/tool-display.ts --write",
19321932
"ts-topology": "node --import tsx scripts/ts-topology.ts",

‎src/node-host/invoke-system-run-allowlist.test.ts‎

Lines changed: 195 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,16 @@
11
/** Tests system.run allowlist planning, output truncation, and argv resolution. */
2+
import { spawn } from "node:child_process";
3+
import fs from "node:fs";
4+
import os from "node:os";
5+
import path from "node:path";
26
import { describe, expect, it } from "vitest";
3-
import { resolveExecApprovalsFromFile } from "../infra/exec-approvals.js";
7+
import { resolveExecApprovalsFromFile, type ExecCommandSegment } from "../infra/exec-approvals.js";
48
import { planShellAuthorization } from "../infra/exec-authorization-plan.js";
59
import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js";
6-
import { resolveSystemRunExecArgv } from "./invoke-system-run-allowlist.js";
10+
import {
11+
evaluateSystemRunAllowlist,
12+
resolveSystemRunExecArgv,
13+
} from "./invoke-system-run-allowlist.js";
714

815
function resolveAllowlistApprovals() {
916
return resolveExecApprovalsFromFile({
@@ -18,7 +25,193 @@ function resolveAllowlistApprovals() {
1825
});
1926
}
2027

28+
function resolveWindowsShellExecArgv(segment: ExecCommandSegment) {
29+
return resolveSystemRunExecArgv({
30+
plannedAllowlistArgv: undefined,
31+
argv: ["powershell.exe", "-Command", "safe --version"],
32+
security: "allowlist",
33+
approvals: resolveAllowlistApprovals(),
34+
safeBins: new Set(),
35+
safeBinProfiles: {},
36+
trustedSafeBinDirs: new Set(),
37+
skillBins: [],
38+
autoAllowSkills: false,
39+
isWindows: true,
40+
policy: {
41+
approvedByAsk: false,
42+
analysisOk: true,
43+
allowlistSatisfied: true,
44+
},
45+
shellCommand: "safe --version",
46+
segments: [segment],
47+
segmentSatisfiedBy: ["allowlist"],
48+
authorizationPlan: undefined,
49+
cwd: "C:\\workspace",
50+
env: undefined,
51+
});
52+
}
53+
54+
function runExecutable(params: {
55+
argv: string[];
56+
cwd: string;
57+
env: NodeJS.ProcessEnv;
58+
}): Promise<{ exitCode: number | null; stdout: string }> {
59+
return new Promise((resolve, reject) => {
60+
const child = spawn(params.argv[0], params.argv.slice(1), {
61+
cwd: params.cwd,
62+
env: params.env,
63+
stdio: ["ignore", "pipe", "pipe"],
64+
windowsHide: true,
65+
});
66+
const stdout: Buffer[] = [];
67+
child.stdout.on("data", (chunk: Buffer) => stdout.push(chunk));
68+
child.once("error", reject);
69+
child.once("close", (exitCode) => {
70+
resolve({ exitCode, stdout: Buffer.concat(stdout).toString("utf8") });
71+
});
72+
});
73+
}
74+
2175
describe("resolveSystemRunExecArgv", () => {
76+
it("pins Windows shell execution to the resolved allowlisted executable", async () => {
77+
const trustedExecutable = "C:\\trusted-bin\\safe.exe";
78+
const result = await resolveWindowsShellExecArgv({
79+
raw: "safe --version",
80+
argv: ["safe", "--version"],
81+
resolution: {
82+
execution: {
83+
rawExecutable: "safe",
84+
resolvedPath: trustedExecutable,
85+
executableName: "safe.exe",
86+
},
87+
policy: {
88+
rawExecutable: "safe",
89+
resolvedPath: trustedExecutable,
90+
executableName: "safe.exe",
91+
},
92+
},
93+
});
94+
95+
expect(result).toEqual([trustedExecutable, "--version"]);
96+
});
97+
98+
it("preserves unresolved Windows shell argv authorized by a bare wildcard", async () => {
99+
const result = await resolveWindowsShellExecArgv({
100+
raw: "safe --version",
101+
argv: ["safe", "--version"],
102+
resolution: null,
103+
});
104+
105+
expect(result).toEqual(["safe", "--version"]);
106+
});
107+
108+
it("fails closed when the Windows shell execution plan is blocked", async () => {
109+
const result = await resolveWindowsShellExecArgv({
110+
raw: "safe --version",
111+
argv: ["safe", "--version"],
112+
resolution: {
113+
policyBlocked: true,
114+
execution: {
115+
rawExecutable: "safe",
116+
executableName: "safe",
117+
},
118+
policy: {
119+
rawExecutable: "safe",
120+
executableName: "safe",
121+
},
122+
},
123+
});
124+
125+
expect(result).toBeNull();
126+
});
127+
128+
it.runIf(process.platform === "win32")(
129+
"executes the allowlisted path instead of a workspace shadow executable",
130+
async () => {
131+
const fixtureRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-windows-shadow-"));
132+
const trustedBin = path.join(fixtureRoot, "trusted-bin");
133+
const workspace = path.join(fixtureRoot, "workspace");
134+
fs.mkdirSync(trustedBin);
135+
fs.mkdirSync(workspace);
136+
const trustedExecutable = path.join(trustedBin, "safe.exe");
137+
const shadowExecutable = path.join(workspace, "safe.exe");
138+
const systemRoot = process.env.SystemRoot ?? process.env.WINDIR ?? "C:\\Windows";
139+
fs.copyFileSync(path.join(systemRoot, "System32", "cmd.exe"), trustedExecutable);
140+
fs.copyFileSync(path.join(systemRoot, "System32", "where.exe"), shadowExecutable);
141+
const env = {
142+
...process.env,
143+
PATH: `${trustedBin}${path.delimiter}${process.env.PATH ?? ""}`,
144+
PATHEXT: process.env.PATHEXT ?? ".COM;.EXE;.BAT;.CMD",
145+
};
146+
const shellCommand = "safe /d /s /c echo TRUSTED_EXECUTABLE";
147+
const commandTail = ["/d", "/s", "/c", "echo", "TRUSTED_EXECUTABLE"];
148+
149+
try {
150+
const bareResult = await runExecutable({
151+
argv: ["safe", ...commandTail],
152+
cwd: workspace,
153+
env,
154+
});
155+
expect(bareResult.exitCode).not.toBe(0);
156+
expect(bareResult.stdout).not.toContain("TRUSTED_EXECUTABLE");
157+
158+
const approvals = resolveExecApprovalsFromFile({
159+
file: {
160+
version: 1,
161+
defaults: { security: "allowlist", ask: "off", askFallback: "deny" },
162+
agents: { main: { allowlist: [{ pattern: trustedExecutable }] } },
163+
},
164+
});
165+
const analysis = await evaluateSystemRunAllowlist({
166+
shellCommand,
167+
argv: ["powershell.exe", "-Command", shellCommand],
168+
approvals,
169+
security: "allowlist",
170+
safeBins: new Set(),
171+
safeBinProfiles: {},
172+
trustedSafeBinDirs: new Set(),
173+
cwd: workspace,
174+
env,
175+
skillBins: [],
176+
autoAllowSkills: false,
177+
});
178+
expect(analysis.analysisOk).toBe(true);
179+
expect(analysis.allowlistSatisfied).toBe(true);
180+
181+
const execArgv = await resolveSystemRunExecArgv({
182+
plannedAllowlistArgv: undefined,
183+
argv: ["powershell.exe", "-Command", shellCommand],
184+
security: "allowlist",
185+
approvals,
186+
safeBins: new Set(),
187+
safeBinProfiles: {},
188+
trustedSafeBinDirs: new Set(),
189+
skillBins: [],
190+
autoAllowSkills: false,
191+
isWindows: true,
192+
policy: {
193+
approvedByAsk: false,
194+
analysisOk: analysis.analysisOk,
195+
allowlistSatisfied: analysis.allowlistSatisfied,
196+
},
197+
shellCommand,
198+
segments: analysis.segments,
199+
segmentSatisfiedBy: analysis.segmentSatisfiedBy,
200+
authorizationPlan: analysis.authorizationPlan,
201+
cwd: workspace,
202+
env,
203+
});
204+
expect(execArgv?.[0]).toBe(fs.realpathSync(trustedExecutable));
205+
206+
const fixedResult = await runExecutable({ argv: execArgv ?? [], cwd: workspace, env });
207+
expect(fixedResult.exitCode).toBe(0);
208+
expect(fixedResult.stdout).toContain("TRUSTED_EXECUTABLE");
209+
} finally {
210+
fs.rmSync(fixtureRoot, { recursive: true, force: true });
211+
}
212+
},
213+
);
214+
22215
it.runIf(process.platform !== "win32")(
23216
"fails closed when shell rewriting has no authorization plan",
24217
async () => {

‎src/node-host/invoke-system-run-allowlist.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,15 @@ export async function resolveSystemRunExecArgv(params: {
165165
params.shellCommand &&
166166
params.policy.analysisOk &&
167167
params.policy.allowlistSatisfied &&
168-
params.segments.length === 1 &&
169-
params.segments[0]?.argv.length > 0
168+
params.segments.length === 1
170169
) {
171-
// Windows shell transports expose a parsed argv segment that is safer than the wrapper argv.
172-
execArgv = params.segments[0].argv;
170+
// Exact-path matches stay bound to the resolved executable, while the bare
171+
// wildcard contract can still authorize unresolved Windows commands.
172+
const plannedArgv = resolvePlannedSegmentArgv(params.segments[0]);
173+
if (!plannedArgv) {
174+
return null;
175+
}
176+
execArgv = plannedArgv;
173177
}
174178
if (
175179
params.security === "allowlist" &&

0 commit comments

Comments
 (0)