From eab05105324301642aab8111d8f8988751ea8c63 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:31:09 -0500 Subject: [PATCH 1/5] fix: isolate self-upgrade installer lookup from project paths Resolve uv and pipx through absolute PATH directories outside the working directory and Git worktree for registry probes and self-upgrades. Preserve Windows PATHEXT handling and eligible PATH ordering, with regression coverage for unsafe search paths and normal installations. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/_version.py | 44 ++- tests/specify_cli/test_version_detection.py | 329 ++++++++++++++++++++ tests/specify_cli/test_version_execution.py | 2 +- 3 files changed, 370 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/_version.py b/src/specify_cli/_version.py index c8fcad7c7e..06e17ef357 100644 --- a/src/specify_cli/_version.py +++ b/src/specify_cli/_version.py @@ -481,6 +481,42 @@ def _editable_marker_seen() -> bool: return editable_root is not None and _git_ancestor(editable_root) is not None +def _find_installer(name: str) -> str | None: + """Find an installer outside the working directory and current Git tree.""" + cwd = Path.cwd().resolve() + project_root = _git_ancestor(cwd) + suffixes = [""] + if sys.platform == "win32": + # Python 3.11's which() does not expand PATHEXT for absolute commands. + suffixes = [ + suffix + for suffix in (os.environ.get("PATHEXT") or ".COM;.EXE;.BAT;.CMD").split( + ";" + ) + if suffix + ] + + for entry in os.get_exec_path(): + if sys.platform == "win32": + entry = entry.strip('"') + directory = Path(entry) + if not directory.is_absolute(): + continue + resolved_directory = directory.resolve() + if directory == cwd or resolved_directory == cwd: + continue + if project_root is not None and ( + directory.is_relative_to(project_root) + or resolved_directory.is_relative_to(project_root) + ): + continue + for suffix in suffixes: + found = shutil.which(str(directory / f"{name}{suffix}")) + if found is not None: + return found + return None + + def _detect_install_method( argv0: str | None = None, include_signals: bool = False, @@ -534,7 +570,7 @@ def _detect_install_method( consulted: list[str] = [] if _tier3_registry_lookup_allowed(argv0_path): uv_tool_match = False - uv_bin = shutil.which("uv") + uv_bin = _find_installer("uv") if uv_bin is not None: consulted.append("uv tool list") try: @@ -554,7 +590,7 @@ def _detect_install_method( pass pipx_match = False - pipx_bin = shutil.which("pipx") + pipx_bin = _find_installer("pipx") if pipx_bin is not None: consulted.append("pipx list --json") try: @@ -635,7 +671,7 @@ def _assemble_installer_argv( source_spec = _source_spec(target_tag) if method == _InstallMethod.UV_TOOL: - uv_bin = shutil.which("uv") + uv_bin = _find_installer("uv") if uv_bin is None: return None return [ @@ -651,7 +687,7 @@ def _assemble_installer_argv( if method == _InstallMethod.PIPX: # pipx 1.5+ removed `--spec`; PACKAGE_SPEC is now positional and the # package name is auto-detected from the source's pyproject.toml. - pipx_bin = shutil.which("pipx") + pipx_bin = _find_installer("pipx") if pipx_bin is None: return None return [ diff --git a/tests/specify_cli/test_version_detection.py b/tests/specify_cli/test_version_detection.py index c769665920..1901ee53d4 100644 --- a/tests/specify_cli/test_version_detection.py +++ b/tests/specify_cli/test_version_detection.py @@ -90,6 +90,7 @@ def test_tier3_uv_tool_when_registry_lists_exact_name( monkeypatch.setattr("sys.argv", [str(tmp_path / "missing" / "specify")]) def fake_which(name): + name = Path(name).stem if Path(name).is_absolute() else name return "uv" if name == "uv" else None def fake_run(argv, *args, **kwargs): @@ -116,6 +117,7 @@ def test_unresolved_bare_argv0_skips_tier3_registry_detection(self, monkeypatch) monkeypatch.setattr("sys.argv", ["specify"]) def fake_which(name): + name = Path(name).stem if Path(name).is_absolute() else name return "uv" if name == "uv" else None def fake_run(argv, *args, **kwargs): @@ -140,6 +142,7 @@ def test_bare_argv0_missing_path_resolution_allows_tier3_registry_detection( monkeypatch.setattr("sys.argv", ["specify"]) def fake_which(name): + name = Path(name).stem if Path(name).is_absolute() else name if name == "specify": return str(missing_specify) if name == "uv": @@ -196,6 +199,7 @@ def test_tier3_uv_tool_ignores_substring_false_positive( unsupported_argv0, ): def fake_which(name): + name = Path(name).stem if Path(name).is_absolute() else name return "uv" if name == "uv" else None def fake_run(argv, *args, **kwargs): @@ -221,6 +225,7 @@ def test_tier3_uv_tool_does_not_override_absolute_unsupported_entrypoint( unsupported_argv0, ): def fake_which(name): + name = Path(name).stem if Path(name).is_absolute() else name return "uv" if name == "uv" else None def fake_run(argv, *args, **kwargs): @@ -254,6 +259,7 @@ def test_tier3_uv_tool_does_not_override_resolved_bare_unsupported_entrypoint( monkeypatch.setattr("sys.argv", ["specify"]) def fake_which(name): + name = Path(name).stem if Path(name).is_absolute() else name if name == "specify": return str(fake_specify) if name == "uv": @@ -352,6 +358,325 @@ def test_missing_uv_returns_no_installer_argv(self): assert _assemble_installer_argv(_InstallMethod.UV_TOOL, "v0.7.6") is None +class TestInstallerLookupSecurity: + @staticmethod + def executable(directory, name): + executable = directory / (f"{name}.exe" if os.name == "nt" else name) + executable.write_bytes(b"installer lookup fixture; never executed\n") + executable.chmod(0o755) + return executable + + @pytest.mark.parametrize( + "method,name", + [ + (_InstallMethod.UV_TOOL, "uv"), + (_InstallMethod.PIPX, "pipx"), + ], + ) + @pytest.mark.parametrize( + "entry", ["", ".", "bin", "absolute", "symlink", "symlink-out"] + ) + def test_project_path_entries_cannot_override_installer( + self, monkeypatch, tmp_path, method, name, entry + ): + project = tmp_path / "project" + project.mkdir() + (project / ".git").mkdir() + project_bin = project / "bin" + project_bin.mkdir() + trusted = tmp_path / "trusted" + trusted.mkdir() + self.executable(project, name) + self.executable(project_bin, name) + expected = self.executable(trusted, name) + if entry == "absolute": + entry = str(project_bin) + elif entry in {"symlink", "symlink-out"}: + link = tmp_path / "project-bin-link" + target = project_bin + if entry == "symlink-out": + link = project / "external-bin" + target = tmp_path / "untrusted" + target.mkdir() + self.executable(target, name) + try: + link.symlink_to(target, target_is_directory=True) + except OSError: + pytest.skip("directory symlinks are unavailable") + entry = str(link) + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", os.pathsep.join([entry, str(trusted)])) + + argv = _assemble_installer_argv(method, "v0.7.6") + + assert argv is not None + assert Path(argv[0]) == expected + + @pytest.mark.parametrize( + "method,name", + [ + (_InstallMethod.UV_TOOL, "uv"), + (_InstallMethod.PIPX, "pipx"), + ], + ) + def test_implicit_windows_cwd_search_is_bypassed( + self, monkeypatch, tmp_path, method, name + ): + project = tmp_path / "project" + project.mkdir() + trusted = tmp_path / "trusted" + trusted.mkdir() + malicious = self.executable(project, name) + expected = self.executable(trusted, name) + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", str(trusted)) + real_which = specify_cli._version.shutil.which + + def windows_style_which(command): + if not Path(command).is_absolute(): + return str(malicious) + return real_which(command) + + with patch( + "specify_cli._version.shutil.which", side_effect=windows_style_which + ): + argv = _assemble_installer_argv(method, "v0.7.6") + + assert argv is not None + assert Path(argv[0]) == expected + + @pytest.mark.parametrize( + "method,name", + [ + (_InstallMethod.UV_TOOL, "uv"), + (_InstallMethod.PIPX, "pipx"), + ], + ) + @pytest.mark.parametrize("path", ["", ".", "bin"]) + def test_only_project_local_installer_is_not_available( + self, monkeypatch, tmp_path, method, name, path + ): + (tmp_path / "bin").mkdir() + self.executable(tmp_path, name) + self.executable(tmp_path / "bin", name) + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("PATH", path) + + assert _assemble_installer_argv(method, "v0.7.6") is None + + def test_registry_probes_do_not_execute_project_local_installers( + self, monkeypatch, tmp_path + ): + self.executable(tmp_path, "uv") + self.executable(tmp_path, "pipx") + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("PATH", str(tmp_path)) + monkeypatch.setattr("sys.argv", [str(tmp_path / "missing" / "specify")]) + + with ( + patch("specify_cli._version._editable_marker_seen", return_value=False), + patch("specify_cli._version.subprocess.run") as run, + ): + method, signals = _detect_install_method(include_signals=True) + + assert method == _InstallMethod.UNSUPPORTED + assert signals.installer_registries_consulted == () + run.assert_not_called() + + @pytest.mark.parametrize( + "method,name", + [ + (_InstallMethod.UV_TOOL, "uv"), + (_InstallMethod.PIPX, "pipx"), + ], + ) + def test_missing_safe_installer_fails_without_execution( + self, monkeypatch, tmp_path, method, name + ): + self.executable(tmp_path, name) + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("PATH", str(tmp_path)) + with ( + patch("specify_cli._version._detect_install_method", return_value=method), + patch("specify_cli._version._get_installed_version", return_value="0.7.5"), + patch("specify_cli._version.subprocess.run") as run, + ): + result = runner.invoke(app, ["self", "upgrade", "--tag", "v0.7.6"]) + + assert result.exit_code == 3 + assert f"Installer {name} not found on PATH" in strip_ansi(result.output) + run.assert_not_called() + + @pytest.mark.parametrize( + "method,name", + [ + (_InstallMethod.UV_TOOL, "uv"), + (_InstallMethod.PIPX, "pipx"), + ], + ) + def test_absolute_path_order_is_preserved( + self, monkeypatch, tmp_path, method, name + ): + project = tmp_path / "project" + project.mkdir() + first = tmp_path / "first" + first.mkdir() + second = tmp_path / "second" + second.mkdir() + expected = self.executable(first, name) + self.executable(second, name) + monkeypatch.chdir(project) + monkeypatch.setenv( + "PATH", + os.pathsep.join([str(tmp_path / "missing"), str(first), str(second)]), + ) + + argv = _assemble_installer_argv(method, "v0.7.6") + + assert argv is not None + assert Path(argv[0]) == expected + + @pytest.mark.parametrize("name", ["uv", "pipx"]) + @pytest.mark.parametrize("pathext", [None, ".CMD;.EXE"]) + @pytest.mark.parametrize("quoted_path", [False, True]) + def test_windows_pathext_is_expanded_before_absolute_lookup( + self, monkeypatch, tmp_path, name, pathext, quoted_path + ): + project = tmp_path / "project" + project.mkdir() + trusted = tmp_path / "trusted" + trusted.mkdir() + expected = str(trusted / f"{name}.EXE") + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", f'"{trusted}"' if quoted_path else str(trusted)) + if pathext is None: + monkeypatch.delenv("PATHEXT", raising=False) + else: + monkeypatch.setenv("PATHEXT", pathext) + + with ( + patch("specify_cli._version.sys") as platform, + patch( + "specify_cli._version.shutil.which", + side_effect=lambda command: expected if command == expected else None, + ) as which, + ): + platform.platform = "win32" + found = specify_cli._version._find_installer(name) + + assert found == expected + assert [call.args[0] for call in which.call_args_list] == [ + str(trusted / f"{name}{suffix}") + for suffix in ([".COM", ".EXE"] if pathext is None else [".CMD", ".EXE"]) + ] + + @pytest.mark.parametrize( + "method,name", + [ + (_InstallMethod.UV_TOOL, "uv"), + (_InstallMethod.PIPX, "pipx"), + ], + ) + def test_registry_probes_use_absolute_installer_path( + self, monkeypatch, tmp_path, method, name + ): + project = tmp_path / "project" + project.mkdir() + trusted = tmp_path / "trusted" + trusted.mkdir() + self.executable(project, "uv") + self.executable(project, "pipx") + expected = self.executable(trusted, name) + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", os.pathsep.join([".", str(trusted)])) + monkeypatch.setattr("sys.argv", [str(project / "missing" / "specify")]) + stdout = ( + "specify-cli v0.7.6\n" if name == "uv" else '{"venvs":{"specify-cli":{}}}' + ) + + with ( + patch("specify_cli._version._editable_marker_seen", return_value=False), + patch( + "specify_cli._version.subprocess.run", + return_value=_completed_process(0, stdout=stdout), + ) as run, + ): + detected = _detect_install_method() + + assert detected == method + run.assert_called_once() + assert Path(run.call_args.args[0][0]) == expected + + @pytest.mark.parametrize( + "method,name", + [ + (_InstallMethod.UV_TOOL, "uv"), + (_InstallMethod.PIPX, "pipx"), + ], + ) + def test_upgrade_executes_selected_absolute_installer( + self, monkeypatch, tmp_path, method, name + ): + project = tmp_path / "project" + project.mkdir() + trusted = tmp_path / "trusted" + trusted.mkdir() + self.executable(project, name) + expected = self.executable(trusted, name) + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", os.pathsep.join([".", str(trusted)])) + + with ( + patch("specify_cli._version._detect_install_method", return_value=method), + patch("specify_cli._version._get_installed_version", return_value="0.7.5"), + patch("specify_cli._version._verify_upgrade", return_value="0.7.6"), + patch( + "specify_cli._version.subprocess.run", + return_value=_completed_process(0), + ) as run, + ): + result = runner.invoke(app, ["self", "upgrade", "--tag", "v0.7.6"]) + + assert result.exit_code == 0 + run.assert_called_once() + argv = run.call_args.args[0] + assert Path(argv[0]) == expected + assert run.call_args.kwargs["shell"] is False + assert specify_cli._version._render_argv(argv) in strip_ansi(result.output) + + @pytest.mark.parametrize("name", ["uv", "pipx"]) + def test_user_bin_below_non_project_working_directory_is_allowed( + self, monkeypatch, tmp_path, name + ): + user_bin = tmp_path / ".local" / "bin" + user_bin.mkdir(parents=True) + expected = self.executable(user_bin, name) + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("PATH", str(user_bin)) + + found = specify_cli._version._find_installer(name) + + assert found is not None + assert Path(found) == expected + + @pytest.mark.parametrize("name", ["uv", "pipx"]) + def test_project_bin_is_rejected_from_nested_working_directory( + self, monkeypatch, tmp_path, name + ): + project = tmp_path / "project" + project.mkdir() + (project / ".git").write_text("gitdir: unused\n") + project_bin = project / "bin" + project_bin.mkdir() + nested = project / "nested" + nested.mkdir() + self.executable(project_bin, name) + monkeypatch.chdir(nested) + monkeypatch.setenv("PATH", str(project_bin)) + + assert specify_cli._version._find_installer(name) is None + + class TestBareUpgradeUvTool: """uv-tool happy path, bare invocation.""" @@ -618,6 +943,7 @@ def test_tier3_pipx_when_no_prefix_match_but_registry_lists_it( monkeypatch.setattr("sys.argv", [str(tmp_path / "missing" / "specify")]) def fake_which(name): + name = Path(name).stem if Path(name).is_absolute() else name return "pipx" if name == "pipx" else None def fake_run(argv, *args, **kwargs): @@ -645,6 +971,7 @@ def test_tier3_pipx_does_not_override_absolute_unsupported_entrypoint( unsupported_argv0, ): def fake_which(name): + name = Path(name).stem if Path(name).is_absolute() else name return "pipx" if name == "pipx" else None def fake_run(argv, *args, **kwargs): @@ -670,6 +997,7 @@ def test_tier3_pipx_ignores_malformed_json_output( unsupported_argv0, ): def fake_which(name): + name = Path(name).stem if Path(name).is_absolute() else name return "pipx" if name == "pipx" else None def fake_run(argv, *args, **kwargs): @@ -698,6 +1026,7 @@ def test_tier3_both_uv_tool_and_pipx_match_is_treated_as_unsupported( monkeypatch.setattr("sys.argv", [str(tmp_path / "missing" / "specify")]) def fake_which(name): + name = Path(name).stem if Path(name).is_absolute() else name if name == "uv": return "uv" if name == "pipx": diff --git a/tests/specify_cli/test_version_execution.py b/tests/specify_cli/test_version_execution.py index 9e3f2514fe..2f2294ee65 100644 --- a/tests/specify_cli/test_version_execution.py +++ b/tests/specify_cli/test_version_execution.py @@ -150,7 +150,7 @@ def fake_run(argv, *args, **kwargs): raise FileNotFoundError(str(fake_uv)) with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", + "specify_cli._version._find_installer", side_effect=lambda name: str(fake_uv) if name == "uv" else None, ), patch("specify_cli._version.subprocess.run", side_effect=fake_run), patch( "specify_cli._version._get_installed_version", return_value="0.7.5" From ee20e70993b0840575b8b305ab93d20162fc57e9 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 29 Oct 2026 08:38:22 -0500 Subject: [PATCH 2/5] fix: reject installer symlink paths through the worktree Validate directory ancestors and intermediate symlink targets, and reject installer executables resolving into the working directory or Git worktree. Execute the validated absolute target and use absolute installer fixtures in lookup mocks. Add regression coverage for alias, file, and combined symlink bypasses across discovery and upgrades. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/_version.py | 47 ++- tests/conftest.py | 13 + .../specify_cli/selfs/test_command_upgrade.py | 67 +++-- tests/specify_cli/test_version_detection.py | 275 ++++++++++++++---- tests/specify_cli/test_version_execution.py | 42 +-- .../specify_cli/test_version_verification.py | 34 ++- 6 files changed, 355 insertions(+), 123 deletions(-) diff --git a/src/specify_cli/_version.py b/src/specify_cli/_version.py index 06e17ef357..c2b8dda5be 100644 --- a/src/specify_cli/_version.py +++ b/src/specify_cli/_version.py @@ -485,6 +485,30 @@ def _find_installer(name: str) -> str | None: """Find an installer outside the working directory and current Git tree.""" cwd = Path.cwd().resolve() project_root = _git_ancestor(cwd) + + def outside_project(path: Path) -> bool: + if project_root is None: + return True + pending = [path] + seen: set[Path] = set() + while pending: + current = pending.pop() + for ancestor in (current, *current.parents): + if ancestor in seen: + continue + seen.add(ancestor) + if ancestor.is_relative_to( + project_root + ) or ancestor.resolve().is_relative_to(project_root): + return False + if ancestor.is_symlink(): + # Preserve intermediate targets when links enter and then leave the worktree. + target = ancestor.readlink() + pending.append( + target if target.is_absolute() else ancestor.parent / target + ) + return True + suffixes = [""] if sys.platform == "win32": # Python 3.11's which() does not expand PATHEXT for absolute commands. @@ -500,20 +524,25 @@ def _find_installer(name: str) -> str | None: if sys.platform == "win32": entry = entry.strip('"') directory = Path(entry) - if not directory.is_absolute(): - continue - resolved_directory = directory.resolve() - if directory == cwd or resolved_directory == cwd: - continue - if project_root is not None and ( - directory.is_relative_to(project_root) - or resolved_directory.is_relative_to(project_root) + if ( + not directory.is_absolute() + or directory.resolve() == cwd + or not outside_project(directory) ): continue for suffix in suffixes: found = shutil.which(str(directory / f"{name}{suffix}")) if found is not None: - return found + executable = Path(found) + if not executable.is_absolute(): + continue + resolved_executable = executable.resolve() + if ( + executable.parent.resolve() != cwd + and resolved_executable.parent != cwd + and outside_project(executable) + ): + return str(resolved_executable) return None diff --git a/tests/conftest.py b/tests/conftest.py index 6fe0c1e2f7..58957ac744 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -176,6 +176,19 @@ def clean_environ(monkeypatch): monkeypatch.delenv("GITHUB_TOKEN", raising=False) +@pytest.fixture +def installer_paths(tmp_path): + bin_dir = tmp_path / "installer-bin" + bin_dir.mkdir() + paths = {} + for name in ("uv", "pipx"): + executable = bin_dir / (f"{name}.exe" if os.name == "nt" else name) + executable.write_bytes(b"installer fixture; never executed\n") + executable.chmod(0o755) + paths[name] = str(executable) + return paths + + def _fake_self_upgrade_argv0(monkeypatch, tmp_path, env_name, path_parts): """Create a fake executable under tmp_path and point sys.argv[0] at it.""" monkeypatch.setenv(env_name, str(tmp_path)) diff --git a/tests/specify_cli/selfs/test_command_upgrade.py b/tests/specify_cli/selfs/test_command_upgrade.py index 5a39a9ddb7..a19b59b826 100644 --- a/tests/specify_cli/selfs/test_command_upgrade.py +++ b/tests/specify_cli/selfs/test_command_upgrade.py @@ -11,9 +11,12 @@ class TestTagValidation: """--tag option parsing and command help.""" - def test_valid_stable_tag(self, uv_tool_argv0, clean_environ): - with patch("specify_cli._version.shutil.which", return_value="uv"), patch( - "specify_cli._version._get_installed_version", return_value="0.7.5" + def test_valid_stable_tag(self, installer_paths, uv_tool_argv0, clean_environ): + with ( + patch( + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] + ), + patch("specify_cli._version._get_installed_version", return_value="0.7.5"), ): result = runner.invoke( app, @@ -21,9 +24,12 @@ def test_valid_stable_tag(self, uv_tool_argv0, clean_environ): ) assert result.exit_code == 0 - def test_valid_dev_suffix_tag(self, uv_tool_argv0, clean_environ): - with patch("specify_cli._version.shutil.which", return_value="uv"), patch( - "specify_cli._version._get_installed_version", return_value="0.7.5" + def test_valid_dev_suffix_tag(self, installer_paths, uv_tool_argv0, clean_environ): + with ( + patch( + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] + ), + patch("specify_cli._version._get_installed_version", return_value="0.7.5"), ): result = runner.invoke( app, @@ -32,9 +38,12 @@ def test_valid_dev_suffix_tag(self, uv_tool_argv0, clean_environ): assert result.exit_code == 0 assert "Target version: v0.8.0.dev0" in strip_ansi(result.output) - def test_valid_rc_tag(self, uv_tool_argv0, clean_environ): - with patch("specify_cli._version.shutil.which", return_value="uv"), patch( - "specify_cli._version._get_installed_version", return_value="0.7.5" + def test_valid_rc_tag(self, installer_paths, uv_tool_argv0, clean_environ): + with ( + patch( + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] + ), + patch("specify_cli._version._get_installed_version", return_value="0.7.5"), ): result = runner.invoke( app, @@ -43,10 +52,15 @@ def test_valid_rc_tag(self, uv_tool_argv0, clean_environ): assert result.exit_code == 0 def test_valid_beta_dot_tag_uses_pep440_equivalent_for_noop( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): - with patch("specify_cli._version.shutil.which", return_value="uv"), patch( - "specify_cli._version._get_installed_version", return_value="1.0.0b1" + with ( + patch( + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] + ), + patch( + "specify_cli._version._get_installed_version", return_value="1.0.0b1" + ), ): result = runner.invoke( app, @@ -57,9 +71,14 @@ def test_valid_beta_dot_tag_uses_pep440_equivalent_for_noop( result.output ) - def test_valid_build_metadata_tag(self, uv_tool_argv0, clean_environ): - with patch("specify_cli._version.shutil.which", return_value="uv"), patch( - "specify_cli._version._get_installed_version", return_value="0.7.5" + def test_valid_build_metadata_tag( + self, installer_paths, uv_tool_argv0, clean_environ + ): + with ( + patch( + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] + ), + patch("specify_cli._version._get_installed_version", return_value="0.7.5"), ): result = runner.invoke( app, @@ -69,10 +88,13 @@ def test_valid_build_metadata_tag(self, uv_tool_argv0, clean_environ): assert "Target version: v0.8.0+build.42" in strip_ansi(result.output) def test_uppercase_v_prefix_is_folded_to_lowercase( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): - with patch("specify_cli._version.shutil.which", return_value="uv"), patch( - "specify_cli._version._get_installed_version", return_value="0.7.5" + with ( + patch( + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] + ), + patch("specify_cli._version._get_installed_version", return_value="0.7.5"), ): result = runner.invoke( app, @@ -82,10 +104,13 @@ def test_uppercase_v_prefix_is_folded_to_lowercase( assert "Target version: v0.7.6" in strip_ansi(result.output) def test_valid_prerelease_with_build_metadata_tag( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): - with patch("specify_cli._version.shutil.which", return_value="uv"), patch( - "specify_cli._version._get_installed_version", return_value="0.7.5" + with ( + patch( + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] + ), + patch("specify_cli._version._get_installed_version", return_value="0.7.5"), ): result = runner.invoke( app, diff --git a/tests/specify_cli/test_version_detection.py b/tests/specify_cli/test_version_detection.py index 1901ee53d4..a9349257c6 100644 --- a/tests/specify_cli/test_version_detection.py +++ b/tests/specify_cli/test_version_detection.py @@ -84,6 +84,7 @@ def test_prefix_match_does_not_accept_sibling_directory(self, monkeypatch, tmp_p def test_tier3_uv_tool_when_registry_lists_exact_name( self, + installer_paths, monkeypatch, tmp_path, ): @@ -91,10 +92,10 @@ def test_tier3_uv_tool_when_registry_lists_exact_name( def fake_which(name): name = Path(name).stem if Path(name).is_absolute() else name - return "uv" if name == "uv" else None + return installer_paths["uv"] if name == "uv" else None def fake_run(argv, *args, **kwargs): - if argv[:3] == ["uv", "tool", "list"]: + if argv[:3] == [installer_paths["uv"], "tool", "list"]: return subprocess.CompletedProcess( args=argv, returncode=0, @@ -113,12 +114,12 @@ def fake_run(argv, *args, **kwargs): assert signals.matched_tier == 3 assert "uv tool list" in signals.installer_registries_consulted - def test_unresolved_bare_argv0_skips_tier3_registry_detection(self, monkeypatch): + def test_unresolved_bare_argv0_skips_tier3_registry_detection(self, installer_paths, monkeypatch): monkeypatch.setattr("sys.argv", ["specify"]) def fake_which(name): name = Path(name).stem if Path(name).is_absolute() else name - return "uv" if name == "uv" else None + return installer_paths["uv"] if name == "uv" else None def fake_run(argv, *args, **kwargs): return subprocess.CompletedProcess( @@ -136,7 +137,7 @@ def fake_run(argv, *args, **kwargs): assert signals.installer_registries_consulted == () def test_bare_argv0_missing_path_resolution_allows_tier3_registry_detection( - self, monkeypatch, tmp_path + self, installer_paths, monkeypatch, tmp_path ): missing_specify = tmp_path / "missing" / "specify" monkeypatch.setattr("sys.argv", ["specify"]) @@ -146,11 +147,11 @@ def fake_which(name): if name == "specify": return str(missing_specify) if name == "uv": - return "uv" + return installer_paths["uv"] return None def fake_run(argv, *args, **kwargs): - if argv[:3] == ["uv", "tool", "list"]: + if argv[:3] == [installer_paths["uv"], "tool", "list"]: return subprocess.CompletedProcess( args=argv, returncode=0, @@ -196,14 +197,15 @@ def fake_which(name): def test_tier3_uv_tool_ignores_substring_false_positive( self, + installer_paths, unsupported_argv0, ): def fake_which(name): name = Path(name).stem if Path(name).is_absolute() else name - return "uv" if name == "uv" else None + return installer_paths["uv"] if name == "uv" else None def fake_run(argv, *args, **kwargs): - if argv[:3] == ["uv", "tool", "list"]: + if argv[:3] == [installer_paths["uv"], "tool", "list"]: return subprocess.CompletedProcess( args=argv, returncode=0, @@ -222,14 +224,15 @@ def fake_run(argv, *args, **kwargs): def test_tier3_uv_tool_does_not_override_absolute_unsupported_entrypoint( self, + installer_paths, unsupported_argv0, ): def fake_which(name): name = Path(name).stem if Path(name).is_absolute() else name - return "uv" if name == "uv" else None + return installer_paths["uv"] if name == "uv" else None def fake_run(argv, *args, **kwargs): - if argv[:3] == ["uv", "tool", "list"]: + if argv[:3] == [installer_paths["uv"], "tool", "list"]: return subprocess.CompletedProcess( args=argv, returncode=0, @@ -248,6 +251,7 @@ def fake_run(argv, *args, **kwargs): def test_tier3_uv_tool_does_not_override_resolved_bare_unsupported_entrypoint( self, + installer_paths, monkeypatch, tmp_path, ): @@ -263,11 +267,11 @@ def fake_which(name): if name == "specify": return str(fake_specify) if name == "uv": - return "uv" + return installer_paths["uv"] return None def fake_run(argv, *args, **kwargs): - if argv[:3] == ["uv", "tool", "list"]: + if argv[:3] == [installer_paths["uv"], "tool", "list"]: return subprocess.CompletedProcess( args=argv, returncode=0, @@ -332,11 +336,11 @@ def test_path_lookup_resolve_oserror_returns_unresolved_lookup_path(self): class TestArgvAssemblyUvTool: """uv-tool installer argv shape.""" - def test_stable_tag_produces_expected_argv(self): - with patch("specify_cli._version.shutil.which", return_value="uv"): + def test_stable_tag_produces_expected_argv(self, installer_paths): + with patch("specify_cli._version.shutil.which", return_value=installer_paths["uv"]): argv = _assemble_installer_argv(_InstallMethod.UV_TOOL, "v0.7.6") assert argv == [ - "uv", + installer_paths["uv"], "tool", "install", "specify-cli", @@ -345,8 +349,8 @@ def test_stable_tag_produces_expected_argv(self): "git+https://github.com/github/spec-kit.git@v0.7.6", ] - def test_dev_suffix_tag_embedded_literally(self): - with patch("specify_cli._version.shutil.which", return_value="uv"): + def test_dev_suffix_tag_embedded_literally(self, installer_paths): + with patch("specify_cli._version.shutil.which", return_value=installer_paths["uv"]): argv = _assemble_installer_argv(_InstallMethod.UV_TOOL, "v0.8.0.dev0") assert "git+https://github.com/github/spec-kit.git@v0.8.0.dev0" in argv assert ( @@ -366,6 +370,152 @@ def executable(directory, name): executable.chmod(0o755) return executable + @pytest.mark.parametrize( + "method,name", + [(_InstallMethod.UV_TOOL, "uv"), (_InstallMethod.PIPX, "pipx")], + ) + @pytest.mark.parametrize( + "kind", + ["aliased-directory", "directory-target", "file", "file-aliased-directory"], + ) + @pytest.mark.parametrize("operation", ["lookup", "registry", "upgrade"]) + @pytest.mark.parametrize("fallback", [False, True]) + def test_symlink_bypasses_are_rejected( + self, monkeypatch, tmp_path, method, name, kind, operation, fallback + ): + project = tmp_path / "project" + project.mkdir() + (project / ".git").mkdir() + external = tmp_path / "external" + external.mkdir() + trusted = tmp_path / "trusted" + trusted.mkdir() + expected = self.executable(trusted, name) + try: + if kind != "file": + self.executable(external, name) + (project / "bin").symlink_to(external, target_is_directory=True) + alias = tmp_path / "alias" + alias.symlink_to(project, target_is_directory=True) + if kind == "aliased-directory": + unsafe = alias / "bin" + elif kind == "directory-target": + unsafe = tmp_path / "directory-target" + unsafe.symlink_to(project / "bin", target_is_directory=True) + else: + unsafe = tmp_path / "file-bin" + unsafe.mkdir() + executable_name = f"{name}.exe" if os.name == "nt" else name + (unsafe / executable_name).symlink_to( + alias / "bin" / executable_name + ) + else: + malicious = self.executable(project, name) + (external / malicious.name).symlink_to(malicious) + unsafe = external + except OSError: + pytest.skip("symlinks are unavailable") + monkeypatch.chdir(project) + monkeypatch.setenv( + "PATH", + os.pathsep.join([str(unsafe), str(trusted)] if fallback else [str(unsafe)]), + ) + monkeypatch.setattr("sys.argv", [str(project / "missing" / "specify")]) + stdout = ( + "specify-cli v0.7.6\n" if name == "uv" else '{"venvs":{"specify-cli":{}}}' + ) + + with ( + patch("specify_cli._version._editable_marker_seen", return_value=False), + patch("specify_cli._version._get_installed_version", return_value="0.7.5"), + patch("specify_cli._version._verify_upgrade", return_value="0.7.6"), + patch( + "specify_cli._version.subprocess.run", + return_value=_completed_process(0, stdout=stdout), + ) as run, + ): + if operation == "lookup": + argv = _assemble_installer_argv(method, "v0.7.6") + if fallback: + assert argv is not None + assert Path(argv[0]) == expected + else: + assert argv is None + run.assert_not_called() + return + if operation == "registry": + detected = _detect_install_method() + assert detected == (method if fallback else _InstallMethod.UNSUPPORTED) + else: + with patch( + "specify_cli._version._detect_install_method", return_value=method + ): + result = runner.invoke(app, ["self", "upgrade", "--tag", "v0.7.6"]) + assert result.exit_code == (0 if fallback else 3) + if not fallback: + assert f"Installer {name} not found on PATH" in strip_ansi( + result.output + ) + + if fallback: + run.assert_called_once() + assert Path(run.call_args.args[0][0]) == expected + else: + run.assert_not_called() + + @pytest.mark.parametrize("name", ["uv", "pipx"]) + def test_external_file_symlink_is_resolved_and_accepted( + self, monkeypatch, tmp_path, name + ): + project = tmp_path / "project" + project.mkdir() + (project / ".git").mkdir() + installed = tmp_path / "installed" + installed.mkdir() + expected = self.executable(installed, name) + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + try: + (bin_dir / expected.name).symlink_to(expected) + except OSError: + pytest.skip("file symlinks are unavailable") + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", str(bin_dir)) + + found = specify_cli._version._find_installer(name) + + assert found is not None + assert Path(found) == expected + + @pytest.mark.parametrize("name", ["uv", "pipx"]) + def test_relative_lookup_result_is_rejected(self, monkeypatch, tmp_path, name): + project = tmp_path / "project" + project.mkdir() + trusted = tmp_path / "trusted" + trusted.mkdir() + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", str(trusted)) + with patch("specify_cli._version.shutil.which", return_value=name): + assert specify_cli._version._find_installer(name) is None + + @pytest.mark.parametrize("name", ["uv", "pipx"]) + def test_file_symlink_to_non_project_cwd_is_rejected( + self, monkeypatch, tmp_path, name + ): + project = tmp_path / "project" + project.mkdir() + malicious = self.executable(project, name) + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + try: + (bin_dir / malicious.name).symlink_to(malicious) + except OSError: + pytest.skip("file symlinks are unavailable") + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", str(bin_dir)) + + assert specify_cli._version._find_installer(name) is None + @pytest.mark.parametrize( "method,name", [ @@ -680,9 +830,9 @@ def test_project_bin_is_rejected_from_nested_working_directory( class TestBareUpgradeUvTool: """uv-tool happy path, bare invocation.""" - def test_happy_path_end_to_end(self, uv_tool_argv0, clean_environ): + def test_happy_path_end_to_end(self, installer_paths, uv_tool_argv0, clean_environ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -701,11 +851,11 @@ def test_happy_path_end_to_end(self, uv_tool_argv0, clean_environ): for call in mock_run.call_args_list: assert call.kwargs.get("shell", False) is False - def test_one_user_action_no_prompt(self, uv_tool_argv0, clean_environ): + def test_one_user_action_no_prompt(self, installer_paths, uv_tool_argv0, clean_environ): # The single `invoke` represents the single user action — no prompt. # If a prompt existed, runner.invoke would hang waiting for input. with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -722,12 +872,12 @@ class TestAlreadyLatestUvTool: """already on latest, no installer launched.""" def test_already_latest_exits_zero_no_subprocess( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli._version.subprocess.run" ) as mock_run, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version._get_installed_version", return_value="0.7.6"): mock_urlopen.return_value = mock_urlopen_response({"tag_name": "v0.7.6"}) result = runner.invoke(app, ["self", "upgrade"]) @@ -737,7 +887,7 @@ def test_already_latest_exits_zero_no_subprocess( assert mock_run.call_count == 0 def test_trailing_zero_equivalent_version_reports_latest_not_newer( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): # Version("1.0") == Version("1.0.0") under packaging even though their # canonical strings differ. The no-op message must use Version equality @@ -745,7 +895,7 @@ def test_trailing_zero_equivalent_version_reports_latest_not_newer( with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli._version.subprocess.run" ) as mock_run, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version._get_installed_version", return_value="1.0"): mock_urlopen.return_value = mock_urlopen_response({"tag_name": "v1.0.0"}) result = runner.invoke(app, ["self", "upgrade"]) @@ -757,12 +907,12 @@ def test_trailing_zero_equivalent_version_reports_latest_not_newer( assert mock_run.call_count == 0 def test_dev_build_ahead_of_release_reports_newer_noop( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli._version.subprocess.run" ) as mock_run, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version._get_installed_version", return_value="0.7.7.dev0"): mock_urlopen.return_value = mock_urlopen_response({"tag_name": "v0.7.6"}) result = runner.invoke(app, ["self", "upgrade"]) @@ -772,12 +922,12 @@ def test_dev_build_ahead_of_release_reports_newer_noop( assert mock_run.call_count == 0 def test_unparseable_current_version_does_not_false_noop( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli._version.subprocess.run" ) as mock_run, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version._get_installed_version", return_value="release-main"): mock_urlopen.return_value = mock_urlopen_response({"tag_name": "v0.7.6"}) mock_run.side_effect = [ @@ -793,12 +943,12 @@ def test_unparseable_current_version_does_not_false_noop( assert mock_run.call_count == 2 def test_unparseable_resolved_target_fails_before_literal_noop( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli._version.subprocess.run" ) as mock_run, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version._get_installed_version", return_value="release-main"): mock_urlopen.return_value = mock_urlopen_response({"tag_name": "release-main"}) result = runner.invoke(app, ["self", "upgrade"]) @@ -811,9 +961,9 @@ def test_unparseable_resolved_target_fails_before_literal_noop( assert mock_run.call_count == 0 def test_pinned_older_tag_still_runs_installer( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): - with patch("specify_cli._version.shutil.which", return_value="uv"), patch( + with patch("specify_cli._version.shutil.which", return_value=installer_paths["uv"]), patch( "specify_cli._version.subprocess.run" ) as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.6" @@ -833,9 +983,9 @@ def test_pinned_older_tag_still_runs_installer( assert mock_run.call_count == 2 def test_pinned_rc_tag_uses_canonical_version_equality_for_noop( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): - with patch("specify_cli._version.shutil.which", return_value="uv"), patch( + with patch("specify_cli._version.shutil.which", return_value=installer_paths["uv"]), patch( "specify_cli._version._get_installed_version", return_value="1.0.0rc1" ): result = runner.invoke(app, ["self", "upgrade", "--tag", "v1.0.0-rc1"]) @@ -849,13 +999,14 @@ class TestDryRunUvTool: def test_dry_run_without_tag_resolves_network_but_no_subprocess( self, + installer_paths, uv_tool_argv0, clean_environ, ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli._version.subprocess.run" ) as mock_run, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version._get_installed_version", return_value="0.7.5"): mock_urlopen.return_value = mock_urlopen_response({"tag_name": "v0.7.6"}) result = runner.invoke(app, ["self", "upgrade", "--dry-run"]) @@ -869,11 +1020,11 @@ def test_dry_run_without_tag_resolves_network_but_no_subprocess( assert "Command that would be executed:" in out assert mock_run.call_count == 0 - def test_dry_run_with_tag_skips_network(self, uv_tool_argv0, clean_environ): + def test_dry_run_with_tag_skips_network(self, installer_paths, uv_tool_argv0, clean_environ): # --dry-run with --tag must NOT hit the network. with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli._version.subprocess.run" - ), patch("specify_cli._version.shutil.which", return_value="uv"), patch( + ), patch("specify_cli._version.shutil.which", return_value=installer_paths["uv"]), patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): result = runner.invoke( @@ -885,12 +1036,12 @@ def test_dry_run_with_tag_skips_network(self, uv_tool_argv0, clean_environ): mock_urlopen.assert_not_called() def test_dry_run_rejects_unparseable_network_tag_before_preview( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli._version.subprocess.run" ) as mock_run, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version._get_installed_version", return_value="0.7.5"): mock_urlopen.return_value = mock_urlopen_response( {"tag_name": "v0.9.0;echo unsafe"} @@ -937,6 +1088,7 @@ def test_posix_pipx_prefix_matches(self, pipx_argv0): def test_tier3_pipx_when_no_prefix_match_but_registry_lists_it( self, + installer_paths, monkeypatch, tmp_path, ): @@ -944,10 +1096,10 @@ def test_tier3_pipx_when_no_prefix_match_but_registry_lists_it( def fake_which(name): name = Path(name).stem if Path(name).is_absolute() else name - return "pipx" if name == "pipx" else None + return installer_paths["pipx"] if name == "pipx" else None def fake_run(argv, *args, **kwargs): - if argv[:3] == ["pipx", "list", "--json"]: + if argv[:3] == [installer_paths["pipx"], "list", "--json"]: return subprocess.CompletedProcess( args=argv, returncode=0, @@ -968,14 +1120,15 @@ def fake_run(argv, *args, **kwargs): def test_tier3_pipx_does_not_override_absolute_unsupported_entrypoint( self, + installer_paths, unsupported_argv0, ): def fake_which(name): name = Path(name).stem if Path(name).is_absolute() else name - return "pipx" if name == "pipx" else None + return installer_paths["pipx"] if name == "pipx" else None def fake_run(argv, *args, **kwargs): - if argv[:3] == ["pipx", "list", "--json"]: + if argv[:3] == [installer_paths["pipx"], "list", "--json"]: return subprocess.CompletedProcess( args=argv, returncode=0, @@ -994,14 +1147,15 @@ def fake_run(argv, *args, **kwargs): def test_tier3_pipx_ignores_malformed_json_output( self, + installer_paths, unsupported_argv0, ): def fake_which(name): name = Path(name).stem if Path(name).is_absolute() else name - return "pipx" if name == "pipx" else None + return installer_paths["pipx"] if name == "pipx" else None def fake_run(argv, *args, **kwargs): - if argv[:3] == ["pipx", "list", "--json"]: + if argv[:3] == [installer_paths["pipx"], "list", "--json"]: return subprocess.CompletedProcess( args=argv, returncode=0, @@ -1020,6 +1174,7 @@ def fake_run(argv, *args, **kwargs): def test_tier3_both_uv_tool_and_pipx_match_is_treated_as_unsupported( self, + installer_paths, monkeypatch, tmp_path, ): @@ -1028,20 +1183,20 @@ def test_tier3_both_uv_tool_and_pipx_match_is_treated_as_unsupported( def fake_which(name): name = Path(name).stem if Path(name).is_absolute() else name if name == "uv": - return "uv" + return installer_paths["uv"] if name == "pipx": - return "pipx" + return installer_paths["pipx"] return None def fake_run(argv, *args, **kwargs): - if argv[:3] == ["uv", "tool", "list"]: + if argv[:3] == [installer_paths["uv"], "tool", "list"]: return subprocess.CompletedProcess( args=argv, returncode=0, stdout="specify-cli v0.7.6\n", stderr="", ) - if argv[:3] == ["pipx", "list", "--json"]: + if argv[:3] == [installer_paths["pipx"], "list", "--json"]: return subprocess.CompletedProcess( args=argv, returncode=0, @@ -1128,9 +1283,9 @@ def locate_file(self, file): class TestTagValidationWhitespace: - def test_tag_whitespace_is_trimmed_before_validation(self, uv_tool_argv0, clean_environ): + def test_tag_whitespace_is_trimmed_before_validation(self, installer_paths, uv_tool_argv0, clean_environ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -1148,11 +1303,11 @@ def test_tag_whitespace_is_trimmed_before_validation(self, uv_tool_argv0, clean_ class TestArgvAssemblyPipx: """pipx installer argv shape — pipx 1.5+ uses positional PACKAGE_SPEC, never `--spec` or `upgrade`.""" - def test_pipx_argv_uses_install_force_positional_not_upgrade(self): - with patch("specify_cli._version.shutil.which", return_value="pipx"): + def test_pipx_argv_uses_install_force_positional_not_upgrade(self, installer_paths): + with patch("specify_cli._version.shutil.which", return_value=installer_paths["pipx"]): argv = _assemble_installer_argv(_InstallMethod.PIPX, "v0.7.6") assert argv == [ - "pipx", + installer_paths["pipx"], "install", "--force", "git+https://github.com/github/spec-kit.git@v0.7.6", @@ -1168,9 +1323,9 @@ def test_missing_pipx_returns_no_installer_argv(self): class TestBareUpgradePipx: """pipx happy path.""" - def test_happy_path(self, pipx_argv0, clean_environ): + def test_happy_path(self, installer_paths, pipx_argv0, clean_environ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="pipx" + "specify_cli._version.shutil.which", return_value=installer_paths["pipx"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -1204,9 +1359,9 @@ def test_pipx_argv0_prefix_short_circuits_before_registry_checks( class TestDryRunPipx: - def test_dry_run_preview_names_pipx(self, pipx_argv0, clean_environ): + def test_dry_run_preview_names_pipx(self, installer_paths, pipx_argv0, clean_environ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="pipx" + "specify_cli._version.shutil.which", return_value=installer_paths["pipx"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): diff --git a/tests/specify_cli/test_version_execution.py b/tests/specify_cli/test_version_execution.py index 2f2294ee65..3d9b336d42 100644 --- a/tests/specify_cli/test_version_execution.py +++ b/tests/specify_cli/test_version_execution.py @@ -231,10 +231,10 @@ def test_relative_installer_path_not_executable_gets_path_specific_message( assert "Installer ./uv is not executable" not in out def test_real_installer_exit_126_is_not_treated_as_invalid_path( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -307,10 +307,10 @@ def test_exec_oserror_is_treated_as_invalid_installer( assert "not found on PATH" not in out def test_bare_invalid_installer_message_does_not_call_it_a_path( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version._get_installed_version", return_value="0.7.5"), patch( "specify_cli._version._assemble_installer_argv", return_value=[ @@ -397,9 +397,9 @@ def test_transient_exec_oserror_is_not_treated_as_invalid_installer( class TestInstallerFailed: """Installer non-zero exit → propagate code, print rollback hint.""" - def test_installer_exit_2_propagates(self, uv_tool_argv0, clean_environ): + def test_installer_exit_2_propagates(self, installer_paths, uv_tool_argv0, clean_environ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -420,9 +420,9 @@ def test_installer_exit_2_propagates(self, uv_tool_argv0, clean_environ): # No verification attempted after a failed installer run. assert mock_run.call_count == 1 - def test_installer_exit_127_propagates(self, uv_tool_argv0, clean_environ): + def test_installer_exit_127_propagates(self, installer_paths, uv_tool_argv0, clean_environ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -432,17 +432,17 @@ def test_installer_exit_127_propagates(self, uv_tool_argv0, clean_environ): assert result.exit_code == 127 def test_installer_timeout_prints_timeout_specific_message( - self, uv_tool_argv0, clean_environ, monkeypatch + self, installer_paths, uv_tool_argv0, clean_environ, monkeypatch ): monkeypatch.setenv("SPECIFY_UPGRADE_TIMEOUT_SECS", "12") with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): mock_urlopen.return_value = mock_urlopen_response({"tag_name": "v0.7.6"}) mock_run.side_effect = [ - subprocess.TimeoutExpired(cmd=["uv"], timeout=12) + subprocess.TimeoutExpired(cmd=[installer_paths["uv"]], timeout=12) ] result = runner.invoke(app, ["self", "upgrade"]) assert result.exit_code == 124 @@ -451,11 +451,11 @@ def test_installer_timeout_prints_timeout_specific_message( assert "SPECIFY_UPGRADE_TIMEOUT_SECS=12" in out def test_non_finite_timeout_warns_and_runs_without_timeout( - self, uv_tool_argv0, clean_environ, monkeypatch + self, installer_paths, uv_tool_argv0, clean_environ, monkeypatch ): monkeypatch.setenv("SPECIFY_UPGRADE_TIMEOUT_SECS", "nan") with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -473,10 +473,10 @@ def test_non_finite_timeout_warns_and_runs_without_timeout( assert mock_run.call_args_list[0].kwargs["timeout"] is None def test_real_installer_exit_124_is_not_treated_as_timeout( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -488,9 +488,9 @@ def test_real_installer_exit_124_is_not_treated_as_timeout( assert "Upgrade failed. Installer exit code: 124." in out assert "Upgrade timed out while waiting for the installer subprocess." not in out - def test_pipx_failure_prints_pipx_rollback_hint(self, pipx_argv0, clean_environ): + def test_pipx_failure_prints_pipx_rollback_hint(self, installer_paths, pipx_argv0, clean_environ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="pipx" + "specify_cli._version.shutil.which", return_value=installer_paths["pipx"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -505,10 +505,10 @@ def test_pipx_failure_prints_pipx_rollback_hint(self, pipx_argv0, clean_environ) ) in out def test_rollback_hint_accepts_normalizable_stable_snapshot( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="v0.7.5" ): @@ -525,10 +525,10 @@ def test_rollback_hint_accepts_normalizable_stable_snapshot( assert "Previous version was not an exact stable release tag" not in out def test_prerelease_failure_degrades_rollback_hint_to_releases_page( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="1.0.0rc1" ): diff --git a/tests/specify_cli/test_version_verification.py b/tests/specify_cli/test_version_verification.py index f3311bb23f..669ac1291e 100644 --- a/tests/specify_cli/test_version_verification.py +++ b/tests/specify_cli/test_version_verification.py @@ -30,11 +30,12 @@ class TestVerificationMismatch: def test_installer_ok_but_verify_returns_old_version( self, + installer_paths, uv_tool_argv0, clean_environ, ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -53,11 +54,12 @@ def test_installer_ok_but_verify_returns_old_version( def test_verify_nonzero_exit_is_not_treated_as_success( self, + installer_paths, uv_tool_argv0, clean_environ, ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -75,11 +77,12 @@ def test_verify_nonzero_exit_is_not_treated_as_success( def test_verify_accepts_pep440_equivalent_rc_version( self, + installer_paths, uv_tool_argv0, clean_environ, ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.9.0" ): @@ -95,11 +98,12 @@ def test_verify_accepts_pep440_equivalent_rc_version( def test_verify_accepts_specify_cli_binary_name_in_version_output( self, + installer_paths, uv_tool_argv0, clean_environ, ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -115,11 +119,12 @@ def test_verify_accepts_specify_cli_binary_name_in_version_output( def test_verify_accepts_capitalized_binary_name_in_version_output( self, + installer_paths, uv_tool_argv0, clean_environ, ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -135,11 +140,12 @@ def test_verify_accepts_capitalized_binary_name_in_version_output( def test_verify_rejects_output_without_parseable_version( self, + installer_paths, uv_tool_argv0, clean_environ, ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -344,12 +350,12 @@ def test_http_error_categorization( assert expected in strip_ansi(result.output) def test_unparseable_resolved_release_tag_exits_1_without_traceback( - self, uv_tool_argv0, clean_environ + self, installer_paths, uv_tool_argv0, clean_environ ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli._version.subprocess.run" ) as mock_run, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version._get_installed_version", return_value="0.7.5"): mock_urlopen.return_value = mock_urlopen_response({"tag_name": "release-main"}) result = runner.invoke(app, ["self", "upgrade"]) @@ -367,11 +373,12 @@ class TestUnknownCurrent: def test_unknown_current_renders_literal_in_notice( self, + installer_paths, uv_tool_argv0, clean_environ, ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="unknown" ): @@ -389,11 +396,12 @@ def test_unknown_current_renders_literal_in_notice( def test_unknown_current_rollback_hint_degrades( self, + installer_paths, uv_tool_argv0, clean_environ, ): with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="unknown" ): @@ -412,6 +420,7 @@ class TestTokenScrubbing: def test_env_passed_to_subprocess_has_no_github_tokens( self, + installer_paths, uv_tool_argv0, monkeypatch, ): @@ -422,7 +431,7 @@ def test_env_passed_to_subprocess_has_no_github_tokens( with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli.authentication.http.urllib.request.build_opener" ) as mock_build_opener, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): @@ -445,6 +454,7 @@ def test_env_passed_to_subprocess_has_no_github_tokens( def test_env_scrubbing_is_case_insensitive( self, + installer_paths, uv_tool_argv0, monkeypatch, ): @@ -455,7 +465,7 @@ def test_env_scrubbing_is_case_insensitive( with patch("specify_cli.authentication.http.urllib.request.urlopen") as mock_urlopen, patch( "specify_cli.authentication.http.urllib.request.build_opener" ) as mock_build_opener, patch( - "specify_cli._version.shutil.which", return_value="uv" + "specify_cli._version.shutil.which", return_value=installer_paths["uv"] ), patch("specify_cli._version.subprocess.run") as mock_run, patch( "specify_cli._version._get_installed_version", return_value="0.7.5" ): From cd1e7d2f424bb42499a3a03fc0d5b7e8e2b48c00 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 29 Oct 2026 08:56:42 -0500 Subject: [PATCH 3/5] fix: normalize Windows installer path comparisons Reuse the existing comparison-only extended-path normalization for installer cwd and worktree checks. Preserve original filesystem access paths while recognizing equivalent Windows drive and UNC namespace spellings, including intermediate symlink targets. Add portable containment regressions for plain and extended roots and targets. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/_version.py | 30 ++++++++++----------- tests/specify_cli/test_version_detection.py | 24 ++++++++++++++++- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/specify_cli/_version.py b/src/specify_cli/_version.py index c2b8dda5be..9e160b3d57 100644 --- a/src/specify_cli/_version.py +++ b/src/specify_cli/_version.py @@ -24,12 +24,13 @@ import urllib.request from dataclasses import dataclass from enum import Enum -from pathlib import Path +from pathlib import Path, PurePath import typer from packaging.version import InvalidVersion, Version from ._download_security import MAX_JSON_METADATA_BYTES, read_response_limited from ._console import console +from .file_helper import _strip_extended_length_prefix GITHUB_API_LATEST = "https://api.github.com/repos/github/spec-kit/releases/latest" _RESOLUTION_FAILURE_OFFLINE = "offline or timeout" @@ -359,17 +360,13 @@ def _expand_prefix(prefix: str) -> Path | None: return None -def _path_is_within_prefix(path: Path, prefix: Path) -> bool: +def _path_is_within_prefix(path: PurePath, prefix: PurePath) -> bool: """Return whether absolute `path` is under absolute `prefix`.""" if not path.is_absolute() or not prefix.is_absolute(): return False - try: - common = os.path.commonpath( - [os.path.normcase(str(path)), os.path.normcase(str(prefix))] - ) - except ValueError: - return False - return common == os.path.normcase(str(prefix)) + return _strip_extended_length_prefix(path).is_relative_to( + _strip_extended_length_prefix(prefix) + ) def _resolve_path_or_original(path: Path) -> Path: @@ -484,6 +481,7 @@ def _editable_marker_seen() -> bool: def _find_installer(name: str) -> str | None: """Find an installer outside the working directory and current Git tree.""" cwd = Path.cwd().resolve() + cwd_comparison = _strip_extended_length_prefix(cwd) project_root = _git_ancestor(cwd) def outside_project(path: Path) -> bool: @@ -497,9 +495,9 @@ def outside_project(path: Path) -> bool: if ancestor in seen: continue seen.add(ancestor) - if ancestor.is_relative_to( - project_root - ) or ancestor.resolve().is_relative_to(project_root): + if _path_is_within_prefix( + ancestor, project_root + ) or _path_is_within_prefix(ancestor.resolve(), project_root): return False if ancestor.is_symlink(): # Preserve intermediate targets when links enter and then leave the worktree. @@ -526,7 +524,7 @@ def outside_project(path: Path) -> bool: directory = Path(entry) if ( not directory.is_absolute() - or directory.resolve() == cwd + or _strip_extended_length_prefix(directory.resolve()) == cwd_comparison or not outside_project(directory) ): continue @@ -538,8 +536,10 @@ def outside_project(path: Path) -> bool: continue resolved_executable = executable.resolve() if ( - executable.parent.resolve() != cwd - and resolved_executable.parent != cwd + _strip_extended_length_prefix(executable.parent.resolve()) + != cwd_comparison + and _strip_extended_length_prefix(resolved_executable.parent) + != cwd_comparison and outside_project(executable) ): return str(resolved_executable) diff --git a/tests/specify_cli/test_version_detection.py b/tests/specify_cli/test_version_detection.py index a9349257c6..751661124f 100644 --- a/tests/specify_cli/test_version_detection.py +++ b/tests/specify_cli/test_version_detection.py @@ -4,7 +4,7 @@ import json import os import subprocess -from pathlib import Path +from pathlib import Path, PureWindowsPath from unittest.mock import patch import pytest @@ -362,6 +362,28 @@ def test_missing_uv_returns_no_installer_argv(self): assert _assemble_installer_argv(_InstallMethod.UV_TOOL, "v0.7.6") is None +class TestWindowsPathContainment: + @pytest.mark.parametrize("unc", [False, True]) + @pytest.mark.parametrize("root_extended", [False, True]) + @pytest.mark.parametrize("target_extended", [False, True]) + @pytest.mark.parametrize("location", ["root", "child", "sibling"]) + def test_namespace_spellings_share_containment( + self, unc, root_extended, target_extended, location + ): + plain = r"\\server\share\project" if unc else r"C:\project" + extended = r"\\?\UNC\server\share\project" if unc else r"\\?\C:\project" + root = PureWindowsPath(extended if root_extended else plain) + target = PureWindowsPath(extended if target_extended else plain) + if location == "child": + target /= "bin" + elif location == "sibling": + target = target.with_name("project-sibling") / "bin" + + assert specify_cli._version._path_is_within_prefix(target, root) is ( + location != "sibling" + ) + + class TestInstallerLookupSecurity: @staticmethod def executable(directory, name): From b98bc93e5083605b28547db9d9cd8c024138e216 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 29 Oct 2026 09:18:42 -0500 Subject: [PATCH 4/5] fix: validate installer paths by filesystem identity Inspect intermediate executable links even without a Git root, compare filesystem identities for cwd and worktree boundaries, and reject unsupported Windows reparse points. Warn and continue after per-entry inspection failures while preserving the selected absolute installer shim spelling. Add case-alias, CWD-chain, dispatcher, failure-recovery, and native Windows namespace/junction regressions. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/_version.py | 93 +++++--- tests/specify_cli/test_version_detection.py | 233 +++++++++++++++++++- 2 files changed, 283 insertions(+), 43 deletions(-) diff --git a/src/specify_cli/_version.py b/src/specify_cli/_version.py index 9e160b3d57..82f8b1ccb8 100644 --- a/src/specify_cli/_version.py +++ b/src/specify_cli/_version.py @@ -17,6 +17,7 @@ import re import shlex import shutil +import stat import subprocess import sys import urllib.error @@ -481,29 +482,48 @@ def _editable_marker_seen() -> bool: def _find_installer(name: str) -> str | None: """Find an installer outside the working directory and current Git tree.""" cwd = Path.cwd().resolve() - cwd_comparison = _strip_extended_length_prefix(cwd) + cwd_status = cwd.stat() project_root = _git_ancestor(cwd) - - def outside_project(path: Path) -> bool: - if project_root is None: - return True - pending = [path] - seen: set[Path] = set() + project_status = project_root.stat() if project_root is not None else None + + def eligible(path: Path, *, executable: bool) -> bool: + # Check that traversal terminates before inspecting intermediate links. + path.resolve(strict=True) + if not executable and os.path.samestat(path.stat(), cwd_status): + return False + pending = [(path, executable)] + seen: set[tuple[Path, bool]] = set() while pending: - current = pending.pop() + current, leaf_executable = pending.pop() + if leaf_executable and os.path.samestat(current.parent.stat(), cwd_status): + return False for ancestor in (current, *current.parents): - if ancestor in seen: + is_executable = leaf_executable and ancestor == current + key = (ancestor, is_executable) + if key in seen: continue - seen.add(ancestor) - if _path_is_within_prefix( - ancestor, project_root - ) or _path_is_within_prefix(ancestor.resolve(), project_root): + seen.add(key) + entry_status = ancestor.lstat() + is_link = stat.S_ISLNK(entry_status.st_mode) + if ( + getattr(entry_status, "st_file_attributes", 0) + & stat.FILE_ATTRIBUTE_REPARSE_POINT + and not is_link + ): return False - if ancestor.is_symlink(): - # Preserve intermediate targets when links enter and then leave the worktree. + if project_status is not None and os.path.samestat( + entry_status, project_status + ): + return False + if is_link: target = ancestor.readlink() pending.append( - target if target.is_absolute() else ancestor.parent / target + ( + target + if target.is_absolute() + else ancestor.parent / target, + is_executable, + ) ) return True @@ -522,27 +542,28 @@ def outside_project(path: Path) -> bool: if sys.platform == "win32": entry = entry.strip('"') directory = Path(entry) - if ( - not directory.is_absolute() - or _strip_extended_length_prefix(directory.resolve()) == cwd_comparison - or not outside_project(directory) - ): + if not directory.is_absolute(): continue - for suffix in suffixes: - found = shutil.which(str(directory / f"{name}{suffix}")) - if found is not None: - executable = Path(found) - if not executable.is_absolute(): - continue - resolved_executable = executable.resolve() - if ( - _strip_extended_length_prefix(executable.parent.resolve()) - != cwd_comparison - and _strip_extended_length_prefix(resolved_executable.parent) - != cwd_comparison - and outside_project(executable) - ): - return str(resolved_executable) + try: + if not eligible(directory, executable=False): + continue + for suffix in suffixes: + found = shutil.which(str(directory / f"{name}{suffix}")) + if found is not None: + executable = Path(found) + if executable.is_absolute() and eligible( + executable, executable=True + ): + return str(executable) + except FileNotFoundError: + continue + except (OSError, RuntimeError) as exc: + console.print( + f"Skipping installer PATH entry {directory} while finding {name}: {exc}", + style="yellow", + markup=False, + soft_wrap=True, + ) return None diff --git a/tests/specify_cli/test_version_detection.py b/tests/specify_cli/test_version_detection.py index 751661124f..4d7003a27a 100644 --- a/tests/specify_cli/test_version_detection.py +++ b/tests/specify_cli/test_version_detection.py @@ -5,6 +5,7 @@ import os import subprocess from pathlib import Path, PureWindowsPath +from types import SimpleNamespace from unittest.mock import patch import pytest @@ -398,7 +399,13 @@ def executable(directory, name): ) @pytest.mark.parametrize( "kind", - ["aliased-directory", "directory-target", "file", "file-aliased-directory"], + [ + "aliased-directory", + "directory-target", + "file", + "file-aliased-directory", + "cwd-file-chain", + ], ) @pytest.mark.parametrize("operation", ["lookup", "registry", "upgrade"]) @pytest.mark.parametrize("fallback", [False, True]) @@ -407,14 +414,23 @@ def test_symlink_bypasses_are_rejected( ): project = tmp_path / "project" project.mkdir() - (project / ".git").mkdir() + if kind != "cwd-file-chain": + (project / ".git").mkdir() external = tmp_path / "external" external.mkdir() trusted = tmp_path / "trusted" trusted.mkdir() expected = self.executable(trusted, name) try: - if kind != "file": + if kind == "cwd-file-chain": + payload_dir = tmp_path / "payload" + payload_dir.mkdir() + payload = self.executable(payload_dir, name) + malicious = project / payload.name + malicious.symlink_to(payload) + (external / payload.name).symlink_to(malicious) + unsafe = external + elif kind != "file": self.executable(external, name) (project / "bin").symlink_to(external, target_is_directory=True) alias = tmp_path / "alias" @@ -486,29 +502,230 @@ def test_symlink_bypasses_are_rejected( run.assert_not_called() @pytest.mark.parametrize("name", ["uv", "pipx"]) - def test_external_file_symlink_is_resolved_and_accepted( - self, monkeypatch, tmp_path, name + @pytest.mark.parametrize("operation", ["lookup", "upgrade"]) + def test_external_file_symlink_preserves_dispatch_name( + self, monkeypatch, tmp_path, name, operation ): project = tmp_path / "project" project.mkdir() (project / ".git").mkdir() installed = tmp_path / "installed" installed.mkdir() - expected = self.executable(installed, name) + dispatcher = self.executable(installed, "dispatcher") bin_dir = tmp_path / "bin" bin_dir.mkdir() + selected = bin_dir / (f"{name}.exe" if os.name == "nt" else name) try: - (bin_dir / expected.name).symlink_to(expected) + selected.symlink_to(dispatcher) except OSError: pytest.skip("file symlinks are unavailable") monkeypatch.chdir(project) monkeypatch.setenv("PATH", str(bin_dir)) + if operation == "lookup": + found = specify_cli._version._find_installer(name) + assert found is not None + assert Path(found) == selected + else: + method = _InstallMethod.UV_TOOL if name == "uv" else _InstallMethod.PIPX + with ( + patch( + "specify_cli._version._detect_install_method", return_value=method + ), + patch( + "specify_cli._version._get_installed_version", return_value="0.7.5" + ), + patch("specify_cli._version._verify_upgrade", return_value="0.7.6"), + patch( + "specify_cli._version.subprocess.run", + return_value=_completed_process(0), + ) as run, + ): + result = runner.invoke(app, ["self", "upgrade", "--tag", "v0.7.6"]) + assert result.exit_code == 0 + assert Path(run.call_args.args[0][0]) == selected + + @pytest.mark.parametrize("name", ["uv", "pipx"]) + @pytest.mark.parametrize("git_project", [False, True]) + @pytest.mark.parametrize("fallback", [False, True]) + def test_case_aliases_are_rejected_on_case_insensitive_volumes( + self, monkeypatch, tmp_path, name, git_project, fallback + ): + project = tmp_path / "project" + project.mkdir() + alias = project.with_name("PROJECT") + if not alias.exists() or not alias.samefile(project): + pytest.skip("case-insensitive volume required") + unsafe = project + if git_project: + (project / ".git").mkdir() + unsafe = project / "bin" + unsafe.mkdir() + self.executable(unsafe, name) + trusted = tmp_path / "trusted" + trusted.mkdir() + expected = self.executable(trusted, name) + monkeypatch.chdir(project) + entry = alias / "bin" if git_project else alias + monkeypatch.setenv( + "PATH", + os.pathsep.join([str(entry), str(trusted)] if fallback else [str(entry)]), + ) + found = specify_cli._version._find_installer(name) + if fallback: + assert found is not None + assert Path(found) == expected + else: + assert found is None + + @pytest.mark.parametrize("name", ["uv", "pipx"]) + @pytest.mark.parametrize("stage", ["resolve", "lstat"]) + @pytest.mark.parametrize( + "error", [PermissionError("denied"), RuntimeError("link loop")] + ) + def test_inspection_failure_warns_and_checks_later_entries( + self, monkeypatch, tmp_path, capsys, name, stage, error + ): + project = tmp_path / "project" + project.mkdir() + (project / ".git").mkdir() + bad = tmp_path / "bad" + bad.mkdir() + trusted = tmp_path / "trusted" + trusted.mkdir() + expected = self.executable(trusted, name) + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", os.pathsep.join([str(bad), str(trusted)])) + original = getattr(Path, stage) + + def inspect(path, *args, **kwargs): + if path == bad: + raise error + return original(path, *args, **kwargs) + + monkeypatch.setattr(Path, stage, inspect) + + found = specify_cli._version._find_installer(name) + assert found is not None + assert Path(found) == expected + output = capsys.readouterr().out + assert "Skipping installer PATH entry" in output + assert str(bad) in output + assert str(error) in output + + @pytest.mark.parametrize("name", ["uv", "pipx"]) + def test_non_symlink_reparse_entries_are_rejected_portably( + self, monkeypatch, tmp_path, name + ): + project = tmp_path / "project" + project.mkdir() + (project / ".git").mkdir() + reparse = tmp_path / "reparse" + reparse.mkdir() + self.executable(reparse, name) + trusted = tmp_path / "trusted" + trusted.mkdir() + expected = self.executable(trusted, name) + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", os.pathsep.join([str(reparse), str(trusted)])) + original = Path.lstat + + def lstat(path, *args, **kwargs): + result = original(path, *args, **kwargs) + if path == reparse: + return SimpleNamespace( + st_mode=result.st_mode, + st_file_attributes=0x400, + st_ino=result.st_ino, + st_dev=result.st_dev, + ) + return result + + monkeypatch.setattr(Path, "lstat", lstat) + found = specify_cli._version._find_installer(name) assert found is not None assert Path(found) == expected + @pytest.mark.skipif( + os.name != "nt", reason="Native Windows namespace paths required" + ) + @pytest.mark.parametrize("name", ["uv", "pipx"]) + @pytest.mark.parametrize("git_project", [False, True]) + @pytest.mark.parametrize("fallback", [False, True]) + def test_native_windows_extended_path_entries_are_rejected( + self, monkeypatch, tmp_path, name, git_project, fallback + ): + project = tmp_path / "project" + project.mkdir() + unsafe = project + if git_project: + (project / ".git").mkdir() + unsafe = project / "bin" + unsafe.mkdir() + self.executable(unsafe, name) + trusted = tmp_path / "trusted" + trusted.mkdir() + expected = self.executable(trusted, name) + raw = str(unsafe) + extended = ( + "\\\\?\\UNC\\" + raw[2:] if raw.startswith("\\\\") else "\\\\?\\" + raw + ) + monkeypatch.chdir(project) + monkeypatch.setenv( + "PATH", + os.pathsep.join([extended, str(trusted)] if fallback else [extended]), + ) + + found = specify_cli._version._find_installer(name) + if fallback: + assert found is not None + assert Path(found) == expected + else: + assert found is None + + @pytest.mark.skipif(os.name != "nt", reason="Native Windows junctions required") + @pytest.mark.parametrize("name", ["uv", "pipx"]) + @pytest.mark.parametrize("fallback", [False, True]) + def test_native_windows_junction_chain_is_rejected( + self, monkeypatch, tmp_path, name, fallback + ): + project = tmp_path / "project" + project.mkdir() + (project / ".git").mkdir() + external = tmp_path / "external" + external.mkdir() + self.executable(external, name) + trusted = tmp_path / "trusted" + trusted.mkdir() + expected = self.executable(trusted, name) + alias = tmp_path / "alias" + for path, target in ((project / "bin", external), (alias, project / "bin")): + result = subprocess.run( + ["cmd.exe", "/c", "mklink", "/J", str(path), str(target)], + capture_output=True, + text=True, + check=False, + ) + if result.returncode: + pytest.skip( + f"Windows junction creation unavailable: {result.stderr or result.stdout}" + ) + assert not alias.is_symlink() + monkeypatch.chdir(project) + monkeypatch.setenv( + "PATH", + os.pathsep.join([str(alias), str(trusted)] if fallback else [str(alias)]), + ) + + found = specify_cli._version._find_installer(name) + if fallback: + assert found is not None + assert Path(found) == expected + else: + assert found is None + @pytest.mark.parametrize("name", ["uv", "pipx"]) def test_relative_lookup_result_is_rejected(self, monkeypatch, tmp_path, name): project = tmp_path / "project" @@ -719,6 +936,8 @@ def test_windows_pathext_is_expanded_before_absolute_lookup( trusted = tmp_path / "trusted" trusted.mkdir() expected = str(trusted / f"{name}.EXE") + Path(expected).write_bytes(b"installer fixture; never executed\n") + Path(expected).chmod(0o755) monkeypatch.chdir(project) monkeypatch.setenv("PATH", f'"{trusted}"' if quoted_path else str(trusted)) if pathext is None: From 1951d9fb96cc17da30bcbcb1751708bcecaab478 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 29 Oct 2026 10:52:09 -0500 Subject: [PATCH 5/5] fix: validate safe external installer junctions Recognize directory mount-point reparse tags and inspect their targets instead of rejecting every Windows junction. Keep unsupported reparse types and CWD-controlled intermediate directory links excluded. Add portable and native Windows coverage for safe external installer paths, registry detection, upgrades, and unsafe junction chains. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/_version.py | 17 ++- tests/specify_cli/test_version_detection.py | 133 +++++++++++++++++--- 2 files changed, 131 insertions(+), 19 deletions(-) diff --git a/src/specify_cli/_version.py b/src/specify_cli/_version.py index 82f8b1ccb8..3f186e2f76 100644 --- a/src/specify_cli/_version.py +++ b/src/specify_cli/_version.py @@ -52,6 +52,7 @@ ) _TIER3_REGISTRY_TIMEOUT_SECS = 5 _VERIFY_TIMEOUT_SECS = 10 +_WINDOWS_MOUNT_POINT_TAG = 0xA0000003 # IO_REPARSE_TAG_MOUNT_POINT def _get_installed_version() -> str: @@ -505,17 +506,25 @@ def eligible(path: Path, *, executable: bool) -> bool: seen.add(key) entry_status = ancestor.lstat() is_link = stat.S_ISLNK(entry_status.st_mode) - if ( + is_reparse = bool( getattr(entry_status, "st_file_attributes", 0) & stat.FILE_ATTRIBUTE_REPARSE_POINT - and not is_link - ): + ) + is_junction = ( + is_reparse + and stat.S_ISDIR(entry_status.st_mode) + and getattr(entry_status, "st_reparse_tag", 0) + == _WINDOWS_MOUNT_POINT_TAG + ) + if is_reparse and not (is_link or is_junction): return False if project_status is not None and os.path.samestat( entry_status, project_status ): return False - if is_link: + if is_link or is_junction: + if os.path.samestat(ancestor.parent.stat(), cwd_status): + return False target = ancestor.readlink() pending.append( ( diff --git a/tests/specify_cli/test_version_detection.py b/tests/specify_cli/test_version_detection.py index 4d7003a27a..1cbce08897 100644 --- a/tests/specify_cli/test_version_detection.py +++ b/tests/specify_cli/test_version_detection.py @@ -3,6 +3,7 @@ import importlib.metadata import json import os +import stat import subprocess from pathlib import Path, PureWindowsPath from types import SimpleNamespace @@ -393,6 +394,19 @@ def executable(directory, name): executable.chmod(0o755) return executable + @staticmethod + def junction(path, target): + result = subprocess.run( + ["cmd.exe", "/c", "mklink", "/J", str(path), str(target)], + capture_output=True, + text=True, + check=False, + ) + if result.returncode: + pytest.skip( + f"Windows junction creation unavailable: {result.stderr or result.stdout}" + ) + @pytest.mark.parametrize( "method,name", [(_InstallMethod.UV_TOOL, "uv"), (_InstallMethod.PIPX, "pipx")], @@ -405,6 +419,7 @@ def executable(directory, name): "file", "file-aliased-directory", "cwd-file-chain", + "cwd-directory-chain", ], ) @pytest.mark.parametrize("operation", ["lookup", "registry", "upgrade"]) @@ -414,7 +429,7 @@ def test_symlink_bypasses_are_rejected( ): project = tmp_path / "project" project.mkdir() - if kind != "cwd-file-chain": + if kind not in {"cwd-file-chain", "cwd-directory-chain"}: (project / ".git").mkdir() external = tmp_path / "external" external.mkdir() @@ -422,7 +437,14 @@ def test_symlink_bypasses_are_rejected( trusted.mkdir() expected = self.executable(trusted, name) try: - if kind == "cwd-file-chain": + if kind == "cwd-directory-chain": + payload_dir = tmp_path / "payload" + payload_dir.mkdir() + self.executable(payload_dir, name) + (project / "bin").symlink_to(payload_dir, target_is_directory=True) + unsafe = external / "alias" + unsafe.symlink_to(project / "bin", target_is_directory=True) + elif kind == "cwd-file-chain": payload_dir = tmp_path / "payload" payload_dir.mkdir() payload = self.executable(payload_dir, name) @@ -615,8 +637,9 @@ def inspect(path, *args, **kwargs): assert str(error) in output @pytest.mark.parametrize("name", ["uv", "pipx"]) + @pytest.mark.parametrize("tag", [0, 0x8000001B]) def test_non_symlink_reparse_entries_are_rejected_portably( - self, monkeypatch, tmp_path, name + self, monkeypatch, tmp_path, name, tag ): project = tmp_path / "project" project.mkdir() @@ -637,6 +660,7 @@ def lstat(path, *args, **kwargs): return SimpleNamespace( st_mode=result.st_mode, st_file_attributes=0x400, + st_reparse_tag=tag, st_ino=result.st_ino, st_dev=result.st_dev, ) @@ -648,6 +672,92 @@ def lstat(path, *args, **kwargs): assert found is not None assert Path(found) == expected + @pytest.mark.parametrize("name", ["uv", "pipx"]) + @pytest.mark.parametrize("operation", ["lookup", "registry", "upgrade"]) + @pytest.mark.parametrize( + "native", + [ + False, + pytest.param( + True, + marks=pytest.mark.skipif( + os.name != "nt", reason="Native Windows junctions required" + ), + ), + ], + ) + def test_external_junction_preserves_installer_paths( + self, monkeypatch, tmp_path, name, operation, native + ): + project = tmp_path / "project" + project.mkdir() + (project / ".git").mkdir() + install = tmp_path / "install" + install.mkdir() + version = install / "version" + version.mkdir() + scripts = version / "Scripts" + scripts.mkdir() + executable = self.executable(scripts, name) + current = install / "current" + if native: + self.junction(current, version) + assert not current.is_symlink() + else: + try: + current.symlink_to(version, target_is_directory=True) + except OSError: + pytest.skip("directory symlinks are unavailable") + original = Path.lstat + + def lstat(path, *args, **kwargs): + result = original(path, *args, **kwargs) + if path == current: + return SimpleNamespace( + st_mode=stat.S_IFDIR | 0o755, + st_file_attributes=stat.FILE_ATTRIBUTE_REPARSE_POINT, + st_reparse_tag=0xA0000003, + st_ino=result.st_ino, + st_dev=result.st_dev, + ) + return result + + monkeypatch.setattr(Path, "lstat", lstat) + selected = current / "Scripts" / executable.name + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", str(selected.parent)) + monkeypatch.setattr("sys.argv", [str(project / "missing" / "specify")]) + method = _InstallMethod.UV_TOOL if name == "uv" else _InstallMethod.PIPX + stdout = ( + "specify-cli v0.7.6\n" if name == "uv" else '{"venvs":{"specify-cli":{}}}' + ) + + with ( + patch("specify_cli._version._editable_marker_seen", return_value=False), + patch("specify_cli._version._get_installed_version", return_value="0.7.5"), + patch("specify_cli._version._verify_upgrade", return_value="0.7.6"), + patch( + "specify_cli._version.subprocess.run", + return_value=_completed_process(0, stdout=stdout), + ) as run, + ): + if operation == "lookup": + argv = _assemble_installer_argv(method, "v0.7.6") + assert argv is not None + assert Path(argv[0]) == selected + run.assert_not_called() + return + if operation == "registry": + assert _detect_install_method() == method + else: + with patch( + "specify_cli._version._detect_install_method", return_value=method + ): + result = runner.invoke(app, ["self", "upgrade", "--tag", "v0.7.6"]) + assert result.exit_code == 0 + run.assert_called_once() + assert Path(run.call_args.args[0][0]) == selected + @pytest.mark.skipif( os.name != "nt", reason="Native Windows namespace paths required" ) @@ -688,12 +798,14 @@ def test_native_windows_extended_path_entries_are_rejected( @pytest.mark.skipif(os.name != "nt", reason="Native Windows junctions required") @pytest.mark.parametrize("name", ["uv", "pipx"]) @pytest.mark.parametrize("fallback", [False, True]) + @pytest.mark.parametrize("git_project", [False, True]) def test_native_windows_junction_chain_is_rejected( - self, monkeypatch, tmp_path, name, fallback + self, monkeypatch, tmp_path, name, fallback, git_project ): project = tmp_path / "project" project.mkdir() - (project / ".git").mkdir() + if git_project: + (project / ".git").mkdir() external = tmp_path / "external" external.mkdir() self.executable(external, name) @@ -702,16 +814,7 @@ def test_native_windows_junction_chain_is_rejected( expected = self.executable(trusted, name) alias = tmp_path / "alias" for path, target in ((project / "bin", external), (alias, project / "bin")): - result = subprocess.run( - ["cmd.exe", "/c", "mklink", "/J", str(path), str(target)], - capture_output=True, - text=True, - check=False, - ) - if result.returncode: - pytest.skip( - f"Windows junction creation unavailable: {result.stderr or result.stdout}" - ) + self.junction(path, target) assert not alias.is_symlink() monkeypatch.chdir(project) monkeypatch.setenv(