From f0aee8a794b406eeec892f3840aa9491e5853d85 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 00:16:35 +0000 Subject: [PATCH 1/2] fix(driver-memory): refuse a call the engine tenant-scoped, instead of answering cross-organization rows The engine scopes an object unless it opts OUT (buildDriverOptions), while the boot guard refuses only an explicit opt-IN (declaresTenantScope). An object that OMITS the tenancy block fell between them: the engine scoped it, the guard never saw it, and the driver discarded the scope and returned every organization's rows. Adds seam 3 to memory-tenancy-guard: assertCallNotTenantScoped judges the scope the engine actually handed over (DriverOptions.tenantId / tenantIds) rather than re-deriving the engine's predicate, and refuses. Called first in every driver door that accepts a DriverOptions, so a refusal leaves no partial effect. Row-level isolation is NOT implemented here and is not the direction: the driver declines to answer. Also records declaresTenantScope's false closing sentence as superseded -- a `single` posture constrains the wall, not the number of organizations. Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg Co-authored-by: Claude --- .../driver-memory/src/memory-driver.ts | 48 ++- .../driver-memory/src/memory-tenancy-guard.ts | 191 ++++++++++- .../src/memory-tenant-scope-refusal.test.ts | 314 ++++++++++++++++++ 3 files changed, 537 insertions(+), 16 deletions(-) create mode 100644 packages/drivers/driver-memory/src/memory-tenant-scope-refusal.test.ts diff --git a/packages/drivers/driver-memory/src/memory-driver.ts b/packages/drivers/driver-memory/src/memory-driver.ts index 1f2dc5ce39..a219221fb7 100644 --- a/packages/drivers/driver-memory/src/memory-driver.ts +++ b/packages/drivers/driver-memory/src/memory-driver.ts @@ -13,7 +13,11 @@ import { hasDanglingLikeEscape, likePatternToRegexSource } from '@objectstack/sp import type { DriverQuery, IDataDriver } from '@objectstack/spec/contracts'; import { Logger, createLogger, nextUtcCalendarDay } from '@objectstack/core'; import { Query, Aggregator } from 'mingo'; -import { assertSingleTenantPosture, assertObjectsNotTenantScoped } from './memory-tenancy-guard.js'; +import { + assertSingleTenantPosture, + assertObjectsNotTenantScoped, + assertCallNotTenantScoped, +} from './memory-tenancy-guard.js'; import { getValueByPath } from './memory-matcher.js'; import { assertFilterConditionShape, @@ -562,6 +566,9 @@ export class InMemoryDriver implements IDataDriver { * result was unchecked. Same repair shape as `update`/`upsert` (#13878). */ async find(object: string, query: DriverQuery, options?: DriverOptions): Promise[]> { + // [#16589] Seam 3: refuse a call the engine scoped — FIRST, before any + // store access or delegation, so a refusal leaves no partial effect. + assertCallNotTenantScoped('find', object, options); this.logger.debug('Find operation', { object, query }); const table = this.getTable(object); @@ -644,6 +651,9 @@ export class InMemoryDriver implements IDataDriver { * to narrow. The same shape `update()` was repaired with (#13878). */ async findOne(object: string, query: DriverQuery, options?: DriverOptions): Promise | null> { + // [#16589] Seam 3: refuse a call the engine scoped — FIRST, before any + // store access or delegation, so a refusal leaves no partial effect. + assertCallNotTenantScoped('findOne', object, options); this.logger.debug('FindOne operation', { object, query }); const results = await this.find(object, { ...query, limit: 1 }, options); @@ -669,6 +679,9 @@ export class InMemoryDriver implements IDataDriver { // breaking change, and method parameters compare bivariantly against the // contract's `Record`, so the declaration is satisfied. async create(object: string, data: Record, options?: DriverOptions): Promise> { + // [#16589] Seam 3: refuse a call the engine scoped — FIRST, before any + // store access or delegation, so a refusal leaves no partial effect. + assertCallNotTenantScoped('create', object, options); this.logger.debug('Create operation', { object, hasData: !!data }); const table = this.getTable(object); @@ -701,6 +714,9 @@ export class InMemoryDriver implements IDataDriver { * `Promise` and no caller was ever asked to narrow. */ async update(object: string, id: string | number, data: Record, options?: DriverOptions): Promise | null> { + // [#16589] Seam 3: refuse a call the engine scoped — FIRST, before any + // store access or delegation, so a refusal leaves no partial effect. + assertCallNotTenantScoped('update', object, options); this.logger.debug('Update operation', { object, id }); const table = this.getTable(object); @@ -733,6 +749,9 @@ export class InMemoryDriver implements IDataDriver { } async upsert(object: string, data: Record, conflictKeys?: string[], options?: DriverOptions): Promise> { + // [#16589] Seam 3: refuse a call the engine scoped — FIRST, before any + // store access or delegation, so a refusal leaves no partial effect. + assertCallNotTenantScoped('upsert', object, options); this.logger.debug('Upsert operation', { object, conflictKeys }); const table = this.getTable(object); @@ -763,6 +782,9 @@ export class InMemoryDriver implements IDataDriver { } async delete(object: string, id: string | number, options?: DriverOptions) { + // [#16589] Seam 3: refuse a call the engine scoped — FIRST, before any + // store access or delegation, so a refusal leaves no partial effect. + assertCallNotTenantScoped('delete', object, options); this.logger.debug('Delete operation', { object, id }); const table = this.getTable(object); @@ -783,6 +805,9 @@ export class InMemoryDriver implements IDataDriver { } async count(object: string, query?: DriverQuery, options?: DriverOptions) { + // [#16589] Seam 3: refuse a call the engine scoped — FIRST, before any + // store access or delegation, so a refusal leaves no partial effect. + assertCallNotTenantScoped('count', object, options); let records = this.getTable(object); if (query?.where) { const mongoQuery = this.convertToMongoQuery(query.where, object); @@ -801,6 +826,9 @@ export class InMemoryDriver implements IDataDriver { // =================================== async bulkCreate(object: string, dataArray: Record[], options?: DriverOptions): Promise[]> { + // [#16589] Seam 3: refuse a call the engine scoped — FIRST, before any + // store access or delegation, so a refusal leaves no partial effect. + assertCallNotTenantScoped('bulkCreate', object, options); this.logger.debug('BulkCreate operation', { object, count: dataArray.length }); const table = this.getTable(object); @@ -845,6 +873,9 @@ export class InMemoryDriver implements IDataDriver { } async updateMany(object: string, query: DriverQuery, data: Record, options?: DriverOptions): Promise { + // [#16589] Seam 3: refuse a call the engine scoped — FIRST, before any + // store access or delegation, so a refusal leaves no partial effect. + assertCallNotTenantScoped('updateMany', object, options); this.logger.debug('UpdateMany operation', { object, query }); const table = this.getTable(object); @@ -887,6 +918,9 @@ export class InMemoryDriver implements IDataDriver { } async deleteMany(object: string, query: DriverQuery, options?: DriverOptions): Promise { + // [#16589] Seam 3: refuse a call the engine scoped — FIRST, before any + // store access or delegation, so a refusal leaves no partial effect. + assertCallNotTenantScoped('deleteMany', object, options); this.logger.debug('DeleteMany operation', { object, query }); const table = this.getTable(object); @@ -947,6 +981,9 @@ export class InMemoryDriver implements IDataDriver { * follows that established convention rather than inventing a second one. */ async bulkUpdate(object: string, updates: { id: string | number, data: Record }[], options?: DriverOptions) { + // [#16589] Seam 3: refuse a call the engine scoped — FIRST, before any + // store access or delegation, so a refusal leaves no partial effect. + assertCallNotTenantScoped('bulkUpdate', object, options); this.logger.debug('BulkUpdate operation', { object, count: updates.length }); const table = this.getTable(object); @@ -1176,6 +1213,9 @@ export class InMemoryDriver implements IDataDriver { * ]); */ async aggregate(object: string, pipeline: Record[] | DriverQuery, options?: DriverOptions): Promise { + // [#16589] Seam 3: refuse a call the engine scoped — FIRST, before any + // store access or delegation, so a refusal leaves no partial effect. + assertCallNotTenantScoped('aggregate', object, options); // ObjectQL's engine calls driver.aggregate(object, AST) with the SAME // DriverQuery shape find() consumes ({ where, groupBy, aggregations }) — not // a MongoDB pipeline. Passing that object into Mingo's Aggregator crashed @@ -1902,6 +1942,9 @@ export class InMemoryDriver implements IDataDriver { // =================================== async syncSchema(object: string, schema: any, options?: DriverOptions) { + // [#16589] Seam 3: refuse a call the engine scoped — FIRST, before any + // store access or delegation, so a refusal leaves no partial effect. + assertCallNotTenantScoped('syncSchema', object, options); // #6915 — metadata-level half of the tenancy guard: an object asking for // row-level isolation cannot get it here, so the table is never allocated. assertObjectsNotTenantScoped([{ object, schema }]); @@ -1949,6 +1992,9 @@ export class InMemoryDriver implements IDataDriver { } async dropTable(object: string, options?: DriverOptions) { + // [#16589] Seam 3: refuse a call the engine scoped — FIRST, before any + // store access or delegation, so a refusal leaves no partial effect. + assertCallNotTenantScoped('dropTable', object, options); if (this.db[object]) { const recordCount = this.db[object].length; delete this.db[object]; diff --git a/packages/drivers/driver-memory/src/memory-tenancy-guard.ts b/packages/drivers/driver-memory/src/memory-tenancy-guard.ts index 4696c2181a..1ad023cd82 100644 --- a/packages/drivers/driver-memory/src/memory-tenancy-guard.ts +++ b/packages/drivers/driver-memory/src/memory-tenancy-guard.ts @@ -1,9 +1,10 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. /** - * In-Memory Driver — multi-tenancy boot guard (#6915, mirroring #3724). + * In-Memory Driver — multi-tenancy refusal guard (#6915 boot seams + #16589 + * per-call seam, mirroring #3724). * - * This driver implements **no row-level tenant isolation**: it never reads + * This driver implements **no row-level tenant isolation**: it never SCOPES by * `DriverOptions.tenantId`, so reads carry no tenant predicate and writes are * never stamped with a tenant column. The SQL family's `resolveTenantField()` + * `applyTenantScope()` layer does not exist here at all — which is why @@ -13,6 +14,12 @@ * `distinct(object, field, query?)` does not even accept a `DriverOptions`, so a * caller has nowhere to pass a tenant even deliberately. * + * Seam 3 below reads `tenantId` / `tenantIds`, and that is not a walking-back of + * the sentence above: it inspects the scope only to **refuse the call**, and + * never to narrow, widen or re-target the rows an operation touches. There is + * still no value of those options under which this driver answers a scoped + * query, which is exactly why it remains outside the chokepoint gate's scan. + * * The platform above the driver assumes tenant isolation is a *platform* * guarantee (object metadata's `tenancy` block, `applySystemFields` injecting * `organization_id`, the engine threading `tenantId` into every driver call). @@ -22,16 +29,63 @@ * * So the driver refuses to run there. It is positioned as a **dev / demo / * in-process** driver (#5704 moved the project's own test backends to sqlite - * `:memory:`) and fails fast — loudly, at startup — the moment it detects - * multi-tenant mode: + * `:memory:`) and fails fast — loudly — the moment it detects multi-tenant + * mode: * * 1. The deployment's tenancy posture is not `single` (deployment-level signal) * → {@link assertSingleTenantPosture}, called from the `InMemoryDriver` * **constructor** and re-checked in `connect()`. * 2. An object declares `tenancy.enabled: true` (metadata-level signal) → * {@link assertObjectsNotTenantScoped}, called from `syncSchema`. + * 3. The engine hands this driver a tenant scope for one call — + * `DriverOptions.tenantId` / `tenantIds` (request-level signal, #16589) → + * {@link assertCallNotTenantScoped}, called from every driver door that + * accepts a `DriverOptions`. + * + * ## Why a THIRD seam — the gap seams 1 and 2 cannot see (#16589) + * + * Seams 1 and 2 read the two STATIC signals: the deployment posture and the + * object's own metadata. The engine's decision to scope is neither. It is + * `Engine.buildDriverOptions`: + * + * ```ts + * const hasTenant = + * execCtx?.tenantId !== undefined && !isTenancyDisabled(objectSchema) && !isFederated; + * ``` + * + * — the engine scopes unless the object opts OUT, while seam 2 refuses only an + * explicit opt-IN. An object that OMITS the `tenancy` block falls between them: + * the engine scopes it, seam 2 never sees it, and seam 1 passes because the + * posture really is `single`. A `single` posture constrains the WALL, not the + * number of organizations, so rows still carry whichever `organization_id` they + * were written with — and this driver used to discard the scope and answer with + * EVERY organization's rows. That is the silent non-isolation this whole guard + * exists to remove, reappearing on the DEFAULT case. + * + * Why the gap cannot be closed at seam 1 or 2 — measured, not assumed: + * `execCtx?.tenantId !== undefined` is a property of the REQUEST, and at + * `syncSchema` time that fact does not exist yet. Every engine call site spells + * `syncSchema(tableName, obj)` and `dropTable(tableName)`, so the DDL doors are + * never handed a `DriverOptions` at all and the scope is structurally invisible + * there. The refusal has to sit where the scope actually arrives. + * + * Widening seam 2 to the engine's own predicate (refuse any object not + * explicitly opted out) was considered and rejected: it would refuse at BOOT on + * objects that merely omit the block — including single-organization apps that + * never carry an organization context and are therefore never scoped. That is + * wider than the refusal this driver owes. The ruled shape is "refuse when + * handed a scope", not "refuse a schema that could one day be scoped". + * + * ## ⚠️ Every isolation measurement previously taken on this driver is VOID * - * ## Why both seams, and not just one + * Before #16589 a suite asserting "tenant A cannot see tenant B's rows" passed + * here trivially — not because isolation worked, but because both tenants' rows + * came back to every caller and the assertion was written against a single + * tenant's fixture. Any isolation property measured on `driver-memory` before + * this refusal landed measured **nothing** and must be **re-taken** on a driver + * that enforces isolation (`@objectstack/driver-sql`, `:memory:` included). + * + * ## Why both boot seams, and not just one * * `connect()` alone is not enough: `ObjectQLEngine.init()` downgrades a driver's * connect rejection to a warning when the operator sets @@ -61,27 +115,44 @@ import { resolveTenancyPosture } from '@objectstack/types'; export const MULTI_TENANT_UNSUPPORTED_CODE = 'MEMORY_MULTI_TENANT_UNSUPPORTED'; const ISSUE_URL = 'https://github.com/objectstack-ai/objectstack/issues/6915'; +const CALL_SEAM_ISSUE_URL = 'https://github.com/objectstack-ai/objectstack/issues/16589'; /** - * Thrown when the in-memory driver is asked to run in a multi-tenant deployment. + * Thrown when the in-memory driver is asked to run in a multi-tenant deployment, + * or to serve one call under a tenant scope it cannot honour. * * Carries {@link MULTI_TENANT_UNSUPPORTED_CODE} as `code` so hosts (CLI boot, * runtime plugin loader, tests) can recognise it without string-matching the * message or relying on cross-realm `instanceof`. + * + * ONE error family covers both, deliberately (#16589): the cause is identical — + * this driver has no row-level tenant isolation — so a host that already + * recognises the boot refusal recognises the per-call one with no new code and + * no second code to learn. `seam` varies only the WORDING, never the `code`. */ export class MemoryMultiTenantUnsupportedError extends Error { public readonly code = MULTI_TENANT_UNSUPPORTED_CODE; - constructor(detected: string, remedy: string) { + constructor(detected: string, remedy: string, seam: 'boot' | 'call' = 'boot') { + const headline = seam === 'boot' ? 'Refusing to start' : 'Refusing to answer'; + const consequence = + seam === 'boot' + ? ` read, update and delete OTHER tenants' records. Rather than run unisolated,\n` + + ` the driver fails at startup.\n` + : ` answering this call would read, update or delete records belonging to OTHER\n` + + ` organizations. Rather than answer it unisolated, the driver refuses it.\n`; + const tracking = + seam === 'boot' + ? ISSUE_URL + : `${CALL_SEAM_ISSUE_URL} (per-call refusal), ${ISSUE_URL} (no isolation here)`; super( - `[driver-memory] Refusing to start: this driver has NO row-level tenant isolation.\n` + + `[driver-memory] ${headline}: this driver has NO row-level tenant isolation.\n` + `\n` + ` Detected: ${detected}\n` + `\n` + - ` InMemoryDriver never reads \`DriverOptions.tenantId\` — reads carry no tenant\n` + - ` predicate and writes are not stamped with a tenant column, so queries would\n` + - ` read, update and delete OTHER tenants' records. Rather than run unisolated,\n` + - ` the driver fails at startup.\n` + + ` InMemoryDriver never scopes by \`DriverOptions.tenantId\` — reads carry no\n` + + ` tenant predicate and writes are not stamped with a tenant column, so\n` + + consequence + `\n` + ` Fix one of:\n` + ` • Use @objectstack/driver-sql (PostgreSQL / MySQL / SQLite) for multi-tenant\n` + @@ -90,7 +161,7 @@ export class MemoryMultiTenantUnsupportedError extends Error { ` is the closest drop-in replacement.\n` + ` ${remedy}\n` + `\n` + - ` Tracking: ${ISSUE_URL}`, + ` Tracking: ${tracking}`, ); this.name = 'MemoryMultiTenantUnsupportedError'; } @@ -107,8 +178,30 @@ export interface TenancyAwareSchema { * Only an **explicit** `tenancy.enabled === true` counts. An absent `tenancy` * block is not treated as a multi-tenant signal here: platform-wide tenant * scoping is driven by the deployment posture (checked separately by - * {@link assertSingleTenantPosture}), and every object in a single-tenant - * deployment omits the block. + * {@link assertSingleTenantPosture}). + * + * ## ⚠️ Superseded reasoning — kept because it was load-bearing (#16589) + * + * This docstring used to close with a second clause, which was **false**: + * + * > …and every object in a single-tenant deployment omits the block. + * + * It reads "single posture ⇒ one tenant ⇒ nothing to scope", and both halves + * fail. `single` constrains the **wall**, not the number of organizations: a + * `single`-posture run was measured holding **13** `sys_organization` rows + * (twelve seeded by the app, one minted by the platform for the admin), with + * rows carrying whichever `organization_id` they were written with. And the + * omission it describes is not the absence of a tenant signal but its default + * PRESENCE — `Engine.buildDriverOptions` scopes an object unless it opts out, + * so "omits the block" is precisely the SCOPED case, not the exempt one. + * + * The sentence is recorded rather than deleted because it is what justified + * this predicate being an opt-IN test, and anyone re-reading that decision needs + * to see the reasoning that was withdrawn. The predicate itself is unchanged and + * still correct **for what it is used for** — seam 2 refuses an object that + * DECLARES isolation. The case the sentence got wrong is not seam 2's to catch: + * it belongs to {@link assertCallNotTenantScoped}, which judges the scope the + * engine actually hands over instead of re-deriving it from metadata. */ export function declaresTenantScope(schema: unknown): boolean { return (schema as TenancyAwareSchema | null | undefined)?.tenancy?.enabled === true; @@ -169,3 +262,71 @@ export function assertObjectsNotTenantScoped( ' if the data is genuinely single-tenant.', ); } + +/** + * Refuse a single driver call that arrives carrying the engine's tenant scope. + * + * This is seam 3 (#16589). It judges the scope the engine **actually handed + * over** — `DriverOptions.tenantId` / `tenantIds` — rather than re-deriving the + * engine's predicate from object metadata. That distinction is the whole point: + * the engine's own reasons for scoping (an `ExecutionContext` tenant, the + * object's ADR-0066 posture, whether the object is federated) stay in the + * engine, and this driver refuses on the one observable fact it can see without + * guessing. A driver that re-derived the predicate would drift from it the first + * time the engine's reasoning changed, and drift here is silent exposure. + * + * Three cases follow from that, and they are what makes the refusal narrow: + * - object omits `tenancy` + a caller with an active organization → the engine + * sends `tenantId` → **refused** (this is the #16589 defect); + * - object declares `tenancy.enabled: false` → the engine sends no `tenantId` + * (ADR-0066, `isTenancyDisabled`) → served unchanged; + * - no organization context at all → no `tenantId` → served unchanged, which is + * the ordinary dev / example-app / single-organization path. + * + * `tenantIds` (ADR-0105 D2, `group` posture) is checked alongside `tenantId` + * because the ruling names both. An absent or EMPTY `tenantIds` is not a scope + * of its own: `DriverOptionsSchema` defines empty as "fall back to `tenantId` + * equality", so treating `[]` as a scope would refuse calls the engine never + * scoped. + * + * ⚠️ **Call this FIRST in the door, before any store access or delegation.** The + * refusal must not leave a partial effect behind: `upsert()` delegates to + * `update()` or `create()`, and a refusal that fell through to the `create` arm + * would land a SECOND row under one primary id — a defect measured on the + * closed round of this card. Refusing at the top of the door makes that + * unreachable, and leaves the store exactly as the call found it. + * + * @param operation the driver door being refused, for the message (`find`, …) + * @param object the object the call names + * @param options the `DriverOptions` as received (may be undefined) + */ +export function assertCallNotTenantScoped( + operation: string, + object: string, + options: unknown, +): void { + const opts = options as { tenantId?: unknown; tenantIds?: unknown } | null | undefined; + const tenantId = opts?.tenantId; + const rawIds = opts?.tenantIds; + const tenantIds = Array.isArray(rawIds) ? rawIds.map((id) => String(id)) : []; + + if (tenantId === undefined && tenantIds.length === 0) return; + + const union = tenantIds.map((id) => `\`${id}\``).join(', '); + const scope = + tenantIds.length > 0 + ? `\`tenantIds\` = [${union}]` + + (tenantId !== undefined ? ` (active \`tenantId\` \`${String(tenantId)}\`)` : '') + : `\`tenantId\` = \`${String(tenantId)}\``; + + throw new MemoryMultiTenantUnsupportedError( + `\`${operation}()\` on \`${object}\` was handed a tenant scope by the engine: ${scope}`, + '• If this object is genuinely platform-global, declare it so:\n' + + ' `tenancy: { enabled: false }` (ADR-0066) stops the engine scoping it and\n' + + ' this driver serves it unchanged. ⛔ Do NOT reach for that to silence this\n' + + ' refusal on data that really is per-organization — it isolates nothing, it\n' + + ' declares there is nothing to isolate, which is the very failure this\n' + + ' refusal exists to surface.', + 'call', + ); +} diff --git a/packages/drivers/driver-memory/src/memory-tenant-scope-refusal.test.ts b/packages/drivers/driver-memory/src/memory-tenant-scope-refusal.test.ts new file mode 100644 index 0000000000..68e90fe34b --- /dev/null +++ b/packages/drivers/driver-memory/src/memory-tenant-scope-refusal.test.ts @@ -0,0 +1,314 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Seam 3 — the per-call tenant-scope refusal (#16589). + * + * The defect: the engine scopes an object unless it opts OUT + * (`buildDriverOptions`), while the boot guard refuses only an explicit opt-IN + * (`declaresTenantScope`). An object that OMITS the `tenancy` block therefore + * fell between them, and this driver discarded the scope and answered with every + * organization's rows. + * + * ## Why this fixture is built the way it is + * + * The acceptance bar for this card is that the SAME read is distinguishable + * **three** ways, because "after, it throws" alone is a reading that cannot + * fail. The closed round of this card hit exactly that trap: with only two + * organizations seeded, a `[ORG_A, ORG_B]` union covered the whole table, so + * "the scope widened" and "no scope ran at all" produced the identical answer. + * + * So three organizations are seeded with row counts 2 / 3 / 7 — deliberately + * chosen so that every reading a driver could produce is a DIFFERENT number: + * + * | reading | rows | what it would mean | + * |:-------------------------------------------|-----:|:----------------------------| + * | nothing | 0 | over-scoped / wrong tenant | + * | the correct subset for ORG_A | 2 | row-level isolation — the | + * | | | direction the ruling REFUSED| + * | the union ORG_A + ORG_B | 5 | a widened `tenantIds` scope | + * | everything | 12 | the #16589 defect | + * + * (2, 3, 7, 5, 10, 9 and 12 are pairwise distinct, so no two of those readings + * can be confused for each other.) + * + * The refusal is then pinned as **none of them**: the driver produces no answer + * at all. That makes the control failable in BOTH wrong directions — a revert to + * silent non-isolation answers 12, and an implementation of row-level isolation + * answers 2, and each one reds this file. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { InMemoryDriver } from './memory-driver.js'; +import { + MemoryMultiTenantUnsupportedError, + MULTI_TENANT_UNSUPPORTED_CODE, +} from './memory-tenancy-guard.js'; + +const ORIGINAL_MULTI_ORG = process.env.OS_MULTI_ORG_ENABLED; +const ORIGINAL_POSTURE = process.env.OS_TENANCY_POSTURE; + +/** Omits `tenancy` entirely — the case the engine scopes and seam 2 cannot see. */ +const SCOPED_OBJECT = 'ats_employer'; +/** Declares `tenancy.enabled: false` — ADR-0066 opt-out, never scoped. */ +const GLOBAL_OBJECT = 'ats_job'; + +const ORG_A = 'org_ats_alpha'; +const ORG_B = 'org_ats_beta'; +const ORG_C = 'org_ats_gamma'; + +/** Distinct counts, so every possible reading is a distinct number — see header. */ +const SEED: ReadonlyArray = [ + [ORG_A, 2], + [ORG_B, 3], + [ORG_C, 7], +]; +const TOTAL = 12; +const ORG_A_SUBSET = 2; +const ORG_A_B_UNION = 5; + +async function seedDriver() { + const driver = new InMemoryDriver({ persistence: false }); + await driver.connect(); + + // Both objects sync clean: neither declares `tenancy.enabled: true`, so seam 2 + // has nothing to say about either of them. That is the premise of this card. + await driver.syncSchema(SCOPED_OBJECT, { + name: SCOPED_OBJECT, + fields: { name: { type: 'text' }, organization_id: { type: 'text' } }, + }); + await driver.syncSchema(GLOBAL_OBJECT, { + name: GLOBAL_OBJECT, + fields: { title: { type: 'text' } }, + tenancy: { enabled: false }, + }); + + for (const [org, count] of SEED) { + for (let i = 0; i < count; i++) { + // Written WITHOUT a scope, exactly as a seeder does — the rows carry the + // organization they belong to, which is what makes a cross-org read + // observable at all. + await driver.create(SCOPED_OBJECT, { name: `${org}-employer-${i}`, organization_id: org }); + } + } + for (let i = 0; i < 4; i++) { + await driver.create(GLOBAL_OBJECT, { title: `job-${i}` }); + } + return driver; +} + +describe('per-call tenant-scope refusal (#16589)', () => { + beforeEach(() => { + delete process.env.OS_MULTI_ORG_ENABLED; + delete process.env.OS_TENANCY_POSTURE; + }); + + afterEach(() => { + if (ORIGINAL_MULTI_ORG === undefined) delete process.env.OS_MULTI_ORG_ENABLED; + else process.env.OS_MULTI_ORG_ENABLED = ORIGINAL_MULTI_ORG; + if (ORIGINAL_POSTURE === undefined) delete process.env.OS_TENANCY_POSTURE; + else process.env.OS_TENANCY_POSTURE = ORIGINAL_POSTURE; + }); + + describe('the fixture itself is discriminating', () => { + it('seeds three organizations whose every reading is a different number', async () => { + const driver = await seedDriver(); + // The premise of the whole card: this driver holds many organizations' + // rows in one table under a `single` posture. If this ever stops being + // true the readings below stop meaning anything, so it is asserted first. + const all = await driver.find(SCOPED_OBJECT, {}); + expect(all).toHaveLength(TOTAL); + + const perOrg = new Map(); + for (const row of all) { + const org = String(row.organization_id); + perOrg.set(org, (perOrg.get(org) ?? 0) + 1); + } + expect(perOrg.get(ORG_A)).toBe(ORG_A_SUBSET); + expect(perOrg.get(ORG_B)).toBe(3); + expect(perOrg.get(ORG_C)).toBe(7); + + // No two readings collide, which is what the closed round got wrong. + const readings = [0, ORG_A_SUBSET, ORG_A_B_UNION, TOTAL]; + expect(new Set(readings).size).toBe(readings.length); + }); + }); + + describe('BEFORE — the defect is still reproducible on the unscoped path', () => { + it('an UNSCOPED read returns every organization, which is what a scoped read used to answer', async () => { + const driver = await seedDriver(); + // This is the #16589 answer, preserved deliberately: the driver has no + // isolation, so with no scope handed to it, it returns the whole table + // including the other two organizations' rows. Before this card, a SCOPED + // read returned exactly this — identical rows, identical count — because + // the scope was silently discarded. That is the defect, and it is what the + // refusal below now stands in front of. + const rows = await driver.find(SCOPED_OBJECT, {}); + expect(rows).toHaveLength(TOTAL); + const orgs = new Set(rows.map((r) => String(r.organization_id))); + expect(orgs).toEqual(new Set([ORG_A, ORG_B, ORG_C])); + }); + }); + + describe('AFTER — the same read, scoped, refuses instead of answering', () => { + it('refuses a `tenantId`-scoped find, and answers with NO row count at all', async () => { + const driver = await seedDriver(); + let thrown: unknown = null; + let answered: unknown[] | null = null; + try { + answered = await driver.find(SCOPED_OBJECT, {}, { tenantId: ORG_A }); + } catch (err) { + thrown = err; + } + + // The three-way discrimination, stated as the readings it is NOT. + expect(answered).toBeNull(); + expect(thrown).toBeInstanceOf(MemoryMultiTenantUnsupportedError); + expect((thrown as { code?: string }).code).toBe(MULTI_TENANT_UNSUPPORTED_CODE); + + // ⛔ Answering 12 would be the defect (scope discarded). + // ⛔ Answering 2 would be row-level isolation — the direction the + // maintainer REFUSED on this card; implementing it reds this line. + // ⛔ Answering 0 would be an over-scope that silently hides rows. + // The only acceptable outcome is that there is no answer. + expect(answered).not.toHaveLength(TOTAL); + expect(answered).not.toHaveLength(ORG_A_SUBSET); + expect(answered).not.toHaveLength(0); + }); + + it('names the operation, the object and the scope it was handed', async () => { + const driver = await seedDriver(); + try { + await driver.find(SCOPED_OBJECT, {}, { tenantId: ORG_A }); + expect.unreachable('expected the driver to refuse'); + } catch (err) { + const message = (err as Error).message; + expect(message).toContain('find()'); + expect(message).toContain(SCOPED_OBJECT); + expect(message).toContain(ORG_A); + // The remedy must name the isolating driver and the ADR-0066 opt-out, + // and must reach the tracking card — a refusal that does not say what to + // do next is the "loud" half without the "locatable" half. + expect(message).toContain('@objectstack/driver-sql'); + expect(message).toContain('tenancy: { enabled: false }'); + expect(message).toContain('16589'); + } + }); + + it('refuses a `tenantIds` union scope too (ADR-0105 D2 group posture)', async () => { + const driver = await seedDriver(); + let answered: unknown[] | null = null; + let thrown: unknown = null; + try { + answered = await driver.find(SCOPED_OBJECT, {}, { tenantId: ORG_A, tenantIds: [ORG_A, ORG_B] }); + } catch (err) { + thrown = err; + } + expect(thrown).toBeInstanceOf(MemoryMultiTenantUnsupportedError); + // ⛔ 5 would be the widened union — the reading the closed round could not + // distinguish from "no scope ran" when only two orgs were seeded. + expect(answered).toBeNull(); + expect(answered).not.toHaveLength(ORG_A_B_UNION); + expect((thrown as Error).message).toContain('tenantIds'); + expect((thrown as Error).message).toContain(ORG_B); + }); + + it('refuses on every door that accepts a DriverOptions, not just find', async () => { + const driver = await seedDriver(); + const scope = { tenantId: ORG_A }; + const doors: Array<[string, () => Promise]> = [ + ['find', () => driver.find(SCOPED_OBJECT, {}, scope)], + ['findOne', () => driver.findOne(SCOPED_OBJECT, {}, scope)], + ['count', () => driver.count(SCOPED_OBJECT, {}, scope)], + ['aggregate', () => driver.aggregate(SCOPED_OBJECT, [], scope)], + ['create', () => driver.create(SCOPED_OBJECT, { name: 'x' }, scope)], + ['update', () => driver.update(SCOPED_OBJECT, 'anything', { name: 'x' }, scope)], + ['upsert', () => driver.upsert(SCOPED_OBJECT, { name: 'x' }, undefined, scope)], + ['delete', () => driver.delete(SCOPED_OBJECT, 'anything', scope)], + ['bulkCreate', () => driver.bulkCreate(SCOPED_OBJECT, [{ name: 'x' }], scope)], + ['updateMany', () => driver.updateMany(SCOPED_OBJECT, {}, { name: 'x' }, scope)], + ['deleteMany', () => driver.deleteMany(SCOPED_OBJECT, {}, scope)], + ['bulkUpdate', () => driver.bulkUpdate(SCOPED_OBJECT, [{ id: 'anything', data: {} }], scope)], + ]; + + for (const [name, call] of doors) { + let err: unknown = null; + try { + await call(); + } catch (e) { + err = e; + } + expect(err, `${name}() must refuse a scoped call`).toBeInstanceOf( + MemoryMultiTenantUnsupportedError, + ); + expect((err as Error).message, `${name}() must name itself`).toContain(`${name}()`); + } + }); + + it('a refused write leaves the store byte-for-byte as it found it', async () => { + const driver = await seedDriver(); + const before = await driver.find(SCOPED_OBJECT, {}); + + // The F2 defect measured on the closed round: a scoped `upsert` by a + // foreign id must NOT fall through to the `create` arm and land a second + // row under one primary id. Seam 3 sits at the top of the door, so the + // create arm is unreachable — this pins that. + await expect( + driver.upsert(SCOPED_OBJECT, { id: 'not-a-real-id', name: 'ghost' }, undefined, { + tenantId: ORG_A, + }), + ).rejects.toBeInstanceOf(MemoryMultiTenantUnsupportedError); + + const after = await driver.find(SCOPED_OBJECT, {}); + expect(after).toHaveLength(TOTAL); + expect(after).toEqual(before); + expect(after.filter((r) => r.id === 'not-a-real-id')).toHaveLength(0); + }); + }); + + describe('BOTH — the paths that must keep working, unchanged', () => { + it('an unscoped read on the same object is served exactly as before', async () => { + const driver = await seedDriver(); + const rows = await driver.find(SCOPED_OBJECT, {}); + expect(rows).toHaveLength(TOTAL); + const one = await driver.findOne(SCOPED_OBJECT, { where: { organization_id: ORG_B } }); + expect(one).not.toBeNull(); + expect(await driver.count(SCOPED_OBJECT, {})).toBe(TOTAL); + }); + + it('an object declaring `tenancy.enabled: false` is served, scope or no scope', async () => { + const driver = await seedDriver(); + // ADR-0066: the engine never sends a `tenantId` for an opted-out object, + // so the driver never sees one and nothing is refused. Asserted through + // the driver's own door rather than the engine's, because it is the + // driver's behaviour on the resulting options that this card changes. + expect(await driver.find(GLOBAL_OBJECT, {})).toHaveLength(4); + expect(await driver.count(GLOBAL_OBJECT, {})).toBe(4); + await expect(driver.create(GLOBAL_OBJECT, { title: 'job-4' })).resolves.toBeTruthy(); + expect(await driver.find(GLOBAL_OBJECT, {})).toHaveLength(5); + }); + + it('options that carry no tenant scope pass straight through', async () => { + const driver = await seedDriver(); + // Everything a caller can legally put in DriverOptions that is NOT a + // tenant scope must remain invisible to seam 3 — an over-eager guard here + // would break the ordinary dev path, which is the risk this card carries. + expect(await driver.find(SCOPED_OBJECT, {}, {})).toHaveLength(TOTAL); + expect(await driver.find(SCOPED_OBJECT, {}, { timezone: 'Asia/Shanghai' })).toHaveLength(TOTAL); + expect(await driver.find(SCOPED_OBJECT, {}, { skipCache: true })).toHaveLength(TOTAL); + expect(await driver.find(SCOPED_OBJECT, {}, { bypassTenantAudit: true })).toHaveLength(TOTAL); + expect(await driver.find(SCOPED_OBJECT, {}, { tenantId: undefined })).toHaveLength(TOTAL); + // `DriverOptionsSchema`: an absent or EMPTY `tenantIds` means "fall back to + // `tenantId` equality", so `[]` is not a scope of its own. + expect(await driver.find(SCOPED_OBJECT, {}, { tenantIds: [] })).toHaveLength(TOTAL); + }); + + it('the DDL doors still sync and drop with no options, as the engine calls them', async () => { + const driver = await seedDriver(); + // Every engine call site spells `syncSchema(tableName, obj)` and + // `dropTable(tableName)` — no options at all. Seam 3 covers these doors + // for uniformity, and this pins that it stays a no-op on the real shape. + await expect(driver.syncSchema('ats_interview', { name: 'ats_interview' })).resolves.not.toThrow(); + await expect(driver.dropTable('ats_interview')).resolves.not.toThrow(); + }); + }); +}); From 1c7ea0404e83ec05eb85264d137f4554b1d3f659 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 00:42:17 +0000 Subject: [PATCH 2/2] test(driver-memory): three-way acceptance fixture + changeset for the tenant-scope refusal Seeds three organizations with counts 2/3/7 so that "returns nothing" (0), "the correct subset" (2), "a widened union" (5) and "everything" (12) are four distinct numbers -- the closed round of this card seeded only two orgs, where a widened union covered the whole table and "the scope widened" was indistinguish- able from "no scope ran". The refusal is pinned as none of those readings, which makes the control failable in BOTH wrong directions: a revert to silent non-isolation answers 12, and an implementation of row-level isolation answers 2, and each reds the file. Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg Co-authored-by: Claude --- .../memory-driver-tenant-scope-refusal.md | 21 +++++++++++++ packages/drivers/driver-memory/src/index.ts | 4 +++ .../src/memory-tenant-scope-refusal.test.ts | 30 +++++++++++-------- 3 files changed, 43 insertions(+), 12 deletions(-) create mode 100644 .changeset/memory-driver-tenant-scope-refusal.md diff --git a/.changeset/memory-driver-tenant-scope-refusal.md b/.changeset/memory-driver-tenant-scope-refusal.md new file mode 100644 index 0000000000..f3374541d0 --- /dev/null +++ b/.changeset/memory-driver-tenant-scope-refusal.md @@ -0,0 +1,21 @@ +--- +"@objectstack/driver-memory": minor +--- + +fix(driver-memory): refuse a call the engine tenant-scoped, instead of silently answering with every organization's rows (#16589) + +**BREAKING** for a `driver-memory` deployment that holds more than one organization's rows: an operation the engine tenant-scoped now refuses loudly instead of answering. Shipped as `minor` under the launch-window convention, the same grading the driver's `update()`/`upsert()` type-surface narrowing used. + +Two predicates decided "is this object tenant-scoped", and they disagreed on the default case. The engine scopes an object **unless** it opts out (`buildDriverOptions`: `execCtx?.tenantId !== undefined && !isTenancyDisabled(objectSchema) && !isFederated`), while this driver's boot guard refused only an explicit opt-**in** (`declaresTenantScope`: `tenancy.enabled === true`). An object that **omits the `tenancy` block entirely** — the common case — therefore fell between them: the engine scoped it, the guard never saw it, the deployment posture really was `single` so the posture check passed, and the driver then discarded the scope and returned every organization's rows. A SQL driver refuses the same read. + +This driver still implements **no row-level tenant isolation**, and deliberately does not gain any: it declines to answer rather than answering correctly. `assertCallNotTenantScoped` is a third seam beside the two boot seams, and it judges the scope the engine actually handed over (`DriverOptions.tenantId` / `tenantIds`) rather than re-deriving the engine's predicate from object metadata — a driver that re-derived it would drift from the engine the first time that reasoning changed, and drift here is silent exposure. It runs first in every driver door that accepts a `DriverOptions`, so a refusal leaves the store exactly as it found it. + +**⚠️ Every isolation measurement previously taken on the memory driver is void and must be re-taken.** A suite asserting "tenant A cannot see tenant B's rows" passed here trivially — not because isolation worked, but because both tenants' rows came back to every caller and the assertion was written against a single tenant's fixture. An app that proved out its isolation model on this driver measured nothing. + +What is unaffected, and why: an object declaring `tenancy: { enabled: false }` is never scoped by the engine (ADR-0066), so the driver never sees a scope for it and serves it unchanged; a caller with no organization context is never scoped either, which is the ordinary dev, example-app and single-organization path. Only a call that actually arrives carrying a tenant scope is refused. A deployment that needs organization-scoped reads in development uses `@objectstack/driver-sql`, whose `:memory:` connection is the closest in-process replacement; a deployment whose data genuinely is platform-global can say so with the ADR-0066 posture, which stops the engine scoping it at all. + +The refusal reuses the existing `MemoryMultiTenantUnsupportedError` and its `MEMORY_MULTI_TENANT_UNSUPPORTED` code rather than introducing a second error family: the cause is identical, so a host that already recognises the boot refusal recognises this one with no new code and no second code to learn. + +Also corrects `declaresTenantScope`'s docstring, which closed on a false sentence — "every object in a single-tenant deployment omits the block". A `single` posture constrains the **wall**, not the number of organizations: a `single`-posture run was measured holding 13 `sys_organization` rows, with each row carrying whichever `organization_id` it was written with. The sentence is recorded as superseded rather than deleted, because it is what justified the predicate being an opt-in test. + + diff --git a/packages/drivers/driver-memory/src/index.ts b/packages/drivers/driver-memory/src/index.ts index 644bae66da..86cee78a7c 100644 --- a/packages/drivers/driver-memory/src/index.ts +++ b/packages/drivers/driver-memory/src/index.ts @@ -18,6 +18,10 @@ export { MULTI_TENANT_UNSUPPORTED_CODE, assertSingleTenantPosture, assertObjectsNotTenantScoped, + // [#16589] Seam 3 — the per-call refusal. Exported on the same reasoning as + // the two boot seams above: a consumer asserting this driver's behaviour under + // a tenant scope needs the refusal's identity, not its message text. + assertCallNotTenantScoped, declaresTenantScope, } from './memory-tenancy-guard.js'; export type { TenancyAwareSchema } from './memory-tenancy-guard.js'; diff --git a/packages/drivers/driver-memory/src/memory-tenant-scope-refusal.test.ts b/packages/drivers/driver-memory/src/memory-tenant-scope-refusal.test.ts index 68e90fe34b..6197c568e2 100644 --- a/packages/drivers/driver-memory/src/memory-tenant-scope-refusal.test.ts +++ b/packages/drivers/driver-memory/src/memory-tenant-scope-refusal.test.ts @@ -160,19 +160,23 @@ describe('per-call tenant-scope refusal (#16589)', () => { thrown = err; } - // The three-way discrimination, stated as the readings it is NOT. - expect(answered).toBeNull(); expect(thrown).toBeInstanceOf(MemoryMultiTenantUnsupportedError); expect((thrown as { code?: string }).code).toBe(MULTI_TENANT_UNSUPPORTED_CODE); - // ⛔ Answering 12 would be the defect (scope discarded). - // ⛔ Answering 2 would be row-level isolation — the direction the - // maintainer REFUSED on this card; implementing it reds this line. - // ⛔ Answering 0 would be an over-scope that silently hides rows. - // The only acceptable outcome is that there is no answer. - expect(answered).not.toHaveLength(TOTAL); - expect(answered).not.toHaveLength(ORG_A_SUBSET); - expect(answered).not.toHaveLength(0); + // The discrimination, stated as the readings this call is NOT. Taken as a + // nullable COUNT rather than with `toHaveLength`, which refuses a null + // target even under `.not` and would pass this block for the wrong reason. + const rowsAnswered = Array.isArray(answered) ? answered.length : null; + + // ⛔ 12 would be the defect (scope discarded, every organization returned). + // ⛔ 2 would be row-level isolation — the direction the maintainer REFUSED + // on this card; implementing it reds this line. + // ⛔ 0 would be an over-scope that silently hides rows. + // The only acceptable outcome is that there is no answer at all. + expect(rowsAnswered).toBeNull(); + expect(rowsAnswered).not.toBe(TOTAL); + expect(rowsAnswered).not.toBe(ORG_A_SUBSET); + expect(rowsAnswered).not.toBe(0); }); it('names the operation, the object and the scope it was handed', async () => { @@ -206,8 +210,10 @@ describe('per-call tenant-scope refusal (#16589)', () => { expect(thrown).toBeInstanceOf(MemoryMultiTenantUnsupportedError); // ⛔ 5 would be the widened union — the reading the closed round could not // distinguish from "no scope ran" when only two orgs were seeded. - expect(answered).toBeNull(); - expect(answered).not.toHaveLength(ORG_A_B_UNION); + const rowsAnswered = Array.isArray(answered) ? answered.length : null; + expect(rowsAnswered).toBeNull(); + expect(rowsAnswered).not.toBe(ORG_A_B_UNION); + expect(rowsAnswered).not.toBe(TOTAL); expect((thrown as Error).message).toContain('tenantIds'); expect((thrown as Error).message).toContain(ORG_B); });