Skip to content

Commit 8372e0a

Browse files
committed
fix(metadata-protocol): the by-id OCC comparison hands over only a row absent from the caller's read
A caller re-read that throws now propagates as raised again, refusals included; only a read that returns no row leaves the answer to the write door. The docblocks, the changeset and the http-protocol paragraph describe that absent-row case only. Pins: a write grant without a read grant keeps its stale-token write from running at the protocol double and at the runtime doors; the dogfood families whose read is refused compare status and code. Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk Co-authored-by: Claude <noreply@anthropic.com>
1 parent 357b2ba commit 8372e0a

7 files changed

Lines changed: 158 additions & 109 deletions

File tree

‎.changeset/harden-occ-unreadable-row-write-door.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22
'@objectstack/metadata-protocol': patch
33
---
44

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
5+
fix(metadata-protocol): the by-id OCC comparison hands a row absent from the caller's read to the write door
66

77
Clause-②: no
88
Metadata-protocol: none
99

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).
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, the comparison is now dropped and the request goes on to the write, whose door answers it: the answer the request gets without a token, and the answer a token on an id naming no row gets. This restores the governing text for the write doors (ruling of record 5985885287).
1111

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.
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 caller read that throws — a refusal or a fault — propagates as raised, as before. A matching token, or none, reads nothing more than before.
13+
- **The write door's answers for a row absent from the caller's read:** a caller who may not update or delete the object gets that refusal (`403`); a caller who may write the object gets `404 RECORD_NOT_FOUND`, as for an id naming no row.

‎content/docs/protocol/kernel/http-protocol.mdx‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -753,10 +753,11 @@ each row of `updateMany` and of `batch` with `operation: "update"`. A caller who
753753
update the object gets `404 RECORD_NOT_FOUND` for an id that names no record, and for
754754
a record its row-level access hides.
755755

756-
A version token (`If-Match`, or `expectedVersion`) changes none of these answers. An
757-
update or delete carrying a stale token is answered `409 CONCURRENT_UPDATE` only for a
758-
record the caller can read; for any other id it gets exactly what the same request
759-
without a token gets.
756+
A version token (`If-Match`, or `expectedVersion`) is compared only against a record the
757+
caller can read: a stale token on such a record is answered `409 CONCURRENT_UPDATE`,
758+
whether or not the caller may write it. For a record the caller's read does not return,
759+
and for an id that names no record, a stale token changes none of the answers above. A
760+
caller refused reading the object gets that read refusal.
760761

761762
### Delete Record
762763

‎packages/metadata-protocol/src/protocol-record-reads.ts‎

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
* messages. The module-level declarations only these bodies read (the OCC
2424
* version-token reading) moved with them, also unedited. One body has changed
2525
* since: `assertVersionOf`'s mismatch branch now hands a row the caller's own
26-
* read does not return to the write door instead of answering it (its
27-
* docblock in `protocol.ts` says why).
26+
* read returns no row for to the write door instead of answering it (its
27+
* docblock in `protocol.ts` says why); a read that throws still propagates.
2828
*
2929
* ## How the moved bodies reach the class
3030
*
@@ -53,7 +53,6 @@
5353
import type { ObjectStackProtocolImplementation } from './protocol.js';
5454
import { ConcurrentUpdateError, MalformedVersionTokenError, canonicalVersionInstant } from './protocol.js';
5555
import { recordNotFoundError } from '@objectstack/core';
56-
import { declaredHttpStatus } from '@objectstack/types';
5756
import { collectInternalWriteResponseFields, omitInternalFieldsFromWriteResponse } from './write-response-internal-fields.js';
5857
import { isMissingTableError } from '@objectstack/metadata/errors';
5958
import { canServeApiOperation, resolveSearchFieldResolution } from '@objectstack/spec/data';
@@ -173,19 +172,6 @@ function versionTokensAgree(current: NormalisedVersion, expected: NormalisedVers
173172
return current.token === expected.token;
174173
}
175174

