diff --git a/src/le_agent_sdk/l402/client.py b/src/le_agent_sdk/l402/client.py index b7d3f63..c4973cf 100644 --- a/src/le_agent_sdk/l402/client.py +++ b/src/le_agent_sdk/l402/client.py @@ -329,38 +329,42 @@ def _check_amount_against_max( challenge: L402Challenge | MppChallenge, effective_max: Optional[int], ) -> None: - """Enforce the payment ceiling before any invoice reaches the wallet. + """Gate an invoice before any of it reaches the wallet. - The invariant is: an amount that cannot be determined is refused. - A budget is a guarantee ("never pay more than N"), and an invoice whose - amount cannot be proven <= N cannot be paid without breaking it. The - wallet callback is arbitrary caller-supplied code, so handing it an - unbounded invoice delegates an unbounded spend. + Two independent rules (ledger #71): - No ceiling configured (None) means the caller explicitly opted out of - budget enforcement, so any invoice — known or unknown — is allowed - through; refusing there would add no safety and break the documented - "None means no limit" contract. + 1. **Unknown/unbounded amount => ALWAYS refuse**, whether or not a ceiling + is configured. An amount that cannot be determined (amountless + invoice, unparseable, or <= 0 — all of which the decoder reports as + None) cannot be proven bounded, and the wallet callback is arbitrary + caller-supplied code: handing it such an invoice delegates an + unbounded, unaudited spend. This is the fail-closed core. Previously + the gate short-circuited when no max was set and paid ANYTHING, so a + caller who merely forgot to set a ceiling opted into unbounded spend. + + 2. **Ceiling comparison** (``amount > max`` => refuse) applies ONLY when a + max is configured. With no ceiling the caller has opted out of a limit + for a *known* amount — that is their explicit choice and is honored; + only the unknown-amount hole above is closed unconditionally. Raises: - ValueError: If the invoice exceeds the ceiling, or if a ceiling is - configured and the amount cannot be determined. + ValueError: If the amount cannot be determined (always), or if a + ceiling is configured and the amount exceeds it. """ - if effective_max is None: - return - invoice_sats = self._decode_invoice_amount_sats(challenge.invoice) + # Rule 1: fail closed on an amount we cannot bound, ceiling or not. if invoice_sats is None: raise ValueError( - "Invoice amount could not be determined, and a maximum of " - f"{effective_max} sats is configured. Refusing to pay: an " - "invoice with no verifiable amount cannot be checked against a " - "budget and would hand an unbounded payment to the wallet " - f"callback. Invoice: {challenge.invoice[:40]}..." + "Invoice amount could not be determined (amountless, unparseable, " + "or <= 0). Refusing to pay: an invoice with no verifiable amount " + "cannot be bounded and would hand an unbounded payment to the " + "wallet callback — this is refused even when no maximum is " + f"configured. Invoice: {challenge.invoice[:40]}..." ) - if invoice_sats > effective_max: + # Rule 2: enforce the ceiling only when one was set. + if effective_max is not None and invoice_sats > effective_max: raise ValueError( f"Invoice amount ({invoice_sats} sats) exceeds maximum allowed " f"({effective_max} sats). Invoice: {challenge.invoice[:40]}..." diff --git a/tests/test_security_regressions.py b/tests/test_security_regressions.py index d1d1a51..a467885 100644 --- a/tests/test_security_regressions.py +++ b/tests/test_security_regressions.py @@ -298,11 +298,16 @@ async def pay_callback(invoice): assert paid == [] @pytest.mark.asyncio - async def test_unparseable_invoice_still_paid_when_no_budget_configured(self): - """No budget configured == caller explicitly accepted unbounded payment. - - Refusing here would break the documented `None means no limit` contract - without adding safety: with no limit, a known 10M-sat invoice is paid too. + async def test_unparseable_invoice_refused_even_when_no_budget_configured(self): + """Ledger #71: an unknown/unbounded amount is refused even with no max. + + Previously this PAID (fail-open): with ``max_amount_sats=None`` the gate + short-circuited and handed ANY invoice to the wallet, so a caller who + forgot to set a ceiling delegated an unbounded, unaudited spend. The + fail-closed rule is now independent of the ceiling: an amount that cannot + be determined (amountless / unparseable / <= 0) is ALWAYS refused. A + *known* amount with no ceiling is still paid (that is the caller's + documented opt-out) — see test_known_amount_paid_when_no_budget_configured. """ garbage = "not-a-parseable-bolt11-invoice" paid = [] @@ -318,10 +323,37 @@ async def pay_callback(invoice): mock_http.request.side_effect = [_make_402(garbage), _make_ok()] mock_ensure.return_value = mock_http + with pytest.raises(ValueError, match="amount"): + await client.access("https://x.test/r") + + assert paid == [], "unbounded invoice was handed to the wallet with no max set" + + @pytest.mark.asyncio + async def test_known_amount_paid_when_no_budget_configured(self): + """The opt-out still holds for a KNOWN amount: no ceiling => pay. + + This pins the other half of ledger #71 so the fail-closed fix does not + over-reach into forcing every payment to declare a max. ``lnbc10u`` is a + determinable 1000-sat invoice; with no ceiling it is paid. + """ + known = "lnbc10u1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rq" # 1000 sats + paid = [] + + async def pay_callback(invoice): + paid.append(invoice) + return "ab" * 32 + + client = L402Client(pay_invoice_callback=pay_callback, max_amount_sats=None) + + with patch.object(client, "_ensure_client") as mock_ensure: + mock_http = AsyncMock() + mock_http.request.side_effect = [_make_402(known), _make_ok()] + mock_ensure.return_value = mock_http + resp = await client.access("https://x.test/r") assert resp.status_code == 200 - assert paid == [garbage] + assert paid == [known] class TestReputationIgnoresOutOfRangeRatings: