Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .changeset/19959-cel-variable-root-comparand-refused.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
"@objectstack/formula": minor
"@objectstack/plugin-security": minor
"@objectstack/plugin-sharing": minor
"@objectstack/lint": minor
"@objectstack/spec": patch
---

A row-level or sharing-rule predicate comparing with `!=` / `==` against the bare `current_user` root is refused at the CEL lowering instead of lowering against the whole caller context object (#19959).

**BREAKING** — an accept-set narrowing, shipped by `@objectstack/formula`, `@objectstack/plugin-security`, `@objectstack/plugin-sharing` and `@objectstack/lint` as `minor` under the repo's launch-window convention for accept-set narrowings. The hand-migration prescription is registered under protocol major 18 as `cel-predicate-variable-root-comparand-refused`.

Clause-②: no (narrowing)

**Security fix for RLS write checks.** A policy written `record.owner_id != current_user` (or `== current_user`, or `!(record.owner_id == current_user)`) named the variable root alone, which resolved to the whole caller context, and lowered to `{ owner_id: { $ne: <that object> } }` (or the bare object, or `$not` around it). A strict compare never equals an object, so a `check` so written admitted and stored every insert and by-id update it was written to refuse, a USING-only such policy admitted every insert, and explain reported the read as narrowed with the caller's membership sets echoed in its `readFilter`. A constant comparison such as `current_user != 'guest'` folded to no restriction.

- `@objectstack/formula`: `compileCelToFilter` refuses `==` / `!=` whose operand is the bare variable root (`unsupported`), in both of its modes, so the authoring shape check (`isPushdownableCel`, `isSupportedRlsExpression`) reports it before any request. A variable that resolves to an object is refused per request; a `Date` still passes.
- `@objectstack/plugin-security`: the RLS compiler drops such a policy and fails closed when no other policy applies (`RLS_DENY_FILTER`: reads return no rows, `check` writes are refused 403, explain answers `denies`).
- `@objectstack/plugin-sharing`: a declared sharing rule with such a `condition` is still skipped at bootstrap, now with reason `unsupported` instead of `unresolved-variable`.
- `@objectstack/lint`: the shape is reported as `rls-predicate-unenforceable` on either RLS clause, where it was silent, and as `sharing-rule-unlowerable-condition` on a sharing condition, where it was `sharing-rule-runtime-variable-condition`.
- `@objectstack/spec`: the migration registry carries the entry.

**What to change.** Compare against the key the predicate means: `record.owner_id != current_user` becomes `record.owner_id != current_user.id` (or `current_user.organization_id`, `current_user.email`); a membership test is `record.owner_id in current_user.org_user_ids`. Scalar keys, `in`, `null`, literals and field-to-field comparisons are unchanged.

<!-- adr-0087: registered cel-predicate-variable-root-comparand-refused -->
156 changes: 156 additions & 0 deletions packages/formula/src/cel-to-filter-variable-root-comparand.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#19959] `==` / `!=` against the bare variable ROOT (`current_user`), or a
* variable that resolves to an object, is a compile error, not a lowering.
*
* `record.owner_id != current_user` used to lower to
* `{ owner_id: { $ne: <the whole caller context> } }`, `==` to the bare object,
* and `!(… == current_user)` to `$not` around it. A strict compare never equals
* an object, so the RLS `check` evaluator admitted every write the policy was
* written to refuse. On the constant branch `current_user != 'guest'` folded to
* "no restriction" for the same reason.
*
* The root is known from the source, so it is refused in BOTH modes — the shape
* check (`isPushdownableCel` / `isSupportedRlsExpression`, which the authoring
* lint and the RLS compiler's "uncompilable predicate" branch read) and every
* compile, whatever it binds. An object-valued `current_user.<key>` exists only
* per request, so that refusal is pinned at request time with the shape check
* still passing.
*/

import { describe, expect, it } from 'vitest';

import { compileCelToFilter, isPushdownableCel } from './cel-to-filter';
import { isSupportedRlsExpression } from './rls-predicate';

/** The RLS compiler's context shape: scalar keys and membership arrays. */
const VARS = {
current_user: {
id: 'u_me',
organization_id: 'org_me',
email: 'me@example.test',
positions: ['pos_me'],
org_user_ids: ['u_me', 'u_peer'],
accessible_org_ids: ['org_me'],
// Not a key any in-tree producer binds: the per-request object guard's subject.
profile: { team: 'team_secret' },
since: new Date('2026-01-01T00:00:00.000Z'),
},
};

/** Every value in {@link VARS} — none may appear in a refusal's detail. */
const VALUES = ['u_me', 'org_me', 'me@example.test', 'pos_me', 'u_peer', 'team_secret'];

const detailOf = (source: string, variables: Record<string, unknown>) => {
const compiled = compileCelToFilter(source, { variables });
expect(compiled.ok).toBe(false);
expect(compiled.ok ? undefined : compiled.reason).toBe('unsupported');
return compiled.ok ? '' : compiled.detail;
};

const REFUSED_ROOT = [
'record.owner_id != current_user',
'record.owner_id == current_user',
'!(record.owner_id == current_user)',
'!(record.owner_id != current_user)',
'current_user != record.owner_id',
'current_user == record.owner_id',
"record.status == 'open' && record.owner_id != current_user",
"record.status == 'open' || !(record.owner_id == current_user)",
// An always-true sibling never folds the refused arm into allow-all.
'1 == 1 || record.owner_id != current_user',
// The constant branch: an object is never strictly equal to a literal, so
// these folded to "no restriction".
"current_user != 'guest'",
"'guest' != current_user",
'current_user != null',
"record.status == 'open' || current_user != 'guest'",
];

describe('[#19959] == / != against the bare variable root is refused, at authoring time and at request time', () => {
for (const source of REFUSED_ROOT) {
it(`${source} — unsupported, naming the root and withholding the context`, () => {
const detail = detailOf(source, VARS);
expect(detail).toContain('`current_user` is the variable root itself');
expect(detail).toContain('current_user.id');
for (const value of VALUES) expect(detail).not.toContain(value);
// The same verdict with nothing bound — the sharing seeder's call.
expect(detailOf(source, {})).toBe(detail);
// Visible before any request: the lint and the RLS compiler's
// "uncompilable predicate" branch read this.
expect(isPushdownableCel(source).ok).toBe(false);
expect(isSupportedRlsExpression(source)).toBe(false);
});
}

it('the legacy SQL-ish spelling is refused by the authoring gate after the bridge', () => {
expect(isSupportedRlsExpression('owner_id = current_user')).toBe(false);
});

it('a custom variable root is refused the same way', () => {
const opts = { fieldRoots: ['row'], variableRoots: ['ctx'], variables: { ctx: { department: 'sales' } } };
const compiled = compileCelToFilter('row.dept != ctx', opts);
expect(compiled.ok ? undefined : compiled.reason).toBe('unsupported');
expect(compiled.ok ? '' : compiled.detail).toContain('`ctx` is the variable root itself');
expect(isPushdownableCel('row.dept != ctx', opts).ok).toBe(false);
});
});

const REFUSED_OBJECT = [
'record.team == current_user.profile',
'record.team != current_user.profile',
'!(record.team == current_user.profile)',
'current_user.profile != record.team',
"current_user.profile != 'guest'",
];

describe('[#19959] == / != against a variable that RESOLVES to an object is refused at request time', () => {
for (const source of REFUSED_OBJECT) {
it(`${source} — unsupported, naming the path and withholding its value`, () => {
const detail = detailOf(source, VARS);
expect(detail).toContain('`current_user.profile` resolves to an object');
for (const value of VALUES) expect(detail).not.toContain(value);
// The value is per request, so the shape check passes the source.
expect(isPushdownableCel(source).ok).toBe(true);
expect(isSupportedRlsExpression(source)).toBe(true);
});
}
});

describe('[#19959] every neighbouring comparison lowers exactly as before', () => {
const ok = (source: string) => {
const r = compileCelToFilter(source, { variables: VARS });
if (!r.ok) throw new Error(`expected "${source}" to lower, got ${r.reason}: ${r.detail}`);
return r.filter;
};

it('a scalar key of the root — the spelling the refusal points at', () => {
expect(ok('record.owner_id == current_user.id')).toEqual({ owner_id: 'u_me' });
expect(ok('record.owner_id != current_user.id')).toEqual({ owner_id: { $ne: 'u_me' } });
expect(ok('!(record.owner_id == current_user.id)')).toEqual({ $not: { owner_id: 'u_me' } });
expect(ok('current_user.email == record.owner')).toEqual({ owner: 'me@example.test' });
expect(ok('record.organization_id == current_user.organization_id')).toEqual({ organization_id: 'org_me' });
});

it('a Date-valued key is a literal comparand and passes', () => {
expect(ok('record.created_at == current_user.since')).toEqual({ created_at: VARS.current_user.since });
});

it('membership sets under `in` / `not in`', () => {
expect(ok('record.owner_id in current_user.org_user_ids')).toEqual({ owner_id: { $in: ['u_me', 'u_peer'] } });
expect(ok('!(record.owner_id in current_user.org_user_ids)')).toEqual({
$not: { owner_id: { $in: ['u_me', 'u_peer'] } },
});
});

it('constant comparisons over scalar keys and literals', () => {
expect(ok('1 == 1')).toEqual({});
expect(ok("current_user.id != 'guest'")).toEqual({});
});

it('field-to-field and null comparisons', () => {
expect(ok('record.owner == record.manager')).toEqual({ owner: { $eq: { $field: 'manager' } } });
expect(ok('record.closed_at != null')).toEqual({ closed_at: { $null: false } });
});
});
100 changes: 91 additions & 9 deletions packages/formula/src/cel-to-filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@
* `unresolved-variable` (the "no active org" fail-closed path) — and so does a
* null/undefined MEMBER of a resolved membership array, which is the same
* unresolved value one level in. See {@link lowerMembership} for why the member
* is refused rather than dropped.
* is refused rather than dropped. The ROOT alone (`current_user`) is the whole
* context object, never a value: `==` / `!=` refuse it (see
* {@link variableObjectComparandRefusal}).
*/

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

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

/**
* Resolve the non-field side of a comparison, refusing a list under `==` / `!=`
* ({@link arrayComparandRefusal}). In shape mode a variable resolves to the
* placeholder, so only a literal list is refused there.
* [#19959] `==` / `!=` compare ONE value, and a variable ROOT is not one value.
*
* `record.owner_id != current_user` names the root alone, and the root is the
* whole context object the caller supplies — for the RLS compiler every
* kernel-resolved key at once, the membership arrays included. Until this
* refusal it lowered to `{ owner_id: { $ne: <that object> } }` (and `==` to the
* bare object, with `$not` around it for `!(… == current_user)`). A strict
* compare never equals an object, so the RLS `check` evaluator admitted every
* write the policy was written to refuse, and explain reported the read as
* narrowed while echoing the caller's membership sets in its `readFilter`.
*
* ADR-0058 D2 declares the operand opposite a field as a literal, a
* `current_user.*` scalar or a pre-resolved `current_user.<key>` set, and the
* published `$eq` / `$ne` contract (`FieldOperatorsSchema`) as a literal or a
* `{ $field }` reference. The root is none of them, so the refusal pulls the
* lowering back to the declared operand set; it narrows nothing either names.
*
* Two guards, one per point at which the fault is knowable:
*
* - The ROOT alone is known from the source (a path of one segment), so it is
* refused BEFORE resolution, in both modes: the shape check
* (`isPushdownableCel`, so `isSupportedRlsExpression` and the authoring lint)
* reports it before any request, exactly as it reports a list literal, and
* every compile — bound variables or none — gives the same `unsupported`.
* - A `current_user.<key>` that RESOLVES to an object is knowable only per
* request, like a resolved array, so it is refused after resolution. No
* in-tree producer binds an object-valued key (the RLS context's keys are
* scalars and membership arrays); the guard closes the class for any caller
* of this published compiler. A `Date` is a literal comparand in the
* `$eq` / `$ne` contract and passes.
*
* Both guards apply wherever `==` / `!=` resolves an operand: opposite a field,
* and on either side of a constant comparison, where `current_user != 'guest'`
* folded to "no restriction" because an object is never strictly equal to a
* literal. `{ $field }` references never reach here (the field-to-field branch
* emits them with `isRef`), and a CEL map literal is refused by
* {@link classify}, so a resolved variable is the only way an object ever became
* a comparand. Ordering operators keep their existing lowering: they are outside
* the `==` / `!=` letter this refusal carries. The message names the variable
* PATH the author wrote and never a value.
*/
function comparandOf(op: string, leaf: Leaf, ctx: Ctx): unknown {
function variableObjectComparandRefusal(op: string, path: string[]): CompileError {
const written = path.join('.');
const comparand =
path.length === 1
? `its comparand \`${written}\` is the variable root itself, the whole context object`
: `its comparand \`${written}\` resolves to an object`;
return new CompileError(
'unsupported',
`\`${op}\` compares one value, but ${comparand} — compare one of its keys instead, ` +
`e.g. \`record.owner_id ${op} ${path[0]}.id\` rather than \`record.owner_id ${op} ${written}\``,
);
}

/** A resolved value that is an object rather than one comparable value. */
function isObjectComparand(value: unknown): boolean {
return value !== null && typeof value === 'object' && !Array.isArray(value) && !(value instanceof Date);
}

/**
* Resolve a comparison operand, refusing under `==` / `!=` a variable root
* before resolution and a variable resolving to an object after it
* ({@link variableObjectComparandRefusal}). In shape mode a variable resolves to
* the placeholder, so only the root is refused there.
*/
function variableOperandOf(op: string, leaf: Leaf, ctx: Ctx): unknown {
const equality = op === '==' || op === '!=';
if (equality && leaf.kind === 'var' && leaf.path.length === 1) throw variableObjectComparandRefusal(op, leaf.path);
const value = resolveValue(leaf, ctx);
if (equality && leaf.kind === 'var' && isObjectComparand(value)) throw variableObjectComparandRefusal(op, leaf.path);
return value;
}

/**
* Resolve the non-field side of a comparison, refusing under `==` / `!=` a list
* ({@link arrayComparandRefusal}) and a variable root or object
* ({@link variableOperandOf}). In shape mode a variable resolves to the
* placeholder, so only a literal list and a bare root are refused there.
*/
function comparandOf(op: string, leaf: Leaf, ctx: Ctx): unknown {
const value = variableOperandOf(op, leaf, ctx);
if ((op === '==' || op === '!=') && Array.isArray(value)) throw arrayComparandRefusal(op, leaf);
return value;
}
Expand Down
30 changes: 30 additions & 0 deletions packages/lint/src/validate-rls-predicate-enforceability.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,36 @@ describe('validateRlsPredicateEnforceability — each kernel key is probed with
);
});

describe('validateRlsPredicateEnforceability — the bare `current_user` root is reported on both clauses (#19959)', () => {
/**
* The CEL lowering refuses `==` / `!=` against the variable ROOT — the whole
* caller context object, not one value — in both compile modes, so the shape
* verdict this rule reads (`isSupportedRlsExpression`) reports it before any
* request. Until then the reference pass bound the root to its probe object,
* the predicate lowered, and a `check` so written admitted every write at
* runtime while this rule said nothing.
*/
it.each([
'owner_id != current_user',
'owner_id == current_user',
'!(owner_id == current_user)',
'current_user != owner_id',
"current_user != 'guest'",
])('%s', (source) => {
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}`],
]);
}
});

it('CONTROL — a key of the root stays clean on both clauses', () => {
expect(ids(siteWith('using', 'owner_id != current_user.id'))).toEqual([]);
expect(ids(siteWith('check', 'owner_id == current_user.id'))).toEqual([]);
});
});

describe('validateRlsPredicateEnforceability — §7.3.1 membership keys stay UNKNOWABLE', () => {
/**
* The false-positive this rule exists on the edge of. An app stages arbitrary
Expand Down
16 changes: 16 additions & 0 deletions packages/lint/src/validate-sharing-rule-enforceability.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,22 @@ describe('validateSharingRuleEnforceability — conditions the runtime DOES read
});
});

describe('validateSharingRuleEnforceability — the bare `current_user` root is unlowerable (#19959)', () => {
/**
* The CEL lowering refuses `==` / `!=` against the variable ROOT before it
* resolves anything, so the seeder's own call (`variables: {}`) answers
* `unsupported` rather than `unresolved-variable`: the shape is refused for
* every binding, and this rule reports the id whose fix is a respelling.
*/
it.each([
'record.owner_id != current_user',
'record.owner_id == current_user',
'!(record.owner_id == current_user)',
])('%s', (source) => {
expect(ids(ruleWith(source))).toEqual([SHARING_RULE_UNLOWERABLE_CONDITION]);
});
});

// ── The predicate is the consumer's, not a model of it ───────────────

describe('validateSharingRuleEnforceability — the verdict IS the seeder\'s verdict', () => {
Expand Down
Loading
Loading