Skip to content

add trailing slash to restore_path for trashed directories#4794

Closed
JF-Cozy wants to merge 2 commits into
masterfrom
feat/restore_path
Closed

add trailing slash to restore_path for trashed directories#4794
JF-Cozy wants to merge 2 commits into
masterfrom
feat/restore_path

Conversation

@JF-Cozy

@JF-Cozy JF-Cozy commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Add a trailing / to restore_path (path of the parent folder) when trashing files and directories. This prevents false positives when querying the trash of shared drives with similar names — without it, a $gte: "/Shared Drives/Test" query also captures files from "/Shared Drives/Test 2/...".

No impact on RestoreDir / RestoreFileDirByPath uses path.Clean which strips trailing slashes, and path.Join normalizes paths.

App must change fetch from

    Q(FILES_DOCTYPE)
      .where({
        dir_id: TRASH_DIR_ID,
        type,
        name: { $gt: null },
        restore_path: { $gte: path, $lt: path + '\ufff0' }
      })

to

    Q(FILES_DOCTYPE)
      .where({
        dir_id: TRASH_DIR_ID,
        type,
        name: { $gt: null },
        restore_path: { $gte: path + '/', $lt: path + '\ufff0' }

@JF-Cozy
JF-Cozy requested a review from a team as a code owner June 11, 2026 11:30
Comment thread model/vfs/file.go

var newdoc *FileDoc
restorePath := path.Dir(oldpath)
restorePath := path.Dir(oldpath) + "/"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what will be for the files with "/" as an oldpath? I think we should keep root unchanged

JF-Cozy added 2 commits June 12, 2026 10:21
Add a trailing `/` to `restore_path` when trashing files and directories. This prevents false positives when querying the trash of shared drives with similar names — without it, a `$gte: "/Shared Drives/Test"` query also captures files from "/Shared Drives/Test 2/...".

No impact on `RestoreDir` / `RestoreFile` — `DirByPath` uses `path.Clean` which strips trailing slashes, and `path.Join` normalizes paths.
@shepilov
shepilov force-pushed the feat/restore_path branch from a19cd7b to dd76641 Compare June 12, 2026 08:21
@shepilov

Copy link
Copy Markdown
Member

@JF-Cozy is it sitll relevant for displaying trash in admin panel?

@JF-Cozy

JF-Cozy commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Not prioritized today, and a more robust approach using referenced_by would be better, so we can close it for now.

Here's a full summary of the problem and solutions:

Context

When two shared drives have names sharing a common prefix (e.g. "Shared" and "Shared Drive"), the trash view of one drive leaks files from the other. The cozy-admin trash query scopes the listing with a lexicographic range on restore_path:

restore_path: { $gte: path, $lt: path + '\ufff0' }

Because "Shared" is a string prefix of "Shared Drive", a $gte: "/Drives/Shared" also matches /Drives/Shared Drive/... — a classic string-prefix collision, not an identity match.

Approach A — trailing slash on restore_path (this PR)

Append / to restore_path at trash time on the backend, and use path + '/' as the lower bound in the app query. The / makes the bound a directory delimiter: /Drives/Shared/ no longer matches /Drives/Shared Drive/... since "Shared D..." > "Shared/".

Pros: minimal diff, immediate fix.
Cons: requires a coordinated change on the app side (both must agree on the /); does not backfill already-trashed docs created before the fix; the root edge case (path.Dir("/")"//") needs a guard; the contract is implicit across two repos.

Approach B — scope by referenced_by (long-term)

Scope the trash to a shared drive via a referenced_by selector pointing at the sharing id, instead of matching the human-readable path. This is the more robust model: it keys on a stable document identity, so no string-prefix collision is possible and it is immune to drive renames.

This is not a drop-in swap today — it requires:

  • per-file tagging (currently only the shared-drive root carries referenced_by {type: io.cozy.sharings, id: sharingID}, children do not),
  • a Mango index on referenced_by (only a CouchDB view exists today),
  • a _find route for shared drives that injects the selector server-side.

Once in place, the app query becomes identity-based and the path-prefix class of bug disappears entirely.

@JF-Cozy JF-Cozy closed this Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants