mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-08-10 08:21:54 +00:00
feat(tools): make the file tools' backing store pluggable
FileReadTool and FileWriterTool assume a durable local disk. That holds on a developer's machine and breaks in any deployment environment where the runtime is ephemeral: whatever an agent writes is discarded when the run ends, and a later run cannot read it back. A crew that generates a report in one task and reads it in the next passes locally and fails there. This adds the seam needed to point those tools at durable storage instead. Both now route every path resolution and every read/write through a FileStore, defaulting to LocalFileStore — the current filesystem behavior, moved rather than rewritten. A deployment registers a different store through register_file_store_factory and the tools pick it up. The store owns its own containment, because the tools call nothing else before doing I/O. For the local store that stays validate_file_path plus the is_relative_to check; another store enforces whatever its own namespace requires, which may be prefix-based rather than realpath-based. resolve() and normalize() are separate so the reader can still pin its declared file for identity without a containment check, and base_dir is anchored through the store so both tools derive the same sandbox root from the same input. open_text() returns a handle rather than a string, which keeps the local store lazy: reading a small window out of a huge file does not pull the whole thing into memory. A store that must fetch eagerly can wrap its payload in StringIO. Behaviour is unchanged: every pre-existing file tool test passes untouched. The new suite stands in a store backed by a dict with no filesystem at all, which is what proves the seam is real — a tool that reached past it to open() or os.path would fail those assertions. It also covers the fallbacks that keep this safe to ship before any integration exists: a factory returning None, and a factory that raises, both leave the local filesystem in place rather than breaking file I/O. No new dependencies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
30
lib/crewai-tools/src/crewai_tools/file_storage/__init__.py
Normal file
30
lib/crewai-tools/src/crewai_tools/file_storage/__init__.py
Normal file
@@ -0,0 +1,30 @@
|
||||
"""Pluggable backing store for :class:`FileReadTool` / :class:`FileWriterTool`.
|
||||
|
||||
The tools default to :class:`LocalFileStore`, which reads and writes the
|
||||
local filesystem exactly as they always have. A deployment environment where
|
||||
the local disk is ephemeral can register a different store, so the same tools
|
||||
persist somewhere durable without the agent, the crew definition, or the tool
|
||||
arguments changing.
|
||||
|
||||
A store owns its own containment. ``resolve`` and ``resolve_within`` must
|
||||
reject any path the caller should not reach, because the tools call nothing
|
||||
else before doing I/O.
|
||||
"""
|
||||
|
||||
from crewai_tools.file_storage.base import FileStore, FileStoreError
|
||||
from crewai_tools.file_storage.local import LocalFileStore
|
||||
from crewai_tools.file_storage.registry import (
|
||||
register_file_store_factory,
|
||||
reset_file_store_factory,
|
||||
resolve_file_store,
|
||||
)
|
||||
|
||||
|
||||
__all__ = [
|
||||
"FileStore",
|
||||
"FileStoreError",
|
||||
"LocalFileStore",
|
||||
"register_file_store_factory",
|
||||
"reset_file_store_factory",
|
||||
"resolve_file_store",
|
||||
]
|
||||
103
lib/crewai-tools/src/crewai_tools/file_storage/base.py
Normal file
103
lib/crewai-tools/src/crewai_tools/file_storage/base.py
Normal file
@@ -0,0 +1,103 @@
|
||||
"""The store protocol the file tools are written against."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from contextlib import AbstractContextManager
|
||||
from typing import Protocol, TextIO, runtime_checkable
|
||||
|
||||
|
||||
class FileStoreError(Exception):
|
||||
"""A store failed for a reason with no stdlib exception that fits.
|
||||
|
||||
Stores should prefer the built-in filesystem exceptions where one
|
||||
applies — ``FileNotFoundError``, ``PermissionError``,
|
||||
``IsADirectoryError``, ``FileExistsError`` — because the tools already
|
||||
translate those into their established messages. Raise this only for
|
||||
failures specific to the backing service, such as an unreachable
|
||||
endpoint or a size limit the local filesystem does not have.
|
||||
"""
|
||||
|
||||
|
||||
@runtime_checkable
|
||||
class FileStore(Protocol):
|
||||
"""Where :class:`FileReadTool` and :class:`FileWriterTool` do their I/O.
|
||||
|
||||
Paths crossing this boundary are *store paths*: whatever ``resolve``
|
||||
returned. For the local store those are absolute filesystem paths; for a
|
||||
remote store they may be keys or workspace-relative paths. The tools
|
||||
never interpret them, they only pass them back.
|
||||
"""
|
||||
|
||||
#: Short human-readable name, used in error messages so a failure makes
|
||||
#: clear which store produced it (e.g. ``"local filesystem"``).
|
||||
label: str
|
||||
|
||||
def resolve(self, path: str, base_dir: str | None = None) -> str:
|
||||
"""Normalize *path* and confirm the caller may touch it.
|
||||
|
||||
Args:
|
||||
path: The caller-supplied path, absolute or relative.
|
||||
base_dir: Optional containment root supplied by the tool.
|
||||
|
||||
Returns:
|
||||
The store path to use for subsequent calls.
|
||||
|
||||
Raises:
|
||||
ValueError: If the path falls outside what the store allows.
|
||||
"""
|
||||
|
||||
def normalize(self, path: str, base_dir: str | None = None) -> str:
|
||||
"""Normalize *path* for identity comparison, without containment.
|
||||
|
||||
:class:`FileReadTool` uses this to pin the file declared at
|
||||
construction, so it can recognize that path again later even if the
|
||||
working directory has since moved. Unlike :meth:`resolve` it never
|
||||
rejects: a path outside the sandbox still has a canonical form.
|
||||
"""
|
||||
|
||||
def resolve_within(self, directory: str, filename: str) -> str:
|
||||
"""Join *filename* under the already-resolved *directory*.
|
||||
|
||||
Kept separate from :meth:`resolve` because the writer applies two
|
||||
levels of containment: the directory must be inside the store's
|
||||
sandbox, and the filename must then stay inside that directory.
|
||||
|
||||
Raises:
|
||||
ValueError: If *filename* escapes *directory*, or names the
|
||||
directory itself.
|
||||
"""
|
||||
|
||||
def display(self, resolved: str, base: str | None = None) -> str:
|
||||
"""Return a label for *resolved* that is safe to show an LLM.
|
||||
|
||||
Must not leak absolute directory prefixes; the tools put the result
|
||||
straight into agent-visible output.
|
||||
"""
|
||||
|
||||
def exists(self, resolved: str) -> bool:
|
||||
"""Whether something already lives at *resolved*."""
|
||||
|
||||
def ensure_parent(self, resolved: str) -> None:
|
||||
"""Create the container *resolved* will live in, if it needs one.
|
||||
|
||||
Raises:
|
||||
FileExistsError: If a non-container already occupies that name.
|
||||
"""
|
||||
|
||||
def open_text(self, resolved: str, encoding: str) -> AbstractContextManager[TextIO]:
|
||||
"""Open *resolved* for reading as text.
|
||||
|
||||
Returning a file-like object rather than a string keeps the local
|
||||
store lazy, so reading a small window out of a huge file does not
|
||||
pull the whole thing into memory. Remote stores that must fetch
|
||||
eagerly can wrap the payload in ``io.StringIO``.
|
||||
"""
|
||||
|
||||
def write_text(
|
||||
self, resolved: str, content: str, encoding: str, *, overwrite: bool
|
||||
) -> None:
|
||||
"""Write *content* to *resolved*.
|
||||
|
||||
Raises:
|
||||
FileExistsError: If the path exists and *overwrite* is false.
|
||||
"""
|
||||
88
lib/crewai-tools/src/crewai_tools/file_storage/local.py
Normal file
88
lib/crewai-tools/src/crewai_tools/file_storage/local.py
Normal file
@@ -0,0 +1,88 @@
|
||||
"""The default store: the local filesystem, sandboxed to a base directory."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from contextlib import AbstractContextManager
|
||||
import os
|
||||
from pathlib import Path
|
||||
from typing import TextIO
|
||||
|
||||
from crewai_tools.security.safe_path import (
|
||||
format_error_for_display,
|
||||
format_path_for_display,
|
||||
validate_file_path,
|
||||
)
|
||||
|
||||
|
||||
class LocalFileStore:
|
||||
"""Reads and writes the local filesystem.
|
||||
|
||||
Containment is :func:`validate_file_path`: a resolved path must stay
|
||||
inside ``base_dir`` (the working directory by default), with symlinks and
|
||||
``..`` segments resolved first.
|
||||
"""
|
||||
|
||||
label = "local filesystem"
|
||||
|
||||
def resolve(self, path: str, base_dir: str | None = None) -> str:
|
||||
"""Resolve *path*, confining it to *base_dir*."""
|
||||
return validate_file_path(path, base_dir)
|
||||
|
||||
def normalize(self, path: str, base_dir: str | None = None) -> str:
|
||||
"""Resolve *path* the way the sandbox does, without rejecting it.
|
||||
|
||||
``validate_file_path`` and ``format_path_for_display`` both join a
|
||||
relative path onto *base_dir* rather than the working directory.
|
||||
Normalization has to agree with them, or the same relative string
|
||||
would mean two different files.
|
||||
"""
|
||||
if os.path.isabs(path):
|
||||
return os.path.realpath(path)
|
||||
base = os.path.realpath(base_dir) if base_dir is not None else os.getcwd()
|
||||
return os.path.realpath(os.path.join(base, path))
|
||||
|
||||
def resolve_within(self, directory: str, filename: str) -> str:
|
||||
"""Join *filename* under *directory*, blocking every escape route.
|
||||
|
||||
``..``, absolute paths and symlinks are all resolved before the
|
||||
check. ``is_relative_to`` compares whole path components, so it is
|
||||
safe on case-insensitive filesystems and avoids the "//" prefix edge
|
||||
case. A filename resolving to the directory itself (an empty
|
||||
filename, say) is not a valid file target.
|
||||
"""
|
||||
root = Path(directory)
|
||||
try:
|
||||
resolved = Path(os.path.join(directory, filename)).resolve()
|
||||
except (OSError, ValueError) as exc:
|
||||
# e.g. an embedded null byte or an over-long name, which trip the
|
||||
# underlying syscall. str() on an OSError carries the absolute
|
||||
# filename, and the tools put this message straight into
|
||||
# agent-visible output, so strip it back to the reason.
|
||||
raise ValueError(format_error_for_display(exc)) from exc
|
||||
|
||||
if not resolved.is_relative_to(root) or resolved == root:
|
||||
raise ValueError("the filename must not escape the target directory")
|
||||
return str(resolved)
|
||||
|
||||
def display(self, resolved: str, base: str | None = None) -> str:
|
||||
"""Return a path label with absolute prefixes stripped."""
|
||||
return format_path_for_display(resolved, base)
|
||||
|
||||
def exists(self, resolved: str) -> bool:
|
||||
return os.path.exists(resolved)
|
||||
|
||||
def ensure_parent(self, resolved: str) -> None:
|
||||
"""Create the parent directory, including any missing ancestors."""
|
||||
os.makedirs(os.path.dirname(resolved) or ".", exist_ok=True)
|
||||
|
||||
def open_text(self, resolved: str, encoding: str) -> AbstractContextManager[TextIO]:
|
||||
return open(resolved, "r", encoding=encoding)
|
||||
|
||||
def write_text(
|
||||
self, resolved: str, content: str, encoding: str, *, overwrite: bool
|
||||
) -> None:
|
||||
# "x" makes the create-exclusive check atomic, so an existence race
|
||||
# surfaces as FileExistsError rather than silently clobbering.
|
||||
mode = "w" if overwrite else "x"
|
||||
with open(resolved, mode, encoding=encoding) as handle:
|
||||
handle.write(content)
|
||||
74
lib/crewai-tools/src/crewai_tools/file_storage/registry.py
Normal file
74
lib/crewai-tools/src/crewai_tools/file_storage/registry.py
Normal file
@@ -0,0 +1,74 @@
|
||||
"""How a deployment swaps in a different store.
|
||||
|
||||
Kept as a process-wide factory rather than a tool argument on purpose: the
|
||||
crews that need this are already written and deployed, and the point is that
|
||||
they keep working unchanged when the runtime is ephemeral. An integration
|
||||
package registers its factory at import time, and every file tool constructed
|
||||
afterwards picks it up.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Callable
|
||||
import logging
|
||||
import threading
|
||||
|
||||
from crewai_tools.file_storage.base import FileStore
|
||||
from crewai_tools.file_storage.local import LocalFileStore
|
||||
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
#: A factory returns the store to use, or ``None`` to decline — which lets an
|
||||
#: integration arm itself only when its backing service is actually
|
||||
#: configured, and fall back to the local filesystem everywhere else.
|
||||
FileStoreFactory = Callable[[], FileStore | None]
|
||||
|
||||
_lock = threading.Lock()
|
||||
_factory: FileStoreFactory | None = None
|
||||
_local = LocalFileStore()
|
||||
|
||||
|
||||
def register_file_store_factory(factory: FileStoreFactory | None) -> None:
|
||||
"""Install the factory consulted for every new file tool.
|
||||
|
||||
Args:
|
||||
factory: Callable returning a :class:`FileStore`, or ``None`` to
|
||||
decline and leave the local filesystem in place. Passing
|
||||
``None`` as the factory itself unregisters.
|
||||
"""
|
||||
global _factory
|
||||
with _lock:
|
||||
_factory = factory
|
||||
|
||||
|
||||
def reset_file_store_factory() -> None:
|
||||
"""Drop any registered factory. Intended for tests."""
|
||||
register_file_store_factory(None)
|
||||
|
||||
|
||||
def resolve_file_store() -> FileStore:
|
||||
"""Return the store the file tools should use.
|
||||
|
||||
A factory that raises is not allowed to take the tools down with it: an
|
||||
integration failing to initialize should degrade to the local filesystem
|
||||
— the behavior before it was installed — not break file I/O outright.
|
||||
"""
|
||||
with _lock:
|
||||
factory = _factory
|
||||
|
||||
if factory is None:
|
||||
return _local
|
||||
|
||||
try:
|
||||
store = factory()
|
||||
except Exception:
|
||||
logger.warning(
|
||||
"file store factory raised; falling back to the local filesystem",
|
||||
exc_info=True,
|
||||
)
|
||||
return _local
|
||||
|
||||
if store is None:
|
||||
return _local
|
||||
return store
|
||||
@@ -1,38 +1,16 @@
|
||||
from itertools import islice
|
||||
import os
|
||||
from typing import Any
|
||||
|
||||
from crewai.tools import BaseTool
|
||||
from pydantic import BaseModel, Field, PrivateAttr
|
||||
|
||||
from crewai_tools.file_storage import FileStore, resolve_file_store
|
||||
from crewai_tools.security.safe_path import (
|
||||
format_error_for_display,
|
||||
format_path_for_display,
|
||||
format_sandbox_error,
|
||||
validate_file_path,
|
||||
)
|
||||
|
||||
|
||||
def _resolve_against_base(path: str, base_dir: str | None) -> str:
|
||||
"""Resolve *path* the way the sandbox does, anchoring relatives to *base_dir*.
|
||||
|
||||
``validate_file_path`` and ``format_path_for_display`` both join a relative
|
||||
path onto *base_dir* rather than the working directory. Resolution has to
|
||||
agree with them, or the same relative string would mean two different files.
|
||||
|
||||
Args:
|
||||
path: The path to resolve.
|
||||
base_dir: The anchor for relative paths. Defaults to the working directory.
|
||||
|
||||
Returns:
|
||||
The resolved absolute path.
|
||||
"""
|
||||
if os.path.isabs(path):
|
||||
return os.path.realpath(path)
|
||||
base = os.path.realpath(base_dir) if base_dir is not None else os.getcwd()
|
||||
return os.path.realpath(os.path.join(base, path))
|
||||
|
||||
|
||||
class FileReadToolSchema(BaseModel):
|
||||
"""Input for FileReadTool."""
|
||||
|
||||
@@ -103,6 +81,9 @@ class FileReadTool(BaseTool):
|
||||
_declared_realpath: str | None = PrivateAttr(default=None)
|
||||
# The label the tool's description shows the LLM for the declared file.
|
||||
_declared_label: str | None = PrivateAttr(default=None)
|
||||
# Resolved once per tool: a deployment installs its store before the crew
|
||||
# is built, and swapping mid-run would change where a path points.
|
||||
_store: FileStore = PrivateAttr(default=None) # type: ignore[assignment]
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
@@ -121,14 +102,16 @@ class FileReadTool(BaseTool):
|
||||
encoding (str): Text encoding used to decode the file.
|
||||
**kwargs: Additional keyword arguments passed to BaseTool.
|
||||
"""
|
||||
store = resolve_file_store()
|
||||
|
||||
# Anchor base_dir once, so the sandbox root cannot move under a later
|
||||
# chdir while the declared file stays pinned to its original location.
|
||||
if base_dir is not None:
|
||||
base_dir = os.path.realpath(base_dir)
|
||||
base_dir = store.normalize(base_dir)
|
||||
|
||||
display_path = None
|
||||
if file_path is not None:
|
||||
display_path = format_path_for_display(file_path, base_dir)
|
||||
display_path = store.display(store.normalize(file_path, base_dir), base_dir)
|
||||
kwargs["description"] = (
|
||||
f"A tool that reads file content. The default file is {display_path}, which is read when 'file_path' is omitted. You can also provide a different 'file_path' parameter to read another file, though reads are confined to the tool's allowed directory and a path that resolves outside it is rejected. Specify 'start_line' and 'line_count' to read specific parts of the file."
|
||||
)
|
||||
@@ -137,10 +120,9 @@ class FileReadTool(BaseTool):
|
||||
self.file_path = file_path
|
||||
self.base_dir = base_dir
|
||||
self.encoding = encoding
|
||||
self._store = store
|
||||
self._declared_realpath = (
|
||||
_resolve_against_base(file_path, base_dir)
|
||||
if file_path is not None
|
||||
else None
|
||||
store.normalize(file_path, base_dir) if file_path is not None else None
|
||||
)
|
||||
self._declared_label = display_path
|
||||
|
||||
@@ -166,10 +148,10 @@ class FileReadTool(BaseTool):
|
||||
declared = self._declared_realpath
|
||||
if declared is not None and (
|
||||
file_path == self._declared_label
|
||||
or _resolve_against_base(file_path, self.base_dir) == declared
|
||||
or self._store.normalize(file_path, self.base_dir) == declared
|
||||
):
|
||||
return declared
|
||||
return validate_file_path(file_path, self.base_dir)
|
||||
return self._store.resolve(file_path, self.base_dir)
|
||||
|
||||
def _run(
|
||||
self,
|
||||
@@ -195,9 +177,10 @@ class FileReadTool(BaseTool):
|
||||
"directory tree.",
|
||||
)
|
||||
|
||||
display_path = format_path_for_display(file_path, self.base_dir)
|
||||
store = self._store
|
||||
display_path = store.display(file_path, self.base_dir)
|
||||
try:
|
||||
with open(file_path, "r", encoding=self.encoding) as file:
|
||||
with store.open_text(file_path, self.encoding) as file:
|
||||
if start_line == 1 and line_count is None:
|
||||
return file.read()
|
||||
|
||||
|
||||
@@ -1,14 +1,10 @@
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
from crewai.tools import BaseTool
|
||||
from pydantic import BaseModel, Field, field_validator
|
||||
from pydantic import BaseModel, Field, PrivateAttr
|
||||
|
||||
from crewai_tools.file_storage import FileStore, resolve_file_store
|
||||
from crewai_tools.security.safe_path import (
|
||||
format_error_for_display,
|
||||
format_path_for_display,
|
||||
format_sandbox_error,
|
||||
validate_file_path,
|
||||
)
|
||||
|
||||
|
||||
@@ -88,11 +84,24 @@ class FileWriterTool(BaseTool):
|
||||
base_dir: str | None = None
|
||||
encoding: str = "utf-8"
|
||||
|
||||
@field_validator("base_dir")
|
||||
@classmethod
|
||||
def _anchor_base_dir(cls, value: str | None) -> str | None:
|
||||
"""Resolve base_dir once so a later chdir cannot move the sandbox."""
|
||||
return os.path.realpath(value) if value is not None else None
|
||||
# Resolved once per tool: a deployment installs its store before the crew
|
||||
# is built, and swapping mid-run would change where a path points.
|
||||
_store: FileStore = PrivateAttr(default=None) # type: ignore[assignment]
|
||||
|
||||
def model_post_init(self, context: object) -> None:
|
||||
"""Bind the store, then anchor base_dir with the store's own grammar.
|
||||
|
||||
Anchoring cannot be a field validator: validators run before
|
||||
``model_post_init``, so ``_store`` is not bound yet and the only option
|
||||
there is ``os.path.realpath`` — local-filesystem semantics applied to a
|
||||
path a remote store may not interpret that way at all. Doing it here
|
||||
keeps every path decision inside the seam, and still resolves once so a
|
||||
later chdir cannot move the sandbox.
|
||||
"""
|
||||
super().model_post_init(context)
|
||||
self._store = resolve_file_store()
|
||||
if self.base_dir is not None:
|
||||
self.base_dir = self._store.normalize(self.base_dir)
|
||||
|
||||
def _run(
|
||||
self,
|
||||
@@ -109,11 +118,13 @@ class FileWriterTool(BaseTool):
|
||||
except ValueError as e:
|
||||
return f"An error occurred while writing to the file: {e!s}"
|
||||
|
||||
store = self._store
|
||||
|
||||
# Confine the target directory to base_dir so an LLM-chosen directory
|
||||
# cannot reach outside the sandbox. validate_file_path also resolves
|
||||
# symlinks and ".." components.
|
||||
# cannot reach outside the sandbox. The store also resolves symlinks
|
||||
# and ".." components before checking.
|
||||
try:
|
||||
resolved_directory = Path(validate_file_path(directory, self.base_dir))
|
||||
resolved_directory = store.resolve(directory, self.base_dir)
|
||||
except ValueError as e:
|
||||
return "Error: Invalid directory: " + format_sandbox_error(
|
||||
e,
|
||||
@@ -121,31 +132,17 @@ class FileWriterTool(BaseTool):
|
||||
"directory tree.",
|
||||
)
|
||||
|
||||
# Keep filename inside the target directory, blocking "..", absolute
|
||||
# paths and symlink escapes. is_relative_to() compares whole path
|
||||
# components, so it is safe on case-insensitive filesystems and avoids
|
||||
# the "//" prefix edge case. A filepath that resolves to the directory
|
||||
# itself (e.g. an empty filename) is not a valid file target.
|
||||
# Then keep filename inside that directory.
|
||||
try:
|
||||
resolved_filepath = Path(
|
||||
os.path.join(resolved_directory, filename)
|
||||
).resolve()
|
||||
except (OSError, ValueError) as e:
|
||||
# e.g. an embedded null byte, which trips the underlying syscall.
|
||||
return f"Error: Invalid file path: {format_error_for_display(e)}"
|
||||
resolved_filepath = store.resolve_within(resolved_directory, filename)
|
||||
except ValueError as e:
|
||||
return f"Error: Invalid file path — {e!s}"
|
||||
|
||||
display_filepath = format_path_for_display(
|
||||
str(resolved_filepath), str(resolved_directory)
|
||||
)
|
||||
if (
|
||||
not resolved_filepath.is_relative_to(resolved_directory)
|
||||
or resolved_filepath == resolved_directory
|
||||
):
|
||||
return "Error: Invalid file path — the filename must not escape the target directory."
|
||||
display_filepath = store.display(resolved_filepath, resolved_directory)
|
||||
|
||||
# Covers both a missing 'directory' and subdirectories inside 'filename'.
|
||||
try:
|
||||
os.makedirs(resolved_filepath.parent, exist_ok=True)
|
||||
store.ensure_parent(resolved_filepath)
|
||||
except FileExistsError:
|
||||
return (
|
||||
f"Error: Cannot write to {display_filepath} because a file already "
|
||||
@@ -157,13 +154,16 @@ class FileWriterTool(BaseTool):
|
||||
f"{format_error_for_display(e)}"
|
||||
)
|
||||
|
||||
if resolved_filepath.exists() and not overwrite_file:
|
||||
if store.exists(resolved_filepath) and not overwrite_file:
|
||||
return f"File {display_filepath} already exists and overwrite option was not passed."
|
||||
|
||||
mode = "w" if overwrite_file else "x"
|
||||
try:
|
||||
with open(resolved_filepath, mode, encoding=self.encoding) as file:
|
||||
file.write(content)
|
||||
store.write_text(
|
||||
resolved_filepath,
|
||||
content,
|
||||
self.encoding,
|
||||
overwrite=overwrite_file,
|
||||
)
|
||||
except FileExistsError:
|
||||
return f"File {display_filepath} already exists and overwrite option was not passed."
|
||||
except Exception as e:
|
||||
|
||||
279
lib/crewai-tools/tests/file_storage/test_file_store_seam.py
Normal file
279
lib/crewai-tools/tests/file_storage/test_file_store_seam.py
Normal file
@@ -0,0 +1,279 @@
|
||||
"""The file tools must route all I/O through the registered store.
|
||||
|
||||
These tests stand in a store that keeps everything in a dict, with no
|
||||
filesystem behind it at all. If a tool reaches past the seam to `open()` or
|
||||
`os.path` the assertions fail, which is the point: it is the guarantee any
|
||||
non-filesystem store depends on.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import io
|
||||
import posixpath
|
||||
|
||||
from crewai_tools import FileReadTool, FileWriterTool
|
||||
from crewai_tools.file_storage import (
|
||||
FileStore,
|
||||
register_file_store_factory,
|
||||
reset_file_store_factory,
|
||||
resolve_file_store,
|
||||
)
|
||||
from crewai_tools.file_storage.local import LocalFileStore
|
||||
import pytest
|
||||
|
||||
|
||||
class MemoryFileStore:
|
||||
"""A store with no filesystem: paths are keys in a dict.
|
||||
|
||||
Modelled on a remote object store — POSIX-ish paths, a flat namespace, no
|
||||
symlinks, and containment by prefix rather than by `realpath`.
|
||||
"""
|
||||
|
||||
label = "memory"
|
||||
|
||||
def __init__(self, root: str = "/ws") -> None:
|
||||
self.root = root
|
||||
self.files: dict[str, str] = {}
|
||||
self.dirs: set[str] = {root}
|
||||
|
||||
def _abs(self, path: str, base_dir: str | None = None) -> str:
|
||||
base = base_dir or self.root
|
||||
joined = path if path.startswith("/") else posixpath.join(base, path)
|
||||
return posixpath.normpath(joined)
|
||||
|
||||
def normalize(self, path: str, base_dir: str | None = None) -> str:
|
||||
return self._abs(path, base_dir)
|
||||
|
||||
def resolve(self, path: str, base_dir: str | None = None) -> str:
|
||||
resolved = self._abs(path, base_dir)
|
||||
# Confine to base_dir when one is given, else to the store root. A
|
||||
# remote store has to honour base_dir the same way the local one does,
|
||||
# or the tools' sandbox argument would silently mean nothing.
|
||||
root = posixpath.normpath(base_dir) if base_dir else self.root
|
||||
if resolved != root and not resolved.startswith(root.rstrip("/") + "/"):
|
||||
raise ValueError(
|
||||
f"Path '{posixpath.basename(resolved)}' is outside the allowed "
|
||||
f"directory."
|
||||
)
|
||||
return resolved
|
||||
|
||||
def resolve_within(self, directory: str, filename: str) -> str:
|
||||
resolved = self._abs(filename, directory)
|
||||
if resolved == directory or not resolved.startswith(directory + "/"):
|
||||
raise ValueError("the filename must not escape the target directory")
|
||||
return resolved
|
||||
|
||||
def display(self, resolved: str, base: str | None = None) -> str:
|
||||
base = base or self.root
|
||||
if resolved.startswith(base + "/"):
|
||||
return resolved[len(base) + 1 :]
|
||||
return posixpath.basename(resolved)
|
||||
|
||||
def exists(self, resolved: str) -> bool:
|
||||
return resolved in self.files
|
||||
|
||||
def ensure_parent(self, resolved: str) -> None:
|
||||
parent = posixpath.dirname(resolved)
|
||||
if parent in self.files:
|
||||
raise FileExistsError(parent)
|
||||
self.dirs.add(parent)
|
||||
|
||||
def open_text(self, resolved: str, encoding: str):
|
||||
if resolved not in self.files:
|
||||
raise FileNotFoundError(resolved)
|
||||
return io.StringIO(self.files[resolved])
|
||||
|
||||
def write_text(
|
||||
self, resolved: str, content: str, encoding: str, *, overwrite: bool
|
||||
) -> None:
|
||||
if resolved in self.files and not overwrite:
|
||||
raise FileExistsError(resolved)
|
||||
self.files[resolved] = content
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def store():
|
||||
memory = MemoryFileStore()
|
||||
register_file_store_factory(lambda: memory)
|
||||
yield memory
|
||||
reset_file_store_factory()
|
||||
|
||||
|
||||
def test_default_store_is_local():
|
||||
assert isinstance(resolve_file_store(), LocalFileStore)
|
||||
|
||||
|
||||
def test_memory_store_satisfies_the_protocol(store):
|
||||
assert isinstance(store, FileStore)
|
||||
|
||||
|
||||
def test_writer_writes_through_the_store(store, tmp_path, monkeypatch):
|
||||
# cwd is a real, empty directory: nothing may touch it.
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
result = FileWriterTool()._run(
|
||||
filename="report.md", content="# Report", overwrite=True
|
||||
)
|
||||
|
||||
assert "successfully written" in result
|
||||
assert store.files == {"/ws/report.md": "# Report"}
|
||||
assert list(tmp_path.iterdir()) == []
|
||||
|
||||
|
||||
def test_reader_reads_through_the_store(store, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
store.files["/ws/notes.txt"] = "line 1\nline 2\nline 3\n"
|
||||
|
||||
assert FileReadTool()._run(file_path="notes.txt") == "line 1\nline 2\nline 3\n"
|
||||
|
||||
|
||||
def test_round_trip_between_the_two_tools(store, tmp_path, monkeypatch):
|
||||
"""The reader must see what the writer wrote — the asymmetry that makes
|
||||
an ephemeral runtime unusable."""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
FileWriterTool()._run(filename="out/data.csv", content="a,b\n1,2\n", overwrite=True)
|
||||
read_back = FileReadTool()._run(file_path="out/data.csv")
|
||||
|
||||
assert read_back == "a,b\n1,2\n"
|
||||
|
||||
|
||||
def test_line_windows_work_on_a_remote_store(store, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
store.files["/ws/big.log"] = "".join(f"L{i}\n" for i in range(1, 101))
|
||||
|
||||
assert (
|
||||
FileReadTool()._run(file_path="big.log", start_line=3, line_count=2)
|
||||
== "L3\nL4\n"
|
||||
)
|
||||
|
||||
|
||||
def test_store_containment_is_honoured(store, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
result = FileReadTool()._run(file_path="/etc/passwd")
|
||||
|
||||
assert "Invalid file path" in result
|
||||
assert "base_dir" in result
|
||||
|
||||
|
||||
def test_writer_containment_is_honoured(store, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
result = FileWriterTool()._run(
|
||||
filename="../escape.txt", content="x", overwrite=True
|
||||
)
|
||||
|
||||
assert "Error" in result
|
||||
assert store.files == {}
|
||||
|
||||
|
||||
def test_overwrite_false_is_reported_by_the_store(store, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
tool = FileWriterTool()
|
||||
|
||||
assert "successfully written" in tool._run(filename="a.txt", content="one")
|
||||
assert "already exists" in tool._run(filename="a.txt", content="two")
|
||||
assert store.files["/ws/a.txt"] == "one"
|
||||
|
||||
|
||||
def test_directory_that_is_a_file_reports_clearly(store, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
store.files["/ws/notadir"] = "x"
|
||||
|
||||
result = FileWriterTool()._run(
|
||||
filename="f.txt", directory="notadir", content="x", overwrite=True
|
||||
)
|
||||
|
||||
assert "a file already exists where a directory is needed" in result
|
||||
|
||||
|
||||
def test_factory_returning_none_falls_back_to_local():
|
||||
register_file_store_factory(lambda: None)
|
||||
try:
|
||||
assert isinstance(resolve_file_store(), LocalFileStore)
|
||||
finally:
|
||||
reset_file_store_factory()
|
||||
|
||||
|
||||
def test_factory_that_raises_falls_back_to_local(caplog):
|
||||
"""A broken integration must not take file I/O down with it."""
|
||||
|
||||
def boom() -> FileStore | None:
|
||||
raise RuntimeError("backing service unreachable")
|
||||
|
||||
register_file_store_factory(boom)
|
||||
try:
|
||||
assert isinstance(resolve_file_store(), LocalFileStore)
|
||||
finally:
|
||||
reset_file_store_factory()
|
||||
assert "falling back to the local filesystem" in caplog.text
|
||||
|
||||
|
||||
def test_store_is_bound_once_per_tool(store, tmp_path, monkeypatch):
|
||||
"""Swapping the factory mid-run must not move an existing tool's files."""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
tool = FileWriterTool()
|
||||
other = MemoryFileStore(root="/other")
|
||||
register_file_store_factory(lambda: other)
|
||||
|
||||
tool._run(filename="a.txt", content="one", overwrite=True)
|
||||
|
||||
assert store.files == {"/ws/a.txt": "one"}
|
||||
assert other.files == {}
|
||||
|
||||
|
||||
def test_writer_anchors_base_dir_through_the_store(store, tmp_path, monkeypatch):
|
||||
"""base_dir must be normalised by the store, not by os.path.realpath.
|
||||
|
||||
A remote store's paths are not filesystem paths, so anchoring with local
|
||||
semantics would compute a sandbox root that its own resolve()/
|
||||
resolve_within() do not agree with.
|
||||
"""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
tool = FileWriterTool(base_dir="scoped")
|
||||
|
||||
# The memory store roots at /ws, so its normalisation is what must show up.
|
||||
assert tool.base_dir == "/ws/scoped"
|
||||
assert str(tmp_path) not in str(tool.base_dir)
|
||||
|
||||
|
||||
def test_writer_base_dir_confines_writes_under_a_remote_store(
|
||||
store, tmp_path, monkeypatch
|
||||
):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
tool = FileWriterTool(base_dir="scoped")
|
||||
|
||||
assert "successfully written" in tool._run(
|
||||
filename="in.txt", content="x", overwrite=True
|
||||
)
|
||||
assert "/ws/scoped/in.txt" in store.files
|
||||
|
||||
escaped = tool._run(filename="x.txt", directory="/ws/elsewhere", content="x")
|
||||
assert "Error" in escaped
|
||||
|
||||
|
||||
def test_reader_and_writer_agree_on_a_relative_base_dir(store, tmp_path, monkeypatch):
|
||||
"""Both tools have to land on the same sandbox root for the same input."""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
assert FileWriterTool(base_dir="shared").base_dir == (
|
||||
FileReadTool(base_dir="shared").base_dir
|
||||
)
|
||||
|
||||
|
||||
def test_os_error_message_does_not_leak_an_absolute_path(tmp_path, monkeypatch):
|
||||
"""resolve_within wraps OSError, and the writer returns that text verbatim.
|
||||
|
||||
str() on an OSError carries the absolute filename, so it has to be reduced
|
||||
to the reason before it reaches an agent.
|
||||
"""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
result = FileWriterTool()._run(
|
||||
filename="a\x00b.txt", content="x", overwrite=True
|
||||
)
|
||||
|
||||
assert "Error" in result
|
||||
assert str(tmp_path) not in result
|
||||
Reference in New Issue
Block a user