diff --git a/README.md b/README.md index 125c29909..105fcbb34 100644 --- a/README.md +++ b/README.md @@ -1203,7 +1203,7 @@ an explicit **Load full file** action before fetching the remainder or mounting the full code editor. Active HTML, XHTML, and SVG artifacts remain forced downloads at the Gateway boundary. -With `AioSandboxProvider`, shell execution runs inside isolated containers. With `LocalSandboxProvider`, file tools still map to per-thread directories on the host, but host `bash` is disabled by default because it is not a secure isolation boundary. Re-enable host bash only for fully trusted local workflows. Host bash commands have a wall-clock timeout, and long-lived processes should be started in the background with output redirected to a workspace log. +With `AioSandboxProvider`, shell execution runs inside isolated containers. With `LocalSandboxProvider`, file tools still map to per-thread directories on the host, but host `bash` is disabled by default because it is not a secure isolation boundary. Re-enable host bash only for fully trusted local workflows. Host bash commands have a wall-clock timeout, and long-lived processes should be started in the background with output redirected to a workspace log. On Windows, Git Bash/MSYS argument-conversion exclusions are limited to safe non-root virtual path prefixes, so host-native CLI launchers retain their normal MSYS compatibility. `AioSandboxProvider` normally detects thread-data mounts from its backend: local containers use the mounted gateway directories, while remote/provisioner diff --git a/backend/packages/harness/deerflow/sandbox/AGENTS.md b/backend/packages/harness/deerflow/sandbox/AGENTS.md index 25d1b9497..98eca0775 100644 --- a/backend/packages/harness/deerflow/sandbox/AGENTS.md +++ b/backend/packages/harness/deerflow/sandbox/AGENTS.md @@ -5,7 +5,7 @@ **Authorization gate** (`sandbox:execute`, RFC #4063 Phase 3): every sandbox acquisition passes through `authorize_sandbox_execution` (`deerflow/authz/sandbox_authz.py`) - a binary `authorize(principal, "sandbox", "execute", target="*")` check before `provider.acquire`. The gate lives at the single acquisition entry point (`ensure_sandbox_initialized` / `_acquire_sandbox_async` in `tools.py`, and `SandboxMiddleware.before_agent` / `abefore_agent`), so it cannot be bypassed regardless of which sandbox-dependent tool triggers it; the reuse path (sandbox already in state) skips the re-check. Deny raises `SandboxAuthorizationError` (`sandbox/exceptions.py`), which propagates out of tool execution as a friendly error `ToolMessage` ("sandbox execution is not permitted for your role") - the eager path catches it and skips acquisition instead, deferring the deny to the first sandbox-touching tool call so both paths share the same semantics. Provider errors (both `authorize()` and provider resolution) follow `authorization.fail_closed` / `fail_open`; no readable `config.yaml` or `authorization.enabled: false` makes the gate a no-op (`safe_app_config` tolerates missing config). Gateway auxiliary sync paths (uploads/artifacts routers) call `try_acquire_sandbox_for_request` (`app/gateway/authz.py`), which gates via `authorize_sandbox_for_request` and skips the sync on deny - the upload/artifact edit itself still succeeds. Tests: `tests/test_sandbox_authorization.py`. **Environment policy** (`sandbox/env_policy.py`): `execute_command` no longer inherits the full `os.environ`. `build_sandbox_env()` scrubs secret-looking names (`*KEY*`/`*SECRET*`/`*TOKEN*`/`*PASS*`/`*CREDENTIAL*`) from the inherited environment before layering injected request secrets on top, so platform credentials (e.g. `OPENAI_API_KEY`) never leak into skill subprocesses. Benign vars (`PATH`, `HOME`, `LANG`, `VIRTUAL_ENV`, ...) are preserved. **Implementations**: -- `LocalSandboxProvider` - Local filesystem execution. `acquire(thread_id)` returns a per-thread `LocalSandbox` (id `local:{thread_id}`) whose `path_mappings` resolve `/mnt/user-data/{workspace,uploads,outputs}` and `/mnt/acp-workspace` to that thread's host directories, so the public `Sandbox` API honours the `/mnt/user-data` contract uniformly with AIO. `acquire()` / `acquire(None)` keeps the legacy generic singleton (id `local`) for callers without a thread context. Per-thread sandboxes are held in an LRU cache (default 256 entries) guarded by a `threading.Lock`. Public, custom, legacy, and managed integration skill mappings point at stable enabled-only projection roots rather than raw skill directories. +- `LocalSandboxProvider` - Local filesystem execution. `acquire(thread_id)` returns a per-thread `LocalSandbox` (id `local:{thread_id}`) whose `path_mappings` resolve `/mnt/user-data/{workspace,uploads,outputs}` and `/mnt/acp-workspace` to that thread's host directories, so the public `Sandbox` API honours the `/mnt/user-data` contract uniformly with AIO. `acquire()` / `acquire(None)` keeps the legacy generic singleton (id `local`) for callers without a thread context. Per-thread sandboxes are held in an LRU cache (default 256 entries) guarded by a `threading.Lock`. Public, custom, legacy, and managed integration skill mappings point at stable enabled-only projection roots rather than raw skill directories. On Windows, Git Bash/MSYS argument-conversion exclusions are limited to safe non-root virtual path prefixes; do not restore a blanket conversion disable, because host-native CLI launchers need normal MSYS path conversion for their own installation paths. - `AioSandboxProvider` (`packages/harness/deerflow/community/`) - Docker-based isolation. Active-cache and warm-pool entries are checked with the backend during acquire/reuse; definitively dead containers are dropped from all in-process maps so the thread can discover or create a fresh sandbox instead of reusing a stale client. Backend health-check failures are treated as unknown, not dead; local discovery likewise treats an unverifiable container as not adoptable and falls through to create rather than failing acquire. `get()` remains an in-memory lookup for event-loop-safe tool paths — it never touches the ownership store (that would be blocking IO on the event loop); ownership is published on acquire/reclaim and refreshed off the event loop by the dedicated renewal thread (`_renew_owned_leases`). `uses_thread_data_mounts` defaults to backend detection (`LocalContainerBackend=True`, remote/provisioner backends=False), while the optional `sandbox.thread_data_mounts` boolean takes precedence for deployments that guarantee the Gateway and sandbox share the same thread user-data directories. Setting it `true` skips upload-time sandbox acquire/sync; a false positive leaves uploads unavailable to the sandbox. Local-container and hostPath-provisioner mounts use the same stable skill projection roots; PVC-backed skills remain governed by the operator-supplied PVC layout until PVC materialization is implemented. Readiness probes and `agent_sandbox` clients classify loopback/private IPs, single-label cluster hosts, and Docker/Podman internal hostnames as direct control-plane destinations and set `trust_env=False`; external FQDNs and public IPs retain environment proxy support. - `E2BSandboxProvider` (`packages/harness/deerflow/community/e2b_sandbox/`) provides E2B remote isolation. New sandboxes receive a one-shot upload from the enabled-only public, custom, diff --git a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py b/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py index 9f70e8a3c..e9c4836ca 100644 --- a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py +++ b/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py @@ -102,6 +102,25 @@ class LocalSandbox(Sandbox): shell_name = LocalSandbox._shell_name(shell) return shell_name in {"sh.exe", "bash.exe"} and any(part in normalized for part in ("/git/", "/mingw", "/msys")) + def _msys_path_conversion_exclusions(self) -> str: + """Return the MSYS argument prefixes owned by this sandbox. + + The blanket conversion disable introduced for #2765 also affects child + processes launched by Git Bash, including Windows-native CLI shims that + need normal MSYS path conversion for their own installation paths. + Excluding only the configured virtual roots preserves DeerFlow path + arguments without changing unrelated child-process behavior. Root and + values containing MSYS exclusion syntax are omitted because they would + broaden the exclusion beyond one virtual path prefix. + """ + safe_roots: dict[str, None] = {} + for mapping in self.path_mappings: + root = mapping.container_path.rstrip("/") + if not root or not root.startswith("/") or ";" in root or "*" in root: + continue + safe_roots[root] = None + return ";".join(safe_roots) + @staticmethod def _find_first_available_shell(candidates: tuple[str, ...]) -> str | None: """Return the first executable shell path or command found from candidates.""" @@ -496,11 +515,12 @@ class LocalSandbox(Sandbox): else: args = [shell, "-c", resolved_command] if self._is_msys_shell(shell): - sandbox_env = { - **sandbox_env, - "MSYS_NO_PATHCONV": "1", - "MSYS2_ARG_CONV_EXCL": "*", - } + exclusions = self._msys_path_conversion_exclusions() + if exclusions: + sandbox_env = { + **sandbox_env, + "MSYS2_ARG_CONV_EXCL": exclusions, + } try: result = subprocess.run( diff --git a/backend/tests/test_local_sandbox_encoding.py b/backend/tests/test_local_sandbox_encoding.py index 0524667cb..368eb3898 100644 --- a/backend/tests/test_local_sandbox_encoding.py +++ b/backend/tests/test_local_sandbox_encoding.py @@ -2,7 +2,7 @@ import builtins from types import SimpleNamespace import deerflow.sandbox.local.local_sandbox as local_sandbox -from deerflow.sandbox.local.local_sandbox import LocalSandbox +from deerflow.sandbox.local.local_sandbox import LocalSandbox, PathMapping def _open(base, file, mode="r", *args, **kwargs): @@ -115,7 +115,7 @@ def test_execute_command_uses_powershell_command_mode_on_windows(monkeypatch): ] -def test_execute_command_uses_posix_shell_command_mode_on_windows(monkeypatch): +def test_execute_command_keeps_msys_path_conversion_for_host_commands_on_windows(monkeypatch): calls: list[tuple[object, dict]] = [] def fake_run(*args, **kwargs): @@ -125,6 +125,7 @@ def test_execute_command_uses_posix_shell_command_mode_on_windows(monkeypatch): monkeypatch.setattr(local_sandbox.os, "name", "nt") monkeypatch.setattr(local_sandbox.os, "environ", {"PATH": r"C:\Program Files\Git\bin"}) monkeypatch.setattr(LocalSandbox, "_get_shell", staticmethod(lambda: r"C:\Program Files\Git\bin\sh.exe")) + monkeypatch.setattr(LocalSandbox, "_msys_path_conversion_exclusions", lambda self: "/mnt/user-data") monkeypatch.setattr(local_sandbox.subprocess, "run", fake_run) output = LocalSandbox("t").execute_command("echo hello") @@ -140,14 +141,68 @@ def test_execute_command_uses_posix_shell_command_mode_on_windows(monkeypatch): "timeout": 600, "env": { "PATH": r"C:\Program Files\Git\bin", - "MSYS_NO_PATHCONV": "1", - "MSYS2_ARG_CONV_EXCL": "*", + "MSYS2_ARG_CONV_EXCL": "/mnt/user-data", }, }, ) ] +def test_execute_command_scopes_msys_path_conversion_exclusions_on_windows(monkeypatch): + calls: list[tuple[object, dict]] = [] + + def fake_run(*args, **kwargs): + calls.append((args[0], kwargs)) + return SimpleNamespace(stdout="ok", stderr="", returncode=0) + + monkeypatch.setattr(local_sandbox.os, "name", "nt") + monkeypatch.setattr(local_sandbox.os, "environ", {"PATH": r"C:\Program Files\Git\bin"}) + monkeypatch.setattr(LocalSandbox, "_get_shell", staticmethod(lambda: r"C:\Program Files\Git\bin\sh.exe")) + monkeypatch.setattr(LocalSandbox, "_msys_path_conversion_exclusions", lambda self: "/mnt/user-data") + monkeypatch.setattr(local_sandbox.subprocess, "run", fake_run) + + output = LocalSandbox("t").execute_command("cat /mnt/user-data/workspace/input.txt") + + assert output == "ok" + assert calls[0][1]["env"] == { + "PATH": r"C:\Program Files\Git\bin", + "MSYS2_ARG_CONV_EXCL": "/mnt/user-data", + } + + +def test_execute_command_ignores_root_msys_mapping_for_host_commands_on_windows(monkeypatch): + calls: list[tuple[object, dict]] = [] + + def fake_run(*args, **kwargs): + calls.append((args[0], kwargs)) + return SimpleNamespace(stdout="ok", stderr="", returncode=0) + + monkeypatch.setattr(local_sandbox.os, "name", "nt") + monkeypatch.setattr(local_sandbox.os, "environ", {"PATH": r"C:\Program Files\Git\bin"}) + monkeypatch.setattr(LocalSandbox, "_get_shell", staticmethod(lambda: r"C:\Program Files\Git\bin\sh.exe")) + monkeypatch.setattr(LocalSandbox, "_msys_path_conversion_exclusions", lambda self: "") + monkeypatch.setattr(local_sandbox.subprocess, "run", fake_run) + + output = LocalSandbox("t").execute_command("echo hello") + + assert output == "ok" + assert calls[0][1]["env"] == {"PATH": r"C:\Program Files\Git\bin"} + + +def test_msys_path_conversion_exclusions_omit_blanket_patterns(): + sandbox = LocalSandbox( + "t", + [ + PathMapping(container_path="/", local_path="C:\\"), + PathMapping(container_path="/mnt/data;*", local_path=r"C:\data"), + PathMapping(container_path="/mnt/user-data/", local_path=r"C:\user-data"), + PathMapping(container_path="/mnt/user-data", local_path=r"C:\user-data"), + ], + ) + + assert sandbox._msys_path_conversion_exclusions() == "/mnt/user-data" + + def test_execute_command_does_not_set_msys_env_for_non_msys_posix_shell_on_windows(monkeypatch): calls: list[tuple[object, dict]] = []