[cristim]

Summary

Closes #678. Stacked on #975 (base fix/cancel-permission-constraints); retarget to main after #975 merges.

DELETE /api/purchases/planned/{id} (authorizePlannedPurchaseCancel) also cancels pending, notified and paused executions. It used to gate on bare cancel-any and on a creator match, so a constrained cancel-any holder who was denied by #975 on POST /cancel could still cancel the same execution here. This PR closes that bypass for the cancel verbs:

• cancel-any branch: uses the same strict per-recommendation constraint sets (revokeExecutionConstraintSets plus requirePermissionConstraints). It falls back to the creator branch only on a 403. Store errors propagate.
• Creator branch: keeps today's semantics (creator match alone, with its own "scheduled purchase" message), so a creator holding delete:purchases without cancel-own can still delete. Cancel-own Constraints apply only when the principal actually being evaluated holds cancel-own.
  • For API-key sessions that principal is the key. sessionHoldsVerb probes the key's effective permissions, which already require the owner's grant as well. A delete-only key keeps working, and a probe error refuses the request; it never falls back.
• The update-any and delete branches are unchanged and tracked by sec(api): planned-purchase DELETE ignores the granting permission's Constraints #961.
• No change to 404/403 concealment for callers without a verb. A missing row still returns errNotFound.
• wrapConstraintDenied is kept on the cancel-any path for consistency. DELETE planned has no token fall-through, so the wrap is inert there; a comment says so.

Behaviour change for API keys: on this route, a key session's cancel-any gate now follows the key. A delete-only key whose owner holds cancel-any can no longer cancel other users' executions through the owner's privilege. This tightens access and matches #975 and #957.

Tests

Added to handler_purchases_cancel_constraints_test.go:

• Route level, no writes on deny:
  • cancel-any constrained out of scope is denied;
  • unconstrained cancel-any is allowed;
  • fallback through cancel-own;
  • an update-any control.
• Creator branch:
  • a creator with delete and no cancel-own still deletes;
  • a creator whose constrained cancel-own is out of scope is denied.
• API keys:
  • TestDeletePlannedPurchase_APIKeyCancelOwnFollowsTheKey: a delete-only key with owner cancel-own allowed, an out-of-scope key denied, an in-scope key allowed.
  • TestDeletePlannedPurchase_APIKeyVerbProbeErrorRefuses.
• TestDeletePlannedPurchase_ConstraintStoreErrorDoesNotFallBack.

The existing router_660 creator-with-delete test now stubs the new probe. Its assertion is unchanged.

Mutants, all killed:

• dropping the planned constraint call;
• falling back on any error;
• reverting key sessions to the owner's permission;
• swallowing a probe error as "doesn't hold".

Verification (local, macOS)

• go test -race -count=1 ./internal/api/... ./internal/auth/... ./internal/server/... passes (4893 tests at ecbf996; ./internal/api passes at bbad6eb).
• golangci-lint v2.10.1 reports 0 issues, and the pre-commit gocyclo hook (threshold 10) passes.

Review

• The plan had two independent passes plus an F7 revision.
• This diff had two independent passes. Pass 1 found that delete-only API keys would regress, and that is fixed in ecbf996. Pass 2 approved ecbf996. bbad6eb adds only the probe-error test the reviewer asked for.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Planned-purchase cancellation now respects permission constraints for both broad and creator-specific cancellation access.
  • API-key cancellation checks use the key’s own permissions and constraints.
  • Permission-check errors now prevent cancellation rather than allowing it to proceed.

[coderabbitai]

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: 3e79dd6c-0942-4415-8477-bfd0e2f3db5a

📥 Commits

Reviewing files that changed from the base of the PR and between f08485d and 74c0b62.

📒 Files selected for processing: 4

• internal/api/handler_purchases.go
• internal/api/handler_purchases_cancel_constraints_test.go
• internal/api/handler_purchases_test.go
• internal/api/router_660_permission_flips_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

Planned-purchase deletion now evaluates cancel-any and cancel-own Constraints against the stored execution’s recommendations. Creator authorization remains available after a cancel-any denial. User-API-key sessions use the key’s effective permissions. update-any remains an unconditional bypass.

Changes: Planned-purchase cancellation authorization

Layer / File(s)	Summary
Deletion authorization and regression coverage internal/api/handler_purchases.go, internal/api/handler_purchases_cancel_constraints_test.go, internal/api/handler_purchases_test.go, internal/api/router_660_permission_flips_test.go	The handler checks cancel-any Constraints and can fall back to creator authorization, where held cancel-own permissions are also checked. User-API-key sessions use the key’s effective permissions. Tests cover scope outcomes, permission-probe errors, and cases where deletion status does not change. update-any remains an unconditional bypass.

Priority: ⬇️ Low

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Handler as Planned-purchase DELETE handler
  participant Store as Execution store
  participant Constraints as Permission constraint check
  participant Probe as API-key permission probe
  Handler->>Store: Load execution and recommendations
  Handler->>Constraints: Check cancel-any constraints
  alt cancel-any denied with 403
    Handler->>Probe: Check effective cancel-own permission for API-key session
    Handler->>Constraints: Check cancel-own constraints for creator
  end

Loading

Suggested reviewers: iamjsd

Merge Risk: ⚪ Minimal · up to 74c0b

