From d3382d7e579c70dabd001c3eaaf89203c75d48d2 Mon Sep 17 00:00:00 2001 From: yorkhellen Date: Thu, 13 Aug 2026 02:42:51 +0900 Subject: [PATCH] fix(exec): guard bare and named-user home paths Co-authored-by: Xubin Ren <52506698+Re-bin@users.noreply.github.com> --- nanobot/agent/tools/shell.py | 60 +++++++++++++++++----- tests/tools/test_tool_validation.py | 77 ++++++++++++++++++++++++++++- 2 files changed, 124 insertions(+), 13 deletions(-) diff --git a/nanobot/agent/tools/shell.py b/nanobot/agent/tools/shell.py index b19c0de86..001be5897 100644 --- a/nanobot/agent/tools/shell.py +++ b/nanobot/agent/tools/shell.py @@ -890,12 +890,27 @@ class ExecTool(Tool): for raw in self._extract_absolute_paths(cmd): try: expanded = os.path.expandvars(raw.strip()) + # Python's expanduser() intentionally does not implement + # shell directory-stack forms. ``~+`` is the active cwd, + # while ``~-`` and indexed forms can resolve outside it; + # normalize the former and fail closed on the latter. + if expanded == "~+": + p = cwd_path + elif expanded.startswith("~+/"): + p = (cwd_path / expanded[3:]).resolve() + elif re.match(r"^~(?:-|[+-]\d+)(?:/|$)", expanded): + return ToolResult.error( + "Error: Command blocked by safety guard " + "(path outside working dir)" + + _WORKSPACE_BOUNDARY_NOTE + ) + else: + p = Path(expanded).expanduser().resolve() # Match against the un-resolved path first. On Linux, # /dev/stderr is a symlink to /proc/self/fd/2 and # ``Path.resolve()`` would mask the device-file intent. if self._is_benign_device_path(expanded): continue - p = Path(expanded).expanduser().resolve() except Exception: continue @@ -1043,8 +1058,10 @@ class ExecTool(Tool): ``shlex`` separates real grouping/redirection operators while preserving parentheses and spaces that were quoted or escaped as part of a path. Embedded scripts (for example ``sh -c \"cat /tmp/x\"``) still need a - small boundary scan. Colons are deliberately not boundaries: treating - them as such misclassifies URLs, ``host:/remote`` and ``C:/Windows``. + small boundary scan. Colons are not general boundaries: treating them + as such misclassifies URLs, ``host:/remote`` and ``C:/Windows``. They + are considered only inside a syntactically valid assignment, where + shells expand each colon-delimited tilde component. """ paths: list[str] = [] for match in re.finditer( @@ -1065,25 +1082,42 @@ class ExecTool(Tool): i = 0 while i < len(token): is_posix = token[i] == "/" - is_home = token.startswith("~/", i) or token.startswith("~+/", i) + home_match = re.match( + r"~(?:[+-](?:\d+)?|[A-Za-z0-9_.@-]+)?(?=/|:|$)", + token[i:], + ) + is_home = home_match is not None if not is_posix and not is_home: i += 1 continue prefix = token[:i] - at_boundary = i == 0 or token[i - 1] in boundary_chars parameter_default = ( i >= 2 and token[i - 2] == ":" and token[i - 1] in "-+?=" ) - if not at_boundary and not parameter_default: - i += 1 - continue - word_start = max( (prefix.rfind(char) for char in " \t\r\n<>|;&"), default=-1, ) + 1 word_prefix = prefix[word_start:] + assignment_component = bool( + re.fullmatch( + r"(?:[A-Za-z_][A-Za-z0-9_]*|--?[A-Za-z0-9_.-]+)=" + r"(?:[^:=\s]*:)*", + word_prefix, + ) + ) + at_boundary = i == 0 or token[i - 1] in boundary_chars + if is_home: + # A shell word beginning with ``~`` is a separate shlex token. + # Mid-token expansion is valid only after ``=`` or a colon in + # an assignment. This avoids PromQL/Loki ``=~`` and ``|~`` + # match operators while covering PATH-like values. + at_boundary = i == 0 or assignment_component + if not at_boundary and not parameter_default: + i += 1 + continue + if re.search(r"[A-Za-z][A-Za-z0-9+.-]*://", word_prefix) or re.match( r"(?:[^/:=\s]+@)?[^/:=\s]+:$", word_prefix, @@ -1095,11 +1129,13 @@ class ExecTool(Tool): i += 1 continue - assignment_value = i > 0 and token[i - 1] == "=" and not any( - char.isspace() for char in prefix - ) + assignment_value = assignment_component if i == 0 or assignment_value: end = len(token) + if assignment_value: + separator = token.find(":", i) + if separator >= 0: + end = separator elif token[i - 1] in {"'", '"'}: quote = token[i - 1] closing = token.find(quote, i) diff --git a/tests/tools/test_tool_validation.py b/tests/tools/test_tool_validation.py index f461ccf3b..68c1908fa 100644 --- a/tests/tools/test_tool_validation.py +++ b/tests/tools/test_tool_validation.py @@ -290,10 +290,11 @@ def test_exec_extract_absolute_paths_captures_home_paths() -> None: def test_exec_extract_absolute_paths_captures_paths_after_equals() -> None: - cmd = "curl --output=/etc/passwd --config=~/.nanobot/config.json" + cmd = "curl --output=/etc/passwd --config=~/.nanobot/config.json --user-home=~root" paths = ExecTool._extract_absolute_paths(cmd) assert "/etc/passwd" in paths assert "~/.nanobot/config.json" in paths + assert "~root" in paths def test_exec_extract_absolute_paths_does_not_capture_query_tilde() -> None: @@ -302,6 +303,29 @@ def test_exec_extract_absolute_paths_does_not_capture_query_tilde() -> None: assert not any(p.startswith("~") for p in paths) +def test_exec_extract_absolute_paths_captures_bare_and_named_user_home_paths() -> None: + paths = ExecTool._extract_absolute_paths("cd ~ && cat ~root/.bashrc") + assert "~" in paths + assert "~root/.bashrc" in paths + + +def test_exec_extract_absolute_paths_captures_tilde_after_shell_operators() -> None: + paths = ExecTool._extract_absolute_paths( + "cat <~root/.bashrc;~root/bin/tool|~daemon/bin/tool" + ) + assert "~root/.bashrc" in paths + assert paths.count("~root/bin/tool") == 1 + assert "~daemon/bin/tool" in paths + + +def test_exec_extract_absolute_paths_captures_tilde_assignment_components() -> None: + paths = ExecTool._extract_absolute_paths( + "HOME=~ PATH=bin:~root/bin curl --config=~" + ) + assert "~" in paths + assert "~root/bin" in paths + + def test_exec_extract_absolute_paths_captures_quoted_paths() -> None: cmd = 'cat "/tmp/data.txt" "~/.nanobot/config.json"' paths = ExecTool._extract_absolute_paths(cmd) @@ -319,6 +343,48 @@ def test_exec_guard_blocks_home_path_outside_workspace(tmp_path) -> None: assert "hard policy boundary" in error +def test_exec_guard_blocks_bare_tilde_cwd_escape(tmp_path) -> None: + tool = ExecTool(restrict_to_workspace=True) + error = tool._guard_command("cd ~ && cat secret.txt", str(tmp_path)) + assert error is not None + assert error.startswith( + "Error: Command blocked by safety guard (path outside working dir)" + ) + + +def test_exec_guard_blocks_named_user_home_path(tmp_path) -> None: + tool = ExecTool(restrict_to_workspace=True) + error = tool._guard_command("cat ~root/.bashrc", str(tmp_path)) + assert error is not None + assert error.startswith( + "Error: Command blocked by safety guard (path outside working dir)" + ) + + +@pytest.mark.parametrize( + "command", + [ + "cat <~root/.bashrc", + "cat ~-/.bashrc", + "cat ~+1/.bashrc", + "cat ~-1/.bashrc", + ], +) +def test_exec_guard_blocks_home_paths_with_special_shell_contexts( + tmp_path, command: str +) -> None: + error = ExecTool(restrict_to_workspace=True)._guard_command(command, str(tmp_path)) + assert error is not None + assert error.startswith( + "Error: Command blocked by safety guard (path outside working dir)" + ) + + +def test_exec_guard_allows_current_directory_tilde(tmp_path) -> None: + tool = ExecTool(restrict_to_workspace=True) + assert tool._guard_command("cat ~+/file.txt", str(tmp_path)) is None + + def test_exec_guard_blocks_equals_home_path_outside_workspace(tmp_path) -> None: tool = ExecTool(restrict_to_workspace=True) error = tool._guard_command("cat --config=~/.nanobot/config.json", str(tmp_path)) @@ -328,6 +394,15 @@ def test_exec_guard_blocks_equals_home_path_outside_workspace(tmp_path) -> None: ) +def test_exec_guard_blocks_equals_named_user_home_path(tmp_path) -> None: + tool = ExecTool(restrict_to_workspace=True) + error = tool._guard_command("cat --config=~root/.bashrc", str(tmp_path)) + assert error is not None + assert error.startswith( + "Error: Command blocked by safety guard (path outside working dir)" + ) + + def test_exec_guard_blocks_quoted_home_path_outside_workspace(tmp_path) -> None: tool = ExecTool(restrict_to_workspace=True) error = tool._guard_command('cat "~/.nanobot/config.json"', str(tmp_path))