Harden audit tooling and release helpers

tools/release/release-doctor.py

@@ -16,6 +16,7 @@ import sys
 import tomllib
 from typing import Any
 from urllib.error import URLError
+from urllib.parse import urlparse
 from urllib.request import Request, urlopen
 
 
@@ -32,6 +33,7 @@ class SuggestedCommand:
     title: str
     command: str
     cwd: str
+    argv: tuple[str, ...] = ()
     mutating: bool = False
     reason: str = ""
 
@@ -897,7 +899,12 @@ def run_suggested_command(item: SuggestedCommand, *, assume_yes: bool = False) -
         if confirmation not in {"y", "yes"}:
             print("Skipped.")
             return None
-    result = subprocess.run(item.command, shell=True, cwd=item.cwd)  # noqa: S602 - only user-confirmed doctor commands are executed.
+    try:
+        argv = command_argv(item)
+    except ValueError as exc:
+        print(f"Cannot run command safely: {exc}")
+        return 2
+    result = subprocess.run(argv, cwd=item.cwd, check=False)
     print(f"Command exited with {result.returncode}.")
     return result.returncode
 
@@ -950,7 +957,7 @@ def run_safe_directory_workflow(report: DoctorReport) -> None:
         return
     for repo in repos:
         assert repo.safe_directory_command is not None
-        result = subprocess.run(repo.safe_directory_command, shell=True, cwd=ROOT)  # noqa: S602 - generated git config command, user-confirmed.
+        result = subprocess.run(safe_directory_argv(Path(repo.path)), cwd=ROOT, check=False)
         print(f"{repo.name}: command exited with {result.returncode}.")
 
 
@@ -1020,9 +1027,8 @@ def commit_and_push_dirty_repositories(report: DoctorReport) -> None:
 
 def page_text(text: str) -> None:
     if sys.stdout.isatty():
-        pager = os.environ.get("PAGER", "less -R")
         try:
-            result = subprocess.run(pager, input=text, shell=True, text=True, check=False)  # noqa: S602 - user-configured pager.
+            result = subprocess.run(["less", "-R"], input=text, text=True, check=False)
             if result.returncode != 127:
                 return
         except OSError:
@@ -1034,28 +1040,61 @@ def report_to_dict(report: DoctorReport) -> dict[str, Any]:
     return asdict(report)
 
 
-def command(cwd: Path, title: str, value: str, *, mutating: bool = False, reason: str = "") -> SuggestedCommand:
-    return SuggestedCommand(title=title, command=value, cwd=str(cwd), mutating=mutating, reason=reason)
+def command(
+    cwd: Path,
+    title: str,
+    value: str,
+    *,
+    mutating: bool = False,
+    reason: str = "",
+    argv: tuple[str, ...] = (),
+) -> SuggestedCommand:
+    return SuggestedCommand(title=title, command=value, cwd=str(cwd), argv=argv, mutating=mutating, reason=reason)
+
+
+def command_argv(item: SuggestedCommand) -> tuple[str, ...]:
+    if item.argv:
+        return item.argv
+    argv = tuple(shlex.split(item.command))
+    if not argv:
+        raise ValueError("empty command")
+    if any(_looks_like_shell_syntax(part) for part in argv):
+        raise ValueError("command contains shell syntax and needs an explicit argv definition")
+    return argv
+
+
+def _looks_like_shell_syntax(value: str) -> bool:
+    return any(marker in value for marker in ("|", "&&", "||", ";", "$(", "`", ">", "<"))
 
 
 def release_tag_command(report: DoctorReport, *, dry_run: bool) -> SuggestedCommand:
     assert report.target_version is not None
     suffix = " --dry-run" if dry_run else ""
+    argv = (
+        "tools/release/push-release-tag.sh",
+        "--version",
+        report.target_version,
+        "--strict-migration-audit",
+    ) + (("--dry-run",) if dry_run else ())
     return command(
         META_ROOT,
         "Preview release tagging workflow" if dry_run else "Run release tagging workflow",
         f"tools/release/push-release-tag.sh --version {shlex.quote(report.target_version)} --strict-migration-audit{suffix}",
         mutating=not dry_run,
+        argv=argv,
     )
 
 
 def migration_audit_command(track: str, *, strict: bool = False) -> SuggestedCommand:
     extra = " --strict" if strict else ""
-    return command(META_ROOT, f"Run {track} migration audit", f"{python_bin()} tools/release/release-migration-audit.py --track {track}{extra}")
+    argv = (python_bin_path(), "tools/release/release-migration-audit.py", "--track", track) + (("--strict",) if strict else ())
+    return command(META_ROOT, f"Run {track} migration audit", f"{python_bin()} tools/release/release-migration-audit.py --track {track}{extra}", argv=argv)
 
 
 def publish_catalog_command(target_version: str | None) -> SuggestedCommand:
     version = shlex.quote(target_version or "<x.y.z>")
+    display_version = target_version or "<x.y.z>"
+    key_dir = Path.home() / ".config" / "govoplan" / "release-keys"
     return command(
         META_ROOT,
         "Publish signed release catalog",
@@ -1063,12 +1102,24 @@ def publish_catalog_command(target_version: str | None) -> SuggestedCommand:
         f"tools/release/publish-release-catalog.sh --version {version} "
         "--catalog-signing-key \"release-key-1=$KEY_DIR/release-key-1.pem\" --build-web",
         mutating=True,
+        argv=(
+            "tools/release/publish-release-catalog.sh",
+            "--version",
+            display_version,
+            "--catalog-signing-key",
+            f"release-key-1={key_dir / 'release-key-1.pem'}",
+            "--build-web",
+        ),
     )
 
 
 def python_bin() -> str:
+    return shlex.quote(python_bin_path())
+
+
+def python_bin_path() -> str:
     local = META_ROOT / ".venv" / "bin" / "python"
-    return shlex.quote(str(local if local.exists() else Path(sys.executable)))
+    return str(local if local.exists() else Path(sys.executable))
 
 
 def normalize_version(value: str) -> str:
@@ -1152,10 +1203,17 @@ def safe_directory_command(path: Path) -> str:
     return "git config --global --add safe.directory " + shlex.quote(str(path.resolve()))
 
 
+def safe_directory_argv(path: Path) -> list[str]:
+    return ["git", "config", "--global", "--add", "safe.directory", str(path.resolve())]
+
+
 def check_url(url: str) -> dict[str, Any]:
+    parsed = urlparse(url)
+    if parsed.scheme not in {"http", "https"} or not parsed.netloc:
+        return {"ok": False, "error": "URL must use http:// or https:// with a hostname", "url": url}
     request = Request(url, method="HEAD")
     try:
-        with urlopen(request, timeout=8) as response:  # noqa: S310 - operator-requested release availability check.
+        with urlopen(request, timeout=8) as response:  # noqa: S310 - validated operator-requested release availability check.  # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected
             return {"ok": 200 <= response.status < 400, "status": response.status, "url": url}
     except URLError as exc:
         return {"ok": False, "error": str(exc), "url": url}