Skip to content

Navigation Menu

Sign in
Sign up

Improve corrupt share permission situation - #64165

Open
marcelklehr wants to merge 3 commits into
master from
fix/improve-corrupt-share-permission-breakage
Open

Improve corrupt share permission situation #64165
marcelklehr wants to merge 3 commits into
master from
fix/improve-corrupt-share-permission-breakage

Conversation

@marcelklehr

@marcelklehr marcelklehr commented Sep 10, 2026

Copy link
Copy Markdown
Member

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

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Copy link
Copy Markdown
Member Author

/backport to stable32

backportbot[bot] reacted with eyes emoji

Copy link
Copy Markdown
Member Author

/backport to stable33

backportbot[bot] reacted with eyes emoji

Copy link
Copy Markdown
Member Author

/backport to stable34

backportbot[bot] reacted with eyes emoji

Copy link
Copy Markdown
Member Author

/backport to stable35

backportbot[bot] reacted with eyes emoji

marcelklehr force-pushed the fix/improve-corrupt-share-permission-breakage branch 2 times, most recently from 49ffffe to 5b66fa3 Compare September 10, 2026 10:25
Marcel Klehr and others added 3 commits September 10, 2026 12:25
...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 5b66fa3 to b163e1f Compare September 10, 2026 10:25
marcelklehr marked this pull request as ready for review September 10, 2026 17:17
marcelklehr requested a review from a team as a code owner 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@icewind1991 icewind1991 Awaiting requested review from icewind1991 icewind1991 is a code owner automatically assigned from nextcloud/server-backend
@Altahrim Altahrim Awaiting requested review from Altahrim Altahrim is a code owner automatically assigned from nextcloud/server-backend
@salmart-dev salmart-dev Awaiting requested review from salmart-dev salmart-dev is a code owner automatically assigned from nextcloud/server-backend
@come-nc come-nc Awaiting requested review from come-nc come-nc is a code owner automatically assigned from nextcloud/server-backend

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Projects

None yet

Milestone

Nextcloud 36

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /