Skip to content

Commit 612f0d3

Browse files
committed
test: pin the by-id OCC handover to the write door at the protocol, runtime and HTTP doors
The protocol pins cover both arms (no row, read refused), the fault that still propagates and the readable-row conflict. The runtime pin adds an object the caller reads but may not write under the real security layer. The showcase pins compare status, code and sentence for a row and for no row, token or none, on a single install and behind the organization wall. Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk Co-authored-by: Claude <noreply@anthropic.com>
1 parent 57b661d commit 612f0d3

5 files changed

Lines changed: 525 additions & 17 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
fix(metadata-protocol): the by-id OCC comparison answers only for a row the caller reads; every other row gets the write door's answer
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
`updateData` and `deleteData` compare a caller's version token (`If-Match`, or `expectedVersion`) before the write is authorized. On a mismatch they re-read the row as the caller. When that read returns no row, or refuses with a declared client-band status, the comparison is now dropped and the request goes on to the write, whose door answers it: the same status, code and sentence the request gets without a token, and the same a token on an id naming no row gets. This restores the governing text for the write doors (ruling of record 5985885287).
11+
12+
- **Unchanged:** a row the caller reads still answers `409 CONCURRENT_UPDATE` with that row as `currentRecord`, including a row the caller reads but may not write. A matching token, or none, reads nothing more than before. A read that faults (a server-band status, or none declared) still propagates as raised.
13+
- **The write door's answers:** a caller who may not update or delete the object gets that refusal (`403`), token or none; a caller who may write the object gets `404 RECORD_NOT_FOUND` for a row its read does not return, as for an id naming no row.

‎packages/metadata-protocol/src/protocol.record-not-found.test.ts‎

Lines changed: 123 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import { describe, it, expect, vi } from 'vitest';
2424
import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/metadata-core';
25+
import { recordNotFoundError } from '@objectstack/core';
2526
import { ObjectStackProtocolImplementation } from './protocol.js';
2627

