From 722f474ae527e6ea0bfa4db693b1016562824575 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sat, 8 Aug 2026 09:17:12 +0200
Subject: [PATCH] fix(test): stop using a carved-out verb as a generic ceiling
 fixture

TestGrantCeiling_ConstraintContainment used execute:ri-exchange as its
fixture verb to exercise general grant-ceiling constraint containment
(narrower-allowed, cap-drop-refused, unheld-provider-refused). PR #1758
then added {execute, ri-exchange} to adminCarvedOuts.

checkGrantCeiling (internal/auth/group_ceiling.go) checks adminCarvedOuts
before the containment logic in grantCeilingAllows, so once the fixture's
verb became carved out, the test's target group (no existing permissions)
made every subtest refuse with ErrPermissionNotGrantable before the
containment logic it exists to exercise ever ran. Both #1737 and #1758
were correct in isolation; the conflict is emergent from their merge
order.

Swap the fixture verb for view:plans, already used elsewhere in this
file, and hoist it to named constants with an assertion that it is not
in adminCarvedOuts -- so the next carve-out addition that collides fails
loudly at the fixture instead of three subtests dying somewhere that
looks unrelated. checkGrantCeiling's carve-out-before-containment
ordering is untouched; it is correct and deliberate.

Refs #1737, #1758
---
 .../auth/group_ceiling_permissions_test.go    | 29 ++++++++++++++-----
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/internal/auth/group_ceiling_permissions_test.go b/internal/auth/group_ceiling_permissions_test.go
index 2d1b564e6..b4318b5b9 100644
--- a/internal/auth/group_ceiling_permissions_test.go
+++ b/internal/auth/group_ceiling_permissions_test.go
@@ -216,14 +216,29 @@ func TestGrantCeiling_InCeilingUpdateSucceeds(t *testing.T) {
 
 // TestGrantCeiling_ConstraintContainment: a constrained holder may hand out a
 // narrower grant but not a broader one.
+//
+// The fixture verb is deliberately NOT one of adminCarvedOuts: this test
+// exercises the general containment logic in grantCeilingAllows, which
+// checkGrantCeiling only reaches for a pair the carve-out check lets through.
+// A carved-out verb refuses unconditionally before containment is ever
+// evaluated (see TestGrantCeiling_CarvedOutNotGrantable), which is exactly
+// what happened here when this test used execute:ri-exchange as its fixture:
+// PR #1758 carved that pair out and every subtest below started failing for
+// the wrong reason. The assertion below fails loudly, at the fixture, if a
+// future carve-out addition collides with this verb again.
 func TestGrantCeiling_ConstraintContainment(t *testing.T) {
 	ctx := context.Background()
 
+	const fixtureAction, fixtureResource = ActionView, ResourcePlans
+	require.False(t, adminCarvedOuts[[2]string{fixtureAction, fixtureResource}],
+		"fixture verb %s:%s must not be carved out, or the carve-out check "+
+			"refuses before this test's containment logic ever runs", fixtureAction, fixtureResource)
+
 	held := []Permission{
 		{Action: ActionUpdate, Resource: ResourceGroups},
 		{
-			Action:   ActionExecute,
-			Resource: ResourceRIExchange,
+			Action:   fixtureAction,
+			Resource: fixtureResource,
 			Constraints: &PermissionConstraints{
 				Providers:         []string{"aws"},
 				MaxPurchaseAmount: 100,
@@ -242,8 +257,8 @@ func TestGrantCeiling_ConstraintContainment(t *testing.T) {
 
 		_, err := svc.UpdateGroupAPI(ctx, ceilingActorID, ceilingTargetID,
 			updateReqWith(APIPermission{
-				Action:   ActionExecute,
-				Resource: ResourceRIExchange,
+				Action:   fixtureAction,
+				Resource: fixtureResource,
 				Constraints: &APIPermissionConstraint{
 					Providers: []string{"aws"},
 					MaxAmount: 50,
@@ -261,7 +276,7 @@ func TestGrantCeiling_ConstraintContainment(t *testing.T) {
 		stubTargetGroup(ctx, mockStore, &Group{ID: ceilingTargetID, Name: "Team"})
 
 		_, err := svc.UpdateGroupAPI(ctx, ceilingActorID, ceilingTargetID,
-			updateReqWith(APIPermission{Action: ActionExecute, Resource: ResourceRIExchange}))
+			updateReqWith(APIPermission{Action: fixtureAction, Resource: fixtureResource}))
 
 		require.Error(t, err)
 		assert.ErrorIs(t, err, ErrPermissionCeiling)
@@ -278,8 +293,8 @@ func TestGrantCeiling_ConstraintContainment(t *testing.T) {
 
 		_, err := svc.UpdateGroupAPI(ctx, ceilingActorID, ceilingTargetID,
 			updateReqWith(APIPermission{
-				Action:   ActionExecute,
-				Resource: ResourceRIExchange,
+				Action:   fixtureAction,
+				Resource: fixtureResource,
 				Constraints: &APIPermissionConstraint{
 					Providers: []string{"aws", "azure"},
 					MaxAmount: 50,