Skip to content

Commit 1fb616e

Browse files
committed
fix(plugin-security): judge every row of an array insert against the row-level check
A row-level `check` (declared, or defaulted from `using`) guarantees that no stored row fails it. The write gate installed that judgement for a single-row insert only: an array payload was excluded, so every row of an array insert was stored unjudged, including under policies that refuse every single-row insert. The gate now installs the same judgement for an array insert. The engine already hands it every live row after the `beforeInsert` chain, so each row is judged on the image that will be stored, and one failing row refuses the whole insert with the existing PERMISSION_DENIED / 403 refusal. A single-row insert is unchanged. Claude-Session: https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2c1011b commit 1fb616e

5 files changed

Lines changed: 249 additions & 10 deletions

File tree

‎packages/plugins/plugin-security/src/rls-check-defaults-to-using.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ const attempt = async (run: () => Promise<unknown>): Promise<Outcome> => {
164164
}
165165
};
166166

167-
/** ⚠️ A SINGLE object, never an array: step 3.6 skips a bulk payload. */
167+
/** A single-row insert; the array shape is pinned in `rls-check-multi-row-writes.test.ts`. */
168168
const insert = (engine: ObjectQL, row: Record<string, unknown>) =>
169169
engine.insert('qa_ticket', { id: 't1', title: 't', ...row } as never, { context: CALLER } as never);
170170

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [ADR-0058 D4] A row-level `check` judges EVERY row a write stores — on an
5+
* ARRAY insert as on a single-row one.
6+
*
7+
* ## The guarantee
8+
*
9+
* `RowLevelSecurityPolicySchema.check` (declared, or defaulted from `using`) is
10+
* the write-side half of a row-level policy: a row the check refuses is never
11+
* stored. An ARRAY insert (`insert(object, [rows])`, which `createManyData`
12+
* calls) escaped it: step 3.6 of the security middleware excluded an array
13+
* payload, so no judgement was installed at all.
14+
*
15+
* ## What this file pins, on both SQL driver families
16+
*
17+
* - the repro, failing first: `[admitted, refused]` is refused on the ADR-0112
18+
* envelope (`PERMISSION_DENIED` / 403) and NOTHING is stored;
19+
* - the over-fix controls: an array whose every row passes is admitted and
20+
* stored; the single insert answers exactly as before;
21+
* - the configurations that refuse EVERY single insert refuse the array insert
22+
* too (an unresolvable sole `using`, an unresolvable declared `check`).
23+
*
24+
* Ground truth is read under a system context: "the gate refused" and "nothing
25+
* was stored" are separate facts, and both are asserted.
26+
*/
27+
28+
import { describe, it, expect, afterEach, vi } from 'vitest';
29+
import { ObjectQL } from '@objectstack/objectql';
30+
import { SqlDriver } from '@objectstack/driver-sql';
31+
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
32+
import { PermissionSetSchema } from '@objectstack/spec/security';
33+
import type { PermissionSet } from '@objectstack/spec/security';
34+
import { SecurityPlugin } from './security-plugin.js';
35+
import { defaultPermissionSets } from './objects/default-permission-sets.js';
36+
37+
const OBJECTS = [
38+
{
39+
name: 'qa_ticket',
40+
label: 'Ticket',
41+
sharingModel: 'public_read_write',
42+
fields: {
43+
id: { name: 'id', type: 'text', primaryKey: true },
44+
title: { name: 'title', type: 'text' },
45+
status: { name: 'status', type: 'text' },
46+
priority: { name: 'priority', type: 'text' },
47+
owner: { name: 'owner', type: 'text' },
48+
},
49+
},
50+
];
51+
52+
const ME = 'a@e.example';
53+
const SOMEONE_ELSE = 'b@e.example';
54+
55+
/** The #19964 repro's predicate. */
56+
const NOT_ARCHIVED = "record.status != 'archived'";
57+
/**
58+
* A `current_user.*` key no resolver publishes: the compiler cannot resolve it,
59+
* drops the policy, and the gate falls back to the deny sentinel — so every
60+
* single insert under it is refused.
61+
*/
62+
const UNRESOLVABLE = 'record.status in current_user.no_such_membership_key';
63+
64+
type Policy = { name: string; operation: string; using?: string; check?: string };
65+
66+
function permissionSet(policies: Policy[]): PermissionSet {
67+
return PermissionSetSchema.parse({
68+
name: 'qa_writer',
69+
objects: { qa_ticket: { allowRead: true, allowCreate: true, allowEdit: true } },
70+
rowLevelSecurity: policies.map((p) => ({ object: 'qa_ticket', ...p })),
71+
});
72+
}
73+
74+
const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!;
75+
const SYS_CTX = { isSystem: true, userId: 'usr_system' };
76+
const CALLER = {
77+
userId: 'usr_a',
78+
email: ME,
79+
positions: ['writer'],
80+
permissions: ['qa_writer'],
81+
posture: 'MEMBER',
82+
};
83+
84+
const engines: ObjectQL[] = [];
85+
afterEach(async () => {
86+
while (engines.length) {
87+
try { await engines.pop()?.destroy(); } catch { /* noop */ }
88+
}
89+
});
90+
91+
const SEED = [
92+
{ id: 't1', title: 'one', status: 'open', priority: 'low', owner: ME },
93+
{ id: 't2', title: 'two', status: 'open', priority: 'high', owner: ME },
94+
{ id: 't3', title: 'three', status: 'open', priority: 'low', owner: SOMEONE_ELSE },
95+
];
96+
97+
async function boot(makeDriver: () => unknown, ps: PermissionSet): Promise<ObjectQL> {
98+
const engine = new ObjectQL();
99+
engine.registerDriver(makeDriver() as never, true);
100+
await engine.init();
101+
engine.registerApp({
102+
id: 'com.objectstack.qa.rls-check-multi-row-writes',
103+
name: 'RLS check on multi-row writes',
104+
version: '1.0.0',
105+
type: 'plugin',
106+
scope: 'system',
107+
objects: OBJECTS,
108+
} as never);
109+
await engine.syncSchemas();
110+
engines.push(engine);
111+
const services: Record<string, unknown> = {
112+
manifest: { register: vi.fn() },
113+
objectql: engine,
114+
metadata: {
115+
get: async (_type: string, name: string) => engine.getSchema(name) ?? null,
116+
list: async () => [MEMBER_DEFAULT, ps],
117+
},
118+
};
119+
const ctx = {
120+
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
121+
registerService: vi.fn(),
122+
getService: (name: string) => {
123+
if (!(name in services)) throw new Error(`service not registered: ${name}`);
124+
return services[name];
125+
},
126+
};
127+
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
128+
await plugin.init(ctx as never);
129+
await plugin.start(ctx as never);
130+
// The expected refusals log at WARN through the engine's own logger.
131+
vi.spyOn((engine as unknown as { logger: { warn: () => void } }).logger, 'warn').mockImplementation(() => undefined);
132+
await engine.insert('qa_ticket', SEED.map((r) => ({ ...r })) as never, { context: SYS_CTX } as never);
133+
return engine;
134+
}
135+
136+
const DRIVERS: Array<[string, () => unknown]> = [
137+
['driver-sql (better-sqlite3 :memory:)',
138+
() => new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true } as never)],
139+
['driver-sqlite-wasm (:memory:)', () => new SqliteWasmDriver({ filename: ':memory:' } as never)],
140+
];
141+
142+
interface Outcome { ok: boolean; code?: string; status?: number; developerMessage?: string }
143+
144+
const attempt = async (run: () => Promise<unknown>): Promise<Outcome> => {
145+
try {
146+
await run();
147+
return { ok: true };
148+
} catch (e) {
149+
const err = e as { code?: string; statusCode?: number; status?: number; developerMessage?: string };
150+
return { ok: false, code: err.code, status: err.statusCode ?? err.status, developerMessage: err.developerMessage };
151+
}
152+
};
153+
154+
/** The 3.6 gate's refusal, on the ADR-0112 envelope — never a bare throw. */
155+
const expectCheckRefusal = (outcome: Outcome, verb: 'insert' | 'update') => {
156+
expect(outcome.ok, `expected a refusal, got a completed ${verb}`).toBe(false);
157+
expect(outcome.code, 'ADR-0112 error code').toBe('PERMISSION_DENIED');
158+
expect(outcome.status, 'ADR-0112 HTTP status').toBe(403);
159+
expect(outcome.developerMessage, 'the developer half names the check gate and the verb')
160+
.toContain(`the ${verb} would violate a row-level CHECK`);
161+
};
162+
163+
/** Every row, id → the columns under test, read past every scope. */
164+
const stored = async (engine: ObjectQL) => {
165+
const rows = (await engine.find('qa_ticket', { context: SYS_CTX } as never)) as Array<Record<string, unknown>>;
166+
return Object.fromEntries(
167+
rows.map((r) => [r.id, { title: r.title, status: r.status, owner: r.owner }]),
168+
) as Record<string, { title: unknown; status: unknown; owner: unknown }>;
169+
};
170+
171+
for (const [driverName, makeDriver] of DRIVERS) {
172+
describe(`[#19964] an ARRAY insert is judged row by row — ${driverName}`, () => {
173+
const insertMany = (engine: ObjectQL, rows: Array<Record<string, unknown>>) =>
174+
engine.insert('qa_ticket', rows as never, { context: CALLER } as never);
175+
const insertOne = (engine: ObjectQL, row: Record<string, unknown>) =>
176+
engine.insert('qa_ticket', row as never, { context: CALLER } as never);
177+
178+
it('the repro: `[admitted, refused]` is refused on the check envelope, and neither row is stored', async () => {
179+
const engine = await boot(makeDriver, permissionSet([{ name: 'not_archived', operation: 'insert', check: NOT_ARCHIVED }]));
180+
expectCheckRefusal(
181+
await attempt(() => insertMany(engine, [
182+
{ id: 'n1', title: 'n1', status: 'closed' },
183+
{ id: 'n2', title: 'n2', status: 'archived' },
184+
])),
185+
'insert',
186+
);
187+
expect(Object.keys(await stored(engine)).sort()).toEqual(['t1', 't2', 't3']);
188+
});
189+
190+
it('⭐ over-fix control: an array whose every row passes is admitted and stored', async () => {
191+
const engine = await boot(makeDriver, permissionSet([{ name: 'not_archived', operation: 'all', check: NOT_ARCHIVED }]));
192+
expect((await attempt(() => insertMany(engine, [
193+
{ id: 'n1', title: 'n1', status: 'closed' },
194+
{ id: 'n2', title: 'n2', status: 'open' },
195+
]))).ok).toBe(true);
196+
const after = await stored(engine);
197+
expect([after.n1?.status, after.n2?.status]).toEqual(['closed', 'open']);
198+
});
199+
200+
it('⭐ the single insert answers exactly as before', async () => {
201+
const engine = await boot(makeDriver, permissionSet([{ name: 'not_archived', operation: 'insert', check: NOT_ARCHIVED }]));
202+
expectCheckRefusal(await attempt(() => insertOne(engine, { id: 'n2', title: 'n2', status: 'archived' })), 'insert');
203+
expect((await attempt(() => insertOne(engine, { id: 'n1', title: 'n1', status: 'closed' }))).ok).toBe(true);
204+
expect(Object.keys(await stored(engine)).sort()).toEqual(['n1', 't1', 't2', 't3']);
205+
});
206+
207+
const REFUSE_EVERY_INSERT: Array<[string, Policy]> = [
208+
['an unresolvable sole `using` on `insert`', { name: 'bad_using', operation: 'insert', using: UNRESOLVABLE }],
209+
['an unresolvable sole `using` on `all`', { name: 'bad_using', operation: 'all', using: UNRESOLVABLE }],
210+
['an unresolvable declared `check`', { name: 'bad_check', operation: 'insert', check: UNRESOLVABLE }],
211+
];
212+
for (const [label, policy] of REFUSE_EVERY_INSERT) {
213+
it(`${label}: every single insert is refused, and so is the array insert`, async () => {
214+
const engine = await boot(makeDriver, permissionSet([policy]));
215+
expectCheckRefusal(await attempt(() => insertOne(engine, { id: 'n1', title: 'n1', status: 'open' })), 'insert');
216+
expectCheckRefusal(
217+
await attempt(() => insertMany(engine, [
218+
{ id: 'n1', title: 'n1', status: 'open' },
219+
{ id: 'n2', title: 'n2', status: 'closed' },
220+
])),
221+
'insert',
222+
);
223+
expect(Object.keys(await stored(engine)).sort()).toEqual(['t1', 't2', 't3']);
224+
});
225+
}
226+
});
227+
}

