Skip to content

Commit 66005f5

Browse files
committed
fix(formula): refuse the bare variable root and object comparands under == / !=
A CEL predicate comparing a field to the bare `current_user` root lowered against the whole caller context object, so a `check` written `record.owner_id != current_user` admitted every write. The root is now refused before resolution in both compile modes (the shape check reports it), and a variable resolving to an object is refused per request. Claude-Session: https://claude.ai/code/session_01CiCTczDo7tGhafXjf61dUJ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9347c1f commit 66005f5

1 file changed

Lines changed: 91 additions & 9 deletions

File tree

‎packages/formula/src/cel-to-filter.ts‎

Lines changed: 91 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@
4242
* `unresolved-variable` (the "no active org" fail-closed path) — and so does a
4343
* null/undefined MEMBER of a resolved membership array, which is the same
4444
* unresolved value one level in. See {@link lowerMembership} for why the member
45-
* is refused rather than dropped.
45+
* is refused rather than dropped. The ROOT alone (`current_user`) is the whole
46+
* context object, never a value: `==` / `!=` refuse it (see
47+
* {@link variableObjectComparandRefusal}).
4648
*/
4749

4850
import type { ASTNode } from '@marcbachmann/cel-js';
@@ -347,15 +349,20 @@ function lowerComparison(op: string, lNode: ASTNode, rNode: ASTNode, ctx: Ctx):
347349
}
348350
// [#19886] `==` / `!=` against a comparand that IS a list — a list literal, or
349351
// a `current_user` variable that resolves to an array — is refused before it
350-
// is emitted. See {@link arrayComparandRefusal}.
352+
// is emitted. See {@link arrayComparandRefusal}. [#19959] So is the variable
353+
// root alone, and a variable that resolves to an object: see
354+
// {@link variableObjectComparandRefusal}.
351355
if (lField) return emit((L as { path: string }).path, op, comparandOf(op, R, ctx), false);
352356
if (rField) return emit((R as { path: string }).path, FLIP[op] ?? op, comparandOf(op, L, ctx), false);
353357

354358
// Neither side is a field: a constant comparison. Fold the always-true case
355359
// (`1 == 1`, the RLS allow-all) to "no restriction"; refuse the rest (a
356-
// non-true constant must fail closed, never become allow-all).
357-
const lv = resolveValue(L, ctx);
358-
const rv = resolveValue(R, ctx);
360+
// non-true constant must fail closed, never become allow-all). [#19959] A
361+
// variable root or object operand is refused first: `current_user != 'guest'`
362+
// folded to "no restriction" because an object is never strictly equal to a
363+
// literal, which is the same whole-object comparison one branch over.
364+
const lv = variableOperandOf(op, L, ctx);
365+
const rv = variableOperandOf(op, R, ctx);
359366
if (ctx.mode === 'shape') return {}; // shape check: structurally fine
360367
const truth = constFold(op, lv, rv);
361368
if (truth === true) return {};
@@ -484,12 +491,87 @@ function arrayComparandRefusal(op: string, leaf: Leaf): CompileError {
484491
}
485492

