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
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
"@objectstack/formula": minor
"@objectstack/driver-mongodb": minor
"@objectstack/plugin-security": minor
"@objectstack/plugin-sharing": minor
"@objectstack/lint": minor
"@objectstack/spec": patch
---

A row-level or sharing-rule predicate comparing a field against a list with `!=` / `==` is refused at the CEL lowering instead of lowering to a filter that widens on driver-mongodb, and driver-mongodb refuses `$ne` with an array comparand (#19886).

**BREAKING** — an accept-set narrowing, shipped by `@objectstack/formula`, `@objectstack/driver-mongodb`, `@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-list-comparand-refused`.

Clause-②: no (narrowing)

**Security fix for RLS reads on MongoDB and RLS write checks.** A policy written `record.status != ['closed', 'archived']` (or `!(record.status == [...])`, or `!=` against a `current_user` membership set) lowered to `{ status: { $ne: [...] } }` (or `$not` around a bare-array equality). The RLS `using` clause is composed into the query after the engine's comparand-shape check, and driver-mongodb passed the shape to the server, where it selects every scalar row: the read returned the rows the policy was written to hide. A `check` written `!=` against a membership set admitted every write.

- `@objectstack/formula`: `compileCelToFilter` refuses `==` / `!=` whose comparand is a list (`unsupported`): a list literal, or a `current_user` variable that resolves to an array. The authoring shape check (`isPushdownableCel`, `isSupportedRlsExpression`) reports the literal; a resolved array is refused per request.
- `@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). A CEL-authored `check` gets this 403; the `INVALID_FILTER` / 400 of `matchesFilterCondition` remains for a filter passed to it directly.
- `@objectstack/plugin-sharing`: a declared sharing rule with such a `condition` is skipped at bootstrap and never seeded.
- `@objectstack/lint`: the list-literal form is reported (`rls-predicate-unenforceable`, `sharing-rule-unlowerable-condition`). The RLS reference pass probes each kernel-resolved `current_user` key with its runtime type.
- `@objectstack/driver-mongodb`: `translateFilter` refuses `$ne` with an array comparand at any depth, with `INVALID_FILTER` / 400, as driver-sql and driver-memory already do.
- `@objectstack/spec`: the migration registry carries the entry.

**What to change.** "One of these values" is `record.status in ['open', 'pending']`; "none of these values" is `!(record.status in ['closed', 'archived'])`. In a raw filter, use `$in` / `$nin`. `in`, scalar `==` / `!=`, `null` and field-to-field comparisons are unchanged.

<!-- adr-0087: registered cel-predicate-list-comparand-refused -->
2 changes: 1 addition & 1 deletion .changeset/19886-formula-array-comparand-refused.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
---
"@objectstack/formula": minor
"@objectstack/plugin-security": minor
Expand All @@ -15,7 +15,7 @@
- a `check` written `record.status != ['closed', 'archived']`, or `!=` against a `current_user` membership array, lowered to `{ status: { $ne: [...] } }` and matched **every** post-image;
- a `check` written `!(record.status == ['closed', 'archived'])` lowered to `{ $not: { status: [...] } }` and did the same.

Every write such a policy was written to refuse was admitted and stored. Both shapes now fail the write with `INVALID_FILTER` / 400, the envelope driver-sql and driver-memory already give the same shape on the read side. The explain engine's record attribution reads the same evaluator, so it refuses too, instead of reporting a row as admitted that the enforcing read refuses. The positive `record.status == ['open', 'pending']` already refused every write (403); it now refuses with the 400 instead.
Every write such a policy was written to refuse was admitted and stored. The positive `record.status == ['open', 'pending']` already refused every write (403).

The message withholds the field, the operator and the value, because the filter is usually an access policy the caller did not write, and the comparand may be a resolved membership set.

Expand Down
43 changes: 43 additions & 0 deletions packages/drivers/driver-mongodb/src/mongodb-filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,29 @@ function classifyFilterKey(key: string, value: unknown, here: string): FilterVer
throw malformedBetweenError(key, value.$between, `${here}.$between`);
}

// [#19886] `$ne`'s comparand is ONE value, gated on the WALK for the reason
// the three gates above give: `{ $or: [ {}, { s: { $ne: [...] } } ] }`
// reduces to TRUE on its first disjunct, so an emitter-side gate would refuse
// or ignore it depending on its siblings. Before this gate the shape was
// emitted unchanged, and MongoDB reads `$ne` against an array operand as
// "not equal to that array and not holding it as an element" — every scalar
// row. On an RLS read that is a widening: the `using` clause is AND-composed
// AFTER the engine's comparand-shape seam, so this driver was the only face
// left between it and the server. `driver-sql` (unbindable comparand) and
// `driver-memory` (array comparand) already refuse it with this envelope.
//
// ⚠️ Deliberately `$ne` ONLY. The equality position (`{ f: [...] }`,
// `$eq: [...]`) is ruled to the shared face, and that ruling's own pin keeps
// this translator passing it through unchanged — a driver-local copy there
// would be a second implementation of the shared rule.
if (
isFilterNode(value) &&
Object.prototype.hasOwnProperty.call(value, '$ne') &&
Array.isArray(value.$ne)
) {
throw arrayNotEqualComparandError();
}

// A field key always contributes a predicate — `'clause'`, exactly as #5239
// classified it. The refusals above do not change that verdict for any shape
// that survives them.
Expand Down Expand Up @@ -370,6 +393,26 @@ function unsupportedFilterError(message: string): Error {
return err;
}

/**
* [#19886] `$ne` whose comparand is an array.
*
* The field, the value and the position are withheld from the message:
* this translator receives the RLS `using` predicate AND-composed into the
* caller's `where`, and the caller who reads the 400 is not the author of that
* predicate, whose comparand may be a resolved membership set (other users'
* ids). That is the posture `driver-sql`'s withheld refusals took for the
* same reason. The message carries the refusal's identity and the remedy.
*/
function arrayNotEqualComparandError(): Error {
return unsupportedFilterError(
'A "$ne" comparison in this filter received an array as its comparand. "$ne" compares one ' +
'value; for "none of these values" use "$nin". It is refused rather than translated because ' +
'MongoDB reads "$ne" against an array operand as "not equal to that array and not holding it ' +
'as an element", which every scalar value satisfies. The field and the value are withheld ' +
'from this message because the filter may be an access policy the caller did not write.',
);
}

/**
* [#5347] `$null` whose comparand is not a boolean.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#19886] `$ne` with an ARRAY comparand is refused by `translateFilter`, with
* the `INVALID_FILTER` / 400 envelope `driver-sql` and `driver-memory` give the
* same shape.
*
* ## Why this driver needed its own gate
*
* The RLS `using` clause is AND-composed into the query AFTER the engine's
* comparand-shape seam, so a policy lowered to `{ f: { $ne: [...] } }` reached
* this translator with no shared face in front of it, and the translator
* emitted it unchanged. MongoDB reads `$ne` against an array operand as "not
* equal to that array and not holding it as an element", which every scalar
* value satisfies: measured through the real `MongoDBDriver` on mingo 7.2.4,
* the named proxy for MongoDB query semantics, an RLS read under
* `record.status != current_user.<membership set>` returned EVERY row. A live
* `mongod` is NOT MEASURED — this fleet cannot fetch the binary, and mingo is
* not a dependency of this package, so the proxy reading was taken outside this
* suite. This suite pins the translator and the driver door: the refusal, and
* that the server is never asked.
*
* ## Scope
*
* `$ne` only. The equality position (`{ f: [...] }`, `$eq: [...]`) is ruled to
* the shared comparand-shape face, which keeps this translator passing it
* through; it is not pinned either way here.
*/

import { describe, it, expect } from 'vitest';
import { translateFilter } from './mongodb-filter.js';
import { MongoDBDriver } from './mongodb-driver.js';

interface WireBearingError extends Error {
code?: string;
status?: number;
}

const refusalOf = (where: unknown): WireBearingError => {
try {
translateFilter(where);
} catch (e) {
return e as WireBearingError;
}
throw new Error('expected the translator to refuse this filter, but it translated');
};

const LIST = ['closed', 'archived'];

describe('[#19886] translateFilter refuses $ne with an array comparand', () => {
const POSITIONS: Array<[string, Record<string, unknown>]> = [
['top level', { status: { $ne: LIST } }],
['beside another operator on the field', { status: { $ne: LIST, $exists: true } }],
['inside $and', { $and: [{ owner: 'u1' }, { status: { $ne: LIST } }] }],
// `{}` reduces the `$or` to TRUE on its first disjunct, so the emitter never
// reaches the second: only a gate on the WALK refuses this one.
['inside $or, beside a TRUE identity', { $or: [{}, { status: { $ne: LIST } }] }],
['inside $not (lowered to $nor)', { $not: { status: { $ne: LIST } } }],
['nested two combinators deep', { $and: [{ $or: [{ $not: { status: { $ne: LIST } } }] }] }],
['an EMPTY array', { status: { $ne: [] } }],
];

for (const [label, where] of POSITIONS) {
it(`${label}: INVALID_FILTER / 400`, () => {
const err = refusalOf(where);
expect(err.code).toBe('INVALID_FILTER');
expect(err.status).toBe(400);
});
}

it('the message names "$nin" and withholds the field and the comparand', () => {
const err = refusalOf({ secret_scope: { $ne: ['usr_member_one', 'usr_member_two'] } });
expect(err.message).toContain('"$nin"');
expect(err.message).not.toContain('secret_scope');
expect(err.message).not.toContain('usr_member_one');
});

it('the driver door refuses before the server is asked', async () => {
const calls: unknown[] = [];
const collection = {
find: (filter: unknown) => { calls.push(filter); return { toArray: async () => [] }; },
findOne: async (filter: unknown) => { calls.push(filter); return null; },
countDocuments: async (filter: unknown) => { calls.push(filter); return 0; },
};
const driver = new MongoDBDriver({ url: 'mongodb://127.0.0.1:1/unused', database: 'unused' });
(driver as unknown as { db: unknown }).db = { collection: () => collection };

for (const run of [
() => driver.find('deal', { where: { status: { $ne: LIST } } } as never),
() => driver.findOne('deal', { where: { status: { $ne: LIST } } } as never),
() => driver.count('deal', { where: { status: { $ne: LIST } } } as never),
]) {
const err = await run().then(() => null, (e: WireBearingError) => e);
expect(err?.code).toBe('INVALID_FILTER');
expect(err?.status).toBe(400);
}
expect(calls).toHaveLength(0);
});
});

describe('[#19886] every neighbouring shape translates exactly as before', () => {
it('$nin / $in with a list', () => {
expect(translateFilter({ status: { $nin: LIST } })).toEqual({ status: { $nin: LIST } });
expect(translateFilter({ status: { $in: LIST } })).toEqual({ status: { $in: LIST } });
});

it('scalar $ne, and $ne: null', () => {
expect(translateFilter({ status: { $ne: 'closed' } })).toEqual({ status: { $ne: 'closed' } });
expect(translateFilter({ status: { $ne: null } })).toEqual({ status: { $ne: null } });
});

it('a scalar equality on an array-valued field — MongoDB\'s element match — is untouched', () => {
expect(translateFilter({ tags: 'a' })).toEqual({ tags: 'a' });
expect(translateFilter({ tags: { $ne: 'a' } })).toEqual({ tags: { $ne: 'a' } });
});

it('$not around a scalar $ne still lowers to $nor', () => {
expect(translateFilter({ $not: { status: { $ne: 'closed' } } })).toEqual({ $nor: [{ status: { $ne: 'closed' } }] });
});
});
131 changes: 131 additions & 0 deletions packages/formula/src/cel-to-filter-list-comparand.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#19886] `==` / `!=` against a LIST is a compile error, not a lowering — a
* list literal, and a `current_user` variable that resolves to an array.
*
* It used to lower to `{ f: { $ne: [...] } }` and to the bare-array
* `{ f: [...] }` (with `$not` around it for `!(… == [...])`). Those shapes
* reached the backends through the RLS `using` clause, which is composed AFTER
* the engine's comparand-shape seam, and two backends widened on them
* (`$ne: [...]` matched every row on the write-check evaluator and every scalar
* row on driver-mongodb). Refused here, every consumer of this compiler fails
* closed on its own existing `unsupported` path: the RLS compiler drops the
* policy, the sharing seeder skips the rule, and the authoring gate
* (`isPushdownableCel` / `isSupportedRlsExpression`) reports a literal.
*
* A variable's value exists only per request, so the shape check cannot see a
* resolved array: that refusal is pinned at request time, with the shape check
* still passing the source.
*/

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

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

const VARS = {
current_user: {
id: 'u_me',
email: 'me@example.test',
org_user_ids: ['u_me', 'u_peer'],
// A §7.3.1 membership set staged by an app resolver, and an emptied one.
blocked_accounts: ['acc_secret'],
empty_set: [] as string[],
},
};

const REFUSED = [
"record.status != ['closed', 'archived']",
"record.status == ['open', 'pending']",
"!(record.status == ['closed', 'archived'])",
"!(record.status != ['closed', 'archived'])",
"['closed', 'archived'] != record.status",
"['open'] == record.status",
'record.status != []',
"record.owner_id == current_user.id && record.status != ['closed']",
"record.owner_id == current_user.id || !(record.status == ['closed'])",
];

describe('[#19886] == / != against a list literal is refused at the lowering', () => {
for (const source of REFUSED) {
it(`${source} — unsupported, at request time and at authoring time`, () => {
const compiled = compileCelToFilter(source, { variables: VARS });
expect(compiled.ok).toBe(false);
expect(compiled.ok ? undefined : compiled.reason).toBe('unsupported');
// The authoring gate asks the same compiler in shape mode, so the refusal
// is visible before any request: the lint and the RLS compiler's
// "uncompilable predicate" branch both read this.
expect(isPushdownableCel(source).ok).toBe(false);
expect(isSupportedRlsExpression(source)).toBe(false);
});
}
});

const REFUSED_RESOLVED = [
'record.reviewer_id != current_user.org_user_ids',
'record.reviewer_id == current_user.org_user_ids',
'!(record.reviewer_id == current_user.org_user_ids)',
'!(record.reviewer_id != current_user.org_user_ids)',
'current_user.org_user_ids != record.reviewer_id',
'current_user.org_user_ids == record.reviewer_id',
'record.account != current_user.blocked_accounts',
'record.account != current_user.empty_set',
"record.status == 'open' && record.reviewer_id != current_user.org_user_ids",
"record.status == 'open' || !(record.reviewer_id == current_user.org_user_ids)",
];

describe('[#19886] == / != against a variable that RESOLVES to a list is refused at request time', () => {
for (const source of REFUSED_RESOLVED) {
it(`${source} — unsupported, naming the variable and withholding its value`, () => {
const compiled = compileCelToFilter(source, { variables: VARS });
expect(compiled.ok).toBe(false);
expect(compiled.ok ? undefined : compiled.reason).toBe('unsupported');
const detail = compiled.ok ? '' : compiled.detail;
expect(detail).toMatch(/current_user\.(org_user_ids|blocked_accounts|empty_set)/);
for (const member of ['u_me', 'u_peer', 'acc_secret']) expect(detail).not.toContain(member);
// The value is per request, so the shape check passes the source: the
// refusal lands on the RLS compiler's per-request denial path instead.
expect(isPushdownableCel(source).ok).toBe(true);
expect(isSupportedRlsExpression(source)).toBe(true);
});
}
});

describe('[#19886] 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('`in` / `not in` against a list literal — the spelling the refusal points at', () => {
expect(ok("record.status in ['open', 'pending']")).toEqual({ status: { $in: ['open', 'pending'] } });
expect(ok("!(record.status in ['closed', 'archived'])")).toEqual({ $not: { status: { $in: ['closed', 'archived'] } } });
});

it('`in` / `not in` against a resolved membership array', () => {
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('scalar == / != against a literal and against a resolved scalar', () => {
expect(ok("record.status != 'closed'")).toEqual({ status: { $ne: 'closed' } });
expect(ok("record.status == 'open'")).toEqual({ status: 'open' });
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('current_user.email == record.owner')).toEqual({ owner: 'me@example.test' });
});

it('== null / != null', () => {
expect(ok('record.closed_at == null')).toEqual({ closed_at: { $null: true } });
expect(ok('record.closed_at != null')).toEqual({ closed_at: { $null: false } });
});

it('field-to-field { $field } references', () => {
expect(ok('record.owner == record.manager')).toEqual({ owner: { $eq: { $field: 'manager' } } });
expect(ok('record.owner != record.manager')).toEqual({ owner: { $ne: { $field: 'manager' } } });
});
});
Loading
Loading