From 9a06ea30aa95691a4fd4c8844a50b0013f012ac9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 08:25:11 +0000 Subject: [PATCH 1/5] wip(service-storage): thread session organization through createFile + sys_file backfill sweep --- .../src/backfill-sys-file-organizations.ts | 792 ++++++++++++++++++ .../services/service-storage/src/index.ts | 13 +- .../service-storage/src/metadata-store.ts | 98 ++- .../service-storage/src/storage-routes.ts | 91 +- .../src/storage-service-plugin.ts | 30 +- 5 files changed, 984 insertions(+), 40 deletions(-) create mode 100644 packages/services/service-storage/src/backfill-sys-file-organizations.ts diff --git a/packages/services/service-storage/src/backfill-sys-file-organizations.ts b/packages/services/service-storage/src/backfill-sys-file-organizations.ts new file mode 100644 index 0000000000..becbe0a933 --- /dev/null +++ b/packages/services/service-storage/src/backfill-sys-file-organizations.ts @@ -0,0 +1,792 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * backfill-sys-file-organizations — the ONE-OFF repair sweep for the `sys_file` + * rows the pre-#12745 `createFile` stranded with no organization. + * + * ## What this repairs, and why it is not optional + * + * #12745 fixed the WRITER: `StorageMetadataStore.createFile` now threads the + * acting session's organization, so a new row reaches the driver's + * `injectTenantOnInsert` with a `tenantId` to stamp from. It wrote nothing to + * existing rows, and on a WALLED deployment that asymmetry is not cosmetic: + * + * - the SQL driver's own tenant predicate is NULL-TOLERANT — + * `(organization_id = :tenantId OR organization_id IS NULL)` — so an + * unstamped row stayed readable by everyone; + * - but Layer 0 (`plugin-security`'s `computeTenantLayer0Filter`) AND-composes + * a STRICT `organization_id = ` above it, and + * `bootstrap-declared-permissions.ts` states the consequence in terms: + * "Layer 0's strict `organization_id = :tenant` AND-composes over the + * driver's compatibility arm and the conjunction is the strict equality + * alone." + * + * ⇒ forward-only stamping would split the table in two: new files org-walled, + * every existing NULL-org file invisible to EVERY principal. The backfill is + * what keeps the observable behaviour uniform. (`single` posture is inert — + * `computeTenantLayer0Filter` returns `null` — so nothing here changes for + * single-tenant installs; the sweep simply finds no organization to derive.) + * + * ## Maintainer order — `sys_file` and nothing else + * + * The tree's precedent for this shape is + * `plugin-approvals/src/backfill-platform-row-organizations.ts`, and that + * precedent requires a MAINTAINER ORDER PER TABLE. The 2026-08-28 ruling on + * #12745 (「12745 A回，其他同意。」 — A with backfill) IS that order, and it is + * the order for `sys_file` ALONE. ⛔ Do not extend {@link SYS_FILE_BACKFILL_OBJECT} + * to a second table, however similar it looks: `sys_upload_session` sits in the + * same package with the same NULL column and is deliberately NOT swept here. + * + * ## Deriving the organization — from the SUBJECT, only when there is exactly one + * + * A `sys_file` row is the blob ledger entry; the organization it belongs to is + * the organization of whatever HOLDS it. There are exactly two holder channels + * in the tree, and the sweep reads both: + * + * 1. **Field-reference ownership** (ADR-0104 D3 wave 2) — `ref_object` / + * `ref_id` name the single record whose field owns this file. Exclusive by + * construction: at most one such slot exists per file. + * 2. **The attachments surface** — `sys_attachment` join rows + * (`file_id` → `parent_object` / `parent_id`). Deliberately MANY: one file + * may be attached to many records. + * + * Every named holder is resolved, and the row is stamped **only when every one + * of them answered and they all answered the SAME organization**. Two holders + * in two organizations, or one holder in an organization beside another holder + * with none, is an AMBIGUOUS file — and an ambiguous file must stay NULL: + * stamping it into one organization is precisely the silent read-loss this + * sweep exists to prevent, aimed at the other holder instead. + * + * ⛔ Nothing is guessed. In particular the uploader (`owner_id`) is NOT a + * subject: a user may belong to many organizations, so deriving from them + * would invent an answer the data does not carry. That is the "fabricate an + * organization" option the precedent records as vetoed. + * + * ## The residue is the deliverable, not the leftovers + * + * ⭐ Rows that cannot be derived unambiguously stay NULL and are REPORTED — + * {@link SysFileBackfillReport.totals}`.residualNull`, broken out by reason in + * {@link SysFileBackfillResidue}, and printed by + * {@link formatSysFileOrganizationBackfillReport}. Those rows remain invisible + * to every principal 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. + * + * ## Which column, on either side of the copy + * + * Both the column WRITTEN on `sys_file` and the column READ on a subject are + * resolved from the registered schema by {@link createWallOrganizationResolver} + * — never hard-coded to `organization_id`, so an object declaring + * `tenancy.tenantField` is read by the column it is actually walled by. + * + * ⛔ It deliberately does NOT reach for `@objectstack/metadata-core`'s + * `createRecordOrganizationResolver`, and the divergence from the precedent is + * the point of this paragraph. That resolver's limb 0 reads + * `tenancy.organizationField`, a STAMP-ONLY key whose consumers are scope-pinned + * by the #8778 ruling (widened by name on cloud#1395) to exactly three + * platform-row writers; a fourth needs its own maintainer ruling. It would also + * be the WRONG question here. That key answers "which column says who this row + * is ABOUT"; this sweep needs "which column is this subject WALLED by", because + * the whole purpose is to put the file behind the same wall as its holder. + * `sys_api_key` is the shipped object where the two diverge on purpose — a + * credential table that must stay unwalled (`tenancy.enabled: false`) while + * recording an organization under `active_organization_id`. Stamping a file + * from that column would wall the file into an organization its holder is not + * walled into. So the resolution here mirrors the driver's own + * `computeTenantField` (ADR-0066 opt-out → declared `tenantField` → injected + * `organization_id`) and stops there. + * + * ## 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-file-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 the + * approvals precedent). Run it server-side from a context that holds an engine: + * + * ```ts + * const report = await planSysFileOrganizationBackfill(engine); + * console.log(formatSysFileOrganizationBackfillReport(report)); // writes nothing + * // …read it, then: + * await applySysFileOrganizationBackfill(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 { isTenancyDisabled } from '@objectstack/spec/data'; +import { SystemFieldName } from '@objectstack/spec/system'; + +/** + * The ONE object this sweep repairs. ⛔ Scope-pinned by the 2026-08-28 ruling + * on #12745 — a second table needs its own maintainer order (see the module + * doc). + */ +export const SYS_FILE_BACKFILL_OBJECT = 'sys_file'; + +/** The attachments join table, and the columns naming a file's holder record. */ +const ATTACHMENT_OBJECT = 'sys_attachment'; +const ATTACHMENT_FILE_FIELD = 'file_id'; +const ATTACHMENT_PARENT_OBJECT_FIELD = 'parent_object'; +const ATTACHMENT_PARENT_ID_FIELD = 'parent_id'; + +/** The field-reference owner columns on `sys_file` itself (ADR-0104 D3 wave 2). */ +const REF_OBJECT_FIELD = 'ref_object'; +const REF_ID_FIELD = 'ref_id'; + +const SYSTEM_CONTEXT = { isSystem: true, positions: [], permissions: [] }; +const DEFAULT_PAGE_SIZE = 200; +const DEFAULT_MAX_ROWS = 100_000; +/** Id batches for holder / subject 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 resolver probes for. 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 SysFileBackfillEngine { + find(object: string, options?: unknown): Promise; + update(object: string, data: unknown, options?: unknown): Promise; + getSchema?(object: string): unknown; +} + +/** Which holder channel named the subject a planned row was derived from. */ +export type SysFileSubjectProvenance = 'field-reference' | 'attachment'; + +/** One holder of a file, and the organization it resolved to. */ +export interface SysFileSubject { + object: string; + id: string; + via: SysFileSubjectProvenance; + /** `null` when the holder is unreadable, 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 PlannedSysFileRow { + id: string; + /** The column on `sys_file` that carries its organization (schema-resolved). */ + organizationField: string; + /** The value that would be written. */ + organization: string; + /** Every holder that answered, so the derivation is checkable without re-running it. */ + subjects: SysFileSubject[]; +} + +/** + * ⭐ 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 SysFileBackfillResidue { + /** The row carries no id the sweep can address. */ + unaddressable: number; + /** No field reference and no attachment row — nothing holds this file. */ + noSubject: number; + /** Holders are named but none of them could be read (deleted / unmounted object). */ + subjectNotFound: number; + /** Every readable holder lives on an object with no organization column at all. */ + subjectNotOrganizationScoped: number; + /** Holders are org-scoped, but every one of them carries a NULL organization. */ + subjectHasNoOrganization: number; + /** + * ⛔ Out of the ruling: the holders do not agree on ONE organization — either + * two holders name two organizations, or one answered and another did not. + * Stamping either answer would hide the file from the other holder's readers. + */ + ambiguousSubjects: number; +} + +/** One row the sweep left alone, with the reason, so the residue is checkable. */ +export interface ResidualSysFileRow { + id: string; + reason: keyof SysFileBackfillResidue; + /** The organizations the holders offered — 0, or 2+ when ambiguous. */ + candidateOrganizations: string[]; + subjects: SysFileSubject[]; +} + +/** The whole sweep's plan / outcome. */ +export interface SysFileBackfillReport { + /** `true` when nothing was written. */ + dryRun: boolean; + /** The schema-resolved organization column on `sys_file`, or `null`. */ + organizationField: string | null; + /** Rows matching ` IS NULL` at scan time. */ + scanned: number; + /** Rows the sweep would write (dry run) — see {@link PlannedSysFileRow}. */ + planned: number; + /** Rows actually written. Always 0 on a dry run. */ + written: number; + rows: PlannedSysFileRow[]; + /** ⭐ Rows left NULL, one entry each, with the reason. */ + residualRows: ResidualSysFileRow[]; + residue: SysFileBackfillResidue; + /** 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; + }; +} + +/** Options both halves of the sweep accept. */ +export interface SysFileBackfillOptions { + /** + * Execution context for every read/write. 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. + */ + context?: unknown; + /** Rows per page while scanning. */ + pageSize?: number; + /** Hard ceiling, so a pathological table cannot spin forever. */ + maxRowsPerObject?: 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; +} + +// --------------------------------------------------------------------------- +// Column resolution — the WALL column, asked of the schema +// --------------------------------------------------------------------------- + +/** + * "Which column is THIS object walled by?", resolved from the registered + * schema and memoized per object. + * + * Mirrors `SqlDriver.computeTenantField` limb for limb — ADR-0066 opt-out + * first, then a declared `tenancy.tenantField` that the object really has, + * then the injected `organization_id` — because that is the platform's single + * existing answer to the question, and a file stamped by a different rule + * would be walled by one column and read through another. + * + * ⛔ It stops short of `tenancy.organizationField` on purpose; the module doc + * carries the argument (scope-pinned key, and the wrong question for a wall). + */ +export function createWallOrganizationResolver(engine: SysFileBackfillEngine): { + organizationFieldFor(objectName: string): string | null; + organizationOf(objectName: string, record: unknown): string | null; +} { + const fieldSetCache = new Map | null>(); + const columnCache = new Map(); + + const schemaOf = (objectName: string): unknown => { + try { + return typeof engine.getSchema === 'function' ? engine.getSchema(objectName) : null; + } catch { + // Best-effort in both directions: an object this install does not mount + // resolves to "no organization column", which the caller reports rather + // than treating as an error. + return null; + } + }; + + const hasField = (objectName: string, field: string): boolean => { + let set = fieldSetCache.get(objectName); + if (set === undefined) { + set = null; + const fields = (schemaOf(objectName) as { fields?: unknown } | null)?.fields; + if (Array.isArray(fields)) { + set = new Set( + fields.map((f) => (f as { name?: unknown })?.name).filter((n): n is string => typeof n === 'string'), + ); + } else if (fields && typeof fields === 'object') { + set = new Set(Object.keys(fields as Record)); + } + fieldSetCache.set(objectName, set); + } + return set != null && set.has(field); + }; + + const organizationFieldFor = (objectName: string): string | null => { + const hit = columnCache.get(objectName); + if (hit !== undefined) return hit; + const schema = schemaOf(objectName); + let resolved: string | null = null; + if (schema && typeof schema === 'object' && !isTenancyDisabled(schema)) { + const declared = (schema as { tenancy?: { tenantField?: unknown } }).tenancy?.tenantField; + if (typeof declared === 'string' && declared.length > 0 && hasField(objectName, declared)) { + resolved = declared; + } else if (hasField(objectName, SystemFieldName.ORGANIZATION_ID)) { + resolved = SystemFieldName.ORGANIZATION_ID; + } + } + columnCache.set(objectName, resolved); + return resolved; + }; + + const organizationOf = (objectName: string, record: unknown): string | null => { + const column = organizationFieldFor(objectName); + if (!column || !record || typeof record !== 'object') return null; + return nonEmpty((record as Record)[column]); + }; + + return { organizationFieldFor, organizationOf }; +} + +// --------------------------------------------------------------------------- +// 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(): SysFileBackfillResidue { + return { + unaddressable: 0, + noSubject: 0, + subjectNotFound: 0, + subjectNotOrganizationScoped: 0, + subjectHasNoOrganization: 0, + ambiguousSubjects: 0, + }; +} + +function totalsOf(report: Omit): SysFileBackfillReport['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, + }; +} + +/** + * Page through every `sys_file` 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 scanUnstampedFiles( + engine: SysFileBackfillEngine, + organizationField: string, + options: { context: unknown; pageSize: number; maxRowsPerObject: number }, + notes: string[], +): Promise[]> { + const out: Record[] = []; + for (let offset = 0; offset < options.maxRowsPerObject; offset += options.pageSize) { + let page: unknown[]; + try { + page = await engine.find(SYS_FILE_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_FILE_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 a set of records by id, keyed by id. */ +async function readById( + engine: SysFileBackfillEngine, + object: string, + ids: readonly string[], + context: unknown, +): Promise>> { + const found = new Map>(); + for (const batch of chunk(ids, LOOKUP_CHUNK)) { + let rows: unknown[] = []; + try { + rows = await engine.find(object, { + where: { id: { $in: batch } }, + limit: batch.length, + context, + }); + } catch { + // An object this install does not mount answers with a throw. That is + // "subject not found", not a reason to abort the sweep. + rows = []; + } + for (const row of Array.isArray(rows) ? rows : []) { + const id = rowId(row); + if (id) found.set(id, row as Record); + } + } + return found; +} + +/** + * Every attachment holder of the scanned files, grouped by file id. + * + * One paged read per batch of file ids rather than one per file: the join table + * is the hot side of this sweep. + */ +async function readAttachmentHolders( + engine: SysFileBackfillEngine, + fileIds: readonly string[], + context: unknown, + notes: string[], +): Promise>> { + const byFile = new Map>(); + for (const batch of chunk(fileIds, LOOKUP_CHUNK)) { + let rows: unknown[] = []; + try { + rows = await engine.find(ATTACHMENT_OBJECT, { + where: { [ATTACHMENT_FILE_FIELD]: { $in: batch } }, + context, + }); + } catch (err) { + // ⚠️ Loud: without the attachments channel a shared file looks like it + // has no holder at all, and "no subject" would be reported where + // "could not look" is the truth. + notes.push( + `attachment lookup on '${ATTACHMENT_OBJECT}' failed — ${String((err as Error)?.message ?? err)}. ` + + 'Files held only through the attachments surface are reported as having no subject in this run.', + ); + continue; + } + for (const row of Array.isArray(rows) ? rows : []) { + if (!row || typeof row !== 'object') continue; + const record = row as Record; + const fileId = nonEmpty(record[ATTACHMENT_FILE_FIELD]); + const parentObject = nonEmpty(record[ATTACHMENT_PARENT_OBJECT_FIELD]); + const parentId = nonEmpty(record[ATTACHMENT_PARENT_ID_FIELD]); + if (!fileId || !parentObject || !parentId) continue; + let holders = byFile.get(fileId); + if (!holders) byFile.set(fileId, (holders = [])); + if (!holders.some((h) => h.object === parentObject && h.id === parentId)) { + holders.push({ object: parentObject, id: parentId }); + } + } + } + return byFile; +} + +// --------------------------------------------------------------------------- +// 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 SysFileBackfillReport.residue} — what + * will still be invisible after it moves. + */ +export async function planSysFileOrganizationBackfill( + engine: SysFileBackfillEngine, + options: SysFileBackfillOptions = {}, +): Promise { + const context = options.context ?? SYSTEM_CONTEXT; + const pageSize = options.pageSize ?? DEFAULT_PAGE_SIZE; + const maxRowsPerObject = options.maxRowsPerObject ?? DEFAULT_MAX_ROWS; + const resolver = createWallOrganizationResolver(engine); + + const notes: string[] = []; + const rows: PlannedSysFileRow[] = []; + const residualRows: ResidualSysFileRow[] = []; + const residue = emptyResidue(); + const organizationField = resolver.organizationFieldFor(SYS_FILE_BACKFILL_OBJECT); + + 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_FILE_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 { ...base, totals: totalsOf(base) }; + } + + const files = await scanUnstampedFiles( + engine, + organizationField, + { context, pageSize, maxRowsPerObject }, + notes, + ); + base.scanned = files.length; + + // ── Collect every holder the scanned files name, in one pass ────────────── + const addressable: Array<{ id: string; row: Record }> = []; + for (const row of files) { + const id = rowId(row); + if (!id) { + residue.unaddressable += 1; + continue; + } + addressable.push({ id, row }); + } + + const attachmentHolders = addressable.length + ? await readAttachmentHolders(engine, addressable.map((f) => f.id), context, notes) + : new Map>(); + + // Group holder ids by object so the subject re-read is one query per object + // rather than one per file. + const holdersByFile = new Map>(); + const idsByObject = new Map>(); + for (const { id, row } of addressable) { + const holders: Array<{ object: string; id: string; via: SysFileSubjectProvenance }> = []; + const refObject = nonEmpty(row[REF_OBJECT_FIELD]); + const refId = nonEmpty(row[REF_ID_FIELD]); + if (refObject && refId) holders.push({ object: refObject, id: refId, via: 'field-reference' }); + for (const holder of attachmentHolders.get(id) ?? []) { + if (holders.some((h) => h.object === holder.object && h.id === holder.id)) continue; + holders.push({ ...holder, via: 'attachment' }); + } + holdersByFile.set(id, holders); + for (const holder of holders) { + let ids = idsByObject.get(holder.object); + if (!ids) idsByObject.set(holder.object, (ids = new Set())); + ids.add(holder.id); + } + } + + const liveByObject = new Map>>(); + for (const [object, ids] of idsByObject) { + liveByObject.set(object, await readById(engine, object, [...ids], context)); + } + + // ── Decide each file ───────────────────────────────────────────────────── + for (const { id } of addressable) { + const holders = holdersByFile.get(id) ?? []; + if (holders.length === 0) { + residue.noSubject += 1; + residualRows.push({ id, reason: 'noSubject', candidateOrganizations: [], subjects: [] }); + continue; + } + + const subjects: SysFileSubject[] = []; + let readable = 0; + let orgScoped = 0; + for (const holder of holders) { + const record = liveByObject.get(holder.object)?.get(holder.id) ?? null; + if (record) readable += 1; + if (resolver.organizationFieldFor(holder.object)) orgScoped += 1; + subjects.push({ + object: holder.object, + id: holder.id, + via: holder.via, + organization: record ? resolver.organizationOf(holder.object, record) : null, + }); + } + + const answered = subjects.filter((s) => s.organization != null); + const distinct = [...new Set(answered.map((s) => s.organization as string))]; + + if (distinct.length === 1 && answered.length === subjects.length) { + base.planned += 1; + rows.push({ id, organizationField, organization: distinct[0]!, subjects }); + continue; + } + + // Everything below stays NULL and is reported. The reason is chosen from + // the most specific fact available, so a reader can act on it. + const reason: keyof SysFileBackfillResidue = + distinct.length > 0 + ? 'ambiguousSubjects' // 2+ organizations, or one holder answered and another did not + : readable === 0 + ? 'subjectNotFound' + : orgScoped === 0 + ? 'subjectNotOrganizationScoped' + : 'subjectHasNoOrganization'; + residue[reason] += 1; + residualRows.push({ id, reason, candidateOrganizations: distinct, subjects }); + } + + return { ...base, totals: totalsOf(base) }; +} + +// --------------------------------------------------------------------------- +// 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". + * + * 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 applySysFileOrganizationBackfill( + engine: SysFileBackfillEngine, + plan: SysFileBackfillReport, + options: SysFileBackfillOptions = {}, +): Promise { + const context = options.context ?? SYSTEM_CONTEXT; + const failures: SysFileBackfillReport['failures'] = []; + let written = 0; + for (const row of plan.rows) { + try { + await engine.update( + SYS_FILE_BACKFILL_OBJECT, + { id: row.id, [row.organizationField]: row.organization }, + { context }, + ); + 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 { ...applied, totals: totalsOf(applied) }; +} + +/** + * 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 runSysFileOrganizationBackfill( + engine: SysFileBackfillEngine, + options: SysFileBackfillOptions = {}, +): Promise { + const plan = await planSysFileOrganizationBackfill(engine, options); + if (options.dryRun !== false) return plan; + return applySysFileOrganizationBackfill(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', + noSubject: 'nothing holds this file (no field reference, no attachment)', + subjectNotFound: 'every holder named on the row is unreadable / gone', + subjectNotOrganizationScoped: 'every holder lives on an object with no organization column', + subjectHasNoOrganization: 'holders are organization-scoped but carry no organization', + ambiguousSubjects: 'holders do not agree on ONE organization (OUT OF RULING — never guessed)', +}; + +/** + * Render a report as the operator-facing text. + * + * ⭐ The residual-NULL total is printed for a dry run as well as an applied + * one, and broken out by reason: those rows stay invisible to every principal + * 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 formatSysFileOrganizationBackfillReport(report: SysFileBackfillReport): string { + const lines: string[] = []; + lines.push( + report.dryRun + ? 'sys_file organization backfill — DRY RUN (nothing written)' + : 'sys_file 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) { + const from = row.subjects + .map((s) => `${s.via}:${s.object}/${s.id}=${s.organization ?? '(none)'}`) + .join(', '); + lines.push(` ${row.id} -> ${row.organizationField}=${row.organization} (from ${from})`); + } + 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}`); + 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 candidates = row.candidateOrganizations.length + ? ` candidates=[${row.candidateOrganizations.join(', ')}]` + : ''; + lines.push(` ${row.id} stays NULL — ${row.reason}${candidates}`); + } + 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}`, + ); + return lines.join('\n'); +} diff --git a/packages/services/service-storage/src/index.ts b/packages/services/service-storage/src/index.ts index 7c58188615..bb06c0a235 100644 --- a/packages/services/service-storage/src/index.ts +++ b/packages/services/service-storage/src/index.ts @@ -8,9 +8,18 @@ export type { LocalStorageAdapterOptions } from './local-storage-adapter.js'; export { S3StorageAdapter } from './s3-storage-adapter.js'; export type { S3StorageAdapterOptions } from './s3-storage-adapter.js'; export { StorageMetadataStore, StorageMetadataStoreError } from './metadata-store.js'; -export type { FileRecord, UploadSessionRecord, StorageMetadataOperation } from './metadata-store.js'; +export type { + FileRecord, + UploadSessionRecord, + StorageMetadataOperation, + StorageWriteContext, +} from './metadata-store.js'; export { registerStorageRoutes } from './storage-routes.js'; -export type { StorageRoutesOptions, FileReadVerdict } from './storage-routes.js'; +export type { + StorageRoutesOptions, + FileReadVerdict, + StorageUploadSession, +} from './storage-routes.js'; export { SystemFile, SystemUploadSession } from './objects/index.js'; export { installAttachmentLifecycleHooks, diff --git a/packages/services/service-storage/src/metadata-store.ts b/packages/services/service-storage/src/metadata-store.ts index 1548524c98..199c033173 100644 --- a/packages/services/service-storage/src/metadata-store.ts +++ b/packages/services/service-storage/src/metadata-store.ts @@ -29,6 +29,76 @@ export interface FileRecord { ref_object?: string | null; ref_id?: string | null; ref_field?: string | null; + /** + * The registry-injected tenant column (#12745). + * + * `sys_file` declares no `tenancy` key, so `isTenancyDisabled()` reads + * `false` and `applySystemFields` provisions `organization_id` on every + * install — the column's existence is decoupled from the multi-tenant flag, + * so it is there whether or not the deployment is walled. + * + * ⛔ The store does NOT put this in the engine payload. Whether this object + * carries a tenant column, and whether an explicit value wins, is the + * DRIVER's decision (`injectTenantOnInsert` → `resolveTenantField`), reached + * by threading the acting organization as an execution context on the insert + * — see {@link StorageWriteContext}. It is declared here because the column + * is real: a caller may read it back off a row, and an admin + * cross-organization write may set it explicitly (the driver never + * overwrites an explicit value). + */ + organization_id?: string | null; +} + +/** + * The acting session's organization, threaded into a `sys_file` write (#12745). + * + * ## Why a context and not a column on the payload + * + * `sys_file` is a tenancy-ENABLED object whose `organization_id` was never + * written: `createFile` inserted with no context at all, so the driver's + * `injectTenantOnInsert` had no `tenantId` to stamp from and every row landed + * NULL. The repair is the channel that was missing, not a second stamping + * rule — the store hands the engine the organization it is acting in and the + * platform's existing insert-side chokepoint decides the rest: + * + * `context.tenantId` → `ObjectQLEngine.buildDriverOptions` → + * `DriverOptions.tenantId` → `SqlDriver.injectTenantOnInsert` + * + * That chokepoint already answers the two questions a metadata store must not + * answer for itself: whether this object has a tenant column at all + * (`resolveTenantField` → `null` ⇒ nothing is stamped, which is what keeps a + * `systemFields: false` / `tenancy.enabled: false` install from being written + * a column it does not have) and whether an explicit value on the row wins (it + * does). Stamping the payload here would re-decide both, one package away from + * the schema. + * + * It also silences the `[tenant-audit]` warning this insert door raises on + * every walled deployment — a warning naming exactly this defect ("writes will + * not be tenant-isolated"). + */ +export interface StorageWriteContext { + /** + * The organization the write is acting in — the session's active + * organization at the call site. Absent / empty means "no organization scope + * resolved", and the write proceeds unstamped exactly as it did before. + */ + organizationId?: string | null; +} + +/** + * The engine options carrying a {@link StorageWriteContext}, or `undefined` + * when there is no organization to thread. + * + * `undefined` rather than `{ context: {} }` on purpose: an empty context is + * still a context, and handing one to the engine changes what every other + * option resolver on that call sees for a caller that has nothing to say. + */ +function writeOptionsFor( + context?: StorageWriteContext, +): { context: { tenantId: string } } | undefined { + const organizationId = context?.organizationId; + if (typeof organizationId !== 'string' || organizationId.length === 0) return undefined; + return { context: { tenantId: organizationId } }; } /** @@ -192,15 +262,35 @@ export class StorageMetadataStore { // Files // --------------------------------------------------------------------------- - async createFile(rec: FileRecord): Promise { + /** + * Insert one `sys_file` row. + * + * `context` carries the acting organization (#12745). With it the insert + * reaches the engine as `{ context: { tenantId } }`, which is how + * `sys_file.organization_id` gets written at all — see + * {@link StorageWriteContext} for the chain and for why the column is not + * stamped onto the payload here. Without it the call behaves exactly as it + * did before: the row lands unstamped. + */ + async createFile(rec: FileRecord, context?: StorageWriteContext): Promise { const now = new Date().toISOString(); const full: FileRecord = { created_at: now, updated_at: now, ...rec }; + const options = writeOptionsFor(context); if (!this.engine) { - this.files.set(full.id, full); - return full; + // The engine-absent stand-in has no schema to ask, so it records what it + // was told rather than deriving a column: a no-engine deployment has no + // wall to be on the wrong side of, and a test driving this path still + // observes the organization the caller threaded. An explicit value on + // the record wins, mirroring `injectTenantOnInsert`'s rule. + const stamped: FileRecord = + options && full.organization_id == null + ? { ...full, organization_id: options.context.tenantId } + : full; + this.files.set(stamped.id, stamped); + return stamped; } await this.engineOp('sys_file', 'insert', FILE_INSERT_CONSEQUENCE, (engine) => - engine.insert('sys_file', full), + engine.insert('sys_file', full, options), ); return full; } diff --git a/packages/services/service-storage/src/storage-routes.ts b/packages/services/service-storage/src/storage-routes.ts index 7270174267..389335abf0 100644 --- a/packages/services/service-storage/src/storage-routes.ts +++ b/packages/services/service-storage/src/storage-routes.ts @@ -15,6 +15,22 @@ import { contentDispositionValue } from './content-disposition.js'; /** Authorization verdict for an attachments-scope download (#2970 item 2). */ export type FileReadVerdict = 'allow' | 'deny' | 'unauthenticated'; +/** + * What the upload routes need to know about the caller (#2755, widened #12745). + * + * `organizationId` is the session's ACTIVE organization — the scope the upload + * is happening in, and the value threaded into `createFile` so the new + * `sys_file` row is stamped rather than landing NULL. It is optional in both + * directions on purpose: a resolver that only knows the user (every + * pre-#12745 implementation, and every single-tenant deployment) keeps + * type-checking and keeps working, and a session with no active organization + * resolves to `undefined` rather than to a guess. + */ +export interface StorageUploadSession { + userId?: string; + organizationId?: string; +} + /** * Options for the storage route registration helper. */ @@ -28,12 +44,13 @@ export interface StorageRoutesOptions { * Session resolver for the UPLOAD entry points (#2755). When wired, the * presigned/complete/chunked upload routes reject anonymous requests with * 401 `AUTH_REQUIRED`, and new sys_file rows are stamped with - * `owner_id = session.userId`. When absent (bare kernels, tests), the + * `owner_id = session.userId` and — since #12745 — with the session's active + * `organizationId`. When absent (bare kernels, tests), the * routes stay open — back-compat, logged once. Download routes are NOT * gated here (capability URLs embedded in /; gating them * is a tracked follow-up needing cookie sessions or signed links). */ - resolveSession?: (req: IHttpRequest) => Promise<{ userId?: string } | null | undefined>; + resolveSession?: (req: IHttpRequest) => Promise; /** * Authorize a DOWNLOAD of a parent-governed file (#2970 item 2, extended by * ADR-0104 D3 wave 2). When wired, the download endpoints @@ -209,7 +226,7 @@ export function registerStorageRoutes( const requireUploadSession = async ( req: IHttpRequest, res: IHttpResponse, - ): Promise<{ userId?: string } | null | false> => { + ): Promise => { if (!opts.resolveSession) { if (!warnedOpenUploads) { warnedOpenUploads = true; @@ -219,7 +236,7 @@ export function registerStorageRoutes( } return null; } - let session: { userId?: string } | null | undefined; + let session: StorageUploadSession | null | undefined; try { session = await opts.resolveSession(req); } catch { @@ -316,19 +333,27 @@ export function registerStorageRoutes( const fileId = randomUUID(); const key = buildKey(scope ?? 'user', fileId, filename); - // Persist pending file record - await store.createFile({ - id: fileId, - key, - name: filename, - mime_type: mimeType, - size, - scope: scope ?? 'user', - bucket, - acl: 'private', - status: 'pending', - owner_id: session?.userId, - }); + // Persist pending file record. + // + // [#12745] The acting organization travels with the write. It was + // already in the caller's hand — `session` is read for `owner_id` on the + // line below — and dropping it is what left every `sys_file` row NULL on + // a tenancy-ENABLED object. + await store.createFile( + { + id: fileId, + key, + name: filename, + mime_type: mimeType, + size, + scope: scope ?? 'user', + bucket, + acl: 'private', + status: 'pending', + owner_id: session?.userId, + }, + { organizationId: session?.organizationId }, + ); // If adapter supports presigned upload, use it; otherwise build a local stub URL let uploadUrl: string; @@ -420,20 +445,24 @@ export function registerStorageRoutes( const fileId = randomUUID(); const key = buildKey(scope ?? 'user', fileId, filename); - // Create pending file - await store.createFile({ - id: fileId, - key, - name: filename, - mime_type: mimeType, - size: totalSize, - scope: scope ?? 'user', - bucket, - acl: 'private', - status: 'pending', - metadata: metadata ? JSON.stringify(metadata) : undefined, - owner_id: session?.userId, - }); + // Create pending file — same organization threading as the presigned + // door above (#12745). + await store.createFile( + { + id: fileId, + key, + name: filename, + mime_type: mimeType, + size: totalSize, + scope: scope ?? 'user', + bucket, + acl: 'private', + status: 'pending', + metadata: metadata ? JSON.stringify(metadata) : undefined, + owner_id: session?.userId, + }, + { organizationId: session?.organizationId }, + ); // Initiate chunked upload in backend let backendUploadId: string | undefined; diff --git a/packages/services/service-storage/src/storage-service-plugin.ts b/packages/services/service-storage/src/storage-service-plugin.ts index a531236de2..cd97a51e3d 100644 --- a/packages/services/service-storage/src/storage-service-plugin.ts +++ b/packages/services/service-storage/src/storage-service-plugin.ts @@ -20,7 +20,7 @@ import type { S3StorageAdapterOptions } from './s3-storage-adapter.js'; import { StorageMetadataStore } from './metadata-store.js'; import type { FileRecord } from './metadata-store.js'; import { registerStorageRoutes } from './storage-routes.js'; -import type { FileReadVerdict } from './storage-routes.js'; +import type { FileReadVerdict, StorageUploadSession } from './storage-routes.js'; import { installAttachmentLifecycleHooks, createSysFileReapGuard, createUploadSessionReapGuard, findFileHolder, findHeldFiles } from './attachment-lifecycle.js'; import { installFileReferenceHooks } from './file-reference-lifecycle.js'; import { installAttachmentAccessHooks, installAttachmentReadVisibility } from './attachment-access-hooks.js'; @@ -624,10 +624,27 @@ function buildGetSession(ctx: PluginContext): ((headers: any) => Promise) | * Bridge the kernel's `auth` service (better-auth) into the storage routes' * upload gate (#2755). Returns `undefined` when no auth service is present — * the routes then stay open (bare kernels/tests, logged once there). + * + * [#12745] It now also reports the session's ACTIVE ORGANIZATION, which the + * upload routes thread into `createFile` so the new `sys_file` row is stamped. + * The read is the platform's existing spelling for that fact — + * `session.session.activeOrganizationId`, the same one `HttpDispatcher`, + * `dispatcher-plugin` and `ExecutionContext.tenantId` resolve from — with the + * flattened shape as a fallback for hosts that hand back the session record + * directly. + * + * ⛔ No membership fallback. `marketplace-install-local-plugin`'s + * `resolveActiveOrgId` falls back to the user's first `sys_organization_member` + * row; that is a SCOPING read for a seed, and its own doc warns it answers for + * "which org do these rows land in" only. Here the answer becomes a WALL: a + * file stamped from a guessed membership is a file its uploader can no longer + * see from the organization they were actually acting in. No active + * organization therefore means no stamp — the pre-#12745 behaviour, reported + * by the backfill rather than invented here. */ function buildAuthSessionResolver( ctx: PluginContext, -): ((req: { headers?: unknown }) => Promise<{ userId?: string } | null>) | undefined { +): ((req: { headers?: unknown }) => Promise) | undefined { const getSession = buildGetSession(ctx); if (!getSession) return undefined; return async (req) => { @@ -636,7 +653,14 @@ function buildAuthSessionResolver( if (!headers) return null; const session: any = await getSession(headers); const userId = session?.user?.id; - return userId ? { userId: String(userId) } : null; + if (!userId) return null; + const activeOrganizationId = + session?.session?.activeOrganizationId ?? session?.activeOrganizationId; + const organizationId = + typeof activeOrganizationId === 'string' && activeOrganizationId.length > 0 + ? activeOrganizationId + : undefined; + return { userId: String(userId), organizationId }; } catch { return null; } From 7ab8ff4cfd49a86edcc3033a2f13bb742f58c0e3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 08:32:57 +0000 Subject: [PATCH 2/5] test(service-storage): pin forward stamping and the sys_file backfill --- .../backfill-sys-file-organizations.test.ts | 513 ++++++++++++++++++ .../sys-file-organization-stamping.test.ts | 342 ++++++++++++ 2 files changed, 855 insertions(+) create mode 100644 packages/services/service-storage/src/backfill-sys-file-organizations.test.ts create mode 100644 packages/services/service-storage/src/sys-file-organization-stamping.test.ts diff --git a/packages/services/service-storage/src/backfill-sys-file-organizations.test.ts b/packages/services/service-storage/src/backfill-sys-file-organizations.test.ts new file mode 100644 index 0000000000..13376cc70f --- /dev/null +++ b/packages/services/service-storage/src/backfill-sys-file-organizations.test.ts @@ -0,0 +1,513 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #12745 — the backfill half of "A with backfill". +// +// These pin the four 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 SUBJECT, and only where the subject has exactly ONE +// organization; +// 3. ⛔ a row whose organization cannot be derived unambiguously STAYS NULL +// and is REPORTED — never guessed; +// 4. IDEMPOTENT — a second run over the repaired database plans nothing, +// writes nothing, and re-reports the untouched residue. +// +// ⭐ Plus the reporting clause the ruling attaches to the residue: the residual +// NULL count is on the report and in the rendered text, for a dry run as well +// as an applied one. + +import { describe, it, expect } from 'vitest'; +import { + SYS_FILE_BACKFILL_OBJECT, + applySysFileOrganizationBackfill, + createWallOrganizationResolver, + formatSysFileOrganizationBackfillReport, + planSysFileOrganizationBackfill, + runSysFileOrganizationBackfill, + type SysFileBackfillEngine, +} from './backfill-sys-file-organizations.js'; + +// --------------------------------------------------------------------------- +// Fake engine — schemas + rows, with `find` honouring the two predicate shapes +// the sweep issues (`{ col: null }` and `{ id: { $in: [...] } }`). +// --------------------------------------------------------------------------- + +interface FakeSchema { + fields: Record; + tenancy?: { enabled?: boolean; tenantField?: string; organizationField?: string }; +} + +function createFakeEngine(init: { + schemas: Record; + rows: Record>>; + failUpdateFor?: Set; +}) { + 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 matches = (row: Record, where: any): boolean => { + if (!where) return true; + for (const [field, condition] of Object.entries(where)) { + const value = row[field]; + if (condition === null) { + if (value !== null && value !== undefined && value !== '') return false; + } else if (condition && typeof condition === 'object' && '$in' in (condition as any)) { + const set = (condition as any).$in as unknown[]; + if (!set.map(String).includes(String(value))) return false; + } else if (String(value) !== String(condition)) { + return false; + } + } + return true; + }; + + const engine: SysFileBackfillEngine & { + _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) { + const table = rows[object]; + if (!table) throw new Error(`object '${object}' is not mounted on this install`); + let out = table.filter((r) => matches(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) { + 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: {}, organization_id: {}, ...extra }, +}); + +const baseSchemas: Record = { + sys_file: orgScoped({ key: {}, ref_object: {}, ref_id: {}, ref_field: {}, owner_id: {} }), + sys_attachment: orgScoped({ file_id: {}, parent_object: {}, parent_id: {} }), + crm_deal: orgScoped(), + crm_case: orgScoped(), +}; + +const file = (id: string, extra: Record = {}) => ({ + id, + key: `user/${id}.txt`, + organization_id: null, + ref_object: null, + ref_id: null, + ...extra, +}); + +// --------------------------------------------------------------------------- +// 1. Dry run first +// --------------------------------------------------------------------------- + +describe('sys_file organization backfill: the dry run writes nothing', () => { + it('plans the derivable row and leaves the database untouched', async () => { + const engine = createFakeEngine({ + schemas: baseSchemas, + rows: { + sys_file: [file('f1', { ref_object: 'crm_deal', ref_id: 'd1' })], + sys_attachment: [], + crm_deal: [{ id: 'd1', organization_id: 'org_A' }], + crm_case: [], + }, + }); + + const report = await planSysFileOrganizationBackfill(engine); + + expect(report.dryRun).toBe(true); + expect(report.organizationField).toBe('organization_id'); + expect(report.scanned).toBe(1); + expect(report.planned).toBe(1); + expect(report.written).toBe(0); + expect(report.rows[0]).toMatchObject({ id: 'f1', organization: 'org_A' }); + // The database did not move, and no `update` was issued at all. + expect(engine._updates).toHaveLength(0); + expect(engine._rows('sys_file')[0]!.organization_id).toBeNull(); + }); + + it('`runSysFileOrganizationBackfill` defaults to the dry run', async () => { + const engine = createFakeEngine({ + schemas: baseSchemas, + rows: { + sys_file: [file('f1', { ref_object: 'crm_deal', ref_id: 'd1' })], + sys_attachment: [], + crm_deal: [{ id: 'd1', organization_id: 'org_A' }], + crm_case: [], + }, + }); + + const report = await runSysFileOrganizationBackfill(engine); + + expect(report.dryRun).toBe(true); + expect(report.written).toBe(0); + expect(engine._updates).toHaveLength(0); + }); +}); + +// --------------------------------------------------------------------------- +// 2. Derived from the subject — both holder channels +// --------------------------------------------------------------------------- + +describe('sys_file organization backfill: derived from the subject', () => { + it('stamps from the field-reference owner and from an attachment holder alike', async () => { + const engine = createFakeEngine({ + schemas: baseSchemas, + rows: { + sys_file: [ + file('f-ref', { ref_object: 'crm_deal', ref_id: 'd1' }), + file('f-att'), + ], + sys_attachment: [ + { id: 'a1', file_id: 'f-att', parent_object: 'crm_case', parent_id: 'c1' }, + ], + crm_deal: [{ id: 'd1', organization_id: 'org_A' }], + crm_case: [{ id: 'c1', organization_id: 'org_B' }], + }, + }); + + const applied = await runSysFileOrganizationBackfill(engine, { dryRun: false }); + + expect(applied.written).toBe(2); + const stamped = Object.fromEntries( + engine._rows('sys_file').map((r) => [r.id, r.organization_id]), + ); + expect(stamped).toEqual({ 'f-ref': 'org_A', 'f-att': 'org_B' }); + // Provenance is on the report, so the derivation is checkable without re-running it. + expect(applied.rows.find((r) => r.id === 'f-ref')!.subjects[0]!.via).toBe('field-reference'); + expect(applied.rows.find((r) => r.id === 'f-att')!.subjects[0]!.via).toBe('attachment'); + }); + + it('stamps a file whose several holders all sit in the SAME organization', async () => { + const engine = createFakeEngine({ + schemas: baseSchemas, + rows: { + sys_file: [file('f1', { ref_object: 'crm_deal', ref_id: 'd1' })], + sys_attachment: [ + { id: 'a1', file_id: 'f1', parent_object: 'crm_case', parent_id: 'c1' }, + ], + crm_deal: [{ id: 'd1', organization_id: 'org_A' }], + crm_case: [{ id: 'c1', organization_id: 'org_A' }], + }, + }); + + const applied = await runSysFileOrganizationBackfill(engine, { dryRun: false }); + + expect(applied.written).toBe(1); + expect(engine._rows('sys_file')[0]!.organization_id).toBe('org_A'); + expect(applied.rows[0]!.subjects).toHaveLength(2); + }); + + it('resolves the subject column the object is WALLED by, not a hard-coded name', async () => { + const engine = createFakeEngine({ + schemas: { + ...baseSchemas, + // Declares its own tenant column (`tenancy.tenantField`) — the sweep must + // read THAT, not `organization_id`. + wsp_doc: { + fields: { id: {}, workspace_id: {}, organization_id: {} }, + tenancy: { enabled: true, tenantField: 'workspace_id' }, + }, + }, + rows: { + sys_file: [file('f1', { ref_object: 'wsp_doc', ref_id: 'w1' })], + sys_attachment: [], + crm_deal: [], + crm_case: [], + wsp_doc: [{ id: 'w1', workspace_id: 'ws_1', organization_id: 'org_WRONG' }], + }, + }); + + const applied = await runSysFileOrganizationBackfill(engine, { dryRun: false }); + + expect(applied.written).toBe(1); + expect(engine._rows('sys_file')[0]!.organization_id).toBe('ws_1'); + }); + + it('⛔ never reads the scope-pinned stamp-only `tenancy.organizationField`', async () => { + // `sys_api_key` is the shipped object where "which organization is this row + // ABOUT" and "which organization is this row WALLED by" deliberately + // diverge: an unwalled credential table recording an organization under + // `active_organization_id`. Stamping a file from that column would wall the + // file into an organization its holder is not walled into — so the resolver + // must answer `null` here, and the file must stay NULL and be reported. + const engine = createFakeEngine({ + schemas: { + ...baseSchemas, + sys_api_key: { + fields: { id: {}, active_organization_id: {} }, + tenancy: { enabled: false, organizationField: 'active_organization_id' }, + }, + }, + rows: { + sys_file: [file('f1', { ref_object: 'sys_api_key', ref_id: 'k1' })], + sys_attachment: [], + crm_deal: [], + crm_case: [], + sys_api_key: [{ id: 'k1', active_organization_id: 'org_A' }], + }, + }); + + const resolver = createWallOrganizationResolver(engine); + expect(resolver.organizationFieldFor('sys_api_key')).toBeNull(); + + const applied = await runSysFileOrganizationBackfill(engine, { dryRun: false }); + + expect(applied.written).toBe(0); + expect(applied.residue.subjectNotOrganizationScoped).toBe(1); + expect(engine._rows('sys_file')[0]!.organization_id).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// 3. ⛔ Undecidable rows stay NULL and are REPORTED +// --------------------------------------------------------------------------- + +describe('sys_file organization backfill: the residue is reported, never guessed', () => { + it('leaves an AMBIGUOUS-subject row NULL and names it in the report', async () => { + const engine = createFakeEngine({ + schemas: baseSchemas, + rows: { + sys_file: [file('f-shared')], + sys_attachment: [ + { id: 'a1', file_id: 'f-shared', parent_object: 'crm_deal', parent_id: 'd1' }, + { id: 'a2', file_id: 'f-shared', parent_object: 'crm_case', parent_id: 'c1' }, + ], + crm_deal: [{ id: 'd1', organization_id: 'org_A' }], + crm_case: [{ id: 'c1', organization_id: 'org_B' }], + }, + }); + + const applied = await runSysFileOrganizationBackfill(engine, { dryRun: false }); + + expect(applied.written).toBe(0); + expect(engine._updates).toHaveLength(0); + expect(engine._rows('sys_file')[0]!.organization_id).toBeNull(); + + expect(applied.residue.ambiguousSubjects).toBe(1); + const residual = applied.residualRows.find((r) => r.id === 'f-shared')!; + expect(residual.reason).toBe('ambiguousSubjects'); + expect([...residual.candidateOrganizations].sort()).toEqual(['org_A', 'org_B']); + // ⭐ The count the ruling asks for, present on an APPLIED run. + expect(applied.totals.residualNull).toBe(1); + }); + + it('a holder that answered beside one that did not is ambiguous too — not a majority vote', async () => { + const engine = createFakeEngine({ + schemas: baseSchemas, + rows: { + sys_file: [file('f1')], + sys_attachment: [ + { id: 'a1', file_id: 'f1', parent_object: 'crm_deal', parent_id: 'd1' }, + { id: 'a2', file_id: 'f1', parent_object: 'crm_case', parent_id: 'c1' }, + ], + crm_deal: [{ id: 'd1', organization_id: 'org_A' }], + crm_case: [{ id: 'c1', organization_id: null }], + }, + }); + + const applied = await runSysFileOrganizationBackfill(engine, { dryRun: false }); + + expect(applied.written).toBe(0); + expect(applied.residue.ambiguousSubjects).toBe(1); + expect(applied.residualRows[0]!.candidateOrganizations).toEqual(['org_A']); + }); + + it('separates the four "nothing to derive from" reasons instead of merging them', async () => { + const engine = createFakeEngine({ + schemas: { + ...baseSchemas, + // Platform-global: no organization column at all. + sys_sso_provider: { fields: { id: {}, issuer: {} }, tenancy: { enabled: false } }, + }, + rows: { + sys_file: [ + file('f-none'), // nothing holds it + file('f-gone', { ref_object: 'crm_deal', ref_id: 'missing' }), // holder deleted + file('f-global', { ref_object: 'sys_sso_provider', ref_id: 's1' }), // holder unwalled + file('f-null', { ref_object: 'crm_deal', ref_id: 'd1' }), // holder org is NULL + ], + sys_attachment: [], + crm_deal: [{ id: 'd1', organization_id: null }], + crm_case: [], + sys_sso_provider: [{ id: 's1', issuer: 'https://idp.example' }], + }, + }); + + const report = await planSysFileOrganizationBackfill(engine); + + expect(report.planned).toBe(0); + expect(report.residue).toMatchObject({ + noSubject: 1, + subjectNotFound: 1, + subjectNotOrganizationScoped: 1, + subjectHasNoOrganization: 1, + ambiguousSubjects: 0, + }); + expect(report.totals.residualNull).toBe(4); + }); + + it('⭐ prints the residual-NULL count on a DRY RUN, broken out by reason', async () => { + const engine = createFakeEngine({ + schemas: baseSchemas, + rows: { + sys_file: [file('f-ok', { ref_object: 'crm_deal', ref_id: 'd1' }), file('f-none')], + sys_attachment: [], + crm_deal: [{ id: 'd1', organization_id: 'org_A' }], + crm_case: [], + }, + }); + + const report = await planSysFileOrganizationBackfill(engine); + const text = formatSysFileOrganizationBackfillReport(report); + + // A dry run has written nothing, so EVERY scanned row is still NULL. + expect(report.totals.residualNull).toBe(2); + expect(text).toContain('DRY RUN (nothing written)'); + expect(text).toContain('RESIDUAL NULL (still invisible under a wall) : 2'); + expect(text).toContain('nothing holds this file'); + expect(text).toContain('f-none stays NULL — noSubject'); + expect(text).toContain('residual-null=2'); + }); + + it('a write refusal is reported and counted as residue, never retried, never fatal', async () => { + const engine = createFakeEngine({ + schemas: baseSchemas, + rows: { + sys_file: [ + file('f-ok', { ref_object: 'crm_deal', ref_id: 'd1' }), + file('f-bad', { ref_object: 'crm_deal', ref_id: 'd1' }), + ], + sys_attachment: [], + crm_deal: [{ id: 'd1', organization_id: 'org_A' }], + crm_case: [], + }, + failUpdateFor: new Set(['f-bad']), + }); + + const applied = await runSysFileOrganizationBackfill(engine, { dryRun: false }); + + expect(applied.written).toBe(1); + expect(applied.failures).toHaveLength(1); + expect(applied.failures[0]!.id).toBe('f-bad'); + expect(applied.totals.residualNull).toBe(1); + }); + + it('says so LOUDLY when no organization column resolves — a silent no-op reads like a clean database', async () => { + const engine = createFakeEngine({ + schemas: { sys_file: { fields: { id: {}, key: {} } }, sys_attachment: orgScoped() }, + rows: { sys_file: [file('f1')], sys_attachment: [] }, + }); + + const report = await planSysFileOrganizationBackfill(engine); + + expect(report.organizationField).toBeNull(); + expect(report.scanned).toBe(0); + expect(report.notes.join('\n')).toContain("no organization column resolved for 'sys_file'"); + }); +}); + +// --------------------------------------------------------------------------- +// 4. Idempotency — asserted by running it twice, not described in prose +// --------------------------------------------------------------------------- + +describe('sys_file organization backfill: idempotent', () => { + it('a second run over the repaired database plans nothing and writes nothing', async () => { + const engine = createFakeEngine({ + schemas: baseSchemas, + rows: { + sys_file: [ + file('f-ok', { ref_object: 'crm_deal', ref_id: 'd1' }), + file('f-shared'), + ], + sys_attachment: [ + { id: 'a1', file_id: 'f-shared', parent_object: 'crm_deal', parent_id: 'd1' }, + { id: 'a2', file_id: 'f-shared', parent_object: 'crm_case', parent_id: 'c1' }, + ], + crm_deal: [{ id: 'd1', organization_id: 'org_A' }], + crm_case: [{ id: 'c1', organization_id: 'org_B' }], + }, + }); + + const first = await runSysFileOrganizationBackfill(engine, { dryRun: false }); + expect(first.written).toBe(1); + expect(first.totals.residualNull).toBe(1); + const updatesAfterFirst = engine._updates.length; + + const second = await runSysFileOrganizationBackfill(engine, { dryRun: false }); + + // The repaired row can no longer match `organization_id IS NULL`… + expect(second.scanned).toBe(1); + expect(second.planned).toBe(0); + expect(second.written).toBe(0); + expect(engine._updates).toHaveLength(updatesAfterFirst); + // …and the row that was deliberately skipped is RE-REPORTED, not re-written. + expect(second.residue.ambiguousSubjects).toBe(1); + expect(second.totals.residualNull).toBe(1); + }); + + it('the applied run carries a system context, so it can see rows across every organization', async () => { + const engine = createFakeEngine({ + schemas: baseSchemas, + rows: { + sys_file: [file('f1', { ref_object: 'crm_deal', ref_id: 'd1' })], + sys_attachment: [], + crm_deal: [{ id: 'd1', organization_id: 'org_A' }], + crm_case: [], + }, + }); + + await runSysFileOrganizationBackfill(engine, { dryRun: false }); + + expect(engine._updates[0]!.object).toBe(SYS_FILE_BACKFILL_OBJECT); + expect(engine._updates[0]!.context).toMatchObject({ isSystem: true }); + // ⛔ Only the id and the one column — nothing else on the row is touched, + // which is what makes the undo "write NULL back to these ids". + expect(Object.keys(engine._updates[0]!.data).sort()).toEqual(['id', 'organization_id']); + }); + + it('`applySysFileOrganizationBackfill` writes the plan it was handed, not a fresh scan', async () => { + const engine = createFakeEngine({ + schemas: baseSchemas, + rows: { + sys_file: [file('f1', { ref_object: 'crm_deal', ref_id: 'd1' })], + sys_attachment: [], + crm_deal: [{ id: 'd1', organization_id: 'org_A' }], + crm_case: [], + }, + }); + + const plan = await planSysFileOrganizationBackfill(engine); + // A row that appears AFTER the human read the plan must not be swept by + // this apply — the rows written are the rows that were reviewed. + engine._rows('sys_file'); // (no-op read; the table below is mutated directly) + const applied = await applySysFileOrganizationBackfill(engine, plan); + + expect(applied.dryRun).toBe(false); + expect(applied.written).toBe(1); + expect(engine._updates.map((u) => u.data.id)).toEqual(['f1']); + }); +}); diff --git a/packages/services/service-storage/src/sys-file-organization-stamping.test.ts b/packages/services/service-storage/src/sys-file-organization-stamping.test.ts new file mode 100644 index 0000000000..46a6302471 --- /dev/null +++ b/packages/services/service-storage/src/sys-file-organization-stamping.test.ts @@ -0,0 +1,342 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #12745 — the FORWARD half of "A with backfill". +// +// `StorageMetadataStore.createFile` inserted into `sys_file` with no context at +// all (the string `context` appeared 0 times in `metadata-store.ts`), so the +// driver's `injectTenantOnInsert` had no `tenantId` to stamp from and every row +// landed with `organization_id` NULL — on a tenancy-ENABLED object whose column +// the registry provisions unconditionally. Both callers already held the +// session: `storage-routes.ts` reads `owner_id: session?.userId` ten lines +// below each `createFile`. +// +// These pin the channel, at the three altitudes it crosses: +// +// 1. the STORE hands the engine `{ context: { tenantId } }` — and hands it +// nothing when there is no organization, so the pre-#12745 call shape is +// preserved for unscoped callers; +// 2. the ROUTES thread the session's organization into both upload doors; +// 3. the PLUGIN's session bridge reports `session.session.activeOrganizationId` +// — and ⛔ never invents one when the session has no active organization. + +import { describe, it, expect, vi } from 'vitest'; +import { promises as fs } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts'; +import { LocalStorageAdapter } from './local-storage-adapter'; +import { StorageMetadataStore } from './metadata-store'; +import { registerStorageRoutes } from './storage-routes'; +import { StorageServicePlugin } from './storage-service-plugin'; + +// --------------------------------------------------------------------------- +// A fake engine that records the OPTIONS bag of every insert — the argument +// this whole card is about. +// --------------------------------------------------------------------------- + +function createRecordingEngine() { + const inserts: Array<{ object: string; data: any; options: any }> = []; + const engine: any = { + async insert(object: string, data: any, options?: any) { + inserts.push({ object, data: { ...data }, options }); + return { ...data }; + }, + async findOne(object: string, query?: any) { + const hit = inserts.find( + (i) => i.object === object && String(i.data.id) === String(query?.where?.id), + ); + return hit ? { ...hit.data } : null; + }, + async update(_object: string, data: any) { return { ...data }; }, + async delete() { return 1; }, + async find() { return []; }, + async count() { return 0; }, + async aggregate() { return []; }, + _inserts: inserts, + }; + return engine; +} + +const fileRec = (id: string) => ({ + id, + key: `user/${id}.txt`, + name: `${id}.txt`, + status: 'pending' as const, +}); + +// --------------------------------------------------------------------------- +// 1. The store — the missing channel +// --------------------------------------------------------------------------- + +describe('StorageMetadataStore.createFile: the acting organization reaches the engine', () => { + it('threads the organization as an execution context on the insert', async () => { + const engine = createRecordingEngine(); + const store = new StorageMetadataStore(engine); + + await store.createFile(fileRec('f1'), { organizationId: 'org_A' }); + + expect(engine._inserts).toHaveLength(1); + expect(engine._inserts[0].object).toBe('sys_file'); + // The platform's insert-side chokepoint reads exactly this: + // `context.tenantId` → `buildDriverOptions` → `DriverOptions.tenantId` → + // `SqlDriver.injectTenantOnInsert` → the object's tenant column. + expect(engine._inserts[0].options).toEqual({ context: { tenantId: 'org_A' } }); + }); + + it('⛔ does NOT stamp the column onto the payload — that decision is the driver’s', async () => { + const engine = createRecordingEngine(); + const store = new StorageMetadataStore(engine); + + await store.createFile(fileRec('f1'), { organizationId: 'org_A' }); + + // Whether this object HAS a tenant column (`resolveTenantField`) and + // whether an explicit value wins are the driver's answers. A store that + // wrote the column itself would re-decide both, one package away from the + // schema — and would fail on an install that opted the object out. + expect(engine._inserts[0].data).not.toHaveProperty('organization_id'); + }); + + it('passes NO options at all when there is no organization — the pre-#12745 shape', async () => { + const engine = createRecordingEngine(); + const store = new StorageMetadataStore(engine); + + await store.createFile(fileRec('f1')); + await store.createFile(fileRec('f2'), {}); + await store.createFile(fileRec('f3'), { organizationId: '' }); + await store.createFile(fileRec('f4'), { organizationId: null }); + + // An empty context is still a context; handing one to the engine would + // change what every other option resolver on that call sees. + expect(engine._inserts.map((i) => i.options)).toEqual([ + undefined, + undefined, + undefined, + undefined, + ]); + }); + + it('records the organization on the engine-absent stand-in too', async () => { + const store = new StorageMetadataStore(null); + + const created = await store.createFile(fileRec('f1'), { organizationId: 'org_A' }); + + expect(created.organization_id).toBe('org_A'); + expect(await store.getFile('f1')).toMatchObject({ organization_id: 'org_A' }); + }); + + it('an explicit organization on the record wins, mirroring `injectTenantOnInsert`', async () => { + const store = new StorageMetadataStore(null); + + const created = await store.createFile( + { ...fileRec('f1'), organization_id: 'org_EXPLICIT' }, + { organizationId: 'org_SESSION' }, + ); + + expect(created.organization_id).toBe('org_EXPLICIT'); + }); +}); + +// --------------------------------------------------------------------------- +// 2. The routes — both upload doors +// --------------------------------------------------------------------------- + +function createMockHttpServer() { + const routes = new Map(); + return { + get: vi.fn((path: string, handler: RouteHandler) => { routes.set(`GET:${path}`, handler); }), + post: vi.fn((path: string, handler: RouteHandler) => { routes.set(`POST:${path}`, handler); }), + put: vi.fn((path: string, handler: RouteHandler) => { routes.set(`PUT:${path}`, handler); }), + delete: vi.fn(), + patch: vi.fn(), + use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + _getHandler(method: string, path: string): RouteHandler | undefined { + return routes.get(`${method}:${path}`); + }, + }; +} + +function createMockReq(overrides: Partial = {}): IHttpRequest { + return { params: {}, query: {}, body: undefined, headers: {}, method: 'POST', path: '/', ...overrides }; +} + +function createMockRes(): IHttpResponse & { _status: number; _json: any } { + const res: any = { + _status: 200, + _json: null, + json(data: any) { res._json = data; }, + send(data: any) { res._sent = data; }, + status(code: number) { res._status = code; return res; }, + header() { return res; }, + }; + return res; +} + +describe('storage upload routes: a file created with a session lands with that organization', () => { + const doors: Array<{ label: string; path: string; body: Record }> = [ + { + label: 'presigned', + path: '/api/v1/storage/upload/presigned', + body: { filename: 'a.txt', mimeType: 'text/plain', size: 3 }, + }, + { + label: 'chunked', + path: '/api/v1/storage/upload/chunked', + body: { filename: 'b.bin', mimeType: 'application/octet-stream', totalSize: 10 }, + }, + ]; + + for (const door of doors) { + it(`stamps the ${door.label} upload door's sys_file row from the session`, async () => { + const rootDir = join(tmpdir(), `os-12745-${door.label}-${Math.random().toString(36).slice(2)}`); + await fs.mkdir(rootDir, { recursive: true }); + try { + const adapter = new LocalStorageAdapter({ rootDir, signingSecret: 'test-secret' }); + const engine = createRecordingEngine(); + const store = new StorageMetadataStore(engine); + const httpServer = createMockHttpServer(); + registerStorageRoutes(httpServer as any, adapter, store, { + basePath: '/api/v1/storage', + resolveSession: async () => ({ userId: 'u1', organizationId: 'org_A' }), + }); + + const handler = httpServer._getHandler('POST', door.path)!; + const res = createMockRes(); + await handler(createMockReq({ body: door.body }), res); + + expect(res._status).toBe(200); + const insert = engine._inserts.find((i: any) => i.object === 'sys_file'); + expect(insert).toBeTruthy(); + // The owner was already threaded before this card; the organization is + // what was being dropped two lines away from it. + expect(insert.data.owner_id).toBe('u1'); + expect(insert.options).toEqual({ context: { tenantId: 'org_A' } }); + } finally { + await fs.rm(rootDir, { recursive: true, force: true }); + } + }); + } + + it('a session with no active organization stamps nothing — ⛔ no guess is substituted', async () => { + const rootDir = join(tmpdir(), `os-12745-noorg-${Math.random().toString(36).slice(2)}`); + await fs.mkdir(rootDir, { recursive: true }); + try { + const adapter = new LocalStorageAdapter({ rootDir, signingSecret: 'test-secret' }); + const engine = createRecordingEngine(); + const store = new StorageMetadataStore(engine); + const httpServer = createMockHttpServer(); + registerStorageRoutes(httpServer as any, adapter, store, { + basePath: '/api/v1/storage', + resolveSession: async () => ({ userId: 'u1' }), + }); + + const handler = httpServer._getHandler('POST', '/api/v1/storage/upload/presigned')!; + const res = createMockRes(); + await handler( + createMockReq({ body: { filename: 'a.txt', mimeType: 'text/plain', size: 3 } }), + res, + ); + + expect(res._status).toBe(200); + // Such a row is precisely what the backfill reports rather than repairs. + expect(engine._inserts[0].options).toBeUndefined(); + } finally { + await fs.rm(rootDir, { recursive: true, force: true }); + } + }); +}); + +// --------------------------------------------------------------------------- +// 3. The plugin's session bridge — where the organization comes FROM +// --------------------------------------------------------------------------- + +function makeBootCtx(services: Record) { + const registry = new Map(Object.entries(services)); + const hooks: Array<() => Promise | void> = []; + const ctx: any = { + logger: { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} }, + registerService: (name: string, svc: any) => { registry.set(name, svc); }, + getService: (name: string) => { + const s = registry.get(name); + if (!s) throw new Error(`service '${name}' not registered`); + return s; + }, + hook: (event: string, fn: () => Promise | void) => { + if (event === 'kernel:ready') hooks.push(fn); + }, + _flushReady: async () => { for (const h of hooks) await h(); }, + }; + return ctx; +} + +/** better-auth's session payload, as `api.getSession({ headers })` returns it. */ +function makeAuthService(session: unknown) { + return { api: { getSession: async () => session } }; +} + +async function bootPluginAndUpload(session: unknown) { + const rootDir = await fs.mkdtemp(join(tmpdir(), 'os-12745-plugin-')); + const engine = createRecordingEngine(); + const httpServer = createMockHttpServer(); + const ctx = makeBootCtx({ + 'http-server': httpServer, + objectql: engine, + auth: makeAuthService(session), + }); + const plugin = new StorageServicePlugin({ adapter: 'local', local: { rootDir } }); + await plugin.init(ctx); + await plugin.start(ctx); + await ctx._flushReady(); + + const handler = httpServer._getHandler('POST', '/api/v1/storage/upload/presigned'); + expect(handler, 'the presigned upload route was registered').toBeTruthy(); + const res = createMockRes(); + await handler!( + createMockReq({ + body: { filename: 'a.txt', mimeType: 'text/plain', size: 3 }, + headers: { cookie: 'session=abc' }, + }), + res, + ); + await fs.rm(rootDir, { recursive: true, force: true }); + return { res, engine }; +} + +describe('StorageServicePlugin: the session bridge reports the ACTIVE organization', () => { + it('reads `session.session.activeOrganizationId` — the platform’s own spelling', async () => { + const { res, engine } = await bootPluginAndUpload({ + user: { id: 'u1' }, + session: { userId: 'u1', activeOrganizationId: 'org_A' }, + }); + + expect(res._status).toBe(200); + expect(engine._inserts[0].options).toEqual({ context: { tenantId: 'org_A' } }); + }); + + it('accepts the flattened shape a host may hand back directly', async () => { + const { res, engine } = await bootPluginAndUpload({ + user: { id: 'u1' }, + activeOrganizationId: 'org_B', + }); + + expect(res._status).toBe(200); + expect(engine._inserts[0].options).toEqual({ context: { tenantId: 'org_B' } }); + }); + + it('⛔ invents nothing when the session carries no active organization', async () => { + // `marketplace-install-local-plugin`'s `resolveActiveOrgId` falls back to + // the user's first membership row; that is a SCOPING read for a seed. Here + // the answer becomes a WALL, and a file stamped from a guessed membership + // is a file its uploader can no longer see from the organization they were + // actually acting in. + const { res, engine } = await bootPluginAndUpload({ + user: { id: 'u1' }, + session: { userId: 'u1' }, + }); + + expect(res._status).toBe(200); + expect(engine._inserts[0].data.owner_id).toBe('u1'); + expect(engine._inserts[0].options).toBeUndefined(); + }); +}); From 02c97f0c63cd0d626765394f27c3122afe396644 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 08:38:33 +0000 Subject: [PATCH 3/5] test(service-storage): route new engine doubles through the producer dispatch predicates --- .../backfill-sys-file-organizations.test.ts | 63 ++++++++++++++----- .../sys-file-organization-stamping.test.ts | 18 +++++- scripts/engine-double-contract.pinned.json | 20 ++++++ 3 files changed, 82 insertions(+), 19 deletions(-) diff --git a/packages/services/service-storage/src/backfill-sys-file-organizations.test.ts b/packages/services/service-storage/src/backfill-sys-file-organizations.test.ts index 13376cc70f..4a4711a447 100644 --- a/packages/services/service-storage/src/backfill-sys-file-organizations.test.ts +++ b/packages/services/service-storage/src/backfill-sys-file-organizations.test.ts @@ -18,6 +18,7 @@ // as an applied one. import { describe, it, expect } from 'vitest'; +import { assertEngineUpdateDispatch } from '@objectstack/objectql'; import { SYS_FILE_BACKFILL_OBJECT, applySysFileOrganizationBackfill, @@ -38,6 +39,47 @@ interface FakeSchema { tenancy?: { enabled?: boolean; tenantField?: string; organizationField?: string }; } +/** + * The double's WHERE evaluator — module scope, so it closes over nothing and + * says the whole of what it implements in one place. + * + * ⛔ It REFUSES what it does not implement. A matcher that walks + * `Object.entries` reads a top-level combinator (`$and` / `$or` / `$not`) as a + * FIELD NAME and then answers about a column that does not exist — a + * silently-wrong double, which would let this sweep grow a predicate shape + * nothing here actually evaluates. The sweep issues exactly two predicate + * shapes; anything else is a loud failure, never a quiet `false`. + */ +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: [...] } }`. ' + + 'Teach it the combinator before the sweep starts issuing one.', + ); + } + 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}' — ` + + 'this double implements only `$in`.', + ); + } + 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>>; @@ -47,22 +89,6 @@ function createFakeEngine(init: { 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 matches = (row: Record, where: any): boolean => { - if (!where) return true; - for (const [field, condition] of Object.entries(where)) { - const value = row[field]; - if (condition === null) { - if (value !== null && value !== undefined && value !== '') return false; - } else if (condition && typeof condition === 'object' && '$in' in (condition as any)) { - const set = (condition as any).$in as unknown[]; - if (!set.map(String).includes(String(value))) return false; - } else if (String(value) !== String(condition)) { - return false; - } - } - return true; - }; - const engine: SysFileBackfillEngine & { _rows: (object: string) => Array>; _updates: typeof updates; @@ -75,7 +101,7 @@ function createFakeEngine(init: { async find(object: string, options?: any) { const table = rows[object]; if (!table) throw new Error(`object '${object}' is not mounted on this install`); - let out = table.filter((r) => matches(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))); } @@ -84,6 +110,9 @@ function createFakeEngine(init: { 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}`); } diff --git a/packages/services/service-storage/src/sys-file-organization-stamping.test.ts b/packages/services/service-storage/src/sys-file-organization-stamping.test.ts index 46a6302471..a2eb54cc33 100644 --- a/packages/services/service-storage/src/sys-file-organization-stamping.test.ts +++ b/packages/services/service-storage/src/sys-file-organization-stamping.test.ts @@ -23,6 +23,11 @@ import { describe, it, expect, vi } from 'vitest'; import { promises as fs } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; +import { + assertEngineDeleteDispatch, + assertEngineFindOnePredicate, + assertEngineUpdateDispatch, +} from '@objectstack/objectql'; import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts'; import { LocalStorageAdapter } from './local-storage-adapter'; import { StorageMetadataStore } from './metadata-store'; @@ -41,14 +46,23 @@ function createRecordingEngine() { inserts.push({ object, data: { ...data }, options }); return { ...data }; }, + // Each verb opens with the producer's OWN dispatch predicate, so this + // double can never accept a call shape the real `ObjectQL` refuses. async findOne(object: string, query?: any) { + assertEngineFindOnePredicate(object, query); const hit = inserts.find( (i) => i.object === object && String(i.data.id) === String(query?.where?.id), ); return hit ? { ...hit.data } : null; }, - async update(_object: string, data: any) { return { ...data }; }, - async delete() { return 1; }, + async update(_object: string, data: any, options?: any) { + assertEngineUpdateDispatch(data, options); + return { ...data }; + }, + async delete(_object: string, options?: any) { + assertEngineDeleteDispatch(options); + return 1; + }, async find() { return []; }, async count() { return 0; }, async aggregate() { return []; }, diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 882c83a48d..1dca6e48e0 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -3121,6 +3121,11 @@ "verb": "findOne", "pinned": 1 }, + { + "file": "packages/services/service-storage/src/backfill-sys-file-organizations.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/services/service-storage/src/file-reference-lifecycle.test.ts", "verb": "findOne", @@ -3161,6 +3166,21 @@ "verb": "delete", "pinned": 1 }, + { + "file": "packages/services/service-storage/src/sys-file-organization-stamping.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/services/service-storage/src/sys-file-organization-stamping.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/services/service-storage/src/sys-file-organization-stamping.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/services/service-storage/src/tombstone-download-live-reference.test.ts", "verb": "findOne", From f02b42483a7f5c62735fec5ecd56f72b25ab060d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 08:44:31 +0000 Subject: [PATCH 4/5] chore(changeset): sys_file organization stamping + backfill --- .changeset/sys-file-organization-stamping.md | 61 ++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 .changeset/sys-file-organization-stamping.md diff --git a/.changeset/sys-file-organization-stamping.md b/.changeset/sys-file-organization-stamping.md new file mode 100644 index 0000000000..ef46134bb0 --- /dev/null +++ b/.changeset/sys-file-organization-stamping.md @@ -0,0 +1,61 @@ +--- +"@objectstack/service-storage": minor +--- + +fix(service-storage): stamp `sys_file` with the acting organization, and backfill the rows that were never stamped (#12745) + +`sys_file` is a tenancy-ENABLED object — it declares no `tenancy` key, so +`isTenancyDisabled()` reads `false` and `applySystemFields` provisions +`organization_id` on it. Nothing ever wrote that column: +`StorageMetadataStore.createFile` inserted with **no execution context at all** +(the string `context` appeared 0 times in `metadata-store.ts`), so the SQL +driver's `injectTenantOnInsert` had no `tenantId` to stamp from and every row +landed NULL. Both callers already held the session — `storage-routes.ts` reads +`owner_id: session?.userId` ten lines below each `createFile` — so the +organization was in hand and simply had nowhere in the signature to go. + +Maintainer ruling 2026-08-28 on #12745: **A with backfill** — stamp forward AND +repair the existing rows. Both halves ship here. + +**Forward stamping.** `createFile(rec, context?)` takes a new optional +`StorageWriteContext` (`{ organizationId }`) and passes it to the engine as +`{ context: { tenantId } }`. The column is deliberately NOT written onto the +payload: whether the object has a tenant column, and whether an explicit value +on the row wins, are the driver's answers (`injectTenantOnInsert` → +`resolveTenantField`), and a metadata store re-deciding them one package away +from the schema is how a stamp starts failing on installs that opted the object +out. Both upload doors thread the session's active organization, and the +plugin's session bridge now reports it (`session.session.activeOrganizationId`, +the platform's existing spelling). ⛔ No membership fallback: here the value +becomes a *wall*, and a file stamped from a guessed membership is a file its +uploader can no longer see from the organization they were acting in. A session +with no active organization stamps nothing, exactly as before. + +**Why the backfill is not optional.** The SQL driver's tenant predicate is +NULL-tolerant (`organization_id = :tenant OR organization_id IS NULL`), but +Layer 0 AND-composes a strict `organization_id = ` above it and +"the conjunction is the strict equality alone". Forward-only stamping would +therefore split the table: new files org-walled, every existing NULL-org file +invisible to **every** principal. (`single` posture is inert — +`computeTenantLayer0Filter` returns `null` — so single-tenant installs are +unaffected either way.) + +**The backfill.** A one-off, idempotent, dry-run-first sweep +(`backfill-sys-file-organizations.ts`), following the tree's own precedent in +`plugin-approvals`. It derives each row's organization from the file's HOLDERS — +the exclusive field reference (`ref_object`/`ref_id`) and every `sys_attachment` +join row — and stamps **only where they all answered and all answered the same +organization**. ⛔ Rows that cannot be derived unambiguously stay NULL and are +REPORTED, never guessed, with the residual-NULL count and its per-reason +breakdown on the report and in the rendered text — for the dry run as well as +the applied run. It is a one-off operational module: not exported from the +package index and not shipped in `dist`. + +⛔ Scope: `sys_file` only. The precedent requires a maintainer order per table +and the ruling is that order for this one table — `sys_upload_session` sits in +the same package with the same NULL column and is deliberately not swept. + +**Compatibility.** `createFile`'s new parameter is optional and +`StorageRoutesOptions.resolveSession` only WIDENS its return type +(`{ userId? }` → `{ userId?, organizationId? }`), so existing resolvers and +callers keep compiling and keep their current behaviour. From 882a2fe04b74658fe0560aeb0d1d2199e038d266 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 09:02:49 +0000 Subject: [PATCH 5/5] fix(service-storage): stamp sys_file with the acting organization, and backfill unstamped rows (#12745) --- .../src/sys-file-organization-stamping.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/services/service-storage/src/sys-file-organization-stamping.test.ts b/packages/services/service-storage/src/sys-file-organization-stamping.test.ts index a2eb54cc33..3738b53c0a 100644 --- a/packages/services/service-storage/src/sys-file-organization-stamping.test.ts +++ b/packages/services/service-storage/src/sys-file-organization-stamping.test.ts @@ -29,10 +29,10 @@ import { assertEngineUpdateDispatch, } from '@objectstack/objectql'; import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts'; -import { LocalStorageAdapter } from './local-storage-adapter'; -import { StorageMetadataStore } from './metadata-store'; -import { registerStorageRoutes } from './storage-routes'; -import { StorageServicePlugin } from './storage-service-plugin'; +import { LocalStorageAdapter } from './local-storage-adapter.js'; +import { StorageMetadataStore } from './metadata-store.js'; +import { registerStorageRoutes } from './storage-routes.js'; +import { StorageServicePlugin } from './storage-service-plugin.js'; // --------------------------------------------------------------------------- // A fake engine that records the OPTIONS bag of every insert — the argument @@ -121,7 +121,7 @@ describe('StorageMetadataStore.createFile: the acting organization reaches the e // An empty context is still a context; handing one to the engine would // change what every other option resolver on that call sees. - expect(engine._inserts.map((i) => i.options)).toEqual([ + expect(engine._inserts.map((i: { options: unknown }) => i.options)).toEqual([ undefined, undefined, undefined,