studio: tighten comments in the sandbox isolation hint
This commit is contained in:
parent
8d99c172a1
commit
830af5f627
4 changed files with 72 additions and 114 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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://<private-host>/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
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue