Skip to content

Commit 81bed00

Browse files
joaomdmouraclaude
andcommitted
fix(tools): anchor base_dir at construction so the sandbox cannot move
Addresses the third review round on #6692. Both remaining findings came from the same habit: storing an unanchored string and re-resolving it later. A relative base_dir was kept verbatim and re-resolved against getcwd() on every call, while the declared file was pinned once at construction. After a chdir the sandbox root moved but the declared default did not, so one tool applied two different roots. base_dir is now resolved once — in the reader's __init__, and via a field_validator on the writer so it also applies on the model_validate path. That also covers the serialization concern. model_dump drops the private pin, and __init__ re-runs on restore, so a relative file_path was re-anchored against whatever the working directory happened to be at load time. With base_dir anchored, restore rebuilds the identical pin. The residual case is a relative file_path with no base_dir, where the sandbox root is the working directory too — so both move together and the tool stays self-consistent. Covered by test_declared_path_survives_a_serialization_round_trip and test_relative_base_dir_is_anchored_at_construction on both tools. Also corrects the writer's 'directory' description, README and docs: the default resolves inside the tool's allowed directory, which is base_dir when one is set, not always the working directory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent f42cb2d commit 81bed00

7 files changed

Lines changed: 91 additions & 6 deletions

File tree

docs/edge/en/tools/file-document/filewritetool.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ The agent supplies these at runtime:
4646

4747
- `filename`: The name of the file to write, relative to `directory`. May include subdirectories, which are created if they don't exist.
4848
- `content`: The text content to write into the file.
49-
- `directory` (optional): The path to the directory where the file will be created. Defaults to the current working directory. If the directory does not exist, it will be created.
49+
- `directory` (optional): The path to the directory where the file will be created. A relative path resolves inside the tool's allowed directory — `base_dir` when set, the current working directory otherwise — and defaults to its root. If the directory does not exist, it will be created.
5050
- `overwrite` (optional): Whether to replace the file when it already exists. Accepts `true`/`false` (also `yes`/`no`, `on`/`off`, `1`/`0`). Defaults to `false`, which reports an error instead of replacing existing content.
5151

5252
You set these when constructing the tool:

