From 20d3bed6f78d21827166b5156baaaf0a261db567 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 19:28:06 +0000 Subject: [PATCH 01/10] wip(sharing): stamp organization_id on sys_record_share writes; ledger row; backfill module Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- .../tenancy-by-object-classification.test.ts | 1 + .../src/tenancy/platform-object-tenancy.ts | 17 + ...backfill-sys-record-share-organizations.ts | 708 ++++++++++++++++++ .../src/sharing-rule-service.ts | 18 +- .../plugin-sharing/src/sharing-service.ts | 135 +++- 5 files changed, 873 insertions(+), 6 deletions(-) create mode 100644 packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.ts diff --git a/packages/objectql/src/tenancy-by-object-classification.test.ts b/packages/objectql/src/tenancy-by-object-classification.test.ts index 87e591d98e..6a53cbbb41 100644 --- a/packages/objectql/src/tenancy-by-object-classification.test.ts +++ b/packages/objectql/src/tenancy-by-object-classification.test.ts @@ -166,6 +166,7 @@ describe('#13491 the inventory — a verdict per object, never a namespace', () 'sys_automation_run', 'sys_file', 'sys_notification_delivery', + 'sys_record_share', 'sys_upload_session', ]); }); diff --git a/packages/objectql/src/tenancy/platform-object-tenancy.ts b/packages/objectql/src/tenancy/platform-object-tenancy.ts index eb224531ff..c439d95cce 100644 --- a/packages/objectql/src/tenancy/platform-object-tenancy.ts +++ b/packages/objectql/src/tenancy/platform-object-tenancy.ts @@ -172,6 +172,23 @@ export const PLATFORM_OBJECT_TENANCY: Readonly` above it, and the conjunction is + * the strict equality alone. + * + * ⇒ 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 + * two: new grants walled, every existing grant gone. The backfill is what keeps + * the observable behaviour uniform. (`single` posture is inert — + * `computeTenantLayer0Filter` returns `null` — so nothing here changes for a + * single-tenant install beyond the column being filled.) + * + * ## Maintainer order — `sys_record_share` and nothing else + * + * The tree's precedents for this shape are + * `plugin-approvals/src/backfill-platform-row-organizations.ts` and + * `service-storage/src/backfill-sys-file-organizations.ts`, and both require a + * MAINTAINER ORDER PER TABLE. The 2026-09-02 ruling on #14484 (decision batch + * #11 item 3, maintainer verbatim 「#13564 转维护者处理；其他同意」 — "其他同意" + * adopts A: tenant-scoped, writer-repaired, existing rows backfilled from the + * record they grant access to) IS that order, and it is the order for + * `sys_record_share` ALONE. ⛔ Do not extend + * {@link SYS_RECORD_SHARE_BACKFILL_OBJECT} to a second table; a second table + * needs its own ruling. + * + * ## Deriving the organization — from the RECORD the grant is about + * + * A grant row says "principal P has level L on (object O, record R)". R lives + * in exactly one organization, so the grant's organization is R's — read off + * the column O is actually walled by ({@link resolveTenantFieldName}: ADR-0066 + * opt-out → declared `tenancy.tenantField` → injected `organization_id`), the + * same column the wall will scope O's rows by. The ruling calls this derivation + * "derivable and lossless", which is why no stored-population survey precedes + * it: the maintainer's standing 「不考虑存量」 applies to surveys, not to a + * derivation. + * + * ⛔ Nothing is guessed. The recipient (`recipient_id`) and the granter + * (`granted_by`) are NOT subjects: a user may belong to many organizations, so + * deriving from either would invent an answer the row does not carry. A rule + * row (`source_id`) is not consulted either: the writer stamps the rule's + * organization at write time because the rule's sweep ran under it, but for a + * row at rest the record is the one subject that is both present and + * unambiguous — and every organization-scoped rule's sweep only ever matched + * records in its own organization (#10119), so the two answers agree wherever + * both exist. + * + * ## ⭐ Orphans — the choice the ruling left to the implementer, and why + * + * A grant row whose record no longer exists is an orphan. The ruling offered + * "delete" or "leave NULL with a logged count". This sweep LEAVES THEM NULL, + * COUNTS them ({@link SysRecordShareBackfillResidue.recordNotFound}, + * `totals.orphans`) and LOGS the count — and never deletes anything, for one + * reason: the invariant "record gone ⇒ the row cannot describe any access" is + * already owned, by `record-orphan-cleanup.ts` and the `kernel:bootstrapped` + * sweep `SharingService.sweepOrphanedRecordShares` (#5103). That sweep runs on + * every boot, ahead of the rule-grant passes, and deletes exactly this + * population. A second deleter here would be the fork that module exists to + * prevent — two copies of one invariant, each with its own chunk size, its + * own "a failed probe deletes NOTHING" rule, drifting apart. So the orphan + * count this sweep reports is the population the next boot reclaims, and a + * non-zero count after a boot is a finding about the sweep, not about this + * module. + * + * ## The residue is the deliverable, not the leftovers + * + * ⭐ Rows that cannot be derived stay NULL and are REPORTED — + * {@link SysRecordShareBackfillReport.totals}`.residualNull`, broken out by + * reason in {@link SysRecordShareBackfillResidue}, and printed by + * {@link formatSysRecordShareOrganizationBackfillReport}. Those rows remain + * invisible to every tenant-scoped reader on a walled deployment, and that is + * the maintainer's to see — ⛔ never silently accepted. The count is reported + * for a DRY RUN too, which is what makes the dry run a decision document + * rather than a preview. + * + * ## Idempotency + * + * The scan is `WHERE IS NULL` and every write fills that + * column, so a repaired row cannot match again: `planned` and `written` are + * both 0 on a second run over an unchanged database. Rows deliberately left + * alone keep matching and keep being REPORTED, never re-written. + * `backfill-sys-record-share-organizations.test.ts` asserts the second run + * rather than describing it. + * + * ## Usage + * + * Not exported from the package index and not shipped in `dist` — this is a + * one-off operational module, not platform surface (the same posture as both + * precedents). Run it server-side from a context that holds an engine: + * + * ```ts + * const report = await planSysRecordShareOrganizationBackfill(engine); + * console.log(formatSysRecordShareOrganizationBackfillReport(report)); // writes nothing + * // …read it, then: + * await applySysRecordShareOrganizationBackfill(engine, report); + * ``` + * + * Rollback posture: the dry run names every row id it would touch and the value + * it would write, so the undo is to write the previous value (NULL) back to + * exactly those ids. + */ + +import { resolveTenantFieldName } from '@objectstack/objectql'; + +/** + * The ONE object this sweep repairs. ⛔ Scope-pinned by the 2026-09-02 ruling + * on #14484 — a second table needs its own maintainer order (see the module + * doc). + */ +export const SYS_RECORD_SHARE_BACKFILL_OBJECT = 'sys_record_share'; + +/** The columns naming the record a grant is about (ADR-0052 §5 pointer pair). */ +const SUBJECT_OBJECT_FIELD = 'object_name'; +const SUBJECT_ID_FIELD = 'record_id'; + +/** + * The sweep's elevation. It has to see rows across every organization — a + * walled read would hide from it exactly the rows it exists to find — and it + * writes with the derived organization threaded as `tenantId`, the shape a + * repaired writer uses (see {@link applySysRecordShareOrganizationBackfill}). + */ +const SYSTEM_CONTEXT = { isSystem: true, positions: [], permissions: [] } as const; +const DEFAULT_PAGE_SIZE = 200; +const DEFAULT_MAX_ROWS = 100_000; +/** Id batches for the subject-record lookups. */ +const LOOKUP_CHUNK = 100; + +/** + * The engine surface the sweep needs — a structural subset of the ObjectQL + * engine, declared here so the module can be driven by a test double without + * pulling the service in. + * + * `getSchema` is what the column resolution probes. An engine without it + * resolves every organization column to `null`, which would make the sweep a + * silent no-op — so the report says so out loud instead (see `notes`). + */ +export interface SysRecordShareBackfillEngine { + find(object: string, options?: unknown): Promise; + update(object: string, data: unknown, options?: unknown): Promise; + getSchema?(object: string): unknown; +} + +/** + * Structural on purpose, and `warn` is REQUIRED: the orphan count — the one + * line the ruling asks this module to log — lands on `warn`, and a logger + * without a guaranteed `warn` is one that line can be lost into (#9754's + * silence rule). Deliberately NO `error` member (see `record-orphan-cleanup.ts` + * for why that shape declares none). + */ +export interface SysRecordShareBackfillLogger { + info?: (msg: any, ...rest: any[]) => void; + warn: (msg: any, ...rest: any[]) => void; +} + +/** The record a grant is about, and the organization it resolved to. */ +export interface SysRecordShareSubject { + object: string; + id: string; + /** `null` when the record is gone, unwalled, or carries no value. */ + organization: string | null; +} + +/** One row the sweep would write, named in full so the dry run is auditable. */ +export interface PlannedSysRecordShareRow { + id: string; + /** The column on `sys_record_share` that carries its organization (schema-resolved). */ + organizationField: string; + /** The value that would be written. */ + organization: string; + /** The record it was derived from, so the derivation is checkable without re-running it. */ + subject: SysRecordShareSubject; +} + +/** + * ⭐ Why each residual row is still NULL. Every counter here is a row that + * stays invisible under a wall, so the breakdown — not just the total — is the + * reportable outcome. + */ +export interface SysRecordShareBackfillResidue { + /** The row carries no usable id, or no `(object_name, record_id)` pair. */ + unaddressable: number; + /** The record's object has no schema this engine can read — the wall column is unknowable. */ + subjectObjectUnknown: number; + /** The record's object has no organization column at all (ADR-0066 opt-out, or none injected). */ + subjectNotOrganizationScoped: number; + /** The record could not be READ (driver error, unmounted table). "Could not ask" is not "gone". */ + subjectReadFailed: number; + /** + * ⭐ ORPHAN — the record no longer exists. Left NULL and counted; the #5103 + * boot sweep (`sweepOrphanedRecordShares`) is the one deleter of this + * population. See the module doc. + */ + recordNotFound: number; + /** The record exists and is organization-scoped, but carries no organization itself. */ + recordHasNoOrganization: number; +} + +/** One row the sweep left alone, with the reason, so the residue is checkable. */ +export interface ResidualSysRecordShareRow { + id: string; + reason: keyof SysRecordShareBackfillResidue; + subject: SysRecordShareSubject | null; +} + +/** The whole sweep's plan / outcome. */ +export interface SysRecordShareBackfillReport { + /** `true` when nothing was written. */ + dryRun: boolean; + /** The schema-resolved organization column on `sys_record_share`, or `null`. */ + organizationField: string | null; + /** Rows matching ` IS NULL` at scan time. */ + scanned: number; + /** Rows the sweep would write (dry run) — see {@link PlannedSysRecordShareRow}. */ + planned: number; + /** Rows actually written. Always 0 on a dry run. */ + written: number; + rows: PlannedSysRecordShareRow[]; + /** ⭐ Rows left NULL, one entry each, with the reason. */ + residualRows: ResidualSysRecordShareRow[]; + residue: SysRecordShareBackfillResidue; + /** Planned rows whose write threw. Reported, never retried, never fatal. */ + failures: Array<{ id: string; error: string }>; + /** Conditions a reader must see, e.g. "this engine exposes no such column". */ + notes: string[]; + totals: { + scanned: number; + planned: number; + written: number; + /** + * ⭐ Rows still carrying a NULL organization when this run finished. On a + * dry run that is every scanned row (nothing was written); on an applied + * run it is what the maintainer is being asked to look at. + */ + residualNull: number; + /** ⭐ The orphan count — `residue.recordNotFound`, surfaced by name because the ruling asks for it. */ + orphans: number; + }; +} + +/** Options both halves of the sweep accept. */ +export interface SysRecordShareBackfillOptions { + /** + * Execution context for every READ. Defaults to a system context — the sweep + * has to see rows across every organization, and a walled read would hide + * from it exactly the rows it exists to find. Writes always thread the + * derived organization as well (see {@link applySysRecordShareOrganizationBackfill}). + */ + context?: unknown; + /** Rows per page while scanning. */ + pageSize?: number; + /** Hard ceiling, so a pathological table cannot spin forever. */ + maxRows?: number; + /** + * `false` writes. Defaults to `true`: a sweep over existing data that + * defaults to writing is one typo away from an unplanned migration, and the + * ruling puts the dry run first anyway. + */ + dryRun?: boolean; + /** Where the orphan count and the run summary are logged. Optional; the report carries both regardless. */ + logger?: SysRecordShareBackfillLogger; +} + +// --------------------------------------------------------------------------- +// Column resolution — the WALL column, asked of the schema +// --------------------------------------------------------------------------- + +/** + * "Which column is THIS object walled by?", resolved from the registered + * schema through the engine's own {@link resolveTenantFieldName} and memoized + * per object. `undefined` (not `null`) when the schema itself could not be + * read — the caller reports that as a different residue from "no column". + */ +function createWallColumnResolver(engine: SysRecordShareBackfillEngine) { + const cache = new Map(); + return (objectName: string): string | null | undefined => { + if (cache.has(objectName)) return cache.get(objectName); + let resolved: string | null | undefined; + if (typeof engine.getSchema !== 'function') { + resolved = undefined; + } else { + let schema: unknown; + try { + schema = engine.getSchema(objectName); + } catch { + schema = undefined; + } + resolved = schema ? resolveTenantFieldName(schema) : undefined; + } + cache.set(objectName, resolved); + return resolved; + }; +} + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/** `''`, `null` and a non-string all mean "no value here". */ +function nonEmpty(value: unknown): string | null { + return typeof value === 'string' && value.length > 0 ? value : null; +} + +function rowId(row: unknown): string | null { + const raw = (row as Record | null)?.id; + if (typeof raw === 'string' && raw.length > 0) return raw; + if (typeof raw === 'number') return String(raw); + return null; +} + +function chunk(items: readonly T[], size: number): T[][] { + const out: T[][] = []; + for (let i = 0; i < items.length; i += size) out.push(items.slice(i, i + size)); + return out; +} + +function emptyResidue(): SysRecordShareBackfillResidue { + return { + unaddressable: 0, + subjectObjectUnknown: 0, + subjectNotOrganizationScoped: 0, + subjectReadFailed: 0, + recordNotFound: 0, + recordHasNoOrganization: 0, + }; +} + +function totalsOf( + report: Omit, +): SysRecordShareBackfillReport['totals'] { + return { + scanned: report.scanned, + planned: report.planned, + written: report.written, + // ⭐ Every scanned row that did not get written is still NULL. On a dry run + // `written` is 0, so this is the whole scan — which is the honest answer to + // "what is still invisible if I stop here?". + residualNull: report.scanned - report.written, + orphans: report.residue.recordNotFound, + }; +} + +/** + * Page through every `sys_record_share` row whose organization column is unset. + * + * Ordered by `id` so the pages partition the population instead of overlapping, + * and read in full BEFORE anything is written — a plan built while writing + * would move rows out from under its own offset. + */ +async function scanUnstampedGrants( + engine: SysRecordShareBackfillEngine, + organizationField: string, + options: { context: unknown; pageSize: number; maxRows: number }, + notes: string[], +): Promise[]> { + const out: Record[] = []; + for (let offset = 0; offset < options.maxRows; offset += options.pageSize) { + let page: unknown[]; + try { + page = await engine.find(SYS_RECORD_SHARE_BACKFILL_OBJECT, { + where: { [organizationField]: null }, + limit: options.pageSize, + offset, + orderBy: [{ field: 'id', order: 'asc' }], + context: options.context, + }); + } catch (err) { + // Named, not thrown: a reader has to be able to tell "no stranded rows" + // from "never looked". + notes.push( + `scan of '${SYS_RECORD_SHARE_BACKFILL_OBJECT}' failed — ${String((err as Error)?.message ?? err)}`, + ); + break; + } + const rows = Array.isArray(page) ? page : []; + for (const row of rows) { + if (row && typeof row === 'object') out.push(row as Record); + } + if (rows.length < options.pageSize) break; + } + return out; +} + +/** + * Read the subject records of one object by id, projected to the wall column. + * Throws on a read failure — the caller MUST treat that as "could not ask", + * never as "none of them exist" (the same rule `findLiveRecordIds` in + * `record-orphan-cleanup.ts` states for the orphan sweep). + */ +async function readSubjects( + engine: SysRecordShareBackfillEngine, + object: string, + organizationField: string, + ids: readonly string[], + context: unknown, +): Promise>> { + const found = new Map>(); + for (const batch of chunk(ids, LOOKUP_CHUNK)) { + const rows = await engine.find(object, { + where: { id: { $in: batch } }, + fields: ['id', organizationField], + limit: batch.length, + context, + }); + for (const row of Array.isArray(rows) ? rows : []) { + const id = rowId(row); + if (id) found.set(id, row as Record); + } + } + return found; +} + +// --------------------------------------------------------------------------- +// Plan (the dry run) +// --------------------------------------------------------------------------- + +/** + * Build the sweep's plan — the DRY RUN. Reads only; `written` is 0. + * + * This is the deliverable in its own right: it is the only thing that shows + * both what will move and — ⭐ via {@link SysRecordShareBackfillReport.residue} + * — what will still be invisible after it moves, the orphan count included. + */ +export async function planSysRecordShareOrganizationBackfill( + engine: SysRecordShareBackfillEngine, + options: SysRecordShareBackfillOptions = {}, +): Promise { + const context = options.context ?? SYSTEM_CONTEXT; + const pageSize = options.pageSize ?? DEFAULT_PAGE_SIZE; + const maxRows = options.maxRows ?? DEFAULT_MAX_ROWS; + const wallColumnOf = createWallColumnResolver(engine); + + const notes: string[] = []; + const rows: PlannedSysRecordShareRow[] = []; + const residualRows: ResidualSysRecordShareRow[] = []; + const residue = emptyResidue(); + const organizationField = wallColumnOf(SYS_RECORD_SHARE_BACKFILL_OBJECT) ?? null; + + const base: Omit = { + dryRun: true, + organizationField, + scanned: 0, + planned: 0, + written: 0, + rows, + residualRows, + residue, + failures: [], + notes, + }; + + if (!organizationField) { + // Not an error, but it MUST be loud: a backfill that silently sweeps + // nothing reads exactly like a clean database. + notes.push( + `no organization column resolved for '${SYS_RECORD_SHARE_BACKFILL_OBJECT}' — nothing scanned. ` + + 'On a multi-tenant install this means the engine exposed no schema for the object; ' + + 'on an install that opted the object out of system fields it is expected.', + ); + return finish(base, options.logger); + } + + const grants = await scanUnstampedGrants( + engine, + organizationField, + { context, pageSize, maxRows }, + notes, + ); + base.scanned = grants.length; + + // ── Address every row, and group the subjects by object ───────────────── + const addressable: Array<{ id: string; object: string; recordId: string }> = []; + const idsByObject = new Map>(); + for (const row of grants) { + const id = rowId(row); + const object = nonEmpty(row[SUBJECT_OBJECT_FIELD]); + const recordId = nonEmpty(row[SUBJECT_ID_FIELD]); + if (!id || !object || !recordId) { + residue.unaddressable += 1; + if (id) residualRows.push({ id, reason: 'unaddressable', subject: null }); + continue; + } + addressable.push({ id, object, recordId }); + let ids = idsByObject.get(object); + if (!ids) idsByObject.set(object, (ids = new Set())); + ids.add(recordId); + } + + // ── One subject read per object, projected to ITS wall column ─────────── + // `undefined` in `subjectsByObject` = the read failed for the whole object; + // an absent map entry = the object was never read (no column to read by). + const subjectsByObject = new Map> | undefined>(); + for (const [object, ids] of idsByObject) { + const column = wallColumnOf(object); + if (!column) continue; + try { + subjectsByObject.set(object, await readSubjects(engine, object, column, [...ids], context)); + } catch (err) { + subjectsByObject.set(object, undefined); + notes.push( + `subject read on '${object}' failed — ${String((err as Error)?.message ?? err)}. ` + + `Its grant rows were left in place (could not ask is not gone).`, + ); + } + } + + // ── Decide each grant ─────────────────────────────────────────────────── + for (const { id, object, recordId } of addressable) { + const column = wallColumnOf(object); + const leave = (reason: keyof SysRecordShareBackfillResidue, organization: string | null = null) => { + residue[reason] += 1; + residualRows.push({ id, reason, subject: { object, id: recordId, organization } }); + }; + if (column === undefined) { leave('subjectObjectUnknown'); continue; } + if (column === null) { leave('subjectNotOrganizationScoped'); continue; } + if (!subjectsByObject.has(object)) { leave('subjectReadFailed'); continue; } + const subjects = subjectsByObject.get(object); + if (subjects === undefined) { leave('subjectReadFailed'); continue; } + const record = subjects.get(recordId); + if (!record) { leave('recordNotFound'); continue; } + const organization = nonEmpty(record[column]); + if (!organization) { leave('recordHasNoOrganization'); continue; } + base.planned += 1; + rows.push({ + id, + organizationField, + organization, + subject: { object, id: recordId, organization }, + }); + } + + return finish(base, options.logger); +} + +/** Seal a report's totals and emit the two log lines the ruling asks for. */ +function finish( + report: Omit, + logger: SysRecordShareBackfillLogger | undefined, +): SysRecordShareBackfillReport { + const sealed = { ...report, totals: totalsOf(report) }; + if (sealed.residue.recordNotFound > 0) { + // ⭐ The orphan count, logged — the half of the ruling's "leave NULL with a + // logged count" that the report alone cannot deliver to an operator's log. + logger?.warn?.( + `[sharing] sys_record_share organization backfill: ${sealed.residue.recordNotFound} grant row(s) ` + + 'reference a record that no longer exists — left NULL, not deleted; the #5103 orphan sweep ' + + '(`sweepOrphanedRecordShares`, kernel:bootstrapped) reclaims them on the next boot', + { orphans: sealed.residue.recordNotFound, dryRun: sealed.dryRun }, + ); + } + logger?.info?.( + `[sharing] sys_record_share organization backfill ${sealed.dryRun ? 'DRY RUN' : 'APPLIED'}`, + sealed.totals, + ); + return sealed; +} + +// --------------------------------------------------------------------------- +// Apply +// --------------------------------------------------------------------------- + +/** + * Write the plan. Each planned row gets ONE update carrying its id and its + * resolved organization column — nothing else on the row is touched, which is + * what makes the undo expressible as "write NULL back to these ids". + * + * 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. + * + * 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 + * sweep is safe here precisely because the next run picks up exactly what is + * still unstamped. + * + * ⛔ Takes a plan rather than building one, so the rows written are the rows a + * human read in the dry run — not a fresh scan that may have moved. + */ +export async function applySysRecordShareOrganizationBackfill( + engine: SysRecordShareBackfillEngine, + plan: SysRecordShareBackfillReport, + options: SysRecordShareBackfillOptions = {}, +): Promise { + const failures: SysRecordShareBackfillReport['failures'] = []; + let written = 0; + for (const row of plan.rows) { + try { + await engine.update( + SYS_RECORD_SHARE_BACKFILL_OBJECT, + { id: row.id, [row.organizationField]: row.organization }, + { context: { ...SYSTEM_CONTEXT, tenantId: row.organization } }, + ); + written += 1; + } catch (err) { + failures.push({ id: row.id, error: String((err as Error)?.message ?? err) }); + } + } + const applied: Omit = { + ...plan, + dryRun: false, + written, + failures, + }; + return finish(applied, options.logger); +} + +/** + * Plan, then (unless `dryRun`) write — the whole sweep in one call. + * + * Idempotent by construction rather than by a guard: the plan is built from + * `WHERE IS NULL`, and every write fills that column, so + * a second call over an unchanged database plans nothing and writes nothing. + */ +export async function runSysRecordShareOrganizationBackfill( + engine: SysRecordShareBackfillEngine, + options: SysRecordShareBackfillOptions = {}, +): Promise { + const plan = await planSysRecordShareOrganizationBackfill(engine, options); + if (options.dryRun !== false) return plan; + return applySysRecordShareOrganizationBackfill(engine, plan, options); +} + +// --------------------------------------------------------------------------- +// Report rendering +// --------------------------------------------------------------------------- + +/** Human-readable label per residue bucket — the reason, stated as a fact. */ +const RESIDUE_LABELS: Record = { + unaddressable: 'row carries no usable id or no (object_name, record_id) pair', + subjectObjectUnknown: "the record's object has no schema this engine can read — wall column unknowable", + subjectNotOrganizationScoped: "the record's object has no organization column at all", + subjectReadFailed: 'the record could not be READ (could not ask is not gone)', + recordNotFound: 'ORPHAN — the record no longer exists (left NULL; the #5103 boot sweep deletes these)', + recordHasNoOrganization: 'the record exists, is organization-scoped, and carries no organization itself', +}; + +/** + * Render a report as the operator-facing text. + * + * ⭐ The residual-NULL total and the orphan count are printed for a dry run as + * well as an applied one, and the residue is broken out by reason: those rows + * stay invisible to every tenant-scoped reader on a walled deployment, and the + * ruling makes that residue the maintainer's to see rather than something the + * sweep may quietly accept. + */ +export function formatSysRecordShareOrganizationBackfillReport( + report: SysRecordShareBackfillReport, +): string { + const lines: string[] = []; + lines.push( + report.dryRun + ? 'sys_record_share organization backfill — DRY RUN (nothing written)' + : 'sys_record_share organization backfill — APPLIED', + ); + lines.push('='.repeat(66)); + lines.push(`organization column : ${report.organizationField ?? '(none resolved)'}`); + lines.push(`scanned (unstamped) : ${report.scanned}`); + lines.push(`${report.dryRun ? 'would write' : 'written '} : ${report.dryRun ? report.planned : report.written}`); + for (const row of report.rows) { + lines.push( + ` ${row.id} -> ${row.organizationField}=${row.organization} ` + + `(from ${row.subject.object}/${row.subject.id})`, + ); + } + for (const failure of report.failures) { + lines.push(` ✗ ${failure.id} NOT written — ${failure.error}`); + } + + lines.push(''); + lines.push('-'.repeat(66)); + lines.push(`RESIDUAL NULL (still invisible under a wall) : ${report.totals.residualNull}`); + lines.push(`ORPHANS (record gone; left NULL, counted) : ${report.totals.orphans}`); + for (const key of Object.keys(report.residue) as Array) { + lines.push(` ${key.padEnd(30)} ${String(report.residue[key]).padStart(6)} — ${RESIDUE_LABELS[key]}`); + } + for (const row of report.residualRows) { + const subject = row.subject ? ` (${row.subject.object}/${row.subject.id})` : ''; + lines.push(` ${row.id} stays NULL — ${row.reason}${subject}`); + } + for (const note of report.notes) lines.push(` ⚠️ ${note}`); + + lines.push(''); + lines.push('-'.repeat(66)); + lines.push( + `TOTAL scanned=${report.totals.scanned} ` + + `${report.dryRun ? 'would-write' : 'written'}=${report.dryRun ? report.totals.planned : report.totals.written} ` + + `residual-null=${report.totals.residualNull} orphans=${report.totals.orphans}`, + ); + return lines.join('\n'); +} diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts index 80028917ab..42a94b415a 100644 --- a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts @@ -1012,6 +1012,16 @@ export class SharingRuleService implements ISharingRuleService { * different axes (`ObjectQLEngine.buildDriverOptions` reads them separately): * the evaluator must still see rows no individual recipient could, it must * just stop seeing rows the RULE has no business in. + * + * ## [#14484] The same context is what the grant is WRITTEN under + * + * `reconcile` / `reconcileForRecord` hand this context to + * `SharingService.grant`, which stamps `sys_record_share.organization_id` + * from it (ruled 2026-09-02: a rule-materialised grant carries the rule's + * organization). One context for the sweep and for the write is what keeps + * the two agreeing — the grant lands in the organization whose records the + * rule was allowed to sweep. A platform-global rule threads none, and its + * grants belong where each record does; `grant` derives that from the record. */ private criteriaContext(rule: SharingRuleRow): ExecutionContext { const orgId = rule.organization_id; @@ -1264,7 +1274,7 @@ export class SharingRuleService implements ISharingRuleService { sourceId: rule.id, reason: `rule:${rule.name}`, } as any, - SYSTEM_CTX, + this.criteriaContext(rule), ); updated += 1; } @@ -1281,7 +1291,7 @@ export class SharingRuleService implements ISharingRuleService { sourceId: rule.id, reason: `rule:${rule.name}`, } as any, - SYSTEM_CTX, + this.criteriaContext(rule), ); created += 1; } @@ -1337,7 +1347,7 @@ export class SharingRuleService implements ISharingRuleService { sourceId: rule.id, reason: `rule:${rule.name}`, } as any, - SYSTEM_CTX, + this.criteriaContext(rule), ); updated += 1; } @@ -1354,7 +1364,7 @@ export class SharingRuleService implements ISharingRuleService { sourceId: rule.id, reason: `rule:${rule.name}`, } as any, - SYSTEM_CTX, + this.criteriaContext(rule), ); created += 1; } diff --git a/packages/plugins/plugin-sharing/src/sharing-service.ts b/packages/plugins/plugin-sharing/src/sharing-service.ts index 888f774853..3a708d6dec 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.ts @@ -21,6 +21,10 @@ import { // implementations with a narrower shape is what forced this file to cast its // way out of its own contract to read fields the caller had already supplied. import type { ExecutionContext } from '@objectstack/spec/kernel'; +// [#14484] The engine's own answer to "which column is this object walled +// by?" — the twin of `SqlDriver.computeTenantField`, so the organization a grant +// is stamped from is read off the SAME column the wall scopes the record by. +import { resolveTenantFieldName } from '@objectstack/objectql'; import { WRITE_ACCESS_LEVELS, normalizeAccessLevel } from './access-level.js'; import { hasPhantomOwnerAnchor } from './federated-phantom-anchors.js'; import { @@ -1205,11 +1209,20 @@ export class SharingService implements ISharingService { // best-effort — the boot backfill, the object-wide re-grant and the // bu-tree re-grant queue log and continue, and the write hooks catch so a // user's insert/update is never failed by it. + // [#14484] The organization this grant belongs to, resolved here so BOTH + // halves of the upsert carry it. A rule-materialised grant carries the + // rule's organization (the evaluator threads it — see + // `SharingRuleService.criteriaContext`); a direct grant carries the shared + // record's. The resolution runs after the authorization pre-flight so a + // refused caller never pays the record read. + let organizationId: string | null; if (context?.isSystem) { this.assertNotInertGrant(input.object); + organizationId = await this.resolveSystemGrantOrganization(input, context); } else { this.assertSharingEnforced(input.object); await this.assertCanManageShares(input.object, input.recordId, context); + organizationId = await this.resolveDirectGrantOrganization(input, context); } // Upsert: if a row with same (object, record, recipient, source) exists, @@ -1233,19 +1246,41 @@ export class SharingService implements ISharingService { const row: any = existing[0]; const patch: any = { id: row.id, + // [#14484] The update half stamps too: a row written before the writer + // was repaired carries NULL, and the next grant that touches it is the + // cheapest repair there is. A resolution of `null` leaves the stored + // value alone rather than clearing one the backfill already wrote. + ...(organizationId ? { organization_id: organizationId } : {}), access_level: accessLevel, source, source_id: input.sourceId ?? row.source_id ?? null, reason: input.reason ?? row.reason ?? null, updated_at: now, }; - await this.engine.update('sys_record_share', patch, { context: SYSTEM_CTX }); + // [#14484] The organization rides the write context as well as the row — + // `{ isSystem, tenantId }` is the shape #8844's refusal prescribes for a + // system write, the same chokepoint a session write goes through + // (`ObjectQLEngine.buildDriverOptions` → `DriverOptions.tenantId`), and + // what satisfies the driver's tenant audit. On this verb the driver's + // scope keeps a NULL row in reach (`organization_id = ? OR IS NULL`) and, + // exactly as on `sys_upload_session`, a row stamped with a DIFFERENT + // organization stays out of it — the wall, not a defect. + await this.engine.update('sys_record_share', patch, { + context: { ...SYSTEM_CTX, tenantId: organizationId ?? undefined }, + }); return { ...row, ...patch } as RecordShare; } const id = makeShareId(); const row: any = { id, + // [#14484] Carried on the row literal itself, explicitly `null` when + // nothing resolved: `sys_record_share` is tenant-scoped in the #13491 + // ledger, so an organization-less system insert is the engine's to + // decide — derived on a `single` install, REFUSED loudly on a walled one + // (`ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED`, #8844) — never a silent + // NULL row on a table a tenant read will one day scope. + organization_id: organizationId, object_name: input.object, record_id: input.recordId, recipient_type: recipientType, @@ -1258,10 +1293,106 @@ export class SharingService implements ISharingService { created_at: now, updated_at: now, }; - await this.engine.insert('sys_record_share', row, { context: SYSTEM_CTX }); + // [#14484] Same write context as the update half — see the note there. + await this.engine.insert('sys_record_share', row, { + context: { ...SYSTEM_CTX, tenantId: organizationId ?? undefined }, + }); return row as RecordShare; } + /** + * [#14484] The organization a SYSTEM caller's grant belongs to. + * + * First the organization the caller THREADS: `SharingRuleService.reconcile` + * / `reconcileForRecord` pass the rule's own `criteriaContext`, so a + * rule-materialised grant carries the rule's organization — the 2026-09-02 + * ruling's first pin, and the same organization the rule's criteria sweep + * ran under (#10119), which is what makes the grant and the sweep agree. + * A platform-global rule (`organization_id = null`, #7795) carries none and + * sweeps every organization; its grant then belongs where the record does. + * + * `tenantId` is read through {@link activeOrganizationId} — the ONE field + * every transport and every system writer puts the organization on; the + * #8844 refusal prescribes exactly `{ isSystem: true, tenantId }`. + */ + private async resolveSystemGrantOrganization( + input: GrantShareInput, + context: ExecutionContext, + ): Promise { + return activeOrganizationId(context) + ?? (await this.recordOrganization(input.object, input.recordId)); + } + + /** + * [#14484] The organization a DIRECT grant belongs to: the organization of + * the record being shared (the ruling's second pin), read from the record + * itself — never the caller's active organization first, which under a + * `single` posture holding several organizations may not be the record's. + * + * The acting session's organization is the FALLBACK for a record that + * carries none — an object with no tenant column, or an organization-less + * row — the `sys_approval_request` writer's ruled shape (subject first, the + * acting context second). It is a fact of the write, not a guess: a + * principal is sharing a record from inside an organization. When neither + * exists the grant carries `null` and the engine's #8844 rule decides. + */ + private async resolveDirectGrantOrganization( + input: GrantShareInput, + context: ExecutionContext, + ): Promise { + return (await this.recordOrganization(input.object, input.recordId)) + ?? activeOrganizationId(context); + } + + /** + * [#14484] The organization `(object, recordId)` is walled by, read off the + * column the object is actually walled by ({@link resolveTenantFieldName}: + * ADR-0066 opt-out → declared `tenancy.tenantField` → injected + * `organization_id`), under the system context so field-level masking cannot + * hide the column from the decision — the same reading + * {@link canManageShares} takes of the owner column. + * + * `null` for an object with no tenant column, a record that is gone, an + * organization-less row, and a read that failed. The last is logged: a read + * that did not happen must not pass for a record with no organization, and + * the write it feeds still ends in the engine's ruled derive-or-refuse rather + * than in a silently stamped guess. + */ + private async recordOrganization(object: string, recordId: string): Promise { + const tenantField = this.tenantFieldOf(object); + if (!tenantField) return null; + try { + const rows = await this.engine.find(object, { + where: { id: recordId }, + fields: ['id', tenantField], + limit: 1, + context: SYSTEM_CTX, + }); + const row: any = Array.isArray(rows) ? rows[0] : undefined; + const value = row?.[tenantField]; + return typeof value === 'string' && value.trim() !== '' ? value : null; + } catch (err: any) { + this.logger?.warn?.( + '[sharing] could not read the shared record\'s organization — the grant carries none from it, ' + + 'and the engine\'s system-write organization rule decides the row (#14484)', + { object, recordId, error: err?.message }, + ); + return null; + } + } + + /** [#14484] The tenant column of `object`, or `null` when it has none / the engine cannot say. */ + private tenantFieldOf(object: string): string | null { + if (typeof this.engine.getSchema !== 'function') return null; + let schema: unknown; + try { + schema = this.engine.getSchema(object); + } catch { + return null; + } + return schema ? resolveTenantFieldName(schema) : null; + } + /** * Delete a share row by id. * From 56b51c514e123204f3a1a155ff1f80e83d4b0523 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 19:42:17 +0000 Subject: [PATCH 02/10] feat(sharing): stamp organization_id on every sys_record_share write, backfill legacy rows, admit the object to the tenancy ledger Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- .changeset/record-share-organization-stamp.md | 16 + .../src/record-share-tenant-wall.test.ts | 226 +++++++++++ ...ill-sys-record-share-organizations.test.ts | 381 ++++++++++++++++++ ...backfill-sys-record-share-organizations.ts | 17 +- .../record-share-organization-stamp.test.ts | 336 +++++++++++++++ .../src/rule-criteria-org-scope.test.ts | 5 + .../src/sharing-service.test.ts | 139 +++++++ scripts/engine-double-contract.pinned.json | 10 + 8 files changed, 1125 insertions(+), 5 deletions(-) create mode 100644 .changeset/record-share-organization-stamp.md create mode 100644 packages/plugins/plugin-security/src/record-share-tenant-wall.test.ts create mode 100644 packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.test.ts create mode 100644 packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts diff --git a/.changeset/record-share-organization-stamp.md b/.changeset/record-share-organization-stamp.md new file mode 100644 index 0000000000..37497743df --- /dev/null +++ b/.changeset/record-share-organization-stamp.md @@ -0,0 +1,16 @@ +--- +"@objectstack/plugin-sharing": minor +"@objectstack/objectql": patch +--- + +`sys_record_share` is tenant-scoped: every grant row now carries `organization_id`, and the rows written before it can be backfilled from the record they grant access to (#14484). + +Every `sys_record_share` row on every deployment was written with `organization_id = NULL`: `SharingService.grant` wrote under a bare system context and the row literal never carried the column, so neither the driver's `injectTenantOnInsert` nor the engine's system-write organization rule had anything to stamp from. Reads agreed with writes — the service's own reads are bare-context too — so nothing was visibly broken; what the NULL cost was the cliff: the first tenant-facing read of the table inherits `plugin-security`'s Layer 0, whose strict `organization_id = :tenant` AND-composes over the driver's NULL-tolerant arm and wins, and every existing grant silently disappears — not refused, simply "this person was never granted access". Maintainer ruling 2026-09-02 (decision batch #11 item 3, A adopted — 「#13564 转维护者处理；其他同意」): tenant-scoped, writer-repaired, existing rows backfilled from the record they reference. The per-table order the `sys_file` precedent requires; it covers `sys_record_share` and no other table. + +**Writer.** `SharingService.grant` stamps `organization_id` on both halves of its upsert. A rule-materialised grant carries the granting RULE's organization — `SharingRuleService.reconcile` / `reconcileForRecord` now hand `grant` the rule's own `criteriaContext` (`{ isSystem, tenantId: rule.organization_id }`), the same context the rule's criteria sweep ran under, so the grant lands in the organization whose records the rule was allowed to sweep. A direct grant carries the shared RECORD's organization, read off the column its object is walled by (`resolveTenantFieldName`: ADR-0066 opt-out → declared `tenancy.tenantField` → injected `organization_id`), with the acting session's organization as the fallback for a record that carries none. The organization rides the write context as `tenantId` as well as the row — the `{ isSystem, tenantId }` shape the #8844 refusal prescribes — so the driver's tenant audit is satisfied and the update half lands through the driver's scope (`organization_id = ? OR IS NULL`, which keeps a pre-repair NULL row in reach). Nothing resolvable ⇒ an explicit `null` on the row, for the engine's ruled rule to decide (below). The service's eleven bare-context READS are unchanged by this change; whether they become tenant-scoped is #13564's question. + +**Ledger (`@objectstack/objectql`).** `sys_record_share` leaves `unclassified` in the #13491 per-object tenancy ledger as `tenant-scoped`, with the ruling as the cited fact. Consequence, per the ledger's own admission semantics: an organization-less SYSTEM insert on `sys_record_share` is now derived on a `single` install with exactly one organization and REFUSED loudly on a walled one (`ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED`, status 500) — and the engine no longer auto-mutes the driver's tenant-audit warning for elevated writes on it. The only writer in this repository is repaired in the same change, so no shipped path meets that refusal; a third-party writer that inserts `sys_record_share` under a bare system context on a walled install will, and the refusal message says how to carry the organization. + +**Ops: the backfill — dry run first, and by default.** `packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.ts` scans only rows whose organization column is unset, re-reads each row's record (`object_name` + `record_id`) at repair time, and stamps the row with the record's own organization off the column that object is walled by. `planSysRecordShareOrganizationBackfill(engine)` reads only and returns a report naming every row it would touch; `runSysRecordShareOrganizationBackfill(engine, { dryRun: false })` writes. Nothing runs at boot and nothing is scheduled: this is an operator-invoked module, run once against an affected install, the posture of both precedents. **Orphans — grant rows whose record no longer exists — are left NULL, counted (`totals.orphans`) and logged, never deleted here:** the "record gone ⇒ the row cannot describe any access" invariant is already owned by the `kernel:bootstrapped` orphan sweep (`sweepOrphanedRecordShares`, #5103), which reclaims exactly that population on the next boot; a second deleter would be the fork `record-orphan-cleanup.ts` exists to prevent. Every other row that cannot be derived — an object with no organization column, a record that carries none, a record whose read failed — stays NULL and is reported by reason, for a dry run too. Idempotent by construction: every scan is `WHERE IS NULL` and every write fills that column, so the test suite runs the sweep twice and pins the second run at zero writes. + +Publishes no runtime code for the backfill: the module is not exported from the package index and not bundled into `dist` (`tsup` builds `src/index.ts`). It is graded rather than skipped because the release notes are where an operator of an affected install learns the repair exists, what it will and will not touch, and that the dry run comes first. diff --git a/packages/plugins/plugin-security/src/record-share-tenant-wall.test.ts b/packages/plugins/plugin-security/src/record-share-tenant-wall.test.ts new file mode 100644 index 0000000000..8aa478da0c --- /dev/null +++ b/packages/plugins/plugin-security/src/record-share-tenant-wall.test.ts @@ -0,0 +1,226 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14484] A tenant-scoped read of `sys_record_share` under plugin-security's + * Layer 0 returns the same grants the bare-context reads return for that + * organization — the cliff the card named, closed and pinned. + * + * ## Why this file lives in plugin-SECURITY + * + * The writer under test is `@objectstack/plugin-sharing`'s `SharingService` + * (its `grant` now stamps `organization_id` on every `sys_record_share` + * insert and update, ruled 2026-09-02), but the VERDICT that made the cliff a + * cliff is computed here: `computeTenantLayer0Filter` composes a STRICT + * `organization_id = ` over every tenant read, which wins over the + * driver's NULL-tolerant arm — so an organization-less grant is not refused, + * it is simply absent, indistinguishable from "never granted". Proving the + * repair therefore needs both packages in one process, and this is the one + * that owns the wall — plugin-security already depends on plugin-sharing for + * the same reason (`share-link-tenant-wall.test.ts`), never the other way. + * + * ## What is real here and what is a double + * + * REAL: `SharingService` (the production writer, its authorization pre-flight + * included) and the tenant wall — `computeTenantLayer0Filter` is called with + * the caller's context exactly as `security-plugin.ts` calls it on a read. + * + * DOUBLE: storage. The engine below is an in-memory table set that applies the + * wall the same way the security middleware does — AND-composed first, on a + * non-system context — so `RLS_DENY_FILTER` denies by being an unmatchable + * predicate rather than by a special case, which is how it denies in + * production. + * + * The backfill half of the repair (legacy rows) is pinned beside the writer in + * plugin-sharing (`backfill-sys-record-share-organizations.test.ts`, which + * composes the same strict wall shape); the control case below shows the + * cliff those rows fall off, for the reader who wants to see it. + */ + +import { describe, it, expect } from 'vitest'; +// The producer's OWN dispatch predicate for the double's `update`, from +// `@objectstack/metadata-core` (where it lives since #5619) — this package +// does not depend on `@objectstack/objectql`. +import { assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import type { TenancyPosture } from '@objectstack/spec/security'; +import { SharingService } from '@objectstack/plugin-sharing'; +import { computeTenantLayer0Filter } from './tenant-layer.js'; + +const OBJECT = 'crm_deal'; +const SHARE = 'sys_record_share'; +const ORG_A = 'org_plant_a'; +const ORG_B = 'org_plant_b'; + +const SCHEMAS: Record = { + [OBJECT]: { + name: OBJECT, + sharingModel: 'private', + fields: { id: {}, name: {}, owner_id: {}, organization_id: {} }, + }, + [SHARE]: { + name: SHARE, + isSystem: true, + fields: { id: {}, object_name: {}, record_id: {}, recipient_type: {}, recipient_id: {}, access_level: {}, source: {}, organization_id: {} }, + }, +}; + +/** Objects that carry `organization_id` — the wall's "is this a tenant object?" input. */ +const TENANT_OBJECTS = new Set(Object.keys(SCHEMAS)); + +function matches(row: any, where: Record): boolean { + return Object.entries(where).every(([k, v]) => { + if (k === '$and') return (v as any[]).every((w) => matches(row, w)); + if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); + if (v === null) return row[k] == null; + if (v && typeof v === 'object' && '$in' in v) return (v as any).$in.includes(row[k]); + return row[k] === v; + }); +} + +/** + * An engine that enforces Layer 0 exactly as the security middleware does: the + * REAL `computeTenantLayer0Filter`, fed the caller's context, AND-composed onto + * the query's own predicate. A system context bypasses it, as it does in + * production. + */ +function makeEngine(tables: Record, posture: TenancyPosture) { + return { + _tables: tables, + getSchema(object: string) { return SCHEMAS[object]; }, + async find(object: string, opts: any) { + const ctx = opts?.context ?? {}; + let rows = tables[object] ?? []; + if (!ctx.isSystem && TENANT_OBJECTS.has(object)) { + const layer0 = computeTenantLayer0Filter({ + tenancyPosture: posture, + organizationId: ctx.tenantId, + accessibleOrgIds: ctx.accessible_org_ids, + objectHasOrgIdField: true, + tenancyDisabled: false, + posturePermitsCrossTenant: false, + isPlatformAdmin: false, + }); + if (layer0) rows = rows.filter((r) => matches(r, layer0)); + } + rows = rows.filter((r) => matches(r, opts?.where ?? {})); + return rows.slice(0, typeof opts?.limit === 'number' ? opts.limit : rows.length).map((r) => ({ ...r })); + }, + async insert(object: string, row: any) { + (tables[object] ??= []).push({ ...row }); + return row; + }, + async update(object: string, data: any, options?: any) { + const dispatch = assertEngineUpdateDispatch(data, options); + const rows = tables[object] ?? []; + if (dispatch.kind === 'by-id') { + const i = rows.findIndex((r) => r.id === dispatch.id); + if (i >= 0) rows[i] = { ...rows[i], ...data }; + return data; + } + const matched = rows.filter((r) => matches(r, options?.where ?? {})); + for (const r of matched) Object.assign(r, data); + return matched.length; + }, + }; +} + +function boot(posture: TenancyPosture = 'isolated') { + const tables: Record = { + [OBJECT]: [ + { id: 'deal_a1', name: 'Plant A deal', owner_id: 'u_a', organization_id: ORG_A }, + { id: 'deal_a2', name: 'Plant A other', owner_id: 'u_a', organization_id: ORG_A }, + { id: 'deal_b1', name: 'Plant B deal', owner_id: 'u_b', organization_id: ORG_B }, + ], + [SHARE]: [], + }; + const engine = makeEngine(tables, posture); + const sharing = new SharingService({ engine: engine as never }); + const ids = (rows: unknown[]) => (rows as Array<{ id: string }>).map((r) => r.id).sort(); + /** The bare-context read the service itself performs (`listShares` under SYSTEM_CTX). */ + const bare = async () => ids(await engine.find(SHARE, { where: { object_name: OBJECT }, context: { isSystem: true } })); + /** A tenant-scoped read of the table, under plugin-security's Layer 0. */ + const tenant = async (org: string) => + ids(await engine.find(SHARE, { where: { object_name: OBJECT }, context: { userId: 'reader', tenantId: org } })); + const bareFor = async (org: string) => + ids((await engine.find(SHARE, { where: { object_name: OBJECT }, context: { isSystem: true } })) + .filter((r: any) => r.organization_id === org)); + return { engine, sharing, tables, bare, tenant, bareFor }; +} + +describe('[#14484] tenant-scoped reads of sys_record_share under Layer 0 agree with the bare reads', () => { + it("a DIRECT grant written by a plant-A owner is returned by plant A's tenant read, and equals the bare read for plant A", async () => { + const { sharing, tenant, bareFor, tables } = boot(); + const r = await sharing.grant( + { object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, + { userId: 'u_a', tenantId: ORG_A } as never, + ); + expect(tables[SHARE][0]).toMatchObject({ id: r.id, organization_id: ORG_A }); + expect(await tenant(ORG_A)).toEqual(await bareFor(ORG_A)); + expect(await tenant(ORG_A)).toEqual([r.id]); + // The wall is live, not bypassed: plant B sees none of plant A's grants. + expect(await tenant(ORG_B)).toEqual([]); + }); + + it("a RULE-materialised grant (system context carrying the rule's organization) agrees the same way", async () => { + const { sharing, tenant, bareFor } = boot(); + // What `SharingRuleService.reconcile` hands `grant`: the rule's own + // `criteriaContext` — elevation plus the rule's organization. + const r = await sharing.grant( + { object: OBJECT, recordId: 'deal_a2', recipientId: 'u_y', source: 'rule', sourceId: 'rule_a' }, + { isSystem: true, tenantId: ORG_A } as never, + ); + expect(await tenant(ORG_A)).toEqual(await bareFor(ORG_A)); + expect(await tenant(ORG_A)).toEqual([r.id]); + expect(await tenant(ORG_B)).toEqual([]); + }); + + it('with grants in BOTH organizations, every tenant read equals the bare read for its organization, and the two are disjoint', async () => { + const { sharing, bare, tenant, bareFor } = boot(); + const a = await sharing.grant({ object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, { userId: 'u_a', tenantId: ORG_A } as never); + const b = await sharing.grant({ object: OBJECT, recordId: 'deal_b1', recipientId: 'u_x' }, { userId: 'u_b', tenantId: ORG_B } as never); + const g = await sharing.grant( + { object: OBJECT, recordId: 'deal_a2', recipientId: 'u_z', source: 'rule', sourceId: 'rule_a' }, + { isSystem: true, tenantId: ORG_A } as never, + ); + expect(await bare()).toEqual([a.id, b.id, g.id].sort()); + expect(await tenant(ORG_A)).toEqual(await bareFor(ORG_A)); + expect(await tenant(ORG_B)).toEqual(await bareFor(ORG_B)); + expect(await tenant(ORG_A)).toEqual([a.id, g.id].sort()); + expect(await tenant(ORG_B)).toEqual([b.id]); + }); + + it('the `group` posture composes the union wall the same way — a member of both plants sees both', async () => { + const { sharing, engine } = boot('group'); + const a = await sharing.grant( + { object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x', source: 'rule', sourceId: 'rule_a' }, + { isSystem: true, tenantId: ORG_A } as never, + ); + const b = await sharing.grant( + { object: OBJECT, recordId: 'deal_b1', recipientId: 'u_x', source: 'rule', sourceId: 'rule_b' }, + { isSystem: true, tenantId: ORG_B } as never, + ); + const both = await engine.find(SHARE, { + where: { object_name: OBJECT }, + context: { userId: 'reader', tenantId: ORG_A, accessible_org_ids: [ORG_A, ORG_B] }, + }); + expect((both as any[]).map((r) => r.id).sort()).toEqual([a.id, b.id].sort()); + const onlyA = await engine.find(SHARE, { + where: { object_name: OBJECT }, + context: { userId: 'reader', tenantId: ORG_A, accessible_org_ids: [ORG_A] }, + }); + expect((onlyA as any[]).map((r) => r.id)).toEqual([a.id]); + }); + + it('CONTROL — the cliff itself: an organization-less legacy row is on the bare read and on NO tenant read', async () => { + // The pre-repair row shape, exactly as every deployment has it today. This + // is what the plugin-sharing backfill repairs; it is here so the reader can + // see the failure the pins above close. + const { tables, bare, tenant } = boot(); + tables[SHARE].push({ + id: 'shr_legacy', object_name: OBJECT, record_id: 'deal_a1', recipient_type: 'user', recipient_id: 'u_x', + access_level: 'read', source: 'manual', organization_id: null, + }); + expect(await bare()).toEqual(['shr_legacy']); + expect(await tenant(ORG_A)).toEqual([]); // not refused — absent, "never granted" + expect(await tenant(ORG_B)).toEqual([]); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.test.ts b/packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.test.ts new file mode 100644 index 0000000000..0c98027b0b --- /dev/null +++ b/packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.test.ts @@ -0,0 +1,381 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #14484 — the backfill half of "A: tenant-scoped, writer-repaired, backfilled". +// +// These pin the properties the maintainer ruling names, as behaviour rather +// than as prose: +// +// 1. DRY RUN FIRST — `{ dryRun: true }` (the default) writes nothing; +// 2. derived from the RECORD the grant is about, off the column its object +// is walled by; +// 3. ⭐ ORPHANS (record gone) stay NULL, are COUNTED and LOGGED, and are +// never deleted here — the #5103 boot sweep owns that invariant; +// 4. every other row that cannot be derived STAYS NULL and is REPORTED, by +// reason — never guessed; +// 5. IDEMPOTENT — a second run over the repaired database plans nothing, +// writes nothing, and re-reports the untouched residue; +// 6. ⭐ the CLIFF the card names is closed by the repair: under a strict +// organization wall (the shape `computeTenantLayer0Filter` composes for +// the `isolated` posture) a tenant-scoped read of the table returns the +// same grants the bare read returns for that organization — before the +// repair it returned NONE of the legacy rows. + +import { describe, it, expect, vi } from 'vitest'; +import { assertEngineUpdateDispatch } from '@objectstack/objectql'; +import { + SYS_RECORD_SHARE_BACKFILL_OBJECT, + applySysRecordShareOrganizationBackfill, + formatSysRecordShareOrganizationBackfillReport, + planSysRecordShareOrganizationBackfill, + runSysRecordShareOrganizationBackfill, + type SysRecordShareBackfillEngine, +} from './backfill-sys-record-share-organizations.js'; + +// --------------------------------------------------------------------------- +// Fake engine — schemas + rows, with `find` honouring the two predicate shapes +// the sweep issues (`{ col: null }` and `{ id: { $in: [...] } }`) plus plain +// equality, and — for the cliff cases — a STRICT organization wall applied to +// every non-system read, the way Layer 0 AND-composes it in production. +// --------------------------------------------------------------------------- + +interface FakeSchema { + fields: Record; + tenancy?: { enabled?: boolean; tenantField?: string }; +} + +/** + * ⛔ REFUSES what it does not implement, so the sweep cannot grow a predicate + * shape nothing here evaluates (see the `sys_file` precedent's double for the + * measured reason). + */ +function matchesWhere(record: Record, where: any): boolean { + if (!where) return true; + for (const [field, condition] of Object.entries(where)) { + if (field.startsWith('$')) { + throw new Error( + `fake engine: unsupported WHERE combinator '${field}' — this double implements only ` + + 'field equality, `{ field: null }` and `{ field: { $in: [...] } }`.', + ); + } + const value = record[field]; + if (condition === null) { + if (value !== null && value !== undefined && value !== '') return false; + } else if (condition && typeof condition === 'object') { + const operators = Object.keys(condition as Record); + if (operators.length !== 1 || operators[0] !== '$in') { + throw new Error(`fake engine: unsupported operator(s) [${operators.join(', ')}] on field '${field}'.`); + } + const accepted = (condition as { $in: unknown[] }).$in; + if (!accepted.map(String).includes(String(value))) return false; + } else if (String(value) !== String(condition)) { + return false; + } + } + return true; +} + +function createFakeEngine(init: { + schemas: Record; + rows: Record>>; + failUpdateFor?: Set; + failFindFor?: Set; + /** When set, every NON-system read is walled to `context.tenantId` by strict equality. */ + strictWall?: boolean; +}) { + const rows: Record>> = {}; + for (const [object, list] of Object.entries(init.rows)) rows[object] = list.map((r) => ({ ...r })); + const updates: Array<{ object: string; data: Record; context: unknown }> = []; + + const engine: SysRecordShareBackfillEngine & { + _rows: (object: string) => Array>; + _updates: typeof updates; + } = { + getSchema(object: string) { + const schema = init.schemas[object]; + if (!schema) throw new Error(`unknown object '${object}'`); + return schema; + }, + async find(object: string, options?: any) { + if (init.failFindFor?.has(object)) throw new Error(`simulated read failure on ${object}`); + const table = rows[object]; + if (!table) throw new Error(`object '${object}' is not mounted on this install`); + let out = table; + const ctx = options?.context ?? {}; + if (init.strictWall && !ctx.isSystem) { + // Layer 0's `isolated` arm: `{ organization_id: }`, + // AND-composed over everything else — the strict equality that wins + // over the driver's NULL-tolerant arm (`backfill-sys-file-organizations.ts`). + const org = ctx.tenantId; + out = out.filter((r) => r.organization_id === org); + } + out = out.filter((r) => matchesWhere(r, options?.where)); + if (options?.orderBy?.[0]?.field === 'id') { + out = [...out].sort((a, b) => String(a.id).localeCompare(String(b.id))); + } + const offset = typeof options?.offset === 'number' ? options.offset : 0; + const limit = typeof options?.limit === 'number' ? options.limit : out.length; + return out.slice(offset, offset + limit).map((r) => ({ ...r })); + }, + async update(object: string, data: any, options?: any) { + // The producer's own dispatch predicate, so this double can never accept + // an update shape the real `ObjectQL.update` refuses. + assertEngineUpdateDispatch(data, options); + if (init.failUpdateFor?.has(String(data?.id))) { + throw new Error(`simulated write refusal for ${data.id}`); + } + updates.push({ object, data: { ...data }, context: options?.context }); + const table = rows[object] ?? []; + const row = table.find((r) => String(r.id) === String(data.id)); + if (row) Object.assign(row, data); + return row ? { ...row } : null; + }, + _rows: (object: string) => (rows[object] ?? []).map((r) => ({ ...r })), + _updates: updates, + }; + return engine; +} + +/** An org-scoped business object: the injected `organization_id` column. */ +const orgScoped = (extra: Record = {}): FakeSchema => ({ + fields: { id: {}, name: {}, owner_id: {}, organization_id: {}, ...extra }, +}); + +const ORG_A = 'org_a'; +const ORG_B = 'org_b'; + +const baseSchemas: Record = { + sys_record_share: orgScoped({ object_name: {}, record_id: {}, recipient_id: {}, access_level: {}, source: {} }), + crm_deal: orgScoped(), + crm_case: orgScoped(), + // ADR-0066: a platform-global object — no wall column at all. + sys_setting: { fields: { id: {}, name: {}, organization_id: {} }, tenancy: { enabled: false } }, + // An object with no organization column injected. + ext_note: { fields: { id: {}, body: {} } }, +}; + +const grant = (id: string, object: string, record: string, extra: Record = {}) => ({ + id, object_name: object, record_id: record, recipient_type: 'user', recipient_id: 'u_x', + access_level: 'read', source: 'manual', organization_id: null, ...extra, +}); + +function seeded(extra: Partial[0]> = {}) { + return createFakeEngine({ + schemas: baseSchemas, + rows: { + crm_deal: [ + { id: 'deal_a', name: 'A', organization_id: ORG_A }, + { id: 'deal_b', name: 'B', organization_id: ORG_B }, + { id: 'deal_orgless', name: 'no org', organization_id: null }, + ], + crm_case: [{ id: 'case_a', name: 'A case', organization_id: ORG_A }], + sys_setting: [{ id: 'set_1', name: 'global', organization_id: ORG_A }], + ext_note: [{ id: 'note_1', body: 'x' }], + sys_record_share: [ + grant('shr_deal_a', 'crm_deal', 'deal_a'), + grant('shr_deal_b', 'crm_deal', 'deal_b'), + grant('shr_case_a', 'crm_case', 'case_a'), + grant('shr_orphan', 'crm_deal', 'deal_gone'), // ⭐ record gone + grant('shr_orgless', 'crm_deal', 'deal_orgless'), // record exists, no org + grant('shr_setting', 'sys_setting', 'set_1'), // object opted out of tenancy + grant('shr_note', 'ext_note', 'note_1'), // object with no org column + grant('shr_unknown', 'app_gone', 'r1'), // object with no schema + grant('shr_unaddressable', 'crm_deal', ''), // no record id + grant('shr_already', 'crm_deal', 'deal_a', { organization_id: ORG_A }), // already stamped: never scanned + ], + }, + ...extra, + }); +} + +const orgOf = (engine: ReturnType, id: string) => + engine._rows(SYS_RECORD_SHARE_BACKFILL_OBJECT).find((r) => r.id === id)?.organization_id ?? null; + +describe('[#14484] sys_record_share organization backfill — plan (dry run)', () => { + it('the default is a DRY RUN: it names every row it would write and writes none of them', async () => { + const engine = seeded(); + const report = await runSysRecordShareOrganizationBackfill(engine); + expect(report.dryRun).toBe(true); + expect(report.organizationField).toBe('organization_id'); + expect(report.scanned).toBe(9); // `shr_already` is already stamped and never matches + expect(report.planned).toBe(3); + expect(report.written).toBe(0); + expect(engine._updates).toHaveLength(0); + // In scan order — by id, so the pages partition the population. + expect(report.rows.map((r) => [r.id, r.organization, r.subject.object, r.subject.id])).toEqual([ + ['shr_case_a', ORG_A, 'crm_case', 'case_a'], + ['shr_deal_a', ORG_A, 'crm_deal', 'deal_a'], + ['shr_deal_b', ORG_B, 'crm_deal', 'deal_b'], + ]); + // ⭐ On a dry run EVERY scanned row is still NULL — the honest answer to + // "what stays invisible if I stop here?". + expect(report.totals).toEqual({ scanned: 9, planned: 3, written: 0, residualNull: 9, orphans: 1 }); + }); + + it('⭐ the residue is broken out by reason, and the orphan is counted AND logged — never deleted', async () => { + const warn = vi.fn(); + const info = vi.fn(); + const engine = seeded(); + const report = await planSysRecordShareOrganizationBackfill(engine, { logger: { warn, info } }); + expect(report.residue).toEqual({ + unaddressable: 1, + subjectObjectUnknown: 1, + subjectNotOrganizationScoped: 2, // sys_setting (opt-out) + ext_note (no column) + subjectReadFailed: 0, + recordNotFound: 1, + recordHasNoOrganization: 1, + }); + const reasons = Object.fromEntries(report.residualRows.map((r) => [r.id, r.reason])); + expect(reasons).toEqual({ + shr_orphan: 'recordNotFound', + shr_orgless: 'recordHasNoOrganization', + shr_setting: 'subjectNotOrganizationScoped', + shr_note: 'subjectNotOrganizationScoped', + shr_unknown: 'subjectObjectUnknown', + shr_unaddressable: 'unaddressable', + }); + // The logged count, on `warn`, naming the deleter that is NOT this module. + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0]![0])).toMatch(/1 grant row\(s\) reference a record that no longer exists/); + expect(String(warn.mock.calls[0]![0])).toMatch(/left NULL, not deleted/); + expect(warn.mock.calls[0]![1]).toMatchObject({ orphans: 1, dryRun: true }); + expect(info).toHaveBeenCalledTimes(1); + // The orphan row is still there. + expect(engine._rows(SYS_RECORD_SHARE_BACKFILL_OBJECT).some((r) => r.id === 'shr_orphan')).toBe(true); + }); + + it('a subject read that FAILS leaves that object\'s rows alone as "could not ask" — the others still plan', async () => { + const engine = seeded({ failFindFor: new Set(['crm_case']) }); + const report = await planSysRecordShareOrganizationBackfill(engine); + expect(report.residue.subjectReadFailed).toBe(1); + expect(report.residualRows.find((r) => r.id === 'shr_case_a')?.reason).toBe('subjectReadFailed'); + expect(report.rows.map((r) => r.id)).toEqual(['shr_deal_a', 'shr_deal_b']); + expect(report.notes.some((n) => /subject read on 'crm_case' failed/.test(n))).toBe(true); + }); + + it('no organization column on sys_record_share ⇒ nothing scanned, said out loud', async () => { + const engine = createFakeEngine({ + schemas: { ...baseSchemas, sys_record_share: { fields: { id: {}, object_name: {}, record_id: {} } } }, + rows: { sys_record_share: [grant('shr_1', 'crm_deal', 'deal_a')], crm_deal: [] }, + }); + const report = await planSysRecordShareOrganizationBackfill(engine); + expect(report.organizationField).toBeNull(); + expect(report.scanned).toBe(0); + expect(report.notes[0]).toMatch(/no organization column resolved/); + }); + + it('a wall column the subject declares by name is honoured, never a hard-coded `organization_id`', async () => { + const engine = createFakeEngine({ + schemas: { + ...baseSchemas, + hr_region: { fields: { id: {}, region_org: {}, organization_id: {} }, tenancy: { tenantField: 'region_org' } }, + }, + rows: { + hr_region: [{ id: 'reg_1', region_org: ORG_B, organization_id: ORG_A }], + sys_record_share: [grant('shr_reg', 'hr_region', 'reg_1')], + }, + }); + const report = await planSysRecordShareOrganizationBackfill(engine); + expect(report.rows.map((r) => [r.id, r.organization])).toEqual([['shr_reg', ORG_B]]); + }); +}); + +describe('[#14484] sys_record_share organization backfill — apply', () => { + it('writes ONE update per planned row, with the organization threaded on the write context, and nothing else', async () => { + const engine = seeded(); + const plan = await planSysRecordShareOrganizationBackfill(engine); + const applied = await applySysRecordShareOrganizationBackfill(engine, plan); + expect(applied.dryRun).toBe(false); + expect(applied.written).toBe(3); + expect(applied.failures).toEqual([]); + expect(engine._updates.map((u) => [u.object, u.data, (u.context as any)?.isSystem, (u.context as any)?.tenantId])).toEqual([ + ['sys_record_share', { id: 'shr_case_a', organization_id: ORG_A }, true, ORG_A], + ['sys_record_share', { id: 'shr_deal_a', organization_id: ORG_A }, true, ORG_A], + ['sys_record_share', { id: 'shr_deal_b', organization_id: ORG_B }, true, ORG_B], + ]); + expect(orgOf(engine, 'shr_deal_a')).toBe(ORG_A); + expect(orgOf(engine, 'shr_deal_b')).toBe(ORG_B); + expect(orgOf(engine, 'shr_case_a')).toBe(ORG_A); + // Residue untouched, orphan still present. + for (const id of ['shr_orphan', 'shr_orgless', 'shr_setting', 'shr_note', 'shr_unknown', 'shr_unaddressable']) { + expect(orgOf(engine, id), id).toBeNull(); + } + expect(applied.totals).toEqual({ scanned: 9, planned: 3, written: 3, residualNull: 6, orphans: 1 }); + }); + + it('a plan-then-apply run logs the orphan count ONCE — the apply half never re-says it', async () => { + const warn = vi.fn(); + const engine = seeded(); + const report = await runSysRecordShareOrganizationBackfill(engine, { dryRun: false, logger: { warn } }); + expect(report.totals.orphans).toBe(1); + expect(warn).toHaveBeenCalledTimes(1); + }); + + it('a row whose write throws is recorded, and the sweep continues', async () => { + const engine = seeded({ failUpdateFor: new Set(['shr_deal_b']) }); + const report = await runSysRecordShareOrganizationBackfill(engine, { dryRun: false }); + expect(report.written).toBe(2); + expect(report.failures).toEqual([{ id: 'shr_deal_b', error: 'simulated write refusal for shr_deal_b' }]); + expect(orgOf(engine, 'shr_deal_a')).toBe(ORG_A); + expect(orgOf(engine, 'shr_deal_b')).toBeNull(); + }); + + it('IDEMPOTENT: a second run plans nothing, writes nothing, and re-reports the residue', async () => { + const engine = seeded(); + await runSysRecordShareOrganizationBackfill(engine, { dryRun: false }); + const second = await runSysRecordShareOrganizationBackfill(engine, { dryRun: false }); + expect(second).toMatchObject({ scanned: 6, planned: 0, written: 0 }); + expect(second.totals.orphans).toBe(1); + expect(engine._updates).toHaveLength(3); + }); +}); + +describe('[#14484] ⭐ the cliff — a strict organization wall over the grant table', () => { + // A tenant-scoped read of `sys_record_share` under a strict wall, as Layer 0 + // composes it for the `isolated` posture: `organization_id = `. + const tenantRead = (engine: ReturnType, org: string) => + engine.find(SYS_RECORD_SHARE_BACKFILL_OBJECT, { where: { object_name: 'crm_deal' }, context: { userId: 'u', tenantId: org } }); + const bareRead = (engine: ReturnType) => + engine.find(SYS_RECORD_SHARE_BACKFILL_OBJECT, { where: { object_name: 'crm_deal' }, context: { isSystem: true } }); + const ids = (rows: unknown[]) => (rows as Array<{ id: string }>).map((r) => r.id).sort(); + + it('BEFORE the repair: the bare read returns the legacy grants, the tenant-scoped read returns NONE of them', async () => { + const engine = seeded({ strictWall: true }); + const bare = ids(await bareRead(engine)); + expect(bare).toContain('shr_deal_a'); + expect(bare).toContain('shr_deal_b'); + // The card's ③: every organization-less grant "silently disappears" — + // only the one row already stamped survives the wall. + expect(ids(await tenantRead(engine, ORG_A))).toEqual(['shr_already']); + expect(ids(await tenantRead(engine, ORG_B))).toEqual([]); + }); + + it('AFTER the repair: the tenant-scoped read equals the bare read filtered to that organization', async () => { + const engine = seeded({ strictWall: true }); + await runSysRecordShareOrganizationBackfill(engine, { dryRun: false }); + + const bare = (await bareRead(engine)) as Array<{ id: string; organization_id: string | null }>; + const bareForA = bare.filter((r) => r.organization_id === ORG_A).map((r) => r.id).sort(); + const bareForB = bare.filter((r) => r.organization_id === ORG_B).map((r) => r.id).sort(); + + expect(ids(await tenantRead(engine, ORG_A))).toEqual(bareForA); + expect(ids(await tenantRead(engine, ORG_B))).toEqual(bareForB); + expect(bareForA).toEqual(['shr_already', 'shr_deal_a']); + expect(bareForB).toEqual(['shr_deal_b']); + // …and the two organizations' reads are disjoint: the wall is live, not bypassed. + expect(bareForA.filter((id) => bareForB.includes(id))).toEqual([]); + }); +}); + +describe('[#14484] sys_record_share organization backfill — the rendered report', () => { + it('prints the residual-NULL total, the orphan count and the per-reason breakdown, for a dry run too', async () => { + const engine = seeded(); + const text = formatSysRecordShareOrganizationBackfillReport(await planSysRecordShareOrganizationBackfill(engine)); + expect(text).toMatch(/DRY RUN \(nothing written\)/); + expect(text).toMatch(/RESIDUAL NULL \(still invisible under a wall\) : 9/); + expect(text).toMatch(/ORPHANS \(record gone; left NULL, counted\) : 1/); + expect(text).toMatch(/recordNotFound\s+1\s+— ORPHAN/); + expect(text).toMatch(/shr_deal_a -> organization_id=org_a \(from crm_deal\/deal_a\)/); + expect(text).toMatch(/shr_orphan stays NULL — recordNotFound \(crm_deal\/deal_gone\)/); + expect(text).toMatch(/TOTAL scanned=9 would-write=3 residual-null=9 orphans=1/); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.ts b/packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.ts index 339969b620..9552c5ee1b 100644 --- a/packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.ts +++ b/packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.ts @@ -473,7 +473,7 @@ export async function planSysRecordShareOrganizationBackfill( + 'On a multi-tenant install this means the engine exposed no schema for the object; ' + 'on an install that opted the object out of system fields it is expected.', ); - return finish(base, options.logger); + return finish(base, options.logger, true); } const grants = await scanUnstampedGrants( @@ -545,16 +545,23 @@ export async function planSysRecordShareOrganizationBackfill( }); } - return finish(base, options.logger); + return finish(base, options.logger, true); } -/** Seal a report's totals and emit the two log lines the ruling asks for. */ +/** + * Seal a report's totals and emit the log lines the ruling asks for. + * + * The orphan line is emitted by the PLAN only: a run that plans and then + * applies (`runSysRecordShareOrganizationBackfill`) would otherwise say it + * twice for one population, and the apply half never changes that count. + */ function finish( report: Omit, logger: SysRecordShareBackfillLogger | undefined, + logOrphans: boolean, ): SysRecordShareBackfillReport { const sealed = { ...report, totals: totalsOf(report) }; - if (sealed.residue.recordNotFound > 0) { + if (logOrphans && sealed.residue.recordNotFound > 0) { // ⭐ The orphan count, logged — the half of the ruling's "leave NULL with a // logged count" that the report alone cannot deliver to an operator's log. logger?.warn?.( @@ -619,7 +626,7 @@ export async function applySysRecordShareOrganizationBackfill( written, failures, }; - return finish(applied, options.logger); + return finish(applied, options.logger, false); } /** diff --git a/packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts b/packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts new file mode 100644 index 0000000000..ac2ac00c39 --- /dev/null +++ b/packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts @@ -0,0 +1,336 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14484] Every `sys_record_share` row carries `organization_id` — on a REAL + * engine over a REAL driver, both write paths, plus the backfill and the + * engine rule the ledger flip switches on. + * + * ## Why a real driver + * + * The service-level pins (`sharing-service.test.ts`) prove what `grant` puts on + * the row. What they cannot prove is the chain a system write now crosses: + * `sys_record_share` is `tenant-scoped` in the #13491 ledger, so + * `Engine.resolveSystemInsertOrganization` (#8844) reads every organization- + * less system insert on it — deriving on a `single` install, REFUSING on a + * walled one — and `SqlDriver.applyTenantScope` decides which row the update + * half's `tenantId` lands on. A double proves none of that. So these cases run + * a real `SqlDriver` on better-sqlite3 `:memory:` behind a real `ObjectQL`, + * the way `rule-criteria-org-scope.test.ts` does for the sweep's scope. + * + * ## The two organizations a grant can carry, stated side by side + * + * Ruled 2026-09-02: a rule-materialised grant carries the RULE's organization, + * a direct grant the RECORD's. `deal_p1` — an organization-less record an + * org-A rule still matches through the driver's compatibility arm — is what + * separates the two: under the org-A rule its grant carries ORG_A (the rule's), + * under the platform-global rule it carries nothing (the record's). + * + * ## The P4 measurement, pinned + * + * Flipping the ledger row ALONE does not repair the writer: on a walled + * posture it turns every organization-less system insert into a loud + * `ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED`. The last block pins both halves — + * the refusal a bare insert now meets, and the repaired writer sailing through + * it because it carries the organization. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; + +import { SharingService } from './sharing-service.js'; +import { SharingRuleService } from './sharing-rule-service.js'; +import { runSysRecordShareOrganizationBackfill } from './backfill-sys-record-share-organizations.js'; + +const OBJECT = 'os14484_deal'; + +const DEAL_FIELDS: Record> = { + id: { type: 'text', name: 'id', label: 'Id', primary: true }, + stage: { type: 'text', name: 'stage', label: 'Stage' }, + owner_id: { type: 'text', name: 'owner_id', label: 'Owner' }, + organization_id: { type: 'text', name: 'organization_id', label: 'Org' }, +}; + +const SHARE_FIELDS: Record> = { + id: { type: 'text', name: 'id', label: 'Id', primary: true }, + organization_id: { type: 'text', name: 'organization_id', label: 'Org' }, + object_name: { type: 'text', name: 'object_name', label: 'Object' }, + record_id: { type: 'text', name: 'record_id', label: 'Record' }, + recipient_type: { type: 'text', name: 'recipient_type', label: 'Recipient type' }, + recipient_id: { type: 'text', name: 'recipient_id', label: 'Recipient' }, + access_level: { type: 'text', name: 'access_level', label: 'Access' }, + source: { type: 'text', name: 'source', label: 'Source' }, + source_id: { type: 'text', name: 'source_id', label: 'Source id' }, + reason: { type: 'text', name: 'reason', label: 'Reason' }, + granted_by: { type: 'text', name: 'granted_by', label: 'Grantor' }, + created_at: { type: 'text', name: 'created_at', label: 'Created' }, + updated_at: { type: 'text', name: 'updated_at', label: 'Updated' }, +}; + +const RULE_FIELDS: Record> = { + id: { type: 'text', name: 'id', label: 'Id', primary: true }, + organization_id: { type: 'text', name: 'organization_id', label: 'Org' }, + name: { type: 'text', name: 'name', label: 'Name' }, + label: { type: 'text', name: 'label', label: 'Label' }, + description: { type: 'text', name: 'description', label: 'Description' }, + object_name: { type: 'text', name: 'object_name', label: 'Object' }, + criteria_json: { type: 'text', name: 'criteria_json', label: 'Criteria' }, + recipient_type: { type: 'text', name: 'recipient_type', label: 'Recipient type' }, + recipient_id: { type: 'text', name: 'recipient_id', label: 'Recipient' }, + access_level: { type: 'text', name: 'access_level', label: 'Access' }, + active: { type: 'boolean', name: 'active', label: 'Active' }, + managed_by: { type: 'text', name: 'managed_by', label: 'Managed by' }, + customized: { type: 'boolean', name: 'customized', label: 'Customized' }, + created_at: { type: 'text', name: 'created_at', label: 'Created' }, + updated_at: { type: 'text', name: 'updated_at', label: 'Updated' }, +}; + +const ORG_A = 'org_a'; +const ORG_B = 'org_b'; + +const SYSTEM: ExecutionContext = { isSystem: true, positions: [], permissions: [] }; + +const ORG_A_ADMIN = { + tenantId: ORG_A, + positions: [], + permissions: [], + systemPermissions: ['manage_sharing'], +} as unknown as ExecutionContext; + +interface ShareRow { + id: string; + record_id: string; + organization_id: string | null; +} + +interface Booted { + driver: SqlDriver; + ql: ObjectQL; + sharing: SharingService; + rules: SharingRuleService; + /** Every `sys_record_share` row, read straight off the driver, keyed by record then id. */ + shares: () => Promise; +} + +const open: SqlDriver[] = []; + +/** + * The same five deals `rule-criteria-org-scope.test.ts` reasons about: + * + * deal_a1 ORG_A stage=won + * deal_a2 ORG_A stage=lost + * deal_b1 ORG_B stage=won + * deal_b2 ORG_B stage=won + * deal_p1 (null) stage=won <- organization-less; the discriminating record + */ +async function boot(posture?: 'single' | 'isolated' | 'group'): Promise { + const driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + open.push(driver); + + const ql = new ObjectQL(); + ql.registerDriver(driver as never, true); + await ql.init(); + if (posture) ql.setTenancyPostureProvider(() => posture); + ql.registerObject({ name: OBJECT, label: 'Deal', sharingModel: 'private', fields: DEAL_FIELDS } as never); + ql.registerObject({ name: 'sys_record_share', label: 'Record Share', isSystem: true, fields: SHARE_FIELDS } as never); + ql.registerObject({ name: 'sys_sharing_rule', label: 'Sharing Rule', isSystem: true, fields: RULE_FIELDS } as never); + await driver.initObjects([ + { name: OBJECT, fields: DEAL_FIELDS } as never, + { name: 'sys_record_share', fields: SHARE_FIELDS } as never, + { name: 'sys_sharing_rule', fields: RULE_FIELDS } as never, + ]); + + // Seeded through the driver: the fixture is the DATA at rest. + await driver.create(OBJECT, { id: 'deal_a1', stage: 'won', owner_id: 'u_a', organization_id: ORG_A } as never); + await driver.create(OBJECT, { id: 'deal_a2', stage: 'lost', owner_id: 'u_a', organization_id: ORG_A } as never); + await driver.create(OBJECT, { id: 'deal_b1', stage: 'won', owner_id: 'u_b', organization_id: ORG_B } as never); + await driver.create(OBJECT, { id: 'deal_b2', stage: 'won', owner_id: 'u_b', organization_id: ORG_B } as never); + await driver.create(OBJECT, { id: 'deal_p1', stage: 'won', owner_id: 'u_p' } as never); + + const sharing = new SharingService({ engine: ql as never }); + const rules = new SharingRuleService({ engine: ql as never, sharing }); + + return { + driver, + ql, + sharing, + rules, + shares: async () => { + const rows = await driver.find('sys_record_share', {} as never); + return rows + .map((r: any) => ({ id: String(r.id), record_id: String(r.record_id), organization_id: r.organization_id ?? null })) + .sort((a, b) => a.record_id.localeCompare(b.record_id) || a.id.localeCompare(b.id)); + }, + }; +} + +afterEach(async () => { + while (open.length) await open.pop()?.disconnect?.(); +}); + +const WON = { stage: 'won' }; +const byRecord = (rows: ShareRow[]) => Object.fromEntries(rows.map((r) => [r.record_id, r.organization_id])); + +describe("[#14484] a rule-materialised grant carries the RULE's organization", () => { + it("an ORG-STAMPED rule's grants all carry ORG_A — the organization-less record included", async () => { + const { rules, shares } = await boot(); + const rule = await rules.defineRule( + { name: 'os14484_org_a_won', label: 'ORG_A won', object: OBJECT, criteria: WON, recipientType: 'user', recipientId: 'u_a', accessLevel: 'read' } as never, + ORG_A_ADMIN, + ); + expect(rule.organization_id).toBe(ORG_A); + + await rules.evaluateRule(rule.id, ORG_A_ADMIN); + const rows = await shares(); + + // `deal_p1` is the discriminating row: the RECORD carries no organization, + // the RULE does, and the ruling says the grant is the rule's. + expect(byRecord(rows)).toEqual({ deal_a1: ORG_A, deal_p1: ORG_A }); + expect(rows.every((r) => r.organization_id === ORG_A)).toBe(true); + }); + + it("a PLATFORM-GLOBAL rule carries none, so each grant belongs where its RECORD does", async () => { + const { rules, shares } = await boot(); + const rule = await rules.defineRule( + { name: 'os14484_platform_won', label: 'Platform won', object: OBJECT, criteria: WON, recipientType: 'user', recipientId: 'u_plat', accessLevel: 'read' } as never, + SYSTEM, + ); + expect(rule.organization_id).toBeNull(); + + await rules.evaluateRule(rule.id, SYSTEM); + + // Record by record. `deal_p1` stays NULL: nothing to derive from, and on a + // `single` install with no organization yet the engine has nothing to + // stamp either (#8844 'no-organization-yet'). + expect(byRecord(await shares())).toEqual({ deal_a1: ORG_A, deal_b1: ORG_B, deal_b2: ORG_B, deal_p1: null }); + }); + + it('the per-record hook pass stamps the same way as the whole-rule pass', async () => { + const { rules, shares } = await boot(); + await rules.defineRule( + { name: 'os14484_org_a_won_hook', label: 'ORG_A won', object: OBJECT, criteria: WON, recipientType: 'user', recipientId: 'u_a', accessLevel: 'read' } as never, + ORG_A_ADMIN, + ); + await rules.evaluateAllForRecord(OBJECT, 'deal_a1', SYSTEM); + expect(byRecord(await shares())).toEqual({ deal_a1: ORG_A }); + }); +}); + +describe("[#14484] a direct grant carries the RECORD's organization", () => { + it("an owner sharing their record stamps the record's organization, whatever the caller carries", async () => { + const { sharing, shares } = await boot(); + // The caller threads NO organization at all — the only place ORG_A can + // come from is the record. + await sharing.grant({ object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, { userId: 'u_a' } as never); + expect(byRecord(await shares())).toEqual({ deal_a1: ORG_A }); + }); + + it("an organization-less record falls back to the acting session's organization", async () => { + const { sharing, shares } = await boot(); + // `deal_p1` is reachable to an ORG_B session through the driver's + // compatibility arm; the record has nothing to give, the session does. + await sharing.grant( + { object: OBJECT, recordId: 'deal_p1', recipientId: 'u_x' }, + { userId: 'u_p', tenantId: ORG_B } as never, + ); + expect(byRecord(await shares())).toEqual({ deal_p1: ORG_B }); + }); + + it('the update half stamps a pre-repair NULL row in place', async () => { + const { driver, sharing, shares } = await boot(); + await driver.create('sys_record_share', { + id: 'shr_legacy', object_name: OBJECT, record_id: 'deal_a1', recipient_type: 'user', recipient_id: 'u_x', + access_level: 'read', source: 'manual', created_at: '2026-01-01T00:00:00Z', + } as never); + expect(byRecord(await shares())).toEqual({ deal_a1: null }); + + const r = await sharing.grant( + { object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x', accessLevel: 'edit' }, + { userId: 'u_a' } as never, + ); + expect(r.id).toBe('shr_legacy'); + const rows = await shares(); + expect(rows).toHaveLength(1); + expect(rows[0]!.organization_id).toBe(ORG_A); + }); +}); + +describe('[#14484] the backfill on a real driver — derived from the record, orphans left NULL and counted', () => { + it('stamps every row that references a live organization-scoped record, twice is a no-op', async () => { + const { driver, ql, shares } = await boot(); + const legacy = (id: string, record: string) => ({ + id, object_name: OBJECT, record_id: record, recipient_type: 'user', recipient_id: 'u_x', + access_level: 'read', source: 'manual', created_at: '2026-01-01T00:00:00Z', + }); + await driver.create('sys_record_share', legacy('shr_a1', 'deal_a1') as never); + await driver.create('sys_record_share', legacy('shr_b1', 'deal_b1') as never); + await driver.create('sys_record_share', legacy('shr_gone', 'deal_gone') as never); + await driver.create('sys_record_share', legacy('shr_p1', 'deal_p1') as never); + + const warn = vi.fn(); + const first = await runSysRecordShareOrganizationBackfill(ql as never, { dryRun: false, logger: { warn } }); + expect(first.scanned).toBe(4); + expect(first.written).toBe(2); + expect(first.residue).toMatchObject({ recordNotFound: 1, recordHasNoOrganization: 1 }); + expect(first.totals).toMatchObject({ orphans: 1, residualNull: 2 }); + // The orphan count is LOGGED, as the ruling asks, and the row is NOT deleted. + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0]![0])).toMatch(/1 grant row\(s\) reference a record that no longer exists/); + + const rows = await shares(); + expect(rows.map((r) => [r.id, r.organization_id])).toEqual([ + ['shr_a1', ORG_A], + ['shr_b1', ORG_B], + ['shr_gone', null], + ['shr_p1', null], + ]); + + // Idempotent: the repaired rows no longer match `IS NULL`; the residue is re-reported, not re-written. + const second = await runSysRecordShareOrganizationBackfill(ql as never, { dryRun: false }); + expect(second).toMatchObject({ scanned: 2, planned: 0, written: 0 }); + expect(second.totals.orphans).toBe(1); + }); +}); + +describe('[#14484] the ledger flip alone would REFUSE walled-posture grants — the repaired writer carries through it', () => { + it.each(['isolated', 'group'] as const)( + '%s posture: a bare system insert with no organization is refused loudly', + async (posture) => { + const { ql, shares } = await boot(posture); + await expect( + ql.insert( + 'sys_record_share', + { id: 'shr_bare', object_name: OBJECT, record_id: 'deal_a1', recipient_type: 'user', recipient_id: 'u_x', access_level: 'read', source: 'manual' }, + { context: SYSTEM } as never, + ), + ).rejects.toMatchObject({ code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED', status: 500 }); + expect(await shares()).toEqual([]); + }, + ); + + it.each(['isolated', 'group'] as const)( + "%s posture: the rule evaluator's grant carries the rule's organization and is NOT refused", + async (posture) => { + const { sharing, shares } = await boot(posture); + await sharing.grant( + { object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x', source: 'rule', sourceId: 'rule_1' }, + { ...SYSTEM, tenantId: ORG_A } as never, + ); + expect(byRecord(await shares())).toEqual({ deal_a1: ORG_A }); + }, + ); + + it("isolated posture: an owner's direct grant carries the record's organization and is NOT refused", async () => { + const { sharing, shares } = await boot('isolated'); + await sharing.grant( + { object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, + { userId: 'u_a', tenantId: ORG_A } as never, + ); + expect(byRecord(await shares())).toEqual({ deal_a1: ORG_A }); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/rule-criteria-org-scope.test.ts b/packages/plugins/plugin-sharing/src/rule-criteria-org-scope.test.ts index b544334839..aa3c3f6e5b 100644 --- a/packages/plugins/plugin-sharing/src/rule-criteria-org-scope.test.ts +++ b/packages/plugins/plugin-sharing/src/rule-criteria-org-scope.test.ts @@ -95,6 +95,11 @@ const DEAL_FIELDS: Record> = { const SHARE_FIELDS: Record> = { id: { type: 'text', name: 'id', label: 'Id', primary: true }, + // [#14484] The tenant column the registry provisions on every platform + // object (`applySystemFields`); this hand-built table must declare it too, + // now that the writer stamps it — the fixture was never spec-faithful + // without it, the omission just had no reader. + organization_id: { type: 'text', name: 'organization_id', label: 'Org' }, object_name: { type: 'text', name: 'object_name', label: 'Object' }, record_id: { type: 'text', name: 'record_id', label: 'Record' }, recipient_type: { type: 'text', name: 'recipient_type', label: 'Recipient type' }, diff --git a/packages/plugins/plugin-sharing/src/sharing-service.test.ts b/packages/plugins/plugin-sharing/src/sharing-service.test.ts index 8e63b5ccd0..69c5646bbe 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.test.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.test.ts @@ -1786,3 +1786,142 @@ describe('[#13551] the record-share `$in` drops nullish `record_id` rows', () => expect(f.$or[1].id.$in).toEqual(['42', 'a1']); }); }); + +// ───────────────────────────────────────────────────────────────────── +// [#14484] Every sys_record_share write carries organization_id +// ───────────────────────────────────────────────────────────────────── +// +// Ruled 2026-09-02 (decision batch #11 item 3, A adopted): a rule-materialised +// grant carries the granting RULE's organization, a direct grant the shared +// RECORD's. Pinned here against the service's own engine double, one direction +// per case, with the precedence stated where two answers exist. +describe('[#14484] grant stamps organization_id on both halves of the upsert', () => { + const ORG_RECORD = 'org_record'; + const ORG_CALLER = 'org_caller'; + const ORG_RULE = 'org_rule'; + + /** An org-walled business object — the injected `organization_id` column is declared. */ + const DEAL_SCHEMA = { + name: 'deal', + sharingModel: 'private', + fields: { id: {}, name: {}, owner_id: {}, organization_id: {} }, + }; + + let engine: ReturnType; + let svc: SharingService; + beforeEach(() => { + engine = makeFakeEngine({ deal: DEAL_SCHEMA, account: ACCOUNT_SCHEMA, sys_record_share: {} }); + svc = new SharingService({ engine }); + engine._tables.deal = [{ id: 'd1', name: 'Big deal', owner_id: 'admin', organization_id: ORG_RECORD }]; + engine._tables.account = [{ id: 'a1', name: 'Acme', owner_id: 'admin' }]; + }); + + it("a DIRECT grant carries the RECORD's organization — not the caller's active one", async () => { + // The caller acts from ORG_CALLER; the record lives in ORG_RECORD. Under a + // `single` posture holding several organizations both are real, and the + // ruling names the record's. + await svc.grant( + { object: 'deal', recordId: 'd1', recipientId: 'bob' }, + { userId: 'admin', tenantId: ORG_CALLER } as any, + ); + expect(engine._tables.sys_record_share[0].organization_id).toBe(ORG_RECORD); + }); + + it('the organization rides the WRITE CONTEXT too — the #8844 `{ isSystem, tenantId }` shape', async () => { + const insert = vi.spyOn(engine, 'insert'); + await svc.grant({ object: 'deal', recordId: 'd1', recipientId: 'bob' }, { userId: 'admin' } as any); + expect(insert).toHaveBeenCalledTimes(1); + // The double's `insert(object, data)` is 2-ary; the service passes the + // options bag as a third argument, which is what this pin reads. + const options = (insert.mock.calls[0] as unknown[])[2] as any; + expect(options.context).toMatchObject({ isSystem: true, tenantId: ORG_RECORD }); + }); + + it("a record on an object with NO tenant column falls back to the acting session's organization", async () => { + // `account` declares no `organization_id`; the acting organization is a + // fact of the write (the `sys_approval_request` writer's ruled fallback). + await svc.grant( + { object: 'account', recordId: 'a1', recipientId: 'bob' }, + { userId: 'admin', tenantId: ORG_CALLER } as any, + ); + expect(engine._tables.sys_record_share[0].organization_id).toBe(ORG_CALLER); + }); + + it('nothing to derive from ⇒ an EXPLICIT null on the row, for the engine\'s #8844 rule to decide', async () => { + const insert = vi.spyOn(engine, 'insert'); + await svc.grant({ object: 'account', recordId: 'a1', recipientId: 'bob' }, { userId: 'admin' } as any); + const row = engine._tables.sys_record_share[0]; + expect(row).toHaveProperty('organization_id', null); + // …and no tenant is threaded on the context either: `undefined`, which the + // engine reads as "not carried" — never `''` or `'null'`. + expect(((insert.mock.calls[0] as unknown[])[2] as any).context.tenantId).toBeUndefined(); + }); + + it("a SYSTEM caller carrying an organization (the rule evaluator's criteriaContext) wins over the record's", async () => { + // `SharingRuleService.reconcile` passes `{ ...SYSTEM_CTX, tenantId: rule.organization_id }`. + // The record lives in ORG_RECORD; the grant belongs to the RULE. + await svc.grant( + { object: 'deal', recordId: 'd1', recipientId: 'bob', source: 'rule', sourceId: 'rule_1' }, + { isSystem: true, tenantId: ORG_RULE } as any, + ); + expect(engine._tables.sys_record_share[0].organization_id).toBe(ORG_RULE); + }); + + it("a SYSTEM caller carrying NO organization (a platform-global rule) derives the record's", async () => { + await svc.grant( + { object: 'deal', recordId: 'd1', recipientId: 'bob', source: 'rule', sourceId: 'rule_global' }, + { isSystem: true } as any, + ); + expect(engine._tables.sys_record_share[0].organization_id).toBe(ORG_RECORD); + }); + + it('the UPDATE half stamps a pre-repair NULL row in place, and threads the organization on its context', async () => { + // A row written before the writer was repaired: same upsert key, no organization. + engine._tables.sys_record_share = [{ + id: 'shr_legacy', object_name: 'deal', record_id: 'd1', recipient_type: 'user', recipient_id: 'bob', + access_level: 'read', source: 'manual', organization_id: null, created_at: '2026-01-01T00:00:00Z', + }]; + const update = vi.spyOn(engine, 'update'); + const r = await svc.grant( + { object: 'deal', recordId: 'd1', recipientId: 'bob', accessLevel: 'edit' }, + { userId: 'admin' } as any, + ); + expect(r.id).toBe('shr_legacy'); + expect(engine._tables.sys_record_share).toHaveLength(1); + expect(engine._tables.sys_record_share[0].organization_id).toBe(ORG_RECORD); + expect((update.mock.calls[0]![2] as any).context).toMatchObject({ isSystem: true, tenantId: ORG_RECORD }); + }); + + it('an update that resolves NOTHING leaves the stored organization alone (never clears a backfilled value)', async () => { + engine._tables.sys_record_share = [{ + id: 'shr_kept', object_name: 'account', record_id: 'a1', recipient_type: 'user', recipient_id: 'bob', + access_level: 'read', source: 'manual', organization_id: 'org_backfilled', created_at: '2026-01-01T00:00:00Z', + }]; + const update = vi.spyOn(engine, 'update'); + await svc.grant( + { object: 'account', recordId: 'a1', recipientId: 'bob', accessLevel: 'edit' }, + { userId: 'admin' } as any, + ); + const patch = update.mock.calls[0]![1] as any; + expect(patch).not.toHaveProperty('organization_id'); + expect(engine._tables.sys_record_share[0].organization_id).toBe('org_backfilled'); + }); + + it('a record read that FAILS is logged and leaves the grant to the engine — never a stamped guess', async () => { + const warn = vi.fn(); + svc = new SharingService({ engine, logger: { warn } }); + const originalFind = engine.find.bind(engine); + vi.spyOn(engine, 'find').mockImplementation(async (object: string, options?: any) => { + // Only the organization read (projected to the tenant column) fails; + // the management pre-flight's reads keep working so the grant reaches it. + if (object === 'deal' && Array.isArray(options?.fields) && options.fields.includes('organization_id')) { + throw new Error('simulated driver outage'); + } + return originalFind(object, options); + }); + await svc.grant({ object: 'deal', recordId: 'd1', recipientId: 'bob' }, { userId: 'admin' } as any); + expect(engine._tables.sys_record_share[0]).toHaveProperty('organization_id', null); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0]![0])).toMatch(/could not read the shared record's organization/); + }); +}); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 89a5672181..d2c99ec754 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -2566,6 +2566,11 @@ "verb": "findOne", "pinned": 1 }, + { + "file": "packages/plugins/plugin-security/src/record-share-tenant-wall.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-security/src/resolve-permission-sets-for-context.pin.test.ts", "verb": "findOne", @@ -2691,6 +2696,11 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-sharing/src/boot-backfill.test.ts", "verb": "delete", From 8cdcf4273c719b6b089828e1b983e61a24d2feb1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 19:53:28 +0000 Subject: [PATCH 03/10] chore(census): re-derive the tenant-audit and system-context censuses on the merged tree (#14484) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- content/docs/permissions/system-context.mdx | 10 ++++----- .../docs/permissions/tenant-audit-census.mdx | 22 +++++++++---------- ...08-tenant-audit-write-call-sites.counts.md | 19 ++++++++-------- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index d512c21852..f378695c45 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -129,11 +129,11 @@ The largest single consumer — **20 of the 109 sites**. | # | Behaviour when `isSystem` | What you get / what you lose | Anchor | |:--|:---|:---|:---| | 30 | **Sharing-rule grant materialisation is skipped on all four record-write hooks** | Lose: **no `sys_record_share` rows are created**. A fully configured sharing rule grants **nothing** on seeded data until a rule is re-evaluated or the boot backfill runs. This is the behaviour that motivated #4707. Since #6783 the skip is no longer silent — it emits an INFO notice (rough edge 2) | `rule-hooks.ts:250`, `:274`, `:293`, `:322` | -| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:654` | -| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:920`, `:1007`, `:1597` | -| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1208` | -| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1286` (guard at `:1311`) | -| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1338` | +| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:658` | +| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:924`, `:1011`, `:1728` | +| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1219` | +| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1417` (guard at `:1442`) | +| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1469` | | 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` | | 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `plugin-sharing/src/share-link-service.ts:449`, `:503`, `:507`, `:580`, `:610` | | 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | diff --git a/content/docs/permissions/tenant-audit-census.mdx b/content/docs/permissions/tenant-audit-census.mdx index 87c684e694..649ebe0e6a 100644 --- a/content/docs/permissions/tenant-audit-census.mdx +++ b/content/docs/permissions/tenant-audit-census.mdx @@ -183,29 +183,29 @@ cannot read, and they are neither in nor out. | what | count | | :--- | ---: | -| write call sites on the application surface | **217** | -| …whose object name is statically decidable | 145 | +| write call sites on the application surface | **218** | +| …whose object name is statically decidable | 146 | | …whose object name is chosen at run time | 72 | -| …against an object with tenancy ENABLED | 145 | +| …against an object with tenancy ENABLED | 146 | | …against an object that declares tenancy off | 0 | -| threading a tenant context | 133 | +| threading a tenant context | 134 | | PROVABLY carrying none (options read, no context key) | **17** | | …of those, against a decidably tenancy-enabled object | **9** | | options argument UNREADABLE — may or may not carry one | 67 | | …of those, against a decidably tenancy-enabled object | 32 | -| threading a decidably ELEVATED (`isSystem`) context | 99 | +| threading a decidably ELEVATED (`isSystem`) context | 100 | | threading a context that is decidably NOT elevated | 0 | | threading a context whose elevation is a run-time fact | 101 | | how the instrument reached the site | count | | :--- | ---: | -| receiver carried a readable engine type | 172 | +| receiver carried a readable engine type | 173 | | receiver erased, placed by the object NAME | 19 | | receiver erased, placed by an `object: string` PARAMETER | 15 | | receiver erased, placed by an `UNTYPED_RECEIVERS` row | 11 | | object name spelled inline | 108 | -| object name spelled through a `const` | 37 | +| object name spelled through a `const` | 38 | | object name is an `object: string` parameter | 19 | | object name is some other run-time expression | 53 | @@ -224,13 +224,13 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-09-01 at `d3ebf3b55`. +Measured on 2026-09-02 at `38f9d540b`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 534 | -| engine-shaped types recognised | 56 | +| tracked non-test sources scanned | 540 | +| engine-shaped types recognised | 57 | | declared objects in the registry | 297 | -| same-named calls subtracted as non-engine | 119 | +| same-named calls subtracted as non-engine | 129 | {/* END GENERATED: tenant-audit-census */} diff --git a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md index f9458b7a03..991e2c7bec 100644 --- a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md +++ b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md @@ -29,17 +29,17 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution. | Measure | Value | |---|---:| -| Write call sites | 217 | -| Object name statically decidable | 145 | +| Write call sites | 218 | +| Object name statically decidable | 146 | | Object name chosen at run time | 72 | -| Against a tenancy-enabled object | 145 | +| Against a tenancy-enabled object | 146 | | Against an object declaring tenancy off | 0 | -| Threading a tenant context | 133 | +| Threading a tenant context | 134 | | Provably carrying none | 17 | | …and decidably tenancy-enabled | 9 | | Options argument unreadable | 67 | | …and decidably tenancy-enabled | 32 | -| Threading a decidably elevated context | 99 | +| Threading a decidably elevated context | 100 | | Threading a decidably non-elevated context | 0 | | Threading a context of undecidable elevation | 101 | @@ -52,14 +52,14 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-09-01 at `d3ebf3b55`. +Measured on 2026-09-02 at `38f9d540b`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 534 | -| engine-shaped types recognised | 56 | +| tracked non-test sources scanned | 540 | +| engine-shaped types recognised | 57 | | declared objects in the registry | 297 | -| same-named calls subtracted as non-engine | 119 | +| same-named calls subtracted as non-engine | 129 | ## Every site @@ -139,6 +139,7 @@ Measured on 2026-09-01 at `d3ebf3b55`. | `packages/plugins/plugin-security/src/suggested-audience-bindings.ts` | `insert` | `sys_audience_binding_suggestion` | enabled | context, elevation undecidable | 1 | | `packages/plugins/plugin-security/src/suggested-audience-bindings.ts` | `update` | `sys_audience_binding_suggestion` | enabled | context, elevation undecidable | 3 | | `packages/plugins/plugin-security/src/suggested-audience-bindings.ts` | `insert` | `sys_position_permission_set` | enabled | context, elevation undecidable | 1 | +| `packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.ts` | `update` | `sys_record_share` | enabled | elevated | 1 | | `packages/plugins/plugin-sharing/src/primary-bu-projection.ts` | `update` | `sys_user` | enabled | elevated | 2 | | `packages/plugins/plugin-sharing/src/record-orphan-cleanup.ts` | `delete` | `table` | undecidable | options unreadable | 2 | | `packages/plugins/plugin-sharing/src/share-link-service.ts` | `insert` | `sys_share_link` | enabled | elevated | 1 | From 250f9de5e28ed3a01d06b25d93e354e5ead0633b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 19:57:18 +0000 Subject: [PATCH 04/10] docs(census): restate the tenant-audit prose figures at the new population (#14484) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- content/docs/permissions/tenant-audit-census.mdx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/content/docs/permissions/tenant-audit-census.mdx b/content/docs/permissions/tenant-audit-census.mdx index 649ebe0e6a..6271c0ad69 100644 --- a/content/docs/permissions/tenant-audit-census.mdx +++ b/content/docs/permissions/tenant-audit-census.mdx @@ -98,7 +98,7 @@ are reported as `undecidable` rather than assumed either way. The same holds twice over for the context. An options argument spelled as a literal can be read; one spelled `options`, `{ ...opts }`, or handed through a -forwarding shim cannot, and **67 of the 217 sites are spelled that way**. A +forwarding shim cannot, and **67 of the 218 sites are spelled that way**. A context resolved from an inline literal or a local `const` can be tested for `isSystem`; one arriving from a helper call cannot. @@ -147,10 +147,10 @@ reproduce them. Where it disagrees, it disagrees on the page: | carried figure | where it survives | this census | | :--- | :--- | ---: | -| 175 write call sites | quoted in the merged changeset | **217** | +| 175 write call sites | quoted in the merged changeset | **218** | | 24 carrying no tenant context | quoted in the merged changeset | **9** provable and tenancy-enabled; **32** more whose options argument is unreadable | -| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **145 of 217** decidable, **72** undecidable | -| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 99 decidably elevated, 0 decidably not, 101 undecidable | +| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **146 of 218** decidable, **72** undecidable | +| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 100 decidably elevated, 0 decidably not, 101 undecidable | | 141 and 132, two independent re-derivations | the card that filed this work | — | **The differences are not reconciled, and deliberately so.** The old census's @@ -161,17 +161,17 @@ at any commit. Two structural facts do plausibly widen this reading against any hand or regex one, and both are counted in the generated tables below: the 45 sites reached -through an erased (`any`) receiver, and the 37 that name their object through a +through an erased (`any`) receiver, and the 38 that name their object through a `const` rather than inline. An instrument that read either the way a person does would report a smaller number and would not say so. The fourth row is the one worth flagging to anyone citing it. **The 135 / 77% figure has no surviving corroboration anywhere in the tree.** This census reads -99 of 217 (45%) as decidably elevated, with 101 more whose elevation is a +100 of 218 (46%) as decidably elevated, with 101 more whose elevation is a run-time fact — so the claim is neither confirmed nor refuted, and the honest answer is that a static reading cannot settle it. -⇒ **Cite `9 / 217`, and say what it is**: the sites whose options argument was +⇒ **Cite `9 / 218`, and say what it is**: the sites whose options argument was READ and holds no tenant context, against a decidably tenancy-enabled object. That is the control's provable yield surface. ⛔ Do not cite it as "the sites without tenant context" — **32 further sites** have an options argument this From bd48379656035b20cb4aeb4b32de374cebb4ca0e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 20:53:08 +0000 Subject: [PATCH 05/10] fix(sharing): keep tracker ids out of operator-facing strings; give the backfill test double the precedent's shape (#14484 gates) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- ...ill-sys-record-share-organizations.test.ts | 42 +++++++++---------- ...backfill-sys-record-share-organizations.ts | 6 ++- .../plugin-sharing/src/sharing-service.ts | 3 +- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.test.ts b/packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.test.ts index 0c98027b0b..80135bfac5 100644 --- a/packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.test.ts +++ b/packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.test.ts @@ -34,8 +34,10 @@ import { // --------------------------------------------------------------------------- // Fake engine — schemas + rows, with `find` honouring the two predicate shapes // the sweep issues (`{ col: null }` and `{ id: { $in: [...] } }`) plus plain -// equality, and — for the cliff cases — a STRICT organization wall applied to -// every non-system read, the way Layer 0 AND-composes it in production. +// equality. The `sys_file` precedent's double, shape for shape; the cliff cases +// below compose Layer 0's predicate into `where` themselves — which is exactly +// what `andComposeLayers` does in production — so the double needs no wall +// branch of its own. // --------------------------------------------------------------------------- interface FakeSchema { @@ -78,12 +80,12 @@ function createFakeEngine(init: { schemas: Record; rows: Record>>; failUpdateFor?: Set; - failFindFor?: Set; - /** When set, every NON-system read is walled to `context.tenantId` by strict equality. */ - strictWall?: boolean; + /** Objects whose TABLE is absent on this install: a read of them throws ("could not ask"). */ + unmounted?: readonly string[]; }) { const rows: Record>> = {}; for (const [object, list] of Object.entries(init.rows)) rows[object] = list.map((r) => ({ ...r })); + for (const object of init.unmounted ?? []) delete rows[object]; const updates: Array<{ object: string; data: Record; context: unknown }> = []; const engine: SysRecordShareBackfillEngine & { @@ -96,19 +98,9 @@ function createFakeEngine(init: { return schema; }, async find(object: string, options?: any) { - if (init.failFindFor?.has(object)) throw new Error(`simulated read failure on ${object}`); const table = rows[object]; if (!table) throw new Error(`object '${object}' is not mounted on this install`); - let out = table; - const ctx = options?.context ?? {}; - if (init.strictWall && !ctx.isSystem) { - // Layer 0's `isolated` arm: `{ organization_id: }`, - // AND-composed over everything else — the strict equality that wins - // over the driver's NULL-tolerant arm (`backfill-sys-file-organizations.ts`). - const org = ctx.tenantId; - out = out.filter((r) => r.organization_id === org); - } - out = out.filter((r) => matchesWhere(r, options?.where)); + let out = table.filter((r) => matchesWhere(r, options?.where)); if (options?.orderBy?.[0]?.field === 'id') { out = [...out].sort((a, b) => String(a.id).localeCompare(String(b.id))); } @@ -244,7 +236,7 @@ describe('[#14484] sys_record_share organization backfill — plan (dry run)', ( }); it('a subject read that FAILS leaves that object\'s rows alone as "could not ask" — the others still plan', async () => { - const engine = seeded({ failFindFor: new Set(['crm_case']) }); + const engine = seeded({ unmounted: ['crm_case'] }); const report = await planSysRecordShareOrganizationBackfill(engine); expect(report.residue.subjectReadFailed).toBe(1); expect(report.residualRows.find((r) => r.id === 'shr_case_a')?.reason).toBe('subjectReadFailed'); @@ -330,16 +322,22 @@ describe('[#14484] sys_record_share organization backfill — apply', () => { }); describe('[#14484] ⭐ the cliff — a strict organization wall over the grant table', () => { - // A tenant-scoped read of `sys_record_share` under a strict wall, as Layer 0 - // composes it for the `isolated` posture: `organization_id = `. + // A tenant-scoped read of `sys_record_share` under a strict wall: Layer 0's + // `isolated` predicate, `{ organization_id: }` + // (`computeTenantLayer0Filter`), AND-composed onto the query's own predicate + // exactly as `andComposeLayers` does in production. The strict equality is + // what wins over the driver's NULL-tolerant arm — the cliff. const tenantRead = (engine: ReturnType, org: string) => - engine.find(SYS_RECORD_SHARE_BACKFILL_OBJECT, { where: { object_name: 'crm_deal' }, context: { userId: 'u', tenantId: org } }); + engine.find(SYS_RECORD_SHARE_BACKFILL_OBJECT, { + where: { object_name: 'crm_deal', organization_id: org }, + context: { userId: 'u', tenantId: org }, + }); const bareRead = (engine: ReturnType) => engine.find(SYS_RECORD_SHARE_BACKFILL_OBJECT, { where: { object_name: 'crm_deal' }, context: { isSystem: true } }); const ids = (rows: unknown[]) => (rows as Array<{ id: string }>).map((r) => r.id).sort(); it('BEFORE the repair: the bare read returns the legacy grants, the tenant-scoped read returns NONE of them', async () => { - const engine = seeded({ strictWall: true }); + const engine = seeded(); const bare = ids(await bareRead(engine)); expect(bare).toContain('shr_deal_a'); expect(bare).toContain('shr_deal_b'); @@ -350,7 +348,7 @@ describe('[#14484] ⭐ the cliff — a strict organization wall over the grant t }); it('AFTER the repair: the tenant-scoped read equals the bare read filtered to that organization', async () => { - const engine = seeded({ strictWall: true }); + const engine = seeded(); await runSysRecordShareOrganizationBackfill(engine, { dryRun: false }); const bare = (await bareRead(engine)) as Array<{ id: string; organization_id: string | null }>; diff --git a/packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.ts b/packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.ts index 9552c5ee1b..63253253d2 100644 --- a/packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.ts +++ b/packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.ts @@ -564,9 +564,11 @@ function finish( if (logOrphans && sealed.residue.recordNotFound > 0) { // ⭐ The orphan count, logged — the half of the ruling's "leave NULL with a // logged count" that the report alone cannot deliver to an operator's log. + // (The sweep named here is #5103's; the id stays out of the string on + // purpose — it reaches operators, who cannot resolve a tracker id.) logger?.warn?.( `[sharing] sys_record_share organization backfill: ${sealed.residue.recordNotFound} grant row(s) ` - + 'reference a record that no longer exists — left NULL, not deleted; the #5103 orphan sweep ' + + 'reference a record that no longer exists — left NULL, not deleted; the boot-time orphan sweep ' + '(`sweepOrphanedRecordShares`, kernel:bootstrapped) reclaims them on the next boot', { orphans: sealed.residue.recordNotFound, dryRun: sealed.dryRun }, ); @@ -655,7 +657,7 @@ const RESIDUE_LABELS: Record = { subjectObjectUnknown: "the record's object has no schema this engine can read — wall column unknowable", subjectNotOrganizationScoped: "the record's object has no organization column at all", subjectReadFailed: 'the record could not be READ (could not ask is not gone)', - recordNotFound: 'ORPHAN — the record no longer exists (left NULL; the #5103 boot sweep deletes these)', + recordNotFound: 'ORPHAN — the record no longer exists (left NULL; the boot-time orphan sweep deletes these)', recordHasNoOrganization: 'the record exists, is organization-scoped, and carries no organization itself', }; diff --git a/packages/plugins/plugin-sharing/src/sharing-service.ts b/packages/plugins/plugin-sharing/src/sharing-service.ts index 3a708d6dec..5206f66985 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.ts @@ -1372,9 +1372,10 @@ export class SharingService implements ISharingService { const value = row?.[tenantField]; return typeof value === 'string' && value.trim() !== '' ? value : null; } catch (err: any) { + // [#14484] The id stays out of the string: it reaches operators. this.logger?.warn?.( '[sharing] could not read the shared record\'s organization — the grant carries none from it, ' - + 'and the engine\'s system-write organization rule decides the row (#14484)', + + 'and the engine\'s system-write organization rule decides the row', { object, recordId, error: err?.message }, ); return null; From 238211787a4599c41568de73679357637487e653 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 20:54:03 +0000 Subject: [PATCH 06/10] chore(census): re-anchor the system-context census after the gate fixes (#14484) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- content/docs/permissions/system-context.mdx | 6 +++--- content/docs/permissions/tenant-audit-census.mdx | 2 +- docs/audits/2026-08-tenant-audit-write-call-sites.counts.md | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index f378695c45..77bf15e17a 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -130,10 +130,10 @@ The largest single consumer — **20 of the 109 sites**. |:--|:---|:---|:---| | 30 | **Sharing-rule grant materialisation is skipped on all four record-write hooks** | Lose: **no `sys_record_share` rows are created**. A fully configured sharing rule grants **nothing** on seeded data until a rule is re-evaluated or the boot backfill runs. This is the behaviour that motivated #4707. Since #6783 the skip is no longer silent — it emits an INFO notice (rough edge 2) | `rule-hooks.ts:250`, `:274`, `:293`, `:322` | | 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:658` | -| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:924`, `:1011`, `:1728` | +| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:924`, `:1011`, `:1729` | | 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1219` | -| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1417` (guard at `:1442`) | -| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1469` | +| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1418` (guard at `:1443`) | +| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1470` | | 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` | | 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `plugin-sharing/src/share-link-service.ts:449`, `:503`, `:507`, `:580`, `:610` | | 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | diff --git a/content/docs/permissions/tenant-audit-census.mdx b/content/docs/permissions/tenant-audit-census.mdx index 6271c0ad69..64e073f45d 100644 --- a/content/docs/permissions/tenant-audit-census.mdx +++ b/content/docs/permissions/tenant-audit-census.mdx @@ -224,7 +224,7 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-09-02 at `38f9d540b`. +Measured on 2026-09-02 at `bd4837965`. | corpus scale (not enforced) | count | | :--- | ---: | diff --git a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md index 991e2c7bec..37341a41b8 100644 --- a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md +++ b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md @@ -52,7 +52,7 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-09-02 at `38f9d540b`. +Measured on 2026-09-02 at `bd4837965`. | corpus scale (not enforced) | count | | :--- | ---: | From 1ad15772f76b24b9256a73f8e1df5d6c71ffb983 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 21:13:52 +0000 Subject: [PATCH 07/10] chore(census): re-derive the generated artefacts on the merged tree (#14484) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- content/docs/permissions/tenant-audit-census.mdx | 2 +- docs/audits/2026-08-tenant-audit-write-call-sites.counts.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/content/docs/permissions/tenant-audit-census.mdx b/content/docs/permissions/tenant-audit-census.mdx index 64e073f45d..b67298f02a 100644 --- a/content/docs/permissions/tenant-audit-census.mdx +++ b/content/docs/permissions/tenant-audit-census.mdx @@ -224,7 +224,7 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-09-02 at `bd4837965`. +Measured on 2026-09-02 at `e8666e3b1`. | corpus scale (not enforced) | count | | :--- | ---: | diff --git a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md index 37341a41b8..e5fd76047d 100644 --- a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md +++ b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md @@ -52,7 +52,7 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-09-02 at `bd4837965`. +Measured on 2026-09-02 at `e8666e3b1`. | corpus scale (not enforced) | count | | :--- | ---: | From 68ad887bd41a957fbfa3fc32c4d646f60deaa17f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 22:12:53 +0000 Subject: [PATCH 08/10] fix(sharing): a failed record read never stamps the acting session's organization on a direct grant (#14484) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patch round 1 of the 2026-09-02 contract review (BLOCKING 1): `recordOrganization` returned the same `null` for "the record carries no organization" and for "the read threw", so `resolveDirectGrantOrganization` fell through to the acting session's organization on a transient read failure — a wrong stamp on a permission-boundary column that the backfill (`WHERE organization_id IS NULL`) could never repair. The reading is now a three-way `RecordOrganizationReading` (`organization` / `none` / `read-failed`). The direct path substitutes the session's organization for `none` only; `read-failed` yields `null` to the engine's ruled derive-or-refuse (#8844). The warn text now states what the failed read does NOT do. Pinned on the double (insert half, update half, and the `none` control) and on a real SqlDriver + ObjectQL: `single` with no organization -> NULL, `single` holding two organizations -> refused as ambiguous, `isolated`/`group` -> refused, plus the read-working control on both. Review §5: note 3 pinned (the scoped update half's silent no-op on a row stamped with another organization — `grant`'s return value, the two-rules `grantsCreated` path and the re-homed-record `grantsUpdated` path); note 1 — the changeset now states the two shipped paths that meet the walled-install refusal (platform-global rule x organization-less record, with the mid-loop reconcile abort; and a failed direct-grant read); note 2 — objectql graded `minor`. Census page re-anchored with `--fix` (8 line-rot anchors, 0 refused). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- .changeset/record-share-organization-stamp.md | 6 +- content/docs/permissions/system-context.mdx | 10 +- .../record-share-organization-stamp.test.ts | 189 +++++++++++++++++- .../src/sharing-service.test.ts | 78 +++++++- .../plugin-sharing/src/sharing-service.ts | 86 ++++++-- 5 files changed, 339 insertions(+), 30 deletions(-) diff --git a/.changeset/record-share-organization-stamp.md b/.changeset/record-share-organization-stamp.md index 37497743df..2074de34b6 100644 --- a/.changeset/record-share-organization-stamp.md +++ b/.changeset/record-share-organization-stamp.md @@ -1,15 +1,15 @@ --- "@objectstack/plugin-sharing": minor -"@objectstack/objectql": patch +"@objectstack/objectql": minor --- `sys_record_share` is tenant-scoped: every grant row now carries `organization_id`, and the rows written before it can be backfilled from the record they grant access to (#14484). Every `sys_record_share` row on every deployment was written with `organization_id = NULL`: `SharingService.grant` wrote under a bare system context and the row literal never carried the column, so neither the driver's `injectTenantOnInsert` nor the engine's system-write organization rule had anything to stamp from. Reads agreed with writes — the service's own reads are bare-context too — so nothing was visibly broken; what the NULL cost was the cliff: the first tenant-facing read of the table inherits `plugin-security`'s Layer 0, whose strict `organization_id = :tenant` AND-composes over the driver's NULL-tolerant arm and wins, and every existing grant silently disappears — not refused, simply "this person was never granted access". Maintainer ruling 2026-09-02 (decision batch #11 item 3, A adopted — 「#13564 转维护者处理；其他同意」): tenant-scoped, writer-repaired, existing rows backfilled from the record they reference. The per-table order the `sys_file` precedent requires; it covers `sys_record_share` and no other table. -**Writer.** `SharingService.grant` stamps `organization_id` on both halves of its upsert. A rule-materialised grant carries the granting RULE's organization — `SharingRuleService.reconcile` / `reconcileForRecord` now hand `grant` the rule's own `criteriaContext` (`{ isSystem, tenantId: rule.organization_id }`), the same context the rule's criteria sweep ran under, so the grant lands in the organization whose records the rule was allowed to sweep. A direct grant carries the shared RECORD's organization, read off the column its object is walled by (`resolveTenantFieldName`: ADR-0066 opt-out → declared `tenancy.tenantField` → injected `organization_id`), with the acting session's organization as the fallback for a record that carries none. The organization rides the write context as `tenantId` as well as the row — the `{ isSystem, tenantId }` shape the #8844 refusal prescribes — so the driver's tenant audit is satisfied and the update half lands through the driver's scope (`organization_id = ? OR IS NULL`, which keeps a pre-repair NULL row in reach). Nothing resolvable ⇒ an explicit `null` on the row, for the engine's ruled rule to decide (below). The service's eleven bare-context READS are unchanged by this change; whether they become tenant-scoped is #13564's question. +**Writer.** `SharingService.grant` stamps `organization_id` on both halves of its upsert. A rule-materialised grant carries the granting RULE's organization — `SharingRuleService.reconcile` / `reconcileForRecord` now hand `grant` the rule's own `criteriaContext` (`{ isSystem, tenantId: rule.organization_id }`), the same context the rule's criteria sweep ran under, so the grant lands in the organization whose records the rule was allowed to sweep. A direct grant carries the shared RECORD's organization, read off the column its object is walled by (`resolveTenantFieldName`: ADR-0066 opt-out → declared `tenancy.tenantField` → injected `organization_id`), with the acting session's organization as the fallback for a record that carries none — and only for one that carries none: a record whose organization could not be READ is unknown, not organization-less, so that grant carries `null` for the engine's rule below rather than the session's organization (which may not be the record's). The organization rides the write context as `tenantId` as well as the row — the `{ isSystem, tenantId }` shape the #8844 refusal prescribes — so the driver's tenant audit is satisfied and the update half lands through the driver's scope (`organization_id = ? OR IS NULL`, which keeps a pre-repair NULL row in reach). Nothing resolvable ⇒ an explicit `null` on the row, for the engine's ruled rule to decide (below). The service's eleven bare-context READS are unchanged by this change; whether they become tenant-scoped is #13564's question. -**Ledger (`@objectstack/objectql`).** `sys_record_share` leaves `unclassified` in the #13491 per-object tenancy ledger as `tenant-scoped`, with the ruling as the cited fact. Consequence, per the ledger's own admission semantics: an organization-less SYSTEM insert on `sys_record_share` is now derived on a `single` install with exactly one organization and REFUSED loudly on a walled one (`ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED`, status 500) — and the engine no longer auto-mutes the driver's tenant-audit warning for elevated writes on it. The only writer in this repository is repaired in the same change, so no shipped path meets that refusal; a third-party writer that inserts `sys_record_share` under a bare system context on a walled install will, and the refusal message says how to carry the organization. +**Ledger (`@objectstack/objectql`).** `sys_record_share` leaves `unclassified` in the #13491 per-object tenancy ledger as `tenant-scoped`, with the ruling as the cited fact. Consequence, per the ledger's own admission semantics: an organization-less SYSTEM insert on `sys_record_share` is now derived on a `single` install with exactly one organization and REFUSED loudly on a walled one (`ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED`, status 500) — and the engine no longer auto-mutes the driver's tenant-audit warning for elevated writes on it. The only writer in this repository is repaired in the same change and carries the organization on every path that can resolve one — but two shipped paths still resolve none, and on a walled install they now meet that refusal instead of writing a NULL row. **(a)** A platform-global sharing rule (`organization_id = null`; its sweep runs unscoped) matching an organization-less record: the grant resolves `null`, the engine refuses it, and because `reconcile` has no per-grant catch that rule's reconcile aborts mid-loop — grants already written in the pass stay, the remaining grants and the stale-row revocations of that pass do not happen (the boot backfill logs the rule and continues; the write hooks catch). **(b)** A direct grant whose read of the shared record's organization failed: the acting session's organization is deliberately not substituted, so the grant is refused with the same error rather than written into an organization that may not be the record's. On a `single` install with exactly one organization both derive it; with several, the same `null` is refused as `ambiguous-organization`. A third-party writer that inserts `sys_record_share` under a bare system context on a walled install meets the refusal too, and the refusal message says how to carry the organization. **Ops: the backfill — dry run first, and by default.** `packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.ts` scans only rows whose organization column is unset, re-reads each row's record (`object_name` + `record_id`) at repair time, and stamps the row with the record's own organization off the column that object is walled by. `planSysRecordShareOrganizationBackfill(engine)` reads only and returns a report naming every row it would touch; `runSysRecordShareOrganizationBackfill(engine, { dryRun: false })` writes. Nothing runs at boot and nothing is scheduled: this is an operator-invoked module, run once against an affected install, the posture of both precedents. **Orphans — grant rows whose record no longer exists — are left NULL, counted (`totals.orphans`) and logged, never deleted here:** the "record gone ⇒ the row cannot describe any access" invariant is already owned by the `kernel:bootstrapped` orphan sweep (`sweepOrphanedRecordShares`, #5103), which reclaims exactly that population on the next boot; a second deleter would be the fork `record-orphan-cleanup.ts` exists to prevent. Every other row that cannot be derived — an object with no organization column, a record that carries none, a record whose read failed — stays NULL and is reported by reason, for a dry run too. Idempotent by construction: every scan is `WHERE IS NULL` and every write fills that column, so the test suite runs the sweep twice and pins the second run at zero writes. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 376bd22242..8f7b4c17d9 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -129,11 +129,11 @@ The largest single consumer — **20 of the 109 sites**. | # | Behaviour when `isSystem` | What you get / what you lose | Anchor | |:--|:---|:---|:---| | 30 | **Sharing-rule grant materialisation is skipped on all four record-write hooks** | Lose: **no `sys_record_share` rows are created**. A fully configured sharing rule grants **nothing** on seeded data until a rule is re-evaluated or the boot backfill runs. This is the behaviour that motivated #4707. Since #6783 the skip is no longer silent — it emits an INFO notice (rough edge 2) | `rule-hooks.ts:250`, `:274`, `:293`, `:322` | -| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:658` | -| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:924`, `:1011`, `:1729` | -| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1219` | -| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1418` (guard at `:1443`) | -| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1470` | +| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:677` | +| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:943`, `:1030`, `:1783` | +| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1238` | +| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1472` (guard at `:1497`) | +| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1524` | | 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` | | 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:449`, `:503`, `:507`, `:580`, `:610` | | 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | diff --git a/packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts b/packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts index ac2ac00c39..89831de181 100644 --- a/packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts +++ b/packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts @@ -86,6 +86,11 @@ const RULE_FIELDS: Record> = { updated_at: { type: 'text', name: 'updated_at', label: 'Updated' }, }; +const ORG_FIELDS: Record> = { + id: { type: 'text', name: 'id', label: 'Id', primary: true }, + name: { type: 'text', name: 'name', label: 'Name' }, +}; + const ORG_A = 'org_a'; const ORG_B = 'org_b'; @@ -124,7 +129,7 @@ const open: SqlDriver[] = []; * deal_b2 ORG_B stage=won * deal_p1 (null) stage=won <- organization-less; the discriminating record */ -async function boot(posture?: 'single' | 'isolated' | 'group'): Promise { +async function boot(posture?: 'single' | 'isolated' | 'group', organizations: readonly string[] = []): Promise { const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, @@ -139,11 +144,21 @@ async function boot(posture?: 'single' | 'isolated' | 'group'): Promise ql.registerObject({ name: OBJECT, label: 'Deal', sharingModel: 'private', fields: DEAL_FIELDS } as never); ql.registerObject({ name: 'sys_record_share', label: 'Record Share', isSystem: true, fields: SHARE_FIELDS } as never); ql.registerObject({ name: 'sys_sharing_rule', label: 'Sharing Rule', isSystem: true, fields: RULE_FIELDS } as never); + // `sys_organization` is what the engine's #8844 rule COUNTS on a `single` + // install (`probeInstallOrganizations`: 0 ⇒ nothing to stamp, 1 ⇒ derived, + // several ⇒ refused as ambiguous). Absent by default — the fixture's + // `no-organization-yet` reading the earlier blocks rely on — and provisioned + // only for the cases that need the install to hold several. + if (organizations.length > 0) { + ql.registerObject({ name: 'sys_organization', label: 'Organization', isSystem: true, fields: ORG_FIELDS } as never); + } await driver.initObjects([ { name: OBJECT, fields: DEAL_FIELDS } as never, { name: 'sys_record_share', fields: SHARE_FIELDS } as never, { name: 'sys_sharing_rule', fields: RULE_FIELDS } as never, + ...(organizations.length > 0 ? [{ name: 'sys_organization', fields: ORG_FIELDS } as never] : []), ]); + for (const id of organizations) await driver.create('sys_organization', { id, name: id } as never); // Seeded through the driver: the fixture is the DATA at rest. await driver.create(OBJECT, { id: 'deal_a1', stage: 'won', owner_id: 'u_a', organization_id: ORG_A } as never); @@ -334,3 +349,175 @@ describe('[#14484] the ledger flip alone would REFUSE walled-posture grants — expect(byRecord(await shares())).toEqual({ deal_a1: ORG_A }); }); }); + +describe("[#14484] a record read that FAILS never stamps the acting session's organization (2026-09-02 review, BLOCKING 1)", () => { + /** + * Fail ONLY the organization read — the one `SharingService.recordOrganization` + * projects to the tenant column. `canManageShares`' owner read (`['id', + * 'owner_id']`) keeps working, so the grant reaches the stamp instead of + * being refused at the pre-flight, and every other engine call — the + * `sys_record_share` upsert lookup, the engine's own `sys_organization` + * probe — is real. + */ + function failOrganizationRead(ql: ObjectQL): () => number { + let failed = 0; + const original = ql.find.bind(ql); + vi.spyOn(ql, 'find').mockImplementation((async (object: string, options?: any) => { + if (object === OBJECT && Array.isArray(options?.fields) && options.fields.includes('organization_id')) { + failed += 1; + throw new Error('simulated driver outage'); + } + return original(object, options); + }) as never); + return () => failed; + } + + // The scenario the review names: the record lives in ORG_A, the caller acts + // from ORG_B — a `single` install holding several organizations, or a + // `group` member active in a sibling — and the read that would say ORG_A + // failed. The session's ORG_B is the wrong answer on every posture; what + // differs per posture is what the engine's #8844 rule does with the `null`. + + it("single posture, no organization yet: the row carries NULL — repairable by the backfill, which a stamped ORG_B never would be", async () => { + const { ql, sharing, shares } = await boot('single'); + const failures = failOrganizationRead(ql); + await sharing.grant( + { object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, + { userId: 'u_a', tenantId: ORG_B } as never, + ); + expect(failures()).toBe(1); + expect(byRecord(await shares())).toEqual({ deal_a1: null }); + }); + + it('single posture holding ORG_A and ORG_B: refused as ambiguous rather than written into the session\'s ORG_B', async () => { + const { ql, sharing, shares } = await boot('single', [ORG_A, ORG_B]); + const failures = failOrganizationRead(ql); + await expect( + sharing.grant( + { object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, + { userId: 'u_a', tenantId: ORG_B } as never, + ), + ).rejects.toMatchObject({ code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED', status: 500 }); + expect(failures()).toBe(1); + expect(await shares()).toEqual([]); + }); + + it.each(['isolated', 'group'] as const)( + "%s posture: refused loudly rather than written into the session's organization", + async (posture) => { + const { ql, sharing, shares } = await boot(posture); + const failures = failOrganizationRead(ql); + await expect( + sharing.grant( + { object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, + { userId: 'u_a', tenantId: ORG_B } as never, + ), + ).rejects.toMatchObject({ code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED', status: 500 }); + expect(failures()).toBe(1); + expect(await shares()).toEqual([]); + }, + ); + + it("CONTROL: the same session grant with the read WORKING carries the record's ORG_A — on a walled posture and with two organizations alike", async () => { + // The refusals above are the failed read's, not the posture's or the + // session's: with the read intact the identical call lands, in ORG_A. + const walled = await boot('group'); + await walled.sharing.grant({ object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, { userId: 'u_a', tenantId: ORG_B } as never); + expect(byRecord(await walled.shares())).toEqual({ deal_a1: ORG_A }); + + const several = await boot('single', [ORG_A, ORG_B]); + await several.sharing.grant({ object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, { userId: 'u_a', tenantId: ORG_B } as never); + expect(byRecord(await several.shares())).toEqual({ deal_a1: ORG_A }); + }); +}); + +describe('[#14484] the scoped UPDATE half cannot reach a row stamped with a DIFFERENT organization — a silent no-op, pinned as such (2026-09-02 review, §5 note 3)', () => { + // `SqlDriver.applyTenantScope` scopes the update half to + // `organization_id = ? OR organization_id IS NULL`: a legacy NULL row is + // reachable (pinned above), a row carrying ANOTHER organization is not — + // the wall, not a defect. What this block pins is the SHAPE of that no-op + // as the callers see it: `grant` returns `{ ...row, ...patch }` and the + // evaluator counts the call, while the stored row is untouched. Reachable + // through an organization-less record that two organizations' rules both + // match (the upsert key excludes `source_id`), or a record re-homed after a + // platform-global rule granted it; it self-heals only through the other + // rule's next revoke. The behaviour stands; the pin is so a reader of the + // counters does not mistake them for rows. + + const allShares = async (driver: SqlDriver) => (await driver.find('sys_record_share', {} as never)) as any[]; + + it("grant: the return value reports the patch; the stored row keeps the other organization's stamp and access level", async () => { + const { driver, sharing } = await boot(); + await driver.create('sys_record_share', { + id: 'shr_org_a', object_name: OBJECT, record_id: 'deal_p1', recipient_type: 'user', recipient_id: 'u_x', + access_level: 'read', source: 'rule', source_id: 'rule_org_a', organization_id: ORG_A, created_at: '2026-01-01T00:00:00Z', + } as never); + + const r = await sharing.grant( + { object: OBJECT, recordId: 'deal_p1', recipientId: 'u_x', accessLevel: 'edit', source: 'rule', sourceId: 'rule_org_b' }, + { ...SYSTEM, tenantId: ORG_B } as never, + ); + // What the caller is told… + expect(r).toMatchObject({ id: 'shr_org_a', access_level: 'edit', organization_id: ORG_B, source_id: 'rule_org_b' }); + // …and what is on disk: one row, ORG_A's, byte for byte as seeded. + const rows = await allShares(driver); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ id: 'shr_org_a', access_level: 'read', organization_id: ORG_A, source_id: 'rule_org_a' }); + }); + + it("reconcile: two organizations' rules on one organization-less record — the second's grant is COUNTED, the row stays the first's", async () => { + const { driver, rules, shares } = await boot(); + const ruleA = await rules.defineRule( + { name: 'os14484_note3_a', label: 'ORG_A won', object: OBJECT, criteria: WON, recipientType: 'user', recipientId: 'u_x', accessLevel: 'read' } as never, + ORG_A_ADMIN, + ); + await rules.evaluateRule(ruleA.id, ORG_A_ADMIN); + expect(byRecord(await shares())).toEqual({ deal_a1: ORG_A, deal_p1: ORG_A }); + + const ORG_B_ADMIN = { ...ORG_A_ADMIN, tenantId: ORG_B } as unknown as ExecutionContext; + const ruleB = await rules.defineRule( + { name: 'os14484_note3_b', label: 'ORG_B won', object: OBJECT, criteria: WON, recipientType: 'user', recipientId: 'u_x', accessLevel: 'edit' } as never, + ORG_B_ADMIN, + ); + const result = await rules.evaluateRule(ruleB.id, ORG_B_ADMIN); + // The ORG_B sweep matches deal_b1, deal_b2 and — through the driver's + // compatibility arm — deal_p1. The evaluator keys its existing set by its + // OWN rule id, so all three are "created" to it; inside `grant`, deal_p1's + // upsert lookup (no `source_id` in the key) finds rule A's row and takes + // the update half, which the ORG_B scope cannot reach. + expect(result).toMatchObject({ matchedRecords: 3, grantsCreated: 3, grantsUpdated: 0 }); + const rows = await allShares(driver); + expect(rows).toHaveLength(4); // deal_a1, deal_b1, deal_b2, deal_p1 — no second row for deal_p1 either + expect(rows.find((r) => r.record_id === 'deal_p1')).toMatchObject({ organization_id: ORG_A, access_level: 'read', source_id: ruleA.id }); + }); + + it("reconcile: a record re-homed after a platform-global rule granted it — the rule's next pass COUNTS an update its scoped write never lands", async () => { + const { driver, rules } = await boot(); + const define = (accessLevel: 'read' | 'edit') => rules.defineRule( + { name: 'os14484_note3_rehome', label: 'Platform won', object: OBJECT, criteria: WON, recipientType: 'user', recipientId: 'u_x', accessLevel } as never, + SYSTEM, + ); + const rule = await define('read'); + await rules.evaluateRule(rule.id, SYSTEM); + expect((await allShares(driver)).find((r) => r.record_id === 'deal_a1')).toMatchObject({ organization_id: ORG_A, access_level: 'read' }); + + // The record moves to ORG_B; its grant row still says ORG_A (the + // organization the record was in when the platform-global rule stamped it). + await driver.update(OBJECT, 'deal_a1', { organization_id: ORG_B }); + // The rule's level changes, so the next pass takes the update half for + // every matched record. deal_a1's resolves ORG_B from the re-homed record, + // and the ORG_B-scoped update cannot see the ORG_A row. + await define('edit'); + const result = await rules.evaluateRule(rule.id, SYSTEM); + expect(result).toMatchObject({ matchedRecords: 4, grantsCreated: 0, grantsUpdated: 4 }); + + const rows = await allShares(driver); + const byId = Object.fromEntries(rows.map((r) => [r.record_id, { organization_id: r.organization_id ?? null, access_level: r.access_level }])); + expect(byId).toEqual({ + deal_a1: { organization_id: ORG_A, access_level: 'read' }, // counted, not landed + deal_b1: { organization_id: ORG_B, access_level: 'edit' }, + deal_b2: { organization_id: ORG_B, access_level: 'edit' }, + deal_p1: { organization_id: null, access_level: 'edit' }, + }); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/sharing-service.test.ts b/packages/plugins/plugin-sharing/src/sharing-service.test.ts index 69c5646bbe..53fc452680 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.test.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.test.ts @@ -1907,21 +1907,89 @@ describe('[#14484] grant stamps organization_id on both halves of the upsert', ( expect(engine._tables.sys_record_share[0].organization_id).toBe('org_backfilled'); }); - it('a record read that FAILS is logged and leaves the grant to the engine — never a stamped guess', async () => { - const warn = vi.fn(); - svc = new SharingService({ engine, logger: { warn } }); + /** + * Fail ONLY the organization read (the one projected to the tenant column); + * the management pre-flight's owner read (`['id', 'owner_id']`) keeps + * working, so the grant reaches the stamp instead of being refused before it. + */ + function failOrganizationRead(): void { const originalFind = engine.find.bind(engine); vi.spyOn(engine, 'find').mockImplementation(async (object: string, options?: any) => { - // Only the organization read (projected to the tenant column) fails; - // the management pre-flight's reads keep working so the grant reaches it. if (object === 'deal' && Array.isArray(options?.fields) && options.fields.includes('organization_id')) { throw new Error('simulated driver outage'); } return originalFind(object, options); }); + } + + it('a record read that FAILS is logged and leaves the grant to the engine — never a stamped guess', async () => { + const warn = vi.fn(); + svc = new SharingService({ engine, logger: { warn } }); + failOrganizationRead(); await svc.grant({ object: 'deal', recordId: 'd1', recipientId: 'bob' }, { userId: 'admin' } as any); expect(engine._tables.sys_record_share[0]).toHaveProperty('organization_id', null); expect(warn).toHaveBeenCalledTimes(1); expect(String(warn.mock.calls[0]![0])).toMatch(/could not read the shared record's organization/); }); + + // ── The with-session-organization variant (2026-09-02 contract review, BLOCKING 1) ── + // + // The record lives in ORG_RECORD; the caller acts from ORG_CALLER — a + // `single` install holding several organizations, or a `group` member + // active in a sibling — and the read that would say ORG_RECORD failed. + // Before the fix a failed read and "no organization" were the same `null`, + // so the direct path fell through to the session's organization and the + // row was written into ORG_CALLER: invisible to ORG_RECORD under a + // tenant-scoped read, visible to the sibling, and not NULL, so the backfill + // (`WHERE organization_id IS NULL`) could never repair it. + + it("a record read that FAILS with a session organization present does NOT stamp the session's — unknown is not absent", async () => { + const warn = vi.fn(); + svc = new SharingService({ engine, logger: { warn } }); + const insert = vi.spyOn(engine, 'insert'); + failOrganizationRead(); + await svc.grant( + { object: 'deal', recordId: 'd1', recipientId: 'bob' }, + { userId: 'admin', tenantId: ORG_CALLER } as any, + ); + const row = engine._tables.sys_record_share[0]; + expect(row).toHaveProperty('organization_id', null); + expect(row.organization_id).not.toBe(ORG_CALLER); + // …and nothing is threaded on the write context either: the engine's + // ruled derive-or-refuse decides the row, not the session. + expect(((insert.mock.calls[0] as unknown[])[2] as any).context.tenantId).toBeUndefined(); + expect(warn).toHaveBeenCalledTimes(1); + // The warn says what was NOT done, which is the one thing its reader needs. + expect(String(warn.mock.calls[0]![0])).toMatch(/acting session's organization is NOT substituted/); + }); + + it("the UPDATE half on a failed read leaves the stored organization alone — never overwritten with the session's", async () => { + engine._tables.sys_record_share = [{ + id: 'shr_stamped', object_name: 'deal', record_id: 'd1', recipient_type: 'user', recipient_id: 'bob', + access_level: 'read', source: 'manual', organization_id: ORG_RECORD, created_at: '2026-01-01T00:00:00Z', + }]; + svc = new SharingService({ engine, logger: { warn: vi.fn() } }); + const update = vi.spyOn(engine, 'update'); + failOrganizationRead(); + await svc.grant( + { object: 'deal', recordId: 'd1', recipientId: 'bob', accessLevel: 'edit' }, + { userId: 'admin', tenantId: ORG_CALLER } as any, + ); + const patch = update.mock.calls[0]![1] as any; + expect(patch).not.toHaveProperty('organization_id'); + expect((update.mock.calls[0]![2] as any).context.tenantId).toBeUndefined(); + expect(engine._tables.sys_record_share[0]).toMatchObject({ organization_id: ORG_RECORD, access_level: 'edit' }); + }); + + it("CONTROL: a record that carries NO organization (a real row, empty column — not a failed read) still falls back to the session's", async () => { + // `none` and `read-failed` are the two facts the fix keeps apart; this is + // the half that must NOT move. `d_orgless` is a live row on the walled + // object whose organization column is simply empty. + engine._tables.deal.push({ id: 'd_orgless', name: 'Orgless', owner_id: 'admin', organization_id: null }); + await svc.grant( + { object: 'deal', recordId: 'd_orgless', recipientId: 'bob' }, + { userId: 'admin', tenantId: ORG_CALLER } as any, + ); + expect(engine._tables.sys_record_share[0].organization_id).toBe(ORG_CALLER); + }); }); diff --git a/packages/plugins/plugin-sharing/src/sharing-service.ts b/packages/plugins/plugin-sharing/src/sharing-service.ts index 5206f66985..ff3ee85913 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.ts @@ -143,6 +143,25 @@ function activeOrganizationId(context: ExecutionContext): string | null { return typeof org === 'string' && org.trim() !== '' ? org : null; } +/** + * [#14484] What `SharingService.recordOrganization` found — three answers the + * direct-grant path treats differently, so they are typed apart rather than + * collapsed into one `null`. `none` (no tenant column, record gone, or an + * organization-less row) earns the acting session's organization as the + * fallback; `read-failed` earns nothing: the record's organization is + * UNKNOWN, not absent, and the session's may not be it — the BLOCKING finding + * of this change's 2026-09-02 contract review, where the two collapsed into + * one `null` and a transient read failure stamped the session's organization + * on a permission-boundary column. + */ +type RecordOrganizationReading = + | { readonly kind: 'organization'; readonly organizationId: string } + | { readonly kind: 'none' } + | { readonly kind: 'read-failed' }; + +const NO_RECORD_ORGANIZATION: RecordOrganizationReading = { kind: 'none' }; +const RECORD_ORGANIZATION_READ_FAILED: RecordOrganizationReading = { kind: 'read-failed' }; + function hasOwnerField(schema: any): boolean { return Boolean(schema?.fields && OWNER_FIELD in schema.fields); } @@ -1319,8 +1338,12 @@ export class SharingService implements ISharingService { input: GrantShareInput, context: ExecutionContext, ): Promise { - return activeOrganizationId(context) - ?? (await this.recordOrganization(input.object, input.recordId)); + const threaded = activeOrganizationId(context); + if (threaded) return threaded; + const reading = await this.recordOrganization(input.object, input.recordId); + // `none` and `read-failed` both end here: this path has no session + // organization to substitute, so both are the engine's to decide. + return reading.kind === 'organization' ? reading.organizationId : null; } /** @@ -1330,18 +1353,38 @@ export class SharingService implements ISharingService { * `single` posture holding several organizations may not be the record's. * * The acting session's organization is the FALLBACK for a record that - * carries none — an object with no tenant column, or an organization-less + * carries NONE — an object with no tenant column, or an organization-less * row — the `sys_approval_request` writer's ruled shape (subject first, the * acting context second). It is a fact of the write, not a guess: a * principal is sharing a record from inside an organization. When neither * exists the grant carries `null` and the engine's #8844 rule decides. + * + * It is NOT the fallback for a record whose organization could not be READ. + * Those are different facts and {@link recordOrganization} keeps them + * apart: a record with no organization has nothing the session's could + * contradict, while a record whose read failed may well carry one — and a + * `single` install holding several organizations, or a `group` member + * active in a sibling organization, puts the session in a different one. + * Stamping the session's there writes the grant into an organization the + * record is not in: invisible to the record's organization under a + * tenant-scoped read, visible to the sibling's, and not NULL, so the + * backfill could never repair it. #8844's rule is that guessing a tenant is + * strictly worse than refusing, so a failed read yields `null` and the + * engine's ruled derive-or-refuse decides the row — derived on a + * single-organization install (where it can only be the record's), refused + * loudly (`ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED`) everywhere else. + * (The BLOCKING finding of this change's 2026-09-02 contract review.) */ private async resolveDirectGrantOrganization( input: GrantShareInput, context: ExecutionContext, ): Promise { - return (await this.recordOrganization(input.object, input.recordId)) - ?? activeOrganizationId(context); + const reading = await this.recordOrganization(input.object, input.recordId); + if (reading.kind === 'organization') return reading.organizationId; + if (reading.kind === 'none') return activeOrganizationId(context); + // `read-failed`: nothing is known about the record's organization, and + // the acting session's is not a stand-in for it — see above. + return null; } /** @@ -1352,15 +1395,19 @@ export class SharingService implements ISharingService { * hide the column from the decision — the same reading * {@link canManageShares} takes of the owner column. * - * `null` for an object with no tenant column, a record that is gone, an - * organization-less row, and a read that failed. The last is logged: a read - * that did not happen must not pass for a record with no organization, and - * the write it feeds still ends in the engine's ruled derive-or-refuse rather - * than in a silently stamped guess. + * Three readings, kept apart by {@link RecordOrganizationReading}: + * `organization` when the record carries one; `none` for an object with no + * tenant column, a record that is gone, or an organization-less row; and + * `read-failed` when the read threw. The last is logged and is NOT a `none`: + * a read that did not happen must not pass for a record with no + * organization — the direct path substitutes the acting session's + * organization for `none` and must never do so here — and the write it + * feeds ends in the engine's ruled derive-or-refuse rather than in a + * silently stamped guess. */ - private async recordOrganization(object: string, recordId: string): Promise { + private async recordOrganization(object: string, recordId: string): Promise { const tenantField = this.tenantFieldOf(object); - if (!tenantField) return null; + if (!tenantField) return NO_RECORD_ORGANIZATION; try { const rows = await this.engine.find(object, { where: { id: recordId }, @@ -1370,15 +1417,22 @@ export class SharingService implements ISharingService { }); const row: any = Array.isArray(rows) ? rows[0] : undefined; const value = row?.[tenantField]; - return typeof value === 'string' && value.trim() !== '' ? value : null; + return typeof value === 'string' && value.trim() !== '' + ? { kind: 'organization', organizationId: value } + : NO_RECORD_ORGANIZATION; } catch (err: any) { - // [#14484] The id stays out of the string: it reaches operators. + // [#14484] The id stays out of the string: it reaches operators. The + // text says what the failed read does NOT do — substitute the acting + // session's organization — because that is the one thing a reader of + // this line needs to know the row was spared. this.logger?.warn?.( '[sharing] could not read the shared record\'s organization — the grant carries none from it, ' - + 'and the engine\'s system-write organization rule decides the row', + + 'the acting session\'s organization is NOT substituted for the record\'s, ' + + 'and the engine\'s system-write organization rule decides the row ' + + '(derived on a single-organization install, refused on a walled one)', { object, recordId, error: err?.message }, ); - return null; + return RECORD_ORGANIZATION_READ_FAILED; } } From c415b592dfe5c9e6d29883a6c3674c205f0f8089 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 22:37:56 +0000 Subject: [PATCH 09/10] test(sharing): pin the reachable failed-read shapes and the measured RECORD_NOT_FOUND abort (#14484) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI on the previous head measured two things the first spelling of these pins assumed wrongly. 1. A plain `{ userId, tenantId: ORG_B }` session never reaches an ORG_A record through `grant`: `assertCanManageShares` -> `isRecordVisible` reads under the CALLER's context and the driver scopes it to `organization_id = :active OR IS NULL` on every posture. The reachable wrong-stamp shape is the `group` one — a multi-member owner active in the sibling organization, whose membership set the engine threads as `tenantIds` — plus the `single` install holding several organizations under a session carrying no active organization (nothing to stamp wrongly, but the failed read must end in the ambiguity refusal, never a NULL row). Both pinned with their read-working controls; `isolated` is unreachable (one organization per session). 2. Review §5 note 3 read the scoped update half's unreachable-row case as a SILENT no-op. Measured on the real engine it is loud: `ObjectQL.update` reports the unreachable row as RECORD_NOT_FOUND (404), `grant` throws it, and `evaluateRule`'s pass aborts there — grants before it stay, the ones after it and the stale-row revocation never run. Pinned as measured (the direct grant, the two-organizations path with a stale-row witness, the re-homed-record path); the implementation is unchanged, and the `grant` comment that called it silent now says what was measured. Census page re-anchored with `--fix` (4 line-rot anchors, 0 refused). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- content/docs/permissions/system-context.mdx | 6 +- .../record-share-organization-stamp.test.ts | 203 ++++++++++-------- .../plugin-sharing/src/sharing-service.ts | 6 +- 3 files changed, 120 insertions(+), 95 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 8f7b4c17d9..0d8e2164fa 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -130,10 +130,10 @@ The largest single consumer — **20 of the 109 sites**. |:--|:---|:---|:---| | 30 | **Sharing-rule grant materialisation is skipped on all four record-write hooks** | Lose: **no `sys_record_share` rows are created**. A fully configured sharing rule grants **nothing** on seeded data until a rule is re-evaluated or the boot backfill runs. This is the behaviour that motivated #4707. Since #6783 the skip is no longer silent — it emits an INFO notice (rough edge 2) | `rule-hooks.ts:250`, `:274`, `:293`, `:322` | | 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:677` | -| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:943`, `:1030`, `:1783` | +| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:943`, `:1030`, `:1787` | | 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1238` | -| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1472` (guard at `:1497`) | -| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1524` | +| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1476` (guard at `:1501`) | +| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1528` | | 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` | | 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:449`, `:503`, `:507`, `:580`, `:610` | | 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | diff --git a/packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts b/packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts index 89831de181..084cb03bbc 100644 --- a/packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts +++ b/packages/plugins/plugin-sharing/src/record-share-organization-stamp.test.ts @@ -353,11 +353,10 @@ describe('[#14484] the ledger flip alone would REFUSE walled-posture grants — describe("[#14484] a record read that FAILS never stamps the acting session's organization (2026-09-02 review, BLOCKING 1)", () => { /** * Fail ONLY the organization read — the one `SharingService.recordOrganization` - * projects to the tenant column. `canManageShares`' owner read (`['id', - * 'owner_id']`) keeps working, so the grant reaches the stamp instead of - * being refused at the pre-flight, and every other engine call — the - * `sys_record_share` upsert lookup, the engine's own `sys_organization` - * probe — is real. + * projects to the tenant column. Every other engine call stays real: the + * pre-flight's visibility read (caller context, `fields: ['id']`) and owner + * read (`['id', 'owner_id']`), the `sys_record_share` upsert lookup, and + * the engine's own `sys_organization` probe. */ function failOrganizationRead(ql: ObjectQL): () => number { let failed = 0; @@ -372,100 +371,111 @@ describe("[#14484] a record read that FAILS never stamps the acting session's or return () => failed; } - // The scenario the review names: the record lives in ORG_A, the caller acts - // from ORG_B — a `single` install holding several organizations, or a - // `group` member active in a sibling — and the read that would say ORG_A - // failed. The session's ORG_B is the wrong answer on every posture; what - // differs per posture is what the engine's #8844 rule does with the `null`. - - it("single posture, no organization yet: the row carries NULL — repairable by the backfill, which a stamped ORG_B never would be", async () => { - const { ql, sharing, shares } = await boot('single'); + // Which sessions can reach a record in ANOTHER organization at all — the + // precondition for a wrong stamp — is decided before the stamp, by the + // pre-flight's visibility read under the CALLER's context + // (`assertCanManageShares` → `isRecordVisible`), which the driver scopes to + // `organization_id = :active OR IS NULL` whenever the context carries an + // organization. So a plain `{ userId, tenantId: ORG_B }` session never sees + // `deal_a1` (ORG_A) on any posture, `single` included — measured: the first + // spelling of these pins died there with `NOT_FOUND`. Two shapes DO reach it: + // + // `group`, a multi-member owner active in ORG_B: the engine threads the + // membership set (`accessible_org_ids` → `DriverOptions.tenantIds`), so the + // record is visible while the ACTIVE organization is the sibling's — the + // review's scenario, and the one where the session's organization is the + // wrong answer. Before the fix this call wrote the row into ORG_B. + // + // `single` holding several organizations, a session carrying NO active + // organization: unscoped, so the record is visible. Nothing can be stamped + // wrongly here, but the failed read still has to end in the engine's + // ambiguity refusal rather than a NULL row. + // + // `isolated` cannot: a session there has exactly one organization. + + const GROUP_MEMBER_ACTIVE_IN_B = { + userId: 'u_a', + tenantId: ORG_B, + accessible_org_ids: [ORG_A, ORG_B], + } as unknown as ExecutionContext; + const NO_ACTIVE_ORGANIZATION = { userId: 'u_a' } as unknown as ExecutionContext; + + it('group posture, owner active in the sibling organization: refused loudly — never written into ORG_B', async () => { + const { ql, sharing, shares } = await boot('group'); const failures = failOrganizationRead(ql); - await sharing.grant( - { object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, - { userId: 'u_a', tenantId: ORG_B } as never, - ); + await expect( + sharing.grant({ object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, GROUP_MEMBER_ACTIVE_IN_B), + ).rejects.toMatchObject({ code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED', status: 500 }); expect(failures()).toBe(1); - expect(byRecord(await shares())).toEqual({ deal_a1: null }); + expect(await shares()).toEqual([]); + }); + + it("CONTROL: the same session with the read WORKING carries the record's ORG_A — the refusal above is the failed read's, not the posture's", async () => { + const { sharing, shares } = await boot('group'); + await sharing.grant({ object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, GROUP_MEMBER_ACTIVE_IN_B); + expect(byRecord(await shares())).toEqual({ deal_a1: ORG_A }); }); - it('single posture holding ORG_A and ORG_B: refused as ambiguous rather than written into the session\'s ORG_B', async () => { + it('single posture holding ORG_A and ORG_B, no active organization: refused as ambiguous — never a NULL row', async () => { const { ql, sharing, shares } = await boot('single', [ORG_A, ORG_B]); const failures = failOrganizationRead(ql); await expect( - sharing.grant( - { object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, - { userId: 'u_a', tenantId: ORG_B } as never, - ), + sharing.grant({ object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, NO_ACTIVE_ORGANIZATION), ).rejects.toMatchObject({ code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED', status: 500 }); expect(failures()).toBe(1); expect(await shares()).toEqual([]); }); - it.each(['isolated', 'group'] as const)( - "%s posture: refused loudly rather than written into the session's organization", - async (posture) => { - const { ql, sharing, shares } = await boot(posture); - const failures = failOrganizationRead(ql); - await expect( - sharing.grant( - { object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, - { userId: 'u_a', tenantId: ORG_B } as never, - ), - ).rejects.toMatchObject({ code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED', status: 500 }); - expect(failures()).toBe(1); - expect(await shares()).toEqual([]); - }, - ); - - it("CONTROL: the same session grant with the read WORKING carries the record's ORG_A — on a walled posture and with two organizations alike", async () => { - // The refusals above are the failed read's, not the posture's or the - // session's: with the read intact the identical call lands, in ORG_A. - const walled = await boot('group'); - await walled.sharing.grant({ object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, { userId: 'u_a', tenantId: ORG_B } as never); - expect(byRecord(await walled.shares())).toEqual({ deal_a1: ORG_A }); - - const several = await boot('single', [ORG_A, ORG_B]); - await several.sharing.grant({ object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, { userId: 'u_a', tenantId: ORG_B } as never); - expect(byRecord(await several.shares())).toEqual({ deal_a1: ORG_A }); + it("CONTROL: the same call with the read WORKING carries the record's ORG_A on the two-organization install", async () => { + const { sharing, shares } = await boot('single', [ORG_A, ORG_B]); + await sharing.grant({ object: OBJECT, recordId: 'deal_a1', recipientId: 'u_x' }, NO_ACTIVE_ORGANIZATION); + expect(byRecord(await shares())).toEqual({ deal_a1: ORG_A }); }); }); -describe('[#14484] the scoped UPDATE half cannot reach a row stamped with a DIFFERENT organization — a silent no-op, pinned as such (2026-09-02 review, §5 note 3)', () => { +describe('[#14484] the scoped UPDATE half cannot reach a row stamped with a DIFFERENT organization — measured: a loud RECORD_NOT_FOUND, not a silent no-op (2026-09-02 review, §5 note 3)', () => { // `SqlDriver.applyTenantScope` scopes the update half to // `organization_id = ? OR organization_id IS NULL`: a legacy NULL row is // reachable (pinned above), a row carrying ANOTHER organization is not — - // the wall, not a defect. What this block pins is the SHAPE of that no-op - // as the callers see it: `grant` returns `{ ...row, ...patch }` and the - // evaluator counts the call, while the stored row is untouched. Reachable - // through an organization-less record that two organizations' rules both - // match (the upsert key excludes `source_id`), or a record re-homed after a - // platform-global rule granted it; it self-heals only through the other - // rule's next revoke. The behaviour stands; the pin is so a reader of the - // counters does not mistake them for rows. + // the wall, not a defect. The review's note 3 read the consequence as a + // SILENT no-op (`grant` returning `{ ...row, ...patch }`, `reconcile` + // counting `updated += 1`). Measured on the real engine, it is not: the + // engine's update reports the unreachable row as `RECORD_NOT_FOUND` (404), + // `grant` throws it, and a reconcile pass that meets it ABORTS there — the + // grants written before it stay, the ones after it and the pass's stale-row + // revocations do not happen (`evaluateRule` has no per-grant catch; + // `evaluateAllRulesForObject` catches per RULE and logs). Loud beats a + // wrong count, so the behaviour stands; these pins hold the measured shape + // so the next reader does not inherit the note's reading. Reachable through + // an organization-less record that two organizations' rules both match (the + // upsert key excludes `source_id`), or a record re-homed after a + // platform-global rule granted it. const allShares = async (driver: SqlDriver) => (await driver.find('sys_record_share', {} as never)) as any[]; + const brief = (rows: any[]) => Object.fromEntries( + rows.map((r) => [r.record_id, { organization_id: r.organization_id ?? null, access_level: r.access_level, source_id: r.source_id ?? null }]), + ); - it("grant: the return value reports the patch; the stored row keeps the other organization's stamp and access level", async () => { + it("grant: throws RECORD_NOT_FOUND (404); the stored row keeps the other organization's stamp, level and source", async () => { const { driver, sharing } = await boot(); await driver.create('sys_record_share', { id: 'shr_org_a', object_name: OBJECT, record_id: 'deal_p1', recipient_type: 'user', recipient_id: 'u_x', access_level: 'read', source: 'rule', source_id: 'rule_org_a', organization_id: ORG_A, created_at: '2026-01-01T00:00:00Z', } as never); - const r = await sharing.grant( - { object: OBJECT, recordId: 'deal_p1', recipientId: 'u_x', accessLevel: 'edit', source: 'rule', sourceId: 'rule_org_b' }, - { ...SYSTEM, tenantId: ORG_B } as never, - ); - // What the caller is told… - expect(r).toMatchObject({ id: 'shr_org_a', access_level: 'edit', organization_id: ORG_B, source_id: 'rule_org_b' }); - // …and what is on disk: one row, ORG_A's, byte for byte as seeded. + await expect( + sharing.grant( + { object: OBJECT, recordId: 'deal_p1', recipientId: 'u_x', accessLevel: 'edit', source: 'rule', sourceId: 'rule_org_b' }, + { ...SYSTEM, tenantId: ORG_B } as never, + ), + ).rejects.toMatchObject({ code: 'RECORD_NOT_FOUND', status: 404, object: 'sys_record_share' }); + const rows = await allShares(driver); expect(rows).toHaveLength(1); expect(rows[0]).toMatchObject({ id: 'shr_org_a', access_level: 'read', organization_id: ORG_A, source_id: 'rule_org_a' }); }); - it("reconcile: two organizations' rules on one organization-less record — the second's grant is COUNTED, the row stays the first's", async () => { + it("reconcile: two organizations' rules on one organization-less record — the second's pass aborts at that record; grants before it stay, its stale revocation never runs", async () => { const { driver, rules, shares } = await boot(); const ruleA = await rules.defineRule( { name: 'os14484_note3_a', label: 'ORG_A won', object: OBJECT, criteria: WON, recipientType: 'user', recipientId: 'u_x', accessLevel: 'read' } as never, @@ -479,19 +489,32 @@ describe('[#14484] the scoped UPDATE half cannot reach a row stamped with a DIFF { name: 'os14484_note3_b', label: 'ORG_B won', object: OBJECT, criteria: WON, recipientType: 'user', recipientId: 'u_x', accessLevel: 'edit' } as never, ORG_B_ADMIN, ); - const result = await rules.evaluateRule(ruleB.id, ORG_B_ADMIN); + // A stale rule-B row on a record the rule does not match (`deal_a2` is + // `lost`): a completed pass revokes it. It is the witness that the pass + // never reached its revoke loop. + await driver.create('sys_record_share', { + id: 'shr_stale_b', object_name: OBJECT, record_id: 'deal_a2', recipient_type: 'user', recipient_id: 'u_x', + access_level: 'edit', source: 'rule', source_id: ruleB.id, organization_id: ORG_B, created_at: '2026-01-01T00:00:00Z', + } as never); + // The ORG_B sweep matches deal_b1, deal_b2 and — through the driver's - // compatibility arm — deal_p1. The evaluator keys its existing set by its - // OWN rule id, so all three are "created" to it; inside `grant`, deal_p1's - // upsert lookup (no `source_id` in the key) finds rule A's row and takes - // the update half, which the ORG_B scope cannot reach. - expect(result).toMatchObject({ matchedRecords: 3, grantsCreated: 3, grantsUpdated: 0 }); - const rows = await allShares(driver); - expect(rows).toHaveLength(4); // deal_a1, deal_b1, deal_b2, deal_p1 — no second row for deal_p1 either - expect(rows.find((r) => r.record_id === 'deal_p1')).toMatchObject({ organization_id: ORG_A, access_level: 'read', source_id: ruleA.id }); + // compatibility arm — deal_p1, in that order. The evaluator keys its + // existing set by its OWN rule id, so all three are "new" to it; inside + // `grant`, deal_p1's upsert lookup (no `source_id` in the key) finds rule + // A's ORG_A row and takes the update half, which the ORG_B scope cannot + // reach — and that is where the pass dies. + await expect(rules.evaluateRule(ruleB.id, ORG_B_ADMIN)).rejects.toMatchObject({ code: 'RECORD_NOT_FOUND', status: 404 }); + + expect(brief(await allShares(driver))).toEqual({ + deal_a1: { organization_id: ORG_A, access_level: 'read', source_id: ruleA.id }, + deal_p1: { organization_id: ORG_A, access_level: 'read', source_id: ruleA.id }, // rule A's row, untouched + deal_b1: { organization_id: ORG_B, access_level: 'edit', source_id: ruleB.id }, // written before the abort + deal_b2: { organization_id: ORG_B, access_level: 'edit', source_id: ruleB.id }, // written before the abort + deal_a2: { organization_id: ORG_B, access_level: 'edit', source_id: ruleB.id }, // stale, NOT revoked + }); }); - it("reconcile: a record re-homed after a platform-global rule granted it — the rule's next pass COUNTS an update its scoped write never lands", async () => { + it("reconcile: a record re-homed after a platform-global rule granted it — the rule's next pass aborts at it, and nothing in that pass lands", async () => { const { driver, rules } = await boot(); const define = (accessLevel: 'read' | 'edit') => rules.defineRule( { name: 'os14484_note3_rehome', label: 'Platform won', object: OBJECT, criteria: WON, recipientType: 'user', recipientId: 'u_x', accessLevel } as never, @@ -499,25 +522,23 @@ describe('[#14484] the scoped UPDATE half cannot reach a row stamped with a DIFF ); const rule = await define('read'); await rules.evaluateRule(rule.id, SYSTEM); - expect((await allShares(driver)).find((r) => r.record_id === 'deal_a1')).toMatchObject({ organization_id: ORG_A, access_level: 'read' }); + const before = brief(await allShares(driver)); + expect(before).toEqual({ + deal_a1: { organization_id: ORG_A, access_level: 'read', source_id: rule.id }, + deal_b1: { organization_id: ORG_B, access_level: 'read', source_id: rule.id }, + deal_b2: { organization_id: ORG_B, access_level: 'read', source_id: rule.id }, + deal_p1: { organization_id: null, access_level: 'read', source_id: rule.id }, + }); // The record moves to ORG_B; its grant row still says ORG_A (the // organization the record was in when the platform-global rule stamped it). await driver.update(OBJECT, 'deal_a1', { organization_id: ORG_B }); // The rule's level changes, so the next pass takes the update half for - // every matched record. deal_a1's resolves ORG_B from the re-homed record, - // and the ORG_B-scoped update cannot see the ORG_A row. + // every matched record. deal_a1 comes first: `grant` resolves ORG_B from + // the re-homed record, the ORG_B-scoped update cannot see the ORG_A row, + // and the pass dies before any other record is touched. await define('edit'); - const result = await rules.evaluateRule(rule.id, SYSTEM); - expect(result).toMatchObject({ matchedRecords: 4, grantsCreated: 0, grantsUpdated: 4 }); - - const rows = await allShares(driver); - const byId = Object.fromEntries(rows.map((r) => [r.record_id, { organization_id: r.organization_id ?? null, access_level: r.access_level }])); - expect(byId).toEqual({ - deal_a1: { organization_id: ORG_A, access_level: 'read' }, // counted, not landed - deal_b1: { organization_id: ORG_B, access_level: 'edit' }, - deal_b2: { organization_id: ORG_B, access_level: 'edit' }, - deal_p1: { organization_id: null, access_level: 'edit' }, - }); + await expect(rules.evaluateRule(rule.id, SYSTEM)).rejects.toMatchObject({ code: 'RECORD_NOT_FOUND', status: 404 }); + expect(brief(await allShares(driver))).toEqual(before); }); }); diff --git a/packages/plugins/plugin-sharing/src/sharing-service.ts b/packages/plugins/plugin-sharing/src/sharing-service.ts index ff3ee85913..4b7024507f 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.ts @@ -1283,7 +1283,11 @@ export class SharingService implements ISharingService { // what satisfies the driver's tenant audit. On this verb the driver's // scope keeps a NULL row in reach (`organization_id = ? OR IS NULL`) and, // exactly as on `sys_upload_session`, a row stamped with a DIFFERENT - // organization stays out of it — the wall, not a defect. + // organization stays out of it — the wall, not a defect. Out of reach + // is LOUD, not silent (measured 2026-09-02): the engine reports the + // unreachable row as `RECORD_NOT_FOUND` (404) and this call throws it, + // so a reconcile pass that meets one aborts there instead of counting + // an update it never landed (pinned in record-share-organization-stamp). await this.engine.update('sys_record_share', patch, { context: { ...SYSTEM_CTX, tenantId: organizationId ?? undefined }, }); From 8344d31b5c7de3b802f85b0a99b144d83544bf87 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 01:30:12 +0000 Subject: [PATCH 10/10] chore(census): re-derive the generated artefacts on the merged tree (#14484) The merge of origin/main conflicted in two generated census artefacts and silently dropped one side of a third. All three are resolved by regeneration, never by hand: node scripts/tenant-audit-census.mjs --write pnpm gen:system-context-census The tenant-audit population is the joint one: main's suspended-run-store row and this branch's backfill row both land, moving the census 218 -> 219 write call sites. The nine hand-written prose figures the gate holds to that census (check B) follow it; `scripts/engine-double-contract.pinned.json` needed no regeneration. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- content/docs/permissions/system-context.mdx | 10 +++--- .../docs/permissions/tenant-audit-census.mdx | 34 +++++++++---------- ...08-tenant-audit-write-call-sites.counts.md | 17 +++++----- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index b26e78e827..3b454189c4 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -129,11 +129,11 @@ The largest single consumer — **20 of the 109 sites**. | # | Behaviour when `isSystem` | What you get / what you lose | Anchor | |:--|:---|:---|:---| | 30 | **Sharing-rule grant materialisation is skipped on all four record-write hooks** | Lose: **no `sys_record_share` rows are created**. A fully configured sharing rule grants **nothing** on seeded data until a rule is re-evaluated or the boot backfill runs. This is the behaviour that motivated #4707. Since #6783 the skip is no longer silent — it emits an INFO notice (rough edge 2) | `rule-hooks.ts:250`, `:274`, `:293`, `:322` | -| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:654` | -| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:920`, `:1007`, `:1597` | -| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1208` | -| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1286` (guard at `:1311`) | -| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1338` | +| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:677` | +| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:943`, `:1030`, `:1787` | +| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1238` | +| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1476` (guard at `:1501`) | +| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1528` | | 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` | | 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:449`, `:503`, `:507`, `:580`, `:610` | | 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | diff --git a/content/docs/permissions/tenant-audit-census.mdx b/content/docs/permissions/tenant-audit-census.mdx index 9d95e28107..1425f813e1 100644 --- a/content/docs/permissions/tenant-audit-census.mdx +++ b/content/docs/permissions/tenant-audit-census.mdx @@ -98,7 +98,7 @@ are reported as `undecidable` rather than assumed either way. The same holds twice over for the context. An options argument spelled as a literal can be read; one spelled `options`, `{ ...opts }`, or handed through a -forwarding shim cannot, and **67 of the 218 sites are spelled that way**. A +forwarding shim cannot, and **67 of the 219 sites are spelled that way**. A context resolved from an inline literal or a local `const` can be tested for `isSystem`; one arriving from a helper call cannot. @@ -147,10 +147,10 @@ reproduce them. Where it disagrees, it disagrees on the page: | carried figure | where it survives | this census | | :--- | :--- | ---: | -| 175 write call sites | quoted in the merged changeset | **218** | +| 175 write call sites | quoted in the merged changeset | **219** | | 24 carrying no tenant context | quoted in the merged changeset | **9** provable and tenancy-enabled; **32** more whose options argument is unreadable | -| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **146 of 218** decidable, **72** undecidable | -| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 100 decidably elevated, 0 decidably not, 101 undecidable | +| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **147 of 219** decidable, **72** undecidable | +| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 101 decidably elevated, 0 decidably not, 101 undecidable | | 141 and 132, two independent re-derivations | the card that filed this work | — | **The differences are not reconciled, and deliberately so.** The old census's @@ -161,17 +161,17 @@ at any commit. Two structural facts do plausibly widen this reading against any hand or regex one, and both are counted in the generated tables below: the 45 sites reached -through an erased (`any`) receiver, and the 38 that name their object through a +through an erased (`any`) receiver, and the 39 that name their object through a `const` rather than inline. An instrument that read either the way a person does would report a smaller number and would not say so. The fourth row is the one worth flagging to anyone citing it. **The 135 / 77% figure has no surviving corroboration anywhere in the tree.** This census reads -100 of 218 (46%) as decidably elevated, with 101 more whose elevation is a +101 of 219 (46%) as decidably elevated, with 101 more whose elevation is a run-time fact — so the claim is neither confirmed nor refuted, and the honest answer is that a static reading cannot settle it. -⇒ **Cite `9 / 218`, and say what it is**: the sites whose options argument was +⇒ **Cite `9 / 219`, and say what it is**: the sites whose options argument was READ and holds no tenant context, against a decidably tenancy-enabled object. That is the control's provable yield surface. ⛔ Do not cite it as "the sites without tenant context" — **32 further sites** have an options argument this @@ -183,29 +183,29 @@ cannot read, and they are neither in nor out. | what | count | | :--- | ---: | -| write call sites on the application surface | **218** | -| …whose object name is statically decidable | 146 | +| write call sites on the application surface | **219** | +| …whose object name is statically decidable | 147 | | …whose object name is chosen at run time | 72 | -| …against an object with tenancy ENABLED | 146 | +| …against an object with tenancy ENABLED | 147 | | …against an object that declares tenancy off | 0 | -| threading a tenant context | 134 | +| threading a tenant context | 135 | | PROVABLY carrying none (options read, no context key) | **17** | | …of those, against a decidably tenancy-enabled object | **9** | | options argument UNREADABLE — may or may not carry one | 67 | | …of those, against a decidably tenancy-enabled object | 32 | -| threading a decidably ELEVATED (`isSystem`) context | 100 | +| threading a decidably ELEVATED (`isSystem`) context | 101 | | threading a context that is decidably NOT elevated | 0 | | threading a context whose elevation is a run-time fact | 101 | | how the instrument reached the site | count | | :--- | ---: | -| receiver carried a readable engine type | 173 | +| receiver carried a readable engine type | 174 | | receiver erased, placed by the object NAME | 19 | | receiver erased, placed by an `object: string` PARAMETER | 15 | | receiver erased, placed by an `UNTYPED_RECEIVERS` row | 11 | | object name spelled inline | 108 | -| object name spelled through a `const` | 38 | +| object name spelled through a `const` | 39 | | object name is an `object: string` parameter | 19 | | object name is some other run-time expression | 53 | @@ -224,12 +224,12 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-09-02 at `5daab8df0`. +Measured on 2026-09-03 at `98b1cf0b7`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 539 | -| engine-shaped types recognised | 56 | +| tracked non-test sources scanned | 540 | +| engine-shaped types recognised | 57 | | declared objects in the registry | 297 | | same-named calls subtracted as non-engine | 130 | diff --git a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md index fc16789018..39b4ad0b64 100644 --- a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md +++ b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md @@ -29,17 +29,17 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution. | Measure | Value | |---|---:| -| Write call sites | 218 | -| Object name statically decidable | 146 | +| Write call sites | 219 | +| Object name statically decidable | 147 | | Object name chosen at run time | 72 | -| Against a tenancy-enabled object | 146 | +| Against a tenancy-enabled object | 147 | | Against an object declaring tenancy off | 0 | -| Threading a tenant context | 134 | +| Threading a tenant context | 135 | | Provably carrying none | 17 | | …and decidably tenancy-enabled | 9 | | Options argument unreadable | 67 | | …and decidably tenancy-enabled | 32 | -| Threading a decidably elevated context | 100 | +| Threading a decidably elevated context | 101 | | Threading a decidably non-elevated context | 0 | | Threading a context of undecidable elevation | 101 | @@ -52,12 +52,12 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-09-02 at `5daab8df0`. +Measured on 2026-09-03 at `98b1cf0b7`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 539 | -| engine-shaped types recognised | 56 | +| tracked non-test sources scanned | 540 | +| engine-shaped types recognised | 57 | | declared objects in the registry | 297 | | same-named calls subtracted as non-engine | 130 | @@ -139,6 +139,7 @@ Measured on 2026-09-02 at `5daab8df0`. | `packages/plugins/plugin-security/src/suggested-audience-bindings.ts` | `insert` | `sys_audience_binding_suggestion` | enabled | context, elevation undecidable | 1 | | `packages/plugins/plugin-security/src/suggested-audience-bindings.ts` | `update` | `sys_audience_binding_suggestion` | enabled | context, elevation undecidable | 3 | | `packages/plugins/plugin-security/src/suggested-audience-bindings.ts` | `insert` | `sys_position_permission_set` | enabled | context, elevation undecidable | 1 | +| `packages/plugins/plugin-sharing/src/backfill-sys-record-share-organizations.ts` | `update` | `sys_record_share` | enabled | elevated | 1 | | `packages/plugins/plugin-sharing/src/primary-bu-projection.ts` | `update` | `sys_user` | enabled | elevated | 2 | | `packages/plugins/plugin-sharing/src/record-orphan-cleanup.ts` | `delete` | `table` | undecidable | options unreadable | 2 | | `packages/plugins/plugin-sharing/src/share-link-service.ts` | `insert` | `sys_share_link` | enabled | elevated | 1 |