From 8ccdf596aa8e69f9f949c07098cbced0c37ae111 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Mon, 8 Jun 2026 03:40:59 -0700 Subject: [PATCH] Studio: stop leaking internal exceptions to API clients; harden sandbox path (#6072) * Studio: stop leaking internal exceptions to API clients; harden sandbox path Security hardening for the FastAPI backend. Error exposure (CodeQL py/stack-trace-exposure): many route handlers returned raw caught-exception text to clients via HTTPException detail / response bodies, which can leak internal filesystem paths and stack detail. Add shared helpers in utils/utils.py (safe_error_detail, log_and_http_error) that log the full exception server-side and return a generic message, and sweep the route layer (inference, models, export, training, datasets, chat_history, providers, mcp_servers, settings, data_recipe/{jobs,seed,validate,mcp}) to use them. Intentionally user-facing validation messages, the existing _friendly_error SSE paths, and upstream-service body passthrough (llama-server / OpenAI) are kept; absolute server paths echoed in models.py browse/read errors are redacted. Path injection (CodeQL py/path-injection): serve_sandbox_file already does basename + realpath containment; add a strict filename allowlist (^[A-Za-z0-9._-]{1,255}$) before the path is built as defense-in-depth and to give the analyzer a clear sanitizer. No behavior change beyond error-message text; status codes preserved. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address review: keep curated error messages, fix remaining load leak - inference.py /load non-native path: redact str(e) instead of leaking it (matched the native branch which already redacted). - llama_extra_args validation: return the curated, path-redacted message instead of the generic fallback so users see the offending flag. - sandbox file serving: allowlist now forbids only separators/control chars via fullmatch, so generated images like 'loss curve.png' render again while traversal is still blocked by basename + extension + realpath. - Add safe_curated_detail() for domain/validation exceptions whose message is intentionally user-facing; apply it to data_recipe job/validate, chat conflict, provider test, and MCP probe paths (these were collapsing to 'An internal error occurred', and 'connection' even mis-mapped to an upstream-service message). Generic Exception paths keep safe_error_detail. - log_and_http_error: tolerate stdlib loggers (no structlog kwargs). - delete_openai_container: log transport errors with exc_info like list/create. - Drop helper/HTTPException imports this change left unused. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * log_and_http_error: log original error traceback on stdlib-logger fallback * Tidy error-helper and sandbox comments for PR #6072 * Trim redundant comments in studio error-hardening routes for PR #6072 * Re-trigger CI now that unsloth-zoo #727 is merged (Core pulls zoo main) * Address PR #6072 review feedback - inference.py: keep the actionable NativePathLeaseError detail (path-redacted) instead of collapsing it to the generic message, matching the other curated validation paths in this file. - utils.py: log via a single formatted log.error(exc_info=error) call that works for structlog and stdlib loggers; drop the now-unneeded try/except helper. - models.py: use Path.name instead of os.path.basename(str(current)). --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- studio/backend/routes/chat_history.py | 28 ++++- studio/backend/routes/data_recipe/jobs.py | 51 ++++++-- studio/backend/routes/data_recipe/mcp.py | 17 ++- studio/backend/routes/data_recipe/seed.py | 50 ++++++-- studio/backend/routes/data_recipe/validate.py | 25 +++- studio/backend/routes/datasets.py | 6 +- studio/backend/routes/export.py | 18 +-- studio/backend/routes/inference.py | 83 +++++++++---- studio/backend/routes/mcp_servers.py | 25 +++- studio/backend/routes/models.py | 113 ++++++++++++------ studio/backend/routes/providers.py | 20 +++- studio/backend/routes/settings.py | 14 ++- studio/backend/routes/training.py | 57 ++++++--- studio/backend/utils/utils.py | 61 ++++++++++ 14 files changed, 436 insertions(+), 132 deletions(-) diff --git a/studio/backend/routes/chat_history.py b/studio/backend/routes/chat_history.py index 75b5be2e49..4da3c7d621 100644 --- a/studio/backend/routes/chat_history.py +++ b/studio/backend/routes/chat_history.py @@ -11,6 +11,8 @@ from fastapi import APIRouter, Depends, HTTPException, Query from pydantic import BaseModel, ConfigDict, Field, ValidationError from auth.authentication import get_current_subject +from loggers import get_logger +from utils.utils import safe_curated_detail, log_and_http_error from storage.studio_db import ( ChatMessageConflictError, CorruptSettingsError, @@ -40,6 +42,8 @@ from storage.studio_db import ( router = APIRouter() +logger = get_logger(__name__) + class ChatThread(BaseModel): id: str @@ -406,7 +410,13 @@ async def save_thread_message( try: return ChatMessage(**upsert_chat_message(payload.model_dump())) except ChatMessageConflictError as exc: - raise HTTPException(status_code = 409, detail = str(exc)) from exc + raise log_and_http_error( + exc, + 409, + safe_curated_detail(exc), + event = "chat_history.save_message_conflict", + log = logger, + ) from exc @router.put("/threads/{thread_id}/messages", response_model = ChatMessageListResponse) @@ -442,7 +452,13 @@ async def replace_thread_messages( ] ) except ChatMessageConflictError as exc: - raise HTTPException(status_code = 409, detail = str(exc)) from exc + raise log_and_http_error( + exc, + 409, + safe_curated_detail(exc), + event = "chat_history.replace_messages_conflict", + log = logger, + ) from exc @router.get("/count", response_model = ChatCountResponse) @@ -497,7 +513,13 @@ async def put_settings( settings = upsert_chat_settings_merge(parsed.model_dump(exclude_unset = True)) ) except CorruptSettingsError as exc: - raise HTTPException(status_code = 409, detail = str(exc)) from exc + raise log_and_http_error( + exc, + 409, + safe_curated_detail(exc), + event = "chat_history.put_settings_conflict", + log = logger, + ) from exc @router.get("/export", response_model = ChatExportResponse) diff --git a/studio/backend/routes/data_recipe/jobs.py b/studio/backend/routes/data_recipe/jobs.py index 107a1657f3..238ff08006 100644 --- a/studio/backend/routes/data_recipe/jobs.py +++ b/studio/backend/routes/data_recipe/jobs.py @@ -19,13 +19,16 @@ from core.data_recipe.huggingface import ( publish_recipe_dataset, ) from core.data_recipe.jobs import get_job_manager +from loggers import get_logger from models.data_recipe import ( JobCreateResponse, PublishDatasetRequest, PublishDatasetResponse, RecipePayload, ) +from utils.utils import safe_error_detail, safe_curated_detail, log_and_http_error +logger = get_logger(__name__) router = APIRouter() @@ -439,14 +442,24 @@ def create_job(payload: RecipePayload, request: Request): RunConfig.model_validate(run_config_raw) except (ImportError, ValidationError, TypeError, ValueError) as exc: - raise HTTPException( - status_code = 400, detail = f"invalid run_config: {exc}" + raise log_and_http_error( + exc, + 400, + "invalid run_config", + event = "data_recipe.jobs.run_config_invalid", + log = logger, ) from exc try: internal_api_key_id = _inject_local_providers(recipe, request) except ValueError as exc: - raise HTTPException(status_code = 400, detail = str(exc)) from exc + raise log_and_http_error( + exc, + 400, + safe_curated_detail(exc), + event = "data_recipe.jobs.inject_local_providers_failed", + log = logger, + ) from exc # Single try block covers get_job_manager() AND mgr.start() so a workflow # key minted above never outlives the request even when an unexpected @@ -463,11 +476,23 @@ def create_job(payload: RecipePayload, request: Request): except RuntimeError as exc: if internal_api_key_id is not None: _revoke_internal_api_key_safe(internal_api_key_id) - raise HTTPException(status_code = 409, detail = str(exc)) from exc + raise log_and_http_error( + exc, + 409, + safe_curated_detail(exc), + event = "data_recipe.jobs.start_conflict", + log = logger, + ) from exc except ValueError as exc: if internal_api_key_id is not None: _revoke_internal_api_key_safe(internal_api_key_id) - raise HTTPException(status_code = 400, detail = str(exc)) from exc + raise log_and_http_error( + exc, + 400, + safe_curated_detail(exc), + event = "data_recipe.jobs.start_failed", + log = logger, + ) from exc except Exception: if internal_api_key_id is not None: _revoke_internal_api_key_safe(internal_api_key_id) @@ -593,9 +618,21 @@ def publish_job_dataset(job_id: str, payload: PublishDatasetRequest): private = payload.private, ) except RecipeDatasetPublishError as exc: - raise HTTPException(status_code = 400, detail = str(exc)) from exc + raise log_and_http_error( + exc, + 400, + safe_curated_detail(exc), + event = "data_recipe.jobs.publish_failed", + log = logger, + ) from exc except Exception as exc: - raise HTTPException(status_code = 500, detail = str(exc)) from exc + raise log_and_http_error( + exc, + 500, + safe_error_detail(exc), + event = "data_recipe.jobs.publish_error", + log = logger, + ) from exc return { "success": True, diff --git a/studio/backend/routes/data_recipe/mcp.py b/studio/backend/routes/data_recipe/mcp.py index 7184934ce9..2c79d323f3 100644 --- a/studio/backend/routes/data_recipe/mcp.py +++ b/studio/backend/routes/data_recipe/mcp.py @@ -10,12 +10,15 @@ from collections import defaultdict from fastapi import APIRouter from core.data_recipe.service import build_mcp_providers +from loggers import get_logger from models.data_recipe import ( McpToolsListRequest, McpToolsListResponse, McpToolsProviderResult, ) +from utils.utils import safe_error_detail +logger = get_logger(__name__) router = APIRouter() @@ -24,11 +27,16 @@ def list_mcp_tools(payload: McpToolsListRequest) -> McpToolsListResponse: try: from data_designer.engine.mcp import io as mcp_io except ImportError as exc: + logger.error( + "data_recipe.mcp.dependencies_unavailable", + error = str(exc), + exc_info = True, + ) return McpToolsListResponse( providers = [ McpToolsProviderResult( name = "", - error = f"MCP dependencies unavailable: {exc}", + error = "MCP dependencies unavailable.", ) ] ) @@ -73,10 +81,15 @@ def list_mcp_tools(payload: McpToolsListRequest) -> McpToolsListResponse: ) ) except Exception as exc: + logger.error( + "data_recipe.mcp.list_tools_failed", + error = str(exc), + exc_info = True, + ) providers.append( McpToolsProviderResult( name = provider.name or provider_name, - error = str(exc).strip() or "Failed to load tools.", + error = safe_error_detail(exc, fallback = "Failed to load tools."), ) ) diff --git a/studio/backend/routes/data_recipe/seed.py b/studio/backend/routes/data_recipe/seed.py index a18f8f3e32..51ea37452e 100644 --- a/studio/backend/routes/data_recipe/seed.py +++ b/studio/backend/routes/data_recipe/seed.py @@ -30,7 +30,9 @@ except ImportError: normalize_unstructured_text = None resolve_chunking = None from core.data_recipe.jsonable import to_preview_jsonable +from loggers import get_logger from utils.paths import ensure_dir, seed_uploads_root, unstructured_uploads_root +from utils.utils import log_and_http_error from utils.upload_limits import ( LOCAL_SEED_UPLOAD_MAX_BYTES, LOCAL_SEED_UPLOAD_MAX_LABEL, @@ -47,6 +49,7 @@ from models.data_recipe import ( UnstructuredFileUploadResponse, ) +logger = get_logger(__name__) router = APIRouter() DATA_EXTS = (".parquet", ".jsonl", ".json", ".csv") @@ -201,8 +204,12 @@ def _read_preview_rows_from_local_file( try: import pandas as pd except ImportError as exc: - raise HTTPException( - status_code = 500, detail = f"seed inspect dependencies unavailable: {exc}" + raise log_and_http_error( + exc, + 500, + "seed inspect dependencies unavailable", + event = "data_recipe.seed.dependencies_unavailable", + log = logger, ) from exc ext = path.suffix.lower() @@ -231,8 +238,12 @@ def _read_preview_rows_from_local_file( except HTTPException: raise except (ValueError, OSError) as exc: - raise HTTPException( - status_code = 422, detail = f"seed inspect failed: {exc}" + raise log_and_http_error( + exc, + 422, + "seed inspect failed", + event = "data_recipe.seed.local_preview_failed", + log = logger, ) from exc rows = df.to_dict(orient = "records") @@ -260,8 +271,12 @@ def _read_preview_rows_from_unstructured_file( chunk_overlap = overlap, ) except (FileNotFoundError, RuntimeError, ValueError, OSError) as exc: - raise HTTPException( - status_code = 422, detail = f"seed inspect failed: {exc}" + raise log_and_http_error( + exc, + 422, + "seed inspect failed", + event = "data_recipe.seed.unstructured_preview_failed", + log = logger, ) from exc return _serialize_preview_rows(rows) @@ -312,8 +327,12 @@ def inspect_seed_dataset(payload: SeedInspectRequest) -> SeedInspectResponse: try: from datasets import load_dataset except ImportError as exc: - raise HTTPException( - status_code = 500, detail = f"seed inspect dependencies unavailable: {exc}" + raise log_and_http_error( + exc, + 500, + "seed inspect dependencies unavailable", + event = "data_recipe.seed.dependencies_unavailable", + log = logger, ) from exc split = _normalize_optional_text(payload.split) or DEFAULT_SPLIT @@ -356,8 +375,12 @@ def inspect_seed_dataset(payload: SeedInspectRequest) -> SeedInspectResponse: preview_size = preview_size, ) except (ValueError, OSError, RuntimeError) as exc: - raise HTTPException( - status_code = 422, detail = f"seed inspect failed: {exc}" + raise log_and_http_error( + exc, + 422, + "seed inspect failed", + event = "data_recipe.seed.hf_preview_failed", + log = logger, ) from exc if not preview_rows: @@ -480,12 +503,17 @@ async def upload_unstructured_file( except Exception as e: raw_path.unlink(missing_ok = True) extracted_path.unlink(missing_ok = True) + logger.error( + "data_recipe.seed.text_extraction_failed", + error = str(e), + exc_info = True, + ) return UnstructuredFileUploadResponse( file_id = file_id, filename = original_filename, size_bytes = size_bytes, status = "error", - error = f"Text extraction failed: {type(e).__name__}: {e}", + error = "Text extraction failed.", ) try: diff --git a/studio/backend/routes/data_recipe/validate.py b/studio/backend/routes/data_recipe/validate.py index e794d68e54..f6fd9e6046 100644 --- a/studio/backend/routes/data_recipe/validate.py +++ b/studio/backend/routes/data_recipe/validate.py @@ -7,7 +7,7 @@ from __future__ import annotations from typing import Any -from fastapi import APIRouter, HTTPException +from fastapi import APIRouter from core.data_recipe.service import ( build_config_builder, @@ -16,6 +16,7 @@ from core.data_recipe.service import ( ) from loggers import get_logger from models.data_recipe import RecipePayload, ValidateError, ValidateResponse +from utils.utils import safe_error_detail, safe_curated_detail, log_and_http_error logger = get_logger(__name__) router = APIRouter() @@ -170,7 +171,12 @@ def validate(payload: RecipePayload) -> ValidateResponse: missing_module = exc.name, ) except Exception as exc: - detail = str(exc).strip() or "Validation failed." + logger.error( + "data_recipe.validate.github_config_failed", + error = str(exc), + exc_info = True, + ) + detail = safe_error_detail(exc, fallback = "Validation failed.") return ValidateResponse( valid = False, errors = [ValidateError(message = detail)], @@ -181,9 +187,20 @@ def validate(payload: RecipePayload) -> ValidateResponse: try: validate_recipe(recipe) except RuntimeError as exc: - raise HTTPException(status_code = 503, detail = str(exc)) from exc + raise log_and_http_error( + exc, + 503, + safe_error_detail(exc), + event = "data_recipe.validate.service_unavailable", + log = logger, + ) from exc except Exception as exc: - detail = str(exc).strip() or "Validation failed." + logger.error( + "data_recipe.validate.recipe_failed", + error = str(exc), + exc_info = True, + ) + detail = safe_curated_detail(exc, fallback = "Validation failed.") parsed_errors = _collect_validation_errors(recipe) return ValidateResponse( valid = False, diff --git a/studio/backend/routes/datasets.py b/studio/backend/routes/datasets.py index c34d6d8732..1ea696705c 100644 --- a/studio/backend/routes/datasets.py +++ b/studio/backend/routes/datasets.py @@ -647,9 +647,7 @@ def check_format( raise except Exception as e: logger.error(f"Error checking dataset format: {e}", exc_info = True) - raise HTTPException( - status_code = 500, detail = f"Failed to check dataset format: {str(e)}" - ) + raise HTTPException(status_code = 500, detail = "Failed to check dataset format") @router.post("/ai-assist-mapping", response_model = AiAssistMappingResponse) @@ -705,4 +703,4 @@ def ai_assist_mapping( except Exception as e: logger.error(f"AI assist mapping failed: {e}", exc_info = True) - raise HTTPException(status_code = 500, detail = f"AI assist failed: {str(e)}") + raise HTTPException(status_code = 500, detail = "AI assist failed") diff --git a/studio/backend/routes/export.py b/studio/backend/routes/export.py index 7dbc52dbed..2bfca30cee 100644 --- a/studio/backend/routes/export.py +++ b/studio/backend/routes/export.py @@ -26,6 +26,8 @@ if str(backend_path) not in sys.path: # Auth from auth.authentication import get_current_subject +from utils.utils import safe_error_detail + # Import backend functions try: from core.export import get_export_backend @@ -125,7 +127,7 @@ async def load_checkpoint( logger.error(f"Error loading checkpoint: {e}", exc_info = True) raise HTTPException( status_code = 500, - detail = f"Failed to load checkpoint: {str(e)}", + detail = "Failed to load checkpoint", ) @@ -158,7 +160,7 @@ async def cleanup_export_memory( logger.error(f"Error during export memory cleanup: {e}", exc_info = True) raise HTTPException( status_code = 500, - detail = f"Failed to cleanup export memory: {str(e)}", + detail = "Failed to cleanup export memory", ) @@ -180,7 +182,7 @@ async def get_export_status( logger.error(f"Error getting export status: {e}", exc_info = True) raise HTTPException( status_code = 500, - detail = f"Failed to get export status: {str(e)}", + detail = "Failed to get export status", ) @@ -235,7 +237,7 @@ async def export_merged_model( logger.error(f"Error exporting merged model: {e}", exc_info = True) raise HTTPException( status_code = 500, - detail = f"Failed to export merged model: {str(e)}", + detail = "Failed to export merged model", ) @@ -275,7 +277,7 @@ async def export_base_model( logger.error(f"Error exporting base model: {e}", exc_info = True) raise HTTPException( status_code = 500, - detail = f"Failed to export base model: {str(e)}", + detail = "Failed to export base model", ) @@ -314,7 +316,7 @@ async def export_gguf( logger.error(f"Error exporting GGUF model: {e}", exc_info = True) raise HTTPException( status_code = 500, - detail = f"Failed to export GGUF model: {str(e)}", + detail = "Failed to export GGUF model", ) @@ -353,7 +355,7 @@ async def export_lora_adapter( logger.error(f"Error exporting LoRA adapter: {e}", exc_info = True) raise HTTPException( status_code = 500, - detail = f"Failed to export LoRA adapter: {str(e)}", + detail = "Failed to export LoRA adapter", ) @@ -492,7 +494,7 @@ async def stream_export_logs( logger.error("Export log stream failed: %s", exc, exc_info = True) try: yield _format_sse( - json.dumps({"error": str(exc)}), + json.dumps({"error": safe_error_detail(exc)}), event = "error", ) except Exception: diff --git a/studio/backend/routes/inference.py b/studio/backend/routes/inference.py index d3a279319a..5d22b3ff90 100644 --- a/studio/backend/routes/inference.py +++ b/studio/backend/routes/inference.py @@ -227,6 +227,7 @@ from core.inference.key_exchange import decrypt_api_key from core.inference.providers import get_provider_info, get_base_url from core.inference.external_provider import ExternalProviderClient from storage import providers_db +from utils.utils import safe_error_detail, log_and_http_error import io import wave @@ -697,7 +698,13 @@ def _resolve_model_identifier_for_request( allowed_suffixes = (".gguf",), ) except NativePathLeaseError as exc: - raise HTTPException(status_code = 400, detail = str(exc)) from exc + # Curated, client-correctable lease error (expired / wrong type / re-select); + # keep the actionable message, just redact paths. + logger.warning("inference.native_path_lease_failed: %s", exc) + raise HTTPException( + status_code = 400, + detail = redact_native_paths(str(exc)), + ) from exc display_label = ( grant.display_label or Path(request.model_path).name or "Native model" ) @@ -735,7 +742,12 @@ async def load_model( try: extra_llama_args = validate_extra_args(request.llama_extra_args) except ValueError as exc: - raise HTTPException(status_code = 400, detail = str(exc)) + # Keep the curated validation message (names the flag); just strip paths. + logger.warning("inference.validate_extra_args_failed: %s", exc) + raise HTTPException( + status_code = 400, + detail = redact_native_paths(str(exc)), + ) # Re-narrow []-from-None back to None so the inheritance path # below can tell "caller omitted" from "caller explicit []". extra_llama_args: Optional[list[str]] = ( @@ -1221,7 +1233,8 @@ async def load_model( ) raise HTTPException(status_code = 400, detail = redacted_msg) logger.warning("Rejected inference GPU selection: %s", e) - raise HTTPException(status_code = 400, detail = str(e)) + # User-facing validation (e.g. "Invalid gpu_ids [99]"): redact paths, keep detail. + raise HTTPException(status_code = 400, detail = redact_native_paths(str(e))) except Exception as e: # Surface a friendlier message for models that Unsloth cannot load not_supported_hints = [ @@ -1245,7 +1258,7 @@ async def load_model( detail = f"Failed to load native model {model_log_label}: {msg}", ) logger.error(f"Error loading model: {e}", exc_info = True) - msg = str(e) + msg = redact_native_paths(str(e)) if any(h.lower() in msg.lower() for h in not_supported_hints): msg = f"This model is not supported yet. Try a different model. (Original error: {msg})" raise HTTPException(status_code = 500, detail = f"Failed to load model: {msg}") @@ -1324,7 +1337,7 @@ async def validate_model( ) raise HTTPException( status_code = 400, - detail = f"Invalid model: {str(e)}", + detail = "Invalid model", ) @@ -1359,7 +1372,7 @@ async def unload_model( except Exception as e: logger.error(f"Error unloading model: {e}", exc_info = True) - raise HTTPException(status_code = 500, detail = f"Failed to unload model: {str(e)}") + raise HTTPException(status_code = 500, detail = "Failed to unload model") @studio_router.post("/cancel") @@ -1444,8 +1457,12 @@ async def generate_stream( except HTTPException: raise except Exception as e: - raise HTTPException( - status_code = 400, detail = f"Failed to decode image: {str(e)}" + raise log_and_http_error( + e, + 400, + "Failed to decode image", + event = "inference.decode_image_failed", + log = logger, ) async def stream(): @@ -1614,7 +1631,7 @@ async def get_status( except Exception as e: logger.error(f"Error getting status: {e}", exc_info = True) - raise HTTPException(status_code = 500, detail = f"Failed to get status: {str(e)}") + raise HTTPException(status_code = 500, detail = "Failed to get status") @router.get("/load-progress", response_model = LoadProgressResponse) @@ -1715,7 +1732,7 @@ async def generate_audio( ) except Exception as e: logger.error(f"Audio generation error: {e}", exc_info = True) - raise HTTPException(status_code = 500, detail = str(e)) + raise HTTPException(status_code = 500, detail = safe_error_detail(e)) audio_b64 = base64.b64encode(wav_bytes).decode("ascii") return JSONResponse( @@ -2402,9 +2419,12 @@ async def list_openai_containers( detail = f"OpenAI rejected /containers list: {detail}", ) except httpx.HTTPError as exc: - raise HTTPException( - status_code = 502, - detail = f"Failed to reach OpenAI: {exc}", + raise log_and_http_error( + exc, + 502, + "Could not reach OpenAI.", + event = "openai_container_list.transport_error", + log = logger, ) # OpenAI keeps expired containers in /v1/containers indefinitely # with status="expired" — they're effectively dead but still @@ -2443,9 +2463,12 @@ async def create_openai_container( detail = f"OpenAI rejected /containers create: {detail}", ) except httpx.HTTPError as exc: - raise HTTPException( - status_code = 502, - detail = f"Failed to reach OpenAI: {exc}", + raise log_and_http_error( + exc, + 502, + "Could not reach OpenAI.", + event = "openai_container_create.transport_error", + log = logger, ) if not isinstance(raw, dict): raise HTTPException( @@ -2490,14 +2513,12 @@ async def delete_openai_container( detail = f"OpenAI rejected /containers delete: {detail}", ) except httpx.HTTPError as exc: - logger.warning( - "openai_container_delete.transport_error container_id=%s error=%s", - body.container_id, + raise log_and_http_error( exc, - ) - raise HTTPException( - status_code = 502, - detail = f"Failed to reach OpenAI: {exc}", + 502, + "Could not reach OpenAI.", + event = "openai_container_delete.transport_error", + log = logger, ) finally: await client.close() @@ -3262,7 +3283,7 @@ async def openai_chat_completions( except Exception as e: logger.error(f"Error during GGUF completion: {e}", exc_info = True) - raise HTTPException(status_code = 500, detail = str(e)) + raise HTTPException(status_code = 500, detail = safe_error_detail(e)) # ── Standard Unsloth path ───────────────────────────────── @@ -3290,7 +3311,13 @@ async def openai_chat_completions( except HTTPException: raise except Exception as e: - raise HTTPException(status_code = 400, detail = f"Failed to decode image: {e}") + raise log_and_http_error( + e, + 400, + "Failed to decode image", + event = "inference.decode_image_failed", + log = logger, + ) # Classify capability flags from the loaded template. _sf_model_info = backend.models.get(backend.active_model_name, {}) @@ -3817,7 +3844,7 @@ async def openai_chat_completions( except Exception as e: backend.reset_generation_state() logger.error(f"Error during OpenAI completion: {e}", exc_info = True) - raise HTTPException(status_code = 500, detail = str(e)) + raise HTTPException(status_code = 500, detail = safe_error_detail(e)) # ===================================================================== @@ -3869,6 +3896,10 @@ async def serve_sandbox_file( safe_filename = os.path.basename(filename) if not safe_filename or safe_filename in (".", ".."): raise HTTPException(status_code = 404, detail = "Not found") + # Defense-in-depth allowlist (clears CodeQL py/path-injection), still allowing + # names like "loss curve.png"; basename + extension + realpath below are the guards. + if not _re.fullmatch(r"[^/\\\x00-\x1f]{1,255}", safe_filename): + raise HTTPException(status_code = 404, detail = "Not found") # ── Extension allowlist ───────────────────────────────────── ext = os.path.splitext(safe_filename)[1].lower() diff --git a/studio/backend/routes/mcp_servers.py b/studio/backend/routes/mcp_servers.py index 200c0ed452..87cdcf9402 100644 --- a/studio/backend/routes/mcp_servers.py +++ b/studio/backend/routes/mcp_servers.py @@ -26,6 +26,7 @@ from models.mcp_servers import ( McpServerUpdate, ) from storage import mcp_servers_db +from utils.utils import safe_curated_detail, log_and_http_error logger = structlog.get_logger(__name__) @@ -51,7 +52,13 @@ def _validate_url(url: str) -> str: try: parts = parse_stdio_command(trimmed) except ValueError as exc: - raise HTTPException(status_code = 400, detail = f"Invalid command: {exc}") + raise log_and_http_error( + exc, + 400, + "Invalid command. Check quoting and try again.", + event = "mcp_servers.invalid_command", + log = logger, + ) if not parts or not parts[0].strip(): raise HTTPException(status_code = 400, detail = "command must not be empty") if "://" in parts[0]: @@ -241,8 +248,13 @@ async def refresh_mcp_server_tools( use_oauth = use_oauth, ) except Exception as exc: # noqa: BLE001 — surface transport+timeout errors to UI - logger.warning("MCP refresh failed", server_id = server_id, error = str(exc)) - return McpServerProbeResult(ok = False, error = str(exc)) + logger.error( + "mcp_servers.refresh_failed", + server_id = server_id, + error = str(exc), + exc_info = True, + ) + return McpServerProbeResult(ok = False, error = safe_curated_detail(exc)) return McpServerProbeResult(ok = True, tool_count = len(tools)) @@ -265,6 +277,11 @@ async def test_mcp_server( use_oauth = payload.use_oauth, ) except Exception as exc: # noqa: BLE001 - return McpServerProbeResult(ok = False, error = str(exc)) + logger.error( + "mcp_servers.test_failed", + error = str(exc), + exc_info = True, + ) + return McpServerProbeResult(ok = False, error = safe_curated_detail(exc)) return McpServerProbeResult(ok = True, tool_count = len(tools)) diff --git a/studio/backend/routes/models.py b/studio/backend/routes/models.py index 2b729eea9c..0e1e491998 100644 --- a/studio/backend/routes/models.py +++ b/studio/backend/routes/models.py @@ -16,6 +16,7 @@ from fastapi import APIRouter, Body, Depends, HTTPException, Query from typing import List, Optional import structlog from loggers import get_logger +from utils.utils import log_and_http_error import re as _re @@ -825,10 +826,12 @@ async def list_local_models( models = models, ) except Exception as e: - logger.error(f"Error listing local models: {e}", exc_info = True) - raise HTTPException( - status_code = 500, - detail = f"Failed to list local models: {str(e)}", + raise log_and_http_error( + e, + 500, + "Failed to list local models", + event = "models.list_local_models_failed", + log = logger, ) @@ -854,7 +857,10 @@ async def add_scan_folder_endpoint( folder = add_scan_folder(body.path) except ValueError as e: logger.warning("Scan folder rejected: %s (path=%s)", e, body.path) - raise HTTPException(status_code = 400, detail = str(e)) + # Curated, path-free validation message (e.g. "Path does not exist"): + # forward the text, not the raw exception. + rejection_message = str(e) + raise HTTPException(status_code = 400, detail = rejection_message) logger.info("Scan folder added: %s", folder.get("path")) return folder @@ -1182,12 +1188,15 @@ def _match_browse_child(current: Path, name: str) -> Optional[Path]: except PermissionError: raise HTTPException( status_code = 403, - detail = f"Permission denied reading {current}", + detail = f"Permission denied reading {current.name}", ) from None except OSError as exc: + logger.warning( + "browse-folders: could not read %s: %s", current, exc, exc_info = True + ) raise HTTPException( status_code = 500, - detail = f"Could not read {current}: {exc}", + detail = f"Could not read {os.path.basename(str(current))}", ) from exc return None @@ -1219,14 +1228,21 @@ def _resolve_browse_target(path: Optional[str], allowed_roots: list[Path]) -> Pa if child is None: raise HTTPException( status_code = 404, - detail = f"Path does not exist: {requested_path}", + detail = f"Path does not exist: {os.path.basename(requested_path)}", ) try: resolved_child = child.resolve() except OSError as exc: + logger.warning( + "browse-folders: invalid path component %r under %s: %s", + part, + current, + exc, + exc_info = True, + ) raise HTTPException( status_code = 400, - detail = f"Invalid path: {exc}", + detail = "Invalid path", ) from exc if not _is_path_inside_allowlist(resolved_child, resolved_roots): raise HTTPException( @@ -1242,7 +1258,7 @@ def _resolve_browse_target(path: Optional[str], allowed_roots: list[Path]) -> Pa if not current.is_dir(): raise HTTPException( status_code = 400, - detail = f"Not a directory: {current}", + detail = f"Not a directory: {os.path.basename(str(current))}", ) return current @@ -1325,12 +1341,15 @@ async def browse_folders( except PermissionError: raise HTTPException( status_code = 403, - detail = f"Permission denied reading {target}", + detail = f"Permission denied reading {os.path.basename(str(target))}", ) except OSError as exc: + logger.warning( + "browse-folders: could not read %s: %s", target, exc, exc_info = True + ) raise HTTPException( status_code = 500, - detail = f"Could not read {target}: {exc}", + detail = f"Could not read {os.path.basename(str(target))}", ) try: @@ -1533,8 +1552,13 @@ async def list_models( return ModelListResponse(models = all_models, default_models = default_models) except Exception as e: - logger.error(f"Error listing models: {e}", exc_info = True) - raise HTTPException(status_code = 500, detail = f"Failed to list models: {str(e)}") + raise log_and_http_error( + e, + 500, + "Failed to list models", + event = "models.list_models_failed", + log = logger, + ) def _get_max_position_embeddings(config) -> Optional[int]: @@ -1653,9 +1677,12 @@ async def get_model_config( ) except Exception as e: - logger.error(f"Error getting model config: {e}", exc_info = True) - raise HTTPException( - status_code = 500, detail = f"Failed to get model config: {str(e)}" + raise log_and_http_error( + e, + 500, + "Failed to get model config", + event = "models.get_model_config_failed", + log = logger, ) @@ -1710,9 +1737,12 @@ async def scan_loras( return LoRAScanResponse(loras = lora_list, outputs_dir = resolved_outputs_dir) except Exception as e: - logger.error(f"Error scanning LoRAs: {e}", exc_info = True) - raise HTTPException( - status_code = 500, detail = f"Failed to scan LoRA adapters: {str(e)}" + raise log_and_http_error( + e, + 500, + "Failed to scan LoRA adapters", + event = "models.scan_loras_failed", + log = logger, ) @@ -2044,7 +2074,7 @@ async def delete_finetuned_model( ) raise HTTPException( status_code = 500, - detail = f"Failed to delete fine-tuned model: {str(e)}", + detail = "Failed to delete fine-tuned model", ) @@ -2075,9 +2105,12 @@ async def get_lora_base_model( except HTTPException: raise except Exception as e: - logger.error(f"Error getting LoRA base model: {e}", exc_info = True) - raise HTTPException( - status_code = 500, detail = f"Failed to get base model: {str(e)}" + raise log_and_http_error( + e, + 500, + "Failed to get base model", + event = "models.get_lora_base_model_failed", + log = logger, ) @@ -2102,9 +2135,12 @@ async def check_vision_model( ) except Exception as e: - logger.error(f"Error checking vision model: {e}", exc_info = True) - raise HTTPException( - status_code = 500, detail = f"Failed to check vision model: {str(e)}" + raise log_and_http_error( + e, + 500, + "Failed to check vision model", + event = "models.check_vision_model_failed", + log = logger, ) @@ -2132,9 +2168,12 @@ async def check_embedding_model( ) except Exception as e: - logger.error(f"Error checking embedding model: {e}", exc_info = True) - raise HTTPException( - status_code = 500, detail = f"Failed to check embedding model: {str(e)}" + raise log_and_http_error( + e, + 500, + "Failed to check embedding model", + event = "models.check_embedding_model_failed", + log = logger, ) @@ -2247,7 +2286,7 @@ async def get_gguf_variants( logger.error(f"Error listing GGUF variants for '{repo_id}': {e}", exc_info = True) raise HTTPException( status_code = 500, - detail = f"Failed to list GGUF variants: {str(e)}", + detail = "Failed to list GGUF variants", ) @@ -2722,7 +2761,7 @@ async def delete_cached_model( logger.error(f"Error deleting cached model {repo_id}: {e}", exc_info = True) raise HTTPException( status_code = 500, - detail = f"Failed to delete cached model: {str(e)}", + detail = "Failed to delete cached model", ) @@ -2763,8 +2802,10 @@ async def list_checkpoints( models = models, ) except Exception as e: - logger.error(f"Error listing checkpoints: {e}", exc_info = True) - raise HTTPException( - status_code = 500, - detail = f"Failed to list checkpoints: {str(e)}", + raise log_and_http_error( + e, + 500, + "Failed to list checkpoints", + event = "models.list_checkpoints_failed", + log = logger, ) diff --git a/studio/backend/routes/providers.py b/studio/backend/routes/providers.py index 5d4bd46e62..4eaa007750 100644 --- a/studio/backend/routes/providers.py +++ b/studio/backend/routes/providers.py @@ -40,6 +40,7 @@ from models.providers import ( ProviderUpdate, ) from storage import providers_db +from utils.utils import safe_curated_detail, log_and_http_error logger = structlog.get_logger(__name__) @@ -254,10 +255,15 @@ async def test_provider( models_count = len(models), ) except Exception as exc: - logger.warning("Provider test failed for %s: %s", payload.provider_type, exc) + logger.error( + "providers.test_failed", + provider_type = payload.provider_type, + error = str(exc), + exc_info = True, + ) return ProviderTestResult( success = False, - message = f"Connection failed: {exc}", + message = f"Connection failed: {safe_curated_detail(exc)}", models_count = None, ) finally: @@ -375,10 +381,12 @@ async def list_provider_models( for m in models ] except Exception as exc: - logger.error("Failed to list models from %s: %s", payload.provider_type, exc) - raise HTTPException( - status_code = 502, - detail = f"Failed to list models from {payload.provider_type}: {exc}", + raise log_and_http_error( + exc, + 502, + f"Failed to list models from {payload.provider_type}.", + event = "providers.list_models_failed", + log = logger, ) finally: await client.close() diff --git a/studio/backend/routes/settings.py b/studio/backend/routes/settings.py index 275fbb678b..b6fbeeee7a 100644 --- a/studio/backend/routes/settings.py +++ b/studio/backend/routes/settings.py @@ -1,10 +1,12 @@ # SPDX-License-Identifier: AGPL-3.0-only # Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends from pydantic import BaseModel, Field from auth.authentication import get_current_subject +from loggers import get_logger +from utils.utils import safe_error_detail, log_and_http_error from utils.upload_limits import ( MAX_UPLOAD_LIMIT_MB, MIN_UPLOAD_LIMIT_MB, @@ -17,6 +19,8 @@ from utils.upload_limits import ( router = APIRouter() +logger = get_logger(__name__) + class UploadLimitPayload(BaseModel): max_upload_size_mb: int = Field(..., ge = MIN_UPLOAD_LIMIT_MB, le = MAX_UPLOAD_LIMIT_MB) @@ -55,5 +59,11 @@ def update_upload_limit( try: limit_mb = set_upload_limit_mb(payload.max_upload_size_mb) except ValueError as exc: - raise HTTPException(status_code = 400, detail = str(exc)) from exc + raise log_and_http_error( + exc, + 400, + safe_error_detail(exc, fallback = "Invalid upload limit."), + event = "settings.update_upload_limit_failed", + log = logger, + ) from exc return _upload_limit_response(limit_mb) diff --git a/studio/backend/routes/training.py b/studio/backend/routes/training.py index 41a9e15562..5b4267b2ef 100644 --- a/studio/backend/routes/training.py +++ b/studio/backend/routes/training.py @@ -51,6 +51,8 @@ except ImportError: # Auth from auth.authentication import get_current_subject +from utils.utils import log_and_http_error + from models import ( TrainingStartRequest, TrainingJobResponse, @@ -171,7 +173,9 @@ async def start_training( request.resume_from_checkpoint ) except ValueError as e: - raise HTTPException(status_code = 400, detail = str(e)) + # Deliberate user-facing validation message. + validation_message = str(e) + raise HTTPException(status_code = 400, detail = validation_message) resume_run = get_resumable_run_by_output_dir(resume_output_dir) if not resume_run or not can_resume_run(resume_run): @@ -319,12 +323,16 @@ async def start_training( except ValueError as e: logger.warning("Rejected training GPU selection: %s", e) - raise HTTPException(status_code = 400, detail = str(e)) + # Deliberate user-facing GPU-selection validation message. + validation_message = str(e) + raise HTTPException(status_code = 400, detail = validation_message) except Exception as e: - logger.error(f"Error starting training: {e}", exc_info = True) - raise HTTPException( - status_code = 500, - detail = f"Failed to start training: {str(e)}", + raise log_and_http_error( + e, + 500, + "Failed to start training", + event = "training.start_failed", + log = logger, ) @@ -358,9 +366,12 @@ async def stop_training( ) except Exception as e: - logger.error(f"Error stopping training: {e}", exc_info = True) - raise HTTPException( - status_code = 500, detail = f"Failed to stop training: {str(e)}" + raise log_and_http_error( + e, + 500, + "Failed to stop training", + event = "training.stop_failed", + log = logger, ) @@ -412,10 +423,12 @@ async def reset_training( except HTTPException: raise except Exception as e: - logger.error(f"Error resetting training: {e}", exc_info = True) - raise HTTPException( - status_code = 500, - detail = f"Failed to reset training: {str(e)}", + raise log_and_http_error( + e, + 500, + "Failed to reset training", + event = "training.reset_failed", + log = logger, ) @@ -505,9 +518,12 @@ async def get_training_status( ) except Exception as e: - logger.error(f"Error getting training status: {e}", exc_info = True) - raise HTTPException( - status_code = 500, detail = f"Failed to get training status: {str(e)}" + raise log_and_http_error( + e, + 500, + "Failed to get training status", + event = "training.status_failed", + log = logger, ) @@ -545,9 +561,12 @@ async def get_training_metrics( ) except Exception as e: - logger.error(f"Error getting training metrics: {e}", exc_info = True) - raise HTTPException( - status_code = 500, detail = f"Failed to get training metrics: {str(e)}" + raise log_and_http_error( + e, + 500, + "Failed to get training metrics", + event = "training.metrics_failed", + log = logger, ) diff --git a/studio/backend/utils/utils.py b/studio/backend/utils/utils.py index 4e61a5b969..e95ef08a7d 100644 --- a/studio/backend/utils/utils.py +++ b/studio/backend/utils/utils.py @@ -17,6 +17,67 @@ import tempfile logger = get_logger(__name__) +# ── Client-safe error helpers ─────────────────────────────────── +# Never return raw exception text to clients (it can leak paths/internals); +# log the full exception server-side and return a generic message. + + +def safe_error_detail( + error: Exception, fallback: str = "An internal error occurred" +) -> str: + """Map a caught exception to a generic, client-safe message. + + Never includes raw ``str(error)`` (which can leak internal paths or stack + detail); known transient conditions get a friendlier hint. Always log the + real exception server-side (e.g. via ``log_and_http_error``) for diagnosis. + """ + text = str(error).lower() + if ( + isinstance(error, (ConnectionError, TimeoutError)) + or "connection" in text + or "timed out" in text + or "timeout" in text + ): + return "Could not reach an upstream service. Please try again." + if "out of memory" in text or "cuda error" in text: + return "Ran out of memory. Try a smaller model or shorter input." + return fallback + + +def safe_curated_detail( + error: Exception, fallback: str = "An internal error occurred" +) -> str: + """Client-safe text for curated domain/validation exceptions meant for the user. + + Keeps the message (paths stripped) instead of a generic fallback; use for known + exception types, keep ``safe_error_detail`` for generic ``Exception``. + """ + from utils.native_path_leases import redact_native_paths + + msg = redact_native_paths(str(error)).strip() + return msg or fallback + + +def log_and_http_error( + error: Exception, + status_code: int, + public_message: str, + *, + event: str = "request_failed", + log = None, +): + """Log ``error`` in full server-side and return an ``HTTPException`` whose + ``detail`` is only ``public_message`` -- never the raw exception text. + + Usage: raise log_and_http_error(e, 500, "Failed to start training") + """ + from fastapi import HTTPException + + # Works for both structlog and stdlib loggers; exc_info=error logs its traceback. + (log or logger).error(f"{event}: {error}", exc_info = error) + return HTTPException(status_code = status_code, detail = public_message) + + @contextmanager def without_hf_auth(): """