sec(api): listExchangeableAzureRIs returns tenant-wide reservations without allowed-accounts scoping

Summary

GET /api/ri-exchange/azure-instances returns Azure reservations that are tenant-wide and are never intersected with the caller's allowed_accounts scope. Any authenticated session holding view:purchases can read every exchangeable Azure VM reservation the deployment's Azure credentials can see, including reservations belonging to subscriptions outside that user's allowed-accounts set.

Verified on origin/main @ 101f099fbb25f36e16a493bebe45e40e64c4f795.

Where

Route: internal/api/router.go:303 — {ExactPath: "/api/ri-exchange/azure-instances", Method: "GET", Handler: r.listExchangeableAzureRIsHandler, Auth: AuthUser}
Handler: internal/api/handler_ri_exchange.go:232-256 — listExchangeableAzureRIs
Client builder: internal/api/handler_ri_exchange.go:193-222 — buildAzureExchangeClient
Underlying provider call: providers/azure/services/compute/exchange.go:92-112 — ListExchangeableReservations -> armreservations.ReservationClient.NewListAllPager(nil)

The handler body, in full:

func (h *Handler) listExchangeableAzureRIs(ctx context.Context, req *events.LambdaFunctionURLRequest) (any, error) {
	if _, err := h.requirePermission(ctx, req, "view", "purchases"); err != nil {
		return nil, err
	}

	subscriptionID := req.QueryStringParameters["subscription_id"]

	client, err := h.buildAzureExchangeClient(ctx, subscriptionID)
	...
	reservations, err := client.ListExchangeableReservations(ctx)
	...
	return &ExchangeableAzureRIsResponse{Reservations: reservations}, nil
}

It gates on the verb (view:purchases) and nothing else. It never calls getAllowedAccounts, never calls reshapeCloudAccountInScope, and never validates the caller-supplied ?subscription_id= against the session's scope. buildAzureExchangeClient performs no scoping either: it is a plain GetCloudAccountByExternalID(ctx, "azure", subscriptionID) lookup, so registration of the subscription in CUDly is the only gate, not per-caller authorization.

Two independent leaks stack here:

Caller-chosen subscription. subscription_id is taken verbatim from the query string. Any registered Azure subscription GUID is accepted, whether or not it is in the caller's allowed_accounts.
Tenant-wide result set even for an in-scope subscription. ListExchangeableReservations builds the pager with NewListAllPager(nil), which enumerates reservations across the whole tenant rather than the subscription the client was constructed for. The code comment at handler_ri_exchange.go:219-220 acknowledges this ("the tenant-wide armreservations API"). So a scoped user who legitimately owns one Azure subscription still receives every other subscription's reservations in the same tenant.

Why the sibling handlers prove the intended contract

Every other scoped read in the same file applies allowed-accounts resolution. listExchangeableAzureRIs is the outlier:

Handler	Line	Scoping
`listConvertibleRIs`	`handler_ri_exchange.go:343`	`reshapeCloudAccountInScope` at `:351`
`getRIUtilization`	`:379`	`reshapeCloudAccountInScope` at `:386`
`getReshapeRecommendations`	`:532`	`reshapeCloudAccountInScope` at `:539`
`getRIExchangeHistory`	`:1001`	`getAllowedAccounts` at `:1016`
`listExchangeableAzureRIs`	`:232`	none

reshapeCloudAccountInScope (handler_ri_exchange.go:285) is itself a thin wrapper over h.getAllowedAccounts + auth.IsUnrestrictedAccess + auth.MatchesAccount, the same primitives used by internal/api/scoping.go (requireAccountAccess, requirePlanAccess, validatePurchaseRecommendationScope, requireExecutionAccess) and by handler_accounts.go, handler_analytics.go, handler_dashboard.go, handler_history.go, handler_ladder.go, handler_marketplace.go and handler_recommendations.go. The pattern is repo-wide; this endpoint simply misses it.

There is also existing regression coverage asserting exactly this contract for the AWS siblings (internal/api/handler_per_account_perms_test.go:1116, :1202, :1274, headed "getAllowedAccounts scope, issue #1030"), which lists /ri-exchange/instances, /ri-exchange/utilization and /ri-exchange/reshape but not /ri-exchange/azure-instances.

Impact

Cross-account (and, within a tenant, cross-subscription) information disclosure behind a read permission that is widely granted. Leaked fields include reservation IDs, SKUs, quantities, terms and provisioning state, which is commercially sensitive capacity/spend data and also a reconnaissance aid for the exchange execute path.

view:purchases is a read verb held by ordinary non-admin users, so the blast radius is "any logged-in user of a multi-tenant/multi-team deployment", not "an admin".

Relationship to #1527 (separable, not a duplicate)

#1527 covers the execute half: the exchange source reservations are not scoped to the authorized subscription. That code path does not exist on main yet; it arrives with open PR #1515.

