harden: fail closed on any decision that is not an explicit allow

evaluate_governance_sync/_async enumerated the bad decisions — raise on deny,
raise on approval_required, return otherwise. That is a denylist: any value
outside the known set reaches the `return` and the guarded action executes.

This is the same shape @sidclaw/sdk carried until 13c6ab3, where it was a live
governance bypass. It is NOT currently exploitable here: EvaluateResponse
declares `decision: PolicyEffect` as a Literal, and the client returns
`EvaluateResponse.model_validate(...)`, so pydantic rejects an unrecognised
decision before middleware sees it. Verified end to end against a mocked
server returning {"decision": "quarantine"} — it raises ValidationError.

So this is defence in depth, not a fix, and it is deliberately NOT being
published as a hotfix. The reason to add it anyway: the only thing making the
denylist safe is a Literal two modules away. Widen PolicyEffect to add a
decision value — exactly what a new server-side decision would require — and
pydantic stops rejecting it while the denylist silently starts allowing it.
The TypeScript SDK reached its bypass by that route.

tests/test_middleware/test_fail_closed.py covers both layers: that pydantic
rejects unknown decisions, and that the decision-site guard fires when
validation is bypassed (model_construct simulates a widened Literal).
Mutation-checked — removing the guard fails 8 of them.

Version bumped to 0.2.1 with no PyPI release. Leaving it at 0.2.0 would put
different source under an already-published version string, which is the
invisible drift that let three security fixes sit unshipped on the npm side
for three months. Local ahead of PyPI says "unreleased work exists"; local
equal to PyPI while differing says nothing at all.

186 tests pass (167 + 19 new).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: VladUZH

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
 
 [project]
 name = "sidclaw"
-version = "0.2.0"
+version = "0.2.1"
 description = "Python SDK for SidClaw — governance for AI agents"
 readme = "README.md"
 license = "Apache-2.0"

src/sidclaw/_constants.py

@@ -1,4 +1,4 @@
-SDK_VERSION = "0.2.0"
+SDK_VERSION = "0.2.1"
 DEFAULT_BASE_URL = "https://api.sidclaw.com"
 DEFAULT_MAX_RETRIES = 3
 DEFAULT_TIMEOUT = 30.0

src/sidclaw/middleware/_base.py

@@ -16,7 +16,12 @@ def evaluate_governance_sync(
     data_classification: DataClassification = "internal",
     context: dict[str, Any] | None = None,
 ) -> EvaluateResponse:
