diff --git a/.changeset/27767-sys-file-reference-columns.md b/.changeset/27767-sys-file-reference-columns.md new file mode 100644 index 00000000000..a9f15e8ef05 --- /dev/null +++ b/.changeset/27767-sys-file-reference-columns.md @@ -0,0 +1,20 @@ +--- +'@objectstack/service-storage': patch +--- + +fix(service-storage): `sys_file`'s file-reference columns (`ref_object`, `ref_id`, `ref_field`, `released_ref_object`, `released_ref_id`) are owned by the file-reference lifecycle and can no longer be set or changed by a write that does not come from it + +Clause-②: no (narrowing) +Metadata-protocol: none + + + +The five columns that record which record and field hold a file, and which record last let it go, take the same form as the five storage-owned columns: + +- **Update.** They are now `readonly` on `sys_file`. A non-system update, by id or by predicate, keeps the values the lifecycle wrote and lands the rest of the payload; the write answers as before and the engine logs the dropped column at `warn`, as it does for every read-only column. A caller that passes `strictReadonlyWrites` is refused with `ERR_READONLY_FIELD_REJECTED`, as for every read-only column. +- **Create.** The create-side guard of the storage-owned columns now drops these five too: a non-system create of a `sys_file` row keeps the caller's other columns, drops any value it gives them, starts `pending` as before, and logs one `warn` naming the dropped columns. The create is not refused. +- **Unchanged.** The file-reference lifecycle writes them under the system context, as before: a file field's claim, release and copy, and the operator backfill of file references. + +A file is attached to a record by writing its id into the record's file field, never by writing `sys_file` directly. + +`system-file.object.ts` now also states that the presence of `key` and `status` on a `sys_file` row is the storage writers' invariant, not the engine's (the engine checks a `readonly` field for shape only on insert), and the guard's unused helper export is no longer exported. diff --git a/packages/services/service-storage/src/objects/system-file.object.ts b/packages/services/service-storage/src/objects/system-file.object.ts index 846947846ee..f7c568838c7 100644 --- a/packages/services/service-storage/src/objects/system-file.object.ts +++ b/packages/services/service-storage/src/objects/system-file.object.ts @@ -42,9 +42,41 @@ const UPLOAD_SCOPE_LABELS: Record = { * under the system context, move these columns: the upload doors through * `StorageMetadataStore`, the claim, release and copy of a file field, and the * attachment tombstone. + * + * `key` and `status` are also declared `required`, but the engine does not + * enforce that on an INSERT: its record validator checks a `readonly` field + * for shape only. Their presence is the storage writers' invariant, not the + * engine's: every writer above sets both (a presigned or chunked upload starts + * `pending` with its `key`, `/upload/complete` commits it, and a field copy + * writes a fresh `key` and `committed`). A non-system create, the one write + * that does not come from those writers, has its `key` dropped and starts + * `pending`, so no file field can claim it. The declaration stays as it is. */ export const SYS_FILE_STORAGE_OWNED_COLUMNS = ['key', 'mime_type', 'size', 'status', 'etag'] as const; +/** + * The `sys_file` columns the file-reference lifecycle owns: which record and + * field hold the file (`ref_object`, `ref_id`, `ref_field`) and which record + * last let it go (`released_ref_object`, `released_ref_id`), ADR-0104 D3 + * wave 2. + * + * They take the same form as {@link SYS_FILE_STORAGE_OWNED_COLUMNS}: each is + * `readonly` below, so the engine takes a non-system caller's value back out + * of an UPDATE, and the same create-side guard + * (`installSysFileStorageOwnedColumnsGuard`) drops them from a non-system + * INSERT. Neither refuses the write. Only the lifecycle moves them, under the + * system context: a file field's claim, release and copy + * (`file-reference-lifecycle.ts`, `field-file-service.ts`) and the operator + * backfill of file references. + */ +export const SYS_FILE_REFERENCE_COLUMNS = [ + 'ref_object', + 'ref_id', + 'ref_field', + 'released_ref_object', + 'released_ref_id', +] as const; + /** * System File Object * @@ -170,11 +202,15 @@ export const SystemFile = ObjectSchema.create({ // // NULL on: freshly uploaded but not yet claimed by a record write, and on // every attachments-surface file (those are governed by sys_attachment). + // + // Lifecycle-owned (SYS_FILE_REFERENCE_COLUMNS), as are the two released_* + // columns below. ref_object: Field.text({ label: 'Referencing Object', maxLength: 128, description: 'Short object name of the record whose field owns this file', group: 'Field Reference', + readonly: true, }), ref_id: Field.text({ @@ -182,6 +218,7 @@ export const SystemFile = ObjectSchema.create({ maxLength: 64, description: 'Primary key of the record whose field owns this file', group: 'Field Reference', + readonly: true, }), ref_field: Field.text({ @@ -189,6 +226,7 @@ export const SystemFile = ObjectSchema.create({ maxLength: 128, description: 'Field name on the owning record that holds this file id', group: 'Field Reference', + readonly: true, }), // [#26046] The record whose field last owned this file, kept when that @@ -203,6 +241,7 @@ export const SystemFile = ObjectSchema.create({ maxLength: 128, description: 'Short object name of the record that last owned this file, after it let the file go', group: 'Field Reference', + readonly: true, }), released_ref_id: Field.text({ @@ -210,6 +249,7 @@ export const SystemFile = ObjectSchema.create({ maxLength: 64, description: 'Primary key of the record that last owned this file, after it let the file go', group: 'Field Reference', + readonly: true, }), metadata: Field.text({ diff --git a/packages/services/service-storage/src/sys-file-storage-owned-columns.integration.test.ts b/packages/services/service-storage/src/sys-file-storage-owned-columns.integration.test.ts index b62cbf26700..d6a03359323 100644 --- a/packages/services/service-storage/src/sys-file-storage-owned-columns.integration.test.ts +++ b/packages/services/service-storage/src/sys-file-storage-owned-columns.integration.test.ts @@ -18,6 +18,12 @@ * `pending`. Neither refuses the write. The service's own writes, all under * the system context, still move every column: presign, complete, claim. * + * The file-reference columns (`ref_object`, `ref_id`, `ref_field`, + * `released_ref_object`, `released_ref_id`) take the same form: the + * file-reference lifecycle owns them, so they are `readonly` and the same + * create-side guard drops them from a non-system create, while the lifecycle's + * claim, release and copy, all under the system context, still write them. + * * Driven through the doors as they ship, as the #26479 rig is: one real * `ObjectQL` engine over sqlite `:memory:`, `StorageServicePlugin` booted on * the local adapter, the upload doors called as routes, and the data writes @@ -34,7 +40,11 @@ import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { resolveThrownHttpError } from '@objectstack/types'; import { StorageServicePlugin } from './storage-service-plugin.js'; -import { SystemFile, SYS_FILE_STORAGE_OWNED_COLUMNS } from './objects/system-file.object.js'; +import { + SystemFile, + SYS_FILE_REFERENCE_COLUMNS, + SYS_FILE_STORAGE_OWNED_COLUMNS, +} from './objects/system-file.object.js'; import { installDefaultOrganization } from './default-organization.testkit.js'; const BASE = '/api/v1/storage'; @@ -303,4 +313,126 @@ describe('sys_file storage-owned columns are not writable outside the storage se expect(await owned(fileId)).toEqual({ ...FORGED }); }); + + // ── The file-reference columns ──────────────────────────────────── + + const references = async (id: string) => { + const row = await fileRow(id); + return Object.fromEntries(SYS_FILE_REFERENCE_COLUMNS.map((c) => [c, row?.[c] ?? null])); + }; + const UNREFERENCED = { ref_object: null, ref_id: null, ref_field: null, released_ref_object: null, released_ref_id: null }; + /** A value for every reference column that differs from any the lifecycle writes in this file. */ + const NAMED = { + ref_object: 'named_object', + ref_id: 'r_named', + ref_field: 'named_field', + released_ref_object: 'named_released_object', + released_ref_id: 'r_named_released', + } as const; + + /** A committed upload claimed by a fresh `media` record's `poster`: the lifecycle's own claim. */ + const claimedUpload = async () => { + const fileId = await committedUpload(); + const recordId = `m_${++seq}`; + await engine.insert('media', { id: recordId, name: 'owner', poster: fileId } as any, { context: WRITER_CTX }); + return { fileId, recordId }; + }; + + it('declares exactly the five file-reference columns, each readonly on sys_file', () => { + expect([...SYS_FILE_REFERENCE_COLUMNS].sort()).toEqual([ + 'ref_field', 'ref_id', 'ref_object', 'released_ref_id', 'released_ref_object', + ]); + for (const column of SYS_FILE_REFERENCE_COLUMNS) { + expect((SystemFile.fields as Record)[column]?.readonly, column).toBe(true); + } + }); + + it.each(SYS_FILE_REFERENCE_COLUMNS)('an update by id cannot change %s, and the rest of the write lands', async (column) => { + const { fileId } = await claimedUpload(); + const before = await references(fileId); + expect(before.ref_object).toBe('media'); + + await engine.update('sys_file', { id: fileId, name: 'renamed.png', [column]: NAMED[column] } as any, { context: WRITER_CTX }); + + expect(await references(fileId)).toEqual(before); + expect((await fileRow(fileId))?.name).toBe('renamed.png'); + }); + + it('a predicate update cannot change any of them either', async () => { + const fileId = await committedUpload(); + + await engine.update('sys_file', { ...NAMED, acl: 'private' } as any, { where: { id: fileId }, context: WRITER_CTX } as any); + + expect(await references(fileId)).toEqual(UNREFERENCED); + expect((await fileRow(fileId))?.acl).toBe('private'); + }); + + it('a create cannot set any of them: the row is written with none of the caller\'s values', async () => { + const fileId = `f_named_${++seq}`; + + await engine.insert('sys_file', { id: fileId, name: 'named.png', owner_id: WRITER, ...NAMED } as any, { context: WRITER_CTX }); + + expect(await references(fileId)).toEqual(UNREFERENCED); + expect((await fileRow(fileId))?.name).toBe('named.png'); + }); + + it('a batch create cannot set them on any row', async () => { + const a = `f_named_${++seq}`; + const b = `f_named_${++seq}`; + + await engine.insert('sys_file', [ + { id: a, name: 'a.png', ...NAMED }, + { id: b, name: 'b.png', ref_object: 'media', ref_id: 'm_other', ref_field: 'poster' }, + ] as any, { context: WRITER_CTX }); + + expect(await references(a)).toEqual(UNREFERENCED); + expect(await references(b)).toEqual(UNREFERENCED); + }); + + it('the lifecycle\'s claim, release and copy still write them', async () => { + const { fileId, recordId } = await claimedUpload(); + expect(await references(fileId)).toEqual({ ...UNREFERENCED, ref_object: 'media', ref_id: recordId, ref_field: 'poster' }); + + // Writing the owned id into a second slot copies the file into a fresh row the second slot owns. + const second = `m_${++seq}`; + await engine.insert('media', { id: second, name: 'second', poster: fileId } as any, { context: WRITER_CTX }); + const copyId = (await record(second))?.poster as string; + expect(copyId).not.toBe(fileId); + expect(await references(copyId)).toEqual({ ...UNREFERENCED, ref_object: 'media', ref_id: second, ref_field: 'poster' }); + + // Clearing the field releases the file and keeps the record that let it go. + await engine.update('media', { id: recordId, poster: null } as any, { context: WRITER_CTX }); + expect(await references(fileId)).toEqual({ ...UNREFERENCED, released_ref_object: 'media', released_ref_id: recordId }); + }); + + it('a system create and a system update keep the values they write', async () => { + const fileId = `f_system_${++seq}`; + + await engine.insert('sys_file', { id: fileId, name: 'system.png', ...NAMED } as any, { context: SYS }); + expect(await references(fileId)).toEqual({ ...NAMED }); + + await engine.update('sys_file', { id: fileId, ...UNREFERENCED, ref_object: 'media', ref_id: 'm_system' } as any, { context: SYS }); + expect(await references(fileId)).toEqual({ ...UNREFERENCED, ref_object: 'media', ref_id: 'm_system' }); + }); + + // ── The presence of `key` and `status` (#27778) ─────────────────── + + it('every storage writer sets key and status: presign, complete and the field copy', async () => { + const { fileId: presigned } = await presign(); + const pendingRow = await owned(presigned); + expect(pendingRow.status).toBe('pending'); + expect(typeof pendingRow.key === 'string' && pendingRow.key.length > 0, JSON.stringify(pendingRow)).toBe(true); + + const { fileId } = await claimedUpload(); + const committedRow = await owned(fileId); + expect(committedRow.status).toBe('committed'); + expect(typeof committedRow.key === 'string' && committedRow.key.length > 0, JSON.stringify(committedRow)).toBe(true); + + const second = `m_${++seq}`; + await engine.insert('media', { id: second, name: 'copy', poster: fileId } as any, { context: WRITER_CTX }); + const copyRow = await owned((await record(second))?.poster as string); + expect(copyRow.status).toBe('committed'); + expect(typeof copyRow.key === 'string' && copyRow.key.length > 0, JSON.stringify(copyRow)).toBe(true); + expect(copyRow.key).not.toBe(committedRow.key); + }); }); diff --git a/packages/services/service-storage/src/sys-file-storage-owned-columns.ts b/packages/services/service-storage/src/sys-file-storage-owned-columns.ts index afab3e5c6d4..31b212c69be 100644 --- a/packages/services/service-storage/src/sys-file-storage-owned-columns.ts +++ b/packages/services/service-storage/src/sys-file-storage-owned-columns.ts @@ -1,10 +1,11 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. -import { SYS_FILE_STORAGE_OWNED_COLUMNS } from './objects/system-file.object.js'; +import { SYS_FILE_REFERENCE_COLUMNS, SYS_FILE_STORAGE_OWNED_COLUMNS } from './objects/system-file.object.js'; /** * The create-side half of "the storage service owns `sys_file`'s lifecycle - * columns" ({@link SYS_FILE_STORAGE_OWNED_COLUMNS}). + * columns": the storage-owned ones ({@link SYS_FILE_STORAGE_OWNED_COLUMNS}) + * and the file-reference ones ({@link SYS_FILE_REFERENCE_COLUMNS}). * * The update side needs no code here: the columns are `readonly`, and the * engine's static readonly strip takes a non-system caller's value off an @@ -24,6 +25,11 @@ import { SYS_FILE_STORAGE_OWNED_COLUMNS } from './objects/system-file.object.js' * hook does not judge: the upload doors (through `StorageMetadataStore`), the * field-file copy and backfill, and the attachment tombstone. * + * The file-reference columns are owned the same way, by the file-reference + * lifecycle, which alone records which record and field hold a file, + * through its claim, release and copy, all under the system context. A + * non-system create starts with none of them, as a fresh upload does. + * * Judged as the update strip judges: every write whose context is not * `isSystem`, a context-less one included. */ @@ -44,13 +50,16 @@ const PACKAGE_ID = 'com.objectstack.service.storage'; /** The state a non-system create starts in: an upload the service has not committed. */ const NON_SYSTEM_CREATE_STATUS = 'pending'; +/** Every column a non-system create may not set: the storage-owned ones, then the file-reference ones. */ +const LIFECYCLE_OWNED_COLUMNS: readonly string[] = [...SYS_FILE_STORAGE_OWNED_COLUMNS, ...SYS_FILE_REFERENCE_COLUMNS]; + /** - * Drop the storage-owned columns a non-system create supplied, and start the - * row `pending`. Returns the names it dropped. Exported for the guard's pin. + * Drop the lifecycle-owned columns a non-system create supplied, and start the + * row `pending`. Returns the names it dropped. */ -export function dropStorageOwnedColumns(data: Record): string[] { +function dropLifecycleOwnedColumns(data: Record): string[] { const dropped: string[] = []; - for (const column of SYS_FILE_STORAGE_OWNED_COLUMNS) { + for (const column of LIFECYCLE_OWNED_COLUMNS) { if (!Object.prototype.hasOwnProperty.call(data, column)) continue; delete data[column]; dropped.push(column); @@ -70,11 +79,12 @@ export function installSysFileStorageOwnedColumnsGuard( if (ctx?.session?.isSystem) return; const data = ctx?.input?.data; if (!data || typeof data !== 'object' || Array.isArray(data)) return; - const dropped = dropStorageOwnedColumns(data as Record); + const dropped = dropLifecycleOwnedColumns(data as Record); if (dropped.length === 0) return; logger.warn( `[storage] sys_file: dropped ${dropped.join(', ')} from a create that did not come from the storage service. ` + - 'These columns record what the storage backend holds, and only the storage service writes them; ' + + 'These columns record what the storage backend holds and which record owns the file, ' + + 'and only the storage service writes them; ' + `the row was created ${NON_SYSTEM_CREATE_STATUS}. Upload files through the storage upload doors instead.`, { object: 'sys_file', operation: 'insert', fields: dropped, userId: ctx?.session?.userId ?? 'unknown' }, );