fix(lint): 超预算的 RLS 谓词有了自己的规则 id rls-predicate-over-budget (#6778)

.changeset/rls-predicate-over-budget.md

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

packages/lint/src/index.ts

@@ -254,6 +254,7 @@ export {
   validateRlsPredicateEnforceability,
   RLS_PREDICATE_UNENFORCEABLE,
   RLS_PREDICATE_UNPARSEABLE,
+  RLS_PREDICATE_OVER_BUDGET,
 } from './validate-rls-predicate-enforceability.js';
 export type {
   RlsPredicateFinding,

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

@@ -1,12 +1,13 @@
 // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
 
-import { describe, it, expect } from 'vitest';
-import { isSupportedRlsExpression } from '@objectstack/formula';
+import { describe, it, expect, afterEach } from 'vitest';
+import { isSupportedRlsExpression, setCelPushdownLimitsModeForTests } from '@objectstack/formula';
 
 import {
   validateRlsPredicateEnforceability,
   RLS_PREDICATE_UNENFORCEABLE,
   RLS_PREDICATE_UNPARSEABLE,
+  RLS_PREDICATE_OVER_BUDGET,
 } from './validate-rls-predicate-enforceability.js';
 import { AUTHORING_RULES, runAuthoringRules } from './authoring-rules.js';
 
@@ -307,3 +308,202 @@ describe('validateRlsPredicateEnforceability — the verdict IS the RLSCompiler\
       .toContain(RLS_PREDICATE_UNENFORCEABLE);
   });
 });
