From 6da1375396e0557be8e6a1f84e6f12b5f22fc15c Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Tue, 10 Mar 2026 17:06:34 -0500 Subject: [PATCH] fix: address Copilot PR review comments (round 3) - Fix PS Resolve-Template fallback to skip dot-prefixed dirs (.cache) - Rename _catalog to _catalog_name for consistency with extension system - Enforce install_allowed policy in CLI preset add and download_pack() - Fix shell injection: pass registry path via env var instead of string interpolation --- scripts/bash/common.sh | 7 ++++--- scripts/powershell/common.ps1 | 2 +- src/specify_cli/__init__.py | 6 ++++++ src/specify_cli/presets.py | 9 ++++++++- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index 735b8bd3..fb1ab8ec 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -175,10 +175,11 @@ resolve_template() { if [ -f "$registry_file" ] && command -v python3 >/dev/null 2>&1; then # Read preset IDs sorted by priority (lower number = higher precedence) local sorted_presets - sorted_presets=$(python3 -c " -import json, sys + sorted_presets=$(SPECKIT_REGISTRY="$registry_file" python3 -c " +import json, sys, os try: - data = json.load(open('$registry_file')) + with open(os.environ['SPECKIT_REGISTRY']) as f: + data = json.load(f) presets = data.get('presets', {}) for pid, meta in sorted(presets.items(), key=lambda x: x[1].get('priority', 10)): print(pid) diff --git a/scripts/powershell/common.ps1 b/scripts/powershell/common.ps1 index 63d6dd63..5dee601b 100644 --- a/scripts/powershell/common.ps1 +++ b/scripts/powershell/common.ps1 @@ -179,7 +179,7 @@ function Resolve-Template { } } else { # Fallback: alphabetical directory order - foreach ($preset in Get-ChildItem -Path $presetsDir -Directory -ErrorAction SilentlyContinue) { + foreach ($preset in Get-ChildItem -Path $presetsDir -Directory -ErrorAction SilentlyContinue | Where-Object { $_.Name -notlike '.*' }) { $candidate = Join-Path $preset.FullName "templates/$TemplateName.md" if (Test-Path $candidate) { return $candidate } } diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 07a06b9e..d7e77241 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -2002,6 +2002,12 @@ def preset_add( console.print(f"[red]Error:[/red] Preset '{pack_id}' not found in catalog") raise typer.Exit(1) + if not pack_info.get("_install_allowed", True): + catalog_name = pack_info.get("_catalog_name", "unknown") + console.print(f"[red]Error:[/red] Preset '{pack_id}' is from the '{catalog_name}' catalog which is discovery-only (install not allowed).") + console.print("Add the catalog with --install-allowed or install from the preset's repository directly with --from.") + raise typer.Exit(1) + console.print(f"Installing preset [cyan]{pack_info.get('name', pack_id)}[/cyan]...") try: diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 818f9951..0ea0dcb9 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -1096,7 +1096,7 @@ class PresetCatalog: try: data = self._fetch_single_catalog(entry, force_refresh) for pack_id, pack_data in data.get("presets", {}).items(): - pack_data_with_catalog = {**pack_data, "_catalog": entry.name, "_install_allowed": entry.install_allowed} + pack_data_with_catalog = {**pack_data, "_catalog_name": entry.name, "_install_allowed": entry.install_allowed} merged[pack_id] = pack_data_with_catalog except PresetError: continue @@ -1279,6 +1279,13 @@ class PresetCatalog: f"Preset '{pack_id}' not found in catalog" ) + if not pack_info.get("_install_allowed", True): + catalog_name = pack_info.get("_catalog_name", "unknown") + raise PresetError( + f"Preset '{pack_id}' is from the '{catalog_name}' catalog which does not allow installation. " + f"Use --from with the preset's repository URL instead." + ) + download_url = pack_info.get("download_url") if not download_url: raise PresetError(