Skip to content

Commit ee0a5c0

Browse files
committed
test(runtime): pin the OCC conflict answer to what the caller may read, at both /data doors
Claude-Session: https://claude.ai/code/session_01YNquHRZKDpiKsgmHXSd9Jx Co-authored-by: Claude <noreply@anthropic.com>
1 parent 93f3526 commit ee0a5c0

1 file changed

Lines changed: 364 additions & 0 deletions

File tree

Lines changed: 364 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,364 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#23586] A `409 CONCURRENT_UPDATE` reports only what the caller may read —
5+
* with the REAL security layer (`SecurityPlugin` on a real `ObjectQL` over a
6+
* real `SqlDriver`), at both doors of `PATCH` and `DELETE /data/:object/:id`:
7+
* `RestServer`'s routes and the runtime dispatcher's `/data` domain.
8+
*
9+
* ## What was wrong
10+
*
11+
* `updateData` / `deleteData` judge the caller's version token on the row
12+
* `probeRecord` reads with a SYSTEM context (an existence question, on
13+
* purpose), and they judge it before `engine.update` / `engine.delete` runs
14+
* the caller's write policy. The conflict carried that system-read row as
15+
* `currentRecord`. So a stale token got a member a row their RLS hides, or a
16+
* field their FLS hides, and the `409` itself (with `currentVersion`) told
17+
* them a hidden row exists where the same request without a token answers
18+
* `404`.
19+
*
20+
* ## What is held here, per door and per verb
21+
*
22+
* - **A row the caller's RLS hides:** a stale token answers exactly what the
23+
* same request without a token answers (the control): `404
24+
* RECORD_NOT_FOUND`, the same status and the same body, with no
25+
* `currentVersion` and no `currentRecord`. The row is untouched.
26+
* - **A field the caller's FLS hides:** the `409`'s `currentRecord` does not
27+
* carry it, as the caller's own read of the row does not.
28+
* - **A row the caller may read whole (the positive control):** a stale token
29+
* still answers `409` with the row and its version, so conflict detection
30+
* is not weakened.
31+
* - **A token that matches:** the request reads the store exactly as often as
32+
* one with no token. Only the mismatch branch reads again.
33+
*
34+
* ## How each door is read
35+
*
36+
* `RestServer`: its registered handler, the auth boundary (`resolveExecCtx`)
37+
* stubbed, and the body it answered. The dispatcher: `/data` has no local
38+
* catch, so the protocol's throw reaches the host's exit; it is rendered here
39+
* through `declaredEnvelopeForThrow` (`plugin-hono-server`), the exit both
40+
* Hono hosts answer a throw with, and the thrown error itself is read too.
41+
*/
42+
43+
import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
44+
import { PermissionSetSchema } from '@objectstack/spec/security';
45+
import { ObjectQL } from '@objectstack/objectql';
46+
import { SqlDriver } from '@objectstack/driver-sql';
47+
import { SecurityPlugin } from '@objectstack/plugin-security';
48+
import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
49+
import { RestServer } from '@objectstack/rest';
50+
import { declaredEnvelopeForThrow } from '@objectstack/plugin-hono-server';
51+
import { resolveThrownHttpError } from '@objectstack/types';
52+
import { HttpDispatcher } from '../http-dispatcher.js';
53+
54+
const PREFIX = '/api/v1';
55+
const DATA = `${PREFIX}/data`;
56+
57+
/** The object with a row scope and a hidden field. */
58+
const SCOPED = 'occ_vis_ledger';
59+
/** An object no row or field rule names: the positive control. */
60+
const PLAIN = 'occ_vis_plain';
61+
62+
const SYS_CTX = { isSystem: true, userId: 'usr_system' };
63+
64+
const MEMBER_SET = PermissionSetSchema.parse({
65+
name: 'member_default',
66+
label: 'Member',
67+
objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } },
68+
fields: { [`${SCOPED}.secret`]: { readable: false, editable: false } },
69+
rowLevelSecurity: [{ name: 'region_scope', object: SCOPED, operation: 'all', using: "record.region != 'HIDDEN'" }],
70+
});
71+
72+
const MEMBER_CTX = { userId: 'usr_member', positions: [], permissions: [MEMBER_SET.name], posture: 'MEMBER' };
73+
74+
/** A well-formed instant no row here carries: a token read before a write that moved it. */
75+
const STALE = '2000-01-01T00:00:00.000Z';
76+
77+
/** The row RLS hides from the member. */
78+
const HIDDEN_ID = 'h1';
79+
/** The row the member reads, without its `secret`. */
80+
const MASKED_ID = 'v1';
81+
/** The positive control's row. */
82+
const PLAIN_ID = 'p1';
83+
84+
const clone = <T>(v: T): T => (v === undefined ? v : JSON.parse(JSON.stringify(v)));
85+
86+
interface Answer {
87+
status: number;
88+
body: any;
89+
/** The dispatcher only: the error the protocol threw, before the exit rendered it. */
90+
thrown?: any;
91+
}
92+
93+
type Verb = 'PATCH' | 'DELETE';
94+
95+
interface Door {
96+
send(verb: Verb, object: string, id: string, headers?: Record<string, string>): Promise<Answer>;
97+
}
98+
99+
function makeRes() {
100+
const res: any = { statusCode: 200, body: undefined, headers: {} as Record<string, string> };
101+
res.status = (code: number) => { res.statusCode = code; return res; };
102+
res.json = (body: any) => { res.body = body; return res; };
103+
res.send = () => res;
104+
res.end = () => res;
105+
res.header = (k: string, v: string) => { res.headers[k] = v; return res; };
106+
res.setHeader = () => {};
107+
res.write = () => true;
108+
return res;
109+
}
110+
111+
/** The PATCH body every request below sends; DELETE carries none. */
112+
const PATCH_BODY = { title: 'written' };
113+
114+
function restDoor(protocol: ObjectStackProtocolImplementation): Door {
115+
const noop = () => {};
116+
const server = { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} };
117+
const rest: any = new RestServer(server as any, protocol as any, { api: { requireAuth: false } } as any);
118+
rest.resolveExecCtx = async () => clone(MEMBER_CTX);
119+
rest.registerRoutes();
120+
const routeOf = (verb: Verb) => {
121+
const route = rest.getRoutes().find((r: any) => r.method === verb && r.path === `${DATA}/:object/:id`);
122+
if (!route) throw new Error(`${verb} ${DATA}/:object/:id is not registered`);
123+
return route;
124+
};
125+
return {
126+
send: async (verb, object, id, headers = {}) => {
127+
const res = makeRes();
128+
await routeOf(verb).handler({
129+
method: verb, path: `${DATA}/${object}/${id}`, params: { object, id },
130+
headers: Object.fromEntries(Object.entries(headers).map(([k, v]) => [k.toLowerCase(), v])),
131+
query: {},
132+
body: verb === 'PATCH' ? clone(PATCH_BODY) : undefined,
133+
}, res);
134+
return { status: res.statusCode, body: res.body };
135+
},
136+
};
137+
}
138+
139+
/**
140+
* What `createHonoApp`'s `${prefix}/*` catch-all hands `dispatch()`, over a
141+
* real Fetch `Request` (the sibling `data-update-expected-version.test.ts`
142+
* spells the same statements), and the exit's rendering of a throw.
143+
*/
144+
function dispatcherDoor(protocol: ObjectStackProtocolImplementation): Door {
145+
const services: Record<string, unknown> = { protocol };
146+
const get = (n: string) => services[n] ?? null;
147+
const kernel: any = { context: { getService: get }, getService: get, getServiceAsync: async (n: string) => get(n) };
148+
const dispatcher = new HttpDispatcher(kernel);
149+
(dispatcher as any).timedResolveExecutionContext = async () => clone(MEMBER_CTX);
150+
return {
151+
send: async (verb, object, id, headers = {}) => {
152+
const raw = new Request(`http://localhost${DATA}/${object}/${id}`, {
153+
method: verb,
154+
headers: { 'content-type': 'application/json', ...headers },
155+
...(verb === 'PATCH' ? { body: JSON.stringify(PATCH_BODY) } : {}),
156+
});
157+
const url = new URL(raw.url);
158+
let body: unknown;
159+
try { body = JSON.parse(await raw.clone().text()); } catch { body = {}; }
160+
try {
161+
const res: any = await dispatcher.dispatch(verb, url.pathname.substring(PREFIX.length), body, {}, { request: raw } as any, PREFIX);
162+
return { status: res.response?.status ?? 0, body: res.response?.body };
163+
} catch (thrown) {
164+
const envelope = declaredEnvelopeForThrow(thrown);
165+
if (envelope) return { status: envelope.status, body: envelope.body, thrown };
166+
const resolved = resolveThrownHttpError(thrown);
167+
return { status: resolved.status, body: { code: resolved.code, message: resolved.message }, thrown };
168+
}
169+
},
170+
};
171+
}
172+
173+
const ifMatch = (token: string) => ({ 'If-Match': `"${token}"` });
174+
175+
describe('[#23586] a 409 CONCURRENT_UPDATE reports only what the caller may read — the real security layer', () => {
176+
let engine: ObjectQL;
177+
let protocol: ObjectStackProtocolImplementation;
178+
const doors: Array<[string, Door]> = [];
179+
180+
/** The row as the system reads it now (`null` once deleted). */
181+
const systemRow = async (object: string, id: string) =>
182+
(await engine.findOne(object, { where: { id }, context: SYS_CTX } as any)) as any;
183+
/** The row's current version token, as the system reads it. */
184+
const versionOf = async (object: string, id: string): Promise<string> => {
185+
const row = await systemRow(object, id);
186+
const v = row?.updated_at instanceof Date ? row.updated_at.toISOString() : String(row?.updated_at ?? '');
187+
if (!v) throw new Error(`${object}/${id} carries no updated_at, so no version token exists to pin`);
188+
return v;
189+
};
190+
191+
beforeAll(async () => {
192+
engine = new ObjectQL();
193+
engine.registerDriver(
194+
new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true } as any),
195+
true,
196+
);
197+
await engine.init();
198+
engine.registerApp({
199+
id: 'com.objectstack.qa.occ-conflict-caller-visibility',
200+
name: 'OCC conflict caller visibility',
201+
version: '1.0.0',
202+
type: 'plugin',
203+
scope: 'system',
204+
objects: [
205+
{
206+
name: SCOPED,
207+
label: 'Ledger',
208+
sharingModel: 'public_read_write',
209+
fields: {
210+
title: { name: 'title', type: 'text' },
211+
region: { name: 'region', type: 'text' },
212+
secret: { name: 'secret', type: 'text' },
213+
},
214+
},
215+
{
216+
name: PLAIN,
217+
label: 'Plain',
218+
sharingModel: 'public_read_write',
219+
fields: {
220+
title: { name: 'title', type: 'text' },
221+
note: { name: 'note', type: 'text' },
222+
},
223+
},
224+
],
225+
} as never);
226+
await engine.syncSchemas();
227+
228+
const services: Record<string, unknown> = {
229+
manifest: { register: vi.fn() },
230+
objectql: engine,
231+
metadata: {
232+
get: async (_type: string, name: string) => engine.getSchema(name) ?? null,
233+
list: async () => [MEMBER_SET],
234+
},
235+
};
236+
const ctx = {
237+
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
238+
registerService: vi.fn(),
239+
getService: (name: string) => {
240+
if (!(name in services)) throw new Error(`service not registered: ${name}`);
241+
return services[name];
242+
},
243+
};
244+
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
245+
await plugin.init(ctx as never);
246+
await plugin.start(ctx as never);
247+
vi.spyOn((engine as unknown as { logger: { warn: () => void } }).logger, 'warn').mockImplementation(() => undefined);
248+
249+
protocol = new ObjectStackProtocolImplementation(engine as any);
250+
doors.push(['RestServer', restDoor(protocol)], ['the dispatcher', dispatcherDoor(protocol)]);
251+
}, 60_000);
252+
253+
afterAll(async () => {
254+
try { await engine?.destroy(); } catch { /* noop */ }
255+
});
256+
257+
/** A fresh copy of the three rows, so each case starts from a known store. */
258+
async function seed(): Promise<void> {
259+
for (const [object, id] of [[SCOPED, HIDDEN_ID], [SCOPED, MASKED_ID], [PLAIN, PLAIN_ID]] as const) {
260+
if (await systemRow(object, id)) await engine.delete(object, { where: { id }, context: SYS_CTX } as any);
261+
}
262+
await engine.insert(SCOPED, [
263+
{ id: HIDDEN_ID, title: 'hidden', region: 'HIDDEN', secret: 'hidden-secret' },
264+
{ id: MASKED_ID, title: 'visible', region: 'NA', secret: 'masked-secret' },
265+
], { context: SYS_CTX } as never);
266+
await engine.insert(PLAIN, [{ id: PLAIN_ID, title: 'plain', note: 'readable' }], { context: SYS_CTX } as never);
267+
}
268+
269+
const codeOf = (body: any): string | undefined =>
270+
body?.error && typeof body.error === 'object' ? body.error.code : body?.code;
271+
/** Where either door puts a conflict's state: flat beside `code`, or in the envelope's `details`. */
272+
const conflictState = (body: any): Record<string, unknown> =>
273+
(body?.error && typeof body.error === 'object' ? body.error.details : body) ?? {};
274+
275+
for (const verb of ['PATCH', 'DELETE'] as const) {
276+
for (const doorIndex of [0, 1]) {
277+
const doorName = doorIndex === 0 ? 'RestServer' : 'the dispatcher';
278+
const door = () => doors[doorIndex][1];
279+
280+
describe(`${doorName}: ${verb} /data/:object/:id`, () => {
281+
it('a row RLS hides: a stale token answers exactly what the control (no token) answers — 404, no version, no record', async () => {
282+
await seed();
283+
const control = await door().send(verb, SCOPED, HIDDEN_ID);
284+
expect({ status: control.status, code: codeOf(control.body) }, JSON.stringify(control.body))
285+
.toEqual({ status: 404, code: 'RECORD_NOT_FOUND' });
286+
287+
const stale = await door().send(verb, SCOPED, HIDDEN_ID, ifMatch(STALE));
288+
// Indistinguishable: status, and the whole body (code, message, keys).
289+
expect(stale.status, JSON.stringify(stale.body)).toBe(control.status);
290+
expect(stale.body).toEqual(control.body);
291+
const wire = JSON.stringify(stale.body);
292+
expect(wire).not.toContain('currentVersion');
293+
expect(wire).not.toContain('currentRecord');
294+
expect(wire).not.toContain('hidden-secret');
295+
if (stale.thrown !== undefined) {
296+
// The dispatcher's throw itself carries no row and no version.
297+
expect(stale.thrown.currentRecord).toBeUndefined();
298+
expect(stale.thrown.currentVersion).toBeUndefined();
299+
expect(stale.thrown.code).toBe(control.thrown?.code);
300+
}
301+
// Nothing was written.
302+
expect(await systemRow(SCOPED, HIDDEN_ID)).toMatchObject({ title: 'hidden', secret: 'hidden-secret' });
303+
}, 60_000);
304+
305+
it('a field FLS hides: the 409 carries the row the caller reads, without that field', async () => {
306+
await seed();
307+
const version = await versionOf(SCOPED, MASKED_ID);
308+
const stale = await door().send(verb, SCOPED, MASKED_ID, ifMatch(STALE));
309+
expect({ status: stale.status, code: codeOf(stale.body) }, JSON.stringify(stale.body))
310+
.toEqual({ status: 409, code: 'CONCURRENT_UPDATE' });
311+
expect(conflictState(stale.body).currentVersion).toBe(version);
312+
expect(JSON.stringify(stale.body)).not.toContain('masked-secret');
313+
314+
// The record the conflict holds is the caller's own read of the row.
315+
const thrownRecord = stale.thrown?.currentRecord;
316+
const record: any = doorIndex === 0 ? conflictState(stale.body).currentRecord : thrownRecord;
317+
expect(record).toMatchObject({ id: MASKED_ID, title: 'visible', region: 'NA' });
318+
expect(record).not.toHaveProperty('secret');
319+
const callerRead: any = await engine.findOne(SCOPED, { where: { id: MASKED_ID }, context: clone(MEMBER_CTX) } as any);
320+
expect(callerRead).not.toHaveProperty('secret');
321+
expect(record).toEqual(callerRead);
322+
323+
expect(await systemRow(SCOPED, MASKED_ID)).toMatchObject({ title: 'visible', secret: 'masked-secret' });
324+
}, 60_000);
325+
326+
it('positive control: a row the caller reads whole still conflicts, with the whole row and its version', async () => {
327+
await seed();
328+
const version = await versionOf(PLAIN, PLAIN_ID);
329+
const stale = await door().send(verb, PLAIN, PLAIN_ID, ifMatch(STALE));
330+
expect({ status: stale.status, code: codeOf(stale.body) }, JSON.stringify(stale.body))
331+
.toEqual({ status: 409, code: 'CONCURRENT_UPDATE' });
332+
expect(conflictState(stale.body).currentVersion).toBe(version);
333+
const record: any = doorIndex === 0 ? conflictState(stale.body).currentRecord : stale.thrown?.currentRecord;
334+
expect(record).toEqual(await systemRow(PLAIN, PLAIN_ID));
335+
expect(record).toMatchObject({ id: PLAIN_ID, title: 'plain', note: 'readable' });
336+
// Not written.
337+
expect(await systemRow(PLAIN, PLAIN_ID)).toMatchObject({ title: 'plain' });
338+
}, 60_000);
339+
340+
it('a matching token reads the store exactly as often as no token, and is served', async () => {
341+
const findOne = vi.spyOn(engine, 'findOne');
342+
try {
343+
await seed();
344+
findOne.mockClear();
345+
const plain = await door().send(verb, PLAIN, PLAIN_ID);
346+
expect(plain.status, JSON.stringify(plain.body)).toBe(200);
347+
const withoutToken = findOne.mock.calls.length;
348+
349+
await seed();
350+
const version = await versionOf(PLAIN, PLAIN_ID);
351+
findOne.mockClear();
352+
const matched = await door().send(verb, PLAIN, PLAIN_ID, ifMatch(version));
353+
expect(matched.status, JSON.stringify(matched.body)).toBe(200);
354+
// DELETE probes once more when a token is present (it skips the probe without one);
355+
// PATCH probes in both. Nothing else is read on the match path.
356+
expect(findOne.mock.calls.length).toBe(withoutToken + (verb === 'DELETE' ? 1 : 0));
357+
} finally {
358+
findOne.mockRestore();
359+
}
360+
}, 60_000);
361+
});
362+
}
363+
}
364+
});

0 commit comments

Comments
 (0)