Skip to content

EditFileTool permanently destroys non-UTF-8 bytes outside the edited region #7314

Description

@1wos

🔴 Required Information

Describe the Bug:

EditFileTool reads the target file with errors='replace' and then writes the
entire decoded string back. Any byte in the file that is not valid UTF-8 is
replaced by U+FFFD and that replacement is persisted to disk, even when it sits
in a line the edit never touches. The tool reports status: ok, so neither the
model nor the user learns that bytes were replaced.

# src/google/adk/tools/environment/_edit_file_tool.py
104:  content = data_bytes.decode('utf-8', errors='replace')   # loss happens here
...
133:  await self._environment.write_file(path, new_content)    # loss is persisted here

ReadFileTool decodes the same lossy way, but ReadFileTool only changes what
the model is shown and leaves the file on disk alone. EditFileTool is the only
tool that writes the lossy decode back.

Steps to Reproduce:

  1. pip install google-adk
  2. Save the snippet under "Minimal Reproduction Code" as repro.py
  3. Run python repro.py
  4. Compare the before: and after: byte strings

Expected Behavior:

Replacing OLD with NEW changes only that text. The latin-1 byte 0xE9 on
line 1 is outside the edited region and should still be present afterwards:

after : b'# caf\xe9 note\nNEW\n'

Observed Behavior:

0xE9 is rewritten as the UTF-8 encoding of U+FFFD (EF BF BD) and the original
byte is unrecoverable. The tool still reports success:

before: b'# caf\xe9 note\nOLD\n'
result: {'status': 'ok', 'message': 'Edited notes.py'}
after : b'# caf\xef\xbf\xbd note\nNEW\n'

The write-back at line 133 fails a second way. If new_string cannot be encoded,
open(path, 'w') truncates the file before the encode raises, and the file is
left empty:

before: b'# caf\xe9 note\nOLD\n'
raised: UnicodeEncodeError
after : b''

Replacing OLD with a lone surrogate destroys the whole file, not one byte of it.

Environment Details:

  • ADK Library Version: 2.10.0 (reproduced on main @ 044a1ec)
  • Desktop OS: macOS
  • Python Version: 3.12.12

Model Information:

  • Are you using LiteLLM: No
  • Which model is being used: N/A — the defect is in the tool's file I/O and does
    not involve a model call.

🟡 Optional Information

Regression: No. The decode/write-back pair has been in place since the
environment toolset was introduced in 9082b9e3 (2026-03-27), where it sat at
_tools.py:346 and :369; 1cc298ed (2026-05-26) moved it into
_edit_file_tool.py unchanged.

Minimal Reproduction Code:

import asyncio, tempfile
from pathlib import Path
from google.adk.environment._local_environment import LocalEnvironment
from google.adk.tools.environment._edit_file_tool import EditFileTool

async def main():
    with tempfile.TemporaryDirectory() as tmp:
        env = LocalEnvironment(working_dir=Path(tmp))
        await env.initialize()
        target = Path(tmp) / "notes.py"

        # A file with one latin-1 byte in a line the edit never touches.
        target.write_bytes(b"# caf\xe9 note\nOLD\n")
        print("before:", target.read_bytes())

        result = await EditFileTool(env).run_async(
            args={"path": "notes.py", "old_string": "OLD", "new_string": "NEW"},
            tool_context=None,
        )
        print("result:", result)
        print("after :", target.read_bytes())
        await env.close()

asyncio.run(main())

How often has this issue occurred?: Always (100%)

Additional Context:

BaseEnvironment.write_file already accepts str | bytes
(_base_environment.py:125) and LocalEnvironment._sync_write already has a
'wb' branch (_local_environment.py:236-243), so decoding with
errors='surrogateescape' and handing bytes back needs no dependency and no new
name. It preserves byte fidelity the same way this code path already preserves
CRLF via newline=''.

The order matters as much as the codec. Encoding before write_file is called
keeps the zero-byte case from happening, because no file is opened until the
bytes exist. A new_string that still cannot be encoded should return this
tool's own {'status': 'error'} dict rather than raise, since a lone surrogate
reaches the tool from ordinary model output: json.loads('{"x": "\\ud800"}')
produces one, and json is what parses a tool call.

No overlap with the open PRs in this area: #7133 and #7176 touch
_read_file_tool.py alongside _base_environment.py and _local_environment.py,
but not _edit_file_tool.py.

I am happy to send a PR if this is accepted as a bug.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions