mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-28 23:52:09 -04:00
fix(cookbook): accept $(find) subshells in serve command validation
Allow the generated Cookbook mmproj lookup command substitution while keeping serve-command validation constrained to explicit safe subshell patterns. Preserves the existing safe printf substitution, allowlists the generated find/sort/head mmproj lookup shape, and adds negative regression coverage for unrelated substitutions and pipelines. Fixes #4772.
This commit is contained in:
@@ -577,6 +577,16 @@ _SERVE_CMD_ALLOWLIST = {
|
|||||||
_GGUF_PRELUDE_RE = re.compile(
|
_GGUF_PRELUDE_RE = re.compile(
|
||||||
r'^MODEL_FILE=\$\([^\n]*?\)\s*&&\s*\{[^{}]*\}\s*\|\|\s*\{[^{}]*\}\s*&&\s*'
|
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_HOST_ASSIGNMENT_RE = re.compile(r"(?:^|\s)OLLAMA_HOST=([^\s]+)")
|
||||||
_OLLAMA_BIND_RE = re.compile(r"^\[([^\]]+)\]:(\d+)$|^([^:]+):(\d+)$")
|
_OLLAMA_BIND_RE = re.compile(r"^\[([^\]]+)\]:(\d+)$|^([^:]+):(\d+)$")
|
||||||
_OLLAMA_BIND_HOST_RE = re.compile(r"^[A-Za-z0-9._:-]+$")
|
_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:
|
def _validate_serve_cmd(v: str | None) -> str | None:
|
||||||
"""Reject serve commands that aren't in the allowlist or contain shell metachars.
|
"""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())
|
_check_serve_binary(part.strip())
|
||||||
return v
|
return v
|
||||||
|
|
||||||
# Otherwise: a single invocation — no shell metacharacters allowed.
|
# Otherwise: a single invocation — no shell metacharacters allowed. Replace
|
||||||
# Temporarily replace safe $(printf %s ...) expressions with a placeholder
|
# only the exact command substitutions emitted by the Cookbook UI:
|
||||||
# to avoid triggering the metacharacter/command-injection checks.
|
# $(printf %s 'safe-path') and the mmproj lookup
|
||||||
cleaned_v = v
|
# $(find <path> -iname 'mmproj*.gguf' 2>/dev/null | sort | head -1).
|
||||||
printf_matches = list(re.finditer(r"\$\(\s*printf\s+%s\s+([^\n()]*?)\)", v))
|
def _replace_safe_subshell(match: re.Match[str]) -> str:
|
||||||
for match in printf_matches:
|
subshell = match.group(0)
|
||||||
inner = match.group(1)
|
return "/placeholder/safe/path" if _is_safe_serve_subshell(subshell) else subshell
|
||||||
if not any(c in inner for c in (";", "&&", "||", "$(", "`")):
|
|
||||||
cleaned_v = cleaned_v.replace(match.group(0), "/placeholder/safe/path.gguf")
|
cleaned_v = re.sub(r"\$\([^()]*\)", _replace_safe_subshell, v)
|
||||||
|
|
||||||
# (`$(` was the original intent; bare `$` is fine for shell-safe paths.)
|
# (`$(` was the original intent; bare `$` is fine for shell-safe paths.)
|
||||||
if any(c in cleaned_v for c in (";", "&&", "||", "$(")):
|
if any(c in cleaned_v for c in (";", "&&", "||", "$(")):
|
||||||
|
|||||||
@@ -917,3 +917,42 @@ def test_cached_model_scan_runs_additional_hf_cache(tmp_path):
|
|||||||
assert rec["size_bytes"] == len(b"abc123")
|
assert rec["size_bytes"] == len(b"abc123")
|
||||||
assert rec["has_incomplete"] is False
|
assert rec["has_incomplete"] is False
|
||||||
assert rec["is_diffusion"] 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)
|
||||||
|
|||||||
Reference in New Issue
Block a user