From 584b202988f9c10e25df14ab81ec3c75fee60c88 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 11:31:16 +0000 Subject: [PATCH 1/6] feat(scripts): check-tenant-chokepoint refuses an IS NULL arm on a tenant column (ADR-0131 D8) The gate's header and failure text still taught the retired NULL-org platform-row arm as the rule. Restate both, and implement D8's refusal: whereNull/orWhereNull/andWhereNull, a null comparand, the object form and a raw IS NULL fragment on a tenant column reference anywhere in the three SqlDriver-family files is refused. A recognised-predicate floor keeps the rule from passing vacuously. No exemption is needed on main. Claude-Session: https://claude.ai/code/session_017ae4eMXWeERGrG6heTbwBX Co-authored-by: Claude --- scripts/check-tenant-chokepoint.mjs | 475 ++++++++++++++++++++++++++-- 1 file changed, 451 insertions(+), 24 deletions(-) diff --git a/scripts/check-tenant-chokepoint.mjs b/scripts/check-tenant-chokepoint.mjs index af2a6f8c6e9..355e2cf9e2f 100644 --- a/scripts/check-tenant-chokepoint.mjs +++ b/scripts/check-tenant-chokepoint.mjs @@ -8,17 +8,27 @@ * * `SqlDriver.applyTenantScope()` is the single chokepoint for read-side tenant * isolation in the SQL driver: it owns the `tenantId` early-out, the - * "object has no tenant field" early-out, the NULL-org platform-row rule - * (#2734) and the ADR-0105 D2 union posture (#3623). A read door that builds - * its own query and never arrives there is not "inconsistent" — it returns - * other tenants' data to a caller that asked to be scoped. - * - * So: **a method that builds through `this.getBuilder(object, options)` and - * lets that builder escape as a READ must call `this.applyTenantScope()` on - * it.** Write builders are exempt and the exemption is structural, not a - * name list — insert-side tenancy is a different mechanism - * (`injectTenantOnInsert` stamps the column), so a builder whose only escape - * is `.insert(...)` is correct without this call. + * "object has no tenant field" early-out, the one tenant predicate + * (`tenant_column = :tenant`, ADR-0131 D8) and the ADR-0105 D2 union posture + * (`tenant_column IN (:tenants)`). A read door that builds its own query and + * never arrives there is not "inconsistent" — it returns other tenants' data + * to a caller that asked to be scoped. + * + * So, two rules: + * + * 1. **A method that builds through `this.getBuilder(object, options)` and + * lets that builder escape as a READ must call `this.applyTenantScope()` + * on it.** Write builders are exempt and the exemption is structural, not + * a name list — insert-side tenancy is a different mechanism + * (`injectTenantOnInsert` stamps the column), so a builder whose only + * escape is `.insert(...)` is correct without this call. + * 2. **No `IS NULL` arm on a tenant column (ADR-0131 D8).** NULL is not an + * owner (D1): an organization-less row is no tenant's, so no tenant + * predicate may reach it. The v17 predicate carried an + * `OR tenant_column IS NULL` arm under which such a row stood for + * platform state; ADR-0131 retired that reading, and this rule keeps the + * arm from coming back — in `applyTenantScope` or anywhere else in the + * three driver files. See "The D8 null-arm rule" below. * * ## Why the gate exists, and what it already caught * @@ -103,9 +113,44 @@ * there; only the fixture proves it works.** Neither is redundant, and a * change that keeps this green must still keep that file green. * + * ## The D8 null-arm rule + * + * ADR-0131 D8: "`check-tenant-chokepoint` refuses an `IS NULL` arm on a tenant + * column." A TENANT COLUMN REFERENCE, inside any class member of the scanned + * files, is one of: + * + * - a local bound to `this.resolveTenantField(…)`, `this.computeTenantField(…)` + * or `this.computeAndRecordTenantField(…)` (how `applyTenantScope` spells + * it: `const field = this.resolveTenantField(object)`); + * - anything NAMED `tenantField` — a local, a parameter, or a member such as + * `reservation.tenantField` (the sequence-counter paths); + * - the literal `'organization_id'`, the canonical tenant column. + * + * An ARM on it is refused in each spelling Knex compiles to `IS NULL`: + * `whereNull` / `orWhereNull` / `andWhereNull`; a `where` / `orWhere` / + * `andWhere` with a `null` comparand (`where(field, null)`, + * `where(field, 'is', null)`); an object-form `where({ [field]: null })`; and + * a raw fragment (`whereRaw`, `orWhereRaw`, `andWhereRaw`, `havingRaw`, `raw`) + * whose own literal text says `IS NULL` and that names the column — as a + * template substitution, as a binding in its bindings array, or literally. + * `IS NOT NULL` is not an arm and is never refused. + * + * Exemptions: none (`NULL_ARM_EXEMPT` is empty), and the rule needs none on + * `main`. One NULL disjunct survives in the platform BY DESIGN and is outside + * this rule on purpose: the `sys_audit_log.tenant_id` attribution partition + * read by `LifecycleService`. That column is an ADR-0131 D7 attribution + * field, not a D8 tenant column, and it lives outside the driver files this + * gate scans. + * + * What it does NOT see, named: a raw SQL string assembled into a variable + * first and passed by name, and a tenant column reached under a name not + * listed above. The gate refuses the spellings the driver actually uses; + * `sql-driver-tenant-scope-read-doors.test.ts` proves the compiled predicate. + * * node scripts/check-tenant-chokepoint.mjs [--self-test] * - * Exit codes: 0 clean · 1 an unscoped read door · 2 the scan could not run + * Exit codes: 0 clean · 1 an unscoped read door, or an `IS NULL` arm on a + * tenant column · 2 the scan could not run * (no class found, a builder it cannot classify, or a DISCOVERED floor missed). */ @@ -166,6 +211,14 @@ const DECLARED_WATCH_HINTS = [ */ const DISCOVERED_FLOOR = 15; +/** + * The same floor for the D8 null-arm rule: tenant-column predicates it must + * RECOGNISE (`where` / `whereIn` on a tenant column reference — 4 on `main`, + * `applyTenantScope`'s two among them). Below today's count, above zero: a + * rule that stops recognising the tenant column refuses nothing. + */ +const TENANT_PREDICATE_FLOOR = 3; + /** * `'::::'` -> reason a reader can check. * @@ -241,6 +294,201 @@ function isScoped(body, name) { return found; } +// ── The D8 null-arm rule ──────────────────────────────────────────────────── + +/** The calls that answer "which column is this object's tenant column". */ +const TENANT_FIELD_RESOLVERS = new Set(['resolveTenantField', 'computeTenantField', 'computeAndRecordTenantField']); +/** A name that IS a tenant column wherever it appears — local, parameter or member. */ +const TENANT_FIELD_NAME = 'tenantField'; +/** The canonical tenant column, written as a literal. */ +const TENANT_COLUMN_LITERAL = 'organization_id'; + +const NULL_CALLS = new Set(['whereNull', 'orWhereNull', 'andWhereNull']); +const WHERE_CALLS = new Set(['where', 'orWhere', 'andWhere']); +const RAW_CALLS = new Set(['whereRaw', 'orWhereRaw', 'andWhereRaw', 'havingRaw', 'raw']); +/** The calls a tenant predicate is written with — counted for the floor, never refused. */ +const TENANT_PREDICATE_CALLS = new Set(['where', 'whereIn']); +/** `IS NULL`, never `IS NOT NULL` — the latter is not an arm. */ +const IS_NULL_TEXT = /\bis\s+null\b/i; + +/** + * `'::'` -> reason a reader can check. Empty by design: the rule + * needs no exemption on `main`, and an entry is a decision against ADR-0131 + * D8 that a superseding ADR has to make first. + */ +const NULL_ARM_EXEMPT = Object.create(null); + +function isNullLiteral(node) { + return node && node.kind === ts.SyntaxKind.NullKeyword; +} + +/** Locals in `body` bound to a tenant-field resolver call. */ +function tenantBindings(body) { + const names = new Set(); + const callsResolver = (node) => { + let hit = false; + const walk = (n) => { + if (hit) return; + if ( + ts.isCallExpression(n) && + ts.isPropertyAccessExpression(n.expression) && + n.expression.expression.kind === ts.SyntaxKind.ThisKeyword && + TENANT_FIELD_RESOLVERS.has(n.expression.name.text) + ) { + hit = true; + return; + } + ts.forEachChild(n, walk); + }; + walk(node); + return hit; + }; + const walk = (node) => { + if (ts.isVariableDeclaration(node) && ts.isIdentifier(node.name) && node.initializer && callsResolver(node.initializer)) { + names.add(node.name.text); + } + ts.forEachChild(node, walk); + }; + walk(body); + return names; +} + +/** Is `node` a tenant column reference, as the header defines one? */ +function isTenantRef(node, bindings) { + if (!node) return false; + if (ts.isParenthesizedExpression(node) || ts.isNonNullExpression(node) || ts.isAsExpression(node)) { + return isTenantRef(node.expression, bindings); + } + if (ts.isIdentifier(node)) return node.text === TENANT_FIELD_NAME || bindings.has(node.text); + if (ts.isPropertyAccessExpression(node)) return node.name.text === TENANT_FIELD_NAME; + if (ts.isStringLiteralLike(node)) return node.text === TENANT_COLUMN_LITERAL; + // `String(field)` — the driver threads values through `String()` elsewhere. + if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) && node.expression.text === 'String') { + return isTenantRef(node.arguments[0], bindings); + } + return false; +} + +/** The literal text of a raw fragment, and the expressions substituted into it. */ +function rawParts(node) { + if (!node) return null; + if (ts.isStringLiteralLike(node)) return { text: node.text, spans: [] }; + if (ts.isTemplateExpression(node)) { + return { + text: node.head.text + node.templateSpans.map((s) => ` ${s.literal.text}`).join(''), + spans: node.templateSpans.map((s) => s.expression), + }; + } + if (ts.isBinaryExpression(node) && node.operatorToken.kind === ts.SyntaxKind.PlusToken) { + const l = rawParts(node.left) ?? { text: '', spans: [node.left] }; + const r = rawParts(node.right) ?? { text: '', spans: [node.right] }; + return { text: `${l.text} ${r.text}`, spans: [...l.spans, ...r.spans] }; + } + return null; +} + +/** The spelling of a refused arm, or null when `call` is not one. */ +function nullArmOf(call, bindings) { + if (!ts.isPropertyAccessExpression(call.expression)) return null; + const verb = call.expression.name.text; + const args = call.arguments; + if (NULL_CALLS.has(verb) && isTenantRef(args[0], bindings)) return `${verb}()`; + if (WHERE_CALLS.has(verb)) { + if (args.length >= 2 && isTenantRef(args[0], bindings) && isNullLiteral(args[args.length - 1])) { + return `${verb}(, …null)`; + } + const obj = args[0]; + if (args.length === 1 && obj && ts.isObjectLiteralExpression(obj)) { + for (const p of obj.properties) { + if (!ts.isPropertyAssignment(p) || !isNullLiteral(p.initializer)) continue; + const key = ts.isComputedPropertyName(p.name) + ? isTenantRef(p.name.expression, bindings) + : (ts.isIdentifier(p.name) || ts.isStringLiteralLike(p.name)) && p.name.text === TENANT_COLUMN_LITERAL; + if (key) return `${verb}({ : null })`; + } + } + } + if (RAW_CALLS.has(verb)) { + const parts = rawParts(args[0]); + if (parts && IS_NULL_TEXT.test(parts.text)) { + const bound = args[1] && ts.isArrayLiteralExpression(args[1]) && args[1].elements.some((e) => isTenantRef(e, bindings)); + const named = + parts.spans.some((e) => isTenantRef(e, bindings)) || + new RegExp(`\\b${TENANT_COLUMN_LITERAL}\\b`).test(parts.text); + if (bound || named) return `${verb}('… IS NULL …')`; + } + } + return null; +} + +/** The name an outermost container (method, accessor, function, property) is reported under. */ +function containerName(node) { + if (ts.isConstructorDeclaration(node)) return 'constructor'; + if ( + (ts.isMethodDeclaration(node) || ts.isGetAccessorDeclaration(node) || ts.isSetAccessorDeclaration(node) || + ts.isFunctionDeclaration(node) || (ts.isPropertyDeclaration(node) && node.initializer)) && + node.name + ) { + return ts.isIdentifier(node.name) || ts.isPrivateIdentifier(node.name) ? node.name.text : node.name.getText(); + } + if (ts.isVariableDeclaration(node) && ts.isIdentifier(node.name) && node.initializer && + (ts.isArrowFunction(node.initializer) || ts.isFunctionExpression(node.initializer))) { + return node.name.text; + } + return null; +} + +/** + * Every refused arm in one source file — every class member AND every + * module-level function, so a helper outside the class is no hiding place. + * Tenant bindings are scoped to the outermost container they are declared in: + * a `field` that is the tenant column in `applyTenantScope` is not the tenant + * column in the filter compiler that also names a local `field`. + */ +export function nullArmsOf(fileName, text) { + const sf = parseSourceFile(fileName, text); + const arms = []; + // Tenant-column predicates the rule SAW (`where` / `whereIn` on a tenant + // column reference). The floor in main() reads this: a matcher that stops + // recognising the tenant column refuses nothing and still reports clean. + let predicates = 0; + const walk = (node, ctx) => { + let here = ctx; + if (!ctx) { + const name = containerName(node); + if (name) here = { name, bindings: tenantBindings(node) }; + } + if (ts.isCallExpression(node)) { + const bindings = here?.bindings ?? new Set(); + if ( + ts.isPropertyAccessExpression(node.expression) && + TENANT_PREDICATE_CALLS.has(node.expression.name.text) && + isTenantRef(node.arguments[0], bindings) + ) { + predicates += 1; + } + const shape = nullArmOf(node, bindings); + if (shape) { + arms.push({ + method: here?.name ?? '(module scope)', + line: sf.getLineAndCharacterOfPosition(node.getStart(sf)).line + 1, + shape, + }); + } + } + ts.forEachChild(node, (child) => walk(child, here)); + }; + walk(sf, null); + return { arms, predicates }; +} + +/** The D8 verdict as a pure function of the arms found. Self-testable. */ +export function nullArmViolationsOf(file, arms, exempt = NULL_ARM_EXEMPT) { + return arms + .filter((a) => !exempt[`${file}::${a.method}`]) + .map((a) => ({ fatal: false, nullArm: true, text: `${file}:${a.line} ${a.method}() — ${a.shape}` })); +} + /** * Classify every `getBuilder` binding in one source file. * @@ -381,11 +629,12 @@ const SELF_TEST_VERDICT = 'check-tenant-chokepoint self-test reached its verdict // to find what stopped registering. const SELF_TEST_BATTERIES = Object.freeze({ 'check-tenant-chokepoint self-test': 17, + 'D8 null-arm rule': 22, }); // DELETING an entry silences that battery's floor exactly as effectively as // zeroing it, so the roster's own size is pinned too. -const SELF_TEST_BATTERY_FLOOR = 1; +const SELF_TEST_BATTERY_FLOOR = 2; // The key an assertion is filed under when no battery is open. It is not a // declared battery, so it reds by the same set difference rather than silently @@ -556,7 +805,144 @@ function selfTest() { assert(builders.length === 0, 'the empty fixture must discover no builders'); } + // ── the D8 null-arm rule: every spelling reports, its counterpart stays silent + battery('D8 null-arm rule'); + const arms = (src) => nullArmsOf('t.ts', wrap(src)).arms; + const nullArmReports = [ + [ + 'the retired v17 arm itself, inside the chokepoint', + `applyTenantScope(builder, object, options) { + const field = this.resolveTenantField(object); + if (!field) return builder; + return builder.where((qb) => qb.where(field, String(options.tenantId)).orWhereNull(field)); + }`, + ], + [ + 'whereNull on a parameter named tenantField', + `nextValue(table, tenantField, tenantId) { return this.knex(table).whereNull(tenantField); }`, + ], + [ + 'andWhereNull on a member named tenantField', + `reserve(reservation) { return this.knex(reservation.table).andWhereNull(reservation.tenantField); }`, + ], + [ + 'the literal tenant column', + `scan(table) { return this.knex(table).orWhereNull('organization_id'); }`, + ], + [ + 'a null comparand — Knex compiles where(col, null) to IS NULL', + `scope(builder, object) { + const f = this.resolveTenantField(object); + return builder.orWhere(f, null); + }`, + ], + [ + "the three-argument form, where(col, 'is', null)", + `scope(builder, object) { + const f = this.computeTenantField(object); + return builder.orWhere(f, 'is', null); + }`, + ], + [ + 'the object form with a computed tenant key', + `scope(builder, object) { + const f = this.resolveTenantField(object); + return builder.orWhere({ [f]: null }); + }`, + ], + [ + 'a raw fragment substituting the tenant column', + 'scope(builder, object) { const f = this.resolveTenantField(object); return builder.orWhereRaw(`${f} is null`); }', + ], + [ + 'a raw fragment binding the tenant column', + `scope(builder, object) { + const f = this.resolveTenantField(object); + return builder.whereRaw('(?? = ? OR ?? IS NULL)', [f, 'org', f]); + }`, + ], + [ + 'a raw fragment naming the literal tenant column', + `census(table) { return this.knex.raw('select * from ?? where organization_id is null', [table]); }`, + ], + [ + 'a module-level helper outside the class is no hiding place', + `x() {} + } + function widen(qb, tenantField) { return qb.orWhereNull(tenantField); } + class Tail {`, + ], + ]; + for (const [name, src] of nullArmReports) { + assert(arms(src).length >= 1, `D8: expected an IS NULL arm report for: ${name}`); + } + + const nullArmSilent = [ + [ + 'the one predicate, as applyTenantScope spells it today', + `applyTenantScope(builder, object, options) { + const field = this.resolveTenantField(object); + if (!field) return builder; + return builder.where(field, String(options.tenantId)); + }`, + ], + [ + 'IS NOT NULL on the tenant column is not an arm', + `scope(builder, object) { + const field = this.resolveTenantField(object); + return builder.whereNotNull(field).whereRaw('?? is not null', [field]); + }`, + ], + [ + 'whereNull on an ordinary column (the filter compiler)', + `compile(qb, field) { return qb.whereNull(field).orWhere(field, ''); }`, + ], + [ + 'a tenant binding does not leak into a sibling method that reuses the name', + `scope(builder, object) { const field = this.resolveTenantField(object); return builder.where(field, 'o'); } + compile(qb, field) { return qb.orWhereNull(field); }`, + ], + [ + 'a raw IS NULL over a non-tenant expression', + 'negate(qb, expr, testSql) { return qb.whereRaw(`(${expr.sql} IS NULL OR ${testSql})`, expr.bindings); }', + ], + [ + 'a null comparand on an ordinary column', + `clear(qb) { return qb.where('deleted_at', null); }`, + ], + ]; + for (const [name, src] of nullArmSilent) { + const found = arms(src); + assert(found.length === 0, `D8: expected NO arm report for: ${name} (got ${found.map((a) => a.shape).join('; ')})`); + } + + // The predicate counter the floor reads must see the real predicate, and not + // count an ordinary column. + assert( + nullArmsOf('t.ts', wrap(nullArmSilent[0][1])).predicates === 1, + 'D8: the one predicate must be counted for the TENANT_PREDICATE_FLOOR', + ); + assert( + nullArmsOf('t.ts', wrap(nullArmSilent[5][1])).predicates === 0, + 'D8: a predicate on an ordinary column must not be counted', + ); + + // The exemption channel is keyed to one file::method, and empty on main. + { + const found = arms(nullArmReports[0][1]); + assert(nullArmViolationsOf('t.ts', found).length === 1, 'D8: an arm reports without an exemption'); + assert( + nullArmViolationsOf('t.ts', found, { 't.ts::applyTenantScope': 'because' }).length === 0, + 'D8: a NULL_ARM_EXEMPT entry silences exactly its own method', + ); + assert( + nullArmViolationsOf('t.ts', found, { 't.ts::other': 'because' }).length === 1, + 'D8: an entry for another method must NOT silence this one', + ); + } + // ── the dispatch-gates population declaration ──────────────────────────── + battery('check-tenant-chokepoint self-test'); const scannedPosix = SCAN_FILES.map((f) => f.split(sep).join('/')); assert( scannedPosix.every((f) => DECLARED_WATCH_HINTS.includes(f)), @@ -625,7 +1011,8 @@ function selfTest() { } console.log( `✓ check:tenant-chokepoint self-test: ${reports.length} reporting shape(s), ` + - `${silent.length} silent counterpart(s), fatal/exemption/floor channels proved in both directions.`, + `${silent.length} silent counterpart(s), fatal/exemption/floor channels proved in both directions; ` + + `D8 null-arm rule: ${nullArmReports.length} refused spelling(s), ${nullArmSilent.length} silent counterpart(s).`, ); return SELF_TEST_VERDICT; @@ -636,6 +1023,8 @@ function selfTest() { function main() { const problems = []; + const nullArms = []; + let tenantPredicates = 0; let discovered = 0; let scannedFiles = 0; @@ -650,9 +1039,14 @@ function main() { process.exit(2); } scannedFiles += 1; - const analysis = analyzeSource(rel, readFileSync(abs, 'utf8')); + const text = readFileSync(abs, 'utf8'); + const analysis = analyzeSource(rel, text); discovered += analysis.builders.length; - problems.push(...violationsOf(relative(ROOT, abs).replace(/\\/g, '/'), analysis)); + const posix = relative(ROOT, abs).replace(/\\/g, '/'); + problems.push(...violationsOf(posix, analysis)); + const d8 = nullArmsOf(rel, text); + tenantPredicates += d8.predicates; + nullArms.push(...nullArmViolationsOf(posix, d8.arms)); } if (discovered < DISCOVERED_FLOOR) { @@ -666,6 +1060,17 @@ function main() { process.exit(2); } + if (tenantPredicates < TENANT_PREDICATE_FLOOR) { + console.error( + `check:tenant-chokepoint: the D8 null-arm rule recognised only ${tenantPredicates} tenant-column ` + + `predicate(s) across ${scannedFiles} file(s), below the floor of ${TENANT_PREDICATE_FLOOR}.\n` + + 'A rule that no longer recognises the tenant column refuses nothing and reports clean. Either\n' + + 'the driver now names its tenant column some other way — teach isTenantRef() that spelling —\n' + + 'or the matcher is broken. Both are errors, not passes.', + ); + process.exit(2); + } + const fatal = problems.filter((p) => p.fatal); if (fatal.length > 0) { console.error(`✗ check:tenant-chokepoint: ${fatal.length} builder(s) this gate cannot classify\n`); @@ -683,28 +1088,50 @@ function main() { for (const p of problems) console.error(` • ${p.text}`); console.error(` \`applyTenantScope\` is the single chokepoint for read-side tenant isolation: it owns -the tenantId early-out, the no-tenant-field early-out, the NULL-org platform-row rule -(#2734) and the ADR-0105 D2 union posture (#3623). A read door that skips it returns -OTHER TENANTS' data to a caller that asked to be scoped — measured, not theoretical -(#6792: three doors, one of them documented with a runnable example). +the tenantId early-out, the no-tenant-field early-out, the one tenant predicate +(tenant_column = :tenant, ADR-0131 D8) and the ADR-0105 D2 union posture +(tenant_column IN (:tenants)). A read door that skips it returns OTHER TENANTS' data +to a caller that asked to be scoped — measured, not theoretical: three such doors +were found, one of them documented with a runnable example. Add the call beside the getBuilder() line, as every other door does: const builder = this.getBuilder(object, options); this.applyTenantScope(builder, object, options); -Do not re-derive the predicate locally — a bare equality silently drops NULL-org -platform rows (#2734) and collapses the group posture to active-org reach (#3623). +Do not re-derive the predicate locally — a local copy drifts from the one computed +predicate, and an equality written where the group posture is in force collapses +its union to active-org reach. If a door genuinely must build unscoped, add it to EXEMPT in scripts/check-tenant-chokepoint.mjs with a reason a reader can check. "The layer above catches it" is not one: this asserts the chokepoint's own contract.`); - process.exit(1); } + if (nullArms.length > 0) { + console.error(`✗ check:tenant-chokepoint: ${nullArms.length} IS NULL arm(s) on a tenant column\n`); + for (const p of nullArms) console.error(` • ${p.text}`); + console.error(` +ADR-0131 D8: no layer has a NULL arm. NULL is not an owner (D1) — an +organization-less row belongs to no tenant, so no tenant predicate may reach it. +The tenant predicate is \`tenant_column = :tenant\` (or \`IN (:tenants)\` under the +group posture), and nothing else: an \`OR tenant_column IS NULL\` arm hands every +tenant the rows no organization owns. + +Remove the arm. A row that should be readable by a tenant carries that tenant's +organization; deployment-level state lives on an object with no tenant column (D7). +Reading organization-less rows on purpose (a census, a repair) is not a tenant +predicate: do it outside the driver's tenant scope, or record the exemption in +NULL_ARM_EXEMPT in scripts/check-tenant-chokepoint.mjs — which reverses an ADR +decision and needs a superseding ADR first.`); + } + + if (problems.length > 0 || nullArms.length > 0) process.exit(1); + console.log( `✓ check:tenant-chokepoint: ${discovered} ${BUILDER_FACTORY}() binding(s) across ${scannedFiles} ` + - 'file(s); every read builder routes through applyTenantScope(), every unscoped one is an insert.', + 'file(s); every read builder routes through applyTenantScope(), every unscoped one is an insert; ' + + 'no IS NULL arm on a tenant column (ADR-0131 D8).', ); } From e21172fe3da91f3b5f912067f4df8ff58a6f8ccb Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 11:34:56 +0000 Subject: [PATCH 2/6] docs(runtime): retire present-tense prose that the driver still reads organization-less rows ADR-0131 D8 retired the driver's OR organization_id IS NULL arm, but the comments of fifteen runtime files (and one runtime warning) still described it as the live tenant predicate. Restate each: the tenant term is the equality (or IN under group) alone, an organization-less row on a tenant table is reached by no tenant-scoped read, and where code was written against the old arm the text now says so. Claude-Session: https://claude.ai/code/session_017ae4eMXWeERGrG6heTbwBX Co-authored-by: Claude --- .../src/tenancy/platform-object-tenancy.ts | 6 +-- .../src/delegated-admin-gate.ts | 6 +-- .../plugin-security/src/organization-scope.ts | 38 +++++++++------- ...backfill-sys-record-share-organizations.ts | 15 +++++-- .../plugin-webhooks/src/auto-enqueuer.ts | 5 ++- packages/runtime/src/domains/packages.ts | 5 ++- .../src/builtin/http-nodes.ts | 10 ++--- .../src/runtime-identity.ts | 10 ++--- .../service-messaging/src/http-outbox.ts | 15 ++++--- .../src/memory-http-outbox.ts | 10 +++-- .../service-messaging/src/sql-http-outbox.ts | 9 ++-- .../src/backfill-sys-file-organizations.ts | 13 +++--- .../service-storage/src/metadata-store.ts | 43 ++++++++++--------- packages/spec/src/api/events.zod.ts | 14 +++--- packages/spec/src/data/driver.zod.ts | 3 +- 15 files changed, 115 insertions(+), 87 deletions(-) diff --git a/packages/objectql/src/tenancy/platform-object-tenancy.ts b/packages/objectql/src/tenancy/platform-object-tenancy.ts index 4c7851e45bb..9a765253204 100644 --- a/packages/objectql/src/tenancy/platform-object-tenancy.ts +++ b/packages/objectql/src/tenancy/platform-object-tenancy.ts @@ -239,9 +239,9 @@ export const PLATFORM_OBJECT_TENANCY: Readonly` above it, and the conjunction is * the strict equality alone. * + * (ADR-0131 D8 has since retired the driver's NULL arm: both layers now + * compile the equality alone, so an unstamped grant is unreachable by every + * tenant-scoped read.) + * * ⇒ the day anything reads this table under a tenant context, every * organization-less grant becomes invisible — not refused, simply "this person * was never granted access". Forward-only stamping would split the table in @@ -591,9 +595,12 @@ function finish( * * The derived organization is threaded as `tenantId` on the write context too, * beside the elevation: the shape #8844 prescribes for a system write and the - * one the repaired writer uses. On this verb the driver's scope keeps the NULL - * row in reach (`organization_id = ? OR IS NULL`, #2734), so the write lands - * exactly on the row the plan named. + * one the repaired writer uses. ⚠️ That shape predates ADR-0131 D8: it relied + * on the driver's v17 scope `organization_id = ? OR IS NULL` keeping the NULL + * row in reach. D8 retired that arm, so under the SQL driver a write threaded + * with `tenantId` no longer reaches an organization-less row. No runtime path + * calls this pass; the 18.x upgrade ceremony's attribution fate (ADR-0131 + * D10) is what stamps these rows now. * * A row whose write throws is RECORDED and the sweep continues: a driver * rejecting one row must not cost the other N-1 their repair, and a half-done diff --git a/packages/plugins/plugin-webhooks/src/auto-enqueuer.ts b/packages/plugins/plugin-webhooks/src/auto-enqueuer.ts index b6ab043272f..3736af5589e 100644 --- a/packages/plugins/plugin-webhooks/src/auto-enqueuer.ts +++ b/packages/plugins/plugin-webhooks/src/auto-enqueuer.ts @@ -136,7 +136,8 @@ interface CachedSubscription { * (`EnqueueHttpInput.organizationId`), which is what makes the * cross-organization wall on `redeliver()` (#10740) actually exclude other * tenants' rows: a row enqueued without it lands `organization_id = NULL`, - * the driver's global-row arm, visible to every organization. There is no + * a row of no organization that no tenant-scoped read reaches (ADR-0131 + * D8 retired the driver's NULL arm). There is no * request context to read here — the enqueuer runs fire-and-forget off the * write path — so the subscription row is the one honest source. Absent * when the row itself carries no organization (a `single`-posture install): @@ -1175,7 +1176,7 @@ export class AutoEnqueuer { // organization — the one honest tenant in scope on this // fire-and-forget path (no request context exists here). // Absent for an org-less subscription; the row then lands - // NULL, the global-row shape. A deployment-wide declared + // NULL, a row of no organization. A deployment-wide declared // webhook takes the event's organization instead. organizationId: sub.deploymentWide ? organization.organizationId : sub.organizationId, // [#3946] Envelope keys are written LAST so the event payload diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index 767f03991e4..e2c08af6436 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -2690,8 +2690,9 @@ _context: HttpProtocolContext, // Read the just-published seed body. The protocol decides the scope, // not this call site: [ADR-0131 D6] every metadata read is // environment → code, so this read is env-wide by construction: - // `organization_id IS NULL`, the partition a workspace seed is stored - // in and the only one cold boot hydrates. + // `sys_metadata` carries no organization column (ADR-0131 D6/D7), so + // the environment's rows are the only partition there is — the one a + // workspace seed is stored in and cold boot hydrates. // // [commit 8744de9e9] This used to be a two-attempt org-then-env ladder, written // when resolving the wrong scope here is what silently produced "0 diff --git a/packages/services/service-automation/src/builtin/http-nodes.ts b/packages/services/service-automation/src/builtin/http-nodes.ts index 653a613f2e9..04793820d48 100644 --- a/packages/services/service-automation/src/builtin/http-nodes.ts +++ b/packages/services/service-automation/src/builtin/http-nodes.ts @@ -260,9 +260,9 @@ export function registerHttpNodes(engine: AutomationEngine, ctx: PluginContext): // organization, and a wrong value is worse than a null (a // null is visibly missing; a wrong one is silently // authoritative). Without it the sys_http_delivery row - // lands `organization_id = NULL` — the driver's global-row - // arm — visible to and replayable by EVERY organization - // through the redeliver() door (#10740). + // lands `organization_id = NULL` — a row of no + // organization, which no tenant-scoped read or redeliver() + // reaches (ADR-0131 D8 retired the driver's NULL arm). const organizationId = typeof context.tenantId === 'string' && context.tenantId !== '' ? context.tenantId @@ -277,8 +277,8 @@ export function registerHttpNodes(engine: AutomationEngine, ctx: PluginContext): // id; see check:doc-authoring.) ctx.logger.warn( `[http] node '${node.id}': no organization in scope for this durable callout — its ` + - `sys_http_delivery row will carry organization_id = NULL, which is a global row ` + - `every organization's redeliver door can reach on a walled deployment. ` + + `sys_http_delivery row will carry organization_id = NULL, a row of no organization ` + + `that no organization's redeliver door can reach on a walled deployment. ` + `On a multi-organization install the triggering context lost its tenant: give the ` + `flow's trigger an acting organization (AutomationContext.tenantId). On a ` + `single-organization install this is expected and can be ignored.`, diff --git a/packages/services/service-automation/src/runtime-identity.ts b/packages/services/service-automation/src/runtime-identity.ts index 4817a3334e1..31cac353dec 100644 --- a/packages/services/service-automation/src/runtime-identity.ts +++ b/packages/services/service-automation/src/runtime-identity.ts @@ -293,11 +293,11 @@ export function resolveRunDataContext(context: AutomationContext | undefined): R // sequences org-scoped. Without it, every row a user-triggered system // sweep created was born org-NULL — outside the org partition, where // unique indexes don't bite and org-scoped queries don't look. Reads / - // updates / deletes org-scope to `(org = trigger-org OR org IS NULL)` at - // the driver — the same posture the action-body seam ships for its - // `{ ...caller, isSystem: true }` envelope (the hotcrm#548-family fix): - // the tenant wall survives elevation for an org-triggered run, while - // org-NULL (pre-fix / global) rows stay reachable via the OR-NULL arm. + // updates / deletes org-scope to `org = trigger-org` at the driver + // (ADR-0131 D8 — no NULL arm) — the same posture the action-body seam + // ships for its `{ ...caller, isSystem: true }` envelope (the + // hotcrm#548-family fix): the tenant wall survives elevation for an + // org-triggered run. // A run that must genuinely cross orgs is a user-less one (a schedule // carries no org, nothing narrows) or sets explicit fields. ...(context.tenantId ? { tenantId: context.tenantId } : {}), diff --git a/packages/services/service-messaging/src/http-outbox.ts b/packages/services/service-messaging/src/http-outbox.ts index 32c0d01ac5e..56be2c3816a 100644 --- a/packages/services/service-messaging/src/http-outbox.ts +++ b/packages/services/service-messaging/src/http-outbox.ts @@ -109,7 +109,8 @@ export interface HttpDelivery { * [#13546] Organization this delivery row belongs to — the read-back of * the row's kernel-provisioned `organization_id` tenant column, which is * the column the cross-organization wall on {@link IHttpOutbox.redeliver} - * scopes by (#10740). Absent for a global row (`organization_id = NULL`). + * scopes by (#10740). Absent for a row of no organization + * (`organization_id = NULL`). * Mirrors `NotificationDeliveryRecord.organizationId` on the sibling * notification outbox. */ @@ -173,10 +174,11 @@ export interface EnqueueHttpInput { * [#13546] Organization this delivery belongs to. Lands on the row's * kernel-provisioned `organization_id` tenant column — the column the * cross-organization wall on {@link IHttpOutbox.redeliver} scopes by - * (#10740). The driver's tenant term is - * `(organization_id = :tenantId OR organization_id IS NULL)`, so a row - * enqueued WITHOUT one lands in the deliberate global-row arm: visible - * to — and replayable by — every organization on a walled deployment. + * (#10740). The driver's tenant term is `organization_id = :tenantId` + * (ADR-0131 D8 — no NULL arm), so a row enqueued WITHOUT one is reached + * by no tenant-scoped read or redeliver on a walled deployment: an + * organization-less row on a tenant table is invalid state there + * (ADR-0131 D1). * Producers MUST thread the organization they are acting for whenever * they have one: the flow `http` node passes its run's acting tenant * (`AutomationContext.tenantId`, the same source as the `notify` node's @@ -184,7 +186,8 @@ export interface EnqueueHttpInput { * organization (`sys_webhook.organization_id`). Threaded, never * fabricated: a producer with genuinely no organization — a * `single`-posture deployment, a stack before its first organization — - * leaves it absent, and the row lands NULL, the honest global-row shape. + * leaves it absent, and the row lands NULL — honest under `single`, where + * there is no wall and no organization for it to belong to. * * Deliberately OPTIONAL, mirroring `EnqueueDeliveryInput.organizationId` * — the sibling notification outbox's identical repair — so the two diff --git a/packages/services/service-messaging/src/memory-http-outbox.ts b/packages/services/service-messaging/src/memory-http-outbox.ts index b7ed88f5f15..15802bec11a 100644 --- a/packages/services/service-messaging/src/memory-http-outbox.ts +++ b/packages/services/service-messaging/src/memory-http-outbox.ts @@ -203,14 +203,16 @@ export class MemoryHttpOutbox implements IHttpOutbox { * owes the predicate here") — and `insert()` now stamps * {@link HttpDelivery.organizationId}, so the debt is due. * - * The predicate mirrors the SQL driver's tenant term + * The predicate was written to mirror the SQL driver's v17 tenant term * `(organization_id = :tenantId OR organization_id IS NULL)`, all three * arms deliberately: * - a row in ANOTHER organization is INVISIBLE (`RESOURCE_NOT_FOUND`), * never forbidden — the same non-oracle refusal the contract rules; - * - a row with NO organization is a global row every tenant may reach - * (the driver's deliberate fail-open arm — hiding platform rows from - * every tenant is a different defect); + * - a row with NO organization is reachable by every tenant here. ⚠️ The + * SQL driver no longer does this: ADR-0131 D8 retired its NULL arm, so + * there the term is `organization_id = :tenantId` alone and an + * organization-less row is reached by no tenant. This in-memory store + * still admits it; * - a caller with NO tenant (`tenantId: undefined`) stays unscoped, * which is the honest degraded shape {@link RedeliverOptions} rules * for genuinely tenant-less deployments. diff --git a/packages/services/service-messaging/src/sql-http-outbox.ts b/packages/services/service-messaging/src/sql-http-outbox.ts index ceb42436327..97628b775c6 100644 --- a/packages/services/service-messaging/src/sql-http-outbox.ts +++ b/packages/services/service-messaging/src/sql-http-outbox.ts @@ -75,8 +75,9 @@ interface DeliveryRow { payload_json: string; /** * [#13546] Kernel-provisioned tenant column — the predicate of the - * cross-organization wall on `redeliver()` (#10740). NULL = global row, - * visible to every organization (the driver's deliberate fail-open arm). + * cross-organization wall on `redeliver()` (#10740). NULL = a row of no + * organization, which no tenant-scoped read reaches (ADR-0131 D8 retired + * the driver's NULL arm). */ organization_id?: string | null; partition_key: number; @@ -194,8 +195,8 @@ export class SqlHttpOutbox implements IHttpOutbox { // line `SqlOutbox.enqueue` writes. There is no execution context on // this path (both producers run off the write path), so the row // value is the ONE seam that can scope this row; without it every - // row lands in the driver's `organization_id IS NULL` global-row - // arm and the redeliver() wall (#10740) excludes nothing. The + // row lands organization-less, which no tenant-scoped redeliver() + // reaches (ADR-0131 D8 — the driver has no NULL arm). The // explicit `?? null` normalizes "producer has no organization" // to NULL exactly once, here. organization_id: input.organizationId ?? null, diff --git a/packages/services/service-storage/src/backfill-sys-file-organizations.ts b/packages/services/service-storage/src/backfill-sys-file-organizations.ts index c06b9b8865a..eaa9155ee8e 100644 --- a/packages/services/service-storage/src/backfill-sys-file-organizations.ts +++ b/packages/services/service-storage/src/backfill-sys-file-organizations.ts @@ -11,15 +11,16 @@ * `injectTenantOnInsert` with a `tenantId` to stamp from. It wrote nothing to * existing rows, and on a WALLED deployment that asymmetry is not cosmetic: * - * - the SQL driver's own tenant predicate is NULL-TOLERANT — + * - the SQL driver's own tenant predicate was then NULL-TOLERANT — * `(organization_id = :tenantId OR organization_id IS NULL)` — so an * unstamped row stayed readable by everyone; * - but Layer 0 (`plugin-security`'s `computeTenantLayer0Filter`) AND-composes - * a STRICT `organization_id = ` above it, and - * `bootstrap-declared-permissions.ts` states the consequence in terms: - * "Layer 0's strict `organization_id = :tenant` AND-composes over the - * driver's compatibility arm and the conjunction is the strict equality - * alone." + * a STRICT `organization_id = ` above it, and the conjunction + * was the strict equality alone. + * + * (ADR-0131 D8 has since retired the driver's NULL arm: both layers now + * compile the equality alone, so an unstamped row is unreachable by every + * tenant-scoped read — the split this backfill exists to prevent.) * * ⇒ forward-only stamping would split the table in two: new files org-walled, * every existing NULL-org file invisible to EVERY principal. The backfill is diff --git a/packages/services/service-storage/src/metadata-store.ts b/packages/services/service-storage/src/metadata-store.ts index d7257043a4a..dabaa18cf83 100644 --- a/packages/services/service-storage/src/metadata-store.ts +++ b/packages/services/service-storage/src/metadata-store.ts @@ -86,20 +86,19 @@ export interface FileRecord { * - **insert** — `injectTenantOnInsert` STAMPS `options.tenantId` onto the * row when the column is unset. The context supplies a VALUE. * - **update / delete** — `applyTenantScope` SCOPES the statement: the - * predicate becomes `(organization_id = :tenantId OR organization_id IS - * NULL)`. The context supplies a REACH. + * predicate becomes `organization_id = :tenantId` (ADR-0131 D8 — no NULL + * arm). The context supplies a REACH. * * So threading it here buys two distinct things. A row stamped for ANOTHER * organization stops being reachable through these doors — the isolation the - * insert-side repair could not give a door it does not run through. And the - * org-less rows stay reachable: the `OR … IS NULL` arm is #2734's deliberate - * global-row fail-open, so every `sys_upload_session` row written before - * #12928 (deliberately not backfilled — they age out through the object's own - * ADR-0057 TTL sweep) and every `sys_file` row #12745's backfill did not reach - * keeps updating and deleting exactly as it did. ⛔ This file does not restate - * that read semantics as a rule, still less change it — it is - * `applyTenantScope`'s, and it is named here only because it is what makes the - * narrowing safe for legacy rows. + * insert-side repair could not give a door it does not run through. An + * org-less row is out of that reach too: ADR-0131 D8 retired the driver's + * `OR … IS NULL` arm, so a `sys_upload_session` row written before #12928 or + * a `sys_file` row #12745's backfill did not reach is no tenant-scoped write's + * to update or delete — under ADR-0131 such a row is invalid state on a + * walled deployment, and the 18.x upgrade ceremony (D10) gives it its fate + * before boot. ⛔ This file does not restate that read semantics as a rule, + * still less change it — it is `applyTenantScope`'s. * * Third, and the reason this card exists: it silences the `[tenant-audit]` * warning on the `update` and `delete` verbs, which raise it from the same @@ -654,13 +653,14 @@ export class StorageMetadataStore { * {@link updateFile}. Same channel, same chokepoint, and the same split * between stamping and scoping that {@link StorageWriteContext} records. * - * ⚠️ This object is the one where the `OR … IS NULL` arm of the driver's - * scope is load-bearing rather than incidental: #12928 was ruled FORWARD - * STAMP ONLY, so every session row written before it carries - * `organization_id` NULL by decision. Narrowing this statement with strict - * equality would have stranded exactly those rows mid-upload. It does not, - * because the tenant term the driver composes keeps org-less rows in reach - * (#2734). + * ⚠️ This object is the one where the driver's old `OR … IS NULL` arm was + * load-bearing rather than incidental: #12928 was ruled FORWARD STAMP ONLY, + * so every session row written before it carried `organization_id` NULL by + * decision, and that arm kept those rows in reach of this statement + * mid-upload. ADR-0131 D8 retired the arm — the tenant term is strict + * equality now — and under ADR-0131 such a row is invalid state on a walled + * deployment, given its fate by the 18.x upgrade ceremony (D10) before + * boot. */ async updateSession( id: string, @@ -784,8 +784,11 @@ export class StorageMetadataStore { * - the engine-absent stand-in does not scope → `null`, always; * - no organization threaded (`writeOptionsFor` → `undefined`) → the write is * unscoped → `null`; - * - a row stamped with no organization → `null`: the driver's scope keeps - * org-less rows in reach (#2734's global-row arm); + * - a row stamped with no organization → `null`. This branch predates + * ADR-0131 D8: the driver's scope no longer keeps org-less rows in reach + * (the v17 NULL arm is retired), so for such a row the answer is the + * pre-D8 one; under ADR-0131 the row is invalid state on a walled + * deployment, which the 18.x upgrade ceremony (D10) clears before boot; * - otherwise the row is in reach exactly when its organization is the acting * one. * diff --git a/packages/spec/src/api/events.zod.ts b/packages/spec/src/api/events.zod.ts index ea9532c0792..9a371876bfc 100644 --- a/packages/spec/src/api/events.zod.ts +++ b/packages/spec/src/api/events.zod.ts @@ -262,13 +262,15 @@ export const DataEventSchema = lazySchema(() => z.object({ * `@objectstack/spec/security`): there is no organization wall and * nothing stamps the column, so EVERY event is organization-less; * - a row that carries no organization under a walled posture (`group` / - * `isolated`): an environment-wide row (`organization_id IS NULL`), or a - * row of an object that stands outside the wall — `tenancy.enabled: - * false` by declaration, or no `organization_id` column at all (the - * identity tables). + * `isolated`): a row of an object that stands outside the wall — + * `tenancy.enabled: false` by declaration, or no `organization_id` + * column at all (the identity tables, and the deployment-level state of + * ADR-0131 D7). An organization-less row on a tenant table is not a + * third case: ADR-0131 D1 makes it invalid state, and no tenant-scoped + * read reaches it (D8). * In both, a consumer may read absence as "not behind any organization - * wall" — the reading it already gives an `organization_id IS NULL` row on - * the read path. It may NOT read absence as "unknown, resolve it yourself": + * wall" — the reading the read path gives a row of an object with no + * tenant column. It may NOT read absence as "unknown, resolve it yourself": * either the producer had the organization in hand or the record has none, * and a per-event lookup on the fan-out path is exactly the hot-path read * this member exists to make unnecessary. diff --git a/packages/spec/src/data/driver.zod.ts b/packages/spec/src/data/driver.zod.ts index 201bf792f42..4ab081012b3 100644 --- a/packages/spec/src/data/driver.zod.ts +++ b/packages/spec/src/data/driver.zod.ts @@ -60,7 +60,8 @@ export const DriverOptionsSchema = lazySchema(() => z.object({ * * Semantics for drivers that implement native scoping: * - non-empty array → scope reads/updates/deletes/aggregates with `IN`, - * keeping any NULL-tenant global-row carve-out the equality path has; + * and nothing else — no NULL arm, exactly as the equality path + * (ADR-0131 D8); * - absent or empty → fall back to `tenantId` equality (fail toward * isolation, never toward exposure); * - insert-side tenant injection ALWAYS uses `tenantId` (the active From cd794f66c6c4625079b38fd23363f56ccdb25b87 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 11:36:02 +0000 Subject: [PATCH 3/6] docs: restate the duplicates live condition, the NULL-safe bucket, the legacy-composite rule text and SharingRuleRow for ADR-0131 - cli.mdx / seed-tenancy-repair.mdx: the live condition is the record of which counter minted the duplicates; since the counter refuses an organization-less row on a tenant table wherever the organization object is registered, it no longer forecasts more. - schema.mdx: a tenant table holds no organization-less row wherever the organization object is registered. - unique/legacy-organization-composite: carry that same hedge. - SharingRuleRow: deploymentWide has no sys_sharing_rule column; the interface is no longer a 1-to-1 stored shape. Claude-Session: https://claude.ai/code/session_017ae4eMXWeERGrG6heTbwBX Co-authored-by: Claude --- content/docs/deployment/cli.mdx | 8 ++++---- content/docs/deployment/seed-tenancy-repair.mdx | 5 ++--- content/docs/protocol/objectql/schema.mdx | 10 +++++++--- packages/lint/src/data-model-rules.ts | 4 +++- packages/lint/src/rule-explanations.ts | 5 +++-- packages/spec/src/contracts/sharing-service.ts | 8 ++++++-- 6 files changed, 25 insertions(+), 15 deletions(-) diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 8227b1fcfb2..b888e1dd5a4 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -1797,12 +1797,12 @@ this report reads. Half of what it can tell you does not survive that: | Reported | Survives the repair? | | :--- | :--- | -| **The duplicates themselves** — every value held across two partitions, with the id, organization and creation time of each holder | **Yes.** The repair deliberately refuses to adopt a row whose identifier is already taken in the destination partition, so those rows keep `organization_id = NULL` and stay visible | -| **The live condition** — an object still running a global counter beside an organization-scoped one, and therefore about to mint more duplicates | **No.** The repair merges the two counters and deletes the global one. Once that has happened, this line can never be produced again | +| **The duplicates themselves** — every value held across two partitions, with the id, organization and creation time of each holder | **Yes.** The repair deliberately refuses to adopt a row whose identifier is already taken in the destination partition, so those rows keep `organization_id = NULL` and stay findable by this report | +| **The live condition** — an object still holding a global counter beside an organization-scoped one: the record of which counter minted the duplicates above. Under 17.x it also forecast more; since the counter refuses an organization-less row on a tenant table wherever the organization object is registered, nothing mints from the global counter there | **No.** The repair merges the two counters and deletes the global one. Once that has happened, this line can never be produced again | Running it afterwards is still worth doing — the inventory is what you act on, -and it is complete either way. What you cannot recover is the forward-looking -half. +and it is complete either way. What you cannot recover is the counter +evidence. A deployment holding **more than one organization** is skipped by that repair rather than guessed at: there is no derivable answer to which organization owns diff --git a/content/docs/deployment/seed-tenancy-repair.mdx b/content/docs/deployment/seed-tenancy-repair.mdx index 24ba2ab3876..f998494c3ab 100644 --- a/content/docs/deployment/seed-tenancy-repair.mdx +++ b/content/docs/deployment/seed-tenancy-repair.mdx @@ -302,10 +302,9 @@ Be precise about what is lost, because the two halves behave differently: | | Survives the repair? | | :--- | :--- | | The inventory of identifiers **already** minted twice | **Yes.** Colliding rows deliberately keep `organization_id = NULL`, so they remain in two partitions and remain findable. | -| The **live condition** — an install still holding a `__global__` counter beside an organization-scoped one, i.e. *about to mint more* | **No.** The repair deletes that counter row. Once it has run, this condition can never be produced again. | +| The **live condition** — an install still holding a `__global__` counter beside an organization-scoped one: the record of which counter minted the duplicates. Under 17.x it also forecast more; since the counter refuses an organization-less row on a tenant table wherever the organization object is registered, nothing mints from that counter there | **No.** The repair deletes that counter row. Once it has run, this condition can never be produced again. | -The live condition is the only forward-looking line in the report, and it is the -perishable one. On a deployment you have not yet restarted since discovering the +The live condition is the perishable line in the report. On a deployment you have not yet restarted since discovering the problem, run the report first. `os migrate duplicates` writes no row: it boots read-only, declares itself out diff --git a/content/docs/protocol/objectql/schema.mdx b/content/docs/protocol/objectql/schema.mdx index 4995dd41bb2..d7dbf6ebf40 100644 --- a/content/docs/protocol/objectql/schema.mdx +++ b/content/docs/protocol/objectql/schema.mdx @@ -606,9 +606,13 @@ stays valid indefinitely; `'organization'` is simply the preferred spelling in new code. **The organization key part is NULL-safe.** It materializes as -`COALESCE(organization_id, '__global__')`, so rows carrying no organization — -platform rows, and *every* row on a single-organization deployment — form one -platform bucket that is unique among itself. A plain `(organization_id, field)` +`COALESCE(organization_id, '__global__')`, so rows carrying no organization +form one bucket that is unique among itself. Wherever the organization object +is registered, a tenant table holds no such row (ADR-0131: every write is +stamped with its organization — on a single-organization deployment, the +Default Organization); the bucket is for a composition that registers no +organization object, and for residue an uncertified pre-18.x database still +holds. A plain `(organization_id, field)` composite would enforce nothing on those rows, because SQL `UNIQUE` treats every `NULL` as distinct. The value stored in the column is still `NULL`; the sentinel exists only inside the index key, so `WHERE organization_id = '__global__'` diff --git a/packages/lint/src/data-model-rules.ts b/packages/lint/src/data-model-rules.ts index 2cb1309274e..ca5eb1276ad 100644 --- a/packages/lint/src/data-model-rules.ts +++ b/packages/lint/src/data-model-rules.ts @@ -594,7 +594,9 @@ export function lintUniqueDeclarations(objects: any[]): LocatedLintIssue[] { * UNIQUE is NULL-distinct, so on every row where the organization column is * NULL it enforces **nothing** (#5030, measured). A 17.x single-organization * deployment wrote *every* row that way; since ADR-0131 D9 a new row carries its - * organization, so the gap now admits that residue. The `'organization'` + * organization wherever the organization object is registered, so there the gap + * now admits that residue (a composition that registers no organization object + * still writes NULL-organization rows). The `'organization'` * respelling is what closes the hole: the driver makes the LISTED organization * column NULL-safe in place (`COALESCE(organization_id, '__global__')`), so the * NULL rows become one bucket that is unique among themselves. diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index f79859473e8..8d46ddb7334 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -2695,8 +2695,9 @@ const UNIQUE_LEGACY_ORGANIZATION_COMPOSITE_EXPLANATION: RuleExplanation = { 'predates the scope vocabulary (ADR-0120 S6). It reads as "unique per organization" but ' + 'materializes as a plain composite, and SQL UNIQUE is NULL-distinct: it enforces nothing on a ' + 'row whose organization column is NULL — every row a 17.x single-organization deployment ' + - 'wrote. Since ADR-0131 D9 a new row carries its organization, so what the gap still admits is ' + - 'that residue.', + 'wrote. Since ADR-0131 D9 a new row carries its organization wherever the organization object ' + + 'is registered, so what the gap still admits there is that residue; a composition that ' + + 'registers no organization object still writes rows whose organization column is NULL.', 'Respelled `unique: \'organization\'`, the NULL rows become one bucket ' + '(`COALESCE(organization_id, \'__global__\')`) that is unique among themselves, which is what ' + 'closes the hole. The respelling keeps `fields` exactly as they are: the driver makes the listed ' + diff --git a/packages/spec/src/contracts/sharing-service.ts b/packages/spec/src/contracts/sharing-service.ts index 93ab729cacc..7e60c9b3f8c 100644 --- a/packages/spec/src/contracts/sharing-service.ts +++ b/packages/spec/src/contracts/sharing-service.ts @@ -475,8 +475,12 @@ export interface ISharingService { export type SharingRuleRecipientType = 'user' | 'team' | 'business_unit' | 'position' | 'unit_and_subordinates' | 'queue' | 'field'; /** - * Stored shape of a sharing rule. Maps 1-to-1 to `sys_sharing_rule` - * with `criteria` lifted out of the JSON column. + * A sharing rule as the service serves it: a stored `sys_sharing_rule` row + * (`criteria` lifted out of the JSON column), or a DECLARED rule's + * projection. Every member but one maps to a `sys_sharing_rule` column; + * {@link SharingRuleRow.deploymentWide} has no column there — it marks a + * declared rule's deployment-wide projection and is absent on every stored + * row. */ export interface SharingRuleRow { id: string; From 8c68d506bf7b10556a723efa936ba39044672ddf Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 11:48:29 +0000 Subject: [PATCH 4/6] chore(changeset): patch for the ADR-0131 D8 text corrections Claude-Session: https://claude.ai/code/session_017ae4eMXWeERGrG6heTbwBX Co-authored-by: Claude --- .changeset/24583-7z-closing-d8-texts.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 .changeset/24583-7z-closing-d8-texts.md diff --git a/.changeset/24583-7z-closing-d8-texts.md b/.changeset/24583-7z-closing-d8-texts.md new file mode 100644 index 00000000000..2dafb8ef079 --- /dev/null +++ b/.changeset/24583-7z-closing-d8-texts.md @@ -0,0 +1,24 @@ +--- +'@objectstack/spec': patch +'@objectstack/lint': patch +'@objectstack/objectql': patch +'@objectstack/runtime': patch +'@objectstack/plugin-security': patch +'@objectstack/plugin-sharing': patch +'@objectstack/plugin-webhooks': patch +'@objectstack/service-automation': patch +'@objectstack/service-messaging': patch +'@objectstack/service-storage': patch +--- + +docs: shipped texts no longer say the SQL driver reads organization-less rows + +Clause-②: no (texts only: no accept set, served body, error code or behaviour moves) + +ADR-0131 D8 retired the driver's `OR organization_id IS NULL` arm. Several shipped texts still described it as the live tenant predicate. They now state the current rule: the tenant term is `organization_id = :tenant`, or `IN (:tenants)` under the `group` posture, and nothing else. No tenant-scoped read reaches an organization-less row on a tenant table. + +- **`@objectstack/service-automation`.** The `[http]` node's warning for a durable callout with no organization no longer says the `sys_http_delivery` row is "a global row every organization's redeliver door can reach". It now says no organization's redeliver door reaches that row. +- **`@objectstack/objectql`.** The `sys_record_share` entry of the platform-object tenancy inventory carries an updated `evidence` text. +- **`@objectstack/lint`.** `os explain unique/legacy-organization-composite` now says that a new row carries its organization wherever the organization object is registered. A composition that registers no organization object still writes rows whose organization column is NULL. +- **`@objectstack/spec`.** The TSDoc of `SharingRuleRow` no longer calls it a 1-to-1 stored shape: `deploymentWide` has no `sys_sharing_rule` column. The TSDoc of `DriverOptions.tenantIds` and of the events `organizationId` member describe the predicate without a NULL arm. +- **The other packages.** Code comments only. From 4351e295be70a69b3fad38f7e668744509f38e19 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 12:34:40 +0000 Subject: [PATCH 5/6] fix(service-messaging): the in-memory HTTP outbox's redeliver() no longer lets a tenant replay an organization-less row MemoryHttpOutbox.redeliver() still carried the v17 NULL arm: a row with no organization passed every tenant's wall. ADR-0131 D8 rules a NULL arm out in every layer, and SqlHttpOutbox (through the driver's applyTenantScope) already answers RESOURCE_NOT_FOUND there. Match it, and flip the pin that held the old arm into one that refuses it. Claude-Session: https://claude.ai/code/session_017ae4eMXWeERGrG6heTbwBX Co-authored-by: Claude --- .../src/http-outbox-organization.test.ts | 33 +++++++++++-------- .../src/memory-http-outbox.ts | 17 ++++------ 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/packages/services/service-messaging/src/http-outbox-organization.test.ts b/packages/services/service-messaging/src/http-outbox-organization.test.ts index 9bdfddc3ae2..3dacee674d1 100644 --- a/packages/services/service-messaging/src/http-outbox-organization.test.ts +++ b/packages/services/service-messaging/src/http-outbox-organization.test.ts @@ -4,12 +4,12 @@ * [#13546] `sys_http_delivery` rows carry the producer's organization. * * The consequence being pinned: the cross-organization wall on `redeliver()` - * (#10740) scopes by the row's `organization_id`, and the driver's tenant term - * is `(organization_id = :tenantId OR organization_id IS NULL)` — a deliberate - * global-row fail-open. A row enqueued without an organization therefore - * belongs to NO organization and is visible to EVERY one; before this repair - * the enqueue door never stamped the column, so 100% of rows were in that arm - * and the wall excluded nothing. + * (#10740) scopes by the row's `organization_id`. The driver's tenant term was + * then `(organization_id = :tenantId OR organization_id IS NULL)`, a global-row + * fail-open, and the enqueue door never stamped the column, so 100% of rows + * were in that arm and the wall excluded nothing. ADR-0131 D8 has since + * retired the NULL arm: the term is `organization_id = :tenantId` alone, and + * an organization-less row is reached by no tenant-scoped redeliver. * * ⭐ The control for this suite is the sibling notification outbox * (`SqlOutbox.enqueue` writes `organization_id: input.organizationId ?? null`) @@ -166,16 +166,23 @@ describe('#13546 — MemoryHttpOutbox parity', () => { expect(row.organizationId).toBe('org_pin_alpha'); }); - it('an org-less row stays a GLOBAL row any tenant may replay (the driver fail-open arm, mirrored)', async () => { - // The over-denial control: hiding NULL rows from every tenant would be - // a different defect (platform rows invisible to everyone). The memory - // predicate must mirror `(organization_id = :tenantId OR organization_id - // IS NULL)`, not tighten it. + it('an org-less row is INVISIBLE to every tenant-scoped redeliver — no NULL arm (ADR-0131 D8)', async () => { + // The memory predicate mirrors the SQL driver's `organization_id = + // :tenantId` with no `OR organization_id IS NULL` arm: NULL is not an + // owner, so no tenant may replay a row of no organization. const outbox = new MemoryHttpOutbox(); const id = await deadRow(outbox); - const row = await outbox.redeliver(id, { tenantId: 'org_pin_beta' }); - expect(row.status).toBe('pending'); + const attempt = outbox.redeliver(id, { tenantId: 'org_pin_beta' }); + await expect(attempt).rejects.toBeInstanceOf(HttpRedeliverError); + await expect(attempt).rejects.toMatchObject({ code: 'RESOURCE_NOT_FOUND' }); + const [row] = await outbox.list({ status: 'dead' }); + expect(row.id).toBe(id); + expect(row.attempts).toBe(1); + + // Still reachable where nothing scopes: a tenant-less caller. + const replayed = await outbox.redeliver(id, { tenantId: undefined }); + expect(replayed.status).toBe('pending'); }); it('a tenant-less caller stays unscoped (the honest degraded shape RedeliverOptions rules)', async () => { diff --git a/packages/services/service-messaging/src/memory-http-outbox.ts b/packages/services/service-messaging/src/memory-http-outbox.ts index 15802bec11a..2f2964aec20 100644 --- a/packages/services/service-messaging/src/memory-http-outbox.ts +++ b/packages/services/service-messaging/src/memory-http-outbox.ts @@ -203,16 +203,13 @@ export class MemoryHttpOutbox implements IHttpOutbox { * owes the predicate here") — and `insert()` now stamps * {@link HttpDelivery.organizationId}, so the debt is due. * - * The predicate was written to mirror the SQL driver's v17 tenant term - * `(organization_id = :tenantId OR organization_id IS NULL)`, all three - * arms deliberately: + * The predicate mirrors the SQL driver's tenant term + * `organization_id = :tenantId` (ADR-0131 D8 — no layer has a NULL arm): * - a row in ANOTHER organization is INVISIBLE (`RESOURCE_NOT_FOUND`), * never forbidden — the same non-oracle refusal the contract rules; - * - a row with NO organization is reachable by every tenant here. ⚠️ The - * SQL driver no longer does this: ADR-0131 D8 retired its NULL arm, so - * there the term is `organization_id = :tenantId` alone and an - * organization-less row is reached by no tenant. This in-memory store - * still admits it; + * - a row with NO organization is invisible to every tenant-scoped + * caller too: NULL is not an owner (ADR-0131 D1), so no tenant's + * redeliver reaches it — exactly as `SqlHttpOutbox` answers; * - a caller with NO tenant (`tenantId: undefined`) stays unscoped, * which is the honest degraded shape {@link RedeliverOptions} rules * for genuinely tenant-less deployments. @@ -221,9 +218,7 @@ export class MemoryHttpOutbox implements IHttpOutbox { const row = this.rows.get(id); if ( !row || - (options.tenantId !== undefined && - row.organizationId !== undefined && - row.organizationId !== options.tenantId) + (options.tenantId !== undefined && row.organizationId !== options.tenantId) ) { throw new HttpRedeliverError(`Delivery row '${id}' not found`, 'RESOURCE_NOT_FOUND'); } From 2db73df02a49f593d04addbf0616f95cb1924783 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 12:38:21 +0000 Subject: [PATCH 6/6] chore(changeset): record the in-memory HTTP outbox fix Claude-Session: https://claude.ai/code/session_017ae4eMXWeERGrG6heTbwBX Co-authored-by: Claude --- .changeset/24583-7z-closing-d8-texts.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.changeset/24583-7z-closing-d8-texts.md b/.changeset/24583-7z-closing-d8-texts.md index 2dafb8ef079..6d98e7642e6 100644 --- a/.changeset/24583-7z-closing-d8-texts.md +++ b/.changeset/24583-7z-closing-d8-texts.md @@ -11,12 +11,13 @@ '@objectstack/service-storage': patch --- -docs: shipped texts no longer say the SQL driver reads organization-less rows +fix: no shipped text says the SQL driver reads organization-less rows, and the in-memory HTTP outbox no longer does -Clause-②: no (texts only: no accept set, served body, error code or behaviour moves) +Clause-②: no (one fix and texts: the in-memory HTTP outbox's `redeliver()` now answers as `SqlHttpOutbox` already did; no accept set, served body or error code of a shipped composition moves) ADR-0131 D8 retired the driver's `OR organization_id IS NULL` arm. Several shipped texts still described it as the live tenant predicate. They now state the current rule: the tenant term is `organization_id = :tenant`, or `IN (:tenants)` under the `group` posture, and nothing else. No tenant-scoped read reaches an organization-less row on a tenant table. +- **`@objectstack/service-messaging` — fix.** `MemoryHttpOutbox.redeliver()` no longer lets a tenant-scoped caller replay a row with no organization. It answers `RESOURCE_NOT_FOUND`, as `SqlHttpOutbox` does through the driver's tenant scope (ADR-0131 D8: no layer has a NULL arm). A caller with no tenant is still unscoped. No shipped composition constructs the in-memory outbox; the messaging plugin always builds `SqlHttpOutbox`. Code that embeds `MemoryHttpOutbox` directly sees the change. - **`@objectstack/service-automation`.** The `[http]` node's warning for a durable callout with no organization no longer says the `sys_http_delivery` row is "a global row every organization's redeliver door can reach". It now says no organization's redeliver door reaches that row. - **`@objectstack/objectql`.** The `sys_record_share` entry of the platform-object tenancy inventory carries an updated `evidence` text. - **`@objectstack/lint`.** `os explain unique/legacy-organization-composite` now says that a new row carries its organization wherever the organization object is registered. A composition that registers no organization object still writes rows whose organization column is NULL.