fix(api): evaluate planned-purchase full-scope verbs on the API key, not its owner

[IAmJSD]

Summary

Closes #979.

Root cause

Two planned-purchase authz helpers decided full scope by asking HasPermissionAPI(ctx, session.UserID, ...). That checks the key's owner, not the API key:

• authorizePlannedPurchaseCancel (DELETE /api/purchases/planned/{id}) checked update-any and cancel-any this way.
• authorizeExecutionManagement (POST /api/purchases/planned/{id}/pause, /resume, /run) checked update-any this way. These are sibling routes with the same bug. sec(api): planned-purchase DELETE checks update-any on the key owner, not the API key's effective permissions #979 names only DELETE.

So a user API key that held only a narrow verb, enough to pass the route's entry gate, could act on another user's scheduled purchase whenever its owner held update-any. The narrow verbs are delete or cancel-own for DELETE, update for pause/resume and execute for run. For /run, that let the key force-run someone else's scheduled purchase. The fail-before test below confirms this reaches RunPlannedPurchaseNow.

On DELETE, the owner-scoped cancel-any check was already backstopped by #978: the follow-up requirePermissionConstraints call evaluates the key. update-any had no such backstop.

Fix

Both helpers now call sessionHoldsVerb, the existing helper #978 added for the creator branch of this same route. For a user-API-key session it checks the key's effective permissions through HasAPIKeyPermissionForConstraintsAPI, which means the key intersected with its owner. For a bearer session it checks the user through HasPermissionAPI, as before. A probe error refuses the request. It is never read as "not held", so it cannot fall through to the creator branch. The diff touches 3 call sites in internal/api/handler_purchases.go. It adds no new helper and widens nothing.

Authz matrix (row created by another user; the owner holds update-any + cancel-any + every narrow verb)

Caller	Route	Before	After
Key with only delete	DELETE	allowed	403
Key with only cancel-own	DELETE	allowed	403
Key with only update	pause / resume	allowed	403
Key with only execute	run	allowed (purchase runs)	403
Key with narrow verb + update-any	all four	allowed	allowed
Key with cancel-any	DELETE	allowed	allowed
Key with narrow verb, on its owner's own row	all four	allowed	allowed
Owner's bearer session	all four	allowed	allowed
Admin API key	all four	allowed	allowed (unchanged)

Planned-purchase endpoints checked

• GET /api/purchases/planned: read-only. It is gated on view:plans plus account scope, and has no full-scope verb check. Not affected.
• POST .../pause, POST .../resume, POST .../run: affected, fixed through authorizeExecutionManagement.
• DELETE .../{id}: affected, fixed through authorizePlannedPurchaseCancel.

Tests

The new file is internal/api/handler_purchases_apikey_scope_test.go.

• TestPlannedPurchaseRoutes_FullScopeFollowsTheAPIKey: drives Handler.HandleRequest (router, middleware, handler) into a real auth.Service backed by auth.MockStore. Key validation, key-and-owner intersection and every permission decision are therefore the production code. Account scope, session validation and CSRF are mocked. For each of the 4 routes, and each narrow key verb, it covers:
  • the narrow key on another user's row is refused (403/404), with no TransitionExecutionStatus or RunPlannedPurchaseNow;
  • the narrow key on its owner's row is allowed (creator control);
  • the narrow key plus update-any is allowed (the key path is not just always-deny);
  • the owner's bearer session is allowed.
• TestDeletePlannedPurchase_KeyCancelAnyKeepsFullScope: a key that holds cancel-any still cancels another user's row.
• TestPlannedPurchaseRoutes_APIKeyFullScopeProbeErrorRefuses (pause + delete): an update-any probe error refuses the request and does not fall back to the creator branch.
• Existing strict-mock tests (TestHandler_deletePlannedPurchase_UserAPIKey_Allowed, plannedKeyHandler, TestDeletePlannedPurchase_APIKeyVerbProbeErrorRefuses) now stub the key probes in place of the owner HasPermissionAPI calls. Their assertions are unchanged.

Verification (local, macOS; tests run with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null)

