From 1e9867e67daec757d8bc48a778176fb881c7d804 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Wed, 8 Jul 2026 09:26:53 +0200 Subject: [PATCH] fix: resolve poetry upstream, flag it in health check, use raw release tags Three related upstream/version-correctness fixes: - poetry showed 'latest: n/a' because _derive_source() returned 'skip' (homepage python-poetry.org matches neither python.org nor pypi.org, and install_method package_manager carries no pip/uv_tool hint). Support an explicit source_kind in the catalog and set poetry.json to pypi. - The Python package-manager health check only knew pip/pipx, so uv + poetry reported 'optimal'. Detect poetry and conda too. - GitHub/GitLab release URLs were built from the normalized version (.../releases/tag/1.7.12) but the real tag is v1.7.12 -> dead links. The collectors now return the raw tag for the URL while still normalizing the version number for display/compare. make update also hyperlinks the version. Signed-off-by: Sebastian Mendel --- audit.py | 7 +- catalog/poetry.json | 1 + cli_audit/catalog.py | 16 ++++ cli_audit/collectors.py | 43 ++++----- cli_audit/local_state.py | 6 +- cli_audit/tools.py | 6 ++ scripts/check_python_package_managers.sh | 26 +++++- tests/test_local_state.py | 5 +- tests/test_upstream_links.py | 109 +++++++++++++++++++++++ 9 files changed, 193 insertions(+), 26 deletions(-) create mode 100644 tests/test_upstream_links.py diff --git a/audit.py b/audit.py index 98f4eb8..9e70751 100755 --- a/audit.py +++ b/audit.py @@ -29,7 +29,7 @@ from cli_audit.tools import Tool, all_tools, filter_tools, tool_homepage_url, latest_target_url # noqa: E402 from cli_audit.detection import audit_tool_installation, detect_multi_versions # noqa: E402 from cli_audit.snapshot import load_snapshot, write_snapshot, render_from_snapshot, get_snapshot_path # noqa: E402 -from cli_audit.render import render_table, print_summary, status_icon # noqa: E402 +from cli_audit.render import render_table, print_summary, status_icon, osc8 # noqa: E402 from cli_audit.pins import lookup_pin, should_skip as _pin_should_skip # noqa: E402 from cli_audit.collectors import get_github_rate_limit, get_github_rate_limit_help, get_gitlab_rate_limit, is_wsl, collect_endoflife # noqa: E402 from cli_audit import collectors # noqa: E402 @@ -641,6 +641,11 @@ def cmd_update(args: argparse.Namespace) -> int: marker_str = f" [{' '.join(markers)}]" if markers else "" inst_fmt = f"{inst_color}{inst_display}{RESET}" latest_fmt = f"{latest_color}{latest_display}{RESET}" + # Hyperlink the latest version to its release/page URL + # (osc8 self-disables when CLI_AUDIT_LINKS=0 or no URL) + latest_link = result.get("latest_url", "") + if latest_link and latest_display != "n/a": + latest_fmt = osc8(latest_link, latest_fmt) msg = f"# [{completed}/{total}] [{cat}] {tool.name} (installed: {inst_fmt} {op} latest: {latest_fmt}){marker_str}" print(msg, file=sys.stderr, flush=True) diff --git a/catalog/poetry.json b/catalog/poetry.json index b0bec0d..615e5d1 100644 --- a/catalog/poetry.json +++ b/catalog/poetry.json @@ -6,6 +6,7 @@ "homepage": "https://python-poetry.org/", "package_name": "poetry", "binary_name": "poetry", + "source_kind": "pypi", "packages": { "apt": "python3-poetry", "brew": "poetry", diff --git a/cli_audit/catalog.py b/cli_audit/catalog.py index c3ce953..232f0a4 100644 --- a/cli_audit/catalog.py +++ b/cli_audit/catalog.py @@ -68,6 +68,22 @@ def _derive_source(self) -> tuple[str, tuple[str, ...]]: if self._raw_data and self._raw_data.get("skip_upstream"): return ("skip", ()) + # Priority 0a: Explicit source declaration in the catalog. Lets a tool + # whose homepage/install_method don't auto-route (e.g. poetry, whose + # homepage is python-poetry.org and install_method is package_manager) + # still declare its upstream, e.g. {"source_kind": "pypi"}. + explicit_kind = self._raw_data.get("source_kind") if self._raw_data else None + if explicit_kind: + explicit_args = self._raw_data.get("source_args") + if explicit_args: + # A single string must not be split into characters by tuple(). + if isinstance(explicit_args, str): + return (explicit_kind, (explicit_args,)) + return (explicit_kind, tuple(explicit_args)) + if self.package_name: + return (explicit_kind, (self.package_name,)) + return (explicit_kind, ()) + # Priority 0b: Skip version checking for pure package_manager tools # These are OS-managed and can't be manually upgraded if self.install_method == "package_manager" and not self.github_repo and not self.gitlab_project and not self.package_name: diff --git a/cli_audit/collectors.py b/cli_audit/collectors.py index 166e233..05b46c1 100644 --- a/cli_audit/collectors.py +++ b/cli_audit/collectors.py @@ -155,22 +155,24 @@ def collect_github(owner: str, repo: str, offline_cache: dict[str, tuple[str, st last_segment = final_url.rsplit("/", 1)[-1] if last_segment and last_segment.lower() not in ("releases", "latest"): - tag = normalize_version_tag(last_segment) - version = extract_version_number(tag) - logger.debug(f"GitHub {owner}/{repo}: {tag} via redirect") - return tag, version + # Return the RAW tag (e.g. "v1.7.12") so the release URL points + # at the real tag; the version number is normalized separately. + raw_tag = last_segment + version = extract_version_number(raw_tag) + logger.debug(f"GitHub {owner}/{repo}: {raw_tag} via redirect") + return raw_tag, version except Exception as e: logger.debug(f"GitHub redirect failed for {owner}/{repo}: {e}") # Fallback to releases API try: data = json.loads(http_get(f"https://api.github.com/repos/{owner}/{repo}/releases/latest", timeout=3)) - tag = normalize_version_tag(data.get("tag_name", "")) + raw_tag = data.get("tag_name", "") if isinstance(data, dict) else "" - if tag: - version = extract_version_number(tag) - logger.debug(f"GitHub {owner}/{repo}: {tag} via API") - return tag, version + if raw_tag: + version = extract_version_number(raw_tag) + logger.debug(f"GitHub {owner}/{repo}: {raw_tag} via API") + return raw_tag, version except Exception as e: logger.debug(f"GitHub API failed for {owner}/{repo}: {e}") @@ -190,19 +192,20 @@ def collect_github(owner: str, repo: str, offline_cache: dict[str, tuple[str, st if tag and re.match(r"^v?\d+\.\d+(\.\d+)?$", tag): ver = extract_version_number(tag) if ver: - # Parse version as tuple for comparison + # Parse version as tuple for comparison; keep the RAW tag so + # the release URL points at the real tag (e.g. "v3.14.0"). try: nums = tuple(int(x) for x in ver.split(".")) - tup = (nums, tag, ver) + tup = (nums, raw_tag, ver) if best is None or tup[0] > best[0]: best = tup except (ValueError, AttributeError): continue if best is not None: - _, tag, version = best - logger.debug(f"GitHub {owner}/{repo}: {tag} via Atom feed (filtered stable)") - return tag, version + _, raw_tag, version = best + logger.debug(f"GitHub {owner}/{repo}: {raw_tag} via Atom feed (filtered stable)") + return raw_tag, version except Exception as e: logger.debug(f"GitHub Atom feed failed for {owner}/{repo}: {e}") @@ -235,12 +238,12 @@ def collect_gitlab(group: str, project: str, offline_cache: dict[str, tuple[str, url = f"https://gitlab.com/api/v4/projects/{project_path}/releases" data = json.loads(http_get(url)) - if isinstance(data, list) and data: - tag = normalize_version_tag(data[0].get("tag_name", "")) - if tag: - version = extract_version_number(tag) - logger.debug(f"GitLab {group}/{project}: {tag}") - return tag, version + if isinstance(data, list) and data and isinstance(data[0], dict): + raw_tag = data[0].get("tag_name", "") + if raw_tag: + version = extract_version_number(raw_tag) + logger.debug(f"GitLab {group}/{project}: {raw_tag}") + return raw_tag, version except Exception as e: logger.debug(f"GitLab API failed for {group}/{project}: {e}") diff --git a/cli_audit/local_state.py b/cli_audit/local_state.py index b354387..1379211 100644 --- a/cli_audit/local_state.py +++ b/cli_audit/local_state.py @@ -287,8 +287,10 @@ def merge_for_display( "installed_path_selected": loc.installed_path, "classification_reason_selected": loc.classification_reason, "status": loc.status, - # Upstream info (from upstream cache) - "latest_upstream": up.latest_tag, + # Upstream info (from upstream cache). Display the normalized + # version, not latest_tag (which holds the raw tag, e.g. "v1.7.12", + # used for URL construction). + "latest_upstream": up.latest_version or up.latest_tag, "latest_version": up.latest_version, "latest_url": up.latest_url, "tool_url": up.tool_url, diff --git a/cli_audit/tools.py b/cli_audit/tools.py index d78b24c..9868e7c 100644 --- a/cli_audit/tools.py +++ b/cli_audit/tools.py @@ -9,6 +9,7 @@ from __future__ import annotations +import re from dataclasses import dataclass @@ -131,6 +132,11 @@ def latest_target_url(tool: Tool, latest_tag: str, latest_num: str) -> str: Returns: Release URL string """ + # The tag comes from untrusted upstream APIs and is embedded in URLs that + # are later rendered as OSC8 terminal hyperlinks. Strip control characters + # (incl. ESC 0x1b) so a malicious tag can't inject terminal escape sequences. + latest_tag = re.sub(r"[\x00-\x1f\x7f]", "", latest_tag) if latest_tag else latest_tag + if tool.source_kind == "gh" and len(tool.source_args) >= 2: owner, repo = tool.source_args[0], tool.source_args[1] if latest_tag: diff --git a/scripts/check_python_package_managers.sh b/scripts/check_python_package_managers.sh index 681ef68..d3763cc 100755 --- a/scripts/check_python_package_managers.sh +++ b/scripts/check_python_package_managers.sh @@ -13,6 +13,8 @@ NC='\033[0m' # No Color has_pip=false has_pipx=false has_uv=false +has_poetry=false +has_conda=false if command -v pip >/dev/null 2>&1 || command -v pip3 >/dev/null 2>&1; then has_pip=true @@ -26,15 +28,25 @@ if command -v uv >/dev/null 2>&1; then has_uv=true fi +if command -v poetry >/dev/null 2>&1; then + has_poetry=true +fi + +if command -v conda >/dev/null 2>&1; then + has_conda=true +fi + # Count how many are installed count=0 managers=() if $has_pip; then count=$((count + 1)); managers+=("pip"); fi if $has_pipx; then count=$((count + 1)); managers+=("pipx"); fi if $has_uv; then count=$((count + 1)); managers+=("uv"); fi +if $has_poetry; then count=$((count + 1)); managers+=("poetry"); fi +if $has_conda; then count=$((count + 1)); managers+=("conda"); fi # If only uv is installed, all good -if $has_uv && ! $has_pip && ! $has_pipx; then +if $has_uv && ! $has_pip && ! $has_pipx && ! $has_poetry && ! $has_conda; then echo -e "${GREEN}✓ Only uv is installed - optimal configuration!${NC}" exit 0 fi @@ -73,6 +85,18 @@ if [ "$count" -gt 1 ]; then echo "" fi + if $has_poetry; then + echo -e "${YELLOW}poetry is a project-level dependency manager (not a global CLI tool).${NC}" + echo " If it is installed globally but unused, remove it: make uninstall-poetry" + echo "" + fi + + if $has_conda; then + echo -e "${YELLOW}conda manages its own Python environments separately from uv.${NC}" + echo " Keep it only if you rely on conda envs; otherwise deactivate/remove it." + echo "" + fi + echo -e "${BLUE}After migration, you can optionally remove old package managers.${NC}" echo "Note: pip is often bundled with Python and can be left installed for compatibility." echo "" diff --git a/tests/test_local_state.py b/tests/test_local_state.py index 5d7df40..9529f94 100644 --- a/tests/test_local_state.py +++ b/tests/test_local_state.py @@ -519,8 +519,9 @@ def test_merge_for_display_both(self): assert len(result) == 1 tool = result[0] assert tool["tool"] == "ripgrep" - # Upstream data - assert tool["latest_upstream"] == "v15.1.0" + # Upstream data: latest_tag ("v15.1.0") is the raw tag used for URL + # construction; the displayed latest_upstream is the normalized version. + assert tool["latest_upstream"] == "15.1.0" assert tool["latest_version"] == "15.1.0" assert tool["tool_url"] == "https://github.com/BurntSushi/ripgrep" # Local data diff --git a/tests/test_upstream_links.py b/tests/test_upstream_links.py new file mode 100644 index 0000000..505dc94 --- /dev/null +++ b/tests/test_upstream_links.py @@ -0,0 +1,109 @@ +"""Tests for upstream-source fixes: + +- poetry (and any tool) can declare an explicit ``source_kind`` in the catalog +- release URLs use the RAW tag (``v1.7.12``) so GitHub/GitLab links resolve +- the Python-package-manager health check knows about poetry +""" +import json +from pathlib import Path +from unittest.mock import MagicMock, patch + +from cli_audit.catalog import ToolCatalog, ToolCatalogEntry +from cli_audit.collectors import collect_gitlab, collect_github +from cli_audit.tools import Tool, latest_target_url + +ROOT = Path(__file__).resolve().parent.parent + + +class TestExplicitSourceKind: + def test_poetry_resolves_pypi(self): + tool = ToolCatalog().get("poetry").to_tool() + assert tool.source_kind == "pypi" + assert tool.source_args == ("poetry",) + + def test_explicit_source_kind_with_package_name(self): + entry = ToolCatalogEntry( + name="demo", + package_name="demo-pkg", + _raw_data={"source_kind": "pypi"}, + ) + assert entry._derive_source() == ("pypi", ("demo-pkg",)) + + def test_explicit_source_args_win(self): + entry = ToolCatalogEntry( + name="demo", + _raw_data={"source_kind": "gh", "source_args": ["owner", "repo"]}, + ) + assert entry._derive_source() == ("gh", ("owner", "repo")) + + +class TestRawTagUrls: + def test_github_url_keeps_v_prefix(self): + tool = Tool("actionlint", ("actionlint",), "gh", ("rhysd", "actionlint")) + url = latest_target_url(tool, "v1.7.12", "1.7.12") + assert url == "https://github.com/rhysd/actionlint/releases/tag/v1.7.12" + + def test_gitlab_url_keeps_raw_tag(self): + tool = Tool("demo", ("demo",), "gitlab", ("grp", "proj")) + url = latest_target_url(tool, "v2.0.0", "2.0.0") + assert url == "https://gitlab.com/grp/proj/-/releases/v2.0.0" + + def test_collect_github_returns_raw_tag(self): + # Force the redirect path to fail so the API path (http_get) is used. + opener = MagicMock() + opener.open.side_effect = Exception("no redirect in test") + api_body = json.dumps({"tag_name": "v3.4.5"}).encode() + with patch("cli_audit.collectors.urllib.request.build_opener", return_value=opener), \ + patch("cli_audit.collectors.http_get", return_value=api_body): + raw_tag, version = collect_github("owner", "repo") + assert raw_tag == "v3.4.5" # raw tag preserved for the URL + assert version == "3.4.5" # version normalized for display/compare + + def test_collect_gitlab_returns_raw_tag(self): + body = json.dumps([{"tag_name": "v9.9.9"}]).encode() + with patch("cli_audit.collectors.http_get", return_value=body): + raw_tag, version = collect_gitlab("grp", "proj") + assert raw_tag == "v9.9.9" + assert version == "9.9.9" + + +class TestReviewFixes: + def test_merged_display_uses_normalized_version_not_raw_tag(self): + # Regression: latest_tag holds the raw tag ("v1.7.12") for URL building; + # the displayed "latest" must be the normalized version. + from cli_audit.local_state import LocalState, merge_for_display + from cli_audit.upstream_cache import UpstreamCache, UpstreamVersion + + up = UpstreamCache(versions={ + "demo": UpstreamVersion(latest_tag="v1.7.12", latest_version="1.7.12"), + }) + merged = merge_for_display(up, LocalState()) + entry = next(t for t in merged if t["tool"] == "demo") + assert entry["latest_upstream"] == "1.7.12" + + def test_url_strips_terminal_escape_sequences(self): + tool = Tool("x", ("x",), "gh", ("o", "r")) + url = latest_target_url(tool, "v1.0\x1b[31m\x07", "1.0") + assert "\x1b" not in url and "\x07" not in url + + def test_source_args_string_not_split(self): + entry = ToolCatalogEntry( + name="d", _raw_data={"source_kind": "pypi", "source_args": "demo-pkg"} + ) + assert entry._derive_source() == ("pypi", ("demo-pkg",)) + + def test_collect_github_handles_non_dict_json(self): + with patch("cli_audit.collectors.urllib.request.build_opener") as bo, \ + patch("cli_audit.collectors.http_get", return_value=b'["unexpected"]'): + bo.return_value.open.side_effect = Exception("no redirect") + # Must not raise AttributeError on the list response + collect_github("owner", "repo") + + +class TestHealthCheckKnowsPoetry: + def test_script_detects_poetry(self): + script = (ROOT / "scripts" / "check_python_package_managers.sh").read_text() + assert "has_poetry" in script + assert "command -v poetry" in script + # "optimal" must require the absence of poetry + assert "! $has_poetry" in script