Skip to content

FR-26219 Namespace every SpiceDB read to the resolved instance - #81

Open
dianaKhortiuk-frontegg wants to merge 4 commits into
fr-26219-sdk-scopingfrom
fr-26219-sdk-namespaced-reads
Open

FR-26219 Namespace every SpiceDB read to the resolved instance#81
dianaKhortiuk-frontegg wants to merge 4 commits into
fr-26219-sdk-scopingfrom
fr-26219-sdk-namespaced-reads

Conversation

@dianaKhortiuk-frontegg

@dianaKhortiuk-frontegg dianaKhortiuk-frontegg commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Applies the instance model to every SpiceDB read. The only PR in the stack that changes behaviour.

+1384 against #78 — source +340 across 10 files, tests +1044.

Spec

  • C4 — every gRPC-bound type goes through namespace.type() on all seven query paths; a caller type containing / is rejected on both FGA sides and both lookup builders; route cache keyed per namespace.
  • C5instanceId on every public method, per-instance fallback, readSchemaFor. spiceClient becomes private rather than public-and-@deprecated as the spec says: direct access bypasses namespacing entirely, so this is a breaking change and takes the release semver-major.
  • C3 rethrowInstanceResolutionException rethrown ahead of the fallback catch-all, so an unresolved instance throws instead of answering false.

Notes

  • instanceId is a second options argument, not a request field as spec'd.
  • lookup-response.mapper.ts untouched — nothing to strip. It echoes the caller's own type and only decodes ids, so no prefix can reach a response.
  • Route cache keys on prefix, not instanceId. Equivalent, now that duplicate prefixes throw at boot.
  • Legacy equivalence: every threading test runs twice, prefixed and legacy, asserting the built request either way.
  • Not done: instanceId on the three lookup log lines.

Base #78

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.
@dianaKhortiuk-frontegg
dianaKhortiuk-frontegg force-pushed the fr-26219-sdk-namespaced-reads branch from 370be48 to ed92678 Compare September 10, 2026 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants