diff --git a/.changeset/tenancy-tenantfield-no-default.md b/.changeset/tenancy-tenantfield-no-default.md new file mode 100644 index 0000000000..0d2909a8de --- /dev/null +++ b/.changeset/tenancy-tenantfield-no-default.md @@ -0,0 +1,42 @@ +--- +"@objectstack/spec": minor +--- + +fix(spec)!: `tenancy.tenantField` no longer defaults to `'tenant_id'` — an undeclared tenant column stays `undefined` and the driver's `organization_id` fallback is the single source of truth (#5315) + +`TenancyConfigSchema.tenantField` carried `.default('tenant_id')`, so parsing +`tenancy: { enabled: true }` materialized `tenantField: 'tenant_id'` onto every +object that never asked for it. The platform's tenant column is +`organization_id` — the one the kernel injects, the one `rls.zod.ts`'s +`tenantPolicy()` defaults to, and the one the SQL driver actually scopes by. +The default was therefore a value **no consumer could use**: `computeTenantField` +in `driver-sql` honours a declared `tenantField` only when the object really has +that column, so the materialized `'tenant_id'` sent it looking for a column that +does not exist, the branch was skipped, and the fallback to `organization_id` +produced the right answer anyway. A declaration nobody reads (ADR-0078), spelling +a word the vocabulary rejects (ADR-0120 §Terminology fixes the authorable term at +`organization`, refusing `tenant`/`org`). + +**What changes for an author** + +| | before | after | +|:---|:---|:---| +| `tenancy: { enabled: true }` (parsed) | `{ enabled: true, tenantField: 'tenant_id' }` | `{ enabled: true }` | +| effective tenant column | `organization_id` | `organization_id` (unchanged) | +| `tenancy: { enabled: true, tenantField: 'workspace_id' }` | honoured when the column exists | unchanged | + +**The effective tenant column does not move.** That equivalence is pinned end to +end — parse through `ObjectSchema` and then resolve through the driver — by +`#5315 undeclared tenantField resolves to organization_id` in +`packages/drivers/driver-sql/src/sql-driver-tenant-scope.test.ts`. + +**Type-level**: `TenancyConfig['tenantField']` widens from `string` to +`string | undefined`. Code that read the parsed value as a guaranteed string must +handle `undefined` — the correct fallback is `'organization_id'`, matching +`computeTenantField` and `tenantPolicy()`. No such reader existed in this repo, +`objectui`, or `cloud`: both real consumers already guarded (`driver-sql` +`computeTenantField` truthiness-checks it; `@objectstack/lint` +`authoredTenantColumn` falls back to `'organization_id'`). + +Declaring `tenantField` explicitly is unchanged and still honoured — this only +stops the spec from inventing a value the author never wrote. diff --git a/content/docs/data-modeling/objects.mdx b/content/docs/data-modeling/objects.mdx index d94b794c33..75e71bfcc1 100644 --- a/content/docs/data-modeling/objects.mdx +++ b/content/docs/data-modeling/objects.mdx @@ -111,7 +111,20 @@ not object metadata. ```typescript tenancy: { enabled: true, - tenantField: 'tenant_id', +} +``` + +The tenant column defaults to nothing at the spec level: leave `tenantField` +undeclared and the driver scopes by `organization_id` — the kernel-injected +column the RLS predicates use. Declare it only when an object's tenant column +genuinely is not `organization_id`, and only when the object really has that +field (a declared name for a column that does not exist is ignored, and the +`organization_id` fallback applies): + +```typescript +tenancy: { + enabled: true, + tenantField: 'workspace_id', } ``` diff --git a/content/docs/references/data/object.mdx b/content/docs/references/data/object.mdx index 35a9cd9e17..617eaa2d2f 100644 --- a/content/docs/references/data/object.mdx +++ b/content/docs/references/data/object.mdx @@ -312,7 +312,7 @@ Boolean-or-predicates override for a built-in row CRUD affordance. | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | **enabled** | `boolean` | ✅ | Enable multi-tenancy for this object | -| **tenantField** | `string` | ✅ | Field name for tenant identifier | +| **tenantField** | `string` | optional | Column this object is tenant-scoped by. Omit it unless the tenant column genuinely is not the platform's: when undeclared the driver falls back to `organization_id`, the kernel-injected column the RLS predicates and `tenantPolicy()` also assume. A declared name is honoured only when the object really has that field — otherwise the same `organization_id` fallback applies. No default is materialized here on purpose (#5315). | --- 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 40688c4cbf..74ecc4e15c 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 @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { ObjectSchema } from '@objectstack/spec/data'; import { SqlDriver } from '../src/index.js'; /** @@ -219,6 +220,63 @@ describe('SqlDriver tenant scope (organization_id)', () => { }); }); + /** + * #5315 — behavioural-equivalence pin for dropping `.default('tenant_id')` + * from `TenancyConfigSchema.tenantField`. + * + * The contract this pins is deliberately end-to-end: metadata goes through + * `ObjectSchema.parse` (the seam where the default used to materialise) and + * *then* to the driver, because the default was only ever observable on a + * PARSED object. An author who writes `tenancy: { enabled: true }` and no + * `tenantField` must land on `organization_id` — the platform's real tenant + * column — and must keep landing there across this change: + * + * - before: parse filled `tenantField: 'tenant_id'`, `computeTenantField` + * looked for a `tenant_id` column, did not find one, and fell through to + * `organization_id`. The right answer by accident. + * - after: parse leaves `tenantField` undefined, the declared branch is + * skipped outright, and the same fallback yields `organization_id`. + * The right answer on purpose (ADR-0078: no declaration nobody reads). + * + * Same answer, one fewer fiction in between — so this test is GREEN on both + * sides of the change by construction. That is the claim, not a weakness of + * the pin: its job is to fail if anyone reintroduces a default that steers + * the driver at a column the object does not have. + */ + describe('#5315 undeclared tenantField resolves to organization_id (parse → driver)', () => { + it('an object declaring only `tenancy: { enabled: true }` scopes by organization_id', async () => { + await driver.disconnect(); + driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + + // Parse through the real spec schema — this is the seam under test. + const parsed = ObjectSchema.parse({ + name: 'ticket', + label: 'Ticket', + tenancy: { enabled: true }, + fields: { + organization_id: { type: 'text' }, + subject: { type: 'text' }, + }, + }); + + await driver.initObjects([parsed as any]); + + // The effective tenant column, whatever the parse did or did not fill in. + expect((driver as any).tenantFieldByTable['ticket']).toBe('organization_id'); + + // …and it actually isolates, rather than merely being recorded. + await driver.create('ticket', { id: 't1', subject: 'A' }, { tenantId: 'org_a' }); + await driver.create('ticket', { id: 't2', subject: 'B' }, { tenantId: 'org_b' }); + const rowsA = await driver.find('ticket', { object: 'ticket' }, { tenantId: 'org_a' }); + expect(rowsA.map((r) => r.id)).toEqual(['t1']); + expect(rowsA[0].organization_id).toBe('org_a'); + }); + }); + describe('declared tenancy.tenantField (custom column)', () => { it('honors obj.tenancy.tenantField when set', async () => { await driver.disconnect(); diff --git a/packages/spec/src/data/object.test.ts b/packages/spec/src/data/object.test.ts index 94a4f980ac..0dcddb80c0 100644 --- a/packages/spec/src/data/object.test.ts +++ b/packages/spec/src/data/object.test.ts @@ -1404,10 +1404,18 @@ describe('ADR-0066 — object access posture (D2) + requiredPermissions (D3)', ( }); describe('TenancyConfigSchema — #2763 strategy/crossTenantAccess removal', () => { - it('accepts the two live knobs and applies the tenantField default', () => { + it('accepts the two live knobs and materializes NO tenantField default (#5315)', () => { + // An undeclared tenant column stays undeclared. The old `.default('tenant_id')` + // invented a column name the platform does not use and no consumer could act + // on — the effective column is resolved by the driver, which falls back to + // `organization_id`. Parsing must not put words in the author's mouth. const result = TenancyConfigSchema.parse({ enabled: true }); expect(result.enabled).toBe(true); - expect(result.tenantField).toBe('tenant_id'); + expect(result.tenantField).toBeUndefined(); + expect(result).toEqual({ enabled: true }); + expect('tenantField' in result).toBe(false); + + // An explicitly declared column still round-trips untouched. expect(TenancyConfigSchema.parse({ enabled: false, tenantField: 'workspace_id' })) .toEqual({ enabled: false, tenantField: 'workspace_id' }); }); diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index a086bb5f70..cd905725f0 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -395,15 +395,39 @@ const strictTenancyError: z.core.$ZodErrorMap = (issue) => { * `.strict()`: unknown keys (incl. the retired `strategy` / * `crossTenantAccess`, #2763) are rejected with guidance, not stripped (#1535). * - * @example Shared database with tenant_id row isolation + * `tenantField` carries **no default** (#5315). It used to default to + * `'tenant_id'`, which no consumer could act on: the platform's tenant column + * is `organization_id` (kernel-injected; the same column `tenantPolicy()` in + * `security/rls.zod.ts` and the RLS predicates assume), and the SQL driver's + * `computeTenantField` honours a declared name only when the object actually + * has that field — so the materialized `'tenant_id'` merely sent it looking for + * a column that did not exist before falling back to `organization_id` anyway. + * A declaration nobody reads is exactly what ADR-0078 prohibits, and `tenant` + * is a word ADR-0120 §Terminology refuses for the authorable vocabulary + * (`organization` is the product's noun). Undeclared now stays `undefined` and + * the driver's fallback is the single source of truth. + * + * @example Shared database, platform-default tenant column (organization_id) + * { + * enabled: true + * } + * + * @example An object whose tenant column is genuinely not organization_id * { * enabled: true, - * tenantField: 'tenant_id' + * tenantField: 'workspace_id' * } */ export const TenancyConfigSchema = lazySchema(() => z.object({ enabled: z.boolean().describe('Enable multi-tenancy for this object'), - tenantField: z.string().default('tenant_id').describe('Field name for tenant identifier'), + tenantField: z.string().optional().describe( + 'Column this object is tenant-scoped by. Omit it unless the tenant column ' + + "genuinely is not the platform's: when undeclared the driver falls back to " + + '`organization_id`, the kernel-injected column the RLS predicates and ' + + '`tenantPolicy()` also assume. A declared name is honoured only when the ' + + 'object really has that field — otherwise the same `organization_id` ' + + 'fallback applies. No default is materialized here on purpose (#5315).', + ), }, { error: strictTenancyError }).strict()); /**