Commit graph

4 commits

Author SHA1 Message Date
Daniel Han
187144d4e7
Reduce and tighten code comments and docstrings repo-wide (#6095)
Trim and tighten code comments and docstrings across the repository. Comment-only: every changed file verified code-identical to main via AST/token comparison.
2026-06-08 23:09:51 -07:00
Daniel Han
8292e699e4
Studio: make code comments and docstrings more succinct (#6029)
Trim and tighten code comments and docstrings across studio/ Python. Comment-only: every changed file verified code-identical to main via AST/token comparison.
2026-06-08 23:07:28 -07:00
Dariton4000
dac2aeda1a
Studio: expose image size setting in training UI (#5743)
* Studio: add VLM image-size control for training

  Studio vision fine-tuning had no explicit way to cap image resolution, so
  users could not trade visual detail against context and memory use from the
  training UI, YAML config, or API payload. :) Add a nullable `vision_image_size`
  setting that keeps the current model default when unset and applies a
  max-side resize when provided.

  - Add `vision_image_size` to the training request model, route payload, backend
    training config, and frontend API/types plumbing.
  - Validate the value server-side as either null or an integer in the supported
    256-2048 range.
  - Surface an Image Size selector for vision LoRA training with Default plus
    common preset sizes.
  - Include the value in training start payloads only for image-dataset vision
    models, and serialize it into vision-aware YAML configs.
  - Map backend model defaults back into the training store and reset the value
    when reapplying model defaults.
  - Pass the resize through the Torch trainer via `UnslothVisionDataCollator`
    using max-dimension semantics.
  - Apply the same max-dimension resize in the MLX VLM path before mlx-vlm's
    internal collation, preserving aspect ratio and avoiding upscaling.
  - Add backend validation coverage and MLX resize-size tests for the new
    behavior.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: thread vision_image_size into DeepSeek OCR + writable MLX ndarray

- trainer.py: DeepSeek OCR collator now honors the new vision_image_size
  setting as image_size. Falls back to 640 when null. base_size stays at
  1024 and crop_mode stays True so the Gundam preset's dynamic cropping
  of large documents keeps working.
- worker.py: _resize_mlx_vlm_image returns np.array(image, copy=True)
  instead of np.asarray(image). The PIL view from np.asarray is not
  writable, which makes HF VLM processors emit "The given NumPy array
  is not writable, and PyTorch does not support non-writable tensors..."
  when they call torch.from_numpy. copy=True keeps the same shape and
  dtype but produces a writable buffer.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: align YAML export gate with API mapper + extend Image Size dropdown

- training-section.tsx: handleSaveConfig now passes
  isVisionModel && isDatasetImage === true to serializeConfigToYaml,
  matching buildTrainingStartPayload. Stops vision_image_size from
  leaking into exported YAML for text-only datasets where the API
  would have sent null.
- params-section.tsx: add 256 to visionImageSizePresets so the
  dropdown spans the validator's full [256, 2048] range. Also render
  a synthetic SelectItem for the current value when it was loaded
  from YAML or model defaults and is not in the preset list, so the
  controlled Select always shows the active size.

* Studio: validate vision_image_size in YAML/model-default loader

mapBackendModelConfigToTrainingPatch now mirrors the backend validator
at studio/backend/models/training.py:169 by dropping any value that is
not an integer in [256, 2048]. Pre-fix, an imported YAML like
vision_image_size: 4096 or 640.5 would land in the store and the UI
would happily display it, only to fail when Start Training posted to
the backend. With this guard the store never holds a value the backend
would reject.

* Studio: precise error messages for invalid vision_image_size inputs

Switch the field_validator to mode="before" so True/False surface as
bool (not Pydantic's coerced 1/0) and give a precise
"must be an integer or null" message instead of the misleading
"must be in [256, 2048] (got 1)". Also explicitly accepts numpy
Integral and integral Real scalars so YAML or programmatic callers
using numpy ints keep working.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: test that bool inputs yield the precise 'integer or null' error

Regression guard for the validator switch to mode="before". Pre-fix,
vision_image_size: True was rejected with "must be in [256, 2048]
(got 1)" because Pydantic coerced before our check ran. New test
asserts the message now reads "integer or null".

* Studio: tighten vision_image_size loader + YAML save + MLX rounding

Round 2 of follow-up review surfaced three usability issues:

- model-defaults.ts: switching to a model whose backend YAML omits
  vision_image_size now explicitly resets the store value to null.
  Pre-fix, a stale 2048 from a previous model would silently apply
  to the new run because every checked-in model-default file omits
  the key.
- training-section.tsx: handleSaveConfig now includes vision fields
  unless isDatasetImage is definitively false. isDatasetImage is null
  during dataset checks, after dataset edits, and on import; treating
  unknown as "drop" would silently lose the user's selection in those
  windows. Confirmed-text-only datasets still drop the value.
- worker.py: _mlx_vlm_max_resized_size now mirrors the Torch collator's
  integer formula (w * size + size_func // 2) // size_func instead of
  Python round(), which uses banker's rounding and disagreed by 1px on
  half-pixel inputs like 333x1000 with target 500 (was 166, now 167).
  Test_mlx_training_worker_config gains parity assertions.

* Studio: reset vision_image_size in the model-config error fallback path

mapBackendModelConfigToTrainingPatch resets stale image size on the
success path, but if the /api/models/config endpoint throws,
training-config-store.ts falls through to checkVisionModel and only
updates capability flags. Pre-fix that left a stale 2048 (or any
prior selection) in the store, so once dataset detection marked the
new dataset as image, the next training start would silently apply
the previous model's size. The error branch now also resets to the
DEFAULT_HYPERPARAMS.visionImageSize sentinel.

* Studio: revert DeepSeek OCR Image Size knob + move missing-key reset

Round 3 of the parallel-reviewer pass surfaced two issues that I had
introduced earlier in this PR's follow-ups.

- trainer.py: my prior change threaded vision_image_size into the
  DeepSeek OCR collator's image_size argument. The collator's
  (image_size, base_size, crop_mode) is a single preset
  (Tiny / Small / Base / Large / Gundam); changing image_size in
  isolation desynchronizes the per-crop pixel grid from num_queries
  downstream and produces wrong token grids on documents larger than
  the per-crop tile. The fix pins the collator back at the Gundam
  preset and logs a clear "ignored for DeepSeek OCR" notice when the
  user has selected a non-default Image Size.
- model-defaults.ts + training-config-store.ts: the round 4 fix that
  reset visionImageSize when a model YAML omitted the key also fired
  on same-model reloads (ensureModelDefaultsLoaded re-fires on page
  refresh), wiping a value the user had just selected. The reset is
  now in setSelectedModel, gated on selectedModel != previousModel,
  so true model switches still clear stale values while reloads keep
  the user's selection.

* Studio: extend DeepSeek OCR Image Size exclusion to MLX + frontend

Round 4 of the parallel-reviewer pass flagged that the Torch trainer
exclusion I added did not have a matching MLX guard, and that the UI
still offered the dropdown for DeepSeek OCR even though the backend
ignores it.

- worker.py: _run_mlx_training now mirrors the Torch exclusion. When
  the model name matches DeepSeek OCR, vision_image_size is forced
  back to None before _adapt_for_mlx_vlm sees it, so dataset images
  pass through unchanged just like the Torch path. Emits a clear
  status line when this happens.
- params-section.tsx: the Image Size Row is now gated on
  showVisionImageSize (showVisionLora && !isDeepseekOcr) instead of
  showVisionLora alone, so DeepSeek OCR users no longer see a control
  that silently has no effect.
- mappers.ts: buildTrainingStartPayload sends null for vision_image_size
  whenever the selected model is DeepSeek OCR, so the backend log line
  about ignoring the value never fires from a UI-driven start.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: tighten YAML import/save for vision_image_size

Two YAML-path asymmetries that could leak a stale image size into
training:

- parseYamlConfig now treats a missing training.vision_image_size as
  null. Without this, importing a YAML saved before this feature (or
  any config that omits the key) preserved whatever value the user had
  previously set on a different model. The model-defaults reload path
  still uses Object.hasOwn so same-model defaults reloads do not wipe
  a manual selection; only file import normalises the missing key.

- handleSaveConfig now passes a DeepSeek-OCR-specific guard to
  serializeConfigToYaml so saved YAML matches what the API mapper
  actually sends. Previously a state with visionImageSize set could
  emit the key even though Studio ignored it at training time for
  DeepSeek OCR, and a later import for a non-DeepSeek vision model
  would activate the stale value.

serializeConfigToYaml gains an optional third parameter
includeVisionImageSize defaulting to includeVisionFields, preserving
the existing 2-arg call signature for backwards compatibility.

* Studio: also reset vision_image_size when YAML lacks a training section

Round 9's parseYamlConfig normalization only fired when the YAML had a
training mapping that omitted vision_image_size. A lora-only or
logging-only YAML (or one with `training: null`) still left trainingObj
unset, the mapper saw no vision_image_size key, and the previously
selected store value persisted into the next training run.

Now an absent or null training section is synthesised as
{ vision_image_size: null } so model-defaults.ts always patches
visionImageSize back to Default on file import. Same-model defaults
reloads still preserve manual choices via the existing Object.hasOwn
gate in mapBackendModelConfigToTrainingPatch.

* Studio: unify parseYamlConfig non-object training handling

A fresh static review (Opus subagent) flagged P3-1: parseYamlConfig
only synthesised vision_image_size: null when raw.training was either
absent or a plain object missing the key. If raw.training is a scalar
or an array (malformed but still parseable), the value was passed
through unchanged, the mapper's Object.hasOwn returned false, and any
previously selected visionImageSize persisted - the same stale-state
leak the lora-only fallback was added to close.

Treat any non-plain-object raw.training (null, array, scalar) as a
malformed/missing section and reset to { vision_image_size: null }.

* Studio: tighten code comments for vision_image_size path

* Studio: tighten vision_image_size validator + restore lost comment context

Two issues surfaced by a fresh adversarial review of the validator:

1. v.strip().lstrip("+-").isdigit() let "++512" / "--256" / "+-+512"
   slip past the gate, then int("++512") raised an uncaught ValueError
   and Pydantic surfaced "invalid literal for int() with base 10: '++512'"
   instead of the contracted "vision_image_size must be an integer or null".

2. str.isdigit() returns True for Unicode digit families (full-width '512',
   Arabic-Indic '٥١٢', Devanagari '१०२४'), and int() coerces them, so the
   value reaching the backend wasn't the ASCII the user typed.

Replaced the lstrip+isdigit pair with re.fullmatch(r'[+-]?[0-9]+', stripped),
which rejects both shapes with the precise error and accepts the documented
ones ('256', '+512', ' 1024 '). Added 8 regression test cases covering
multi-sign strings, lone sign, and the three Unicode digit families.

Also restored comment context lost in f9c39331:
- model-defaults.ts: name studio/backend/models/training.py:_check_vision_image_size
  as the spec the [256, 2048] range mirrors, so a maintainer changing the
  cap in one file can find the other.
- training-section.tsx: enumerate the three windows in which isDatasetImage
  is null (before a check, after dataset edits, on import) so a future
  maintainer doesn't simplify the gate to `isCheckingDataset`.
- worker.py: qualify the writable-ndarray comment with "when a resize is
  requested" so it doesn't misadvertise the resize=None early-return.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Daniel Han <danielhanchen@gmail.com>
2026-05-27 05:01:24 -07:00
Daniel Han
0881a7a5d7
studio: security and hardening pass (auth rate-limit, sandbox, path containment, schema validation, headers) (#5375)
* studio: contain export and dataset paths under their configured roots

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

The fix is two-layered:

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

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

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

Verified end to end:
- POST /api/export/export/lora {"save_directory":"/tmp/foo"} -> 422
  "save_directory must be a name or relative path under the export
  root; absolute paths are rejected". /tmp/foo is not created.
- "../../etc/escape" -> 422 "may not contain '..' segments".
- save_directory="my_subdir" -> still accepted (400 only because the
  test had no checkpoint loaded yet, not because of validation).
- Internal idempotent re-resolve via resolve_export_dir(absolute path
  that is already under exports_root) returns the same path unchanged.

* studio/sandbox: harden bash + python tool execution

The sandboxed Bash and Python tool channels in Chat ran with a thin
preexec hook (PR_SET_NO_NEW_PRIVS + RLIMIT_FSIZE only). Bash had a
small word blocklist; Python had an AST safety pass aimed at
signal-tampering and shell-escape primitives. An audit pass showed
several gaps that a tool-calling model could trigger inadvertently:

- bash curl/wget/nc reached AWS IMDSv2 and returned live STS
  credentials for the instance role.
- python "import socket; s.connect((169.254.169.254, 80))"
  reached the same endpoint regardless of the bash blocklist.
- "cat /etc/passwd" was blocked at the bash side (because "passwd"
  is in the blocklist), but "open('/etc/passwd').read()" in Python
  happily returned its contents.
- "chr(115)+chr(117)+chr(100)+chr(111)" style dynamic-arg
  construction slipped through the AST shell-escape check.
- The supervisor used proc.kill() on timeout, which only signals
  the immediate pid; bash-backgrounded children survived. A fork
  bomb could spawn for the full 300s timeout window.
- Session work directories under ~/studio_sandbox/<id>/ were
  created with default umask (0o755), so any other UID on the host
  could enumerate them.
- session_id sanitisation used a one-shot str.replace("..",""),
  which is non-iterative and a small footgun.

This commit takes a conservative middle path: the sandbox still
runs as the Studio UID with no namespace tricks where the kernel
disallows them, but every chokepoint is tightened.

_sandbox_preexec now:
- calls os.setsid() so children share a process group; the
  supervisor uses os.killpg(SIGKILL) on timeout/cancel so
  backgrounded children die with the parent (new _kill_process_tree
  helper, wired into _cancel_watcher and both _bash_exec /
  _python_exec timeout branches).
- calls os.umask(0o077) so files the child writes default to 0o600.
- applies PR_SET_PDEATHSIG=SIGKILL so an orphaned child dies if
  Studio exits.
- best-effort unshare(CLONE_NEWNET) for a private network namespace
  (failure is logged and swallowed; defense-in-depth is still in
  place via the bash blocklist and the AST checker below).
- sets RLIMIT_NPROC=10000 (tunable via UNSLOTH_STUDIO_SANDBOX_NPROC),
  RLIMIT_AS=8GB, RLIMIT_CPU=300, RLIMIT_NOFILE=1024. The 10k NPROC
  figure is chosen to sit well above the ~500 LWPs a healthy Studio
  + llama-server combination already uses while still capping a
  runaway fork bomb. NPROC counts LWPs per real UID, so a lower
  figure (e.g. 256) starves legitimate bash forks
  ("bash: fork: retry: Resource temporarily unavailable").

_get_workdir:
- rejects session_id that doesn't match [A-Za-z0-9_-]{1,64};
  non-matching values bucket into a shared "_invalid" dir.
- chmod 0o700 on both the workdir and on ~/studio_sandbox/ so
  other UIDs cannot read another session's contents.

_BLOCKED_COMMANDS_COMMON gains: doas, pkexec, halt, poweroff, curl,
wget, nc, ncat, netcat, socat, ssh, scp, sftp, rsync, eval, source.
The intent is to keep general bash usage working (echo, ls, pipes,
loops, for, head, etc.) while denying the obvious egress and
escalation paths.

The AST checker (_check_signal_escape_patterns) is split into the
existing shell/signal/loop checks plus a new narrow IO denylist:
- Always flag non-literal args to anything in _SHELL_EXEC_FUNCS,
  not just _STRING_SHELL_FUNCS. Closes the dynamic-arg bypass.
- Reject calls to socket.create_connection, socket.socket().connect,
  urllib.request.urlopen, http.client.HTTP*Connection, requests.*,
  httpx.* whose literal host argument is in a cloud-metadata
  denylist (169.254.169.254 + 169.254.* + 100.64.*, plus the
  GCP/Alibaba/ECS metadata hostnames and IPv6 link-local). Public
  hosts (example.com, huggingface.co, ...) still work. Dynamic
  hosts cannot be statically blocked; mitigated by the bash
  blocklist + the netns where the kernel allows it.
- Reject literal open("/etc/passwd"), /etc/shadow, /etc/sudoers,
  /etc/ssh/*, and /proc/<pid>/environ. Other files
  (/etc/os-release, /etc/hostname, /tmp/*, user dirs) still work.

The _check_code_safety summariser is updated to include the new
network_calls and sensitive_file_reads buckets in its error string.

Regression-checked: echo, sleep, ls /tmp, for loops, piped helpers
(echo a | tr a A), urllib.request.urlopen("http://example.com"),
socket.getaddrinfo("example.com",80), open("/etc/os-release"),
open("/tmp/...","w") all still succeed. curl, wget, nc, ssh, rm,
socket.create_connection(("169.254.169.254",80)),
open("/etc/passwd"), open("/proc/self/environ") all correctly
blocked.

* studio: rate-limit login, rotate refresh tokens, add logout, security headers, gate bootstrap injection

A pass over the auth surface found a cluster of related issues that this
commit closes together.

Login (routes/auth.py):
- Add an in-memory per-IP login rate limiter. Five failed POSTs to
  /api/auth/login inside a 60s window produce 429 with Retry-After.
  A successful login clears the bucket. Previously 30 wrong passwords
  in under one second was accepted as 30x 401, which combined with
  the (now fixed) admin-username leak from /api/auth/status made
  brute-force trivial against a small password.

Logout (routes/auth.py):
- New POST /api/auth/logout returns 204 and calls
  storage.revoke_user_refresh_tokens(subject) so the refresh token
  is no longer valid. Previously POST /api/auth/logout returned 405
  and there was no way to invalidate refresh tokens short of
  changing the password. Frontend session.ts already calls
  clearAuthTokens() to drop localStorage; the new endpoint lets the
  client also tell the server to revoke server-side state.

Refresh-token rotation (routes/auth.py + auth/storage.py):
- New storage.consume_refresh_token(token) atomically validates +
  deletes a refresh token, returning (username, is_desktop). The
  /api/auth/refresh handler now mints both a new access AND a new
  refresh token; the supplied token becomes invalid. Replaying a
  consumed refresh returns 401 "Invalid or expired refresh token".
  The previous refresh_access_token helper is left in place for
  callers that intentionally want the non-rotating shape; nothing
  in the route layer uses it now.

/api/auth/status no longer leaks default_username (models/auth.py +
routes/auth.py):
- AuthStatusResponse.default_username becomes Optional[str] with a
  None default; the handler always returns None. The frontend already
  hardcodes HIDDEN_LOGIN_USERNAME = "unsloth" (auth-form.tsx:82), so
  no UI change is required.

window.__UNSLOTH_BOOTSTRAP__ no longer auto-injects (main.py):
- _inject_bootstrap is now opt-in via the
  UNSLOTH_STUDIO_INJECT_BOOTSTRAP env var. The previous default
  (inject whenever requires_password_change is true) embedded the
  plaintext bootstrap password into the first-boot HTML for any
  caller that hit /, /change-password, or any unknown SPA path.
  Browser extensions and any XSS payload on the page could read it
  trivially. With the new gate the bootstrap password lives only in
  the auth/.bootstrap_password file (mode 0o600) where it has always
  been; users typing it into a current-password field is the right
  UX. routes/auth.py:change_password also clears
  app.state.bootstrap_password defensively.

Security headers + server fingerprint (main.py + run.py):
- New SecurityHeadersMiddleware adds Content-Security-Policy,
  X-Frame-Options: DENY, X-Content-Type-Options: nosniff,
  Referrer-Policy: no-referrer,
  Permissions-Policy: camera=(), microphone=(), geolocation=(),
  interest-cohort=(), and stamps server: unsloth-studio so the
  generic uvicorn banner no longer fingerprints the stack. The
  uvicorn.Config gains server_header=False so it stops emitting its
  own Server header.

/api/health minimisation (main.py):
- Unauthenticated GET /api/health returns just
  {"status":"healthy","timestamp":...} so load-balancer liveness
  probes keep working without leaking version, device_type,
  chat_only, desktop_protocol_version, or studio_root_id to
  arbitrary callers. A request that presents a valid Bearer token
  still gets the full diagnostic payload so internal launchers and
  sibling-Studio detection (which compares studio_root_id) keep
  working.

Verification:
- 30 wrong-password POSTs to /api/auth/login -> first 5 = 401, 6th
  through 30th = 429.
- POST /api/auth/logout with a fresh token -> 204. The matching
  refresh token then fails 401.
- Login -> R1; /api/auth/refresh with R1 -> new access + R2 (R2 !=
  R1); /api/auth/refresh with R1 again -> 401; /api/auth/refresh
  with R2 -> still succeeds once and rotates again.
- curl /api/auth/status -> default_username: null.
- curl http://127.0.0.1/ does not contain __UNSLOTH_BOOTSTRAP__.
- curl -I / shows CSP, X-Frame-Options: DENY,
  X-Content-Type-Options: nosniff, Referrer-Policy: no-referrer,
  Permissions-Policy, and server: unsloth-studio.
- curl /api/health unauthenticated -> {status, timestamp} only.
  curl with Authorization: Bearer <valid> -> full payload.
- Existing /api/system, /api/models/list, /api/train/status,
  /api/inference/status, /api/auth/api-keys, login flow, SPA root
  all still return 200 after the changes (regression smoke).

* studio: add SecurityHeadersMiddleware, MaxBodyMiddleware, /recipes redirect, gate _inject_bootstrap, minimise /api/health

This commit lands the main.py-side changes that share a single
middleware-registration spot. They are kept together because every
change here is either (a) a top-level middleware definition that has
to be added next to LoggingMiddleware, or (b) a route handler at the
same file-level.

SecurityHeadersMiddleware (Content-Security-Policy, X-Frame-Options:
DENY, X-Content-Type-Options: nosniff, Referrer-Policy: no-referrer,
Permissions-Policy, server: unsloth-studio). The previous responses
emitted no CSP, no XFO, no Referrer-Policy and were stamped
server: uvicorn.

MaxBodyMiddleware rejects POST/PUT/PATCH on the inference / dataset /
data-recipe / train / export prefixes when Content-Length exceeds
UNSLOTH_STUDIO_MAX_BODY_MB (default 100). The audit hit this by
attaching a 50 MB plain-text file to a chat message and watching
Studio base64-encode it into the JSON body; uvicorn has no enforced
cap so the only previous guard was the per-file 50 MB ceiling that
data-recipe upload routes already enforce. The new middleware extends
that ceiling to the OpenAI-compat path that the Chat attachments
flow through. Verified: a 200 MB JSON POST to /v1/chat/completions
returns HTTP 413 "Request body too large (209,715,264 bytes; max
104,857,600)". A small valid request continues to reach the handler.

_inject_bootstrap is gated behind UNSLOTH_STUDIO_INJECT_BOOTSTRAP.
The previous default was to inline window.__UNSLOTH_BOOTSTRAP__ =
{username, password} into the first-boot HTML whenever
requires_password_change was true, which exposed the plaintext
bootstrap password to any browser extension, page script, or LAN
caller on -H 0.0.0.0. The bootstrap password remains in the on-disk
.bootstrap_password file (mode 0o600) where it has always lived;
users typing it into a current-password field is the right UX.

/api/health unauthenticated returns {"status":"healthy","timestamp":
...} only; the previous payload (version, device_type, chat_only,
desktop_protocol_version, supports_desktop_auth, studio_root_id,
native_path_leases_supported) is preserved for callers that present
a valid Bearer token, so internal launchers and sibling-Studio
detection (which compares studio_root_id) keep working.

/recipes -> /data-recipes 308 redirect. The Data Recipes page lives
at /data-recipes; users typing /recipes hit the SPA catch-all and
saw "Not Found". The redirect also preserves any tail path, so
/recipes/<rest> -> /data-recipes/<rest>.

Verified end to end with curl: CSP / XFO / X-Content-Type-Options /
Referrer-Policy / Permissions-Policy all present on /, server header
is now unsloth-studio (uvicorn's own banner is suppressed via
server_header=False in run.py from the auth-batch commit). Followed
the /recipes redirect lands on the SPA HTML.

* studio: bound TrainingStartRequest hyperparameters at the schema level

POST /api/train/start accepted any value for learning_rate, batch_size,
max_steps, max_seq_length, warmup_steps, warmup_ratio, num_epochs,
save_steps, weight_decay, gradient_accumulation_steps, lora_r,
lora_alpha and lora_dropout, including -1, 0, 1e9, and non-numeric
strings like 'abc' or 'two' (which silently coerce to 0 in the
trainer). Probing showed the API returning 200 to learning_rate=-1
and batch_size=0; only max_steps had any partial clamping.

This commit adds field_validator on every numeric hyperparameter.
Bounds are chosen wide enough to span realistic single-host
configurations (B200 with 180 GB of memory comfortably fits the
upper end) while rejecting the values that always produce broken
training:

- learning_rate: parses str/float, requires 0 < lr < 1.0. Non-numeric
  input raises with "learning_rate must be parseable as float (got
  'abc')" instead of silently coercing to 0.
- batch_size: [1, 1024].
- gradient_accumulation_steps: [1, 4096].
- num_epochs: [1, 1000].
- max_steps: [1, 1_000_000].
- max_seq_length: [1, 131072].
- warmup_steps: [0, max_steps].
- warmup_ratio: [0.0, 1.0].
- save_steps: [0, 1_000_000].
- weight_decay: [0, 10] (typical 0..0.1).
- lora_r: [1, 512].
- lora_alpha: [1, 1024].
- lora_dropout: [0.0, 1.0).

Each validator names the offending field in its ValueError message
so the 422 response body identifies which input is bad. The
learning_rate validator returns its result as str (the schema field
type is str("2e-4") for backwards compatibility) so existing call
sites that float() the value continue to work.

Verified:
- learning_rate=-1 -> 422 "learning_rate must be > 0 (got -1.0);
  typical range is 1e-6 .. 1e-3".
- learning_rate='abc' -> 422 "must be parseable as float".
- batch_size=-1 / 0 / 999999 -> 422 "batch_size must be in [1, 1024]".
- batch_size='two' -> 422 (pydantic int parser).
- max_steps=0 / -5 -> 422 "must be a positive int".
- max_seq_length=200000 -> 422 "must be in [1, 131072]".
- warmup_ratio=2.5 -> 422 "must be in [0.0, 1.0]".
- lora_dropout=1.5 -> 422 "must be in [0.0, 1.0)".
- Valid request with learning_rate='2e-4', batch_size=1, max_steps=5
  passes validation and the training run starts as normal.

* studio: redact image-decode errors, clean checkpoint dirs on cancel, tolerate Stop-button + tool-result message shapes

Three small fixes that fall under "do not let the audit findings
become user-visible papercuts".

routes/inference.py - image-decode error redaction (the audit hit
this with a 0-byte / malformed / wrong-extension image upload). The
three image-normalise sites previously raised HTTPException(400,
detail=f"Failed to process image: {e}"). When PIL raised
UnidentifiedImageError(io.BytesIO(raw)) the message string included
"<_io.BytesIO object at 0x7e40a5d7bf60>", leaking both the Python
class name (confirming the PIL/io stack) and a heap address (mildly
useful for ASLR-bypass chaining if another memory-corruption bug is
ever found). Each site now catches UnidentifiedImageError and
returns the generic "Unsupported or corrupt image format"; the
fall-through generic except returns "Failed to process image". No
exception-repr is interpolated into a response body anywhere along
these paths.

core/training/training.py - checkpoint cleanup on cancel. When a
user clicks Cancel Training, the trainer flips _cancel_requested=True
and the supervisor force-terminates the subprocess. The trainer
writes checkpoint-<step> directories under output_dir every
save_steps; previously these survived the cancel and accumulated on
disk (the audit recorded ~67 MB stuck after a 200-step cancel with
save_steps=20). New helper _cleanup_cancelled_checkpoints(output_dir)
globs checkpoint-<int> entries and removes them. It is gated by a
realpath containment check against outputs_root() so it cannot
accidentally rmtree anything outside the configured outputs root.
force_terminate() invokes the helper after the subprocess join when
_cancel_requested is true. Stop-and-Save runs are unaffected because
that path keeps _cancel_requested=False.

models/inference.py - chat message shape tolerance. Two related
frontend interactions used to crash the request validator:

- After the Stop button truncates a generation, the frontend
  retained {role:"assistant", content:""} in the conversation
  history and replayed it on the next send. ChatMessage previously
  required role="assistant" to have non-empty content or tool_calls,
  so the next message returned 422 and the thread was permanently
  broken. The validator now normalises empty assistant content to
  None so the request round-trips and the trailing empty turn can
  be ignored downstream.

- The frontend's second-round tool POST drops the streamed
  tool_call_id, hitting the strict-spec check "role=tool requires
  tool_call_id". The validator now synthesises an opaque id
  (call_<8 hex>) when missing, so the request reaches the handler
  and the model's final summarising response gets generated. The
  proper fix lives in the frontend (carry the streamed id through
  the second POST) and will follow.

Verified end to end with curl: HTTP 400 (model not loaded) on both
the empty-assistant history shape and the tool-result-without-id
shape, instead of HTTP 422 from the schema validator.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* studio: tighten code comments from security-hardening pass

Trim verbose docstrings and inline finding references added in the
previous commits in this branch. Functionality unchanged.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* studio: await get_current_subject in /api/health and make refresh-token consumption atomic

The /api/health auth probe called get_current_subject(creds) without
awaiting it. The coroutine object is truthy, so any caller presenting a
Bearer header (valid or not) received the full diagnostic payload
including version, device_type, studio_root_id, etc. Await the coroutine
and treat HTTPException as 'fall back to the minimal liveness payload'.

consume_refresh_token did SELECT then DELETE WHERE id under default
autocommit isolation. Two concurrent POST /api/auth/refresh requests
could both win the SELECT before either DELETE ran, defeating
single-use refresh-token rotation. Replace with a single
DELETE ... WHERE token_hash = ? AND expires_at >= ? RETURNING ...
statement so the validate-and-delete lands as one atomic op under
SQLite's write lock (3.45.1 supports RETURNING; min was 3.35).

* studio: enforce body cap on chunked uploads and drop unsafe-inline from script-src

MaxBodyMiddleware previously only inspected the declared Content-Length
header; clients omitting it or sending Transfer-Encoding: chunked
bypassed the cap and could still drive an OOM via the downstream
JSON / file readers on /v1/chat/completions, /api/inference, /api/data-recipe,
/api/datasets, /api/train, /api/export. Rewrite as a raw ASGI middleware
that drains and counts http.request frames, replies 413 once the running
total exceeds UNSLOTH_STUDIO_MAX_BODY_MB before invoking the FastAPI
handler, and replays the buffered body to downstream so route code that
calls request.json() / await request.body() works unchanged.

CSP previously included 'unsafe-inline' on script-src, which defeats the
main XSS protection. The frontend bundle does not need inline scripts;
the only inline <script> the backend ever emits is _inject_bootstrap,
which is opt-in via UNSLOTH_STUDIO_INJECT_BOOTSTRAP. Drop 'unsafe-inline'
from script-src by default; when _inject_bootstrap fires, generate a
per-response nonce, embed it on the inlined <script>, and have
SecurityHeadersMiddleware splice 'nonce-XXX' into the CSP for that one
response (the internal x-internal-script-nonce header is popped before
the response leaves the server). 'unsafe-inline' stays on style-src for
Vite-injected styles.

* studio: drop empty assistant sentinel before passthrough

ChatMessage._validate_role_shape normalises role="assistant", content=""
(the post-Stop sentinel emitted by the frontend) to content=None so the
in-process path can drop it via _extract_content_parts. The passthrough
path then ran m.model_dump(exclude_none=True), which strips the now-None
content key entirely, sending {"role":"assistant"} to llama-server / the
OpenAI-compat backend. That fails upstream and leaves the user without a
recoverable Stop->resume.

Add _drop_empty_assistant_sentinels and call it at both passthrough
message origins: _openai_messages_for_passthrough (covers
/v1/chat/completions and the Responses API which routes through it) and
the anthropic_messages_to_openai output before
_anthropic_passthrough_*. Assistant messages that carry only tool_calls
(no content) are preserved.

* studio/tests: cover audit-fix surfaces and rebase pre-existing tests

Adds and updates pytest coverage for the four bot-flagged audit fixes
landed earlier in this branch and rebases two pre-existing tests that
were broken by the relaxed-validator and /api/health auth-gate changes.

studio/backend/tests/test_middleware.py (new)
  MaxBodyMiddleware: small protected, large declared, unprotected
  passthrough, chunked-upload-over-cap rejection (the regression for
  the original Content-Length-only gap), and chunked-under-cap replay.
  SecurityHeadersMiddleware: script-src no longer carries
  'unsafe-inline', style-src still does, default headers
  (XFO/XCTO/Referrer-Policy/Permissions-Policy/server), and the
  internal x-internal-script-nonce header is consumed by the
  middleware and converted to 'nonce-XXX' in the CSP.
  /api/health: no auth -> minimal, invalid Bearer -> minimal
  (the await regression), valid Bearer -> full diagnostic payload.

studio/backend/tests/test_desktop_auth.py
  consume_refresh_token: second-call returns None, expired returns
  None, and a 64-thread concurrent pile-up against the same hash
  produces exactly one successful consumer (regression for the
  SELECT-then-DELETE race).
  test_health_response_reports_desktop_capability_fields: rebase
  against the new health_check(request) signature by going through
  TestClient with a real bearer instead of asyncio.run-ing the
  handler directly.

studio/backend/tests/test_openai_tool_passthrough.py
  Pin the new ChatMessage tolerance: assistant without content or
  tool_calls is tolerated (normalises content -> None), empty-string
  and empty-list assistant content normalise to None, and a missing
  / empty tool_call_id on role='tool' is synthesised as call_<hex>
  rather than raising. Tests for _drop_empty_assistant_sentinels
  cover the three drop shapes (empty string, empty list, missing
  content key), preservation of assistant text and tool_calls-only
  messages, and end-to-end through
  _openai_messages_for_passthrough.

studio/backend/main.py
  SecurityHeadersMiddleware.dispatch used response.headers.pop(...)
  for the nonce-header handoff; Starlette's MutableHeaders has no
  pop. Read-then-del so the internal handoff header is still
  stripped before the response leaves the server.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* studio/tests: rebase three more pre-existing CI tests against this branch

CI on PR #5375 was red on three tests that were tuned for behaviour
predating this branch. Updates each so the assertions match what the
audit fixes intentionally changed; no production code touched.

studio/backend/tests/test_trained_model_scan.py
  test_scan_trained_models_includes_lora_and_full_finetune_outputs
  passed an absolute tmp_path through scan_trained_models, which now
  runs resolve_output_dir / _assert_contained against outputs_root().
  Repoint outputs_root() at tmp_path via monkeypatch so the fixture
  dirs land under the configured root and the realpath containment
  check passes.

tests/test_studio_install_workspace_guard.py
  test_health_endpoint_exposes_studio_root_id_not_raw_path read
  the first 1500 bytes after @app.get("/api/health") and asserted on
  the studio_root_id literal. The handler grew (unauth short-circuit
  + await dependency gate) and the literal slid past the byte window.
  Replace the fixed window with a slice up to the next top-level
  @app.* decorator so the test surveys the whole handler regardless
  of size.

tests/studio/studio_api_smoke.py
  The "login burst (5x wrong pw) -> 401 each" assertion was tagged
  "When/if we add one, this assertion updates in the same PR." We
  added the per-IP rate-limit in routes/auth.py
  (_LOGIN_MAX_FAILS=5/60s) but missed the assertion update. Rewrite
  the burst probe to observe the new invariant: at least one 401,
  eventual transition to 429, and Retry-After present on the 429.
  Adds a small _login_with_headers helper since the existing login()
  helper drops response headers.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* ci(studio-ui): set UNSLOTH_STUDIO_INJECT_BOOTSTRAP=1 for Playwright Studios

The Chat UI Playwright test drives the first-boot change-password
form, which (per playwright_chat_ui.py step "1. Change-password
through the UI") pre-seeds the hidden current_password field from
window.__UNSLOTH_BOOTSTRAP__. That global is only emitted when the
backend's _inject_bootstrap path fires, which since the security
pass on this branch is gated behind UNSLOTH_STUDIO_INJECT_BOOTSTRAP
and defaults to off. Without the global, the React form's
current_password validator never satisfies, the submit button stays
disabled, and the composer.wait_for() probe times out on
/change-password.

Re-enable injection only for the CI Studios that drive the chat UI
across linux/mac/windows. Production deployments are unaffected: the
env var has to be explicitly opted into, and the on-disk
auth/.bootstrap_password remains the source of truth for human users
typing the password in by hand.

Covers all eight Studio launch sites: the primary chat-ui boot and
the "extra UI tests" boot for each of the three OSes, plus the
pipeTransport JSON-crash retry relaunches in the macOS workflow that
re-spawn Studio mid-job.

A follow-up frontend PR will add a visible current_password input so
the form satisfies its own validator without needing the bootstrap
auto-fill at all; once that lands this CI knob can come back out.

* studio/sandbox: drop unshare(CLONE_NEWNET); add trusted-host allowlist; block sandbox file uploads; raise CPU rlimit default to 600 s

CLONE_NEWNET inside _sandbox_preexec silently killed every outbound
HTTP request from sandboxed Python whenever the kernel allowed
unprivileged user namespaces. requests.get('https://huggingface.co'),
urllib.request.urlopen('https://en.wikipedia.org/wiki/...'),
socket.connect(('arxiv.org', 443)) all failed despite the AST visitor
intending to allow them. The bash blocklist (curl / wget / nc / ssh /
scp / sftp / rsync / socat / eval / source) plus the AST-level
metadata-host denylist still carry the network policy after this
change; CLONE_NEWNET was redundant with both.

Add _TRUSTED_PUBLIC_HOST_LITERALS + _TRUSTED_PUBLIC_HOST_SUFFIXES
(~100 informational hosts: Wikipedia language subdomains, Wikimedia,
Wikidata, Google search, Bing, DuckDuckGo, HuggingFace, GitHub,
raw.githubusercontent.com, arXiv, StackOverflow / Stack Exchange,
MDN, docs.python.org, PyTorch / TensorFlow / NumPy / pandas docs,
pypi / files.pythonhosted.org / npmjs / crates.io, ReadTheDocs,
arXiv, Britannica, BBC / Reuters / Nature / Science, NASA / CDC /
NIH / WHO open data, api.weather.gov). The visitor now blocks
literal hosts that are neither metadata nor trusted with a short
LLM-readable string so the model can retry with an allowed source
instead of choking on a multi-line error.

Block upload-shape calls regardless of host: requests.post / put /
patch / delete / request with files= or data=open(...) /
data=bytes_literal; httpx equivalents; urllib.request.urlopen /
Request with data=...; HuggingFace upload_file / upload_folder /
upload_large_folder / create_commit (module-level FQ paths AND
method-name match on any receiver). Message: "Blocked: file upload
disallowed in sandbox".

Bump UNSLOTH_STUDIO_SANDBOX_CPU_S default 300 -> 600 s so long
agentic chains that span multiple tool calls don't get SIGXCPU'd
mid-stride. Env-var override path is unchanged.

Host normalisation now strips trailing dot, userinfo @, and explicit
port before allowlist / denylist comparison so trailing-DNS-dot,
userinfo-smuggling, and explicit-:443 URLs are decided correctly.

* studio: raise default request-body cap from 100 MB to 500 MB

UNSLOTH_STUDIO_MAX_BODY_MB default goes 100 -> 500 to comfortably
cover vision + audio + multi-recipe-batch JSON payloads. The
MaxBodyMiddleware stream-counting logic from this branch's earlier
06ec088 already handles chunked bodies up to the new cap; env-var
override path is unchanged for callers that want a tighter limit.

* studio/auth: restore /api/auth/status.default_username to 'unsloth'

This branch's earlier b39e9a4 changed default_username to None on the
public /api/auth/status endpoint so the username field didn't leak to
unauthenticated callers. In practice this regressed third-party
clients (and the in-tree React login form's pre-fill UX) without
adding meaningful security: the bootstrap password is the actual
secret, and the username 'unsloth' is the documented default.

Pin default_username to storage.DEFAULT_ADMIN_USERNAME ('unsloth')
and tighten the response model so the field is required rather than
Optional. Anyone who needs anonymisation can still reach for an
allow-list deployment with auth disabled.

* studio/training: raise max_seq_length / batch_size / lora_r / lora_alpha caps

This branch's 7102815 introduced field validators with conservative
caps. The follow-up loosens them so long-context experiments and
high-rank LoRA exploration aren't gated at the schema layer:

  _MAX_BATCH_SIZE   1024     -> 4096
  _MAX_SEQ_LENGTH   131_072  -> 2_000_000   (2M tokens)
  lora_r cap        512      -> 16_384      (_MAX_LORA_R)
  lora_alpha cap    1024     -> 32_768      (_MAX_LORA_ALPHA)

_MAX_GRAD_ACCUM / _MAX_STEPS / _MAX_EPOCHS / lora_dropout /
warmup_ratio / weight_decay are unchanged. Hardware (VRAM, host
RAM, kernel launch latency) is now the binding constraint at the
new caps, which is the correct ordering -- the validator stays a
sanity check on -1 / 0 / 'abc' style garbage, not a usability gate.

* studio/tests: cover sandbox allowlist + upload block + raised training caps

studio/backend/tests/test_sandbox_tools.py (new):
  TestMetadataHostDenylist     -- short "Blocked: cloud-metadata host"
                                  message on AWS IMDS, GCP metadata,
                                  Alibaba ECS, AWS IPv6 IMDS, 169.254/16.
  TestTrustedHostAllowlist     -- Wikipedia (any language subdomain),
                                  Google, DuckDuckGo, HF, raw GitHub,
                                  arXiv, StackOverflow / family,
                                  MDN, docs.python.org, pypi, BBC,
                                  api.weather.gov, NumPy / PyTorch docs.
  TestUntrustedHostBlock       -- example.com / random unlisted host
                                  rejected with the short "Blocked: host
                                  not in sandbox allowlist; use an
                                  allowed informational source" message.
                                  Dynamic URLs (computed var) still pass
                                  -- documented limit of static analysis.
  TestHostNormalization        -- trailing dot, explicit :443, uppercase,
                                  userinfo-@-smuggle all decided
                                  correctly without false-block /
                                  false-pass.
  TestUploadDenylist           -- requests / httpx / urllib.urlopen with
                                  files= / data=open / data=bytes,
                                  HfApi().upload_file / upload_folder /
                                  create_commit, module-level
                                  huggingface_hub.upload_folder. POST
                                  json= to trusted host still passes.
  TestSandboxCpuRlimitDefault  -- pin UNSLOTH_STUDIO_SANDBOX_CPU_S=600
                                  default and confirm CLONE_NEWNET
                                  source line is gone.
  TestMaxBodyDefault           -- pin UNSLOTH_STUDIO_MAX_BODY_MB=500
                                  default.

studio/backend/tests/test_studio_train_validation.py (new):
  Pin at-cap-accepts / over-cap-rejects boundaries for
  max_seq_length=2_000_000, batch_size=4_096, lora_r=16_384,
  lora_alpha=32_768 so a future regression that tightens them back
  without explicit user opt-in is caught.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* studio: tighten code comments across the security-hardening pass

* studio: always inject bootstrap credentials on first boot

The UNSLOTH_STUDIO_INJECT_BOOTSTRAP gate added an extra
terminal-to-browser copy-paste on every fresh install. In practice
the LAN credential leak it guarded against is narrow: the password
is one-time, the user rotates it on the very next click, the
default Studio bind is 127.0.0.1, and -H 0.0.0.0 already exposes
the entire API surface. Drop the gate so the inject fires whenever
a bootstrap password is still pending. The CSP nonce wiring stays
in place; the inline script remains the only inline script the
backend ever emits.

The three Playwright UI smoke workflows lose their
UNSLOTH_STUDIO_INJECT_BOOTSTRAP=1 lines along with the explanatory
comment blocks since the inject now happens by default.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Wasim Yousef Said <wasimysdev@gmail.com>
2026-05-13 06:12:18 -07:00