diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py index e8ef98f16..494f8bdaf 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py @@ -1,7 +1,6 @@ import base64 import errno import logging -import shlex import threading import uuid from dataclasses import dataclass, field @@ -11,6 +10,7 @@ from agent_sandbox import Sandbox as AioSandboxClient from agent_sandbox.core.api_error import ApiError from deerflow.config.paths import VIRTUAL_PATH_PREFIX +from deerflow.sandbox.remote_list_dir import parse_remote_list_dir_output, remote_list_dir_command from deerflow.sandbox.sandbox import Sandbox, _validate_extra_env from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path, truncate_line @@ -573,22 +573,23 @@ class AioSandbox(Sandbox): Returns: The contents of the directory. """ + resolved = path with self._lock: try: - result = self._client.shell.exec_command(command=f"find {shlex.quote(path)} -maxdepth {max_depth} -type f -o -type d 2>/dev/null | head -500", no_change_timeout=self._DEFAULT_NO_CHANGE_TIMEOUT) - output = result.data.output if result.data else "" - if output: - # find delimits records with "\n" and nothing else, so split - # on that alone: splitlines() would also break on \v, \f, - # \x1c-\x1e and \x85, all of which are legal inside a Linux - # filename. Do NOT strip entries either — a filename that - # legitimately ends in whitespace would be corrupted and - # never resolve again. - return [line for line in output.split("\n") if line] - return [] + result = self._client.shell.exec_command( + command=remote_list_dir_command(resolved, max_depth), + no_change_timeout=self._DEFAULT_NO_CHANGE_TIMEOUT, + ) except Exception as e: logger.error(f"Failed to list directory in sandbox: {e}") - return [] + raise OSError(f"Failed to list directory '{resolved}' in sandbox: {e}") from e + if result.data is None: + raise OSError(f"Failed to list directory '{resolved}' in sandbox: empty response") + return parse_remote_list_dir_output( + result.data.output or "", + resolved, + pipeline_exit_code=getattr(result.data, "exit_code", None), + ) def write_file(self, path: str, content: str, append: bool = False) -> None: """Write content to a file in the sandbox. diff --git a/backend/packages/harness/deerflow/community/boxlite/box.py b/backend/packages/harness/deerflow/community/boxlite/box.py index c3d21bf2f..3fc42fd45 100644 --- a/backend/packages/harness/deerflow/community/boxlite/box.py +++ b/backend/packages/harness/deerflow/community/boxlite/box.py @@ -25,6 +25,7 @@ import threading from typing import TYPE_CHECKING, TypeVar from deerflow.config.paths import VIRTUAL_PATH_PREFIX +from deerflow.sandbox.remote_list_dir import parse_remote_list_dir_output, remote_list_dir_command from deerflow.sandbox.sandbox import Sandbox, _validate_extra_env from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path, truncate_line @@ -290,10 +291,8 @@ class BoxliteBox(Sandbox): def list_dir(self, path: str, max_depth: int = 2) -> list[str]: resolved = self._resolve_path(path) - r = self._sh(f"find {shlex.quote(resolved)} -maxdepth {int(max_depth)} \\( -type f -o -type d \\) 2>/dev/null | head -500") - # splitlines() already removed the terminators; do NOT strip entries — - # a filename that legitimately ends in whitespace would be corrupted. - return [line for line in (r.stdout or "").splitlines() if line] + r = self._sh(remote_list_dir_command(resolved, max_depth)) + return parse_remote_list_dir_output(r.stdout, resolved, pipeline_exit_code=r.exit_code) def glob( self, diff --git a/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py b/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py index e8bee0613..4208e9325 100644 --- a/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py +++ b/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py @@ -11,6 +11,7 @@ from e2b import FileNotFoundException from e2b_code_interpreter import Sandbox as E2BClientSandbox from deerflow.config.paths import VIRTUAL_PATH_PREFIX +from deerflow.sandbox.remote_list_dir import parse_remote_list_dir_output, remote_list_dir_command from deerflow.sandbox.sandbox import Sandbox, _validate_extra_env from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path, truncate_line @@ -340,17 +341,17 @@ class E2BSandbox(Sandbox): with self._lock: client = self._client if client is None: - return [] + raise RuntimeError("sandbox client has been closed") try: - result = client.commands.run(f"find {shlex.quote(resolved)} -maxdepth {int(max_depth)} \\( -type f -o -type d \\) 2>/dev/null | head -500") - output = getattr(result, "stdout", "") or "" - # splitlines() already removed the terminators; do NOT strip - # entries — a filename that legitimately ends in whitespace - # would be corrupted and never resolve again. - return [line for line in output.splitlines() if line] + result = client.commands.run(remote_list_dir_command(resolved, max_depth)) except Exception as e: logger.error("Failed to list_dir %s in e2b sandbox: %s", resolved, e) - return [] + raise OSError(f"Failed to list_dir {resolved} in e2b sandbox: {e}") from e + return parse_remote_list_dir_output( + getattr(result, "stdout", "") or "", + resolved, + pipeline_exit_code=getattr(result, "exit_code", None), + ) def write_file(self, path: str, content: str, append: bool = False) -> None: resolved = self._resolve_path(path) diff --git a/backend/packages/harness/deerflow/community/opensandbox/sandbox.py b/backend/packages/harness/deerflow/community/opensandbox/sandbox.py index 5f7a20c28..8b5d3b668 100644 --- a/backend/packages/harness/deerflow/community/opensandbox/sandbox.py +++ b/backend/packages/harness/deerflow/community/opensandbox/sandbox.py @@ -12,6 +12,7 @@ from datetime import timedelta from typing import TYPE_CHECKING, Any from deerflow.config.paths import VIRTUAL_PATH_PREFIX +from deerflow.sandbox.remote_list_dir import parse_remote_list_dir_output, remote_list_dir_command from deerflow.sandbox.sandbox import Sandbox, _validate_extra_env from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path, truncate_line @@ -324,10 +325,16 @@ class OpenSandboxSandbox(Sandbox): if depth < 0: raise ValueError("max_depth must be non-negative") resolved = self._resolve_path(path) - execution = self._run(f"find {shlex.quote(resolved)} -maxdepth {depth} \\( -type f -o -type d \\) 2>/dev/null | head -500") - # splitlines() already removed the terminators; do NOT strip entries — - # a filename that legitimately ends in whitespace would be corrupted. - return [line for line in execution_stdout(execution).splitlines() if line] + execution = self._run(remote_list_dir_command(resolved, depth)) + error = getattr(execution, "error", None) + if error is not None: + detail = f"{getattr(error, 'name', type(error).__name__)}: {getattr(error, 'value', error)}" + raise OSError(f"Failed to list_dir {resolved}: {detail}") + return parse_remote_list_dir_output( + execution_stdout(execution), + resolved, + pipeline_exit_code=getattr(execution, "exit_code", None), + ) def glob(self, path: str, pattern: str, *, include_dirs: bool = False, max_results: int = 200) -> tuple[list[str], bool]: if max_results <= 0: diff --git a/backend/packages/harness/deerflow/community/tenki/sandbox.py b/backend/packages/harness/deerflow/community/tenki/sandbox.py index d120a52f2..30668798e 100644 --- a/backend/packages/harness/deerflow/community/tenki/sandbox.py +++ b/backend/packages/harness/deerflow/community/tenki/sandbox.py @@ -29,6 +29,7 @@ import threading from typing import TYPE_CHECKING, Any, TypeVar from deerflow.config.paths import VIRTUAL_PATH_PREFIX +from deerflow.sandbox.remote_list_dir import parse_remote_list_dir_output, remote_list_dir_command from deerflow.sandbox.sandbox import Sandbox, _validate_extra_env from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path, truncate_line @@ -368,10 +369,13 @@ class TenkiSandbox(Sandbox): def list_dir(self, path: str, max_depth: int = 2) -> list[str]: resolved = self._resolve_path(path) - r = self._sh(f"find {shlex.quote(resolved)} -maxdepth {int(max_depth)} \\( -type f -o -type d \\) 2>/dev/null | head -500") - # splitlines() already removed the terminators; do NOT strip entries — - # a filename that legitimately ends in whitespace would be corrupted. - return [self._virtual_path(line) for line in (r.stdout_text or "").splitlines() if line] + r = self._sh(remote_list_dir_command(resolved, max_depth)) + entries = parse_remote_list_dir_output( + r.stdout_text or "", + resolved, + pipeline_exit_code=getattr(r, "exit_code", None), + ) + return [self._virtual_path(line) for line in entries] def glob( self, diff --git a/backend/packages/harness/deerflow/sandbox/AGENTS.md b/backend/packages/harness/deerflow/sandbox/AGENTS.md index 9fb5a1019..02082defb 100644 --- a/backend/packages/harness/deerflow/sandbox/AGENTS.md +++ b/backend/packages/harness/deerflow/sandbox/AGENTS.md @@ -1,8 +1,8 @@ ### Sandbox System (`packages/harness/deerflow/sandbox/`) -**Interface**: Abstract `Sandbox` exposes `execute_command(command, env=None)`, additive `execute_command_in_scope(..., scope_id=...)` / `release_command_scope(scope_id)` hooks, `read_file`, `write_file`, `list_dir`, `glob`, and `grep`. Providers without server-side shell sessions use pass-through scoped hooks, preserving third-party subclasses. `grep` accepts one text file or a directory tree. Optional `env` injects per-call variables (request-scoped secrets — see Request-Scoped Secrets below); `LocalSandbox` merges them into the host subprocess environment and `AioSandbox` uses a fresh `bash.exec(env=...)` session. -**Provider Pattern**: `SandboxProvider` has an `acquire`, `acquire_async`, `get`, `release` lifecycle. Async agent/tool paths use async lifecycle hooks so Docker creation, discovery, cross-process locking, readiness polling, and release stay off the event loop. Providers that enforce a lead Agent's explicit skill policy across its tool surface set `supports_agent_skill_isolation=True`; bind-mount providers observe prepared thread roots, while upload providers implement `sync_agent_skills`. Host-backed providers report false whenever an enabled shell can bypass path mappings. The middleware fails closed before acquiring from an unsupported provider under an explicit policy. -**Shared components** (RFC #4741): remote providers derive deterministic IDs with `derive_sandbox_scope_token` (`sandbox/identity.py`; its keyword-only SHA-256/16-hex contract must not change or existing containers become orphaned), and serialize selected acquire/release transitions with `AcquireSerializer` (`sandbox/acquire_serialization.py`): a refcounted per-key `threading.Lock` table with bounded growth, a bounded dedicated executor so async waits never touch the event loop or default executor, worker-owned cancellation cleanup independent of a cancelled event-loop task resuming, and idempotent `close()` from provider `shutdown()`/`reset()`. AIO keys by `(user_id, thread_id)`; E2B by `(user_id, thread_id, skills_root)`; BoxLite/Tenki/OpenSandbox by the derived id. `thread_id=None` acquires (random UUIDs) bypass the serializer. +**Interface**: `Sandbox`: `execute_command(command, env=None)`, additive `execute_command_in_scope(..., scope_id=...)` / `release_command_scope(scope_id)`, `read_file`, `write_file`, `list_dir`, `glob`, `grep`. Scoped hooks default to pass-through without server-side sessions, preserving third-party subclasses. `grep` accepts a text file or directory tree. Per-call `env` injects secrets: `LocalSandbox` merges into the subprocess environment; `AioSandbox` uses fresh `bash.exec(env=...)` sessions. `list_dir`: missing path → `FileNotFoundError`; command/client failure → `OSError`, never `[]` (`ls_tool`: `(empty)`). Remotes use `sandbox/remote_list_dir.py`: capture `find`'s status, not `| head`'s (`sh -lc` lacks `pipefail`); missing binary → `OSError`; truncation SIGPIPE → success. +**Provider Pattern**: `SandboxProvider` exposes `acquire`, `acquire_async`, `get`, `release`. Async agent/tool paths use async hooks to keep Docker creation, discovery, cross-process locking, readiness polling, and release off-loop. Set `supports_agent_skill_isolation=True` only when the whole tool surface enforces explicit lead Agent policy: bind mounts use prepared thread roots; upload providers implement `sync_agent_skills`. Host-backed providers report false if an enabled shell bypasses path mappings. Under explicit policy, middleware rejects unsupported providers before acquire. +**Shared components** (RFC #4741): remote IDs use `derive_sandbox_scope_token` (`sandbox/identity.py`); preserve its keyword-only SHA-256/16-hex contract to avoid orphaning containers. `AcquireSerializer` (`sandbox/acquire_serialization.py`) serializes selected acquire/release transitions with a bounded, refcounted per-key `threading.Lock` table and dedicated bounded executor (no event-loop/default-executor blocking). Workers own cancellation cleanup without waiting for cancelled tasks to resume; provider `shutdown()`/`reset()` calls idempotent `close()`. Keys: AIO `(user_id, thread_id)`, E2B `(user_id, thread_id, skills_root)`, BoxLite/Tenki/OpenSandbox derived id. Random-UUID `thread_id=None` acquires bypass serialization. **Execution leases** (`sandbox/lease.py`, #5128): cross-instance ownership decides which Gateway may reap a container; process-local `SandboxLeaseManager` tracks concurrent lead, subagent, Gateway-request, and channel-upload users of one client. Runs get ephemeral owners, persisted sandboxes are retained idempotently, and the last holder performs any pending `SandboxProvider.release`. Outer lifecycle fences repeat idempotent release after their complete graph/tool/request batch drains; per-tool terminal `Command` wrappers never release because sibling handlers may still run. Fork-restored children and upload syncs use non-releasing holders: they fence the client and own scope cleanup without themselves requesting a park; an earlier normal-owner request waits for them, and a missing fork client is replaced by a normal owner. Persisted lookup plus retention is serialized per `(user_id, thread_id)`; stale bindings fall through to acquire, and a post-acquire lookup miss rolls back before raising. Provider I/O does not hold the metadata lock. Repeated cancellation cannot interrupt acquire/rollback/release reconciliation or let a `to_thread` sandbox operation outlive its enclosing execution/request holder; failures are logged without replacing the original cancellation. Lease/scope context IDs are server-owned: Gateway and worker scrub caller values; only the internal subagent path assigns a task ID. Managers are registered by provider object identity, not hash/equality, so unhashable custom providers remain valid. Subagent owners also serve as `sandbox_command_scope_id`: AIO gives each scope one ordered persistent shell session, replaces it after `ErrorObservation`, and cleans it on lease release. Registry identity is revalidated after every scope-lock wait, preventing queued commands from resurrecting released sessions. Env-bearing commands use fresh `bash.exec` sessions so secrets do not persist. **Authorization gate** (`sandbox:execute`, RFC #4063 Phase 3): every sandbox-backed tool call passes through the gate in `deerflow/authz/sandbox_authz.py` - a binary `authorize(principal, "sandbox", "execute", target="*")` check before either reusing a persisted sandbox id or calling `provider.acquire`. Rechecking reuse is required because authorization config and user roles can change while the sandbox remains cached. Sync tool invocations call `authorize_sandbox_execution`; async tool invocations await `authorize_sandbox_execution_async` exactly once. A task-local `ContextVar` scopes that single decision across the complete composed tool invocation, including `ReadBeforeWriteMiddleware`'s pre-write inspection, tool body, and post-read mark; the value is copied into `asyncio.to_thread` workers. Authorization denial is converted to the normal error `ToolMessage` at the composed middleware boundary and is explicitly excluded from the gate's generic fail-open handlers. Async config loading and provider class discovery/import are offloaded before `aauthorize()` so reused sandbox calls do not hash config files or import custom modules on the event loop; provider construction remains on the running event loop because async providers may initialize loop-affine clients. The gate lives at the single tool initialization entry point (`ensure_sandbox_initialized` / `ensure_sandbox_initialized_async` in `tools.py`), while `SandboxMiddleware.before_agent` / `abefore_agent` apply the matching sync/async check to eager acquisition. Deny raises `SandboxAuthorizationError` (`sandbox/exceptions.py`), which propagates out of ordinary 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 (authorization calls 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 upload/artifact sync calls `try_acquire_sandbox_for_request` (`app/gateway/authz.py`), which gates, returns a request lease, and skips sync on deny while preserving the primary operation. Callers release after their last sandbox operation; artifacts request normal parking, uploads do not. Tests: `tests/test_sandbox_authorization.py` and `tests/blocking_io/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. diff --git a/backend/packages/harness/deerflow/sandbox/local/list_dir.py b/backend/packages/harness/deerflow/sandbox/local/list_dir.py index 35e51f848..c020a1303 100644 --- a/backend/packages/harness/deerflow/sandbox/local/list_dir.py +++ b/backend/packages/harness/deerflow/sandbox/local/list_dir.py @@ -14,13 +14,17 @@ def list_dir(path: str, max_depth: int = 2) -> list[str]: Returns: A list of absolute paths for files and directories, - excluding items matching IGNORE_PATTERNS. + excluding items matching IGNORE_PATTERNS. An existing empty + directory returns an empty list. + + Raises: + FileNotFoundError: If ``path`` does not exist or is not a directory. """ result: list[str] = [] root_path = Path(path).resolve() if not root_path.is_dir(): - return result + raise FileNotFoundError(path) def _is_within_root(candidate: Path) -> bool: try: diff --git a/backend/packages/harness/deerflow/sandbox/remote_list_dir.py b/backend/packages/harness/deerflow/sandbox/remote_list_dir.py new file mode 100644 index 000000000..9e2cc1ed9 --- /dev/null +++ b/backend/packages/harness/deerflow/sandbox/remote_list_dir.py @@ -0,0 +1,87 @@ +"""Remote ``list_dir`` find command and stdout contract. + +Remote providers list with ``find ... | head`` under ``sh -lc``. POSIX ``sh`` +does not enable ``pipefail``, so the pipeline's exit code is ``head``'s, not +``find``'s. A missing ``find`` binary (127) then looks like an empty listing +and becomes ``FileNotFoundError``. + +The command below writes ``find``'s own status after the bounded listing so +callers can tell a missing path from a command failure. ``head`` closing the +pipe can kill ``find`` with SIGPIPE (141); that is a successful truncation, +not an error. +""" + +from __future__ import annotations + +import shlex + +_STATUS_PREFIX = "__DF_FIND_STATUS__:" +_LIST_LIMIT = 500 +# 0 = ok, 1 = find reported a missing start point / tree error, 141 = SIGPIPE +# from head truncating a large listing. +_FIND_OK = (0, 1, 141) + + +def remote_list_dir_command(path: str, max_depth: int, *, limit: int = _LIST_LIMIT) -> str: + """Return a POSIX ``sh -lc`` script that lists ``path`` and records find status.""" + quoted = shlex.quote(path) + depth = int(max_depth) + n = int(limit) + # Status file is written by the find side of the pipe, then printed AFTER + # head so a 500-line listing cannot truncate the marker. ``set +e`` undoes + # a login-profile ``set -e`` so a failing find still records $?. End with + # ``exit`` of that status (126 if the file is missing): the last command + # would otherwise be ``rm``, whose 0/1 is not find's status. + return ( + f"set +e; _st=/tmp/df_find_$$; " + f"{{ find -H {quoted} -maxdepth {depth} \\( -type f -o -type d \\) 2>/dev/null; " + f'echo $? > "$_st"; }} | head -n {n}; ' + f'st=$(cat "$_st" 2>/dev/null); ' + f"printf '\\n%s\\n' {_STATUS_PREFIX}$st; " + f'rm -f "$_st"; exit "${{st:-126}}"' + ) + + +def parse_remote_list_dir_output( + stdout: str | None, + resolved: str, + *, + pipeline_exit_code: int | None = None, +) -> list[str]: + """Parse listing stdout, preferring the find-status marker over pipeline status. + + Raises: + OSError: Command/client failure (missing binary, invocation error, ...). + FileNotFoundError: ``find`` ran and produced no entries (missing path). + """ + # find delimits records with "\n" only. splitlines() would also split on + # \v, \f, \x1c-\x1e and \x85, which are legal in Linux filenames. Do not + # strip entries: trailing whitespace can be part of the name. + lines = (stdout or "").split("\n") + if lines and lines[-1] == "": + lines.pop() + + find_status: int | None = None + if lines and lines[-1].startswith(_STATUS_PREFIX): + raw = lines.pop()[len(_STATUS_PREFIX) :] + try: + find_status = int(raw) + except ValueError: + find_status = None + if lines and lines[-1] == "": + lines.pop() + + if find_status is None: + # Do not treat a missing marker as success. The process status used to + # be ``rm``'s (0/1, both in _FIND_OK), which reclassified a lost 127 + # as FileNotFoundError. + if pipeline_exit_code is not None and pipeline_exit_code not in _FIND_OK: + raise OSError(f"Failed to list_dir {resolved}: command exited with code {pipeline_exit_code}") + raise OSError(f"Failed to list_dir {resolved}: find status marker missing") + if find_status not in _FIND_OK: + raise OSError(f"Failed to list_dir {resolved}: command exited with code {find_status}") + + entries = [line for line in lines if line] + if not entries: + raise FileNotFoundError(resolved) + return entries diff --git a/backend/packages/harness/deerflow/sandbox/sandbox.py b/backend/packages/harness/deerflow/sandbox/sandbox.py index 35ef971df..ab69c0a2d 100644 --- a/backend/packages/harness/deerflow/sandbox/sandbox.py +++ b/backend/packages/harness/deerflow/sandbox/sandbox.py @@ -175,7 +175,15 @@ class Sandbox(ABC): max_depth: The maximum depth to traverse. Default is 2. Returns: - The contents of the directory. + The contents of the directory. An existing empty directory may + return an empty list. A missing path must not. + + Raises: + FileNotFoundError: If ``path`` does not exist or is not a directory. + OSError: If the listing cannot be performed (command/client failure). + Both local and remote implementations must raise rather than + return ``[]`` for failure or a missing path: ``ls_tool`` + renders an empty list as ``(empty)``. """ pass diff --git a/backend/tests/test_aio_sandbox.py b/backend/tests/test_aio_sandbox.py index 1035c724b..e7d0e51ed 100644 --- a/backend/tests/test_aio_sandbox.py +++ b/backend/tests/test_aio_sandbox.py @@ -602,7 +602,7 @@ class TestListDirSerialization: """list_dir should hold the lock during execution.""" lock_was_held = [] - original_exec = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(output="/a\n/b"))) + original_exec = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(output="/a\n/b\n\n__DF_FIND_STATUS__:0\n", exit_code=0))) def tracking_exec(command, **kwargs): lock_was_held.append(sandbox._lock.locked()) @@ -614,6 +614,39 @@ class TestListDirSerialization: assert result == ["/a", "/b"] assert lock_was_held == [True], "list_dir must hold the lock during exec_command" + def test_list_dir_raises_when_exec_fails(self, sandbox): + sandbox._client.shell.exec_command = MagicMock(side_effect=RuntimeError("sandbox down")) + + with pytest.raises(OSError, match="Failed to list directory"): + sandbox.list_dir("/test") + + def test_list_dir_raises_when_find_returns_no_entries(self, sandbox): + sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(output="\n__DF_FIND_STATUS__:1\n", exit_code=1))) + + with pytest.raises(FileNotFoundError): + sandbox.list_dir("/missing") + + def test_list_dir_raises_oserror_when_result_data_is_none(self, sandbox): + sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=None)) + + with pytest.raises(OSError, match="Failed to list directory"): + sandbox.list_dir("/test") + + def test_list_dir_raises_oserror_when_find_exit_is_not_missing_path(self, sandbox): + sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(output="", exit_code=127))) + + with pytest.raises(OSError, match="exited with code 127"): + sandbox.list_dir("/test") + + def test_list_dir_uses_find_H(self, sandbox): + sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(output="/test\n\n__DF_FIND_STATUS__:0\n", exit_code=0))) + + sandbox.list_dir("/test") + + command = sandbox._client.shell.exec_command.call_args.kwargs["command"] + assert "find -H " in command + assert "\\( -type f -o -type d \\)" in command + class TestNoChangeTimeout: """Verify that no_change_timeout is forwarded to every exec_command call.""" @@ -657,7 +690,7 @@ class TestNoChangeTimeout: def mock_exec(command, **kwargs): calls.append(kwargs) - return SimpleNamespace(data=SimpleNamespace(output="/a\n/b")) + return SimpleNamespace(data=SimpleNamespace(output="/a\n/b\n\n__DF_FIND_STATUS__:0\n", exit_code=0)) sandbox._client.shell.exec_command = mock_exec @@ -904,6 +937,6 @@ class TestClose: def test_list_dir_preserves_trailing_space_in_filename(sandbox): """ "notes.txt " (trailing space) is a legal Linux filename; find prints it verbatim, one entry per line, so a per-line strip() corrupts the name.""" - sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(output="/test/notes.txt \n/test/sub\n"))) + sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(output="/test/notes.txt \n/test/sub\n\n__DF_FIND_STATUS__:0\n", exit_code=0))) assert sandbox.list_dir("/test") == ["/test/notes.txt ", "/test/sub"] diff --git a/backend/tests/test_boxlite_provider.py b/backend/tests/test_boxlite_provider.py index d13949616..a4c5f6543 100644 --- a/backend/tests/test_boxlite_provider.py +++ b/backend/tests/test_boxlite_provider.py @@ -1309,7 +1309,7 @@ def test_list_dir_and_glob_preserve_trailing_space_in_filename() -> None: # verbatim, one entry per line, so a per-line strip() corrupts the name. class _FindBox: async def exec(self, *argv, env=None, timeout=None): - return types.SimpleNamespace(stdout="/mnt/user-data/workspace/notes.txt \n", stderr="", exit_code=0) + return types.SimpleNamespace(stdout="/mnt/user-data/workspace/notes.txt \n\n__DF_FIND_STATUS__:0\n", stderr="", exit_code=0) box = BoxliteBox("box-id", box=_FindBox(), run=_fake_run) @@ -1318,3 +1318,40 @@ def test_list_dir_and_glob_preserve_trailing_space_in_filename() -> None: found, truncated = box.glob("/mnt/user-data/workspace", "notes*") assert found == ["/mnt/user-data/workspace/notes.txt "] assert truncated is False + + +def test_list_dir_raises_when_find_returns_no_entries() -> None: + class _EmptyBox: + async def exec(self, *argv, env=None, timeout=None): + return types.SimpleNamespace(stdout="\n__DF_FIND_STATUS__:1\n", stderr="", exit_code=1) + + box = BoxliteBox("box-id", box=_EmptyBox(), run=_fake_run) + + with pytest.raises(FileNotFoundError): + box.list_dir("/mnt/user-data/workspace") + + +def test_list_dir_raises_oserror_when_find_exit_is_not_missing_path() -> None: + # find exit 1 is "start point absent"; 127 (no binary) must not look missing. + class _MissingBinaryBox: + async def exec(self, *argv, env=None, timeout=None): + return types.SimpleNamespace(stdout="", stderr="", exit_code=127) + + box = BoxliteBox("box-id", box=_MissingBinaryBox(), run=_fake_run) + + with pytest.raises(OSError, match="exited with code 127"): + box.list_dir("/mnt/user-data/workspace") + + +def test_list_dir_uses_find_H_to_dereference_start_point() -> None: + captured: list[tuple] = [] + + class _FindBox: + async def exec(self, *argv, env=None, timeout=None): + captured.append(argv) + return types.SimpleNamespace(stdout="/mnt/user-data/workspace\n\n__DF_FIND_STATUS__:0\n", stderr="", exit_code=0) + + box = BoxliteBox("box-id", box=_FindBox(), run=_fake_run) + + assert box.list_dir("/mnt/user-data/workspace") == ["/mnt/user-data/workspace"] + assert any(len(argv) >= 3 and "find -H " in str(argv[2]) for argv in captured) diff --git a/backend/tests/test_e2b_sandbox_provider.py b/backend/tests/test_e2b_sandbox_provider.py index bb5ef817b..e332343a3 100644 --- a/backend/tests/test_e2b_sandbox_provider.py +++ b/backend/tests/test_e2b_sandbox_provider.py @@ -5198,13 +5198,60 @@ def test_list_dir_preserves_trailing_space_in_filename(): # "notes.txt " (trailing space) is a legal Linux filename; find prints it # verbatim, one entry per line, so a per-line strip() corrupts the name and # every follow-up file API call on the listed path misses the real file. - listing = SimpleNamespace(stdout="/home/user/notes.txt \n/home/user/sub\n", stderr="", exit_code=0) + listing = SimpleNamespace(stdout="/home/user/notes.txt \n/home/user/sub\n\n__DF_FIND_STATUS__:0\n", stderr="", exit_code=0) client = FakeClient(commands=FakeCommandsAPI([listing])) sb = _make_sandbox(client) assert sb.list_dir("/home/user") == ["/home/user/notes.txt ", "/home/user/sub"] +def test_list_dir_raises_when_command_fails(): + client = FakeClient(commands=FakeCommandsAPI([FakeCommandsAPI.GONE])) + sb = _make_sandbox(client) + + with pytest.raises(OSError, match="Failed to list_dir"): + sb.list_dir("/home/user") + + +def test_list_dir_raises_when_client_closed(): + sb = _make_sandbox(FakeClient()) + sb.close() + + with pytest.raises(RuntimeError, match="closed"): + sb.list_dir("/home/user") + + +def test_list_dir_raises_when_find_returns_no_entries(): + # `find ... 2>/dev/null` on a missing path yields empty stdout; that is not + # a real empty directory (`find -type d` still prints the directory itself). + listing = SimpleNamespace(stdout="\n__DF_FIND_STATUS__:1\n", stderr="", exit_code=1) + client = FakeClient(commands=FakeCommandsAPI([listing])) + sb = _make_sandbox(client) + + with pytest.raises(FileNotFoundError): + sb.list_dir("/home/user/missing") + + +def test_list_dir_raises_oserror_when_find_exit_is_not_missing_path(): + listing = SimpleNamespace(stdout="", stderr="", exit_code=127) + client = FakeClient(commands=FakeCommandsAPI([listing])) + sb = _make_sandbox(client) + + with pytest.raises(OSError, match="exited with code 127"): + sb.list_dir("/home/user") + + +def test_list_dir_uses_find_H_to_dereference_start_point(): + # find defaults to -P, so a symlink start point (E2B /mnt/acp-workspace) + # would produce empty stdout and raise FileNotFoundError without -H. + listing = SimpleNamespace(stdout="/mnt/acp-workspace\n\n__DF_FIND_STATUS__:0\n", stderr="", exit_code=0) + commands = FakeCommandsAPI([listing]) + sb = _make_sandbox(FakeClient(commands=commands)) + + assert sb.list_dir("/mnt/acp-workspace") == ["/mnt/acp-workspace"] + assert commands.calls and "find -H " in commands.calls[0] + + def test_glob_preserves_trailing_space_in_filename(): listing = SimpleNamespace(stdout="/home/user/notes.txt \n", stderr="", exit_code=0) client = FakeClient(commands=FakeCommandsAPI([listing])) diff --git a/backend/tests/test_local_sandbox_provider_mounts.py b/backend/tests/test_local_sandbox_provider_mounts.py index 557b31d0d..8eb0b2604 100644 --- a/backend/tests/test_local_sandbox_provider_mounts.py +++ b/backend/tests/test_local_sandbox_provider_mounts.py @@ -330,6 +330,31 @@ class TestSymlinkEscapes: assert "/mnt/data/nested/linked-dir/" in entries assert "/mnt/data/dir-link" not in entries + def test_list_dir_raises_when_path_is_missing(self, tmp_path): + mount_dir = tmp_path / "mount" + mount_dir.mkdir() + sandbox = LocalSandbox( + "test", + [ + PathMapping(container_path="/mnt/data", local_path=str(mount_dir), read_only=False), + ], + ) + + with pytest.raises(FileNotFoundError): + sandbox.list_dir("/mnt/data/missing") + + def test_list_dir_empty_directory_returns_empty(self, tmp_path): + mount_dir = tmp_path / "mount" + mount_dir.mkdir() + sandbox = LocalSandbox( + "test", + [ + PathMapping(container_path="/mnt/data", local_path=str(mount_dir), read_only=False), + ], + ) + + assert sandbox.list_dir("/mnt/data") == [] + def test_write_file_blocks_symlink_into_nested_read_only_mount(self, tmp_path): repo_dir = tmp_path / "repo" repo_dir.mkdir() diff --git a/backend/tests/test_opensandbox_provider.py b/backend/tests/test_opensandbox_provider.py index 69cc25763..e1d51bc64 100644 --- a/backend/tests/test_opensandbox_provider.py +++ b/backend/tests/test_opensandbox_provider.py @@ -136,20 +136,25 @@ class _FakeCommands: return _execution(exit_code=9) if command == "missing-complete": return _execution(stderr=("stream ended",), exit_code=None) - if command.startswith("find "): + if command.startswith("find ") or "find -H " in command: return self._find(command) if command.startswith(("grep ", "{ grep ")): return self._grep(command) return _execution() def _find(self, command: str) -> _Execution: - tokens = shlex.split(command) - root = tokens[1].rstrip("/") or "/" - include_dirs = "d" in tokens + match = re.search(r"(?:^|[\s;{])find(?:\s+-[HLP])*\s+(\S+)", command) + root = (match.group(1).strip("'\"") if match else "").rstrip("/") or "/" + include_dirs = "-type d" in command paths = list(self._owner.file_data) if include_dirs: paths.extend(self._owner.directories) matches = sorted(path for path in set(paths) if path == root or path.startswith(f"{root}/")) + if "__DF_FIND_STATUS__:" in command: + status = 0 if matches else 1 + marker = f"__DF_FIND_STATUS__:{status}" + stdout = (*matches, "", marker) if matches else ("", marker) + return _execution(stdout=stdout, exit_code=status) return _execution(stdout=tuple(matches)) def _grep(self, command: str) -> _Execution: @@ -759,6 +764,23 @@ def test_sandbox_id_matches_shared_identity(): assert OpenSandboxProvider._sandbox_id("t-1", "") == derive_sandbox_scope_token(user_id="", thread_id="t-1") +def test_list_dir_raises_when_find_returns_no_entries() -> None: + remote = _FakeRemote("remote") + box = _box(remote) + + with pytest.raises(FileNotFoundError): + box.list_dir("/mnt/user-data/missing") + + +def test_list_dir_raises_oserror_when_find_exit_is_not_missing_path() -> None: + # find exit 1 is "start point absent"; 127 (no binary) must not look missing. + box = _box(_FakeRemote("remote")) + box._run = lambda *args, **kwargs: _execution(exit_code=127) + + with pytest.raises(OSError, match="exited with code 127"): + box.list_dir("/mnt/user-data/workspace") + + def test_list_dir_and_glob_preserve_trailing_space_in_filename() -> None: # "notes.txt " (trailing space) is a legal Linux filename; find prints it # verbatim, one entry per line, so a per-line strip() corrupts the name. diff --git a/backend/tests/test_remote_list_dir.py b/backend/tests/test_remote_list_dir.py new file mode 100644 index 000000000..2cbbeef1a --- /dev/null +++ b/backend/tests/test_remote_list_dir.py @@ -0,0 +1,186 @@ +"""Remote list_dir command/parser contract, including a real POSIX sh pipeline.""" + +from __future__ import annotations + +import os +import shutil +import stat +import subprocess + +import pytest + +from deerflow.sandbox.remote_list_dir import parse_remote_list_dir_output, remote_list_dir_command + +_POSIX_SH = pytest.mark.skipif( + os.name == "nt" or shutil.which("sh") is None or shutil.which("head") is None, + reason="POSIX sh pipeline required", +) + + +def test_parse_marker_127_is_command_failure_not_missing_path() -> None: + stdout = "\n__DF_FIND_STATUS__:127\n" + with pytest.raises(OSError, match="exited with code 127"): + parse_remote_list_dir_output(stdout, "/dir", pipeline_exit_code=0) + + +def test_parse_marker_1_empty_is_missing_path() -> None: + stdout = "\n__DF_FIND_STATUS__:1\n" + with pytest.raises(FileNotFoundError): + parse_remote_list_dir_output(stdout, "/dir", pipeline_exit_code=0) + + +def test_parse_marker_0_returns_listing_and_keeps_trailing_space() -> None: + stdout = "/dir/notes.txt \n/dir/sub\n\n__DF_FIND_STATUS__:0\n" + assert parse_remote_list_dir_output(stdout, "/dir", pipeline_exit_code=0) == [ + "/dir/notes.txt ", + "/dir/sub", + ] + + +def test_parse_sigpipe_truncated_listing_is_success() -> None: + lines = "\n".join(f"/dir/f{i}" for i in range(500)) + stdout = f"{lines}\n\n__DF_FIND_STATUS__:141\n" + entries = parse_remote_list_dir_output(stdout, "/dir", pipeline_exit_code=0) + assert len(entries) == 500 + assert entries[0] == "/dir/f0" + assert entries[-1] == "/dir/f499" + + +def test_parse_falls_back_to_pipeline_exit_without_marker() -> None: + with pytest.raises(OSError, match="exited with code 127"): + parse_remote_list_dir_output("", "/dir", pipeline_exit_code=127) + with pytest.raises(OSError, match="marker missing"): + parse_remote_list_dir_output("", "/dir", pipeline_exit_code=0) + with pytest.raises(OSError, match="marker missing"): + parse_remote_list_dir_output("/dir\n", "/dir", pipeline_exit_code=0) + + +@_POSIX_SH +def test_parse_without_marker_real_subprocess_status_is_not_always_ok() -> None: + """``rm -f`` exits 0/1, both in _FIND_OK. A real process status with no marker must not become FileNotFoundError.""" + for script in ("exit 0", "exit 1"): + proc = subprocess.run(["sh", "-c", script], capture_output=True, text=True, check=False) + with pytest.raises(OSError, match="marker missing"): + parse_remote_list_dir_output( + proc.stdout, + "/dir", + pipeline_exit_code=proc.returncode, + ) + + +def test_command_records_find_status_after_head() -> None: + command = remote_list_dir_command("/mnt/acp-workspace", 2) + assert command.startswith("set +e; ") + assert "find -H " in command + assert "\\( -type f -o -type d \\)" in command + assert "head -n 500" in command + assert "__DF_FIND_STATUS__:" in command + assert command.index("find -H ") < command.index("head -n") + assert command.index("head -n") < command.index("__DF_FIND_STATUS__:") + assert 'exit "${st:-126}"' in command + + +def _run_list_dir_script(command: str, *, env: dict[str, str] | None = None) -> subprocess.CompletedProcess[str]: + # Providers invoke ``sh -lc``; tests use ``sh -c`` so an injected fake + # ``find`` on PATH is not overwritten by a login profile. + return subprocess.run( + ["sh", "-c", command], + capture_output=True, + text=True, + env=env, + check=False, + ) + + +def _env_with_bin(bin_dir: str) -> dict[str, str]: + env = os.environ.copy() + env["PATH"] = bin_dir + os.pathsep + env.get("PATH", "") + return env + + +@_POSIX_SH +def test_naive_find_head_pipeline_hides_find_127(tmp_path) -> None: + """Reproduction: without capturing find's status, head's 0 wins.""" + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + fake_find = fake_bin / "find" + fake_find.write_text("#!/bin/sh\nexit 127\n", encoding="utf-8") + fake_find.chmod(fake_find.stat().st_mode | stat.S_IEXEC) + + naive = "find -H /dir -maxdepth 2 \\( -type f -o -type d \\) 2>/dev/null | head -n 500" + proc = _run_list_dir_script(naive, env=_env_with_bin(str(fake_bin))) + assert proc.returncode == 0 + assert proc.stdout == "" + + +def _write_fake_find(tmp_path, script: str): + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + fake_find = fake_bin / "find" + fake_find.write_text(script, encoding="utf-8") + fake_find.chmod(fake_find.stat().st_mode | stat.S_IEXEC) + return fake_bin + + +@_POSIX_SH +def test_list_dir_command_surfaces_find_127_not_head_0(tmp_path) -> None: + fake_bin = _write_fake_find(tmp_path, "#!/bin/sh\nexit 127\n") + proc = _run_list_dir_script( + remote_list_dir_command("/dir", 2), + env=_env_with_bin(str(fake_bin)), + ) + assert proc.returncode == 127 + with pytest.raises(OSError, match="exited with code 127"): + parse_remote_list_dir_output(proc.stdout, "/dir", pipeline_exit_code=proc.returncode) + + +@_POSIX_SH +def test_list_dir_command_records_find_127_under_set_e(tmp_path) -> None: + fake_bin = _write_fake_find(tmp_path, "#!/bin/sh\nexit 127\n") + proc = _run_list_dir_script( + "set -e; " + remote_list_dir_command("/dir", 2), + env=_env_with_bin(str(fake_bin)), + ) + assert proc.returncode == 127 + with pytest.raises(OSError, match="exited with code 127"): + parse_remote_list_dir_output(proc.stdout, "/dir", pipeline_exit_code=proc.returncode) + + +@_POSIX_SH +@pytest.mark.skipif(shutil.which("find") is None, reason="system find required") +def test_list_dir_command_missing_path_is_file_not_found(tmp_path) -> None: + missing = tmp_path / "no-such-dir" + proc = _run_list_dir_script(remote_list_dir_command(str(missing), 2)) + with pytest.raises(FileNotFoundError): + parse_remote_list_dir_output(proc.stdout, str(missing), pipeline_exit_code=proc.returncode) + + +@_POSIX_SH +@pytest.mark.skipif(shutil.which("find") is None, reason="system find required") +def test_list_dir_command_empty_dir_lists_the_start_point(tmp_path) -> None: + empty = tmp_path / "empty" + empty.mkdir() + proc = _run_list_dir_script(remote_list_dir_command(str(empty), 2)) + entries = parse_remote_list_dir_output(proc.stdout, str(empty), pipeline_exit_code=proc.returncode) + assert str(empty) in entries + + +@_POSIX_SH +def test_list_dir_command_head_truncation_is_not_an_error(tmp_path) -> None: + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + fake_find = fake_bin / "find" + fake_find.write_text( + '#!/bin/sh\ni=1\nwhile [ "$i" -le 5000 ]; do\n echo "/dir/f$i"\n i=$((i+1))\ndone\nexit 0\n', + encoding="utf-8", + ) + fake_find.chmod(fake_find.stat().st_mode | stat.S_IEXEC) + + proc = _run_list_dir_script( + remote_list_dir_command("/dir", 2), + env=_env_with_bin(str(fake_bin)), + ) + entries = parse_remote_list_dir_output(proc.stdout, "/dir", pipeline_exit_code=proc.returncode) + assert len(entries) == 500 + assert entries[0] == "/dir/f1" + assert entries[-1] == "/dir/f500" diff --git a/backend/tests/test_tenki_provider.py b/backend/tests/test_tenki_provider.py index 9a3c70340..1b0555ceb 100644 --- a/backend/tests/test_tenki_provider.py +++ b/backend/tests/test_tenki_provider.py @@ -12,6 +12,7 @@ from __future__ import annotations import errno import logging import os +import re import shlex import sys import threading @@ -154,10 +155,15 @@ class _FakeSandbox: return _FakeResult(exit_code=1, stdout=b"5 passed, 1 error\n") if "BOOTSTRAP_OK" in script: # provider create-time bootstrap script return _FakeResult(stdout=b"BOOTSTRAP_OK\n") - if script.startswith("find "): - root = shlex.split(script)[1] + if script.startswith("find ") or "find -H " in script: + match = re.search(r"(?:^|[\s;{])find(?:\s+-[HLP])*\s+(\S+)", script) + root = match.group(1).strip("'\"") if match else "" hits = [p for p in self.files if p == root or p.startswith(f"{root.rstrip('/')}/")] - return _FakeResult(stdout=("\n".join(hits) + "\n").encode() if hits else b"") + listing = ("\n".join(hits) + "\n") if hits else "" + if "__DF_FIND_STATUS__:" in script: + status = 0 if hits else 1 + return _FakeResult(stdout=f"{listing}\n__DF_FIND_STATUS__:{status}\n".encode(), exit_code=status) + return _FakeResult(stdout=listing.encode()) if script.startswith("grep "): # grep -e 2>/dev/null | head -N tokens = shlex.split(script) @@ -628,8 +634,9 @@ def test_glob_include_dirs_adds_directory_type_to_find() -> None: def test_list_dir_forwards_max_depth() -> None: fake = _FakeSandbox() box = TenkiSandbox("sb", fake) + box.write_file("/mnt/user-data/workspace/a.txt", "x") box.list_dir("/mnt/user-data/workspace", max_depth=4) - find_scripts = [c["argv"][2] for c in fake.exec_calls if c["argv"][:2] == ("sh", "-lc") and c["argv"][2].startswith("find ")] + find_scripts = [c["argv"][2] for c in fake.exec_calls if c["argv"][:2] == ("sh", "-lc") and "find " in c["argv"][2]] assert find_scripts and "-maxdepth 4" in find_scripts[-1] @@ -1017,6 +1024,22 @@ def test_sandbox_id_matches_shared_identity(): assert TenkiSandboxProvider._sandbox_id("t-1", "") == derive_sandbox_scope_token(user_id="", thread_id="t-1") +def test_list_dir_raises_when_find_returns_no_entries() -> None: + box = TenkiSandbox("sb", _FakeSandbox()) + + with pytest.raises(FileNotFoundError): + box.list_dir("/mnt/user-data/missing") + + +def test_list_dir_raises_oserror_when_find_exit_is_not_missing_path() -> None: + # find exit 1 is "start point absent"; 127 (no binary) must not look missing. + box = TenkiSandbox("sb", _FakeSandbox()) + box._sh = lambda *args, **kwargs: _FakeResult(exit_code=127) + + with pytest.raises(OSError, match="exited with code 127"): + box.list_dir("/mnt/user-data/workspace") + + def test_list_dir_and_glob_preserve_trailing_space_in_filename() -> None: # "notes.txt " (trailing space) is a legal Linux filename; find prints it # verbatim, one entry per line, so a per-line strip() corrupts the name.