Skip to content

Commit 57b661d

Browse files
committed
fix(metadata-protocol): the by-id OCC comparison defers to the write door when the caller's read returns no row
On a version-token mismatch, updateData/deleteData re-read the row as the caller. When that read returns nothing, 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 answer the same request without a token gets. A row the caller reads still answers 409 CONCURRENT_UPDATE with that row; a read that faults still propagates as raised. Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk Co-authored-by: Claude <noreply@anthropic.com>
1 parent dd6349e commit 57b661d

3 files changed

Lines changed: 73 additions & 21 deletions

File tree

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

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

724+
A version token (`If-Match`, or `expectedVersion`) changes none of these answers. An
725+
update or delete carrying a stale token is answered `409 CONCURRENT_UPDATE` only for a
726+
record the caller can read; for any other id it gets exactly what the same request
727+
without a token gets.
728+
724729
### Delete Record
725730

726731
Delete a record by ID.

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

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import type { ObjectStackProtocolImplementation } from './protocol.js';
5151
import { ConcurrentUpdateError, MalformedVersionTokenError, canonicalVersionInstant } from './protocol.js';
5252
import { recordNotFoundError } from '@objectstack/core';
53+
import { declaredHttpStatus } from '@objectstack/types';
5354
import { collectInternalWriteResponseFields, omitInternalFieldsFromWriteResponse } from './write-response-internal-fields.js';
5455
import { isMissingTableError } from '@objectstack/metadata/errors';
5556
import { canServeApiOperation, resolveSearchFieldResolution } from '@objectstack/spec/data';
@@ -169,6 +170,19 @@ function versionTokensAgree(current: NormalisedVersion, expected: NormalisedVers
169170
return current.token === expected.token;
170171
}
171172

