From eefef159087281f578cb5833669962a55a477fad Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 14:49:24 +0000 Subject: [PATCH 1/3] fix(storage)!: a record write naming a file its caller may not read is refused at the field A write that names a file its caller may not read, or an id the acting organization cannot see, now fails before the driver runs with one 400 VALIDATION_FAILED naming each such field with the ADR-0114 code invalid_value, the form door's envelope. The two answer identically and name the field only. Every named file is judged before any is copied, so a refused field never leaves a copy behind. System and context-less writes, and the outage envelope, are unchanged. Replaces the quiet answer (the id kept as written) in the ADR-0104 2026-10-26 addendum. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01EGkwyoUjm7LjHY7uBXFSgv --- .../26043-unreadable-file-refused-at-field.md | 19 +++ ...0104-field-runtime-value-shape-contract.md | 31 ++-- .../src/file-reference-lifecycle.test.ts | 90 ++++++++--- .../src/file-reference-lifecycle.ts | 148 +++++++++++++----- ...reference-read-verdict.integration.test.ts | 110 +++++++++++-- ...ence-release-read-rule.integration.test.ts | 31 ++-- 6 files changed, 338 insertions(+), 91 deletions(-) create mode 100644 .changeset/26043-unreadable-file-refused-at-field.md diff --git a/.changeset/26043-unreadable-file-refused-at-field.md b/.changeset/26043-unreadable-file-refused-at-field.md new file mode 100644 index 00000000000..89c5c05832b --- /dev/null +++ b/.changeset/26043-unreadable-file-refused-at-field.md @@ -0,0 +1,19 @@ +--- +'@objectstack/service-storage': patch +--- + +fix(storage)!: a record write naming a file its caller may not read is refused at the field + +A record write that names a file its caller may not read is now refused at that field, instead of saving the id as written. The answer is one `400 VALIDATION_FAILED` whose `fields[]` names each such field with the ADR-0114 code `invalid_value`, the same envelope the public form door gives an anonymous submission that carries a file. It names the field only. An id that names no file of the acting organization gets the same answer, so the write cannot tell the two apart. Nothing is claimed, copied or written. System writes and writes with no caller context reference files as before, and an authorization-store outage still fails the write with its declared envelope. + +Clause-②: no (narrowing) — a record write that names a file its caller may not read, or an id no file of the organization has, was saved and is now refused. Nothing that was refused becomes accepted. + +Metadata-protocol: none (a write-time refusal on file fields; nothing a metadata author writes changes) + + + +**BREAKING** for a caller that writes a file id it may not read, shipped as `patch` because the metadata protocol does not move. + +- **Before.** The write saved, and the field kept an id its readers could not use. +- **Now.** The write is refused with `400 VALIDATION_FAILED`, naming the field with `invalid_value`. +- **Fix.** Name a file the caller may read (upload it, or choose one they can open), or leave the field empty. diff --git a/docs/adr/0104-field-runtime-value-shape-contract.md b/docs/adr/0104-field-runtime-value-shape-contract.md index 86d7ec994de..40cf2b1248a 100644 --- a/docs/adr/0104-field-runtime-value-shape-contract.md +++ b/docs/adr/0104-field-runtime-value-shape-contract.md @@ -28,6 +28,12 @@ kept as written and never claimed or copied, the answer #25682 gave across organizations. This qualifies the 2026-07-27 addendum's worst-case row for exclusive ownership. See the 2026-10-26 addendum. + · **Amended** (2026-10-26, #26043) — that kept id is replaced by a + refusal at the field: a write naming a file its caller may not read, or an + id the acting organization cannot see, fails with one `400 + VALIDATION_FAILED` naming each such field with the ADR-0114 code + `invalid_value`, and nothing is written. See "The field refusal" in the + 2026-10-26 addendum. - **Date**: 2026-07-22 - **Issue**: design follow-up generalizing #3405 / #3406 (inline lookup param silently stripped); relates #3407 (silently dropped writes), #1878 / #1891 @@ -1464,17 +1470,20 @@ exists in the acting organization. One file had two access rules. collected (#26046); a member of the organization for a file no record has owned; a reader of any parent for an attachments-scope file; anyone for a `public_read` file. -- **The quiet answer.** At the write door, a file the writer may not read is - answered as one the organization-scoped lookup cannot see: the id is kept - as written, nothing is claimed or copied, and constraint verdicts do not run - ahead of the read check. #25682 gave this answer across organizations; this - addendum states it within one. Unless the record already owned the file, - the kept id gives no reader anything through it: a reader refused - `sys_file` read gets the refused marker (`metadataRefused`), and the - download door judges the file by its own rule, never by this record. A slot - holding a file it does not own is a state that step 5's - `verify-references` classifies as blocking. #26043 tracks replacing this - answer with a field-level refusal. +- **The field refusal.** *(Amended 2026-10-26, #26043. It replaces the quiet + answer this item first stated, which kept the id as written and so left a + slot holding a file it does not own, the state step 5's + `verify-references` classifies as blocking.)* At the write door, a write + naming a file its caller may not read is refused at the field, before the + driver runs: one `400 VALIDATION_FAILED` whose `fields[]` names each such + field with the ADR-0114 code `invalid_value`, the envelope the form door + gives an anonymous submission that carries a file (#25682). Nothing is + claimed, copied or written, and constraint verdicts do not run ahead of the + read check. An id the organization-scoped lookup cannot see gets the same + answer, and the answer names the field only, never the file or the record + that holds it, so the write door cannot tell "may not read" from "does not + exist in this organization". A write with no caller (below) keeps such an + id as written, as before. - **A write's own copy** is claimed by the same write without being asked again. - **Outage.** An authorization-store outage fails the write with its declared 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 fb83cc17550..4a11eebcd1a 100644 --- a/packages/services/service-storage/src/file-reference-lifecycle.test.ts +++ b/packages/services/service-storage/src/file-reference-lifecycle.test.ts @@ -1081,6 +1081,9 @@ describe('File Reference Ownership (ADR-0104 D3 wave 2)', () => { * 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. + * + * [#26043] A file the verdict refuses refuses the write at the field, before + * the driver runs; an id that names no file gets the same answer. */ describe('a write references only a file its caller may read', () => { const CALLER = { userId: 'u_writer', tenantId: 'org_1' }; @@ -1146,16 +1149,26 @@ describe('a write references only a file its caller may read', () => { 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'); + const refusedBy = (p: Promise) => p.then(() => null, (e) => e); + + /** [#26043] The field refusal, as the HTTP boundary answers it: one `invalid_value` entry per refused field. */ + const expectFieldRefused = (err: unknown, fields: string[]) => { + const answered = resolveThrownHttpError(err); + expect({ status: answered.status, code: answered.code }).toEqual({ status: 400, code: 'VALIDATION_FAILED' }); + expect((answered.details?.fields as any[]).map((f) => [f.field, f.code])) + .toEqual(fields.map((f) => [f, 'invalid_value'])); + }; describe('a file another record owns', () => { - it('is not copied for a caller the verdict refuses: the value is kept as written', async () => { + it('is refused at the field for a caller the verdict refuses, and not copied', 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'); + const err = await refusedBy(insertAs(engine, 'product', { image: 'file_a' }, 'p2')); - expect(row.image).toBe('file_a'); + expectFieldRefused(err, ['image']); + expect((engine.tables.product ?? []).map((r) => r.id)).toEqual(['p1']); expect(storage.download).not.toHaveBeenCalled(); expect(storage.upload).not.toHaveBeenCalled(); expect(engine.tables.sys_file).toHaveLength(1); @@ -1163,7 +1176,7 @@ describe('a write references only a file its caller may read', () => { expect(sysFileWrites(engine)).toEqual([]); }); - it('is not copied by an update for a caller the verdict refuses', async () => { + it('is refused at the field by an update for a caller the verdict refuses, and not copied', async () => { const engine = fakeEngine({ files: [ownedByP1()], records: { product: [{ id: 'p1', image: 'file_a' }, { id: 'p2', image: null }] }, @@ -1171,9 +1184,10 @@ describe('a write references only a file its caller may read', () => { const { authorizeRead } = verdict([]); const { storage } = installWith(engine, authorizeRead); - const row = await updateAs(engine, 'product', 'p2', { image: 'file_a' }); + const err = await refusedBy(updateAs(engine, 'product', 'p2', { image: 'file_a' })); - expect(row?.image).toBe('file_a'); + expectFieldRefused(err, ['image']); + expect(engine.tables.product.find((r) => r.id === 'p2')?.image).toBeNull(); expect(storage.download).not.toHaveBeenCalled(); expect(engine.tables.sys_file).toHaveLength(1); expect(engine.tables.sys_file[0]).toMatchObject({ ref_id: 'p1' }); @@ -1199,25 +1213,28 @@ describe('a write references only a file its caller may read', () => { }); describe('a file no record owns', () => { - it('is not claimed for a caller the verdict refuses', async () => { + it('is refused at the field for a caller the verdict refuses, and not claimed', async () => { const engine = fakeEngine({ files: [file()] }); const { authorizeRead } = verdict([]); installWith(engine, authorizeRead); - const row = await insertAs(engine, 'product', { image: 'file_a' }, 'p1'); + const err = await refusedBy(insertAs(engine, 'product', { image: 'file_a' }, 'p1')); - expect(row.image).toBe('file_a'); + expectFieldRefused(err, ['image']); + expect(engine.tables.product ?? []).toEqual([]); 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 () => { + it('is refused at the field by an update for a caller the verdict refuses, and not claimed', 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' }); + const err = await refusedBy(updateAs(engine, 'product', 'p1', { image: 'file_a' })); + expectFieldRefused(err, ['image']); + expect(engine.tables.product[0].image).toBeNull(); expect(engine.tables.sys_file[0].ref_id ?? null).toBeNull(); expect(sysFileWrites(engine)).toEqual([]); }); @@ -1227,7 +1244,7 @@ describe('a write references only a file its caller may read', () => { const { authorizeRead } = verdict([]); installWith(engine, authorizeRead); - await insertAs(engine, 'product', { image: 'file_a' }, 'p1'); + expectFieldRefused(await refusedBy(insertAs(engine, 'product', { image: 'file_a' }, 'p1')), ['image']); 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(); @@ -1243,21 +1260,55 @@ describe('a write references only a file its caller may read', () => { 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 () => { + it('one id of a multiple:true field the caller may not read refuses the field, and none of its ids is taken', 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'); + expectFieldRefused(await refusedBy(insertAs(engine, 'product', { gallery: ['file_b', 'file_a'] }, 'p1')), ['gallery']); - 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'], + ['file_b', null], ]); }); }); + it('[#26043] an id that names no file gets the answer a file the caller may not read gets', async () => { + const engine = fakeEngine({ files: [file()] }); + const { authorizeRead } = verdict([]); + installWith(engine, authorizeRead); + + const unreadable = await refusedBy(insertAs(engine, 'product', { image: 'file_a' }, 'p1')); + const nowhere = await refusedBy(insertAs(engine, 'product', { image: 'file_nowhere' }, 'p2')); + + expectFieldRefused(nowhere, ['image']); + expect(resolveThrownHttpError(nowhere)).toEqual(resolveThrownHttpError(unreadable)); + expect(engine.tables.product ?? []).toEqual([]); + }); + + it('[#26043] every refused field is named, in one answer', async () => { + const engine = fakeEngine({ files: [file()] }); + const { authorizeRead } = verdict([]); + installWith(engine, authorizeRead); + + const err = await refusedBy(insertAs(engine, 'product', { image: 'file_a', gallery: ['file_nowhere'] }, 'p1')); + + expectFieldRefused(err, ['image', 'gallery']); + }); + + it('[#26043] a refused field fails the write before another field\'s file is copied', async () => { + const engine = fakeEngine({ files: [ownedByP1()], records: { product: [{ id: 'p1', image: 'file_a' }] } }); + const { authorizeRead } = verdict(['file_a']); + const { storage } = installWith(engine, authorizeRead); + + const err = await refusedBy(insertAs(engine, 'product', { image: 'file_a', gallery: ['file_nowhere'] }, 'p2')); + + expectFieldRefused(err, ['gallery']); + expect(storage.download).not.toHaveBeenCalled(); + expect(engine.tables.sys_file).toHaveLength(1); + }); + 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 })], @@ -1269,9 +1320,10 @@ describe('a write references only a file its caller may read', () => { const { authorizeRead } = verdict([]); installWith(engine, authorizeRead); - const row = await insertAs(engine, 'product', { image: 'file_a' }, 'p1'); + const err = await refusedBy(insertAs(engine, 'product', { image: 'file_a' }, 'p1')); - expect(row.image).toBe('file_a'); + // The read refusal, not the constraint's `ERR_FILE_CONSTRAINT`. + expectFieldRefused(err, ['image']); expect(engine.tables.sys_file[0].ref_id ?? null).toBeNull(); }); @@ -1298,7 +1350,7 @@ describe('a write references only a file its caller may read', () => { installWith(engine, authorizeRead); const context = { ...CALLER, permissions: ['member_default'], __operationPrivate: 'dropped' }; - await insertAs(engine, 'product', { image: 'file_a' }, 'p2', { session, context }); + await refusedBy(insertAs(engine, 'product', { image: 'file_a' }, 'p2', { session, context })); expect(asked.length).toBeGreaterThan(0); for (const q of asked) { diff --git a/packages/services/service-storage/src/file-reference-lifecycle.ts b/packages/services/service-storage/src/file-reference-lifecycle.ts index a08b784f2c4..a9069ab6eba 100644 --- a/packages/services/service-storage/src/file-reference-lifecycle.ts +++ b/packages/services/service-storage/src/file-reference-lifecycle.ts @@ -46,12 +46,20 @@ import { callerContext } from './attachment-access-hooks.js'; * 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. + * plugin binds to the door's verdict rather than restating it here. System + * writes and context-less programmatic calls carry no caller to ask, and + * reference files as before. + * + * [#26043] A write naming a file its caller may not read is refused at the + * field, before the driver runs and before any file is copied: one + * `400 VALIDATION_FAILED` with an ADR-0114 `invalid_value` entry per field, + * naming the field and nothing about the file ({@link refuseUnreadableFile}). + * An id the organization-scoped lookup cannot see ({@link systemReadContext}) + * gets the same answer, so the write door cannot tell "may not read" from + * "does not exist". It replaces #25951's quiet answer, which kept the id as + * written: that left a slot holding a file it does not own, the state + * ADR-0104's `verify-references` counts as blocking. Constraint verdicts still + * never run ahead of the read check. * * ### An anonymous write carries no file * @@ -183,8 +191,10 @@ function systemWriteContext(organizationId?: string | null): Record { @@ -279,15 +289,19 @@ function claimingCallerOf(ctx: any, fileId: string, caller: ExecutionContext | n } /** - * 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. + * The `sys_file` row a field's id token names, as THIS write may reference it. + * 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. + * caller, whether that caller may read it. + * + * [#26043] When a caller is asked about, a file it may not read and an id the + * lookup cannot see are one answer, `'refused'`, which copy-on-claim refuses + * at the field. With no caller to ask, an id the lookup cannot see is `null`: + * an id the platform does not manage, kept as written. */ async function findReferenceableFile( engine: FileReferenceEngine, @@ -295,11 +309,51 @@ async function findReferenceableFile( organizationId: string | null, caller: ExecutionContext | null, authorizeRead: FileReferenceReadAuthorizer | undefined, -): Promise | null> { +): Promise | 'refused' | 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; + if (!caller || !authorizeRead) return row ?? null; + if (!row) return 'refused'; + return (await authorizeRead(row, caller)) ? row : 'refused'; +} + +/** One field a file-field judgement refuses: the ADR-0114 entry the form door's envelope carries. */ +interface FileFieldRefusal { + field: string; + code: 'invalid_value'; + message: string; + label: string; +} + +/** A field's declared label, or its name when it declares none. */ +function fieldLabelOf(engine: FileReferenceEngine, object: string, field: string): string { + const def = fieldDefOf(engine, object, field); + const declared = typeof def?.label === 'string' ? def.label.trim() : ''; + return declared.length > 0 ? declared : field; +} + +/** + * The form door's envelope (`form-routes.ts`, #25572) for a write's refused + * file fields: one `400 VALIDATION_FAILED` naming every refused field, with + * the closed ADR-0114 field code `invalid_value`. Throws nothing when none is. + */ +function assertNoFileFieldRefused(refused: FileFieldRefusal[]): void { + if (refused.length > 0) throw validationFailure(refused.map((f) => f.message).join('; '), refused); +} + +/** + * [#26043] The refusal of a field naming a file its write's caller may not + * read, or one the acting organization cannot see. It names the field only: + * the same words for both, and nothing about the file or who holds it. + */ +function refuseUnreadableFile(engine: FileReferenceEngine, object: string, field: string): FileFieldRefusal { + const label = fieldLabelOf(engine, object, field); + return { + field, + code: 'invalid_value', + message: `${label} cannot reference this file: it is not a file you can open. ` + + 'Choose a file you can open, or upload it again.', + label, + }; } /** @@ -312,9 +366,8 @@ async function findReferenceableFile( * * The form door's own judgement (`form-routes.ts`, #25572), held at the * engine: the same emptiness rule (`isEmptyForRequired`), and the same - * envelope — one `400 VALIDATION_FAILED` naming every refused field by its - * label with the closed ADR-0114 field code `invalid_value`. The form door - * keeps its own judgement, which answers before anything reaches the engine. + * envelope ({@link assertNoFileFieldRefused}). The form door keeps its own + * judgement, which answers before anything reaches the engine. * * `caller` is {@link referencingCallerOf}'s answer, the caller the read * verdict is asked about: a system write and a context-less programmatic @@ -328,13 +381,11 @@ function assertAnonymousWriteCarriesNoFile( caller: ExecutionContext | null, ): void { if (!caller || caller.userId) return; - const refused: Array<{ field: string; code: 'invalid_value'; message: string; label: string }> = []; + const refused: FileFieldRefusal[] = []; for (const field of fileFields) { if (!Object.prototype.hasOwnProperty.call(data, field)) continue; - const def = fieldDefOf(engine, object, field); - if (isEmptyForRequired(def, data[field])) continue; - const declared = typeof def?.label === 'string' ? def.label.trim() : ''; - const label = declared.length > 0 ? declared : field; + if (isEmptyForRequired(fieldDefOf(engine, object, field), data[field])) continue; + const label = fieldLabelOf(engine, object, field); refused.push({ field, code: 'invalid_value', @@ -343,7 +394,7 @@ function assertAnonymousWriteCarriesNoFile( label, }); } - if (refused.length > 0) throw validationFailure(refused.map((f) => f.message).join('; '), refused); + assertNoFileFieldRefused(refused); } /** Bound on owned files released per record delete. */ @@ -691,8 +742,9 @@ async function copyOwnedFile( * * `recordId` is null on insert: a new record can never be the current owner, * 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. + * ({@link referencingCallerOf}). [#26043] Every file the write names is judged + * for that caller before any is copied or held to its field's declaration, + * and a field naming one it may not read refuses the whole write. */ async function applyCopyOnClaim( engine: FileReferenceEngine, @@ -707,21 +759,40 @@ async function applyCopyOnClaim( authorizeRead: FileReferenceReadAuthorizer | undefined, minted: Set | null, ): Promise { + // [#26043] Judge every file the write names first, so a refused field fails + // the write before any bytes are copied for another one. + const named: Array<{ field: string; value: unknown; rows: Array<[string, Record]> }> = []; + const refused: FileFieldRefusal[] = []; for (const field of fileFields) { if (!(field in data)) continue; const value = data[field]; const tokens = idTokensIn(value); if (tokens.length === 0) continue; - const replacements = new Map(); + const rows: Array<[string, Record]> = []; + let fieldRefused = false; for (const token of [...new Set(tokens)]) { - const row = await findReferenceableFile(engine, token, organizationId, caller, authorizeRead); - // Unknown id: not a file this platform manages (external/legacy value), - // 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; + const found = await findReferenceableFile(engine, token, organizationId, caller, authorizeRead); + // [#26043] A file this write's caller may not read, or one the acting + // organization cannot see: the field is refused. + if (found === 'refused') { + fieldRefused = true; + break; + } + // Unknown id, and no caller to judge: 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. + if (found) rows.push([token, found]); + } + if (fieldRefused) refused.push(refuseUnreadableFile(engine, object, field)); + else if (rows.length > 0) named.push({ field, value, rows }); + } + assertNoFileFieldRefused(refused); + for (const { field, value, rows } of named) { + const replacements = new Map(); + for (const [token, row] of rows) { // 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 @@ -793,7 +864,10 @@ async function claimFile( authorizeRead: FileReferenceReadAuthorizer | undefined, ): Promise { const row = await findReferenceableFile(engine, fileId, organizationId, caller, authorizeRead); - if (!row) return; // not a platform-managed file this write may reference — nothing to own + // Not a platform-managed file this write may reference — nothing to own. + // [#26043] Copy-on-claim refused such a write before it ran; a claim that + // finds one now still takes nothing. + if (!row || row === 'refused') return; if (row.ref_id != null && !isSameSlot(row, object, recordId, field)) { logger.warn( `[storage] file reference: ${fileId} is already owned by ` + 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 index 9491e222827..e0925dbd9b9 100644 --- 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 @@ -18,25 +18,29 @@ * 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: + * when the door serves it, and [#26043] a write naming a file the door refuses + * is refused at the field, with nothing written: * - * - 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; + * - a file owned by a record the writer cannot read: door 403, write refused; * - an attachments-scope file whose only parent the writer cannot read: door - * 403, and the write does not claim it; + * 403, write refused; * - an owner object that declares a `fileAccessDelegate`: the DELEGATE * decides on both doors, refusal and admission alike; + * - a file id that names no file of the organization: the write is refused + * with the same answer, so the write door cannot tell "may not read" from + * "does not exist"; * - 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. + * door refuses the writer, and a context-less write keeps an id that names no + * file, as they 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. + * closed — the write is refused, and the file is never taken. * * ## The stand-ins, and only these * @@ -89,6 +93,10 @@ const F_DEC_ALLOW = 'f_decallow_gg'; const F_DEC_DENY = 'f_decdeny_hh'; /** No owner at all, `acl: 'public_read'`: anyone's to download. */ const F_PUBLIC_FREE = 'f_publicfree_ii'; +/** [#26043] Another unclaimed file of the organization, for the update pin. */ +const F_FREE_SECOND = 'f_freesecond_kk'; +/** [#26043] Token-shaped, and names no `sys_file` row at all. */ +const F_NOWHERE = 'f_nowhere_jj'; /** [#25694] The guest principal an `authRequired: false` endpoint's write executes as (ADR-0138 D9). */ const GUEST_CTX = { principalKind: 'guest' as const, positions: ['guest'], isSystem: false }; @@ -129,6 +137,7 @@ const FILES = [ fileRow(F_DEC_ALLOW, owned('decision', 'dec_allow', 'proof')), fileRow(F_DEC_DENY, owned('decision', 'dec_deny', 'proof')), fileRow(F_PUBLIC_FREE, { acl: 'public_read' }), + fileRow(F_FREE_SECOND), ]; function makeCtx() { @@ -232,7 +241,7 @@ describe('#25692 — a record write claims or copies only a file the download do label: 'Contract', fields: { title: { name: 'title', type: 'text' }, - doc: { name: 'doc', type: 'file' }, + doc: { name: 'doc', label: 'Document', type: 'file' }, annexes: { name: 'annexes', type: 'file', multiple: true }, }, }, @@ -329,29 +338,92 @@ describe('#25692 — a record write claims or copies only a file the download do return { recordId, held, outcome }; }; - describe('a file the door refuses the writer is neither copied nor claimed by the writer\'s write', () => { + /** + * [#26043] The refusal of a write naming a file its caller may not read, as + * the HTTP boundary answers it (ADR-0112: status and code, through the one + * mapping both dispatcher exits and the REST door share): one ADR-0114 + * `invalid_value` entry per refused field, naming the field and nothing + * about the file. The body is the same whatever the file is, so it is + * pinned whole. + */ + const refusedBody = (field: string, label: string) => { + const message = `${label} cannot reference this file: it is not a file you can open. ` + + 'Choose a file you can open, or upload it again.'; + return { + status: 400, + code: 'VALIDATION_FAILED', + declaredStatus: 400, + declaredCode: 'VALIDATION_FAILED', + message, + details: { fields: [{ field, code: 'invalid_value', message, label }] }, + }; + }; + const refusedDocBody = refusedBody('doc', 'Document'); + + /** Write `fileId` into a new contract's `doc` as `context`, expecting the refusal: nothing is written. */ + const expectFieldRefused = async (fileId: string, context: Record) => { + const recordId = `c_new_${++seq}`; + const before = await fileRows(); + const err = await engine + .insert('contract', { id: recordId, title: recordId, doc: fileId } as any, { context }) + .then(() => null, (e: any) => e); + expect(resolveThrownHttpError(err)).toEqual(refusedDocBody); + expect(await storedDoc(recordId)).toBeUndefined(); + expect(await fileRows()).toEqual(before); + }; + + describe('[#26043] a write naming a file the door refuses the writer is refused at the field', () => { 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); + await expectFieldRefused(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); + await expectFieldRefused(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'); + await expectFieldRefused(F_DEC_DENY, WRITER_CTX); + }); + + it('a file id that names no file of the organization gets the same answer', async () => { + expect(await fileById(F_NOWHERE)).toBeUndefined(); + + await expectFieldRefused(F_NOWHERE, WRITER_CTX); + }); + + it('an update naming such a file is refused, and the record keeps what it held', async () => { + const { recordId, held } = await writeAs(F_FREE_SECOND, WRITER_CTX); + const err = await engine + .update('contract', { id: recordId, doc: F_CLOSED } as any, { context: WRITER_CTX }) + .then(() => null, (e: any) => e); + + expect(resolveThrownHttpError(err)).toEqual(refusedDocBody); + expect(await storedDoc(recordId)).toBe(held); + expect(await fileById(String(held))).toMatchObject(owned('contract', recordId, 'doc')); + expect(await fileById(F_CLOSED)).toMatchObject(owned('vault', 'v_closed', 'doc')); + }); + + it('a refused file in one field fails the whole write, before any other field\'s file is copied', async () => { + const recordId = `c_new_${++seq}`; + const before = await fileRows(); + const err = await engine + .insert('contract', { id: recordId, title: recordId, doc: F_OPEN, annexes: [F_NOWHERE] } as any, { context: WRITER_CTX }) + .then(() => null, (e: any) => e); + + // `doc` is declared first, so its copy would run first; the field + // without a label is named by its name. + expect(resolveThrownHttpError(err)).toEqual(refusedBody('annexes', 'annexes')); + expect(await fileRows()).toEqual(before); }); }); @@ -411,13 +483,14 @@ describe('#25692 — a record write claims or copies only a file the download do expect((await fileById(F_ATTACHED))?.ref_id ?? null).toBeNull(); }); - it('any other fault fails closed: the file is kept as written, never taken', async () => { + it('any other fault fails closed: the write is refused, and the file is never taken', async () => { attachmentReadFault = new Error('driver fault'); try { - expect((await writeAs(F_ATTACHED, WRITER_CTX)).outcome).toBe('kept'); + await expectFieldRefused(F_ATTACHED, WRITER_CTX); } finally { attachmentReadFault = null; } + expect((await fileById(F_ATTACHED))?.ref_id ?? null).toBeNull(); }); }); @@ -427,6 +500,13 @@ describe('#25692 — a record write claims or copies only a file the download do expect(outcome).toBe('copied'); }); + it('[#26043] a context-less write has no caller to judge, and keeps an id that names no file, as before', async () => { + const recordId = `c_new_${++seq}`; + await engine.insert('contract', { id: recordId, title: recordId, doc: F_NOWHERE } as any); + + expect(await storedDoc(recordId)).toBe(F_NOWHERE); + }); + describe('[#25694] an anonymous write carries no file, whatever the door would serve it', () => { /** * The refusal as the HTTP boundary answers it (ADR-0112: code and status), diff --git a/packages/services/service-storage/src/file-reference-release-read-rule.integration.test.ts b/packages/services/service-storage/src/file-reference-release-read-rule.integration.test.ts index 142c2220cec..f13afeb05b6 100644 --- a/packages/services/service-storage/src/file-reference-release-read-rule.integration.test.ts +++ b/packages/services/service-storage/src/file-reference-release-read-rule.integration.test.ts @@ -18,7 +18,8 @@ * that held the file and one who may not. For every way a file leaves its * slot, under both collection settings: * - * - the member refused the record is refused the file, on both doors; + * - the member refused the record is refused the file, on both doors (a + * write naming it is refused at the field, #26043); * - the member who reads the record still reads it, and may take it back; * - once the record is deleted, only the uploader reads the file. * @@ -46,6 +47,7 @@ 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 { resolveThrownHttpError } from '@objectstack/types'; import { StorageServicePlugin } from './storage-service-plugin.js'; import { installDefaultOrganization } from './default-organization.testkit.js'; @@ -261,17 +263,28 @@ async function boot(tombstones: boolean) { /** * Write `fileId` into a new contract's `doc` as `user`, and report what the - * write did with it: `claimed` (the record owns the file it named) or `kept` - * (the record holds the id as written and owns nothing). + * write did with it: `claimed` (the record owns the file it named) or + * [#26043] `refused` (the write was refused at `doc`, and no record was + * written). */ let seq = 0; const writeAs = async (fileId: string, user: string) => { const recordId = `c_${++seq}`; - await engine.insert('contract', { id: recordId, doc: fileId } as any, { context: { userId: user } }); + const err = await engine + .insert('contract', { id: recordId, doc: fileId } as any, { context: { userId: user } }) + .then(() => null, (e: any) => e); const held = ((await engine.find('contract', { where: { id: recordId }, context: SYS_RAW })) as any[])[0]?.doc; + if (err) { + const answered = resolveThrownHttpError(err); + const fields = (answered.details?.fields as any[] | undefined)?.map((f) => [f.field, f.code]); + return answered.status === 400 && answered.code === 'VALIDATION_FAILED' && held === undefined + && JSON.stringify(fields) === JSON.stringify([['doc', 'invalid_value']]) + ? 'refused' + : `unexpected (${answered.status} ${answered.code})`; + } const row = await fileById(fileId); const owns = row?.ref_object === 'contract' && row?.ref_id === recordId && row?.ref_field === 'doc'; - return held === fileId && owns ? 'claimed' : held === fileId && !owns ? 'kept' : `unexpected (held ${String(held)})`; + return held === fileId && owns ? 'claimed' : `unexpected (held ${String(held)}, owns ${owns})`; }; return { @@ -301,7 +314,7 @@ describe('#26046 — a file its owning record lets go keeps that record\'s read describe.each(RELEASES)('$name', (r) => { it('a member refused the record is refused the file, at the download door and at a write', async () => { expect(await h.door(r.file, OUTSIDER)).toEqual(DENIED); - expect(await h.writeAs(r.file, OUTSIDER)).toBe('kept'); + expect(await h.writeAs(r.file, OUTSIDER)).toBe('refused'); expect((await h.fileById(r.file))?.ref_id ?? null).toBeNull(); }); @@ -316,7 +329,7 @@ describe('#26046 — a file its owning record lets go keeps that record\'s read async () => { if (!r.recordSurvives) { expect(await h.door(r.file, READER)).toEqual(DENIED); - expect(await h.writeAs(r.file, READER)).toBe('kept'); + expect(await h.writeAs(r.file, READER)).toBe('refused'); return; } expect(await h.door(r.file, READER)).toEqual(ALLOWED); @@ -347,7 +360,7 @@ describe('#26046 — a file its owning record lets go keeps that record\'s read describe.each(RELEASES)('$name', (r) => { it('a write by a member refused the record neither takes nor revives the file', async () => { - expect(await h.writeAs(r.file, OUTSIDER)).toBe('kept'); + expect(await h.writeAs(r.file, OUTSIDER)).toBe('refused'); expect(await h.fileById(r.file)).toMatchObject({ status: 'deleted', ref_id: null }); }); @@ -357,7 +370,7 @@ describe('#26046 — a file its owning record lets go keeps that record\'s read : 'with the record gone, its reader may not take it back either', async () => { if (!r.recordSurvives) { - expect(await h.writeAs(r.file, READER)).toBe('kept'); + expect(await h.writeAs(r.file, READER)).toBe('refused'); expect(await h.fileById(r.file)).toMatchObject({ status: 'deleted', ref_id: null }); return; } From 66398e790e92bc8d4abf1c90768de56bab5c0788 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 14:49:24 +0000 Subject: [PATCH 2/3] fix(storage): a file governed by a record's rule gets the record-governed link lifetime The download door now picks the signed-link lifetime from the same recognition the read verdict uses (releasingRecordOf), so every file a record's rule governs gets the record-governed lifetime. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01EGkwyoUjm7LjHY7uBXFSgv --- .../26043-unreadable-file-refused-at-field.md | 2 ++ .../service-storage/src/metadata-store.ts | 16 ++++++++++++++++ .../service-storage/src/storage-routes.test.ts | 14 ++++++++++++++ .../service-storage/src/storage-routes.ts | 16 +++++++++++----- .../src/storage-service-plugin.ts | 10 +++++----- 5 files changed, 48 insertions(+), 10 deletions(-) diff --git a/.changeset/26043-unreadable-file-refused-at-field.md b/.changeset/26043-unreadable-file-refused-at-field.md index 89c5c05832b..7afdd6d62ab 100644 --- a/.changeset/26043-unreadable-file-refused-at-field.md +++ b/.changeset/26043-unreadable-file-refused-at-field.md @@ -6,6 +6,8 @@ fix(storage)!: a record write naming a file its caller may not read is refused a A record write that names a file its caller may not read is now refused at that field, instead of saving the id as written. The answer is one `400 VALIDATION_FAILED` whose `fields[]` names each such field with the ADR-0114 code `invalid_value`, the same envelope the public form door gives an anonymous submission that carries a file. It names the field only. An id that names no file of the acting organization gets the same answer, so the write cannot tell the two apart. Nothing is claimed, copied or written. System writes and writes with no caller context reference files as before, and an authorization-store outage still fails the write with its declared envelope. +A file governed by a record's rule also gets the record-governed link lifetime at the download door. + Clause-②: no (narrowing) — a record write that names a file its caller may not read, or an id no file of the organization has, was saved and is now refused. Nothing that was refused becomes accepted. Metadata-protocol: none (a write-time refusal on file fields; nothing a metadata author writes changes) diff --git a/packages/services/service-storage/src/metadata-store.ts b/packages/services/service-storage/src/metadata-store.ts index b9c7a21098b..5df48ec2ed1 100644 --- a/packages/services/service-storage/src/metadata-store.ts +++ b/packages/services/service-storage/src/metadata-store.ts @@ -54,6 +54,22 @@ export interface FileRecord { organization_id?: string | null; } +/** + * [#26046] The record that let this file go, when one did and none has + * claimed the file since, or `null`. Such a file is governed by that record's + * read rule: the download verdict judges it so, and its signed link gets the + * record-governed lifetime. Read by both, so they cannot disagree about which + * files a record still governs. + */ +export function releasingRecordOf( + file: Pick, +): { object: string; id: string } | null { + const object = file.released_ref_object; + const id = file.released_ref_id; + if (!object || id == null || id === '') return null; + return { object: String(object), id: String(id) }; +} + /** * The acting session's organization, threaded into a `sys_file` write (#12745) * or a `sys_upload_session` write (#12928). diff --git a/packages/services/service-storage/src/storage-routes.test.ts b/packages/services/service-storage/src/storage-routes.test.ts index c0471011639..ef6f3f72ee0 100644 --- a/packages/services/service-storage/src/storage-routes.test.ts +++ b/packages/services/service-storage/src/storage-routes.test.ts @@ -573,6 +573,20 @@ describe('Storage REST Routes', () => { expect(ttlOf(res._json.data.url)).toBeGreaterThan(1700); expect(authorizeFileRead).toHaveBeenCalledOnce(); }); + + // [#26046] A file a record let go is still governed by that record's + // rule, so its link gets the record-governed lifetime too. + it('mints a file a record let go with the record-governed lifetime', async () => { + const { server, store: s, authorizeFileRead } = serverWith('allow', { downloadTtl: 120, presignedTtl: 1800 }); + await commit(s, owned('fo7', { + ref_object: null, ref_id: null, ref_field: null, + released_ref_object: 'product', released_ref_id: 'p1', + })); + const res = await hit(server, '/api/v1/storage/files/:fileId/url', 'fo7'); + expect(res._status).toBe(200); + expect(ttlOf(res._json.data.url)).toBeLessThanOrEqual(120); + expect(authorizeFileRead).toHaveBeenCalledOnce(); + }); }); }); diff --git a/packages/services/service-storage/src/storage-routes.ts b/packages/services/service-storage/src/storage-routes.ts index b34c59e7fdc..95d899032bc 100644 --- a/packages/services/service-storage/src/storage-routes.ts +++ b/packages/services/service-storage/src/storage-routes.ts @@ -28,7 +28,7 @@ import type { // [#22175] Whether a door's scoped by-id write can reach the row it names — // the store's own answer, asked before the write. [#22332] And the chunk // door's conditional progress write. -import { organizationOutOfWriteReach, updateSessionProgressIfUnchanged } from './metadata-store.js'; +import { organizationOutOfWriteReach, releasingRecordOf, updateSessionProgressIfUnchanged } from './metadata-store.js'; import type { LocalStorageAdapter } from './local-storage-adapter.js'; // The ONE refusal `verifyToken` throws, recognised by its brand — never by // reading the message, which once let a genuine fault whose text said @@ -362,9 +362,11 @@ export interface StorageRoutesOptions { */ resolveFileHolder?: (file: FileRecord) => Promise; /** - * TTL (seconds) for the signed URL minted on a GATED attachments download. - * Short by design — the link is followed immediately after an explicit - * click. Default 300 (5 min). Non-gated downloads keep `presignedTtl`. + * TTL (seconds) for the signed URL minted on a download governed by a + * record's rule: an attachments-scope file, a field-owned file, and + * [#26046] a file a record let go. Short by design — the link is followed + * immediately after an explicit click. Default 300 (5 min). Every other + * download keeps `presignedTtl`. */ downloadTtl?: number; /** Optional logger for the one-time open-mode notice. */ @@ -601,7 +603,11 @@ export function registerStorageFamily( } return false; } - return gated ? downloadTtl : currentLimits().presignedTtl; + // [#26046] A file a record let go is judged by that record's rule, so its + // link gets the record-governed lifetime too. Who is served stays the + // verdict's alone: the session gate above asks it, as for any file no + // record owns. + return gated || releasingRecordOf(file) ? downloadTtl : currentLimits().presignedTtl; }; // ── Download servability (#10246) ──────────────────────────────────── diff --git a/packages/services/service-storage/src/storage-service-plugin.ts b/packages/services/service-storage/src/storage-service-plugin.ts index 24540c7fe6b..97a098bb395 100644 --- a/packages/services/service-storage/src/storage-service-plugin.ts +++ b/packages/services/service-storage/src/storage-service-plugin.ts @@ -56,7 +56,7 @@ import { LocalStorageAdapter } from './local-storage-adapter.js'; import type { LocalStorageAdapterOptions } from './local-storage-adapter.js'; import { S3StorageAdapter } from './s3-storage-adapter.js'; import type { S3StorageAdapterOptions } from './s3-storage-adapter.js'; -import { StorageMetadataStore } from './metadata-store.js'; +import { StorageMetadataStore, releasingRecordOf } from './metadata-store.js'; import type { FileRecord } from './metadata-store.js'; import type { TenancyPosture } from '@objectstack/spec/security'; import { registerStorageFamily } from './storage-routes.js'; @@ -1595,10 +1595,10 @@ async function judgeFileRead( // it since. It is still the file that record held, so it keeps that // record's read rule, asked exactly as the arm above asks it. A record that // no longer exists is read by nobody, so only the uploader reads the file. - if (file.released_ref_object && file.released_ref_id != null && file.released_ref_id !== '') { - const formerId = String(file.released_ref_id); - const readable = await readableFieldOwners(registry, engine, String(file.released_ref_object), [formerId], context); - return readable.has(formerId) ? 'allow' : 'deny'; + const releasing = releasingRecordOf(file); + if (releasing) { + const readable = await readableFieldOwners(registry, engine, releasing.object, [releasing.id], context); + return readable.has(releasing.id) ? 'allow' : 'deny'; } // [#25693] Unclaimed (neither an attachments scope nor a field owner): From c075544b6b3d23fad8f4da0fc0c7ede8459a511d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 15:47:41 +0000 Subject: [PATCH 3/3] test(dogfood): the field-zoo round-trip writes real files into its media fields The matrix wrote ids that name no file, relying on the write keeping them as written. Such a write is now refused at the field, so the HTTP suite uploads one real file per media field and asserts the read form carries its id. The matrix keeps its stored-shape stand-ins for the value-shape test. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01EGkwyoUjm7LjHY7uBXFSgv --- .../test/field-zoo-roundtrip.dogfood.test.ts | 39 ++++++++++++++++++- packages/qa/dogfood/test/field-zoo.matrix.ts | 7 ++-- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/packages/qa/dogfood/test/field-zoo-roundtrip.dogfood.test.ts b/packages/qa/dogfood/test/field-zoo-roundtrip.dogfood.test.ts index 7753cc59806..2a7ba6f2c72 100644 --- a/packages/qa/dogfood/test/field-zoo-roundtrip.dogfood.test.ts +++ b/packages/qa/dogfood/test/field-zoo-roundtrip.dogfood.test.ts @@ -18,6 +18,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { SECRET_MASK } from '@objectstack/objectql'; +import { FILE_REFERENCE_TYPES } from '@objectstack/spec/data'; import { type VerifyStack } from '@objectstack/verify'; import { bootShowcase } from './showcase-boot.js'; @@ -27,6 +28,8 @@ describe('dogfood: field-type capability matrix round-trips over HTTP (#2004)', let record: Record; /** Resolved reference ids, keyed by field — the assertions compare to these. */ const referenceIds: Record = {}; + /** Uploaded `sys_file` ids, keyed by media field — the assertions compare to these. */ + const fileIds: Record = {}; beforeAll(async () => { stack = await bootShowcase(); @@ -54,13 +57,38 @@ describe('dogfood: field-type capability matrix round-trips over HTTP (#2004)', referenceIds[target.field] = id as string; } + // [#26043] Upload a REAL file for each media field. A write naming an id + // that is no file of the organization is now refused at the field, as one + // naming a file the writer may not read is, so the matrix's stand-in ids + // would be refused. What this file proves for the media family is + // unchanged: the stored id round-trips as the same file. + for (const c of MATRIX) { + if (!FILE_REFERENCE_TYPES.has(c.type) || !('write' in c.check) || c.check.write === undefined) continue; + const bytes = `zoo ${c.field}`; + const presign = await stack.apiAs(token, 'POST', '/storage/upload/presigned', { + filename: `${c.field}.txt`, mimeType: 'text/plain', size: bytes.length, + }); + expect(presign.status, `presign for ${c.field}`).toBe(200); + const { data } = (await presign.json()) as { data: { uploadUrl: string; fileId: string; headers?: Record } }; + const put = await stack.raw(String(data.uploadUrl).replace(/^https?:\/\/[^/]+/, ''), { + method: 'PUT', headers: data.headers ?? { 'content-type': 'text/plain' }, body: bytes, + }); + expect(put.status, `upload for ${c.field}`).toBeLessThan(300); + const complete = await stack.apiAs(token, 'POST', '/storage/upload/complete', { fileId: data.fileId }); + expect(complete.status, `complete for ${c.field}`).toBe(200); + fileIds[c.field] = String(data.fileId); + } + // Build the create body from every entry that carries a `write` value // (+ required name). `present`/`computed` server-owned fields are skipped. - // A reference placeholder resolves to the id seeded above. + // A reference placeholder resolves to the id seeded above, and a media + // field to the file uploaded above. const body: Record = { name: 'zoo-roundtrip' }; for (const c of MATRIX) { if (!('write' in c.check) || c.check.write === undefined) continue; - body[c.field] = c.field in referenceIds ? referenceIds[c.field] : c.check.write; + body[c.field] = c.field in referenceIds + ? referenceIds[c.field] + : c.field in fileIds ? fileIds[c.field] : c.check.write; } const created = await stack.apiAs(token, 'POST', '/data/showcase_field_zoo', body); @@ -88,6 +116,13 @@ describe('dogfood: field-type capability matrix round-trips over HTTP (#2004)', const actual = record[c.field]; switch (c.check.kind) { case 'equal': + // A media field reads back in its expanded form, the file object + // the stored id resolves to (ADR-0104 D3 wave 2), so its `id` is + // the stored id. + if (c.field in fileIds) { + expect(actual).toMatchObject({ id: fileIds[c.field] }); + break; + } expect(actual).toEqual( c.field in referenceIds ? referenceIds[c.field] : c.check.write, ); diff --git a/packages/qa/dogfood/test/field-zoo.matrix.ts b/packages/qa/dogfood/test/field-zoo.matrix.ts index ecf2e17cf01..964286288a0 100644 --- a/packages/qa/dogfood/test/field-zoo.matrix.ts +++ b/packages/qa/dogfood/test/field-zoo.matrix.ts @@ -138,9 +138,10 @@ export const MATRIX: FieldCase[] = [ { field: 'f_record', type: 'record', check: { kind: 'equal', write: { home: '+1', work: '+2' } } }, // media — the STORED form is an opaque sys_file id (ADR-0104 D3 wave 2); the // inline `{url, …}` blob is the expanded READ form, derived by the resolver - // rather than written. These ids match no sys_file row here, which is the - // point: the round-trip must return the stored id verbatim when there is - // nothing to expand it into. + // rather than written. The `write` values are stand-ins in the stored shape + // (the value-shape test parses them): [#26043] a write naming an id that is + // no file of the organization is refused, so the HTTP suite uploads a real + // file for each and asserts the read form carries its id. { field: 'f_video', type: 'video', check: { kind: 'equal', write: 'file_zoo_video' } }, { field: 'f_audio', type: 'audio', check: { kind: 'equal', write: 'file_zoo_audio' } }, { field: 'f_composite', type: 'composite', check: { kind: 'equal', write: { label: 'x', n: 1 } } },