[cristim]

What

Fixes item 4 of #386: authorizeSessionRevoke (purchase-history revoke and the revoke quote endpoint) returned early for revoke-any, so a user holding revoke-any but restricted to some cloud accounts could quote and return an executed Azure reservation in an account outside their scope. Same class as #92.

revoke-any now lifts only the ownership requirement, not the account scope. The revoke-any and revoke-own branches share one check (checkRevokeAccountAccess):

• unrestricted scope + revoke-any: allowed, including rows with no cloud_account_id (admins keep revoking legacy rows);
• scoped revoke-any: the row's account must be in scope; a nil or empty cloud_account_id fails closed with 403;
• revoke-own: unchanged (nil account is a 403 even for unrestricted scope; out of scope is a 403).

The stateless admin API key still short-circuits before any lookup.

How verified

GOTOOLCHAIN=go1.26.6 GOWORK=off GOFLAGS='-p=2 -count=1' AWS_EC2_METADATA_DISABLED=true

New tests:

• TestAuthorizeSessionRevoke_RevokeAny_AccountScope: scoped revoke-any user against in-scope, other-account, nil and empty account rows.
• TestRevokePurchase_RevokeAnyOutOfScopeNeverCallsAzure: drives revokePurchase (mocked store/auth, stubbed Azure client factory) for a scoped revoke-any user against an Azure purchase in another account; asserts 403 and that no Azure client is built.

On main (fix reverted, tests kept):

--- FAIL: TestAuthorizeSessionRevoke_RevokeAny_AccountScope/other_account
        Messages: expected ClientError, got <nil>: <nil>
--- FAIL: TestAuthorizeSessionRevoke_RevokeAny_AccountScope/nil_account
--- FAIL: TestAuthorizeSessionRevoke_RevokeAny_AccountScope/empty_account
--- FAIL: TestRevokePurchase_RevokeAnyOutOfScopeNeverCallsAzure
        Messages: expected ClientError, got *fmt.wrapError: revoke azure: obtain credential: azure must not be reached

(The in_scope subtest and the two adjusted existing tests also fail on main, but only because main never calls GetAllowedAccountsAPI on the revoke-any path, so the mock expectation goes unmet.)

With the fix: go test ./internal/api/ ./internal/auth/ passes (3268 tests). go build ./..., go vet ./internal/api/, go vet -tags integration ./internal/api/ and golangci-lint run ./internal/api/... are clean.

Mutation check: changing the guard to revokeAny || scope.AllowsAll() (revoke-any skips scope again, unrestricted own skips attribution) fails other_account, nil_account, empty_account, the endpoint test and TestAuthorizeSessionRevoke_RevokeOwn_NilAccountID.

These are mock-based unit tests. No real Azure return or purchase was performed.

Deliberately left for follow-up (from #386)

• Item 1 (in-progress ramp shape change restarts at step 0): needs an owner decision between carrying completed percentage into the new schedule and rejecting shape changes with 409. Not touched.
• Item 2 (updatePlan read-modify-write outside the ramp lock): appears covered by fix(purchase): preserve ramp progress across stale plan updates #464. UpdatePurchasePlanTx now updates WHERE id = $1 AND updated_at = $8, and updatePlan passes the snapshot's UpdatedAt, so a step completing between read and write produces a 409 instead of a stale CurrentStep.
• Item 3 (run-now error mapping, ExecutedAt/ExecutedByUserID stamping, hardcoded "completed" status): still open on main (handler_purchases.go maps every RunPlannedPurchaseNow error to 409 and returns "status": "completed"). It is a separate concern and gets its own PR.

Reviewer notes

• scope.Allows(id, "") matches by account ID only, as the existing revoke-own check did. A scoped revoke-any user whose allow-list names accounts by display name will now get a 403 on those rows. That fails closed, but say if you want the name resolved (lookupCloudAccount) here.

Refs #386

Summary by CodeRabbit

• Bug Fixes
  • Purchase revocation now respects account access restrictions, including for users with broad revocation permissions.
  • Revocations are denied when a purchase has no associated cloud account or falls outside the caller’s allowed accounts.

[coderabbitai]

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

Warning

Review limit reached

• Run on-demand review

_{This review includes 1 billable file and costs up to $0.25.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 36 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 60 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 51ba9325-74fb-4349-ac33-8923d0f42fc0

📥 Commits

Reviewing files that changed from the base of the PR and between 8761425 and 4e08927.

📒 Files selected for processing (1)

• internal/auth/types.go

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: ac108239-6e5c-4913-8ef5-e7fad53da878

📥 Commits

Reviewing files that changed from the base of the PR and between 115ff70 and 8761425.

📒 Files selected for processing (2)

• internal/api/handler_purchases_revoke.go
• internal/api/handler_purchases_revoke_test.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 1 review per hour.

---

📝 Walkthrough

Walkthrough

Revocation authorization now applies account-scope checks to revoke-any callers unless their scope allows all accounts. Tests cover account associations and confirm that an out-of-scope Azure purchase is rejected before Azure credentials are created.

Changes

Purchase revocation authorization

Layer / File(s)	Summary
Account-scope authorization and validation internal/api/handler_purchases_revoke.go, internal/api/handler_purchases_revoke_test.go	revoke-any callers are subject to account-scope checks unless their scope allows all accounts. Tests cover matching and out-of-scope accounts, nil and empty associations, and rejection of an out-of-scope Azure purchase before credential creation.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 87614

The change closes a gap where callers restricted to specific cloud accounts could revoke purchases outside their scope. Tests cover the scoped and unrestricted cases. No merge-blocking risk was identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: applying account scope to revoke-any for executed purchases.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ 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.}

[cristim]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.