Repository navigation
fix(purchase): close two narrow double-buy windows on finalize and legacy retries #1232
Description
Two narrow double-buy risks on the purchase path, found in independent reviews of #1111 and #1110. Both predate those PRs.
1. Unguarded failed write in finalizePurchaseStatus (internal/api/handler_purchases.go, ~3101; disclosed in #1111's body). After the approval email is sent, finalizePurchaseStatus writes failed over the just-inserted pending row with no status guard. If a session approve lands while the email is being sent, the row can end up failed for a commitment that was actually bought. That row is then eligible for retry, which risks buying it again. Fix: guard the write on status = 'pending', like the CAS-guarded saves in #1111, and treat a lost guard as a no-op with a log line. Add a race test with approve landing during the send.
2. Retry of legacy EC2 executions without Details (#1110 review). recommendation_identity.go:30-33 reads an EC2 rec with empty Details as the known variant {az, "", ""} rather than unknown. The retry path (handler_purchases.go:2541) builds suppressions from persisted recs without re-matching them. A failed execution saved before Details existed (#453) would therefore leave the stored Linux/UNIX rec unsuppressed after a retry. Fix: treat empty EC2 Details as unknown, which means suppressing every variant, or re-match before building suppressions. Add a test with a legacy row.
Activity
IAmJSD commentedon Oct 22, 2026
Claiming: working on this
cristim commentedon Oct 25, 2026
Item 2 is addressed by #1411: an EC2 recommendation with empty Details now writes a wildcard suppression. The independent review of #1411 found two narrower legacy shapes that can still leave the bought variant unsuppressed after a retry; tracking them here:
shared, which never matches today'sdefault, so the suppression variant doesn't match the bought one. Fix: normalisesharedtodefaultwhen building or matching suppressions.Both err toward over-buying, so they are worth closing; a migration or matcher normalisation plus a test per shape.
fix(config): wildcard legacy compute-slug EC2 suppressions
fix(purchase): suppress every variant when a legacy EC2 recommendatio…
fix(api/purchases): guard the email-failure write in finalizePurchase…
cristim commentedon Oct 26, 2026
Taking the two legacy suppression shapes from my comment above (tenancy
sharedvsdefault, and the Details-only AZ for rows from 2026-10-09 to 2026-10-12) as a separate PR, following #1411. It will say "Part of #1232", so it won't close this issue.fix(config): match legacy EC2 tenancy and AZ shapes in purchase suppr…
refactor(config): tidy suppression tenancy and AZ helpers
fix(config): scope suppression tenancy matching to AWS EC2 (#1426)
fix(api/purchases): return a 500 when the approval-email failure cann…
IAmJSD commentedon Oct 26, 2026
Closing as fixed on main. Item 1 (guarded finalizePurchaseStatus write plus race tests) was fixed by #1237. Item 2 (legacy EC2 with empty Details) was fixed by #1411. The two legacy shapes cristim added in comments (shared/default tenancy, Details-only AZ) were fixed by #1424 and #1426. All are merged.
The fix PRs said Refs, so GitHub did not close this automatically. Checked against main on 2026-10-26 by IAmJSD's agent (IAmJSD is acting owner while the maintainers are away). Reopen if anything here is still wrong.