diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e2ce094..82f573c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,4 +58,4 @@ jobs: with: bandit_scan_dirs: src package_manager: uv - post_pr_comment: ${{ github.event_name == 'pull_request' }} + comment_on: never #${{ github.event_name == 'pull_request' && 'always' || 'never' }} diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 8e5af1b..a1bc81a 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -223,7 +223,7 @@ jobs: bandit_scan_dirs: ${{ matrix.bandit_scan_dirs }} bandit_severity_threshold: ${{ matrix.bandit_severity_threshold }} pip_audit_block_on: ${{ matrix.pip_audit_block_on }} - post_pr_comment: 'false' + comment_on: never artifact_name: security-audit-${{ matrix.id }} # --- Record outcome so the validate job can reconstruct NEEDS_JSON --- diff --git a/README.md b/README.md index d780154..8e40d8d 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ [![OpenSSF Scorecard](https://api.scorecard.dev/projects/github.com/developmentseed/action-python-security-auditing/badge)](https://scorecard.dev/viewer/?uri=github.com/developmentseed/action-python-security-auditing) -A GitHub Action that runs **[bandit](https://bandit.readthedocs.io/)** (static code analysis) and **[pip-audit](https://pypi.org/project/pip-audit/)** (dependency vulnerability scanning) on a Python repository, then puts the results in one PR comment, the workflow step summary, and a downloadable artifact. +A GitHub Action that runs **[bandit](https://bandit.readthedocs.io/)** (static code analysis) and **[pip-audit](https://pypi.org/project/pip-audit/)** (dependency vulnerability scanning) on a Python repository, then surfaces findings as inline PR annotations, a workflow step summary, and a downloadable artifact. ## When this might be useful @@ -11,8 +11,9 @@ Running bandit and pip-audit directly — or using the official focused actions This action exists for workflows where you want **both** scanners behind **one** step and **one** place to read the outcome. It is a thin wrapper around the same tools, not a different kind of analysis. The things it adds on top of running the tools individually: - **Single step, unified report** — one action replaces two, with no need to coordinate SARIF uploads or chain step outputs between jobs. -- **One PR comment for both scanners** — created on the first run and updated in place on every subsequent push, so the PR thread stays clean. Neither official action provides this out of the box. -- **Workflow step summary** — the same report is written to the "Summary" tab of the workflow run. +- **Inline PR annotations** — bandit findings appear as inline annotations on the "Files changed" tab, pointing directly to the affected file and line. pip-audit findings appear as summary-level annotations. Annotations generate no email notifications, so they don't add to developer fatigue on active PRs. +- **Workflow step summary** — the full report is written to the "Summary" tab of the workflow run. +- **Optional PR comment** — set `comment_on: blocking` or `comment_on: always` to post a unified PR comment as well. The comment is created once and updated in place on every push, so the PR thread stays clean. Disabled by default to avoid notification noise. - **Block on fixable-only vulnerabilities** — `pip_audit_block_on: fixable` (the default) fails CI only when a patched version exists, so you can act on it immediately; unfixable CVEs are reported but don't block. The official pip-audit action does not have this mode. - **Automatic requirements export** — pass `package_manager: uv|poetry|pipenv` and the action runs the appropriate export command before invoking pip-audit. With the official pip-audit action, you must add a separate step to export first. @@ -44,7 +45,7 @@ jobs: with: inputs: requirements.txt # Note: no built-in "fixable-only" blocking mode - # Note: findings appear only in the Actions log — no PR comment + # Note: no unified report, no inline PR annotations ``` **Using this action (equivalent result, one step):** @@ -55,7 +56,6 @@ jobs: runs-on: ubuntu-latest permissions: contents: read - pull-requests: write # for the unified PR comment security-events: write steps: - uses: actions/checkout@v4 @@ -64,12 +64,35 @@ jobs: package_manager: uv # export handled automatically bandit_scan_dirs: 'src/' pip_audit_block_on: fixable # only block when a fix exists - # Posts a unified PR comment and step summary automatically + # Inline PR annotations and step summary are written automatically ``` -## What the PR comment looks like +## Feedback channels -When issues are found, the comment posted to the PR looks like this: +### Inline annotations (default — no email notifications) + +Bandit findings are emitted as inline workflow annotations that appear directly on the affected file and line in the PR "Files changed" tab: + +``` +::error file=src/app.py,line=2::[B404] Consider possible security implications associated with subprocess module. +::warning file=src/app.py,line=5::[B602] subprocess call with shell=True identified, security issue. +``` + +pip-audit findings appear as summary-level annotations (no file/line available): + +``` +::warning::pip-audit: requests@2.25.0 — GHSA-j8r2-6x86-q33q (fix: 2.31.0) +``` + +Annotation severity maps to bandit severity: HIGH → error, MEDIUM → warning, LOW → notice. Annotations are always emitted and generate no email notifications. + +### Step summary (default) + +The full report is written to the workflow run "Summary" tab on every run. + +### PR comment (opt-in via `comment_on`) + +Set `comment_on: blocking` or `comment_on: always` to also post a PR comment. When issues are found, the comment looks like this: ``` # Security Audit Report @@ -95,22 +118,7 @@ _1 vulnerability/vulnerabilities found (1 fixable) across 1 package(s)._ **Result: ❌ Blocking issues found — see details above.** ``` -When everything is clean: - -``` -## Bandit — Static Security Analysis -✅ No issues found. - -## pip-audit — Dependency Vulnerabilities -✅ No vulnerabilities found. - ---- -**Result: ✅ No blocking issues found.** -``` - -The comment is idempotent — it is created once and updated in place on every push, so the PR thread stays clean. - -Each section also includes a direct link: the Bandit section links to the repository's GitHub Code Scanning page, and the pip-audit section links to the Dependabot security alerts page. These links appear when GitHub repository context is available (i.e. when running inside a GitHub Actions workflow). +The comment is idempotent — created once and updated in place on every push, so the PR thread stays clean. Each section includes a direct link to the repository's GitHub Code Scanning page (Bandit) and Dependabot security alerts page (pip-audit). ## Quickstart @@ -136,16 +144,24 @@ This runs both bandit and pip-audit with sensible defaults: blocks the job on HI ## Required permissions -The action needs these permissions on the job: +The default configuration (annotations + step summary, no PR comment) only needs: + +```yaml +permissions: + contents: read + security-events: write # upload bandit SARIF to GitHub Code Scanning +``` + +When `comment_on` is set to `blocking` or `always`, add: ```yaml permissions: contents: read - pull-requests: write # post/update the PR comment (when post_pr_comment: true) - security-events: write # upload bandit SARIF to GitHub Code Scanning + pull-requests: write # post/update the PR comment + security-events: write ``` -If you only use `post_pr_comment: false` and don't care about Code Scanning integration, `contents: read` alone is sufficient. +If you don't need Code Scanning integration, `contents: read` alone is sufficient. ## Usage examples @@ -244,7 +260,7 @@ Block on any bandit finding at MEDIUM or above, and on all known vulnerabilities ### Gradual adoption (audit-only, never block) -Add the action first as an observer: it posts findings to the PR comment and step summary without ever failing the job. Tighten the thresholds once your team has addressed the backlog: +Add the action first as an observer: findings appear as inline annotations and in the step summary without ever failing the job. Tighten the thresholds once your team has addressed the backlog: ```yaml - uses: developmentseed/action-python-security-auditing@12efad3bddc3efd3668cf6ac6799f94837f4fb3d # v0.5.0 @@ -252,6 +268,7 @@ Add the action first as an observer: it posts findings to the PR comment and ste package_manager: uv bandit_severity_threshold: low # report everything pip_audit_block_on: none # never block + comment_on: always # optionally post findings to the PR comment too ``` ### Scheduled scan on the default branch @@ -278,12 +295,12 @@ jobs: - uses: developmentseed/action-python-security-auditing@12efad3bddc3efd3668cf6ac6799f94837f4fb3d # v0.5.0 with: package_manager: uv - post_pr_comment: false # no PR to comment on for scheduled runs + # comment_on defaults to never — no PR comment is posted for scheduled runs ``` ### Multiple workflows posting separate PR comments -If you run this action from more than one workflow on the same PR (e.g. a general security workflow and a focused API service workflow), each workflow automatically gets its own PR comment. No configuration is needed — the comment is keyed on the workflow name, so the two comments stay independent and update in place separately. +If you run this action from more than one workflow on the same PR with `comment_on: blocking` or `comment_on: always`, each workflow automatically gets its own PR comment. No extra configuration is needed — the comment is keyed on the workflow name, so the two comments stay independent and update in place separately. ## How blocking works @@ -316,15 +333,16 @@ The job fails (non-zero exit) when **either** tool finds issues above its config | `package_manager` | `requirements` | How to resolve deps for pip-audit: `uv`, `pip`, `poetry`, `pipenv`, `requirements` | | `requirements_file` | `requirements.txt` | Path to requirements file when `package_manager=requirements` | | `working_directory` | `.` | Directory to run the audit from (useful for monorepos) | -| `post_pr_comment` | `true` | Post/update a PR comment with scan results | -| `github_token` | `${{ github.token }}` | Token used for posting PR comments | +| `comment_on` | `never` | When to post a PR comment: `never`, `blocking` (only when issues block the job), or `always` | +| `github_token` | `${{ github.token }}` | Token used for posting PR comments (only needed when `comment_on` is not `never`) | | `artifact_name` | `security-audit-reports` | Name of the uploaded artifact | | `debug` | `false` | Enable verbose debug logging; also activates automatically when re-running a workflow with "Enable debug logging" | ## Outputs -- **PR comment** — created on first run, updated in place on every subsequent run. The comment is keyed on a hidden `` marker, so multiple workflows on the same PR each maintain their own separate comment. -- **Step summary** — the same report is written to the workflow run summary, visible under the "Summary" tab. +- **Annotations** — always emitted. Bandit findings appear as inline annotations on the PR "Files changed" tab (keyed to file and line). pip-audit findings appear as summary-level annotations. No email notifications are generated. +- **Step summary** — the full report is written to the workflow run summary, visible under the "Summary" tab. +- **PR comment** — opt-in via `comment_on: blocking` or `comment_on: always`. Created on first run, updated in place on every subsequent run. The comment is keyed on a hidden `` marker, so multiple workflows on the same PR each maintain their own separate comment. - **Artifact** — `pip-audit-report.json` and `results.sarif` uploaded under the name set by `artifact_name` (default: `security-audit-reports`) for download or downstream steps. The `results.sarif` file is the bandit SARIF report; it is also uploaded to GitHub Code Scanning automatically by the underlying `lhoupert/bandit-action` step, making findings visible in the repository's Security tab when the job has `security-events: write` permission. - **Exit code** — non-zero when blocking issues are found, so the job fails and branch protections can enforce it. diff --git a/action.yml b/action.yml index 5b014db..5e355e0 100644 --- a/action.yml +++ b/action.yml @@ -21,9 +21,9 @@ inputs: requirements_file: description: Path to requirements file when package_manager=requirements default: requirements.txt - post_pr_comment: - description: Post or update a PR comment with the scan results - default: 'true' + comment_on: + description: When to post a PR comment — never, blocking (only when issues block the job), or always + default: 'never' github_token: description: GitHub token used for posting PR comments default: ${{ github.token }} @@ -84,7 +84,7 @@ runs: PIP_AUDIT_BLOCK_ON: ${{ inputs.pip_audit_block_on }} PACKAGE_MANAGER: ${{ inputs.package_manager }} REQUIREMENTS_FILE: ${{ inputs.requirements_file }} - POST_PR_COMMENT: ${{ inputs.post_pr_comment }} + COMMENT_ON: ${{ inputs.comment_on }} GITHUB_TOKEN: ${{ inputs.github_token }} PR_NUMBER: ${{ github.event.pull_request.number }} INPUT_DEBUG: ${{ inputs.debug }} diff --git a/src/python_security_auditing/__main__.py b/src/python_security_auditing/__main__.py index 1910a3b..71f9c98 100644 --- a/src/python_security_auditing/__main__.py +++ b/src/python_security_auditing/__main__.py @@ -6,6 +6,7 @@ from pathlib import Path from typing import Any +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 @@ -47,11 +48,15 @@ def main() -> None: markdown = build_markdown(bandit_report, pip_audit_report, settings) write_step_summary(markdown, settings) + emit_annotations(bandit_report, pip_audit_report, settings) - if settings.post_pr_comment and settings.github_token: - upsert_pr_comment(markdown, settings) + has_blocking = check_thresholds(bandit_report, pip_audit_report, settings) - if 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 has_blocking: sys.exit(1) diff --git a/src/python_security_auditing/annotations.py b/src/python_security_auditing/annotations.py new file mode 100644 index 0000000..a6b0469 --- /dev/null +++ b/src/python_security_auditing/annotations.py @@ -0,0 +1,57 @@ +"""Emit GitHub Actions workflow annotations for security findings.""" + +from __future__ import annotations + +from typing import Any + +from .settings import Settings + +_SEVERITY_TO_LEVEL: dict[str, str] = { + "HIGH": "error", + "MEDIUM": "warning", + "LOW": "notice", +} +_SEVERITY_ORDER = {"HIGH": 0, "MEDIUM": 1, "LOW": 2} + + +def emit_annotations( + bandit_report: dict[str, Any], + pip_audit_report: list[dict[str, Any]], + settings: Settings, +) -> None: + """Print GitHub Actions workflow commands to stdout. + + These produce inline annotations on the PR 'Files changed' tab (for bandit, + which has file+line) and summary-level annotations (for pip-audit). + No email notifications are generated by annotations. + """ + if "bandit" in settings.enabled_tools: + results: list[dict[str, Any]] = bandit_report.get("results", []) + + def _sort_key(r: dict[str, Any]) -> int: + return _SEVERITY_ORDER.get(r.get("issue_severity", "LOW"), 2) + + for result in sorted(results, key=_sort_key): + sev = result.get("issue_severity", "LOW") + level = _SEVERITY_TO_LEVEL.get(sev, "notice") + 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") + ) + print(f"::{level} file={fname},line={line}::[{test_id}] {text}") + + if "pip-audit" in settings.enabled_tools: + for pkg in pip_audit_report: + if not pkg.get("vulns"): + continue + name = pkg.get("name", "") + version = pkg.get("version", "") + for vuln in pkg["vulns"]: + vid = vuln.get("id", "") + fix = ", ".join(vuln.get("fix_versions", [])) or "no fix available" + print(f"::warning::pip-audit: {name}@{version} — {vid} (fix: {fix})") diff --git a/src/python_security_auditing/pr_comment.py b/src/python_security_auditing/pr_comment.py index f5fcf1f..9eac8b3 100644 --- a/src/python_security_auditing/pr_comment.py +++ b/src/python_security_auditing/pr_comment.py @@ -59,7 +59,7 @@ def upsert_pr_comment(markdown: str, settings: Settings) -> None: Skips silently if no PR is found or if posting is disabled. """ - if not settings.post_pr_comment or not settings.github_token: + if not settings.github_token: return pr_number = resolve_pr_number(settings) diff --git a/src/python_security_auditing/settings.py b/src/python_security_auditing/settings.py index bb447f7..c5e72fb 100644 --- a/src/python_security_auditing/settings.py +++ b/src/python_security_auditing/settings.py @@ -47,7 +47,7 @@ def debug(self) -> bool: requirements_file: str = "requirements.txt" # PR comment config - post_pr_comment: bool = True + comment_on: Literal["never", "blocking", "always"] = "never" github_token: str = "" # GitHub context (standard env vars set by GitHub Actions) diff --git a/tests/test_annotations.py b/tests/test_annotations.py new file mode 100644 index 0000000..fbdce69 --- /dev/null +++ b/tests/test_annotations.py @@ -0,0 +1,128 @@ +"""Tests for annotations.py — GitHub Actions workflow command emission.""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any, cast + +import pytest +from python_security_auditing.annotations import emit_annotations +from python_security_auditing.settings import Settings + +FIXTURES = Path(__file__).parent / "fixtures" + + +def load(name: str) -> Any: + return json.loads((FIXTURES / name).read_text()) + + +@pytest.fixture() +def bandit_issues() -> dict[str, Any]: + return cast(dict[str, Any], load("bandit_issues.json")) + + +@pytest.fixture() +def bandit_clean() -> dict[str, Any]: + return cast(dict[str, Any], load("bandit_clean.json")) + + +@pytest.fixture() +def pip_fixable() -> list[Any]: + return cast(list[Any], load("pip_audit_fixable.json")) + + +@pytest.fixture() +def pip_clean() -> list[Any]: + return cast(list[Any], load("pip_audit_clean.json")) + + +def test_bandit_high_emits_error( + bandit_issues: dict[str, Any], pip_clean: list[Any], capsys: pytest.CaptureFixture[str] +) -> None: + s = Settings() + emit_annotations(bandit_issues, pip_clean, s) + out = capsys.readouterr().out + assert "::error file=src/app.py,line=2::[B404]" in out + + +def test_bandit_medium_emits_warning( + bandit_issues: dict[str, Any], pip_clean: list[Any], capsys: pytest.CaptureFixture[str] +) -> None: + s = Settings() + emit_annotations(bandit_issues, pip_clean, s) + out = capsys.readouterr().out + assert "::warning file=src/app.py,line=5::[B602]" in out + + +def test_bandit_high_before_medium( + bandit_issues: dict[str, Any], pip_clean: list[Any], capsys: pytest.CaptureFixture[str] +) -> None: + """HIGH findings must appear before MEDIUM in output.""" + s = Settings() + emit_annotations(bandit_issues, pip_clean, s) + out = capsys.readouterr().out + assert out.index("::error") < out.index("::warning") + + +def test_bandit_clean_emits_nothing( + bandit_clean: dict[str, Any], pip_clean: list[Any], capsys: pytest.CaptureFixture[str] +) -> None: + s = Settings() + emit_annotations(bandit_clean, pip_clean, s) + out = capsys.readouterr().out + assert out == "" + + +def test_pip_audit_fixable_emits_warning( + bandit_clean: dict[str, Any], pip_fixable: list[Any], capsys: pytest.CaptureFixture[str] +) -> None: + s = Settings() + emit_annotations(bandit_clean, pip_fixable, s) + out = capsys.readouterr().out + assert "::warning::pip-audit:" in out + assert "GHSA-" in out + + +def test_pip_audit_no_file_line_in_annotation( + bandit_clean: dict[str, Any], pip_fixable: list[Any], capsys: pytest.CaptureFixture[str] +) -> None: + """pip-audit annotations must not include file= or line= (no file context).""" + s = Settings() + emit_annotations(bandit_clean, pip_fixable, s) + out = capsys.readouterr().out + pip_lines = [line for line in out.splitlines() if "pip-audit" in line] + for line in pip_lines: + assert "file=" not in line + + +def test_pip_clean_emits_nothing( + bandit_clean: dict[str, Any], pip_clean: list[Any], capsys: pytest.CaptureFixture[str] +) -> None: + s = Settings() + emit_annotations(bandit_clean, pip_clean, s) + assert capsys.readouterr().out == "" + + +def test_bandit_only_tool_skips_pip( + bandit_clean: dict[str, Any], + pip_fixable: list[Any], + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + monkeypatch.setenv("TOOLS", "bandit") + s = Settings() + emit_annotations(bandit_clean, pip_fixable, s) + assert capsys.readouterr().out == "" + + +def test_pip_only_tool_skips_bandit( + bandit_issues: dict[str, Any], + pip_clean: list[Any], + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + monkeypatch.setenv("TOOLS", "pip-audit") + s = Settings() + emit_annotations(bandit_issues, pip_clean, s) + assert capsys.readouterr().out == "" diff --git a/tests/test_main.py b/tests/test_main.py index d365574..bbf6eab 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -12,6 +12,107 @@ FIXTURES = Path(__file__).parent / "fixtures" +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 test_comment_on_never_never_calls_upsert( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """comment_on=never (default) must never call upsert_pr_comment, even with a token.""" + 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("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.__main__.emit_annotations"), + patch("python_security_auditing.__main__.upsert_pr_comment") as mock_comment, + ): + with pytest.raises(SystemExit): + main() + mock_comment.assert_not_called() + + +def test_comment_on_blocking_calls_upsert_when_blocking( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """comment_on=blocking must call upsert_pr_comment when blocking issues exist.""" + 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.setenv("GITHUB_TOKEN", "tok") + 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.__main__.emit_annotations"), + patch("python_security_auditing.__main__.upsert_pr_comment") as mock_comment, + ): + with pytest.raises(SystemExit): + main() + mock_comment.assert_called_once() + + +def test_comment_on_blocking_skips_upsert_when_clean( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """comment_on=blocking must not call upsert_pr_comment when no blocking issues.""" + sarif_path = tmp_path / "results.sarif" + sarif_path.write_text((FIXTURES / "bandit_clean.sarif").read_text()) + monkeypatch.setenv("PACKAGE_MANAGER", "uv") + monkeypatch.setenv("TOOLS", "bandit,pip-audit") + monkeypatch.setenv("BANDIT_SARIF_PATH", str(sarif_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") + + 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"), + patch("python_security_auditing.__main__.upsert_pr_comment") as mock_comment, + ): + main() + mock_comment.assert_not_called() + + def test_main_succeeds_when_uv_lockfile_missing_and_no_bandit_issues( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: @@ -20,7 +121,6 @@ def test_main_succeeds_when_uv_lockfile_missing_and_no_bandit_issues( monkeypatch.setenv("PACKAGE_MANAGER", "uv") monkeypatch.setenv("TOOLS", "bandit,pip-audit") monkeypatch.setenv("BANDIT_SARIF_PATH", str(sarif_path)) - monkeypatch.setenv("POST_PR_COMMENT", "false") monkeypatch.chdir(tmp_path) uv_exc = subprocess.CalledProcessError(2, "uv", stderr="No uv.lock found") @@ -33,6 +133,7 @@ def mock_subprocess(cmd: list[str], **kwargs: object) -> MagicMock: 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) @@ -46,7 +147,6 @@ def test_main_fails_when_bandit_blocks_despite_missing_lockfile( monkeypatch.setenv("TOOLS", "bandit,pip-audit") monkeypatch.setenv("BANDIT_SARIF_PATH", str(sarif_path)) monkeypatch.setenv("BANDIT_SEVERITY_THRESHOLD", "high") - monkeypatch.setenv("POST_PR_COMMENT", "false") monkeypatch.chdir(tmp_path) uv_exc = subprocess.CalledProcessError(2, "uv", stderr="No uv.lock found") @@ -59,6 +159,7 @@ def mock_subprocess(cmd: list[str], **kwargs: object) -> MagicMock: 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"), ): with pytest.raises(SystemExit) as exc_info: main() diff --git a/tests/test_pr_comment.py b/tests/test_pr_comment.py index 5b36edb..c7120e7 100644 --- a/tests/test_pr_comment.py +++ b/tests/test_pr_comment.py @@ -32,17 +32,7 @@ def test_comment_marker_backward_compat() -> None: # --------------------------------------------------------------------------- -def test_upsert_skips_when_disabled(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setenv("POST_PR_COMMENT", "false") - monkeypatch.setenv("GITHUB_TOKEN", "tok") - s = Settings() - with patch("python_security_auditing.pr_comment.subprocess.run") as mock_run: - upsert_pr_comment("# Report", s) - mock_run.assert_not_called() - - def test_upsert_skips_when_no_token(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setenv("POST_PR_COMMENT", "true") monkeypatch.setenv("GITHUB_TOKEN", "") s = Settings() with patch("python_security_auditing.pr_comment.subprocess.run") as mock_run: @@ -56,7 +46,6 @@ def test_upsert_skips_when_no_token(monkeypatch: pytest.MonkeyPatch) -> None: def test_upsert_creates_new_comment(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setenv("POST_PR_COMMENT", "true") monkeypatch.setenv("GITHUB_TOKEN", "tok") monkeypatch.setenv("GITHUB_REPOSITORY", "org/repo") monkeypatch.setenv("PR_NUMBER", "42") @@ -81,7 +70,6 @@ def test_upsert_creates_new_comment(monkeypatch: pytest.MonkeyPatch) -> None: def test_upsert_updates_existing_comment(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setenv("POST_PR_COMMENT", "true") monkeypatch.setenv("GITHUB_TOKEN", "tok") monkeypatch.setenv("GITHUB_REPOSITORY", "org/repo") monkeypatch.setenv("PR_NUMBER", "42") @@ -107,7 +95,6 @@ def test_upsert_updates_existing_comment(monkeypatch: pytest.MonkeyPatch) -> Non def test_upsert_does_not_match_different_workflow(monkeypatch: pytest.MonkeyPatch) -> None: """A comment from workflow-b must not be reused by workflow-a.""" - monkeypatch.setenv("POST_PR_COMMENT", "true") monkeypatch.setenv("GITHUB_TOKEN", "tok") monkeypatch.setenv("GITHUB_REPOSITORY", "org/repo") monkeypatch.setenv("PR_NUMBER", "42") @@ -132,7 +119,6 @@ def test_upsert_does_not_match_different_workflow(monkeypatch: pytest.MonkeyPatc def test_upsert_raises_when_gh_not_found(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setenv("POST_PR_COMMENT", "true") monkeypatch.setenv("GITHUB_TOKEN", "tok") monkeypatch.setenv("GITHUB_REPOSITORY", "org/repo") monkeypatch.setenv("PR_NUMBER", "42") diff --git a/tests/test_settings.py b/tests/test_settings.py index 07d9089..15d7425 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -13,7 +13,7 @@ def test_defaults() -> None: assert s.pip_audit_block_on == "fixable" assert s.package_manager == "requirements" assert s.requirements_file == "requirements.txt" - assert s.post_pr_comment is True + assert s.comment_on == "never" assert s.github_token == "" @@ -75,10 +75,16 @@ def test_pr_number_empty_string(monkeypatch: pytest.MonkeyPatch) -> None: assert s.pr_number is None -def test_post_pr_comment_false(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setenv("POST_PR_COMMENT", "false") +def test_comment_on_blocking(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("COMMENT_ON", "blocking") s = Settings() - assert s.post_pr_comment is False + assert s.comment_on == "blocking" + + +def test_comment_on_always(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("COMMENT_ON", "always") + s = Settings() + assert s.comment_on == "always" def test_github_context(monkeypatch: pytest.MonkeyPatch) -> None: