golem15/summercms
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(18): record code review disposition after gap closure 18-08

Browse Source
This commit is contained in:
Jakub Zych
2026-10-14 00:34:32 +02:00
parent 195882c39d
commit e691b8300b
1 changed files with 14 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
.planning/phases/18-payment-gateway/18-REVIEW-DISPOSITION.md
View File

@@ -2,10 +2,11 @@
phase: 18
review: 18-REVIEW.md
findings:
open: 48
total: 48
open: 54
fixed: 1
total: 55
unparsed: 0
recorded: 2026-10-13T16:01:01.576Z
recorded: 2026-10-13T22:34:31Z
---
# Phase 18: Code Review Disposition
@@ -14,8 +15,8 @@ Written by hand: the review uses part-prefixed IDs (A- for framework, pgstripe a
| Finding | Severity | Disposition | Source |
|---------|----------|-------------|--------|
| A-CR-01 | critical | open | `payment_intent.payment_failed` rejects a payment the customer can still complete, so a paid order stays Rejected |
| A-CR-02 | critical | open | A "paid" signal for a Rejected or Canceled payment is dropped silently (no log, no refund, Checkout Session left open after a cancel) |
| A-CR-01 | critical | fixed | `payment_intent.payment_failed` rejects a payment the customer can still complete, so a paid order stays Rejected. Fixed by plan 18-08, sm-pgstripe-plugin `0c4fde7` |
| A-CR-02 | critical | open | A "paid" signal for a Rejected or Canceled payment is dropped silently (no log, no refund, Checkout Session left open after a cancel). Partly resolved by plan 18-08 (`f5c10e8`: the refused paid signal is now an Error log line); still open: the Checkout Session is not expired on cancel, and there is no event, notice or refund |
| A-CR-03 | critical | open | The admin money input reads a comma as the decimal separator in every locale, so `1,234` in `en` is saved as `1.234` |
| A-WR-01 | warning | open | Webhook verification, refunds and Stripe lookups always use the currently active mode's keys; the payment does not record its own mode |
| A-WR-02 | warning | open | Accepting an event does not check the event's amount, currency or session/intent id against the payment |
@@ -27,6 +28,7 @@ Written by hand: the review uses part-prefixed IDs (A- for framework, pgstripe a
| A-WR-08 | warning | open | A money field without `max` has no upper bound, so overflow is an opaque 500 instead of a 422 |
| A-WR-09 | warning | open | The layer navigates to any `redirect_url` without checking the scheme |
| A-WR-10 | warning | open | The layer's `formatMoney` throws on an unknown currency code and breaks the whole render |
| A-WR-11 | warning | open | A payment whose asynchronous Checkout payment is still processing is canceled by the sweep, and the later `async_payment_succeeded` is refused (18-08 re-review) |
| A-IN-01 | info | open | An authentic event for a payment that is not yet visible is answered 200 "ignored", so Stripe never retries it |
| A-IN-02 | info | open | `checkout.session.expired` is not handled |
| A-IN-03 | info | open | Method-backed dropdowns dispatch on the field name and ignore the declared method; submitted values are not checked against the options |
@@ -35,6 +37,13 @@ Written by hand: the review uses part-prefixed IDs (A- for framework, pgstripe a
| A-IN-06 | info | open | Enabling the operator without a secret key turns every checkout into a 500 |
| A-IN-07 | info | open | `readWinterFields` reports every read error as "File not found" |
| A-IN-08 | info | open | `isZeroMoney` treats `""`, `"-"` and `"."` as zero |
| A-IN-09 | info | open | The decline Warn line says "the payment stays open for a retry" whatever the payment's state (18-08 re-review) |
| A-IN-10 | info | open | The refused-accept Error line is written per signal, and the success URL can repeat it (18-08 re-review) |
| A-IN-11 | info | open | The decline text is still logged on the purchase-time decline path (18-08 re-review) |
| A-IN-12 | info | open | `PaymentError.Type` is decoded and exported but nothing reads it (18-08 re-review) |
| A-IN-13 | info | open | `completedEvent` names the first payment's session and intent, and `paid_after_final_state` uses it for the third purchase (18-08 re-review) |
| A-IN-14 | info | open | Test-quality notes on the 18-08 tests (18-08 re-review) |
| A-IN-15 | info | open | The README and the settings page list different webhook events (18-08 re-review) |
| B-CR-01 | critical | open | Guest access to an order is forgeable. `guest_code` is a deterministic hashids of the order id, and legacy rows used the public salt `KEIOS.PAYMENTGATEWAY` |
| B-CR-02 | critical | open | The PaymentAccepted bridge writes `successful_payment_id` before checking `Can(Pay)`, overwriting the real successful payment on Paid, Canceled or Finished orders (PHP did not persist it) |
| B-CR-03 | critical | open | `default_currency` can be changed at any time, which silently re-bases every stored exchange ratio and native amount |