Skip to content

Commit ac12b39

Browse files
steipetesteipete
committed
fix(exec): bind approval trust to realpaths
1 parent c1bc6ad commit ac12b39

20 files changed

Lines changed: 496 additions & 43 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ Docs: https://docs.openclaw.ai
3939
- Agents/GitHub Copilot: normalize replayed Responses tool-call IDs before dispatch so resumed sessions with historical overlong tool IDs continue instead of failing Copilot schema validation. (#82750) Thanks @galiniliev.
4040
- CLI/web: resolve provider-scoped web search/fetch SecretRefs for `infer web ... --provider ...` while leaving unrelated plugin secrets untouched. Fixes #82621. Thanks @leno23.
4141
- Providers/Anthropic Vertex: resolve installed provider public surfaces from package-local `dist/`, restoring `anthropic-vertex/*` model calls after plugin externalization. Fixes #82781. Thanks @0L1v3DaD.
42+
- Gateway/exec approvals: bind path-shaped allowlists, safe-bin trust, skill auto-allow, Allow Always persistence, and approval audit metadata to the executable realpath so symlinked binaries cannot keep approvals after retargeting. Fixes #45595. Thanks @jasonftl.
4243
- Mac app: let menu gateway/session error text wrap across a few lines and stop rebuilding dynamic Context/Gateway menu rows while the menu is open, reducing flicker.
4344
- Mac app: make device pairing approval sheets friendlier, with concise Mac/device copy, shortened identifiers, friendly scope labels, and Approve as the primary action.
4445
- Providers/Qwen: honor session thinking level for `qwen-chat-template` payloads so `/think off` disables nested llama.cpp chat-template thinking controls. Fixes #82768. Thanks @bfox55.

‎src/agents/bash-tools.exec-host-gateway.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ vi.mock("../infra/exec-approvals.js", () => ({
106106
requiresExecApproval: vi.fn(() => false),
107107
recordAllowlistUse: vi.fn(),
108108
recordAllowlistMatchesUse: recordAllowlistMatchesUseMock,
109-
resolveApprovalAuditCandidatePath: vi.fn(() => null),
109+
resolveApprovalAuditTrustPath: vi.fn(() => null),
110110
resolveAllowAlwaysPatterns: vi.fn(() => []),
111111
resolveExecApprovalAllowedDecisions: vi.fn(() => ["allow-once", "allow-always", "deny"]),
112112
addAllowlistEntry: vi.fn(),

‎src/agents/bash-tools.exec-host-gateway.ts‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
hasDurableExecApproval,
1313
persistAllowAlwaysPatterns,
1414
recordAllowlistMatchesUse,
15-
resolveApprovalAuditCandidatePath,
15+
resolveApprovalAuditTrustPath,
1616
requiresExecApproval,
1717
} from "../infra/exec-approvals.js";
1818
import type { SafeBinProfile } from "../infra/exec-safe-bin-policy.js";
@@ -499,7 +499,7 @@ export async function processGatewayAllowlist(
499499
agentId: params.agentId,
500500
sessionKey: params.sessionKey,
501501
}),
502-
resolvedPath: resolveApprovalAuditCandidatePath(
502+
resolvedPath: resolveApprovalAuditTrustPath(
503503
allowlistEval.segments[0]?.resolution ?? null,
504504
params.workdir,
505505
),
@@ -549,7 +549,7 @@ export async function processGatewayAllowlist(
549549
}
550550

551551
recordMatchedAllowlistUse(
552-
resolveApprovalAuditCandidatePath(
552+
resolveApprovalAuditTrustPath(
553553
allowlistEval.segments[0]?.resolution ?? null,
554554
params.workdir,
555555
),
@@ -559,7 +559,7 @@ export async function processGatewayAllowlist(
559559
allowWithoutEnforcedCommand: enforcedCommand === undefined,
560560
};
561561
}
562-
const resolvedPath = resolveApprovalAuditCandidatePath(
562+
const resolvedPath = resolveApprovalAuditTrustPath(
563563
allowlistEval.segments[0]?.resolution ?? null,
564564
params.workdir,
565565
);
@@ -736,10 +736,7 @@ export async function processGatewayAllowlist(
736736
}
737737

738738
recordMatchedAllowlistUse(
739-
resolveApprovalAuditCandidatePath(
740-
allowlistEval.segments[0]?.resolution ?? null,
741-
params.workdir,
742-
),
739+
resolveApprovalAuditTrustPath(allowlistEval.segments[0]?.resolution ?? null, params.workdir),
743740
);
744741

745742
return { execCommandOverride: enforcedCommand };

‎src/infra/exec-allowlist-matching.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,4 +122,30 @@ describe("exec allowlist matching", () => {
122122
expect(matchAllowlist([{ pattern }], resolution)?.pattern).toBe(pattern);
123123
}
124124
});
125+
126+
it("matches path-shaped allowlist entries against the executable trust realpath", () => {
127+
const resolution = {
128+
rawExecutable: "rg",
129+
resolvedPath: "/opt/homebrew/bin/rg",
130+
resolvedRealPath: "/opt/homebrew/Cellar/ripgrep/14.1.1/bin/rg",
131+
executableName: "rg",
132+
};
133+
134+
expect(
135+
matchAllowlist([{ pattern: "/opt/homebrew/Cellar/ripgrep/14.1.1/bin/rg" }], resolution)
136+
?.pattern,
137+
).toBe("/opt/homebrew/Cellar/ripgrep/14.1.1/bin/rg");
138+
expect(matchAllowlist([{ pattern: "/opt/homebrew/bin/rg" }], resolution)).toBeNull();
139+
});
140+
141+
it("keeps basename allowlist entries on the PATH-resolved executable name", () => {
142+
const resolution = {
143+
rawExecutable: "rg",
144+
resolvedPath: "/opt/homebrew/bin/rg",
145+
resolvedRealPath: "/opt/homebrew/Cellar/ripgrep/14.1.1/bin/rg",
146+
executableName: "rg",
147+
};
148+
149+
expect(matchAllowlist([{ pattern: "rg" }], resolution)?.pattern).toBe("rg");
150+
});
125151
});

‎src/infra/exec-approvals-allow-always.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,27 @@ describe("resolveAllowAlwaysPatterns", () => {
224224
expect(patterns).toStrictEqual([]);
225225
});
226226

227+
it("persists allow-always executable patterns with the trust realpath", () => {
228+
const patterns = resolveAllowAlwaysPatterns({
229+
segments: [
230+
{
231+
raw: "rg -n needle",
232+
argv: ["rg", "-n", "needle"],
233+
resolution: makeMockCommandResolution({
234+
execution: makeMockExecutableResolution({
235+
rawExecutable: "rg",
236+
resolvedPath: "/opt/homebrew/bin/rg",
237+
resolvedRealPath: "/opt/homebrew/Cellar/ripgrep/14.1.1/bin/rg",
238+
executableName: "rg",
239+
}),
240+
}),
241+
},
242+
],
243+
});
244+
245+
expect(patterns).toEqual(["/opt/homebrew/Cellar/ripgrep/14.1.1/bin/rg"]);
246+
});
247+
227248
it("persists benign awk interpreters when strict inline-eval is enabled", () => {
228249
if (process.platform === "win32") {
229250
return;

‎src/infra/exec-approvals-allowlist.ts‎

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,14 @@ import {
1414
analyzeShellCommand,
1515
isWindowsPlatform,
1616
matchAllowlist,
17+
resolveExecutableTrustPath,
1718
resolveExecutionTargetCandidatePath,
1819
resolveExecutionTargetResolution,
20+
resolveExecutionTargetTrustPath,
1921
resolveCommandResolutionFromArgv,
2022
resolvePolicyTargetCandidatePath,
2123
resolvePolicyTargetResolution,
24+
resolvePolicyTargetTrustPath,
2225
splitCommandChain,
2326
splitCommandChainWithOperators,
2427
type ExecCommandAnalysis,
@@ -96,13 +99,14 @@ export function isSafeBinUsage(params: {
9699
if (!matchesSafeBin) {
97100
return false;
98101
}
99-
if (!resolution?.resolvedPath) {
102+
const trustPath = resolveExecutableTrustPath(resolution);
103+
if (!trustPath) {
100104
return false;
101105
}
102106
const isTrustedPath = params.isTrustedSafeBinPathFn ?? isTrustedSafeBinPath;
103107
if (
104108
!isTrustedPath({
105-
resolvedPath: resolution.resolvedPath,
109+
resolvedPath: trustPath,
106110
trustedDirs: params.trustedSafeBinDirs,
107111
})
108112
) {
@@ -212,15 +216,16 @@ function isSkillAutoAllowedSegment(params: {
212216
}
213217
const resolution = params.segment.resolution;
214218
const execution = resolveExecutionTargetResolution(resolution);
215-
if (!execution?.resolvedPath) {
219+
const trustPath = resolveExecutionTargetTrustPath(resolution);
220+
if (!execution?.resolvedPath || !trustPath) {
216221
return false;
217222
}
218223
const rawExecutable = execution.rawExecutable?.trim() ?? "";
219224
if (!rawExecutable || isPathScopedExecutableToken(rawExecutable)) {
220225
return false;
221226
}
222227
const executableName = normalizeSkillBinName(execution.executableName);
223-
const resolvedPath = normalizeSkillBinResolvedPath(execution.resolvedPath);
228+
const resolvedPath = normalizeSkillBinResolvedPath(trustPath);
224229
if (!executableName || !resolvedPath) {
225230
return false;
226231
}
@@ -439,6 +444,7 @@ function executableResolutionsReferToSameTarget(
439444
return (
440445
left.rawExecutable === right.rawExecutable &&
441446
left.resolvedPath === right.resolvedPath &&
447+
left.resolvedRealPath === right.resolvedRealPath &&
442448
left.executableName === right.executableName
443449
);
444450
}
@@ -534,9 +540,10 @@ function resolveSegmentAllowlistMatch(params: {
534540
params.segment.resolution,
535541
params.context.cwd,
536542
);
543+
const trustPath = resolvePolicyTargetTrustPath(params.segment.resolution, params.context.cwd);
537544
const candidateResolution =
538545
candidatePath && executableResolution
539-
? { ...executableResolution, resolvedPath: candidatePath }
546+
? { ...executableResolution, resolvedPath: candidatePath, resolvedRealPath: trustPath }
540547
: executableResolution;
541548
const inlineCommand = extractBindableShellWrapperInlineCommand(allowlistSegment.argv);
542549
const powerShellFileScriptArgv = resolvePowerShellFileScriptArgv({
@@ -573,6 +580,7 @@ function resolveSegmentAllowlistMatch(params: {
573580
{
574581
rawExecutable: shellPositionalArgvCandidatePath,
575582
resolvedPath: shellPositionalArgvCandidatePath,
583+
resolvedRealPath: resolveCandidateTrustPath(shellPositionalArgvCandidatePath),
576584
executableName: path.basename(shellPositionalArgvCandidatePath),
577585
},
578586
undefined,
@@ -602,6 +610,7 @@ function resolveSegmentAllowlistMatch(params: {
602610
{
603611
rawExecutable: shellScriptCandidatePath,
604612
resolvedPath: shellScriptCandidatePath,
613+
resolvedRealPath: resolveCandidateTrustPath(shellScriptCandidatePath),
605614
executableName: path.basename(shellScriptCandidatePath),
606615
},
607616
shellScriptArgv,
@@ -1096,6 +1105,17 @@ function addAllowAlwaysPattern(
10961105
}
10971106
}
10981107

1108+
function resolveCandidateTrustPath(candidatePath: string | undefined): string | undefined {
1109+
if (!candidatePath) {
1110+
return undefined;
1111+
}
1112+
return resolveExecutableTrustPath({
1113+
rawExecutable: candidatePath,
1114+
resolvedPath: candidatePath,
1115+
executableName: path.basename(candidatePath),
1116+
});
1117+
}
1118+
10991119
function collectAllowAlwaysPatterns(params: {
11001120
segment: ExecCommandSegment;
11011121
cwd?: string;
@@ -1123,7 +1143,7 @@ function collectAllowAlwaysPatterns(params: {
11231143
resolution: resolveCommandResolutionFromArgv(trustPlan.argv, params.cwd, params.env),
11241144
};
11251145

1126-
const candidatePath = resolveExecutionTargetCandidatePath(segment.resolution, params.cwd);
1146+
const candidatePath = resolveExecutionTargetTrustPath(segment.resolution, params.cwd);
11271147
if (!candidatePath) {
11281148
return;
11291149
}
@@ -1152,7 +1172,10 @@ function collectAllowAlwaysPatterns(params: {
11521172
})
11531173
: undefined;
11541174
if (positionalArgvPath) {
1155-
addAllowAlwaysPattern(params.out, positionalArgvPath);
1175+
addAllowAlwaysPattern(
1176+
params.out,
1177+
resolveCandidateTrustPath(positionalArgvPath) ?? positionalArgvPath,
1178+
);
11561179
return;
11571180
}
11581181
if (!inlineCommand) {
@@ -1163,13 +1186,14 @@ function collectAllowAlwaysPatterns(params: {
11631186
cwd: params.cwd,
11641187
});
11651188
if (scriptPath) {
1189+
const scriptTrustPath = resolveCandidateTrustPath(scriptPath) ?? scriptPath;
11661190
const argPattern = buildScriptArgPatternFromArgv(
11671191
powerShellFileScriptArgv ?? params.segment.argv,
11681192
scriptPath,
11691193
params.cwd,
11701194
params.platform,
11711195
);
1172-
addAllowAlwaysPattern(params.out, scriptPath, argPattern);
1196+
addAllowAlwaysPattern(params.out, scriptTrustPath, argPattern);
11731197
}
11741198
return;
11751199
}

‎src/infra/exec-approvals-analysis.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,6 +1123,7 @@ describe("matchAllowlist with argPattern", () => {
11231123
const resolution = {
11241124
rawExecutable: "python3",
11251125
resolvedPath: "/usr/bin/python3",
1126+
resolvedRealPath: "/usr/bin/python3",
11261127
executableName: "python3",
11271128
};
11281129

‎src/infra/exec-approvals-analysis.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,17 @@ export {
1515
parseExecArgvToken,
1616
resolveAllowlistCandidatePath,
1717
resolveApprovalAuditCandidatePath,
18+
resolveApprovalAuditTrustPath,
1819
resolveCommandResolution,
1920
resolveCommandResolutionFromArgv,
2021
resolveExecutionTargetCandidatePath,
2122
resolveExecutionTargetResolution,
23+
resolveExecutionTargetTrustPath,
2224
resolvePolicyAllowlistCandidatePath,
2325
resolvePolicyTargetCandidatePath,
2426
resolvePolicyTargetResolution,
27+
resolvePolicyTargetTrustPath,
28+
resolveExecutableTrustPath,
2529
type CommandResolution,
2630
type ExecutableResolution,
2731
type ExecArgvToken,

‎src/infra/exec-approvals-config.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ describe("exec approvals node host allowlist check", () => {
6363
resolution: {
6464
rawExecutable: "python3",
6565
resolvedPath: "/usr/bin/python3",
66+
resolvedRealPath: "/usr/bin/python3",
6667
executableName: "python3",
6768
},
6869
entries: [{ pattern: "/usr/bin/python3" }],

‎src/infra/exec-approvals-safe-bins.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
SAFE_BIN_PROFILES,
2020
resolveSafeBinProfiles,
2121
} from "./exec-safe-bin-policy.js";
22+
import { buildTrustedSafeBinDirs } from "./exec-safe-bin-trust.js";
2223

2324
describe("exec approvals safe bins", () => {
2425
type SafeBinCase = {
@@ -332,6 +333,44 @@ describe("exec approvals safe bins", () => {
332333
expect(ok).toBe(true);
333334
});
334335

336+
it("checks safe-bin trusted dirs against the real executable identity", () => {
337+
if (process.platform === "win32") {
338+
return;
339+
}
340+
const resolution = {
341+
rawExecutable: "jq",
342+
resolvedPath: "/opt/homebrew/bin/jq",
343+
resolvedRealPath: "/opt/homebrew/Cellar/jq/1.7.1/bin/jq",
344+
executableName: "jq",
345+
};
346+
expect(
347+
isSafeBinUsage({
348+
argv: ["jq", ".foo"],
349+
resolution,
350+
safeBins: normalizeSafeBins(["jq"]),
351+
trustedSafeBinDirs: new Set(["/opt/homebrew/bin"]),
352+
}),
353+
).toBe(false);
354+
expect(
355+
isSafeBinUsage({
356+
argv: ["jq", ".foo"],
357+
resolution,
358+
safeBins: normalizeSafeBins(["jq"]),
359+
trustedSafeBinDirs: buildTrustedSafeBinDirs({
360+
extraDirs: ["/opt/homebrew/Cellar/jq/1.7.1/bin"],
361+
}),
362+
}),
363+
).toBe(true);
364+
expect(
365+
isSafeBinUsage({
366+
argv: ["jq", ".foo"],
367+
resolution,
368+
safeBins: normalizeSafeBins(["jq"]),
369+
trustedSafeBinDirs: new Set(["/tmp/other-bin"]),
370+
}),
371+
).toBe(false);
372+
});
373+
335374
it("supports injected platform for deterministic safe-bin checks", () => {
336375
const ok = isSafeBinUsage({
337376
argv: ["jq", ".foo"],

0 commit comments

Comments
 (0)