Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions src/specify_cli/presets/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5540,7 +5540,7 @@ def resolve_content(
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).
Expand All @@ -5550,8 +5550,18 @@ def _read_layer_content(layer: Dict[str, Any]) -> str:
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:
Expand Down Expand Up @@ -5589,6 +5599,8 @@ def _read_layer_content(layer: Dict[str, Any]) -> str:
# 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
Expand Down Expand Up @@ -5632,7 +5644,12 @@ def _split_frontmatter(text: str) -> tuple:

# 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:
Expand Down
107 changes: 107 additions & 0 deletions tests/test_presets.py
Original file line number Diff line number Diff line change
Expand Up @@ -11353,6 +11353,113 @@ def test_resolve_content_nonexistent(self, project_dir):
content = resolver.resolve_content("nonexistent")
assert content is None

def test_resolve_content_unreadable_winning_layer_returns_none(self, project_dir):
Comment thread
marcelsafin marked this conversation as resolved.
"""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)
Expand Down