Skip to content

Commit 425c6e8

Browse files
mnriemCopilot
andcommitted
Force UTF-8 and full manifest validation
Force UTF-8 decoding for registry and manifest reads in the Bash and PowerShell embedded-Python parsers so resolution no longer depends on the process locale, and validate every manifest template entry's required fields, type, and strategy consistent with the canonical PresetManifest. Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3158e06f-95df-4e3a-843f-f159a35aa30c
1 parent 11d33aa commit 425c6e8

4 files changed

Lines changed: 160 additions & 19 deletions

File tree

scripts/bash/common.sh

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,7 @@ registered = {}
428428
registry = root / '.registry'
429429
if registry.is_file():
430430
try:
431-
data = json.loads(registry.read_text())
431+
data = json.loads(registry.read_text(encoding='utf-8'))
432432
value = data.get('extensions', {}) if isinstance(data, dict) else {}
433433
registered = value if isinstance(value, dict) else {}
434434
except Exception:
@@ -503,7 +503,7 @@ resolve_template() {
503503
if sorted_presets=$(SPECKIT_REGISTRY="$registry_file" "${python_cmd[@]}" -c "
504504
import json, re, sys, os
505505
try:
506-
with open(os.environ['SPECKIT_REGISTRY']) as f:
506+
with open(os.environ['SPECKIT_REGISTRY'], encoding='utf-8') as f:
507507
data = json.load(f)
508508
presets = data.get('presets', {})
509509
def priority(meta):
@@ -622,7 +622,7 @@ resolve_template_content() {
622622
if sorted_presets=$(SPECKIT_REGISTRY="$registry_file" "${python_cmd[@]}" -c "
623623
import json, re, sys, os
624624
try:
625-
with open(os.environ['SPECKIT_REGISTRY']) as f:
625+
with open(os.environ['SPECKIT_REGISTRY'], encoding='utf-8') as f:
626626
data = json.load(f)
627627
presets = data.get('presets', {})
628628
def priority(meta):
@@ -675,7 +675,7 @@ except ImportError:
675675
print('yaml_missing', file=sys.stderr)
676676
sys.exit(2)
677677
try:
678-
with open(os.environ['SPECKIT_MANIFEST']) as f:
678+
with open(os.environ['SPECKIT_MANIFEST'], encoding='utf-8') as f:
679679
data = yaml.safe_load(f)
680680
if not isinstance(data, dict):
681681
raise ValueError('manifest root must be a mapping')
@@ -685,15 +685,26 @@ try:
685685
templates = provides.get('templates', [])
686686
if not isinstance(templates, list):
687687
raise ValueError('manifest templates must be a list')
688+
valid_types = ('template', 'command', 'script')
689+
valid_strategies = ('replace', 'prepend', 'append', 'wrap')
688690
for t in templates:
689691
if not isinstance(t, dict):
690692
raise ValueError('manifest template entries must be mappings')
691-
file_value = t.get('file', '')
693+
if 'type' not in t or 'name' not in t or 'file' not in t:
694+
raise ValueError('manifest template entry missing type, name, or file')
695+
for field in ('type', 'name', 'file'):
696+
if not isinstance(t[field], str):
697+
raise ValueError('manifest template ' + field + ' must be a string')
698+
if t['type'] not in valid_types:
699+
raise ValueError('invalid manifest template type')
692700
strategy = t.get('strategy', 'replace')
693-
if not isinstance(file_value, str):
694-
raise ValueError('manifest template file must be a string')
695701
if not isinstance(strategy, str):
696702
raise ValueError('manifest template strategy must be a string')
703+
strategy = strategy.lower()
704+
if strategy not in valid_strategies:
705+
raise ValueError('invalid manifest template strategy')
706+
if t['type'] == 'script' and strategy not in ('replace', 'wrap'):
707+
raise ValueError('invalid manifest script strategy')
697708
for t in templates:
698709
if t.get('name') == os.environ['SPECKIT_TMPL'] and t.get('type', 'template') == 'template':
699710
file_value = t.get('file', '')

scripts/powershell/common.ps1

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -598,7 +598,7 @@ except ImportError:
598598
print('yaml_missing', file=sys.stderr)
599599
sys.exit(2)
600600
try:
601-
with open(sys.argv[1]) as f:
601+
with open(sys.argv[1], encoding='utf-8') as f:
602602
data = yaml.safe_load(f)
603603
if not isinstance(data, dict):
604604
raise ValueError('manifest root must be a mapping')
@@ -608,15 +608,26 @@ try:
608608
templates = provides.get('templates', [])
609609
if not isinstance(templates, list):
610610
raise ValueError('manifest templates must be a list')
611+
valid_types = ('template', 'command', 'script')
612+
valid_strategies = ('replace', 'prepend', 'append', 'wrap')
611613
for t in templates:
612614
if not isinstance(t, dict):
613615
raise ValueError('manifest template entries must be mappings')
614-
file_value = t.get('file', '')
616+
if 'type' not in t or 'name' not in t or 'file' not in t:
617+
raise ValueError('manifest template entry missing type, name, or file')
618+
for field in ('type', 'name', 'file'):
619+
if not isinstance(t[field], str):
620+
raise ValueError('manifest template ' + field + ' must be a string')
621+
if t['type'] not in valid_types:
622+
raise ValueError('invalid manifest template type')
615623
strategy = t.get('strategy', 'replace')
616-
if not isinstance(file_value, str):
617-
raise ValueError('manifest template file must be a string')
618624
if not isinstance(strategy, str):
619625
raise ValueError('manifest template strategy must be a string')
626+
strategy = strategy.lower()
627+
if strategy not in valid_strategies:
628+
raise ValueError('invalid manifest template strategy')
629+
if t['type'] == 'script' and strategy not in ('replace', 'wrap'):
630+
raise ValueError('invalid manifest script strategy')
620631
for t in templates:
621632
if t.get('name') == sys.argv[2] and t.get('type', 'template') == 'template':
622633
file_value = t.get('file', '')

scripts/python/common.py

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,36 @@ class TemplateResolutionError(RuntimeError):
335335
"""Raised when template layers exist but cannot be composed safely."""
336336

337337

338+
# Mirror the canonical PresetManifest contract (see src/specify_cli/presets)
339+
# so runtime resolution rejects the same structurally malformed manifests.
340+
_VALID_TEMPLATE_TYPES = ("template", "command", "script")
341+
_VALID_TEMPLATE_STRATEGIES = ("replace", "prepend", "append", "wrap")
342+
_VALID_SCRIPT_STRATEGIES = ("replace", "wrap")
343+
344+
345+
def _validate_manifest_template_entry(entry: object) -> None:
346+
"""Validate a single manifest template entry against the canonical rules."""
347+
if not isinstance(entry, dict):
348+
raise ValueError("manifest template entries must be mappings")
349+
if "type" not in entry or "name" not in entry or "file" not in entry:
350+
raise ValueError("manifest template entry missing type, name, or file")
351+
for field in ("type", "name", "file"):
352+
if not isinstance(entry[field], str):
353+
raise ValueError(f"manifest template {field} must be a string")
354+
if entry["type"] not in _VALID_TEMPLATE_TYPES:
355+
raise ValueError(f"invalid manifest template type '{entry['type']}'")
356+
strategy = entry.get("strategy", "replace")
357+
if not isinstance(strategy, str):
358+
raise ValueError("manifest template strategy must be a string")
359+
strategy = strategy.lower()
360+
if strategy not in _VALID_TEMPLATE_STRATEGIES:
361+
raise ValueError(f"invalid manifest template strategy '{strategy}'")
362+
if entry["type"] == "script" and strategy not in _VALID_SCRIPT_STRATEGIES:
363+
raise ValueError(
364+
f"invalid manifest script strategy '{strategy}'"
365+
)
366+
367+
338368
def _preset_template_layer(
339369
preset_dir: Path, template_name: str
340370
) -> tuple[Path, str] | None:
@@ -363,14 +393,7 @@ def _preset_template_layer(
363393
if not isinstance(templates, list):
364394
raise ValueError("manifest templates must be a list")
365395
for entry in templates:
366-
if not isinstance(entry, dict):
367-
raise ValueError("manifest template entries must be mappings")
368-
file_value = entry.get("file", "")
369-
strategy = entry.get("strategy", "replace")
370-
if not isinstance(file_value, str):
371-
raise ValueError("manifest template file must be a string")
372-
if not isinstance(strategy, str):
373-
raise ValueError("manifest template strategy must be a string")
396+
_validate_manifest_template_entry(entry)
374397
for entry in templates:
375398
if (
376399
entry.get("name") != template_name

tests/test_resolve_template_python_parity.py

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,59 @@ def test_all_variants_preserve_composition_parity(
9191
)
9292

9393

94+
@requires_bash
95+
def test_all_variants_read_utf8_registry_under_ascii_locale(
96+
tmp_path: Path,
97+
) -> None:
98+
"""Registry/manifest reads must force UTF-8, not the process locale.
99+
100+
With UTF-8 mode disabled and a C locale, the interpreter's default text
101+
encoding is ASCII. Non-ASCII *metadata* in the registry or a manifest must
102+
still resolve, because the resolvers open those files as UTF-8 explicitly.
103+
Template content stays ASCII so the pure-Python variant can emit it on the
104+
ASCII stdout this configuration forces.
105+
"""
106+
repo = make_repo(tmp_path)
107+
install_scripts(repo, SCRIPT)
108+
expected = install_composition_stack(repo, TEMPLATE, "# Core\n")
109+
110+
# Inject non-ASCII metadata into the preset registry and a manifest so a
111+
# locale-dependent decode would raise instead of resolving cleanly.
112+
registry = repo / ".specify" / "presets" / ".registry"
113+
registry_data = json.loads(registry.read_text(encoding="utf-8"))
114+
registry_data["presets"]["wrap-pack"]["description"] = "Café ✓ wrapper"
115+
registry.write_text(
116+
json.dumps(registry_data, separators=(",", ":")) + "\n",
117+
encoding="utf-8",
118+
)
119+
manifest = repo / ".specify" / "presets" / "wrap-pack" / "preset.yml"
120+
manifest.write_text(
121+
manifest.read_text(encoding="utf-8") + ' description: "Café ✓"\n',
122+
encoding="utf-8",
123+
)
124+
125+
env = clean_env()
126+
# Force the interpreter's default text encoding to ASCII so an unqualified
127+
# open() would fail on the non-ASCII metadata above.
128+
env["PYTHONUTF8"] = "0"
129+
env["PYTHONCOERCECLOCALE"] = "0"
130+
env["LC_ALL"] = "C"
131+
env["LANG"] = "C"
132+
133+
results = [
134+
run(bash_cmd(repo, SCRIPT, TEMPLATE, "--json"), repo, env),
135+
run(py_cmd(repo, SCRIPT, TEMPLATE, "--json"), repo, env),
136+
]
137+
if HAS_POWERSHELL:
138+
results.append(run(ps_cmd(repo, SCRIPT, TEMPLATE, "-Json"), repo, env))
139+
140+
assert all(result.returncode == 0 for result in results)
141+
assert all(
142+
json_stdout(result)["TEMPLATE_CONTENT"] == expected
143+
for result in results
144+
)
145+
146+
94147
@requires_bash
95148
@pytest.mark.parametrize(
96149
"template_name",
@@ -531,6 +584,45 @@ def test_bash_fails_when_override_read_fails(tmp_path: Path) -> None:
531584
name: unrelated-template
532585
file: null
533586
strategy: append
587+
""",
588+
f"""provides:
589+
templates:
590+
- name: {TEMPLATE}
591+
file: templates/{TEMPLATE}.md
592+
strategy: wrap
593+
- type: template
594+
name: unrelated-template
595+
file: templates/other.md
596+
""",
597+
f"""provides:
598+
templates:
599+
- type: template
600+
name: {TEMPLATE}
601+
file: templates/{TEMPLATE}.md
602+
strategy: wrap
603+
- type: template
604+
name: unrelated-template
605+
""",
606+
f"""provides:
607+
templates:
608+
- type: template
609+
name: {TEMPLATE}
610+
file: templates/{TEMPLATE}.md
611+
strategy: wrap
612+
- type: bogus
613+
name: unrelated-template
614+
file: templates/other.md
615+
""",
616+
f"""provides:
617+
templates:
618+
- type: template
619+
name: {TEMPLATE}
620+
file: templates/{TEMPLATE}.md
621+
strategy: wrap
622+
- type: template
623+
name: unrelated-template
624+
file: templates/other.md
625+
strategy: merge
534626
""",
535627
],
536628
ids=[
@@ -541,6 +633,10 @@ def test_bash_fails_when_override_read_fails(tmp_path: Path) -> None:
541633
"non_string_file",
542634
"non_string_strategy",
543635
"malformed_entry_after_match",
636+
"entry_missing_type",
637+
"entry_missing_file",
638+
"unsupported_type",
639+
"unsupported_strategy",
544640
],
545641
)
546642
def test_all_variants_fail_for_malformed_preset_manifest(

0 commit comments

Comments
 (0)