test(api): cover retry-any:purchases carve-out at the retry handler

[cristim]

Closes #1743

What

Handler-level coverage for the retry-any:purchases carve-out (#923) at the retry handler.
Test-only: one new file, no production code touched.

The issue's premise was wrong, and the reason is the finding

#1743 was filed on the possibility that this verb might be unenforced. It is not. Full premise
correction with evidence is on the issue:
#1743 (comment)

Please read that before reviewing, so the disproved "no non-test reference in internal/api" claim
does not get re-derived from scratch.

Short version. The verb is enforced at internal/api/handler_purchases.go:1962 and the path is
reachable:

POST /api/purchases/retry/{id}          router.go:178  (Auth: AuthUser)
  -> Handler.retryPurchase              handler_purchases.go:1611
  -> loadAndValidateRetryRequest        handler_purchases.go:1672
  -> Handler.authorizeSessionRetry      handler_purchases.go:1699 -> 1952
  -> h.auth.HasPermissionAPI(ActionRetryAny, ResourcePurchases)   handler_purchases.go:1962
  -> auth.Service.permissionsAllow      service_group.go:325
  -> adminCarvedOuts consulted          service_group.go:328

Why the gap existed

retry-any is the only one of the three carved-out money verbs whose enforcement point is not
requirePermission, so the handler coverage added by #1744 sits at a seam this path never crosses.

Every existing retryPurchase test goes through buildSessionRetryHandler
(handler_purchases_test.go:3053-3076), which hand-registers the decision with a caller-supplied
boolean and the bare literal "retry-any" rather than auth.ActionRetryAny. The real matcher, and
with it adminCarvedOuts, is never consulted on this path. That is also why instrumenting
grantAdmin recorded zero asks for the pair.

Measured: removing retry-any from adminCarvedOuts fails only the generic requirePermission
test from #1744. Zero retryPurchase tests notice.

REVIEWERS: please do not simplify the message assertion

TestRetryCarveOut_PlainAdminIsRefused asserts that err.Error() contains
"another user's failed purchase". That single line is the regression barrier.

Dropping retry-any from adminCarvedOuts lets the caller past the RBAC gate, but retryPurchase
then hits the SEC-01 constraint gate, which asks execute:purchases, still carved out, and refuses
with a different message and still a 403. So a bare require.Error passes against the mutated code
and asserts nothing.

Relaxing that assert.Contains deletes the coverage with no test going red. There is a comment on
the line saying exactly this; please keep both.

What the tests do

Two tests differing in the PRINCIPAL only, same failed row, same request, same fixture, so the
carve-out is the sole variable:

• TestRetryCarveOut_PlainAdminIsRefused: admin:* is refused.
• TestRetryCarveOut_AdminPurchaserIsAllowed: admin + Purchaser is allowed, successor written and
  linked to the original.

Both drive the real retryPurchase end to end against the real matcher, rather than
requirePermission. The fixture row is created by a different user on purpose: a creator-owned row
is reachable via retry-own, which is deliberately not carved out, so only a foreign row isolates
retry-any as the deciding grant.

Verification

• Mutation (remove the verb from adminCarvedOuts): TestRetryCarveOut_PlainAdminIsRefused
  fails by assertion, not panic. Restored, both pass. Re-run against the committed code.
• go build ./... passes.
• go test ./...: 7073 passed, 0 failed, 43 packages.
• golangci-lint at the CI-pinned v2.10.1: 0 issues, exit 0. Checked at the pin deliberately,
  since a newer local version returns a false exit 0.
• internal/mocks.TestNoUnfailableMockAssertions passes. It rejected the first draft, where the
  mocks were returned from a helper via multi-value assignment and the guard could not resolve the
  receivers; each test now binds new(MockAuthService) in its own scope so the sites are actually
  checked rather than reported as unanalyzable.

Vacuity traps handled explicitly: AssertExpectations via t.Cleanup; a positive AssertCalled on
the retry-any pair before any AssertNotCalled, so the negative cannot be satisfied by a request
that never reached the permission gate; and the name-only AssertNotCalled form on
MockConfigStore, which shadows that helper (internal/mocks/assertions.go:154) and reads a call
log where name-only is the strongest form rather than the vacuous one.

Known residual gap, pre-existing and not introduced here

The tests exercise AuthContext.HasPermission through the mock, but production reaches the
carve-out via Service.permissionsAllow. Both consult adminCarvedOuts and both were read to
confirm it, but nothing pins them in sync: a regression removing the check from permissionsAllow
alone would not be caught. Identical in shape for the two sibling verbs and for #1644, so it may
warrant a separate issue.

Label note

The type/security and severity/high labels were applied on the unenforced-control premise, which
does not hold. Labels are mirrored from the issue as-is here; re-triage is the maintainer's call and
is flagged on the issue thread rather than acted on.

Related: #1596, #1744, #923, #1644

Summary by CodeRabbit

• Tests
  • Added regression coverage for purchase retry permissions.
  • Verified unauthorized retries are rejected with a clear error.
  • Verified appropriately authorized retries succeed and retain their retry linkage.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 44a21d1f-5d37-4578-9275-9120c0b508c4

📥 Commits

Reviewing files that changed from the base of the PR and between 9be12fc and 958068f.

📒 Files selected for processing (1)

• internal/api/retry_carveout_test.go

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

📝 Walkthrough

Walkthrough

The PR adds handler-level regression tests for retry-any:purchases. The tests verify that plain admins receive a 403, while admins with explicit retry-any permission create and persist a retry successor.

Changes

Retry authorization coverage

Layer / File(s)	Summary
Handler authorization and retry persistence internal/api/retry_carveout_test.go	Adds fixtures and permission wiring. Tests deny plain admin retries with the expected 403 and creator-check error. Tests allow explicit retry-any permission and verify successor and linkage writes.

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

Merge Risk: ⚪ Minimal · up to 95806

This is a localized test-only change with build, test, and lint checks reported as passing; no actionable merge-blocking risk remains.

Possibly related PRs

• LeanerCloud/CUDly#216: Adds handler-level tests for a different purchase authorization carve-out.
• LeanerCloud/CUDly#1744: Adds related handler-level authorization carve-out tests using the real permission matcher.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies handler-level tests for the retry-any:purchases carve-out.
Linked Issues check	✅ Passed	The tests cover refusal and allowance through the real matcher and use a foreign failed purchase to isolate retry-any:purchases.
Out of Scope Changes check	✅ Passed	The pull request adds only handler-level regression tests directly related to issue #1743 and does not modify production code.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch sec/1743-retry-any-purchases-enforcement

---

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