External/spec-kit
1
0
Fork 0
mirror of https://github.com/github/spec-kit.git synced 2026-08-15 14:02:04 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(presets): return None for an unreadable layer in resolve_content (#3959)

Browse Source 
* fix(presets): return None for an unreadable layer in resolve_content

PresetResolver.resolve_content() reads the winning layer (and each
composition layer) with a bare read_text(), so a layer file that cannot
be read or decoded crashed command registration with a raw
OSError/UnicodeDecodeError. The docstring already promises 'Composed
content string, or None if not found', and since #3896
collect_all_layers() deliberately tolerates a non-UTF-8 legacy layer —
moving the crash here, where both callers (_register_commands and
_reconcile_composed_commands) are unguarded.

Return None when the winning or base layer cannot be read, treating an
unreadable layer like a missing one per the documented contract.

Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* test: cover the base guard and composing-layer read

Review follow-up: add an unreadable replace base beneath a valid
composing layer, and a mocked-PermissionError composing layer over a
valid base, so every new boundary and both exception types are covered.

Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Marsel Safin
2026-08-06 08:16:08 -05:00
committed by GitHub
co-authored by Copilot
parent 3d4f71c90e
commit fe3732e268
2 changed files with 127 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/specify_cli/presets/__init__.py
+20 -3
View File
@@ -5576,7 +5576,7 @@ class PresetResolver:
if not layers:
return None
def _read_layer_content(layer: Dict[str, Any]) -> str:
def _read_layer_content(layer: Dict[str, Any]) -> Optional[str]:
"""Read a layer's raw text, rewriting extension-relative subdir
references (agents/, knowledge-base/, etc.) to their installed
location when the layer is extension-provided (#2101).
@@ -5586,8 +5586,18 @@ class PresetResolver:
rewrite when it wins outright above or serves as the
composition base below — never as a mid-stack composing
(append/prepend/wrap) layer.
Returns None when the layer cannot be read or decoded:
collect_all_layers deliberately keeps a non-UTF-8 legacy layer
(with its "replace" default) so unrelated commands still
resolve, so the same tolerance must apply here — the documented
contract is "Composed content string, or None if not found",
not a raw UnicodeDecodeError at composition time.
"""
text = layer["path"].read_text(encoding="utf-8")
try:
text = layer["path"].read_text(encoding="utf-8")
except (OSError, UnicodeDecodeError):
return None
extension_id = layer.get("extension_id")
extension_dir = layer.get("extension_dir")
if extension_id and extension_dir:
@@ -5625,6 +5635,8 @@ class PresetResolver:
# Convert to reversed_layers index
base_reversed_idx = len(layers) - 1 - base_layer_idx
content = _read_layer_content(layers[base_layer_idx])
if content is None:
return None
# Compose only the layers above the base (higher priority = lower index in layers,
# higher index in reversed_layers). Process bottom-up from base+1.
start_idx = base_reversed_idx + 1
@@ -5668,7 +5680,12 @@ class PresetResolver:
# Apply composition layers from bottom to top
for layer in reversed_layers[start_idx:]:
layer_content = layer["path"].read_text(encoding="utf-8")
try:
layer_content = layer["path"].read_text(encoding="utf-8")
except (OSError, UnicodeDecodeError):
# Same tolerance as _read_layer_content: an unreadable layer
# means the composed result cannot be produced.
return None
strategy = layer["strategy"]
if is_command:
tests/test_presets.py
+107
View File
@@ -11425,6 +11425,113 @@ class TestResolveContent:
content = resolver.resolve_content("nonexistent")
assert content is None
def test_resolve_content_unreadable_winning_layer_returns_none(self, project_dir):
"""An undecodable winning layer must yield None, not a raw traceback.
``collect_all_layers`` deliberately keeps a non-UTF-8 legacy command
layer (with its ``replace`` default) so unrelated commands still
resolve. ``resolve_content`` then read that same file without a
boundary, so the tolerated layer crashed with ``UnicodeDecodeError``
at composition time — reachable from ``specify preset add`` via
``_register_commands``. The documented contract is "Composed content
string, or None if not found".
"""
presets_dir = project_dir / ".specify" / "presets"
command_path = (
presets_dir / "legacy-pack" / "commands" / "speckit.legacy.md"
)
command_path.parent.mkdir(parents=True)
command_path.write_bytes(b"\xff\xfe")
PresetRegistry(presets_dir).add(
"legacy-pack", {"version": "1.0.0", "priority": 10}
)
resolver = PresetResolver(project_dir)
content = resolver.resolve_content("speckit.legacy", "command")
assert content is None
def test_resolve_content_unreadable_base_under_composing_layer(
self, project_dir, temp_dir, valid_pack_data
):
"""An undecodable base beneath a valid composing layer yields None.
Covers the base-read guard: the winning layer composes (append), so
resolution reads the base layer beneath it — here the core template,
corrupted to non-UTF-8 — and must return None instead of crashing.
"""
pack_data = {**valid_pack_data}
pack_data["preset"] = {**valid_pack_data["preset"], "id": "append-pack", "name": "Append"}
pack_data["provides"] = {
"templates": [{
"type": "template",
"name": "spec-template",
"file": "templates/spec-template.md",
"strategy": "append",
}]
}
pack_dir = temp_dir / "append-pack"
pack_dir.mkdir()
with open(pack_dir / "preset.yml", 'w') as f:
yaml.dump(pack_data, f)
(pack_dir / "templates").mkdir()
(pack_dir / "templates" / "spec-template.md").write_text("## Appended Section\n")
manager = PresetManager(project_dir)
manager.install_from_directory(pack_dir, "0.1.5")
core_spec = project_dir / ".specify" / "templates" / "spec-template.md"
core_spec.write_bytes(b"\xff\xfe")
resolver = PresetResolver(project_dir)
assert resolver.resolve_content("spec-template") is None
def test_resolve_content_unreadable_composing_layer(
self, project_dir, temp_dir, valid_pack_data, monkeypatch
):
"""An unreadable composing layer over a valid base yields None.
Covers the composition-loop read and the ``OSError`` half of the
boundary: the base (core template) reads fine, but the append layer
raises a mocked ``PermissionError`` — mocked so the case also holds
under privileged CI where permission bits are not enforced.
"""
pack_data = {**valid_pack_data}
pack_data["preset"] = {**valid_pack_data["preset"], "id": "append-pack", "name": "Append"}
pack_data["provides"] = {
"templates": [{
"type": "template",
"name": "spec-template",
"file": "templates/spec-template.md",
"strategy": "append",
}]
}
pack_dir = temp_dir / "append-pack"
pack_dir.mkdir()
with open(pack_dir / "preset.yml", 'w') as f:
yaml.dump(pack_data, f)
(pack_dir / "templates").mkdir()
(pack_dir / "templates" / "spec-template.md").write_text("## Appended Section\n")
manager = PresetManager(project_dir)
manager.install_from_directory(pack_dir, "0.1.5")
layer_path = (
project_dir / ".specify" / "presets" / "append-pack"
/ "templates" / "spec-template.md"
)
assert layer_path.is_file()
original_read_text = Path.read_text
def failing_read_text(self_path, *args, **kwargs):
if self_path == layer_path:
raise PermissionError(13, "Permission denied")
return original_read_text(self_path, *args, **kwargs)
monkeypatch.setattr(Path, "read_text", failing_read_text)
resolver = PresetResolver(project_dir)
assert resolver.resolve_content("spec-template") is None
def test_resolve_content_replace_strategy(self, project_dir, temp_dir, valid_pack_data):
"""Test resolve_content with default replace strategy."""
manager = PresetManager(project_dir)