sec(auth): carve out money-verb wildcards at grant and check

[cristim]

What

Closes #1901.

The separation-of-duties carve-out that stops an admin granting money verbs was keyed on exact (action, resource) pairs, while enforcement treats a stored Resource == "*" as matching every resource. An admin holding only admin:* could therefore create a group carrying execute:*, approve-any:* and retry-any:*: the ceiling looked up the wildcard form, found nothing, and allowed the write. Every member of that group then passed the concrete execute:purchases, approve-any:purchases, retry-any:purchases and execute:ri-exchange checks.

The fix adds one predicate, coversCarvedOut(perm), which asks enforcement's own matcher whether a permission would satisfy any carved-out pair, and replaces all five exact-pair lookups with it. Grant-time guards and enforcement now agree by construction rather than by both being written correctly.

Source diff is +21/-5 across four files. The rest is tests.

The four guards

The issue named three. A fourth was found while planning, and it is the one that matters for already-issued credentials.

Guard	Site	What it stops
Group grant ceiling	group_ceiling.go:85	creating or updating a group with a money-verb wildcard
Self-membership	service_user.go:551	joining yourself to such a group
API-key creation	types.go:167	minting a key carrying the wildcard
Effective-permission intersection	service_group.go:332	an already stored wildcard key granting the verb at use time

Verification

Each guard has a regression test that was observed failing before the fix and passing after. The refusal tests stub no write on the mock, so a missing guard panics on an unexpected CreateGroup, UpdateUser or CreateAPIKey rather than passing quietly.

An independent reviewer ran ten mutants of the predicate against the auth suite. Nine were killed. The survivor is the mirror branch in grantCeilingAllows, which the code documents as unreachable for carved-out pairs because checkGrantCeiling refuses them first, so it is an equivalent mutant rather than a coverage gap.

Check	Result
go build ./..., go vet	exit 0
go test ./internal/auth/	799 passed
go test ./internal/api/	2189 passed
go test ./internal/...	5938 passed, 23 packages
golangci-lint at the CI pin (v2.10.1, Go 1.26.6)	0 issues
gocyclo -over 10 on the touched files	clean

No legitimate grant regresses. Every up-migration was checked: the only wildcard-resource permission ever seeded is {admin,*}. Purchaser seeds the three concrete purchases pairs and RI Exchanger seeds execute:ri-exchange, so nothing in a default deployment relies on the wildcard form.

Operator action required after merge

This fix does not neutralise data already written through the bypass. A group that already stores execute:* keeps granting the money verbs to its members, and narrowing it through the API is now refused, so it must be corrected in SQL or the group deleted. Find affected rows with:

SELECT id, name FROM groups, jsonb_array_elements(permissions) p
 WHERE p->>'resource' = '*' AND p->>'action' IN ('execute','approve-any','retry-any');

SELECT id, user_id, name FROM api_keys, jsonb_array_elements(permissions) p
 WHERE p->>'resource' = '*' AND p->>'action' IN ('execute','approve-any','retry-any');

API keys behave differently from groups and need no SQL: a stored wildcard key whose owner is an admin loses the execute verb entirely at use time rather than being narrowed to execute:purchases. That is fail-closed and intended, but a key that worked yesterday can stop executing, so it is worth knowing before someone reports it as a regression.

Scope

This closes the wildcard forms only. LeanerCloud/cloud-commitments-platform#226 remains open and is the other route to spend authority: membership writes have no ceiling, so an admin can still create a user directly inside the Purchaser group, or use update:users to move an account into it. That is a different defect and is not addressed here.

Summary by CodeRabbit

• Bug Fixes

  • Corrected authorization checks for wildcard permissions so restricted actions cannot be granted through wildcard resource patterns.
  • Prevented unauthorized wildcard permissions during group changes, self-membership updates, and API-key creation.
  • Ensured existing administrative API keys no longer retain restricted wildcard permissions in their effective access.
  • Preserved permitted behavior for explicitly authorized users and unrelated administrative permissions.

• Tests

  • Added comprehensive coverage for wildcard permission enforcement and edge cases.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Essentials

Run ID: 6d1f60ec-63fb-4cbc-ac38-9d2de2a37993

📥 Commits

Reviewing files that changed from the base of the PR and between fdf9c29 and 7277591.

📒 Files selected for processing (7)

• internal/auth/carveout_wildcard_test.go
• internal/auth/group_ceiling.go
• internal/auth/group_ceiling_permissions_test.go
• internal/auth/service_group.go
• internal/auth/service_user.go
• internal/auth/types.go
• internal/auth/types_test.go

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

---

📝 Walkthrough

Walkthrough

The change adds wildcard-aware carve-out matching for authorization checks. Grant ceilings, group permissions, self-membership, API-key creation, and effective permissions now enforce carved-out money actions. Regression tests cover wildcard and explicit permission cases.

Changes

Wildcard carve-out enforcement

Layer / File(s)	Summary
Shared carve-out matching internal/auth/types.go, internal/auth/types_test.go	coversCarvedOut matches permissions using the existing permission matcher. HasPermission and matcher tests now handle wildcard resources and exact-match boundaries.
Authorization path integration internal/auth/group_ceiling.go, internal/auth/service_group.go, internal/auth/service_user.go, internal/auth/group_ceiling_permissions_test.go	Grant ceilings, admin permission checks, self-membership validation, and fixture guards now use coversCarvedOut.
Wildcard enforcement regression coverage internal/auth/carveout_wildcard_test.go	Tests reject wildcard money permissions during group changes, self-membership, and API-key creation. Tests also verify effective-permission removal and permitted explicit wildcard holders.

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

Severity of issue fixed: High

Merge Risk: ⚪ Minimal · up to 72775

Wildcard money permissions no longer bypass separation-of-duties protections across group, membership, API-key, and effective-permission paths. The change is ready to merge.

🚥 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 describes the primary change: enforcing carve-outs for money-verb wildcard permissions during grant and permission checks.
Linked Issues check	✅ Passed	The changes satisfy issue [#1901]. The shared coversCarvedOut matcher closes wildcard bypasses across grant validation, self-membership, API-key permission handling, effective permissions, and enforce…
Out of Scope Changes check	✅ Passed	The changes remain within the scope of issue [#1901]. They update carve-out matching logic and add focused regression tests. No unrelated membership-write changes are included.
Docstring Coverage	✅ Passed	Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 7 files.

✨ 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 fix/1901-auth-wildcard-carveout

---

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