Skip to content

Commit da1a64c

Browse files
fix(lint): regex/JSON Schema 编译不过的校验规则改在发布期拒绝 —— 声明了却什么都不拦的两个静态产物 (#4762) (#5027)
* fix(lint): reject a validation rule whose regex or JSON Schema does not compile (#4762) 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, so the rule is declared, listed and enforces nothing on every record. Both faults are STATIC — decidable from the metadata alone — so they are rejected at authoring/publish time by a new gating rule, `validateRuleCompilability`, wired into AUTHORING_RULES (all three commands). Detection uses the real compilers: `new RegExp(source)` for the regex, and ajv constructed with the SAME options the runtime's shared instance uses (`{ allErrors: true, strict: false }`), pinned against `rule-validator.ts`'s own source so the two cannot drift. `packages/objectql` is deliberately untouched: the `#4649 — unchanged neighbours` pins and the module header stand as the honest record that the RUNTIME half is still fail-open. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NrmBxj8rK2uGCnh9aipjwX * fix(lint): cap `conditional` nesting depth in the rule-compilability walk (#4762) `os lint` never parses, so the walker is handed whatever object the author's own module built — and `const r = {…}; r.then = r` is a two-line accident that would hang the lint. Same promise `flow-walk.ts`'s `MAX_REGION_DEPTH` makes, with a non-vacuous test: a broken regex parked at the bottom of a legal nest is still reported, one level past the cap is where the walker stops looking. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NrmBxj8rK2uGCnh9aipjwX --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9fa6bab commit da1a64c

9 files changed

Lines changed: 880 additions & 7 deletions
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
---
2+
"@objectstack/lint": patch
3+
---
4+
5+
fix(lint): reject a validation rule whose regex or JSON Schema does not compile, at authoring time (#4762)
6+
7+
Two of the six object validation-rule types carry a **static artifact** that the
8+
write path hands to a real compiler, inside a `try/catch` that logs and returns
9+
`null`:
10+
11+
- `format``new RegExp(rule.regex)`*"Validation rule '…' has an invalid regex — skipped"*
12+
- `json_schema``ajv.compile(rule.schema)`*"Validation rule '…' has an uncompilable JSON Schema — skipped"*
13+
14+
"Skipped" means the rule is declared, appears in the metadata, appears in every
15+
"what protects this object" listing — and enforces nothing, on every record, for
16+
as long as the metadata is deployed, with a WARN line in a log nobody reads as
17+
the only signal. That is the shape #4649 was filed about one rule type over;
18+
#4761 flipped the CEL predicates to fail closed and deliberately left these two,
19+
because their blast radius differs (see below).
20+
21+
**New gate — `validateRuleCompilability`**, a `gating` entry in
22+
`AUTHORING_RULES`, so it runs on all three authoring commands (`os validate`,
23+
`os build`, `os lint`) with no per-command wiring. Two rule ids:
24+
25+
| id | fires when |
26+
|:---|:---|
27+
| `validation-rule-regex-uncompilable` | a `format` rule's `regex` throws in `new RegExp(...)` |
28+
| `validation-rule-json-schema-uncompilable` | a `json_schema` rule's `schema` throws in `ajv.compile(...)` |
29+
30+
Each finding names the rule, the object and the config path, and carries the
31+
**compiler's own error text verbatim** — an author cannot act on "invalid
32+
regex", but can act on `Invalid regular expression: /([/: Unterminated character
33+
class`. Rules nested in a `conditional`'s `then` / `otherwise` are judged too
34+
(`evaluateRule` recurses into them and reaches the very same checkers), and the
35+
finding names the branch it is in.
36+
37+
**Detection is the real compilers, never a pattern that judges a pattern.** The
38+
regex is compiled with `new RegExp(source)` — the exact call `checkFormat`
39+
makes. The schema is compiled with ajv constructed with the **same options the
40+
runtime's shared instance uses** (`{ allErrors: true, strict: false }`), read
41+
back out of `rule-validator.ts`'s source by a parity test so the day those
42+
options change, this gate is told rather than left quietly disagreeing.
43+
`strict: false` is load-bearing in both directions: a gate running `strict: true`
44+
would reject author-written schemas carrying vendor keywords that the write path
45+
compiles happily — a gate that turns working metadata red gets switched off, and
46+
then protects nothing.
47+
48+
`ajv` is a new dependency of `@objectstack/lint`, loaded **lazily**: only a
49+
stack that actually declares a `json_schema` validation rule pays for it, pinned
50+
by the package's `lazy-deps.test.ts` alongside `typescript` and `sucrase`. The
51+
kernel boot path (`@objectstack/lint/runtime`) never loads it at all.
52+
53+
**The runtime half is deliberately unchanged.** `rule-validator.ts` still fails
54+
open on both, and the `#4649 — unchanged neighbours` pins that record it stand
55+
exactly as they are. A broken regex or schema is *static* — decidable from the
56+
metadata alone, with no record in hand — so the authoring door closes the class
57+
outright without ever bricking a running deployment, whereas rejecting at write
58+
time would reject **every** write touching that field for as long as the bad
59+
metadata is deployed. Whether a runtime backstop is still wanted on top of a
60+
closed authoring door stays open on #4762.

packages/lint/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
"@objectstack/formula": "workspace:*",
3030
"@objectstack/sdui-parser": "workspace:*",
3131
"@objectstack/spec": "workspace:*",
32+
"ajv": "^8.20.0",
3233
"sucrase": "^3.35.1",
3334
"typescript": "^6.0.3"
3435
},

packages/lint/src/authoring-rules.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ import { validateSecurityPosture } from './validate-security-posture.js';
122122
import { validateOrgAxisRedLines } from './validate-org-axis-red-lines.js';
123123
import { validateSharingRuleEnforceability } from './validate-sharing-rule-enforceability.js';
124124
import { validateRlsPredicateEnforceability } from './validate-rls-predicate-enforceability.js';
125+
import { validateRuleCompilability } from './validate-rule-compilability.js';
125126
import { validateActionLocations } from './validate-action-locations.js';
126127
import { lintFlowPatterns } from './lint-flow-patterns.js';
127128
import { lintLivenessProperties } from './lint-liveness-properties.js';
@@ -846,6 +847,27 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [
846847
+ 'never run at a door should not claim it.',
847848
run: (stack) => validateRlsPredicateEnforceability(stack),
848849
},
850+
// #4762 — the same "declared but enforces nothing" question, for the two
851+
// STATIC artifacts an object validation rule carries. A `format` rule's
852+
// `regex` that `new RegExp(...)` throws on, and a `json_schema` rule's schema
853+
// ajv cannot compile, are both logged and SKIPPED on the write path
854+
// (`rule-validator.ts`), so the rule ships, lists, and protects nothing.
855+
// Neither needs a record to judge, so the authoring door is the right one:
856+
// rejecting a broken regex at RUNTIME instead would reject every write
857+
// touching that field for as long as the metadata is deployed (#4762's own
858+
// analysis — the runtime-backstop question stays open for the maintainer).
859+
// Gating for the `lint-flow-patterns.ts` bar: no reading of the metadata
860+
// behaves as written, because the rule does not run at all.
861+
{
862+
name: 'validateRuleCompilability',
863+
tier: 'gating',
864+
input: 'parsed',
865+
commands: ALL,
866+
source: 'packages/lint/src/validate-rule-compilability.ts',
867+
surfaces: CLI_ONLY,
868+
surfaceReason: RUNTIME_OBJECT_WRITES_P2,
869+
run: (stack) => validateRuleCompilability(stack),
870+
},
849871
];
850872

851873
// ─── Runner ─────────────────────────────────────────────────────────

packages/lint/src/index.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,23 @@ export {
281281
} from './validate-chart-bindings.js';
282282
export type { ChartBindingFinding, ChartBindingSeverity } from './validate-chart-bindings.js';
283283

284+
// #4762 — the two STATIC artifacts an object validation rule carries (a
285+
// `format` rule's `regex`, a `json_schema` rule's `schema`) are fail-OPEN at
286+
// runtime: one that does not compile is logged and skipped, so the rule is
287+
// declared and enforces nothing. Both are decidable without a record, so they
288+
// are rejected at authoring/publish time — with the REAL compilers, and for ajv
289+
// with the runtime's own options.
290+
export {
291+
validateRuleCompilability,
292+
RUNTIME_AJV_OPTIONS,
293+
VALIDATION_RULE_REGEX_UNCOMPILABLE,
294+
VALIDATION_RULE_SCHEMA_UNCOMPILABLE,
295+
} from './validate-rule-compilability.js';
296+
export type {
297+
RuleCompilabilityFinding,
298+
RuleCompilabilitySeverity,
299+
} from './validate-rule-compilability.js';
300+
284301
export { validateNavAccess, NAV_OBJECT_UNGRANTED } from './validate-nav-access.js';
285302
export type { NavAccessFinding, NavAccessSeverity } from './validate-nav-access.js';
286303

packages/lint/src/lazy-deps.test.ts

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@
99
// by the L2 body write-set gates (validate-hook-body-writes.ts since
1010
// #4271, validate-action-body-writes.ts since #4345);
1111
// - `sucrase` (~1.5 MB), loaded by the react syntax gate
12-
// (validate-react-pages.ts).
12+
// (validate-react-pages.ts);
13+
// - `ajv` (~2.4 MB installed), loaded by the #4762 publish gate
14+
// (validate-rule-compilability.ts) only when a stack declares a
15+
// `json_schema` validation rule — the one rule type whose static artifact
16+
// needs a JSON-Schema compiler to judge.
1317
//
1418
// "A react page" was the whole story when this file was written; it is not any
1519
// more, and the cases below say which trigger they are pinning. Keep them
@@ -38,7 +42,7 @@ const distDir = join(srcDir, '..', 'dist');
3842

3943
// Deps that must never load at import time. Extend this list when another
4044
// heavy, rarely-hit dependency joins the package.
41-
const LAZY_DEPS = ['typescript', 'sucrase'];
45+
const LAZY_DEPS = ['typescript', 'sucrase', 'ajv'];
4246

4347
const depLoaded = (cache: Record<string, unknown> | undefined, dep: string) =>
4448
Object.keys(cache ?? {}).some((p) => p.split(/[/\\]/).join('/').includes(`/node_modules/${dep}/`));
@@ -101,6 +105,18 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {
101105
const props = mod.validateReactPageProps(${reactStack('function Page(){ return <ObjectForm mode="edit" />; }')});
102106
if (!loaded('typescript')) fail('typescript was not loaded by a react-page props validation');
103107
if (!props.some((f) => f.rule === 'react-prop-missing-required')) fail('props gate produced no finding');
108+
// #4762 — the JSON-Schema compiler is the third lazy dep. A stack whose
109+
// validation rules are all format rules never pays for it.
110+
const ruleStack = (validation) => ({ objects: [{ name: 'a', fields: { payload: {} }, validations: [validation] }] });
111+
mod.validateRuleCompilability(ruleStack({ type: 'format', name: 'f', field: 'payload', regex: '([', message: 'm' }));
112+
if (loaded('ajv')) fail('the rule-compilability gate must not load ajv to judge a format rule');
113+
const schemaFindings = mod.validateRuleCompilability(
114+
ruleStack({ type: 'json_schema', name: 'j', field: 'payload', schema: { type: 'not-a-type' }, message: 'm' }),
115+
);
116+
if (!loaded('ajv')) fail('ajv was not loaded by a json_schema validation-rule check');
117+
if (!schemaFindings.some((f) => f.rule === 'validation-rule-json-schema-uncompilable')) {
118+
fail('rule-compilability gate produced no finding');
119+
}
104120
console.log('OK');
105121
};
106122
`;
@@ -132,8 +148,13 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {
132148

133149
it('loads each dep lazily in-process and the gates still work', async () => {
134150
const req = createRequire(import.meta.url);
135-
const { validateReactPages, validateReactPageProps, validateHookBodyWrites, validateActionBodyWrites } =
136-
await import('./index.js');
151+
const {
152+
validateReactPages,
153+
validateReactPageProps,
154+
validateHookBodyWrites,
155+
validateActionBodyWrites,
156+
validateRuleCompilability,
157+
} = await import('./index.js');
137158

138159
// Stacks without a react-source page never touch either dep.
139160
expect(validateReactPages({ pages: [{ name: 'p', kind: 'object' }] })).toEqual([]);
@@ -157,10 +178,29 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {
157178
actions: [action({ language: 'js', source: 'ctx.input.amout = 1; return { ok: true };' })],
158179
}),
159180
).toEqual([]);
181+
// #4762 — nor does judging a `format` rule's regex: that needs only the
182+
// JavaScript engine's own `new RegExp`, so a stack with no `json_schema`
183+
// validation rule never loads a JSON-Schema compiler.
184+
const ruleStack = (validation: unknown) => ({
185+
objects: [{ name: 'a', fields: { payload: {} }, validations: [validation] }],
186+
});
187+
expect(
188+
validateRuleCompilability(
189+
ruleStack({ type: 'format', name: 'f', field: 'payload', regex: '([', message: 'm' }),
190+
).map((f) => f.rule),
191+
).toEqual(['validation-rule-regex-uncompilable']);
192+
160193
for (const dep of LAZY_DEPS) {
161194
expect(depLoaded(req.cache, dep), `${dep} loaded before any react-source or L2-body validation`).toBe(false);
162195
}
163196

197+
// The first `json_schema` rule pays for ajv — and the gate still works.
198+
const schemaFindings = validateRuleCompilability(
199+
ruleStack({ type: 'json_schema', name: 'j', field: 'payload', schema: { type: 'not-a-type' }, message: 'm' }),
200+
);
201+
expect(depLoaded(req.cache, 'ajv')).toBe(true);
202+
expect(schemaFindings.map((f) => f.rule)).toEqual(['validation-rule-json-schema-uncompilable']);
203+
164204
// The first react page with source pays the cost of exactly its own gate's
165205
// dep — and the gates still work.
166206
const syntax = validateReactPages({

packages/lint/src/runtime-lazy-deps.test.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
//
33
// The kernel boot-path contract for `@objectstack/lint/runtime` (#4463).
44
//
5-
// `lazy-deps.test.ts` next door pins that IMPORTING the package loads neither
6-
// `typescript` (~9 MB) nor `sucrase`. That was enough while the only consumer
5+
// `lazy-deps.test.ts` next door pins that IMPORTING the package loads none of
6+
// `typescript` (~9 MB), `sucrase` or `ajv`. That was enough while the only consumer
77
// was the CLI, which may load anything. #4463 gave the package a consumer on
88
// the kernel boot path — `@objectstack/metadata-protocol`, reached by every
99
// runtime metadata write — and that consumer needs the stronger claim:
@@ -30,7 +30,12 @@ import { describe, it, expect } from 'vitest';
3030
const srcDir = dirname(fileURLToPath(import.meta.url));
3131
const distDir = join(srcDir, '..', 'dist');
3232

33-
const LAZY_DEPS = ['typescript', 'sucrase'];
33+
// `ajv` joined the list in #4762: the rule-compilability gate needs a real
34+
// JSON-Schema compiler to judge a `json_schema` validation rule, and that rule
35+
// is CLI-only (`surfaceReason: RUNTIME_OBJECT_WRITES_P2`). So the boot path must
36+
// not pay for it — not at import, and not while gating. Should that rule ever be
37+
// widened to `runtime-publish`, this assertion is what says so out loud.
38+
const LAZY_DEPS = ['typescript', 'sucrase', 'ajv'];
3439

3540
const depLoaded = (cache: Record<string, unknown> | undefined, dep: string) =>
3641
Object.keys(cache ?? {}).some((p) => p.split(/[/\\]/).join('/').includes(`/node_modules/${dep}/`));

0 commit comments

Comments
 (0)