diff --git a/.changeset/adr-0104-d1-media-strict-per-deployment.md b/.changeset/adr-0104-d1-media-strict-per-deployment.md new file mode 100644 index 000000000..e43c1f579 --- /dev/null +++ b/.changeset/adr-0104-d1-media-strict-per-deployment.md @@ -0,0 +1,42 @@ +--- +"@objectstack/objectql": minor +--- + +feat(objectql)!: media value shapes enforce once THIS deployment has verified its file migration (#3438 D1 media half, gated by #3617) + +A `file` / `image` / `avatar` / `video` / `audio` value that does not match the +stored contract (an opaque `sys_file` id) now **rejects with `invalid_type`** +instead of warning — but only on a deployment that has run +`os migrate files-to-references --apply` and passed its self-check. + +**Why this is not a version-wide flip.** The legacy media values this rejects — +inline `{url, name, …}` blobs, bare URLs — are exactly what that migration +converts. A deployment that has run it has been *shown* to hold none; a +deployment that has not would have every media-field update start failing the +moment it upgraded. So the enforcement follows the evidence, per deployment, +rather than the release. Nothing changes for a deployment until it migrates. + +**Upgrading:** + +```bash +os migrate files-to-references # dry run: reports what would convert +os migrate files-to-references --apply # convert, verify, record the flag +``` + +If a write starts failing after you migrate, the value genuinely does not match +the contract — the error names the field. `OS_ALLOW_LAX_MEDIA_VALUES=1` re-opens +media leniency while you diagnose. + +**Scope — deliberately only media.** `OS_DATA_VALUE_SHAPE_STRICT_ENABLED` is +unchanged and still opts every class into strict (and still forces media strict +on a deployment that has not migrated). Reference types (`lookup`, `user`, …) +and structured JSON (`location`, `address`, `repeater`, …) stay warn-first: the +file migration is evidence about file values and says nothing about whether a +`location` is well formed, so gating them on its flag would be borrowing +evidence for a fact it does not cover. They flip when something can vouch for +them — see #3438. + +**Cost.** Dormant unless the written object declares a media field, and the +flag read is memoized, so this is one query per process for apps that store +files and zero for those that do not. A running server picks up a +newly-recorded migration on restart, or via `engine.invalidateDataMigrationFlags()`. diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 0cb1d7525..79ad3ab17 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -545,18 +545,31 @@ field instead. The run then reconciles what records actually hold against what `sys_file` records as each file's owner. Zero blocking discrepancies is what records the -flag — and that flag, not the version number, is what later enables features -that act irreversibly on migrated data (reclaiming the bytes of released files; -rejecting rather than warning about legacy media values). Never running it is -safe: files are simply retained forever. +flag — and that flag, not the version number, is what enables behaviour that +depends on the data actually being migrated. Never running it is safe: files +are simply retained forever, and media values keep warning instead of failing. Exit status is `0` only when the self-check passes, so CI can gate on it. +#### What the flag turns on + +| Once verified | Effect | +| :--- | :--- | +| **Media value shapes** | A malformed `file` / `image` / `avatar` / `video` / `audio` value is **rejected** (`400 invalid_type`) instead of warned about. Set `OS_ALLOW_LAX_MEDIA_VALUES=1` to re-open leniency while diagnosing. | +| **Released-file collection** | Not yet shipped — the flag is the gate it will read. | + +Other value classes are unaffected: a `lookup` or `location` value keeps its own +warn-first rollout, because this migration is evidence about *file* values and +says nothing about theirs. + A dry run writes **nothing** — not the conversions, and not the flag either, even when the self-check would pass. `--apply` is the only writing mode. A later run that *fails* its self-check clears the flag's verified state, so a database that has drifted closes its own gate. + +A running server reads the flag once; after migrating, **restart it** for +enforcement to take effect. ### Scaffolding diff --git a/docs/adr/0104-field-runtime-value-shape-contract.md b/docs/adr/0104-field-runtime-value-shape-contract.md index 3f6f3ea1f..e424177a5 100644 --- a/docs/adr/0104-field-runtime-value-shape-contract.md +++ b/docs/adr/0104-field-runtime-value-shape-contract.md @@ -419,8 +419,12 @@ Therefore the target end-state is **two enforcement points, each with one job**: - **Build/validate time → hard reject.** Net-new metadata (the AI's output) must fail loudly at authoring. This is the primary defence against AI error. - **Runtime → warn-first, then flip.** Deployed data keeps working through the - warn window; the flip to strict-by-default (tracked in #3438) closes it once - telemetry is quiet. + warn window; the flip to strict-by-default (tracked in #3438) closes it. + *(Amended by the second 2026-07-27 addendum: "once telemetry is quiet" was + our telemetry deciding for their deployments. The **media** half now flips + per deployment, when that deployment's own file-as-reference migration has + verified. The reference and structured-JSON halves still await an evidence + source of their own — the file migration does not vouch for them.)* This refines D2 (today runtime-only) and reshapes the #3438 flip: the priority is adding the **build-time** rejection for value shapes and action params, not @@ -670,6 +674,28 @@ a deployment when that deployment has completed and verified its migration — not when a version number arrives. One flag, not two gates that can disagree: they are gating on the same fact. +**Amendment while implementing this (2026-07-27, later still).** "#3438 reads +the flag" was too broad: #3438 is three things, and the flag only covers one. +The flag asserts *this deployment's file-field values have been migrated and +reconciled*. That is evidence about **media** value shapes and nothing else — +it says nothing about whether a `lookup` id or a `location` payload is well +formed, and nothing at all about D2's action parameters. Gating those on it +would be borrowing evidence for a fact it does not cover, which is the same +error one layer down: an authority answering a question it was not asked. + +So the split is: + +| | evidence | status | +|---|---|---| +| D1 media (`file`/`image`/`avatar`/`video`/`audio`) | the `adr-0104-file-references` flag | flips per deployment | +| D1 references + structured JSON | none yet — needs its own | stays warn-first | +| D2 action params | unrelated to any data migration | stays warn-first | + +The last two are not blocked on the mechanism — it exists and works. They are +blocked on someone deciding what would constitute evidence that a deployment's +`location` values are sound, which is a different question from the one this +addendum answers. + ### What cannot be automated, and does not block Whether an **external URL** (`https://cdn…`) should become an explicit `url` diff --git a/packages/cli/src/commands/migrate/files-to-references.ts b/packages/cli/src/commands/migrate/files-to-references.ts index 568a01ee8..cade4777b 100644 --- a/packages/cli/src/commands/migrate/files-to-references.ts +++ b/packages/cli/src/commands/migrate/files-to-references.ts @@ -253,7 +253,9 @@ export default class MigrateFilesToReferences extends Command { if (apply) { printSuccess( 'Self-check passed — deployment flag recorded (adr-0104-file-references). ' + - 'This deployment is now eligible for strict media enforcement and, once shipped, file collection.', + 'Media value shapes are now ENFORCED on this deployment: a malformed ' + + 'file/image value is rejected rather than warned about. ' + + '(Set OS_ALLOW_LAX_MEDIA_VALUES=1 to re-open leniency while diagnosing.)', ); } else if (result.backfill.converted > 0) { printInfo( diff --git a/packages/objectql/src/engine.test.ts b/packages/objectql/src/engine.test.ts index 4d05da81d..5373955dc 100644 --- a/packages/objectql/src/engine.test.ts +++ b/packages/objectql/src/engine.test.ts @@ -2075,3 +2075,132 @@ describe('ObjectQL Engine', () => { }); }); }); + +/** + * #3617 / #3438 — the engine carries the deployment's own migration evidence + * into the (synchronous, per-write) value-shape validator. + * + * The fact lives in `sys_migration`, so it needs a database read; the write + * path needs it on every insert/update. These prove the read happens once, + * and that every way of NOT knowing lands on lenient — a deployment whose + * evidence cannot be read keeps writing rather than starting to reject. + */ +describe('ObjectQL — file-as-reference migration flag (#3617)', () => { + let engine: ObjectQL; + let driver: IDataDriver; + + const verifiedRow = { + id: 'adr-0104-file-references', + last_run_at: '2026-07-27T00:00:00.000Z', + verified_at: '2026-07-27T00:00:00.000Z', + blocking: 0, + }; + + beforeEach(() => { + vi.clearAllMocks(); + driver = { + name: 'default-driver', + connect: vi.fn().mockResolvedValue(undefined), + disconnect: vi.fn().mockResolvedValue(undefined), + find: vi.fn().mockResolvedValue([]), + findOne: vi.fn(), + create: vi.fn().mockResolvedValue({ id: '1' }), + update: vi.fn().mockResolvedValue({ id: '1' }), + delete: vi.fn(), + count: vi.fn(), + capabilities: {} as any, + } as unknown as IDataDriver; + engine = new ObjectQL(); + engine.registerDriver(driver, true); + }); + + /** `doc` holds a legacy inline blob — exactly what the migration converts. */ + const withMediaObject = () => { + vi.mocked(SchemaRegistry.getObject).mockImplementation((name: string) => { + if (name === 'note') return { name: 'note', fields: { doc: { type: 'file' } } } as any; + if (name === 'sys_migration') return { name: 'sys_migration', fields: { id: { type: 'text' } } } as any; + return undefined; + }); + }; + const legacyBlob = { doc: { url: 'https://cdn/f.pdf', name: 'f.pdf' } }; + + it('a verified flag makes a malformed media value reject', async () => { + withMediaObject(); + vi.mocked(driver.find).mockResolvedValue([verifiedRow] as any); + + await expect(engine.insert('note', { ...legacyBlob })).rejects.toThrow(/invalid file value/i); + }); + + it('no flag row → lenient (the write goes through)', async () => { + withMediaObject(); + vi.mocked(driver.find).mockResolvedValue([]); + + await expect(engine.insert('note', { ...legacyBlob })).resolves.toBeDefined(); + }); + + it('a failed run (verified_at cleared) → lenient', async () => { + withMediaObject(); + vi.mocked(driver.find).mockResolvedValue([{ ...verifiedRow, verified_at: null, blocking: 3 }] as any); + + await expect(engine.insert('note', { ...legacyBlob })).resolves.toBeDefined(); + }); + + it('an unreadable sys_migration → lenient, and the write is not failed by the probe', async () => { + withMediaObject(); + vi.mocked(driver.find).mockRejectedValue(new Error('no such table: sys_migration')); + + await expect(engine.insert('note', { ...legacyBlob })).resolves.toBeDefined(); + }); + + /** + * Dormancy, mirroring the storage module's own rule: an object with no + * file-class field can hold no malformed media value, so it must not pay + * even one query to learn that. Nearly every object is in this case. + */ + it('costs no query for an object that declares no media field', async () => { + vi.mocked(SchemaRegistry.getObject).mockImplementation((name: string) => { + if (name === 'invoice') return { name: 'invoice', fields: { amount: { type: 'number' } } } as any; + if (name === 'sys_migration') return { name: 'sys_migration', fields: { id: { type: 'text' } } } as any; + return undefined; + }); + vi.mocked(driver.find).mockResolvedValue([verifiedRow] as any); + + await expect(engine.insert('invoice', { amount: 10 })).resolves.toBeDefined(); + expect(vi.mocked(driver.find).mock.calls.filter((c) => c[0] === 'sys_migration')).toHaveLength(0); + }); + + /** No storage service → no sys_migration object → not even a query. */ + it('costs no query on a kernel without the storage objects', async () => { + vi.mocked(SchemaRegistry.getObject).mockImplementation((name: string) => + name === 'note' ? ({ name: 'note', fields: { doc: { type: 'file' } } } as any) : undefined, + ); + vi.mocked(driver.find).mockResolvedValue([]); + + await expect(engine.insert('note', { ...legacyBlob })).resolves.toBeDefined(); + expect(driver.find).not.toHaveBeenCalled(); + }); + + it('reads the flag once per process, not once per write', async () => { + withMediaObject(); + vi.mocked(driver.find).mockResolvedValue([verifiedRow] as any); + + await expect(engine.insert('note', { doc: 'file_01' })).resolves.toBeDefined(); + await expect(engine.insert('note', { doc: 'file_02' })).resolves.toBeDefined(); + await expect(engine.insert('note', { doc: 'file_03' })).resolves.toBeDefined(); + + const flagReads = vi.mocked(driver.find).mock.calls.filter((c) => c[0] === 'sys_migration'); + expect(flagReads).toHaveLength(1); + }); + + it('invalidateDataMigrationFlags forces a re-read', async () => { + withMediaObject(); + vi.mocked(driver.find).mockResolvedValue([verifiedRow] as any); + + await engine.insert('note', { doc: 'file_01' }); + engine.invalidateDataMigrationFlags(); + await engine.insert('note', { doc: 'file_02' }); + + const flagReads = vi.mocked(driver.find).mock.calls.filter((c) => c[0] === 'sys_migration'); + expect(flagReads).toHaveLength(2); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 01d62a82b..23bad4c13 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -13,6 +13,11 @@ import { } from '@objectstack/spec/data'; import type { WriteObservabilityOptions } from '@objectstack/spec/contracts'; import { parseAutonumberFormat, renderAutonumber, missingFieldValues, isTenancyDisabled, FILE_REFERENCE_TYPES, isFileIdToken, RAW_FILE_VALUES_CONTEXT_KEY } from '@objectstack/spec/data'; +import { + DATA_MIGRATION_FLAG_OBJECT, + FILE_REFERENCES_MIGRATION_ID, + isDataMigrationFlagVerified, +} from '@objectstack/spec/system'; import { ExecutionContext, ExecutionContextSchema } from '@objectstack/spec/kernel'; import { IDataDriver, IDataEngine, Logger, createLogger, withTransientRetry, type RetryOptions } from '@objectstack/core'; import { SummaryRecomputeError, type SummaryRecomputeFailure } from './summary-errors.js'; @@ -1878,6 +1883,96 @@ export class ObjectQL implements IDataEngine { this.logger.info('ObjectQL engine initialization complete'); } + /** + * Does this object declare a media field at all? Cached per schema object — + * the registry hands back the same instance per object, so this scans a + * field map once rather than on every write. + */ + private objectHasMediaField(objectSchema: any): boolean { + if (!objectSchema?.fields) return false; + const cached = ObjectQL.mediaFieldPresence.get(objectSchema); + if (cached !== undefined) return cached; + const present = Object.values(objectSchema.fields).some( + (def: any) => def && FILE_REFERENCE_TYPES.has(def.type), + ); + ObjectQL.mediaFieldPresence.set(objectSchema, present); + return present; + } + private static readonly mediaFieldPresence = new WeakMap(); + + /** + * The media value-shape verdict for a write against `objectSchema`. + * + * Dormant by design, mirroring the storage module's own rule: an object with + * no file-class field can hold no malformed media value, so it must not pay + * even one query to learn that. Only objects that actually declare media + * consult the deployment flag — and that read is itself memoized, so the + * whole mechanism costs one query per process, and zero for an app that + * stores no files. + */ + private async mediaValueShapeStrictFor(objectSchema: any): Promise { + if (!this.objectHasMediaField(objectSchema)) return false; + return this.isFileReferencesMigrationVerified(); + } + + /** + * Has this deployment completed AND verified the ADR-0104 file-as-reference + * migration (#3617)? Read once per process from the `sys_migration` flag. + * + * Every way of not knowing answers `false` — no storage service (so no + * `sys_migration` object registered), no row, an unreadable table, a + * malformed row. That is the same posture the flag's other consumer takes: + * enforcement derives from evidence, and absent evidence is not permission. + * Here "no" means media value shapes keep warning instead of rejecting, so + * a deployment that cannot be asked keeps writing. + * + * Costs nothing on a kernel without the storage objects: the registry lookup + * short-circuits before any query. + */ + private async isFileReferencesMigrationVerified(): Promise { + if (!this.fileReferencesMigrationVerified) { + this.fileReferencesMigrationVerified = (async () => { + if (!this._registry.getObject(DATA_MIGRATION_FLAG_OBJECT)) return false; + try { + const rows = await this.find(DATA_MIGRATION_FLAG_OBJECT, { + where: { id: FILE_REFERENCES_MIGRATION_ID }, + limit: 1, + context: { isSystem: true } as ExecutionContext, + }); + const row: any = rows?.[0]; + if (!row || row.id !== FILE_REFERENCES_MIGRATION_ID) return false; + const verified = isDataMigrationFlagVerified({ + id: FILE_REFERENCES_MIGRATION_ID, + last_run_at: String(row.last_run_at ?? ''), + verified_at: row.verified_at == null ? null : String(row.verified_at), + // A non-numeric count must read as "not zero", not as 0 — a bad + // coercion lands on NaN, which fails the === 0 test. + blocking: typeof row.blocking === 'number' ? row.blocking : Number(row.blocking ?? Number.NaN), + }); + if (verified) { + this.logger.info( + '[value-shape] this deployment has verified the file-as-reference migration — ' + + 'media value shapes are enforced (ADR-0104 D1 / #3617)', + ); + } + return verified; + } catch { + return false; // unreadable evidence → stay lenient + } + })(); + } + return this.fileReferencesMigrationVerified; + } + + /** + * Drop the memoized deployment migration flags so the next write re-reads + * them. For a host that runs a data migration in-process and wants its + * effect without a restart. + */ + invalidateDataMigrationFlags(): void { + this.fileReferencesMigrationVerified = null; + } + async destroy() { this.logger.info('Destroying ObjectQL engine', { driverCount: this.drivers.size }); @@ -1903,6 +1998,20 @@ export class ObjectQL implements IDataEngine { * lazily seeded from the current max in the store. */ private readonly autonumberCounters = new Map(); + /** + * Memoized answer to "has THIS deployment completed and verified the + * ADR-0104 file-as-reference migration?" (#3617) — the fact that decides + * whether a malformed media value rejects or merely warns. + * + * Memoized as a PROMISE so concurrent first writes share one read rather + * than racing several. Deliberately not refreshed on a timer: the flag is + * written by `os migrate files-to-references --apply`, a deliberate + * operator action, and a process that has not seen it yet simply stays + * lenient — the safe direction. A host that migrates in-process can call + * {@link invalidateDataMigrationFlags} instead of waiting for a restart. + */ + private fileReferencesMigrationVerified: Promise | null = null; + /** Lazily-built index: child object name → roll-up summary descriptors on * parent objects that aggregate it. Invalidated when packages register. */ private summaryIndex: Map | null = null; @@ -2618,11 +2727,14 @@ export class ObjectQL implements IDataEngine { rowErrors[i] = e; } } + // Resolved once for the whole batch; dormant unless the object declares + // a media field, and memoized after the first object that does. + const mediaValueShapeStrict = await this.mediaValueShapeStrictFor(schemaForValidation); for (let i = 0; i < rows.length; i++) { if (rowErrors[i] !== undefined) continue; try { normalizeMultiValueFields(schemaForValidation, rows[i]); - validateRecord(schemaForValidation, rows[i], 'insert'); + validateRecord(schemaForValidation, rows[i], 'insert', { mediaValueShapeStrict }); evaluateValidationRules(schemaForValidation as any, rows[i], 'insert', { logger: this.logger, currentUser: this.buildEvalUser(opCtx.context), skipStateMachine: shouldSkipStateMachine(opCtx.context) }); } catch (e) { if (!partialMode) throw e; @@ -2889,10 +3001,11 @@ export class ObjectQL implements IDataEngine { // which silently fails when `previous` is absent. let priorRecord: Record | null = null; const updateSchema = this._registry.getObject(object); + const mediaValueShapeStrict = await this.mediaValueShapeStrictFor(updateSchema); if (hookContext.input.id) { await this.encryptSecretFields(object, hookContext.input.data as Record, opCtx.context, hookContext.input.options); normalizeMultiValueFields(updateSchema, hookContext.input.data as Record); - validateRecord(updateSchema, hookContext.input.data as Record, 'update'); + validateRecord(updateSchema, hookContext.input.data as Record, 'update', { mediaValueShapeStrict }); if (needsPriorRecord(updateSchema as any) || (this.hooks.get('afterUpdate')?.length ?? 0) > 0) { const priorAst: QueryAST = { object, where: { id: hookContext.input.id }, limit: 1 }; priorRecord = await driver.findOne(object, priorAst, hookContext.input.options as any); @@ -2918,7 +3031,7 @@ export class ObjectQL implements IDataEngine { } else if (options?.multi && driver.updateMany) { await this.encryptSecretFields(object, hookContext.input.data as Record, opCtx.context, hookContext.input.options); normalizeMultiValueFields(updateSchema, hookContext.input.data as Record); - validateRecord(updateSchema, hookContext.input.data as Record, 'update'); + validateRecord(updateSchema, hookContext.input.data as Record, 'update', { mediaValueShapeStrict }); // [#2982] Consume the middleware-composed AST seeded above, so // the injected row-scoping (RLS write filter, sharing's // editable-rows filter) actually binds the driver operation. Fail diff --git a/packages/objectql/src/validation/record-validator.test.ts b/packages/objectql/src/validation/record-validator.test.ts index 7c86fbff3..be06898db 100644 --- a/packages/objectql/src/validation/record-validator.test.ts +++ b/packages/objectql/src/validation/record-validator.test.ts @@ -417,3 +417,93 @@ describe('validateRecord — ADR-0104 value shapes (warn-first / strict)', () => }); }); }); + +/** + * #3617 / #3438 — media value shapes enforce per DEPLOYMENT, not per release. + * + * A verified deployment has RUN the file-as-reference migration and had its + * ownership ledger reconciled, so it has been shown to hold no legacy media + * values. Nothing about that migration vouches for a `lookup` or `location` + * value, so those classes keep their own (unchanged) warn-first rollout — + * gating them on this flag would borrow evidence for a fact it does not cover. + */ +describe('validateRecord — media value shapes gate on the deployment flag (#3617)', () => { + const schema = { + fields: { + doc: { type: 'file' }, + cover: { type: 'image' }, + account: { type: 'lookup', reference: 'accounts' }, + geo: { type: 'location' }, + }, + }; + // A legacy inline blob — precisely what the migration converts. + const legacyMedia = { doc: { url: 'https://cdn/f.pdf', name: 'f.pdf' } }; + const malformedNonMedia = { geo: { latitude: 1, longitude: 2 } }; + + const strict = { mediaValueShapeStrict: true }; + + const withEnv = (key: string, fn: () => void) => { + process.env[key] = '1'; + try { fn(); } finally { delete process.env[key]; } + }; + + it('unverified deployment (the default): media stays warn-first', () => { + expect(() => validateRecord(schema, { ...legacyMedia }, 'update')).not.toThrow(); + expect(() => validateRecord(schema, { ...legacyMedia }, 'update', {})).not.toThrow(); + expect(() => + validateRecord(schema, { ...legacyMedia }, 'update', { mediaValueShapeStrict: false }), + ).not.toThrow(); + }); + + it('verified deployment: a malformed media value rejects with invalid_type', () => { + try { + validateRecord(schema, { ...legacyMedia }, 'update', strict); + expect.unreachable('expected ValidationError'); + } catch (e) { + expect(e).toBeInstanceOf(ValidationError); + const err = e as ValidationError; + expect(err.fields[0]?.field).toBe('doc'); + expect(err.fields[0]?.code).toBe('invalid_type'); + } + // and the conformant reference form still passes + expect(() => validateRecord(schema, { doc: 'file_01HXYZ', cover: 'file_02' }, 'update', strict)).not.toThrow(); + }); + + /** + * The whole reason for splitting #3438: a verified file migration is not + * evidence about lookup/location values, so it must not start rejecting them. + */ + it('verified deployment does NOT flip non-media classes', () => { + expect(() => validateRecord(schema, { ...malformedNonMedia }, 'update', strict)).not.toThrow(); + expect(() => validateRecord(schema, { account: { id: 'acc_1' } }, 'update', strict)).not.toThrow(); + }); + + it('the blanket opt-in still forces media strict on an unverified deployment', () => { + withEnv('OS_DATA_VALUE_SHAPE_STRICT_ENABLED', () => { + expect(() => validateRecord(schema, { ...legacyMedia }, 'update')).toThrow(ValidationError); + }); + }); + + it('OS_ALLOW_LAX_MEDIA_VALUES re-opens media on a verified deployment', () => { + withEnv('OS_ALLOW_LAX_MEDIA_VALUES', () => { + expect(() => validateRecord(schema, { ...legacyMedia }, 'update', strict)).not.toThrow(); + }); + }); + + /** + * A contradictory configuration lands on the lenient side: a warning nobody + * reads costs less than an app that stops writing. + */ + it('opt-out beats opt-in when both are set', () => { + withEnv('OS_DATA_VALUE_SHAPE_STRICT_ENABLED', () => { + withEnv('OS_ALLOW_LAX_MEDIA_VALUES', () => { + expect(() => validateRecord(schema, { ...legacyMedia }, 'update', strict)).not.toThrow(); + }); + }); + }); + + it('applies on insert too, not only update', () => { + expect(() => validateRecord(schema, { ...legacyMedia }, 'insert', strict)).toThrow(ValidationError); + expect(() => validateRecord(schema, { ...legacyMedia }, 'insert')).not.toThrow(); + }); +}); diff --git a/packages/objectql/src/validation/record-validator.ts b/packages/objectql/src/validation/record-validator.ts index 3bcec2de3..f63ae2f81 100644 --- a/packages/objectql/src/validation/record-validator.ts +++ b/packages/objectql/src/validation/record-validator.ts @@ -225,7 +225,13 @@ export function coerceBooleanFields>( return (copy ?? row) as T; } -function validateOne(name: string, def: FieldDef, value: unknown, skipRequired = false): FieldValidationError | null { +function validateOne( + name: string, + def: FieldDef, + value: unknown, + skipRequired = false, + mediaStrict = false, +): FieldValidationError | null { // ── required ──────────────────────────────────────────────────── // `autonumber` is runtime-owned: the value is generated by the engine / // driver (the SQL driver assigns it from a persistent sequence AFTER this @@ -341,23 +347,43 @@ function validateOne(name: string, def: FieldDef, value: unknown, skipRequired = } // ── previously-opaque types: value-shape contract (ADR-0104 D1) ───── - // Single-value references (id string), file-likes (id/url string or the - // pre-D3 inline metadata object), and structured JSON payloads - // (location/address/composite/repeater/record/vector) are checked against - // the spec's `valueSchemaFor`. Warn-first rollout (ADR-0104 R1/R2): a - // violation logs instead of rejecting, so legacy rows written under the - // lax regime don't strand their records; set - // `OS_DATA_VALUE_SHAPE_STRICT_ENABLED=1` to enforce as a 400. The flip to - // error-by-default rides a later minor once telemetry is quiet. + // Single-value references (id string), file-likes (a `sys_file` id), and + // structured JSON payloads (location/address/composite/repeater/record/ + // vector) are checked against the spec's `valueSchemaFor`. + // + // Enforcement differs by class, because the EVIDENCE for enforcing differs + // (#3617, splitting what #3438 originally packaged as one flip): + // + // - **Media** (`file`/`image`/`avatar`/`video`/`audio`) enforces as soon as + // THIS DEPLOYMENT has completed and self-check-verified its + // file-as-reference migration. Its legacy values — inline blobs, bare + // URLs — are exactly what that migration converts, so a verified + // deployment has been SHOWN to have none left. `mediaStrict` carries that + // per-deployment fact in (the engine reads the `sys_migration` flag); + // ADR-0104 R7 external URLs are the deliberate exception the migration + // reports and never converts, so a deployment still holding them fails + // its self-check and stays lenient here. + // - **References and structured JSON** stay warn-first: the file migration + // says nothing about whether a `lookup` or `location` value is well + // formed, so gating them on ITS flag would be borrowing evidence for a + // fact it does not cover. They flip when something can vouch for them. if (REFERENCE_VALUE_TYPES.has(t) || FILE_REFERENCE_TYPES.has(t) || STRUCTURED_JSON_TYPES.has(t)) { const parsed = shapeSchemaFor(def).safeParse(value); if (!parsed.success) { const detail = parsed.error.issues[0]?.message ?? 'invalid value shape'; const message = `${name} has an invalid ${t} value: ${detail}`; - if (VALUE_SHAPE_STRICT()) { + const isMedia = FILE_REFERENCE_TYPES.has(t); + if (isMedia ? mediaStrictEffective(mediaStrict) : VALUE_SHAPE_STRICT()) { return { field: name, code: 'invalid_type', message }; } - warnOnce(`${t}:${name}`, `[value-shape] ${message} — accepted for now (ADR-0104 warn-first; set OS_DATA_VALUE_SHAPE_STRICT_ENABLED=1 to enforce)`); + warnOnce( + `${t}:${name}`, + `[value-shape] ${message} — accepted for now (ADR-0104 warn-first; ` + + (isMedia + ? 'run `os migrate files-to-references --apply` to migrate this deployment and enforce' + : 'set OS_DATA_VALUE_SHAPE_STRICT_ENABLED=1 to enforce') + + ')', + ); } return null; } @@ -372,6 +398,29 @@ function VALUE_SHAPE_STRICT(): boolean { return typeof process !== 'undefined' && process.env?.OS_DATA_VALUE_SHAPE_STRICT_ENABLED === '1'; } +/** + * Escape hatch: keep media value shapes lenient even on a deployment whose + * migration verified. For an operator who hits an unforeseen rejection and + * needs their app writing again before diagnosing. + */ +function LAX_MEDIA_VALUES(): boolean { + return typeof process !== 'undefined' && process.env?.OS_ALLOW_LAX_MEDIA_VALUES === '1'; +} + +/** + * Does a media value-shape violation reject, for this deployment? + * + * Three inputs, and the precedence is chosen so a CONTRADICTORY configuration + * lands on the safe side: the opt-out wins over the opt-in, because the cost + * of wrongly staying lenient is a warning nobody reads, while the cost of + * wrongly enforcing is a working app that stops writing. + */ +function mediaStrictEffective(deploymentVerified: boolean): boolean { + if (LAX_MEDIA_VALUES()) return false; + if (VALUE_SHAPE_STRICT()) return true; + return deploymentVerified; +} + const warnedShapes = new Set(); function warnOnce(key: string, message: string): void { if (warnedShapes.has(key)) return; @@ -396,6 +445,21 @@ function shapeSchemaFor(def: FieldDef): ReturnType { return schema; } +export interface ValidateRecordOptions { + /** + * Has THIS DEPLOYMENT completed and self-check-verified the ADR-0104 + * file-as-reference migration (#3617)? When true, a malformed media value + * rejects instead of warning. + * + * Passed in rather than read here because the fact lives in the database + * (`sys_migration`) while this validator is synchronous and per-write — the + * engine reads it once and memoizes. Defaults to `false`: a caller that + * cannot say stays lenient, so nothing starts rejecting writes merely + * because the evidence was unavailable. + */ + mediaValueShapeStrict?: boolean; +} + /** * Validate a payload against a list of declared fields. `objectSchema` * comes from `ObjectQL.getRegistry().getObject(name)` and exposes a @@ -407,11 +471,13 @@ export function validateRecord( objectSchema: { fields?: Record } | undefined | null, data: Record | undefined | null, mode: Mode, + options: ValidateRecordOptions = {}, ): void { if (!objectSchema?.fields || !data) return; const errors: FieldValidationError[] = []; const fields = objectSchema.fields; + const mediaStrict = options.mediaValueShapeStrict === true; if (mode === 'insert') { // Walk all declared fields — required check applies even when @@ -419,7 +485,7 @@ export function validateRecord( for (const [name, def] of Object.entries(fields)) { if (SKIP_FIELDS.has(name)) continue; if (def.system || def.readonly) continue; - const err = validateOne(name, def, data[name]); + const err = validateOne(name, def, data[name], false, mediaStrict); if (err) errors.push(err); } } else { @@ -432,7 +498,7 @@ export function validateRecord( // skipRequired: PATCH-omitted fields must not 400. (No def clone — the // registry's own field object flows through so the ADR-0104 value-shape // schema cache, keyed on def identity, hits.) - const err = validateOne(name, def, value, true); + const err = validateOne(name, def, value, true, mediaStrict); if (err) errors.push(err); } }