Studio: expose Windows drive roots in the folder browser - #7082

Fix drive-root descendants in legacy browser

When /api/models/browse-folders runs on Windows, this new allowlist root is stored as D:\; _is_path_inside_allowlist() later checks descendants with target_real.startswith(root_real + os.sep), which becomes D:\\, so a child such as D:\models is rejected after _match_browse_child resolves it. The suggestion can open the drive root, but clicking any subfolder under that drive root returns 403 in this route; handle root paths with a trailing separator or use commonpath as the hub browser does.

Useful? React with 👍 / 👎.

Confirmed and kept fixed in af0cc8b. _is_path_inside_allowlist now uses os.path.splitdrive + os.path.commonpath, so a drive root (D:) authorizes its descendants (D:\models) via component-wise containment instead of a string prefix. Covered by test_windows_external_drive_paths.py (drive-root descendant, cross-drive, case-insensitive).

Preserve the system-prefix denylist when adding drive roots

When Windows exposes C: (or whichever drive hosts SystemRoot/ProgramFiles), this returns the whole drive root and both browse allowlists add it directly. I checked routes/models.py::_resolve_browse_target and hub/services/models/folder_browser.py::_resolve_browse_target: after this root matches they only block credential/config components, while storage/studio_db.py and hub/storage/scan_folders.py explicitly reject SystemRoot/ProgramFiles, so requests like /api/models/browse-folders?path=C:\Windows or the hub equivalent can now enumerate directories that the existing scan-folder policy denies. Filter those denied prefixes during browsing, or avoid adding system-drive roots wholesale.

Useful? React with 👍 / 👎.

Good catch, fixed in af0cc8b. Instead of filtering at drive-root construction, both browse resolvers now re-apply the system-prefix denylist through a new is_denied_system_path() in storage/studio_db.py and hub/storage/scan_folders.py, mirroring add_scan_folder (including the /run/media carve-out and Windows normcase). C:\Windows and C:\Program Files are refused on descent while C:\ stays browseable, and they are also hidden from the listing and chips. Covered by test_browse_denylist.py.

Keep POSIX root registrations from unlocking every child

On POSIX, when the legacy browse allowlist contains / (for example from an existing scan_folders row, since storage.studio_db.add_scan_folder does not reject the filesystem root), this new prefix becomes /, so every absolute path now passes _is_path_inside_allowlist. Before this change the descendant prefix was //, which only allowed browsing / itself; now /etc, /proc, etc. can be entered through /api/models/browse-folders even though those prefixes are otherwise denied by the scan-folder policy. Special-case Windows drive roots instead of any separator-terminated root.

Useful? React with 👍 / 👎.

Correct, fixed in af0cc8b. _is_path_inside_allowlist now special-cases a bare POSIX filesystem root (empty splitdrive and dirname == path): it matches / itself but does not authorize descendants, so /etc is no longer reachable via a / scan-folder row. The browse-time denylist is a second gate for stale rows, and add_scan_folder now also rejects registering the filesystem root. Regression tests in test_browse_denylist.py.

Allow UNC share roots to be registered

On Windows, a UNC share root such as \\nas\models also satisfies os.path.dirname(normalized) == normalized, so this new guard rejects it as a filesystem root. That blocks users whose model directory is the root of a network share from adding it through /api/models/scan-folders, even though it is not above local system directories like C:\Windows; please distinguish bare drive/filesystem roots from UNC share roots.

Useful? React with 👍 / 👎.

Thanks. I did try exactly this: a carve-out that allowed a bare UNC share root while still rejecting C:\ and /. The follow-up review found it was only half-complete though. \\nas\models\ with its usual trailing separator was still rejected, and even once registered the share root was unbrowsable because ntpath.commonpath raises for the no-trailing-separator form that realpath stores. Fully supporting a bare share root means adding UNC-specific normalization to the security-critical containment path, which I cannot validate off Windows, so I reverted to rejecting it consistently with C:\ and /. Registering a subdirectory of the share (for example \\nas\models\gguf) still works and browses normally, which covers the real use case.

Do not reject UNC share roots as filesystem roots

On Windows, a UNC share root such as \\nas\models also satisfies ntpath.dirname(normalized) == normalized, so this new guard rejects a readable network share root before the existing denylist logic runs. That blocks users from registering a share root that was valid before and does not expose local drive system directories; the same guard was added in hub/storage/scan_folders.py, so both scan-folder endpoints need the narrower check (e.g. only POSIX / and drive roots like C:\).

Useful? React with 👍 / 👎.

Fixed in 1983329. The register guard now calls is_local_filesystem_root, which still rejects a bare local root (/ or C:) but exempts a UNC share root (\server\share): it has no denied system dirs under it and was registerable before this guard was added. Applied to both the legacy (studio_db.py) and hub (scan_folders.py) browsers through one shared helper, with a parametrized ntpath/posixpath regression test.

