diff --git a/.changeset/24583-7a1-tenant-scope-null-arm-retired.md b/.changeset/24583-7a1-tenant-scope-null-arm-retired.md new file mode 100644 index 00000000000..823ff6636b2 --- /dev/null +++ b/.changeset/24583-7a1-tenant-scope-null-arm-retired.md @@ -0,0 +1,20 @@ +--- +'@objectstack/driver-sql': minor +'@objectstack/objectql': minor +--- + +feat(driver-sql)!: a tenant-scoped read no longer answers a row with no organization; the driver's `OR organization_id IS NULL` arm is retired + +Clause-②: no (narrowing) + + + +**BREAKING** read-scope narrowing, shipped as `minor` under the repo's launch-window convention for breaking changes (ADR-0131 D1, D14). Step 7 of the 18.x organization-ownership line: it lands after the upgrade ceremony opens. + +- **Before.** `SqlDriver.applyTenantScope` compiled a tenant scope as `(organization_id = :tenant OR organization_id IS NULL)`, and the `group` posture's union as `(organization_id IN (…) OR organization_id IS NULL)`. That was the v17 reading, in which a row with no organization stood for platform state, and every tenant's scoped read, count, update and delete reached it. The Turso remote face compiles the same scope, so it answered the same rows. +- **Now.** The scope is `organization_id = :tenant`, or `organization_id IN (…)` under `group`. NULL is not an owner: a row with no organization is no tenant's, and only an unscoped read (no tenant on the call) reaches it. The lifecycle sweep's global retention and archive pass over an `organization_id` partition is a bare `$nin` of the overriding tenants: it no longer selects organization-less rows. +- **Unchanged.** A call with no tenant context and an object with no tenant column are unscoped, as before. The `single` posture's driver reads carry no tenant scope at all (ADR-0131 D8). The `sys_audit_log` attribution partition (`tenant_id`, ADR-0131 D7) keeps its NULL arm: a deployment-level ledger row is about no organization by design, and the global pass still reaps it. Insert stamping (`injectTenantOnInsert`) is unchanged. + +**Who is affected.** Only a database that still holds organization-less rows in a tenant-scoped table, and only under a walled posture (`group` / `isolated`) or a caller that passes `tenantId` to the driver itself. A certified 18.x database holds none: a v17 database is refused at boot until `os migrate organization-ownership` certifies it, and a fresh install never writes one. + +**The remedy.** If a scoped read misses a row you expect, the row has no organization: run the upgrade ceremony (`os migrate organization-ownership` to review its plan, then the same command with `--apply`), which stamps each such row with its owner, or stamp it yourself (`organization_id = `). diff --git a/packages/drivers/driver-sql/src/sql-driver-21227-create-answers-stored-row.test.ts b/packages/drivers/driver-sql/src/sql-driver-21227-create-answers-stored-row.test.ts index 62528ec076c..ccf0a4050a9 100644 --- a/packages/drivers/driver-sql/src/sql-driver-21227-create-answers-stored-row.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-21227-create-answers-stored-row.test.ts @@ -195,6 +195,22 @@ function suite(face: Face) { ]); }); + it('a batch with no tenant on the call answers every row it stored, the organization-less one included', async () => { + // [ADR-0131 D1] The tenant scope carries no `IS NULL` arm, so the rows' + // own union (`IN ('org_b')`) would miss the unstamped row this call just + // wrote. The read is scoped by that union only when every row carries a + // tenant; here the call's own options (none) decide, and the ids bound it. + const batch = await driver.bulkCreate(TABLE, [ + { id: 't-m1', title: 'named', organization_id: 'org_b' }, + { id: 't-m2', title: 'unstamped' }, + ]); + expect(batch.map((r) => [r.id, r.organization_id ?? null])).toEqual([ + ['t-m1', 'org_b'], + ['t-m2', null], + ]); + expect(batch[1]).toEqual(await stored('t-m2')); + }); + it('inside a caller transaction, create and bulkCreate answer the uncommitted rows and the rollback keeps', async () => { const trx = await driver.beginTransaction(); let single!: Record; diff --git a/packages/drivers/driver-sql/src/sql-driver-tenant-scope-read-doors.test.ts b/packages/drivers/driver-sql/src/sql-driver-tenant-scope-read-doors.test.ts index 8777370c739..1b57c25168c 100644 --- a/packages/drivers/driver-sql/src/sql-driver-tenant-scope-read-doors.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-tenant-scope-read-doors.test.ts @@ -58,12 +58,14 @@ * answering `[]` to an unscoped admin read is a different defect, not a fix. So * each exposure case is stated beside the read it must not become. * - * The NULL-org row is here for the same reason. `applyTenantScope` emits - * `(field = ? OR field IS NULL)` on purpose (#2734: strict equality hid every - * platform-seeded RBAC row from every tenant admin). A door "fixed" with a bare - * equality would pass a naive cross-tenant assertion and silently lose p1, so - * p1 is what distinguishes routing through the chokepoint from reimplementing - * a worse copy of it. + * The NULL-org row is here for the same reason. The measurement above is the + * v17 chokepoint, `(field = ? OR field IS NULL)` (#2734). [ADR-0131 D1, D14] + * 18.x retired that arm after the upgrade ceremony: NULL is not an owner, so + * `applyTenantScope` emits `field = ?` alone and no scoped read reaches p1. p1 + * now holds every door to the SAME rule from both sides: absent under a + * tenant, present with none. A door that kept its own copy of the old arm, or + * that dropped the no-tenant early-out, answers p1 where the chokepoint does + * not. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -136,8 +138,8 @@ describe('driver-sql — every read door routes through applyTenantScope (#6792) await driver.create(TABLE, { id: 'a2', organization_id: 'org_a', name: 'A2', balance: 20 } as any); await driver.create(TABLE, { id: 'b1', organization_id: 'org_b', name: 'B1', balance: 30 } as any); await driver.create(TABLE, { id: 'b2', organization_id: 'org_b', name: 'B2', balance: 40 } as any); - // Platform row: belongs to no tenant, so it belongs to no OTHER tenant - // either and must stay visible (#2734). + // An organization-less row: no tenant's, so no scoped read reaches it + // (ADR-0131 D1); only an unscoped one does. await driver.create(TABLE, { id: 'p1', name: 'P1', balance: 50 } as any); await driver.create(UNSCOPED_TABLE, { id: 'l1', name: 'L1', balance: 1 } as any); @@ -155,7 +157,7 @@ describe('driver-sql — every read door routes through applyTenantScope (#6792) const rows = await driver.findWithWindowFunctions(TABLE, { ...WINDOW } as any, { tenantId: 'org_a', } as any); - expect(ids(rows)).toEqual(['a1', 'a2', 'p1']); + expect(ids(rows)).toEqual(['a1', 'a2']); expect(ids(rows)).not.toContain('b1'); expect(ids(rows)).not.toContain('b2'); }); @@ -163,8 +165,8 @@ describe('driver-sql — every read door routes through applyTenantScope (#6792) it('gives each tenant its own rows, and only its own', async () => { const a = await driver.findWithWindowFunctions(TABLE, { ...WINDOW } as any, { tenantId: 'org_a' } as any); const b = await driver.findWithWindowFunctions(TABLE, { ...WINDOW } as any, { tenantId: 'org_b' } as any); - expect(ids(a)).toEqual(['a1', 'a2', 'p1']); - expect(ids(b)).toEqual(['b1', 'b2', 'p1']); + expect(ids(a)).toEqual(['a1', 'a2']); + expect(ids(b)).toEqual(['b1', 'b2']); }); it('agrees with `find()` on the same read — one driver, one answer', async () => { @@ -173,9 +175,9 @@ describe('driver-sql — every read door routes through applyTenantScope (#6792) expect(ids(viaWindow)).toEqual(ids(viaFind)); }); - it('keeps the NULL-org platform row visible — the chokepoint, not a bare equality (#2734)', async () => { + it('does not reach the organization-less row under a tenant — NULL is not an owner (ADR-0131 D1)', async () => { const rows = await driver.findWithWindowFunctions(TABLE, { ...WINDOW } as any, { tenantId: 'org_a' } as any); - expect(ids(rows)).toContain('p1'); + expect(ids(rows)).not.toContain('p1'); }); it('honours the ADR-0105 D2 union posture the same way the chokepoint does (#3623)', async () => { @@ -183,13 +185,13 @@ describe('driver-sql — every read door routes through applyTenantScope (#6792) tenantId: 'org_a', tenantIds: ['org_a', 'org_b'], } as any); - expect(ids(rows)).toEqual(['a1', 'a2', 'b1', 'b2', 'p1']); + expect(ids(rows)).toEqual(['a1', 'a2', 'b1', 'b2']); const narrow = await driver.findWithWindowFunctions(TABLE, { ...WINDOW } as any, { tenantId: 'org_a', tenantIds: ['org_a'], } as any); - expect(ids(narrow)).toEqual(['a1', 'a2', 'p1']); + expect(ids(narrow)).toEqual(['a1', 'a2']); }); it('still applies the caller\'s own `where` beside the tenant predicate', async () => { @@ -198,10 +200,9 @@ describe('driver-sql — every read door routes through applyTenantScope (#6792) { ...WINDOW, where: { balance: { $gte: 20 } } } as any, { tenantId: 'org_a' } as any, ); - // `p1` (balance 50) is the NULL-org platform row and is legitimately - // visible to org_a — it passes the filter AND the chokepoint. b1/b2 - // (30/40) pass the filter and must not survive the tenant predicate. - expect(ids(rows)).toEqual(['a2', 'p1']); + // `p1` (balance 50) and b1/b2 (30/40) pass the filter, and none of them + // survives the tenant predicate: p1 is no tenant's (ADR-0131 D1). + expect(ids(rows)).toEqual(['a2']); }); it('still computes the window function itself', async () => { @@ -245,7 +246,7 @@ describe('driver-sql — every read door routes through applyTenantScope (#6792) describe('distinct — the third read door, found by the gate rather than by the card', () => { it('does not return another tenant\'s values', async () => { const values = await driver.distinct(TABLE, 'name', undefined, { tenantId: 'org_a' } as any); - expect(values.sort()).toEqual(['A1', 'A2', 'P1']); + expect(values.sort()).toEqual(['A1', 'A2']); expect(values).not.toContain('B1'); expect(values).not.toContain('B2'); }); @@ -260,9 +261,9 @@ describe('driver-sql — every read door routes through applyTenantScope (#6792) const values = await driver.distinct(TABLE, 'name', { balance: { $gte: 20 } } as any, { tenantId: 'org_a', } as any); - // `P1` is the NULL-org platform row (balance 50) — see the window-door - // case above. `B1`/`B2` also pass the filter and must not survive. - expect(values.sort()).toEqual(['A2', 'P1']); + // `P1` is the organization-less row (balance 50) — see the window-door + // case above. It and `B1`/`B2` pass the filter and must not survive. + expect(values.sort()).toEqual(['A2']); }); it('is UNSCOPED with no tenantId — the admin / seed path stays open', async () => { diff --git a/packages/drivers/driver-sql/src/sql-driver-tenant-scope.test.ts b/packages/drivers/driver-sql/src/sql-driver-tenant-scope.test.ts index 2dd289621e4..7dc50a5498c 100644 --- a/packages/drivers/driver-sql/src/sql-driver-tenant-scope.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-tenant-scope.test.ts @@ -133,30 +133,43 @@ describe('SqlDriver tenant scope (organization_id)', () => { }); }); - describe('NULL tenant column = GLOBAL/platform row (#2734)', () => { + // [ADR-0131 D1, D14] NULL is not an owner. The v17 reading (#2734) answered + // an organization-less row to EVERY tenant's scoped read, as platform state; + // 18.x retired that arm with the upgrade ceremony, after which no such row + // exists on a certified database. A row written with no organization is + // therefore no tenant's: only an unscoped read reaches it. + describe('NULL tenant column = no tenant\'s row (ADR-0131 D1)', () => { beforeEach(async () => { - // A platform-seeded row with no organization (bootstrap positions, - // permission sets, business units, pre-org first-boot seeds). - await driver.create('account', { id: 'g1', name: 'Global', tier: 'global' }); + await driver.create('account', { id: 'g1', name: 'Orphan', tier: 'none' }); }); - it('a scoped read still sees the org-less row (any tenant)', async () => { + it('a scoped read does not see the organization-less row, under any tenant', async () => { const rowsA = await driver.find('account', {}, { tenantId: 'org_a' }); const rowsB = await driver.find('account', {}, { tenantId: 'org_b' }); - expect(rowsA.map((r: any) => r.id)).toContain('g1'); - expect(rowsB.map((r: any) => r.id)).toContain('g1'); - // …while cross-tenant rows stay hidden exactly as before. - expect(rowsA.map((r: any) => r.id).sort()).toEqual(['a1', 'a2', 'g1']); + expect(rowsA.map((r: any) => r.id).sort()).toEqual(['a1', 'a2']); + expect(rowsB.map((r: any) => r.id).sort()).toEqual(['b1', 'b2']); }); - it('a scoped by-id read resolves the global row', async () => { + it('a scoped by-id read does not resolve it', async () => { const row = await driver.findOne('account', { where: { id: 'g1' } }, { tenantId: 'org_a' }); - expect(row?.id).toBe('g1'); + expect(row).toBeNull(); }); - it('a scoped count includes the global row', async () => { + it('a scoped count excludes it', async () => { const a = await driver.count!('account', {}, { tenantId: 'org_a' }); - expect(a).toBe(3); // a1, a2, g1 — never org_b's rows + expect(a).toBe(2); // a1, a2 + }); + + it('a scoped update or delete does not reach it', async () => { + await driver.update('account', 'g1', { name: 'Touched' }, { tenantId: 'org_a' }).catch(() => undefined); + await driver.delete('account', 'g1', { tenantId: 'org_a' }).catch(() => undefined); + const row = await driver.findOne('account', { where: { id: 'g1' } }); + expect(row?.name).toBe('Orphan'); + }); + + it('an unscoped read still sees it — no tenant context is no scope', async () => { + const rows = await driver.find('account', {}); + expect(rows.map((r: any) => r.id)).toContain('g1'); }); }); @@ -464,14 +477,14 @@ describe('SqlDriver tenant scope (organization_id)', () => { expect(rows.map(r => r.id)).not.toContain('c1'); }); - it('keeps the NULL-tenant global-row carve-out (#2734) on the union path', async () => { - await driver.create('account', { id: 'g1', name: 'GLOBAL' }); + it('the union path does not reach an organization-less row either (ADR-0131 D1)', async () => { + await driver.create('account', { id: 'g1', name: 'ORPHAN' }); const rows = await driver.find( 'account', {}, - { tenantId: 'org_a', tenantIds: ['org_a'] } as any, + { tenantId: 'org_a', tenantIds: ['org_a', 'org_b'] } as any, ); - expect(rows.map(r => r.id).sort()).toEqual(['a1', 'a2', 'g1']); + expect(rows.map(r => r.id).sort()).toEqual(['a1', 'a2', 'b1', 'b2']); }); it('an empty or malformed tenantIds falls back to tenantId equality (fail toward isolation)', async () => { diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 28ad32650fd..5e40140c8ff 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -7616,11 +7616,16 @@ export class SqlDriver implements IDataDriver { * are never overwritten), a read scoped to the caller's ACTIVE org would miss * a row that really landed. A batch may name several tenants, so the scope is * their union through `tenantIds`, which `applyTenantScope` already compiles - * as `IN (…) OR IS NULL`; the caller's own `tenantIds` membership set is - * replaced, not widened. With no tenant field, or no tenant on the rows and - * none on the call, `applyTenantScope` leaves the read unscoped by its own - * contract. The ids are this call's own and `id` is the PRIMARY KEY, so the - * read cannot answer a row this call did not write, in any organization. + * as `IN (…)`; the caller's own `tenantIds` membership set is replaced, not + * widened. A row written with no tenant is no tenant's, and no tenant scope + * reaches it (ADR-0131 D1), so the rows' union scopes the read only when + * every row carries one. Otherwise the call's own options decide, and a call + * with no tenant leaves a row unstamped only because it carries none: + * `injectTenantOnInsert` fills every absent column on a call that does. With + * no tenant field, or no tenant on the call, `applyTenantScope` leaves the + * read unscoped by its own contract. The ids are this call's own and `id` is + * the PRIMARY KEY, so the read cannot answer a row this call did not write, + * in any organization. * * # A row that is not there * @@ -7646,11 +7651,12 @@ export class SqlDriver implements IDataDriver { ), ] : []; - const scopeOptions: DriverOptions = { - ...options, - tenantId: writtenTenants[0] ?? options?.tenantId, - tenantIds: writtenTenants.length > 0 ? writtenTenants : undefined, - }; + const everyRowCarriesTenant = + tenantField !== null + && written.every((row) => row[tenantField] !== undefined && row[tenantField] !== null && row[tenantField] !== ''); + const scopeOptions: DriverOptions = everyRowCarriesTenant + ? { ...options, tenantId: writtenTenants[0], tenantIds: writtenTenants } + : { ...options }; const builder = this.getBuilder(writeTable, options); this.applyTenantScope(builder, object, scopeOptions); const stored: Record[] = await builder.whereIn( @@ -11475,8 +11481,8 @@ export class SqlDriver implements IDataDriver { // // Placed BESIDE `getBuilder` and above the caller's `where`, which is the // position `findRows()` uses: the predicate has to be on the builder before - // anything reads it, and `applyTenantScope` is what owns the NULL-org - // platform-row and ADR-0105 D2 union semantics. Re-deriving either here + // anything reads it, and `applyTenantScope` is what owns the equality and + // ADR-0105 D2 union semantics. Re-deriving either here // would be a second, worse copy of the wall. this.applyTenantScope(builder, object, options); @@ -15600,27 +15606,19 @@ export class SqlDriver implements IDataDriver { ? rawIds.filter((v: unknown) => typeof v === 'string' && v !== '') : []; if (tenantIds.length > 0) { - return builder.where((b) => { - void b.whereIn(field, tenantIds.map(String)).orWhereNull(field); - }); + return builder.whereIn(field, tenantIds.map(String)); } - // `(field = :tenantId OR field IS NULL)`. The NULL arm is the v17 reading - // (#2734), in which an org-less row stood for platform state (seeded - // positions and permission sets, business units, first-boot seeds). - // ADR-0131 replaces that reading with three rules. A managed package's - // metadata is not in the database: its definitions are sealed, and - // customization is environment-level state (D2, D6). A row with an - // organization column was written by that organization, and NULL is not - // an owner (D1); references to declared items are by machine name (D4). A - // table with no organization column is deployment-level or code-level, - // protected by permissions rather than by this wall (D7). So on a fresh - // 18.0 `single` install, where every insert carries its organization, the - // arm matches nothing. The arm is removed on the 18.x line, after the - // upgrade ceremony (ADR-0131 D14). Rows stamped with a DIFFERENT tenant - // stay invisible either way. - return builder.where((b) => { - void b.where(field, String(tenantId)).orWhereNull(field); - }); + // `field = :tenantId`, and nothing else. [ADR-0131 D1, D14] NULL is not an + // owner: an organization-less row is no tenant's, so no tenant scope reaches + // it. The v17 reading (#2734) added an `OR field IS NULL` arm here, under + // which such a row stood for platform state; ADR-0131 replaced that reading + // (D2, D6, D7) and the 18.x upgrade ceremony retired the rows it served: a + // certified database holds no NULL organization, a fresh install never + // writes one, and a v17 database is refused at boot until it is certified. + // A composition that registers no organization object can still write + // organization-less rows, and it never reaches this line with a tenant: no + // session, guest or system write there carries an organization. + return builder.where(field, String(tenantId)); } /** diff --git a/packages/drivers/driver-turso/src/remote-transport.ts b/packages/drivers/driver-turso/src/remote-transport.ts index 43b26640187..5a6c9470346 100644 --- a/packages/drivers/driver-turso/src/remote-transport.ts +++ b/packages/drivers/driver-turso/src/remote-transport.ts @@ -981,7 +981,7 @@ export type JsonColumnResolver = (object: string, field: string) => JsonColumnFi * * The driver builds it, and this class never decides it. `TursoDriver` asks the * local face's own chokepoint, `SqlDriver.applyTenantScope`, for the predicate - * and hands over what that compiles to: the NULL-organization arm, the `group` + * and hands over what that compiles to: the organization equality, the `group` * posture's membership set, and the no-tenant-context exit are all that * method's. This class is told WHICH rows, not WHY, as with the upsert `fence` * (#21185). Absent means the call carries no scope (no tenant context, or no diff --git a/packages/drivers/driver-turso/src/turso-driver.ts b/packages/drivers/driver-turso/src/turso-driver.ts index fb56e0d7998..88f0db0e362 100644 --- a/packages/drivers/driver-turso/src/turso-driver.ts +++ b/packages/drivers/driver-turso/src/turso-driver.ts @@ -2520,8 +2520,8 @@ export class TursoDriver extends SqlDriver { * {@link SqlDriver.applyTenantScope} a bare Knex query builder and compiles * what the chokepoint added. So the rule's decisions all stay in that one * method: - * - the NULL-organization arm (a platform row with no organization is no - * other tenant's); + * - the organization equality (a row with no organization is no tenant's, + * ADR-0131 D1, so no scope reaches it); * - the `group` posture's membership set (`tenantIds`, the union Layer 0 * enforces); * - the exits for a call with no tenant context and for an object with no diff --git a/packages/drivers/driver-turso/src/turso-local-remote-tenant-scope-parity.test.ts b/packages/drivers/driver-turso/src/turso-local-remote-tenant-scope-parity.test.ts index 1e814c6257a..28f31ace84b 100644 --- a/packages/drivers/driver-turso/src/turso-local-remote-tenant-scope-parity.test.ts +++ b/packages/drivers/driver-turso/src/turso-local-remote-tenant-scope-parity.test.ts @@ -27,9 +27,11 @@ * For each door, on both faces, a call scoped to one organization against * another organization's rows: excluded, `null`, `false`, `0` or untouched on * disk, and the scoped call's same-organization control answers as an unscoped - * call would. A platform row (no organization) is inside the scope on both - * faces: that is the chokepoint's NULL-organization arm, kept by construction - * because the remote face compiles the chokepoint's own predicate. The final + * call would. A row with no organization is OUTSIDE the scope on both faces: + * NULL is not an owner (ADR-0131 D1), and the chokepoint carries no `IS NULL` + * arm since 18.x retired it after the upgrade ceremony (D14). Both faces agree + * by construction, because the remote face compiles the chokepoint's own + * predicate; the orphan row is what holds them to the same answer. The final * test runs one script on both faces and requires equal answers, and that the * answers they agree on are the scoped ones. * @@ -67,13 +69,14 @@ const SEED = [ { id: 'other_1', organization_id: OTHER_ORG, title: 'other one', amount: 100 }, { id: 'other_2', organization_id: OTHER_ORG, title: 'other two', amount: 200 }, { id: 'third_1', organization_id: THIRD_ORG, title: 'third one', amount: 300 }, - { id: 'platform_1', organization_id: null, title: 'platform', amount: 1000 }, + { id: 'orphan_1', organization_id: null, title: 'orphan', amount: 1000 }, ] as const; -const OTHER_IDS = ['other_1', 'other_2', 'third_1']; +/** Every row a scoped call to the caller's organization must not reach: no organization is no tenant's. */ +const OTHER_IDS = ['other_1', 'other_2', 'third_1', 'orphan_1']; /** What a scoped call to the caller's organization may reach. */ -const IN_SCOPE_IDS = ['own_1', 'own_2', 'platform_1']; +const IN_SCOPE_IDS = ['own_1', 'own_2']; /** The two scoped option shapes the engine sends with no Layer 0 predicate above the driver. */ const SCOPED_SHAPES: ReadonlyArray<[string, DriverOptions]> = [ @@ -147,7 +150,7 @@ describe('a tenant-scoped call on BOTH TursoDriver faces answers one row set', ( }); for (const [shape, options] of SCOPED_SHAPES) { - it(`find answers the caller's organization and platform rows only (${shape})`, async () => { + it(`find answers the caller's organization rows only, and no organization-less row (${shape})`, async () => { const driver = drivers[face]; expect(ids(await driver.find(LEDGER.name, {}, options))).toEqual(IN_SCOPE_IDS); // A filter naming another organization's rows still answers none. @@ -187,7 +190,7 @@ describe('a tenant-scoped call on BOTH TursoDriver faces answers one row set', ( { aggregations: [{ function: 'sum', field: 'amount', alias: 'total' }] } as never, SCOPED, ); - expect(Number(total[0]?.total)).toBe(10 + 20 + 1000); + expect(Number(total[0]?.total)).toBe(10 + 20); const grouped = await driver.aggregate( LEDGER.name, @@ -197,6 +200,7 @@ describe('a tenant-scoped call on BOTH TursoDriver faces answers one row set', ( const groups = grouped.map((g) => g.organization_id ?? null); expect(groups).not.toContain(OTHER_ORG); expect(groups).not.toContain(THIRD_ORG); + expect(groups).not.toContain(null); expect(groups).toContain(CALLER_ORG); }); diff --git a/packages/drivers/driver-turso/src/turso-remote-inherited-members.test.ts b/packages/drivers/driver-turso/src/turso-remote-inherited-members.test.ts index 061bf6e7625..e81077ed4d3 100644 --- a/packages/drivers/driver-turso/src/turso-remote-inherited-members.test.ts +++ b/packages/drivers/driver-turso/src/turso-remote-inherited-members.test.ts @@ -78,7 +78,7 @@ const TENANT = { name: 'probe_org', fields: { name: { type: 'text' }, organizati const TENANT_ROWS = [ { id: 't1', name: 'A', organization_id: 'org_a' }, { id: 't2', name: 'B', organization_id: 'org_b' }, - { id: 't3', name: 'P', organization_id: null }, + { id: 't3', name: 'P', organization_id: 'org_c' }, ]; /** [#21595] A date and a datetime on ISO-week boundary days, and the empty bucket. */ const BUCKET = { name: 'probe_bucket', fields: { on: { type: 'date' }, at: { type: 'datetime' } } }; @@ -203,7 +203,7 @@ describe('distinct(): answered on the remote face, as the local face answers it' it('a tenant-scoped call is refused on the remote face, where the local face scopes it', async () => { const scoped: Parameters = ['probe_org', 'name', undefined, { tenantId: 'org_a' }]; - expect(sorted(await (await seeded('local')).distinct(...scoped))).toEqual(['A', 'P']); + expect(sorted(await (await seeded('local')).distinct(...scoped))).toEqual(['A']); expectRefused(await refusalOf(async () => (await seeded('remote')).distinct(...scoped)), 'A tenant-scoped `distinct()`'); }); }); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 491bfce234a..b857b0a3d3e 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -7043,7 +7043,7 @@ export class ObjectQL implements IObjectQLEngine { * * 2. **`external != null`** — a federated object (ADR-0015), whose schema * is owned by the REMOTE database (#7738). The driver turns `tenantId` - * into `(organization_id = :tenant OR organization_id IS NULL)`, and + * into `organization_id = :tenant`, and * against a remote table that carries no such column that is a SQL error * on Postgres/MySQL — or worse on SQLite, whose quoted-identifier * fallback reinterprets the unresolvable identifier as the string @@ -9693,8 +9693,9 @@ export class ObjectQL implements IObjectQLEngine { * `reference_not_found`. The tenancy exemptions stay where they are decided: * `buildDriverOptions` withholds `tenantId` for a `tenancy.enabled: false` * (platform-global) object and for a federated one, so a reference to either - * still resolves from an org-bound caller, and a NULL-organization row passes - * the driver's `OR … IS NULL` term. + * still resolves from an org-bound caller. A row with no organization is no + * tenant's (ADR-0131 D1): the driver's scope is the organization equality, + * with no `IS NULL` arm, so an org-bound caller's probe does not find one. * * ## …except a master-detail header, which the caller must be able to read (#22519) * @@ -10072,8 +10073,9 @@ export class ObjectQL implements IObjectQLEngine { * organization outside that scope — another organization, or under `group` * one outside the caller's membership set — is not found, and it binds * exactly as a header that exists nowhere does — absent. The tenancy exemptions stay where they are decided: - * a `tenancy.enabled: false` or federated master gets no `tenantId`, and a - * NULL-organization header passes the driver's `OR … IS NULL` term. A system + * a `tenancy.enabled: false` or federated master gets no `tenantId`. A + * header with no organization is no tenant's (ADR-0131 D1): the driver's + * scope carries no `IS NULL` arm, so a scoped read does not find it. A system * caller with no `tenantId` (seed replay, boot) still reads unscoped. * * `null` on any failure — no relation, no id, header not returned by the diff --git a/packages/objectql/src/lifecycle/lifecycle-service.attribution-partition.test.ts b/packages/objectql/src/lifecycle/lifecycle-service.attribution-partition.test.ts index 5e245ea0760..25e7f6295ea 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.attribution-partition.test.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.attribution-partition.test.ts @@ -155,10 +155,19 @@ function sweepOnce(engine: ObjectQL) { }).sweep(); } -/** The two passes a partitioned sweep issues: the tenant's own window, then everyone else's. */ +/** + * The two passes a partitioned sweep issues: the tenant's own window, then + * everyone else's. On the attribution field the global pass keeps its NULL arm: + * a deployment-level row of the ledger is about no organization by design + * (ADR-0131 D7), and a bare `$nin` would never reap it. On `organization_id` it + * carries none (ADR-0131 D1, D14): after the upgrade ceremony no tenant-scoped + * row is organization-less. + */ const partitioned = (column: string) => [ { created_at: { $lt: isoCutoff('365d') }, [column]: 'org_reg' }, - { created_at: { $lt: isoCutoff('90d') }, $or: [{ [column]: { $nin: ['org_reg'] } }, { [column]: null }] }, + column === 'organization_id' + ? { created_at: { $lt: isoCutoff('90d') }, [column]: { $nin: ['org_reg'] } } + : { created_at: { $lt: isoCutoff('90d') }, $or: [{ [column]: { $nin: ['org_reg'] } }, { [column]: null }] }, ]; const onePass = [{ created_at: { $lt: isoCutoff('90d') } }]; diff --git a/packages/objectql/src/lifecycle/lifecycle-service.no-tenant-column.test.ts b/packages/objectql/src/lifecycle/lifecycle-service.no-tenant-column.test.ts index a784e857655..3c84958cf48 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.no-tenant-column.test.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.no-tenant-column.test.ts @@ -175,7 +175,7 @@ describe('LifecycleService.sweep — an object with no tenant column has no tena { created_at: { $lt: isoCutoff('90d') }, organization_id: 'org_reg' }, { created_at: { $lt: isoCutoff('30d') }, - $or: [{ organization_id: { $nin: ['org_reg'] } }, { organization_id: null }], + organization_id: { $nin: ['org_reg'] }, }, ]); }); diff --git a/packages/objectql/src/lifecycle/lifecycle-service.organization-registry.test.ts b/packages/objectql/src/lifecycle/lifecycle-service.organization-registry.test.ts index 9a66456a8e3..5e2e10613f1 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.organization-registry.test.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.organization-registry.test.ts @@ -166,7 +166,7 @@ describe('LifecycleService.sweep — the tenant scan asks the registry first (#2 { created_at: { $lt: isoCutoff('90d') }, organization_id: 'org_reg' }, { created_at: { $lt: isoCutoff('30d') }, - $or: [{ organization_id: { $nin: ['org_reg'] } }, { organization_id: null }], + organization_id: { $nin: ['org_reg'] }, }, ]); expect(report.errors).toEqual([]); diff --git a/packages/objectql/src/lifecycle/lifecycle-service.test.ts b/packages/objectql/src/lifecycle/lifecycle-service.test.ts index b29220daad9..97208cd5720 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.test.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.test.ts @@ -1210,10 +1210,12 @@ describe('LifecycleService.sweep — Archiver governance (#10528)', () => { * Null handling is the platform's settled answer, not a JS accident: a * value that is not there satisfies no positive comparison (#5298/#5299, * every backend's `nullValueSatisfiesOperator` ends `default: return - * false`). That is exactly why the global pass needs the explicit - * `{ organization_id: null }` branch beside its `$nin` — a bare `$nin` - * skips NULL-org rows — and modelling it here is what makes that branch - * load-bearing in the tenant test rather than decorative. + * false`). That is exactly why the global pass on an ATTRIBUTION field + * keeps an explicit `{ [field]: null }` branch beside its `$nin` — a bare + * `$nin` skips the rows about no organization — and modelling it here is + * what makes that branch load-bearing rather than decorative. On + * `organization_id` the branch is retired (ADR-0131 D1, D14): a bare `$nin` + * there is the rule, not an omission. */ const matchesWhere = (row: Record, where: Record): boolean => Object.entries(where).every(([key, cond]) => { @@ -1313,8 +1315,8 @@ describe('LifecycleService.sweep — Archiver governance (#10528)', () => { const error = vi.fn(); const stores = governedStores({ sys_audit_log: [ - { id: 'old', created_at: at(-200 * DAY), organization_id: null }, - { id: 'young', created_at: at(-2 * DAY), organization_id: null }, + { id: 'old', created_at: at(-200 * DAY), organization_id: 'org_plain' }, + { id: 'young', created_at: at(-2 * DAY), organization_id: 'org_plain' }, ], }); const { engine } = captureEngine([AUDIT_OBJ], { @@ -1361,7 +1363,7 @@ describe('LifecycleService.sweep — Archiver governance (#10528)', () => { it('DEFECT CONTROL (floor): reports a DECLARED archive window below the floor, and still enforces it', async () => { const error = vi.fn(); const stores = governedStores({ - sys_audit_log: [{ id: 'old', created_at: at(-200 * DAY), organization_id: null }], + sys_audit_log: [{ id: 'old', created_at: at(-200 * DAY), organization_id: 'org_plain' }], }); const { engine } = captureEngine([AUDIT_OBJ], { driver: stores.hot, @@ -1398,8 +1400,8 @@ describe('LifecycleService.sweep — Archiver governance (#10528)', () => { const error = vi.fn(); const stores = governedStores({ sys_audit_log: [ - { id: 'past-60d', created_at: at(-70 * DAY), organization_id: null }, - { id: 'within-60d', created_at: at(-40 * DAY), organization_id: null }, + { id: 'past-60d', created_at: at(-70 * DAY), organization_id: 'org_plain' }, + { id: 'within-60d', created_at: at(-40 * DAY), organization_id: 'org_plain' }, ], }); const { engine } = captureEngine([AUDIT_OBJ], { @@ -1428,10 +1430,10 @@ describe('LifecycleService.sweep — Archiver governance (#10528)', () => { it('DEFECT CONTROL (override): a global maxAge override moves the archive cutoff', async () => { const stores = governedStores({ sys_audit_log: [ - { id: 'past-1y', created_at: at(-400 * DAY), organization_id: null }, + { id: 'past-1y', created_at: at(-400 * DAY), organization_id: 'org_plain' }, // Past the DECLARED 90d but inside the overridden 1y: the row that // separates the two policies. Pre-fix it is archived. - { id: 'past-90d-only', created_at: at(-200 * DAY), organization_id: null }, + { id: 'past-90d-only', created_at: at(-200 * DAY), organization_id: 'org_plain' }, ], }); const { engine } = captureEngine([AUDIT_OBJ], { @@ -1455,8 +1457,8 @@ describe('LifecycleService.sweep — Archiver governance (#10528)', () => { // (commit 530c1df65). A fix that always consulted `maxAge` would leave this red. const stores = governedStores({ sys_audit_log: [ - { id: 'expired-400d', created_at: at(-2 * DAY), expires_at: at(-400 * DAY), organization_id: null }, - { id: 'expired-200d', created_at: at(-2 * DAY), expires_at: at(-200 * DAY), organization_id: null }, + { id: 'expired-400d', created_at: at(-2 * DAY), expires_at: at(-400 * DAY), organization_id: 'org_plain' }, + { id: 'expired-200d', created_at: at(-2 * DAY), expires_at: at(-200 * DAY), organization_id: 'org_plain' }, ], }); const { engine } = captureEngine([TTL_ARCHIVE_OBJ], { @@ -1477,7 +1479,7 @@ describe('LifecycleService.sweep — Archiver governance (#10528)', () => { // age-selected one. Both must leave the declared window standing — the // two keys are two windows, not two spellings of one. const ttlStores = governedStores({ - sys_audit_log: [{ id: 'r', created_at: at(-400 * DAY), expires_at: at(-100 * DAY), organization_id: null }], + sys_audit_log: [{ id: 'r', created_at: at(-400 * DAY), expires_at: at(-100 * DAY), organization_id: 'org_plain' }], }); const ttlEngine = captureEngine([TTL_ARCHIVE_OBJ], { driver: ttlStores.hot, @@ -1489,7 +1491,7 @@ describe('LifecycleService.sweep — Archiver governance (#10528)', () => { expect(ttlStores.readsFor('sys_audit_log')).toEqual([{ expires_at: { $lt: isoCutoff('90d') } }]); const ageStores = governedStores({ - sys_audit_log: [{ id: 'r', created_at: at(-100 * DAY), organization_id: null }], + sys_audit_log: [{ id: 'r', created_at: at(-100 * DAY), organization_id: 'org_plain' }], }); const ageEngine = captureEngine([AUDIT_OBJ], { driver: ageStores.hot, @@ -1506,8 +1508,8 @@ describe('LifecycleService.sweep — Archiver governance (#10528)', () => { // retention + archive at 90d, both sweep in the same pass, and only the // named one may move. const stores = governedStores({ - sys_audit_log: [{ id: 'audit-200d', created_at: at(-200 * DAY), organization_id: null }], - sys_metadata_audit: [{ id: 'meta-200d', created_at: at(-200 * DAY), organization_id: null }], + sys_audit_log: [{ id: 'audit-200d', created_at: at(-200 * DAY), organization_id: 'org_plain' }], + sys_metadata_audit: [{ id: 'meta-200d', created_at: at(-200 * DAY), organization_id: 'org_plain' }], }); const { engine } = captureEngine([AUDIT_OBJ, METADATA_AUDIT_OBJ], { driver: stores.hot, @@ -1536,9 +1538,11 @@ describe('LifecycleService.sweep — Archiver governance (#10528)', () => { // makes the tenant leg falsifiable rather than merely unobserved. { id: 'reg-200d', created_at: at(-200 * DAY), organization_id: 'org_reg' }, { id: 'plain-200d', created_at: at(-200 * DAY), organization_id: 'org_plain' }, - // NULL org: covered by the global pass's explicit null branch, never - // by a bare `$nin`. - { id: 'no-org-200d', created_at: at(-200 * DAY), organization_id: null }, + // An organization with no override of its own: the global pass's + // `$nin` covers it. [ADR-0131 D1, D14] No NULL-org row is seeded: after + // the upgrade ceremony a tenant-scoped table holds none, and the + // global pass on `organization_id` carries no NULL arm. + { id: 'other-200d', created_at: at(-200 * DAY), organization_id: 'org_other' }, ], }); const { engine } = captureEngine([AUDIT_OBJ], { @@ -1557,13 +1561,13 @@ describe('LifecycleService.sweep — Archiver governance (#10528)', () => { { created_at: { $lt: isoCutoff('2y') }, organization_id: 'org_reg' }, { created_at: { $lt: isoCutoff('90d') }, - $or: [{ organization_id: { $nin: ['org_reg'] } }, { organization_id: null }], + organization_id: { $nin: ['org_reg'] }, }, ]); - expect(stores.archivedFrom('sys_audit_log')).toEqual(['reg-800d', 'plain-200d', 'no-org-200d']); + expect(stores.archivedFrom('sys_audit_log')).toEqual(['reg-800d', 'plain-200d', 'other-200d']); expect(stores.bulkDeleted).toEqual([ { object: 'sys_audit_log', ids: ['reg-800d'] }, - { object: 'sys_audit_log', ids: ['plain-200d', 'no-org-200d'] }, + { object: 'sys_audit_log', ids: ['plain-200d', 'other-200d'] }, ]); expect(stores.remaining('sys_audit_log')).toEqual(['reg-200d']); @@ -1810,10 +1814,11 @@ describe('LifecycleService.sweep — governance (P4)', () => { created_at: { $lt: isoCutoff('2y') }, organization_id: 'org_reg', }); - // …then the global 30d pass excluding it but INCLUDING NULL-org rows. + // …then the global 30d pass excluding it. [ADR-0131 D1, D14] No NULL arm + // on `organization_id`: no NULL-org row exists after the ceremony. expect(reaps[1].where).toEqual({ created_at: { $lt: isoCutoff('30d') }, - $or: [{ organization_id: { $nin: ['org_reg'] } }, { organization_id: null }], + organization_id: { $nin: ['org_reg'] }, }); expect(reaps).toHaveLength(2); expect(deletes.map((d) => d.where)).toEqual([{ id: 'tenant_row' }, { id: 'global_row' }]); @@ -1857,7 +1862,7 @@ describe('LifecycleService.sweep — governance (P4)', () => { }); expect(reaps[1].where).toEqual({ created_at: { $lt: isoCutoff('30d') }, - $or: [{ organization_id: { $nin: ['org_reg'] } }, { organization_id: null }], + organization_id: { $nin: ['org_reg'] }, ...predicate, }); expect(reaps).toHaveLength(2); @@ -1985,7 +1990,7 @@ describe('LifecycleService.sweep — governance (P4)', () => { { expires_at: { $lt: isoCutoff('2y') }, organization_id: 'org_reg' }, { expires_at: { $lt: isoCutoff('1d') }, - $or: [{ organization_id: { $nin: ['org_reg'] } }, { organization_id: null }], + organization_id: { $nin: ['org_reg'] }, }, ]); }); @@ -3318,7 +3323,7 @@ describe('LifecycleService.sweep — governance tenant scan failure (#12853)', ( { created_at: { $lt: isoCutoff('90d') }, organization_id: 'org_reg' }, { created_at: { $lt: isoCutoff('30d') }, - $or: [{ organization_id: { $nin: ['org_reg'] } }, { organization_id: null }], + organization_id: { $nin: ['org_reg'] }, }, ]); expect(box.deletes).toHaveLength(2); diff --git a/packages/objectql/src/lifecycle/lifecycle-service.ts b/packages/objectql/src/lifecycle/lifecycle-service.ts index f0bbbeb9512..d42236c4920 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.ts @@ -245,6 +245,34 @@ const ATTRIBUTION_PARTITION_COLUMNS: Readonly> = Object.f interface TenantPartition { column: string; windows: Array<{ tenantId: string; maxAge?: string; expireAfter?: string }>; + /** + * Whether a row with no value in `column` is a live row the global pass must + * still cover: `true` for an attribution field only (see + * {@link globalPassSelector}). + */ + nullIsLive: boolean; +} + +/** + * The global pass's selector over a partitioned object: every row no override + * claimed. Which reading applies is decided once, by `tenantWindowsFor`, as + * {@link TenantPartition.nullIsLive}. + * + * - On `organization_id`, the rows of every other organization. [ADR-0131 D1, + * D14] NULL is not an owner, and after the 18.x upgrade ceremony no row of a + * tenant-scoped table carries one: a certified database holds none and a + * fresh install never writes one. The v17 `OR organization_id IS NULL` arm is + * retired with the driver's, so this is the bare `$nin`. + * - On an attribution field ({@link ATTRIBUTION_PARTITION_COLUMNS}), also the + * rows about no organization. There NULL is a live value, not a pre-ceremony + * one: a deployment-level row of the ledger leaves the field NULL by design + * (ADR-0131 D7), and a bare `$nin` would skip it, since a value that is not + * there satisfies no positive comparison (#5298/#5299). That NULL arm stays. + */ +function globalPassSelector(partition: TenantPartition): Record { + const { column, windows, nullIsLive } = partition; + const others = { [column]: { $nin: windows.map((t) => t.tenantId) } }; + return nullIsLive ? { $or: [others, { [column]: null }] } : others; } /** @@ -1482,9 +1510,7 @@ export class LifecycleService { // [commit 7d483e1e5] Per-tenant windows (ADR-0057 §3.2), in the shape `reap()` uses: // each overriding tenant gets its own cutoff on its own rows, then one - // global pass covers everyone else INCLUDING rows with no organization — - // a bare `$nin` would silently skip NULL-org rows, since a value that is - // not there satisfies no positive comparison (#5298/#5299). + // global pass covers everyone else ({@link globalPassSelector}). // // Only genuinely tenant-scoped overrides reach `tenantOverrides` (see // `loadGovernance`), so an object nobody has tuned per tenant still runs @@ -1518,13 +1544,7 @@ export class LifecycleService { const tCutoff = new Date(this.now() - tMs).toISOString(); archived += await archivePass({ [dueField]: { $lt: tCutoff }, [column]: t.tenantId }); } - archived += await archivePass({ - [dueField]: { $lt: cutoff }, - $or: [ - { [column]: { $nin: windows.map((t) => t.tenantId) } }, - { [column]: null }, - ], - }); + archived += await archivePass({ [dueField]: { $lt: cutoff }, ...globalPassSelector(partition) }); } // Cold-side retention: `keep` bounds the archive itself. @@ -1591,7 +1611,9 @@ export class LifecycleService { * organization they are ABOUT in an attribution field the author declared * ({@link ATTRIBUTION_PARTITION_COLUMNS}): that field is then the partition, * and a tenant's override selects the rows about that tenant. The global - * pass's NULL arm keeps the rows about no organization in the global window. + * pass's NULL arm, kept for an attribution field only + * ({@link globalPassSelector}), keeps the rows about no organization in the + * global window. * * Answers `null` when there is no partition: no override, or no column. */ @@ -1607,11 +1629,11 @@ export class LifecycleService { !isFederatedUnprovisionedInjectedColumn(obj, 'organization_id') && resolveInjectedColumnProvenance(obj, 'organization_id') !== 'absent' ) { - return { column: 'organization_id', windows }; + return { column: 'organization_id', windows, nullIsLive: false }; } const attribution = ATTRIBUTION_PARTITION_COLUMNS[obj.name]; if (attribution !== undefined && resolveInjectedColumnProvenance(obj, attribution) === 'author') { - return { column: attribution, windows }; + return { column: attribution, windows, nullIsLive: true }; } return null; } @@ -1688,18 +1710,8 @@ export class LifecycleService { const tCutoff = new Date(this.now() - tMs).toISOString(); accumulate(await reapWhere({ [field]: { $lt: tCutoff }, [column]: t.tenantId, ...scope })); } - // …and the global pass covers everyone else, INCLUDING rows with no - // organization (a bare `$nin` would silently skip NULL-org rows). - accumulate( - await reapWhere({ - [field]: { $lt: cutoff }, - $or: [ - { [column]: { $nin: windows.map((t) => t.tenantId) } }, - { [column]: null }, - ], - ...scope, - }), - ); + // …and the global pass covers everyone else ({@link globalPassSelector}). + accumulate(await reapWhere({ [field]: { $lt: cutoff }, ...globalPassSelector(partition), ...scope })); } report.swept.push({ object, class: lc.class, policy, cutoff, deleted: total }); diff --git a/packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts b/packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts index aa8fe9d144a..d5660786159 100644 --- a/packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts +++ b/packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts @@ -20,10 +20,11 @@ * ## The two organizations a grant can carry, stated side by side * * Ruled 2026-09-02: a rule-materialised grant carries the RULE's organization, - * a direct grant the RECORD's. `deal_p1` — an organization-less record an - * org-A rule still matches through the driver's compatibility arm — is what - * separates the two: under the org-A rule its grant carries ORG_A (the rule's), - * under the platform-global rule it carries nothing (the record's). + * a direct grant the RECORD's. `deal_p1` is an organization-less record. Under + * the platform-global rule its grant carries nothing (the record's). No walled + * org-A sweep reaches it: since 18.x the driver's scope carries no `IS NULL` + * arm (ADR-0131 D1, D14), so the v17 case in which the org-A rule's grant on + * it carried ORG_A no longer arises. * * ## The P4 measurement, pinned * @@ -192,7 +193,7 @@ const WON = { stage: 'won' }; const byRecord = (rows: ShareRow[]) => Object.fromEntries(rows.map((r) => [r.record_id, r.organization_id])); describe("[#14484] a rule-materialised grant carries the RULE's organization", () => { - it("an ORG-STAMPED rule's grants all carry ORG_A — the organization-less record included", async () => { + it("an ORG-STAMPED rule's grants all carry ORG_A, and its walled sweep does not reach the organization-less record", async () => { // [ADR-0131 D8] `isolated`: the sweep's organization scope is the wall's, // and under `single` there is none to keep ORG_B's won deals out. const { rules, shares } = await boot('isolated'); @@ -205,9 +206,10 @@ describe("[#14484] a rule-materialised grant carries the RULE's organization", ( await rules.evaluateRule(rule.id, ORG_A_ADMIN); const rows = await shares(); - // `deal_p1` is the discriminating row: the RECORD carries no organization, - // the RULE does, and the ruling says the grant is the rule's. - expect(byRecord(rows)).toEqual({ deal_a1: ORG_A, deal_p1: ORG_A }); + // `deal_p1` carries no organization, and NULL is not an owner (ADR-0131 D1, + // D14): the sweep's scope is the organization equality, with no `IS NULL` + // arm, so it never matches here and gets no grant. + expect(byRecord(rows)).toEqual({ deal_a1: ORG_A }); expect(rows.every((r) => r.organization_id === ORG_A)).toBe(true); }); @@ -249,8 +251,9 @@ describe("[#14484] a direct grant carries the RECORD's organization", () => { it("an organization-less record falls back to the acting session's organization", async () => { const { sharing, shares } = await boot(); - // `deal_p1` is reachable to an ORG_B session through the driver's - // compatibility arm; the record has nothing to give, the session does. + // `deal_p1` is reachable to an ORG_B session under `single`, where the + // driver carries no organization scope (ADR-0131 D8); the record has + // nothing to give, the session does. await sharing.grant( { object: OBJECT, recordId: 'deal_p1', recipientId: 'u_x' }, { userId: 'u_p', tenantId: ORG_B } as never, @@ -377,8 +380,8 @@ describe("[#14484] a record read that FAILS never stamps the acting session's or // precondition for a wrong stamp — is decided before the stamp, by the // pre-flight's visibility read under the CALLER's context // (`assertCanManageShares` → `isRecordVisible`), which the driver scopes to - // `organization_id = :active OR IS NULL` whenever the context carries an - // organization. So a plain `{ userId, tenantId: ORG_B }` session never sees + // `organization_id = :active` whenever the context carries an organization + // under a wall. So a plain `{ userId, tenantId: ORG_B }` session never sees // `deal_a1` (ORG_A) on any posture, `single` included — measured: the first // spelling of these pins died there with `NOT_FOUND`. Two shapes DO reach it: // @@ -441,9 +444,9 @@ describe('[#14484] the scoped UPDATE half cannot reach a row stamped with a DIFF // re-home case below). // // `SqlDriver.applyTenantScope` scopes the update half to - // `organization_id = ? OR organization_id IS NULL`: a legacy NULL row is - // reachable (pinned above), a row carrying ANOTHER organization is not — - // the wall, not a defect. The review's note 3 read the consequence as a + // `organization_id = ?`: a row carrying ANOTHER organization is not + // reachable — the wall, not a defect — and since 18.x neither is a row with + // none (ADR-0131 D1, D14). The review's note 3 read the consequence as a // SILENT no-op (`grant` returning `{ ...row, ...patch }`, `reconcile` // counting `updated += 1`). Measured on the real engine, it is not: the // engine's update reports the unreachable row as `RECORD_NOT_FOUND` (404), @@ -453,9 +456,10 @@ describe('[#14484] the scoped UPDATE half cannot reach a row stamped with a DIFF // `evaluateAllRulesForObject` catches per RULE and logs). Loud beats a // wrong count, so the behaviour stands; these pins hold the measured shape // so the next reader does not inherit the note's reading. Reachable through - // an organization-less record that two organizations' rules both match (the - // upsert key excludes `source_id`), or a record re-homed after a - // platform-global rule granted it. + // a grant that names a record another organization's grant already holds + // (the upsert key excludes `source_id`). An organization-less record that two + // organizations' rules both matched was the other road in; no walled sweep + // reaches such a record any more, so each pass completes (pinned below). const allShares = async (driver: SqlDriver) => (await driver.find('sys_record_share', {} as never)) as any[]; const brief = (rows: any[]) => Object.fromEntries( @@ -481,14 +485,14 @@ describe('[#14484] the scoped UPDATE half cannot reach a row stamped with a DIFF expect(rows[0]).toMatchObject({ id: 'shr_org_a', access_level: 'read', organization_id: ORG_A, source_id: 'rule_org_a' }); }); - it("reconcile: two organizations' rules on one organization-less record — the second's pass aborts at that record; grants before it stay, its stale revocation never runs", async () => { + it("reconcile: two organizations' rules no longer meet on an organization-less record — neither walled sweep reaches it, so the second pass completes", async () => { const { driver, rules, shares } = await boot('isolated'); const ruleA = await rules.defineRule( { name: 'os14484_note3_a', label: 'ORG_A won', object: OBJECT, criteria: WON, recipientType: 'user', recipientId: 'u_x', accessLevel: 'read' } as never, ORG_A_ADMIN, ); await rules.evaluateRule(ruleA.id, ORG_A_ADMIN); - expect(byRecord(await shares())).toEqual({ deal_a1: ORG_A, deal_p1: ORG_A }); + expect(byRecord(await shares())).toEqual({ deal_a1: ORG_A }); const ORG_B_ADMIN = { ...ORG_A_ADMIN, tenantId: ORG_B } as unknown as ExecutionContext; const ruleB = await rules.defineRule( @@ -497,26 +501,22 @@ describe('[#14484] the scoped UPDATE half cannot reach a row stamped with a DIFF ); // A stale rule-B row on a record the rule does not match (`deal_a2` is // `lost`): a completed pass revokes it. It is the witness that the pass - // never reached its revoke loop. + // reached its revoke loop. await driver.create('sys_record_share', { id: 'shr_stale_b', object_name: OBJECT, record_id: 'deal_a2', recipient_type: 'user', recipient_id: 'u_x', access_level: 'edit', source: 'rule', source_id: ruleB.id, organization_id: ORG_B, created_at: '2026-01-01T00:00:00Z', } as never); - // The ORG_B sweep matches deal_b1, deal_b2 and — through the driver's - // compatibility arm — deal_p1, in that order. The evaluator keys its - // existing set by its OWN rule id, so all three are "new" to it; inside - // `grant`, deal_p1's upsert lookup (no `source_id` in the key) finds rule - // A's ORG_A row and takes the update half, which the ORG_B scope cannot - // reach — and that is where the pass dies. - await expect(rules.evaluateRule(ruleB.id, ORG_B_ADMIN)).rejects.toMatchObject({ code: 'RECORD_NOT_FOUND', status: 404 }); + // [ADR-0131 D1, D14] The ORG_B sweep matches deal_b1 and deal_b2 only: the + // driver's scope carries no `IS NULL` arm, so `deal_p1` — where the v17 + // reading made both rules meet, and the second pass died on the first's + // row — is reached by neither. + await rules.evaluateRule(ruleB.id, ORG_B_ADMIN); expect(brief(await allShares(driver))).toEqual({ deal_a1: { organization_id: ORG_A, access_level: 'read', source_id: ruleA.id }, - deal_p1: { organization_id: ORG_A, access_level: 'read', source_id: ruleA.id }, // rule A's row, untouched - deal_b1: { organization_id: ORG_B, access_level: 'edit', source_id: ruleB.id }, // written before the abort - deal_b2: { organization_id: ORG_B, access_level: 'edit', source_id: ruleB.id }, // written before the abort - deal_a2: { organization_id: ORG_B, access_level: 'edit', source_id: ruleB.id }, // stale, NOT revoked + deal_b1: { organization_id: ORG_B, access_level: 'edit', source_id: ruleB.id }, + deal_b2: { organization_id: ORG_B, access_level: 'edit', source_id: ruleB.id }, }); }); diff --git a/packages/plugins/plugin-sharing/src/rule-criteria-org-scope.test.ts b/packages/plugins/plugin-sharing/src/rule-criteria-org-scope.test.ts index 3bc2ebc03e8..ad0d6df2b2d 100644 --- a/packages/plugins/plugin-sharing/src/rule-criteria-org-scope.test.ts +++ b/packages/plugins/plugin-sharing/src/rule-criteria-org-scope.test.ts @@ -29,8 +29,7 @@ * ## Why a real driver * * The scope is a DRIVER decision: `SqlDriver.applyTenantScope` turns - * `DriverOptions.tenantId` into `(organization_id = ? OR organization_id IS - * NULL)`, and the engine threads it as the projection of the tenant wall's + * `DriverOptions.tenantId` into `organization_id = ?`, and the engine threads it as the projection of the tenant wall's * verdict (ADR-0131 D8, `ObjectQLEngine.buildDriverOptions`) — so only under * a walled posture: the org-stamped cases boot `isolated`, and under `single` * there is no organization scope for a sweep to keep. @@ -48,34 +47,23 @@ * declared to perform (#7795, documented at the `deleteRule` guard). So each * org-stamped case is stated beside the null-org case it must not become. * - * The NULL-org RECORD is here for the third direction: `applyTenantScope` - * emits `field = ? OR field IS NULL`, so a scope "fixed" with a bare equality - * would pass the cross-org assertions and silently lose that record. `deal_p1` - * is what distinguishes ROUTING THROUGH THE CHOKEPOINT from reimplementing a - * worse copy of it — and that is the whole of what it pins. + * The NULL-org RECORD is here for the third direction. [ADR-0131 D1, D14] + * NULL is not an owner: `applyTenantScope` emits `field = ?` alone, since 18.x + * retired the v17 `OR field IS NULL` arm after the upgrade ceremony, so an + * org-stamped sweep routed through the chokepoint does NOT reach `deal_p1`, + * while the platform-global sweep, which carries no scope, still does. A sweep + * that kept its own copy of the retired arm would grant it. * * ## What `deal_p1` does and does NOT assert (#10103) * - * It used to be justified as the platform bucket: a NULL organization marks a - * platform row that must stay visible to every tenant (#2734, ADR-0120 D3). - * That reading is HISTORICAL BEHAVIOUR, SUPERSEDED as a general rule. The - * ruling on #10103 makes the single general rule the opposite one — a row - * belonging to no organization is INVALID STATE, to be refused or warned about - * loudly, never a platform-wide default — and demotes NULL-means-global to a - * DRIVER-LEVEL COMPATIBILITY NOTE. The RBAC catalog that reading was invented - * for (#2734's "every tenant admin saw ZERO RBAC rows") is now materialized per - * organization instead, so no NULL row is load-bearing anywhere. - * - * The assertion is nevertheless CORRECT and is kept verbatim, because the wall - * is untouched at both layers: `applyTenantScope` still emits its `OR ... IS - * NULL` arm — that arm IS the compatibility behaviour — so an org-stamped rule - * routed through it still matches an organization-less record. What changed is - * the JUSTIFICATION, not the behaviour: this case pins that the scope goes - * through the driver's chokepoint, and it would be equally load-bearing if the - * arm existed for no reason at all. Reading `deal_p1` as an endorsement of - * NULL-means-global is what is now wrong; deleting it would cost this suite its - * most valuable axis, and weakening it to a bare-equality expectation would - * pin the defect. + * It used to be justified as the platform bucket: a NULL organization marked a + * platform row that had to stay visible to every tenant (#2734, ADR-0120 D3). + * The ruling on #10103 superseded that reading (a row belonging to no + * organization is INVALID STATE, never a platform-wide default) and kept the + * arm as a driver-level compatibility note; ADR-0131 D14 retired the note with + * the ceremony that removes the rows it served. `deal_p1` stays as the record + * no organization's sweep reaches: a certified database holds no such row, so + * this is the shape a stray one would take, not a population a tenant relies on. */ import { describe, it, expect, afterEach } from 'vitest'; @@ -165,11 +153,9 @@ const open: SqlDriver[] = []; * deal_a2 ORG_A stage=lost <- ORG_A's own non-match (criteria still bite) * deal_b1 ORG_B stage=won <- the cross-org match this card is about * deal_b2 ORG_B stage=won <- second one, so a count cannot pass by luck - * deal_p1 (null) stage=won <- organization-less row, still returned by - * `applyTenantScope`'s compatibility arm. - * [#10103] Kept as the chokepoint-vs-bare- - * equality probe, no longer as a claim that - * NULL means platform-wide — see the header. + * deal_p1 (null) stage=won <- organization-less row: no org-stamped + * sweep reaches it (ADR-0131 D1), the + * platform-global one does — see the header. */ async function boot(posture?: 'single' | 'isolated' | 'group'): Promise { const driver = new SqlDriver({ @@ -266,16 +252,13 @@ describe('[#10119] sharing-rule criteria sweep is scoped to the rule\'s organiza // failure names them rather than printing a set diff. expect(granted).not.toContain('deal_b1'); expect(granted).not.toContain('deal_b2'); - // ORG_A's own match, plus the organization-less row that `applyTenantScope` - // still returns through its compatibility arm. [#10103] That arm is - // driver-level COMPATIBILITY behaviour, not the platform-bucket rule it - // was once justified as — see this file's header. `deal_p1` earns its - // place here by distinguishing a scope routed through the chokepoint from - // one reimplemented as a bare equality, which is true either way. - // `deal_a2` is absent because the CRITERIA still bite — scoping did not - // replace them. - expect(granted).toEqual(['deal_a1', 'deal_p1']); - expect(result.matchedRecords).toBe(2); + // ORG_A's own match, and not the organization-less row: NULL is not an + // owner (ADR-0131 D1), so the scope routed through the chokepoint does + // not reach `deal_p1` — see this file's header. `deal_a2` is absent + // because the CRITERIA still bite — scoping did not replace them. + expect(granted).not.toContain('deal_p1'); + expect(granted).toEqual(['deal_a1']); + expect(result.matchedRecords).toBe(1); }); it('a NULL-ORG (platform-global) rule still sweeps EVERY organization', async () => { diff --git a/packages/plugins/plugin-sharing/src/share-link-organization-stamp.test.ts b/packages/plugins/plugin-sharing/src/share-link-organization-stamp.test.ts index 590ab9bb58e..b7cd9fef9da 100644 --- a/packages/plugins/plugin-sharing/src/share-link-organization-stamp.test.ts +++ b/packages/plugins/plugin-sharing/src/share-link-organization-stamp.test.ts @@ -20,7 +20,10 @@ * 1. walled: the row carries the RECORD's organization — even when the caller * acts in another one (the link exposes the record, so it belongs where * the record does); - * 2. walled: an organization-less record takes the caller's organization; + * 2. walled: an organization-less record takes the caller's organization — + * reached by a system caller acting in one; a session acting in one does + * not reach it at all (ADR-0131 D1: NULL is not an owner, and the + * driver's scope carries no `IS NULL` arm), so it mints nothing; * 3. `single`: neither names one — the Default Organization (the only one); * 4. walled: neither names one — refused with the engine's ADR-0112 envelope, * and no row is written. @@ -112,10 +115,19 @@ describe('[ADR-0131 D9] a minted share link carries an organization', () => { it("walled: an organization-less record takes the caller's organization", async () => { const { service, links } = await boot('isolated', [ORG_A, ORG_B]); - await service.createLink({ object: OBJECT, recordId: 'brief_p' }, caller(ORG_B)); + await service.createLink({ object: OBJECT, recordId: 'brief_p' }, { ...caller(ORG_B), isSystem: true } as never); expect(await links()).toEqual([{ record_id: 'brief_p', organization_id: ORG_B }]); }); + it('walled: a session acting in an organization does not reach an organization-less record — refused, and no row is written', async () => { + const { service, links } = await boot('isolated', [ORG_A, ORG_B]); + const refusal = await service + .createLink({ object: OBJECT, recordId: 'brief_p' }, caller(ORG_B)) + .then(() => undefined, (err: unknown) => err); + expect(refusal).toMatchObject({ code: 'FORBIDDEN', status: 403 }); + expect(await links()).toEqual([]); + }); + it('single: neither names one — the Default Organization, the only one there is', async () => { const { service, links } = await boot('single', [DEFAULT_ORG]); const link = await service.createLink({ object: OBJECT, recordId: 'brief_p' }, caller()); diff --git a/packages/services/service-storage/src/tenant-audit-update-delete-half-repairs.test.ts b/packages/services/service-storage/src/tenant-audit-update-delete-half-repairs.test.ts index e19db0abd7f..8cf0f8d22b1 100644 --- a/packages/services/service-storage/src/tenant-audit-update-delete-half-repairs.test.ts +++ b/packages/services/service-storage/src/tenant-audit-update-delete-half-repairs.test.ts @@ -710,7 +710,10 @@ beforeEach(async () => { await driver.initObjects([AUDITED_OBJECT]); await driver.create('sys_file', { id: 'own', key: 'a', status: 'pending', organization_id: 'org_A' }, { tenantId: 'org_A' }); await driver.create('sys_file', { id: 'other', key: 'b', status: 'pending', organization_id: 'org_B' }, { tenantId: 'org_B' }); - await driver.create('sys_file', { id: 'legacy', key: 'c', status: 'pending', organization_id: null }, { tenantId: 'org_A' }); + // No tenant on the call: a tenant on it would stamp the NULL column + // (`injectTenantOnInsert` fills an absent value), and the row would not be + // organization-less at all. + await driver.create('sys_file', { id: 'legacy', key: 'c', status: 'pending', organization_id: null }, { bypassTenantAudit: true }); warns = []; }); @@ -769,18 +772,20 @@ describe('[#13178] the driver leg: what the threaded tenantId actually buys', () expect(await readStatus('own')).toBe('committed'); }); - it('an organization-less row stays reachable — the pre-#12928 population is NOT stranded', async () => { - // This is the load-bearing one. #12928 was ruled FORWARD STAMP ONLY, so - // every session row written before it carries NULL by decision, and #12745's - // backfill did not claim to reach every `sys_file` row either. Strict - // equality here would have stranded exactly those rows mid-upload; the - // driver's `OR … IS NULL` arm (#2734's global-row fail-open) is why it does - // not. ⛔ This asserts that behaviour, it does not change or re-decide it. + it('an organization-less row is not reachable either — NULL is not an owner (ADR-0131 D1)', async () => { + // #12928 was ruled FORWARD STAMP ONLY, so the session rows written before + // it carried NULL, and the driver's v17 `OR … IS NULL` arm kept them + // reachable mid-upload. [ADR-0131 D14] 18.x retired that arm after the + // upgrade ceremony, which stamps each such `sys_file` / `sys_upload_session` + // row from its holder and holds certification on any it cannot attribute: + // a certified database carries none. So a scoped write reaches only its + // own organization's rows. + expect(await readStatus('legacy')).toBe('pending'); await driver.update('sys_file', 'legacy', { status: 'committed' }, { tenantId: 'org_A' }); - expect(await readStatus('legacy')).toBe('committed'); + expect(await readStatus('legacy')).toBe('pending'); await driver.delete('sys_file', 'legacy', { tenantId: 'org_A' }); - expect(await readStatus('legacy')).toBeNull(); + expect(await readStatus('legacy')).toBe('pending'); }); }); @@ -898,9 +903,9 @@ describe('[#21908] D2. ⛔ under the opt-in, a door tenant still cannot reach an beforeEach(async () => { // Seeded under the default posture: an organization-less row is exactly - // what the walled posture refuses to WRITE now, and exactly the legacy - // population the driver's `OR … IS NULL` arm keeps reachable. The doors - // then run walled. + // what the walled posture refuses to WRITE now, and the legacy population + // the driver's retired v17 `OR … IS NULL` arm used to keep reachable. The + // doors then run walled. delete process.env.OS_TENANCY_POSTURE; sql = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); driverWrites = []; @@ -1003,20 +1008,31 @@ describe('[#21908] D2. ⛔ under the opt-in, a door tenant still cannot reach an for (const w of driverWrites) expect(w.options?.tenantId).toBe('org_A'); }); - it('still works: the caller’s own row and an organization-less row are reached', async () => { + it('still works: the caller’s own row is reached', async () => { // A refusal that refused everything would score green above; this is the // other half. expect(await store.updateFile('own', { status: 'committed' }, { organizationId: 'org_A' })).toBeTruthy(); - expect(await store.updateSession('legacy', { status: 'completed' }, { organizationId: 'org_A' })).toBeTruthy(); - await store.deleteFile('legacy', { organizationId: 'org_A' }); await store.deleteSession('own', { organizationId: 'org_A' }); expect((await stored('sys_file', 'own'))?.status).toBe('committed'); - expect((await stored('sys_upload_session', 'legacy'))?.status).toBe('completed'); - expect(await stored('sys_file', 'legacy')).toBeNull(); expect(await stored('sys_upload_session', 'own')).toBeNull(); // …and the stored tenant column is the one the row already had. expect((await stored('sys_file', 'own'))?.organization_id).toBe('org_A'); + }); + + it('an organization-less row is refused like a foreign one — NULL is not an owner (ADR-0131 D1, D14)', async () => { + // The pre-#12928 population the v17 `OR … IS NULL` arm kept reachable is + // stamped by the upgrade ceremony before a database boots; a stray one is + // no organization's, so the door's scope does not reach it. + await expect( + store.updateSession('legacy', { status: 'completed' }, { organizationId: 'org_A' }), + ).rejects.toMatchObject(notFound('sys_upload_session', 'update')); + await expect(store.deleteFile('legacy', { organizationId: 'org_A' })).rejects.toMatchObject( + notFound('sys_file', 'delete'), + ); + + expect((await stored('sys_upload_session', 'legacy'))?.status).toBe('in_progress'); + expect(await stored('sys_file', 'legacy')).not.toBeNull(); expect((await stored('sys_upload_session', 'legacy'))?.organization_id ?? null).toBeNull(); }); });