FR-26219 Namespace every SpiceDB read to the resolved instance - #81
Open
dianaKhortiuk-frontegg wants to merge 10 commits into
Open
dianaKhortiuk-frontegg wants to merge 10 commits into
dianaKhortiuk-frontegg wants to merge 10 commits into
Conversation
dianaKhortiuk-frontegg
force-pushed
the
fr-26219-sdk-scoping
branch
from
September 9, 2026 11:45
e4a057e to
d5758d1
Compare
dianaKhortiuk-frontegg
force-pushed
the
fr-26219-sdk-namespaced-reads
branch
2 times, most recently
from
September 9, 2026 11:51
2f347e5 to
a147ef4
Compare
This was referenced Sep 9, 2026
dianaKhortiuk-frontegg
force-pushed
the
fr-26219-sdk-namespaced-reads
branch
4 times, most recently
from
September 16, 2026 08:45
ed92678 to
25c14a6
Compare
dianaKhortiuk-frontegg
force-pushed
the
fr-26219-sdk-namespaced-reads
branch
from
September 16, 2026 09:46
25c14a6 to
1f9c52e
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1f9c52e. Configure here.
Applies the model from the previous PR to every read path. This is the PR that changes existing behaviour, so it is the one to read for regressions. The client resolves the instance before the try block, so a resolution failure throws rather than degrading into the fallback boolean — a fallback would be answering a question scoped to no instance at all. The resolved namespace is threaded through all four query strategies, both lookup builders and the batch feature path; the route cache key is now per prefix, where it previously held every vendor's routes under one key. New public surface: instanceId as a trailing options argument on all five methods, readSchemaFor(instanceId), and per-instance fallback config. Raw spiceClient is marked @deprecated. Log lines carry instanceId. With no instances configured every request resolves to a legacy namespace and the built gRPC requests are byte-identical to master. 253 tests, tsc clean, 0 lint errors.
InvalidObjectTypeException extends Error, not ConfigurationInputIsInvalidException, so the rethrow guards did not cover it and a caller passing another instance's prefix — v_other/document — was answered with the fallback boolean instead of an error. That is the behaviour d05162d exists to prevent. Single calls throw, which is what isEntitledTo should do for a caller-input error. Batches fail per item instead: rethrowing inside the Promise.all made one bad entityType discard every other result, so the offending item now carries `error` while its neighbours answer normally. `result` is left undefined rather than false, so nothing is granted and a caller can tell a refusal from a denial. EntitlementsResult gains an optional `error`, which is additive for consumers. Verified against an unreachable endpoint, so SpiceDB is never consulted: before, isEntitledTo resolved to {"result":false}; now it throws. A three-item batch with one bad entityType returns true / error / true.
The block splitter counted every brace in a line, including braces inside
string literals and comments. A caveat body containing x == "{" left the block
open, so everything after it was returned as part of that instance:
definition v_aaa/user {}
caveat v_aaa/c(x string) { x == "{" }
definition v_bbb/secret {}
readSchemaFor('a') -> "... definition v_bbb/secret {}"
SpiceDB v1.53.0 accepts that schema, so it was reachable, and it defeats the
reason readSchemaFor exists. Braces are now counted only outside single and
double quoted strings, line comments and block comments, with escapes handled.
A block-comment line is also no longer mistaken for a block header.
Six cases pinned in spicedb-entitlements.client.read-schema.spec.ts, including
that the instance's own caveat body still comes back intact.
BREAKING CHANGE: spiceClient is no longer public. Direct access bypasses instance namespacing and can read another instance's data. Use isEntitledTo, the lookup methods, or readSchemaFor with an instanceId instead. The field carried @deprecated in an earlier revision of this branch. A private field cannot be deprecated for callers who can no longer reach it, so the note goes with it.
… document them - ClientConfiguration extends InstancesConfiguration now that the client reads it - specs follow the registry API from the base branch (instanceId on every namespace, 'legacy' id) - README documents instances, defaultInstanceId, schemaPrefix and the resolution exceptions
…nstance, and fail closed on bad input
- the schema block parser lives in its own module, carries string and comment state across lines, and throws SchemaParseException on unbalanced input
- readSchemaFor takes options and returns the instance's blocks with its prefix stripped
- a failing isEntitledToMany item is { result: false, error } and is logged with its instanceId
- LoggingClient.log and error accept an optional { instanceId }
- only InvalidObjectTypeException is treated as caller input; prefix-escape specs run through the real query client
- route checks log the unprefixed type and key the cache by schema prefix
- the client builds its registry once; the injectable registry parameter is gone
…ranch - InstanceOptions lives with the other instance types - the schema block parser is a utils module with its constants in instance.constants.ts
A caveat body using a normal CEL field access broke readSchemaFor for every
instance, not just the one whose caveat it was:
caveat v_aaa/c(attrs map<any>) {
attrs.definition == 1
}
SchemaParseException: 'definition' starts before the previous block is closed
The header scan already refused a keyword glued to an identifier or preceded by
a slash, but a dot passed, so the `definition` in `attrs.definition` looked like
a new block. It now refuses a dot too.
Failing closed rather than leaking was already the safer half of this; what was
left is that the read failed at all.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… canonical The rewritten base made SchemaNamespace's constructor private behind prefixed() and legacy(); three spec call sites replayed from this branch still used new. They follow the base now rather than reopening the constructor. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
dianaKhortiuk-frontegg
force-pushed
the
fr-26219-sdk-namespaced-reads
branch
from
September 16, 2026 11:56
f4a7ab0 to
8a0ce4a
Compare
SpiceDB accepts a comment between the keyword and the name, and this is
valid input:
definition /* generated */ v_aaa/user {}
blockNameAfter skipped only whitespace, so the comment became the name, the
prefix check failed, and readSchemaFor dropped that instance's own block and
returned an empty schema rather than raising SchemaParseException. Silent and
wrong, which is the failure mode this splitter exists to prevent.
The name is now read past line and block comments, using the same
endOfComment the top-level scan already uses. Verified against SpiceDB
v1.42.1: it accepts the schema this rejected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

