Skip to content

Commit 7f35b76

Browse files
joaomdmouraclaude
andcommitted
fix(tools): anchor the declared read path to base_dir, not the cwd
Addresses the second round of review feedback on #6692. The previous commit pinned a relative constructor file_path with os.path.realpath, which anchors to the working directory, while both format_path_for_display and validate_file_path anchor a relative path to base_dir. With the two roots disagreeing, the same relative string meant two different files — and the tool served the cwd one under a label that looks like it belongs to the sandbox: FileReadTool(file_path="data.txt", base_dir="/allowed") # cwd=/work label advertised to the model -> "data.txt" run(file_path="data.txt") -> contents of /work/data.txt That reads a file from outside base_dir, so it was a sandbox escape introduced by the exemption itself, not just a wrong-file bug. Resolution now goes through a single _resolve_against_base helper that anchors relative paths exactly the way the sandbox does, so the pinned path, the advertised label and the containment check all agree. Covered by test_relative_declared_path_anchors_to_base_dir. Also softens "always readable" to "always allowed past the containment check" in the docstring, README and docs, since bypassing containment does not guarantee the read succeeds — it can still fail on a missing file, a directory, or permissions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 3c8bdb2 commit 7f35b76

5 files changed

Lines changed: 53 additions & 6 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ You set these when constructing the tool:
6868
Because the file path is usually chosen by an LLM at runtime, reads are confined to a sandbox:
6969

7070
- Paths supplied at runtime must resolve inside `base_dir`, which defaults to the current working directory. `..` segments and symlinks are resolved before the check, so they cannot be used to escape.
71-
- A `file_path` passed to the constructor is developer-declared intent, so it is always readable — even outside `base_dir`. It is pinned when the tool is built, so a later change of working directory cannot repoint it, and the agent can address it either by omitting `file_path` or by using the name shown in the tool's description. Declaring one file does not expose its siblings.
71+
- A `file_path` passed to the constructor is developer-declared intent, so it is always allowed past the containment check — even outside `base_dir`. The read itself can still fail if the file is missing, is a directory, or is not permitted. It is pinned when the tool is built, so a later change of working directory cannot repoint it, and the agent can address it either by omitting `file_path` or by using the name shown in the tool's description. Declaring one file does not expose its siblings.
7272

7373
To let an agent read a directory tree outside the working directory, point `base_dir` at it:
7474

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ You set these when constructing the tool:
5252
Because the file path is usually chosen by an LLM at runtime, reads are confined to a sandbox:
5353

5454
- Paths supplied at runtime must resolve inside `base_dir` (the current working directory by default). `..` segments and symlinks are resolved before the check, so they cannot be used to escape.
55-
- A `file_path` passed to the constructor is developer-declared intent, so it is always readable — even outside `base_dir`. It is pinned when the tool is built, so a later change of working directory cannot repoint it, and the agent can address it either by omitting `file_path` or by using the name shown in the tool's description. Declaring one file does not expose its siblings.
55+
- A `file_path` passed to the constructor is developer-declared intent, so it is always allowed past the containment check — even outside `base_dir`. The read itself can still fail if the file is missing, is a directory, or is not permitted. It is pinned when the tool is built, so a later change of working directory cannot repoint it, and the agent can address it either by omitting `file_path` or by using the name shown in the tool's description. Declaring one file does not expose its siblings.
5656

5757
To let an agent read a directory tree outside the working directory, point `base_dir` at it:
5858

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

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,26 @@
1212
)
1313

1414

15+
def _resolve_against_base(path: str, base_dir: str | None) -> str:
16+
"""Resolve *path* the way the sandbox does, anchoring relatives to *base_dir*.
17+
18+
``validate_file_path`` and ``format_path_for_display`` both join a relative
19+
path onto *base_dir* rather than the working directory. Resolution has to
20+
agree with them, or the same relative string would mean two different files.
21+
22+
Args:
23+
path: The path to resolve.
24+
base_dir: The anchor for relative paths. Defaults to the working directory.
25+
26+
Returns:
27+
The resolved absolute path.
28+
"""
29+
if os.path.isabs(path):
30+
return os.path.realpath(path)
31+
base = os.path.realpath(base_dir) if base_dir is not None else os.getcwd()
32+
return os.path.realpath(os.path.join(base, path))
33+
34+
1535
class FileReadToolSchema(BaseModel):
1636
"""Input for FileReadTool."""
1737

