From 0876822bb93094572a14a1b9a1fd3cc8ea00fd36 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 03:46:34 +0000 Subject: [PATCH 1/6] fix(storage)!: a record write claims or copies only a file its caller may read The file-reference claim path asks the download door's own read judgement, under the write's caller context, before it copies or claims a referenced file. A file the caller may not read is answered as one the organization-scoped lookup cannot see: the value is kept as written, nothing is claimed or copied, and the field's declared constraints are not judged against it. System and context-less writes are unchanged. The door's judgement moves into one function both the door and the lifecycle call, so the two cannot diverge. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016DTRymn12to8YSGMcTzS7h --- .../src/attachment-access-hooks.ts | 7 +- .../src/file-reference-lifecycle.ts | 135 +++++++++++-- .../src/storage-service-plugin.ts | 187 ++++++++++++------ 3 files changed, 248 insertions(+), 81 deletions(-) diff --git a/packages/services/service-storage/src/attachment-access-hooks.ts b/packages/services/service-storage/src/attachment-access-hooks.ts index c258cb47d12..aea52d91205 100644 --- a/packages/services/service-storage/src/attachment-access-hooks.ts +++ b/packages/services/service-storage/src/attachment-access-hooks.ts @@ -268,7 +268,10 @@ function asIdList(id: unknown): Array | null { * context resolved for the object of the CURRENT operation, which here is * `sys_attachment`. Every gate in this module asks about the PARENT record's * object, never about `sys_attachment`, so carrying any of them across is one - * object's widening applied to another object's question. + * object's widening applied to another object's question. [#25692] The + * file-reference lifecycle reads a record write's caller through the same + * {@link callerContext}, for the same reason: it asks about a FILE's owning + * record, never about the object being written. * * [#7145] The general rule — which keys those are, why they are dropped by * PREFIX rather than by a name list, and why the copy is load-bearing in both @@ -318,7 +321,7 @@ function asIdList(id: unknown): Array | null { * object's grant. Where that judgement is not served, record sharing is asked * with no depth, so its owner-match stays at its narrowest (`own`) — the safe * direction. */ -function callerContext(ctx: any): ExecutionContext { +export function callerContext(ctx: any): ExecutionContext { const exec = ctx?.input?.options?.context; if (exec && typeof exec === 'object') { return withoutOperationPrivateKeys(exec as Record); diff --git a/packages/services/service-storage/src/file-reference-lifecycle.ts b/packages/services/service-storage/src/file-reference-lifecycle.ts index 319ad0b2f3f..5ba76857c12 100644 --- a/packages/services/service-storage/src/file-reference-lifecycle.ts +++ b/packages/services/service-storage/src/file-reference-lifecycle.ts @@ -3,6 +3,8 @@ import { randomUUID } from 'node:crypto'; import { FILE_REFERENCE_TYPES, isFileIdToken, RAW_FILE_VALUES_CONTEXT_KEY } from '@objectstack/spec/data'; import type { IStorageService } from '@objectstack/spec/contracts'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { callerContext } from './attachment-access-hooks.js'; /** * Field-reference file ownership (ADR-0104 D3 wave 2). @@ -31,8 +33,24 @@ import type { IStorageService } from '@objectstack/spec/contracts'; * 2. **Read authorisation derives from exactly one parent record**, not the * union of every referrer's. Copying a private record's file id into a * world-readable record — the sort of thing generated code does without - * any visible mistake — cannot widen who can read the bytes, because the - * second record gets its own copy. + * any visible mistake — cannot widen who can read the ORIGINAL, because + * the second record gets its own copy. + * + * ### A write references only a file its writer may read + * + * [#25692] A copy is new bytes the writing record's readers can read, and a + * claim hands a file to the writing record's readers. So neither may run for + * a writer who could not read the file in the first place — otherwise holding + * a file's id would be holding its bytes, which is exactly what ADR-0104 D3's + * governed download ended. The question is the download door's own, asked of + * the write's caller: {@link FileReferenceReadAuthorizer}, which the storage + * plugin binds to the door's verdict rather than restating it here. A file the + * writer may not read is answered exactly as one the organization-scoped + * lookup cannot see ({@link systemReadContext}): the value is kept as written, + * nothing is claimed or copied, and its field's `accept` / `maxSize` are not + * judged against it, so the write learns nothing about the file. System + * writes and context-less programmatic calls carry no caller to ask, and + * reference files as before. * * ### The one write that cannot honour it, and is refused * @@ -169,6 +187,61 @@ function actingOrganizationOf(ctx: any): string | null { return typeof org === 'string' && org.length > 0 ? org : null; } +/** + * [#25692] "May the caller of this record write read this file?" — the + * download door's verdict on the file's bytes, asked of the write's caller. + * + * Supplied by the storage plugin, which binds it to the door's own judgement + * (`storage-service-plugin.ts`), and never decided in this module: one file, + * one access rule, so the bytes, the metadata and a reference to them are + * judged by one predicate. `true` admits; `false` refuses. A throw is not a + * verdict — the plugin's binding lets only an authorization-store outage + * through, and it propagates, so an unreadable store fails the write rather + * than reading as either answer. + */ +export type FileReferenceReadAuthorizer = ( + file: Record, + context: ExecutionContext, +) => Promise; + +/** + * The caller to ask {@link FileReferenceReadAuthorizer} about, or `null` when + * there is none: a context-less programmatic call (no session — a bare kernel, + * a seed) or a system write (engine self-writes, migrations, a `runAs: + * 'system'` hook or flow). Neither has a principal whose read could be judged, + * and the attachment gates exempt both on the same grounds. An anonymous + * caller IS a caller: a session with no user, refused by the door for every + * file it does not serve anonymously. + */ +function referencingCallerOf(ctx: any): ExecutionContext | null { + if (!ctx?.session || ctx.session.isSystem) return null; + return callerContext(ctx); +} + +/** + * The `sys_file` row a field's id token names, as THIS write may reference it, + * or `null`. Both halves of the claim path — copy-on-claim before the write, + * the claim after it — ask through here, so they cannot disagree about which + * file a write may take. + * + * 1. The row as the acting organization reaches it ({@link systemReadContext}). + * 2. [#25692] Then, when the plugin bound a read verdict and the write has a + * caller, whether that caller may read it. A file it may not read is + * answered as the lookup answers a file it cannot see. + */ +async function findReferenceableFile( + engine: FileReferenceEngine, + fileId: string, + organizationId: string | null, + caller: ExecutionContext | null, + authorizeRead: FileReferenceReadAuthorizer | undefined, +): Promise | null> { + const row = await engine.findOne('sys_file', { where: { id: fileId }, context: systemReadContext(organizationId) }); + if (!row) return null; + if (!caller || !authorizeRead) return row; + return (await authorizeRead(row, caller)) ? row : null; +} + /** Bound on owned files released per record delete. */ const RELEASE_BATCH_LIMIT = 1_000; @@ -513,7 +586,9 @@ async function copyOwnedFile( * read of the same rows. * * `recordId` is null on insert: a new record can never be the current owner, - * so every already-owned id is copied. + * so every already-owned id is copied. [#25692] `caller` is the write's caller + * ({@link referencingCallerOf}): a file it may not read is neither copied nor + * judged against the field's declaration. */ async function applyCopyOnClaim( engine: FileReferenceEngine, @@ -524,6 +599,8 @@ async function applyCopyOnClaim( data: Record, fileFields: string[], organizationId: string | null, + caller: ExecutionContext | null, + authorizeRead: FileReferenceReadAuthorizer | undefined, ): Promise { for (const field of fileFields) { if (!(field in data)) continue; @@ -533,17 +610,17 @@ async function applyCopyOnClaim( const replacements = new Map(); for (const token of [...new Set(tokens)]) { - const row = await engine.findOne('sys_file', { - where: { id: token }, - context: systemReadContext(organizationId), - }); + const row = await findReferenceableFile(engine, token, organizationId, caller, authorizeRead); // Unknown id: not a file this platform manages (external/legacy value), - // or not one the acting organization can reach (`systemReadContext`). - // Left verbatim — the read resolver ignores it for the same reason. + // not one the acting organization can reach (`systemReadContext`), or + // [#25692] not one this write's caller may read. Left verbatim — the + // read resolver ignores it for the same reason. if (!row) continue; - // The file is real and about to be referenced by this field, so its - // declared constraints bind now — before ownership, before any copy. + // The file is real, this caller may read it, and it is about to be + // referenced by this field, so its declared constraints bind now — + // before ownership, before any copy. [#25692] Never before the read + // verdict: the refusal names the file's size or type. assertFileConstraints(fieldDefOf(engine, object, field), field, row); // Unclaimed: the after-hook will claim it for this slot. Nothing to copy. @@ -592,9 +669,10 @@ async function applyCopyOnClaim( * a second record pointing at bytes it does not own is visible to * `verify-references`, whereas a silently transferred owner would not be. * - * The file is looked up as the acting organization reaches it - * ({@link systemReadContext}), the same lookup copy-on-claim made before the - * write: a file outside that reach is not claimed. + * The file is looked up as copy-on-claim looked it up before the write + * ({@link findReferenceableFile}): as the acting organization reaches it and, + * [#25692] when the write has a caller, as that caller may read it. A file + * outside either is not claimed. */ async function claimFile( engine: FileReferenceEngine, @@ -604,9 +682,11 @@ async function claimFile( recordId: string, field: string, organizationId: string | null, + caller: ExecutionContext | null, + authorizeRead: FileReferenceReadAuthorizer | undefined, ): Promise { - const row = await engine.findOne('sys_file', { where: { id: fileId }, context: systemReadContext(organizationId) }); - if (!row) return; // not a platform-managed file the acting organization reaches — nothing to own + const row = await findReferenceableFile(engine, fileId, organizationId, caller, authorizeRead); + if (!row) return; // not a platform-managed file this write may reference — nothing to own if (row.ref_id != null && !isSameSlot(row, object, recordId, field)) { logger.warn( `[storage] file reference: ${fileId} is already owned by ` + @@ -727,11 +807,18 @@ async function ownedBySlot( * Install the ownership hooks. Global (no object filter): any object may * declare a file-class field, and {@link activeFileFields} exits immediately * for those that do not. + * + * [#25692] `authorizeRead` is the download door's read verdict, bound by the + * storage plugin: with it, a write references only a file its caller may read + * (see the module header). Absent — a bare kernel, a test of the bookkeeping + * alone — no caller is asked, which is also where the door itself binds no + * gate and serves every file. */ export function installFileReferenceHooks( engine: FileReferenceEngine, getStorage: () => IStorageService | null | undefined, logger: FileReferenceLogger, + authorizeRead?: FileReferenceReadAuthorizer, ): void { // ── beforeInsert / beforeUpdate: copy-on-claim ──────────────────── // Runs before the driver write so the persisted value is already the id @@ -744,7 +831,10 @@ export function installFileReferenceHooks( if (!object || !data || typeof data !== 'object') return; const fileFields = activeFileFields(engine, object); if (fileFields.length === 0) return; - await applyCopyOnClaim(engine, getStorage, logger, object, null, data, fileFields, actingOrganizationOf(ctx)); + await applyCopyOnClaim( + engine, getStorage, logger, object, null, data, fileFields, + actingOrganizationOf(ctx), referencingCallerOf(ctx), authorizeRead, + ); }, { packageId: PACKAGE_ID }, ); @@ -798,7 +888,10 @@ export function installFileReferenceHooks( // copy-on-claim pass is what makes "the before hook always reconciles the // payload it is given" a property of this handler instead of a case // analysis a later edit has to re-derive. - await applyCopyOnClaim(engine, getStorage, logger, object, recordId, data, fileFields, actingOrganizationOf(ctx)); + await applyCopyOnClaim( + engine, getStorage, logger, object, recordId, data, fileFields, + actingOrganizationOf(ctx), referencingCallerOf(ctx), authorizeRead, + ); }, { packageId: PACKAGE_ID }, ); @@ -815,10 +908,11 @@ export function installFileReferenceHooks( const recordId = row.id; if (recordId == null) return; const organizationId = actingOrganizationOf(ctx); + const caller = referencingCallerOf(ctx); for (const field of fileFields) { for (const token of [...new Set(idTokensIn(row[field]))]) { try { - await claimFile(engine, logger, token, object, String(recordId), field, organizationId); + await claimFile(engine, logger, token, object, String(recordId), field, organizationId, caller, authorizeRead); } catch (err) { logger.warn( `[storage] file reference: failed to claim ${token} for ${object}/${String(recordId)}.${field} ` + @@ -865,6 +959,7 @@ export function installFileReferenceHooks( if (!ids || ids.length !== 1) return; const recordId = String(ids[0]); const organizationId = actingOrganizationOf(ctx); + const caller = referencingCallerOf(ctx); for (const field of fileFields) { // Only a field the write actually TOUCHED can have changed ownership; @@ -878,7 +973,7 @@ export function installFileReferenceHooks( const held = new Set(current.map((r) => String(r.id))); for (const token of incoming) { if (held.has(token)) continue; // already ours - await claimFile(engine, logger, token, object, recordId, field, organizationId); + await claimFile(engine, logger, token, object, recordId, field, organizationId, caller, authorizeRead); } } catch (err) { logger.warn( diff --git a/packages/services/service-storage/src/storage-service-plugin.ts b/packages/services/service-storage/src/storage-service-plugin.ts index 1b929b2e9d0..329a8af6636 100644 --- a/packages/services/service-storage/src/storage-service-plugin.ts +++ b/packages/services/service-storage/src/storage-service-plugin.ts @@ -62,7 +62,7 @@ import type { TenancyPosture } from '@objectstack/spec/security'; import { registerStorageFamily } from './storage-routes.js'; import type { FileReadVerdict, StorageGatedSessionReader, StorageUploadSession } from './storage-routes.js'; import { installAttachmentLifecycleHooks, createSysFileReapGuard, createUploadSessionReapGuard, findFileHolder, findHeldFiles } from './attachment-lifecycle.js'; -import { installFileReferenceHooks } from './file-reference-lifecycle.js'; +import { installFileReferenceHooks, type FileReferenceReadAuthorizer } from './file-reference-lifecycle.js'; import { installAttachmentAccessHooks, installAttachmentReadVisibility } from './attachment-access-hooks.js'; import { contributeAttachmentDeleteFloorAlternate } from './attachment-delete-floor-alternate.js'; import { FileAttachmentService, type FileAttachmentEngine } from './file-attachment-service.js'; @@ -526,7 +526,16 @@ export class StorageServicePlugin implements Plugin { // the file into the declared grace window (#3459 PR-5b); the reap // guard below re-verifies the ownership columns — and re-reads the // deployment flag, fresh — before any byte is deleted. - installFileReferenceHooks(engine as any, () => this.storage, ctx.logger); + // + // [#25692] A write claims or copies only a file its caller may read: + // the hooks ask the download door's own judgement, bound here from the + // live registry, so the reference rule and the door cannot diverge. + installFileReferenceHooks( + engine as any, + () => this.storage, + ctx.logger, + buildFileReferenceReadAuthorizer(toGateRegistry(ctx), engine), + ); // "Is anything still holding this tombstone?" for RECORD FILE-FIELD // HYDRATION (commit c3c72a4bc). The download path got this question in #10246; // the record read kept the older `status === 'committed'` rule, so one @@ -1528,63 +1537,7 @@ function buildFileReadAuthorizer( }); if (!context?.userId) return 'unauthenticated'; - // Uploader / owner may always download. - if (file.owner_id && String(file.owner_id) === String(context.userId)) return 'allow'; - - // Field-owned (ADR-0104 D3 wave 2): exactly ONE record's field holds - // this file, so access is that record's READ access — never a union. - // The verdict is `readableFieldOwners`, asked for this one owner: the - // same function record file-field hydration asks (#22637), so the bytes - // and the metadata of a field-owned file have one authority. - if (file.ref_object && file.ref_id != null && file.ref_id !== '') { - const ownerId = String(file.ref_id); - const readable = await readableFieldOwners(registry, engine, String(file.ref_object), [ownerId], context); - return readable.has(ownerId) ? 'allow' : 'deny'; - } - - // [#25693] Unclaimed (neither an attachments scope nor a field owner): - // no parent record to derive access from, so the file is read as every - // other organization-scoped row is — within the caller's organization, - // by the organization wall's own verdict ({@link isWithinCallerOrganization}). - if (file.scope !== 'attachments') { - return (await isWithinCallerOrganization(engine, file.id, context, tenancyServiceResolver(registry))) - ? 'allow' - : 'deny'; - } - - // Otherwise: readable via any parent record this file is attached to. - const links = (await (engine as any).find('sys_attachment', { - where: { file_id: file.id }, - fields: ['parent_object', 'parent_id'], - limit: 500, - context: { isSystem: true }, - })) as Array>; - - const byObject = new Map>(); - for (const link of links) { - const po = link.parent_object; - const pid = link.parent_id; - if (typeof po !== 'string' || !po || po === 'sys_attachment') continue; - if (pid === undefined || pid === null || pid === '') continue; - let ids = byObject.get(po); - if (!ids) byObject.set(po, (ids = new Set())); - ids.add(String(pid)); - } - for (const [parentObject, idSet] of byObject) { - const ids = [...idSet]; - try { - const visible = (await (engine as any).find(parentObject, { - where: { id: { $in: ids } }, - fields: ['id'], - limit: ids.length, - context, - })) as Array>; - if (visible.length) return 'allow'; - } catch { - // unknown/failing parent object — try the next - } - } - return 'deny'; + return await judgeFileRead(registry, engine, file, context); } catch (err) { // [commit 6a180e42d] A permission-store outage is not a read verdict. Re-raised so // the file-read door answers the 503 it is instead of a silent 'deny' @@ -1601,6 +1554,122 @@ function buildFileReadAuthorizer( }; } +/** + * The download door's verdict on ONE file for a caller it has already + * resolved — every arm after the door's own `'unauthenticated'` answer. Read + * by two doors, so that one file has one access rule (ADR-0104 D3): + * + * - {@link buildFileReadAuthorizer}, for a download, under the context it + * resolved from the request; + * - [#25692] {@link buildFileReferenceReadAuthorizer}, for a record write + * about to claim or copy the file, under the write's own caller context. A + * write may take a file only as its caller may read it, so holding a file's + * id is never holding its bytes. + * + * `context` carries a `userId`: both callers answer a caller without one + * before asking. Throws what its reads throw (an authorization-store outage + * from the posture read, a failed row read); each caller decides what a throw + * means at its own door. + */ +async function judgeFileRead( + registry: StorageGateRegistry, + engine: IDataEngine, + file: FileRecord, + context: ExecutionContext, +): Promise<'allow' | 'deny'> { + // Uploader / owner may always download. + if (file.owner_id && String(file.owner_id) === String(context.userId)) return 'allow'; + + // Field-owned (ADR-0104 D3 wave 2): exactly ONE record's field holds + // this file, so access is that record's READ access — never a union. + // The verdict is `readableFieldOwners`, asked for this one owner: the + // same function record file-field hydration asks (#22637), so the bytes + // and the metadata of a field-owned file have one authority. + if (file.ref_object && file.ref_id != null && file.ref_id !== '') { + const ownerId = String(file.ref_id); + const readable = await readableFieldOwners(registry, engine, String(file.ref_object), [ownerId], context); + return readable.has(ownerId) ? 'allow' : 'deny'; + } + + // [#25693] Unclaimed (neither an attachments scope nor a field owner): + // no parent record to derive access from, so the file is read as every + // other organization-scoped row is — within the caller's organization, + // by the organization wall's own verdict ({@link isWithinCallerOrganization}). + if (file.scope !== 'attachments') { + return (await isWithinCallerOrganization(engine, file.id, context, tenancyServiceResolver(registry))) + ? 'allow' + : 'deny'; + } + + // Otherwise: readable via any parent record this file is attached to. + const links = (await (engine as any).find('sys_attachment', { + where: { file_id: file.id }, + fields: ['parent_object', 'parent_id'], + limit: 500, + context: { isSystem: true }, + })) as Array>; + + const byObject = new Map>(); + for (const link of links) { + const po = link.parent_object; + const pid = link.parent_id; + if (typeof po !== 'string' || !po || po === 'sys_attachment') continue; + if (pid === undefined || pid === null || pid === '') continue; + let ids = byObject.get(po); + if (!ids) byObject.set(po, (ids = new Set())); + ids.add(String(pid)); + } + for (const [parentObject, idSet] of byObject) { + const ids = [...idSet]; + try { + const visible = (await (engine as any).find(parentObject, { + where: { id: { $in: ids } }, + fields: ['id'], + limit: ids.length, + context, + })) as Array>; + if (visible.length) return 'allow'; + } catch { + // unknown/failing parent object — try the next + } + } + return 'deny'; +} + +/** + * [#25692] The file-reference lifecycle's read verdict + * (`FileReferenceReadAuthorizer`): "may the caller of this record write read + * the file its field is about to reference?", answered by the download door's + * own judgement ({@link judgeFileRead}) under the write's caller context. + * + * The door's arms before the judgement, in the door's order: a `public_read` + * file is anyone's; a caller with no user (an anonymous write) reads no other + * file. The door's wiring fact is read per write, as the door reads it: a + * kernel with no `auth` service binds no download gate, every file is served + * there, and so every file may be referenced — the reference rule is never + * stronger or weaker than the door it mirrors. + * + * A throw is not a verdict. An authorization-store outage is re-raised so the + * write fails as the outage it is, never as an answer about the file; every + * other fault fails closed, as at the door. + */ +function buildFileReferenceReadAuthorizer( + registry: StorageGateRegistry, + engine: IDataEngine, +): FileReferenceReadAuthorizer { + return async (file, context) => { + if (!resolveGateAuthService(registry)) return true; + if (file.acl === 'public_read') return true; + if (!context?.userId) return false; + try { + return (await judgeFileRead(registry, engine, file as unknown as FileRecord, context)) === 'allow'; + } catch (err) { + if (isAuthzStoreUnavailableError(err)) throw err; + return false; // fail closed + } + }; +} + /** * [#25693] Is this unclaimed file within the caller's organization — the * download door's verdict for a file no record owns? From 35695ad0759bf319a53a1fda18b8a5fcd95444df Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 03:49:20 +0000 Subject: [PATCH 2/6] test(storage): pin the claim path's read verdict with a stand-in verdict A write's own copy is claimed without a second question; the verdict is asked of the source file only. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016DTRymn12to8YSGMcTzS7h --- .../src/file-reference-lifecycle.test.ts | 287 ++++++++++++++++++ .../src/file-reference-lifecycle.ts | 55 +++- 2 files changed, 337 insertions(+), 5 deletions(-) diff --git a/packages/services/service-storage/src/file-reference-lifecycle.test.ts b/packages/services/service-storage/src/file-reference-lifecycle.test.ts index 84d9af429a1..f3182267aff 100644 --- a/packages/services/service-storage/src/file-reference-lifecycle.test.ts +++ b/packages/services/service-storage/src/file-reference-lifecycle.test.ts @@ -1070,3 +1070,290 @@ describe('File Reference Ownership (ADR-0104 D3 wave 2)', () => { expect(engine.calls.filter((c) => c.op === 'update' && c.object === 'sys_file')).toHaveLength(0); }); }); + +/** + * [#25692] A write references only a file its caller may read. + * + * The hooks ask the read verdict the storage plugin binds (the download + * door's own judgement) of the WRITE's caller, before copy-on-claim and + * before the claim. Here the verdict is a stand-in that admits a fixed set of + * file ids; `file-reference-read-verdict.integration.test.ts` pins the real + * binding against the door. Each refusal pin below is ablated in the PR that + * introduced it. + */ +describe('a write references only a file its caller may read', () => { + const CALLER = { userId: 'u_writer', tenantId: 'org_1' }; + const session = { userId: CALLER.userId, organizationId: CALLER.tenantId }; + + /** A verdict that admits only `readable`, recording every question asked. */ + function verdict(readable: string[]) { + const asked: Array<{ fileId: unknown; context: any }> = []; + const authorizeRead = vi.fn(async (f: Record, context: any) => { + asked.push({ fileId: f.id, context }); + return readable.includes(String(f.id)); + }); + return { authorizeRead, asked }; + } + + function installWith(engine: Engine, authorizeRead: any, storage: any = fakeStorage()) { + installFileReferenceHooks(engine, () => storage, silentLogger(), authorizeRead); + return { storage }; + } + + /** {@link driveInsert}, as a signed-in caller's write carries it: a hook + * session plus the caller's context on the operation options. */ + async function insertAs( + engine: Engine, + object: string, + data: Record, + id: string, + caller: { session?: any; context?: any } = { session, context: CALLER }, + ) { + const ctx: any = { + object, + event: 'beforeInsert', + input: { data, options: caller.context ? { context: caller.context } : {} }, + session: caller.session, + dispatch: recordDispatch(), + }; + await engine.trigger('beforeInsert', ctx); + const row: Record & { id: string } = { ...(ctx.input.data as Record), id }; + (engine.tables[object] ??= []).push(row); + ctx.event = 'afterInsert'; + ctx.result = row; + await engine.trigger('afterInsert', ctx); + return row; + } + + async function updateAs(engine: Engine, object: string, id: string, data: Record) { + const ctx: any = { + object, + event: 'beforeUpdate', + input: { id, data, options: { context: CALLER } }, + session, + dispatch: recordDispatch(), + }; + await engine.trigger('beforeUpdate', ctx); + const row = (engine.tables[object] ?? []).find((r) => String(r.id) === String(id)); + if (row) Object.assign(row, ctx.input.data); + ctx.event = 'afterUpdate'; + ctx.result = row; + await engine.trigger('afterUpdate', ctx); + return row; + } + + const ownedByP1 = () => file({ ref_object: 'product', ref_id: 'p1', ref_field: 'image' }); + const sysFileWrites = (engine: Engine) => + engine.calls.filter((c) => (c.op === 'update' || c.op === 'insert') && c.object === 'sys_file'); + + describe('a file another record owns', () => { + it('is not copied for a caller the verdict refuses: the value is kept as written', async () => { + const engine = fakeEngine({ files: [ownedByP1()], records: { product: [{ id: 'p1', image: 'file_a' }] } }); + const { authorizeRead } = verdict([]); + const { storage } = installWith(engine, authorizeRead); + + const row = await insertAs(engine, 'product', { image: 'file_a' }, 'p2'); + + expect(row.image).toBe('file_a'); + expect(storage.download).not.toHaveBeenCalled(); + expect(storage.upload).not.toHaveBeenCalled(); + expect(engine.tables.sys_file).toHaveLength(1); + expect(engine.tables.sys_file[0]).toMatchObject({ id: 'file_a', ref_object: 'product', ref_id: 'p1', ref_field: 'image' }); + expect(sysFileWrites(engine)).toEqual([]); + }); + + it('is not copied by an update for a caller the verdict refuses', async () => { + const engine = fakeEngine({ + files: [ownedByP1()], + records: { product: [{ id: 'p1', image: 'file_a' }, { id: 'p2', image: null }] }, + }); + const { authorizeRead } = verdict([]); + const { storage } = installWith(engine, authorizeRead); + + const row = await updateAs(engine, 'product', 'p2', { image: 'file_a' }); + + expect(row?.image).toBe('file_a'); + expect(storage.download).not.toHaveBeenCalled(); + expect(engine.tables.sys_file).toHaveLength(1); + expect(engine.tables.sys_file[0]).toMatchObject({ ref_id: 'p1' }); + }); + + it('CONTROL — is copied for a caller the verdict admits, exactly as before; the copy is claimed without a second question', async () => { + const engine = fakeEngine({ files: [ownedByP1()], records: { product: [{ id: 'p1', image: 'file_a' }] } }); + const { authorizeRead, asked } = verdict(['file_a']); + const { storage } = installWith(engine, authorizeRead); + + const row = await insertAs(engine, 'product', { image: 'file_a' }, 'p2'); + + expect(row.image).not.toBe('file_a'); + expect(storage.download).toHaveBeenCalledWith('user/file_a.png'); + const copy = engine.tables.sys_file.find((f) => f.id === row.image)!; + expect(copy).toMatchObject({ ref_object: 'product', ref_id: 'p2', ref_field: 'image' }); + expect(engine.tables.sys_file[0]).toMatchObject({ id: 'file_a', ref_id: 'p1' }); + // The verdict was asked about the SOURCE only — the stand-in admits + // nothing else, so a claim that re-asked about the copy would have + // left it unclaimed. + expect(asked.map((q) => q.fileId)).toEqual(['file_a']); + }); + }); + + describe('a file no record owns', () => { + it('is not claimed for a caller the verdict refuses', async () => { + const engine = fakeEngine({ files: [file()] }); + const { authorizeRead } = verdict([]); + installWith(engine, authorizeRead); + + const row = await insertAs(engine, 'product', { image: 'file_a' }, 'p1'); + + expect(row.image).toBe('file_a'); + expect(engine.tables.sys_file[0].ref_id ?? null).toBeNull(); + expect(sysFileWrites(engine)).toEqual([]); + }); + + it('is not claimed by an update for a caller the verdict refuses', async () => { + const engine = fakeEngine({ files: [file()], records: { product: [{ id: 'p1', image: null }] } }); + const { authorizeRead } = verdict([]); + installWith(engine, authorizeRead); + + await updateAs(engine, 'product', 'p1', { image: 'file_a' }); + + expect(engine.tables.sys_file[0].ref_id ?? null).toBeNull(); + expect(sysFileWrites(engine)).toEqual([]); + }); + + it('a tombstoned file is not brought back for a caller the verdict refuses', async () => { + const engine = fakeEngine({ files: [file({ status: 'deleted', deleted_at: '2026-01-01T00:00:00.000Z' })] }); + const { authorizeRead } = verdict([]); + installWith(engine, authorizeRead); + + await insertAs(engine, 'product', { image: 'file_a' }, 'p1'); + + expect(engine.tables.sys_file[0]).toMatchObject({ status: 'deleted', deleted_at: '2026-01-01T00:00:00.000Z' }); + expect(engine.tables.sys_file[0].ref_id ?? null).toBeNull(); + }); + + it('CONTROL — is claimed for a caller the verdict admits', async () => { + const engine = fakeEngine({ files: [file()] }); + const { authorizeRead } = verdict(['file_a']); + installWith(engine, authorizeRead); + + await insertAs(engine, 'product', { image: 'file_a' }, 'p1'); + + expect(engine.tables.sys_file[0]).toMatchObject({ ref_object: 'product', ref_id: 'p1', ref_field: 'image' }); + }); + + it('each id of a multiple:true field is judged on its own', async () => { + const engine = fakeEngine({ files: [file(), file({ id: 'file_b', key: 'user/file_b.png' })] }); + const { authorizeRead } = verdict(['file_b']); + installWith(engine, authorizeRead); + + const row = await insertAs(engine, 'product', { gallery: ['file_a', 'file_b'] }, 'p1'); + + expect(row.gallery).toEqual(['file_a', 'file_b']); + expect(engine.tables.sys_file.map((f) => [f.id, f.ref_id ?? null])).toEqual([ + ['file_a', null], + ['file_b', 'p1'], + ]); + }); + }); + + it('a field\'s declared accept / maxSize are not judged against a file the caller may not read', async () => { + const engine = fakeEngine({ + files: [file({ mime_type: 'application/pdf', size: 10_000_000 })], + registry: { + sys_file: REGISTRY.sys_file, + product: { name: 'product', fields: { id: { type: 'text' }, image: { type: 'image', accept: ['image/*'], maxSize: 1000 } } }, + }, + }); + const { authorizeRead } = verdict([]); + installWith(engine, authorizeRead); + + const row = await insertAs(engine, 'product', { image: 'file_a' }, 'p1'); + + expect(row.image).toBe('file_a'); + expect(engine.tables.sys_file[0].ref_id ?? null).toBeNull(); + }); + + it('CONTROL — a file the caller may read is still judged against its field\'s declaration', async () => { + const engine = fakeEngine({ + files: [file({ mime_type: 'application/pdf', size: 10 })], + registry: { + sys_file: REGISTRY.sys_file, + product: { name: 'product', fields: { id: { type: 'text' }, image: { type: 'image', accept: ['image/*'] } } }, + }, + }); + const { authorizeRead } = verdict(['file_a']); + installWith(engine, authorizeRead); + + const err = await insertAs(engine, 'product', { image: 'file_a' }, 'p1').then(() => null, (e) => e); + + expect(err).toBeInstanceOf(FileConstraintError); + expect({ code: err.code, status: err.status }).toEqual({ code: 'ERR_FILE_CONSTRAINT', status: 400 }); + }); + + it('asks the verdict of the write\'s caller context, taken from the operation options', async () => { + const engine = fakeEngine({ files: [ownedByP1()], records: { product: [{ id: 'p1', image: 'file_a' }] } }); + const { authorizeRead, asked } = verdict([]); + installWith(engine, authorizeRead); + const context = { ...CALLER, permissions: ['member_default'], __operationPrivate: 'dropped' }; + + await insertAs(engine, 'product', { image: 'file_a' }, 'p2', { session, context }); + + expect(asked.length).toBeGreaterThan(0); + for (const q of asked) { + expect(q.fileId).toBe('file_a'); + expect(q.context).toMatchObject({ userId: 'u_writer', tenantId: 'org_1', permissions: ['member_default'] }); + expect(q.context).not.toHaveProperty('__operationPrivate'); + } + }); + + it('an authorization-store outage fails the write rather than reading as either answer', async () => { + const engine = fakeEngine({ files: [ownedByP1()], records: { product: [{ id: 'p1', image: 'file_a' }] } }); + const outage = Object.assign(new Error('authorization store unavailable'), { code: 'SERVICE_UNAVAILABLE', status: 503 }); + const authorizeRead = vi.fn(async () => { throw outage; }); + const { storage } = installWith(engine, authorizeRead); + + const err = await insertAs(engine, 'product', { image: 'file_a' }, 'p2').then(() => null, (e) => e); + + expect(err).toBe(outage); + expect(storage.download).not.toHaveBeenCalled(); + // The before hook threw, so the driver never wrote the record. + expect((engine.tables.product ?? []).map((r) => r.id)).toEqual(['p1']); + }); + + describe('writes with no caller to ask reference files as before', () => { + it('a system write is not asked, and copies as before', async () => { + const engine = fakeEngine({ files: [ownedByP1()], records: { product: [{ id: 'p1', image: 'file_a' }] } }); + const { authorizeRead } = verdict([]); + installWith(engine, authorizeRead); + + const row = await insertAs(engine, 'product', { image: 'file_a' }, 'p2', { + session: { ...session, isSystem: true }, + context: { ...CALLER, isSystem: true }, + }); + + expect(authorizeRead).not.toHaveBeenCalled(); + expect(row.image).not.toBe('file_a'); + }); + + it('a context-less programmatic write is not asked, and claims as before', async () => { + const engine = fakeEngine({ files: [file()] }); + const { authorizeRead } = verdict([]); + installWith(engine, authorizeRead); + + await insertAs(engine, 'product', { image: 'file_a' }, 'p1', {}); + + expect(authorizeRead).not.toHaveBeenCalled(); + expect(engine.tables.sys_file[0]).toMatchObject({ ref_id: 'p1' }); + }); + + it('no verdict bound (a bare kernel): nothing is asked, and files are claimed as before', async () => { + const engine = fakeEngine({ files: [file()] }); + install(engine); + + await insertAs(engine, 'product', { image: 'file_a' }, 'p1'); + + expect(engine.tables.sys_file[0]).toMatchObject({ ref_id: 'p1' }); + }); + }); +}); diff --git a/packages/services/service-storage/src/file-reference-lifecycle.ts b/packages/services/service-storage/src/file-reference-lifecycle.ts index 5ba76857c12..6f1ad3458d0 100644 --- a/packages/services/service-storage/src/file-reference-lifecycle.ts +++ b/packages/services/service-storage/src/file-reference-lifecycle.ts @@ -218,6 +218,42 @@ function referencingCallerOf(ctx: any): ExecutionContext | null { return callerContext(ctx); } +/** + * [#25692] The copies copy-on-claim minted for one write, so that write's own + * claim takes them without asking the read verdict a second time: the verdict + * was asked of the SOURCE file before its bytes were copied, and the copy is a + * file only this write has ever named. + * + * Keyed by the write's per-write scratch (`dispatch.scope`, one object shared + * by every dispatch of a write across both phases) or, without one, by the hook + * context itself (one object for both phases of a single-record write) — + * weakly, so an entry lives exactly as long as its write. A write whose two + * phases share neither finds no entry, and its claim then judges the copy as + * any unclaimed file is judged; a refusal there leaves the copy unclaimed — + * retained, never collected — which is the safe direction. + */ +const mintedCopies = new WeakMap>(); + +function writeKeyOf(ctx: any): object | null { + const scope = ctx?.dispatch?.scope; + if (scope && typeof scope === 'object') return scope; + return ctx && typeof ctx === 'object' ? ctx : null; +} + +function mintedCopiesFor(ctx: any): Set | null { + const key = writeKeyOf(ctx); + if (!key) return null; + let minted = mintedCopies.get(key); + if (!minted) mintedCopies.set(key, (minted = new Set())); + return minted; +} + +/** The caller a claim of `fileId` asks about: none for a copy this write minted. */ +function claimingCallerOf(ctx: any, fileId: string, caller: ExecutionContext | null): ExecutionContext | null { + const key = writeKeyOf(ctx); + return key && mintedCopies.get(key)?.has(fileId) ? null : caller; +} + /** * The `sys_file` row a field's id token names, as THIS write may reference it, * or `null`. Both halves of the claim path — copy-on-claim before the write, @@ -601,6 +637,7 @@ async function applyCopyOnClaim( organizationId: string | null, caller: ExecutionContext | null, authorizeRead: FileReferenceReadAuthorizer | undefined, + minted: Set | null, ): Promise { for (const field of fileFields) { if (!(field in data)) continue; @@ -644,7 +681,9 @@ async function applyCopyOnClaim( continue; } try { - replacements.set(token, await copyOwnedFile(engine, storage, row, organizationId)); + const copyId = await copyOwnedFile(engine, storage, row, organizationId); + minted?.add(copyId); + replacements.set(token, copyId); logger.debug?.( `[storage] file reference: copied ${token} for ${object}.${field} (exclusive ownership)`, ); @@ -833,7 +872,7 @@ export function installFileReferenceHooks( if (fileFields.length === 0) return; await applyCopyOnClaim( engine, getStorage, logger, object, null, data, fileFields, - actingOrganizationOf(ctx), referencingCallerOf(ctx), authorizeRead, + actingOrganizationOf(ctx), referencingCallerOf(ctx), authorizeRead, mintedCopiesFor(ctx), ); }, { packageId: PACKAGE_ID }, @@ -890,7 +929,7 @@ export function installFileReferenceHooks( // analysis a later edit has to re-derive. await applyCopyOnClaim( engine, getStorage, logger, object, recordId, data, fileFields, - actingOrganizationOf(ctx), referencingCallerOf(ctx), authorizeRead, + actingOrganizationOf(ctx), referencingCallerOf(ctx), authorizeRead, mintedCopiesFor(ctx), ); }, { packageId: PACKAGE_ID }, @@ -912,7 +951,10 @@ export function installFileReferenceHooks( for (const field of fileFields) { for (const token of [...new Set(idTokensIn(row[field]))]) { try { - await claimFile(engine, logger, token, object, String(recordId), field, organizationId, caller, authorizeRead); + await claimFile( + engine, logger, token, object, String(recordId), field, organizationId, + claimingCallerOf(ctx, token, caller), authorizeRead, + ); } catch (err) { logger.warn( `[storage] file reference: failed to claim ${token} for ${object}/${String(recordId)}.${field} ` + @@ -973,7 +1015,10 @@ export function installFileReferenceHooks( const held = new Set(current.map((r) => String(r.id))); for (const token of incoming) { if (held.has(token)) continue; // already ours - await claimFile(engine, logger, token, object, recordId, field, organizationId, caller, authorizeRead); + await claimFile( + engine, logger, token, object, recordId, field, organizationId, + claimingCallerOf(ctx, token, caller), authorizeRead, + ); } } catch (err) { logger.warn( From 2aad5c8788935d02c0c55a0a167b17d6ab45f6dc Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 03:52:23 +0000 Subject: [PATCH 3/6] test(storage): the claim path and the download door give one verdict per file Through a real engine with the storage plugin booted as it ships, each file is asked of the download door and of a record write by the same caller; the write takes the file exactly when the door serves it. An authorization-store outage fails the write with its declared envelope; any other fault fails closed. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016DTRymn12to8YSGMcTzS7h --- ...reference-read-verdict.integration.test.ts | 423 ++++++++++++++++++ 1 file changed, 423 insertions(+) create mode 100644 packages/services/service-storage/src/file-reference-read-verdict.integration.test.ts diff --git a/packages/services/service-storage/src/file-reference-read-verdict.integration.test.ts b/packages/services/service-storage/src/file-reference-read-verdict.integration.test.ts new file mode 100644 index 00000000000..2e16d2d6b1a --- /dev/null +++ b/packages/services/service-storage/src/file-reference-read-verdict.integration.test.ts @@ -0,0 +1,423 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #25692 — a record write claims or copies only a file its caller may read, + * and "may read" is the download door's own verdict. + * + * ADR-0104 D3: one file, one owning record, one access rule. The claim path + * (`installFileReferenceHooks`) copies a file another record owns into a fresh + * `sys_file` the writing record owns, and claims a file no record owns for it. + * Either hands the file's bytes to the writing record's readers, so neither + * runs for a caller the door would not serve the file to. The storage plugin + * binds the hooks to the door's judgement (`judgeFileRead`, shared with the + * download authorizer), so the two cannot disagree. + * + * ## What is pinned + * + * One real `ObjectQL` engine over sqlite `:memory:`, with `StorageServicePlugin` + * booted and wired exactly as it ships, asked about ONE fixture through both + * doors: the download door (`GET /files/:fileId/url`, as the writer) and a + * record write (as the same writer). For every file the write takes it exactly + * when the door serves it: + * + * - a file owned by a record the writer cannot read: door 403, and the write + * neither copies nor claims it — the value is kept as written; + * - an attachments-scope file whose only parent the writer cannot read: door + * 403, and the write does not claim it; + * - an owner object that declares a `fileAccessDelegate`: the DELEGATE + * decides on both doors, refusal and admission alike; + * - controls, served by the door and taken by the write: a file owned by a + * record the writer can read (copied), the writer's own upload (copied — + * the uploader arm), a `public_read` file (copied), and an unclaimed file + * of the organization (claimed). + * + * And the writes with no caller to judge: a system write takes a file the + * door refuses the writer, as it did before. + * + * A throw from the judgement is not a verdict: an authorization-store outage + * fails the write with its declared envelope, and any other fault fails + * closed — the file is kept as written, never taken. + * + * ## The stand-ins, and only these + * + * - The security layer's refusals: an engine middleware that throws + * `PERMISSION_DENIED` for every non-system read of `vault` and of + * `sys_file`, the shape `field-owned-file-hydration-door-agreement.test.ts` + * uses for the same layer. + * - The permission store the door's context resolution reads: reads of an + * object this fixture never registered answer empty. + * - A fault switch on the judgement's `sys_attachment` read, for the + * outage pins. + * - Authentication: one `x-test-user` header per session. + */ + +import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; +import { promises as fs } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { AuthzStoreUnavailableError, AUTHZ_STORE_UNAVAILABLE_CODE, AUTHZ_STORE_UNAVAILABLE_STATUS } from '@objectstack/core'; +import type { IStorageService, RouteHandler } from '@objectstack/spec/contracts'; +import { RAW_FILE_VALUES_CONTEXT_KEY, type EngineQueryOptions } from '@objectstack/spec/data'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { StorageServicePlugin } from './storage-service-plugin.js'; +import { installDefaultOrganization } from './default-organization.testkit.js'; + +const URL_ROUTE = '/api/v1/storage/files/:fileId/url'; +const SYS = { isSystem: true }; +const SYS_RAW = { isSystem: true, [RAW_FILE_VALUES_CONTEXT_KEY]: true }; +const WRITER = 'u_writer'; +const WRITER_CTX = { userId: WRITER }; +const DELEGATE = 'test_decision_files'; + +/** Owned by `vault/v_closed.doc` — a record the writer cannot read. */ +const F_CLOSED = 'f_closed_aa'; +/** Owned by `contract/c_open.doc` — a record the writer can read. */ +const F_OPEN = 'f_open_bb'; +/** Owned by `vault/v_closed.doc`'s sibling slot, uploaded by the writer. */ +const F_UPLOADED = 'f_uploaded_cc'; +/** Owned by `vault/v_closed.annex`, `acl: 'public_read'`. */ +const F_PUBLIC = 'f_public_dd'; +/** No field owner: an attachments-scope file attached to `v_closed` only. */ +const F_ATTACHED = 'f_attached_ee'; +/** No owner at all, no attachment: an unclaimed file of the organization. */ +const F_FREE = 'f_free_ff'; +/** Owned by `decision/dec_allow.proof` — the delegate admits the writer. */ +const F_DEC_ALLOW = 'f_decallow_gg'; +/** Owned by `decision/dec_deny.proof` — the delegate refuses the writer. */ +const F_DEC_DENY = 'f_decdeny_hh'; + +const silentLogger = () => ({ + info: vi.fn(), warn: vi.fn(), debug: vi.fn(), error: vi.fn(), + trace: vi.fn(), fatal: vi.fn(), child() { return this; }, +}); + +/** The refusal the security layer throws — `PermissionDeniedError`'s declared envelope. */ +const refusal = () => + Object.assign(new Error('You do not have permission to perform this action.'), { + name: 'PermissionDeniedError', code: 'PERMISSION_DENIED', status: 403, statusCode: 403, + }); + +const fileRow = (id: string, extra: Record = {}) => ({ + id, + key: `files/${id}.pdf`, + name: `${id}.pdf`, + size: 2048, + mime_type: 'application/pdf', + scope: 'user', + acl: 'private', + status: 'committed', + owner_id: 'u_uploader', + ...extra, +}); +const owned = (object: string, recordId: string, field: string) => ({ + ref_object: object, ref_id: recordId, ref_field: field, +}); + +const FILES = [ + fileRow(F_CLOSED, owned('vault', 'v_closed', 'doc')), + fileRow(F_OPEN, owned('contract', 'c_open', 'doc')), + fileRow(F_UPLOADED, { ...owned('vault', 'v_closed', 'annex'), owner_id: WRITER }), + fileRow(F_PUBLIC, { ...owned('vault', 'v_mine', 'annex'), acl: 'public_read' }), + fileRow(F_ATTACHED, { scope: 'attachments' }), + fileRow(F_FREE), + fileRow(F_DEC_ALLOW, owned('decision', 'dec_allow', 'proof')), + fileRow(F_DEC_DENY, owned('decision', 'dec_deny', 'proof')), +]; + +function makeCtx() { + const services = new Map(); + const hooks: Array<() => Promise | void> = []; + return { + logger: silentLogger(), + registerService: (name: string, svc: unknown) => { services.set(name, svc); }, + getService: (name: string): T => { + if (!services.has(name)) throw new Error(`service '${name}' not registered`); + return services.get(name) as T; + }, + hook: (event: string, fn: () => Promise | void) => { + if (event === 'kernel:ready') hooks.push(fn); + }, + flushReady: async () => { for (const h of hooks) await h(); }, + }; +} + +function makeHttpServer() { + const routes = new Map(); + const put = (m: string) => (path: string, handler: RouteHandler) => { routes.set(`${m}:${path}`, handler); }; + return { + routes, + server: { + get: put('GET'), post: put('POST'), put: put('PUT'), + delete: vi.fn(), patch: vi.fn(), use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + }, + }; +} + +describe('#25692 — a record write claims or copies only a file the download door serves its caller', () => { + let rootDir: string; + let engine: ObjectQL; + let routes: Map; + let seq = 0; + /** When set, the judgement's `sys_attachment` read rejects with it. */ + let attachmentReadFault: Error | null = null; + + beforeAll(async () => { + rootDir = join(tmpdir(), `os-25692-${Date.now()}-${Math.random().toString(36).slice(2)}`); + await fs.mkdir(rootDir, { recursive: true }); + + engine = new ObjectQL({ logger: silentLogger() } as any); + engine.registerDriver(new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }) as any, true); + await engine.init(); + + // The data door the plugin is handed: the real engine, except that a read + // of an object this fixture never registered — the permission store the + // door's context resolution consults — answers empty. + const dataEngine = new Proxy(engine, { + get(target, prop) { + if (prop === 'find') { + return (object: string, query?: EngineQueryOptions) => { + if (object === 'sys_attachment' && attachmentReadFault) return Promise.reject(attachmentReadFault); + return target.getSchema(object) ? target.find(object, query) : Promise.resolve([]); + }; + } + const value = Reflect.get(target, prop, target); + return typeof value === 'function' ? value.bind(target) : value; + }, + }); + + const ctx = makeCtx(); + const http = makeHttpServer(); + routes = http.routes; + ctx.registerService('objectql', dataEngine); + ctx.registerService('http-server', http.server); + ctx.registerService('manifest', { register: (manifest: any) => engine.registerApp(manifest) }); + ctx.registerService('auth', { + api: { + getSession: async ({ headers }: { headers: Headers }) => { + const userId = headers.get('x-test-user'); + return userId ? { user: { id: userId }, session: {} } : undefined; + }, + }, + }); + ctx.registerService(DELEGATE, { + authorizeFileRead: async (recordId: string, context: any) => + recordId === 'dec_allow' && context?.userId === WRITER, + }); + + const plugin = new StorageServicePlugin({ adapter: 'local', local: { rootDir }, bindToSettings: false }); + await plugin.init(ctx as any); + + engine.registerApp({ + id: 'com.objectstack.test.file-reference-read-verdict', + name: 'File reference read verdict', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [ + { + name: 'contract', + label: 'Contract', + fields: { + title: { name: 'title', type: 'text' }, + doc: { name: 'doc', type: 'file' }, + annexes: { name: 'annexes', type: 'file', multiple: true }, + }, + }, + { + name: 'vault', + label: 'Vault', + fields: { doc: { name: 'doc', type: 'file' }, annex: { name: 'annex', type: 'file' } }, + }, + { + name: 'decision', + label: 'Decision', + fileAccessDelegate: DELEGATE, + fields: { proof: { name: 'proof', type: 'file' } }, + }, + ], + } as never); + await engine.syncSchemas(); + await installDefaultOrganization(engine); + + // Seeded BEFORE kernel:ready, so the field-reference hooks do not rewrite + // the fixture. + for (const f of FILES) await engine.insert('sys_file', f as any, { context: SYS }); + await engine.insert('sys_attachment', { + id: 'att_closed', file_id: F_ATTACHED, parent_object: 'vault', parent_id: 'v_closed', + file_name: `${F_ATTACHED}.pdf`, mime_type: 'application/pdf', size: 2048, + } as any, { context: SYS }); + await engine.insert('vault', { id: 'v_closed', doc: F_CLOSED, annex: F_UPLOADED } as any, { context: SYS }); + await engine.insert('vault', { id: 'v_mine', annex: F_PUBLIC } as any, { context: SYS }); + await engine.insert('contract', { id: 'c_open', title: 'Open', doc: F_OPEN } as any, { context: SYS }); + await engine.insert('decision', { id: 'dec_allow', proof: F_DEC_ALLOW } as any, { context: SYS }); + await engine.insert('decision', { id: 'dec_deny', proof: F_DEC_DENY } as any, { context: SYS }); + + const refuseNonSystem = async (opCtx: any, next: () => Promise) => { + if (!opCtx.context?.isSystem) throw refusal(); + return next(); + }; + engine.registerMiddleware(refuseNonSystem, { object: 'sys_file' }); + engine.registerMiddleware(refuseNonSystem, { object: 'vault' }); + + await plugin.start(ctx as any); + await ctx.flushReady(); + + const storage = ctx.getService('storage'); + for (const f of FILES) await storage.upload(f.key, Buffer.from(`bytes of ${f.id}`)); + }); + + afterAll(async () => { + try { await engine?.destroy(); } catch { /* already torn down */ } + if (rootDir) await fs.rm(rootDir, { recursive: true, force: true }); + }); + + /** What the download door tells `user` about one file. */ + const door = async (fileId: string, user: string = WRITER) => { + const res: any = { + _status: 200, _json: null, + json(data: any) { res._json = data; return res; }, + send() { return res; }, + status(code: number) { res._status = code; return res; }, + header() { return res; }, + }; + await routes.get(`GET:${URL_ROUTE}`)!( + { params: { fileId }, query: {}, headers: { 'x-test-user': user }, method: 'GET', path: URL_ROUTE } as any, + res, + ); + return { status: res._status as number, code: res._json?.error?.code as string | undefined }; + }; + + const fileRows = async () => (await engine.find('sys_file', { context: SYS })) as Array>; + const fileById = async (id: string) => (await fileRows()).find((r) => r.id === id); + const storedDoc = async (recordId: string) => + ((await engine.find('contract', { where: { id: recordId }, context: SYS_RAW })) as any[])[0]?.doc; + + /** + * Write `fileId` into a new contract's `doc`, as `context`, and report what + * the write did with it: `copied` (the record holds a fresh file it owns), + * `claimed` (the record owns the file it named) or `kept` (the record holds + * the id as written and owns nothing). + */ + const writeAs = async (fileId: string, context: Record) => { + const recordId = `c_new_${++seq}`; + const before = (await fileRows()).length; + await engine.insert('contract', { id: recordId, title: recordId, doc: fileId } as any, { context }); + const held = await storedDoc(recordId); + const heldRow = await fileById(String(held)); + const ownsHeld = heldRow?.ref_object === 'contract' && heldRow?.ref_id === recordId && heldRow?.ref_field === 'doc'; + const minted = (await fileRows()).length - before; + const outcome = held !== fileId && ownsHeld && minted === 1 + ? 'copied' + : held === fileId && ownsHeld && minted === 0 + ? 'claimed' + : held === fileId && !ownsHeld && minted === 0 + ? 'kept' + : `unexpected (held ${String(held)}, owns ${ownsHeld}, minted ${minted})`; + return { recordId, held, outcome }; + }; + + describe('a file the door refuses the writer is neither copied nor claimed by the writer\'s write', () => { + it('a file owned by a record the writer cannot read', async () => { + expect(await door(F_CLOSED)).toEqual({ status: 403, code: 'FILE_DOWNLOAD_DENIED' }); + + const { outcome } = await writeAs(F_CLOSED, WRITER_CTX); + + expect(outcome).toBe('kept'); + expect(await fileById(F_CLOSED)).toMatchObject(owned('vault', 'v_closed', 'doc')); + }); + + it('an attachments-scope file whose only parent the writer cannot read', async () => { + expect(await door(F_ATTACHED)).toEqual({ status: 403, code: 'ATTACHMENT_DOWNLOAD_DENIED' }); + + const { outcome } = await writeAs(F_ATTACHED, WRITER_CTX); + + expect(outcome).toBe('kept'); + expect((await fileById(F_ATTACHED))?.ref_id ?? null).toBeNull(); + }); + + it('a file whose owner object\'s delegate refuses the writer', async () => { + expect(await door(F_DEC_DENY)).toEqual({ status: 403, code: 'FILE_DOWNLOAD_DENIED' }); + + expect((await writeAs(F_DEC_DENY, WRITER_CTX)).outcome).toBe('kept'); + }); + }); + + describe('CONTROL — a file the door serves the writer is taken by the writer\'s write, as before', () => { + it('a file owned by a record the writer can read is copied', async () => { + expect(await door(F_OPEN)).toEqual({ status: 200, code: undefined }); + + const { held, outcome } = await writeAs(F_OPEN, WRITER_CTX); + + expect(outcome).toBe('copied'); + // The copy is the writer's record's own, and the door serves it to the writer. + expect(await door(String(held))).toEqual({ status: 200, code: undefined }); + expect(await fileById(F_OPEN)).toMatchObject(owned('contract', 'c_open', 'doc')); + }); + + it('the writer\'s own upload, owned by a record the writer cannot read, is copied (the uploader arm)', async () => { + expect(await door(F_UPLOADED)).toEqual({ status: 200, code: undefined }); + + expect((await writeAs(F_UPLOADED, WRITER_CTX)).outcome).toBe('copied'); + }); + + it('a public_read file is copied', async () => { + expect(await door(F_PUBLIC)).toEqual({ status: 200, code: undefined }); + + expect((await writeAs(F_PUBLIC, WRITER_CTX)).outcome).toBe('copied'); + }); + + it('a file whose owner object\'s delegate admits the writer is copied', async () => { + expect(await door(F_DEC_ALLOW)).toEqual({ status: 200, code: undefined }); + + expect((await writeAs(F_DEC_ALLOW, WRITER_CTX)).outcome).toBe('copied'); + }); + + it('an unclaimed file of the organization is claimed', async () => { + expect(await door(F_FREE)).toEqual({ status: 200, code: undefined }); + + expect((await writeAs(F_FREE, WRITER_CTX)).outcome).toBe('claimed'); + }); + }); + + describe('a throw from the judgement is not a verdict', () => { + it('an authorization-store outage fails the write with its declared envelope', async () => { + attachmentReadFault = new AuthzStoreUnavailableError('sys_attachment'); + const recordId = `c_new_${++seq}`; + try { + const err = await engine + .insert('contract', { id: recordId, title: recordId, doc: F_ATTACHED } as any, { context: WRITER_CTX }) + .then(() => null, (e: any) => e); + expect({ code: err?.code, status: err?.status }).toEqual({ + code: AUTHZ_STORE_UNAVAILABLE_CODE, + status: AUTHZ_STORE_UNAVAILABLE_STATUS, + }); + } finally { + attachmentReadFault = null; + } + expect(await storedDoc(recordId)).toBeUndefined(); + expect((await fileById(F_ATTACHED))?.ref_id ?? null).toBeNull(); + }); + + it('any other fault fails closed: the file is kept as written, never taken', async () => { + attachmentReadFault = new Error('driver fault'); + try { + expect((await writeAs(F_ATTACHED, WRITER_CTX)).outcome).toBe('kept'); + } finally { + attachmentReadFault = null; + } + }); + }); + + it('a system write has no caller to judge, and copies a file the door refuses the writer, as before', async () => { + const { outcome } = await writeAs(F_CLOSED, { ...SYS, userId: WRITER }); + + expect(outcome).toBe('copied'); + }); +}); From 6491301cafba483a5198b952372dafea84a14584 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 03:54:12 +0000 Subject: [PATCH 4/6] chore(changeset): service-storage patch for the claim path's read verdict Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_016DTRymn12to8YSGMcTzS7h --- ...25692-file-reference-claim-read-verdict.md | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .changeset/25692-file-reference-claim-read-verdict.md diff --git a/.changeset/25692-file-reference-claim-read-verdict.md b/.changeset/25692-file-reference-claim-read-verdict.md new file mode 100644 index 00000000000..d917786ef6a --- /dev/null +++ b/.changeset/25692-file-reference-claim-read-verdict.md @@ -0,0 +1,21 @@ +--- +'@objectstack/service-storage': patch +--- + +fix(storage)!: a record write claims or copies only a file its caller may read + +The file-reference claim path (ADR-0104 D3: a file field holds a `sys_file` id, at most one record's field owns a file, and writing an id another record owns copies the bytes into a fresh file the writing record owns) now asks, before it copies or claims a referenced file, whether the write's caller may read that file. The question is the download door's own verdict, asked under the write's caller context: the uploader, a reader of the one record whose field owns the file (its object's `fileAccessDelegate` where one is declared), a member of the organization for a file no record owns, a reader of a parent record for an attachments-scope file, anyone for a `public_read` file. The door and the claim path now call one judgement, so one file has one access rule. + +A file the caller may not read is answered as the organization-scoped lookup answers a file it cannot see: the value is kept as written, nothing is claimed or copied, and the field's declared `accept` / `maxSize` are not judged against it (constraint verdicts no longer run ahead of the read check). A copy this write made is claimed by the same write without a second question. System writes and context-less programmatic calls have no caller to ask and reference files as before; so does a kernel with no `auth` service, where the download door binds no gate. An authorization-store outage fails the write with its declared envelope instead of reading as either answer. + +Clause-②: no (narrowing) — a record write reaches fewer files: a file its caller may not read is no longer copied or claimed for it. Nothing that was refused becomes accepted. + +Metadata-protocol: none (a write path's file bookkeeping; nothing a metadata author writes changes) + + + +**BREAKING** for a write that referenced a file its caller may not read, shipped as `patch` because the metadata protocol does not move. + +- **Before.** The claim path copied or claimed a referenced file without asking whether the write's caller may read it. +- **Now.** Both happen only when the caller may read the file under the download door's rule. Otherwise the record keeps the id as written and owns nothing: the file's readers are unchanged, and the written record's readers gain no access to it through that record. +- **Fix.** Reference a file as a caller who may read it (its uploader, or a reader of the record that owns it), or upload the file again. A server-side process that copies files between records on purpose runs as a system write. From c61d92c80b691959b762ad6e1ca914dee8ff28ab Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 04:06:36 +0000 Subject: [PATCH 5/6] docs(storage): state the claim path's constraint ordering in one sentence Code comments, one test name and the changeset now say only that constraint verdicts no longer run ahead of the read check. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_016DTRymn12to8YSGMcTzS7h --- .changeset/25692-file-reference-claim-read-verdict.md | 2 +- .../src/file-reference-lifecycle.test.ts | 2 +- .../service-storage/src/file-reference-lifecycle.ts | 11 +++++------ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/.changeset/25692-file-reference-claim-read-verdict.md b/.changeset/25692-file-reference-claim-read-verdict.md index d917786ef6a..060682ab9c1 100644 --- a/.changeset/25692-file-reference-claim-read-verdict.md +++ b/.changeset/25692-file-reference-claim-read-verdict.md @@ -6,7 +6,7 @@ fix(storage)!: a record write claims or copies only a file its caller may read The file-reference claim path (ADR-0104 D3: a file field holds a `sys_file` id, at most one record's field owns a file, and writing an id another record owns copies the bytes into a fresh file the writing record owns) now asks, before it copies or claims a referenced file, whether the write's caller may read that file. The question is the download door's own verdict, asked under the write's caller context: the uploader, a reader of the one record whose field owns the file (its object's `fileAccessDelegate` where one is declared), a member of the organization for a file no record owns, a reader of a parent record for an attachments-scope file, anyone for a `public_read` file. The door and the claim path now call one judgement, so one file has one access rule. -A file the caller may not read is answered as the organization-scoped lookup answers a file it cannot see: the value is kept as written, nothing is claimed or copied, and the field's declared `accept` / `maxSize` are not judged against it (constraint verdicts no longer run ahead of the read check). A copy this write made is claimed by the same write without a second question. System writes and context-less programmatic calls have no caller to ask and reference files as before; so does a kernel with no `auth` service, where the download door binds no gate. An authorization-store outage fails the write with its declared envelope instead of reading as either answer. +A file the caller may not read is answered as the organization-scoped lookup answers a file it cannot see: the value is kept as written, nothing is claimed or copied, and constraint verdicts no longer run ahead of the read check. A copy this write made is claimed by the same write without a second question. System writes and context-less programmatic calls have no caller to ask and reference files as before; so does a kernel with no `auth` service, where the download door binds no gate. An authorization-store outage fails the write with its declared envelope instead of reading as either answer. Clause-②: no (narrowing) — a record write reaches fewer files: a file its caller may not read is no longer copied or claimed for it. Nothing that was refused becomes accepted. diff --git a/packages/services/service-storage/src/file-reference-lifecycle.test.ts b/packages/services/service-storage/src/file-reference-lifecycle.test.ts index f3182267aff..e540ae4260e 100644 --- a/packages/services/service-storage/src/file-reference-lifecycle.test.ts +++ b/packages/services/service-storage/src/file-reference-lifecycle.test.ts @@ -1257,7 +1257,7 @@ describe('a write references only a file its caller may read', () => { }); }); - it('a field\'s declared accept / maxSize are not judged against a file the caller may not read', async () => { + it('constraint verdicts do not run ahead of the read check', async () => { const engine = fakeEngine({ files: [file({ mime_type: 'application/pdf', size: 10_000_000 })], registry: { diff --git a/packages/services/service-storage/src/file-reference-lifecycle.ts b/packages/services/service-storage/src/file-reference-lifecycle.ts index 6f1ad3458d0..5bb83f791d9 100644 --- a/packages/services/service-storage/src/file-reference-lifecycle.ts +++ b/packages/services/service-storage/src/file-reference-lifecycle.ts @@ -47,10 +47,9 @@ import { callerContext } from './attachment-access-hooks.js'; * plugin binds to the door's verdict rather than restating it here. A file the * writer may not read is answered exactly as one the organization-scoped * lookup cannot see ({@link systemReadContext}): the value is kept as written, - * nothing is claimed or copied, and its field's `accept` / `maxSize` are not - * judged against it, so the write learns nothing about the file. System - * writes and context-less programmatic calls carry no caller to ask, and - * reference files as before. + * nothing is claimed or copied, and constraint verdicts do not run ahead of + * the read check. System writes and context-less programmatic calls carry no + * caller to ask, and reference files as before. * * ### The one write that cannot honour it, and is refused * @@ -656,8 +655,8 @@ async function applyCopyOnClaim( // The file is real, this caller may read it, and it is about to be // referenced by this field, so its declared constraints bind now — - // before ownership, before any copy. [#25692] Never before the read - // verdict: the refusal names the file's size or type. + // before ownership, before any copy, and [#25692] never ahead of the + // read check above. assertFileConstraints(fieldDefOf(engine, object, field), field, row); // Unclaimed: the after-hook will claim it for this slot. Nothing to copy. From 4f3c21e550894267d48aff9ac2985567ea2b3c18 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 04:22:19 +0000 Subject: [PATCH 6/6] docs(permissions): census row for the claim path's system-write exemption The file-reference claim path reads the hook session's system flag to skip the read verdict for a system write; the system-context page now anchors it. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_016DTRymn12to8YSGMcTzS7h --- content/docs/permissions/system-context.mdx | 1 + 1 file changed, 1 insertion(+) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 8c901f14aa1..f1e86fa8aeb 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -178,6 +178,7 @@ One of the largest consumers of the flag; | 47c | The inbound-mail IMAP sync: `syncMailbox` and `syncAll` admit the system context ALONE | service-mailbox | Get: the sync job reads every mailbox row (its organization, address and IMAP cursor), writes the cursor back, and hands each fetched message to `ingest` under the system context. Lose: nothing a user held — every other caller is refused (`PERMISSION_DENIED`), because a sync stores a mailbox's mail and moves its cursor | `packages/services/service-mailbox/src/sync/mailbox-sync-service.ts#syncMailbox`, `#syncAll` | | 47d | The print service REFUSES the system context: `printRecordToFile` admits a human user acting for themselves alone | service-print | Get: nothing — a system caller is refused (`PERMISSION_DENIED`) before any page is read, session minted or browser started. Lose: nothing a user held — the PDF is rendered as the requesting user and never as the system, so a record is archived only by someone who could read it | `packages/services/service-print/src/print-service.ts#refuseUnlessHumanCaller` | | 47e | The caller-scoped attach verbs REFUSE the system context: `attachFileAs` and `assertMayAttach` admit a signed-in user alone | service-storage | Get: nothing — a system caller is refused (`PERMISSION_DENIED`); the system attaches through `attachFile`, whose rows name no user. Lose: nothing a user held — the caller-scoped verbs insert the attachment under the caller's context so every insert gate judges that user and both rows name them | `packages/services/service-storage/src/file-attachment-service.ts#requireUserContext` | +| 47f | The file-reference claim path asks no read verdict of a system write: a record write claims or copies a referenced file only as its caller may read it (the download door's verdict), and a system write carries no caller to ask | service-storage | Get: a system write claims or copies any file the organization-scoped lookup reaches, as before #25692; a deliberate server-side copy between records runs this way. Lose: nothing a user held — a user write is judged by the download door's verdict, and a context-less programmatic write is not asked either | `packages/services/service-storage/src/file-reference-lifecycle.ts#referencingCallerOf` | ### 5. Actions, metadata plane, provenance, the organization wall