[cristim]

What

Finishes the tenant-scoping sweep from #29 and #384 for three read paths:

1. GET /api/plans (listPlans, sec(plans): listPlans does not scope account_ids to caller's allowed_accounts #29 and sec(api): finish tenant-scoping sweep: inventory filter, RI-exchange quote, plans list (follow-ups to #374/#379) #384 item 3). The handler discarded the session and passed the client's account_ids straight to the store. A scoped user got every tenant's plans with no params, and ?account_ids=<other tenant> narrowed the list to that tenant. It now intersects account_ids with allowed_accounts through intersectAccountFilterScope and returns an empty list when nothing remains.
2. Store query (buildListPlansQuery, owner comment on sec(plans): listPlans does not scope account_ids to caller's allowed_accounts #29). The OR NOT EXISTS (plan_accounts) disjunct was unconditional, so every unassigned plan came back to any account-filtered caller. It is now gated behind a new PurchasePlanFilter.IncludeUnassigned, and only unrestricted callers set it. Scoped callers never see unassigned plans. This matches requirePlanAccess (by-id reads already 404 them for scoped users) and the marketplace/revoke precedent (unattributed rows are refused to scoped users).
3. Inventory (fetchCommitmentRecords, sec(api): finish tenant-scoping sweep: inventory filter, RI-exchange quote, plans list (follow-ups to #374/#379) #384 item 1). An explicit account_id now goes through intersectAccountFilterScope like the dashboard does, and the empty-scope sentinel short-circuit is kept. Before this change an out-of-scope account was read from the store, and only the callers' in-memory post-filter hid the rows. So this item is hardening, not a live leak.
4. RI-exchange quote (getExchangeQuote, sec(api): finish tenant-scoping sweep: inventory filter, RI-exchange quote, plans list (follow-ups to #374/#379) #384 item 2). It now applies requireReshapeAccountScope, the gate executeExchange uses: 403 when the deployment's AWS account is outside the session scope. A quote is a single action, not a list, so it uses the same 403 as execute rather than the empty list the list siblings return. A scope-lookup error stays a 500 (fail closed).

#384 item 4 was already covered by #389. Revoke endpoints (#404) are untouched.

How verified

Ran on macOS with GOTOOLCHAIN=go1.26.6 GOWORK=off GOFLAGS='-p=2 -count=1' AWS_EC2_METADATA_DISABLED=true:

• go build ./..., go vet (with and without -tags integration) for internal/api, internal/config and internal/server, plus golangci-lint run (0 issues) and the pre-commit hooks (including gocyclo): all pass.
• go test ./internal/api/ ./internal/config/ ./internal/server/: all ok.
• Real Postgres (testcontainers, postgres:16-alpine): go test -tags integration ./internal/config/ -run TestPostgresStoreDB_ListPurchasePlans_UnassignedBucket passes. It now has three cases: an admin sees the unassigned plan, a scoped caller sees neither the unassigned plan nor the other account's plan, and the no-filter path is unchanged.

Failing before / passing after. Each fix file was reverted to origin/main with the new tests kept, and the tests were run:

=== pre-fix internal/api/handler_plans.go
--- FAIL: TestListPlans_ScopedCallerSeesOnlyInScopePlans
--- FAIL: TestListPlans_ScopedCallerOutOfScopeFilterReturnsNothing
        Should be empty, but was [a2222222-... a4444444-...]
--- FAIL: TestListPlans_UnrestrictedCallerStillSeesUnassignedPlans
--- FAIL: TestListPlans_ScopeLookupErrorFailsClosed
=== pre-fix internal/api/handler_inventory.go
--- FAIL: TestListActiveCommitments_ScopedCallerOutOfScopeAccountFilterSkipsStore
        mock: GetActivePurchaseHistory was called 1 time(s) ... expected none
=== pre-fix internal/api/handler_ri_exchange.go
--- FAIL: TestGetExchangeQuote_OutOfScopeDeploymentAccountReturns403
        "exchange quote failed" does not contain "allowed accounts"
=== pre-fix internal/config/store_postgres.go (integration)
--- FAIL: .../account-filtered_result_without_IncludeUnassigned_excludes_unassigned_plan
        planB (unassigned) must not reach a scoped caller

All of them pass with the fix. A narrower mutation (IncludeUnassigned: true for scoped callers too) fails TestListPlans_ScopedCallerSeesOnlyInScopePlans.

The handler tests use a store fake that applies the same predicate as buildListPlansQuery, so a wrong filter fails on an assertion, not on an unstubbed-mock panic. The quote tests use failing AWS credentials, so they never call EC2. This evidence comes from fixtures and mocks; the store predicate itself is checked against real Postgres.

Frontend: plans.ts renders the "Unassigned" section only when such plans are present, so scoped users simply don't get that section. The inventory and RI-exchange pages need no change.

Deliberately left

• Revoke endpoints and per-permission Constraints (sec(auth): enforce per-permission Constraints on ladder/config/RI-exchange-config/marketplace/revoke (SEC-01 follow-up to #60) #404).

Closes #29
Refs #384

Summary by CodeRabbit

• Bug Fixes
  • Account-filtered commitment and purchase-plan results now respect the caller’s permitted account scope.
  • Exchange quote requests are rejected when the deployment’s account is outside the caller’s permitted scope.
  • Unassigned plans are included in account-filtered results for unrestricted sessions, but excluded for scoped sessions.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: b4b8f1f8-b8b3-49c9-ab8e-ffbaf2a082a3

📥 Commits

Reviewing files that changed from the base of the PR and between dc8e979 and cf2cfe9.

📒 Files selected for processing (9)

• internal/api/handler_inventory.go
• internal/api/handler_plans.go
• internal/api/handler_plans_test.go
• internal/api/handler_ri_exchange.go
• internal/api/tenant_scope_sweep_test.go
• internal/config/store_postgres.go
• internal/config/store_postgres_pgxmock_test.go
• internal/config/store_postgres_unassigned_test.go
• internal/config/types.go

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.

---

📝 Walkthrough

Walkthrough

The API now scopes plan and commitment account filters to the caller’s allowed accounts and rejects RI-exchange quotes for out-of-scope deployment accounts. Account-filtered plan queries include unassigned plans only when enabled; unrestricted plan listing enables that option.

Changes

Tenant account scoping

Layer / File(s)	Summary
Plan filter and query behavior internal/config/types.go, internal/config/store_postgres.go, internal/config/store_postgres_pgxmock_test.go, internal/config/store_postgres_unassigned_test.go	PurchasePlanFilter adds IncludeUnassigned. Account-filtered queries include unassigned plans only when this flag is set. Tests cover enabled and unset behavior.
Scoped plan listing internal/api/handler_plans.go, internal/api/handler_plans_test.go, internal/api/tenant_scope_sweep_test.go	listPlans intersects requested accounts with session scope. It skips the store query when no requested account is accessible and includes unassigned plans for unrestricted sessions. Tests cover scoped visibility, empty results, unrestricted access, and scope lookup errors.
Commitment and quote scope checks internal/api/handler_inventory.go, internal/api/handler_ri_exchange.go, internal/api/tenant_scope_sweep_test.go	Commitment reads intersect account filters with session scope. RI-exchange quotes check the deployment account against session scope. Tests cover in-scope and out-of-scope requests.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant listPlans
  participant scopedPlanFilter
  participant PurchasePlanStore
  Client->>listPlans: Request plans with account_ids
  listPlans->>scopedPlanFilter: Build filter from session scope and account_ids
  scopedPlanFilter-->>listPlans: Scoped filter and account availability
  listPlans->>PurchasePlanStore: ListPurchasePlans with scoped filter
  PurchasePlanStore-->>listPlans: Matching plans
  listPlans-->>Client: Plan list

Loading

Merge Risk: ⚪ Minimal · up to cf2cf

The change scopes plan, inventory, and RI-exchange quote reads to the caller's allowed accounts, and no actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 3 | ❌ 1 | ❓ 1

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 43.48% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 9 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check	❓ Inconclusive	#29 is addressed: listPlans intersects requested accounts with the caller’s scope, returns no results for an empty intersection, and limits unassigned plans to unrestricted callers. The added regres…	Verify whether the reviewed head contains regression tests for both #384 item 4 behaviors: a restricted in-scope session reaching HasPermissionForConstraintsAPI, and a GetAllowedAccountsAPI error producing HTTP 500 rather than 403.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Out of Scope Changes check	✅ Passed	The changed handlers, store filter, and tests support the linked tenant-scoping requirements in #29 and #384. The changes do not show unrelated behavior. The PR description states that revoke endpoint…
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main changes: scoping plan lists, inventory filters, and RI quotes to allowed accounts.

Full details: Linked Issues check

Explanation

#29 is addressed: listPlans intersects requested accounts with the caller’s scope, returns no results for an empty intersection, and limits unassigned plans to unrestricted callers. The added regression tests cover cross-tenant plans and unassigned plans. #384 items 1–3 are addressed by inventory scope intersection, the quote scope gate, and plans-list filtering, with regression tests. I cannot establish item 4: the reviewed summary does not show its requested tests for a restricted in-scope session reaching HasPermissionForConstraintsAPI or for GetAllowedAccountsAPI errors returning HTTP 500 rather than 403. The inspected fail-closed test covers scope-error propagation at a helper level, not those specific behaviors. The PR’s reference to #389 does not establish that the tests are present at this head.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}