Fix /recommended-folders 500 on unreadable model directories (Python 3.12+) (#5523)

* Fix /recommended-folders 500 on unreadable model dirs (Python 3.12+)

get_recommended_folders probed candidate paths with a bare
Path(p).is_dir(). On Python <= 3.11 that returned False for an
unreadable path, but on Python 3.12+ is_dir() propagates
PermissionError (EACCES) instead, so a stock root-owned ollama
install at /usr/share/ollama/.ollama/models (mode 700) made the
endpoint 500 through the entire middleware stack.

Move the directory-accessibility check into a stdlib-only helper
utils.fs_access.is_accessible_dir that swallows OSError and keeps
the existing os.access(R_OK|X_OK) filter, restoring the pre-3.12
"unreadable path is simply not a candidate" behaviour. Add a
dependency-free regression test.

* Address review: inline helper, no new file, cover all probe sites

- Drop studio/backend/utils/fs_access.py and the cross-module import;
  the guard is now a small module-level _safe_is_dir() in models.py
  (also moots the import-placement / ModuleNotFoundError feedback,
  since there is no longer an import to misplace).
- Apply _safe_is_dir to every system-location probe with the
  vulnerable bare is_dir() pattern, not just /recommended-folders:
  _build_browse_allowlist._add and the /browse-folders _add_sug
  helper, so the same Python 3.12+ PermissionError cannot 500 those
  endpoints either. Each site keeps its exact prior semantics
  (recommended-folders retains its os.access(R_OK|X_OK) filter);
  the only behavioural change is "no longer crashes".
- Rewrite the regression test to extract the real _safe_is_dir from
  source via ast, keeping it dependency-free without standing up the
  FastAPI app, and correct the mode-000 case.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Roland Tannous <115670425+rolandtannous@users.noreply.github.com>

studio/backend/routes/models.py

@@ -26,6 +26,22 @@ def _is_valid_repo_id(repo_id: str) -> bool:
     return bool(_VALID_REPO_ID.fullmatch(repo_id))
 
 
+def _safe_is_dir(path) -> bool:
+    """``Path.is_dir()`` that returns ``False`` instead of raising.
+
+    On Python >= 3.12 ``is_dir()``'s ``os.stat`` only suppresses
+    "not found"-class errors and now propagates ``PermissionError``
+    (EACCES); on Python <= 3.11 it returned ``False``. The folder-scan
+    endpoints probe well-known system locations (e.g. a root-owned,
+    mode-700 ``/usr/share/ollama/.ollama/models``) and must treat an
+    un-stat-able path as "not a directory", never 500.
+    """
+    try:
+        return Path(path).is_dir()
+    except OSError:
+        return False
+
+
 # Add backend directory to path
 backend_path = Path(__file__).parent.parent.parent
 if str(backend_path) not in sys.path:
@@ -882,7 +898,7 @@ async def get_recommended_folders(
             return
         if resolved in seen:
             return
-        if Path(resolved).is_dir() and os.access(resolved, os.R_OK | os.X_OK):
+        if _safe_is_dir(resolved) and os.access(resolved, os.R_OK | os.X_OK):
             seen.add(resolved)
             folders.append(resolved)
 
@@ -1056,7 +1072,7 @@ def _build_browse_allowlist() -> list[Path]:
             resolved = p.resolve()
         except OSError:
             return
-        if resolved.is_dir():
+        if _safe_is_dir(resolved):
             candidates.append(resolved)
 
     _add(Path.home())
@@ -1389,7 +1405,7 @@ async def browse_folders(
             return
         if resolved in seen_sug:
             return
-        if Path(resolved).is_dir():
+        if _safe_is_dir(resolved):
             seen_sug.add(resolved)
             suggestions.append(resolved)
 

studio/backend/tests/test_recommended_folders_permission.py

@@ -0,0 +1,125 @@
+# SPDX-License-Identifier: AGPL-3.0-only
+# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
+
+"""
+Regression test for the /recommended-folders (and /browse-folders) 500
+caused by an unreadable model directory, e.g. a stock root-owned
+``ollama`` install at ``/usr/share/ollama/.ollama/models``.
+
+Root cause: the folder-scan helpers in ``routes.models`` probed candidate
+paths with a bare ``Path(p).is_dir()``. On Python <= 3.11 that returned
+``False`` for an unreadable path; on Python >= 3.12 ``is_dir()`` propagates
+``PermissionError`` (EACCES), so the endpoint 500-ed through the whole
+middleware stack instead of just skipping the directory. The probes now go
+through the module-level ``_safe_is_dir`` helper.
+
+``routes.models`` pulls the full backend dependency tree (fastapi,
+structlog, the models package, ...), so rather than stand up the app we
+extract the real ``_safe_is_dir`` definition from the source file and
+exercise that exact function in isolation. The test therefore stays
+dependency-free while still running the shipped code.
+
+Run:
+    python -m pytest studio/backend/tests/test_recommended_folders_permission.py -v
+"""
+
+import ast
+import os
+import sys
+from pathlib import Path
+
+import pytest
+
+_backend_root = Path(__file__).resolve().parent.parent
+_models_src = _backend_root / "routes" / "models.py"
+
+
+def _load_safe_is_dir():
+    """Return the real ``_safe_is_dir`` from routes/models.py without
+    importing the (heavily dependency-laden) module."""
+    tree = ast.parse(_models_src.read_text())
+    fn = next(
+        node
+        for node in tree.body
+        if isinstance(node, ast.FunctionDef) and node.name == "_safe_is_dir"
+    )
+    module = ast.Module(body = [fn], type_ignores = [])
+    ns: dict = {"Path": Path, "os": os}
+    exec(compile(module, f"<extracted {_models_src}>", "exec"), ns)
+    return ns["_safe_is_dir"]
+
+
+safe_is_dir = _load_safe_is_dir()
+
+# Permission bits are bypassed for the superuser, so the chmod-000 setup
+# below would not actually deny access when running as root.
+_skip_as_root = pytest.mark.skipif(
+    hasattr(os, "geteuid") and os.geteuid() == 0,
+    reason = "root bypasses filesystem permission bits",
+)
+
+
+def test_helper_exists_in_source():
+    # Guards against a refactor silently dropping the helper the fix
+    # depends on (the extractor would then raise StopIteration).
+    assert callable(safe_is_dir)
+
+
+def test_readable_dir_is_true(tmp_path):
+    assert safe_is_dir(tmp_path) is True
+
+
+def test_missing_path_is_false(tmp_path):
+    assert safe_is_dir(tmp_path / "does-not-exist") is False
+
+
+def test_file_is_false(tmp_path):
+    f = tmp_path / "weights.gguf"
+    f.write_bytes(b"x")
+    assert safe_is_dir(f) is False
+
+
+@_skip_as_root
+def test_mode000_dir_itself_is_still_a_dir(tmp_path):
+    """A mode-000 directory is still stat-able via its (traversable)
+    parent, so _safe_is_dir reports True without raising. Filtering out
+    dirs we cannot actually *read* is the caller's separate
+    os.access(R_OK|X_OK) check, not this helper's job."""
+    locked = tmp_path / "locked"
+    locked.mkdir()
+    os.chmod(locked, 0o000)
+    try:
+        assert safe_is_dir(locked) is True  # must not raise
+    finally:
+        os.chmod(locked, 0o755)
+
+
+@_skip_as_root
+def test_path_under_unreadable_parent_returns_false_not_raises(tmp_path):
+    """The exact production scenario: stat()-ing a child of a mode-700
+    system directory, e.g. ``/usr/share/ollama/.ollama/models``."""
+    parent = tmp_path / "ollama"
+    parent.mkdir()
+    os.chmod(parent, 0o000)
+    try:
+        assert safe_is_dir(parent / ".ollama" / "models") is False
+    finally:
+        os.chmod(parent, 0o755)
+
+
+@_skip_as_root
+@pytest.mark.skipif(
+    sys.version_info < (3, 12),
+    reason = "is_dir() only propagates PermissionError on Python >= 3.12",
+)
+def test_demonstrates_the_underlying_stdlib_regression(tmp_path):
+    """Documents *why* _safe_is_dir exists: the old bare pattern raises
+    on the interpreters Studio ships on (3.12+)."""
+    parent = tmp_path / "ollama"
+    parent.mkdir()
+    os.chmod(parent, 0o000)
+    try:
+        with pytest.raises(PermissionError):
+            Path(parent / ".ollama" / "models").is_dir()  # pre-fix expr
+    finally:
+        os.chmod(parent, 0o755)