No blocking issue was identified for planned-purchase cancellation; the change is mergeable after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: enforcing cancel permission constraints for planned purchase cancellation.
Linked Issues check		Active direct issue #678 requires constraint checks for cancel-any and cancel-own against stored execution data, with fallback from cancel-any to cancel-own only on a 403. The reported implementation …
Out of Scope Changes check		The changed handler code implements #678 authorization behavior. The added and adjusted tests verify that behavior and update existing mocks for the new permission probe. The reported unchanged update…

Full details: Linked Issues check

Explanation

Active direct issue #678 requires constraint checks for cancel-any and cancel-own against stored execution data, with fallback from cancel-any to cancel-own only on a 403. The reported implementation applies these checks to planned-purchase DELETE, keeps store and probe errors fail-closed, evaluates API-key sessions as the key, and preserves creator semantics. The added tests cover constraint dimensions, fallback behavior, API-key behavior, error propagation, and no writes after denial.

Full details: Out of Scope Changes check

Explanation

The changed handler code implements #678 authorization behavior. The added and adjusted tests verify that behavior and update existing mocks for the new permission probe. The reported unchanged update-any and delete branches remain outside this change and do not establish an unrelated change.

• 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

• 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 verdict: CLEAN at bbad6eb

Reviewer: the same fresh Claude security reviewer as #975 (Sonnet, high effort), not the author, cold brief. It reviewed the delta 75b906ff..bbad6ebc, and the PR body was treated as a claim. Coordinator: claude-cc-reviewer.

Blocking findings: none. On the planned DELETE route, cancel-any and cancel-own are now constrained by the stored execution's scope and evaluated on the session's effective permissions.

Non-blocking, pre-existing (not introduced here):

1. handler_purchases.go ~:441-448: update-any still returns early with no Constraints, and it is checked on the key owner (HasPermissionAPI(session.UserID)) rather than the API key's effective permissions. A user-API-key session holding only a narrow verb can cancel any planned purchase if its owner holds update-any. The same owner-only check exists on main for both update-any and cancel-any; this PR fixes the cancel-any half. The escalation is filed as sec(api): planned-purchase DELETE checks update-any on the key owner, not the API key's effective permissions #979, and the Constraints half is sec(api): planned-purchase DELETE ignores the granting permission's Constraints #961.

2. ~:496-506: the delete:purchases verb's Constraints are not evaluated (sec(api): planned-purchase DELETE ignores the granting permission's Constraints #961). Exposure is limited, because delete holders can only cancel their own executions.

3. Test gaps: two mutations survive with 0 failures:

   • skipping the cancel-own Constraints entirely on this route;
   • dropping the cancel-own wrapConstraintDenied there, which is harmless because the route has no token branch.

   The fix(api): enforce cancel permission constraints on session cancels #975 finding 2 test (the token fall-through) is also still missing.

Entry points after #975 + #978: session cancel, cancelPurchaseViaSession, the revoke token route's session branch, and planned DELETE cancel-any/cancel-own are covered. /{id}/revoke is covered by #681. Planned DELETE update-any/delete are exempt and tracked in #961/#979. Token/email cancel, SQS cancel and the revoke token branch are excluded by design. Pause/resume are not cancel transitions. No row is missed.

What was attacked and how (local macOS, unit tests with mocks):

• go vet ./internal/api/ is clean. go test -race -count=1 ./internal/api/... ./internal/auth/... passes.
• Fail-before: with handler_purchases.go reverted to 75b906ff, -run 'Cancel|Revoke|Planned|Delete' has 4 failures. With the fix it has 0.
• Fallback: cancel-any falls back to the creator branch only on 403. A non-creator gets the terminal constraint denial, and store and auth errors propagate.
• Effective permissions: for user-API-key sessions, sessionHoldsVerb probes the key's effective permissions. Empty or unknown scope is strict, so it fails closed.
• Not-found path: a missing execution gives errNotFound, and a caller with no verb is still rejected at requireDeleteOrCancelPurchasePermission. The 404/403 comparison against main was done by code reading only.
• Mutation probes: "fall back on any error" and "drop the cancel-any constraint call" are killed; the two above survive.

Gate:

• Labels are mirrored from sec(auth): enforce per-permission Constraints on cancel-any/cancel-own (SEC-01 follow-up to #404) #678.
• Merge order is fix(api): enforce cancel permission constraints on session cancels #975 first, then this PR. It will then be retargeted to main and rebased, with an empty git range-diff of its own commits required to keep this verdict, and CI must be green at that head.
• sec(auth): enforce per-permission Constraints on cancel-any/cancel-own (SEC-01 follow-up to #404) #678 may auto-close when fix(api): enforce cancel permission constraints on session cancels #975 merges, because GitHub links it. This PR completes it immediately after.

[cristim]

Retargeted to main after #975 merged (f08485d) and rebased with git rebase --onto origin/main 75b906ff.

1:  33e997f2 = 1:  0b34af12 fix(api): enforce cancel permission constraints on planned purchase delete
2:  ecbf9966 = 2:  093f4e23 fix(api): keep delete-only API keys working on planned cancel
3:  bbad6ebc = 3:  74c0b623 test(api): refuse planned cancel when the key verb probe errors

All three commits are identical, so the CLEAN verdict above carries over to 74c0b62. A local go build ./... and go test -run 'Cancel|Planned|Delete|Revoke' ./internal/api/ pass on the rebased tree. Merge waits on CI at this head. #678 was reopened after #975 auto-closed it, and this PR closes it. (claude-cc-reviewer)