Uh oh!
There was an error while loading. Please reload this page.
fix(objectql): the read-only strip stops calling the addressed row's own id a forged caller write (#8141) - #8216
Conversation
…own id a forged caller write (#8141) WIP scaffold — implementation in, tests and changeset to follow. `stripReadonlyFields` gains an `addressKey` option: the named key is still stripped, it just no longer logs. The by-id update branch passes the SAME `idAddressesThisRow` predicate #8093 wired to the report channel, so log and report cannot disagree about what an address is. Every other call site passes nothing and is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014C8pAprWdmtecFsEprZax4
… must not silence (#8141) Engine-level acceptance pair on a real ObjectQL + recording driver/logger: the card's measured repro (by-id PATCH, body carrying no `id`) emits no WARN and the driver's SET clause is unchanged; a really-forged read-only field in the same write still WARNs byte-identically against the exported message, at `warn`. Plus the multi branch, the ruled-non-id `primary_key` diagnostic, a non-readonly `id`, and the `isSystem` exemption, all unmoved. Helper-level cases cover `addressKey` opt-in-ness (no option ⇒ the line still prints, which is what keeps the other two call sites unchanged), composition with `preserveAudit`, and the runtime-owned message. Changeset: patch, this changes observable log behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014C8pAprWdmtecFsEprZax4
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 15 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
⛔ merge queue 构建失败 — 先分诊，再决定要不要重排队列构建 31643011741 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集）， 失败的 job（日志抽取，best effort）：
历史信号：
分诊清单：
Generated by Claude Code · merge-queue-triage workflow (#4859) |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#8141
The read-only strip logged, on every single-record update of every platform object, that the addressed row's own primary key was a caller forgery:
All three claims are false for that write. The REST ingress folds the path id into the body (#6479) before the engine's caller snapshot, so the value was never caller-supplied; nothing the caller wanted was dropped; nothing it asked for was left out of the commit. #8093 fixed this in the report channel (
droppedFields/onFieldsDropped) and deliberately left the strip and its log alone. This is the log half.The change
stripReadonlyFieldsgainsoptions.addressKey— the key that carries the write's address. That key is still stripped; it just no longer logs. The by-id update branch passes it from the sameidAddressesThisRowpredicate #8093 already wired to the report channel, so the two channels cannot disagree about what an address is (one predicate, two consumers — no second derivation).Ruled scope, honoured: option A only. Not "stop stripping
id" (a same-value primary-key write is a no-op on SQL but an outright rejection on stores with immutable primary keys); not option B (removing the address before the read-only pass changes what the driver receives for objects whoseidis not readonly — #6435's explicitly separate decision).The acceptance pair, predicted then measured
Reverse verification: drop the
addressKeyargument at theengine.tscall site, leaving everything else — including #8093's report exclusion — in place.id)idlinereadonlyStripWarning('id', 'pref')update(obj, { id, ...fields })readonlyfielddata.id(primary_keystrip)idis not readonlyisSystemcaller3 failed / 6 passedin the new file,0 failedeverywhere else — the prediction held exactly. The direction did not invert the way #8093's strict rows did: this option feeds no counter and no derived refusal.strictReadonlyWritesis keyed onreportDroppedFields, which already excluded the address, so the loud half cannot move when only the log line does.With the fix in place, the tripwire is asserted positively: a forged
readonlyfield still warns, compared withtoBeagainst the exportedreadonlyStripWarning(so a reworded line fails) and with the emitted level asserted (so a downgrade fails).The message wording and its remedy
Checked as asked, and not changed — the acceptance bar pins it unchanged for the cases that still print, and it holds up:
supplied-snapshot own-property plusObject.isguards make that exact);{ context: { isSystem: true } }remedy becomes correct by construction once the line prints only for callers that really did write a read-only column — the card's complaint was that following it for the address write would exempt that caller from the strip entirely, and the address write no longer prints.Two remaining wording defects were found, measured, and filed rather than folded in, because fixing either changes the text this PR's acceptance bar pins as unchanged:
readonlyStripWarningpromises a commit that strict mode refuses, and omits thepreserveAuditremedy its own strip implements #8214 — "COMMITTED WITHOUT IT" is false understrictReadonlyWrites: measuredERR_READONLY_FIELD_REJECTED, 0 driver writes, 1 warn line claiming the commit. Also: the message namesisSystembut neverpreserveAudit, while its runtime-owned twin names both (and has a test pinning that it does).preserveAuditby-id update hands the primary key to the driver:isPreservableUnderAuditreinstates a platform object's ownid#8215 —isPreservableUnderAuditreturns true for any non-systemfield, so apreserveAuditby-id update keeps a platform object'sidand handsSET id = ... WHERE id = ...to the driver. Pinned as observed behaviour in the new unit case; it is pre-existing and untouched here.Verification
packages/objectqlfull suite: 193 files / 3415 tests passed (with the fix; the one earlier failure was this PR's own draft expectation, corrected to the measuredpreserveAuditbehaviour).pnpm --filter @objectstack/objectql typecheck— exit 0.node scripts/pm/dispatch-gates.mjsover the changed paths, all exit 0:check:adr-anchors,check:changeset-gate-self-tests,check:cross-package-test-inputs,check:durability-log-level,check:engine-double-contract,check:error-code-casing,check:objectui-changeset,check:stack-collection-maps,check:nul-bytes,check:query-options-erasure,check:type-check-coverage,check-changeset-no-major.mjs,check-engine-split-ratio.mjs. No baseline or ratchet was raised.check-objectui-pin-fresh.mjsexits 1 at this base and is not caused by this branch: it compares.objectui-shaagainst objectuimain, a file this PR does not touch (last bumped by chore: bump the console pin to objectui 6d77acfe3125 (123 commits, not 405) #8137).Changeset:
patchfor@objectstack/objectql— observable log behaviour changes, so this is not askip-changesetPR.Cross-seat: none.
packages/spec/src/contracts/data-engine.tswas not touched; #8093'sWriteObservabilityOptionsinvariant still reads true, because this change moves no report, no refusal and no payload.Generated by Claude Code
Generated by Claude Code