feat(tools): make the file tools' backing store pluggable

[joaomdmoura]

Why

FileReadTool and FileWriterTool do their I/O directly against the local disk. That is correct on a developer's machine, and it is correct within a single run anywhere: the tasks of one execution share a working directory, so a report written by one task is readable by the next.

What it does not survive is the end of a run. Where the runtime is provisioned per execution, the working directory goes away with it, and nothing a crew wrote is reachable afterwards — not by the next run, not by an operator wanting the artifact. A crew that accumulates a dataset across runs, or writes an output meant to be collected later, has no way to do it with these tools today. There is no seam to point them anywhere else, so the only workaround is to stop using them.

This adds that seam. It changes no current behavior.

Earlier revision: #6698 argued this as an intra-execution failure. @gvieira pointed out that tasks in the same execution do share files, and he was right — the durability gap is across runs, not within one. Reframed here rather than defended.

What this adds

A FileStore seam. Both tools now do every path resolution and every read/write through a store, defaulting to LocalFileStore — today's filesystem behavior, moved rather than rewritten. A deployment swaps in a different store with register_file_store_factory.

from crewai_tools.file_storage import register_file_store_factory

register_file_store_factory(lambda: MyDurableStore())   # every file tool built after this uses it

Design notes worth reviewing:

• The store owns containment. The tools call nothing else before doing I/O, so resolve() / resolve_within() must reject anything out of bounds. Locally 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. The reader pins the file declared at construction so a later chdir can't repoint it — that needs canonicalization without a containment check, which normalize() provides.
• open_text() returns a handle, not a string. That keeps the local store lazy, so start_line/line_count still reads a window out of a huge file without pulling it into memory. Stores that must fetch eagerly wrap the payload in StringIO.
• The store binds once per tool. Swapping the factory mid-run must not move where an existing tool's paths point.
• Derivation happens in model_post_init, not __init__. BaseTool._resolve_tool_dict rebuilds a serialized tool with model_validate, which skips __init__. Anything derived only there comes back missing — see below.

Safety

Three fallbacks keep this inert until a store is registered, and keep a broken integration from taking file I/O down:

