From e2cee9659c226823c07b815720d3b1c249d60e82 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 6 Sep 2026 12:26:02 -0400 Subject: [PATCH] =?UTF-8?q?=E2=9C=85=20test:=20Add=20the=20Portable=20Tena?= =?UTF-8?q?nt-Isolation=20Contract=20(#15662)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `policy.spec.ts` proves the tenant rules with no database; `probe.spec.ts` proves the Mongoose binding reaches the wire scoped. Neither answers what a second storage engine is actually required to do. `describeTenantIsolationConformance(harness)` is that answer — 13 behaviours an engine must exhibit, expressed against a small harness interface rather than any engine's query surface. Mongoose supplies the first harness; a PostgreSQL engine satisfies the same suite, which is what turns the seam from hypothetical into real. Engine-specific cases stay in `models/plugins/tenantIsolation.spec.ts`. Writing the contract surfaced one asymmetry worth pinning rather than rediscovering: an insert naming another tenant is refused in strict mode but tolerated otherwise, because pre-tenancy backfill has to write rows for a tenant other than the ambient one. Both halves are now asserted. Test-only; nothing here is bundled. Originally #15585, reopened directly on `dev` because GitHub locks the base of a stacked PR and this change is independent of the runtime fix in #15582. --- .../src/tenant/conformance.mongoose.spec.ts | 83 +++++++ .../data-schemas/src/tenant/conformance.ts | 208 ++++++++++++++++++ 2 files changed, 291 insertions(+) create mode 100644 packages/data-schemas/src/tenant/conformance.mongoose.spec.ts create mode 100644 packages/data-schemas/src/tenant/conformance.ts diff --git a/packages/data-schemas/src/tenant/conformance.mongoose.spec.ts b/packages/data-schemas/src/tenant/conformance.mongoose.spec.ts new file mode 100644 index 0000000000..98f53b9ac4 --- /dev/null +++ b/packages/data-schemas/src/tenant/conformance.mongoose.spec.ts @@ -0,0 +1,83 @@ +import mongoose from 'mongoose'; +import { MongoMemoryServer } from 'mongodb-memory-server'; +import type { TenantRow, TenantEngineHarness } from './conformance'; +import { applyTenantIsolation } from '~/models/plugins/tenantIsolation'; +import { describeTenantIsolationConformance } from './conformance'; + +/** + * The Mongoose binding's answer to the portable contract. A PostgreSQL engine + * satisfies the same suite by supplying its own harness — that is what makes + * the seam real rather than hypothetical. + */ + +interface ConformanceDocument { + name: string; + tenantId?: string; +} + +let server: MongoMemoryServer; +let Model: mongoose.Model; + +/** Reads and writes that must not be tenant-scoped, for fixtures and assertions. */ +const raw = () => mongoose.connection.db!.collection('conformances'); + +const harness: TenantEngineHarness = { + name: 'mongoose', + + async setup() { + server = await MongoMemoryServer.create(); + await mongoose.connect(server.getUri()); + + const schema = new mongoose.Schema({ + name: String, + tenantId: { type: String, index: true }, + }); + applyTenantIsolation(schema); + Model = mongoose.model('Conformance', schema); + }, + + async teardown() { + await mongoose.disconnect(); + await server.stop(); + }, + + async reset() { + await raw().deleteMany({}); + }, + + async seed(rows) { + await raw().insertMany(rows.map((row) => ({ ...row }))); + }, + + async readAllUnscoped() { + const rows = await raw().find({}).toArray(); + return rows.map((row): TenantRow => ({ name: row.name, tenantId: row.tenantId })); + }, + + async insert(row) { + await Model.create({ ...row }); + }, + + async findNames() { + const rows = await Model.find({}).lean(); + return rows.map((row) => row.name); + }, + + async count() { + return Model.countDocuments({}); + }, + + async rename(from, to) { + await Model.updateOne({ name: from }, { $set: { name: to } }); + }, + + async renameAndReassign(from, to, tenantId) { + await Model.updateOne({ name: from }, { $set: { name: to, tenantId } }); + }, + + async remove(name) { + await Model.deleteOne({ name }); + }, +}; + +describeTenantIsolationConformance(harness); diff --git a/packages/data-schemas/src/tenant/conformance.ts b/packages/data-schemas/src/tenant/conformance.ts new file mode 100644 index 0000000000..3d3dd9e1bb --- /dev/null +++ b/packages/data-schemas/src/tenant/conformance.ts @@ -0,0 +1,208 @@ +/* eslint-disable jest/no-export -- This file is a shared contract-test suite, not a + spec: it exports the harness interface and the suite factory that each engine's + own spec imports. The rule guards against accidental exports from spec files. */ +/** + * The portable tenant-isolation contract. + * + * `~/tenant/policy` defines the rules and `policy.spec.ts` proves them with no + * database. This file covers what only an engine can be asked: whether a given + * binding actually wires those rules to the operations it exposes. + * + * An engine satisfies the contract by implementing `TenantEngineHarness` and + * calling `describeTenantIsolationConformance` from a spec. The suite is + * deliberately small — it pins portable behaviour, not any one engine's + * surface, which is why the Mongoose-specific cases stay in + * `models/plugins/tenantIsolation.spec.ts`. + */ + +import { tenantStorage, SYSTEM_TENANT_ID } from '~/config/tenantContext'; +import { resetTenantStrictCache } from './policy'; + +/** A row as the engine stores it. */ +export interface TenantRow { + readonly name: string; + readonly tenantId?: string; +} + +/** + * The operations an engine must expose for the contract to be checkable. + * + * Every method except `seed` and `readAllUnscoped` runs under whatever tenant + * context the suite has established, and is expected to be subject to the + * engine's tenant enforcement. Those two deliberately bypass it so the suite + * can build and inspect cross-tenant fixtures. + */ +export interface TenantEngineHarness { + readonly name: string; + setup(): Promise; + teardown(): Promise; + /** Removes all rows, bypassing enforcement. */ + reset(): Promise; + /** Inserts rows bypassing enforcement, for cross-tenant fixtures. */ + seed(rows: readonly TenantRow[]): Promise; + /** Reads every row bypassing enforcement, for assertions. */ + readAllUnscoped(): Promise; + + insert(row: TenantRow): Promise; + findNames(): Promise; + count(): Promise; + rename(from: string, to: string): Promise; + /** Applies an update that also attempts to set `tenantId`. */ + renameAndReassign(from: string, to: string, tenantId: string): Promise; + remove(name: string): Promise; +} + +const withStrict = async (value: boolean, fn: () => Promise): Promise => { + const previous = process.env.TENANT_ISOLATION_STRICT; + process.env.TENANT_ISOLATION_STRICT = String(value); + resetTenantStrictCache(); + try { + await fn(); + } finally { + if (previous === undefined) { + delete process.env.TENANT_ISOLATION_STRICT; + } else { + process.env.TENANT_ISOLATION_STRICT = previous; + } + resetTenantStrictCache(); + } +}; + +const A = 'tenant-a'; +const B = 'tenant-b'; + +export function describeTenantIsolationConformance(harness: TenantEngineHarness): void { + const asA = (fn: () => Promise): Promise => tenantStorage.run({ tenantId: A }, fn); + const asSystem = (fn: () => Promise): Promise => + tenantStorage.run({ tenantId: SYSTEM_TENANT_ID }, fn); + + describe(`tenant isolation conformance: ${harness.name}`, () => { + beforeAll(() => harness.setup()); + afterAll(() => harness.teardown()); + beforeEach(async () => { + resetTenantStrictCache(); + await harness.reset(); + }); + + describe('reads', () => { + it('sees only the active tenant', async () => { + await harness.seed([ + { name: 'mine', tenantId: A }, + { name: 'theirs', tenantId: B }, + ]); + + expect(await asA(() => harness.findNames())).toEqual(['mine']); + }); + + it('counts only the active tenant', async () => { + await harness.seed([ + { name: 'mine', tenantId: A }, + { name: 'theirs', tenantId: B }, + ]); + + expect(await asA(() => harness.count())).toBe(1); + }); + + it('sees every tenant under the system sentinel', async () => { + await harness.seed([ + { name: 'mine', tenantId: A }, + { name: 'theirs', tenantId: B }, + ]); + + expect([...(await asSystem(() => harness.findNames()))].sort()).toEqual(['mine', 'theirs']); + }); + + it('sees every tenant with no context, for pre-tenancy deployments', async () => { + await harness.seed([ + { name: 'mine', tenantId: A }, + { name: 'theirs', tenantId: B }, + ]); + + expect([...(await harness.findNames())].sort()).toEqual(['mine', 'theirs']); + }); + }); + + describe('writes', () => { + it('stamps the active tenant onto an insert', async () => { + await asA(() => harness.insert({ name: 'fresh' })); + + const rows = await harness.readAllUnscoped(); + expect(rows).toEqual([expect.objectContaining({ name: 'fresh', tenantId: A })]); + }); + + /** + * A sharp edge worth stating explicitly rather than discovering: an insert + * that names another tenant is REFUSED in strict mode but TOLERATED + * otherwise, because pre-tenancy backfill has to be able to write rows for + * a tenant other than the ambient one. An engine must reproduce both + * halves, or deliberately decide not to. + */ + it('refuses a caller-chosen tenant on insert in strict mode', async () => { + await withStrict(true, async () => { + await expect( + asA(() => harness.insert({ name: 'smuggled', tenantId: B })), + ).rejects.toThrow('[TenantIsolation]'); + }); + + expect(await harness.readAllUnscoped()).toHaveLength(0); + }); + + it('tolerates a caller-chosen tenant on insert outside strict mode', async () => { + await withStrict(false, async () => { + await asA(() => harness.insert({ name: 'backfilled', tenantId: B })); + }); + + const rows = await harness.readAllUnscoped(); + expect(rows[0]).toEqual(expect.objectContaining({ name: 'backfilled', tenantId: B })); + }); + + it('cannot rename another tenant row', async () => { + await harness.seed([{ name: 'theirs', tenantId: B }]); + + await asA(() => harness.rename('theirs', 'stolen')); + + const rows = await harness.readAllUnscoped(); + expect(rows[0].name).toBe('theirs'); + }); + + it('cannot delete another tenant row', async () => { + await harness.seed([{ name: 'theirs', tenantId: B }]); + + await asA(() => harness.remove('theirs')); + + expect(await harness.readAllUnscoped()).toHaveLength(1); + }); + + it('refuses an update that reassigns the tenant', async () => { + await harness.seed([{ name: 'mine', tenantId: A }]); + + await expect(asA(() => harness.renameAndReassign('mine', 'moved', B))).rejects.toThrow( + '[TenantIsolation]', + ); + + const rows = await harness.readAllUnscoped(); + expect(rows[0]).toEqual(expect.objectContaining({ name: 'mine', tenantId: A })); + }); + }); + + describe('strict mode', () => { + it('refuses a read with no tenant context', async () => { + await withStrict(true, async () => { + await expect(harness.findNames()).rejects.toThrow('[TenantIsolation]'); + }); + }); + + it('refuses a write with no tenant context', async () => { + await withStrict(true, async () => { + await expect(harness.insert({ name: 'orphan' })).rejects.toThrow('[TenantIsolation]'); + }); + }); + + it('still allows the system sentinel through', async () => { + await withStrict(true, async () => { + await expect(asSystem(() => harness.findNames())).resolves.toEqual([]); + }); + }); + }); + }); +}