486493
/**
487-
* Resolve the non-field side of a comparison, refusing a list under `==` / `!=`
488-
* ({@link arrayComparandRefusal}). In shape mode a variable resolves to the
489-
* placeholder, so only a literal list is refused there.
494+
* [#19959] `==` / `!=` compare ONE value, and a variable ROOT is not one value.
495+
*
496+
* `record.owner_id != current_user` names the root alone, and the root is the
497+
* whole context object the caller supplies — for the RLS compiler every
498+
* kernel-resolved key at once, the membership arrays included. Until this
499+
* refusal it lowered to `{ owner_id: { $ne: <that object> } }` (and `==` to the
500+
* bare object, with `$not` around it for `!(… == current_user)`). A strict
501+
* compare never equals an object, so the RLS `check` evaluator admitted every
502+
* write the policy was written to refuse, and explain reported the read as
503+
* narrowed while echoing the caller's membership sets in its `readFilter`.
504+
*
505+
* ADR-0058 D2 declares the operand opposite a field as a literal, a
506+
* `current_user.*` scalar or a pre-resolved `current_user.<key>` set, and the
507+
* published `$eq` / `$ne` contract (`FieldOperatorsSchema`) as a literal or a
508+
* `{ $field }` reference. The root is none of them, so the refusal pulls the
509+
* lowering back to the declared operand set; it narrows nothing either names.
510+
*
511+
* Two guards, one per point at which the fault is knowable:
512+
*
513+
* - The ROOT alone is known from the source (a path of one segment), so it is
514+
* refused BEFORE resolution, in both modes: the shape check
515+
* (`isPushdownableCel`, so `isSupportedRlsExpression` and the authoring lint)
516+
* reports it before any request, exactly as it reports a list literal, and
517+
* every compile — bound variables or none — gives the same `unsupported`.
518+
* - A `current_user.<key>` that RESOLVES to an object is knowable only per
519+
* request, like a resolved array, so it is refused after resolution. No
520+
* in-tree producer binds an object-valued key (the RLS context's keys are
521+
* scalars and membership arrays); the guard closes the class for any caller
522+
* of this published compiler. A `Date` is a literal comparand in the
523+
* `$eq` / `$ne` contract and passes.
524+
*
525+
* Both guards apply wherever `==` / `!=` resolves an operand: opposite a field,
526+
* and on either side of a constant comparison, where `current_user != 'guest'`
527+
* folded to "no restriction" because an object is never strictly equal to a
528+
* literal. `{ $field }` references never reach here (the field-to-field branch
529+
* emits them with `isRef`), and a CEL map literal is refused by
530+
* {@link classify}, so a resolved variable is the only way an object ever became
531+
* a comparand. Ordering operators keep their existing lowering: they are outside
532+
* the `==` / `!=` letter this refusal carries. The message names the variable
533+
* PATH the author wrote and never a value.
490534
*/
491-
function comparandOf(op: string, leaf: Leaf, ctx: Ctx): unknown {
535+
function variableObjectComparandRefusal(op: string, path: string[]): CompileError {
536+
const written = path.join('.');
537+
const comparand =
538+
path.length === 1
539+
? `its comparand \`${written}\` is the variable root itself, the whole context object`
540+
: `its comparand \`${written}\` resolves to an object`;
541+
return new CompileError(
542+
'unsupported',
543+
`\`${op}\` compares one value, but ${comparand} — compare one of its keys instead, ` +
544+
`e.g. \`record.owner_id ${op} ${path[0]}.id\` rather than \`record.owner_id ${op} ${written}\``,
545+
);
546+
}
547+
548+
/** A resolved value that is an object rather than one comparable value. */
549+
function isObjectComparand(value: unknown): boolean {
550+
return value !== null && typeof value === 'object' && !Array.isArray(value) && !(value instanceof Date);
551+
}
552+
553+
/**
554+
* Resolve a comparison operand, refusing under `==` / `!=` a variable root
555+
* before resolution and a variable resolving to an object after it
556+
* ({@link variableObjectComparandRefusal}). In shape mode a variable resolves to
557+
* the placeholder, so only the root is refused there.
558+
*/
559+
function variableOperandOf(op: string, leaf: Leaf, ctx: Ctx): unknown {
560+
const equality = op === '==' || op === '!=';
561+
if (equality && leaf.kind === 'var' && leaf.path.length === 1) throw variableObjectComparandRefusal(op, leaf.path);
492562
const value = resolveValue(leaf, ctx);
563+
if (equality && leaf.kind === 'var' && isObjectComparand(value)) throw variableObjectComparandRefusal(op, leaf.path);
564+
return value;
565+
}
566+
567+
/**
568+
* Resolve the non-field side of a comparison, refusing under `==` / `!=` a list
569+
* ({@link arrayComparandRefusal}) and a variable root or object
570+
* ({@link variableOperandOf}). In shape mode a variable resolves to the
571+
* placeholder, so only a literal list and a bare root are refused there.
572+
*/
573+
function comparandOf(op: string, leaf: Leaf, ctx: Ctx): unknown {
574+
const value = variableOperandOf(op, leaf, ctx);
493575
if ((op === '==' || op === '!=') && Array.isArray(value)) throw arrayComparandRefusal(op, leaf);
494576
return value;
495577
}

0 commit comments

Comments
 (0)