|
1 | 1 | // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
2 | 2 |
|
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'; |
5 | 5 |
|
6 | 6 | import { |
7 | 7 | validateRlsPredicateEnforceability, |
8 | 8 | RLS_PREDICATE_UNENFORCEABLE, |
9 | 9 | RLS_PREDICATE_UNPARSEABLE, |
| 10 | + RLS_PREDICATE_OVER_BUDGET, |
10 | 11 | } from './validate-rls-predicate-enforceability.js'; |
11 | 12 | import { AUTHORING_RULES, runAuthoringRules } from './authoring-rules.js'; |
12 | 13 |
|
@@ -307,3 +308,202 @@ describe('validateRlsPredicateEnforceability — the verdict IS the RLSCompiler\ |
307 | 308 | .toContain(RLS_PREDICATE_UNENFORCEABLE); |
308 | 309 | }); |
309 | 310 | }); |
| 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