fix(purchase): refuse a plan-less execution whose recs span cloud accounts

internal/api/handler_purchases.go

@@ -2297,11 +2297,11 @@ func normalizeCapacityPercent(execReq *ExecutePurchaseRequest) error {
 }
 
 // validateExecutePurchaseRecommendations runs the per-rec #643 boundary
-// validation over every rec in a web execute request, returning the first
-// failure. On success it also returns the payment-option coercions that
-// occurred (nil-free, in rec order) so the response can surface them to the
-// caller (#1503 follow-up). Extracted so validateExecutePurchaseRequest stays
-// under the gocyclo threshold.
+// validation over every rec in a web execute request, then the one-account-
+// per-batch rule (#1902), returning the first failure. On success it also
+// returns the payment-option coercions that occurred (nil-free, in rec order)
+// so the response can surface them to the caller (#1503 follow-up). Extracted
+// so validateExecutePurchaseRequest stays under the gocyclo threshold.
 func validateExecutePurchaseRecommendations(recs []config.RecommendationRecord) ([]PaymentAdjustment, error) {
 	var adjustments []PaymentAdjustment
 	for i := range recs {
@@ -2313,6 +2313,13 @@ func validateExecutePurchaseRecommendations(recs []config.RecommendationRecord)
 			adjustments = append(adjustments, *adjustment)
 		}
 	}
+	// One plan-less execution targets exactly one cloud account (#1902).
+	// Same rule the executor enforces in resolveSingleAccountProvider, applied
+	// here so the batch is refused before an execution row exists, an
+	// approval email goes out, or a History row lands as failed.
+	if _, err := purchase.SingleCloudAccountIDFromRecs(recs); err != nil {
+		return nil, NewClientError(400, err.Error())
+	}
 	return adjustments, nil
 }
 

internal/api/handler_purchases_test.go

@@ -4010,6 +4010,65 @@ func TestHandler_executePurchase_DirectExec_ExecuteOwn_Owner(t *testing.T) {
 	assert.Equal(t, true, resultMap["direct_execute"])
 }
 
+// TestHandler_executePurchase_MultiAccountBatch_Rejected is the #1902 boundary
+// guard: a web execute request whose recommendations span two cloud accounts
+// is refused with a 400 naming both accounts before an execution row is
+// persisted, in both submit modes. The executor enforces the same rule
+// (purchase.SingleCloudAccountIDFromRecs); this test pins the early refusal so
+// a multi-account batch never becomes a pending row that fails at approval.
+func TestHandler_executePurchase_MultiAccountBatch_Rejected(t *testing.T) {
+	const multiAccountBody = `{
+  "recommendations": [
+    {"id": "rec-a", "provider": "aws", "service": "ec2", "region": "us-east-1", "count": 1, "term": 1, "payment": "all-upfront", "upfront_cost": 500.0, "savings": 100.0, "selected": true, "cloud_account_id": "acct-a"},
+    {"id": "rec-b", "provider": "aws", "service": "ec2", "region": "us-east-1", "count": 1, "term": 1, "payment": "all-upfront", "upfront_cost": 700.0, "savings": 120.0, "selected": true, "cloud_account_id": "acct-b"}
+  ]%s
+}`
+	cases := []struct {
+		name      string
+		modeField string
+	}{
+		{name: "direct mode", modeField: `, "execute_mode": "direct"`},
+		{name: "approval mode", modeField: ``},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.Background()
+			mockStore := new(MockConfigStore)
+			mockAuth := new(MockAuthService)
+			mockPurchase := new(MockPurchaseManager)
+			t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+
+			session := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"}
+			mockAuth.On("ValidateSession", ctx, "admin-token").Return(session, nil)
+			mockAuth.On("HasPermissionAPI", ctx, session.UserID, "execute", "purchases").Return(true, nil)
+			mockAuth.allowConstraintChecks()
+			mockAuth.On("GetAllowedAccountsAPI", ctx, session.UserID).Return([]string{}, nil)
+			// Registered as optional so a pre-fix run (which persists the row,
+			// then direct-executes or emails) fails on the assertions below
+			// rather than on an unexpected-call panic.
+			mockAuth.On("HasPermissionAPI", ctx, session.UserID, "execute-any", "purchases").Return(true, nil).Maybe()
+			mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil).Maybe()
+			mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil).Maybe()
+			mockStore.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{}, 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 admin-token"},
+				Body:    fmt.Sprintf(multiAccountBody, tc.modeField),
+			}
+			_, 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, 400, ce.code)
+			assert.Contains(t, ce.Error(), "2 cloud accounts (acct-a, acct-b)")
+			mockStore.AssertNotCalled(t, "SavePurchaseExecution", mock.Anything, mock.Anything)
+			mockPurchase.AssertNotCalled(t, "ApproveAndExecute", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+		})
+	}
+}
+
 // TestHandler_executePurchase_DirectExec_FourEyesOn_DeniesSelfExecute is the
 // true end-to-end regression test for the HIGH finding on PR #1500's
 // adversarial review: execute_mode="direct" bypassed 4-eyes mode entirely

