fix(service-storage): refuse a predicate update that writes a file field (#7102) - #7224
Merged
Merged
Conversation
…eld (#7102) A predicate (`multi: true`) update has ONE payload for N matched rows, so a file id written through one landed in every matched record while at most one of them could own it — read authorisation for those bytes then derived from a record the others have nothing to do with, which is the exact widening the exclusive-ownership design exists to prevent. Two log warnings were the only signal and nothing failed. That write is now refused in `beforeUpdate`, before the driver runs, with an ADR-0112 envelope error (`FILE_FIELD_BULK_WRITE_REFUSED` / 400). The refusal is scoped to a file id TOKEN reaching a file-class field — decided by `isFileIdToken`, the same arbiter copy-on-claim already uses — so a bulk clear, an external URL and a legacy inline blob still work per row, and every single-record path is byte-identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015fkdTyGmMD5s8ZtEifvuGy
…k-file-field-refusal
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 107 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
os-help
marked this pull request as ready for review
August 10, 2026 03:25
This was referenced Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7102
The premise, reproduced before implementing
The issue is a lead, so the three-records-one-file end state was reproduced against
origin/mainfirst, with a per-row multi-UPDATE driver modelled on the engine's real dispatch (dispatchPerRowBeforeHooks+buildPerRowAfterContexts). Three matched records, one payload:One copy resolved once in
beforeUpdate, written by oneSETclause to all three rows, claimed by the first and refused to the other two byclaimFile's never-steal branch. Read authorisation for those bytes then derives from a record the other two have nothing to do with — the widening exclusive ownership exists to prevent. Premise confirmed; the card's quoted line numbers had moved (measured at2d1ddf0, now:433-436and:584after #7101), the facts had not.The change
The lane ruled refuse, and the refusal lands in the
beforeUpdateleg:Both halves of the PM's mechanism assumption were verified against
packages/objectql/src/engine.tsand hold:hookContext.dispatch.modeis bound to'per-row'at theisPredicatePathbranch, before the before phase; the module's existingperRowDispatch()already reads it.dispatchPerRowBeforeHooksis called outsideupdate()'stryblock andtriggerHooksawaits handlers with nocatch, so the error propagates uncaught to the caller. ADR-0058 Addendum II states the same intent in prose: per-rowprevious"is supplied so a guard can REFUSE the write (throw)".The refusal fires on the first row, so the remaining N−1 dispatches and the
updateManynever happen. Nothing is written, nothing is copied, and not even asys_filelookup runs.Scope — deliberately narrow
The condition is a file id token reaching a file-class field, decided by
isFileIdToken— the same single arbiter copy-on-claim and the read resolver already ask, so the refusal covers exactly the writes this module would otherwise have to take ownership for, and nothing else. Three predicate writes own nothing and keep working per row, each pinned by a test:{ image: 'file_a' }{ image: null }{ image: 'https://…' }{ name: 'new' }sys_filecostSingle-record updates, inserts and every delete path are byte-identical. Option 2 (copy per row) was ruled out by the card and not attempted — ADR-0058 Addendum II D3 names a row-conditioned rewrite of the shared
SETclause as out of contract forbefore*hooks.#7101's once-per-writebeforeUpdateshape and its per-rowafterUpdateguard are both left in place. TheafterUpdatecomment that filed this card is updated rather than deleted: what a predicate write can still deliver there is a bulk clear or a non-token value, and reconciling those per row is exactly right.The falsifiable premise held
The ruling hung on "no legitimate in-repo consumer depends on a predicate update writing a file-class field succeeding". Verified and not falsified:
multi: truecall site outsidepackages/objectqlwas read; the only file-adjacent hits aresys_attachmentjoin rows and e-mail attachments, neither of which is a file-class field write;image,gallery,avatar,cover,attachments,profile_pic,resume,doc,f_video,f_audio, …) appear in no bulk-update payload inpackages/,examples/(showcase, CRM, todo),packages/qa/(dogfood, field-zoo, http-conformance) or the docs corpus;@objectstack/service-storageis fully green (334/334) and so is@objectstack/spec(9382/9382).Reverse verification
Direction predicted before running: red. The refusal block was removed (via a scripted string strip, never
git stash) and the whole suite re-run:The five failures are exactly the five refusal pins; each fails as
promise resolved "[ …(3) ]" instead of rejecting, i.e. the write went through and produced the shared id. The four "deliberately not refused" cases and the single-record contrast case stayed green in both directions, which is what shows the refusal is scoped and not a blanket ban. With the block restored, 334/334.Rejection assertions
Every refusal case asserts the error's
codeandstatus, never a barerejects.toThrow(). That is load-bearing here in the way #6050/PR #6142 describes: the pre-fix code does not throw at all on this path — it answers — so a throw-only assertion could not separate "refused with the wrong envelope" from "did not refuse".Surface note — one file outside the declared surface, declared here rather than silently
The card scoped this to the
file-reference-lifecyclepair. Minting a new error code cannot be done inside that surface: ADR-0112 D3 requires registration inERROR_CODE_LEDGER, and the REST layer promotes a thrown error's.codeonto the envelope — so an unregistered code would become a wire code by side effect, which is the "silent fourth state" the ADR exists to abolish. One row was therefore added topackages/spec/src/api/error-code-ledger.zod.ts, with the generatedcontent/docs/references/api/*.mdxrefreshed bypnpm --filter @objectstack/spec gen:docs. Those nine extra.mdxdiffs are mechanical (… +257 more→… +258 more, theErrorCodeunion count) andcheck:docsreports 231 generated files in sync.The alternative — reusing an already-registered code such as
INVALID_REQUEST— is what the ledger's own header warns registration friction pushes authors toward, and it would leave clients unable to branch on a brand-new refusal. Flagging for the PM: this is a registry entry the ruled fix requires, not a consumer patched to dodge the refusal.Verification
All of the above re-run after merging
origin/main(3566e5520), with a clean tree afterwards.tsc --noEmiton the package reports 51 errors in its test layer; all four in this file are pre-existingdriveInsertreturn-type errors in tests this PR does not touch, and this package has nocheck:test-typecheckscript (onlyspecandclientdo), so the count is unchanged by this change.Changeset:
patchfor@objectstack/service-storageand@objectstack/spec, naming the before/after.Generated by Claude Code