• Fail-before / pass-after: I reverted only handler_purchases.go to origin/main (181af4f) and kept the new tests. All 5 "is refused" subtests failed: delete/delete, delete/cancel-own, pause, resume and run. Every control passed. With the fix applied, all pass.
• Mutation probes, each applied to handler_purchases.go alone:
  • authorizeExecutionManagement back to the owner HasPermissionAPI: killed (pause/resume/run refusals).
  • DELETE update-any back to the owner: killed (both DELETE refusals).
  • Key probe always grants: killed (8 failures). Key probe always denies: killed (9).
  • authorizeExecutionManagement ignores update-any: killed (owner-session and update-any-key controls).
  • Swallowing the probe error, in the management gate and in DELETE: killed by the probe-error test. I added that test after the first round showed both mutants survived.
  • DELETE cancel-any back to the owner: killed only by the strict-mock call pin in TestHandler_deletePlannedPurchase_UserAPIKey_Allowed. It is behaviourally equivalent through the real auth service, because fix(api): enforce cancel permission constraints on planned purchase cancel #978's key-scoped Constraints check denies anyway. That is the existing defence in depth, and this PR switches the check for consistency.
• go build ./... and go vet ./...: clean.
• go test ./internal/api/... ./internal/auth/... ./internal/server/... ./internal/mocks/... -count=1: everything passes except TestAuthPublicRoutes_EveryRouteIsCSRFGuardedOrExempt. That test fails on clean origin/main (181af4f) too, with my changes stashed. Route POST /api/purchases/approve/*/details from fix(approval): let email-link approvers load purchase details without view:purchases #990 has no CSRF classification in test(api): class-level CSRF guard for AuthPublic money routes; remove the vacuous test and its dead code #993's guard. It is unrelated to this PR and left alone.
• golangci-lint v2.10.1 run --allow-parallel-runners ./internal/api/...: 0 issues.
• gocyclo -over 10 internal/api/handler_purchases.go: no output.
• git-secrets --scan --cached over the changed files, in a temp repo with an isolated HOME: clean. bash scripts/test-git-secrets-allowlist.sh: 42 PASS, 0 FAIL.
• Not verified against a real deployment or database. The auth decision runs real auth.Service code over a mocked store.

Out of scope

• update-any and delete Constraints are still not applied on these routes. That is sec(api): planned-purchase DELETE ignores the granting permission's Constraints #961.
• getAccountScope resolves allowed accounts from the key's owner (GetAllowedAccountsAPI(session.UserID)), not from the key. That is pre-existing and separate from the verb checks; not changed here.
• Other execution endpoints (revoke, approve, execute, cancel, retry) also call HasPermissionAPI on session.UserID for their *-any / *-own verbs. I did not change them. Several are backstopped by key-scoped requirePermissionConstraints. They may deserve their own audit issue.
• The pre-existing CSRF-guard test failure on main described above.

🤖 Generated with Claude Code

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 8bbedad9-833f-4835-83da-fdbe3fb89322

📥 Commits

Reviewing files that changed from the base of the PR and between 0a5f790 and fe70fbb.

📒 Files selected for processing: 4

• internal/api/handler_purchases.go
• internal/api/handler_purchases_apikey_scope_test.go
• internal/api/handler_purchases_cancel_constraints_test.go
• internal/api/handler_purchases_test.go

+---------------------------------------+
| Preventing the Matrix from glitching. |
+---------------------------------------+
  \
   \     /)/)
        ( '')
        /  |
      o(___)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

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

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

[cristim]

Independent review of fe70fbb: draft: blocks ready

No BLOCKING findings in the changed helpers.

Fix mechanism: both helpers now call sessionHoldsVerb (handler_purchases.go:519), which already existed on main and is used for cancel-own (:504). For session.UserAPIKeyID != "" it calls HasAPIKeyPermissionForConstraintsAPI (:523, auth/service_apikeys_api.go:412), which requires both the key's effective permissions and the owner's group permissions to allow the verb. This is the same API requirePermissionConstraints uses (handler.go:514). User sessions still call HasPermissionAPI with session.UserID (:525), same as before, and the apiKeyAdminUserID early return is unchanged. Per-account/creator checks in both helpers are untouched (only the verb probes changed). Error text is unchanged ("permission check failed: ...").

Findings (all NON-BLOCKING, follow-ups; recommend one tracked issue "owner-vs-key HasPermissionAPI on remaining purchase authz helpers"):

• Same owner-vs-key confusion remains outside these helpers: handler_purchases.go:1072/1080 (approve), :1121/1128, :1418/1426 (execute), :1951/1970 (cancel, other path), :2535/2543 (retry); handler_purchases_revoke.go:314/326, :415/427 (revoke); handler_ri_exchange.go:2429/2436, :2463/2471 (approve). handler.go:313/423 are the route-level gate and they do branch to the key path at :338/:445. sessionHoldsVerb could replace these directly. Not in scope of #979.
• middleware.go:453 admin check on session.UserID: not reviewed for key semantics, include in the follow-up.

What I attacked:

• Mutation: authorizePlannedPurchaseCancel reverted to the owner check -> TestPlannedPurchaseRoutes_FullScopeFollowsTheAPIKey (delete: key delete / key cancel-own on another user's row) and ..._APIKeyFullScopeProbeErrorRefuses/delete fail. authorizeExecutionManagement reverted -> the same test fails for pause, resume, run, plus ..._ProbeErrorRefuses/pause. Both reverted, worktree clean.
• Fail-before/pass-after follows from the mutations (reverted code == pre-PR behaviour).
• go test ./internal/api/... at this SHA: 3541 passed, 1 failed: TestAuthPublicRoutes_EveryRouteIsCSRFGuardedOrExempt (POST /api/purchases/approve/*/details, router.go:168). Unrelated to the diff and main has no diff in the CSRF files, so it is most likely failing on main too (not run separately on main).

