diff --git a/agentscore_commerce/checkout.py b/agentscore_commerce/checkout.py index b131cd1..277f3a9 100644 --- a/agentscore_commerce/checkout.py +++ b/agentscore_commerce/checkout.py @@ -2413,7 +2413,26 @@ async def _run_wallet_sanctions_only(self, ctx: CheckoutContext) -> CheckoutResu ) decision = result.get("decision") if isinstance(result, dict) else None - if decision == "deny": + + # Fail closed on anything that is not an explicit allow. This branched on + # `decision == "deny"`, so every other value was permitted by structure: + # None from an unreadable response, and any decision the API adds later. + # This path is strict-liability wallet OFAC screening and already denies + # on an API outage above, so treating an unreadable answer as a pass + # contradicted its own posture. + if decision is None: + reason = DenialReason( + code="api_error", + message="assess returned no decision", + ) + return CheckoutResult( + status=denial_reason_status(reason), + body=denial_reason_to_body(reason), + headers={}, + reference_id=ctx.reference_id, + settled=False, + ) + if decision != "allow": decision_reasons = result.get("decision_reasons") or [] if isinstance(result, dict) else [] reason = DenialReason( code="wallet_not_trusted", diff --git a/agentscore_commerce/checkout_compute_first.py b/agentscore_commerce/checkout_compute_first.py index 9ad835c..8716cc7 100644 --- a/agentscore_commerce/checkout_compute_first.py +++ b/agentscore_commerce/checkout_compute_first.py @@ -495,7 +495,17 @@ async def _enforce_wallet_sanctions( if denial_reason is None: decision = result.get("decision") if isinstance(result, dict) else None - if decision == "deny": + # Fail closed on anything that is not an explicit allow. This branched + # on `decision == "deny"`, so every other value was permitted by + # structure: None from an unreadable response, and any decision the + # API adds later. The except above already denies on an outage, so + # letting an unreadable answer through contradicted that posture. + if decision is None: + denial_reason = DenialReason( + code="api_error", + message="assess returned no decision", + ) + elif decision != "allow": decision_reasons = list(result.get("decision_reasons") or []) if isinstance(result, dict) else [] denial_reason = DenialReason( code="wallet_not_trusted", diff --git a/tests/test_checkout_compute_first_ofac.py b/tests/test_checkout_compute_first_ofac.py index 9b1c4d4..59101ae 100644 --- a/tests/test_checkout_compute_first_ofac.py +++ b/tests/test_checkout_compute_first_ofac.py @@ -135,6 +135,59 @@ async def test_sdn_signer_denies_before_x402_settle( fake_server.verify_payment.assert_not_called() +@pytest.mark.asyncio +async def test_missing_decision_fails_closed(monkeypatch: pytest.MonkeyPatch) -> None: + """A response carrying no decision must DENY rather than settle. + + This branched on `decision == "deny"`, so anything else was permitted by + structure, including None from an unreadable response. The except above + already denies on an outage, so letting an unreadable answer through + contradicted that posture, and here it would have settled a payment. + """ + monkeypatch.setenv("AGENTSCORE_API_KEY", "ask_test_key") + fake_server = _make_fake_x402_server() + checkout = ComputeFirstCheckout( + name="search", + url="https://api.example.com/search", + unit_price_cents=1, + rails=_make_rails(), + x402_server=fake_server, + run_work=_run_one, + ) + await checkout.handle(_req(body={"q": "x"})) + with patch( + "agentscore_commerce.api.AgentScore.aassess", + new=AsyncMock(return_value={"decision_reasons": []}), + ): + result = await checkout.handle(_req(headers={"x-payment": _x402_header()}, body={"q": "x"})) + assert result[1]["error"]["code"] == "api_error" + fake_server.verify_payment.assert_not_called() + + +@pytest.mark.asyncio +async def test_unrecognised_decision_fails_closed(monkeypatch: pytest.MonkeyPatch) -> None: + """A decision value we do not know is not an approval either.""" + monkeypatch.setenv("AGENTSCORE_API_KEY", "ask_test_key") + fake_server = _make_fake_x402_server() + checkout = ComputeFirstCheckout( + name="search", + url="https://api.example.com/search", + unit_price_cents=1, + rails=_make_rails(), + x402_server=fake_server, + run_work=_run_one, + ) + await checkout.handle(_req(body={"q": "x"})) + with patch( + "agentscore_commerce.api.AgentScore.aassess", + new=AsyncMock(return_value={"decision": "review", "decision_reasons": []}), + ): + result = await checkout.handle(_req(headers={"x-payment": _x402_header()}, body={"q": "x"})) + assert result[0] == 403 + assert result[1]["error"]["code"] == "wallet_not_trusted" + fake_server.verify_payment.assert_not_called() + + @pytest.mark.asyncio async def test_clean_signer_continues_to_settle(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("AGENTSCORE_API_KEY", "ask_test_key") diff --git a/tests/test_checkout_wallet_ofac_default.py b/tests/test_checkout_wallet_ofac_default.py index 697daec..d0d89ff 100644 --- a/tests/test_checkout_wallet_ofac_default.py +++ b/tests/test_checkout_wallet_ofac_default.py @@ -124,6 +124,57 @@ async def test_clean_signer_with_no_gate_allows_settle_to_proceed( assert result.status != 403 or "wallet_not_trusted" not in str(result.body) +@pytest.mark.asyncio +@respx.mock +async def test_missing_decision_fails_closed(monkeypatch: pytest.MonkeyPatch) -> None: + """A response carrying no decision must DENY, not fall through as an allow. + + This branched on `decision == "deny"`, so every other value was permitted by + structure: None from an unreadable response, and any decision the API adds + later. This path is strict-liability wallet OFAC screening and already denies + on an API outage, so treating an unreadable answer as a pass contradicted its + own posture. + """ + monkeypatch.setenv("AGENTSCORE_API_KEY", "ask_test_key") + respx.post(ASSESS_URL).mock(return_value=httpx.Response(200, json={"decision_reasons": []})) + checkout = _checkout(gate=None) + request = _req(headers={"x-payment": _x402_payment_header(SDN_WALLET)}) + result = await checkout.handle(request) + assert result.settled is False + assert "api_error" in str(result.body) + + +@pytest.mark.asyncio +@respx.mock +async def test_null_decision_fails_closed(monkeypatch: pytest.MonkeyPatch) -> None: + """An explicit null decision is the same unreadable case as a missing one.""" + monkeypatch.setenv("AGENTSCORE_API_KEY", "ask_test_key") + respx.post(ASSESS_URL).mock(return_value=httpx.Response(200, json={"decision": None, "decision_reasons": []})) + checkout = _checkout(gate=None) + request = _req(headers={"x-payment": _x402_payment_header(SDN_WALLET)}) + result = await checkout.handle(request) + assert result.settled is False + assert "api_error" in str(result.body) + + +@pytest.mark.asyncio +@respx.mock +async def test_unrecognised_decision_fails_closed(monkeypatch: pytest.MonkeyPatch) -> None: + """A decision value we do not know is not an approval either. + + Guards the forward case: adding a decision on the API side must not silently + open this gate on an already-released SDK. + """ + monkeypatch.setenv("AGENTSCORE_API_KEY", "ask_test_key") + _mock_assess("review", reasons=[]) + checkout = _checkout(gate=None) + request = _req(headers={"x-payment": _x402_payment_header(SDN_WALLET)}) + result = await checkout.handle(request) + assert result.status == 403 + assert result.settled is False + assert "wallet_not_trusted" in str(result.body) + + @pytest.mark.asyncio async def test_no_api_key_logs_warn_once_and_skips( monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture