diff --git a/routes/cookbook_helpers.py b/routes/cookbook_helpers.py index 836b26c8a..bba10e6e8 100644 --- a/routes/cookbook_helpers.py +++ b/routes/cookbook_helpers.py @@ -577,6 +577,16 @@ _SERVE_CMD_ALLOWLIST = { _GGUF_PRELUDE_RE = re.compile( r'^MODEL_FILE=\$\([^\n]*?\)\s*&&\s*\{[^{}]*\}\s*\|\|\s*\{[^{}]*\}\s*&&\s*' ) +_SAFE_SUBSHELL_TEXT = r"[^'\n;&|`$()<>]+" +_SAFE_SUBSHELL_DQ_HOME_PATH = r'"\$HOME/[^"\n;&|`()<>]*"' +_SAFE_PRINTF_SUBSHELL_RE = re.compile( + rf"^\$\(printf[ \t]+%s[ \t]+(?:'{_SAFE_SUBSHELL_TEXT}'|\$\{{HOME\}}'/{_SAFE_SUBSHELL_TEXT}')\)$" +) +_SAFE_FIND_MMPROJ_SUBSHELL_RE = re.compile( + rf"^\$\(find[ \t]+(?:'{_SAFE_SUBSHELL_TEXT}'|{_SAFE_SUBSHELL_DQ_HOME_PATH}|{_SAFE_SUBSHELL_TEXT})" + r"[ \t]+-iname[ \t]+'mmproj\*\.gguf'" + r"(?:[ \t]+2>/dev/null)?[ \t]*\|[ \t]*sort[ \t]*\|[ \t]*head[ \t]+-1\)$" +) _OLLAMA_HOST_ASSIGNMENT_RE = re.compile(r"(?:^|\s)OLLAMA_HOST=([^\s]+)") _OLLAMA_BIND_RE = re.compile(r"^\[([^\]]+)\]:(\d+)$|^([^:]+):(\d+)$") _OLLAMA_BIND_HOST_RE = re.compile(r"^[A-Za-z0-9._:-]+$") @@ -677,6 +687,13 @@ def _check_serve_binary(seg: str) -> None: ) +def _is_safe_serve_subshell(subshell: str) -> bool: + return bool( + _SAFE_PRINTF_SUBSHELL_RE.fullmatch(subshell) + or _SAFE_FIND_MMPROJ_SUBSHELL_RE.fullmatch(subshell) + ) + + def _validate_serve_cmd(v: str | None) -> str | None: """Reject serve commands that aren't in the allowlist or contain shell metachars. @@ -708,15 +725,15 @@ def _validate_serve_cmd(v: str | None) -> str | None: _check_serve_binary(part.strip()) return v - # Otherwise: a single invocation — no shell metacharacters allowed. - # Temporarily replace safe $(printf %s ...) expressions with a placeholder - # to avoid triggering the metacharacter/command-injection checks. - cleaned_v = v - printf_matches = list(re.finditer(r"\$\(\s*printf\s+%s\s+([^\n()]*?)\)", v)) - for match in printf_matches: - inner = match.group(1) - if not any(c in inner for c in (";", "&&", "||", "$(", "`")): - cleaned_v = cleaned_v.replace(match.group(0), "/placeholder/safe/path.gguf") + # Otherwise: a single invocation — no shell metacharacters allowed. Replace + # only the exact command substitutions emitted by the Cookbook UI: + # $(printf %s 'safe-path') and the mmproj lookup + # $(find -iname 'mmproj*.gguf' 2>/dev/null | sort | head -1). + def _replace_safe_subshell(match: re.Match[str]) -> str: + subshell = match.group(0) + return "/placeholder/safe/path" if _is_safe_serve_subshell(subshell) else subshell + + cleaned_v = re.sub(r"\$\([^()]*\)", _replace_safe_subshell, v) # (`$(` was the original intent; bare `$` is fine for shell-safe paths.) if any(c in cleaned_v for c in (";", "&&", "||", "$(")): diff --git a/tests/test_cookbook_helpers.py b/tests/test_cookbook_helpers.py index be684b177..bf6c47d4b 100644 --- a/tests/test_cookbook_helpers.py +++ b/tests/test_cookbook_helpers.py @@ -917,3 +917,42 @@ def test_cached_model_scan_runs_additional_hf_cache(tmp_path): assert rec["size_bytes"] == len(b"abc123") assert rec["has_incomplete"] is False assert rec["is_diffusion"] is False + + +def test_validate_serve_cmd_accepts_find_subshell_for_mmproj(): + """$(find …) for mmproj path should be accepted, same as $(printf %s …).""" + cmd = ( + "HIP_VISIBLE_DEVICES=0 llama-server " + "--model \"$(printf %s '/app/.cache/huggingface/hub/models--unsloth--gemma-4-E2B-it-GGUF" + "/snapshots/90f9618340396838ee7ff5b0ba2da27da62953d3/gemma-4-E2B-it-Q4_K_M.gguf')\" " + "--host 0.0.0.0 --port 8000 -ngl 99 -c 131072 " + "--flash-attn on --cache-type-k q8_0 --cache-type-v q8_0 " + "--mmproj \"$(find '/app/.cache/huggingface/hub/models--unsloth--gemma-4-E2B-it-GGUF" + "/snapshots' -iname 'mmproj*.gguf' 2>/dev/null | sort | head -1)\" " + "--image-max-tokens 1024" + ) + assert _validate_serve_cmd(cmd) == cmd + + +def test_validate_serve_cmd_rejects_unrelated_subshells(): + for cmd in [ + "llama-server --model \"$(curl https://example.invalid/model.gguf)\" --host 0.0.0.0 --port 8000", + "llama-server --model \"$(rm -rf /tmp/not-a-model)\" --host 0.0.0.0 --port 8000", + ]: + with pytest.raises(HTTPException): + _validate_serve_cmd(cmd) + + +def test_validate_serve_cmd_rejects_unrelated_subshell_pipelines(): + for cmd in [ + ( + "llama-server --model model.gguf " + "--mmproj \"$(find '/app/models' -iname 'mmproj*.gguf' | xargs head -1)\"" + ), + ( + "llama-server --model model.gguf " + "--mmproj \"$(find '/app/models' -iname '*.gguf' 2>/dev/null | sort | head -1)\"" + ), + ]: + with pytest.raises(HTTPException): + _validate_serve_cmd(cmd)