fix(environment): prevent truncation on invalid file content - #7319
Open
iarjunganesh wants to merge 1 commit into
Open
iarjunganesh wants to merge 1 commit into
iarjunganesh wants to merge 1 commit into
Conversation
Validate content type and encode text before opening the target so invalid model output cannot truncate an existing file. Signed-off-by: Arjun Ganesh <iarjunganesh@gmail.com>
9 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to Issue or Description of Change
Related: #7316 addresses a separate
EditFileTooldecode/write path. This PRfixes the
LocalEnvironmentwrite path used byWriteFileTooland other callers.Problem:
LocalEnvironment._sync_writeopens an existing file before Pythonencodes model-supplied text as UTF-8. A lone surrogate raises
UnicodeEncodeErrorafter the file has been truncated. An unsupported contenttype can similarly fail after the binary file is opened.
WriteFileToolreportsan error, but the original file bytes are already gone.
Reproduction: Write
b'keep these bytes\n'to an existing file, then callwrite_filewith"replacement \ud800". Before the fix, the call raises andthe file reads back as
b''. The new regression test fails this way onmain.Expected behavior: Invalid write content reports an error without modifying
the existing file.
Solution: Encode text and validate the
str | bytescontent type beforeopening the target. Write the prepared bytes in binary mode. Existing tests
cover text, raw bytes, and explicit CRLF sequences.
Testing Plan
Unit tests:
that existing file bytes remain unchanged.
python -m pytest tests/unittests/environment/test_local_environment.py tests/unittests/tools/environment/test_edit_file_tool.py -q— 18 passed, 2 skipped.tox -e py312suite, run serially on the same code tree: 16,235 passed,85 skipped, 26 xfailed, 2 xpassed.
tox -p 5across Python 3.10–3.14. Each version passed morethan 16,200 tests, but that parallel run was not green for unrelated test
environment reasons described below.
Manual end-to-end test:
Called
WriteFileTool.run_asyncagainst a realLocalEnvironmentcontainingb'original\n', with content"bad\ud800". The tool returnedstatus: errorand the file still contained
b'original\n'.Checks: Applicable pre-commit hooks passed. The Windows runner could not
launch the bash-only
check-new-py-prefixwrapper (/bin/bashabsent); itsunderlying
python scripts/check_new_py_files.py --new-dir .check passed.The initial temporary Linux archive gave
check_new_py_files.shCRLF endingseven though its Git blob is LF. Its two shell-wrapper tests passed in all five
Python versions after restoring the exact Git blob. Parallel contention caused
intermittent deployment-test failures; the complete deployment test file then
passed serially in Python 3.10, 3.11, and 3.13 (68 tests in each). Ubuntu's
system Python 3.14 preloads
sitecustomize, causing two import allowlist testfailures; the full import-loading file passed with uv-managed Python 3.14.7
(15 tests).
Checklist
CONTRIBUTING.mdand reviewed the change.