2728
const SCHEMA = { name: 'task', fields: { title: { name: 'title', type: 'text' } } };
@@ -43,18 +44,40 @@ function writeRefusal(): Error {
4344
* the write is judged before the row is looked up. `leavesScope` names rows a
4445
* landed update moves out of the caller's row scope, so their readback is
4546
* `null` while the row is still there.
47+
*
48+
* `hidden` names rows a CALLER's read does not return (the system's read
49+
* still does), and a by-id write of one is answered the way the security
50+
* layer answers it: the read question is asked ahead of the write, and a row
51+
* the caller cannot read gets what an id naming no row gets. `readRefusal`
52+
* makes every caller read throw instead — a refusal, or a fault.
4653
*/
4754
function makeProtocol(
4855
rows: Record<string, any> = {},
49-
opts: { refuse?: string; leavesScope?: ReadonlySet<string> } = {},
56+
opts: {
57+
refuse?: string;
58+
leavesScope?: ReadonlySet<string>;
59+
hidden?: ReadonlySet<string>;
60+
readRefusal?: () => Error;
61+
} = {},
5062
) {
5163
const store = new Map<string, any>(Object.entries(rows));
5264
const refused = (o: any) => opts.refuse !== undefined && o?.context?.userId === opts.refuse;
53-
const findOne = vi.fn(async (_object: string, o: any) => { assertEngineFindOnePredicate(_object, o); return store.get(String(o?.where?.id)) ?? null; });
65+
/** A read or write made as a caller rather than as the system. */
66+
const asCaller = (o: any) => o?.context?.isSystem !== true;
67+
const findOne = vi.fn(async (_object: string, o: any) => {
68+
assertEngineFindOnePredicate(_object, o);
69+
const id = String(o?.where?.id);
70+
if (asCaller(o)) {
71+
if (opts.readRefusal) throw opts.readRefusal();
72+
if (opts.hidden?.has(id)) return null;
73+
}
74+
return store.get(id) ?? null;
75+
});
5476
const update = vi.fn(async (_object: string, data: any, o: any) => {
5577
assertEngineUpdateDispatch(data, o);
5678
if (refused(o)) throw writeRefusal();
5779
const id = String(o?.where?.id);
80+
if (asCaller(o) && opts.hidden?.has(id)) throw recordNotFoundError('task', id);
5881
if (!store.has(id)) return null;
5982
const next = { ...store.get(id), ...data };
6083
store.set(id, next);
@@ -63,7 +86,9 @@ function makeProtocol(
6386
const del = vi.fn(async (_object: string, o: any) => {
6487
assertEngineDeleteDispatch(o);
6588
if (refused(o)) throw writeRefusal();
66-
return store.delete(String(o?.where?.id));
89+
const id = String(o?.where?.id);
90+
if (asCaller(o) && opts.hidden?.has(id)) throw recordNotFoundError('task', id);
91+
return store.delete(id);
6792
});
6893
const engine = {
6994
registry: { getObject: (n: string) => (n === 'task' ? SCHEMA : undefined) },
@@ -358,3 +383,98 @@ describe('[#4435] deleteManyData reports per id, not per request', () => {
358383
expect(res.succeeded + res.failed).toBe(res.total);
359384
});
360385
});
386+
387+
describe('a stale version token on a row the caller does not read gets the write door\'s answer, as no token does', () => {
388+
// The by-id OCC comparison runs before the write is authorized. When the
389+
// tokens disagree it re-reads the row as the caller, and a row that read
390+
// does not return — absent, or the read refused — leaves the answer to the
391+
// write door: what the same request without a token gets, status, code and
392+
// sentence, which is also what a token on an id naming no row gets (ruling
393+
// of record 5985885287: a row the caller cannot read answers what a
394+
// nonexistent id answers, on update and delete alike).
395+
const CURRENT = '2026-10-30T08:00:00.000Z';
396+
const STALE = '2001-01-01T00:00:00.000Z';
397+
const CALLER = { userId: 'u1' };
398+
const MISSING = 'definitely_not_a_row';
399+
const row = (id: string) => ({ id, title: 'x', updated_at: CURRENT });
400+
401+
/** A refused call as the doors render it: status, code and sentence. */
402+
const answerOf = (run: () => Promise<unknown>) => run().then(
403+
() => { throw new Error('expected a refusal, but the call resolved'); },
404+
(e: any) => ({ code: e.code, status: e.status, message: e.message }),
405+
);
406+
/** The answer with the id its sentence names written out, so the answers for two ids compare. */
407+
const withoutId = (a: { message: unknown }, id: string) => ({ ...a, message: String(a.message).split(id).join('ID') });
408+
409+
for (const verb of ['updateData', 'deleteData'] as const) {
410+
const write = (p: any, id: string, expectedVersion?: string) => verb === 'updateData'
411+
? p.updateData({ object: 'task', id, data: { title: 'y' }, context: CALLER, ...(expectedVersion ? { expectedVersion } : {}) })
412+
: p.deleteData({ object: 'task', id, context: CALLER, ...(expectedVersion ? { expectedVersion } : {}) });
413+
414+
describe(verb, () => {
415+
it('a hidden row, the write refused: the write refusal answers, for the row and for no row, token or none', async () => {
416+
const { p, store } = makeProtocol({ real: row('real') }, { refuse: 'u1', hidden: new Set(['real']) });
417+
const stale = await answerOf(() => write(p, 'real', STALE));
418+
const noToken = await answerOf(() => write(p, 'real'));
419+
const missing = await answerOf(() => write(p, MISSING, STALE));
420+
expect(stale).toMatchObject({ code: 'PERMISSION_DENIED', status: 403 });
421+
expect(stale).toEqual(noToken);
422+
expect(withoutId(missing, MISSING)).toEqual(withoutId(stale, 'real'));
423+
expect(store.get('real')).toEqual(row('real'));
424+
});
425+
426+
it('the caller\'s read REFUSES: the write door\'s refusal answers, never the read\'s', async () => {
427+
// A declared client-band status in either spelling is a refusal.
428+
for (const readRefusal of [
429+
() => Object.assign(new Error('Reading this object is not permitted.'), { code: 'PERMISSION_DENIED', status: 403 }),
430+
() => Object.assign(new Error('Reading this object is not permitted.'), { code: 'PERMISSION_DENIED', statusCode: 403 }),
431+
]) {
432+
const { p, store } = makeProtocol({ real: row('real') }, { refuse: 'u1', readRefusal });
433+
const stale = await answerOf(() => write(p, 'real', STALE));
434+
const noToken = await answerOf(() => write(p, 'real'));
435+
const missing = await answerOf(() => write(p, MISSING, STALE));
436+
expect(stale).toEqual(noToken);
437+
expect(stale.message).toBe(writeRefusal().message);
438+
expect(withoutId(missing, MISSING)).toEqual(withoutId(stale, 'real'));
439+
expect(store.get('real')).toEqual(row('real'));
440+
}
441+
});
442+
443+
it('a hidden row, the write admitted up to its read question: the write door\'s 404, for the row and for no row alike', async () => {
444+
const { p, store, update, del } = makeProtocol({ real: row('real') }, { hidden: new Set(['real']) });
445+
const stale = await answerOf(() => write(p, 'real', STALE));
446+
expect(stale).toMatchObject({ code: 'RECORD_NOT_FOUND', status: 404 });
447+
// The write was asked: the 404 is the write door's, not one of the comparison's own.
448+
expect((verb === 'updateData' ? update : del)).toHaveBeenCalledTimes(1);
449+
expect(await answerOf(() => write(p, 'real'))).toEqual(stale);
450+
expect(withoutId(await answerOf(() => write(p, MISSING, STALE)), MISSING)).toEqual(withoutId(stale, 'real'));
451+
expect(store.get('real')).toEqual(row('real'));
452+
});
453+
454+
it('a read that FAULTS propagates as raised, and the write is never asked', async () => {
455+
for (const fault of [
456+
new Error('connection reset'),
457+
Object.assign(new Error('store unavailable'), { code: 'SERVICE_UNAVAILABLE', status: 503 }),
458+
]) {
459+
const { p, update, del } = makeProtocol({ real: row('real') }, { readRefusal: () => fault });
460+
await expect(write(p, 'real', STALE)).rejects.toBe(fault);
461+
expect(update).not.toHaveBeenCalled();
462+
expect(del).not.toHaveBeenCalled();
463+
}
464+
});
465+
466+
it('control: a row the caller reads still conflicts — 409 with that row, and the write is never asked', async () => {
467+
const { p, store, update, del } = makeProtocol({ real: row('real') });
468+
await expect(write(p, 'real', STALE)).rejects.toMatchObject({
469+
code: 'CONCURRENT_UPDATE',
470+
status: 409,
471+
currentVersion: CURRENT,
472+
currentRecord: row('real'),
473+
});
474+
expect(update).not.toHaveBeenCalled();
475+
expect(del).not.toHaveBeenCalled();
476+
expect(store.get('real')).toEqual(row('real'));
477+
});
478+
});
479+
}
480+
});

0 commit comments

Comments
 (0)