From 9f694ab750e90bdd55c261f3c0e9a895b6620db4 Mon Sep 17 00:00:00 2001 From: Anmol Mishra Date: Mon, 15 Jun 2026 15:02:10 +0530 Subject: [PATCH 01/91] fix(studio): Windows GGUF cancel hang + CPU spinlock overhead (#5692) (#5749) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(studio): Windows GGUF cancel hang + CPU spinlock overhead (#5692) Two fixes for Windows-native GGUF inference via llama-server: **Issue 1 — GPU/CUDA Hang on Stream Cancellation:** - Add `Connection: close` header to all httpx requests proxying to llama-server, preventing Keep-Alive from masking downstream socket closure. - Introduce `_await_disconnect_then_close` background watcher that polls `request.is_disconnected()` every 100ms and calls `resp.aclose()` immediately when the client disconnects. This runs alongside the existing cancel-POST watcher and covers client aborts that never reach the /cancel endpoint (tab close, proxy aborts, Colab, mobile navigation, etc.). - Change all StreamingResponse `Connection: keep-alive` headers to `Connection: close`. **Issue 2 — High CPU Spinlock & KV Cache Backup Overhead:** - Set OMP_WAIT_POLICY=PASSIVE and OMP_NUM_THREADS=2 in the llama-server subprocess environment on Windows to prevent OpenMP from spin-waiting on all logical cores while the GPU decodes. - Limit `--threads` to 2 on Windows when the model is fully GPU-offloaded (`-ngl -1`). Auto-detect otherwise. - Pass `--cache-ram 0 --ctx-checkpoints 0 --no-cache-prompt --checkpoint-every-n-tokens -1` on Windows to disable prompt-cache snapshots that copy KV cache to system RAM over the WDDM/PCI-E bus. Closes #5692. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix: use local import to avoid ruff F823 (sys used before assignment) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * review: address gemini review feedback - Simplify _fully_gpu_offloaded init: default to False, only set True in the gpu_indices branch, drop redundant else. - Log exceptions in _await_disconnect_then_close at debug level instead of silent pass, per review suggestion. * Adjust review feedback for PR #5749 - _await_disconnect_then_close: set cancel_event before resp.aclose() so the streamer's RemoteProtocolError handler treats the watcher-driven close as cancellation, not an upstream error. Both call sites pass cancel_event through. - Windows --cache-ram / --no-cache-prompt / --ctx-checkpoints block: gate on _fully_gpu_offloaded so CPU and partial-offload Windows runs keep prompt-cache reuse across turns. - Windows OMP_WAIT_POLICY / OMP_NUM_THREADS env: same gate so CPU and partial-offload Windows runs keep default OpenMP parallelism. * Shorten code comments touched by PR #5749 * Clean up local imports and rename underscore locals in PR #5749 - Drop the function-local `import sys as _sys` introduced as an F823 workaround; remove the redundant in-function `import os`/`import sys` block so module-level imports resolve sys/os instead. F823 no longer triggers because no shadowing import remains inside load_model. - Rename `_fully_gpu_offloaded` and `_t` to `fully_gpu_offloaded` and `threads_arg`. Underscore-prefixed names usually mean private/module- level; plain locals match Python style for in-function temporaries. No behavior change. ruff clean, py_compile clean, 35 studio cancel- infra tests + 13 launch-gating AST locks + 6 disconnect-watcher locks + 4 spoof live-import tests all pass. * Fix Windows GGUF follow-ups for PR #5749 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix cache flag gating for PR #5749 * Fix Python 3.9 annotations for PR #5749 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Anmol Mishra Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han Co-authored-by: wasimysaid Co-authored-by: Lee Jackson <130007945+Imagineer99@users.noreply.github.com> --- studio/backend/core/inference/llama_cpp.py | 102 ++++++++++--- studio/backend/routes/inference.py | 137 ++++++++++++++---- .../tests/test_llama_cpp_mtp_detection.py | 73 ++++++++++ .../test_stream_cancel_registration_timing.py | 34 +++++ 4 files changed, 303 insertions(+), 43 deletions(-) diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index 57cc97f3b5..8657179b73 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -23,7 +23,7 @@ import sys import threading import time from pathlib import Path -from typing import Callable, Generator, Iterable, List, Optional +from typing import Callable, Collection, Generator, Iterable, List, Optional, Union import httpx @@ -567,14 +567,27 @@ def _auto_mode_drops_mtp( def _extra_args_set_spec_type(extra_args: Optional[Iterable[str]]) -> bool: """User passed --spec-type / --spec-default? llama-server takes one --spec-type (comma-separated to chain), so suppress auto-emit.""" + return _extra_args_set_any_flag(extra_args, {"--spec-type", "--spec-default"}) + + +_GPU_OFFLOAD_OVERRIDE_FLAGS = frozenset({"-ngl", "--gpu-layers", "--n-gpu-layers", "-fit", "--fit"}) +_THREAD_OVERRIDE_FLAGS = frozenset({"-t", "--threads"}) + + +def _extra_arg_flag_name(token: str) -> Optional[str]: + if not token.startswith("-") or token in {"-", "--"}: + return None + if len(token) >= 2 and (token[1].isdigit() or token[1] == "."): + return None + return token.split("=", 1)[0] + + +def _extra_args_set_any_flag(extra_args: Optional[Iterable[str]], flags: Collection[str]) -> bool: if not extra_args: return False for raw in extra_args: - tok = str(raw) - if not tok.startswith("--"): - continue - flag = tok.split("=", 1)[0] - if flag in ("--spec-type", "--spec-default"): + flag = _extra_arg_flag_name(str(raw)) + if flag in flags: return True return False @@ -1217,7 +1230,7 @@ class LlamaCppBackend: def probe_server_capabilities(cls, binary: Optional[str] = None) -> dict[str, object]: """Parse `llama-server --help` for feature flags. Returns {found, mtp_token, supports_mtp, ngram_mod_flavor, - supports_ngram_mod, spec_draft_n_max_flag}. + supports_ngram_mod, spec_draft_n_max_flag, cache flag support}. ``ngram_mod_flavor``: ``"new"`` when the post-rename ``--spec-ngram-mod-n-match / -n-min / -n-max`` are real args; @@ -1242,6 +1255,9 @@ class LlamaCppBackend: "spec_draft_n_max_flag": None, "supports_kv_unified": False, "supports_fit_ctx": False, + "supports_cache_ram": False, + "supports_ctx_checkpoints": False, + "supports_no_cache_prompt": False, } try: mtime = int(Path(bin_path).stat().st_mtime) @@ -1257,6 +1273,9 @@ class LlamaCppBackend: spec_draft_n_max_flag: Optional[str] = None supports_kv_unified = False supports_fit_ctx = False + supports_cache_ram = False + supports_ctx_checkpoints = False + supports_no_cache_prompt = False try: result = subprocess.run( [bin_path, "--help"], @@ -1347,6 +1366,9 @@ class LlamaCppBackend: supports_kv_unified = _is_real("--kv-unified") supports_fit_ctx = _is_real("--fit-ctx") + supports_cache_ram = _is_real("--cache-ram") + supports_ctx_checkpoints = _is_real("--ctx-checkpoints") + supports_no_cache_prompt = _is_real("--no-cache-prompt") except (OSError, subprocess.SubprocessError) as exc: logger.debug(f"llama-server --help probe failed: {exc}") @@ -1359,6 +1381,9 @@ class LlamaCppBackend: "spec_draft_n_max_flag": spec_draft_n_max_flag, "supports_kv_unified": supports_kv_unified, "supports_fit_ctx": supports_fit_ctx, + "supports_cache_ram": supports_cache_ram, + "supports_ctx_checkpoints": supports_ctx_checkpoints, + "supports_no_cache_prompt": supports_no_cache_prompt, } cls._capability_cache[cache_key] = info return info @@ -3924,27 +3949,43 @@ class LlamaCppBackend: "--no-context-shift", ] + fully_gpu_offloaded = False if use_fit: cmd.extend(["--fit", "on"]) elif gpu_indices is not None: # Fits on selected GPU(s) -- offload all layers cmd.extend(["-ngl", "-1"]) + fully_gpu_offloaded = True + server_caps = self.probe_server_capabilities(binary) cmd.extend( self._ctx_integrity_flags( n_parallel, use_fit, requested_ctx, effective_ctx, - self.probe_server_capabilities(binary), + server_caps, ) ) + offload_overridden = _extra_args_set_any_flag( + extra_args, _GPU_OFFLOAD_OVERRIDE_FLAGS + ) + threads_overridden = _extra_args_set_any_flag(extra_args, _THREAD_OVERRIDE_FLAGS) + full_offload_tuning_active = fully_gpu_offloaded and not offload_overridden - # -1 = llama.cpp auto-detect (physical cores). Pass explicitly - # so we don't inherit llama-server's internal default, which - # has varied (hardware concurrency incl. hyperthreads on some - # builds). - cmd.extend(["--threads", str(n_threads if n_threads is not None else -1)]) + # Pass --threads explicitly so we do not inherit llama-server + # defaults. Windows + full offload caps at 2 to stop OpenMP + # spin-wait burning CPU during GPU decode. User pass-through + # offload/thread flags keep last-wins semantics. #5692. + if ( + sys.platform == "win32" + and full_offload_tuning_active + and not threads_overridden + ): + threads_arg = 2 + else: + threads_arg = n_threads if n_threads is not None else -1 + cmd.extend(["--threads", str(threads_arg)]) # Enable Jinja chat template rendering cmd.extend(["--jinja"]) @@ -4086,6 +4127,28 @@ class LlamaCppBackend: else: self._api_key = None + # Windows + full offload: disable KV checkpoints (WDDM/PCI-E + # overhead). CPU/partial offload keeps prompt caching. #5692. + if sys.platform == "win32" and full_offload_tuning_active: + unsupported_cache_flags: list[str] = [] + if server_caps.get("supports_cache_ram"): + cmd.extend(["--cache-ram", "0"]) + else: + unsupported_cache_flags.append("--cache-ram") + if server_caps.get("supports_ctx_checkpoints"): + cmd.extend(["--ctx-checkpoints", "0"]) + else: + unsupported_cache_flags.append("--ctx-checkpoints") + if server_caps.get("supports_no_cache_prompt"): + cmd.append("--no-cache-prompt") + else: + unsupported_cache_flags.append("--no-cache-prompt") + if unsupported_cache_flags: + logger.info( + "Skipping unsupported Windows cache flags for llama-server: %s", + ", ".join(unsupported_cache_flags), + ) + # User pass-through args go last so llama.cpp's last-wins parsing # lets the user override Studio's auto-set flags. Already # validated by the route via validate_extra_args(). @@ -4101,9 +4164,6 @@ class LlamaCppBackend: logger.info(f"Starting llama-server: {' '.join(_log_cmd)}") # Library paths so llama-server finds its shared libs and CUDA DLLs. - import os - import sys - env = child_env_without_native_path_secret() binary_dir = str(Path(binary).parent) @@ -4131,6 +4191,14 @@ class LlamaCppBackend: existing_path = env.get("PATH", "") env["PATH"] = ";".join(path_dirs) + ";" + existing_path + # Windows + full offload: PASSIVE OMP + 2 threads stop + # spin-wait burning CPU. CPU/partial offload keeps + # default OMP parallelism. #5692. + if full_offload_tuning_active: + env.setdefault("OMP_WAIT_POLICY", "PASSIVE") + if not threads_overridden: + env.setdefault("OMP_NUM_THREADS", "2") + # ROCm: the prebuilt bundles rocblas.dll but NOT the Tensile # kernel files (rocblas/library/*.dat + *.hsaco); the DLL # searches /rocblas/library/ which doesn't exist @@ -5512,7 +5580,7 @@ class LlamaCppBackend: reasoning_effort: Optional[str] = None, preserve_thinking: Optional[bool] = None, seed: Optional[int] = None, - ) -> Generator[str | dict, None, None]: + ) -> Generator[Union[str, dict], None, None]: """ Send a chat completion to llama-server and stream tokens back. diff --git a/studio/backend/routes/inference.py b/studio/backend/routes/inference.py index 2f9ed35936..3e13774639 100644 --- a/studio/backend/routes/inference.py +++ b/studio/backend/routes/inference.py @@ -1194,6 +1194,23 @@ async def _await_cancel_then_close(cancel_event, resp) -> None: return +async def _await_disconnect_then_close(request, resp, cancel_event) -> None: + """Close ``resp`` on client disconnect; sets ``cancel_event`` first so + the streamer's RemoteProtocolError handler treats it as cancellation. + Catches aborts the in-loop /cancel check misses during prefill. #5692. + """ + try: + while not await request.is_disconnected(): + await asyncio.sleep(0.1) + cancel_event.set() + try: + await resp.aclose() + except Exception as e: + logger.debug("Failed to close response on disconnect: %s", e) + except asyncio.CancelledError: + return + + # Centralized local/server tool nudge. Keep render_html guidance gated to turns # where the artifact tool is actually present in the tool schema; otherwise # small local models can hallucinate a missing tool call instead of following @@ -2302,7 +2319,7 @@ async def generate_stream( media_type = "text/event-stream", headers = { "Cache-Control": "no-cache", - "Connection": "keep-alive", + "Connection": "close", }, ) @@ -3696,7 +3713,7 @@ async def openai_chat_completions( media_type = "text/event-stream", headers = { "Cache-Control": "no-cache", - "Connection": "keep-alive", + "Connection": "close", "X-Accel-Buffering": "no", }, ) @@ -4117,7 +4134,7 @@ async def openai_chat_completions( media_type = "text/event-stream", headers = { "Cache-Control": "no-cache", - "Connection": "keep-alive", + "Connection": "close", "X-Accel-Buffering": "no", }, ) @@ -4260,7 +4277,7 @@ async def openai_chat_completions( media_type = "text/event-stream", headers = { "Cache-Control": "no-cache", - "Connection": "keep-alive", + "Connection": "close", "X-Accel-Buffering": "no", }, ) @@ -4664,7 +4681,7 @@ async def openai_chat_completions( media_type = "text/event-stream", headers = { "Cache-Control": "no-cache", - "Connection": "keep-alive", + "Connection": "close", "X-Accel-Buffering": "no", }, ) @@ -4865,7 +4882,7 @@ async def openai_chat_completions( media_type = "text/event-stream", headers = { "Cache-Control": "no-cache", - "Connection": "keep-alive", + "Connection": "close", "X-Accel-Buffering": "no", }, ) @@ -5120,8 +5137,12 @@ async def openai_completions(request: Request, current_subject: str = Depends(ge client = httpx.AsyncClient(timeout = _llama_streaming_generation_timeout()) resp = None bytes_iter = None + disconnect_event = threading.Event() + disconnect_watcher = None try: - req = client.build_request("POST", target_url, json = body) + req = client.build_request( + "POST", target_url, json = body, headers = {"Connection": "close"} + ) first_token_deadline = time.monotonic() + _DEFAULT_FIRST_TOKEN_TIMEOUT_S resp = await _send_stream_with_preheader_cancel(client, req, request = request) if resp is None: @@ -5130,10 +5151,14 @@ async def openai_completions(request: Request, current_subject: str = Depends(ge err_bytes = await resp.aread() err_text = err_bytes.decode("utf-8", errors = "replace") raise RuntimeError(f"llama-server returned {resp.status_code}: {err_text}") + disconnect_watcher = asyncio.create_task( + _await_disconnect_then_close(request, resp, disconnect_event) + ) bytes_iter = resp.aiter_bytes() buffer = b"" async for chunk in _aiter_llama_stream_items( bytes_iter, + cancel_event = disconnect_event, request = request, first_token_deadline = first_token_deadline, response = resp, @@ -5144,19 +5169,33 @@ async def openai_completions(request: Request, current_subject: str = Depends(ge out = _cmpl_stream_event_out(event, _include_usage) if out is not None: yield out + b"\n\n" - if buffer: + if not disconnect_event.is_set() and buffer: out = _cmpl_stream_event_out(buffer, _include_usage) if out is not None: # Re-add the SSE separator the split consumed, so a final # event arriving without a trailing blank line is still # terminated for the client's parser. yield out + b"\n\n" + except (httpx.RemoteProtocolError, httpx.ReadError, httpx.CloseError) as e: + if not disconnect_event.is_set(): + logger.error("openai_completions stream error: %s", e) + error_chunk = _openai_stream_error_chunk(e) + yield f"data: {json.dumps(error_chunk)}\n\n".encode("utf-8") + return except Exception as e: + if disconnect_event.is_set(): + return logger.error("openai_completions stream error: %s", e) error_chunk = _openai_stream_error_chunk(e) yield f"data: {json.dumps(error_chunk)}\n\n".encode("utf-8") return finally: + if disconnect_watcher is not None: + disconnect_watcher.cancel() + try: + await disconnect_watcher + except (asyncio.CancelledError, Exception): + pass if bytes_iter is not None: try: await bytes_iter.aclose() @@ -6063,8 +6102,12 @@ async def _responses_stream( client = httpx.AsyncClient(timeout = _llama_streaming_generation_timeout()) resp = None lines_iter = None + disconnect_event = threading.Event() + disconnect_watcher = None try: - req = client.build_request("POST", target_url, json = body) + req = client.build_request( + "POST", target_url, json = body, headers = {"Connection": "close"} + ) first_token_deadline = time.monotonic() + _DEFAULT_FIRST_TOKEN_TIMEOUT_S try: resp = await _send_stream_with_preheader_cancel(client, req, request = request) @@ -6117,9 +6160,13 @@ async def _responses_stream( ) return + disconnect_watcher = asyncio.create_task( + _await_disconnect_then_close(request, resp, disconnect_event) + ) lines_iter = resp.aiter_lines() async for raw_line in _aiter_llama_stream_items( lines_iter, + cancel_event = disconnect_event, request = request, first_token_deadline = first_token_deadline, response = resp, @@ -6240,7 +6287,18 @@ async def _responses_stream( if usage: input_tokens = usage.get("prompt_tokens", input_tokens) output_tokens = usage.get("completion_tokens", output_tokens) + except (httpx.RemoteProtocolError, httpx.ReadError, httpx.CloseError) as e: + if not disconnect_event.is_set(): + logger.error("responses stream error: %s", e) + status_code = 400 if _classify_llama_generation_error(e) is not None else 500 + yield _sse( + "response.failed", + _failed_response_payload(e, status_code), + ) + return except Exception as e: + if disconnect_event.is_set(): + return logger.error("responses stream error: %s", e) status_code = 400 if _classify_llama_generation_error(e) is not None else 500 yield _sse( @@ -6249,6 +6307,12 @@ async def _responses_stream( ) return finally: + if disconnect_watcher is not None: + disconnect_watcher.cancel() + try: + await disconnect_watcher + except (asyncio.CancelledError, Exception): + pass if lines_iter is not None: try: await lines_iter.aclose() @@ -6264,6 +6328,9 @@ async def _responses_stream( except Exception: pass + if disconnect_event.is_set(): + return + final_reasoning, final_visible = extractor.finish() if final_reasoning: for event in _ensure_reasoning_open(): @@ -6470,7 +6537,7 @@ async def _responses_stream( media_type = "text/event-stream", headers = { "Cache-Control": "no-cache", - "Connection": "keep-alive", + "Connection": "close", "X-Accel-Buffering": "no", }, ) @@ -7071,7 +7138,7 @@ async def _anthropic_tool_stream( media_type = "text/event-stream", headers = { "Cache-Control": "no-cache", - "Connection": "keep-alive", + "Connection": "close", "X-Accel-Buffering": "no", }, ) @@ -7138,7 +7205,7 @@ async def _anthropic_plain_stream( media_type = "text/event-stream", headers = { "Cache-Control": "no-cache", - "Connection": "keep-alive", + "Connection": "close", "X-Accel-Buffering": "no", }, ) @@ -7472,8 +7539,11 @@ async def _anthropic_passthrough_stream( resp = None lines_iter = None cancel_watcher = None + disconnect_watcher = None try: - req = client.build_request("POST", target_url, json = body) + req = client.build_request( + "POST", target_url, json = body, headers = {"Connection": "close"} + ) first_token_deadline = time.monotonic() + _DEFAULT_FIRST_TOKEN_TIMEOUT_S resp = await _send_stream_with_preheader_cancel( client, req, cancel_event, request = request @@ -7502,11 +7572,12 @@ async def _anthropic_passthrough_stream( ) return - # See _openai_passthrough_stream for rationale: aiter_lines() - # blocks during llama-server prefill, so the in-loop cancel - # check is unreachable until the first SSE chunk arrives. - # The watcher closes `resp` on cancel, raising in aiter_lines. + # Watchers unblock aiter_lines() during prefill, before in-loop + # cancel/disconnect checks can run. cancel_watcher = asyncio.create_task(_await_cancel_then_close(cancel_event, resp)) + disconnect_watcher = asyncio.create_task( + _await_disconnect_then_close(request, resp, cancel_event) + ) lines_iter = resp.aiter_lines() async for raw_line in _aiter_llama_stream_items( lines_iter, @@ -7555,6 +7626,12 @@ async def _anthropic_passthrough_stream( await cancel_watcher except (asyncio.CancelledError, Exception): pass + if disconnect_watcher is not None: + disconnect_watcher.cancel() + try: + await disconnect_watcher + except (asyncio.CancelledError, Exception): + pass if lines_iter is not None: try: await lines_iter.aclose() @@ -7579,7 +7656,7 @@ async def _anthropic_passthrough_stream( media_type = "text/event-stream", headers = { "Cache-Control": "no-cache", - "Connection": "keep-alive", + "Connection": "close", "X-Accel-Buffering": "no", }, ) @@ -7994,7 +8071,9 @@ async def _openai_passthrough_stream( ) while True: try: - req = client.build_request("POST", target_url, json = body) + req = client.build_request( + "POST", target_url, json = body, headers = {"Connection": "close"} + ) first_token_deadline = time.monotonic() + _DEFAULT_FIRST_TOKEN_TIMEOUT_S resp = await _send_stream_with_preheader_cancel( client, req, cancel_event, request = request @@ -8064,13 +8143,12 @@ async def _openai_passthrough_stream( # save resp.aiter_lines() so the finally block can aclose() it on # our task. See that function for full rationale. lines_iter = None - # During llama-server prefill, `aiter_lines()` blocks until the - # first SSE chunk arrives. The in-loop `cancel_event` check can't - # fire until then -- the exact proxy/Colab scenario the cancel POST - # recovers from. Run a tiny watcher that closes `resp` as soon as - # cancel fires, unblocking the iterator with a RemoteProtocolError - # caught in the except clause below. + # Watchers unblock aiter_lines() during prefill, before in-loop + # cancel/disconnect checks can run. cancel_watcher = asyncio.create_task(_await_cancel_then_close(cancel_event, resp)) + disconnect_watcher = asyncio.create_task( + _await_disconnect_then_close(request, resp, cancel_event) + ) try: lines_iter = resp.aiter_lines() async for raw_line in _aiter_llama_stream_items( @@ -8101,6 +8179,8 @@ async def _openai_passthrough_stream( if not cancel_event.is_set(): raise except Exception as e: + if cancel_event.is_set(): + return # 200 headers already flushed; errors must go in the SSE body. logger.error("openai passthrough stream error: %s", e) err = _openai_stream_error_chunk(e) @@ -8111,6 +8191,11 @@ async def _openai_passthrough_stream( await cancel_watcher except (asyncio.CancelledError, Exception): pass + disconnect_watcher.cancel() + try: + await disconnect_watcher + except (asyncio.CancelledError, Exception): + pass if lines_iter is not None: try: await lines_iter.aclose() @@ -8131,7 +8216,7 @@ async def _openai_passthrough_stream( media_type = "text/event-stream", headers = { "Cache-Control": "no-cache", - "Connection": "keep-alive", + "Connection": "close", "X-Accel-Buffering": "no", }, ) diff --git a/studio/backend/tests/test_llama_cpp_mtp_detection.py b/studio/backend/tests/test_llama_cpp_mtp_detection.py index b00cd7169e..2cc0fb4184 100644 --- a/studio/backend/tests/test_llama_cpp_mtp_detection.py +++ b/studio/backend/tests/test_llama_cpp_mtp_detection.py @@ -9,6 +9,7 @@ the _already_in_target_state mirror that prevents needless reloads. from __future__ import annotations +import inspect import struct import sys import types as _types @@ -52,9 +53,12 @@ import pytest from core.inference.llama_cpp import ( LlamaCppBackend, + _GPU_OFFLOAD_OVERRIDE_FLAGS, + _THREAD_OVERRIDE_FLAGS, _backfill_usage_from_timings, _build_ngram_mod_flags, _canonicalize_spec_mode, + _extra_args_set_any_flag, _extra_args_set_spec_type, _is_mtp_model_name, ) @@ -315,6 +319,46 @@ def test_extra_args_set_spec_type_passes_on_non_spec_type_args(extra_args): assert _extra_args_set_spec_type(extra_args) is False +@pytest.mark.parametrize( + "extra_args", + [ + ["-ngl", "12"], + ["--gpu-layers", "12"], + ["--n-gpu-layers=12"], + ["-fit", "off"], + ["--fit=off"], + ], +) +def test_extra_args_detect_gpu_offload_overrides(extra_args): + assert _extra_args_set_any_flag(extra_args, _GPU_OFFLOAD_OVERRIDE_FLAGS) is True + + +@pytest.mark.parametrize("extra_args", [["-t", "8"], ["--threads=8"]]) +def test_extra_args_detect_thread_overrides(extra_args): + assert _extra_args_set_any_flag(extra_args, _THREAD_OVERRIDE_FLAGS) is True + + +def test_windows_full_offload_flags_use_current_llama_server_args(): + src = inspect.getsource(LlamaCppBackend.load_model) + stale_checkpoint_flag = "--checkpoint-" + "every-n-tokens" + assert '"--cache-ram"' in src + assert '"--ctx-checkpoints"' in src + assert '"--no-cache-prompt"' in src + assert stale_checkpoint_flag not in src + + +def test_load_model_sets_threads_once(): + src = inspect.getsource(LlamaCppBackend.load_model) + assert src.count('cmd.extend(["--threads", str(') == 1 + + +def test_llama_cpp_annotations_stay_python39_safe(): + src = inspect.getsource(LlamaCppBackend.generate_chat_completion) + helper_src = inspect.getsource(_extra_args_set_any_flag) + assert "Generator[str | dict" not in src + assert "set[str] | frozenset[str]" not in helper_src + + def test_already_in_target_state_user_spec_type_override_matches_clean_backend(): # User --spec-type none suppressed auto-MTP; repeat /load must not re-promote. backend = _mtp_backend( @@ -554,6 +598,9 @@ def test_probe_server_capabilities_handles_missing_binary(): caps = LlamaCppBackend.probe_server_capabilities("/no/such/llama-server") assert caps["found"] is False assert caps["supports_mtp"] is False + assert caps["supports_cache_ram"] is False + assert caps["supports_ctx_checkpoints"] is False + assert caps["supports_no_cache_prompt"] is False # ngram-mod flag flavor detection (new vs legacy llama-server). @@ -588,6 +635,12 @@ _LEGACY_HELP = """\ --spec-type none,ngram-mod,ngram-simple comma-separated list of types of speculative decoding to use """ +_CACHE_FLAGS_HELP = """\ +--cache-ram N store prompt cache in RAM (default: 0) +--ctx-checkpoints N number of context checkpoints (default: 0) +--no-cache-prompt do not reuse prompt cache +""" + @_NEEDS_BASH def test_probe_detects_post_rename_ngram_mod_flavor(tmp_path): @@ -634,6 +687,26 @@ def test_probe_no_ngram_mod_on_minimal_binary(tmp_path): assert caps["supports_ngram_mod"] is False +@_NEEDS_BASH +def test_probe_detects_windows_cache_flags(tmp_path): + fake = _make_fake_llama_server(tmp_path / "llama-server", _CACHE_FLAGS_HELP) + _clear_caps_cache() + caps = LlamaCppBackend.probe_server_capabilities(str(fake)) + assert caps["supports_cache_ram"] is True + assert caps["supports_ctx_checkpoints"] is True + assert caps["supports_no_cache_prompt"] is True + + +@_NEEDS_BASH +def test_probe_reports_windows_cache_flags_absent_for_older_binary(tmp_path): + fake = _make_fake_llama_server(tmp_path / "llama-server", "--threads N\n") + _clear_caps_cache() + caps = LlamaCppBackend.probe_server_capabilities(str(fake)) + assert caps["supports_cache_ram"] is False + assert caps["supports_ctx_checkpoints"] is False + assert caps["supports_no_cache_prompt"] is False + + def test_build_ngram_mod_flags_new(): flags = _build_ngram_mod_flags({"ngram_mod_flavor": "new"}) assert flags == [ diff --git a/tests/studio/test_stream_cancel_registration_timing.py b/tests/studio/test_stream_cancel_registration_timing.py index 6bd74f00de..3fd91a5188 100644 --- a/tests/studio/test_stream_cancel_registration_timing.py +++ b/tests/studio/test_stream_cancel_registration_timing.py @@ -69,6 +69,21 @@ def _finalbody_has_tracker_exit(finalbody) -> bool: return False +def _async_function(name: str) -> ast.AsyncFunctionDef: + for node in ast.walk(_TREE): + if isinstance(node, ast.AsyncFunctionDef) and node.name == name: + return node + raise AssertionError(f"{name} handler missing") + + +def _calls_name(node: ast.AST, name: str) -> bool: + for sub in ast.walk(node): + if isinstance(sub, ast.Call) and isinstance(sub.func, ast.Name): + if sub.func.id == name: + return True + return False + + # ── Structural tests ───────────────────────────────────────── @@ -160,6 +175,25 @@ def test_streaming_responses_have_no_background_task(): ) +def test_direct_llama_server_streams_install_disconnect_watcher(): + required = { + "openai_completions", + "_responses_stream", + "_anthropic_passthrough_stream", + "_openai_passthrough_stream", + } + missing = [ + name + for name in sorted(required) + if not _calls_name(_async_function(name), "_await_disconnect_then_close") + ] + assert not missing, ( + "Direct httpx streams to llama-server must close the upstream response " + "when the downstream client disconnects during prefill. Missing in: " + f"{missing}" + ) + + # ── Behavioral helpers ─────────────────────────────────────── _WANTED = { From 930dd17086a1b75317f9e1d20de39a134cbb82fe Mon Sep 17 00:00:00 2001 From: maattm <139474836+maattm@users.noreply.github.com> Date: Mon, 15 Jun 2026 10:35:33 +0100 Subject: [PATCH 02/91] feat: add Anthropic-compatible thinking parameter (#5856) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add Anthropic-compatible thinking parameter Add `thinking` parameter using Anthropic's format ({type: 'disabled'} / {type: 'enabled'}) alongside the existing `enable_thinking` boolean for backward compatibility. The new parameter is mapped internally to `enable_thinking` at the route layer so all downstream templates and backends continue to work unchanged. Changes: - Add ThinkingConfig model and `thinking` field to ChatCompletionRequest - Add mapping logic in routes: thinking.type -> enable_thinking - Add `thinking` field to frontend TypeScript types - Update frontend request building to send thinking parameter - Add tests for new thinking parameter * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix: move thinking→enable_thinking mapping to model_validator The Gemini review correctly identified that the route-level mapping bypasses normalization for external provider requests. Moving the mapping into a @model_validator on ChatCompletionRequest ensures it runs during Pydantic validation regardless of routing path. * Document ThinkingConfig scope and thinking validation behavior --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han Co-authored-by: Lee Jackson <130007945+Imagineer99@users.noreply.github.com> --- studio/backend/models/inference.py | 29 +++++ .../backend/tests/test_thinking_parameter.py | 106 ++++++++++++++++++ .../src/features/chat/api/chat-adapter.ts | 4 +- .../frontend/src/features/chat/types/api.ts | 1 + 4 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 studio/backend/tests/test_thinking_parameter.py diff --git a/studio/backend/models/inference.py b/studio/backend/models/inference.py index 4e6a5937d8..5b68364687 100644 --- a/studio/backend/models/inference.py +++ b/studio/backend/models/inference.py @@ -581,6 +581,16 @@ class ChatMessage(BaseModel): return self +class ThinkingConfig(BaseModel): + """Anthropic-compatible thinking/reasoning configuration. + Use type='disabled' to turn off thinking, or type='enabled' to turn it on. + Only type is read; extra fields (e.g. budget_tokens) are ignored, since + Studio sets provider thinking budgets itself. + """ + + type: Literal["disabled", "enabled"] = "disabled" + + class ChatCompletionRequest(BaseModel): """OpenAI-compatible chat completion request. @@ -694,6 +704,11 @@ class ChatCompletionRequest(BaseModel): None, description = "[x-unsloth] When true, keep historical blocks from past assistant turns in the prompt (Qwen3.6 templates). Independent of enable_thinking / reasoning_effort.", ) + thinking: Optional[ThinkingConfig] = Field( + None, + description = "[Anthropic-compatible] Thinking configuration. " + "Use {type: 'disabled'} to disable thinking, {type: 'enabled'} to enable.", + ) enable_tools: Optional[bool] = Field( None, description = "[x-unsloth] Enable tool calling for supported models", @@ -952,6 +967,20 @@ class ChatCompletionRequest(BaseModel): msg.tool_call_id = picked return self + @model_validator(mode = "after") + def _map_thinking_to_enable_thinking(self) -> "ChatCompletionRequest": + """Map Anthropic-style ``thinking`` parameter to internal ``enable_thinking``. + + ``thinking: {type: 'enabled'}`` sets ``enable_thinking = True`` and + ``thinking: {type: 'disabled'}`` sets ``enable_thinking = False``. + ``enable_thinking`` takes precedence when both are provided so that + callers who already use the internal field are unaffected. Invalid + ``thinking`` shapes are rejected at validation time (422). + """ + if self.thinking is not None and self.enable_thinking is None: + self.enable_thinking = self.thinking.type == "enabled" + return self + class ToolConfirmRequest(BaseModel): session_id: Optional[str] = None diff --git a/studio/backend/tests/test_thinking_parameter.py b/studio/backend/tests/test_thinking_parameter.py new file mode 100644 index 0000000000..92e08912cb --- /dev/null +++ b/studio/backend/tests/test_thinking_parameter.py @@ -0,0 +1,106 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 + +""" +Unit tests for the Anthropic-compatible thinking parameter. + +Covers: +- ThinkingConfig model validation +- ChatCompletionRequest with thinking parameter +- Mapping logic: thinking.type -> enable_thinking +""" + +import os +import sys + +_backend = os.path.join(os.path.dirname(__file__), "..") +sys.path.insert(0, _backend) + +from models.inference import ChatCompletionRequest, ThinkingConfig + + +def test_thinking_config_defaults_to_disabled(): + """ThinkingConfig should default to type='disabled'.""" + config = ThinkingConfig() + assert config.type == "disabled" + + +def test_thinking_config_explicit_disabled(): + """ThinkingConfig should accept type='disabled'.""" + config = ThinkingConfig(type = "disabled") + assert config.type == "disabled" + + +def test_thinking_config_explicit_enabled(): + """ThinkingConfig should accept type='enabled'.""" + config = ThinkingConfig(type = "enabled") + assert config.type == "enabled" + + +def test_chat_completion_request_with_thinking_disabled(): + """thinking.type='disabled' should map to enable_thinking=False.""" + req = ChatCompletionRequest.model_validate( + { + "model": "test-model", + "messages": [{"role": "user", "content": "hello"}], + "thinking": {"type": "disabled"}, + } + ) + assert req.thinking is not None + assert req.thinking.type == "disabled" + assert req.enable_thinking is False + + +def test_chat_completion_request_with_thinking_enabled(): + """thinking.type='enabled' should map to enable_thinking=True.""" + req = ChatCompletionRequest.model_validate( + { + "model": "test-model", + "messages": [{"role": "user", "content": "hello"}], + "thinking": {"type": "enabled"}, + } + ) + assert req.thinking is not None + assert req.thinking.type == "enabled" + assert req.enable_thinking is True + + +def test_chat_completion_request_without_thinking(): + """ChatCompletionRequest should work without thinking parameter.""" + req = ChatCompletionRequest.model_validate( + { + "model": "test-model", + "messages": [{"role": "user", "content": "hello"}], + } + ) + assert req.thinking is None + assert req.enable_thinking is None + + +def test_chat_completion_request_backward_compatible_enable_thinking(): + """ChatCompletionRequest should still support enable_thinking.""" + req = ChatCompletionRequest.model_validate( + { + "model": "test-model", + "messages": [{"role": "user", "content": "hello"}], + "enable_thinking": True, + } + ) + assert req.enable_thinking is True + assert req.thinking is None + + +def test_thinking_overrides_enable_thinking_when_both_provided(): + """When both thinking and enable_thinking are provided, + enable_thinking takes precedence (no override).""" + req = ChatCompletionRequest.model_validate( + { + "model": "test-model", + "messages": [{"role": "user", "content": "hello"}], + "thinking": {"type": "enabled"}, + "enable_thinking": False, + } + ) + # enable_thinking is explicitly set, so it takes precedence + assert req.enable_thinking is False + assert req.thinking.type == "enabled" diff --git a/studio/frontend/src/features/chat/api/chat-adapter.ts b/studio/frontend/src/features/chat/api/chat-adapter.ts index 7d9ee20a5f..6122b51dfd 100644 --- a/studio/frontend/src/features/chat/api/chat-adapter.ts +++ b/studio/frontend/src/features/chat/api/chat-adapter.ts @@ -2428,7 +2428,7 @@ export function createOpenAIStreamAdapter(): ChatModelAdapter { : { reasoning_effort: fallbackExternalEffort, } - : { enable_thinking: reasoningEnabled } + : { thinking: { type: reasoningEnabled ? "enabled" : "disabled" } } : {}), }; } @@ -2457,7 +2457,7 @@ export function createOpenAIStreamAdapter(): ChatModelAdapter { ? reasoningEnabled ? { reasoning_effort: localReasoningEffort } : {} - : { enable_thinking: reasoningEnabled } + : { thinking: { type: reasoningEnabled ? "enabled" : "disabled" } } : {}), ...(supportsPreserveThinking ? { preserve_thinking: preserveThinking } diff --git a/studio/frontend/src/features/chat/types/api.ts b/studio/frontend/src/features/chat/types/api.ts index 022624de60..2f0e30fdcf 100644 --- a/studio/frontend/src/features/chat/types/api.ts +++ b/studio/frontend/src/features/chat/types/api.ts @@ -289,6 +289,7 @@ export interface OpenAIChatCompletionsRequest { | "xhigh" | null; preserve_thinking?: boolean | null; + thinking?: {type: "disabled" | "enabled";} | null; enable_tools?: boolean | null; enabled_tools?: string[]; /** Local models + enable_tools only. */ From ca0528d1f84ad7cd2646c5a6fde1ac6b462d538f Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Mon, 15 Jun 2026 04:04:22 -0700 Subject: [PATCH 03/91] Studio: Bypass Permissions (skip confirmation, disable tool sandbox) (#5895) * Studio: Add inline confirmation (Allow/Always allow/Deny) for tool calls * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix race in tool-call confirmation gate * Studio: gate built-in tool calls and harden the confirmation handshake The Allow / Always allow / Deny controls only lived in the fallback tool card, but the built-in tools (web search, python, terminal, code execution, image generation) render with their own components and so never showed the buttons. Those calls paused after tool_start with no way to approve them, hanging until the 1 hour timeout. Only MCP tools, which use the fallback renderer, actually worked. Render the controls for every tool card by wrapping each registered tool component (and the fallback) in thread.tsx with a shared ToolConfirmationControls, so the gate applies uniformly. Also make the handshake robust: - The gate keys on a per-call approval_id minted by the backend and echoed in tool_start, instead of session_id alone, so a stale or concurrent confirmation can no longer resolve the wrong call. - The approval slot is registered before tool_start is yielded, closing the race where a fast click or an auto "Always allow" could reach the backend before the waiter existed. - The frontend resolves with the same session id the request was sent with (plus the approval_id), fixing the new-thread mismatch where the confirmation targeted a different session than the blocked stream. - The confirm endpoint returns {resolved}; the UI keeps the buttons and shows a retry hint until the backend confirms a match, instead of hiding them on a failed or mistargeted post. - The gate runs after the disabled-tool and duplicate-call checks, so a call that will not execute is not put up for approval. A denied call is still excluded from duplicate detection, so re-issuing and approving it works. - "Always allow" is scoped per session to match the backend gate. Add backend tests for the approval registry, the SSE no-deadlock handshake, and the loop integration (allow, deny, disabled, duplicate, re-issue after deny). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Move "Confirm tool calls" to the Tools section * Studio: add Bypass Permissions (skip confirmation, disable tool sandbox) Adds an opt-in Bypass Permissions toggle next to Confirm tool calls. When on, no tool call shows a confirmation prompt and the python/terminal sandbox is disabled: safety checks, command blocklist, and resource limits are skipped. Secret env vars are still stripped and HOME stays repointed at the session workdir. Default off keeps current behavior, and it takes precedence over Confirm tool calls. Enabling it requires accepting a warning each time. * Studio: harden Bypass Permissions secret handling and fix Anthropic tool path Follow-up to the Bypass Permissions feature. Addresses the review findings: - Anthropic /v1/messages 500: declare bypass_permissions on AnthropicMessagesRequest so tool requests that omit the field default to False instead of raising AttributeError (extra='allow' does not set absent attributes). - /proc parent-env leak: stripping the child env did not stop a same-uid bypassed child from reading /proc//environ to recover the tool-executing process's unfiltered secrets. Clear PR_SET_DUMPABLE on that process before the first bypass exec so its /proc entries become root-owned. Hardening is fail-closed: if prctl is denied, bypass execution is refused rather than run with the parent environ still readable. Mitigation, not a full boundary; documented in the code. - Broker/capability vars: strip SSH_AUTH_SOCK, SSH_AGENT_PID, GPG_AGENT_INFO, GNUPGHOME, KUBECONFIG, DOCKER_HOST so a bypassed tool cannot use the operator's live agents. - Credential-bearing URL values: drop any env var whose value embeds URL userinfo (scheme://user:pass@ and token-only scheme://token@) regardless of the variable name. Benign proxy/index URLs without credentials are kept, so proxy-only and internal-index setups still work in bypass mode. - Windows temp isolation: repoint TEMP and TMP (not just TMPDIR) at the per-session sandbox dir. - Frontend: stop persisting bypassPermissions; a reload now starts with the sandbox/confirmation bypass off and requires re-accepting the warning dialog. Adds regression tests for each finding in test_bypass_permissions.py. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: strip cred-location env vars (HF_HOME etc.) in Bypass Permissions Repointing HOME did not stop SDKs auto-reading cached creds via vars that point at the real home/cache/config: HF_HOME (startup always sets it; token lives under $HF_HOME/token), HF/XDG cache roots, NETRC/BOTO_CONFIG/ PIP_CONFIG_FILE, and Windows HOMEDRIVE/HOMEPATH. Drop those, and repoint USERPROFILE/APPDATA/LOCALAPPDATA at the per-session workdir. Adds regression tests. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: lock in bypass HF token resolution with an end-to-end test The drop-based fix relies on the whole HF_HOME/XDG fallback chain being removed so huggingface_hub resolves under the repointed HOME. Add a test that sets HF_HOME and XDG_CACHE_HOME at a real cache and asserts the resolved token path lands under the workdir, not the operator's cache. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: strip npm _auth, MYSQL_PWD, and BASH_ENV from bypass env Three more credential vectors dodged the bypass scrubber: NPM_CONFIG__AUTH (npm _auth, base64 so no URL userinfo and no AUTH marker), MYSQL_PWD (markers use PASSWD, not PWD, since PWD is the cwd var), and BASH_ENV (bash -c sources it for non-interactive shells, so a startup file can re-export stripped secrets). Add an AUTH marker, the exact MYSQL_PWD name, and drop BASH_ENV plus PGPASSFILE. Adds regression tests incl. an end-to-end check that a bypass terminal call does not source BASH_ENV. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: extend bypass env scrubber and enforce confirm precedence in loops From a parallel review pass over the bypass changes: - Drop more credential-location vars in _build_bypass_env: npm/yarn/git/cargo/ rclone config pointers (NPM_CONFIG_USERCONFIG, NPM_CONFIG_GLOBALCONFIG, YARN_RC_FILENAME, GIT_CONFIG_GLOBAL, GIT_CONFIG_SYSTEM, CARGO_HOME, RCLONE_CONFIG) and the GIT_ASKPASS/SSH_ASKPASS auth helpers. - Enforce confirm_tool_calls AND NOT bypass_permissions inside the safetensors and GGUF tool loops, not just at the route, so a direct internal caller passing both flags never prompts. - Soften the toggle hint: environment secrets are stripped, but bypassed code can still read files and credentials on the machine (no overclaim that keys stay hidden). Adds regression tests for the new names and the loop-level precedence. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: add GGUF loop test for bypass-over-confirm precedence The safetensors loop precedence is covered behaviorally; the GGUF loop needs a live llama-server so add an AST guard asserting its _needs_confirm gate references both confirm_tool_calls and bypass_permissions, matching the other llama_cpp source-inspection tests. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: add red Bypass Permissions badge in the composer When Bypass Permissions is on, show a persistent red pill in the composer tool-pill row (like the Search/Code pills), matching Claude Code's always- visible bypass indicator. Clicking it turns bypass off, mirroring the other composer toggles. Enabling still goes through the settings toggle + warning dialog. Adds a data-variant=danger style for the destructive-colored pill. * Studio: show Bypass Permissions badge in the Thread composer too The empty-state and active Thread render their own composer (thread.tsx), not shared-composer, so the badge only appeared in the split layout. Mirror the red dismissible pill in ComposerAction so it shows in every composer. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: keep the Bypass Permissions badge visible when the composer is collapsed The Thread composer only renders the pill row when expanded, so the active-mode badge vanished on the default (collapsed) empty state. Render it before the expand gate (it returns null when bypass is off) so the red indicator always shows while bypass is on. * Studio: make the Bypass Permissions confirm button a solid red button The destructive button variant is a subtle 10% tint that read as bare red text next to the outlined Cancel. Force the solid destructive fill (the variant's class loses to the tint through AlertDialogAction's Slot merge, so use the ! override the codebase already uses for this case) and shorten the label to 'I understand' so it fits the small dialog's two-column footer. * Studio: add Bypass Permissions to the composer + More menu Adds a 'Bypass Permissions' entry to the composer plus-menu (under More by default) in both composers, so it can be toggled without opening Run settings. Enabling routes through the same danger warning dialog; disabling is immediate. A shared BypassPermissionsMenuItem keeps the two composers in sync. * Studio: harden bypass env scrubber for IMDS opt-out and connection strings Two gaps in the Bypass Permissions secret scrubber: - The broad AWS_ prefix also dropped AWS_EC2_METADATA_DISABLED, a non-secret opt-out. Removing it re-opens the IMDS instance-role credential path that the operator explicitly disabled, so a bypassed boto/AWS-CLI call could recover cloud creds. Keep that flag (and AWS_EC2_METADATA_V1_DISABLED) via a keep-list while still stripping the real AWS credential vars. - Azure App Service connection strings (SQLCONNSTR_/CUSTOMCONNSTR_/..., WEBSITE_CONTENTAZUREFILECONNECTIONSTRING) and values like Password=/AccountKey= /SharedAccessKey= slipped past the name and URL-only value classifiers. Add CONNSTR/CONNECTIONSTRING name markers and a connection-string value matcher. * Studio: let Bypass Permissions suppress the confirm-tool-calls guards The confirm-vs-bypass precedence (confirm and not bypass) was applied at the loop call sites but not at the earlier request guards, so a client sending confirm_tool_calls + bypass_permissions together was rejected (stream=true required / unsupported for external or Anthropic tools) before the precedence took effect. Gate all four confirm guards on not bypass_permissions so both flags together proceed with the gate suppressed, matching the documented rule. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: oobabooga <112222186+oobabooga@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- studio/backend/core/inference/llama_cpp.py | 6 +- studio/backend/core/inference/orchestrator.py | 2 + .../core/inference/safetensors_agentic.py | 6 +- studio/backend/core/inference/tools.py | 323 +++++++- studio/backend/models/inference.py | 8 + studio/backend/routes/inference.py | 44 +- .../backend/tests/test_bypass_permissions.py | 732 ++++++++++++++++++ .../tests/test_safetensors_tool_loop.py | 1 + .../backend/tests/test_tool_confirm_loop.py | 1 + .../src/components/assistant-ui/thread.tsx | 29 + .../src/features/chat/api/chat-adapter.ts | 6 +- .../chat/bypass-permissions-menu-item.tsx | 82 ++ .../src/features/chat/chat-settings-sheet.tsx | 101 ++- .../src/features/chat/shared-composer.tsx | 20 + .../chat/stores/chat-runtime-store.ts | 16 + .../chat/stores/plus-menu-prefs-store.ts | 6 +- studio/frontend/src/index.css | 5 + 17 files changed, 1348 insertions(+), 40 deletions(-) create mode 100644 studio/backend/tests/test_bypass_permissions.py create mode 100644 studio/frontend/src/features/chat/bypass-permissions-menu-item.tsx diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index 8657179b73..a4fd07c3b6 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -5778,6 +5778,7 @@ class LlamaCppBackend: seed: Optional[int] = None, disable_parallel_tool_use: bool = False, confirm_tool_calls: bool = False, + bypass_permissions: bool = False, ) -> Generator[dict, None, None]: """ Agentic loop: let the model call tools, execute them, and continue. @@ -6478,7 +6479,9 @@ class LlamaCppBackend: decision.as_assistant_tool_call() ) - needs_confirm = bool(confirm_tool_calls) + # Bypass wins over the confirm gate at the loop level too, + # so a direct internal caller with both flags never prompts. + needs_confirm = bool(confirm_tool_calls) and not bypass_permissions approval_id = new_approval_id() if needs_confirm else "" decision_slot = ( begin_tool_decision(session_id, approval_id) if needs_confirm else None @@ -6539,6 +6542,7 @@ class LlamaCppBackend: timeout = _effective_timeout, session_id = session_id, rag_scope = rag_scope, + disable_sandbox = bypass_permissions, ) if decision.tool_name == "search_knowledge_base": _kb_search_count += 1 diff --git a/studio/backend/core/inference/orchestrator.py b/studio/backend/core/inference/orchestrator.py index 4ccac2912e..4fa48a2d88 100644 --- a/studio/backend/core/inference/orchestrator.py +++ b/studio/backend/core/inference/orchestrator.py @@ -862,6 +862,7 @@ class InferenceOrchestrator: session_id: Optional[str] = None, rag_scope: Optional[dict] = None, confirm_tool_calls: bool = False, + bypass_permissions: bool = False, use_adapter: Optional[Union[bool, str]] = None, stats_holder: Optional[dict] = None, **_unused, @@ -924,6 +925,7 @@ class InferenceOrchestrator: session_id = session_id, rag_scope = rag_scope, confirm_tool_calls = confirm_tool_calls, + bypass_permissions = bypass_permissions, ) def generate_with_adapter_control( diff --git a/studio/backend/core/inference/safetensors_agentic.py b/studio/backend/core/inference/safetensors_agentic.py index 3b6a393f3d..06b6cbe57f 100644 --- a/studio/backend/core/inference/safetensors_agentic.py +++ b/studio/backend/core/inference/safetensors_agentic.py @@ -154,6 +154,7 @@ def run_safetensors_tool_loop( session_id: Optional[str] = None, rag_scope: Optional[dict] = None, confirm_tool_calls: bool = False, + bypass_permissions: bool = False, ) -> Generator[dict, None, None]: """Drive an agentic tool loop on top of a cumulative-text generator. @@ -517,7 +518,9 @@ def run_safetensors_tool_loop( else: assistant_msg.setdefault("tool_calls", []).append(decision.as_assistant_tool_call()) - needs_confirm = bool(confirm_tool_calls) + # Bypass wins over the confirm gate at the loop level too, so a + # direct internal caller passing both flags never prompts. + needs_confirm = bool(confirm_tool_calls) and not bypass_permissions approval_id = new_approval_id() if needs_confirm else "" decision_slot = begin_tool_decision(session_id, approval_id) if needs_confirm else None start_event = decision.tool_start_event() @@ -575,6 +578,7 @@ def run_safetensors_tool_loop( timeout = eff_timeout, session_id = session_id, rag_scope = rag_scope, + disable_sandbox = bypass_permissions, ) except Exception as exc: logger.exception("Tool %s raised: %s", decision.tool_name, exc) diff --git a/studio/backend/core/inference/tools.py b/studio/backend/core/inference/tools.py index 43c9610282..b29a221ee7 100644 --- a/studio/backend/core/inference/tools.py +++ b/studio/backend/core/inference/tools.py @@ -316,6 +316,196 @@ def _build_safe_env(workdir: str) -> dict[str, str]: return env +# Credential env vars dropped even in bypass mode so tool code cannot read the +# operator's keys. Over-strips on purpose (a benign var is harmless to lose). +_BYPASS_ENV_SECRET_NAMES = frozenset( + { + "HF_TOKEN", + "HF_HUB_TOKEN", + "HUGGING_FACE_HUB_TOKEN", + "HUGGINGFACE_TOKEN", + "HUGGINGFACEHUB_API_TOKEN", + "WANDB_API_KEY", + "GH_TOKEN", + "GITHUB_TOKEN", + "OPENAI_API_KEY", + "ANTHROPIC_API_KEY", + "GEMINI_API_KEY", + "GOOGLE_API_KEY", + "GROQ_API_KEY", + "OPENROUTER_API_KEY", + "REPLICATE_API_TOKEN", + "COHERE_API_KEY", + "MISTRAL_API_KEY", + "NGC_API_KEY", + "KAGGLE_KEY", + "MYSQL_PWD", # exact name: markers use PASSWD, not PWD (PWD is the cwd var) + "LD_PRELOAD", + # Auth brokers / capability handles: not secrets by value, but they + # hand the child the operator's live agent (ssh/gpg), kube config, or + # docker daemon. Names are listed because there is no value signal to + # key off. URL config vars (HTTP_PROXY, PIP_INDEX_URL, DATABASE_URL, + # ...) are intentionally NOT name-listed: a benign proxy/index without + # credentials must keep working in bypass mode, while a credentialed + # value is dropped by _is_secret_env_value() regardless of its name. + "SSH_AUTH_SOCK", + "SSH_AGENT_PID", + "GPG_AGENT_INFO", + "GNUPGHOME", + "KUBECONFIG", + "DOCKER_HOST", + } +) +_BYPASS_ENV_SECRET_PREFIXES = ("AWS_", "AZURE_", "GOOGLE_", "GCP_", "GCLOUD_", "DYLD_") +_BYPASS_ENV_SECRET_MARKERS = ( + "TOKEN", + "API_KEY", + "APIKEY", + "SECRET", + "PASSWORD", + "PASSWD", + "CREDENTIAL", + "PRIVATE_KEY", + "AUTH", # e.g. NPM_CONFIG__AUTH (npm _auth), REDISCLI_AUTH + # Azure App Service connection strings: SQLCONNSTR_/CUSTOMCONNSTR_/... and + # WEBSITE_CONTENTAZUREFILECONNECTIONSTRING carry DB/storage credentials. + "CONNSTR", + "CONNECTIONSTRING", +) +# Non-secret hardening flags that match a secret prefix/marker but must be KEPT +# so bypass mode does not silently undo an operator's opt-out. AWS_EC2_METADATA_ +# DISABLED tells the AWS SDK/CLI not to pull instance-role creds from IMDS; +# dropping it would re-open that path for a bypassed tool. +_BYPASS_ENV_KEEP_NAMES = frozenset( + { + "AWS_EC2_METADATA_DISABLED", + "AWS_EC2_METADATA_V1_DISABLED", + } +) +# Matches a URL that embeds userinfo before the host, covering both +# "scheme://user:pass@host" and token-only "scheme://token@host" (and +# percent-encoded variants). The userinfo must precede the first '/', so an '@' +# in a path or query does not false-positive. Used to scrub credential-bearing +# URL values regardless of the variable's name. +_URL_USERINFO_RE = re.compile(r"://[^/\s@]+@") +# Connection-string credential fields (ADO.NET / Azure storage / Service Bus): +# "...;Password=...", "...;AccountKey=...", "...;SharedAccessKey=...". Catches +# credential-bearing values whose names dodge the name classifier. "accesskey" +# also covers Shared/Secret AccessKey via substring; the Name fields (e.g. +# SharedAccessKeyName=) do not match since "=" must follow the keyword. +_SECRET_VALUE_RE = re.compile(r"(?i)(?:password|pwd|accountkey|accesskey)\s*=\s*[^\s;]") + +# Names that hold no secret value but point SDKs at the operator's real +# home/cache/config (cached tokens, cred files), defeating the HOME repoint. +# Startup always sets HF_HOME (-> $HF_HOME/token), so this is the live leak. +# Dropped in bypass mode so tools fall back to the empty repointed HOME. +_BYPASS_ENV_CRED_LOCATION_NAMES = frozenset( + { + # HF cache roots (token lives under $HF_HOME/token) + "HF_HOME", + "HF_HUB_CACHE", + "HUGGINGFACE_HUB_CACHE", + "HF_XET_CACHE", + "TRANSFORMERS_CACHE", + "HF_DATASETS_CACHE", + "HF_ASSETS_CACHE", + # XDG base dirs (resolved before $HOME) + "XDG_CONFIG_HOME", + "XDG_CACHE_HOME", + "XDG_DATA_HOME", + # explicit cred/config file pointers honoured before $HOME + "NETRC", + "PGPASSFILE", + "BOTO_CONFIG", + "PIP_CONFIG_FILE", + "CLOUDSDK_CONFIG", + "KAGGLE_CONFIG_DIR", + "DOCKER_CONFIG", + "WANDB_DIR", + "WANDB_CONFIG_DIR", + "WANDB_CACHE_DIR", + # package-manager / git / cloud config pointers to real cred files + "NPM_CONFIG_USERCONFIG", + "NPM_CONFIG_GLOBALCONFIG", + "YARN_RC_FILENAME", + "GIT_CONFIG_GLOBAL", + "GIT_CONFIG_SYSTEM", + "CARGO_HOME", + "RCLONE_CONFIG", + # auth-helper scripts that hand creds to git/ssh + "GIT_ASKPASS", + "SSH_ASKPASS", + # shell startup hook: bash -c sources $BASH_ENV (can re-export secrets) + "BASH_ENV", + # Windows: HOMEDRIVE+HOMEPATH compose a home that bypasses HOME + "HOMEDRIVE", + "HOMEPATH", + } +) +# Windows profile dirs SDKs read creds under; repointed (not dropped) since +# callers expect them present. +_BYPASS_ENV_WINDOWS_PROFILE_VARS = ("USERPROFILE", "APPDATA", "LOCALAPPDATA") + + +def _is_secret_env_name(name: str) -> bool: + """True if an env var name looks like it carries a credential.""" + upper = name.upper() + if upper in _BYPASS_ENV_KEEP_NAMES: + return False # non-secret hardening flag; keep it + if upper in _BYPASS_ENV_SECRET_NAMES: + return True + if any(upper.startswith(p) for p in _BYPASS_ENV_SECRET_PREFIXES): + return True + return any(marker in upper for marker in _BYPASS_ENV_SECRET_MARKERS) + + +def _is_cred_location_env_name(name: str) -> bool: + """True for vars that point SDKs at the real home/cache/config (cached creds).""" + return name.upper() in _BYPASS_ENV_CRED_LOCATION_NAMES + + +def _is_secret_env_value(value: str) -> bool: + """True if a value embeds credentials regardless of its name. + + Catches URL userinfo (``scheme://user:token@host`` in DATABASE_URL / + PIP_INDEX_URL / HTTP_PROXY) and connection-string credential fields + (``...;Password=...`` / ``...;AccountKey=...``) whose names dodge the name + classifier. + """ + if not value: + return False + return _URL_USERINFO_RE.search(value) is not None or _SECRET_VALUE_RE.search(value) is not None + + +def _build_bypass_env(workdir: str) -> dict[str, str]: + """Env for bypass exec: full host env (unrestricted) minus credential vars, + with HOME/TMPDIR repointed at the workdir so SDKs cannot read cached creds. + + Note: stripping the child env is necessary but not sufficient on its own - + a same-UID child can still read the parent's environment via procfs, so + callers also harden the parent (see _harden_parent_against_proc_env_leak). + """ + env = { + k: v + for k, v in os.environ.items() + if not _is_secret_env_name(k) + and not _is_secret_env_value(v) + and not _is_cred_location_env_name(k) + } + env["HOME"] = workdir + env["TMPDIR"] = workdir + # Windows tempfile / SDKs honour TEMP/TMP, not TMPDIR; repoint all three so + # the bypassed tool writes under the per-session sandbox dir on every OS. + env["TEMP"] = workdir + env["TMP"] = workdir + # Windows SDKs read creds under the profile dirs, not $HOME; repoint set + # ones to the workdir (HOMEDRIVE/HOMEPATH are dropped above). + for var in _BYPASS_ENV_WINDOWS_PROFILE_VARS: + if var in os.environ: + env[var] = workdir + return env + + def _sandbox_preexec(): """Best-effort sandbox setup for sandboxed subprocesses (modules are resolved at import time so the forked child runs no imports).""" @@ -377,6 +567,65 @@ def _sandbox_preexec(): pass +def _bypass_preexec(): + """Minimal pre-exec for bypass exec: os.setsid() only. + + Required, not a restriction: _kill_process_tree does killpg(getpgid(child)), + so without a new session a timeout/cancel would kill the Studio server too. + """ + try: + os.setsid() + except OSError: + pass + + +# Hardening the Studio parent is done once (PR_SET_DUMPABLE is process-global +# and sticky); guarded so repeated bypass calls do not re-issue the prctl. +_parent_proc_hardened = False + + +def _harden_parent_against_proc_env_leak() -> bool: + """Make the Studio process's /proc//environ unreadable to its children. + + Stripping the child env is not enough on Linux: a bypassed same-UID child + runs unsandboxed and can read /proc//environ to recover the + tool-executing process's *unfiltered* secrets (HF_TOKEN, cloud keys, ...). + Clearing the dumpable flag (PR_SET_DUMPABLE=0) reparents this process's + /proc entries to root, so a same-UID child can no longer read its environ. + + Returns True when the process is hardened or hardening is unnecessary (no + /proc leak off Linux), and False when it is needed but could not be applied + (e.g. prctl denied by a seccomp policy). Callers must fail closed - refuse + the unsandboxed exec - when this returns False, rather than running with the + parent environ still readable. + + Scope: this closes the direct parent read (the demonstrated leak). It is a + mitigation, not a full boundary - a bypassed tool is unsandboxed by design, + so it can still walk /proc to a same-UID *ancestor* (e.g. the launching + shell) or read on-disk credentials by absolute path. Complete isolation + needs a separate uid / PID+mount namespace, which is out of scope here; the + UI already warns the mode is dangerous. Applied lazily on first bypass exec + so non-bypass operation is unchanged. + """ + global _parent_proc_hardened + if _parent_proc_hardened: + return True + if sys.platform != "linux": + return True # no /proc//environ same-UID leak to close + if _libc is None: + return False # on Linux but cannot issue prctl -> cannot harden + try: + # prctl(PR_SET_DUMPABLE=4, SUID_DUMP_DISABLE=0). ctypes returns the + # syscall result (-1 on failure) and does NOT raise, so check it. + ret = _libc.prctl(4, 0, 0, 0, 0) + except (OSError, AttributeError): + return False + if ret != 0: + return False + _parent_proc_hardened = True + return True + + def _get_shell_cmd(command: str) -> list[str]: """Return the platform-appropriate shell invocation for a command string.""" if sys.platform == "win32": @@ -723,6 +972,7 @@ def execute_tool( timeout: int | None = _TIMEOUT_UNSET, session_id: str | None = None, rag_scope: dict | None = None, + disable_sandbox: bool = False, ) -> str: """Execute a tool by name with the given arguments; returns a string. @@ -730,6 +980,9 @@ def execute_tool( ``session_id``: optional ID for per-conversation sandbox isolation. ``rag_scope``: hidden per-request RAG context the model never sees; consumed by ``search_knowledge_base``. + ``disable_sandbox``: Bypass Permissions; run python/terminal without the + safety checks, blocklist, or resource caps (secrets still stripped). Only + affects local code tools; web_search / MCP are unchanged. """ logger.info(f"execute_tool: name={name}, session_id={session_id}, timeout={timeout}") effective_timeout = _EXEC_TIMEOUT if timeout is _TIMEOUT_UNSET else timeout @@ -765,9 +1018,21 @@ def execute_tool( timeout = effective_timeout, ) if name == "python": - return _python_exec(arguments.get("code", ""), cancel_event, effective_timeout, session_id) + return _python_exec( + arguments.get("code", ""), + cancel_event, + effective_timeout, + session_id, + disable_sandbox = disable_sandbox, + ) if name == "terminal": - return _bash_exec(arguments.get("command", ""), cancel_event, effective_timeout, session_id) + return _bash_exec( + arguments.get("command", ""), + cancel_event, + effective_timeout, + session_id, + disable_sandbox = disable_sandbox, + ) return f"Unknown tool: {name}" @@ -2242,15 +2507,28 @@ def _python_exec( cancel_event = None, timeout: int = _EXEC_TIMEOUT, session_id: str | None = None, + disable_sandbox: bool = False, ) -> str: - """Execute Python code in a subprocess sandbox.""" + """Execute Python code in a subprocess sandbox. + + disable_sandbox (Bypass Permissions): skip the safety analysis and rlimit + pre-exec, and use the host env minus secrets. + """ if not code or not code.strip(): return "No code provided." - # Validate imports and code safety - error = _check_code_safety(code) - if error: - return error + # Validate imports and code safety (skipped when the sandbox is disabled) + if not disable_sandbox: + error = _check_code_safety(code) + if error: + return error + elif not _harden_parent_against_proc_env_leak(): + # Close the /proc//environ secret-recovery path first; if it + # cannot be applied, fail closed rather than leak the parent environ. + return ( + "Execution error: could not harden the Studio process against " + "/proc environment reads; refusing bypass execution." + ) tmp_path = None workdir = _get_workdir(session_id) @@ -2270,7 +2548,7 @@ def _python_exec( with os.fdopen(fd, "w") as f: f.write(code) - safe_env = _build_safe_env(workdir) + safe_env = _build_bypass_env(workdir) if disable_sandbox else _build_safe_env(workdir) popen_kwargs = dict( stdout = subprocess.PIPE, stderr = subprocess.STDOUT, @@ -2279,7 +2557,7 @@ def _python_exec( env = safe_env, ) if sys.platform != "win32": - popen_kwargs["preexec_fn"] = _sandbox_preexec + popen_kwargs["preexec_fn"] = _bypass_preexec if disable_sandbox else _sandbox_preexec else: popen_kwargs["creationflags"] = subprocess.CREATE_NO_WINDOW @@ -2346,19 +2624,32 @@ def _bash_exec( cancel_event = None, timeout: int = _EXEC_TIMEOUT, session_id: str | None = None, + disable_sandbox: bool = False, ) -> str: - """Execute a bash command in a subprocess sandbox.""" + """Execute a bash command in a subprocess sandbox. + + disable_sandbox (Bypass Permissions): skip the command blocklist and rlimit + pre-exec, and use the host env minus secrets. + """ if not command or not command.strip(): return "No command provided." - # Block dangerous commands (shlex + regex based) - blocked = _find_blocked_commands(command) - if blocked: - return f"Blocked command(s) for safety: {', '.join(sorted(blocked))}" + # Block dangerous commands (skipped when the sandbox is disabled) + if not disable_sandbox: + blocked = _find_blocked_commands(command) + if blocked: + return f"Blocked command(s) for safety: {', '.join(sorted(blocked))}" + elif not _harden_parent_against_proc_env_leak(): + # Close the /proc//environ secret-recovery path first; if it + # cannot be applied, fail closed rather than leak the parent environ. + return ( + "Execution error: could not harden the Studio process against " + "/proc environment reads; refusing bypass execution." + ) try: workdir = _get_workdir(session_id) - safe_env = _build_safe_env(workdir) + safe_env = _build_bypass_env(workdir) if disable_sandbox else _build_safe_env(workdir) popen_kwargs = dict( stdout = subprocess.PIPE, stderr = subprocess.STDOUT, @@ -2367,7 +2658,7 @@ def _bash_exec( env = safe_env, ) if sys.platform != "win32": - popen_kwargs["preexec_fn"] = _sandbox_preexec + popen_kwargs["preexec_fn"] = _bypass_preexec if disable_sandbox else _sandbox_preexec else: popen_kwargs["creationflags"] = subprocess.CREATE_NO_WINDOW diff --git a/studio/backend/models/inference.py b/studio/backend/models/inference.py index 5b68364687..82949870da 100644 --- a/studio/backend/models/inference.py +++ b/studio/backend/models/inference.py @@ -732,6 +732,10 @@ class ChatCompletionRequest(BaseModel): None, description = "[x-unsloth] When true, pause before each tool call and wait for the user to allow/deny it via POST /api/inference/tool-confirm.", ) + bypass_permissions: Optional[bool] = Field( + False, + description = "[x-unsloth] Bypass Permissions: when true, skip the tool-call confirmation gate AND disable the python/terminal execution sandbox (safety checks, command blocklist, resource limits). Secret env vars are still stripped. Takes precedence over confirm_tool_calls.", + ) auto_heal_tool_calls: Optional[bool] = Field( True, description = "[x-unsloth] Auto-detect and fix malformed tool calls from model output.", @@ -1560,6 +1564,10 @@ class AnthropicMessagesRequest(BaseModel): enabled_tools: Optional[list[str]] = None session_id: Optional[str] = None cancel_id: Optional[str] = None + bypass_permissions: Optional[bool] = Field( + False, + description = "[x-unsloth] Bypass Permissions: when true, disable the python/terminal execution sandbox (safety checks, command blocklist, resource limits) for server-side tool calls. Secret env vars are still stripped. Declared explicitly (not relied on via extra='allow') so omitted requests default to False instead of raising AttributeError.", + ) model_config = {"extra": "allow"} @model_validator(mode = "before") diff --git a/studio/backend/routes/inference.py b/studio/backend/routes/inference.py index 3e13774639..c272c67944 100644 --- a/studio/backend/routes/inference.py +++ b/studio/backend/routes/inference.py @@ -3522,12 +3522,18 @@ async def openai_chat_completions( # ── External provider routing ──────────────────────────────── # encrypted_api_key is optional -- local providers (llama.cpp / vLLM / Ollama) may run without auth. if payload.provider_id or payload.provider_type: - if payload.confirm_tool_calls and ( - payload.enable_tools is True - or bool(payload.enabled_tools) - or bool(payload.tools) - or bool(payload.openai_code_exec_container_id) - or bool(payload.anthropic_code_exec_container_id) + # Bypass Permissions suppresses the confirm gate, so do not reject a + # request that sets both flags (effective confirm is then False). + if ( + payload.confirm_tool_calls + and not payload.bypass_permissions + and ( + payload.enable_tools is True + or bool(payload.enabled_tools) + or bool(payload.tools) + or bool(payload.openai_code_exec_container_id) + or bool(payload.anthropic_code_exec_container_id) + ) ): raise HTTPException( status_code = 400, @@ -3909,7 +3915,9 @@ async def openai_chat_completions( use_tools = False if use_tools: - if payload.confirm_tool_calls and not payload.stream: + # Bypass Permissions suppresses confirm, so the stream requirement + # (the gate needs streaming to prompt) no longer applies. + if payload.confirm_tool_calls and not payload.bypass_permissions and not payload.stream: raise HTTPException( status_code = 400, detail = openai_error_body( @@ -3989,7 +3997,11 @@ async def openai_chat_completions( session_id = payload.session_id, rag_scope = payload.rag_scope, disable_parallel_tool_use = payload.parallel_tool_calls is False, - confirm_tool_calls = bool(payload.confirm_tool_calls), + # Bypass Permissions takes precedence over the confirm gate: + # never prompt while bypassing. + confirm_tool_calls = bool(payload.confirm_tool_calls) + and not bool(payload.bypass_permissions), + bypass_permissions = bool(payload.bypass_permissions), ) _tool_sentinel = object() @@ -4454,7 +4466,9 @@ async def openai_chat_completions( _sf_use_tools = False if _sf_use_tools: - if payload.confirm_tool_calls and not payload.stream: + # Bypass Permissions suppresses confirm, so the stream requirement + # (the gate needs streaming to prompt) no longer applies. + if payload.confirm_tool_calls and not payload.bypass_permissions and not payload.stream: raise HTTPException( status_code = 400, detail = openai_error_body( @@ -4540,7 +4554,11 @@ async def openai_chat_completions( else 300, session_id = payload.session_id, rag_scope = payload.rag_scope, - confirm_tool_calls = bool(payload.confirm_tool_calls), + # Bypass Permissions takes precedence over the confirm gate: + # never prompt while bypassing. + confirm_tool_calls = bool(payload.confirm_tool_calls) + and not bool(payload.bypass_permissions), + bypass_permissions = bool(payload.bypass_permissions), use_adapter = payload.use_adapter, stats_holder = _sf_stats_holder, ) @@ -6927,7 +6945,10 @@ async def anthropic_messages( ) if server_tools: - if bool(getattr(payload, "confirm_tool_calls", False)): + # Bypass Permissions suppresses confirm, so both flags together is fine. + if bool(getattr(payload, "confirm_tool_calls", False)) and not bool( + getattr(payload, "bypass_permissions", False) + ): raise HTTPException( status_code = 400, detail = anthropic_error_body( @@ -6984,6 +7005,7 @@ async def anthropic_messages( # Anthropic passthrough has no rag_scope field (RAG is local-only). rag_scope = getattr(payload, "rag_scope", None), disable_parallel_tool_use = _disable_parallel, + bypass_permissions = bool(payload.bypass_permissions), ) if payload.stream: diff --git a/studio/backend/tests/test_bypass_permissions.py b/studio/backend/tests/test_bypass_permissions.py new file mode 100644 index 0000000000..563f146816 --- /dev/null +++ b/studio/backend/tests/test_bypass_permissions.py @@ -0,0 +1,732 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 + +"""Tests for Bypass Permissions (skip confirmation + disable sandbox). + +Covers the secret-name classifier, the two env builders, the +``disable_sandbox`` branch of ``_python_exec`` / ``_bash_exec`` (which env is +used, which pre-exec is used, and that safety checks / the blocklist are +skipped), the request-model default, the confirm-vs-bypass precedence rule the +route enforces, and that the agentic loop forwards ``disable_sandbox`` while +never gating under bypass. + +Run with: ``PYTHONPATH=studio/backend python -m pytest studio/backend/tests/test_bypass_permissions.py -q`` +""" + +import os +import sys + +import pytest + +import core.inference.tools as tools +from core.inference.tools import ( + _bash_exec, + _build_bypass_env, + _build_safe_env, + _is_cred_location_env_name, + _is_secret_env_name, + _is_secret_env_value, + _python_exec, +) +from core.inference.safetensors_agentic import run_safetensors_tool_loop + +_POSIX_ONLY = pytest.mark.skipif( + sys.platform == "win32", reason = "preexec_fn / setsid are POSIX-only" +) + + +# ── secret-name classifier ────────────────────────────────────────── + + +@pytest.mark.parametrize( + "name", + [ + "HF_TOKEN", + "HUGGING_FACE_HUB_TOKEN", + "WANDB_API_KEY", + "GH_TOKEN", + "GITHUB_TOKEN", + "OPENAI_API_KEY", + "ANTHROPIC_API_KEY", + "AWS_SECRET_ACCESS_KEY", + "AWS_ACCESS_KEY_ID", + "AZURE_CLIENT_SECRET", + "GOOGLE_APPLICATION_CREDENTIALS", + "MY_DB_PASSWORD", + "x_api_key", + "SOME_PRIVATE_KEY", + "LD_PRELOAD", + ], +) +def test_secret_names_are_flagged(name): + assert _is_secret_env_name(name) is True + + +@pytest.mark.parametrize( + "name", ["PATH", "HOME", "LANG", "TERM", "PWD", "SHELL", "HOSTVAR", "MY_VAR"] +) +def test_benign_names_are_not_flagged(name): + assert _is_secret_env_name(name) is False + + +# ── env builders ──────────────────────────────────────────────────── + + +def test_bypass_env_keeps_benign_strips_secret_repoints_home(monkeypatch, tmp_path): + monkeypatch.setenv("HOSTVAR", "benign-123") + monkeypatch.setenv("HF_TOKEN", "secret-abc") + env = _build_bypass_env(str(tmp_path)) + assert env.get("HOSTVAR") == "benign-123" # full host env inherited + assert "HF_TOKEN" not in env # ...minus secrets + assert env["HOME"] == str(tmp_path) # $HOME-based cred lookups defused + assert env["TMPDIR"] == str(tmp_path) + + +def test_safe_env_excludes_host_and_secret(monkeypatch, tmp_path): + monkeypatch.setenv("HOSTVAR", "benign-123") + monkeypatch.setenv("HF_TOKEN", "secret-abc") + env = _build_safe_env(str(tmp_path)) + assert "HOSTVAR" not in env # whitelist build -> host vars never reach child + assert "HF_TOKEN" not in env + + +# ── Popen kwargs capture (no real execution) ──────────────────────── + + +class _FakeProc: + returncode = 0 + + def communicate(self, timeout = None): + return ("FAKEOUT", None) + + def poll(self): + return 0 + + def kill(self): + pass + + +@pytest.fixture +def captured_popen(monkeypatch): + cap = {} + + def fake_popen(cmd, **kwargs): + cap["cmd"] = cmd + cap["kwargs"] = kwargs + return _FakeProc() + + monkeypatch.setattr(tools.subprocess, "Popen", fake_popen) + return cap + + +@_POSIX_ONLY +def test_python_sandboxed_uses_sandbox_preexec_and_safe_env(captured_popen, monkeypatch): + monkeypatch.setenv("HF_TOKEN", "secret-abc") + _python_exec("print(1)", None, 5, "t", disable_sandbox = False) + assert captured_popen["kwargs"]["preexec_fn"] is tools._sandbox_preexec + assert "HF_TOKEN" not in captured_popen["kwargs"]["env"] + + +@_POSIX_ONLY +def test_python_bypass_uses_bypass_preexec_and_bypass_env(captured_popen, monkeypatch): + monkeypatch.setenv("HOSTVAR", "benign-xyz") + monkeypatch.setenv("HF_TOKEN", "secret-abc") + _python_exec("print(1)", None, 5, "t", disable_sandbox = True) + assert captured_popen["kwargs"]["preexec_fn"] is tools._bypass_preexec + env = captured_popen["kwargs"]["env"] + assert env.get("HOSTVAR") == "benign-xyz" + assert "HF_TOKEN" not in env + + +def test_bash_blocklist_enforced_when_sandboxed(captured_popen): + out = _bash_exec("rm -rf /", None, 5, "t", disable_sandbox = False) + assert "Blocked" in out + assert "cmd" not in captured_popen # never reached Popen + + +def test_bash_blocklist_skipped_when_bypassed(captured_popen): + out = _bash_exec("rm -rf /", None, 5, "t", disable_sandbox = True) + assert out == "FAKEOUT" # blocklist skipped -> reached (faked) execution + assert captured_popen["cmd"][0] in ("bash", "cmd") + + +@_POSIX_ONLY +def test_bash_bypass_uses_bypass_preexec(captured_popen): + _bash_exec("echo hi", None, 5, "t", disable_sandbox = True) + assert captured_popen["kwargs"]["preexec_fn"] is tools._bypass_preexec + + +# ── real end-to-end python execution under bypass ─────────────────── + + +@_POSIX_ONLY +def test_python_bypass_real_exec_sees_host_env_but_not_secret(monkeypatch): + monkeypatch.setenv("HOSTVAR", "benign-xyz") + monkeypatch.setenv("HF_TOKEN", "secret-pqr") + code = ( + "import os;" + "print('H=' + str(os.environ.get('HOSTVAR'))," + " 'T=' + str(os.environ.get('HF_TOKEN')))" + ) + out = _python_exec(code, None, 30, "test-bypass", disable_sandbox = True) + assert "H=benign-xyz" in out # unrestricted: real host var visible + assert "T=None" in out # ...but the secret was stripped + assert "secret-pqr" not in out + + +# ── _bypass_preexec is setsid-only (no rlimits) ───────────────────── + + +@_POSIX_ONLY +def test_bypass_preexec_only_sets_session(monkeypatch): + calls = {"setsid": 0} + monkeypatch.setattr( + tools.os, "setsid", lambda: calls.__setitem__("setsid", calls["setsid"] + 1) + ) + # _resource must not be touched by the bypass pre-exec. + if tools._resource is not None: + monkeypatch.setattr( + tools._resource, + "setrlimit", + lambda *a, **k: pytest.fail("bypass pre-exec must not set rlimits"), + ) + tools._bypass_preexec() + assert calls["setsid"] == 1 + + +# ── request model default ─────────────────────────────────────────── + + +def test_request_model_bypass_default_false(): + from models.inference import ChatCompletionRequest + assert ChatCompletionRequest.model_fields["bypass_permissions"].default is False + + +# ── confirm-vs-bypass precedence (mirrors the route rule) ─────────── + + +@pytest.mark.parametrize( + "confirm,bypass,effective_confirm", + [ + (False, False, False), + (True, False, True), + (False, True, False), + (True, True, False), + ], +) +def test_confirm_precedence_rule(confirm, bypass, effective_confirm): + # The route computes: confirm_tool_calls = confirm and not bypass. + assert (bool(confirm) and not bool(bypass)) is effective_confirm + + +# ── agentic loop forwards disable_sandbox, never gates under bypass ── + +_DEFAULT_TOOLS = [ + {"type": "function", "function": {"name": "python"}}, + {"type": "function", "function": {"name": "web_search"}}, +] + + +def _tool_call(name, args_json): + return f'{{"name": "{name}", "arguments": {args_json}}}' + + +def _multi_turn(turns): + it = iter(turns) + + def _gen(_messages): + try: + yield next(it) + except StopIteration: + return + + return _gen + + +def test_loop_forwards_disable_sandbox_and_does_not_gate(): + seen = [] + + def fake_exec( + name, + arguments, + *, + cancel_event = None, + timeout = None, + session_id = None, + rag_scope = None, + disable_sandbox = False, + ): + seen.append(disable_sandbox) + return f"RAN[{name}]" + + events = list( + run_safetensors_tool_loop( + single_turn = _multi_turn([_tool_call("python", '{"code": "x"}'), "done"]), + messages = [{"role": "user", "content": "hi"}], + tools = _DEFAULT_TOOLS, + execute_tool = fake_exec, + session_id = "s", + confirm_tool_calls = False, # route forces this off under bypass + bypass_permissions = True, + ) + ) + assert seen == [True] # disable_sandbox threaded through + starts = [e for e in events if e["type"] == "tool_start"] + assert starts and starts[0]["awaiting_confirmation"] is False + assert starts[0]["approval_id"] == "" + + +def test_loop_bypass_overrides_confirm_for_direct_callers(): + # Even if a direct internal caller passes confirm_tool_calls=True, bypass + # must suppress the confirm gate at the loop level (not only at the route). + def fake_exec( + name, + arguments, + *, + cancel_event = None, + timeout = None, + session_id = None, + rag_scope = None, + disable_sandbox = False, + ): + return f"RAN[{name}]" + + events = list( + run_safetensors_tool_loop( + single_turn = _multi_turn([_tool_call("python", '{"code": "x"}'), "done"]), + messages = [{"role": "user", "content": "hi"}], + tools = _DEFAULT_TOOLS, + execute_tool = fake_exec, + session_id = "s", + confirm_tool_calls = True, # raw caller leaves this on... + bypass_permissions = True, # ...but bypass must still win + ) + ) + starts = [e for e in events if e["type"] == "tool_start"] + assert starts and starts[0]["awaiting_confirmation"] is False + assert starts[0]["approval_id"] == "" + + +def test_gguf_loop_confirm_gate_respects_bypass(): + # The GGUF loop needs a live llama-server, so (per the other llama_cpp + # tests) assert via AST that its _needs_confirm gate applies the bypass + # precedence, mirroring the safetensors behavioral test above. + import ast + import inspect + import textwrap + + llama_cpp = pytest.importorskip("core.inference.llama_cpp") + src = textwrap.dedent( + inspect.getsource(llama_cpp.LlamaCppBackend.generate_chat_completion_with_tools) + ) + gates = [ + node + for node in ast.walk(ast.parse(src)) + if isinstance(node, ast.Assign) + and any(getattr(t, "id", None) == "needs_confirm" for t in node.targets) + ] + assert gates, "could not find the needs_confirm gate in the GGUF loop" + names = {n.id for g in gates for n in ast.walk(g.value) if isinstance(n, ast.Name)} + assert "confirm_tool_calls" in names + assert "bypass_permissions" in names # bypass must suppress the GGUF gate + + +# ── broker / capability env vars are stripped (regression) ────────── + + +@pytest.mark.parametrize( + "name", + ["SSH_AUTH_SOCK", "SSH_AGENT_PID", "GPG_AGENT_INFO", "GNUPGHOME", "KUBECONFIG"], +) +def test_broker_capability_names_are_flagged(name): + # Not secrets by value, but they hand the child the operator's live agent + # (ssh/gpg) or kube credentials, so bypass mode must drop them. + assert _is_secret_env_name(name) is True + + +# ── credential-bearing URL values stripped regardless of name ─────── + + +@pytest.mark.parametrize( + "value", + [ + "https://user:s3cr3t@feed.example.invalid/simple", # user:pass@ + "https://ghp_deadbeef@github.com/org/private.git", # token-only@ + "https://__token__@pypi.example.invalid/simple", + "https://ghp_1234:@npm.pkg.github.com/simple", # empty password + "postgres://dbuser:dbpass@db.example.invalid/app", + ], +) +def test_url_userinfo_values_are_flagged(value): + assert _is_secret_env_value(value) is True + + +@pytest.mark.parametrize( + "value", + [ + "https://example.invalid/simple", # no userinfo + "http://proxy.corp.example:8080", # benign proxy + "https://pypi.corp.example/simple", # benign internal index + "redis://localhost:6379/0", # no creds + "https://example.invalid/path?ref=a@b", # '@' only in query, not userinfo + ], +) +def test_non_credential_url_values_are_not_flagged(value): + assert _is_secret_env_value(value) is False + + +def test_url_userinfo_value_is_stripped_even_with_benign_name(monkeypatch, tmp_path): + # NAME dodges the classifier, but the VALUE embeds userinfo -> must go. + monkeypatch.setenv("MY_FEED", "https://user:s3cr3t@feed.example.invalid/simple") + monkeypatch.setenv("REPO_URL", "https://ghp_deadbeef@github.com/org/private.git") + # A URL without credentials is harmless and should be kept. + monkeypatch.setenv("PLAIN_URL", "https://example.invalid/simple") + env = _build_bypass_env(str(tmp_path)) + assert "MY_FEED" not in env + assert "REPO_URL" not in env + assert env.get("PLAIN_URL") == "https://example.invalid/simple" + + +def test_bypass_env_keeps_noncredential_proxy_and_index_urls(monkeypatch, tmp_path): + # Benign routing/config vars must survive bypass mode (proxy-only or + # internal-index networks); only credentialed values are dropped. + monkeypatch.setenv("HTTP_PROXY", "http://proxy.corp.example:8080") + monkeypatch.setenv("PIP_INDEX_URL", "https://pypi.corp.example/simple") + monkeypatch.setenv("PIP_EXTRA_INDEX_URL", "https://user:token@pypi.example.invalid/simple") + env = _build_bypass_env(str(tmp_path)) + assert env["HTTP_PROXY"] == "http://proxy.corp.example:8080" + assert env["PIP_INDEX_URL"] == "https://pypi.corp.example/simple" + assert "PIP_EXTRA_INDEX_URL" not in env # this one carries credentials + + +# ── AWS IMDS-disable hardening flag is kept (regression) ──────────── + + +def test_aws_imds_disable_flag_is_kept_but_creds_stripped(monkeypatch, tmp_path): + # AWS_EC2_METADATA_DISABLED is a non-secret opt-out: dropping it would let a + # bypassed boto/AWS-CLI call fall back to the instance role via IMDS even + # though the operator disabled that path. Keep it; drop the real creds. + monkeypatch.setenv("AWS_EC2_METADATA_DISABLED", "true") + monkeypatch.setenv("AWS_ACCESS_KEY_ID", "AKIAEXAMPLE") + monkeypatch.setenv("AWS_SECRET_ACCESS_KEY", "shhh") + assert _is_secret_env_name("AWS_EC2_METADATA_DISABLED") is False + assert _is_secret_env_name("AWS_ACCESS_KEY_ID") is True + env = _build_bypass_env(str(tmp_path)) + assert env.get("AWS_EC2_METADATA_DISABLED") == "true" + assert "AWS_ACCESS_KEY_ID" not in env + assert "AWS_SECRET_ACCESS_KEY" not in env + + +# ── connection-string env vars are stripped (regression) ──────────── + + +@pytest.mark.parametrize( + "name", + [ + "SQLCONNSTR_DB", # Azure App Service injected connection strings + "MYSQLCONNSTR_DB", + "SQLAZURECONNSTR_DB", + "POSTGRESQLCONNSTR_DB", + "CUSTOMCONNSTR_CACHE", + "WEBSITE_CONTENTAZUREFILECONNECTIONSTRING", + ], +) +def test_connection_string_names_are_flagged(name): + assert _is_secret_env_name(name) is True + + +@pytest.mark.parametrize( + "value", + [ + "Server=tcp:db;Database=app;User ID=u;Password=p@ss;", # ADO.NET + "DefaultEndpointsProtocol=https;AccountName=x;AccountKey=abc123==;", # storage + "Endpoint=sb://x;SharedAccessKeyName=n;SharedAccessKey=zzz=", # Service Bus + ], +) +def test_connection_string_values_are_flagged(value): + assert _is_secret_env_value(value) is True + + +@pytest.mark.parametrize( + "value", + [ + "Server=tcp:db;Database=app;User ID=u;", # no password field + "Endpoint=sb://x;SharedAccessKeyName=n", # key NAME only, no secret + "AccountName=x;EndpointSuffix=core.windows.net", # no AccountKey + ], +) +def test_connection_string_noncredential_values_are_not_flagged(value): + assert _is_secret_env_value(value) is False + + +def test_connection_string_value_stripped_even_with_benign_name(monkeypatch, tmp_path): + # NAME dodges the classifier, but the VALUE is a credentialed conn string. + monkeypatch.setenv("APP_DB", "Server=tcp:db;Database=app;User ID=u;Password=p@ss;") + monkeypatch.setenv("SQLCONNSTR_DB", "DefaultEndpointsProtocol=https;AccountKey=abc==") + env = _build_bypass_env(str(tmp_path)) + assert "APP_DB" not in env # value-based catch + assert "SQLCONNSTR_DB" not in env # name-based catch + + +# ── temp dirs repointed on every platform (regression) ────────────── + + +def test_bypass_env_repoints_all_temp_vars(monkeypatch, tmp_path): + # Windows tempfile honours TEMP/TMP, not TMPDIR; all three must repoint. + monkeypatch.setenv("TEMP", "/host/tmp") + monkeypatch.setenv("TMP", "/host/tmp") + env = _build_bypass_env(str(tmp_path)) + assert env["TMPDIR"] == str(tmp_path) + assert env["TEMP"] == str(tmp_path) + assert env["TMP"] == str(tmp_path) + + +# ── credential-location redirect vars are dropped (regression) ────────── +# Vars that point SDKs at the real home/cache/config (cached tokens), e.g. +# HF_HOME which startup always sets -> the live leak the HOME repoint missed. + + +@pytest.mark.parametrize( + "name", + [ + "HF_HOME", + "HF_HUB_CACHE", + "HUGGINGFACE_HUB_CACHE", + "HF_XET_CACHE", + "TRANSFORMERS_CACHE", + "HF_DATASETS_CACHE", + "XDG_CONFIG_HOME", + "XDG_CACHE_HOME", + "XDG_DATA_HOME", + "NETRC", + "BOTO_CONFIG", + "PIP_CONFIG_FILE", + "CLOUDSDK_CONFIG", + "KAGGLE_CONFIG_DIR", + "DOCKER_CONFIG", + "WANDB_DIR", + "WANDB_CONFIG_DIR", + "NPM_CONFIG_USERCONFIG", + "NPM_CONFIG_GLOBALCONFIG", + "YARN_RC_FILENAME", + "GIT_CONFIG_GLOBAL", + "GIT_CONFIG_SYSTEM", + "CARGO_HOME", + "RCLONE_CONFIG", + "GIT_ASKPASS", + "SSH_ASKPASS", + "BASH_ENV", + "HOMEDRIVE", + "HOMEPATH", + ], +) +def test_cred_location_names_are_flagged(name): + assert _is_cred_location_env_name(name) is True + + +@pytest.mark.parametrize("name", ["PATH", "HOME", "LANG", "PWD", "MY_VAR"]) +def test_benign_names_not_flagged_as_cred_location(name): + assert _is_cred_location_env_name(name) is False + + +def test_bypass_env_drops_hf_home_so_cached_token_unreachable(monkeypatch, tmp_path): + # The live leak: startup sets HF_HOME at the real cache, whose $HF_HOME/token + # holds the operator's token. Repointing HOME does not stop huggingface_hub + # from reading $HF_HOME/token, so HF_HOME must be dropped in bypass mode. + real_cache = tmp_path / "real_hf_cache" + real_cache.mkdir() + (real_cache / "token").write_text("hf_cachedOperatorToken") + monkeypatch.setenv("HF_HOME", str(real_cache)) + monkeypatch.setenv("HF_HUB_CACHE", str(real_cache / "hub")) + env = _build_bypass_env(str(tmp_path)) + assert "HF_HOME" not in env # dropped -> HF falls back to $HOME/.cache (empty) + assert "HF_HUB_CACHE" not in env + + +def test_bypass_env_hf_token_resolves_outside_real_cache(monkeypatch, tmp_path): + # End-to-end: even when HF_HOME and XDG_CACHE_HOME both point at the real + # cache, the bypass env must make huggingface_hub resolve the token under the + # workdir (guards the XDG fallback chain, not just "HF_HOME absent"). + pytest.importorskip("huggingface_hub") + import subprocess + + real_cache = tmp_path / "real_hf" + real_cache.mkdir() + workdir = tmp_path / "sandbox" + workdir.mkdir() + monkeypatch.setenv("HF_HOME", str(real_cache)) + monkeypatch.setenv("XDG_CACHE_HOME", str(real_cache)) + monkeypatch.setenv("XDG_CONFIG_HOME", str(real_cache)) + env = _build_bypass_env(str(workdir)) + token_path = subprocess.run( + [ + sys.executable, + "-c", + "import huggingface_hub.constants as c; print(c.HF_TOKEN_PATH)", + ], + env = env, + capture_output = True, + text = True, + ).stdout.strip() + assert str(real_cache) not in token_path # never the operator's cache + assert token_path.startswith(str(workdir)) # resolved under the sandbox + + +def test_bypass_env_drops_credential_config_path_vars(monkeypatch, tmp_path): + # NETRC / BOTO_CONFIG / PIP_CONFIG_FILE point clients at real credential + # files before $HOME, so they must not survive into the bypassed child. + monkeypatch.setenv("NETRC", "/home/op/.netrc") + monkeypatch.setenv("PGPASSFILE", "/home/op/.pgpass") + monkeypatch.setenv("BOTO_CONFIG", "/home/op/.boto") + monkeypatch.setenv("PIP_CONFIG_FILE", "/home/op/.pip/pip.conf") + env = _build_bypass_env(str(tmp_path)) + assert "NETRC" not in env + assert "PGPASSFILE" not in env + assert "BOTO_CONFIG" not in env + assert "PIP_CONFIG_FILE" not in env + + +def test_bypass_env_strips_npm_auth_and_mysql_pwd(monkeypatch, tmp_path): + # NPM_CONFIG__AUTH (npm _auth, base64) and MYSQL_PWD dodge the URL-value + # check and the PASSWD marker, but must still be dropped. + monkeypatch.setenv("NPM_CONFIG__AUTH", "aGVsbG86c2VjcmV0") + monkeypatch.setenv("MYSQL_PWD", "db-password") + assert _is_secret_env_name("NPM_CONFIG__AUTH") is True + assert _is_secret_env_name("MYSQL_PWD") is True + env = _build_bypass_env(str(tmp_path)) + assert "NPM_CONFIG__AUTH" not in env + assert "MYSQL_PWD" not in env + + +@_POSIX_ONLY +def test_bash_bypass_does_not_source_bash_env(monkeypatch, tmp_path): + # bash -c sources $BASH_ENV for non-interactive shells; an operator startup + # file could re-export stripped secrets, so a real bypass call must not see it. + startup = tmp_path / "startup.sh" + startup.write_text("export RECOVERED=leaked\n") + monkeypatch.setenv("BASH_ENV", str(startup)) + out = _bash_exec("echo R=$RECOVERED", None, 30, "bash-env-test", disable_sandbox = True) + assert "R=leaked" not in out # BASH_ENV dropped -> startup not sourced + assert "R=" in out + + +def test_bypass_env_repoints_windows_profile_vars(monkeypatch, tmp_path): + # On Windows, SDKs read cached creds under USERPROFILE/APPDATA/LOCALAPPDATA, + # not $HOME. Set ones are repointed at the workdir; HOMEDRIVE/HOMEPATH drop. + monkeypatch.setenv("USERPROFILE", "/host/profile") + monkeypatch.setenv("APPDATA", "/host/profile/AppData/Roaming") + monkeypatch.setenv("LOCALAPPDATA", "/host/profile/AppData/Local") + monkeypatch.setenv("HOMEDRIVE", "C:") + monkeypatch.setenv("HOMEPATH", "\\Users\\op") + env = _build_bypass_env(str(tmp_path)) + assert env["USERPROFILE"] == str(tmp_path) + assert env["APPDATA"] == str(tmp_path) + assert env["LOCALAPPDATA"] == str(tmp_path) + assert "HOMEDRIVE" not in env + assert "HOMEPATH" not in env + + +def test_bypass_env_does_not_add_unset_windows_profile_vars(monkeypatch, tmp_path): + # Only repoint Windows profile vars that were actually set (no pollution on + # Linux/macOS where they are absent). + monkeypatch.delenv("USERPROFILE", raising = False) + monkeypatch.delenv("APPDATA", raising = False) + monkeypatch.delenv("LOCALAPPDATA", raising = False) + env = _build_bypass_env(str(tmp_path)) + assert "USERPROFILE" not in env + assert "APPDATA" not in env + assert "LOCALAPPDATA" not in env + + +# ── parent /proc env-leak hardening (regression) ──────────────────── + + +@_POSIX_ONLY +def test_bypass_exec_hardens_parent_proc_env(monkeypatch, captured_popen): + # Stripping the child env is not enough: a same-UID child can read the + # parent's /proc environ. The exec paths must invoke the parent hardening + # when (and only when) the sandbox is disabled. + calls = {"n": 0} + + def fake_harden(): + calls["n"] += 1 + return True + + monkeypatch.setattr(tools, "_harden_parent_against_proc_env_leak", fake_harden) + _python_exec("print(1)", None, 5, "t", disable_sandbox = True) + _bash_exec("echo hi", None, 5, "t", disable_sandbox = True) + assert calls["n"] == 2 + + calls["n"] = 0 + _python_exec("print(1)", None, 5, "t", disable_sandbox = False) + _bash_exec("echo hi", None, 5, "t", disable_sandbox = False) + assert calls["n"] == 0 # never hardened on the sandboxed path + + +def test_bypass_exec_fails_closed_when_hardening_fails(monkeypatch, captured_popen): + # If the parent cannot be hardened (e.g. prctl denied), the unsandboxed + # child must NOT run - otherwise the parent environ stays readable. + monkeypatch.setattr(tools, "_harden_parent_against_proc_env_leak", lambda: False) + out_py = _python_exec("print(1)", None, 5, "t", disable_sandbox = True) + out_sh = _bash_exec("echo hi", None, 5, "t", disable_sandbox = True) + assert "refusing bypass execution" in out_py + assert "refusing bypass execution" in out_sh + assert "cmd" not in captured_popen # never reached Popen + + +@_POSIX_ONLY +def test_proc_env_unreadable_after_hardening(): + # Mechanism check: after hardening, a same-UID child can no longer read the + # parent process /proc environ. Restores the dumpable flag afterwards so the + # process-global state does not leak into later tests. + import subprocess + + if tools._libc is None: + pytest.skip("no libc/prctl available") + pid = os.getpid() + probe = ( + "try:\n" + f" open('/proc/{pid}/environ', 'rb').read()\n" + " print('READABLE')\n" + "except PermissionError:\n" + " print('DENIED')\n" + ) + prev_dumpable = tools._libc.prctl(3, 0, 0, 0, 0) # PR_GET_DUMPABLE + prev_guard = tools._parent_proc_hardened + try: + # Establish a clean readable baseline: another test may have already + # cleared the dumpable flag on this process. + tools._libc.prctl(4, 1, 0, 0, 0) # PR_SET_DUMPABLE = 1 + before = subprocess.run( + [sys.executable, "-c", probe], capture_output = True, text = True + ).stdout.strip() + if before != "READABLE": + pytest.skip("/proc already restricted in this environment") + + tools._parent_proc_hardened = False + assert tools._harden_parent_against_proc_env_leak() is True + + after = subprocess.run( + [sys.executable, "-c", probe], capture_output = True, text = True + ).stdout.strip() + assert after == "DENIED" + finally: + if prev_dumpable in (0, 1): + try: + tools._libc.prctl(4, prev_dumpable, 0, 0, 0) + except (OSError, AttributeError): + pass + tools._parent_proc_hardened = prev_guard + + +# ── Anthropic request model declares the field (regression) ───────── + + +def test_anthropic_request_model_bypass_default_false(): + # Omitting the field on the Anthropic path must default to False rather than + # raising AttributeError (extra='allow' does not set absent attributes). + from models.inference import AnthropicMessagesRequest + + assert AnthropicMessagesRequest.model_fields["bypass_permissions"].default is False + req = AnthropicMessagesRequest(model = "x", messages = [], max_tokens = 8) + assert bool(req.bypass_permissions) is False diff --git a/studio/backend/tests/test_safetensors_tool_loop.py b/studio/backend/tests/test_safetensors_tool_loop.py index 12731783a0..024d1801c0 100644 --- a/studio/backend/tests/test_safetensors_tool_loop.py +++ b/studio/backend/tests/test_safetensors_tool_loop.py @@ -208,6 +208,7 @@ class FakeExecuteTool: timeout = None, session_id = None, rag_scope = None, + disable_sandbox = False, ): self.calls.append((name, arguments)) result = self.results.pop(0) if self.results else "OK" diff --git a/studio/backend/tests/test_tool_confirm_loop.py b/studio/backend/tests/test_tool_confirm_loop.py index ce7852c95f..17ef697674 100644 --- a/studio/backend/tests/test_tool_confirm_loop.py +++ b/studio/backend/tests/test_tool_confirm_loop.py @@ -43,6 +43,7 @@ class _FakeExecuteTool: timeout = None, session_id = None, rag_scope = None, + disable_sandbox = False, ): self.calls.append((name, arguments)) return f"RESULT[{name}]" diff --git a/studio/frontend/src/components/assistant-ui/thread.tsx b/studio/frontend/src/components/assistant-ui/thread.tsx index 839c44d940..ec252b74a8 100644 --- a/studio/frontend/src/components/assistant-ui/thread.tsx +++ b/studio/frontend/src/components/assistant-ui/thread.tsx @@ -67,6 +67,7 @@ import { parseExternalModelId } from "@/features/chat/external-providers"; import { McpComposerButton } from "@/features/chat/mcp-composer-button"; import { getExternalReasoningCapabilities } from "@/features/chat/provider-capabilities"; import { useRagToolDisabled } from "@/features/chat/hooks/use-rag-tool-disabled"; +import { BypassPermissionsMenuItem } from "@/features/chat/bypass-permissions-menu-item"; import { useChatRuntimeStore } from "@/features/chat/stores/chat-runtime-store"; import { useExternalProvidersStore } from "@/features/chat/stores/external-providers-store"; import { @@ -1223,6 +1224,9 @@ const Composer: FC<{ data-pill-compact={pillsCompact ? "true" : undefined} > + {/* Active-mode badge: always visible when bypass is on, even while + the pill row is collapsed (returns null when off). */} + {composerExpanded ? ( <> @@ -1931,6 +1935,30 @@ const ArtifactsToggle: FC = () => { ); }; +// Red pill shown while Bypass Permissions is on; click to turn it off. +// Mirror of shared-composer's badge so both composers surface the state. +const BypassPermissionsToggle: FC = () => { + const bypassPermissions = useChatRuntimeStore((s) => s.bypassPermissions); + const setBypassPermissions = useChatRuntimeStore( + (s) => s.setBypassPermissions, + ); + if (!bypassPermissions) return null; + return ( + + ); +}; + const ToolStatusDisplay: FC = () => { const toolStatus = useChatRuntimeStore((s) => s.toolStatus); const isThreadRunning = useAuiState(({ thread }) => thread.isRunning); @@ -2276,6 +2304,7 @@ const ComposerToolsMenu: FC<{ side?: "top" | "bottom" }> = ({ ) : null} ), + bypassPermissions: , projects: ( diff --git a/studio/frontend/src/features/chat/api/chat-adapter.ts b/studio/frontend/src/features/chat/api/chat-adapter.ts index 6122b51dfd..c1a618862e 100644 --- a/studio/frontend/src/features/chat/api/chat-adapter.ts +++ b/studio/frontend/src/features/chat/api/chat-adapter.ts @@ -1551,6 +1551,7 @@ export function createOpenAIStreamAdapter(): ChatModelAdapter { artifactsEnabled, mcpEnabledForChat, confirmToolCalls, + bypassPermissions, webFetchToolsEnabled, ragEnabled, ragSource, @@ -2483,7 +2484,10 @@ export function createOpenAIStreamAdapter(): ChatModelAdapter { : []), ], mcp_enabled: mcpEnabledForChat, - confirm_tool_calls: confirmToolCalls, + // Bypass Permissions wins: never request the confirm gate + // while bypassing, and tell the backend to drop the sandbox. + confirm_tool_calls: confirmToolCalls && !bypassPermissions, + bypass_permissions: bypassPermissions, // Scope: thread_id = this thread's docs, kb_id = a KB, // project_id = the thread's project sources (auto-on whenever // the project has indexed sources, no Docs pill needed). diff --git a/studio/frontend/src/features/chat/bypass-permissions-menu-item.tsx b/studio/frontend/src/features/chat/bypass-permissions-menu-item.tsx new file mode 100644 index 0000000000..824d946d06 --- /dev/null +++ b/studio/frontend/src/features/chat/bypass-permissions-menu-item.tsx @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: AGPL-3.0-only +// Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 + +import { ShieldOffIcon } from "lucide-react"; +import { useState } from "react"; + +import { HugeiconsIcon } from "@hugeicons/react"; + +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@/components/ui/alert-dialog"; +import { DropdownMenuItem } from "@/components/ui/dropdown-menu"; +import { useChatRuntimeStore } from "@/features/chat/stores/chat-runtime-store"; +import { Tick02Icon } from "@/lib/tick-icon"; + +// "Bypass Permissions" entry for the composer "+" -> More menu. Mirrors the +// settings toggle: enabling demands the danger warning, disabling is immediate. +// onSelect preventDefault keeps the menu mounted so the warning dialog (which +// lives in this same fragment) survives instead of unmounting with the menu. +export function BypassPermissionsMenuItem() { + const bypassPermissions = useChatRuntimeStore((s) => s.bypassPermissions); + const setBypassPermissions = useChatRuntimeStore( + (s) => s.setBypassPermissions, + ); + const [dialogOpen, setDialogOpen] = useState(false); + + return ( + <> + { + if (bypassPermissions) { + setBypassPermissions(false); + } else { + e.preventDefault(); + setDialogOpen(true); + } + }} + > + + Bypass Permissions + {bypassPermissions ? ( + + ) : null} + + + + + Enable Bypass Permissions? + + Bypass Permissions is dangerous since the AI model might delete, + corrupt your machine, and or cause real world damage to you or the + world - only accept if you are certain + + + + Cancel + { + setBypassPermissions(true); + setDialogOpen(false); + }} + > + I understand + + + + + + ); +} diff --git a/studio/frontend/src/features/chat/chat-settings-sheet.tsx b/studio/frontend/src/features/chat/chat-settings-sheet.tsx index c69265bf59..d6b5d658da 100644 --- a/studio/frontend/src/features/chat/chat-settings-sheet.tsx +++ b/studio/frontend/src/features/chat/chat-settings-sheet.tsx @@ -6,6 +6,16 @@ import { AlertDescription, AlertTitle, } from "@/components/ui/alert"; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@/components/ui/alert-dialog"; import { Button } from "@/components/ui/button"; import { Dialog, @@ -1527,6 +1537,7 @@ export function ChatSettingsPanel({
+
@@ -1713,27 +1724,99 @@ function AutoHealToolCallsToggle() { function ConfirmToolCallsToggle() { const confirmToolCalls = useChatRuntimeStore((s) => s.confirmToolCalls); const setConfirmToolCalls = useChatRuntimeStore((s) => s.setConfirmToolCalls); + const bypassPermissions = useChatRuntimeStore((s) => s.bypassPermissions); return (
-
- - Confirm tool calls - - - When on, local Studio tool calls pause for your approval before they - run. Provider-hosted tools are not gated here. - +
+
+ + Confirm tool calls + + + When on, local Studio tool calls pause for your approval before they + run. Provider-hosted tools are not gated here. + +
+ {bypassPermissions ? ( + + Overridden by Bypass Permissions + + ) : null}
); } +function BypassPermissionsToggle() { + const bypassPermissions = useChatRuntimeStore((s) => s.bypassPermissions); + const setBypassPermissions = useChatRuntimeStore( + (s) => s.setBypassPermissions, + ); + const [dialogOpen, setDialogOpen] = useState(false); + + return ( +
+
+
+ + Bypass Permissions + + + Dangerous. Runs every tool call with no confirmation and disables + the python/terminal sandbox. Environment secrets are stripped, but + code can still read files and credentials on your machine. + +
+ { + if (next) setDialogOpen(true); + else setBypassPermissions(false); + }} + /> +
+ {bypassPermissions ? ( + + Tool calls run with no confirmation and no sandbox. + + ) : null} + + + + Enable Bypass Permissions? + + Bypass Permissions is dangerous since the AI model might delete, + corrupt your machine, and or cause real world damage to you or the + world - only accept if you are certain + + + + Cancel + { + setBypassPermissions(true); + setDialogOpen(false); + }} + > + I understand + + + + +
+ ); +} + function ChatTemplateFields() { const defaultTemplate = useChatRuntimeStore((s) => s.defaultChatTemplate); const override = useChatRuntimeStore((s) => s.chatTemplateOverride); diff --git a/studio/frontend/src/features/chat/shared-composer.tsx b/studio/frontend/src/features/chat/shared-composer.tsx index 8f547f8307..40063705ab 100644 --- a/studio/frontend/src/features/chat/shared-composer.tsx +++ b/studio/frontend/src/features/chat/shared-composer.tsx @@ -59,6 +59,7 @@ import { } from "./prompt-storage/prompt-storage-dialog"; import { listPromptEntries, type PromptEntry } from "./api/prompts-api"; import { McpComposerButton } from "./mcp-composer-button"; +import { BypassPermissionsMenuItem } from "./bypass-permissions-menu-item"; import { KnowledgeBaseComposerButton } from "@/features/rag/components/knowledge-base-composer-button"; import { NewProjectDialog } from "./components/new-project-dialog"; import { useChatProjects } from "./hooks/use-chat-projects"; @@ -533,6 +534,10 @@ export function SharedComposer({ const setWebFetchToolsEnabled = useChatRuntimeStore( (s) => s.setWebFetchToolsEnabled, ); + const bypassPermissions = useChatRuntimeStore((s) => s.bypassPermissions); + const setBypassPermissions = useChatRuntimeStore( + (s) => s.setBypassPermissions, + ); const ragEnabled = useChatRuntimeStore((s) => s.ragEnabled); const setRagEnabled = useChatRuntimeStore((s) => s.setRagEnabled); const activeThreadId = useChatRuntimeStore((s) => s.activeThreadId); @@ -1240,6 +1245,7 @@ export function SharedComposer({ ) : null} ), + bypassPermissions: , projects: ( @@ -1684,6 +1690,20 @@ export function SharedComposer({ ) : null} {mcpEnabledForChat ? : null} + {bypassPermissions && ( + + )}
{/* mr-0.5 matches the send button inset from the edge in normal chat; gap-1.5 matches its control spacing. */} diff --git a/studio/frontend/src/features/chat/stores/chat-runtime-store.ts b/studio/frontend/src/features/chat/stores/chat-runtime-store.ts index c2f3578803..003a32ddb1 100644 --- a/studio/frontend/src/features/chat/stores/chat-runtime-store.ts +++ b/studio/frontend/src/features/chat/stores/chat-runtime-store.ts @@ -35,6 +35,7 @@ export const CHAT_ALLOW_ARTIFACT_NETWORK_ACCESS_KEY = "unsloth_chat_allow_artifact_network_access"; export const CHAT_MCP_ENABLED_KEY = "unsloth_chat_mcp_enabled"; export const CHAT_CONFIRM_TOOL_CALLS_KEY = "unsloth_chat_confirm_tool_calls"; +export const CHAT_BYPASS_PERMISSIONS_KEY = "unsloth_chat_bypass_permissions"; export const CHAT_WEB_FETCH_TOOLS_ENABLED_KEY = "unsloth_chat_web_fetch_tools_enabled"; export const CHAT_RAG_SOURCE_KEY = "unsloth_chat_rag_source"; @@ -487,6 +488,12 @@ type ChatRuntimeStore = { * chat before they run. */ confirmToolCalls: boolean; + /** + * Bypass Permissions: when on, tool calls run with no confirmation gate + * AND the python/terminal execution sandbox is disabled on the backend + * (secrets are still stripped). Takes precedence over confirmToolCalls. + */ + bypassPermissions: boolean; /** * Per-chat set of tool names the user chose to auto-approve via "Always * allow". Keyed by UI confirmation scope, not necessarily the backend @@ -606,6 +613,7 @@ type ChatRuntimeStore = { setAllowArtifactNetworkAccess: (enabled: boolean) => void; setMcpEnabledForChat: (enabled: boolean) => void; setConfirmToolCalls: (enabled: boolean) => void; + setBypassPermissions: (enabled: boolean) => void; allowToolAlways: (sessionId: string, toolName: string) => void; setToolConfirmation: ( toolCallId: string, @@ -883,6 +891,10 @@ export const useChatRuntimeStore = create((set, get) => ({ ), mcpEnabledForChat: loadBool(CHAT_MCP_ENABLED_KEY, false), confirmToolCalls: loadBool(CHAT_CONFIRM_TOOL_CALLS_KEY, false), + // Never restore Bypass Permissions from storage: it disables the sandbox and + // the confirmation gate, so it must be re-enabled (through the warning + // dialog) each session rather than silently reactivating on reload. + bypassPermissions: false, alwaysAllowToolsBySession: new Map>(), toolConfirmations: {}, webFetchToolsEnabled: loadBool(CHAT_WEB_FETCH_TOOLS_ENABLED_KEY, false), @@ -1225,6 +1237,10 @@ export const useChatRuntimeStore = create((set, get) => ({ saveBool(CHAT_CONFIRM_TOOL_CALLS_KEY, confirmToolCalls); return { confirmToolCalls }; }), + setBypassPermissions: (bypassPermissions) => + // Deliberately not persisted (see init): a reload must not silently keep + // the sandbox/confirmation bypass active without re-accepting the warning. + set(() => ({ bypassPermissions })), allowToolAlways: (sessionId, toolName) => set((state) => { const current = state.alwaysAllowToolsBySession.get(sessionId); diff --git a/studio/frontend/src/features/chat/stores/plus-menu-prefs-store.ts b/studio/frontend/src/features/chat/stores/plus-menu-prefs-store.ts index f94e3814e6..822c66eeb5 100644 --- a/studio/frontend/src/features/chat/stores/plus-menu-prefs-store.ts +++ b/studio/frontend/src/features/chat/stores/plus-menu-prefs-store.ts @@ -14,7 +14,8 @@ export type PlusMenuItemId = | "compareChat" | "exportChat" | "canvas" - | "projects"; + | "projects" + | "bypassPermissions"; // Canonical order used both for the pinned items at the top level and for the // items that fall into the "More" overflow submenu. @@ -26,6 +27,7 @@ export const PLUS_MENU_ORDER: PlusMenuItemId[] = [ "exportChat", "canvas", "projects", + "bypassPermissions", ]; // Defaults reproduce the historical layout: Chat with Files, MCP and Projects @@ -38,6 +40,8 @@ const DEFAULT_PINS: Record = { compareChat: false, exportChat: false, canvas: false, + // Lives under "More" by default; it is a rarely toggled, dangerous mode. + bypassPermissions: false, }; export interface PlusMenuPrefsState { diff --git a/studio/frontend/src/index.css b/studio/frontend/src/index.css index d0ca2d2426..118d5f9383 100644 --- a/studio/frontend/src/index.css +++ b/studio/frontend/src/index.css @@ -1046,6 +1046,11 @@ .composer-pill-btn[data-active="true"] { color: var(--primary); } + /* Bypass Permissions badge: red, always-visible active warning. */ + .composer-pill-btn[data-variant="danger"] { + @apply bg-destructive/10 hover:bg-destructive/15; + color: var(--destructive); + } /* With more than 4 tools on, drop pill labels to icons only to cut clutter. Compare keeps its label via data-keep-label. */ From 40c8ad78b94e904e698190d2c47ec231f9daacdd Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Mon, 15 Jun 2026 04:18:15 -0700 Subject: [PATCH 04/91] Studio: add --secure Cloudflare-only mode and revamp API usage examples (#6300) * Studio: add --secure Cloudflare-only mode and revamp API usage examples --secure / --not-secure on `unsloth studio` and `unsloth studio run`: - --secure binds 127.0.0.1, requires the Cloudflare tunnel, and advertises only the Cloudflare link. cloudflared reaches the server over localhost, so the raw port is never exposed on a public interface. - If the tunnel cannot start, fail closed with a clear message instead of silently leaving a raw 0.0.0.0 link. - Default stays not-secure (no behavior change); coexists with the existing --cloudflare/--no-cloudflare flag. Host defaults are unchanged. - /api/health (authed) now reports the live tunnel URL. API usage examples (Profile > API): - Example tabs for curl, Python, curl + tools, Python + tools, plus an OS row (Linux/macOS/WSL vs Windows) auto-detected from the platform. - Windows curl passes the JSON body via a file so PowerShell does not strip the quotes when calling curl.exe. - Python + tools forwards enable_tools/enabled_tools through extra_body and guards chunk.choices, since tool-lifecycle events carry no choices. - Shows the loaded model name and the real API key while it is still revealed. - A Cloudflare Tunnel toggle (default on) shows the public tunnel URL and uses it as the base_url in the examples when a tunnel is running. Tests cover the tunnel start gate and the --secure flag on both commands. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: gate --secure tools on public exposure and harden API examples In secure mode the server binds loopback but is reachable via the public Cloudflare tunnel, so resolve the tool policy against the public exposure (0.0.0.0) rather than the loopback bind. This keeps server-side tools off by default and prompts before enabling them, instead of inheriting the loopback default of on. The startup tool notice now names the public surface. Also reject --secure with --no-cloudflare directly in run_server and the run.py argparse (not only the CLI), JSON-encode interpolated model names so Windows paths and quotes cannot produce invalid JSON or broken snippets, and force-refresh /api/health on the API panel so a tunnel that starts after the first health read still surfaces its URL. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: API examples show direct host when tunnel toggle is off; move Copy onto code The Cloudflare Tunnel toggle had no visible effect when Studio was opened through the tunnel: the off state fell back to window.location.origin, which equals the tunnel URL in that case. /api/health now reports the direct host:port (server_url), and the API panel uses it for the off state so it shows the real non-tunnel base. Also move the Copy button out of the tab row and onto the code block. * Studio: highlight API examples, add advanced tabs, fix tunnel toggle row Syntax-highlight the curl/PowerShell/Python snippets with the app's shared shiki plugin (bash/powershell/python). Add 'curl + advanced' and 'Python + advanced' tabs that set temperature/top_p/top_k/min_p/ repetition_penalty/max_tokens, enable thinking, and turn on all tools. The Cloudflare Tunnel row no longer shifts the code block: the tunnel URL is always rendered (dimmed when off) so toggling keeps the row height constant. Key the highlighted block on its content so it remounts when only the base URL changes (the renderer's block memo otherwise kept a stale URL). * Studio: rename API tunnel toggle to Secure HTTPS, hint --secure when exposed Rename the API examples toggle from Cloudflare Tunnel to Secure HTTPS. When the server was not launched with --secure, show an info tooltip noting the raw 0.0.0.0 port is still globally reachable and pointing at --secure. /api/health now reports whether --secure was used so the hint is hidden in secure mode. * Studio: force tools off for plain network/secure launches The plain 'unsloth studio --secure' (and '-H 0.0.0.0') launcher re-execs run.py and never installed a tool policy, so the process default (honor per-request enable_tools) let any API-key holder run Python/terminal tools over the public endpoint. Force the policy off at the run.py entrypoint when network-reachable (0.0.0.0 or --secure); 'unsloth studio run' still installs its own resolved policy and does not go through this path. * Studio: apply default tool policy in run_server, not the run.py entrypoint The plain launcher runs from the studio venv and calls run_server directly, so it never hit the run.py __main__ guard. Move the network/secure default-off tool policy into run_server so every launch path (plain, --secure, direct run.py) gets it; the run subcommand still overrides it with its resolved policy. * Studio: clarify --secure help text on the network exposure tradeoff Spell out in --help (both unsloth studio and unsloth studio run, plus the run.py argparse) that --not-secure also serves the raw 0.0.0.0 port reachable from anywhere on the network, matching the API panel's Secure HTTPS hint. * Studio: cache API-key PBKDF2 derivation to cut per-request /v1 auth overhead validate_api_key re-ran the 100k-round PBKDF2 on every authenticated request, adding ~15ms to each /v1 call made with an sk-unsloth- key. Benchmarked against the bare llama-server it proxies to, API-key requests carried ~22ms of fixed overhead vs ~7ms for the JWT path; the gap was entirely this redundant key derivation (Pydantic validation measured 0.005ms, so it is not a factor). The raw-key to hash mapping is a pure deterministic function of the fixed server salt, so memoize it per process, keyed by a salted HMAC of the key (never the key or a recoverable digest). The cached value equals what is already stored at rest. Revocation and expiry remain enforced by the SQLite read on every call, so a cache hit only skips the KDF, never the active or expiry checks. Only keys that exist in the DB are cached, so unknown-key spam cannot grow it. After the change the API-key /v1 overhead drops to ~8ms, at parity with JWT, while the at-rest PBKDF2 hashing is unchanged. Adds test_api_key_expiry.py covering API-key and JWT expiry enforcement and the new cache: it skips the KDF on repeat and still rejects revoked or expired keys. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: tighten comments across the secure-tunnel and API-key changes Condense multi-line comments and docstrings to one or two lines, drop the ones that restate obvious code, and remove an orphaned test section header. Comment-only: verified with comment_tools.py check (9/9 code unchanged), the auth/secure-tunnel/CLI test suites, and a clean frontend typecheck and build. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- studio/backend/auth/storage.py | 35 +- studio/backend/main.py | 4 + studio/backend/run.py | 94 +++- studio/backend/tests/test_api_key_expiry.py | 198 +++++++ .../backend/tests/test_secure_tunnel_gate.py | 95 ++++ studio/frontend/src/config/env.ts | 26 +- .../settings/components/usage-examples.tsx | 486 ++++++++++++++++-- .../features/settings/tabs/api-keys-tab.tsx | 2 +- studio/frontend/src/i18n/locales/en.ts | 10 + studio/frontend/src/i18n/locales/zh-CN.ts | 10 + unsloth_cli/commands/studio.py | 93 +++- unsloth_cli/tests/test_studio_secure_flag.py | 294 +++++++++++ 12 files changed, 1276 insertions(+), 71 deletions(-) create mode 100644 studio/backend/tests/test_api_key_expiry.py create mode 100644 studio/backend/tests/test_secure_tunnel_gate.py create mode 100644 unsloth_cli/tests/test_studio_secure_flag.py diff --git a/studio/backend/auth/storage.py b/studio/backend/auth/storage.py index fa5b985513..ee6678d9b9 100644 --- a/studio/backend/auth/storage.py +++ b/studio/backend/auth/storage.py @@ -4,9 +4,11 @@ """SQLite storage for auth data (user credentials + JWT secret).""" import hashlib +import hmac import os import secrets import sqlite3 +import threading from datetime import datetime, timezone from typing import Optional, Tuple @@ -236,6 +238,29 @@ def _pbkdf2_desktop_secret(raw_secret: str) -> str: return _pbkdf2_api_key(raw_secret) +# Memoize the deterministic raw-key -> PBKDF2-hash derivation so the 100k-round +# KDF runs once per key instead of on every authenticated request. Keyed by a +# salted HMAC of the key (not the key itself); revocation/expiry are still +# enforced by the SQLite read on every call, so a cache hit only skips the KDF. +# Only keys present in the DB are cached, so unknown-key spam can't grow it. +_api_key_hash_cache: dict[str, str] = {} +_API_KEY_HASH_CACHE_MAX = 4096 +_api_key_hash_cache_lock = threading.Lock() + + +def _api_key_cache_id(raw_key: str) -> str: + """Cache id for a raw key: salted HMAC-SHA256 (not the key itself).""" + return hmac.new( + _get_or_create_api_key_pbkdf2_salt(), raw_key.encode("utf-8"), hashlib.sha256 + ).hexdigest() + + +def _reset_api_key_hash_cache() -> None: + """Drop memoized derivations (tests / salt change).""" + with _api_key_hash_cache_lock: + _api_key_hash_cache.clear() + + def is_initialized() -> bool: """Check if auth is ready for login (at least one user exists in DB).""" conn = get_connection() @@ -704,7 +729,9 @@ def validate_api_key(raw_key: str) -> Optional[str]: Also updates ``last_used_at`` on success. """ - key_hash = _pbkdf2_api_key(raw_key) + cache_id = _api_key_cache_id(raw_key) + cached_hash = _api_key_hash_cache.get(cache_id) + key_hash = cached_hash if cached_hash is not None else _pbkdf2_api_key(raw_key) conn = get_connection() try: cur = conn.execute( @@ -714,6 +741,12 @@ def validate_api_key(raw_key: str) -> Optional[str]: row = cur.fetchone() if row is None: return None + # Real key: memoize so later requests skip the KDF. Bounded; clear on overflow. + if cached_hash is None: + with _api_key_hash_cache_lock: + if len(_api_key_hash_cache) >= _API_KEY_HASH_CACHE_MAX: + _api_key_hash_cache.clear() + _api_key_hash_cache[cache_id] = key_hash if not row["is_active"]: return None if row["expires_at"] is not None: diff --git a/studio/backend/main.py b/studio/backend/main.py index 064e261753..2022074c08 100644 --- a/studio/backend/main.py +++ b/studio/backend/main.py @@ -873,6 +873,10 @@ async def health_check(request: Request): "version": UNSLOTH_VERSION, "studio_version": STUDIO_VERSION, "device_type": device_type, + # API-screen fields (authed-only; they fingerprint how the host is exposed). + "cloudflare_url": getattr(request.app.state, "cloudflare_url", None), + "server_url": getattr(request.app.state, "server_url", None), + "secure": bool(getattr(request.app.state, "secure", False)), } diff --git a/studio/backend/run.py b/studio/backend/run.py index 18e96f11c4..a9d1f1db26 100644 --- a/studio/backend/run.py +++ b/studio/backend/run.py @@ -395,7 +395,23 @@ def _verify_global_reachability(display_host: str, port: int) -> None: pass -def _emit_startup_output(host: str, port: int, display_host: str) -> None: +def _emit_secure_startup_output(port: int) -> None: + """Secure-mode banner: only the Cloudflare link (loopback has no public raw URL).""" + print("") + print("🦥 Unsloth Studio is running (secure)") + print("─" * 52) + _print_cloudflare_line() + print(f" On this machine only: http://127.0.0.1:{port}/") + print("─" * 52) + print_studio_stop_hint() + + +def _emit_startup_output( + host: str, + port: int, + display_host: str, + secure: bool = False, +) -> None: """Print the access banner plus any post-startup warnings. Extracted from ``_run`` so the banner/warning wiring is testable. The @@ -404,6 +420,9 @@ def _emit_startup_output(host: str, port: int, display_host: str) -> None: non-127.0.0.1 bind, and wildcard binds are never 127.0.0.1), so the trailing stop hint is emitted exactly once. """ + if secure: + _emit_secure_startup_output(port) + return wildcard_bind = host in ("0.0.0.0", "::") localhost_mismatch_url = _localhost_ipv6_mismatch_url(host, port) # For wildcard binds, run the reachability check between the URL @@ -811,6 +830,25 @@ def _setup_server_disk_logging(): return log_path +def _cloudflare_tunnel_should_start( + *, cloudflare: bool, host: str, secure: bool, api_only: bool, is_colab: bool +) -> bool: + """Whether to start the Cloudflare tunnel. --secure tunnels a loopback bind too; + non-secure keeps the 0.0.0.0-only rule. Colab/api-only never tunnel.""" + return cloudflare and (host == "0.0.0.0" or secure) and not api_only and not is_colab + + +def _apply_default_tool_policy(host: str, secure: bool) -> None: + """Force server-side tools off on network-reachable launches (0.0.0.0 or --secure) + so a public endpoint can't run code via a client's `enable_tools`. `unsloth studio + run` installs its own resolved policy and bypasses this.""" + if not (secure or host == "0.0.0.0"): + return + from state.tool_policy import set_tool_policy + + set_tool_policy(False) + + def run_server( host: str = "127.0.0.1", port: int = 8888, @@ -819,6 +857,7 @@ def run_server( api_only: bool = False, llama_parallel_slots: int = 1, cloudflare: bool = True, + secure: bool = False, ): """ Start the FastAPI server. @@ -837,6 +876,18 @@ def run_server( """ global _server, _shutdown_event + # --secure exposes only the Cloudflare link: force a loopback bind so the raw + # port is never public (even with -H 0.0.0.0), and reject the contradictory combo. + if secure and not cloudflare: + raise SystemExit( + "A secure Cloudflare link is not allowed, use --not-secure which provides a 0.0.0.0 link" + ) + if secure: + host = "127.0.0.1" + + # `unsloth studio run` overrides this afterward with its resolved policy. + _apply_default_tool_policy(host, secure) + # Windows cp1252 can't encode emoji; reconfigure stdout to UTF-8. if sys.platform == "win32" and hasattr(sys.stdout, "reconfigure"): try: @@ -975,6 +1026,13 @@ def run_server( # backend, not whatever a proxy/tunnel exposed. For ephemeral binds (port==0) # leave it unset so handlers fall back to the request scope / base_url. app.state.server_port = port if port and port > 0 else None + # Direct (non-tunnel) base for the API panel; resolve 0.0.0.0 to the LAN IP. + if port and port > 0: + _direct_host = _resolve_external_ip() if host == "0.0.0.0" else host + app.state.server_url = f"http://{_direct_host}:{port}" + else: + app.state.server_url = None + app.state.secure = secure app.state.llama_parallel_slots = llama_parallel_slots # Expose a shutdown callable before the server accepts requests so @@ -1036,7 +1094,13 @@ def run_server( global _cloudflare_url _cloudflare_url = None app.state.cloudflare_url = None - _cloudflare_enabled = cloudflare and host == "0.0.0.0" and not api_only and not _IS_COLAB + _cloudflare_enabled = _cloudflare_tunnel_should_start( + cloudflare = cloudflare, + host = host, + secure = secure, + api_only = api_only, + is_colab = _IS_COLAB, + ) if _cloudflare_enabled: try: # best-effort: any failure must not block startup from cloudflare_tunnel import start_studio_tunnel, stop_studio_tunnel @@ -1049,8 +1113,19 @@ def run_server( except Exception as e: logger.debug("Cloudflare tunnel skipped: %s", e) + # --secure fails closed: no tunnel means no public link, so exit rather than + # silently fall back to a raw port. + if secure and not _cloudflare_url: + print( + "A secure Cloudflare link is not allowed, use --not-secure which provides a 0.0.0.0 link", + file = sys.stderr, + flush = True, + ) + _graceful_shutdown(_server) + sys.exit(1) + if not silent: - _emit_startup_output(host, port, display_host) + _emit_startup_output(host, port, display_host, secure = secure) return app @@ -1094,6 +1169,14 @@ if __name__ == "__main__": help = "Auto-create a free Cloudflare HTTPS tunnel when bound to 0.0.0.0 " "(default on; --no-cloudflare to disable)", ) + parser.add_argument( + "--secure", + action = argparse.BooleanOptionalAction, + default = False, + help = "Expose ONLY a Cloudflare HTTPS link: bind localhost and fail closed " + "if the tunnel can't start. Without it, --not-secure also serves the raw " + "0.0.0.0 port, which is reachable from anywhere on the network", + ) # Mirror unsloth_cli/commands/studio.py's _PARALLEL_*. Default 1 is for direct # backend launches; `unsloth studio run` always passes its own value (4). _PARALLEL_MIN = 1 @@ -1113,6 +1196,10 @@ if __name__ == "__main__": args = parser.parse_args() if not _PARALLEL_MIN <= args.parallel <= _PARALLEL_MAX: parser.error(f"--parallel must be between {_PARALLEL_MIN} and {_PARALLEL_MAX}") + if args.secure and not args.cloudflare: + parser.error( + "--secure requires the Cloudflare tunnel; do not combine it with --no-cloudflare" + ) kwargs = dict( host = args.host, @@ -1121,6 +1208,7 @@ if __name__ == "__main__": api_only = args.api_only, llama_parallel_slots = args.parallel, cloudflare = args.cloudflare, + secure = args.secure, ) if args.frontend is not None: kwargs["frontend_path"] = Path(args.frontend) diff --git a/studio/backend/tests/test_api_key_expiry.py b/studio/backend/tests/test_api_key_expiry.py new file mode 100644 index 0000000000..0dacb4c61e --- /dev/null +++ b/studio/backend/tests/test_api_key_expiry.py @@ -0,0 +1,198 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 + +"""Expiry enforcement for API keys (tz-aware ``expires_at``) and JWT access +tokens (``exp`` claim). Both must surface as 401 on protected routes.""" + +from __future__ import annotations + +import asyncio +import secrets +from datetime import datetime, timedelta, timezone + +import pytest +from fastapi import HTTPException +from fastapi.security import HTTPAuthorizationCredentials + +from auth import storage +from auth.authentication import create_access_token, get_current_subject + + +@pytest.fixture(autouse = True) +def isolated_auth_db(tmp_path, monkeypatch): + monkeypatch.setattr(storage, "DB_PATH", tmp_path / "auth.db") + monkeypatch.setattr(storage, "_BOOTSTRAP_PW_PATH", tmp_path / ".bootstrap_password") + monkeypatch.setattr(storage, "_bootstrap_password", None) + monkeypatch.setattr(storage, "_api_key_pbkdf2_salt_cache", None) + storage._reset_api_key_hash_cache() + yield + storage._reset_api_key_hash_cache() + + +def seed_user(): + storage.create_initial_user( + username = storage.DEFAULT_ADMIN_USERNAME, + password = "human-password-123", + jwt_secret = secrets.token_urlsafe(64), + ) + + +def iso_from_now(**delta): + return (datetime.now(timezone.utc) + timedelta(**delta)).isoformat() + + +def make_key(expires_at): + raw, _row = storage.create_api_key( + username = storage.DEFAULT_ADMIN_USERNAME, + name = "test", + expires_at = expires_at, + ) + return raw + + +def subject_of(token): + """Run the real FastAPI auth dependency against a bearer token.""" + credentials = HTTPAuthorizationCredentials(scheme = "Bearer", credentials = token) + return asyncio.run(get_current_subject(credentials)) + + +# --- validate_api_key (storage layer) --------------------------------------- + + +def test_unexpired_key_validates(): + seed_user() + assert ( + storage.validate_api_key(make_key(iso_from_now(days = 1))) == storage.DEFAULT_ADMIN_USERNAME + ) + + +def test_never_expiring_key_validates(): + seed_user() + assert storage.validate_api_key(make_key(None)) == storage.DEFAULT_ADMIN_USERNAME + + +def test_expired_key_rejected(): + seed_user() + assert storage.validate_api_key(make_key(iso_from_now(seconds = -1))) is None + + +def test_key_expiring_far_in_past_rejected(): + seed_user() + assert storage.validate_api_key(make_key(iso_from_now(days = -30))) is None + + +def test_revoked_key_rejected(): + seed_user() + raw, row = storage.create_api_key( + username = storage.DEFAULT_ADMIN_USERNAME, + name = "doomed", + expires_at = iso_from_now(days = 1), + ) + storage.revoke_api_key(storage.DEFAULT_ADMIN_USERNAME, int(row["id"])) + assert storage.validate_api_key(raw) is None + + +def test_unknown_key_rejected(): + seed_user() + assert storage.validate_api_key(storage.API_KEY_PREFIX + secrets.token_hex(16)) is None + + +# --- get_current_subject (route dependency) --------------------------------- + + +def test_dependency_accepts_unexpired_key(): + seed_user() + assert subject_of(make_key(iso_from_now(days = 1))) == storage.DEFAULT_ADMIN_USERNAME + + +def test_dependency_rejects_expired_key_as_401(): + seed_user() + with pytest.raises(HTTPException) as exc: + subject_of(make_key(iso_from_now(seconds = -1))) + assert exc.value.status_code == 401 + assert exc.value.detail == "Invalid or expired API key" + + +# --- JWT access-token expiry ------------------------------------------------ + + +def test_dependency_accepts_unexpired_jwt(): + seed_user() + token = create_access_token(storage.DEFAULT_ADMIN_USERNAME, timedelta(minutes = 5)) + assert subject_of(token) == storage.DEFAULT_ADMIN_USERNAME + + +def test_dependency_rejects_expired_jwt_as_401(): + seed_user() + token = create_access_token(storage.DEFAULT_ADMIN_USERNAME, timedelta(seconds = -1)) + with pytest.raises(HTTPException) as exc: + subject_of(token) + assert exc.value.status_code == 401 + assert exc.value.detail == "Invalid or expired token" + + +# --- derivation cache: speeds repeats without bypassing checks -------------- + + +def test_cache_skips_pbkdf2_on_repeat(monkeypatch): + seed_user() + raw = make_key(iso_from_now(days = 1)) + assert storage.validate_api_key(raw) == storage.DEFAULT_ADMIN_USERNAME # warms cache + + calls = {"n": 0} + real = storage._pbkdf2_api_key + + def counting(key): + calls["n"] += 1 + return real(key) + + monkeypatch.setattr(storage, "_pbkdf2_api_key", counting) + assert storage.validate_api_key(raw) == storage.DEFAULT_ADMIN_USERNAME + assert storage.validate_api_key(raw) == storage.DEFAULT_ADMIN_USERNAME + assert calls["n"] == 0 # served from cache, KDF not re-run + + +def test_cache_does_not_bypass_revocation(): + seed_user() + raw, row = storage.create_api_key( + username = storage.DEFAULT_ADMIN_USERNAME, + name = "revoke-after-cache", + expires_at = iso_from_now(days = 1), + ) + assert storage.validate_api_key(raw) == storage.DEFAULT_ADMIN_USERNAME # cached + storage.revoke_api_key(storage.DEFAULT_ADMIN_USERNAME, int(row["id"])) + assert storage.validate_api_key(raw) is None # cache hit still re-checks is_active + + +def test_cache_does_not_bypass_expiry(): + seed_user() + # Expires between the two calls: the first warms the cache, the second is still rejected. + near = (datetime.now(timezone.utc) + timedelta(milliseconds = 600)).isoformat() + raw = make_key(near) + assert storage.validate_api_key(raw) == storage.DEFAULT_ADMIN_USERNAME + import time + + time.sleep(0.8) + assert storage.validate_api_key(raw) is None + + +def test_unknown_key_not_cached(): + seed_user() + bogus = storage.API_KEY_PREFIX + secrets.token_hex(16) + assert storage.validate_api_key(bogus) is None + cache_id = storage._api_key_cache_id(bogus) + assert cache_id not in storage._api_key_hash_cache # spam can't grow the cache + + +def test_create_api_key_route_stores_tz_aware_expiry(): + from datetime import datetime as _dt + + seed_user() + raw, row = storage.create_api_key( + username = storage.DEFAULT_ADMIN_USERNAME, + name = "route", + expires_at = iso_from_now(days = 30), + ) + parsed = _dt.fromisoformat(row["expires_at"]) + assert parsed.tzinfo is not None # tz-aware: comparison in validate_api_key won't raise + assert storage.validate_api_key(raw) == storage.DEFAULT_ADMIN_USERNAME diff --git a/studio/backend/tests/test_secure_tunnel_gate.py b/studio/backend/tests/test_secure_tunnel_gate.py new file mode 100644 index 0000000000..6d9b6be892 --- /dev/null +++ b/studio/backend/tests/test_secure_tunnel_gate.py @@ -0,0 +1,95 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 + +"""Cloudflare tunnel start gate, incl. --secure on loopback. Imports run.py +directly, so run under the Studio venv.""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + +_BACKEND = Path(__file__).resolve().parents[1] +if str(_BACKEND) not in sys.path: + sys.path.insert(0, str(_BACKEND)) + +from run import _cloudflare_tunnel_should_start as should_start # noqa: E402 + + +@pytest.mark.parametrize( + "cloudflare,host,secure,api_only,is_colab,expected", + [ + # Non-secure: historical 0.0.0.0-only behaviour preserved. + (True, "0.0.0.0", False, False, False, True), + (True, "127.0.0.1", False, False, False, False), + (True, "localhost", False, False, False, False), + # --secure tunnels a loopback bind too. + (True, "127.0.0.1", True, False, False, True), + (True, "0.0.0.0", True, False, False, True), + # --no-cloudflare always wins. + (False, "0.0.0.0", False, False, False, False), + (False, "127.0.0.1", True, False, False, False), + # api-only and Colab never tunnel. + (True, "0.0.0.0", False, True, False, False), + (True, "127.0.0.1", True, True, False, False), + (True, "0.0.0.0", False, False, True, False), + (True, "127.0.0.1", True, False, True, False), + ], +) +def test_cloudflare_gate(cloudflare, host, secure, api_only, is_colab, expected): + assert ( + should_start( + cloudflare = cloudflare, + host = host, + secure = secure, + api_only = api_only, + is_colab = is_colab, + ) + is expected + ) + + +def test_run_server_accepts_secure_kwarg(): + import inspect + + import run + + assert "secure" in inspect.signature(run.run_server).parameters + assert inspect.signature(run.run_server).parameters["secure"].default is False + + +def test_plain_network_launch_forces_tools_off(): + # Network-reachable launches (0.0.0.0 or --secure) must force server-side tools off. + import run + from state.tool_policy import get_tool_policy, reset_tool_policy + + reset_tool_policy() + run._apply_default_tool_policy("127.0.0.1", False) + assert get_tool_policy() is None # loopback: untouched (per-request honored) + + run._apply_default_tool_policy("0.0.0.0", False) + assert get_tool_policy() is False # network bind: forced off + + reset_tool_policy() + run._apply_default_tool_policy("127.0.0.1", True) # --secure (public tunnel) + assert get_tool_policy() is False + reset_tool_policy() + + +def test_run_server_rejects_secure_without_cloudflare(): + # Direct backend callers (not just the CLI) must reject the contradictory combo. + import run + with pytest.raises(SystemExit) as exc: + run.run_server(secure = True, cloudflare = False) + assert "A secure Cloudflare link is not allowed" in str(exc.value) + + +def test_failclosed_message_present_in_source(): + # The exact, user-facing fail-closed message must not drift. + src = (_BACKEND / "run.py").read_text(encoding = "utf-8") + assert ( + "A secure Cloudflare link is not allowed, use --not-secure which provides a 0.0.0.0 link" + in src + ) diff --git a/studio/frontend/src/config/env.ts b/studio/frontend/src/config/env.ts index 61ab111d45..60f9e876f9 100644 --- a/studio/frontend/src/config/env.ts +++ b/studio/frontend/src/config/env.ts @@ -18,6 +18,11 @@ export type DeviceType = "mac" | "windows" | "linux" | string; interface PlatformState { deviceType: DeviceType; chatOnly: boolean; + // From /api/health (authed): live tunnel URL, direct (non-tunnel) base, and + // whether the server was launched with --secure. + cloudflareUrl: string | null; + serverUrl: string | null; + secure: boolean; fetched: boolean; isChatOnly: () => boolean; } @@ -37,13 +42,19 @@ const localDeviceType = detectLocalPlatform(); export const usePlatformStore = create()((_, get) => ({ deviceType: localDeviceType, chatOnly: localDeviceType === "mac", + cloudflareUrl: null, + serverUrl: null, + secure: false, fetched: false, isChatOnly: () => get().chatOnly, })); -export async function fetchDeviceType(): Promise { +// `force` re-reads /api/health even if cached, to pick up a late-arriving tunnel URL. +export async function fetchDeviceType(options?: { + force?: boolean; +}): Promise { const { fetched } = usePlatformStore.getState(); - if (fetched) return usePlatformStore.getState().deviceType; + if (fetched && !options?.force) return usePlatformStore.getState().deviceType; try { // /api/health only reports the server's device_type to authed callers. @@ -57,7 +68,13 @@ export async function fetchDeviceType(): Promise { headers: token ? { Authorization: `Bearer ${token}` } : undefined, }); if (res.ok) { - const data = (await res.json()) as { device_type?: string; chat_only?: boolean }; + const data = (await res.json()) as { + device_type?: string; + chat_only?: boolean; + cloudflare_url?: string | null; + server_url?: string | null; + secure?: boolean; + }; const deviceType = data.device_type ?? detectLocalPlatform(); const chatOnly = data.chat_only ?? false; // Cache only a server-reported platform. Unauthenticated responses fall @@ -66,6 +83,9 @@ export async function fetchDeviceType(): Promise { usePlatformStore.setState({ deviceType, chatOnly, + cloudflareUrl: data.cloudflare_url ?? null, + serverUrl: data.server_url ?? null, + secure: data.secure ?? false, fetched: data.device_type !== undefined, }); return deviceType; diff --git a/studio/frontend/src/features/settings/components/usage-examples.tsx b/studio/frontend/src/features/settings/components/usage-examples.tsx index 2fd387062f..874c9fa930 100644 --- a/studio/frontend/src/features/settings/components/usage-examples.tsx +++ b/studio/frontend/src/features/settings/components/usage-examples.tsx @@ -1,25 +1,85 @@ // SPDX-License-Identifier: AGPL-3.0-only // Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 +import { createCodePlugin } from "@/components/assistant-ui/code-plugin"; +import { + unslothDarkTheme, + unslothLightTheme, +} from "@/components/assistant-ui/code-themes"; +import { Switch } from "@/components/ui/switch"; +import { + Tooltip, + TooltipContent, + TooltipTrigger, +} from "@/components/ui/tooltip"; +import { fetchDeviceType, usePlatformStore } from "@/config/env"; +import { useChatRuntimeStore } from "@/features/chat"; +import { useT } from "@/i18n"; +import type { TranslationKey } from "@/i18n"; +import { copyToClipboard } from "@/lib/copy-to-clipboard"; import { Tick02Icon } from "@/lib/tick-icon"; import { cn } from "@/lib/utils"; -import { copyToClipboard } from "@/lib/copy-to-clipboard"; -import { useT } from "@/i18n"; import { ArrowUpRight01Icon, Copy01Icon, + InformationCircleIcon, } from "@hugeicons/core-free-icons"; import { HugeiconsIcon } from "@hugeicons/react"; -import { useMemo, useState } from "react"; +import { useEffect, useMemo, useState } from "react"; +import { Streamdown } from "streamdown"; -type Lang = "curl" | "python" | "tools"; +// API call type; OS axis applies to curl only (Python is OS-identical). +type ExampleType = + | "curl" + | "python" + | "curlTools" + | "pythonTools" + | "curlAdvanced" + | "pythonAdvanced"; +type Os = "unix" | "windows"; +// plain = bare call; tools = server-side tools; advanced = sampling + thinking + tools. +type Variant = "plain" | "tools" | "advanced"; -const TABS: { id: Lang; label: string }[] = [ +const TYPE_TABS: { id: ExampleType; label: string }[] = [ { id: "curl", label: "curl" }, { id: "python", label: "Python" }, - { id: "tools", label: "Tools" }, + { id: "curlTools", label: "curl + tools" }, + { id: "pythonTools", label: "Python + tools" }, + { id: "curlAdvanced", label: "curl + advanced" }, + { id: "pythonAdvanced", label: "Python + advanced" }, ]; +const TYPE_LABEL_KEY: Partial> = { + curlTools: "settings.apiKeys.exampleCurlTools", + pythonTools: "settings.apiKeys.examplePythonTools", + curlAdvanced: "settings.apiKeys.exampleCurlAdvanced", + pythonAdvanced: "settings.apiKeys.examplePythonAdvanced", +}; + +const OS_AWARE: Record = { + curl: true, + python: false, + curlTools: true, + pythonTools: false, + curlAdvanced: true, + pythonAdvanced: false, +}; + +const CURL_TYPES = new Set(["curl", "curlTools", "curlAdvanced"]); + +const PROMPT = "Can Unsloth Studio do API calling?"; +// web_search + python + terminal are the reliable built-in tools. +const TOOLS = ["web_search", "python", "terminal"]; +// Sampling/thinking knobs for the "+ advanced" examples. +const ADV = { + temperature: 0.7, + top_p: 0.8, + top_k: 20, + min_p: 0.05, + repetition_penalty: 1.1, + max_tokens: 1024, +} as const; + const DOC_LINKS = [ { label: "Claude Code", @@ -43,52 +103,275 @@ const DOC_LINKS = [ }, ]; -function buildSnippets(base: string) { - return { - curl: `curl ${base}/v1/chat/completions \\ - -H "Authorization: Bearer sk-unsloth-YOUR_KEY" \\ +// JSON-encode; also a valid Python literal, so odd model names never break output. +const j = (s: string): string => JSON.stringify(s); +// Embed in a POSIX single-quoted string: close, escaped quote, reopen. +const shSingle = (s: string): string => s.replace(/'/g, "'\\''"); +// Embed in a PowerShell single-quoted string: '' is a literal quote. +const psSingle = (s: string): string => s.replace(/'/g, "''"); +const toolsJson = TOOLS.map(j).join(", "); + +// Shared body fields (after model/messages, before stream) per variant. +function bodyExtraLines(variant: Variant, indent: string): string[] { + const lines: string[] = []; + if (variant === "advanced") { + lines.push(`${indent}"temperature": ${ADV.temperature},`); + lines.push(`${indent}"top_p": ${ADV.top_p},`); + lines.push(`${indent}"top_k": ${ADV.top_k},`); + lines.push(`${indent}"min_p": ${ADV.min_p},`); + lines.push(`${indent}"repetition_penalty": ${ADV.repetition_penalty},`); + lines.push(`${indent}"max_tokens": ${ADV.max_tokens},`); + lines.push(`${indent}"enable_thinking": true,`); + } + if (variant !== "plain") { + lines.push(`${indent}"enable_tools": true,`); + lines.push(`${indent}"enabled_tools": [${toolsJson}],`); + } + return lines; +} + +function curlBodyPretty(model: string, variant: Variant): string { + const lines = [ + ` "model": ${j(model)},`, + ` "messages": [{"role": "user", "content": ${j(PROMPT)}}],`, + ...bodyExtraLines(variant, " "), + ` "stream": true`, + ]; + return `{\n${lines.join("\n")}\n }`; +} + +// One-line JSON for the Windows body file (PowerShell mangles inline quotes to curl.exe). +function winBody(model: string, variant: Variant): string { + const body: Record = { + model, + messages: [{ role: "user", content: PROMPT }], + }; + if (variant === "advanced") { + body.temperature = ADV.temperature; + body.top_p = ADV.top_p; + body.top_k = ADV.top_k; + body.min_p = ADV.min_p; + body.repetition_penalty = ADV.repetition_penalty; + body.max_tokens = ADV.max_tokens; + body.enable_thinking = true; + } + if (variant !== "plain") { + body.enable_tools = true; + body.enabled_tools = TOOLS; + } + body.stream = true; + return JSON.stringify(body); +} + +function curlUnix( + base: string, + key: string, + model: string, + variant: Variant, +): string { + return `curl ${base}/v1/chat/completions \\ + -H "Authorization: Bearer ${key}" \\ -H "Content-Type: application/json" \\ - -d '{ - "messages": [{"role": "user", "content": "Hello"}], - "stream": true - }'`, - python: `from openai import OpenAI + -d '${shSingle(curlBodyPretty(model, variant))}'`; +} + +// Windows PowerShell: curl aliases to Invoke-WebRequest, so use curl.exe + body file. +function curlWindows( + base: string, + key: string, + model: string, + variant: Variant, +): string { + return `$body = '${psSingle(winBody(model, variant))}' +Set-Content -Path body.json -Value $body -Encoding ascii +curl.exe ${base}/v1/chat/completions \` + -H "Authorization: Bearer ${key}" \` + -H "Content-Type: application/json" \` + -d "@body.json"`; +} + +function pythonSnippet( + base: string, + key: string, + model: string, + variant: Variant, +): string { + // Standard OpenAI args are named; Unsloth extensions go through extra_body. + const named = + variant === "advanced" + ? ` + temperature=${ADV.temperature}, + top_p=${ADV.top_p}, + max_tokens=${ADV.max_tokens},` + : ""; + const extra: string[] = []; + if (variant === "advanced") { + extra.push(` "top_k": ${ADV.top_k},`); + extra.push(` "min_p": ${ADV.min_p},`); + extra.push(` "repetition_penalty": ${ADV.repetition_penalty},`); + extra.push(` "enable_thinking": True,`); + } + if (variant !== "plain") { + extra.push(` "enable_tools": True,`); + extra.push(` "enabled_tools": [${toolsJson}],`); + } + const extraBody = extra.length + ? ` + extra_body={ +${extra.join("\n")} + },` + : ""; + // With tools, some chunks are tool-lifecycle events with no choices; guard it. + const loop = + variant !== "plain" + ? `for chunk in response: + if chunk.choices: + print(chunk.choices[0].delta.content or "", end="")` + : `for chunk in response: + print(chunk.choices[0].delta.content or "", end="")`; + return `from openai import OpenAI client = OpenAI( - base_url="${base}/v1", - api_key="sk-unsloth-YOUR_KEY", + base_url=${j(`${base}/v1`)}, + api_key=${j(key)}, ) response = client.chat.completions.create( - model="current", - messages=[{"role": "user", "content": "Hello"}], + model=${j(model)}, + messages=[{"role": "user", "content": ${j(PROMPT)}}],${named}${extraBody} stream=True, ) -for chunk in response: - print(chunk.choices[0].delta.content or "", end="")`, - tools: `curl ${base}/v1/chat/completions \\ - -H "Authorization: Bearer sk-unsloth-YOUR_KEY" \\ - -H "Content-Type: application/json" \\ - -d '{ - "messages": [{"role": "user", "content": "Search Python 3.13 features"}], - "enable_tools": true, - "enabled_tools": ["web_search", "python"], - "stream": true - }'`, +${loop}`; +} + +function buildSnippets( + base: string, + key: string, + model: string, + os: Os, +): Record { + const curl = os === "windows" ? curlWindows : curlUnix; + return { + curl: curl(base, key, model, "plain"), + python: pythonSnippet(base, key, model, "plain"), + curlTools: curl(base, key, model, "tools"), + pythonTools: pythonSnippet(base, key, model, "tools"), + curlAdvanced: curl(base, key, model, "advanced"), + pythonAdvanced: pythonSnippet(base, key, model, "advanced"), }; } -export function UsageExamples() { - const t = useT(); - const [lang, setLang] = useState("curl"); - const [copied, setCopied] = useState(false); - const snippets = useMemo( - () => - buildSnippets( - typeof window !== "undefined" ? window.location.origin : "", - ), - [], +const KEY_PLACEHOLDER = "sk-unsloth-YOUR_KEY"; +const MODEL_FALLBACK = "unsloth/gemma-4-E4B-it-GGUF:UD-Q5_K_XL"; + +// Default ON: when a tunnel exists, examples should show the public base_url. +const USE_TUNNEL_KEY = "unsloth_api_use_tunnel"; + +function readUseTunnelPref(): boolean { + if (typeof window === "undefined") return true; + try { + return window.localStorage.getItem(USE_TUNNEL_KEY) !== "false"; + } catch { + return true; + } +} + +function writeUseTunnelPref(value: boolean): void { + if (typeof window === "undefined") return; + try { + window.localStorage.setItem(USE_TUNNEL_KEY, value ? "true" : "false"); + } catch { + // Non-fatal: the toggle still applies for this session. + } +} + +// Active local checkpoint as repo[:variant]; external/none falls back to a default. +function useLoadedModelName(): string { + const checkpoint = useChatRuntimeStore((s) => s.params.checkpoint); + const ggufVariant = useChatRuntimeStore((s) => s.activeGgufVariant); + return useMemo(() => { + if (!checkpoint || checkpoint.startsWith("external::")) { + return MODEL_FALLBACK; + } + if (ggufVariant && !checkpoint.includes(":")) { + return `${checkpoint}:${ggufVariant}`; + } + return checkpoint; + }, [checkpoint, ggufVariant]); +} + +// shiki highlighting via the app's shared code plugin + themes (same as chat). +const SHIKI_THEMES = [unslothLightTheme, unslothDarkTheme] as [ + typeof unslothLightTheme, + typeof unslothDarkTheme, +]; +const codePlugin = createCodePlugin({ themes: SHIKI_THEMES }); + +function HighlightedCode({ + code, + language, +}: { + code: string; + language: string; +}) { + // Fence so Streamdown's shiki plugin highlights it (no markdown inside a fence). + const markdown = useMemo( + () => `\`\`\`${language}\n${code}\n\`\`\``, + [code, language], ); + return ( +
+ + {markdown} + +
+ ); +} + +export function UsageExamples({ apiKey }: { apiKey?: string | null }) { + const t = useT(); + const deviceType = usePlatformStore((s) => s.deviceType); + const cloudflareUrl = usePlatformStore((s) => s.cloudflareUrl); + const serverUrl = usePlatformStore((s) => s.serverUrl); + const secure = usePlatformStore((s) => s.secure); + const [lang, setLang] = useState("curl"); + const [os, setOs] = useState( + deviceType === "windows" ? "windows" : "unix", + ); + const [copied, setCopied] = useState(false); + const [copiedUrl, setCopiedUrl] = useState(false); + const [useTunnel, setUseTunnel] = useState(readUseTunnelPref); + + // Tunnel may start after the first /api/health read; refresh so it surfaces here. + useEffect(() => { + void fetchDeviceType({ force: true }); + }, []); + + const model = useLoadedModelName(); + // Real key while revealed (before "Done"); otherwise a placeholder. + const key = apiKey || KEY_PLACEHOLDER; + // Toggle on + tunnel up: public tunnel URL. Off: backend direct host:port + // (origin is only a last-resort fallback). + const origin = typeof window !== "undefined" ? window.location.origin : ""; + const base = + useTunnel && cloudflareUrl ? cloudflareUrl : (serverUrl ?? origin); + + const snippets = useMemo( + () => buildSnippets(base, key, model, os), + [base, key, model, os], + ); + + const osAware = OS_AWARE[lang]; + const shikiLang = CURL_TYPES.has(lang) + ? os === "windows" + ? "powershell" + : "bash" + : "python"; const handleCopy = async () => { if (await copyToClipboard(snippets[lang])) { @@ -97,16 +380,86 @@ export function UsageExamples() { } }; + const handleToggleTunnel = (next: boolean) => { + setUseTunnel(next); + writeUseTunnelPref(next); + }; + + const handleCopyUrl = async () => { + if (cloudflareUrl && (await copyToClipboard(cloudflareUrl))) { + setCopiedUrl(true); + setTimeout(() => setCopiedUrl(false), 1800); + } + }; + return (

{t("settings.apiKeys.usageExamples")}

+ {cloudflareUrl ? ( +
+
+ + + {t("settings.apiKeys.secureHttps")} + + {/* Only when not launched with --secure: the raw 0.0.0.0 port is + still globally reachable, so point the user at --secure. */} + {!secure ? ( + + + + + + {t("settings.apiKeys.secureHttpsHint")} + + + ) : null} +
+ {/* Always rendered (dimmed when off) so toggling never changes the + row height and shifts the code block below. */} + +
+ ) : null}
-
- {TABS.map((tab) => { +
+ {TYPE_TABS.map((tab) => { const active = lang === tab.id; + const labelKey = TYPE_LABEL_KEY[tab.id]; return ( ); })}
+
+ {osAware ? ( +
+ + +
+ ) : null} +
+ {/* key on the snippet so Streamdown remounts and re-highlights when + only a substring (e.g. the base URL) changes; its block memo + otherwise keeps the stale render. */} +
-
-          {snippets[lang]}
-        
{t("settings.apiKeys.setupDocs")} {DOC_LINKS.map((link) => ( diff --git a/studio/frontend/src/features/settings/tabs/api-keys-tab.tsx b/studio/frontend/src/features/settings/tabs/api-keys-tab.tsx index d490d6b23a..64f3ef6621 100644 --- a/studio/frontend/src/features/settings/tabs/api-keys-tab.tsx +++ b/studio/frontend/src/features/settings/tabs/api-keys-tab.tsx @@ -166,7 +166,7 @@ export function ApiKeysTab() { )}
- + !o && setRevokeTarget(null)}> diff --git a/studio/frontend/src/i18n/locales/en.ts b/studio/frontend/src/i18n/locales/en.ts index 41964d9475..8998322ced 100644 --- a/studio/frontend/src/i18n/locales/en.ts +++ b/studio/frontend/src/i18n/locales/en.ts @@ -288,6 +288,16 @@ export const en = { copyNow: "Copy now - this won't be shown again.", usageExamples: "Usage examples", usageTools: "Tools", + exampleCurlTools: "curl + tools", + examplePythonTools: "Python + tools", + exampleCurlAdvanced: "curl + advanced", + examplePythonAdvanced: "Python + advanced", + osUnix: "Linux / macOS / WSL", + osWindows: "Windows", + secureHttps: "Secure HTTPS", + secureHttpsHint: + "The 0.0.0.0 port is still reachable globally. For full security, launch Unsloth Studio with --secure to expose only this HTTPS link.", + copyTunnelUrl: "Copy tunnel URL", copySnippet: "Copy snippet", copy: "Copy", copied: "Copied", diff --git a/studio/frontend/src/i18n/locales/zh-CN.ts b/studio/frontend/src/i18n/locales/zh-CN.ts index e7e377564e..9141a4c104 100644 --- a/studio/frontend/src/i18n/locales/zh-CN.ts +++ b/studio/frontend/src/i18n/locales/zh-CN.ts @@ -261,6 +261,16 @@ export const zhCN = { copyNow: "现在复制 - 之后不会再次显示。", usageExamples: "使用示例", usageTools: "工具", + exampleCurlTools: "curl + 工具", + examplePythonTools: "Python + 工具", + exampleCurlAdvanced: "curl + 高级", + examplePythonAdvanced: "Python + 高级", + osUnix: "Linux / macOS / WSL", + osWindows: "Windows", + secureHttps: "安全 HTTPS", + secureHttpsHint: + "0.0.0.0 端口仍可被全网访问。如需完全安全,请使用 --secure 启动 Unsloth Studio,仅暴露此 HTTPS 链接。", + copyTunnelUrl: "复制隧道链接", copySnippet: "复制代码片段", copy: "复制", copied: "已复制", diff --git a/unsloth_cli/commands/studio.py b/unsloth_cli/commands/studio.py index 400625363c..69df81acad 100644 --- a/unsloth_cli/commands/studio.py +++ b/unsloth_cli/commands/studio.py @@ -661,6 +661,13 @@ def studio_default( "--cloudflare/--no-cloudflare", help = "Auto-create a free Cloudflare HTTPS tunnel when bound to 0.0.0.0 (default on).", ), + secure: bool = typer.Option( + False, + "--secure/--not-secure", + help = "Expose ONLY a Cloudflare HTTPS link: bind localhost and fail closed " + "if the tunnel can't start. Without it, --not-secure also serves the raw " + "0.0.0.0 port, which is reachable from anywhere on the network.", + ), ): """Launch the Unsloth Studio server.""" # Runs before every subcommand (run/setup/update/...). @@ -688,8 +695,29 @@ def studio_default( err = True, ) raise typer.Exit(2) + # Same for --secure: it would not reach the subcommand. + if secure: + typer.echo( + f"Error: --secure on `unsloth studio` applies to the " + f"plain-server path only. For `unsloth studio " + f"{ctx.invoked_subcommand}`, put it after the subcommand: " + f"`unsloth studio {ctx.invoked_subcommand} --secure ...`", + err = True, + ) + raise typer.Exit(2) return + # --secure requires the tunnel; force a loopback bind. + if secure: + if not cloudflare: + typer.echo( + "Error: --secure requires the Cloudflare tunnel; do not combine it " + "with --no-cloudflare.", + err = True, + ) + raise typer.Exit(2) + host = "127.0.0.1" + # Use the studio venv if it exists and we aren't already in it. studio_venv_dir = STUDIO_HOME / "unsloth_studio" in_studio_venv = sys.prefix.startswith(str(studio_venv_dir)) @@ -724,6 +752,7 @@ def studio_default( args.append("--api-only") # Forward the explicit polarity (matches run.py's BooleanOptionalAction). args.append("--cloudflare" if cloudflare else "--no-cloudflare") + args.append("--secure" if secure else "--not-secure") # On Windows os.execvp keeps the parent alive, so Ctrl+C # would orphan the child; use Popen+wait instead. if sys.platform == "win32": @@ -766,6 +795,7 @@ def studio_default( api_only = api_only, llama_parallel_slots = parallel, cloudflare = cloudflare, + secure = secure, ) if frontend is not None: run_kwargs["frontend_path"] = frontend @@ -943,6 +973,13 @@ def run( "--cloudflare/--no-cloudflare", help = "Auto-create a free Cloudflare HTTPS tunnel when bound to 0.0.0.0 (default on).", ), + secure: bool = typer.Option( + False, + "--secure/--not-secure", + help = "Expose ONLY a Cloudflare HTTPS link: bind localhost and fail closed " + "if the tunnel can't start. Without it, --not-secure also serves the raw " + "0.0.0.0 port, which is reachable from anywhere on the network.", + ), tensor_parallel: bool = typer.Option( False, "--tensor-parallel/--no-tensor-parallel", @@ -1011,12 +1048,27 @@ def run( model = parsed_repo gguf_variant = gguf_variant or embedded_variant + # --secure requires the tunnel; force a loopback bind so the raw port is never public. + if secure: + if not cloudflare: + typer.echo( + "Error: --secure requires the Cloudflare tunnel; do not combine it " + "with --no-cloudflare.", + err = True, + ) + raise typer.Exit(2) + host = "127.0.0.1" + + # Gate tools on the *public* exposure: --secure is public via the tunnel, so + # tools default off even though the bind is loopback. + tool_policy_host = "0.0.0.0" if secure else host + # Resolve tool policy here so the re-exec'd child inherits a # concrete decision and never re-prompts. from unsloth_cli._tool_policy import is_external_host, resolve_tool_policy enable_tools = resolve_tool_policy( - host = host, + host = tool_policy_host, flag = enable_tools, yes = yes, silent = silent, @@ -1065,15 +1117,15 @@ def run( args.append("--enable-tools") else: args.append("--disable-tools") - # Forward --yes if the parent already cleared the network-bind - # prompt, else the child re-prompts. - if yes or (enable_tools and is_external_host(host)): + # Forward --yes if the parent already cleared the network-bind prompt. + if yes or (enable_tools and is_external_host(tool_policy_host)): args.append("--yes") # Typer claims --parallel outside ctx.args; without this the # child reverts to its default and silently drops the value. args.extend(["--parallel", str(parallel)]) # Forward the explicit polarity (same rationale as --load-in-4bit above). args.append("--cloudflare" if cloudflare else "--no-cloudflare") + args.append("--secure" if secure else "--not-secure") args.append("--tensor-parallel" if tensor_parallel else "--no-tensor-parallel") # llama-server pass-through extras → child ctx.args → load payload. if extra_llama_args: @@ -1106,6 +1158,7 @@ def run( silent = True, llama_parallel_slots = parallel, cloudflare = cloudflare, + secure = secure, ) if frontend is not None: run_kwargs["frontend_path"] = frontend @@ -1157,22 +1210,26 @@ def run( display_host = run_mod._resolve_external_ip() if host == "0.0.0.0" else host base_url = f"http://{display_host}:{actual_port}" sdk_base_url = f"{base_url}/v1" - # run_server started the tunnel during the silent run above (0.0.0.0 only). + # run_server started the tunnel during the silent run above (0.0.0.0 or --secure). _cf_url = getattr(app.state, "cloudflare_url", None) + # --secure: examples must use the public tunnel URL, not the loopback address. + if secure and _cf_url: + sdk_base_url = f"{_cf_url}/v1" # Orange so the tool-policy notice stands out; printed under # --silent / --yes too so the policy is never invisible. _tool_notice_fg = (217, 119, 87) - _is_external = is_external_host(host) + _is_external = is_external_host(tool_policy_host) + _exposure = "the public Cloudflare tunnel" if secure else host if _is_external and enable_tools: _tool_notice = ( - f"Server-side tools are ENABLED on {host} (network-reachable). " + f"Server-side tools are ENABLED on {_exposure} (network-reachable). " f"Anyone with the API key can run code on this machine. " f"Do not share the API key." ) elif _is_external: _tool_notice = ( - f"Server-side tools are disabled by default on {host} " + f"Server-side tools are disabled by default on {_exposure} " f"(network-reachable). Pass --enable-tools to turn on " f"(you will be warned about API-key risk)." ) @@ -1187,9 +1244,13 @@ def run( if not silent: typer.echo("") typer.echo("=" * 56) - typer.echo(f" Unsloth Studio running at {base_url}") - if _cf_url: - typer.echo(f" Secure link access via Cloudflare: {_cf_url}") + if secure and _cf_url: + typer.echo(f" Unsloth Studio running (secure) at {_cf_url}") + typer.echo(f" On this machine only: {base_url}") + else: + typer.echo(f" Unsloth Studio running at {base_url}") + if _cf_url: + typer.echo(f" Secure link access via Cloudflare: {_cf_url}") typer.echo(f" Model loaded: {loaded_model}{display_variant}") if context_length_line: typer.echo(context_length_line) @@ -1224,9 +1285,13 @@ def run( typer.echo("") else: # Silent still prints URL + API key + tool-status policy. - typer.echo(f"URL: {base_url}") - if _cf_url: - typer.echo(f"Secure link access via Cloudflare: {_cf_url}") + if secure and _cf_url: + typer.echo(f"URL: {_cf_url}") + typer.echo(f"Local: {base_url}") + else: + typer.echo(f"URL: {base_url}") + if _cf_url: + typer.echo(f"Secure link access via Cloudflare: {_cf_url}") if context_length_line: typer.echo(context_length_line.strip()) typer.echo(f"API Key: {api_key}") diff --git a/unsloth_cli/tests/test_studio_secure_flag.py b/unsloth_cli/tests/test_studio_secure_flag.py new file mode 100644 index 0000000000..2c6f1ebf9e --- /dev/null +++ b/unsloth_cli/tests/test_studio_secure_flag.py @@ -0,0 +1,294 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 + +"""Tests for the `--secure/--not-secure` Studio flag: option registration, +re-exec/run_server forwarding, the forced 127.0.0.1 bind, and rejection +alongside --no-cloudflare or before a subcommand. Modeled on +test_studio_cloudflare_flag.py.""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +_REPO_ROOT = Path(__file__).resolve().parents[2] +if str(_REPO_ROOT) not in sys.path: + sys.path.insert(0, str(_REPO_ROOT)) + + +def _studio(): + from unsloth_cli.commands import studio as _studio_mod + return _studio_mod + + +_BASE = ["--model", "unsloth/Qwen3-1.7B-GGUF"] + + +# ── option registration ────────────────────────────────────────────── + + +def test_run_exposes_secure_option_default_off(): + import inspect + + opt = inspect.signature(_studio().run).parameters["secure"].default + decls = set(getattr(opt, "param_decls", []) or []) + assert "--secure/--not-secure" in decls + assert getattr(opt, "default", None) is False + + +def test_studio_default_exposes_secure_option_default_off(): + import inspect + + opt = inspect.signature(_studio().studio_default).parameters["secure"].default + decls = set(getattr(opt, "param_decls", []) or []) + assert "--secure/--not-secure" in decls + assert getattr(opt, "default", None) is False + + +# ── re-exec capture plumbing (mirrors test_studio_cloudflare_flag.py) ─ + + +class _ExecCaptured(SystemExit): + def __init__(self, argv): + super().__init__(0) + self.argv = list(argv) + + +def _install_run_reexec_capture(monkeypatch): + studio_mod = _studio() + captured = [] + monkeypatch.setattr(sys, "prefix", "/nonexistent/outer/venv") + fake_venv = Path("/fake/studio/venv/unsloth_studio") + monkeypatch.setattr(studio_mod, "_studio_venv_python", lambda: fake_venv / "bin" / "python") + fake_bin = fake_venv / "bin" / "unsloth" + real_is_file = Path.is_file + monkeypatch.setattr( + Path, + "is_file", + lambda self: True if str(self) == str(fake_bin) else real_is_file(self), + ) + from unsloth_cli import _tool_policy as _tp_mod + + monkeypatch.setattr( + _tp_mod, + "resolve_tool_policy", + lambda host, flag, yes, silent: False if flag is None else bool(flag), + ) + monkeypatch.setattr(sys, "platform", "linux") + + def fake_execvp(file, argv): + captured.append(list(argv)) + raise _ExecCaptured(argv) + + monkeypatch.setattr(studio_mod.os, "execvp", fake_execvp) + return captured + + +def _invoke_run(monkeypatch, args): + import typer as _typer + + captured = _install_run_reexec_capture(monkeypatch) + app = _typer.Typer() + app.command( + context_settings = {"allow_extra_args": True, "ignore_unknown_options": True}, + )(_studio().run) + CliRunner().invoke(app, args, catch_exceptions = True) + return captured + + +def _invoke_studio_default(monkeypatch, args): + import typer as _typer + + studio_mod = _studio() + captured = [] + monkeypatch.setattr(sys, "prefix", "/nonexistent/outer/venv") + monkeypatch.setattr(studio_mod, "_ensure_studio_env_exported", lambda: None) + fake_venv = Path("/fake/studio/venv/unsloth_studio") + monkeypatch.setattr(studio_mod, "_studio_venv_python", lambda: fake_venv / "bin" / "python") + monkeypatch.setattr(studio_mod, "_find_run_py", lambda: Path("/fake/studio/run.py")) + monkeypatch.setattr(studio_mod, "_find_frontend_dist", lambda: None) + monkeypatch.setattr(sys, "platform", "linux") + + def fake_execvp(file, argv): + captured.append(list(argv)) + raise _ExecCaptured(argv) + + monkeypatch.setattr(studio_mod.os, "execvp", fake_execvp) + app = _typer.Typer() + app.command()(studio_mod.studio_default) + CliRunner().invoke(app, args, catch_exceptions = True) + return captured + + +# ── re-exec forwarding ──────────────────────────────────────────────── + + +@pytest.mark.parametrize( + "user_flag,expected,unexpected", + [ + (None, "--not-secure", "--secure"), # default off + ("--secure", "--secure", "--not-secure"), + ("--not-secure", "--not-secure", "--secure"), + ], +) +def test_run_reexec_forwards_secure_polarity(monkeypatch, user_flag, expected, unexpected): + extras = [user_flag] if user_flag else [] + captured = _invoke_run(monkeypatch, _BASE + extras) + assert len(captured) == 1, captured + argv = captured[0] + assert expected in argv and unexpected not in argv, argv + + +def test_run_secure_forces_localhost_in_reexec(monkeypatch): + # `unsloth studio run -H 0.0.0.0 --secure` must re-exec with --host 127.0.0.1. + captured = _invoke_run(monkeypatch, _BASE + ["-H", "0.0.0.0", "--secure"]) + assert len(captured) == 1, captured + argv = captured[0] + assert "--secure" in argv + assert argv[argv.index("--host") + 1] == "127.0.0.1", argv + + +def test_studio_default_reexec_forwards_secure(monkeypatch): + captured = _invoke_studio_default(monkeypatch, ["-H", "0.0.0.0", "--secure"]) + assert len(captured) == 1, captured + argv = captured[0] + assert "--secure" in argv + # studio_default also forces the loopback bind under --secure. + assert argv[argv.index("--host") + 1] == "127.0.0.1", argv + + +# ── in-venv path forwards secure + forced host into run_server ──────── + + +class _RunServerCaptured(SystemExit): + def __init__(self, kwargs): + super().__init__(0) + self.kwargs = dict(kwargs) + + +def test_run_in_venv_passes_secure_and_forces_host(monkeypatch): + import types + + studio_mod = _studio() + fake_venv = Path("/fake/studio/venv/unsloth_studio") + monkeypatch.setattr(sys, "prefix", str(fake_venv)) + monkeypatch.setattr(studio_mod, "STUDIO_HOME", fake_venv.parent) + + from unsloth_cli import _tool_policy as _tp_mod + + monkeypatch.setattr( + _tp_mod, + "resolve_tool_policy", + lambda host, flag, yes, silent: False if flag is None else bool(flag), + ) + + captured: dict = {} + + def fake_run_server(**kwargs): + captured.update(kwargs) + raise _RunServerCaptured(kwargs) + + fake_backend_run = sys.modules.setdefault( + "studio.backend.run", types.ModuleType("studio.backend.run") + ) + fake_backend_run.run_server = fake_run_server + fake_backend_run._resolve_external_ip = lambda: "127.0.0.1" + monkeypatch.setattr(studio_mod, "_RUN_MODULE", fake_backend_run) + + import typer as _typer + + app = _typer.Typer() + app.command( + context_settings = {"allow_extra_args": True, "ignore_unknown_options": True}, + )(studio_mod.run) + CliRunner().invoke(app, _BASE + ["-H", "0.0.0.0", "--secure"], catch_exceptions = True) + + assert captured.get("secure") is True, captured + assert captured.get("host") == "127.0.0.1", captured + + +# ── --secure + --no-cloudflare is rejected ─────────────────────────── + + +def test_run_secure_rejects_no_cloudflare(monkeypatch): + studio_mod = _studio() + import typer as _typer + + app = _typer.Typer() + app.command( + context_settings = {"allow_extra_args": True, "ignore_unknown_options": True}, + )(studio_mod.run) + result = CliRunner().invoke(app, _BASE + ["--secure", "--no-cloudflare"]) + assert result.exit_code == 2, result.output + + +def test_studio_default_rejects_secure_with_subcommand(): + import typer as _typer + + studio_mod = _studio() + app = _typer.Typer() + app.add_typer(studio_mod.studio_app, name = "studio") + result = CliRunner().invoke(app, ["studio", "--secure", "run", "--model", "X"]) + assert result.exit_code == 2, result.output + combined = (result.output or "") + (getattr(result, "stderr", "") or "") + assert "--secure" in combined, combined + + +# ── secure resolves tools against the PUBLIC exposure, not the loopback bind ── + + +def test_run_secure_resolves_tools_against_public_host(monkeypatch): + # --secure is public via the tunnel, so tools resolve against 0.0.0.0 (OFF), not loopback (ON). + studio_mod = _studio() + monkeypatch.setattr(sys, "prefix", "/nonexistent/outer/venv") + fake_venv = Path("/fake/studio/venv/unsloth_studio") + monkeypatch.setattr(studio_mod, "_studio_venv_python", lambda: fake_venv / "bin" / "python") + fake_bin = fake_venv / "bin" / "unsloth" + real_is_file = Path.is_file + monkeypatch.setattr( + Path, + "is_file", + lambda self: True if str(self) == str(fake_bin) else real_is_file(self), + ) + monkeypatch.setattr(sys, "platform", "linux") + + from unsloth_cli import _tool_policy as _tp_mod + + calls = [] + + def rec(host, flag, yes, silent): + calls.append(host) + return (not _tp_mod.is_external_host(host)) if flag is None else bool(flag) + + monkeypatch.setattr(_tp_mod, "resolve_tool_policy", rec) + + captured = [] + + def fake_execvp(file, argv): + captured.append(list(argv)) + raise _ExecCaptured(argv) + + monkeypatch.setattr(studio_mod.os, "execvp", fake_execvp) + + import typer as _typer + + app = _typer.Typer() + app.command( + context_settings = {"allow_extra_args": True, "ignore_unknown_options": True}, + )(studio_mod.run) + CliRunner().invoke(app, _BASE + ["-H", "0.0.0.0", "--secure"], catch_exceptions = True) + + assert calls and calls[0] == "0.0.0.0", calls + assert len(captured) == 1, captured + assert "--disable-tools" in captured[0] and "--enable-tools" not in captured[0], captured[0] + + +def test_run_secure_enable_tools_forwards_yes(monkeypatch): + # Enabling tools on a secure endpoint forwards --yes so the child doesn't re-prompt. + captured = _invoke_run(monkeypatch, _BASE + ["-H", "0.0.0.0", "--secure", "--enable-tools"]) + assert len(captured) == 1, captured + argv = captured[0] + assert "--enable-tools" in argv and "--yes" in argv, argv From 6c919bba8256f4b7ac7e60b883c15d9073f6f52d Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Mon, 15 Jun 2026 04:22:12 -0700 Subject: [PATCH 05/91] Studio: arm the VRAM-settle wait after the startup orphan reaper (#6315) On a restart the constructor reaps the previous run's orphaned llama-server, but the driver does not reclaim that VRAM synchronously. _kill_orphaned_servers now returns the number of processes it killed, and __init__ arms _last_kill_monotonic when that count is positive, so the first load_model waits for VRAM to settle before ranking GPUs by free memory instead of pinning the model onto the smaller card. The compute-graph / auto-fit reserve is handled separately in #6312. --- studio/backend/core/inference/llama_cpp.py | 19 +++-- .../test_llama_cpp_wait_for_vram_settle.py | 70 +++++++++++++++++++ 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index a4fd07c3b6..00dfe14e63 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -816,7 +816,11 @@ class LlamaCppBackend: # to decide whether to wait for the VRAM reclaim to finish. self._last_kill_monotonic: float = 0.0 - self._kill_orphaned_servers() + _reaped = self._kill_orphaned_servers() + if _reaped: + # Reaped VRAM frees lazily; arm the settle wait so the first load + # waits before ranking GPUs by free memory. + self._last_kill_monotonic = time.monotonic() atexit.register(self._cleanup) # ── Properties ──────────────────────────────────────────────── @@ -5081,7 +5085,7 @@ class LlamaCppBackend: self._llama_log_fh = None @staticmethod - def _kill_orphaned_servers(): + def _kill_orphaned_servers() -> int: """Kill orphaned llama-server processes started by studio. Only kills processes whose resolved binary lives under a known @@ -5093,7 +5097,11 @@ class LlamaCppBackend: Uses psutil for cross-platform support (Linux, macOS, Windows); falls back to pgrep + /proc//exe on Linux when psutil is absent. + + Returns the count of processes killed; callers arm the VRAM-settle + wait on a positive count. """ + killed = 0 try: # -- Build the ownership allowlist -------------------------------- # exact_binaries -- env var overrides (exact path match). @@ -5185,6 +5193,7 @@ class LlamaCppBackend: continue proc.kill() + killed += 1 logger.info( f"Killed orphaned llama-server process (pid={proc.info['pid']})" ) @@ -5197,7 +5206,7 @@ class LlamaCppBackend: else: # -- Fallback: pgrep + /proc//exe (Linux only) ----------- if sys.platform != "linux": - return + return killed result = subprocess.run( ["pgrep", "-a", "-f", "llama-server"], capture_output = True, @@ -5206,7 +5215,7 @@ class LlamaCppBackend: env = child_env_without_native_path_secret(), ) if result.returncode != 0: - return + return killed for line in result.stdout.strip().splitlines(): parts = line.strip().split(None, 1) @@ -5237,6 +5246,7 @@ class LlamaCppBackend: try: os.kill(pid, signal.SIGKILL) + killed += 1 logger.info(f"Killed orphaned llama-server process (pid={pid})") except ProcessLookupError: pass @@ -5244,6 +5254,7 @@ class LlamaCppBackend: pass except Exception: logger.warning("Error during orphan server cleanup", exc_info = True) + return killed def _cleanup(self): """atexit handler to ensure llama-server is terminated.""" diff --git a/studio/backend/tests/test_llama_cpp_wait_for_vram_settle.py b/studio/backend/tests/test_llama_cpp_wait_for_vram_settle.py index 493bb93e8c..7f1d0c8a42 100644 --- a/studio/backend/tests/test_llama_cpp_wait_for_vram_settle.py +++ b/studio/backend/tests/test_llama_cpp_wait_for_vram_settle.py @@ -315,3 +315,73 @@ def test_helper_is_static_method_callable_off_class(): LlamaCppBackend._wait_for_vram_settle( **_kw(max_wait = 0.1, interval = 0.05), ) + + +# --------------------------------------------------------------------------- +# Startup orphan-reaper arms the settle clock (the "wrong card after restart" +# root cause: reaped VRAM frees lazily, so the first load must wait). +# --------------------------------------------------------------------------- + + +def test_kill_orphaned_servers_returns_count(): + """The reaper reports how many owned orphans it killed, so __init__ can + arm the settle wait. Only Studio-owned llama-server procs count.""" + import os + + mypid = os.getpid() + fake_path = "/tmp/unsloth-test-llama/llama-server" + killed: list[int] = [] + + class _FakeProc: + def __init__(self, pid, name, exe): + self.info = {"pid": pid, "name": name, "exe": exe} + + def kill(self): + killed.append(self.info["pid"]) + + owned = _FakeProc(mypid + 1, "llama-server", fake_path) # exact-path match + foreign = _FakeProc(mypid + 2, "llama-server", "/usr/bin/llama-server") + unrelated = _FakeProc(mypid + 3, "python3", "/usr/bin/python3") + + fake_psutil = _types.ModuleType("psutil") + fake_psutil.NoSuchProcess = type("NoSuchProcess", (Exception,), {}) + fake_psutil.AccessDenied = type("AccessDenied", (Exception,), {}) + fake_psutil.ZombieProcess = type("ZombieProcess", (Exception,), {}) + fake_psutil.process_iter = lambda attrs = None: [owned, foreign, unrelated] + + with ( + patch.dict(sys.modules, {"psutil": fake_psutil}), + patch.dict(os.environ, {"LLAMA_SERVER_PATH": fake_path}), + ): + n = LlamaCppBackend._kill_orphaned_servers() + assert n == 1, "only the Studio-owned orphan should be counted" + assert killed == [mypid + 1] + + # No owned orphans -> zero, so __init__ leaves the cold-start sentinel. + fake_psutil.process_iter = lambda attrs = None: [foreign, unrelated] + killed.clear() + with ( + patch.dict(sys.modules, {"psutil": fake_psutil}), + patch.dict(os.environ, {"LLAMA_SERVER_PATH": fake_path}), + ): + assert LlamaCppBackend._kill_orphaned_servers() == 0 + assert killed == [] + + +def test_startup_reaper_arms_settle_timestamp(): + """__init__ arms ``_last_kill_monotonic`` when the startup reaper kills an + orphan (so the first load_model waits for VRAM to settle), and leaves the + 0.0 cold-start sentinel when nothing was reaped.""" + with patch.object(LlamaCppBackend, "_kill_orphaned_servers", staticmethod(lambda: 1)): + before = time.monotonic() + backend = LlamaCppBackend() + after = time.monotonic() + assert ( + before <= backend._last_kill_monotonic <= after + ), "a positive reap count must arm the settle clock" + + with patch.object(LlamaCppBackend, "_kill_orphaned_servers", staticmethod(lambda: 0)): + backend_cold = LlamaCppBackend() + assert ( + backend_cold._last_kill_monotonic == 0.0 + ), "no reap must leave the cold-start sentinel so the wait is skipped" From a8c20124015838cd496ce0abf0af25de10b521c5 Mon Sep 17 00:00:00 2001 From: narakai Date: Mon, 15 Jun 2026 19:24:52 +0800 Subject: [PATCH 06/91] Studio: fix Mac IME input-method switch leaving composer Send disabled (#5762) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Studio: fix Mac IME input-method switch leaving composer Send disabled On macOS, switching input method (Ctrl+Space / menu-bar language icon) fires compositionstart but never compositionend — leaving composingRef pinned at true and the Send button permanently disabled even after switching back to English. Two immediate recovery paths added to useImeComposerInputHandlers (thread.tsx) and SharedComposer (shared-composer.tsx): * onKeyDown else-if: clears composingRef on the first non-IME keystroke after a stuck composition, unblocking Send on that very keydown rather than waiting for the 2500ms watchdog. * onBlur handler: clears composingRef unconditionally on textarea focus loss — safe because the OS always commits or cancels any active composition before surrendering focus to another element. Two new Playwright regression steps (6e, 6f) added to playwright_chat_ime_i18n.py assert recovery within 1500ms (well below the 2500ms watchdog), covering both recovery paths. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix IME regression test idle handoff for PR #5762 * Fix IME cleanup console guard for PR #5762 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix IME Enter guard for PR #5762 * [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> Co-authored-by: wasimysaid --- .../src/components/assistant-ui/thread.tsx | 27 ++- .../src/features/chat/shared-composer.tsx | 22 +- tests/studio/playwright_chat_ime_i18n.py | 225 +++++++++++++++++- .../test_composer_rtl_bidi_attribute.py | 39 ++- 4 files changed, 301 insertions(+), 12 deletions(-) diff --git a/studio/frontend/src/components/assistant-ui/thread.tsx b/studio/frontend/src/components/assistant-ui/thread.tsx index ec252b74a8..838fce4fa7 100644 --- a/studio/frontend/src/components/assistant-ui/thread.tsx +++ b/studio/frontend/src/components/assistant-ui/thread.tsx @@ -1416,11 +1416,35 @@ function useImeComposerInputHandlers() { if (e.nativeEvent.isComposing || e.keyCode === 229) { composingRef.current = true; refreshStuckTimer(); + } else if (composingRef.current) { + // Candidate-confirming Enter can arrive as non-composing; keep it gated. + if (e.key === "Enter") { + if (!e.shiftKey) { + e.preventDefault(); + } + refreshStuckTimer(); + return; + } + // Non-IME key while composingRef is stuck; the input method was likely + // switched away on macOS without firing compositionend (issue #5546 + // pattern, but triggered by input-method switch rather than WSL). + // Clear immediately so Send is unblocked on the first non-IME keystroke + // rather than waiting for the 2500ms watchdog. + setCompositionState(false); } }, - [refreshStuckTimer], + [refreshStuckTimer, setCompositionState], ); + // On macOS, switching input methods (e.g. ABC → Pinyin) while the textarea + // is focused can fire compositionstart without a matching compositionend, + // leaving composingRef pinned and Send permanently blocked. The OS always + // commits or cancels any in-progress composition before surrendering focus, + // so blur is a safe unconditional reset point. + const onBlur = useCallback(() => { + setCompositionState(false); + }, [setCompositionState]); + return { inputProps: { onCompositionStart, @@ -1428,6 +1452,7 @@ function useImeComposerInputHandlers() { onCompositionEnd, onChange, onKeyDown, + onBlur, }, isComposing, isComposingRef: composingRef, diff --git a/studio/frontend/src/features/chat/shared-composer.tsx b/studio/frontend/src/features/chat/shared-composer.tsx index 40063705ab..cbfc42c6ae 100644 --- a/studio/frontend/src/features/chat/shared-composer.tsx +++ b/studio/frontend/src/features/chat/shared-composer.tsx @@ -746,7 +746,7 @@ export function SharedComposer({ queueIndexRef.current = 0; setQueueProgress({ current: 0, total: 0 }); toast.error("Prompt queue stopped", { - description: "A compare step failed — remaining prompts were not sent.", + description: "A compare step failed; remaining prompts were not sent.", }); return; } @@ -1113,6 +1113,20 @@ export function SharedComposer({ refreshStuckImeTimer(); return; } + // Non-IME key while composingRef is stuck; mirrors the fix in thread.tsx. + // On macOS, switching input methods without composing can leave composingRef + // pinned; clear it immediately on the first non-IME keystroke. + if (composingRef.current) { + // Candidate-confirming Enter can arrive as non-composing; keep it gated. + if (e.key === "Enter") { + if (!e.shiftKey) { + e.preventDefault(); + } + refreshStuckImeTimer(); + return; + } + setCompositionState(false); + } if (e.key === "Enter" && !e.shiftKey) { e.preventDefault(); if (!busy) { @@ -1396,6 +1410,12 @@ export function SharedComposer({ setText(e.currentTarget.value); }} onKeyDown={onKeyDown} + onBlur={() => { + // Mac: switching input methods can fire compositionstart without a + // matching compositionend, leaving composingRef pinned. The OS always + // commits or cancels composition before the element loses focus. + setCompositionState(false); + }} placeholder="Send to both models..." className="composer-input" rows={1} diff --git a/tests/studio/playwright_chat_ime_i18n.py b/tests/studio/playwright_chat_ime_i18n.py index f27266193d..7686013fcf 100644 --- a/tests/studio/playwright_chat_ime_i18n.py +++ b/tests/studio/playwright_chat_ime_i18n.py @@ -3,12 +3,14 @@ """Studio chat composer IME + multilingual regression smoke. -Covers three surfaces: +Covers four surfaces: A. Stuck IME composition (#5318 / PR #5327): duplicate compositionstart with no compositionend left isComposing=true, dropping keystrokes. B. Multilingual paste round-trip across 31 scripts (Unicode plumbing). C. Stuck compositionend (#5546): WSL Chrome never fires compositionend, wedging Send disabled; the useImeComposerInputHandlers watchdog releases it. + D. Mac input-method switch: compositionstart without compositionend leaves + composingRef stuck; keydown and blur recover immediately. Model-free; the bug surface is the composer, not inference. Env contract matches playwright_chat_ui.py: BASE_URL, STUDIO_NEW_PW, PW_ART_DIR, @@ -123,6 +125,7 @@ with sync_playwright() as p: page_errors: list[str] = [] console_errors: list[str] = [] + expected_probe_cancel_500s = [0] def _on_console(m): if m.type != "error": @@ -294,6 +297,29 @@ with sync_playwright() as p: def clear() -> None: set_value_via_setter("") + def restore_idle_composer_after_probe(label: str) -> None: + """Cancel a real run started by a submit probe before the next case.""" + stop_btn = page.locator('button[aria-label="Stop generating"]') + send_btn = page.locator('button[aria-label="Send message"]') + allowed_cancel_500 = False + try: + stop_btn.wait_for(state = "visible", timeout = 5_000) + expected_probe_cancel_500s[0] += 1 + allowed_cancel_500 = True + stop_btn.click(timeout = 5_000) + info(f"{label}: stopped generation started by submit probe") + except Exception: + if allowed_cancel_500: + expected_probe_cancel_500s[0] = max(0, expected_probe_cancel_500s[0] - 1) + try: + expect(send_btn).to_be_visible(timeout = 15_000) + except Exception: + shoot(f"{label}-idle-restore-FAIL") + fail( + "Composer did not return to idle after the submit probe; " + "Send button is unavailable for the next IME regression case." + ) + # 3. Baseline: ASCII keyboard typing works. Bail fast if not. step("baseline ASCII keyboard typing") clear() @@ -437,7 +463,7 @@ with sync_playwright() as p: bubbles:true, inputType:'insertCompositionText', data:'你好', isComposing:true, })); - // Deliberately omit compositionend — that is the WSL/Chrome + // Deliberately omit compositionend; that is the WSL/Chrome // bug surface. The watchdog in useImeComposerInputHandlers // should reset isComposing after IME_STUCK_TIMEOUT_MS. }""" @@ -453,7 +479,7 @@ with sync_playwright() as p: except Exception: shoot("06b-compositionend-watchdog-FAIL") fail( - "Send button stayed disabled with no compositionend — " + "Send button stayed disabled with no compositionend; " "watchdog did not release the composing flag (issue #5546)." ) after_value = read_value() @@ -521,7 +547,7 @@ with sync_playwright() as p: # 6d. Keydown re-pin must also re-arm the watchdog. On the WSL+Chrome # stuck-compositionend path no follow-up event arrives, so after keydown # re-pins composingRef the watchdog must clear it again or Send re-locks - # permanently. (Codex P1, commit 597af0d0.) + # permanently. step("BUG REPRO: keydown re-pin re-arms watchdog (#5546 follow-up regression)") clear() composer.click() @@ -582,7 +608,7 @@ with sync_playwright() as p: fail( "After the keydown re-pin the watchdog never re-armed; Send " "stayed permanently locked on the WSL+Chrome stuck-end path " - "(#5546 follow-up Codex P1)." + "(#5546 follow-up regression)." ) info( "watchdog re-armed after keydown re-pin: textarea flushed from " @@ -590,13 +616,196 @@ with sync_playwright() as p: ) shoot("06d-keydown-rearm") info("keydown re-pin re-arm PASS") + restore_idle_composer_after_probe("06d-keydown-rearm") + clear() + + # 6e. Mac input-method switch - onKeyDown immediate recovery. + # On macOS, pressing Ctrl+Space or clicking the menu-bar language icon + # fires compositionstart but never fires compositionend (the OS commits + # nothing because no candidate was selected). When the user types their + # first English key after switching back, onKeyDown receives a native + # event with isComposing=false and a regular keyCode. The else-if branch + # added for this bug clears composingRef immediately, before the 2500ms + # watchdog would fire, so Send is unblocked on that very keystroke. + # + # To isolate the onKeyDown else-if path (and not the onChange path which + # also clears composing on normal input), we dispatch a synthetic KeyboardEvent + # with isComposing=false but do NOT dispatch a follow-up input event. + # onChange never fires, so the only recovery path is onKeyDown. + step("BUG REPRO: Mac IME switch - onKeyDown immediate recovery") + clear() + composer.click() + # Seed sendable content so the Send button's state reflects composition + # state only, not empty-content gating. set_value_via_setter uses + # insertFromPaste which is not composing, so composingRef stays false here. + set_value_via_setter("hello") + # Simulate switching TO Chinese input method: compositionstart fires but + # compositionend never arrives (user switched away without committing text). + composer.evaluate( + """(el) => { + el.focus(); + el.dispatchEvent(new CompositionEvent('compositionstart', {bubbles:true, data:''})); + }""" + ) + # Give React a tick to process the compositionstart and update isComposing. + page.wait_for_timeout(200) + send_btn_mac_kd = page.locator('button[aria-label="Send message"]') + # Dispatch ONLY a keydown (isComposing=false, keyCode=65) with no follow-up + # input event. This fires onKeyDown but NOT onChange, so the else-if branch + # is the only path that can clear composingRef. page.keyboard.type() would + # also fire an input event and trigger onChange, which already clears + # composing on ASCII input, which would make the test a false positive. + composer.evaluate( + """(el) => { + el.focus(); + el.dispatchEvent(new KeyboardEvent('keydown', { + bubbles: true, key: 'a', code: 'KeyA', keyCode: 65, + isComposing: false, + })); + }""" + ) + if send_btn_mac_kd.count() == 0: + soft_fail("Send button not found for Mac IME switch (onKeyDown) repro") + else: + try: + # 1500ms is well below the 2500ms watchdog: only the onKeyDown + # else-if path can clear composingRef this quickly. + expect(send_btn_mac_kd).not_to_be_disabled(timeout = 1_500) + info( + "Send button enabled within 1500ms after Mac IME switch + " + "English keydown (onKeyDown else-if branch fired, not watchdog)" + ) + except Exception: + shoot("06e-mac-ime-keydown-FAIL") + fail( + "Send button stayed disabled after Mac input-method switch + " + "English keydown; the onKeyDown else-if branch did not clear " + "composingRef immediately (expected recovery in < 1500ms)." + ) + shoot("06e-mac-ime-keydown") + info("Mac IME switch onKeyDown recovery PASS") + clear() + + # 6f. Candidate-confirming Enter must not unblock submit. + # Some IMEs/browsers report the candidate-confirming Enter as + # isComposing=false with keyCode=13 while composingRef is still pinned. + # That Enter must be swallowed and keep Send disabled; otherwise it can + # become a form submit before the candidate is committed. + step("BUG REPRO: Mac IME switch - Enter must not unblock submit") + clear() + composer.click() + set_value_via_setter("hello") + composer.evaluate( + """(el) => { + el.focus(); + el.dispatchEvent(new CompositionEvent('compositionstart', {bubbles:true, data:''})); + }""" + ) + page.wait_for_timeout(200) + send_btn_mac_enter = page.locator('button[aria-label="Send message"]') + composer.evaluate( + """(el) => { + el.focus(); + el.dispatchEvent(new KeyboardEvent('keydown', { + bubbles: true, cancelable: true, key: 'Enter', code: 'Enter', + keyCode: 13, isComposing: false, + })); + }""" + ) + try: + expect(send_btn_mac_enter).to_be_disabled(timeout = 500) + info("Enter while composingRef is stuck kept Send disabled") + except Exception: + shoot("06f-mac-ime-enter-guard-FAIL") + fail( + "Enter cleared a stuck composingRef immediately; candidate-confirming " + "Enter can fall through to submit." + ) + composer.evaluate( + """(el) => { + el.focus(); + el.dispatchEvent(new KeyboardEvent('keydown', { + bubbles: true, key: 'a', code: 'KeyA', keyCode: 65, + isComposing: false, + })); + }""" + ) + try: + expect(send_btn_mac_enter).not_to_be_disabled(timeout = 1_500) + info("non-Enter key after Enter guard still recovers immediately") + except Exception: + shoot("06f-mac-ime-enter-guard-recovery-FAIL") + fail("After guarding Enter, a later non-IME key did not clear composingRef immediately.") + shoot("06f-mac-ime-enter-guard") + info("Mac IME switch Enter guard PASS") + clear() + + # 6g. Mac input-method switch - onBlur immediate recovery. + # Some Mac IME switches steal focus from the textarea (e.g. clicking + # the menu-bar language icon). The onBlur handler added for this bug + # resets composingRef unconditionally when the textarea loses focus. + # This is always safe: the OS commits or cancels any active composition + # before surrendering focus, so blur is a reliable reset point. + step("BUG REPRO: Mac IME switch - onBlur immediate recovery") + clear() + composer.click() + # Seed sendable content so the Send button's enabled/disabled state reflects + # composition state only, not empty-content gating. + set_value_via_setter("hello") + # Simulate switching TO Chinese: compositionstart fires, compositionend never comes. + composer.evaluate( + """(el) => { + el.focus(); + el.dispatchEvent(new CompositionEvent('compositionstart', {bubbles:true, data:''})); + }""" + ) + page.wait_for_timeout(200) + send_btn_mac_blur = page.locator('button[aria-label="Send message"]') + # Blur the textarea to simulate the OS stealing focus during an IME switch + # (e.g. the user clicks the menu-bar language icon). + composer.evaluate("(el) => el.blur()") + # onBlur calls setCompositionState(false) immediately; re-focus so React + # can render the updated Send-button state and we can locate it. + composer.click() + if send_btn_mac_blur.count() == 0: + soft_fail("Send button not found for Mac IME switch (onBlur) repro") + else: + try: + # 1500ms is well below the 2500ms watchdog: only onBlur can clear + # composingRef this quickly when no keydown is fired. + expect(send_btn_mac_blur).not_to_be_disabled(timeout = 1_500) + info( + "Send button enabled within 1500ms after Mac IME switch + " + "textarea blur (onBlur handler fired, not watchdog)" + ) + except Exception: + shoot("06f-mac-ime-blur-FAIL") + fail( + "Send button stayed disabled after Mac input-method switch + " + "textarea blur; the onBlur handler did not reset composingRef " + "(expected recovery in < 1500ms)." + ) + shoot("06g-mac-ime-blur") + info("Mac IME switch onBlur recovery PASS") clear() # 7. Final state. Filter benign 401 noise from the change-password redirect # via is_benign_*; fail only on real errors. shoot("07-final") real_page_errors = [e for e in page_errors if not is_benign_page_error(e)] - real_console_errors = [e for e in console_errors if not is_benign_console_error(e)] + probe_cancel_500_allowance = expected_probe_cancel_500s[0] + real_console_errors = [] + for error in console_errors: + if is_benign_console_error(error): + continue + if ( + probe_cancel_500_allowance > 0 + and "Failed to load resource: the server responded with a status of 500" in error + and "Internal Server Error" in error + ): + probe_cancel_500_allowance -= 1 + continue + real_console_errors.append(error) info( f"page_errors={len(page_errors)} ({len(real_page_errors)} non-benign); " f"console_errors={len(console_errors)} " @@ -621,7 +830,9 @@ with sync_playwright() as p: f"DONE: ascii=OK paste={len(I18N_SAMPLES)}/{len(I18N_SAMPLES)} " f"normal_composition=OK stuck_recovery=OK " f"compositionend_watchdog=OK keydown_repin=OK " - f"keydown_repin_rearm=OK" + f"keydown_repin_rearm=OK " + f"mac_ime_switch_keydown=OK mac_ime_switch_enter_guard=OK " + f"mac_ime_switch_blur=OK" ) _watchdog.cancel() browser.close() diff --git a/tests/studio/test_composer_rtl_bidi_attribute.py b/tests/studio/test_composer_rtl_bidi_attribute.py index 33e5c450cf..f1eb75035c 100644 --- a/tests/studio/test_composer_rtl_bidi_attribute.py +++ b/tests/studio/test_composer_rtl_bidi_attribute.py @@ -155,8 +155,7 @@ def _extract_block( def test_main_composer_keydown_rearms_watchdog(): """After the keydown re-pin sets composingRef=true the watchdog must be re-armed; otherwise the WSL+Chrome no-compositionend path this PR - targets would lock Send permanently after any IME keypress - (Codex P1 on commit 597af0d0).""" + targets would lock Send permanently after any IME keypress.""" src = THREAD_TSX.read_text() block = _extract_block(src, "const onKeyDown = useCallback") assert "refreshStuckTimer" in block, ( @@ -168,7 +167,7 @@ def test_main_composer_keydown_rearms_watchdog(): "clearStuckTimer,", "" ), ( "main composer keydown gate must not leave the watchdog only " - "cleared — that's the Codex P1 regression" + "cleared; that would regress the stuck-compositionend path" ) @@ -180,3 +179,37 @@ def test_compare_composer_keydown_rearms_watchdog(): "compare composer keydown gate must call refreshStuckImeTimer " "after re-pinning composingRef" ) + + +def _assert_enter_guard_before_immediate_recovery(block: str, refresh_call: str) -> None: + enter_idx = block.find('e.key === "Enter"') + recovery_idx = block.find("setCompositionState(false)") + assert enter_idx != -1, "keydown handler is missing an Enter guard" + assert recovery_idx != -1, "keydown handler is missing immediate recovery" + assert enter_idx < recovery_idx, ( + "stuck-composition recovery must guard Enter before clearing " + "composingRef; candidate-confirming Enter must not submit" + ) + guard_block = block[enter_idx:recovery_idx] + assert "preventDefault()" in guard_block, ( + "Enter while composingRef is stuck must prevent the same key from " + "falling through to submit" + ) + assert ( + refresh_call in guard_block + ), "Enter while composingRef is stuck must keep the watchdog armed" + assert ( + "return;" in guard_block + ), "Enter while composingRef is stuck must not reach immediate recovery" + + +def test_main_composer_stuck_enter_does_not_clear_before_submit(): + src = THREAD_TSX.read_text() + block = _extract_block(src, "const onKeyDown = useCallback") + _assert_enter_guard_before_immediate_recovery(block, "refreshStuckTimer") + + +def test_compare_composer_stuck_enter_does_not_clear_before_submit(): + src = SHARED_TSX.read_text() + block = _extract_block(src, "function onKeyDown", opener = "{", closer = "}") + _assert_enter_guard_before_immediate_recovery(block, "refreshStuckImeTimer") From 08c3878919f48bb6a294ff4aaf66b645fab7310a Mon Sep 17 00:00:00 2001 From: Matt Van Horn Date: Mon, 15 Jun 2026 04:26:04 -0700 Subject: [PATCH 07/91] fix: use partial hipinfo output on crash to avoid CPU fallback (RDNA 4 / gfx1200) (#6292) * fix: use partial hipinfo output on crash to avoid CPU fallback (#6043) `hipinfo.exe` on some RDNA 4 hosts (e.g. RX 9060 XT / gfx1200) exits with STATUS_ACCESS_VIOLATION (0xC0000005) after printing the gcnArchName line. The previous guard `$LASTEXITCODE -eq 0` in studio/setup.ps1 and `if result.returncode == 0` in install_python_stack.py discarded this partial-but-valid output, causing the installer to fall through to WMI name inference which sets HasROCm=false and installs CPU PyTorch instead of the ROCm wheel. Fix: check for gcnArchName in stdout first; accept the arch regardless of exit code. Only fall through to the amd-smi / WMI path when no gcnArchName is present at all (crash before any output, or a genuine "no device" error). A cyan INFO substep is emitted when the arch is recovered from a crashed hipinfo run so users can see what happened. Adds a regression test covering the crash-with-valid-output path. Fixes #6043 * Fix/adjust hipinfo crash fallback for PR #6292 --------- Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: wasimysaid Co-authored-by: Lee Jackson <130007945+Imagineer99@users.noreply.github.com> --- install.ps1 | 11 +++++-- studio/install_python_stack.py | 24 ++++++++------ studio/setup.ps1 | 11 +++++-- tests/studio/install/test_rocm_support.py | 40 +++++++++++++++++++++-- 4 files changed, 70 insertions(+), 16 deletions(-) diff --git a/install.ps1 b/install.ps1 index dccd8e4fc1..1073ab5726 100644 --- a/install.ps1 +++ b/install.ps1 @@ -1553,7 +1553,9 @@ shell.Run cmd, 0, False $HipSdkInstalled = $true # binary found → SDK is installed regardless of device state try { $hipOut = & $hipinfoExe.Source 2>&1 | Out-String - if ($LASTEXITCODE -eq 0 -and $hipOut -match "(?i)gcnArchName") { + if ($hipOut -match "(?i)gcnArchName") { + # hipinfo can crash after printing gcnArchName (#6043). + # Once the arch is printed, keep the ROCm wheel path. $HasROCm = $true $_hipAllArches = @([regex]::Matches($hipOut, "(?im)^\s*gcnArchName\s*:\s*(\S+)") | ForEach-Object { ($_.Groups[1].Value -split ':')[0].Trim().ToLower() }) $_hipVisIdx = if ($env:HIP_VISIBLE_DEVICES -match '^\d') { [int]($env:HIP_VISIBLE_DEVICES -split ',')[0] } elseif ($env:ROCR_VISIBLE_DEVICES -match '^\d') { [int]($env:ROCR_VISIBLE_DEVICES -split ',')[0] } else { 0 } @@ -1563,8 +1565,13 @@ shell.Run cmd, 0, False } else { $ROCmGpuLabel = "AMD ROCm" } + if ($LASTEXITCODE -ne 0) { + Write-Host " [INFO] hipinfo exited with code $LASTEXITCODE but reported gcnArchName -- treating as ROCm-capable (see #6043)" -ForegroundColor Cyan + } } elseif ($LASTEXITCODE -ne 0) { - # hipinfo ran but returned a HIP runtime error (e.g. "no ROCm-capable device detected") + # hipinfo ran but returned a HIP runtime error without any gcnArchName + # output (e.g. "no ROCm-capable device detected"), or crashed before + # printing device info. $firstLine = ($hipOut -split '\r?\n' | Where-Object { $_.Trim() } | Select-Object -First 1) Write-Host " [WARN] hipinfo returned a HIP runtime error (exit $LASTEXITCODE)" -ForegroundColor Yellow Write-Host " $firstLine" -ForegroundColor Yellow diff --git a/studio/install_python_stack.py b/studio/install_python_stack.py index d404041ca7..a8bd73ad9b 100644 --- a/studio/install_python_stack.py +++ b/studio/install_python_stack.py @@ -354,16 +354,20 @@ def _detect_windows_gfx_arch() -> str | None: stderr = subprocess.DEVNULL, timeout = 10, ) - if result.returncode == 0: - text = result.stdout.decode(errors = "replace") - # findall gets every gcnArchName line so multi-GPU hosts are - # enumerable and HIP_VISIBLE_DEVICES selects correctly. - _tokens = [ - t.strip().lower() for t in re.findall(r"(?im)^\s*gcnArchName\s*:\s*(\S+)", text) - ] - _pick = _dedup_pick(_tokens) - if _pick: - return _pick + # Accept partial output even when hipinfo crashes (e.g. exit code + # 0xC0000005 / STATUS_ACCESS_VIOLATION on some RDNA 4 hosts): if + # gcnArchName is present in stdout the device was enumerated before + # the crash, so the arch is trustworthy. Ignoring it causes a + # silent CPU PyTorch fallback (issue #6043). + text = result.stdout.decode(errors = "replace") + # findall gets every gcnArchName line so multi-GPU hosts are + # enumerable and HIP_VISIBLE_DEVICES selects correctly. + _tokens = [ + t.strip().lower() for t in re.findall(r"(?im)^\s*gcnArchName\s*:\s*(\S+)", text) + ] + _pick = _dedup_pick(_tokens) + if _pick: + return _pick except Exception: pass diff --git a/studio/setup.ps1 b/studio/setup.ps1 index acc7e92875..f02ed586be 100644 --- a/studio/setup.ps1 +++ b/studio/setup.ps1 @@ -823,7 +823,9 @@ if (-not $HasNvidiaSmi) { $HipSdkInstalled = $true # binary found → SDK is installed regardless of device state try { $hipOut = & $hipinfoExe.Source 2>&1 | Out-String - if ($LASTEXITCODE -eq 0 -and $hipOut -match "(?i)gcnArchName") { + if ($hipOut -match "(?i)gcnArchName") { + # hipinfo can crash after printing gcnArchName (#6043). + # Once the arch is printed, keep the ROCm wheel path. $HasROCm = $true $_hipAllArches = @([regex]::Matches($hipOut, "(?im)^\s*gcnArchName\s*:\s*(\S+)") | ForEach-Object { ($_.Groups[1].Value -split ':')[0].Trim().ToLower() }) $_hipVisIdx = if ($env:HIP_VISIBLE_DEVICES -match '^\d') { [int]($env:HIP_VISIBLE_DEVICES -split ',')[0] } elseif ($env:ROCR_VISIBLE_DEVICES -match '^\d') { [int]($env:ROCR_VISIBLE_DEVICES -split ',')[0] } else { 0 } @@ -833,8 +835,13 @@ if (-not $HasNvidiaSmi) { } else { $ROCmGpuLabel = "AMD ROCm" } + if ($LASTEXITCODE -ne 0) { + substep "[INFO] hipinfo exited with code $LASTEXITCODE but reported gcnArchName -- treating as ROCm-capable (see #6043)" "Cyan" + } } elseif ($LASTEXITCODE -ne 0) { - # hipinfo ran but returned a HIP runtime error (e.g. "no ROCm-capable device detected") + # hipinfo ran but returned a HIP runtime error without any gcnArchName + # output (e.g. "no ROCm-capable device detected"), or crashed before + # printing device info. $firstLine = ($hipOut -split '\r?\n' | Where-Object { $_.Trim() } | Select-Object -First 1) substep "[WARN] hipinfo returned a HIP runtime error (exit $LASTEXITCODE)" "Yellow" substep " $firstLine" "Yellow" diff --git a/tests/studio/install/test_rocm_support.py b/tests/studio/install/test_rocm_support.py index a3c9555d0b..b0abaaeacd 100644 --- a/tests/studio/install/test_rocm_support.py +++ b/tests/studio/install/test_rocm_support.py @@ -1820,10 +1820,27 @@ class TestDetectWindowsGfxArch: result = stack_mod._detect_windows_gfx_arch() assert result == "gfx1200" - def test_returns_none_on_nonzero_returncode(self): + def test_returns_arch_on_crash_with_gcnarchname_in_output(self): + # Regression test for issue #6043: hipinfo may exit with a non-zero + # code (e.g. 0xC0000005 / STATUS_ACCESS_VIOLATION on RDNA 4 hosts) + # while still printing the gcnArchName line before crashing. The + # previous guard `if result.returncode == 0` discarded this output, + # causing a CPU PyTorch fallback. The fix: accept the arch whenever + # gcnArchName is present in stdout regardless of exit code. + mock_result = MagicMock() + mock_result.returncode = -1073741819 # 0xC0000005 STATUS_ACCESS_VIOLATION + mock_result.stdout = b"gcnArchName : gfx1200\nsome other line\n" + with patch("shutil.which", return_value = "/usr/bin/hipinfo"): + with patch("subprocess.run", return_value = mock_result): + result = stack_mod._detect_windows_gfx_arch() + assert result == "gfx1200" + + def test_returns_none_on_nonzero_returncode_without_gcnarchname(self): + # Non-zero exit without any gcnArchName output (e.g. no device detected) + # must still return None and fall through to amd-smi / WMI. mock_result = MagicMock() mock_result.returncode = 1 - mock_result.stdout = b"gcnArchName : gfx1200\n" + mock_result.stdout = b"HIP runtime error: no device detected\n" with patch("shutil.which", return_value = "/usr/bin/hipinfo"): with patch("subprocess.run", return_value = mock_result): result = stack_mod._detect_windows_gfx_arch() @@ -2673,6 +2690,15 @@ class TestHipSdkEnvPathResolution: """Verify that both install scripts resolve hipinfo/hipconfig via HIP_PATH and ROCM_PATH when the tools are not on $PATH, and emit explicit warnings.""" + @staticmethod + def _assert_accepts_partial_hipinfo_output(source: str): + hipout_idx = source.find("$hipOut = & $hipinfoExe.Source") + assert hipout_idx != -1 + hipinfo_block = source[hipout_idx : hipout_idx + 1600] + assert 'if ($hipOut -match "(?i)gcnArchName")' in hipinfo_block + assert "$LASTEXITCODE -eq 0 -and $hipOut -match" not in hipinfo_block + assert "but reported gcnArchName" in hipinfo_block + # ── hipinfo resolution ──────────────────────────────────────────────────── def test_setup_checks_hip_path_for_hipinfo(self): @@ -2747,6 +2773,16 @@ class TestHipSdkEnvPathResolution: source = _INSTALL_PS1_PATH.read_text(encoding = "utf-8") assert "HIP runtime error" in source or "runtime error" in source.lower() + def test_setup_accepts_hipinfo_gcnarchname_on_nonzero_exit(self): + """setup.ps1 must accept partial hipinfo output from the #6043 crash path.""" + source = _SETUP_PS1_PATH.read_text(encoding = "utf-8") + self._assert_accepts_partial_hipinfo_output(source) + + def test_install_accepts_hipinfo_gcnarchname_on_nonzero_exit(self): + """install.ps1 must accept partial hipinfo output from the #6043 crash path.""" + source = _INSTALL_PS1_PATH.read_text(encoding = "utf-8") + self._assert_accepts_partial_hipinfo_output(source) + # ── hipconfig resolution ────────────────────────────────────────────────── def test_setup_resolves_hipconfig_via_bin_subdir(self): From 9d7740a82f2ec2649c74d95154e91f63c376b3ef Mon Sep 17 00:00:00 2001 From: Wasim Yousef Said Date: Mon, 15 Jun 2026 13:38:39 +0200 Subject: [PATCH 08/91] Rename chat artifacts copy to canvas (#6298) Co-authored-by: Lee Jackson <130007945+Imagineer99@users.noreply.github.com> --- studio/backend/core/inference/tools.py | 14 ++++++------- studio/backend/routes/inference.py | 12 +++++------ .../backend/tests/test_anthropic_messages.py | 4 ++-- .../backend/tests/test_llama_cpp_tool_loop.py | 20 +++++++++---------- .../tests/test_safetensors_tool_loop.py | 8 ++++---- .../tests/test_tool_loop_controller.py | 2 +- .../components/assistant-ui/markdown-text.tsx | 6 +++--- .../components/assistant-ui/tool-group.tsx | 2 +- .../assistant-ui/tool-ui-render-html.tsx | 12 +++++------ .../src/features/chat/api/chat-adapter.ts | 12 +++++------ .../features/chat/artifacts/artifact-card.tsx | 2 +- .../chat/artifacts/artifact-surface.tsx | 20 +++++++++---------- .../features/chat/artifacts/html-frame.tsx | 4 ++-- .../src/features/chat/artifacts/types.ts | 4 ++-- .../frontend/src/features/chat/chat-page.tsx | 2 +- studio/frontend/src/i18n/locales/en.ts | 8 ++++---- 16 files changed, 66 insertions(+), 66 deletions(-) diff --git a/studio/backend/core/inference/tools.py b/studio/backend/core/inference/tools.py index b29a221ee7..6960310018 100644 --- a/studio/backend/core/inference/tools.py +++ b/studio/backend/core/inference/tools.py @@ -773,10 +773,10 @@ RENDER_HTML_TOOL = { "function": { "name": "render_html", "description": ( - "Render a self-contained HTML/CSS/JavaScript artifact for the user. " + "Render a self-contained HTML/CSS/JavaScript canvas for the user. " "Call this at most once per assistant response unless the user " "explicitly asks for changes in that response. Future user requests " - "for new artifacts may call render_html once. Put the entire document " + "for new canvases may call render_html once. Put the entire document " "in code, including any CSS in