From 554c289538ff53e59b02e9fa26d774ba867cffa4 Mon Sep 17 00:00:00 2001 From: Anmol Mishra Date: Fri, 12 Jun 2026 16:22:57 +0530 Subject: [PATCH] fix: respect absolute export paths to prevent cross-drive copy failures (WinError 112) (#6088) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: allow absolute save_directory in export paths to prevent cross-drive copy failures The GGUF export pipeline (and all other export flows) forced every save_directory through resolve_export_dir(), which always resolved the path under exports_root() — typically ~/.unsloth/studio/exports/ on the system drive (C: on Windows). When a user selected an output directory on a different drive (E:): 1. The absolute path was rejected at the Pydantic validator level. 2. Even if it got through, resolve_export_dir would re-resolve it under C:\Users\.unsloth\studio\exports\. 3. After GGUF conversion completed on E:, the relocation step would try to move/copy the finished files to C:, causing: - WinError 17 (cross-drive move failure when shutil.move falls through to a cross-filesystem copy) - WinError 112 (disk full on C:) Fix both layers: - _validate_save_directory: accept absolute paths (they represent an explicit user choice of output location). - resolve_export_dir, resolve_output_dir, resolve_tensorboard_dir: return absolute paths as-is instead of forcing them under the default root. Keep the existing safety checks (null bytes, '..' segments) and fall through to resolve_under_root for relative paths. Fixes: https://github.com/unslothai/unsloth/issues/6082 * refactor: centralize user path validation into _resolve_user_path helper Addresses code review feedback: the null-byte, '..', and absolute-path checks were duplicated across resolve_output_dir, resolve_export_dir, and resolve_tensorboard_dir. Extract a single _resolve_user_path helper that all three delegate to. No behavioral change — pure consolidation. * fix: address code review — contain destructive cleanup and scope absolute paths Address all review feedback from gemini-code-assist: 1. P1: destructive subdirectory cleanup (export_gguf) The flattening loop in export_gguf previously rmtree'd every subdirectory under abs_save_dir. When targeting an existing user directory on a different drive (#6082), this could nuke unrelated subdirectories. Now snapshot existing subdirectories before the export and only clean up dirs created during this run. 2. P2: keep scan/read endpoints contained Only resolve_export_dir accepts absolute paths (export is a write path where user picks location). Reverted resolve_output_dir and resolve_tensorboard_dir to use resolve_under_root directly — these are used by scan/read/training endpoints that must stay contained under their respective roots. 3. Centralization feedback Removed the _resolve_user_path helper since it's no longer needed with the narrowed scope. resolve_export_dir has the absolute path logic inline with a clear docstring. * fix: skip pre-existing subdirs in GGUF flatten loop and clean stale export intermediates Two issues caught in code review (chatgpt-codex-connector): 1. The flattening loop moved ALL .gguf files from ALL subdirectories into abs_save_dir, including pre-existing unrelated user subdirs. Now skip pre-existing subdirs entirely unless they are known export-owned intermediates (model/, model_gguf/). 2. After a failed export, known export-owned subdirectories (model/, model_gguf/) were snapshotted as pre-existing on retry and never cleaned up. These are now always cleaned up regardless, since they are known intermediates created by the export pipeline. * fix: separate write vs read export paths, guard same-dir rmtree Three issues caught in code review (chatgpt-codex-connector): 1. P1: scan endpoint containment resolve_export_dir was changed to accept absolute paths, but it's also used by scan/read endpoints (routes/models.py) that must stay contained under exports_root(). Split into: - resolve_export_dir: contained, used by scans - resolve_export_write_dir: accepts absolute paths, used by export backend only 2. P1: same-directory rmtree When a non-PEFT checkpoint's gguf_dir resolves to the same path as abs_save_dir (user selected the checkpoint's gguf output as their export directory), shutil.rmtree(gguf_dir) would delete the user's chosen output directory. Now skip relocation when both paths resolve to the same location. 3. P1: pre-existing subdir flatten loop Reverted _EXPORT_OWNED_SUBDIRS logic — 'model/' and 'model_gguf/' are common directory names in shared model folders and don't prove export ownership. Now only clean up subdirs that didn't exist before the export started. * fix: remove dead _EXPORT_OWNED_SUBDIRS and fix _export_details for absolute paths Two fixes from review comments: 1. Remove unused _EXPORT_OWNED_SUBDIRS declaration (leftover from previous iteration that was intentionally removed). 2. _export_details now returns the full absolute path when the export target is outside exports_root(), instead of truncating to basename. Users who export to E:\ can now see the full destination path in the success dialog. * fix: use unique tmp dir for GGUF intermediates to avoid overwriting user dirs When exporting to an absolute destination that already contains a model/ subdirectory (e.g. a shared models folder), the hard-coded model_save_path would overwrite files in that unrelated directory. Use _tmp_model_ as the intermediate path instead, so user directories are never touched. The tmp dir is created as a new subdir of abs_save_dir and cleaned up by the flatten loop after GGUF files are relocated. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix GGUF local export paths for PR #6088 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address GGUF export follow-ups for PR #6088 * Clean GGUF temp dirs on export failure for PR #6088 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix/adjust export path tests for PR #6088 * Fix/adjust export path review findings for PR #6088 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix/adjust home export path handling for PR #6088 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: wasimysaid --- studio/backend/core/export/export.py | 37 +- studio/backend/models/export.py | 20 +- studio/backend/routes/export.py | 30 +- .../tests/test_export_absolute_paths.py | 472 ++++++++++++++++++ studio/backend/utils/paths/__init__.py | 4 +- studio/backend/utils/paths/storage_roots.py | 56 ++- .../export/components/export-dialog.tsx | 113 ++++- .../src/features/export/export-page.tsx | 90 +++- 8 files changed, 768 insertions(+), 54 deletions(-) create mode 100644 studio/backend/tests/test_export_absolute_paths.py diff --git a/studio/backend/core/export/export.py b/studio/backend/core/export/export.py index d5a3de06df..b28b61f088 100644 --- a/studio/backend/core/export/export.py +++ b/studio/backend/core/export/export.py @@ -18,7 +18,12 @@ from utils.hardware import clear_gpu_cache from utils.models import is_vision_model, get_base_model_from_lora from utils.models.model_config import detect_audio_type -from utils.paths import ensure_dir, outputs_root, resolve_export_dir, resolve_output_dir +from utils.paths import ( + ensure_dir, + outputs_root, + resolve_export_write_dir, + resolve_output_dir, +) from core.inference import get_inference_backend # GPU-only imports — guarded for Apple Silicon where these aren't needed @@ -336,7 +341,7 @@ class ExportBackend: save_method = "merged_16bit" if save_directory: - save_directory = str(resolve_export_dir(save_directory)) + save_directory = str(resolve_export_write_dir(save_directory)) logger.info(f"Saving merged model locally to: {save_directory}") ensure_dir(Path(save_directory)) @@ -436,7 +441,7 @@ class ExportBackend: output_path: Optional[str] = None try: if save_directory: - save_directory = str(resolve_export_dir(save_directory)) + save_directory = str(resolve_export_write_dir(save_directory)) logger.info(f"Saving base model locally to: {save_directory}") ensure_dir(Path(save_directory)) @@ -563,6 +568,7 @@ class ExportBackend: return False, "No model loaded. Please select a checkpoint first.", None output_path: Optional[str] = None + model_tmp_to_cleanup: Optional[str] = None try: # unsloth expects lowercase quant method quant_method = quantization_method.lower() @@ -588,9 +594,8 @@ class ExportBackend: _LLAMA_CPP_SCRIPTS_WARNING_EMITTED = True if save_directory: - save_directory = str(resolve_export_dir(save_directory)) - # Absolute path so unsloth's relative-path internals resolve - # against the repo root cwd, not the export directory. + save_directory = str(resolve_export_write_dir(save_directory)) + # Keep unsloth relative-path internals anchored to the repo cwd. abs_save_dir = os.path.abspath(save_directory) logger.info(f"Saving GGUF model locally to: {abs_save_dir}") @@ -604,9 +609,15 @@ class ExportBackend: cwd = os.getcwd() pre_existing_ggufs = set(glob.glob(os.path.join(cwd, "*.gguf"))) - model_save_path = os.path.join(abs_save_dir, "model") + pre_existing_subs = {d.name for d in Path(abs_save_dir).iterdir() if d.is_dir()} + + # Avoid clobbering an existing user-owned model/ directory. + import uuid + + _model_tmp = os.path.join(abs_save_dir, f"_tmp_model_{uuid.uuid4().hex[:8]}") + model_tmp_to_cleanup = _model_tmp self.current_model.save_pretrained_gguf( - model_save_path, + _model_tmp, self.current_tokenizer, quantization_method = quant_method, ) @@ -618,10 +629,12 @@ class ExportBackend: shutil.move(src, dest) logger.info(f"Relocated GGUF: {os.path.basename(src)} → {abs_save_dir}/") - # Flatten any .gguf from subdirs (e.g. model_gguf/) into abs_save_dir. + # Flatten GGUF files from subdirs created during this export. for sub in list(Path(abs_save_dir).iterdir()): if not sub.is_dir(): continue + if sub.name in pre_existing_subs: + continue for src in sub.glob("*.gguf"): dest = os.path.join(abs_save_dir, src.name) shutil.move(str(src), dest) @@ -634,7 +647,7 @@ class ExportBackend: if self.current_checkpoint: ckpt = Path(self.current_checkpoint) gguf_dir = ckpt.parent / f"{ckpt.name}_gguf" - if gguf_dir.is_dir(): + if gguf_dir.is_dir() and gguf_dir.resolve() != Path(abs_save_dir).resolve(): for src in gguf_dir.glob("*.gguf"): dest = os.path.join(abs_save_dir, src.name) shutil.move(str(src), dest) @@ -683,6 +696,8 @@ class ExportBackend: ) except Exception as e: + if model_tmp_to_cleanup: + shutil.rmtree(model_tmp_to_cleanup, ignore_errors = True) logger.error(f"Error exporting GGUF model: {e}") import traceback @@ -712,7 +727,7 @@ class ExportBackend: output_path: Optional[str] = None try: if save_directory: - save_directory = str(resolve_export_dir(save_directory)) + save_directory = str(resolve_export_write_dir(save_directory)) logger.info(f"Saving LoRA adapter locally to: {save_directory}") ensure_dir(Path(save_directory)) diff --git a/studio/backend/models/export.py b/studio/backend/models/export.py index 0d54b435a6..584f82dea0 100644 --- a/studio/backend/models/export.py +++ b/studio/backend/models/export.py @@ -3,14 +3,14 @@ """Pydantic schemas for Export API.""" -from pathlib import Path +from pathlib import Path, PureWindowsPath 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 export root.""" + """Validate save_directory — allows absolute paths (user may want a different drive).""" if value is None: raise ValueError("save_directory is required") raw = str(value).strip() @@ -20,15 +20,15 @@ def _validate_save_directory(value: str) -> str: 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: + path_parts = (*path.parts, *PureWindowsPath(raw).parts, *raw.replace("\\", "/").split("/")) + if any(len(part) > 255 for part in path_parts if part not in ("", ".", "/", "\\")): + raise ValueError("save_directory path components must be <= 255 characters") + if ( + ".." in path.parts + or ".." in PureWindowsPath(raw).parts + or ".." in raw.replace("\\", "/").split("/") + ): raise ValueError("save_directory may not contain '..' segments") return raw diff --git a/studio/backend/routes/export.py b/studio/backend/routes/export.py index ef37daa158..f7b3a56a71 100644 --- a/studio/backend/routes/export.py +++ b/studio/backend/routes/export.py @@ -154,19 +154,41 @@ async def get_export_status(current_subject: str = Depends(get_current_subject)) ) +def _try_register_external_export(path: Path) -> tuple[bool, Optional[str]]: + """Best-effort registration so absolute exports show up in local scans.""" + try: + from storage.studio_db import add_scan_folder + folder = add_scan_folder(str(path)) + return True, str(folder.get("path") or path) + except Exception as exc: + logger.warning("Could not register export scan folder %s: %s", path, exc) + return False, None + + def _export_details(output_path: Optional[str]) -> Optional[Dict[str, Any]]: - """Return the export path relative to exports_root, hiding the install path.""" + """Return relative export paths, keeping external absolute paths visible.""" if not output_path: return None try: from utils.paths.storage_roots import exports_root + path = Path(output_path) + # If it's outside exports_root, return the full absolute path + # so users can find their files on a different drive. + if path.is_absolute(): + try: + path.resolve().relative_to(exports_root().resolve()) + except ValueError: + registered, registered_path = _try_register_external_export(path) + return { + "output_path": str(path), + "scan_folder_registered": registered, + "scan_folder_path": registered_path, + } rel = os.path.relpath(output_path, exports_root()) - if rel.startswith(".."): - rel = os.path.basename(output_path) return {"output_path": rel} except Exception: - return {"output_path": os.path.basename(output_path)} + return {"output_path": output_path} @router.post("/export/merged", response_model = ExportOperationResponse) diff --git a/studio/backend/tests/test_export_absolute_paths.py b/studio/backend/tests/test_export_absolute_paths.py new file mode 100644 index 0000000000..f333a590c2 --- /dev/null +++ b/studio/backend/tests/test_export_absolute_paths.py @@ -0,0 +1,472 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 + +import importlib.machinery +import importlib.util +import sys +import types +from pathlib import Path + +import pytest + + +_BACKEND_DIR = Path(__file__).resolve().parent.parent + + +def _load_module( + module_name: str, + relative_path: str, + monkeypatch = None, +): + path = _BACKEND_DIR / relative_path + spec = importlib.util.spec_from_file_location(module_name, path) + assert spec is not None + assert spec.loader is not None + module = importlib.util.module_from_spec(spec) + if monkeypatch is None: + sys.modules[module_name] = module + else: + monkeypatch.setitem(sys.modules, module_name, module) + spec.loader.exec_module(module) + return module + + +class _DummyLogger: + def __getattr__(self, _name): + return lambda *args, **kwargs: None + + +class _Router: + def get(self, *args, **kwargs): + return lambda fn: fn + + def post(self, *args, **kwargs): + return lambda fn: fn + + def delete(self, *args, **kwargs): + return lambda fn: fn + + +class _HTTPException(Exception): + def __init__( + self, + status_code: int, + detail: str | None = None, + ): + super().__init__(detail) + self.status_code = status_code + self.detail = detail + + +class _LocalModelInfo: + def __init__(self, **kwargs): + self.__dict__.update(kwargs) + + +def _identity_decorator(*_args, **_kwargs): + return lambda fn: fn + + +def _install_lightweight_backend_stubs(monkeypatch): + fastapi = types.ModuleType("fastapi") + fastapi.APIRouter = lambda: _Router() + fastapi.Body = lambda default = None, **_kwargs: default + fastapi.Depends = lambda dependency = None, **_kwargs: dependency + fastapi.HTTPException = _HTTPException + fastapi.Query = lambda default = None, **_kwargs: default + fastapi.Request = object + monkeypatch.setitem(sys.modules, "fastapi", fastapi) + + fastapi_responses = types.ModuleType("fastapi.responses") + fastapi_responses.StreamingResponse = object + monkeypatch.setitem(sys.modules, "fastapi.responses", fastapi_responses) + + monkeypatch.setitem( + sys.modules, + "structlog", + types.SimpleNamespace( + BoundLogger = _DummyLogger, + get_logger = lambda *args, **kwargs: _DummyLogger(), + ), + ) + loggers = types.ModuleType("loggers") + loggers.get_logger = lambda *args, **kwargs: _DummyLogger() + monkeypatch.setitem(sys.modules, "loggers", loggers) + + auth_pkg = types.ModuleType("auth") + auth_mod = types.ModuleType("auth.authentication") + auth_mod.get_current_subject = lambda: None + monkeypatch.setitem(sys.modules, "auth", auth_pkg) + monkeypatch.setitem(sys.modules, "auth.authentication", auth_mod) + + core_pkg = types.ModuleType("core") + core_export = types.ModuleType("core.export") + core_export.get_export_backend = lambda: None + core_inference = types.ModuleType("core.inference") + core_inference.get_inference_backend = lambda: None + monkeypatch.setitem(sys.modules, "core", core_pkg) + monkeypatch.setitem(sys.modules, "core.export", core_export) + monkeypatch.setitem(sys.modules, "core.inference", core_inference) + + utils_pkg = types.ModuleType("utils") + utils_pkg.__path__ = [] + utils_paths = types.ModuleType("utils.paths") + storage_roots = _load_module( + "utils.paths.storage_roots", + "utils/paths/storage_roots.py", + monkeypatch, + ) + utils_pkg.paths = utils_paths + utils_paths.storage_roots = storage_roots + utils_paths.is_local_path = lambda value: Path(str(value)).is_absolute() + utils_paths.outputs_root = lambda: Path("outputs") + utils_paths.exports_root = storage_roots.exports_root + utils_paths.resolve_cached_repo_id_case = lambda value: value + utils_paths.resolve_output_dir = lambda value = None: Path(value or "outputs") + utils_paths.resolve_export_dir = storage_roots.resolve_export_dir + monkeypatch.setitem(sys.modules, "utils", utils_pkg) + monkeypatch.setitem(sys.modules, "utils.paths", utils_paths) + + utils_utils = types.ModuleType("utils.utils") + utils_utils.log_and_http_error = lambda *args, **kwargs: (_ for _ in ()).throw( + _HTTPException(kwargs.get("status_code", 500), kwargs.get("detail")) + ) + utils_utils.safe_error_detail = lambda value: str(value) + monkeypatch.setitem(sys.modules, "utils.utils", utils_utils) + + utils_models = types.ModuleType("utils.models") + for name in ( + "scan_trained_models", + "scan_exported_models", + "scan_checkpoints", + "list_gguf_variants", + ): + setattr(utils_models, name, lambda *args, **kwargs: []) + for name in ( + "get_base_model_from_checkpoint", + "get_base_model_from_lora", + "load_model_defaults", + ): + setattr(utils_models, name, lambda *args, **kwargs: None) + utils_models.is_vision_model = lambda *args, **kwargs: False + utils_models.is_embedding_model = lambda *args, **kwargs: False + utils_models.ModelConfig = object + monkeypatch.setitem(sys.modules, "utils.models", utils_models) + + utils_model_config = types.ModuleType("utils.models.model_config") + utils_model_config._pick_best_gguf = lambda variants: variants[0] if variants else None + utils_model_config._extract_quant_label = lambda value: value + utils_model_config.is_audio_input_type = lambda *args, **kwargs: None + monkeypatch.setitem( + sys.modules, + "utils.models.model_config", + utils_model_config, + ) + + models_pkg = types.ModuleType("models") + models_pkg.__path__ = [] + for name in ( + "CheckpointInfo", + "CheckpointListResponse", + "LocalModelListResponse", + "ModelCheckpoints", + "ModelDetails", + "LoRAScanResponse", + "LoRAInfo", + "ModelListResponse", + "LoadCheckpointRequest", + "ExportStatusResponse", + "ExportOperationResponse", + "ExportMergedModelRequest", + "ExportBaseModelRequest", + "ExportGGUFRequest", + "ExportLoRAAdapterRequest", + ): + setattr(models_pkg, name, object) + models_pkg.LocalModelInfo = _LocalModelInfo + monkeypatch.setitem(sys.modules, "models", models_pkg) + + models_models = types.ModuleType("models.models") + for name in ( + "BrowseEntry", + "BrowseFoldersResponse", + "GgufVariantDetail", + "GgufVariantsResponse", + "ScanFolderInfo", + "AddScanFolderRequest", + ): + setattr(models_models, name, object) + models_models.ModelType = str + monkeypatch.setitem(sys.modules, "models.models", models_models) + + models_responses = types.ModuleType("models.responses") + for name in ( + "LoRABaseModelResponse", + "VisionCheckResponse", + "EmbeddingCheckResponse", + ): + setattr(models_responses, name, object) + monkeypatch.setitem(sys.modules, "models.responses", models_responses) + + +def _install_pydantic_stub(monkeypatch): + pydantic = types.ModuleType("pydantic") + pydantic.BaseModel = object + pydantic.Field = lambda default = None, **_kwargs: default + pydantic.field_validator = _identity_decorator + monkeypatch.setitem(sys.modules, "pydantic", pydantic) + + +def _install_export_backend_stubs(monkeypatch): + _install_lightweight_backend_stubs(monkeypatch) + + unsloth = types.ModuleType("unsloth") + unsloth.FastLanguageModel = object + unsloth.FastVisionModel = object + unsloth._IS_MLX = True + unsloth.__spec__ = importlib.machinery.ModuleSpec("unsloth", loader = None) + monkeypatch.setitem(sys.modules, "unsloth", unsloth) + + unsloth_zoo = types.ModuleType("unsloth_zoo") + unsloth_zoo.__path__ = [] + unsloth_zoo.__spec__ = importlib.machinery.ModuleSpec( + "unsloth_zoo", + loader = None, + is_package = True, + ) + llama_cpp = types.ModuleType("unsloth_zoo.llama_cpp") + llama_cpp.LLAMA_CPP_DEFAULT_DIR = str(Path("/tmp/llama.cpp")) + llama_cpp._resolve_local_convert_script = lambda *args, **kwargs: None + llama_cpp.__spec__ = importlib.machinery.ModuleSpec( + "unsloth_zoo.llama_cpp", + loader = None, + ) + monkeypatch.setitem(sys.modules, "unsloth_zoo", unsloth_zoo) + monkeypatch.setitem(sys.modules, "unsloth_zoo.llama_cpp", llama_cpp) + + huggingface_hub = types.ModuleType("huggingface_hub") + huggingface_hub.HfApi = object + huggingface_hub.ModelCard = object + monkeypatch.setitem(sys.modules, "huggingface_hub", huggingface_hub) + + utils_hardware = types.ModuleType("utils.hardware") + utils_hardware.clear_gpu_cache = lambda: None + monkeypatch.setitem(sys.modules, "utils.hardware", utils_hardware) + + utils_models = sys.modules["utils.models"] + utils_models.get_base_model_from_lora = lambda *args, **kwargs: None + utils_models.is_vision_model = lambda *args, **kwargs: False + + utils_model_config = sys.modules["utils.models.model_config"] + utils_model_config.detect_audio_type = lambda *args, **kwargs: None + + utils_paths = sys.modules["utils.paths"] + utils_paths.ensure_dir = lambda path: Path(path).mkdir(parents = True, exist_ok = True) + utils_paths.resolve_export_write_dir = lambda value = None: Path(value or "exports") + utils_paths.resolve_output_dir = lambda value = None: Path(value or "outputs") + + +def test_gguf_export_cleans_temp_dir_when_post_processing_fails(tmp_path, monkeypatch): + _install_export_backend_stubs(monkeypatch) + export_mod = _load_module("test_core_export_backend", "core/export/export.py", monkeypatch) + + cwd = tmp_path / "cwd" + save_dir = tmp_path / "export" + cwd.mkdir() + monkeypatch.chdir(cwd) + monkeypatch.setattr(export_mod, "resolve_export_write_dir", lambda _value: save_dir) + monkeypatch.setattr( + export_mod.shutil, + "move", + lambda *args, **kwargs: (_ for _ in ()).throw(OSError("move failed")), + ) + + class _Model: + def save_pretrained_gguf(self, model_save_path, tokenizer, quantization_method): + Path(model_save_path).mkdir(parents = True) + (Path(model_save_path) / "model.safetensors").write_bytes(b"weights") + (cwd / "converted.gguf").write_bytes(b"gguf") + + backend = export_mod.ExportBackend.__new__(export_mod.ExportBackend) + backend.current_model = _Model() + backend.current_tokenizer = object() + backend.current_checkpoint = None + + success, message, output_path = backend.export_gguf(str(save_dir), "Q4_K_M") + + assert success is False + assert "move failed" in message + assert output_path is None + assert list(save_dir.glob("_tmp_model_*")) == [] + + +def test_save_directory_validator_rejects_windows_parent_segments(monkeypatch): + _install_pydantic_stub(monkeypatch) + export_models = _load_module("test_models_export", "models/export.py", monkeypatch) + + with pytest.raises(ValueError, match = r"\.\."): + export_models._validate_save_directory(r"E:\AI\..\secret") + + +def test_save_directory_validator_allows_deep_absolute_paths(monkeypatch, tmp_path): + _install_pydantic_stub(monkeypatch) + export_models = _load_module("test_models_export_deep_path", "models/export.py", monkeypatch) + + deep_path = tmp_path + for index in range(40): + deep_path /= f"segment-{index:02d}" + raw = str(deep_path) + + assert len(raw) > 255 + assert export_models._validate_save_directory(raw) == raw + + +def test_save_directory_validator_rejects_long_path_component(monkeypatch, tmp_path): + _install_pydantic_stub(monkeypatch) + export_models = _load_module( + "test_models_export_long_component", "models/export.py", monkeypatch + ) + + with pytest.raises(ValueError, match = "path components"): + export_models._validate_save_directory(str(tmp_path / ("a" * 256))) + + +def test_export_write_dir_accepts_external_absolute_but_read_dir_rejects(tmp_path, monkeypatch): + storage_roots = _load_module( + "test_storage_roots_accept_external", + "utils/paths/storage_roots.py", + ) + + export_root = tmp_path / "exports" + external = tmp_path / "external" + export_root.mkdir() + external.mkdir() + monkeypatch.setattr(storage_roots, "exports_root", lambda: export_root) + + assert storage_roots.resolve_export_write_dir(str(external)) == external + + with pytest.raises(ValueError, match = "path escapes root"): + storage_roots.resolve_export_dir(str(external)) + + +def test_export_write_dir_accepts_expanded_home_path(tmp_path, monkeypatch): + storage_roots = _load_module( + "test_storage_roots_accept_home_path", + "utils/paths/storage_roots.py", + ) + + export_root = tmp_path / "exports" + home = tmp_path / "home" + export_root.mkdir() + home.mkdir() + monkeypatch.setattr(storage_roots, "exports_root", lambda: export_root) + if storage_roots.os.name == "nt": + monkeypatch.setenv("USERPROFILE", str(home)) + else: + monkeypatch.setenv("HOME", str(home)) + + assert storage_roots.resolve_export_write_dir("~/exports/model") == home / "exports" / "model" + + +def test_resolve_export_write_dir_rejects_backslash_parent_segment(): + storage_roots = _load_module( + "test_storage_roots_reject_parent", + "utils/paths/storage_roots.py", + ) + + with pytest.raises(ValueError, match = r"\.\."): + storage_roots.resolve_export_write_dir(r"exports\..\outside") + + +def test_export_write_dir_handles_non_native_windows_absolute_as_relative(tmp_path, monkeypatch): + storage_roots = _load_module( + "test_storage_roots_non_native_windows_path", + "utils/paths/storage_roots.py", + ) + + export_root = tmp_path / "exports" + export_root.mkdir() + monkeypatch.setattr(storage_roots, "exports_root", lambda: export_root) + + if storage_roots.os.name == "nt": + pytest.skip("Windows drive paths are native on Windows") + + assert ( + storage_roots.resolve_export_write_dir(r"C:\exports\model") + == export_root / r"C:\exports\model" + ) + + +def test_export_details_registers_external_absolute_output(tmp_path, monkeypatch): + _install_lightweight_backend_stubs(monkeypatch) + export_route = _load_module( + "test_routes_export_external", + "routes/export.py", + monkeypatch, + ) + + output = tmp_path / "Gemma4_26B_gguf" + output.mkdir() + export_root = tmp_path / "studio" / "exports" + export_root.mkdir(parents = True) + registered = [] + + monkeypatch.setattr( + export_route, + "_try_register_external_export", + lambda path: (registered.append(path) is None, str(path)), + ) + monkeypatch.setattr( + "utils.paths.storage_roots.exports_root", + lambda: export_root, + ) + + details = export_route._export_details(str(output)) + + assert details == { + "output_path": str(output), + "scan_folder_registered": True, + "scan_folder_path": str(output), + } + assert registered == [output] + + +def test_export_details_does_not_register_contained_exports(tmp_path, monkeypatch): + _install_lightweight_backend_stubs(monkeypatch) + export_route = _load_module( + "test_routes_export_contained", + "routes/export.py", + monkeypatch, + ) + + export_root = tmp_path / "exports" + output = export_root / "model-gguf" + output.mkdir(parents = True) + + monkeypatch.setattr( + export_route, + "_try_register_external_export", + lambda path: pytest.fail(f"unexpected registration: {path}"), + ) + monkeypatch.setattr( + "utils.paths.storage_roots.exports_root", + lambda: export_root, + ) + + assert export_route._export_details(str(output)) == {"output_path": "model-gguf"} + + +def test_registered_absolute_export_folder_is_discoverable(tmp_path, monkeypatch): + _install_lightweight_backend_stubs(monkeypatch) + models_route = _load_module("test_routes_models", "routes/models.py", monkeypatch) + + export_dir = tmp_path / "Gemma4_26B_gguf" + export_dir.mkdir() + gguf_file = export_dir / "Gemma4_26B.BF16-00001-of-00002.gguf" + gguf_file.write_bytes(b"gguf") + + found = models_route._scan_models_dir(export_dir) + + assert len(found) == 1 + assert found[0].path == str(gguf_file) + assert found[0].source == "models_dir" diff --git a/studio/backend/utils/paths/__init__.py b/studio/backend/utils/paths/__init__.py index 6913a3be73..eff9b64678 100644 --- a/studio/backend/utils/paths/__init__.py +++ b/studio/backend/utils/paths/__init__.py @@ -43,13 +43,14 @@ from .storage_roots import ( resolve_under_root, resolve_output_dir, resolve_export_dir, + resolve_export_write_dir, resolve_tensorboard_dir, resolve_dataset_path, ) # Re-export shim: mark project-path helpers as used so the import-hoist # safety net does not flag them as unused. -_REEXPORTED = (documents_root, project_workspaces_root) +_REEXPORTED = (documents_root, project_workspaces_root, resolve_export_write_dir) __all__ = [ "normalize_path", @@ -89,6 +90,7 @@ __all__ = [ "resolve_under_root", "resolve_output_dir", "resolve_export_dir", + "resolve_export_write_dir", "resolve_tensorboard_dir", "resolve_dataset_path", ] diff --git a/studio/backend/utils/paths/storage_roots.py b/studio/backend/utils/paths/storage_roots.py index 2d4f5ac243..d336bc2e71 100644 --- a/studio/backend/utils/paths/storage_roots.py +++ b/studio/backend/utils/paths/storage_roots.py @@ -6,7 +6,7 @@ from __future__ import annotations import json import os import sys -from pathlib import Path +from pathlib import Path, PurePosixPath, PureWindowsPath import tempfile @@ -317,6 +317,26 @@ def _clean_relative_path(path_value: str, *, strip_prefixes: tuple[str, ...] = ( return Path(*parts) if parts else Path() +def _has_parent_segment(raw: str, path: Path) -> bool: + """Return true when a user path contains a parent-directory segment. + + On POSIX, ``Path("E:\\foo\\..\\bar")`` treats backslashes as normal + characters, so check both the host parser and Windows-style parsing. + """ + if ".." in path.parts: + return True + if ".." in PureWindowsPath(raw).parts: + return True + return ".." in raw.replace("\\", "/").split("/") + + +def _is_absolute_user_path(path: Path) -> bool: + expanded = str(path) + if os.name == "nt": + return path.is_absolute() and PureWindowsPath(expanded).is_absolute() + return path.is_absolute() and PurePosixPath(expanded).is_absolute() + + def _assert_contained(resolved: Path, root: Path) -> None: """Raise ValueError if ``resolved`` realpaths outside ``root``.""" try: @@ -351,10 +371,10 @@ def resolve_under_root( raise ValueError("path may not contain null bytes") path = Path(raw).expanduser() - if ".." in path.parts: + if _has_parent_segment(raw, path): raise ValueError(f"path may not contain '..' segments: {raw!r}") - if path.is_absolute(): + if _is_absolute_user_path(path): _assert_contained(path, root) return path @@ -373,6 +393,36 @@ def resolve_output_dir(path_value: str | None = None) -> Path: def resolve_export_dir(path_value: str | None = None) -> Path: + """Resolve an export directory — contained under exports_root(). + + Used by scan/read endpoints. Use :func:`resolve_export_write_dir` + for the export write path where absolute paths are accepted. + """ + return resolve_under_root( + path_value, + root = exports_root(), + strip_prefixes = ("exports",), + ) + + +def resolve_export_write_dir(path_value: str | None = None) -> Path: + """Resolve an export save directory — accepts absolute paths. + + Unlike :func:`resolve_export_dir`, this function passes absolute + paths through as-is so users can target a different drive when + their Studio install lives on a constrained system volume + (see :gh-issue:`6082`). Used only by the export write path. + """ + if not path_value or not str(path_value).strip(): + return exports_root() + raw = str(path_value).strip() + if "\x00" in raw: + raise ValueError("path may not contain null bytes") + path = Path(raw).expanduser() + if _has_parent_segment(raw, path): + raise ValueError(f"path may not contain '..' segments: {raw!r}") + if _is_absolute_user_path(path): + return path return resolve_under_root( path_value, root = exports_root(), diff --git a/studio/frontend/src/features/export/components/export-dialog.tsx b/studio/frontend/src/features/export/components/export-dialog.tsx index c529cadab6..2b40356a8c 100644 --- a/studio/frontend/src/features/export/components/export-dialog.tsx +++ b/studio/frontend/src/features/export/components/export-dialog.tsx @@ -2,6 +2,7 @@ // Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 import { Button } from "@/components/ui/button"; +import { FolderBrowser } from "@/components/assistant-ui/model-selector/folder-browser"; import { Dialog, DialogContent, @@ -18,7 +19,12 @@ import { } from "@/components/ui/input-group"; import { Spinner } from "@/components/ui/spinner"; import { Switch } from "@/components/ui/switch"; -import { AlertCircleIcon, ArrowRight01Icon, CheckmarkCircle02Icon, Key01Icon } from "@hugeicons/core-free-icons"; +import { + Tooltip, + TooltipContent, + TooltipTrigger, +} from "@/components/ui/tooltip"; +import { AlertCircleIcon, ArrowRight01Icon, CheckmarkCircle02Icon, FolderSearchIcon, Key01Icon } from "@hugeicons/core-free-icons"; import { HugeiconsIcon } from "@hugeicons/react"; import { AnimatePresence, motion } from "motion/react"; import { useEffect, useRef, useState } from "react"; @@ -200,6 +206,10 @@ function formatLogLine(entry: ExportLogEntry): string { return entry.line.replace(/\r+$/g, ""); } +function isAbsoluteFolderPath(path: string): boolean { + return path.startsWith("/") || /^[A-Za-z]:([\\/]|$)/.test(path) || /^\\\\/.test(path); +} + type Destination = "local" | "hub"; interface ExportDialogProps { @@ -213,6 +223,10 @@ interface ExportDialogProps { isAdapter: boolean; destination: Destination; onDestinationChange: (v: Destination) => void; + saveDirectory: string; + defaultSaveDirectory: string; + saveDirectoryOverridden: boolean; + onSaveDirectoryChange: (v: string | null) => void; hfUsername: string; onHfUsernameChange: (v: string) => void; modelName: string; @@ -243,6 +257,10 @@ export function ExportDialog({ isAdapter, destination, onDestinationChange, + saveDirectory, + defaultSaveDirectory, + saveDirectoryOverridden, + onSaveDirectoryChange, hfUsername, onHfUsernameChange, modelName, @@ -263,6 +281,7 @@ export function ExportDialog({ exportMethod === "gguf" || exportMethod === "lora"; const showCompletionScreen = exportSuccess && !showLogPanel; + const [folderBrowserOpen, setFolderBrowserOpen] = useState(false); const { lines: logLines, connected: logConnected, error: logError } = useExportLogs(exporting && showLogPanel, exportMethod, open); @@ -287,17 +306,18 @@ export function ExportDialog({ }; return ( - { - if (exporting) return; - onOpenChange(v); - }} - > - { if (exporting) e.preventDefault(); }} + <> + { + if (exporting) return; + onOpenChange(v); + }} > + { if (exporting) e.preventDefault(); }} + > {showCompletionScreen ? ( <>
@@ -358,6 +378,66 @@ export function ExportDialog({
+ {destination === "local" && ( +
+
+ + {saveDirectoryOverridden && ( + + )} +
+
+ onSaveDirectoryChange(e.target.value)} + disabled={exporting} + spellCheck={false} + title={saveDirectory} + placeholder={defaultSaveDirectory} + /> + + + + + + Browse + + +
+

+ {saveDirectory !== defaultSaveDirectory ? ( + <> + Default: {defaultSaveDirectory} + + ) : ( + <> + Paste an absolute path if the folder browser cannot reach the drive. + + )} +

+
+ )} + {destination === "hub" && ( @@ -598,7 +678,14 @@ export function ExportDialog({ )} - - + + + onSaveDirectoryChange(path)} + /> + ); } diff --git a/studio/frontend/src/features/export/export-page.tsx b/studio/frontend/src/features/export/export-page.tsx index aa18be37f7..d8d3a78dd1 100644 --- a/studio/frontend/src/features/export/export-page.tsx +++ b/studio/frontend/src/features/export/export-page.tsx @@ -77,6 +77,38 @@ import { exportTourSteps } from "./tour"; const SEARCH_INPUT_REASONS = new Set(["input-change", "input-paste", "input-clear"]); +function buildRelativeSaveDirectory( + exportMethod: ExportMethod | null, + sourceBaseModelName: string, + selectedModelIdx: string | null, + checkpoint: string | null, +): string { + if (exportMethod === "gguf") { + return `${(sourceBaseModelName.split("/").pop() ?? selectedModelIdx ?? "model") + .replace(/[^a-zA-Z0-9._-]/g, "-")}-gguf`; + } + return `${selectedModelIdx ?? "model"}/${checkpoint}`; +} + +function siblingGgufDirectory(sourcePath: string): string | null { + const trimmed = sourcePath.trim().replace(/[\\/]+$/, ""); + if (!trimmed) return null; + const slash = Math.max(trimmed.lastIndexOf("/"), trimmed.lastIndexOf("\\")); + if (slash < 0) return `${trimmed}_gguf`; + const parent = + slash === 0 || (slash === 2 && /^[A-Za-z]:/.test(trimmed)) + ? trimmed.slice(0, slash + 1) + : trimmed.slice(0, slash); + const name = trimmed.slice(slash + 1); + if (!name) return null; + const sep = parent.endsWith("/") || parent.endsWith("\\") + ? "" + : trimmed.includes("\\") + ? "\\" + : "/"; + return `${parent}${sep}${name}_gguf`; +} + export function ExportPage() { const { hfToken, setHfToken } = useTrainingConfigStore( useShallow((s) => ({ @@ -114,6 +146,9 @@ export function ExportPage() { const [dialogOpen, setDialogOpen] = useState(false); const [destination, setDestination] = useState<"local" | "hub">("local"); + const [customSaveDirectory, setCustomSaveDirectory] = useState( + null, + ); const [hfUsername, setHfUsername] = useState(""); const [modelName, setModelName] = useState(""); const [privateRepo, setPrivateRepo] = useState(false); @@ -336,6 +371,36 @@ export function ExportPage() { const estimatedSize = getEstimatedSize(exportMethod, quantLevels); const selectedExportSource = sourceMode === "checkpoint" ? checkpoint : selectedSourceModel; + const defaultSaveDirectory = useMemo(() => { + const relative = buildRelativeSaveDirectory( + exportMethod, + sourceBaseModelName, + selectedModelIdx, + checkpoint, + ); + if ( + exportMethod === "gguf" && + sourceMode === "model" && + modelSource === "local" && + selectedSourceModel + ) { + const localModel = localMetaById.get(selectedSourceModel); + if (localModel && (localModel.source === "models_dir" || localModel.source === "custom")) { + return siblingGgufDirectory(localModel.path) ?? relative; + } + } + return relative; + }, [ + checkpoint, + exportMethod, + localMetaById, + modelSource, + selectedModelIdx, + selectedSourceModel, + sourceBaseModelName, + sourceMode, + ]); + const saveDirectory = customSaveDirectory?.trim() || defaultSaveDirectory; const canExport = !!( selectedExportSource && exportMethod && @@ -380,6 +445,10 @@ export function ExportPage() { setSelectedSourceModel(next || null); }, []); + useEffect(() => { + setCustomSaveDirectory(null); + }, [checkpoint, exportMethod, modelSource, selectedModelIdx, selectedSourceModel, sourceMode]); + const handleLocalSourceInputChange = useCallback( (value: string, eventDetails?: { reason?: string }) => { localModelInputRef.current = value; @@ -410,13 +479,6 @@ export function ExportPage() { setExportSuccess(false); setExportOutputPath(null); - // For GGUF, use a flat folder like "exports/gemma-3-4b-it-finetune-gguf" - // For other formats, nest under training-run/checkpoint - const saveDir = - exportMethod === "gguf" - ? `${(sourceBaseModelName.split("/").pop() ?? selectedModelIdx ?? "model") - .replace(/[^a-zA-Z0-9._-]/g, "-")}-gguf` - : `${selectedModelIdx ?? "model"}/${checkpoint}`; const pushToHub = destination === "hub"; const repoId = pushToHub && hfUsername && modelName ? `${hfUsername}/${modelName}` @@ -444,7 +506,7 @@ export function ExportPage() { if (exportMethod === "merged") { if (isAdapter) { const resp = await exportMerged({ - save_directory: saveDir, + save_directory: saveDirectory, push_to_hub: pushToHub, repo_id: repoId, hf_token: token, @@ -453,7 +515,7 @@ export function ExportPage() { lastOutputPath = resp.details?.output_path ?? null; } else { const resp = await exportBase({ - save_directory: saveDir, + save_directory: saveDirectory, push_to_hub: pushToHub, repo_id: repoId, hf_token: token, @@ -465,7 +527,7 @@ export function ExportPage() { } else if (exportMethod === "gguf") { for (const quant of quantLevels) { const resp = await exportGGUF({ - save_directory: saveDir, + save_directory: saveDirectory, quantization_method: quant, push_to_hub: pushToHub, repo_id: repoId, @@ -475,7 +537,7 @@ export function ExportPage() { } } else if (exportMethod === "lora") { const resp = await exportLoRA({ - save_directory: saveDir, + save_directory: saveDirectory, push_to_hub: pushToHub, repo_id: repoId, hf_token: token, @@ -507,9 +569,9 @@ export function ExportPage() { selectedModelData, exportMethod, isAdapter, - sourceBaseModelName, quantLevels, destination, + saveDirectory, hfUsername, modelName, hfToken, @@ -1080,6 +1142,10 @@ export function ExportPage() { isAdapter={sourceMode === "checkpoint" && isAdapter} destination={destination} onDestinationChange={setDestination} + saveDirectory={saveDirectory} + defaultSaveDirectory={defaultSaveDirectory} + saveDirectoryOverridden={!!customSaveDirectory} + onSaveDirectoryChange={setCustomSaveDirectory} hfUsername={hfUsername} onHfUsernameChange={setHfUsername} modelName={modelName}