mirror of
https://github.com/github/spec-kit.git
synced 2026-08-15 14:02:04 +02:00
fix(presets): treat an unreadable core template as missing (#3961)
* fix(presets): treat an unreadable core template as missing _substitute_core_template() read the resolved core template with a bare read_text(), so one corrupted project-owned override in .specify/templates/commands/ crashed the whole wrap-strategy command registration with a raw UnicodeDecodeError. Both callers (CommandRegistrar.register_pack and _register_commands) are unguarded here, even though register_pack already skips an unreadable preset source with a warning a few lines above the call. Treat an unreadable core template like a missing one — warn and return the body unchanged with empty frontmatter — matching the function's documented no-core contract. Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: assert the unreadable-core warning instead of suppressing it Review follow-up: use pytest.warns so removing or changing the promised warning fails the test. 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:
@@ -178,7 +178,7 @@ def _substitute_core_template(
|
||||
by the core template body and core_frontmatter holds the core template's parsed
|
||||
frontmatter (so callers can inherit scripts/agent_scripts from it). Both are
|
||||
unchanged / empty when the placeholder is absent or the core template file does
|
||||
not exist.
|
||||
not exist or cannot be read.
|
||||
"""
|
||||
if "{CORE_TEMPLATE}" not in body:
|
||||
return body, {}
|
||||
@@ -208,7 +208,23 @@ def _substitute_core_template(
|
||||
if core_file is None:
|
||||
return body, {}
|
||||
|
||||
core_frontmatter, core_body = registrar.parse_frontmatter(core_file.read_text(encoding="utf-8"))
|
||||
# Treat an unreadable/undecodable core template like a missing one so a
|
||||
# single corrupted project override cannot crash command registration —
|
||||
# the wrap-strategy callers already skip an unreadable preset source with
|
||||
# a warning (CommandRegistrar.register_pack).
|
||||
try:
|
||||
core_content = core_file.read_text(encoding="utf-8")
|
||||
except (OSError, UnicodeDecodeError) as exc:
|
||||
import warnings
|
||||
|
||||
warnings.warn(
|
||||
f"Ignoring core template for command '{cmd_name}': could not read "
|
||||
f"'{core_file.name}' ({exc.__class__.__name__}: {exc}).",
|
||||
stacklevel=2,
|
||||
)
|
||||
return body, {}
|
||||
|
||||
core_frontmatter, core_body = registrar.parse_frontmatter(core_content)
|
||||
return body.replace("{CORE_TEMPLATE}", core_body), core_frontmatter
|
||||
|
||||
|
||||
|
||||
@@ -10790,6 +10790,35 @@ class TestWrapStrategy:
|
||||
assert "{CORE_TEMPLATE}" in result
|
||||
assert core_fm == {}
|
||||
|
||||
def test_substitute_core_template_unreadable_core_treated_as_missing(
|
||||
self, project_dir
|
||||
):
|
||||
"""An undecodable core template must not crash substitution.
|
||||
|
||||
The wrap-strategy callers (``CommandRegistrar.register_pack`` and
|
||||
``_register_commands``) skip an unreadable preset source with a
|
||||
warning, but the core template read inside
|
||||
``_substitute_core_template`` had no boundary, so one corrupted
|
||||
project-owned override in ``.specify/templates/commands/`` crashed
|
||||
the whole registration with a raw ``UnicodeDecodeError``. An
|
||||
unreadable core is treated like a missing one.
|
||||
"""
|
||||
from specify_cli.presets import _substitute_core_template
|
||||
from specify_cli.agents import CommandRegistrar
|
||||
|
||||
core_dir = project_dir / ".specify" / "templates" / "commands"
|
||||
core_dir.mkdir(parents=True, exist_ok=True)
|
||||
(core_dir / "specify.md").write_bytes(b"\xff\xfe not utf-8")
|
||||
|
||||
registrar = CommandRegistrar()
|
||||
body = "Pre.\n\n{CORE_TEMPLATE}\n\nPost.\n"
|
||||
with pytest.warns(UserWarning, match="Ignoring core template"):
|
||||
result, core_fm = _substitute_core_template(
|
||||
body, "specify", project_dir, registrar
|
||||
)
|
||||
assert result == body
|
||||
assert core_fm == {}
|
||||
|
||||
def test_register_commands_substitutes_core_template_for_wrap_strategy(self, project_dir):
|
||||
"""register_commands substitutes {CORE_TEMPLATE} when strategy: wrap."""
|
||||
from specify_cli.agents import CommandRegistrar
|
||||
|
||||
Reference in New Issue
Block a user