mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-08-05 05:51:45 +00:00
Compare commits
8 Commits
codex/llm-
...
feat/file-
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
c870f96f30 | ||
|
|
2018e950b6 | ||
|
|
73d16ac6ba | ||
|
|
953e1b94ab | ||
|
|
1b198531cd | ||
|
|
1c252e2d83 | ||
|
|
b306793f1c | ||
|
|
77fb6bdb69 |
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",
|
||||
]
|
||||
113
lib/crewai-tools/src/crewai_tools/file_storage/base.py
Normal file
113
lib/crewai-tools/src/crewai_tools/file_storage/base.py
Normal file
@@ -0,0 +1,113 @@
|
||||
"""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.
|
||||
|
||||
Must be a pure computation on the string: no I/O, no network, and no
|
||||
raising. The tools call it while a tool is being constructed —
|
||||
including when pydantic rebuilds a serialized crew — so a store that
|
||||
reaches its backing service here turns a transient outage into a crew
|
||||
that cannot load at all. Defer anything that can fail to
|
||||
:meth:`resolve`, :meth:`open_text` or :meth:`write_text`, where the
|
||||
tools report failures instead of propagating them.
|
||||
"""
|
||||
|
||||
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. Pure and non-raising for the same
|
||||
reason as :meth:`normalize` — the reader labels its declared file at
|
||||
construction time.
|
||||
"""
|
||||
|
||||
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,36 +1,18 @@
|
||||
from itertools import islice
|
||||
import os
|
||||
import logging
|
||||
from typing import Any
|
||||
|
||||
from crewai.tools import BaseTool
|
||||
from pydantic import BaseModel, Field, PrivateAttr
|
||||
|
||||
from crewai_tools.file_storage import FileStore, FileStoreError, 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))
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class FileReadToolSchema(BaseModel):
|
||||
@@ -72,6 +54,15 @@ class FileReadTool(BaseTool):
|
||||
construction, so a later chdir cannot repoint it, and it can be addressed
|
||||
either by omitting ``file_path`` or by the label shown in the description.
|
||||
|
||||
That pin is anchored from what was declared, which matters when a tool is
|
||||
rebuilt from a serialized crew in a different working directory. An
|
||||
absolute ``file_path``, or a relative one with ``base_dir`` set, names the
|
||||
same file after the rebuild as before it. A *relative* ``file_path`` with
|
||||
no ``base_dir`` names nothing absolute, so it re-anchors to the working
|
||||
directory of the process doing the rebuilding — the same file the same
|
||||
arguments would have named there. Pass ``base_dir`` when a declared
|
||||
relative path must survive a move.
|
||||
|
||||
Args:
|
||||
file_path (Optional[str]): Path to the file to be read. If provided,
|
||||
this becomes the default file path for the tool.
|
||||
@@ -103,6 +94,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,28 +115,76 @@ class FileReadTool(BaseTool):
|
||||
encoding (str): Text encoding used to decode the file.
|
||||
**kwargs: Additional keyword arguments passed to BaseTool.
|
||||
"""
|
||||
# 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)
|
||||
|
||||
display_path = None
|
||||
# Hand these to pydantic rather than assigning them afterwards, so the
|
||||
# fields are already populated by the time model_post_init derives
|
||||
# everything that depends on them. That hook is the only place the
|
||||
# derivation can live: see its docstring.
|
||||
if file_path is not None:
|
||||
display_path = format_path_for_display(file_path, 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."
|
||||
)
|
||||
kwargs["file_path"] = file_path
|
||||
if base_dir is not None:
|
||||
kwargs["base_dir"] = base_dir
|
||||
kwargs["encoding"] = encoding
|
||||
|
||||
super().__init__(**kwargs)
|
||||
self.file_path = file_path
|
||||
self.base_dir = base_dir
|
||||
self.encoding = encoding
|
||||
self._declared_realpath = (
|
||||
_resolve_against_base(file_path, base_dir)
|
||||
if file_path is not None
|
||||
else None
|
||||
)
|
||||
self._declared_label = display_path
|
||||
|
||||
def model_post_init(self, context: object) -> None:
|
||||
"""Bind the store, then derive everything that depends on it.
|
||||
|
||||
This has to happen here rather than in ``__init__`` because pydantic
|
||||
reconstructs a serialized tool through ``model_validate``, which skips
|
||||
``__init__`` entirely — ``BaseTool._resolve_tool_dict`` does exactly
|
||||
that when a crew carrying this tool is rebuilt. Deriving here means a
|
||||
reconstructed reader binds its store and pins its declared path just
|
||||
like a freshly constructed one, instead of coming back with no store
|
||||
and failing on the first read.
|
||||
"""
|
||||
super().model_post_init(context)
|
||||
|
||||
store = resolve_file_store()
|
||||
self._store = 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.
|
||||
# Deliberately not guarded: anchoring is a containment guarantee, and
|
||||
# quietly leaving the root relative would let a later chdir move the
|
||||
# sandbox. A store that cannot normalize should fail loudly here rather
|
||||
# than hand back a weaker sandbox than the caller asked for.
|
||||
if self.base_dir is not None:
|
||||
self.base_dir = store.normalize(self.base_dir)
|
||||
|
||||
if self.file_path is not None:
|
||||
# Guarded, unlike base_dir above: the declared default is a
|
||||
# convenience, so a store hiccuping while canonicalizing a filename
|
||||
# should not stop a serialized crew from loading. The reader simply
|
||||
# comes back without a default file, and any real problem resurfaces
|
||||
# on the first read, where _run reports it instead of raising.
|
||||
try:
|
||||
self._declared_realpath = store.normalize(self.file_path, self.base_dir)
|
||||
self._declared_label = store.display(
|
||||
self._declared_realpath, self.base_dir
|
||||
)
|
||||
except (FileStoreError, OSError):
|
||||
logger.warning(
|
||||
"the %s store could not resolve the declared file %r; the "
|
||||
"tool will have no default file",
|
||||
getattr(store, "label", "configured"),
|
||||
self.file_path,
|
||||
exc_info=True,
|
||||
)
|
||||
self._declared_realpath = None
|
||||
self._declared_label = None
|
||||
# The description has to come back too. On a rebuild it arrives
|
||||
# from the serialized data already advertising a default file,
|
||||
# so leaving it would promise the LLM something omitting
|
||||
# 'file_path' can no longer deliver — it would call the tool
|
||||
# with no arguments and get "No file path provided". Restoring
|
||||
# the class default keeps what the tool says matched to what it
|
||||
# does.
|
||||
default = type(self).model_fields["description"].default
|
||||
if isinstance(default, str):
|
||||
self.description = default
|
||||
else:
|
||||
self.description = f"A tool that reads file content. The default file is {self._declared_label}, 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."
|
||||
|
||||
def _resolve_path(self, file_path: str) -> str:
|
||||
"""Resolve *file_path* and confirm the tool is allowed to read it.
|
||||
@@ -166,10 +208,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,
|
||||
@@ -178,6 +220,27 @@ class FileReadTool(BaseTool):
|
||||
line_count: int | None = None,
|
||||
) -> str:
|
||||
"""Read a file, or a window of its lines, as text."""
|
||||
try:
|
||||
return self._read(file_path, start_line, line_count)
|
||||
except (FileStoreError, OSError) as e:
|
||||
# A store can fail for reasons the local filesystem never had: an
|
||||
# unreachable endpoint, a rejected request. Every other exit from
|
||||
# this tool is an agent-visible string, and raising here would kill
|
||||
# the agent's step rather than let it react, so this one is too.
|
||||
# Path resolution and the store's own path labelling both live
|
||||
# under here, which is why the message carries no path.
|
||||
return (
|
||||
f"Error: the {self._store.label} store failed. "
|
||||
f"{format_error_for_display(e)}"
|
||||
)
|
||||
|
||||
def _read(
|
||||
self,
|
||||
file_path: str | None,
|
||||
start_line: int | None,
|
||||
line_count: int | None,
|
||||
) -> str:
|
||||
"""Do the read. Wrapped by :meth:`_run`, which reports store failures."""
|
||||
start_line = start_line or 1
|
||||
line_count = line_count or None
|
||||
|
||||
@@ -195,9 +258,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, FileStoreError, 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,
|
||||
@@ -102,6 +111,28 @@ class FileWriterTool(BaseTool):
|
||||
overwrite: str | bool = False,
|
||||
) -> str:
|
||||
"""Write *content* to *filename*, confined to the tool's sandbox."""
|
||||
try:
|
||||
return self._write(filename, content, directory, overwrite)
|
||||
except (FileStoreError, OSError) as e:
|
||||
# A store can fail for reasons the local filesystem never had: an
|
||||
# unreachable endpoint, a size ceiling. Every other exit from this
|
||||
# tool is an agent-visible string, and raising here would kill the
|
||||
# agent's step rather than let it react, so this one is too. It
|
||||
# also covers the calls with no handler of their own — exists() and
|
||||
# the store's own path labelling — which is why it carries no path.
|
||||
return (
|
||||
f"An error occurred while writing to the file: the "
|
||||
f"{self._store.label} store failed. {format_error_for_display(e)}"
|
||||
)
|
||||
|
||||
def _write(
|
||||
self,
|
||||
filename: str,
|
||||
content: str,
|
||||
directory: str | None,
|
||||
overwrite: str | bool,
|
||||
) -> str:
|
||||
"""Do the write. Wrapped by :meth:`_run`, which reports store failures."""
|
||||
directory = directory or "./"
|
||||
|
||||
try:
|
||||
@@ -109,11 +140,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 +154,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 +176,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:
|
||||
|
||||
659
lib/crewai-tools/tests/file_storage/test_file_store_seam.py
Normal file
659
lib/crewai-tools/tests/file_storage/test_file_store_seam.py
Normal file
@@ -0,0 +1,659 @@
|
||||
"""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,
|
||||
FileStoreError,
|
||||
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
|
||||
|
||||
|
||||
class FailingFileStore(MemoryFileStore):
|
||||
"""A store where one named operation fails, the way a remote one can.
|
||||
|
||||
The local filesystem cannot produce most of these — an unreachable
|
||||
endpoint has no local equivalent — so a store that only ever fails the
|
||||
way `open()` does would not exercise the tools' error paths at all.
|
||||
"""
|
||||
|
||||
def __init__(self, failing: str, exc: Exception | None = None) -> None:
|
||||
super().__init__()
|
||||
self.failing = failing
|
||||
self.exc = exc or FileStoreError("workspace endpoint unreachable")
|
||||
|
||||
def _maybe_fail(self, name: str) -> None:
|
||||
if name == self.failing:
|
||||
raise self.exc
|
||||
|
||||
def normalize(self, path: str, base_dir: str | None = None) -> str:
|
||||
self._maybe_fail("normalize")
|
||||
return super().normalize(path, base_dir)
|
||||
|
||||
def resolve(self, path: str, base_dir: str | None = None) -> str:
|
||||
self._maybe_fail("resolve")
|
||||
return super().resolve(path, base_dir)
|
||||
|
||||
def resolve_within(self, directory: str, filename: str) -> str:
|
||||
self._maybe_fail("resolve_within")
|
||||
return super().resolve_within(directory, filename)
|
||||
|
||||
def display(self, resolved: str, base: str | None = None) -> str:
|
||||
self._maybe_fail("display")
|
||||
return super().display(resolved, base)
|
||||
|
||||
def exists(self, resolved: str) -> bool:
|
||||
self._maybe_fail("exists")
|
||||
return super().exists(resolved)
|
||||
|
||||
def ensure_parent(self, resolved: str) -> None:
|
||||
self._maybe_fail("ensure_parent")
|
||||
super().ensure_parent(resolved)
|
||||
|
||||
def open_text(self, resolved: str, encoding: str):
|
||||
self._maybe_fail("open_text")
|
||||
return super().open_text(resolved, encoding)
|
||||
|
||||
def write_text(
|
||||
self, resolved: str, content: str, encoding: str, *, overwrite: bool
|
||||
) -> None:
|
||||
self._maybe_fail("write_text")
|
||||
super().write_text(resolved, content, encoding, overwrite=overwrite)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def store():
|
||||
memory = MemoryFileStore()
|
||||
register_file_store_factory(lambda: memory)
|
||||
yield memory
|
||||
reset_file_store_factory()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def failing_store():
|
||||
"""Register a store whose *named* operation fails; the test picks which."""
|
||||
created: list[FailingFileStore] = []
|
||||
|
||||
def _register(failing: str, exc: Exception | None = None) -> FailingFileStore:
|
||||
broken = FailingFileStore(failing, exc)
|
||||
register_file_store_factory(lambda: broken)
|
||||
created.append(broken)
|
||||
return broken
|
||||
|
||||
yield _register
|
||||
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
|
||||
|
||||
|
||||
# --- reconstruction through pydantic -----------------------------------------
|
||||
#
|
||||
# BaseTool rebuilds a serialized tool with `model_validate` (see
|
||||
# `_resolve_tool_dict`), which skips `__init__` entirely. Anything derived only
|
||||
# in `__init__` comes back missing, and for the store that means every read
|
||||
# raising AttributeError on a None.
|
||||
|
||||
|
||||
def test_reconstructed_reader_reads_through_the_store(store, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
store.files["/ws/notes.txt"] = "reconstructed\n"
|
||||
|
||||
tool = FileReadTool.model_validate({"file_path": "notes.txt"})
|
||||
|
||||
assert tool._store is store
|
||||
assert tool._run() == "reconstructed\n"
|
||||
|
||||
|
||||
def test_reconstructed_writer_writes_through_the_store(store, tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
tool = FileWriterTool.model_validate({"base_dir": "scoped"})
|
||||
|
||||
assert tool._store is store
|
||||
assert tool.base_dir == "/ws/scoped"
|
||||
assert "successfully written" in tool._run(
|
||||
filename="a.txt", content="x", overwrite=True
|
||||
)
|
||||
assert store.files == {"/ws/scoped/a.txt": "x"}
|
||||
|
||||
|
||||
def test_reconstructed_reader_keeps_its_declared_file(store, tmp_path, monkeypatch):
|
||||
"""The declared path is pinned in the hook, so a rebuild pins it too."""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
store.files["/ws/sub/pinned.txt"] = "kept\n"
|
||||
|
||||
fresh = FileReadTool(file_path="sub/pinned.txt")
|
||||
rebuilt = FileReadTool.model_validate({"file_path": "sub/pinned.txt"})
|
||||
|
||||
assert rebuilt._declared_realpath == fresh._declared_realpath == "/ws/sub/pinned.txt"
|
||||
assert rebuilt._declared_label == fresh._declared_label
|
||||
assert rebuilt.description == fresh.description
|
||||
# Addressable by the label the description shows, same as a fresh tool.
|
||||
assert rebuilt._run(file_path=rebuilt._declared_label) == "kept\n"
|
||||
|
||||
|
||||
def test_reader_round_trips_through_model_dump(store, tmp_path, monkeypatch):
|
||||
"""The full serialize/deserialize cycle, not just a hand-built dict."""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
store.files["/ws/notes.txt"] = "dumped\n"
|
||||
|
||||
original = FileReadTool(file_path="notes.txt")
|
||||
rebuilt = FileReadTool.model_validate(original.model_dump())
|
||||
|
||||
assert rebuilt._run() == "dumped\n"
|
||||
|
||||
|
||||
# --- a store that fails while a tool is being built --------------------------
|
||||
#
|
||||
# `normalize` and `display` are specified as pure and non-raising, precisely
|
||||
# because the tools call them during construction — including when pydantic
|
||||
# rebuilds a serialized crew. A store that breaks that contract anyway must not
|
||||
# take a whole crew down with it, but the two derivations are not equal: the
|
||||
# declared default file is a convenience, `base_dir` is a containment
|
||||
# guarantee. So one degrades and one does not.
|
||||
|
||||
|
||||
def test_a_store_failing_on_the_declared_file_still_builds_the_tool(
|
||||
failing_store, tmp_path, monkeypatch, caplog
|
||||
):
|
||||
"""Convenience: no default file rather than no crew."""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
store = failing_store("display")
|
||||
|
||||
tool = FileReadTool(file_path="notes.txt")
|
||||
|
||||
assert tool._declared_realpath is None
|
||||
assert tool._declared_label is None
|
||||
assert "no default file" in caplog.text
|
||||
# Still a working tool — explicit paths go through the store as usual.
|
||||
store.failing = ""
|
||||
store.files["/ws/other.txt"] = "still working\n"
|
||||
assert tool._run(file_path="other.txt") == "still working\n"
|
||||
# And omitting the path reports the missing default rather than raising.
|
||||
assert "No file path provided" in tool._run()
|
||||
|
||||
|
||||
def test_a_rebuild_that_loses_the_pin_stops_advertising_a_default(
|
||||
store, failing_store, tmp_path, monkeypatch
|
||||
):
|
||||
"""What the tool says must match what it does.
|
||||
|
||||
The description is serialized, so on a rebuild it arrives already naming a
|
||||
default file. If the pin is then lost, leaving that text would tell the LLM
|
||||
it can omit `file_path` — and it would get "No file path provided" back.
|
||||
"""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
dumped = FileReadTool(file_path="notes.txt").model_dump()
|
||||
assert "The default file is" in dumped["description"]
|
||||
|
||||
# Same tool, rebuilt against a store that can no longer label the path.
|
||||
failing_store("display")
|
||||
rebuilt = FileReadTool.model_validate(dumped)
|
||||
|
||||
assert rebuilt._declared_label is None
|
||||
assert "The default file is" not in rebuilt.description
|
||||
assert rebuilt.description == FileReadTool.model_fields["description"].default
|
||||
assert "No file path provided" in rebuilt._run()
|
||||
|
||||
|
||||
def test_a_store_failing_on_base_dir_is_not_swallowed(
|
||||
failing_store, tmp_path, monkeypatch
|
||||
):
|
||||
"""Containment: a sandbox root that cannot be anchored must not be faked.
|
||||
|
||||
Degrading here would leave `base_dir` relative, so a later chdir could move
|
||||
the sandbox — a weaker guarantee than the caller asked for, arrived at
|
||||
silently. Failing loudly is the point.
|
||||
"""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
failing_store("normalize")
|
||||
|
||||
with pytest.raises((FileStoreError, OSError)):
|
||||
FileReadTool(base_dir="scoped")
|
||||
|
||||
with pytest.raises((FileStoreError, OSError)):
|
||||
FileWriterTool(base_dir="scoped")
|
||||
|
||||
|
||||
# --- what the declared-path pin means across a rebuild -----------------------
|
||||
#
|
||||
# The pin is derived from what was *declared*, so whether it survives a rebuild
|
||||
# in a different working directory depends on whether the declaration named
|
||||
# somewhere absolute. These three cases are the whole story; they are pinned
|
||||
# here so the behavior is a decision rather than an accident. Note the local
|
||||
# store is the one that makes this observable, since its `normalize` is the one
|
||||
# that consults the process cwd.
|
||||
|
||||
|
||||
def test_an_absolute_declared_path_survives_a_rebuild_elsewhere(tmp_path, monkeypatch):
|
||||
"""The strongest case: an absolute declaration is cwd-independent."""
|
||||
here, there = tmp_path / "here", tmp_path / "there"
|
||||
here.mkdir(), there.mkdir()
|
||||
(here / "notes.txt").write_text("from here\n")
|
||||
(there / "notes.txt").write_text("from there\n")
|
||||
|
||||
monkeypatch.chdir(here)
|
||||
dumped = FileReadTool(file_path=str(here / "notes.txt")).model_dump()
|
||||
monkeypatch.chdir(there)
|
||||
rebuilt = FileReadTool.model_validate(dumped)
|
||||
|
||||
assert rebuilt._run() == "from here\n"
|
||||
|
||||
|
||||
def test_a_declared_base_dir_pins_a_relative_path_across_a_rebuild(tmp_path, monkeypatch):
|
||||
"""base_dir is anchored at construction, so it carries the pin with it."""
|
||||
here, there = tmp_path / "here", tmp_path / "there"
|
||||
here.mkdir(), there.mkdir()
|
||||
(here / "notes.txt").write_text("from here\n")
|
||||
(there / "notes.txt").write_text("from there\n")
|
||||
|
||||
monkeypatch.chdir(here)
|
||||
dumped = FileReadTool(file_path="notes.txt", base_dir=str(here)).model_dump()
|
||||
monkeypatch.chdir(there)
|
||||
rebuilt = FileReadTool.model_validate(dumped)
|
||||
|
||||
assert rebuilt._run() == "from here\n"
|
||||
|
||||
|
||||
def test_a_bare_relative_declared_path_reanchors_on_rebuild(tmp_path, monkeypatch):
|
||||
"""A relative path with no base_dir names nothing absolute to preserve.
|
||||
|
||||
It re-anchors to the rebuilding process's working directory — the same file
|
||||
the same arguments would name there. Documented rather than "fixed": the
|
||||
alternative is pinning a path from a working directory that, for a rebuild
|
||||
in a fresh container, no longer exists. Callers needing the pin to survive
|
||||
pass `base_dir`, which the test above covers.
|
||||
"""
|
||||
here, there = tmp_path / "here", tmp_path / "there"
|
||||
here.mkdir(), there.mkdir()
|
||||
(here / "notes.txt").write_text("from here\n")
|
||||
(there / "notes.txt").write_text("from there\n")
|
||||
|
||||
monkeypatch.chdir(here)
|
||||
dumped = FileReadTool(file_path="notes.txt").model_dump()
|
||||
monkeypatch.chdir(there)
|
||||
rebuilt = FileReadTool.model_validate(dumped)
|
||||
|
||||
assert rebuilt._run() == "from there\n"
|
||||
# And it is a real re-anchor, not a stale absolute path that happens to read.
|
||||
# resolve() because the store canonicalizes, and /tmp is a symlink on macOS.
|
||||
assert rebuilt._declared_realpath == str((there / "notes.txt").resolve())
|
||||
|
||||
|
||||
# --- a store that fails ------------------------------------------------------
|
||||
#
|
||||
# A store may fail where the local filesystem never could. Every exit from
|
||||
# these tools is an agent-visible string, so a store failure has to become one
|
||||
# too rather than raising into the agent's step.
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"failing", ["resolve", "resolve_within", "display", "exists", "ensure_parent"]
|
||||
)
|
||||
def test_writer_reports_a_store_failure_instead_of_raising(
|
||||
failing, failing_store, tmp_path, monkeypatch
|
||||
):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
failing_store(failing)
|
||||
|
||||
result = FileWriterTool()._run(filename="a.txt", content="x", overwrite=True)
|
||||
|
||||
assert "error occurred while writing" in result
|
||||
assert "memory" in result
|
||||
assert "workspace endpoint unreachable" in result
|
||||
|
||||
|
||||
def test_writer_reports_a_failing_write(failing_store, tmp_path, monkeypatch):
|
||||
"""write_text already had a handler; it must keep its own wording."""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
failing_store("write_text")
|
||||
|
||||
result = FileWriterTool()._run(filename="a.txt", content="x", overwrite=True)
|
||||
|
||||
assert "An error occurred while writing to the file" in result
|
||||
assert "workspace endpoint unreachable" in result
|
||||
|
||||
|
||||
@pytest.mark.parametrize("failing", ["resolve", "display"])
|
||||
def test_reader_reports_a_store_failure_instead_of_raising(
|
||||
failing, failing_store, tmp_path, monkeypatch
|
||||
):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
failing_store(failing)
|
||||
|
||||
result = FileReadTool()._run(file_path="notes.txt")
|
||||
|
||||
assert "Error" in result
|
||||
assert "memory" in result
|
||||
assert "workspace endpoint unreachable" in result
|
||||
|
||||
|
||||
def test_reader_reports_a_failing_open(failing_store, tmp_path, monkeypatch):
|
||||
"""open_text already had a handler; it must keep its own wording."""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
failing_store("open_text")
|
||||
|
||||
result = FileReadTool()._run(file_path="notes.txt")
|
||||
|
||||
assert "Failed to read file" in result
|
||||
assert "workspace endpoint unreachable" in result
|
||||
|
||||
|
||||
def _run_either(tool_cls, tool):
|
||||
"""Drive whichever tool this is with its minimal arguments."""
|
||||
if tool_cls is FileReadTool:
|
||||
return tool._run(file_path="notes.txt")
|
||||
return tool._run(filename="a.txt", content="x", overwrite=True)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("tool_cls", [FileReadTool, FileWriterTool])
|
||||
def test_an_os_error_from_the_store_is_reported_too(
|
||||
tool_cls, failing_store, tmp_path, monkeypatch
|
||||
):
|
||||
"""Not every store failure arrives as FileStoreError.
|
||||
|
||||
A store wrapping a socket or a subprocess can raise OSError from resolve,
|
||||
which is neither a ValueError nor something the read/write handlers see.
|
||||
``strerror`` is what survives redaction, so that is what an agent gets.
|
||||
"""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
failing_store("resolve", OSError(104, "Connection reset by peer"))
|
||||
|
||||
result = _run_either(tool_cls, tool_cls())
|
||||
|
||||
assert "Connection reset by peer" in result
|
||||
|
||||
|
||||
@pytest.mark.parametrize("tool_cls", [FileReadTool, FileWriterTool])
|
||||
def test_a_bare_os_error_degrades_to_its_type_without_raising(
|
||||
tool_cls, failing_store, tmp_path, monkeypatch
|
||||
):
|
||||
"""A single-arg OSError has no strerror, so only its type survives.
|
||||
|
||||
That is `format_error_for_display` holding the line it was given in #6692:
|
||||
an OS-populated OSError renders its absolute filename into `str()`, so the
|
||||
message is never passed through wholesale. The cost is a thin report for a
|
||||
hand-raised `OSError("...")`; stores wanting a legible message should raise
|
||||
`FileStoreError`, whose text is preserved. What matters here is that the
|
||||
tool still returns rather than raising.
|
||||
"""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
failing_store("resolve", OSError("connection reset by peer"))
|
||||
|
||||
result = _run_either(tool_cls, tool_cls())
|
||||
|
||||
assert "OSError" in result
|
||||
assert "store failed" in result
|
||||
|
||||
|
||||
def test_a_store_failure_does_not_leak_an_absolute_path(
|
||||
failing_store, tmp_path, monkeypatch
|
||||
):
|
||||
"""The store-failure path is agent-visible, so it gets the same redaction."""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
failing_store("resolve", OSError(2, "No such file", str(tmp_path / "secret.txt")))
|
||||
|
||||
result = FileWriterTool()._run(filename="a.txt", content="x", overwrite=True)
|
||||
|
||||
assert str(tmp_path) not in result
|
||||
@@ -10558,7 +10558,7 @@
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"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\")",
|
||||
"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\nThat pin is anchored from what was declared, which matters when a tool is\nrebuilt from a serialized crew in a different working directory. An\nabsolute ``file_path``, or a relative one with ``base_dir`` set, names the\nsame file after the rebuild as before it. A *relative* ``file_path`` with\nno ``base_dir`` names nothing absolute, so it re-anchors to the working\ndirectory of the process doing the rebuilding \u2014 the same file the same\narguments would have named there. Pass ``base_dir`` when a declared\nrelative path must survive a move.\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\")",
|
||||
"properties": {
|
||||
"base_dir": {
|
||||
"anyOf": [
|
||||
|
||||
Reference in New Issue
Block a user