fix(file-manager): keep exclusive shares in /mnt/user space (OS-475)#2679
Draft
elibosley wants to merge 7 commits into
Draft
fix(file-manager): keep exclusive shares in /mnt/user space (OS-475)#2679elibosley wants to merge 7 commits into
elibosley wants to merge 7 commits into
Conversation
Contributor
Walkthrough
ChangesFile Manager path and navigation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Request
participant PathHandlers
participant Filesystem
Request->>PathHandlers: provide requested directory
PathHandlers->>Filesystem: resolve with realpath()
Filesystem-->>PathHandlers: resolved filesystem path
PathHandlers->>PathHandlers: canonicalize /mnt/user lexical path
PathHandlers-->>Request: permitted canonical or resolved path
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
Opening an exclusive share in the File Manager silently switched the whole view - and every folder/row link - from /mnt/user/<share> to the raw /mnt/<disk>/<share> path. Cause: validdir() ran the requested dir through realpath(), which follows the symlink that backs an exclusive share (/mnt/user/<share> -> /mnt/<pool|disk>/<share>). validdir() now still validates with realpath() (the resolved path must live under /mnt or /boot, so traversal is still blocked), but re-anchors to a lexically-canonicalized /mnt/(user0?|rootshare) path when that is what was requested. FUSE is bypassed for exclusive shares, so this is the same data at native speed - just shown under the path the user clicked. Applied to both Browse.page and include/Browse.php. Also swap the open-in-File-Manager launcher icon from icon-u-tab (a square-with-arrow / open-in-tab glyph that doesn't say what it does) to fa fa-folder-open across DiskList, ShareList and device_list, matching the folder-open icon already used elsewhere. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
elibosley
force-pushed
the
os-filemanager-exclusive-user-link
branch
from
June 23, 2026 22:52
09b0b6f to
f86397e
Compare
…e link The folder-open browse icon sits immediately left of the device/share name link and inherited the same link blue, so the two read as a single element - you couldn't tell the icon was a separate clickable action. Mute the browse icon to --alt-text-color (brightening to the link color on hover to keep the clickable affordance) and add a right-margin gap before the name. Applied once on the shared a.view rule, so DiskList, ShareList and the device list all get it; the empty .view placeholders take the same slot, so column alignment is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Use a closed folder (fa-folder-o) at rest for the browse links and swap to the open variant on hover, so the icon visibly previews the "open this location" action. Done via a single a.view:hover content override in CSS; the icon class changes to fa-folder-o in DiskList, ShareList and the device list. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The ZFS/btrfs pool health text linked to the pool's details section but looked like a stray blue label - same color as the device-name link next to it - so it wasn't clear it was clickable or what it did. Give it a dedicated .pool-status style (muted text, brightens + underlines on hover) and a trailing chevron to mark it navigable, keeping the existing "View pool details" tooltip. Applies to the healthy and problem states. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address the affordance gaps with comprehension cues, not just styling:
- Browse folder links now carry Unraid's standard a.info tooltip ("Open
in File Manager") instead of the slow native title=, so hovering says
what the icon does. Combined with the closed->open swap and hover
color, the action is now self-explanatory. Applied on DiskList,
ShareList and the device list.
- Pool health (ONLINE) is no longer a hyperlink - a clickable status word
is an odd pattern. It's now plain status text, with a small info-circle
beside it as the only clickable element (a.info tooltip "View pool
details") linking to the pool summary. The existing warning icon for
DEGRADED/ERRORS states gets the same styled tooltip.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
elibosley
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@emhttp/plugins/dynamix/include/ShareList.php`:
- Line 269: Replace htmlspecialchars with rawurlencode for URL path values in
ShareList.php lines 269-269 ($name), device_list lines 510-514 ($dir),
device_list lines 1399-1407 ($bootBrowseUrl and $poolBrowseUrl), and
DiskList.php lines 113-113 ($name), preserving the surrounding link generation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 36295cec-49f1-41fa-b630-cbe787f810fb
📒 Files selected for processing (7)
emhttp/plugins/dynamix/Browse.pageemhttp/plugins/dynamix/include/Browse.phpemhttp/plugins/dynamix/include/DiskList.phpemhttp/plugins/dynamix/include/FileTree.phpemhttp/plugins/dynamix/include/ShareList.phpemhttp/plugins/dynamix/nchan/device_listemhttp/plugins/dynamix/styles/default-base.css
Member
Author
|
QA follow-up addressed in The destination tree had its own Verification:
The current retest plugin is |
unraid-bot
added
the
7.4.0
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
Exclusive shares now stay in
/mnt/userthroughout both File Manager browsing and copy/move destination selection, preventing nested destinations from being rewritten to raw backing-storage paths and rejected as invalid targets.Why This Exists
Opening an exclusive share originally ran the requested path through
realpath(), which follows the exclusive-share symlink from/mnt/user/<share>to/mnt/<pool|disk>/<share>. The first fix preserved the path in the main File Manager listing, but QA found the destination picker had an independentrealpath()boundary inFileTree.php. Selecting a folder below an exclusive share therefore still changed the target path and caused anInvalid targeterror.Resolution
Preserve a lexically canonicalized
/mnt/user,/mnt/user0, or/mnt/rootsharepath after confirming the requested directory exists withrealpath(). The same rule now applies in the main browser and the destination tree. Disk, pool, boot, and Unassigned Devices paths retain their existing resolved behavior.The PR also improves the File Manager launcher affordance with a labeled folder icon, URL-encodes browse destinations containing reserved characters, and clarifies pool-status navigation.
Reviewer Considerations
emhttp/webGuiis a hard-linked mirror ofplugins/dynamix; the tracked files underplugins/dynamixcover both runtime paths.include/FileTree.phptogether with the existingvaliddir()changes: both boundaries must preserve the requested user-share namespace to avoid reintroducing mixed/mnt/userand/mnt/<disk>paths.realpath()andis_dir()success before preserving a user-share path, so nonexistent destinations are not accepted.rawurlencode()because they are query parameter values; HTML escaping does not protect reserved URL characters such as&and#.Behavior Changes
/mnt/user/<share>./mnt/user/<share>/...in the target field./mnt/<disk|pool>/....Open in File Managertooltip.Implementation Summary
Browse.pageandinclude/Browse.php.Verification
php -lonFileTree.php,ShareList.php,DiskList.php, andnchan/device_list— pass.git diff --check— pass./mnt/user/share -> /mnt/cache/share:/mnt/user/share/folder— pass;/mnt/disk1/data— pass;realpath()resolves the nested user path to/mnt/cache/share/folder— pass.My & Share#1— pass.Risk
Low. The path-preservation change is confined to File Tree presentation for existing user-share directories; filesystem existence is still established from the symlink-resolved path, and non-user-share paths retain the prior behavior. Browse-link encoding uses the standard query-value encoding already used elsewhere in these views.
Linear
Summary by CodeRabbit
/mntand/boot.