Skip to content

Commit 5ba52b0

Browse files
xuyushun441-sysos-zhuangclaude
authored
fix(driver-sql): honor tenancy.enabled:false in driver org-scoping (ADR-0066) (#2241)
* fix(driver-sql): honor tenancy.enabled:false in driver org-scoping (ADR-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> * chore(changeset): driver-sql tenancy.enabled:false opt-out patch Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 57261f3 commit 5ba52b0

3 files changed

Lines changed: 108 additions & 31 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
"@objectstack/driver-sql": patch
3+
---
4+
5+
fix(driver-sql): honor `tenancy.enabled:false` in driver org-scoping
6+
7+
The driver auto-detects `organization_id` as a tenant-isolation column and, when
8+
the caller passes `DriverOptions.tenantId`, scopes reads/updates/deletes to that
9+
tenant (and injects the column on inserts). The implicit column-detection
10+
fallback ignored an explicit `tenancy.enabled === false`, so a platform-global
11+
object that opts out of tenancy but carries an optional `organization_id` FK
12+
(e.g. `sys_license`) was still org-scoped — an authenticated caller's active-org
13+
`tenantId` then hid every NULL-org / cross-org row. The opt-out is now honored in
14+
a single shared `computeTenantField()` used by both `initObjects` and
15+
`registerExternalObject` (which had drifted). Covers `TursoDriver` (extends
16+
`SqlDriver`). Genuine org-scoped objects are unaffected.

packages/plugins/driver-sql/src/sql-driver-tenant-scope.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,60 @@ describe('SqlDriver tenant scope (organization_id)', () => {
219219
});
220220
});
221221

