fix(api): scope plans list, inventory filter and RI quote to allowed accounts

internal/api/handler_inventory.go

@@ -87,37 +87,27 @@ func (h *Handler) listActiveCommitments(ctx context.Context, req *events.LambdaF
 // #701/#498/#866 bug was that passing the UUID to the single-column
 // GetPurchaseHistory (account_id) matched nothing.
 //
-// When no explicit `account_id` is supplied, a restricted session's
-// allowed_accounts scope is pushed into the SQL read via
-// resolveAllowedAccountScope, the same contract as the dashboard KPI path
-// (issue #956): without it the store would read every tenant's active
-// commitments and only trim them in memory afterwards. A restricted session
-// whose allowed_accounts match no account resolves to the non-nil-but-empty
-// sentinel and short-circuits to an empty result without querying (an empty
-// scope sent to the store would match ALL rows, not none). Unrestricted /
-// admin sessions resolve to a nil scope and keep the all-accounts read. The
+// The filter is then narrowed to the session's allowed_accounts by
+// intersectAccountFilterScope, the same contract as the dashboard KPI path
+// (issues #956, #384): a restricted session's scope is pushed into the SQL
+// read, and an explicit account_id outside it is dropped rather than queried.
+// An empty intersection resolves to the non-nil-but-empty sentinel and
+// short-circuits to an empty result without querying (an empty scope sent to
+// the store would match ALL rows, not none). Unrestricted / admin sessions
+// keep the caller's filter, or the all-accounts read without one. The
 // in-memory filterPurchaseHistoryByAllowedAccounts in the callers still runs
-// afterwards; it is what trims an explicit account_id outside the session's
-// scope and serves as defense in depth for the no-param path.
+// afterwards as defense in depth.
 //
 // `provider` filtering is applied in-memory after the store read so the
 // record set is small enough that a post-read filter has negligible cost.
 func (h *Handler) fetchCommitmentRecords(ctx context.Context, asOf time.Time, session *Session, params map[string]string) ([]config.PurchaseHistoryRecord, error) {
-	var uuids []string
-	var externalIDsByProvider map[string][]string
-	if accountID := params["account_id"]; accountID != "" {
-		uuids, externalIDsByProvider = h.resolveSingleAccountFilterIDs(ctx, accountID)
-	} else {
-		var err error
-		uuids, externalIDsByProvider, err = h.resolveAllowedAccountScope(ctx, session)
-		if err != nil {
-			return nil, err
-		}
-		// Non-nil empty scope: restricted session with zero accessible
-		// accounts. Querying with it would match every row, so short-circuit.
-		if uuids != nil && len(uuids) == 0 && len(externalIDsByProvider) == 0 {
-			return nil, nil
-		}
+	filterUUIDs, filterExternalIDsByProvider := h.resolveSingleAccountFilterIDs(ctx, params["account_id"])
+	uuids, externalIDsByProvider, err := h.intersectAccountFilterScope(ctx, session, filterUUIDs, filterExternalIDsByProvider)
+	if err != nil {
+		return nil, err
+	}
+	if uuids != nil && len(uuids) == 0 && len(externalIDsByProvider) == 0 {
+		return nil, nil
 	}
 	rows, err := h.config.GetActivePurchaseHistory(ctx, asOf, uuids, externalIDsByProvider)
 	if err != nil {

internal/api/handler_plans.go

@@ -18,8 +18,8 @@ import (
 
 // Plans handlers.
 func (h *Handler) listPlans(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (*PlansResponse, error) {
-	// Require view:plans permission
-	if _, err := h.requirePermission(ctx, req, "view", "plans"); err != nil {
+	session, err := h.requirePermission(ctx, req, "view", "plans")
+	if err != nil {
 		return nil, err
 	}
 
@@ -30,7 +30,13 @@ func (h *Handler) listPlans(ctx context.Context, req *events.LambdaFunctionURLRe
 		return nil, NewClientError(400, err.Error())
 	}
 
-	filter := config.PurchasePlanFilter{AccountIDs: accountIDs}
+	filter, anyAccount, err := h.scopedPlanFilter(ctx, session, accountIDs)
+	if err != nil {
+		return nil, err
+	}
+	if !anyAccount {
+		return &PlansResponse{Plans: []PlanWithHealth{}}, nil
+	}
 	plans, err := h.config.ListPurchasePlans(ctx, filter)
 	if err != nil {
 		return nil, err
@@ -47,6 +53,23 @@ func (h *Handler) listPlans(ctx context.Context, req *events.LambdaFunctionURLRe
 	return &PlansResponse{Plans: h.attachPlanHealth(ctx, plans, now)}, nil
 }
 
+// scopedPlanFilter narrows account_ids to the session's allowed_accounts. Only
+// unrestricted callers get unassigned plans, matching requirePlanAccess.
+func (h *Handler) scopedPlanFilter(ctx context.Context, session *Session, accountIDs []string) (filter config.PurchasePlanFilter, anyAccount bool, err error) {
+	scope, err := h.getAccountScope(ctx, session)
+	if err != nil {
+		return filter, false, fmt.Errorf("failed to get allowed accounts: %w", err)
+	}
+	if scope.AllowsAll() {
+		return config.PurchasePlanFilter{AccountIDs: accountIDs, IncludeUnassigned: true}, true, nil
+	}
+	allowed, _, err := h.intersectAccountFilterScope(ctx, session, accountIDs, nil)
+	if err != nil {
+		return filter, false, err
+	}
+	return config.PurchasePlanFilter{AccountIDs: allowed}, len(allowed) > 0, nil
+}
+
 // attachPlanHealth computes the per-plan health-score badge (issue #340
 // follow-up) for every plan in the list, from exact per-plan execution
 // counts over the trailing planHealthLookbackDays

internal/api/handler_plans_test.go

@@ -33,7 +33,7 @@ func TestHandler_listPlans(t *testing.T) {
 
 	mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
 	mockAuth.grantAdmin()
-	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil)
+	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{IncludeUnassigned: true}).Return(plans, nil)
 	// listPlans now also counts each plan's failed/canceled executions to
 	// compute its health-score badge (issue #340 follow-up).
 	mockStore.On("CountExecutionsByPlanAndStatus", ctx, planHealthExecutionStatuses, mock.AnythingOfType("time.Time")).
@@ -73,7 +73,7 @@ func TestHandler_listPlans_AccountIDsFilter(t *testing.T) {
 	}
 
 	accountID := "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"
-	expectedFilter := config.PurchasePlanFilter{AccountIDs: []string{accountID}}
+	expectedFilter := config.PurchasePlanFilter{AccountIDs: []string{accountID}, IncludeUnassigned: true}
 
 	mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
 	mockAuth.grantAdmin()
@@ -117,7 +117,7 @@ func TestHandler_listPlans_HealthScoreReflectsFailedExecutions(t *testing.T) {
 		{ID: troubledPlanID, Name: "Troubled Plan", Enabled: true},
 		{ID: otherPlanID, Name: "Other Plan", Enabled: true},
 	}
-	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil)
+	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{IncludeUnassigned: true}).Return(plans, nil)
 	mockStore.On("CountExecutionsByPlanAndStatus", ctx, planHealthExecutionStatuses, mock.AnythingOfType("time.Time")).
 		Return(map[string]config.ExecutionStatusCounts{
 			troubledPlanID: {"failed": 2},
@@ -169,7 +169,7 @@ func TestHandler_listPlans_HealthScoreUnknownOnCountsFetchError(t *testing.T) {
 	plans := []config.PurchasePlan{
 		{ID: "11111111-1111-1111-1111-111111111111", Name: "Some Plan", Enabled: true},
 	}
-	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil)
+	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{IncludeUnassigned: true}).Return(plans, nil)
 	mockStore.On("CountExecutionsByPlanAndStatus", ctx, planHealthExecutionStatuses, mock.AnythingOfType("time.Time")).
 		Return(nil, errors.New("db unavailable"))
 
@@ -206,7 +206,7 @@ func TestHandler_listPlans_HealthUsesExactCountsNotTruncatedPage(t *testing.T) {
 	mockAuth.grantAdmin()
 
 	planID := "11111111-1111-1111-1111-111111111111"
-	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).
+	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{IncludeUnassigned: true}).
 		Return([]config.PurchasePlan{{ID: planID, Name: "Old Failures", Enabled: true}}, nil)
 	mockStore.On("CountExecutionsByPlanAndStatus", ctx, planHealthExecutionStatuses, mock.AnythingOfType("time.Time")).
 		Return(map[string]config.ExecutionStatusCounts{planID: {"failed": 3}}, nil)
@@ -239,7 +239,7 @@ func TestHandler_listPlans_HealthCountsUseTheLookbackWindow(t *testing.T) {
 	adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"}
 	mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
 	mockAuth.grantAdmin()
-	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).
+	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{IncludeUnassigned: true}).
 		Return([]config.PurchasePlan{{ID: "11111111-1111-1111-1111-111111111111", Enabled: true}}, nil)
 
 	var gotSince time.Time
@@ -273,7 +273,7 @@ func TestHandler_listPlans_EmptyListSkipsCountsQuery(t *testing.T) {
 	adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"}
 	mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
 	mockAuth.grantAdmin()
-	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).
+	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{IncludeUnassigned: true}).
 		Return([]config.PurchasePlan{}, nil)
 
 	handler := &Handler{config: mockStore, auth: mockAuth}
@@ -1732,7 +1732,7 @@ func TestHandler_listPlans_StuckRampFetchFailureLeavesHealthUnknown(t *testing.T
 	mockAuth.grantAdmin()
 
 	planID := "11111111-1111-1111-1111-111111111111"
-	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).
+	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{IncludeUnassigned: true}).
 		Return([]config.PurchasePlan{{ID: planID, Name: "Ramping Plan", Enabled: true}}, nil)
 	mockStore.On("CountExecutionsByPlanAndStatus", ctx, planHealthExecutionStatuses, mock.AnythingOfType("time.Time")).
 		Return(map[string]config.ExecutionStatusCounts{}, nil)
@@ -1768,7 +1768,7 @@ func TestHandler_listPlans_StuckRampProducesTheBlockedFactor(t *testing.T) {
 		Type: "weekly", PercentPerStep: 25, StepIntervalDays: 7,
 		CurrentStep: 2, TotalSteps: 4, StartDate: time.Now().AddDate(0, 0, -28),
 	}
-	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).
+	mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{IncludeUnassigned: true}).
 		Return([]config.PurchasePlan{
 			{ID: planID, Name: "Stuck Plan", Enabled: true, RampSchedule: ramp},
 			{ID: quietPlanID, Name: "Late Plan", Enabled: true, RampSchedule: ramp},

internal/api/handler_ri_exchange.go

@@ -1715,7 +1715,13 @@ func validateExchangeRIIDs(riIDs []string) error {
 
 // getExchangeQuote gets a quote for an RI exchange.
 func (h *Handler) getExchangeQuote(ctx context.Context, req *events.LambdaFunctionURLRequest) (any, error) {
-	if _, err := h.requirePermission(ctx, req, "view", "purchases"); err != nil {
+	session, permErr := h.requirePermission(ctx, req, "view", "purchases")
+	if permErr != nil {
+		return nil, permErr
+	}
+	// A quote reveals the deployment account's RI ids and prices, so it takes
+	// the same session-scope gate as executeExchange (issue #384).
+	if err := h.requireReshapeAccountScope(ctx, session); err != nil {
 		return nil, err
 	}
 

internal/api/tenant_scope_sweep_test.go

@@ -0,0 +1,198 @@
+package api
+
+import (
+	"context"
+	"errors"
+	"slices"
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/aws/aws-lambda-go/events"
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+// Account-scope regression tests for GET /api/plans, the inventory commitment
+// read and the RI-exchange quote (issues #29, #384).
+
+const (
+	planInScope    = "a1111111-1111-4111-8111-111111111111"
+	planOutOfScope = "a2222222-2222-4222-8222-222222222222"
+	planUnassigned = "a3333333-3333-4333-8333-333333333333"
+	planBothScopes = "a4444444-4444-4444-8444-444444444444"
+)
+
+// planListStore applies buildListPlansQuery's predicate, so a wrong filter
+// fails by the plans returned rather than by an unstubbed-call panic.
+type planListStore struct {
+	*MockConfigStore
+	plans    []config.PurchasePlan
+	accounts map[string][]string
+	filters  []config.PurchasePlanFilter
+}
+
+func (s *planListStore) ListPurchasePlans(_ context.Context, f config.PurchasePlanFilter) ([]config.PurchasePlan, error) {
+	s.filters = append(s.filters, f)
+	out := []config.PurchasePlan{}
+	for _, p := range s.plans {
+		accts := s.accounts[p.ID]
+		matches := slices.ContainsFunc(accts, func(a string) bool { return slices.Contains(f.AccountIDs, a) })
+		if len(f.AccountIDs) == 0 || matches || (f.IncludeUnassigned && len(accts) == 0) {
+			out = append(out, p)
+		}
+	}
+	return out, nil
+}
+
+func newPlanListStore(base *MockConfigStore) *planListStore {
+	base.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) {
+		return []config.CloudAccount{inScopeAccount(), outOfScopeAccount()}, nil
+	}
+	base.On("CountExecutionsByPlanAndStatus", mock.Anything, mock.Anything, mock.Anything).
+		Return(map[string]config.ExecutionStatusCounts{}, nil).Maybe()
+	return &planListStore{
+		MockConfigStore: base,
+		plans: []config.PurchasePlan{
+			{ID: planInScope, Name: "in scope"},
+			{ID: planOutOfScope, Name: "other tenant"},
+			{ID: planUnassigned, Name: "unassigned"},
+			{ID: planBothScopes, Name: "shared"},
+		},
+		accounts: map[string][]string{
+			planInScope:    {scopedInAccount},
+			planOutOfScope: {scopedOutAccount},
+			planBothScopes: {scopedInAccount, scopedOutAccount},
+		},
+	}
+}
+
+func listPlanIDs(t *testing.T, h *Handler, params map[string]string) []string {
+	t.Helper()
+	resp, err := h.listPlans(context.Background(), scopedRequest(""), params)
+	require.NoError(t, err)
+	ids := make([]string, 0, len(resp.Plans))
+	for _, p := range resp.Plans {
+		ids = append(ids, p.ID)
+	}
+	slices.Sort(ids)
+	return ids
+}
+
+func TestListPlans_ScopedCallerSeesOnlyInScopePlans(t *testing.T) {
+	h, base := scopedHandler(t, scopedInAccount)
+	h.config = newPlanListStore(base)
+
+	assert.Equal(t, []string{planInScope, planBothScopes}, listPlanIDs(t, h, map[string]string{}),
+		"a scoped caller must see neither another tenant's plans nor unassigned plans")
+}
+
+func TestListPlans_ScopedCallerOutOfScopeFilterReturnsNothing(t *testing.T) {
+	h, base := scopedHandler(t, scopedInAccount)
+	store := newPlanListStore(base)
+	h.config = store
+
+	ids := listPlanIDs(t, h, map[string]string{"account_ids": scopedOutAccount})
+
+	assert.Empty(t, ids, "account_ids naming an out-of-scope account must not narrow the list to that tenant")
+	assert.Empty(t, store.filters, "an empty intersection must not reach the store, where no filter means all plans")
+}
+
+func TestListPlans_UnrestrictedCallerStillSeesUnassignedPlans(t *testing.T) {
+	h, base := scopedHandler(t)
+	h.config = newPlanListStore(base)
+
+	assert.Equal(t, []string{planInScope, planUnassigned, planBothScopes},
+		listPlanIDs(t, h, map[string]string{"account_ids": scopedInAccount}))
+}
+
+func TestListPlans_ScopeLookupErrorFailsClosed(t *testing.T) {
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	mockAuth.On("ValidateSession", ctx, scopedToken).Return(&Session{UserID: scopedUserID}, nil)
+	mockAuth.On("HasPermissionAPI", ctx, scopedUserID, "view", "plans").Return(true, nil)
+	mockAuth.On("GetAllowedAccountsAPI", ctx, scopedUserID).Return(nil, errors.New("auth store unavailable"))
+	store := newPlanListStore(new(MockConfigStore))
+	h := &Handler{auth: mockAuth, config: store}
+
+	_, err := h.listPlans(ctx, scopedRequest(""), map[string]string{})
+
+	require.Error(t, err)
+	_, isClient := IsClientError(err)
+	assert.False(t, isClient, "a scope-lookup failure must surface as a server error, got %v", err)
+	assert.Empty(t, store.filters)
+}
+
+func TestListActiveCommitments_ScopedCallerOutOfScopeAccountFilterSkipsStore(t *testing.T) {
+	h, store := scopedHandler(t, scopedInAccount)
+	store.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) {
+		return []config.CloudAccount{inScopeAccount(), outOfScopeAccount()}, nil
+	}
+	store.On("GetActivePurchaseHistory", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+		Return([]config.PurchaseHistoryRecord{}, nil).Maybe()
+
+	_, err := h.listActiveCommitments(context.Background(), scopedRequest(""), map[string]string{"account_id": scopedOutAccount})
+	require.NoError(t, err)
+
+	store.AssertNotCalled(t, "GetActivePurchaseHistory", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+}
+
+func TestListActiveCommitments_ScopedCallerInScopeAccountFilterReachesStore(t *testing.T) {
+	h, store := scopedHandler(t, scopedInAccount)
+	store.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) {
+		return []config.CloudAccount{inScopeAccount(), outOfScopeAccount()}, nil
+	}
+	store.On("GetActivePurchaseHistory", mock.Anything, mock.Anything, []string{scopedInAccount}, map[string][]string(nil)).
+		Return([]config.PurchaseHistoryRecord{}, nil).Once()
+
+	_, err := h.listActiveCommitments(context.Background(), scopedRequest(""), map[string]string{"account_id": scopedInAccount})
+	require.NoError(t, err)
+	store.AssertExpectations(t)
+}
+
+// quoteHandler scopes the caller to scopedInAccount; the failing credentials
+// keep a request that slips past the gate from calling EC2.
+func quoteHandler(t *testing.T, deploymentAccount string) *Handler {
+	h, store := scopedHandler(t, scopedInAccount)
+	store.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) {
+		return []config.CloudAccount{inScopeAccount(), outOfScopeAccount()}, nil
+	}
+	h.reshapeAccountResolver = func(_ context.Context) (string, error) { return deploymentAccount, nil }
+	h.awsCfgOnce.Do(func() {
+		h.awsCfg = aws.Config{
+			Region: "eu-west-1",
+			Credentials: aws.CredentialsProviderFunc(func(context.Context) (aws.Credentials, error) {
+				return aws.Credentials{}, errors.New("test: no AWS credentials")
+			}),
+		}
+	})
+	return h
+}
+
+const quoteBody = `{"ri_ids":["ri-123"],"target_offering_id":"4b2293b4-5fbc-4017-9c75-d5a9d3aa8c91","target_count":1}`
+
+func TestGetExchangeQuote_OutOfScopeDeploymentAccountReturns403(t *testing.T) {
+	h := quoteHandler(t, scopedOutAccount)
+
+	_, err := h.getExchangeQuote(context.Background(), &events.LambdaFunctionURLRequest{
+		Headers: scopedRequest("").Headers, Body: quoteBody,
+	})
+
+	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(), "allowed accounts")
+}
+
+func TestGetExchangeQuote_InScopeDeploymentAccountPassesGate(t *testing.T) {
+	h := quoteHandler(t, scopedInAccount)
+
+	_, err := h.getExchangeQuote(context.Background(), &events.LambdaFunctionURLRequest{
+		Headers: scopedRequest("").Headers, Body: quoteBody,
+	})
+
+	require.Error(t, err, "the test AWS credentials fail, so the quote itself cannot succeed")
+	assert.Contains(t, err.Error(), "exchange quote failed", "an in-scope caller must reach the AWS quote call")
+}