Skip to content

Commit ca522e9

Browse files
fix(lint): 超预算的 RLS 谓词有了自己的规则 id rls-predicate-over-budget (#6778) (#6831)
`rowLevelSecurity[].using` / `.check` 里一条语法完美、可下推、只是超出平台解析 预算的 CEL 谓词(80 项合取越过 `maxAstNodes` 256),此前报在 `rls-predicate-unparseable` 名下,而那条规则的提示语讲的是 SQL 与 CEL 的方言 混淆。判决是对的,指路是错的:作者要做的是把谓词改小或拆开。 新增第三个 id,键控在 formula 姊妹入口 `parseCelToAstWithReason` 的 `kind: 'bounds'` 载荷上:消息点名具体越界的那个界与平台取值(三个界各报各的, 不写死),提示语给出真正的补救,并写明只有顶层 `||` 可以拆成多条策略——策略之间 是 OR,拆顶层 `&&` 会放大访问权限。 无行为变更:判定边界仍是 `isSupportedRlsExpression`,一字未动;区分只发生在解释 里。规则对 `cel-pushdown-limits.ts` 的 GA 开关保持中立,两个位置都有测试钉住。 Claude-Session: https://claude.ai/code/session_01AZgRyPVwi1jLb1mNNuUQ9o Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2205363 commit ca522e9

4 files changed

Lines changed: 358 additions & 5 deletions

File tree

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/lint": patch
3+
---
4+
5+
fix(lint): 超预算的 RLS 谓词有了自己的规则 id,不再被当成方言写错 (#6778)
6+
7+
`rowLevelSecurity[].using` / `.check` 里一条**语法完美、可下推**、只是太大的 CEL
8+
谓词(例如 80 项合取,超过 `maxAstNodes` 256),此前报在
9+
`rls-predicate-unparseable` 名下——那条规则的提示语讲的是 SQL 与 CEL 的方言混淆
10+
("用 `&&` 别用 `AND`"、"`LIKE` 没有 CEL 拼法")。判决是**对的**,指路是错的:
11+
作者要做的是把谓词改小或拆开,而不是检查自己的方言。
12+
13+
新增第三个 id **`rls-predicate-over-budget`**(与既有两个并列导出):
14+
15+
- 消息点名**具体越界的那个界**和平台取值——`maxAstNodes` (256) / `maxDepth` (32)
16+
/ `maxListElements` (64) 各自报各自的,取自 formula 姊妹入口
17+
`parseCelToAstWithReason``kind: 'bounds'` 载荷,而不是写死一个;被告知去缩短
18+
错误的那根轴,作者就会改错地方。越界谓词按定义很长,引文因此截断到 200 字符。
19+
- 提示语给的是真正的补救:把长 `||` 链折成 `field in [...]`;把集合预解析成
20+
`current_user.<key>` 成员键(ADR-0105 D11);把重复子表达式反范式化成本对象上的
21+
一个 formula/rollup 字段;以及——**只对顶层 `||`** 可以拆成多条策略(适用策略之
22+
间是 OR),顶层 `&&` 这样拆会**放大**访问权限而不是保持它。
23+
24+
**没有行为变更。** 判定边界仍然是 `isSupportedRlsExpression` 本身,一个字符没动;
25+
同样的输入照样被拒,只是其中一类被告知了真正的原因。区分只发生在**解释**里:
26+
运行时把越界折叠进 `reason: 'parse-error'` 是有意为之("每个消费者都已经把这个
27+
reason 路由到自己的拒绝路径"),对只需决定拒不拒的运行时是对的,对职责就是点明该
28+
改哪里的授时诊断则不然。
29+
30+
该规则不读 `cel-pushdown-limits.ts` 的 GA 日期开关,对它保持中立:17.0.0-rc.x 宽限
31+
窗口内越界谓词仍被放行,本规则一条都不报(实测 0 条);v17 GA 翻转后同一谓词被拒,
32+
落到新 id 上。两个开关位置都有测试钉住,越界与真正的语法错误两侧各自成对钉住,
33+
将来任何把二者重新合并的改动都会变红。

packages/lint/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ export {
254254
validateRlsPredicateEnforceability,
255255
RLS_PREDICATE_UNENFORCEABLE,
256256
RLS_PREDICATE_UNPARSEABLE,
257+
RLS_PREDICATE_OVER_BUDGET,
257258
} from './validate-rls-predicate-enforceability.js';
258259
export type {
259260
RlsPredicateFinding,

packages/lint/src/validate-rls-predicate-enforceability.test.ts

Lines changed: 202 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

3-
import { describe, it, expect } from 'vitest';
4-
import { isSupportedRlsExpression } from '@objectstack/formula';
3+
import { describe, it, expect, afterEach } from 'vitest';
4+
import { isSupportedRlsExpression, setCelPushdownLimitsModeForTests } from '@objectstack/formula';
55

66
import {
77
validateRlsPredicateEnforceability,
88
RLS_PREDICATE_UNENFORCEABLE,
99
RLS_PREDICATE_UNPARSEABLE,
10+
RLS_PREDICATE_OVER_BUDGET,
1011
} from './validate-rls-predicate-enforceability.js';
1112
import { AUTHORING_RULES, runAuthoringRules } from './authoring-rules.js';
1213

@@ -307,3 +308,202 @@ describe('validateRlsPredicateEnforceability — the verdict IS the RLSCompiler\
307308
.toContain(RLS_PREDICATE_UNENFORCEABLE);
308309
});
309310
});
311+
312+
// ── Over budget is not a dialect mistake (#6778) ─────────────────────
313+
//
314+
// The pushdown compiler collapses a `DEFAULT_LIMITS` overrun into
315+
// `reason: 'parse-error'` deliberately — it is the reason every consumer
316+
// already routes to its deny path. Correct for the runtime, whose only
317+
// decision is deny-or-not; wrong for an authoring diagnostic, whose job is to
318+
// name the edit. Before #6778 an 80-term conjunction — valid, lowerable CEL
319+
// that is merely too big — was reported under `rls-predicate-unparseable`,
320+
// whose hint explains SQL-vs-CEL syntax confusion.
321+
//
322+
// These cases run at BOTH positions of `cel-pushdown-limits.ts`'s dated GA
323+
// switch, because the two positions are where the whole question lives: during
324+
// 17.0.0-rc.x the grace window admits an over-limit predicate and this rule
325+
// must stay silent; at the v17 GA flip the same predicate is refused and must
326+
// be told the truth about why.
327+
328+
/** Over one `DEFAULT_LIMITS` bound each, and nothing else wrong with them. */
329+
const OVER_BUDGET = {
330+
maxAstNodes: Array.from({ length: 80 }, (_, i) => `record.f${i} == ${i}`).join(' && '),
331+
maxDepth: '('.repeat(40) + 'record.a == 1' + ')'.repeat(40),
332+
maxListElements: `record.x in [${Array.from({ length: 100 }, (_, i) => i).join(', ')}]`,
333+
} as const;
334+
335+
/** Genuinely not CEL — the class `rls-predicate-unparseable` was written for. */
336+
const NOT_CEL = {
337+
'SQL AND': 'a = current_user.id AND b = 1',
338+
'a subquery': 'id IN (SELECT id FROM users)',
339+
'a stray operator': 'record.stage ==',
340+
} as const;
341+
342+
describe('validateRlsPredicateEnforceability — a bounds overrun is its own id (#6778)', () => {
343+
afterEach(() => {
344+
// A suite must not leak a mode into the next file.
345+
setCelPushdownLimitsModeForTests('rc-grace')();
346+
});
347+
348+
const atGa = <T>(fn: () => T): T => {
349+
const restore = setCelPushdownLimitsModeForTests('fail-closed');
350+
try {
351+
return fn();
352+
} finally {
353+
restore();
354+
}
355+
};
356+
357+
// ── The shipped position: nothing changes today ───────────────────
358+
it.each(Object.entries(OVER_BUDGET))(
359+
'stays silent on an over-%s predicate during the rc grace window — no behaviour change today',
360+
(_limit, source) => {
361+
// The grace window admits it (it still compiles and WARNs), so
362+
// `isSupportedRlsExpression` is true and the rule never fires.
363+
expect(isSupportedRlsExpression(source)).toBe(true);
364+
expect(validateRlsPredicateEnforceability(policyWith('using', source))).toEqual([]);
365+
},
366+
);
367+
368+
// ── The GA position: the whole point of the card ──────────────────
369+
it('at the GA flip, an over-budget predicate reports over-budget — naming the bound and its value', () => {
370+
const findings = atGa(() => validateRlsPredicateEnforceability(policyWith('using', OVER_BUDGET.maxAstNodes)));
371+
expect(findings).toHaveLength(1);
372+
expect(findings[0]).toMatchObject({
373+
severity: 'error',
374+
rule: RLS_PREDICATE_OVER_BUDGET,
375+
path: 'permissions[0].rowLevelSecurity[0].using',
376+
where: 'permission set "sales_rep" policy "own_leads" on object "lead"',
377+
});
378+
// The bound and the budget are the two facts "shrink it to fit" needs.
379+
expect(findings[0].message).toMatch(/maxAstNodes/);
380+
expect(findings[0].message).toMatch(/platform limit 256/);
381+
expect(findings[0].message).toMatch(/Exceeded maxAstNodes \(256\)/);
382+
// The verdict is unchanged, so the consequence prose must still be there.
383+
expect(findings[0].message).toMatch(/DROPS the policy at request time/);
384+
expect(findings[0].message).toMatch(/ZERO rows/);
385+
// An over-budget predicate is long by definition — the quote is bounded.
386+
expect(findings[0].message).toContain('...');
387+
expect(findings[0].message.length).toBeLessThan(OVER_BUDGET.maxAstNodes.length + 1200);
388+
});
389+
390+
it('prescribes shrinking, and never sends the author to check their dialect', () => {
391+
const [f] = atGa(() => validateRlsPredicateEnforceability(policyWith('using', OVER_BUDGET.maxAstNodes)));
392+
// The real remedies.
393+
expect(f.hint).toMatch(/field in \[a, b, \]/);
394+
expect(f.hint).toMatch(/current_user\.<key>/);
395+
expect(f.hint).toMatch(/[Dd]enormalise/);
396+
expect(f.hint).toMatch(/hook or action body/);
397+
// Splitting is only sound on a top-level `||`; policies are OR-ed, so
398+
// splitting an `&&` would WIDEN access. Saying so is the point of the hint.
399+
expect(f.hint).toMatch(/never split a top-level `&&`/);
400+
expect(f.hint).toMatch(/WIDEN access/);
401+
// …and explicitly NOT the SQL-vs-CEL prose this class used to get.
402+
expect(f.hint).toMatch(/no syntax or dialect error/);
403+
expect(f.hint).not.toMatch(/canonical CEL \(ADR-0058 D1\)/);
404+
expect(f.hint).not.toMatch(/rather than SQL `AND` \/ `OR`/);
405+
expect(f.hint).not.toMatch(/LIKE/);
406+
});
407+
408+
it('names the bound that was actually blown, not a hard-coded one', () => {
409+
// Reading `limit` / `limitValue` off the sister entrance's payload rather
410+
// than assuming `maxAstNodes` is what makes the hint worth reading: an
411+
// author told to shorten the wrong axis edits the wrong thing.
412+
const [depth] = atGa(() => validateRlsPredicateEnforceability(policyWith('using', OVER_BUDGET.maxDepth)));
413+
expect(depth.rule).toBe(RLS_PREDICATE_OVER_BUDGET);
414+
expect(depth.message).toMatch(/maxDepth/);
415+
expect(depth.message).toMatch(/platform limit 32/);
416+
expect(depth.message).not.toMatch(/maxAstNodes/);
417+
418+
const [list] = atGa(() => validateRlsPredicateEnforceability(policyWith('using', OVER_BUDGET.maxListElements)));
419+
expect(list.rule).toBe(RLS_PREDICATE_OVER_BUDGET);
420+
expect(list.message).toMatch(/maxListElements/);
421+
expect(list.message).toMatch(/platform limit 64/);
422+
expect(list.message).not.toMatch(/maxAstNodes/);
423+
});
424+
425+
it('carries the WRITE-path consequence when the over-budget clause is `check`', () => {
426+
const [f] = atGa(() => validateRlsPredicateEnforceability(policyWith('check', OVER_BUDGET.maxAstNodes)));
427+
expect(f).toMatchObject({
428+
rule: RLS_PREDICATE_OVER_BUDGET,
429+
path: 'permissions[0].rowLevelSecurity[0].check',
430+
});
431+
expect(f.message).toMatch(/PermissionDeniedError/);
432+
expect(f.message).not.toMatch(/ZERO rows/);
433+
});
434+
435+
// ── The discrimination, which IS the card ─────────────────────────
436+
//
437+
// A split that cannot be shown to separate the two classes is decoration.
438+
// Both halves are pinned in one table so a future change that collapses them
439+
// — in either direction — goes red here rather than silently mislabelling
440+
// one class again.
441+
it('discriminates over-budget from not-CEL at the GA position, in both directions', () => {
442+
const expected: Array<[string, string, string]> = [
443+
...Object.entries(OVER_BUDGET).map(
444+
([limit, src]) => [`over ${limit}`, src, RLS_PREDICATE_OVER_BUDGET] as [string, string, string],
445+
),
446+
...Object.entries(NOT_CEL).map(
447+
([label, src]) => [label, src, RLS_PREDICATE_UNPARSEABLE] as [string, string, string],
448+
),
449+
];
450+
const actual = atGa(() =>
451+
expected.map(([label, src]) => [label, ids(policyWith('using', src))] as const),
452+
);
453+
expect(actual).toEqual(expected.map(([label, , rule]) => [label, [rule]]));
454+
});
455+
456+
it('a predicate that is BOTH unparseable and huge is unparseable — syntax is judged first', () => {
457+
// 80 SQL `AND` terms: over `maxAstNodes` in size, but the bridge does not
458+
// cover `AND`, so it is not CEL at all. Shortening it would not help; the
459+
// author has to rewrite it, so the syntax id is the useful one. The parse
460+
// never reaches a bounds fault because it throws on `AND` first.
461+
const source = Array.from({ length: 80 }, (_, i) => `f${i} = ${i}`).join(' AND ');
462+
expect(source.length).toBeGreaterThan(OVER_BUDGET.maxAstNodes.length / 2);
463+
expect(atGa(() => ids(policyWith('using', source)))).toEqual([RLS_PREDICATE_UNPARSEABLE]);
464+
});
465+
466+
it('leaves the unenforceable class alone — an over-budget check never steals a shape fault', () => {
467+
// Reported at BOTH switch positions: this class does not involve the parse
468+
// bounds at all, so neither position may re-route it.
469+
for (const source of ['size(record.tags) > 0', "record.account.region == 'EU'", 'amount + 1 > 2']) {
470+
expect(ids(policyWith('using', source))).toEqual([RLS_PREDICATE_UNENFORCEABLE]);
471+
expect(atGa(() => ids(policyWith('using', source)))).toEqual([RLS_PREDICATE_UNENFORCEABLE]);
472+
}
473+
});
474+
475+
// ── The red/green boundary is untouched ───────────────────────────
476+
it('refuses exactly what it refused before — only the explanation moved', () => {
477+
// #6778 is explicitly NOT a behaviour change. The rule's verdict is still
478+
// `isSupportedRlsExpression`, so lint-clean must remain that function's own
479+
// answer at BOTH switch positions, over-budget sources included.
480+
const corpus = [
481+
...Object.values(OVER_BUDGET),
482+
...Object.values(NOT_CEL),
483+
'owner_id == current_user.id',
484+
"status = 'published'",
485+
'size(record.tags) > 0',
486+
];
487+
for (const mode of ['rc-grace', 'fail-closed'] as const) {
488+
const restore = setCelPushdownLimitsModeForTests(mode);
489+
try {
490+
for (const source of corpus) {
491+
const lintIsClean = validateRlsPredicateEnforceability(policyWith('using', source)).length === 0;
492+
expect({ mode, source: source.slice(0, 40), lintIsClean }).toEqual({
493+
mode,
494+
source: source.slice(0, 40),
495+
lintIsClean: isSupportedRlsExpression(source),
496+
});
497+
}
498+
} finally {
499+
restore();
500+
}
501+
}
502+
});
503+
504+
it('reaches the author through the real registry, not just a direct call', () => {
505+
const stack = policyWith('using', OVER_BUDGET.maxAstNodes);
506+
expect(atGa(() => runAuthoringRules('validate', { normalized: stack, parsed: stack }).map((f) => f.rule)))
507+
.toEqual([RLS_PREDICATE_OVER_BUDGET]);
508+
});
509+
});

0 commit comments

Comments
 (0)