fix(trash): allow restoring trashed items in "no permission" folders#4880
Open
solracsf wants to merge 1 commit into
Open
fix(trash): allow restoring trashed items in "no permission" folders#4880solracsf wants to merge 1 commit into
solracsf wants to merge 1 commit into
Conversation
solracsf
added
bug
3. to review
Member
Author
|
/backport to stable34 |
Member
Author
|
/backport to stable33 |
provokateurin
requested changes
Jul 15, 2026
Comment on lines
-336
to
347
| $activePermissions = $aclManager->getACLPermissionsForPath($item->folder->id, $item->getGroupTrashFolderStorageId(), $trashPath); | ||
| $activePermissions = $aclManager->getACLPermissionsForPath($item->folder->id, $item->getGroupTrashFolderStorageId(), $trashPath, baseOverride: Constants::PERMISSION_ALL); | ||
| $originalPath = $item->folder->rootCacheEntry->getPath() . '/' . $item->getInternalOriginalLocation() . $pathInsideItem; | ||
| $originalLocationPermissions = $aclManager->getACLPermissionsForPath($item->folder->id, $item->getGroupFolderStorageId(), $originalPath); |
Member
There was a problem hiding this comment.
Why do we check trash ACLs in the first place, if we check the ACLs for the original location anyway? The argument from your PR description doesn't make a lot of sense to me, because must either check all ACLs the same way or only check the ones of the original location.
Member
Author
There was a problem hiding this comment.
Rules are keyed by file id and move with the item into the trash storage, so the trash path is the only place the item's own rules can still be found, the original location only yields the parent rules. They're now checked the same way: one evaluation, original-location permissions as base, trash-path rules on top.
Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
solracsf
force-pushed
the
fix/allow-restoring-trashed-items
branch
from
July 15, 2026 15:47
9012fbe to
46b4957
Compare
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 #4830
With
acl_default_no_permissionset, trash items could not be restored or permanently deleted, and were hidden from regular users. Two causes:TrashBackend::userHasAccessToItem()applied the deny-by-default base even when ACL is disabled (the creation dialog sets the flag without enabling ACL). It now skips the per-path check when ACL is off, andcreateFolder()enables ACL when the flag is set.0the trash side could never rise above0. It is now a single evaluation: original-location permissions (base + parent rules), with the item's own rules, which move with the item into the trash storage, applied on top.Behavior deltas, both matching the live mounts: leftover rules are ignored for trash while ACL is disabled; an allow rule on the item itself now overrides a parent deny (previously denied by the AND), which also makes item-only grants restorable.
🤖 AI (if applicable)