From 6e8bf4d51ba69181c8a6d1d532b2a0f2267938e8 Mon Sep 17 00:00:00 2001 From: Roland Tannous <115670425+rolandtannous@users.noreply.github.com> Date: Wed, 13 May 2026 19:40:54 +0400 Subject: [PATCH] studio: fix training page regressions from the security hardening pass (#5409) * studio: allow huggingface.co and datasets-server.huggingface.co in CSP connect-src The security hardening pass (0881a7a5) added connect-src 'self', which blocked the Training page's direct browser calls to HuggingFace. Model search (@huggingface/hub listModels/modelInfo/whoAmI -> huggingface.co) and dataset subset/split discovery (datasets-server.huggingface.co/splits) both returned nothing as a result. Extend connect-src to permit the two HF hosts the SPA actually talks to. No other directive changes; HF tokens still stay client-side. * studio: format FastAPI 422 detail arrays in training error messages readError in train-api.ts stringified payload.detail directly. On a 422 the detail is an array of {loc, msg} objects, which JS coerces to '[object Object],[object Object]' -- the UI showed that instead of the actual validator message. Format the array into 'field.path: msg; ...' so the offending field and the validator's message surface in the UI and toast. * studio: allow num_epochs/max_steps = 0 sentinel through TrainingStartRequest The hyperparameter validators added in the security pass rejected 0 for both num_epochs and max_steps. But Studio's steps-vs-epochs toggle uses 0 as a sentinel: when training by max_steps the frontend sends num_epochs=0, and when training by epochs it sends max_steps=0. The trainer expects this and ignores the zeroed field. Widen both validators to [0, MAX]. They still catch the actual out-of-range and non-integer inputs they were added for. * studio: reject TrainingStartRequest when num_epochs and max_steps are both 0 Each field's validator accepts 0 as a "use the other one" sentinel, but on their own they don't catch the case where both are 0 (or max_steps is None and num_epochs is 0). That payload would otherwise produce a no-op training job. Add a model-level validator that rejects it with a clear 422 message. * studio: add Optional[int] type hints to _check_max_steps and _check_warmup_steps Brings these two validators in line with the rest of the TrainingStartRequest validators in the same file, which all carry explicit cls/v/return hints. --- studio/backend/main.py | 2 +- studio/backend/models/training.py | 25 +++++++++---- .../src/features/training/api/train-api.ts | 35 +++++++++++++++++-- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/studio/backend/main.py b/studio/backend/main.py index 81964c98e0..4955e988e6 100644 --- a/studio/backend/main.py +++ b/studio/backend/main.py @@ -278,7 +278,7 @@ def _build_csp(script_nonce: "str | None" = None) -> str: "img-src 'self' data: blob: https://t0.gstatic.com " "https://t1.gstatic.com https://t2.gstatic.com " "https://t3.gstatic.com; " - "connect-src 'self'; " + "connect-src 'self' https://huggingface.co https://datasets-server.huggingface.co; " "style-src 'self' 'unsafe-inline'; " f"{script_src}; " "font-src 'self' data:; " diff --git a/studio/backend/models/training.py b/studio/backend/models/training.py index 6b5e95e188..31f1d575d7 100644 --- a/studio/backend/models/training.py +++ b/studio/backend/models/training.py @@ -130,20 +130,23 @@ class TrainingStartRequest(BaseModel): @field_validator("num_epochs") @classmethod def _check_num_epochs(cls, v: int) -> int: + # 0 is a sentinel meaning "use max_steps instead"; the frontend's + # steps-vs-epochs toggle sends it. if v is None: return 1 - if v < 1 or v > _MAX_EPOCHS: - raise ValueError(f"num_epochs must be in [1, {_MAX_EPOCHS}] (got {v!r})") + if v < 0 or v > _MAX_EPOCHS: + raise ValueError(f"num_epochs must be in [0, {_MAX_EPOCHS}] (got {v!r})") return v @field_validator("max_steps") @classmethod - def _check_max_steps(cls, v): + def _check_max_steps(cls, v: Optional[int]) -> Optional[int]: + # 0 is the frontend's sentinel for "use num_epochs instead". if v is None: return v - if not isinstance(v, int) or v < 1 or v > _MAX_STEPS: + if not isinstance(v, int) or v < 0 or v > _MAX_STEPS: raise ValueError( - f"max_steps must be a positive int <= {_MAX_STEPS} (got {v!r})" + f"max_steps must be a non-negative int <= {_MAX_STEPS} (got {v!r})" ) return v @@ -158,7 +161,7 @@ class TrainingStartRequest(BaseModel): @field_validator("warmup_steps") @classmethod - def _check_warmup_steps(cls, v): + def _check_warmup_steps(cls, v: Optional[int]) -> Optional[int]: if v is None: return v if not isinstance(v, int) or v < 0 or v > _MAX_STEPS: @@ -321,6 +324,16 @@ class TrainingStartRequest(BaseModel): description = "Physical GPU indices to use, for example [0, 1]. Omit or pass [] to use automatic selection. Explicit gpu_ids are unsupported when the parent CUDA_VISIBLE_DEVICES uses UUID/MIG entries.", ) + @model_validator(mode = "after") + def _check_steps_or_epochs(self) -> "TrainingStartRequest": + # num_epochs and max_steps each accept 0 as a "use the other one" + # sentinel. If both resolve to 0 there's nothing to train against. + if (self.max_steps is None or self.max_steps == 0) and self.num_epochs == 0: + raise ValueError( + "Either num_epochs or max_steps must be > 0; both cannot be 0." + ) + return self + class TrainingJobResponse(BaseModel): """Immediate response when training is initiated""" diff --git a/studio/frontend/src/features/training/api/train-api.ts b/studio/frontend/src/features/training/api/train-api.ts index bb18fb34aa..781dbe6139 100644 --- a/studio/frontend/src/features/training/api/train-api.ts +++ b/studio/frontend/src/features/training/api/train-api.ts @@ -17,10 +17,41 @@ function isAbortError(error: unknown): boolean { return error instanceof DOMException && error.name === "AbortError"; } +type FastApiValidationError = { + loc?: unknown[]; + msg?: string; +}; + +function formatDetail(detail: unknown): string | null { + if (typeof detail === "string" && detail) return detail; + if (!Array.isArray(detail)) return null; + const parts = detail + .map((entry) => { + if (!entry || typeof entry !== "object") return ""; + const { loc, msg } = entry as FastApiValidationError; + const path = Array.isArray(loc) + ? loc.filter((segment) => segment !== "body").join(".") + : ""; + const message = typeof msg === "string" ? msg : ""; + if (path && message) return `${path}: ${message}`; + return path || message; + }) + .filter(Boolean); + return parts.length > 0 ? parts.join("; ") : null; +} + async function readError(response: Response): Promise { try { - const payload = (await response.json()) as { detail?: string; message?: string }; - return payload.detail || payload.message || `Request failed (${response.status})`; + const payload = (await response.json()) as { + detail?: unknown; + message?: string; + }; + const formattedDetail = formatDetail(payload.detail); + if (formattedDetail) return formattedDetail; + if (typeof payload.message === "string" && payload.message) { + return payload.message; + } + return `Request failed (${response.status})`; } catch { return `Request failed (${response.status})`; }