* tests: read checked-in files as UTF-8 instead of the platform default
Path.read_text() with no encoding uses locale.getpreferredencoding(), which
is UTF-8 on the Linux runners and cp1252 on a stock Windows install. Nine
module-level reads of checked-in source files were relying on that default.
studio/backend/routes/inference.py carries the DeepSeek tool-call token
regexes, so it holds U+FF5C and U+2581. Under cp1252 that read raised
UnicodeDecodeError on byte 0x81 at position 97806, and because the reads run
at import time it took test_cancel_atomicity.py and test_cancel_id_wiring.py
out at collection, not as failures. Green on CI, permanently broken for a
Windows contributor running the suite locally.
Adds a guard: at module scope there is no tmp_path fixture, so a bare
read_text()/write_text()/open() there is always touching a checked-in file.
That makes the rule mechanical enough to enforce with no allowlist, while
staying quiet about temp-dir I/O inside test bodies where the platform
default is harmless.
The repo already spells this correctly in 464 other places; this only stops
the stragglers coming back.
* tests: cover import-time helper reads and keep the guard py3.9-safe
Follows up on the Codex review:
- add `from __future__ import annotations`, since `str | None` in
`_offender` is evaluated at import on Python 3.9 and pyproject declares
requires-python ">=3.9,<3.15".
- widen the guard from module scope to import time. Class bodies and the
bodies of module-level helpers called from an executing statement run
during collection too, so `CODE = _extract_mixed_precision_code()` was
the same hazard as an inline read. `if __name__ == "__main__":` blocks
are skipped: pytest never executes them.
- scan studio/backend/tests/ as well as tests/. Both trees are collected
on Windows by separate CI jobs, and the offender that started this,
test_tool_xml_strip.py reading routes/inference.py, lives there.
Widening it surfaced seven more import-time reads of checked-in sources;
all now name utf-8.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Harden the import-time encoding guard for PR #7438
Close the detector gaps raised in review, all of which I reproduced against
the actual AST before changing anything.
False negatives (the guard let a real hazard through):
- _is_main_guard ignored the comparison operator, so if __name__ != "__main__"
counted as script-only even though its body runs at import.
- The else arm of a main guard was discarded with the rest of the If node.
- Decorators and argument defaults on a module-level def were skipped with the
body, though both are evaluated when the def executes.
- Path.open() in text mode was invisible; only builtin open() was matched.
- encoding = None and encoding = "locale" both re-select the platform default,
but the keyword merely being present counted as pinned.
False positives (the guard would have blocked a compliant contributor):
- A non-literal mode fell through to the "r" default, so open(p, mode) was
flagged even when mode is "rb", where adding encoding= is a ValueError and
there is no edit that satisfies the rule.
- Same for open(*args) and a **kwargs splat, which hide the mode and can hide
an encoding.
- Lambda bodies and comprehension elements were walked even though neither runs
at definition.
Verified: still reports the same 22 offenders on unpatched main, green on this
branch and on the tree merged with latest main (557 files), and an adversarial
corpus of 33 cases now scores zero false positives and zero false negatives.
Also corrected two docstring claims: neither collecting job runs on Windows,
and the read is governed by locale.getencoding().
* Walk eager comprehensions and treat io.open as the builtin
Two regressions from the previous commit, both reproduced against the AST
before changing anything.
Lumping list, set and dict comprehensions in with generator expressions was
wrong. Only a genexp is lazy; the other three run their element expression,
their filters and their nested iterators immediately, so
CONTENTS = [p.read_text() for p in PATHS] at module scope is an import-time
read the guard was silently missing. Comprehensions are now walked in full and
only the genexp keeps the outermost-iterable-only treatment.
io was also in the not-a-path-opener list, but io.open is the builtin, with the
same mode position and the same platform default. io.open(CHECKED_IN_FILE) is
exactly the hazard this guard exists for, so it is matched now, with binary
modes and a pinned encoding still exempt. tarfile.open and fitz.open stay
exempt since neither has an encoding to name.
Verified: 13 targeted cases covering all five eager comprehension forms and
io.open in text, binary and pinned shapes all classify correctly; still 22
offenders on unpatched main; green on this branch and on the tree merged with
latest main.
* Close three more walker gaps in the import-time guard
All three reproduced against the AST first.
A generator expression handed straight to a call is consumed there, so
DATA = "".join(p.read_text() for p in paths) runs its element at import. Only
an unconsumed genexp bound to a name stays lazy, so the walker now follows the
consumed ones in full and keeps the outermost-iterable-only treatment for the
rest.
if "__main__" == __name__ is an equivalent and accepted spelling of the main
guard, but requiring __name__ on the left meant its body was treated as
import-time code. That is a false positive on a block pytest never runs, so
both operand orders are recognised now.
The helper table was built from module-level defs only, so a def in a class
body invoked while the class is constructed was never followed, contradicting
the walker's stated coverage of class bodies. Helpers are now collected from
the module body and from class bodies at any nesting.
Verified: 15 targeted cases including all three fixes and the earlier ones
still classify correctly; still 22 offenders on unpatched main; green on this
branch and on the tree merged with latest main.
* Handle positional read_text encodings, lazy generators and nested helpers
* Guard reads reached from test bodies, unbound Path calls and __file__ paths
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Follow derived paths, skip lazy generator helpers, cover compressed openers
* Guard the CLI tests, helper parameters and unbound Path arguments
* Discover test roots and follow literal, in-place and tuple-derived paths
* Identify module openers by import, unwrap starred paths, pin subprocess snippets
* Resolve import origins, seed helper locals, follow named generators and parametrize
* Scope imports lexically, list tracked test files, bind unpacked names
* Resolve aliased openers, keyword-only params, destructured targets, next()
* Pin the encoding on subprocess snippets, workflow lint and CLI output for PR #7438
* Harden the CLI encoding guard against detached streams for PR #7438
* Tighten the encoding guard's path and scope analysis for PR #7438
* Resolve path provenance more precisely and keep POSIX stream encodings for PR #7438
* Resolve qualified path classes and scope conditional imports for PR #7438
* Scope CLI stream setup to the entry point and align two encoding pairs for PR #7438
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <danielhanchen@gmail.com>
* Default tool-call permission to Approve for me, prompting only on high-risk actions
Make "auto" ("Approve for me") the product default permission mode for local
tool calls, and narrow what it prompts on so ordinary development commands run
without interruption.
Before, an omitted permission_mode behaved as "ask" (or ran ungated on a
non-streaming request), and "auto" paused on any call that was not read-only
(pip install, mkdir, cp, python train.py, git commit, any redirect). Now:
- Unset permission_mode normalizes to "auto" at the API boundary and in both
tool loops; the Field defaults are "auto" too. An unrecognized value still
falls back to the stricter "ask".
- "auto" pauses only on genuinely high-risk calls via a new
is_high_risk_tool_call classifier: credential/secret path access, privilege
escalation (sudo/su/doas/pkexec), destructive or persistence commands
(rm/dd/mkfs/crontab/systemctl/recursive chmod, ...), and network exec/exfil
(curl piped to a shell, ssh/scp/nc, curl uploads). Everything else runs.
Python prompts on shell escapes, network egress, sensitive reads, and
dynamically built code; ordinary in-workdir writes run.
- Frontend sends permission_mode for every local chat and omits
confirm_tool_calls for "auto" so the safe-only no-stream exception still
applies; the picker and store describe the new behavior.
The hard-block command set, code-safety static analysis, resource limits,
secret-env stripping, and the per-session sandbox workdir remain in force under
every mode, and "ask" is still available for users who want to confirm every
call.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Keep non-streaming tool requests working under the auto default
The default-permission change made an omitted permission_mode normalize to
auto at the request boundary, so a non-streaming enable_tools request hit the
confirm-without-stream guard and returned 400 instead of running (regression
against the #6570 non-streaming tool-call contract used by non-interactive
clients and health checks).
Keep permission_mode unset at the request boundary (the confirm gate can only
prompt while streaming, so an unset non-streaming request stays lenient and
runs), while the tool loops continue to normalize an unset mode to auto for the
per-call gate. Net: streaming requests default to auto and pause high-risk
calls; non-streaming requests keep the prior run-without-gate behavior.
* Harden the auto high-risk classifier against review-flagged bypasses
Address Codex/Gemini review of the default-permission change by gating the
destructive/exec cases that were reaching auto mode without a prompt:
- Terminal: a non-shell interpreter running inline code (python -c, node -e,
perl -E, php -r), destructive git subcommands (git clean, git reset --hard,
git push --force), and a command synthesized by a command-position
substitution ($(printf rm) -rf build) now prompt. Ordinary python <script>,
git commit/push, and argument-position substitutions (echo $(date)) run.
- Python tool: exec/eval/compile/__import__ invoked by keyword (compile(source=
...), import_module(name=...)) is now caught alongside the positional form.
- MCP: an execution tool (run_command, execute_script, invoke_shell) is gated
like a terminal call, since it runs arbitrary commands on the MCP server
outside the terminal sandbox; ordinary create/list/read tools still run.
The curl/wget exfil and shell eval cases the review raised are already refused
by the sandbox hard-block set, so no gate change was needed there; the PR
description now notes the classifier layers on top of that hard-block.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Recurse shell -c payloads and literal exec source in the high-risk gate
Second review round on the auto high-risk classifier:
- A high-risk command wrapped in a shell -c payload (bash -c 'git clean -fd',
sh -c 'truncate -s 0 x') is now screened by recursing into the payload,
bounded by depth. The sandbox hard-block only recurses for its own smaller
command set, so git/truncate wrapped this way previously ran unprompted.
- A literal exec/eval/compile source is screened for what it runs rather than
assumed harmless: exec('import urllib...urlopen(...)') now prompts, while
exec('x = 1') and a literal __import__('os') name still run.
- git global options that take a value (git -C repo clean, git -c k=v clean)
consume their value before the subcommand is read, so the real subcommand
is judged.
- The network exfil check also runs over the assignment-expanded command, so a
curl/wget name assembled from variables (c=cu d=rl; $c$d -F ...) is seen.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Cover attached inline flags, env -S/-C, camelCase MCP, folded python paths
Third review round on the auto high-risk classifier:
- Interpreter inline code in the attached short form (python -c'...',
node -e'...') is now matched by the -c/-e/-E/-r prefix, not only the exact
flag token.
- env -S / --split-string runs its string as a command (screened recursively)
and env -C / --chdir changes the working directory (asks), so a destructive
command behind env is no longer treated as a plain wrapper.
- camelCase MCP tool names are split on the case boundary (runCommand ->
run_Command) before the execution / sensitive-noun regexes, so camelCase
execution tools are gated like snake_case ones.
- A sensitive path folded across string-literal variables, os.path.join,
sep.join([...]), or an f-string (p='/etc'; open(p+'/shadow')) is now folded
and re-checked; an unresolved fragment folds to a sentinel so a partial fold
never false-positives.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Gate substitution-built shell payloads and keep explicit confirm opt-in
Two auto-mode gaps from review:
- A command substitution stashed in a variable and then executed dynamically
(x=`printf 'git clean -fd'`; bash -c "$x", or ...; $x, or eval "$x") never
appears as literal command text, so the token scan could not see the real
command and git clean ran without a prompt. Fail closed when a command
substitution coincides with a variable executed as a command. Ordinary
substitutions captured into a value/argument (d=$(date); mkdir build_$d) still
run.
- An explicit confirm_tool_calls=True with no permission_mode is the
pre-permission-mode opt-in to confirm every call. It now resolves to "ask" at
the request layer instead of the "auto" product default, so those callers keep
per-call gating rather than only prompting on high-risk calls. A bare unset
request (confirm flag not set) still defaults to auto.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Cover CLI-forced confirm, Windows delete built-ins, and pathlib reads
Three more auto-mode gaps from review:
- An explicit confirm_tool_calls=True with no permission_mode is now resolved to
"ask" regardless of the request-level tool flags, so a process-wide
--enable-tools policy that forces the loop when the request sets neither
enable_tools nor mcp_enabled still gates every call. Setting only the mode is
inert unless the loop runs, so a passthrough request is unaffected;
external-provider requests are still left untouched.
- The Windows cmd.exe delete built-ins del, erase, and rd are added to the
high-risk terminal set. The terminal executor runs cmd /c on Windows and these
are not in the hard-block set, so del /q file.csv would otherwise run in the
workdir without a prompt.
- A sensitive path assembled with pathlib (Path('/etc') / 'passwd', joinpath, or
a Path bound to a variable then joined) is now gated. The python high-risk
folder reuses the shared _folded_path builder plus _folded_is_sensitive, which
already handle the / operator, path constructors, os.path.join, str.join,
f-strings, and %/.format. Relative in-workdir and unknown-base paths still run.
* Gate combined -c, versioned interpreters, busybox, and sensitive chdir
Four more auto-mode classifier gaps from review, plus a sandbox backstop:
- Combined shell flag clusters (bash -lc, bash -xc) and the attached form
(bash -c'...') now have their -c payload screened recursively; the same
cluster handling closes python -Bc inline code. Previously only an exact -c
matched, so bash -lc 'git clean -fd' ran without a prompt.
- Versioned interpreter binaries (python3.11, python2.7, pypy3.10) are recognized
as inline-code interpreters, so python3.11 -c '...' is gated like python3 -c.
- busybox / toybox are treated as command wrappers, so the applet
(busybox rm -rf) is judged instead of the multicall binary, which was slipping
through as an unknown-but-safe command.
- A chdir into a sensitive directory (cd /proc/$PPID; cat environ, cd /etc) is
gated: the read happens after the directory change so no single token spells
out the sensitive path. Ordinary in-workdir chdirs still run.
- Backstop for the /proc/<parent>/environ read: the sandbox now hardens the
Unsloth process against same-UID /proc environ reads in normal sandboxed mode
too, not only in bypass mode, so a classifier miss cannot recover the parent
environment. Best-effort in the sandbox (the child env is already scrubbed), so
a host where prctl is unavailable still runs.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Harden parent proc-env on the sandboxed python path too
The previous commit hardened the Unsloth process against same-UID
/proc/<parent>/environ reads on the sandboxed bash path; apply the same
best-effort hardening on the sandboxed python exec path so both tools are
symmetric. Update test_bypass_exec_hardens_parent_proc_env, which asserted the
sandboxed path never hardened, to expect the backstop on both paths.
* Tighten the curl/wget exfil check for attached and wget upload flags
The network exec/exfil classifier missed a curl upload flag when it was attached
to its value (curl -Ffile=@dump.sql, curl -d@f) because the token was split on =
first, and it did not cover wget's upload flags (--post-data, --post-file,
--body-data, --body-file). curl short upload flags are now matched prefix-wise and
wget's upload flags are checked separately, which also removes a false positive
where a benign wget short option (wget -T timeout, wget -F force-html) was read as
an upload. curl and wget remain hard-blocked by the sandbox regardless; this only
tightens when auto mode pauses for approval.
* Tighten the high-risk auto-mode classifier: wrapper, interpreter, git, python-fs, MCP, and persistence-write gaps
Close reachable gaps where a genuinely dangerous tool call was auto-approved
without a prompt in Approve-for-me mode:
- Process-launch wrappers: setsid/exec/builtin forward the command position, so
screen their child (setsid git clean, exec python -c) instead of the wrapper.
- Inline-code interpreters: node/bun -p/--print evaluate code like -e; pwsh
-Command/-EncodedCommand run inline code (not hard-blocked off Windows).
- Windows cmd.exe /c|/k recurses into the nested command (cmd /c del x).
- git restore (default --worktree) and git checkout -- . / git checkout .
discard tracked edits irrecoverably, same class as the already-gated git clean.
- Python destructive filesystem calls (os.remove, shutil.rmtree, Path.unlink,
os.rmdir/removedirs, incl. bare imports) pair with the terminal rm gate.
- MCP: a read-named tool carrying a destructive payload (DELETE/DROP SQL,
GraphQL mutation, mutating HTTP method) still prompts; honestly-named
create/update/delete MCP calls keep running.
- System persistence writes: a write into /etc/profile.d, /etc/cron*,
/etc/systemd, /etc/ld.so.preload, /etc/rc.local, /etc/init.d installs a
boot/login/preload hook. The sandbox keeps host-fs access, so gate these;
ordinary /etc reads (hostname, resolv.conf) and in-workdir writes still run.
Adds table-driven regression rows for every new prompt case and its
guard-against-over-prompt counterpart.
* Extend the high-risk auto-mode gate: non-curl network clients, destructive MCP verbs, array-fed shell payloads
Round-two Codex hardening on the auto (Approve-for-me) classifier:
- Network exfil beyond curl/wget: gate nc/ncat/netcat/telnet/socat/ssh/scp/sftp
at command position and openssl s_client/s_server. The sandbox has no network
namespace, so tar czf - . | openssl s_client -connect host:443 was streaming
the workdir without a prompt. Local openssl (dgst/enc) and a filename that
merely contains a client name still run.
- Destructive MCP tools: an honestly-named delete_file/delete_repo/drop_table/
purge_index/revoke_token runs outside the terminal sandbox and loses data, so
gate the destructive verb on the name. Non-destructive create/update/list/get
still run; a substring like undelete does not match on the segment boundary.
- Dynamically constructed shell payloads: x=(git clean -fd); bash -c "${x[*]}"
carries no command substitution and is not resolved by assignment expansion,
so it slipped the var-executed check. Fail closed when an array expansion is
run as a command; a benign array print (echo "${a[@]}") is untouched.
Adds regression rows for every new prompt case and its benign counterpart.
* Gate user-level persistence writes in auto mode
Extend the persistence-write gate from the /etc set to user-level startup and
autostart locations: a write into ~/.bashrc, ~/.zshrc, ~/.profile and the other
shell rc/profile files, ~/.config/autostart, ~/.config/systemd/user, or
~/.config/environment.d runs on the next login/session, the same boot-hook risk
but needing no root (Studio commonly runs unprivileged, so this is the more
reachable vector). The sandbox does not confine absolute paths, so an append to
~/.bashrc reaches the real file. A non-persistence ~/.config dir and ordinary
reads still run. Adds regression rows.
* Close three more auto-mode gate gaps: curl destructive methods, the dot source synonym, aliased os.remove
- curl -X DELETE / --request DELETE|PUT|PATCH (separated, attached, and
--request= forms) mutates or deletes a remote resource, so gate it; a plain
download and GET still run.
- The hard-block set blocked source but not its POSIX synonym '.', so
. ./script.sh ran the file's contents past the classifier. Block '.' at
command position too; a path argument (find . -type f, cd .) is unaffected.
- os.remove reached through an aliased module (import os as fs; fs.remove(...))
was missed because only the literal receiver 'os' was recognized; resolve
import os as ... aliases, matching the existing safety analyzer.
Adds regression rows for each case and its benign counterpart.
* Close three more obfuscation bypasses of the auto-mode gate and hard block
- ANSI-C quoting hid the command name: a $'rm' -rf x form tokenized as $rm, so
both the high-risk scan and _find_blocked_commands missed it while Bash ran
rm. Decode ANSI-C ($'...') before classifying, in both the terminal
classifier and the blocklist; an ANSI-C string in argument position stays
benign.
- Process substitution executed as a script (an interpreter consuming a <(...)
whose generated content is unscreenable) ran without a prompt; the prior <(
check was unreachable without curl/wget. Gate a process substitution consumed
by an interpreter; a non-interpreter consumer (diff over two <(sort ...))
still runs.
- os.remove bound to a name (f = os.remove; f(x)) or reached via getattr(os,
'remove') bypassed the direct-attribute scan. Track assignment aliases and
getattr with a literal attribute name; a bound list.remove still runs.
Adds regression rows for each case and its benign counterpart.
* Gate container runtimes, MCP privilege grants, arg-embedded exec, and network listeners
- Container/VM runtimes (docker, podman, nerdctl, ctr, crictl, lxc, machinectl,
kubectl) act through a daemon with host privileges, so a bind mount writes the
real filesystem and escapes the child process workdir and rlimits entirely.
Gated wholesale because the escape lives in the arguments.
- MCP privilege grants: an unambiguous privilege verb (grant/authorize/elevate/
escalate/impersonate) prompts on its own; a softer verb (assign/add/set/
attach/bind/put/update/create) prompts only next to a privilege noun (role,
permission, policy, acl, scope, membership), so assign_issue and add_label
keep running while grant_role and add_permission ask.
- A flag whose value is a command the tool then executes (GNU tar
--checkpoint-action=exec=CMD, --rsh, --rsync-path) hid a payload inside an
argument, past both the classifier and the blocklist. Ordinary archiving runs.
- An interpreter serving on the network (python -m http.server, uvicorn,
gunicorn, waitress) exposes the session workdir since the sandbox keeps no
network namespace. A non-server module (python -m pytest, -m pip) still runs.
Adds regression rows for each case and its benign counterpart.
* Close the parallel-review gaps: over-prompting regressions and asymmetric high-risk omissions
Over-prompting fixes (auto mode was pausing on ordinary work):
- The network-listener check matched a server name ANYWHERE in the command, so
`pip install uvicorn`, `grep uvicorn reqs.txt` and even `echo uvicorn`
prompted. Scope it to the two forms that actually listen: a module after
`-m`, or a server binary at command position.
- Inline-code flags were one shared set, so `python -E` (ignore env) and
`python -Werror` read as eval. Resolve them per interpreter: python -c,
node/deno/bun -e/--eval, ruby -e, perl -e/-E, php -r.
- The curl upload scan read option letters from unrelated commands in the same
line (`ls -T && echo curl`). Scope the scan to the segment whose command is
actually curl/wget.
Under-prompting fixes (destructive actions the narrowed gate stopped catching,
each the twin of something already gated):
- git: switch -f/--force/--discard-changes, stash clear/drop, branch -D/-M,
rm, push --delete/--mirror/--prune and the +src / :dst refspec forms.
- Platform twins: unlink, ftp, tftp, format, diskpart, diskutil, schtasks,
reg, sc, launchctl.
- Python: posix/nt module twins (including bare imports), os.truncate,
os.ftruncate, os.kill, os.killpg, and a file handle's truncate. Gated via the
handle name so pandas DataFrame.truncate() keeps running.
- MCP: clear/reset/empty/flush/prune/expire destructive verbs, promote.
- deno/bun expose inline eval as a subcommand, not a flag.
- A bare redirect (`> file`, `: > file`) truncates; a redirect after a real
command is an ordinary write and still runs.
- A forwarded git command keeps its git context (`find -exec git clean`,
`xargs git clean`), and an unquoted `cmd /c` payload spans the remainder.
Adds regression rows for every case and its benign counterpart.
* Gate shell control flow, bash -c clusters, wrapper option values, and annotated aliases
- `if`/`while`/`until` are followed by a condition the shell runs, so a command
there is at command position. `if rm -rf build; then :; fi` slipped both the
classifier and the blocklist (they share the keyword set, so both are fixed).
- A short letter run after `-c` (bash -ce, bash -cl) is more bash options, not
an attached payload: bash still reads the command string from the next token,
so the real payload was never screened.
- A wrapper option taking a separate value (env -u NAME, stdbuf -o L, timeout
--signal TERM, nice -n 5) had its value read as the wrapped command, so
`env -u FOO rm -rf build` resolved the command `FOO` and never judged `rm`.
env -C/--chdir is deliberately excluded: it is gated as a chdir already.
- An annotated binding (f: object = os.remove) is the same alias as a plain
assignment; only ast.Assign was collected.
Adds regression rows for each case and its benign counterpart.
* Fix two gate regressions and close seven more bypasses
Regressions from the previous round, both caught by review:
- Shell keywords were treated as separators anywhere, so `grep if rm README.md`
resolved `rm` as a command and was blocked. A keyword only separates where a
command may start, so gate the check on command position (all three scanners).
- The wrapper option-value table was shared across wrappers, but `env -i` is
valueless while `stdbuf -i` takes a value. `env -i git clean -fd` therefore
consumed `git` and never judged the subcommand. The table is per wrapper now.
New gaps closed:
- `git -c alias.NAME=PAYLOAD` defines code git then runs. Screen the payload: a
`!` alias as a shell command, a plain one as `git <payload>`.
- A script fed to a shell over a pipe (printf '...' | bash) or a herestring
(bash <<< '...') never appears at command position. Ordinary pipes still run.
- `chroot`, `nsenter` and `unshare` cross a privilege or namespace boundary and
then exec a nested command the wrapper hides.
- A bare runtime name (mcp__srv__python, __node, __code) is an MCP execution
tool even without a verb.
- `m = __import__("os")` binds the module like `import os as m`, and
`getattr(__import__("os"), "remove")` reaches it inline.
Declined: gating every command substitution used as a path argument (would
prompt on `echo $(date)` / `make $(FILES)`), and bare `git checkout <path>`
(statically indistinguishable from the very common `git checkout <branch>`).
Adds regression rows for each case and its benign counterpart.
* Pin the auto-mode contract with benign and dangerous corpora
The value of defaulting to "Approve for me" rests on two properties that pull
in opposite directions: ordinary development work must run silently, and
genuinely dangerous work must still prompt. Every denylist change risks
trading one for the other, and a regression in the benign direction is easy to
miss because nothing fails, the mode just starts nagging.
Add two corpora that pin both directions: 62 ordinary commands, python
snippets and MCP calls that must NOT prompt (package installs, builds, tests,
git workflow, reads, ordinary pipes and redirects), and 55 dangerous ones that
must (credential reads, destructive and persistence changes, privilege
escalation, network exec and exfil, container escapes, obfuscated forms).
125 cases, currently 100 percent in both directions.
* Scope four over-prompting checks and close six more gate gaps
Over-prompting fixes (auto mode was pausing on ordinary work):
- find/fd were marked forwarding from the command itself, so every later
positional looked executable and a search whose pattern happened to equal a
gated command name prompted. They only forward after an explicit
-exec/-execdir/-ok flag now.
- The openssl s_client check was not command-position aware, so grepping for
the string in a README prompted.
- An exec-valued flag (--checkpoint-action, --rsh, --rsync-path) counted no
matter which command owned it, so printf '%s' --rsh prompted. It now
requires the owning utility (tar/rsync/scp/sftp) in the same command.
- A listener behind a wrapper or given by absolute path was missed instead
(env uvicorn, timeout 60 gunicorn, /usr/local/bin/uvicorn); resolving the
binary at command position covers all three.
New gaps closed:
- git checkout <commit> <path> overwrites the file from that commit, as does
--pathspec-from-file. A single positional stays ambiguous with a branch name
and is still left alone.
- git config alias.NAME BODY stores code git runs on the next invocation, so
the body is screened like the -c form.
- systemd-run launches a nested command as a transient unit.
- Version-suffixed perl/ruby/php/node still run inline code with -e/-r.
- A file handle bound by `with open(...) as f` is tracked for truncate, not
just an assigned one.
- Exceeding the shell nesting depth now fails closed, matching the docstring,
instead of letting an unscreened payload through.
Declined: rebinding a command name through the bash hash builtin. Like the
alias/read/awk/coproc family already declined, it is deliberate
self-obfuscation of an already-gated command rather than anything a model
emits, and the always-on backstops cover it.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Scope two more over-prompting checks and close four gate gaps
Over-prompting fixes (auto mode was pausing on ordinary work):
- A recursive flag was looked for across the whole command line, so
`grep -R pattern . && chmod +x build.sh` made the chmod look recursive and
prompted. The flag is now scoped to the segment that owns the command.
- The startup-file names were matched anywhere in the line, so `cat
notes.profile.bak` and `my.zshrc.template` prompted. They now have to sit on
a path boundary, while the real dotfiles still prompt.
New gaps closed:
- A pending wrapper option value leaked past a command separator, so the
command after it was never screened (`env -u` followed by a recursive delete
was missed). The pending state is cleared at every separator now.
- git plumbing and maintenance that loses data: update-ref, reflog, gc, prune
and history rewriting drop refs and unreachable objects, the same loss the
porcelain forms already gate.
- A module pulled in dynamically is screened against the same set as a static
import, so a dynamically imported socket or shutil is treated alike.
- MCP names that move money or ship artefacts (transfer, payout, charge,
refund, wire, publish, deploy) are irreversible for the operator even though
they are not destructive in the filesystem sense.
Declined two items:
- Gating arbitrary interpreters that can shell out (awk BEGIN blocks and
friends). Consistent with the alias/read/coproc/trap family already declined
here: it inverts the denylist into an allowlist and costs real ergonomics for
payloads a model does not emit in normal work.
- Prompting on every write outside the session workdir. Ordinary builds and
scripts write to the standard temp directories constantly, so this would
prompt on routine work. Persistence and credential paths are already gated
specifically.
* Resolve command-position globs and keep quoted data out of shell syntax
- A glob at command position is expanded by bash after this scan runs, so
`/bin/r[m] -rf x` was screened under a name that never executes. The
always-on blocklist now resolves such a pattern against the blocked names,
and the classifier asks when a command word cannot be resolved at all. The
test builtins are excluded, and a pattern carrying no literal character
resolves to nothing in particular.
- A dollar-quoted word expands to a single word, so a newline inside it is
data rather than a separator. Decoding it before tokenization made
`printf '%s'` with multiline data read as two commands and the call was
refused outright. The decoded text can no longer introduce shell syntax,
while an escape-obfuscated command name still resolves.
- An attribute name assembled from literals is folded before it is screened,
so a deletion spelled as a concatenation is treated like the plain form. A
name on a filesystem module that cannot be folded at all fails closed, since
there is nothing left to screen.
- An MCP name with no separators never reached the segment boundaries, so a
server-side execution tool was classified as ordinary even though the
previous classifier failed closed on it. The verb and object compounds are
matched directly now, while a name that merely starts with those letters is
left alone.
Also narrowing a verb pair added in the previous commit: subscribing to a
topic is not a billing subscription, and pub/sub tools should not prompt.
* Screen attached exec values, wrapped openssl, php code flags, worktree removal and sysctl writes
- fd accepts the command attached to the flag (--exec=<cmd>, --exec-batch=),
and that spelling was stripped and discarded without ever being screened.
The value is treated as command position now, in the classifier and in the
always-on blocklist. Only the long spellings are read this way: a short -x
belongs to too many other utilities for its neighbour to be a command.
- The openssl socket check was anchored at command position, so a wrapper in
front of it (env, timeout) hid the very thing it was meant to catch. The
subcommand is checked on the resolved command segment now, so the wrapped
and absolute forms are covered. Local openssl (dgst, enc) still runs.
- php runs code from -B, -R and -E as well as -r, which are begin, per-line
and end blocks. Only -r was listed, so the other three ran inline programs
unscreened.
- git worktree remove --force deletes a linked worktree even when it holds
uncommitted work or is locked, but only the first-level subcommand was read
so the nested action was invisible. An unforced remove refuses on a dirty
worktree and stays out, matching how the checkout and switch discard flags
are handled.
- sysctl -w, --system and -p change kernel parameters, and the assignment form
writes without needing a flag. A read-only query stays automatic.
* Fail closed on unscreenable MCP names, alias bodies and stored lookups
- An MCP name whose verb this classifier does not recognise now asks. MCP
tools run on an external server, outside the terminal sandbox and every
backstop under it, and their names are an open vocabulary rather than the
finite set of POSIX utilities, so the denylists could never be complete: a
name built from an unfamiliar verb sailed through as ordinary. A generous
read and write vocabulary keeps the everyday tools running, and the reverse
or repeat of a recognised verb (undelete, reopen, resend) counts as
recognised too. Measured against thirty tool names taken from the common
servers, one still prompts, and that one is the pre-existing execution rule
rather than this one.
- A shell alias body is a command bash runs when the alias is invoked, so it
is screened as a command in its own right, in the classifier and in the
always-on blocklist. This is the same shape as a git alias body, which was
already handled; leaving the shell form out was inconsistent.
- git --config-env=<key>=<envvar> takes its value from the environment, so an
alias key stores code that never appears in the command text at all. The
attached form was skipped entirely because the parser required no equals
sign. An alias key gates it now; ordinary keys are untouched.
- A destructive lookup stored before it is called (a name bound to
getattr(os, "remove")) matched neither the direct call shape nor the alias
collection, so it ran. The binding is tracked now.
- A credential basename only names a file when it appears in a string, but the
whole Python source was being scanned, so `credentials = {}`, a function
called load_credentials and even a comment mentioning credentials all
prompted while performing no I/O. The check applies to string literals now,
with the raw scan kept for source that does not parse.
* Split git short-option clusters and close five more gate gaps
- Git combines short options, so `git push -qf`, `git checkout -qf` and
`git branch -qD` never matched the exact-string flag sets and ran without a
prompt. Clusters are split before the destructive flags are checked. Also
adds the short `-f` spelling to the branch set, which moves a ref and can
abandon its commits.
- `getent shadow` and `getent gshadow` return password hashes straight from
NSS, so the read never spells out a path for the sensitive-path check to
find. The database name is gated instead; ordinary lookups (hosts, passwd)
still run.
- The account-management set covered useradd and usermod but not adduser,
deluser, addgroup, delgroup, groupmod, gpasswd, newusers or chgpasswd, so
`gpasswd -a user sudo` granted group membership silently.
- at and batch hand a payload to atd, which runs it later as this user and
outside this invocation's blocklist, resource limits, timeout and
cancellation. They belong with crontab.
- A command word bash builds without the NAME=value form (printf -v, read)
left nothing at command position to screen. A bare variable executed as a
command that assignment expansion could not resolve now fails closed. A
variable used as a path prefix is deliberately excluded: ${VENV}/bin/python
still leaves a literal basename the scan can read.
* Stop prompting on six inspection shapes and close eighteen gate gaps
Over-prompting fixes, which matter most here since not interrupting ordinary
work is the point of the change:
- `git clean -n` and `--dry-run` list what would be removed and remove nothing,
so they are inspection commands. The subcommand was gated regardless of its
flags; a dry run is now recognised in the same segment.
- The listener check matched a module name anywhere in the line, so
`echo 'python -m http.server'` and grepping for it prompted. It is anchored at
command position now, like the server-binary check beside it.
- An MCP name that reads names its SUBJECT, not the action: `get_release`,
`get_invoice`, `search_code` and `get_code` were prompting because the impact
and runtime-noun patterns fired on the noun. A read verb now suppresses both,
while an execution verb still wins.
- Free text is not a statement. An issue body or chat message that mentions
DELETE FROM, a credential file or a path was read as an action. Statements are
taken from the query-bearing argument names, and paths are skipped only for
the prose names, since a path can be carried under any other name.
- curl and wget presence was decided by substring, so `grep curl notes.txt &&
wget -T 5 ...` lent curl's option letters to wget.
Gaps closed:
- git checkout-index -f overwrites the working tree from the index; git tag -d
and -f delete or replace a ref; git switch -C and checkout -B reset an
existing branch the way branch -f does.
- Ending a process (kill, pkill, killall, taskkill, tskill) or the machine
(shutdown, reboot, halt, poweroff) was ungated, though the Python os.kill
equivalent already prompted. setcap grants file capabilities without sudo.
- A network client behind a wrapper (env curl -T) was missed because the client
check ran before the wrapper was resolved. slogin is a standard ssh alias and
was in neither set. wget spells the request method --method=DELETE.
- A tracer (strace, ltrace, valgrind, perf) runs the rest of the line as a
child, so the real command sat in argument position behind it.
- A redirection may precede the command word, so `</dev/null` hid what followed
from both scanners. `exec -a NAME cmd` puts a name where the command goes, and
the Windows `if exist FILE cmd` form puts an operand there.
- In Python: a walrus binds a module or a callee just like an assignment,
builtins.__import__ is the attribute form of __import__, and psutil ends a
process exactly as os.kill does. The psutil check is keyed on the import so an
unrelated .kill() on a user object keeps running.
- Over MCP: a credential carried in an argument NAME (Authorization, X-API-Key,
Cookie) goes out whatever its value looks like; collaborator and team-member
grants are access changes like the role verbs; and a recurring subscription
bills repeatedly.
* Bound the classifier's input and stop prompting on four more ordinary shapes
Found by simulating the whole corpus against pre-PR main on Linux, macOS and
Windows tokenizers and diffing the two, then feeding the classifier adversarial
input.
Robustness:
- The credential-path pattern backtracks superlinearly, so a long argument made
a single classification take seconds. Measured on main as well as here, so it
predates this change, but this change makes the auto gate the default and so
runs it on every call. Text far past any real path, and a command far past any
real command, now fail closed: they ask rather than spending unbounded time
deciding. Worst case over the adversarial set drops from a hang to 13 ms.
Over-prompting fixes:
- A container CLI reading its own state (docker ps, docker images, docker logs,
kubectl get) is inspection. The whole CLI was gated because the escape lives
in the arguments of run/exec, so the read subcommands were caught with it. An
unrecognised subcommand still asks, so the list can only be too small.
- A python payload is screened with the same analyzer the python tool uses, so
`python -c 'import torch; print(torch.__version__)'` runs while a destructive
one-liner still asks. A payload that does not parse fails closed, since shell
quoting may have mangled it. The other runtimes have no analyzer here and stay
gated.
- An assignment with no command after it runs nothing: every terminal call gets
its own shell process, so `export PATH=...` on its own dies with that process.
Verified against real bash rather than assumed.
- For the search paths other than PATH (PYTHONPATH and friends), a relative
entry points inside the session workdir, which is the agent's own directory,
so `PYTHONPATH=. pytest` runs. An absolute or escaping entry can shadow a real
module and still asks. PATH itself counts for every value, because a relative
entry there is the sharpest form of the hijack (`PATH=. ls` runs ./ls).
Net effect on the probe corpus, identical on all three platforms: ordinary and
inspection commands go from 99 of 136 prompting to 0, dangerous stays at 99 of
99, and the always-on hard-block set loses nothing and gains six entries.
* Tighten the permission-mode comments
Comment-only pass over the code this branch added. Every explanation is
collapsed to the fewest lines that still read clearly, redundant restatements
of the code are dropped, and a handful of blocks that had drifted away from the
constant or branch they describe are moved back next to it.
The non-obvious behaviours keep their note, just shorter: an unforced
`git worktree remove` refusing on a dirty worktree, a bare `-c` yielding an
empty attached value rather than None, `.` being the POSIX synonym for
`source`, prose keys being skipped rather than path keys allowlisted, and the
route keeping an unset mode lenient so non-streaming clients still work.
No code, string literal or test expectation changed.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Gate the navigation sinks reached by bracket access
The canvas egress check gated location.assign / location.replace and an
assignment to location.href, and it already handled bracket access for the
fetch family, but not for the navigation sinks. So `location['assign'](url)`
and `location['href'] = url` auto-ran and could navigate the preview frame to
an attacker URL with the page contents appended, which is the same egress the
dot forms already gate.
Both bracket forms are covered now, including a fully bracketed host
(`window['location']['href']`). The names are anchored to location so ordinary
bracket keys stay static: a string's own `['replace']`, an object's `['href']`,
and reading `location['href']` all still run without a prompt.
* Gate seven more ways a command reaches the shell in auto mode
git submodule foreach runs its argument in every submodule, so the payload is
a command in its own right; it now recurses through the terminal classifier and
through the hard-block scan. An awk program can shell out with system() or by
piping to "sh", so the program text is screened for those two shapes while
ordinary field work (awk '{print $1}') keeps running.
setpriv changes privilege and then execs what follows, so it is transparent to
the scan (setpriv --nnp rm -f x resolves rm) and its privilege-raising flags
(--reuid, --ambient-caps, --bounding-set) prompt on their own. fallocate
punches, zeroes or collapses a range in place, which destroys file contents,
so those flags prompt while plain allocation (-l SIZE) does not.
vars(os)["remove"] and os.__dict__["unlink"] resolve an attribute the same way
getattr does, so the module namespace dict is screened with the same key rules,
anchored to a filesystem module so an ordinary d["remove"] stays out.
Removing a package (pip uninstall torch, uv pip uninstall, conda remove) tears
down the environment the backend itself runs in; installing into it does not,
and stays automatic.
The listener check was anchored at command position, so a wrapper in front of
it (env python -m http.server, timeout 60 python -m uvicorn) slipped past. The
module after -m is now resolved at the token level, after wrapper resolution.
Adds 54 rows to the classifier tables covering both directions.
---------
Co-authored-by: danielhanchen <unslothshared@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <unslothai@gmail.com>
* fix(studio): resolve bare git on Windows sandbox PATH
Sandboxed terminal tools rebuilt PATH as venv + System32 only, so
user-installed Git under Program Files never resolved by bare name.
Append absolute host PATH dirs after the curated prefix and inherit
PATHEXT on Windows (#7317).
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* fix(studio): restrict sandbox PATH inheritance to Windows Git dirs (#7323)
Only append Git-for-Windows install directories from the host PATH on
Windows, instead of every absolute entry. This fixes bare `git` resolution
(#7317) without letting user-writable dirs (venv, node_modules/.bin)
shadow auto-safe terminal commands.
* Pin sandbox PATHEXT to block cwd script hijacks (#7317)
Use a fixed .EXE;.COM list instead of inheriting the host PATHEXT so
cmd cannot resolve auto-approved bare names from workdir .BAT/.CMD stubs.
* Resolve sandbox git dir via shutil.which and disable cwd exe lookup
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Keep non-exe git launchers resolvable under restricted PATHEXT
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Restrict inherited sandbox git dir to system install roots
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Drop SystemRoot trust and canonicalize short paths for sandbox git
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Resolve Program Files via known-folder API and append canonical git dir
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Trust native Program Files on 32-bit Windows and stub program roots in tests
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Scan PATH for a trusted git and derive native Program Files root
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Drop ProgramFiles env from the trusted-root fallback
* Fail closed when trusted Program Files root cannot be resolved
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Daniel Han <danielhanchen@gmail.com>
* Studio: stream live tool output with SSE heartbeats and fix web page extraction
Server-side python/terminal tools now stream incremental stdout to the chat
UI while running (new tool_output SSE event), and every blocking tool
execution emits heartbeat keepalives so reverse proxies (Cloudflare tunnels
cap idle streams at ~100s) cannot drop the connection mid-turn. The tool
loop routes also emit a stall keepalive during silent prompt prefill between
tool iterations. The final role=tool message the model sees is byte-identical
to before, so tool-call parsing, nudging, and healing are untouched.
web_search page fetches now extract main content: GitHub repo root pages are
rewritten to the README API (with HTML fallback), hidden/aria-hidden client
error placeholders are dropped, conversion scopes to article/main, and known
boilerplate fragments are stripped. Non-HTML responses are returned raw
instead of being run through the HTML converter.
The frontend renders live-scrolling tool output inside running python and
terminal cards, and a chat stream that ends without a terminal signal now
surfaces an explicit interrupted state with a Retry action instead of
silently ending the turn.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: fix content-type sniffing, unlimited-timeout drain, and env parity in tool streaming
Content-Type sniffing: get_content_type() defaults to text/plain when the
header is absent, so the sniffing fallback never fired and header-less HTML
came back as raw markup. Report an empty type for a missing header and sniff
the body whenever the declared type is not HTML, so mislabeled text/plain
HTML pages are converted like before the extraction change.
Unlimited timeout drain: with tool_call_timeout disabled the old path used
communicate(timeout=None) and waited for EOF, but the streaming drain capped
the post-exit drain at a 5 second join, truncating output from a grandchild
that holds stdout open. When timeout is None, drain until EOF or the cancel
event fires; finite timeouts keep the bounded remaining-budget join.
Env parity: drop the PYTHONUNBUFFERED=1 injection on the streaming path so
the child invocation is byte-identical with and without streaming (the env
var was model-visible via os.getenv). Live streaming granularity now depends
on the child flushing; unflushed output arrives in ~8 KB chunks or at exit
and the final result is unchanged, with SSE heartbeats covering the gaps.
* Studio: stream tool-call arguments while the model writes them
A model writing a large tool call (a full python game is minutes of
generation) produced nothing on the stream: the structured path
accumulated delta.tool_calls fragments silently after the provisional
card, and the text path's DRAINING state consumed everything until
stream end. The user saw a dead Running spinner while the model was in
fact writing code, and the byte-silent SSE segment was also the window
where proxies drop the connection.
New tool_args SSE events stream the arguments as they generate. The
structured path forwards each fragment once a provisional card exists
(backlog first, so the card starts from the top of the call). The text
path sniffs the drained call for an enabled tool name and streams the
raw call text under the id the stream-end parser assigns its first call
(call_0), so the final tool_start reconciles the same card; the sniff is
gated on enabled names plus the provisional size floor, and prose or
ordinary JSON answers never spawn a card. The safetensors loop streams
the drained render_html call to its existing provisional card the same
way.
The chat adapter accumulates the raw stream per card and feeds a partial
JSON parse (call envelopes and stringified arguments unwrapped) into the
part's args, so the python and terminal cards render the code live and
the render_html canvas builds while streaming; both cards now say
Writing code / Writing command during this phase via useToolArgsStatus.
Display only: the parser input, the executed call, and the conversation
the model sees are byte-identical, covered by new loop-level tests for
the structured path, the text path, and the no-tool JSON answer.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: keep full tool output visible past the model cap; heal /mnt/data habits
Live testing surfaced two issues in the tool streaming UX.
First, a long python stdout ended in '... (truncated' in the finished
card: the model-visible result is capped by tools._truncate
(_MAX_OUTPUT_CHARS, previously 8000 chars) to protect the context
window, and the card rendered that capped text even though the live
stream had already shown everything. The cap stays (raised to 16000,
overridable via UNSLOTH_TOOL_RESULT_MAX_CHARS) but display and model
concerns are now split: the adapter preserves the accumulated live
stream on tool_end whenever it captured more than the final result, and
the finished python/terminal cards prefer it. The live-stream ceiling
rises from 16 KB to 400 KB (chunks batch per poll, so SSE stays cheap),
and both the live pane and the finished card render only the last 2000
lines with a Show all control so a huge output cannot jank the DOM. The
truncation notice now tells the model the user saw the full output and
that written files persist in the working directory. The final result
string remains byte-identical with and without streaming.
Second, models trained on ChatGPT code-interpreter transcripts write to
/mnt/data, which does not exist here (the sandbox CWD is a per-thread
persistent dir). Three layers, all identical across streaming and
non-streaming paths: the python/terminal tool descriptions gain one
sentence saying to use relative paths in the persistent CWD; a failed
execution whose output shows a missing-file error on a known
code-interpreter prefix (/mnt/data, /mnt/outputs, /home/sandbox,
/workspace) gets a model-visible retry hint appended after truncation
so it always survives; and a sitecustomize shim on the sandbox
PYTHONPATH remaps those prefixes onto the CWD in open()/os.makedirs()
with a one-line stderr notice, covering the python tool and any Python
launched from the terminal tool without touching the exec wrapper (so
tracebacks keep their line numbers). Bash-level file operations cannot
be redirected without root or mount namespaces, so they rely on the
description and the hint.
* Studio: fix hidden-element parsing, heartbeat gaps, and tool output id collisions
Review follow-ups on the tool streaming work:
- _html_to_md: treat any present hidden attribute value as hidden (it is an
enumerated attribute whose invalid value default is the Hidden state, so
hidden="false" is still hidden), and implement HTML5 optional end tags so
an unclosed <p hidden> or <li hidden> ends at the next sibling start tag
instead of swallowing every following sibling until the parent closes
- tool_stream_exec: keep heartbeats flowing after the live-output cap; a
tool that keeps printing past the cap kept the queue non-empty, so neither
tool_output nor heartbeat events were emitted and the SSE stream went
silent past proxy idle timeouts
- routes/inference: forward tool heartbeats before the
disable_parallel_tool_use drop window swallows events, so a dropped call
that executes server-side cannot leave the Anthropic stream silent
- llama_cpp: close the provisional text tool card with a tool_end when the
drained call fails to parse (DRAINING false-positive path), so the card
cannot spin forever while the text is delivered as content
- tools: decode terminal output as utf-8 with errors=replace like the python
tool; invalid bytes used to raise UnicodeDecodeError from communicate() on
the non-streaming path and silently truncate the streaming reader, so the
two paths diverged
- sitecustomize: patch io.open alongside builtins.open; pathlib Path.open,
read_text and write_text call io.open directly and bypassed the remap
- frontend: scope the toolLiveOutput/toolFullOutput store keys by pane
(modelType and pairId) and clear stale entries on tool_start; backend ids
like call_0 repeat across turns and across concurrently streaming panes
(compare mode), so a later turn or another pane could display the wrong
preserved output, and run-end cleanup now clears only its own keys
Each backend fix carries a regression test that fails on the previous code;
the byte-identity tests between streaming and non-streaming stay green.
* Studio: keep tool failure status visible and truncation/remap notices truthful
Finished python/terminal cards preferred the fuller live stream by length
alone, so a tool that printed a lot then timed out or exited non-zero showed
the captured stdout but dropped the final result's status (timeout notice,
Exit code N). preferFullToolOutput now shows the stream when the result is
just its truncated prefix, and appends the result otherwise so the failure
tail always survives and the copy button copies both.
The result truncation notice claimed the user was shown the full output, but
the same wrapper serves non-streaming chat/API and direct execute_tool()
callers where nothing is streamed to anyone. The notice is now mode-neutral
and stays byte-identical with and without an output_callback, keeping the
streaming vs non-streaming invariant intact.
The sandbox sitecustomize shim now remaps /tmp/outputs into the working
directory only while it does not already exist, so a real /tmp/outputs the
user's own code created is never shadowed; /tmp/outputs also joins the
missing-path retry-hint list.
* Studio: suppress hidden void elements and keep live output scroll pinned only when at bottom
* Studio: drop capped tool output without concatenating; remap pathlib mkdir
Past the live-output cap stream_tool_execution built item + _drain_pending()
(the current chunk joined with every queued sibling) only to discard it in the
capped branch, so a chatty tool (yes, a tight print loop) could enqueue far
more than one poll interval of text and blow past the memory/CPU ceiling the
cap exists to enforce. Drain and drop queued items without building a combined
string, still counting each drain toward the heartbeat cadence so the SSE
keepalive survives.
Generated code often prepares code-interpreter paths with
Path('/mnt/data').mkdir(parents=True, exist_ok=True); pathlib drives that
through os.mkdir (not the patched os.makedirs) per component and, on
FileExistsError, probes the unpatched os.stat via Path.is_dir(), so the setup
raised before open() ever ran. Patch os.mkdir with the same remap and patch
Path.mkdir so the whole parents/exist_ok dance lands on the mapped working
directory and stays idempotent; real paths still pass through.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: generalize sandbox write remap and hint to any hallucinated absolute path
Models invent absolute paths from seeing their CWD (a DeepSeek run did
open('/home/ubuntu/Sandbox/flappy_bird.html', 'w') and died with
FileNotFoundError). A prefix list cannot enumerate these, so the sitecustomize
shim gains a write-mode fallback in open()/io.open(): when a write/create-mode
open targets an absolute path outside the CWD whose parent directory does not
exist, redirect it to the basename in the CWD and emit the same one-line stderr
notice, echoing the original path. The prefix remaps still run first (they cover
reads and preserve subpaths); read modes never hit the fallback so real system
files fail or succeed truthfully; bytes paths pass through. The fallback is not
applied to mkdir/makedirs/Path.mkdir, since creating an arbitrary absolute
directory can legitimately succeed on the host, and that decision is documented
in a comment.
The model-visible retry hint now echoes the real failing path (parsed from the
traceback tail) instead of the canned /mnt/data example, and fires for any
absolute path outside the working directory, not just the enumerated prefixes,
while a relative miss still gets no hint.
The shim wrapper still adds one frame to tracebacks that surface open() errors;
suppressing only our frame has no clean standard mechanism (a wrapper always
adds a frame), so the frame is left as an accepted compromise.
Tests: hallucinated absolute write remaps to the CWD basename across w/a/x/w+;
reads of a missing absolute path pass through untouched; writes to an existing
external dir pass through; prefix subpaths still preserved; end-to-end write
fallback lands the file in the sandbox workdir identically with and without
streaming; the hint echoes the actual path for convention and non-convention
absolute paths alike.
* Studio: kill exited process groups on drain; bound the over-cap output batch
_drain_process_output killed the process only via _kill_process_tree, which
short-circuits once the parent has exited, so a grandchild that inherited
stdout and outlived the parent was never signaled: a finite-timeout run could
return while it kept holding the pipe, and a timeout=None cancel left it
behind. Capture the setsid process group before waiting and SIGKILL that group
at both give-up points so the whole tree is torn down.
The streaming wrapper's first over-cap batch joined the current chunk with the
entire pending backlog before enforcing the live-output cap, so a chatty tool
could allocate far past the cap on the crossing batch. Bound the drain to the
remaining budget and drop the surplus in place, keeping the truncated output
byte-identical to joining everything.
* Studio: harden sandbox path healing and process/generator cleanup
Sandbox sitecustomize shim:
- Make the generalized write fallback collision-safe: never redirect an
invented absolute path onto an already-present CWD file (refuse and let the
original open raise FileNotFoundError, preserving the workspace file).
- Only w/a/x create a file; r+/rb+ are read-update modes that require the
target to exist, so a bare + no longer trips the write fallback.
- Gate every convention-prefix remap (/mnt/data, /mnt/outputs, /home/sandbox,
/workspace) on the prefix root being absent, so a real host mount is never
shadowed; a miss under an existing real prefix passes through.
- Patch os.open so Path.touch and other low-level creators heal convention
paths too, matching the Path.mkdir patch.
Local code execution (tools.py):
- Capture the setsid process group right after Popen (before any watcher can
poll/reap the leader) and thread it through the cancel watcher and drain.
- Kill the captured group in the non-streaming python/terminal timeout branch
so an exited leader no longer leaks a stdout-holding grandchild (matches the
streaming drain path).
- Guard os.getpgid/os.killpg by platform so streamed execution no longer
raises on Windows; fall back to single-pid kill.
- Judge missing-path hints against the executor's real workdir so a legitimate
miss inside a project workspace outside the sandbox root is not mislabeled.
Tool streaming routes (routes/inference.py):
- Drain a pending next(gen) worker before closing the generator in the
safetensors and Anthropic tool streams, so a disconnect no longer races
gen.close() (generator already executing) or leaks the thread/generator.
HTML to markdown:
- Only drop boilerplate lines composed entirely of known furniture phrases so
real prose that merely quotes one (for example "we use cookies to
authenticate requests") is preserved.
Adds hermetic tests for each change.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: keep aside callouts, contain sandbox path remaps, and keepalive dropped Anthropic tool events
_html_to_md: stop dropping <aside> unconditionally. Documentation pages
render notes/warnings/examples as aside admonition callouts; those inside
the selected article/main scope are real content. A furniture aside outside
the scope is already excluded by the main-content pass.
sitecustomize: contain the code-interpreter path remap under the sandbox
CWD. A hallucinated habit path such as /mnt/data/../other_session/file no
longer escapes the per-conversation workdir; parent-traversal components in
the suffix are dropped and a '.'/'..' write-fallback basename is refused.
routes/inference: emit a rate-limited comment keepalive when the Anthropic
Messages stream drops tool_output/tool_args events. A chatty tool keeps the
generator busy so the stall keepalive never fires and the tool wrapper emits
heartbeats only while idle, which left the SSE stream silent past proxy idle
caps; the OpenAI passthrough paths forward these events, this path now keeps
the connection alive.
* Studio: bound the tool-output chunk that first crosses the live cap
_drain_queue joined the entire chunk that first crossed the live-output
cap before dropping the rest, so a single multi-megabyte line (or any
chunk dequeued once the budget was already met at max_chars <= 0) was
materialized in full only to be truncated away, defeating the memory
ceiling the cap enforces. Slice the crossing chunk to one character past
the budget: that preserves the caller's overflow signal and its
byte-identical truncation while dropping the arbitrarily large remainder
in place.
* Studio: scope missing-path hint to the failing line, keepalive dropped-call output, and preserve truncated tool streams over byte length
- tools._missing_path_hint: the code-interpreter convention-prefix trigger
scanned the whole output, so a convention prefix mentioned only in a
traceback frame (a /workspace project root) or printed by the user's code
would add a misleading 'use a relative path' hint even when the actual
FileNotFoundError was a relative or in-workdir path. Scope the convention
test to the failing-path error line(s), matching _extract_missing_abs_path.
- _anthropic_tool_stream: the tool_output/tool_args rate-limited keepalive sat
after the drop_until_tool_end skip, so under disable_parallel_tool_use a
chatty second-or-later tool call was dropped whole with no keepalive, letting
an idle proxy kill the SSE stream. Check the keepalive branch before the drop
skip (like the heartbeat branch) so dropped-call output keeps the stream alive.
- preferFullToolOutput / chat-adapter: a truncated result can be longer than
the live stream by byte count once its footer, an 'Exit code N:' notice, or an
__IMAGES__ base64 tail is appended, so the length-only gate discarded the full
stream and the finished card fell back to the truncated text. Add a shared
truncation-aware shouldPreserveFullOutput used by both the write and read
sites: preserve the stream whenever the result carries the truncation footer.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: skip the habit-path hint for real project paths under a convention prefix
* Kill captured process group on streamed wait-timeout
The streamed drain path's proc.wait() timeout branch only called
_kill_process_tree(proc). If the leader exits in the narrow window between
the wait timing out and _kill_process_tree sampling its pgid, that helper
short-circuits on the reaped leader and a stdout-holding grandchild in the
same group survives. Also kill the captured pgid there, matching the
non-streaming communicate() timeout path. Adds a hermetic regression test
that models the reaped-leader race by stubbing _kill_process_tree.
* Fix 3.10 pathlib write_text remap and honor cancel in finite drain
On Python < 3.11 pathlib routes Path.open / read_text / write_text through
a module-level accessor singleton whose open attribute captured the original
io.open at import time (_NormalAccessor.open = io.open). Patching io.open in
the sandbox shim therefore never reached that captured reference, so a
Path('/mnt/data/x').write_text(...) raised FileNotFoundError on 3.10 while
passing on 3.11+ (which dropped the accessor and calls io.open at call time).
Repoint _NormalAccessor.open at the same io.open wrapper via a staticmethod,
guarded so it is an idempotent no-op on 3.11+. Keep the test save/restore
helpers symmetric so the accessor is restored too, and add a hermetic
write_text/read_text remap test that covers every version.
Also honor cancellation while draining inherited stdout after the leader
exits. Once the leader is reaped the cancel watcher returns (its loop is
while proc.poll() is None), so the finite-timeout drain did one blocking
reader.join(timeout=remaining) that ignored cancel_event and kept draining a
chatty grandchild for the whole budget after a disconnect/Stop. Poll
cancel_event in 0.5s slices against a deadline like the timeout=None branch
and kill the captured process group promptly on cancel. The normal path still
reaches EOF on its own, so the streamed vs non-streamed result is unchanged.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: port no-tool stream keepalive/drain and fix subprocess/queue/extraction asymmetries
Streaming no-tool paths now match their tool twins:
- _anthropic_plain_stream, safetensors/MLX no-tool stream, and standard GGUF
no-tool stream run next(gen) in a worker with a timed SSE keepalive loop so a
long prompt prefill cannot leave the stream idle past a proxy cap.
- The Anthropic plain and safetensors/MLX no-tool teardowns now drain the
pending next(gen) worker and close the generator on disconnect instead of
leaking the suspended generator.
Other asymmetries:
- Non-streaming _python_exec/_bash_exec always drain via _drain_process_output
(output_callback may be None) so a cancelled run reaps a stdout-holding
grandchild that outlived the leader instead of blocking in communicate(). The
joined bytes are identical to communicate(), so streamed vs non-streamed
results stay byte-identical.
- _build_bypass_env installs the sitecustomize path shim on PYTHONPATH (prepend,
keeping the operator's entries) so /mnt/data remap works in bypass mode too.
- GGUF forwards output_callback to execute_tool only when the callable accepts
it (shared accepts_output_callback), matching safetensors and preserving
legacy monkey-patched signatures.
- tool_stream_exec bounds accepted live output at the producer boundary so a
chatty tool cannot grow the queue without limit under consumer backpressure
and cannot keep the drain spinning and starve heartbeats.
- html_to_md implicit-close now searches past unclosed inline descendants so a
hidden <p>/<li> is closed by a following block; main-content scoping gates on
the largest single <article>/<main> so a swarm of tiny cards cannot pass the
threshold in aggregate and displace the real main.
- preferFullToolOutput re-attaches the "Exit code N:" prefix to the fuller
stream instead of appending the still-prefixed result, so a failed truncated
tool no longer duplicates its stdout in the finished card.
Adds hermetic tests for each.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Preserve short live output on timed-out tools; strip inline-CSS-hidden subtrees and score truncated main-content scopes
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Tighten chat tool streaming comments and docstrings
* Keep HTML READMEs from the GitHub API and preserve interrupted tool output
Convert a 200 HTML README body from the GitHub README API to Markdown
instead of discarding it and falling back to the repo page chrome, and
promote captured live stdout to full output when a tool never reaches
tool_end (stream interrupted or cancelled) so the partial diagnostics
stay on the finished card.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: anchor HTML sniff, keep repeated sandbox writes, reuse textual tool ids
Anchor _looks_like_html to the leading doctype/tag so a Markdown README that
opens with a fenced HTML example stays Markdown (no html_to_markdown
corruption), while bare HTML fragments (<body>/<article>/<section>) are still
detected and converted on a missing/wrong Content-Type.
Let the sandbox write fallback re-serve a target it already healed for the same
invented absolute path, so iterative overwrites of a generated artifact stop
failing with FileNotFoundError while the anti-clobber guard still refuses
unrelated same-basename files.
Reconcile the first textual tool call carrying an explicit id onto the open
provisional TEXT card instead of spawning a duplicate card under that id.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: run implicit-close before skipping tags and keep leading README tables as Markdown
A skipped block (<nav>/<footer>) is an HTML5 optional-end-tag closer of an
open <p>, but handle_starttag returned before the implicit-close bookkeeping,
so a never-closed <p hidden> kept its hidden mark and swallowed every following
sibling. Run _close_implicit before the skip decision so the hidden mark is
released and trailing content renders.
Drop <table> (and its <thead>/<tbody>/<tr>/<td>/<th> children) from the
_looks_like_html leading set: Markdown READMEs routinely open with a raw HTML
<table> badge/layout row, and sniffing that as HTML collapsed the whole
Markdown body through html_to_markdown, exactly like the already-excluded
<div align>/<p align> layout headers.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: make bypass-permissions Popen double faithful to the unified drain path
The non-streaming _python_exec/_bash_exec now share _drain_process_output,
which reads proc.stdout in a reader thread and calls proc.wait(); the test
double only implemented communicate(), so bypass-mode bash returned an
AttributeError instead of the faked output. Give _FakeProc a readable stdout
pipe (yields the fake line then EOF), wait()/poll()/pid, so the test exercises
the real drain path on both the python and bash bypass branches.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: persist sandbox path heals across runs and suppress nested hidden lists
* Studio: run tool Python child unbuffered (-u) so unflushed prints stream live
A long-running snippet doing bare print() without flush=True never reached
the live-output pane: CPython block-buffers stdout when writing to a pipe, so
_drain_process_output's readline() saw nothing until the buffer filled or the
process exited. Launch the child with the interpreter -u flag so stdout is
unbuffered and each print streams as it is produced.
-u is applied unconditionally on both the streaming and non-streaming path, so
the child invocation stays byte-identical with and without streaming and the
final joined result is unchanged (buffering/timing only). Unlike the earlier
PYTHONUNBUFFERED=1 env injection that was removed, -u does not pollute the
child's os.environ and is not visible via os.getenv.
* Render only the selected main-content subtree in html_to_markdown
The main-content heuristic sized each <article>/<main> candidate
individually to pick the largest subtree, but then rendered every
matching tag in the document. A page with one real article plus
sibling related-post cards or comment threads passed the size gate on
the real article yet still emitted the unrelated siblings.
Size and render the same chosen subtree so only the selected
main-content subtree reaches the output.
* Studio: tighten chat-tool-streaming fix comments
* Studio: store tool-output-scope separators as unicode escapes
The pane-scope and tool-output-key separators were literal NUL (0x00) bytes, which made git treat the file as binary and hide its diff and blame. Write them as \u0000 escapes instead; the runtime key value is unchanged.
* Studio: bound tool-stream teardown when the client disconnects
stream_tool_execution ran its yield loop with no try/finally, so a gen.close() on client disconnect (GeneratorExit at a yield) skipped the worker join and never signalled cancellation. A tool that does not poll cancel_event mid-flight (web_search, MCP, search_knowledge_base) then kept request teardown blocked until the tool's own timeout. Thread the request cancel_event into the wrapper, set it only on the abnormal-exit path so a clean multi-tool turn is unaffected, and bound the worker join to a few seconds; the daemon worker cannot outlive the process.
* Studio: sandbox path remap no longer masks missing reads
The sandbox sitecustomize shim remapped code-interpreter prefixes (/mnt/data, /workspace, ...) onto the working directory for every open mode, including reads. A read of a path that truly did not exist was silently redirected onto a same-basename workdir file instead of raising on the path the model used, hiding real missing-input errors. Remap writes and creates as before, but remap a read only when the mapped workdir target already exists (re-reading a just-written artifact); otherwise keep the original absolute path so the failure stays truthful.
* Studio: bound web fetch with one overall deadline and cancellation
The web fetch applied timeouts per network operation, so a GitHub README API attempt plus its HTML fallback plus up to five redirect hops could run well past the tool timeout, and nothing aborted once the client had disconnected. Add a single wall-clock deadline shared across the API attempt, the fallback, every redirect hop and the body read, cap each hop's socket timeout at the time left on the budget, and poll cancel_event. SSRF host pinning, per-hop redirect revalidation, the five-hop cap and the size cap are unchanged.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: keep tool-stream teardown off the event loop on disconnect
The bounded worker join added for disconnect safety still ran on an abnormal close, so a client disconnect could wait the full join timeout; and the safetensors and Anthropic tool streams closed their generator synchronously on the event loop, unlike the GGUF path. On abnormal exit the daemon worker is abandoned, so join with a zero timeout instead of waiting; offload the safetensors and Anthropic gen.close to a thread to match GGUF; and surface a heartbeat as soon as cancel_event is set while the worker is silent so the route regains control at once instead of after a heartbeat interval.
* Studio: extend the web-fetch deadline to DNS, the body read, and search
The overall fetch deadline did not cover host resolution or the response body read, and query-mode web_search ignored cancellation. Resolve hosts (initial and every redirect) on a budget-polled helper so a slow or pre-cancelled getaddrinfo aborts on time; read the capped body in chunks with the budget re-checked between them so a slow-drip server cannot stretch a single read past the deadline; and gate the blocking DDGS query on cancel_event on both sides. SSRF host pinning, per-hop redirect revalidation, the five-hop cap and the size cap are unchanged.
* Studio: defer the sandbox remap notice and tighten os.open create flags
The one-shot remap notice fired while computing the mapping, so a read that kept its original path emitted a false notice and spent the notice a later genuine remap needed. Only emit it once _remap_open commits to the redirect. Separately, os.open classified O_TRUNC / O_APPEND without O_CREAT as creating, but those cannot create a missing file, so a missing target now stays truthful (only O_CREAT maps to the creating mode).
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: convert only genuine HTML README bodies, not Markdown with a leading block tag
The GitHub README API returns the raw file, almost always Markdown. _looks_like_html classified a Markdown README opening with a block tag (<ul>, <ol>, <dl>, <pre>, <blockquote>) as HTML, so _fetch_page_text ran it through html_to_markdown and collapsed its headings, lists and fenced code into a single line. Sniff the README body with a stricter document-level check (doctype or a leading <html>/<head>/<body>) so only a real .html README is converted; the general page path is unchanged.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Surface unclassified mid-stream Anthropic errors as SSE error events
The local Anthropic tool-stream and plain-stream paths called
_anthropic_stream_error_event(e) with force defaulting to False, so an
unclassified mid-stream failure (llama-server crash, decode OOM, a
dropped upstream socket) returned no event. The except block then fell
through to emitter.finish(), emitting a normal message_delta and
message_stop that masked a truncated turn as a clean finish.
Pass force = True at both fall-through sites so an unclassified failure
emits a 500 SSE error event and returns, matching the Anthropic
passthrough path that already forces it. Add regression tests covering
both stream paths.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: give each tool run a unique part id so finished cards keep their own output
Backend tool ids restart at call_0 every assistant response, and the
transient toolLiveOutput/toolFullOutput store maps were keyed by pane
scope plus that bare backend id. Two turns in the same pane therefore
shared one key: the stale-clear at tool_start only guards the forward
direction, so when a later call_0 finished and wrote its preserved full
output, every earlier still-mounted finished card reading the same key
re-rendered and displayed the newer tool's output instead of its own.
Mint one per-run-unique part id per backend id (call_0:<uuid>) and route
tool_start/output/args/end through a single resolver so all events for a
call resolve the same id. The durable part carries the unique id, so the
finished-card readers derive a collision-free key with no change, and the
awaiting-confirmation path keeps its own synthesized id. Outbound replay
stays paired (the assistant tool_call id and the role=tool result
tool_call_id both come from the part id) and gains unique ids across
turns, which strict providers require.
* Studio: tighten PR comments
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <michaelhan2050@gmail.com>
Trim and tighten code comments and docstrings across the repository. Comment-only: every changed file verified code-identical to main via AST/token comparison.
Trim and tighten code comments and docstrings across studio/ Python. Comment-only: every changed file verified code-identical to main via AST/token comparison.
Raise ruff line-length to 100 and extend the local pre-commit format pipeline (def-signature magic-comma normalization, short multi-line assert collapse, kwarg '=' spacing, blank-line-after-short-import removal, adjacent string-literal / f-string+plain merge, redundant-pass pruning). Every transform re-checks the file AST and is dropped if it would differ; the whole-repo reformat is verified AST-identical per file and idempotent.
* studio: cap training dataset uploads
* studio: clean up failed dataset uploads
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: raise upload limits to 500MB
* studio: make upload limit configurable
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: stream upload routes
* studio: split recipe upload caps
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: tighten upload limit handling
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: import settings router directly
* studio: polish upload cap setting control
* studio: cap settings request bodies
* studio: stub settings route in desktop auth test
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* studio: tighten sandbox blocklist precision (bash, hf upload, NOFILE)
Three precision fixes in core/inference/tools.py. Same security
boundary; fewer false positives that broke legitimate sandbox use.
bash blocklist:
The per-token loop introduced in #5375 fired on any blocklist word in
any token position, so the entirely benign `grep -r curl .`,
`echo source the data`, and `ls /usr/bin/curl` were rejected with
"blocked command 'curl'". The position-anchored regex already covers
real command-position invocations, including `;rm`, `&&wget`, `$(rm)`,
`<(rm)`, backticked subshells, and `/usr/bin/sudo`. The token loop is
re-scoped: it only fires when the previous shlex token is a shell
separator (or at start of line), so split-quoting obfuscations like
`r''m -rf /` are still caught (shlex collapses them to a single
command-position token) while argument-position blocklist words pass
through. Trailing meta-chars glued to a shlex token (`rm;`) are
stripped before basename matching.
hf upload AST gate:
`_method_call_is_hf_upload` previously matched any method named
`upload_file` / `upload_folder` / `upload_large_folder` / `create_commit`
on any receiver, so paramiko.SFTPClient.upload_file, boto3.create_commit,
and similar non-HF SDK methods were rejected. The fallback now requires
an `import huggingface_hub` / `import hf_api` / `from huggingface_hub
import ...` somewhere in the same module. Fully-qualified
huggingface_hub.upload_file(...) calls are unchanged.
NOFILE env knob:
`RLIMIT_NOFILE = (1024, 1024)` was the only sandbox rlimit without an
env override. 1024 is below Linux's typical soft default and below
what multi-shard safetensors mmap chains need on Llama-3 70B-class
loads. Default is now 16384 with UNSLOTH_STUDIO_SANDBOX_NOFILE, parity
with the other rlimits.
15 new bash-blocklist-position tests pin both the false-positive
fixes and the still-blocked invariants (semicolon, &&, subshell,
backtick, split-quote, /usr/bin/ prefix, nested bash -c).
4 new hf-upload-import-gate tests pin both the false-positive
allowances and that HF-imported uses are still blocked.
1 new pin asserts the NOFILE env var is wired.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: cover command wrappers, find -exec, dynamic HF imports, NOFILE clamp
Reviewer follow-ups to the sandbox blocklist precision change.
Command-position scanner missed Bash command-prefix wrappers and inline
shell assignments. shlex tokenised `env curl`, `time curl`, `nohup rm`,
`FOO=bar curl`, `sudo rm`, etc. with the prefix at command position and
the real command at argument position, so the position-anchored check
returned set() while pre-PR's per-token scan caught them. Likewise the
position-anchored regex requires `^` or a shell separator before the
command, so `env curl` slipped through.
Reworked the scanner to track an expect_command flag plus a
prefix_pending flag:
- assignments (FOO=bar) keep expect_command=True for the next token,
- flags ('-oL', '--') keep it intact while prefix_pending is set,
- numeric duration args ('timeout 1 cmd') skip without breaking
expect_command,
- known wrappers (env, command, builtin, exec, time, nohup, nice,
setsid, stdbuf, timeout, ionice, chroot, sudo, doas, su, xargs)
set prefix_pending so the wrapper's command is still checked,
- shell separators now include `{`, `}`, `)`, `then`, `do`,
`else`, `elif` so brace groups and if/then/while/do bodies are
recognised as command positions.
Also lex with `shlex.shlex(punctuation_chars=";&|()`")` so split-quote
forms like `echo done; r''m -rf /tmp/x` and `echo done;r''m` tokenise
as `[..., ';', 'rm', ...]` and the command position check fires.
Added a small `find -exec CMD ... ;` / `-execdir CMD ... ;` pass so
`find . -exec rm -f {} +` and friends are caught even though the
direct token is at argument position to `find`.
Dynamic Hugging Face imports were treated as no-HF-in-scope. The
upload-method gate now also resolves `__import__('huggingface_hub')`,
`importlib.import_module('huggingface_hub')`, and bare
`import_module('huggingface_hub')` (via `from importlib import
import_module`) as HF imports, so HfApi().upload_file via dynamic
import is still blocked.
RLIMIT_NOFILE: setrlimit(NOFILE, (16384, 16384)) silently failed if
the parent's hard cap is below the requested value; the broad
except swallowed the OSError and left the sandbox at the parent's
default. Clamp the requested value to the inherited hard limit
before calling setrlimit.
Test cleanup: the existing test_cat_with_word_source_allowed had
`assert ... or True` so it could not fail; rewrote it to assert the
actual return value plus the two membership checks. Added
parametrised coverage for shell prefix wrappers, find -exec / xargs,
brace groups, if/then, while/do, split-quote command-name forms, and
dynamic HF import upload patterns.
Test:
- python -m pytest studio/backend/tests/test_sandbox_tools.py -q
-> 90 passed (was 67 before this commit)
- full studio/backend/tests/ minus llama_cpp_load_progress_live and
GPU CUDA_VISIBLE_DEVICES tests (pre-existing isolation flake)
-> 1063 passed
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: catch bare-name HF upload calls in AST gate
`from huggingface_hub import upload_file; upload_file(...)` is a
canonical HF call shape that the previous Attribute-only check missed:
the bare-name call lands as ast.Name (not ast.Attribute), so the
fuzzy gate skipped it.
Extend _method_call_is_hf_upload to also match ast.Name when HF is in
scope. Same import-gating discipline as the Attribute branch, so
paramiko/boto3 and locally-defined `def upload_file(...)` helpers
without HF imports still pass.
Pins: 4 new TestHfUploadImportGate cases (upload_file/folder/create_commit
bare-name imports blocked; local upload_file without HF import allowed).
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: scope HF uploads to sandbox-local literals; block env / token leaks
The previous gate dropped every HF upload call. Two refinements make it
precise enough to allow legitimate sandbox->HF uploads while still
catching credential / file exfil:
- path_or_fileobj / folder_path / create_commit operation paths must be
sandbox-local relative-path literals (no '/', '~', drive letter, or
'..' segments). Variable / dynamic paths are rejected.
- Any positional or keyword argument that statically resolves to
os.environ / os.environ.get / os.getenv / bare getenv / subprocess
shape readers is rejected (env-var exfil).
- token / hf_token / api_token / api_key / auth_token / access_token /
password / secret kwargs are always rejected; sandbox env strips all
parent credentials by construction, so any value here is hard-coded
or lifted.
Recursive subtree walk in _reads_env_or_secret catches wrapper shapes
(str(os.environ), json.dumps(os.environ.items()), etc.).
Add TestSandboxEnvIsolation: pin that _build_safe_env builds the env
from a whitelist, not by stripping. Cover Linux/macOS/WSL/Windows
secret shapes. The whitelist is PATH / HOME / TMPDIR / LANG / TERM /
PYTHONIOENCODING (+ VIRTUAL_ENV / SystemRoot when applicable); HOME
points at the sandbox workdir, so HF / wandb / aws SDKs cannot reach
the operator's ~/.cache credentials.
Test classes added:
- TestHfUploadSandboxLocalPaths (relative literals allowed; absolute,
drive-letter, '~', '..', mid-path traversal, dynamic vars, and
open() of unsafe paths blocked, including create_commit recursion).
- TestHfUploadEnvAndSecretLeakBlock (os.environ subscript/get/getenv,
bare getenv, subprocess.check_output, str(os.environ), token=,
hf_token=, api_key=, and create_commit operations referencing env).
- TestSandboxEnvIsolation (no parent secret leaks into sandbox env).
131 tests in test_sandbox_tools.py pass.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* 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>
Leo Borcherding