-    """Evaluate an action and raise on deny/approval_required."""
+    """Evaluate an action, allowing it only on an explicit ``allow`` decision.
+
+    Raises ``ActionDeniedError`` on ``deny``, on ``approval_required``, and on
+    any other value — the guarded action proceeds only when the policy engine
+    explicitly permits it.
+    """
     decision = client.evaluate(
         EvaluateParams(
             operation=operation,
@@ -41,6 +46,20 @@ def evaluate_governance_sync(
             policy_rule_id=decision.policy_rule_id,
         )
 
+    # Fail closed on anything that is not an explicit allow.
+    #
+    # Enumerating only the bad decisions is a denylist: an unknown, misspelled,
+    # absent, null, or newly-added decision value fell through to ALLOW and the
+    # tool executed ungoverned. @sidclaw/sdk fixed this in 13c6ab3; this is the
+    # Python port of that fix. The authoritative set is PolicyEffectValues =
+    # ('allow', 'approval_required', 'deny').
+    if decision.decision != "allow":
+        raise ActionDeniedError(
+            f"Unexpected policy decision: {decision.decision!r}",
+            trace_id=decision.trace_id,
+            policy_rule_id=decision.policy_rule_id,
+        )
+
     return decision
 
 
@@ -53,7 +72,12 @@ async def evaluate_governance_async(
     data_classification: DataClassification = "internal",
     context: dict[str, Any] | None = None,
 ) -> EvaluateResponse:
-    """Async version: evaluate an action and raise on deny/approval_required."""
+    """Async version: allow only on an explicit ``allow`` decision.
+
+    Raises ``ActionDeniedError`` on ``deny``, on ``approval_required``, and on
+    any other value — the guarded action proceeds only when the policy engine
+    explicitly permits it.
+    """
     decision = await client.evaluate(
         EvaluateParams(
             operation=operation,
@@ -78,6 +102,20 @@ async def evaluate_governance_async(
             policy_rule_id=decision.policy_rule_id,
         )
 
+    # Fail closed on anything that is not an explicit allow.
+    #
+    # Enumerating only the bad decisions is a denylist: an unknown, misspelled,
+    # absent, null, or newly-added decision value fell through to ALLOW and the
+    # tool executed ungoverned. @sidclaw/sdk fixed this in 13c6ab3; this is the
+    # Python port of that fix. The authoritative set is PolicyEffectValues =
+    # ('allow', 'approval_required', 'deny').
+    if decision.decision != "allow":
+        raise ActionDeniedError(
+            f"Unexpected policy decision: {decision.decision!r}",
+            trace_id=decision.trace_id,
+            policy_rule_id=decision.policy_rule_id,
+        )
+
     return decision
 
 

tests/test_middleware/test_fail_closed.py

@@ -0,0 +1,130 @@
+"""The guarded action runs only on an explicit ``allow``.
+
+Two layers enforce this, and these tests cover both:
+
+1. ``EvaluateResponse.decision`` is a ``Literal``, so pydantic rejects any
+   value the SDK does not know about before middleware ever sees it.
+2. ``evaluate_governance_*`` re-checks for an explicit ``allow`` at the
+   decision site.
+
+Layer 2 looks redundant against layer 1 today — that is the point. The
+TypeScript SDK had only the equivalent of layer 2, wrote it as a denylist
+("raise on deny, raise on approval_required, otherwise proceed"), and shipped
+a live governance bypass: any unrecognised decision became ALLOW. Python is
+safe from that today only because of layer 1. The moment someone widens the
+``PolicyEffect`` literal to add a decision value, layer 1 stops rejecting it
+and layer 2 is the only thing standing between a new server decision and an
+ungoverned tool call. These tests fail if that guard is ever removed.
+"""
+
+import httpx
+import pytest
+import respx
+
+from sidclaw import AsyncSidClaw, SidClaw
+from sidclaw._errors import ActionDeniedError
+from sidclaw._types import EvaluateResponse
+from sidclaw.middleware._base import evaluate_governance_async, evaluate_governance_sync
+
+
+def _body(decision: str) -> dict:
+    return {
+        "decision": decision,
+        "trace_id": "t-1",
+        "approval_request_id": None,
+        "reason": "because",
+        "policy_rule_id": None,
+    }
+
+
+@pytest.fixture
+def fc_client():
+    return SidClaw(api_key="k", base_url="https://fc.api", agent_id="a", max_retries=0)
+
+
+@pytest.fixture
+def fc_async_client():
+    return AsyncSidClaw(api_key="k", base_url="https://fc.api", agent_id="a", max_retries=0)
+
+
+@pytest.fixture
+def fc_mock():
+    with respx.mock(base_url="https://fc.api") as m:
+        yield m
+
+
+class TestLayerOneRejectsUnknownDecisions:
+    """An unrecognised decision never reaches middleware."""
+
+    @pytest.mark.parametrize("decision", ["quarantine", "log", "ALLOW", "Allow", "", "allow_once"])
+    def test_unknown_decision_does_not_allow(self, fc_client, fc_mock, decision):
+        fc_mock.post("/api/v1/evaluate").mock(return_value=httpx.Response(200, json=_body(decision)))
+        # Must raise something. What matters is that it does NOT return and
+        # let the caller proceed — that is the fail-open failure mode.
+        with pytest.raises(Exception):
+            evaluate_governance_sync(fc_client, "op")
+
+    def test_missing_decision_field_does_not_allow(self, fc_client, fc_mock):
+        fc_mock.post("/api/v1/evaluate").mock(
+            return_value=httpx.Response(200, json={"trace_id": "t-1", "reason": "no decision key"})
+        )
+        with pytest.raises(Exception):
+            evaluate_governance_sync(fc_client, "op")
+
+
+class TestLayerTwoGuardsTheDecisionSite:
+    """The explicit-allow check, exercised directly.
+
+    ``model_construct`` skips pydantic validation, which simulates the world
+    where ``PolicyEffect`` has been widened and layer 1 no longer rejects the
+    value. Without the guard in ``_base.py`` these calls return normally and
+    the guarded action executes ungoverned.
+    """
+
+    @pytest.mark.parametrize("decision", ["quarantine", "log", "", "ALLOW"])
+    def test_sync_raises_on_non_allow(self, fc_client, monkeypatch, decision):
+        monkeypatch.setattr(
+            fc_client,
+            "evaluate",
+            lambda _params: EvaluateResponse.model_construct(
+                decision=decision, trace_id="t-1", approval_request_id=None, reason="r", policy_rule_id=None
+            ),
+        )
+        with pytest.raises(ActionDeniedError, match="Unexpected policy decision"):
+            evaluate_governance_sync(fc_client, "op")
+
+    @pytest.mark.parametrize("decision", ["quarantine", "log", "", "ALLOW"])
+    async def test_async_raises_on_non_allow(self, fc_async_client, monkeypatch, decision):
+        async def _fake(_params):
+            return EvaluateResponse.model_construct(
+                decision=decision, trace_id="t-1", approval_request_id=None, reason="r", policy_rule_id=None
+            )
+
+        monkeypatch.setattr(fc_async_client, "evaluate", _fake)
+        with pytest.raises(ActionDeniedError, match="Unexpected policy decision"):
+            await evaluate_governance_async(fc_async_client, "op")
+
+
+class TestKnownDecisionsStillBehave:
+    """The guard must not change the three documented outcomes."""
+
+    def test_allow_returns(self, fc_client, fc_mock):
+        fc_mock.post("/api/v1/evaluate").mock(return_value=httpx.Response(200, json=_body("allow")))
+        assert evaluate_governance_sync(fc_client, "op").decision == "allow"
+
+    def test_deny_raises(self, fc_client, fc_mock):
+        fc_mock.post("/api/v1/evaluate").mock(return_value=httpx.Response(200, json=_body("deny")))
+        with pytest.raises(ActionDeniedError):
+            evaluate_governance_sync(fc_client, "op")
+
+    def test_approval_required_raises(self, fc_client, fc_mock):
+        fc_mock.post("/api/v1/evaluate").mock(
+            return_value=httpx.Response(200, json=_body("approval_required"))
+        )
+        with pytest.raises(ActionDeniedError, match="Approval required"):
+            evaluate_governance_sync(fc_client, "op")
+
+    async def test_async_allow_returns(self, fc_async_client, fc_mock):
+        fc_mock.post("/api/v1/evaluate").mock(return_value=httpx.Response(200, json=_body("allow")))
+        result = await evaluate_governance_async(fc_async_client, "op")
+        assert result.decision == "allow"