diff --git a/.changeset/rls-denial-names-its-reason.md b/.changeset/rls-denial-names-its-reason.md new file mode 100644 index 0000000000..bed6447da2 --- /dev/null +++ b/.changeset/rls-denial-names-its-reason.md @@ -0,0 +1,31 @@ +--- +'@objectstack/plugin-security': patch +--- + +**An RLS denial caused by an unresolved variable now leaves a trace — and the trace carries the reason.** + +When `compileCelToFilter` refuses a policy predicate, it produces a precise `detail`: which +`current_user.*` variable did not resolve, or which member of a pre-resolved membership array +came back `null`, and at what index. `RLSCompiler.compileExpression` consumed only `!ok` and +threw that `detail` away one line before the only place that could surface it, and the warn +sitting beside the drop was gated on `isSupportedRlsExpression` — a SHAPE-only test that +answers "supported" for exactly these shapes, so nothing logged. + +The result was the worst-shaped failure an operator can be handed: the caller sees zero rows, +no error is raised, nothing appears in the log — and the denial is *deliberate*, the +fail-closed path working as designed, so a correct refusal is indistinguishable from "the data +genuinely doesn't match". + +The drop site now keeps the compiler's reason and, when every applicable policy has dropped and +the clause actually fails closed, logs one line naming the policy, the object, the clause, the +predicate, the variable path, the member index and the consequence (`__rls_deny__`, zero rows, +a refusal rather than an empty result set). The same line covers the emptied-membership drop, +which the compiler reports as a success and this file then refuses — silent for the same reason. + +Nothing about the decision moves. `RLS_DENY_FILTER` still lands in the read filter, record +attribution still excludes, zero rows still means zero rows, and `compileExpression` keeps its +published `Record | null` signature. A predicate that never compiles for any input keeps its +existing "DROPPED (no enforcement)" line (now also carrying the compiler's reason) rather than +gaining a second one; a dropped policy whose sibling still grants stays silent, because that +caller sees rows; and because this seam runs on read paths the denial line is emitted once per +distinct cause rather than once per request. diff --git a/packages/plugins/plugin-security/src/rls-compiler.ts b/packages/plugins/plugin-security/src/rls-compiler.ts index 9360869c63..3c54ac68dc 100644 --- a/packages/plugins/plugin-security/src/rls-compiler.ts +++ b/packages/plugins/plugin-security/src/rls-compiler.ts @@ -9,6 +9,32 @@ import type { ExecutionContext } from '@objectstack/spec/kernel'; // now a consumer of that one definition, exactly as the lint gate is; there is // no second copy for the `=` / `IN` bridge to drift against. import { compileCelToFilter, isSupportedRlsExpression, sqlPredicateToCel } from '@objectstack/formula'; +import type { CelFilterFailReason } from '@objectstack/formula'; + +/** + * Why a policy's predicate produced no filter — the compiler's OWN answer, + * carried instead of discarded. + * + * `compileCelToFilter` already returns `{ reason, detail }` on every refusal; + * until #13639 `compileExpression` consumed `!ok` and threw the rest away one + * line before the only place that could surface it. The extra member is this + * file's own drop, which the compiler reports as a SUCCESS (`ok: true`) and + * {@link isEmptyMembershipFilter} then refuses — same silent denial, so it + * joins the same vocabulary rather than staying unnamed. + */ +type RlsDropReason = CelFilterFailReason | 'empty-membership'; + +/** A dropped policy's cause: the compiler's reason plus its human `detail`. */ +interface RlsDropCause { + reason: RlsDropReason; + /** The compiler's `detail` — names the variable path, the member index, the bound. */ + detail: string; +} + +/** {@link RLSCompiler.compileExpressionOutcome}'s answer: the filter, or why there is none. */ +type RlsCompileOutcome = + | { filter: Record; cause?: undefined } + | { filter: null; cause: RlsDropCause }; /** * RLS User Context @@ -207,6 +233,22 @@ export class RLSCompiler { this.logger = logger; } + /** + * Causes already WARNed about by {@link warnFailClosedDenial}, so a policy + * that denies on every read warns ONCE per distinct cause rather than once + * per request. This seam runs on the read path: the "no active organization" + * denial is a persistent SESSION state, not a one-off, so an un-memoised line + * would be a line per query for as long as the state lasts. + * + * Not a new mechanism — it is `cel-to-filter.ts`'s `warnedOverLimit` memo, the + * immediately-upstream module in this same call chain, at the same bound and + * with the same clear-on-overflow: an unbounded set keyed by author-controlled + * strings is a leak. Per INSTANCE (the plugin holds one long-lived compiler), + * so a test's fresh `new RLSCompiler()` starts from an empty memo. + */ + private warnedDenials = new Set(); + private static readonly WARNED_DENIALS_MAX = 500; + /** * Compile RLS policies into a query filter for the given user context. * Multiple policies for the same object/operation are OR-combined (any match allows access). @@ -252,6 +294,11 @@ export class RLSCompiler { } const filters: Record[] = []; + /** + * [#13639] Policies whose SHAPE was fine but whose evaluation refused — the + * class that produced the reported failure mode: zero rows, no error, no log. + */ + const deniedBy: { policy: RowLevelSecurityPolicy; cause: RlsDropCause }[] = []; let applicable = 0; for (const policy of policies) { @@ -265,19 +312,29 @@ export class RLSCompiler { // WITHOUT counting it toward the fail-closed deny below. if (!predicate) continue; applicable++; - const filter = this.compileExpression(predicate, userCtx); - if (filter) { - filters.push(filter); + const outcome = this.compileExpressionOutcome(predicate, userCtx); + if (outcome.filter) { + filters.push(outcome.filter); } else if (!isSupportedRlsExpression(predicate)) { // ADR-0056 D4: an UNSUPPORTED-SHAPE predicate (e.g. arithmetic, functions, // subqueries) compiles to nothing and would silently vanish, leaving the - // object unprotected. Surface it instead of dropping in silence. (A - // SUPPORTED shape that returned null is the intentional "context var - // absent" path — it fails closed downstream and is not warned here.) + // object unprotected. Surface it instead of dropping in silence. This + // branch is an AUTHORING fault — the predicate can never enforce, for any + // input — so it warns on every drop, denial or not, and it now carries the + // compiler's `detail` (WHICH shape was refused) rather than only the fact. this.logger?.warn?.( `[RLS] policy '${(policy as { name?: string }).name ?? '(unnamed)'}' on '${(policy as { object?: string }).object ?? '?'}' ` + - `has an uncompilable predicate (${clause} clause) and was DROPPED (no enforcement): ${predicate}`, + `has an uncompilable predicate (${clause} clause) and was DROPPED (no enforcement): ${predicate}` + + ` — ${outcome.cause.detail}`, ); + } else { + // [#13639] The previously SILENT branch. The shape is fine; the REQUEST + // could not be evaluated — an unresolved `current_user.*` variable, an + // unresolved MEMBER of a membership array, or an emptied membership set. + // Collected, not warned yet: on its own a dropped policy is not a denial + // (a sibling policy may still grant, and the caller sees rows), so the + // line is emitted below only if this clause actually fails closed. + deniedBy.push({ policy, cause: outcome.cause }); } } @@ -291,6 +348,13 @@ export class RLSCompiler { // expression we couldn't compile). Fail closed — return a sentinel // filter that matches no rows. This prevents the "user without an // active org sees every tenant's data" class of bug. + // + // [#13639] And SAY SO. This is the fail-closed path working as designed, + // which is exactly why it needs a trace: the caller gets zero rows, no + // error is raised, and a correct refusal is indistinguishable from "the + // data genuinely doesn't match" — a search that costs hours and that the + // compiler's own `detail` ends in one line. + for (const { policy, cause } of deniedBy) this.warnFailClosedDenial(policy, clause, cause); return RLS_DENY_FILTER; } if (filters.length === 1) return filters[0]; @@ -299,6 +363,47 @@ export class RLSCompiler { return { $or: filters }; } + /** + * [#13639] The line an operator needs when a read returns nothing. + * + * The failure shape this exists for is the worst-shaped one available: the + * user sees zero rows, no error is raised, and every other signal points away + * from the cause. The information that ends the search — WHICH variable did + * not resolve, and at which member index — was computed by the compiler and + * then discarded one line before it could be used. This carries it. + * + * Emitted only when the clause actually DENIES (see {@link compileFilter}) and + * only once per distinct cause, because this seam runs on read paths. + */ + private warnFailClosedDenial( + policy: RowLevelSecurityPolicy, + clause: 'using' | 'check', + cause: RlsDropCause, + ): void { + const name = (policy as { name?: string }).name ?? '(unnamed)'; + const object = (policy as { object?: string }).object ?? '?'; + const predicate = (policy as { using?: string; check?: string })[clause] ?? policy.using ?? ''; + const key = `${object}|${clause}|${name}|${cause.reason}|${cause.detail}`; + if (this.warnedDenials.has(key)) return; + if (this.warnedDenials.size >= RLSCompiler.WARNED_DENIALS_MAX) this.warnedDenials.clear(); + this.warnedDenials.add(key); + this.logger?.warn?.( + `[RLS] DENY (fail closed): policy '${name}' on '${object}' could not be evaluated for this request ` + + `(${clause} clause, ${cause.reason}): ${cause.detail}. Every applicable policy dropped, so the request ` + + `is filtered by RLS_DENY_FILTER ('${String(RLS_DENY_FILTER.id).split(':')[0]}') and returns ZERO ROWS — ` + + `a deliberate REFUSAL, not an empty result set. Predicate: ${predicate}`, + { + object, + policy: name, + clause, + reason: cause.reason, + detail: cause.detail, + predicate, + filter: RLS_DENY_FILTER.id, + }, + ); + } + /** * Compile a single RLS predicate into a query filter (ADR-0058 D1/D2). * @@ -323,7 +428,22 @@ export class RLSCompiler { expression: string, userCtx: RLSUserContext ): Record | null { - if (!expression) return null; + return this.compileExpressionOutcome(expression, userCtx).filter; + } + + /** + * [#13639] {@link compileExpression}'s answer WITH the reason it refused. + * + * Same compile, same decision, same returned filter — the only difference is + * that the compiler's `{ reason, detail }` survives to the caller instead of + * being collapsed into `null` at the `!result.ok` line. `compileExpression` + * stays exactly as published (`Record | null`) and delegates here. + */ + private compileExpressionOutcome( + expression: string, + userCtx: RLSUserContext + ): RlsCompileOutcome { + if (!expression) return { filter: null, cause: { reason: 'parse-error', detail: 'empty expression' } }; // [ADR-0058 D1] CEL is canonical. The legacy SQL-ish form still compiles via // the transitional bridge, but we surface it so authored policies migrate to // CEL — the bridge will be removed once no stored predicate needs it. @@ -340,7 +460,11 @@ export class RLSCompiler { // Any fault — unsupported shape, parse error, or an unresolved/null // `current_user.*` variable — drops the policy. With a single applicable // policy this surfaces as RLS_DENY_FILTER upstream (fail closed). - if (!result.ok) return null; + // [#13639] The refusal keeps its REASON. `reason` + `detail` are what the + // compiler already computed — the variable path, the member index, the bound + // that was overrun — and discarding them here is what left an operator with + // zero rows and no signal at all. + if (!result.ok) return { filter: null, cause: { reason: result.reason, detail: result.detail } }; // Parity: an empty pre-resolved membership (`field in current_user.`) // compiles to `{ field: { $in: [] } }`. The legacy compiler dropped the // policy in this case; preserve that so the deny sentinel (not a literal @@ -348,8 +472,21 @@ export class RLSCompiler { // polarity-aware: the same emptied set under a supported `not in` // (`$not` wrapping, at any composition depth) is dropped too — otherwise // it inverts to a constant-TRUE clause and the policy compiles ALLOW-ALL. - if (isEmptyMembershipFilter(result.filter as Record)) return null; - return result.filter as Record; + if (isEmptyMembershipFilter(result.filter as Record)) { + // [#13639] The compiler answered `ok`, so there is no `detail` to carry — + // this drop is THIS file's, and it is just as silent and just as + // fail-closed. It names itself rather than being reported as a success. + return { + filter: null, + cause: { + reason: 'empty-membership', + detail: + 'a pre-resolved membership set is EMPTY, so the policy is degenerate ' + + `(compiled to ${JSON.stringify(result.filter)}) and was dropped rather than enforced`, + }, + }; + } + return { filter: result.filter as Record }; } /** diff --git a/packages/plugins/plugin-security/src/rls-denial-observability.test.ts b/packages/plugins/plugin-security/src/rls-denial-observability.test.ts new file mode 100644 index 0000000000..f9e031e201 --- /dev/null +++ b/packages/plugins/plugin-security/src/rls-denial-observability.test.ts @@ -0,0 +1,291 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#13639] An RLS denial caused by an unresolved variable must leave a TRACE. + * + * The measured failure shape is the worst-shaped one an operator can be handed: + * the user sees zero rows, no error is raised, nothing appears in the log, and + * the built-in explanation tool says the policy "narrows" rather than "denies" — + * every available signal points away from the cause. And the denial is + * DELIBERATE (fail-closed working as designed), which is precisely why it needs + * a trace: a correct refusal that is indistinguishable from "the data genuinely + * doesn't match" costs hours, and the information that ends that search — the + * variable path, the member index — was computed by `compileCelToFilter` and + * then discarded one line before it could be used. + * + * What these pins hold, in the order the repair has to satisfy them: + * 1. the refusal now LOGS, and the line carries the REASON (the variable path + * and the index), not merely the fact that something was dropped; + * 2. the DECISION is byte-for-byte unmoved — `RLS_DENY_FILTER` still lands, + * `compileExpression` still returns `null`, the sentinel still excludes + * every record it is matched against. This is an observability change and + * nothing about the verdict may move; + * 3. the negatives — a shape that correctly logged BEFORE still logs (exactly + * once, with its own message), a normal resolving expression logs nothing, + * and a dropped policy that is NOT a denial (a sibling still grants) stays + * silent, because that caller sees rows and has no mystery to debug; + * 4. the line does not become noise on a read path: once per distinct cause, + * not once per request. + * + * ⛔ Deliberately NOT covered here: `explain`'s `isDenyAll` / the + * `__deny_all__` vs `__rls_deny__` sentinel question. That is a design fork the + * filer declined to answer and this slice does not touch. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { RowLevelSecurityPolicy } from '@objectstack/spec/security'; +import { matchesFilterCondition } from '@objectstack/formula'; + +import { RLSCompiler, RLS_DENY_FILTER } from './rls-compiler.js'; + +type WarnCall = [string, Record?]; + +function compilerWithLogger() { + const warn = vi.fn(); + const compiler = new RLSCompiler(); + compiler.setLogger({ warn }); + const lines = () => (warn.mock.calls as WarnCall[]).map((c) => String(c[0])); + return { compiler, warn, lines }; +} + +function policy(using: string, name = 'territory_scope', object = 'task'): RowLevelSecurityPolicy { + return { name, object, operation: 'select', using } as RowLevelSecurityPolicy; +} + +/** A membership array with a NULL member — the shape #13630 added a refusal for. */ +const MEMBER_NULL_CTX = { + userId: 'u1', + tenantId: 'org-1', + rlsMembership: { territory_user_ids: ['u2', null, 'u4'] }, +} as never; + +/** No active organization — the long-standing SCALAR fail-closed path. */ +const NO_ORG_CTX = { userId: 'u1' } as never; + +describe('#13639 — a fail-closed RLS denial logs, with the REASON', () => { + it('a membership shape whose member is null: the line names the VARIABLE PATH and the INDEX', () => { + const { compiler, warn, lines } = compilerWithLogger(); + + const filter = compiler.compileFilter( + [policy('assigned_to_id in current_user.territory_user_ids')], + MEMBER_NULL_CTX, + ); + + // The decision is the same fail-closed one it has always been. + expect(filter).toEqual(RLS_DENY_FILTER); + + // …and it is no longer silent. + expect(warn).toHaveBeenCalledTimes(1); + const [message, meta] = warn.mock.calls[0] as WarnCall; + + // The REASON, not merely the fact — this is the whole defect. The compiler + // computed every one of these and they were thrown away at `!result.ok`. + expect(message).toContain('current_user.territory_user_ids'); // WHICH variable + expect(message).toContain('unresolved member at index 1'); // WHICH member + expect(message).toContain('assigned_to_id in current_user.territory_user_ids'); // WHICH predicate + expect(message).toContain("policy 'territory_scope'"); // WHICH policy + expect(message).toContain("on 'task'"); // WHICH object + // And the consequence, so "zero rows" is readable as a refusal. + expect(message).toContain('ZERO ROWS'); + expect(message).toContain('__rls_deny__'); + + expect(meta).toMatchObject({ + object: 'task', + policy: 'territory_scope', + clause: 'using', + reason: 'unresolved-variable', + filter: RLS_DENY_FILTER.id, + }); + expect(String(meta?.detail)).toContain('unresolved member at index 1'); + + expect(lines()).toHaveLength(1); + }); + + it('the SCALAR "no active organization" path names its variable too', () => { + const { compiler, warn } = compilerWithLogger(); + + const filter = compiler.compileFilter( + [policy('organization_id == current_user.organization_id', 'tenant_isolation')], + NO_ORG_CTX, + ); + + expect(filter).toEqual(RLS_DENY_FILTER); + expect(warn).toHaveBeenCalledTimes(1); + const [message, meta] = warn.mock.calls[0] as WarnCall; + expect(message).toContain('current_user.organization_id'); + expect(message).toContain('DENY (fail closed)'); + expect(meta).toMatchObject({ reason: 'unresolved-variable' }); + }); + + it('an EMPTY pre-resolved membership set names itself (the compiler answers `ok`, so it has no detail to carry)', () => { + const { compiler, warn } = compilerWithLogger(); + + const filter = compiler.compileFilter( + [policy('assigned_to_id in current_user.territory_user_ids')], + { userId: 'u1', tenantId: 'org-1', rlsMembership: { territory_user_ids: [] } } as never, + ); + + expect(filter).toEqual(RLS_DENY_FILTER); + expect(warn).toHaveBeenCalledTimes(1); + const [message, meta] = warn.mock.calls[0] as WarnCall; + expect(message).toContain('membership set is EMPTY'); + expect(meta).toMatchObject({ reason: 'empty-membership' }); + }); + + it('the `check` clause denies and reports under its own clause name (ADR-0058 D4)', () => { + const { compiler, warn } = compilerWithLogger(); + + const p = { + name: 'own_rows', + object: 'task', + operation: 'update', + using: 'owner_id == current_user.id', + check: 'assigned_to_id in current_user.territory_user_ids', + } as RowLevelSecurityPolicy; + + expect(compiler.compileFilter([p], MEMBER_NULL_CTX, 'check')).toEqual(RLS_DENY_FILTER); + expect(warn).toHaveBeenCalledTimes(1); + const [message, meta] = warn.mock.calls[0] as WarnCall; + expect(message).toContain('check clause'); + expect(meta).toMatchObject({ clause: 'check', reason: 'unresolved-variable' }); + }); +}); + +describe('#13639 — the DECISION is unchanged (observability only)', () => { + it('`__rls_deny__` still lands, and still excludes every record it is matched against', () => { + const { compiler } = compilerWithLogger(); + + const filter = compiler.compileFilter( + [policy('assigned_to_id in current_user.territory_user_ids')], + MEMBER_NULL_CTX, + ); + + // The sentinel itself, unchanged — value, shape and identity. + expect(filter).toEqual(RLS_DENY_FILTER); + expect(filter).toEqual({ id: '__rls_deny__:00000000-0000-0000-0000-000000000000' }); + expect(Object.keys(filter as object)).toEqual(['id']); + + // Record attribution still excludes: zero rows still means zero rows. This + // is the same predicate test `explain`'s record-grained pass runs. + for (const record of [ + { id: 'rec-1', assigned_to_id: 'u2' }, + { id: '00000000-0000-0000-0000-000000000000', assigned_to_id: 'u4' }, + { id: '', assigned_to_id: null }, + ]) { + expect(matchesFilterCondition(record, filter as never)).toBe(false); + } + }); + + it('`compileExpression` keeps its published `Record | null` contract', () => { + const { compiler } = compilerWithLogger(); + const ctx = { id: 'u1', territory_user_ids: ['u2', null] } as never; + + // The refusal is still spelled `null` — the reason rides on the internal + // seam, never on this method's return type. + expect(compiler.compileExpression('assigned_to_id in current_user.territory_user_ids', ctx)).toBeNull(); + expect(compiler.compileExpression('', ctx)).toBeNull(); + // …and a resolving predicate still returns exactly the filter it always did. + expect(compiler.compileExpression('owner_id == current_user.id', ctx)).toEqual({ owner_id: 'u1' }); + }); + + it('a policy that RESOLVES is unaffected — same filter, no sentinel', () => { + const { compiler } = compilerWithLogger(); + const filter = compiler.compileFilter( + [policy('assigned_to_id in current_user.territory_user_ids')], + { userId: 'u1', rlsMembership: { territory_user_ids: ['u2', 'u4'] } } as never, + ); + expect(filter).toEqual({ assigned_to_id: { $in: ['u2', 'u4'] } }); + expect(filter).not.toEqual(RLS_DENY_FILTER); + }); +}); + +describe('#13639 — the negatives', () => { + it('a normal RESOLVING expression logs nothing at all', () => { + const { compiler, warn } = compilerWithLogger(); + const filter = compiler.compileFilter( + [policy('owner_id == current_user.id', 'own_rows')], + { userId: 'u1', tenantId: 'org-1' } as never, + ); + expect(filter).toEqual({ owner_id: 'u1' }); + expect(warn).not.toHaveBeenCalled(); + }); + + it('a shape that CORRECTLY logged before still logs — once, with its own message', () => { + const { compiler, warn, lines } = compilerWithLogger(); + + // Arithmetic: genuinely non-pushdownable, an AUTHORING fault. It earned the + // ADR-0056 D4 "DROPPED (no enforcement)" line before this change and must + // still earn exactly that one line — not a second, differently-worded one. + const filter = compiler.compileFilter([policy('amount + 1 > 2', 'bad')], { userId: 'u1' } as never); + + expect(filter).toEqual(RLS_DENY_FILTER); + expect(warn).toHaveBeenCalledTimes(1); + expect(lines()[0]).toContain('uncompilable predicate'); + expect(lines()[0]).toContain('DROPPED (no enforcement)'); + // …now carrying the compiler's reason as well as the fact. + expect(lines()[0]).toContain('unsupported operand "+"'); + expect(lines()[0]).not.toContain('DENY (fail closed)'); + }); + + it('a dropped policy that is NOT a denial stays silent — the caller sees rows', () => { + const { compiler, warn } = compilerWithLogger(); + + // One policy cannot resolve; a sibling grants. The caller gets rows, so + // there is no mystery to explain and no line to spend on a read path. + const filter = compiler.compileFilter( + [ + policy('assigned_to_id in current_user.territory_user_ids', 'territory'), + policy('owner_id == current_user.id', 'own_rows'), + ], + MEMBER_NULL_CTX, + ); + + expect(filter).toEqual({ owner_id: 'u1' }); + expect(warn).not.toHaveBeenCalled(); + }); + + it('no applicable policy for the clause is NOT a denial and logs nothing', () => { + const { compiler, warn } = compilerWithLogger(); + const checkOnly = { name: 'post_image', object: 'task', operation: 'all', check: 'x == 1' } as RowLevelSecurityPolicy; + expect(compiler.compileFilter([checkOnly], NO_ORG_CTX, 'using')).toBeNull(); + expect(warn).not.toHaveBeenCalled(); + }); +}); + +describe('#13639 — the line is a trace, not a flood (this seam runs on read paths)', () => { + it('the SAME cause across repeated requests warns ONCE', () => { + const { compiler, warn } = compilerWithLogger(); + const p = policy('assigned_to_id in current_user.territory_user_ids'); + + for (let i = 0; i < 25; i++) { + expect(compiler.compileFilter([p], MEMBER_NULL_CTX)).toEqual(RLS_DENY_FILTER); + } + + // 25 denials — all correct, all fail-closed — and one line to read. + expect(warn).toHaveBeenCalledTimes(1); + }); + + it('a DIFFERENT cause is a different line (dedup never swallows a new fact)', () => { + const { compiler, warn, lines } = compilerWithLogger(); + const p = policy('assigned_to_id in current_user.territory_user_ids'); + + compiler.compileFilter([p], MEMBER_NULL_CTX); + // Same policy, same variable — a different member index is a different cause. + compiler.compileFilter([p], { + userId: 'u1', + rlsMembership: { territory_user_ids: ['u2', 'u4', undefined] }, + } as never); + // A different policy on a different object is a different cause too. + compiler.compileFilter([policy('org_id == current_user.organization_id', 'wall', 'deal')], NO_ORG_CTX); + + expect(warn).toHaveBeenCalledTimes(3); + expect(lines()[0]).toContain('index 1'); + expect(lines()[1]).toContain('index 2'); + expect(lines()[2]).toContain("on 'deal'"); + }); + + it('a compiler with NO logger bound never throws on the deny path', () => { + const compiler = new RLSCompiler(); + expect(compiler.compileFilter([policy('x in current_user.nope')], NO_ORG_CTX)).toEqual(RLS_DENY_FILTER); + }); +}); diff --git a/packages/plugins/plugin-security/src/security-plugin.test.ts b/packages/plugins/plugin-security/src/security-plugin.test.ts index f408bc465c..ffdf2caecb 100644 --- a/packages/plugins/plugin-security/src/security-plugin.test.ts +++ b/packages/plugins/plugin-security/src/security-plugin.test.ts @@ -3460,15 +3460,22 @@ describe('RLSCompiler D4 — uncompilable predicates are surfaced', () => { expect(warned[0]).toContain('uncompilable predicate'); }); - it('does NOT warn a SUPPORTED shape whose context var is merely absent', () => { + it('a SUPPORTED shape whose context var is absent gets the DENY line, not the uncompilable one', () => { const warned: string[] = []; const compiler = new RLSCompiler(); compiler.setLogger({ warn: (message: string) => warned.push(message) }); - // valid shape; `department` simply isn't in the context → intentional fail-closed skip. - // CEL form (SQL `=` would now additionally emit a deprecation warn). + // Valid shape; `department` simply isn't in the context. This is the + // fail-closed path, NOT an authoring fault — so it must not be reported as + // "uncompilable". [#13639] It used to be reported as nothing at all: the + // caller got zero rows, no error, and an empty log, which is the exact + // failure shape that card measured. It now says which variable did not + // resolve. (CEL form; SQL `=` would additionally emit a deprecation warn.) const policy: any = { name: 'dept', object: 'thing', operation: 'select', using: 'dept == current_user.department' }; - compiler.compileFilter([policy], { userId: 'u1' } as any); - expect(warned.length).toBe(0); + expect(compiler.compileFilter([policy], { userId: 'u1' } as any)).toEqual(RLS_DENY_FILTER); + expect(warned.length).toBe(1); + expect(warned[0]).toContain('DENY (fail closed)'); + expect(warned[0]).toContain('current_user.department'); + expect(warned[0]).not.toContain('uncompilable predicate'); }); });