Found while implementing #9974 (extending the unscoped-multi-write dispatch to beforeUpdate). Filed rather than fixed: #9974's scope fence sends packages/ findings to their own card.
What is there and what is not
installAttachmentAccessHooks (packages/services/service-storage/src/attachment-access-hooks.ts) registers exactly two guards:
| line | event | rule |
|---|
| 157 | beforeInsert | caller must READ the parent record; uploaded_by server-stamped |
| 206 | beforeDelete | caller must be the uploader OR hold edit on the parent |
There is no beforeUpdate registration, here or anywhere else. Verified structurally over the whole tree:
grep -rn "sys_attachment" --include=*.ts packages/ | grep -v /dist/ | grep -v '\.test\.ts' | grep -iE "beforeUpdate|registerHook"
returns only the beforeInsert capability hook in audit-writers.ts:1617 and the delete-side registration.
Why this looks like a gap rather than a decision
The module's own header states the threat it exists to close — "the default member permission sets grant wildcard CRUD with no row scoping — without these hooks any member can attach files to records they cannot see and delete any other user's attachments" — and then never mentions update.
sys_comment's kit is explicitly derived from this one (comment-access-hooks.ts: "This is the sys_attachment kit … applied to the comment thread"), and it does gate beforeUpdate with the author-or-parent-editor rule, with a stated rationale: "Rewriting or removing someone else's words is moderation." The derived module guards a verb its source does not.
So on today's code a member with the default wildcard CRUD can update any sys_attachment row — including re-pointing parent_id at a record they cannot read, or rewriting uploaded_by — with no record-level check. An unscoped multi: true update of sys_attachment is likewise ungated (and is not affected by #9974: that card's refusal only reaches registrations that declare the flag, and sys_attachment declares it on beforeDelete only — deliberately, since #4757 declares a delete-only refusal).
Not assumed, worth confirming during triage
Whether some other layer already covers this — a readonly field posture on the object definition, a storage-service seam that owns all attachment mutation, or an intentional decision that attachment rows are only ever written by the lifecycle. I did not find one; a triage read should confirm before this is treated as a defect.
Suggested shape if it is one
The uploader-or-parent-editor rule the delete guard already implements, applied to beforeUpdate, plus the insert rule on a parent_id re-point — exactly the shape comment-access-hooks.ts uses for a thread re-point. Whether the unscoped-shape refusal should also be declared on that registration (dispatchUnscopedMultiWrite, now valid on both write verbs after #9974) is part of the same decision.
Backlinks: #9974 (the dispatch this was found under), #4757 (the delete-only unscoped refusal), #4630 (the comment kit that gates all three verbs), #2755 (the attachment kit's own card).
Generated by Claude Code
Found while implementing #9974 (extending the unscoped-multi-write dispatch to
beforeUpdate). Filed rather than fixed: #9974's scope fence sendspackages/findings to their own card.What is there and what is not
installAttachmentAccessHooks(packages/services/service-storage/src/attachment-access-hooks.ts) registers exactly two guards:beforeInsertuploaded_byserver-stampedbeforeDeleteThere is no
beforeUpdateregistration, here or anywhere else. Verified structurally over the whole tree:returns only the
beforeInsertcapability hook inaudit-writers.ts:1617and the delete-side registration.Why this looks like a gap rather than a decision
The module's own header states the threat it exists to close — "the default member permission sets grant wildcard CRUD with no row scoping — without these hooks any member can attach files to records they cannot see and delete any other user's attachments" — and then never mentions update.
sys_comment's kit is explicitly derived from this one (comment-access-hooks.ts: "This is the sys_attachment kit … applied to the comment thread"), and it does gatebeforeUpdatewith the author-or-parent-editor rule, with a stated rationale: "Rewriting or removing someone else's words is moderation." The derived module guards a verb its source does not.So on today's code a member with the default wildcard CRUD can
updateanysys_attachmentrow — including re-pointingparent_idat a record they cannot read, or rewritinguploaded_by— with no record-level check. An unscopedmulti: trueupdate ofsys_attachmentis likewise ungated (and is not affected by #9974: that card's refusal only reaches registrations that declare the flag, andsys_attachmentdeclares it onbeforeDeleteonly — deliberately, since #4757 declares a delete-only refusal).Not assumed, worth confirming during triage
Whether some other layer already covers this — a
readonlyfield posture on the object definition, a storage-service seam that owns all attachment mutation, or an intentional decision that attachment rows are only ever written by the lifecycle. I did not find one; a triage read should confirm before this is treated as a defect.Suggested shape if it is one
The uploader-or-parent-editor rule the delete guard already implements, applied to
beforeUpdate, plus the insert rule on aparent_idre-point — exactly the shapecomment-access-hooks.tsuses for a thread re-point. Whether the unscoped-shape refusal should also be declared on that registration (dispatchUnscopedMultiWrite, now valid on both write verbs after #9974) is part of the same decision.Backlinks: #9974 (the dispatch this was found under), #4757 (the delete-only unscoped refusal), #4630 (the comment kit that gates all three verbs), #2755 (the attachment kit's own card).
Generated by Claude Code