diff --git a/lib/crewai-tools/src/crewai_tools/security/safe_path.py b/lib/crewai-tools/src/crewai_tools/security/safe_path.py index 6b9a3a1c9..481547f9a 100644 --- a/lib/crewai-tools/src/crewai_tools/security/safe_path.py +++ b/lib/crewai-tools/src/crewai_tools/security/safe_path.py @@ -40,23 +40,50 @@ def _get_allowed_roots( is compared by its real location. Empty entries are ignored and duplicates are collapsed while preserving order. The first element is always the primary root used to resolve relative candidate paths. + + The filesystem root (``os.sep``, e.g. ``"/"``) is never accepted as an + *implicitly defaulted* root. When ``base_dir`` is not supplied and the + current working directory is ``/`` -- common in containers started without + a ``WORKDIR`` -- defaulting to it would make every absolute path "within" + the allow-list and disable confinement entirely. In that case the cwd + default is dropped; an operator who genuinely wants the whole filesystem + must opt in explicitly via ``base_dir``, ``allowed_dirs``, or + ``CREWAI_TOOLS_ALLOWED_DIRS``. If no usable root remains, a ``ValueError`` + is raised rather than silently allowing everything. """ - raw_roots: list[str] = [base_dir if base_dir is not None else os.getcwd()] + primary_explicit = base_dir is not None + primary = base_dir if base_dir is not None else os.getcwd() + + # (root, is_explicit) -- explicit roots are operator-provided and may + # legitimately include the filesystem root as an opt-in. + raw_roots: list[tuple[str, bool]] = [(primary, primary_explicit)] env_dirs = os.environ.get(_ALLOWED_DIRS_ENV, "") if env_dirs: - raw_roots.extend(d for d in env_dirs.split(os.pathsep) if d) + raw_roots.extend((d, True) for d in env_dirs.split(os.pathsep) if d) if allowed_dirs: - raw_roots.extend(d for d in allowed_dirs if d) + raw_roots.extend((d, True) for d in allowed_dirs if d) resolved: list[str] = [] seen: set[str] = set() - for root in raw_roots: + for root, is_explicit in raw_roots: real = os.path.realpath(root) + if real == os.sep and not is_explicit: + # Refuse to let an unconfigured cwd of "/" open the whole filesystem. + continue if real not in seen: seen.add(real) resolved.append(real) + + if not resolved: + raise ValueError( + "No safe allowed directory could be determined: the current working " + f"directory is the filesystem root ('{os.sep}'). Set " + f"{_ALLOWED_DIRS_ENV} to an explicit directory, pass " + f"base_dir/allowed_dirs, or set {_UNSAFE_PATHS_ENV}=true to bypass " + "path validation." + ) return resolved diff --git a/lib/crewai-tools/tests/utilities/test_safe_path.py b/lib/crewai-tools/tests/utilities/test_safe_path.py index ae88fa181..65b24d117 100644 --- a/lib/crewai-tools/tests/utilities/test_safe_path.py +++ b/lib/crewai-tools/tests/utilities/test_safe_path.py @@ -250,3 +250,36 @@ class TestAllowList: assert validate_file_path(str(b / "fb.txt"), base_dir=str(base)) == str( b / "fb.txt" ) + + def test_cwd_root_default_is_not_an_allowed_root(self, tmp_path, monkeypatch): + """An unconfigured cwd of '/' must not open the whole filesystem. + + Regression for the deny-by-default allow-list silently defaulting to the + filesystem root in containers started without a WORKDIR. + """ + monkeypatch.delenv("CREWAI_TOOLS_ALLOWED_DIRS", raising=False) + monkeypatch.delenv("CREWAI_TOOLS_ALLOW_UNSAFE_PATHS", raising=False) + monkeypatch.setattr(os, "getcwd", lambda: os.sep) + with pytest.raises(ValueError, match="filesystem root"): + validate_file_path("/etc/passwd") + + def test_cwd_root_with_explicit_allowed_dirs_confines( + self, tmp_path, monkeypatch + ): + """With cwd '/', confinement falls back to the explicit allow-list.""" + monkeypatch.delenv("CREWAI_TOOLS_ALLOWED_DIRS", raising=False) + monkeypatch.setattr(os, "getcwd", lambda: os.sep) + (tmp_path / "data.txt").touch() + assert validate_file_path( + str(tmp_path / "data.txt"), allowed_dirs=[str(tmp_path)] + ) == str(tmp_path / "data.txt") + with pytest.raises(ValueError, match="outside the allowed directories"): + validate_file_path("/etc/passwd", allowed_dirs=[str(tmp_path)]) + + def test_explicit_base_dir_root_is_opt_in(self, monkeypatch): + """An explicit base_dir of '/' is honored as a deliberate opt-in.""" + monkeypatch.delenv("CREWAI_TOOLS_ALLOWED_DIRS", raising=False) + monkeypatch.delenv("CREWAI_TOOLS_ALLOW_UNSAFE_PATHS", raising=False) + assert validate_file_path("/etc/passwd", base_dir=os.sep) == os.path.realpath( + "/etc/passwd" + )