fix(test): stop using a carved-out verb as a generic ceiling fixture

[cristim]

Summary

main is red: TestGrantCeiling_ConstraintContainment in internal/auth/group_ceiling_permissions_test.go fails (3 subtests + parent), blocking CI on every open PR.

Root cause: an emergent conflict between two independently-green PRs, not a logic bug.

• The test uses execute:ri-exchange as its fixture verb to exercise general grant-ceiling constraint containment (narrower grant allowed / cap-drop refused / unheld-provider-add refused), against a target group with an empty existing-permissions list.
• sec(auth): carve execute:ri-exchange out of admin:* and seed the granting group #1758 then added {ActionExecute, ResourceRIExchange} to adminCarvedOuts.
• checkGrantCeiling (internal/auth/group_ceiling.go:73-96) checks adminCarvedOuts before the containment logic in grantCeilingAllows. Once the fixture's verb became carved out, and the target group didn't already hold it, every subtest hit the unconditional carve-out refusal (ErrPermissionNotGrantable) before the containment logic it exists to test ever ran.

Both #1737 (grant ceiling) and #1758 (RI-exchange carve-out) are correct in isolation. Neither is at fault; the merge sequencing created a stale fixture.

Fix

Test-only. Swapped the fixture verb from execute:ri-exchange to view:plans (already used elsewhere in the same file, not a carved-out pair) and hoisted it to named constants with an assertion that the verb is not in adminCarvedOuts, so the next carve-out addition that collides with a generic fixture fails loudly at the fixture instead of three subtests dying somewhere that looks unrelated.

checkGrantCeiling's carve-out-before-containment ordering is untouched -- verified git diff 7c4e8405c origin/main -- internal/auth/group_ceiling.go is empty, so the squash-merge did not alter it. That ordering is correct and deliberate (issue #923 separation of duties): a carved-out verb must refuse unconditionally regardless of what containment logic would otherwise allow.

Sweep for latent occurrences of the same defect

adminCarvedOuts currently holds four pairs: {execute,purchases}, {approve-any,purchases}, {retry-any,purchases}, {execute,ri-exchange}. Searched every construction of a Permission/APIPermission value across internal/auth and the rest of the repo for a carved-out pair used as a generic fixture (as opposed to a test that is deliberately about carve-out behavior).

Fixed: TestGrantCeiling_ConstraintContainment (this PR).

Inspected and deliberately left alone (each uses a carved-out pair on purpose, to test the carve-out itself):

• internal/auth/group_ceiling_permissions_test.go: TestGrantCeiling_CarvedOutNotGrantable, TestGrantCeiling_CarvedOutNotGrantableByPurchaserAdmin, TestGrantCeiling_CarvedOutMayBeKeptNotWidened -- the sec(auth): the #923 money separation-of-duties carve-out is voidable by any admin in one request #1550 attack scenarios.
• internal/auth/group_system_managed_test.go: TestSystemManagedGroup_Immutable.
• internal/auth/self_escalation_carveout_test.go: all TestSelfCarvedOutGrant_* tests.
• internal/auth/service_group_only_authz_test.go: TestGroupOnlyAuthz_AdminEquivalence, TestGroupOnlyAuthz_NonAdminDenied -- admin-wildcard-vs-carve-out coverage.
• internal/auth/service_group_test.go: TestHasPermission_RegionConstraintDeniesUnpermittedTargetRegion -- RI-exchange region-matching, calls HasPermission directly, not the ceiling.
• internal/auth/types_test.go: adminCarvedOuts / HasPermission coverage.
• internal/api/grantadmin_carveout_test.go, internal/api/ri_exchange_carveout_test.go: handler-level carve-out enforcement tests.
• internal/database/postgres/migrations/000096_seed_ri_exchanger_group_test.go: the seeded-group migration test.

None of these are affected, today or on a future carve-out addition, because their subject is carve-out behavior.

internal/auth/group_ceiling_validation_test.go was also checked -- it only uses ActionView/blank fields, no carved-out pairs.

Verification

• go test ./internal/auth/... -- 735 passed (was 4 failing, now 0).
• go build ./..., go vet ./..., go vet -tags=integration ./... -- clean.
• go vet -tags=e2e ./... in tests/e2e -- clean.
• go test -race -short ./... in ., pkg, providers/aws, providers/azure, providers/gcp -- all green.
• gocyclo -over 10 -ignore "_test\.go" . -- 0 findings.
• golangci-lint run at CI-pinned v2.10.1 -- exit 0 and a genuine 0 issues. line (not just exit 0, per the known v2 false-clean modes on a removed flag / concurrent-run lock).

Test plan

• internal/auth full package green
• Six-module build/vet/test gate green
• gocyclo clean
• golangci-lint v2.10.1 clean (genuine 0 issues.)

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 43 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e4a5f2d5-0ef2-4e1f-a4a6-e54de1a3349a

📥 Commits

Reviewing files that changed from the base of the PR and between 6d5275c and 722f474.

📒 Files selected for processing (1)

• internal/auth/group_ceiling_permissions_test.go

---

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

[cristim]

Merging immediately to unblock main. All 20 checks complete, zero failures, zero unresolved threads.

Merging without an independent review round, deliberately. This is a test-only fix (one file, +22/-7) restoring a red main that is corrupting CI signal on every open PR and on three in-flight implementations. I verified the fix myself before it was pushed — go test ./internal/auth/ -count=1 -> ok 6.287s. The cost of main sitting red exceeds the marginal value of another pass on a 22-line test change.

Root cause: an emergent conflict between two independently-green PRs. TestGrantCeiling_ConstraintContainment used execute:ri-exchange as its fixture verb for exercising general constraint-containment logic, with a target group stub carrying no existing permissions. #1758 then added {ActionExecute, ResourceRIExchange} to adminCarvedOuts.

checkGrantCeiling checks adminCarvedOuts first, before grantCeilingAllows — so the verb was refused unconditionally with the #923 separation-of-duties error and the test never reached the logic it exists to exercise. Neither PR was wrong. Merging them in sequence without testing the combination was.

It never failed to compile. go build ./..., go vet ./... and go vet -tags=integration ./... all exit 0 on the broken main, and the test binary links (go test -run XXXNONEXISTENT ./internal/auth/ -> ok). Production behaviour was correct throughout — checkGrantCeiling refusing a carved-out verb ahead of containment is exactly right. Only the fixture was stale.

Confirmed before fixing, so the blast radius was known: internal/auth is the only affected package — a full package run showed TestGrantCeiling_ConstraintContainment as the sole failure. And git diff 7c4e8405c origin/main -- internal/auth/group_ceiling.go is empty, so the squash-merge did not alter checkGrantCeiling; its ordering is exactly as reviewed and is untouched here.

The fix hoists the fixture verb to named constants and asserts it is not carved out:

const fixtureAction, fixtureResource = ActionView, ResourcePlans
require.False(t, adminCarvedOuts[[2]string{fixtureAction, fixtureResource}],
    "refuses before this test's containment logic ever runs", ...)

That second line is the part that matters. adminCarvedOuts gained a member this morning and will gain more; the next addition now fails loudly at the fixture, naming the cause, instead of three subtests dying somewhere that looks unrelated. It fixes the class rather than the instance.

Follow-up owed: any other test using a carved-out pair as a generic fixture is latent even where it passes today. That sweep is bounded to what is currently green, so it is not urgent — but it should be recorded rather than assumed complete.

Unblocks #1764, #1767, #1770, #1771, and the held sec/1758-review-followup branch, all of which currently inherit this failure.