From 6a6aeb4bf2d2ce9c31cee4bb916ec2f1236995a6 Mon Sep 17 00:00:00 2001 From: Hyeonsang Cho Date: Fri, 16 Oct 2026 18:18:17 +0900 Subject: [PATCH] fix(sandbox): reject shell-variable path prefixes in the host-bash guard (#6930) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(sandbox): reject shell-variable path prefixes in the host-bash guard The local path guard scans commands for absolute paths with a regex whose lookbehind skips a `/` that follows a word character. `~/`, `${HOME}/` and `"$HOME"/` were rejected, but the bare `$HOME/...` form (quoted or not) was never matched, so `cat $HOME/.ssh/id_rsa` ran on the host when `sandbox.allow_host_bash` was enabled. Scan `$NAME/...` separately and reject it, since the variable's value is unknown at validation time. Variables inside URLs and non-ASCII prose keep the existing exemptions. `$PWD/...` stays allowed because it names the same file as the equivalent relative path, which the guard already accepts, unless the command rebinds `PWD` itself. * docs(changelog): reference #6930 for the host-bash variable path fix * fix(sandbox): cover positional parameters and CDPATH in the host-bash guard Review follow-up. `cat $1/etc/passwd` slipped both scans: the new variable scan only matched names, and the absolute-path regex skips a `/` after a digit. `bash -c` gets no positionals, so `$1` expands to nothing and the shell opens `/etc/passwd`. Match `$0`-`$9` as well, but keep awk-style arithmetic such as `$1/1024` allowed, since it was accepted before and standard host roots do not start with a digit. `CDPATH=$HOME cd docs` moved a relative `cd` outside the workspace, taking both `$PWD/x` and a plain relative `x` with it. Reject any command that mentions CDPATH in the working-directory check, so the `$PWD` exemption's equivalence with relative paths holds. * fix(sandbox): cover named homes and non-ASCII variable paths in the host-bash guard - Reject `~name/...`, which expands to that user's home and slips the absolute-path scan the same way `$HOME/...` did. - Stop applying the non-ASCII prose exemption to the variable scan: `$HOME/文档/...` is a host path. - Allow awk field division `$1/$2`, which main accepts, and capture the remainder in a lookahead so skipping `$1/` does not hide a later `$2/etc/passwd`. --- CHANGELOG.md | 15 ++ CHANGELOG_zh.md | 11 ++ .../harness/deerflow/sandbox/tools.py | 50 ++++++ backend/tests/test_sandbox_tools_security.py | 145 ++++++++++++++++++ 4 files changed, 221 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c664ae43cd..0f34a2adbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5968,6 +5968,20 @@ This release closes that milestone with **621 merged pull requests**. whose head stamp was lost logged 29 (SQLite) or 36 (Postgres), burying the real type mismatch the warning exists to catch. Both sides are now compared as the connected database spells them. ([#6865]) +- **sandbox:** With host bash enabled, the local path guard now rejects a + shell variable used as a path prefix, such as `cat $HOME/.ssh/id_rsa`. + `~/`, `${HOME}/` and `"$HOME"/` were already rejected, but the + absolute-path scan skips a `/` that follows a word character, so the + bare `$HOME/...` form (quoted or not) reached the host. Positional + parameters are covered too: `bash -c` gets none, so `$1/etc/passwd` + opens `/etc/passwd`; awk-style `$1/1024` and `$1/$2` stay allowed. A + named home such as `~root/.ssh/id_rsa` is rejected the same way, and a + non-ASCII segment after a variable (`$HOME/文档`) is treated as a path, + not prose. Commands that mention `CDPATH` are rejected, because it moves + a relative `cd` outside the workspace. `$PWD/...` stays allowed because it + names the same file as the equivalent relative path, unless the command + rebinds `PWD` itself. Variables inside URLs are unaffected. The guard + remains best-effort and is not an isolation boundary. ([#6930]) ### Security @@ -11763,3 +11777,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#6876]: https://github.com/bytedance/deer-flow/pull/6876 [#6897]: https://github.com/bytedance/deer-flow/pull/6897 [#6909]: https://github.com/bytedance/deer-flow/pull/6909 +[#6930]: https://github.com/bytedance/deer-flow/pull/6930 diff --git a/CHANGELOG_zh.md b/CHANGELOG_zh.md index bdaa5213ef..1d388532ce 100644 --- a/CHANGELOG_zh.md +++ b/CHANGELOG_zh.md @@ -4528,6 +4528,16 @@ 数据库升级时都会记录一条，丢失 head stamp 的 schema 会记录 29 条（SQLite）或 36 条（Postgres），淹没了这个警告本应发现的真实类型不一致。现在两侧都按所连接 数据库的写法进行比较。([#6865]) +- **沙箱：** 启用 host bash 时，本地路径守卫现在会拒绝把 shell 变量用作路径前缀的 + 命令，例如 `cat $HOME/.ssh/id_rsa`。`~/`、`${HOME}/` 和 `"$HOME"/` 原本就会被拒绝， + 但绝对路径扫描会跳过紧跟在单词字符之后的 `/`，因此不带花括号的 `$HOME/...` + （无论是否加引号）会直达主机。位置参数同样会被检查：`bash -c` 不带位置参数， + 因此 `$1/etc/passwd` 会打开 `/etc/passwd`；awk 风格的 `$1/1024` 和 `$1/$2` 仍然 + 允许。`~root/.ssh/id_rsa` 这类指定用户的主目录同样会被拒绝，变量之后的非 ASCII + 路径段（`$HOME/文档`）按路径处理，不再视为普通文本。提及 `CDPATH` 的命令会被拒绝， + 因为它会把相对路径的 `cd` 移出工作区。`$PWD/...` 仍然允许，因为它与对应的相对路径 + 指向同一文件，除非命令自己重新绑定了 `PWD`。URL 中的变量不受影响。该守卫仍是尽力 + 而为的检查，并非隔离边界。([#6930]) ### 安全 @@ -9380,3 +9390,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写，核心包含子 [#6876]: https://github.com/bytedance/deer-flow/pull/6876 [#6897]: https://github.com/bytedance/deer-flow/pull/6897 [#6909]: https://github.com/bytedance/deer-flow/pull/6909 +[#6930]: https://github.com/bytedance/deer-flow/pull/6930 diff --git a/backend/packages/harness/deerflow/sandbox/tools.py b/backend/packages/harness/deerflow/sandbox/tools.py index b963dcc3a7..b5c6a4ed9d 100644 --- a/backend/packages/harness/deerflow/sandbox/tools.py +++ b/backend/packages/harness/deerflow/sandbox/tools.py @@ -69,6 +69,25 @@ _SANDBOX_AUTHORIZATION_CHECKED: ContextVar[bool] = ContextVar( ) _ABSOLUTE_PATH_PATTERN = re.compile(r"(?()]+)") +# ``$NAME/...`` expands to a host path at runtime (``$HOME/.ssh/id_rsa``), but +# the absolute-path regex skips a ``/`` that follows a word character, so the +# bare-name form needs its own scan. Positional ``$1/etc/passwd`` slips the +# same way, and ``bash -c`` gets no positionals, so it expands to ``/etc/passwd``. +# ``${NAME}/``, ``~/`` and special parameters (``$@/``, ``$?/``) already surface +# through the absolute-path regex because ``}``, ``~``, ``@`` and ``?`` are not +# word chars. The remainder is captured in a lookahead so a skipped prefix does +# not hide a later one (``$1/$2/etc/passwd``). +_SHELL_VARIABLE_PATH_PATTERN = re.compile(r"\$([A-Za-z_][A-Za-z0-9_]*|[0-9])/(?=([^\s\"'`;&|<>()]*))") +# ``~name/`` expands to that user's home (``~root/.ssh/id_rsa``) and slips the +# absolute-path regex for the same reason. Tilde expansion only applies at the +# start of a word or after ``=``/``:``, so ``HEAD~1/`` stays out of scope. +_NAMED_TILDE_PATH_PATTERN = re.compile(r"(?()\"'`])~[A-Za-z0-9._-]+/[^\s\"'`;&|<>()]*") +# ``PWD`` written anywhere except as a ``$PWD`` expansion (``PWD=/``, +# ``read PWD``, ``for PWD in``) may rebind it away from the workspace. +_PWD_REBIND_PATTERN = re.compile(r"(? None: + with pytest.raises(PermissionError, match="Unsafe absolute paths") as exc_info: + validate_local_bash_command_paths(command, _THREAD_DATA) + + assert rejected in str(exc_info.value) + + +@pytest.mark.parametrize( + "command", + [ + # Variables that are not used as a path prefix stay allowed. + "echo $HOME", + "echo $PATH", + "echo $((total/2))", + # A variable inside a URL is a URL segment, not a host path. + "curl https://api.github.com/repos/$OWNER/$REPO/issues", + "curl https://example.com/~alice/index.html", + # ``~`` mid-word is not tilde expansion. + "cat drafts~v2/notes.md", + # ``$PWD/x`` names the same file as the relative ``x``, which the guard + # already accepts (``$PWD/../x`` is rejected like ``../x``). + "cat $PWD/notes.txt", + 'export PATH="$PWD/.venv/bin:$PATH"', + "cd /mnt/user-data/uploads && cat $PWD/report.pdf", + # awk field arithmetic is not a path. + "du -k /mnt/user-data/workspace | awk '{print $1/1024}'", + "awk -F, '{print $1/$2}' /mnt/user-data/uploads/data.csv", + ], +) +def test_validate_local_bash_command_paths_allows_non_path_variable_uses(command: str) -> None: + validate_local_bash_command_paths(command, _THREAD_DATA) + + +@pytest.mark.parametrize( + "command", + [ + # CDPATH relocates a relative ``cd`` outside the workspace, taking both + # ``$PWD`` and plain relative paths with it. + "CDPATH=$HOME cd docs && cat $PWD/x", + "CDPATH=/ cd etc && cat passwd", + "export CDPATH=/; cd etc; cat passwd", + ], +) +def test_validate_local_bash_command_paths_rejects_cdpath_relocation(command: str) -> None: + with pytest.raises(PermissionError, match="Unsafe working directory change via CDPATH"): + validate_local_bash_command_paths(command, _THREAD_DATA) + + +def test_validate_local_bash_command_paths_rejects_pwd_traversal_like_relative_traversal() -> None: + for command in ("cat ../../etc/passwd", "cat $PWD/../../etc/passwd"): + with pytest.raises(PermissionError, match="path traversal"): + validate_local_bash_command_paths(command, _THREAD_DATA) + + +def test_bash_tool_rejects_variable_prefixed_host_path_before_execution(monkeypatch) -> None: + runtime = SimpleNamespace( + state={"sandbox": {"sandbox_id": "local"}, "thread_data": _THREAD_DATA.copy()}, + context={"thread_id": "thread-1"}, + ) + monkeypatch.setattr( + "deerflow.sandbox.tools.ensure_sandbox_initialized", + lambda runtime: SimpleNamespace(execute_command=lambda command: pytest.fail("unsafe command should not execute")), + ) + monkeypatch.setattr("deerflow.sandbox.tools.ensure_thread_directories_exist", lambda runtime: None) + monkeypatch.setattr("deerflow.sandbox.tools.is_host_bash_allowed", lambda: True) + + result = bash_tool.func(runtime=runtime, description="read key", command="cat $HOME/.ssh/id_rsa") + + assert "Unsafe absolute paths in command: $HOME/.ssh/id_rsa" in result + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX shell expands $HOME") +@pytest.mark.parametrize( + ("command", "rejected"), + [ + ('cat "$HOME/.ssh/id_rsa"', "Unsafe absolute paths in command: $HOME/.ssh/id_rsa"), + ('CDPATH="$HOME" cd .ssh && cat $PWD/id_rsa', "Unsafe working directory change via CDPATH"), + ('cat "$HOME/文档/id_rsa"', "Unsafe absolute paths in command: $HOME/文档/id_rsa"), + ], +) +def test_local_bash_tool_does_not_read_variable_prefixed_host_file(tmp_path: Path, monkeypatch, command: str, rejected: str) -> None: + """End to end through a real ``LocalSandbox``: before the fix the shell + expanded ``$HOME`` and printed the host file.""" + home = tmp_path / "home" + (home / ".ssh").mkdir(parents=True) + (home / ".ssh" / "id_rsa").write_text("host-private-key") + (home / "文档").mkdir() + (home / "文档" / "id_rsa").write_text("host-private-key") + monkeypatch.setenv("HOME", str(home)) + thread_root = tmp_path / "threads" / "t1" / "user-data" + thread_data = {f"{name}_path": str(thread_root / name) for name in ("workspace", "uploads", "outputs")} + for path in thread_data.values(): + Path(path).mkdir(parents=True) + workspace = str(Path(thread_data["workspace_path"]).resolve()) + sandbox = LocalSandbox( + id="local", + path_mappings=[PathMapping(container_path=f"{VIRTUAL_PATH_PREFIX}/workspace", local_path=workspace)], + ) + runtime = SimpleNamespace( + state={"sandbox": {"sandbox_id": "local"}, "thread_data": thread_data}, + context={"thread_id": "t1"}, + config={}, + ) + monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: sandbox) + monkeypatch.setattr("deerflow.sandbox.tools.is_host_bash_allowed", lambda: True) + + result = bash_tool.func(runtime=runtime, description="read key", command=command) + + assert "host-private-key" not in result + assert rejected in result + + def test_bash_tool_rejects_host_bash_when_local_sandbox_default(monkeypatch) -> None: runtime = SimpleNamespace( state={"sandbox": {"sandbox_id": "local"}, "thread_data": _THREAD_DATA.copy()},