Skip to content

Commit 4894d90

Browse files
steipetesteipete
committed
refactor(exec-approvals): unify system.run binding and generate host env policy
1 parent baf1c8e commit 4894d90

18 files changed

+857
-341
lines changed

‎apps/macos/Sources/OpenClaw/HostEnvSanitizer.swift‎

Lines changed: 4 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,11 @@
11
import Foundation
22

33
enum HostEnvSanitizer {
4-
/// Keep in sync with src/infra/host-env-security-policy.json.
4+
/// Generated from src/infra/host-env-security-policy.json via scripts/generate-host-env-security-policy-swift.mjs.
55
/// Parity is validated by src/infra/host-env-security.policy-parity.test.ts.
6-
private static let blockedKeys: Set<String> = [
7-
"NODE_OPTIONS",
8-
"NODE_PATH",
9-
"PYTHONHOME",
10-
"PYTHONPATH",
11-
"PERL5LIB",
12-
"PERL5OPT",
13-
"RUBYLIB",
14-
"RUBYOPT",
15-
"BASH_ENV",
16-
"ENV",
17-
"GIT_EXTERNAL_DIFF",
18-
"SHELL",
19-
"SHELLOPTS",
20-
"PS4",
21-
"GCONV_PATH",
22-
"IFS",
23-
"SSLKEYLOGFILE",
24-
]
25-
26-
private static let blockedPrefixes: [String] = [
27-
"DYLD_",
28-
"LD_",
29-
"BASH_FUNC_",
30-
]
31-
private static let blockedOverrideKeys: Set<String> = [
32-
"HOME",
33-
"ZDOTDIR",
34-
]
6+
private static let blockedKeys = HostEnvSecurityPolicy.blockedKeys
7+
private static let blockedPrefixes = HostEnvSecurityPolicy.blockedPrefixes
8+
private static let blockedOverrideKeys = HostEnvSecurityPolicy.blockedOverrideKeys
359
private static let shellWrapperAllowedOverrideKeys: Set<String> = [
3610
"TERM",
3711
"LANG",

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Generated file. Do not edit directly.
2+
// Source: src/infra/host-env-security-policy.json
3+
// Regenerate: node scripts/generate-host-env-security-policy-swift.mjs
4+
5+
import Foundation
6+
7+
enum HostEnvSecurityPolicy {
8+
static let blockedKeys: Set<String> = [
9+
"NODE_OPTIONS",
10+
"NODE_PATH",
11+
"PYTHONHOME",
12+
"PYTHONPATH",
13+
"PERL5LIB",
14+
"PERL5OPT",
15+
"RUBYLIB",
16+
"RUBYOPT",
17+
"BASH_ENV",
18+
"ENV",
19+
"GIT_EXTERNAL_DIFF",
20+
"SHELL",
21+
"SHELLOPTS",
22+
"PS4",
23+
"GCONV_PATH",
24+
"IFS",
25+
"SSLKEYLOGFILE"
26+
]
27+
28+
static let blockedOverrideKeys: Set<String> = [
29+
"HOME",
30+
"ZDOTDIR"
31+
]
32+
33+
static let blockedPrefixes: [String] = [
34+
"DYLD_",
35+
"LD_",
36+
"BASH_FUNC_"
37+
]
38+
}

‎scripts/generate-host-env-security-policy-swift.mjs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
#!/usr/bin/env node
2+
import fs from "node:fs";
3+
import path from "node:path";
4+
import { fileURLToPath } from "node:url";
5+
6+
const here = path.dirname(fileURLToPath(import.meta.url));
7+
const repoRoot = path.resolve(here, "..");
8+
const policyPath = path.join(repoRoot, "src", "infra", "host-env-security-policy.json");
9+
const outputPath = path.join(
10+
repoRoot,
11+
"apps",
12+
"macos",
13+
"Sources",
14+
"OpenClaw",
15+
"HostEnvSecurityPolicy.generated.swift",
16+
);
17+
18+
/** @type {{blockedKeys: string[]; blockedOverrideKeys?: string[]; blockedPrefixes: string[]}} */
19+
const policy = JSON.parse(fs.readFileSync(policyPath, "utf8"));
20+
21+
const renderSwiftStringArray = (items) => items.map((item) => ` "${item}"`).join(",\n");
22+
23+
const swift = `// Generated file. Do not edit directly.
24+
// Source: src/infra/host-env-security-policy.json
25+
// Regenerate: node scripts/generate-host-env-security-policy-swift.mjs
26+
27+
import Foundation
28+
29+
enum HostEnvSecurityPolicy {
30+
static let blockedKeys: Set<String> = [
31+
${renderSwiftStringArray(policy.blockedKeys)}
32+
]
33+
34+
static let blockedOverrideKeys: Set<String> = [
35+
${renderSwiftStringArray(policy.blockedOverrideKeys ?? [])}
36+
]
37+
38+
static let blockedPrefixes: [String] = [
39+
${renderSwiftStringArray(policy.blockedPrefixes)}
40+
]
41+
}
42+
`;
43+
44+
fs.writeFileSync(outputPath, swift);
45+
console.log(`Wrote ${path.relative(repoRoot, outputPath)}`);

‎src/agents/bash-tools.exec-approval-request.ts‎

Lines changed: 47 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,52 @@ export type RequestExecApprovalDecisionParams = {
2424
turnSourceThreadId?: string | number;
2525
};
2626

27+
type ExecApprovalRequestToolParams = {
28+
id: string;
29+
command: string;
30+
commandArgv?: string[];
31+
env?: Record<string, string>;
32+
cwd: string;
33+
nodeId?: string;
34+
host: "gateway" | "node";
35+
security: ExecSecurity;
36+
ask: ExecAsk;
37+
agentId?: string;
38+
resolvedPath?: string;
39+
sessionKey?: string;
40+
turnSourceChannel?: string;
41+
turnSourceTo?: string;
42+
turnSourceAccountId?: string;
43+
turnSourceThreadId?: string | number;
44+
timeoutMs: number;
45+
twoPhase: true;
46+
};
47+
48+
function buildExecApprovalRequestToolParams(
49+
params: RequestExecApprovalDecisionParams,
50+
): ExecApprovalRequestToolParams {
51+
return {
52+
id: params.id,
53+
command: params.command,
54+
commandArgv: params.commandArgv,
55+
env: params.env,
56+
cwd: params.cwd,
57+
nodeId: params.nodeId,
58+
host: params.host,
59+
security: params.security,
60+
ask: params.ask,
61+
agentId: params.agentId,
62+
resolvedPath: params.resolvedPath,
63+
sessionKey: params.sessionKey,
64+
turnSourceChannel: params.turnSourceChannel,
65+
turnSourceTo: params.turnSourceTo,
66+
turnSourceAccountId: params.turnSourceAccountId,
67+
turnSourceThreadId: params.turnSourceThreadId,
68+
timeoutMs: DEFAULT_APPROVAL_TIMEOUT_MS,
69+
twoPhase: true,
70+
};
71+
}
72+
2773
type ParsedDecision = { present: boolean; value: string | null };
2874

2975
function parseDecision(value: unknown): ParsedDecision {
@@ -65,26 +111,7 @@ export async function registerExecApprovalRequest(
65111
}>(
66112
"exec.approval.request",
67113
{ timeoutMs: DEFAULT_APPROVAL_REQUEST_TIMEOUT_MS },
68-
{
69-
id: params.id,
70-
command: params.command,
71-
commandArgv: params.commandArgv,
72-
env: params.env,
73-
cwd: params.cwd,
74-
nodeId: params.nodeId,
75-
host: params.host,
76-
security: params.security,
77-
ask: params.ask,
78-
agentId: params.agentId,
79-
resolvedPath: params.resolvedPath,
80-
sessionKey: params.sessionKey,
81-
turnSourceChannel: params.turnSourceChannel,
82-
turnSourceTo: params.turnSourceTo,
83-
turnSourceAccountId: params.turnSourceAccountId,
84-
turnSourceThreadId: params.turnSourceThreadId,
85-
timeoutMs: DEFAULT_APPROVAL_TIMEOUT_MS,
86-
twoPhase: true,
87-
},
114+
buildExecApprovalRequestToolParams(params),
88115
{ expectFinal: false },
89116
);
90117
const decision = parseDecision(registrationResult);

‎src/gateway/node-invoke-system-run-approval-match.test.ts‎

Lines changed: 53 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
import { describe, expect, test } from "vitest";
2-
import { approvalMatchesSystemRunRequest } from "./node-invoke-system-run-approval-match.js";
3-
import { buildSystemRunApprovalEnvBinding } from "./system-run-approval-env-binding.js";
2+
import { evaluateSystemRunApprovalMatch } from "./node-invoke-system-run-approval-match.js";
3+
import {
4+
buildSystemRunApprovalBindingV1,
5+
buildSystemRunApprovalEnvBinding,
6+
} from "./system-run-approval-binding.js";
47

5-
describe("approvalMatchesSystemRunRequest", () => {
8+
describe("evaluateSystemRunApprovalMatch", () => {
69
test("matches legacy command text when binding fields match", () => {
7-
const result = approvalMatchesSystemRunRequest({
10+
const result = evaluateSystemRunApprovalMatch({
811
cmdText: "echo SAFE",
912
argv: ["echo", "SAFE"],
1013
request: {
@@ -20,11 +23,11 @@ describe("approvalMatchesSystemRunRequest", () => {
2023
sessionKey: "session-1",
2124
},
2225
});
23-
expect(result).toBe(true);
26+
expect(result).toEqual({ ok: true });
2427
});
2528

2629
test("rejects legacy command mismatch", () => {
27-
const result = approvalMatchesSystemRunRequest({
30+
const result = evaluateSystemRunApprovalMatch({
2831
cmdText: "echo PWNED",
2932
argv: ["echo", "PWNED"],
3033
request: {
@@ -37,47 +40,65 @@ describe("approvalMatchesSystemRunRequest", () => {
3740
sessionKey: null,
3841
},
3942
});
40-
expect(result).toBe(false);
43+
expect(result.ok).toBe(false);
44+
if (result.ok) {
45+
throw new Error("unreachable");
46+
}
47+
expect(result.code).toBe("APPROVAL_REQUEST_MISMATCH");
4148
});
4249

43-
test("enforces exact argv binding when commandArgv is set", () => {
44-
const result = approvalMatchesSystemRunRequest({
50+
test("enforces exact argv binding in v1 object", () => {
51+
const result = evaluateSystemRunApprovalMatch({
4552
cmdText: "echo SAFE",
4653
argv: ["echo", "SAFE"],
4754
request: {
4855
host: "node",
4956
command: "echo SAFE",
50-
commandArgv: ["echo", "SAFE"],
57+
systemRunBindingV1: buildSystemRunApprovalBindingV1({
58+
argv: ["echo", "SAFE"],
59+
cwd: null,
60+
agentId: null,
61+
sessionKey: null,
62+
}).binding,
5163
},
5264
binding: {
5365
cwd: null,
5466
agentId: null,
5567
sessionKey: null,
5668
},
5769
});
58-
expect(result).toBe(true);
70+
expect(result).toEqual({ ok: true });
5971
});
6072

61-
test("rejects argv mismatch even when command text matches", () => {
62-
const result = approvalMatchesSystemRunRequest({
73+
test("rejects argv mismatch in v1 object", () => {
74+
const result = evaluateSystemRunApprovalMatch({
6375
cmdText: "echo SAFE",
6476
argv: ["echo", "SAFE"],
6577
request: {
6678
host: "node",
6779
command: "echo SAFE",
68-
commandArgv: ["echo SAFE"],
80+
systemRunBindingV1: buildSystemRunApprovalBindingV1({
81+
argv: ["echo SAFE"],
82+
cwd: null,
83+
agentId: null,
84+
sessionKey: null,
85+
}).binding,
6986
},
7087
binding: {
7188
cwd: null,
7289
agentId: null,
7390
sessionKey: null,
7491
},
7592
});
76-
expect(result).toBe(false);
93+
expect(result.ok).toBe(false);
94+
if (result.ok) {
95+
throw new Error("unreachable");
96+
}
97+
expect(result.code).toBe("APPROVAL_REQUEST_MISMATCH");
7798
});
7899

79-
test("rejects env overrides when approval record lacks env hash", () => {
80-
const result = approvalMatchesSystemRunRequest({
100+
test("rejects env overrides when approval record lacks env binding", () => {
101+
const result = evaluateSystemRunApprovalMatch({
81102
cmdText: "git diff",
82103
argv: ["git", "diff"],
83104
request: {
@@ -92,22 +113,26 @@ describe("approvalMatchesSystemRunRequest", () => {
92113
env: { GIT_EXTERNAL_DIFF: "/tmp/pwn.sh" },
93114
},
94115
});
95-
expect(result).toBe(false);
116+
expect(result.ok).toBe(false);
117+
if (result.ok) {
118+
throw new Error("unreachable");
119+
}
120+
expect(result.code).toBe("APPROVAL_ENV_BINDING_MISSING");
96121
});
97122

98123
test("accepts matching env hash with reordered keys", () => {
99-
const binding = buildSystemRunApprovalEnvBinding({
124+
const envBinding = buildSystemRunApprovalEnvBinding({
100125
SAFE_A: "1",
101126
SAFE_B: "2",
102127
});
103-
const result = approvalMatchesSystemRunRequest({
128+
const result = evaluateSystemRunApprovalMatch({
104129
cmdText: "git diff",
105130
argv: ["git", "diff"],
106131
request: {
107132
host: "node",
108133
command: "git diff",
109134
commandArgv: ["git", "diff"],
110-
envHash: binding.envHash,
135+
envHash: envBinding.envHash,
111136
},
112137
binding: {
113138
cwd: null,
@@ -116,11 +141,11 @@ describe("approvalMatchesSystemRunRequest", () => {
116141
env: { SAFE_B: "2", SAFE_A: "1" },
117142
},
118143
});
119-
expect(result).toBe(true);
144+
expect(result).toEqual({ ok: true });
120145
});
121146

122147
test("rejects non-node host requests", () => {
123-
const result = approvalMatchesSystemRunRequest({
148+
const result = evaluateSystemRunApprovalMatch({
124149
cmdText: "echo SAFE",
125150
argv: ["echo", "SAFE"],
126151
request: {
@@ -133,6 +158,10 @@ describe("approvalMatchesSystemRunRequest", () => {
133158
sessionKey: null,
134159
},
135160
});
136-
expect(result).toBe(false);
161+
expect(result.ok).toBe(false);
162+
if (result.ok) {
163+
throw new Error("unreachable");
164+
}
165+
expect(result.code).toBe("APPROVAL_REQUEST_MISMATCH");
137166
});
138167
});

0 commit comments

Comments
 (0)