mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-19 05:01:15 +00:00
fix(exec): align node shell allowlist wrappers (#62401)
* fix(exec): align node shell allowlist wrappers * fix: align node shell allowlist wrappers (#62401) (thanks @ngutman)
This commit is contained in:
@@ -38,6 +38,7 @@ Docs: https://docs.openclaw.ai
|
||||
- TUI: route `/status` through the shared session-status command, keep commentary hidden in history, strip raw envelope metadata from async command notices, preserve fallback streaming before per-attempt failures finalize, and restore Kitty keyboard state on exit or fatal crashes. (#49130, #59985, #60043, #61463) Thanks @biefan, @MoerAI, @jwchmodx, and @100yenadmin.
|
||||
- Sessions/model selection: resolve the explicitly selected session model separately from runtime fallback resolution so session status and live model switching stay aligned with the chosen model.
|
||||
- iOS/Watch exec approvals: keep Apple Watch review and approval recovery working while the iPhone is locked or backgrounded, including reconnect recovery, pending approval persistence, notification cleanup, and APNs-backed watch refresh recovery. (#61757) Thanks @ngutman.
|
||||
- Nodes/exec approvals: keep `host=node` POSIX transport shell wrappers (`/bin/sh -lc ...`) aligned with inner-command allowlist analysis so allowlisted scripts stop prompting unnecessarily, while Windows `cmd.exe` wrapper runs stay approval-gated. (#62401) Thanks @ngutman.
|
||||
- Agents/context overflow: combine oversized and aggregate tool-result recovery in one pass and restore a total-context overflow backstop so recoverable sessions retry instead of failing early. (#61651) Thanks @Takhoffman.
|
||||
- Agents/exec: preserve explicit `host=node` routing under elevated defaults when `tools.exec.host=auto`, fail loud on invalid elevated cross-host overrides, and keep `strictInlineEval` commands blocked after approval timeouts instead of falling through to automatic execution. (#61739) Thanks @obviyus.
|
||||
- Providers/Ollama: honor the selected provider's `baseUrl` during streaming so multi-Ollama setups stop routing every stream to the first configured Ollama endpoint. (#61678)
|
||||
|
||||
@@ -127,12 +127,13 @@ describe("evaluateSystemRunPolicy", () => {
|
||||
expect(denied.errorMessage).toBe("SYSTEM_RUN_DENIED: allowlist miss");
|
||||
});
|
||||
|
||||
it("treats shell wrappers as allowlist misses", () => {
|
||||
const denied = expectDeniedDecision(
|
||||
it("keeps POSIX shell wrapper decisions tied to allowlist analysis", () => {
|
||||
const allowed = expectAllowedDecision(
|
||||
evaluateSystemRunPolicy(buildPolicyParams({ shellWrapperInvocation: true })),
|
||||
);
|
||||
expect(denied.shellWrapperBlocked).toBe(true);
|
||||
expect(denied.errorMessage).toContain("shell wrappers like sh/bash/zsh -c");
|
||||
expect(allowed.shellWrapperBlocked).toBe(false);
|
||||
expect(allowed.analysisOk).toBe(true);
|
||||
expect(allowed.allowlistSatisfied).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps Windows-specific guidance for cmd.exe wrappers", () => {
|
||||
|
||||
@@ -61,9 +61,16 @@ export function evaluateSystemRunPolicy(params: {
|
||||
cmdInvocation: boolean;
|
||||
shellWrapperInvocation: boolean;
|
||||
}): SystemRunPolicyDecision {
|
||||
const shellWrapperBlocked = params.security === "allowlist" && params.shellWrapperInvocation;
|
||||
// POSIX node execution intentionally uses `/bin/sh -lc` as a transport wrapper.
|
||||
// Keep allowlist decisions based on the analyzed inner shell payload there.
|
||||
// Windows `cmd.exe /c` wrappers still require explicit approval because they
|
||||
// change execution semantics for builtins and quoting/parsing behavior.
|
||||
const windowsShellWrapperBlocked =
|
||||
shellWrapperBlocked && params.isWindows && params.cmdInvocation;
|
||||
params.security === "allowlist" &&
|
||||
params.shellWrapperInvocation &&
|
||||
params.isWindows &&
|
||||
params.cmdInvocation;
|
||||
const shellWrapperBlocked = windowsShellWrapperBlocked;
|
||||
const analysisOk = shellWrapperBlocked ? false : params.analysisOk;
|
||||
const allowlistSatisfied = shellWrapperBlocked ? false : params.allowlistSatisfied;
|
||||
const approvedByAsk = params.approvalDecision !== null || params.approved === true;
|
||||
|
||||
@@ -383,6 +383,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
|
||||
sendExecFinishedEvent?: HandleSystemRunInvokeOptions["sendExecFinishedEvent"];
|
||||
sendNodeEvent?: HandleSystemRunInvokeOptions["sendNodeEvent"];
|
||||
skillBinsCurrent?: () => Promise<Array<{ name: string; resolvedPath: string }>>;
|
||||
isCmdExeInvocation?: HandleSystemRunInvokeOptions["isCmdExeInvocation"];
|
||||
}): Promise<{
|
||||
runCommand: MockedRunCommand;
|
||||
runViaMacAppExecHost: MockedRunViaMacAppExecHost;
|
||||
@@ -441,7 +442,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
|
||||
execHostFallbackAllowed: true,
|
||||
resolveExecSecurity: () => params.security ?? "full",
|
||||
resolveExecAsk: () => params.ask ?? "off",
|
||||
isCmdExeInvocation: () => false,
|
||||
isCmdExeInvocation: params.isCmdExeInvocation ?? (() => false),
|
||||
sanitizeEnv: () => undefined,
|
||||
runCommand,
|
||||
runViaMacAppExecHost,
|
||||
@@ -1512,6 +1513,93 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it.runIf(process.platform !== "win32")(
|
||||
"auto-runs allowlisted inner scripts through transport shell wrappers",
|
||||
async () => {
|
||||
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-shell-wrapper-inner-"));
|
||||
try {
|
||||
const scriptsDir = path.join(tempDir, "scripts");
|
||||
fs.mkdirSync(scriptsDir, { recursive: true });
|
||||
const scriptPath = path.join(scriptsDir, "check_mail.sh");
|
||||
fs.writeFileSync(scriptPath, "#!/bin/sh\necho ok\n");
|
||||
fs.chmodSync(scriptPath, 0o755);
|
||||
|
||||
await withTempApprovalsHome({
|
||||
approvals: createAllowlistOnMissApprovals({
|
||||
agents: {
|
||||
main: {
|
||||
allowlist: [{ pattern: scriptPath }],
|
||||
},
|
||||
},
|
||||
}),
|
||||
run: async () => {
|
||||
const invoke = await runSystemInvoke({
|
||||
preferMacAppExecHost: false,
|
||||
command: ["/bin/sh", "-lc", "./scripts/check_mail.sh --limit 5"],
|
||||
rawCommand: '/bin/sh -lc "./scripts/check_mail.sh --limit 5"',
|
||||
cwd: tempDir,
|
||||
security: "allowlist",
|
||||
ask: "on-miss",
|
||||
runCommand: vi.fn(async () => createLocalRunResult("shell-wrapper-inner-ok")),
|
||||
});
|
||||
|
||||
expect(invoke.runCommand).toHaveBeenCalledTimes(1);
|
||||
expectInvokeOk(invoke.sendInvokeResult, {
|
||||
payloadContains: "shell-wrapper-inner-ok",
|
||||
});
|
||||
},
|
||||
});
|
||||
} finally {
|
||||
fs.rmSync(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("keeps cmd.exe transport wrappers approval-gated on Windows", async () => {
|
||||
const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32");
|
||||
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-cmd-wrapper-allow-"));
|
||||
try {
|
||||
const scriptPath = path.join(tempDir, "check_mail.cmd");
|
||||
fs.writeFileSync(scriptPath, "@echo off\r\necho ok\r\n");
|
||||
|
||||
await withTempApprovalsHome({
|
||||
approvals: createAllowlistOnMissApprovals({
|
||||
agents: {
|
||||
main: {
|
||||
allowlist: [{ pattern: scriptPath }],
|
||||
},
|
||||
},
|
||||
}),
|
||||
run: async () => {
|
||||
const invoke = await runSystemInvoke({
|
||||
preferMacAppExecHost: false,
|
||||
command: ["cmd.exe", "/d", "/s", "/c", `${scriptPath} --limit 5`],
|
||||
cwd: tempDir,
|
||||
security: "allowlist",
|
||||
ask: "on-miss",
|
||||
isCmdExeInvocation: (argv) => {
|
||||
const token = argv[0]?.trim();
|
||||
if (!token) {
|
||||
return false;
|
||||
}
|
||||
const base = path.win32.basename(token).toLowerCase();
|
||||
return base === "cmd.exe" || base === "cmd";
|
||||
},
|
||||
});
|
||||
|
||||
expect(invoke.runCommand).not.toHaveBeenCalled();
|
||||
expectApprovalRequiredDenied({
|
||||
sendNodeEvent: invoke.sendNodeEvent,
|
||||
sendInvokeResult: invoke.sendInvokeResult,
|
||||
});
|
||||
},
|
||||
});
|
||||
} finally {
|
||||
platformSpy.mockRestore();
|
||||
fs.rmSync(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("reuses exact-command durable trust for shell-wrapper reruns", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
|
||||
@@ -359,9 +359,10 @@ async function evaluateSystemRunPolicyPhase(
|
||||
.find((entry) => entry !== null) ?? null)
|
||||
: null;
|
||||
const isWindows = process.platform === "win32";
|
||||
const cmdInvocation = parsed.shellPayload
|
||||
? opts.isCmdExeInvocation(segments[0]?.argv ?? [])
|
||||
: opts.isCmdExeInvocation(parsed.argv);
|
||||
// Detect Windows wrapper transport from the original request argv, not the
|
||||
// analyzed inner shell payload. Once parsing unwraps `cmd.exe /c ...`, the
|
||||
// inner segments no longer retain the wrapper marker we need for policy.
|
||||
const cmdInvocation = opts.isCmdExeInvocation(parsed.argv);
|
||||
const durableApprovalSatisfied = hasDurableExecApproval({
|
||||
analysisOk,
|
||||
segmentAllowlistEntries,
|
||||
@@ -407,13 +408,8 @@ async function evaluateSystemRunPolicyPhase(
|
||||
return null;
|
||||
}
|
||||
|
||||
// Fail closed if policy/runtime drift re-allows unapproved shell wrappers.
|
||||
if (
|
||||
security === "allowlist" &&
|
||||
parsed.shellPayload &&
|
||||
!policy.approvedByAsk &&
|
||||
!durableApprovalSatisfied
|
||||
) {
|
||||
// Fail closed if policy/runtime drift re-allows Windows shell wrappers.
|
||||
if (policy.shellWrapperBlocked && !policy.approvedByAsk && !durableApprovalSatisfied) {
|
||||
await sendSystemRunDenied(opts, parsed.execution, {
|
||||
reason: "approval-required",
|
||||
message: "SYSTEM_RUN_DENIED: approval required",
|
||||
|
||||
Reference in New Issue
Block a user