This issue is the read/list half. It is already reachable on main today, on a route that is live in the current deployment, and it does not depend on #1515 merging or on #1527 being fixed. Fixing #1527 in the execute path leaves this listing endpoint leaking, and vice versa. Filing them separately keeps each fix independently verifiable and independently landable; they should probably share a helper once both land.

Reproduction sketch

Register two Azure subscriptions in the same tenant, SUB-A and SUB-B, as cloud accounts.
Create a non-admin user whose allowed_accounts contains only the account for SUB-A, and grant view:purchases.
As that user: GET /api/ri-exchange/azure-instances?subscription_id=<SUB-B-guid>.
- Expected: empty list, 403, or 404.
- Actual: reservations are returned.
As the same user, repeat with the in-scope subscription: GET /api/ri-exchange/azure-instances?subscription_id=<SUB-A-guid>.
- Expected: only SUB-A reservations.
- Actual: reservations from SUB-B are included too, because NewListAllPager is tenant-wide.

Step 4 is the important one: it fails even without a hostile query parameter, so a fix that only validates subscription_id is incomplete.

Suggested fix direction

Capture the session: session, err := h.requirePermission(ctx, req, "view", "purchases") (the sibling handlers already do this; the return value is currently discarded with _).
Resolve the caller's scope via h.getAllowedAccounts(ctx, session) and short-circuit for auth.IsUnrestrictedAccess.
Reject or intersect the caller-supplied subscription_id: when the session is scoped and the requested subscription's CloudAccount does not satisfy auth.MatchesAccount, return the same not-found/empty shape the siblings use rather than the reservations. When subscription_id is absent, derive the set of in-scope Azure subscriptions instead of leaving it caller-controlled.
Filter the returned []azurecompute.ExchangeableReservation down to the in-scope subscriptions, since the provider call is tenant-wide by construction. The subscription is recoverable from each reservation's ARM resource ID; alternatively push a subscription filter into providers/azure/services/compute/exchange.go.
Add regression coverage alongside internal/api/handler_per_account_perms_test.go:1116/1202/1274, covering both the out-of-scope subscription_id case and the tenant-wide-bleed case from step 4 of the reproduction. Both tests must fail against current main.

Steps 3 and 4 are separate defects; a fix that stops at 3 still leaks.

Note for triage (not covered by this issue)

listTargetOfferings (internal/api/handler_ri_exchange.go:115) also gates only on view:purchases with no allowed-accounts resolution. It reads through the deployment's ambient AWS credentials rather than a caller-supplied account, so the exposure shape is different and it needs its own assessment. Flagging it here so it is not lost; it is deliberately out of scope for this issue.

Additional evidence from the 2026-07-28 full-repo review. Reviewed commit be11bdcb5; note that origin/main moved to 3e9660d06 during the review, so re-verify against current main before changing code.

This is a structural detail that changes the shape of the fix, so recording it here rather than filing a second issue.

Where: providers/azure/services/.../exchange.go:28-59 (ExchangeableReservation struct), alongside the tenant-wide NewListAllPager(nil) this issue already records. SDK reference: armreservations@v1.1.0/models.go:497-500.

What: beyond the listing being tenant-wide, the ExchangeableReservation struct drops AppliedScopes and AppliedScopeType even though the SDK exposes both on the source model.

Why it matters for the fix: no downstream consumer is structurally capable of scoping the result, even if it wanted to. The scope information is discarded at the conversion boundary, so a caller cannot filter on it and cannot even tell that it is missing.

That means the fix is not "add a filter at the call site". It is "carry the scope fields through the type, then filter". Any patch that only adds a subscription filter to the pager call will leave every other consumer of ExchangeableReservation unable to scope, and the next call site added will reintroduce the same gap.

Suggested fix direction: add AppliedScopes and AppliedScopeType to the struct and populate them in the converter, then filter on them (or on BillingScopeID) before returning, so scoping is expressible rather than merely applied in one place.

Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code, since a finding may have been fixed or moved.

Independently re-derived and re-verified on be11bdcb5 during the 2026-07-28 full review of internal/api (this issue was verified on 101f099fb). The finding matches this issue, so recording as evidence rather than filing a second one. Two additions:

1. The no-parameter case is also leaking, and the issue's reproduction does not cover it. The repro here starts from a supplied ?subscription_id=. When subscription_id is omitted, buildAzureExchangeClient (internal/api/handler_ri_exchange.go:193-222) still resolves credentials for whichever registered Azure account matches first, and ListExchangeableReservations returns the same tenant-wide set. So the endpoint leaks with an empty query string, from a plain view:purchases session, with no knowledge of any subscription GUID at all. Any regression test should include the bare GET /api/ri-exchange/azure-instances case alongside the two already listed.

