Skip to content

fix(core): route read_file content through FileSystemService - #29110

Open
Abdullah-Builds wants to merge 3 commits into
google-gemini:mainfrom
Abdullah-Builds:fix/read-file-honor-fs-capability
Open

fix(core): route read_file content through FileSystemService#29110
Abdullah-Builds wants to merge 3 commits into
google-gemini:mainfrom
Abdullah-Builds:fix/read-file-honor-fs-capability

Conversation

@Abdullah-Builds

Copy link
Copy Markdown

Summary

read_file reads file contents directly off local disk, ignoring the FileSystemService injected into it — unlike write_file and replace, which already route their I/O through config.getFileSystemService(). This means a client connected over ACP that advertises fs: { readTextFile: true } never sees an fs/read_text_file request for read_file; the CLI silently returns raw on-disk bytes instead. The same gap causes SandboxedFileSystemService to be bypassed for reads. This PR routes read_file's content read through the injected service so it's consistent with the other file tools.

Details

processSingleFileContent (called by read-file.ts) received a FileSystemService parameter but never used it — it was prefixed _fileSystemService and content was read via fs.promises.readFile/readFileWithEncoding regardless of what service was configured.

Added a readTextFileContent() helper that:

  • Calls fileSystemService.readTextFile(filePath) for any non-default service, so an ACP client's fs/read_text_file or a sandboxed service is honored, matching write_file/replace.
  • Falls back to the existing BOM-aware readFileWithEncoding reader specifically for the default StandardFileSystemService, so UTF-8/16/32 byte-order-mark handling for plain local reads isn't regressed (the default StandardFileSystemService.readTextFile uses a plain fs.readFile(..., 'utf-8'), which doesn't strip/decode BOMs).

Existence/size/type checks (fs.existsSync, fs.promises.stat, detectFileType) are left as direct local fs calls — this mirrors what write_file's own validation already does (fs.existsSync/fs.lstatSync for its directory check), and the ACP fs capability only covers text content, not stat metadata.

Related Issues

Fixes #29108

How to Validate

  1. cd packages/core && npx vitest run src/utils/fileUtils.test.ts src/tools/read-file.test.ts src/tools/write-file.test.ts src/services/fileSystemService.test.ts — all 181 tests pass, including 2 new regression tests:
    • one asserting processSingleFileContent calls a mock client-backed FileSystemService.readTextFile() rather than reading local disk directly
    • one asserting the default StandardFileSystemService path still strips a UTF-8 BOM correctly
  2. Manual ACP check: run gemini --acp with a client that advertises fs: { readTextFile: true, writeTextFile: true } and trace the wire frames while the model calls read_file on a path within the workspace root — an fs/read_text_file request should now appear (previously none did), matching the frames already seen for write_file/replace.
  3. Regression check for the non-ACP default case: read_file a UTF-8/UTF-16/UTF-32-BOM-prefixed file locally and confirm the BOM is still stripped from the returned content, same as before this change.

Pre-Merge Checklist

  • Updated relevant documentation and README (if needed)
  • Added/updated tests (if needed)
  • Noted breaking changes (if any) — none
  • Validated on required platforms/methods:
    • MacOS
      • npm run
      • npx
      • Docker
      • Podman
      • Seatbelt
    • Windows
      • npm run
      • npx
      • Docker
    • Linux
      • npm run
      • npx
      • Docker

@Abdullah-Builds
Abdullah-Builds requested a review from a team as a code owner August 27, 2026 19:30
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves an inconsistency where read_file bypassed the injected FileSystemService, preventing ACP clients and sandboxed environments from properly intercepting file reads. By routing these operations through the service layer while preserving BOM-aware reading for standard local access, the change ensures consistent I/O behavior across all file-related tools.

Highlights

  • FileSystemService Integration: Updated read_file to route content reads through the injected FileSystemService instead of reading directly from the local disk.
  • BOM-Aware Fallback: Introduced readTextFileContent to ensure that the default StandardFileSystemService maintains existing behavior by using a BOM-aware reader for local files.
  • ACP and Sandbox Support: Ensured that ACP clients and sandboxed environments can now correctly intercept and serve file read requests, consistent with write_file and replace tools.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@github-actions github-actions Bot added the size/m A medium sized PR label Aug 27, 2026
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown

📊 PR Size: size/L

  • Lines changed: 276
  • Additions: +261
  • Deletions: -15
  • Files changed: 8

@google-cla

google-cla Bot commented Aug 27, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates processSingleFileContent in packages/core/src/utils/fileUtils.ts to route text and SVG file reads through the provided FileSystemService instead of always reading directly from the local disk. This allows non-standard implementations (like sandboxed or client-backed services) to handle text reads. A helper function readTextFileContent is introduced to maintain BOM-aware reading for the standard file system service, and corresponding unit tests are added. However, a critical security and correctness issue was identified: binary file types (audio, image, PDF, video) still bypass the FileSystemService and read directly from the disk using fs.promises.readFile. This creates a potential sandbox bypass vulnerability and a correctness bug for remote/virtual clients. It is recommended to extend FileSystemService to support binary reads and route all file types through it.

Comment thread packages/core/src/utils/fileUtils.ts
@gemini-cli gemini-cli Bot added the area/agent Issues related to Core Agent, Tools, Memory, Sub-Agents, Hooks, Agent Quality label Aug 27, 2026
@Abdullah-Builds

Copy link
Copy Markdown
Author

@mbleigh waiting for approval

@rsloane82-create

rsloane82-create commented Aug 30, 2026 via email

Copy link
Copy Markdown

@rsloane82-create

rsloane82-create commented Aug 30, 2026 via email

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/agent Issues related to Core Agent, Tools, Memory, Sub-Agents, Hooks, Agent Quality size/l A large sized PR size/m A medium sized PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

read_file ignores the client's fs/read_text_file capability, unlike write_file and replace

2 participants