From 5db2a849adadb10f7b8e1718f973512a2c3562fd Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 02:59:07 +0000 Subject: [PATCH 1/3] =?UTF-8?q?feat(storage):=20legacy=20file-value=20back?= =?UTF-8?q?fill=20=E2=80=94=20ADR-0104=20D3=20wave=202=20(PR-6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit backfillFileReferences() converts the pre-reference forms a file/image/avatar/ video/audio field may hold — an inline metadata blob ({url, name, size, …}) or a bare URL string — into the reference form: an opaque sys_file id, owned by the record's field. What it will and will not convert: - A URL naming this platform's own resolver (…/storage/files/:id) already identifies a sys_file; the field is rewritten to the bare id, no bytes move. - A data: URI carries its bytes inline; they are uploaded, a sys_file is registered, and the field is rewritten to its id. - An external URL is REPORTED, NEVER CONVERTED. Re-hosting third-party content is a bandwidth, licensing and privacy decision that is not a migration's to make. ADR-0104 R7 retires these toward an explicit `url` field, which under AI authoring is the point: it stops "managed file" and "external link" being the same declaration. Dry run by default — nothing is written unless apply is set, and the dry-run report has the same shape as the applied one so the plan can be reviewed and diffed against what actually happened. Idempotent — a value already in reference form is recorded as already_id and left alone, so a partially completed run is safe to repeat. The backfill never writes the ref_* columns itself: it rewrites the record, and the claim hooks observe that write and record ownership. One claiming path, so there is nothing that can disagree with itself — asserted by a test that fails if the backfill ever starts touching sys_file directly. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SHpGw3GBA9aFpfwVArRWfd --- .changeset/adr-0104-d3w2-pr6-backfill.md | 34 ++ .../src/backfill-file-references.test.ts | 251 +++++++++++ .../src/backfill-file-references.ts | 413 ++++++++++++++++++ .../services/service-storage/src/index.ts | 9 + 4 files changed, 707 insertions(+) create mode 100644 .changeset/adr-0104-d3w2-pr6-backfill.md create mode 100644 packages/services/service-storage/src/backfill-file-references.test.ts create mode 100644 packages/services/service-storage/src/backfill-file-references.ts diff --git a/.changeset/adr-0104-d3w2-pr6-backfill.md b/.changeset/adr-0104-d3w2-pr6-backfill.md new file mode 100644 index 0000000000..a926d700c3 --- /dev/null +++ b/.changeset/adr-0104-d3w2-pr6-backfill.md @@ -0,0 +1,34 @@ +--- +"@objectstack/service-storage": minor +--- + +feat(storage): legacy file-value backfill — ADR-0104 D3 wave 2 (PR-6) + +`backfillFileReferences()` converts the pre-reference forms a `file`/`image`/ +`avatar`/`video`/`audio` field may hold — an inline metadata blob +(`{url, name, size, …}`) or a bare URL string — into the reference form: an +opaque `sys_file` id, owned by the record's field. + +What it will and will not convert: + +- **A URL naming this platform's own resolver** (`…/storage/files/:id`) already + identifies a `sys_file`; the field is rewritten to the bare id and no bytes + move. +- **A `data:` URI** carries its bytes inline; they are uploaded, a `sys_file` is + registered, and the field is rewritten to its id. +- **An external URL** is reported, never converted. Re-hosting third-party + content is a bandwidth, licensing and privacy decision that is not a + migration's to make — ADR-0104 R7 retires these toward an explicit `url` + field, which under AI authoring is the point: it stops "managed file" and + "external link" being the same declaration. + +**Dry run by default** — nothing is written unless `apply` is set, and the +dry-run report has the same shape as the applied one so the plan can be reviewed +and diffed. **Idempotent** — a value already in reference form is recorded and +left alone, so a partially-completed run is safe to repeat. + +The backfill never writes the ownership columns itself: it rewrites the record, +and the claim hooks observe that write and record ownership. One claiming path, +so there is nothing that can disagree with itself. Run +`verifyFileReferences()` afterwards to confirm the two agree — that +reconciliation is the gate the irreversible collection change must pass. diff --git a/packages/services/service-storage/src/backfill-file-references.test.ts b/packages/services/service-storage/src/backfill-file-references.test.ts new file mode 100644 index 0000000000..55b312cbd1 --- /dev/null +++ b/packages/services/service-storage/src/backfill-file-references.test.ts @@ -0,0 +1,251 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, vi } from 'vitest'; +import { + backfillFileReferences, + formatBackfillReport, + type BackfillEngine, +} from './backfill-file-references.js'; + +const silentLogger = () => ({ info: vi.fn(), warn: vi.fn(), debug: vi.fn() }); + +const REGISTRY: Record = { + sys_file: { fields: { id: { type: 'text' } } }, + product: { + fields: { + id: { type: 'text' }, + name: { type: 'text' }, + image: { type: 'image' }, + gallery: { type: 'image', multiple: true }, + }, + }, + tag: { fields: { id: { type: 'text' }, label: { type: 'text' } } }, +}; + +function fakeEngine(tables: Record>>) { + const calls: Array<{ op: string; object: string; arg: any }> = []; + const engine: BackfillEngine & { tables: typeof tables; calls: typeof calls } = { + getObject: (name) => REGISTRY[name], + getConfigs: () => REGISTRY, + async find(object, options: any) { + const rows = tables[object] ?? []; + const start = typeof options?.offset === 'number' ? options.offset : 0; + const end = typeof options?.limit === 'number' ? start + options.limit : undefined; + return rows.slice(start, end); + }, + async insert(object, data: any) { + calls.push({ op: 'insert', object, arg: data }); + (tables[object] ??= []).push({ ...data }); + return data; + }, + async update(object, data: any) { + calls.push({ op: 'update', object, arg: data }); + const row = (tables[object] ?? []).find((r) => String(r.id) === String(data.id)); + if (row) Object.assign(row, data); + return row; + }, + tables, + calls, + }; + return engine; +} + +const fakeStorage = () => + ({ + upload: vi.fn(async () => {}), + download: vi.fn(async () => Buffer.from('x')), + delete: vi.fn(async () => {}), + exists: vi.fn(async () => true), + getInfo: vi.fn(async () => ({ key: 'k', size: 1, contentType: 'text/plain', lastModified: new Date() })), + }) as any; + +const run = (engine: any, opts: any = {}, storage: any = fakeStorage()) => + backfillFileReferences(engine, () => storage, silentLogger(), opts); + +describe('backfillFileReferences (ADR-0104 D3 wave 2)', () => { + it('rewrites a URL that already names a sys_file to the bare id, moving no bytes', async () => { + const engine = fakeEngine({ + product: [{ id: 'p1', image: 'https://app.example.com/api/v1/storage/files/file_a' }], + sys_file: [], + }); + const storage = fakeStorage(); + + const report = await run(engine, { apply: true }, storage); + + expect(engine.tables.product[0].image).toBe('file_a'); + expect(report.converted).toBe(1); + expect(storage.upload).not.toHaveBeenCalled(); + expect(report.actions[0].kind).toBe('resolved_local_url'); + }); + + it('resolves the same URL when it is wrapped in a legacy inline blob', async () => { + const engine = fakeEngine({ + product: [ + { id: 'p1', image: { url: '/api/v1/storage/files/file_b', name: 'b.png', size: 12 } }, + ], + sys_file: [], + }); + + await run(engine, { apply: true }); + + expect(engine.tables.product[0].image).toBe('file_b'); + }); + + it('uploads inline data: bytes and rewrites the field to the new id', async () => { + const engine = fakeEngine({ + product: [{ id: 'p1', image: { url: 'data:image/png;base64,aGVsbG8=', name: 'hi.png' } }], + sys_file: [], + }); + const storage = fakeStorage(); + + const report = await run(engine, { apply: true }, storage); + + expect(storage.upload).toHaveBeenCalledTimes(1); + const created = engine.tables.sys_file[0]; + expect(created).toMatchObject({ name: 'hi.png', mime_type: 'image/png', status: 'committed' }); + expect(created.size).toBe(5); // "hello" + expect(engine.tables.product[0].image).toBe(created.id); + expect(report.converted).toBe(1); + }); + + /** + * Converting an external URL would mean fetching third-party content and + * silently re-hosting it — a bandwidth/licensing/privacy decision that is not + * a migration's to make. + */ + it('reports an external URL and never re-hosts it', async () => { + const engine = fakeEngine({ + product: [{ id: 'p1', image: 'https://cdn.example.com/logo.png' }], + sys_file: [], + }); + const storage = fakeStorage(); + + const report = await run(engine, { apply: true }, storage); + + expect(engine.tables.product[0].image).toBe('https://cdn.example.com/logo.png'); + expect(storage.upload).not.toHaveBeenCalled(); + expect(report.externalUrls).toBe(1); + expect(report.converted).toBe(0); + expect(formatBackfillReport(report)).toContain('NOT re-hosted'); + }); + + it('handles a multiple:true field element-wise', async () => { + const engine = fakeEngine({ + product: [ + { + id: 'p1', + gallery: [ + '/api/v1/storage/files/file_a', + 'https://cdn.example.com/x.png', + 'file_already', + ], + }, + ], + sys_file: [], + }); + + const report = await run(engine, { apply: true }); + + expect(engine.tables.product[0].gallery).toEqual([ + 'file_a', + 'https://cdn.example.com/x.png', + 'file_already', + ]); + expect(report.converted).toBe(1); + expect(report.externalUrls).toBe(1); + expect(report.alreadyReferences).toBe(1); + }); + + // ── Dry run ─────────────────────────────────────────────────────── + it('writes nothing on a dry run but reports the same plan', async () => { + const engine = fakeEngine({ + product: [ + { id: 'p1', image: '/api/v1/storage/files/file_a' }, + { id: 'p2', image: 'data:text/plain;base64,aGk=' }, + ], + sys_file: [], + }); + const storage = fakeStorage(); + + const report = await run(engine, {}, storage); + + expect(report.applied).toBe(false); + expect(report.converted).toBe(2); + expect(engine.calls).toHaveLength(0); + expect(engine.tables.product[0].image).toBe('/api/v1/storage/files/file_a'); + expect(storage.upload).not.toHaveBeenCalled(); + expect(formatBackfillReport(report)).toContain('DRY RUN'); + }); + + // ── Idempotence / safety ────────────────────────────────────────── + it('is idempotent — a second run converts nothing', async () => { + const engine = fakeEngine({ + product: [{ id: 'p1', image: '/api/v1/storage/files/file_a' }], + sys_file: [], + }); + + await run(engine, { apply: true }); + const second = await run(engine, { apply: true }); + + expect(second.converted).toBe(0); + expect(second.alreadyReferences).toBe(1); + }); + + it('never writes the ref_* columns itself — claiming stays on one path', async () => { + const engine = fakeEngine({ + product: [{ id: 'p1', image: '/api/v1/storage/files/file_a' }], + sys_file: [], + }); + + await run(engine, { apply: true }); + + // The record write is what the claim hooks observe; the backfill must not + // record ownership behind their back, or there would be two claiming paths + // that could disagree. + for (const c of engine.calls) { + expect(c.arg).not.toHaveProperty('ref_object'); + expect(c.arg).not.toHaveProperty('ref_id'); + expect(c.arg).not.toHaveProperty('ref_field'); + } + expect(engine.calls.filter((c) => c.object === 'sys_file')).toHaveLength(0); + }); + + it('leaves data: values alone when no storage service is available', async () => { + const engine = fakeEngine({ + product: [{ id: 'p1', image: 'data:text/plain;base64,aGk=' }], + sys_file: [], + }); + + const report = await backfillFileReferences(engine, () => null, silentLogger(), { apply: true }); + + expect(report.unresolvable).toBe(1); + expect(report.converted).toBe(0); + expect(engine.tables.product[0].image).toBe('data:text/plain;base64,aGk='); + }); + + it('skips objects with no file-class fields', async () => { + const engine = fakeEngine({ tag: [{ id: 't1', label: 'x' }], product: [], sys_file: [] }); + + const report = await run(engine, { apply: true }); + + expect(report.scannedObjects).toEqual(['product']); + }); + + it('pages through large objects and flags truncation at the bound', async () => { + const many = Array.from({ length: 500 }, (_, i) => ({ + id: `p${i}`, + image: `/api/v1/storage/files/file_${i}`, + })); + const engine = fakeEngine({ product: many, sys_file: [] }); + + const full = await run(engine, {}); + expect(full.scannedRecords).toBe(500); + expect(full.truncated).toBe(false); + + const bounded = await run(fakeEngine({ product: many, sys_file: [] }), { + maxRecordsPerObject: 200, + }); + expect(bounded.truncated).toBe(true); + expect(formatBackfillReport(bounded)).toContain('truncated'); + }); +}); diff --git a/packages/services/service-storage/src/backfill-file-references.ts b/packages/services/service-storage/src/backfill-file-references.ts new file mode 100644 index 0000000000..9885f3bfe0 --- /dev/null +++ b/packages/services/service-storage/src/backfill-file-references.ts @@ -0,0 +1,413 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { randomUUID } from 'node:crypto'; +import { FILE_REFERENCE_TYPES, isFileIdToken } from '@objectstack/spec/data'; +import type { IStorageService } from '@objectstack/spec/contracts'; + +/** + * Legacy file-value backfill (ADR-0104 D3 wave 2). + * + * Converts the pre-reference forms a `file`/`image`/`avatar`/`video`/`audio` + * field may hold — an inline metadata blob (`{url, name, size, …}`) or a bare + * URL string — into the reference form: an opaque `sys_file` id, owned by the + * record's field. Ownership is what a later, gated change consults before + * deleting bytes, so a tenant must be backfilled (and then reconciled with + * `verifyFileReferences`) before collection may be enabled. + * + * ## What it will and will not convert + * + * - **A URL pointing at this platform's own resolver** (`…/storage/files/:id`) + * already names a `sys_file`; the field is rewritten to that bare id. No + * bytes move. + * - **A `data:` URI** carries its bytes inline; they are uploaded, a `sys_file` + * is created, and the field is rewritten to its id. + * - **An external URL** (`https://cdn.example.com/…`) is **reported, never + * converted.** Converting would mean fetching third-party content and + * silently re-hosting it — a bandwidth, licensing and privacy decision that + * is not a migration's to make. ADR-0104 R7 retires these toward an explicit + * `url` field instead, which under AI authoring is the point: it stops + * "managed file" and "external link" being the same declaration. + * + * ## Dry run by default + * + * Nothing is written unless `apply` is set. The dry-run report is the same + * shape as the applied one, so the plan can be reviewed — and diffed against + * what actually happened — before any tenant data changes. + * + * Idempotent: a value already in reference form is recorded as `already_id` + * and left alone, so a partially-completed run is safe to repeat. + */ + +const SYSTEM_CTX = { isSystem: true } as const; +const SCAN_PAGE_SIZE = 200; + +/** `…/storage/files/` (optionally with a trailing `/url` or query). */ +const LOCAL_FILE_URL_RE = /\/storage\/files\/([A-Za-z0-9_-]{1,64})(?:\/url)?(?:[?#].*)?$/; +/** `data:;base64,` — the only inline form carrying real bytes. */ +const DATA_URI_RE = /^data:([^;,]*)(;base64)?,(.*)$/s; + +export type BackfillActionKind = + | 'already_id' + | 'resolved_local_url' + | 'uploaded_inline_data' + | 'external_url_skipped' + | 'unresolvable'; + +export interface BackfillAction { + kind: BackfillActionKind; + object: string; + recordId: string; + field: string; + /** Short description of the value found (URLs truncated). */ + before: string; + /** The `sys_file` id the field now names, when converted. */ + after?: string; + detail: string; +} + +export interface BackfillReport { + scannedObjects: string[]; + scannedRecords: number; + /** Values rewritten into reference form (or that would be, on a dry run). */ + converted: number; + /** External URLs left alone — these need an explicit modelling decision. */ + externalUrls: number; + /** Values that could not be interpreted at all. */ + unresolvable: number; + /** Values already in reference form. */ + alreadyReferences: number; + actions: BackfillAction[]; + /** False on a dry run — no writes were made. */ + applied: boolean; + truncated: boolean; +} + +export interface BackfillEngine { + getObject(name: string): any | undefined; + getConfigs?(): Record; + find(object: string, options: Record): Promise>>; + insert(object: string, data: Record, options?: Record): Promise; + update(object: string, data: Record, options: Record): Promise; +} + +export interface BackfillOptions { + /** Restrict to these objects (default: every object with a file field). */ + objects?: string[]; + /** Write the conversions. Omit for a dry run (the default). */ + apply?: boolean; + maxRecordsPerObject?: number; +} + +export interface BackfillLogger { + info(msg: string, meta?: unknown): void; + warn(msg: string, meta?: unknown): void; + debug?(msg: string, meta?: unknown): void; +} + +function fileFieldsOf(engine: BackfillEngine, objectName: string): string[] { + const schema: any = engine.getObject(objectName); + if (!schema?.fields) return []; + return Object.entries(schema.fields) + .filter(([, def]: [string, any]) => def && FILE_REFERENCE_TYPES.has(def.type)) + .map(([name]) => name); +} + +function truncate(v: string, n = 96): string { + return v.length <= n ? v : `${v.slice(0, n)}…`; +} + +/** The URL a legacy value points at, if any (inline blob or bare string). */ +function urlOf(value: unknown): string | null { + if (typeof value === 'string') return value; + if (value && typeof value === 'object') { + const u = (value as any).url; + if (typeof u === 'string' && u) return u; + } + return null; +} + +/** Upload a `data:` URI's bytes and register the `sys_file` row. */ +async function materializeDataUri( + engine: BackfillEngine, + storage: IStorageService, + dataUri: string, + legacy: unknown, +): Promise { + const m = DATA_URI_RE.exec(dataUri); + if (!m) throw new Error('not a data: URI'); + const mime = m[1] || 'application/octet-stream'; + const isBase64 = !!m[2]; + const payload = m[3] ?? ''; + const bytes = isBase64 + ? Buffer.from(payload, 'base64') + : Buffer.from(decodeURIComponent(payload), 'utf-8'); + + const legacyName = + legacy && typeof legacy === 'object' && typeof (legacy as any).name === 'string' + ? (legacy as any).name + : ''; + const newId = randomUUID(); + const name = legacyName || `${newId}`; + const ext = name.includes('.') ? '.' + name.split('.').pop() : ''; + const key = `user/${newId}${ext}`; + + await storage.upload(key, bytes, { contentType: mime } as any); + const now = new Date().toISOString(); + await engine.insert( + 'sys_file', + { + id: newId, + key, + name, + mime_type: mime, + size: bytes.length, + scope: 'user', + status: 'committed', + created_at: now, + updated_at: now, + }, + { context: { ...SYSTEM_CTX } }, + ); + return newId; +} + +/** + * Scan legacy file values and convert what can be converted. + * + * @param getStorage resolves the storage service; when absent, `data:` values + * cannot be materialised and are reported `unresolvable` rather than failing + * the run — a URL-only tenant still backfills fully. + */ +export async function backfillFileReferences( + engine: BackfillEngine, + getStorage: () => IStorageService | null | undefined, + logger: BackfillLogger, + options: BackfillOptions = {}, +): Promise { + const apply = options.apply === true; + const maxPerObject = options.maxRecordsPerObject ?? 100_000; + const actions: BackfillAction[] = []; + let scannedRecords = 0; + let truncated = false; + + const candidates = + options.objects ?? + (typeof engine.getConfigs === 'function' ? Object.keys(engine.getConfigs()) : []); + const scannedObjects = candidates.filter( + (name) => name !== 'sys_file' && fileFieldsOf(engine, name).length > 0, + ); + + for (const object of scannedObjects) { + const fileFields = fileFieldsOf(engine, object); + let offset = 0; + + for (;;) { + let page: Array>; + try { + page = await engine.find(object, { + fields: ['id', ...fileFields], + limit: SCAN_PAGE_SIZE, + offset, + context: { ...SYSTEM_CTX }, + }); + } catch (err) { + logger.warn( + `[storage] backfill: cannot read ${object} (${(err as Error)?.message ?? err}) — skipped`, + ); + break; + } + if (!page || page.length === 0) break; + + for (const record of page) { + const recordId = record?.id; + if (recordId == null) continue; + scannedRecords++; + + for (const field of fileFields) { + const raw = record[field]; + if (raw == null) continue; + const isArray = Array.isArray(raw); + const values = isArray ? raw : [raw]; + const next: unknown[] = []; + let changed = false; + + for (const value of values) { + const record_ = { object, recordId: String(recordId), field }; + + if (isFileIdToken(value)) { + actions.push({ + ...record_, + kind: 'already_id', + before: String(value), + after: String(value), + detail: 'already in reference form', + }); + next.push(value); + continue; + } + + const url = urlOf(value); + if (!url) { + actions.push({ + ...record_, + kind: 'unresolvable', + before: truncate(typeof value === 'string' ? value : JSON.stringify(value ?? null)), + detail: 'no url to interpret — left as-is', + }); + next.push(value); + continue; + } + + const local = LOCAL_FILE_URL_RE.exec(url); + if (local) { + const fileId = local[1]; + actions.push({ + ...record_, + kind: 'resolved_local_url', + before: truncate(url), + after: fileId, + detail: 'url already names a sys_file — rewritten to the bare id, no bytes moved', + }); + next.push(fileId); + changed = true; + continue; + } + + if (url.startsWith('data:')) { + const storage = getStorage(); + if (!storage) { + actions.push({ + ...record_, + kind: 'unresolvable', + before: truncate(url), + detail: 'inline data: bytes need a storage service to materialise — left as-is', + }); + next.push(value); + continue; + } + if (!apply) { + actions.push({ + ...record_, + kind: 'uploaded_inline_data', + before: truncate(url), + detail: 'would upload the inline bytes and rewrite to the new file id (dry run)', + }); + next.push(value); + changed = true; + continue; + } + try { + const newId = await materializeDataUri(engine, storage, url, value); + actions.push({ + ...record_, + kind: 'uploaded_inline_data', + before: truncate(url), + after: newId, + detail: 'inline bytes uploaded and registered as a sys_file', + }); + next.push(newId); + changed = true; + } catch (err) { + actions.push({ + ...record_, + kind: 'unresolvable', + before: truncate(url), + detail: `failed to materialise inline bytes (${(err as Error)?.message ?? err}) — left as-is`, + }); + next.push(value); + } + continue; + } + + // External URL. Deliberately not converted — see the module docs. + actions.push({ + ...record_, + kind: 'external_url_skipped', + before: truncate(url), + detail: + 'external URL — not re-hosted; model it as a `url` field rather than a file field (ADR-0104 R7)', + }); + next.push(value); + } + + if (!changed || !apply) continue; + try { + await engine.update( + object, + { id: recordId, [field]: isArray ? next : next[0] }, + { context: { ...SYSTEM_CTX } }, + ); + // The claim hooks installed by installFileReferenceHooks see this + // write and record ownership — the backfill does not write the + // ref_* columns itself, so there is exactly one claiming path. + } catch (err) { + logger.warn( + `[storage] backfill: failed to rewrite ${object}/${String(recordId)}.${field} ` + + `(${(err as Error)?.message ?? err})`, + ); + } + } + } + + offset += page.length; + if (page.length < SCAN_PAGE_SIZE) break; + if (offset >= maxPerObject) { + truncated = true; + break; + } + } + } + + const count = (k: BackfillActionKind) => actions.filter((a) => a.kind === k).length; + return { + scannedObjects, + scannedRecords, + converted: count('resolved_local_url') + count('uploaded_inline_data'), + externalUrls: count('external_url_skipped'), + unresolvable: count('unresolvable'), + alreadyReferences: count('already_id'), + actions, + applied: apply, + truncated, + }; +} + +/** Render a backfill report as human-readable lines. */ +export function formatBackfillReport(report: BackfillReport): string { + const lines: string[] = []; + lines.push( + `${report.applied ? 'Backfill applied' : 'Backfill DRY RUN (nothing written)'} — ` + + `${report.scannedRecords} record(s) across ${report.scannedObjects.length} object(s).`, + ); + lines.push( + ` converted: ${report.converted} already references: ${report.alreadyReferences} ` + + `external URLs left: ${report.externalUrls} unresolvable: ${report.unresolvable}`, + ); + if (report.truncated) { + lines.push('⚠ Scan was truncated by maxRecordsPerObject — re-run to continue.'); + } + if (report.externalUrls > 0) { + lines.push( + `\nExternal URLs are NOT re-hosted. Each needs a modelling decision — a \`url\` field rather ` + + `than a file field (ADR-0104 R7). They stay readable meanwhile:`, + ); + for (const a of report.actions.filter((x) => x.kind === 'external_url_skipped').slice(0, 20)) { + lines.push(` ${a.object}/${a.recordId}.${a.field}: ${a.before}`); + } + const extra = report.externalUrls - 20; + if (extra > 0) lines.push(` … and ${extra} more`); + } + if (report.unresolvable > 0) { + lines.push(`\nUnresolvable values (left untouched):`); + for (const a of report.actions.filter((x) => x.kind === 'unresolvable').slice(0, 20)) { + lines.push(` ${a.object}/${a.recordId}.${a.field}: ${a.detail}`); + } + } + if (!report.applied && report.converted > 0) { + lines.push(`\nRe-run with apply to perform ${report.converted} conversion(s).`); + } + if (report.applied) { + lines.push('\nNext: run verifyFileReferences to confirm ownership matches what records hold.'); + } + return lines.join('\n'); +} diff --git a/packages/services/service-storage/src/index.ts b/packages/services/service-storage/src/index.ts index b599af5bbb..c70ade7f7f 100644 --- a/packages/services/service-storage/src/index.ts +++ b/packages/services/service-storage/src/index.ts @@ -28,5 +28,14 @@ export type { VerifyReferencesEngine, VerifyReferencesOptions, } from './verify-file-references.js'; +export { backfillFileReferences, formatBackfillReport } from './backfill-file-references.js'; +export type { + BackfillAction, + BackfillActionKind, + BackfillEngine, + BackfillLogger, + BackfillOptions, + BackfillReport, +} from './backfill-file-references.js'; export { installAttachmentAccessHooks, installAttachmentReadVisibility } from './attachment-access-hooks.js'; export type { AttachmentSharingLike } from './attachment-access-hooks.js'; From 8bbaaa38fd7de06038955d2fabe9af3d017218ff Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 03:00:41 +0000 Subject: [PATCH 2/3] docs(adr-0104): backfill must precede the write cutover, not follow it The 2026-07-27 addendum sequenced the write cutover before the backfill. That is wrong for the same reason the backfill must precede collection, one step further back: narrowing the accepted stored form while records still hold legacy values would reject any update that rewrites such a field, breaking working apps until the backfill catches up. Backfilling first leaves the cutover nothing legacy to reject. Also states plainly why the last two steps are held to different standards. The cutover breaks loudly and recoverably (a rejected write); enabling collection breaks silently and permanently (deleted bytes). Only the second earns the reconciliation evidence requirement, and neither it nor the migration run should happen without an explicit human decision. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SHpGw3GBA9aFpfwVArRWfd --- ...0104-field-runtime-value-shape-contract.md | 36 ++++++++++++++----- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/docs/adr/0104-field-runtime-value-shape-contract.md b/docs/adr/0104-field-runtime-value-shape-contract.md index 456323a71a..46495ce473 100644 --- a/docs/adr/0104-field-runtime-value-shape-contract.md +++ b/docs/adr/0104-field-runtime-value-shape-contract.md @@ -567,14 +567,17 @@ sequences as: actually holds an id, so it lands safely ahead of the cutover. 2. **Governed download** — read authorisation derived from the one owning record; anonymous capability URL demoted to opt-in `acl: 'public_read'`. -3. **Write cutover** (protocol major) — stored form narrows to an id; +3. **Backfill** — converts legacy inline blobs and own-resolver URLs to + `sys_file` references, which the claim hooks then own; external URLs are + reported, never re-hosted. Dry-run by default, idempotent. +4. **Write cutover** (protocol major) — stored form narrows to an id; `accept` / `maxSize` enforced. Still deletes nothing. -4. **Backfill** — `os migrate` converts legacy inline blobs to `sys_file` rows - and claims them; external URLs reported, not converted. -5. **Reconciliation soak** — `os storage verify-references` compares what - records actually hold against recorded ownership, reporting *over-claim* - (safe: file retained longer than needed), *under-claim* (**blocking**: a held - file with no owner would be treated as free), and unclaimed orphans. +5. **Reconciliation soak** — `verify-references` compares what records actually + hold against recorded ownership, and splits disagreements by whether they can + cause data loss: *blocking* (a held file nothing owns; a file held by a slot + that does not own it; one file held by two slots) versus *advisory* (owned + but no longer held — fails toward retention; unreferenced committed files — + storage cost only). 6. **GC enable** — *gated, irreversible*. Relaxing the `scope === 'attachments'` tombstone guardrail and extending the `sys_file` reap guard's sweep-time re-verify to the ownership columns **must ship in the same change**. Half of @@ -582,9 +585,24 @@ sequences as: re-verifies only `sys_attachment` — always empty for a field file — turns every release into a guaranteed byte delete rather than a risky one. +Backfill precedes the cutover for the same reason it precedes collection, one +step further back: narrowing the accepted stored form while records still hold +legacy values would reject any update that rewrites such a field, breaking +working apps until the backfill catches up. Backfilling first leaves the cutover +nothing legacy to reject. + R4's acceptance gate is now executable rather than aspirational: step 6 may not -merge until step 5 reports **zero under-claims for ≥7 consecutive days** on real -tenant data. R5 (public-posture inventory) and R6 (sub-key read scan) stand. +merge until step 5 reports **zero blocking discrepancies for ≥7 consecutive +days** on real tenant data. R5 (public-posture inventory) and R6 (sub-key read +scan) stand. + +Steps 4 and 6 are the two that can break something. Step 4 is a declared +protocol major and breaks *loudly* — a rejected write, recoverable. Step 6 +breaks *silently and permanently* — deleted bytes. They are therefore held to +different standards: step 4 rides the planned v17 window paired with the client +adoption that consumes the new form, while step 6 additionally requires the +reconciliation evidence above, and neither the enable nor the migration run +should be performed without an explicit human decision. ### Why this stays inside ADR-0104 rather than a new ADR From 1b87aabbc61f3f2956a534b156f78ced7ea9de2f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 03:12:11 +0000 Subject: [PATCH 3/3] fix(storage): parse resolver URLs structurally instead of with a backtracking regex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL js/polynomial-redos: the resolver-URL pattern was anchored only at the tail, so the engine retried from every start position. The values it scans come straight out of tenant records, so a value repeating the matched prefix turns that into a polynomial blowup on attacker-influenced data. Replaced with slicing and splitting — linear in the URL's length whatever it contains, and clearer about what it actually accepts. Covered by a timing regression on an adversarial value plus a table of accepted shapes and near-misses. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SHpGw3GBA9aFpfwVArRWfd --- .../src/backfill-file-references.test.ts | 48 +++++++++++++++++++ .../src/backfill-file-references.ts | 33 +++++++++++-- 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/packages/services/service-storage/src/backfill-file-references.test.ts b/packages/services/service-storage/src/backfill-file-references.test.ts index 55b312cbd1..330c15095a 100644 --- a/packages/services/service-storage/src/backfill-file-references.test.ts +++ b/packages/services/service-storage/src/backfill-file-references.test.ts @@ -231,6 +231,54 @@ describe('backfillFileReferences (ADR-0104 D3 wave 2)', () => { expect(report.scannedObjects).toEqual(['product']); }); + /** + * The values scanned here come straight out of tenant records, so URL + * matching must not be able to backtrack. A pattern anchored only at the tail + * retries from every start position, which a value repeating the matched + * prefix turns into a polynomial blowup (CodeQL js/polynomial-redos). + */ + it('matches resolver URLs in linear time on an adversarial value', async () => { + // Many repetitions of the matched prefix, ending in a segment that is not + // an id token — the shape that makes a tail-anchored pattern retry from + // every start position. + const hostile = '/storage/files/'.repeat(6000) + '!'; + const engine = fakeEngine({ product: [{ id: 'p1', image: hostile }], sys_file: [] }); + + const started = process.hrtime.bigint(); + const report = await run(engine, {}); + const elapsedMs = Number(process.hrtime.bigint() - started) / 1e6; + + // Not a reference (the trailing segment is not an id token) — and reaching + // that verdict must not depend on how many times the prefix repeats. + expect(report.converted).toBe(0); + expect(elapsedMs).toBeLessThan(1000); + }); + + it('accepts the resolver URL shapes and rejects near-misses', async () => { + const engine = fakeEngine({ + product: [ + { id: 'p1', image: 'https://app.example.com/api/v1/storage/files/aaa' }, + { id: 'p2', image: '/api/v1/storage/files/bbb/url' }, + { id: 'p3', image: '/api/v1/storage/files/ccc?v=2' }, + { id: 'p4', image: '/api/v1/storage/files/ddd/' }, + { id: 'p5', image: 'https://cdn.example.com/files/eee' }, // no /storage + { id: 'p6', image: '/api/v1/storage/files/' }, // no id + ], + sys_file: [], + }); + + await run(engine, { apply: true }); + + expect(engine.tables.product.map((r) => r.image)).toEqual([ + 'aaa', + 'bbb', + 'ccc', + 'ddd', + 'https://cdn.example.com/files/eee', + '/api/v1/storage/files/', + ]); + }); + it('pages through large objects and flags truncation at the bound', async () => { const many = Array.from({ length: 500 }, (_, i) => ({ id: `p${i}`, diff --git a/packages/services/service-storage/src/backfill-file-references.ts b/packages/services/service-storage/src/backfill-file-references.ts index 9885f3bfe0..f4b0f98ba6 100644 --- a/packages/services/service-storage/src/backfill-file-references.ts +++ b/packages/services/service-storage/src/backfill-file-references.ts @@ -41,8 +41,6 @@ import type { IStorageService } from '@objectstack/spec/contracts'; const SYSTEM_CTX = { isSystem: true } as const; const SCAN_PAGE_SIZE = 200; -/** `…/storage/files/` (optionally with a trailing `/url` or query). */ -const LOCAL_FILE_URL_RE = /\/storage\/files\/([A-Za-z0-9_-]{1,64})(?:\/url)?(?:[?#].*)?$/; /** `data:;base64,` — the only inline form carrying real bytes. */ const DATA_URI_RE = /^data:([^;,]*)(;base64)?,(.*)$/s; @@ -116,6 +114,32 @@ function truncate(v: string, n = 96): string { return v.length <= n ? v : `${v.slice(0, n)}…`; } +/** + * The `sys_file` id a URL names, if it points at this platform's own resolver + * (`…/storage/files/`, optionally `/url`-suffixed or query-decorated). + * + * Parsed structurally rather than with a pattern. The obvious regex for this is + * unanchored at the head, so a hostile value repeating `/storage/files/` makes + * the engine retry from every position — polynomial backtracking on data that, + * here, comes straight out of tenant records. Slicing and splitting is linear + * in the URL's length no matter what it contains. + */ +function localFileIdFrom(url: string): string | null { + let path = url; + for (const sep of ['?', '#']) { + const at = path.indexOf(sep); + if (at >= 0) path = path.slice(0, at); + } + if (path.endsWith('/url')) path = path.slice(0, -'/url'.length); + while (path.endsWith('/')) path = path.slice(0, -1); + + const parts = path.split('/'); + if (parts.length < 3) return null; + if (parts[parts.length - 3] !== 'storage' || parts[parts.length - 2] !== 'files') return null; + const id = parts[parts.length - 1]; + return isFileIdToken(id) ? id : null; +} + /** The URL a legacy value points at, if any (inline blob or bare string). */ function urlOf(value: unknown): string | null { if (typeof value === 'string') return value; @@ -258,9 +282,8 @@ export async function backfillFileReferences( continue; } - const local = LOCAL_FILE_URL_RE.exec(url); - if (local) { - const fileId = local[1]; + const fileId = localFileIdFrom(url); + if (fileId) { actions.push({ ...record_, kind: 'resolved_local_url',