diff --git a/docs/qa/platform-checklist/areas/attachments-storage.json b/docs/qa/platform-checklist/areas/attachments-storage.json index f694612fd0..aa4621d589 100644 --- a/docs/qa/platform-checklist/areas/attachments-storage.json +++ b/docs/qa/platform-checklist/areas/attachments-storage.json @@ -41,7 +41,7 @@ "call": "PUT /api/v1/meta/objects/qa_nofiles?package=com.objectstack.qa.attachments", "body": { "name": "qa_nofiles", "label": "QA No Files", "sharingModel": "public_read", "fields": { "name": { "type": "text", "label": "Name", "required": true } } }, "expect": "2xx. `enable.files` is OMITTED deliberately (it defaults to false) — this is the FILES_DISABLED probe target, purpose-built rather than borrowed from showcase_account, so the negative keeps holding if showcase ever enables files on its own objects.", - "source": "packages/spec/src/data/object.zod.ts:281 — enable.files defaults to false; 'true surfaces the panel and permits attachments targeting this object; otherwise creation is rejected'" + "source": "packages/spec/src/data/object.zod.ts:281 — enable.files defaults to false; 'Opt-in: true surfaces the panel and permits attachments to target this object; otherwise any write that makes an attachment target it is rejected (403 FILES_DISABLED) — a create and an update that re-points an existing attachment alike' (re-quoted post-#10733; the prior citation ended at 'creation is rejected', which #10170 made stale — see attach-requires-parent-edit clause 5)" }, { "step": 5, @@ -340,10 +340,10 @@ "title": "Attaching requires EDIT on the parent record; deleting requires uploader-or-parent-editor; unscoped multi-delete is refused outright", "since": "v15.1", "status": "active", - "revision": 4, + "revision": 5, "priority": "P1", "surface": "api", - "personas": ["member with EDIT on the parent", "member with READ but not EDIT on the parent", "member who uploaded the attachment", "member who is neither uploader nor parent editor"], + "personas": ["member with EDIT on the parent", "member with READ but not EDIT on the parent", "member who uploaded the attachment", "member who is neither uploader nor parent editor", "admin (isolates clause 5's FILES_DISABLED refusal from ATTACHMENT_PARENT_ACCESS by holding EDIT on both the original and target parent)"], "fixtures": { "app": "showcase", "requires": [ @@ -357,13 +357,14 @@ "knownGaps": [ "MARKER (QA run #9401 finding 5, carried here so runs stop re-deriving it): the CROSS-TENANT clauses of this area are structurally unprovable in the OSS workspace. `@objectstack/organizations` is cloud-private, so the dogfood matrix's cross-tenant block is gated behind `describe.skipIf(!organizationsAvailable)` and skips BY DESIGN here. That is blocked(dependency), never a pass and never a defect. A run that is supposed to ship the package sets OS_TEST_MULTI_ORG_ENABLED=1, which turns the skip into a hard failure instead of a silent one.", "clause 3 ('unscoped multi-delete refused outright', #4757) IS PINNED as of 2026-08-23. The product gap this entry used to record (#9719) was fixed by PR #9797 and extended to the update verb by #9974: the engine now offers an opt-in whole-operation dispatch (`dispatchUnscopedMultiWrite`) that fires ONCE, before any row is resolved and zero-match included, and attachment-access-hooks.ts declares it on BOTH sys_attachment write registrations — so the refusal answers with ATTACHMENT_DELETE_DENIED / 403 / the 'Refusing an unscoped multi-delete of attachments' message. ⭐ The durable lesson, which outlives the fix: a fixture whose rows SPLIT entitled/not cannot tell 'refused outright' from 'the per-row gate refused one of the rows' — both answer 403 ATTACHMENT_DELETE_DENIED, so the clause reads green either way. Measured on this suite while re-verifying: with the declaration removed from both registrations and service-storage rebuilt, the original split-fixture block stayed 5/5 GREEN. Score clause 3 ONLY from a caller entitled to EVERY matched row (or from the message text) — never from a split fixture. The REST lane still cannot express the shape at all (DeleteManyDataRequestSchema strips options.where and requires ids, #3897), so the reachable callers are server-side engine holders — flows, actions, scripts, MCP tools — which is why this clause's oracle is `test` rather than `api`.", - "the FILES_DISABLED clause currently probes showcase_account, which is files-disabled only incidentally — if the showcase ever enables files on it, the clause goes green for the wrong reason. The recipe's qa_nofiles exists to be probed instead; prefer it, and treat showcase_account as the fallback." + "the FILES_DISABLED create-side clause (acceptance clause 4) still probes showcase_account, which is files-disabled only incidentally — if the showcase ever enables files on it, that clause goes green for the wrong reason. The recipe's qa_nofiles exists to be probed instead; prefer it there too, and treat showcase_account as the fallback. (The new UPDATE-side clause 5 below already uses qa_nofiles, since it needed a real parent_id to re-point onto.)" ] }, "steps": [ "as the read-only member, POST /api/v1/data/sys_attachment { parent_object, parent_id, file_id, file_name, mime_type, size } against the readable-but-not-editable record and capture the refusal", "as the same member, list that parent's attachments — reading must still work (read inherits parent READ, attach requires parent EDIT: two different gates)", "as a parent editor, POST the same attach payload with a spoofed uploaded_by of another user and read the row back", + "as admin, POST one throwaway qa_nofiles record ({ name: 'qa-nofiles-probe' }) for a real parent_id (qa_nofiles is a scratch OBJECT from the recipe with no standing rows), then PATCH the attachment created in the previous step to re-point it — { parent_object: 'qa_nofiles', parent_id: } — as admin (who can edit both the original parent and the qa_nofiles record it just created, isolating the refusal from ATTACHMENT_PARENT_ACCESS) and capture the refusal. This is the UPDATE half of the #10170 gate (enforceFilesCapability registered on beforeUpdate as well as beforeInsert, packages/plugins/plugin-audit/src/audit-writers.ts:1483-1503): re-read the attachment afterward and confirm parent_object/parent_id are UNCHANGED", "as a member who is neither the uploader nor a parent editor, DELETE the attachment and capture the refusal", "as the uploader, DELETE their own attachment on a parent they cannot edit — the uploader may always detach", "drive a predicate-less multi-delete from an ENGINE HOLDER — ql.delete('sys_attachment', { multi: true, context }) with no id and no where — and capture the refusal. ⛔ Not over REST: DeleteManyDataRequestSchema requires an ids list (#3897), so the HTTP lane cannot express the shape and its schema rejection must never be scored as this refusal" @@ -398,18 +399,28 @@ "oracle": "api", "verify": "an attach targeting e.g. showcase_account (no enable.files) is refused with FILES_DISABLED", "evidence": "the refusal" + }, + { + "clause": "an UPDATE that re-points an existing attachment's parent_object onto an object without enable.files is refused identically — 403 FILES_DISABLED — because #10170 registered the enforceFilesCapability gate on beforeUpdate as well as beforeInsert: enable.files is a property of the TARGET object a write makes an attachment point at, not of the verb that does the pointing, so a create-only probe (clause 4 above) would keep passing while this half regressed", + "oracle": "api", + "verify": "the PATCH re-pointing parent_object to the files-disabled qa_nofiles record answers 403 FILES_DISABLED, and a re-read of the sys_attachment row shows parent_object/parent_id UNCHANGED from before the attempted re-point — same authoritative-rejection rule as the delete clauses above", + "evidence": "the 403 body + the before/after row reads" } ], "negative": [ "a successful attach by the read-only member is a FAIL even if the UI hides the upload control — the hook, not the panel, is the boundary", - "a rejected write that still created the sys_attachment row (verify by re-listing) is a FAIL — the rejection must be authoritative, not cosmetic" + "a rejected write that still created the sys_attachment row (verify by re-listing) is a FAIL — the rejection must be authoritative, not cosmetic", + "a re-point that succeeds in moving an attachment onto a files-disabled object — whether outright or by only half-applying the new parent_object/parent_id pair — is the exact #10170 regression clause 5 exists to catch: FAIL" ], "traps": ["wrong-persona"], - "automated": { "kind": "api", "ref": "packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts (clauses 0, 1, 2 and 4: attach without parent EDIT is 403 ATTACHMENT_PARENT_ACCESS while the same member's LIST still succeeds, uploaded_by is server-stamped over a spoofed value, delete splits uploader-vs-outsider, and FILES_DISABLED) + packages/qa/dogfood/test/attachments-unscoped-delete-gate.dogfood.test.ts, which now carries TWO blocks and pins CLAUSE 3 end to end. Block 1 (split fixture — the caller owns one row and may not touch the other): the predicate-less `{ multi: true }` delete is refused with ATTACHMENT_DELETE_DENIED / 403 / the 'Refusing an unscoped multi-delete of attachments' message and removes NOTHING, not even the row it WAS entitled to, so the refusal is authoritative and cannot land as a partial delete; a match-all `where: {}` reaches the same verdict by a DIFFERENT rule and asserts the PER-ROW message instead, which is what keeps the #9797 boundary honest (the whole-operation dispatch is deliberately scoped to a delete with no `where` at all, so `where: {}` stays a real query); and a predicate that MATCHES NOTHING is allowed through, the live half of the #4757 reasoning. Block 2 (the discriminating fixture — the caller uploaded EVERY row, so the per-row gate has nothing to refuse): the predicate-less delete is still refused, which is the only shape that proves 'refused OUTRIGHT' rather than 'every row happened to be denied'; the refusal also fires on an EMPTY table (the zero-match limb — the per-row dispatch is gated on matched rows, so a handler-only fix could never reach it); and both sides are held by the same caller sweeping the same rows with a SCOPED predicate successfully, plus a scoped `where: {}` over the empty table resolving. In both blocks the member is granted the sys_attachment delete bit first and the grant is asserted, because otherwise RBAC refuses ahead of the attachment hook and the file would be green for a reason it is not about. ⭐ Block 2 exists because block 1 provably cannot see this clause: ablating `dispatchUnscopedMultiWrite` from both registrations (rebuilt, dist-preflighted) leaves block 1 5/5 GREEN, while block 2 goes red BY RESOLVING — 'promise resolved \"2\"', which is #9719's measured wipe reproduced. Unit-level companion, also through a wired engine rather than a hand-built context: packages/services/service-storage/src/attachment-access-hooks.test.ts, describe 'unscoped multi-delete (no id, no where) — #4757 through the wired engine (#9719)'." }, + "automated": { "kind": "api", "ref": "packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts (clauses 0, 1, 2 and 4: attach without parent EDIT is 403 ATTACHMENT_PARENT_ACCESS while the same member's LIST still succeeds, uploaded_by is server-stamped over a spoofed value, delete splits uploader-vs-outsider, and FILES_DISABLED) + packages/qa/dogfood/test/attachments-unscoped-delete-gate.dogfood.test.ts, which now carries TWO blocks and pins CLAUSE 3 end to end. Block 1 (split fixture — the caller owns one row and may not touch the other): the predicate-less `{ multi: true }` delete is refused with ATTACHMENT_DELETE_DENIED / 403 / the 'Refusing an unscoped multi-delete of attachments' message and removes NOTHING, not even the row it WAS entitled to, so the refusal is authoritative and cannot land as a partial delete; a match-all `where: {}` reaches the same verdict by a DIFFERENT rule and asserts the PER-ROW message instead, which is what keeps the #9797 boundary honest (the whole-operation dispatch is deliberately scoped to a delete with no `where` at all, so `where: {}` stays a real query); and a predicate that MATCHES NOTHING is allowed through, the live half of the #4757 reasoning. Block 2 (the discriminating fixture — the caller uploaded EVERY row, so the per-row gate has nothing to refuse): the predicate-less delete is still refused, which is the only shape that proves 'refused OUTRIGHT' rather than 'every row happened to be denied'; the refusal also fires on an EMPTY table (the zero-match limb — the per-row dispatch is gated on matched rows, so a handler-only fix could never reach it); and both sides are held by the same caller sweeping the same rows with a SCOPED predicate successfully, plus a scoped `where: {}` over the empty table resolving. In both blocks the member is granted the sys_attachment delete bit first and the grant is asserted, because otherwise RBAC refuses ahead of the attachment hook and the file would be green for a reason it is not about. ⭐ Block 2 exists because block 1 provably cannot see this clause: ablating `dispatchUnscopedMultiWrite` from both registrations (rebuilt, dist-preflighted) leaves block 1 5/5 GREEN, while block 2 goes red BY RESOLVING — 'promise resolved \"2\"', which is #9719's measured wipe reproduced. Unit-level companion, also through a wired engine rather than a hand-built context: packages/services/service-storage/src/attachment-access-hooks.test.ts, describe 'unscoped multi-delete (no id, no where) — #4757 through the wired engine (#9719)'. CLAUSE 5 (the UPDATE-verb FILES_DISABLED re-point, new) has a HANDLER/ENGINE-level pin, NOT the wired dogfood/REST lane above: packages/plugins/plugin-audit/src/capability-gate-update-verb.test.ts, describe '[#10170] enable.files is asked on the UPDATE verb too' — runs a real ObjectQL (stub driver) rather than the hand-rolled fake in audit-writers.test.ts (whose registerHook ignores the { object } scope, so it cannot observe a registration-scope regression), and pins BOTH dispatch shapes (by-id AND predicate/per-row) refusing 403 FILES_DISABLED on re-point while leaving the row's parent_object unchanged, plus the positive (a re-point onto a files-ENABLED parent still succeeds) and the negative-control (an update that never names parent_object is not re-checked at all). This engine-level pin is why the checklist step above can isolate the refusal to FILES_DISABLED with confidence; it is NOT itself evidence about the wired REST PATCH lane, which is what clause 5's own oracle drives by hand." }, "source": [ "docs/plans/release-15.1-test-plan.md §A12 (attach 需 parent EDIT)", - "packages/services/service-storage/src/attachment-access-hooks.ts (beforeInsert canEdit gate + uploaded_by stamping; beforeDelete/beforeUpdate uploader-or-editor + #4757 unscoped refusal reached via the `dispatchUnscopedMultiWrite` declaration (#9719/#9974), MULTI_WRITE_AUTH_LIMIT fail-closed)", - "packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts (items 3, a, f; FILES_DISABLED)" + "packages/services/service-storage/src/attachment-access-hooks.ts (beforeInsert canEdit gate + uploaded_by stamping; beforeDelete/beforeUpdate uploader-or-editor + #4757 unscoped refusal reached via the `dispatchUnscopedMultiWrite` declaration (#9719/#9974), MULTI_WRITE_AUTH_LIMIT fail-closed; :373-410 authorizes an update-verb re-point onto the NEW parent, the access half clause 5 relies on)", + "packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts (items 3, a, f; FILES_DISABLED)", + "packages/plugins/plugin-audit/src/audit-writers.ts:1483-1503 (enforceFilesCapability — the #2727 enable.files gate; [#10170] registered it on beforeUpdate as well as beforeInsert because the flag gates the TARGET object a write makes an attachment point at, not the create/update verb) — the FILES_DISABLED source for both clause 4 (create) and the new clause 5 (update/re-point)", + "packages/spec/src/data/object.zod.ts:281 (enable.files describe string, post-#10733: 'otherwise any write that makes an attachment target it is rejected (403 FILES_DISABLED) — a create and an update that re-points an existing attachment alike')", + "packages/plugins/plugin-audit/src/capability-gate-update-verb.test.ts (the #10170 engine-level pin for the update-verb capability gate — both files and feeds, both dispatch shapes)" ], "history": [ { "revision": 1, "date": "2026-08-07", "change": "new item from the access-hook source: EDIT-not-read attach gate, server stamping, delete authorization matrix and the #4757 unscoped-delete refusal, each with its exact deny code", "ref": "claude/platform-test-checklist-ocwugl" }, @@ -425,6 +436,12 @@ "date": "2026-08-23", "change": "clause 3 flipped from NOT pinned to PINNED, and the item's own scoring hazards corrected. Re-verifying this card on the merged ref found the blocker resolved: #9719 was fixed by PR #9797 (opt-in whole-operation `dispatchUnscopedMultiWrite` dispatch, firing once before any row is resolved, zero-match included) and extended to beforeUpdate by #9974, so the #4757 refusal fires again through the wired engine and the clause is pinnable AS WRITTEN — no need to pin behaviour a fix would turn red. Three corrections beyond the flip. (a) The ref and knownGaps had gone stale in the dangerous direction: they told the next runner a shipped, working safety guard was broken. (b) The item's oracle for this clause was `api` and its step 6 prescribed a REST probe, which the item's OWN knownGaps says the HTTP lane cannot express (DeleteManyDataRequestSchema requires an ids list, #3897) — a runner following it would score a schema rejection as the refusal, i.e. green for the wrong reason; oracle is now `test` and the step names an engine holder. (c) The dogfood pin gained a second block, because the first one provably cannot see this clause: its fixture splits entitled/not, so the per-row gate answers with the same code and the block stays 5/5 green with the dispatch ablated. The new block seeds a caller entitled to EVERY matched row — the only shape that distinguishes 'refused outright' from 'every row was denied' — plus the empty-table limb; under ablation it fails by RESOLVING, reproducing #9719's measured wipe. Source list also corrected: MULTI_DELETE_AUTH_LIMIT was renamed MULTI_WRITE_AUTH_LIMIT.", "ref": "#9483" + }, + { + "revision": 5, + "date": "2026-08-25", + "change": "added clause 5, the UPDATE half of the #10170 FILES_DISABLED gate, which this item was missing entirely: a re-point of an existing sys_attachment's parent_object onto the qa_nofiles fixture (already provisioned by qa-scratch-authz, #7670 — one throwaway record is inserted into it, no new fixture) now has its own step, persona note, negative entry and acceptance clause, isolated from ATTACHMENT_PARENT_ACCESS by running as admin (who can edit both the original and target parents). Filed because #10336/PR #10733 rewrote the enable.files describe string to name the update case explicitly ('a create and an update that re-points an existing attachment alike') while this item's create-only probe (clause 4) kept passing regardless of whether the update-verb enforcement #10170 added still held — the same gap the fixtures.qa-scratch-authz step-4 source re-quote (this file, above) makes visible. source/knownGaps/negative/automated updated to match, including the engine-level #10170 pin (capability-gate-update-verb.test.ts) that grounds the isolation claim.", + "ref": "#10735" } ] },