fix: harden exec sandbox fallback semantics (#23398) (thanks @bmendonca3)

This commit is contained in:
Peter Steinberger
2026-02-22 10:49:15 +01:00
parent b7d45b3695
commit 67f6e3bfd4
8 changed files with 49 additions and 7 deletions

View File

@@ -62,6 +62,7 @@ Docs: https://docs.openclaw.ai
- Security/Config: make parsed chat allowlist checks fail closed when `allowFrom` is empty, restoring expected DM/pairing gating.
- Security/Exec: in non-default setups that manually add `sort` to `tools.exec.safeBins`, block `sort --compress-program` so allowlist-mode safe-bin checks cannot bypass approval. Thanks @tdjackey for reporting.
- Security/Exec approvals: when users choose `allow-always` for shell-wrapper commands (for example `/bin/zsh -lc ...`), persist allowlist patterns for the inner executable(s) instead of the wrapper shell binary, preventing accidental broad shell allowlisting in moderate mode. (#23276) Thanks @xrom2863.
- Security/Exec: fail closed when `tools.exec.host=sandbox` is configured/requested but sandbox runtime is unavailable, and default implicit exec host routing to `gateway` when no sandbox runtime exists. (#23398) Thanks @bmendonca3.
- Security/macOS app beta: enforce path-only `system.run` allowlist matching (drop basename matches like `echo`), migrate legacy basename entries to last resolved paths when available, and harden shell-chain handling to fail closed on unsafe parse/control syntax (including quoted command substitution/backticks). This is an optional allowlist-mode feature; default installs remain deny-by-default. This ships in the next npm release. Thanks @tdjackey for reporting.
- Security/Agents: auto-generate and persist a dedicated `commands.ownerDisplaySecret` when `commands.ownerDisplay=hash`, remove gateway token fallback from owner-ID prompt hashing across CLI and embedded agent runners, and centralize owner-display secret resolution in one shared helper. This ships in the next npm release. Thanks @aether-ai-agent for reporting.
- Security/SSRF: expand IPv4 fetch guard blocking to include RFC special-use/non-global ranges (including benchmarking, TEST-NET, multicast, and reserved/broadcast blocks), and centralize range checks into a single CIDR policy table to reduce classifier drift.

View File

@@ -49,7 +49,7 @@ Notes:
- `tools.exec.notifyOnExit` (default: true): when true, backgrounded exec sessions enqueue a system event and request a heartbeat on exit.
- `tools.exec.approvalRunningNoticeMs` (default: 10000): emit a single “running” notice when an approval-gated exec runs longer than this (0 disables).
- `tools.exec.host` (default: `sandbox`)
- `tools.exec.host` (default: runtime-aware: `sandbox` when sandbox runtime is active, `gateway` otherwise)
- `tools.exec.security` (default: `deny` for sandbox, `allowlist` for gateway + node when unset)
- `tools.exec.ask` (default: `on-miss`)
- `tools.exec.node` (default: unset)

View File

@@ -127,4 +127,31 @@ describe("exec host env validation", () => {
}),
).rejects.toThrow(/Security Violation: Environment variable 'LD_DEBUG' is forbidden/);
});
it("defaults to gateway when sandbox runtime is unavailable", async () => {
const { createExecTool } = await import("./bash-tools.exec.js");
const tool = createExecTool({ security: "full", ask: "off" });
const err = await tool
.execute("call1", {
command: "echo ok",
host: "sandbox",
})
.then(() => null)
.catch((error: unknown) => (error instanceof Error ? error : new Error(String(error))));
expect(err).toBeTruthy();
expect(err?.message).toMatch(/exec host not allowed/);
expect(err?.message).toMatch(/tools\.exec\.host=gateway/);
});
it("fails closed when sandbox host is explicitly configured without sandbox runtime", async () => {
const { createExecTool } = await import("./bash-tools.exec.js");
const tool = createExecTool({ host: "sandbox", security: "full", ask: "off" });
await expect(
tool.execute("call1", {
command: "echo ok",
}),
).rejects.toThrow(/sandbox runtime is unavailable/);
});
});

View File

@@ -33,7 +33,12 @@ test("exec disposes PTY listeners after normal exit", async () => {
kill: vi.fn(),
}));
const tool = createExecTool({ allowBackground: false });
const tool = createExecTool({
allowBackground: false,
host: "gateway",
security: "full",
ask: "off",
});
const result = await tool.execute("toolcall", {
command: "echo ok",
pty: true,
@@ -64,7 +69,12 @@ test("exec tears down PTY resources on timeout", async () => {
kill,
}));
const tool = createExecTool({ allowBackground: false });
const tool = createExecTool({
allowBackground: false,
host: "gateway",
security: "full",
ask: "off",
});
await expect(
tool.execute("toolcall", {
command: "sleep 5",

View File

@@ -26,7 +26,12 @@ test("exec cleans session state when PTY fallback spawn also fails", async () =>
.mockRejectedValueOnce(new Error("pty spawn failed"))
.mockRejectedValueOnce(new Error("child fallback failed"));
const tool = createExecTool({ allowBackground: false });
const tool = createExecTool({
allowBackground: false,
host: "gateway",
security: "full",
ask: "off",
});
await expect(
tool.execute("toolcall", {

View File

@@ -279,7 +279,7 @@ export function createExecTool(
if (elevatedRequested) {
logInfo(`exec: elevated command ${truncateMiddle(params.command, 120)}`);
}
const configuredHost = defaults?.host ?? "sandbox";
const configuredHost = defaults?.host ?? (defaults?.sandbox ? "sandbox" : "gateway");
const sandboxHostConfigured = defaults?.host === "sandbox";
const requestedHost = normalizeExecHost(params.host) ?? null;
let host: ExecHost = requestedHost ?? configuredHost;

View File

@@ -602,7 +602,6 @@ describe("Agent-specific tool filtering", () => {
tools: {
deny: ["process"],
exec: {
host: "gateway",
security: "full",
ask: "off",
},

View File

@@ -178,7 +178,7 @@ export type GroupToolPolicyConfig = {
export type GroupToolPolicyBySenderConfig = Record<string, GroupToolPolicyConfig>;
export type ExecToolConfig = {
/** Exec host routing (default: sandbox). */
/** Exec host routing (default: sandbox with sandbox runtime, otherwise gateway). */
host?: "sandbox" | "gateway" | "node";
/** Exec security mode (default: deny). */
security?: "deny" | "allowlist" | "full";