test(api): pin reshape security paths — STS-error fail-closed + region normalization regression guards

[cristim]

Summary

PR #79 + the follow-up commit f44b06567 hardened resolveAWSCloudAccountID to fail closed on STS errors, preventing cross-tenant recommendation leaks in multi-tenant deployments. The fix landed without regression tests for the new failure paths — the existing internal/api/exchange_lookup_test.go covers the happy paths and the legitimate "no AWS context" fall-through but does not exercise:

• Transient STS error → resolveAWSCloudAccountID returns a non-nil error, purchaseRecLookupFromStore aborts, handler returns 500/error to client.
• AWS SDK config load failure → returns ("", nil) and skips the AccountIDs filter (legitimate path, distinct from the STS-error path).
• Region query-string omitted → handler normalizes from cfg.Region, store query is region-scoped (the MAJOR fix from afc3aa1ff).
• ListCloudAccounts error → bubbles up as wrapped error with list cloud accounts for reshape scope: prefix.

Without these, a future refactor can silently break the security gates that the recent fixes added.

Why these specifically

These arent UI/UX concerns — they are security regression guards. The CRITICAL failure mode they prevent is "transient STS error in a multi-tenant deployment shows tenant A their alternatives + tenant B sources mixed in". The fix is in place; the test pinning it isnt.

Proposed tests

All in internal/api/exchange_lookup_test.go:

// Mock the STS-resolver primitive to return an error.
func TestResolveAWSCloudAccountID_FailsClosedOnSTSError(t *testing.T) { ... }

// Same primitive returns ("", nil) — legitimate Azure/GCP host path.
func TestResolveAWSCloudAccountID_PassesNilWhenNoAWSContext(t *testing.T) { ... }

// Mock a fake recsLister returning an error from ListStoredRecommendations.
func TestPurchaseRecLookupFromStore_PropagatesStoreError(t *testing.T) { ... }

// Region query-string empty — handler normalizes from cfg.Region.
func TestHandleRIExchangeReshape_EmptyRegionUsesConfigRegion(t *testing.T) { ... }

The STS-error test is the most important — it pins the multi-tenant safety contract.

References

• Hardening commits: 50548797e, afc3aa1ff, f44b06567
• PR perf(exchange): rec-driven cross-family alternatives via cached Cost Explorer recommendations #79
• Files: internal/api/exchange_lookup.go, internal/api/exchange_lookup_test.go, internal/api/handler_ri_exchange.go

Severity

Low-Medium — no current bug, but a regression guard for an addressed CRITICAL.

Effort

Small — ~80 LOC, all in one test file. Mocking STS GetCallerIdentity requires an extra interface seam (stsClient) which doesnt exist today; either add it or use a higher-level test that drives the failure through the public Handler API.

[cristim]

P2: no current bug, but these tests pin the multi-tenant STS fail-closed contract introduced in commits 5054879/afc3aa1ff/f44b06567. Without them, a future refactor of resolveAWSCloudAccountID could silently break the cross-tenant isolation guarantee. The STS-error test is the critical one. (triage agent wave1-D)

[cristim]

Verification (post-merge of #79)

Confirming this issue is partially addressed — recommend keep open.

What I checked on feat/multicloud-web-frontend

internal/api/exchange_lookup_test.go covers the store-adapter layer well:

• TestPurchaseRecLookupFromStore_RegionFilter
• TestPurchaseRecLookupFromStore_AccountFilter
• TestPurchaseRecLookupFromStore_NoAccountFilterWhenEmpty (legitimate ("", nil) skip path)
• TestPurchaseRecLookupFromStore_NoRecsReturnsEmpty
• TestPurchaseRecLookupFromStore_StoreErrorPropagates (covers the ListCloudAccounts/store error propagation goal)
• TestPurchaseRecLookupFromStore_MapsFields
• TestPurchaseRecLookupFromStore_ThreeYearTerm

What is still missing

The security-critical paths called out in the issue body are NOT pinned:

1. STS-error fail-closed — resolveAWSCloudAccountID is the function the multi-tenant safety contract lives in (exchange_lookup.go:158); its error path ("STS reachable-but-failed must NOT fall through to an unscoped read") has no direct test. The integration test (handler_ri_exchange_integration_test.go:76) instead stubs out the resolver with reshapeAccountResolver: func(...) ("", nil), so the real function and its error path are untested.
2. Region normalization (the major afc3aa1ff fix) — handler_ri_exchange.go:241-244 adopts cfg.Region when ?region= is empty, but no test in handler_ri_exchange_test.go or handler_ri_exchange_integration_test.go exercises the empty-region path and asserts the normalized region propagates to the store query.
3. resolveAWSCloudAccountID "resolved-but-unregistered" → fail-closed branch (the worst-case multi-tenant leak guard) is also untested.

Verdict

The lookup adapter is well-tested; the resolver and handler paths the issue specifically calls out as security regression guards remain un-pinned.

Recommend keep open — small, focused follow-up to add the four tests sketched in the issue body.