From 346df6c6726ca46ac1f46e5b36af5364865e8ff1 Mon Sep 17 00:00:00 2001 From: Dave Liu <7david12liu@gmail.com> Date: Mon, 13 Jul 2026 18:00:43 -0700 Subject: [PATCH] PAY-005A1: contain invalid partial refunds --- GITHUB_ISSUES.md | 6 +- GITHUB_ISSUE_SLICES.md | 1 + OPERATIONS_RUNBOOK.md | 2 + SECURITY.md | 1 + docs/officers/EVENTS_SHOP_MEMBERS.md | 45 ++++ functions/adminOrderAction.js | 44 +++- functions/adminRegistrationAction.js | 58 +++-- functions/commerceControlEntryPoints.test.js | 210 +++++++++++++++++++ functions/refundValidation.js | 37 ++++ functions/refundValidation.test.js | 76 +++++++ 10 files changed, 451 insertions(+), 29 deletions(-) create mode 100644 functions/refundValidation.js create mode 100644 functions/refundValidation.test.js diff --git a/GITHUB_ISSUES.md b/GITHUB_ISSUES.md index 23d55447..dce78e1d 100644 --- a/GITHUB_ISSUES.md +++ b/GITHUB_ISSUES.md @@ -856,13 +856,15 @@ Do not mark local cancelled before considering the live Stripe Session and recov ## PAY-005 — Build idempotent refunds, disputes, and reconciliation **Labels:** `priority:P0`, `type:security`, `type:reliability`, `area:stripe`, `size:L` -**Status:** Proposed +**Status:** Partial. Immediate amount-containment source/tests are tracked in [#200](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/200); the broader refund/dispute/reconciliation system remains proposed. **Depends on:** PAY-002, PAY-003, AUTH-003 for final permissions ### Problem Refund functions call Stripe without idempotency, immediately overwrite local status, weakly validate amount/current state/remaining balance, and do not track cumulative actual refund. Dispute handling is registration-only and audit-only. No scheduled reconciliation proves Stripe and Firestore agree. +PAY-005A1 [#200](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/200) is the narrow immediate containment outcome. It makes both current partial-refund entry points accept only a primitive positive safe-integer amount below the stored original cents, requires every admitted partial request to send that exact `amount` to Stripe, and derives partial/full classification from the explicit action. Invalid caller or stored values stop before Stripe construction/refund creation and before business-record writes. A provider exception returns a fixed result-not-confirmed, do-not-retry, and escalate message because Stripe may already have accepted the request. This does not add idempotency, remaining-refundable provider truth, concurrency control, event-finalized totals, capabilities, deployment, or live proof. + ### Scope - Create refund command records with stable ID, actor/capability, reason, requested integer cents, payment snapshot, status, and Stripe idempotency key. @@ -890,7 +892,7 @@ Refund functions call Stripe without idempotency, immediately overwrite local st ### Agent handoff -Separate refund command/state from the broader reconciliation worker into sequential PRs if necessary. Never hand-edit Firestore to make reconciliation pass. +Do not duplicate PAY-005A1 [#200](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/200). After its immediate containment, separate refund command/state from the broader reconciliation worker into sequential PRs. Never hand-edit Firestore to make reconciliation pass. --- diff --git a/GITHUB_ISSUE_SLICES.md b/GITHUB_ISSUE_SLICES.md index be522ef4..6951df8a 100644 --- a/GITHUB_ISSUE_SLICES.md +++ b/GITHUB_ISSUE_SLICES.md @@ -95,6 +95,7 @@ An agent-ready child has one observable outcome, one trust boundary, explicit de | ID | Status/dependency | Bounded deliverable | Close evidence | | --- | --- | --- | --- | +| PAY-005A1 | tracked by [#200](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/200); immediate containment only | Reject malformed/equal/over partial amounts in both existing admin Functions before Stripe/write side effects; every admitted partial request carries the exact integer amount and only explicit full refund omits it. | Red/green dual-endpoint matrices for caller/stored values, valid boundaries, provider failure, exact payload/classification; source/merge/deploy/provider/live states separated. | | PAY-005A | proposed; PAY-001D, PAY-002A1, PAY-002B2C3C, PAY-003B | Add idempotent refund commands, remaining-balance/concurrency validation, stable Stripe keys, and event-finalized totals. | Same/conflicting/concurrent partial/full/timeout and repeated `partially_refunded` status with increasing confirmed total tests. | | PAY-005B | proposed; PAY-003B | Model per-dispute lifecycle for orders and registrations with finance alert references. | Duplicate/reordered warning lifecycle, inquiry escalation, created/updated/lost/won, rare lost-to-won late win, and `prevented` only under an explicitly validated upgraded webhook Event version. | | PAY-005C | proposed; A/B | Build read-only scheduled/operator reconciliation and mismatch/quarantine report. | Orphan/mismatch/refund/event/counter synthetic report. | diff --git a/OPERATIONS_RUNBOOK.md b/OPERATIONS_RUNBOOK.md index feba83e8..3972e1f0 100644 --- a/OPERATIONS_RUNBOOK.md +++ b/OPERATIONS_RUNBOOK.md @@ -473,6 +473,8 @@ Capture the commit, test IDs, timestamps, screenshots without secrets/PII, and t ## 12. Refund procedure +**Status: NOT AVAILABLE YET.** PAY-005A1 [#200](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/200) adds one source safeguard: an invalid partial amount stops instead of being omitted and becoming a full refund. It accepts only a positive whole number of cents below the stored original total, and only the explicit full-refund action may omit an amount. An unconfirmed provider result says do not retry and escalate because Stripe may already have accepted it. This source guard is not deployed or proven live and does not supply idempotency, remaining-balance/concurrency protection, verified final totals, finance capabilities, or approved policy. Do not use the steps below in production. + 1. Authenticate with finance capability and satisfy recent-auth/MFA policy. 2. Locate the local business record and corresponding Stripe PaymentIntent/Charge. 3. Confirm identity using the approved support procedure without asking for card data. diff --git a/SECURITY.md b/SECURITY.md index e6283461..87eaf155 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -95,6 +95,7 @@ These entries are implementation evidence, not a production risk-acceptance deci | Risks | Source result | Remaining closure evidence | | --- | --- | --- | +| Immediate input-containment part of RISK-020 | PAY-005A1 is tracked in live [#200](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/200). Both existing admin refund Functions share one non-coercing validator: a partial amount must be a primitive positive safe integer below the stored positive safe-integer original cents. Invalid caller or stored values stop before Stripe construction/refund creation and Firestore mutation; valid partials always include the exact `amount`; only the explicit full action omits it. Provider exceptions expose no detail and return a fixed result-not-confirmed, do-not-retry, and escalate message. Focused tests cover both race and merchandise paths, fixed errors, valid boundaries, provider failure, and malformed/equal/over/non-finite/coerced inputs. | This immediate guard does not prove the remaining refundable Stripe balance, cumulative prior refunds, command identity/idempotency, concurrency safety, pending/event-finalized state, capability/MFA/recent-auth, approved policy, Firebase deployment, Stripe configuration, or live behavior. An unconfirmed provider result remains ambiguous and requires reconciliation, not a blind retry. Complete PAY-005A, its PAY-002/PAY-003 dependencies, AUTH-003, and every listed residual gate before authorizing production refunds. | | Foundation only for parts of RISK-011, RISK-020, RISK-021 | PAY-002A1 is tracked in live [#161](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/161). The source-only child defines versioned pure payment, registration, fulfillment, confirmed-refund, and per-dispute structural reducers; fixed cross-dimension review/rejection; and a deterministic synthetic legacy classifier/aggregate report with no IDs. Orders/registrations carry no singular canonical dispute state. Forbidden regressions fail closed, refunds/disputes do not overwrite payment state, and `paid_after_cancellation` remains paid plus cancelled plus review rather than becoming a payment enum. | This does not yet mitigate those risks in a runtime. An unchanged enum does not prove duplicate Event/command/evidence, authorization, amount monotonicity, provider state, or club policy. No endpoint imports the library and no business record is read/written or migrated. PAY-002B1 [#163](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/163) supplies pure command/attempt keys; PAY-002B2A [#165](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/165) supplies only unused registration/audit persistence; PAY-002B2B [#169](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/169) supplies unused lease/fence/terminal-commitment source and tests without provider permission; PAY-002B2C1 [#173](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/173) supplies only unused lease-bound attempt-1 plan commitments/audit; PAY-002B2C2 [#182](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/182) supplies only unused pre-send evidence/audit and the exact persisted retry cutoff; PAY-002B2C3A [#184](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/184) supplies only an unused pure closed evidence classifier with no persistence or authorization; C3B/C3C must still persist verified proof and authorize/version any later attempt; PAY-002C/D persistence-first checkout; PAY-003B Event/version/evidence adoption including same-state amount changes and separate dispute records; PAY-004/PAY-005 cancellation/refund operations; MERCH-002/LEGAL-001 return/policy gates; #113 and a later migration child real inventory/dry-run. Firebase/Stripe deployment and live behavior remain unproven. | | Foundation only for retry/duplication parts of RISK-003 and RISK-020 | PAY-002B1 is tracked in live [#163](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/163). The pure source child defines a canonical UUID v4 boundary, a domain-separated caller/environment command hash, a command-type plus canonical frozen-payload fingerprint, and a deterministic environment/mode/operation/command/provider-attempt Stripe key. Invalid production/test or non-production/live pairs fail. Raw caller, UUID, operation, and payload values never appear in outputs. PAY-002B2C1 is tracked in live [#173](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/173) for an immutable lease-bound attempt-1 plan and deterministic audit whose account, canonical parameters, and B1 key are stored only as command-bound commitments. PAY-002B2C2 is tracked in live [#182](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/182) for a separate atomic pre-send marker/audit, a complete command-bound C1-plan digest in both partners, a post-transaction lease/deadline freshness check, and persisted 23-hour attempt-1 cutoff. PAY-002B2C3A is tracked in live [#184](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/184) for the pure exact attempt-1 reconciliation evidence policy. | Hashes and commitments are pseudonymous server-only equality evidence—not anonymization, authorization, configured-account proof, durable replay, or provider-execution proof. No endpoint imports these libraries. PAY-002B2A/#165 registers only a command/audit pair. PAY-002B2B/#169 adds a separate fixed-duration lease, holder fingerprint, monotonic fence, terminal commitment, and transition audit without provider permission. C1 does not permit a send. C2's fixed `send_permitted` is only a retry-safety classification under the exact lease/fence/plan; it is not caller authorization, configured-account proof, a Stripe call/outcome, or response replay. C2 makes no provider request and never advances attempt `1`. A changed/replaced plan fails its stored digest. Lease/deadline equality, later time, rollback, or missing/unreadable paired time returns `reconciliation_required`. C3A returns only fixed `existing_attempt_found`, `new_attempt_candidate`, or `reconciliation_required` results from closed enum evidence; a candidate is not send/advance permission. Timeout, connection loss, provider error, old/pruned key, missing/not-found object, partial/empty search, processing/unknown state, and conflict cannot become a candidate. C3B/C3C must persist verified evidence and separately authorize/version any later attempt. PAY-002C/D and PAY-005 must adopt the complete B2C chain with endpoint schemas and failure-boundary tests. Deployment, provider configuration, and live behavior remain unproven. | | Foundation only for registration/audit parts of RISK-003, RISK-020, and RISK-032 | PAY-002B2A is tracked in live [#165](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/165). The unused server-only transaction creates one immutable `checkoutRequests/{commandKeyHash}` registered record and deterministic revision-1 `auditEvents` partner atomically. Exact retries are read-only. Same-key type/schema/payload conflicts and corrupt, orphan, future, or unexpected stored pairs fail closed without repair. Environment/caller-scope changes derive different B1 keys; a stored mismatch under one key is corruption, not a cross-scope lookup. Fixed results expose no raw identity, UUID, payload, hash, path, or Firestore detail. CI-001B3 [#167](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/167) runs its dedicated exact demo-project/opt-in suite as a named hosted job and protected-release prerequisite. | Registration is neither authorization nor execution permission. #169 keeps the root and revision-1 audit immutable, places lifecycle state in the separate server-only subcollection, fails closed on malformed partners, and proves in synthetic tests that stale workers cannot finalize. The hosted emulator result is still synthetic source proof, not deployment or provider proof. No TTL is safe: [#110](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/110) must approve retention and a server-only tombstone or equivalent durable duplicate barrier before deletion. Source/tests/merge do not prove Firebase deployment, Stripe configuration, or live behavior. | diff --git a/docs/officers/EVENTS_SHOP_MEMBERS.md b/docs/officers/EVENTS_SHOP_MEMBERS.md index b2207d2c..e24aea02 100644 --- a/docs/officers/EVENTS_SHOP_MEMBERS.md +++ b/docs/officers/EVENTS_SHOP_MEMBERS.md @@ -241,6 +241,51 @@ Officer steps after every prerequisite has proof: **Escalation:** membership lead plus privacy/platform owner; use the private incident path under #112 if exposure is suspected. +## Partial-refund amount guard — SOURCE ONLY, NOT LIVE + +**Purpose:** make an invalid partial-refund amount stop. It must never silently become a full refund. + +**Approver:** treasurer plus platform/security owner. + +**Prerequisites:** the pull request for issue [#200](https://github.com/Run-MPRC/Run-MPRC.github.io/issues/200) is merged, a protected staging Firebase project, both exact refund Functions are deployed and read back there, Stripe test mode, made-up order and race records, and an approved refund policy. The broader safe refund procedure is still **NOT AVAILABLE YET**. + +```mermaid +flowchart TD + Ask["Request a partial refund"] --> Stored{"Stored original is valid whole cents?"} + Stored -- "No" --> Stop["Stop before Stripe and before a record change"] + Stored -- "Yes" --> Check{"Requested amount is positive whole cents and lower?"} + Check -- "No" --> Stop["Stop before Stripe and before a record change"] + Check -- "Yes" --> Partial["Send the exact partial amount"] + Partial --> Result{"Did Stripe return a refund record?"} + Result -- "No; outcome is unknown" --> Unknown["Keep local record unchanged\nDo not retry; escalate"] + Result -- "Yes" --> Returned["Use returned result\nLater reconciliation still required"] + Full["Explicit full-refund request"] --> Omit["Only path allowed to omit the amount"] + Omit --> Result +``` + +In words: a missing or malformed stored original, or an invalid, equal, or over-limit partial amount, stops before Stripe or a record change; only a deliberate full-refund request can omit the amount. If the server cannot confirm whether Stripe accepted the request, the local record stays unchanged, the officer must not retry, and later reconciliation is required. + +Officer review steps after every prerequisite has proof: + +1. Keep all live website refunds unavailable. +2. Ask the platform specialist to show the fixed synthetic test report for both race and shop refunds. +3. Confirm a missing, non-number, non-positive, fraction, non-finite, or too-large stored original stops before any Stripe refund call or record change. +4. Confirm the report rejects missing, non-number, fraction, zero, negative, equal, over-limit, and non-finite partial amounts. +5. Confirm every rejected caller case also shows no Stripe refund call and no order or registration change. +6. Confirm the smallest valid amount and one cent below the stored original send the exact test-mode amount and stay partial. +7. Confirm an unconfirmed Stripe test result leaves the made-up record unchanged and says: do not retry; escalate to the treasurer and platform owner. +8. Stop after this review. Do not approve a production refund until the remaining PAY-005 safety work and provider/deployment proof are complete. + +**Expected result:** a malformed stored original or rejected partial amount causes one fixed error and no provider or record change. An admitted partial request always carries its exact amount. Only the explicit full action can omit it. If the provider result cannot be confirmed, the page says not to retry and to escalate; it does not claim failure. + +**Stop conditions:** any real order, registration, member, card, Stripe payment record, refund, production Firebase project, Stripe live mode, missing deployment/readback, request to edit Firestore by hand, or retry after an unconfirmed result. + +**Success proof:** exact #200 pull request and merge commit; red proof showing the old unsafe cases; green focused and full tests; readback of both Functions in staging; made-up Stripe test-mode results; and a dated treasurer/platform review. A green source workflow alone is not deployment or provider proof. + +**Undo:** use one reviewed two-Function revert or safe roll-forward through the protected backend release. Never undo by issuing another refund or changing a payment record. + +**Escalation:** treasurer plus platform/security owner. Use the private incident path if any unexpected refund or real record was involved. + ## Admin screens — NOT AVAILABLE YET Admin event and product editors exist in source, but their live permissions, backup, preview, and rollback behavior have not been approved. Saving can write directly to production Firestore. Officers must not use these screens as a continuity procedure yet. diff --git a/functions/adminOrderAction.js b/functions/adminOrderAction.js index b68404f7..d63d7bbc 100644 --- a/functions/adminOrderAction.js +++ b/functions/adminOrderAction.js @@ -13,6 +13,10 @@ const { COMMERCE_OPERATIONS, requireCommerceAdmission, } = require('./commerceControl'); +const { + REFUND_VALIDATION_REASONS, + validatePartialRefundAmount, +} = require('./refundValidation'); const ACTIONS = new Set([ 'mark_fulfilled', @@ -127,20 +131,40 @@ exports.adminOrderAction = functions 'No Stripe payment intent on this order', ); } - const amountCents = action === 'refund_partial' ? Number(payload.amountCents) : null; - if (action === 'refund_partial' && (!amountCents || amountCents <= 0)) { - throw new functions.https.HttpsError( - 'invalid-argument', - 'amountCents required for partial refund', - ); + let amountCents = null; + if (action === 'refund_partial') { + const validation = validatePartialRefundAmount({ + amountCents: payload?.amountCents, + originalAmountCents: order.amountCents, + }); + if (!validation.ok) { + const invalidStoredTotal = ( + validation.reason === REFUND_VALIDATION_REASONS.INVALID_STORED_TOTAL + ); + throw new functions.https.HttpsError( + invalidStoredTotal ? 'failed-precondition' : 'invalid-argument', + invalidStoredTotal + ? 'Stored refund total is unavailable' + : 'Invalid partial refund amount', + ); + } + amountCents = validation.amountCents; } const stripe = getStripe(); const refundPayload = { payment_intent: order.stripePaymentIntentId }; - if (amountCents && amountCents < order.amountCents) { + if (action === 'refund_partial') { refundPayload.amount = amountCents; } - const stripeRefund = await stripe.refunds.create(refundPayload); - const isFull = !refundPayload.amount; + let stripeRefund; + try { + stripeRefund = await stripe.refunds.create(refundPayload); + } catch { + throw new functions.https.HttpsError( + 'internal', + 'Refund result could not be confirmed. Do not retry. Escalate to the treasurer and platform owner.', + ); + } + const isFull = action === 'refund_full'; await ref.update({ status: isFull ? 'refunded' : 'partially_refunded', refundedAt: Timestamp.now(), @@ -151,7 +175,7 @@ exports.adminOrderAction = functions actorUid: actor.uid, actorEmail: actor.email, action: isFull ? 'admin.refund_full' : 'admin.refund_partial', - note: `refund=${stripeRefund.id} amount=${refundPayload.amount || order.amountCents}`, + note: `refund=${stripeRefund.id} amount=${isFull ? order.amountCents : amountCents}`, }), ), }); diff --git a/functions/adminRegistrationAction.js b/functions/adminRegistrationAction.js index 3e1f7dfe..80df38e7 100644 --- a/functions/adminRegistrationAction.js +++ b/functions/adminRegistrationAction.js @@ -15,6 +15,10 @@ const { COMMERCE_OPERATIONS, requireCommerceAdmission, } = require('./commerceControl'); +const { + REFUND_VALIDATION_REASONS, + validatePartialRefundAmount, +} = require('./refundValidation'); const ACTIONS = new Set([ 'refund_full', @@ -33,21 +37,23 @@ function regRef(eventId, registrationId) { } async function refund({ - stripe, doc, reg, actor, amountCents, + stripe, doc, reg, actor, action, amountCents, }) { - if (!reg.stripePaymentIntentId) { - throw new functions.https.HttpsError( - 'failed-precondition', - 'No Stripe payment intent recorded for this registration', - ); - } const refundPayload = { payment_intent: reg.stripePaymentIntentId }; - if (amountCents && amountCents > 0 && amountCents < reg.amountCents) { + if (action === 'refund_partial') { refundPayload.amount = amountCents; } - const stripeRefund = await stripe.refunds.create(refundPayload); + let stripeRefund; + try { + stripeRefund = await stripe.refunds.create(refundPayload); + } catch { + throw new functions.https.HttpsError( + 'internal', + 'Refund result could not be confirmed. Do not retry. Escalate to the treasurer and platform owner.', + ); + } - const isFull = !refundPayload.amount; + const isFull = action === 'refund_full'; await doc.ref.update({ status: isFull ? 'refunded' : 'partially_refunded', refundedAt: Timestamp.now(), @@ -58,7 +64,7 @@ async function refund({ actorUid: actor.uid, actorEmail: actor.email, action: isFull ? 'admin.refund_full' : 'admin.refund_partial', - note: `refund=${stripeRefund.id} amount=${refundPayload.amount || reg.amountCents}`, + note: `refund=${stripeRefund.id} amount=${isFull ? reg.amountCents : amountCents}`, }), ), }); @@ -351,16 +357,34 @@ exports.adminRegistrationAction = functions } if (action === 'refund_full' || action === 'refund_partial') { - const stripe = getStripe(); - const amountCents = action === 'refund_partial' ? Number(payload.amountCents) : null; - if (action === 'refund_partial' && (!amountCents || amountCents <= 0)) { + if (!reg.stripePaymentIntentId) { throw new functions.https.HttpsError( - 'invalid-argument', - 'amountCents required for partial refund', + 'failed-precondition', + 'No Stripe payment intent recorded for this registration', ); } + let amountCents = null; + if (action === 'refund_partial') { + const validation = validatePartialRefundAmount({ + amountCents: payload?.amountCents, + originalAmountCents: reg.amountCents, + }); + if (!validation.ok) { + const invalidStoredTotal = ( + validation.reason === REFUND_VALIDATION_REASONS.INVALID_STORED_TOTAL + ); + throw new functions.https.HttpsError( + invalidStoredTotal ? 'failed-precondition' : 'invalid-argument', + invalidStoredTotal + ? 'Stored refund total is unavailable' + : 'Invalid partial refund amount', + ); + } + amountCents = validation.amountCents; + } + const stripe = getStripe(); return refund({ - stripe, doc: snap, reg, actor, amountCents, + stripe, doc: snap, reg, actor, action, amountCents, }); } diff --git a/functions/commerceControlEntryPoints.test.js b/functions/commerceControlEntryPoints.test.js index b043f493..0e3b1747 100644 --- a/functions/commerceControlEntryPoints.test.js +++ b/functions/commerceControlEntryPoints.test.js @@ -143,6 +143,64 @@ function registrationInput(amountCents) { }; } +const REFUND_ENTRY_POINTS = [ + { + name: 'registration', + handler: adminRegistrationAction, + data: { + eventId: 'race-1', + registrationId: 'reg-1', + }, + recordPath: 'events/race-1/registrations/reg-1', + }, + { + name: 'order', + handler: adminOrderAction, + data: { orderId: 'order-1' }, + recordPath: 'orders/order-1', + }, +]; + +const INVALID_PARTIAL_REFUND_AMOUNTS = [ + ['missing', undefined], + ['null', null], + ['zero', 0], + ['negative zero', -0], + ['negative', -1], + ['equal to original', 2500], + ['over original', 2501], + ['fraction', 1.5], + ['NaN', Number.NaN], + ['positive infinity', Number.POSITIVE_INFINITY], + ['negative infinity', Number.NEGATIVE_INFINITY], + ['numeric string', '1'], + ['infinity string', 'Infinity'], + ['boolean', true], + ['array', [1]], + ['object', { amount: 1 }], + ['unsafe integer', Number.MAX_SAFE_INTEGER + 1], +]; + +const INVALID_STORED_REFUND_TOTALS = [ + ['missing', undefined], + ['null', null], + ['zero', 0], + ['negative', -1], + ['string', '2500'], + ['fraction', 2500.5], + ['NaN', Number.NaN], + ['infinity', Number.POSITIVE_INFINITY], + ['unsafe integer', Number.MAX_SAFE_INTEGER + 1], +]; + +function partialRefundData(entry, amountCents) { + return { + ...entry.data, + action: 'refund_partial', + payload: { amountCents }, + }; +} + describe('commerce admission at admin entry points', () => { beforeEach(() => { admin.__clear(); @@ -184,6 +242,74 @@ describe('commerce admission at admin entry points', () => { delete process.env.ENFORCE_APP_CHECK; }); + // Keep these before every admitted provider path. stripeHelpers caches its client, + // so this ordering makes the constructor assertion prove validation runs first. + test.each(REFUND_ENTRY_POINTS.flatMap((entry) => ( + INVALID_PARTIAL_REFUND_AMOUNTS.map(([caseName, amountCents]) => ([ + entry.name, + caseName, + entry, + amountCents, + ])) + )))('%s rejects %s partial-refund input before side effects', async ( + _entryName, + _caseName, + entry, + amountCents, + ) => { + admin.__seed('systemConfig/commerce', control()); + admin.__seed(entry.recordPath, { + status: 'paid', + amountCents: 2500, + stripePaymentIntentId: 'pi_synthetic', + }); + + await expect(entry.handler( + partialRefundData(entry, amountCents), + adminContext(), + )).rejects.toMatchObject({ + code: 'invalid-argument', + message: 'Invalid partial refund amount', + }); + + expect(mockStripeConstructor).not.toHaveBeenCalled(); + expect(mockRefundCreate).not.toHaveBeenCalled(); + expect(mockBusinessWrite).not.toHaveBeenCalled(); + }); + + test.each(REFUND_ENTRY_POINTS.flatMap((entry) => ( + INVALID_STORED_REFUND_TOTALS.map(([caseName, originalAmountCents]) => ([ + entry.name, + caseName, + entry, + originalAmountCents, + ])) + )))('%s rejects %s stored refund total before side effects', async ( + _entryName, + _caseName, + entry, + originalAmountCents, + ) => { + admin.__seed('systemConfig/commerce', control()); + admin.__seed(entry.recordPath, { + status: 'paid', + amountCents: originalAmountCents, + stripePaymentIntentId: 'pi_synthetic', + }); + + await expect(entry.handler( + partialRefundData(entry, 1), + adminContext(), + )).rejects.toMatchObject({ + code: 'failed-precondition', + message: 'Stored refund total is unavailable', + }); + + expect(mockStripeConstructor).not.toHaveBeenCalled(); + expect(mockRefundCreate).not.toHaveBeenCalled(); + expect(mockBusinessWrite).not.toHaveBeenCalled(); + }); + test.each([ ['mark_comp', registrationInput(0)], ['add_late_registration', registrationInput(0)], @@ -252,10 +378,12 @@ describe('commerce admission at admin entry points', () => { eventId: 'race-1', registrationId: 'reg-1', action: 'refund_full', + payload: { amountCents: 1 }, }, 'events/race-1/registrations/reg-1'], ['order', adminOrderAction, { orderId: 'order-1', action: 'refund_full', + payload: { amountCents: 1 }, }, 'orders/order-1'], ])('blocks %s refund when incident refunds are off', async ( _name, @@ -284,10 +412,12 @@ describe('commerce admission at admin entry points', () => { eventId: 'race-1', registrationId: 'reg-1', action: 'refund_full', + payload: { amountCents: 1 }, }, 'events/race-1/registrations/reg-1'], ['order', adminOrderAction, { orderId: 'order-1', action: 'refund_full', + payload: { amountCents: 1 }, }, 'orders/order-1'], ])('admits %s refund independently while new commerce is off', async ( _name, @@ -316,9 +446,89 @@ describe('commerce admission at admin entry points', () => { expect(mockRefundCreate).toHaveBeenCalledWith({ payment_intent: 'pi_synthetic', }); + expect(mockRefundCreate).toHaveBeenCalledTimes(1); + expect(admin.__get(recordPath)).toMatchObject({ + status: 'refunded', + auditLog: [{ action: 'admin.refund_full' }], + }); + expect(mockBusinessWrite).toHaveBeenCalledTimes(1); + }); + + test.each(REFUND_ENTRY_POINTS.flatMap((entry) => ([1, 2499].map((amountCents) => ([ + entry.name, + amountCents, + entry, + ])))))('%s sends exact valid partial amount %i', async ( + _entryName, + amountCents, + entry, + ) => { + admin.__seed('systemConfig/commerce', control()); + admin.__seed(entry.recordPath, { + status: 'paid', + amountCents: 2500, + stripePaymentIntentId: 'pi_synthetic', + }); + + await expect(entry.handler( + partialRefundData(entry, amountCents), + adminContext(), + )).resolves.toEqual({ ok: true, refundId: 're_synthetic' }); + + expect(mockRefundCreate).toHaveBeenCalledWith({ + payment_intent: 'pi_synthetic', + amount: amountCents, + }); + expect(mockRefundCreate).toHaveBeenCalledTimes(1); + expect(admin.__get(entry.recordPath)).toMatchObject({ + status: 'partially_refunded', + auditLog: [{ action: 'admin.refund_partial' }], + }); expect(mockBusinessWrite).toHaveBeenCalledTimes(1); }); + test.each(REFUND_ENTRY_POINTS.map((entry) => [entry.name, entry]))( + '%s leaves business state unchanged when Stripe rejects a valid partial refund', + async (_entryName, entry) => { + admin.__seed('systemConfig/commerce', control()); + admin.__seed(entry.recordPath, { + status: 'paid', + amountCents: 2500, + stripePaymentIntentId: 'pi_synthetic', + }); + const hostileProviderDetail = [ + 'synthetic provider failure', + 'amount=7777', + 'payment=pi_must_not_escape', + ].join(' '); + mockRefundCreate.mockRejectedValueOnce(new Error(hostileProviderDetail)); + + const error = await entry.handler( + partialRefundData(entry, 1), + adminContext(), + ).catch((received) => received); + + expect(error).toMatchObject({ + code: 'internal', + message: 'Refund result could not be confirmed. Do not retry. Escalate to the treasurer and platform owner.', + }); + expect(error.message).not.toContain(hostileProviderDetail); + expect(error.message).not.toContain('pi_must_not_escape'); + expect(error.message).not.toContain('7777'); + + expect(mockRefundCreate).toHaveBeenCalledWith({ + payment_intent: 'pi_synthetic', + amount: 1, + }); + expect(mockRefundCreate).toHaveBeenCalledTimes(1); + expect(mockBusinessWrite).not.toHaveBeenCalled(); + expect(admin.__get(entry.recordPath)).toMatchObject({ + status: 'paid', + amountCents: 2500, + }); + }, + ); + test('existing-record note bypasses an absent runtime commerce control', async () => { process.env.COMMERCE_ENABLED = 'false'; admin.__seed('events/race-1/registrations/reg-1', { diff --git a/functions/refundValidation.js b/functions/refundValidation.js new file mode 100644 index 00000000..c41ebf27 --- /dev/null +++ b/functions/refundValidation.js @@ -0,0 +1,37 @@ +'use strict'; + +const REFUND_VALIDATION_REASONS = Object.freeze({ + INVALID_PARTIAL_AMOUNT: 'invalid_partial_amount', + INVALID_STORED_TOTAL: 'invalid_stored_total', +}); + +const INVALID_PARTIAL_AMOUNT = Object.freeze({ + ok: false, + reason: REFUND_VALIDATION_REASONS.INVALID_PARTIAL_AMOUNT, +}); + +const INVALID_STORED_TOTAL = Object.freeze({ + ok: false, + reason: REFUND_VALIDATION_REASONS.INVALID_STORED_TOTAL, +}); + +function validatePartialRefundAmount({ amountCents, originalAmountCents } = {}) { + if (!Number.isSafeInteger(originalAmountCents) || originalAmountCents <= 0) { + return INVALID_STORED_TOTAL; + } + + if ( + !Number.isSafeInteger(amountCents) + || amountCents <= 0 + || amountCents >= originalAmountCents + ) { + return INVALID_PARTIAL_AMOUNT; + } + + return Object.freeze({ ok: true, amountCents }); +} + +module.exports = { + REFUND_VALIDATION_REASONS, + validatePartialRefundAmount, +}; diff --git a/functions/refundValidation.test.js b/functions/refundValidation.test.js new file mode 100644 index 00000000..96ee0b51 --- /dev/null +++ b/functions/refundValidation.test.js @@ -0,0 +1,76 @@ +'use strict'; + +const { + REFUND_VALIDATION_REASONS, + validatePartialRefundAmount, +} = require('./refundValidation'); + +const INVALID_PARTIAL_AMOUNTS = [ + ['missing', undefined], + ['null', null], + ['zero', 0], + ['negative zero', -0], + ['negative', -1], + ['equal to original', 2500], + ['over original', 2501], + ['fraction', 1.5], + ['NaN', Number.NaN], + ['positive infinity', Number.POSITIVE_INFINITY], + ['negative infinity', Number.NEGATIVE_INFINITY], + ['numeric string', '1'], + ['infinity string', 'Infinity'], + ['boolean', true], + ['array', [1]], + ['object', { amount: 1 }], + ['unsafe integer', Number.MAX_SAFE_INTEGER + 1], +]; + +const INVALID_STORED_TOTALS = [ + ['missing', undefined], + ['null', null], + ['zero', 0], + ['negative', -1], + ['string', '2500'], + ['fraction', 2500.5], + ['NaN', Number.NaN], + ['positive infinity', Number.POSITIVE_INFINITY], + ['unsafe integer', Number.MAX_SAFE_INTEGER + 1], +]; + +describe('validatePartialRefundAmount', () => { + test.each(INVALID_PARTIAL_AMOUNTS)('rejects %s caller input without coercion', ( + _caseName, + amountCents, + ) => { + expect(validatePartialRefundAmount({ + amountCents, + originalAmountCents: 2500, + })).toEqual({ + ok: false, + reason: REFUND_VALIDATION_REASONS.INVALID_PARTIAL_AMOUNT, + }); + }); + + test.each(INVALID_STORED_TOTALS)('rejects a %s stored total', ( + _caseName, + originalAmountCents, + ) => { + expect(validatePartialRefundAmount({ + amountCents: 1, + originalAmountCents, + })).toEqual({ + ok: false, + reason: REFUND_VALIDATION_REASONS.INVALID_STORED_TOTAL, + }); + }); + + test.each([1, 2499])('returns exact valid boundary amount %i', (amountCents) => { + const result = validatePartialRefundAmount({ + amountCents, + originalAmountCents: 2500, + }); + + expect(result).toEqual({ ok: true, amountCents }); + expect(Object.isFrozen(result)).toBe(true); + }); +});