diff --git a/.changeset/19951-rls-lint-per-request-refusals.md b/.changeset/19951-rls-lint-per-request-refusals.md new file mode 100644 index 00000000000..adbae4f30a1 --- /dev/null +++ b/.changeset/19951-rls-lint-per-request-refusals.md @@ -0,0 +1,25 @@ +--- +"@objectstack/lint": minor +--- + +`rls-predicate-unenforceable` now reports the RLS predicates the runtime refuses on every request, which the shape check cannot see (#19951). + +`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. + +**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. + +- 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)`. +- 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 `!(...)`. +- FROM silent TO `rls-predicate-unenforceable`: `record.f in current_user.id` (any one-value key). Write `record.f == current_user.id`. +- 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)`. +- 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. + +**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. + +- 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. +- FROM silent TO `rls-predicate-unenforceable`: `record.f in [null]`. Write `record.f == null`. +- 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. + +Each finding's hint carries the rewrite with the predicate's own field and key. + +**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. diff --git a/packages/lint/src/validate-rls-predicate-enforceability.test.ts b/packages/lint/src/validate-rls-predicate-enforceability.test.ts index 1a5cb643d10..17416519c4c 100644 --- a/packages/lint/src/validate-rls-predicate-enforceability.test.ts +++ b/packages/lint/src/validate-rls-predicate-enforceability.test.ts @@ -1,7 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, afterEach } from 'vitest'; -import { isSupportedRlsExpression, setCelPushdownLimitsModeForTests } from '@objectstack/formula'; +import { compileCelToFilter, isSupportedRlsExpression, setCelPushdownLimitsModeForTests } from '@objectstack/formula'; import { RESERVED_RLS_MEMBERSHIP_KEYS } from '@objectstack/spec/contracts'; @@ -698,6 +698,24 @@ describe('validateRlsPredicateEnforceability — the messages name the COST, not }); }); +/** + * What each kernel-resolved key holds on every request. Read off the resolver, + * not off any list: `compileFilter` builds its `current_user` context in + * `plugin-security/src/rls-compiler.ts` from `ExecutionContext` (`id` ← + * `userId`, `organization_id` ← `tenantId`, the rest by name), and + * `ExecutionContextSchema` declares `userId`, `tenantId` and `email` as strings + * and `positions`, `org_user_ids` and `accessible_org_ids` as string arrays; + * `resolveAuthzContext` produces the same types. + */ +const KERNEL_KEY_RUNTIME_TYPE: Record = { + id: 'scalar', + organization_id: 'scalar', + email: 'scalar', + positions: 'array', + org_user_ids: 'array', + accessible_org_ids: 'array', +}; + describe('validateRlsPredicateEnforceability — the `current_user` set is DERIVED, not transcribed', () => { /** * The known set is {@link RESERVED_RLS_MEMBERSHIP_KEYS} from @@ -713,16 +731,24 @@ describe('validateRlsPredicateEnforceability — the `current_user` set is DERIV * different, stale shape (`tenantId`, `department`, `attributes`) the RLS * compiler never binds — neither is the authority. */ - it('accepts every kernel-resolved key in BOTH positions', () => { + it('never reports a kernel-resolved key as UNKNOWN — in its own position it is clean, in the other it is a type fault', () => { expect(RESERVED_RLS_MEMBERSHIP_KEYS.length).toBeGreaterThan(0); for (const key of RESERVED_RLS_MEMBERSHIP_KEYS) { - // ⚠️ BOTH, asserted separately. An `a.length === 0 || b.length === 0` - // here would pass on whichever position happened to be silent, and this - // rule is silent in both — so the disjunction pinned nothing about - // position at all and would have survived a position-blind rewrite. + // ⚠️ BOTH positions, asserted separately and by the key's RUNTIME type. + // Until #19951 this pinned `[]` for both, which was the rule's blind spot + // written down: the position the key's type cannot fill (`== `, + // `in `) is dropped by the runtime on every request. That + // position is now `rls-predicate-unenforceable` — and still never + // `rls-predicate-unknown-user-variable`, which is what "derived, not + // transcribed" means for the kernel's own keys. + const type = KERNEL_KEY_RUNTIME_TYPE[key]; const scalar = ids(siteWith('using', `owner_id == current_user.${key}`)); const member = ids(siteWith('using', `owner_id in current_user.${key}`)); - expect({ key, scalar, member }).toEqual({ key, scalar: [], member: [] }); + expect({ key, scalar, member }).toEqual({ + key, + scalar: type === 'scalar' ? [] : [RLS_PREDICATE_UNENFORCEABLE], + member: type === 'array' ? [] : [RLS_PREDICATE_UNENFORCEABLE], + }); } }); @@ -742,24 +768,10 @@ describe('validateRlsPredicateEnforceability — each kernel key is probed with * the wrong type silences the reference half: an array bound to `id` made * every `field == current_user.id` policy compile to nothing here, and its * field and variable checks never ran. Each key is therefore probed with the - * type `RLSCompiler.compileFilter` hands the compiler at runtime. - * - * Read off the resolver, not off any list: `compileFilter` builds its - * `current_user` context in `plugin-security/src/rls-compiler.ts` from - * `ExecutionContext` (`id` ← `userId`, `organization_id` ← `tenantId`, the - * rest by name), and `ExecutionContextSchema` declares `userId`, `tenantId` - * and `email` as strings and `positions`, `org_user_ids` and - * `accessible_org_ids` as string arrays; `resolveAuthzContext` produces the - * same types. + * type `RLSCompiler.compileFilter` hands the compiler at runtime — the table + * {@link KERNEL_KEY_RUNTIME_TYPE} says where that is read from. */ - const RUNTIME_TYPE: Record = { - id: 'scalar', - organization_id: 'scalar', - email: 'scalar', - positions: 'array', - org_user_ids: 'array', - accessible_org_ids: 'array', - }; + const RUNTIME_TYPE = KERNEL_KEY_RUNTIME_TYPE; it('covers exactly the kernel-resolved keys the contract names', () => { expect(Object.keys(RUNTIME_TYPE).sort()).toEqual([...RESERVED_RLS_MEMBERSHIP_KEYS].sort()); @@ -814,6 +826,147 @@ describe('validateRlsPredicateEnforceability — the bare `current_user` root is }); }); +describe('validateRlsPredicateEnforceability — the refusals the SHAPE check cannot see are reported (#19951)', () => { + /** + * Every source below passes `isSupportedRlsExpression`, `os validate` and this + * rule as they stood, and none of them enforces what it says. Measured through + * the real `SecurityPlugin` + ObjectQL + driver-sql: + * + * - TYPE faults — a `current_user` value of the wrong type for its position. + * The compiler refuses the position on every request, `RLSCompiler` drops + * the policy: a `using` read returns zero rows (with only a per-request + * "DENY (fail closed)" WARN) and a `check` write is refused with 403. + * - NULL comparands — the shared filter faces refuse a `null` list member and + * a `null` ordering bound by ruling. They compile, the RLS layer never runs + * the faces on its own filter, and the backend answers: on driver-sql + * `x in [null]`, `!(x in ['a', null])` and `x > null` read nothing, while + * the `check` evaluator admits a write the negated read hides. + */ + const TYPE_FAULTS: ReadonlyArray = [ + [ + 'record.assigned_to_id != current_user.org_user_ids', + 'replace `record.assigned_to_id != current_user.org_user_ids` with `!(record.assigned_to_id in current_user.org_user_ids)`', + ], + [ + '!(record.assigned_to_id == current_user.org_user_ids)', + 'replace `record.assigned_to_id == current_user.org_user_ids` with `record.assigned_to_id in current_user.org_user_ids`', + ], + [ + 'record.assigned_to_id in current_user.id', + 'replace `record.assigned_to_id in current_user.id` with `record.assigned_to_id == current_user.id`', + ], + ['record.assigned_to_id in current_user', '`record.assigned_to_id in current_user.org_user_ids`'], + ['record.assigned_to_id.startsWith(current_user)', '`record.assigned_to_id.startsWith(current_user.id)`'], + ['record.assigned_to_id.contains(current_user)', '`record.assigned_to_id.contains(current_user.email)`'], + ['record.assigned_to_id.endsWith(current_user)', '`record.assigned_to_id.endsWith(current_user.organization_id)`'], + [ + 'record.assigned_to_id.startsWith(current_user.org_user_ids)', + 'the membership test `record.assigned_to_id in current_user.org_user_ids`', + ], + ]; + + const NULL_COMPARANDS: ReadonlyArray = [ + [ + "record.status in ['open', null]", + 'replace `record.status in ["open", null]` with `(record.status in ["open"] || record.status == null)`', + ], + ['record.status in [null]', 'replace `record.status in [null]` with `record.status == null`'], + [ + "!(record.status in ['open', null])", + 'replace `record.status in ["open", null]` with `(record.status in ["open"] || record.status == null)`', + ], + ['record.status > null', 'replace `record.status > null` with `record.status != null`'], + ['record.status >= null', 'replace `record.status >= null` with `record.status != null`'], + ['record.status < null', 'replace `record.status < null` with `record.status != null`'], + ['record.status <= null', 'replace `record.status <= null` with `record.status != null`'], + ]; + + it.each([...TYPE_FAULTS, ...NULL_COMPARANDS])('%s — one finding per clause, with the pasteable rewrite', (source, rewrite) => { + // Why it was silent: the shape verdict this rule reads says yes. + expect(isSupportedRlsExpression(source)).toBe(true); + for (const clause of ['using', 'check'] as const) { + const findings = validateRlsPredicateEnforceability(siteWith(clause, source)); + expect(findings.map((f) => [f.rule, f.path])).toEqual([ + [RLS_PREDICATE_UNENFORCEABLE, `permissions[0].rowLevelSecurity[0].${clause}`], + ]); + expect(findings[0].hint).toContain(rewrite); + } + }); + + it('a TYPE fault names the reference, what it holds, and a DROP with the clause’s own consequence', () => { + const [using] = validateRlsPredicateEnforceability(siteWith('using', TYPE_FAULTS[0][0])); + expect(using.message).toContain('`current_user.org_user_ids` is a membership set'); + expect(using.message).toMatch(/DROPS the policy on EVERY request/); + expect(using.message).toMatch(/RLS_DENY_FILTER/); + + const [check] = validateRlsPredicateEnforceability(siteWith('check', 'record.assigned_to_id in current_user.id')); + expect(check.message).toContain('`current_user.id` holds ONE value'); + expect(check.message).toMatch(/PermissionDeniedError/); + }); + + it('a NULL comparand is NOT reported as a drop — the policy survives and the backend answers', () => { + const [f] = validateRlsPredicateEnforceability(siteWith('using', NULL_COMPARANDS[2][0])); + expect(f.message).toMatch(/the policy is NOT dropped/); + expect(f.message).not.toMatch(/DROPS the policy/); + }); + + it('rewrites EVERY site the fault lands in, in one finding', () => { + const findings = validateRlsPredicateEnforceability( + siteWith('using', 'record.assigned_to_id != current_user.org_user_ids && record.owner_id != current_user.org_user_ids'), + ); + expect(findings.map((f) => f.rule)).toEqual([RLS_PREDICATE_UNENFORCEABLE]); + expect(findings[0].hint).toContain('`!(record.assigned_to_id in current_user.org_user_ids)`'); + expect(findings[0].hint).toContain('`!(record.owner_id in current_user.org_user_ids)`'); + }); + + it('CONTROLS — the literal list keeps its one shape finding, and every spelling the hints point at is clean', () => { + expect(ids(siteWith('using', "record.status != ['closed', 'archived']"))).toEqual([RLS_PREDICATE_UNENFORCEABLE]); + for (const source of [ + 'record.assigned_to_id == current_user.id', + '!(record.assigned_to_id in current_user.org_user_ids)', + 'record.assigned_to_id in current_user.org_user_ids', + 'record.assigned_to_id.startsWith(current_user.id)', + "(record.status in ['open'] || record.status == null)", + 'record.status == null', + 'record.status != null', + ]) { + for (const clause of ['using', 'check'] as const) { + expect({ source, clause, rules: ids(siteWith(clause, source)) }).toEqual({ source, clause, rules: [] }); + } + } + }); + + it('CONTROL — a refusal that depends on WHICH caller asks is a probe artefact and stays silent', () => { + // Load-bearing: the compiler really does refuse this for the probe's value. + // The runtime answers it PER CALLER — no restriction for the one it names, + // a refusal for every other — so no probe value can stand for the request, + // and reporting "refused" would be reporting the lint's own probe. + const named = "current_user.email == 'ops@acme.com'"; + expect(compileCelToFilter(named, { variables: { current_user: { email: 'ops@acme.com' } } })).toEqual({ + ok: true, + filter: {}, + }); + expect(compileCelToFilter(named, { variables: { current_user: { email: 'rep@acme.com' } } })).toMatchObject({ + ok: false, + reason: 'unsupported', + }); + for (const source of [named, `${named} || record.owner_id == current_user.id`]) { + expect(isSupportedRlsExpression(source)).toBe(true); + for (const clause of ['using', 'check'] as const) { + expect({ source, clause, rules: ids(siteWith(clause, source)) }).toEqual({ source, clause, rules: [] }); + } + } + }); + + it('reaches `os validate` through the real rule table', () => { + const stack = siteWith('using', TYPE_FAULTS[0][0]); + const rls = runAuthoringRules('validate', { normalized: stack, parsed: stack }) + .filter((f) => f.rule.startsWith('rls-predicate')) + .map((f) => f.rule); + expect(rls).toEqual([RLS_PREDICATE_UNENFORCEABLE]); + }); +}); + describe('validateRlsPredicateEnforceability — §7.3.1 membership keys stay UNKNOWABLE', () => { /** * The false-positive this rule exists on the edge of. An app stages arbitrary diff --git a/packages/lint/src/validate-rls-predicate-enforceability.ts b/packages/lint/src/validate-rls-predicate-enforceability.ts index cb850a12ff2..8589ec8c05a 100644 --- a/packages/lint/src/validate-rls-predicate-enforceability.ts +++ b/packages/lint/src/validate-rls-predicate-enforceability.ts @@ -168,6 +168,7 @@ import { } from '@objectstack/formula'; import type { CelBoundsOverrun } from '@objectstack/formula'; import { RESERVED_RLS_MEMBERSHIP_KEYS } from '@objectstack/spec/contracts'; +import { assertListComparandShapes, normalizeFilterComparandTypes } from '@objectstack/spec/data'; import { ExecutionContextSchema } from '@objectstack/spec/kernel'; import { describeFieldPathVerdict, @@ -286,11 +287,25 @@ const CHECK_SET_QUALIFIER = ' That holds when no other applicable policy for the operation declares a `check` that compiles; ' + 'when one does, that `check` alone decides and this one contributes nothing.'; +/** How an UNCOMPILABLE predicate is dropped: the runtime's own WARN names the predicate. */ +const DROPPED_UNCOMPILABLE = + 'so `RLSCompiler` DROPS the policy at request time (one WARN line — "has an uncompilable predicate ' + + '… and was DROPPED (no enforcement)" — is the only signal, and nothing reports it at authoring time). '; + +/** + * How a predicate the compiler refuses PER REQUEST is dropped. The shape check + * `compileFilter` consults passes (the refused value exists only per request), + * so it never logs the "uncompilable predicate" line: it collects the policy as + * that request's denial and WARNs only when that denial is what the request gets. + */ +const DROPPED_EVERY_REQUEST = + '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. '; + /** What the runtime does with a predicate it cannot compile, per clause. */ -function consequence(clause: 'using' | 'check'): string { - const dropped = - 'so `RLSCompiler` DROPS the policy at request time (one WARN line — "has an uncompilable predicate ' + - '… and was DROPPED (no enforcement)" — is the only signal, and nothing reports it at authoring time). '; +function consequence(clause: 'using' | 'check', dropped: string = DROPPED_UNCOMPILABLE): string { return clause === 'using' ? dropped + 'When it is the only applicable policy for that object and operation, `compileFilter` returns the ' + @@ -345,7 +360,16 @@ function consequence(clause: 'using' | 'check'): string { * predicate / fix the name / pre-resolve the variable), and an author who * suppresses one must not thereby suppress the other. These run only where the * shape check PASSED, so the guards are disjoint by construction — a predicate - * is judged by the shape ids or by these, never both. + * is judged by the shape pass or by the reference pass, never both. + * + * The reference pass ALSO reports under {@link RLS_PREDICATE_UNENFORCEABLE} + * (#19951), for the two refusals the shape check cannot see because they + * depend on a value, and whose fix is a rewrite of the predicate rather than a + * name: a `current_user` value of the wrong TYPE for its position (see + * {@link attributeTypeFault}), and a lowered comparand the shared filter faces + * refuse (see {@link sharedFaceRefusal}). Same id, because the author's edit is + * the same kind — rewrite the predicate inside the enforceable subset — and an + * allowlist keyed on it already means "this predicate enforces nothing". */ /** @@ -499,25 +523,33 @@ function isPreresolvedPath(path: string): boolean { * rather than `while (true)`: a linter must terminate on input it did not * anticipate, and stopping early only costs a finding. */ -function resolveReferences( - bridged: string, -): { filter: Record | null; unresolvedScalars: string[] } { +function resolveReferences(bridged: string): { + filter: Record | null; + unresolvedScalars: string[]; + /** The probe as the last compile saw it — every discovered key bound. */ + probe: UserProbe; + /** The compiler's `detail` when that compile was refused `unsupported`. */ + refusal: string | null; +} { const probe = baseUserProbe(); const unresolvedScalars: string[] = []; + const stop = (refusal: string | null = null) => ({ filter: null, unresolvedScalars, probe, refusal }); for (let pass = 0; pass < 32; pass++) { const res = compileWithProbe(bridged, probe); - if (res.ok) return { filter: res.filter as Record, unresolvedScalars }; + if (res.ok) return { filter: res.filter as Record, unresolvedScalars, probe, refusal: null }; if (res.reason !== 'unresolved-variable') { - // The predicate passed `isSupportedRlsExpression`, so a shape refusal here - // can only be a value the position cannot take — a membership set under - // `==` / `!=` or handed to `startsWith`, a scalar key on the right of `in`. - // The runtime refuses the same value in the same position, so nothing - // further is decidable; report what was found. - return { filter: null, unresolvedScalars }; + // [#19951] The predicate passed `isSupportedRlsExpression`, so a shape + // refusal here was produced by a VALUE — a membership set under `==` / + // `!=` or handed to `startsWith`, a one-value key on the right of `in`, + // the whole `current_user` object where one value or one set belongs — + // or by a constant comparison whose fold depends on WHICH value the + // probe holds. The caller decides which ({@link attributeTypeFault}): + // only the first recurs on every request. + return stop(res.reason === 'unsupported' ? res.detail : null); } const path = unresolvedVariablePath(res.detail); if (!path || !path.startsWith('current_user.') || isPreresolvedPath(path)) { - return { filter: null, unresolvedScalars }; + return stop(); } if (userVariableIsScalarPositioned(bridged, probe, path)) { unresolvedScalars.push(path); @@ -528,7 +560,7 @@ function resolveReferences( setProbePath(probe, path, PROBE_ARRAY); } } - return { filter: null, unresolvedScalars }; + return stop(); } /** Collect every `{ $field: '' }` reference nested anywhere under a value. */ @@ -575,6 +607,301 @@ function filterFieldPaths(filter: Record | null): Set { return fields; } +/* ──────────────────────────────────────────────────────────────────────────── + * #19951 — the refusals the SHAPE check cannot see. + * + * `isSupportedRlsExpression` judges shape with every `current_user` value + * replaced by a placeholder, so two refusals pass it and land per request: + * + * 1. A `current_user` value of the wrong TYPE for its position: + * `record.reviewer_id != current_user.org_user_ids` (a membership set under + * `!=`), `record.x in current_user.id` (one value on the right of `in`), + * `record.x in current_user` / `record.x.startsWith(current_user)` (the + * whole context object). The compiler refuses each on EVERY request, and + * `RLSCompiler` drops the policy — measured through the real plugin: zero + * rows on a `using` read, 403 on a `check` write. This pass used to stop at + * that refusal and report nothing. + * 2. A lowered comparand the shared filter faces refuse by ruling — a `null` + * list member (`x in ['a', null]`) or a `null` ordering bound (`x > null`). + * Those compile; the RLS layer does not pass its own filter through the + * faces, so the backend answers with semantics the platform left undefined. + */ + +/** + * The value standing in for the OTHER runtime type in a one-reference swap. + * + * It begins with {@link PROBE_SCALAR}, so a constant ORDERING fold + * (`current_user.id > 'm'`) reads the same after the swap — `[SWAP_SCALAR]` + * coerces to this string too — and it is never equal to the probe, so an + * equality fold between two references cannot turn true by the swap. Both + * keep a VALUE-dependent refusal refused, which is what the discriminator needs. + */ +const SWAP_SCALAR = `${PROBE_SCALAR}swap`; + +/** A lowered field position, read off the compiler's output. */ +interface LoweredSite { + field: string; + /** `==` for an implicit equality, otherwise the lowered operator (`$ne`, `$in`, `$startsWith`, …). */ + op: string; + operand: unknown; +} + +/** Every lowered field position whose operand satisfies `hit`, in walk order. */ +function loweredSites(filter: unknown, hit: (op: string, operand: unknown) => boolean): LoweredSite[] { + const sites: LoweredSite[] = []; + const walk = (node: unknown): void => { + if (Array.isArray(node)) { + for (const item of node) walk(item); + return; + } + if (!node || typeof node !== 'object') return; + for (const [key, value] of Object.entries(node as Record)) { + if (key === '$and' || key === '$or' || key === '$not') { + walk(value); + continue; + } + if (key.startsWith('$')) continue; + if (value && typeof value === 'object' && !Array.isArray(value)) { + for (const [op, operand] of Object.entries(value as Record)) { + if (hit(op, operand)) sites.push({ field: key, op, operand }); + } + } else if (hit('==', value)) { + sites.push({ field: key, op: '==', operand: value }); + } + } + }; + walk(filter); + return sites; +} + +/** What one `current_user` reference holds on every request. */ +type RuntimeHolding = 'list' | 'one value' | 'context object'; + +interface TypeFault { + /** The reference whose runtime TYPE the position cannot take (`current_user.org_user_ids`, `current_user`). */ + variable: string; + holds: RuntimeHolding; + /** Where the swapped value landed once the predicate lowered — what the prescription rewrites. */ + sites: LoweredSite[]; +} + +/** + * Does ONE `current_user` reference's runtime TYPE cause this refusal — so it + * recurs on every request — or is the refusal an artefact of the probe's VALUES? + * + * Asked of the COMPILER, the way {@link userVariableIsScalarPositioned} asks + * it: rebind one reference to a value of the OTHER type and compile again. If + * that one swap makes the predicate lower, the refusal was that reference's + * TYPE, and the type is not the probe's to choose — a kernel key holds what + * `ExecutionContextSchema` declares on every request ({@link kernelKeyProbe}), + * and the bare root is the whole context object on every request — so the + * runtime refuses the same position every time. + * + * What no single swap cures stays SILENT, and that is the probe-artefact class: + * a constant comparison with no field (`current_user.email == 'ops@acme.com'`) + * folds on the VALUE, lowering to "no restriction" for the one caller it names + * and refused for every other caller — a per-caller policy the probe cannot + * stand for ({@link SWAP_SCALAR} keeps that fold refused under every swap). It + * is also the answer for a predicate with two type faults, which no single swap + * cures: the conservative direction for a new finding. + * + * The cure must also LAND: the swapped value is found in a lowered field + * position, which is what the prescription names. + */ +function attributeTypeFault(bridged: string, probe: UserProbe): TypeFault | null { + const swaps: Array<{ variable: string; holds: RuntimeHolding; context: unknown; marker: unknown }> = []; + for (const key of PRERESOLVED_USER_KEYS) { + const isList = Array.isArray(probe[key]); + const marker = isList ? SWAP_SCALAR : [SWAP_SCALAR]; + swaps.push({ + variable: `current_user.${key}`, + holds: isList ? 'list' : 'one value', + context: { ...probe, [key]: marker }, + marker, + }); + } + // The root, twice: as a list that still carries every key (so a + // `current_user.id` beside `x in current_user` keeps resolving), and as one + // string (a string method's argument). + const rootAsList = Object.assign([SWAP_SCALAR], probe); + swaps.push({ variable: 'current_user', holds: 'context object', context: rootAsList, marker: rootAsList }); + swaps.push({ variable: 'current_user', holds: 'context object', context: SWAP_SCALAR, marker: SWAP_SCALAR }); + + for (const swap of swaps) { + const res = compileCelToFilter(bridged, { variables: { current_user: swap.context } }); + if (!res.ok) continue; + const sites = loweredSites(res.filter, (_op, operand) => operand === swap.marker); + if (sites.length > 0) return { variable: swap.variable, holds: swap.holds, sites }; + } + return null; +} + +/** The kernel keys that hold `holding` on every request, from their declared types. */ +function kernelKeysHolding(holding: 'list' | 'one value'): string[] { + return [...PRERESOLVED_USER_KEYS] + .filter((key) => Array.isArray(kernelKeyProbe(key)) === (holding === 'list')) + .sort(); +} + +/** The lowered string-method operators, back to the CEL method an author writes. */ +const STRING_METHOD_OF: ReadonlyMap = new Map([ + ['$startsWith', 'startsWith'], + ['$endsWith', 'endsWith'], + ['$contains', 'contains'], +]); + +/** The pasteable rewrite for one lowered site of a type fault. */ +function typeFaultRewrite(fault: TypeFault, site: LoweredSite): string { + const f = `record.${site.field}`; + const v = fault.variable; + const method = STRING_METHOD_OF.get(site.op); + const oneValueKeys = kernelKeysHolding('one value'); + if (fault.holds === 'list') { + if (site.op === '==') return `replace \`${f} == ${v}\` with \`${f} in ${v}\``; + if (site.op === '$ne') return `replace \`${f} != ${v}\` with \`!(${f} in ${v})\``; + if (method) { + return ( + `replace \`${f}.${method}(${v})\` with the membership test \`${f} in ${v}\`, or pass one value: ` + + oneValueKeys.map((key) => `\`${f}.${method}(current_user.${key})\``).join(' / ') + ); + } + } + if (fault.holds === 'one value' && site.op === '$in') { + return ( + `replace \`${f} in ${v}\` with \`${f} == ${v}\` (and \`!(${f} in ${v})\` with \`${f} != ${v}\`); for a ` + + `set, name a membership key: ` + + kernelKeysHolding('list').map((key) => `\`${f} in current_user.${key}\``).join(' / ') + ); + } + if (fault.holds === 'context object') { + if (site.op === '$in') { + return ( + `replace \`${f} in current_user\` with the key that holds the set: ` + + kernelKeysHolding('list').map((key) => `\`${f} in current_user.${key}\``).join(' / ') + + `, or an app-staged §7.3.1 set \`${f} in current_user.\`` + ); + } + if (method) { + return ( + `replace \`${f}.${method}(current_user)\` with the key that holds the string: ` + + oneValueKeys.map((key) => `\`${f}.${method}(current_user.${key})\``).join(' / ') + ); + } + } + return `rewrite the comparison on \`${f}\` so \`${v}\` sits where its value fits`; +} + +const HOLDING_SENTENCE: Readonly string>> = { + list: (v) => + `\`${v}\` 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.', + 'one value': (v) => + `\`${v}\` 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.', + 'context object': () => + '`current_user` alone is the whole caller context object on every request, never one value and never one set.', +}; + +/** The one-sentence rule each holding breaks, leading its hint. */ +const HOLDING_RULE: Readonly> = { + list: + '`==` / `!=` 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)`.', + 'one value': '`in` tests membership in a SET; one value is compared with `==` / `!=`.', + 'context object': '`current_user` is the caller context, not a value: name the key that holds what you mean.', +}; + +/** + * The verdict of the shared filter faces on this predicate's LOWERED filter — + * `assertListComparandShapes` and `normalizeFilterComparandTypes` + * (`@objectstack/spec/data`), the same pair the analytics read-scope compiler + * runs on every RLS read scope (`assertReadScopeComparandsRunnable`) and the + * engine runs on every caller-supplied filter. The RLS data path does not run + * them on its own filter, which is why a refused comparand gets through there. + * + * Graded by the refusal's `code` (`INVALID_FILTER`), never by its prose (#6223); + * the returned message is quoted, never parsed. + * + * Not a probe artefact: the faces accept a string and a list of strings by + * construction, and those are the only values the probe binds — so a refusal + * here is about a comparand the AUTHOR wrote, and recurs on every request. A + * probe that bound a nested record (a `current_user.a.b` path) is not judged. + */ +function sharedFaceRefusal(filter: Record, probe: UserProbe, clause: string): string | null { + if (!Object.values(probe).every((v) => typeof v === 'string' || Array.isArray(v))) return null; + try { + assertListComparandShapes(filter, undefined, clause); + normalizeFilterComparandTypes(filter, undefined, clause); + return null; + } catch (err) { + if ((err as { code?: unknown } | null)?.code !== 'INVALID_FILTER') return null; + return err instanceof Error ? err.message : String(err); + } +} + +/** The lowered ordering operators, back to the CEL operator an author writes. */ +const ORDERING_SYMBOL: ReadonlyMap = new Map([ + ['$gt', '>'], + ['$gte', '>='], + ['$lt', '<'], + ['$lte', '<='], +]); + +/** A CEL literal for one list member — a JSON string literal is valid CEL. */ +function celLiteral(value: unknown): string { + return typeof value === 'string' ? JSON.stringify(value) : String(value); +} + +/** + * The pasteable rewrites for a face refusal: every lowered `null` list member + * and `null` ordering bound, rewritten with the null predicate. Each is a LOCAL + * replacement, correct under any enclosing `!`, `&&` or `||` — `!(x in ['a'] || + * x == null)` is exactly "none of these, and has a value". + */ +function nullComparandRewrites(filter: Record): string[] { + const rewrites: string[] = []; + for (const site of loweredSites(filter, (op, operand) => + (op === '$in' && Array.isArray(operand) && operand.includes(null)) || + (ORDERING_SYMBOL.has(op) && operand === null))) { + const f = `record.${site.field}`; + if (site.op === '$in') { + const list = site.operand as unknown[]; + const written = `${f} in [${list.map(celLiteral).join(', ')}]`; + const rest = list.filter((member) => member !== null); + rewrites.push( + rest.length === 0 + ? `replace \`${written}\` with \`${f} == null\`` + : `replace \`${written}\` with \`(${f} in [${rest.map(celLiteral).join(', ')}] || ${f} == null)\` ` + + `to keep matching a missing value, or drop the member: \`${f} in [${rest.map(celLiteral).join(', ')}]\``, + ); + } else { + rewrites.push( + `replace \`${f} ${ORDERING_SYMBOL.get(site.op)} null\` with \`${f} != null\` (has a value) or ` + + `\`${f} == null\` (has none), or compare against a real bound`, + ); + } + } + return rewrites; +} + +/** The leading sentence of a quoted refusal, so an author sees its point and not its whole essay. */ +function firstSentence(message: string): string { + const cut = message.indexOf('. '); + return cut === -1 ? message : message.slice(0, cut); +} + +/** + * What a face-refused comparand does at request time. Not a drop: the compile + * succeeds and the policy's filter reaches the backend unjudged. + */ +const FACE_REFUSED_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.'; + /** * What a reference miss costs at request time, per clause. Measured, not inferred. * @@ -666,7 +993,7 @@ function referenceFindings( ): RlsPredicateFinding[] { const findings: RlsPredicateFinding[] = []; const bridged = sqlPredicateToCel(source); - const { filter, unresolvedScalars } = resolveReferences(bridged); + const { filter, unresolvedScalars, probe, refusal } = resolveReferences(bridged); for (const fieldPath of filterFieldPaths(filter)) { const verdict = resolveFieldPath(graph, object, fieldPath); @@ -718,6 +1045,46 @@ function referenceFindings( }); } + // [#19951] The refusals the shape check cannot see (the block above + // {@link attributeTypeFault}). A refusal no single type swap cures — the + // probe-artefact class — reports nothing. + const fault = refusal !== null ? attributeTypeFault(bridged, probe) : null; + if (refusal !== null && fault) { + findings.push({ + severity: 'error', + rule: RLS_PREDICATE_UNENFORCEABLE, + where, + path, + message: + `RLS ${clause} \`${quote(source)}\` passes the shape check, but the compiler refuses it for the value ` + + `\`${fault.variable}\` holds on every request (${refusal}). ${HOLDING_SENTENCE[fault.holds](fault.variable)} ` + + consequence(clause, DROPPED_EVERY_REQUEST), + hint: + `${HOLDING_RULE[fault.holds]} In this predicate: ` + + `${fault.sites.map((site) => typeFaultRewrite(fault, site)).join('; ')}.`, + }); + } + + const faced = filter ? sharedFaceRefusal(filter, probe, clause) : null; + if (filter && faced !== null) { + const rewrites = nullComparandRewrites(filter); + findings.push({ + severity: 'error', + rule: RLS_PREDICATE_UNENFORCEABLE, + where, + path, + message: + `RLS ${clause} \`${quote(source)}\` lowers, but to a comparand the platform's shared filter check ` + + `refuses (${firstSentence(faced)}). ${FACE_REFUSED_CONSEQUENCE}`, + hint: + rewrites.length > 0 + ? '`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\`: ${rewrites.join('; ')}.` + : 'Rewrite the comparand the check names inside the lowerable subset. ' + PUSHDOWN_SUBSET, + }); + } + return findings; }