Avoid accumulating timed-out drive probe threads

When a disconnected mapped drive makes os.path.isdir() block past _DRIVE_PROBE_TIMEOUT_S, this code returns without joining the daemon thread, but both browse endpoints call windows_drive_roots() on every folder request. Repeated navigation with one stale mapping can therefore spawn another stuck probe thread per request until the OS calls eventually return, causing unbounded thread buildup even though each individual request times out promptly; cache/serialize in-flight probes or otherwise avoid starting a fresh thread for the same hung drive each time.

Useful? React with 👍 / 👎.

Leaving this as is. A probe that blocks on a disconnected mapping is bounded by the OS: roughly 21s to abandon a TCP connect to an unreachable host, or the SMB SessTimeout (default about 60s) at the session layer, after which the daemon thread returns and exits. Stuck threads therefore self-drain to a steady state of about browse_rate times that timeout rather than growing without limit, and the returned root set is already correct because a timed-out drive is dropped. Running the isdir check in a bounded worker thread is the recommended mitigation for the redirector stall, so the current design is intentional.

Reject device-prefixed drive roots as local roots

When a Windows path uses the Win32 device prefix (\\?\C:\ or \\.\C:\), ntpath.splitdrive() returns a drive beginning with \\, so this UNC carve-out treats it as non-local even though it is a local drive root. In that scenario add_scan_folder() accepts the root as a scan folder; once it is in the browse allowlist, paths like \\?\C:\Windows are contained by that root and also miss the normal C:\Windows denylist prefix, so the system-directory block can be bypassed.

Useful? React with 👍 / 👎.

Fixed in 3911798. is_local_filesystem_root now collapses the Windows device / extended-length prefix before classifying, so \?\C:\ and \.\C:\ are judged as the C:\ drive root and rejected, while \?\UNC\server\share still maps to the \server\share share root and stays registerable. Added parametrized ntpath cases for both forms.

Bound accumulated drive-probe threads

When a mapped drive stays hung longer than the timeout, each /browse-folders request starts a fresh daemon thread for that drive and returns without joining or cancelling it. With a disconnected network drive and repeated folder-browser navigation, these stuck probes accumulate until the OS calls eventually return, so the request timeout still allows unbounded thread/resource growth in the server process.

Useful? React with 👍 / 👎.

Leaving this as is, same as the earlier thread on this function. A probe that blocks on a disconnected mapping is bounded by the OS: roughly 21s to abandon a TCP connect to an unreachable host, or the SMB SessTimeout (default about 60s) at the session layer, after which the daemon thread returns and exits. Stuck threads therefore self-drain to a steady state of about browse_rate times that timeout rather than growing without bound, and the returned root set is already correct because a timed-out drive is dropped. Running the isdir check in a bounded worker thread is the recommended mitigation for the redirector stall, so I would rather not add a module-level probe cache and its locking to the hot browse path.

Preserve UNC share roots when checking containment

When a registered Windows UNC share is stored as \\server\share (no trailing slash), ntpath.commonpath(["\\server\share\models", "\\server\share"]) raises ValueError, so this containment check returns false and browsing any child of the newly allowed UNC root 403s. This regresses the commit's UNC-share exception unless the user happens to register the share with a trailing separator.

Useful? React with 👍 / 👎.

Fixed in 3911798. os.path.commonpath raises 'can't mix absolute and relative' on a bare \server\share, so _is_path_inside_allowlist now detects a UNC share root (drive set, empty splitdrive tail) and authorizes its descendants with a boundary-safe prefix test on the normcased realpath. Applied to both the legacy and hub browsers, with a parametrized test that a registered share authorizes its children but not a sibling share or a different volume.

Keep volume GUID roots classified as local roots

When a Windows path uses a volume/device root such as \\?\Volume{GUID}\, this prefix-stripping turns it into Volume{GUID}\, so _pathmod.dirname(path) != path and is_local_filesystem_root() returns False. add_scan_folder() can then accept a whole local volume even though drive roots are supposed to be rejected; because the denylist only matches drive-letter paths like C:\Windows, browsing that registered volume can expose system directories on the OS volume. Treat \\?\Volume.../\\.\Volume... as local filesystem roots before stripping or add an explicit volume-GUID case.

Useful? React with 👍 / 👎.

Fixed in 86355eb. is_local_filesystem_root now resolves the device namespace fully: after a \?\ or \.\ prefix, a bare volume specifier with no further component (a drive letter C: or a Volume{GUID}) is a local volume root and rejected, while \?\UNC\server\share still maps to the \server\share share root and stays registerable. Added parametrized \?\Volume{GUID}\ and deep extended-length path cases.