fix(reconcile): tighten permission name and namespace validation to SpiceDB grammar

internal/api/v1beta1connect/permission.go

@@ -26,11 +26,12 @@ func (h *ConnectHandler) CreatePermission(ctx context.Context, request *connect.
 		if permName == "" || permNamespace == "" {
 			return nil, connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation)
 		}
-		if !schema.IsValidPermissionName(permName) {
-			return nil, connect.NewError(connect.CodeInvalidArgument, errors.New("permission name cannot contain special characters"))
-		}
-		if !schema.IsValidPermissionNamespace(permNamespace) {
-			return nil, connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation)
+		// One shared check for the verb grammar, the namespace grammar, the reserved
+		// verbs, and the slug length, so the create API and the reconcile plan agree
+		// and a permission that would fail when SpiceDB compiles the schema is
+		// rejected up front with a message that says which rule it broke.
+		if err := schema.ValidateCustomPermission(permNamespace, permName); err != nil {
+			return nil, connect.NewError(connect.CodeInvalidArgument, err)
 		}
 		permissionSlugs = append(permissionSlugs, schema.FQPermissionNameFromNamespace(permNamespace, permName))
 
@@ -123,11 +124,10 @@ func (h *ConnectHandler) UpdatePermission(ctx context.Context, request *connect.
 	if permNamespace == "" || permName == "" {
 		return nil, connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation)
 	}
