From fa4693685ff8eb5d361ddfb3ea5f7e41a7d3a4e2 Mon Sep 17 00:00:00 2001 From: Michael Han Date: Tue, 21 Jul 2026 23:26:27 -0700 Subject: [PATCH] Studio: close third-round sandbox guard review gaps Runtime guard (sitecustomize): - Keep the network guard active when a sandbox child deletes UNSLOTH_STUDIO_SANDBOXED but still loads this shim from the sandbox_site dir on PYTHONPATH. Bypass runs under bypass_site (guard short-circuits on __name__), so an absent flag with sandbox_site loaded is tampering, not bypass. Regression test spawns a real deleted-flag child. Terminal startup-guard (hard block in _bash_exec): - A here-doc piped into a consumer (cat <<'PY' | python) keeps the post-delimiter pipeline so the body is scanned as that python's stdin program. - Process substitution: recurse into <(...)/>(...) inner commands, and fail closed when python reads its program from one (python <(printf ...)). render_html network gate (auto-approve path): - Module re-exports (export * from 'https://...', export {a} from '/mod.js') are gated like static imports; relative specifiers stay static. - A reassigned computed-key alias (var k='src'; img[k]=URL; var k='title') is position-dependent, so it is dropped from the flat alias map and fails closed on a network-looking assigned value. Adds blocked + safe regression cases for each. Not addressed: the spoofed-trusted-httpx-frame P1 (exec(compile(payload, httpx.__file__,'exec'), httpx.__dict__)). Same same-interpreter forgeability class already flagged for a below-the-Python-layer redesign; a frame check keyed on caller-suppliable co_filename cannot close it robustly. --- .../inference/sandbox_site/sitecustomize.py | 15 +++--- studio/backend/core/inference/tools.py | 48 +++++++++++++++++-- .../backend/tests/test_bypass_permissions.py | 10 ++++ studio/backend/tests/test_permission_mode.py | 9 ++++ studio/backend/tests/test_sandbox_tools.py | 27 +++++++++++ 5 files changed, 98 insertions(+), 11 deletions(-) diff --git a/studio/backend/core/inference/sandbox_site/sitecustomize.py b/studio/backend/core/inference/sandbox_site/sitecustomize.py index d098b75184..511e4e8cbf 100644 --- a/studio/backend/core/inference/sandbox_site/sitecustomize.py +++ b/studio/backend/core/inference/sandbox_site/sitecustomize.py @@ -592,17 +592,18 @@ def _sandbox_guard_should_activate(): """Decide whether to install the runtime network guard. Normal sandbox children set ``UNSLOTH_STUDIO_SANDBOXED=1``. Bypass (full - access) removes the variable entirely, so an absent flag means "do not - guard". A flag that is PRESENT but not ``"1"`` (e.g. sandbox code running - ``os.environ['UNSLOTH_STUDIO_SANDBOXED']='0'`` before spawning a child to - escape the guard) is tampering: keep the guard on as long as this shim was - still loaded from the sandbox site dir the launcher put on PYTHONPATH. + access) runs under ``bypass_site`` (which never activates this guard because + it is executed via ``runpy`` with ``__name__ != "sitecustomize"``) and never + puts this ``sandbox_site`` directory on the child's PYTHONPATH. So whenever + this shim actually loads *as* ``sitecustomize`` from the sandbox site dir, + the child is running under the sandbox launcher and must be guarded — + regardless of whether the flag is ``"1"``, altered (e.g. sandbox code running + ``os.environ['UNSLOTH_STUDIO_SANDBOXED']='0'``), or deleted outright + (``del os.environ['UNSLOTH_STUDIO_SANDBOXED']``) before spawning a child. """ flag = os.environ.get("UNSLOTH_STUDIO_SANDBOXED") if flag == "1": return True - if flag is None: - return False return _loaded_from_sandbox_site() diff --git a/studio/backend/core/inference/tools.py b/studio/backend/core/inference/tools.py index 90432a25d4..ee695b9165 100644 --- a/studio/backend/core/inference/tools.py +++ b/studio/backend/core/inference/tools.py @@ -558,7 +558,13 @@ def _shell_command_substitutions(command: str) -> list[str]: if command.startswith("$((", index): index += 3 continue - if not command.startswith("$(", index): + # $(...) command substitution and <(...)/>(...) process substitution all + # run their inner command; recurse into each. + if not ( + command.startswith("$(", index) + or command.startswith("<(", index) + or command.startswith(">(", index) + ): index += 1 continue start = index + 2 @@ -602,6 +608,13 @@ def _shell_command_substitutions(command: str) -> list[str]: _HEREDOC_START_RE = re.compile(r"<<-?\s*(?P['\"]?)(?P[A-Za-z_][A-Za-z0-9_.-]*)") +# A Python interpreter reading its program from a process substitution +# (python <(printf ...)) runs a generated script this static scan cannot see; +# the inner generator's output is the program, so fail closed on this shape. +_PYTHON_PROCESS_SUB_SCRIPT_RE = re.compile( + r"(?:^|[\s;&|(])(?:[\w./\\-]*/)?python(?:w)?[0-9.]*(?:\.exe)?(?:\s+-[^\s]*)*\s+<\(", + re.IGNORECASE, +) def _shell_here_doc_entries(command: str) -> tuple[list[tuple[str, str]], bool]: @@ -633,7 +646,16 @@ def _shell_here_doc_entries(command: str) -> tuple[list[tuple[str, str]], bool]: if body_end >= len(lines): malformed = True continue - entries.append((line[: match.start()], "\n".join(lines[body_start:body_end]))) + # Keep the command text after the delimiter so a here-doc piped into + # another process (cat <<'PY' | python) still exposes that consumer: + # the body is the consumer's stdin program. Only reconstruct the tail + # for a single here-doc on the line to avoid mis-pairing multi-doc + # openers (cmd < 4: return True + if _PYTHON_PROCESS_SUB_SCRIPT_RE.search(command): + return True here_doc_entries, malformed_here_doc = _shell_here_doc_entries(command) if malformed_here_doc: return True @@ -3581,6 +3605,9 @@ _RENDER_HTML_NETWORK_RE = re.compile( # (a relative './mod.js' specifier has no https:/root prefix and stays static). r"\bimport\s*\(\s*[\"'`]?\s*(?:https?:|/)|" r"\bimport\b[^;\n]*?[\"'`]\s*(?:https?:|/)|" + # Module re-exports (export * from '...' / export {a} from '/...') fetch the + # referenced module just like a static import. + r"\bexport\b[^;\n]*?\bfrom\s*[\"'`]\s*(?:https?:|/)|" r"]*\bsrc\s*=|" # Self-navigation sinks: location.assign/replace(...), window.open(...), and # assigning a URL to (window.)location(.href). location.reload()/history.back @@ -3850,10 +3877,23 @@ def _static_js_assignment_string(expression: str) -> str | None: def _render_html_static_js_name_aliases(code: str) -> dict[str, str]: aliases: dict[str, str] = {} + ambiguous: set[str] = set() for match in _RENDER_HTML_JS_STATIC_NAME_START_RE.finditer(code): value = _static_js_assignment_string(code[match.end() :]) - if value is not None: - aliases[match.group("name")] = value + if value is None: + continue + name = match.group("name") + if name in ambiguous: + continue + # A name reassigned to a different literal is position-dependent; this + # flat map cannot say which value is live at a given use, so drop it and + # let the caller fail closed on a network-looking assigned value rather + # than trusting only the final definition. + if name in aliases and aliases[name] != value: + del aliases[name] + ambiguous.add(name) + continue + aliases[name] = value return aliases diff --git a/studio/backend/tests/test_bypass_permissions.py b/studio/backend/tests/test_bypass_permissions.py index 5094b3de05..47f45a3e6e 100644 --- a/studio/backend/tests/test_bypass_permissions.py +++ b/studio/backend/tests/test_bypass_permissions.py @@ -247,6 +247,12 @@ def test_bash_blocklist_enforced_when_sandboxed(captured_popen): # declare -x / typeset -x export an emptied PYTHONPATH to the child. "declare -x PYTHONPATH=; python -c 'import boto3'", "typeset -x PYTHONPATH=; python -c 'import boto3'", + # A here-doc piped into python feeds the body to that python as stdin. + "cat <<'PY' | python\nimport subprocess\nsubprocess.run(['python','-S','-c','import boto3'])\nPY", + # Process substitution: the inner command is a python bypass, and a + # generated-script form feeds python an unscannable program. + "diff <(python -S -c 'import boto3') /dev/null", + 'python <(printf %s "import subprocess; subprocess.run([\'python\',\'-S\',\'-c\',\'import boto3\'])")', ], ) def test_bash_blocks_python_startup_guard_bypasses(captured_popen, command): @@ -285,6 +291,10 @@ def test_bash_blocks_python_startup_guard_bypasses(captured_popen, command): 'alias py="python"; py script.py', # env -S with a plain launch (no skip flag / env mutation). 'env -S "python -c print(1)"', + # Process substitution feeding a non-Python consumer stays static. + "diff <(sort a.txt) <(sort b.txt)", + # A here-doc piped to python whose body is a benign program. + "cat <<'PY' | python\nprint(1)\nPY", ], ) def test_bash_allows_python_without_startup_guard_bypass(captured_popen, command): diff --git a/studio/backend/tests/test_permission_mode.py b/studio/backend/tests/test_permission_mode.py index 9e23780c92..c91d1e479a 100644 --- a/studio/backend/tests/test_permission_mode.py +++ b/studio/backend/tests/test_permission_mode.py @@ -1251,6 +1251,15 @@ def test_render_html_gated_only_when_networked(): # An entity-obfuscated CSS URL is a network load after the browser decodes it. assert rh('
') is True assert rh('
& local
') is False + # Module re-exports of a remote/root URL fetch that module; relative stays static. + assert rh("") is True + assert rh("") is True + assert rh("") is False + assert rh("") is False + # A reassigned computed-key alias is position-dependent, so it fails closed on + # a network value but a same-valued redefinition stays resolvable/static. + assert rh("") is True + assert rh("") is False assert ( rh( "