fix(crewai-tools): let DirectoryReadTool use a fixed directory outside cwd

PR #6248 confined file tools to an allow-listed root to block path
traversal. FileReadTool and FileWriterTool pin a declared, construction-time
path as always-allowed developer intent, resolved once so a later chdir
cannot move it, while runtime/LLM-supplied paths still go through the
containment check.

DirectoryReadTool was not updated to match. Its `_run` validated the
directory via `validate_directory_path(directory)` with no base_dir, which
defaults to `os.getcwd()`. Any fixed `directory` passed at construction
time that is not the current working directory is rejected, and worse,
with an unhandled ValueError instead of a graceful error string, unlike
its sibling tools.

This breaks the tool's own headline documented use case,
`DirectoryReadTool(directory="/some/dir")`, whenever that directory is not
the process's cwd.

This adds a `base_dir` parameter mirroring FileReadTool, pins the
construction-time `directory` as `_declared_realpath` so it always bypasses
the containment check (developer-declared intent, resolved once so a later
chdir cannot move it), while runtime/LLM-supplied directories still go
through validate_directory_path and remain sandboxed. Missing-directory
and OS errors are now caught and returned as graceful error strings
instead of raising.

Author: manjunathbhaskar

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

@@ -2,9 +2,13 @@
 from typing import Any
 
 from crewai.tools import BaseTool
-from pydantic import BaseModel, Field
+from pydantic import BaseModel, Field, PrivateAttr
 
-from crewai_tools.security.safe_path import validate_directory_path
+from crewai_tools.security.safe_path import (
+    format_error_for_display,
+    format_sandbox_error,
+    validate_directory_path,
+)
 
 
 class FixedDirectoryReadToolSchema(BaseModel):
@@ -18,20 +22,59 @@ class DirectoryReadToolSchema(FixedDirectoryReadToolSchema):
 
 
 class DirectoryReadTool(BaseTool):
+    """A tool for recursively listing a directory's content.
+
+    A ``directory`` given at construction time is developer-declared intent
+    and is always allowed past the containment check, even when it lives
+    outside ``base_dir`` (the tool's operation can still fail if the path
+    does not exist). It is pinned at construction, so a later chdir cannot
+    repoint it. A directory supplied at runtime -- typically chosen by an
+    LLM -- must resolve inside ``base_dir`` (the current working directory
+    by default).
+
+    Args:
+        directory (Optional[str]): Directory to list. If provided, this
+            becomes the fixed directory for the tool and the LLM can no
+            longer choose a different one.
+        base_dir (Optional[str]): Directory that runtime-supplied paths
+            must stay inside. Defaults to the current working directory.
+        **kwargs: Additional keyword arguments passed to BaseTool.
+    """
+
     name: str = "List files in directory"
     description: str = (
         "A tool that can be used to recursively list a directory's content."
     )
     args_schema: type[BaseModel] = DirectoryReadToolSchema
     directory: str | None = None
+    base_dir: str | None = None
+
+    _declared_realpath: str | None = PrivateAttr(default=None)
 
-    def __init__(self, directory: str | None = None, **kwargs: Any) -> None:
+    def __init__(
+        self,
+        directory: str | None = None,
+        base_dir: str | None = None,
+        **kwargs: Any,
+    ) -> None:
         super().__init__(**kwargs)
+        if base_dir is not None:
+            base_dir = os.path.realpath(base_dir)
+        self.base_dir = base_dir
+
         if directory is not None:
             self.directory = directory
             self.description = f"A tool that can be used to list {directory}'s content."
             self.args_schema = FixedDirectoryReadToolSchema
             self._generate_description()
