Repository navigation
fix(azure): a reservation with nil purchaseDate bypasses the duplicate-purchase guard #454
Description
Problem
providers/azure/services/internal/reservations/inventory.go (~:266, as of #453) fills StartDate from PurchaseDate. When Azure returns a reservation with a nil PurchaseDate, StartDate stays zero and StartDateIsDay stays false. The duplicate guard (pkg/recfilter/dedupe.go:116-119) then treats the reservation as ancient, so it does not suppress a re-purchase.
This fails open on a money path. Azure normally always returns purchaseDate, so the risk is low, but the guard should not depend on that.
Expected
A reservation with no purchase date is treated as possibly recent, so the guard suppresses a duplicate. The alternative is an error that stops the purchase, never a silent skip.
Evidence
Found during the independent review of #453 (a non-blocking note, and not introduced by #453).
Fix sketch
When PurchaseDate is nil, either set StartDate to now with StartDateIsDay set (counts as recent), or return an error from populateCommitmentFields. Add a regression test for a nil PurchaseDate that must suppress the duplicate.
Activity
cristim commentedon Oct 16, 2026
Claiming: working on this.
cristim commentedon Oct 16, 2026
Checked against main: the production path already fails loud.
CommitmentFromReservationisStrictCommitment(internal/reservations/strict.go:30), used by computeGetExistingCommitments(compute/client.go:282).strict.go:79-81addspurchaseDateto the missing-field list and returns an error for a live row, so inventory fails andAdjustRecommendationsForExistingreturns that error instead of buying.compute/inventory_completeness_sdk_test.go:60already decodes SDK JSON withoutpurchaseDateand asserts that error (fix(azure/compute): fail closed on incomplete live VM reservations #317, refactor(azure): move strict reservation inventory checks to the shared reader #328).pkg/recfilter/dedupe.gobeforeStartDateis read, so a zero start is harmless there.What remains is that the exported, lenient
CommitmentFromReservation(inventory.go:266-270) still returns a live commitment with a zero start if a future caller uses it directly. Proposed rephrase: "makeCommitmentFromReservationitself reject a live row withoutpurchaseDate, and drop the duplicate check instrict.go" (provider-only, about 100 lines), or close as fixed by #317/#328. Owner's call; markingstatus/needs-infoand releasing my claim meanwhile.The same fail-open does exist for Azure savings plans: filed as #481 (the AWS savings-plan variant is in #53).
cristim commentedon Oct 20, 2026
Verified on current main. The only production caller of CommitmentFromReservation is StrictCommitment (providers/azure/services/internal/reservations/strict.go:32, used by compute GetExistingCommitments at compute/client.go:299), and validateLiveReservationFields errors on a nil purchaseDate for a live row (strict.go:91-93) so inventory fails and the purchase is stopped; the SDK-JSON test 'purchaseDate nil' pins it (compute/inventory_completeness_sdk_test.go:61, #317/#328). The exported lenient CommitmentFromReservation (inventory.go:232) still leaves a zero StartDate if a future caller bypasses StrictCommitment; that is a hardening option, not the reported bug. [claude-cc-libs]