Found while implementing #4630 (the symmetric record-level gates for sys_comment). Filed unassigned — the fix belongs to service-storage, not to that PR.
What
packages/services/service-storage/src/attachment-access-hooks.ts, the beforeDelete gate, resolves the rows a delete matches in two ways and then short-circuits when it found none:
const ids = asIdList(ctx?.input?.id);
if (ids) { /* resolve each by id */ }
else if (ctx?.input?.options?.where) { /* resolve the match set, bounded */ }
if (!rows.length) return; // nothing matched — nothing to authorizeA delete with no id and no where takes neither branch, so rows stays empty and the gate returns allow. That is not "nothing matched" — nothing was ever queried.
The engine then treats the same call as a bulk delete over everything (packages/objectql/src/engine.ts, the delete path):
if (!id) {
opCtx.ast = { object, ...(options?.where !== undefined ? { where: options.where } : {}) };
}
…
} else if (options?.multi && driver.deleteMany) {
const ast = opCtx.ast; // { object } — no `where`
result = await driver.deleteMany(object, ast, …);
}So ql.delete('sys_attachment', { multi: true, context: <a real caller's context> }) reaches deleteMany with an unscoped AST.
Why the other layers do not catch it
- plugin-sharing —
buildWriteFilter returns null for an object with no owner field, and sys_attachment's provenance column is uploaded_by, not owner_id, so no row-scoping predicate is composed onto the AST.
- plugin-security — the
member_default baseline carries no allowDelete (ADR-0090 D5), so a rank-and-file member is refused by RBAC first. But an app that ships a domain grant with the delete bit on sys_attachment — which is exactly what the attachments panel requires, and what attachmentsFixture's att_attachment_manager models — passes RBAC and lands on the ungated path.
Scope
I have not checked whether the REST layer can produce this shape (its delete routes look id-bound); the reachable surfaces are the SDK / ObjectQL / flow delete_record-style callers that pass multi: true without a predicate.
Suggested fix
Fail closed instead of falling through: when there is neither an id nor a where, refuse rather than returning. #4630's sys_comment gate does this — see resolveTargetRows in packages/plugins/plugin-audit/src/comment-access-hooks.ts:
forbid(`Refusing an unscoped multi-${verb} of comments — scope the write to the rows you mean`);
sys_attachment should get the same posture (403 ATTACHMENT_DELETE_DENIED), plus a unit test alongside the existing attachment-access-hooks.test.ts cases. Worth a look at whether any other beforeDelete gate in the repo shares the "no rows resolved ⇒ allow" shape.
Found while implementing #4630 (the symmetric record-level gates for
sys_comment). Filed unassigned — the fix belongs toservice-storage, not to that PR.What
packages/services/service-storage/src/attachment-access-hooks.ts, thebeforeDeletegate, resolves the rows a delete matches in two ways and then short-circuits when it found none:A delete with no id and no
wheretakes neither branch, sorowsstays empty and the gate returns allow. That is not "nothing matched" — nothing was ever queried.The engine then treats the same call as a bulk delete over everything (
packages/objectql/src/engine.ts, thedeletepath):So
ql.delete('sys_attachment', { multi: true, context: <a real caller's context> })reachesdeleteManywith an unscoped AST.Why the other layers do not catch it
buildWriteFilterreturnsnullfor an object with no owner field, andsys_attachment's provenance column isuploaded_by, notowner_id, so no row-scoping predicate is composed onto the AST.member_defaultbaseline carries noallowDelete(ADR-0090 D5), so a rank-and-file member is refused by RBAC first. But an app that ships a domain grant with the delete bit onsys_attachment— which is exactly what the attachments panel requires, and whatattachmentsFixture'satt_attachment_managermodels — passes RBAC and lands on the ungated path.Scope
I have not checked whether the REST layer can produce this shape (its delete routes look id-bound); the reachable surfaces are the SDK / ObjectQL / flow
delete_record-style callers that passmulti: truewithout a predicate.Suggested fix
Fail closed instead of falling through: when there is neither an id nor a
where, refuse rather than returning. #4630'ssys_commentgate does this — seeresolveTargetRowsinpackages/plugins/plugin-audit/src/comment-access-hooks.ts:sys_attachmentshould get the same posture (403ATTACHMENT_DELETE_DENIED), plus a unit test alongside the existingattachment-access-hooks.test.tscases. Worth a look at whether any otherbeforeDeletegate in the repo shares the "no rows resolved ⇒ allow" shape.