176-
/**
177-
* Did the caller's own read of a row REFUSE, rather than fault? A refusal
178-
* declares a client-band status (`4xx`, in either spelling
179-
* `declaredHttpStatus` reads) — the read door's verdict on this caller, which
180-
* says nothing about the row. A server-band status, or none at all (a bare
181-
* driver `Error`), is a fault, and the OCC comparison lets it propagate as
182-
* raised rather than read it as "the caller reads no row".
183-
*/
184-
function isCallerReadRefusal(e: unknown): boolean {
185-
const status = declaredHttpStatus(e);
186-
return status !== undefined && status < 500;
187-
}
188-
189175
/**
190176
* The `ObjectStackProtocolImplementation` members the functions below read —
191177
* every one of them, and no other; each function's `this` is the `Pick` of the
@@ -244,18 +230,13 @@ export async function assertVersionOf(
244230
// it names, turning a resolvable conflict into an unresolvable one.
245231
const currentVersion = currentToken.instant ?? currentToken.token;
246232
const expectedVersionText = expected.instant ?? expected.token;
247-
// A row the caller's own read does not return — absent, or the read
248-
// refused — is no conflict this gate may report, and no answer of its
249-
// own either: the comparison is dropped and the write door answers,
250-
// exactly as it answers the same request without a token. The
251-
// method's docblock in `protocol.ts` carries the reasoning.
252-
let visible: unknown;
253-
try {
254-
visible = await this.readAsCaller(object, id, context);
255-
} catch (e) {
256-
if (!isCallerReadRefusal(e)) throw e;
257-
return;
258-
}
233+
// A row the caller's own read RETURNS NO ROW for is no conflict this
234+
// gate may report, and no answer of its own either: the comparison is
235+
// dropped and the write door answers, as it answers the same request
236+
// without a token. ONLY an absent row: a read that throws — a refusal
237+
// or a fault — propagates as raised. The method's docblock in
238+
// `protocol.ts` carries the reasoning.
239+
const visible = await this.readAsCaller(object, id, context);
259240
if (!visible) return;
260241
throw new ConcurrentUpdateError({
261242
currentVersion,

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

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -384,19 +384,23 @@ describe('[#4435] deleteManyData reports per id, not per request', () => {
384384
});
385385
});
386386

387-
describe('a stale version token on a row the caller does not read gets the write door\'s answer, as no token does', () => {
387+
describe('a stale version token on a row absent from the caller\'s read gets the write door\'s answer, as no token does', () => {
388388
// The by-id OCC comparison runs before the write is authorized. When the
389389
// 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).
390+
// RETURNS NO ROW for leaves the answer to the write door: what the same
391+
// request without a token gets, status, code and sentence, which is also
392+
// what a token on an id naming no row gets (ruling of record 5985885287: a
393+
// row the caller cannot read answers what a nonexistent id answers, on
394+
// update and delete alike). Only an absent row is handed over: a caller
395+
// read that THROWS — a refusal or a fault — propagates as raised.
395396
const CURRENT = '2026-10-30T08:00:00.000Z';
396397
const STALE = '2001-01-01T00:00:00.000Z';
397398
const CALLER = { userId: 'u1' };
398399
const MISSING = 'definitely_not_a_row';
399400
const row = (id: string) => ({ id, title: 'x', updated_at: CURRENT });
401+
/** The read door's refusal of a caller without the object's read grant. */
402+
const readRefusalError = () =>
403+
Object.assign(new Error('Reading this object is not permitted.'), { code: 'PERMISSION_DENIED', status: 403 });
400404

