mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-04-30 23:05:51 +00:00
fix(shell): reject LLM-supplied working_dir outside workspace (#2826)
This commit is contained in:
parent
00fb491bc9
commit
3f59bd1443
@ -101,6 +101,21 @@ class ExecTool(Tool):
|
|||||||
timeout: int | None = None, **kwargs: Any,
|
timeout: int | None = None, **kwargs: Any,
|
||||||
) -> str:
|
) -> str:
|
||||||
cwd = working_dir or self.working_dir or os.getcwd()
|
cwd = working_dir or self.working_dir or os.getcwd()
|
||||||
|
|
||||||
|
# Prevent an LLM-supplied working_dir from escaping the configured
|
||||||
|
# workspace when restrict_to_workspace is enabled (#2826). Without
|
||||||
|
# this, a caller can pass working_dir="/etc" and then all absolute
|
||||||
|
# paths under /etc would pass the _guard_command check that anchors
|
||||||
|
# on cwd.
|
||||||
|
if self.restrict_to_workspace and self.working_dir:
|
||||||
|
try:
|
||||||
|
requested = Path(cwd).expanduser().resolve()
|
||||||
|
workspace_root = Path(self.working_dir).expanduser().resolve()
|
||||||
|
except Exception:
|
||||||
|
return "Error: working_dir could not be resolved"
|
||||||
|
if requested != workspace_root and workspace_root not in requested.parents:
|
||||||
|
return "Error: working_dir is outside the configured workspace"
|
||||||
|
|
||||||
guard_error = self._guard_command(command, cwd)
|
guard_error = self._guard_command(command, cwd)
|
||||||
if guard_error:
|
if guard_error:
|
||||||
return guard_error
|
return guard_error
|
||||||
|
|||||||
@ -113,3 +113,71 @@ def test_exec_allows_reads_of_history_jsonl(command):
|
|||||||
tool = ExecTool()
|
tool = ExecTool()
|
||||||
result = tool._guard_command(command, "/tmp")
|
result = tool._guard_command(command, "/tmp")
|
||||||
assert result is None
|
assert result is None
|
||||||
|
|
||||||
|
|
||||||
|
# --- #2826: working_dir must not escape the configured workspace ---------
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_exec_blocks_working_dir_outside_workspace(tmp_path):
|
||||||
|
"""An LLM-supplied working_dir outside the workspace must be rejected."""
|
||||||
|
workspace = tmp_path / "workspace"
|
||||||
|
workspace.mkdir()
|
||||||
|
tool = ExecTool(working_dir=str(workspace), restrict_to_workspace=True)
|
||||||
|
result = await tool.execute(command="rm calendar.ics", working_dir="/etc")
|
||||||
|
assert "outside the configured workspace" in result
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_exec_blocks_absolute_rm_via_hijacked_working_dir(tmp_path):
|
||||||
|
"""Regression for #2826: `rm /abs/path` via working_dir hijack."""
|
||||||
|
workspace = tmp_path / "workspace"
|
||||||
|
workspace.mkdir()
|
||||||
|
victim_dir = tmp_path / "outside"
|
||||||
|
victim_dir.mkdir()
|
||||||
|
victim = victim_dir / "file.ics"
|
||||||
|
victim.write_text("data")
|
||||||
|
|
||||||
|
tool = ExecTool(working_dir=str(workspace), restrict_to_workspace=True)
|
||||||
|
result = await tool.execute(
|
||||||
|
command=f"rm {victim}",
|
||||||
|
working_dir=str(victim_dir),
|
||||||
|
)
|
||||||
|
assert "outside the configured workspace" in result
|
||||||
|
assert victim.exists(), "victim file must not have been deleted"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_exec_allows_working_dir_within_workspace(tmp_path):
|
||||||
|
"""A working_dir that is a subdirectory of the workspace is fine."""
|
||||||
|
workspace = tmp_path / "workspace"
|
||||||
|
subdir = workspace / "project"
|
||||||
|
subdir.mkdir(parents=True)
|
||||||
|
tool = ExecTool(working_dir=str(workspace), restrict_to_workspace=True, timeout=5)
|
||||||
|
result = await tool.execute(command="echo ok", working_dir=str(subdir))
|
||||||
|
assert "ok" in result
|
||||||
|
assert "outside the configured workspace" not in result
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_exec_allows_working_dir_equal_to_workspace(tmp_path):
|
||||||
|
"""Passing working_dir equal to the workspace root must be allowed."""
|
||||||
|
workspace = tmp_path / "workspace"
|
||||||
|
workspace.mkdir()
|
||||||
|
tool = ExecTool(working_dir=str(workspace), restrict_to_workspace=True, timeout=5)
|
||||||
|
result = await tool.execute(command="echo ok", working_dir=str(workspace))
|
||||||
|
assert "ok" in result
|
||||||
|
assert "outside the configured workspace" not in result
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_exec_ignores_workspace_check_when_not_restricted(tmp_path):
|
||||||
|
"""Without restrict_to_workspace, the LLM may still choose any working_dir."""
|
||||||
|
workspace = tmp_path / "workspace"
|
||||||
|
workspace.mkdir()
|
||||||
|
other = tmp_path / "other"
|
||||||
|
other.mkdir()
|
||||||
|
tool = ExecTool(working_dir=str(workspace), restrict_to_workspace=False, timeout=5)
|
||||||
|
result = await tool.execute(command="echo ok", working_dir=str(other))
|
||||||
|
assert "ok" in result
|
||||||
|
assert "outside the configured workspace" not in result
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user