Gate status:

• Independent review: this comment, SHA above.
• CI: runs pending on this SHA (Unit Tests, E2E, etc.); fork run, not yet green.
• Labels: match #979 (priority/p1, severity/high, urgency/this-sprint, impact/few, effort/s, type/security, triaged).
• CodeRabbit: draft, not reviewed; no threads.
• Branch is behind main (merge-base a74e6ea vs main 0ed85aa); rebase may be needed.
• Local verification: mocks-based unit tests exercising the real helpers with key and user sessions; no real-account run.

Blocks ready:

1. Mark ready for review (author), CI green on the final SHA.
2. Resolve the unrelated CSRF-guard test failure (or confirm it is red on main).
3. Rebase onto current main, then this review needs a delta check.
4. File the follow-up issue for the remaining call sites listed above.

[cristim]

Gate record for fe70fbb (origin/main CLAUDE.md "Review gate"):

• Independent review of fe70fbb: no blocking findings.
  • Both planned-purchase helpers (authorizePlannedPurchaseCancel, authorizeExecutionManagement) now call the existing sessionHoldsVerb (handler_purchases.go:519). For API-key sessions, that requires both the key's and the owner's permission (HasAPIKeyPermissionForConstraintsAPI, the same call requirePermissionConstraints uses).
  • User sessions are unchanged. The admin bypass and the per-account/creator checks are unchanged.
• Follow-up: the same owner-vs-key pattern remains at other purchase/revoke/RI-exchange sites. That is tracked in sec(api): owner-vs-key HasPermissionAPI on remaining purchase authz helpers #1005 and fixed by fix(api): probe the API key, not its owner, for remaining purchase *-any/*-own verbs #1015.
• CI on fe70fbb: CI Success and Run pre-commit hooks pass. This is a fresh pull_request run (38940440546) against main after test(api): classify the approval-details route in the AuthPublic CSRF guard (fixes red main) #1003, which fixed the main-wide CSRF-guard test.
• Local verification (macOS, mocks):
  • Reverting either helper to the owner check fails TestPlannedPurchaseRoutes_FullScopeFollowsTheAPIKey (delete; pause/resume/run) and the probe-error test. This is the fail-before proof.
  • go test ./internal/api/... passes. No real account.
• CR skipped: draft and quota-limited; covered by the independent review plus local verification.
• Labels mirrored from sec(api): planned-purchase DELETE checks update-any on the key owner, not the API key's effective permissions #979.

Marking ready (owner-authorized) and merging at fe70fbb with --match-head-commit.