fix(extensions): reject duplicate provides.templates/scripts names (#4016)
The resolver returns the first entry matching a declared name, so a later duplicate within provides.templates or provides.scripts was silently unreachable while still counted by ExtensionManifest properties. Reject duplicates at manifest-validation time instead. Also clarify EXTENSION-DEVELOPMENT-GUIDE.md's provides section: hooks and events are top-level manifest fields, not provides sub-fields, so the "at least one of ..." wording doesn't imply they can be nested under provides.
This commit is contained in:
@@ -177,12 +177,17 @@ Compatibility requirements.
|
||||
|
||||
What the extension provides.
|
||||
|
||||
**Optional sub-fields** (at least one of `commands`, `templates`, `scripts`, `hooks`, or `events` is required):
|
||||
**Optional sub-fields:**
|
||||
|
||||
- `commands`: Array of command objects
|
||||
- `templates`: Array of template objects
|
||||
- `scripts`: Array of script objects
|
||||
|
||||
`hooks` and `events` are separate top-level manifest fields (siblings of
|
||||
`provides`, not nested under it — see [`hooks`](#hooks) below). At least one
|
||||
of `provides.commands`, `provides.templates`, `provides.scripts`, `hooks`, or
|
||||
`events` is required.
|
||||
|
||||
**Command object**:
|
||||
|
||||
- `name`: Command name (must match `speckit.{ext-id}.{command}`)
|
||||
|
||||
@@ -577,8 +577,13 @@ class ExtensionManifest:
|
||||
behavior for extension layers in presets/__init__.py). A present
|
||||
'strategy' key is rejected rather than silently ignored, so an author
|
||||
who copies a preset-style entry gets a clear error instead of a
|
||||
silently-dropped field.
|
||||
silently-dropped field. Duplicate names within a section are also
|
||||
rejected: the resolver returns the first matching entry by name
|
||||
(``PresetResolver._extension_manifest_declared_template``), so a
|
||||
later duplicate would be silently unreachable while still being
|
||||
exposed by ``ExtensionManifest.templates``/``.scripts``.
|
||||
"""
|
||||
seen_names: set[str] = set()
|
||||
for entry in entries:
|
||||
if not isinstance(entry, dict):
|
||||
raise ValidationError(
|
||||
@@ -597,6 +602,11 @@ class ExtensionManifest:
|
||||
f"Invalid {singular} name '{name}': "
|
||||
"must be lowercase alphanumeric with hyphens only"
|
||||
)
|
||||
if name in seen_names:
|
||||
raise ValidationError(
|
||||
f"Duplicate {singular} name '{name}' in 'provides.{section}'"
|
||||
)
|
||||
seen_names.add(name)
|
||||
|
||||
file_value = entry["file"]
|
||||
reason = relative_extension_path_violation(file_value)
|
||||
|
||||
@@ -1162,6 +1162,29 @@ class TestExtensionManifestTemplatesAndScripts:
|
||||
with pytest.raises(ValidationError, match="must be lowercase alphanumeric with hyphens only"):
|
||||
ExtensionManifest(manifest_path)
|
||||
|
||||
@pytest.mark.parametrize("section", ["templates", "scripts"])
|
||||
def test_provides_entry_duplicate_name_rejected(self, temp_dir, valid_manifest_data, section):
|
||||
"""Two entries in the same section sharing a name are rejected.
|
||||
|
||||
The resolver (PresetResolver._extension_manifest_declared_template)
|
||||
returns the first entry matching a name, so a later duplicate would
|
||||
be silently unreachable while still counted by ExtensionManifest
|
||||
properties -- reject it up front instead.
|
||||
"""
|
||||
import yaml
|
||||
|
||||
valid_manifest_data["provides"][section] = [
|
||||
{"name": "dup", "file": f"{section}/a.txt"},
|
||||
{"name": "dup", "file": f"{section}/b.txt"},
|
||||
]
|
||||
|
||||
manifest_path = temp_dir / "extension.yml"
|
||||
with open(manifest_path, 'w', encoding="utf-8") as f:
|
||||
yaml.dump(valid_manifest_data, f)
|
||||
|
||||
with pytest.raises(ValidationError, match=f"Duplicate .* name 'dup' in 'provides.{section}'"):
|
||||
ExtensionManifest(manifest_path)
|
||||
|
||||
@pytest.mark.parametrize("section", ["templates", "scripts"])
|
||||
def test_provides_entry_path_traversal_rejected(self, temp_dir, valid_manifest_data, section):
|
||||
"""The 'file' field is checked with the same path-safety policy as commands."""
|
||||
|
||||
Reference in New Issue
Block a user