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.
This commit is contained in:
parent
0881a7a5d7
commit
6e8bf4d51b
3 changed files with 53 additions and 9 deletions
|
|
@ -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:; "
|
||||
|
|
|
|||
|
|
@ -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"""
|
||||
|
|
|
|||
|
|
@ -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<string> {
|
||||
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})`;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue