You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit f95d0db
Browse filesBrowse the repository at this point in the historyBrowse files
fix(metadata-protocol): the by-id OCC comparison answers only for a row the caller reads (#28731)
domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk
Clause-②: no
Metadata-protocol: none
## 维护者速读
- 改了什么:按 id 更新/删除时的版本令牌(If-Match / expectedVersion)比对,只对调用者读得到的记录给出
409;调用者回读"返回无行"时,比对不再自己作答,交给写入门,得到与不带令牌时相同的答复。回读抛出(被拒或故障)一律原样抛出,与本 PR
之前相同。
- 为什么改:QA 第三轮发现的安全加固(细节按披露裁决不公开)
- 风险与回滚:风险低。只改比对分支在"回读返回无行"时的出口;读得到的行仍是 409 带
currentRecord(含能读不能写的行);回读被拒或故障仍原样抛出。回滚 = revert 本 PR。
-
先看:`packages/metadata-protocol/src/protocol-record-reads.ts`(assertVersionOf
的无行交接);`packages/metadata-protocol/src/protocol.record-not-found.test.ts`(有写无读一类的钉子);`packages/runtime/src/domains/data-conflict-caller-visibility.test.ts`(同一类在真实安全层两扇门上的钉子)
- 两分钟验证:`pnpm --filter @objectstack/metadata-protocol exec vitest run
src/protocol.record-not-found.test.ts`,预期 30 个用例全绿。
- 需要你做的:无,席位落地
## Route (i)
Contract review 6561342764 asked for a ruling between two routes; the
maintainer ruled route (i): 「#28731 同意你的推荐」. Only a row the caller's
re-read **returns no row for** is handed to the write door; a re-read
that throws — a refusal or a fault — propagates as raised, as before
this PR.
## What changes
- **`assertVersionOf`**
(`packages/metadata-protocol/src/protocol-record-reads.ts`, the
comparison half of the by-id OCC gate that `updateData` and `deleteData`
run before the write is authorized). On a token mismatch it re-reads the
row as the caller, as before:
- the read returns no row → the comparison gives no answer of its own;
the request goes on to `engine.update` / `engine.delete`, whose door
answers it — the answer the same request gets without a token, and the
answer a token on an id naming no row gets;
- the read returns a row → unchanged: `409 CONCURRENT_UPDATE` with that
row as `currentRecord`, field-masked as the caller reads it, including a
row the caller reads but may not write (option a′ ruled out);
- the read throws → unchanged: propagates as raised.
- **Governing text restored for the absent row:** ruling of record
5985885287 ("a row the caller cannot read answers exactly what a
nonexistent id answers, on update and delete alike"), and the
`assertVersionOf` docblock's own "the stale token then gets exactly the
answer the same request without a token gets".
- **Docblocks** in `protocol.ts` (`ConcurrentUpdateError`, `updateData`,
`probeRecord`, `assertVersionOf`) and the record-reads module header
describe the absent-row handover only, and say a thrown read propagates.
The changeset does the same. **Docs:** one paragraph in the
http-protocol page's refusal section: a stale token is answered `409`
only for a record the caller can read; for a record absent from the
caller's read, and for an id naming none, it changes none of the
answers; a caller refused reading the object gets that read refusal.
- **Bulk faces checked, nothing to align:** `updateManyData` and
`batchData`'s `update` arm carry no version token.
- **Clause-② is `no`, patch:** a refusal swap on existing doors, the
same classification as #28615. **Metadata-protocol: none.** Changeset:
`@objectstack/metadata-protocol` patch.
## Tests
- **Protocol** (`protocol.record-not-found.test.ts`, its existing pinned
engine double, caller visibility added; no ledger change), per verb: a
hidden row with the write refused; a hidden row with the write admitted
up to the write door's read question; a read refused while the write is
refused too (status and code compared, see below); **a principal with a
write grant and no read grant — the read refuses, the write door would
admit the write: the stale token propagates the read refusal, the write
is never asked and the row is unchanged, while the same double admits
the no-token write (the control)**; a read that faults or throws any
other 4xx propagates; the readable-row `409` control.
- **Runtime, real security layer**
(`data-conflict-caller-visibility.test.ts`), per door (RestServer,
dispatcher) and verb: an object the caller reads but may not write, with
an RLS-hidden row (stale token = the control's `403` body; unknown id
the same; a readable row still `409`); **an object the caller may update
and delete but not read, with no row rule: a stale token answers `403
PERMISSION_DENIED` and the row is unchanged, on update and on delete;
the no-token request lands (`200`), so the class is real.**
- **Showcase, single install**
(`update-by-id-refusal-order.dogfood.test.ts`): with the token in
`If-Match`, the PATCH body's `expectedVersion` and the DELETE's
`?expectedVersion`, the two private-OWD rows the member's row scope
hides compare status, code **and sentence** for the row, for no row and
without a token. On the two objects the member may neither read nor
write, its read is refused and propagates, so those families compare
**status and code only** and deliberately do not compare the sentence:
the read door's refusal and the write door's may word an
operation-naming refusal differently. That sentence-level residue is
accepted and handled by a separate D12 sentence-alignment item. Controls
as before (edit-grant holder `409` carrying the row and `200` with the
current token; the plain member's `409` on a row it reads but may not
write).
- **Showcase behind the organization wall**
(`occ-stale-token-organization-wall.dogfood.test.ts`): unchanged;
another organization's row is absent from the member's read, so every
carrier answers what no token answers.
- **Measured set for the showcase:** the four dogfood files that send a
version token (declared narrowing, as before: the changed branch is
reachable only through a by-id write carrying a non-empty token). The
write-grant-without-read-grant class has no showcase principal; it is
pinned at the protocol double and at the runtime's two real doors.
## Acceptance notes
- **Accepted consequence (per ruling 5985885287):** for a caller the
write door refuses first, a mismatched token on a row absent from its
read now gets that refusal, as without a token; a caller who may write
the object still gets `404 RECORD_NOT_FOUND`, now produced by the write
door.
- **Accepted residue (route (i)):** a caller with neither the read nor
the write grant gets the read door's refusal on an existing row and the
write door's on an id naming none — the same status and code (`403
PERMISSION_DENIED`); an operation-naming sentence may differ. Handled by
a separate D12 sentence-alignment item; the pin for that case asserts
status and code only.
- **Observation, not filed (carrier: none):** `@objectstack/rest`'s
built `dist/index.js` carries bundled `metadata-protocol` source, so the
changed bytes ship in rest's tarball too; rest's own non-test source
neither imports that package nor constructs the protocol class.
## Verification
Head `8372e0a15` (route (i), on top of a merge of `origin/main` at
`c2a5be03b`, merge commit `357b2bac2`). Exit codes captured before any
pipe; one locked battery, `os-verify-lock: VERDICT command-exit 0` (held
5078s, shared box).
<details>
<summary>Build, pins, full suites and typechecks — all green</summary>
- `pnpm --workspace-concurrency=2 --filter '@objectstack/dogfood^...'
--filter '@objectstack/runtime^...' --filter '@objectstack/runtime'
build` — exit 0
- `pnpm --filter @objectstack/metadata-protocol exec vitest run
--maxWorkers=1 src/protocol.record-not-found.test.ts` — Tests 30 passed
(30), exit 0
- `pnpm --filter @objectstack/runtime exec vitest run --project local
--maxWorkers=1 src/domains/data-conflict-caller-visibility.test.ts` —
Tests 24 passed (24), exit 0
- `pnpm --filter @objectstack/dogfood exec vitest run --maxWorkers=1`
over the four token-carrying files (`update-by-id-refusal-order`,
`occ-stale-token-organization-wall`, `data-request-shape-live`,
`object-designer-field-reorder`) — Test Files 4 passed (4), Tests 39
passed (39), exit 0
- metadata-protocol `vitest run --maxWorkers=1` — Test Files 301 passed,
4 skipped (305); Tests 22139 passed, 26 skipped (22165); exit 0.
`typecheck` exit 0.
- runtime `vitest run --project local --maxWorkers=1` — Test Files 515
passed (515); Tests 8343 passed, 19 skipped (8362); exit 0. `typecheck`
exit 0.
- dogfood `typecheck` — exit 0
</details>
<details>
<summary>One-shot ablation of the reviewed head's refusal hand-over,
restore proven</summary>
Mutation through `scripts/ablation-replace.mjs` (wrap mode) plus an
EXIT/INT/TERM trap on the absolute path: the reviewed head's catch
re-instated (a declared client-band read refusal handed to the write
door). A first attempt inside the battery was void — its tree reading
was given the dist spelling of the marker, refused with exit 3, and no
test ran under the mutation; it is reported here, not counted. The valid
run (`VERDICT command-exit 0`, held 169s):
- landed: anchor 1 to 0, blob `78150561999f` to `aa7a51f44c28`; injected
text 1, removed text 0 on disk; rebuild exit 0; `ablation-dist-preflight
--source-marker`: marker present in 2 built files
- protocol pins: 6 failed, 24 passed — per verb, the
write-grant-without-read-grant case, the both-refused case, and the
any-other-4xx propagation case
- runtime pins: 4 failed, 20 passed — the write-only object, both doors,
both verbs
- showcase pins: 21 passed — expected: no showcase principal is in the
class, and the refused-read families compare status and code, which both
versions share
Restore: `git checkout HEAD --` on the absolute path; blob equals the
HEAD blob `78150561999f`, `git diff HEAD` empty, working tree clean;
rebuild exit 0; `ablation-dist-preflight --absent --source-marker`:
marker absent. Restored runs: 30/30, 24/24, 21/21.
</details>
<details>
<summary>Derived gate families — 92 derived, 90 exit 0, 2 NOT
MEASURED</summary>
`node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
--commands` at `8372e0a15` (8 paths vs merge base `c2a5be03b`): 92
commands, each run under the lock and recorded with its exit code;
`--ran`: "92 derived famil(ies) accounted for — 90 run, 2 NOT-MEASURED
(2 DERIVED from a recorded exit 3)". NOT MEASURED (PREREQUISITE NOT MET,
packages outside this build closure unbuilt): `pnpm --filter
@objectstack/spec run check:skill-examples`, `pnpm
check:dual-build-cjs-loads`. `pnpm check:type-check-debt` ran this time,
exit 0.
CI at `8372e0a15`: Lint & Repo Gates, TypeScript Type Check, Test Core,
Build Core, Governed Surface Queue Guard, Test UI, Live E2E UI, Check
Changeset — success; Lint UI, Type Check UI, Build & E2E UI — skipped.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
fix(metadata-protocol): the by-id OCC comparison hands a row absent from the caller's read to the write door
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, 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).
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 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.
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});
0 commit comments