feat(extensions): scaffold config templates on extension add/enable

[mvanhorn]

Description

Rework of #1929 per @mnriem's feedback. Instead of a standalone specify extension init command, config scaffolding now runs automatically as part of the extension lifecycle:

• extension add: after installing an extension, config templates from provides.config are deployed to .specify/
• extension enable: when re-enabling a disabled extension, missing config templates are deployed

Existing user-customized config files are never overwritten. Extensions without a provides.config section are unaffected.

Changes:

• ExtensionManifest.config property reads provides.config from the manifest
• ExtensionManager.scaffold_config() copies config templates to the project, skipping existing files
• extension_add replaces the "Configuration may be required" warning with automatic deployment
• extension_enable deploys config on re-enable

Video Demo

The demo shows: config template exists in extension dir, no project config yet, scaffold_config deploys it, re-running preserves existing config (idempotent).

Testing

• Tested locally with uv run specify --help
• Ran existing tests with uv sync --extra test && uv run python -m pytest - all 890 tests pass
• Added 4 new tests covering scaffold_config: deploy, preserve existing, no config section, missing template
• Tested with a sample project (see demo above)

AI Disclosure

• I did use AI assistance (describe below)

This contribution was developed with AI assistance (Claude Code + Codex CLI).

Closes #1929

[mnriem]

On a side note you moved us into the two thousands! ;)

[Copilot]

Pull request overview

This PR updates the extension lifecycle so that configuration templates declared by an extension (provides.config) are automatically scaffolded into a project’s .specify/ directory during specify extension add and specify extension enable, without overwriting existing user config.

Changes:

• Add ExtensionManifest.config to expose provides.config from extension.yml.
• Add ExtensionManager.scaffold_config() to copy declared config templates into .specify/, skipping existing files.
• Invoke config scaffolding from extension add and extension enable, and add tests for scaffold behavior.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.

File	Description
src/specify_cli/extensions.py	Adds manifest accessor for config and implements scaffold_config() to deploy templates.
src/specify_cli/__init__.py	Wires scaffolding into extension add and extension enable CLI flows and prints status output.
tests/test_extensions.py	Adds new tests covering config scaffolding deploy/preserve/no-config/missing-template cases.
docs/screenshots/scaffold-config-demo.gif	Adds demo asset showing scaffolding behavior.

Comments suppressed due to low confidence (1)

src/specify_cli/extensions.py:1158

• scaffold_config() assumes every config_entry is a dict (config_entry.get(...)). Since provides.config isn’t currently validated, a non-dict entry will raise AttributeError here. Even with validation, it’s safer to defensively skip non-dict entries (or raise ValidationError) to keep lifecycle commands from crashing on bad extensions.

        for config_entry in manifest.config:
            template_name = config_entry.get("template", "")
            target_name = config_entry.get("name", template_name)
            if not template_name:
                continue

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

In extension_add, when manifest.config is present but scaffold_config() returns an empty list, the message always says “Config files already exist (preserved).” That’s also the result when templates are missing/unreadable and nothing was deployed, which is misleading. Consider having scaffold_config() distinguish “skipped because target existed” vs “skipped because template missing/invalid” (or return richer status) and adjust the CLI messaging accordingly (e.g., warn when templates declared in the manifest can’t be found).

Suggested change

	console.print("\n[dim]Config files already exist (preserved).[/dim]")
	# No configs were scaffolded even though the manifest declares them.
	# Distinguish between "targets already exist" vs "templates/targets missing".
	existing = False
	missing = False
	configs = manifest.config if isinstance(manifest.config, list) else []
	for cfg in configs:
	target_path: Optional[Path] = None
	# Support both simple string entries and dict-based config entries.
	if isinstance(cfg, str):
	target_path = specify_dir / cfg
	elif isinstance(cfg, dict):
	target_value = cfg.get("target") or cfg.get("path")
	if isinstance(target_value, str):
	target_path = specify_dir / target_value
	if target_path is not None:
	if target_path.exists():
	existing = True
	else:
	missing = True
	if existing:
	console.print("\n[dim]Config files already exist (preserved).[/dim]")
	else:
	console.print(
	"\n[yellow]Warning:[/yellow] No config templates were scaffolded. "
	"Declared config templates may be missing or invalid; please verify "
	"your extension manifest and template files."
	)

[Copilot]