internal/purchase/execution.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -26,8 +27,11 @@ import (
 // When the plan has associated cloud accounts and a credential store is configured,
 // it fans out execution in parallel — one goroutine per account, each with its own
 // PurchaseExecution record tagged with cloud_account_id.
-// If no accounts are configured or no credential store is available, it falls back
-// to single-account execution using ambient credentials.
+// If no accounts are configured or no credential store is available, it takes the
+// single-account path, which resolves credentials from the recommendations' own
+// cloud account and refuses a batch spanning more than one (see #1902). Only a
+// batch whose selected recommendations carry no account at all falls back to
+// ambient credentials.
 // executePurchase runs the purchase for a single execution. When the plan has
 // associated cloud accounts it fans out via executeMultiAccount (which saves its
 // own per-account records); otherwise it runs the single-account path. The root
@@ -39,9 +43,10 @@ func (m *Manager) executePurchase(ctx context.Context, exec *config.PurchaseExec
 	// with no associated plan. PlanID is empty and the Postgres UUID
 	// column rejects "" with SQLSTATE 22P02, so skip the plan/accounts
 	// fetch entirely and synthesize a placeholder plan whose Name is the
-	// only field downstream history/notification code reads. By
-	// definition direct-execute purchases target a single account, so
-	// fall straight through to the legacy single-account path.
+	// only field downstream history/notification code reads. Direct-execute
+	// purchases target exactly one account: resolveSingleAccountProvider
+	// refuses a batch whose selected recs span accounts (#1902), so fall
+	// straight through to the single-account path.
 	var plan *config.PurchasePlan
 	if exec.PlanID == "" {
 		plan = &config.PurchasePlan{Name: "Direct purchase"}
@@ -351,9 +356,12 @@ func applyAccountOutcome(acctExec *config.PurchaseExecution, purchaseErrors []st
 // caller to fall back to the ambient AWS STS identity.
 //
 // The account is taken from exec.CloudAccountID when set (plan-with-single-
-// account executions), or derived from the shared cloud_account_id on the
-// recommendations when all selected recs agree on exactly one account (direct-
-// execute purchases where PlanID is empty and exec.CloudAccountID is nil).
+// account executions), or derived from the shared cloud_account_id of the
+// SELECTED recommendations (direct-execute purchases where PlanID is empty and
+// exec.CloudAccountID is nil). When the selected recs span more than one
+// account, or mix attributed and unattributed recs, this returns
+// errAmbiguousAccountScope: the single-account path cannot honor such a
+// batch and must never fall back to ambient credentials for it (#1902).
 //
 // A non-nil error means a target account was identified but could not be
 // resolved (lookup failed, the account does not exist, or credentials could
@@ -370,7 +378,11 @@ func (m *Manager) resolveSingleAccountProvider(ctx context.Context, exec *config
 
 	cloudAccountID := exec.CloudAccountID
 	if cloudAccountID == nil {
-		cloudAccountID = singleCloudAccountIDFromRecs(exec.Recommendations)
+		var scopeErr error
+		cloudAccountID, scopeErr = SingleCloudAccountIDFromRecs(exec.Recommendations)
+		if scopeErr != nil {
+			return nil, "", fmt.Errorf("execution %s: %w", exec.ExecutionID, scopeErr)
+		}
 	}
 	if cloudAccountID == nil {
 		return nil, "", nil
@@ -863,26 +875,49 @@ func indexKeys(idx []int) []string {
 	return out
 }
 
-// singleCloudAccountIDFromRecs returns the shared cloud_account_id when
-// all recommendations in the slice agree on exactly one non-empty account ID.
-// Returns nil when the slice is empty, all IDs are absent, or more than one
-// distinct ID is present (the caller falls back to ambient credentials in the
-// first two cases and to fan-out in the last).
-func singleCloudAccountIDFromRecs(recs []config.RecommendationRecord) *string {
+// errAmbiguousAccountScope is returned when the selected recommendations of a
+// plan-less execution do not resolve to exactly one cloud account. The
+// single-account path cannot honor such a batch: buying it under ambient
+// credentials (the pre-#1902 behavior) purchases every commitment in the
+// CUDly host account and stamps the ambient identity on history (#646).
+var errAmbiguousAccountScope = errors.New("selected recommendations do not resolve to a single cloud account")
+
+// SingleCloudAccountIDFromRecs returns the cloud_account_id shared by every
+// SELECTED recommendation, nil when no selected recommendation carries one
+// (the ambient single-account deployment), and errAmbiguousAccountScope when
+// the selected recs span more than one account or mix attributed and
+// unattributed entries (#1902). Only selected recs count: they are the recs
+// the money moves for (processPurchaseRecommendations buys selectedIndices).
+// Exported so the API boundary (validateExecutePurchaseRecommendations)
+// applies the identical rule before an execution row exists.
+func SingleCloudAccountIDFromRecs(recs []config.RecommendationRecord) (*string, error) {
+	var ids []string
 	var found *string
+	selected, unattributed := 0, 0
 	for i := range recs {
-		id := recs[i].CloudAccountID
-		if id == nil || *id == "" {
+		rec := &recs[i]
+		if !rec.Selected {
 			continue
 		}
-		if found == nil {
-			found = id
-		} else if *found != *id {
-			// More than one distinct account — not the single-account path.
-			return nil
+		selected++
+		if rec.CloudAccountID == nil || *rec.CloudAccountID == "" {
+			unattributed++
+			continue
+		}
+		if !slices.Contains(ids, *rec.CloudAccountID) {
+			ids = append(ids, *rec.CloudAccountID)
+			found = rec.CloudAccountID
 		}
 	}
-	return found
+	switch {
+	case len(ids) > 1:
+		return nil, fmt.Errorf("%w: %d selected recommendation(s) target %d cloud accounts (%s); submit one purchase per account",
+			errAmbiguousAccountScope, selected, len(ids), strings.Join(ids, ", "))
+	case len(ids) == 1 && unattributed > 0:
+		return nil, fmt.Errorf("%w: %d selected recommendation(s) carry no cloud_account_id while %d target account %s",
+			errAmbiguousAccountScope, unattributed, selected-unattributed, ids[0])
+	}
+	return found, nil
 }
 
 // savePurchaseHistory persists one purchase_history row for a successful

internal/purchase/execution_test.go

@@ -1579,66 +1579,104 @@ func TestExecutePurchase_SingleAccount_AccountNotFound(t *testing.T) {
 }
 
 // TestSingleCloudAccountIDFromRecs covers the helper that derives a shared
-// cloud_account_id from a recommendation slice.
+// cloud_account_id from the SELECTED recommendations of a plan-less
+// execution, and the #1902 ambiguous-scope errors it returns for the
+// multi-account and mixed attributed/unattributed shapes.
 func TestSingleCloudAccountIDFromRecs(t *testing.T) {
 	aid1 := "acct-1"
 	aid2 := "acct-2"
+	aid3 := "acct-3"
 
 	tests := []struct {
-		want *string
-		name string
-		recs []config.RecommendationRecord
+		want    *string
+		name    string
+		recs    []config.RecommendationRecord
+		wantErr bool
+		wantMsg string
 	}{
 		{
 			name: "empty slice returns nil",
 			recs: []config.RecommendationRecord{},
 			want: nil,
 		},
 		{
-			name: "all nil account IDs returns nil",
+			name: "all unattributed returns nil",
 			recs: []config.RecommendationRecord{
-				{CloudAccountID: nil},
-				{CloudAccountID: nil},
+				{Selected: true, CloudAccountID: nil},
+				{Selected: true, CloudAccountID: nil},
 			},
 			want: nil,
 		},
 		{
-			name: "single rec with account ID returns it",
+			name: "single attributed rec returns it",
 			recs: []config.RecommendationRecord{
-				{CloudAccountID: &aid1},
+				{Selected: true, CloudAccountID: &aid1},
 			},
 			want: &aid1,
 		},
 		{
-			name: "all recs share same account ID returns it",
+			name: "all recs share one account",
 			recs: []config.RecommendationRecord{
-				{CloudAccountID: &aid1},
-				{CloudAccountID: &aid1},
+				{Selected: true, CloudAccountID: &aid1},
+				{Selected: true, CloudAccountID: &aid1},
 			},
 			want: &aid1,
 		},
 		{
-			name: "mixed nil and same non-nil returns the non-nil ID",
+			name: "unselected rec from another account is ignored",
 			recs: []config.RecommendationRecord{
-				{CloudAccountID: nil},
-				{CloudAccountID: &aid1},
-				{CloudAccountID: &aid1},
+				{Selected: true, CloudAccountID: &aid1},
+				{Selected: false, CloudAccountID: &aid2},
 			},
 			want: &aid1,
 		},
 		{
-			name: "two distinct account IDs returns nil (multi-account, not this path)",
+			name: "only unselected recs returns nil",
 			recs: []config.RecommendationRecord{
-				{CloudAccountID: &aid1},
-				{CloudAccountID: &aid2},
+				{Selected: false, CloudAccountID: &aid1},
 			},
 			want: nil,
 		},
+		{
+			name: "two distinct accounts is an error",
+			recs: []config.RecommendationRecord{
+				{Selected: true, CloudAccountID: &aid1},
+				{Selected: true, CloudAccountID: &aid2},
+			},
+			wantErr: true,
+			wantMsg: "2 cloud accounts (acct-1, acct-2)",
+		},
+		{
+			name: "mixed attributed and unattributed is an error",
+			recs: []config.RecommendationRecord{
+				{Selected: true, CloudAccountID: nil},
+				{Selected: true, CloudAccountID: &aid1},
+			},
+			wantErr: true,
+			wantMsg: "carry no cloud_account_id",
+		},
+		{
+			name: "three distinct accounts lists all three in order",
+			recs: []config.RecommendationRecord{
+				{Selected: true, CloudAccountID: &aid1},
+				{Selected: true, CloudAccountID: &aid2},
+				{Selected: true, CloudAccountID: &aid3},
+			},
+			wantErr: true,
+			wantMsg: "(acct-1, acct-2, acct-3)",
+		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			got := singleCloudAccountIDFromRecs(tc.recs)
+			got, err := SingleCloudAccountIDFromRecs(tc.recs)
+			if tc.wantErr {
+				require.ErrorIs(t, err, errAmbiguousAccountScope)
+				assert.Contains(t, err.Error(), tc.wantMsg)
+				assert.Nil(t, got)
+				return
+			}
+			require.NoError(t, err)
 			if tc.want == nil {
 				assert.Nil(t, got)
 			} else {