From 66005f5b4ba64d572a0e9fa019edde874dda8a99 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 05:55:58 +0000 Subject: [PATCH 1/5] 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 --- packages/formula/src/cel-to-filter.ts | 100 +++++++++++++++++++++++--- 1 file changed, 91 insertions(+), 9 deletions(-) diff --git a/packages/formula/src/cel-to-filter.ts b/packages/formula/src/cel-to-filter.ts index 5f37b5eb859..389f4f83178 100644 --- a/packages/formula/src/cel-to-filter.ts +++ b/packages/formula/src/cel-to-filter.ts @@ -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'; @@ -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 {}; @@ -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: } }` (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.` 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.` 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; } From cf59288fd92c8b338673aa3669a27109eedde51a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 06:00:11 +0000 Subject: [PATCH 2/5] test(formula): pin the variable-root and object comparand refusals Claude-Session: https://claude.ai/code/session_01CiCTczDo7tGhafXjf61dUJ Co-authored-by: Claude --- ...-to-filter-variable-root-comparand.test.ts | 156 ++++++++++++++++++ 1 file changed, 156 insertions(+) create mode 100644 packages/formula/src/cel-to-filter-variable-root-comparand.test.ts diff --git a/packages/formula/src/cel-to-filter-variable-root-comparand.test.ts b/packages/formula/src/cel-to-filter-variable-root-comparand.test.ts new file mode 100644 index 00000000000..de21a290224 --- /dev/null +++ b/packages/formula/src/cel-to-filter-variable-root-comparand.test.ts @@ -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: } }`, `==` 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.` 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) => { + 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 } }); + }); +}); From 9e8808564d753b9c5df048c01dbb72c8dae692e7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 06:07:22 +0000 Subject: [PATCH 3/5] test(plugin-security): pin the variable-root RLS predicate failing closed on every leg Sole using, sole check (insert and by-id update), USING-only on the write pass, OR-sibling, explain and the delegator leg, each with a control spelled the way the refusal points. Claude-Session: https://claude.ai/code/session_01CiCTczDo7tGhafXjf61dUJ Co-authored-by: Claude --- ...riable-root-comparand-fails-closed.test.ts | 300 ++++++++++++++++++ 1 file changed, 300 insertions(+) create mode 100644 packages/plugins/plugin-security/src/rls-variable-root-comparand-fails-closed.test.ts diff --git a/packages/plugins/plugin-security/src/rls-variable-root-comparand-fails-closed.test.ts b/packages/plugins/plugin-security/src/rls-variable-root-comparand-fails-closed.test.ts new file mode 100644 index 00000000000..b55676268ce --- /dev/null +++ b/packages/plugins/plugin-security/src/rls-variable-root-comparand-fails-closed.test.ts @@ -0,0 +1,300 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#19959] A row-level policy that compares a field with `==` / `!=` against the + * bare `current_user` ROOT fails closed on every leg, through the real plugin + * and engine on driver-sql. + * + * It used to lower to `{ reviewer_id: { $ne: } }` (or + * `==` / `$not` around the bare object). A strict compare never equals an + * object, so a `check` written `!= current_user` or `!(== current_user)` ADMITTED + * and stored every forbidden insert and by-id update, a USING-only such policy + * admitted every insert on the write pass, and explain reported `narrows` / + * `visible: true` with the caller's membership sets echoed in its `readFilter`. + * + * `compileCelToFilter` now refuses the root (`unsupported`) in both modes, so + * `RLSCompiler.compileFilter` drops the policy on its "uncompilable predicate" + * branch and, with nothing else applicable, answers `RLS_DENY_FILTER`: reads + * return ZERO rows, writes are refused with the row-level CHECK envelope + * (`PERMISSION_DENIED` / 403) and nothing is stored. An OR-sibling alone decides. + * + * Each leg has a control spelled the way the refusal points (`current_user.id`), + * which proves the leg discriminates. The ground truth is read past every scope. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { SysMember, SysUser } from '@objectstack/platform-objects/identity'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import { SecurityPlugin } from './security-plugin.js'; +import { RLS_DENY_FILTER } from './rls-compiler.js'; +import { defaultPermissionSets } from './objects/default-permission-sets.js'; + +const OBJ = 'qa_ticket'; +const GUARD_SET = 'qa_ticket_guard'; +const AGENT_SET = 'qa_ticket_agent'; +const SYS_CTX = { isSystem: true, userId: 'usr_system' }; +const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; +const DELEGATOR = 'usr_delegator'; + +type Policy = { name: string; using?: string; check?: string }; + +const engines: ObjectQL[] = []; +afterEach(async () => { + while (engines.length) { + try { await engines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +/** + * One harness for every leg. The guard set is the deployment's fallback, so a + * human principal — the caller, and the agent's delegator — resolves it; the + * agent's own set carries CRUD and no row-level policy. + */ +async function boot(policies: Policy[]) { + const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + const engine = new ObjectQL(); + engine.registerDriver(driver as never, true); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.qa.rls-variable-root-19959', + name: 'RLS variable root', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [ + SysUser, + SysMember, + { + name: OBJ, + label: 'Ticket', + sharingModel: 'public_read_write', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + status: { name: 'status', type: 'text' }, + reviewer_id: { name: 'reviewer_id', type: 'text' }, + }, + }, + ], + } as never); + await engine.syncSchemas(); + engines.push(engine); + + const crud = { allowRead: true, allowCreate: true, allowEdit: true }; + const guard = PermissionSetSchema.parse({ + name: GUARD_SET, + objects: { [OBJ]: crud }, + rowLevelSecurity: policies.map((p) => ({ object: OBJ, operation: 'all', ...p })), + }); + const agent = PermissionSetSchema.parse({ name: AGENT_SET, objects: { [OBJ]: crud } }); + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata: { + get: async (_type: string, name: string) => engine.getSchema(name) ?? null, + list: async () => [MEMBER_DEFAULT, guard, agent], + }, + }; + const ctx = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService: vi.fn(), + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const plugin = new SecurityPlugin({ fallbackPermissionSet: GUARD_SET }); + await plugin.init(ctx as never); + await plugin.start(ctx as never); + vi.spyOn((engine as unknown as { logger: { warn: () => void } }).logger, 'warn').mockImplementation(() => undefined); + + // Every principal is a real `sys_user`, written past every scope: the + // delegator must exist for the agent leg to resolve, and the engine stamps + // and validates the writer as the new row's owner. + const table = (driver as unknown as { knex: (t: string) => { insert(r: unknown): Promise } }).knex; + await table('sys_user').insert( + ['usr_member', 'agent_1', DELEGATOR].map((id) => ({ id, name: id, email: `${id}@example.test` })), + ); + + const caller = { + userId: 'usr_member', + positions: ['qa_pos'], + permissions: [GUARD_SET], + posture: 'MEMBER', + email: 'usr_member@example.test', + org_user_ids: ['usr_member', 'usr_peer'], + }; + const agentCaller = { + userId: 'agent_1', + principalKind: 'agent', + positions: [], + permissions: [AGENT_SET], + onBehalfOf: { userId: DELEGATOR }, + org_user_ids: ['usr_member', 'usr_peer'], + }; + const stored = async () => + ((await engine.find(OBJ, { context: SYS_CTX } as never)) as Array<{ id: string; status: string }>) + .map((r) => `${r.id}:${r.status}`) + .sort(); + const seed = () => engine.insert(OBJ, ROWS, { context: SYS_CTX } as never); + return { engine, plugin, caller, agentCaller, stored, seed }; +} + +const ROWS = [ + { id: 'r_mine', status: 'open', reviewer_id: 'usr_member' }, + { id: 'r_other', status: 'open', reviewer_id: 'usr_other' }, +]; + +/** The spellings that compared against the whole caller context. */ +const ROOT = [ + ['`!=` against the root', 'record.reviewer_id != current_user'], + ['`==` against the root', 'record.reviewer_id == current_user'], + ['a negated `==` against the root', '!(record.reviewer_id == current_user)'], +] as const; + +type Refusal = { code?: string; status?: number; statusCode?: number } | null; +const attempt = (op: () => Promise): Promise => + op().then(() => null, (e: { code?: string; status?: number; statusCode?: number }) => e); + +function expectCheckDenial(err: Refusal) { + expect(err, 'the write was admitted').not.toBeNull(); + expect(err?.code).toBe('PERMISSION_DENIED'); + expect(err?.statusCode ?? err?.status).toBe(403); +} + +describe('[#19959] a sole `using` comparing against the root reads ZERO rows', () => { + for (const [spelling, using] of ROOT) { + it(spelling, async () => { + const { engine, caller, stored, seed } = await boot([{ name: 'root', using }]); + await seed(); + + const rows = (await engine.find(OBJ, { context: caller } as never)) as Array<{ id: string }>; + + expect(rows).toEqual([]); + expect(await stored()).toEqual(['r_mine:open', 'r_other:open']); + }); + } + + it('CONTROL — `record.reviewer_id != current_user.id` reads exactly the rows it admits', async () => { + const { engine, caller, seed } = await boot([{ name: 'key', using: 'record.reviewer_id != current_user.id' }]); + await seed(); + + const rows = (await engine.find(OBJ, { context: caller } as never)) as Array<{ id: string }>; + + expect(rows.map((r) => r.id)).toEqual(['r_other']); + }); +}); + +describe('[#19959] a sole `check` comparing against the root refuses every write', () => { + for (const [spelling, check] of ROOT) { + it(`${spelling}: insert and by-id update are PERMISSION_DENIED / 403, nothing lands`, async () => { + const { engine, caller, stored, seed } = await boot([{ name: 'root', check }]); + await seed(); + + expectCheckDenial(await attempt(() => + engine.insert(OBJ, { id: 'ins_bad', status: 'open', reviewer_id: 'usr_other' }, { context: caller } as never))); + expectCheckDenial(await attempt(() => + engine.update(OBJ, { id: 'r_other', status: 'closed' }, { context: caller } as never))); + + expect(await stored()).toEqual(['r_mine:open', 'r_other:open']); + }); + } + + it('CONTROL — `record.reviewer_id != current_user.id` admits the other owner and refuses the caller', async () => { + const { engine, caller, stored } = await boot([{ name: 'key', check: 'record.reviewer_id != current_user.id' }]); + + expect(await attempt(() => + engine.insert(OBJ, { id: 'ins_ok', status: 'open', reviewer_id: 'usr_other' }, { context: caller } as never))).toBeNull(); + expectCheckDenial(await attempt(() => + engine.insert(OBJ, { id: 'ins_bad', status: 'open', reviewer_id: 'usr_member' }, { context: caller } as never))); + + expect(await stored()).toEqual(['ins_ok:open']); + }); +}); + +describe('[#19959] a USING-only root policy is the insert check on the write pass, and refuses', () => { + it('`!=` against the root in `using`, no `check`: PERMISSION_DENIED / 403, nothing stored', async () => { + const { engine, caller, stored } = await boot([{ name: 'root', using: 'record.reviewer_id != current_user' }]); + + expectCheckDenial(await attempt(() => + engine.insert(OBJ, { id: 'ins_bad', status: 'open', reviewer_id: 'usr_other' }, { context: caller } as never))); + + expect(await stored()).toEqual([]); + }); +}); + +describe('[#19959] beside a compiling OR-sibling, the sibling alone decides', () => { + it('read: the sibling’s rows only', async () => { + const { engine, caller, seed } = await boot([ + { name: 'root', using: 'record.reviewer_id != current_user' }, + { name: 'mine', using: 'record.reviewer_id == current_user.id' }, + ]); + await seed(); + + const rows = (await engine.find(OBJ, { context: caller } as never)) as Array<{ id: string }>; + + expect(rows.map((r) => r.id)).toEqual(['r_mine']); + }); + + it('write: the sibling’s check admits `open` and refuses `archived`', async () => { + const { engine, caller, stored } = await boot([ + { name: 'root', check: 'record.reviewer_id != current_user' }, + { name: 'not_archived', check: "record.status != 'archived'" }, + ]); + + expect(await attempt(() => + engine.insert(OBJ, { id: 'ins_open', status: 'open', reviewer_id: 'usr_other' }, { context: caller } as never))).toBeNull(); + expectCheckDenial(await attempt(() => + engine.insert(OBJ, { id: 'ins_archived', status: 'archived', reviewer_id: 'usr_other' }, { context: caller } as never))); + + expect(await stored()).toEqual(['ins_open:open']); + }); +}); + +describe('[#19959] explain agrees with the enforcement', () => { + it('a root `using` explains as `denies` / not visible, and echoes no membership set', async () => { + const { plugin, caller, seed } = await boot([{ name: 'root', using: 'record.reviewer_id != current_user' }]); + await seed(); + + const decision = await plugin.explainAccessForCaller({ object: OBJ, operation: 'read', recordId: 'r_other' }, caller); + + expect(decision.layers.find((l) => l.layer === 'rls')?.verdict).toBe('denies'); + expect(decision.allowed).toBe(false); + expect(decision.record?.visible).toBe(false); + expect(decision.readFilter).toEqual(RLS_DENY_FILTER); + expect(JSON.stringify(decision)).not.toContain('usr_peer'); + }); + + it('CONTROL — the `current_user.id` spelling explains as `narrows` and the row as visible', async () => { + const { plugin, caller, seed } = await boot([{ name: 'key', using: 'record.reviewer_id != current_user.id' }]); + await seed(); + + const decision = await plugin.explainAccessForCaller({ object: OBJ, operation: 'read', recordId: 'r_other' }, caller); + + expect(decision.layers.find((l) => l.layer === 'rls')?.verdict).toBe('narrows'); + expect(decision.record?.visible).toBe(true); + }); +}); + +describe('[#19959] the delegator leg: an agent acting for a user whose set carries the root `check`', () => { + it('insert is PERMISSION_DENIED / 403 and nothing is stored', async () => { + const { engine, agentCaller, stored } = await boot([{ name: 'root', check: 'record.reviewer_id != current_user' }]); + + expectCheckDenial(await attempt(() => + engine.insert(OBJ, { id: 'ins_bad', status: 'open', reviewer_id: 'usr_other' }, { context: agentCaller } as never))); + + expect(await stored()).toEqual([]); + }); + + it('CONTROL — a scalar `check` on the delegator’s set admits `open` and refuses `closed`', async () => { + const { engine, agentCaller, stored } = await boot([{ name: 'not_closed', check: "record.status != 'closed'" }]); + + expect(await attempt(() => + engine.insert(OBJ, { id: 'ins_open', status: 'open', reviewer_id: 'usr_other' }, { context: agentCaller } as never))).toBeNull(); + expectCheckDenial(await attempt(() => + engine.insert(OBJ, { id: 'ins_closed', status: 'closed', reviewer_id: 'usr_other' }, { context: agentCaller } as never))); + + expect(await stored()).toEqual(['ins_open:open']); + }); +}); From 21a4f45afd57dd46f64758dd72a7994329964c79 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 06:14:02 +0000 Subject: [PATCH 4/5] test(lint): pin both lint rules reporting the bare variable-root comparand Claude-Session: https://claude.ai/code/session_01CiCTczDo7tGhafXjf61dUJ Co-authored-by: Claude --- ...idate-rls-predicate-enforceability.test.ts | 30 +++++++++++++++++++ ...lidate-sharing-rule-enforceability.test.ts | 16 ++++++++++ 2 files changed, 46 insertions(+) diff --git a/packages/lint/src/validate-rls-predicate-enforceability.test.ts b/packages/lint/src/validate-rls-predicate-enforceability.test.ts index 1f5cc14bb8d..1a5cb643d10 100644 --- a/packages/lint/src/validate-rls-predicate-enforceability.test.ts +++ b/packages/lint/src/validate-rls-predicate-enforceability.test.ts @@ -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 diff --git a/packages/lint/src/validate-sharing-rule-enforceability.test.ts b/packages/lint/src/validate-sharing-rule-enforceability.test.ts index edbdec47b1b..dcfd2305058 100644 --- a/packages/lint/src/validate-sharing-rule-enforceability.test.ts +++ b/packages/lint/src/validate-sharing-rule-enforceability.test.ts @@ -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', () => { From 60da27e665f8b15aa74609b5a83e72f5c35197d2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 06:16:15 +0000 Subject: [PATCH 5/5] chore(spec, changeset): register the variable-root comparand refusal under ADR-0087 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One semantic entry under protocol major 18, the regenerated registry, and a BREAKING changeset (Clause-② no, narrowing) at minor for formula, plugin-security, plugin-sharing and lint, patch for spec. Claude-Session: https://claude.ai/code/session_01CiCTczDo7tGhafXjf61dUJ Co-authored-by: Claude --- ...959-cel-variable-root-comparand-refused.md | 25 ++++++++++ ...edicate-variable-root-comparand-refused.ts | 50 +++++++++++++++++++ packages/spec/src/migrations/registry.ts | 46 +++++++++++++++++ 3 files changed, 121 insertions(+) create mode 100644 .changeset/19959-cel-variable-root-comparand-refused.md create mode 100644 packages/spec/src/migrations/entries/semantic/18.cel-predicate-variable-root-comparand-refused.ts diff --git a/.changeset/19959-cel-variable-root-comparand-refused.md b/.changeset/19959-cel-variable-root-comparand-refused.md new file mode 100644 index 00000000000..2f35b3f8f71 --- /dev/null +++ b/.changeset/19959-cel-variable-root-comparand-refused.md @@ -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: } }` (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. + + diff --git a/packages/spec/src/migrations/entries/semantic/18.cel-predicate-variable-root-comparand-refused.ts b/packages/spec/src/migrations/entries/semantic/18.cel-predicate-variable-root-comparand-refused.ts new file mode 100644 index 00000000000..36b399a4247 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.cel-predicate-variable-root-comparand-refused.ts @@ -0,0 +1,50 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// The variable-ROOT sibling of cel-predicate-list-comparand-refused, one +// comparand kind over: the same pushdown compiler, the same consumers, the same +// fail-closed path. Recorded as its own entry because the surface an author +// rewrites is a different comparand, with a different replacement. +export const entry: SemanticMigration = { + id: 'cel-predicate-variable-root-comparand-refused', + // No backticks in `surface` — build-upgrade-guide renders it inside a code + // span already, and a nested backtick would close it. + surface: + 'security.PermissionSet rowLevelSecurity[].using and .check, and sharingRules[].condition — a ' + + 'CEL predicate comparing with != or == against the bare current_user root, the variable with ' + + 'no key named after it, whether the other side is a field or a literal, and the negation of ' + + 'such a comparison. For a caller of the published compiler that binds its own variables, also ' + + 'a variable that resolves to an object', + replacement: + 'the key of current_user the comparison means: record.owner_id == current_user.id, or ' + + 'current_user.organization_id, or current_user.email. A membership test is in: ' + + 'record.owner_id in current_user.org_user_ids. Scalar keys, membership sets under in, ' + + 'literals, null and field-to-field comparisons lower exactly as before', + reason: + 'The @objectstack/formula pushdown compiler resolved the bare root to the whole caller context ' + + 'object, every kernel-resolved key at once with the membership arrays included, and lowered ' + + 'the comparison to a $ne carrying that object, to a bare-object equality, or to a $not around ' + + 'one; a constant comparison such as current_user != "guest" folded to no restriction. A ' + + 'strict compare never equals an object, so through the real SecurityPlugin on driver-sql a ' + + 'check written != against the root, or its negated ==, admitted and stored every insert and ' + + 'by-id update it was written to refuse, a USING-only such policy admitted every insert on ' + + 'the write pass, and explain reported the read as narrowed with the caller membership sets ' + + 'echoed 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 set, and the published $eq / $ne ' + + 'contract declares a literal or a { $field } reference; the root is none of them. The ' + + 'compiler now refuses it with reason unsupported in both of its modes, so the authoring lint ' + + 'reports it (rls-predicate-unenforceable on either clause, sharing-rule-unlowerable-condition ' + + 'on a sharing condition), and the RLS compiler drops the policy and fails closed when no ' + + 'other policy applies: reads under it return no rows, check writes are refused 403, and ' + + 'explain answers denies. A declared sharing rule with such a condition is skipped at ' + + 'bootstrap as it already was, now with reason unsupported instead of unresolved-variable. ' + + 'Metadata AT REST is not rewritten and this entry adds no D2 conversion: the platform cannot ' + + 'tell which key the author meant, and a policy rewritten on the author\'s behalf would change ' + + 'which rows it admits. ADR-0058 D2 / ADR-0087.', + acceptanceCriteria: + 'Grep the rowLevelSecurity using and check predicates of your permission sets, and the ' + + 'condition of your sharing rules, for != or == whose other side is current_user with no key ' + + 'after it, then rewrite each against the key it means (current_user.id, ' + + 'current_user.organization_id or current_user.email), or with in against a membership set.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index f12837a5b95..7e10338c7f2 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -6295,6 +6295,52 @@ const step18: MigrationStep = { + 'or its negation. On driver-mongodb, ' + 'grep stored query filters for $ne with an array value and rewrite each with $nin.', }, + // The variable-ROOT sibling of cel-predicate-list-comparand-refused, one + // comparand kind over: the same pushdown compiler, the same consumers, the same + // fail-closed path. Recorded as its own entry because the surface an author + // rewrites is a different comparand, with a different replacement. + { + id: 'cel-predicate-variable-root-comparand-refused', + // No backticks in `surface` — build-upgrade-guide renders it inside a code + // span already, and a nested backtick would close it. + surface: + 'security.PermissionSet rowLevelSecurity[].using and .check, and sharingRules[].condition — a ' + + 'CEL predicate comparing with != or == against the bare current_user root, the variable with ' + + 'no key named after it, whether the other side is a field or a literal, and the negation of ' + + 'such a comparison. For a caller of the published compiler that binds its own variables, also ' + + 'a variable that resolves to an object', + replacement: + 'the key of current_user the comparison means: record.owner_id == current_user.id, or ' + + 'current_user.organization_id, or current_user.email. A membership test is in: ' + + 'record.owner_id in current_user.org_user_ids. Scalar keys, membership sets under in, ' + + 'literals, null and field-to-field comparisons lower exactly as before', + reason: + 'The @objectstack/formula pushdown compiler resolved the bare root to the whole caller context ' + + 'object, every kernel-resolved key at once with the membership arrays included, and lowered ' + + 'the comparison to a $ne carrying that object, to a bare-object equality, or to a $not around ' + + 'one; a constant comparison such as current_user != "guest" folded to no restriction. A ' + + 'strict compare never equals an object, so through the real SecurityPlugin on driver-sql a ' + + 'check written != against the root, or its negated ==, admitted and stored every insert and ' + + 'by-id update it was written to refuse, a USING-only such policy admitted every insert on ' + + 'the write pass, and explain reported the read as narrowed with the caller membership sets ' + + 'echoed 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 set, and the published $eq / $ne ' + + 'contract declares a literal or a { $field } reference; the root is none of them. The ' + + 'compiler now refuses it with reason unsupported in both of its modes, so the authoring lint ' + + 'reports it (rls-predicate-unenforceable on either clause, sharing-rule-unlowerable-condition ' + + 'on a sharing condition), and the RLS compiler drops the policy and fails closed when no ' + + 'other policy applies: reads under it return no rows, check writes are refused 403, and ' + + 'explain answers denies. A declared sharing rule with such a condition is skipped at ' + + 'bootstrap as it already was, now with reason unsupported instead of unresolved-variable. ' + + 'Metadata AT REST is not rewritten and this entry adds no D2 conversion: the platform cannot ' + + 'tell which key the author meant, and a policy rewritten on the author\'s behalf would change ' + + 'which rows it admits. ADR-0058 D2 / ADR-0087.', + acceptanceCriteria: + 'Grep the rowLevelSecurity using and check predicates of your permission sets, and the ' + + 'condition of your sharing rules, for != or == whose other side is current_user with no key ' + + 'after it, then rewrite each against the key it means (current_user.id, ' + + 'current_user.organization_id or current_user.email), or with in against a membership set.', + }, { id: 'change-management-duration-keys-retired', surface: