diff --git a/studio/backend/core/inference/tools.py b/studio/backend/core/inference/tools.py index af9c06d846..541f6f530a 100644 --- a/studio/backend/core/inference/tools.py +++ b/studio/backend/core/inference/tools.py @@ -143,8 +143,8 @@ _BLOCKED_COMMANDS = ( else _BLOCKED_COMMANDS_COMMON ) -# Blocked commands that reach the network / another machine; hitting one means -# the model wants files the sandbox cannot reach, so we steer it to ask for an upload. +# Blocked commands that reach the network / another machine; hitting one steers +# the model to fetch via code or ask for an accessible path. _NETWORK_BLOCKED_COMMANDS = frozenset( {"curl", "wget", "nc", "ncat", "netcat", "socat", "ssh", "scp", "sftp", "rsync"} ) @@ -2949,16 +2949,12 @@ WEB_SEARCH_TOOL = { }, } -# Appended to the python/terminal descriptions: stop models writing to a -# nonexistent /mnt/data or cd/grep-ing a guessed local path for a repo the -# user only mentioned but never uploaded. -# Split so the Bypass Permissions variant can drop the network-restriction -# sentence: under bypass, _python_exec/_bash_exec skip the safety analysis and -# the curl/wget blocklist (network policy is enforced only by that AST host check -# and the bash blocklist -- there is no network namespace), so egress is not -# limited to the allowlist and curl/wget work. Keeping the sentence there would -# falsely tell a full-permissions session those downloads are unavailable and can -# block a user-supplied remote resource. +# Appended to python/terminal descriptions: stop models writing to /mnt/data or +# guessing a local path for a repo the user mentioned but never uploaded. +# Split so the Bypass Permissions variant can drop the network sentence: under +# bypass, _python_exec/_bash_exec skip the safety analysis and curl/wget blocklist +# (there is no network namespace), so egress works and claiming those downloads +# are unavailable would falsely block a user-supplied remote resource. _SANDBOX_PATHS_NOTE_INTRO = ( " The working directory is an isolated scratch space that may already hold " "files from earlier work in this conversation or project, plus anything you " @@ -2984,13 +2980,13 @@ _SANDBOX_PATHS_NOTE_TAIL = ( "you need are not here, ask the user to provide them or an exact path " "instead of guessing one." ) -# Default (sandboxed) note: steers the python tool at public sources and keeps -# the curl/wget restriction (without overstating the host check as a hard wall). +# Default (sandboxed) note: steers python at public sources, keeps the curl/wget +# restriction without overstating the host check as a hard wall. _SANDBOX_PATHS_NOTE = ( _SANDBOX_PATHS_NOTE_INTRO + _SANDBOX_PATHS_NOTE_NETWORK + _SANDBOX_PATHS_NOTE_TAIL ) -# Bypass Permissions variant: same guidance without the network-restriction -# sentence that bypass removes (stays neutral rather than claiming egress works). +# Bypass variant: same guidance minus the network sentence (stays neutral rather +# than claiming egress works). _SANDBOX_PATHS_NOTE_BYPASS = _SANDBOX_PATHS_NOTE_INTRO + _SANDBOX_PATHS_NOTE_TAIL PYTHON_TOOL = { @@ -3096,8 +3092,8 @@ ALL_TOOLS = [ def _with_sandbox_note(tool: dict, note: str) -> dict: - """Shallow copy of a python/terminal tool spec with its sandbox-paths note - swapped for ``note`` (the default note is stripped first).""" + """Copy of a python/terminal tool spec with its sandbox-paths note swapped for + ``note`` (the default note is stripped first).""" fn = dict(tool["function"]) base = fn["description"] if base.endswith(_SANDBOX_PATHS_NOTE): @@ -3106,8 +3102,7 @@ def _with_sandbox_note(tool: dict, note: str) -> dict: return {**tool, "function": fn} -# Bypass Permissions variants: descriptions omit the allowlist/curl/wget -# restriction because that safety analysis and blocklist are skipped when the +# Bypass variants: descriptions omit the curl/wget restriction, skipped when the # sandbox is disabled (disable_sandbox = bypass_permissions in the tool loops). PYTHON_TOOL_BYPASS = _with_sandbox_note(PYTHON_TOOL, _SANDBOX_PATHS_NOTE_BYPASS) TERMINAL_TOOL_BYPASS = _with_sandbox_note(TERMINAL_TOOL, _SANDBOX_PATHS_NOTE_BYPASS) @@ -3118,10 +3113,8 @@ _BYPASS_TOOL_OVERRIDES = { def apply_bypass_tool_notes(tools: list[dict]) -> list[dict]: - """Return ``tools`` with the python/terminal specs swapped for their Bypass - Permissions variants (only their descriptions differ). Call this for a request - whose execution disables the sandbox so the note matches what the tools - actually enforce; a no-op for tool lists without python/terminal.""" + """Return ``tools`` with python/terminal swapped for their Bypass variants + (only the description differs); no-op for lists without python/terminal.""" swapped = False result: list[dict] = [] for tool in tools: diff --git a/studio/backend/routes/inference.py b/studio/backend/routes/inference.py index 6690253853..e3612aad72 100644 --- a/studio/backend/routes/inference.py +++ b/studio/backend/routes/inference.py @@ -2146,14 +2146,10 @@ def _permission_mode_confirm(payload) -> bool: def _sandbox_disabled(payload) -> bool: """Whether python/terminal execution actually runs unsandboxed for this request. - Both agent loops (llama_cpp.py and safetensors_agentic.py) normalize the - permissive switch the same way: permission_mode "full" is turned into - bypass_permissions=True, and an explicit bypass_permissions is turned into - permission_mode "full". Execution then passes disable_sandbox=bypass_permissions, - so "full" and bypass_permissions are equivalent and BOTH skip _check_code_safety - and the terminal curl/wget blocklist. The tool notes and action nudge must match - what executes, so they key off this effective flag rather than bypass_permissions - alone. + Both agent loops fold permission_mode "full" and bypass_permissions into the + same disable_sandbox flag, so both skip _check_code_safety and the curl/wget + blocklist. The tool notes and nudge key off this effective flag, not + bypass_permissions alone. """ return bool(getattr(payload, "bypass_permissions", False)) or ( getattr(payload, "permission_mode", None) == "full" @@ -2430,9 +2426,8 @@ _TOOL_CODE_TIP = ( "you need is not present, ask the user to provide it or give an exact path " "rather than running commands against a guessed one." ) -# Bypass Permissions variant: drops the "internet access is limited" clause, -# which is false when the sandbox is disabled (curl/wget and arbitrary-host -# requests work), while keeping the workdir-default framing. +# Bypass variant: drops the "internet access is limited" clause (false when the +# sandbox is disabled), keeping the workdir-default framing. _TOOL_CODE_TIP_BYPASS = ( "Use code execution for math, calculations, data processing, or to parse " "and analyze information from tool results. It runs in a sandbox whose " @@ -2523,10 +2518,8 @@ async def _select_request_tools( # Drop the RAG tool without a scope: nothing to search over. if not payload.rag_scope: tools = [t for t in tools if t["function"]["name"] != "search_knowledge_base"] - # A sandbox-disabled request (bypass_permissions, or permission_mode "full" - # which both loops normalize to bypass_permissions=True) runs python/terminal - # with disable_sandbox=True, so the descriptions must not claim the - # allowlist/curl-wget block that no longer applies at execution. + # A sandbox-disabled request runs python/terminal with disable_sandbox=True, so + # the descriptions must not claim the curl/wget block that no longer applies. if _sandbox_disabled(payload): tools = apply_bypass_tool_notes(tools) if mcp_allowed: @@ -4306,12 +4299,10 @@ async def _load_model_impl(request: LoadRequest, fastapi_request: Request, curre # parse against a freshly-supplied first-class field. if request.llama_extra_args is None and llama_backend.extra_args: source = llama_backend.extra_args_source - # Compare against the resolved variant, not the request - # field: callers commonly omit gguf_variant for local - # ``.gguf`` paths and HF auto-pick flows. ``config.gguf_ - # variant`` is the variant load_model was actually - # invoked with (see the HF / local branches below), so - # both sides of the comparison key off the same string. + # Compare against the resolved variant, not the request field: + # callers commonly omit gguf_variant for local ``.gguf`` paths and + # HF auto-pick flows. ``config.gguf_variant`` is the variant + # load_model was actually invoked with, so both sides key off it. resolved_variant = (config.gguf_variant or "").lower() request_variant = (request.gguf_variant or "").lower() stored_variant = (source[1] or "").lower() if source else "" @@ -4333,15 +4324,12 @@ async def _load_model_impl(request: LoadRequest, fastapi_request: Request, curre # inherit via "no opinion" semantics. extra_llama_args = [] else: - # Strip only the groups whose first-class field was set by - # the caller, so an inherited --chat-template-file survives - # an Apply that omits chat_template_override. A bundled family - # template (e.g. the gemma-4 override) is an effective - # first-class template setting even when the raw request - # omits chat_template_override, so strip the inherited - # --chat-template-file in that case too -- otherwise the stale - # extra arg (appended last) shadows the bundled template while - # Unsloth reports the bundled template's capabilities. + # Strip only the groups whose first-class field the caller set, + # so an inherited --chat-template-file survives an Apply that + # omits chat_template_override. A bundled family template (e.g. + # gemma-4) counts as first-class too, so strip the inherited + # --chat-template-file then as well, else the stale arg (appended + # last) shadows it while Unsloth reports its capabilities. fields_set = getattr(request, "model_fields_set", set()) stripped = strip_shadowing_flags( llama_backend.extra_args, @@ -12599,9 +12587,8 @@ async def anthropic_messages( requested_studio_tools, payload.enabled_tools, ) - # A sandbox-disabled request (bypass_permissions or permission_mode "full") - # runs unsandboxed, so drop the allowlist/curl-wget restriction from the - # python/terminal descriptions here too. + # A sandbox-disabled request runs unsandboxed, so drop the curl/wget + # restriction from the python/terminal descriptions here too. if _sandbox_disabled(payload): openai_tools = apply_bypass_tool_notes(openai_tools) diff --git a/studio/backend/tests/test_anthropic_messages.py b/studio/backend/tests/test_anthropic_messages.py index f53dbbd7fb..fc0839ce47 100644 --- a/studio/backend/tests/test_anthropic_messages.py +++ b/studio/backend/tests/test_anthropic_messages.py @@ -174,10 +174,8 @@ class TestToolActionNudge: assert _build_tool_action_nudge(tools = [], model_name = "Llama-3.1-8B-Instruct") == "" def test_code_tip_does_not_name_a_disabled_sibling_code_tool(self): - # has_code fires when either python or terminal is present, so the code tip - # must stay tool-neutral: a request that enabled only one must never name - # the other as an available tool (the missing-tool hallucination the - # render_html gate already avoids). + # has_code fires for either python or terminal, so the code tip must stay + # tool-neutral: enabling one must never name the other as available. py_only = _build_tool_action_nudge( tools = [{"type": "function", "function": {"name": "python"}}], model_name = "Llama-3.1-8B-Instruct", diff --git a/studio/backend/tests/test_sandbox_paths_note.py b/studio/backend/tests/test_sandbox_paths_note.py index e1e33ee62b..8cb68825cc 100644 --- a/studio/backend/tests/test_sandbox_paths_note.py +++ b/studio/backend/tests/test_sandbox_paths_note.py @@ -1,17 +1,12 @@ # SPDX-License-Identifier: AGPL-3.0-only # Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 -"""Accuracy guards for the sandbox-paths note appended to the python/terminal -tool descriptions (#7242). +"""Accuracy guards for the sandbox-paths note on the python/terminal tools (#7242). -The note must not misdirect the model with claims that are false for the real -sandbox: (1) the sandbox is not network-isolated -- the python tool should fetch -from public hosts (github.com / huggingface.co / pypi.org), so it must not claim -it "cannot reach ... remote hosts" at all, nor overstate the host check as a hard -wall that makes private/arbitrary hosts impossible; (2) a project sandbox is a -shared per-project directory, so a new thread can inherit files from prior -threads -- the note must not claim the workdir "starts empty" or "persists only -for this conversation". +The note must not misdirect the model: (1) the sandbox is not network-isolated -- +the python tool fetches from public hosts, so it must not claim it "cannot reach +remote hosts" nor overstate the host check as a hard wall; (2) a project sandbox +is shared across threads, so it must not claim the workdir "starts empty". """ from __future__ import annotations @@ -37,14 +32,13 @@ from core.inference.tools import ( def test_note_does_not_claim_full_network_isolation(): lowered = _SANDBOX_PATHS_NOTE.lower() - # The old absolute claim is false for the python tool (it does reach egress). + # False for the python tool, which does reach egress. assert "cannot reach other machines or remote hosts" not in lowered - # It must not overstate the host check as an enforced hard boundary: - # _sandbox_preexec leaves networking on and the AST check only inspects - # literal hosts, so a dynamically built request to a private host still runs. + # Don't overstate the host check as a hard wall: networking stays on and the + # AST check only inspects literal hosts, so a dynamic private-host request runs. assert "only from a fixed allowlist" not in lowered assert "arbitrary addresses" not in lowered - # It should still steer to public sources so the model fetches valid URLs. + # Still steer to public sources so the model fetches valid URLs. assert "public sources" in lowered assert "github.com" in lowered and "huggingface.co" in lowered assert "private" in lowered @@ -61,11 +55,8 @@ def test_note_does_not_claim_project_sandbox_starts_empty(): def test_note_does_not_claim_local_files_are_inaccessible(): lowered = _SANDBOX_PATHS_NOTE.lower() - # On a locally hosted Studio the child runs on the host with no filesystem - # isolation on this branch (Landlock is a separate change), and cat is an - # auto-safe terminal command, so an exact local path is readable; the note - # must not claim otherwise. It should frame the workdir as the default work - # location instead. + # No filesystem isolation on this branch, so an exact local path is readable; + # the note must frame the workdir as the default work location, not deny access. assert "cannot see the user's own computer" not in lowered assert "default location for your work" in lowered @@ -76,10 +67,9 @@ def test_note_is_appended_to_both_tool_descriptions(): def test_note_scopes_network_block_to_the_terminal_not_all_shell_commands(): - # The terminal leaves the network namespace intact and does not block git/pip, - # so the note must not claim shell network is fully blocked; it names the - # commands that are blocked (curl / wget) and attributes the host allowlist to - # the python tool. + # The terminal keeps the network namespace and doesn't block git/pip, so the + # note names only the blocked commands (curl/wget) and puts the allowlist on the + # python tool rather than claiming shell network is fully blocked. lowered = _SANDBOX_PATHS_NOTE.lower() assert "shell network commands are blocked" not in lowered assert "curl" in lowered and "wget" in lowered @@ -97,11 +87,9 @@ def test_note_distinguishes_attachments_from_sandbox_uploads(): def test_blocked_network_command_message_gates_code_fallback_on_tool_availability(): - # A blocked curl/wget must not tell the model the whole sandbox is offline, and - # must not name a specific tool (e.g. python) as the remedy: when only the - # terminal is enabled the python tool is absent from the schema, so an - # unconditional "fetch it from Python code" instruction invites an invalid tool - # call. The fallback is gated on a code-execution tool being enabled this turn. + # A blocked curl/wget must not claim the sandbox is offline, nor name a specific + # remedy tool (python may be absent from the schema -- an invalid call); the + # fallback is gated on a code-execution tool being enabled this turn. msg = _bash_exec("curl https://raw.githubusercontent.com/foo/bar/main/x.py").lower() assert "blocked command" in msg assert "cannot reach other machines or remote hosts" not in msg @@ -111,10 +99,9 @@ def test_blocked_network_command_message_gates_code_fallback_on_tool_availabilit def test_blocked_network_command_message_scopes_claim_to_the_command(): - # The block is by command name; the terminal keeps its network namespace and - # does not block git/pip, so `git clone http:///repo` can still - # reach a private host. The message must not assert the destination itself is - # unreachable, and must attribute the block to the command by name. + # The block is by command name; the terminal keeps networking (git/pip work), + # so a private host is still reachable. The message must attribute the block to + # the command name, not assert the destination is unreachable. msg = _bash_exec("wget http://10.0.0.5/internal/repo.tar.gz").lower() assert "by name" in msg assert "not reachable" not in msg @@ -123,18 +110,16 @@ def test_blocked_network_command_message_scopes_claim_to_the_command(): def test_blocked_network_command_message_does_not_recommend_chat_upload(): - # Chat attachments land in the RAG store, not the sandbox workdir, so telling - # the user to upload the file to chat is a dead end. The message must instead - # point at an accessible path or placing the file in the working directory. + # Chat attachments land in the RAG store, not the workdir, so "upload to chat" + # is a dead end; point at the working directory or an accessible path instead. msg = _bash_exec("curl https://raw.githubusercontent.com/foo/bar/main/x.py").lower() assert "upload" not in msg assert "working directory" in msg or "path the sandbox can read" in msg def test_bypass_note_drops_the_curl_wget_allowlist_restriction(): - # Under Bypass Permissions _python_exec/_bash_exec skip the safety analysis and - # the curl/wget blocklist, so egress is not limited to the allowlist and those - # downloads work. The bypass note must not tell the model they are blocked. + # Under bypass, _python_exec/_bash_exec skip the safety analysis and curl/wget + # blocklist, so egress works; the bypass note must not call them blocked. lowered = _SANDBOX_PATHS_NOTE_BYPASS.lower() assert "curl" not in lowered and "wget" not in lowered assert "allowlist" not in lowered @@ -142,8 +127,7 @@ def test_bypass_note_drops_the_curl_wget_allowlist_restriction(): # It keeps the workdir-default framing and the "not a copy of the host" guard. assert "default location for your work" in lowered assert "do not assume files elsewhere on the host are already here" in lowered - # The bypass note is a strict prefix+suffix of the default note (only the - # network sentence is removed), so the rest of the guidance is unchanged. + # Bypass note is the default note minus only the network sentence. assert _SANDBOX_PATHS_NOTE_BYPASS != _SANDBOX_PATHS_NOTE assert "curl" in _SANDBOX_PATHS_NOTE.lower() @@ -189,10 +173,9 @@ def test_bypass_code_execution_nudge_drops_the_limited_internet_claim(): def test_code_execution_nudge_does_not_deny_local_file_access(): - # On this no-Landlock branch the child runs on the host with only cwd set, so an - # exact local path the user supplies is readable. The code-execution nudge must - # frame the workdir as the default work location, not assert the user's own - # computer is inaccessible, and it must still allow an exact path. + # No filesystem isolation here, so an exact local path is readable; the nudge + # must frame the workdir as the default work location and still allow an exact + # path, not deny access to the user's computer. from routes.inference import _TOOL_CODE_TIP lowered = _TOOL_CODE_TIP.lower() @@ -202,11 +185,9 @@ def test_code_execution_nudge_does_not_deny_local_file_access(): def test_sandbox_disabled_treats_permission_mode_full_as_unsandboxed(): - # Both agent loops fold permission_mode "full" into bypass_permissions=True and - # pass disable_sandbox=bypass_permissions at execution, so "full" runs python / - # terminal unsandboxed (skips _check_code_safety and the curl/wget blocklist). - # The tool notes and action nudge key off the effective flag so they always - # match what executes, even if the model-layer fold is ever refactored away. + # Both agent loops fold permission_mode "full" into disable_sandbox, so "full" + # runs python/terminal unsandboxed. _sandbox_disabled keys off the effective + # flag so the notes match what executes even if that fold is refactored away. from types import SimpleNamespace from routes.inference import _sandbox_disabled @@ -215,8 +196,7 @@ def test_sandbox_disabled_treats_permission_mode_full_as_unsandboxed(): assert ( _sandbox_disabled(SimpleNamespace(bypass_permissions = True, permission_mode = "ask")) is True ) - # "full" even without bypass set on the object -> still unsandboxed (decoupled - # from the model-layer fold, so the notes never overclaim a live restriction). + # "full" without bypass set -> still unsandboxed (decoupled from the fold). assert ( _sandbox_disabled(SimpleNamespace(bypass_permissions = False, permission_mode = "full")) is True )