From 2c7b7ad6fea2f4e2df449f6396e0723c62a3006d Mon Sep 17 00:00:00 2001 From: nelsonduarte Date: Sat, 11 Jul 2026 11:18:20 +0100 Subject: [PATCH 1/3] ci: add security-deps workflow for pip-audit on PRs Runs pip-audit against requirements.txt and flatpak/requirements-pinned.txt on every PR that touches those files, plus weekly cron as safety net. - Fails the build if any known CVE is found (blocks merge via pip-audit --strict) - Posts an informational summary comment on the PR (updates existing comment rather than piling up new ones on every push) - Includes an outdated-packages report against PyPI (non-blocking) - Uses gh CLI for PR comment (no third-party action dependency in a security workflow) Manual dispatch supported via `gh workflow run security-deps.yml`. --- .github/workflows/security-deps.yml | 213 +++++++++++++++++++++++++++ tests/test_security_deps_workflow.py | 124 ++++++++++++++++ 2 files changed, 337 insertions(+) create mode 100644 .github/workflows/security-deps.yml create mode 100644 tests/test_security_deps_workflow.py diff --git a/.github/workflows/security-deps.yml b/.github/workflows/security-deps.yml new file mode 100644 index 0000000..345c622 --- /dev/null +++ b/.github/workflows/security-deps.yml @@ -0,0 +1,213 @@ +name: Security - Python dependencies + +on: + pull_request: + paths: + - 'requirements.txt' + - 'flatpak/requirements-pinned.txt' + - '.github/workflows/security-deps.yml' + push: + branches: [main] + paths: + - 'requirements.txt' + - 'flatpak/requirements-pinned.txt' + schedule: + # Weekly safety net: catch new CVEs even when deps have not changed. + - cron: '0 6 * * 1' + workflow_dispatch: + +permissions: + contents: read + pull-requests: write + +concurrency: + group: security-deps-${{ github.ref }} + cancel-in-progress: true + +jobs: + audit: + name: pip-audit + outdated check + runs-on: ubuntu-latest + timeout-minutes: 10 + + steps: + - name: Checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - name: Set up Python 3.14 + uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0 + with: + python-version: '3.14' + + - name: Install pip-audit + run: | + python -m pip install --upgrade pip + python -m pip install pip-audit + + - name: Run pip-audit on requirements.txt (informational render) + continue-on-error: true + run: | + echo "=== pip-audit requirements.txt ===" + # Never fail the workflow here — the strict gate at the end + # is what blocks the PR. This step only produces the human + # markdown report that gets posted as a comment. + pip-audit -r requirements.txt --format markdown \ + --output audit-main.md \ + || echo "_(pip-audit reported findings — see strict gate below.)_" >> audit-main.md + if [ ! -s audit-main.md ]; then + echo "No known vulnerabilities." > audit-main.md + fi + cat audit-main.md + + - name: Run pip-audit on flatpak/requirements-pinned.txt (informational render) + continue-on-error: true + run: | + if [ -f flatpak/requirements-pinned.txt ]; then + echo "=== pip-audit flatpak/requirements-pinned.txt ===" + pip-audit -r flatpak/requirements-pinned.txt --format markdown \ + --output audit-flatpak.md \ + || echo "_(pip-audit reported findings — see strict gate below.)_" >> audit-flatpak.md + if [ ! -s audit-flatpak.md ]; then + echo "No known vulnerabilities." > audit-flatpak.md + fi + cat audit-flatpak.md + else + echo "_flatpak/requirements-pinned.txt not present._" > audit-flatpak.md + fi + + - name: Check outdated packages (informational) + continue-on-error: true + run: | + # Simple outdated check: compare each direct pin in + # requirements.txt against the latest PyPI version. + # Purely informational — never blocks the PR. + python <<'PY' + import json + import re + import urllib.request + + direct_deps = [] + try: + with open('requirements.txt', encoding='utf-8') as f: + for line in f: + s = line.strip() + if not s or s.startswith('#'): + continue + m = re.match(r'^([A-Za-z0-9][A-Za-z0-9_.\-]*)', s) + if m: + direct_deps.append((m.group(1), s)) + except FileNotFoundError: + pass + + rows = [] + for pkg, raw in direct_deps: + latest = 'unknown' + try: + req = urllib.request.Request( + f'https://pypi.org/pypi/{pkg}/json', + headers={'User-Agent': 'pdfapps-security-deps/1.0'}, + ) + with urllib.request.urlopen(req, timeout=15) as r: + data = json.loads(r.read()) + latest = data['info']['version'] + except Exception as exc: + latest = f'error ({type(exc).__name__})' + rows.append((pkg, raw, latest)) + + def parse_parts(v): + parts = [] + for chunk in re.split(r'[.\-+]', v): + if chunk.isdigit(): + parts.append(int(chunk)) + else: + # Stop at first non-numeric segment for a + # coarse-but-safe comparison. + break + return tuple(parts) + + with open('outdated.md', 'w', encoding='utf-8') as f: + f.write('| Package | Pin | Latest on PyPI | Status |\n') + f.write('|---|---|---|---|\n') + for pkg, raw, latest in rows: + m = re.search(r'(?:==|>=|~=|>)\s*([0-9][^\s,;]*)', raw) + pin_ver = m.group(1) if m else '' + status = '' + if pin_ver and not latest.startswith('error') and latest != 'unknown': + pv = parse_parts(pin_ver) + lv = parse_parts(latest) + if pv and lv: + if pv < lv: + status = 'outdated' + elif pv == lv: + status = 'up-to-date' + else: + status = 'ahead' + f.write( + '| {pkg} | `{raw}` | `{latest}` | {status} |\n'.format( + pkg=pkg, raw=raw, latest=latest, status=status + ) + ) + + with open('outdated.md', encoding='utf-8') as f: + print(f.read()) + PY + + - name: Assemble PR comment body + if: github.event_name == 'pull_request' + run: | + { + echo "## Python dependencies security check" + echo "" + echo "### pip-audit: \`requirements.txt\`" + echo "" + cat audit-main.md + echo "" + echo "### pip-audit: \`flatpak/requirements-pinned.txt\`" + echo "" + cat audit-flatpak.md + echo "" + echo "### Outdated check (informational)" + echo "" + cat outdated.md + echo "" + echo "_Auto-generated by \`security-deps\` workflow. Merge is blocked only when \`pip-audit --strict\` reports vulnerabilities._" + } > pr-comment.md + echo "--- assembled comment ---" + cat pr-comment.md + + - name: Post or update PR comment + if: github.event_name == 'pull_request' + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} + run: | + # Locate a previous comment posted by this workflow so we + # replace it in place instead of piling up new ones on every + # push. The marker is the section heading, which is unique to + # this workflow. + COMMENT_ID=$(gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" --paginate \ + --jq '.[] | select(.user.login == "github-actions[bot]") | select(.body | contains("## Python dependencies security check")) | .id' \ + | head -1) + + if [ -n "$COMMENT_ID" ]; then + echo "Updating existing comment $COMMENT_ID" + gh api "repos/${REPO}/issues/comments/${COMMENT_ID}" -X PATCH \ + -F body=@pr-comment.md + else + echo "Creating new comment on PR #${PR_NUMBER}" + gh pr comment "${PR_NUMBER}" --repo "${REPO}" --body-file pr-comment.md + fi + + - name: Strict gate - fail on any known vulnerability + run: | + # pip-audit exits non-zero when any known CVE is found, which + # blocks the PR merge via required-check policy. --strict also + # fails if pip-audit itself can't complete (missing metadata, + # skipped dep, etc.), which is the safe default for a CI gate. + echo "=== Strict gate: requirements.txt ===" + pip-audit -r requirements.txt --strict + if [ -f flatpak/requirements-pinned.txt ]; then + echo "=== Strict gate: flatpak/requirements-pinned.txt ===" + pip-audit -r flatpak/requirements-pinned.txt --strict + fi diff --git a/tests/test_security_deps_workflow.py b/tests/test_security_deps_workflow.py new file mode 100644 index 0000000..eb6ea3b --- /dev/null +++ b/tests/test_security_deps_workflow.py @@ -0,0 +1,124 @@ +"""Smoke tests for the security-deps CI workflow. + +These tests do not execute pip-audit — they only assert that the +workflow file exists and its structural contract is preserved +(triggers, targets, strict gate, PR-comment mechanism). If any of +these regress, the PR-level security check either stops running or +stops blocking merges, which is exactly the kind of silent failure +we want CI to shout about. +""" + +from pathlib import Path + +import pytest + +try: + import yaml +except ImportError: # pragma: no cover - PyYAML is a dev dependency + yaml = None + +ROOT = Path(__file__).resolve().parent.parent +WORKFLOW_PATH = ROOT / ".github" / "workflows" / "security-deps.yml" + + +def _read_workflow_text() -> str: + return WORKFLOW_PATH.read_text(encoding="utf-8") + + +def _load_workflow_yaml() -> dict: + if yaml is None: + pytest.skip("PyYAML not available") + with WORKFLOW_PATH.open(encoding="utf-8") as f: + return yaml.safe_load(f) + + +def test_security_deps_workflow_file_exists(): + assert WORKFLOW_PATH.exists(), f"{WORKFLOW_PATH} missing" + + +def test_security_deps_workflow_is_valid_yaml(): + data = _load_workflow_yaml() + assert isinstance(data, dict) + assert "jobs" in data + + +def test_security_deps_workflow_targets_expected_files(): + src = _read_workflow_text() + # The workflow must audit both dependency files. Losing either + # means half of the runtime surface goes unchecked. + assert "requirements.txt" in src + assert "flatpak/requirements-pinned.txt" in src + + +def test_security_deps_workflow_uses_pip_audit(): + src = _read_workflow_text() + assert "pip-audit" in src + + +def test_security_deps_workflow_triggers_on_pull_request(): + data = _load_workflow_yaml() + # PyYAML parses the literal ``on:`` key as the boolean True. + triggers = data.get(True, data.get("on")) + assert isinstance(triggers, dict), f"unexpected triggers shape: {triggers!r}" + assert "pull_request" in triggers + assert "push" in triggers + assert "schedule" in triggers + assert "workflow_dispatch" in triggers + + +def test_security_deps_workflow_pull_request_path_filter_covers_deps(): + data = _load_workflow_yaml() + triggers = data.get(True, data.get("on")) + pr = triggers["pull_request"] + assert isinstance(pr, dict) + paths = pr.get("paths", []) + assert "requirements.txt" in paths + assert "flatpak/requirements-pinned.txt" in paths + + +def test_security_deps_workflow_has_strict_gate(): + src = _read_workflow_text() + # The strict gate is what actually blocks the merge. If someone + # removes ``--strict`` the workflow silently degrades to + # informational-only, which is exactly the failure mode this test + # exists to prevent. + assert "--strict" in src + + +def test_security_deps_workflow_posts_pr_comment_via_gh_cli(): + src = _read_workflow_text() + # Prefer the ``gh`` CLI over a third-party action to avoid adding + # supply-chain dependencies to a security workflow. + assert "gh pr comment" in src or "gh api" in src + # Must feed the assembled markdown to the comment (not an inline + # literal that skips the audit output). + assert "pr-comment.md" in src + + +def test_security_deps_workflow_pins_actions_by_sha(): + src = _read_workflow_text() + # Both actions must be pinned by full commit SHA (the leading + # ``@`` followed by 40 hex chars) — matching the convention in + # build.yml / codeql.yml. Version-tag pins (@v4) are rejected. + import re + + uses_lines = [ + line.strip() for line in src.splitlines() if line.strip().startswith("uses:") + ] + assert uses_lines, "no ``uses:`` lines found" + sha_re = re.compile(r"uses:\s+\S+@[0-9a-f]{40}\b") + for line in uses_lines: + assert sha_re.match(line), f"action not pinned by SHA: {line}" + + +def test_security_deps_workflow_uses_python_314(): + src = _read_workflow_text() + assert "'3.14'" in src or '"3.14"' in src + + +def test_security_deps_workflow_has_write_permission_for_pr_comments(): + data = _load_workflow_yaml() + perms = data.get("permissions", {}) + assert perms.get("pull-requests") == "write", ( + "workflow cannot post its report comment without pull-requests: write" + ) From 49e927d01b89266cfa8ad2811fab9b2952af3d36 Mon Sep 17 00:00:00 2001 From: nelsonduarte Date: Sat, 11 Jul 2026 11:18:24 +0100 Subject: [PATCH 2/3] docs: mention security-deps workflow in CONTRIBUTING.md Documents the automated pip-audit gate that now runs on every PR, so contributors know why an outdated pin can block their merge and how to trigger the workflow manually. --- CONTRIBUTING.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 32c7dc6..a5a0d41 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1520,6 +1520,24 @@ When writing code that handles files, user input, or external processes, follow **Dependencies:** - Run `pip-audit` before releases to check for known CVEs - Keep minimum versions in `requirements.txt` updated when vulnerabilities are patched +- CI enforces this automatically via the `Security - Python dependencies` workflow (`.github/workflows/security-deps.yml`) — see [CI security check](#ci-security-check) below + +### CI security check + +The `security-deps` workflow runs `pip-audit` against `requirements.txt` and `flatpak/requirements-pinned.txt` on every PR that touches those files. It also runs weekly (Monday 06:00 UTC) as a safety net so newly-published CVEs are caught even when the pins have not changed. + +| Trigger | When | +|---|---| +| `pull_request` | The PR modifies `requirements.txt`, `flatpak/requirements-pinned.txt`, or the workflow itself | +| `push` to `main` | Same paths as above | +| `schedule` | Weekly, Monday 06:00 UTC | +| `workflow_dispatch` | Manual: `gh workflow run security-deps.yml` | + +**What it does:** +- Posts (or updates) a single comment on the PR with the `pip-audit` findings for both requirement files, plus an informational outdated-packages table +- Runs `pip-audit --strict` as a final gate — the PR merge is blocked if any known vulnerability is reported + +The PR comment is posted via the `gh` CLI (no third-party action), so there is no external supply-chain dependency in the security workflow itself. ### Before a Release From 44c6d854f35d3109c56420ebd65530a819887988 Mon Sep 17 00:00:00 2001 From: nelsonduarte Date: Sat, 11 Jul 2026 11:23:46 +0100 Subject: [PATCH 3/3] fix(ci): let strict gate run even if PR comment fails on fork PRs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 11 review caught that fork PRs have GITHUB_TOKEN read-only regardless of declared permissions, so `gh api PATCH` / `gh pr comment` returns 403 -> comment step fails -> strict gate is skipped by default `if: success()`. The workflow marks itself as failed, but the CVE gate never runs, giving a false signal to reviewers. Adds `continue-on-error: true` to the comment step so the strict gate runs regardless. Comment failure is acceptable on fork PRs — users see the CI check status instead. Also pins pip-audit to 2.10.1 so an upstream regression cannot silently break the security gate, and adds a regression test that verifies continue-on-error is present on the comment step. --- .github/workflows/security-deps.yml | 13 ++++++++++++- tests/test_security_deps_workflow.py | 20 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/.github/workflows/security-deps.yml b/.github/workflows/security-deps.yml index 345c622..eb71120 100644 --- a/.github/workflows/security-deps.yml +++ b/.github/workflows/security-deps.yml @@ -42,7 +42,10 @@ jobs: - name: Install pip-audit run: | python -m pip install --upgrade pip - python -m pip install pip-audit + # Pinned to a specific stable release so an upstream pip-audit + # regression cannot silently break the security gate. Bump + # deliberately when a new release is vetted. + python -m pip install pip-audit==2.10.1 - name: Run pip-audit on requirements.txt (informational render) continue-on-error: true @@ -177,6 +180,14 @@ jobs: - name: Post or update PR comment if: github.event_name == 'pull_request' + # Fork PRs receive a read-only GITHUB_TOKEN regardless of the + # `permissions:` block above, so `gh api PATCH` / `gh pr comment` + # returns 403. Without continue-on-error the step failure would + # skip the strict gate below (default `if: success()`), marking + # the workflow red without ever running the CVE check — a false + # signal to reviewers. Fork PRs still get the CI status; only + # the convenience comment is lost. + continue-on-error: true env: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ github.event.pull_request.number }} diff --git a/tests/test_security_deps_workflow.py b/tests/test_security_deps_workflow.py index eb6ea3b..6d3336b 100644 --- a/tests/test_security_deps_workflow.py +++ b/tests/test_security_deps_workflow.py @@ -122,3 +122,23 @@ def test_security_deps_workflow_has_write_permission_for_pr_comments(): assert perms.get("pull-requests") == "write", ( "workflow cannot post its report comment without pull-requests: write" ) + + +def test_pr_comment_step_has_continue_on_error(): + """Regression for review F1: without continue-on-error on the + comment step, fork PRs skip the strict gate (comment fails on the + read-only GITHUB_TOKEN -> gate step's implicit if: success() prevents + the CVE check from running, giving a false red signal to reviewers). + """ + content = _read_workflow_text() + assert "Post or update PR comment" in content + comment_idx = content.find("Post or update PR comment") + assert comment_idx > 0 + # Check that continue-on-error appears within the step definition + # (defensive text-based check — YAML parsing would be more robust + # but adds complexity for a marker-style assertion). + snippet = content[comment_idx:comment_idx + 800] + assert "continue-on-error: true" in snippet, ( + "Comment step must have continue-on-error: true so the strict " + "gate still runs when the comment fails on fork PRs." + )