From 65eaaeff4c70a3b3d22a6004a4a58bcb252ac3ee Mon Sep 17 00:00:00 2001 From: archdex-art Date: Tue, 14 Jul 2026 20:19:27 +0530 Subject: [PATCH] fix(cookbook): validate model_dir segments in /api/model/cached MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /api/model/cached endpoint split model_dir on commas and normalized bare Linux paths, but never validated segments before embedding them into the generated scan script — unlike the sibling /api/model/download endpoint, which runs local_dir through _validate_local_dir. Extracts the split/normalize/validate logic into a new _parse_model_dirs() helper (routes/cookbook_helpers.py, alongside _validate_local_dir and _cached_model_scan_script, which it now composes) so both endpoints share identical validation: shell metacharacters and leading-dash (option injection) segments are rejected with a 400 instead of reaching the generated scanner. Paths were already embedded via Python repr rather than shell-interpolated, so this was not an RCE — but the inconsistency meant invalid paths could still trigger unintended filesystem walks. The route handler is now a one-line call: model_dirs = _parse_model_dirs(model_dir). Fixes #5508 --- routes/cookbook_helpers.py | 27 +++++++++++++++++++++++++++ routes/cookbook_routes.py | 10 ++-------- tests/test_cookbook_helpers.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/routes/cookbook_helpers.py b/routes/cookbook_helpers.py index a4e29853b4..63329fd620 100644 --- a/routes/cookbook_helpers.py +++ b/routes/cookbook_helpers.py @@ -372,6 +372,33 @@ def _user_shell_path_bootstrap() -> list[str]: ] +def _parse_model_dirs(model_dir: str | None) -> list[str]: + """Split the comma-separated ``model_dir`` query param for /api/model/cached + into individual, validated directory paths. + + Bare paths users paste without a leading slash (``home/...``, ``mnt/...``, + etc.) are prefixed with ``/`` first, matching the convention the generated + scanner's own ``normalize_model_dir`` uses. Each resulting segment is then + run through :func:`_validate_local_dir` — the same validator the sibling + ``/api/model/download`` endpoint applies to ``local_dir`` — so an invalid + path (shell metacharacters, a segment starting with ``-``) is rejected + with a 400 instead of silently reaching the generated scan script. + """ + if not model_dir: + return [] + result = [] + for segment in model_dir.split(","): + segment = segment.strip() + if not segment: + continue + if segment.startswith(("home/", "mnt/", "media/", "data/", "opt/", "srv/", "var/")): + segment = "/" + segment + validated = _validate_local_dir(segment) + if validated: + result.append(validated) + return result + + def _cached_model_scan_script(model_dirs: list[str] | None = None, add_hf_cache: str | None = None) -> str: """Build the standalone Python scanner used by /api/model/cached. Allows for an additional HuggingFace cache path to be scanned (i.e. Windows HF cache for local WSL envs.) diff --git a/routes/cookbook_routes.py b/routes/cookbook_routes.py index b64f1d3c36..499a8c50e5 100644 --- a/routes/cookbook_routes.py +++ b/routes/cookbook_routes.py @@ -52,6 +52,7 @@ _ps_squote, _bash_squote, _validate_serve_cmd, _parse_serve_phase, OLLAMA_MISSING_HINT, _safe_env_prefix, _local_tooling_path_export, _append_serve_preflight_exit_lines, _append_serve_exit_code_lines, _append_llama_cpp_linux_accel_build_lines, _cached_model_scan_script, + _parse_model_dirs, load_stored_hf_token, _append_vllm_linux_preflight_lines, _ollama_bind_from_cmd, _pip_install_fallback_chain, _pip_install_no_cache, _user_shell_path_bootstrap, _venv_safe_local_pip_install_cmd, @@ -1353,14 +1354,7 @@ async def model_cached(request: Request, host: str | None = None, model_dir: str ssh_port = validate_ssh_port(ssh_port) TMUX_LOG_DIR.mkdir(parents=True, exist_ok=True) - model_dirs = [] - if model_dir: - for d in model_dir.split(','): - d = d.strip() - if d: - if d.startswith(("home/", "mnt/", "media/", "data/", "opt/", "srv/", "var/")): - d = "/" + d - model_dirs.append(d) + model_dirs = _parse_model_dirs(model_dir) paths_code = _cached_model_scan_script(model_dirs) scan_py = TMUX_LOG_DIR / "scan_cache.py" diff --git a/tests/test_cookbook_helpers.py b/tests/test_cookbook_helpers.py index bf6c47d4b1..f00c71a9e4 100644 --- a/tests/test_cookbook_helpers.py +++ b/tests/test_cookbook_helpers.py @@ -16,6 +16,7 @@ _llama_cpp_rebuild_cmd, _append_vllm_linux_preflight_lines, _local_tooling_path_export, + _parse_model_dirs, _pip_install_attempt, _pip_install_fallback_chain, _ollama_bind_from_cmd, @@ -190,6 +191,35 @@ def test_validate_local_dir_rejects_leading_dash_segments(): _validate_local_dir(path) +def test_parse_model_dirs_returns_empty_for_none_or_blank(): + assert _parse_model_dirs(None) == [] + assert _parse_model_dirs("") == [] + assert _parse_model_dirs(" , ,") == [] + + +def test_parse_model_dirs_splits_and_validates_each_segment(): + assert _parse_model_dirs("/models/a, /models/b") == ["/models/a", "/models/b"] + + +def test_parse_model_dirs_prefixes_bare_unix_style_paths(): + # Users often paste Linux absolute paths without the leading slash. + assert _parse_model_dirs("home/user/models") == ["/home/user/models"] + assert _parse_model_dirs("mnt/data/models,srv/models") == ["/mnt/data/models", "/srv/models"] + + +def test_parse_model_dirs_rejects_shell_metacharacters_like_download_local_dir(): + # Same validator the sibling /api/model/download endpoint applies to + # local_dir — a segment with shell metacharacters must 400, not reach + # the generated scan script. + with pytest.raises(HTTPException): + _parse_model_dirs("/models/a; touch /tmp/pwned") + + +def test_parse_model_dirs_rejects_leading_dash_segments(): + with pytest.raises(HTTPException): + _parse_model_dirs("/models/-rf") + + def test_validate_gpus_accepts_indexes_only(): assert _validate_gpus("0,1,2") == "0,1,2" with pytest.raises(HTTPException):