Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions .changeset/patch-path-id-wins-over-body-id.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
"@objectstack/metadata-protocol": patch
---

fix(metadata-protocol): a single-record update binds the row the CALLER named, not the row the body names (#6479)

`PATCH /data/:object/:id` decided which row to write **twice, differently**. The
protocol's `updateData` probed existence and validated `If-Match` /
`expectedVersion` against the path `:id`, built `{ where: { id: request.id } }`,
and then handed the request body to the engine verbatim — where the dispatch
reads the payload first, so a truthy scalar `data.id` outranks `where.id`.

So `PATCH /data/task/rec_1` with a body of `{"id":"rec_2","title":"x"}`:

- probed **rec_1** for existence (404 gate, #4435);
- version-checked **rec_1** against the caller's `If-Match`;
- **wrote rec_2**; and
- answered `{ id: "rec_1", record: <rec_2's readback> }` — a receipt whose two
halves name different rows.

rec_2 was never probed and never version-checked, so the most common client
shape there is — GET a record, edit a field, PUT the whole body back — performed
a **silent cross-row write straight past its own optimistic-concurrency check**
whenever the body carried another row's id (a mis-clicked list row, a stale
refresh, a generated client that copied the wrong field).

`updateData` now merges the path id over the payload before dispatch
(`{ ...request.data, id: request.id }`) — the same shape the **bulk** ingress has
always used for this question (`ql.update(op.object, { ...data, id }, …)`), so the
two ingresses give one answer instead of two. The probed row, the OCC-checked
row, the written row and the receipt's `id`/`record` are now the same row: the
one in the URL.

Nothing else moves:

- **The engine is untouched.** ObjectQL's payload-first dispatch (#5748) and its
by-id payload strip (#6435) are unchanged and still correct for a caller who
hands ObjectQL a payload and nothing else; this was a gap at the REST/protocol
ingress, which had already named the row.
- **No new rejection, no request-shape change.** A body `id` equal to the path
id behaves exactly as before, and a differing one is now simply overridden
rather than refused — `UpdateDataRequestSchema` still accepts the same bodies.
- **Non-record payloads pass through untouched** (`undefined`, `null`, an array),
so the engine's own diagnostics for a malformed call still surface unchanged.

Callers that deliberately relied on the body's `id` redirecting a
single-record PATCH must address the intended row in the URL instead — the bulk
endpoint has never honoured a body id either.
50 changes: 49 additions & 1 deletion packages/metadata-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5774,7 +5774,55 @@ export class ObjectStackProtocolImplementation implements
// listener never breaks the write (the engine catches + logs).
const dropped: DroppedFieldsEvent[] = [];
opts.onFieldsDropped = (e: DroppedFieldsEvent) => { dropped.push(e); };
const result = await this.engine.update(request.object, request.data, opts);
// [#6479] At THIS ingress the row is the one the caller named — `request.id`,
// the path `:id` — and nothing in the payload gets to move it.
//
// The engine's dispatch reads the PAYLOAD first: a truthy scalar `data.id`
// outranks `options.where.id` (`engine-update-dispatch.ts`, case *"a SCALAR
// data.id still wins over a scalar where.id"* — `expectId: 'rec_1'`). That
// rule is correct and deliberate for a caller who hands ObjectQL a payload
// and nothing else (#5748 / PR #5919, ruling A); it is a HOLE here, because
// this caller has already named the row twice — in the URL and in `where` —
// and the three gates around this line all judge THAT row:
//
// probe → `probeRecord(object, request.id)` (existence, #4435)
// OCC → `assertVersionOf(…, request.id, …)` (If-Match / expectedVersion)
// receipt → `{ id: request.id, record: result }`
//
// Passing `request.data` verbatim let a body `{"id":"rec_2"}` on
// `PATCH /data/task/rec_1` bind rec_2: probed rec_1, OCC-checked rec_1,
// WROTE rec_2, and answered `id: rec_1` beside rec_2's readback. rec_2 was
// never probed and never version-checked, so a client that GETs a record,
// edits it and PUTs the whole body back — with the wrong row's id picked up
// from a mis-clicked list or a stale refresh — performed a silent cross-row
// write past its own `If-Match`.
//
// The fix is the shape the BULK ingress has always used for the same
// question (`rest-server.ts`, batch `update`: `ql.update(op.object,
// { ...data, id }, …)` — the operation's id after the spread, so it wins).
// Two ingresses, one answer (#4550 / #4434). It changes no engine verdict:
// the call still dispatches `by-id`, on the id `where` already carried.
//
// Deliberately NOT route B (400 on mismatch) or route C (ban `id` in
// `UpdateDataRequestSchema`) — both were rejected by the 2026-08-08 triage
// ruling on #6479; B installs a new rejection on a shipped API and C
// changes the accepted request shape.
//
// A non-record payload is passed through UNTOUCHED (`undefined`, `null`, an
// array): the engine reads `data.id` unguarded on purpose, so `undefined`
// is its `TypeError`, and an ingress that answered a non-record payload
// more kindly than the producer would be the very looseness
// `engine-update-dispatch.ts` exists to prevent. Those shapes carry no
// scalar `id` to outrank `where.id` either, so the invariant holds for them
// through `opts.where` alone.
const writeData = (
request.data !== null
&& typeof request.data === 'object'
&& !Array.isArray(request.data)
)
? { ...(request.data as Record<string, unknown>), id: request.id }
: request.data;
const result = await this.engine.update(request.object, writeData, opts);
return {
object: request.object,
id: request.id,
Expand Down
Loading
Loading