From 9a1124e9169c62ed53b261e40aedff985b5f341e Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 21 Sep 2026 12:23:21 +0200 Subject: [PATCH 01/17] fix(audit): skip virtualenv bin dirs when detecting installations An always-activated ~/.venv puts ~/.venv/bin first on PATH. The audit took shutil.which's answer and reported ~/.venv/bin/black (25.11.0, "via manual") as the installation. The upgrade updated the real uv tool in ~/.local/bin to 26.5.1, the re-audit found the venv copy again, and black, isort and python@3.14 stayed "outdated" run after run. Environments are not installations. reconcile.py and capability.sh already skip venv/conda bin dirs; detection.py did not. The predicate moves from reconcile.py to detection.py (reconcile imports it), and find_paths plus the multi-version lookup search PATH without venv dirs. Catalog version_commands name the tool ("black --version"), so they now run with the detected binary's dir first and venv dirs removed. Measured on this machine, 4 of 117 tools change: black, isort and python@3.14 now report the installed 26.5.1, 9.0.1 and 3.14.7 as current. flake8 exists only in ~/.venv and is now "not installed". Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- CHANGELOG.md | 1 + cli_audit/detection.py | 63 ++++++++++++++++++++++++--- cli_audit/reconcile.py | 33 +------------- tests/test_detection_venv.py | 84 ++++++++++++++++++++++++++++++++++++ 4 files changed, 144 insertions(+), 37 deletions(-) create mode 100644 tests/test_detection_venv.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 80c60e7..113672a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0. - Binary-probe fallback in `guide.sh` when the post-install snapshot refresh is stale. ### Fixed +- Audit detection skips virtualenv/conda bin dirs, like reconcile already did. An activated `~/.venv` made the audit report its own copy (`~/.venv/bin/black` 25.11.0) instead of the installation (`uv tool` black 26.5.1), so every upgrade of black, isort and python@3.14 looked like a no-op. Catalog `version_command`s now resolve the tool name to the detected binary too. A tool that exists only inside a virtualenv is now reported as not installed. - `make upgrade` hid every pinned tool, whatever the pin. A release skipped with `s` ("ask again when newer patch available") hid the tool for good. A pin now hides a tool only while it is `never`, equals the target release (`s`), equals the installed version (`p`), or equals the cycle. - `make upgrade` auto-update no longer reports an upgrade as "Updated" just because the install script exited 0. The version is compared after the re-audit, the same check the interactive `Y`/`a` answers use; an unchanged version counts as "Failed" with the old and target version. A package manager without a newer package (`bwrap` on apt) and a binary identical to the target release with a stale version string (`sd` 1.1.0 reports 1.0.0) count as "Skipped". "Held back" requires the install command to succeed and, for apt, the installed package to be the candidate and to own the binary found on PATH; otherwise the unchanged version counts as "Failed". Without an upstream version the result is reported as unverified ("Skipped"). - difftastic 0.71.0 puts the version into its release file names (`difft-0.71.0-x86_64-unknown-linux-gnu.tar.gz`); the catalog download URL now includes it. byobu is tagged `trustmux-v7.19` since the trustmux rename, and those tags fill the first page of the tags API, so the installer found no stable tag; it now accepts both tag forms. diff --git a/cli_audit/detection.py b/cli_audit/detection.py index 90a7bb8..74c67b6 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -38,6 +38,53 @@ # catalog version_command instead of a binary on disk. VERSION_COMMAND_PATH = "" +# Environment-name patterns for env managers without a pyvenv.cfg (conda etc.). +# Mirrors the venv skip list in scripts/lib/capability.sh:detect_all_installations. +_ENV_DIR_PATTERNS = ( + "/venv/bin", + "/.venv/bin", + "/env/bin", + "/venvs/", + "/.venvs/", + "/virtualenvs/", + "/.virtualenvs/", + "/envs/", + "/conda/", + "/miniconda", + "/anaconda", +) + + +def _is_virtualenv_bin(bin_dir: str) -> bool: + """True if bin_dir is a virtualenv/conda environment's bin directory. + + Environments are not installations: their binaries vanish with the env, + and classifying them by method (e.g. `uv` because the tool also appears + in `uv tool list`) makes removal delete a DIFFERENT installation. + """ + # Definitive signal: PEP 405 venvs carry pyvenv.cfg next to bin/ + if os.path.isfile(os.path.join(os.path.dirname(bin_dir), "pyvenv.cfg")): + return True + # Name-based fallback for conda/virtualenvwrapper layouts + normalized = bin_dir.rstrip("/") + "/" + return any(pat in normalized for pat in _ENV_DIR_PATTERNS) + + +def _installation_path() -> str: + """PATH without virtualenv/conda bin dirs. + + An activated environment puts its bin dir first on PATH, so a plain + lookup reports the environment's copy (e.g. ~/.venv/bin/black) and an + upgrade of the real installation never shows up in the audit. + """ + dirs = [d for d in os.environ.get("PATH", "").split(os.pathsep) if d] + return os.pathsep.join(d for d in dirs if not _is_virtualenv_bin(d)) + + +def _which(command_name: str) -> str | None: + """shutil.which restricted to installation dirs (see _installation_path).""" + return shutil.which(command_name, path=_installation_path()) + def find_paths(command_name: str, deep: bool = False) -> list[str]: """Find all paths for a command. @@ -52,7 +99,7 @@ def find_paths(command_name: str, deep: bool = False) -> list[str]: paths: list[str] = [] # Fast path: shutil.which - p = shutil.which(command_name) + p = _which(command_name) if p: paths.append(p) @@ -67,7 +114,8 @@ def find_paths(command_name: str, deep: bool = False) -> list[str]: text=True, timeout=0.2, check=False, - env={**os.environ, "TERM": "dumb"}, # Disable ANSI output + # Disable ANSI output; search installation dirs only + env={**os.environ, "TERM": "dumb", "PATH": _installation_path()}, ) for line in (proc.stdout or "").splitlines(): line = line.strip() @@ -182,6 +230,11 @@ def get_version_line( # from user input — e.g. `uv python list --only-installed | grep … | sed …`. # shell=True is required for the pipelines used in the catalog. if version_command: + # The command names the tool, not the path: resolve it to the detected + # binary first, and never to an activated environment's copy. + search_path = _installation_path() + if path: + search_path = os.pathsep.join([os.path.dirname(path), search_path]) try: proc = subprocess.run( # nosec B602 version_command, @@ -192,7 +245,7 @@ def get_version_line( text=True, timeout=TIMEOUT_SECONDS, check=False, - env={**os.environ, "TERM": "dumb"}, + env={**os.environ, "TERM": "dumb", "PATH": search_path}, ) line = (proc.stdout or "").strip() if line: @@ -523,7 +576,7 @@ def detect_multi_versions( # (used only when no version-specific binary like go1.25 is found) go_default_info = None if tool_name == "go": - default_go = shutil.which("go") + default_go = _which("go") if default_go: version_line = get_version_line(default_go, "go", version_flag="version") default_version = extract_version_number(version_line or "") @@ -555,7 +608,7 @@ def detect_multi_versions( found_path = binary_name else: # Search in PATH - path = shutil.which(binary_name) + path = _which(binary_name) if path: found_path = path diff --git a/cli_audit/reconcile.py b/cli_audit/reconcile.py index dc805ef..454666e 100644 --- a/cli_audit/reconcile.py +++ b/cli_audit/reconcile.py @@ -22,6 +22,7 @@ from .common import vlog from .config import Config +from .detection import _is_virtualenv_bin from .environment import Environment from .upgrade import compare_versions @@ -181,38 +182,6 @@ def summary(self) -> str: """ -# Environment-name patterns for env managers without a pyvenv.cfg (conda etc.). -# Mirrors the venv skip list in scripts/lib/capability.sh:detect_all_installations. -_ENV_DIR_PATTERNS = ( - "/venv/bin", - "/.venv/bin", - "/env/bin", - "/venvs/", - "/.venvs/", - "/virtualenvs/", - "/.virtualenvs/", - "/envs/", - "/conda/", - "/miniconda", - "/anaconda", -) - - -def _is_virtualenv_bin(bin_dir: str) -> bool: - """True if bin_dir is a virtualenv/conda environment's bin directory. - - Environments are not installations: their binaries vanish with the env, - and classifying them by method (e.g. `uv` because the tool also appears - in `uv tool list`) makes removal delete a DIFFERENT installation. - """ - # Definitive signal: PEP 405 venvs carry pyvenv.cfg next to bin/ - if os.path.isfile(os.path.join(os.path.dirname(bin_dir), "pyvenv.cfg")): - return True - # Name-based fallback for conda/virtualenvwrapper layouts - normalized = bin_dir.rstrip("/") + "/" - return any(pat in normalized for pat in _ENV_DIR_PATTERNS) - - def detect_installations( tool_name: str, candidates: Sequence[str] | None = None, diff --git a/tests/test_detection_venv.py b/tests/test_detection_venv.py new file mode 100644 index 0000000..c8088d3 --- /dev/null +++ b/tests/test_detection_venv.py @@ -0,0 +1,84 @@ +"""Tests for virtualenv exclusion in audit detection. + +An always-activated ~/.venv put ~/.venv/bin first on PATH. The audit reported +~/.venv/bin/black (25.11.0, "via manual") as the installation, so every +`uv tool upgrade black` of the real ~/.local/bin/black (26.5.1) looked like a +no-op and the tool stayed "outdated" run after run. Environments are not +installations: reconcile.py and capability.sh already skip them, and the +audit detection must too. +""" + +from __future__ import annotations + +import os +import sys +from pathlib import Path + +import pytest + +from cli_audit.detection import audit_tool_installation, find_paths + +pytestmark = pytest.mark.skipif( + sys.platform == "win32", + reason="Uses Unix-style paths and PATH separator (:)", +) + + +def _make_bin(bin_dir: Path, name: str, version: str) -> Path: + bin_dir.mkdir(parents=True, exist_ok=True) + binary = bin_dir / name + binary.write_text(f"#!/bin/sh\necho '{name} {version}'\n") + binary.chmod(0o755) + return binary + + +def _make_venv(root: Path) -> Path: + root.mkdir(parents=True, exist_ok=True) + (root / "pyvenv.cfg").write_text("home = /usr/bin\n") + return root / "bin" + + +def test_activated_venv_does_not_shadow_the_installation(tmp_path, monkeypatch): + venv_bin = _make_venv(tmp_path / "home" / ".venv") + _make_bin(venv_bin, "fakeblack", "25.11.0") + local_bin = tmp_path / "home" / ".local" / "bin" + real = _make_bin(local_bin, "fakeblack", "26.5.1") + monkeypatch.setenv("PATH", os.pathsep.join([str(venv_bin), str(local_bin)])) + + version, _line, path, _method = audit_tool_installation("fakeblack", ("fakeblack",)) + + assert (version, path) == ("26.5.1", str(real)) + + +def test_deep_search_skips_venv_bin(tmp_path, monkeypatch): + venv_bin = _make_venv(tmp_path / "env-with-any-name") + _make_bin(venv_bin, "faketool", "1.0.0") + other_bin = tmp_path / "other" / "bin" + real = _make_bin(other_bin, "faketool", "2.0.0") + monkeypatch.setenv("PATH", os.pathsep.join([str(venv_bin), str(other_bin)])) + + assert find_paths("faketool", deep=True) == [str(real)] + + +def test_tool_only_in_venv_is_not_installed(tmp_path, monkeypatch): + venv_bin = _make_venv(tmp_path / ".venv") + _make_bin(venv_bin, "fakeonlyvenv", "7.3.0") + monkeypatch.setenv("PATH", str(venv_bin)) + + assert find_paths("fakeonlyvenv", deep=True) == [] + + +def test_version_command_runs_the_detected_binary(tmp_path, monkeypatch): + # Catalog version_command names the tool ("black --version"); it must not + # resolve to the activated venv's copy either + venv_bin = _make_venv(tmp_path / ".venv") + _make_bin(venv_bin, "fakeblack2", "25.11.0") + local_bin = tmp_path / ".local" / "bin" + real = _make_bin(local_bin, "fakeblack2", "26.5.1") + monkeypatch.setenv("PATH", os.pathsep.join([str(venv_bin), str(local_bin)])) + + version, _line, path, _method = audit_tool_installation( + "fakeblack2", ("fakeblack2",), version_command="fakeblack2 --version" + ) + + assert (version, path) == ("26.5.1", str(real)) From e72a8d7a8ecfdeb55352d3114b7d88bda5241613 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 21 Sep 2026 12:33:15 +0200 Subject: [PATCH 02/17] fix(audit): drop the detected-path prefix from the version_command PATH SonarCloud flagged S6547 (environment variable built from a value it treats as untrusted): the PATH for a catalog version_command started with the directory of the detected binary. The prefix is not needed. Without venv dirs, the tool name resolves to the same binary that find_paths selected, which is what the version_command test asserts. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- cli_audit/detection.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cli_audit/detection.py b/cli_audit/detection.py index 74c67b6..3319d9f 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -230,11 +230,9 @@ def get_version_line( # from user input — e.g. `uv python list --only-installed | grep … | sed …`. # shell=True is required for the pipelines used in the catalog. if version_command: - # The command names the tool, not the path: resolve it to the detected - # binary first, and never to an activated environment's copy. + # The command names the tool, not the path: resolve that name the way + # find_paths does, never to an activated environment's copy. search_path = _installation_path() - if path: - search_path = os.pathsep.join([os.path.dirname(path), search_path]) try: proc = subprocess.run( # nosec B602 version_command, From 4d9e3815674fb03e509cad71690c5179c0bf630f Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 21 Sep 2026 12:34:23 +0200 Subject: [PATCH 03/17] fix(audit): require a directory boundary after venv/bin patterns "/venv/bin", "/.venv/bin" and "/env/bin" were matched as substrings of the normalized PATH entry, so /opt/venv/bin-extra/ counted as an environment and was skipped. The patterns now end with "/"; the normalized entry always ends with "/", so real environment bin dirs still match. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- cli_audit/detection.py | 6 +++--- tests/test_detection_venv.py | 9 +++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/cli_audit/detection.py b/cli_audit/detection.py index 3319d9f..f1ba5d3 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -41,9 +41,9 @@ # Environment-name patterns for env managers without a pyvenv.cfg (conda etc.). # Mirrors the venv skip list in scripts/lib/capability.sh:detect_all_installations. _ENV_DIR_PATTERNS = ( - "/venv/bin", - "/.venv/bin", - "/env/bin", + "/venv/bin/", + "/.venv/bin/", + "/env/bin/", "/venvs/", "/.venvs/", "/virtualenvs/", diff --git a/tests/test_detection_venv.py b/tests/test_detection_venv.py index c8088d3..f7345ac 100644 --- a/tests/test_detection_venv.py +++ b/tests/test_detection_venv.py @@ -82,3 +82,12 @@ def test_version_command_runs_the_detected_binary(tmp_path, monkeypatch): ) assert (version, path) == ("26.5.1", str(real)) + + +def test_bin_pattern_needs_a_directory_boundary(tmp_path, monkeypatch): + # "/venv/bin" must not match /opt/venv/bin-extra: that is no environment + extra_bin = tmp_path / "opt" / "venv" / "bin-extra" + real = _make_bin(extra_bin, "faketool3", "3.0.0") + monkeypatch.setenv("PATH", str(extra_bin)) + + assert find_paths("faketool3") == [str(real)] From 3d70af95a077b1550cc906cc45c702fe873c4270 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 21 Sep 2026 12:56:15 +0200 Subject: [PATCH 04/17] fix(audit): use the venv-free lookup for install checks too Review findings on the venv exclusion: - The deep-search tests set PATH to temp dirs only, so `which -a` itself was not found, the error was swallowed, and only the fast path ran. The tests now keep the directory of `which` on PATH; reverting the filtered PATH of the deep search makes two of them fail. - bulk.get_missing_tools and installer.validate_installation still used the plain PATH. A venv-only tool counted as installed for bulk installs while the audit reported it missing, and the post-install validation ran ` --version` by name, i.e. the venv copy. Both now search the same PATH as the audit, and validation runs the binary it found. - With PATH unset, the filtered lookup returned no match instead of falling back to os.defpath like shutil.which does. - CHANGELOG names conda environments, which the same patterns exclude. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- CHANGELOG.md | 2 +- cli_audit/bulk.py | 74 +++++++++++++++++++++--------------- cli_audit/detection.py | 2 +- cli_audit/installer.py | 54 ++++++++++++++++---------- tests/test_bulk.py | 45 ++++++++++++---------- tests/test_detection_venv.py | 34 ++++++++++++++++- 6 files changed, 136 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 113672a..dede940 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0. - Binary-probe fallback in `guide.sh` when the post-install snapshot refresh is stale. ### Fixed -- Audit detection skips virtualenv/conda bin dirs, like reconcile already did. An activated `~/.venv` made the audit report its own copy (`~/.venv/bin/black` 25.11.0) instead of the installation (`uv tool` black 26.5.1), so every upgrade of black, isort and python@3.14 looked like a no-op. Catalog `version_command`s now resolve the tool name to the detected binary too. A tool that exists only inside a virtualenv is now reported as not installed. +- Audit detection skips virtualenv/conda bin dirs, like reconcile already did. An activated `~/.venv` made the audit report its own copy (`~/.venv/bin/black` 25.11.0) instead of the installation (`uv tool` black 26.5.1), so every upgrade of black, isort and python@3.14 looked like a no-op. Catalog `version_command`s now resolve the tool name to the detected binary too. A tool that exists only inside a virtualenv or conda environment is now reported as not installed; the bulk missing-tool check and the post-install validation use the same lookup. - `make upgrade` hid every pinned tool, whatever the pin. A release skipped with `s` ("ask again when newer patch available") hid the tool for good. A pin now hides a tool only while it is `never`, equals the target release (`s`), equals the installed version (`p`), or equals the cycle. - `make upgrade` auto-update no longer reports an upgrade as "Updated" just because the install script exited 0. The version is compared after the re-audit, the same check the interactive `Y`/`a` answers use; an unchanged version counts as "Failed" with the old and target version. A package manager without a newer package (`bwrap` on apt) and a binary identical to the target release with a stale version string (`sd` 1.1.0 reports 1.0.0) count as "Skipped". "Held back" requires the install command to succeed and, for apt, the installed package to be the candidate and to own the binary found on PATH; otherwise the unchanged version counts as "Failed". Without an upstream version the result is reported as unverified ("Skipped"). - difftastic 0.71.0 puts the version into its release file names (`difft-0.71.0-x86_64-unknown-linux-gnu.tar.gz`); the catalog download URL now includes it. byobu is tagged `trustmux-v7.19` since the trustmux rename, and those tags fill the first page of the tags API, so the installer found no stable tag; it now accepts both tag forms. diff --git a/cli_audit/bulk.py b/cli_audit/bulk.py index 25488b2..7734851 100644 --- a/cli_audit/bulk.py +++ b/cli_audit/bulk.py @@ -21,6 +21,7 @@ from .common import vlog from .config import Config +from .detection import _installation_path from .environment import Environment from .installer import InstallResult, install_tool from .package_managers import select_package_manager @@ -38,6 +39,7 @@ class ToolSpec: language: Tool language/ecosystem (e.g., "python", "rust") dependencies: Tool names that must be installed first """ + tool_name: str package_name: str target_version: str = "latest" @@ -65,6 +67,7 @@ class ProgressTracker: _progress: Progress state for each tool _callbacks: Callbacks to invoke on progress updates """ + _lock: threading.Lock = field(default_factory=threading.Lock) _progress: dict[str, dict] = field(default_factory=dict) _callbacks: list[Callable[[str, str, str], None]] = field(default_factory=list) @@ -132,6 +135,7 @@ class BulkInstallResult: duration_seconds: Total execution time rollback_script: Path to generated rollback script (if any) """ + tools_attempted: tuple[str, ...] successes: tuple[InstallResult, ...] failures: tuple[InstallResult, ...] @@ -164,7 +168,8 @@ def get_missing_tools(tool_names: Sequence[str], verbose: bool = False) -> list[ """ missing = [] for tool_name in tool_names: - binary_path = shutil.which(tool_name) + # Same lookup as the audit: a copy inside an activated venv is no installation + binary_path = shutil.which(tool_name, path=_installation_path()) if not binary_path: missing.append(tool_name) vlog(f"Tool not found: {tool_name}", verbose) @@ -252,26 +257,30 @@ def get_tools_to_install( return [] for name in tool_names: tool_config = config.get_tool_config(name) - specs.append(ToolSpec( - tool_name=name, - package_name=name, - target_version=tool_config.version if tool_config else "latest", - language=None, - dependencies=(), - )) + specs.append( + ToolSpec( + tool_name=name, + package_name=name, + target_version=tool_config.version if tool_config else "latest", + language=None, + dependencies=(), + ) + ) elif mode == "missing": all_tools = list(config.tools.keys()) missing = get_missing_tools(all_tools, verbose) for name in missing: tool_config = config.get_tool_config(name) - specs.append(ToolSpec( - tool_name=name, - package_name=name, - target_version=tool_config.version if tool_config else "latest", - language=None, - dependencies=(), - )) + specs.append( + ToolSpec( + tool_name=name, + package_name=name, + target_version=tool_config.version if tool_config else "latest", + language=None, + dependencies=(), + ) + ) elif mode == "preset": if not preset_name or not hasattr(config, "presets"): @@ -280,25 +289,29 @@ def get_tools_to_install( preset_tools = getattr(config.presets, preset_name, []) for name in preset_tools: tool_config = config.get_tool_config(name) - specs.append(ToolSpec( - tool_name=name, - package_name=name, - target_version=tool_config.version if tool_config else "latest", - language=None, - dependencies=(), - )) + specs.append( + ToolSpec( + tool_name=name, + package_name=name, + target_version=tool_config.version if tool_config else "latest", + language=None, + dependencies=(), + ) + ) elif mode == "all": all_tools = list(config.tools.keys()) for name in all_tools: tool_config = config.get_tool_config(name) - specs.append(ToolSpec( - tool_name=name, - package_name=name, - target_version=tool_config.version if tool_config else "latest", - language=None, - dependencies=(), - )) + specs.append( + ToolSpec( + tool_name=name, + package_name=name, + target_version=tool_config.version if tool_config else "latest", + language=None, + dependencies=(), + ) + ) vlog(f"Mode '{mode}' resolved to {len(specs)} tools", verbose) return specs @@ -502,6 +515,7 @@ def bulk_install( # Determine max workers if max_workers is None: import os + max_workers = min(16, os.cpu_count() or 4 + 4) # Execute installations level by level @@ -551,7 +565,7 @@ def bulk_install( # Stop if fail-fast triggered if fail_fast and failures: # Mark remaining tools as skipped - for level in levels[level_idx + 1:]: + for level in levels[level_idx + 1 :]: for spec in level: skipped.append(spec.tool_name) progress_tracker.update(spec.tool_name, "skipped", "Skipped due to fail-fast") diff --git a/cli_audit/detection.py b/cli_audit/detection.py index f1ba5d3..9b642e5 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -77,7 +77,7 @@ def _installation_path() -> str: lookup reports the environment's copy (e.g. ~/.venv/bin/black) and an upgrade of the real installation never shows up in the audit. """ - dirs = [d for d in os.environ.get("PATH", "").split(os.pathsep) if d] + dirs = [d for d in os.environ.get("PATH", os.defpath).split(os.pathsep) if d] return os.pathsep.join(d for d in dirs if not _is_virtualenv_bin(d)) diff --git a/cli_audit/installer.py b/cli_audit/installer.py index 8f2fc99..a8be466 100644 --- a/cli_audit/installer.py +++ b/cli_audit/installer.py @@ -16,6 +16,7 @@ from .common import vlog from .config import Config +from .detection import _installation_path from .environment import Environment from .install_plan import InstallStep, generate_install_plan from .package_managers import select_package_manager @@ -37,6 +38,7 @@ class StepResult: error_message: Human-readable error message if failed attempt_number: Which retry attempt this was (1-indexed) """ + step: InstallStep success: bool stdout: str @@ -77,6 +79,7 @@ class InstallResult: error_message: Human-readable error message if failed binary_path: Path to installed binary (if validation passed) """ + tool_name: str success: bool installed_version: str | None @@ -113,6 +116,7 @@ class InstallError(Exception): retryable: Whether this error can be retried remediation: Suggested fix for the error """ + def __init__( self, message: str, @@ -137,23 +141,29 @@ def is_retryable_error(exit_code: int, stderr: str) -> bool: True if error is transient and should be retried """ # Network-related errors - if any(indicator in stderr.lower() for indicator in [ - "connection refused", - "connection timed out", - "connection reset", - "temporary failure", - "network unreachable", - "could not resolve host", - ]): + if any( + indicator in stderr.lower() + for indicator in [ + "connection refused", + "connection timed out", + "connection reset", + "temporary failure", + "network unreachable", + "could not resolve host", + ] + ): return True # Package manager lock contention - if any(indicator in stderr.lower() for indicator in [ - "could not get lock", - "lock file exists", - "waiting for cache lock", - "dpkg frontend lock", - ]): + if any( + indicator in stderr.lower() + for indicator in [ + "could not get lock", + "lock file exists", + "waiting for cache lock", + "dpkg frontend lock", + ] + ): return True # Temporary failure exit codes @@ -176,7 +186,7 @@ def calculate_backoff_delay(attempt: int, base_delay: float = 1.0, max_delay: fl Delay in seconds with jitter applied """ # Exponential backoff: base * 2^attempt - delay = base_delay * (2 ** attempt) + delay = base_delay * (2**attempt) delay = min(delay, max_delay) # Add jitter (±20%) @@ -371,8 +381,9 @@ def validate_installation( Returns: Tuple of (success, binary_path, actual_version) """ - # Check if binary exists in PATH - binary_path = shutil.which(tool_name) + # Check if binary exists in PATH. Same lookup as the audit: a copy inside + # an activated venv would shadow the tool that was just installed. + binary_path = shutil.which(tool_name, path=_installation_path()) if not binary_path: vlog(f"Binary not found in PATH: {tool_name}", verbose) return (False, None, None) @@ -381,9 +392,9 @@ def validate_installation( # Try to get version version_commands = [ - (tool_name, "--version"), - (tool_name, "-V"), - (tool_name, "version"), + (binary_path, "--version"), + (binary_path, "-V"), + (binary_path, "version"), ] actual_version = None @@ -399,7 +410,8 @@ def validate_installation( if result.returncode == 0 and result.stdout: # Extract version from output (first line, first version-like pattern) import re - version_pattern = r'\d+\.\d+(?:\.\d+)?(?:-[\w.]+)?' + + version_pattern = r"\d+\.\d+(?:\.\d+)?(?:-[\w.]+)?" match = re.search(version_pattern, result.stdout) if match: actual_version = match.group(0) diff --git a/tests/test_bulk.py b/tests/test_bulk.py index 31dca63..ccca00d 100644 --- a/tests/test_bulk.py +++ b/tests/test_bulk.py @@ -31,10 +31,7 @@ from cli_audit.installer import InstallResult, StepResult # Skip marker for Windows (rollback scripts are Unix shell scripts) -skip_on_windows = pytest.mark.skipif( - sys.platform == "win32", - reason="Rollback scripts use Unix shell syntax" -) +skip_on_windows = pytest.mark.skipif(sys.platform == "win32", reason="Rollback scripts use Unix shell syntax") class TestToolSpec: @@ -250,7 +247,8 @@ def test_get_missing_tools_all_installed(self, mock_which): @patch("cli_audit.bulk.shutil.which") def test_get_missing_tools_mixed(self, mock_which): """Test when some tools are installed.""" - def which_side_effect(tool): + + def which_side_effect(tool, path=None): if tool in ("ripgrep", "mypy"): return "/usr/bin/" + tool return None @@ -377,11 +375,13 @@ def test_get_tools_missing_mode(self, mock_get_missing): """Test missing mode.""" mock_get_missing.return_value = ["ripgrep", "black"] - config = Config(tools={ - "ripgrep": ToolConfig(version="14.1.1"), - "black": ToolConfig(version="24.10.0"), - "mypy": ToolConfig(version="1.8.0"), # not missing - }) + config = Config( + tools={ + "ripgrep": ToolConfig(version="14.1.1"), + "black": ToolConfig(version="24.10.0"), + "mypy": ToolConfig(version="1.8.0"), # not missing + } + ) specs = get_tools_to_install( mode="missing", @@ -396,11 +396,13 @@ def test_get_tools_missing_mode(self, mock_get_missing): def test_get_tools_all_mode(self): """Test all mode.""" - config = Config(tools={ - "ripgrep": ToolConfig(version="14.1.1"), - "black": ToolConfig(version="24.10.0"), - "mypy": ToolConfig(version="1.8.0"), - }) + config = Config( + tools={ + "ripgrep": ToolConfig(version="14.1.1"), + "black": ToolConfig(version="24.10.0"), + "mypy": ToolConfig(version="1.8.0"), + } + ) specs = get_tools_to_install( mode="all", @@ -437,6 +439,7 @@ def test_group_by_package_manager_single(self, mock_select): @patch("cli_audit.bulk.select_package_manager") def test_group_by_package_manager_multiple(self, mock_select): """Test grouping with multiple package managers.""" + def select_side_effect(tool_name, language, config, env, verbose=False): if language == "rust": return ("cargo", "hierarchy") @@ -777,11 +780,13 @@ def install_side_effect(*args, **kwargs): # Create tools with dependency chain to ensure multi-level execution # tool1 → tool2 → tool3 - config = Config(tools={ - "tool1": ToolConfig(), - "tool2": ToolConfig(), - "tool3": ToolConfig(), - }) + config = Config( + tools={ + "tool1": ToolConfig(), + "tool2": ToolConfig(), + "tool3": ToolConfig(), + } + ) env = Environment(mode="workstation", confidence=1.0) # We need to test the actual bulk_install, but with dependency resolution diff --git a/tests/test_detection_venv.py b/tests/test_detection_venv.py index f7345ac..6d61b28 100644 --- a/tests/test_detection_venv.py +++ b/tests/test_detection_venv.py @@ -11,12 +11,15 @@ from __future__ import annotations import os +import shutil import sys from pathlib import Path import pytest +from cli_audit.bulk import get_missing_tools from cli_audit.detection import audit_tool_installation, find_paths +from cli_audit.installer import validate_installation pytestmark = pytest.mark.skipif( sys.platform == "win32", @@ -24,6 +27,11 @@ ) +# The deep search runs `which -a`; without its dir on PATH the subprocess +# fails, the error is swallowed and only the fast path would be tested +WHICH_DIR = os.path.dirname(shutil.which("which") or "/usr/bin/which") + + def _make_bin(bin_dir: Path, name: str, version: str) -> Path: bin_dir.mkdir(parents=True, exist_ok=True) binary = bin_dir / name @@ -55,7 +63,7 @@ def test_deep_search_skips_venv_bin(tmp_path, monkeypatch): _make_bin(venv_bin, "faketool", "1.0.0") other_bin = tmp_path / "other" / "bin" real = _make_bin(other_bin, "faketool", "2.0.0") - monkeypatch.setenv("PATH", os.pathsep.join([str(venv_bin), str(other_bin)])) + monkeypatch.setenv("PATH", os.pathsep.join([str(venv_bin), str(other_bin), WHICH_DIR])) assert find_paths("faketool", deep=True) == [str(real)] @@ -63,7 +71,7 @@ def test_deep_search_skips_venv_bin(tmp_path, monkeypatch): def test_tool_only_in_venv_is_not_installed(tmp_path, monkeypatch): venv_bin = _make_venv(tmp_path / ".venv") _make_bin(venv_bin, "fakeonlyvenv", "7.3.0") - monkeypatch.setenv("PATH", str(venv_bin)) + monkeypatch.setenv("PATH", os.pathsep.join([str(venv_bin), WHICH_DIR])) assert find_paths("fakeonlyvenv", deep=True) == [] @@ -91,3 +99,25 @@ def test_bin_pattern_needs_a_directory_boundary(tmp_path, monkeypatch): monkeypatch.setenv("PATH", str(extra_bin)) assert find_paths("faketool3") == [str(real)] + + +def test_missing_tool_check_ignores_venv_copy(tmp_path, monkeypatch): + # bulk install must agree with the audit: a venv-only tool is missing + venv_bin = _make_venv(tmp_path / ".venv") + _make_bin(venv_bin, "fakeonlyvenv2", "7.3.0") + monkeypatch.setenv("PATH", str(venv_bin)) + + assert get_missing_tools(["fakeonlyvenv2"]) == ["fakeonlyvenv2"] + + +def test_install_validation_checks_the_installed_copy(tmp_path, monkeypatch): + venv_bin = _make_venv(tmp_path / ".venv") + _make_bin(venv_bin, "fakeblack3", "25.11.0") + local_bin = tmp_path / ".local" / "bin" + real = _make_bin(local_bin, "fakeblack3", "26.5.1") + monkeypatch.setenv("PATH", os.pathsep.join([str(venv_bin), str(local_bin)])) + + ok, path, version = validate_installation("fakeblack3") + + assert (ok, path) == (True, str(real)) + assert "26.5.1" in (version or "") From e8937d63ac57438096683f2670a8e0be50fc3442 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 21 Sep 2026 13:11:37 +0200 Subject: [PATCH 05/17] fix(reconcile): keep uv-tool and pipx installs, resolve the active copy like the audit Review round 2: - reconcile skipped every binary whose real path lies in a directory with a pyvenv.cfg next to bin/. uv tool and pipx install each tool into such a venv, so detect_installations("black") returned [] for the uv tool black. Per-tool venvs under uv/tools/ and pipx/venvs/ count as installations again. - reconcile marked the active copy with a plain shutil.which, so with a venv active no real installation was active. It uses the filtered PATH now. - _is_virtualenv_bin missed "~/env/bin/" (trailing slash): dirname stayed inside bin/. The path is normalized first. - CHANGELOG: version_commands run with the filtered PATH; the earlier sentence described the removed prefix mechanism. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- CHANGELOG.md | 2 +- cli_audit/detection.py | 14 ++++++++++++++ cli_audit/reconcile.py | 8 +++++--- tests/test_detection_venv.py | 35 ++++++++++++++++++++++++++++++++++- 4 files changed, 54 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dede940..bda80c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0. - Binary-probe fallback in `guide.sh` when the post-install snapshot refresh is stale. ### Fixed -- Audit detection skips virtualenv/conda bin dirs, like reconcile already did. An activated `~/.venv` made the audit report its own copy (`~/.venv/bin/black` 25.11.0) instead of the installation (`uv tool` black 26.5.1), so every upgrade of black, isort and python@3.14 looked like a no-op. Catalog `version_command`s now resolve the tool name to the detected binary too. A tool that exists only inside a virtualenv or conda environment is now reported as not installed; the bulk missing-tool check and the post-install validation use the same lookup. +- Audit detection skips virtualenv/conda bin dirs, like reconcile already did. An activated `~/.venv` made the audit report its own copy (`~/.venv/bin/black` 25.11.0) instead of the installation (`uv tool` black 26.5.1), so every upgrade of black, isort and python@3.14 looked like a no-op. Catalog `version_command`s run with the same filtered PATH. Reconcile no longer drops uv-tool and pipx installations, whose per-tool directories also carry a `pyvenv.cfg`. A tool that exists only inside a virtualenv or conda environment is now reported as not installed; the bulk missing-tool check and the post-install validation use the same lookup. - `make upgrade` hid every pinned tool, whatever the pin. A release skipped with `s` ("ask again when newer patch available") hid the tool for good. A pin now hides a tool only while it is `never`, equals the target release (`s`), equals the installed version (`p`), or equals the cycle. - `make upgrade` auto-update no longer reports an upgrade as "Updated" just because the install script exited 0. The version is compared after the re-audit, the same check the interactive `Y`/`a` answers use; an unchanged version counts as "Failed" with the old and target version. A package manager without a newer package (`bwrap` on apt) and a binary identical to the target release with a stale version string (`sd` 1.1.0 reports 1.0.0) count as "Skipped". "Held back" requires the install command to succeed and, for apt, the installed package to be the candidate and to own the binary found on PATH; otherwise the unchanged version counts as "Failed". Without an upstream version the result is reported as unverified ("Skipped"). - difftastic 0.71.0 puts the version into its release file names (`difft-0.71.0-x86_64-unknown-linux-gnu.tar.gz`); the catalog download URL now includes it. byobu is tagged `trustmux-v7.19` since the trustmux rename, and those tags fill the first page of the tags API, so the installer found no stable tag; it now accepts both tag forms. diff --git a/cli_audit/detection.py b/cli_audit/detection.py index 9b642e5..079524a 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -62,6 +62,8 @@ def _is_virtualenv_bin(bin_dir: str) -> bool: and classifying them by method (e.g. `uv` because the tool also appears in `uv tool list`) makes removal delete a DIFFERENT installation. """ + # "/x/env/bin/" must behave like "/x/env/bin" (dirname would stay in bin/) + bin_dir = os.path.normpath(bin_dir) # Definitive signal: PEP 405 venvs carry pyvenv.cfg next to bin/ if os.path.isfile(os.path.join(os.path.dirname(bin_dir), "pyvenv.cfg")): return True @@ -70,6 +72,18 @@ def _is_virtualenv_bin(bin_dir: str) -> bool: return any(pat in normalized for pat in _ENV_DIR_PATTERNS) +# Tool managers install each tool into a venv of its own; a binary linked +# from there (~/.local/bin/black -> ~/.local/share/uv/tools/black/bin/black) +# is an installation, not an environment. +_TOOL_ENV_ROOTS = ("/uv/tools/", "/pipx/venvs/") + + +def _is_tool_manager_env(bin_dir: str) -> bool: + """True if bin_dir belongs to a uv-tool or pipx per-tool venv.""" + normalized = os.path.normpath(bin_dir) + "/" + return any(root in normalized for root in _TOOL_ENV_ROOTS) + + def _installation_path() -> str: """PATH without virtualenv/conda bin dirs. diff --git a/cli_audit/reconcile.py b/cli_audit/reconcile.py index 454666e..712f2e4 100644 --- a/cli_audit/reconcile.py +++ b/cli_audit/reconcile.py @@ -22,7 +22,7 @@ from .common import vlog from .config import Config -from .detection import _is_virtualenv_bin +from .detection import _installation_path, _is_tool_manager_env, _is_virtualenv_bin from .environment import Environment from .upgrade import compare_versions @@ -246,7 +246,8 @@ def detect_installations( continue # A symlink can point into an environment as well - if _is_virtualenv_bin(os.path.dirname(real_path)): + real_dir = os.path.dirname(real_path) + if _is_virtualenv_bin(real_dir) and not _is_tool_manager_env(real_dir): vlog(f" Skipping environment binary: {real_path}", verbose) continue @@ -279,7 +280,8 @@ def detect_installations( method = classify_install_method(real_path, tool_name, verbose) # Check if this is the active installation - active_path = shutil.which(candidate) + # The active copy is resolved the same way the audit resolves it + active_path = shutil.which(candidate, path=_installation_path()) is_active = (os.path.realpath(active_path) == real_path) if active_path else False installations.append( diff --git a/tests/test_detection_venv.py b/tests/test_detection_venv.py index 6d61b28..a03740c 100644 --- a/tests/test_detection_venv.py +++ b/tests/test_detection_venv.py @@ -76,7 +76,7 @@ def test_tool_only_in_venv_is_not_installed(tmp_path, monkeypatch): assert find_paths("fakeonlyvenv", deep=True) == [] -def test_version_command_runs_the_detected_binary(tmp_path, monkeypatch): +def test_version_command_skips_the_venv_copy(tmp_path, monkeypatch): # Catalog version_command names the tool ("black --version"); it must not # resolve to the activated venv's copy either venv_bin = _make_venv(tmp_path / ".venv") @@ -121,3 +121,36 @@ def test_install_validation_checks_the_installed_copy(tmp_path, monkeypatch): assert (ok, path) == (True, str(real)) assert "26.5.1" in (version or "") + + +def test_trailing_slash_venv_bin_is_skipped(tmp_path, monkeypatch): + # export PATH=~/proj-env/bin/:$PATH + venv_bin = _make_venv(tmp_path / "proj-env") + _make_bin(venv_bin, "faketool4", "1.0.0") + other_bin = tmp_path / "other" / "bin" + real = _make_bin(other_bin, "faketool4", "2.0.0") + monkeypatch.setenv("PATH", os.pathsep.join([str(venv_bin) + "/", str(other_bin)])) + + assert find_paths("faketool4") == [str(real)] + + +def test_reconcile_keeps_uv_tool_installation(tmp_path, monkeypatch): + # ~/.local/bin/black -> ~/.local/share/uv/tools/black/bin/black; the tool + # dir carries a pyvenv.cfg but the tool is installed, not an environment + from cli_audit.reconcile import clear_detection_cache, detect_installations + + tool_env = tmp_path / "share" / "uv" / "tools" / "fakeuvtool" + real = _make_bin(_make_venv(tool_env), "fakeuvtool", "26.5.1") + local_bin = tmp_path / "local" / "bin" + local_bin.mkdir(parents=True) + (local_bin / "fakeuvtool").symlink_to(real) + # an activated venv with its own copy sits in front + venv_bin = _make_venv(tmp_path / ".venv") + _make_bin(venv_bin, "fakeuvtool", "25.11.0") + monkeypatch.setenv("PATH", os.pathsep.join([str(venv_bin), str(local_bin)])) + clear_detection_cache() + + installs = detect_installations("fakeuvtool", ["fakeuvtool"]) + + assert [i.path for i in installs] == [str(real)] + assert installs[0].active From c8fd0f7ca2e8276c34714d026b557d1c9c000e44 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 21 Sep 2026 13:20:43 +0200 Subject: [PATCH 06/17] fix(reconcile): classify uv-tool and pipx installs by their directory Review round 3: - With uv-tool and pipx installs now detected, classify_install_method ran its package-manager queries first. Those match the tool name as a substring of `cargo install --list`, `pipx list` or `uv tool list`, so a uv tool could be labelled pipx, and removal would run the wrong uninstaller. A binary inside a uv-tool or pipx per-tool venv is now classified by that directory before any query. - Relocated tool dirs (UV_TOOL_DIR, PIPX_HOME, PIPX_GLOBAL_HOME) are recognised, not only the default ~/.local/share/uv/tools and .../pipx/venvs. - "Preferred installation is not active" advised putting the manager's internal bin dir (~/.local/share/uv/tools/black/bin) first in PATH. It now names the PATH dir the binary was found in (~/.local/bin). - The reconcile test covers uv, pipx and a relocated uv tool dir. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- cli_audit/detection.py | 25 +++++++++++++++++++++---- cli_audit/reconcile.py | 14 ++++++++++++-- tests/test_detection_venv.py | 24 +++++++++++++++--------- 3 files changed, 48 insertions(+), 15 deletions(-) diff --git a/cli_audit/detection.py b/cli_audit/detection.py index 079524a..b9fef2b 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -74,14 +74,31 @@ def _is_virtualenv_bin(bin_dir: str) -> bool: # Tool managers install each tool into a venv of its own; a binary linked # from there (~/.local/bin/black -> ~/.local/share/uv/tools/black/bin/black) -# is an installation, not an environment. -_TOOL_ENV_ROOTS = ("/uv/tools/", "/pipx/venvs/") +# is an installation, not an environment. Default locations, matched as path +# fragments; relocated ones come from the managers' own variables. +_TOOL_ENV_ROOTS = (("uv", "/uv/tools/"), ("pipx", "/pipx/venvs/")) + + +def tool_manager_of(bin_dir: str) -> str: + """Return "uv" or "pipx" if bin_dir belongs to that manager's per-tool venv, else "".""" + normalized = os.path.normpath(bin_dir) + "/" + for manager, fragment in _TOOL_ENV_ROOTS: + if fragment in normalized: + return manager + relocated = ( + ("uv", os.environ.get("UV_TOOL_DIR", "")), + ("pipx", os.path.join(os.environ["PIPX_HOME"], "venvs") if os.environ.get("PIPX_HOME") else ""), + ("pipx", os.path.join(os.environ["PIPX_GLOBAL_HOME"], "venvs") if os.environ.get("PIPX_GLOBAL_HOME") else ""), + ) + for manager, root in relocated: + if root and normalized.startswith(os.path.normpath(root) + "/"): + return manager + return "" def _is_tool_manager_env(bin_dir: str) -> bool: """True if bin_dir belongs to a uv-tool or pipx per-tool venv.""" - normalized = os.path.normpath(bin_dir) + "/" - return any(root in normalized for root in _TOOL_ENV_ROOTS) + return bool(tool_manager_of(bin_dir)) def _installation_path() -> str: diff --git a/cli_audit/reconcile.py b/cli_audit/reconcile.py index 712f2e4..b7bbca7 100644 --- a/cli_audit/reconcile.py +++ b/cli_audit/reconcile.py @@ -22,7 +22,7 @@ from .common import vlog from .config import Config -from .detection import _installation_path, _is_tool_manager_env, _is_virtualenv_bin +from .detection import _installation_path, _is_tool_manager_env, _is_virtualenv_bin, tool_manager_of from .environment import Environment from .upgrade import compare_versions @@ -85,6 +85,8 @@ class Installation: active: bool valid: bool = True preference_score: tuple[int, str, int] = (0, "0.0.0", 0) + # PATH dir the binary was found in (differs from dirname(path) for symlinks) + path_dir: str = "" def to_dict(self) -> dict: """Convert to dictionary for JSON serialization.""" @@ -292,6 +294,7 @@ def detect_installations( path=real_path, active=is_active, valid=valid, + path_dir=path_dir, ) ) @@ -327,6 +330,13 @@ def classify_install_method( Returns: Installation method string (cargo, pipx, apt, brew, etc.) """ + # A binary inside a uv-tool or pipx venv belongs to that manager. The + # queries below match the tool name as a substring of `pipx list` or + # `uv tool list` and could name the wrong one, i.e. the wrong uninstaller. + manager = tool_manager_of(os.path.dirname(path)) + if manager: + return manager + # Try package manager queries first method = _classify_via_queries(path, tool_name, verbose) if method != "unknown": @@ -914,7 +924,7 @@ def _check_path_ordering( f"Preferred installation is not active\n" f" Preferred: {preferred.path}\n" f" Active: {active.path}\n" - f" Fix: Ensure {os.path.dirname(preferred.path)} appears first in PATH" + f" Fix: Ensure {preferred.path_dir or os.path.dirname(preferred.path)} appears first in PATH" ) return tuple(issues) diff --git a/tests/test_detection_venv.py b/tests/test_detection_venv.py index a03740c..df0082f 100644 --- a/tests/test_detection_venv.py +++ b/tests/test_detection_venv.py @@ -134,17 +134,20 @@ def test_trailing_slash_venv_bin_is_skipped(tmp_path, monkeypatch): assert find_paths("faketool4") == [str(real)] -def test_reconcile_keeps_uv_tool_installation(tmp_path, monkeypatch): - # ~/.local/bin/black -> ~/.local/share/uv/tools/black/bin/black; the tool - # dir carries a pyvenv.cfg but the tool is installed, not an environment - from cli_audit.reconcile import clear_detection_cache, detect_installations +@pytest.mark.parametrize( + "tool_env, manager", + [("share/uv/tools/fakeuvtool", "uv"), ("share/pipx/venvs/fakeuvtool", "pipx"), ("relocated/fakeuvtool", "uv")], +) +def test_reconcile_keeps_tool_manager_installation(tmp_path, monkeypatch, tool_env, manager): + # ~/.local/bin/ -> //bin/; the tool dir carries + # a pyvenv.cfg but the tool is installed, not an environment + from cli_audit.reconcile import _check_path_ordering, clear_detection_cache, detect_installations - tool_env = tmp_path / "share" / "uv" / "tools" / "fakeuvtool" - real = _make_bin(_make_venv(tool_env), "fakeuvtool", "26.5.1") + monkeypatch.setenv("UV_TOOL_DIR", str(tmp_path / "relocated")) + real = _make_bin(_make_venv(tmp_path / tool_env), "fakeuvtool", "26.5.1") local_bin = tmp_path / "local" / "bin" local_bin.mkdir(parents=True) (local_bin / "fakeuvtool").symlink_to(real) - # an activated venv with its own copy sits in front venv_bin = _make_venv(tmp_path / ".venv") _make_bin(venv_bin, "fakeuvtool", "25.11.0") monkeypatch.setenv("PATH", os.pathsep.join([str(venv_bin), str(local_bin)])) @@ -152,5 +155,8 @@ def test_reconcile_keeps_uv_tool_installation(tmp_path, monkeypatch): installs = detect_installations("fakeuvtool", ["fakeuvtool"]) - assert [i.path for i in installs] == [str(real)] - assert installs[0].active + assert [(i.path, i.method, i.active) for i in installs] == [(str(real), manager, True)] + # PATH advice names the dir on PATH, not the manager's internal bin dir + inactive = installs[0].__class__(**{**installs[0].__dict__, "active": False}) + other = installs[0].__class__(**{**installs[0].__dict__, "path": "/usr/bin/fakeuvtool"}) + assert f"Ensure {local_bin} appears first" in _check_path_ordering(inactive, other, False)[0] From 76b3cda0e3df6e158c14f03f08e5df911ac47bc0 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 21 Sep 2026 13:29:37 +0200 Subject: [PATCH 07/17] fix(reconcile): resolve relocated tool dirs, uninstall by package and scope Review round 4: - A relocated UV_TOOL_DIR / PIPX_HOME / PIPX_GLOBAL_HOME was compared unresolved against a resolved binary path. With a symlink in the root (symlinked home, macOS /var) or a relative value the tool venv counted as an environment and the installation vanished from reconcile again. The root is resolved with realpath now. Reachable only since uv-tool and pipx installs are detected (before this PR reconcile skipped them, so it never removed them): - `uv tool uninstall` / `pipx uninstall` got the catalog name; catalog gam installs the gam7 package. They get the package name from the per-tool venv dir now. - A `pipx install --global` copy (PIPX_GLOBAL_HOME, default /opt/pipx) was uninstalled without --global, which removes the user-scope copy of the same package instead. - The broken-survivor message suggested "sudo uv install" / "sudo pipx install"; it names the manager's own install command. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- cli_audit/detection.py | 4 +++- cli_audit/reconcile.py | 30 +++++++++++++++++++++++--- tests/test_detection_venv.py | 41 +++++++++++++++++++++++++++++++++++- 3 files changed, 70 insertions(+), 5 deletions(-) diff --git a/cli_audit/detection.py b/cli_audit/detection.py index b9fef2b..1e82400 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -91,7 +91,9 @@ def tool_manager_of(bin_dir: str) -> str: ("pipx", os.path.join(os.environ["PIPX_GLOBAL_HOME"], "venvs") if os.environ.get("PIPX_GLOBAL_HOME") else ""), ) for manager, root in relocated: - if root and normalized.startswith(os.path.normpath(root) + "/"): + # bin_dir is a resolved path; resolve the root the same way (symlinked + # home, macOS /var -> /private/var, a relative value) + if root and normalized.startswith(os.path.realpath(root) + "/"): return manager return "" diff --git a/cli_audit/reconcile.py b/cli_audit/reconcile.py index b7bbca7..6399a1c 100644 --- a/cli_audit/reconcile.py +++ b/cli_audit/reconcile.py @@ -884,7 +884,7 @@ def _reconcile_aggressive( if not probe: errors.append( f"kept installation {preferred.path} no longer works after removal — " - f"reinstall the removed package (e.g. sudo {removed[0].method} install {tool_name}) " + f"reinstall the removed package (e.g. {_reinstall_hint(removed[0].method, tool_name)}) " f"or remove the broken survivor" ) @@ -1019,6 +1019,30 @@ def _cargo_package_for(binary: str, tool: str) -> str: return tool +def _tool_env_package(path: str, tool: str) -> str: + """Package name of a uv-tool or pipx install: its venv dir, //bin/. + + The catalog name can differ (catalog gam installs the gam7 package), and + `uv tool uninstall` / `pipx uninstall` need the package. + """ + bin_dir = os.path.dirname(path) + if tool_manager_of(bin_dir): + return os.path.basename(os.path.dirname(bin_dir)) or tool + return tool + + +def _is_pipx_global(path: str) -> bool: + """True if path lies in pipx's global venvs (`pipx install --global`).""" + root = os.path.realpath(os.environ.get("PIPX_GLOBAL_HOME") or "/opt/pipx") + return os.path.normpath(path).startswith(os.path.join(root, "venvs") + "/") + + +def _reinstall_hint(method: str, tool: str) -> str: + """Command that reinstalls a removed package, for the broken-survivor message.""" + commands = {"uv": "uv tool install", "pipx": "pipx install", "cargo": "cargo install"} + return f"{commands.get(method, f'sudo {method} install')} {tool}" + + def _uninstall_installation(installation: Installation, verbose: bool) -> tuple[bool, str | None]: """ Uninstall a single installation. @@ -1053,7 +1077,7 @@ def _uninstall_installation(installation: Installation, verbose: bool) -> tuple[ elif method == "pipx": try: result = subprocess.run( - ["pipx", "uninstall", tool], + ["pipx", "uninstall"] + (["--global"] if _is_pipx_global(path) else []) + [_tool_env_package(path, tool)], capture_output=True, text=True, timeout=30, @@ -1070,7 +1094,7 @@ def _uninstall_installation(installation: Installation, verbose: bool) -> tuple[ elif method == "uv": try: result = subprocess.run( - ["uv", "tool", "uninstall", tool], + ["uv", "tool", "uninstall", _tool_env_package(path, tool)], capture_output=True, text=True, timeout=30, diff --git a/tests/test_detection_venv.py b/tests/test_detection_venv.py index df0082f..d3c863a 100644 --- a/tests/test_detection_venv.py +++ b/tests/test_detection_venv.py @@ -136,7 +136,12 @@ def test_trailing_slash_venv_bin_is_skipped(tmp_path, monkeypatch): @pytest.mark.parametrize( "tool_env, manager", - [("share/uv/tools/fakeuvtool", "uv"), ("share/pipx/venvs/fakeuvtool", "pipx"), ("relocated/fakeuvtool", "uv")], + [ + ("share/uv/tools/fakeuvtool", "uv"), + ("share/pipx/venvs/fakeuvtool", "pipx"), + ("relocated/fakeuvtool", "uv"), + ("linked/fakeuvtool", "uv"), # UV_TOOL_DIR names a symlink to the real dir + ], ) def test_reconcile_keeps_tool_manager_installation(tmp_path, monkeypatch, tool_env, manager): # ~/.local/bin/ -> //bin/; the tool dir carries @@ -144,6 +149,11 @@ def test_reconcile_keeps_tool_manager_installation(tmp_path, monkeypatch, tool_e from cli_audit.reconcile import _check_path_ordering, clear_detection_cache, detect_installations monkeypatch.setenv("UV_TOOL_DIR", str(tmp_path / "relocated")) + if tool_env.startswith("linked/"): + (tmp_path / "real-tools").mkdir() + (tmp_path / "linked").symlink_to(tmp_path / "real-tools") + tool_env = tool_env.replace("linked/", "real-tools/") + monkeypatch.setenv("UV_TOOL_DIR", str(tmp_path / "linked")) real = _make_bin(_make_venv(tmp_path / tool_env), "fakeuvtool", "26.5.1") local_bin = tmp_path / "local" / "bin" local_bin.mkdir(parents=True) @@ -160,3 +170,32 @@ def test_reconcile_keeps_tool_manager_installation(tmp_path, monkeypatch, tool_e inactive = installs[0].__class__(**{**installs[0].__dict__, "active": False}) other = installs[0].__class__(**{**installs[0].__dict__, "path": "/usr/bin/fakeuvtool"}) assert f"Ensure {local_bin} appears first" in _check_path_ordering(inactive, other, False)[0] + + +@pytest.mark.parametrize( + "layout, method, expected", + [ + ("uv/tools/gam7/bin/gam", "uv", ["uv", "tool", "uninstall", "gam7"]), + ("pipx/venvs/httpie/bin/http", "pipx", ["pipx", "uninstall", "httpie"]), + ("global-pipx/venvs/httpie/bin/http", "pipx", ["pipx", "uninstall", "--global", "httpie"]), + ], +) +def test_uninstall_names_the_package_and_scope(tmp_path, monkeypatch, layout, method, expected): + # catalog gam installs the gam7 package; a global pipx install needs --global + from unittest.mock import MagicMock, patch + + from cli_audit.reconcile import Installation, _uninstall_installation + + monkeypatch.setenv("PIPX_GLOBAL_HOME", str(tmp_path / "global-pipx")) + inst = Installation(tool=layout.split("/")[-1], version="1", method=method, path=str(tmp_path / layout), active=False) + with patch("cli_audit.reconcile.subprocess.run", return_value=MagicMock(returncode=0)) as run: + assert _uninstall_installation(inst, False) == (True, None) + assert run.call_args[0][0] == expected + + +def test_reinstall_hint_uses_the_managers_command(): + from cli_audit.reconcile import _reinstall_hint + + assert _reinstall_hint("uv", "black") == "uv tool install black" + assert _reinstall_hint("pipx", "black") == "pipx install black" + assert _reinstall_hint("apt", "byobu") == "sudo apt install byobu" From c3cfe39d5f1afd0d236d8646c1706cd9a7aadfa5 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 21 Sep 2026 13:34:53 +0200 Subject: [PATCH 08/17] fix(reconcile): exact tool-venv layout, manual global pipx, precise reinstall hints Review round 5 (no correctness bug in the uninstall itself): - tool_manager_of matched any path below a tool root. It now requires //bin, so a stray binary in /bin is not taken for the package "". - PIPX_HOME / PIPX_GLOBAL_HOME / UV_TOOL_DIR are expanded (~) before they are resolved, as pipx reads them. - A global pipx venv is root-owned. As a normal user the removal is reported as "requires manual sudo: sudo pipx uninstall --global " instead of running and failing, like apt/dnf/pacman removals. - The broken-survivor hint names what was removed: the uv/pipx package (gam7, not gam), --global with sudo for a global pipx copy, the cargo crate from the catalog (fd-find, git-delta), and brew without sudo. - A uv/pipx per-tool bin dir put on PATH directly was skipped as an environment; it is an installation. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- cli_audit/detection.py | 32 +++++++++++----- cli_audit/reconcile.py | 43 +++++++++++++++------ tests/test_detection_venv.py | 72 +++++++++++++++++++++++++++++++----- 3 files changed, 115 insertions(+), 32 deletions(-) diff --git a/cli_audit/detection.py b/cli_audit/detection.py index 1e82400..32b195a 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -79,21 +79,33 @@ def _is_virtualenv_bin(bin_dir: str) -> bool: _TOOL_ENV_ROOTS = (("uv", "/uv/tools/"), ("pipx", "/pipx/venvs/")) +def _env_dir(name: str, *parts: str) -> str: + """Resolved directory from an environment variable ("" if unset), as the managers read it.""" + value = os.environ.get(name, "") + if not value: + return "" + # bin_dir is a resolved path; resolve the root the same way (~, a symlinked + # home, macOS /var -> /private/var, a relative value) + return os.path.realpath(os.path.join(os.path.expanduser(value), *parts)) + + def tool_manager_of(bin_dir: str) -> str: - """Return "uv" or "pipx" if bin_dir belongs to that manager's per-tool venv, else "".""" - normalized = os.path.normpath(bin_dir) + "/" + """Return "uv" or "pipx" if bin_dir is a manager's per-tool venv bin dir, //bin, else "".""" + normalized = os.path.normpath(bin_dir) + if os.path.basename(normalized) != "bin": + return "" + # is the dir above /bin + root = os.path.dirname(os.path.dirname(normalized)) + "/" for manager, fragment in _TOOL_ENV_ROOTS: - if fragment in normalized: + if root.endswith(fragment): return manager relocated = ( - ("uv", os.environ.get("UV_TOOL_DIR", "")), - ("pipx", os.path.join(os.environ["PIPX_HOME"], "venvs") if os.environ.get("PIPX_HOME") else ""), - ("pipx", os.path.join(os.environ["PIPX_GLOBAL_HOME"], "venvs") if os.environ.get("PIPX_GLOBAL_HOME") else ""), + ("uv", _env_dir("UV_TOOL_DIR")), + ("pipx", _env_dir("PIPX_HOME", "venvs")), + ("pipx", _env_dir("PIPX_GLOBAL_HOME", "venvs")), ) - for manager, root in relocated: - # bin_dir is a resolved path; resolve the root the same way (symlinked - # home, macOS /var -> /private/var, a relative value) - if root and normalized.startswith(os.path.realpath(root) + "/"): + for manager, env_root in relocated: + if env_root and root == env_root + "/": return manager return "" diff --git a/cli_audit/reconcile.py b/cli_audit/reconcile.py index 6399a1c..eb4c2f4 100644 --- a/cli_audit/reconcile.py +++ b/cli_audit/reconcile.py @@ -22,7 +22,7 @@ from .common import vlog from .config import Config -from .detection import _installation_path, _is_tool_manager_env, _is_virtualenv_bin, tool_manager_of +from .detection import _env_dir, _installation_path, _is_tool_manager_env, _is_virtualenv_bin, tool_manager_of from .environment import Environment from .upgrade import compare_versions @@ -224,8 +224,9 @@ def detect_installations( # Search each PATH directory for path_dir in path_dirs: - # Virtualenv/conda bins are environments, not installations - if _is_virtualenv_bin(path_dir): + # Virtualenv/conda bins are environments, not installations (a uv/pipx + # per-tool bin dir put on PATH directly is an installation) + if _is_virtualenv_bin(path_dir) and not _is_tool_manager_env(path_dir): vlog(f" Skipping environment dir: {path_dir}", verbose) continue for candidate in candidates: @@ -650,6 +651,9 @@ def _catalog_meta(tool_name: str) -> dict: raw = getattr(entry, "_raw_data", None) or {} meta["version_flag"] = raw.get("version_flag") meta["version_command"] = raw.get("version_command") + for method in raw.get("available_methods") or (): + if method.get("method") == "cargo" and (method.get("config") or {}).get("crate"): + meta["cargo_crate"] = method["config"]["crate"] except Exception: meta = {} # Only cache successful lookups — an empty result may be transient. @@ -884,7 +888,7 @@ def _reconcile_aggressive( if not probe: errors.append( f"kept installation {preferred.path} no longer works after removal — " - f"reinstall the removed package (e.g. {_reinstall_hint(removed[0].method, tool_name)}) " + f"reinstall the removed package (e.g. {_reinstall_hint(removed[0])}) " f"or remove the broken survivor" ) @@ -1033,14 +1037,23 @@ def _tool_env_package(path: str, tool: str) -> str: def _is_pipx_global(path: str) -> bool: """True if path lies in pipx's global venvs (`pipx install --global`).""" - root = os.path.realpath(os.environ.get("PIPX_GLOBAL_HOME") or "/opt/pipx") - return os.path.normpath(path).startswith(os.path.join(root, "venvs") + "/") - - -def _reinstall_hint(method: str, tool: str) -> str: - """Command that reinstalls a removed package, for the broken-survivor message.""" - commands = {"uv": "uv tool install", "pipx": "pipx install", "cargo": "cargo install"} - return f"{commands.get(method, f'sudo {method} install')} {tool}" + root = _env_dir("PIPX_GLOBAL_HOME", "venvs") or os.path.realpath("/opt/pipx/venvs") + return os.path.normpath(path).startswith(root + "/") + + +def _reinstall_hint(installation: Installation) -> str: + """Command that reinstalls a removed installation, for the broken-survivor message.""" + method, tool, path = installation.method, installation.tool, installation.path + if method == "uv": + return f"uv tool install {_tool_env_package(path, tool)}" + if method == "pipx": + scope = "sudo pipx install --global" if _is_pipx_global(path) else "pipx install" + return f"{scope} {_tool_env_package(path, tool)}" + if method == "cargo": + return f"cargo install {_catalog_meta(tool).get('cargo_crate') or tool}" + if method == "brew": + return f"brew install {tool}" + return f"sudo {method} install {tool}" def _uninstall_installation(installation: Installation, verbose: bool) -> tuple[bool, str | None]: @@ -1075,6 +1088,12 @@ def _uninstall_installation(installation: Installation, verbose: bool) -> tuple[ # Pipx elif method == "pipx": + if _is_pipx_global(path) and hasattr(os, "geteuid") and os.geteuid() != 0: + # Global pipx venvs are root-owned; this tool never runs sudo itself + return ( + False, + f"System package removal requires manual sudo: sudo pipx uninstall --global {_tool_env_package(path, tool)}", + ) try: result = subprocess.run( ["pipx", "uninstall"] + (["--global"] if _is_pipx_global(path) else []) + [_tool_env_package(path, tool)], diff --git a/tests/test_detection_venv.py b/tests/test_detection_venv.py index d3c863a..e8d6799 100644 --- a/tests/test_detection_venv.py +++ b/tests/test_detection_venv.py @@ -173,29 +173,81 @@ def test_reconcile_keeps_tool_manager_installation(tmp_path, monkeypatch, tool_e @pytest.mark.parametrize( - "layout, method, expected", + "layout, method, euid, expected", [ - ("uv/tools/gam7/bin/gam", "uv", ["uv", "tool", "uninstall", "gam7"]), - ("pipx/venvs/httpie/bin/http", "pipx", ["pipx", "uninstall", "httpie"]), - ("global-pipx/venvs/httpie/bin/http", "pipx", ["pipx", "uninstall", "--global", "httpie"]), + ("uv/tools/gam7/bin/gam", "uv", 1000, ["uv", "tool", "uninstall", "gam7"]), + ("pipx/venvs/httpie/bin/http", "pipx", 1000, ["pipx", "uninstall", "httpie"]), + ("global-pipx/venvs/httpie/bin/http", "pipx", 0, ["pipx", "uninstall", "--global", "httpie"]), ], ) -def test_uninstall_names_the_package_and_scope(tmp_path, monkeypatch, layout, method, expected): +def test_uninstall_names_the_package_and_scope(tmp_path, monkeypatch, layout, method, euid, expected): # catalog gam installs the gam7 package; a global pipx install needs --global from unittest.mock import MagicMock, patch from cli_audit.reconcile import Installation, _uninstall_installation monkeypatch.setenv("PIPX_GLOBAL_HOME", str(tmp_path / "global-pipx")) + monkeypatch.setattr(os, "geteuid", lambda: euid, raising=False) inst = Installation(tool=layout.split("/")[-1], version="1", method=method, path=str(tmp_path / layout), active=False) with patch("cli_audit.reconcile.subprocess.run", return_value=MagicMock(returncode=0)) as run: assert _uninstall_installation(inst, False) == (True, None) assert run.call_args[0][0] == expected -def test_reinstall_hint_uses_the_managers_command(): - from cli_audit.reconcile import _reinstall_hint +def test_global_pipx_removal_is_manual_for_a_normal_user(tmp_path, monkeypatch): + # /opt/pipx is root-owned and this tool never runs sudo: report, do not run + from unittest.mock import patch - assert _reinstall_hint("uv", "black") == "uv tool install black" - assert _reinstall_hint("pipx", "black") == "pipx install black" - assert _reinstall_hint("apt", "byobu") == "sudo apt install byobu" + from cli_audit.reconcile import Installation, _is_manual_removal_error, _uninstall_installation + + monkeypatch.setenv("PIPX_GLOBAL_HOME", str(tmp_path / "global-pipx")) + monkeypatch.setattr(os, "geteuid", lambda: 1000, raising=False) + path = tmp_path / "global-pipx" / "venvs" / "httpie" / "bin" / "http" + inst = Installation(tool="httpie", version="1", method="pipx", path=str(path), active=False) + with patch("cli_audit.reconcile.subprocess.run") as run: + ok, message = _uninstall_installation(inst, False) + assert not ok and not run.called + assert "sudo pipx uninstall --global httpie" in message + assert _is_manual_removal_error(message) + + +def test_reinstall_hint_names_package_crate_and_scope(tmp_path, monkeypatch): + from cli_audit.reconcile import Installation, _reinstall_hint + + monkeypatch.setenv("PIPX_GLOBAL_HOME", str(tmp_path / "global-pipx")) + + def inst(tool, method, path): + return Installation(tool=tool, version="1", method=method, path=str(path), active=False) + + assert _reinstall_hint(inst("gam", "uv", tmp_path / "uv/tools/gam7/bin/gam")) == "uv tool install gam7" + assert _reinstall_hint(inst("httpie", "pipx", tmp_path / "pipx/venvs/httpie/bin/http")) == "pipx install httpie" + assert ( + _reinstall_hint(inst("httpie", "pipx", tmp_path / "global-pipx/venvs/httpie/bin/http")) + == "sudo pipx install --global httpie" + ) + assert _reinstall_hint(inst("fd", "cargo", "/home/u/.cargo/bin/fd")) == "cargo install fd-find" + assert _reinstall_hint(inst("jq", "brew", "/usr/local/bin/jq")) == "brew install jq" + assert _reinstall_hint(inst("byobu", "apt", "/usr/bin/byobu")) == "sudo apt install byobu" + + +def test_tool_manager_needs_package_bin_layout(tmp_path, monkeypatch): + from cli_audit.detection import tool_manager_of + + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setenv("PIPX_HOME", "~/pipx-home") # literal ~, as a systemd unit or .env passes it + assert tool_manager_of(str(tmp_path / "share/uv/tools/black/bin")) == "uv" + assert tool_manager_of(str(tmp_path / "pipx-home/venvs/httpie/bin")) == "pipx" + # not //bin + assert tool_manager_of(str(tmp_path / "share/uv/tools/bin")) == "" + assert tool_manager_of(str(tmp_path / "share/uv/tools/black/lib")) == "" + + +def test_tool_bin_dir_directly_on_path_is_kept(tmp_path, monkeypatch): + from cli_audit.reconcile import clear_detection_cache, detect_installations + + tool_bin = _make_venv(tmp_path / "share" / "uv" / "tools" / "fakedirect") + real = _make_bin(tool_bin, "fakedirect", "1.0.0") + monkeypatch.setenv("PATH", str(tool_bin)) + clear_detection_cache() + + assert [i.path for i in detect_installations("fakedirect", ["fakedirect"])] == [str(real)] From 3824ebc192953793f41800cad23028532a1758ff Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 21 Sep 2026 13:38:53 +0200 Subject: [PATCH 09/17] fix(audit): one environment rule for audit and reconcile Review round 6: - Reconcile kept a uv/pipx per-tool bin dir that sits directly on PATH, but the audit's filtered PATH still dropped it (every tool venv has a pyvenv.cfg). The audit and bulk installs then reported the tool missing, and reconcile marked it inactive. _is_environment_bin is now the single rule ("venv, and not a tool manager's per-tool venv") for _installation_path and both reconcile checks. - Tool roots are resolved, so the rule resolves the dir too: a relocated UV_TOOL_DIR reached through a symlink on PATH is kept. - _catalog_meta skips non-dict available_methods entries instead of losing all of the tool's catalog data to the exception. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- cli_audit/detection.py | 11 ++++++++- cli_audit/reconcile.py | 8 ++++--- tests/test_detection_venv.py | 45 +++++++++++++++++++++++++++++++++++- 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/cli_audit/detection.py b/cli_audit/detection.py index 32b195a..3514fc6 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -115,6 +115,15 @@ def _is_tool_manager_env(bin_dir: str) -> bool: return bool(tool_manager_of(bin_dir)) +def _is_environment_bin(bin_dir: str) -> bool: + """True if bin_dir is an environment's bin dir and no tool manager's per-tool venv. + + The one rule for "environment, not installation", shared by the audit and + reconcile. Tool roots are compared resolved, so bin_dir is resolved too. + """ + return _is_virtualenv_bin(bin_dir) and not _is_tool_manager_env(os.path.realpath(bin_dir)) + + def _installation_path() -> str: """PATH without virtualenv/conda bin dirs. @@ -123,7 +132,7 @@ def _installation_path() -> str: upgrade of the real installation never shows up in the audit. """ dirs = [d for d in os.environ.get("PATH", os.defpath).split(os.pathsep) if d] - return os.pathsep.join(d for d in dirs if not _is_virtualenv_bin(d)) + return os.pathsep.join(d for d in dirs if not _is_environment_bin(d)) def _which(command_name: str) -> str | None: diff --git a/cli_audit/reconcile.py b/cli_audit/reconcile.py index eb4c2f4..07fd8d5 100644 --- a/cli_audit/reconcile.py +++ b/cli_audit/reconcile.py @@ -22,7 +22,7 @@ from .common import vlog from .config import Config -from .detection import _env_dir, _installation_path, _is_tool_manager_env, _is_virtualenv_bin, tool_manager_of +from .detection import _env_dir, _installation_path, _is_environment_bin, tool_manager_of from .environment import Environment from .upgrade import compare_versions @@ -226,7 +226,7 @@ def detect_installations( for path_dir in path_dirs: # Virtualenv/conda bins are environments, not installations (a uv/pipx # per-tool bin dir put on PATH directly is an installation) - if _is_virtualenv_bin(path_dir) and not _is_tool_manager_env(path_dir): + if _is_environment_bin(path_dir): vlog(f" Skipping environment dir: {path_dir}", verbose) continue for candidate in candidates: @@ -250,7 +250,7 @@ def detect_installations( # A symlink can point into an environment as well real_dir = os.path.dirname(real_path) - if _is_virtualenv_bin(real_dir) and not _is_tool_manager_env(real_dir): + if _is_environment_bin(real_dir): vlog(f" Skipping environment binary: {real_path}", verbose) continue @@ -652,6 +652,8 @@ def _catalog_meta(tool_name: str) -> dict: meta["version_flag"] = raw.get("version_flag") meta["version_command"] = raw.get("version_command") for method in raw.get("available_methods") or (): + if not isinstance(method, dict): + continue if method.get("method") == "cargo" and (method.get("config") or {}).get("crate"): meta["cargo_crate"] = method["config"]["crate"] except Exception: diff --git a/tests/test_detection_venv.py b/tests/test_detection_venv.py index e8d6799..8693d01 100644 --- a/tests/test_detection_venv.py +++ b/tests/test_detection_venv.py @@ -250,4 +250,47 @@ def test_tool_bin_dir_directly_on_path_is_kept(tmp_path, monkeypatch): monkeypatch.setenv("PATH", str(tool_bin)) clear_detection_cache() - assert [i.path for i in detect_installations("fakedirect", ["fakedirect"])] == [str(real)] + assert [(i.path, i.active) for i in detect_installations("fakedirect", ["fakedirect"])] == [(str(real), True)] + # the audit must agree: the tool is installed + assert find_paths("fakedirect") == [str(real)] + + +def test_symlinked_relocated_tool_dir_on_path_is_kept(tmp_path, monkeypatch): + from cli_audit.reconcile import clear_detection_cache, detect_installations + + (tmp_path / "real-tools").mkdir() + (tmp_path / "linked").symlink_to(tmp_path / "real-tools") + monkeypatch.setenv("UV_TOOL_DIR", str(tmp_path / "linked")) + real = _make_bin(_make_venv(tmp_path / "real-tools" / "fakelinked"), "fakelinked", "1.0.0") + on_path = tmp_path / "linked" / "fakelinked" / "bin" + monkeypatch.setenv("PATH", str(on_path)) + clear_detection_cache() + + assert [i.path for i in detect_installations("fakelinked", ["fakelinked"])] == [str(real)] + assert find_paths("fakelinked") == [str(on_path / "fakelinked")] + + +def test_malformed_available_method_keeps_other_catalog_data(monkeypatch): + from cli_audit import reconcile + + class Entry: + _raw_data = {"version_flag": "--ver", "available_methods": ["cargo", {"method": "cargo", "config": {"crate": "c"}}]} + + def to_tool(self): + class T: + candidates = ("x",) + + return T() + + class Catalog: + def get(self, name): + return Entry() + + def all_tools(self): + return [1] + + monkeypatch.setattr(reconcile, "_catalog_instance", Catalog()) + monkeypatch.setattr(reconcile, "_catalog_cache", {}) + meta = reconcile._catalog_meta("x") + assert meta["version_flag"] == "--ver" + assert meta["cargo_crate"] == "c" From 6677fe9a75df4d0a9224305eb5e3819b21aa78f5 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 21 Sep 2026 13:46:32 +0200 Subject: [PATCH 10/17] fix(reconcile): only a tool's own entry points count as its installation Review round 7: - With a uv/pipx per-tool bin dir on PATH (directly, or put there by `uv tool run`), every executable in it counted as an installation of that tool's package, including its dependencies' entry points (pygmentize in httpie's venv). Aggressive reconcile of a catalog tool that is also a dependency could then run `uv tool uninstall `. An executable in a per-tool venv now counts only if the manager's own record lists it: uv-receipt.toml [tool] entrypoints, pipx_metadata.json main_package.apps (plus injected packages installed with --include-apps). Audit lookups skip such executables and keep searching PATH; reconcile skips them. - Default tool roots that are themselves symlinks (~/.local/share/uv/tools -> /data/uvtools) are recognised by their resolved path, like relocated ones. - _catalog_meta also survives a non-dict "config" or a non-list available_methods. Checked on this machine: black, isort, gam (package gam7) and http (httpie) resolve to their uv tool venvs, and the audit differs from main only for black, isort, python@3.14 and flake8. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- cli_audit/detection.py | 64 ++++++++++++++++++++++++++++++++++-- cli_audit/reconcile.py | 20 +++++++---- tests/test_detection_venv.py | 54 +++++++++++++++++++++++++++--- 3 files changed, 124 insertions(+), 14 deletions(-) diff --git a/cli_audit/detection.py b/cli_audit/detection.py index 3514fc6..817858f 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -6,10 +6,12 @@ from __future__ import annotations +import json import os import re import shutil import subprocess +import tomllib from typing import Sequence # Constants @@ -99,10 +101,16 @@ def tool_manager_of(bin_dir: str) -> str: for manager, fragment in _TOOL_ENV_ROOTS: if root.endswith(fragment): return manager + data_home = os.environ.get("XDG_DATA_HOME") or os.path.join(os.path.expanduser("~"), ".local", "share") relocated = ( ("uv", _env_dir("UV_TOOL_DIR")), ("pipx", _env_dir("PIPX_HOME", "venvs")), ("pipx", _env_dir("PIPX_GLOBAL_HOME", "venvs")), + # default roots that are themselves symlinks (tools kept on another disk) + ("uv", os.path.realpath(os.path.join(data_home, "uv", "tools"))), + ("pipx", os.path.realpath(os.path.join(data_home, "pipx", "venvs"))), + ("pipx", os.path.realpath(os.path.join(os.path.expanduser("~"), ".local", "pipx", "venvs"))), + ("pipx", os.path.realpath("/opt/pipx/venvs")), ) for manager, env_root in relocated: if env_root and root == env_root + "/": @@ -115,6 +123,48 @@ def _is_tool_manager_env(bin_dir: str) -> bool: return bool(tool_manager_of(bin_dir)) +def tool_entrypoints(bin_dir: str) -> set[str] | None: + """Executables the manager installed from a per-tool venv, per its own record. + + uv writes uv-receipt.toml ([tool] entrypoints), pipx pipx_metadata.json + (main_package.apps, plus apps of injected packages installed with + --include-apps). Everything else in that bin dir belongs to dependencies. + None if the venv has no readable record. + """ + venv = os.path.dirname(os.path.normpath(bin_dir)) + try: + receipt = os.path.join(venv, "uv-receipt.toml") + if os.path.isfile(receipt): + with open(receipt, "rb") as f: + entries = tomllib.load(f).get("tool", {}).get("entrypoints", []) + return {e["name"] for e in entries if isinstance(e, dict) and e.get("name")} + metadata = os.path.join(venv, "pipx_metadata.json") + if os.path.isfile(metadata): + with open(metadata, encoding="utf-8") as f: + data = json.load(f) + packages = [data.get("main_package") or {}] + packages += [p for p in (data.get("injected_packages") or {}).values() if p.get("include_apps")] + return {app for p in packages for app in (p.get("apps") or [])} + except OSError, ValueError, AttributeError, TypeError, KeyError: + return None + return None + + +def _is_tool_dependency_binary(path: str) -> bool: + """True if path resolves into a uv/pipx per-tool venv but is not one of its entry points. + + A dependency's executable there (pygmentize in httpie's venv) is no + installation of anything: removing it as a duplicate would uninstall the + tool that pulled it in. + """ + real = os.path.realpath(path) + real_dir = os.path.dirname(real) + if not tool_manager_of(real_dir): + return False + names = tool_entrypoints(real_dir) + return names is None or os.path.basename(real) not in names + + def _is_environment_bin(bin_dir: str) -> bool: """True if bin_dir is an environment's bin dir and no tool manager's per-tool venv. @@ -136,8 +186,16 @@ def _installation_path() -> str: def _which(command_name: str) -> str | None: - """shutil.which restricted to installation dirs (see _installation_path).""" - return shutil.which(command_name, path=_installation_path()) + """shutil.which restricted to installation dirs (see _installation_path). + + Skips a dependency's executable inside a uv/pipx per-tool venv and keeps + searching the next PATH dir. + """ + for path_dir in _installation_path().split(os.pathsep): + found = shutil.which(command_name, path=path_dir) if path_dir else None + if found and not _is_tool_dependency_binary(found): + return found + return None def find_paths(command_name: str, deep: bool = False) -> list[str]: @@ -174,7 +232,7 @@ def find_paths(command_name: str, deep: bool = False) -> list[str]: for line in (proc.stdout or "").splitlines(): line = line.strip() if line and os.path.isfile(line) and os.access(line, os.X_OK): - if line not in paths: + if line not in paths and not _is_tool_dependency_binary(line): paths.append(line) except Exception: pass diff --git a/cli_audit/reconcile.py b/cli_audit/reconcile.py index 07fd8d5..a3553f2 100644 --- a/cli_audit/reconcile.py +++ b/cli_audit/reconcile.py @@ -22,7 +22,13 @@ from .common import vlog from .config import Config -from .detection import _env_dir, _installation_path, _is_environment_bin, tool_manager_of +from .detection import ( + _env_dir, + _installation_path, + _is_environment_bin, + _is_tool_dependency_binary, + tool_manager_of, +) from .environment import Environment from .upgrade import compare_versions @@ -250,7 +256,7 @@ def detect_installations( # A symlink can point into an environment as well real_dir = os.path.dirname(real_path) - if _is_environment_bin(real_dir): + if _is_environment_bin(real_dir) or _is_tool_dependency_binary(real_path): vlog(f" Skipping environment binary: {real_path}", verbose) continue @@ -651,11 +657,11 @@ def _catalog_meta(tool_name: str) -> dict: raw = getattr(entry, "_raw_data", None) or {} meta["version_flag"] = raw.get("version_flag") meta["version_command"] = raw.get("version_command") - for method in raw.get("available_methods") or (): - if not isinstance(method, dict): - continue - if method.get("method") == "cargo" and (method.get("config") or {}).get("crate"): - meta["cargo_crate"] = method["config"]["crate"] + methods = raw.get("available_methods") + for method in methods if isinstance(methods, list) else (): + config = method.get("config") if isinstance(method, dict) else None + if isinstance(config, dict) and method.get("method") == "cargo" and config.get("crate"): + meta["cargo_crate"] = config["crate"] except Exception: meta = {} # Only cache successful lookups — an empty result may be transient. diff --git a/tests/test_detection_venv.py b/tests/test_detection_venv.py index 8693d01..16d777f 100644 --- a/tests/test_detection_venv.py +++ b/tests/test_detection_venv.py @@ -10,6 +10,7 @@ from __future__ import annotations +import json import os import shutil import sys @@ -40,6 +41,17 @@ def _make_bin(bin_dir: Path, name: str, version: str) -> Path: return binary +def _make_tool_venv(root: Path, *entrypoints: str) -> Path: + """A uv/pipx per-tool venv whose own record lists these entry points.""" + bin_dir = _make_venv(root) + if "/pipx/" in str(root): + (root / "pipx_metadata.json").write_text(json.dumps({"main_package": {"apps": list(entrypoints)}})) + else: + lines = ",\n".join(f' {{ name = "{e}", install-path = "/x/{e}" }}' for e in entrypoints) + (root / "uv-receipt.toml").write_text(f"[tool]\nentrypoints = [\n{lines}\n]\n") + return bin_dir + + def _make_venv(root: Path) -> Path: root.mkdir(parents=True, exist_ok=True) (root / "pyvenv.cfg").write_text("home = /usr/bin\n") @@ -154,7 +166,7 @@ def test_reconcile_keeps_tool_manager_installation(tmp_path, monkeypatch, tool_e (tmp_path / "linked").symlink_to(tmp_path / "real-tools") tool_env = tool_env.replace("linked/", "real-tools/") monkeypatch.setenv("UV_TOOL_DIR", str(tmp_path / "linked")) - real = _make_bin(_make_venv(tmp_path / tool_env), "fakeuvtool", "26.5.1") + real = _make_bin(_make_tool_venv(tmp_path / tool_env, "fakeuvtool"), "fakeuvtool", "26.5.1") local_bin = tmp_path / "local" / "bin" local_bin.mkdir(parents=True) (local_bin / "fakeuvtool").symlink_to(real) @@ -245,7 +257,7 @@ def test_tool_manager_needs_package_bin_layout(tmp_path, monkeypatch): def test_tool_bin_dir_directly_on_path_is_kept(tmp_path, monkeypatch): from cli_audit.reconcile import clear_detection_cache, detect_installations - tool_bin = _make_venv(tmp_path / "share" / "uv" / "tools" / "fakedirect") + tool_bin = _make_tool_venv(tmp_path / "share" / "uv" / "tools" / "fakedirect", "fakedirect") real = _make_bin(tool_bin, "fakedirect", "1.0.0") monkeypatch.setenv("PATH", str(tool_bin)) clear_detection_cache() @@ -261,7 +273,7 @@ def test_symlinked_relocated_tool_dir_on_path_is_kept(tmp_path, monkeypatch): (tmp_path / "real-tools").mkdir() (tmp_path / "linked").symlink_to(tmp_path / "real-tools") monkeypatch.setenv("UV_TOOL_DIR", str(tmp_path / "linked")) - real = _make_bin(_make_venv(tmp_path / "real-tools" / "fakelinked"), "fakelinked", "1.0.0") + real = _make_bin(_make_tool_venv(tmp_path / "real-tools" / "fakelinked", "fakelinked"), "fakelinked", "1.0.0") on_path = tmp_path / "linked" / "fakelinked" / "bin" monkeypatch.setenv("PATH", str(on_path)) clear_detection_cache() @@ -274,7 +286,10 @@ def test_malformed_available_method_keeps_other_catalog_data(monkeypatch): from cli_audit import reconcile class Entry: - _raw_data = {"version_flag": "--ver", "available_methods": ["cargo", {"method": "cargo", "config": {"crate": "c"}}]} + _raw_data = { + "version_flag": "--ver", + "available_methods": ["cargo", {"method": "cargo", "config": "x"}, {"method": "cargo", "config": {"crate": "c"}}], + } def to_tool(self): class T: @@ -294,3 +309,34 @@ def all_tools(self): meta = reconcile._catalog_meta("x") assert meta["version_flag"] == "--ver" assert meta["cargo_crate"] == "c" + + +def test_dependency_executable_in_a_tool_venv_is_no_installation(tmp_path, monkeypatch): + # httpie's uv venv also holds pygmentize (a dependency). Treating it as an + # installation would let reconcile run `uv tool uninstall httpie` for it. + from cli_audit.reconcile import clear_detection_cache, detect_installations + + tool_bin = _make_tool_venv(tmp_path / "share" / "uv" / "tools" / "fakehttpie", "fakehttp") + _make_bin(tool_bin, "fakehttp", "3.2.4") + _make_bin(tool_bin, "fakepygmentize", "2.19.0") + other_bin = tmp_path / "usr" / "bin" + real_pyg = _make_bin(other_bin, "fakepygmentize", "2.18.0") + monkeypatch.setenv("PATH", os.pathsep.join([str(tool_bin), str(other_bin), WHICH_DIR])) + clear_detection_cache() + + assert [i.path for i in detect_installations("fakepygmentize", ["fakepygmentize"])] == [str(real_pyg)] + assert find_paths("fakepygmentize", deep=True) == [str(real_pyg)] + assert find_paths("fakehttp") == [str(tool_bin / "fakehttp")] + + +def test_symlinked_default_tool_root_is_recognised(tmp_path, monkeypatch): + # ~/.local/share/uv/tools -> /data/uvtools, UV_TOOL_DIR unset + from cli_audit.detection import tool_manager_of + + monkeypatch.delenv("UV_TOOL_DIR", raising=False) + monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path / "share")) + (tmp_path / "data" / "uvtools" / "black" / "bin").mkdir(parents=True) + (tmp_path / "share" / "uv").mkdir(parents=True) + (tmp_path / "share" / "uv" / "tools").symlink_to(tmp_path / "data" / "uvtools") + + assert tool_manager_of(str(tmp_path / "data" / "uvtools" / "black" / "bin")) == "uv" From fb892a403d6bbb290f96917e40e8898b50ed905e Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 21 Sep 2026 16:03:53 +0200 Subject: [PATCH 11/17] fix(audit): read a tool record only inside its manager's root SonarCloud findings on the entry-point check: - tool_entrypoints built its path from bin_dir, which comes from PATH, and tried both record names in any directory (S8707, path traversal). It now resolves the manager first and opens only that manager's record name below its own tool root. - The cargo crate lookup moves out of _catalog_meta into _cargo_crate_of, which keeps the function under the complexity limit and also survives a non-list available_methods. - One test asserted two things in one statement. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- cli_audit/detection.py | 28 ++++++++++++++++------------ cli_audit/reconcile.py | 21 ++++++++++++++++----- tests/test_detection_venv.py | 20 +++++++++++++------- 3 files changed, 45 insertions(+), 24 deletions(-) diff --git a/cli_audit/detection.py b/cli_audit/detection.py index 817858f..b276965 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -131,23 +131,27 @@ def tool_entrypoints(bin_dir: str) -> set[str] | None: --include-apps). Everything else in that bin dir belongs to dependencies. None if the venv has no readable record. """ - venv = os.path.dirname(os.path.normpath(bin_dir)) + # bin_dir comes from PATH. Read only inside a manager's own tool root, and + # only that manager's record name, so no other file can be reached. + manager = tool_manager_of(bin_dir) + if not manager: + return None + venv = os.path.dirname(os.path.realpath(bin_dir)) + record = os.path.join(venv, "uv-receipt.toml" if manager == "uv" else "pipx_metadata.json") + if not os.path.isfile(record): + return None try: - receipt = os.path.join(venv, "uv-receipt.toml") - if os.path.isfile(receipt): - with open(receipt, "rb") as f: + if manager == "uv": + with open(record, "rb") as f: entries = tomllib.load(f).get("tool", {}).get("entrypoints", []) return {e["name"] for e in entries if isinstance(e, dict) and e.get("name")} - metadata = os.path.join(venv, "pipx_metadata.json") - if os.path.isfile(metadata): - with open(metadata, encoding="utf-8") as f: - data = json.load(f) - packages = [data.get("main_package") or {}] - packages += [p for p in (data.get("injected_packages") or {}).values() if p.get("include_apps")] - return {app for p in packages for app in (p.get("apps") or [])} + with open(record, encoding="utf-8") as f: + data = json.load(f) + packages = [data.get("main_package") or {}] + packages += [p for p in (data.get("injected_packages") or {}).values() if p.get("include_apps")] + return {app for p in packages for app in (p.get("apps") or [])} except OSError, ValueError, AttributeError, TypeError, KeyError: return None - return None def _is_tool_dependency_binary(path: str) -> bool: diff --git a/cli_audit/reconcile.py b/cli_audit/reconcile.py index a3553f2..ef1a75c 100644 --- a/cli_audit/reconcile.py +++ b/cli_audit/reconcile.py @@ -630,6 +630,19 @@ def get_preference_tier(installation: Installation) -> int: _catalog_lock = threading.Lock() +def _cargo_crate_of(available_methods: object) -> str: + """Crate name from a catalog entry's cargo install method ("" if it has none).""" + if not isinstance(available_methods, list): + return "" + for method in available_methods: + if not isinstance(method, dict) or method.get("method") != "cargo": + continue + config = method.get("config") + if isinstance(config, dict) and isinstance(config.get("crate"), str): + return config["crate"] + return "" + + def _catalog_meta(tool_name: str) -> dict: """Return cached catalog metadata for a tool: candidates + version command. @@ -657,11 +670,9 @@ def _catalog_meta(tool_name: str) -> dict: raw = getattr(entry, "_raw_data", None) or {} meta["version_flag"] = raw.get("version_flag") meta["version_command"] = raw.get("version_command") - methods = raw.get("available_methods") - for method in methods if isinstance(methods, list) else (): - config = method.get("config") if isinstance(method, dict) else None - if isinstance(config, dict) and method.get("method") == "cargo" and config.get("crate"): - meta["cargo_crate"] = config["crate"] + crate = _cargo_crate_of(raw.get("available_methods")) + if crate: + meta["cargo_crate"] = crate except Exception: meta = {} # Only cache successful lookups — an empty result may be transient. diff --git a/tests/test_detection_venv.py b/tests/test_detection_venv.py index 16d777f..d477c52 100644 --- a/tests/test_detection_venv.py +++ b/tests/test_detection_venv.py @@ -218,7 +218,8 @@ def test_global_pipx_removal_is_manual_for_a_normal_user(tmp_path, monkeypatch): inst = Installation(tool="httpie", version="1", method="pipx", path=str(path), active=False) with patch("cli_audit.reconcile.subprocess.run") as run: ok, message = _uninstall_installation(inst, False) - assert not ok and not run.called + assert not ok + assert not run.called assert "sudo pipx uninstall --global httpie" in message assert _is_manual_removal_error(message) @@ -282,14 +283,19 @@ def test_symlinked_relocated_tool_dir_on_path_is_kept(tmp_path, monkeypatch): assert find_paths("fakelinked") == [str(on_path / "fakelinked")] -def test_malformed_available_method_keeps_other_catalog_data(monkeypatch): +@pytest.mark.parametrize( + "available_methods", + [ + ["cargo", {"method": "cargo", "config": "x"}, {"method": "cargo", "config": {"crate": "c"}}], + 5, # not a list at all + ], + ids=["malformed-entries", "not-a-list"], +) +def test_malformed_available_method_keeps_other_catalog_data(monkeypatch, available_methods): from cli_audit import reconcile class Entry: - _raw_data = { - "version_flag": "--ver", - "available_methods": ["cargo", {"method": "cargo", "config": "x"}, {"method": "cargo", "config": {"crate": "c"}}], - } + _raw_data = {"version_flag": "--ver", "available_methods": available_methods} def to_tool(self): class T: @@ -308,7 +314,7 @@ def all_tools(self): monkeypatch.setattr(reconcile, "_catalog_cache", {}) meta = reconcile._catalog_meta("x") assert meta["version_flag"] == "--ver" - assert meta["cargo_crate"] == "c" + assert meta.get("cargo_crate", "") == ("c" if isinstance(available_methods, list) else "") def test_dependency_executable_in_a_tool_venv_is_no_installation(tmp_path, monkeypatch): From 254e93453d4fa1829723261bdb8db3e17366bb6a Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 21 Sep 2026 16:04:00 +0200 Subject: [PATCH 12/17] fix(audit): keep dependency copies out of every lookup Review findings on the entry-point check: - A catalog version_command runs the tool by name. It used the PATH that keeps a manager's per-tool venv, so a dependency's executable there could answer, and the standalone fallback could report a tool as installed when only a dependency copy exists. Such a command now runs with _command_path(), which drops every environment bin dir. - reconcile's active-copy lookup, bulk.get_missing_tools and installer.validate_installation called shutil.which with that same PATH and skipped the dependency check. All three use _which now. - An unreadable tool record made every executable in that venv count as a dependency without saying why; the path and the error are logged. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- cli_audit/bulk.py | 5 ++--- cli_audit/detection.py | 23 +++++++++++++++++++---- cli_audit/installer.py | 5 ++--- cli_audit/reconcile.py | 4 ++-- tests/test_bulk.py | 8 ++++---- tests/test_detection_venv.py | 33 +++++++++++++++++++++++++++++++++ 6 files changed, 62 insertions(+), 16 deletions(-) diff --git a/cli_audit/bulk.py b/cli_audit/bulk.py index 7734851..a623f9c 100644 --- a/cli_audit/bulk.py +++ b/cli_audit/bulk.py @@ -8,7 +8,6 @@ from __future__ import annotations import os -import shutil import subprocess import tempfile import threading @@ -21,7 +20,7 @@ from .common import vlog from .config import Config -from .detection import _installation_path +from .detection import _which from .environment import Environment from .installer import InstallResult, install_tool from .package_managers import select_package_manager @@ -169,7 +168,7 @@ def get_missing_tools(tool_names: Sequence[str], verbose: bool = False) -> list[ missing = [] for tool_name in tool_names: # Same lookup as the audit: a copy inside an activated venv is no installation - binary_path = shutil.which(tool_name, path=_installation_path()) + binary_path = _which(tool_name) if not binary_path: missing.append(tool_name) vlog(f"Tool not found: {tool_name}", verbose) diff --git a/cli_audit/detection.py b/cli_audit/detection.py index b276965..d339a1c 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -7,6 +7,7 @@ from __future__ import annotations import json +import logging import os import re import shutil @@ -150,7 +151,10 @@ def tool_entrypoints(bin_dir: str) -> set[str] | None: packages = [data.get("main_package") or {}] packages += [p for p in (data.get("injected_packages") or {}).values() if p.get("include_apps")] return {app for p in packages for app in (p.get("apps") or [])} - except OSError, ValueError, AttributeError, TypeError, KeyError: + except (OSError, ValueError, AttributeError, TypeError, KeyError) as exc: + # Unreadable record: every executable in that venv then counts as a + # dependency, so say which file and why + logging.getLogger(__name__).debug("unreadable tool record %s: %s", record, exc) return None @@ -178,6 +182,17 @@ def _is_environment_bin(bin_dir: str) -> bool: return _is_virtualenv_bin(bin_dir) and not _is_tool_manager_env(os.path.realpath(bin_dir)) +def _command_path() -> str: + """PATH for running a tool by its name: no environment bin dirs at all. + + Stricter than _installation_path, which keeps a manager's per-tool venv: + running a name there can hit a dependency's executable (pygmentize in + httpie's venv) instead of the installation. + """ + dirs = [d for d in os.environ.get("PATH", os.defpath).split(os.pathsep) if d] + return os.pathsep.join(d for d in dirs if not _is_virtualenv_bin(d)) + + def _installation_path() -> str: """PATH without virtualenv/conda bin dirs. @@ -346,9 +361,9 @@ def get_version_line( # from user input — e.g. `uv python list --only-installed | grep … | sed …`. # shell=True is required for the pipelines used in the catalog. if version_command: - # The command names the tool, not the path: resolve that name the way - # find_paths does, never to an activated environment's copy. - search_path = _installation_path() + # The command names the tool, not the path: resolve that name outside + # every environment, including a tool manager's per-tool venv. + search_path = _command_path() try: proc = subprocess.run( # nosec B602 version_command, diff --git a/cli_audit/installer.py b/cli_audit/installer.py index a8be466..c149248 100644 --- a/cli_audit/installer.py +++ b/cli_audit/installer.py @@ -9,14 +9,13 @@ import hashlib import random -import shutil import subprocess import time from dataclasses import dataclass from .common import vlog from .config import Config -from .detection import _installation_path +from .detection import _which from .environment import Environment from .install_plan import InstallStep, generate_install_plan from .package_managers import select_package_manager @@ -383,7 +382,7 @@ def validate_installation( """ # Check if binary exists in PATH. Same lookup as the audit: a copy inside # an activated venv would shadow the tool that was just installed. - binary_path = shutil.which(tool_name, path=_installation_path()) + binary_path = _which(tool_name) if not binary_path: vlog(f"Binary not found in PATH: {tool_name}", verbose) return (False, None, None) diff --git a/cli_audit/reconcile.py b/cli_audit/reconcile.py index ef1a75c..31d76f3 100644 --- a/cli_audit/reconcile.py +++ b/cli_audit/reconcile.py @@ -24,9 +24,9 @@ from .config import Config from .detection import ( _env_dir, - _installation_path, _is_environment_bin, _is_tool_dependency_binary, + _which, tool_manager_of, ) from .environment import Environment @@ -290,7 +290,7 @@ def detect_installations( # Check if this is the active installation # The active copy is resolved the same way the audit resolves it - active_path = shutil.which(candidate, path=_installation_path()) + active_path = _which(candidate) is_active = (os.path.realpath(active_path) == real_path) if active_path else False installations.append( diff --git a/tests/test_bulk.py b/tests/test_bulk.py index ccca00d..0b1dc85 100644 --- a/tests/test_bulk.py +++ b/tests/test_bulk.py @@ -222,7 +222,7 @@ def test_bulk_install_result_to_dict(self): class TestGetMissingTools: """Tests for get_missing_tools function.""" - @patch("cli_audit.bulk.shutil.which") + @patch("cli_audit.bulk._which") def test_get_missing_tools_all_missing(self, mock_which): """Test when all tools are missing.""" mock_which.return_value = None @@ -233,7 +233,7 @@ def test_get_missing_tools_all_missing(self, mock_which): assert missing == tools assert mock_which.call_count == 3 - @patch("cli_audit.bulk.shutil.which") + @patch("cli_audit.bulk._which") def test_get_missing_tools_all_installed(self, mock_which): """Test when all tools are installed.""" mock_which.return_value = "/usr/bin/tool" @@ -244,11 +244,11 @@ def test_get_missing_tools_all_installed(self, mock_which): assert missing == [] assert mock_which.call_count == 3 - @patch("cli_audit.bulk.shutil.which") + @patch("cli_audit.bulk._which") def test_get_missing_tools_mixed(self, mock_which): """Test when some tools are installed.""" - def which_side_effect(tool, path=None): + def which_side_effect(tool): if tool in ("ripgrep", "mypy"): return "/usr/bin/" + tool return None diff --git a/tests/test_detection_venv.py b/tests/test_detection_venv.py index d477c52..adc2ee7 100644 --- a/tests/test_detection_venv.py +++ b/tests/test_detection_venv.py @@ -346,3 +346,36 @@ def test_symlinked_default_tool_root_is_recognised(tmp_path, monkeypatch): (tmp_path / "share" / "uv" / "tools").symlink_to(tmp_path / "data" / "uvtools") assert tool_manager_of(str(tmp_path / "data" / "uvtools" / "black" / "bin")) == "uv" + + +def test_dependency_copy_answers_no_lookup(tmp_path, monkeypatch): + # A tool venv bin dir first on PATH (uv tool run): its dependency copies + # must not answer for the audit, the bulk check, the install validation, + # the version_command, or reconcile + tool_bin = _make_tool_venv(tmp_path / "share" / "uv" / "tools" / "fakehttpie2", "fakehttp2") + _make_bin(tool_bin, "fakedep", "2.19.0") + real_bin = tmp_path / "local" / "bin" + real = _make_bin(real_bin, "fakedep", "2.18.0") + monkeypatch.setenv("PATH", os.pathsep.join([str(tool_bin), str(real_bin), WHICH_DIR])) + + from cli_audit.reconcile import clear_detection_cache, detect_installations + + clear_detection_cache() + assert [(i.path, i.active) for i in detect_installations("fakedep", ["fakedep"])] == [(str(real), True)] + assert get_missing_tools(["fakedep"]) == [] + assert validate_installation("fakedep")[1] == str(real) + version, _line, path, _method = audit_tool_installation("fakedep", ("fakedep",), version_command="fakedep --version") + assert (version, path) == ("2.18.0", str(real)) + + +def test_tool_venv_is_not_used_to_resolve_a_command(tmp_path, monkeypatch): + # A tool's own entry point is still found through its PATH dir, but a + # version_command runs outside every environment + from cli_audit.detection import _command_path, _installation_path + + tool_bin = _make_tool_venv(tmp_path / "share" / "uv" / "tools" / "fakeonly", "fakeonly") + _make_bin(tool_bin, "fakeonly", "1.0.0") + monkeypatch.setenv("PATH", str(tool_bin)) + + assert _installation_path() == str(tool_bin) + assert _command_path() == "" From d4c4d42528df8df42e5eae146adecbfae67fadff Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 21 Sep 2026 16:04:08 +0200 Subject: [PATCH 13/17] fix(audit): build a tool record path from a known root and a checked name SonarCloud kept flagging tool_entrypoints for path injection: the file it opened was derived from a PATH entry, and the guard that constrained it lived in another function. _tool_venv_of resolves a bin dir once and returns (manager, root, package): the root is one of this machine's uv/pipx tool roots, and the package dir name must be a plain package name. tool_entrypoints then joins root, package and the manager's fixed record name, so nothing derived from PATH reaches open(). Tool roots now come from that list only (UV_TOOL_DIR, PIPX_HOME, PIPX_GLOBAL_HOME, XDG_DATA_HOME or ~/.local/share, ~/.local/pipx/venvs, /opt/pipx/venvs) instead of a "/uv/tools/" path fragment, so a venv that merely sits under a path containing that fragment no longer counts. The join is equivalent to the resolved path by construction; the validated package name is the part that decides, and a test covers it. Tests set the manager variables, because the roots are now per machine. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- cli_audit/detection.py | 77 ++++++++++++++++----------- tests/integration/test_e2e_install.py | 34 ++++++------ tests/test_detection_venv.py | 42 +++++++++++++-- 3 files changed, 100 insertions(+), 53 deletions(-) diff --git a/cli_audit/detection.py b/cli_audit/detection.py index d339a1c..9c9f68e 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -77,9 +77,11 @@ def _is_virtualenv_bin(bin_dir: str) -> bool: # Tool managers install each tool into a venv of its own; a binary linked # from there (~/.local/bin/black -> ~/.local/share/uv/tools/black/bin/black) -# is an installation, not an environment. Default locations, matched as path -# fragments; relocated ones come from the managers' own variables. -_TOOL_ENV_ROOTS = (("uv", "/uv/tools/"), ("pipx", "/pipx/venvs/")) +# is an installation, not an environment. +_TOOL_RECORDS = {"uv": "uv-receipt.toml", "pipx": "pipx_metadata.json"} + +# A package directory name: no separator, no "..", so it cannot leave the root +_PACKAGE_DIR_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9._+-]*\Z") def _env_dir(name: str, *parts: str) -> str: @@ -92,31 +94,43 @@ def _env_dir(name: str, *parts: str) -> str: return os.path.realpath(os.path.join(os.path.expanduser(value), *parts)) -def tool_manager_of(bin_dir: str) -> str: - """Return "uv" or "pipx" if bin_dir is a manager's per-tool venv bin dir, //bin, else "".""" - normalized = os.path.normpath(bin_dir) - if os.path.basename(normalized) != "bin": - return "" - # is the dir above /bin - root = os.path.dirname(os.path.dirname(normalized)) + "/" - for manager, fragment in _TOOL_ENV_ROOTS: - if root.endswith(fragment): - return manager - data_home = os.environ.get("XDG_DATA_HOME") or os.path.join(os.path.expanduser("~"), ".local", "share") - relocated = ( - ("uv", _env_dir("UV_TOOL_DIR")), - ("pipx", _env_dir("PIPX_HOME", "venvs")), - ("pipx", _env_dir("PIPX_GLOBAL_HOME", "venvs")), - # default roots that are themselves symlinks (tools kept on another disk) - ("uv", os.path.realpath(os.path.join(data_home, "uv", "tools"))), - ("pipx", os.path.realpath(os.path.join(data_home, "pipx", "venvs"))), - ("pipx", os.path.realpath(os.path.join(os.path.expanduser("~"), ".local", "pipx", "venvs"))), - ("pipx", os.path.realpath("/opt/pipx/venvs")), +def _tool_roots() -> tuple[tuple[str, str], ...]: + """(manager, root) for every per-tool venv root of uv and pipx on this machine.""" + home = os.path.expanduser("~") + data_home = os.environ.get("XDG_DATA_HOME") or os.path.join(home, ".local", "share") + roots = ( + ("uv", _env_dir("UV_TOOL_DIR") or os.path.realpath(os.path.join(data_home, "uv", "tools"))), + ("pipx", _env_dir("PIPX_HOME", "venvs") or os.path.realpath(os.path.join(data_home, "pipx", "venvs"))), + ("pipx", _env_dir("PIPX_GLOBAL_HOME", "venvs") or os.path.realpath("/opt/pipx/venvs")), + # pipx before 1.5 kept its venvs outside the data dir + ("pipx", os.path.realpath(os.path.join(home, ".local", "pipx", "venvs"))), ) - for manager, env_root in relocated: - if env_root and root == env_root + "/": - return manager - return "" + return tuple((manager, root) for manager, root in roots if root) + + +def _tool_venv_of(bin_dir: str) -> tuple[str, str, str]: + """(manager, root, package) if bin_dir is a per-tool venv's //bin, else ("", "", ""). + + The package directory name is validated, and callers rebuild any path from + root + package, so a bin_dir from PATH cannot reach another directory. + """ + resolved = os.path.realpath(bin_dir) + if os.path.basename(resolved) != "bin": + return ("", "", "") + venv = os.path.dirname(resolved) + package = os.path.basename(venv) + if not _PACKAGE_DIR_RE.match(package): + return ("", "", "") + parent = os.path.dirname(venv) + for manager, root in _tool_roots(): + if parent == root: + return (manager, root, package) + return ("", "", "") + + +def tool_manager_of(bin_dir: str) -> str: + """Return "uv" or "pipx" if bin_dir is a manager's per-tool venv bin dir, else "".""" + return _tool_venv_of(bin_dir)[0] def _is_tool_manager_env(bin_dir: str) -> bool: @@ -132,13 +146,12 @@ def tool_entrypoints(bin_dir: str) -> set[str] | None: --include-apps). Everything else in that bin dir belongs to dependencies. None if the venv has no readable record. """ - # bin_dir comes from PATH. Read only inside a manager's own tool root, and - # only that manager's record name, so no other file can be reached. - manager = tool_manager_of(bin_dir) + # bin_dir comes from PATH, so the path that gets opened is rebuilt from a + # known tool root, a validated package dir name and a fixed record name. + manager, root, package = _tool_venv_of(bin_dir) if not manager: return None - venv = os.path.dirname(os.path.realpath(bin_dir)) - record = os.path.join(venv, "uv-receipt.toml" if manager == "uv" else "pipx_metadata.json") + record = os.path.join(root, package, _TOOL_RECORDS[manager]) if not os.path.isfile(record): return None try: diff --git a/tests/integration/test_e2e_install.py b/tests/integration/test_e2e_install.py index 854d34d..627167a 100644 --- a/tests/integration/test_e2e_install.py +++ b/tests/integration/test_e2e_install.py @@ -4,27 +4,25 @@ Tests complete installation scenarios from detection through execution. """ +import shutil import sys -import pytest import tempfile -import shutil from pathlib import Path -from unittest.mock import patch, MagicMock +from unittest.mock import MagicMock, patch + +import pytest # Skip marker for Windows (rollback scripts use Unix paths and shell syntax) -skip_on_windows = pytest.mark.skipif( - sys.platform == "win32", - reason="Rollback scripts use Unix paths (/tmp) and shell syntax" -) +skip_on_windows = pytest.mark.skipif(sys.platform == "win32", reason="Rollback scripts use Unix paths (/tmp) and shell syntax") -from cli_audit import ( - install_tool, - bulk_install, +from cli_audit import ( # noqa: E402 (imported after the skip marker) + BulkInstallResult, Config, Environment, InstallResult, - BulkInstallResult, ToolSpec, + bulk_install, + install_tool, ) @@ -32,7 +30,7 @@ class TestSingleToolInstallation: """Integration tests for single tool installation.""" @patch("cli_audit.installer.subprocess.run") - @patch("cli_audit.installer.shutil.which") + @patch("cli_audit.installer._which") @patch("cli_audit.package_managers.subprocess.run") def test_install_python_tool_with_pipx(self, mock_pm_run, mock_which, mock_run): """Test installing a Python tool using pipx.""" @@ -71,7 +69,7 @@ def test_install_python_tool_with_pipx(self, mock_pm_run, mock_which, mock_run): assert len(result.steps_completed) > 0 @patch("cli_audit.installer.subprocess.run") - @patch("cli_audit.installer.shutil.which") + @patch("cli_audit.installer._which") @patch("cli_audit.package_managers.subprocess.run") def test_install_rust_tool_with_cargo(self, mock_pm_run, mock_which, mock_run): """Test installing a Rust tool using cargo.""" @@ -113,8 +111,8 @@ def test_install_with_retry_on_network_failure(self, mock_run): MagicMock(returncode=0, stdout="Success", stderr=""), ] - from cli_audit.installer import execute_step_with_retry from cli_audit.install_plan import InstallStep + from cli_audit.installer import execute_step_with_retry step = InstallStep("Download package", ("curl", "-O", "package.tar.gz")) result = execute_step_with_retry(step, max_retries=3) @@ -169,6 +167,7 @@ def mock_install_fn(tool_name, **kwargs): @patch("cli_audit.bulk.install_tool") def test_bulk_install_with_fail_fast(self, mock_install): """Test bulk installation with fail-fast enabled.""" + # First tool succeeds, second fails, third should be skipped def mock_install_fn(tool_name, **kwargs): if tool_name == "fd": @@ -218,7 +217,7 @@ class TestDependencyResolution: def test_resolve_dependencies_simple_chain(self): """Test resolving simple dependency chain.""" - from cli_audit.bulk import resolve_dependencies, ToolSpec + from cli_audit.bulk import ToolSpec, resolve_dependencies specs = [ ToolSpec("tool_a", "tool_a", dependencies=()), @@ -236,7 +235,7 @@ def test_resolve_dependencies_simple_chain(self): def test_resolve_dependencies_parallel(self): """Test resolving parallel dependencies.""" - from cli_audit.bulk import resolve_dependencies, ToolSpec + from cli_audit.bulk import ToolSpec, resolve_dependencies specs = [ ToolSpec("tool_a", "tool_a", dependencies=()), @@ -261,6 +260,7 @@ class TestRollbackScenarios: @patch("cli_audit.bulk.install_tool") def test_atomic_rollback_on_failure(self, mock_install, mock_generate, mock_execute): """Test atomic rollback when installation fails.""" + # First tool succeeds, second fails def mock_install_fn(tool_name, **kwargs): if tool_name == "tool_b": @@ -308,7 +308,7 @@ class TestConfigurationIntegration: def test_config_with_custom_preferences(self): """Test installation with custom preferences.""" - from cli_audit.config import Preferences, BulkPreferences + from cli_audit.config import BulkPreferences, Preferences # Create config with custom preferences bulk_prefs = BulkPreferences( diff --git a/tests/test_detection_venv.py b/tests/test_detection_venv.py index adc2ee7..4d26014 100644 --- a/tests/test_detection_venv.py +++ b/tests/test_detection_venv.py @@ -41,6 +41,15 @@ def _make_bin(bin_dir: Path, name: str, version: str) -> Path: return binary +@pytest.fixture(autouse=True) +def tool_roots_in_tmp(tmp_path, monkeypatch): + """Point uv's and pipx's roots at tmp_path, as they are per machine.""" + monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path / "share")) + monkeypatch.setenv("UV_TOOL_DIR", str(tmp_path / "uv" / "tools")) + monkeypatch.setenv("PIPX_HOME", str(tmp_path / "pipx")) + monkeypatch.setenv("PIPX_GLOBAL_HOME", str(tmp_path / "global-pipx")) + + def _make_tool_venv(root: Path, *entrypoints: str) -> Path: """A uv/pipx per-tool venv whose own record lists these entry points.""" bin_dir = _make_venv(root) @@ -166,6 +175,10 @@ def test_reconcile_keeps_tool_manager_installation(tmp_path, monkeypatch, tool_e (tmp_path / "linked").symlink_to(tmp_path / "real-tools") tool_env = tool_env.replace("linked/", "real-tools/") monkeypatch.setenv("UV_TOOL_DIR", str(tmp_path / "linked")) + if tool_env.startswith("share/"): + # reached through XDG_DATA_HOME, so no variable may shadow it + monkeypatch.delenv("UV_TOOL_DIR", raising=False) + monkeypatch.delenv("PIPX_HOME", raising=False) real = _make_bin(_make_tool_venv(tmp_path / tool_env, "fakeuvtool"), "fakeuvtool", "26.5.1") local_bin = tmp_path / "local" / "bin" local_bin.mkdir(parents=True) @@ -198,7 +211,6 @@ def test_uninstall_names_the_package_and_scope(tmp_path, monkeypatch, layout, me from cli_audit.reconcile import Installation, _uninstall_installation - monkeypatch.setenv("PIPX_GLOBAL_HOME", str(tmp_path / "global-pipx")) monkeypatch.setattr(os, "geteuid", lambda: euid, raising=False) inst = Installation(tool=layout.split("/")[-1], version="1", method=method, path=str(tmp_path / layout), active=False) with patch("cli_audit.reconcile.subprocess.run", return_value=MagicMock(returncode=0)) as run: @@ -212,7 +224,6 @@ def test_global_pipx_removal_is_manual_for_a_normal_user(tmp_path, monkeypatch): from cli_audit.reconcile import Installation, _is_manual_removal_error, _uninstall_installation - monkeypatch.setenv("PIPX_GLOBAL_HOME", str(tmp_path / "global-pipx")) monkeypatch.setattr(os, "geteuid", lambda: 1000, raising=False) path = tmp_path / "global-pipx" / "venvs" / "httpie" / "bin" / "http" inst = Installation(tool="httpie", version="1", method="pipx", path=str(path), active=False) @@ -227,8 +238,6 @@ def test_global_pipx_removal_is_manual_for_a_normal_user(tmp_path, monkeypatch): def test_reinstall_hint_names_package_crate_and_scope(tmp_path, monkeypatch): from cli_audit.reconcile import Installation, _reinstall_hint - monkeypatch.setenv("PIPX_GLOBAL_HOME", str(tmp_path / "global-pipx")) - def inst(tool, method, path): return Installation(tool=tool, version="1", method=method, path=str(path), active=False) @@ -247,7 +256,10 @@ def test_tool_manager_needs_package_bin_layout(tmp_path, monkeypatch): from cli_audit.detection import tool_manager_of monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.delenv("UV_TOOL_DIR", raising=False) monkeypatch.setenv("PIPX_HOME", "~/pipx-home") # literal ~, as a systemd unit or .env passes it + (tmp_path / "share/uv/tools/black/bin").mkdir(parents=True) + (tmp_path / "pipx-home/venvs/httpie/bin").mkdir(parents=True) assert tool_manager_of(str(tmp_path / "share/uv/tools/black/bin")) == "uv" assert tool_manager_of(str(tmp_path / "pipx-home/venvs/httpie/bin")) == "pipx" # not //bin @@ -258,6 +270,7 @@ def test_tool_manager_needs_package_bin_layout(tmp_path, monkeypatch): def test_tool_bin_dir_directly_on_path_is_kept(tmp_path, monkeypatch): from cli_audit.reconcile import clear_detection_cache, detect_installations + monkeypatch.delenv("UV_TOOL_DIR", raising=False) tool_bin = _make_tool_venv(tmp_path / "share" / "uv" / "tools" / "fakedirect", "fakedirect") real = _make_bin(tool_bin, "fakedirect", "1.0.0") monkeypatch.setenv("PATH", str(tool_bin)) @@ -322,6 +335,7 @@ def test_dependency_executable_in_a_tool_venv_is_no_installation(tmp_path, monke # installation would let reconcile run `uv tool uninstall httpie` for it. from cli_audit.reconcile import clear_detection_cache, detect_installations + monkeypatch.delenv("UV_TOOL_DIR", raising=False) tool_bin = _make_tool_venv(tmp_path / "share" / "uv" / "tools" / "fakehttpie", "fakehttp") _make_bin(tool_bin, "fakehttp", "3.2.4") _make_bin(tool_bin, "fakepygmentize", "2.19.0") @@ -352,6 +366,7 @@ def test_dependency_copy_answers_no_lookup(tmp_path, monkeypatch): # A tool venv bin dir first on PATH (uv tool run): its dependency copies # must not answer for the audit, the bulk check, the install validation, # the version_command, or reconcile + monkeypatch.delenv("UV_TOOL_DIR", raising=False) tool_bin = _make_tool_venv(tmp_path / "share" / "uv" / "tools" / "fakehttpie2", "fakehttp2") _make_bin(tool_bin, "fakedep", "2.19.0") real_bin = tmp_path / "local" / "bin" @@ -373,9 +388,28 @@ def test_tool_venv_is_not_used_to_resolve_a_command(tmp_path, monkeypatch): # version_command runs outside every environment from cli_audit.detection import _command_path, _installation_path + monkeypatch.delenv("UV_TOOL_DIR", raising=False) tool_bin = _make_tool_venv(tmp_path / "share" / "uv" / "tools" / "fakeonly", "fakeonly") _make_bin(tool_bin, "fakeonly", "1.0.0") monkeypatch.setenv("PATH", str(tool_bin)) assert _installation_path() == str(tool_bin) assert _command_path() == "" + + +@pytest.mark.parametrize("package", ["weird name", ".hidden", "-rf"], ids=["space", "dot", "dash"]) +def test_odd_package_dir_name_is_no_tool_venv(tmp_path, monkeypatch, package): + # No uv/pipx package is named like this; the record path is built from this + # name, so only a plain package name is accepted + from cli_audit.detection import _tool_venv_of + + monkeypatch.delenv("UV_TOOL_DIR", raising=False) + bin_dir = tmp_path / "share" / "uv" / "tools" / package / "bin" + bin_dir.mkdir(parents=True) + + assert _tool_venv_of(str(bin_dir)) == ("", "", "") + assert _tool_venv_of(str(tmp_path / "share" / "uv" / "tools" / "black" / "bin")) == ( + "uv", + str(tmp_path / "share" / "uv" / "tools"), + "black", + ) From 3e3c4ecbad18b3818a5a477988af06350e597a28 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Mon, 21 Sep 2026 16:04:17 +0200 Subject: [PATCH 14/17] test(integration): stub the prerequisite lookup, not only the installer's The two install tests patched cli_audit.installer.shutil.which, which set the attribute on the shutil module itself and therefore also answered for prerequisites.check_prerequisites. Patching the installer's own _which left that check on the real machine: where the package manager is missing, it asks "Install pipx now?" and pytest aborts with "reading from stdin while output is captured". All three CI platforms failed that way; locally the tests passed because the binaries exist. Reproduced with PATH=/usr/bin:/bin: without the added patch the same OSError, with it 10 passed. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- tests/integration/test_e2e_install.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_e2e_install.py b/tests/integration/test_e2e_install.py index 627167a..c6a615f 100644 --- a/tests/integration/test_e2e_install.py +++ b/tests/integration/test_e2e_install.py @@ -30,9 +30,10 @@ class TestSingleToolInstallation: """Integration tests for single tool installation.""" @patch("cli_audit.installer.subprocess.run") + @patch("cli_audit.prerequisites.shutil.which", return_value="/usr/bin/python3") @patch("cli_audit.installer._which") @patch("cli_audit.package_managers.subprocess.run") - def test_install_python_tool_with_pipx(self, mock_pm_run, mock_which, mock_run): + def test_install_python_tool_with_pipx(self, mock_pm_run, mock_which, mock_prereq_which, mock_run): """Test installing a Python tool using pipx.""" # Setup: pipx is available mock_pm_run.return_value = MagicMock(returncode=0) @@ -69,9 +70,10 @@ def test_install_python_tool_with_pipx(self, mock_pm_run, mock_which, mock_run): assert len(result.steps_completed) > 0 @patch("cli_audit.installer.subprocess.run") + @patch("cli_audit.prerequisites.shutil.which", return_value="/usr/bin/python3") @patch("cli_audit.installer._which") @patch("cli_audit.package_managers.subprocess.run") - def test_install_rust_tool_with_cargo(self, mock_pm_run, mock_which, mock_run): + def test_install_rust_tool_with_cargo(self, mock_pm_run, mock_which, mock_prereq_which, mock_run): """Test installing a Rust tool using cargo.""" # Setup: cargo is available mock_pm_run.return_value = MagicMock(returncode=0) From d5e025190c6b9434f64401abaf6b0f4cfb65d0d3 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 22 Sep 2026 07:37:23 +0200 Subject: [PATCH 15/17] fix(audit): one rule for foreign binaries, and keep the lookup cheap Review round 9: - `pipx install --include-deps ` links a dependency's apps on purpose and records them as apps_of_dependencies. They were treated as dependency copies, so such a tool (ansible-playbook) became "not installed" and a reinstall candidate. Those apps count now. - A symlink from an ordinary PATH dir into a venv (~/.local/bin/tool -> ~/proj/.venv/bin/tool) was rejected by reconcile but accepted by the audit. _is_foreign_binary now answers that question for both: the resolved dir is an environment, or the binary is a dependency copy in a tool venv. - The filtered PATH was rebuilt per lookup, and the filter stats every PATH entry. On this WSL host (128 entries, 37 of them /mnt/c) that was 42.7 ms per tool, about 5 s over a 117-tool audit. It is memoised on the PATH string, and _which asks once before walking the dirs: 2.9 ms per tool, about 0.34 s over 117 tools. - Tool roots also cover pipx's macOS home (platformdirs: ~/Library/Application Support/pipx), and XDG_DATA_HOME is expanded like the other variables. - _is_pipx_global resolves the path it compares, so a symlinked /opt still takes the sudo route. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- cli_audit/detection.py | 49 ++++++++++++++++++++++++------- cli_audit/reconcile.py | 7 ++--- tests/test_detection_venv.py | 57 ++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 15 deletions(-) diff --git a/cli_audit/detection.py b/cli_audit/detection.py index 9c9f68e..a63f773 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -13,6 +13,7 @@ import shutil import subprocess import tomllib +from functools import lru_cache from typing import Sequence # Constants @@ -97,13 +98,15 @@ def _env_dir(name: str, *parts: str) -> str: def _tool_roots() -> tuple[tuple[str, str], ...]: """(manager, root) for every per-tool venv root of uv and pipx on this machine.""" home = os.path.expanduser("~") - data_home = os.environ.get("XDG_DATA_HOME") or os.path.join(home, ".local", "share") + data_home = os.path.expanduser(os.environ.get("XDG_DATA_HOME") or "") or os.path.join(home, ".local", "share") roots = ( ("uv", _env_dir("UV_TOOL_DIR") or os.path.realpath(os.path.join(data_home, "uv", "tools"))), ("pipx", _env_dir("PIPX_HOME", "venvs") or os.path.realpath(os.path.join(data_home, "pipx", "venvs"))), ("pipx", _env_dir("PIPX_GLOBAL_HOME", "venvs") or os.path.realpath("/opt/pipx/venvs")), # pipx before 1.5 kept its venvs outside the data dir ("pipx", os.path.realpath(os.path.join(home, ".local", "pipx", "venvs"))), + # pipx >= 1.5 asks platformdirs, which answers differently on macOS + ("pipx", os.path.realpath(os.path.join(home, "Library", "Application Support", "pipx", "venvs"))), ) return tuple((manager, root) for manager, root in roots if root) @@ -163,7 +166,10 @@ def tool_entrypoints(bin_dir: str) -> set[str] | None: data = json.load(f) packages = [data.get("main_package") or {}] packages += [p for p in (data.get("injected_packages") or {}).values() if p.get("include_apps")] - return {app for p in packages for app in (p.get("apps") or [])} + apps = {app for p in packages for app in (p.get("apps") or [])} + # `pipx install --include-deps` links a dependency's apps on purpose + apps |= {app for p in packages if p.get("include_dependencies") for app in (p.get("apps_of_dependencies") or [])} + return apps except (OSError, ValueError, AttributeError, TypeError, KeyError) as exc: # Unreadable record: every executable in that venv then counts as a # dependency, so say which file and why @@ -186,6 +192,17 @@ def _is_tool_dependency_binary(path: str) -> bool: return names is None or os.path.basename(real) not in names +def _is_foreign_binary(path: str) -> bool: + """True if path is no installation: it resolves into an environment, or it is + a dependency's executable inside a tool manager's per-tool venv. + + The audit and reconcile both ask this, so a symlink from an ordinary PATH + dir into a venv is judged the same way on both sides. + """ + real = os.path.realpath(path) + return _is_environment_bin(os.path.dirname(real)) or _is_tool_dependency_binary(real) + + def _is_environment_bin(bin_dir: str) -> bool: """True if bin_dir is an environment's bin dir and no tool manager's per-tool venv. @@ -195,6 +212,13 @@ def _is_environment_bin(bin_dir: str) -> bool: return _is_virtualenv_bin(bin_dir) and not _is_tool_manager_env(os.path.realpath(bin_dir)) +@lru_cache(maxsize=8) +def _filter_path(path_env: str, strict: bool) -> str: + dirs = [d for d in path_env.split(os.pathsep) if d] + keep = _is_virtualenv_bin if strict else _is_environment_bin + return os.pathsep.join(d for d in dirs if not keep(d)) + + def _command_path() -> str: """PATH for running a tool by its name: no environment bin dirs at all. @@ -202,8 +226,7 @@ def _command_path() -> str: running a name there can hit a dependency's executable (pygmentize in httpie's venv) instead of the installation. """ - dirs = [d for d in os.environ.get("PATH", os.defpath).split(os.pathsep) if d] - return os.pathsep.join(d for d in dirs if not _is_virtualenv_bin(d)) + return _filter_path(os.environ.get("PATH", os.defpath), True) def _installation_path() -> str: @@ -213,8 +236,7 @@ def _installation_path() -> str: lookup reports the environment's copy (e.g. ~/.venv/bin/black) and an upgrade of the real installation never shows up in the audit. """ - dirs = [d for d in os.environ.get("PATH", os.defpath).split(os.pathsep) if d] - return os.pathsep.join(d for d in dirs if not _is_environment_bin(d)) + return _filter_path(os.environ.get("PATH", os.defpath), False) def _which(command_name: str) -> str | None: @@ -223,10 +245,15 @@ def _which(command_name: str) -> str | None: Skips a dependency's executable inside a uv/pipx per-tool venv and keeps searching the next PATH dir. """ - for path_dir in _installation_path().split(os.pathsep): - found = shutil.which(command_name, path=path_dir) if path_dir else None - if found and not _is_tool_dependency_binary(found): - return found + search_path = _installation_path() + found = shutil.which(command_name, path=search_path) + if not found or not _is_foreign_binary(found): + return found + # rare: keep looking in the dirs after the one that answered + for path_dir in search_path.split(os.pathsep): + other = shutil.which(command_name, path=path_dir) if path_dir else None + if other and not _is_foreign_binary(other): + return other return None @@ -264,7 +291,7 @@ def find_paths(command_name: str, deep: bool = False) -> list[str]: for line in (proc.stdout or "").splitlines(): line = line.strip() if line and os.path.isfile(line) and os.access(line, os.X_OK): - if line not in paths and not _is_tool_dependency_binary(line): + if line not in paths and not _is_foreign_binary(line): paths.append(line) except Exception: pass diff --git a/cli_audit/reconcile.py b/cli_audit/reconcile.py index 31d76f3..9e30771 100644 --- a/cli_audit/reconcile.py +++ b/cli_audit/reconcile.py @@ -25,7 +25,7 @@ from .detection import ( _env_dir, _is_environment_bin, - _is_tool_dependency_binary, + _is_foreign_binary, _which, tool_manager_of, ) @@ -255,8 +255,7 @@ def detect_installations( continue # A symlink can point into an environment as well - real_dir = os.path.dirname(real_path) - if _is_environment_bin(real_dir) or _is_tool_dependency_binary(real_path): + if _is_foreign_binary(real_path): vlog(f" Skipping environment binary: {real_path}", verbose) continue @@ -1057,7 +1056,7 @@ def _tool_env_package(path: str, tool: str) -> str: def _is_pipx_global(path: str) -> bool: """True if path lies in pipx's global venvs (`pipx install --global`).""" root = _env_dir("PIPX_GLOBAL_HOME", "venvs") or os.path.realpath("/opt/pipx/venvs") - return os.path.normpath(path).startswith(root + "/") + return os.path.realpath(path).startswith(root + "/") def _reinstall_hint(installation: Installation) -> str: diff --git a/tests/test_detection_venv.py b/tests/test_detection_venv.py index 4d26014..12af586 100644 --- a/tests/test_detection_venv.py +++ b/tests/test_detection_venv.py @@ -413,3 +413,60 @@ def test_odd_package_dir_name_is_no_tool_venv(tmp_path, monkeypatch, package): str(tmp_path / "share" / "uv" / "tools"), "black", ) + + +def test_pipx_include_deps_app_is_an_installation(tmp_path, monkeypatch): + # `pipx install --include-deps ansible` links ansible-playbook on purpose + venv = tmp_path / "pipx" / "venvs" / "fakeansible" + bin_dir = _make_venv(venv) + (venv / "pipx_metadata.json").write_text( + json.dumps( + { + "main_package": { + "apps": ["fakeansible"], + "include_dependencies": True, + "apps_of_dependencies": ["fakeansible-playbook"], + } + } + ) + ) + _make_bin(bin_dir, "fakeansible", "2.21.4") + real = _make_bin(bin_dir, "fakeansible-playbook", "2.21.4") + _make_bin(bin_dir, "fakedeponly", "1.0.0") + local_bin = tmp_path / "local" / "bin" + local_bin.mkdir(parents=True) + (local_bin / "fakeansible-playbook").symlink_to(real) + monkeypatch.setenv("PATH", os.pathsep.join([str(local_bin), WHICH_DIR])) + + assert find_paths("fakeansible-playbook") == [str(local_bin / "fakeansible-playbook")] + assert get_missing_tools(["fakeansible-playbook"]) == [] + # a dependency the manager did not expose stays out + monkeypatch.setenv("PATH", os.pathsep.join([str(bin_dir), WHICH_DIR])) + assert find_paths("fakedeponly") == [] + + +def test_symlink_from_path_dir_into_a_venv_is_no_installation(tmp_path, monkeypatch): + # ~/.local/bin/tool -> ~/proj/.venv/bin/tool: the audit and reconcile must agree + from cli_audit.reconcile import clear_detection_cache, detect_installations + + venv_bin = _make_venv(tmp_path / "proj" / ".venv") + venv_copy = _make_bin(venv_bin, "fakelinked2", "1.0.0") + local_bin = tmp_path / "local" / "bin" + local_bin.mkdir(parents=True) + (local_bin / "fakelinked2").symlink_to(venv_copy) + monkeypatch.setenv("PATH", os.pathsep.join([str(local_bin), WHICH_DIR])) + clear_detection_cache() + + assert find_paths("fakelinked2", deep=True) == [] + assert detect_installations("fakelinked2", ["fakelinked2"]) == [] + + +def test_filtered_path_is_computed_once_per_path(monkeypatch): + # the filter stats every PATH entry; a WSL PATH makes that expensive + import cli_audit.detection as detection + + monkeypatch.setenv("PATH", os.pathsep.join(["/usr/bin", "/bin"])) + detection._filter_path.cache_clear() + detection._installation_path() + detection._installation_path() + assert detection._filter_path.cache_info().hits >= 1 From 253eef9b14decbc0004a5140845d4b27ded9becd Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 22 Sep 2026 07:46:01 +0200 Subject: [PATCH 16/17] fix(audit): resolve a PATH entry before classifying it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A PATH entry can be a symlink to an environment's bin dir (~/current-bin -> ~/proj/.venv/bin). normpath does not follow it, so that dir stayed on the filtered PATH and a version_command could still resolve its tool to the venv copy. _is_virtualenv_bin resolves the dir first; the tool-venv exemption is unaffected, because it already compares resolved paths. Cost on this WSL host (128 PATH entries): the filtered PATH is built once per PATH value in 225 ms, then 0.07 ms per lookup — about 233 ms over a 117-tool audit. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- cli_audit/detection.py | 6 ++++-- tests/test_detection_venv.py | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/cli_audit/detection.py b/cli_audit/detection.py index a63f773..8910815 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -66,8 +66,10 @@ def _is_virtualenv_bin(bin_dir: str) -> bool: and classifying them by method (e.g. `uv` because the tool also appears in `uv tool list`) makes removal delete a DIFFERENT installation. """ - # "/x/env/bin/" must behave like "/x/env/bin" (dirname would stay in bin/) - bin_dir = os.path.normpath(bin_dir) + # Resolve first: a PATH entry can be a symlink to an environment's bin dir + # (~/bin -> ~/proj/.venv/bin), and normpath alone would not see the venv. + # This also makes "/x/env/bin/" behave like "/x/env/bin". + bin_dir = os.path.realpath(os.path.expanduser(bin_dir)) # Definitive signal: PEP 405 venvs carry pyvenv.cfg next to bin/ if os.path.isfile(os.path.join(os.path.dirname(bin_dir), "pyvenv.cfg")): return True diff --git a/tests/test_detection_venv.py b/tests/test_detection_venv.py index 12af586..2fd7136 100644 --- a/tests/test_detection_venv.py +++ b/tests/test_detection_venv.py @@ -470,3 +470,20 @@ def test_filtered_path_is_computed_once_per_path(monkeypatch): detection._installation_path() detection._installation_path() assert detection._filter_path.cache_info().hits >= 1 + + +def test_symlinked_path_dir_into_a_venv_is_skipped(tmp_path, monkeypatch): + # PATH holds ~/current-bin -> ~/proj/.venv/bin + from cli_audit.detection import _command_path, _installation_path + + venv_bin = _make_venv(tmp_path / "proj" / ".venv") + _make_bin(venv_bin, "fakelinked3", "1.0.0") + link = tmp_path / "current-bin" + link.symlink_to(venv_bin) + other_bin = tmp_path / "other" / "bin" + real = _make_bin(other_bin, "fakelinked3", "2.0.0") + monkeypatch.setenv("PATH", os.pathsep.join([str(link), str(other_bin), WHICH_DIR])) + + assert str(link) not in _installation_path() + assert str(link) not in _command_path() + assert find_paths("fakelinked3") == [str(real)] From 0d35bf2f14c69f2d5d44d6dbe5136bb8bf8f95d9 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Tue, 22 Sep 2026 08:02:27 +0200 Subject: [PATCH 17/17] refactor(reconcile): take the audit's filtered PATH instead of its own copy Review round 10, no bug findings; these are the minor ones. - detect_installations split the raw PATH and filtered it entry by entry, so it paid the per-entry stats the memoised filter exists to avoid and kept a second copy of the rule. It uses _installation_path() now: same predicate, and the same PATH source _which already used, so with PATH unset both fall back to os.defpath where the listing used to find nothing. No test pins this; the cost is what moves. - The comment claiming _ENV_DIR_PATTERNS mirrors capability.sh was wrong: the shell side matches the unresolved path and skips every */venvs/*/bin, so it drops pipx per-tool venvs the Python side keeps. The comment says that instead of claiming a mirror. - _PACKAGE_DIR_RE also accepts "@", which pipx --suffix can put in a venv dir name (still no separator). - The rare fallback walk no longer re-scans the dir that already answered, and the filter predicate is no longer called "keep" while meaning the opposite. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_017qjwSFFBZpMj3uw5bPdrnD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel --- cli_audit/detection.py | 15 ++++++++++----- cli_audit/reconcile.py | 13 ++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cli_audit/detection.py b/cli_audit/detection.py index 8910815..29bde9f 100644 --- a/cli_audit/detection.py +++ b/cli_audit/detection.py @@ -43,7 +43,8 @@ VERSION_COMMAND_PATH = "" # Environment-name patterns for env managers without a pyvenv.cfg (conda etc.). -# Mirrors the venv skip list in scripts/lib/capability.sh:detect_all_installations. +# scripts/lib/capability.sh has a similar list, but it matches the unresolved +# path and skips every */venvs/*/bin, so the two no longer agree by construction. _ENV_DIR_PATTERNS = ( "/venv/bin/", "/.venv/bin/", @@ -84,7 +85,7 @@ def _is_virtualenv_bin(bin_dir: str) -> bool: _TOOL_RECORDS = {"uv": "uv-receipt.toml", "pipx": "pipx_metadata.json"} # A package directory name: no separator, no "..", so it cannot leave the root -_PACKAGE_DIR_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9._+-]*\Z") +_PACKAGE_DIR_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9._+@-]*\Z") def _env_dir(name: str, *parts: str) -> str: @@ -217,8 +218,8 @@ def _is_environment_bin(bin_dir: str) -> bool: @lru_cache(maxsize=8) def _filter_path(path_env: str, strict: bool) -> str: dirs = [d for d in path_env.split(os.pathsep) if d] - keep = _is_virtualenv_bin if strict else _is_environment_bin - return os.pathsep.join(d for d in dirs if not keep(d)) + is_foreign_dir = _is_virtualenv_bin if strict else _is_environment_bin + return os.pathsep.join(d for d in dirs if not is_foreign_dir(d)) def _command_path() -> str: @@ -252,7 +253,11 @@ def _which(command_name: str) -> str | None: if not found or not _is_foreign_binary(found): return found # rare: keep looking in the dirs after the one that answered - for path_dir in search_path.split(os.pathsep): + dirs = search_path.split(os.pathsep) + answered = os.path.dirname(found) + if answered in dirs: + dirs = dirs[dirs.index(answered) + 1 :] + for path_dir in dirs: other = shutil.which(command_name, path=path_dir) if path_dir else None if other and not _is_foreign_binary(other): return other diff --git a/cli_audit/reconcile.py b/cli_audit/reconcile.py index 9e30771..cfbce91 100644 --- a/cli_audit/reconcile.py +++ b/cli_audit/reconcile.py @@ -24,7 +24,7 @@ from .config import Config from .detection import ( _env_dir, - _is_environment_bin, + _installation_path, _is_foreign_binary, _which, tool_manager_of, @@ -224,17 +224,12 @@ def detect_installations( installations = [] seen_paths = set() - # Get PATH directories - path_env = os.environ.get("PATH", "") - path_dirs = [d for d in path_env.split(os.pathsep) if d] + # PATH without environment bin dirs — the audit's own filtered PATH, so + # both sides apply one rule and pay for the filtering once + path_dirs = [d for d in _installation_path().split(os.pathsep) if d] # Search each PATH directory for path_dir in path_dirs: - # Virtualenv/conda bins are environments, not installations (a uv/pipx - # per-tool bin dir put on PATH directly is an installation) - if _is_environment_bin(path_dir): - vlog(f" Skipping environment dir: {path_dir}", verbose) - continue for candidate in candidates: full_path = os.path.join(path_dir, candidate)