+
+// ── Over budget is not a dialect mistake (#6778) ─────────────────────
+//
+// The pushdown compiler collapses a `DEFAULT_LIMITS` overrun into
+// `reason: 'parse-error'` deliberately — it is the reason every consumer
+// already routes to its deny path. Correct for the runtime, whose only
+// decision is deny-or-not; wrong for an authoring diagnostic, whose job is to
+// name the edit. Before #6778 an 80-term conjunction — valid, lowerable CEL
+// that is merely too big — was reported under `rls-predicate-unparseable`,
+// whose hint explains SQL-vs-CEL syntax confusion.
+//
+// These cases run at BOTH positions of `cel-pushdown-limits.ts`'s dated GA
+// switch, because the two positions are where the whole question lives: during
+// 17.0.0-rc.x the grace window admits an over-limit predicate and this rule
+// must stay silent; at the v17 GA flip the same predicate is refused and must
+// be told the truth about why.
+
+/** Over one `DEFAULT_LIMITS` bound each, and nothing else wrong with them. */
+const OVER_BUDGET = {
+  maxAstNodes: Array.from({ length: 80 }, (_, i) => `record.f${i} == ${i}`).join(' && '),
+  maxDepth: '('.repeat(40) + 'record.a == 1' + ')'.repeat(40),
+  maxListElements: `record.x in [${Array.from({ length: 100 }, (_, i) => i).join(', ')}]`,
+} as const;
+
+/** Genuinely not CEL — the class `rls-predicate-unparseable` was written for. */
+const NOT_CEL = {
+  'SQL AND': 'a = current_user.id AND b = 1',
+  'a subquery': 'id IN (SELECT id FROM users)',
+  'a stray operator': 'record.stage ==',
+} as const;
+
+describe('validateRlsPredicateEnforceability — a bounds overrun is its own id (#6778)', () => {
+  afterEach(() => {
+    // A suite must not leak a mode into the next file.
+    setCelPushdownLimitsModeForTests('rc-grace')();
+  });
+
+  const atGa = <T>(fn: () => T): T => {
+    const restore = setCelPushdownLimitsModeForTests('fail-closed');
+    try {
+      return fn();
+    } finally {
+      restore();
+    }
+  };
+
+  // ── The shipped position: nothing changes today ───────────────────
+  it.each(Object.entries(OVER_BUDGET))(
+    'stays silent on an over-%s predicate during the rc grace window — no behaviour change today',
+    (_limit, source) => {
+      // The grace window admits it (it still compiles and WARNs), so
+      // `isSupportedRlsExpression` is true and the rule never fires.
+      expect(isSupportedRlsExpression(source)).toBe(true);
+      expect(validateRlsPredicateEnforceability(policyWith('using', source))).toEqual([]);
+    },
+  );
+
+  // ── The GA position: the whole point of the card ──────────────────
+  it('at the GA flip, an over-budget predicate reports over-budget — naming the bound and its value', () => {
+    const findings = atGa(() => validateRlsPredicateEnforceability(policyWith('using', OVER_BUDGET.maxAstNodes)));
+    expect(findings).toHaveLength(1);
+    expect(findings[0]).toMatchObject({
+      severity: 'error',
+      rule: RLS_PREDICATE_OVER_BUDGET,
+      path: 'permissions[0].rowLevelSecurity[0].using',
+      where: 'permission set "sales_rep" policy "own_leads" on object "lead"',
+    });
+    // The bound and the budget are the two facts "shrink it to fit" needs.
+    expect(findings[0].message).toMatch(/maxAstNodes/);
+    expect(findings[0].message).toMatch(/platform limit 256/);
+    expect(findings[0].message).toMatch(/Exceeded maxAstNodes \(256\)/);
+    // The verdict is unchanged, so the consequence prose must still be there.
+    expect(findings[0].message).toMatch(/DROPS the policy at request time/);
+    expect(findings[0].message).toMatch(/ZERO rows/);
+    // An over-budget predicate is long by definition — the quote is bounded.
+    expect(findings[0].message).toContain('...');
+    expect(findings[0].message.length).toBeLessThan(OVER_BUDGET.maxAstNodes.length + 1200);
+  });
+
+  it('prescribes shrinking, and never sends the author to check their dialect', () => {
+    const [f] = atGa(() => validateRlsPredicateEnforceability(policyWith('using', OVER_BUDGET.maxAstNodes)));
+    // The real remedies.
+    expect(f.hint).toMatch(/field in \[a, b, …\]/);
+    expect(f.hint).toMatch(/current_user\.<key>/);
+    expect(f.hint).toMatch(/[Dd]enormalise/);
+    expect(f.hint).toMatch(/hook or action body/);
+    // Splitting is only sound on a top-level `||`; policies are OR-ed, so
+    // splitting an `&&` would WIDEN access. Saying so is the point of the hint.
+    expect(f.hint).toMatch(/never split a top-level `&&`/);
+    expect(f.hint).toMatch(/WIDEN access/);
+    // …and explicitly NOT the SQL-vs-CEL prose this class used to get.
+    expect(f.hint).toMatch(/no syntax or dialect error/);
+    expect(f.hint).not.toMatch(/canonical CEL \(ADR-0058 D1\)/);
+    expect(f.hint).not.toMatch(/rather than SQL `AND` \/ `OR`/);
+    expect(f.hint).not.toMatch(/LIKE/);
+  });
+
+  it('names the bound that was actually blown, not a hard-coded one', () => {
+    // Reading `limit` / `limitValue` off the sister entrance's payload rather
+    // than assuming `maxAstNodes` is what makes the hint worth reading: an
+    // author told to shorten the wrong axis edits the wrong thing.
+    const [depth] = atGa(() => validateRlsPredicateEnforceability(policyWith('using', OVER_BUDGET.maxDepth)));
+    expect(depth.rule).toBe(RLS_PREDICATE_OVER_BUDGET);
+    expect(depth.message).toMatch(/maxDepth/);
+    expect(depth.message).toMatch(/platform limit 32/);
+    expect(depth.message).not.toMatch(/maxAstNodes/);
+
+    const [list] = atGa(() => validateRlsPredicateEnforceability(policyWith('using', OVER_BUDGET.maxListElements)));
+    expect(list.rule).toBe(RLS_PREDICATE_OVER_BUDGET);
+    expect(list.message).toMatch(/maxListElements/);
+    expect(list.message).toMatch(/platform limit 64/);
+    expect(list.message).not.toMatch(/maxAstNodes/);
+  });
+
+  it('carries the WRITE-path consequence when the over-budget clause is `check`', () => {
+    const [f] = atGa(() => validateRlsPredicateEnforceability(policyWith('check', OVER_BUDGET.maxAstNodes)));
+    expect(f).toMatchObject({
+      rule: RLS_PREDICATE_OVER_BUDGET,
+      path: 'permissions[0].rowLevelSecurity[0].check',
+    });
+    expect(f.message).toMatch(/PermissionDeniedError/);
+    expect(f.message).not.toMatch(/ZERO rows/);
+  });
+
+  // ── The discrimination, which IS the card ─────────────────────────
+  //
+  // A split that cannot be shown to separate the two classes is decoration.
+  // Both halves are pinned in one table so a future change that collapses them
+  // — in either direction — goes red here rather than silently mislabelling
+  // one class again.
+  it('discriminates over-budget from not-CEL at the GA position, in both directions', () => {
+    const expected: Array<[string, string, string]> = [
+      ...Object.entries(OVER_BUDGET).map(
+        ([limit, src]) => [`over ${limit}`, src, RLS_PREDICATE_OVER_BUDGET] as [string, string, string],
+      ),
+      ...Object.entries(NOT_CEL).map(
+        ([label, src]) => [label, src, RLS_PREDICATE_UNPARSEABLE] as [string, string, string],
+      ),
+    ];
+    const actual = atGa(() =>
+      expected.map(([label, src]) => [label, ids(policyWith('using', src))] as const),
+    );
+    expect(actual).toEqual(expected.map(([label, , rule]) => [label, [rule]]));
+  });
+
+  it('a predicate that is BOTH unparseable and huge is unparseable — syntax is judged first', () => {
+    // 80 SQL `AND` terms: over `maxAstNodes` in size, but the bridge does not
+    // cover `AND`, so it is not CEL at all. Shortening it would not help; the
+    // author has to rewrite it, so the syntax id is the useful one. The parse
+    // never reaches a bounds fault because it throws on `AND` first.
+    const source = Array.from({ length: 80 }, (_, i) => `f${i} = ${i}`).join(' AND ');
+    expect(source.length).toBeGreaterThan(OVER_BUDGET.maxAstNodes.length / 2);
+    expect(atGa(() => ids(policyWith('using', source)))).toEqual([RLS_PREDICATE_UNPARSEABLE]);
+  });
+
+  it('leaves the unenforceable class alone — an over-budget check never steals a shape fault', () => {
+    // Reported at BOTH switch positions: this class does not involve the parse
+    // bounds at all, so neither position may re-route it.
+    for (const source of ['size(record.tags) > 0', "record.account.region == 'EU'", 'amount + 1 > 2']) {
+      expect(ids(policyWith('using', source))).toEqual([RLS_PREDICATE_UNENFORCEABLE]);
+      expect(atGa(() => ids(policyWith('using', source)))).toEqual([RLS_PREDICATE_UNENFORCEABLE]);
+    }
+  });
+
+  // ── The red/green boundary is untouched ───────────────────────────
+  it('refuses exactly what it refused before — only the explanation moved', () => {
+    // #6778 is explicitly NOT a behaviour change. The rule's verdict is still
+    // `isSupportedRlsExpression`, so lint-clean must remain that function's own
+    // answer at BOTH switch positions, over-budget sources included.
+    const corpus = [
+      ...Object.values(OVER_BUDGET),
+      ...Object.values(NOT_CEL),
+      'owner_id == current_user.id',
+      "status = 'published'",
+      'size(record.tags) > 0',
+    ];
+    for (const mode of ['rc-grace', 'fail-closed'] as const) {
+      const restore = setCelPushdownLimitsModeForTests(mode);
+      try {
+        for (const source of corpus) {
+          const lintIsClean = validateRlsPredicateEnforceability(policyWith('using', source)).length === 0;
+          expect({ mode, source: source.slice(0, 40), lintIsClean }).toEqual({
+            mode,
+            source: source.slice(0, 40),
+            lintIsClean: isSupportedRlsExpression(source),
+          });
+        }
+      } finally {
+        restore();
+      }
+    }
+  });
+
+  it('reaches the author through the real registry, not just a direct call', () => {
+    const stack = policyWith('using', OVER_BUDGET.maxAstNodes);
+    expect(atGa(() => runAuthoringRules('validate', { normalized: stack, parsed: stack }).map((f) => f.rule)))
+      .toEqual([RLS_PREDICATE_OVER_BUDGET]);
+  });
+});