fix(tool-policy): deny write no longer silently hides apply_patch (#76749)

Removing deny coupling between write and apply_patch in
makeToolPolicyMatcher. Previously deny: ["write"] would also block
apply_patch, contradicting the documented allow coupling (apply_patch
inherits write allow). Allow inheritance is preserved; deny must be
stated explicitly.

Adds regression tests verifying:
- deny: ["write"] leaves apply_patch accessible
- deny: ["apply_patch"] still denies directly
- allow: ["write"] still grants apply_patch
- deny: ["write", "apply_patch"] still denies when both explicit

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
HCL
2026-05-03 23:06:02 +08:00
committed by Peter Steinberger
parent 9185e500bd
commit 8da9d8c55f
2 changed files with 21 additions and 3 deletions

View File

@@ -16,9 +16,6 @@ function makeToolPolicyMatcher(policy: SandboxToolPolicy) {
if (matchesAnyGlobPattern(normalized, deny)) {
return false;
}
if (normalized === "apply_patch" && matchesAnyGlobPattern("write", deny)) {
return false;
}
if (allow.length === 0) {
return true;
}

View File

@@ -3,6 +3,7 @@ import type { OpenClawConfig } from "../config/config.js";
import { DEFAULT_GATEWAY_HTTP_TOOL_DENY } from "../security/dangerous-tools.js";
import { isToolAllowed, resolveSandboxToolPolicyForAgent } from "./sandbox/tool-policy.js";
import type { SandboxToolPolicy } from "./sandbox/types.js";
import { isToolAllowedByPolicyName } from "./tool-policy-match.js";
import { TOOL_POLICY_CONFORMANCE } from "./tool-policy.conformance.js";
import {
applyOwnerOnlyToolPolicy,
@@ -264,3 +265,23 @@ describe("resolveSandboxToolPolicyForAgent", () => {
expect(resolved.deny).toEqual(["image"]);
});
});
describe("isToolAllowedByPolicyName — apply_patch / write deny decoupling (#76749)", () => {
it("does not deny apply_patch when write is denied", () => {
expect(isToolAllowedByPolicyName("apply_patch", { deny: ["write"] })).toBe(true);
});
it("still denies apply_patch when apply_patch is explicitly denied", () => {
expect(isToolAllowedByPolicyName("apply_patch", { deny: ["apply_patch"] })).toBe(false);
});
it("still allows apply_patch via write in the allow list", () => {
expect(isToolAllowedByPolicyName("apply_patch", { allow: ["write"], deny: [] })).toBe(true);
});
it("denies apply_patch when both write and apply_patch are denied", () => {
expect(isToolAllowedByPolicyName("apply_patch", { deny: ["write", "apply_patch"] })).toBe(
false,
);
});
});