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

[rohilsurana]

What

Plan-time validation for a permission's verb and service/resource namespace was looser than the identifier grammar SpiceDB enforces at apply. An ordinary input planned cleanly and then failed at apply with a 500:

• a verb shorter than three characters (e.g. id)
• an uppercase verb (e.g. Read)
• a service or resource part starting with a digit, shorter than three characters, or uppercase

A permission's verb becomes a SpiceDB relation name directly, and its service/resource becomes a SpiceDB object type name, so both must satisfy SpiceDB's grammar: start with a lowercase letter, lowercase alphanumerics, three to sixty-four characters.

This tightens IsValidPermissionName and permissionNamespaceRe to that grammar. The no-underscore rule for the namespace parts and the verb is kept, since the slug joins service, resource, and verb with _.

Why

RFC 0001 Rules 2 and 3: the file states a desired state that can actually apply, and validation happens before apply. A plan that looks clean must not fail at the server.

Testing

• New TestIsValidPermissionName and expanded TestIsValidPermissionNamespace cover the tightened cases.
• Updated the CreatePermission handler test fixtures, which used uppercase names SpiceDB would reject.
• internal/bootstrap/schema, internal/reconcile, and internal/api/v1beta1connect pass.

Stack

Stacked on fix/reconcile-framework-known-fields.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
frontier	Ready	Preview	Aug 19, 2026 9:34am

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 477b049b-cac1-4413-9c63-97d540fd351e

📥 Commits

Reviewing files that changed from the base of the PR and between eca7ba0 and adfa630.

📒 Files selected for processing (6)

• internal/api/v1beta1connect/permission.go
• internal/api/v1beta1connect/permission_test.go
• internal/bootstrap/schema/schema.go
• internal/bootstrap/schema/schema_test.go
• internal/reconcile/permission.go
• internal/reconcile/permission_test.go

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Added unified validation for custom permissions across creation, updates, and reconciliation.
  • Custom permission names and namespaces must use lowercase alphanumeric segments between 3–64 characters.
  • Added limits for combined permission slug length and protection against reserved permission verbs.

• Bug Fixes

  • Invalid custom permissions now return clear invalid-argument errors consistently.
  • Improved rejection of malformed, uppercase, overly short, reserved, and overlong permission values.

Walkthrough

Changes

Custom permission validation

Layer / File(s)	Summary
Define custom permission validation internal/bootstrap/schema/schema.go, internal/bootstrap/schema/schema_test.go	The schema package enforces lowercase permission grammar, namespace rules, flattened slug limits, and reserved generated relation names. Tests cover boundaries and validation errors.
Apply validation during reconciliation internal/reconcile/permission.go, internal/reconcile/permission_test.go	Permission reconciliation delegates custom validation to schema.ValidateCustomPermission while retaining base namespace rejection.
Apply validation at API endpoints internal/api/v1beta1connect/permission.go, internal/api/v1beta1connect/permission_test.go	Create and update endpoints use shared validation and test valid keys plus invalid grammar, namespace, reserved-verb, and slug cases.

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

Merge Risk: ⚪ Minimal · up to adfa6

The change tightens permission and namespace validation to prevent invalid configurations from reaching apply time; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: whoabhisheksah, amangit07

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coveralls]

Coverage Report for CI Build 32238271615

Coverage increased (+0.03%) to 48.782%

Details

• Coverage increased (+0.03%) from the base build.
• Patch coverage: 26 of 26 lines across 3 files are fully covered (100%).
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	40095
Covered Lines:	19559
Line Coverage:	48.78%
Coverage Strength:	15.69 hits per line

---

💛 - Coveralls

[rohilsurana]

Review: tighten permission grammar to SpiceDB (RFC 0001 Rule 3)

Verified on this branch: go build ./..., go test ./internal/reconcile/... ./internal/bootstrap/schema/... ./internal/api/v1beta1connect/..., and golangci-lint run on the changed packages all pass.

This meets Rule 3 for the modes it claims. The new patterns permissionNameRe = ^[a-z][a-z0-9]{2,63}$ and permissionNamespaceRe = ^[a-z][a-z0-9]{2,63}/[a-z][a-z0-9]{2,63}$ (schema.go) enforce lowercase, a leading letter, 3 to 64 chars per part, and no underscore, and validation runs over every entry before any server write (in both Validate and diffPermissions). No new Rule 5 break: every shape it newly rejects (leading digit, uppercase, under three chars) is also rejected by SpiceDB itself, so the server cannot hold it and export cannot emit it.

Verdict: approve with nits.

I am putting the two permission-validation items here, since this is the grammar PR. #1892 now stacks on top of this branch and inherits this grammar, so fixing them here fixes them for both.

Nits:

1. The combined slug length is still unbounded. Each part caps at 64, but the slug service_resource_verb (from FQPermissionNameFromNamespace) can reach 194 chars, while SpiceDB caps a relation name at 64. Concrete case: namespace aaaa...(30 chars)/bbbb...(30 chars) plus name get passes validatePermissionSpec and reconcile --dry-run plans cleanly, but a real apply fails at schema compile because the 65-char relation is rejected. This is the exact "plan clean, apply 500" case the Why section cites Rule 3 to prevent. Add a bound like len(service)+len(resource)+len(name)+2 <= 64.
2. Reachable Rule 5 gap from an API/reconcile validation asymmetry (pre-existing, not introduced here, but this is the right PR to close it). CreatePermission (internal/api/v1beta1connect/permission.go:32) validates the verb via IsValidPermissionName but never calls IsValidPermissionNamespace on the namespace. SpiceDB allows underscores inside object-type names, so a permission compute/sub_order:read is creatable through the API and stored. Export then emits it and reconcile rejects it, because the namespace regex forbids the underscore. Adding IsValidPermissionNamespace to the API create path closes this for both the current format and fix(reconcile): identify permissions by key instead of namespace and name #1892's key format.

[rohilsurana]

Follow-up on the two items.

• Slug length: done. Added schema.PermissionSlugWithinLimit, called it from validatePermissionSpec, and covered it with a unit test and a reconcile test. A namespace and verb that each pass on their own but flatten to a slug over 64 chars now fails the plan instead of the apply.
• API namespace grammar: leaving it out of this PR on purpose. It is a different case from the length check. A too-long slug can never compile in SpiceDB, so rejecting it early breaks nothing. But underscores are legal in SpiceDB object-type names, so a permission like compute/sub_order:read is creatable and stored today through the API. Adding IsValidPermissionNamespace to the create path would reject that and break existing API-only callers. That is a public-API change with its own compat call, so it does not belong in the reconcile grammar PR. Reconcile already fails loud on such a namespace, which is the safe side.

[rohilsurana]

Re-review (head f60571f)

Build, tests, and lint are all green. The slug-length fix is correct and complete. PermissionSlugWithinLimit uses the same service_resource_verb flattening as apply and the bound is <= 64, so a 64-char slug passes and 65 fails, with no off-by-one. It runs server-free in both Validate and diffPermissions, over every entry, before any apply, so Rule 3 holds.

Still open (pre-existing, not claimed in this commit): the CreatePermission API (internal/api/v1beta1connect/permission.go:32) validates the verb via IsValidPermissionName but never calls IsValidPermissionNamespace and never checks the slug length. So an underscore namespace like compute/sub_order, or an over-long namespace/name, is still creatable through the API and then rejected on export and reconcile. Worth closing in the API create path in a follow-up.

Verdict: approve with nits.