From 7585c420f22848bf5a1d3b95804c344472a93802 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Mon, 17 Aug 2026 11:06:58 +0530 Subject: [PATCH 1/3] fix(reconcile): tighten permission name and namespace validation to SpiceDB grammar --- .../api/v1beta1connect/permission_test.go | 6 +-- internal/bootstrap/schema/schema.go | 38 ++++++++++--------- internal/bootstrap/schema/schema_test.go | 37 +++++++++++++++++- internal/reconcile/permission.go | 2 +- 4 files changed, 60 insertions(+), 23 deletions(-) diff --git a/internal/api/v1beta1connect/permission_test.go b/internal/api/v1beta1connect/permission_test.go index 0f10963865..80d176a4d2 100644 --- a/internal/api/v1beta1connect/permission_test.go +++ b/internal/api/v1beta1connect/permission_test.go @@ -24,7 +24,7 @@ var ( testPermissions = []permission.Permission{ { ID: uuid.New().String(), - Name: "Read", + Name: "read", NamespaceID: "app/resource", Metadata: map[string]any{}, CreatedAt: time.Time{}, @@ -32,14 +32,14 @@ var ( }, { 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{}, diff --git a/internal/bootstrap/schema/schema.go b/internal/bootstrap/schema/schema.go index 612f76be92..e2e4ad5e3e 100644 --- a/internal/bootstrap/schema/schema.go +++ b/internal/bootstrap/schema/schema.go @@ -299,27 +299,29 @@ 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. diff --git a/internal/bootstrap/schema/schema_test.go b/internal/bootstrap/schema/schema_test.go index aa03d6af95..b0c7854cab 100644 --- a/internal/bootstrap/schema/schema_test.go +++ b/internal/bootstrap/schema/schema_test.go @@ -106,7 +106,11 @@ func TestIsValidPermissionNamespace(t *testing.T) { {"user/project", true}, {"org/user", true}, {"compute/disk", true}, - {"a1/b2", true}, + {"a1/b2", false}, // parts shorter than 3 are not valid SpiceDB object types + {"ab/order", false}, // service part shorter than 3 + {"compute/ab", false}, // resource part shorter than 3 + {"1compute/order", false}, // a part cannot start with a digit + {"compute/2order", false}, // a part cannot start with a digit {"resource_order/item", false}, // underscore in a part collides on the slug {"resource/order_item", false}, {"Compute/order", false}, // uppercase is not a SpiceDB object type @@ -127,6 +131,37 @@ func TestIsValidPermissionNamespace(t *testing.T) { } } +func TestIsValidPermissionName(t *testing.T) { + // A permission's verb becomes a SpiceDB relation name directly, so it must + // satisfy SpiceDB's relation grammar: start with a lowercase letter, then + // lowercase alphanumerics, at least three characters. Underscore is left out + // because the slug joins service, resource, and verb with "_". + tests := []struct { + name string + want bool + }{ + {"get", true}, + {"create", true}, + {"list", true}, + {"abc", true}, // exactly three characters + {"id", false}, // shorter than three characters + {"ab", false}, + {"Get", false}, // uppercase is not a SpiceDB relation + {"getAll", false}, // uppercase anywhere + {"1get", false}, // cannot start with a digit + {"get_all", false}, // underscore is not allowed in a verb + {"get-all", false}, // hyphen is not alphanumeric + {"", false}, // empty + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := schema.IsValidPermissionName(tt.name); got != tt.want { + t.Errorf("IsValidPermissionName(%q) = %v, want %v", tt.name, got, tt.want) + } + }) + } +} + func TestIsBootstrapServiceUser(t *testing.T) { tests := []struct { name string diff --git a/internal/reconcile/permission.go b/internal/reconcile/permission.go index bce2192340..aa4bd9929a 100644 --- a/internal/reconcile/permission.go +++ b/internal/reconcile/permission.go @@ -40,7 +40,7 @@ func validatePermissionSpec(s PermissionSpec) error { return fmt.Errorf("namespace and name are required") } if !schema.IsValidPermissionName(s.Name) { - return fmt.Errorf("invalid name %q (alphanumeric only)", s.Name) + return fmt.Errorf("invalid name %q (must be lowercase, start with a letter, and be at least three characters)", s.Name) } if isBaseNamespace(s.Namespace) { return fmt.Errorf("namespace %q is part of the base schema, which the server manages", s.Namespace) From 1f2b8d581e9724154cc20982d32c84478f3daf44 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Mon, 17 Aug 2026 13:05:03 +0530 Subject: [PATCH 2/3] fix(reconcile): reject a permission whose flattened slug exceeds SpiceDB's limit --- internal/bootstrap/schema/schema.go | 14 ++++++++++++++ internal/bootstrap/schema/schema_test.go | 24 ++++++++++++++++++++++++ internal/reconcile/permission.go | 6 ++++++ internal/reconcile/permission_test.go | 12 ++++++++++++ 4 files changed, 56 insertions(+) diff --git a/internal/bootstrap/schema/schema.go b/internal/bootstrap/schema/schema.go index e2e4ad5e3e..b23b48ece2 100644 --- a/internal/bootstrap/schema/schema.go +++ b/internal/bootstrap/schema/schema.go @@ -329,6 +329,20 @@ 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 +} + func IsPlatformPermission(name string) bool { name = strings.ToLower(name) return name == PlatformSudoPermission || name == PlatformCheckPermission diff --git a/internal/bootstrap/schema/schema_test.go b/internal/bootstrap/schema/schema_test.go index b0c7854cab..cb90b7c90e 100644 --- a/internal/bootstrap/schema/schema_test.go +++ b/internal/bootstrap/schema/schema_test.go @@ -1,6 +1,7 @@ package schema_test import ( + "strings" "testing" "github.com/raystack/frontier/internal/bootstrap/schema" @@ -162,6 +163,29 @@ func TestIsValidPermissionName(t *testing.T) { } } +func TestPermissionSlugWithinLimit(t *testing.T) { + // The slug service_resource_verb becomes a SpiceDB relation, which is capped at + // sixty-four characters. Each part is valid on its own, but together they can + // overflow, so a plan can pass and the schema still fail to compile. + tests := []struct { + ns, name string + want bool + }{ + {"compute/order", "get", true}, + // 30 + 29 + 3 plus the two underscores is exactly sixty-four + {strings.Repeat("a", 30) + "/" + strings.Repeat("b", 29), "get", true}, + // one more character tips it over the limit + {strings.Repeat("a", 30) + "/" + strings.Repeat("b", 30), "get", false}, + } + for _, tt := range tests { + t.Run(tt.ns+"."+tt.name, func(t *testing.T) { + if got := schema.PermissionSlugWithinLimit(tt.ns, tt.name); got != tt.want { + t.Errorf("PermissionSlugWithinLimit(%q, %q) = %v, want %v", tt.ns, tt.name, got, tt.want) + } + }) + } +} + func TestIsBootstrapServiceUser(t *testing.T) { tests := []struct { name string diff --git a/internal/reconcile/permission.go b/internal/reconcile/permission.go index aa4bd9929a..49fdf36fd9 100644 --- a/internal/reconcile/permission.go +++ b/internal/reconcile/permission.go @@ -55,6 +55,12 @@ func validatePermissionSpec(s PermissionSpec) error { if !schema.IsValidPermissionNamespace(s.Namespace) { return fmt.Errorf("invalid namespace %q (each of service/resource must be lowercase alphanumeric)", s.Namespace) } + // Each part is capped on its own, but the flattened slug service_resource_verb + // becomes a SpiceDB relation, which is capped at sixty-four characters. Reject a + // slug over that here so a plan cannot pass and then fail when the schema compiles. + if !schema.PermissionSlugWithinLimit(s.Namespace, s.Name) { + return fmt.Errorf("permission %s is too long: service_resource_verb must be at most 64 characters", s) + } return nil } diff --git a/internal/reconcile/permission_test.go b/internal/reconcile/permission_test.go index 3c442e35c1..61bfaa558f 100644 --- a/internal/reconcile/permission_test.go +++ b/internal/reconcile/permission_test.go @@ -1,6 +1,7 @@ package reconcile import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -93,6 +94,17 @@ func TestDiffPermissions(t *testing.T) { } }) + t.Run("rejects a permission whose flattened slug is too long", func(t *testing.T) { + // Each part is valid on its own, but the slug service_resource_verb overflows + // SpiceDB's sixty-four character relation limit, so it must fail the plan + // rather than pass and then fail when the schema compiles. + ns := strings.Repeat("a", 30) + "/" + strings.Repeat("b", 30) + _, err := diffPermissions([]PermissionSpec{{Namespace: ns, Name: "get"}}, nil) + if assert.Error(t, err) { + assert.ErrorContains(t, err, "too long") + } + }) + t.Run("rejects base-schema namespaces and bad shapes", func(t *testing.T) { for _, s := range []PermissionSpec{ {Namespace: "app/organization", Name: "hack"}, From adfa6306ba312398cae0d3272cf68ff75ee87b62 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 19 Aug 2026 12:19:56 +0530 Subject: [PATCH 3/3] fix(permission): share one validation rule across create API and reconcile, reject reserved verbs --- internal/api/v1beta1connect/permission.go | 20 ++-- .../api/v1beta1connect/permission_test.go | 106 +++++++++++++++++- internal/bootstrap/schema/schema.go | 39 +++++++ internal/bootstrap/schema/schema_test.go | 55 +++++++++ internal/reconcile/permission.go | 25 +---- internal/reconcile/permission_test.go | 12 ++ 6 files changed, 224 insertions(+), 33 deletions(-) diff --git a/internal/api/v1beta1connect/permission.go b/internal/api/v1beta1connect/permission.go index 973c30834a..601681c5b3 100644 --- a/internal/api/v1beta1connect/permission.go +++ b/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(), diff --git a/internal/api/v1beta1connect/permission_test.go b/internal/api/v1beta1connect/permission_test.go index 80d176a4d2..7fa248eb95 100644 --- a/internal/api/v1beta1connect/permission_test.go +++ b/internal/api/v1beta1connect/permission_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "testing" "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", diff --git a/internal/bootstrap/schema/schema.go b/internal/bootstrap/schema/schema.go index b23b48ece2..3ab54a2a4c 100644 --- a/internal/bootstrap/schema/schema.go +++ b/internal/bootstrap/schema/schema.go @@ -343,6 +343,45 @@ 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 diff --git a/internal/bootstrap/schema/schema_test.go b/internal/bootstrap/schema/schema_test.go index cb90b7c90e..5af7e0ba86 100644 --- a/internal/bootstrap/schema/schema_test.go +++ b/internal/bootstrap/schema/schema_test.go @@ -186,6 +186,61 @@ func TestPermissionSlugWithinLimit(t *testing.T) { } } +func TestIsReservedPermissionVerb(t *testing.T) { + // These are the relations the generator adds to every custom resource, so a + // verb equal to one of them would declare the same relation twice. + tests := []struct { + name string + want bool + }{ + {"owner", true}, + {"project", true}, + {"granted", true}, + {"Owner", true}, // reserved check is case-insensitive + {"get", false}, + {"delete", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := schema.IsReservedPermissionVerb(tt.name); got != tt.want { + t.Errorf("IsReservedPermissionVerb(%q) = %v, want %v", tt.name, got, tt.want) + } + }) + } +} + +func TestValidateCustomPermission(t *testing.T) { + tests := []struct { + title string + ns string + name string + wantErr string // substring the error must contain; empty means no error + }{ + {"valid", "compute/order", "get", ""}, + {"uppercase verb", "compute/order", "Read", "invalid permission verb"}, + {"short verb", "compute/order", "id", "invalid permission verb"}, + {"reserved verb owner", "compute/order", "owner", "reserved"}, + {"reserved verb granted", "compute/order", "granted", "reserved"}, + {"underscore namespace", "res_order/item", "get", "invalid permission namespace"}, + {"missing resource", "compute", "get", "invalid permission namespace"}, + {"slug too long", strings.Repeat("a", 30) + "/" + strings.Repeat("b", 30), "get", "too long"}, + } + for _, tt := range tests { + t.Run(tt.title, func(t *testing.T) { + err := schema.ValidateCustomPermission(tt.ns, tt.name) + if tt.wantErr == "" { + if err != nil { + t.Errorf("ValidateCustomPermission(%q, %q) = %v, want nil", tt.ns, tt.name, err) + } + return + } + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("ValidateCustomPermission(%q, %q) = %v, want error containing %q", tt.ns, tt.name, err, tt.wantErr) + } + }) + } +} + func TestIsBootstrapServiceUser(t *testing.T) { tests := []struct { name string diff --git a/internal/reconcile/permission.go b/internal/reconcile/permission.go index 49fdf36fd9..1c45439615 100644 --- a/internal/reconcile/permission.go +++ b/internal/reconcile/permission.go @@ -39,29 +39,14 @@ func validatePermissionSpec(s PermissionSpec) error { if strings.TrimSpace(s.Namespace) == "" || strings.TrimSpace(s.Name) == "" { return fmt.Errorf("namespace and name are required") } - if !schema.IsValidPermissionName(s.Name) { - return fmt.Errorf("invalid name %q (must be lowercase, start with a letter, and be at least three characters)", s.Name) - } if isBaseNamespace(s.Namespace) { return fmt.Errorf("namespace %q is part of the base schema, which the server manages", s.Namespace) } - if parts := strings.Split(s.Namespace, "/"); len(parts) != 2 || parts[0] == "" || parts[1] == "" { - return fmt.Errorf("namespace %q must be in service/resource form", s.Namespace) - } - // The slug joins service, resource, and verb with "_", so an underscore inside - // a namespace part would make two different namespaces flatten to the same slug - // and be silently treated as one. Require each part to be lowercase alphanumeric - // so the slug is one-to-one and a plan cannot mistake one namespace for another. - if !schema.IsValidPermissionNamespace(s.Namespace) { - return fmt.Errorf("invalid namespace %q (each of service/resource must be lowercase alphanumeric)", s.Namespace) - } - // Each part is capped on its own, but the flattened slug service_resource_verb - // becomes a SpiceDB relation, which is capped at sixty-four characters. Reject a - // slug over that here so a plan cannot pass and then fail when the schema compiles. - if !schema.PermissionSlugWithinLimit(s.Namespace, s.Name) { - return fmt.Errorf("permission %s is too long: service_resource_verb must be at most 64 characters", s) - } - return nil + // One shared check so the reconcile plan and the CreatePermission API agree on + // what a valid custom permission is: SpiceDB grammar, no reserved verb, and a + // slug that fits SpiceDB's relation-name limit. This stops a plan that passes and + // then fails when the schema compiles. + return schema.ValidateCustomPermission(s.Namespace, s.Name) } // currentPermission is one custom permission as returned by ListPermissions. diff --git a/internal/reconcile/permission_test.go b/internal/reconcile/permission_test.go index 61bfaa558f..a8d358e969 100644 --- a/internal/reconcile/permission_test.go +++ b/internal/reconcile/permission_test.go @@ -105,6 +105,18 @@ func TestDiffPermissions(t *testing.T) { } }) + t.Run("rejects a verb that collides with a generated relation", func(t *testing.T) { + // The generator adds owner, project, and granted to every custom resource, so + // a verb equal to one of them would declare the same relation twice and the + // schema would fail to compile. The plan must reject it up front. + for _, name := range []string{"owner", "project", "granted"} { + _, err := diffPermissions([]PermissionSpec{{Namespace: "compute/order", Name: name}}, nil) + if assert.Error(t, err, name) { + assert.ErrorContains(t, err, "reserved") + } + } + }) + t.Run("rejects base-schema namespaces and bad shapes", func(t *testing.T) { for _, s := range []PermissionSpec{ {Namespace: "app/organization", Name: "hack"},