From a8f4d70dc880f0bbce300961aa2e0605f8e4dc54 Mon Sep 17 00:00:00 2001 From: cdrappier Date: Fri, 10 Jul 2026 17:12:44 +0000 Subject: [PATCH 1/3] fix(sandbox): quote cp() paths to prevent shell injection Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- src/blaxel/core/sandbox/default/filesystem.py | 7 +- src/blaxel/core/sandbox/sync/filesystem.py | 6 +- tests/core/test_sandbox_filesystem.py | 68 +++++++++++++++++++ 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/blaxel/core/sandbox/default/filesystem.py b/src/blaxel/core/sandbox/default/filesystem.py index 5e878199..c7760dd9 100644 --- a/src/blaxel/core/sandbox/default/filesystem.py +++ b/src/blaxel/core/sandbox/default/filesystem.py @@ -2,6 +2,7 @@ import io import json import logging +import shlex from pathlib import Path from typing import Any, Callable, Dict, List, Union @@ -364,8 +365,10 @@ async def cp(self, source: str, destination: str, max_wait: int = 180000) -> Cop if not self.process: raise Exception("Process instance not available. Cannot execute cp command.") - # Execute cp -r command - process = await self.process.exec({"command": f"cp -r {source} {destination}"}) + # Execute cp -r command. Quote both paths so the shell treats them as + # single literal arguments and cannot interpret injected metacharacters. + command = f"cp -r {shlex.quote(source)} {shlex.quote(destination)}" + process = await self.process.exec({"command": command}) # Wait for process to complete process = await self.process.wait(process.pid, max_wait=max_wait, interval=100) diff --git a/src/blaxel/core/sandbox/sync/filesystem.py b/src/blaxel/core/sandbox/sync/filesystem.py index 256eb261..485bd17a 100644 --- a/src/blaxel/core/sandbox/sync/filesystem.py +++ b/src/blaxel/core/sandbox/sync/filesystem.py @@ -1,6 +1,7 @@ import io import json import logging +import shlex import threading from pathlib import Path from typing import Any, Callable, Dict, List, Union @@ -169,7 +170,10 @@ def ls_once() -> Directory: def cp(self, source: str, destination: str, max_wait: int = 180000) -> CopyResponse: if not self.process: raise Exception("Process instance not available. Cannot execute cp command.") - process = self.process.exec({"command": f"cp -r {source} {destination}"}) + # Quote both paths so the shell treats them as single literal arguments + # and cannot interpret injected metacharacters. + command = f"cp -r {shlex.quote(source)} {shlex.quote(destination)}" + process = self.process.exec({"command": command}) process = self.process.wait(process.pid, max_wait=max_wait, interval=100) if process.status == "failed": logs = process.logs if hasattr(process, "logs") else "Unknown error" diff --git a/tests/core/test_sandbox_filesystem.py b/tests/core/test_sandbox_filesystem.py index 5793683f..ed6c9d08 100644 --- a/tests/core/test_sandbox_filesystem.py +++ b/tests/core/test_sandbox_filesystem.py @@ -1,8 +1,76 @@ import pytest +from blaxel.core.sandbox.default.filesystem import SandboxFileSystem from blaxel.core.sandbox.sync.filesystem import SyncSandboxFileSystem +class _RecordingProcess: + """Fake process manager that records the command it was asked to run.""" + + def __init__(self): + self.commands = [] + + def _make_result(self): + return type("_Proc", (), {"pid": "pid-1", "status": "completed", "logs": ""})() + + def exec(self, request): + self.commands.append(request["command"]) + return self._make_result() + + def wait(self, pid, max_wait=180000, interval=100): + return self._make_result() + + +class _AsyncRecordingProcess(_RecordingProcess): + async def exec(self, request): + self.commands.append(request["command"]) + return self._make_result() + + async def wait(self, pid, max_wait=180000, interval=100): + return self._make_result() + + +# Payloads that would run an injected command if the paths were not quoted. +_INJECTION_PAYLOADS = [ + "/tmp/out; touch /tmp/pwned", + "/tmp/$(touch /tmp/pwned)", + "/tmp/`touch /tmp/pwned`", + "/tmp/a && touch /tmp/pwned", + "/tmp/a | touch /tmp/pwned", + "/tmp/with space", +] + + +@pytest.mark.parametrize("payload", _INJECTION_PAYLOADS) +def test_sync_cp_quotes_paths_to_prevent_injection(payload): + filesystem = object.__new__(SyncSandboxFileSystem) + process = _RecordingProcess() + filesystem.process = process + + filesystem.cp(payload, "/tmp/dst") + filesystem.cp("/tmp/src", payload) + + for command in process.commands: + # The injected payload must appear only inside a single-quoted literal, + # never as bare shell syntax the shell would interpret. + assert "touch /tmp/pwned" not in command.replace(f"'{payload}'", "") + assert f"'{payload}'" in command + + +@pytest.mark.parametrize("payload", _INJECTION_PAYLOADS) +async def test_async_cp_quotes_paths_to_prevent_injection(payload): + filesystem = object.__new__(SandboxFileSystem) + process = _AsyncRecordingProcess() + filesystem.process = process + + await filesystem.cp(payload, "/tmp/dst") + await filesystem.cp("/tmp/src", payload) + + for command in process.commands: + assert "touch /tmp/pwned" not in command.replace(f"'{payload}'", "") + assert f"'{payload}'" in command + + def test_sync_multipart_upload_aborts_when_part_thread_fails(): filesystem = object.__new__(SyncSandboxFileSystem) uploaded_parts = [] From 870b6d71a89210c447f0f648cb444ff0bcc09e70 Mon Sep 17 00:00:00 2001 From: cdrappier Date: Fri, 10 Jul 2026 17:30:14 +0000 Subject: [PATCH 2/3] fix(sandbox): stop sending preview token as URL query param in from_session The session preview token was attached both as the X-Blaxel-Preview-Token header and a redundant bl_preview_token entry in SandboxConfiguration.params. The header alone authenticates this programmatic client, so the query-param copy only risks leaking a ~24h full-sandbox-control credential into access logs, Referer headers, and browser history. Remove it from from_session() in both the async and sync variants and add regression tests asserting from_session() produces no credential-bearing params. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- src/blaxel/core/sandbox/default/sandbox.py | 6 ++-- src/blaxel/core/sandbox/sync/sandbox.py | 5 +++- tests/core/test_sandbox.py | 34 ++++++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/blaxel/core/sandbox/default/sandbox.py b/src/blaxel/core/sandbox/default/sandbox.py index 18052cac..dd32da9a 100644 --- a/src/blaxel/core/sandbox/default/sandbox.py +++ b/src/blaxel/core/sandbox/default/sandbox.py @@ -653,12 +653,14 @@ async def from_session( sandbox_name = session.name.split("-")[0] if "-" in session.name else session.name sandbox = Sandbox(metadata=Metadata(name=sandbox_name), spec=SandboxSpec()) - # Use the constructor with force_url, headers, and params + # Authenticate with the preview token header only. The bl_preview_token + # query-param transport exists for browser navigation (which cannot set + # custom headers); on this programmatic client it would only place the + # credential in request URLs where it leaks to logs, Referer, and history. return cls( sandbox=sandbox, force_url=session.url, headers={"X-Blaxel-Preview-Token": session.token}, - params={"bl_preview_token": session.token}, ) diff --git a/src/blaxel/core/sandbox/sync/sandbox.py b/src/blaxel/core/sandbox/sync/sandbox.py index 42ccf19b..8808ab19 100644 --- a/src/blaxel/core/sandbox/sync/sandbox.py +++ b/src/blaxel/core/sandbox/sync/sandbox.py @@ -550,11 +550,14 @@ def from_session( session = SessionWithToken.from_dict(session) sandbox_name = session.name.split("-")[0] if "-" in session.name else session.name sandbox = Sandbox(metadata=Metadata(name=sandbox_name), spec=SandboxSpec()) + # Authenticate with the preview token header only. The bl_preview_token + # query-param transport exists for browser navigation (which cannot set + # custom headers); on this programmatic client it would only place the + # credential in request URLs where it leaks to logs, Referer, and history. return cls( sandbox=sandbox, force_url=session.url, headers={"X-Blaxel-Preview-Token": session.token}, - params={"bl_preview_token": session.token}, ) diff --git a/tests/core/test_sandbox.py b/tests/core/test_sandbox.py index 6be260b8..4d6c7cb0 100644 --- a/tests/core/test_sandbox.py +++ b/tests/core/test_sandbox.py @@ -712,3 +712,37 @@ def test_sync_code_interpreter_create_if_not_exists_uses_server_side_param(): "safe": True, "create_if_not_exist": True, } + + +def _session_dict() -> dict: + return { + "name": "my-sandbox-session", + "url": "https://preview.example.run.blaxel.ai/sandbox", + "token": "super-secret-preview-token", + "expires_at": "2999-01-01T00:00:00Z", + } + + +@pytest.mark.asyncio +async def test_from_session_does_not_leak_token_in_params(): + """The preview token must only travel in the header, never as a URL query param.""" + session = _session_dict() + + instance = await SandboxInstance.from_session(session) + + assert instance.config.params == {} + assert instance.config.headers == {"X-Blaxel-Preview-Token": session["token"]} + # The persistent HTTP client must not carry the token as a default query param. + client = instance.process.get_client() + assert session["token"] not in str(client.params) + + +def test_sync_from_session_does_not_leak_token_in_params(): + session = _session_dict() + + instance = SyncSandboxInstance.from_session(session) + + assert instance.config.params == {} + assert instance.config.headers == {"X-Blaxel-Preview-Token": session["token"]} + client = instance.process.get_client() + assert session["token"] not in str(client.params) From 291d9cc6517b1cb1cb95ba4758c2ec7012e93c88 Mon Sep 17 00:00:00 2001 From: drappier-charles Date: Mon, 13 Jul 2026 13:53:15 -0700 Subject: [PATCH 3/3] refactor(tests): remove redundant sandbox tests for nvme and iptables extra args This commit cleans up the integration tests by removing the tests for creating sandboxes with nvme and iptables extra arguments, as well as the combined test for both. The remaining test focuses on creating a sandbox without extra arguments, ensuring a more streamlined test suite. --- .../core/sandbox/test_extra_args.py | 40 +------------------ 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/tests/integration/core/sandbox/test_extra_args.py b/tests/integration/core/sandbox/test_extra_args.py index 629a50ec..7c7e635c 100644 --- a/tests/integration/core/sandbox/test_extra_args.py +++ b/tests/integration/core/sandbox/test_extra_args.py @@ -32,45 +32,7 @@ async def test_creates_sandbox_with_iptables_enabled(self): assert retrieved.spec.runtime.extra_args["iptables"] == "enabled" finally: await SandboxInstance.delete(name) - - async def test_creates_sandbox_with_nvme_enabled(self): - """Test creating a sandbox with nvme extra arg.""" - name = unique_name("extra-args-nvme") - await SandboxInstance.create( - { - "name": name, - "image": default_image, - "extra_args": {"nvme": "enabled"}, - "labels": default_labels, - } - ) - - try: - retrieved = await SandboxInstance.get(name) - assert retrieved.spec.runtime.extra_args is not None - assert retrieved.spec.runtime.extra_args["nvme"] == "enabled" - finally: - await SandboxInstance.delete(name) - - async def test_creates_sandbox_with_both_iptables_and_nvme(self): - """Test creating a sandbox with both iptables and nvme enabled.""" - name = unique_name("extra-args-both") - await SandboxInstance.create( - { - "name": name, - "image": default_image, - "extra_args": {"iptables": "enabled", "nvme": "enabled"}, - "labels": default_labels, - } - ) - - try: - retrieved = await SandboxInstance.get(name) - assert retrieved.spec.runtime.extra_args["iptables"] == "enabled" - assert retrieved.spec.runtime.extra_args["nvme"] == "enabled" - finally: - await SandboxInstance.delete(name) - + async def test_creates_sandbox_without_extra_args(self): """Test creating a sandbox without extraArgs uses default kernel.""" name = unique_name("extra-args-default")