From 02077429df1af1b5f4c96c640c921e646f0ead13 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 14 Sep 2026 09:51:52 +0900 Subject: [PATCH] fix(python-security): stop reporting pip-audit transport failures as vulnerabilities Closes #2158. The hard gate folded every non-zero pip-audit exit into one message asserting known-vulnerable dependencies, so a PyPI advisory-query ConnectionResetError (no findings at all) read as a security finding and misdirected triage on #2157. Each invocation now runs through run_audit(), which keeps the gate closed on any failure but classifies it: pip-audit 2.10.1 prints "Found N known vulnerabilit(y|ies) ... in N package(s)" on stderr only when it has findings (pip_audit/_cli.py), so that line selects the finding error; anything else is reported as an audit-service/transport failure with the last diagnostic line and a rerun instruction. No retry budget is added (directive 3.1). Behaviour test executes the real step body with a fake pip-audit on PATH: findings -> exit 1 + finding error; traceback without findings -> exit 1 + could-not-complete error; clean -> exit 0, no ::error. RED on main. Co-Authored-By: Claude Fable 5.1 --- .github/workflows/python-security.yml | 38 +++++-- ...curity_pip_audit_failure_classification.py | 107 ++++++++++++++++++ 2 files changed, 135 insertions(+), 10 deletions(-) create mode 100644 tests/test_python_security_pip_audit_failure_classification.py diff --git a/.github/workflows/python-security.yml b/.github/workflows/python-security.yml index 8453895027..1788cfd40b 100644 --- a/.github/workflows/python-security.yml +++ b/.github/workflows/python-security.yml @@ -227,6 +227,30 @@ jobs: run: | set -euo pipefail status=0 + audit_log="$(mktemp)" + + # Run one pip-audit invocation and classify a non-zero exit. pip-audit + # 2.10.1 prints "Found N known vulnerabilit(y|ies) ... in N package(s)" + # on stderr only when it has findings (pip_audit/_cli.py); any other + # non-zero exit is an audit-service/transport failure (advisory query + # traceback, resolver/_fatal error) and must not be reported as a + # vulnerability finding (#2158). Both keep the gate closed. + run_audit() { + local label="$1" + shift + echo "::group::pip-audit ${label}" + if pip-audit "$@" 2>&1 | tee "${audit_log}"; then + echo "::endgroup::" + return 0 + fi + echo "::endgroup::" + status=1 + if grep -Eq "Found [0-9]+ known vulnerabilit" "${audit_log}"; then + echo "::error::pip-audit found known-vulnerable Python dependencies in ${label}. Remediate the pins listed above." + else + echo "::error::pip-audit could not complete for ${label}: $(grep -Ev '^[[:space:]]*$' "${audit_log}" | tail -n 1). This is an audit-service/transport failure, not a vulnerability finding; rerun the job before treating it as a security result." + fi + } # Audit every discovered requirements file. while IFS= read -r req; do @@ -251,15 +275,11 @@ jobs: base="${req%.txt}" unhashed_base="${base%-hashes}" if [ "$base" != "$unhashed_base" ] && [ -f "${unhashed_base}-overrides.txt" ]; then - echo "::group::pip-audit -r ${req} (--disable-pip --no-deps: overridden lock)" - pip-audit --strict --desc=on --no-deps --disable-pip -r "${req}" || status=1 - echo "::endgroup::" + run_audit "-r ${req} (--disable-pip --no-deps: overridden lock)" --strict --desc=on --no-deps --disable-pip -r "${req}" elif [ "$base" = "$unhashed_base" ] && [ -f "${unhashed_base}-overrides.txt" ]; then echo "::notice::Skipping pip-audit for ${req}: it is the raw input to an overridden lock (${unhashed_base}-hashes.txt), never itself a pip install --require-hashes target, and its compiled hashes file is audited separately with full resolution." else - echo "::group::pip-audit -r ${req}" - pip-audit --strict --desc=on -r "${req}" || status=1 - echo "::endgroup::" + run_audit "-r ${req}" --strict --desc=on -r "${req}" fi done < <(find . -type f -name 'requirements*.txt' -not -path './.git/*') @@ -267,12 +287,10 @@ jobs: if find . -maxdepth 2 -type f \ \( -name 'pyproject.toml' -o -name 'pylock.*.toml' \) \ -not -path './.git/*' -print -quit | grep -q .; then - echo "::group::pip-audit . (project manifest)" - pip-audit --strict --desc=on . || status=1 - echo "::endgroup::" + run_audit ". (project manifest)" --strict --desc=on . fi if [ "${status}" != "0" ]; then - echo "::error::pip-audit reported known-vulnerable Python dependencies." + echo "::error::pip-audit failed for at least one input; the per-input errors above say whether it was a finding or an audit-service failure." exit 1 fi diff --git a/tests/test_python_security_pip_audit_failure_classification.py b/tests/test_python_security_pip_audit_failure_classification.py new file mode 100644 index 0000000000..9cf131ef8d --- /dev/null +++ b/tests/test_python_security_pip_audit_failure_classification.py @@ -0,0 +1,107 @@ +"""Behaviour contract for the `python-security.yml` pip-audit hard gate. + +Issue #2158: the step folded every non-zero pip-audit exit into one message +that asserted "known-vulnerable Python dependencies", so a PyPI transport +failure (`ConnectionResetError` from the advisory query, no findings at all) +read like a security finding and misdirected triage. The gate must stay +closed on every failure, but the printed evidence has to say which of the +two things happened. pip-audit 2.10.1 (the pinned version) prints +`Found N known vulnerabilit(y|ies) ... in N package(s)` to stderr when it has +findings (`pip_audit/_cli.py`), so that line is the discriminator. + +The tests execute the real step body with a fake `pip-audit` on PATH, the +same technique as `test_workflow_file_detection_pipefail_regression.py`. +""" + +from __future__ import annotations + +import os +from pathlib import Path +import re +import stat +import subprocess + +REPO_ROOT = Path(__file__).resolve().parents[1] +WORKFLOW = REPO_ROOT / ".github/workflows/python-security.yml" +STEP_MARKER = " - name: Run pip-audit (hard gate on any known vulnerability)\n" + + +def _extract_pip_audit_script(workflow_text: str) -> str: + """Return the step's `run:` body with the YAML block indentation removed.""" + start = workflow_text.index(STEP_MARKER) + run_start = workflow_text.index(" run: |\n", start) + len(" run: |\n") + rest = workflow_text[run_start:] + # The body ends at the next step (` - name:`) or the next top-level + # job key (two spaces then a non-space); blank lines inside the script are + # followed by ten-space indentation and must not terminate it. + boundary = re.search(r"\n(?: - name:|\n \S)", rest) + block = rest[: boundary.start()] if boundary else rest + return "\n".join(line[10:] for line in block.splitlines()) + + +def _fake_pip_audit(bin_dir: Path, body: str) -> None: + """Install a `pip-audit` shim whose behaviour is the given shell body.""" + shim = bin_dir / "pip-audit" + shim.write_text("#!/usr/bin/env bash\n" + body + "\n", encoding="utf-8") + shim.chmod(shim.stat().st_mode | stat.S_IXUSR) + + +def _run_step(tmp_path: Path, shim_body: str) -> subprocess.CompletedProcess[str]: + """Run the extracted step in a repo holding one requirements file.""" + repo = tmp_path / "repo" + repo.mkdir() + (repo / "requirements-demo-ci.txt").write_text("requests==2.32.0\n", encoding="utf-8") + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + _fake_pip_audit(bin_dir, shim_body) + script = _extract_pip_audit_script(WORKFLOW.read_text(encoding="utf-8")) + return subprocess.run( + ["bash", "-c", script], + cwd=repo, + capture_output=True, + text=True, + timeout=60, + env={**os.environ, "PATH": f"{bin_dir}:{os.environ['PATH']}"}, + ) + + +def test_genuine_findings_fail_closed_and_are_reported_as_findings(tmp_path): + """A real advisory hit still fails the job and names the vulnerable input.""" + result = _run_step( + tmp_path, + 'echo "Found 2 known vulnerabilities in 1 package" >&2; exit 1', + ) + assert result.returncode == 1 + assert "::error::pip-audit found known-vulnerable Python dependencies in -r ./requirements-demo-ci.txt" in result.stdout + assert "could not complete" not in result.stdout + + +def test_transport_failure_fails_closed_but_is_not_called_a_vulnerability(tmp_path): + """The #2158 shape: no findings, then an unhandled PyPI connection error.""" + result = _run_step( + tmp_path, + 'echo "No known vulnerabilities found" >&2; ' + 'echo "Traceback (most recent call last):" >&2; ' + 'echo "ConnectionResetError: [Errno 104] Connection reset by peer" >&2; exit 1', + ) + assert result.returncode == 1 + assert "known-vulnerable" not in result.stdout + assert ( + "::error::pip-audit could not complete for -r ./requirements-demo-ci.txt: " + "ConnectionResetError: [Errno 104] Connection reset by peer" + ) in result.stdout + assert "not a vulnerability finding" in result.stdout + + +def test_clean_audit_passes_without_error_annotations(tmp_path): + """A clean run exits 0 and prints no `::error::` line.""" + result = _run_step(tmp_path, 'echo "No known vulnerabilities found" >&2; exit 0') + assert result.returncode == 0, result.stderr + assert "::error::" not in result.stdout + + +def test_step_no_longer_asserts_a_finding_for_every_failure(): + """The single catch-all message must be gone from the workflow text.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + assert "::error::pip-audit reported known-vulnerable Python dependencies." not in workflow + assert "Found [0-9]+ known vulnerabilit" in workflow