scaffold_config() only checks template_path.exists(). If the manifest points to a directory (or a symlink), shutil.copy2() may fail or copy unexpected content. Consider requiring template_path.is_file() (and potentially disallowing symlinks) before copying.

Suggested change

	if not template_path.exists():
	if not template_path.exists() or not template_path.is_file():

[Copilot]

ExtensionManifest.config returns the raw provides.config value without validating its shape. Because the manifest validator currently doesn’t enforce provides.config, a malformed manifest (e.g., config: {} or config: "foo") will make callers like scaffold_config() crash when iterating / calling .get() on entries. Consider normalizing here (return [] unless it’s a list of dicts) or adding validation in _validate() so invalid config sections raise a ValidationError deterministically.

This issue also appears on line 1154 of the same file.

Suggested change

	"""Get list of provided config templates."""
	return self.data.get("provides", {}).get("config", [])
	"""Get list of provided config templates.
	Always returns a list of dicts. If the manifest's ``provides.config``
	field is missing or has an unexpected shape, this returns an empty
	list instead of the raw value.
	"""
	provides = self.data.get("provides", {})
	raw_config = provides.get("config", [])
	# Normalize: require a list of dicts; otherwise, return an empty list.
	if not isinstance(raw_config, list):
	return []
	if not all(isinstance(item, dict) for item in raw_config):
	return []
	return raw_config

[Copilot]

scaffold_config() uses template_name and target_name from the manifest to build paths via ext_dir / template_name and project_root/.specify/target_name without any path-traversal containment checks. A malicious or malformed manifest could use absolute paths or .. segments to read templates from outside the extension dir and/or write outside .specify/. Recommend rejecting non-file names (e.g., require Path(name).is_absolute() == False and len(Path(name).parts)==1, or otherwise resolve() + relative_to() checks to ensure template_path stays within ext_dir and target_path stays within project_root/.specify).

[Copilot]

The new scaffolding behavior is security-sensitive, but the tests don’t currently cover rejection of unsafe template/name values (absolute paths or .. traversal) or malformed provides.config shapes. Adding a couple of tests around scaffold_config() for these cases would help prevent regressions once path containment and type validation are implemented.

Suggested change

	def test_scaffold_config_rejects_unsafe_paths(self, tmp_path):
	"""Unsafe config names/templates (absolute or traversal) must be rejected."""
	from specify_cli.extensions import ExtensionManager
	project = tmp_path / "project"
	specify_dir = project / ".specify"
	specify_dir.mkdir(parents=True)
	ext_dir = specify_dir / "extensions" / "test-ext"
	# Create an extension that advertises unsafe config paths.
	self._make_extension(ext_dir, config_entries=[
	{
	"name": "/absolute-config.yml",
	"template": "config-template.yml",
	"description": "Absolute path should be rejected",
	},
	{
	"name": "../traversal-config.yml",
	"template": "config-template.yml",
	"description": "Path traversal name should be rejected",
	},
	{
	"name": "ok-config.yml",
	"template": "../traversal-template.yml",
	"description": "Path traversal template should be rejected",
	},
	])
	# Provide a real template so that any failure is due to path validation,
	# not missing files.
	(ext_dir / "config-template.yml").write_text("setting: default")
	(ext_dir / "traversal-template.yml").write_text("setting: unsafe")
	manager = ExtensionManager(project)
	with pytest.raises(ExtensionError):
	manager.scaffold_config("test-ext")
	# Ensure that no unsafe files were created outside the .specify directory.
	assert not (project.parent / "absolute-config.yml").exists()
	assert not (project.parent / "traversal-config.yml").exists()
	def test_scaffold_config_rejects_malformed_provides_config(self, tmp_path):
	"""Malformed provides.config entries should cause validation failure."""
	from specify_cli.extensions import ExtensionManager
	project = tmp_path / "project"
	specify_dir = project / ".specify"
	specify_dir.mkdir(parents=True)
	ext_dir = specify_dir / "extensions" / "test-ext"
	# Start with a valid extension manifest, then corrupt provides.config.
	self._make_extension(ext_dir, config_entries=[{
	"name": "test-config.yml",
	"template": "config-template.yml",
	"description": "Test config",
	}])
	(ext_dir / "config-template.yml").write_text("setting: default")
	# Overwrite the manifest so that provides.config has an invalid shape.
	manifest_path = ext_dir / "extension.json"
	if manifest_path.exists():
	manifest_data = json.loads(manifest_path.read_text())
	# Introduce multiple shape issues: config is not a list, and contains
	# a non-mapping entry.
	manifest_data.setdefault("provides", {})
	manifest_data["provides"]["config"] = "not-a-list"
	manifest_path.write_text(json.dumps(manifest_data))
	manager = ExtensionManager(project)
	with pytest.raises(ValidationError):
	manager.scaffold_config("test-ext")
	# No config file should have been created.
	assert not (specify_dir / "test-config.yml").exists()

