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)
This commit is contained in:
黄云龙 2026-07-11 18:40:07 +08:00 committed by GitHub
parent 7d1a8fb753
commit 4fd521e88e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 18 additions and 1 deletions

View File

@ -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:

View File

@ -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