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/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/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/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) 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 = [] 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")