What
Every SpiceDB read the SDK issues is now namespaced to the resolved instance, and every public method takes an
instanceId. Part 2/3 of the entitlements-client work for FR-26219, on top of #78. This is the only PR in the stack that changes behaviour.Breaking:
spiceClientbecomesprivate. Direct access bypassed namespacing entirely, so the field could not stay reachable. Ships as a semver-major.How
Every public method gains an optional trailing
options:The client resolves the instance once per call and hands its
SchemaNamespaceto the query strategy. Every gRPC-bound object type goes throughnamespace.type()and every type in a response goes throughnamespace.strip(), so prefixes never appear in SDK inputs or outputs.Legacy callers pass nothing, resolve to the legacy instance, and build byte-identical requests to today.
Where the namespace is applied
isEntitledTo/isEntitledToMany— feature, permission, route, entitylookupResources,lookupSubjectsRouteSpiceDBQuerycachereadSchemaForThree decisions worth a look
1.
instanceIdis anoptionsargument, not a request field. The request objects are shared with the v2 client and serialised into cache keys; a routing concern does not belong in them. One extra optional parameter is the smallest possible change for existing call sites.2. Instance-resolution errors are rethrown ahead of the fallback.
isEntitledTocatches everything and returns the configured fallback. An unknowninstanceId, a missing default, or a/in a caller-supplied type is a caller bug, not an outage, and answeringfalse(ortrue) would hide it. Those throw; everything else still falls back.3.
readSchemaForis a read-only view. It filters the live schema by prefix using a brace-depth block splitter, so definitions with nested braces survive. The result drops top-level directives and is not writable back; that is documented on the method.Not covered
lookup-response.mapper.tsis untouched: it echoes the caller's own type and decodes ids, so no prefix can reach a response through it. The three lookup log lines do not yet carryinstanceId.Testing
schema-namespace-threading.spec: every query path built twice, prefixed and legacy, asserting the exact gRPC request either wayspicedb-entitlements.client.instances.spec: resolution through the public API, the rethrow-vs-fallback split, a legacy caller keeping a namespaced entityTypespicedb-entitlements.client.read-schema.spec: nested braces, unknown prefix, legacy full schemaroute-spicedb.query.spec: cache isolation between two prefixesStack
#78 → #81 → #79. Needs the major release; #79 needs this merged first.
Note
High Risk
Changes core authorization request shaping and tenant/instance isolation; mis-routing or prefix bugs could deny or leak access across vendors, and making
spiceClientprivate is a breaking API change.Overview
Adds multi-instance configuration so one SpiceDB engine can serve several Frontegg vendors: each call can pass
{ instanceId }, reads resolve a schema prefix (fromvendorIdor override), and all gRPC object types are built viaSchemaNamespace.type(). Legacy configs with noinstanceskeep unprefixed, pass-through types (e.g.acme/document).Public APIs (
isEntitledTo, lookups,lookupEntitlements,readSchemaFor) take optional instance options;spiceClientis now private (semver-major). Instance-resolution failures and types containing/when instances are configured throw instead of using the error fallback;isEntitledToManyreturns{ result: false, error }per bad item. Logginglog/errorgain optional{ instanceId }; route relationship cache is keyed per prefix.readSchemaForfilters live schema text to one instance’sdefinition/caveatblocks (prefix stripped) viafilterSchemaBlocks, throwingSchemaParseExceptionon unsafe parse. Per-instancefallbackConfigurationoverrides the client default.Reviewed by Cursor Bugbot for commit 132411a. Bugbot is set up for automated code reviews on this repo. Configure here.
Verified against a running SpiceDB
The SDK was built and driven as a consumer would use it, against SpiceDB v1.42.1 holding two prefixed instances (
v_aaaa1111,v_bbbb2222), each with its own feature:isEntitledTowithinstanceId: 'eu'sees its own feature'us'does not seeeu's featureinstanceIdomitted with two configuredInstanceIdRequiredExceptioninstanceIdUnknownInstanceException/InvalidObjectTypeException, not a silent falsereadSchemaFor('eu')v_bbbb2222blocks, prefix stripped from the outputinstanceIdomittedTwo notes from doing it. Permission checks run at SpiceDB's default consistency, so a check fired immediately after a write can miss it — expected, but it makes naive tests flaky. And
isEntitledToreturns the fallback on any internal error, so a misconfiguration is indistinguishable from a denial withoutlogging.logResults.