diff --git a/.changeset/rule-compilability-publish-gate.md b/.changeset/rule-compilability-publish-gate.md new file mode 100644 index 0000000000..9fba474eff --- /dev/null +++ b/.changeset/rule-compilability-publish-gate.md @@ -0,0 +1,60 @@ +--- +"@objectstack/lint": patch +--- + +fix(lint): reject a validation rule whose regex or JSON Schema does not compile, at authoring time (#4762) + +Two of the six object validation-rule types carry a **static artifact** that the +write path hands to a real compiler, inside a `try/catch` that logs and returns +`null`: + +- `format` → `new RegExp(rule.regex)` → *"Validation rule '…' has an invalid regex — skipped"* +- `json_schema` → `ajv.compile(rule.schema)` → *"Validation rule '…' has an uncompilable JSON Schema — skipped"* + +"Skipped" means the rule is declared, appears in the metadata, appears in every +"what protects this object" listing — and enforces nothing, on every record, for +as long as the metadata is deployed, with a WARN line in a log nobody reads as +the only signal. That is the shape #4649 was filed about one rule type over; +#4761 flipped the CEL predicates to fail closed and deliberately left these two, +because their blast radius differs (see below). + +**New gate — `validateRuleCompilability`**, a `gating` entry in +`AUTHORING_RULES`, so it runs on all three authoring commands (`os validate`, +`os build`, `os lint`) with no per-command wiring. Two rule ids: + +| id | fires when | +|:---|:---| +| `validation-rule-regex-uncompilable` | a `format` rule's `regex` throws in `new RegExp(...)` | +| `validation-rule-json-schema-uncompilable` | a `json_schema` rule's `schema` throws in `ajv.compile(...)` | + +Each finding names the rule, the object and the config path, and carries the +**compiler's own error text verbatim** — an author cannot act on "invalid +regex", but can act on `Invalid regular expression: /([/: Unterminated character +class`. Rules nested in a `conditional`'s `then` / `otherwise` are judged too +(`evaluateRule` recurses into them and reaches the very same checkers), and the +finding names the branch it is in. + +**Detection is the real compilers, never a pattern that judges a pattern.** The +regex is compiled with `new RegExp(source)` — the exact call `checkFormat` +makes. The schema is compiled with ajv constructed with the **same options the +runtime's shared instance uses** (`{ allErrors: true, strict: false }`), read +back out of `rule-validator.ts`'s source by a parity test so the day those +options change, this gate is told rather than left quietly disagreeing. +`strict: false` is load-bearing in both directions: a gate running `strict: true` +would reject author-written schemas carrying vendor keywords that the write path +compiles happily — a gate that turns working metadata red gets switched off, and +then protects nothing. + +`ajv` is a new dependency of `@objectstack/lint`, loaded **lazily**: only a +stack that actually declares a `json_schema` validation rule pays for it, pinned +by the package's `lazy-deps.test.ts` alongside `typescript` and `sucrase`. The +kernel boot path (`@objectstack/lint/runtime`) never loads it at all. + +**The runtime half is deliberately unchanged.** `rule-validator.ts` still fails +open on both, and the `#4649 — unchanged neighbours` pins that record it stand +exactly as they are. A broken regex or schema is *static* — decidable from the +metadata alone, with no record in hand — so the authoring door closes the class +outright without ever bricking a running deployment, whereas rejecting at write +time would reject **every** write touching that field for as long as the bad +metadata is deployed. Whether a runtime backstop is still wanted on top of a +closed authoring door stays open on #4762. diff --git a/packages/lint/package.json b/packages/lint/package.json index 191db40e0a..e8666dc479 100644 --- a/packages/lint/package.json +++ b/packages/lint/package.json @@ -29,6 +29,7 @@ "@objectstack/formula": "workspace:*", "@objectstack/sdui-parser": "workspace:*", "@objectstack/spec": "workspace:*", + "ajv": "^8.20.0", "sucrase": "^3.35.1", "typescript": "^6.0.3" }, diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index 7e740c6846..177b4634e0 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -122,6 +122,7 @@ import { validateSecurityPosture } from './validate-security-posture.js'; import { validateOrgAxisRedLines } from './validate-org-axis-red-lines.js'; import { validateSharingRuleEnforceability } from './validate-sharing-rule-enforceability.js'; import { validateRlsPredicateEnforceability } from './validate-rls-predicate-enforceability.js'; +import { validateRuleCompilability } from './validate-rule-compilability.js'; import { validateActionLocations } from './validate-action-locations.js'; import { lintFlowPatterns } from './lint-flow-patterns.js'; import { lintLivenessProperties } from './lint-liveness-properties.js'; @@ -846,6 +847,27 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ + 'never run at a door should not claim it.', run: (stack) => validateRlsPredicateEnforceability(stack), }, + // #4762 — the same "declared but enforces nothing" question, for the two + // STATIC artifacts an object validation rule carries. A `format` rule's + // `regex` that `new RegExp(...)` throws on, and a `json_schema` rule's schema + // ajv cannot compile, are both logged and SKIPPED on the write path + // (`rule-validator.ts`), so the rule ships, lists, and protects nothing. + // Neither needs a record to judge, so the authoring door is the right one: + // rejecting a broken regex at RUNTIME instead would reject every write + // touching that field for as long as the metadata is deployed (#4762's own + // analysis — the runtime-backstop question stays open for the maintainer). + // Gating for the `lint-flow-patterns.ts` bar: no reading of the metadata + // behaves as written, because the rule does not run at all. + { + name: 'validateRuleCompilability', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-rule-compilability.ts', + surfaces: CLI_ONLY, + surfaceReason: RUNTIME_OBJECT_WRITES_P2, + run: (stack) => validateRuleCompilability(stack), + }, ]; // ─── Runner ───────────────────────────────────────────────────────── diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 9b1670b14c..acde454239 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -281,6 +281,23 @@ export { } from './validate-chart-bindings.js'; export type { ChartBindingFinding, ChartBindingSeverity } from './validate-chart-bindings.js'; +// #4762 — the two STATIC artifacts an object validation rule carries (a +// `format` rule's `regex`, a `json_schema` rule's `schema`) are fail-OPEN at +// runtime: one that does not compile is logged and skipped, so the rule is +// declared and enforces nothing. Both are decidable without a record, so they +// are rejected at authoring/publish time — with the REAL compilers, and for ajv +// with the runtime's own options. +export { + validateRuleCompilability, + RUNTIME_AJV_OPTIONS, + VALIDATION_RULE_REGEX_UNCOMPILABLE, + VALIDATION_RULE_SCHEMA_UNCOMPILABLE, +} from './validate-rule-compilability.js'; +export type { + RuleCompilabilityFinding, + RuleCompilabilitySeverity, +} from './validate-rule-compilability.js'; + export { validateNavAccess, NAV_OBJECT_UNGRANTED } from './validate-nav-access.js'; export type { NavAccessFinding, NavAccessSeverity } from './validate-nav-access.js'; diff --git a/packages/lint/src/lazy-deps.test.ts b/packages/lint/src/lazy-deps.test.ts index b39d1997ae..97ad40dedc 100644 --- a/packages/lint/src/lazy-deps.test.ts +++ b/packages/lint/src/lazy-deps.test.ts @@ -9,7 +9,11 @@ // by the L2 body write-set gates (validate-hook-body-writes.ts since // #4271, validate-action-body-writes.ts since #4345); // - `sucrase` (~1.5 MB), loaded by the react syntax gate -// (validate-react-pages.ts). +// (validate-react-pages.ts); +// - `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. // // "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 @@ -38,7 +42,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']; +const LAZY_DEPS = ['typescript', 'sucrase', 'ajv']; const depLoaded = (cache: Record | undefined, dep: string) => Object.keys(cache ?? {}).some((p) => p.split(/[/\\]/).join('/').includes(`/node_modules/${dep}/`)); @@ -101,6 +105,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. + 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'); + 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 (!schemaFindings.some((f) => f.rule === 'validation-rule-json-schema-uncompilable')) { + fail('rule-compilability gate produced no finding'); + } console.log('OK'); }; `; @@ -132,8 +148,13 @@ describe('lazy dependency loading (kernel boot-path contract)', () => { it('loads each dep lazily in-process and the gates still work', async () => { const req = createRequire(import.meta.url); - const { validateReactPages, validateReactPageProps, validateHookBodyWrites, validateActionBodyWrites } = - await import('./index.js'); + const { + validateReactPages, + validateReactPageProps, + validateHookBodyWrites, + validateActionBodyWrites, + validateRuleCompilability, + } = await import('./index.js'); // Stacks without a react-source page never touch either dep. expect(validateReactPages({ pages: [{ name: 'p', kind: 'object' }] })).toEqual([]); @@ -157,10 +178,29 @@ describe('lazy dependency loading (kernel boot-path contract)', () => { actions: [action({ language: 'js', source: 'ctx.input.amout = 1; return { ok: true };' })], }), ).toEqual([]); + // #4762 — nor does judging a `format` rule's regex: that needs only the + // JavaScript engine's own `new RegExp`, so a stack with no `json_schema` + // validation rule never loads a JSON-Schema compiler. + const ruleStack = (validation: unknown) => ({ + objects: [{ name: 'a', fields: { payload: {} }, validations: [validation] }], + }); + expect( + validateRuleCompilability( + ruleStack({ type: 'format', name: 'f', field: 'payload', regex: '([', message: 'm' }), + ).map((f) => f.rule), + ).toEqual(['validation-rule-regex-uncompilable']); + for (const dep of LAZY_DEPS) { 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. + 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(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 // dep — and the gates still work. const syntax = validateReactPages({ diff --git a/packages/lint/src/runtime-lazy-deps.test.ts b/packages/lint/src/runtime-lazy-deps.test.ts index bcaa407b63..0f2f8fc4c7 100644 --- a/packages/lint/src/runtime-lazy-deps.test.ts +++ b/packages/lint/src/runtime-lazy-deps.test.ts @@ -2,8 +2,8 @@ // // The kernel boot-path contract for `@objectstack/lint/runtime` (#4463). // -// `lazy-deps.test.ts` next door pins that IMPORTING the package loads neither -// `typescript` (~9 MB) nor `sucrase`. That was enough while the only consumer +// `lazy-deps.test.ts` next door pins that IMPORTING the package loads none of +// `typescript` (~9 MB), `sucrase` or `ajv`. That was enough while the only consumer // was the CLI, which may load anything. #4463 gave the package a consumer on // the kernel boot path — `@objectstack/metadata-protocol`, reached by every // runtime metadata write — and that consumer needs the stronger claim: @@ -30,7 +30,12 @@ import { describe, it, expect } from 'vitest'; const srcDir = dirname(fileURLToPath(import.meta.url)); const distDir = join(srcDir, '..', 'dist'); -const LAZY_DEPS = ['typescript', 'sucrase']; +// `ajv` joined the list in #4762: the rule-compilability gate needs a real +// JSON-Schema compiler to judge a `json_schema` validation rule, and that rule +// 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']; 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 new file mode 100644 index 0000000000..7e52735267 --- /dev/null +++ b/packages/lint/src/validate-rule-compilability.test.ts @@ -0,0 +1,426 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #4762 — a `format` rule whose `regex` does not compile, and a `json_schema` +// rule whose schema ajv cannot compile, are fail-OPEN on the write path: logged +// once and skipped, so the rule is declared, listed, and enforces nothing on +// every record. Both are decidable from the metadata alone, so they are +// rejected here, at authoring/publish time. +// +// The two halves this file has to prove, because either alone is worthless: +// +// 1. the broken artifacts go RED, naming the rule, the object and the +// compiler's own error text (an author cannot fix "invalid regex"); +// 2. rich, legitimate artifacts stay GREEN — including a schema carrying +// vendor keywords, which is the case a gate running ajv `strict: true` +// would reject while the runtime compiles it happily. A gate that turns +// working metadata red gets switched off, and then protects nothing. + +import { existsSync, readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, it, expect } from 'vitest'; + +import { + validateRuleCompilability, + MAX_RULE_NESTING_DEPTH, + RUNTIME_AJV_OPTIONS, + VALIDATION_RULE_REGEX_UNCOMPILABLE, + VALIDATION_RULE_SCHEMA_UNCOMPILABLE, +} from './validate-rule-compilability.js'; +import { AUTHORING_COMMANDS, AUTHORING_RULES, authoringRulesFor, runAuthoringRules } from './authoring-rules.js'; + +const repoRoot = join(dirname(fileURLToPath(import.meta.url)), '..', '..', '..'); +const RUNTIME_VALIDATOR = 'packages/objectql/src/validation/rule-validator.ts'; + +/** One object carrying the given validation rules. */ +const objectWith = (...validations: unknown[]) => ({ + objects: [ + { + name: 'account', + label: 'Account', + fields: { tax_id: { type: 'text' }, support_config: { type: 'json' } }, + validations, + }, + ], +}); + +const ids = (stack: unknown) => validateRuleCompilability(stack).map((f) => f.rule); + +// ── Red: declared, schema-valid, and enforcing nothing ─────────────── + +describe('validateRuleCompilability — the fail-open artifacts go RED', () => { + it('rejects a `format` rule whose regex does not compile, with the regex error verbatim', () => { + const findings = validateRuleCompilability( + objectWith({ + type: 'format', + name: 'tax_id_format', + field: 'tax_id', + regex: '([', + message: 'Tax ID must look like 12-3456789.', + }), + ); + + expect(findings).toHaveLength(1); + const [f] = findings; + expect(f.severity).toBe('error'); + expect(f.rule).toBe(VALIDATION_RULE_REGEX_UNCOMPILABLE); + // Named: which rule, on which object, and where in the config. + expect(f.where).toBe("object 'account' · validation 'tax_id_format'"); + expect(f.path).toBe('objects.account.validations.tax_id_format.regex'); + expect(f.message).toContain("'tax_id_format'"); + expect(f.message).toContain("object 'account'"); + // The compiler's own text, not a paraphrase — this is what an author acts on. + let native = ''; + try { + new RegExp('(['); + } catch (err) { + native = (err as Error).message; + } + expect(native).not.toBe(''); + expect(f.message).toContain(native); + // And the consequence is stated, not implied. + expect(f.message).toContain('enforces nothing'); + expect(f.hint).toMatch(/escaped|format/); + }); + + it('rejects a `json_schema` rule ajv cannot compile, with ajv’s error verbatim', () => { + const findings = validateRuleCompilability( + objectWith({ + type: 'json_schema', + name: 'support_config_shape', + field: 'support_config', + schema: { type: 'not-a-type' }, + message: 'Support config must be an object.', + }), + ); + + expect(findings).toHaveLength(1); + const [f] = findings; + expect(f.severity).toBe('error'); + expect(f.rule).toBe(VALIDATION_RULE_SCHEMA_UNCOMPILABLE); + expect(f.where).toBe("object 'account' · validation 'support_config_shape'"); + expect(f.path).toBe('objects.account.validations.support_config_shape.schema'); + expect(f.message).toContain("'support_config_shape'"); + expect(f.message).toContain("object 'account'"); + // ajv's own wording, whatever this ajv version words it as. + expect(f.message).toMatch(/schema is invalid/i); + expect(f.message).toContain('enforces nothing'); + }); + + it('finds a broken artifact nested in a `conditional` branch, and names the branch', () => { + // `evaluateRule` recurses into `then`/`otherwise`, so a nested `format` rule + // reaches the very same fail-open `checkFormat`. A gate that only walked the + // top level would leave the nested half exactly as unprotected as before. + const findings = validateRuleCompilability( + objectWith({ + type: 'conditional', + name: 'churn_reason_consistency', + when: "record.status == 'churned'", + message: 'Churn reason consistency.', + then: { + type: 'format', + name: 'churn_code_shape', + field: 'tax_id', + regex: 'a{2,1}', + message: 'Bad churn code.', + }, + otherwise: { + type: 'json_schema', + name: 'config_shape', + field: 'support_config', + schema: { required: 'tier' }, + message: 'Bad config.', + }, + }), + ); + + expect(findings.map((f) => f.rule).sort()).toEqual( + [VALIDATION_RULE_REGEX_UNCOMPILABLE, VALIDATION_RULE_SCHEMA_UNCOMPILABLE].sort(), + ); + const regexFinding = findings.find((f) => f.rule === VALIDATION_RULE_REGEX_UNCOMPILABLE)!; + expect(regexFinding.where).toBe( + "object 'account' · validation 'churn_reason_consistency' → 'churn_code_shape'", + ); + expect(regexFinding.path).toBe( + 'objects.account.validations.churn_reason_consistency.then.churn_code_shape.regex', + ); + const schemaFinding = findings.find((f) => f.rule === VALIDATION_RULE_SCHEMA_UNCOMPILABLE)!; + expect(schemaFinding.path).toBe( + 'objects.account.validations.churn_reason_consistency.otherwise.config_shape.schema', + ); + }); + + it('reports every broken rule in one run rather than stopping at the first', () => { + expect( + ids( + objectWith( + { type: 'format', name: 'a', field: 'tax_id', regex: '(', message: 'm' }, + { type: 'format', name: 'b', field: 'tax_id', regex: '[z-a]', message: 'm' }, + { type: 'json_schema', name: 'c', field: 'support_config', schema: { type: 1 }, message: 'm' }, + ), + ), + ).toEqual([ + VALIDATION_RULE_REGEX_UNCOMPILABLE, + VALIDATION_RULE_REGEX_UNCOMPILABLE, + VALIDATION_RULE_SCHEMA_UNCOMPILABLE, + ]); + }); +}); + +// ── Green: rich, legitimate metadata is untouched ──────────────────── + +describe('validateRuleCompilability — rich but VALID artifacts stay green', () => { + it('accepts a demanding regex (lookahead, unicode escapes, backreference, nested groups)', () => { + expect( + ids( + objectWith( + // The showcase's own EIN rule — a doubled backslash in TS source is a + // single one in the compiled pattern. + { type: 'format', name: 'ein', field: 'tax_id', regex: '^\\d{2}-\\d{7}$', message: 'm' }, + { + type: 'format', + name: 'strong_code', + field: 'tax_id', + regex: '^(?=.*[A-Z])(?=.*\\d)(?!.*\\s)[A-Za-z\\d._%+-]{8,64}$', + message: 'm', + }, + { type: 'format', name: 'repeat', field: 'tax_id', regex: '^(\\w+)-\\1$', message: 'm' }, + { type: 'format', name: 'uni', field: 'tax_id', regex: '^[\\u4e00-\\u9fa5]{2,10}$', message: 'm' }, + // The named-format branch carries no regex at all. + { type: 'format', name: 'named_only', field: 'tax_id', format: 'email', message: 'm' }, + ), + ), + ).toEqual([]); + }); + + it('accepts a rich JSON Schema — $defs/$ref, nested arrays, conditionals', () => { + expect( + ids( + objectWith({ + type: 'json_schema', + name: 'support_config_shape', + field: 'support_config', + message: 'm', + schema: { + $defs: { + contact: { + type: 'object', + properties: { email: { type: 'string', format: 'email' }, phone: { type: 'string' } }, + required: ['email'], + additionalProperties: false, + }, + }, + type: 'object', + properties: { + tier: { type: 'string', enum: ['standard', 'premium', 'enterprise'] }, + seats: { type: 'integer', minimum: 1 }, + contacts: { type: 'array', items: { $ref: '#/$defs/contact' }, minItems: 1 }, + window: { + type: 'object', + properties: { from: { type: 'string' }, to: { type: 'string' } }, + dependentRequired: { from: ['to'] }, + }, + }, + required: ['tier'], + additionalProperties: false, + allOf: [{ if: { properties: { tier: { const: 'enterprise' } } }, then: { required: ['contacts'] } }], + }, + }), + ), + ).toEqual([]); + }); + + it('accepts vendor keywords — the runtime runs ajv `strict: false`, so this gate must too', () => { + // The parity case with teeth: under `strict: true` ajv REJECTS an unknown + // keyword, so a gate that quietly chose stricter options than the runtime + // would fail metadata the write path validates fine — a new declared ≠ + // enforced gap one level up, in the gate itself. + expect( + ids( + objectWith({ + type: 'json_schema', + name: 'vendor_keywords', + field: 'support_config', + message: 'm', + schema: { + type: 'object', + properties: { tier: { type: 'string', 'x-ui-widget': 'segmented' } }, + 'x-objectstack-hint': 'rendered by the console', + }, + }), + ), + ).toEqual([]); + }); + + it('judges nothing but `format`/`json_schema` artifacts', () => { + expect( + ids( + objectWith( + { + type: 'state_machine', + name: 'lifecycle', + field: 'status', + transitions: { prospect: ['active'] }, + message: 'm', + }, + { type: 'script', name: 'positive', condition: 'record.seats < 0', message: 'm' }, + { type: 'cross_field', name: 'dates', condition: 'record.a > record.b', fields: ['a'], message: 'm' }, + ), + ), + ).toEqual([]); + }); +}); + +// ── Shape tolerance: both authored collection shapes, and junk ─────── + +describe('validateRuleCompilability — walks what authors actually write', () => { + it('handles the name-keyed object map shape as well as the array shape', () => { + const findings = validateRuleCompilability({ + objects: { + account: { + fields: { tax_id: { type: 'text' } }, + validations: [{ type: 'format', name: 'ein', field: 'tax_id', regex: '([', message: 'm' }], + }, + }, + }); + expect(findings.map((f) => f.where)).toEqual(["object 'account' · validation 'ein'"]); + }); + + it('reads `validationRules` too — the same list `validate-expressions.ts` reads', () => { + expect( + ids({ + objects: [ + { + name: 'account', + validationRules: [{ type: 'format', name: 'ein', field: 'tax_id', regex: '([', message: 'm' }], + }, + ], + }), + ).toEqual([VALIDATION_RULE_REGEX_UNCOMPILABLE]); + }); + + it('terminates on a self-referential `conditional` — `os lint` never parses', () => { + // The pre-parse stack is whatever the author's own module built, so a + // self-referential rule is a two-line accident rather than a hypothetical. + // Same promise `flow-walk.ts`'s MAX_REGION_DEPTH makes: no lint may hang. + const cyclic: Record = { + type: 'conditional', + name: 'loop', + when: 'true', + message: 'm', + }; + cyclic.then = cyclic; + + const findings = validateRuleCompilability(objectWith(cyclic)); + expect(findings).toEqual([]); + + // Non-vacuity: the walk really does descend, and really does stop. A broken + // regex parked at the bottom of a legal nest is still reported… + const nest = (depth: number): Record => + depth === 0 + ? { type: 'format', name: `leaf`, field: 'tax_id', regex: '([', message: 'm' } + : { type: 'conditional', name: `c${depth}`, when: 'true', message: 'm', then: nest(depth - 1) }; + + expect(ids(objectWith(nest(MAX_RULE_NESTING_DEPTH)))).toEqual([VALIDATION_RULE_REGEX_UNCOMPILABLE]); + // …and one level past the cap is where the walker stops looking. + expect(ids(objectWith(nest(MAX_RULE_NESTING_DEPTH + 1)))).toEqual([]); + }); + + it('never throws on a stack that is missing, malformed or empty', () => { + for (const stack of [undefined, null, 'nonsense', 42, {}, { objects: null }, { objects: [null, 7] }]) { + expect(validateRuleCompilability(stack)).toEqual([]); + } + expect(ids(objectWith(null, 'nonsense', { type: 'format', name: 'x', field: 'tax_id' }))).toEqual([]); + // An empty regex is not a broken one — `checkFormat` only compiles a truthy + // `rule.regex`, so an empty string is skipped by the runtime as well. + expect(ids(objectWith({ type: 'format', name: 'x', field: 'tax_id', regex: '', message: 'm' }))).toEqual([]); + }); +}); + +// ── The verdict cannot drift from the runtime's ────────────────────── + +describe('validateRuleCompilability — parity with the write path (#4762)', () => { + it('compiles JSON Schemas with the SAME ajv options the runtime constructs', () => { + // A gate that compiles with different options can pass what the runtime + // rejects (or reject what it accepts), which is a fresh declared ≠ enforced + // gap one level up. Read from the runtime's source so the day someone + // changes those options, THIS goes red instead of the two silently + // disagreeing. + const path = join(repoRoot, RUNTIME_VALIDATOR); + expect(existsSync(path), `${RUNTIME_VALIDATOR} must exist — it IS the surface this gate mirrors`).toBe(true); + const source = readFileSync(path, 'utf8'); + + const match = source.match(/new Ajv\(\s*\{([^}]*)\}\s*\)/); + expect(match, `${RUNTIME_VALIDATOR} no longer constructs \`new Ajv({ … })\``).not.toBeNull(); + const pairs = match![1] + .split(',') + .map((s) => s.trim()) + .filter(Boolean) + .sort(); + expect(pairs, 'the runtime ajv options changed — mirror them in RUNTIME_AJV_OPTIONS').toEqual([ + 'allErrors: true', + 'strict: false', + ]); + expect(RUNTIME_AJV_OPTIONS).toEqual({ allErrors: true, strict: false }); + + // …and the regex half: the runtime compiles the raw source with no flags. + expect(source).toContain('new RegExp(rule.regex)'); + }); + + 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 + // path still waves them through. #4762's ruling leaves that pin standing and + // closes the authoring door instead — so the same two fixtures must be + // exactly what this gate refuses to publish. + expect(ids(objectWith({ type: 'format', name: 'fmt', field: 'tax_id', regex: '([', message: 'bad' }))).toEqual([ + VALIDATION_RULE_REGEX_UNCOMPILABLE, + ]); + expect( + ids( + objectWith({ + type: 'json_schema', + name: 'js', + field: 'support_config', + schema: { type: 'not-a-type' }, + message: 'bad', + }), + ), + ).toEqual([VALIDATION_RULE_SCHEMA_UNCOMPILABLE]); + }); +}); + +// ── Wiring: the gate an author actually meets ──────────────────────── + +describe('validateRuleCompilability — registry wiring', () => { + it('is a gating registry rule on all three authoring commands', () => { + const entry = AUTHORING_RULES.find((r) => r.name === 'validateRuleCompilability'); + expect(entry, 'validateRuleCompilability must be registered in AUTHORING_RULES').toBeDefined(); + expect(entry!.tier).toBe('gating'); + expect([...entry!.commands].sort()).toEqual([...AUTHORING_COMMANDS].sort()); + for (const command of AUTHORING_COMMANDS) { + expect( + authoringRulesFor(command).map((r) => r.name), + `os ${command} must run validateRuleCompilability`, + ).toContain('validateRuleCompilability'); + } + }); + + it('reaches the author through runAuthoringRules on every command', () => { + const normalized = objectWith({ + type: 'format', + name: 'tax_id_format', + field: 'tax_id', + regex: '([', + message: 'm', + }); + for (const command of AUTHORING_COMMANDS) { + const findings = runAuthoringRules(command, { normalized, parsed: normalized }); + expect( + findings.filter((f) => f.rule === VALIDATION_RULE_REGEX_UNCOMPILABLE), + `os ${command} must surface the finding`, + ).toHaveLength(1); + expect(findings.find((f) => f.rule === VALIDATION_RULE_REGEX_UNCOMPILABLE)!.severity).toBe('error'); + } + }); +}); diff --git a/packages/lint/src/validate-rule-compilability.ts b/packages/lint/src/validate-rule-compilability.ts new file mode 100644 index 0000000000..13fe5f0c00 --- /dev/null +++ b/packages/lint/src/validate-rule-compilability.ts @@ -0,0 +1,299 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #4762 — the two STATIC artifacts a validation rule carries must compile at + * publish time, because at runtime they are fail-OPEN. + * + * A `format` rule's `regex` and a `json_schema` rule's `schema` are handed to a + * real compiler on the write path (`objectql/src/validation/rule-validator.ts`): + * + * • `checkFormat` → `new RegExp(rule.regex)`, inside a `try/catch` whose + * catch logs `… has an invalid regex — skipped` and returns `null`; + * • `checkJsonSchema` → `ajv.compile(rule.schema)`, inside a `try/catch` whose + * catch logs `… has an uncompilable JSON Schema — skipped` and returns `null`. + * + * "Skipped" means the rule is declared, appears in the metadata, appears in any + * "what protects this object" listing — and enforces nothing, for every record, + * forever, with a WARN line in a log nobody reads as the only signal. That is + * the shape #4649 was filed about one rule type over, and #4761 closed for the + * CEL predicates (`script` / `cross_field` / `conditional.when`, now fail-closed). + * + * ## Why this gate is at AUTHORING time, and the runtime is deliberately untouched + * + * #4762's ruling: route 1 only. A broken regex or an uncompilable schema is + * **static** — it is decidable from the metadata alone, with no record in hand — + * so it can be rejected before it is ever deployed, which kills the rule class + * outright ("declared = enforced", PD #12). The runtime alternative is strictly + * worse as a FIRST move: an unevaluable *predicate* rejects only the writes whose + * data triggers the fault, whereas a broken *regex* would reject **every** write + * touching that field for as long as the bad metadata is deployed. So the + * authoring door is closed here, and whether a runtime backstop is still wanted + * stays open for the maintainer on #4762. + * + * Consequently `rule-validator.ts` is untouched by this rule's PR, and its + * "Deliberately NOT changed here" paragraph plus the `#4649 — unchanged + * neighbours` pin tests in `rule-fail-closed.test.ts` stand exactly as they are: + * they remain the honest record that the RUNTIME half is still fail-open. + * + * ## Detection is the real compilers, never a pattern that judges a pattern + * + * Parsing a regex with a regex, or type-checking a JSON Schema by hand, would + * build a SECOND opinion about compilability that can only drift from the + * runtime's — a fresh declared≠enforced gap one level up. So: + * + * - 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 }`. + * `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. + * + * 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. + * + * ### One ajv instance per schema, on purpose + * + * The runtime keeps ONE module-level ajv for the whole process; this gate + * constructs a fresh one per schema. That is not a mismatch of options, it is a + * deliberate narrowing of the question: "can ajv compile THIS schema?" is + * decidable per rule, while a shared instance additionally fails on a duplicate + * `$id` — an outcome that depends on which other schemas happened to be compiled + * first, i.e. on process history a build-time gate cannot know. Rejecting on + * that would be a verdict invented from ordering. (A duplicate `$id` between two + * deployed rules therefore remains a runtime fail-open case this gate does not + * see; it is part of the runtime-backstop question #4762 leaves open, not a + * silent omission.) + * + * ## Scope + * + * Object validation rules only — `object.validations[]`, including the rules + * nested in a `conditional`'s `then` / `otherwise`, since `evaluateRule` + * recurses into those and reaches the very same `checkFormat` / + * `checkJsonSchema`. Nothing else in the stack is judged here. + * + * ## Why `error` + * + * The severity bar `lint-flow-patterns.ts` states — gate when NO reading of the + * metadata behaves as written. A rule that cannot compile does not validate + * loosely or partially; it does not run at all. There is no deployment in which + * the author's declared guarantee holds. + */ + +import { createRequire } from 'node:module'; +// `import type` only — ajv must not load at import time. `@objectstack/lint` is +// on the kernel boot path (`@objectstack/lint/runtime`, reached by every runtime +// 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`. +import type { Options as AjvOptions } from 'ajv'; + +export type RuleCompilabilitySeverity = 'error'; + +export interface RuleCompilabilityFinding { + severity: RuleCompilabilitySeverity; + /** Stable diagnostic rule id (`--json` consumers and allowlists key on it). */ + rule: string; + /** Human-readable location, e.g. `object 'account' · validation 'tax_id_format'`. */ + where: string; + /** Config path, e.g. `objects.account.validations.tax_id_format.regex`. */ + path: string; + message: string; + hint: string; +} + +/** A `format` rule whose `regex` `new RegExp(...)` refuses to compile. */ +export const VALIDATION_RULE_REGEX_UNCOMPILABLE = 'validation-rule-regex-uncompilable'; +/** A `json_schema` rule whose `schema` ajv refuses to compile. */ +export const VALIDATION_RULE_SCHEMA_UNCOMPILABLE = 'validation-rule-json-schema-uncompilable'; + +/** + * The ajv options the runtime's shared instance is constructed with + * (`rule-validator.ts`). Exported so the parity test can assert BOTH halves — + * that this object is what the gate compiles with, and that the runtime source + * still says the same thing. + */ +export const RUNTIME_AJV_OPTIONS: AjvOptions = { allErrors: true, strict: false }; + +type AnyRec = Record; + +const isRec = (v: unknown): v is AnyRec => !!v && typeof v === 'object' && !Array.isArray(v); + +/** Coerce an array-or-name-keyed-map collection to an array of records. */ +function asArray(v: unknown): AnyRec[] { + if (Array.isArray(v)) return v.filter(isRec); + if (isRec(v)) { + return Object.entries(v) + .filter(([, def]) => isRec(def)) + .map(([name, def]) => ({ name, ...(def as AnyRec) })); + } + return []; +} + +/** + * Minimal structural type for the slice of ajv this gate uses. Declared here + * rather than imported as a value so no import statement can accidentally become + * eager — see the lazy-load note at the top. + */ +type AjvLike = { compile: (schema: unknown) => unknown }; +type AjvCtor = new (options?: AjvOptions) => AjvLike; + +let cachedAjv: AjvCtor | null = null; + +/** + * Load ajv on first use. `node:module` is a Node builtin untouched by + * esbuild/tsup, so the static `createRequire` import survives bundling; the + * `createRequire(...)` CALL is deferred because `import.meta.url` is rewritten to + * an empty stub in the CJS build (the same pattern + * `validate-hook-body-writes.ts` uses for `typescript`). + */ +function loadAjv(): AjvCtor { + if (cachedAjv) return cachedAjv; + 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'); + } catch (err) { + throw new Error( + `@objectstack/lint: checking a \`json_schema\` validation rule requires the "ajv" 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" in the image; it is only loaded ` + + `when a stack declares a \`json_schema\` validation rule.`, + ); + } + // ajv 8 is CJS with an ESM-interop default export; both shapes are seen + // depending on which build of this package is running. + const ctor = (isRec(mod) && 'default' in mod ? (mod as AnyRec).default : mod) as AjvCtor; + cachedAjv = ctor; + return ctor; +} + +/** The message a thrown compile error contributes, verbatim. */ +function errorText(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} + +/** + * Depth cap on `conditional` nesting. Parsed metadata is a tree, so this is not + * a cycle guard — it is the same cheap promise `flow-walk.ts`'s + * `MAX_REGION_DEPTH` makes: a hand-authored (PRE-parse) stack cannot make a lint + * hang. `os lint` never parses, so it hands this walker whatever object the + * author's module actually built, and `const r = {…}; r.then = r` is a two-line + * accident. Well past anything reviewable — three levels of nested conditional + * validation is already unreadable. + */ +export const MAX_RULE_NESTING_DEPTH = 16; + +/** + * Every rule reachable from one authored `validations[]` entry: the rule itself + * plus, for a `conditional`, its `then` / `otherwise` branches — which + * `evaluateRule` dispatches to exactly like a top-level rule, and which + * therefore reach `checkFormat` / `checkJsonSchema` on the same terms. + * + * `label` carries the nesting so a finding points at the branch the author has + * to edit, not merely at the outermost rule's name. + */ +function flattenRules( + rule: AnyRec, + labelTrail: string, + pathTrail: string, + depth = 0, +): Array<{ rule: AnyRec; label: string; path: string }> { + const name = typeof rule.name === 'string' && rule.name ? rule.name : '?'; + // The two trails are deliberately different: the LABEL reads as prose for a + // human (`'outer' → 'inner'`), the PATH points at the config key an editor + // has to open (`outer.then.inner`). + const label = labelTrail ? `${labelTrail} → '${name}'` : `'${name}'`; + const path = pathTrail ? `${pathTrail}.${name}` : name; + const out = [{ rule, label, path }]; + if (depth >= MAX_RULE_NESTING_DEPTH) return out; + for (const branch of ['then', 'otherwise'] as const) { + const nested = rule[branch]; + if (isRec(nested)) out.push(...flattenRules(nested, label, `${path}.${branch}`, depth + 1)); + } + return out; +} + +/** + * Reject every object validation rule whose static artifact — a `format` rule's + * `regex`, a `json_schema` rule's `schema` — the runtime's own compiler cannot + * compile. Pure `(stack) => Finding[]`; never throws. + */ +export function validateRuleCompilability(stack: unknown): RuleCompilabilityFinding[] { + const findings: RuleCompilabilityFinding[] = []; + if (!isRec(stack)) return findings; + + for (const obj of asArray(stack.objects)) { + const objectName = typeof obj.name === 'string' ? obj.name : '(unnamed object)'; + // `validations` is the spec key; `validationRules` is read for the same + // reason `validate-expressions.ts` reads it — so the two rules that judge + // one object's validation list can never see different lists. Not a new + // dialect: no key is invented here, and neither rule rewrites anything. + const validations = obj.validations ?? obj.validationRules; + + for (const authored of asArray(validations)) { + for (const { rule, label, path } of flattenRules(authored, '', '')) { + const where = `object '${objectName}' · validation ${label}`; + const basePath = `objects.${objectName}.validations.${path}`; + + if (rule.type === 'format' && typeof rule.regex === 'string' && rule.regex !== '') { + try { + // The exact call `checkFormat` makes — no flags, same constructor. + new RegExp(rule.regex); + } catch (err) { + findings.push({ + severity: 'error', + rule: VALIDATION_RULE_REGEX_UNCOMPILABLE, + where, + path: `${basePath}.regex`, + message: + `\`format\` validation ${label} on object '${objectName}' declares a \`regex\` that does not ` + + `compile: ${errorText(err)}. The write path builds it with \`new RegExp(rule.regex)\` and ` + + `SKIPS the rule when that throws (rule-validator.ts \`checkFormat\`), so the rule is declared, ` + + `listed in the metadata, and enforces nothing on any record.`, + hint: + `Fix the pattern so \`new RegExp('${rule.regex}')\` compiles — a literal \`(\`, \`[\` or \`\\\` ` + + `must be escaped (\`\\\\(\`, \`\\\\[\`, \`\\\\\\\\\`), and the source is a STRING, so a backslash ` + + `is written twice in TypeScript ('^\\\\d{2}-\\\\d{7}$'). Or drop \`regex\` and use a named ` + + `\`format\` ('email' | 'url' | 'phone' | 'json').`, + }); + } + } + + 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); + } catch (err) { + findings.push({ + severity: 'error', + rule: VALIDATION_RULE_SCHEMA_UNCOMPILABLE, + where, + path: `${basePath}.schema`, + 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 ` + + `(rule-validator.ts \`checkJsonSchema\`), so the rule is declared and enforces nothing on any ` + + `record.`, + hint: + `Correct the schema so ajv compiles it — the message above names the offending keyword. ` + + `\`type\` must be one of null|boolean|object|array|number|string|integer (or an array of ` + + `those), \`required\` an array of strings, and every \`$ref\` must resolve. Vendor keywords ` + + `are fine (the runtime runs \`strict: false\`); a MALFORMED standard keyword is not.`, + }); + } + } + } + } + } + + return findings; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e7d4812ae3..f38b56293d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -832,6 +832,9 @@ importers: '@objectstack/spec': specifier: workspace:* version: link:../spec + ajv: + specifier: ^8.20.0 + version: 8.20.0 sucrase: specifier: ^3.35.1 version: 3.35.1