lib/crewai-tools/src/crewai_tools/tools/file_read_tool/file_read_tool.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,11 @@ def __init__(
120120
encoding (str): Text encoding used to decode the file.
121121
**kwargs: Additional keyword arguments passed to BaseTool.
122122
"""
123+
# Anchor base_dir once, so the sandbox root cannot move under a later
124+
# chdir while the declared file stays pinned to its original location.
125+
if base_dir is not None:
126+
base_dir = os.path.realpath(base_dir)
127+
123128
display_path = None
124129
if file_path is not None:
125130
display_path = format_path_for_display(file_path, base_dir)

lib/crewai-tools/src/crewai_tools/tools/file_writer_tool/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ The agent supplies these at runtime:
3333

3434
- `filename`: The name of the file to write, relative to `directory`. May include subdirectories, which are created if they don't exist.
3535
- `content`: The text content to write into the file.
36-
- `directory` (optional): The path to the directory where the file will be created. Defaults to the current working directory. If the directory does not exist, it will be created.
36+
- `directory` (optional): The path to the directory where the file will be created. A relative path resolves inside the tool's allowed directory — `base_dir` when set, the current working directory otherwise — and defaults to its root. If the directory does not exist, it will be created.
3737
- `overwrite` (optional): Whether to replace the file when it already exists. Accepts `true`/`false` (also `yes`/`no`, `on`/`off`, `1`/`0`). Defaults to `false`, which reports an error instead of replacing existing content.
3838

3939
You set these when constructing the tool:

lib/crewai-tools/src/crewai_tools/tools/file_writer_tool/file_writer_tool.py

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
from pathlib import Path
33

44
from crewai.tools import BaseTool
5-
from pydantic import BaseModel, Field
5+
from pydantic import BaseModel, Field, field_validator
66

77
from crewai_tools.security.safe_path import (
88
format_error_for_display,
@@ -47,8 +47,9 @@ class FileWriterToolInput(BaseModel):
4747
directory: str | None = Field(
4848
"./",
4949
description=(
50-
"Directory to write the file into. Created if it does not exist. "
51-
"Defaults to the current working directory."
50+
"Directory to write the file into. A relative path resolves inside "
51+
"the tool's allowed directory, and defaults to its root. Created if "
52+
"it does not exist."
5253
),
5354
)
5455
overwrite: str | bool = Field(
@@ -86,6 +87,12 @@ class FileWriterTool(BaseTool):
8687
base_dir: str | None = None
8788
encoding: str = "utf-8"
8889

90+
@field_validator("base_dir")
91+
@classmethod
92+
def _anchor_base_dir(cls, value: str | None) -> str | None:
93+
"""Resolve base_dir once so a later chdir cannot move the sandbox."""
94+
return os.path.realpath(value) if value is not None else None
95+
8996
def _run(
9097
self,
9198
filename: str,

lib/crewai-tools/tests/file_read_tool_test.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,48 @@ def test_relative_declared_path_anchors_to_base_dir(tmp_path, monkeypatch):
298298
assert tool.run(file_path="data.txt") == "sandbox file"
299299

300300

301+
def test_relative_base_dir_is_anchored_at_construction(tmp_path, monkeypatch):
302+
"""A relative base_dir must not follow a later chdir.
303+
304+
Otherwise the sandbox root moves while the declared file stays pinned, and
305+
the tool applies two different roots.
306+
"""
307+
monkeypatch.chdir(tmp_path)
308+
allowed = tmp_path / "allowed"
309+
allowed.mkdir()
310+
(allowed / "data.txt").write_text("sandbox file")
311+
nested = tmp_path / "sub"
312+
nested.mkdir()
313+
314+
tool = FileReadTool(base_dir="allowed")
315+
assert tool.base_dir == str(allowed)
316+
317+
monkeypatch.chdir(nested)
318+
assert tool._run(file_path="data.txt") == "sandbox file"
319+
320+
321+
def test_declared_path_survives_a_serialization_round_trip(tmp_path, monkeypatch):
322+
"""model_dump drops private attrs, so the pin must be rebuilt on restore."""
323+
workspace = tmp_path / "workspace"
324+
workspace.mkdir()
325+
monkeypatch.chdir(workspace)
326+
allowed = tmp_path / "allowed"
327+
allowed.mkdir()
328+
(allowed / "data.txt").write_text("sandbox file")
329+
330+
tool = FileReadTool(file_path="data.txt", base_dir=str(allowed))
331+
restored = FileReadTool.model_validate(tool.model_dump())
332+
333+
assert restored._declared_realpath == tool._declared_realpath
334+
assert restored.run() == "sandbox file"
335+
336+
# A chdir between dump and restore must not repoint the declared file.
337+
nested = workspace / "sub"
338+
nested.mkdir()
339+
monkeypatch.chdir(nested)
340+
assert FileReadTool.model_validate(tool.model_dump()).run() == "sandbox file"
341+
342+
301343
def test_constructor_path_does_not_widen_the_sandbox(tmp_path, monkeypatch):
302344
"""Declaring one file must not expose its siblings to the LLM."""
303345
workspace = tmp_path / "workspace"

lib/crewai-tools/tests/tools/test_file_writer_tool.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,37 @@ def test_base_dir_anchors_relative_directories(temp_env):
301301
shutil.rmtree(outside_dir, ignore_errors=True)
302302

303303

304+
def test_relative_base_dir_is_anchored_at_construction(temp_env, monkeypatch):
305+
"""A relative base_dir must not follow a later chdir."""
306+
allowed = os.path.join(temp_env["temp_dir"], "allowed")
307+
os.makedirs(allowed)
308+
nested = os.path.join(temp_env["temp_dir"], "sub")
309+
os.makedirs(nested)
310+
311+
scoped = FileWriterTool(base_dir="allowed")
312+
assert scoped.base_dir == os.path.realpath(allowed)
313+
314+
monkeypatch.chdir(nested)
315+
result = scoped._run(filename="x.txt", content="written", overwrite=True)
316+
317+
assert "successfully written" in result
318+
assert read_file(os.path.join(allowed, "x.txt")) == "written"
319+
320+
321+
def test_base_dir_survives_a_serialization_round_trip(temp_env):
322+
outside = tempfile.mkdtemp()
323+
try:
324+
restored = FileWriterTool.model_validate(
325+
FileWriterTool(base_dir=outside).model_dump()
326+
)
327+
assert restored.base_dir == os.path.realpath(outside)
328+
assert "successfully written" in restored._run(
329+
filename="x.txt", directory=outside, content="written", overwrite=True
330+
)
331+
finally:
332+
shutil.rmtree(outside, ignore_errors=True)
333+
334+
304335
def test_base_dir_still_blocks_escapes(temp_env):
305336
"""base_dir moves the sandbox; it does not remove it."""
306337
allowed_dir = tempfile.mkdtemp()

lib/crewai-tools/tool.specs.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10143,7 +10143,7 @@
1014310143
}
1014410144
],
1014510145
"default": "./",
10146-
"description": "Directory to write the file into. Created if it does not exist. Defaults to the current working directory.",
10146+
"description": "Directory to write the file into. A relative path resolves inside the tool's allowed directory, and defaults to its root. Created if it does not exist.",
1014710147
"title": "Directory"
1014810148
},
1014910149
"filename": {

0 commit comments

Comments
 (0)