2. The fix must filter the result set, not just the parameter. Restating for emphasis because it is the easy thing to get wrong: getAllowedAccounts is consulted for neither the requested subscription_id nor the returned reservations. A guard on the request parameter alone is insufficient by construction, since the underlying client returns tenant-wide data. The comment at handler_ri_exchange.go:219-221 already states the call "uses the tenant-wide armreservations API which is not scoped to a region"; it is not scoped to a subscription either. The returned []ExchangeableReservation slice must be post-filtered by the subscriptions the session is entitled to.

Failure scenario as re-derived: user U is entitled to Azure subscription S1 only. GET /api/ri-exchange/azure-instances?subscription_id=<S1> succeeds and the response contains every exchangeable VM reservation in the whole tenant, including S2's and S3's reservation IDs, SKUs, quantities, terms and expiry dates.

Triage note: this issue is currently priority/p2 / severity/medium. The reviewer rated the same finding HIGH: it is unconditional cross-subscription disclosure of commitment inventory behind a widely granted read verb, reachable on main today, and the leaked reservation IDs are reconnaissance for the exchange execute path. Worth a severity re-look alongside the sibling scoping issues from this review.

Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code, since a finding may have been fixed or moved.

Ordering constraint from the 2026-07-28 full-repo review, verified against origin/main @ 3e9660d06.

The API-filter half of this issue is impossible to implement as stated, because the provider type carries no scope data to filter on. The provider struct has to be widened and populated first. Any fix that adds a scope filter before that lands will look correct in review, pass its tests, and change nothing.

The struct, verified

providers/azure/services/compute/exchange.go:28-60:

type ExchangeableReservation struct {
	ReservationOrderID  string    `json:"reservation_order_id"`  // :31
	ReservationID       string    `json:"reservation_id"`        // :36
	SKU                 string    `json:"sku"`                   // :39
	Quantity            int32     `json:"quantity"`              // :42
	Region              string    `json:"region,omitempty"`      // :46
	Term                string    `json:"term,omitempty"`        // :49
	ExpiryDate          time.Time `json:"expiry_date,omitempty"` // :52
	InstanceFlexibility string    `json:"instance_flexibility"`  // :56
	DisplayName         string    `json:"display_name,omitempty"`// :59
}

Small correction to the line range this was originally reported under: the struct spans :28-60, not :28-59. :59 is the last field; the closing brace is :60. Every field above was re-read on current main.

Fields that are present: reservation order ID, reservation ID, SKU, quantity, region, term, expiry date, instance flexibility, display name. Nine fields, all of them describing what the reservation is.

Fields that are absent: there is no subscription ID, no AppliedScopes, and no AppliedScopeType -- nothing describing whose it is or where it applies. Not under a different name either; grep -rn AppliedScope providers/azure/ returns exactly three hits in this file's neighbourhood, and all three are in providers/azure/services/savingsplans/client.go (:248, :258, :340, :352, on the Savings Plan purchase path). Inside exchange.go the only occurrence is a comment at :45 noting that Region "may be empty for reservations with AppliedScopeType == Shared" -- the code is aware the concept exists and still does not carry it.

ReservationOrderID is not a substitute. It is parsed out of the ARM resource ID (parseReservationOrderID, :220), and a reservation order is a billing container, not a subscription. It tells you nothing about which subscription the benefit applies to.

Why the filter cannot be written today

The full data path:

ListExchangeableReservations (:92-112) builds the pager with client.NewListAllPager(nil) at :111 -- tenant-wide by construction, as this issue's body already documents.
collectExchangeableReservations (:118-133) iterates and calls convertToExchangeableReservation on each *armreservations.ReservationResponse.
extractReservationFields (:156-183) pulls exactly seven values off the SDK item: item.ID, item.SKU.Name, item.Location, props.Quantity, props.Term, props.ExpiryDate, props.DisplayName.
convertToExchangeableReservation (:185-209) assembles the nine-field struct from those seven values plus the parsed order ID and a hardcoded InstanceFlexibility.
The handler (internal/api/handler_ri_exchange.go:232-256) receives []azurecompute.ExchangeableReservation and returns it.

By the time the value reaches the handler -- the only place an allowed_accounts intersection could be applied -- the scope information has already been discarded at step 3. reshapeCloudAccountInScope or getAllowedAccounts would have nothing to compare against. A filter written at the handler would either be a no-op or would have to fall back to the caller-supplied ?subscription_id=, which is the untrusted input this issue is about in the first place.

The data is available and is simply not extracted. In armreservations v1.1.0, ReservationResponse.Properties is a *armreservations.Properties, and that type carries both fields (models.go:549-553):

// The applied scope type
AppliedScopeType *AppliedScopeType `json:"appliedScopeType,omitempty"`

// The list of applied scopes
AppliedScopes []*string `json:"appliedScopes,omitempty"`

