From fd365efa9b2426e0d1f4be57c28e1bf9194ce426 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 29 Oct 2026 09:18:29 -0500 Subject: [PATCH 1/4] fix: contain persisted feature paths and protect plan creation Require stored feature directories to resolve within the project while retaining explicit external overrides. Reject linked plan destinations and create new plans exclusively across all three script ports. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/reference/core.md | 2 +- scripts/bash/common.sh | 41 ++++++- scripts/bash/setup-plan.sh | 10 +- scripts/powershell/common.ps1 | 65 ++++++++++- scripts/powershell/setup-plan.ps1 | 23 +++- scripts/python/common.py | 15 +++ scripts/python/setup_plan.py | 11 +- tests/test_setup_plan_python_parity.py | 142 +++++++++++++++++++++++++ 8 files changed, 301 insertions(+), 8 deletions(-) diff --git a/docs/reference/core.md b/docs/reference/core.md index 82f976e3ed..c99e0315e6 100644 --- a/docs/reference/core.md +++ b/docs/reference/core.md @@ -83,7 +83,7 @@ specify init my-project --integration copilot --preset compliance | `SPECIFY_FEATURE` | Explicitly override the active feature **label** (e.g. `001-photo-albums`) — the identifier the core helpers report as the current feature/branch (`get_current_branch` in Bash, `Get-CurrentBranch` in PowerShell). Those helpers never inspect Git: when the variable is set they return it verbatim, and when it is unset they return an empty string. The basename fallback happens later — `get_feature_paths` / `Get-FeaturePathsEnv` substitute the resolved feature directory's basename so the reported label is still usable — so calling the named helpers directly does **not** give you that fallback. You set it yourself: the Bash and Python feature scripts only **print** a commented `export SPECIFY_FEATURE=…` / `$env:SPECIFY_FEATURE = …` hint for you to run, because a child process cannot change its parent's environment, and `/speckit.specify` persists `feature_directory` to `.specify/feature.json` instead of setting this variable. (The PowerShell feature scripts do assign `$env:SPECIFY_FEATURE`, but that only reaches you when the script runs inside your current PowerShell session.) It does **not** locate the feature directory: with only `SPECIFY_FEATURE` set, `get_feature_paths` fails with *"Feature directory not found. Set `SPECIFY_FEATURE_DIRECTORY` or run the specify command to create `.specify/feature.json`."* Use `SPECIFY_FEATURE_DIRECTORY` (above) or `.specify/feature.json` to select the directory. | | `SPECIFY_FEATURE_NO_PERSIST` | Set to `1` or `true` to stop every core script from writing `.specify/feature.json`, even when it would otherwise persist `SPECIFY_FEATURE_DIRECTORY` on read. Useful when multiple agents run concurrently against the same checkout, each with its own `SPECIFY_FEATURE_DIRECTORY`: without it, each invocation's persist step can overwrite another agent's pinned feature directory. | -> **Two resolution axes.** `SPECIFY_INIT_DIR` selects the **project** (which directory contains `.specify/`); `SPECIFY_FEATURE_DIRECTORY` / `.specify/feature.json` select the **feature** — its default location is under `specs/` in that project, but an explicitly selected feature directory is not restricted to living inside it. They are independent — project first, then feature. +> **Two resolution axes.** `SPECIFY_INIT_DIR` selects the **project** (which directory contains `.specify/`); `SPECIFY_FEATURE_DIRECTORY` / `.specify/feature.json` select the **feature**. Paths loaded from `.specify/feature.json` must resolve inside the project, including after resolving symbolic links and `..` components. External feature directories require an explicit `SPECIFY_FEATURE_DIRECTORY` override on every invocation, even if a previous override was persisted to `feature.json`. Plan setup rejects symbolic links at the destination `plan.md` and creates new plans exclusively, without overwriting existing files. > > **Version control.** `specify init` scaffolds a managed `.specify/.gitignore` that excludes machine-local state — `feature.json` (the current-feature pointer, rewritten on every feature switch) and per-machine extension `extensions/*/local-config.yml` overrides — while leaving everything else under `.specify/` (constitution, templates, scripts, extension config) shareable so teams stay aligned. Like the rest of `.specify/`'s shared scripts and templates, the file is tracked in the shared-infrastructure manifest: your edits are preserved on re-init and `specify init --here --force` restores the managed content. It is intentionally left in place by `specify integration uninstall`, which only removes the uninstalled agent's own files. > diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index ff18cba4d9..006c91a93d 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -160,6 +160,36 @@ _persist_feature_json() { fi } +# Resolve existing links and missing directory suffixes without creating anything. +_resolve_feature_directory() { + local remaining="$1" resolved="/" component target links=0 + while [[ -n "$remaining" ]]; do + component="${remaining%%/*}" + if [[ "$remaining" == */* ]]; then + remaining="${remaining#*/}" + else + remaining="" + fi + case "$component" in + ""|.) continue ;; + ..) resolved="$(dirname -- "$resolved")"; continue ;; + esac + target="${resolved%/}/$component" + if [[ -L "$target" ]]; then + links=$((links + 1)) + [[ "$links" -le 40 ]] || return 1 + target="$(readlink -- "$target")" || return 1 + remaining="$target${remaining:+/$remaining}" + [[ "$target" != /* ]] || resolved="/" + elif [[ -e "$target" && ! -d "$target" ]]; then + return 1 + else + resolved="$target" + fi + done + printf '%s\n' "$resolved" +} + get_feature_paths() { # Read-only callers (e.g. check-prerequisites.sh --paths-only) pass # --no-persist so pure path resolution never writes .specify/feature.json, @@ -193,8 +223,8 @@ get_feature_paths() { feature_dir="$SPECIFY_FEATURE_DIRECTORY" # Normalize relative paths to absolute under repo root [[ "$feature_dir" != /* ]] && feature_dir="$repo_root/$feature_dir" - # Persist to feature.json so future sessions without the env var still - # work — unless the caller opted out for read-only resolution (#3025). + # Persist unless read-only resolution was requested (#3025). External + # paths still require the explicit override on subsequent invocations. if [[ "$no_persist" != true ]]; then _persist_feature_json "$repo_root" "$SPECIFY_FEATURE_DIRECTORY" fi @@ -205,6 +235,13 @@ get_feature_paths() { feature_dir="$_fd" # Normalize relative paths to absolute under repo root [[ "$feature_dir" != /* ]] && feature_dir="$repo_root/$feature_dir" + local physical_root + physical_root=$(CDPATH="" cd -- "$repo_root" && pwd -P) || return 1 + if ! feature_dir=$(_resolve_feature_directory "$feature_dir") || + [[ "$feature_dir" != "$physical_root" && "$feature_dir" != "$physical_root/"* ]]; then + echo "ERROR: Stored feature directory is invalid or outside the project. Set SPECIFY_FEATURE_DIRECTORY explicitly to use an external directory." >&2 + return 1 + fi else echo "ERROR: Feature directory not found. Set SPECIFY_FEATURE_DIRECTORY or ensure .specify/feature.json contains feature_directory." >&2 return 1 diff --git a/scripts/bash/setup-plan.sh b/scripts/bash/setup-plan.sh index 9c8d163409..615ff33cff 100644 --- a/scripts/bash/setup-plan.sh +++ b/scripts/bash/setup-plan.sh @@ -35,6 +35,11 @@ unset _paths_output # Ensure the feature directory exists mkdir -p "$FEATURE_DIR" +if [[ -L "$IMPL_PLAN" ]]; then + echo "ERROR: Plan destination must not be a symbolic link." >&2 + exit 1 +fi + # Copy plan template if plan doesn't already exist if [[ -f "$IMPL_PLAN" ]]; then if $JSON_MODE; then @@ -51,7 +56,10 @@ else echo "Restore a valid plan template and rerun plan setup." >&2 exit 1 fi - cat "$template_file" > "$IMPL_PLAN" + if ! (set -o noclobber; cat "$template_file" > "$IMPL_PLAN"); then + echo "ERROR: Cannot create plan: $IMPL_PLAN" >&2 + exit 1 + fi if $JSON_MODE; then echo "Copied plan template to $IMPL_PLAN" >&2 else diff --git a/scripts/powershell/common.ps1 b/scripts/powershell/common.ps1 index c0aa9154b5..aeed4cf039 100644 --- a/scripts/powershell/common.ps1 +++ b/scripts/powershell/common.ps1 @@ -158,6 +158,50 @@ function Save-FeatureJson { [System.IO.File]::WriteAllText($fjPath, $json, $utf8NoBom) } +function Resolve-FeatureDirectoryPath { + param([string]$Path, [int]$LinkCount = 0) + + if ($LinkCount -gt 40) { throw 'Too many symbolic links in feature directory' } + $root = [System.IO.Path]::GetPathRoot($Path) + if (-not $root) { throw 'Feature directory must be absolute' } + $resolved = $root + $separators = [regex]::Escape( + [string][System.IO.Path]::DirectorySeparatorChar + + [string][System.IO.Path]::AltDirectorySeparatorChar + ) + foreach ($component in ($Path.Substring($root.Length) -split "[$separators]")) { + if (-not $component -or $component -eq '.') { continue } + if ($component -eq '..') { + $parent = [System.IO.Path]::GetDirectoryName($resolved.TrimEnd('/', '\')) + $resolved = if ($parent) { $parent } else { $root } + continue + } + $candidate = Join-Path $resolved $component + $entry = $null + if (Test-Path -LiteralPath $candidate) { + # Get-Item recognizes Windows short-name and trailing-dot aliases too. + $entry = Get-Item -LiteralPath $candidate -Force -ErrorAction Stop + } elseif (Test-Path -LiteralPath $resolved -PathType Container) { + # Parent enumeration also sees dangling links on Windows PowerShell 5.1. + $entry = Get-ChildItem -LiteralPath $resolved -Force -ErrorAction Stop | + Where-Object { $_.Name -eq $component } | Select-Object -First 1 + } + if ($entry -and ($entry.Attributes -band [System.IO.FileAttributes]::ReparsePoint)) { + $target = @($entry.Target)[0] + if (-not $target) { throw "Cannot resolve reparse point: $candidate" } + if (-not [System.IO.Path]::IsPathRooted($target)) { + $target = Join-Path $resolved $target + } + $resolved = Resolve-FeatureDirectoryPath -Path $target -LinkCount ($LinkCount + 1) + } elseif ($entry -and -not $entry.PSIsContainer) { + throw "Feature directory component is not a directory: $candidate" + } else { + $resolved = $candidate + } + } + return $resolved +} + function Get-FeaturePathsEnv { # Read-only callers (e.g. check-prerequisites.ps1 -PathsOnly) pass -NoPersist # so pure path resolution never writes .specify/feature.json, which would @@ -188,8 +232,8 @@ function Get-FeaturePathsEnv { if (-not [System.IO.Path]::IsPathRooted($featureDir)) { $featureDir = Join-Path $repoRoot $featureDir } - # Persist to feature.json so future sessions without the env var still - # work - unless the caller opted out for read-only resolution (#3025). + # Persist unless read-only resolution was requested (#3025). External + # paths still require the explicit override on subsequent invocations. if (-not $noPersist) { Save-FeatureJson -RepoRoot $repoRoot -FeatureDirectory $env:SPECIFY_FEATURE_DIRECTORY } @@ -208,6 +252,23 @@ function Get-FeaturePathsEnv { if (-not [System.IO.Path]::IsPathRooted($featureDir)) { $featureDir = Join-Path $repoRoot $featureDir } + try { + $featureDir = Resolve-FeatureDirectoryPath -Path $featureDir + $physicalRoot = Resolve-FeatureDirectoryPath -Path $repoRoot + $prefix = $physicalRoot.TrimEnd('/', '\') + [System.IO.Path]::DirectorySeparatorChar + $comparison = [System.StringComparison]::Ordinal + if ([System.IO.Path]::DirectorySeparatorChar -eq '\') { + $comparison = [System.StringComparison]::OrdinalIgnoreCase + } + if (-not $featureDir.Equals($physicalRoot, $comparison) -and + -not $featureDir.StartsWith($prefix, $comparison)) { + throw 'Feature directory escapes the project' + } + } catch { + [Console]::Error.WriteLine("ERROR: Stored feature directory is invalid or outside the project. Set SPECIFY_FEATURE_DIRECTORY explicitly to use an external directory.") + if ($ReturnNullOnError) { return $null } + exit 1 + } } else { [Console]::Error.WriteLine("ERROR: Feature directory not found. Set SPECIFY_FEATURE_DIRECTORY or ensure .specify/feature.json contains feature_directory.") if ($ReturnNullOnError) { return $null } diff --git a/scripts/powershell/setup-plan.ps1 b/scripts/powershell/setup-plan.ps1 index 88b342c8bb..4d76f35ef3 100644 --- a/scripts/powershell/setup-plan.ps1 +++ b/scripts/powershell/setup-plan.ps1 @@ -38,6 +38,13 @@ if (-not $paths) { # Ensure the feature directory exists New-Item -ItemType Directory -Path $paths.FEATURE_DIR -Force | Out-Null +$planEntry = Get-ChildItem -LiteralPath $paths.FEATURE_DIR -Force -ErrorAction Stop | + Where-Object { $_.Name -eq 'plan.md' } | Select-Object -First 1 +if ($planEntry -and ($planEntry.Attributes -band [System.IO.FileAttributes]::ReparsePoint)) { + [Console]::Error.WriteLine("ERROR: Plan destination must not be a symbolic link.") + exit 1 +} + # Copy plan template if plan doesn't already exist if (Test-Path -LiteralPath $paths.IMPL_PLAN -PathType Leaf) { if ($Json) { @@ -56,7 +63,21 @@ if (Test-Path -LiteralPath $paths.IMPL_PLAN -PathType Leaf) { exit 1 } $utf8NoBom = New-Object System.Text.UTF8Encoding($false) - [System.IO.File]::WriteAllText($paths.IMPL_PLAN, $content, $utf8NoBom) + try { + $plan = [System.IO.File]::Open( + $paths.IMPL_PLAN, [System.IO.FileMode]::CreateNew, + [System.IO.FileAccess]::Write, [System.IO.FileShare]::None + ) + try { + $bytes = $utf8NoBom.GetBytes($content) + $plan.Write($bytes, 0, $bytes.Length) + } finally { + $plan.Dispose() + } + } catch { + [Console]::Error.WriteLine("ERROR: Cannot create plan: $($_.Exception.Message)") + exit 1 + } # Emit the copy status like the bash twin (setup-plan.sh); route to stderr # in -Json mode so stdout stays pure JSON, matching the sibling messages. if ($Json) { diff --git a/scripts/python/common.py b/scripts/python/common.py index 0cd17e013d..9655e165f0 100644 --- a/scripts/python/common.py +++ b/scripts/python/common.py @@ -165,6 +165,21 @@ def get_feature_paths( feature_dir = Path(stored) if not feature_dir.is_absolute(): feature_dir = repo_root / feature_dir + try: + # Non-strict resolve() tolerates link loops on newer Python versions. + try: + feature_dir.stat() + except FileNotFoundError: + pass + feature_dir = feature_dir.resolve() + feature_dir.relative_to(repo_root.resolve()) + except (OSError, RuntimeError, ValueError) as exc: + print( + "ERROR: Stored feature directory is invalid or outside the project. " + "Set SPECIFY_FEATURE_DIRECTORY explicitly to use an external directory.", + file=sys.stderr, + ) + raise SystemExit(1) from exc else: print( "ERROR: Feature directory not found. Set SPECIFY_FEATURE_DIRECTORY " diff --git a/scripts/python/setup_plan.py b/scripts/python/setup_plan.py index 113b27ca2c..8ccc50e6f2 100644 --- a/scripts/python/setup_plan.py +++ b/scripts/python/setup_plan.py @@ -57,6 +57,10 @@ def main(argv: list[str] | None = None) -> int: paths.feature_dir.mkdir(parents=True, exist_ok=True) + if paths.impl_plan.is_symlink(): + print("ERROR: Plan destination must not be a symbolic link.", file=sys.stderr) + return 1 + # Status messages go to stderr in JSON mode so stdout stays pure JSON. status_stream = sys.stderr if json_mode else sys.stdout if paths.impl_plan.is_file(): @@ -81,7 +85,12 @@ def main(argv: list[str] | None = None) -> int: file=sys.stderr, ) return 1 - paths.impl_plan.write_bytes(template_content.encode("utf-8")) + try: + with paths.impl_plan.open("xb") as plan: + plan.write(template_content.encode("utf-8")) + except OSError as exc: + print(f"ERROR: Cannot create plan: {exc}", file=sys.stderr) + return 1 print(f"Copied plan template to {paths.impl_plan}", file=status_stream) else: print( diff --git a/tests/test_setup_plan_python_parity.py b/tests/test_setup_plan_python_parity.py index 5c5ea1c8e4..a63ef4497f 100644 --- a/tests/test_setup_plan_python_parity.py +++ b/tests/test_setup_plan_python_parity.py @@ -10,6 +10,8 @@ from tests.parity_helpers import ( HAS_POWERSHELL, POWERSHELL_EXE, + WINDOWS_POWERSHELL, + _bash_posix_path, bash_cmd, break_wrap_layer, clean_env, @@ -33,10 +35,23 @@ marks=pytest.mark.skipif(not HAS_POWERSHELL, reason="no PowerShell available"), ), pytest.param("python"), + pytest.param( + "windows-powershell", + marks=pytest.mark.skipif( + not WINDOWS_POWERSHELL, reason="Windows PowerShell 5.1 not available" + ), + ), ] def _command(repo: Path, port: str, json_mode: bool) -> list[str]: + if port == "windows-powershell": + assert WINDOWS_POWERSHELL + return [ + WINDOWS_POWERSHELL, "-NoProfile", "-File", + str(repo / ".specify" / "scripts" / "powershell" / f"{SCRIPT}.ps1"), + *(("-Json",) if json_mode else ()), + ] if port == "powershell": return ps_cmd(repo, SCRIPT, *(("-Json",) if json_mode else ())) command = bash_cmd if port == "bash" else py_cmd @@ -60,6 +75,133 @@ def repo(tmp_path: Path) -> Path: return _setup_repo(tmp_path) +@pytest.mark.parametrize("port", PORTS) +@pytest.mark.parametrize( + "kind", + ["relative", "absolute", "prefix", "ancestor_link", "dangling_ancestor", "link_parent"], +) +def test_stored_feature_cannot_escape_project( + repo: Path, port: str, kind: str +) -> None: + outside = repo.parent / ("proj-other" if kind == "prefix" else "outside") + if kind in {"ancestor_link", "dangling_ancestor", "link_parent"}: + if kind != "dangling_ancestor": + outside.mkdir() + try: + (repo / "linked").symlink_to(outside, target_is_directory=True) + except OSError as exc: + pytest.skip(f"Cannot create directory symlink: {exc}") + value = "linked/new-feature" + if kind == "link_parent": + value = "linked/../outside/new-feature" + elif kind == "relative": + value = "../outside/new-feature" + else: + value = ( + _bash_posix_path(outside / "new-feature") + if port == "bash" else str(outside / "new-feature") + ) + write_feature_json(repo, value) + before = (repo / ".specify" / "feature.json").read_bytes() + + result = run(_command(repo, port, True), repo) + + assert result.returncode != 0 + assert result.stdout == "" + assert "outside the project" in result.stderr + assert not (outside / "new-feature").exists() + assert (repo / ".specify" / "feature.json").read_bytes() == before + + +@pytest.mark.parametrize("port", PORTS) +def test_stored_feature_link_loop_is_rejected(repo: Path, port: str) -> None: + try: + (repo / "loop").symlink_to("loop", target_is_directory=True) + except OSError as exc: + pytest.skip(f"Cannot create directory symlink: {exc}") + write_feature_json(repo, "loop/new-feature") + + result = run(_command(repo, port, True), repo, timeout=15) + + assert result.returncode != 0 + assert result.stdout == "" + assert "outside the project" in result.stderr + + +@pytest.mark.parametrize("port", PORTS) +@pytest.mark.parametrize("absolute", [False, True]) +def test_external_feature_requires_override_on_every_invocation( + repo: Path, port: str, absolute: bool +) -> None: + outside = repo.parent / "outside" / "new-feature" + env = clean_env() + absolute_value = _bash_posix_path(outside) if port == "bash" else str(outside) + env["SPECIFY_FEATURE_DIRECTORY"] = absolute_value if absolute else "../outside/new-feature" + + result = run(_command(repo, port, True), repo, env) + + assert result.returncode == 0, result.stderr + assert (outside / "plan.md").read_text(encoding="utf-8") == TEMPLATE_BODY + retry = run(_command(repo, port, True), repo) + assert retry.returncode != 0 + assert "outside the project" in retry.stderr + + +@pytest.mark.parametrize("port", PORTS) +@pytest.mark.parametrize("existing", [False, True]) +@pytest.mark.parametrize("override", [False, True]) +def test_plan_symlink_is_rejected( + repo: Path, port: str, existing: bool, override: bool +) -> None: + target = repo.parent / "target.md" + if existing: + target.write_bytes(b"preserve me") + plan = repo / "specs" / "001-my-feature" / "plan.md" + try: + plan.symlink_to(target) + except OSError as exc: + pytest.skip(f"Cannot create file symlink: {exc}") + env = clean_env() + if override: + env["SPECIFY_FEATURE_DIRECTORY"] = ( + _bash_posix_path(plan.parent) if port == "bash" else str(plan.parent) + ) + + result = run(_command(repo, port, True), repo, env) + + assert result.returncode != 0 + assert result.stdout == "" + assert "symbolic link" in result.stderr + assert plan.is_symlink() + assert target.exists() == existing + if existing: + assert target.read_bytes() == b"preserve me" + + +@pytest.mark.parametrize("port", PORTS) +@pytest.mark.parametrize("kind", ["relative", "absolute", "normalized", "internal_link"]) +def test_contained_stored_feature_is_supported(repo: Path, port: str, kind: str) -> None: + feature = repo / "specs" / "new-feature" + value = "specs/new-feature" + if kind == "absolute": + value = _bash_posix_path(feature) if port == "bash" else str(feature) + elif kind == "normalized": + value = "specs/unused/../new-feature" + elif kind == "internal_link": + try: + (repo / "linked").symlink_to(repo / "specs", target_is_directory=True) + except OSError as exc: + pytest.skip(f"Cannot create directory symlink: {exc}") + value = "linked/new-feature" + write_feature_json(repo, value) + + result = run(_command(repo, port, True), repo) + + assert result.returncode == 0, result.stderr + assert (feature / "plan.md").read_text(encoding="utf-8") == TEMPLATE_BODY + assert not (repo / "specs" / "unused").exists() + + def test_python_json_preserves_non_ascii_feature_on_legacy_stdout(repo: Path) -> None: feature = repo / "specs/001-功能" feature.mkdir() From 10b557989ca821d0794e87b21e4cec24feaa4938 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 29 Oct 2026 11:25:07 -0500 Subject: [PATCH 2/4] fix: address feature path containment review Publish Bash plans without opening existing destinations. Bound total PowerShell link traversals, preserve non-link reparse directories, and use platform-correct name matching. Correct Windows path assertions and propagate strict Python resolution failures with regression coverage. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/reference/core.md | 2 +- scripts/bash/setup-plan.sh | 8 +- scripts/powershell/common.ps1 | 18 +- scripts/powershell/setup-plan.ps1 | 6 +- scripts/python/common.py | 7 +- tests/test_setup_plan_python_parity.py | 271 ++++++++++++++++++++++++- 6 files changed, 292 insertions(+), 20 deletions(-) diff --git a/docs/reference/core.md b/docs/reference/core.md index c99e0315e6..5ff5139082 100644 --- a/docs/reference/core.md +++ b/docs/reference/core.md @@ -83,7 +83,7 @@ specify init my-project --integration copilot --preset compliance | `SPECIFY_FEATURE` | Explicitly override the active feature **label** (e.g. `001-photo-albums`) — the identifier the core helpers report as the current feature/branch (`get_current_branch` in Bash, `Get-CurrentBranch` in PowerShell). Those helpers never inspect Git: when the variable is set they return it verbatim, and when it is unset they return an empty string. The basename fallback happens later — `get_feature_paths` / `Get-FeaturePathsEnv` substitute the resolved feature directory's basename so the reported label is still usable — so calling the named helpers directly does **not** give you that fallback. You set it yourself: the Bash and Python feature scripts only **print** a commented `export SPECIFY_FEATURE=…` / `$env:SPECIFY_FEATURE = …` hint for you to run, because a child process cannot change its parent's environment, and `/speckit.specify` persists `feature_directory` to `.specify/feature.json` instead of setting this variable. (The PowerShell feature scripts do assign `$env:SPECIFY_FEATURE`, but that only reaches you when the script runs inside your current PowerShell session.) It does **not** locate the feature directory: with only `SPECIFY_FEATURE` set, `get_feature_paths` fails with *"Feature directory not found. Set `SPECIFY_FEATURE_DIRECTORY` or run the specify command to create `.specify/feature.json`."* Use `SPECIFY_FEATURE_DIRECTORY` (above) or `.specify/feature.json` to select the directory. | | `SPECIFY_FEATURE_NO_PERSIST` | Set to `1` or `true` to stop every core script from writing `.specify/feature.json`, even when it would otherwise persist `SPECIFY_FEATURE_DIRECTORY` on read. Useful when multiple agents run concurrently against the same checkout, each with its own `SPECIFY_FEATURE_DIRECTORY`: without it, each invocation's persist step can overwrite another agent's pinned feature directory. | -> **Two resolution axes.** `SPECIFY_INIT_DIR` selects the **project** (which directory contains `.specify/`); `SPECIFY_FEATURE_DIRECTORY` / `.specify/feature.json` select the **feature**. Paths loaded from `.specify/feature.json` must resolve inside the project, including after resolving symbolic links and `..` components. External feature directories require an explicit `SPECIFY_FEATURE_DIRECTORY` override on every invocation, even if a previous override was persisted to `feature.json`. Plan setup rejects symbolic links at the destination `plan.md` and creates new plans exclusively, without overwriting existing files. +> **Two resolution axes.** `SPECIFY_INIT_DIR` selects the **project** (which directory contains `.specify/`); `SPECIFY_FEATURE_DIRECTORY` / `.specify/feature.json` select the **feature**. Paths loaded from `.specify/feature.json` must resolve inside the project, including after resolving symbolic links and `..` components. External feature directories require an explicit `SPECIFY_FEATURE_DIRECTORY` override on every invocation, even if a previous override was persisted to `feature.json`. Plan setup rejects symbolic links at the destination `plan.md` and creates new plans exclusively, without overwriting existing files. Bash plan creation uses `link(1)` to publish the completed plan atomically and requires a filesystem that supports hard links. > > **Version control.** `specify init` scaffolds a managed `.specify/.gitignore` that excludes machine-local state — `feature.json` (the current-feature pointer, rewritten on every feature switch) and per-machine extension `extensions/*/local-config.yml` overrides — while leaving everything else under `.specify/` (constitution, templates, scripts, extension config) shareable so teams stay aligned. Like the rest of `.specify/`'s shared scripts and templates, the file is tracked in the shared-infrastructure manifest: your edits are preserved on re-init and `specify init --here --force` restores the managed content. It is intentionally left in place by `specify integration uninstall`, which only removes the uninstalled agent's own files. > diff --git a/scripts/bash/setup-plan.sh b/scripts/bash/setup-plan.sh index 615ff33cff..02d6d2667d 100644 --- a/scripts/bash/setup-plan.sh +++ b/scripts/bash/setup-plan.sh @@ -48,15 +48,17 @@ if [[ -f "$IMPL_PLAN" ]]; then echo "Plan already exists at $IMPL_PLAN, skipping template copy" fi else - template_file=$(mktemp) - trap 'rm -f "$template_file"' EXIT + template_dir=$(mktemp -d "$FEATURE_DIR/.plan-template.XXXXXX") + template_file="$template_dir/plan.md" + trap 'rm -f "$template_file"; rmdir "$template_dir"' EXIT if resolve_template_content "plan-template" "$REPO_ROOT" > "$template_file"; then if [[ ! -s "$template_file" ]]; then echo "ERROR: Plan template is empty; no new plan.md was created." >&2 echo "Restore a valid plan template and rerun plan setup." >&2 exit 1 fi - if ! (set -o noclobber; cat "$template_file" > "$IMPL_PLAN"); then + # link(1) publishes atomically without opening any existing destination. + if ! link "$template_file" "$IMPL_PLAN"; then echo "ERROR: Cannot create plan: $IMPL_PLAN" >&2 exit 1 fi diff --git a/scripts/powershell/common.ps1 b/scripts/powershell/common.ps1 index aeed4cf039..ceb3d51dfe 100644 --- a/scripts/powershell/common.ps1 +++ b/scripts/powershell/common.ps1 @@ -159,12 +159,15 @@ function Save-FeatureJson { } function Resolve-FeatureDirectoryPath { - param([string]$Path, [int]$LinkCount = 0) + param([string]$Path, [ref]$LinkCount = ([ref]0)) - if ($LinkCount -gt 40) { throw 'Too many symbolic links in feature directory' } $root = [System.IO.Path]::GetPathRoot($Path) if (-not $root) { throw 'Feature directory must be absolute' } $resolved = $root + $pathComparison = [System.StringComparison]::Ordinal + if ([System.IO.Path]::DirectorySeparatorChar -eq '\') { + $pathComparison = [System.StringComparison]::OrdinalIgnoreCase + } $separators = [regex]::Escape( [string][System.IO.Path]::DirectorySeparatorChar + [string][System.IO.Path]::AltDirectorySeparatorChar @@ -184,15 +187,20 @@ function Resolve-FeatureDirectoryPath { } elseif (Test-Path -LiteralPath $resolved -PathType Container) { # Parent enumeration also sees dangling links on Windows PowerShell 5.1. $entry = Get-ChildItem -LiteralPath $resolved -Force -ErrorAction Stop | - Where-Object { $_.Name -eq $component } | Select-Object -First 1 + Where-Object { $_.Name.Equals($component, $pathComparison) } | + Select-Object -First 1 } - if ($entry -and ($entry.Attributes -band [System.IO.FileAttributes]::ReparsePoint)) { + # Cloud placeholders can be reparse directories without redirecting names. + if ($entry -and ($entry.Attributes -band [System.IO.FileAttributes]::ReparsePoint) -and + ($entry.LinkType -in @('SymbolicLink', 'Junction') -or $entry.Target)) { + $LinkCount.Value += 1 + if ($LinkCount.Value -gt 40) { throw 'Too many symbolic links in feature directory' } $target = @($entry.Target)[0] if (-not $target) { throw "Cannot resolve reparse point: $candidate" } if (-not [System.IO.Path]::IsPathRooted($target)) { $target = Join-Path $resolved $target } - $resolved = Resolve-FeatureDirectoryPath -Path $target -LinkCount ($LinkCount + 1) + $resolved = Resolve-FeatureDirectoryPath -Path $target -LinkCount $LinkCount } elseif ($entry -and -not $entry.PSIsContainer) { throw "Feature directory component is not a directory: $candidate" } else { diff --git a/scripts/powershell/setup-plan.ps1 b/scripts/powershell/setup-plan.ps1 index 4d76f35ef3..8c84905ea0 100644 --- a/scripts/powershell/setup-plan.ps1 +++ b/scripts/powershell/setup-plan.ps1 @@ -38,8 +38,12 @@ if (-not $paths) { # Ensure the feature directory exists New-Item -ItemType Directory -Path $paths.FEATURE_DIR -Force | Out-Null +$pathComparison = [System.StringComparison]::Ordinal +if ([System.IO.Path]::DirectorySeparatorChar -eq '\') { + $pathComparison = [System.StringComparison]::OrdinalIgnoreCase +} $planEntry = Get-ChildItem -LiteralPath $paths.FEATURE_DIR -Force -ErrorAction Stop | - Where-Object { $_.Name -eq 'plan.md' } | Select-Object -First 1 + Where-Object { $_.Name.Equals('plan.md', $pathComparison) } | Select-Object -First 1 if ($planEntry -and ($planEntry.Attributes -band [System.IO.FileAttributes]::ReparsePoint)) { [Console]::Error.WriteLine("ERROR: Plan destination must not be a symbolic link.") exit 1 diff --git a/scripts/python/common.py b/scripts/python/common.py index 9655e165f0..f15822c26d 100644 --- a/scripts/python/common.py +++ b/scripts/python/common.py @@ -166,12 +166,11 @@ def get_feature_paths( if not feature_dir.is_absolute(): feature_dir = repo_root / feature_dir try: - # Non-strict resolve() tolerates link loops on newer Python versions. try: - feature_dir.stat() + feature_dir = feature_dir.resolve(strict=True) except FileNotFoundError: - pass - feature_dir = feature_dir.resolve() + # Missing feature directories are valid; other resolution errors are not. + feature_dir = feature_dir.resolve() feature_dir.relative_to(repo_root.resolve()) except (OSError, RuntimeError, ValueError) as exc: print( diff --git a/tests/test_setup_plan_python_parity.py b/tests/test_setup_plan_python_parity.py index a63ef4497f..357c962a63 100644 --- a/tests/test_setup_plan_python_parity.py +++ b/tests/test_setup_plan_python_parity.py @@ -2,10 +2,13 @@ from __future__ import annotations +import os +import stat from pathlib import Path import pytest +from scripts.python import common as python_common from tests.conftest import requires_bash from tests.parity_helpers import ( HAS_POWERSHELL, @@ -28,13 +31,11 @@ SCRIPT = "setup-plan" TEMPLATE_BODY = "# Plan Template\n\nBody.\n" -PORTS = [ - pytest.param("bash", marks=requires_bash), +POWERSHELL_PORTS = [ pytest.param( "powershell", marks=pytest.mark.skipif(not HAS_POWERSHELL, reason="no PowerShell available"), ), - pytest.param("python"), pytest.param( "windows-powershell", marks=pytest.mark.skipif( @@ -42,6 +43,7 @@ ), ), ] +PORTS = [pytest.param("bash", marks=requires_bash), *POWERSHELL_PORTS, "python"] def _command(repo: Path, port: str, json_mode: bool) -> list[str]: @@ -106,13 +108,247 @@ def test_stored_feature_cannot_escape_project( result = run(_command(repo, port, True), repo) - assert result.returncode != 0 - assert result.stdout == "" - assert "outside the project" in result.stderr + if kind == "link_parent" and os.name == "nt" and port == "python": + # Windows normalizes parent components before resolving directory links. + assert result.returncode == 0, result.stderr + contained = repo / "outside" / "new-feature" + assert Path(json_stdout(result)["FEATURE_DIR"]) == contained + assert (contained / "plan.md").read_text(encoding="utf-8") == TEMPLATE_BODY + else: + assert result.returncode != 0 + assert result.stdout == "" + assert "outside the project" in result.stderr assert not (outside / "new-feature").exists() assert (repo / ".specify" / "feature.json").read_bytes() == before +@pytest.mark.parametrize("port", [pytest.param("bash", marks=requires_bash), "python"]) +@pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="POSIX FIFO required") +def test_plan_fifo_is_rejected_without_writing(repo: Path, port: str) -> None: + plan = repo / "specs" / "001-my-feature" / "plan.md" + os.mkfifo(plan) + # Keep a reader open so the unfixed Bash writer fails the assertion, not a hang. + with os.fdopen(os.open(plan, os.O_RDONLY | os.O_NONBLOCK), "rb", buffering=0) as reader: + result = run(_command(repo, port, True), repo, timeout=15) + written = reader.read() + + assert result.returncode != 0 + assert result.stdout == "" + assert written == b"" + assert stat.S_ISFIFO(plan.stat().st_mode) + assert list(plan.parent.iterdir()) == [plan] + + +@pytest.mark.parametrize("port", PORTS) +def test_plan_directory_is_rejected_without_modification(repo: Path, port: str) -> None: + plan = repo / "specs" / "001-my-feature" / "plan.md" + plan.mkdir() + sentinel = plan / "keep.md" + sentinel.write_bytes(b"keep") + + result = run(_command(repo, port, True), repo, timeout=15) + + assert result.returncode != 0 + assert result.stdout == "" + assert list(plan.iterdir()) == [sentinel] + assert sentinel.read_bytes() == b"keep" + assert list(plan.parent.iterdir()) == [plan] + + +@requires_bash +def test_bash_plan_publication_failure_is_reported(repo: Path) -> None: + bin_dir = repo / "bin" + bin_dir.mkdir() + link = bin_dir / "link" + link.write_text("#!/bin/sh\necho 'Hard links unavailable' >&2\nexit 1\n", encoding="utf-8") + link.chmod(0o755) + env = clean_env() + env["PATH"] = str(bin_dir) + os.pathsep + env["PATH"] + + result = run(bash_cmd(repo, SCRIPT, "--json"), repo, env) + + assert result.returncode != 0 + assert result.stdout == "" + assert "Cannot create plan" in result.stderr + assert "Hard links unavailable" in result.stderr + assert list((repo / "specs" / "001-my-feature").iterdir()) == [] + + +@pytest.mark.parametrize("port", POWERSHELL_PORTS) +@pytest.mark.parametrize("count", [40, 41]) +def test_powershell_link_traversal_limit(repo: Path, port: str, count: int) -> None: + try: + (repo / "link").symlink_to(".", target_is_directory=True) + except OSError as exc: + pytest.skip(f"Cannot create directory symlink: {exc}") + write_feature_json(repo, "link/" * count + "new-feature") + + result = run(_command(repo, port, True), repo, timeout=15) + + if count == 40: + assert result.returncode == 0, result.stderr + assert (repo / "new-feature" / "plan.md").read_text(encoding="utf-8") == TEMPLATE_BODY + else: + assert result.returncode != 0 + assert result.stdout == "" + assert "outside the project" in result.stderr + assert not (repo / "new-feature").exists() + + +@pytest.mark.parametrize("port", POWERSHELL_PORTS) +@pytest.mark.parametrize("depth", [4, 5], ids=["within-budget", "over-budget"]) +def test_powershell_counts_all_link_traversals( + repo: Path, port: str, depth: int +) -> None: + try: + (repo / "l0").symlink_to(".", target_is_directory=True) + for index in range(1, depth + 1): + previous = f"l{index - 1}" + (repo / f"l{index}").symlink_to( + f"{previous}/{previous}", target_is_directory=True + ) + except OSError as exc: + pytest.skip(f"Cannot create directory symlink: {exc}") + write_feature_json(repo, f"l{depth}/new-feature") + + result = run(_command(repo, port, True), repo, timeout=15) + + if depth == 4: + assert result.returncode == 0, result.stderr + assert (repo / "new-feature" / "plan.md").read_text(encoding="utf-8") == TEMPLATE_BODY + else: + assert result.returncode != 0 + assert result.stdout == "" + assert "outside the project" in result.stderr + assert not (repo / "new-feature").exists() + + +@pytest.mark.parametrize("port", POWERSHELL_PORTS) +@pytest.mark.parametrize( + ("link_type", "target"), + [ + ("", ""), + ("SymbolicLink", ""), + ("Junction", ""), + ("SymbolicLink", "outside"), + ("Junction", "outside"), + ], + ids=["cloud-placeholder", "unresolved-symlink", "unresolved-junction", + "external-symlink", "external-junction"], +) +def test_powershell_reparse_directory_types( + repo: Path, port: str, link_type: str, target: str +) -> None: + feature = repo / "placeholder" / "new-feature" + feature.parent.mkdir() + write_feature_json(repo, "placeholder/new-feature") + env = clean_env() + env["SPECIFY_INIT_DIR"] = str(repo) + env["TEST_REPARSE_ENTRY"] = str(feature.parent) + env["TEST_LINK_TYPE"] = link_type + env["TEST_LINK_TARGET"] = str(repo.parent / target) if target else "" + program = r""" +function Get-Item { + [CmdletBinding()] + param([string]$LiteralPath, [switch]$Force) + if ($LiteralPath -ceq $env:TEST_REPARSE_ENTRY) { + return [PSCustomObject]@{ + Name = 'placeholder' + Attributes = [System.IO.FileAttributes]::Directory -bor + [System.IO.FileAttributes]::ReparsePoint + PSIsContainer = $true + LinkType = $env:TEST_LINK_TYPE + Target = $env:TEST_LINK_TARGET + } + } + Microsoft.PowerShell.Management\Get-Item @PSBoundParameters +} +& (Join-Path $env:SPECIFY_INIT_DIR '.specify/scripts/powershell/setup-plan.ps1') -Json +""" + + result = run(_command(repo, port, False)[:2] + ["-Command", program], repo, env) + + if not link_type: + assert result.returncode == 0, result.stderr + assert (feature / "plan.md").read_text(encoding="utf-8") == TEMPLATE_BODY + else: + assert result.returncode != 0 + assert result.stdout == "" + assert "outside the project" in result.stderr + assert not feature.exists() + + +@pytest.mark.parametrize("port", POWERSHELL_PORTS) +@pytest.mark.skipif(os.name == "nt", reason="Non-Windows case-sensitive name matching") +def test_powershell_missing_directory_does_not_match_case_sibling( + repo: Path, port: str +) -> None: + env = clean_env() + env["SPECIFY_INIT_DIR"] = str(repo) + env["TEST_MISSING_DIR"] = str(repo / "features") + env["TEST_OUTSIDE"] = str(repo.parent / "outside") + program = r""" +. (Join-Path $env:SPECIFY_INIT_DIR '.specify/scripts/powershell/common.ps1') +function Test-Path { + [CmdletBinding()] + param([string]$LiteralPath, [string]$PathType) + if ($LiteralPath -ceq $env:TEST_MISSING_DIR) { return $false } + Microsoft.PowerShell.Management\Test-Path @PSBoundParameters +} +function Get-ChildItem { + [CmdletBinding()] + param([string]$LiteralPath, [switch]$Force) + Microsoft.PowerShell.Management\Get-ChildItem @PSBoundParameters + if ($LiteralPath -ceq $env:SPECIFY_INIT_DIR) { + [PSCustomObject]@{ + Name = 'Features' + Attributes = [System.IO.FileAttributes]::Directory -bor + [System.IO.FileAttributes]::ReparsePoint + PSIsContainer = $true + LinkType = 'SymbolicLink' + Target = $env:TEST_OUTSIDE + } + } +} +Resolve-FeatureDirectoryPath -Path (Join-Path $env:TEST_MISSING_DIR 'new-feature') +""" + + result = run(_command(repo, port, False)[:2] + ["-Command", program], repo, env) + + assert result.returncode == 0, result.stderr + assert Path(result.stdout.strip()) == repo / "features" / "new-feature" + + +@pytest.mark.parametrize("port", POWERSHELL_PORTS) +@pytest.mark.skipif(os.name == "nt", reason="Non-Windows case-sensitive name matching") +def test_powershell_plan_does_not_match_case_sibling(repo: Path, port: str) -> None: + feature = repo / "specs" / "001-my-feature" + env = clean_env() + env["SPECIFY_INIT_DIR"] = str(repo) + env["TEST_FEATURE_DIR"] = str(feature) + program = r""" +function Get-ChildItem { + [CmdletBinding()] + param([string]$LiteralPath, [switch]$Force) + Microsoft.PowerShell.Management\Get-ChildItem @PSBoundParameters + if ($LiteralPath -ceq $env:TEST_FEATURE_DIR) { + [PSCustomObject]@{ + Name = 'PLAN.MD' + Attributes = [System.IO.FileAttributes]::ReparsePoint + LinkType = 'SymbolicLink' + Target = 'unrelated' + } + } +} +& (Join-Path $env:SPECIFY_INIT_DIR '.specify/scripts/powershell/setup-plan.ps1') -Json +""" + + result = run(_command(repo, port, False)[:2] + ["-Command", program], repo, env) + + assert result.returncode == 0, result.stderr + assert (feature / "plan.md").read_text(encoding="utf-8") == TEMPLATE_BODY + + @pytest.mark.parametrize("port", PORTS) def test_stored_feature_link_loop_is_rejected(repo: Path, port: str) -> None: try: @@ -128,6 +364,29 @@ def test_stored_feature_link_loop_is_rejected(repo: Path, port: str) -> None: assert "outside the project" in result.stderr +def test_python_strict_path_resolution_failure_is_reported( + repo: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + feature = repo / "specs" / "001-my-feature" + original_resolve = Path.resolve + + def resolve(path: Path, strict: bool = False) -> Path: + if path == feature: + if strict: + raise OSError(1921, "The name cannot be resolved by the system") + return path + return original_resolve(path, strict=strict) + + monkeypatch.setenv("SPECIFY_INIT_DIR", str(repo)) + monkeypatch.delenv("SPECIFY_FEATURE_DIRECTORY", raising=False) + monkeypatch.setattr(Path, "resolve", resolve) + + with pytest.raises(SystemExit, match="1"): + python_common.get_feature_paths() + + assert "outside the project" in capsys.readouterr().err + + @pytest.mark.parametrize("port", PORTS) @pytest.mark.parametrize("absolute", [False, True]) def test_external_feature_requires_override_on_every_invocation( From 29e96d23de9f04445c109467d883e1cd17a1ac9e Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 30 Oct 2026 07:11:54 -0500 Subject: [PATCH 3/4] fix: preserve filesystem path semantics in feature setup Resolve both PowerShell separator styles before containment checks. Inspect literal plan destinations so filesystem aliases and cloud placeholders are classified correctly, retaining case-sensitive fallback matching. Preserve trailing newlines through Bash feature metadata and symlink resolution, with cross-port regressions. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- scripts/bash/common.sh | 33 +++-- scripts/powershell/common.ps1 | 17 +-- scripts/powershell/setup-plan.ps1 | 11 +- tests/test_setup_plan_python_parity.py | 161 +++++++++++++++++++++++++ 4 files changed, 203 insertions(+), 19 deletions(-) diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index 006c91a93d..e2aac266de 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -104,16 +104,21 @@ read_feature_json_feature_directory() { # an availability-gated `elif` would pick python3, swallow its failure, and # never reach the grep/sed fallback -- leaving feature.json unreadable even # though it is valid (issue #3304). + # A success marker preserves trailing newlines in parsed path values. local _fd='' if command -v jq >/dev/null 2>&1; then - if ! _fd=$(jq -r '.feature_directory // empty' "$fj" 2>/dev/null); then + if ! _fd=$(jq -jr '.feature_directory // empty' "$fj" 2>/dev/null && printf '.'); then _fd='' + else + _fd="${_fd%.}" fi fi if [[ -z "$_fd" ]] && command -v python3 >/dev/null 2>&1; then # Use Python so pretty-printed/multi-line JSON still parses correctly. - if ! _fd=$(python3 -c "import json,sys; d=json.load(open(sys.argv[1], encoding='utf-8')); v=d.get('feature_directory'); print(v if v else '')" "$fj" 2>/dev/null); then + if ! _fd=$(python3 -c "import json,sys; d=json.load(open(sys.argv[1], encoding='utf-8')); v=d.get('feature_directory'); print(v if v else '', end='')" "$fj" 2>/dev/null && printf '.'); then _fd='' + else + _fd="${_fd%.}" fi fi if [[ -z "$_fd" ]]; then @@ -144,7 +149,8 @@ _persist_feature_json() { # Read current value (if any) and skip write when unchanged local current_val - current_val=$(read_feature_json_feature_directory "$repo_root") + current_val=$(read_feature_json_feature_directory "$repo_root" && printf '.') + current_val="${current_val%.}" if [[ "$current_val" == "$feature_dir_value" ]]; then return 0 fi @@ -172,13 +178,19 @@ _resolve_feature_directory() { fi case "$component" in ""|.) continue ;; - ..) resolved="$(dirname -- "$resolved")"; continue ;; + ..) + resolved="${resolved%/*}" + resolved="${resolved:-/}" + continue + ;; esac target="${resolved%/}/$component" if [[ -L "$target" ]]; then links=$((links + 1)) [[ "$links" -le 40 ]] || return 1 - target="$(readlink -- "$target")" || return 1 + # The marker retains target newlines across command substitution. + target="$(readlink -n -- "$target" && printf '.')" || return 1 + target="${target%.}" remaining="$target${remaining:+/$remaining}" [[ "$target" != /* ]] || resolved="/" elif [[ -e "$target" && ! -d "$target" ]]; then @@ -187,7 +199,7 @@ _resolve_feature_directory() { resolved="$target" fi done - printf '%s\n' "$resolved" + printf '%s' "$resolved" } get_feature_paths() { @@ -230,18 +242,21 @@ get_feature_paths() { fi elif [[ -f "$repo_root/.specify/feature.json" ]]; then local _fd - _fd=$(read_feature_json_feature_directory "$repo_root") + _fd=$(read_feature_json_feature_directory "$repo_root" && printf '.') + _fd="${_fd%.}" if [[ -n "$_fd" ]]; then feature_dir="$_fd" # Normalize relative paths to absolute under repo root [[ "$feature_dir" != /* ]] && feature_dir="$repo_root/$feature_dir" local physical_root physical_root=$(CDPATH="" cd -- "$repo_root" && pwd -P) || return 1 - if ! feature_dir=$(_resolve_feature_directory "$feature_dir") || - [[ "$feature_dir" != "$physical_root" && "$feature_dir" != "$physical_root/"* ]]; then + local resolved_feature_dir + if ! resolved_feature_dir=$(_resolve_feature_directory "$feature_dir" && printf '.') || + [[ "${resolved_feature_dir%.}" != "$physical_root" && "${resolved_feature_dir%.}" != "$physical_root/"* ]]; then echo "ERROR: Stored feature directory is invalid or outside the project. Set SPECIFY_FEATURE_DIRECTORY explicitly to use an external directory." >&2 return 1 fi + feature_dir="${resolved_feature_dir%.}" else echo "ERROR: Feature directory not found. Set SPECIFY_FEATURE_DIRECTORY or ensure .specify/feature.json contains feature_directory." >&2 return 1 diff --git a/scripts/powershell/common.ps1 b/scripts/powershell/common.ps1 index ceb3d51dfe..f71d8aa63e 100644 --- a/scripts/powershell/common.ps1 +++ b/scripts/powershell/common.ps1 @@ -158,6 +158,13 @@ function Save-FeatureJson { [System.IO.File]::WriteAllText($fjPath, $json, $utf8NoBom) } +function Test-NameRedirectingLink { + param([object]$Entry) + + return ($Entry -and ($Entry.Attributes -band [System.IO.FileAttributes]::ReparsePoint) -and + ($Entry.LinkType -in @('SymbolicLink', 'Junction') -or $Entry.Target)) +} + function Resolve-FeatureDirectoryPath { param([string]$Path, [ref]$LinkCount = ([ref]0)) @@ -168,11 +175,8 @@ function Resolve-FeatureDirectoryPath { if ([System.IO.Path]::DirectorySeparatorChar -eq '\') { $pathComparison = [System.StringComparison]::OrdinalIgnoreCase } - $separators = [regex]::Escape( - [string][System.IO.Path]::DirectorySeparatorChar + - [string][System.IO.Path]::AltDirectorySeparatorChar - ) - foreach ($component in ($Path.Substring($root.Length) -split "[$separators]")) { + # The PowerShell FileSystem provider recognizes both separators on every OS. + foreach ($component in ($Path.Substring($root.Length) -split '[\\/]')) { if (-not $component -or $component -eq '.') { continue } if ($component -eq '..') { $parent = [System.IO.Path]::GetDirectoryName($resolved.TrimEnd('/', '\')) @@ -191,8 +195,7 @@ function Resolve-FeatureDirectoryPath { Select-Object -First 1 } # Cloud placeholders can be reparse directories without redirecting names. - if ($entry -and ($entry.Attributes -band [System.IO.FileAttributes]::ReparsePoint) -and - ($entry.LinkType -in @('SymbolicLink', 'Junction') -or $entry.Target)) { + if (Test-NameRedirectingLink -Entry $entry) { $LinkCount.Value += 1 if ($LinkCount.Value -gt 40) { throw 'Too many symbolic links in feature directory' } $target = @($entry.Target)[0] diff --git a/scripts/powershell/setup-plan.ps1 b/scripts/powershell/setup-plan.ps1 index 8c84905ea0..ccae608ce0 100644 --- a/scripts/powershell/setup-plan.ps1 +++ b/scripts/powershell/setup-plan.ps1 @@ -42,9 +42,14 @@ $pathComparison = [System.StringComparison]::Ordinal if ([System.IO.Path]::DirectorySeparatorChar -eq '\') { $pathComparison = [System.StringComparison]::OrdinalIgnoreCase } -$planEntry = Get-ChildItem -LiteralPath $paths.FEATURE_DIR -Force -ErrorAction Stop | - Where-Object { $_.Name.Equals('plan.md', $pathComparison) } | Select-Object -First 1 -if ($planEntry -and ($planEntry.Attributes -band [System.IO.FileAttributes]::ReparsePoint)) { +try { + $planEntry = Get-Item -LiteralPath $paths.IMPL_PLAN -Force -ErrorAction Stop +} catch [System.Management.Automation.ItemNotFoundException] { + # Windows PowerShell 5.1 may only expose dangling links through their parent. + $planEntry = Get-ChildItem -LiteralPath $paths.FEATURE_DIR -Force -ErrorAction Stop | + Where-Object { $_.Name.Equals('plan.md', $pathComparison) } | Select-Object -First 1 +} +if (Test-NameRedirectingLink -Entry $planEntry) { [Console]::Error.WriteLine("ERROR: Plan destination must not be a symbolic link.") exit 1 } diff --git a/tests/test_setup_plan_python_parity.py b/tests/test_setup_plan_python_parity.py index 357c962a63..568a37cdf6 100644 --- a/tests/test_setup_plan_python_parity.py +++ b/tests/test_setup_plan_python_parity.py @@ -22,6 +22,7 @@ install_scripts, json_stdout, make_repo, + make_python3_path_shim, normalize_repo_paths, ps_cmd, py_cmd, @@ -122,6 +123,166 @@ def test_stored_feature_cannot_escape_project( assert (repo / ".specify" / "feature.json").read_bytes() == before +@pytest.mark.parametrize("port", POWERSHELL_PORTS) +@pytest.mark.parametrize("absolute", [False, True]) +@pytest.mark.parametrize("escape", [False, True], ids=["contained", "outside"]) +def test_powershell_mixed_separators_are_resolved_before_containment( + repo: Path, port: str, absolute: bool, escape: bool +) -> None: + value = r"..\outside/new-feature" if escape else r"specs\new-feature" + if absolute: + value = str(repo) + "/" + value + write_feature_json(repo, value) + before = (repo / ".specify" / "feature.json").read_bytes() + + result = run(_command(repo, port, True), repo) + + if escape: + assert result.returncode != 0 + assert result.stdout == "" + assert "outside the project" in result.stderr + assert not (repo.parent / "outside" / "new-feature").exists() + else: + feature = repo / "specs" / "new-feature" + assert result.returncode == 0, result.stderr + assert Path(json_stdout(result)["FEATURE_DIR"]) == feature + assert (feature / "plan.md").read_text(encoding="utf-8") == TEMPLATE_BODY + assert (repo / ".specify" / "feature.json").read_bytes() == before + + +@requires_bash +@pytest.mark.skipif(os.name == "nt", reason="POSIX filenames containing newlines") +@pytest.mark.parametrize("parser", ["default", "python"]) +@pytest.mark.parametrize( + "kind", ["link-target", "parent-component", "terminal-link", "stored-path", "outside-link"] +) +def test_bash_feature_paths_preserve_trailing_newlines( + repo: Path, kind: str, parser: str +) -> None: + directory = (repo.parent if kind == "outside-link" else repo) / "feature\n\n" + directory.mkdir() + expected = directory / "new-feature" + if kind == "parent-component": + (directory / "child").mkdir() + value = "feature\n\n/child/../new-feature" + elif kind == "stored-path": + value = "feature\n\n" + expected = directory + else: + target = "../feature\n\n" if kind == "outside-link" else "feature\n\n" + (repo / "linked").symlink_to(target, target_is_directory=True) + value = "linked/new-feature" + if kind == "terminal-link": + value = "linked" + expected = directory + write_feature_json(repo, value) + env = clean_env() + if parser == "python": + bin_dir = make_python3_path_shim(repo / "bin") + jq = bin_dir / "jq" + jq.write_text("#!/bin/sh\nexit 1\n", encoding="utf-8") + jq.chmod(0o755) + env["PATH"] = str(bin_dir) + os.pathsep + env["PATH"] + + result = run(_command(repo, "bash", parser == "default"), repo, env) + + if kind == "outside-link": + assert result.returncode != 0 + assert result.stdout == "" + assert "outside the project" in result.stderr + assert not (directory / "new-feature").exists() + else: + assert result.returncode == 0, result.stderr + if parser == "default": + assert Path(json_stdout(result)["FEATURE_DIR"]) == expected + else: + assert f"FEATURE_DIR: {expected}\n" in result.stdout + assert (expected / "plan.md").read_text(encoding="utf-8") == TEMPLATE_BODY + assert not (repo / "feature").exists() + if kind == "stored-path": + feature_json = repo / ".specify" / "feature.json" + before = feature_json.stat().st_mtime_ns + env["SPECIFY_FEATURE_DIRECTORY"] = value + retry = run(_command(repo, "bash", parser == "default"), repo, env) + assert retry.returncode == 0, retry.stderr + assert feature_json.stat().st_mtime_ns == before + + +@pytest.mark.parametrize("port", POWERSHELL_PORTS) +@pytest.mark.parametrize("existing", [False, True]) +def test_powershell_plan_alias_uses_filesystem_casing( + repo: Path, port: str, existing: bool +) -> None: + feature = repo / "specs" / "001-my-feature" + target = repo.parent / "target.md" + if existing: + target.write_bytes(b"keep") + alias = feature / "PLAN.MD" + try: + alias.symlink_to(target) + except OSError as exc: + pytest.skip(f"Cannot create file symlink: {exc}") + plan = feature / "plan.md" + same_entry = plan.is_symlink() + + result = run(_command(repo, port, True), repo) + + if same_entry: + assert result.returncode != 0 + assert result.stdout == "" + assert "symbolic link" in result.stderr + else: + assert result.returncode == 0, result.stderr + assert plan.read_text(encoding="utf-8") == TEMPLATE_BODY + assert alias.is_symlink() + assert target.exists() == existing + if existing: + assert target.read_bytes() == b"keep" + + +@pytest.mark.parametrize("port", POWERSHELL_PORTS) +def test_powershell_preserves_cloud_backed_plan(repo: Path, port: str) -> None: + feature = repo / "specs" / "001-my-feature" + plan = feature / "plan.md" + plan.write_bytes(b"existing cloud plan") + before = plan.stat().st_mtime_ns + env = clean_env() + env["SPECIFY_INIT_DIR"] = str(repo) + env["TEST_FEATURE_DIR"] = str(feature) + env["TEST_PLAN"] = str(plan) + program = r""" +function Get-CloudPlan { + [PSCustomObject]@{ + Name = 'plan.md' + Attributes = [System.IO.FileAttributes]::ReparsePoint + PSIsContainer = $false + LinkType = $null + Target = $null + } +} +function Get-Item { + [CmdletBinding()] + param([string]$LiteralPath, [switch]$Force) + if ($LiteralPath -ceq $env:TEST_PLAN) { return (Get-CloudPlan) } + Microsoft.PowerShell.Management\Get-Item @PSBoundParameters +} +function Get-ChildItem { + [CmdletBinding()] + param([string]$LiteralPath, [switch]$Force) + if ($LiteralPath -ceq $env:TEST_FEATURE_DIR) { return (Get-CloudPlan) } + Microsoft.PowerShell.Management\Get-ChildItem @PSBoundParameters +} +& (Join-Path $env:SPECIFY_INIT_DIR '.specify/scripts/powershell/setup-plan.ps1') -Json +""" + + result = run(_command(repo, port, False)[:2] + ["-Command", program], repo, env) + + assert result.returncode == 0, result.stderr + assert "already exists" in result.stderr + assert plan.read_bytes() == b"existing cloud plan" + assert plan.stat().st_mtime_ns == before + + @pytest.mark.parametrize("port", [pytest.param("bash", marks=requires_bash), "python"]) @pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="POSIX FIFO required") def test_plan_fifo_is_rejected_without_writing(repo: Path, port: str) -> None: From 9539ffd1c061ae57df882278ef40ff0b7d04819c Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 30 Oct 2026 10:16:55 -0500 Subject: [PATCH 4/4] fix: align external-feature guidance and root containment Require an explicit external feature override on each downstream invocation, including paths through symlinks. Handle slash-rooted Bash projects without constructing a double-slash containment prefix, with safe root-boundary and cross-port invocation regressions. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- scripts/bash/common.sh | 2 +- templates/commands/specify.md | 3 +- tests/test_setup_plan_python_parity.py | 69 ++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index e2aac266de..b26d2e4ed2 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -252,7 +252,7 @@ get_feature_paths() { physical_root=$(CDPATH="" cd -- "$repo_root" && pwd -P) || return 1 local resolved_feature_dir if ! resolved_feature_dir=$(_resolve_feature_directory "$feature_dir" && printf '.') || - [[ "${resolved_feature_dir%.}" != "$physical_root" && "${resolved_feature_dir%.}" != "$physical_root/"* ]]; then + [[ "${resolved_feature_dir%.}" != "$physical_root" && "${resolved_feature_dir%.}" != "${physical_root%/}/"* ]]; then echo "ERROR: Stored feature directory is invalid or outside the project. Set SPECIFY_FEATURE_DIRECTORY explicitly to use an external directory." >&2 return 1 fi diff --git a/templates/commands/specify.md b/templates/commands/specify.md index 52b2b24565..1ca2add202 100644 --- a/templates/commands/specify.md +++ b/templates/commands/specify.md @@ -103,7 +103,8 @@ Given that feature description, do this: } ``` Write the actual resolved directory path value (for example, `specs/003-user-auth`), not the literal string `SPECIFY_FEATURE_DIRECTORY`. - This allows downstream commands (`__SPECKIT_COMMAND_PLAN__`, `__SPECKIT_COMMAND_TASKS__`, etc.) to locate the feature directory without relying on git branch name conventions. + For feature directories that resolve inside the project, this allows downstream commands (`__SPECKIT_COMMAND_PLAN__`, `__SPECKIT_COMMAND_TASKS__`, etc.) to locate the feature directory without relying on git branch name conventions. + For feature directories that resolve outside the project, including paths through symlinks, you MUST explicitly supply `SPECIFY_FEATURE_DIRECTORY` in the environment of every downstream script or command invocation. The persisted value alone is not sufficient. **IMPORTANT**: - You must only create one feature per `__SPECKIT_COMMAND_SPECIFY__` invocation diff --git a/tests/test_setup_plan_python_parity.py b/tests/test_setup_plan_python_parity.py index 568a37cdf6..e94e55b339 100644 --- a/tests/test_setup_plan_python_parity.py +++ b/tests/test_setup_plan_python_parity.py @@ -78,6 +78,46 @@ def repo(tmp_path: Path) -> Path: return _setup_repo(tmp_path) +@requires_bash +@pytest.mark.parametrize("case", ["filesystem-root", "project-root", "outside-project"]) +def test_bash_feature_containment_handles_filesystem_root( + repo: Path, case: str +) -> None: + feature = ( + repo.parent / "outside" / "new-feature" + if case == "outside-project" else repo / "specs" / "001-my-feature" + ) + write_feature_json(repo, _bash_posix_path(feature)) + env = clean_env() + env["TEST_REPO"] = _bash_posix_path(repo) + env["TEST_COMMON"] = _bash_posix_path( + repo / ".specify" / "scripts" / "bash" / "common.sh" + ) + env["TEST_PHYSICAL_ROOT"] = ( + "/" if case == "filesystem-root" else _bash_posix_path(repo.resolve()) + ) + program = r""" +source "$TEST_COMMON" +get_repo_root() { printf '%s\n' "$TEST_REPO"; } +# Simulate a root-mounted project without creating /.specify or writing at /. +pwd() { printf '%s\n' "$TEST_PHYSICAL_ROOT"; } +paths=$(get_feature_paths --no-persist) || exit 1 +eval "$paths" +printf '%s' "$FEATURE_DIR" +""" + + result = run(["bash", "-c", program], repo, env) + + if case == "outside-project": + assert result.returncode != 0 + assert result.stdout == "" + assert "outside the project" in result.stderr + assert not feature.exists() + else: + assert result.returncode == 0, result.stderr + assert result.stdout == _bash_posix_path(feature.resolve()) + + @pytest.mark.parametrize("port", PORTS) @pytest.mark.parametrize( "kind", @@ -567,6 +607,35 @@ def test_external_feature_requires_override_on_every_invocation( assert "outside the project" in retry.stderr +@pytest.mark.parametrize("port", PORTS) +def test_external_feature_symlink_requires_override_on_every_invocation( + repo: Path, port: str +) -> None: + outside = repo.parent / "outside" + outside.mkdir() + try: + (repo / "linked-external").symlink_to(outside, target_is_directory=True) + except OSError as exc: + pytest.skip(f"Cannot create directory symlink: {exc}") + env = clean_env() + env["SPECIFY_FEATURE_DIRECTORY"] = "linked-external/new-feature" + + result = run(_command(repo, port, True), repo, env) + + assert result.returncode == 0, result.stderr + plan = outside / "new-feature" / "plan.md" + assert plan.read_text(encoding="utf-8") == TEMPLATE_BODY + before = plan.stat().st_mtime_ns + retry = run(_command(repo, port, True), repo) + assert retry.returncode != 0 + assert retry.stdout == "" + assert "outside the project" in retry.stderr + assert plan.stat().st_mtime_ns == before + restored = run(_command(repo, port, True), repo, env) + assert restored.returncode == 0, restored.stderr + assert plan.stat().st_mtime_ns == before + + @pytest.mark.parametrize("port", PORTS) @pytest.mark.parametrize("existing", [False, True]) @pytest.mark.parametrize("override", [False, True])