hackclub / hackclub/hcb

[Invoices] sync_remote! can overwrite the record of a completed payment

Open
#14,932 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Ruby
Stars
870
Forks
138
Avg merge
1d 14h
Merged PRs (30d)
208

Description

## Problem

`Invoice#set_fields_from_stripe_invoice` unconditionally copies `amount_paid`, `amount_due`, `amount_remaining`, `status`, `subtotal`, and `total` from the current remote Stripe invoice. There's no guard for invoices HCB has already transitioned to `paid_v2` and paid out.

So if a Stripe invoice changes state out of band after being paid (e.g. voided from the Stripe Dashboard), the next sync overwrites the local payment fields. The invoice stays `paid_v2` with a real `InvoicePayout`, but ends up recording `amount_paid = 0` and a remote status of `void`.

Example: invoice `4881`.

## Impact

No money is affected — payouts and fee reimbursements derive from the charge's balance transaction (`payout_creation_balance_net` / `payout_creation_balance_stripe_fee`), not from these columns. The damage is to the record: the invoice no longer reflects what was actually collected, and anything reading `amount_paid` / `amount_remaining` for it is wrong.

## Where it can fire unexpectedly

`invoices#hosted` and `invoices#pdf` both call `sync_remote!` (which ends in `save!`) on a GET. Viewing the hosted invoice or the PDF is enough to rewrite these columns.

## Possible directions

- Don't overwrite payment fields once an invoice is `paid_v2` and/or has a payout.
- Report the divergence instead of silently overwriting it.
- Reconsider writing to the database from those GET actions.

---
Filed by Claude at @garyhtou's request.

Contributor guide

Open the contributing guide

Research direction

Start with Invoice#set_fields_from_stripe_invoice and trace the sync_remote! calls from invoices#hosted and invoices#pdf, paying attention to their save! behavior on GET. Use invoice 4881 as the reported example and define done as preventing a paid_v2 invoice with an InvoicePayout from losing its recorded payment fields when the remote invoice changes state.

Written by the indexing model from the issue text.

Assessment

Tech stack
ruby
Domain
backend, payments
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.