[mvanhorn]

Ha, PR #2000! Didn't even notice. Lucky number.

[mnriem]

Please address Copilot feedback. If not applicable, please explain why.

[mvanhorn]

Addressed all Copilot feedback in 86606ef:

1. Path traversal protection - scaffold_config() now rejects template/target names with .. segments or absolute paths
2. File type check - uses is_file() instead of exists() to reject directory templates
3. Manifest validation - config property normalizes malformed entries (non-list or non-dict values return [])
4. Richer messaging - returns (deployed, skipped) tuple so the CLI distinguishes "already exists" from "template missing"
5. Security tests - added test coverage for path traversal rejection, directory template rejection, and malformed manifest handling

[Copilot]

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

src/specify_cli/extensions.py:1164

• template_name/target_name are assumed to be strings, but manifest data is untrusted input. If either value is non-string (e.g., int/None), Path(template_name) will raise TypeError and abort scaffolding. Add type checks (or coerce to str only when appropriate) and skip invalid entries to keep scaffolding robust.

        for config_entry in manifest.config:
            template_name = config_entry.get("template", "")
            target_name = config_entry.get("name", template_name)
            if not template_name:
                continue

src/specify_cli/extensions.py:1174

• The current path traversal protection relies on Path(...).is_absolute() and checking for '..' segments. This misses some edge cases (e.g., Windows drive-relative paths like C:foo, and symlinks inside the extension that resolve outside ext_dir). Prefer the same resolve() + relative_to() containment pattern already used for ZIP extraction in this file, and consider rejecting symlinks for templates before copying.

            # Reject path traversal and absolute paths
            if Path(template_name).is_absolute() or ".." in Path(template_name).parts:
                continue
            if Path(target_name).is_absolute() or ".." in Path(target_name).parts:
                continue

            template_path = ext_dir / template_name
            if not template_path.exists() or not template_path.is_file():
                continue

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This test writes an invalid manifest shape (missing required schema_version/extension/requires/provides.commands). ExtensionManifest(...) currently validates those required fields and will raise ValidationError, so the test will fail/crash rather than asserting manifest.config == []. Adjust the test to use a syntactically valid manifest with provides.config set to a non-list, or assert that invalid manifests raise ValidationError.

[Copilot]

extension_enable calls manager.scaffold_config(extension_id) without any error handling. Since scaffold_config currently instantiates ExtensionManifest (which can raise ValidationError on corrupted/missing manifests), this can cause specify extension enable to crash with a traceback. Consider making scaffold_config non-raising (return empty results on validation errors) and/or catching exceptions here to degrade gracefully with a warning.

Suggested change

	deployed, skipped = manager.scaffold_config(extension_id)
	if deployed:
	console.print("\n[bold cyan]Config scaffolded:[/bold cyan]")
	for cfg in deployed:
	console.print(f" • .specify/{cfg}")
	if skipped:
	console.print(f"\n[dim]Config files already exist (preserved): {', '.join(skipped)}[/dim]")
	try:
	deployed, skipped = manager.scaffold_config(extension_id)
	except Exception as exc:
	console.print(
	f"[yellow]Warning:[/yellow] Failed to scaffold config for extension '{display_name}'."
	)
	console.print(f"[dim]Details: {exc}[/dim]")
	deployed, skipped = [], []
	if deployed:
	console.print("\n[bold cyan]Config scaffolded:[/bold cyan]")
	for cfg in deployed:
	console.print(f" • .specify/{cfg}")
	if skipped:
	console.print(
	f"\n[dim]Config files already exist (preserved): {', '.join(skipped)}[/dim]"
	)

[Copilot]

scaffold_config() is documented/used as returning a 2-tuple, but when extension.yml is missing it returns a single list (return []). This will raise a ValueError when callers unpack the result (e.g., in extension_add/extension_enable). Return a consistent 2-tuple like ([], []) (and consider tightening the return type annotation accordingly).