From 28b81e9a7f408d4856ad03ce64b1bd1fcb4fbadc Mon Sep 17 00:00:00 2001 From: ppcvote Date: Thu, 13 Aug 2026 14:35:17 +0800 Subject: [PATCH] fix(diffStix): write the changelog markdown as UTF-8 `get_new_changelog_md` opened the markdown file without an encoding, so CPython fell back to the locale codec. ATT&CK changelog text is not ASCII, so on any machine whose locale encoding is not UTF-8 the write raises: UnicodeEncodeError: 'cp950' codec can't encode character '\u041e' changelog_helper.py:2433 in get_new_changelog_md Reproduced on Windows with a cp950 console; cp1252, the default on en-US Windows, fails the same way on Cyrillic. The write sits at the top of the output block, so the exception also skips the HTML, detailed HTML, layer and JSON writers, leaving an empty markdown file and no other artifacts. This is an oversight rather than a policy: the sibling writers in the same module at lines 1849 and 1955 both pass `encoding="utf-8"` already. The remaining bare opens in this module (1861, 1866, 1871, 2465) go through `json.dump`, whose `ensure_ascii` default keeps their payload ASCII, so they do not raise today and are left alone. The three test-side `read_text()` calls are given the matching encoding. Before this change the round trip was symmetric by accident, locale-encoded on both ends; once the writer is UTF-8 the readers have to say so, or `file_content == markdown_result` compares a mis-decoded string. The new test drives the writer in a subprocess under `-X warn_default_encoding -W error::EncodingWarning`, rather than asserting on a round trip. A round trip passes on an unfixed tree wherever the locale encoding is already UTF-8, so on Linux CI it would have gone green either way and pinned nothing. Under PEP 597 it fails on every platform if the encoding argument goes missing again. Verified: fails before, passes after. `tests/changelog` has the same 7 failures before and after this change, all in `cli/` and all pre-existing Windows environment assumptions such as `pytest.raises(PermissionError)`. --- mitreattack/diffStix/changelog_helper.py | 2 +- .../test_diffstix_output_encoding.py | 134 ++++++++++++++++++ .../test_diffstix_output_generation.py | 2 +- tests/changelog/test_utils.py | 4 +- 4 files changed, 138 insertions(+), 4 deletions(-) create mode 100644 tests/changelog/integration/test_diffstix_output_encoding.py diff --git a/mitreattack/diffStix/changelog_helper.py b/mitreattack/diffStix/changelog_helper.py index fe88b70a..9eeff6bd 100644 --- a/mitreattack/diffStix/changelog_helper.py +++ b/mitreattack/diffStix/changelog_helper.py @@ -2429,7 +2429,7 @@ def get_new_changelog_md( if markdown_file: logger.info("Writing markdown to file") Path(markdown_file).parent.mkdir(parents=True, exist_ok=True) - with open(markdown_file, "w") as file: + with open(markdown_file, "w", encoding="utf-8") as file: file.write(md_string) if html_file: diff --git a/tests/changelog/integration/test_diffstix_output_encoding.py b/tests/changelog/integration/test_diffstix_output_encoding.py new file mode 100644 index 00000000..64a546ca --- /dev/null +++ b/tests/changelog/integration/test_diffstix_output_encoding.py @@ -0,0 +1,134 @@ +"""Integration test for the encoding of files written by get_new_changelog_md.""" + +import json +import subprocess +import sys +import textwrap + +IDENTITY = { + "type": "identity", + "id": "identity--c78cb6e5-0c4b-4611-8297-d1b8b55e40b5", + "spec_version": "2.1", + "name": "The MITRE Corporation", + "identity_class": "organization", + "created": "2017-06-01T00:00:00.000Z", + "modified": "2017-06-01T00:00:00.000Z", +} + +MARKING = { + "type": "marking-definition", + "id": "marking-definition--fa42a846-8d90-4e51-bc29-71d5b4802168", + "spec_version": "2.1", + "created": "2017-06-01T00:00:00.000Z", + "definition_type": "statement", + "definition": {"statement": "Copyright 2017, MITRE."}, +} + +# Cyrillic and a Turkish dotted lowercase g, neither representable in cp1252 or +# cp950. ATT&CK content does carry non-ASCII, notably in contributor names. +NON_ASCII_NAME = "Обход контроля Türkçe" + + +def _technique(name, attack_id, stix_id, created, modified): + return { + "type": "attack-pattern", + "id": stix_id, + "spec_version": "2.1", + "name": name, + "description": "test technique", + "created": created, + "modified": modified, + "x_mitre_version": "1.0", + "x_mitre_domains": ["enterprise-attack"], + "x_mitre_attack_spec_version": "3.2.0", + "created_by_ref": IDENTITY["id"], + "object_marking_refs": [MARKING["id"]], + "external_references": [ + { + "source_name": "mitre-attack", + "external_id": attack_id, + "url": f"https://attack.mitre.org/techniques/{attack_id}", + } + ], + } + + +def _write_bundles(tmp_path): + old_technique = _technique( + "Old Technique", + "T9999", + "attack-pattern--1f523a8f-a50f-490a-a0a3-48c8c1f889de", + "2023-01-01T00:00:00.000Z", + "2023-01-01T00:00:00.000Z", + ) + added_technique = _technique( + NON_ASCII_NAME, + "T9998", + "attack-pattern--2f523a8f-a50f-490a-a0a3-48c8c1f889df", + "2023-06-01T00:00:00.000Z", + "2023-06-01T00:00:00.000Z", + ) + for name, objects in [ + ("old", [IDENTITY, MARKING, old_technique]), + ("new", [IDENTITY, MARKING, old_technique, added_technique]), + ]: + directory = tmp_path / name + directory.mkdir() + (directory / "enterprise-attack.json").write_text( + json.dumps({"type": "bundle", "id": f"bundle--{name}", "objects": objects}), + encoding="utf-8", + ) + return tmp_path / "old", tmp_path / "new" + + +def test_markdown_file_is_written_as_utf8(tmp_path): + """The markdown file must be UTF-8 regardless of the locale encoding. + + Written as a subprocess under PEP 597 rather than a write-then-read-back + assertion. A round trip passes on an unfixed tree wherever the locale + encoding already is UTF-8, so on Linux CI it would go green either way and + pin nothing. `-X warn_default_encoding -W error::EncodingWarning` turns any + open() that relies on the locale codec into an error, so this fails on every + platform if the encoding argument goes missing again. + + Only the markdown file is requested. The layer and JSON writers also open + without an encoding, but they emit through json.dump, whose ensure_ascii + default keeps their output ASCII, so they are a separate concern. + """ + old_dir, new_dir = _write_bundles(tmp_path) + markdown_file = tmp_path / "changelog.md" + + driver = tmp_path / "driver.py" + driver.write_text( + textwrap.dedent( + f""" + from mitreattack.diffStix.changelog_helper import get_new_changelog_md + + get_new_changelog_md( + domains=["enterprise-attack"], + old={str(old_dir)!r}, + new={str(new_dir)!r}, + layers=[], + json_file=None, + markdown_file={str(markdown_file)!r}, + ) + """ + ), + encoding="utf-8", + ) + + result = subprocess.run( + [ + sys.executable, + "-X", + "warn_default_encoding", + "-W", + "error::EncodingWarning", + str(driver), + ], + capture_output=True, + text=True, + ) + + assert result.returncode == 0, result.stderr + assert NON_ASCII_NAME in markdown_file.read_text(encoding="utf-8") diff --git a/tests/changelog/integration/test_diffstix_output_generation.py b/tests/changelog/integration/test_diffstix_output_generation.py index 47ca399f..9bb191b2 100644 --- a/tests/changelog/integration/test_diffstix_output_generation.py +++ b/tests/changelog/integration/test_diffstix_output_generation.py @@ -155,7 +155,7 @@ def test_end_to_end_output_generation(self, minimal_stix_bundles, tmp_path, setu assert Path(layer_files[0]).exists() # Verify markdown file content - markdown_content = markdown_file.read_text() + markdown_content = markdown_file.read_text(encoding="utf-8") assert markdown_content == markdown_result assert "## Key" in markdown_content diff --git a/tests/changelog/test_utils.py b/tests/changelog/test_utils.py index cc0ef428..e97878ac 100644 --- a/tests/changelog/test_utils.py +++ b/tests/changelog/test_utils.py @@ -230,7 +230,7 @@ def validate_markdown_file_content(file_path: Union[str, Path]) -> str: path_obj = Path(file_path) assert path_obj.exists(), f"Markdown file should exist at {file_path}" - content = path_obj.read_text() + content = path_obj.read_text(encoding="utf-8") assert_basic_markdown_structure(content) return content @@ -405,7 +405,7 @@ def validate_comprehensive_output_generation( # Validate markdown file content matches return value if "markdown" in file_paths: - file_content = Path(file_paths["markdown"]).read_text() + file_content = Path(file_paths["markdown"]).read_text(encoding="utf-8") assert file_content == markdown_result, "Markdown file content should match return value"