From 4fd521e88e9ac575c7f690940f22aa5bd3251498 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E9=BB=84=E4=BA=91=E9=BE=99?=
 <76432572+nankingjing@users.noreply.github.com>
Date: Sat, 11 Jul 2026 18:40:07 +0800
Subject: [PATCH] fix(guardrails): empty allowlist must deny all tools instead
 of failing open (#4067)

* fix(guardrails): empty allowlist must deny all tools, not fail open

* test(guardrails): empty allowlist blocks all tools (regression)
---
 .../packages/harness/deerflow/guardrails/builtin.py |  6 +++++-
 backend/tests/test_guardrail_middleware.py          | 13 +++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/backend/packages/harness/deerflow/guardrails/builtin.py b/backend/packages/harness/deerflow/guardrails/builtin.py
index 53ce9f8d8..c1575cc0c 100644
--- a/backend/packages/harness/deerflow/guardrails/builtin.py
+++ b/backend/packages/harness/deerflow/guardrails/builtin.py
@@ -9,7 +9,11 @@ class AllowlistProvider:
     name = "allowlist"
 
     def __init__(self, *, allowed_tools: list[str] | None = None, denied_tools: list[str] | None = None):
-        self._allowed = set(allowed_tools) if allowed_tools else None
+        # Distinguish "no allowlist configured" (None -> allow all) from an
+        # explicitly empty allowlist ([] -> allow nothing). A truthiness test
+        # would collapse [] into None and fail open, letting every tool through
+        # when the operator intended to permit none.
+        self._allowed = set(allowed_tools) if allowed_tools is not None else None
         self._denied = set(denied_tools) if denied_tools else set()
 
     def evaluate(self, request: GuardrailRequest) -> GuardrailDecision:
diff --git a/backend/tests/test_guardrail_middleware.py b/backend/tests/test_guardrail_middleware.py
index 8985d1a97..4aa74df85 100644
--- a/backend/tests/test_guardrail_middleware.py
+++ b/backend/tests/test_guardrail_middleware.py
@@ -114,6 +114,19 @@ class TestAllowlistProvider:
         decision = provider.evaluate(req)
         assert decision.allow is True
 
+    def test_empty_allowlist_blocks_all(self):
+        """An explicitly empty allowlist means "permit no tools" and must fail closed.
+
+        Regression test: a truthiness check would collapse ``[]`` into the
+        ``None`` sentinel ("no allowlist -> allow all"), silently letting every
+        tool through when the operator intended to permit none.
+        """
+        provider = AllowlistProvider(allowed_tools=[])
+        for tool in ("bash", "web_search", "read_file"):
+            decision = provider.evaluate(GuardrailRequest(tool_name=tool, tool_input={}))
+            assert decision.allow is False, f"empty allowlist should block {tool!r}"
+            assert decision.reasons[0].code == "oap.tool_not_allowed"
+
     def test_both_allowed_and_denied(self):
         provider = AllowlistProvider(allowed_tools=["bash", "web_search"], denied_tools=["bash"])
         # bash is in both: allowlist passes, denylist blocks