diff --git a/.changeset/27399-sys-file-storage-owned-columns.md b/.changeset/27399-sys-file-storage-owned-columns.md new file mode 100644 index 00000000000..624693b50f2 --- /dev/null +++ b/.changeset/27399-sys-file-storage-owned-columns.md @@ -0,0 +1,18 @@ +--- +'@objectstack/service-storage': patch +--- + +fix(service-storage): `sys_file`'s storage-owned columns (`key`, `mime_type`, `size`, `status`, `etag`) can no longer be set or changed by a write that does not come from the storage service + +Clause-②: no (narrowing) +Metadata-protocol: none + + + +A file field refuses a file whose upload never completed by reading `sys_file.status`, and judges its `accept` / `maxSize` by `mime_type` / `size`. Those columns were plain writable fields, so a caller allowed to write `sys_file` rows (an administrator, on a stock boot) could `PATCH /api/v1/data/sys_file/ID` a presigned upload that never received its bytes to `committed`, and a record's file field then claimed it; or rewrite `size` / `mime_type` after commit so the field's constraints judged values that do not describe the stored bytes. + +- **Update.** The five columns are now `readonly` on `sys_file`. A non-system update, by id or by predicate, keeps the values the storage service 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.** A non-system create of a `sys_file` row keeps the caller's other columns, drops any value it gives these five, and starts the row `pending`, with one `warn` naming the dropped columns. A file field refuses such a file, and the lifecycle's `pending` retention collects it. The create is not refused. +- **Unchanged.** The storage service's own writes all run under the system context and still move every column: presign, PUT and `/upload/complete`, the chunked upload, a file field's claim, release and copy, and the attachment tombstone. Every other `sys_file` column (`name`, `acl`, `scope`, ownership) is written through the data door as before. + +To change what a file is, upload it again through the storage upload doors (`POST /api/v1/storage/upload/presigned`, then `/upload/complete`). diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 1afe01b4b54..b95a7ccacc6 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -184,6 +184,7 @@ One of the largest consumers of the flag; | 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 | `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 | `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 | `referencingCallerOf` | +| 47g | The `sys_file` storage-owned columns guard returns early on create: a system insert keeps the `key`, `mime_type`, `size`, `status` and `etag` it writes | service-storage | Get: the storage service's own inserts (the upload doors, the field-file copy and backfill) write those five columns. Lose: nothing a user held — a non-system create has them dropped and starts `pending`, and on update the same columns are `readonly`, so the UPDATE `readonly` strip keeps them for every non-system caller | `installSysFileStorageOwnedColumnsGuard` | ### 5. Actions, metadata plane, provenance, the organization wall @@ -659,6 +660,7 @@ and CI refuses one that differs from what the tree produces. | `packages/services/service-storage/src/attachment-access-hooks.ts#installAttachmentReadVisibility` | 1 | 44 | | `packages/services/service-storage/src/file-attachment-service.ts#requireUserContext` | 1 | 47e | | `packages/services/service-storage/src/file-reference-lifecycle.ts#referencingCallerOf` | 1 | 47f | +| `packages/services/service-storage/src/sys-file-storage-owned-columns.ts#installSysFileStorageOwnedColumnsGuard` | 1 | 47g | {/* END GENERATED: system-context-census */} diff --git a/packages/qa/dogfood/test/sys-file-metadata-write-refusal.dogfood.test.ts b/packages/qa/dogfood/test/sys-file-metadata-write-refusal.dogfood.test.ts index 106e82778e9..6c046070d49 100644 --- a/packages/qa/dogfood/test/sys-file-metadata-write-refusal.dogfood.test.ts +++ b/packages/qa/dogfood/test/sys-file-metadata-write-refusal.dogfood.test.ts @@ -1,7 +1,8 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. // // [#11076] REFUSAL PIN — a non-admin cannot write `sys_file.mime_type` / `size` -// through the data API; a platform admin still can. +// through the data API; a platform admin's write reaches the row, and (since +// #27399) the two columns still keep what the storage service wrote. // // ── What this file is, and what it deliberately is NOT ────────────────────── // @@ -18,12 +19,13 @@ // explicit-allow platform baseline `member_default` (#5491) NAMES no `sys_file` // grant, and object access comes from OWDs plus profile / permission-set // declarations only — so the refusal is produced by the baseline's SILENCE -// about this object. `sys_file` reinforces that from the schema side: it -// declares no ADR-0103 `managedBy` bucket at all (seven sibling platform -// objects in `platform-objects/src/audit` do — `sys_attachment` names -// `platform` outright, the rest name `engine-owned` / `append-only` / `config`), -// and neither `mime_type` nor `size` carries `readonly`, though `id` on the same -// object does. So nothing on the object itself is holding this line. +// about this object. `sys_file` declares no ADR-0103 `managedBy` bucket at all +// (seven sibling platform objects in `platform-objects/src/audit` do — +// `sys_attachment` names `platform` outright, the rest name `engine-owned` / +// `append-only` / `config`). [#27399] Since then `mime_type` and `size` (with +// `key`, `status` and `etag`) are `readonly` storage-owned columns, so a +// non-system UPDATE keeps the stored values whoever sends it; the 403 below is +// still the RBAC answer, which comes before any column is judged. // // An absence has no gate. A baseline change that hands members a `sys_file` // grant — the shape a "while we are here" widening naturally takes — would @@ -31,12 +33,10 @@ // maxSize re-check reads these columns) with every suite still green. This file // is what reddens instead. // -// ⛔ It does NOT decide whether `sys_file` SHOULD be engine-owned, or whether -// these columns should become readonly / system-managed. That is an un-ruled -// platform posture call — it removes an affordance from app authors — and this -// pin is written to survive every outcome of it: if the posture tightens the -// refusal stays a refusal, and if it never does, this file is what tells you -// when the absence stops holding. +// ⛔ It does NOT decide whether `sys_file` SHOULD be engine-owned. The columns' +// own posture was decided on #27399 (triage: read-only to the data door), and +// this pin survived it as it was written to: the refusal stays a refusal, and +// only the contrast control's read-back moved. // // ── Three assertions, and why each one is load-bearing ────────────────────── // @@ -45,7 +45,9 @@ // server rejects for everyone. // // The PLATFORM-ADMIN CONTRAST CONTROL, on the SAME row and the SAME two -// columns, is part of the pin and not optional. Without it the refusal cases +// columns, is part of the pin and not optional: the admin's write answers 200 +// and LANDS (a writable column in the same payload changes), so the request +// reaches the row. Without it the refusal cases // pass just as well when the fixture never reaches the endpoint at all — // which is how a refusal pin rots into a tautology. This lane has the scar: // a `get-session` pin that answered `200` for anonymous (#10954). @@ -232,16 +234,21 @@ describe('[#11076] `sys_file.mime_type` / `size` are not writable by a non-admin // row and the SAME two columns they did. Without it, all four cases above pass // unchanged if the fixture stops reaching the endpoint — a request that never // arrives is refused by nothing and looks exactly like a gate holding. - it('a platform admin still succeeds on the same row and the same columns', async () => { + it('a platform admin\'s write on the same row and the same columns reaches it, and the storage-owned columns keep their values', async () => { const ADMIN_WRITE = { mime_type: 'text/x-admin-11076', size: 777 } as const; - expect(ADMIN_WRITE, 'the control must actually move the columns').not.toEqual({ ...SEEDED }); + expect(ADMIN_WRITE, 'the control must actually try to move the columns').not.toEqual({ ...SEEDED }); + const ADMIN_NAME = 'pin-admin-11076.bin'; - const answer = await write(adminTok, { ...ADMIN_WRITE }); + const answer = await write(adminTok, { ...ADMIN_WRITE, name: ADMIN_NAME }); expect(answer.status, `admin status — body: ${answer.body}`).toBe(200); - // Persisted, read back the same way the refusals were: the control proves a - // write of these columns REACHES and LANDS, so the 403s above are a verdict - // about the caller and not about the request. - expect(await stored()).toEqual({ ...ADMIN_WRITE }); + // Read back the same way the refusals were. The writable column in the + // same payload LANDED, so the request reached the row and the 403s above + // are a verdict about the caller, not about the request. [#27399] The two + // storage-owned columns keep what the storage service wrote: the engine's + // readonly strip takes a non-system caller's value off, an admin's too. + const row = await ql.findOne(OBJECT, { where: { id: ROW_ID }, context: SYS }); + expect(row?.name, 'the admin write reached and landed').toBe(ADMIN_NAME); + expect(await stored()).toEqual({ ...SEEDED }); }); }); 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 15924561baa..846947846ee 100644 --- a/packages/services/service-storage/src/objects/system-file.object.ts +++ b/packages/services/service-storage/src/objects/system-file.object.ts @@ -24,6 +24,27 @@ const UPLOAD_SCOPE_LABELS: Record = { attachments: 'Attachments', }; +/** + * The `sys_file` columns the storage service owns: where the bytes live + * (`key`), what the backend was told they are (`mime_type`, `size`, `etag`) and + * how far the upload got (`status`). A file field refuses a file whose upload + * never completed by reading `status`, and judges `accept` / `maxSize` by + * reading `mime_type` / `size`, so a caller who could write them could make a + * file with no bytes read as uploaded, or one the field forbids read as + * allowed. + * + * Each is declared `readonly` below, so on an UPDATE the engine's static + * readonly strip takes a non-system caller's value back out (the PATCH answers + * as before and the column keeps what the service wrote). On an INSERT the + * engine leaves a `sys_` object's columns to the object's own guard, and that + * guard reads this list: `installSysFileStorageOwnedColumnsGuard` + * (`sys-file-storage-owned-columns.ts`). Only the service's own writes, all + * 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. + */ +export const SYS_FILE_STORAGE_OWNED_COLUMNS = ['key', 'mime_type', 'size', 'status', 'etag'] as const; + /** * System File Object * @@ -57,10 +78,12 @@ export const SystemFile = ObjectSchema.create({ readonly: true, }), + // Storage-owned (SYS_FILE_STORAGE_OWNED_COLUMNS). key: Field.text({ label: 'Storage Key', required: true, searchable: true, + readonly: true, }), name: Field.text({ @@ -69,12 +92,16 @@ export const SystemFile = ObjectSchema.create({ searchable: true, }), + // Storage-owned (SYS_FILE_STORAGE_OWNED_COLUMNS). mime_type: Field.text({ label: 'MIME Type', + readonly: true, }), + // Storage-owned (SYS_FILE_STORAGE_OWNED_COLUMNS). size: Field.number({ label: 'Size (bytes)', + readonly: true, }), // A logical key prefix and a lifecycle discriminator — never an access @@ -104,9 +131,11 @@ export const SystemFile = ObjectSchema.create({ ], }), + // Storage-owned (SYS_FILE_STORAGE_OWNED_COLUMNS). status: Field.select({ label: 'Status', required: true, + readonly: true, options: [ { label: 'Pending Upload', value: 'pending' }, { label: 'Committed', value: 'committed' }, @@ -114,8 +143,10 @@ export const SystemFile = ObjectSchema.create({ ], }), + // Storage-owned (SYS_FILE_STORAGE_OWNED_COLUMNS). etag: Field.text({ label: 'ETag', + readonly: true, }), owner_id: Field.text({ diff --git a/packages/services/service-storage/src/storage-service-plugin.test.ts b/packages/services/service-storage/src/storage-service-plugin.test.ts index 635d387796d..e47ee5ad47d 100644 --- a/packages/services/service-storage/src/storage-service-plugin.test.ts +++ b/packages/services/service-storage/src/storage-service-plugin.test.ts @@ -432,16 +432,22 @@ describe('StorageServicePlugin: sys_file orphan lifecycle wiring (#2755)', () => const hookEvents: string[] = []; const globalHookEvents: string[] = []; + const sysFileHookEvents: string[] = []; const middlewares: Array<{ object?: string }> = []; ctx.registerService('objectql', { registerHook: (event: string, _fn: unknown, opts: any) => { // File-reference ownership (ADR-0104 D3 wave 2) registers GLOBALLY — - // any object may declare a file-class field. Everything else is - // scoped to sys_attachment. + // any object may declare a file-class field. The create-side guard of + // sys_file's storage-owned columns is scoped to sys_file. Everything + // else is scoped to sys_attachment. if (opts?.object === undefined) { globalHookEvents.push(event); return; } + if (opts?.object === 'sys_file') { + sysFileHookEvents.push(event); + return; + } expect(opts?.object).toBe('sys_attachment'); hookEvents.push(event); }, @@ -488,6 +494,9 @@ describe('StorageServicePlugin: sys_file orphan lifecycle wiring (#2755)', () => 'beforeUpdate', 'beforeUpdate', ]); + // The storage-owned columns' create-side guard + // (sys-file-storage-owned-columns.ts): one `beforeInsert`, nothing else. + expect(sysFileHookEvents).toEqual(['beforeInsert']); // Field-reference ownership: claim/copy on write, release on delete // (file-reference-lifecycle.ts). Registered without an object filter. expect(globalHookEvents.sort()).toEqual([ diff --git a/packages/services/service-storage/src/storage-service-plugin.ts b/packages/services/service-storage/src/storage-service-plugin.ts index 00a7d0699d2..38bfdf413d1 100644 --- a/packages/services/service-storage/src/storage-service-plugin.ts +++ b/packages/services/service-storage/src/storage-service-plugin.ts @@ -63,6 +63,7 @@ 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, type FileReferenceEngine } from './file-reference-lifecycle.js'; +import { installSysFileStorageOwnedColumnsGuard } from './sys-file-storage-owned-columns.js'; import type { FileReferenceReadAuthorizer } from './referenceable-file.js'; import { installAttachmentAccessHooks, @@ -488,6 +489,10 @@ export class StorageServicePlugin implements Plugin { // lifecycle sweep. Both degrade silently on bare kernels. if (engine && typeof (engine as any).registerHook === 'function') { installAttachmentLifecycleHooks(engine as any, ctx.logger); + // The create-side guard of sys_file's storage-owned columns: the engine + // strips them off a non-system UPDATE (they are `readonly`) but leaves a + // `sys_` INSERT to the object's own guard. + installSysFileStorageOwnedColumnsGuard(engine as any, ctx.logger); // [#25692] The download door's verdict on one file for one caller, // bound once from the live registry and handed to BOTH write doors // that name a `sys_file` id — a file-class field and a 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 new file mode 100644 index 00000000000..b62cbf26700 --- /dev/null +++ b/packages/services/service-storage/src/sys-file-storage-owned-columns.integration.test.ts @@ -0,0 +1,306 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `sys_file`'s storage-owned columns (`key`, `mime_type`, `size`, `status`, + * `etag`) cannot be set or changed by a write that does not come from the + * storage service. + * + * Two checks trust them: a file field refuses a file whose upload never + * completed by reading `status`, and judges `accept` / `maxSize` by + * `mime_type` / `size`. They were plain writable columns, so a data-door + * `PATCH` of a presigned upload that never received its bytes could set it + * `committed` (and its `size` to anything), and a record's file field then + * claimed it. + * + * On an UPDATE the engine's static readonly strip now takes the caller's value + * back out, by id and by predicate, and the rest of the write lands. On a + * CREATE the storage service's own guard drops them and starts the row + * `pending`. Neither refuses the write. The service's own writes, all under + * the system context, still move every column: presign, complete, claim. + * + * 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 + * made as a signed-in caller with no system context. + */ + +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 type { 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 { SystemFile, SYS_FILE_STORAGE_OWNED_COLUMNS } from './objects/system-file.object.js'; +import { installDefaultOrganization } from './default-organization.testkit.js'; + +const BASE = '/api/v1/storage'; +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 DECLARED = 100; + +const silentLogger = () => ({ + info: vi.fn(), warn: vi.fn(), debug: vi.fn(), error: vi.fn(), + trace: vi.fn(), fatal: vi.fn(), child() { return this; }, +}); + +type Res = { _status: number; _json: any }; + +describe('sys_file storage-owned columns are not writable outside the storage service', () => { + let rootDir: string; + let engine: ObjectQL; + const routes = new Map(); + let seq = 0; + + beforeAll(async () => { + rootDir = join(tmpdir(), `os-sysfile-owned-${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 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) => + 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 services = new Map(); + const ready: Array<() => Promise | void> = []; + const mount = (m: string) => (path: string, handler: RouteHandler) => { routes.set(`${m}:${path}`, handler); }; + const ctx = { + 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') ready.push(fn); }, + }; + ctx.registerService('objectql', dataEngine); + ctx.registerService('http-server', { + get: mount('GET'), post: mount('POST'), put: mount('PUT'), + delete: vi.fn(), patch: vi.fn(), use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + }); + 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; + }, + }, + }); + + const plugin = new StorageServicePlugin({ adapter: 'local', local: { rootDir }, bindToSettings: false }); + await plugin.init(ctx as any); + + // The card's file field: `poster` accepts PNG/JPEG up to 1 KiB. + engine.registerApp({ + id: 'com.objectstack.test.sys-file-storage-owned-columns', + name: 'sys_file storage-owned columns', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [{ + name: 'media', + label: 'Media', + fields: { + name: { name: 'name', type: 'text' }, + poster: { name: 'poster', label: 'Poster', type: 'image', accept: ['image/png', 'image/jpeg'], maxSize: 1024 }, + }, + }], + } as never); + await engine.syncSchemas(); + await installDefaultOrganization(engine); + await plugin.start(ctx as any); + for (const fn of ready) await fn(); + }); + + afterAll(async () => { + try { await engine?.destroy(); } catch { /* already torn down */ } + if (rootDir) await fs.rm(rootDir, { recursive: true, force: true }); + }); + + /** Call a storage route as the writer. */ + const call = async (method: string, path: string, req: Record): Promise => { + 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; }, + }; + const handler = routes.get(`${method}:${path}`); + if (!handler) throw new Error(`no route ${method} ${path}`); + await handler({ params: {}, query: {}, headers: { 'x-test-user': WRITER }, method, path, ...req } as any, res); + return res; + }; + + /** The card's step 1: presign, and send no bytes. */ + const presign = async () => { + const presigned = await call('POST', `${BASE}/upload/presigned`, { + body: { filename: 'ghost.png', mimeType: 'image/png', size: DECLARED }, + }); + expect(presigned._status, JSON.stringify(presigned._json)).toBe(200); + return presigned._json.data as { fileId: string; uploadUrl: string }; + }; + + /** Presign, PUT the declared bytes and complete: the service's own upload → commit path. */ + const committedUpload = async () => { + const { fileId, uploadUrl } = await presign(); + const token = uploadUrl.slice(uploadUrl.lastIndexOf('/') + 1); + const put = await call('PUT', `${BASE}/_local/raw/:token`, { + params: { token }, rawBody: async () => Buffer.alloc(DECLARED, 1), + }); + expect(put._status, JSON.stringify(put._json)).toBe(200); + const done = await call('POST', `${BASE}/upload/complete`, { body: { fileId, eTag: 'etag-from-complete' } }); + expect(done._status, JSON.stringify(done._json)).toBe(200); + return fileId; + }; + + const fileRow = async (id: string) => + ((await engine.find('sys_file', { where: { id }, context: SYS })) as Array>)[0]; + const owned = async (id: string) => { + const row = await fileRow(id); + return Object.fromEntries(SYS_FILE_STORAGE_OWNED_COLUMNS.map((c) => [c, row?.[c] ?? null])); + }; + const record = async (id: string) => + ((await engine.find('media', { where: { id }, context: SYS_RAW })) as Array>)[0]; + const failureOf = (write: Promise) => + write.then(() => null, (e: unknown) => resolveThrownHttpError(e)); + + /** The file field's refusal of a file whose upload never completed (#26479), as the HTTP boundary answers it. */ + const neverCompleted = (fileId: string) => { + const message = `Poster cannot reference file ${fileId}: its upload was never completed. Complete the upload, then save again.`; + return { + status: 400, + code: 'VALIDATION_FAILED', + declaredStatus: 400, + declaredCode: 'VALIDATION_FAILED', + message, + details: { fields: [{ field: 'poster', code: 'invalid_value', message, label: 'Poster' }] }, + }; + }; + + const FORGED = { + key: 'tenant/someone-else.png', + mime_type: 'image/jpeg', + size: 999, + status: 'committed', + etag: 'forged-etag', + } as const; + /** What a caller would write over a COMMITTED file: every value differs from the one the service wrote. */ + const FORGED_OVER_COMMITTED = { ...FORGED, status: 'pending' } as const; + + it('declares exactly the five storage-owned columns, each readonly on sys_file', () => { + expect([...SYS_FILE_STORAGE_OWNED_COLUMNS].sort()).toEqual(['etag', 'key', 'mime_type', 'size', 'status']); + for (const column of SYS_FILE_STORAGE_OWNED_COLUMNS) { + expect((SystemFile.fields as Record)[column]?.readonly, column).toBe(true); + } + }); + + it('the card\'s steps 1–5: a never-uploaded file stays pending after the PATCH, and a file field still refuses it', async () => { + const { fileId } = await presign(); + const before = await owned(fileId); + expect(before.status).toBe('pending'); + + await engine.update('sys_file', { id: fileId, status: 'committed', size: 999 } as any, { context: WRITER_CTX }); + + expect(await owned(fileId)).toEqual(before); + const id = `m_${++seq}`; + expect(await failureOf( + engine.insert('media', { id, name: 'ghost', poster: fileId } as any, { context: WRITER_CTX }), + )).toEqual(neverCompleted(fileId)); + expect(await record(id)).toBeUndefined(); + }); + + it.each(SYS_FILE_STORAGE_OWNED_COLUMNS)('an update by id cannot change %s, and the rest of the write lands', async (column) => { + const fileId = await committedUpload(); + const before = await owned(fileId); + + await engine.update('sys_file', { id: fileId, name: 'renamed.png', [column]: FORGED_OVER_COMMITTED[column] } as any, { context: WRITER_CTX }); + + expect(await owned(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(); + const before = await owned(fileId); + + await engine.update('sys_file', { ...FORGED_OVER_COMMITTED, acl: 'private' } as any, { where: { id: fileId }, context: WRITER_CTX } as any); + + expect(await owned(fileId)).toEqual(before); + expect((await fileRow(fileId))?.acl).toBe('private'); + }); + + it('a create cannot set any of them: the row starts pending with none of the caller\'s values, and a file field refuses it', async () => { + const fileId = `f_forged_${++seq}`; + + await engine.insert('sys_file', { id: fileId, name: 'forged.png', owner_id: WRITER, ...FORGED } as any, { context: WRITER_CTX }); + + expect(await owned(fileId)).toEqual({ key: null, mime_type: null, size: null, status: 'pending', etag: null }); + expect((await fileRow(fileId))?.name).toBe('forged.png'); + const id = `m_${++seq}`; + expect(await failureOf( + engine.insert('media', { id, name: 'forged', poster: fileId } as any, { context: WRITER_CTX }), + )).toEqual(neverCompleted(fileId)); + }); + + it('a batch create cannot set them on any row', async () => { + const a = `f_forged_${++seq}`; + const b = `f_forged_${++seq}`; + + await engine.insert('sys_file', [ + { id: a, name: 'a.png', ...FORGED }, + { id: b, name: 'b.png', status: 'committed' }, + ] as any, { context: WRITER_CTX }); + + const startsPending = { key: null, mime_type: null, size: null, status: 'pending', etag: null }; + expect(await owned(a)).toEqual(startsPending); + expect(await owned(b)).toEqual(startsPending); + }); + + it('the service\'s own upload → commit → claim path still writes every column', async () => { + const fileId = await committedUpload(); + + const row = await owned(fileId); + expect(row).toMatchObject({ mime_type: 'image/png', size: DECLARED, status: 'committed', etag: 'etag-from-complete' }); + expect(typeof row.key).toBe('string'); + expect(row.key).toContain(fileId); + + const id = `m_${++seq}`; + await engine.insert('media', { id, name: 'real', poster: fileId } as any, { context: WRITER_CTX }); + expect((await record(id))?.poster).toBe(fileId); + expect((await fileRow(fileId))?.ref_id).toBe(id); + }); + + it('a system create keeps the values it writes', async () => { + const fileId = `f_system_${++seq}`; + + await engine.insert('sys_file', { id: fileId, name: 'copy.png', ...FORGED } as any, { context: SYS }); + + expect(await owned(fileId)).toEqual({ ...FORGED }); + }); +}); 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 new file mode 100644 index 00000000000..afab3e5c6d4 --- /dev/null +++ b/packages/services/service-storage/src/sys-file-storage-owned-columns.ts @@ -0,0 +1,84 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { 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}). + * + * 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 + * UPDATE of any object, `sys_` included. On an INSERT the engine deliberately + * leaves a `sys_` object's columns to the object's own guard + * (`staticReadonlyInsertSubject` in `@objectstack/objectql`), so without this + * hook a data-door create could still write a row that reads as an uploaded, + * `committed` file with a `key`, `size` and `mime_type` of the caller's + * choosing, which a file field would then claim. + * + * What a non-system create gets is what the update side gives it: the + * caller's values for these columns are dropped, the rest of the row is + * written, and the write is not refused. The row starts `pending` (the state + * of every upload the service has not committed), so a file field refuses it + * and the lifecycle's `pending` retention collects it. Every writer of these + * columns is the storage service itself, under the system context, which this + * hook does not judge: the upload doors (through `StorageMetadataStore`), the + * field-file copy and backfill, and the attachment tombstone. + * + * Judged as the update strip judges: every write whose context is not + * `isSystem`, a context-less one included. + */ + +/** The slice of the data engine the guard installs on. */ +export interface SysFileStorageOwnedColumnsEngine { + registerHook( + event: 'beforeInsert', + handler: (ctx: any) => void | Promise, + options: { object: string; packageId?: string }, + ): void; +} + +type GuardLogger = { warn: (message: string, meta?: Record) => void }; + +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'; + +/** + * 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. + */ +export function dropStorageOwnedColumns(data: Record): string[] { + const dropped: string[] = []; + for (const column of SYS_FILE_STORAGE_OWNED_COLUMNS) { + if (!Object.prototype.hasOwnProperty.call(data, column)) continue; + delete data[column]; + dropped.push(column); + } + data.status = NON_SYSTEM_CREATE_STATUS; + return dropped; +} + +/** Install the create-side guard on `sys_file`. */ +export function installSysFileStorageOwnedColumnsGuard( + engine: SysFileStorageOwnedColumnsEngine, + logger: GuardLogger, +): void { + engine.registerHook( + 'beforeInsert', + (ctx: any) => { + 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); + 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; ' + + `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' }, + ); + }, + { object: 'sys_file', packageId: PACKAGE_ID }, + ); +}