So this is a plumbing gap, not an API limitation. No SDK bump is needed.

Required order of work

Widen ExchangeableReservation. Add AppliedScopeType string and AppliedScopes []string (and, if a single-subscription identifier is wanted for the common case, derive it rather than inventing a parallel field). Decide up front how AppliedScopeType == Shared is represented, because a shared-scope reservation genuinely applies tenant-wide and has no single owning subscription -- the filter has to have a defined answer for it rather than falling through.
Populate them. Extend extractReservationFields (:156-183) to read props.AppliedScopeType and props.AppliedScopes, and extend the literal in convertToExchangeableReservation (:198-208). Both are nil-guarded pointer reads, matching the existing style in that function.
Add provider-level tests that assert the new fields survive the conversion, using the injectable pager (SetExchangeablePager, :76) with fixtures that carry Single-scope and Shared-scope reservations. This is the step that proves the data is actually arriving, and it is the step whose absence would let a later filter silently pass.
Only then add the handler-side intersection: resolve the caller's allowed_accounts, map them to Azure subscription IDs, and filter the list on AppliedScopes. Reject or ignore the caller-supplied ?subscription_id= when it is outside the resolved scope, rather than trusting it.
Decide the Shared policy explicitly and write it down. A shared-scope reservation is visible to every subscription in the tenant. Excluding them entirely is the conservative default and probably right for a multi-tenant deployment; including them for any in-scope caller is defensible for a single-tenant one. What is not acceptable is leaving it undefined, because "no AppliedScopes entries" will otherwise behave as either match-everything or match-nothing depending on how the predicate happens to be written -- the same empty-set-means-unrestricted trap that has produced cross-tenant leaks elsewhere in this codebase.

The failure mode to guard against

A fix that filters on data the struct never carried is indistinguishable from a working fix, from the outside:

It compiles.
A unit test that builds ExchangeableReservation values by hand and passes them through the filter will pass, because the hand-built fixtures set the scope field the production conversion never populates.
Code review sees an intersection against allowed_accounts in the handler and reads it as scoped.
In production every reservation arrives with the zero value for the scope field, and the predicate returns a constant -- either everything (leak unchanged) or nothing (endpoint silently empty for all users, which is at least loud).

This repo has the exact precedent: an account-filter defect that merged four separate times with green tests and clean review, each time because the test filtered the column the fix touched rather than the column the real data used. The structural guard against repeating it here is step 3: the regression test must start from an armreservations.ReservationResponse fixture fed through SetExchangeablePager, so the conversion is inside the test's blast radius. A test whose input is a hand-built ExchangeableReservation does not verify this fix and should not be accepted as doing so.

Scope note

Steps 1-3 are a self-contained, independently mergeable provider change with no behaviour change to the endpoint. Landing them first as their own PR makes step 4 a small, reviewable diff and gives the eventual fix something real to be tested against. It also means the struct widening is not blocked on the scoping-policy discussion in step 5.

Verified resolved at 3c0f8ac: listExchangeableAzureRIs now captures the session, rejects a caller-supplied subscription_id outside the caller's allowed-accounts scope via requireAzureSubscriptionScope, and post-filters the tenant-wide reservation list by BillingScopeID, either to the requested subscription or, when none is given, to the subscriptions the session's scope covers; reservations without a BillingScopeID are dropped rather than kept. Evidence: internal/api/handler_ri_exchange.go:284 (PR #1711, commit 85a0b4b). Residual axes checked: the bare GET with no subscription_id goes through filterAzureReservationsByScope; the provider type now carries BillingScopeID so scoping is expressible downstream; regression tests at internal/api/handler_ri_exchange_test.go:748-923 cover out-of-scope, in-scope-with-tenant-bleed, and no-parameter cases. Closing as completed; reopen if the behaviour recurs.

Navigation Menu

Repository navigation

sec(api): listExchangeableAzureRIs returns tenant-wide reservations without allowed-accounts scoping #1532

Summary

Where

Why the sibling handlers prove the intended contract

Impact

Relationship to #1527 (separable, not a duplicate)

Reproduction sketch

Suggested fix direction

Note for triage (not covered by this issue)

The struct, verified

Why the filter cannot be written today

Required order of work

The failure mode to guard against

Scope note

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

sec(api): listExchangeableAzureRIs returns tenant-wide reservations without allowed-accounts scoping #1532

Description

Summary

Where

Why the sibling handlers prove the intended contract

Impact

Relationship to #1527 (separable, not a duplicate)

Reproduction sketch

Suggested fix direction

Note for triage (not covered by this issue)

Activity

cristim commented on Jul 28, 2026

cristim commented on Jul 28, 2026

cristim commented on Jul 28, 2026

The struct, verified

Why the filter cannot be written today

Required order of work

The failure mode to guard against

Scope note

cristim commented on Sep 2, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions