Skip to content

Commit af32cf9

Browse files
fix(lint): RLS predicate enforceability reports the per-request refusals the shape check cannot see (#20210)
Fixes #19951 Clause-②: no `validateRlsPredicateEnforceability` judged an RLS predicate's shape with every `current_user` value replaced by a placeholder, so a predicate the runtime refuses because of a value passed `os validate` cleanly and enforced nothing. The rule's reference pass now reports two such classes as `rls-predicate-unenforceable` (`error`), each with a pasteable rewrite that uses the predicate's own field and key: 1. **A `current_user` value of the wrong type for its position.** Examples: a membership set under `==` / `!=` or handed to a string method, a one-value key on the right of `in`, the bare `current_user` object under `in` or a string method. The compiler refuses these on every request and `RLSCompiler` drops the policy. 2. **A lowered comparand the shared filter faces refuse by ruling.** A `null` list member or a `null` ordering bound. These compile, and the RLS layer never runs the faces on its own filter, so the backend answers with semantics the platform left undefined. Only `packages/lint/src/validate-rls-predicate-enforceability.ts`, its test and one changeset change. No runtime source is touched (formula lowering, `RLSCompiler` and the shared comparand faces are unchanged). ## Shape table (Zone 2, item 1) Lint readings come from a stack that declares the fields. Runtime readings come from a throwaway probe (never committed) built on the `rls-list-literal-comparand-fails-closed.test.ts` harness: real `SecurityPlugin` + ObjectQL + `SqlDriver` on better-sqlite3, caller `usr_member`, rows `r_open` (status open) and `r_closed` (status closed, reviewer usr_member), and `check` inserts of one `open` and one `closed` row. "main" is `560b724c`; "this PR" is `77060e6d`. | authored predicate | lint on main | lint, this PR | `using` read (runtime) | `check` write (runtime) | |:--|:--|:--|:--|:--| | `record.reviewer_id != current_user.org_user_ids` | 0 | unenforceable | 0 rows, `DENY (fail closed)` WARN | 403 / 403 | | `!(record.reviewer_id == current_user.org_user_ids)` | 0 | unenforceable | 0 rows, `DENY (fail closed)` | 403 / 403 | | `record.reviewer_id == current_user.positions` | 0 | unenforceable | 0 rows, `DENY (fail closed)` | 403 / 403 | | `record.reviewer_id in current_user.id` | 0 | unenforceable | 0 rows, `DENY (fail closed)` | 403 / 403 | | `record.reviewer_id in current_user` | 0 | unenforceable | 0 rows, `DENY (fail closed)` | 403 / 403 | | `record.reviewer_id.startsWith(current_user)` | 0 | unenforceable | 0 rows, `DENY (fail closed)` | 403 / 403 | | `record.reviewer_id.contains(current_user)` | 0 | unenforceable | 0 rows, `DENY (fail closed)` | 403 / 403 | | `record.reviewer_id.startsWith(current_user.org_user_ids)` | 0 | unenforceable | 0 rows, `DENY (fail closed)` | 403 / 403 | | `record.status in ['open', null]` | 0 | unenforceable | `r_open` (acts as `in ['open']`), no WARN | open admitted / closed 403 | | `record.status in [null]` | 0 | unenforceable | 0 rows, no WARN | 403 / 403 | | `!(record.status in ['open', null])` | 0 | unenforceable | **0 rows** (`r_closed` hidden), no WARN | open 403 / **closed admitted** | | `record.status > null` | 0 | unenforceable | 0 rows, no WARN | 403 / 403 | | `record.status <= null` | 0 | unenforceable | 0 rows, no WARN | 403 / 403 | | `record.reviewer_id != current_user` | unenforceable (since #20189) | unenforceable (unchanged) | 0 rows, `uncompilable predicate` WARN | 403 / 403 | | CONTROL `record.status != ['closed', 'archived']` | unenforceable | unenforceable (unchanged) | 0 rows, `uncompilable predicate` WARN | 403 / 403 | | CONTROL `record.reviewer_id == current_user.id` | 0 | 0 | `r_closed` | per value | | CONTROL `!(record.reviewer_id in current_user.org_user_ids)` | 0 | 0 | `r_open` | admitted | | CONTROL (probe artefact) `current_user.email == 'member@example.com'` | 0 | 0 | every row for that caller | admitted | | CONTROL (probe artefact) `current_user.email == 'someone@else.com'` | 0 | 0 | 0 rows, `DENY (fail closed)` | 403 | Reading of Zone 2's assumption 1: - It holds for the type family: the runtime refuses those predicates on every request while the lint reports 0. - It is **falsified for the null family**. The null shapes are not refused on the RLS data path: they compile, reach the driver, and are answered with SQL null semantics. Within one policy, the `using` read and the `check` evaluator disagree on `!(record.status in ['open', null])`. The shared faces refuse the same comparands wherever those faces run: the engine seam on every caller-supplied filter, and `assertReadScopeComparandsRunnable` in analytics (read from code, `service-analytics/src/read-scope-sql.ts`; not re-measured here). The data-path gap is in the Acceptance notes and the report, not addressed here. - `f != current_user` was already moved by #20189 and stays reported by the shape branch. ## The discriminator (Zone 2, item 2) **Type family: a one-reference type swap must make the same compile lower** (`attributeTypeFault`). When the reference pass's probe compile is refused `unsupported`, the rule rebinds ONE reference to a value of the other runtime type and compiles again. The candidates are each kernel key (scalar to list, or list to scalar) and the bare root (as a list that still carries the keys, then as one string). If that single swap makes the predicate lower, and the swapped value lands in a lowered field position, the refusal came from that reference's type. The type is not the probe's to choose: `ExecutionContextSchema` declares it for the kernel keys (read via `kernelKeyProbe`), and the root is always the whole context object. So the runtime refuses the same position on every request. A refusal no single swap cures reports nothing. That is the probe-artefact class. A constant comparison with no field, such as `current_user.email == 'ops@acme.com'`, folds on the value: it lowers to "no restriction" for the caller it names and is refused for everyone else. The swap value begins with the probe string and never equals it, so ordering folds and equality folds stay refused under every swap. The pinned control asserts `compileCelToFilter` really answers `{ ok: true, filter: {} }` for the named caller and `unsupported` for another, then asserts the lint stays silent on both clauses, alone and inside `|| record.owner_id == current_user.id`. **Null family: the verdict of the same two faces analytics runs** (`sharedFaceRefusal`): `assertListComparandShapes` + `normalizeFilterComparandTypes` from `@objectstack/spec/data`, run on the probe-compiled filter and graded by the refusal's `code` (`INVALID_FILTER`), never its prose. It is not a probe artefact, because the probe binds only strings and string lists, which both faces accept by construction; a probe carrying a nested record is not judged. The face-probe control `record.assigned_to_id in current_user.org_user_ids` stays clean. Conservative by construction, and recorded in the docblock: a predicate with two type faults (no single swap cures it) stays silent. ## New author-facing text (quoted verbatim; `KEY` stands where the shipped string spells an angle-bracketed placeholder) Type-family message: ``RLS CLAUSE `SOURCE` passes the shape check, but the compiler refuses it for the value `VARIABLE` holds on every request (COMPILER_DETAIL). HOLDING_SENTENCE `` + the existing clause consequence with a new preamble: > so `RLSCompiler` DROPS the policy on EVERY request (a request that resolves no value drops it too). The shape check passes, so the "uncompilable predicate" WARN is never logged; the only signal is a per-request "DENY (fail closed)" WARN, emitted only when nothing else applicable compiles, and nothing reports it at authoring time. Holding sentences: > `current_user.org_user_ids` is a membership set: `ExecutionContext` declares it, and the kernel resolves it, as a LIST on every request, so no request can ever put one value in this position. > `current_user.id` holds ONE value: `ExecutionContext` declares it, and the kernel resolves it, as a scalar on every request, so no request can ever put a list in this position. > `current_user` alone is the whole caller context object on every request, never one value and never one set. Hint leads, one per holding: > `==` / `!=` compare ONE value and a string method takes ONE string; a membership set is tested with `in` — "one of these" is `x in set`, "none of these" is `!(x in set)`. > `in` tests membership in a SET; one value is compared with `==` / `!=`. > `current_user` is the caller context, not a value: name the key that holds what you mean. Each hint lead is followed by `In this predicate: ` and the rewrites, for example: - ``replace `record.reviewer_id != current_user.org_user_ids` with `!(record.reviewer_id in current_user.org_user_ids)` `` - ``replace `record.reviewer_id == current_user.org_user_ids` with `record.reviewer_id in current_user.org_user_ids` `` - ``replace `record.reviewer_id in current_user.id` with `record.reviewer_id == current_user.id` (and `!(record.reviewer_id in current_user.id)` with `record.reviewer_id != current_user.id`); for a set, name a membership key: `record.reviewer_id in current_user.accessible_org_ids` / … `org_user_ids` / … `positions` `` - ``replace `record.reviewer_id in current_user` with the key that holds the set: `record.reviewer_id in current_user.accessible_org_ids` / … `org_user_ids` / … `positions`, or an app-staged §7.3.1 set `record.reviewer_id in current_user.KEY` `` - ``replace `record.reviewer_id.startsWith(current_user)` with the key that holds the string: `record.reviewer_id.startsWith(current_user.email)` / … `(current_user.id)` / … `(current_user.organization_id)` `` The key lists are derived from the declared types (`kernelKeysHolding`) and are not transcribed. Null-family message: ``RLS CLAUSE `SOURCE` lowers, but to a comparand the platform's shared filter check refuses (FACE_FIRST_SENTENCE). `` + this consequence: > The RLS layer does not pass its own filter through that check, so the policy is NOT dropped: the refused comparand reaches the backend, which answers it with semantics the platform has ruled undefined — backends disagree, and on the SQL drivers a `null` member or bound matches no row, so a negated list holding `null` or an ordering against `null` can admit NOTHING. One policy can even disagree with itself: a `using` read on a SQL driver hides rows that its `check` evaluator admits on write. Every analytics query over the object is refused outright, because its read-scope compiler runs the same check. Null-family hint: > `null` has no place inside a list or opposite an ordering operator — the platform refuses both in every filter it is sent, because no two backends agree on what they match. Test for no value with `== null` and for a value with `!= null`: replace `record.status in ["open", null]` with `(record.status in ["open"] || record.status == null)` to keep matching a missing value, or drop the member: `record.status in ["open"]`. Its other rewrites are ``replace `record.status in [null]` with `record.status == null` `` and ``replace `record.status > null` with `record.status != null` (has a value) or `record.status == null` (has none), or compare against a real bound``. Every rewrite is a local replacement, so it stays correct under any enclosing `!`, `&&` or `||`. If the face refuses something the locator cannot place, the hint falls back to the existing lowerable-subset sentence. The existing `consequence(clause)` text for uncompilable predicates is byte-identical; it only became parameterised. ## Pins - **Flipped:** `accepts every kernel-resolved key in BOTH positions` asserted `[]` for `owner_id == current_user.KEY` and `owner_id in current_user.KEY` for every kernel key, which wrote the blind spot down as a pin. It is now `never reports a kernel-resolved key as UNKNOWN — in its own position it is clean, in the other it is a type fault`. It asserts, per key and by its runtime type (the table hoisted to `KERNEL_KEY_RUNTIME_TYPE`), `[]` in the key's own position and exactly `[rls-predicate-unenforceable]` in the other, and never `rls-predicate-unknown-user-variable`. - **Pin sweep, repo-wide:** outside `packages/lint`, no test asserts an `rls-predicate-*` id. The one hit is prose in `packages/qa/dogfood/test/authz-conformance.matrix.ts`. The shape-branch pins (literal list, bare root, `size()`, SQL `AND`, over-budget), the verdict-parity corpus and the disjointness test are unchanged and green. A genuinely illegal shape keeps its refusal verbatim. - **New block** `the refusals the SHAPE check cannot see are reported (#19951)`, table-driven over both clauses: - 8 type-fault rows and 7 null rows; each asserts `isSupportedRlsExpression(source) === true` (why it was silent), then exactly `[[rls-predicate-unenforceable, permissions[0].rowLevelSecurity[0].CLAUSE]]`, and that the hint contains the pasteable rewrite. - Message substance for each family: a DROP with `RLS_DENY_FILTER` on `using` and `PermissionDeniedError` on `check`, versus "NOT dropped". - Every site rewritten in one finding. - Controls: the literal list keeps one finding, and every spelling the hints point at is clean on both clauses. - The probe-artefact control. - Reach through `runAuthoringRules('validate', …)`. ## Verification (at `77060e6d`, the merge of `origin/main` `0bd11261`) - `pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2 --reporter=verbose src/validate-rls-predicate-enforceability.test.ts`: `Tests 120 passed (120)`. - `pnpm --filter @objectstack/lint test`: `Test Files 108 passed (108)`, `Tests 4204 passed (4204)`. This is the whole package, the rule's only home. - `pnpm --filter @objectstack/lint typecheck`: exit 0. `tsc --noEmit`, then `check:test-typecheck` `OK` against `tsconfig.test.json`. `--listFiles` includes the test file, and it carries 0 type errors; the 6 ledgered test-layer errors live in two other files. - `pnpm --filter @objectstack/lint build`: exit 0. - **Ablations**, one-time and never left in the tree. Each went through `node scripts/ablation-replace.mjs` in WRAP mode, with the anchor hit once (1 to 0), a new blob, and the restore proven (blob == HEAD `8589ec8c`, `git diff HEAD` empty). The subject is imported from `src`, so no `dist` step applies. - **A.** `attributeTypeFault(...)` replaced by `?? FAKE_FAULT` (every refusal reported). Result: 2 failed / 118. Red: the probe-artefact control, and `§7.3.1 … a key used in BOTH positions takes the membership answer`. That is the discriminator is load-bearing. - **B.** `sharedFaceRefusal` made to return `null`. Result: 8 failed / 112, the 7 null rows plus "NOT reported as a drop". - **C.** The swap may never land (`sites.length > 0` becomes `< 0`). Result: 12 failed / 108: the 8 type rows, the flipped kernel-key pin, the message test, the every-site test, and the `os validate` reach. - **Census (Zone 2, item 4):** the BASE rule (`git show 560b724:…`, blob `cb850a12`, as a throwaway module) and this PR's rule were run in one process over every `using` / `check` literal. Outside tests: 126 predicates, 77 of them `current_user` (the note's control of 77 reproduced), **0 changed**. Including every test fixture: 327 predicates, 205 `current_user`, **0 changed**. The 4 template-literal predicates the extraction skipped (`${ownerField} == current_user.id` and similar) and the one JSON fixture are all type-correct. No in-repo producer is newly flagged. - **Interplay (Zone 2, item 5):** the null-family check calls the shared face `assertListComparandShapes` (`packages/spec/src/data/filter-comparand-shape.ts`) as it stands on `main`. #19886 stage 2b edits that file's `$ne` / array arm. The lint only ever hands it filters the compiler already accepted, and the compiler refuses `==` / `!=` against a list, so a `$ne` array never reaches the face from here. Measured on `main`; not waited on. - **Gates:** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, re-derived at `77060e6d` (59 commands, a superset of the lead's 52 once the changeset exists), all run. Reconciled with `--ran`: `59 derived, 57 run, 2 NOT-MEASURED, 0 UNRUN`; all 57 exited 0. NOT MEASURED: `pnpm check:dual-build-cjs-loads` and `pnpm check:type-check-debt`. Both exit 3, PREREQUISITE NOT MET: they need every package's `dist/` (a whole-repo build), which is CI's. - **Consumer suites** (`cli`, `mcp`, `metadata-protocol`, `platform-objects`, `cloud-connection`): not run locally and declared to CI. The public surface is byte-identical: no export, no wire shape. The census above measures every in-repo predicate, fixtures included, at 0 flips, and no suite outside `packages/lint` asserts an `rls-predicate-*` id. - **Dogfood:** the showcase predicates (`owner == current_user.email` and similar) are inside the 0-flip census. ## Choices settled here (four axes) 1. **Report the null family although the data path does not refuse it.** - **Business need:** measured. `!(record.status in ['open', null])` hides every `closed` row on read while its own `check` admits the write, and an AI writing a blocklist "neither closed nor missing" lands exactly here. - **Long-term soundness:** the contract (the null rulings) already refuses these comparands at every face that judges them. The lint agrees with the contract rather than with the one path that skips it. - **Guarding against AI metadata mistakes:** a loud authoring refusal, with a rewrite, instead of backend-dependent silence. - **Startup scope:** no new capability, no runtime change. - Not reporting it would leave "validated clean, behaves per backend" in place. 2. **Same id (`rls-predicate-unenforceable`), not a new one.** Zone 1's direction names the id for the compile-refusal family, and the null family's fix is the same kind of edit (rewrite inside the enforceable subset). An allowlist keyed on the id already means "this predicate enforces nothing", and a new public id would widen `@objectstack/lint`'s exported surface for no new decision. The message says which class it is. 3. **The discriminator** as above: asked of the compiler, the way `userVariableIsScalarPositioned` already asks it, and never by grading prose. ## Acceptance notes - **Runtime gap (reported, not addressed here):** the RLS data path does not run the shared comparand faces on its own compiled filter. A `null` list member or `null` ordering bound in a policy reaches the driver and the `check` evaluator, which disagree. - Measured through the real plugin on driver-sql: `!(record.status in ['open', null])` as `using` reads 0 rows (the `closed` row is hidden). As `check`, it admits a `closed` insert and refuses an `open` one. - `record.status in ['open', null]` reads `r_open` only. - `record.status > null` reads 0 rows and refuses both inserts. - The contract's own text calls these cells "constructively unreachable through the compile face" (`driver-memory/src/memory-matcher.ts`), and they are reachable through RLS. - Handed to the PM seat for the filing gate; this PR changes no runtime source. - `packages/spec/src/migrations/entries/semantic/18.cel-predicate-list-comparand-refused.ts` says "The authoring lint reports a list literal as rls-predicate-unenforceable; a membership set holds its value only per request, so that form is refused at request time." That is still true, but no longer complete: the lint now reports the membership-set form too. Carrier: none. - An ordering against a membership set (`record.f > current_user.positions`) lowers to `$gt` over a list. Neither the compiler nor the faces refuse it, and its runtime answer was not exercised here. Carrier: none. - Conservative limits, by design: a predicate with two type faults stays silent. So does an unknown app key in a scalar position when a later type fault masks it; that is `userVariableIsScalarPositioned`'s existing behaviour. --- _Generated by [Claude Code](https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 84156c7 commit af32cf9

3 files changed

Lines changed: 587 additions & 42 deletions

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
`rls-predicate-unenforceable` now reports the RLS predicates the runtime refuses on every request, which the shape check cannot see (#19951).
6+
7+
`validateRlsPredicateEnforceability` judged a predicate's shape with every `current_user` value replaced by a placeholder, so a predicate whose refusal depends on a value passed `os validate` cleanly and enforced nothing. Its reference pass now reports two such classes as `error`, so **a previously green `os validate` can fail** on a stack that declares one of these predicates. Measured over this repository, 0 of the 126 authored `using` / `check` predicates (77 of them `current_user` predicates) change verdict.
8+
9+
**A `current_user` value of the wrong type for its position.** The kernel resolves `positions`, `org_user_ids` and `accessible_org_ids` as lists and `id`, `organization_id` and `email` as one value on every request, and `current_user` alone is the whole caller context. The compiler refuses a mismatch on every request, and the RLS compiler drops the policy: reads return no rows and `check` writes are refused 403.
10+
11+
- FROM silent TO `rls-predicate-unenforceable`: `record.f != current_user.org_user_ids` (any list key). Write `!(record.f in current_user.org_user_ids)`.
12+
- FROM silent TO `rls-predicate-unenforceable`: `record.f == current_user.positions` and `!(record.f == current_user.positions)`. Write `record.f in current_user.positions`, keeping any enclosing `!(...)`.
13+
- FROM silent TO `rls-predicate-unenforceable`: `record.f in current_user.id` (any one-value key). Write `record.f == current_user.id`.
14+
- 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)`.
15+
- 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.
16+
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.
18+
19+
- 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.
20+
- FROM silent TO `rls-predicate-unenforceable`: `record.f in [null]`. Write `record.f == null`.
21+
- FROM silent TO `rls-predicate-unenforceable`: `record.f > null`, `>= null`, `< null` and `<= null`. Write `record.f != null` or `record.f == null`, or compare against a real bound.
22+
23+
Each finding's hint carries the rewrite with the predicate's own field and key.
24+
25+
**Unchanged.** A comparison whose answer depends on which caller asks, such as `current_user.email == 'ops@acme.com'`, stays silent: it grants everything to the caller it names and is refused for everyone else, so no probe value can stand for it. The list literal (`record.f != ['a', 'b']`) and the bare root under `==` / `!=` (`record.f != current_user`) were already reported. `field == current_user.id`, `field in current_user.org_user_ids` and `field == null` stay clean.

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

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

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

66
import { RESERVED_RLS_MEMBERSHIP_KEYS } from '@objectstack/spec/contracts';
77

@@ -698,6 +698,24 @@ describe('validateRlsPredicateEnforceability — the messages name the COST, not
698698
});
699699
});
700700

701+
/**
702+
* What each kernel-resolved key holds on every request. Read off the resolver,
703+
* not off any list: `compileFilter` builds its `current_user` context in
704+
* `plugin-security/src/rls-compiler.ts` from `ExecutionContext` (`id` ←
705+
* `userId`, `organization_id` ← `tenantId`, the rest by name), and
706+
* `ExecutionContextSchema` declares `userId`, `tenantId` and `email` as strings
707+
* and `positions`, `org_user_ids` and `accessible_org_ids` as string arrays;
708+
* `resolveAuthzContext` produces the same types.
709+
*/
710+
const KERNEL_KEY_RUNTIME_TYPE: Record<string, 'scalar' | 'array'> = {
711+
id: 'scalar',
712+
organization_id: 'scalar',
713+
email: 'scalar',
714+
positions: 'array',
715+
org_user_ids: 'array',
716+
accessible_org_ids: 'array',
717+
};
718+
701719
describe('validateRlsPredicateEnforceability — the `current_user` set is DERIVED, not transcribed', () => {
702720
/**
703721
* The known set is {@link RESERVED_RLS_MEMBERSHIP_KEYS} from
@@ -713,16 +731,24 @@ describe('validateRlsPredicateEnforceability — the `current_user` set is DERIV
713731
* different, stale shape (`tenantId`, `department`, `attributes`) the RLS
714732
* compiler never binds — neither is the authority.
715733
*/
716-
it('accepts every kernel-resolved key in BOTH positions', () => {
734+
it('never reports a kernel-resolved key as UNKNOWN — in its own position it is clean, in the other it is a type fault', () => {
717735
expect(RESERVED_RLS_MEMBERSHIP_KEYS.length).toBeGreaterThan(0);
718736
for (const key of RESERVED_RLS_MEMBERSHIP_KEYS) {
719-
// ⚠️ BOTH, asserted separately. An `a.length === 0 || b.length === 0`
720-
// here would pass on whichever position happened to be silent, and this
721-
// rule is silent in both — so the disjunction pinned nothing about
722-
// position at all and would have survived a position-blind rewrite.
737+
// ⚠️ BOTH positions, asserted separately and by the key's RUNTIME type.
738+
// Until #19951 this pinned `[]` for both, which was the rule's blind spot
739+
// written down: the position the key's type cannot fill (`== <a set>`,
740+
// `in <one value>`) is dropped by the runtime on every request. That
741+
// position is now `rls-predicate-unenforceable` — and still never
742+
// `rls-predicate-unknown-user-variable`, which is what "derived, not
743+
// transcribed" means for the kernel's own keys.
744+
const type = KERNEL_KEY_RUNTIME_TYPE[key];
723745
const scalar = ids(siteWith('using', `owner_id == current_user.${key}`));
724746
const member = ids(siteWith('using', `owner_id in current_user.${key}`));
725-
expect({ key, scalar, member }).toEqual({ key, scalar: [], member: [] });
747+
expect({ key, scalar, member }).toEqual({
748+
key,
749+
scalar: type === 'scalar' ? [] : [RLS_PREDICATE_UNENFORCEABLE],
750+
member: type === 'array' ? [] : [RLS_PREDICATE_UNENFORCEABLE],
751+
});
726752
}
727753
});
728754

@@ -742,24 +768,10 @@ describe('validateRlsPredicateEnforceability — each kernel key is probed with
742768
* the wrong type silences the reference half: an array bound to `id` made
743769
* every `field == current_user.id` policy compile to nothing here, and its
744770
* field and variable checks never ran. Each key is therefore probed with the
745-
* type `RLSCompiler.compileFilter` hands the compiler at runtime.
746-
*
747-
* Read off the resolver, not off any list: `compileFilter` builds its
748-
* `current_user` context in `plugin-security/src/rls-compiler.ts` from
749-
* `ExecutionContext` (`id` ← `userId`, `organization_id` ← `tenantId`, the
750-
* rest by name), and `ExecutionContextSchema` declares `userId`, `tenantId`
751-
* and `email` as strings and `positions`, `org_user_ids` and
752-
* `accessible_org_ids` as string arrays; `resolveAuthzContext` produces the
753-
* same types.
771+
* type `RLSCompiler.compileFilter` hands the compiler at runtime — the table
772+
* {@link KERNEL_KEY_RUNTIME_TYPE} says where that is read from.
754773
*/
755-
const RUNTIME_TYPE: Record<string, 'scalar' | 'array'> = {
756-
id: 'scalar',
757-
organization_id: 'scalar',
758-
email: 'scalar',
759-
positions: 'array',
760-
org_user_ids: 'array',
761-
accessible_org_ids: 'array',
762-
};
774+
const RUNTIME_TYPE = KERNEL_KEY_RUNTIME_TYPE;
763775

764776
it('covers exactly the kernel-resolved keys the contract names', () => {
765777
expect(Object.keys(RUNTIME_TYPE).sort()).toEqual([...RESERVED_RLS_MEMBERSHIP_KEYS].sort());
@@ -814,6 +826,147 @@ describe('validateRlsPredicateEnforceability — the bare `current_user` root is
814826
});
815827
});
816828

829+
describe('validateRlsPredicateEnforceability — the refusals the SHAPE check cannot see are reported (#19951)', () => {
830+
/**
831+
* Every source below passes `isSupportedRlsExpression`, `os validate` and this
832+
* rule as they stood, and none of them enforces what it says. Measured through
833+
* the real `SecurityPlugin` + ObjectQL + driver-sql:
834+
*
835+
* - TYPE faults — a `current_user` value of the wrong type for its position.
836+
* The compiler refuses the position on every request, `RLSCompiler` drops
837+
* the policy: a `using` read returns zero rows (with only a per-request
838+
* "DENY (fail closed)" WARN) and a `check` write is refused with 403.
839+
* - 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.
844+
*/
845+
const TYPE_FAULTS: ReadonlyArray<readonly [string, string]> = [
846+
[
847+
'record.assigned_to_id != current_user.org_user_ids',
848+
'replace `record.assigned_to_id != current_user.org_user_ids` with `!(record.assigned_to_id in current_user.org_user_ids)`',
849+
],
850+
[
851+
'!(record.assigned_to_id == current_user.org_user_ids)',
852+
'replace `record.assigned_to_id == current_user.org_user_ids` with `record.assigned_to_id in current_user.org_user_ids`',
853+
],
854+
[
855+
'record.assigned_to_id in current_user.id',
856+
'replace `record.assigned_to_id in current_user.id` with `record.assigned_to_id == current_user.id`',
857+
],
858+
['record.assigned_to_id in current_user', '`record.assigned_to_id in current_user.org_user_ids`'],
859+
['record.assigned_to_id.startsWith(current_user)', '`record.assigned_to_id.startsWith(current_user.id)`'],
860+
['record.assigned_to_id.contains(current_user)', '`record.assigned_to_id.contains(current_user.email)`'],
861+
['record.assigned_to_id.endsWith(current_user)', '`record.assigned_to_id.endsWith(current_user.organization_id)`'],
862+
[
863+
'record.assigned_to_id.startsWith(current_user.org_user_ids)',
864+
'the membership test `record.assigned_to_id in current_user.org_user_ids`',
865+
],
866+
];
867+
868+
const NULL_COMPARANDS: ReadonlyArray<readonly [string, string]> = [
869+
[
870+
"record.status in ['open', null]",
871+
'replace `record.status in ["open", null]` with `(record.status in ["open"] || record.status == null)`',
872+
],
873+
['record.status in [null]', 'replace `record.status in [null]` with `record.status == null`'],
874+
[
875+
"!(record.status in ['open', null])",
876+
'replace `record.status in ["open", null]` with `(record.status in ["open"] || record.status == null)`',
877+
],
878+
['record.status > null', 'replace `record.status > null` with `record.status != null`'],
879+
['record.status >= null', 'replace `record.status >= null` with `record.status != null`'],
880+
['record.status < null', 'replace `record.status < null` with `record.status != null`'],
881+
['record.status <= null', 'replace `record.status <= null` with `record.status != null`'],
882+
];
883+
884+
it.each([...TYPE_FAULTS, ...NULL_COMPARANDS])('%s — one finding per clause, with the pasteable rewrite', (source, rewrite) => {
885+
// Why it was silent: the shape verdict this rule reads says yes.
886+
expect(isSupportedRlsExpression(source)).toBe(true);
887+
for (const clause of ['using', 'check'] as const) {
888+
const findings = validateRlsPredicateEnforceability(siteWith(clause, source));
889+
expect(findings.map((f) => [f.rule, f.path])).toEqual([
890+
[RLS_PREDICATE_UNENFORCEABLE, `permissions[0].rowLevelSecurity[0].${clause}`],
891+
]);
892+
expect(findings[0].hint).toContain(rewrite);
893+
}
894+
});
895+
896+
it('a TYPE fault names the reference, what it holds, and a DROP with the clause’s own consequence', () => {
897+
const [using] = validateRlsPredicateEnforceability(siteWith('using', TYPE_FAULTS[0][0]));
898+
expect(using.message).toContain('`current_user.org_user_ids` is a membership set');
899+
expect(using.message).toMatch(/DROPS the policy on EVERY request/);
900+
expect(using.message).toMatch(/RLS_DENY_FILTER/);
901+
902+
const [check] = validateRlsPredicateEnforceability(siteWith('check', 'record.assigned_to_id in current_user.id'));
903+
expect(check.message).toContain('`current_user.id` holds ONE value');
904+
expect(check.message).toMatch(/PermissionDeniedError/);
905+
});
906+
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/);
911+
});
912+
913+
it('rewrites EVERY site the fault lands in, in one finding', () => {
914+
const findings = validateRlsPredicateEnforceability(
915+
siteWith('using', 'record.assigned_to_id != current_user.org_user_ids && record.owner_id != current_user.org_user_ids'),
916+
);
917+
expect(findings.map((f) => f.rule)).toEqual([RLS_PREDICATE_UNENFORCEABLE]);
918+
expect(findings[0].hint).toContain('`!(record.assigned_to_id in current_user.org_user_ids)`');
919+
expect(findings[0].hint).toContain('`!(record.owner_id in current_user.org_user_ids)`');
920+
});
921+
922+
it('CONTROLS — the literal list keeps its one shape finding, and every spelling the hints point at is clean', () => {
923+
expect(ids(siteWith('using', "record.status != ['closed', 'archived']"))).toEqual([RLS_PREDICATE_UNENFORCEABLE]);
924+
for (const source of [
925+
'record.assigned_to_id == current_user.id',
926+
'!(record.assigned_to_id in current_user.org_user_ids)',
927+
'record.assigned_to_id in current_user.org_user_ids',
928+
'record.assigned_to_id.startsWith(current_user.id)',
929+
"(record.status in ['open'] || record.status == null)",
930+
'record.status == null',
931+
'record.status != null',
932+
]) {
933+
for (const clause of ['using', 'check'] as const) {
934+
expect({ source, clause, rules: ids(siteWith(clause, source)) }).toEqual({ source, clause, rules: [] });
935+
}
936+
}
937+
});
938+
939+
it('CONTROL — a refusal that depends on WHICH caller asks is a probe artefact and stays silent', () => {
940+
// Load-bearing: the compiler really does refuse this for the probe's value.
941+
// The runtime answers it PER CALLER — no restriction for the one it names,
942+
// a refusal for every other — so no probe value can stand for the request,
943+
// and reporting "refused" would be reporting the lint's own probe.
944+
const named = "current_user.email == 'ops@acme.com'";
945+
expect(compileCelToFilter(named, { variables: { current_user: { email: 'ops@acme.com' } } })).toEqual({
946+
ok: true,
947+
filter: {},
948+
});
949+
expect(compileCelToFilter(named, { variables: { current_user: { email: 'rep@acme.com' } } })).toMatchObject({
950+
ok: false,
951+
reason: 'unsupported',
952+
});
953+
for (const source of [named, `${named} || record.owner_id == current_user.id`]) {
954+
expect(isSupportedRlsExpression(source)).toBe(true);
955+
for (const clause of ['using', 'check'] as const) {
956+
expect({ source, clause, rules: ids(siteWith(clause, source)) }).toEqual({ source, clause, rules: [] });
957+
}
958+
}
959+
});
960+
961+
it('reaches `os validate` through the real rule table', () => {
962+
const stack = siteWith('using', TYPE_FAULTS[0][0]);
963+
const rls = runAuthoringRules('validate', { normalized: stack, parsed: stack })
964+
.filter((f) => f.rule.startsWith('rls-predicate'))
965+
.map((f) => f.rule);
966+
expect(rls).toEqual([RLS_PREDICATE_UNENFORCEABLE]);
967+
});
968+
});
969+
817970
describe('validateRlsPredicateEnforceability — §7.3.1 membership keys stay UNKNOWABLE', () => {
818971
/**
819972
* The false-positive this rule exists on the edge of. An app stages arbitrary

0 commit comments

Comments
 (0)