The Apple and Play purchase path, whole.
| Pull request | #17 |
| Compared from | review-base-2026-09-04-payments — 14027d128b830c18ae7211b94b2c11b4caa44522 |
| Compared to | review/salus-payment-functions — dcc82a12bec24961aba08412806255e8d43ce187 |
| Commit range | main at 6876fa554a3ff296699b9386a771b0e39d7e2f29 |
| Ran | 04:24 to 04:55 BST, 4 September 2026 |
| Allowance | free review 2 of 3 |
| Reviewer's own verify pass | 20 found, 25 verified, 3 refuted, deduplicated to 10 |
| Diff | 12 files, 3,509 lines added |
The pull request is REVIEW ONLY and is never merged (operational ledger 557). The fixes
below are committed to main instead.
The Apple and Play purchase path holds its three laws, and every finding is in the edges around them: one write that can overwrite a paying non-Apple subscriber, one refund that can outlive itself, one store notification that a transient lookup failure loses for ever, and seven smaller faults of contract, rate and noise.
Ordered by what can lose money or entitlement, not by the order the reviewer wrote them.
severity is the reviewer's own word mapped to the board's vocabulary: normal becomes
medium, nit becomes low. Nothing was upgraded or downgraded on the way through.
| # | File | Line | Severity | What is wrong |
|---|---|---|---|---|
| 1 | _shared/apple-grant.ts |
233-246 | medium | applyAppleGrant's replace action overwrites a live non-Apple subscription row (Stripe, PayPal, lifetime or Play) for the same user. stripe-webhook has the symmetric guard at its own line 534: it alarms and skips. Apple has none, so a Stripe subscriber whose Apple purchase lands keeps being billed at Stripe with no local record of it. |
| 4 | apple-server-notifications/index.ts |
222-245 | medium | The owner-conflict pre-check compares the row's user_id against Apple's token. An orphan-rebound row deliberately carries no token and is owned by the caller, so every later notification for it looks like a conflict and is answered 200 skipped. Refunds, revocations and expirations for those rows are dropped, and entitlement outlives the refund. |
| 5 | play-rtdn-handler/index.ts 416, apple-server-notifications/index.ts 208 and 262 |
416 | medium | A transient getUserById failure is coerced to "this owner does not exist" and answered 200. The store takes 200 as delivered and never retries, so the notification is lost for ever. Play checks no status code at all; Apple checks one but then treats the resulting null as absent. |
| 3 | apple-iap-verify/index.ts |
407-499 | medium | When applyAppleGrant ignores a stale INITIAL_VERIFY, nothing re-reads the row, so the answer is built from the receipt: ok:true, status:active while the row already says expired because a refund landed first. play-iap-verify does re-read, through verifyAnswer. |
| 2 | apple-iap-verify/index.ts |
327-344 | medium | attribution.refusal is only honoured when there is no owner. When Apple names a different live user the grant is written to that user and the caller is still told ok:true, against the documented contract that a refusal is answered "even if a grant was written". |
| 6 | claim-entitlement/index.ts |
189-217 | medium | The OTP request path caps concurrent pending rows and nothing else. One signed-in caller can re-post action=request in a loop and send unlimited one-time-code emails to any address. |
| 7 | apple-server-notifications/index.ts |
133-146 | medium | captureException fires on every Apple notification while REVENUECAT_APPLE_NOTIFICATION_URL is unset, so a forward nobody has configured yet reads as a Sentry alert storm. |
| 8 | apple-server-notifications/revenuecat-forward.ts |
107-114 | low | A RevenueCat 429 is treated as a permanent 4xx and dropped after one attempt. A throttle is the exact case a retry loop exists for. |
| 9 | play-rtdn-handler/index.ts |
62-89 | low | The SANDBOX_TESTER_EMAILS and SANDBOX_OPEN pinning block is triplicated across three handlers, with two personal addresses hard-coded as the default. |
| 10 | apple-iap-verify/index.ts |
145-168 | low | ALLOWED_ORIGINS and cors() are copy-pasted verbatim across handlers instead of living in _shared. |
2026-09-04-pr17.json beside this file is the machine record, in the shape
README.md declares, and it carries the reviewer's untouched output as raw_findings.
Each finding's outcome and closed_by say what happened to it.
See salus-brain/reports/council_payment-path-review-fixes_2026-09-04-*.md for the
Council's verdict on the fixes, and the outcome field in the JSON record for the
per-finding disposition.
Fixed at the cause, each with a failing test written first: findings 1, 2, 3, 4, 5, 6, 7
and 8. Deferred with a reason: findings 9 and 10 (see the outcome field in the JSON).
Fix commit bd85fef59; record commit 93debd5d5.
Proof it can fail. supabase/functions/_shared/review-pr17-redproof.sh plants 38
faults, one at a time, and requires the suite to go red for each. 38 killed, 0 survived,
against a healthy control that must be green before any mutation counts. The whole Deno
suite is 385 green. Two of the fixes were unguarded on the first run of that harness and
four mutations were measuring nothing; both were repaired before this was called done.
Council. Three rounds. BLOCK (04:24), BLOCK (07:15), WARN (07:45) — one WARN
and three PROCEED, with DeepSeek failing to sit in either round, so the panel was four
seats and not five. Every blocking finding was answered in the code before the re-run.
First-run proof on the live path. The four changed functions were deployed at
04:43:09Z on 4 September 2026 with supabase functions deploy <fn> --project-ref egywowuyixfqytaucihf, taking verify_jwt from supabase/config.toml rather than a flag
typed by hand. Probed at 04:43:23Z: apple-iap-verify and claim-entitlement answer
204 to a CORS preflight; apple-server-notifications and play-rtdn-handler answer
405 to both OPTIONS and GET, which is their own method guard. The deployed
apple-iap-verify bundle, read back through the Supabase API, carries
callerMustBeRefused, verifyAnswer, provider_collision, RIVAL_BILLING_STATUSES and
rivalIsApple. public.money_alarm at 04:44Z holds nothing but the two
money-alarm-probe heartbeats — the deploy raised no alarm.
The backup, named honestly. supabase functions download is refused by CLI 2.75, so
the recoverable backup at ~/Salus-Vault/MASTERdrive/Tools/deploy-backups/2026-09-04-pr17
is the last committed pre-fix source (57b4be1e6), not a read-back of the bundle that was
actually running.