fix(security): block env depth-overflow approval bypass

This commit is contained in:
Peter Steinberger
2026-02-25 00:13:33 +00:00
parent 1970a1e9e5
commit 57c9a18180
4 changed files with 137 additions and 0 deletions

View File

@@ -45,6 +45,7 @@ Docs: https://docs.openclaw.ai
- Security/Native images: enforce `tools.fs.workspaceOnly` for native prompt image auto-load (including history refs), preventing out-of-workspace sandbox mounts from being implicitly ingested as vision input. This ships in the next npm release. Thanks @tdjackey for reporting.
- Security/Exec approvals: bind `system.run` command display/approval text to full argv when shell-wrapper inline payloads carry positional argv values, and reject payload-only `rawCommand` mismatches for those wrapper-carrier forms, preventing hidden command execution under misleading approval text. This ships in the next npm release. Thanks @tdjackey for reporting.
- Security/Exec companion host: forward canonical `system.run` display text (not payload-only shell snippets) to the macOS exec host, and enforce rawCommand/argv consistency there for shell-wrapper positional-argv carriers and env-modifier preludes, preventing companion-side approval/display drift. This ships in the next npm release. Thanks @tdjackey for reporting.
- Security/Exec approvals: fail closed when transparent dispatch-wrapper unwrapping exceeds the depth cap, so nested `/usr/bin/env` chains cannot bypass shell-wrapper approval gating in `allowlist` + `ask=on-miss` mode. This ships in the next npm release. Thanks @tdjackey for reporting.
- Security/Exec: limit default safe-bin trusted directories to immutable system paths (`/bin`, `/usr/bin`) and require explicit opt-in (`tools.exec.safeBinTrustedDirs`) for package-manager/user bin paths (for example Homebrew), add security-audit findings for risky trusted-dir choices, warn at runtime when explicitly trusted dirs are group/world writable, and add doctor hints when configured `safeBins` resolve outside trusted dirs. This ships in the next npm release. Thanks @tdjackey for reporting.
- Telegram/Media fetch: prioritize IPv4 before IPv6 in SSRF pinned DNS address ordering so media downloads still work on hosts with broken IPv6 routing. (#24295, #23975) Thanks @Glucksberg.
- Telegram/Replies: when markdown formatting renders to empty HTML (for example syntax-only chunks in threaded replies), retry delivery with plain text, and fail loud when both formatted and plain payloads are empty to avoid false delivered states. (#25096, #25091) Thanks @Glucksberg.

View File

@@ -299,6 +299,36 @@ describe("exec approvals command resolution", () => {
expect(allowlistEval.segmentSatisfiedBy).toEqual([null]);
});
it("fails closed when transparent env wrappers exceed unwrap depth", () => {
if (process.platform === "win32") {
return;
}
const dir = makeTempDir();
const binDir = path.join(dir, "bin");
fs.mkdirSync(binDir, { recursive: true });
const envPath = path.join(binDir, "env");
fs.writeFileSync(envPath, "#!/bin/sh\n");
fs.chmodSync(envPath, 0o755);
const analysis = analyzeArgvCommand({
argv: [envPath, envPath, envPath, envPath, envPath, "/bin/sh", "-c", "echo pwned"],
cwd: dir,
env: makePathEnv(binDir),
});
const allowlistEval = evaluateExecAllowlist({
analysis,
allowlist: [{ pattern: envPath }],
safeBins: normalizeSafeBins([]),
cwd: dir,
});
expect(analysis.ok).toBe(true);
expect(analysis.segments[0]?.resolution?.policyBlocked).toBe(true);
expect(analysis.segments[0]?.resolution?.blockedWrapper).toBe("env");
expect(allowlistEval.allowlistSatisfied).toBe(false);
expect(allowlistEval.segmentSatisfiedBy).toEqual([null]);
});
it("unwraps env wrapper with shell inner executable", () => {
const resolution = resolveCommandResolutionFromArgv(["/usr/bin/env", "bash", "-lc", "echo hi"]);
expect(resolution?.rawExecutable).toBe("bash");

View File

@@ -478,6 +478,17 @@ export function resolveDispatchWrapperExecutionPlan(
}
current = unwrap.argv;
}
if (wrappers.length >= maxDepth) {
const overflow = unwrapKnownDispatchWrapperInvocation(current);
if (overflow.kind === "blocked" || overflow.kind === "unwrapped") {
return {
argv: current,
wrappers,
policyBlocked: true,
blockedWrapper: overflow.wrapper,
};
}
}
return { argv: current, wrappers, policyBlocked: false };
}

View File

@@ -348,4 +348,99 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
}),
);
});
it("denies nested env shell payloads when wrapper depth is exceeded", async () => {
if (process.platform === "win32") {
return;
}
const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-env-depth-overflow-"));
const previousOpenClawHome = process.env.OPENCLAW_HOME;
const marker = path.join(tempHome, "pwned.txt");
process.env.OPENCLAW_HOME = tempHome;
saveExecApprovals({
version: 1,
defaults: {
security: "allowlist",
ask: "on-miss",
askFallback: "deny",
},
agents: {
main: {
allowlist: [{ pattern: "/usr/bin/env" }],
},
},
});
const runCommand = vi.fn(async () => {
fs.writeFileSync(marker, "executed");
return {
success: true,
stdout: "local-ok",
stderr: "",
timedOut: false,
truncated: false,
exitCode: 0,
error: null,
};
});
const sendInvokeResult = vi.fn(async () => {});
const sendNodeEvent = vi.fn(async () => {});
try {
await handleSystemRunInvoke({
client: {} as never,
params: {
command: [
"/usr/bin/env",
"/usr/bin/env",
"/usr/bin/env",
"/usr/bin/env",
"/usr/bin/env",
"/bin/sh",
"-c",
`echo PWNED > ${marker}`,
],
sessionKey: "agent:main:main",
},
skillBins: {
current: async () => [],
},
execHostEnforced: false,
execHostFallbackAllowed: true,
resolveExecSecurity: () => "allowlist",
resolveExecAsk: () => "on-miss",
isCmdExeInvocation: () => false,
sanitizeEnv: () => undefined,
runCommand,
runViaMacAppExecHost: vi.fn(async () => null),
sendNodeEvent,
buildExecEventPayload: (payload) => payload,
sendInvokeResult,
sendExecFinishedEvent: vi.fn(async () => {}),
preferMacAppExecHost: false,
});
} finally {
if (previousOpenClawHome === undefined) {
delete process.env.OPENCLAW_HOME;
} else {
process.env.OPENCLAW_HOME = previousOpenClawHome;
}
fs.rmSync(tempHome, { recursive: true, force: true });
}
expect(runCommand).not.toHaveBeenCalled();
expect(fs.existsSync(marker)).toBe(false);
expect(sendNodeEvent).toHaveBeenCalledWith(
expect.anything(),
"exec.denied",
expect.objectContaining({ reason: "approval-required" }),
);
expect(sendInvokeResult).toHaveBeenCalledWith(
expect.objectContaining({
ok: false,
error: expect.objectContaining({
message: "SYSTEM_RUN_DENIED: approval required",
}),
}),
);
});
});