173+
/**
174+
* Did the caller's own read of a row REFUSE, rather than fault? A refusal
175+
* declares a client-band status (`4xx`, in either spelling
176+
* `declaredHttpStatus` reads) — the read door's verdict on this caller, which
177+
* says nothing about the row. A server-band status, or none at all (a bare
178+
* driver `Error`), is a fault, and the OCC comparison lets it propagate as
179+
* raised rather than read it as "the caller reads no row".
180+
*/
181+
function isCallerReadRefusal(e: unknown): boolean {
182+
const status = declaredHttpStatus(e);
183+
return status !== undefined && status < 500;
184+
}
185+
172186
/**
173187
* The `ObjectStackProtocolImplementation` members the functions below read —
174188
* every one of them, and no other; each function's `this` is the `Pick` of the
@@ -227,8 +241,19 @@ export async function assertVersionOf(
227241
// it names, turning a resolvable conflict into an unresolvable one.
228242
const currentVersion = currentToken.instant ?? currentToken.token;
229243
const expectedVersionText = expected.instant ?? expected.token;
230-
const visible = await this.readAsCaller(object, id, context);
231-
if (!visible) throw recordNotFoundError(object, id);
244+
// A row the caller's own read does not return — absent, or the read
245+
// refused — is no conflict this gate may report, and no answer of its
246+
// own either: the comparison is dropped and the write door answers,
247+
// exactly as it answers the same request without a token. The
248+
// method's docblock in `protocol.ts` carries the reasoning.
249+
let visible: unknown;
250+
try {
251+
visible = await this.readAsCaller(object, id, context);
252+
} catch (e) {
253+
if (!isCallerReadRefusal(e)) throw e;
254+
return;
255+
}
256+
if (!visible) return;
232257
throw new ConcurrentUpdateError({
233258
currentVersion,
234259
currentRecord: visible,

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

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3199,8 +3199,9 @@ function simpleHash(str: string): string {
31993199
* and includes both the current server-side version and the current record
32003200
* payload so the client can render an informed conflict-resolution UI
32013201
* ("Reload latest" vs. "Overwrite anyway"). The record is the row as the
3202-
* caller reads it, and a row the caller cannot read is never a conflict: it
3203-
* answers not-found instead (#23586, see `assertVersionOf`).
3202+
* caller reads it, and a row the caller cannot read is never a conflict: the
3203+
* write door answers it, as it answers the same request without a token
3204+
* (#23586, see `assertVersionOf`).
32043205
*
32053206
* NOTE: This is an *application-level* compare-and-set — not an atomic
32063207
* storage-layer CAS. There is a small TOCTOU window between the version
@@ -14444,8 +14445,10 @@ export class ObjectStackProtocolImplementation implements
1444414445
//
1444514446
// OCC is asked the way `deleteData` asks it — a probe only when the
1444614447
// caller sent a token ({@link assertVersionMatch}) — and a row that
14447-
// probe does not find is left to the engine's answer: OCC has never
14448-
// treated a missing record as a concurrency conflict.
14448+
// probe does not find, or that the caller's own read does not return,
14449+
// is left to the engine's answer: OCC has never treated a missing
14450+
// record as a concurrency conflict, and a row the caller cannot read
14451+
// answers what a missing one does ({@link assertVersionOf}).
1444914452
await this.assertVersionMatch(request.object, request.id, request.expectedVersion, request.context);
1445014453
const opts: any = { where: { id: request.id } };
1445114454
if (request.context !== undefined) opts.context = request.context;
@@ -14663,13 +14666,14 @@ export class ObjectStackProtocolImplementation implements
1466314666
* where it was. The one behaviour this adds is the 404 the issue asked for:
1466414667
* an id that names no row at all.
1466514668
*
14666-
* ⛔ And it is never asked IN FRONT of the write's authorization. Its
14667-
* readers are OCC's `updated_at` comparison, asked only when the caller sent
14668-
* a token ({@link assertVersionMatch}), and the by-id update's existence
14669-
* check, asked only after `engine.update` has judged and resolved the write
14670-
* (`updateData`, and the bulk faces through {@link assertRecordExists}) —
14671-
* so an update by id answers the same refusal whether or not the id
14672-
* exists, as delete does.
14669+
* ⛔ And what it finds never answers IN FRONT of the write's
14670+
* authorization. Its readers are OCC's `updated_at` comparison, asked only
14671+
* when the caller sent a token ({@link assertVersionMatch}) and answering
14672+
* only for a row the caller's own read returns ({@link assertVersionOf}),
14673+
* and the by-id update's existence check, asked only after `engine.update`
14674+
* has judged and resolved the write (`updateData`, and the bulk faces
14675+
* through {@link assertRecordExists}) — so an update by id answers the same
14676+
* refusal whether or not the id exists, as delete does, token or none.
1467314677
*/
1467414678
private async probeRecord(object: string, id: string): Promise<any> {
1467514679
return this.engine.findOne(object, { where: { id }, context: { isSystem: true } } as any);
@@ -14772,18 +14776,36 @@ export class ObjectStackProtocolImplementation implements
1477214776
* from it. It re-reads the row under the caller's own `context`, the read
1477314777
* door's question (every RLS and FLS rule applies):
1477414778
*
14775-
* - no row → `recordNotFoundError`, the producer the security layer's
14776-
* by-id write gate answers a row the caller cannot read with. The stale
14777-
* token then gets exactly the answer the same request without a token
14778-
* gets: no `currentVersion`, no `currentRecord`, so the `409` no longer
14779-
* tells a caller that a hidden row exists or when it was last written;
1478014779
* - a row → the `409`, with THAT row as `currentRecord`, field-masked as
1478114780
* the caller's own read is. `currentVersion` stays the token this
14782-
* comparison ran on.
14781+
* comparison ran on. A caller who reads the row but may not write it
14782+
* gets this `409` too: the row it is shown is one it can GET anyway;
14783+
* - no row, or a read that REFUSES (a declared `4xx`) → no answer from
14784+
* this gate at all. The comparison is dropped and the request goes on
14785+
* to the write, whose door answers it: the stale token then gets
14786+
* exactly the answer the same request without a token gets — status,
14787+
* code and sentence — and so does a token on an id that names no row
14788+
* ("Record not found → no check" above), so the two cannot answer
14789+
* apart. That is the governing text for the write doors (ruling of
14790+
* record 5985885287): a row the caller cannot read answers what a
14791+
* nonexistent id answers, on update and delete alike.
14792+
*
14793+
* Why the write door's answer and not one of this gate's own: this gate
14794+
* runs BEFORE the write is authorized, so any answer it gives for a row the
14795+
* caller cannot read — a `404`, or the read door's refusal rethrown —
14796+
* differs from the write door's for every caller the write door refuses
14797+
* first (no update or delete grant, the delegated-administration gate on
14798+
* the RBAC tables), and from what an id naming no row gets. Handing over
14799+
* loses no guard: the write door refuses a by-id write of a row the
14800+
* caller cannot read (the security layer asks the read question ahead of
14801+
* the write and answers `recordNotFoundError`), and a token that could not
14802+
* be compared never lets a write land that the same request without a
14803+
* token would not.
1478314804
*
1478414805
* The read is issued only on a mismatch: a request whose token agrees, or
14785-
* that sends none, reads exactly what it read before. A read that throws
14786-
* (the caller's read grant withheld, a store fault) propagates as raised.
14806+
* that sends none, reads exactly what it read before. A read that FAULTS —
14807+
* a server-band status, or none declared at all — propagates as raised: a
14808+
* store fault is no verdict about the caller.
1478714809
*/
1478814810
private assertVersionOf(
1478914811
object: string,

0 commit comments

Comments
 (0)