-	if !schema.IsValidPermissionName(permName) {
-		return nil, connect.NewError(connect.CodeInvalidArgument, errors.New("permission name cannot contain special characters"))
-	}
-	if !schema.IsValidPermissionNamespace(permNamespace) {
-		return nil, connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation)
+	// Same shared check as create, so an update cannot rename a permission to a key
+	// SpiceDB will reject and then break the next schema compile at boot.
+	if err := schema.ValidateCustomPermission(permNamespace, permName); err != nil {
+		return nil, connect.NewError(connect.CodeInvalidArgument, err)
 	}
 	updatedPermission, err := h.permissionService.Update(ctx, permission.Permission{
 		ID:          request.Msg.GetId(),

internal/api/v1beta1connect/permission_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
 	"testing"
 	"time"
 
@@ -24,22 +25,22 @@ var (
 	testPermissions   = []permission.Permission{
 		{
 			ID:          uuid.New().String(),
-			Name:        "Read",
+			Name:        "read",
 			NamespaceID: "app/resource",
 			Metadata:    map[string]any{},
 			CreatedAt:   time.Time{},
 			UpdatedAt:   time.Time{},
 		},
 		{
 			ID:          uuid.New().String(),
-			Name:        "Write",
+			Name:        "write",
 			NamespaceID: "app/resource",
 			CreatedAt:   time.Time{},
 			UpdatedAt:   time.Time{},
 		},
 		{
 			ID:          uuid.New().String(),
-			Name:        "Manage",
+			Name:        "manage",
 			NamespaceID: "app/resource",
 			CreatedAt:   time.Time{},
 			UpdatedAt:   time.Time{},
@@ -198,6 +199,105 @@ func TestHandler_CreatePermission(t *testing.T) {
 			}),
 			wantErr: nil,
 		},
+		{
+			name: "should return success if permission service return nil error with permission key",
+			setup: func(as *mocks.PermissionService, bs *mocks.BootstrapService) {
+				bs.EXPECT().AppendSchema(mock.AnythingOfType("context.backgroundCtx"), schema.ServiceDefinition{
+					Permissions: []schema.ResourcePermission{
+						{
+							Name:      testPermissions[testPermissionIdx].Name + "0",
+							Namespace: testPermissions[testPermissionIdx].NamespaceID,
+						},
+					},
+				}).Return(nil)
+				as.EXPECT().List(mock.Anything, permission.Filter{
+					Slugs: []string{
+						schema.FQPermissionNameFromNamespace(testPermissions[testPermissionIdx].NamespaceID, testPermissions[testPermissionIdx].Name+"0"),
+					},
+				}).Return([]permission.Permission{
+					{
+						ID:          testPermissions[testPermissionIdx].ID,
+						Name:        testPermissions[testPermissionIdx].Name + "0",
+						NamespaceID: testPermissions[testPermissionIdx].NamespaceID,
+					},
+				}, nil)
+			},
+			request: connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{
+				Bodies: []*frontierv1beta1.PermissionRequestBody{
+					{
+						Key: schema.PermissionKeyFromNamespaceAndName(testPermissions[testPermissionIdx].NamespaceID, testPermissions[testPermissionIdx].Name+"0"),
+					},
+				},
+			}),
+			want: connect.NewResponse(&frontierv1beta1.CreatePermissionResponse{
+				Permissions: []*frontierv1beta1.Permission{
+					{
+						Id:        testPermissions[testPermissionIdx].ID,
+						Name:      testPermissions[testPermissionIdx].Name + "0",
+						Namespace: testPermissions[testPermissionIdx].NamespaceID,
+						Key:       schema.PermissionKeyFromNamespaceAndName(testPermissions[testPermissionIdx].NamespaceID, testPermissions[testPermissionIdx].Name+"0"),
+						CreatedAt: timestamppb.New(testPermissions[testPermissionIdx].CreatedAt),
+						UpdatedAt: timestamppb.New(testPermissions[testPermissionIdx].UpdatedAt),
+					},
+				},
+			}),
+			wantErr: nil,
+		},
+		{
+			name:  "should reject an uppercase verb with the grammar error",
+			setup: func(as *mocks.PermissionService, bs *mocks.BootstrapService) {},
+			request: connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{
+				Bodies: []*frontierv1beta1.PermissionRequestBody{
+					{Key: "compute.order.Read"},
+				},
+			}),
+			want:    nil,
+			wantErr: connect.NewError(connect.CodeInvalidArgument, schema.ValidateCustomPermission("compute/order", "Read")),
+		},
+		{
+			name:  "should reject a too-short verb with the grammar error",
+			setup: func(as *mocks.PermissionService, bs *mocks.BootstrapService) {},
+			request: connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{
+				Bodies: []*frontierv1beta1.PermissionRequestBody{
+					{Key: "compute.order.id"},
+				},
+			}),
+			want:    nil,
+			wantErr: connect.NewError(connect.CodeInvalidArgument, schema.ValidateCustomPermission("compute/order", "id")),
+		},
+		{
+			name:  "should reject a verb that collides with a generated relation",
+			setup: func(as *mocks.PermissionService, bs *mocks.BootstrapService) {},
+			request: connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{
+				Bodies: []*frontierv1beta1.PermissionRequestBody{
+					{Key: "compute.order.owner"},
+				},
+			}),
+			want:    nil,
+			wantErr: connect.NewError(connect.CodeInvalidArgument, schema.ValidateCustomPermission("compute/order", "owner")),
+		},
+		{
+			name:  "should reject a namespace with an underscore in a part",
+			setup: func(as *mocks.PermissionService, bs *mocks.BootstrapService) {},
+			request: connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{
+				Bodies: []*frontierv1beta1.PermissionRequestBody{
+					{Key: "res_order.item.get"},
+				},
+			}),
+			want:    nil,
+			wantErr: connect.NewError(connect.CodeInvalidArgument, schema.ValidateCustomPermission("res_order/item", "get")),
+		},
+		{
+			name:  "should reject a permission whose flattened slug is too long",
+			setup: func(as *mocks.PermissionService, bs *mocks.BootstrapService) {},
+			request: connect.NewRequest(&frontierv1beta1.CreatePermissionRequest{
+				Bodies: []*frontierv1beta1.PermissionRequestBody{
+					{Key: strings.Repeat("a", 30) + "." + strings.Repeat("b", 30) + ".get"},
+				},
+			}),
+			want:    nil,
+			wantErr: connect.NewError(connect.CodeInvalidArgument, schema.ValidateCustomPermission(strings.Repeat("a", 30)+"/"+strings.Repeat("b", 30), "get")),
+		},
 	}
 
 	for _, tt := range tests {
@@ -317,7 +417,7 @@ func TestHandler_UpdatePermission(t *testing.T) {
 			wantErr: connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation),
 		},
 		{
-			name:  "should return bad request error if key name has special characters",
+			name:  "should return bad request error if the key verb breaks the grammar",
 			setup: func(as *mocks.PermissionService) {},
 			request: connect.NewRequest(&frontierv1beta1.UpdatePermissionRequest{
 				Id: testPermissions[testPermissionIdx].ID,
@@ -326,7 +426,7 @@ func TestHandler_UpdatePermission(t *testing.T) {
 				},
 			}),
 			want:    nil,
-			wantErr: connect.NewError(connect.CodeInvalidArgument, errors.New("permission name cannot contain special characters")),
+			wantErr: connect.NewError(connect.CodeInvalidArgument, schema.ValidateCustomPermission("app/resource", "we$rd")),
 		},
 		{
 			name:  "should return bad request error if a key part is empty",
@@ -350,7 +450,7 @@ func TestHandler_UpdatePermission(t *testing.T) {
 				},
 			}),
 			want:    nil,
