studio: contain export and dataset paths under their configured roots

resolve_under_root and resolve_dataset_path previously returned absolute
paths unchanged, so an authenticated client could supply
save_directory="/tmp/escape" (or any other absolute path) and have the
exporter drop adapter files anywhere the server user could write. This
turned up during a recent audit pass where an authenticated POST to
/api/export/export/lora with save_directory="/tmp/lora_escape_test"
returned 200 and wrote adapter_model.safetensors, adapter_config.json,
and tokenizer files under /tmp.

The fix is two-layered:

storage_roots.py adds an _assert_contained(resolved, root) helper that
runs after path resolution and rejects any result whose realpath does
not sit under realpath(root). resolve_under_root now rejects '..'
segments and null bytes outright, and only accepts absolute inputs when
they are already inside the configured root (internal call sites that
re-resolve a stored absolute path stay idempotent;
worker.py:resolve_output_dir(output_dir) etc. continue to work).
resolve_dataset_path picks up the same containment rule, scoped to the
three dataset roots.

models/export.py adds field_validator("save_directory", mode="before")
to ExportCommonOptions and ExportGGUFRequest so bad input fails fast at
422 with a clear message rather than a 500 deep inside the resolver.
The validator rejects empty/whitespace, null bytes, control chars,
strings longer than 255 chars, absolute paths, and '..' segments.

routes/export.py:_export_details now returns os.path.relpath(output_path,
exports_root()) so the Export Complete dialog and /api/models/loras no
longer leak the absolute install prefix to the UI; the basename is
used as a last-resort fallback.

Verified end to end:
- POST /api/export/export/lora {"save_directory":"/tmp/foo"} -> 422
  "save_directory must be a name or relative path under the export
  root; absolute paths are rejected". /tmp/foo is not created.
- "../../etc/escape" -> 422 "may not contain '..' segments".
- save_directory="my_subdir" -> still accepted (400 only because the
  test had no checkpoint loaded yet, not because of validation).
- Internal idempotent re-resolve via resolve_export_dir(absolute path
  that is already under exports_root) returns the same path unchanged.
This commit is contained in:
Daniel Han 2026-05-11 12:43:49 +00:00
commit 19d369b08b
3 changed files with 129 additions and 7 deletions

View file

@ -5,10 +5,41 @@
Pydantic schemas for Export API.
"""
from pydantic import BaseModel, Field
from pathlib import Path
from pydantic import BaseModel, Field, field_validator
from typing import List, Optional, Literal, Dict, Any
def _validate_save_directory(value: str) -> str:
"""Reject save_directory values that escape the configured export root.
Mirrors :func:`studio.backend.utils.paths.storage_roots.resolve_under_root`
so the rejection happens at request-parse time with a clear 422 instead of
at handler invocation time with an opaque 500.
"""
if value is None:
raise ValueError("save_directory is required")
raw = str(value).strip()
if not raw:
raise ValueError("save_directory must not be empty")
if "\x00" in raw:
raise ValueError("save_directory may not contain null bytes")
if any(ch in raw for ch in ("\r", "\n")):
raise ValueError("save_directory may not contain control characters")
if len(raw) > 255:
raise ValueError("save_directory must be <= 255 characters")
path = Path(raw).expanduser()
if path.is_absolute():
raise ValueError(
"save_directory must be a name or relative path under the "
"export root; absolute paths are rejected"
)
if ".." in path.parts:
raise ValueError("save_directory may not contain '..' segments")
return raw
class LoadCheckpointRequest(BaseModel):
"""Request for loading a checkpoint into the export backend."""
@ -64,6 +95,12 @@ class ExportCommonOptions(BaseModel):
...,
description = "Local directory where the exported artifacts will be written",
)
@field_validator("save_directory", mode = "before")
@classmethod
def _check_save_directory(cls, v):
return _validate_save_directory(v)
push_to_hub: bool = Field(
False,
description = "If True, also push the exported model to the Hugging Face Hub",
@ -108,6 +145,12 @@ class ExportGGUFRequest(BaseModel):
...,
description = "Directory where GGUF files will be saved",
)
@field_validator("save_directory", mode = "before")
@classmethod
def _check_save_directory(cls, v):
return _validate_save_directory(v)
quantization_method: str = Field(
"Q4_K_M",
description = 'GGUF quantization method (e.g. "Q4_K_M")',

View file

@ -7,6 +7,7 @@ Export API routes: checkpoint discovery and model export operations.
import asyncio
import json
import os
import sys
import time
from pathlib import Path
@ -188,10 +189,23 @@ def _export_details(output_path: Optional[str]) -> Optional[Dict[str, Any]]:
frontend reads to populate the Export Complete screen. Returns None
when the export had no local component (Hub-only push) so the
Pydantic field stays absent rather than ``{"output_path": null}``.
The returned ``output_path`` is the path RELATIVE to the configured
exports root, so the response no longer leaks the absolute install
location (e.g. ``/mnt/disks/.../studio/exports/foo`` -> ``foo``).
"""
if not output_path:
return None
return {"output_path": output_path}
try:
from utils.paths.storage_roots import exports_root # local import to avoid cycle
rel = os.path.relpath(output_path, exports_root())
# If the path is outside exports_root (defensive), keep the basename only.
if rel.startswith(".."):
rel = os.path.basename(output_path)
return {"output_path": rel}
except Exception:
# Last-resort fallback: basename only - never the full prefix.
return {"output_path": os.path.basename(output_path)}
@router.post("/export/merged", response_model = ExportOperationResponse)