222+
describe('tenancy.enabled:false opts out of driver org-scoping', () => {
223+
// Regression: a platform-global object (e.g. sys_license, ADR-0066) keeps an
224+
// optional, often-NULL `organization_id` FK but declares `tenancy.enabled:
225+
// false`. The driver previously detected the `organization_id` column via the
226+
// implicit fallback and org-scoped it anyway, so an authenticated caller's
227+
// active-org `tenantId` injected `WHERE organization_id = <org>` and the
228+
// NULL-org rows vanished — the platform admin read zero rows while an
229+
// unscoped read still saw them. tenancy.enabled:false must win.
230+
const platformGlobal = [
231+
{
232+
name: 'sys_license',
233+
tenancy: { enabled: false, strategy: 'shared' },
234+
fields: {
235+
customer: { type: 'string' },
236+
organization_id: { type: 'string' }, // optional owner FK, may be NULL
237+
status: { type: 'string' },
238+
},
239+
},
240+
];
241+
242+
beforeEach(async () => {
243+
await driver.disconnect();
244+
driver = new SqlDriver({
245+
client: 'better-sqlite3',
246+
connection: { filename: ':memory:' },
247+
useNullAsDefault: true,
248+
});
249+
await driver.initObjects(platformGlobal);
250+
// A NULL-org platform row + an org-mapped one.
251+
await driver.create('sys_license', { id: 'lic_global', customer: 'ACME', organization_id: null, status: 'active' });
252+
await driver.create('sys_license', { id: 'lic_org_b', customer: 'Beta', organization_id: 'org_b', status: 'active' });
253+
});
254+
255+
it('does NOT register a tenant field for a tenancy-disabled object', () => {
256+
expect((driver as any).tenantFieldByTable['sys_license']).toBeNull();
257+
});
258+
259+
it('read is unscoped even when the caller passes tenantId (admin with active org sees all)', async () => {
260+
const adminRead = await driver.find('sys_license', { object: 'sys_license' }, { tenantId: 'org_admin_active' });
261+
expect(adminRead.map(r => r.id).sort()).toEqual(['lic_global', 'lic_org_b']);
262+
});
263+
264+
it('matches the unscoped (anonymous) read — no auth-dependent divergence', async () => {
265+
const scoped = await driver.find('sys_license', { object: 'sys_license' }, { tenantId: 'org_admin_active' });
266+
const unscoped = await driver.find('sys_license', { object: 'sys_license' });
267+
expect(scoped.map(r => r.id).sort()).toEqual(unscoped.map(r => r.id).sort());
268+
});
269+
270+
it('does NOT auto-inject organization_id on insert when tenancy is disabled', async () => {
271+
const created = await driver.create('sys_license', { id: 'lic_new', customer: 'Gamma', status: 'active' }, { tenantId: 'org_admin_active' });
272+
expect(created.organization_id ?? null).toBeNull();
273+
});
274+
});
275+
222276
describe('audit warn on missing tenantId', () => {
223277
it('logs once per object:op when writing without tenantId', async () => {
224278
await driver.disconnect();

packages/plugins/driver-sql/src/sql-driver.ts

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,6 +1306,39 @@ export class SqlDriver implements IDataDriver {
13061306
await this.knex.schema.dropTableIfExists(object);
13071307
}
13081308

1309+
/**
1310+
* Resolve the per-table tenant-isolation column for a schema, honoring an
1311+
* explicit tenancy opt-out. Single source of truth for both {@link initObjects}
1312+
* and {@link registerExternalObject} (they previously inlined this logic and
1313+
* drifted).
1314+
*
1315+
* Precedence:
1316+
* 1. `tenancy.enabled === false` → `null` (NO driver-level org scope), even
1317+
* when the object carries an `organization_id` column. Platform-global
1318+
* objects (e.g. `sys_license`) keep an optional, often-NULL org FK but must
1319+
* NOT be tenant-scoped: otherwise an authenticated caller's active-org
1320+
* `DriverOptions.tenantId` injects `WHERE organization_id = <org>` and every
1321+
* NULL-org / cross-org row silently disappears (the platform admin then
1322+
* reads zero licenses while an unscoped/anonymous read still sees them).
1323+
* The declarative branch below already respected `enabled !== false`; the
1324+
* implicit `organization_id` fallback did not — this closes that gap.
1325+
* 2. Declared `tenancy.tenantField` (when that field exists on the object).
1326+
* 3. Implicit `organization_id` column detection (legacy objects whose
1327+
* multi-tenant column was injected by the kernel without a spec migration).
1328+
*/
1329+
protected computeTenantField(schema: { fields?: Record<string, any>; tenancy?: any }): string | null {
1330+
const tenancyDecl = (schema as any)?.tenancy;
1331+
// Explicit opt-out wins over any column-presence heuristic.
1332+
if (tenancyDecl?.enabled === false) return null;
1333+
const fields = schema?.fields;
1334+
if (tenancyDecl?.tenantField) {
1335+
const declared = String(tenancyDecl.tenantField);
1336+
if (fields && Object.prototype.hasOwnProperty.call(fields, declared)) return declared;
1337+
}
1338+
if (fields && Object.prototype.hasOwnProperty.call(fields, 'organization_id')) return 'organization_id';
1339+
return null;
1340+
}
1341+
13091342
/**
13101343
* Batch-initialise tables from an array of object definitions.
13111344
*/
@@ -1368,18 +1401,7 @@ export class SqlDriver implements IDataDriver {
13681401
const datetimeCols: string[] = [];
13691402
const autoNumberCols: Array<{ name: string; format: string; tokens: AutonumberToken[]; tenantField: string | null }> = [];
13701403

1371-
const tenancyDecl = (schema as any)?.tenancy;
1372-
let tenantField: string | null = null;
1373-
if (tenancyDecl && tenancyDecl.enabled !== false && tenancyDecl.tenantField) {
1374-
const declared = String(tenancyDecl.tenantField);
1375-
if (schema.fields && Object.prototype.hasOwnProperty.call(schema.fields, declared)) {
1376-
tenantField = declared;
1377-
}
1378-
}
1379-
if (!tenantField) {
1380-
const hasOrgField = !!(schema.fields && Object.prototype.hasOwnProperty.call(schema.fields, 'organization_id'));
1381-
tenantField = hasOrgField ? 'organization_id' : null;
1382-
}
1404+
const tenantField = this.computeTenantField(schema);
13831405
if (schema.fields) {
13841406
for (const [name, field] of Object.entries<any>(schema.fields)) {
13851407
const type = field.type || 'string';
@@ -1425,25 +1447,10 @@ export class SqlDriver implements IDataDriver {
14251447
const booleanCols: string[] = [];
14261448
const numericCols: string[] = [];
14271449
const autoNumberCols: Array<{ name: string; format: string; tokens: AutonumberToken[]; tenantField: string | null }> = [];
1428-
// Auto-detect tenant field. Convention: the field named
1429-
// `organization_id` (matching tenantPolicy default) scopes the
1430-
// Resolve tenant scope declaratively first (obj.tenancy.{enabled,
1431-
// tenantField}) — that's the user's explicit intent. Fall back to the
1432-
// implicit "has an organization_id field" detection so legacy objects
1433-
// (whose multi-tenant column was injected by the kernel implicitly)
1434-
// keep working without a spec migration.
1435-
const tenancyDecl = (obj as any)?.tenancy;
1436-
let tenantField: string | null = null;
1437-
if (tenancyDecl && tenancyDecl.enabled !== false && tenancyDecl.tenantField) {
1438-
const declared = String(tenancyDecl.tenantField);
1439-
if (obj.fields && Object.prototype.hasOwnProperty.call(obj.fields, declared)) {
1440-
tenantField = declared;
1441-
}
1442-
}
1443-
if (!tenantField) {
1444-
const hasOrgField = !!(obj.fields && Object.prototype.hasOwnProperty.call(obj.fields, 'organization_id'));
1445-
tenantField = hasOrgField ? 'organization_id' : null;
1446-
}
1450+
// Tenant-isolation column: explicit tenancy opt-out → declared field →
1451+
// implicit `organization_id`. See {@link computeTenantField} (shared with
1452+
// registerExternalObject so the two paths can't drift).
1453+
const tenantField = this.computeTenantField(obj);
14471454
if (obj.fields) {
14481455
for (const [name, field] of Object.entries<any>(obj.fields)) {
14491456
const type = field.type || 'string';

0 commit comments

Comments
 (0)