From af8bff696607f131e57c46459fe41a24f5f62935 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 3 Aug 2026 23:05:41 +0200 Subject: [PATCH 1/3] fix(api): scope Azure exchangeable-RI listing to session's allowed accounts listExchangeableAzureRIs enumerated every reservation the deployment's Azure credential could read across the whole tenant, regardless of the caller's allowed_accounts scope. A caller with view:purchases saw every subscription's reservations, including ones outside their scope. Narrow the GET listing the same way the POST siblings (compatible- offerings, execute) already narrow their own scope: - subscription_id supplied: requireAzureSubscriptionScope gates access to that subscription (errNotFound, matching the POST siblings so a scoped caller cannot probe which subscriptions exist), then the listing is narrowed to that subscription's own rows via BillingScopeID. - No subscription_id: the listing is narrowed to whatever the session's allowed_accounts scope covers, mirroring filterDashboardRecommendations. Unrestricted/admin sessions pass through unchanged. Rows with no BillingScopeID, or one matching no registered CloudAccount, are dropped rather than kept, so an unresolvable scope never degrades to "show everything" and the two empty-result cases stay indistinguishable. Split the scope filter into azureScopeIndex and filterReservationsByScopeIndex to keep filterAzureReservationsByScope under the project's gocyclo threshold. Adds regression tests covering: cross-subscription filtering with no subscription_id, the two indistinguishable-empty cases, and the out-of-scope subscription_id 404 shape. Closes #1656 --- internal/api/handler_ri_exchange.go | 167 ++++++++++++++++++++++- internal/api/handler_ri_exchange_test.go | 144 +++++++++++++++++++ 2 files changed, 305 insertions(+), 6 deletions(-) diff --git a/internal/api/handler_ri_exchange.go b/internal/api/handler_ri_exchange.go index 265859ea4..b03d8db50 100644 --- a/internal/api/handler_ri_exchange.go +++ b/internal/api/handler_ri_exchange.go @@ -232,21 +232,59 @@ func (h *Handler) buildAzureExchangeClient(ctx context.Context, subscriptionID s return azurecompute.NewClient(cred, subscriptionID, ""), nil } -// listExchangeableAzureRIs returns all active Azure VM reservations that are +// listExchangeableAzureRIs returns active Azure VM reservations that are // eligible for the cross-SKU/cross-region exchange flow (InstanceFlexibility // == On, ProvisioningState == Succeeded). Requires "view:purchases" permission. // -// The optional ?subscription_id= query parameter scopes the credential lookup -// to the matching registered CloudAccount. When no Azure account is configured -// for the requested subscription (or no subscription is specified and none are -// registered), the handler returns an empty reservations list rather than a 500. +// ListExchangeableReservations is tenant-wide by design (see its doc +// comment: "the Azure Capacity exchange API operates on reservation order +// IDs which span subscriptions"), and that breadth does not change just +// because ?subscription_id= was supplied -- the parameter only picks which +// registered CloudAccount's credentials authenticate the call, not which +// rows come back. Left unfiltered, every caller with "view:purchases" saw +// every reservation the deployment's Azure credential could read, including +// subscriptions they are not scoped to (issue #1656). Both POST siblings on +// this resource deliberately withhold that same breadth (requireAzureSubscriptionScope +// + requireAzureSourceOwnership, both indistinguishable-denial), so this GET +// narrows the same way: +// +// - ?subscription_id= supplied: requireAzureSubscriptionScope gates +// whether the session may use that subscription at all (errNotFound, +// not 403, matching the POST siblings so a scoped caller cannot probe +// which subscriptions exist). The listing is then narrowed to that +// subscription's own rows via BillingScopeID -- the same discriminator +// requireAzureSourceOwnership uses -- since a reservation billed +// elsewhere could never be named as a valid exchange source against +// this subscription anyway. +// - No subscription_id: the listing is narrowed to whatever the session's +// allowed_accounts scope covers (filterAzureReservationsByScope). +// Unrestricted/admin sessions see everything, matching every other +// listing endpoint in this package. +// +// Rows that cannot be attributed to a registered CloudAccount (no +// BillingScopeID, or one that matches no CloudAccount) are dropped rather +// than kept, so an unresolvable scope never degrades to "show everything". +// That also keeps the two empty-result cases indistinguishable: a scoped +// caller whose own subscriptions hold zero exchangeable reservations sees +// the same empty list as one whose rows could not be attributed. +// +// When no Azure account is configured for the requested subscription (or no +// subscription is specified and none are registered), the handler returns an +// empty reservations list rather than a 500. func (h *Handler) listExchangeableAzureRIs(ctx context.Context, req *events.LambdaFunctionURLRequest) (any, error) { - if _, err := h.requirePermission(ctx, req, "view", "purchases"); err != nil { + session, err := h.requirePermission(ctx, req, "view", "purchases") + if err != nil { return nil, err } subscriptionID := req.QueryStringParameters["subscription_id"] + if subscriptionID != "" { + if scopeErr := h.requireAzureSubscriptionScope(ctx, session, subscriptionID); scopeErr != nil { + return nil, scopeErr + } + } + client, err := h.buildAzureExchangeClient(ctx, subscriptionID) if err != nil { return nil, fmt.Errorf("failed to build Azure exchange client: %w", err) @@ -263,9 +301,126 @@ func (h *Handler) listExchangeableAzureRIs(ctx context.Context, req *events.Lamb return nil, fmt.Errorf("failed to list exchangeable Azure reservations: %w", err) } + if subscriptionID != "" { + reservations = filterAzureReservationsBySubscription(reservations, subscriptionID) + } else { + reservations, err = h.filterAzureReservationsByScope(ctx, session, reservations) + if err != nil { + return nil, err + } + } + return &ExchangeableAzureRIsResponse{Reservations: reservations}, nil } +// filterAzureReservationsBySubscription narrows a tenant-wide reservation +// listing down to the rows billed to subscriptionID, using the same +// BillingScopeID discriminator requireAzureSourceOwnership keys on (issue +// #1527). Reservations with no BillingScopeID, or one billed to a different +// subscription, are dropped. +// +// Called only after requireAzureSubscriptionScope has cleared the caller for +// subscriptionID; a reservation billed elsewhere could never be named as a +// valid exchange source against this subscription anyway +// (requireAzureSourceOwnership refuses cross-subscription sources), so +// returning the wider tenant listing here would just be the enumeration leak +// issue #1656 describes. +func filterAzureReservationsBySubscription(reservations []azurecompute.ExchangeableReservation, subscriptionID string) []azurecompute.ExchangeableReservation { + scope := azureBillingScopeID(subscriptionID) + filtered := make([]azurecompute.ExchangeableReservation, 0, len(reservations)) + for i := range reservations { + if strings.EqualFold(reservations[i].BillingScopeID, scope) { + filtered = append(filtered, reservations[i]) + } + } + return filtered +} + +// filterAzureReservationsByScope narrows a tenant-wide Azure reservation +// listing down to the rows billed to a subscription the session's +// allowed_accounts scope covers. Mirrors filterDashboardRecommendations +// (handler_dashboard.go): unrestricted/admin sessions pass through +// unchanged; a restricted session keeps only the rows it can resolve back to +// one of its allowed CloudAccounts. +// +// Each reservation's BillingScopeID ("/subscriptions/{subscriptionID}") is +// the same ownership signal requireAzureSourceOwnership keys on (issue +// #1527) -- the subscription actually charged, which is the correct +// discriminator even for AppliedScopeType == Shared. +// +// Fails closed: a reservation with no BillingScopeID, or one that matches no +// registered Azure CloudAccount, is dropped rather than kept. Counts, not +// the dropped reservation/billing-scope identifiers, are safe to log -- +// logging the identifiers would just relocate the disclosure this filter +// exists to prevent. +func (h *Handler) filterAzureReservationsByScope(ctx context.Context, session *Session, reservations []azurecompute.ExchangeableReservation) ([]azurecompute.ExchangeableReservation, error) { + allowed, err := h.getAllowedAccounts(ctx, session) + if err != nil { + return nil, fmt.Errorf("failed to get allowed accounts: %w", err) + } + if auth.IsUnrestrictedAccess(allowed) { + return reservations, nil + } + + provider := "azure" + accounts, err := h.config.ListCloudAccounts(ctx, config.CloudAccountFilter{Provider: &provider}) + if err != nil { + return nil, fmt.Errorf("failed to list cloud accounts: %w", err) + } + + filtered := filterReservationsByScopeIndex(reservations, azureScopeIndex(accounts), allowed) + // Counts, not the dropped reservation/billing-scope identifiers, are safe + // to log -- logging the identifiers would just relocate the disclosure + // this filter exists to prevent. + if dropped := len(reservations) - len(filtered); dropped > 0 { + logging.Debugf("azure exchange listing: filtered %d of %d tenant-wide reservations outside session scope", dropped, len(reservations)) + } + return filtered, nil +} + +// azureScopeIndex builds a lookup from ARM billing scope (lower-cased) to the +// registered Azure CloudAccount that owns it, so filterAzureReservationsByScope +// can resolve each reservation's BillingScopeID straight to its CloudAccount +// without re-deriving azureBillingScopeID per row. Accounts with no +// ExternalID are skipped -- they cannot own any BillingScopeID. +// +// Split out of filterAzureReservationsByScope, together with +// filterReservationsByScopeIndex below, to stay under the project's gocyclo +// threshold (mirrors validateAzureOfferingsBody's precedent). +func azureScopeIndex(accounts []config.CloudAccount) map[string]config.CloudAccount { + index := make(map[string]config.CloudAccount, len(accounts)) + for i := range accounts { + if accounts[i].ExternalID == "" { + continue + } + index[strings.ToLower(azureBillingScopeID(accounts[i].ExternalID))] = accounts[i] + } + return index +} + +// filterReservationsByScopeIndex narrows reservations down to the rows whose +// BillingScopeID resolves, via scopeToAccount, to a CloudAccount the allowed +// list covers (auth.MatchesAccount). Fails closed: a reservation with no +// BillingScopeID, or one that resolves to no registered CloudAccount, is +// dropped rather than kept. +func filterReservationsByScopeIndex(reservations []azurecompute.ExchangeableReservation, scopeToAccount map[string]config.CloudAccount, allowed []string) []azurecompute.ExchangeableReservation { + filtered := make([]azurecompute.ExchangeableReservation, 0, len(reservations)) + for i := range reservations { + r := reservations[i] + if r.BillingScopeID == "" { + continue + } + account, ok := scopeToAccount[strings.ToLower(r.BillingScopeID)] + if !ok { + continue + } + if auth.MatchesAccount(allowed, account.ID, account.Name) { + filtered = append(filtered, r) + } + } + return filtered +} + // maxAzureExchangeItems caps the number of sources/targets accepted per // Azure exchange request, guarding against an oversized request fanning out // into an enormous CalculateExchange payload. diff --git a/internal/api/handler_ri_exchange_test.go b/internal/api/handler_ri_exchange_test.go index 9c8c8b126..eee7cbb73 100644 --- a/internal/api/handler_ri_exchange_test.go +++ b/internal/api/handler_ri_exchange_test.go @@ -822,6 +822,150 @@ func TestListExchangeableAzureRIs_SubscriptionIDPassedToFactory(t *testing.T) { assert.Equal(t, "sub-abc", capturedSubID) } +// --- allowed_accounts scoping on the tenant-wide listing (issue #1656) --- +// +// ListExchangeableReservations returns every reservation the deployment's +// Azure credential can read, tenant-wide. Before this fix, a caller with +// "view:purchases" saw every subscription's reservations regardless of their +// own allowed_accounts scope. These tests exercise the fix directly: a +// scoped session must see only the rows billed to a subscription it is +// scoped to, both with and without ?subscription_id=. + +// TestListExchangeableAzureRIs_ScopedFiltersToOwnSubscription is the core +// regression: a session scoped to account A's subscription, calling the +// listing with NO subscription_id filter, must receive only account A's +// reservation. Account B's reservation -- present in the same tenant-wide +// listing -- must not appear anywhere in the response. +func TestListExchangeableAzureRIs_ScopedFiltersToOwnSubscription(t *testing.T) { + ctx := context.Background() + mockAuth := scopedAzureAuth(t, "view", "purchases", []string{"acct-a"}) + store := &MockConfigStore{} + store.ListCloudAccountsFn = func(_ context.Context, filter config.CloudAccountFilter) ([]config.CloudAccount, error) { + require.NotNil(t, filter.Provider, "the scope filter must narrow to azure accounts") + require.Equal(t, "azure", *filter.Provider) + return []config.CloudAccount{ + {ID: "acct-a", Name: "A Team", Provider: "azure", ExternalID: "sub-a"}, + {ID: "acct-b", Name: "B Team", Provider: "azure", ExternalID: "sub-b"}, + }, nil + } + stub := &stubAzureExchangeClient{reservations: []azurecompute.ExchangeableReservation{ + {ReservationID: "res-a", BillingScopeID: "/subscriptions/sub-a", SKU: "Standard_D2s_v3", Quantity: 1, Region: "eastus"}, + {ReservationID: "res-b", BillingScopeID: "/subscriptions/sub-b", SKU: "Standard_D4s_v3", Quantity: 3, Region: "westeurope"}, + }} + + h := &Handler{ + auth: mockAuth, + config: store, + azureExchangeFactory: func(_ string) azureExchangeClient { return stub }, + } + + res, err := h.listExchangeableAzureRIs(ctx, &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"authorization": "Bearer tok"}, + }) + require.NoError(t, err) + resp, ok := res.(*ExchangeableAzureRIsResponse) + require.True(t, ok) + require.Len(t, resp.Reservations, 1, "only the scoped subscription's reservation must be returned") + assert.Equal(t, "res-a", resp.Reservations[0].ReservationID) + for _, r := range resp.Reservations { + assert.NotEqual(t, "res-b", r.ReservationID, "account B's reservation must never appear in a scoped response") + assert.NotContains(t, r.BillingScopeID, "sub-b", "account B's billing scope must not leak into a scoped response") + } +} + +// TestListExchangeableAzureRIs_ScopedEmptyCasesIndistinguishable asserts +// that a scoped caller cannot tell "my subscriptions genuinely have zero +// exchangeable reservations" apart from "the tenant-wide listing had rows, +// but none could be attributed to a registered account I'm scoped to" -- +// both must render as the same empty list, per the doc comment on +// listExchangeableAzureRIs: an unresolvable scope must never degrade to +// "show everything", and must not leak its distinctness either. +func TestListExchangeableAzureRIs_ScopedEmptyCasesIndistinguishable(t *testing.T) { + ctx := context.Background() + + // Case 1: the scoped account (A) is registered, but the only tenant-wide + // reservation belongs to a different, unscoped subscription (B). After + // filtering, account A itself simply has zero reservations. + zeroOwnAuth := scopedAzureAuth(t, "view", "purchases", []string{"acct-a"}) + zeroOwnStore := &MockConfigStore{} + zeroOwnStore.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) { + return []config.CloudAccount{ + {ID: "acct-a", Name: "A Team", Provider: "azure", ExternalID: "sub-a"}, + {ID: "acct-b", Name: "B Team", Provider: "azure", ExternalID: "sub-b"}, + }, nil + } + zeroOwnStub := &stubAzureExchangeClient{reservations: []azurecompute.ExchangeableReservation{ + {ReservationID: "res-b", BillingScopeID: "/subscriptions/sub-b", SKU: "Standard_D4s_v3", Quantity: 1, Region: "eastus"}, + }} + hZeroOwn := &Handler{ + auth: zeroOwnAuth, + config: zeroOwnStore, + azureExchangeFactory: func(_ string) azureExchangeClient { return zeroOwnStub }, + } + zeroOwnRes, zeroOwnErr := hZeroOwn.listExchangeableAzureRIs(ctx, &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"authorization": "Bearer tok"}, + }) + + // Case 2: the scoped account (A) is registered, and the tenant-wide + // listing has a row, but that row cannot be attributed to ANY registered + // CloudAccount (no matching BillingScopeID) -- e.g. a reservation billed + // to a subscription that was never onboarded. + unattributedAuth := scopedAzureAuth(t, "view", "purchases", []string{"acct-a"}) + unattributedStore := &MockConfigStore{} + unattributedStore.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) { + return []config.CloudAccount{ + {ID: "acct-a", Name: "A Team", Provider: "azure", ExternalID: "sub-a"}, + }, nil + } + unattributedStub := &stubAzureExchangeClient{reservations: []azurecompute.ExchangeableReservation{ + {ReservationID: "res-orphan", BillingScopeID: "/subscriptions/sub-unregistered", SKU: "Standard_D4s_v3", Quantity: 1, Region: "eastus"}, + }} + hUnattributed := &Handler{ + auth: unattributedAuth, + config: unattributedStore, + azureExchangeFactory: func(_ string) azureExchangeClient { return unattributedStub }, + } + unattributedRes, unattributedErr := hUnattributed.listExchangeableAzureRIs(ctx, &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"authorization": "Bearer tok"}, + }) + + require.NoError(t, zeroOwnErr) + require.NoError(t, unattributedErr) + zeroOwnResp, ok := zeroOwnRes.(*ExchangeableAzureRIsResponse) + require.True(t, ok) + unattributedResp, ok := unattributedRes.(*ExchangeableAzureRIsResponse) + require.True(t, ok) + assert.Empty(t, zeroOwnResp.Reservations) + assert.Empty(t, unattributedResp.Reservations) + assert.Equal(t, zeroOwnResp.Reservations, unattributedResp.Reservations, + "a scoped caller with genuinely zero reservations and one whose rows could not be attributed must render identically") +} + +// TestListExchangeableAzureRIs_SubscriptionIDOutOfScope asserts the +// ?subscription_id= path returns the same not-found shape (errNotFound) the +// POST siblings (getAzureCompatibleOfferings, executeAzureExchange) use for +// an out-of-scope subscription, so a scoped caller cannot use the listing +// endpoint to probe which subscriptions exist outside their scope. +func TestListExchangeableAzureRIs_SubscriptionIDOutOfScope(t *testing.T) { + ctx := context.Background() + opsClient := new(mockAzureExchangeOpsClient) + ownsAzureSource(opsClient) // Maybe(): must never actually be called + t.Cleanup(func() { opsClient.AssertExpectations(t) }) + + h := &Handler{ + auth: scopedAzureAuth(t, "view", "purchases", []string{"acct-mine"}), + config: scopedAzureStore(), // only registered account is acct-other + azureExchangeFactory: func(_ string) azureExchangeClient { return opsClient }, + } + + _, err := h.listExchangeableAzureRIs(ctx, &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"authorization": "Bearer tok"}, + QueryStringParameters: map[string]string{"subscription_id": "sub-1"}, + }) + require.Error(t, err, "a scoped session must not list an out-of-scope subscription's reservations") + assert.ErrorIs(t, err, errNotFound, "must be the generic scope-check 404, matching the POST siblings") +} + // --- Azure credential-resolution path tests (issue #871) --- // // These tests exercise the production path of buildAzureExchangeClient, which From 0192acbe5a1e487f533bef3c53134de56b2a4726 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 3 Aug 2026 23:54:43 +0200 Subject: [PATCH 2/3] fix(api): address adversarial review findings on Azure RI exchange scoping F2: filterAzureReservationsByScope has no production caller today -- buildAzureExchangeClient resolves subscriptionID via an exact external_id match, so an empty subscriptionID never resolves a registered account and the graceful empty-state branch returns before the tenant-wide listing is ever fetched. Correct the doc comments on listExchangeableAzureRIs and buildAzureExchangeClient to say so plainly instead of describing the no-subscription_id branch as live. Add a regression test that exercises the filter production actually reaches on this path, filterAzureReservationsBySubscription, against a mix of in-scope and out-of-scope rows. F7: normalize a nil reservations slice to [] on the admin passthrough so an unrestricted session can't emit {"reservations": null} while every other path on this handler emits []. Also drops a duplicated log-safety comment (doc comment already states it) and a stale "AND no accounts registered" conjunction in buildAzureExchangeClient's doc that the code never implements. --- internal/api/handler_ri_exchange.go | 26 ++++++++++--- internal/api/handler_ri_exchange_test.go | 47 ++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/internal/api/handler_ri_exchange.go b/internal/api/handler_ri_exchange.go index b03d8db50..a298663d5 100644 --- a/internal/api/handler_ri_exchange.go +++ b/internal/api/handler_ri_exchange.go @@ -194,8 +194,10 @@ type azureExchangeClient interface { // wired OIDC signer so managed_identity / client_secret / WIF all work. // // Graceful empty-state rules (returns nil client, nil error): -// - subscriptionID is empty AND no Azure accounts are registered at all: -// Azure is not configured; the caller returns an empty reservations list. +// - subscriptionID is empty: GetCloudAccountByExternalID looks up an exact +// match on external_id, and no registered account has an empty +// external_id, so this unconditionally misses and the caller returns an +// empty reservations list. // - subscriptionID is provided but no matching CloudAccount is found: // treated as "Azure not configured for this subscription". // @@ -259,7 +261,15 @@ func (h *Handler) buildAzureExchangeClient(ctx context.Context, subscriptionID s // - No subscription_id: the listing is narrowed to whatever the session's // allowed_accounts scope covers (filterAzureReservationsByScope). // Unrestricted/admin sessions see everything, matching every other -// listing endpoint in this package. +// listing endpoint in this package. In production this branch is +// defense-in-depth rather than the live path today: buildAzureExchangeClient +// resolves subscriptionID via an exact external_id match, so an empty +// subscriptionID never matches a registered account and the graceful +// empty-state branch below returns before ListExchangeableReservations +// is ever called. It becomes the live path the moment a caller can reach +// ListExchangeableReservations without a subscription_id -- the +// azureExchangeFactory test seam already exercises exactly that -- which +// is why the filter stays rather than being deleted as unreachable. // // Rows that cannot be attributed to a registered CloudAccount (no // BillingScopeID, or one that matches no CloudAccount) are dropped rather @@ -359,6 +369,13 @@ func (h *Handler) filterAzureReservationsByScope(ctx context.Context, session *S return nil, fmt.Errorf("failed to get allowed accounts: %w", err) } if auth.IsUnrestrictedAccess(allowed) { + // client.ListExchangeableReservations may return a nil slice for zero + // results; every other path here returns a non-nil empty slice, so + // normalize here too rather than letting an admin session alone see + // {"reservations": null}. + if reservations == nil { + reservations = []azurecompute.ExchangeableReservation{} + } return reservations, nil } @@ -369,9 +386,6 @@ func (h *Handler) filterAzureReservationsByScope(ctx context.Context, session *S } filtered := filterReservationsByScopeIndex(reservations, azureScopeIndex(accounts), allowed) - // Counts, not the dropped reservation/billing-scope identifiers, are safe - // to log -- logging the identifiers would just relocate the disclosure - // this filter exists to prevent. if dropped := len(reservations) - len(filtered); dropped > 0 { logging.Debugf("azure exchange listing: filtered %d of %d tenant-wide reservations outside session scope", dropped, len(reservations)) } diff --git a/internal/api/handler_ri_exchange_test.go b/internal/api/handler_ri_exchange_test.go index eee7cbb73..38ae845ac 100644 --- a/internal/api/handler_ri_exchange_test.go +++ b/internal/api/handler_ri_exchange_test.go @@ -966,6 +966,53 @@ func TestListExchangeableAzureRIs_SubscriptionIDOutOfScope(t *testing.T) { assert.ErrorIs(t, err, errNotFound, "must be the generic scope-check 404, matching the POST siblings") } +// TestListExchangeableAzureRIs_SubscriptionIDFiltersToOwnRows exercises +// filterAzureReservationsBySubscription directly through the handler -- the +// row-level filter on the ?subscription_id= path, which is the only filter +// this listing's production callers can actually reach (buildAzureExchangeClient +// resolves subscriptionID via an exact external_id match, so the +// no-subscription_id branch that reaches filterAzureReservationsByScope never +// gets a live Azure client in production; see the doc comment on +// listExchangeableAzureRIs). +// +// The scoped session is authorized for subscription sub-1 (requireAzureSubscriptionScope +// passes), and the tenant-wide listing the injected client returns mixes rows +// billed to sub-1 and sub-2 -- the same tenant-wide breadth +// ListExchangeableReservations always returns regardless of which +// subscription's credentials made the call. Only the sub-1 row must survive. +func TestListExchangeableAzureRIs_SubscriptionIDFiltersToOwnRows(t *testing.T) { + ctx := context.Background() + store := &MockConfigStore{} + store.GetCloudAccountByExternalIDFn = func(_ context.Context, provider, externalID string) (*config.CloudAccount, error) { + require.Equal(t, "azure", provider) + require.Equal(t, "sub-1", externalID) + return &config.CloudAccount{ID: "acct-mine", Name: "Mine Team", Provider: "azure", ExternalID: "sub-1"}, nil + } + stub := &stubAzureExchangeClient{reservations: []azurecompute.ExchangeableReservation{ + {ReservationID: "res-own", BillingScopeID: "/subscriptions/sub-1", SKU: "Standard_D2s_v3", Quantity: 1, Region: "eastus"}, + {ReservationID: "res-other", BillingScopeID: "/subscriptions/sub-2", SKU: "Standard_D4s_v3", Quantity: 5, Region: "westeurope"}, + }} + + h := &Handler{ + auth: scopedAzureAuth(t, "view", "purchases", []string{"acct-mine"}), + config: store, + azureExchangeFactory: func(_ string) azureExchangeClient { return stub }, + } + + res, err := h.listExchangeableAzureRIs(ctx, &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"authorization": "Bearer tok"}, + QueryStringParameters: map[string]string{"subscription_id": "sub-1"}, + }) + require.NoError(t, err) + resp, ok := res.(*ExchangeableAzureRIsResponse) + require.True(t, ok) + require.Len(t, resp.Reservations, 1, "only the requested subscription's own reservation must be returned") + assert.Equal(t, "res-own", resp.Reservations[0].ReservationID) + for _, r := range resp.Reservations { + assert.NotEqual(t, "res-other", r.ReservationID, "a reservation billed to a different subscription must never appear") + } +} + // --- Azure credential-resolution path tests (issue #871) --- // // These tests exercise the production path of buildAzureExchangeClient, which From cbd12d9cd65ed55a43be404e8de61ecddc7f8b35 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 3 Aug 2026 23:57:38 +0200 Subject: [PATCH 3/3] test(api): assert the out-of-scope subscription gate never lists reservations ownsAzureSource registers ListExchangeableReservations with .Maybe(), so AssertExpectations alone passes whether it was called or not. Add an explicit AssertNotCalled to prove the scope gate refuses the request before the tenant-wide listing is ever fetched. --- internal/api/handler_ri_exchange_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/api/handler_ri_exchange_test.go b/internal/api/handler_ri_exchange_test.go index 38ae845ac..11fb15e81 100644 --- a/internal/api/handler_ri_exchange_test.go +++ b/internal/api/handler_ri_exchange_test.go @@ -964,6 +964,11 @@ func TestListExchangeableAzureRIs_SubscriptionIDOutOfScope(t *testing.T) { }) require.Error(t, err, "a scoped session must not list an out-of-scope subscription's reservations") assert.ErrorIs(t, err, errNotFound, "must be the generic scope-check 404, matching the POST siblings") + // ownsAzureSource registers ListExchangeableReservations with .Maybe(), so + // AssertExpectations above passes whether it was called or not; assert it + // explicitly to prove the scope gate refuses the request before the + // tenant-wide listing is ever fetched. + opsClient.AssertNotCalled(t, "ListExchangeableReservations") } // TestListExchangeableAzureRIs_SubscriptionIDFiltersToOwnRows exercises