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) diff --git a/src/kimi_cli/tools/file/replace.py b/src/kimi_cli/tools/file/replace.py index 4f551de4f4..f92781a065 100644 --- a/src/kimi_cli/tools/file/replace.py +++ b/src/kimi_cli/tools/file/replace.py @@ -131,6 +131,30 @@ 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 "\ufffd" in content: + raw_bytes = await p.read_bytes() + try: + raw_bytes.decode("utf-8") + except UnicodeDecodeError as decode_error: + return ToolError( + message=( + f"`{params.path}` is not valid UTF-8 " + 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." + ), + 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..b95ab08cbd 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 \ufffd 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 \ufffd 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()