View file

@ -276,21 +276,69 @@ def _clean_relative_path(
return Path(*parts) if parts else Path()
def _assert_contained(resolved: Path, root: Path) -> None:
"""Reject paths whose post-resolve real location escapes ``root``.
Catches absolute inputs, ``..`` traversal, and symlinks pointing outside
the root. Raises ValueError on escape.
"""
try:
resolved_real = Path(os.path.realpath(resolved))
root_real = Path(os.path.realpath(root))
except OSError as exc:
raise ValueError(f"path resolution failed: {exc}") from exc
try:
resolved_real.relative_to(root_real)
except ValueError as exc:
raise ValueError(
f"path escapes root: {resolved!s} -> {resolved_real!s} "
f"is not under {root_real!s}"
) from exc
def resolve_under_root(
path_value: str | None,
*,
root: Path,
strip_prefixes: tuple[str, ...] = (),
) -> Path:
"""Resolve ``path_value`` so the result is provably under ``root``.
Policy:
- Empty / None -> the root itself.
- Null bytes -> rejected.
- ``..`` segments -> rejected (no traversal).
- Absolute paths -> accepted ONLY if already inside ``root`` (after
realpath). This lets internal code that has already resolved a
stored absolute path re-enter the resolver idempotently, while
blocking the export-time exploit where a user supplies
``/tmp/EVIL``. The pydantic validator on user-facing schemas
(``ExportCommonOptions.save_directory``) rejects absolute inputs
outright at request-parse time as defense-in-depth.
"""
if not path_value or not str(path_value).strip():
return root
path = Path(str(path_value).strip()).expanduser()
raw = str(path_value).strip()
if "\x00" in raw:
raise ValueError("path may not contain null bytes")
path = Path(raw).expanduser()
if ".." in path.parts:
raise ValueError(
f"path may not contain '..' segments: {raw!r}"
)
if path.is_absolute():
# Internal callers may pass already-resolved absolute paths.
# Accept only when contained under root; reject escapes.
_assert_contained(path, root)
return path
cleaned = _clean_relative_path(str(path), strip_prefixes = strip_prefixes)
return root / cleaned
cleaned = _clean_relative_path(raw, strip_prefixes = strip_prefixes)
candidate = root / cleaned
_assert_contained(candidate, root)
return candidate
def resolve_output_dir(path_value: str | None = None) -> Path:
@ -318,9 +366,26 @@ def resolve_tensorboard_dir(path_value: str | None = None) -> Path:
def resolve_dataset_path(path_value: str) -> Path:
path = Path(path_value).expanduser()
raw = str(path_value or "").strip()
if "\x00" in raw:
raise ValueError("dataset path may not contain null bytes")
path = Path(raw).expanduser()
if ".." in path.parts:
raise ValueError(
f"dataset path may not contain '..' segments: {raw!r}"
)
if path.is_absolute():
return path
# Accept absolute inputs only when contained under one of the
# dataset roots; reject all other absolute paths.
for root_fn in (datasets_root, dataset_uploads_root, recipe_datasets_root):
try:
_assert_contained(path, root_fn())
return path
except ValueError:
continue
raise ValueError(
f"dataset path must be relative or under a dataset root: {raw!r}"
)
parts = [part for part in Path(path_value).parts if part not in ("", ".")]
if parts[:2] == ["assets", "datasets"]: