From 3936b035550c1f51d79441f9f5fecf74f8be6266 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 29 Sep 2026 00:00:26 +0200 Subject: [PATCH] fix(api): refuse marketplace pricing for an RI with no purchase timestamp computeRemainingMonths returned 1 for a zero timestamp, so a row without a timestamp was listed at 1/termMonths of its upfront cost. It now returns an error for a zero timestamp or non-positive term; the separate Term <= 0 check in marketplaceList folds into it. --- internal/api/handler_marketplace.go | 34 +++++++------- internal/api/handler_marketplace_test.go | 58 +++++++++++++++++++----- 2 files changed, 63 insertions(+), 29 deletions(-) diff --git a/internal/api/handler_marketplace.go b/internal/api/handler_marketplace.go index 3a45025af..9fa73b0c0 100644 --- a/internal/api/handler_marketplace.go +++ b/internal/api/handler_marketplace.go @@ -144,17 +144,13 @@ func (h *Handler) marketplaceList(ctx context.Context, req *events.LambdaFunctio // purchase_history.term is stored in years (1 or 3); both // computeRemainingMonths and resolveMarketplacePriceSchedule operate in - // months. Reject a zero or negative term rather than silently computing - // garbage on this money path (no-silent-fallbacks policy). - if row.Term <= 0 { - return nil, fmt.Errorf("purchase has invalid term %d (expected 1 or 3 years); cannot compute marketplace pricing", row.Term) - } + // months. computeRemainingMonths rejects a non-positive term and a missing + // timestamp rather than pricing garbage on this money path. termMonths := row.Term * 12 - - // Compute actual remaining months from the purchase timestamp and total - // term so the default price schedule reflects real remaining value - // rather than the full contract term (which overprices older RIs). - remainingMonths := computeRemainingMonths(row.Timestamp, termMonths) + remainingMonths, err := computeRemainingMonths(row.Timestamp, termMonths) + if err != nil { + return nil, err + } // Validate and normalise the price schedule. Row-total UpfrontCost and the // instance Count are passed so pricing is computed per instance; recurring @@ -442,20 +438,24 @@ func (h *Handler) authorizeAllowedAccount(ctx context.Context, session *Session, } // computeRemainingMonths returns the number of whole months remaining on an RI -// given its purchase timestamp and total term in months. The result is floored -// at 1 so defensive callers always get a positive value. -func computeRemainingMonths(purchaseTime time.Time, termMonths int) int { - if purchaseTime.IsZero() || termMonths <= 0 { - return 1 +// given its purchase timestamp and total term in months. A zero timestamp is +// absent data and returns an error: pricing it as 1 remaining month would list +// the RI for a fraction of its residual value. The result is floored at 1. +func computeRemainingMonths(purchaseTime time.Time, termMonths int) (int, error) { + if purchaseTime.IsZero() { + return 0, fmt.Errorf("purchase has no timestamp; cannot compute remaining term for marketplace pricing") + } + if termMonths <= 0 { + return 0, fmt.Errorf("invalid term of %d months; cannot compute remaining term for marketplace pricing", termMonths) } elapsed := time.Since(purchaseTime) elapsedMonths := elapsed.Hours() / (24 * 30.4375) remaining := float64(termMonths) - elapsedMonths r := int(math.Floor(remaining)) if r < 1 { - return 1 + return 1, nil } - return r + return r, nil } // awsMarketplaceFeePercent is the AWS Marketplace transaction fee percentage diff --git a/internal/api/handler_marketplace_test.go b/internal/api/handler_marketplace_test.go index 34043bfe1..bdc89e7b5 100644 --- a/internal/api/handler_marketplace_test.go +++ b/internal/api/handler_marketplace_test.go @@ -636,29 +636,40 @@ func TestMarketplaceList_EmptyOfferingClassFetchedConvertible(t *testing.T) { // TestComputeRemainingMonths exercises the helper directly, confirming it treats // its termMonths parameter as months (not years). func TestComputeRemainingMonths(t *testing.T) { - // Zero purchase time -> defensive floor. - assert.Equal(t, 1, computeRemainingMonths(time.Time{}, 36), "zero time should return 1") + remaining := func(purchase time.Time, termMonths int) int { + t.Helper() + r, err := computeRemainingMonths(purchase, termMonths) + require.NoError(t, err) + return r + } - // Non-positive term -> defensive floor. - assert.Equal(t, 1, computeRemainingMonths(time.Now(), 0), "zero term should return 1") - assert.Equal(t, 1, computeRemainingMonths(time.Now(), -1), "negative term should return 1") + // Absent timestamp or non-positive term -> explicit error (issue #98). + for _, tc := range []struct { + name string + purchase time.Time + termMonths int + }{ + {"zero time", time.Time{}, 36}, + {"zero term", time.Now(), 0}, + {"negative term", time.Now(), -1}, + } { + _, err := computeRemainingMonths(tc.purchase, tc.termMonths) + assert.Error(t, err, tc.name) + } // Fresh purchase (elapsed ~ 0): 36-month term should return ~36. - r := computeRemainingMonths(time.Now(), 36) - assert.InDelta(t, 36, r, 1, "fresh 36-month RI should have ~36 months remaining") + assert.InDelta(t, 36, remaining(time.Now(), 36), 1, "fresh 36-month RI should have ~36 months remaining") // 1-year RI: fresh purchase, 12-month term should return ~12. - r = computeRemainingMonths(time.Now(), 12) - assert.InDelta(t, 12, r, 1, "fresh 12-month RI should have ~12 months remaining") + assert.InDelta(t, 12, remaining(time.Now(), 12), 1, "fresh 12-month RI should have ~12 months remaining") // 6 months elapsed on a 36-month term -> ~30 remaining. sixMonthsAgo := time.Now().Add(-6 * 30 * 24 * time.Hour) - r = computeRemainingMonths(sixMonthsAgo, 36) - assert.InDelta(t, 30, r, 2, "36-month RI bought 6 months ago should have ~30 months remaining") + assert.InDelta(t, 30, remaining(sixMonthsAgo, 36), 2, "36-month RI bought 6 months ago should have ~30 months remaining") // Fully elapsed term -> floor at 1, never zero or negative. old := time.Now().Add(-40 * 30 * 24 * time.Hour) - assert.Equal(t, 1, computeRemainingMonths(old, 36), "expired RI should floor to 1") + assert.Equal(t, 1, remaining(old, 36), "expired RI should floor to 1") } // TestMarketplaceList_TermYearsConvertedToMonths is the end-to-end regression @@ -770,3 +781,26 @@ func TestMarketplaceList_InvalidTermReturnsError(t *testing.T) { assert.Contains(t, err.Error(), "invalid term") cfgStore.AssertExpectations(t) } + +// TestMarketplaceList_ZeroTimestampReturnsError: a purchase_history row with no +// timestamp must be refused, not priced as if 1 month of the term remained +// (issue #98: a $36k 3-year RI would otherwise be listed at ~$950). +func TestMarketplaceList_ZeroTimestampReturnsError(t *testing.T) { + cfgStore := &MockConfigStore{} + authSvc := &MockAuthService{} + adminSession(authSvc) + + row := standardRow() + row.Timestamp = time.Time{} + cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID). + Return(row, nil) + + ec2 := &stubMarketplaceEC2{} + h := newMarketplaceHandler(cfgStore, authSvc, ec2) + _, err := h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID) + + require.Error(t, err) + assert.Contains(t, err.Error(), "no timestamp") + assert.Zero(t, ec2.createCallCount, "no listing may be created for an unpriceable RI") + cfgStore.AssertExpectations(t) +}