Skip to content

Commit 442a581

Browse files
fix(cli): pin UTF-8 encoding on init-options and .extensionignore I/O (#2686)
* fix(cli): pin UTF-8 encoding on init-options and .extensionignore I/O ``Path.read_text`` / ``Path.write_text`` default to the system locale codec, which is cp1252 / gb2312 / cp932 on Windows. Two user-facing file paths in spec-kit were calling them without an explicit ``encoding=`` argument: - ``src/specify_cli/__init__.py:400,412`` — ``save_init_options`` / ``load_init_options`` for ``.specify/init-options.json``. A peer machine with a different default locale (or a UTF-8 Unix CI runner reading a file written on a cp1252 Windows host) cannot decode the file, raising ``UnicodeDecodeError``. ``UnicodeDecodeError`` is a subclass of ``ValueError`` — not ``OSError`` / ``json.JSONDecodeError`` — so the existing fall-back ``except`` tuple in ``load_init_options`` also misses it and the error propagates raw to the CLI. - ``src/specify_cli/extensions.py:764`` — ``.extensionignore`` pattern reader. The very next line already normalises backslashes "so Windows-authored files work", proving the codebase expects Windows authors to write this file. Multibyte UTF-8 patterns (Chinese filenames, accented directory names) silently mojibake when the host locale is not UTF-8, so the patterns fail to match and unintended files are shipped with the extension. The sibling integration-catalog reader at ``src/specify_cli/integrations/catalog.py:150,156,193,202,374`` already pins ``encoding="utf-8"`` everywhere. PR #2280 fixed the symmetric PowerShell-template BOM bug. This change brings the two remaining drifted paths in line with that precedent. Regression tests: - ``tests/test_presets.py::TestInitOptions`` — parametrized non-ASCII round-trip (CJK, Latin-1, Greek, emoji) plus a corrupted-file case that asserts the existing "fall back to {}" contract still holds when a peer file contains bytes invalid as UTF-8. - ``tests/test_extensions.py::TestExtensionIgnore`` — Japanese (``ドキュメント/``) and Latin-1 (``café/``) ignore patterns correctly exclude their directories during install. * fix(cli): wrap .extensionignore decode error and tighten UTF-8 contract Addresses Copilot review feedback on this PR. Three issues, three fixes: 1. ``save_init_options`` now writes JSON with ``ensure_ascii=False``. Without that flag, ``json.dumps`` emits ASCII-only ``\uXXXX`` escapes, which means the ``encoding="utf-8"`` pin on the surrounding ``Path.write_text`` makes no observable difference for any value we currently write. Flipping ``ensure_ascii`` makes the non-ASCII bytes hit the file directly, so the encoding pin becomes the thing that decides between cp1252 garbage and clean UTF-8 on Windows. The comment above the call now describes the real reason instead of the previously-misleading rationale Copilot flagged. 2. ``test_save_load_round_trip_preserves_non_ascii`` was a no-op under the old ``ensure_ascii=True`` writer (Copilot's second comment). Added ``test_save_writes_real_utf8_bytes`` that asserts the on-disk bytes contain the UTF-8 encoding of ``café`` (``0xC3 0xA9``), not its JSON escape form ``é``. Removing either ``ensure_ascii=False`` or ``encoding="utf-8"`` from the writer now breaks this test — the contract is pinned. 3. ``.extensionignore`` reader wraps ``UnicodeDecodeError`` as ``ValidationError`` with a pointer to the offending byte (Copilot's third comment). Mirrors ``ExtensionManifest._load_yaml``'s existing handler for ``extension.yml``. Adds ``test_extensionignore_invalid_utf8_raises_validation_error`` asserting installation aborts with the wrapped error instead of a raw Python traceback.
1 parent ed10b32 commit 442a581

4 files changed

Lines changed: 207 additions & 6 deletions

File tree

src/specify_cli/__init__.py

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,28 @@ def save_init_options(project_path: Path, options: dict[str, Any]) -> None:
287287
"""
288288
dest = project_path / INIT_OPTIONS_FILE
289289
dest.parent.mkdir(parents=True, exist_ok=True)
290-
dest.write_text(json.dumps(options, indent=2, sort_keys=True))
290+
# Write JSON as real UTF-8 instead of ``\uXXXX`` escape sequences
291+
# (``ensure_ascii=False``) and pin the file encoding to match.
292+
#
293+
# The default ``json.dumps`` output is ASCII-only — any non-ASCII
294+
# character is encoded as a ``\uXXXX`` escape — so without the
295+
# ``ensure_ascii=False`` flip below the encoding pin alone would be
296+
# a no-op for any payload we plausibly write today. We pair the two
297+
# so the on-disk bytes match a human's expectation of "this file is
298+
# UTF-8" (greppable, readable in editors that don't decode JSON
299+
# escapes, friendly to peers running ``cat`` or ``Get-Content``) and
300+
# so the encoding pin is a real contract instead of a future hedge.
301+
#
302+
# ``Path.write_text`` without ``encoding=`` falls back to the system
303+
# locale codec (cp1252 / gb2312 / cp932 on Windows), which would
304+
# mis-encode non-ASCII bytes locally and produce a file a peer with
305+
# a different locale couldn't decode. The sibling integration-
306+
# catalog writer in ``integrations/catalog.py`` pins
307+
# ``encoding="utf-8"`` for the same reason.
308+
dest.write_text(
309+
json.dumps(options, indent=2, sort_keys=True, ensure_ascii=False),
310+
encoding="utf-8",
311+
)
291312

292313

293314
def load_init_options(project_path: Path) -> dict[str, Any]:
@@ -299,8 +320,17 @@ def load_init_options(project_path: Path) -> dict[str, Any]:
299320
if not path.exists():
300321
return {}
301322
try:
302-
return json.loads(path.read_text())
303-
except (json.JSONDecodeError, OSError):
323+
# Match the explicit UTF-8 used by ``save_init_options``; without
324+
# it ``read_text`` falls back to the system codec on Windows and
325+
# raises ``UnicodeDecodeError`` on any file containing the
326+
# multi-byte UTF-8 sequences ``save_init_options`` now writes
327+
# directly. ``UnicodeDecodeError`` is a subclass of
328+
# ``ValueError``, not ``OSError`` / ``json.JSONDecodeError``, so
329+
# it must be listed explicitly here to preserve the existing
330+
# "fall back to empty dict" contract for corrupted / foreign-
331+
# codec files.
332+
return json.loads(path.read_text(encoding="utf-8"))
333+
except (json.JSONDecodeError, OSError, UnicodeDecodeError):
304334
return {}
305335

306336

src/specify_cli/extensions.py

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -761,7 +761,28 @@ def _load_extensionignore(source_dir: Path) -> Optional[Callable[[str, List[str]
761761
if not ignore_file.exists():
762762
return None
763763

764-
lines: List[str] = ignore_file.read_text().splitlines()
764+
# Pin UTF-8 explicitly: ``Path.read_text`` defaults to the system
765+
# locale codec on Windows (cp1252 / gb2312 / cp932), which silently
766+
# corrupts multibyte patterns when the file is shared across
767+
# machines with different locales. The next line already
768+
# normalises backslashes "so Windows-authored files work" — the
769+
# codebase already expects Windows authors to write this file.
770+
#
771+
# A file that is not valid UTF-8 is a user-authoring mistake, so
772+
# surface it as ``ValidationError`` with a pointer to the offending
773+
# byte — the same pattern ``ExtensionManifest._load_yaml`` uses
774+
# for ``extension.yml`` (see ``UnicodeDecodeError`` handler in
775+
# this module). Without the wrap, the raw ``UnicodeDecodeError``
776+
# would abort installation with a Python traceback instead of a
777+
# clear message naming the file.
778+
try:
779+
raw = ignore_file.read_text(encoding="utf-8")
780+
except UnicodeDecodeError as e:
781+
raise ValidationError(
782+
f".extensionignore is not valid UTF-8: {ignore_file} "
783+
f"({e.reason} at byte {e.start})"
784+
)
785+
lines: List[str] = raw.splitlines()
765786

766787
# Normalise backslashes in patterns so Windows-authored files work
767788
normalised: List[str] = []

tests/test_extensions.py

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3358,9 +3358,13 @@ def _make_extension(self, temp_dir, valid_manifest_data, extra_files=None, ignor
33583358
else:
33593359
p.write_text(content)
33603360

3361-
# Write .extensionignore
3361+
# Write .extensionignore. Pinned to UTF-8 so non-ASCII patterns
3362+
# in tests (see ``test_extensionignore_utf8_patterns``) survive
3363+
# the round-trip on Windows runners with non-UTF-8 default locales.
33623364
if ignore_content is not None:
3363-
(ext_dir / ".extensionignore").write_text(ignore_content)
3365+
(ext_dir / ".extensionignore").write_text(
3366+
ignore_content, encoding="utf-8"
3367+
)
33643368

33653369
return ext_dir
33663370

@@ -3590,6 +3594,73 @@ def test_extensionignore_windows_backslash_patterns(self, temp_dir, valid_manife
35903594
assert (dest / "docs" / "guide.md").exists()
35913595
assert not (dest / "docs" / "internal" / "draft.md").exists()
35923596

3597+
def test_extensionignore_utf8_patterns(self, temp_dir, valid_manifest_data):
3598+
"""Non-ASCII patterns in .extensionignore work on every locale.
3599+
3600+
``Path.read_text`` defaults to the system locale codec on Windows
3601+
(cp1252 / gb2312 / cp932). Without an explicit ``encoding="utf-8"``,
3602+
a pattern like ``ドキュメント/`` written by a UTF-8 host becomes
3603+
mojibake on a cp1252 host and silently fails to match — leaking
3604+
files the author intended to exclude. The existing
3605+
``test_extensionignore_windows_backslash_patterns`` already shows
3606+
the codebase treats this as a Windows-author-friendly file; UTF-8
3607+
is part of that same contract.
3608+
"""
3609+
ext_dir = self._make_extension(
3610+
temp_dir,
3611+
valid_manifest_data,
3612+
extra_files={
3613+
"ドキュメント/private.md": "secret",
3614+
"ドキュメント/public.md": "public",
3615+
"docs/guide.md": "# Guide",
3616+
"café/résumé.txt": "draft",
3617+
},
3618+
ignore_content="ドキュメント/\ncafé/\n",
3619+
)
3620+
3621+
proj_dir = temp_dir / "project"
3622+
proj_dir.mkdir()
3623+
(proj_dir / ".specify").mkdir()
3624+
3625+
manager = ExtensionManager(proj_dir)
3626+
manager.install_from_directory(ext_dir, "0.1.0", register_commands=False)
3627+
3628+
dest = proj_dir / ".specify" / "extensions" / "test-ext"
3629+
# Multibyte patterns excluded.
3630+
assert not (dest / "ドキュメント").exists()
3631+
assert not (dest / "café").exists()
3632+
# ASCII path with no matching pattern is unaffected.
3633+
assert (dest / "docs" / "guide.md").exists()
3634+
3635+
def test_extensionignore_invalid_utf8_raises_validation_error(
3636+
self, temp_dir, valid_manifest_data
3637+
):
3638+
"""A non-UTF-8 ``.extensionignore`` surfaces as ``ValidationError``.
3639+
3640+
Pinning ``encoding="utf-8"`` on the reader means an
3641+
``.extensionignore`` written in some other codec (cp1252, etc.)
3642+
now triggers ``UnicodeDecodeError`` instead of silently
3643+
mojibake-ing patterns. Wrap that exception as ``ValidationError``
3644+
with a pointer to the offending byte — the same pattern
3645+
``ExtensionManifest._load_yaml`` uses for ``extension.yml`` —
3646+
so installation aborts with a user-friendly message instead of a
3647+
raw Python traceback.
3648+
"""
3649+
ext_dir = self._make_extension(temp_dir, valid_manifest_data)
3650+
# Write an .extensionignore whose bytes are not valid UTF-8.
3651+
# 0xE9 is 'é' in cp1252 but an invalid lead byte in UTF-8.
3652+
(ext_dir / ".extensionignore").write_bytes(b"caf\xe9/\n")
3653+
3654+
proj_dir = temp_dir / "project"
3655+
proj_dir.mkdir()
3656+
(proj_dir / ".specify").mkdir()
3657+
3658+
manager = ExtensionManager(proj_dir)
3659+
with pytest.raises(
3660+
ValidationError, match=r"\.extensionignore is not valid UTF-8"
3661+
):
3662+
manager.install_from_directory(ext_dir, "0.1.0", register_commands=False)
3663+
35933664
def test_extensionignore_star_does_not_cross_directories(self, temp_dir, valid_manifest_data):
35943665
"""'*' should NOT match across directory boundaries (gitignore semantics)."""
35953666
ext_dir = self._make_extension(

tests/test_presets.py

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2269,6 +2269,85 @@ def test_load_returns_empty_on_invalid_json(self, project_dir):
22692269

22702270
assert load_init_options(project_dir) == {}
22712271

2272+
@pytest.mark.parametrize(
2273+
"value",
2274+
["名前-プロジェクト", "café-résumé", "Ωmega-Δelta", "🚀-launch"],
2275+
)
2276+
def test_save_load_round_trip_preserves_non_ascii(self, project_dir, value):
2277+
"""Non-ASCII values round-trip via explicit UTF-8 encoding.
2278+
2279+
``Path.write_text`` / ``Path.read_text`` default to the system
2280+
locale codec on Windows (cp1252 / gb2312 / cp932). Without
2281+
``encoding="utf-8"`` pinned on both ends, a project name like
2282+
``café`` written on a UTF-8 host becomes garbled or unreadable on
2283+
a cp1252 host (and vice versa). Pin UTF-8 explicitly so init
2284+
options round-trip across machines and CI.
2285+
2286+
Note: this test only meaningfully exercises the encoding pin
2287+
because ``save_init_options`` now writes JSON with
2288+
``ensure_ascii=False`` — otherwise ``json.dumps`` would output
2289+
ASCII-only ``\\uXXXX`` escapes and the encoding pin would be a
2290+
no-op for any value here. ``test_save_writes_real_utf8_bytes``
2291+
below asserts that contract directly.
2292+
"""
2293+
from specify_cli import save_init_options, load_init_options
2294+
2295+
save_init_options(project_dir, {"ai": "claude", "project_name": value})
2296+
2297+
loaded = load_init_options(project_dir)
2298+
assert loaded["project_name"] == value
2299+
2300+
def test_save_writes_real_utf8_bytes(self, project_dir):
2301+
"""The on-disk file contains real UTF-8 bytes, not ``\\uXXXX`` escapes.
2302+
2303+
Pinning ``encoding="utf-8"`` on ``write_text`` only makes a
2304+
difference when the serialiser actually emits non-ASCII
2305+
characters. With ``ensure_ascii=False`` on ``json.dumps`` the
2306+
non-ASCII bytes hit the file, so the encoding pin is the thing
2307+
that decides between cp1252 garbage and clean UTF-8 on Windows.
2308+
2309+
This test pins that behaviour: the on-disk bytes are valid UTF-8
2310+
and contain the multi-byte encoding of ``café``, not its
2311+
``\\u00e9`` escape form. Reviewers can verify that removing
2312+
``ensure_ascii=False`` or ``encoding="utf-8"`` from the writer
2313+
breaks this test, which is what Copilot's review pointed out the
2314+
original round-trip test failed to do.
2315+
"""
2316+
from specify_cli import save_init_options
2317+
2318+
save_init_options(project_dir, {"project_name": "café"})
2319+
2320+
opts_file = project_dir / ".specify" / "init-options.json"
2321+
raw = opts_file.read_bytes()
2322+
# 'café' in UTF-8 ends with bytes 0xC3 0xA9 ('é'). The cp1252
2323+
# encoding of 'é' is the single byte 0xE9. The JSON-escape form
2324+
# would be the 6-byte literal '\\u00e9'. We assert the UTF-8 form
2325+
# is present so the test pins the actual contract.
2326+
assert b"caf\xc3\xa9" in raw, (
2327+
"Expected UTF-8 bytes for 'café' in the on-disk file, "
2328+
f"got: {raw!r}"
2329+
)
2330+
# And the whole file decodes cleanly as UTF-8.
2331+
raw.decode("utf-8")
2332+
2333+
def test_load_returns_empty_on_locale_corrupted_file(self, project_dir):
2334+
"""A file written in a non-UTF-8 codec falls back to {}, not crash.
2335+
2336+
Simulates a file produced by an old client (or by a peer machine
2337+
with a different default locale) that contains bytes invalid as
2338+
UTF-8. ``load_init_options`` should fall back to ``{}`` per the
2339+
existing contract — never propagate a raw ``UnicodeDecodeError``
2340+
to the CLI surface.
2341+
"""
2342+
from specify_cli import load_init_options
2343+
2344+
opts_file = project_dir / ".specify" / "init-options.json"
2345+
opts_file.parent.mkdir(parents=True, exist_ok=True)
2346+
# 0xE9 is 'é' in cp1252 but an invalid lead byte in UTF-8.
2347+
opts_file.write_bytes(b'{"project_name": "caf\xe9"}')
2348+
2349+
assert load_init_options(project_dir) == {}
2350+
22722351

22732352
class TestPresetSkills:
22742353
"""Tests for preset skill registration and unregistration.

0 commit comments

Comments
 (0)