fix: pre-empt review feedback on pins, predicate reuse, and baseline gates

- align the setup-uv pin in security.yml with test.yml (v8.2.0)
- use is_https_or_localhost_http for the preset_add/extension_add URL
  checks and pass strict_redirects=True to the latest-release fetch and
  the release-asset resolver call sites
- baseline gate scripts fail closed on unresolvable refs and git read
  errors instead of treating them as "baseline did not exist"; the
  security workflow re-runs on labeled/unlabeled so the ack label can
  turn the gate green without a push
- regenerate the bandit baseline against HEAD (two entries referenced
  removed code, one had drifted); track baseline entries by
  file+test_id in tests so line drift no longer breaks them
- raise ZIP size-limit errors outside the broad except in
  safe_extract_zip so an error_type subclassing OSError/RuntimeError
  cannot re-wrap them
- tests: drop two redirect tests duplicated from test_authentication,
  move the downgrade test next to its siblings, assert the workflow
  catalog max_bytes, route OpenerDirector.open through urlopen in the
  modules that patch urlopen, add set -euo pipefail to the secret scan,
  misc cleanup (unused helper, redundant imports, EOF-less fake read)

Author: PascalThuet

.github/bandit-baseline.json

@@ -1,47 +1,7 @@
 {
   "results": [
     {
-      "code": "103     if not req.get_header(\"Authorization\") and not strict_redirects:\n104         return urllib.request.urlopen(req, timeout=timeout)\n105 \n",
-      "col_offset": 15,
-      "end_col_offset": 59,
-      "filename": "src/specify_cli/_github_http.py",
-      "issue_confidence": "HIGH",
-      "issue_cwe": {
-        "id": 22,
-        "link": "https://cwe.mitre.org/data/definitions/22.html"
-      },
-      "issue_severity": "MEDIUM",
-      "issue_text": "Audit url open for permitted schemes. Allowing use of file:/ or custom schemes is often unexpected.",
-      "line_number": 104,
-      "line_range": [
-        104
-      ],
-      "more_info": "https://bandit.readthedocs.io/en/1.9.4/blacklists/blacklist_calls.html#b310-urllib-urlopen",
-      "test_id": "B310",
-      "test_name": "blacklist"
-    },
-    {
-      "code": "113 \n114             with urllib.request.urlopen(req, timeout=30) as resp:  # noqa: S310\n115                 payload = _json.loads(\n",
-      "col_offset": 17,
-      "end_col_offset": 56,
-      "filename": "src/specify_cli/authentication/azure_devops.py",
-      "issue_confidence": "HIGH",
-      "issue_cwe": {
-        "id": 22,
-        "link": "https://cwe.mitre.org/data/definitions/22.html"
-      },
-      "issue_severity": "MEDIUM",
-      "issue_text": "Audit url open for permitted schemes. Allowing use of file:/ or custom schemes is often unexpected.",
-      "line_number": 114,
-      "line_range": [
-        114
-      ],
-      "more_info": "https://bandit.readthedocs.io/en/1.9.4/blacklists/blacklist_calls.html#b310-urllib-urlopen",
-      "test_id": "B310",
-      "test_name": "blacklist"
-    },
-    {
-      "code": "170         return opener.open(req, timeout=timeout)\n171     return urllib.request.urlopen(req, timeout=timeout)  # noqa: S310\n",
+      "code": "168         return opener.open(req, timeout=timeout)\n169     return urllib.request.urlopen(req, timeout=timeout)  # noqa: S310\n",
       "col_offset": 11,
       "end_col_offset": 55,
       "filename": "src/specify_cli/authentication/http.py",
@@ -52,9 +12,9 @@
       },
       "issue_severity": "MEDIUM",
       "issue_text": "Audit url open for permitted schemes. Allowing use of file:/ or custom schemes is often unexpected.",
-      "line_number": 171,
+      "line_number": 169,
       "line_range": [
-        171
+        169
       ],
       "more_info": "https://bandit.readthedocs.io/en/1.9.4/blacklists/blacklist_calls.html#b310-urllib-urlopen",
       "test_id": "B310",

.github/scripts/check_bandit_baseline.py

@@ -43,14 +43,38 @@
 ACK_LABEL = "security-baseline-change"
 
 
+def _git_ok(*args: str) -> bool:
+    """True if the git command exits 0 (output discarded)."""
+    return (
+        subprocess.run(
+            ["git", *args],
+            cwd=REPO_ROOT,
+            stdout=subprocess.DEVNULL,
+            stderr=subprocess.DEVNULL,
+        ).returncode
+        == 0
+    )
+
+
 def _read_baseline_at(ref: str) -> tuple[dict, bool]:
     """Return (baseline_json, file_existed_at_ref).
 
     Used for the base side. The head side reads the working tree to avoid
     silently fail-opening on an unfetched/invalid head ref.
+
+    Only a missing *path* at a resolvable ref counts as "did not exist";
+    an unresolvable ref or a failing ``git show`` aborts instead, so a
+    transient git failure cannot silently disable the gate.
     """
     if not ref:
         return {"results": []}, False
+    if not _git_ok("rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"):
+        raise SystemExit(
+            f"Base ref {ref!r} cannot be resolved (unfetched or invalid). "
+            f"Refusing to fail-open on a security gate."
+        )
+    if not _git_ok("cat-file", "-e", f"{ref}:{BASELINE_PATH}"):
+        return {"results": []}, False
     try:
         blob = subprocess.run(
             ["git", "show", f"{ref}:{BASELINE_PATH}"],
@@ -60,8 +84,11 @@ def _read_baseline_at(ref: str) -> tuple[dict, bool]:
             stderr=subprocess.PIPE,
             text=True,
         ).stdout
-    except subprocess.CalledProcessError:
-        return {"results": []}, False
+    except subprocess.CalledProcessError as exc:
+        raise SystemExit(
+            f"Could not read baseline at {ref!r}: {exc.stderr.strip()}. "
+            f"Refusing to fail-open on a security gate."
+        )
     try:
         return json.loads(blob), True
     except json.JSONDecodeError:

.github/scripts/check_secrets_baseline.py

@@ -56,10 +56,35 @@ def log_safe(self) -> str:
         )
 
 
+def _git_ok(*args: str) -> bool:
+    """True if the git command exits 0 (output discarded)."""
+    return (
+        subprocess.run(
+            ["git", *args],
+            cwd=REPO_ROOT,
+            stdout=subprocess.DEVNULL,
+            stderr=subprocess.DEVNULL,
+        ).returncode
+        == 0
+    )
+
+
 def _read_baseline_at(ref: str) -> tuple[dict, bool]:
-    """Return (baseline_json, file_existed_at_ref). Base side only."""
+    """Return (baseline_json, file_existed_at_ref). Base side only.
+
+    Only a missing *path* at a resolvable ref counts as "did not exist";
+    an unresolvable ref or a failing ``git show`` aborts instead, so a
+    transient git failure cannot silently disable the gate.
+    """
     if not ref:
         return {"results": {}}, False
+    if not _git_ok("rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"):
+        raise SystemExit(
+            f"Base ref {ref!r} cannot be resolved (unfetched or invalid). "
+            f"Refusing to fail-open on a security gate."
+        )
+    if not _git_ok("cat-file", "-e", f"{ref}:{BASELINE_PATH}"):
+        return {"results": {}}, False
     try:
         blob = subprocess.run(
             ["git", "show", f"{ref}:{BASELINE_PATH}"],
@@ -69,8 +94,11 @@ def _read_baseline_at(ref: str) -> tuple[dict, bool]:
             stderr=subprocess.PIPE,
             text=True,
         ).stdout
-    except subprocess.CalledProcessError:
-        return {"results": {}}, False
+    except subprocess.CalledProcessError as exc:
+        raise SystemExit(
+            f"Could not read baseline at {ref!r}: {exc.stderr.strip()}. "
+            f"Refusing to fail-open on a security gate."
+        )
     try:
         return json.loads(blob), True
     except json.JSONDecodeError:

.github/workflows/security.yml

@@ -7,6 +7,9 @@ on:
   push:
     branches: ["main"]
   pull_request:
+    # labeled/unlabeled so the baseline-growth gates re-evaluate when the
+    # acknowledgement label is added or removed, without requiring a push.
+    types: [opened, synchronize, reopened, labeled, unlabeled]
   schedule:
     - cron: "17 4 * * 1"
   workflow_dispatch:
@@ -27,7 +30,7 @@ jobs:
           fetch-depth: 2
 
       - name: Install uv
-        uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0
+        uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # v8.2.0
 
       - name: Set up Python ${{ matrix.python-version }}
         uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6
@@ -73,7 +76,7 @@ jobs:
           fetch-depth: 0
 
       - name: Install uv
-        uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0
+        uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # v8.2.0
 
       - name: Set up Python
         uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6
@@ -154,7 +157,7 @@ jobs:
           fetch-depth: 0
 
       - name: Install uv
-        uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0
+        uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # v8.2.0
 
       - name: Set up Python
         uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6
@@ -168,6 +171,7 @@ jobs:
       # rewriting the baseline file (so there's no spurious git diff).
       - name: Run detect-secrets
         run: |
+          set -euo pipefail
           git ls-files -z \
             -- ':!:.secrets.baseline' \
                ':!:uv.lock' \

src/specify_cli/__init__.py

@@ -689,11 +689,8 @@ def preset_add(
 
         elif from_url:
             # Validate URL scheme before downloading
-            from urllib.parse import urlparse as _urlparse
-            _parsed = _urlparse(from_url)
-            _is_localhost = _parsed.hostname in ("localhost", "127.0.0.1", "::1")
-            if _parsed.scheme != "https" and not (_parsed.scheme == "http" and _is_localhost):
-                console.print(f"[red]Error:[/red] URL must use HTTPS (got {_parsed.scheme}://). HTTP is only allowed for localhost.")
+            if not is_https_or_localhost_http(from_url):
+                console.print("[red]Error:[/red] URL must use HTTPS. HTTP is only allowed for localhost.")
                 raise typer.Exit(1)
 
             console.print(f"Installing preset from [cyan]{from_url}[/cyan]...")
@@ -703,11 +700,15 @@ def preset_add(
             with tempfile.TemporaryDirectory() as tmpdir:
                 zip_path = Path(tmpdir) / "preset.zip"
                 try:
+                    from functools import partial
+
                     from specify_cli.authentication.http import open_url as _open_url
                     from specify_cli._github_http import resolve_github_release_asset_api_url
 
                     _preset_extra_headers = None
-                    _resolved_from_url = resolve_github_release_asset_api_url(from_url, _open_url)
+                    _resolved_from_url = resolve_github_release_asset_api_url(
+                        from_url, partial(_open_url, strict_redirects=True)
+                    )
                     if _resolved_from_url:
                         from_url = _resolved_from_url
                         _preset_extra_headers = {"Accept": "application/octet-stream"}
@@ -1601,13 +1602,9 @@ def extension_add(
     # Guard with ``not dev`` so that --dev + --from does not show a
     # confusing confirmation for a URL that will be ignored.
     if from_url and not dev:
-        from urllib.parse import urlparse
         from rich.markup import escape as _escape_markup
 
-        parsed = urlparse(from_url)
-        is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1")
-
-        if parsed.scheme != "https" and not (parsed.scheme == "http" and is_localhost):
+        if not is_https_or_localhost_http(from_url):
             console.print("[red]Error:[/red] URL must use HTTPS for security.")
             console.print("HTTP is only allowed for localhost URLs.")
             raise typer.Exit(1)
@@ -3083,6 +3080,8 @@ def _validate_and_install_local(yaml_path: Path, source_label: str) -> None:
 
     # Try as URL (http/https)
     if source.startswith("http://") or source.startswith("https://"):
+        from functools import partial
+
         from specify_cli.authentication.http import open_url as _open_url
 
         if not is_https_or_localhost_http(source):
@@ -3092,7 +3091,9 @@ def _validate_and_install_local(yaml_path: Path, source_label: str) -> None:
         from specify_cli._github_http import resolve_github_release_asset_api_url as _resolve_gh_asset
 
         _wf_url_extra_headers = None
-        _resolved_wf_url = _resolve_gh_asset(source, _open_url, timeout=30)
+        _resolved_wf_url = _resolve_gh_asset(
+            source, partial(_open_url, strict_redirects=True), timeout=30
+        )
         if _resolved_wf_url:
             source = _resolved_wf_url
             _wf_url_extra_headers = {"Accept": "application/octet-stream"}
@@ -3177,11 +3178,15 @@ def _validate_and_install_local(yaml_path: Path, source_label: str) -> None:
     workflow_file = workflow_dir / "workflow.yml"
 
     try:
+        from functools import partial
+
         from specify_cli.authentication.http import open_url as _open_url
         from specify_cli._github_http import resolve_github_release_asset_api_url as _resolve_gh_asset
 
         _wf_cat_extra_headers = None
-        _resolved_workflow_url = _resolve_gh_asset(workflow_url, _open_url, timeout=30)
+        _resolved_workflow_url = _resolve_gh_asset(
+            workflow_url, partial(_open_url, strict_redirects=True), timeout=30
+        )
         if _resolved_workflow_url:
             workflow_url = _resolved_workflow_url
             _wf_cat_extra_headers = {"Accept": "application/octet-stream"}

src/specify_cli/_download_security.py

@@ -38,8 +38,9 @@
 def is_https_or_localhost_http(url: str) -> bool:
     """Return True if *url* is HTTPS, or HTTP limited to loopback hosts.
 
-    Shared redirect-safety predicate used by the GitHub and auth HTTP redirect
-    handlers so the rule (and any future tightening of it) lives in one place.
+    Shared scheme-safety predicate used by the auth HTTP redirect handler and
+    by the direct URL validations in the CLI download flows, so the rule (and
+    any future tightening of it) lives in one place.
 
     The loopback allowance is a deliberate *exact-string* match on
     ``localhost`` / ``127.0.0.1`` / ``::1``, not an IP-range check: other
@@ -304,6 +305,10 @@ def safe_extract_zip(
                     exc,
                 )
             written = 0
+            # Raised outside the try below: if error_type subclasses OSError or
+            # RuntimeError, raising inside would re-wrap the limit error as
+            # "Failed to extract" and lose the size-bound message.
+            limit_error: str | None = None
             try:
                 with zf.open(member, "r") as source, member_path.open("wb") as dest:
                     while True:
@@ -312,22 +317,24 @@ def safe_extract_zip(
                             break
                         written += len(chunk)
                         if written > max_member_bytes:
-                            _raise(
-                                error_type,
+                            limit_error = (
                                 f"ZIP member {member.filename} exceeds maximum size "
-                                f"of {max_member_bytes} bytes",
+                                f"of {max_member_bytes} bytes"
                             )
+                            break
                         total_written += len(chunk)
                         if total_written > max_total_bytes:
-                            _raise(
-                                error_type,
+                            limit_error = (
                                 f"ZIP archive exceeds maximum uncompressed size "
-                                f"of {max_total_bytes} bytes",
+                                f"of {max_total_bytes} bytes"
                             )
+                            break
                         dest.write(chunk)
             except (OSError, zipfile.BadZipFile, RuntimeError) as exc:
                 _raise_from(
                     error_type,
                     f"Failed to extract ZIP member {member.filename}: {exc}",
                     exc,
                 )
+            if limit_error is not None:
+                _raise(error_type, limit_error)

src/specify_cli/_version.py

@@ -119,6 +119,7 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]:
             GITHUB_API_LATEST,
             timeout=5,
             extra_headers={"Accept": "application/vnd.github+json"},
+            strict_redirects=True,
         ) as resp:
             payload = json.loads(
                 read_response_limited(

tests/http_helpers.py

@@ -2,8 +2,11 @@
 
 import io
 import json
+import urllib.request
 from unittest.mock import MagicMock
 
+import pytest
+
 
 def mock_urlopen_response(payload: dict) -> MagicMock:
     """Build a urlopen context-manager mock whose read returns JSON."""
@@ -14,3 +17,25 @@ def mock_urlopen_response(payload: dict) -> MagicMock:
     cm.__enter__.return_value = resp
     cm.__exit__.return_value = False
     return cm
+
+
+@pytest.fixture(autouse=True)
+def route_opener_open_through_urlopen(monkeypatch):
+    """Route OpenerDirector.open through urllib.request.urlopen.
+
+    ``open_url(..., strict_redirects=True)`` fetches via
+    ``build_opener(...).open()``, which bypasses ``urllib.request.urlopen``
+    — and with it the urlopen patches these test modules are built on.
+    Delegating ``open()`` to urlopen at call time keeps those patches
+    effective; the redirect handler's own behavior is covered by
+    ``TestRedirectStripping`` in test_authentication.py.
+
+    Import this fixture into a test module to activate it there.
+    """
+    monkeypatch.setattr(
+        urllib.request.OpenerDirector,
+        "open",
+        lambda self, req, data=None, timeout=None: urllib.request.urlopen(
+            req, timeout=timeout
+        ),
+    )