-			wantErr: connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation),
+			wantErr: connect.NewError(connect.CodeInvalidArgument, schema.ValidateCustomPermission("Ab/resource", "get")),
 		},
 		{
 			name:  "should return bad request error if deprecated fields are sent without key",

internal/bootstrap/schema/schema.go

@@ -299,34 +299,89 @@ func IsSystemNamespace(namespace string) bool {
 		namespace == PATPrincipal || namespace == PlatformNamespace
 }
 
-// IsValidPermissionName checks if the provided name is a valid permission name
+// permissionNameRe matches a permission verb. The verb becomes a SpiceDB
+// relation name directly, so it must satisfy SpiceDB's relation grammar
+// (^[a-z][a-z0-9_]{1,62}[a-z0-9]$): start with a lowercase letter, then
+// lowercase alphanumerics, three to sixty-four characters. Underscore is left
+// out here because the slug joins service, resource, and verb with "_", so an
+// underscore in the verb is treated the same as in a namespace part.
+var permissionNameRe = regexp.MustCompile(`^[a-z][a-z0-9]{2,63}$`)
+
+// IsValidPermissionName reports whether name is a valid permission verb that
+// SpiceDB will accept as a relation name.
 func IsValidPermissionName(name string) bool {
-	if name == "" {
-		return false
-	}
-	// check if name contains anything other than alphanumeric characters
-	for _, r := range name {
-		if !((r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9')) {
-			return false
-		}
-	}
-	return true
+	return permissionNameRe.MatchString(name)
 }
 
-// permissionNamespaceRe matches a custom permission namespace: service/resource
-// with each part one or more lowercase alphanumeric characters. It forbids the
-// underscore inside a part, because FQPermissionNameFromNamespace joins service,
-// resource, and the verb with "_", so an underscore in a part would let two
-// different namespaces flatten to the same slug. Uppercase is forbidden because
-// SpiceDB object type names are lowercase, so it could never be stored anyway.
-var permissionNamespaceRe = regexp.MustCompile(`^[a-z0-9]+/[a-z0-9]+$`)
+// permissionNamespaceRe matches a custom permission namespace: service/resource,
+// where each part is a valid SpiceDB object type segment (^[a-z][a-z0-9_]{1,62}
+// [a-z0-9]$): start with a lowercase letter, then lowercase alphanumerics, three
+// to sixty-four characters. It forbids the underscore inside a part, because
+// FQPermissionNameFromNamespace joins service, resource, and the verb with "_",
+// so an underscore in a part would let two different namespaces flatten to the
+// same slug. Uppercase and a leading digit are forbidden because SpiceDB rejects
+// them in an object type name, so such a namespace could never be stored anyway.
+var permissionNamespaceRe = regexp.MustCompile(`^[a-z][a-z0-9]{2,63}/[a-z][a-z0-9]{2,63}$`)
 
 // IsValidPermissionNamespace reports whether namespace is in service/resource
 // form with each part lowercase alphanumeric.
 func IsValidPermissionNamespace(namespace string) bool {
 	return permissionNamespaceRe.MatchString(namespace)
 }
 
+// maxPermissionSlugLen is SpiceDB's ceiling on a relation name. The reconcile and
+// API paths flatten a permission to the slug service_resource_verb and register
+// that slug as a SpiceDB relation, so a longer slug is rejected when the schema is
+// compiled.
+const maxPermissionSlugLen = 64
+
+// PermissionSlugWithinLimit reports whether the flattened slug
+// service_resource_verb fits SpiceDB's relation-name limit. Each part can be up to
+// sixty-four characters on its own, so a valid namespace and verb can still combine
+// into a slug that SpiceDB will not store.
+func PermissionSlugWithinLimit(namespace, name string) bool {
+	return len(FQPermissionNameFromNamespace(namespace, name)) <= maxPermissionSlugLen
+}
+
+// reservedPermissionVerbs are the relation names the schema generator adds to
+// every custom resource definition (see internal/bootstrap/generator.go's owner,
+// project, and granted relations). A permission whose verb equals one of these
+// makes the generator declare the same relation twice, which SpiceDB rejects when
+// it compiles the schema.
+var reservedPermissionVerbs = map[string]bool{
+	OwnerRelationName:     true, // owner
+	ProjectRelationName:   true, // project
+	RoleGrantRelationName: true, // granted
+}
+
+// IsReservedPermissionVerb reports whether name collides with a relation the
+// generator always adds to a custom resource.
+func IsReservedPermissionVerb(name string) bool {
+	return reservedPermissionVerbs[strings.ToLower(name)]
+}
+
+// ValidateCustomPermission checks that a custom permission's namespace and verb
+// can be created and then reconciled without a plan that passes and an apply that
+// fails: the verb and each namespace part must satisfy SpiceDB's grammar, the verb
+// must not collide with a generated relation, and the flattened slug must fit
+// SpiceDB's relation-name limit. Both the reconcile validator and the
+// CreatePermission handler call this, so the two paths agree.
+func ValidateCustomPermission(namespace, name string) error {
+	if !IsValidPermissionName(name) {
+		return fmt.Errorf("invalid permission verb %q: must be lowercase, start with a letter, and be at least three characters", name)
+	}
+	if IsReservedPermissionVerb(name) {
+		return fmt.Errorf("permission verb %q is reserved: the schema already defines a relation with that name on every resource", name)
+	}
+	if !IsValidPermissionNamespace(namespace) {
+		return fmt.Errorf("invalid permission namespace %q: service and resource must each be lowercase alphanumeric and three to sixty-four characters", namespace)
+	}
+	if !PermissionSlugWithinLimit(namespace, name) {
+		return fmt.Errorf("permission %s:%s is too long: the flattened service_resource_verb must be at most 64 characters", namespace, name)
+	}
+	return nil
+}
+
 func IsPlatformPermission(name string) bool {
 	name = strings.ToLower(name)
 	return name == PlatformSudoPermission || name == PlatformCheckPermission