diff --git a/internal/api/handler_per_account_perms_test.go b/internal/api/handler_per_account_perms_test.go index e016f58a4..ded493a06 100644 --- a/internal/api/handler_per_account_perms_test.go +++ b/internal/api/handler_per_account_perms_test.go @@ -711,6 +711,11 @@ func TestPerAccountPerms_ExecutePurchase_AllowedAccountAccepted(t *testing.T) { mockStore.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) { return permsAccountList(), nil } + // #1905: the rec is now priced from the stored recommendation set. + accA := permsAccA + expectStoredRecs(mockStore, config.RecommendationRecord{ + Provider: "aws", Service: "ec2", CloudAccountID: &accA, Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: 100, Savings: 10, + }) handler := &Handler{ auth: scopedAuthMock(ctx), diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go index 26a25deb2..9d0adf1c6 100644 --- a/internal/api/handler_purchases.go +++ b/internal/api/handler_purchases.go @@ -1626,7 +1626,10 @@ func (h *Handler) retryPurchase(ctx context.Context, req *events.LambdaFunctionU // relaunch another user's over-cap failed purchase that the retrying // session's OWN constraints would deny, and constraints tightened after // the original submission would never apply on replay (adversarial - // review follow-up to #1210). + // review follow-up to #1210). failedExec.Recommendations carry the + // store-derived prices stamped at submit time + // (priceRecommendationsFromStore), so this re-check never sees + // client-supplied numbers. if constraintErr := h.enforcePurchaseConstraints(ctx, session, failedExec.Recommendations); constraintErr != nil { return nil, constraintErr } @@ -2100,6 +2103,11 @@ func (h *Handler) getPurchaseDetails(ctx context.Context, req *events.LambdaFunc // ExecutePurchaseRequest represents the request to execute purchases. type ExecutePurchaseRequest struct { + // Recommendations are matched against the stored recommendation set on + // (provider, cloud_account_id, service, region, resource_type, engine, + // term, payment). Only count, recommended_count and selected are taken + // from the request; id, details and every cost field are replaced from + // the stored row scaled by count (priceRecommendationsFromStore, #1905). Recommendations []config.RecommendationRecord `json:"recommendations"` // CapacityPercent is what fraction (1..100) of the originally- // recommended counts the user chose in the bulk Purchase flow. @@ -2169,16 +2177,17 @@ func (h *Handler) validateExecutePurchaseRequest(ctx context.Context, req *event if err := validateCapacityConsistency(execReq.Recommendations, execReq.CapacityPercent); err != nil { return ExecutePurchaseRequest{}, nil, nil, err } - // Enforce the per-permission Constraints (MaxPurchaseAmount, Providers, - // Services, Regions, AccountIDs) configured on the granting - // execute:purchases permission (SEC-01, issue #1141). Runs after the - // per-rec validation above so provider tokens are already normalized. - // Each recommendation must individually be granted by a permission; the - // amount cap is checked against the batch's total commitment (upfront - // plus recurring, see recTotalCommitment) so it cannot be evaded by - // splitting a large purchase across recs or by a no-upfront commitment - // whose real cost is entirely recurring. - if err := h.enforcePurchaseConstraints(ctx, session, execReq.Recommendations); err != nil { + // Price every rec from the stored recommendation set, then enforce the + // per-permission Constraints (MaxPurchaseAmount, Providers, Services, + // Regions, AccountIDs) on the granting execute:purchases permission + // (SEC-01, issue #1141) against those store-derived values. Runs after + // the per-rec validation above so provider and payment tokens are + // canonical before the identity match, and after the scope check so an + // out-of-scope account is refused as 403 before any pricing lookup. The + // client's own upfront_cost / monthly_cost / savings / details are never + // used (issue #1905): a purchaser under a MaxPurchaseAmount cap could + // otherwise state any number and buy at list price. + if err := h.priceAndEnforcePurchaseConstraints(ctx, session, execReq.Recommendations); err != nil { return ExecutePurchaseRequest{}, nil, nil, err } return execReq, session, adjustments, nil @@ -2191,7 +2200,10 @@ func (h *Handler) validateExecutePurchaseRequest(ctx context.Context, req *event // Shared by the direct-execute request validation and the retry path // (retryPurchase) so both re-derive and check the exact same Constraints // (SEC-01, issue #1141; adversarial review follow-up to #1210 -- the retry -// path previously skipped this check entirely). +// path previously skipped this check entirely). The web execute path +// reaches this through priceAndEnforcePurchaseConstraints, so recs carry +// store-derived costs; the retry path enforces against the persisted row, +// which was priced when it was written. func (h *Handler) enforcePurchaseConstraints(ctx context.Context, session *Session, recs []config.RecommendationRecord) error { constraintSets := purchaseConstraintSets(recs) if err := requireNonZeroCommitment(constraintSets); err != nil { @@ -2251,7 +2263,9 @@ func purchaseConstraintSets(recs []config.RecommendationRecord) []auth.Permissio // same *12 convention used throughout this package. A nil MonthlyCost // (all-upfront commitments have no recurring charge) or a non-positive // Term (can't be converted to months) contribute nothing to the recurring -// leg, matching the Term<=0 handling elsewhere in this package. +// leg, matching the Term<=0 handling elsewhere in this package. On the web +// execute path the two cost fields are the store-derived values written by +// priceRecommendationsFromStore. func recTotalCommitment(rec *config.RecommendationRecord) float64 { total := rec.UpfrontCost if rec.Term > 0 && rec.MonthlyCost != nil { @@ -2341,7 +2355,10 @@ func (h *Handler) finalizePurchaseStatus(ctx context.Context, execution *config. return "failed" } -// validateAndTotalRecommendations validates each recommendation and returns totals. +// validateAndTotalRecommendations validates each recommendation and returns +// totals. Runs on store-derived recs for the web path and on the persisted +// row for retry; the negative and $10M guards stay as the sanity check for +// both. func validateAndTotalRecommendations(recs []config.RecommendationRecord) (upfront, savings float64, err error) { const maxAmount = 10_000_000 // $10M sanity cap for i := range recs { diff --git a/internal/api/handler_purchases_guards_test.go b/internal/api/handler_purchases_guards_test.go index 8ff602897..f6113a785 100644 --- a/internal/api/handler_purchases_guards_test.go +++ b/internal/api/handler_purchases_guards_test.go @@ -363,6 +363,12 @@ func TestHandler_executePurchase_SurfacesPaymentAdjustments(t *testing.T) { mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil) mockStore.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{}, nil) + // #1905: both recs canonicalise to azure/vm/monthly before pricing, so + // they both match this one stored row; the second rec's client + // upfront_cost is ignored. + expectStoredRecs(mockStore, config.RecommendationRecord{ + Provider: "azure", Service: "vm", Count: 1, Term: 1, Payment: "monthly", UpfrontCost: 100, Savings: 50, + }) handler := &Handler{config: mockStore, auth: mockAuth} @@ -413,6 +419,9 @@ func TestHandler_executePurchase_NoAdjustmentsWhenCanonical(t *testing.T) { mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil) mockStore.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{}, nil) + expectStoredRecs(mockStore, config.RecommendationRecord{ + Provider: "azure", Service: "vm", Count: 1, Term: 1, Payment: "monthly", UpfrontCost: 100, Savings: 50, + }) handler := &Handler{config: mockStore, auth: mockAuth} diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go index e4a1a80e4..8d96ae34d 100644 --- a/internal/api/handler_purchases_test.go +++ b/internal/api/handler_purchases_test.go @@ -2202,6 +2202,15 @@ func TestHandler_executePurchase_Success(t *testing.T) { // The #644 idempotency lookup queries pending executions before creating. // No prior pending row → not a duplicate → proceeds to create. mockStore.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{}, nil) + // #1905: recs are now priced from the stored recommendation set. The two + // rows need distinct identity tuples (ResourceType here) — the real store + // can never hold two rows under the same tuple (migration 000043's unique + // index), and an identical-tuple fixture would trip the loadStoredRecommendationIndex + // duplicate-key guard. + expectStoredRecs(mockStore, + config.RecommendationRecord{Provider: "aws", Service: "ec2", ResourceType: "m5.large", Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: 100, Savings: 50}, + config.RecommendationRecord{Provider: "aws", Service: "ec2", ResourceType: "m5.xlarge", Count: 2, Term: 1, Payment: "all-upfront", UpfrontCost: 200, Savings: 100}, + ) handler := &Handler{config: mockStore, auth: mockAuth} @@ -2209,7 +2218,7 @@ func TestHandler_executePurchase_Success(t *testing.T) { Headers: map[string]string{ "Authorization": "Bearer admin-token", }, - Body: `{"recommendations": [{"id": "rec-1", "provider": "aws", "service": "ec2", "count": 1, "term": 1, "payment": "all-upfront", "upfront_cost": 100.0, "savings": 50.0}, {"id": "rec-2", "provider": "aws", "service": "ec2", "count": 2, "term": 1, "payment": "all-upfront", "upfront_cost": 200.0, "savings": 100.0}]}`, + Body: `{"recommendations": [{"id": "rec-1", "provider": "aws", "service": "ec2", "resource_type": "m5.large", "count": 1, "term": 1, "payment": "all-upfront", "upfront_cost": 100.0, "savings": 50.0}, {"id": "rec-2", "provider": "aws", "service": "ec2", "resource_type": "m5.xlarge", "count": 2, "term": 1, "payment": "all-upfront", "upfront_cost": 200.0, "savings": 100.0}]}`, } result, err := handler.executePurchase(ctx, req) require.NoError(t, err) @@ -2283,6 +2292,7 @@ func TestHandler_executePurchase_EmptyRecommendations(t *testing.T) { func TestHandler_executePurchase_NegativeUpfrontCost(t *testing.T) { ctx := context.Background() + mockStore := new(MockConfigStore) mockAuth := new(MockAuthService) adminSession := &Session{ @@ -2292,8 +2302,14 @@ func TestHandler_executePurchase_NegativeUpfrontCost(t *testing.T) { mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) mockAuth.grantAdminPurchaser() + // #1905: the client's upfront_cost is now ignored; a negative cost on + // the STORED row is what must be refused (the client value can no + // longer poison the check). + expectStoredRecs(mockStore, config.RecommendationRecord{ + Provider: "aws", Service: "ec2", Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: -100, + }) - handler := &Handler{auth: mockAuth} + handler := &Handler{config: mockStore, auth: mockAuth} req := &events.LambdaFunctionURLRequest{ Headers: map[string]string{ @@ -2304,11 +2320,12 @@ func TestHandler_executePurchase_NegativeUpfrontCost(t *testing.T) { result, err := handler.executePurchase(ctx, req) assert.Error(t, err) assert.Nil(t, result) - assert.Contains(t, err.Error(), "negative upfront cost") + assert.Contains(t, err.Error(), "no usable price") } func TestHandler_executePurchase_NegativeSavings(t *testing.T) { ctx := context.Background() + mockStore := new(MockConfigStore) mockAuth := new(MockAuthService) adminSession := &Session{ @@ -2318,14 +2335,23 @@ func TestHandler_executePurchase_NegativeSavings(t *testing.T) { mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) mockAuth.grantAdminPurchaser() + // #1905: pricing passes (stored upfront/count are usable); the negative + // savings on the stored row still trips the validateAndTotalRecommendations + // guard the same as before. The request itself carries a VALID savings + // value (50.0) so this only fails because the stored row's savings (-50) + // governs, not because the client's own number happens to be negative + // too (CodeRabbit finding on #2073). + expectStoredRecs(mockStore, config.RecommendationRecord{ + Provider: "aws", Service: "ec2", Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: 100, Savings: -50, + }) - handler := &Handler{auth: mockAuth} + handler := &Handler{config: mockStore, auth: mockAuth} req := &events.LambdaFunctionURLRequest{ Headers: map[string]string{ "Authorization": "Bearer admin-token", }, - Body: `{"recommendations": [{"id": "rec-1", "provider": "aws", "service": "ec2", "count": 1, "term": 1, "payment": "all-upfront", "upfront_cost": 100.0, "savings": -50.0}]}`, + Body: `{"recommendations": [{"id": "rec-1", "provider": "aws", "service": "ec2", "count": 1, "term": 1, "payment": "all-upfront", "upfront_cost": 100.0, "savings": 50.0}]}`, } result, err := handler.executePurchase(ctx, req) assert.Error(t, err) @@ -2372,6 +2398,7 @@ func TestHandler_executePurchase_TooManyRecommendations(t *testing.T) { func TestHandler_executePurchase_ExceedsMaxAmount(t *testing.T) { ctx := context.Background() + mockStore := new(MockConfigStore) mockAuth := new(MockAuthService) adminSession := &Session{ @@ -2381,14 +2408,22 @@ func TestHandler_executePurchase_ExceedsMaxAmount(t *testing.T) { mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) mockAuth.grantAdminPurchaser() + // #1905: the $10M sanity guard now fires against the stored cost. The + // request itself carries a VALID upfront_cost (100.0, well under the + // sanity limit) so this only fails because the stored row's cost + // ($15M) governs, not because the client's own number also exceeds + // the limit (CodeRabbit finding on #2073). + expectStoredRecs(mockStore, config.RecommendationRecord{ + Provider: "aws", Service: "ec2", Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: 15_000_000, Savings: 50, + }) - handler := &Handler{auth: mockAuth} + handler := &Handler{config: mockStore, auth: mockAuth} req := &events.LambdaFunctionURLRequest{ Headers: map[string]string{ "Authorization": "Bearer admin-token", }, - Body: `{"recommendations": [{"id": "rec-1", "provider": "aws", "service": "ec2", "count": 1, "term": 1, "payment": "all-upfront", "upfront_cost": 15000000.0, "savings": 50.0}]}`, + Body: `{"recommendations": [{"id": "rec-1", "provider": "aws", "service": "ec2", "count": 1, "term": 1, "payment": "all-upfront", "upfront_cost": 100.0, "savings": 50.0}]}`, } result, err := handler.executePurchase(ctx, req) assert.Error(t, err) @@ -2411,6 +2446,9 @@ func TestHandler_executePurchase_SaveError(t *testing.T) { mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(errors.New("database error")) mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil) mockStore.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{}, nil) + expectStoredRecs(mockStore, config.RecommendationRecord{ + Provider: "aws", Service: "ec2", Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: 100, Savings: 50, + }) handler := &Handler{config: mockStore, auth: mockAuth} @@ -3674,6 +3712,10 @@ func setupDirectExecMocks(ctx context.Context, store *MockConfigStore) { store.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) store.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil) store.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{}, nil) + // #1905: directExecRecBody's rec-1 is now priced from the stored set. + expectStoredRecs(store, config.RecommendationRecord{ + ID: "rec-1", Provider: "aws", Service: "ec2", Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: 500, Savings: 100, + }) } // TestHandler_executePurchase_DirectExec_NoPermission verifies the fail-closed @@ -3767,6 +3809,11 @@ func TestHandler_executePurchase_PermissionConstraintsDenied(t *testing.T) { assert.ObjectsAreEqual([]string{"ec2"}, sets[1].Services) && assert.ObjectsAreEqual([]string{"eu-west-1"}, sets[1].Regions) })).Return(false, nil) + // #1905: constraint sets are now built from the stored costs. + expectStoredRecs(mockStore, + config.RecommendationRecord{Provider: "aws", Service: "ec2", Region: "us-east-1", Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: 3000, Savings: 50}, + config.RecommendationRecord{Provider: "aws", Service: "ec2", Region: "eu-west-1", Count: 2, Term: 1, Payment: "all-upfront", UpfrontCost: 2500, Savings: 100}, + ) handler := &Handler{config: mockStore, auth: mockAuth} req := &events.LambdaFunctionURLRequest{ @@ -3823,6 +3870,12 @@ func TestHandler_executePurchase_NoUpfrontBatch_TotalCommitmentEnforced(t *testi mock.MatchedBy(func(sets []auth.PermissionConstraints) bool { return len(sets) == 1 && sets[0].MaxPurchaseAmount == wantTotalCommitment })).Return(false, nil) + // #1905: the total-commitment constraint is now built from the stored + // upfront/monthly cost. + expectStoredRecs(mockStore, config.RecommendationRecord{ + Provider: "aws", Service: "ec2", Region: "us-east-1", Count: 1, Term: 3, Payment: "no-upfront", + UpfrontCost: 0, MonthlyCost: float64Ptr(600), Savings: 50, + }) handler := &Handler{config: mockStore, auth: mockAuth} req := &events.LambdaFunctionURLRequest{ @@ -3881,6 +3934,10 @@ func TestHandler_executePurchase_UserAPIKeyConstraintsDenied(t *testing.T) { } return sets[0].MaxPurchaseAmount == 500.0 })).Return(false, nil) + // #1905: the constraint set is now built from the stored cost. + expectStoredRecs(mockStore, config.RecommendationRecord{ + Provider: "aws", Service: "ec2", Region: "us-east-1", Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: 500, Savings: 50, + }) handler := &Handler{config: mockStore, auth: mockAuth} req := &events.LambdaFunctionURLRequest{ @@ -4107,6 +4164,10 @@ func TestHandler_executePurchase_DirectExec_FourEyesOn_DeniesSelfExecute(t *test mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) mockStore.On("GetGlobalConfig", ctx).Return(fourEyesCfgOn(), nil) mockStore.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{}, nil) + // #1905: directExecRecBody's rec-1 is now priced from the stored set. + expectStoredRecs(mockStore, config.RecommendationRecord{ + ID: "rec-1", Provider: "aws", Service: "ec2", Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: 500, Savings: 100, + }) // enforceFourEyesPolicy re-loads the (freshly-created, randomly-ID'd) // execution to read CreatedByUserID; the real one always carries the // direct-executor's own UUID (see comment above), which is what makes diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index e3529bd54..ca2dcde5f 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -466,6 +466,29 @@ paths: $ref: '#/components/responses/Unauthorized' '403': $ref: '#/components/responses/Forbidden' + '409': + description: > + One of two conflict families. Idempotency: an identical submit + already holds the idempotency claim, so THIS request was not + executed (see the shared Conflict response for how to interpret + that case). Stored-recommendation conflict (#1905): a + recommendation in the request does not match any row in the + current stored recommendation set (it was purchased, superseded, + or never existed), two requested recommendations resolve to the + same identity key, two rows in the stored set share one identity + key (which is refused rather than resolved arbitrarily, and can + occur even for a single-recommendation request), a matched stored + row has no usable price to derive the spend cap from, or a + Savings Plan's requested count does not equal the stored + recommended count. Recovery differs by + cause: for the idempotency case, verify the earlier submit's + outcome with the provider before resubmitting; for a + stored-recommendation conflict, refresh the recommendation set + and resubmit with the current data. + content: + application/json: + schema: + $ref: '#/components/schemas/Error' /api/purchases/approve/{id}: parameters: @@ -2620,8 +2643,28 @@ components: type: string engine: type: string + cloud_account_id: + type: string + nullable: true + description: > + The cloud account this recommendation belongs to. Matched + against the stored recommendation set together with provider, + service, region, resource_type, engine, term and payment + (#1905); an account-scoped recommendation cannot be resolved + without it. Null or omitted only for recommendations that are + not tied to a specific account. count: type: integer + recommended_count: + type: integer + description: > + The pre-scaling count originally recommended, before any + Capacity % scaling was applied to produce count. Optional: + omitted or 0 means "not supplied" and the capacity-consistency + check is skipped for this recommendation. For a Savings Plan, + the stored value of this field is also the count that count + must equal, since Savings Plans are priced by hourly commitment + rather than by count. term: type: integer payment: @@ -2848,6 +2891,15 @@ components: $ref: '#/components/schemas/RecommendationRecord' minItems: 1 maxItems: 1000 + description: > + Each item must match a recommendation in the current recommendation set on + (provider, cloud_account_id, service, region, resource_type, engine, term, + payment). Only count, recommended_count and selected are honoured; every other + field (including id, details, upfront_cost, monthly_cost, savings, + on_demand_cost, purchased, purchase_id and error) is replaced server-side from + the stored recommendation, with the cost fields scaled by count. The spend-cap + check, execution record and approval email use those replaced values. Savings + plan items must keep the recommended count. ExecutePurchaseResponse: type: object diff --git a/internal/api/purchase_pricing.go b/internal/api/purchase_pricing.go new file mode 100644 index 000000000..c32089a2d --- /dev/null +++ b/internal/api/purchase_pricing.go @@ -0,0 +1,160 @@ +package api + +import ( + "context" + "fmt" + "strconv" + "strings" + + "github.com/LeanerCloud/CUDly/internal/config" + "github.com/LeanerCloud/CUDly/pkg/common" +) + +// priceAndEnforcePurchaseConstraints replaces every rec's cost fields with +// values derived from the stored recommendation that matches it, then +// enforces the execute:purchases Constraints against those values. The +// client-supplied upfront_cost / monthly_cost / savings / details are never +// consulted for the spend cap, the execution row or the approval email +// (issue #1905, audit A01-001). Wrapped in one call so +// validateExecutePurchaseRequest keeps its gocyclo score (it is at the +// pre-commit ceiling of 10). +func (h *Handler) priceAndEnforcePurchaseConstraints(ctx context.Context, session *Session, recs []config.RecommendationRecord) error { + if err := h.priceRecommendationsFromStore(ctx, recs); err != nil { + return err + } + return h.enforcePurchaseConstraints(ctx, session, recs) +} + +// priceRecommendationsFromStore rewrites recs in place. Each rec must match +// a row of the stored recommendation set on its identity tuple +// (recIdentityKey); unmatched recs are refused rather than priced from the +// request. Matching by tuple instead of by rec.ID lets the purchase modal's +// term/payment change (#111, #197, #1903) resolve to the stored variant the +// user actually chose: AWS stores every (term, payment) combination and +// Azure stores both payment variants, each under its own id. +func (h *Handler) priceRecommendationsFromStore(ctx context.Context, recs []config.RecommendationRecord) error { + stored, err := h.loadStoredRecommendationIndex(ctx, recs) + if err != nil { + return err + } + for i := range recs { + match, ok := stored[recIdentityKey(&recs[i])] + if !ok { + return NewClientError(409, fmt.Sprintf( + "recommendation %d (%s) is not in the current recommendation set; refresh the recommendations and try again", + i, describeRec(&recs[i]))) + } + priced, err := priceFromStored(&recs[i], &match, i) + if err != nil { + return err + } + recs[i] = priced + } + return nil +} + +// loadStoredRecommendationIndex reads the stored rows for every provider in +// the batch (one query per distinct provider, at most three) and indexes +// them by identity tuple. The store's unique index on the same tuple +// (migration 000043) guarantees one row per key only when provider and +// payment are byte-identical: the index is case-sensitive on both columns, +// while recIdentityKey folds their case, so two rows differing only in case +// would collide here (unreachable today because the scheduler always writes +// lowercase, but not guaranteed by the index itself). Rather than silently +// picking whichever row wins the map insert, a collision is refused: this is +// a money path, and the caller (priceRecommendationsFromStore) must never +// price a purchase off an arbitrarily chosen row. +func (h *Handler) loadStoredRecommendationIndex(ctx context.Context, recs []config.RecommendationRecord) (map[string]config.RecommendationRecord, error) { + index := make(map[string]config.RecommendationRecord) + seen := make(map[string]bool) + for i := range recs { + provider := recs[i].Provider + if seen[provider] { + continue + } + seen[provider] = true + rows, err := h.config.ListStoredRecommendations(ctx, config.RecommendationFilter{Provider: provider}) + if err != nil { + return nil, fmt.Errorf("load stored recommendations for %s: %w", provider, err) + } + for j := range rows { + key := recIdentityKey(&rows[j]) + if _, dup := index[key]; dup { + return nil, fmt.Errorf("stored recommendations for %s contain more than one row for identity key %q", provider, key) + } + index[key] = rows[j] + } + } + return index, nil +} + +// recIdentityKey is the tuple a price is a function of: the same eight +// components the scheduler encodes into RecommendationRecord.ID, with a nil +// and an empty CloudAccountID collapsed together (as purchaseConstraintSets +// and the store's account_key both do). Provider and payment are lowercased +// because validatePurchaseRecommendation canonicalises the request side. +func recIdentityKey(rec *config.RecommendationRecord) string { + account := "" + if rec.CloudAccountID != nil { + account = *rec.CloudAccountID + } + return strings.Join([]string{ + strings.ToLower(rec.Provider), account, rec.Service, rec.Region, rec.ResourceType, + rec.Engine, strconv.Itoa(rec.Term), strings.ToLower(rec.Payment), + }, "\x1f") +} + +// priceFromStored returns a copy of stored carrying the request's Count, +// RecommendedCount and Selected, with UpfrontCost, MonthlyCost, Savings and +// OnDemandCost scaled by Count/stored.Count (stored costs are totals for +// stored.Count; RI/CUD/Azure prices are linear in count). Savings Plans are +// not count-denominated, so their Count must equal the stored placeholder. +// A stored row that cannot yield a positive price is refused: the cap must +// never be evaluated against a zero or negative commitment. +func priceFromStored(req, stored *config.RecommendationRecord, idx int) (config.RecommendationRecord, error) { + if stored.Count <= 0 { + return config.RecommendationRecord{}, NewClientError(409, fmt.Sprintf( + "recommendation %d (%s): stored recommendation has count %d, cannot derive a per-unit price", + idx, describeRec(req), stored.Count)) + } + if stored.UpfrontCost < 0 || (stored.MonthlyCost != nil && *stored.MonthlyCost < 0) || recTotalCommitment(stored) <= 0 { + return config.RecommendationRecord{}, NewClientError(409, fmt.Sprintf( + "recommendation %d (%s): stored recommendation carries no usable price (upfront %.2f, monthly %s); the spend cap cannot be enforced against it", + idx, describeRec(req), stored.UpfrontCost, formatOptionalCost(stored.MonthlyCost))) + } + if common.IsSavingsPlan(common.ServiceType(stored.Service)) && req.Count != stored.Count { + return config.RecommendationRecord{}, NewClientError(409, fmt.Sprintf( + "recommendation %d (%s): savings plan count %d must equal the recommended count %d; savings plans are priced by hourly commitment, not by count", + idx, describeRec(req), req.Count, stored.Count)) + } + ratio := float64(req.Count) / float64(stored.Count) + out := *stored + out.Count = req.Count + out.RecommendedCount = req.RecommendedCount + out.Selected = req.Selected + out.UpfrontCost = stored.UpfrontCost * ratio + out.Savings = stored.Savings * ratio + out.MonthlyCost = scaledCost(stored.MonthlyCost, ratio) + out.OnDemandCost = scaledCost(stored.OnDemandCost, ratio) + return out, nil +} + +func scaledCost(v *float64, ratio float64) *float64 { + if v == nil { + return nil + } + s := *v * ratio + return &s +} + +func formatOptionalCost(v *float64) string { + if v == nil { + return "absent" + } + return fmt.Sprintf("%.2f", *v) +} + +func describeRec(rec *config.RecommendationRecord) string { + return fmt.Sprintf("%s/%s %s %s engine=%q term=%d payment=%s account=%s", + rec.Provider, rec.Service, rec.Region, rec.ResourceType, rec.Engine, rec.Term, rec.Payment, derefStringOrEmpty(rec.CloudAccountID)) +} diff --git a/internal/api/purchase_pricing_test.go b/internal/api/purchase_pricing_test.go new file mode 100644 index 000000000..d43fed38d --- /dev/null +++ b/internal/api/purchase_pricing_test.go @@ -0,0 +1,612 @@ +package api + +import ( + "context" + "encoding/json" + "errors" + "testing" + + "github.com/LeanerCloud/CUDly/internal/auth" + "github.com/LeanerCloud/CUDly/internal/config" + "github.com/LeanerCloud/CUDly/pkg/common" + "github.com/aws/aws-lambda-go/events" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// expectStoredRecs registers the ListStoredRecommendations expectation the +// pricing lookup issues: one call per distinct provider, filtered by provider +// only. Registering the exact filter pins that contract. +func expectStoredRecs(store *MockConfigStore, recs ...config.RecommendationRecord) { + byProvider := map[string][]config.RecommendationRecord{} + for _, r := range recs { + byProvider[r.Provider] = append(byProvider[r.Provider], r) + } + for provider, rows := range byProvider { + store.On("ListStoredRecommendations", mock.Anything, config.RecommendationFilter{Provider: provider}).Return(rows, nil) + } +} + +// TestHandler_executePurchase_CapUsesStoredPriceNotClientPrice is the issue's +// reproduction (audit A01-001): a purchaser holding execute:purchases with a +// $1,000 MaxPurchaseAmount cap submits upfront_cost: 1 for 100 x m5.24xlarge +// x 3yr all-upfront reservations whose real stored price is $1,000,000. The +// cap must be enforced against the stored price, not the client's number. +func TestHandler_executePurchase_CapUsesStoredPriceNotClientPrice(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + mockPurchase := new(MockPurchaseManager) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + t.Cleanup(func() { mockPurchase.AssertExpectations(t) }) + + session := &Session{UserID: "dddddddd-dddd-dddd-dddd-dddddddddddd", Email: "capped@example.com"} + mockAuth.On("ValidateSession", ctx, "cap-token").Return(session, nil) + mockAuth.grantPermissions([]auth.Permission{ + {Action: auth.ActionExecute, Resource: auth.ResourcePurchases, Constraints: &auth.PermissionConstraints{MaxPurchaseAmount: 1000}}, + {Action: auth.ActionExecuteAny, Resource: auth.ResourcePurchases}, + }) + + expectStoredRecs(mockStore, config.RecommendationRecord{ + ID: "aws|123456789012|ec2|us-east-1|m5.24xlarge||3|all-upfront", + Provider: "aws", Service: "ec2", Region: "us-east-1", ResourceType: "m5.24xlarge", + Count: 100, Term: 3, Payment: "all-upfront", UpfrontCost: 1_000_000, Savings: 40_000, + }) + // Registered .Maybe(): pre-fix these are reached, post-fix they must not + // be. Not requiring GetGlobalConfig/GetPendingExecutions here because the + // mock's isExpected guard on GetGlobalConfig defaults gracefully, and + // this test is not asserting on the persisted row's suppression window; + // GetPendingExecutions has no such guard, so it must be stubbed to avoid + // a panic on the pre-fix run, which does reach persistExecutionAndSuppressions. + mockStore.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil).Maybe() + mockStore.On("GetPendingExecutions", mock.Anything).Return([]config.PurchaseExecution{}, nil).Maybe() + mockStore.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil).Maybe() + mockPurchase.On("ApproveAndExecute", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe() + + handler := &Handler{config: mockStore, auth: mockAuth, purchase: mockPurchase} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer cap-token"}, + Body: `{"recommendations":[{"id":"aws|123456789012|ec2|us-east-1|m5.24xlarge||3|all-upfront","provider":"aws","service":"ec2","region":"us-east-1","resource_type":"m5.24xlarge","count":100,"term":3,"payment":"all-upfront","upfront_cost":1,"monthly_cost":null,"savings":40000,"selected":true}],"execute_mode":"direct"}`, + } + _, err := handler.executePurchase(ctx, req) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "expected a clientError, got: %v", err) + assert.Equal(t, 403, ce.code) + assert.Contains(t, ce.Error(), "constraints") + mockStore.AssertNotCalled(t, "SavePurchaseExecution", mock.Anything, mock.Anything) + mockPurchase.AssertNotCalled(t, "ApproveAndExecute", mock.Anything, mock.Anything, mock.Anything, mock.Anything) +} + +// TestHandler_executePurchase_StoredPriceUnderCapProceeds is the no-over- +// rejection companion to T1: the client overstates upfront_cost far beyond +// the cap, but the STORED price is well under it, so the purchase must +// proceed and the response must reflect the stored numbers. +func TestHandler_executePurchase_StoredPriceUnderCapProceeds(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + mockPurchase := new(MockPurchaseManager) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + t.Cleanup(func() { mockPurchase.AssertExpectations(t) }) + + session := &Session{UserID: "dddddddd-dddd-dddd-dddd-dddddddddddd", Email: "capped@example.com"} + mockAuth.On("ValidateSession", ctx, "cap-token").Return(session, nil) + mockAuth.grantPermissions([]auth.Permission{ + {Action: auth.ActionExecute, Resource: auth.ResourcePurchases, Constraints: &auth.PermissionConstraints{MaxPurchaseAmount: 1000}}, + {Action: auth.ActionExecuteAny, Resource: auth.ResourcePurchases}, + }) + + expectStoredRecs(mockStore, config.RecommendationRecord{ + ID: "aws|123456789012|ec2|us-east-1|m5.24xlarge||3|all-upfront", + Provider: "aws", Service: "ec2", Region: "us-east-1", ResourceType: "m5.24xlarge", + Count: 1, Term: 3, Payment: "all-upfront", UpfrontCost: 900, Savings: 30, + }) + mockStore.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil) + mockStore.On("GetPendingExecutions", mock.Anything).Return([]config.PurchaseExecution{}, nil) + mockStore.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil) + mockPurchase.On("ApproveAndExecute", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + + handler := &Handler{config: mockStore, auth: mockAuth, purchase: mockPurchase} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer cap-token"}, + Body: `{"recommendations":[{"id":"aws|123456789012|ec2|us-east-1|m5.24xlarge||3|all-upfront","provider":"aws","service":"ec2","region":"us-east-1","resource_type":"m5.24xlarge","count":1,"term":3,"payment":"all-upfront","upfront_cost":5000,"savings":30,"selected":true}],"execute_mode":"direct"}`, + } + result, err := handler.executePurchase(ctx, req) + require.NoError(t, err) + resultMap := result.(map[string]any) + assert.Equal(t, "completed", resultMap["status"]) + assert.Equal(t, 900.0, resultMap["total_upfront_cost"]) + assert.Equal(t, 30.0, resultMap["estimated_savings"]) +} + +// TestHandler_executePurchase_PersistsStoredCostsNotClientCosts pins the +// second half of #1905: the execution row and the approval email must carry +// the store-derived costs, not the client's. +func TestHandler_executePurchase_PersistsStoredCostsNotClientCosts(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + + adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"} + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.grantAdminPurchaser() + + notify := "notify@example.com" + mockStore.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{NotificationEmail: ¬ify}, nil) + mockStore.On("GetPendingExecutions", mock.Anything).Return([]config.PurchaseExecution{}, nil) + + var saved *config.PurchaseExecution + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")). + Run(func(args mock.Arguments) { saved = args.Get(1).(*config.PurchaseExecution) }). + Return(nil) + + expectStoredRecs(mockStore, config.RecommendationRecord{ + ID: "stored-id", Provider: "aws", Service: "ec2", Region: "us-east-1", ResourceType: "m5.24xlarge", + Count: 4, Term: 3, Payment: "all-upfront", + UpfrontCost: 4000, MonthlyCost: float64Ptr(40), Savings: 400, OnDemandCost: float64Ptr(1000), + Details: json.RawMessage(`{"platform":"Linux/UNIX"}`), + }) + + notifier := &recordingEmailNotifier{} + handler := &Handler{config: mockStore, auth: mockAuth, emailNotifier: notifier} + + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + Body: `{"recommendations":[{"id":"client-id","provider":"aws","service":"ec2","region":"us-east-1","resource_type":"m5.24xlarge","count":2,"term":3,"payment":"all-upfront","recommended_count":4,"upfront_cost":1,"monthly_cost":1,"savings":999,"on_demand_cost":1,"details":{"platform":"Windows"},"selected":true}],"capacity_percent":50}`, + } + result, err := handler.executePurchase(ctx, req) + require.NoError(t, err) + + require.NotNil(t, saved) + require.Len(t, saved.Recommendations, 1) + rec := saved.Recommendations[0] + assert.Equal(t, "stored-id", rec.ID) + assert.Equal(t, 2, rec.Count) + assert.Equal(t, 4, rec.RecommendedCount) + assert.Equal(t, 2000.0, rec.UpfrontCost) + require.NotNil(t, rec.MonthlyCost) + assert.Equal(t, 20.0, *rec.MonthlyCost) + assert.Equal(t, 200.0, rec.Savings) + require.NotNil(t, rec.OnDemandCost) + assert.Equal(t, 500.0, *rec.OnDemandCost) + assert.Equal(t, `{"platform":"Linux/UNIX"}`, string(rec.Details)) + assert.True(t, rec.Selected) + + assert.Equal(t, 2000.0, saved.TotalUpfrontCost) + assert.Equal(t, 200.0, saved.EstimatedSavings) + + assert.Equal(t, 2000.0, notifier.captured.TotalUpfrontCost) + assert.Equal(t, 200.0, notifier.captured.TotalSavings) + + resultMap := result.(map[string]any) + assert.Equal(t, 2000.0, resultMap["total_upfront_cost"]) + assert.Equal(t, 200.0, resultMap["estimated_savings"]) + assert.Equal(t, true, resultMap["email_sent"]) +} + +// TestHandler_executePurchase_UnknownRecommendationRefused: a rec that +// matches no stored recommendation is refused with 409, before anything is +// persisted. +func TestHandler_executePurchase_UnknownRecommendationRefused(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + + adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"} + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.grantAdminPurchaser() + + // Registered .Maybe(): grantAdminPurchaser carries no MaxPurchaseAmount + // cap, so pre-fix the (unpriced) constraint check trivially passes and + // the flow reaches persistExecutionAndSuppressions, which needs these to + // avoid a panic on the unstubbed mock; post-fix they are never reached. + mockStore.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil).Maybe() + mockStore.On("GetPendingExecutions", mock.Anything).Return([]config.PurchaseExecution{}, nil).Maybe() + mockStore.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil).Maybe() + expectStoredRecs(mockStore, config.RecommendationRecord{ + Provider: "aws", Service: "ec2", ResourceType: "m5.large", Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: 100, + }) + + handler := &Handler{config: mockStore, auth: mockAuth} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + Body: `{"recommendations":[{"id":"rec-1","provider":"aws","service":"ec2","resource_type":"m5.xlarge","count":1,"term":1,"payment":"all-upfront","upfront_cost":100,"savings":10}]}`, + } + _, err := handler.executePurchase(ctx, req) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "expected a clientError, got: %v", err) + assert.Equal(t, 409, ce.code) + assert.Contains(t, ce.Error(), "not in the current recommendation set") + mockStore.AssertNotCalled(t, "SavePurchaseExecution", mock.Anything, mock.Anything) +} + +// TestHandler_executePurchase_CrossAccountMismatchRefused is a handler-level +// companion to TestRecIdentityKey_TupleSemantics' "a different account +// yields a different key" subtest: that pure unit test proves recIdentityKey +// includes the account, but not that the scope check and the key agree end +// to end, through the real handler, on what "account" means (a mutation +// that dropped CloudAccountID from recIdentityKey would still pass an +// unrestricted admin session's scope check). Stored recommendations exist +// only under account A; the request claims the identical resource under +// account B. The mismatch must be refused with 409 before anything is +// persisted or the provider is contacted. +func TestHandler_executePurchase_CrossAccountMismatchRefused(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + mockPurchase := new(MockPurchaseManager) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + t.Cleanup(func() { mockPurchase.AssertExpectations(t) }) + + adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"} + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.grantAdminPurchaser() + + // Registered .Maybe(): see TestHandler_executePurchase_UnknownRecommendationRefused. + mockStore.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil).Maybe() + mockStore.On("GetPendingExecutions", mock.Anything).Return([]config.PurchaseExecution{}, nil).Maybe() + mockStore.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil).Maybe() + mockPurchase.On("ApproveAndExecute", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe() + + accountA := "111111111111" + expectStoredRecs(mockStore, config.RecommendationRecord{ + Provider: "aws", Service: "ec2", Region: "us-east-1", ResourceType: "m5.large", + CloudAccountID: &accountA, Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: 100, + }) + + handler := &Handler{config: mockStore, auth: mockAuth, purchase: mockPurchase} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + Body: `{"recommendations":[{"id":"rec-1","provider":"aws","service":"ec2","region":"us-east-1","resource_type":"m5.large","cloud_account_id":"222222222222","count":1,"term":1,"payment":"all-upfront","upfront_cost":100,"savings":10}]}`, + } + _, err := handler.executePurchase(ctx, req) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "expected a clientError, got: %v", err) + assert.Equal(t, 409, ce.code) + assert.Contains(t, ce.Error(), "not in the current recommendation set") + mockStore.AssertNotCalled(t, "SavePurchaseExecution", mock.Anything, mock.Anything) + mockPurchase.AssertNotCalled(t, "ApproveAndExecute", mock.Anything, mock.Anything, mock.Anything, mock.Anything) +} + +// TestHandler_executePurchase_StoredRecWithoutPriceRefused: a matched stored +// row that carries no usable price is refused with 409. +func TestHandler_executePurchase_StoredRecWithoutPriceRefused(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + + adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"} + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.grantAdminPurchaser() + + // Registered .Maybe(): see TestHandler_executePurchase_UnknownRecommendationRefused. + mockStore.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil).Maybe() + mockStore.On("GetPendingExecutions", mock.Anything).Return([]config.PurchaseExecution{}, nil).Maybe() + mockStore.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil).Maybe() + expectStoredRecs(mockStore, config.RecommendationRecord{ + Provider: "aws", Service: "ec2", ResourceType: "m5.large", Count: 1, Term: 1, Payment: "all-upfront", + UpfrontCost: 0, MonthlyCost: nil, + }) + + handler := &Handler{config: mockStore, auth: mockAuth} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + Body: `{"recommendations":[{"id":"rec-1","provider":"aws","service":"ec2","resource_type":"m5.large","count":1,"term":1,"payment":"all-upfront","upfront_cost":100,"savings":10}]}`, + } + _, err := handler.executePurchase(ctx, req) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "expected a clientError, got: %v", err) + assert.Equal(t, 409, ce.code) + assert.Contains(t, ce.Error(), "no usable price") + mockStore.AssertNotCalled(t, "SavePurchaseExecution", mock.Anything, mock.Anything) +} + +// TestHandler_executePurchase_StoredRecWithZeroCountRefused: a matched +// stored row with a non-positive Count cannot yield a per-unit price and +// must be refused with 409 rather than dividing by zero or by a negative +// count (section 4 of the #1905 plan). +func TestHandler_executePurchase_StoredRecWithZeroCountRefused(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + + adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"} + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.grantAdminPurchaser() + + // Registered .Maybe(): see TestHandler_executePurchase_UnknownRecommendationRefused. + mockStore.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil).Maybe() + mockStore.On("GetPendingExecutions", mock.Anything).Return([]config.PurchaseExecution{}, nil).Maybe() + mockStore.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil).Maybe() + expectStoredRecs(mockStore, config.RecommendationRecord{ + Provider: "aws", Service: "ec2", ResourceType: "m5.large", Count: 0, Term: 1, Payment: "all-upfront", UpfrontCost: 100, + }) + + handler := &Handler{config: mockStore, auth: mockAuth} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + Body: `{"recommendations":[{"id":"rec-1","provider":"aws","service":"ec2","resource_type":"m5.large","count":1,"term":1,"payment":"all-upfront","upfront_cost":100,"savings":10}]}`, + } + _, err := handler.executePurchase(ctx, req) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "expected a clientError, got: %v", err) + assert.Equal(t, 409, ce.code) + assert.Contains(t, ce.Error(), "cannot derive a per-unit price") + mockStore.AssertNotCalled(t, "SavePurchaseExecution", mock.Anything, mock.Anything) +} + +// TestHandler_executePurchase_StoreErrorFailsClosed: a store read error is a +// 500, never a client-value fallback. +func TestHandler_executePurchase_StoreErrorFailsClosed(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + + adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"} + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.grantAdminPurchaser() + + // Registered .Maybe(): see TestHandler_executePurchase_UnknownRecommendationRefused. + mockStore.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil).Maybe() + mockStore.On("GetPendingExecutions", mock.Anything).Return([]config.PurchaseExecution{}, nil).Maybe() + mockStore.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil).Maybe() + mockStore.On("ListStoredRecommendations", mock.Anything, config.RecommendationFilter{Provider: "aws"}). + Return(nil, errors.New("pg down")) + + handler := &Handler{config: mockStore, auth: mockAuth} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + Body: `{"recommendations":[{"id":"rec-1","provider":"aws","service":"ec2","resource_type":"m5.large","count":1,"term":1,"payment":"all-upfront","upfront_cost":100,"savings":10}]}`, + } + _, err := handler.executePurchase(ctx, req) + require.Error(t, err) + _, isClient := IsClientError(err) + assert.False(t, isClient, "a store read failure must not be reported as a client error") + assert.ErrorContains(t, err, "load stored recommendations for aws") + mockStore.AssertNotCalled(t, "SavePurchaseExecution", mock.Anything, mock.Anything) +} + +// TestHandler_executePurchase_PaymentChangePricedFromStoredVariant: the +// purchase modal's term/payment change (#111, #197, #1903) resolves to the +// stored variant the user actually chose, not the id's original variant. +func TestHandler_executePurchase_PaymentChangePricedFromStoredVariant(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + + session := &Session{UserID: "dddddddd-dddd-dddd-dddd-dddddddddddd", Email: "capped@example.com"} + mockAuth.On("ValidateSession", ctx, "cap-token").Return(session, nil) + mockAuth.grantPermissions([]auth.Permission{ + {Action: auth.ActionExecute, Resource: auth.ResourcePurchases, Constraints: &auth.PermissionConstraints{MaxPurchaseAmount: 1500}}, + }) + + mockStore.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil) + mockStore.On("GetPendingExecutions", mock.Anything).Return([]config.PurchaseExecution{}, nil) + var saved *config.PurchaseExecution + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")). + Run(func(args mock.Arguments) { saved = args.Get(1).(*config.PurchaseExecution) }). + Return(nil) + + expectStoredRecs(mockStore, + config.RecommendationRecord{ + ID: "aws|acct|ec2|us-east-1|m5.large||1|all-upfront", + Provider: "aws", Service: "ec2", Region: "us-east-1", ResourceType: "m5.large", + Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: 3000, Savings: 10, + }, + config.RecommendationRecord{ + ID: "aws|acct|ec2|us-east-1|m5.large||1|no-upfront", + Provider: "aws", Service: "ec2", Region: "us-east-1", ResourceType: "m5.large", + Count: 1, Term: 1, Payment: "no-upfront", UpfrontCost: 0, MonthlyCost: float64Ptr(100), Savings: 10, + }, + ) + + handler := &Handler{config: mockStore, auth: mockAuth} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer cap-token"}, + // Client sends variant A's id, but the NEW payment "no-upfront" with + // stale variant-A costs. + Body: `{"recommendations":[{"id":"aws|acct|ec2|us-east-1|m5.large||1|all-upfront","provider":"aws","service":"ec2","region":"us-east-1","resource_type":"m5.large","count":1,"term":1,"payment":"no-upfront","upfront_cost":3000,"savings":10}]}`, + } + result, err := handler.executePurchase(ctx, req) + require.NoError(t, err) + require.NotNil(t, saved) + require.Len(t, saved.Recommendations, 1) + assert.Equal(t, "aws|acct|ec2|us-east-1|m5.large||1|no-upfront", saved.Recommendations[0].ID) + assert.Equal(t, 0.0, saved.Recommendations[0].UpfrontCost) + require.NotNil(t, saved.Recommendations[0].MonthlyCost) + assert.Equal(t, 100.0, *saved.Recommendations[0].MonthlyCost) + resultMap := result.(map[string]any) + assert.Equal(t, 0.0, resultMap["total_upfront_cost"]) +} + +// TestHandler_executePurchase_SavingsPlanCountMustMatchStored: a Savings Plan +// rec is priced by hourly commitment, not by count; a count mismatch is +// refused rather than scaled. +func TestHandler_executePurchase_SavingsPlanCountMustMatchStored(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + + adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"} + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.grantAdminPurchaser() + + // Registered .Maybe(): see TestHandler_executePurchase_UnknownRecommendationRefused. + mockStore.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil).Maybe() + mockStore.On("GetPendingExecutions", mock.Anything).Return([]config.PurchaseExecution{}, nil).Maybe() + mockStore.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil).Maybe() + expectStoredRecs(mockStore, config.RecommendationRecord{ + Provider: "aws", Service: string(common.ServiceSavingsPlansCompute), Count: 1, Term: 1, Payment: "all-upfront", + UpfrontCost: 0, MonthlyCost: float64Ptr(200), + }) + + handler := &Handler{config: mockStore, auth: mockAuth} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + Body: `{"recommendations":[{"id":"rec-1","provider":"aws","service":"savings-plans-compute","count":2,"term":1,"payment":"all-upfront","upfront_cost":0,"savings":10}]}`, + } + _, err := handler.executePurchase(ctx, req) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "expected a clientError, got: %v", err) + assert.Equal(t, 409, ce.code) + assert.Contains(t, ce.Error(), "savings plan count") + mockStore.AssertNotCalled(t, "SavePurchaseExecution", mock.Anything, mock.Anything) +} + +// TestPriceFromStored_ScalesByCountAndKeepsStoredIdentity is a pure unit +// test of priceFromStored's scaling and field-provenance contract. +func TestPriceFromStored_ScalesByCountAndKeepsStoredIdentity(t *testing.T) { + stored := config.RecommendationRecord{ + ID: "stored-id", Provider: "aws", Service: "ec2", Count: 4, + UpfrontCost: 1000, Savings: 100, MonthlyCost: nil, OnDemandCost: float64Ptr(2000), + Details: json.RawMessage(`{"platform":"Linux/UNIX"}`), + } + + t.Run("ratio 0.5 with nil MonthlyCost stays nil", func(t *testing.T) { + req := config.RecommendationRecord{Count: 2, RecommendedCount: 4, Selected: true} + out, err := priceFromStored(&req, &stored, 0) + require.NoError(t, err) + assert.Equal(t, 500.0, out.UpfrontCost) + assert.Equal(t, 50.0, out.Savings) + assert.Nil(t, out.MonthlyCost) + require.NotNil(t, out.OnDemandCost) + assert.Equal(t, 1000.0, *out.OnDemandCost) + }) + + t.Run("ratio 1 reproduces stored values exactly", func(t *testing.T) { + req := config.RecommendationRecord{Count: 4, RecommendedCount: 4, Selected: true} + out, err := priceFromStored(&req, &stored, 0) + require.NoError(t, err) + assert.Equal(t, stored.UpfrontCost, out.UpfrontCost) + assert.Equal(t, stored.Savings, out.Savings) + require.NotNil(t, out.OnDemandCost) + assert.Equal(t, *stored.OnDemandCost, *out.OnDemandCost) + }) + + t.Run("Selected and RecommendedCount come from the request", func(t *testing.T) { + req := config.RecommendationRecord{Count: 4, RecommendedCount: 8, Selected: false} + out, err := priceFromStored(&req, &stored, 0) + require.NoError(t, err) + assert.Equal(t, 8, out.RecommendedCount) + assert.False(t, out.Selected) + }) + + t.Run("ID, Details, Purchased, PurchaseID and Error come from stored even when the request sets Purchased", func(t *testing.T) { + req := config.RecommendationRecord{ + Count: 4, RecommendedCount: 4, Selected: true, + Purchased: true, PurchaseID: "req-purchase-id", Error: "req-error", + } + out, err := priceFromStored(&req, &stored, 0) + require.NoError(t, err) + assert.Equal(t, "stored-id", out.ID) + assert.Equal(t, string(stored.Details), string(out.Details)) + assert.False(t, out.Purchased, "Purchased must come from the stored row, not the request") + assert.Empty(t, out.PurchaseID) + assert.Empty(t, out.Error) + }) +} + +// TestLoadStoredRecommendationIndex_CaseFoldCollisionRefused: the store's +// unique index on the identity tuple (migration 000043) is case-sensitive on +// provider and payment, while recIdentityKey folds their case. Two stored +// rows differing only in case therefore collide under the fold even though +// the index allowed both rows to exist; the index build must refuse rather +// than silently keep whichever row inserted last (unreachable today because +// the scheduler always writes lowercase, but this is a money path and must +// fail loud rather than pick a row on a broken assumption). +func TestLoadStoredRecommendationIndex_CaseFoldCollisionRefused(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + + mockStore.On("ListStoredRecommendations", mock.Anything, config.RecommendationFilter{Provider: "aws"}).Return([]config.RecommendationRecord{ + {ID: "lower-id", Provider: "aws", Service: "ec2", Region: "us-east-1", ResourceType: "m5.large", Term: 1, Payment: "all-upfront", Count: 1, UpfrontCost: 100}, + {ID: "upper-id", Provider: "AWS", Service: "ec2", Region: "us-east-1", ResourceType: "m5.large", Term: 1, Payment: "ALL-UPFRONT", Count: 1, UpfrontCost: 200}, + }, nil) + + handler := &Handler{config: mockStore} + _, err := handler.loadStoredRecommendationIndex(ctx, []config.RecommendationRecord{ + {Provider: "aws", Service: "ec2", Region: "us-east-1", ResourceType: "m5.large", Term: 1, Payment: "all-upfront"}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "more than one row for identity key") + _, isClient := IsClientError(err) + assert.False(t, isClient, "an index invariant violation is a server-side bug, not a client error") +} + +// TestRecIdentityKey_TupleSemantics pins the identity-tuple contract: +// nil and "" CloudAccountID collapse to the same key, provider/payment case +// is folded, and every other component change yields a different key. +func TestRecIdentityKey_TupleSemantics(t *testing.T) { + base := config.RecommendationRecord{ + Provider: "aws", Service: "ec2", Region: "us-east-1", ResourceType: "m5.large", + Engine: "mysql", Term: 3, Payment: "all-upfront", + } + + t.Run("nil and empty CloudAccountID collapse to the same key", func(t *testing.T) { + empty := "" + withNil := base + withEmpty := base + withEmpty.CloudAccountID = &empty + assert.Equal(t, recIdentityKey(&withNil), recIdentityKey(&withEmpty)) + }) + + t.Run("provider and payment case is folded", func(t *testing.T) { + upper := base + upper.Provider = "AWS" + upper.Payment = "ALL-UPFRONT" + assert.Equal(t, recIdentityKey(&base), recIdentityKey(&upper)) + }) + + t.Run("a different payment yields a different key", func(t *testing.T) { + other := base + other.Payment = "no-upfront" + assert.NotEqual(t, recIdentityKey(&base), recIdentityKey(&other)) + }) + + t.Run("a different term yields a different key", func(t *testing.T) { + other := base + other.Term = 1 + assert.NotEqual(t, recIdentityKey(&base), recIdentityKey(&other)) + }) + + t.Run("a different engine yields a different key", func(t *testing.T) { + other := base + other.Engine = "postgres" + assert.NotEqual(t, recIdentityKey(&base), recIdentityKey(&other)) + }) + + t.Run("a different account yields a different key", func(t *testing.T) { + acctA := "acct-a" + acctB := "acct-b" + withA := base + withA.CloudAccountID = &acctA + withB := base + withB.CloudAccountID = &acctB + assert.NotEqual(t, recIdentityKey(&withA), recIdentityKey(&withB)) + }) +}