fix(driver-sql): honor tenancy.enabled:false in driver org-scoping (ADR-0066) - #2241
Merged
Merged
Conversation
…DR-0066) The SqlDriver auto-detects `organization_id` as a tenant-isolation column and, when the caller passes `DriverOptions.tenantId`, injects `WHERE organization_id = <tenantId>` on reads/updates/deletes (and the column on inserts). The detection had two branches: a declarative one that correctly respected `tenancy.enabled !== false`, and an implicit `organization_id`-column fallback that did NOT — so an object explicitly opting OUT of tenancy still got org-scoped whenever it happened to carry an `organization_id` column. Impact (surfaced verifying ADR-0066 Phase 1): `sys_license` is platform-global (`tenancy.enabled:false`) but keeps an optional, often-NULL `organization_id` owner FK. A platform admin with an active org reads it through the data API with `ctx.tenantId = <org>`, so the engine threads that into DriverOptions and the driver silently filtered to `organization_id = <org>` — excluding the NULL-org rows. Net: the admin saw ZERO licenses while an unscoped/anonymous read still returned them. The filter is injected inside the driver's query builder, not the AST, which is why it presented as an empty result with no visible RLS `where` (astWhere=undefined). Fix: extract a single `computeTenantField()` helper (shared by `initObjects` and `registerExternalObject`, which had drifted) that returns `null` for any schema with `tenancy.enabled === false`, before the implicit column heuristic. Genuine org-scoped objects (no tenancy decl, or `enabled:true`) are unaffected. TursoDriver extends SqlDriver and delegates initObjects to super, so the prod control plane (Neon/Postgres and libsql) is covered by the same fix. Adds regression tests: tenancy-disabled object registers no tenant field, reads are unscoped regardless of `tenantId`, scoped/unscoped reads agree, and inserts don't auto-inject `organization_id`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 8 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
github-actions
Bot
added
documentation
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
2 participants
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.
Problem
SqlDriverauto-detectsorganization_idas a tenant-isolation column and, when the caller passesDriverOptions.tenantId, injectsWHERE organization_id = <tenantId>on reads/updates/deletes (and the column on inserts) — inside the query builder, not the AST. The detection had two branches: a declarative one that correctly respectedtenancy.enabled !== false, and an implicitorganization_id-column fallback that did not. So an object that explicitly opts out of tenancy still got org-scoped whenever it happened to carry anorganization_idcolumn.Impact (surfaced verifying ADR-0066 Phase 1)
sys_licenseis platform-global (tenancy.enabled:false) but keeps an optional, often-NULLorganization_idowner FK. A platform admin with an active org reads it through the data API withctx.tenantId = <org>, which the engine threads intoDriverOptions, so the driver silently filtered toorganization_id = <org>— excluding the NULL-org rows. Net: the admin saw zero licenses while an unscoped/anonymous read still returned them. Because the filter lives in the driver's query builder (not the AST), it presented as an empty result with no visible RLSwhere(astWhere=undefined) — easy to misdiagnose as a datasource-routing bug. It is neither routing nor the ADR-0066 security layer (which correctly applies no filter / fires the RLS read-bypass); it's purely the driver's tenant-column detection.Fix
Extract a single
computeTenantField()helper (shared byinitObjectsandregisterExternalObject, which had drifted) that returnsnullfor any schema withtenancy.enabled === false, before the implicit column heuristic. Genuine org-scoped objects (no tenancy decl, orenabled:true) are unaffected.TursoDriver extends SqlDriverand delegatesinitObjectstosuper, so the prod control plane (Neon/Postgres and libsql) is covered by the same fix.Tests
Adds regression coverage to
sql-driver-tenant-scope.test.ts: a tenancy-disabled object registers no tenant field, reads are unscoped regardless oftenantId, scoped/unscoped reads agree, and inserts don't auto-injectorganization_id.pnpm --filter @objectstack/driver-sql test→ 207/207 pass (incl. the 4 new cases).tenantFieldByTable['sys_license']is nownull, the admin (withtenantId) reads the NULL-org row, and a genuine org-scoped object (sys_environment) is still scoped — no regression to real tenant isolation.🤖 Generated with Claude Code