From 3e536944b734e1f084ae595bf273d703397283cb Mon Sep 17 00:00:00 2001 From: Totoro Date: Sat, 12 Sep 2026 15:47:58 +0800 Subject: [PATCH] fix(sandbox): stop remote grep/glob from reporting failures as no matches (#5380) * fix(sandbox): stop remote grep/glob from reporting failures as no matches E2B, OpenSandbox, BoxLite and Tenki ran grep/find behind `2>/dev/null | head`, so a missing search root, a missing grep/find binary or an unreadable tree exited 0 with empty stdout and the tools reported "No matches found". Wrap the search in sandbox/remote_search.py, which checks the root first and records the search's own status after head, as remote_list_dir does for list_dir: a missing root raises FileNotFoundError, a failed search raises OSError, and a genuine no-match still returns []. glob's find gains -H for symlinked roots, OpenSandbox's BusyBox fallback keeps the primary grep status, and E2B no longer swallows client errors. Regression tests run each provider's real command in a local POSIX sh. Fixes #5376 Co-Authored-By: Claude Opus 5 * fix(sandbox): fail remote grep/glob on partial traversal errors grep 2 / find 1 after some results were printed (an unreadable file or subdirectory) were returned as a complete search. Callers have no partial-result channel, and #5376 asks for permission and command failures to raise, so these statuses now raise OSError like any other failure. Only grep 0/1/141 and find 0/141 pass. The error for grep 2 / find 1 says that some files or directories could not be read and asks for a narrower path, so the agent can retry instead of giving up. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> Co-authored-by: Claude Opus 5 --- .../harness/deerflow/community/boxlite/box.py | 17 +- .../community/e2b_sandbox/e2b_sandbox.py | 26 ++- .../deerflow/community/opensandbox/sandbox.py | 19 +- .../deerflow/community/tenki/sandbox.py | 17 +- .../harness/deerflow/sandbox/AGENTS.md | 2 +- .../harness/deerflow/sandbox/remote_search.py | 79 +++++++ backend/tests/test_boxlite_provider.py | 76 ++++++- backend/tests/test_e2b_sandbox_provider.py | 96 +++++++- backend/tests/test_opensandbox_provider.py | 91 +++++++- backend/tests/test_remote_search.py | 212 ++++++++++++++++++ backend/tests/test_tenki_provider.py | 93 +++++++- 11 files changed, 688 insertions(+), 40 deletions(-) create mode 100644 backend/packages/harness/deerflow/sandbox/remote_search.py create mode 100644 backend/tests/test_remote_search.py diff --git a/backend/packages/harness/deerflow/community/boxlite/box.py b/backend/packages/harness/deerflow/community/boxlite/box.py index 3fc42fd45..531571d56 100644 --- a/backend/packages/harness/deerflow/community/boxlite/box.py +++ b/backend/packages/harness/deerflow/community/boxlite/box.py @@ -26,6 +26,7 @@ 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.remote_search import parse_remote_search_output, remote_search_command from deerflow.sandbox.sandbox import Sandbox, _validate_extra_env from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path, truncate_line @@ -306,12 +307,16 @@ class BoxliteBox(Sandbox): types = ("f", "d") if include_dirs else ("f",) type_expr = " -o ".join(f"-type {t}" for t in types) hard_limit = max(max_results * 4, max_results + 50) - r = self._sh(f"find {shlex.quote(resolved)} \\( {type_expr} \\) -print 2>/dev/null | head -{hard_limit}") + # -H follows a symlinked search root, as list_dir does. + search = f"find -H {shlex.quote(resolved)} \\( {type_expr} \\) -print 2>/dev/null" + r = self._sh(remote_search_command(search, resolved, limit=hard_limit)) + # A missing root or a failed find must not read as "no files matched" (#5376). + output = parse_remote_search_output(r.stdout, resolved, tool="find") matches: list[str] = [] root = resolved.rstrip("/") or "/" root_prefix = root if root == "/" else f"{root}/" - for entry in (r.stdout or "").splitlines(): + for entry in output.splitlines(): # Do NOT strip: trailing whitespace can be part of the filename. if not entry or (entry != root and not entry.startswith(root_prefix)): continue @@ -352,13 +357,15 @@ class BoxliteBox(Sandbox): flags.append("-i") flags.append("-F" if literal else "-E") total_cap = max(max_results * 4, max_results + 50) - cmd = "grep " + " ".join(flags) + f" -e {shlex.quote(pattern)} {shlex.quote(resolved)} 2>/dev/null | head -{total_cap}" - r = self._sh(cmd) + search = "grep " + " ".join(flags) + f" -e {shlex.quote(pattern)} {shlex.quote(resolved)} 2>/dev/null" + r = self._sh(remote_search_command(search, resolved, limit=total_cap)) + # A missing root, a missing grep or an unreadable tree must not read as "no matches" (#5376). + output = parse_remote_search_output(r.stdout, resolved, tool="grep") include = glob.split("/")[-1] if glob else None matches: list[GrepMatch] = [] truncated = False - for raw in (r.stdout or "").splitlines(): + for raw in output.splitlines(): try: file_path, line_no_str, line_text = raw.split(":", 2) except ValueError: 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 4208e9325..8977d2ea2 100644 --- a/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py +++ b/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py @@ -12,6 +12,7 @@ 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.remote_search import parse_remote_search_output, remote_search_command from deerflow.sandbox.sandbox import Sandbox, _validate_extra_env from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path, truncate_line @@ -406,18 +407,20 @@ class E2BSandbox(Sandbox): ) -> tuple[list[str], bool]: resolved = self._resolve_path(path) types = "f,d" if include_dirs else "f" + hard_limit = max(max_results * 4, max_results + 50) + # -H follows a symlinked search root (e.g. /mnt/acp-workspace), as list_dir does. + search = f"find -H {shlex.quote(resolved)} \\( " + " -o ".join(f"-type {t}" for t in types.split(",")) + " \\) -print 2>/dev/null" with self._lock: client = self._client if client is None: - return [], False + raise RuntimeError("sandbox client has been closed") try: - hard_limit = max(max_results * 4, max_results + 50) - cmd = f"find {shlex.quote(resolved)} \\( " + " -o ".join(f"-type {t}" for t in types.split(",")) + f" \\) -print 2>/dev/null | head -{hard_limit}" - result = client.commands.run(cmd) - output = getattr(result, "stdout", "") or "" + result = client.commands.run(remote_search_command(search, resolved, limit=hard_limit)) except Exception as e: logger.error("Failed to glob in e2b sandbox: %s", e) - return [], False + raise OSError(f"Failed to glob {resolved} in e2b sandbox: {e}") from e + # A missing root or a failed find must not read as "no files matched" (#5376). + output = parse_remote_search_output(getattr(result, "stdout", "") or "", resolved, tool="find") matches: list[str] = [] root = resolved.rstrip("/") or "/" @@ -478,18 +481,19 @@ class E2BSandbox(Sandbox): total_cap = max(max_results * 4, max_results + 50) flags.append(f"-m{per_file_cap}") - cmd = "grep " + " ".join(flags) + f" -- {shlex.quote(regex_source)} {shlex.quote(resolved)} 2>/dev/null" + f" | head -{total_cap}" + search = "grep " + " ".join(flags) + f" -- {shlex.quote(regex_source)} {shlex.quote(resolved)} 2>/dev/null" with self._lock: client = self._client if client is None: - return [], False + raise RuntimeError("sandbox client has been closed") try: - result = client.commands.run(cmd) - output = getattr(result, "stdout", "") or "" + result = client.commands.run(remote_search_command(search, resolved, limit=total_cap)) except Exception as e: logger.error("Failed to grep in e2b sandbox: %s", e) - return [], False + raise OSError(f"Failed to grep {resolved} in e2b sandbox: {e}") from e + # A missing root, a missing grep or an unreadable tree must not read as "no matches" (#5376). + output = parse_remote_search_output(getattr(result, "stdout", "") or "", resolved, tool="grep") root = resolved.rstrip("/") or "/" root_prefix = root if root == "/" else f"{root}/" diff --git a/backend/packages/harness/deerflow/community/opensandbox/sandbox.py b/backend/packages/harness/deerflow/community/opensandbox/sandbox.py index 8b5d3b668..54d1dc81f 100644 --- a/backend/packages/harness/deerflow/community/opensandbox/sandbox.py +++ b/backend/packages/harness/deerflow/community/opensandbox/sandbox.py @@ -13,6 +13,7 @@ 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.remote_search import parse_remote_search_output, remote_search_command from deerflow.sandbox.sandbox import Sandbox, _validate_extra_env from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path, truncate_line @@ -343,12 +344,16 @@ class OpenSandboxSandbox(Sandbox): types = ("f", "d") if include_dirs else ("f",) type_expr = " -o ".join(f"-type {entry_type}" for entry_type in types) hard_limit = max(max_results * 4, max_results + 50) - execution = self._run(f"find {shlex.quote(resolved)} \\( {type_expr} \\) -print 2>/dev/null | head -{hard_limit}") + # -H follows a symlinked search root, as list_dir does. + search = f"find -H {shlex.quote(resolved)} \\( {type_expr} \\) -print 2>/dev/null" + execution = self._run(remote_search_command(search, resolved, limit=hard_limit)) + # A missing root or a failed find must not read as "no files matched" (#5376). + output = parse_remote_search_output(execution_stdout(execution), resolved, tool="find") matches: list[str] = [] root = resolved.rstrip("/") or "/" root_prefix = root if root == "/" else f"{root}/" - for entry in execution_stdout(execution).splitlines(): + for entry in output.splitlines(): # Do NOT strip: trailing whitespace can be part of the filename. if not entry or (entry != root and not entry.startswith(root_prefix)) or should_ignore_path(entry): continue @@ -388,14 +393,18 @@ class OpenSandboxSandbox(Sandbox): arguments = f" -e {shlex.quote(pattern)} {shlex.quote(resolved)} 2>/dev/null" primary = "grep " + " ".join(flags) + arguments fallback = "grep " + " ".join(portable_flags) + arguments - command = f'{{ {primary}; status=$?; [ "$status" -eq 2 ] && {fallback}; }} | head -{hard_limit}' - execution = self._run(command) + # Retry without --include/-m only when the primary grep errors (BusyBox + # lacks them). Keep the primary's status otherwise, so a missing grep + # (127) is not reported as "no matches" (#5376). + search = f'{primary}; status=$?; if [ "$status" -eq 2 ]; then {fallback}; status=$?; fi; (exit "$status")' + execution = self._run(remote_search_command(search, resolved, limit=hard_limit)) + output = parse_remote_search_output(execution_stdout(execution), resolved, tool="grep") root = resolved.rstrip("/") or "/" root_prefix = root if root == "/" else f"{root}/" matches: list[GrepMatch] = [] seen_positions: set[tuple[str, int]] = set() - for raw in execution_stdout(execution).splitlines(): + for raw in output.splitlines(): try: file_path, line_number_text, line = raw.split(":", 2) line_number = int(line_number_text) diff --git a/backend/packages/harness/deerflow/community/tenki/sandbox.py b/backend/packages/harness/deerflow/community/tenki/sandbox.py index 30668798e..2fbe48dde 100644 --- a/backend/packages/harness/deerflow/community/tenki/sandbox.py +++ b/backend/packages/harness/deerflow/community/tenki/sandbox.py @@ -30,6 +30,7 @@ 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.remote_search import parse_remote_search_output, remote_search_command from deerflow.sandbox.sandbox import Sandbox, _validate_extra_env from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path, truncate_line @@ -389,12 +390,16 @@ class TenkiSandbox(Sandbox): types = ("f", "d") if include_dirs else ("f",) type_expr = " -o ".join(f"-type {t}" for t in types) hard_limit = max(max_results * 4, max_results + 50) - r = self._sh(f"find {shlex.quote(resolved)} \\( {type_expr} \\) -print 2>/dev/null | head -{hard_limit}") + # -H follows a symlinked search root, as list_dir does. + search = f"find -H {shlex.quote(resolved)} \\( {type_expr} \\) -print 2>/dev/null" + r = self._sh(remote_search_command(search, resolved, limit=hard_limit)) + # A missing root or a failed find must not read as "no files matched" (#5376). + output = parse_remote_search_output(r.stdout_text, resolved, tool="find") matches: list[str] = [] root = resolved.rstrip("/") or "/" root_prefix = root if root == "/" else f"{root}/" - for entry in (r.stdout_text or "").splitlines(): + for entry in output.splitlines(): # Do NOT strip: trailing whitespace can be part of the filename. if not entry or (entry != root and not entry.startswith(root_prefix)): continue @@ -437,14 +442,16 @@ class TenkiSandbox(Sandbox): flags.append("-i") flags.append("-F" if literal else "-E") total_cap = max(max_results * 4, max_results + 50) - cmd = "grep " + " ".join(flags) + f" -e {shlex.quote(pattern)} {shlex.quote(resolved)} 2>/dev/null | head -{total_cap}" - r = self._sh(cmd) + search = "grep " + " ".join(flags) + f" -e {shlex.quote(pattern)} {shlex.quote(resolved)} 2>/dev/null" + r = self._sh(remote_search_command(search, resolved, limit=total_cap)) + # A missing root, a missing grep or an unreadable tree must not read as "no matches" (#5376). + output = parse_remote_search_output(r.stdout_text, resolved, tool="grep") root = resolved.rstrip("/") or "/" root_prefix = root if root == "/" else f"{root}/" matches: list[GrepMatch] = [] truncated = False - for raw in (r.stdout_text or "").splitlines(): + for raw in output.splitlines(): try: file_path, line_no_str, line_text = raw.split(":", 2) except ValueError: diff --git a/backend/packages/harness/deerflow/sandbox/AGENTS.md b/backend/packages/harness/deerflow/sandbox/AGENTS.md index 02082defb..236f977fc 100644 --- a/backend/packages/harness/deerflow/sandbox/AGENTS.md +++ b/backend/packages/harness/deerflow/sandbox/AGENTS.md @@ -1,6 +1,6 @@ ### Sandbox System (`packages/harness/deerflow/sandbox/`) -**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. +**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)`). Remote `glob`/`grep` share it via `sandbox/remote_search.py`: missing root → `FileNotFoundError`, failed search → `OSError`; only a genuine no-match returns `[]`. 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. diff --git a/backend/packages/harness/deerflow/sandbox/remote_search.py b/backend/packages/harness/deerflow/sandbox/remote_search.py new file mode 100644 index 000000000..9fd96f2f2 --- /dev/null +++ b/backend/packages/harness/deerflow/sandbox/remote_search.py @@ -0,0 +1,79 @@ +"""Remote ``grep`` / ``glob`` command wrapper and stdout contract. + +Remote providers search with ``grep ... | head`` or ``find ... | head`` under +``sh -lc``. POSIX ``sh`` has no ``pipefail`` and the search's stderr is +discarded, so the pipeline status is ``head``'s: a missing search root, a +missing ``grep``/``find`` binary (127) or an unreadable tree all printed +nothing and exited 0, exactly like a genuine "no matches" (#5376). + +:func:`remote_search_command` checks the root first and records the search +command's own status after the bounded output, the same technique as +:mod:`deerflow.sandbox.remote_list_dir`. The script always exits 0 so SDKs that +raise on a non-zero exit still return the marker; the marker alone decides. +""" + +from __future__ import annotations + +import shlex +from typing import Literal + +SearchTool = Literal["grep", "find"] + +_STATUS_PREFIX = "__DF_SEARCH_STATUS__:" +_MISSING_ROOT = "missing" +# head closing the pipe after the limit kills the search with SIGPIPE: a +# successful truncation, not an error. +_SIGPIPE = 141 +# Statuses of a complete search. grep: 0 = matches, 1 = no match. Everything +# else fails, even after printed results. +_OK_STATUSES: dict[str, tuple[int, ...]] = {"grep": (0, 1, _SIGPIPE), "find": (0, _SIGPIPE)} +# grep 2 / find 1 usually mean an unreadable file or subdirectory. Printed +# results are then incomplete and callers have no partial-result channel, so +# the error tells the agent to narrow the search instead. +_READ_ERROR_STATUS: dict[str, int] = {"grep": 2, "find": 1} + + +def remote_search_command(search: str, root: str, *, limit: int) -> str: + """Wrap a ``grep``/``find`` command so its outcome survives ``| head``. + + ``search`` must write only results to stdout; callers keep ``2>/dev/null``. + """ + quoted = shlex.quote(root) + n = int(limit) + return ( + f"set +e; if [ ! -e {quoted} ]; then printf '%s\\n' {_STATUS_PREFIX}{_MISSING_ROOT}; exit 0; fi; " + f'_st=/tmp/df_search_$$; {{ {search}; echo $? > "$_st"; }} | head -n {n}; ' + f'st=$(cat "$_st" 2>/dev/null); rm -f "$_st"; ' + f"printf '\\n%s\\n' {_STATUS_PREFIX}\"$st\"; exit 0" + ) + + +def parse_remote_search_output(stdout: str | None, root: str, *, tool: SearchTool) -> str: + """Return the search output without the status marker. + + Raises: + FileNotFoundError: The search root does not exist. + OSError: The search did not complete (missing binary, unreadable + root or subtree, invalid invocation) or its status was lost. + """ + # Split on "\n" only: splitlines() would also split on characters that are + # legal in Linux filenames. Callers keep their own per-line handling. + lines = (stdout or "").split("\n") + if lines and lines[-1] == "": + lines.pop() + if not lines or not lines[-1].startswith(_STATUS_PREFIX): + raise OSError(f"Failed to {tool} under {root}: search status marker missing") + raw = lines.pop()[len(_STATUS_PREFIX) :] + if raw == _MISSING_ROOT: + raise FileNotFoundError(root) + if lines and lines[-1] == "": + lines.pop() + try: + status = int(raw) + except ValueError: + raise OSError(f"Failed to {tool} under {root}: search status unavailable") from None + if status in _OK_STATUSES[tool]: + return "\n".join(lines) + if status == _READ_ERROR_STATUS[tool]: + raise OSError(f"Failed to {tool} under {root}: {tool} exited with code {status}, usually because some files or directories could not be read; results would be incomplete, so search a narrower path") + raise OSError(f"Failed to {tool} under {root}: command exited with code {status}") diff --git a/backend/tests/test_boxlite_provider.py b/backend/tests/test_boxlite_provider.py index a4c5f6543..29fd12d95 100644 --- a/backend/tests/test_boxlite_provider.py +++ b/backend/tests/test_boxlite_provider.py @@ -9,6 +9,9 @@ from __future__ import annotations import asyncio import hashlib import logging +import os +import shutil +import subprocess import sys import threading import time @@ -48,6 +51,9 @@ class _FakeBox: # Health check: box.execute_command("echo ok") → exec("sh", "-lc", "echo ok") if len(argv) >= 3 and argv[0] == "sh" and argv[1] == "-lc" and argv[2] == "echo ok": return type("_FakeResult", (), {"stdout": "ok\n", "stderr": "", "exit_code": 0})() + if len(argv) >= 3 and argv[0] == "sh" and argv[1] == "-lc" and "__DF_SEARCH_STATUS__:" in argv[2]: + # A search that ran and found nothing (see sandbox/remote_search.py). + return type("_FakeResult", (), {"stdout": "\n__DF_SEARCH_STATUS__:1\n", "stderr": "", "exit_code": 0})() return _FakeResult() async def stop(self): @@ -207,7 +213,7 @@ def test_grep_always_prints_filename_for_single_file_paths() -> None: box.grep("/mnt/user-data/uploads/report.md", "needle") - grep_commands = [argv[0][2] for argv in fake._exec_history if argv[0][:2] == ("sh", "-lc") and argv[0][2].startswith("grep ")] + grep_commands = [argv[0][2] for argv in fake._exec_history if argv[0][:2] == ("sh", "-lc") and "grep -r " in argv[0][2]] assert grep_commands assert "-H" in grep_commands[-1].split() @@ -1309,7 +1315,8 @@ 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\n__DF_FIND_STATUS__:0\n", stderr="", exit_code=0) + marker = "__DF_SEARCH_STATUS__" if "__DF_SEARCH_STATUS__:" in argv[2] else "__DF_FIND_STATUS__" + return types.SimpleNamespace(stdout=f"/mnt/user-data/workspace/notes.txt \n\n{marker}:0\n", stderr="", exit_code=0) box = BoxliteBox("box-id", box=_FindBox(), run=_fake_run) @@ -1355,3 +1362,68 @@ def test_list_dir_uses_find_H_to_dereference_start_point() -> None: 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) + + +# ── Remote grep/glob failure contract against a real POSIX sh (#5376) ───────── + +_RS_POSIX = pytest.mark.skipif( + os.name == "nt" or any(shutil.which(tool) is None for tool in ("sh", "head", "grep", "find")), + reason="POSIX sh, head, grep and find required", +) + + +def _rs_env(tmp_path, failing: str | None = None) -> dict[str, str]: + env = os.environ.copy() + if failing is not None: + bin_dir = tmp_path / "fake-bin" + bin_dir.mkdir() + fake = bin_dir / failing + fake.write_text("#!/bin/sh\nexit 127\n", encoding="utf-8") + fake.chmod(0o755) + env["PATH"] = str(bin_dir) + os.pathsep + env.get("PATH", "") + return env + + +def _rs_box(tmp_path, monkeypatch, failing: str | None = None) -> BoxliteBox: + box = BoxliteBox("box-id", box=_FakeBox(name="box-id"), run=_fake_run) + shell_env = _rs_env(tmp_path, failing) + + def sh(script: str, env=None, timeout=None): + # ``sh -c`` (not ``-lc``) keeps a login profile from overriding the fake PATH. + proc = subprocess.run(["sh", "-c", script], capture_output=True, text=True, env=shell_env, check=False) + return types.SimpleNamespace(stdout=proc.stdout, stderr=proc.stderr, exit_code=proc.returncode) + + monkeypatch.setattr(box, "_sh", sh) + return box + + +def _rs_search(box, op: str, root: str): + return box.grep(root, "needle") if op == "grep" else box.glob(root, "**/*.py") + + +@_RS_POSIX +@pytest.mark.parametrize("op", ["grep", "glob"]) +def test_remote_search_missing_root_raises_file_not_found(tmp_path, monkeypatch, op) -> None: + with pytest.raises(FileNotFoundError): + _rs_search(_rs_box(tmp_path, monkeypatch), op, str(tmp_path / "missing")) + + +@_RS_POSIX +@pytest.mark.parametrize(("op", "binary"), [("grep", "grep"), ("glob", "find")]) +def test_remote_search_missing_binary_raises_instead_of_no_matches(tmp_path, monkeypatch, op, binary) -> None: + with pytest.raises(OSError, match="exited with code 127"): + _rs_search(_rs_box(tmp_path, monkeypatch, failing=binary), op, str(tmp_path)) + + +@_RS_POSIX +def test_remote_search_keeps_real_matches_and_genuine_no_match(tmp_path, monkeypatch) -> None: + (tmp_path / "src").mkdir() + (tmp_path / "src" / "app.py").write_text("def needle():\n", encoding="utf-8") + box = _rs_box(tmp_path, monkeypatch) + + matches, _ = box.grep(str(tmp_path), "needle") + assert [(os.path.basename(m.path), m.line_number) for m in matches] == [("app.py", 1)] + assert box.grep(str(tmp_path), "zzz_nothing") == ([], False) + found, _ = box.glob(str(tmp_path), "**/*.py") + assert [os.path.basename(path) for path in found] == ["app.py"] + assert box.glob(str(tmp_path), "*.md") == ([], False) diff --git a/backend/tests/test_e2b_sandbox_provider.py b/backend/tests/test_e2b_sandbox_provider.py index e332343a3..4066f645f 100644 --- a/backend/tests/test_e2b_sandbox_provider.py +++ b/backend/tests/test_e2b_sandbox_provider.py @@ -7,6 +7,8 @@ import hashlib import importlib import json import os +import shutil +import subprocess import threading import time from collections import OrderedDict @@ -3586,11 +3588,16 @@ def test_sync_outputs_to_host_skips_oversize_files(monkeypatch, tmp_path): # ────────────────────────────────────────────────────────────────────────────── +def _search_stdout(raw: str, *, status: int = 0) -> str: + """Stdout of ``remote_search_command``: the results, then the search's status marker.""" + return f"{raw}\n__DF_SEARCH_STATUS__:{status}\n" + + def test_grep_scoped_glob_excludes_unrelated_directory_matches(): """Regression: grep(glob="src/*.js") must not leak matches from sibling directories that merely share the file extension.""" raw_stdout = "/home/user/workspace/other_dir/unrelated.js:1:console.log('needle in other_dir');\n/home/user/workspace/src/app.js:1:console.log('needle in src');\n" - client = FakeClient(commands=FakeCommandsAPI([SimpleNamespace(stdout=raw_stdout, stderr="", exit_code=0)])) + client = FakeClient(commands=FakeCommandsAPI([SimpleNamespace(stdout=_search_stdout(raw_stdout), stderr="", exit_code=0)])) sb = _make_sandbox(client) matches, truncated = sb.grep("/mnt/user-data/workspace", "needle", glob="src/*.js") @@ -3606,7 +3613,7 @@ def test_grep_plain_glob_matches_files_in_any_directory(): keep matching files at any depth, same as before the directory-scoping fix.""" raw_stdout = "/home/user/workspace/other_dir/deep/mod.py:1:needle in a deeply nested file\n/home/user/workspace/src/app.py:1:needle in a python file too\n" - client = FakeClient(commands=FakeCommandsAPI([SimpleNamespace(stdout=raw_stdout, stderr="", exit_code=0)])) + client = FakeClient(commands=FakeCommandsAPI([SimpleNamespace(stdout=_search_stdout(raw_stdout), stderr="", exit_code=0)])) sb = _make_sandbox(client) matches, truncated = sb.grep("/mnt/user-data/workspace", "needle", glob="*.py") @@ -3624,7 +3631,7 @@ def test_grep_scoped_glob_still_passes_coarse_include_flag(): optimization (it narrows what grep has to search) even though it can't express directory scoping by itself -- the real scoping enforcement happens in the post-filter, not by dropping ``--include``.""" - client = FakeClient(commands=FakeCommandsAPI([SimpleNamespace(stdout="", stderr="", exit_code=0)])) + client = FakeClient(commands=FakeCommandsAPI([SimpleNamespace(stdout=_search_stdout("", status=1), stderr="", exit_code=0)])) sb = _make_sandbox(client) sb.grep("/mnt/user-data/workspace", "needle", glob="src/*.js") @@ -3636,7 +3643,7 @@ def test_grep_without_glob_is_unaffected(): """No regression: omitting ``glob`` entirely must return every match with no path-based post-filtering.""" raw_stdout = "/home/user/workspace/anywhere/file.txt:3:needle here\n" - client = FakeClient(commands=FakeCommandsAPI([SimpleNamespace(stdout=raw_stdout, stderr="", exit_code=0)])) + client = FakeClient(commands=FakeCommandsAPI([SimpleNamespace(stdout=_search_stdout(raw_stdout), stderr="", exit_code=0)])) sb = _make_sandbox(client) matches, truncated = sb.grep("/mnt/user-data/workspace", "needle") @@ -3648,7 +3655,7 @@ def test_grep_without_glob_is_unaffected(): def test_grep_single_file_path_with_matching_glob(): """A basename glob must also apply when the search root is one file.""" raw_stdout = "/home/user/uploads/report.md:2:needle here\n" - client = FakeClient(commands=FakeCommandsAPI([SimpleNamespace(stdout=raw_stdout, stderr="", exit_code=0)])) + client = FakeClient(commands=FakeCommandsAPI([SimpleNamespace(stdout=_search_stdout(raw_stdout), stderr="", exit_code=0)])) sb = _make_sandbox(client) matches, truncated = sb.grep("/mnt/user-data/uploads/report.md", "needle", glob="*.md") @@ -5253,7 +5260,7 @@ def test_list_dir_uses_find_H_to_dereference_start_point(): def test_glob_preserves_trailing_space_in_filename(): - listing = SimpleNamespace(stdout="/home/user/notes.txt \n", stderr="", exit_code=0) + listing = SimpleNamespace(stdout=_search_stdout("/home/user/notes.txt \n"), stderr="", exit_code=0) client = FakeClient(commands=FakeCommandsAPI([listing])) sb = _make_sandbox(client) @@ -5330,3 +5337,80 @@ def test_append_decodes_bytes_preimage(): sb.write_file("/mnt/user-data/outputs/report.txt", " world", append=True) assert files.write_calls == [("/home/user/outputs/report.txt", "hello world")] + + +# ── Remote grep/glob failure contract against a real POSIX sh (#5376) ───────── + +_RS_POSIX = pytest.mark.skipif( + os.name == "nt" or any(shutil.which(tool) is None for tool in ("sh", "head", "grep", "find")), + reason="POSIX sh, head, grep and find required", +) + + +def _rs_env(tmp_path, failing: str | None = None) -> dict[str, str]: + env = os.environ.copy() + if failing is not None: + bin_dir = tmp_path / "fake-bin" + bin_dir.mkdir() + fake = bin_dir / failing + fake.write_text("#!/bin/sh\nexit 127\n", encoding="utf-8") + fake.chmod(0o755) + env["PATH"] = str(bin_dir) + os.pathsep + env.get("PATH", "") + return env + + +class _RsShellCommands: + """``client.commands`` that runs each command string in a real local ``sh``.""" + + def __init__(self, env: dict[str, str]) -> None: + self.calls: list[str] = [] + self._env = env + + def run(self, cmd: str, envs: dict[str, str] | None = None, **kwargs) -> SimpleNamespace: + self.calls.append(cmd) + # ``sh -c`` (not ``-lc``) keeps a login profile from overriding the fake PATH. + proc = subprocess.run(["sh", "-c", cmd], capture_output=True, text=True, env=self._env, check=False) + return SimpleNamespace(stdout=proc.stdout, stderr=proc.stderr, exit_code=proc.returncode) + + +def _rs_sandbox(tmp_path, failing: str | None = None): + return _make_sandbox(FakeClient(commands=_RsShellCommands(_rs_env(tmp_path, failing)))) + + +def _rs_search(sb, op: str, root: str): + return sb.grep(root, "needle") if op == "grep" else sb.glob(root, "**/*.py") + + +@_RS_POSIX +@pytest.mark.parametrize("op", ["grep", "glob"]) +def test_remote_search_missing_root_raises_file_not_found(tmp_path, op): + with pytest.raises(FileNotFoundError): + _rs_search(_rs_sandbox(tmp_path), op, str(tmp_path / "missing")) + + +@_RS_POSIX +@pytest.mark.parametrize(("op", "binary"), [("grep", "grep"), ("glob", "find")]) +def test_remote_search_missing_binary_raises_instead_of_no_matches(tmp_path, op, binary): + with pytest.raises(OSError, match="exited with code 127"): + _rs_search(_rs_sandbox(tmp_path, failing=binary), op, str(tmp_path)) + + +@_RS_POSIX +def test_remote_search_keeps_real_matches_and_genuine_no_match(tmp_path): + (tmp_path / "src").mkdir() + (tmp_path / "src" / "app.py").write_text("def needle():\n", encoding="utf-8") + sb = _rs_sandbox(tmp_path) + + matches, _ = sb.grep(str(tmp_path), "needle") + assert [(os.path.basename(m.path), m.line_number) for m in matches] == [("app.py", 1)] + assert sb.grep(str(tmp_path), "zzz_nothing") == ([], False) + found, _ = sb.glob(str(tmp_path), "**/*.py") + assert [os.path.basename(path) for path in found] == ["app.py"] + assert sb.glob(str(tmp_path), "*.md") == ([], False) + + +@pytest.mark.parametrize("op", ["grep", "glob"]) +def test_remote_search_raises_when_the_client_call_fails(op): + sb = _make_sandbox(FakeClient(commands=FakeCommandsAPI([FakeCommandsAPI.GONE]))) + with pytest.raises(OSError): + _rs_search(sb, op, "/mnt/user-data/workspace") diff --git a/backend/tests/test_opensandbox_provider.py b/backend/tests/test_opensandbox_provider.py index e1d51bc64..2432095f3 100644 --- a/backend/tests/test_opensandbox_provider.py +++ b/backend/tests/test_opensandbox_provider.py @@ -11,8 +11,11 @@ from __future__ import annotations import asyncio import errno import logging +import os import re import shlex +import shutil +import subprocess import sys import threading import time @@ -136,6 +139,8 @@ class _FakeCommands: return _execution(exit_code=9) if command == "missing-complete": return _execution(stderr=("stream ended",), exit_code=None) + if "__DF_SEARCH_STATUS__:" in command: + return self._search(command) if command.startswith("find ") or "find -H " in command: return self._find(command) if command.startswith(("grep ", "{ grep ")): @@ -175,6 +180,20 @@ class _FakeCommands: rows.extend(rows) return _execution(stdout=tuple(rows)) + def _search(self, command: str) -> _Execution: + # remote_search_command: a root-existence check, then the wrapped search and its status marker. + root = shlex.split(re.search(r"\[ ! -e (.+?) \]; then", command).group(1))[0].rstrip("/") or "/" + paths = set(self._owner.file_data) | set(self._owner.directories) + if not any(path == root or path.startswith(f"{root}/") for path in paths): + return _execution(stdout=("__DF_SEARCH_STATUS__:missing",)) + inner = command[command.index("{ ") + 2 : command.index('; echo $? > "$_st"; }')] + if inner.startswith("find "): + rows, status = [message.text for message in self._find(inner).logs.stdout], 0 + else: + rows = [message.text for message in self._grep(inner).logs.stdout] + status = 0 if rows else 1 + return _execution(stdout=(*rows, "", f"__DF_SEARCH_STATUS__:{status}")) + class _FakeRemote: def __init__(self, remote_id: str, *, bootstrap_exit_code: int | None = 0) -> None: @@ -626,8 +645,11 @@ def test_list_glob_and_grep_return_virtual_paths() -> None: unsafe_glob_tokens = shlex.split(remote.commands.calls[-1][0]) assert "--include=*.py; echo injected" in unsafe_glob_tokens assert unsafe_glob_tokens.count("grep") == 2 - assert 'status=$?; [ "$status" -eq 2 ] &&' in remote.commands.calls[-1][0] - fallback_tokens = unsafe_glob_tokens[unsafe_glob_tokens.index("grep", 2) :] + # The fallback runs only on the primary's status 2, and the primary's + # status is kept otherwise so a missing grep (127) is not "no matches". + assert 'status=$?; if [ "$status" -eq 2 ]; then' in remote.commands.calls[-1][0] + assert '(exit "$status")' in remote.commands.calls[-1][0] + fallback_tokens = unsafe_glob_tokens[unsafe_glob_tokens.index("grep", unsafe_glob_tokens.index("grep") + 1) :] assert not any(token.startswith("--include=") or token.startswith("-m") for token in fallback_tokens) @@ -793,3 +815,68 @@ 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 + + +# ── Remote grep/glob failure contract against a real POSIX sh (#5376) ───────── + +_RS_POSIX = pytest.mark.skipif( + os.name == "nt" or any(shutil.which(tool) is None for tool in ("sh", "head", "grep", "find")), + reason="POSIX sh, head, grep and find required", +) + + +def _rs_env(tmp_path, failing: str | None = None) -> dict[str, str]: + env = os.environ.copy() + if failing is not None: + bin_dir = tmp_path / "fake-bin" + bin_dir.mkdir() + fake = bin_dir / failing + fake.write_text("#!/bin/sh\nexit 127\n", encoding="utf-8") + fake.chmod(0o755) + env["PATH"] = str(bin_dir) + os.pathsep + env.get("PATH", "") + return env + + +def _rs_box(tmp_path, monkeypatch, failing: str | None = None) -> OpenSandboxSandbox: + box = _box(_FakeRemote("remote")) + shell_env = _rs_env(tmp_path, failing) + + def run(command: str, *, env=None, timeout=None) -> _Execution: + # ``sh -c`` (not ``-lc``) keeps a login profile from overriding the fake PATH. + proc = subprocess.run(["sh", "-c", command], capture_output=True, text=True, env=shell_env, check=False) + return _execution(stdout=(proc.stdout,), stderr=(proc.stderr,) if proc.stderr else (), exit_code=proc.returncode) + + monkeypatch.setattr(box, "_run", run) + return box + + +def _rs_search(box, op: str, root: str): + return box.grep(root, "needle") if op == "grep" else box.glob(root, "**/*.py") + + +@_RS_POSIX +@pytest.mark.parametrize("op", ["grep", "glob"]) +def test_remote_search_missing_root_raises_file_not_found(tmp_path, monkeypatch, op) -> None: + with pytest.raises(FileNotFoundError): + _rs_search(_rs_box(tmp_path, monkeypatch), op, str(tmp_path / "missing")) + + +@_RS_POSIX +@pytest.mark.parametrize(("op", "binary"), [("grep", "grep"), ("glob", "find")]) +def test_remote_search_missing_binary_raises_instead_of_no_matches(tmp_path, monkeypatch, op, binary) -> None: + with pytest.raises(OSError, match="exited with code 127"): + _rs_search(_rs_box(tmp_path, monkeypatch, failing=binary), op, str(tmp_path)) + + +@_RS_POSIX +def test_remote_search_keeps_real_matches_and_genuine_no_match(tmp_path, monkeypatch) -> None: + (tmp_path / "src").mkdir() + (tmp_path / "src" / "app.py").write_text("def needle():\n", encoding="utf-8") + box = _rs_box(tmp_path, monkeypatch) + + matches, _ = box.grep(str(tmp_path), "needle") + assert [(os.path.basename(m.path), m.line_number) for m in matches] == [("app.py", 1)] + assert box.grep(str(tmp_path), "zzz_nothing") == ([], False) + found, _ = box.glob(str(tmp_path), "**/*.py") + assert [os.path.basename(path) for path in found] == ["app.py"] + assert box.glob(str(tmp_path), "*.md") == ([], False) diff --git a/backend/tests/test_remote_search.py b/backend/tests/test_remote_search.py new file mode 100644 index 000000000..a2652090f --- /dev/null +++ b/backend/tests/test_remote_search.py @@ -0,0 +1,212 @@ +"""Remote grep/glob command wrapper and stdout contract, including a real POSIX sh pipeline (#5376).""" + +from __future__ import annotations + +import os +import shlex +import shutil +import stat +import subprocess + +import pytest + +from deerflow.sandbox.remote_search import parse_remote_search_output, remote_search_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", +) +_REAL_GREP = pytest.mark.skipif(shutil.which("grep") is None, reason="system grep required") +_REAL_FIND = pytest.mark.skipif(shutil.which("find") is None, reason="system find required") + + +def _grep(root: str, pattern: str = "needle") -> str: + return f"grep -r -H -n -I -E -e {shlex.quote(pattern)} {shlex.quote(root)} 2>/dev/null" + + +def _find(root: str) -> str: + return f"find -H {shlex.quote(root)} \\( -type f \\) -print 2>/dev/null" + + +def _run(command: str, *, env: dict[str, str] | None = None) -> str: + # Providers invoke ``sh -lc``; tests use ``sh -c`` so an injected fake + # binary on PATH is not overwritten by a login profile. + proc = subprocess.run(["sh", "-c", command], capture_output=True, text=True, env=env, check=False) + # The wrapper always exits 0 so SDKs that raise on a non-zero exit still + # return the status marker. + assert proc.returncode == 0, proc.stderr + return proc.stdout + + +def _env_with_fake(tmp_path, name: str, script: str) -> dict[str, str]: + bin_dir = tmp_path / "bin" + bin_dir.mkdir(exist_ok=True) + fake = bin_dir / name + fake.write_text(script, encoding="utf-8") + fake.chmod(fake.stat().st_mode | stat.S_IEXEC) + env = os.environ.copy() + env["PATH"] = str(bin_dir) + os.pathsep + env.get("PATH", "") + return env + + +# ── parser ──────────────────────────────────────────────────────────────── + + +def test_parse_missing_root_marker_is_file_not_found() -> None: + with pytest.raises(FileNotFoundError): + parse_remote_search_output("__DF_SEARCH_STATUS__:missing\n", "/dir", tool="grep") + + +@pytest.mark.parametrize("stdout", ["", "/dir/a.py:1:needle\n"]) +def test_parse_without_marker_is_a_failure_not_a_result(stdout: str) -> None: + with pytest.raises(OSError, match="status marker missing"): + parse_remote_search_output(stdout, "/dir", tool="grep") + + +@pytest.mark.parametrize(("tool", "status"), [("grep", 0), ("grep", 141), ("find", 0), ("find", 141)]) +def test_parse_success_keeps_output_and_trailing_space(tool: str, status: int) -> None: + stdout = f"/dir/notes.txt \n\n__DF_SEARCH_STATUS__:{status}\n" + assert parse_remote_search_output(stdout, "/dir", tool=tool) == "/dir/notes.txt " + + +def test_parse_genuine_no_match_is_empty() -> None: + assert parse_remote_search_output("\n__DF_SEARCH_STATUS__:1\n", "/dir", tool="grep") == "" + assert parse_remote_search_output("\n__DF_SEARCH_STATUS__:0\n", "/dir", tool="find") == "" + + +@pytest.mark.parametrize(("tool", "status"), [("grep", 2), ("grep", 126), ("grep", 127), ("find", 1), ("find", 127)]) +def test_parse_failure_without_output_raises(tool: str, status: int) -> None: + with pytest.raises(OSError, match=f"exited with code {status}"): + parse_remote_search_output(f"\n__DF_SEARCH_STATUS__:{status}\n", "/dir", tool=tool) + + +@pytest.mark.parametrize(("tool", "status"), [("grep", 2), ("find", 1)]) +def test_parse_error_after_partial_output_still_raises(tool: str, status: int) -> None: + # An unreadable file or subdirectory leaves the result incomplete, and callers + # have no partial-result channel: it must not pass as a complete search, and + # the error must tell the agent how to recover. + with pytest.raises(OSError, match=f"exited with code {status}") as info: + parse_remote_search_output(f"/dir/a.py\n\n__DF_SEARCH_STATUS__:{status}\n", "/dir", tool=tool) + assert "could not be read" in str(info.value) + assert "narrower path" in str(info.value) + + +@pytest.mark.parametrize(("tool", "status"), [("grep", 126), ("grep", 127), ("find", 127)]) +def test_parse_other_failures_do_not_blame_unreadable_paths(tool: str, status: int) -> None: + with pytest.raises(OSError, match=f"exited with code {status}") as info: + parse_remote_search_output(f"\n__DF_SEARCH_STATUS__:{status}\n", "/dir", tool=tool) + assert "could not be read" not in str(info.value) + + +def test_parse_unparseable_status_is_a_failure() -> None: + with pytest.raises(OSError, match="status unavailable"): + parse_remote_search_output("\n__DF_SEARCH_STATUS__:\n", "/dir", tool="find") + + +def test_command_checks_root_first_and_records_status_after_head() -> None: + command = remote_search_command(_grep("/mnt/data dir"), "/mnt/data dir", limit=450) + assert command.startswith("set +e; ") + assert "[ ! -e '/mnt/data dir' ]" in command + assert command.index("[ ! -e ") < command.index("grep ") < command.index("head -n 450") + assert command.index("head -n 450") < command.rindex("__DF_SEARCH_STATUS__:") + assert command.endswith("exit 0") + + +# ── real POSIX sh ───────────────────────────────────────────────────────── + + +@_POSIX_SH +@_REAL_GREP +def test_naive_grep_head_pipeline_hides_a_missing_root(tmp_path) -> None: + """Reproduction of #5376: without the wrapper, head's 0 wins and stdout is empty.""" + proc = subprocess.run(["sh", "-c", _grep(str(tmp_path / "missing")) + " | head -450"], capture_output=True, text=True, check=False) + assert (proc.returncode, proc.stdout) == (0, "") + + +@_POSIX_SH +@_REAL_GREP +def test_grep_distinguishes_match_no_match_and_missing_root(tmp_path) -> None: + (tmp_path / "src").mkdir() + (tmp_path / "src" / "app.py").write_text("def needle():\n return 1\n", encoding="utf-8") + root = str(tmp_path) + + found = parse_remote_search_output(_run(remote_search_command(_grep(root), root, limit=450)), root, tool="grep") + assert found == f"{tmp_path / 'src' / 'app.py'}:1:def needle():" + none = parse_remote_search_output(_run(remote_search_command(_grep(root, "zzz_nothing"), root, limit=450)), root, tool="grep") + assert none == "" + + missing = str(tmp_path / "missing") + with pytest.raises(FileNotFoundError): + parse_remote_search_output(_run(remote_search_command(_grep(missing), missing, limit=450)), missing, tool="grep") + + +@_POSIX_SH +@pytest.mark.parametrize("binary", ["grep", "find"]) +def test_missing_search_binary_is_a_failure_not_a_no_match(tmp_path, binary: str) -> None: + env = _env_with_fake(tmp_path, binary, "#!/bin/sh\nexit 127\n") + root = str(tmp_path) + search, tool = (_grep(root), "grep") if binary == "grep" else (_find(root), "find") + with pytest.raises(OSError, match="exited with code 127"): + parse_remote_search_output(_run(remote_search_command(search, root, limit=450), env=env), root, tool=tool) + + +@_POSIX_SH +@_REAL_FIND +def test_find_follows_a_symlinked_root_and_reports_an_empty_tree(tmp_path) -> None: + real = tmp_path / "real" + (real / "sub").mkdir(parents=True) + (real / "sub" / "a.txt").write_text("x", encoding="utf-8") + link = tmp_path / "link" + link.symlink_to(real, target_is_directory=True) + out = parse_remote_search_output(_run(remote_search_command(_find(str(link)), str(link), limit=850)), str(link), tool="find") + assert out == f"{link}/sub/a.txt" + + empty = tmp_path / "empty" + empty.mkdir() + assert parse_remote_search_output(_run(remote_search_command(_find(str(empty)), str(empty), limit=850)), str(empty), tool="find") == "" + + +@_POSIX_SH +@_REAL_GREP +@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="root can read an unreadable directory") +def test_unreadable_root_is_a_failure_not_a_no_match(tmp_path) -> None: + locked = tmp_path / "locked" + locked.mkdir() + (locked / "a.txt").write_text("needle\n", encoding="utf-8") + locked.chmod(0) + try: + with pytest.raises(OSError, match="exited with code 2"): + parse_remote_search_output(_run(remote_search_command(_grep(str(locked)), str(locked), limit=450)), str(locked), tool="grep") + finally: + locked.chmod(0o700) + + +@_POSIX_SH +@_REAL_GREP +@_REAL_FIND +@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="root can read an unreadable directory") +def test_unreadable_subtree_is_a_failure_not_a_partial_result(tmp_path) -> None: + (tmp_path / "open").mkdir() + (tmp_path / "open" / "a.txt").write_text("needle\n", encoding="utf-8") + locked = tmp_path / "locked" + locked.mkdir() + (locked / "b.txt").write_text("needle\n", encoding="utf-8") + locked.chmod(0) + root = str(tmp_path) + try: + # Both searches print the readable match before failing on the locked subtree. + with pytest.raises(OSError, match="exited with code 2.*could not be read"): + parse_remote_search_output(_run(remote_search_command(_grep(root), root, limit=450)), root, tool="grep") + with pytest.raises(OSError, match="exited with code 1.*could not be read"): + parse_remote_search_output(_run(remote_search_command(_find(root), root, limit=850)), root, tool="find") + finally: + locked.chmod(0o700) + + +@_POSIX_SH +def test_head_truncation_is_not_a_failure(tmp_path) -> None: + env = _env_with_fake(tmp_path, "grep", '#!/bin/sh\ni=1\nwhile [ "$i" -le 5000 ]; do\n echo "/dir/f$i:1:needle"\n i=$((i+1))\ndone\nexit 0\n') + out = parse_remote_search_output(_run(remote_search_command(_grep("/"), "/", limit=450), env=env), "/", tool="grep") + lines = out.split("\n") + assert len(lines) == 450 + assert lines[0] == "/dir/f1:1:needle" diff --git a/backend/tests/test_tenki_provider.py b/backend/tests/test_tenki_provider.py index 1b0555ceb..df0c32da8 100644 --- a/backend/tests/test_tenki_provider.py +++ b/backend/tests/test_tenki_provider.py @@ -14,6 +14,8 @@ import logging import os import re import shlex +import shutil +import subprocess import sys import threading import time @@ -108,6 +110,13 @@ class _FakeFS: return _FakeFileInfo(path, len(self._owner.files[path])) +def _search_inner(script: str) -> str: + """The grep/find command inside ``remote_search_command``, or ``script`` itself.""" + if "__DF_SEARCH_STATUS__:" not in script: + return script + return script[script.index("{ ") + 2 : script.index('; echo $? > "$_st"; }')] + + class _FakeSandbox: """A fake tenki ``Sandbox``: a native ``fs`` API plus the handful of shell commands the adapter still emits for search, backed by one in-memory @@ -155,6 +164,8 @@ 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 "__DF_SEARCH_STATUS__:" in script: + return self._search(script) 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 "" @@ -178,6 +189,16 @@ class _FakeSandbox: return _FakeResult(stdout=("\n".join(lines) + "\n").encode() if lines else b"") return _FakeResult() + def _search(self, script: str) -> _FakeResult: + # remote_search_command: a root-existence check, then the wrapped search and its status marker. + root = shlex.split(re.search(r"\[ ! -e (.+?) \]; then", script).group(1))[0].rstrip("/") or "/" + if not any(p == root or p.startswith(f"{root}/") for p in (*self.files, *self.dirs)): + return _FakeResult(stdout=b"__DF_SEARCH_STATUS__:missing\n") + inner = _search_inner(script) + listing = self._run_script(inner).stdout_text + status = 0 if listing or inner.startswith("find ") else 1 + return _FakeResult(stdout=f"{listing}\n__DF_SEARCH_STATUS__:{status}\n".encode()) + def close(self): self.closed = True if self.close_error is not None: @@ -583,7 +604,7 @@ def test_grep_passes_capital_h_so_single_file_matches_parse() -> None: box = TenkiSandbox("sb", fake) box.write_file("/mnt/user-data/workspace/a.txt", "needle here\n") box.grep("/mnt/user-data/workspace", "needle") - grep_scripts = [c["argv"][2] for c in fake.exec_calls if c["argv"][:2] == ("sh", "-lc") and c["argv"][2].startswith("grep ")] + grep_scripts = [_search_inner(c["argv"][2]) for c in fake.exec_calls if c["argv"][:2] == ("sh", "-lc") and _search_inner(c["argv"][2]).startswith("grep ")] assert grep_scripts and "-H" in shlex.split(grep_scripts[0]) @@ -598,7 +619,7 @@ def test_grep_single_file_path_with_matching_glob() -> None: def _grep_script(fake: _FakeSandbox) -> list[str]: - scripts = [c["argv"][2] for c in fake.exec_calls if c["argv"][:2] == ("sh", "-lc") and c["argv"][2].startswith("grep ")] + scripts = [_search_inner(c["argv"][2]) for c in fake.exec_calls if c["argv"][:2] == ("sh", "-lc") and _search_inner(c["argv"][2]).startswith("grep ")] assert scripts, "no grep command was issued" return shlex.split(scripts[0]) @@ -626,8 +647,9 @@ def test_grep_case_sensitive_omits_ignore_case_flag() -> None: def test_glob_include_dirs_adds_directory_type_to_find() -> None: fake = _FakeSandbox() box = TenkiSandbox("sb", fake) + box.write_file("/mnt/user-data/workspace/a.txt", "x") # glob searches an existing root box.glob("/mnt/user-data/workspace", "*", include_dirs=True) - 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 = [_search_inner(c["argv"][2]) for c in fake.exec_calls if c["argv"][:2] == ("sh", "-lc") and _search_inner(c["argv"][2]).startswith("find ")] assert find_scripts and "-type d" in find_scripts[-1] # dirs requested, not just files @@ -1051,3 +1073,68 @@ 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 + + +# ── Remote grep/glob failure contract against a real POSIX sh (#5376) ───────── + +_RS_POSIX = pytest.mark.skipif( + os.name == "nt" or any(shutil.which(tool) is None for tool in ("sh", "head", "grep", "find")), + reason="POSIX sh, head, grep and find required", +) + + +def _rs_env(tmp_path, failing: str | None = None) -> dict[str, str]: + env = os.environ.copy() + if failing is not None: + bin_dir = tmp_path / "fake-bin" + bin_dir.mkdir() + fake = bin_dir / failing + fake.write_text("#!/bin/sh\nexit 127\n", encoding="utf-8") + fake.chmod(0o755) + env["PATH"] = str(bin_dir) + os.pathsep + env.get("PATH", "") + return env + + +def _rs_box(tmp_path, monkeypatch, failing: str | None = None) -> TenkiSandbox: + box = TenkiSandbox("sb", _FakeSandbox()) + shell_env = _rs_env(tmp_path, failing) + + def sh(script: str, env=None, timeout=None) -> _FakeResult: + # ``sh -c`` (not ``-lc``) keeps a login profile from overriding the fake PATH. + proc = subprocess.run(["sh", "-c", script], capture_output=True, text=True, env=shell_env, check=False) + return _FakeResult(exit_code=proc.returncode, stdout=proc.stdout.encode(), stderr=proc.stderr.encode()) + + monkeypatch.setattr(box, "_sh", sh) + return box + + +def _rs_search(box, op: str, root: str): + return box.grep(root, "needle") if op == "grep" else box.glob(root, "**/*.py") + + +@_RS_POSIX +@pytest.mark.parametrize("op", ["grep", "glob"]) +def test_remote_search_missing_root_raises_file_not_found(tmp_path, monkeypatch, op) -> None: + with pytest.raises(FileNotFoundError): + _rs_search(_rs_box(tmp_path, monkeypatch), op, str(tmp_path / "missing")) + + +@_RS_POSIX +@pytest.mark.parametrize(("op", "binary"), [("grep", "grep"), ("glob", "find")]) +def test_remote_search_missing_binary_raises_instead_of_no_matches(tmp_path, monkeypatch, op, binary) -> None: + with pytest.raises(OSError, match="exited with code 127"): + _rs_search(_rs_box(tmp_path, monkeypatch, failing=binary), op, str(tmp_path)) + + +@_RS_POSIX +def test_remote_search_keeps_real_matches_and_genuine_no_match(tmp_path, monkeypatch) -> None: + (tmp_path / "src").mkdir() + (tmp_path / "src" / "app.py").write_text("def needle():\n", encoding="utf-8") + box = _rs_box(tmp_path, monkeypatch) + + matches, _ = box.grep(str(tmp_path), "needle") + assert [(os.path.basename(m.path), m.line_number) for m in matches] == [("app.py", 1)] + assert box.grep(str(tmp_path), "zzz_nothing") == ([], False) + found, _ = box.glob(str(tmp_path), "**/*.py") + assert [os.path.basename(path) for path in found] == ["app.py"] + assert box.glob(str(tmp_path), "*.md") == ([], False)