feat(fygaro): server-signed card checkout with a pre-charge allowance check - #486
Merged
Conversation
… check On 2026-08-16 a customer topped up $80 with only $4.48 of daily allowance left. Fygaro captured the card, the webhook correctly refused the credit, and the customer was left charged and empty-handed pending a manual fix. Two things made that possible. The app built the Fygaro URL on the device with `?amount=` and `?custom_reference=` in the query string, both of which the payer can edit in the WebView before paying. And the daily limit was only ever checked in the webhook — after the card was charged, which is the one moment when "no" costs the customer money. This moves the decision in front of the charge and takes the amount out of the customer's hands: - `fygaroCheckoutCreate` authorises an amount against the minimum, the single-payment ceiling and the account's remaining daily allowance, then returns a checkout URL with every payment parameter signed into a JWT (`?jwt=`) and nothing editable in the query string. A refusal now happens before any money moves, and carries the remaining allowance so the client can say how much would be accepted. - The webhook cross-checks what was paid against what was authorised and records — never credits — a mismatch, so a signing or config mistake on our side cannot silently become a credit. A missing or expired intent is not a mismatch: it is the legacy position, and the existing credit gate still applies in full. - Unreadable settings or history refuse the authorisation rather than defaulting to a clean slate, so an ERPNext outage cannot hand every account its full allowance again. Off by default behind `fygaro.checkout.enabled`; the JWT link format needs a Fygaro plan that supports it. With the flag off, behaviour is unchanged and the device-built URL keeps working, as it must for every app version already in the wild. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
Review fixes on the pre-charge authorisation path. - Check the yaml master gates (fygaro.enabled, fygaro.credit.enabled) before minting anything. credit.enabled ships default-false, so the very first rollout state -- checkout on, credit off -- would have minted signed links, charged the card and recorded the payment with reason `credit-disabled`. With fygaro.enabled off the webhook 503s and the payment is not even recorded. - Run evaluateCreditGate instead of reimplementing four of its seven gates. The duplicated ladder had already dropped `non-positive-net`, so a top-up above the minimum whose fixed fees sink the net to <= 0 was authorised here and refused there. A probe call with an unknown history keeps the ERPNext read off the paths that fail before it. - Make authorisation a reservation: live intents are indexed per account in a Redis sorted set and subtracted from the remaining allowance, then re-read after reserving so a race backs out rather than over-issuing. Without it, N calls against a $100 allowance each minted a $100 link. - Gate intent redemption on consumeCacheKey's atomic DEL removed-count rather than get-then-clear, which the cache layer's own docstring calls out as a TOCTOU race where every concurrent consumer "succeeds". - Split reading an intent from redeeming it. The webhook consumed it before the gate had decided anything, so every delivery that returns 500 for a provider retry burned it and the retry that actually credited ran with no cross-check. Redemption now happens only on the terminal 200 paths. - Backdate the JWT `nbf` by 60s. Zero leeway against Fygaro's clock means a server a second fast mints a link rejected as "not yet valid" -- a payer-facing dead end. Inbound already tolerates 5 minutes of skew. - Give `no-daily-limit-for-level` its own FYGARO_LEVEL_NOT_ELIGIBLE error. A level-0 account was told "could not check your allowance right now", forever. - Drop the fabricated `intent-preauth-<accountId>` sentinel: excludeRequestId is now optional and simply omitted where there is no row to exclude. - Fix the input-validation branch of the resolver: CentAmount returns an Apollo InputValidationError, which fell off the end of mapError's switch and threw assertUnreachable, turning a bad client amount into an internal error. Tests: the webhook intent branch had zero coverage -- added match, both mismatch shapes, legacy fall-through and terminal-only redemption to payment.spec.ts; new specs for the intent store and the resolver; pre-charge spec extended for the master gates, reservations and gate parity.
Review fixes on the signed-checkout PR. Seven findings, all about the
reservation the pre-charge check now takes.
- An abandoned payment page used to lock the account out for the full
JWT window with a refusal that was also false ("you have $0.00 left of
today's top-up limit" to an account that had spent nothing), and the
client had no way to clear its own hold. Settled spend and live holds
are now told apart: the intent record carries the signed URL, so a
retry for the same amount is handed its OWN live link back, and when it
cannot be re-offered the refusal is `checkout-already-open` /
FYGARO_CHECKOUT_ALREADY_OPEN, naming the amount and how long the hold
has left (relative, not a UTC clock time the customer cannot act on).
- The webhook leaked the hold on two terminal answers that did not
credit: the credit-FAILURE 200 and the unattributed 200. The ERPNext
row already counts the payment, so a leaked hold double-counted it and
refused the account's next legitimate top-up on top of an already-bad
event. Both now release the reservation without consuming the record,
so a manual credit still has the cross-check.
- A Redis reservation-index failure was reported as `history-unavailable`
and refused in silence: the operator went to ERPNext for a fault in
Redis, and a 100%-unavailable pre-charge path paged nobody. It now has
its own reason, and all three transient refusals fire a warning with a
static per-reason dedup key.
- The race-rollback reported the stale pre-race allowance — a bigger
number than reality in exactly the case where it is wrong. Recomputed
from the re-read, minus the reservation just released.
- fygaroCheckoutCreate had no rate limit while running a Frappe list
query per call, and `under-minimum` sits AFTER the history gate, so
fygaroCheckoutCreate(amount: 1) in a loop was unbounded ERPNext load
against the read every other top-up depends on. Per-account limiter,
same as invoiceCreate / onChainAddressCreate / inviteCreate.
- The intent-store docstring claimed Redis never blocks a top-up. Since
the reservation index fails closed, it does. Rewritten to say which
half is fail-closed and which is fail-open.
- readIntent warned on CacheUndefinedError, i.e. on every ordinary miss
(expired, evicted, already redeemed, every re-delivery), which buried
the line that means "Redis is broken". Only real faults warn now.
18 new unit tests; 1920 total green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
Three gaps left by the previous round, all in what happens when the store this feature now depends on is unreachable. `RedisRateLimitService.consume` collapsed every rejection into RateLimiterExceededError. With no insuranceLimiter configured a Redis outage rejects with the raw store error, so the outage was reported to users as "too many card top-up attempts, please wait" — on their first attempt of the day — and paged nobody. It now returns UnknownRateLimitServiceError for a store fault and keeps the exceeded error for a genuine RateLimiterRes breach; `consumeLimiter` only translates the latter into the caller's own error. This is shared infrastructure, so the correction applies to every limiter (login, invoice create, invite): all of them still refuse on a fault, they just stop misnaming it. The checkout mutation then treats a limiter fault as "limiter unavailable" and falls through to authorizeFygaroTopup, which fails closed on its own Redis read and alerts as `reservations-unavailable`. That restores the documented triage contract, and cannot open an abuse window: no link can be minted while the store holding the reservation index is unreachable. The reservation READ was fail-closed while the WRITE was fail-open, so a failover landing between them minted a full-allowance link with no hold — the exact over-issue the reservation exists to prevent. A failed reservation write is now its own error class and refuses; a failed cross-check record still proceeds, because that half genuinely is only verification. Not adopted as suggested: the review asked for `checkout-already-open` to report allowance against settled spend only. Holding an $80 link against a $100 cap would then report "$100 remaining", inviting a retry that must fail. The number was already right — what was wrong was the schema description implying it described spending. It now says outright that open unpaid links are subtracted, and `holdAmountCents` / `holdExpiresAt` travel alongside it so the client can explain why. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
bobodread876
approved these changes
Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The incident this fixes
On 2026-08-16 an account with $4.48 of daily top-up allowance left paid $80 through Fygaro. Fygaro captured the card, the webhook correctly refused the credit, and the customer was left charged and empty-handed until it was credited by hand.
Two things made that possible:
?amount=and?custom_reference=in the query string. Both are editable by the payer in the WebView before they pay — so the amount the app showed and the amount actually charged were never the same value.What changes
The decision moves in front of the charge. A new
fygaroCheckoutCreatemutation authorises an amount before the customer is handed to Fygaro. It does not reimplement the decision: it callsevaluateCreditGate, the same function the webhook runs after the charge, so the two answers cannot drift.Authorising is a reservation, not just a check. Live unpaid links are held in a per-account index and subtracted from the allowance, then re-read after reserving so two racing requests both back out rather than both minting. Without this, N calls against a $100 allowance each mint a $100 link, and paying two captures $200 while only one gets credited.
The amount stops being the customer's to edit. The returned URL carries every payment parameter inside an HS256 JWT (
?jwt=) and nothing editable in the query string.custom_referencebecomesusername|intentId— still attributable, now also verifiable.Abandoning a checkout is not a lockout. Opening a payment page and walking away is the most common thing a payer does. A retry for the same amount gets that same link back rather than a refusal, and a retry for a different amount is told what is being held and when it expires — not "you have $0.00 left" to an account that has spent nothing.
The webhook cross-checks what was paid against what we authorised. A mismatch on amount or account records the payment and refuses the credit, with both numbers in the alert. Holds are released on every terminal non-crediting path, so a treasury-float failure does not leave the payment counted twice.
Behaviour when a dependency is down
This is the part worth reading before an on-call page.
fygaroCheckoutCreaterefuses every request withFYGARO_ALLOWANCE_UNAVAILABLEand pages viafygaro:authorize-unavailable:reservations-unavailable. Card top-ups are unavailable rather than over-issued. (An earlier version of this description said a Redis failure never blocks a top-up. That was true before the reservation work and is no longer true — corrected here so nobody rules Redis out on my say-so.)RedisRateLimitService.consumecollapsed every rejection into the exceeded error, so an outage told users they were rate limited on their first attempt and paged nobody. Store faults are nowUnknownRateLimitServiceError; only a realRateLimiterResbreach becomes the caller's rate-limit error. This is shared infrastructure — login, invoice create and invite are affected. All of them still refuse on a fault; they just stop misnaming it.What deliberately does not change
BLOCKEDfor API keys, alongside the cashout and Bridge money-movement mutations: it mints a link that charges a card.Rollout
Off by default behind
fygaro.checkout.enabled, and additionally refuses unless thefygaro.enabledandfygaro.credit.enabledmaster gates are both on — otherwise the first rollout state (checkout on, credit still off) would mint links that charge and never credit.The JWT link format requires a Fygaro plan that supports it (Pro or above) — turning this on without the plan breaks checkout rather than degrading, so the plan tier needs confirming before the flag is set.
Tests
194 suites / 1924 tests green. New coverage includes a direct replay of the incident ($4.48 left, $80 requested → refused with the remaining amount reported), the reservation race and rollback, abandoned-link reuse, hold release on each terminal webhook path, both master gates, the webhook cross-check (amount mismatch, account mismatch, matching intent, missing intent), and the rate-limiter fault-vs-breach split.
Follow-ups (not in this PR)
🤖 Generated with Claude Code
https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV