Skip to content

Commit 2f43daf

Browse files
jesse-merhijesse-merhi
committed
restore exact command allow-always fallback
1 parent e49def0 commit 2f43daf

5 files changed

Lines changed: 61 additions & 26 deletions

File tree

‎src/agents/bash-tools.exec-host-node-phases.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ export async function analyzeNodeApprovalRequirement(params: {
329329
)}.`,
330330
);
331331
}
332-
if ((params.hostAsk === "always" || params.hostSecurity === "allowlist") && analysisOk) {
332+
if (params.hostAsk === "always" || params.hostSecurity === "allowlist") {
333333
try {
334334
const approvalsSnapshot = await callGatewayTool<{ file: string }>(
335335
"exec.approvals.node.get",

‎src/agents/bash-tools.exec.approval-id.test.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -853,7 +853,7 @@ describe("exec approvals", () => {
853853
expect(calls).toContain("exec.approval.request");
854854
});
855855

856-
it("keeps prompt-only node shell-wrapper reruns approval-gated despite exact-command durable trust", async () => {
856+
it("allows prompt-only node shell-wrapper reruns with exact-command durable trust", async () => {
857857
const calls: string[] = [];
858858
vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => {
859859
calls.push(method);
@@ -905,8 +905,9 @@ describe("exec approvals", () => {
905905
command: "cd .",
906906
});
907907

908-
expect(result.details.status).toBe("approval-pending");
909-
expect(calls).toContain("exec.approval.request");
908+
expect(result.details.status).toBe("completed");
909+
expect(getResultText(result)).toContain("node-shell-wrapper-ok");
910+
expect(calls).not.toContain("exec.approval.request");
910911
});
911912

912913
it("requires approval for elevated ask when allowlist misses", async () => {

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -757,14 +757,16 @@ describe("exec approvals store helpers", () => {
757757
expect(patterns).not.toEqual([{ pattern: earlyTool, argPattern: undefined }]);
758758
});
759759

760-
it("blocks exact-command allow-always fallback for unsafe POSIX shell", async () => {
760+
it("keeps exact-command allow-always fallback available for POSIX shell text", async () => {
761761
if (process.platform === "win32") {
762762
return;
763763
}
764764
const dir = createHomeDir();
765765

766766
for (const commandText of [
767767
"$(cat /tmp/tool) --help",
768+
'echo "$(date)"',
769+
"python -c 'print(1)'",
768770
"printf x > out.txt",
769771
"echo 'unterminated",
770772
"sh -c './tool'",
@@ -787,7 +789,7 @@ describe("exec approvals store helpers", () => {
787789
env: { PATH: process.env.PATH ?? "" },
788790
platform: process.platform,
789791
}),
790-
).resolves.toBe(false);
792+
).resolves.toBe(true);
791793
}
792794
});
793795

@@ -800,7 +802,7 @@ describe("exec approvals store helpers", () => {
800802
).resolves.toBe(true);
801803
});
802804

803-
it("blocks exact-command allow-always fallback for resolved POSIX executables", async () => {
805+
it("keeps exact-command allow-always fallback available for resolved POSIX executables", async () => {
804806
if (process.platform === "win32") {
805807
return;
806808
}
@@ -816,7 +818,7 @@ describe("exec approvals store helpers", () => {
816818
env: { PATH: binDir },
817819
platform: process.platform,
818820
}),
819-
).resolves.toBe(false);
821+
).resolves.toBe(true);
820822
});
821823

822824
it("keeps planner-backed allow-always persistence scoped to POSIX executable units", async () => {
@@ -901,7 +903,7 @@ describe("exec approvals store helpers", () => {
901903
await expect(persistCommand("python -c 'print(1)'")).resolves.toStrictEqual([]);
902904
});
903905

904-
it("does not persist allow-always patterns when POSIX shell analysis fails", async () => {
906+
it("leaves exact-command fallback available when POSIX shell analysis fails", async () => {
905907
if (process.platform === "win32") {
906908
return;
907909
}
@@ -936,6 +938,15 @@ describe("exec approvals store helpers", () => {
936938

937939
expect(analysis.analysisOk).toBe(false);
938940
expect(patterns).toStrictEqual([]);
941+
await expect(
942+
canPersistExactCommandAllowAlways({
943+
analysisOk: analysis.analysisOk,
944+
commandText,
945+
cwd: dir,
946+
env,
947+
platform: process.platform,
948+
}),
949+
).resolves.toBe(true);
939950
expect(allowlistEntries(dir, "worker")).toStrictEqual([]);
940951
});
941952

‎src/infra/exec-approvals.ts‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,15 +1209,7 @@ export async function canPersistExactCommandAllowAlways(params: {
12091209
platform?: string | null;
12101210
}): Promise<boolean> {
12111211
const commandText = params.commandText?.trim();
1212-
if (!commandText || params.analysisOk === false) {
1213-
return false;
1214-
}
1215-
if (isWindowsPlatform(params.platform)) {
1216-
return true;
1217-
}
1218-
// POSIX exact command hashes do not bind the resolved executable identity. Use
1219-
// planner-backed path entries instead so PATH changes cannot inherit trust.
1220-
return false;
1212+
return Boolean(commandText);
12211213
}
12221214

12231215
export async function persistAllowAlwaysPatterns(params: {

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

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1595,7 +1595,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
15951595
}
15961596
});
15971597

1598-
it("does not persist exact-command allow-always approvals for planner-rejected shell", async () => {
1598+
it("persists exact-command allow-always approvals for planner-rejected shell", async () => {
15991599
if (process.platform === "win32") {
16001600
return;
16011601
}
@@ -1617,7 +1617,41 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
16171617

16181618
expect(runCommand).toHaveBeenCalledTimes(1);
16191619
expectInvokeOk(sendInvokeResult, { payloadContains: "allow-always-ok" });
1620-
expect(loadExecApprovals().agents?.main?.allowlist ?? []).toStrictEqual([]);
1620+
const allowlist = loadExecApprovals().agents?.main?.allowlist ?? [];
1621+
expect(allowlist).toHaveLength(1);
1622+
expect(allowlist[0]).toEqual(
1623+
expect.objectContaining({
1624+
pattern: expect.stringMatching(/^=command:[0-9a-f]{16}$/),
1625+
source: "allow-always",
1626+
}),
1627+
);
1628+
1629+
const rerun = await runSystemInvoke({
1630+
preferMacAppExecHost: false,
1631+
command: ["/bin/sh", "-lc", "printf x > out.txt"],
1632+
rawCommand: "printf x > out.txt",
1633+
cwd: createFixtureDir("openclaw-exact-fallback-rerun-"),
1634+
security: "allowlist",
1635+
ask: "on-miss",
1636+
runCommand: vi.fn(async () => createLocalRunResult("exact-rerun-ok")),
1637+
});
1638+
expect(rerun.runCommand).toHaveBeenCalledTimes(1);
1639+
expectInvokeOk(rerun.sendInvokeResult, { payloadContains: "exact-rerun-ok" });
1640+
1641+
const changed = await runSystemInvoke({
1642+
preferMacAppExecHost: false,
1643+
command: ["/bin/sh", "-lc", "printf y > out.txt"],
1644+
rawCommand: "printf y > out.txt",
1645+
cwd: createFixtureDir("openclaw-exact-fallback-changed-"),
1646+
security: "allowlist",
1647+
ask: "on-miss",
1648+
runCommand: vi.fn(async () => createLocalRunResult("changed-ok")),
1649+
});
1650+
expect(changed.runCommand).not.toHaveBeenCalled();
1651+
expectApprovalRequiredDenied({
1652+
sendNodeEvent: changed.sendNodeEvent,
1653+
sendInvokeResult: changed.sendInvokeResult,
1654+
});
16211655
},
16221656
});
16231657
});
@@ -2434,7 +2468,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
24342468
}
24352469
});
24362470

2437-
it("keeps prompt-only shell-wrapper reruns approval-gated despite exact-command durable trust", async () => {
2471+
it("allows prompt-only shell-wrapper reruns with exact-command durable trust", async () => {
24382472
if (process.platform === "win32") {
24392473
return;
24402474
}
@@ -2480,11 +2514,8 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
24802514
runCommand: vi.fn(async () => createLocalRunResult("shell-wrapper-reused")),
24812515
});
24822516

2483-
expect(rerun.runCommand).not.toHaveBeenCalled();
2484-
expectApprovalRequiredDenied({
2485-
sendNodeEvent: rerun.sendNodeEvent,
2486-
sendInvokeResult: rerun.sendInvokeResult,
2487-
});
2517+
expect(rerun.runCommand).toHaveBeenCalledTimes(1);
2518+
expectInvokeOk(rerun.sendInvokeResult, { payloadContains: "shell-wrapper-reused" });
24882519
},
24892520
});
24902521
});

0 commit comments

Comments
 (0)