Repository navigation
Silent save-failure after irreversible Execute can bypass MaxPaymentDailyUSD #1310
Description
Problem
In processAutoExchange (pkg/exchange/auto.go), after the irreversible AWS Execute call succeeds, the completed-record save error is only logged, never surfaced:
// pkg/exchange/auto.go:441-447
if err := params.Store.SaveRIExchangeRecord(ctx, record); err != nil {
logging.Errorf("failed to save completed exchange record for %s: %v", rec.SourceRIID, err)
}
outcome.RecordID = record.ID
outcome.ExchangeID = exchangeID
return outcomeThis is the same fail-open shape as COR-04 (PR #1229, just merged) but on a different code path:
- AWS exchange has been committed (money irreversibly spent)
- DB row is missing (status="completed" never persisted)
- Next iteration of the
for _, rec := range recsloop callsGetRIExchangeDailySpend, which sumsstatus = 'completed'rows -- and now under-reports total spend by exactly this exchange'spayment_due - The under-reported
dailySpentallows the cap checknewTotal.Cmp(dailyCap) > 0to pass on the next iteration when it should have rejected, blowing pastMaxPaymentDailyUSDby the missing amount
Likelihood is low (DB write failure on Postgres is rare on the same connection that just queried), but the impact is direct cap bypass on a money path. This is also the exact shape feedback_no_silent_fallbacks.md calls out.
Acceptance criteria
SaveRIExchangeRecord failure on the completed-record path must either:
- Block the loop from continuing (return an error from
RunAutoExchange, halting subsequent exchanges until the audit trail is reliable), OR - Be reflected in the outcome and the next iteration's cap check (e.g., maintain an in-process tally that the cap check adds on top of
GetRIExchangeDailySpend), so an in-memory sum carries forward when the DB row is missing.
Mirror the existing dailySpent / payment-due failure-closed branches' shape: logging.Errorf + outcome.Error set + return. The caller (RunAutoExchange) should stop processing further recommendations once any audit save has failed during an auto run; otherwise an operator has no way to know the cap budget left.
Regression test: drive processAutoExchange with a mock store whose SaveRIExchangeRecord fails only for completed-status records, run two back-to-back recommendations whose sum exceeds MaxPaymentDailyUSD, and assert the second exchange is rejected (cap check blocks) even though the first row never made it to the DB.
References
- PR fix(exchange): fail closed on unparseable PaymentDue in daily-cap check #1229 (COR-04) fixes the symmetrical fail-open for unparseable
PaymentDuein the same function - PR fix(purchase): surface save error on failed-status execution record #1237 (COR-10) fixes a similar AUDIT LOSS in
internal/purchase/execution.go(different code path, same anti-pattern) feedback_no_silent_fallbacks.md(CUDly memory garden)
Activity
cristim commentedon Jul 27, 2026
Verification sweep against
main(101f099fb) on 2026-07-27 finds this already resolved.saveLedgerRecord now retries and then halts the loop, so a persistent ledger-write failure can no longer bypass the daily cap.
Evidence: commit c848a07 (PR #1457).
Recommending close.
cristim commentedon Sep 2, 2026
Verified resolved at 3c0f8ac: after a successful Execute the completed-record save goes through saveLedgerRecord with retries, and a persistent failure sets outcome.Error and halts RunAutoExchange so no further exchange can be evaluated against an under-counted daily spend. Evidence: pkg/exchange/auto.go:453 (saveLedgerRecord), :588-592 (halt), :213-219 (loop break) (#1457 commit c848a07); regression test at pkg/exchange/auto_test.go:732-771. Residual axes checked: the manual approve path fails the request on ledger-write failure at internal/api/handler_ri_exchange.go:2473-2479; the pending and failed record saves at auto.go:390 and :629 happen with no money moved and do not feed the completed-status cap sum. Closing as completed; reopen if the behaviour recurs.