Skip to content

Commit 0d3ec47

Browse files
fix(plugin-security)!: a row-level policy whose compiled filter carries a null list member or null ordering bound fails closed on both clauses, so its read and its write check agree (#20212) (#20243)
Fixes #20212 Clause-②: no (narrowing) ## What changes `RLSCompiler.compileFilter` (`packages/plugins/plugin-security/src/rls-compiler.ts`) now runs the two shared comparand faces, `assertListComparandShapes` and `normalizeFilterComparandTypes` from `@objectstack/spec/data`, on every compiled policy filter, for `using` and `check` alike, with or without a field guard. A policy the faces refuse joins `deniedBy` with the new drop reason `refused-comparand`. That is the per-request route a list under `==` / `!=` (the type family) already takes. Alone, the clause answers `RLS_DENY_FILTER` with one `[RLS] DENY (fail closed)` WARN. Beside a policy that compiles, the refused policy drops out of the OR silently. The faces are called and their verdict is routed; no null rule is written in `plugin-security` or in a driver. A thrown value without the ADR-0112 envelope is re-thrown, not treated as a verdict. `@objectstack/lint`: the `rls-predicate-unenforceable` message for a null-family comparand said the RLS layer does not run the faces, so the policy survives and analytics refuses every query. This change made that false, so the message now gives the drop and the clause's own consequence, as it already does for a type fault. ## Seam: where the RLS filter is composed and where `check` is evaluated (H1, H2) - Composing layer: `SecurityPlugin`'s engine middleware, step 3 "RLS filter injection" (`computeLayeredRlsFilter`, then `RLSCompiler.compileFilter(…, 'using')`, then `andComposeLayers`), writes `opCtx.ast.where = { $and: [callerWhere, …] }`. - Evaluating layer for `check`: the write gate, step 3.6: `computeWriteCheckFilter`, then `RLSCompiler.compileFilter(…, 'check')`, then `matchesFilterCondition(image, filter)` (`@objectstack/formula`), in-process. - The engine runs the faces only in `lowerWhereFilterArray`, on the caller's `where`, before `executeWithMiddleware`. Nothing judges `opCtx.ast.where` after the middleware composes it. Measured: at base the null-family filter reached SqlDriver and was executed (P1 read returned `r_none`), where any face would have thrown `INVALID_FILTER`. `judgeFilter` runs the same stage 1 (`assertListComparandShapes`, `assertFilterIsMaterializable`, `assertTextOperatorTargetsAreStringCapable`, `assertTemporalComparandsInterpretable`, `normalizeFilterComparandTypes`, or the array branch's `parseFilterAST`) and stage 2 (`resolveWhereFilterTokens`), on the caller's `where` only. - Chosen seam: `RLSCompiler.compileFilter`, for both halves. Both clauses compile there (read middleware, write gate and analytics' `getReadFilter`), and the type family's `DENY (fail closed)` route already lives there. See Deviations. - The engine's composed-`where` seam gets no call. The other producers that compose onto `ast.where` are the Layer 0 tenant wall, controlled-by-parent (id lists), plugin-sharing (ids, owner, self scope), the attachment and comment access hooks, and the engine's own search filter. All are platform-produced; none carries an authored comparand. A call there could only answer a refusal as a 400 the caller cannot act on (the note in objectql's `filter-comparand-shape.ts` says why injected filters are not judged there), and it would never fire for any producer in this repository. ## Measured: base `805af4f290` against head (compiler blob `2438357788`) The real `SecurityPlugin` + `ObjectQL` stack, caller `usr_member`, rows `r_open` / `r_closed` / `r_none` (status NULL), on InMemoryDriver, SqlDriver (better-sqlite3) and SqlDriver on a private PostgreSQL 16 server. Each predicate was run once as `using` (find / findOne / count) and once as `check` (insert `closed`, insert `open`, by-id update to `closed`). | predicate | `using` read at base (memory / sqlite / pg) | `check` at base (closed / open / update) | head, every driver | | --- | --- | --- | --- | | P1 `!(record.status in ['open', null])` | `r_closed` / `r_none` / `r_none`, no WARN | ok / 403 / ok | 0 rows + WARN; 403 / 403 / 403 + WARN | | P2 `record.status in ['open', null]` | `r_none r_open` / `r_open` / `r_open` | 403 / ok / 403 | 0 rows + WARN; 403 / 403 / 403 + WARN | | P3 `record.status > null` (`$gt: null`) | none / none / none, no WARN | 403 / 403 / 403, no WARN | 0 rows + WARN; 403 / 403 / 403 + WARN | | P4 at-most `null` (`$lte: null`) | `r_none` / none / none | 403 / 403 / 403 | 0 rows + WARN; 403 / 403 / 403 + WARN | | P5 `record.status in [null]` | `r_none` / none / none | 403 / 403 / 403 | 0 rows + WARN; 403 / 403 / 403 + WARN | | control `record.status == current_user.positions` | 0 rows + WARN, all three | 403 x3 + WARN | byte-identical to base | | control `record.status != current_user.positions` | 0 rows + WARN, all three | 403 x3 + WARN | byte-identical to base | At base, P2 and P5 also admitted a NULL-status insert. At head that insert is refused too. - Read and check now agree on every row, and each null-family row answers exactly as the type-family control does, WARN included (`reason: 'refused-comparand'` against the control's `unsupported`). - Several policies, OR-combined (H3): P1 beside `record.status == 'open'` reads `r_open` with no WARN and admits `open` / refuses `closed`. That is exactly the control beside the same sibling. At base the P1 pair read `r_closed r_open` (memory) or `r_none r_open` (SQL) and admitted `closed`. - The caller's own `where` (H3), measured with `$in: ['open', null]`, `$not` of that, `$gt: null`, `$lte: null` and the array form: `INVALID_FILTER` / 400, same message, byte-identical base to head. - Null checks (H3): `record.status == null` and `!= null` are byte-identical base to head. - No collateral (H4): 243 cells per side (the three drivers). Every cell whose policy carries no null-family comparand is byte-identical base to head. That covers read rows, write answers, the RLS / Security WARN lines, `getReadFilter` and `explain`. The spread: `==`, `!=`, lists without null and their negation, ordering (`>`, `at-most`, a two-sided range), `== null` / `!= null`, `startsWith` / `endsWith` / `contains`, `in current_user.org_user_ids` and its negation, `current_user.id` / `.email`, an `||` and an `&&` composite, an emptied membership, an undeclared column, an unresolved `current_user` key, and tenant-scoped objects on SQLite and PostgreSQL (InMemoryDriver refuses multi-tenant objects with `MEMORY_MULTI_TENANT_UNSUPPORTED` at base and head alike). The only differing cells are the null-family ones: the five, the three multi-policy rows carrying one, and one tenant-scoped null-family row. ## Tests (at `2a0d7da0b1` unless stated) - New: `packages/plugins/plugin-security/src/rls-null-comparand-fails-closed.test.ts`. On the real stack, SqlDriver: the five shapes and the two controls, as `using` and as `check`; the sibling position for each family; null-check and list controls; the caller's own `where`. Also new: `rls-compiled-comparand-faces.test.ts`, a unit test on `compileFilter`: seven shapes x two clauses x with and without a guard, the sibling, two refused policies, and 18 accepted predicates that compile to exactly `compileCelToFilter`'s output with no WARN. 84 passed. - `pnpm --filter @objectstack/plugin-security test`: 139 files, 2839 tests passed. `run typecheck`: exit 0, `check:test-typecheck` OK. - `pnpm --filter @objectstack/lint test`: 109 files, 4232 passed (at `f3eb1d06c4`). `run typecheck`: exit 0 (at `4d66d4d572`; its lint source is unchanged since). - `pnpm --filter @objectstack/driver-memory test`: 56 files, 1362 passed (at `f3eb1d06c4`). - `pnpm --filter @objectstack/objectql test`: 318 files, 5762 passed (at `4d66d4d572`, before merging `origin/main`; this diff does not touch objectql). - `pnpm --filter @objectstack/driver-sql test` with `OS_TEST_POSTGRES_URL` set to a private PostgreSQL 16 server (zone `Asia/Shanghai`, process `TZ=America/New_York`): 196 files passed, 3 skipped, 3806 passed, 86 skipped (MySQL cells, not provisioned); live postgres RAN (at `4d66d4d572`). - Reverse verification at `2a0d7da0b1`: `scripts/ablation-replace.mjs` replaced the one `judgeCompiledComparands(outcome.filter)` call with a pass-through (anchor 1 to 0, blob `2438357788` to `e6a410b0b5`). The two new files went 42 failed, 42 passed out of 84: every null-family cell was red, and every control and accepted-predicate cell stayed green. The restore was verified by blob equal to HEAD and an empty `git diff HEAD`. The subject is imported from `src/` by relative path, so no `dist/` is on the resolution path. - `eslint --no-inline-config --format json` on the 5 changed TypeScript files: 5 files, 0 errors, 0 warnings. All 5 are inside eslint's population (`--print-config` resolves each). The config enables no type-aware linting (`parserOptions.project` / `projectService` null), so this diff cannot move any untouched file's verdict. ## Gates `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `2a0d7da0b1` derived 63 commands. `--ran` reconciliation: 63 accounted, 60 run, 3 NOT MEASURED, 0 unrun. Every run command exits 0 except `check-empty-changeset --base origin/main` (exit 1, by design; see Deliberate corrections). The six families the dispatch named that this diff no longer derives also exit 0: `check-engine-split-ratio` (both), `check:dispatcher-error-vocabulary`, `check:durability-log-level`, `check:stack-collection-maps` and `check:swallow-census-controls`. So do `check:authz-resolver`, `check:error-code-casing` and `check:filter-alias-parity`, and `node scripts/check-issue-citations.mjs --base 615c468` (6 citations resolve). - NOT MEASURED: `check:dual-build-cjs-loads`, `check:i18n`, `check:type-check-debt`. Reason: PREREQUISITE NOT MET (exit 3), each needs the whole-workspace build; CI builds it. ## Deliberate corrections of two pending changesets This change made one sentence false in each of two pending release notes, and each gets a one-clause correction. `check-empty-changeset` stays red on both by design, which puts the decision in front of a person. Please confirm. - `.changeset/19951-rls-lint-per-request-refusals.md` said "The RLS layer does not run that check on its own filter, so the backend answers with its own semantics … Every analytics query over the object is refused." It now says the RLS compiler runs that check too and drops the policy: reads return no rows, and `check` writes are refused 403. - `.changeset/20018-native-read-scope-faces.md` said such a policy "now gets the withheld 500 on every analytics face". It now says the compiler drops the policy, so the analytics faces receive the deny sentinel and answer zero rows. ## Deviations - The seam is `RLSCompiler.compileFilter`, not "the one engine seam" in triage's words. The write check never passes the engine's composed `where`, so a call only there would have flipped the disagreement: the read would deny while the check admits. The compile step is the one place both clauses (and analytics' read scope) pass, and the type family's fail-closed route lives there. The dispatch's ruling section anticipated this choice. - The real-stack pin runs on SqlDriver only. Triage's execution note 1 names InMemoryDriver too, and it was measured (table above, base and head), but a new test consumer of `@objectstack/driver-memory` is a ledgered, maintainer-ruled decision (`pnpm check:driver-memory-census`: "Is this consumer legitimate? is a maintainer ruling"). A first version of the pin added the devDependency and the census went red, so it was reverted. The open question is in the report. - `packages/lint` is off the claim's file surface. Its finding text read false after this change, so it was corrected in the same PR. ## Acceptance notes - Analytics (execution note 2; #20010's door is untouched): measured with the three analytics faces (ObjectQL execute, the `/analytics/sql` echo run on the database, NativeSQL execute). With the base read scope of each null-family policy, all three answer `READ_SCOPE_COMPILE_FAILED` / 500. With the head read scope (`getReadFilter` now answers the deny sentinel), all three answer zero rows. So a null-family policy now answers the same on the data path and the analytics path. Beside a granting sibling, analytics moves from refusing the whole query to serving the sibling's rows, as it already did for the type family. - `explain` read verdict: `rls: narrows` at base, `rls: denies` at head, the same as the type-family control. - driver-memory's "constructively unreachable through the compile face" note in `memory-matcher.ts` is now true on the RLS path too. The head column shows no null-family filter reaches any driver. That file is not edited. - `packages/lint/src/validate-rls-predicate-enforceability.ts` still says the analytics read-scope compiler runs the faces on every RLS read scope. That stays true, but a null-family policy no longer produces such a scope. Noted, not edited. --- _Generated by [Claude Code](https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent fdb2669 commit 0d3ec47

8 files changed

Lines changed: 570 additions & 27 deletions

‎.changeset/19951-rls-lint-per-request-refusals.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
- FROM silent TO `rls-predicate-unenforceable`: `record.f in current_user`, `record.f.startsWith(current_user)`, `.endsWith(current_user)` and `.contains(current_user)`. Name the key that holds the value, for example `record.f in current_user.org_user_ids` or `record.f.startsWith(current_user.email)`.
1515
- FROM silent TO `rls-predicate-unenforceable`: `record.f.startsWith(current_user.org_user_ids)` (a list handed to a string method). Write `record.f in current_user.org_user_ids`, or pass a one-value key.
1616

17-
**A `null` list member or `null` ordering bound.** The platform refuses both in every filter it is sent. The RLS layer does not run that check on its own filter, so the backend answers with its own semantics: on a SQL driver `record.f in [null]`, `!(record.f in ['a', null])` and `record.f > null` read no rows. Every analytics query over the object is refused.
17+
**A `null` list member or `null` ordering bound.** The platform refuses both in every filter it is sent. The RLS compiler runs that check on its own filter too (#20212) and drops the policy on every request: reads return no rows and `check` writes are refused 403.
1818

1919
- FROM silent TO `rls-predicate-unenforceable`: `record.f in ['a', null]` and `!(record.f in ['a', null])`. Write `(record.f in ['a'] || record.f == null)`, or drop the `null` member.
2020
- FROM silent TO `rls-predicate-unenforceable`: `record.f in [null]`. Write `record.f == null`.

‎.changeset/20018-native-read-scope-faces.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ each is refused by a standing ruling the shared faces carry. Measured on SQLite:
4040
analytics face already refused all of them. No in-repo read-scope producer emits
4141
them for a policy in this repository. An RLS `using` predicate can still be
4242
written so that it lowers into the null shapes (a literal `null` inside an `in`
43-
list, or an ordering comparison against `null`), and such a policy now gets the
44-
withheld 500 on every analytics face.
43+
list, or an ordering comparison against `null`), but the RLS compiler drops such a
44+
policy (#20212), so the analytics faces receive the deny sentinel and answer zero rows.
4545

4646
**Fix.** State absence with the null predicate. "One of these values, or no
4747
value" is `{ "$or": [{ "f": { "$in": ["a"] } }, { "f": { "$null": true } }] }`,
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@objectstack/plugin-security": minor
3+
"@objectstack/lint": patch
4+
---
5+
6+
fix(plugin-security)!: a row-level policy whose compiled filter carries a `null` list member or a `null` ordering bound now fails closed on both clauses, so its read and its write check agree (#20212)
7+
8+
Clause-②: no (narrowing)
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) an enforcement change in the RLS compiler: no authorable key, spelling or stored shape moves, so a stored `sys_metadata` row needs no conversion. The two shapes were ruled at the shared comparand face with no ledger entry, because which explicit spelling matches the author's intent is an authoring decision no migration entry can perform. `os validate` already reports each such predicate (`rls-predicate-unenforceable`) with a rewrite. -->
11+
12+
**BREAKING**: this narrows what a row-level policy grants. A policy that returned rows on a read, or admitted a write, can now return no rows and refuse the write with 403. It ships as `minor` under the launch-window convention for accept-set narrowings.
13+
14+
`RLSCompiler.compileFilter` now runs the platform's two shared comparand faces (`assertListComparandShapes` and `normalizeFilterComparandTypes` from `@objectstack/spec/data`) on every compiled policy filter, for `using` and `check` alike. These are the functions the engine already runs on a caller's own `where`. It runs them before the middleware chain adds the RLS filter to that `where`, so until now a compiled policy filter reached the driver unjudged. A policy they refuse is dropped the way a policy with an unresolved `current_user` variable, or a list under `==`, is already dropped. When no other applicable policy compiles, the clause answers `RLS_DENY_FILTER` and logs one `[RLS] DENY (fail closed)` WARN with `reason: 'refused-comparand'`.
15+
16+
**Why.** The rulings refuse a `null` member of `$in` / `$nin` and a `null` comparand of `$gt` / `$gte` / `$lt` / `$lte` in every filter, because no two backends agree on what they match. On the RLS path they reached the backend, and the `check` clause of the same policy was evaluated in-process by another matcher. One policy then gave two answers. Measured with rows `open`, `closed` and a NULL status:
17+
18+
| predicate | `using` read before, SqlDriver / InMemoryDriver | `check` insert `closed` / `open` before | after, both clauses |
19+
| --- | --- | --- | --- |
20+
| `!(record.status in ['open', null])` | the NULL row / the `closed` row | admitted / 403 | no rows, 403 |
21+
| `record.status in ['open', null]` | the `open` row / the `open` and NULL rows | 403 / admitted | no rows, 403 |
22+
| `record.status > null` | no rows / no rows | 403 / 403 | no rows, 403 |
23+
| `record.status <= null`, `record.status in [null]` | no rows / the NULL row | 403 / 403 | no rows, 403 |
24+
25+
On SqlDriver the first row's read hid the `closed` row that its own write check admitted. PostgreSQL answered as SQLite.
26+
27+
**What now answers differently.**
28+
29+
- A read (`find`, `findOne`, `count`) under such a policy returns no rows when no other applicable policy compiles, and logs the WARN. Beside another policy that compiles, this policy no longer contributes rows: the read returns what the other policies grant, with no WARN.
30+
- A `check` (declared, or defaulted from `using`) refuses every insert and update it governs with the row-level CHECK denial, `403 PERMISSION_DENIED`, when no other applicable `check` compiles.
31+
- `explain` reports the RLS layer as `denies` instead of `narrows`.
32+
- Analytics: `getReadFilter` hands the deny sentinel to the analytics faces, which answer zero rows. They previously refused the whole query with `READ_SCOPE_COMPILE_FAILED` / 500.
33+
34+
**Who is affected.** A deployment whose stored policies carry one of these shapes, for example a policy saved without `os validate`. No policy in this repository does: every `using` / `check` string under `examples/` and `packages/` (outside tests) that names `null` is a null check (`== null`, `!= null`), which is unchanged.
35+
36+
**Fix.** Test for no value with `== null` and for a value with `!= null`. "One of these, or no value" is `record.status in ['open'] || record.status == null`. "Has a value" is `record.status != null`. `os validate` prints the rewrite for each finding.
37+
38+
**Unchanged.** A policy whose compiled filter the faces accept compiles to the same filter as before, with the same WARNs. A caller's own `where` carrying these shapes is still refused `INVALID_FILTER` / 400 by the engine. The null checks `record.f == null` / `record.f != null` lower to `$null` and are not refused.
39+
40+
`@objectstack/lint`: the `rls-predicate-unenforceable` finding for a `null` list member or `null` ordering bound now says what the runtime does: the policy is dropped on every request, with the clause's own fail-closed consequence. It used to say the policy survived and the backend answered.

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -837,10 +837,10 @@ describe('validateRlsPredicateEnforceability — the refusals the SHAPE check ca
837837
* the policy: a `using` read returns zero rows (with only a per-request
838838
* "DENY (fail closed)" WARN) and a `check` write is refused with 403.
839839
* - NULL comparands — the shared filter faces refuse a `null` list member and
840-
* a `null` ordering bound by ruling. They compile, the RLS layer never runs
841-
* the faces on its own filter, and the backend answers: on driver-sql
842-
* `x in [null]`, `!(x in ['a', null])` and `x > null` read nothing, while
843-
* the `check` evaluator admits a write the negated read hides.
840+
* a `null` ordering bound by ruling. They compile, and `RLSCompiler` runs
841+
* the same faces on the compiled filter (#20212), so it drops the policy
842+
* exactly as it drops a TYPE fault: zero rows with the per-request WARN on
843+
* a `using` read, a 403 on a `check` write.
844844
*/
845845
const TYPE_FAULTS: ReadonlyArray<readonly [string, string]> = [
846846
[
@@ -904,10 +904,14 @@ describe('validateRlsPredicateEnforceability — the refusals the SHAPE check ca
904904
expect(check.message).toMatch(/PermissionDeniedError/);
905905
});
906906

907-
it('a NULL comparand is NOT reported as a drop — the policy survives and the backend answers', () => {
908-
const [f] = validateRlsPredicateEnforceability(siteWith('using', NULL_COMPARANDS[2][0]));
909-
expect(f.message).toMatch(/the policy is NOT dropped/);
910-
expect(f.message).not.toMatch(/DROPS the policy/);
907+
it('a NULL comparand is reported as a DROP with the clause’s own consequence, as the runtime drops it', () => {
908+
const [using] = validateRlsPredicateEnforceability(siteWith('using', NULL_COMPARANDS[2][0]));
909+
expect(using.message).toMatch(/DROPS the policy on EVERY request/);
910+
expect(using.message).toMatch(/RLS_DENY_FILTER/);
911+
912+
const [check] = validateRlsPredicateEnforceability(siteWith('check', NULL_COMPARANDS[2][0]));
913+
expect(check.message).toMatch(/DROPS the policy on EVERY request/);
914+
expect(check.message).toMatch(/PermissionDeniedError/);
911915
});
912916

913917
it('rewrites EVERY site the fault lands in, in one finding', () => {

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

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -815,9 +815,9 @@ const HOLDING_RULE: Readonly<Record<RuntimeHolding, string>> = {
815815
* The verdict of the shared filter faces on this predicate's LOWERED filter —
816816
* `assertListComparandShapes` and `normalizeFilterComparandTypes`
817817
* (`@objectstack/spec/data`), the same pair the analytics read-scope compiler
818-
* runs on every RLS read scope (`assertReadScopeComparandsRunnable`) and the
819-
* engine runs on every caller-supplied filter. The RLS data path does not run
820-
* them on its own filter, which is why a refused comparand gets through there.
818+
* runs on every RLS read scope (`assertReadScopeComparandsRunnable`), the
819+
* engine runs on every caller-supplied filter and, since #20212, `RLSCompiler`
820+
* runs on every compiled policy filter, which drops a policy they refuse.
821821
*
822822
* Graded by the refusal's `code` (`INVALID_FILTER`), never by its prose (#6223);
823823
* the returned message is quoted, never parsed.
@@ -891,16 +891,17 @@ function firstSentence(message: string): string {
891891
}
892892

893893
/**
894-
* What a face-refused comparand does at request time. Not a drop: the compile
895-
* succeeds and the policy's filter reaches the backend unjudged.
894+
* How a face-refused comparand is dropped at request time. [#20212]
895+
* `RLSCompiler.compileFilter` runs the same two faces on every compiled policy
896+
* filter, for `using` and `check` alike, so the policy joins the per-request
897+
* denial route before any backend or the in-process `check` evaluator sees the
898+
* comparand: the same drop, WARN and per-clause consequence as a TYPE fault.
896899
*/
897-
const FACE_REFUSED_CONSEQUENCE =
898-
'The RLS layer does not pass its own filter through that check, so the policy is NOT dropped: the refused ' +
899-
'comparand reaches the backend, which answers it with semantics the platform has ruled undefined — backends ' +
900-
'disagree, and on the SQL drivers a `null` member or bound matches no row, so a negated list holding `null` ' +
901-
'or an ordering against `null` can admit NOTHING. One policy can even disagree with itself: a `using` read ' +
902-
'on a SQL driver hides rows that its `check` evaluator admits on write. Every analytics query over the ' +
903-
'object is refused outright, because its read-scope compiler runs the same check.';
900+
const DROPPED_FACE_REFUSED =
901+
'so `RLSCompiler` DROPS the policy on EVERY request: it runs that same check on every compiled policy ' +
902+
'filter, before any backend sees it. The shape check passes, so the "uncompilable predicate" WARN is never ' +
903+
'logged; the only signal is a per-request "DENY (fail closed)" WARN, emitted only when nothing else ' +
904+
'applicable compiles. ';
904905

905906
/**
906907
* What a reference miss costs at request time, per clause. Measured, not inferred.
@@ -1075,7 +1076,7 @@ function referenceFindings(
10751076
path,
10761077
message:
10771078
`RLS ${clause} \`${quote(source)}\` lowers, but to a comparand the platform's shared filter check ` +
1078-
`refuses (${firstSentence(faced)}). ${FACE_REFUSED_CONSEQUENCE}`,
1079+
`refuses (${firstSentence(faced)}), ${consequence(clause, DROPPED_FACE_REFUSED)}`,
10791080
hint:
10801081
rewrites.length > 0
10811082
? '`null` has no place inside a list or opposite an ordering operator — the platform refuses both in ' +
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#20212] `RLSCompiler.compileFilter` runs the platform's two shared comparand
5+
* faces (`assertListComparandShapes`, `normalizeFilterComparandTypes`,
6+
* `@objectstack/spec/data`) on every compiled policy filter, for `using` and
7+
* `check` alike, with or without a field guard.
8+
*
9+
* What these pins hold, unit-side (the real-stack half is
10+
* `rls-null-comparand-fails-closed.test.ts`):
11+
* 1. a policy whose compiled filter the faces refuse is DROPPED onto the
12+
* per-request fail-closed route: alone, the clause answers `RLS_DENY_FILTER`
13+
* and one `DENY (fail closed)` WARN carrying `reason: 'refused-comparand'`;
14+
* 2. beside a granting sibling it vanishes from the OR in silence, exactly as a
15+
* per-request refusal does;
16+
* 3. every other compiled filter passes through unchanged: the faces add
17+
* nothing to a filter they accept (they return it by reference).
18+
*/
19+
20+
import { describe, it, expect, vi } from 'vitest';
21+
import type { RowLevelSecurityPolicy } from '@objectstack/spec/security';
22+
import { compileCelToFilter } from '@objectstack/formula';
23+
24+
import { RLSCompiler, RLS_DENY_FILTER } from './rls-compiler.js';
25+
26+
type WarnCall = [string, Record<string, unknown>?];
27+
28+
function compilerWithLogger() {
29+
const warn = vi.fn();
30+
const compiler = new RLSCompiler();
31+
compiler.setLogger({ warn });
32+
return { compiler, warn };
33+
}
34+
35+
function policy(clause: 'using' | 'check', predicate: string, name = 'status_scope'): RowLevelSecurityPolicy {
36+
return { name, object: 'ticket', operation: 'all', [clause]: predicate } as RowLevelSecurityPolicy;
37+
}
38+
39+
const CTX = {
40+
userId: 'u1',
41+
tenantId: 'org-1',
42+
positions: ['p_sales'],
43+
org_user_ids: ['u1', 'u2'],
44+
email: 'u1@example.com',
45+
} as never;
46+
47+
/** The field guard the plugin passes when the schema resolves. */
48+
const GUARD = { declared: new Set(['id', 'status', 'owner', 'score']) };
49+
50+
const NULL_FAMILY = [
51+
"!(record.status in ['open', null])",
52+
"record.status in ['open', null]",
53+
'record.status > null',
54+
'record.status >= null',
55+
'record.status < null',
56+
'record.status <= null',
57+
'record.status in [null]',
58+
] as const;
59+
60+
describe('[#20212] a compiled filter the shared faces refuse is dropped, fail closed', () => {
61+
for (const clause of ['using', 'check'] as const) {
62+
for (const guard of [undefined, GUARD]) {
63+
for (const predicate of NULL_FAMILY) {
64+
it(`${clause} \`${predicate}\` (${guard ? 'with' : 'without'} a field guard) → RLS_DENY_FILTER, one refused-comparand WARN`, () => {
65+
const { compiler, warn } = compilerWithLogger();
66+
67+
const filter = compiler.compileFilter([policy(clause, predicate)], CTX, clause, guard);
68+
69+
expect(filter).toBe(RLS_DENY_FILTER);
70+
expect(warn).toHaveBeenCalledTimes(1);
71+
const [message, meta] = warn.mock.calls[0] as WarnCall;
72+
expect(message.startsWith('[RLS] DENY (fail closed)')).toBe(true);
73+
expect(meta).toMatchObject({
74+
object: 'ticket',
75+
policy: 'status_scope',
76+
clause,
77+
reason: 'refused-comparand',
78+
filter: RLS_DENY_FILTER.id,
79+
});
80+
expect(String(meta?.detail)).toContain('INVALID_FILTER');
81+
});
82+
}
83+
}
84+
}
85+
86+
it('beside a granting sibling the refused policy vanishes from the OR, with no WARN, like a per-request refusal', () => {
87+
for (const clause of ['using', 'check'] as const) {
88+
const results: unknown[] = [];
89+
for (const refused of [NULL_FAMILY[0], 'record.status == current_user.positions']) {
90+
const { compiler, warn } = compilerWithLogger();
91+
const filter = compiler.compileFilter(
92+
[policy(clause, refused, 'refused'), policy(clause, "record.status == 'open'", 'sibling')],
93+
CTX,
94+
clause,
95+
GUARD,
96+
);
97+
results.push({ filter, warned: warn.mock.calls.length });
98+
}
99+
expect(results, clause).toEqual([
100+
{ filter: { status: 'open' }, warned: 0 },
101+
{ filter: { status: 'open' }, warned: 0 },
102+
]);
103+
}
104+
});
105+
106+
it('two refused policies deny with one line each', () => {
107+
const { compiler, warn } = compilerWithLogger();
108+
const filter = compiler.compileFilter(
109+
[policy('using', 'record.status > null', 'a'), policy('using', 'record.status in [null]', 'b')],
110+
CTX,
111+
);
112+
expect(filter).toBe(RLS_DENY_FILTER);
113+
expect((warn.mock.calls as WarnCall[]).map(([, meta]) => [meta?.policy, meta?.reason])).toEqual([
114+
['a', 'refused-comparand'],
115+
['b', 'refused-comparand'],
116+
]);
117+
});
118+
});
119+
120+
describe('[#20212] CONTROL — a compiled filter the faces accept passes through unchanged', () => {
121+
const ACCEPTED = [
122+
"record.status == 'open'",
123+
"record.status != 'open'",
124+
"record.status in ['open', 'pending']",
125+
"!(record.status in ['open', 'pending'])",
126+
'record.score > 2',
127+
'record.score <= 2',
128+
'record.score >= 1 && record.score < 5',
129+
'record.status == null',
130+
'record.status != null',
131+
"record.status.startsWith('op')",
132+
"record.status.endsWith('ed')",
133+
"record.status.contains('lose')",
134+
'record.owner in current_user.org_user_ids',
135+
'!(record.owner in current_user.org_user_ids)',
136+
'record.owner == current_user.id',
137+
'record.owner != current_user.id',
138+
'record.owner == current_user.email',
139+
"record.status == 'open' || record.owner == current_user.id",
140+
];
141+
142+
for (const clause of ['using', 'check'] as const) {
143+
for (const predicate of ACCEPTED) {
144+
it(`${clause} \`${predicate}\``, () => {
145+
const { compiler, warn } = compilerWithLogger();
146+
const compiled = compileCelToFilter(predicate, {
147+
variables: {
148+
current_user: { id: 'u1', organization_id: 'org-1', positions: ['p_sales'], org_user_ids: ['u1', 'u2'], email: 'u1@example.com' },
149+
},
150+
});
151+
expect(compiled.ok, predicate).toBe(true);
152+
153+
const filter = compiler.compileFilter([policy(clause, predicate)], CTX, clause, GUARD);
154+
155+
expect(filter).toEqual(compiled.ok ? compiled.filter : undefined);
156+
expect(warn).not.toHaveBeenCalled();
157+
});
158+
}
159+
}
160+
});

0 commit comments

Comments
 (0)