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..060682ab9c1 --- /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 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. 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 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.test.ts b/packages/services/service-storage/src/file-reference-lifecycle.test.ts index 84d9af429a1..e540ae4260e 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('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: { + 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 319ad0b2f3f..5bb83f791d9 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,23 @@ 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 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 * @@ -169,6 +186,97 @@ 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); +} + +/** + * [#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, + * 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 +621,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 +634,9 @@ async function applyCopyOnClaim( data: Record, fileFields: string[], organizationId: string | null, + caller: ExecutionContext | null, + authorizeRead: FileReferenceReadAuthorizer | undefined, + minted: Set | null, ): Promise { for (const field of fileFields) { if (!(field in data)) continue; @@ -533,17 +646,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, 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. @@ -567,7 +680,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)`, ); @@ -592,9 +707,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 +720,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 +845,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 +869,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, mintedCopiesFor(ctx), + ); }, { packageId: PACKAGE_ID }, ); @@ -798,7 +926,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, mintedCopiesFor(ctx), + ); }, { packageId: PACKAGE_ID }, ); @@ -815,10 +946,14 @@ 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, + claimingCallerOf(ctx, token, caller), authorizeRead, + ); } catch (err) { logger.warn( `[storage] file reference: failed to claim ${token} for ${object}/${String(recordId)}.${field} ` + @@ -865,6 +1000,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 +1014,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); + await claimFile( + engine, logger, token, object, recordId, field, organizationId, + claimingCallerOf(ctx, token, caller), authorizeRead, + ); } } catch (err) { logger.warn( 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'); + }); +}); 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?