From d95be2c384a3fcf0f91cb00436bd429ff4a1ec0e Mon Sep 17 00:00:00 2001
From: Peter Steinberger <steipete@gmail.com>
Date: Mon, 16 Feb 2026 01:52:03 +0000
Subject: [PATCH] fix: preserve sandbox allow-all semantics

---
 src/agents/sandbox/tool-policy.e2e.test.ts | 21 ------
 src/agents/sandbox/tool-policy.ts          |  3 +
 src/agents/tool-policy.e2e.test.ts         | 88 ++++++++++++++++++++++
 3 files changed, 91 insertions(+), 21 deletions(-)
 delete mode 100644 src/agents/sandbox/tool-policy.e2e.test.ts

diff --git a/src/agents/sandbox/tool-policy.e2e.test.ts b/src/agents/sandbox/tool-policy.e2e.test.ts
deleted file mode 100644
index 319a84a9749..00000000000
--- a/src/agents/sandbox/tool-policy.e2e.test.ts
+++ /dev/null
@@ -1,21 +0,0 @@
-import { describe, expect, it } from "vitest";
-import type { SandboxToolPolicy } from "./types.js";
-import { isToolAllowed } from "./tool-policy.js";
-
-describe("sandbox tool policy", () => {
-  it("allows all tools with * allow", () => {
-    const policy: SandboxToolPolicy = { allow: ["*"], deny: [] };
-    expect(isToolAllowed(policy, "browser")).toBe(true);
-  });
-
-  it("denies all tools with * deny", () => {
-    const policy: SandboxToolPolicy = { allow: [], deny: ["*"] };
-    expect(isToolAllowed(policy, "read")).toBe(false);
-  });
-
-  it("supports wildcard patterns", () => {
-    const policy: SandboxToolPolicy = { allow: ["web_*"] };
-    expect(isToolAllowed(policy, "web_fetch")).toBe(true);
-    expect(isToolAllowed(policy, "read")).toBe(false);
-  });
-});
diff --git a/src/agents/sandbox/tool-policy.ts b/src/agents/sandbox/tool-policy.ts
index b50a363846b..083cfcac4af 100644
--- a/src/agents/sandbox/tool-policy.ts
+++ b/src/agents/sandbox/tool-policy.ts
@@ -89,6 +89,9 @@ export function resolveSandboxToolPolicyForAgent(
   // `image` is essential for multimodal workflows; always include it in sandboxed
   // sessions unless explicitly denied.
   if (
+    // Empty allowlist means "allow all" for `isToolAllowed`, so don't inject a
+    // single tool that would accidentally turn it into an explicit allowlist.
+    expandedAllow.length > 0 &&
     !expandedDeny.map((v) => v.toLowerCase()).includes("image") &&
     !expandedAllow.map((v) => v.toLowerCase()).includes("image")
   ) {
diff --git a/src/agents/tool-policy.e2e.test.ts b/src/agents/tool-policy.e2e.test.ts
index 171a1723da6..9fb44696b6b 100644
--- a/src/agents/tool-policy.e2e.test.ts
+++ b/src/agents/tool-policy.e2e.test.ts
@@ -1,5 +1,8 @@
 import { describe, expect, it } from "vitest";
+import type { OpenClawConfig } from "../config/config.js";
+import type { SandboxToolPolicy } from "./sandbox/types.js";
 import type { AnyAgentTool } from "./tools/common.js";
+import { isToolAllowed, resolveSandboxToolPolicyForAgent } from "./sandbox/tool-policy.js";
 import { TOOL_POLICY_CONFORMANCE } from "./tool-policy.conformance.js";
 import {
   applyOwnerOnlyToolPolicy,
@@ -94,3 +97,88 @@ describe("TOOL_POLICY_CONFORMANCE", () => {
     expect(() => JSON.stringify(TOOL_POLICY_CONFORMANCE)).not.toThrow();
   });
 });
+
+describe("sandbox tool policy", () => {
+  it("allows all tools with * allow", () => {
+    const policy: SandboxToolPolicy = { allow: ["*"], deny: [] };
+    expect(isToolAllowed(policy, "browser")).toBe(true);
+  });
+
+  it("denies all tools with * deny", () => {
+    const policy: SandboxToolPolicy = { allow: [], deny: ["*"] };
+    expect(isToolAllowed(policy, "read")).toBe(false);
+  });
+
+  it("supports wildcard patterns", () => {
+    const policy: SandboxToolPolicy = { allow: ["web_*"] };
+    expect(isToolAllowed(policy, "web_fetch")).toBe(true);
+    expect(isToolAllowed(policy, "read")).toBe(false);
+  });
+
+  it("applies deny before allow", () => {
+    const policy: SandboxToolPolicy = { allow: ["*"], deny: ["web_*"] };
+    expect(isToolAllowed(policy, "web_fetch")).toBe(false);
+    expect(isToolAllowed(policy, "read")).toBe(true);
+  });
+
+  it("treats empty allowlist as allow-all (with deny exceptions)", () => {
+    const policy: SandboxToolPolicy = { allow: [], deny: ["web_*"] };
+    expect(isToolAllowed(policy, "web_fetch")).toBe(false);
+    expect(isToolAllowed(policy, "read")).toBe(true);
+  });
+
+  it("expands tool groups + aliases in patterns", () => {
+    const policy: SandboxToolPolicy = {
+      allow: ["group:fs", "BASH"],
+      deny: ["apply_*"],
+    };
+    expect(isToolAllowed(policy, "read")).toBe(true);
+    expect(isToolAllowed(policy, "exec")).toBe(true);
+    expect(isToolAllowed(policy, "apply_patch")).toBe(false);
+  });
+
+  it("normalizes whitespace + case", () => {
+    const policy: SandboxToolPolicy = { allow: [" WEB_* "] };
+    expect(isToolAllowed(policy, "WEB_FETCH")).toBe(true);
+  });
+});
+
+describe("resolveSandboxToolPolicyForAgent", () => {
+  it("keeps allow-all semantics when allow is []", () => {
+    const cfg = {
+      tools: { sandbox: { tools: { allow: [], deny: ["browser"] } } },
+    } as unknown as OpenClawConfig;
+
+    const resolved = resolveSandboxToolPolicyForAgent(cfg, undefined);
+    expect(resolved.sources.allow).toEqual({
+      source: "global",
+      key: "tools.sandbox.tools.allow",
+    });
+    expect(resolved.allow).toEqual([]);
+    expect(resolved.deny).toEqual(["browser"]);
+
+    const policy: SandboxToolPolicy = { allow: resolved.allow, deny: resolved.deny };
+    expect(isToolAllowed(policy, "read")).toBe(true);
+    expect(isToolAllowed(policy, "browser")).toBe(false);
+  });
+
+  it("auto-adds image to explicit allowlists unless denied", () => {
+    const cfg = {
+      tools: { sandbox: { tools: { allow: ["read"], deny: ["browser"] } } },
+    } as unknown as OpenClawConfig;
+
+    const resolved = resolveSandboxToolPolicyForAgent(cfg, undefined);
+    expect(resolved.allow).toEqual(["read", "image"]);
+    expect(resolved.deny).toEqual(["browser"]);
+  });
+
+  it("does not auto-add image when explicitly denied", () => {
+    const cfg = {
+      tools: { sandbox: { tools: { allow: ["read"], deny: ["image"] } } },
+    } as unknown as OpenClawConfig;
+
+    const resolved = resolveSandboxToolPolicyForAgent(cfg, undefined);
+    expect(resolved.allow).toEqual(["read"]);
+    expect(resolved.deny).toEqual(["image"]);
+  });
+});