• A factory returning None (its backing service isn't configured) → local filesystem.
• A factory that raises → local filesystem, with a warning logged. Degrading to today's behavior beats breaking every file tool in the process.
• A store that raises at use → the tool returns its error string, as it does for every other failure. A store can fail where a local disk cannot (unreachable endpoint, size ceiling), and that must not raise into the agent's step.

Review round

Two defects the bots found, both only reachable once a non-local store is registered — which is exactly why the local-filesystem tests could not see them:

Reader lost its store on reconstruction (Cursor, High). _store was bound in __init__ only, and model_validate skips __init__, so a rebuilt reader came back with _store as None and raised AttributeError on the first read — a regression, since before this branch it called validate_file_path/open directly and worked. Binding moved to model_post_init, along with the declared path, its label and the generated description, so a reconstructed reader is indistinguishable from a fresh one. The writer already did this correctly; the two now match.

Store failures escaped _run (Copilot ×2, CodeRabbit). Every boundary caught ValueError alone, while the protocol explicitly sanctions FileStoreError for failures the local filesystem cannot have. Both tools now wrap the whole operation, which also covers exists() and the store's own display() — neither of which had a handler at all. The established messages stay on their specific boundaries, so nothing an existing test asserts has moved.

The declared-path pin across a rebuild (Cursor, on the fix above). Moving derivation into model_post_init meant _declared_realpath is re-derived from the serialized file_path, and Bugbot spotted that this can repoint the declared default. Traced to exactly one case of three: an absolute file_path, or a relative one with base_dir set, names the same file after a rebuild; a bare relative file_path re-anchors to the rebuilding process's cwd. I kept the re-anchor deliberately — a bare relative path names nothing absolute to preserve, and pinning a directory that no longer exists in a fresh container is the worse failure — and it is not a regression in any case, since before this branch a rebuilt reader lost the declared file outright. All three cases are now tests and the class docstring says which is which. Two stronger fixes exist (normalize file_path in place; carry the pin in a serialized field); both are contract or schema changes, so they're written up on the thread rather than taken unilaterally. Decided: keeping the re-anchor; the two stronger fixes were a public-contract change and a schema change respectively, neither worth the one case they close. Thread resolved.

Construction-time store failures (Copilot ×2, on the fix above). model_post_init runs during model_validate too, so a store raising while normalizing could stop a serialized crew from loading. Copilot suggested falling back to the local store; I didn't, because for this feature that swaps a loud transient failure for silent permanent data loss — a crew configured for durable storage would quietly start writing to a disk that gets discarded. Instead: the protocol now states that normalize/display must be pure and non-raising (both real stores already comply — CdoFileStore does posixpath arithmetic and never touches its client), the reader's declared-file derivation is guarded because a crew failing to load over a convenience default filename is indefensible, and base_dir anchoring stays unguarded on purpose because it is a containment guarantee and silently leaving the root relative would let a later chdir move the sandbox. Convenience degrades, guarantees don't.

One thing I did not change: a bare OSError("...") from a store still reports as its type rather than its message, because format_error_for_display passes only strerror through. That is #6692 holding a line worth holding — an OS-populated OSError renders its absolute filename into str(). Stores wanting a legible message should raise FileStoreError, whose text is preserved. Widening a security redaction from this PR seemed the wrong trade; happy to revisit separately.

Testing

104 pass across the file tools, 40 of them in the new seam suite (from 21 at open), and every pre-existing file tool test is untouched — the refactor is behavior-preserving, which is the main thing to check.

The new tests stand in a store backed by a plain dict with no filesystem behind it. That's what proves the seam is real: a tool that reached past it to open() or os.path would fail them. They assert the writer leaves the real cwd empty, that a file written by one tool is readable by the other (the round-trip that breaks across runs), that line windows work against a non-filesystem store, that containment is honoured in both directions, that both tools derive the same sandbox root from the same base_dir, and that an OSError message never carries an absolute path.

This round adds: reconstruction through both model_validate and a full model_dump round-trip (reader and writer), a store failing at each of resolve / resolve_within / display / exists / ensure_parent / open_text / write_text, the redaction holding on the new failure path, and the three declared-path-across-a-rebuild cases above. Those last ones deliberately use the local store: the memory store's normalize never consults the process cwd, which is precisely why the earlier round-trip coverage could not see the issue.

Rebased onto current main. Two red checks are pre-existing and unrelated, both verified against a clean origin/main checkout: tests trips a different pre-existing flake per run — tests/llms/test_tool_call_streaming.py (4 identical failures on bare main) on one run, and the test_trace_enable_disable VCR cassette exhaustion on the next, which is the same flake this PR hit back in July before any of these changes and which passes locally on both this branch and clean main, and Vulnerability Scan fails on dependency advisories (aiohttp, and nltk on main's own last run) — this PR adds no dependencies and touches no lockfile, so it cannot have introduced them. The advisory floors belong in their own PR per repo convention. ruff, ruff format and mypy clean on the touched files. tool.specs.json picks up the expanded class docstring — the generate-specs job committed that as chore: update tool specifications, and a local regenerate is now a no-op against it. The two suites that fail locally (test_oxylabs_tools needs an extra CI installs, test_mongodb_vector_search_tool reads from stdin) fail identically with this branch stashed.

🤖 Generated with Claude Code

---

Note

Medium Risk
Refactors security-sensitive path sandboxing and all file I/O behind a new abstraction; default behavior is intended to be unchanged, but misconfigured custom stores or serialization edge cases could affect where agents read/write.

Overview
Introduces a FileStore seam so FileReadTool and FileWriterTool can persist reads/writes outside ephemeral local disk without changing crew or tool arguments. Deployments register a factory via register_file_store_factory; unregistered or broken factories still use LocalFileStore (today’s filesystem behavior, largely moved behind the protocol).

Both tools resolve paths, open files, and write through the bound store (once per tool instance). FileReadTool moves declared-path pinning and description generation into model_post_init so model_validate / serialized crews rebuild with a store and default file semantics intact. Store FileStoreError / OSError failures are returned as agent-visible strings instead of aborting the step.

Adds a dict-backed test store and a broad seam test suite (round-trips, containment, reconstruction, factory fallbacks, failure paths). tool.specs.json picks up the expanded FileReadTool docstring.

^{Reviewed by Cursor Bugbot for commit c870f96. Bugbot is set up for automated code reviews on this repo. Configure here.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

The pull request adds a pluggable FileStore contract, a sandboxed local implementation, and a registry with local fallback behavior. FileReadTool and FileWriterTool now use the store seam for path handling and text I/O, with in-memory tests covering routing, containment, fallback, and error behavior.

File storage seam

Layer / File(s)	Summary
Storage contract and local backend lib/crewai-tools/src/crewai_tools/file_storage/*	Defines the FileStore protocol, exports the storage API, and implements sandboxed local path resolution and text I/O.
Store registration and fallback lib/crewai-tools/src/crewai_tools/file_storage/registry.py	Adds thread-safe factory registration and fallback to LocalFileStore when no usable configured store is available.
Reader and writer integration lib/crewai-tools/src/crewai_tools/tools/file_read_tool/file_read_tool.py, lib/crewai-tools/src/crewai_tools/tools/file_writer_tool/file_writer_tool.py	Binds a store per tool instance and delegates normalization, containment, display, reading, directory preparation, and writing to it.
In-memory seam and behavior validation lib/crewai-tools/tests/file_storage/test_file_store_seam.py	Adds MemoryFileStore tests for tool routing, round trips, line reads, containment, overwrite handling, fallback, store binding, shared bases, and sanitized errors.

Sequence Diagram(s)

sequenceDiagram
  participant FileWriterTool
  participant FileStore
  participant FileReadTool
  FileWriterTool->>FileStore: resolve and resolve_within target
  FileWriterTool->>FileStore: ensure_parent and write_text
  FileReadTool->>FileStore: normalize and resolve path
  FileReadTool->>FileStore: open_text resolved file
  FileStore-->>FileReadTool: text content

Loading

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely identifies the main change: making the file tools' backing store pluggable.
Description check	✅ Passed	The description directly explains the pluggable FileStore implementation, fallback behavior, safety design, and test coverage.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/file-tools-pluggable-store

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

This PR introduces a pluggable FileStore seam so FileReadTool / FileWriterTool can route all path resolution and I/O through a durable backing store (while defaulting to the current local-filesystem behavior via LocalFileStore).

Changes:

• Added a FileStore protocol plus a LocalFileStore implementation and a process-wide registry (register_file_store_factory / resolve_file_store).
• Updated FileReadTool and FileWriterTool to bind a store once per tool instance and perform all resolve/read/write operations through it.
• Added dict-backed tests proving file tools do not bypass the store seam and verifying sandbox/containment and fallback behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
lib/crewai-tools/tests/file_storage/test_file_store_seam.py	Adds seam-focused tests using an in-memory store to prove all I/O routes through FileStore.
lib/crewai-tools/src/crewai_tools/tools/file_writer_tool/file_writer_tool.py	Refactors writer to resolve paths and write via a bound FileStore instance.
lib/crewai-tools/src/crewai_tools/tools/file_read_tool/file_read_tool.py	Refactors reader to resolve/pin paths and read via a bound FileStore instance.
lib/crewai-tools/src/crewai_tools/file_storage/registry.py	Adds the global factory registry with safe fallbacks to the local store.
lib/crewai-tools/src/crewai_tools/file_storage/local.py	Implements LocalFileStore preserving existing sandboxed filesystem behavior.
lib/crewai-tools/src/crewai_tools/file_storage/base.py	Defines the FileStore protocol and FileStoreError.
lib/crewai-tools/src/crewai_tools/file_storage/init.py	Exposes the new storage seam API surface from the package.

Comments suppressed due to low confidence (1)

lib/crewai-tools/src/crewai_tools/tools/file_writer_tool/file_writer_tool.py:139

• store.resolve_within(...) is only caught as ValueError. If a custom FileStore raises OSError/FileStoreError (e.g. invalid key syntax, backend failure), _run will raise instead of returning a structured error message.

        # Then keep filename inside that directory.
        try:
            resolved_filepath = store.resolve_within(resolved_directory, filename)
        except ValueError as e:
            return f"Error: Invalid file path — {e!s}"

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@lib/crewai-tools/src/crewai_tools/tools/file_writer_tool/file_writer_tool.py`:
- Around line 121-139: Handle FileStoreError at every identified file-store
boundary, returning the same sanitized file-tool error format instead of
allowing it to escape. In file_writer_tool.py ranges 121-139 and 144-158, add
handling around store.resolve, store.resolve_within, store.ensure_parent, and
unguarded store.exists; in file_read_tool.py range 166-178, cover _resolve_path
and the read I/O boundary. Preserve existing handling for other exception types
and use each error’s details in the returned message.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fb209830-11e6-43ab-a681-8b0101074b91

📥 Commits

Reviewing files that changed from the base of the PR and between f15844b and 0609959.

📒 Files selected for processing (7)

• lib/crewai-tools/src/crewai_tools/file_storage/__init__.py
• lib/crewai-tools/src/crewai_tools/file_storage/base.py
• lib/crewai-tools/src/crewai_tools/file_storage/local.py
• lib/crewai-tools/src/crewai_tools/file_storage/registry.py
• lib/crewai-tools/src/crewai_tools/tools/file_read_tool/file_read_tool.py
• lib/crewai-tools/src/crewai_tools/tools/file_writer_tool/file_writer_tool.py
• lib/crewai-tools/tests/file_storage/test_file_store_seam.py

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (3)

lib/crewai-tools/src/crewai_tools/tools/file_writer_tool/file_writer_tool.py:126

• _run() only catches (FileStoreError, OSError). If a non-local store (or its dependencies) raises a different exception type during resolve()/display()/exists()/ensure_parent() (e.g. TimeoutError, RuntimeError), it will still propagate and can abort the agent step instead of returning an agent-visible error string as intended for store failures.

        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)}"
            )

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

• _run() only catches (FileStoreError, OSError). If a non-local store raises another exception type during path resolution or labelling (e.g. TimeoutError, RuntimeError), it can still propagate and abort the agent step rather than returning an agent-visible error string.

        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)}"
            )

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

• line_count = line_count or None treats line_count=0 as “read the entire file” (because it becomes None). If callers/LLMs pass 0, the tool will unexpectedly read everything instead of returning an empty result.

        start_line = start_line or 1
        line_count = line_count or None

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (3)

lib/crewai-tools/src/crewai_tools/file_storage/base.py:68

• Same as resolve(): resolve_within() ValueError messages can be surfaced to the agent by FileWriterTool, so the protocol should require these messages to be safe for agent-visible output (no absolute/store-internal prefixes).

        Raises:
            ValueError: If *filename* escapes *directory*, or names the
                directory itself.
        """

lib/crewai-tools/src/crewai_tools/tools/file_writer_tool/file_writer_tool.py:162

• resolve_within() ValueError text is returned verbatim to the agent. For non-local stores this message is untrusted and can accidentally include absolute/store-internal path prefixes (similar to why OSError is redacted), and it also makes the user-facing error wording depend on store implementation details. Prefer a stable, non-leaky message here and keep details for errors that are explicitly sanitized (e.g., OSError via format_error_for_display) or surfaced via FileStoreError.

        try:
            resolved_filepath = store.resolve_within(resolved_directory, filename)
        except ValueError as e:
            return f"Error: Invalid file path — {e!s}"

lib/crewai-tools/src/crewai_tools/file_storage/base.py:47

• The tools surface ValueError messages from resolve() directly (e.g., via format_sandbox_error(str(error))). To prevent accidental leakage of absolute/store-internal prefixes from third-party stores, the protocol should explicitly require that ValueError messages are safe for agent-visible output.

This issue also appears on line 65 of the same file.

        Raises:
            ValueError: If the path falls outside what the store allows.
        """

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (3)

lib/crewai-tools/src/crewai_tools/file_storage/base.py:63

• The FileStore protocol doc for normalize() says it must do “no I/O … and no raising”, but the default LocalFileStore.normalize() uses os.path.realpath()/os.getcwd() (filesystem-dependent) and can raise for invalid paths (e.g., embedded NUL). This makes the protocol contract inaccurate and could mislead third-party store implementers.

        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

lib/crewai-tools/src/crewai_tools/file_storage/registry.py:74

• resolve_file_store() assumes the factory returns a valid FileStore. If an integration returns the wrong object type (a realistic “broken integration” case), the tools will hit AttributeError later (not caught by the current (FileStoreError, OSError) wrappers) and the agent step can crash. Consider validating the returned object against the runtime-checkable FileStore protocol and falling back to the local store with a warning.

    if store is None:
        return _local
    return store

lib/crewai-tools/src/crewai_tools/file_storage/base.py:85

• Similarly, the FileStore.display() doc says it must be “pure” for the same reason as normalize(), but the local implementation delegates to format_path_for_display(), which calls os.getcwd()/os.path.realpath(). The guarantee that matters here is “no backend calls” and “no leaking absolute prefixes”, not “no I/O”.

        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.
        """

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 953e1b9. Configure here.}

[joaomdmoura]

On the two red checks — both pre-existing, neither from this PR

Flagging this so the ❌ doesn't stall review. Every change in this PR is under lib/crewai-tools/; no lib/crewai test, no dependency, and no lockfile is touched.

tests — a different test has failed on each run, all of them network/VCR-dependent tests in lib/crewai:

run	failing test
30411470317 (Jul 29, before any of this round's changes)	tracing/test_trace_enable_disable.py::test_trace_calls_when_enabled_via_env — CannotOverwriteExistingCassetteException, cassette already consumed
30857407129	llms/test_tool_call_streaming.py — 3 failures, "Should receive tool call streaming events"
30858876378	tracing/test_trace_enable_disable.py::test_trace_calls_when_enabled_via_env again
30859563355	utilities/test_events.py::test_llm_emits_call_failed_event — "DID NOT RAISE"

Each one passes locally on this branch and on a clean origin/main checkout — I ran both sides for all three. The test_tool_call_streaming failures reproduce identically on bare main. The shape (VCR cassette exhaustion, ConnectionError: Failed to connect to OpenAI API, an expected-exception test not raising) is consistent with cassette/network state under xdist, not with a code defect. Note also that the four tests (3.x) entries showing cancel are fail-fast cancelling siblings, not four independent failures — exactly one job genuinely fails per run.

pip-audit — dependency advisories. Already failing on main independently (nltk==3.9.4 on main's last run, aiohttp==3.14.1 here as newer advisories land). This PR adds no dependencies, so it cannot have introduced them; the floors belong in their own PR per the repo's convention.

The checks that do cover this PR's code — lint, lint-run, type-checker on 3.10–3.13, generate-specs, CodeQL — are all green, and the file-tool suites pass locally at 104 tests.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (1)

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

• In model_post_init, the declared-file derivation is meant to be a convenience that degrades rather than preventing a crew/tool from loading. Catching only (FileStoreError, OSError) still allows other store/implementation failures (e.g., ValueError, TypeError, RuntimeError) from normalize()/display() to propagate during construction / model_validate, which can still break reconstruction despite the intended safety behavior.

Consider broadening this handler to except Exception so any unexpected store failure while deriving the default file results in “no default file” + warning, instead of an exception at load time.

            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):