Repository navigation
Bulk delete ignores every draft rule: others' drafts deleted, soft-deleted, no draft.removed #4665
Description
Found during a Strict review of #4580 (release development → beta). Present on development at 8e7bbd87; introduced in #4566, #4600.
Impact
filterUuidsForPermissions() reads across all magic tables and checks only the schema's delete permission, so a bulk delete removes other users' drafts that a single delete would refuse.
The chain
The draft delete rules live only in deleteObject(): the lookup under the read RBAC (which hides somebody else's draft), isDeletableDraft() for archival schemas, and permanent = true here. deleteObjects(), behind POST /api/bulk/{register}/{schema}/delete (BulkController::delete, @NoAdminRequired), has none of them.
| Contract branch (spec draft-owner-activation-and-lifetime) | Single delete | Bulk delete |
|---|---|---|
| Only somebody who can read a draft may change it | ✓ RBAC lookup hides it → 404 | ✗ filterUuidsForPermissions() → hasPermission(delete, objectOwner), which has no draft rule |
| A draft's delete is permanent, with its files | ✓ line 3190 | ✗ deleteHandler->deleteObject() without permanent, so a soft-deleted tombstone keeps the unsent input |
One draft.removed audit entry without content |
✓ written in the permanent branch of DeleteObject::delete() |
✗ not permanent, so no entry |
| A draft is deletable on an archival schema | ✓ isDeletableDraft() |
✗ rejectIfArchivalImmutable() refuses it, so the owner cannot bulk-discard their own draft there |
So any user with delete rights in a schema can remove another user's draft by uuid, and the removed draft's content stays in the trash.
Sibling of the open thread at DraftStatusPolicy.php:295 (bulk save over somebody else's draft). That thread covers the bulk save path, not the bulk delete path. The root cause is the same: PermissionHandler::hasPermission() has no draft rule. Putting keptFrom() into hasPermission() when an object is passed would close both threads and f3.
Suggested fix: in deleteObjects(), for each pre-resolved row that isDraft(): skip it (reason OBJECT_NOT_PERMITTED, the same answer as an unreadable row) when $_rbac is on and DraftStatusPolicy::keptFrom() is true; otherwise pass permanent: true and skip the archival gate, as deleteObject() does.
Done when:
- a
deleteObjects()test: user B bulk-deleting user A's draft gets it inskipped_uuidsand the draft still exists; A bulk-deleting it removes it permanently and writes onedraft.removedentry - the same test goes red with the new branch removed
Verified against 8e7bbd87eede51a19cd16b1eda5f1f9cbe21dfc0.
Reproduction
Verified by reading the code at 8e7bbd87; not executed.
Done when
- ObjectService deleteObjects test: B's bulk delete of A's draft lands in skipped_uuids and the row remains; red without the draft branch
- A's bulk delete of own draft is permanent and writes exactly one draft.removed entry
Activity
github-actions commentedon Oct 16, 2026
Thank you for opening this issue! It has been assigned to @rubenvdlinde for review.
What happens next:
Triage labels:
triage:accept-feature— accepted as a new featuretriage:accept-bugfix— accepted as a bug fixtriage:needs-info— more information neededtriage:wontfix— will not be addressedtriage:duplicate— duplicate of another issue