Studio: make the Cloudflare tunnel opt-in (off by default) - #7046
Conversation
A wildcard bind (`-H 0.0.0.0`) auto-started a public trycloudflare.com tunnel, so exposing Studio on the LAN also published it to the public internet. Flip the default so the tunnel is opt-in. - `--cloudflare` is now tri-state (Optional[bool], default None = off), mirroring the existing --enable-tools/--disable-tools handling. Pass --cloudflare to expose a public HTTPS link for a wildcard bind; --secure still implies the tunnel. - --secure + --no-cloudflare is still rejected as a contradiction. - Update the parent-command guard, re-exec forwarding, startup-banner wording, the colab comment, README, and tests.
The post-install launch hints only mentioned --secure for a public link. Now that the tunnel is opt-in, clarify that -H 0.0.0.0 exposes the raw port on the LAN (not a public URL), and surface --cloudflare as the explicit opt-in for a public HTTPS link (--secure keeps the raw port private). Applied to install.ps1, install.sh, and studio/setup.sh.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request changes the Cloudflare tunnel in Unsloth Studio to be opt-in (off by default) rather than enabled by default. It updates documentation, installation scripts, CLI commands, and tests to reflect this change. The review feedback suggests preserving the cloudflare parameter as an Optional[bool] rather than casting it to a boolean. This would maintain the distinction between the default state (None) and an explicit opt-out (False), allowing the startup banner to dynamically display the correct context ((default) vs (--no-cloudflare)).
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0562510fa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Two review points from the bots: - Gemini: keep `cloudflare` as Optional[bool] in run_server instead of casting None -> False, so the startup banner can distinguish "OFF (default)" (unset) from "OFF (--no-cloudflare)" (explicit). `_cloudflare_flag` and the banner branch now carry the tri-state. - Codex (P1): `unsloth studio run` re-execs the studio venv's console script, which can be an older build whose --cloudflare defaulted on; omitting the flag let it re-enable the tunnel. That path now forwards the default polarity explicitly (--no-cloudflare, or nothing under --secure since --secure implies the tunnel). The plain `unsloth studio` path runs the same-version in-tree run.py (resolved via _find_run_py), so it keeps forwarding only an explicit polarity and still shows the accurate "(default)" banner. Tests updated for the tri-state banner labels, the None gate cases, and the new re-exec forwarding.
|
Pushed 33d5011 addressing both bots:
Added tests for the banner labels, the unset gate cases, and the new re-exec forwarding. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33d5011953
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codex follow-up: _find_run_py falls back to STUDIO_HOME/.../studio/backend/ run.py when the package copy is absent, so the plain `unsloth studio` re-exec can land on an older run.py whose --cloudflare defaults on. Forward the default polarity explicitly there too (--no-cloudflare, or nothing under --secure), matching the run subcommand. The common in-venv launch skips the re-exec and still shows the tri-state "(default)" banner.
|
Fixed in e420677. _find_run_py can fall back to the studio venv's run.py (an older build), so the plain |
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request configures the Cloudflare tunnel in Unsloth Studio to be off by default, changing it to an opt-in feature. The --cloudflare option now defaults to None (off), and users must explicitly provide --cloudflare or --secure to enable it. Updates have been made across documentation, installation scripts, CLI commands, backend logic, and test suites to support this change and prevent accidental tunnel activation in mixed-version environments. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e42067758d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codex P3: the launch hint listed --cloudflare next to the loopback `unsloth studio -p 8888` command, but the tunnel only starts for wildcard binds, so `--cloudflare` alone on 127.0.0.1 does nothing. Show `-H 0.0.0.0 --cloudflare` in the hints (install.ps1, install.sh, studio/setup.sh) and clarify the same in the README.
|
Fixed in e8e80ac. The hint listed --cloudflare next to the loopback |
|
@codex review |
|
/gemini review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
Per-keystroke '*' echo (POSIX termios cbreak / Windows msvcrt.getwch), backspace editing, Ctrl-C abort, EOF handling, confirmation loop with re-prompt on mismatch or policy failure. Pure should_prompt gate for the --secure/--cloudflare exposure paths.
…osure When a launch will start the Cloudflare tunnel (--secure, or --cloudflare on a non-api-only wildcard bind) and the admin account still has its seeded bootstrap password, prompt for a new password in the terminal (masked with '*', confirmed, re-prompting until valid) before any re-exec or server exists. The change is committed in the parent so it never crosses argv or the environment and older studio-venv children see it immediately. Without a terminal, warn and fall back to the backend bootstrap shutdown timer. Mirrors backend update_password semantics in one transaction: rehash, rotate the JWT secret, clear must_change_password, revoke refresh tokens, drop the desktop secret, then remove the stale credential files.
…stop) Never publish a trycloudflare URL while the seeded admin password is active: run_server now runs a terminal password-change gate after the tunnel decision and strictly before start_studio_tunnel. Interactive refusal fails closed (shutdown + exit 1, mirroring the secure gate); without a tty it warns and keeps the bootstrap deadline. Success applies the same effects as the change-password route (update_password + revoke_user_refresh_tokens) and drops the stale app.state.bootstrap_password. MIN_PASSWORD_LENGTH centralised in auth/storage.py and referenced by the HTTP schema. terminal_prompt.py carries the pure gate helper (interactive loop stubbed; supplied by the masked-input module). Also migrates the studio/setup.ps1 launch footer that still showed the bare wildcard hint.
- run.py: run the gate BEFORE the uvicorn socket binds. On a wildcard --cloudflare launch the served HTML injects the bootstrap credential for first login, so a pre-gate listener would hand the default password to anyone who reaches the raw port while the operator is still typing. The gate now also seeds the admin row itself (it can run before lifespan startup). - Headless launches that nothing would protect now fail closed: the bootstrap deadline never arms for api-only serving and UNSLOTH_STUDIO_BOOTSTRAP_TIMEOUT=0 disables it, so warn-and-proceed would have promised a shutdown that never comes. Both the CLI and the backend refuse to publish in that case; the ordinary headless path still warns and relies on the 1h deadline, and no longer auto-fills the default credential into HTML served on a public URL. - storage.update_password gains revoke_refresh_tokens to delete the user's refresh tokens in the SAME transaction as the password commit; the change-password route and the backend gate use it (a separable follow-up delete could fail after the commit and leave a stale refresh token able to mint access tokens under the rotated secret). - clear_bootstrap_password is best-effort: a locked/undeletable file must not surface as a failed password change. - CLI masked reader: disable ISIG like the backend so Ctrl-Z cannot suspend the process with the shared terminal stuck in no-echo mode; handle Ctrl-C/Ctrl-Z as characters; treat stream EOF mid-line as an abort instead of submitting a partial password. Both readers restore terminal attrs from a SIGTERM/SIGHUP handler since a finally block cannot run when a default-disposition signal terminates the process. - Backend reader: decode byte-at-a-time through an incremental UTF-8 decoder so multi-byte characters split across read boundaries are no longer dropped; isatty checks tolerate closed/None streams.
|
Pushed a few additions on top of this branch (thanks for the opt-in work, it made this next step possible): Forced terminal password change before the first public exposure. When a launch will actually start the Cloudflare tunnel (
Also folded in: the missed Tests: 546 passing across the touched CLI and backend suites, including a real pty end-to-end check of the masked echo, both re-prompt paths, and the resulting auth DB state. The change-password route also now revokes refresh tokens atomically with the password commit. |
…nd before the strip Three follow-ups to the pre-exposure hardening: reset-password now deletes auth.db FIRST and proves it is gone before touching the seeded credential files. If the DB cannot be removed (a running Studio or Windows holds it open, or a read-only auth dir) it aborts with the credential files untouched, so a forgotten-password reset is not left half-done with the recovery credentials deleted while an un-resettable must_change_password=1 DB survives. After the DB is gone it invalidates the stale credential files (unlink, else truncate) and fails closed if a file can be neither removed nor truncated, since a surviving plaintext would be re-seeded by generate_bootstrap_password() and re-validate the revoked password. The in-venv (in-process) launch path had no analogue of the re-exec launcher check: a headless public launch would seed the admin and strip the seeded .bootstrap_password in the gate before _load_run_module() later failed on a broken/partial venv, leaving must_change_password=1 with no password to log in. Add _validate_inproc_backend_before_strip, called on the in-venv path (both `unsloth studio` and `unsloth studio run`) before the gate on the headless public path, so a broken backend fails cleanly before anything is stripped. It is scoped to the headless path so an interactive prompt is not delayed behind a full backend import.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6abaa263b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _validate_inproc_backend_before_strip( | ||
| cloudflare = cloudflare, host = host, secure = secure, api_only = api_only | ||
| ) |
There was a problem hiding this comment.
When the installed venv's unsloth is invoked directly, this in-process branch runs and a headless public launch (--secure or wildcard --cloudflare with no TTY) never calls _require_servable_frontend_or_exit before _enforce_password_change_before_exposure. If the frontend dist is missing or --frontend is bad, the gate deletes .bootstrap_password, then run_server() aborts while setting up the frontend, leaving must_change_password=1 with no bootstrap password/login page until reset-password; the re-exec path above already guards this before stripping.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7b8dd41. The in-venv unsloth studio path now calls _require_servable_frontend_or_exit before the gate (cheap frontend check first, backend import after), so a headless public launch with a missing/bad dist aborts before the strip, and the resolved dist is served in-process. See test_studio_default_in_venv_missing_frontend_exits_before_stripping_bootstrap.
| _validate_inproc_backend_before_strip( | ||
| cloudflare = cloudflare, host = host, secure = secure, api_only = api_only | ||
| ) |
There was a problem hiding this comment.
studio run before stripping the bootstrap file
The in-venv studio run path has the same gap: for a headless public launch it only validates that the backend imports, then _enforce_password_change_before_exposure can strip the seeded .bootstrap_password before any frontend validation. With a missing/bad frontend dist, run_server() exits before the UI is available, leaving the seeded admin at must_change_password=1 without a recovery credential; the non-venv path validates the frontend before the strip.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7b8dd41. Same guard added to the in-venv studio run path: it now validates a servable frontend before the strip (in addition to the backend import). See test_run_in_venv_missing_frontend_exits_before_stripping_bootstrap.
| # must have a servable login page BEFORE the gate strips the seeded | ||
| # password, or the re-exec'd child is left with no way to change it | ||
| # (same lockout as `unsloth studio`). Validate here, before the strip. | ||
| _require_servable_frontend_or_exit( |
There was a problem hiding this comment.
For the studio run re-exec path, _require_servable_frontend_or_exit() returns the concrete dist that satisfied the pre-strip check, but this call discards it and later only forwards --frontend when the user supplied one. In a mixed/shadowed install where the parent can find a built dist but the studio-venv child cannot, a headless public launch strips .bootstrap_password, execs the child without that path, and the child aborts during frontend setup, leaving the seeded admin without a login page/recovery credential until reset-password.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7b8dd41. The studio run re-exec now forwards the dist returned by _require_servable_frontend_or_exit (resolved_frontend), not just a user-supplied --frontend, so a shadowed child that cannot self-resolve a dist still serves the parent-validated one. See test_run_reexec_forwards_resolved_frontend_on_public_launch.
| # must_change_password stays set in the DB, so the login page still | ||
| # forces a change and the bootstrap shutdown timer still arms; only | ||
| # the plaintext-on-disk copy of the credential is removed. | ||
| _strip_seeded_bootstrap_password_or_exit(context = "no terminal to change it") |
There was a problem hiding this comment.
In a headless --secure launch with the seeded admin still active, this branch deletes the only plaintext recovery credential before the child has proven that Cloudflare can start; if cloudflared is missing, cannot download, or fails to register, run_server() later exits via the secure fail-closed path while must_change_password is still set and .bootstrap_password is gone. That leaves the user locked out until reset-password even though no public URL was published.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7b8dd41. Added _tunnel_binary_confirmed_unavailable (loads the stdlib-only backend cloudflare_tunnel helper by path and runs ensure_cloudflared). On --secure only (loopback bind, so the tunnel is the only exposure), if cloudflared is provably unavailable the launch is refused with the seeded password preserved instead of stripped. Wildcard --cloudflare binds 0.0.0.0 publicly regardless of the tunnel so it still strips, and any uncertainty (helper not loadable) also still strips, since a possible credential leak outweighs a recoverable lockout. See test_studio_default_secure_tunnel_unavailable_preserves_bootstrap and test_studio_default_wildcard_cloudflare_strips_even_if_tunnel_unavailable.
| _BOOTSTRAP_PW_PATH.write_text("") | ||
| except OSError: | ||
| pass |
There was a problem hiding this comment.
If both unlinking and truncating .bootstrap_password fail (for example a read-only/locked file in a read-only auth dir), this swallows the second error but still prints that the contents were cleared. In that case the stale plaintext remains on disk and can be reused if auth.db is later removed/reset, so the warning should not claim the credential was made unreusable unless write_text("") actually succeeded.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7b8dd41. clear_bootstrap_password now tracks whether write_text("") actually succeeded: if both unlink and truncate fail it warns that the old bootstrap password is still on disk and to remove it manually, instead of claiming the contents were cleared. See test_clear_bootstrap_password_warns_truthfully_when_not_cleared.
…blic path Five follow-ups closing the remaining pre-exposure-strip lockouts: The in-venv (in-process) paths of both `unsloth studio` and `unsloth studio run` validated the backend but not the frontend before the gate, so a headless public launch with a missing/bad dist would strip the seeded .bootstrap_password and then abort in run_server() during frontend setup, leaving must_change_password=1 with no login page. Both now validate a servable frontend before the strip (cheap check first, backend import after) and serve the resolved dist in-process. The `studio run` re-exec discarded the dist that satisfied the pre-strip check and only forwarded a user-supplied --frontend. In a shadowed install where the parent finds a built dist the child cannot, it stripped and exec'd without the path, and the child aborted during frontend setup. It now forwards the resolved dist, matching `unsloth studio`. On a headless --secure launch the bind is loopback, so the Cloudflare tunnel is the only public exposure. If cloudflared is provably unavailable (found nowhere and undownloadable) the tunnel cannot start, so stripping the recovery credential would just lock the user out with no public URL ever served. Add _tunnel_binary_confirmed_unavailable and, on --secure only, refuse the launch with the credential preserved rather than strip. Wildcard --cloudflare binds 0.0.0.0 publicly regardless of the tunnel, so it still strips; any uncertainty (helper not loadable) also still strips, since a possible credential leak outweighs a recoverable lockout. clear_bootstrap_password no longer claims it cleared the file's contents when both unlink and truncate failed; it now reports the stale password is still on disk and asks the user to remove it manually.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b8dd41411
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return False | ||
| module = importlib.util.module_from_spec(spec) | ||
| spec.loader.exec_module(module) | ||
| return module.ensure_cloudflared() is None |
There was a problem hiding this comment.
When a headless first --secure launch runs from the outer CLI, this probe executes before run.py has added studio/backend to sys.path. In that context cloudflare_tunnel.ensure_cloudflared() cannot import its top-level utils.paths.storage_roots fallback when cloudflared is not already on PATH, so it returns None and this line treats the tunnel as provably unavailable. That makes unsloth studio --secure refuse to start even when the Studio cache already contains cloudflared or the child process could download it; add the backend directory to sys.path for this probe or resolve the cache path directly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 239e34c. The probe now adds studio/backend to sys.path (and removes it after) before calling ensure_cloudflared, so _cache_path can import utils.paths.storage_roots and resolve the Studio bin cache exactly as the child will. Without it the cache was unresolvable and the probe false-reported the tunnel unavailable, wrongly refusing --secure. See test_tunnel_probe_adds_backend_to_syspath.
| # user out. Refuse the launch with the credential preserved. (Wildcard | ||
| # --cloudflare binds 0.0.0.0 publicly regardless of the tunnel, so it | ||
| # still strips below; any uncertainty also still strips.) | ||
| if secure and _tunnel_binary_confirmed_unavailable(): |
There was a problem hiding this comment.
For a headless first --secure launch where cloudflared is installed but cannot establish the tunnel (for example blocked outbound connectivity or a Cloudflare outage), this guard is false, so the code deletes .bootstrap_password below and re-execs before any public URL is proven live. The child then fails closed when no Cloudflare URL is produced, leaving must_change_password=1 with no recovery password or UI until reset-password; only strip the bootstrap credential after the secure tunnel is actually usable, or refuse before stripping.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 239e34c. The strip is only needed when the re-exec child is an OLD studio-venv backend with no pre-bind suppression. This install own run.py sets app.state.suppress_bootstrap_injection before binding and never serves the seeded credential publicly, so added _child_self_suppresses (true in-process, or when the re-exec target is this install run.py by path identity) and skip the strip in that case, preserving .bootstrap_password as a local recovery credential. The strip stays fully in force for the studio-venv console-script path and any venv-fallback run.py, where an old child is actually possible. A missing-cloudflared --secure launch still refuses (preserving recovery), and wildcard --cloudflare still strips. Verified leak-safe: run.py loads all suppression-path modules via sys.path.insert(0, backend_dir), so a venv copy cannot shadow them. See test_studio_default_reexec_outer_runpy_keeps_bootstrap_for_local_recovery.
… self-suppressing child Two follow-ups to the --secure pre-exposure hardening: The cloudflared availability probe loaded cloudflare_tunnel by file path but not its backend deps: ensure_cloudflared() -> _cache_path() lazily imports utils.paths.storage_roots, which only resolves when studio/backend is on sys.path. From the outer CLI it is not, so the probe saw ensure_cloudflared() return None (cache unresolvable) and wrongly treated the tunnel as unavailable, refusing --secure even when cloudflared was cached or downloadable. Add the backend dir to sys.path for the probe (and remove it after) so the cache path resolves as it will in the child. A headless --secure launch stripped the seeded .bootstrap_password before the child proved the tunnel could actually connect, so a cloudflared that is present but cannot establish the tunnel (blocked connectivity, Cloudflare outage) left must_change_password=1 with no recovery credential. But the strip is only needed when the re-exec'd child is an OLD studio-venv backend with no pre-bind suppression: this install's own run.py sets app.state.suppress_bootstrap_injection before binding and never serves the seeded credential publicly. Add _child_self_suppresses (true in-process, or when the re-exec target is this install's own run.py by path identity) and skip the strip in that case, keeping .bootstrap_password as a local recovery credential; the strip stays fully in force for the studio-venv console-script path and any venv-fallback run.py, where an old child is actually possible.
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Ran a quick visual pass over what this PR does now, from a local Studio launch under a dedicated Cloudflare tunnel is opt-in (off by default)
Starting with no flags stays local (loopback) and prints no public URL: Mode 1 -- public launch (
|
|
I tested this on a Linux box with CUDA GPUs. What I exercised:
The gate itself is sound. Two small things I'd change here. 1.
|
…ompt wording
- --secure/--secure run: emit a Note (not an error) when -H is a non-loopback
host, since --secure forces the loopback bind and would otherwise discard -H
silently.
- Reword the pre-exposure terminal prompt to 'exposed on the public internet'
in both the backend gate and the CLI mirror.
- Align the CLI success line with the backend ("Password updated for '<user>'.").
- Tests for the new -H warning (present when overridden, absent on loopback).
|
Thanks for the thorough test pass on real hardware, and for confirming the tunnel login does not re-prompt. Pushed all three:
Head is now b6dbf72. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6dbf72738
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (STUDIO_HOME / "auth" / stale).unlink(missing_ok = True) | ||
| except OSError: | ||
| typer.echo( | ||
| f"Warning: could not remove stale {stale} file; the credential " | ||
| "in it is no longer valid, but consider deleting it manually.", |
There was a problem hiding this comment.
When the pre-exposure prompt changes the password on a machine where .bootstrap_password cannot be unlinked but is still writable (for example, a locked file or a directory that denies deletes), this handler only warns and leaves the old plaintext in place. If auth.db is later reset or removed while that file remains, storage.generate_bootstrap_password() can read it back and make the previously revoked bootstrap password valid again; the backend clear_bootstrap_password() added in this commit truncates on unlink failure specifically to prevent that, so the CLI mirror should do the same for BOOTSTRAP_PASSWORD_FILE.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, fixed in c092ef8. _cli_update_password now truncates the file on unlink failure so the stale plaintext cannot be re-seeded by generate_bootstrap_password() after a later reset, matching the backend clear_bootstrap_password(). The change is already committed so it never rolls back, and the warning now states truthfully whether the contents were cleared or the file must be removed manually. Added a regression test.
Headless hosts (CI, containers, systemd units) have no TTY, so the forced first-exposure password change could not be completed unattended. Add a non-interactive way to set the INITIAL admin password before the server binds: - --password <value>, the UNSLOTH_STUDIO_PASSWORD env var, or --password - (read one line from stdin). Off by default; unset falls back to the normal interactive terminal prompt / browser setup. - Applies on any launch (public --secure/--cloudflare or a headless -H 0.0.0.0 bind), only when the account still has its seeded bootstrap password. An already-set password is a hard error, never an override; an invalid value (too short, or equal to the bootstrap) fails closed before bind. - The CLI applies the change in the parent, never forwards --password to the re-exec child, and strips UNSLOTH_STUDIO_PASSWORD from the child env so the secret never crosses to the child. run.py does the same on the direct path and strips the env var so spawned subprocesses (cloudflared, llama-server, tools) cannot inherit it. Mirrors resolve_supplied_password across the CLI and backend, documents the option in the README (including the argv-visibility caveat), and covers all flows (env/stdin/literal, fail-closed cases, no-forward, env-strip, reset-password roundtrip) in the CLI, backend, and unit suites.
|
@codex review |
1 similar comment
|
@codex review |
…password change The post-change cleanup in _cli_update_password only warned when .bootstrap_password could not be unlinked but was still writable (locked file, read-only auth dir), leaving the old plaintext on disk. If auth.db is later reset or removed, generate_bootstrap_password() reads that file back and re-validates the revoked bootstrap password. Truncate the file on unlink failure so its stale plaintext cannot be re-seeded, mirroring the backend clear_bootstrap_password(); the password change is already committed, so this never rolls it back. The warning now states truthfully whether the contents were cleared or the file must be removed manually.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
3 participants
What
Make the Cloudflare tunnel opt-in. Today a wildcard bind (
-H 0.0.0.0)auto-starts a public
trycloudflare.comtunnel, so exposing Studio on theLAN also publishes it to the public internet. This flips the default so the
tunnel only starts when explicitly requested.
Why change the default
-H 0.0.0.0and "publish to the public internet" are two different intentions,and the old default coupled them silently. Binding a wildcard host is a
LAN-scoped decision (let other machines on my network reach Studio), but it also
started a
trycloudflare.comtunnel and put a live publichttps://URL infront of Studio, reachable by anyone with the link.
That matters more than a typical UI exposure because Studio's server-side tools
(Python / terminal execution) are on by default: the auto-public URL is a remote
code-execution surface, gated only by the API key, which is easy to leak
(terminal banner, logs, screenshots, shell history). Opt-out is the wrong
polarity for a public-internet, code-executing exposure; it should be opt-in.
Behavior change
unsloth studio -H 0.0.0.0now binds the raw port only (LAN / any reachableinterface). It no longer creates a public URL.
--cloudflareopts in to the publichttps://*.trycloudflare.comlink for awildcard bind.
--secureis unchanged: it still serves only through the tunnel (loopbackbind, fails closed) and implies the tunnel.
Original reachability goal is preserved
The tunnel was added in #6204 to solve a real problem: a raw
0.0.0.0:<port>bind on a cloud box is often unreachable (HTTPS-vs-HTTP, blocked high ports,
closed security groups). This PR keeps that escape hatch, just behind an explicit
--cloudflare(or--secure), so remote reachability still works with one flag.It only removes the implicit public exposure. The later
--securemode (#6300)already moved toward explicit-tunnel thinking, so this is in line with that
direction.
Implementation
--cloudflareis now tri-state (Optional[bool], defaultNone= off) ratherthan a plain default-
Falsebool. That lets--secureimply the tunnel(
None-> on) while still rejecting the genuine contradiction--secure --no-cloudflare(False-> error); a two-state default couldn't tell"unset" from "explicitly off". This mirrors the existing
--enable-tools/--disable-toolshandling in the same module.Updated to match: the
studioandstudio runCLI options,run.py(
run_server+ argparse + startup-banner wording), the parent-command flagguard, re-exec forwarding, the Colab helper comment, the README remote-access
section, the installer/setup launch hints (
install.ps1,install.sh,studio/setup.sh), and the affected tests.Wording note:
-H 0.0.0.0is still described as network/LAN-exposed, not"private". It just no longer creates a public URL unless requested.
Tests
test_studio_cloudflare_flag.py,test_secure_tunnel_gate.py,test_cloudflare_tunnel.pyupdated for the new default and re-expectationedfor the tri-state.
unsloth_cli/testsis green apart from 3pre-existing Windows-only
start_new_sessionfailures unrelated to this change.