From 2e8c902a53d5bf1b3290eb52fd8954380f7df931 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Houpert?= <10154151+lhoupert@users.noreply.github.com> Date: Tue, 6 Oct 2026 15:06:06 +0100 Subject: [PATCH 1/5] fix: fail closed when pip-audit cannot run; reject package_manager pip A failed uv/poetry/pipenv export, a missing requirements file or a pip-audit crash (exit 1, the same code as "vulnerabilities found") was reported as "No vulnerabilities found" with exit 0. Any run without a pip-audit JSON report now exits non-zero and writes "pip-audit did NOT run" to the step summary. - uv export drops the caller's project (--no-emit-project), and pinned exports (uv, poetry, pipenv) are audited with --disable-pip, so pip-audit no longer builds the project in a venv (--no-deps was a no-op without it). requirements mode keeps pip resolution. - pipenv without Pipfile.lock fails instead of auditing an empty list. - package_manager pip ran `pip freeze` inside the action's own uv environment, so it audited the action's dependencies, not the caller's. It now fails with a pointer to requirements mode. - validate_results.py errors when a pip-audit case has no report or audited zero dependencies. Refs #84 Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 4 +- action.yml | 2 +- .../tests/test_validate_results.py | 77 ++++++++- integration-tests/validate_results.py | 27 ++- src/python_security_auditing/__main__.py | 15 +- src/python_security_auditing/runners.py | 83 ++++----- tests/test_main.py | 79 +++------ tests/test_runners.py | 160 +++++++++--------- 8 files changed, 248 insertions(+), 199 deletions(-) diff --git a/README.md b/README.md index 9ba6fc0..3ff4d6a 100644 --- a/README.md +++ b/README.md @@ -304,7 +304,7 @@ If you run this action from more than one workflow on the same PR with `comment_ ## How blocking works -The job fails (non-zero exit) when **either** tool finds issues above its configured threshold. +The job fails (non-zero exit) when **either** tool finds issues above its configured threshold. It also fails when pip-audit cannot run, for example when the export fails or the requirements file is missing. **Bandit threshold** (`bandit_severity_threshold`): findings at or above the threshold block the job. @@ -330,7 +330,7 @@ The job fails (non-zero exit) when **either** tool finds issues above its config | `bandit_scan_dirs` | `.` | Comma-separated directories for bandit to scan (relative to `working_directory`) | | `bandit_severity_threshold` | `high` | Minimum severity that blocks the job: `high`, `medium`, or `low` | | `pip_audit_block_on` | `fixable` | When pip-audit findings block the job: `fixable`, `all`, or `none` | -| `package_manager` | `requirements` | How to resolve deps for pip-audit: `uv`, `pip`, `poetry`, `pipenv`, `requirements` | +| `package_manager` | `requirements` | How to resolve deps for pip-audit: `uv`, `poetry`, `pipenv`, `requirements`. For a pip environment, run `pip freeze > requirements.txt` first and use `requirements` | | `requirements_file` | `requirements.txt` | Path to requirements file when `package_manager=requirements` | | `working_directory` | `.` | Directory to run the audit from (useful for monorepos) | | `comment_on` | `never` | When to post a PR comment: `never`, `blocking` (only when issues block the job), or `always` | diff --git a/action.yml b/action.yml index d25f35f..4a66b67 100644 --- a/action.yml +++ b/action.yml @@ -16,7 +16,7 @@ inputs: description: When to block on pip-audit findings — fixable, all, or none default: fixable package_manager: - description: How to resolve dependencies for pip-audit (uv, pip, poetry, pipenv, requirements) + description: How to resolve dependencies for pip-audit (uv, poetry, pipenv, requirements) default: requirements requirements_file: description: Path to requirements file when package_manager=requirements diff --git a/integration-tests/tests/test_validate_results.py b/integration-tests/tests/test_validate_results.py index cee24ab..09ca751 100644 --- a/integration-tests/tests/test_validate_results.py +++ b/integration-tests/tests/test_validate_results.py @@ -4,6 +4,7 @@ - TestParseSarif - TestParsePipAudit - TestValidateTest (pure logic, no I/O) + - TestCheckAudited - TestGenerateReport - TestMain (integration, uses tmp_path + monkeypatch) """ @@ -248,6 +249,46 @@ def test_expected_pip_audit_package_absent_returns_error(self) -> None: assert any("requests" in e for e in errors) +# --------------------------------------------------------------------------- +# check_audited +# --------------------------------------------------------------------------- + + +class TestCheckAudited: + """A pip-audit case without a report, or with nothing audited, must not pass as clean.""" + + def _write(self, tmp_path: Path, deps: list[dict[str, Any]]) -> Path: + path = tmp_path / "pip-audit-report.json" + path.write_text(json.dumps({"dependencies": deps, "fixes": []})) + return path + + def test_missing_report_returns_error(self) -> None: + errors = vr.check_audited({"expected_conclusion": "success"}, None) + + assert any("did not run" in e for e in errors) + + @pytest.mark.parametrize( + "deps", + [[], [{"name": "my-app", "skip_reason": "Dependency not found on PyPI"}]], + ) + def test_zero_audited_dependencies_returns_error( + self, tmp_path: Path, deps: list[dict[str, Any]] + ) -> None: + errors = vr.check_audited({"expected_conclusion": "success"}, self._write(tmp_path, deps)) + + assert any("did not run" in e for e in errors) + + def test_audited_dependency_returns_no_error(self, tmp_path: Path) -> None: + path = self._write(tmp_path, [{"name": "requests", "version": "2.32.0", "vulns": []}]) + + assert vr.check_audited({"expected_conclusion": "success"}, path) == [] + + def test_pip_audit_disabled_needs_no_report(self) -> None: + expected = {"expected_conclusion": "failure", "pip_audit_disabled": True} + + assert vr.check_audited(expected, None) == [] + + # --------------------------------------------------------------------------- # generate_report # --------------------------------------------------------------------------- @@ -314,16 +355,20 @@ def test_happy_path_returns_zero(self, tmp_path: Path, monkeypatch: pytest.Monke } self._write_expected_yml(tmp_path, tests) - # Test 01: clean SARIF, no pip-audit artifact + clean_audit = {"dependencies": [{"name": "requests", "version": "2.32.0", "vulns": []}]} + + # Test 01: clean SARIF, clean pip-audit report art01 = tmp_path / "artifacts" / "security-audit-01" art01.mkdir(parents=True) (art01 / "results.sarif").write_text(json.dumps({"runs": [{"results": []}]})) + (art01 / "pip-audit-report.json").write_text(json.dumps(clean_audit)) - # Test 02: B602 finding in SARIF + # Test 02: B602 finding in SARIF; pip-audit report nested, as artifact upload does art02 = tmp_path / "artifacts" / "security-audit-02" - art02.mkdir(parents=True) + (art02 / "02-case").mkdir(parents=True) sarif02 = {"runs": [{"results": [{"ruleId": "B602", "level": "error"}]}]} (art02 / "results.sarif").write_text(json.dumps(sarif02)) + (art02 / "02-case" / "pip-audit-report.json").write_text(json.dumps(clean_audit)) needs = {"test-01": {"result": "success"}, "test-02": {"result": "failure"}} monkeypatch.setenv("NEEDS_JSON", json.dumps(needs)) @@ -335,6 +380,32 @@ def test_happy_path_returns_zero(self, tmp_path: Path, monkeypatch: pytest.Monke assert vr.main() == 0 + def test_missing_pip_audit_report_returns_one( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + tests = { + "01": { + "name": "clean", + "expected_conclusion": "success", + "bandit_findings": [], + "pip_audit_findings": [], + }, + } + self._write_expected_yml(tmp_path, tests) + art01 = tmp_path / "artifacts" / "security-audit-01" + art01.mkdir(parents=True) + (art01 / "results.sarif").write_text(json.dumps({"runs": [{"results": []}]})) + + monkeypatch.setenv("NEEDS_JSON", json.dumps({"test-01": {"result": "success"}})) + monkeypatch.setattr(vr, "EXPECTED_COUNT", 1) + monkeypatch.setattr(vr, "ARTIFACTS_DIR", tmp_path / "artifacts") + monkeypatch.setattr(vr, "EXPECTED_RESULTS_PATH", tmp_path / "expected_results.yml") + monkeypatch.delenv("GITHUB_STEP_SUMMARY", raising=False) + monkeypatch.chdir(tmp_path) + + assert vr.main() == 1 + assert "did not run" in (tmp_path / "validation-report.md").read_text() + def test_conclusion_mismatch_returns_one( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/integration-tests/validate_results.py b/integration-tests/validate_results.py index 6b2d740..ac7dd47 100644 --- a/integration-tests/validate_results.py +++ b/integration-tests/validate_results.py @@ -139,6 +139,20 @@ def validate_test( return errors +def check_audited(expected: dict[str, Any], pip_audit_path: Path | None) -> list[str]: + """Error when a pip-audit case has no report or audited zero dependencies. + + The action used to report a crashed or empty audit as clean, so a case + without a report would otherwise pass as "no vulns". + """ + if expected.get("pip_audit_disabled", False): + return [] + deps = json.loads(pip_audit_path.read_bytes())["dependencies"] if pip_audit_path else [] + if not any("skip_reason" not in dep for dep in deps): + return ["pip-audit: no report or zero dependencies audited (the audit did not run)"] + return [] + + # --------------------------------------------------------------------------- # Report generation # --------------------------------------------------------------------------- @@ -251,21 +265,18 @@ def main() -> int: if sarif_path.exists(): bandit_findings = parse_sarif(sarif_path) - pip_audit_path = artifact_dir / "pip-audit-report.json" - if pip_audit_path.exists(): + # Artifact upload uses least common ancestor, so the file may be nested + # e.g. artifacts/security-audit-08/08-poetry-src-both/pip-audit-report.json + pip_audit_path = next(artifact_dir.rglob("pip-audit-report.json"), None) + if pip_audit_path: pip_audit_findings = parse_pip_audit(pip_audit_path) - else: - # Artifact upload uses least common ancestor, so the file may be nested - # e.g. artifacts/security-audit-08/08-poetry-src-both/pip-audit-report.json - nested = next(artifact_dir.rglob("pip-audit-report.json"), None) - if nested: - pip_audit_findings = parse_pip_audit(nested) all_bandit[num] = bandit_findings all_pip_audit[num] = pip_audit_findings conclusion = conclusions.get(num, "missing") errors = validate_test(num, exp, conclusion, bandit_findings, pip_audit_findings) + errors += check_audited(exp, pip_audit_path) all_errors[num] = errors status = "✅" if not errors else "❌" diff --git a/src/python_security_auditing/__main__.py b/src/python_security_auditing/__main__.py index 71f9c98..2363132 100644 --- a/src/python_security_auditing/__main__.py +++ b/src/python_security_auditing/__main__.py @@ -9,7 +9,7 @@ from .annotations import emit_annotations from .pr_comment import upsert_pr_comment from .report import build_markdown, check_thresholds, write_step_summary -from .runners import generate_requirements, read_bandit_sarif, run_pip_audit +from .runners import AuditError, generate_requirements, read_bandit_sarif, run_pip_audit from .settings import Settings @@ -39,10 +39,15 @@ def main() -> None: f"[debug] generating requirements for package_manager={settings.package_manager}", file=sys.stderr, ) - requirements_path = generate_requirements(settings) - if settings.debug: - print(f"[debug] running pip-audit on {requirements_path}", file=sys.stderr) - pip_audit_report = run_pip_audit(requirements_path, settings) + try: + requirements_path = generate_requirements(settings) + if settings.debug: + print(f"[debug] running pip-audit on {requirements_path}", file=sys.stderr) + pip_audit_report = run_pip_audit(requirements_path, settings) + except (AuditError, FileNotFoundError) as exc: + headline = "pip-audit did NOT run, so dependencies were NOT audited." + write_step_summary(f"## ❌ {headline}\n\n```\n{exc}\n```\n", settings) + sys.exit(f"{headline}\n{exc}") if settings.debug: print(f"[debug] pip-audit findings: {len(pip_audit_report)}", file=sys.stderr) diff --git a/src/python_security_auditing/runners.py b/src/python_security_auditing/runners.py index 8accbe2..30f3a68 100644 --- a/src/python_security_auditing/runners.py +++ b/src/python_security_auditing/runners.py @@ -13,6 +13,10 @@ from .settings import Settings +class AuditError(Exception): + """The dependency list or the pip-audit report could not be produced.""" + + def _resolve_exe(name: str) -> str: """Resolve an executable name to its full path via PATH, raising if not found.""" resolved = shutil.which(name) @@ -24,14 +28,24 @@ def _resolve_exe(name: str) -> str: def generate_requirements(settings: Settings) -> Path: """Return a requirements.txt Path suitable for pip-audit. - For package managers that don't produce a file directly (pip, pipenv), + For package managers that don't produce a file directly (pipenv), captures stdout into a temp file. For 'requirements', returns the - configured path unchanged. + configured path unchanged. Raises AuditError when no list can be produced. """ pm = settings.package_manager + if pm == "pip": + # `pip freeze` here would list this action's own environment, not the caller's. + raise AuditError( + "package_manager 'pip' is not supported: run `pip freeze > requirements.txt` " + "in your job and use `package_manager: requirements`." + ) + if pm == "requirements": - return Path(settings.requirements_file) + path = Path(settings.requirements_file) + if not path.is_file(): + raise AuditError(f"requirements file not found: {path}") + return path tmp = tempfile.NamedTemporaryFile(suffix="-requirements.txt", delete=False, mode="w") tmp.close() @@ -44,6 +58,7 @@ def generate_requirements(settings: Settings) -> Path: "--format", "requirements-txt", "--no-hashes", + "--no-emit-project", "-o", str(out_path), ] @@ -57,23 +72,12 @@ def generate_requirements(settings: Settings) -> Path: text=True, ) except subprocess.CalledProcessError as exc: - print( - f"uv export failed (no lockfile?): {exc.stderr.strip()}", - file=sys.stderr, - ) - return out_path + raise AuditError(f"uv export failed: {exc.stderr.strip()}") from exc if settings.debug: print( f"[debug] generated requirements ({out_path}):\n{out_path.read_text()}", file=sys.stderr, ) - elif pm == "pip": - result = subprocess.run( # nosec B603,B605 -- list args, full path via _resolve_exe() - [_resolve_exe("pip"), "freeze"], capture_output=True, text=True, check=True - ) - out_path.write_text(result.stdout) - if settings.debug: - print(f"[debug] pip freeze output ({out_path}):\n{result.stdout}", file=sys.stderr) elif pm == "poetry": # poetry-plugin-export is bundled in Poetry 1.8+; ignore failure here subprocess.run( # nosec B603,B605 -- list args, full path via _resolve_exe() @@ -98,28 +102,23 @@ def generate_requirements(settings: Settings) -> Path: text=True, ) except subprocess.CalledProcessError as exc: - print( - f"poetry export failed (no lockfile?): {exc.stderr.strip()}", - file=sys.stderr, - ) - return out_path + raise AuditError(f"poetry export failed: {exc.stderr.strip()}") from exc if settings.debug: print( f"[debug] poetry export output ({out_path}):\n{out_path.read_text()}", file=sys.stderr, ) elif pm == "pipenv": + # Without a lockfile, `pipenv requirements` exits 0 and prints no packages. + if not Path("Pipfile.lock").is_file(): + raise AuditError("Pipfile.lock not found: commit it or run `pipenv lock` first.") try: result = subprocess.run( # nosec B603,B605 -- list args, full path via _resolve_exe() [_resolve_exe("pipenv"), "requirements"], capture_output=True, text=True, check=True ) out_path.write_text(result.stdout) except subprocess.CalledProcessError as exc: - print( - f"pipenv requirements failed (no lockfile?): {exc.stderr.strip()}", - file=sys.stderr, - ) - return out_path + raise AuditError(f"pipenv requirements failed: {exc.stderr.strip()}") from exc if settings.debug: print( f"[debug] pipenv requirements output ({out_path}):\n{result.stdout}", @@ -174,20 +173,21 @@ def read_bandit_sarif(sarif_path: Path) -> dict[str, Any]: def run_pip_audit( requirements_path: Path, settings: Settings | None = None ) -> list[dict[str, Any]]: - """Run pip-audit, write pip-audit-report.json, return parsed report.""" + """Run pip-audit, write pip-audit-report.json, return parsed report. + + Raises AuditError when pip-audit does not produce a JSON report. + """ output_file = Path("pip-audit-report.json") cmd = [_resolve_exe("pip-audit"), "-r", str(requirements_path), "--no-deps", "-f", "json"] + if settings and settings.package_manager != "requirements": + # Exports are fully pinned: audit them as written. Without --disable-pip, + # pip-audit resolves them with pip (building the project) and ignores --no-deps. + cmd.append("--disable-pip") if settings and settings.debug: print(f"[debug] pip-audit command: {cmd}", file=sys.stderr) result = subprocess.run(cmd, capture_output=True, text=True) # nosec B603,B605 -- list args, full path via _resolve_exe() - # pip-audit exits 1 when vulnerabilities are found — that is expected - if result.returncode not in (0, 1): - print( - f"pip-audit exited with unexpected code {result.returncode}:\n{result.stderr}", - file=sys.stderr, - ) if settings and settings.debug: print( @@ -196,12 +196,13 @@ def run_pip_audit( file=sys.stderr, ) - raw = result.stdout.strip() - if raw: - parsed: Any = json.loads(raw) - output_file.write_text(raw) - # pip-audit 2.7+ wraps output in {"dependencies": [...], "fixes": [...]} - if isinstance(parsed, dict): - return list(parsed.get("dependencies", [])) - return list(parsed) - return [] + # pip-audit exits 1 both when it finds vulnerabilities and when it fails, + # so only a JSON report proves that the audit ran. + try: + dependencies = list(json.loads(result.stdout)["dependencies"]) + except (ValueError, KeyError, TypeError) as exc: + raise AuditError( + f"pip-audit failed (exit {result.returncode}):\n{result.stderr.strip()}" + ) from exc + output_file.write_text(result.stdout) + return dependencies diff --git a/tests/test_main.py b/tests/test_main.py index bbf6eab..3d57321 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -12,6 +12,11 @@ FIXTURES = Path(__file__).parent / "fixtures" +def _clean_audit(cmd: list[str], **kwargs: object) -> MagicMock: + """uv export succeeds and pip-audit reports no vulnerabilities.""" + return MagicMock(returncode=0, stderr="", stdout='{"dependencies": [], "fixes": []}') + + def _make_sarif_mock(sarif_content: str, pip_stdout: str = "[]") -> object: """Return a mock_subprocess factory that feeds a SARIF file and pip-audit output.""" @@ -33,16 +38,9 @@ def test_comment_on_never_never_calls_upsert( monkeypatch.setenv("GITHUB_TOKEN", "tok") monkeypatch.chdir(tmp_path) - uv_exc = subprocess.CalledProcessError(2, "uv", stderr="No uv.lock found") - - def mock_subprocess(cmd: list[str], **kwargs: object) -> MagicMock: - if cmd[0] == "uv": - raise uv_exc - return MagicMock(returncode=0, stderr="", stdout="[]") - with ( patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), - patch("python_security_auditing.runners.subprocess.run", side_effect=mock_subprocess), + patch("python_security_auditing.runners.subprocess.run", side_effect=_clean_audit), patch("python_security_auditing.__main__.emit_annotations"), patch("python_security_auditing.__main__.upsert_pr_comment") as mock_comment, ): @@ -65,16 +63,9 @@ def test_comment_on_blocking_calls_upsert_when_blocking( monkeypatch.setenv("COMMENT_ON", "blocking") monkeypatch.chdir(tmp_path) - uv_exc = subprocess.CalledProcessError(2, "uv", stderr="No uv.lock found") - - def mock_subprocess(cmd: list[str], **kwargs: object) -> MagicMock: - if cmd[0] == "uv": - raise uv_exc - return MagicMock(returncode=0, stderr="", stdout="[]") - with ( patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), - patch("python_security_auditing.runners.subprocess.run", side_effect=mock_subprocess), + patch("python_security_auditing.runners.subprocess.run", side_effect=_clean_audit), patch("python_security_auditing.__main__.emit_annotations"), patch("python_security_auditing.__main__.upsert_pr_comment") as mock_comment, ): @@ -96,16 +87,9 @@ def test_comment_on_blocking_skips_upsert_when_clean( monkeypatch.setenv("COMMENT_ON", "blocking") monkeypatch.chdir(tmp_path) - uv_exc = subprocess.CalledProcessError(2, "uv", stderr="No uv.lock found") - - def mock_subprocess(cmd: list[str], **kwargs: object) -> MagicMock: - if cmd[0] == "uv": - raise uv_exc - return MagicMock(returncode=0, stderr="", stdout="[]") - with ( patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), - patch("python_security_auditing.runners.subprocess.run", side_effect=mock_subprocess), + patch("python_security_auditing.runners.subprocess.run", side_effect=_clean_audit), patch("python_security_auditing.__main__.emit_annotations"), patch("python_security_auditing.__main__.upsert_pr_comment") as mock_comment, ): @@ -113,54 +97,31 @@ def mock_subprocess(cmd: list[str], **kwargs: object) -> MagicMock: mock_comment.assert_not_called() -def test_main_succeeds_when_uv_lockfile_missing_and_no_bandit_issues( +def test_main_fails_closed_when_uv_export_fails( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: + """A failed export must fail the step and say so, not report 'No vulnerabilities found'.""" sarif_path = tmp_path / "results.sarif" sarif_path.write_text((FIXTURES / "bandit_clean.sarif").read_text()) + summary_path = tmp_path / "summary.md" monkeypatch.setenv("PACKAGE_MANAGER", "uv") monkeypatch.setenv("TOOLS", "bandit,pip-audit") monkeypatch.setenv("BANDIT_SARIF_PATH", str(sarif_path)) + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary_path)) monkeypatch.chdir(tmp_path) uv_exc = subprocess.CalledProcessError(2, "uv", stderr="No uv.lock found") - - def mock_subprocess(cmd: list[str], **kwargs: object) -> MagicMock: - if cmd[0] == "uv": - raise uv_exc - return MagicMock(returncode=0, stderr="", stdout="[]") - - with ( - patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), - patch("python_security_auditing.runners.subprocess.run", side_effect=mock_subprocess), - patch("python_security_auditing.__main__.emit_annotations"), - ): - main() # should return normally without calling sys.exit(1) - - -def test_main_fails_when_bandit_blocks_despite_missing_lockfile( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path -) -> None: - sarif_path = tmp_path / "results.sarif" - sarif_path.write_text((FIXTURES / "bandit_issues.sarif").read_text()) - monkeypatch.setenv("PACKAGE_MANAGER", "uv") - monkeypatch.setenv("TOOLS", "bandit,pip-audit") - monkeypatch.setenv("BANDIT_SARIF_PATH", str(sarif_path)) - monkeypatch.setenv("BANDIT_SEVERITY_THRESHOLD", "high") - monkeypatch.chdir(tmp_path) - - uv_exc = subprocess.CalledProcessError(2, "uv", stderr="No uv.lock found") - - def mock_subprocess(cmd: list[str], **kwargs: object) -> MagicMock: - if cmd[0] == "uv": - raise uv_exc - return MagicMock(returncode=0, stderr="", stdout="[]") - with ( patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), - patch("python_security_auditing.runners.subprocess.run", side_effect=mock_subprocess), + patch("python_security_auditing.runners.subprocess.run", side_effect=uv_exc), patch("python_security_auditing.__main__.emit_annotations"), ): with pytest.raises(SystemExit) as exc_info: main() - assert exc_info.value.code == 1 + + message = str(exc_info.value.code) # a str code exits with status 1 + assert "pip-audit did NOT run" in message + assert "uv export failed: No uv.lock found" in message + summary = summary_path.read_text() + assert "pip-audit did NOT run" in summary + assert "No vulnerabilities found" not in summary diff --git a/tests/test_runners.py b/tests/test_runners.py index 4c0e22d..e6c82a2 100644 --- a/tests/test_runners.py +++ b/tests/test_runners.py @@ -8,10 +8,16 @@ from unittest.mock import MagicMock, patch import pytest -from python_security_auditing.runners import generate_requirements, read_bandit_sarif, run_pip_audit +from python_security_auditing.runners import ( + AuditError, + generate_requirements, + read_bandit_sarif, + run_pip_audit, +) from python_security_auditing.settings import Settings FIXTURES = Path(__file__).parent / "fixtures" +CLEAN_REPORT = json.dumps({"dependencies": [], "fixes": []}) # --------------------------------------------------------------------------- @@ -19,13 +25,26 @@ # --------------------------------------------------------------------------- -def test_requirements_mode_returns_configured_path(monkeypatch: pytest.MonkeyPatch) -> None: +def test_requirements_mode_returns_configured_path( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.chdir(tmp_path) + (tmp_path / "custom-requirements.txt").write_text("requests==2.31.0\n") monkeypatch.setenv("PACKAGE_MANAGER", "requirements") monkeypatch.setenv("REQUIREMENTS_FILE", "custom-requirements.txt") s = Settings() assert generate_requirements(s) == Path("custom-requirements.txt") +def test_requirements_mode_raises_when_file_missing( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("PACKAGE_MANAGER", "requirements") + with pytest.raises(AuditError, match="requirements file not found: requirements.txt"): + generate_requirements(Settings()) + + def test_uv_mode_calls_uv_export(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: monkeypatch.setenv("PACKAGE_MANAGER", "uv") s = Settings() @@ -42,24 +61,18 @@ def test_uv_mode_calls_uv_export(monkeypatch: pytest.MonkeyPatch, tmp_path: Path assert "export" in cmd assert "--format" in cmd assert "requirements-txt" in cmd + # the caller's own project is not on PyPI: emitting it makes pip-audit build it + assert "--no-emit-project" in cmd assert str(result).endswith("-requirements.txt") -def test_pip_mode_calls_pip_freeze(monkeypatch: pytest.MonkeyPatch) -> None: +def test_pip_mode_is_rejected(monkeypatch: pytest.MonkeyPatch) -> None: + """`pip freeze` would list the action's own environment, so pip mode must fail.""" monkeypatch.setenv("PACKAGE_MANAGER", "pip") - s = Settings() - - freeze_output = "requests==2.31.0\n" - with ( - patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), - patch("python_security_auditing.runners.subprocess.run") as mock_run, - ): - mock_run.return_value = MagicMock(returncode=0, stdout=freeze_output) - result = generate_requirements(s) - - cmd = mock_run.call_args[0][0] - assert cmd == ["pip", "freeze"] - assert result.read_text() == freeze_output + with patch("python_security_auditing.runners.subprocess.run") as mock_run: + with pytest.raises(AuditError, match=r"pip freeze > requirements\.txt"): + generate_requirements(Settings()) + mock_run.assert_not_called() def test_poetry_mode_calls_poetry_export(monkeypatch: pytest.MonkeyPatch) -> None: @@ -82,7 +95,11 @@ def test_poetry_mode_calls_poetry_export(monkeypatch: pytest.MonkeyPatch) -> Non assert str(result).endswith("-requirements.txt") -def test_pipenv_mode_calls_pipenv_requirements(monkeypatch: pytest.MonkeyPatch) -> None: +def test_pipenv_mode_calls_pipenv_requirements( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.chdir(tmp_path) + (tmp_path / "Pipfile.lock").write_text("{}") monkeypatch.setenv("PACKAGE_MANAGER", "pipenv") s = Settings() @@ -181,19 +198,27 @@ def test_run_pip_audit_parses_json(tmp_path: Path, monkeypatch: pytest.MonkeyPat assert (tmp_path / "pip-audit-report.json").exists() -def test_run_pip_audit_returns_empty_on_no_output( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch +@pytest.mark.parametrize( + ("returncode", "stdout"), + [(1, ""), (0, ""), (1, "Traceback (most recent call last):"), (0, "[]"), (2, "{}")], +) +def test_run_pip_audit_raises_without_json_report( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, returncode: int, stdout: str ) -> None: + """pip-audit exits 1 for findings AND for fatal errors: only a JSON report counts.""" monkeypatch.chdir(tmp_path) with ( patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), patch("python_security_auditing.runners.subprocess.run") as mock_run, ): - mock_run.return_value = MagicMock(returncode=0, stderr="", stdout="") - report = run_pip_audit(Path("requirements.txt")) + mock_run.return_value = MagicMock( + returncode=returncode, stderr="ERROR:pip_audit._cli:boom", stdout=stdout + ) + with pytest.raises(AuditError, match=rf"exit {returncode}\):\nERROR:pip_audit\._cli:boom"): + run_pip_audit(Path("requirements.txt")) - assert report == [] + assert not (tmp_path / "pip-audit-report.json").exists() def test_run_pip_audit_uses_requirements_path( @@ -206,7 +231,7 @@ def test_run_pip_audit_uses_requirements_path( patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), patch("python_security_auditing.runners.subprocess.run") as mock_run, ): - mock_run.return_value = MagicMock(returncode=0, stderr="", stdout="[]") + mock_run.return_value = MagicMock(returncode=0, stderr="", stdout=CLEAN_REPORT) run_pip_audit(req_path) cmd = mock_run.call_args[0][0] @@ -215,85 +240,60 @@ def test_run_pip_audit_uses_requirements_path( assert "json" in cmd -def test_run_pip_audit_command_includes_no_deps( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch +@pytest.mark.parametrize( + ("package_manager", "disable_pip"), + [("uv", True), ("poetry", True), ("pipenv", True), ("requirements", False)], +) +def test_run_pip_audit_disables_pip_for_pinned_exports( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, package_manager: str, disable_pip: bool ) -> None: + """Exports are fully pinned; a requirements file may not be, so pip resolves it.""" monkeypatch.chdir(tmp_path) + monkeypatch.setenv("PACKAGE_MANAGER", package_manager) with ( patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), patch("python_security_auditing.runners.subprocess.run") as mock_run, ): - mock_run.return_value = MagicMock(returncode=0, stderr="", stdout="[]") - run_pip_audit(Path("requirements.txt")) + mock_run.return_value = MagicMock(returncode=0, stderr="", stdout=CLEAN_REPORT) + run_pip_audit(Path("requirements.txt"), Settings()) cmd = mock_run.call_args[0][0] assert "--no-deps" in cmd + assert ("--disable-pip" in cmd) is disable_pip # --------------------------------------------------------------------------- -# generate_requirements — missing lockfile handling +# generate_requirements — export failures must fail, not return an empty file # --------------------------------------------------------------------------- -def test_generate_requirements_uv_returns_empty_on_missing_lockfile( - monkeypatch: pytest.MonkeyPatch, -) -> None: - monkeypatch.setenv("PACKAGE_MANAGER", "uv") - s = Settings() - exc = subprocess.CalledProcessError(2, "uv", stderr="No uv.lock found") - with ( - patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), - patch("python_security_auditing.runners.subprocess.run", side_effect=exc), - ): - result = generate_requirements(s) - assert result.exists() - assert result.stat().st_size == 0 - - -def test_generate_requirements_poetry_returns_empty_on_missing_lockfile( - monkeypatch: pytest.MonkeyPatch, +@pytest.mark.parametrize("package_manager", ["uv", "poetry", "pipenv"]) +def test_generate_requirements_raises_when_export_fails( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, package_manager: str ) -> None: - monkeypatch.setenv("PACKAGE_MANAGER", "poetry") - s = Settings() - export_exc = subprocess.CalledProcessError(1, "poetry", stderr="poetry.lock not found") + monkeypatch.chdir(tmp_path) + (tmp_path / "Pipfile.lock").write_text("{}") + monkeypatch.setenv("PACKAGE_MANAGER", package_manager) + exc = subprocess.CalledProcessError(2, package_manager, stderr="lockfile is broken") + # poetry first runs `poetry self add`, which may fail without consequence + side_effect = [MagicMock(returncode=1), exc] if package_manager == "poetry" else exc with ( patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), - patch("python_security_auditing.runners.subprocess.run") as mock_run, + patch("python_security_auditing.runners.subprocess.run", side_effect=side_effect), ): - # self add uses check=False so a non-zero return is silently ignored; - # export raises CalledProcessError to simulate a missing lockfile - mock_run.side_effect = [MagicMock(returncode=1), export_exc] - result = generate_requirements(s) - assert result.exists() - assert result.stat().st_size == 0 + with pytest.raises(AuditError, match=f"{package_manager} .*failed: lockfile is broken"): + generate_requirements(Settings()) -def test_generate_requirements_pipenv_returns_empty_on_missing_lockfile( - monkeypatch: pytest.MonkeyPatch, +def test_generate_requirements_pipenv_raises_without_lockfile( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: + """`pipenv requirements` exits 0 with no packages when Pipfile.lock is missing.""" + monkeypatch.chdir(tmp_path) monkeypatch.setenv("PACKAGE_MANAGER", "pipenv") - s = Settings() - exc = subprocess.CalledProcessError(1, "pipenv", stderr="Pipfile.lock not found") - with ( - patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), - patch("python_security_auditing.runners.subprocess.run", side_effect=exc), - ): - result = generate_requirements(s) - assert result.exists() - assert result.stat().st_size == 0 - - -def test_generate_requirements_uv_warns_on_missing_lockfile( - monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] -) -> None: - monkeypatch.setenv("PACKAGE_MANAGER", "uv") - s = Settings() - exc = subprocess.CalledProcessError(2, "uv", stderr="No uv.lock found") - with ( - patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), - patch("python_security_auditing.runners.subprocess.run", side_effect=exc), - ): - generate_requirements(s) - assert "uv export failed" in capsys.readouterr().err + with patch("python_security_auditing.runners.subprocess.run") as mock_run: + with pytest.raises(AuditError, match="Pipfile.lock not found"): + generate_requirements(Settings()) + mock_run.assert_not_called() # --------------------------------------------------------------------------- From fc6f3e88eb560a90791e8eefe591807944b19cc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Houpert?= <10154151+lhoupert@users.noreply.github.com> Date: Tue, 6 Oct 2026 15:22:07 +0100 Subject: [PATCH 2/5] fix: honour PIPENV_PIPFILE and stop leaking temp files on export failure The Pipfile.lock check only looked in the current directory. pipenv keeps the lock at .lock, so it now honours PIPENV_PIPFILE. The lock check and the executable lookup now run before the temp requirements file is created, and a failed export deletes that file. Refs #84 Co-Authored-By: Claude Opus 5.5 (1M context) --- src/python_security_auditing/runners.py | 24 +++++++++++++++++------- tests/test_runners.py | 23 +++++++++++++++++++++++ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/python_security_auditing/runners.py b/src/python_security_auditing/runners.py index 30f3a68..bfdd056 100644 --- a/src/python_security_auditing/runners.py +++ b/src/python_security_auditing/runners.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import os import shutil import subprocess import sys @@ -47,13 +48,22 @@ def generate_requirements(settings: Settings) -> Path: raise AuditError(f"requirements file not found: {path}") return path + if pm == "pipenv": + # pipenv keeps the lock at .lock. Without it, `pipenv requirements` + # exits 0 and prints no packages. + lock = Path(os.environ.get("PIPENV_PIPFILE", "Pipfile") + ".lock") + if not lock.is_file(): + raise AuditError(f"{lock} not found: commit it or run `pipenv lock` first.") + + # Check everything that can fail before creating the temp file, so it is not leaked. + exe = _resolve_exe(pm) tmp = tempfile.NamedTemporaryFile(suffix="-requirements.txt", delete=False, mode="w") tmp.close() out_path = Path(tmp.name) if pm == "uv": cmd = [ - _resolve_exe("uv"), + exe, "export", "--format", "requirements-txt", @@ -72,6 +82,7 @@ def generate_requirements(settings: Settings) -> Path: text=True, ) except subprocess.CalledProcessError as exc: + out_path.unlink() raise AuditError(f"uv export failed: {exc.stderr.strip()}") from exc if settings.debug: print( @@ -81,7 +92,7 @@ def generate_requirements(settings: Settings) -> Path: elif pm == "poetry": # poetry-plugin-export is bundled in Poetry 1.8+; ignore failure here subprocess.run( # nosec B603,B605 -- list args, full path via _resolve_exe() - [_resolve_exe("poetry"), "self", "add", "poetry-plugin-export"], + [exe, "self", "add", "poetry-plugin-export"], check=False, capture_output=True, text=True, @@ -89,7 +100,7 @@ def generate_requirements(settings: Settings) -> Path: try: subprocess.run( # nosec B603,B605 -- list args, full path via _resolve_exe() [ - _resolve_exe("poetry"), + exe, "export", "--format", "requirements.txt", @@ -102,6 +113,7 @@ def generate_requirements(settings: Settings) -> Path: text=True, ) except subprocess.CalledProcessError as exc: + out_path.unlink() raise AuditError(f"poetry export failed: {exc.stderr.strip()}") from exc if settings.debug: print( @@ -109,15 +121,13 @@ def generate_requirements(settings: Settings) -> Path: file=sys.stderr, ) elif pm == "pipenv": - # Without a lockfile, `pipenv requirements` exits 0 and prints no packages. - if not Path("Pipfile.lock").is_file(): - raise AuditError("Pipfile.lock not found: commit it or run `pipenv lock` first.") try: result = subprocess.run( # nosec B603,B605 -- list args, full path via _resolve_exe() - [_resolve_exe("pipenv"), "requirements"], capture_output=True, text=True, check=True + [exe, "requirements"], capture_output=True, text=True, check=True ) out_path.write_text(result.stdout) except subprocess.CalledProcessError as exc: + out_path.unlink() raise AuditError(f"pipenv requirements failed: {exc.stderr.strip()}") from exc if settings.debug: print( diff --git a/tests/test_runners.py b/tests/test_runners.py index e6c82a2..6002a76 100644 --- a/tests/test_runners.py +++ b/tests/test_runners.py @@ -4,6 +4,7 @@ import json import subprocess +import tempfile from pathlib import Path from unittest.mock import MagicMock, patch @@ -273,6 +274,9 @@ def test_generate_requirements_raises_when_export_fails( monkeypatch.chdir(tmp_path) (tmp_path / "Pipfile.lock").write_text("{}") monkeypatch.setenv("PACKAGE_MANAGER", package_manager) + tmpdir = tmp_path / "tmp" + tmpdir.mkdir() + monkeypatch.setattr(tempfile, "tempdir", str(tmpdir)) exc = subprocess.CalledProcessError(2, package_manager, stderr="lockfile is broken") # poetry first runs `poetry self add`, which may fail without consequence side_effect = [MagicMock(returncode=1), exc] if package_manager == "poetry" else exc @@ -282,6 +286,7 @@ def test_generate_requirements_raises_when_export_fails( ): with pytest.raises(AuditError, match=f"{package_manager} .*failed: lockfile is broken"): generate_requirements(Settings()) + assert list(tmpdir.iterdir()) == [] # no leaked temp requirements file def test_generate_requirements_pipenv_raises_without_lockfile( @@ -296,6 +301,24 @@ def test_generate_requirements_pipenv_raises_without_lockfile( mock_run.assert_not_called() +def test_generate_requirements_pipenv_uses_lockfile_next_to_pipenv_pipfile( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """pipenv keeps the lock at .lock, not in the current directory.""" + monkeypatch.chdir(tmp_path) + (tmp_path / "sub").mkdir() + (tmp_path / "sub" / "Pipfile.lock").write_text("{}") + monkeypatch.setenv("PIPENV_PIPFILE", str(tmp_path / "sub" / "Pipfile")) + monkeypatch.setenv("PACKAGE_MANAGER", "pipenv") + with ( + patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), + patch("python_security_auditing.runners.subprocess.run") as mock_run, + ): + mock_run.return_value = MagicMock(returncode=0, stdout="requests==2.31.0\n") + result = generate_requirements(Settings()) + assert result.read_text() == "requests==2.31.0\n" + + # --------------------------------------------------------------------------- # generate_requirements / run_pip_audit — tool-not-found handling # --------------------------------------------------------------------------- From 2ab9129cbad3efdcc6f6a566dec605de5a26d0d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Houpert?= <10154151+lhoupert@users.noreply.github.com> Date: Tue, 6 Oct 2026 15:23:27 +0100 Subject: [PATCH 3/5] fix: keep the full report when pip-audit cannot run, and flag empty audits When pip-audit could not run, the action exited before it wrote the bandit section, emitted annotations or updated the PR comment. A red job could then sit next to a stale "all clean" comment. It now renders the whole report with a "pip-audit did NOT run" section, emits an ::error:: annotation with the reason, updates the comment and then exits non-zero. A pip-audit-report.json left by an earlier run is deleted first, so the always() upload never publishes stale results. A valid report that audited zero dependencies, or skipped all of them, no longer says "No vulnerabilities found". The section shows the audited and skipped counts, and a ::warning:: is emitted when nothing was audited. Such a run does not fail: a project with no dependencies is legitimate. Refs #84 Co-Authored-By: Claude Opus 5.5 (1M context) --- src/python_security_auditing/__main__.py | 24 +++++++---- src/python_security_auditing/annotations.py | 17 +++++--- src/python_security_auditing/report.py | 19 +++++++-- src/python_security_auditing/runners.py | 5 ++- tests/fixtures/pip_audit_clean.json | 2 +- tests/test_annotations.py | 19 +++++++++ tests/test_main.py | 44 +++++++++++++++++++-- tests/test_report.py | 30 ++++++++++++++ 8 files changed, 138 insertions(+), 22 deletions(-) diff --git a/src/python_security_auditing/__main__.py b/src/python_security_auditing/__main__.py index 2363132..06bf994 100644 --- a/src/python_security_auditing/__main__.py +++ b/src/python_security_auditing/__main__.py @@ -9,7 +9,13 @@ from .annotations import emit_annotations from .pr_comment import upsert_pr_comment from .report import build_markdown, check_thresholds, write_step_summary -from .runners import AuditError, generate_requirements, read_bandit_sarif, run_pip_audit +from .runners import ( + PIP_AUDIT_REPORT, + AuditError, + generate_requirements, + read_bandit_sarif, + run_pip_audit, +) from .settings import Settings @@ -21,6 +27,7 @@ def main() -> None: bandit_report: dict[str, Any] = {} pip_audit_report: list[dict[str, Any]] = [] + pip_audit_error = "" if "bandit" in settings.enabled_tools: if settings.debug: @@ -39,28 +46,31 @@ def main() -> None: f"[debug] generating requirements for package_manager={settings.package_manager}", file=sys.stderr, ) + PIP_AUDIT_REPORT.unlink(missing_ok=True) # never upload an earlier run's report try: requirements_path = generate_requirements(settings) if settings.debug: print(f"[debug] running pip-audit on {requirements_path}", file=sys.stderr) pip_audit_report = run_pip_audit(requirements_path, settings) except (AuditError, FileNotFoundError) as exc: - headline = "pip-audit did NOT run, so dependencies were NOT audited." - write_step_summary(f"## ❌ {headline}\n\n```\n{exc}\n```\n", settings) - sys.exit(f"{headline}\n{exc}") + pip_audit_error = str(exc) if settings.debug: print(f"[debug] pip-audit findings: {len(pip_audit_report)}", file=sys.stderr) - markdown = build_markdown(bandit_report, pip_audit_report, settings) + markdown = build_markdown(bandit_report, pip_audit_report, settings, pip_audit_error) write_step_summary(markdown, settings) - emit_annotations(bandit_report, pip_audit_report, settings) + emit_annotations(bandit_report, pip_audit_report, settings, pip_audit_error) - has_blocking = check_thresholds(bandit_report, pip_audit_report, settings) + has_blocking = bool(pip_audit_error) or check_thresholds( + bandit_report, pip_audit_report, settings + ) if settings.github_token and settings.comment_on != "never": if settings.comment_on == "always" or has_blocking: upsert_pr_comment(markdown, settings) + if pip_audit_error: + sys.exit(f"pip-audit did NOT run, so dependencies were NOT audited.\n{pip_audit_error}") if has_blocking: sys.exit(1) diff --git a/src/python_security_auditing/annotations.py b/src/python_security_auditing/annotations.py index a6b0469..fb83df1 100644 --- a/src/python_security_auditing/annotations.py +++ b/src/python_security_auditing/annotations.py @@ -14,10 +14,16 @@ _SEVERITY_ORDER = {"HIGH": 0, "MEDIUM": 1, "LOW": 2} +def _escape(text: str) -> str: + """Escape workflow-command message data.""" + return text.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + + def emit_annotations( bandit_report: dict[str, Any], pip_audit_report: list[dict[str, Any]], settings: Settings, + pip_audit_error: str = "", ) -> None: """Print GitHub Actions workflow commands to stdout. @@ -37,15 +43,14 @@ def _sort_key(r: dict[str, Any]) -> int: fname = result.get("filename", "") line = result.get("line_number", 0) test_id = result.get("test_id", "") - text = ( - result.get("issue_text", "") - .replace("%", "%25") - .replace("\r", "%0D") - .replace("\n", "%0A") - ) + text = _escape(result.get("issue_text", "")) print(f"::{level} file={fname},line={line}::[{test_id}] {text}") if "pip-audit" in settings.enabled_tools: + if pip_audit_error: + print(f"::error::pip-audit did NOT run: {_escape(pip_audit_error)}") + elif not any("skip_reason" not in pkg for pkg in pip_audit_report): + print("::warning::pip-audit audited 0 dependencies, so nothing was checked") for pkg in pip_audit_report: if not pkg.get("vulns"): continue diff --git a/src/python_security_auditing/report.py b/src/python_security_auditing/report.py index 30ae842..b4e303a 100644 --- a/src/python_security_auditing/report.py +++ b/src/python_security_auditing/report.py @@ -13,6 +13,7 @@ def build_markdown( bandit_report: dict[str, Any], pip_audit_report: list[dict[str, Any]], settings: Settings, + pip_audit_error: str = "", ) -> str: """Build a full markdown security report.""" sections: list[str] = ["# Security Audit Report\n"] @@ -29,9 +30,9 @@ def build_markdown( sections.append(_bandit_section(bandit_report, settings)) if "pip-audit" in settings.enabled_tools: - sections.append(_pip_audit_section(pip_audit_report, settings)) + sections.append(_pip_audit_section(pip_audit_report, settings, pip_audit_error)) - blocking = check_thresholds(bandit_report, pip_audit_report, settings) + blocking = check_thresholds(bandit_report, pip_audit_report, settings) or bool(pip_audit_error) sections.append("---\n") if blocking: sections.append("**Result: ❌ Blocking issues found — see details above.**\n") @@ -103,7 +104,7 @@ def _bandit_section(report: dict[str, Any], settings: Settings) -> str: return "\n".join(lines) -def _pip_audit_section(report: list[dict[str, Any]], settings: Settings) -> str: +def _pip_audit_section(report: list[dict[str, Any]], settings: Settings, error: str) -> str: vulnerable = [pkg for pkg in report if pkg.get("vulns")] security_url = ( f"https://github.com/{settings.github_repository}/security/dependabot" @@ -117,6 +118,18 @@ def _pip_audit_section(report: list[dict[str, Any]], settings: Settings) -> str: ) lines = [heading] + if error: + lines.append( + f"❌ pip-audit did NOT run, so dependencies were NOT audited.\n\n```\n{error}\n```\n" + ) + return "\n".join(lines) + + audited = sum("skip_reason" not in pkg for pkg in report) + lines.append(f"_Dependencies audited: {audited}, skipped: {len(report) - audited}._\n") + if not audited: + lines.append("⚠️ No dependencies were audited.\n") + return "\n".join(lines) + if not vulnerable: lines.append("✅ No vulnerabilities found.\n") return "\n".join(lines) diff --git a/src/python_security_auditing/runners.py b/src/python_security_auditing/runners.py index bfdd056..32976c6 100644 --- a/src/python_security_auditing/runners.py +++ b/src/python_security_auditing/runners.py @@ -13,6 +13,8 @@ from .settings import Settings +PIP_AUDIT_REPORT = Path("pip-audit-report.json") + class AuditError(Exception): """The dependency list or the pip-audit report could not be produced.""" @@ -187,7 +189,6 @@ def run_pip_audit( Raises AuditError when pip-audit does not produce a JSON report. """ - output_file = Path("pip-audit-report.json") cmd = [_resolve_exe("pip-audit"), "-r", str(requirements_path), "--no-deps", "-f", "json"] if settings and settings.package_manager != "requirements": # Exports are fully pinned: audit them as written. Without --disable-pip, @@ -214,5 +215,5 @@ def run_pip_audit( raise AuditError( f"pip-audit failed (exit {result.returncode}):\n{result.stderr.strip()}" ) from exc - output_file.write_text(result.stdout) + PIP_AUDIT_REPORT.write_text(result.stdout) return dependencies diff --git a/tests/fixtures/pip_audit_clean.json b/tests/fixtures/pip_audit_clean.json index fe51488..adbf1f9 100644 --- a/tests/fixtures/pip_audit_clean.json +++ b/tests/fixtures/pip_audit_clean.json @@ -1 +1 @@ -[] +[{"name": "requests", "version": "2.32.0", "vulns": []}] diff --git a/tests/test_annotations.py b/tests/test_annotations.py index fbdce69..641c3d0 100644 --- a/tests/test_annotations.py +++ b/tests/test_annotations.py @@ -104,6 +104,25 @@ def test_pip_clean_emits_nothing( assert capsys.readouterr().out == "" +@pytest.mark.parametrize( + "report", + [[], [{"name": "my-app", "version": "0.1.0", "skip_reason": "Dependency not found on PyPI"}]], +) +def test_pip_zero_audited_emits_warning( + bandit_clean: dict[str, Any], report: list[Any], capsys: pytest.CaptureFixture[str] +) -> None: + emit_annotations(bandit_clean, report, Settings()) + assert capsys.readouterr().out.startswith("::warning::pip-audit audited 0 dependencies") + + +def test_pip_audit_error_emits_escaped_error( + bandit_clean: dict[str, Any], capsys: pytest.CaptureFixture[str] +) -> None: + emit_annotations(bandit_clean, [], Settings(), pip_audit_error="uv failed\n100% broken") + out = capsys.readouterr().out + assert out == "::error::pip-audit did NOT run: uv failed%0A100%25 broken\n" + + def test_bandit_only_tool_skips_pip( bandit_clean: dict[str, Any], pip_fixable: list[Any], diff --git a/tests/test_main.py b/tests/test_main.py index 3d57321..d478ffe 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -98,7 +98,7 @@ def test_comment_on_blocking_skips_upsert_when_clean( def test_main_fails_closed_when_uv_export_fails( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: """A failed export must fail the step and say so, not report 'No vulnerabilities found'.""" sarif_path = tmp_path / "results.sarif" @@ -109,12 +109,13 @@ def test_main_fails_closed_when_uv_export_fails( monkeypatch.setenv("BANDIT_SARIF_PATH", str(sarif_path)) monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary_path)) monkeypatch.chdir(tmp_path) + # a report left by an earlier run must not be uploaded as this run's result + (tmp_path / "pip-audit-report.json").write_text('{"dependencies": [], "fixes": []}') - uv_exc = subprocess.CalledProcessError(2, "uv", stderr="No uv.lock found") + uv_exc = subprocess.CalledProcessError(2, "uv", stderr="No uv.lock found\nsecond line") with ( patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), patch("python_security_auditing.runners.subprocess.run", side_effect=uv_exc), - patch("python_security_auditing.__main__.emit_annotations"), ): with pytest.raises(SystemExit) as exc_info: main() @@ -125,3 +126,40 @@ def test_main_fails_closed_when_uv_export_fails( summary = summary_path.read_text() assert "pip-audit did NOT run" in summary assert "No vulnerabilities found" not in summary + out = capsys.readouterr().out + assert "::error::pip-audit did NOT run: uv export failed: No uv.lock found%0Asecond line" in out + assert not (tmp_path / "pip-audit-report.json").exists() + + +def test_main_reports_bandit_and_comments_when_pip_audit_cannot_run( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A failed audit must not hide bandit results or leave a stale PR comment.""" + sarif_path = tmp_path / "results.sarif" + sarif_path.write_text((FIXTURES / "bandit_issues.sarif").read_text()) + summary_path = tmp_path / "summary.md" + monkeypatch.setenv("PACKAGE_MANAGER", "uv") + monkeypatch.setenv("TOOLS", "bandit,pip-audit") + monkeypatch.setenv("BANDIT_SARIF_PATH", str(sarif_path)) + monkeypatch.setenv("BANDIT_SEVERITY_THRESHOLD", "high") + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary_path)) + monkeypatch.setenv("GITHUB_TOKEN", "tok") + monkeypatch.setenv("COMMENT_ON", "blocking") + monkeypatch.chdir(tmp_path) + + uv_exc = subprocess.CalledProcessError(2, "uv", stderr="No uv.lock found") + with ( + patch("python_security_auditing.runners.shutil.which", side_effect=lambda exe: exe), + patch("python_security_auditing.runners.subprocess.run", side_effect=uv_exc), + patch("python_security_auditing.__main__.upsert_pr_comment") as mock_comment, + ): + with pytest.raises(SystemExit) as exc_info: + main() + + assert exc_info.value.code not in (None, 0) + summary = summary_path.read_text() + assert "B404" in summary # bandit section still rendered + assert "pip-audit did NOT run" in summary + assert "::error file=src/app.py,line=2::[B404]" in capsys.readouterr().out + mock_comment.assert_called_once() + assert "pip-audit did NOT run" in mock_comment.call_args[0][0] diff --git a/tests/test_report.py b/tests/test_report.py index 90ba923..42ac3f2 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -189,6 +189,36 @@ def test_markdown_pip_table(bandit_clean: dict[str, Any], pip_fixable: list[Any] assert "GHSA-j8r2-6x86-q33q" in md +def test_markdown_pip_counts_audited_and_skipped(bandit_clean: dict[str, Any]) -> None: + report: list[dict[str, Any]] = [ + {"name": "requests", "version": "2.32.0", "vulns": []}, + {"name": "my-app", "version": "0.1.0", "skip_reason": "Dependency not found on PyPI"}, + ] + md = build_markdown(bandit_clean, report, Settings()) + assert "Dependencies audited: 1, skipped: 1" in md + assert "No vulnerabilities found" in md + + +@pytest.mark.parametrize( + "report", + [[], [{"name": "my-app", "version": "0.1.0", "skip_reason": "Dependency not found on PyPI"}]], +) +def test_markdown_zero_audited_is_not_reported_clean( + bandit_clean: dict[str, Any], report: list[Any] +) -> None: + md = build_markdown(bandit_clean, report, Settings()) + assert "No vulnerabilities found" not in md + assert "No dependencies were audited" in md + + +def test_markdown_pip_audit_error(bandit_issues: dict[str, Any]) -> None: + md = build_markdown(bandit_issues, [], Settings(), pip_audit_error="uv export failed: boom") + assert "B404" in md # bandit results are still shown + assert "pip-audit did NOT run" in md + assert "uv export failed: boom" in md + assert "No vulnerabilities found" not in md + + def test_markdown_run_url( bandit_clean: dict[str, Any], pip_clean: list[Any], monkeypatch: pytest.MonkeyPatch ) -> None: From 6dc80216a753cf0b6e528a56ad9587d5afd478ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Houpert?= <10154151+lhoupert@users.noreply.github.com> Date: Tue, 6 Oct 2026 15:23:41 +0100 Subject: [PATCH 4/5] test: drop the unused _make_sarif_mock helper It had no callers, and its pip_stdout="[]" default is no longer a valid pip-audit report. The clean-audit mock now reports one audited dependency, as a real clean run does. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_main.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index d478ffe..c4919ee 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -12,18 +12,12 @@ FIXTURES = Path(__file__).parent / "fixtures" -def _clean_audit(cmd: list[str], **kwargs: object) -> MagicMock: - """uv export succeeds and pip-audit reports no vulnerabilities.""" - return MagicMock(returncode=0, stderr="", stdout='{"dependencies": [], "fixes": []}') - +CLEAN_REPORT = '{"dependencies": [{"name": "requests", "version": "2.32.0", "vulns": []}]}' -def _make_sarif_mock(sarif_content: str, pip_stdout: str = "[]") -> object: - """Return a mock_subprocess factory that feeds a SARIF file and pip-audit output.""" - def mock_subprocess(cmd: list[str], **kwargs: object) -> MagicMock: - return MagicMock(returncode=0, stderr="", stdout=pip_stdout) - - return mock_subprocess +def _clean_audit(cmd: list[str], **kwargs: object) -> MagicMock: + """uv export succeeds and pip-audit reports no vulnerabilities.""" + return MagicMock(returncode=0, stderr="", stdout=CLEAN_REPORT) def test_comment_on_never_never_calls_upsert( From 7ca39e276062e2f88e7ef63695cbe612c3239620 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Houpert?= <10154151+lhoupert@users.noreply.github.com> Date: Tue, 6 Oct 2026 15:24:19 +0100 Subject: [PATCH 5/5] fix: pin bandit fork without its nested checkout lhoupert/bandit-action e2d4893 is 18022d5 minus its "Checkout repository" step. That nested actions/checkout ran with clean: true (git clean -ffdx and git reset --hard) in the caller's workspace. It deleted lockfiles the caller generated before this action (integration cases 04, 07 and 10), and it reverted the `uv lock --upgrade` in ci.yml's self-audit before pip-audit ran. Co-Authored-By: Claude Opus 5.5 (1M context) --- action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/action.yml b/action.yml index 4a66b67..74b4e90 100644 --- a/action.yml +++ b/action.yml @@ -63,7 +63,7 @@ runs: - name: Run Bandit (static security analysis) if: contains(inputs.tools, 'bandit') continue-on-error: true - uses: lhoupert/bandit-action@18022d5292d04b21fae1bfa44597b94402ba7365 + uses: lhoupert/bandit-action@e2d48932beda5cc8c50cb45ed39f8d873ef2d365 with: targets: ${{ steps.resolve-targets.outputs.targets }}