+            # Anchor now so a later chdir cannot move the declared directory,
+            # and so it is never rejected by the base_dir containment check
+            # below -- the developer named it explicitly.
+            self._declared_realpath = (
+                os.path.realpath(directory)
+                if os.path.isabs(directory)
+                else os.path.realpath(os.path.join(base_dir or os.getcwd(), directory))
+            )
 
     def _run(
         self,
@@ -41,13 +84,29 @@ def _run(
         if directory is None:
             raise ValueError("Directory must be provided.")
 
-        directory = validate_directory_path(directory)
+        if self._declared_realpath is not None and directory == self.directory:
+            directory = self._declared_realpath
+            if not os.path.isdir(directory):
+                return f"Error: '{directory}' is not a directory."
+        else:
+            try:
+                directory = validate_directory_path(directory, self.base_dir)
+            except ValueError as e:
+                return "Error: Invalid directory: " + format_sandbox_error(
+                    e,
+                    "Pass base_dir to DirectoryReadTool to allow listing another "
+                    "directory tree.",
+                )
+
         if directory[-1] == "/":
             directory = directory[:-1]
-        files_list = [
-            f"{directory}/{(os.path.join(root, filename).replace(directory, '').lstrip(os.path.sep))}"
-            for root, dirs, files in os.walk(directory)
-            for filename in files
-        ]
+        try:
+            files_list = [
+                f"{directory}/{(os.path.join(root, filename).replace(directory, '').lstrip(os.path.sep))}"
+                for root, dirs, files in os.walk(directory)
+                for filename in files
+            ]
+        except OSError as e:
+            return f"Error: Could not list '{directory}'. {format_error_for_display(e)}"
         files = "\n- ".join(files_list)
         return f"File paths: \n-{files}"

lib/crewai-tools/tests/directory_read_tool_test.py

@@ -0,0 +1,110 @@
+"""Regression tests for DirectoryReadTool.
+
+Covers a bug where a *developer-configured* fixed ``directory`` outside the
+process's current working directory was always rejected as "outside the
+allowed directory", because ``_run`` validated it against
+``validate_directory_path``'s default ``base_dir`` (``os.getcwd()``) with no
+way to widen or pin it -- unlike the sibling ``FileReadTool``/
+``FileWriterTool``, which pin a declared path at construction time.
+
+This made the tool's most basic documented use case --
+``DirectoryReadTool(directory="/some/other/dir")`` -- unusable whenever that
+directory was not the current working directory, and raised an *unhandled*
+``ValueError`` instead of returning a graceful error string.
+"""
+
+import os
+
+import pytest
+
+from crewai_tools.tools.directory_read_tool.directory_read_tool import (
+    DirectoryReadTool,
+)
+
+
+@pytest.fixture
+def outside_dir(tmp_path):
+    """A directory that is guaranteed not to be the cwd used by tests."""
+    target = tmp_path / "outside"
+    target.mkdir()
+    (target / "a.txt").write_text("hello")
+    (target / "nested").mkdir()
+    (target / "nested" / "b.txt").write_text("world")
+    return target
+
+
+def test_fixed_directory_outside_cwd_is_listed(outside_dir, monkeypatch):
+    """A directory declared at construction time must work even when it is
+    not the current working directory (the primary documented use case)."""
+    monkeypatch.chdir(outside_dir.parent)
+    tool = DirectoryReadTool(directory=str(outside_dir))
+
+    result = tool._run()
+
+    assert "Error" not in result
+    assert "a.txt" in result
+    assert "b.txt" in result
+
+
+def test_fixed_directory_survives_chdir(outside_dir, tmp_path, monkeypatch):
+    """The declared directory is pinned at construction time, like
+    FileReadTool's declared file_path, so a later chdir cannot break it."""
+    monkeypatch.chdir(outside_dir)
+    tool = DirectoryReadTool(directory=str(outside_dir))
+
+    other_cwd = tmp_path / "elsewhere"
+    other_cwd.mkdir()
+    monkeypatch.chdir(other_cwd)
+
+    result = tool._run()
+
+    assert "Error" not in result
+    assert "a.txt" in result
+
+
+def test_runtime_directory_outside_base_dir_is_rejected(outside_dir, tmp_path, monkeypatch):
+    """Sandboxing must be preserved for LLM-supplied paths at runtime: only
+    the declared/construction-time directory bypasses containment."""
+    cwd = tmp_path / "cwd"
+    cwd.mkdir()
+    monkeypatch.chdir(cwd)
+    tool = DirectoryReadTool()
+
+    result = tool._run(directory=str(outside_dir))
+
+    assert "Error" in result
+    assert "outside the allowed directory" in result
+
+
+def test_runtime_directory_inside_cwd_is_listed(tmp_path, monkeypatch):
+    monkeypatch.chdir(tmp_path)
+    (tmp_path / "f.txt").write_text("hi")
+    tool = DirectoryReadTool()
+
+    result = tool._run(directory=".")
+
+    assert "Error" not in result
+    assert "f.txt" in result
+
+
+def test_base_dir_widens_runtime_sandbox(outside_dir, tmp_path, monkeypatch):
+    cwd = tmp_path / "cwd"
+    cwd.mkdir()
+    monkeypatch.chdir(cwd)
+    tool = DirectoryReadTool(base_dir=str(outside_dir.parent))
+
+    result = tool._run(directory=str(outside_dir))
+
+    assert "Error" not in result
+    assert "a.txt" in result
+
+
+def test_missing_directory_returns_error_not_exception(tmp_path, monkeypatch):
+    """A nonexistent fixed directory must return a graceful error, not raise."""
+    monkeypatch.chdir(tmp_path)
+    missing = tmp_path / "does-not-exist"
+    tool = DirectoryReadTool(directory=str(missing))
+
+    result = tool._run()
+
+    assert "Error" in result