‎packages/plugins/plugin-security/src/rls-phantom-column-negation.test.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -319,10 +319,9 @@ for (const [driverName, makeDriver] of DRIVERS) {
319319

320320
describe(`[#17042] WRITE face, end to end — ${driverName}`, () => {
321321
/**
322-
* ⚠️ A SINGLE object, never an array. Step 3.6 is guarded by
323-
* `!Array.isArray(opCtx.data)`, so a bulk payload skips the check gate
324-
* entirely and every cell below would read "permitted" for a reason that
325-
* has nothing to do with this card.
322+
* A single-row insert, so each cell reads one row's verdict. (An array
323+
* insert is judged row by row too; that shape is pinned in
324+
* `rls-check-multi-row-writes.test.ts`.)
326325
*/
327326
const insert = (engine: ObjectQL, isPrivate: boolean) =>
328327
engine.insert(

‎packages/plugins/plugin-security/src/security-plugin.test.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,14 @@ import { BUILTIN_OPERATION_MESSAGES } from '@objectstack/spec/system';
3030
*
3131
* So the executor honours the seam the way the engine does: the flag first (it
3232
* answers "did the seam run", never "did the write pass"), then the judgement.
33-
* These doubles run no hooks, so the row that would be stored IS `opCtx.data`.
33+
* These doubles run no hooks, so the rows an insert would store ARE
34+
* `opCtx.data` — each element of an array insert ([#19964]).
3435
*/
3536
const runEngineWriteBody = async (opCtx: any): Promise<void> => {
3637
const seam = opCtx?.postHookWriteImageCheck;
3738
if (!seam) return;
3839
seam.honoured = true;
39-
await seam.evaluate([opCtx.data]);
40+
await seam.evaluate(Array.isArray(opCtx.data) ? opCtx.data : [opCtx.data]);
4041
};
4142

4243
// ---------------------------------------------------------------------------

‎packages/plugins/plugin-security/src/security-plugin.ts‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2993,11 +2993,21 @@ export class SecurityPlugin implements Plugin {
29932993
// checked field must arrive from the caller — is REFUSED, not deferred:
29942994
// it institutionalises the contradiction (the caller sending the value the
29952995
// hook exists to make un-sendable) and needs a permanent lint to keep it.
2996+
//
2997+
// ── [#19964] EVERY row an insert stores ───────────────────────────────
2998+
//
2999+
// The check is a guarantee about each stored row, so an insert that
3000+
// stores several rows is judged once per row, and ONE failing row
3001+
// refuses the whole write. An ARRAY insert used to be excluded by a
3002+
// non-array guard here, so no judgement was installed and every row was
3003+
// stored unjudged. The seam already receives every live row of an array
3004+
// insert, so the guard now admits an array for `insert` (an `update`
3005+
// still takes one payload).
29963006
if (
29973007
(opCtx.operation === 'insert' || opCtx.operation === 'update') &&
29983008
opCtx.data &&
29993009
typeof opCtx.data === 'object' &&
3000-
!Array.isArray(opCtx.data) &&
3010+
(opCtx.operation === 'insert' || !Array.isArray(opCtx.data)) &&
30013011
permissionSets.length > 0 &&
30023012
!!opCtx.context?.userId
30033013
) {
@@ -3051,8 +3061,10 @@ export class SecurityPlugin implements Plugin {
30513061
checkParts.every((f) => matchesFilterCondition(image as any, f as any));
30523062

30533063
if (opCtx.operation === 'insert') {
3054-
// [#16608] Install the judgement; the engine runs it on the row the
3055-
// `beforeInsert` chain produced. The compiled filter is captured
3064+
// [#16608] Install the judgement; the engine runs it on the rows the
3065+
// `beforeInsert` chain produced — [#19964] every row of an array
3066+
// insert, and the first that fails refuses the whole write. The
3067+
// compiled filter is captured
30563068
// HERE — while the caller's permission sets, the delegator's, the
30573069
// staged membership and this request's context are all resolved —
30583070
// and only the IMAGE is deferred. Deferring the compilation too

0 commit comments

Comments
 (0)