From ce8c640aae46adc8f559277ce899cc85f23978c5 Mon Sep 17 00:00:00 2001 From: "Gary T. Giesen" Date: Sat, 4 Jul 2026 16:33:38 -0400 Subject: [PATCH 1/2] Fix file state UnicodeDecodeError on encoding mismatch The file.comment, file.append and file.prepend states read the target file as bytes and decode it with the system encoding purely to build a diff. A strict decode raised UnicodeDecodeError and aborted the state when the file contained bytes invalid in that encoding. Decode with errors="replace" since the result is only used for the diff. Fixes #50903 --- changelog/50903.fixed.md | 1 + salt/states/file.py | 14 +++--- tests/pytests/unit/states/file/test_append.py | 48 +++++++++++++++++++ 3 files changed, 56 insertions(+), 7 deletions(-) create mode 100644 changelog/50903.fixed.md create mode 100644 tests/pytests/unit/states/file/test_append.py diff --git a/changelog/50903.fixed.md b/changelog/50903.fixed.md new file mode 100644 index 000000000000..912dd06030c2 --- /dev/null +++ b/changelog/50903.fixed.md @@ -0,0 +1 @@ +Fixed the file.comment, file.append, and file.prepend states failing with UnicodeDecodeError when the target file contains bytes that are not valid in the system encoding. diff --git a/salt/states/file.py b/salt/states/file.py index 96bc5cab9141..3fa4899eceea 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -6306,7 +6306,7 @@ def comment(name, regex, char="#", backup=".bak", ignore_missing=False): with salt.utils.files.fopen(name, "rb") as fp_: slines = fp_.read() - slines = slines.decode(__salt_system_encoding__) + slines = slines.decode(__salt_system_encoding__, errors="replace") slines = slines.splitlines(True) # Perform the edit @@ -6314,7 +6314,7 @@ def comment(name, regex, char="#", backup=".bak", ignore_missing=False): with salt.utils.files.fopen(name, "rb") as fp_: nlines = fp_.read() - nlines = nlines.decode(__salt_system_encoding__) + nlines = nlines.decode(__salt_system_encoding__, errors="replace") nlines = nlines.splitlines(True) # Check the result @@ -6643,7 +6643,7 @@ def append( with salt.utils.files.fopen(name, "rb") as fp_: slines = fp_.read() - slines = slines.decode(__salt_system_encoding__) + slines = slines.decode(__salt_system_encoding__, errors="replace") slines = slines.splitlines() append_lines = [] @@ -6689,7 +6689,7 @@ def append( with salt.utils.files.fopen(name, "rb") as fp_: nlines = fp_.read() - nlines = nlines.decode(__salt_system_encoding__) + nlines = nlines.decode(__salt_system_encoding__, errors="replace") nlines = nlines.splitlines() if slines != nlines: @@ -6928,7 +6928,7 @@ def prepend( with salt.utils.files.fopen(name, "rb") as fp_: slines = fp_.read() - slines = slines.decode(__salt_system_encoding__) + slines = slines.decode(__salt_system_encoding__, errors="replace") slines = slines.splitlines(True) count = 0 @@ -6976,7 +6976,7 @@ def prepend( with salt.utils.files.fopen(name, "rb") as fp_: # read as many lines of target file as length of user input contents = fp_.read() - contents = contents.decode(__salt_system_encoding__) + contents = contents.decode(__salt_system_encoding__, errors="replace") contents = contents.splitlines(True) target_head = contents[0 : len(preface)] target_lines = [] @@ -6995,7 +6995,7 @@ def prepend( with salt.utils.files.fopen(name, "rb") as fp_: nlines = fp_.read() - nlines = nlines.decode(__salt_system_encoding__) + nlines = nlines.decode(__salt_system_encoding__, errors="replace") nlines = nlines.splitlines(True) if slines != nlines: diff --git a/tests/pytests/unit/states/file/test_append.py b/tests/pytests/unit/states/file/test_append.py new file mode 100644 index 000000000000..65c8c166171d --- /dev/null +++ b/tests/pytests/unit/states/file/test_append.py @@ -0,0 +1,48 @@ +import builtins + +import pytest + +import salt.states.file as filestate +from tests.support.mock import MagicMock, patch + + +@pytest.fixture +def configure_loader_modules(): + return { + filestate: { + "__env__": "base", + "__salt__": {}, + "__opts__": {"test": False, "cachedir": ""}, + "__instance_id__": "", + "__low__": {}, + "__utils__": {}, + } + } + + +def test_append_file_encoding_mismatch(tmp_path): + """ + file.append must not raise UnicodeDecodeError when the target file + contains bytes that are not valid in the system encoding. The decoded + contents are only used to build the diff, so undecodable bytes should + be tolerated rather than aborting the state. + + Regression test for #50903. + """ + name = tmp_path / "bugfile" + # 0xed is not valid ASCII and not valid UTF-8 on its own + name.write_bytes(b"abc\xedxyz\n") + + salt_mock = { + "file.search": MagicMock(return_value=False), + "file.append": MagicMock(return_value=None), + } + utils_mock = {"files.is_text": MagicMock(return_value=True)} + + with patch.object(builtins, "__salt_system_encoding__", "ascii"), patch.dict( + filestate.__salt__, salt_mock + ), patch.dict(filestate.__utils__, utils_mock): + result = filestate.append(name=str(name), text="cheese") + + assert result["result"] is True + salt_mock["file.append"].assert_called_once() From e8484bb9bf76f2ea8cae09dfff62f00ba6b8c34a Mon Sep 17 00:00:00 2001 From: "Gary T. Giesen" Date: Sun, 5 Jul 2026 23:24:00 -0400 Subject: [PATCH 2/2] Add direct and inverse regression tests for file state encoding mismatch fix The direct tests call file.comment and file.prepend (the two changed state functions previously untested for this case) with production-shape args (name+regex / name+text, as the state compiler passes from an SLS) against a real file holding a 0xed byte while __salt_system_encoding__ is ascii, pinning the errors="replace" decode fix; both fail with UnicodeDecodeError when the fix is reverted. The inverse test guards against overcorrection: file.append on a cleanly-decodable UTF-8 file must still produce an unmangled diff with no U+FFFD replacement characters, and passes with and without the fix. Claude-Session: https://claude.ai/code/session_01MF2AuQNhBZg4HDt1x6xxCu --- tests/pytests/unit/states/file/test_append.py | 41 +++++++++++++++++++ .../pytests/unit/states/file/test_comment.py | 33 +++++++++++++++ .../pytests/unit/states/file/test_prepend.py | 32 +++++++++++++++ 3 files changed, 106 insertions(+) diff --git a/tests/pytests/unit/states/file/test_append.py b/tests/pytests/unit/states/file/test_append.py index 65c8c166171d..af788fee8165 100644 --- a/tests/pytests/unit/states/file/test_append.py +++ b/tests/pytests/unit/states/file/test_append.py @@ -3,6 +3,7 @@ import pytest import salt.states.file as filestate +import salt.utils.files from tests.support.mock import MagicMock, patch @@ -46,3 +47,43 @@ def test_append_file_encoding_mismatch(tmp_path): assert result["result"] is True salt_mock["file.append"].assert_called_once() + + +def test_append_clean_encoding_unaffected_50903(tmp_path): + """ + Guard against overcorrection of the #50903 fix: decoding with + errors="replace" must not change behaviour for files that decode + cleanly in the system encoding. The diff must still be generated, + contain the original non-ASCII text unmangled, and hold no U+FFFD + replacement characters. This test passes with and without the fix. + """ + name = tmp_path / "cleanfile" + # Valid UTF-8 content that decodes cleanly with the utf-8 system encoding + name.write_bytes("h\u00e9llo\n".encode("utf-8")) + + def fake_append(fname, args=None): + # Perform the real append so the state's second read sees a change + with salt.utils.files.fopen(fname, "a", encoding="utf-8") as fp_: + for line in args: + fp_.write(line + "\n") + + salt_mock = { + "file.search": MagicMock(return_value=False), + "file.append": MagicMock(side_effect=fake_append), + } + utils_mock = {"files.is_text": MagicMock(return_value=True)} + + # Production callers (the state compiler running an SLS file.append) + # pass name and text; __salt_system_encoding__ is the locale-derived + # builtin read by the decode sites, so it is patched rather than passed. + with patch.object(builtins, "__salt_system_encoding__", "utf-8"), patch.dict( + filestate.__salt__, salt_mock + ), patch.dict(filestate.__utils__, utils_mock): + result = filestate.append(name=str(name), text="cheese") + + assert result["result"] is True + assert result["comment"] == "Appended 1 lines" + diff = result["changes"]["diff"] + assert "+cheese" in diff + assert "h\u00e9llo" in diff + assert "\ufffd" not in diff diff --git a/tests/pytests/unit/states/file/test_comment.py b/tests/pytests/unit/states/file/test_comment.py index 6ef8b72de28c..82e67b67337e 100644 --- a/tests/pytests/unit/states/file/test_comment.py +++ b/tests/pytests/unit/states/file/test_comment.py @@ -1,3 +1,4 @@ +import builtins import logging import os @@ -115,6 +116,38 @@ def test_comment(): assert filestate.comment(name, regex) == ret +def test_comment_file_encoding_mismatch_50903(tmp_path): + """ + file.comment must not raise UnicodeDecodeError when the target file + contains bytes that are not valid in the system encoding. The decoded + contents are only used to build the diff, so undecodable bytes should + be tolerated rather than aborting the state. + + Regression test for #50903. + """ + name = tmp_path / "fstab" + # 0xed is not valid ASCII and not valid UTF-8 on its own + name.write_bytes(b"bind 127.0.0.1\nabc\xedxyz\n") + + salt_mock = { + # First search: uncommented pattern found; second: commented after edit + "file.search": MagicMock(side_effect=[True, True]), + "file.comment_line": MagicMock(return_value=True), + } + + # Production callers (the state compiler running an SLS file.comment) + # pass name and regex; __salt_system_encoding__ is the locale-derived + # builtin read by the decode sites, so it is patched rather than passed. + with patch.object(builtins, "__salt_system_encoding__", "ascii"), patch.dict( + filestate.__salt__, salt_mock + ): + result = filestate.comment(str(name), "^bind 127.0.0.1") + + assert result["result"] is True + assert result["comment"] == "Commented lines successfully" + salt_mock["file.comment_line"].assert_called_once() + + # 'uncomment' function tests: 1 def test_uncomment(): """ diff --git a/tests/pytests/unit/states/file/test_prepend.py b/tests/pytests/unit/states/file/test_prepend.py index db528519ec0f..60f66a30eff3 100644 --- a/tests/pytests/unit/states/file/test_prepend.py +++ b/tests/pytests/unit/states/file/test_prepend.py @@ -1,3 +1,4 @@ +import builtins import logging import os @@ -114,3 +115,34 @@ def test_prepend(): comt = "Prepended 1 lines" ret.update({"comment": comt, "result": True, "changes": {}}) assert filestate.prepend(name, text=text) == ret + + +def test_prepend_file_encoding_mismatch_50903(tmp_path): + """ + file.prepend must not raise UnicodeDecodeError when the target file + contains bytes that are not valid in the system encoding. The decoded + contents are only used to build the diff, so undecodable bytes should + be tolerated rather than aborting the state. + + Regression test for #50903. + """ + name = tmp_path / "motd" + # 0xed is not valid ASCII and not valid UTF-8 on its own + name.write_bytes(b"abc\xedxyz\n") + + salt_mock = { + "file.search": MagicMock(return_value=False), + "file.prepend": MagicMock(return_value=True), + } + + # Production callers (the state compiler running an SLS file.prepend) + # pass name and text; __salt_system_encoding__ is the locale-derived + # builtin read by the decode sites, so it is patched rather than passed. + with patch.object(builtins, "__salt_system_encoding__", "ascii"), patch.dict( + filestate.__salt__, salt_mock + ): + result = filestate.prepend(name=str(name), text="Trust no one") + + assert result["result"] is True + assert result["comment"] == "Prepended 1 lines" + salt_mock["file.prepend"].assert_called_once()