From 5a06433ca4da10fe969a2c937c6bd5d45f63997b Mon Sep 17 00:00:00 2001 From: Jeremy Schoemaker Date: Thu, 6 Aug 2026 18:16:35 -0500 Subject: [PATCH 1/3] fix(StrReplaceFile): refuse to edit files that are not valid UTF-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit StrReplaceFile decodes the whole file with errors="replace", applies the edit to the string, and writes the whole string back. Any byte in the file that is not valid UTF-8 — including bytes nowhere near the edit — comes back as U+FFFD and is written out as EF BF BD, so the file changes outside the requested edit and the approval diff cannot show it, because the diff is built from the already-lossy string. Detect the lossy decode and return a ToolError instead of writing. The check runs before the approval request, so a corrupting edit is never offered for approval in the first place. U+FFFD in the decoded text is only a symptom: the file may legitimately contain one. The raw bytes are re-read and strictly decoded to tell the two apart, and only when a U+FFFD is present, so a file with no U+FFFD — the overwhelming majority — still costs exactly one read as before. The strict decode is deliberate and is caught rather than propagated, so it cannot panic on malformed UTF-8, which is what the errors="replace" convention in tests_ai/test_encoding_error_handling.md exists to prevent. Fixes #2591 --- src/kimi_cli/tools/file/replace.py | 23 ++++++++++++ tests/tools/test_str_replace_file.py | 56 ++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/src/kimi_cli/tools/file/replace.py b/src/kimi_cli/tools/file/replace.py index 4f551de4f4..6b36e30d3e 100644 --- a/src/kimi_cli/tools/file/replace.py +++ b/src/kimi_cli/tools/file/replace.py @@ -131,6 +131,29 @@ async def __call__(self, params: Params) -> ToolReturnValue: # Read the file content content = await p.read_text(errors="replace") + # This tool reads the whole file, edits the string, and writes the whole + # string back, so every undecodable byte in the file — including bytes + # nowhere near the edit — would come back as U+FFFD and be written out as + # EF BF BD. Refuse rather than silently rewrite bytes the edit never asked + # to touch. A U+FFFD present in the decoded text is only a symptom: it may + # equally be a real U+FFFD stored in the file, so confirm against the raw + # bytes before rejecting. The strict decode below is deliberate and is + # caught, not propagated, so it cannot panic on malformed UTF-8. + if "�" in content: + try: + (await p.read_bytes()).decode("utf-8") + except UnicodeDecodeError as decode_error: + return ToolError( + message=( + f"`{params.path}` is not valid UTF-8 " + f"(byte 0x{decode_error.object[decode_error.start]:02x} at offset " + f"{decode_error.start}). Editing it with StrReplaceFile would " + "replace that byte, and every other undecodable byte in the file, " + "with U+FFFD. No changes were made." + ), + brief="File is not valid UTF-8", + ) + original_content = content edits = [params.edit] if isinstance(params.edit, Edit) else params.edit diff --git a/tests/tools/test_str_replace_file.py b/tests/tools/test_str_replace_file.py index a16dad303b..be1ddc54ac 100644 --- a/tests/tools/test_str_replace_file.py +++ b/tests/tools/test_str_replace_file.py @@ -246,3 +246,59 @@ async def test_replace_empty_strings( assert not result.is_error assert "successfully edited" in result.message assert await file_path.read_text() == "Hello !" + + +async def test_replace_refuses_file_with_undecodable_bytes( + str_replace_file_tool: StrReplaceFile, temp_work_dir: KaosPath +): + """A file that is not valid UTF-8 is left byte-for-byte alone.""" + file_path = temp_work_dir / "invalid.txt" + # The undecodable byte is nowhere near the edit, on a line the edit never mentions. + original_bytes = b"alpha\nbeta \xff gamma\ndelta\n" + await file_path.write_bytes(original_bytes) + + result = await str_replace_file_tool( + Params(path=str(file_path), edit=Edit(old="alpha", new="ALPHA")) + ) + + assert result.is_error + assert "not valid UTF-8" in result.message + # Without the guard the file is rewritten with \xff replaced by \xef\xbf\xbd, + # growing by two bytes on an edit that only asked to touch "alpha". + assert await file_path.read_bytes() == original_bytes + + +async def test_replace_allows_file_containing_real_replacement_character( + str_replace_file_tool: StrReplaceFile, temp_work_dir: KaosPath +): + """U+FFFD stored in the file is legitimate content, not a failed decode.""" + file_path = temp_work_dir / "fffd.txt" + original_content = "alpha\nbeta � gamma\ndelta\n" + await file_path.write_text(original_content) + + result = await str_replace_file_tool( + Params(path=str(file_path), edit=Edit(old="alpha", new="ALPHA")) + ) + + assert not result.is_error + assert await file_path.read_text() == "ALPHA\nbeta � gamma\ndelta\n" + + +async def test_replace_allows_crlf_file( + str_replace_file_tool: StrReplaceFile, temp_work_dir: KaosPath +): + """CRLF files must not be mistaken for undecodable ones. + + Reads translate CRLF to LF, so any detection that compares the decoded text + against the raw bytes would reject every Windows-line-ending file. (That the + write then normalizes the endings to LF is a separate bug, #2191.) + """ + file_path = temp_work_dir / "crlf.txt" + await file_path.write_bytes(b"alpha\r\nbeta\r\n") + + result = await str_replace_file_tool( + Params(path=str(file_path), edit=Edit(old="alpha", new="ALPHA")) + ) + + assert not result.is_error + assert b"ALPHA" in await file_path.read_bytes() From 2ecc6dad1c6d98676b483f15384dc65b20dde4df Mon Sep 17 00:00:00 2001 From: Jeremy Schoemaker Date: Thu, 6 Aug 2026 18:17:16 -0500 Subject: [PATCH 2/3] docs(changelog): note the StrReplaceFile UTF-8 refusal --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e500e09c2..8f4b47934c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Only write entries that are worth mentioning to users. ## Unreleased - Kosong: Stop sending an empty `anthropic-beta` header when no beta features are declared — adaptive thinking removes the interleaved-thinking beta, which previously left an empty header value that some backends reject +- Tools: `StrReplaceFile` now refuses to edit a file that is not valid UTF-8 instead of silently corrupting it — the whole-file round trip replaced every undecodable byte with U+FFFD, anywhere in the file, including bytes far from the edit and invisible in the approval diff ## 1.49.0 (2026-07-16) From e318cc06bad15c71327e52af42a847a6a3218ea5 Mon Sep 17 00:00:00 2001 From: Jeremy Schoemaker Date: Thu, 6 Aug 2026 18:29:28 -0500 Subject: [PATCH 3/3] refactor(StrReplaceFile): use the \ufffd escape and name the raw bytes Matches the existing convention in tests/ui_and_conv/test_prompt_history.py:67 and test_prompt_placeholders.py:153, which both write the sentinel as an escape rather than a literal glyph. Binding read_bytes() to a name also makes it obvious the strict decode and the error message read the same buffer. --- src/kimi_cli/tools/file/replace.py | 7 ++++--- tests/tools/test_str_replace_file.py | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/kimi_cli/tools/file/replace.py b/src/kimi_cli/tools/file/replace.py index 6b36e30d3e..f92781a065 100644 --- a/src/kimi_cli/tools/file/replace.py +++ b/src/kimi_cli/tools/file/replace.py @@ -139,14 +139,15 @@ async def __call__(self, params: Params) -> ToolReturnValue: # equally be a real U+FFFD stored in the file, so confirm against the raw # bytes before rejecting. The strict decode below is deliberate and is # caught, not propagated, so it cannot panic on malformed UTF-8. - if "�" in content: + if "\ufffd" in content: + raw_bytes = await p.read_bytes() try: - (await p.read_bytes()).decode("utf-8") + raw_bytes.decode("utf-8") except UnicodeDecodeError as decode_error: return ToolError( message=( f"`{params.path}` is not valid UTF-8 " - f"(byte 0x{decode_error.object[decode_error.start]:02x} at offset " + f"(byte 0x{raw_bytes[decode_error.start]:02x} at offset " f"{decode_error.start}). Editing it with StrReplaceFile would " "replace that byte, and every other undecodable byte in the file, " "with U+FFFD. No changes were made." diff --git a/tests/tools/test_str_replace_file.py b/tests/tools/test_str_replace_file.py index be1ddc54ac..b95ab08cbd 100644 --- a/tests/tools/test_str_replace_file.py +++ b/tests/tools/test_str_replace_file.py @@ -273,7 +273,7 @@ async def test_replace_allows_file_containing_real_replacement_character( ): """U+FFFD stored in the file is legitimate content, not a failed decode.""" file_path = temp_work_dir / "fffd.txt" - original_content = "alpha\nbeta � gamma\ndelta\n" + original_content = "alpha\nbeta \ufffd gamma\ndelta\n" await file_path.write_text(original_content) result = await str_replace_file_tool( @@ -281,7 +281,7 @@ async def test_replace_allows_file_containing_real_replacement_character( ) assert not result.is_error - assert await file_path.read_text() == "ALPHA\nbeta � gamma\ndelta\n" + assert await file_path.read_text() == "ALPHA\nbeta \ufffd gamma\ndelta\n" async def test_replace_allows_crlf_file(