fix(reconcile): validate the permission namespace charset at plan time

internal/bootstrap/schema/schema.go

@@ -4,6 +4,7 @@ import (
 	_ "embed"
 	"errors"
 	"fmt"
+	"regexp"
 	"strings"
 
 	"github.com/google/uuid"
@@ -311,6 +312,20 @@ func IsValidPermissionName(name string) bool {
 	return true
 }
 
+// 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]+$`)
+
+// IsValidPermissionNamespace reports whether namespace is in service/resource
+// form with each part lowercase alphanumeric.
+func IsValidPermissionNamespace(namespace string) bool {
+	return permissionNamespaceRe.MatchString(namespace)
+}
+
 func IsPlatformPermission(name string) bool {
 	name = strings.ToLower(name)
 	return name == PlatformSudoPermission || name == PlatformCheckPermission

internal/bootstrap/schema/schema_test.go

@@ -50,6 +50,36 @@ func TestFQPermissionNameFromNamespace(t *testing.T) {
 	}
 }
 
+func TestIsValidPermissionNamespace(t *testing.T) {
+	tests := []struct {
+		ns   string
+		want bool
+	}{
+		{"resource/aoi", true},
+		{"user/project", true},
+		{"org/user", true},
+		{"compute/disk", true},
+		{"a1/b2", true},
+		{"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
+		{"compute/Order", false},
+		{"compute", false},    // one part
+		{"a/b/c", false},      // three parts
+		{"/x", false},         // empty part
+		{"x/", false},         // empty part
+		{"", false},           // empty
+		{"comp-ute/x", false}, // hyphen is not alphanumeric
+	}
+	for _, tt := range tests {
+		t.Run(tt.ns, func(t *testing.T) {
+			if got := schema.IsValidPermissionNamespace(tt.ns); got != tt.want {
+				t.Errorf("IsValidPermissionNamespace(%q) = %v, want %v", tt.ns, got, tt.want)
+			}
+		})
+	}
+}
+
 func TestIsBootstrapServiceUser(t *testing.T) {
 	tests := []struct {
 		name string

internal/reconcile/permission.go

@@ -48,6 +48,13 @@ func validatePermissionSpec(s PermissionSpec) error {
 	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)
+	}
 	return nil
 }
 

internal/reconcile/permission_reconciler_test.go

@@ -77,6 +77,18 @@ func TestPermissionReconciler(t *testing.T) {
 		assert.Empty(t, api.created)
 	})
 
+	t.Run("an ambiguous namespace fails Validate before any server call", func(t *testing.T) {
+		// The charset check is server-free, so a namespace that would collide on the
+		// slug must fail the whole file up front (rule 3), not at apply.
+		api := &fakePermissionAPI{}
+		spec := []byte("- {namespace: resource_order/item, name: get}\n")
+
+		err := NewPermissionReconciler(api, "").Validate(spec)
+
+		assert.ErrorContains(t, err, "resource_order/item")
+		assert.Empty(t, api.created)
+	})
+
 	t.Run("reconciling an exported document plans no changes", func(t *testing.T) {
 		api := &fakePermissionAPI{perms: []*frontierv1beta1.Permission{
 			permissionPB("b1", "app/project", "get"),

internal/reconcile/permission_test.go

@@ -63,14 +63,34 @@ func TestDiffPermissions(t *testing.T) {
 		assert.ErrorContains(t, err, "listed both with and without delete")
 	})
 
-	t.Run("distinct entries that flatten to the same slug fail", func(t *testing.T) {
-		_, err := diffPermissions([]PermissionSpec{
-			{Namespace: "compute/order", Name: "get"},
-			{Namespace: "compute/order", Name: "legacy"},
-			{Namespace: "resource/order_item", Name: "get"},
-			{Namespace: "resource_order/item", Name: "get"},
-		}, current)
-		assert.ErrorContains(t, err, `collide on the same slug "resource_order_item_get"`)
+	t.Run("a file namespace that would collide with a server slug is rejected, not absorbed", func(t *testing.T) {
+		// The server stores resource/order_item; the file lists a genuinely different
+		// namespace resource_order/item that flattens to the same slug. The diff used
+		// to treat it as already present and plan zero ops (the rule 2 gap). The
+		// ambiguous namespace is now rejected at validation, so it cannot be absorbed.
+		server := []currentPermission{{ID: "p1", Namespace: "resource/order_item", Name: "get"}}
+		_, err := diffPermissions([]PermissionSpec{{Namespace: "resource_order/item", Name: "get"}}, server)
+		assert.ErrorContains(t, err, "resource_order/item")
+	})
+
+	t.Run("rejects a namespace with an underscore or uppercase in a part", func(t *testing.T) {
+		// The slug joins service, resource, and verb with "_", so an underscore in a
+		// part makes two namespaces flatten to one slug; uppercase cannot be a
+		// SpiceDB object type. Both are rejected so the slug stays one-to-one.
+		for _, ns := range []string{"resource_order/item", "resource/order_item", "Compute/order", "compute/Order"} {
+			_, err := diffPermissions([]PermissionSpec{{Namespace: ns, Name: "get"}}, nil)
+			if assert.Error(t, err, ns) {
+				assert.ErrorContains(t, err, "namespace")
+			}
+		}
+	})
+
+	t.Run("accepts valid custom namespaces", func(t *testing.T) {
+		for _, ns := range []string{"resource/aoi", "user/project", "org/user", "compute/disk"} {
+			ops, err := diffPermissions([]PermissionSpec{{Namespace: ns, Name: "get"}}, nil)
+			assert.NoError(t, err, ns)
+			assert.Len(t, ops, 1) // a valid new permission plans an add
+		}
 	})
 
 	t.Run("rejects base-schema namespaces and bad shapes", func(t *testing.T) {