diff --git a/.changeset/json-schema-rule-format-enforced.md b/.changeset/json-schema-rule-format-enforced.md new file mode 100644 index 0000000000..82187f6e68 --- /dev/null +++ b/.changeset/json-schema-rule-format-enforced.md @@ -0,0 +1,96 @@ +--- +"@objectstack/objectql": minor +"@objectstack/lint": patch +--- + +fix(objectql,lint)!: a `json_schema` validation rule's `format` keyword is now ENFORCED — records that passed before can start failing (#5029) + +> **⚠️ BEHAVIOUR CHANGE ON DEPLOYED DATA — READ BEFORE UPGRADING.** A `format` +> keyword inside a `json_schema` validation rule used to enforce **nothing**. +> It now enforces. If any deployed object carries such a rule, writes that +> succeeded on the previous version can be **rejected** after this upgrade — +> including writes from flows, seeds, imports and integrations, not just the UI. +> Nothing about the metadata changed; the runtime simply started honouring what +> the metadata always said. See "Before you upgrade" below. + +## What was broken + +`packages/objectql/src/validation/rule-validator.ts` built its shared ajv as +`new Ajv({ allErrors: true, strict: false })` and stopped there. In ajv 8 the +`format` keyword is **not built in** — it ships in the separate `ajv-formats` +package — and under `strict: false` an unregistered format is not an error: ajv +logs one line at compile time and **drops the keyword**. + +So this rule: + +```ts +{ + type: 'json_schema', + name: 'support_config_shape', + field: 'support_config', + message: 'Support config is invalid.', + schema: { + type: 'object', + properties: { email: { type: 'string', format: 'email' } }, + required: ['email'], + }, +} +``` + +compiled fine, ran on **every** write, enforced `type` and `required` — and +enforced **nothing at all** for `format`. `{ email: 'not-an-email' }` was +accepted, for every record, forever. The only signal was a stderr line at +compile time naming no rule and no object. + +This is the #4649 / #4762 family one level in, and the partial failure is what +made it nasty: the rule visibly rejects a bad `type` / missing `required` +payload in dev, so it reads as *working* while the `format` half never fires. +`format` is also one of the most reached-for JSON Schema keywords (`email`, +`uri`, `uuid`, `date`, `date-time`, `ipv4`), so this was not an exotic corner — +and it is exactly the shape an AI writing metadata reaches for first. + +## What changed + +- **`@objectstack/objectql`** now depends on `ajv-formats` and registers it on + the shared instance (`addFormats(ajv)`). The **default (full)** format set is + used deliberately: `fast` mode trades correctness for speed on precisely the + formats authors reach for most, and a format that "mostly" matches is the same + declared ≠ enforced defect with a smaller hole. +- **`@objectstack/lint`** — the #4762 publish gate + (`validate-rule-compilability.ts`) compiles every `json_schema` rule with the + SAME ajv environment the runtime uses, on purpose, so it registers the same + plugin. This is not cosmetic parity: `ajv-formats` also installs the + `formatMinimum` / `formatMaximum` keywords, so a gate without it treats them + as unknown keywords (`strict: false` ⇒ silently ignored) and would publish a + schema the runtime then refuses to compile — a rule that passes review and + enforces nothing, which is the failure that gate exists to prevent. The parity + test now reads the plugin registration out of the runtime's source, so the two + cannot drift apart silently. + +**Authoring is unchanged.** `format` stays a legal, publishable JSON Schema +keyword; the publish gate does not refuse it (option 2 on #5029 was considered +and rejected — refusing standard JSON Schema would push authors into private +spellings). What changed is only that the declaration is now true. + +## Before you upgrade + +1. Find the rules at risk: any `object.validations[]` entry with + `type: 'json_schema'` whose `schema` contains a `format` key, at any depth + (including inside `$defs` / `$ref` and a `conditional`'s `then` / + `otherwise` branch). +2. For each, audit the existing column against that format. Rows already stored + are **not** re-validated — nothing is rejected retroactively, and no + migration runs — but the **next write that touches the field** is checked, + which includes an unrelated PATCH that merely resends the JSON blob. +3. If a format was aspirational rather than real, remove that `format` key (or + relax it) *before* upgrading. Deleting the keyword is now a meaningful, + visible act rather than a no-op. + +## Known limitation, recorded deliberately + +A **misspelled** format name is still ignored. `format: 'emial'` compiles under +`strict: false` — ajv logs `unknown format "emial" ignored` and drops it — in +both the runtime and the publish gate, so a typo still enforces nothing. That +behaviour is unchanged here and pinned by test in both packages, so it is a +known boundary rather than an oversight; closing it is an authoring-time +decision of its own and is tracked separately. diff --git a/packages/lint/package.json b/packages/lint/package.json index 60d2403ba9..fc0be6b722 100644 --- a/packages/lint/package.json +++ b/packages/lint/package.json @@ -31,6 +31,7 @@ "@objectstack/sdui-parser": "workspace:*", "@objectstack/spec": "workspace:*", "ajv": "^8.20.0", + "ajv-formats": "^3.0.1", "sucrase": "^3.35.1", "typescript": "^6.0.3" }, diff --git a/packages/lint/src/lazy-deps.test.ts b/packages/lint/src/lazy-deps.test.ts index 97ad40dedc..c10c9699a4 100644 --- a/packages/lint/src/lazy-deps.test.ts +++ b/packages/lint/src/lazy-deps.test.ts @@ -13,7 +13,12 @@ // - `ajv` (~2.4 MB installed), loaded by the #4762 publish gate // (validate-rule-compilability.ts) only when a stack declares a // `json_schema` validation rule — the one rule type whose static artifact -// needs a JSON-Schema compiler to judge. +// needs a JSON-Schema compiler to judge; +// - `ajv-formats`, loaded by the same gate for the same trigger (#5029). Small +// on its own, but it `require`s `ajv/dist/compile/codegen`, so an eager +// import of it would drag ajv onto the boot path through the back door — +// i.e. it must be listed here even though its own weight would not justify +// it. // // "A react page" was the whole story when this file was written; it is not any // more, and the cases below say which trigger they are pinning. Keep them @@ -42,7 +47,7 @@ const distDir = join(srcDir, '..', 'dist'); // Deps that must never load at import time. Extend this list when another // heavy, rarely-hit dependency joins the package. -const LAZY_DEPS = ['typescript', 'sucrase', 'ajv']; +const LAZY_DEPS = ['typescript', 'sucrase', 'ajv', 'ajv-formats']; const depLoaded = (cache: Record | undefined, dep: string) => Object.keys(cache ?? {}).some((p) => p.split(/[/\\]/).join('/').includes(`/node_modules/${dep}/`)); @@ -105,15 +110,18 @@ describe('lazy dependency loading (kernel boot-path contract)', () => { const props = mod.validateReactPageProps(${reactStack('function Page(){ return ; }')}); if (!loaded('typescript')) fail('typescript was not loaded by a react-page props validation'); if (!props.some((f) => f.rule === 'react-prop-missing-required')) fail('props gate produced no finding'); - // #4762 — the JSON-Schema compiler is the third lazy dep. A stack whose - // validation rules are all format rules never pays for it. + // #4762 — the JSON-Schema compiler is the third lazy dep, and #5029 added + // its formats plugin as the fourth. A stack whose validation rules are all + // format rules never pays for either. const ruleStack = (validation) => ({ objects: [{ name: 'a', fields: { payload: {} }, validations: [validation] }] }); mod.validateRuleCompilability(ruleStack({ type: 'format', name: 'f', field: 'payload', regex: '([', message: 'm' })); if (loaded('ajv')) fail('the rule-compilability gate must not load ajv to judge a format rule'); + if (loaded('ajv-formats')) fail('the rule-compilability gate must not load ajv-formats to judge a format rule'); const schemaFindings = mod.validateRuleCompilability( ruleStack({ type: 'json_schema', name: 'j', field: 'payload', schema: { type: 'not-a-type' }, message: 'm' }), ); if (!loaded('ajv')) fail('ajv was not loaded by a json_schema validation-rule check'); + if (!loaded('ajv-formats')) fail('ajv-formats was not loaded by a json_schema validation-rule check (#5029 parity)'); if (!schemaFindings.some((f) => f.rule === 'validation-rule-json-schema-uncompilable')) { fail('rule-compilability gate produced no finding'); } @@ -194,11 +202,14 @@ describe('lazy dependency loading (kernel boot-path contract)', () => { expect(depLoaded(req.cache, dep), `${dep} loaded before any react-source or L2-body validation`).toBe(false); } - // The first `json_schema` rule pays for ajv — and the gate still works. + // The first `json_schema` rule pays for ajv AND its formats plugin (#5029 — + // the gate compiles in the runtime's environment, which now registers + // formats) — and the gate still works. const schemaFindings = validateRuleCompilability( ruleStack({ type: 'json_schema', name: 'j', field: 'payload', schema: { type: 'not-a-type' }, message: 'm' }), ); expect(depLoaded(req.cache, 'ajv')).toBe(true); + expect(depLoaded(req.cache, 'ajv-formats')).toBe(true); expect(schemaFindings.map((f) => f.rule)).toEqual(['validation-rule-json-schema-uncompilable']); // The first react page with source pays the cost of exactly its own gate's diff --git a/packages/lint/src/runtime-lazy-deps.test.ts b/packages/lint/src/runtime-lazy-deps.test.ts index 0f2f8fc4c7..2c2c128a38 100644 --- a/packages/lint/src/runtime-lazy-deps.test.ts +++ b/packages/lint/src/runtime-lazy-deps.test.ts @@ -35,7 +35,11 @@ const distDir = join(srcDir, '..', 'dist'); // is CLI-only (`surfaceReason: RUNTIME_OBJECT_WRITES_P2`). So the boot path must // not pay for it — not at import, and not while gating. Should that rule ever be // widened to `runtime-publish`, this assertion is what says so out loud. -const LAZY_DEPS = ['typescript', 'sucrase', 'ajv']; +// `ajv-formats` joined in #5029 for the same gate and the same trigger: the gate +// compiles in the runtime's ajv ENVIRONMENT, and that environment now registers +// the formats plugin. It carries ajv in with it, so listing it here is not +// belt-and-braces — it is the second door onto the same load. +const LAZY_DEPS = ['typescript', 'sucrase', 'ajv', 'ajv-formats']; const depLoaded = (cache: Record | undefined, dep: string) => Object.keys(cache ?? {}).some((p) => p.split(/[/\\]/).join('/').includes(`/node_modules/${dep}/`)); diff --git a/packages/lint/src/validate-rule-compilability.test.ts b/packages/lint/src/validate-rule-compilability.test.ts index 723a021ffe..3172f68c47 100644 --- a/packages/lint/src/validate-rule-compilability.test.ts +++ b/packages/lint/src/validate-rule-compilability.test.ts @@ -363,10 +363,114 @@ describe('validateRuleCompilability — parity with the write path (#4762)', () ]); expect(RUNTIME_AJV_OPTIONS).toEqual({ allErrors: true, strict: false }); + // …and the PLUGINS half (#5029). Options alone stopped describing the + // environment the day the runtime registered `ajv-formats`: the plugin adds + // the `format` implementations AND the `formatMinimum`/`formatMaximum` + // keywords, so two instances built from identical options can still disagree + // about whether a schema compiles. Read the registration out of the runtime's + // source for the same reason the options are read out of it. + expect( + source, + 'the runtime no longer registers ajv-formats — this gate must drop it too, or it starts disagreeing', + ).toContain('addFormats(ajv)'); + // The default (full) format set, not `fast` mode: a plugin call carrying an + // options object is a different environment and must be mirrored, not + // assumed. (`addFormats(ajv, {...})` would fail this.) + expect(source).toMatch(/addFormats\(ajv\)\s*;/); + // …and the regex half: the runtime compiles the raw source with no flags. expect(source).toContain('new RegExp(rule.regex)'); }); + it('compiles a `format`-bearing schema exactly as before — the gate publishes it (#5029)', () => { + // The #5029 change is a RUNTIME enforcement change, not an authoring + // restriction: `format: 'email'` is standard JSON Schema and stays + // publishable. Option 2 on that issue (refuse `format` at publish time) was + // rejected, and this is the pin that says so — if the gate ever starts + // rejecting these, it is rejecting legal metadata the write path enforces. + expect( + ids( + objectWith({ + type: 'json_schema', + name: 'support_config_shape', + field: 'support_config', + message: 'm', + schema: { + type: 'object', + properties: { + email: { type: 'string', format: 'email' }, + id: { type: 'string', format: 'uuid' }, + at: { type: 'string', format: 'date-time' }, + site: { type: 'string', format: 'uri' }, + }, + required: ['email'], + }, + }), + ), + ).toEqual([]); + }); + + it('a MISSPELLED format name is published, not refused — pinned as-is, not changed (#5029)', () => { + // Under `strict: false` ajv logs `unknown format "emial" ignored` and drops + // the keyword — in the runtime AND here. So the gate must publish it: a + // finding would be this gate inventing a verdict the write path does not + // share, which is the `strict: true` mistake wearing a different hat. This + // records the residual gap (a typo'd format still enforces nothing) as a + // KNOWN, deliberate boundary of #5029 rather than an oversight; closing it + // is an authoring-time decision of its own. + expect( + ids( + objectWith({ + type: 'json_schema', + name: 'typo_format', + field: 'support_config', + message: 'm', + schema: { type: 'object', properties: { email: { type: 'string', format: 'emial' } } }, + }), + ), + ).toEqual([]); + }); + + it('judges `formatMinimum` the way the runtime does — the keyword only EXISTS with ajv-formats (#5029)', () => { + // The case that makes the plugin half of the parity load-bearing rather + // than tidy. `ajv-formats` registers `formatMinimum`/`formatMaximum` with a + // metaschema; without the plugin they are unknown keywords, and + // `strict: false` ignores an unknown keyword silently. So a gate lacking the + // plugin publishes a malformed `formatMinimum` that the runtime then refuses + // to compile — the rule ships, is skipped for every record, and this file's + // whole reason for existing is defeated. A WELL-FORMED use stays green. + expect( + ids( + objectWith({ + type: 'json_schema', + name: 'window_ok', + field: 'support_config', + message: 'm', + schema: { + type: 'object', + properties: { from: { type: 'string', format: 'date', formatMinimum: '2020-01-01' } }, + }, + }), + ), + ).toEqual([]); + // …and the malformed one is refused, which is only possible because the + // keyword is registered. + expect( + ids( + objectWith({ + type: 'json_schema', + name: 'window_bad', + field: 'support_config', + message: 'm', + schema: { + type: 'object', + properties: { from: { type: 'string', format: 'date', formatMinimum: 42 } }, + }, + }), + ), + ).toEqual([VALIDATION_RULE_SCHEMA_UNCOMPILABLE]); + }); + it('rejects exactly the two artifacts the runtime still skips', () => { // The runtime's own fail-open pin (`rule-fail-closed.test.ts` › `#4649 — // unchanged neighbours`) uses these two fixtures to record that the WRITE diff --git a/packages/lint/src/validate-rule-compilability.ts b/packages/lint/src/validate-rule-compilability.ts index d8abdc64d3..bfe0e5c595 100644 --- a/packages/lint/src/validate-rule-compilability.ts +++ b/packages/lint/src/validate-rule-compilability.ts @@ -44,14 +44,28 @@ * - the regex is compiled with `new RegExp(source)`, the exact call * `checkFormat` makes (no flags, same as the runtime); * - the schema is compiled with ajv, constructed with the SAME options the - * runtime's shared instance uses — `{ allErrors: true, strict: false }`. + * runtime's shared instance uses — `{ allErrors: true, strict: false }` — + * and carrying the SAME plugins, which since #5029 means `ajv-formats`. * `strict: false` is load-bearing in both directions: it is what lets an * author-written schema carry vendor keywords, so a gate running `strict: - * true` would reject schemas the runtime compiles happily. + * true` would reject schemas the runtime compiles happily. `ajv-formats` is + * load-bearing in one direction and it is the dangerous one: the plugin also + * registers `formatMinimum` / `formatMaximum`, so a gate WITHOUT it treats + * those as unknown keywords (`strict: false` ⇒ ignored) and publishes a + * schema the runtime then refuses to compile — a rule that passes review and + * enforces nothing, which is the exact failure this file was written for. * - * Those options are pinned against `rule-validator.ts`'s own source by - * `validate-rule-compilability.test.ts`, so the day the runtime changes them - * this gate is told rather than left quietly disagreeing. + * Those options *and* that plugin registration are pinned against + * `rule-validator.ts`'s own source by `validate-rule-compilability.test.ts`, so + * the day the runtime changes either one this gate is told rather than left + * quietly disagreeing. + * + * What this gate deliberately does NOT judge is a MISSPELLED format name. + * `format: 'emial'` compiles in both environments — under `strict: false` ajv + * logs one line and drops the keyword — so refusing it here would be the gate + * inventing a verdict the runtime does not share, the mirror image of the + * `strict: true` mistake above. #5029 pins that behaviour rather than changing + * it; closing it is an authoring-time decision of its own. * * ### One ajv instance per schema, on purpose * @@ -115,7 +129,9 @@ import { createRequire } from 'node:module'; // metadata write), while this gate needs a JSON-Schema compiler only when a // stack actually declares a `json_schema` validation rule. Same lazy-load // contract as `typescript`/`sucrase`, and guarded the same way by -// `lazy-deps.test.ts`. +// `lazy-deps.test.ts`. `ajv-formats` (#5029) is under the same contract and for +// a sharper reason: it `require`s `ajv/dist/compile/codegen`, so importing it +// eagerly would drag ajv onto the boot path through the back door. import type { Options as AjvOptions } from 'ajv'; export type RuleCompilabilitySeverity = 'error'; @@ -167,8 +183,11 @@ function asArray(v: unknown): AnyRec[] { */ type AjvLike = { compile: (schema: unknown) => unknown }; type AjvCtor = new (options?: AjvOptions) => AjvLike; +/** `ajv-formats`' plugin entry — mutates the instance it is handed (#5029). */ +type AddFormats = (ajv: AjvLike) => unknown; let cachedAjv: AjvCtor | null = null; +let cachedAddFormats: AddFormats | null = null; /** * Load ajv on first use. `node:module` is a Node builtin untouched by @@ -203,6 +222,53 @@ function loadAjv(): AjvCtor { return ctor; } +/** + * Load `ajv-formats` on first use — same deferral, same reason, as {@link loadAjv} + * (#5029). Kept as its own loader rather than folded into that one so the + * failure message names the package the deployment actually pruned. + */ +function loadAddFormats(): AddFormats { + if (cachedAddFormats) return cachedAddFormats; + const anchor = + typeof import.meta !== 'undefined' && import.meta.url + ? import.meta.url + : typeof __filename !== 'undefined' + ? __filename + : process.cwd() + '/'; + let mod: unknown; + try { + mod = createRequire(anchor)('ajv-formats'); + } catch (err) { + throw new Error( + `@objectstack/lint: checking a \`json_schema\` validation rule requires the "ajv-formats" package, which ` + + `could not be loaded (${err instanceof Error ? err.message : String(err)}). It is a declared dependency ` + + `of @objectstack/lint — if this deployment prunes packages, keep "ajv-formats" in the image; it is only ` + + `loaded when a stack declares a \`json_schema\` validation rule. The runtime registers it too, and this ` + + `gate must compile in the SAME environment or it starts disagreeing with the write path.`, + ); + } + const plugin = (isRec(mod) && 'default' in mod ? (mod as AnyRec).default : mod) as AddFormats; + cachedAddFormats = plugin; + return plugin; +} + +/** + * A fresh ajv in the runtime's EXACT environment: the runtime's options, plus + * the `ajv-formats` plugin the runtime registers (#5029). + * + * Registering formats is not cosmetic parity. `ajv-formats` also installs the + * `formatMinimum` / `formatMaximum` keywords, which are *unknown keywords* + * without it — and `strict: false` ignores an unknown keyword while a + * registered one is metaschema-checked. So a schema the runtime would refuse to + * compile is one this gate would otherwise wave through, which is precisely the + * disagreement this whole file exists to prevent. + */ +function createRuntimeAjv(): AjvLike { + const instance = new (loadAjv())(RUNTIME_AJV_OPTIONS); + loadAddFormats()(instance); + return instance; +} + /** The message a thrown compile error contributes, verbatim. */ function errorText(err: unknown): string { return err instanceof Error ? err.message : String(err); @@ -298,7 +364,7 @@ export function validateRuleCompilability(stack: unknown): RuleCompilabilityFind if (rule.type === 'json_schema' && isRec(rule.schema)) { try { // A fresh instance per schema — see "One ajv instance per schema". - new (loadAjv())(RUNTIME_AJV_OPTIONS).compile(rule.schema); + createRuntimeAjv().compile(rule.schema); } catch (err) { findings.push({ severity: 'error', @@ -308,7 +374,7 @@ export function validateRuleCompilability(stack: unknown): RuleCompilabilityFind message: `\`json_schema\` validation ${label} on object '${objectName}' declares a \`schema\` ajv cannot ` + `compile: ${errorText(err)}. The write path compiles it with the same ajv ` + - `(\`new Ajv({ allErrors: true, strict: false })\`) and SKIPS the rule when that throws ` + + `(\`new Ajv({ allErrors: true, strict: false })\` + \`ajv-formats\`) and SKIPS the rule when that throws ` + `(rule-validator.ts \`checkJsonSchema\`), so the rule is declared and enforces nothing on any ` + `record.`, hint: diff --git a/packages/objectql/package.json b/packages/objectql/package.json index ad9b915f8b..ca0b5ddcca 100644 --- a/packages/objectql/package.json +++ b/packages/objectql/package.json @@ -30,6 +30,7 @@ "@objectstack/spec": "workspace:*", "@objectstack/types": "workspace:*", "ajv": "^8.20.0", + "ajv-formats": "^3.0.1", "zod": "^4.4.3" }, "devDependencies": { diff --git a/packages/objectql/src/validation/rule-validator.test.ts b/packages/objectql/src/validation/rule-validator.test.ts index 659dc218b0..84a3860520 100644 --- a/packages/objectql/src/validation/rule-validator.test.ts +++ b/packages/objectql/src/validation/rule-validator.test.ts @@ -902,6 +902,122 @@ describe('json_schema enforcement', () => { }); }); +// ── #5029 — the `format` keyword inside a `json_schema` rule ───────────── +// +// In ajv 8 `format` is NOT built in; it ships in `ajv-formats`. Without the +// plugin, and under the runtime's `strict: false`, an unknown format is not an +// error — ajv logs one line at COMPILE time and drops the keyword. So a rule +// declaring `format: 'email'` compiled fine, ran on every write, enforced +// `type`/`required`, and enforced NOTHING for `format`, for every record. +// +// That is the #4649 / #4762 family one level in, and nastier than either +// because the failure is PARTIAL: the rule visibly rejects bad `type` payloads +// in dev, so it reads as working while the `format` half never fires. These +// cases are the pin that it fires now. +describe('json_schema — `format` is actually enforced (#5029)', () => { + const withSchema = (schema: Record) => ({ + validations: [ + { type: 'json_schema' as const, name: 'cfg', message: 'Config does not match schema.', field: 'config', schema }, + ], + }); + + const EMAIL = withSchema({ + type: 'object', + properties: { email: { type: 'string', format: 'email' } }, + required: ['email'], + }); + + it('rejects a value that violates `format: email`', () => { + // The issue's exact repro. Before the plugin was registered this write + // sailed through — `type: 'string'` and `required` both held. + expect(() => evaluateValidationRules(EMAIL, { config: { email: 'not-an-email' } }, 'insert')).toThrow( + ValidationError, + ); + }); + + it('accepts a valid address, and still speaks the json_schema violation code', () => { + expect(() => evaluateValidationRules(EMAIL, { config: { email: 'ops@objectstack.ai' } }, 'insert')).not.toThrow(); + try { + evaluateValidationRules(EMAIL, { config: { email: 'not-an-email' } }, 'insert'); + throw new Error('expected throw'); + } catch (e) { + // A format failure is a schema violation like any other — it must not + // invent a new error code or a new field. + expect((e as ValidationError).fields[0].code).toBe('json_schema_violation'); + expect((e as ValidationError).fields[0].field).toBe('config'); + } + }); + + it('enforces the other formats authors actually reach for — uuid, date-time, uri', () => { + // The DEFAULT (full) `ajv-formats` set, deliberately: `fast` mode trades + // correctness for speed on exactly these, and a format that "mostly" + // matches is the same declared ≠ enforced defect with a smaller hole. + const rich = withSchema({ + type: 'object', + properties: { + id: { type: 'string', format: 'uuid' }, + at: { type: 'string', format: 'date-time' }, + site: { type: 'string', format: 'uri' }, + }, + }); + const ok = { + id: '123e4567-e89b-12d3-a456-426614174000', + at: '2026-08-04T10:00:00Z', + site: 'https://objectstack.ai/docs', + }; + expect(() => evaluateValidationRules(rich, { config: ok }, 'insert')).not.toThrow(); + for (const bad of [{ id: 'nope' }, { at: 'yesterday' }, { site: 'not a uri' }]) { + expect(() => evaluateValidationRules(rich, { config: { ...ok, ...bad } }, 'insert'), JSON.stringify(bad)).toThrow( + ValidationError, + ); + } + }); + + it('enforces a format nested behind $ref, and inside an array item', () => { + // The keyword is registered on the shared instance, so it reaches every + // sub-schema — not merely a top-level property. Worth pinning: a plugin + // registered per-compile-call would pass the flat case and fail this one. + const nested = withSchema({ + $defs: { contact: { type: 'object', properties: { email: { type: 'string', format: 'email' } }, required: ['email'] } }, + type: 'object', + properties: { contacts: { type: 'array', items: { $ref: '#/$defs/contact' } } }, + }); + expect(() => + evaluateValidationRules(nested, { config: { contacts: [{ email: 'a@b.com' }] } }, 'insert'), + ).not.toThrow(); + expect(() => + evaluateValidationRules(nested, { config: { contacts: [{ email: 'a@b.com' }, { email: 'nope' }] } }, 'insert'), + ).toThrow(ValidationError); + }); + + it('enforces `format` through a JSON STRING value too', () => { + // `checkJsonSchema` parses a string field before validating, so the string + // path must not be a way round the new enforcement. + expect(() => evaluateValidationRules(EMAIL, { config: '{"email":"a@b.com"}' }, 'insert')).not.toThrow(); + expect(() => evaluateValidationRules(EMAIL, { config: '{"email":"nope"}' }, 'insert')).toThrow(ValidationError); + }); + + it('DOCUMENTS the residual gap: a misspelled format name is still ignored', () => { + // Not the behaviour we want; it is the behaviour we have, and #5029 + // deliberately did not change it. `strict: false` is what lets an + // author-written schema carry vendor keywords (the reason it is set), and + // the same setting downgrades an unrecognised format to a logged line. So + // `format: 'emial'` compiles and enforces nothing — filed separately as an + // authoring-time question. Pinned here so the gap is a RECORD, not a + // surprise, and so flipping it is a deliberate act that turns this red. + const typo = withSchema({ type: 'object', properties: { email: { type: 'string', format: 'emial' } } }); + expect(() => evaluateValidationRules(typo, { config: { email: 'not-an-email' } }, 'insert')).not.toThrow(); + // …while the neighbouring keywords in the very same schema still bite, which + // is exactly what made the pre-#5029 defect read as "working". + const mixed = withSchema({ + type: 'object', + properties: { email: { type: 'string', format: 'emial' } }, + required: ['email'], + }); + expect(() => evaluateValidationRules(mixed, { config: {} }, 'insert')).toThrow(ValidationError); + }); +}); + describe('conditional enforcement', () => { const schema = { validations: [ diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index 8f21bfadef..54c3fdbfb5 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -28,7 +28,9 @@ * (`email` / `url` / `phone` / `json`). Only runs when the write touches * the field and the value is non-empty (emptiness is the field-shape * validator's job, not the format rule's). - * - `json_schema` — a JSON field validated against a JSON Schema via ajv. + * - `json_schema` — a JSON field validated against a JSON Schema via ajv, + * with `ajv-formats` registered so the standard `format` keyword (`email`, + * `uri`, `uuid`, `date-time`, …) is actually enforced (#5029). * - `conditional` — evaluates the `when` CEL predicate and then recurses into * `then` (true) or `otherwise` (false). The nested rule's violation message * is surfaced; the *outer* conditional's `severity` decides whether it @@ -137,6 +139,9 @@ import { ExpressionEngine, collectCelRootIdentifiers } from '@objectstack/formul import type { Expression } from '@objectstack/spec'; import { AUDIT_PROVENANCE_FIELDS } from '@objectstack/spec/data'; import Ajv, { type ValidateFunction } from 'ajv'; +// #5029 — `format` is NOT built into ajv 8; it ships in this separate package. +// See the `const ajv` note below for why the runtime registers it. +import addFormats from 'ajv-formats'; import { ValidationError, buildFieldError, @@ -224,8 +229,42 @@ interface RuleContext { * Shared ajv instance. `strict: false` tolerates author-written JSON Schemas * that use vendor keywords; `compile` results are memoised per schema object * (see `jsonSchemaCache`) so we don't recompile on every write. + * + * ## Why `addFormats` (#5029) + * + * In ajv 8 the `format` keyword is **not built in** — it lives in the separate + * `ajv-formats` package. Without it, and under `strict: false`, an unknown + * format is not an error: ajv logs one line at compile time and **ignores the + * keyword**. So a rule declaring + * `{ email: { type: 'string', format: 'email' } }` compiled fine, ran on every + * write, enforced `type` and `required` — and enforced *nothing* for `format`, + * for every record, forever. The failure was PARTIAL, which is what made it + * nastier than an uncompilable schema (#4762): the rule visibly rejects bad + * `type` / `required` payloads in dev, so it reads as working while the + * `format` half never fires. Same #4649 family, one level in: declared ≠ + * enforced with only a stderr line — naming no rule and no object — as signal. + * + * Registering the plugin makes the declaration true. The **default (full)** + * format set is used deliberately: `fast` mode trades correctness for speed on + * exactly the formats authors reach for most (`email`, `uri`, `date-time`), and + * a format that "mostly" matches is the same declared ≠ enforced defect with a + * smaller hole. Cost of doing this at all: it is a behaviour change on deployed + * data — records that passed while `format` was inert can now be rejected at + * write time. That is the point, and the changeset says so loudly. + * + * `format` remains *tolerant of a typo* by construction: under `strict: false` + * an unrecognised format name (`'emial'`) is still logged-and-ignored rather + * than rejected. Registering the standard set does not change that, and this + * PR deliberately does not either — flipping it is an authoring-time question + * for the publish gate, filed separately. + * + * The #4762 publish gate (`@objectstack/lint`'s + * `validate-rule-compilability.ts`) compiles with the SAME ajv environment on + * purpose, so it registers the same plugin; its parity test reads this file's + * source and goes red if these two lines drift apart. */ const ajv = new Ajv({ allErrors: true, strict: false }); +addFormats(ajv); const jsonSchemaCache = new WeakMap(); export interface EvaluateRulesOptions { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 54117488c0..ed230b3863 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -835,6 +835,9 @@ importers: ajv: specifier: ^8.20.0 version: 8.20.0 + ajv-formats: + specifier: ^3.0.1 + version: 3.0.1(ajv@8.20.0) sucrase: specifier: ^3.35.1 version: 3.35.1 @@ -1039,6 +1042,9 @@ importers: ajv: specifier: ^8.20.0 version: 8.20.0 + ajv-formats: + specifier: ^3.0.1 + version: 3.0.1(ajv@8.20.0) zod: specifier: ^4.4.3 version: 4.4.3