Skip to content

fix: deduplicate Stripe webhook credits via ledger reference index #124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
antojoseph wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: deduplicate Stripe webhook credits via ledger reference index #124

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
51e28db
security: idempotent ledger credits prevent Stripe double-billing
antojoseph Apr 30, 2026
e9e396c
fix: scope ledger reference uniqueness to account_id and abort on race
antojoseph May 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion coordinator/internal/store/postgres.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ func (s *PostgresStore) migrate(ctx context.Context) error {
created_at TIMESTAMPTZ NOT NULL DEFAULT NOW()
)`,
`CREATE INDEX IF NOT EXISTS idx_ledger_account ON ledger_entries(account_id, created_at DESC)`,
`CREATE UNIQUE INDEX IF NOT EXISTS idx_ledger_reference ON ledger_entries(entry_type, reference) WHERE reference <> ''`,
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Scope reference uniqueness to avoid dropping valid credits

The new unique index on (entry_type, reference) is global, so any repeated non-empty reference for the same entry type will suppress later credits across all accounts. This now conflicts with existing callers that intentionally reuse references (for example LedgerRefund with "reservation_refund" in coordinator/internal/api/consumer.go and default "admin_credit" in coordinator/internal/api/billing_handlers.go), causing legitimate credits to be silently skipped after the first one.

Useful? React with 👍 / 👎.


Copy link
Copy Markdown
Contributor

@hankbobtheresearchoor hankbobtheresearchoor May 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 This startup migration will fail if production already has duplicate non-empty (account_id, entry_type, reference) rows. Given existing reused references like reservation_refund / default admin_credit, that seems plausible. Add a preflight/dedupe migration or narrow the unique index predicate to the Stripe deposit reference shape so deploys do not fail closed on existing ledger history.

// Referral system tables
`CREATE TABLE IF NOT EXISTS referrers (
Expand Down Expand Up @@ -838,6 +839,24 @@ func nullableCreatedAt(ts time.Time) any {
}

func creditTx(ctx context.Context, tx pgx.Tx, accountID string, amountMicroUSD int64, entryType LedgerEntryType, reference string, createdAt time.Time) error {
// Idempotency guard: if a non-empty reference has already been recorded for
// this entry_type, skip the credit entirely. Prevents double-crediting on
// duplicate Stripe webhook deliveries. The unique index on
// (entry_type, reference) enforces this at the DB level even under races.
if reference != "" {
var exists bool
err := tx.QueryRow(ctx,
`SELECT EXISTS(SELECT 1 FROM ledger_entries WHERE entry_type = $1 AND reference = $2)`,
string(entryType), reference,
).Scan(&exists)
if err != nil {
return fmt.Errorf("store: check ledger reference: %w", err)
}
if exists {
return nil
}
}

_, err := tx.Exec(ctx,
`INSERT INTO balances (account_id, balance_micro_usd, updated_at)
VALUES ($1, $2, NOW())
Expand All @@ -860,7 +879,8 @@ func creditTx(ctx context.Context, tx pgx.Tx, accountID string, amountMicroUSD i

_, err = tx.Exec(ctx,
`INSERT INTO ledger_entries (account_id, entry_type, amount_micro_usd, balance_after, reference, created_at)
VALUES ($1, $2, $3, $4, $5, COALESCE($6, NOW()))`,
VALUES ($1, $2, $3, $4, $5, COALESCE($6, NOW()))
ON CONFLICT (entry_type, reference) WHERE reference <> '' DO NOTHING`,
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Abort transaction when dedupe conflict drops ledger row

This ON CONFLICT ... DO NOTHING runs after the balance increment, and the result is not checked. Under concurrent duplicate deliveries with the same reference, both transactions can pass the pre-check and credit balances, but one ledger insert will be skipped here and still return success, committing an extra balance increase with no corresponding ledger entry.

Useful? React with 👍 / 👎.

accountID, string(entryType), amountMicroUSD, balanceAfter, reference, nullableCreatedAt(createdAt),
)
if err != nil {
Expand Down
Loading