401405
/** A refused call as the doors render it: status, code and sentence. */
402406
const answerOf = (run: () => Promise<unknown>) => run().then(
@@ -423,21 +427,31 @@ describe('a stale version token on a row the caller does not read gets the write
423427
expect(store.get('real')).toEqual(row('real'));
424428
});
425429

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-
}
430+
it('the caller\'s read REFUSES and the write is refused too: the read refusal propagates, with the write\'s status and code', async () => {
431+
// A caller holding neither grant. The refused read is the answer, so its
432+
// sentence is the read door's: status and code are compared here, the
433+
// sentence deliberately is not.
434+
const { p, store } = makeProtocol({ real: row('real') }, { refuse: 'u1', readRefusal: readRefusalError });
435+
const stale = await answerOf(() => write(p, 'real', STALE));
436+
const noToken = await answerOf(() => write(p, 'real'));
437+
const missing = await answerOf(() => write(p, MISSING, STALE));
438+
expect(stale).toEqual({ code: 'PERMISSION_DENIED', status: 403, message: readRefusalError().message });
439+
expect({ code: noToken.code, status: noToken.status }).toEqual({ code: stale.code, status: stale.status });
440+
expect({ code: missing.code, status: missing.status }).toEqual({ code: stale.code, status: stale.status });
441+
expect(store.get('real')).toEqual(row('real'));
442+
});
443+
444+
it('the caller\'s read REFUSES and the write door admits the write (a write grant without a read grant): the read refusal propagates and nothing is written', async () => {
445+
const { p, store, update, del } = makeProtocol({ real: row('real') }, { readRefusal: readRefusalError });
446+
await expect(write(p, 'real', STALE)).rejects.toMatchObject({ code: 'PERMISSION_DENIED', status: 403 });
447+
expect(update).not.toHaveBeenCalled();
448+
expect(del).not.toHaveBeenCalled();
449+
expect(store.get('real')).toEqual(row('real'));
450+
// Control: without a token the same double admits the same write, so the
451+
// pin above is not vacuous.
452+
await write(p, 'real');
453+
if (verb === 'updateData') expect(store.get('real')).toMatchObject({ title: 'y' });
454+
else expect(store.has('real')).toBe(false);
441455
});
442456

443457
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 () => {
@@ -451,10 +465,11 @@ describe('a stale version token on a row the caller does not read gets the write
451465
expect(store.get('real')).toEqual(row('real'));
452466
});
453467

