mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-06 05:51:15 +08:00
fix(infra): align env key normalization in approval binding path (#59182)
* fix: address issue * fix: address PR review feedback * fix: address review feedback * fix: address review feedback * chore: add changelog for Windows env approval binding --------- Co-authored-by: Devin Robison <drobison@nvidia.com>
This commit is contained in:
@@ -17,6 +17,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Plugins/Google: separate OAuth CSRF state from PKCE code verifier during Gemini browser sign-in so state validation and token exchange use independent values. (#59116) Thanks @eleqtrizit.
|
||||
- Exec/Windows: reject malformed drive-less rooted executable paths like `:\Users\...` so approval and allowlist candidate resolution no longer treat them as cwd-relative commands. (#58040) Thanks @SnowSky1.
|
||||
- Exec/preflight: fail closed on complex interpreter invocations that would otherwise skip script-content validation, and correctly inspect quoted script paths before host execution. Thanks @pgondhi987.
|
||||
- Exec/Windows: include Windows-compatible env override keys like `ProgramFiles(x86)` in system-run approval binding so changed approved values are rejected instead of silently passing unbound. (#59182) Thanks @pgondhi987.
|
||||
|
||||
## 2026.4.2-beta.1
|
||||
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { beforeAll, describe, expect, it } from "vitest";
|
||||
import { beforeAll, describe, expect, it, vi } from "vitest";
|
||||
|
||||
let buildDiscordComponentCustomId: typeof import("../component-custom-id.js").buildDiscordComponentCustomId;
|
||||
let buildDiscordModalCustomId: typeof import("../component-custom-id.js").buildDiscordModalCustomId;
|
||||
let buildDiscordComponentCustomId: typeof import("../components.js").buildDiscordComponentCustomId;
|
||||
let buildDiscordModalCustomId: typeof import("../components.js").buildDiscordModalCustomId;
|
||||
let createDiscordComponentButton: typeof import("./agent-components.js").createDiscordComponentButton;
|
||||
let createDiscordComponentChannelSelect: typeof import("./agent-components.js").createDiscordComponentChannelSelect;
|
||||
let createDiscordComponentMentionableSelect: typeof import("./agent-components.js").createDiscordComponentMentionableSelect;
|
||||
@@ -11,8 +11,7 @@ let createDiscordComponentStringSelect: typeof import("./agent-components.js").c
|
||||
let createDiscordComponentUserSelect: typeof import("./agent-components.js").createDiscordComponentUserSelect;
|
||||
|
||||
beforeAll(async () => {
|
||||
({ buildDiscordComponentCustomId, buildDiscordModalCustomId } =
|
||||
await import("../component-custom-id.js"));
|
||||
({ buildDiscordComponentCustomId, buildDiscordModalCustomId } = await import("../components.js"));
|
||||
({
|
||||
createDiscordComponentButton,
|
||||
createDiscordComponentChannelSelect,
|
||||
|
||||
@@ -123,6 +123,32 @@ describe("evaluateSystemRunApprovalMatch", () => {
|
||||
expect(result).toEqual({ ok: true });
|
||||
});
|
||||
|
||||
test("rejects mismatched Windows-compatible env override values", () => {
|
||||
const result = evaluateSystemRunApprovalMatch({
|
||||
argv: ["cmd.exe", "/c", "echo ok"],
|
||||
request: {
|
||||
host: "node",
|
||||
command: "cmd.exe /c echo ok",
|
||||
systemRunBinding: buildSystemRunApprovalBinding({
|
||||
argv: ["cmd.exe", "/c", "echo ok"],
|
||||
cwd: null,
|
||||
agentId: null,
|
||||
sessionKey: null,
|
||||
env: { "ProgramFiles(x86)": "C:\\Program Files (x86)" },
|
||||
}).binding,
|
||||
},
|
||||
binding: {
|
||||
...defaultBinding,
|
||||
env: { "ProgramFiles(x86)": "D:\\malicious" },
|
||||
},
|
||||
});
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) {
|
||||
throw new Error("unreachable");
|
||||
}
|
||||
expect(result.code).toBe("APPROVAL_ENV_MISMATCH");
|
||||
});
|
||||
|
||||
test("rejects non-node host requests", () => {
|
||||
const result = evaluateSystemRunApprovalMatch({
|
||||
argv: ["echo", "SAFE"],
|
||||
|
||||
@@ -656,6 +656,37 @@ describe("exec approval handlers", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("includes Windows-compatible env keys in approval env bindings", async () => {
|
||||
const { handlers, broadcasts, respond, context } = createExecApprovalFixture();
|
||||
await requestExecApproval({
|
||||
handlers,
|
||||
respond,
|
||||
context,
|
||||
params: {
|
||||
timeoutMs: 10,
|
||||
commandArgv: ["cmd.exe", "/c", "echo", "ok"],
|
||||
command: "cmd.exe /c echo ok",
|
||||
env: {
|
||||
"ProgramFiles(x86)": "C:\\Program Files (x86)",
|
||||
},
|
||||
},
|
||||
});
|
||||
const requested = broadcasts.find((entry) => entry.event === "exec.approval.requested");
|
||||
expect(requested).toBeTruthy();
|
||||
const request = (requested?.payload as { request?: Record<string, unknown> })?.request ?? {};
|
||||
const envBinding = buildSystemRunApprovalEnvBinding({
|
||||
"ProgramFiles(x86)": "C:\\Program Files (x86)",
|
||||
});
|
||||
expect(request["envKeys"]).toEqual(envBinding.envKeys);
|
||||
expect(request["systemRunBinding"]).toEqual(
|
||||
buildSystemRunApprovalBinding({
|
||||
argv: ["cmd.exe", "/c", "echo", "ok"],
|
||||
cwd: "/tmp",
|
||||
env: { "ProgramFiles(x86)": "C:\\Program Files (x86)" },
|
||||
}).binding,
|
||||
);
|
||||
});
|
||||
|
||||
it("stores sorted env keys for gateway approvals without node-only binding", async () => {
|
||||
const { handlers, broadcasts, respond, context } = createExecApprovalFixture();
|
||||
await requestExecApproval({
|
||||
|
||||
@@ -68,7 +68,7 @@ export function normalizeEnvVarKey(
|
||||
return key;
|
||||
}
|
||||
|
||||
function normalizeHostOverrideEnvVarKey(rawKey: string): string | null {
|
||||
export function normalizeHostOverrideEnvVarKey(rawKey: string): string | null {
|
||||
const key = normalizeEnvVarKey(rawKey);
|
||||
if (!key) {
|
||||
return null;
|
||||
|
||||
@@ -117,6 +117,19 @@ describe("buildSystemRunApprovalEnvBinding", () => {
|
||||
envKeys: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("includes Windows-compatible override keys in env binding", () => {
|
||||
const base = buildSystemRunApprovalEnvBinding({
|
||||
"ProgramFiles(x86)": "C:\\Program Files (x86)",
|
||||
});
|
||||
const changed = buildSystemRunApprovalEnvBinding({
|
||||
"ProgramFiles(x86)": "D:\\SDKs",
|
||||
});
|
||||
|
||||
expect(base.envKeys).toEqual(["ProgramFiles(x86)"]);
|
||||
expect(base.envHash).toBeTypeOf("string");
|
||||
expect(base.envHash).not.toEqual(changed.envHash);
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildSystemRunApprovalBinding", () => {
|
||||
@@ -175,6 +188,20 @@ describe("matchSystemRunApprovalEnvHash", () => {
|
||||
details: { envKeys: ["ALPHA"] },
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "reports missing approval env binding when actual env keys are present without hashes",
|
||||
params: {
|
||||
expectedEnvHash: null,
|
||||
actualEnvHash: null,
|
||||
actualEnvKeys: ["ProgramFiles(x86)"],
|
||||
},
|
||||
expected: {
|
||||
ok: false,
|
||||
code: "APPROVAL_ENV_BINDING_MISSING",
|
||||
message: "approval id missing env binding for requested env overrides",
|
||||
details: { envKeys: ["ProgramFiles(x86)"] },
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "reports env hash mismatches",
|
||||
params: {
|
||||
|
||||
@@ -4,7 +4,7 @@ import type {
|
||||
SystemRunApprovalFileOperand,
|
||||
SystemRunApprovalPlan,
|
||||
} from "./exec-approvals.js";
|
||||
import { normalizeEnvVarKey } from "./host-env-security.js";
|
||||
import { normalizeHostOverrideEnvVarKey } from "./host-env-security.js";
|
||||
import { normalizeNonEmptyString, normalizeStringArray } from "./system-run-normalize.js";
|
||||
|
||||
type NormalizedSystemRunEnvEntry = [key: string, value: string];
|
||||
@@ -75,7 +75,7 @@ function normalizeSystemRunEnvEntries(env: unknown): NormalizedSystemRunEnvEntry
|
||||
if (typeof rawValue !== "string") {
|
||||
continue;
|
||||
}
|
||||
const key = normalizeEnvVarKey(rawKey, { portable: true });
|
||||
const key = normalizeHostOverrideEnvVarKey(rawKey);
|
||||
if (!key) {
|
||||
continue;
|
||||
}
|
||||
@@ -162,6 +162,16 @@ export function matchSystemRunApprovalEnvHash(params: {
|
||||
actualEnvHash: string | null;
|
||||
actualEnvKeys: string[];
|
||||
}): SystemRunApprovalMatchResult {
|
||||
// Fail closed if callers provide inconsistent hash/key state. This guards against
|
||||
// normalization drift between approval and execution paths.
|
||||
if (!params.expectedEnvHash && !params.actualEnvHash && params.actualEnvKeys.length > 0) {
|
||||
return {
|
||||
ok: false,
|
||||
code: "APPROVAL_ENV_BINDING_MISSING",
|
||||
message: "approval id missing env binding for requested env overrides",
|
||||
details: { envKeys: params.actualEnvKeys },
|
||||
};
|
||||
}
|
||||
if (!params.expectedEnvHash && !params.actualEnvHash) {
|
||||
return { ok: true };
|
||||
}
|
||||
|
||||
@@ -48,6 +48,30 @@
|
||||
},
|
||||
"expected": { "ok": false, "code": "APPROVAL_ENV_MISMATCH" }
|
||||
},
|
||||
{
|
||||
"name": "binding rejects mismatched Windows-compatible env values",
|
||||
"request": {
|
||||
"host": "node",
|
||||
"command": "cmd.exe /c echo ok",
|
||||
"binding": {
|
||||
"argv": ["cmd.exe", "/c", "echo", "ok"],
|
||||
"cwd": null,
|
||||
"agentId": null,
|
||||
"sessionKey": null,
|
||||
"env": { "ProgramFiles(x86)": "C:\\Program Files (x86)" }
|
||||
}
|
||||
},
|
||||
"invoke": {
|
||||
"argv": ["cmd.exe", "/c", "echo", "ok"],
|
||||
"binding": {
|
||||
"cwd": null,
|
||||
"agentId": null,
|
||||
"sessionKey": null,
|
||||
"env": { "ProgramFiles(x86)": "D:\\malicious" }
|
||||
}
|
||||
},
|
||||
"expected": { "ok": false, "code": "APPROVAL_ENV_MISMATCH" }
|
||||
},
|
||||
{
|
||||
"name": "binding rejects unbound env overrides",
|
||||
"request": {
|
||||
|
||||
Reference in New Issue
Block a user