@@ -46,7 +66,8 @@ class FileReadTool(BaseTool):
4666
Paths supplied at runtime must resolve inside ``base_dir`` (the current
4767
working directory by default), since they are typically chosen by an LLM.
4868
A ``file_path`` given at construction time is developer-declared intent and
49-
is always readable, even when it lives outside ``base_dir``. It is pinned at
69+
is always allowed past the containment check, even when it lives outside
70+
``base_dir`` (the read itself can still fail). It is pinned at
5071
construction, so a later chdir cannot repoint it, and it can be addressed
5172
either by omitting ``file_path`` or by the label shown in the description.
5273
@@ -111,7 +132,9 @@ def __init__(
111132
self.base_dir = base_dir
112133
self.encoding = encoding
113134
self._declared_realpath = (
114-
os.path.realpath(file_path) if file_path is not None else None
135+
_resolve_against_base(file_path, base_dir)
136+
if file_path is not None
137+
else None
115138
)
116139
self._declared_label = display_path
117140

@@ -136,7 +159,8 @@ def _resolve_path(self, file_path: str) -> str:
136159
"""
137160
declared = self._declared_realpath
138161
if declared is not None and (
139-
file_path == self._declared_label or os.path.realpath(file_path) == declared
162+
file_path == self._declared_label
163+
or _resolve_against_base(file_path, self.base_dir) == declared
140164
):
141165
return declared
142166
return validate_file_path(file_path, self.base_dir)

lib/crewai-tools/tests/file_read_tool_test.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,29 @@ def test_declared_relative_path_survives_chdir(tmp_path, monkeypatch):
275275
assert tool._run(file_path="rel.txt") == "original"
276276

277277

278+
def test_relative_declared_path_anchors_to_base_dir(tmp_path, monkeypatch):
279+
"""A relative declared path must resolve against base_dir, not the cwd.
280+
281+
The advertised label is built against base_dir, so pinning against the cwd
282+
would make the same name mean two different files — and would serve a file
283+
from outside base_dir under a label that looks like it is inside.
284+
"""
285+
allowed = tmp_path / "allowed"
286+
allowed.mkdir()
287+
work = tmp_path / "work"
288+
work.mkdir()
289+
(allowed / "data.txt").write_text("sandbox file")
290+
(work / "data.txt").write_text("cwd file")
291+
monkeypatch.chdir(work)
292+
293+
tool = FileReadTool(file_path="data.txt", base_dir=str(allowed))
294+
295+
assert tool._declared_label == "data.txt"
296+
assert tool._declared_realpath == str(allowed / "data.txt")
297+
assert tool.run() == "sandbox file"
298+
assert tool.run(file_path="data.txt") == "sandbox file"
299+
300+
278301
def test_constructor_path_does_not_widen_the_sandbox(tmp_path, monkeypatch):
279302
"""Declaring one file must not expose its siblings to the LLM."""
280303
workspace = tmp_path / "workspace"

lib/crewai-tools/tool.specs.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9972,7 +9972,7 @@
99729972
"type": "object"
99739973
}
99749974
},
9975-
"description": "A tool for reading file contents.\n\nThis tool inherits its schema handling from BaseTool to avoid recursive schema\ndefinition issues. The args_schema is set to FileReadToolSchema, whose\nfile_path parameter is optional so the tool's default file can be read by\nomitting it. The schema should not be overridden in the constructor as it\nwould break the inheritance chain and cause infinite loops.\n\nThe tool supports two ways of specifying the file path:\n1. At construction time via the file_path parameter\n2. At runtime via the file_path parameter in the tool's input\n\nPaths supplied at runtime must resolve inside ``base_dir`` (the current\nworking directory by default), since they are typically chosen by an LLM.\nA ``file_path`` given at construction time is developer-declared intent and\nis always readable, even when it lives outside ``base_dir``. It is pinned at\nconstruction, so a later chdir cannot repoint it, and it can be addressed\neither by omitting ``file_path`` or by the label shown in the description.\n\nArgs:\n file_path (Optional[str]): Path to the file to be read. If provided,\n this becomes the default file path for the tool.\n base_dir (Optional[str]): Directory that runtime paths must stay inside.\n Defaults to the current working directory.\n encoding (str): Text encoding used to decode the file. Defaults to UTF-8.\n **kwargs: Additional keyword arguments passed to BaseTool.\n\nExample:\n >>> tool = FileReadTool(file_path=\"/path/to/file.txt\")\n >>> content = tool.run() # Reads /path/to/file.txt\n >>> content = tool.run(file_path=\"/path/to/other.txt\") # Reads other.txt\n >>> content = tool.run(\n ... file_path=\"/path/to/file.txt\", start_line=100, line_count=50\n ... ) # Reads lines 100-149\n >>> # Widen the sandbox so the agent may read anything under /data:\n >>> tool = FileReadTool(base_dir=\"/data\")",
9975+
"description": "A tool for reading file contents.\n\nThis tool inherits its schema handling from BaseTool to avoid recursive schema\ndefinition issues. The args_schema is set to FileReadToolSchema, whose\nfile_path parameter is optional so the tool's default file can be read by\nomitting it. The schema should not be overridden in the constructor as it\nwould break the inheritance chain and cause infinite loops.\n\nThe tool supports two ways of specifying the file path:\n1. At construction time via the file_path parameter\n2. At runtime via the file_path parameter in the tool's input\n\nPaths supplied at runtime must resolve inside ``base_dir`` (the current\nworking directory by default), since they are typically chosen by an LLM.\nA ``file_path`` given at construction time is developer-declared intent and\nis always allowed past the containment check, even when it lives outside\n``base_dir`` (the read itself can still fail). It is pinned at\nconstruction, so a later chdir cannot repoint it, and it can be addressed\neither by omitting ``file_path`` or by the label shown in the description.\n\nArgs:\n file_path (Optional[str]): Path to the file to be read. If provided,\n this becomes the default file path for the tool.\n base_dir (Optional[str]): Directory that runtime paths must stay inside.\n Defaults to the current working directory.\n encoding (str): Text encoding used to decode the file. Defaults to UTF-8.\n **kwargs: Additional keyword arguments passed to BaseTool.\n\nExample:\n >>> tool = FileReadTool(file_path=\"/path/to/file.txt\")\n >>> content = tool.run() # Reads /path/to/file.txt\n >>> content = tool.run(file_path=\"/path/to/other.txt\") # Reads other.txt\n >>> content = tool.run(\n ... file_path=\"/path/to/file.txt\", start_line=100, line_count=50\n ... ) # Reads lines 100-149\n >>> # Widen the sandbox so the agent may read anything under /data:\n >>> tool = FileReadTool(base_dir=\"/data\")",
99769976
"properties": {
99779977
"base_dir": {
99789978
"anyOf": [

0 commit comments

Comments
 (0)