Improve corrupt share permission situation - #64165
Open
marcelklehr wants to merge 3 commits into
Open
Conversation
Member
Author
|
/backport to stable32 |
Member
Author
|
/backport to stable33 |
Member
Author
|
/backport to stable34 |
Member
Author
|
/backport to stable35 |
marcelklehr
force-pushed
the
fix/improve-corrupt-share-permission-breakage
branch
2 times, most recently
from
September 10, 2026 10:25
49ffffe to
5b66fa3
Compare
…rmissions `SharedStorage::isValid()` returns false when the share source lost its share permission, which makes every permission check return 0 while reading keeps working. For the recipient that looks like arbitrary breakage: uploads fail with 403, renames get reverted by the sync client, and the file list still shows the folder as writable. Nothing was logged, so there was no way to tell this apart from real corruption. Log a warning once per storage instance naming the share, the owner and the source file id plus its cached permissions, so the source that needs repairing can be found. Assisted-By: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
When the source of a share lost its share permission in the file cache, `SharedStorage::getPermissions()` returns 0 and every write is refused. The shared cache never took part in that decision: it reported `filecache.permissions & share.permissions`, so the file list showed the folder and its contents as writable while every upload, move and rename was answered with a 403. The web UI turns that into "Operation is blocked by access control" and sync clients revert renames a few seconds later, which makes a wrong permission value look like arbitrary corruption. Mask the reported permissions down to read when the source is invalid, so what the cache advertises is what the storage grants. Reading is left untouched because it keeps working: `fopen()` in read mode bypasses the permission checks. The validity check moves into `SharedStorage::hasValidSourcePermissions()` so the storage and its cache share one definition. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
marcelklehr
force-pushed
the
fix/improve-corrupt-share-permission-breakage
branch
from
September 10, 2026 10:25
5b66fa3 to
b163e1f
Compare
marcelklehr
marked this pull request as ready for review
September 10, 2026 17:17
marcelklehr
requested review from
Altahrim,
come-nc,
icewind1991 and
salmart-dev
and removed request for
a team
September 10, 2026 17:17
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.
Summary
Patch 1: fix(files_sharing): log when a share is disabled by invalid source permissions
SharedStorage::isValid() silently disabled a whole share when its source lost the share permission bit in the file cache. Now it logs one warning per storage instance naming the share id, owner, source file id and the source's cached permissions, with a pointer to occ files:scan. Unresolvable sources (fileid -1, the FailedCache placeholder) are excluded so orphaned shares don't spam the log; that's a different failure, already handled by FailedStorage.
Patch 2: fix(files_sharing): don't advertise permissions an invalid share denies
The shared cache reported filecache.permissions & share.permissions without consulting isValid(), so a folder looked writable while every write got a 403. The bit test moved into SharedStorage::hasValidSourcePermissions(), and Cache::formatCacheEntry() now masks reported permissions down to READ when the source is invalid; matching what the storage actually grants, since reads keep working via fopen() in read mode. Includes a regression test.
Together they turn the reported symptom set, the missing share icon, "Resharing is not allowed", 403 uploads, renames reverting, into a visible, diagnosable read-only share plus a log line naming the row to repair.
TODO
Checklist
3. to review, feature component)stable32)AI (if applicable)