wutongyuonce d8ed8160c9
fix(sandbox): stop list_dir from reporting failures as empty (#5264)
* fix(sandbox): stop list_dir from reporting failures as empty

Remote providers swallowed find/client errors as [] and 2>/dev/null
missing paths as empty stdout. ls_tool then told the agent the
directory was (empty). Raise OSError/FileNotFoundError instead so
the tool returns Error.

* fix(sandbox): list_dir raises on missing local paths and uses find -H

Empty stdout is not a missing path when find's start point is a
symlink (E2B /mnt/acp-workspace). Dereference only the start point
with find -H. LocalSandbox now raises FileNotFoundError for a
non-directory root, matching remote providers. AIO maps a missing
result.data to OSError rather than FileNotFoundError.

* fix(sandbox): group AIO list_dir find type predicates

Without parentheses, find PATH -maxdepth N -type f -o -type d applies
-type d without maxdepth and can drop files from the listing.

* fix(sandbox): distinguish list_dir command failure from missing path

Tenki, Boxlite, and OpenSandbox treated any empty find stdout as
FileNotFoundError, so a missing find binary (exit 127) or SDK error
looked like a missing directory. Raise OSError when find status is
outside (0, 1); keep FileNotFoundError for the find-ran-but-empty case.

* fix(sandbox): apply list_dir exit-status contract to AIO and E2B

Same gap as Tenki/Boxlite/OpenSandbox: empty find stdout with exit 127
was FileNotFoundError. Raise OSError when the status is outside (0, 1).

* fix(sandbox): classify list_dir by find status not head status

find | head under sh -lc reports head's exit code, so a missing find
binary (127) became FileNotFoundError. Record find's own status after
the bounded listing, treat SIGPIPE 141 as truncation success, and add
a shell-level regression test.

* test(auth): include projects permissions in /me contract pins

#5265 added projects:read/write/delete to the registered route set.
The /auth/me tests still pinned the pre-projects list, so CI failed
after merging main.

* fix(sandbox): do not treat missing list_dir marker as success

The generated script ended on `rm -f`, so process status was 0/1 even
when find's marker never landed. Both codes are in _FIND_OK, and the
parser fallback then classified an empty listing as FileNotFoundError —
the 127 misclassification this helper was meant to close.

Exit with find's status (126 if unknown). A missing marker is now
OSError unless the process status is already a non-OK failure.

* test(sandbox): emit list_dir status marker in provider fixtures

Parser now requires __DF_FIND_STATUS__ and refuses marker-less stdout.
Update AIO/Boxlite/E2B stubs and OpenSandbox/Tenki find fakes so listings
carry :0 and missing paths carry :1 with matching exit codes.

* style(sandbox): format list dir test fixture

* style(sandbox): format remote list dir helper

* docs(sandbox): keep guidance within the tested size budget

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-09-09 10:12:45 +08:00

229 lines
9.0 KiB
Python

import re
from abc import ABC, abstractmethod
from deerflow.sandbox.search import GrepMatch
# POSIX env-var name rule: letter or underscore, then letters/digits/underscores.
# Used to validate ``env`` keys before they reach a sandbox implementation.
# No current implementation splices a key into a shell string — the local
# sandbox passes the dict to ``subprocess.run(env=...)`` (no shell), the AIO
# sandbox forwards it via the ``bash.exec`` structured ``env`` field, and e2b
# forwards it as the SDK's ``envs``. The check is defense-in-depth for the
# contract: a future shell-splicing implementation must not have to re-derive
# its own rule.
_ENV_NAME_PATTERN = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
def _validate_extra_env(extra_env: dict[str, str] | None) -> None:
"""Reject ``env`` keys that are not valid POSIX env-var names.
The :meth:`Sandbox.execute_command` contract accepts arbitrary ``str``
keys. Today no implementation splices a key into a shell string — the
local sandbox passes the dict to ``subprocess.run(env=...)`` (no shell),
the AIO sandbox forwards it via the ``bash.exec`` structured ``env``
field (no command-string splice), and e2b forwards it as the SDK's
``envs``. Enforcing the POSIX env-name rule in the abstract layer is
defense-in-depth for the contract: a future implementation that does
route a key through a shell must not have to re-derive its own
validation rule, and a caller passing a key derived from config /
payload / user input fails fast with ``ValueError`` instead of silently
producing an exploit should a future implementation regress to splicing.
Raises:
ValueError: When ``extra_env`` is not None and any key does not
match ``^[A-Za-z_][A-Za-z0-9_]*$``. ``None`` and empty dicts
pass through unchanged.
"""
if not extra_env:
return
for key in extra_env:
if not isinstance(key, str) or not _ENV_NAME_PATTERN.fullmatch(key):
raise ValueError(f"extra_env key {key!r} is not a valid POSIX environment variable name (must match ^[A-Za-z_][A-Za-z0-9_]*$). This protects shell-using sandbox implementations from command injection via the key.")
class Sandbox(ABC):
"""Abstract base class for sandbox environments"""
_id: str
#: Whether ``execute_command`` reuses one persistent shell session across
#: calls (shell state — exports, cwd, functions — survives from one call
#: into the next). When True, a recorded command's environment cannot be
#: proven clean from the command text alone, so evidence consumers (the
#: acceptance checklist's ``tests_passed`` matcher) must treat recorded
#: bash evidence as untrusted and degrade to UNVERIFIED.
#:
#: Tri-state, failing closed: ``None`` (the default) means the
#: implementation has NOT declared its session semantics — custom
#: providers are loaded by class path and may reuse a persistent
#: session, so silence cannot be read as fresh-shell. Consumers must
#: trust only an explicit ``False`` and degrade to UNVERIFIED on
#: ``None`` exactly as on ``True``. Every shipped implementation
#: declares explicitly (AIO: ``True``; the per-call exec providers:
#: ``False``).
persistent_shell_sessions: bool | None = None
def __init__(self, id: str):
self._id = id
@property
def id(self) -> str:
return self._id
@abstractmethod
def execute_command(
self,
command: str,
env: dict[str, str] | None = None,
timeout: float | None = None,
) -> str:
"""Execute bash command in sandbox.
Args:
command: The command to execute.
env: Optional per-call environment variables to inject into the
command's process. Used to pass request-scoped secrets (e.g. a
short-lived end-user token for skill scripts, issue #3861, or a
GitHub App installation token for ``git push`` / ``gh``) without
placing them in the prompt, tool arguments, or the command
string. When ``None`` the sandbox uses its default environment.
Keys must be valid POSIX environment-variable names
(``^[A-Za-z_][A-Za-z0-9_]*$``); implementations validate
via :func:`_validate_extra_env` before use. Values are
arbitrary strings — shell-using implementations
``shlex.quote`` them on splice.
timeout: Optional per-call wall-clock timeout in seconds. Local
sandboxes use this to bound host bash commands so long-lived
foreground processes cannot hang a turn indefinitely. Remote/AIO
implementations may ignore it when their backend does not expose
an equivalent command-timeout control separate from its own API
timeouts.
Returns:
The standard or error output of the command.
Raises:
ValueError: when an ``env`` key is not a valid env-var name.
"""
pass
def execute_command_in_scope(
self,
command: str,
env: dict[str, str] | None = None,
timeout: float | None = None,
*,
scope_id: str | None = None,
) -> str:
"""Execute a command in an optional agent execution scope.
Providers without server-side shell sessions inherit the ordinary
command behavior. Session-aware providers may isolate concurrent agent
executions while preserving serialization inside one scope.
"""
del scope_id
return self.execute_command(command, env=env, timeout=timeout)
def release_command_scope(self, scope_id: str) -> None:
"""Release provider-specific command state for one execution scope."""
del scope_id
@abstractmethod
def read_file(
self,
path: str,
start_line: int | None = None,
end_line: int | None = None,
) -> str:
"""Read the content of a file.
Args:
path: The absolute path of the file to read.
start_line: Optional starting line number (1-indexed, inclusive).
end_line: Optional ending line number (1-indexed, inclusive).
Returns:
The content of the file.
"""
pass
@abstractmethod
def download_file(self, path: str) -> bytes:
"""Download the binary content of a file.
Args:
path: The absolute path of the file to download.
Returns:
Raw file bytes.
Raises:
PermissionError: If path traversal is detected or the path is outside
the allowed virtual prefix.
OSError: If the file cannot be read or does not exist. Both local
and remote implementations must raise ``OSError`` so callers
have a single exception type to handle.
"""
pass
@abstractmethod
def list_dir(self, path: str, max_depth=2) -> list[str]:
"""List the contents of a directory.
Args:
path: The absolute path of the directory to list.
max_depth: The maximum depth to traverse. Default is 2.
Returns:
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
@abstractmethod
def write_file(self, path: str, content: str, append: bool = False) -> None:
"""Write content to a file.
Args:
path: The absolute path of the file to write to.
content: The text content to write to the file.
append: Whether to append the content to the file. If False, the file will be created or overwritten.
"""
pass
@abstractmethod
def glob(self, path: str, pattern: str, *, include_dirs: bool = False, max_results: int = 200) -> tuple[list[str], bool]:
"""Find paths that match a glob pattern under a root directory."""
pass
@abstractmethod
def grep(
self,
path: str,
pattern: str,
*,
glob: str | None = None,
literal: bool = False,
case_sensitive: bool = False,
max_results: int = 100,
) -> tuple[list[GrepMatch], bool]:
"""Search for matches inside a text file or files under a directory."""
pass
@abstractmethod
def update_file(self, path: str, content: bytes) -> None:
"""Update a file with binary content.
Args:
path: The absolute path of the file to update.
content: The binary content to write to the file.
"""
pass