diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 8ca29547b7..c42e878c6a 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -45,6 +45,50 @@ may omit the digest. > **Note:** All preset commands require a project already initialized with `specify init`. +### Exclude development files with `.presetignore` + +Place a UTF-8 `.presetignore` (with or without a byte-order mark) beside +`preset.yml` in the preset source root to filter files copied into +`.specify/presets//`. For example: + +```gitignore +# Development-only directories +.git/ +.idea/ +tests/ +``` + +Filtering applies to local directory installs and extracted archives, including +reinstalls and replacements installed by `preset update`. Without +`.presetignore`, all source files are copied as before. **`.gitignore` is not +used for installation filtering**, and there are no implicit exclusions for +development files. + +Patterns follow the same Git-style matching and normalization as +`.extensionignore`: + +- Use one pattern per line; blank lines and `#` comments are ignored. +- `*` and `?` do not cross directory boundaries; `**` can match across directories. +- A trailing `/` matches directories only. A leading `/`, or a `/` within a + pattern other than its trailing slash, anchors the pattern to the source root. + Patterns without such a slash match names at any depth. +- A later `!` pattern re-includes a match. An excluded directory is not traversed, + so re-including a child requires re-including its parent directory too. +- Pattern whitespace is trimmed and backslashes are normalized to `/`, matching + the existing extension behavior. Use UTF-8 for non-ASCII names. + +Only the source root's `.presetignore` is read; nested ignore files do not add +rules. Files named `.presetignore` are themselves excluded, even if negated. +Manifest and declared asset files are not exempt from filtering: keep +`preset.yml` and any templates, commands, or scripts the preset needs included. +Excluding them can leave an incomplete or unusable installation. + +The source-directory installer reads and validates `.presetignore` before +removing an existing destination, including during a forced reinstall. Invalid +UTF-8 raises a preset validation error. The separate `preset update` command +retains its remove-then-add behavior described below; it does not preflight the +replacement ignore file before removal. + ## Update a Preset ```bash diff --git a/src/specify_cli/_install_ignore.py b/src/specify_cli/_install_ignore.py new file mode 100644 index 0000000000..43c7ed7afa --- /dev/null +++ b/src/specify_cli/_install_ignore.py @@ -0,0 +1,71 @@ +"""Shared source-directory installation filtering for extensions and presets.""" + +from pathlib import Path +from typing import Callable, List, Optional, Set + +import pathspec + + +def load_install_ignore( + source_dir: Path, + ignore_filename: str, + validation_error: type[Exception], +) -> Optional[Callable[[str, List[str]], Set[str]]]: + """Load a UTF-8 ignore file as a copytree callback, or return None if absent. + + Match the extension installer's Git-style patterns, including whitespace + trimming and backslash normalization. Only the named root ignore file is + read; there are no default exclusions except the ignore filename itself. + Invalid encoding raises the caller's domain validation error before copying. + Excluded directories are pruned by copytree, so child negations cannot + restore files inside them unless the parent directory is also re-included. + """ + ignore_file = source_dir / ignore_filename + if not ignore_file.exists(): + return None + + # Accept UTF-8 with or without the BOM emitted by Windows PowerShell 5.1. + # Otherwise the BOM becomes part of the first pattern and prevents matching. + try: + raw = ignore_file.read_text(encoding="utf-8-sig") + except UnicodeDecodeError as e: + raise validation_error( + f"{ignore_filename} is not valid UTF-8: {ignore_file} " + f"({e.reason} at byte {e.start})" + ) + lines: List[str] = raw.splitlines() + + # Normalise backslashes in patterns so Windows-authored files work + normalised: List[str] = [] + for line in lines: + stripped = line.strip() + if stripped and not stripped.startswith("#"): + normalised.append(stripped.replace("\\", "/")) + else: + # Preserve blanks/comments so pathspec line numbers stay stable + normalised.append(line) + + # Always ignore the ignore file itself, even after a negation pattern. + normalised.append(ignore_filename) + + spec = pathspec.GitIgnoreSpec.from_lines(normalised) + + def _ignore(directory: str, entries: List[str]) -> Set[str]: + ignored: Set[str] = set() + rel_dir = Path(directory).relative_to(source_dir) + for entry in entries: + rel_path = str(rel_dir / entry) if str(rel_dir) != "." else entry + # Normalise to forward slashes for consistent matching + rel_path_fwd = rel_path.replace("\\", "/") + + entry_full = Path(directory) / entry + if entry_full.is_dir(): + # Append '/' so directory-only patterns (e.g. tests/) match + if spec.match_file(rel_path_fwd + "/"): + ignored.add(entry) + else: + if spec.match_file(rel_path_fwd): + ignored.add(entry) + return ignored + + return _ignore diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index d03cb85770..c22445ebe0 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -26,7 +26,6 @@ from pathlib import Path from typing import Any, BinaryIO, Callable, Dict, List, Optional, Set -import pathspec import yaml from packaging import version as pkg_version from packaging.specifiers import InvalidSpecifier, SpecifierSet @@ -43,6 +42,7 @@ safe_extract_archive, ) from .._init_options import is_ai_skills_enabled +from .._install_ignore import load_install_ignore from .._invocation_style import is_dollar_skills_agent, is_slash_skills_agent from .._utils import dump_frontmatter, relative_extension_path_violation, version_satisfies from ..catalogs import CatalogEntry as BaseCatalogEntry @@ -1275,67 +1275,7 @@ def _load_extensionignore( An ignore function compatible with shutil.copytree, or None if no .extensionignore file exists. """ - ignore_file = source_dir / ".extensionignore" - if not ignore_file.exists(): - return None - - # Pin UTF-8 explicitly: ``Path.read_text`` defaults to the system - # locale codec on Windows (cp1252 / gb2312 / cp932), which silently - # corrupts multibyte patterns when the file is shared across - # machines with different locales. The next line already - # normalises backslashes "so Windows-authored files work" — the - # codebase already expects Windows authors to write this file. - # - # A file that is not valid UTF-8 is a user-authoring mistake, so - # surface it as ``ValidationError`` with a pointer to the offending - # byte — the same pattern ``ExtensionManifest._load_yaml`` uses - # for ``extension.yml`` (see ``UnicodeDecodeError`` handler in - # this module). Without the wrap, the raw ``UnicodeDecodeError`` - # would abort installation with a Python traceback instead of a - # clear message naming the file. - try: - raw = ignore_file.read_text(encoding="utf-8") - except UnicodeDecodeError as e: - raise ValidationError( - f".extensionignore is not valid UTF-8: {ignore_file} " - f"({e.reason} at byte {e.start})" - ) - lines: List[str] = raw.splitlines() - - # Normalise backslashes in patterns so Windows-authored files work - normalised: List[str] = [] - for line in lines: - stripped = line.strip() - if stripped and not stripped.startswith("#"): - normalised.append(stripped.replace("\\", "/")) - else: - # Preserve blanks/comments so pathspec line numbers stay stable - normalised.append(line) - - # Always ignore the .extensionignore file itself - normalised.append(".extensionignore") - - spec = pathspec.GitIgnoreSpec.from_lines(normalised) - - def _ignore(directory: str, entries: List[str]) -> Set[str]: - ignored: Set[str] = set() - rel_dir = Path(directory).relative_to(source_dir) - for entry in entries: - rel_path = str(rel_dir / entry) if str(rel_dir) != "." else entry - # Normalise to forward slashes for consistent matching - rel_path_fwd = rel_path.replace("\\", "/") - - entry_full = Path(directory) / entry - if entry_full.is_dir(): - # Append '/' so directory-only patterns (e.g. tests/) match - if spec.match_file(rel_path_fwd + "/"): - ignored.add(entry) - else: - if spec.match_file(rel_path_fwd): - ignored.add(entry) - return ignored - - return _ignore + return load_install_ignore(source_dir, ".extensionignore", ValidationError) @project_registration def _get_skills_dir(self, *, create: bool = True) -> Optional[Path]: diff --git a/src/specify_cli/presets/_manager.py b/src/specify_cli/presets/_manager.py index 963bb5c640..cf6c61f434 100644 --- a/src/specify_cli/presets/_manager.py +++ b/src/specify_cli/presets/_manager.py @@ -12,6 +12,7 @@ from .._download_security import safe_extract_archive from .._init_options import is_ai_skills_enabled, resolve_active_agent_for_registration +from .._install_ignore import load_install_ignore from .._utils import version_satisfies from ..extensions import REINSTALL_COMMAND, normalize_priority from ..shared_infra import ( @@ -402,13 +403,16 @@ def install_from_directory( f"Preset '{manifest.id}' is already installed. " f"Use 'specify preset remove {manifest.id}' first." ) + # Validate before remove() changes registry/agent artifacts or deleting + # an unregistered destination left behind by an earlier installation. + ignore_fn = load_install_ignore(source_dir, ".presetignore", PresetValidationError) before_file_change(dest_dir, removal=True) try: if installed: self.remove(manifest.id) if dest_dir.exists(): shutil.rmtree(dest_dir) - shutil.copytree(source_dir, dest_dir) + shutil.copytree(source_dir, dest_dir, ignore=ignore_fn) finally: after_file_change(dest_dir) diff --git a/tests/integration/test_preset_update_workflow.py b/tests/integration/test_preset_update_workflow.py index a0d2e5b8a9..1049aff7a0 100644 --- a/tests/integration/test_preset_update_workflow.py +++ b/tests/integration/test_preset_update_workflow.py @@ -162,7 +162,8 @@ def test_preset_update_cli_contract(): assert "--dry-run" not in help_output -def test_preset_update_replaces_installed_preset(project: Path, tmp_path: Path): +@pytest.mark.parametrize("use_ignore", [False, True]) +def test_preset_update_replaces_installed_preset(project: Path, tmp_path: Path, use_ignore): """A successful update really swaps the installed preset on disk.""" runner = CliRunner() @@ -178,6 +179,9 @@ def test_preset_update_replaces_installed_preset(project: Path, tmp_path: Path): body="# Version Two Template\n", extra_file="only-in-v2.md", ) + (replacement / "dev-only.txt").write_text("development notes\n", encoding="utf-8") + if use_ignore: + (replacement / ".presetignore").write_text("dev-only.txt\n", encoding="utf-8") install = runner.invoke( app, ["preset", "add", "--dev", str(original), "--priority", "20"] @@ -201,6 +205,8 @@ def test_preset_update_replaces_installed_preset(project: Path, tmp_path: Path): template = installed_dir / "templates" / "spec-template.md" assert template.read_text(encoding="utf-8") == "# Version Two Template\n" assert (installed_dir / "only-in-v2.md").exists() + assert (installed_dir / "dev-only.txt").exists() is not use_ignore + assert not (installed_dir / ".presetignore").exists() assert not (installed_dir / "only-in-v1.md").exists(), ( "the previous preset's files must be removed, not merged with the " "replacement" diff --git a/tests/specify_cli/presets/test_manager.py b/tests/specify_cli/presets/test_manager.py index 91e0805593..ff1738ab4c 100644 --- a/tests/specify_cli/presets/test_manager.py +++ b/tests/specify_cli/presets/test_manager.py @@ -101,6 +101,121 @@ def unreadable(path, *args, **kwargs): ) +class TestPresetIgnore: + """Installation filtering follows the extension ignore contract.""" + + @staticmethod + def _write_files(root, paths): + for relative in paths: + path = root / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(f"contents of {relative}\n", encoding="utf-8") + + @pytest.mark.parametrize("ignore_content", [None, "", "!.presetignore\n"]) + def test_no_default_exclusions(self, project_dir, pack_dir, ignore_content): + paths = [".git/config", ".idea/project.xml", "tests/test_example.py", "debug.log"] + self._write_files(pack_dir, paths) + (pack_dir / ".gitignore").write_text("*\n", encoding="utf-8") + (pack_dir / ".extensionignore").write_text("*\n", encoding="utf-8") + if ignore_content is not None: + (pack_dir / ".presetignore").write_text(ignore_content, encoding="utf-8") + + PresetManager(project_dir).install_from_directory(pack_dir, "0.1.5") + + dest = project_dir / ".specify/presets/test-pack" + for relative in [ + *paths, ".gitignore", ".extensionignore", "preset.yml", "templates/spec-template.md" + ]: + assert (dest / relative).read_bytes() == (pack_dir / relative).read_bytes() + assert not (dest / ".presetignore").exists() + + @pytest.mark.parametrize("root_ignore", [False, True]) + def test_nested_ignore_is_not_loaded(self, project_dir, pack_dir, root_ignore): + self._write_files(pack_dir, ["nested/keep.txt"]) + (pack_dir / "nested/.presetignore").write_bytes(b"*\n\xff\n") + if root_ignore: + (pack_dir / ".presetignore").write_text("", encoding="utf-8") + + PresetManager(project_dir).install_from_directory(pack_dir, "0.1.5") + + dest = project_dir / ".specify/presets/test-pack/nested" + assert (dest / "keep.txt").is_file() + assert (dest / ".presetignore").exists() is not root_ignore + + @pytest.mark.parametrize("registered", [False, True]) + def test_invalid_utf8_preserves_existing_state( + self, project_dir, pack_dir, valid_pack_data, registered + ): + """Validate before deleting even an unregistered leftover destination.""" + valid_pack_data["provides"]["templates"].append({ + "type": "command", "name": "speckit.ignore.demo", "file": "commands/demo.md", + "description": "Ignore test command", + }) + (pack_dir / "preset.yml").write_text(yaml.safe_dump(valid_pack_data), encoding="utf-8") + self._write_files(pack_dir, ["commands/demo.md"]) + (project_dir / ".claude/skills").mkdir(parents=True) + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") + assert manager.registry.get("test-pack")["registered_commands"] + if not registered: + manager.registry.remove("test-pack") + + def snapshot(): + return { + str(path.relative_to(project_dir)): path.read_bytes() + for path in project_dir.rglob("*") if path.is_file() + } + + before = snapshot() + (pack_dir / ".presetignore").write_bytes(b"caf\xe9/\n") + with pytest.raises(PresetValidationError, match=r"\.presetignore is not valid UTF-8.*byte 3"): + manager.install_from_directory(pack_dir, "0.1.5", force=registered) + + assert snapshot() == before + assert manager.registry.is_installed("test-pack") is registered + + @pytest.mark.parametrize("route", ["directory", "zip", "tar"]) + def test_reinstall_filters_new_source(self, project_dir, pack_dir, temp_dir, route): + self._write_files(pack_dir, ["tests/dev.py", "keep.txt"]) + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") + dest = project_dir / ".specify/presets/test-pack" + assert (dest / "tests/dev.py").is_file() + (pack_dir / ".presetignore").write_text("tests/\n", encoding="utf-8") + (pack_dir / "keep.txt").write_text("updated\n", encoding="utf-8") + + if route == "directory": + manager.install_from_directory(pack_dir, "0.1.5", force=True) + elif route == "zip": + archive_path = temp_dir / "preset.zip" + with zipfile.ZipFile(archive_path, "w") as archive: + for path in pack_dir.rglob("*"): + if path.is_file(): + archive.write(path, Path("package") / path.relative_to(pack_dir)) + manager.install_from_zip(archive_path, "0.1.5", force=True) + else: + archive_path = temp_dir / "preset.tar.gz" + with tarfile.open(archive_path, "w:gz") as archive: + archive.add(pack_dir, arcname="package") + manager.install_from_archive(archive_path, "0.1.5", force=True) + + assert not (dest / "tests").exists() + assert not (dest / ".presetignore").exists() + assert (dest / "keep.txt").read_text(encoding="utf-8") == "updated\n" + assert manager.registry.is_installed("test-pack") + + @pytest.mark.parametrize("excluded", ["preset.yml", "templates/spec-template.md"]) + def test_declared_files_are_not_exempt(self, project_dir, pack_dir, excluded): + """Like extensions, filtering does not protect the manifest or declared assets.""" + (pack_dir / ".presetignore").write_text(excluded + "\n", encoding="utf-8") + manager = PresetManager(project_dir) + manifest = manager.install_from_directory(pack_dir, "0.1.5") + + assert manifest.id == "test-pack" + assert manager.registry.is_installed("test-pack") + assert not (project_dir / ".specify/presets/test-pack" / excluded).exists() + + class TestPresetManager: """Test PresetManager installation and removal.""" diff --git a/tests/specify_cli/test_install_ignore.py b/tests/specify_cli/test_install_ignore.py new file mode 100644 index 0000000000..5ef4215e9c --- /dev/null +++ b/tests/specify_cli/test_install_ignore.py @@ -0,0 +1,130 @@ +"""Contract tests for shared installation filtering in specify_cli._install_ignore.""" + +import shutil +from pathlib import Path + +import pytest + +from specify_cli._install_ignore import load_install_ignore +from specify_cli.extensions import ValidationError +from specify_cli.presets import PresetValidationError + + +@pytest.fixture(params=[ + pytest.param((".extensionignore", ValidationError), id="extension"), + pytest.param((".presetignore", PresetValidationError), id="preset"), +]) +def ignore_contract(request): + return request.param + + +@pytest.mark.parametrize( + ("patterns", "excluded", "included"), + [ + (".git/\n.idea/\ntests/\n", [".git/config", ".idea/x", "tests/a.py"], ["README.md"]), + ("*.pyc\n", ["cache.pyc", "nested/cache.pyc"], ["nested/code.py"]), + ("# comment\n\n notes.txt \n\n", ["notes.txt"], ["README.md"]), + ("docs/internal/draft.md\n", ["docs/internal/draft.md"], ["docs/guide.md"]), + ("docs\\internal\\draft.md\n", ["docs/internal/draft.md"], ["docs/guide.md"]), + ("ドキュメント/\ncafé/\n", ["ドキュメント/private.md", "café/résumé.txt"], ["docs/guide.md"]), + ("docs/*.draft.md\n", ["docs/api.draft.md"], ["docs/sub/api.draft.md"]), + ("docs/**/*.draft.md\n", ["docs/api.draft.md", "docs/sub/api.draft.md"], ["docs/guide.md"]), + ("docs/*.md\n!docs/api.md\n", ["docs/guide.md"], ["docs/api.md"]), + ("/notes.txt\n", ["notes.txt"], ["nested/notes.txt"]), + ("docs/draft.md\n", ["docs/draft.md"], ["nested/docs/draft.md"]), + ("cache/\n", ["cache/data", "nested/cache/data"], ["other/cache"]), + ("file?.txt\n", ["file1.txt"], ["file12.txt"]), + ("../sibling/\n/etc/passwd\n", [], ["sibling/data", "passwd"]), + # copytree never visits an excluded parent; child negation cannot restore it. + ("docs/\n!docs/api.md\n", ["docs/api.md"], ["README.md"]), + ("docs/\n!docs/\n", [], ["docs/api.md"]), + ], +) +def test_pattern_semantics(tmp_path, ignore_contract, patterns, excluded, included): + """Both callers use the same pattern contract with copytree traversal.""" + ignore_filename, validation_error = ignore_contract + source = tmp_path / "source" + source.mkdir() + for relative in [*excluded, *included, "keep.txt"]: + path = source / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(f"contents of {relative}\n", encoding="utf-8") + (source / ignore_filename).write_text(patterns, encoding="utf-8") + + dest = tmp_path / "installed" + shutil.copytree(source, dest, ignore=load_install_ignore(source, ignore_filename, validation_error)) + + for relative in excluded: + assert not (dest / relative).exists() + for relative in [*included, "keep.txt"]: + assert (dest / relative).read_bytes() == (source / relative).read_bytes() + assert not (dest / ignore_filename).exists() + + +def test_absent_ignore_returns_no_filter(tmp_path, ignore_contract): + ignore_filename, validation_error = ignore_contract + (tmp_path / ".gitignore").write_text("*\n", encoding="utf-8") + + assert load_install_ignore(tmp_path, ignore_filename, validation_error) is None + + +@pytest.mark.parametrize("directory", ["tests", "café"]) +def test_utf8_bom_first_pattern(tmp_path, ignore_contract, directory): + """Accept the UTF-8 BOM and CRLF bytes emitted by Windows PowerShell 5.1.""" + ignore_filename, validation_error = ignore_contract + source = tmp_path / "source" + (source / directory).mkdir(parents=True) + (source / directory / "private.txt").write_text("development only", encoding="utf-8") + (source / "keep.txt").write_text("shipped", encoding="utf-8") + (source / ignore_filename).write_bytes(b"\xef\xbb\xbf" + f"{directory}/\r\n".encode("utf-8")) + + dest = tmp_path / "installed" + shutil.copytree(source, dest, ignore=load_install_ignore(source, ignore_filename, validation_error)) + + assert not (dest / directory).exists() + assert (dest / "keep.txt").read_text(encoding="utf-8") == "shipped" + assert not (dest / ignore_filename).exists() + + +@pytest.mark.parametrize("negate_self", [False, True]) +def test_ignore_file_is_always_excluded(tmp_path, ignore_contract, negate_self): + ignore_filename, validation_error = ignore_contract + source = tmp_path / "source" + source.mkdir() + (source / ignore_filename).write_text( + f"!{ignore_filename}\n" if negate_self else "", encoding="utf-8" + ) + (source / "keep.txt").write_text("keep\n", encoding="utf-8") + + dest = tmp_path / "installed" + shutil.copytree(source, dest, ignore=load_install_ignore(source, ignore_filename, validation_error)) + + assert sorted(path.name for path in dest.iterdir()) == ["keep.txt"] + + +def test_invalid_utf8_uses_callers_validation_error(tmp_path, ignore_contract): + ignore_filename, validation_error = ignore_contract + ignore_file = tmp_path / ignore_filename + ignore_file.write_bytes(b"caf\xe9/\n") + + with pytest.raises(validation_error) as caught: + load_install_ignore(tmp_path, ignore_filename, validation_error) + + assert f"{ignore_filename} is not valid UTF-8: {ignore_file}" in str(caught.value) + assert "byte 3" in str(caught.value) + + +def test_read_failure_is_propagated(tmp_path, ignore_contract, monkeypatch): + ignore_filename, validation_error = ignore_contract + ignore_file = tmp_path / ignore_filename + ignore_file.write_text("tests/\n", encoding="utf-8") + original_read = Path.read_text + + def fail_read(path, *args, **kwargs): + if path == ignore_file: + raise PermissionError("ignore file is unreadable") + return original_read(path, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", fail_read) + with pytest.raises(PermissionError, match="ignore file is unreadable"): + load_install_ignore(tmp_path, ignore_filename, validation_error) diff --git a/tests/test_extensions.py b/tests/test_extensions.py index b512742389..f2a75d08a4 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -7190,6 +7190,21 @@ def test_search_results_include_catalog_metadata(self, temp_dir): class TestExtensionIgnore: """Test .extensionignore support during extension installation.""" + @pytest.mark.parametrize("excluded", ["extension.yml", "commands/hello.md"]) + def test_declared_files_are_not_exempt(self, temp_dir, valid_manifest_data, excluded): + """Filtering does not implicitly protect manifests or declared command files.""" + ext_dir = self._make_extension( + temp_dir, valid_manifest_data, ignore_content=excluded + "\n" + ) + project = temp_dir / "project" + (project / ".specify").mkdir(parents=True) + manager = ExtensionManager(project) + + manifest = manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) + + assert manager.registry.is_installed(manifest.id) + assert not (project / ".specify/extensions" / manifest.id / excluded).exists() + def _make_extension(self, temp_dir, valid_manifest_data, extra_files=None, ignore_content=None): """Helper to create an extension directory with optional extra files and .extensionignore.""" import yaml