mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix(bundler): degrade non-UTF-8 config reads into BundlerError (#3784)
yamlio.py is the single chokepoint for every bundler read, and its module
docstring states the contract: "All reads/writes go through these functions so
that IO failures degrade into actionable BundlerError rather than raw
tracebacks."
Both readers catch only OSError, but `Path.read_text(encoding="utf-8")` and
`json.load()` raise UnicodeDecodeError on a non-UTF-8 file --
`issubclass(UnicodeDecodeError, OSError)` is False (its MRO is UnicodeError ->
ValueError). So the decode error escaped uncaught:
load_yaml: LEAKED UnicodeDecodeError -> 'utf-8' codec can't decode byte 0xff
load_json: LEAKED UnicodeDecodeError -> 'utf-8' codec can't decode byte 0xff
In load_json, json.JSONDecodeError does not help: it is a *sibling* of
UnicodeDecodeError, not a parent.
This is realistic rather than theoretical -- on Windows, PowerShell 5.1's
`Out-File` and `>` default to UTF-16, so a hand-edited
`.specify/bundle-catalogs.yml` or records file hits it.
Widen both read clauses to `(OSError, UnicodeError)`, matching the sibling
catalog readers (catalogs.py:101, workflows/catalog.py:336). JSONDecodeError
deliberately stays FIRST so malformed-but-decodable JSON keeps its more
specific "Invalid JSON" message; a regression test locks that ordering.
Write paths are unaffected -- verified that dump_yaml/dump_json do not leak
UnicodeEncodeError (both escape unencodable input), so this stays scoped to the
two read paths.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -58,7 +58,12 @@ def load_yaml(path: Path) -> Any:
|
||||
raise BundlerError(f"File not found: {path}")
|
||||
try:
|
||||
text = path.read_text(encoding="utf-8")
|
||||
except OSError as exc:
|
||||
except (OSError, UnicodeError) as exc:
|
||||
# A non-UTF-8 file raises UnicodeDecodeError, which is a ValueError --
|
||||
# NOT an OSError -- so it escaped this module's "IO failures degrade
|
||||
# into actionable BundlerError" contract as a raw traceback. Realistic
|
||||
# on Windows, where PowerShell 5.1's `Out-File`/`>` default to UTF-16.
|
||||
# Matches the sibling catalog readers (catalogs.py, workflows/catalog.py).
|
||||
raise BundlerError(f"Could not read {path}: {exc}") from exc
|
||||
try:
|
||||
has_node = yaml.compose(text) is not None
|
||||
@@ -98,9 +103,15 @@ def load_json(path: Path) -> Any:
|
||||
try:
|
||||
with path.open("r", encoding="utf-8") as handle:
|
||||
return json.load(handle)
|
||||
# JSONDecodeError stays FIRST: it and UnicodeDecodeError are sibling
|
||||
# ValueError subclasses (neither subsumes the other), so malformed-but-
|
||||
# decodable JSON keeps its more specific "Invalid JSON" message while a
|
||||
# decode failure falls through to the read-error clause below.
|
||||
except json.JSONDecodeError as exc:
|
||||
raise BundlerError(f"Invalid JSON in {path}: {exc}") from exc
|
||||
except OSError as exc:
|
||||
except (OSError, UnicodeError) as exc:
|
||||
# See load_yaml: a non-UTF-8 file raises UnicodeDecodeError, which is
|
||||
# not an OSError, and previously escaped as a raw traceback.
|
||||
raise BundlerError(f"Could not read {path}: {exc}") from exc
|
||||
|
||||
|
||||
|
||||
@@ -3,7 +3,10 @@ from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from specify_cli.bundler.lib.yamlio import dump_yaml, load_yaml
|
||||
import pytest
|
||||
|
||||
from specify_cli.bundler import BundlerError
|
||||
from specify_cli.bundler.lib.yamlio import dump_yaml, load_json, load_yaml
|
||||
|
||||
|
||||
def test_dump_yaml_preserves_unicode(tmp_path: Path):
|
||||
@@ -24,3 +27,32 @@ def test_dump_yaml_round_trips_unicode(tmp_path: Path):
|
||||
data = {"note": "café", "city": "münchen"}
|
||||
dump_yaml(path, data)
|
||||
assert load_yaml(path) == data
|
||||
|
||||
|
||||
def test_load_yaml_non_utf8_raises_bundler_error(tmp_path: Path):
|
||||
"""A non-UTF-8 file must degrade into BundlerError, per this module's
|
||||
documented contract. UnicodeDecodeError is a ValueError, not an OSError, so
|
||||
it previously escaped as a raw traceback. UTF-16 is the realistic case:
|
||||
PowerShell 5.1's `Out-File`/`>` default to it."""
|
||||
path = tmp_path / "bundle-catalogs.yml"
|
||||
path.write_bytes('catalogs: []\n'.encode("utf-16"))
|
||||
with pytest.raises(BundlerError, match="Could not read"):
|
||||
load_yaml(path)
|
||||
|
||||
|
||||
def test_load_json_non_utf8_raises_bundler_error(tmp_path: Path):
|
||||
"""Same for the JSON reader: json.JSONDecodeError is a *sibling* of
|
||||
UnicodeDecodeError, so it does not cover a decode failure."""
|
||||
path = tmp_path / "records.json"
|
||||
path.write_bytes('{"bundles": []}'.encode("utf-16"))
|
||||
with pytest.raises(BundlerError, match="Could not read"):
|
||||
load_json(path)
|
||||
|
||||
|
||||
def test_load_json_malformed_still_reports_invalid_json(tmp_path: Path):
|
||||
"""Clause order regression guard: decodable-but-malformed JSON must keep the
|
||||
more specific 'Invalid JSON' message rather than the read-error one."""
|
||||
path = tmp_path / "records.json"
|
||||
path.write_text('{"bundles": [', encoding="utf-8")
|
||||
with pytest.raises(BundlerError, match="Invalid JSON"):
|
||||
load_json(path)
|
||||
|
||||
Reference in New Issue
Block a user