454-
it('a read that FAULTS propagates as raised, and the write is never asked', async () => {
468+
it('a read that FAULTS, or throws any other 4xx, propagates as raised, and the write is never asked', async () => {
455469
for (const fault of [
456470
new Error('connection reset'),
457471
Object.assign(new Error('store unavailable'), { code: 'SERVICE_UNAVAILABLE', status: 503 }),
472+
Object.assign(new Error('bad read'), { code: 'VALIDATION_FAILED', statusCode: 400 }),
458473
]) {
459474
const { p, update, del } = makeProtocol({ real: row('real') }, { readRefusal: () => fault });
460475
await expect(write(p, 'real', STALE)).rejects.toBe(fault);

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 31 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3327,9 +3327,10 @@ function simpleHash(str: string): string {
33273327
* and includes both the current server-side version and the current record
33283328
* payload so the client can render an informed conflict-resolution UI
33293329
* ("Reload latest" vs. "Overwrite anyway"). The record is the row as the
3330-
* caller reads it, and a row the caller cannot read is never a conflict: the
3331-
* write door answers it, as it answers the same request without a token
3332-
* (#23586, see `assertVersionOf`).
3330+
* caller reads it, and a row the caller's read returns no row for is never a
3331+
* conflict: the write door answers it, as it answers the same request without
3332+
* a token; a caller read that is refused propagates as that refusal (#23586,
3333+
* see `assertVersionOf`).
33333334
*
33343335
* NOTE: This is an *application-level* compare-and-set — not an atomic
33353336
* storage-layer CAS. There is a small TOCTOU window between the version
@@ -14748,10 +14749,10 @@ export class ObjectStackProtocolImplementation implements
1474814749
//
1474914750
// OCC is asked the way `deleteData` asks it — a probe only when the
1475014751
// caller sent a token ({@link assertVersionMatch}) — and a row that
14751-
// probe does not find, or that the caller's own read does not return,
14752-
// is left to the engine's answer: OCC has never treated a missing
14753-
// record as a concurrency conflict, and a row the caller cannot read
14754-
// answers what a missing one does ({@link assertVersionOf}).
14752+
// probe does not find, or that the caller's own read returns no row
14753+
// for, is left to the engine's answer: OCC has never treated a missing
14754+
// record as a concurrency conflict, and a row absent from the caller's
14755+
// read answers what a missing one does ({@link assertVersionOf}).
1475514756
await this.assertVersionMatch(request.object, request.id, request.expectedVersion, request.context);
1475614757
const opts: any = { where: { id: request.id } };
1475714758
if (request.context !== undefined) opts.context = request.context;
@@ -14989,7 +14990,8 @@ export class ObjectStackProtocolImplementation implements
1498914990
* and the by-id update's existence check, asked only after `engine.update`
1499014991
* has judged and resolved the write (`updateData`, and the bulk faces
1499114992
* through {@link assertRecordExists}) — so an update by id answers the same
14992-
* refusal whether or not the id exists, as delete does, token or none.
14993+
* refusal whether or not the id exists, as delete does. With a mismatched
14994+
* token, a caller whose read of the row is refused gets that read refusal.
1499314995
*/
1499414996
private async probeRecord(object: string, id: string): Promise<any> {
1499514997
return this.engine.findOne(object, { where: { id }, context: { isSystem: true } } as any);
@@ -15096,32 +15098,29 @@ export class ObjectStackProtocolImplementation implements
1509615098
* the caller's own read is. `currentVersion` stays the token this
1509715099
* comparison ran on. A caller who reads the row but may not write it
1509815100
* gets this `409` too: the row it is shown is one it can GET anyway;
15099-
* - no row, or a read that REFUSES (a declared `4xx`) → no answer from
15100-
* this gate at all. The comparison is dropped and the request goes on
15101-
* to the write, whose door answers it: the stale token then gets
15102-
* exactly the answer the same request without a token gets — status,
15103-
* code and sentence — and so does a token on an id that names no row
15104-
* ("Record not found → no check" above), so the two cannot answer
15105-
* apart. That is the governing text for the write doors (ruling of
15106-
* record 5985885287): a row the caller cannot read answers what a
15107-
* nonexistent id answers, on update and delete alike.
15108-
*
15109-
* Why the write door's answer and not one of this gate's own: this gate
15110-
* runs BEFORE the write is authorized, so any answer it gives for a row the
15111-
* caller cannot read — a `404`, or the read door's refusal rethrown —
15112-
* differs from the write door's for every caller the write door refuses
15113-
* first (no update or delete grant, the delegated-administration gate on
15114-
* the RBAC tables), and from what an id naming no row gets. Handing over
15115-
* loses no guard: the write door refuses a by-id write of a row the
15116-
* caller cannot read (the security layer asks the read question ahead of
15117-
* the write and answers `recordNotFoundError`), and a token that could not
15118-
* be compared never lets a write land that the same request without a
15119-
* token would not.
15101+
* - no row — the read RETURNS none → no answer from this gate at all.
15102+
* The comparison is dropped and the request goes on to the write,
15103+
* whose door answers it: the stale token then gets exactly the answer
15104+
* the same request without a token gets, and so does a token on an id
15105+
* that names no row ("Record not found → no check" above). That is the
15106+
* governing text for the write doors (ruling of record 5985885287): a
15107+
* row the caller cannot read answers what a nonexistent id answers, on
15108+
* update and delete alike;
15109+
* - a read that THROWS — a refusal (the caller's read grant withheld) or
15110+
* a fault — propagates as raised, exactly as it did before the
15111+
* absent-row handover existed. A refused read is not an absent row,
15112+
* and only an absent row is handed over.
15113+
*
15114+
* Why the write door's answer for an absent row, and not one of this
15115+
* gate's own: this gate runs BEFORE the write is authorized, so a `404` of
15116+
* its own differs from the write door's answer for every caller the write
15117+
* door refuses first (no update or delete grant), and from what an id
15118+
* naming no row gets. For an absent row the write door holds the guard
15119+
* itself: it asks the caller's read question ahead of a by-id write, and a
15120+
* row that read does not return is answered `recordNotFoundError`.
1512015121
*
1512115122
* The read is issued only on a mismatch: a request whose token agrees, or
15122-
* that sends none, reads exactly what it read before. A read that FAULTS —
15123-
* a server-band status, or none declared at all — propagates as raised: a
15124-
* store fault is no verdict about the caller.
15123+
* that sends none, reads exactly what it read before.
1512515124
*/
1512615125
private assertVersionOf(
1512715126
object: string,

0 commit comments

Comments
 (0)