From 07667a85d373316085bee68cb7e0e7a81d114073 Mon Sep 17 00:00:00 2001 From: Advait Patel Date: Sun, 20 Sep 2026 14:01:00 -0500 Subject: [PATCH 1/4] feat: golden-file tests, ten examples, case studies, PR comment mode The three strategy items that were still outstanding (1.5, 3.6, 3.7). Golden-file tests cover the terminal summary, the --json payload and the SARIF report. Each renders from a fixed findings set - no scanner, no network, no clock - and compares against a stored file, so an unintended change fails CI instead of reaching a user. Verified by reintroducing the fix-command ordering regression from #187: two tests fail, and pass again when it is reverted. Re-record with DOCKSEC_UPDATE_GOLDEN=1 and read the diff. Ten examples with documented expected findings, every count produced by running the scan. Eight Dockerfiles across Node, Python, Java, Go and BuildKit secret mounts, plus the existing two compose stacks, each insecure file paired with a hardened one. Two carry a lesson beyond the delta: the distroless Go image reports a HEALTHCHECK finding that is correct to keep, and the BuildKit example passes a build secret without tripping the secret rule that the ENV-based examples do. Three case studies against official images - node:18 (2,200 findings, 9 worth acting on today, one at the 100th EPSS percentile), python:3.12-slim (44 findings, none fixable) and nginx:1.31.6-alpine (clean, and what that does not prove). Official images keep the numbers reproducible and name no third party unfavourably. PR comment mode ships as two workflows. Stage one runs in the untrusted pull-request context with contents: read and no secrets, and emits an artifact. Stage two runs on workflow_run in the base repository, reads only that artifact, never checks out the PR's code, and posts the comment. Everything the renderer prints is escaped, because a fork controls the text of its own findings; the PR number is validated as an integer before it reaches an API path. Injection cases are covered by tests: pipes, HTML, Markdown links, newline-injected rows and backticks. --- .github/scripts/collect_pr_scan.sh | 52 +++++ .github/scripts/merge_pr_scan.py | 29 +++ .github/scripts/render_pr_comment.py | 139 +++++++++++ .github/workflows/pr-comment.yml | 105 +++++++++ .github/workflows/pr-scan.yml | 69 ++++++ CHANGELOG.md | 31 +++ README.md | 2 + docs/case-studies/README.md | 32 +++ docs/case-studies/nginx-alpine.md | 50 ++++ docs/case-studies/node-18.md | 71 ++++++ docs/case-studies/python-slim.md | 61 +++++ examples/README.md | 66 ++++++ .../dockerfile/Dockerfile.buildkit-secrets | 25 ++ .../dockerfile/Dockerfile.golang-multistage | 23 ++ examples/dockerfile/Dockerfile.java-hardened | 27 +++ examples/dockerfile/Dockerfile.java-insecure | 16 ++ examples/dockerfile/Dockerfile.node-hardened | 29 +++ examples/dockerfile/Dockerfile.node-insecure | 21 ++ .../dockerfile/Dockerfile.python-hardened | 31 +++ .../dockerfile/Dockerfile.python-insecure | 18 ++ tests/golden/scan.json | 100 ++++++++ tests/golden/scan.sarif | 196 ++++++++++++++++ tests/golden/summary.txt | 31 +++ tests/test_golden_output.py | 217 ++++++++++++++++++ tests/test_pr_comment.py | 137 +++++++++++ 25 files changed, 1578 insertions(+) create mode 100755 .github/scripts/collect_pr_scan.sh create mode 100644 .github/scripts/merge_pr_scan.py create mode 100644 .github/scripts/render_pr_comment.py create mode 100644 .github/workflows/pr-comment.yml create mode 100644 .github/workflows/pr-scan.yml create mode 100644 docs/case-studies/README.md create mode 100644 docs/case-studies/nginx-alpine.md create mode 100644 docs/case-studies/node-18.md create mode 100644 docs/case-studies/python-slim.md create mode 100644 examples/README.md create mode 100644 examples/dockerfile/Dockerfile.buildkit-secrets create mode 100644 examples/dockerfile/Dockerfile.golang-multistage create mode 100644 examples/dockerfile/Dockerfile.java-hardened create mode 100644 examples/dockerfile/Dockerfile.java-insecure create mode 100644 examples/dockerfile/Dockerfile.node-hardened create mode 100644 examples/dockerfile/Dockerfile.node-insecure create mode 100644 examples/dockerfile/Dockerfile.python-hardened create mode 100644 examples/dockerfile/Dockerfile.python-insecure create mode 100644 tests/golden/scan.json create mode 100644 tests/golden/scan.sarif create mode 100644 tests/golden/summary.txt create mode 100644 tests/test_golden_output.py create mode 100644 tests/test_pr_comment.py diff --git a/.github/scripts/collect_pr_scan.sh b/.github/scripts/collect_pr_scan.sh new file mode 100755 index 0000000..fcd8579 --- /dev/null +++ b/.github/scripts/collect_pr_scan.sh @@ -0,0 +1,52 @@ +#!/usr/bin/env bash +# Scan every container file a pull request touches and write one JSON document. +# +# Runs in the untrusted stage: no secrets, no write permissions, no commenting. +# Output is data only, consumed later by render_pr_comment.py in the trusted +# stage. +set -uo pipefail + +base_ref="${1:?base ref required}" +out_dir="${2:?output directory required}" + +mkdir -p "${out_dir}" + +changed="$(git diff --name-only "origin/${base_ref}...HEAD" -- \ + '*Dockerfile*' '*docker-compose*.yml' '*docker-compose*.yaml' \ + '*compose*.yml' '*compose*.yaml' 2>/dev/null || true)" + +if [ -z "${changed}" ]; then + echo "no container files changed" + printf '{"files": []}\n' > "${out_dir}/results.json" + exit 0 +fi + +echo "changed files:" +echo "${changed}" + +tmp="$(mktemp -d)" +trap 'rm -rf "${tmp}"' EXIT +index=0 + +while IFS= read -r file; do + [ -n "${file}" ] || continue + [ -f "${file}" ] || continue + + case "${file}" in + *compose*) args=(--compose "${file}") ;; + *) args=("${file}") ;; + esac + + # A scan that finds problems exits non-zero by design. That is a result, + # not a workflow failure, so the exit code is deliberately ignored here. + if docksec "${args[@]}" --scan-only --no-config --json \ + > "${tmp}/scan.json" 2>/dev/null || true; then :; fi + + if [ -s "${tmp}/scan.json" ]; then + index=$((index + 1)) + cp "${tmp}/scan.json" "${tmp}/entry-${index}.json" + printf '%s\n' "${file}" > "${tmp}/entry-${index}.path" + fi +done <<< "${changed}" + +python3 "$(dirname "$0")/merge_pr_scan.py" "${tmp}" "${out_dir}/results.json" diff --git a/.github/scripts/merge_pr_scan.py b/.github/scripts/merge_pr_scan.py new file mode 100644 index 0000000..7e8910f --- /dev/null +++ b/.github/scripts/merge_pr_scan.py @@ -0,0 +1,29 @@ +"""Merge per-file DockSec scans into the single artifact stage two reads.""" + +import json +import sys +from pathlib import Path + + +def main() -> int: + tmp = Path(sys.argv[1]) + out = Path(sys.argv[2]) + + files = [] + for path_file in sorted(tmp.glob("entry-*.path")): + scan_file = path_file.with_suffix(".json") + if not scan_file.exists(): + continue + try: + data = json.loads(scan_file.read_text()) + except ValueError: + continue + files.append({"file": path_file.read_text().strip(), "data": data}) + + out.write_text(json.dumps({"files": files}, indent=2)) + print(f"scanned {len(files)} file(s)") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.github/scripts/render_pr_comment.py b/.github/scripts/render_pr_comment.py new file mode 100644 index 0000000..43c89f4 --- /dev/null +++ b/.github/scripts/render_pr_comment.py @@ -0,0 +1,139 @@ +"""Render DockSec pull-request scan results as a Markdown comment. + +Runs in stage two of the two-stage PR workflow, in the base repository's +trusted context. Its input is the JSON artifact produced by the untrusted +scan job, so **everything it reads is attacker-controlled**: a fork can put +any string in a Dockerfile and therefore in a finding's title. + +Nothing from that input is ever interpolated raw. `_md` escapes the +characters that would otherwise let a finding title close a table cell, start +an HTML tag, or smuggle a Markdown link into the rendered comment. +""" + +import argparse +import html +import json +import re +import sys +from pathlib import Path + +MAX_ROWS = 15 +SEVERITY_ORDER = {"CRITICAL": 0, "HIGH": 1, "MEDIUM": 2, "LOW": 3, "UNKNOWN": 4} +MARKER = "" + + +def _md(value, limit: int = 160) -> str: + """Make untrusted text safe to place inside a Markdown table cell.""" + text = str(value if value is not None else "") + text = text.replace("\r", " ").replace("\n", " ") + # Collapse whitespace so a long run cannot stretch the table. + text = re.sub(r"\s+", " ", text).strip() + if len(text) > limit: + text = text[: limit - 1].rstrip() + "…" + # Escape HTML first: a raw '<' would otherwise open a tag in the comment. + text = html.escape(text, quote=False) + # Then neutralise the Markdown metacharacters that matter in a table. + for char in ("\\", "|", "`", "*", "_", "[", "]", "<", ">"): + text = text.replace(char, "\\" + char) + return text or "-" + + +def _rank(finding) -> tuple: + severity = str(finding.get("Severity", "UNKNOWN")).upper() + # fix_now first, then severity, so the comment leads with what to do. + priority = 0 if finding.get("Priority") == "fix_now" else 1 + return (priority, SEVERITY_ORDER.get(severity, 99), str(finding.get("VulnerabilityID", ""))) + + +def render(payload: dict) -> str: + files = payload.get("files") or [] + if not files: + return f"{MARKER}\n## DockSec\n\nNo container files changed in this pull request." + + total = 0 + serious = 0 + blocks = [] + + for entry in files: + path = entry.get("file", "?") + data = entry.get("data") or {} + findings = data.get("vulnerabilities") or [] + counts = data.get("severity_counts") or {} + score = (data.get("scan_info") or {}).get("analysis_score") + total += len(findings) + serious += counts.get("CRITICAL", 0) + counts.get("HIGH", 0) + + header = ( + f"
\n{_md(path)} - " + f"{counts.get('CRITICAL', 0)} critical, {counts.get('HIGH', 0)} high" + + (f", score {score}" if score is not None else "") + + "\n" + ) + + if not findings: + blocks.append(header + "\nNo findings.\n
") + continue + + rows = ["", "| Severity | ID | Finding | Fix |", "| --- | --- | --- | --- |"] + # One row per rule, not per occurrence: a Dockerfile with two secrets + # in ENV trips DS031 twice, and two identical rows read as a bug. + deduped = {} + for finding in findings: + key = (finding.get("VulnerabilityID"), finding.get("PkgName")) + existing = deduped.get(key) + if existing is None or _rank(finding) < _rank(existing): + deduped[key] = finding + ordered = sorted(deduped.values(), key=_rank) + hidden = len(findings) - len(ordered) + for finding in ordered[:MAX_ROWS]: + tier = " (Fix Now)" if finding.get("Priority") == "fix_now" else "" + fix = finding.get("FixedVersion") or finding.get("Remediation") or "-" + rows.append( + f"| {_md(finding.get('Severity'))}{tier} " + f"| {_md(finding.get('VulnerabilityID'), 40)} " + f"| {_md(finding.get('Title'))} " + f"| {_md(fix, 80)} |" + ) + remaining = max(0, len(ordered) - MAX_ROWS) + hidden + if remaining: + rows.append(f"\n_{remaining} more finding(s) not shown._") + blocks.append(header + "\n".join(rows) + "\n") + + verdict = ( + f"**{serious} finding(s) at CRITICAL or HIGH** across {len(files)} file(s)." + if serious + else f"No CRITICAL or HIGH findings across {len(files)} file(s)." + ) + + return "\n".join([ + MARKER, + "## DockSec", + "", + verdict, + f"\n{total} finding(s) in total. Ordered by exploitation likelihood (EPSS), " + "so the top rows are what to fix first.", + "", + *blocks, + "", + "Run locally: `docksec --scan-only`", + ]) + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("results", type=Path) + parser.add_argument("--out", type=Path, required=True) + args = parser.parse_args() + + try: + payload = json.loads(args.results.read_text()) + except (OSError, ValueError) as exc: + print(f"could not read scan results: {exc}", file=sys.stderr) + return 1 + + args.out.write_text(render(payload)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.github/workflows/pr-comment.yml b/.github/workflows/pr-comment.yml new file mode 100644 index 0000000..34456a5 --- /dev/null +++ b/.github/workflows/pr-comment.yml @@ -0,0 +1,105 @@ +# Stage two of two: render the scan results and post them as a PR comment. +# +# THREAT MODEL. This workflow runs on `workflow_run`, so it executes in the +# BASE repository's context with a token that can write comments - even when +# the pull request came from a fork. That makes it a privileged workflow, and +# it is written accordingly: +# +# - It never checks out the pull request's code. The only thing it reads +# from the PR is the artifact stage one produced. +# - It runs no code from the PR: no build, no install of the PR's +# dependencies, no scanning. Stage one already did all of that, without +# privileges. +# - The artifact's contents are untrusted data. `render_pr_comment.py` +# escapes every value before it reaches the comment body, because a fork +# controls the text of its own findings. +# - The PR number comes from the artifact and is validated as an integer +# before use. +# +# If you change this workflow, keep those four properties. Checking out the +# head ref here, or scanning here, would hand a fork the base repo's token. + +name: PR comment + +on: + workflow_run: + workflows: ["PR scan"] + types: [completed] + +permissions: + contents: read + +jobs: + comment: + name: Post scan results + runs-on: ubuntu-latest + if: github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion == 'success' + permissions: + contents: read + pull-requests: write + actions: read + steps: + # Only the scripts are checked out, from the base repository, never the + # pull request's revision. + - name: Checkout the base repository + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + sparse-checkout: .github/scripts + sparse-checkout-cone-mode: false + + - name: Set up Python + uses: actions/setup-python@e9d6f990972a57673cdb72ec29e19d42ba28880f # v6.0.0 + with: + python-version: '3.13' + + - name: Download the findings + uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + with: + name: docksec-pr-scan + path: pr-scan + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Read the PR number + id: pr + run: | + set -euo pipefail + raw="$(cat pr-scan/pr-number.txt 2>/dev/null || echo '')" + # The artifact is attacker-controlled, so this must be an integer + # before it is ever used as an API path segment. + if ! printf '%s' "${raw}" | grep -Eq '^[0-9]+$'; then + echo "::error::pr-number.txt is not a plain integer" + exit 1 + fi + echo "number=${raw}" >> "$GITHUB_OUTPUT" + + - name: Render the comment + run: | + set -euo pipefail + python3 .github/scripts/render_pr_comment.py \ + pr-scan/results.json --out comment.md + echo "--- rendered ---" + cat comment.md + + # Update the existing comment rather than adding one per push, so a busy + # pull request does not accumulate a wall of bot comments. + - name: Post or update the comment + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ steps.pr.outputs.number }} + REPO: ${{ github.repository }} + run: | + set -euo pipefail + existing="$(gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" \ + --jq '.[] | select(.body | startswith("")) | .id' \ + | head -n 1)" + + if [ -n "${existing}" ]; then + gh api --method PATCH "repos/${REPO}/issues/comments/${existing}" \ + -F body=@comment.md > /dev/null + echo "updated comment ${existing}" + else + gh api --method POST "repos/${REPO}/issues/${PR_NUMBER}/comments" \ + -F body=@comment.md > /dev/null + echo "posted a new comment" + fi diff --git a/.github/workflows/pr-scan.yml b/.github/workflows/pr-scan.yml new file mode 100644 index 0000000..23650f1 --- /dev/null +++ b/.github/workflows/pr-scan.yml @@ -0,0 +1,69 @@ +# Stage one of two: scan a pull request and emit data only. +# +# THREAT MODEL. This workflow runs on `pull_request`, so for a fork PR it +# executes code the author controls (their Dockerfile, their compose file, +# their build context). It therefore holds NO write permissions and NO access +# to secrets, and it does not comment. It writes the findings to an artifact +# and stops. +# +# Stage two (`pr-comment.yml`) runs on `workflow_run` in the base repository's +# context, where it does have permission to comment, and it reads only that +# artifact - never the PR's code. Rendering happens there. +# +# Splitting it this way is what makes the feature safe on forks. Do not merge +# the two workflows, and do not add `pull_request_target` here: that would run +# the PR's code with the base repo's token, which is the exact vulnerability +# this design avoids. + +name: PR scan + +on: + pull_request: + paths: + - '**/Dockerfile*' + - '**/docker-compose*.yml' + - '**/docker-compose*.yaml' + - '**/compose*.yml' + - '**/compose*.yaml' + +permissions: + contents: read + +concurrency: + group: pr-scan-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + scan: + name: Scan changed container files + runs-on: ubuntu-latest + steps: + - name: Checkout the pull request + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + + - name: Set up Python + uses: actions/setup-python@e9d6f990972a57673cdb72ec29e19d42ba28880f # v6.0.0 + with: + python-version: '3.13' + + - name: Install DockSec and Trivy + run: | + set -euo pipefail + python -m pip install --upgrade pip + pip install . + python -m docksec.setup_external_tools + + - name: Scan changed files + run: .github/scripts/collect_pr_scan.sh "${{ github.base_ref }}" pr-scan + + # The PR number has to travel with the artifact: stage two runs from a + # workflow_run event and cannot otherwise know which PR to comment on. + - name: Record the PR number + run: echo "${{ github.event.pull_request.number }}" > pr-scan/pr-number.txt + + - name: Upload the findings + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: docksec-pr-scan + path: pr-scan/ + retention-days: 3 diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fece76..d675241 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,37 @@ All notable changes to DockSec are documented in this file. ### Added (adoption) +- **Golden-file tests for the three output surfaces.** The terminal summary, + the `--json` payload and the SARIF report are now compared against stored + files, so an unintended change to any of them fails CI rather than reaching + a user. Re-record deliberately with `DOCKSEC_UPDATE_GOLDEN=1 pytest + tests/test_golden_output.py` and review the diff. This closes the gap that + let the fix-command ordering and the SARIF severity fallback change without + a test noticing. + +- **Ten examples with documented expected findings** + (`examples/README.md`). Eight Dockerfiles across Node, Python, Java, Go and + BuildKit secret mounts, plus the two compose stacks, each insecure file + paired with a hardened counterpart so the delta is the lesson. Two are + deliberately instructive: the distroless Go image reports a HEALTHCHECK + finding that is correct to keep, and the BuildKit example passes a secret to + a build without tripping the secret rule. + +- **Three case studies against official images** (`docs/case-studies/`): + `node:18` (2,200 findings, 9 worth acting on today), `python:3.12-slim` + (44 findings, none with an available fix), and `nginx:1.31.6-alpine` + (clean, and what that does not prove). Official images were chosen so the + numbers are reproducible and no third party is named unfavourably. + +- **Pull-request comment mode**, as a two-stage workflow. `pr-scan.yml` runs + in the untrusted pull-request context with `contents: read` and no secrets, + and emits data only; `pr-comment.yml` runs on `workflow_run` in the base + repository, reads just that artifact, and renders the comment. The renderer + escapes every value it prints, because a fork controls the text of its own + findings; the PR number is validated as an integer before use. The comment + is updated in place rather than reposted, and rows lead with `Fix Now`. + + - **`docksec --fix`** applies the mechanical subset of the suggested Dockerfile changes, re-scans, and reports the before/after finding counts. It adds a non-root `USER` before `CMD`, inserts a placeholder `HEALTHCHECK`, adds diff --git a/README.md b/README.md index 49e35d0..86c8191 100644 --- a/README.md +++ b/README.md @@ -725,6 +725,8 @@ always in a position to undo the change. | [Exploit chains](docs/exploit-chains.md) | Cross-service attack paths, and their limits | | [Compose rule reference](docs/rules/README.md) | All 17 rules: what each catches, and when keeping it is reasonable | | [CI integration](docs/ci/README.md) | Jenkins, GitLab, Azure Pipelines, pre-commit | +| [Examples](examples/README.md) | Ten Dockerfiles and compose stacks with their expected findings | +| [Case studies](docs/case-studies/README.md) | Real scans of official images, with the numbers | ## Roadmap diff --git a/docs/case-studies/README.md b/docs/case-studies/README.md new file mode 100644 index 0000000..1c2cf8a --- /dev/null +++ b/docs/case-studies/README.md @@ -0,0 +1,32 @@ +# Case studies + +Real scans of official Docker Hub images, recorded so the numbers can be +checked rather than taken on trust. Every command is copy-pasteable and every +figure below came from running it. + +Official images were chosen deliberately: they are maintained by people who +know what they are doing, they are what most teams actually build on, and +publishing their findings names no third party unfavourably. If DockSec finds +something here, the point is not that the maintainers were careless - it is +that a base image is a supply chain you inherit whether or not you look at it. + +**Scanned 2026-09-20** with DockSec 2026.9.21, Trivy 0.74.0. Vulnerability +counts move as advisories are published, so expect your numbers to differ; +the shape of the result is what these studies are about. + +| Study | Image | Findings | The point | +| --- | --- | --- | --- | +| [1](node-18.md) | `node:18` | 2,200 | Triage: 9 CVEs deserve attention today, not 2,200 | +| [2](python-slim.md) | `python:3.12-slim` | 44 | A well-maintained image with nothing you can fix | +| [3](nginx-alpine.md) | `nginx:1.31.6-alpine` | 0 | What a clean result looks like, and what it does not prove | + +## Why these three + +They are the three outcomes a user will actually meet, and each one needs a +different response: + +- **Overwhelming** (`node:18`) - the case for prioritisation. +- **Unactionable** (`python:3.12-slim`) - findings with no available fix, where + the honest answer is "change the base image or accept the risk". +- **Clean** (`nginx:1.31.6-alpine`) - where the interesting question becomes + what the scan did *not* check. diff --git a/docs/case-studies/nginx-alpine.md b/docs/case-studies/nginx-alpine.md new file mode 100644 index 0000000..914ad2f --- /dev/null +++ b/docs/case-studies/nginx-alpine.md @@ -0,0 +1,50 @@ +# Case study 3: `nginx:1.31.6-alpine` + +**Zero findings. Here is what that does and does not mean.** + +```bash +docksec -i nginx:1.31.6-alpine --image-only --scan-only +``` + +## Result + +| | | +| --- | --- | +| Total findings | 0 | +| CRITICAL / HIGH | 0 | +| Security score | 100 / 100 | + +Alpine's package set is small, musl replaces glibc, and the image carries +almost nothing beyond nginx itself. Fewer packages means fewer advisories - +this is the strongest practical argument for a minimal base image, and it is +worth more than any amount of post-hoc hardening. + +## What a clean result does not prove + +This is the part most tools leave out, and the reason DockSec prints coverage +notes on every run including this one: + +- **Not "no vulnerabilities".** It means no *known, published* advisory matches + a package in this image today. Tomorrow's advisory applies to the same bytes. +- **Not "your application is safe".** Nothing here scanned your code, your + dependencies, your Dockerfile or your compose file. A clean base image with + a root-running service and a mounted Docker socket is not a secure + deployment - see + [`examples/compose/docker-compose-insecure.yml`](../../examples/compose/docker-compose-insecure.yml). +- **Not "reachability was checked".** DockSec does not prove a vulnerable code + path is invoked, and does not claim to. +- **Not a statement about configuration.** Run the Dockerfile and compose + scans for that; they are where most real findings live. + +## What to actually do + +Pin it and keep it pinned: + +```dockerfile +FROM nginx:1.31.6-alpine@sha256: +``` + +A tag moves; a digest does not. Then re-scan on a schedule, because the result +above has a shelf life. Use `--baseline` so CI tells you when a new advisory +lands against an image that was clean yesterday - that transition is the +signal worth alerting on. diff --git a/docs/case-studies/node-18.md b/docs/case-studies/node-18.md new file mode 100644 index 0000000..bf1e2b1 --- /dev/null +++ b/docs/case-studies/node-18.md @@ -0,0 +1,71 @@ +# Case study 1: `node:18` + +**2,200 findings. Nine of them matter this week.** + +```bash +docksec -i node:18 --image-only --scan-only +``` + +## Result + +| | | +| --- | --- | +| Total findings | 2,200 | +| CRITICAL | 226 | +| HIGH | 1,974 | +| With a fix available | 1,712 | +| **`Fix Now`** | **9 distinct CVEs** | +| Security score | 28.6 / 100 | + +## Why this image is like this + +`node:18` is built on Debian and ships a full userland: a compiler toolchain, +`linux-libc-dev`, OpenSSL, SQLite, and the rest. None of that is a mistake - +it is what makes the image useful for building native modules. It is also +2,200 findings worth of surface area, and Node 18 reached end of life in +April 2025, so the base is no longer refreshed on the old cadence. + +A conventional scanner reports all 2,200 and stops. That output cannot be +acted on: the team either ignores it or spends a sprint on it, and both +choices are wrong. + +## What DockSec does with it + +EPSS is exploitation data - the modelled probability that a CVE is exploited +in the wild in the next 30 days - and it separates the list sharply: + +``` +CVE-2026-31431 HIGH EPSS 0.999 (100th percentile) linux-libc-dev +CVE-2026-43284 HIGH EPSS 0.932 (99.8th) linux-libc-dev +CVE-2026-43500 HIGH EPSS 0.929 (99.8th) linux-libc-dev +CVE-2025-6965 HIGH EPSS 0.725 (99.4th) libsqlite3-0 +CVE-2025-15467 HIGH EPSS 0.482 (98.8th) libssl-dev +CVE-2025-37924 HIGH EPSS 0.207 (97.4th) linux-libc-dev +``` + +`CVE-2026-31431` sits at the 100th percentile: near-certain exploitation. It +is rated HIGH, not CRITICAL, so a severity-sorted list puts it below 226 +CRITICAL findings that nobody is exploiting. **That inversion is the entire +argument for this tool.** + +Note what the ranking does *not* claim. EPSS says a CVE is being exploited +somewhere, not that it is reachable in your image. DockSec does not prove +reachability and says so in its coverage notes. + +## What to actually do + +The ranked list makes the decision obvious: + +1. **Move off `node:18`.** It is end-of-life. `node:22-alpine` is a different + base with a fraction of the surface - see + [`examples/dockerfile/Dockerfile.node-hardened`](../../examples/dockerfile/Dockerfile.node-hardened). +2. **If you cannot move yet**, `docksec --fix` and the printed `apt-get` + commands resolve 1,712 of the 2,200, including all nine `Fix Now` CVEs. +3. **Gate on the ones that matter**, not the total: + `docksec -i node:18 --image-only --fail-on high --baseline .docksec-baseline.json` + +## The honest caveat + +A scan of `node:18` measures the base image, not your application. Your own +dependencies, your configuration and your secrets are separate problems - +scan the Dockerfile and the compose file too. diff --git a/docs/case-studies/python-slim.md b/docs/case-studies/python-slim.md new file mode 100644 index 0000000..7fa7fd8 --- /dev/null +++ b/docs/case-studies/python-slim.md @@ -0,0 +1,61 @@ +# Case study 2: `python:3.12-slim` + +**44 findings. Not one of them has a fix.** + +```bash +docksec -i python:3.12-slim --image-only --scan-only +``` + +## Result + +| | | +| --- | --- | +| Total findings | 44 | +| CRITICAL | 0 | +| HIGH | 44 | +| **With a fix available** | **0** | +| `Fix Now` | 0 | +| Security score | 28.9 / 100 | + +## The interesting part + +This is a well-maintained image. There are no CRITICAL findings and nothing is +being actively exploited - every finding lands in `Fix Soon`, none in +`Fix Now`. The 44 HIGH findings concentrate in a handful of `util-linux` +packages (`libblkid1`, `libmount1`, `libuuid1`, `bsdutils` and friends) that +Debian has not yet patched in the `slim` base. + +**Zero of the 44 have a fixed version upstream.** There is no `apt-get` command +that resolves them, and `docksec --fix` correctly offers nothing. DockSec says +so directly rather than printing advice that cannot be followed: + +``` +Coverage + ! 44 finding(s) have no fixed version available upstream; no upgrade will + resolve them yet. +``` + +## What to actually do + +There are exactly three honest options, and a scanner that pretends otherwise +is lying to you: + +1. **Accept and record the risk.** Waive them with a reason and an expiry in + `.docksec-ignore.yml`, so the decision is visible and gets revisited rather + than quietly forgotten. +2. **Change the base.** `python:3.12-alpine` uses musl and a different package + set, so it does not inherit these. That is a real migration with its own + trade-offs (wheels, glibc assumptions), not a free win. +3. **Wait.** Debian will patch these. Re-scan on a schedule and let the + baseline tell you when the number moves. + +## A note on the score + +28.9 looks alarming for an image with nothing exploitable and nothing +fixable. The score is severity-weighted and deterministic; it does not +discount a finding for being unfixable, because "you cannot fix it" and "it +does not matter" are different statements. + +Read the score as *exposure*, not as a grade on your work. If that exposure is +unacceptable, option 2 is the answer - and the score is doing its job by +making that uncomfortable. diff --git a/examples/README.md b/examples/README.md new file mode 100644 index 0000000..9eb6721 --- /dev/null +++ b/examples/README.md @@ -0,0 +1,66 @@ +# Examples + +Ten examples with documented expected findings. Each insecure file has a +hardened counterpart, so the delta is the lesson rather than the absolute +number. + +Every figure below was produced by running the command shown. Rule findings +are stable; image CVE counts move as advisories are published. + +## Dockerfiles + +```bash +docksec examples/dockerfile/Dockerfile.node-insecure --scan-only +``` + +| File | Findings | Rules fired | +| --- | --- | --- | +| `Dockerfile.node-insecure` | 1 CRITICAL, 1 HIGH, 2 MEDIUM, 2 LOW | DS031, DS001, DS026, DS029, DL3008, DL3009 | +| `Dockerfile.node-hardened` | none | - | +| `Dockerfile.python-insecure` | 2 CRITICAL, 2 HIGH, 2 MEDIUM, 3 LOW | DS031, DS005, DS026, DS029, DL3008, DL3009, DL3020, DL3042 | +| `Dockerfile.python-hardened` | none | - | +| `Dockerfile.java-insecure` | 2 CRITICAL, 1 HIGH, 1 MEDIUM, 2 LOW | DS031, DS026, DS029, DL3008, DL3009 | +| `Dockerfile.java-hardened` | none | - | +| `Dockerfile.golang-multistage` | 1 LOW | DS026 | +| `Dockerfile.buildkit-secrets` | none | - | + +Two of these are worth a second look: + +**`Dockerfile.golang-multistage`** reports `DS026` (no HEALTHCHECK) and that is +correct, not a false positive: the image is distroless, so there is no shell +for a healthcheck to run. This is what the per-rule documentation means by +"when you might legitimately keep it" - orchestrator-level health probes are +the right answer here. See [`docs/rules/`](../docs/rules/). + +**`Dockerfile.buildkit-secrets`** scans clean, which is the point. It uses +`RUN --mount=type=secret`, so the credential never enters a layer or the image +history. Compare it with the `ENV`-based secrets in the insecure files, which +are reported as CRITICAL (`DS031`) - both "pass a secret to the build", only +one of them safely. + +## Compose stacks + +```bash +docksec --compose examples/compose/docker-compose-insecure.yml --scan-only +``` + +| File | Findings | Notable | +| --- | --- | --- | +| `docker-compose-insecure.yml` | 6 CRITICAL/HIGH config findings | 3 exploit chains, score 0 | +| `docker-compose-secure.yml` | 0 config findings | base-image CVEs only | + +The insecure stack is the best demonstration of what DockSec does that a +per-file scanner cannot: it reports **exploit chains** across services - +a socket mount plus a published port is one path to host compromise, not two +unrelated findings. + +The hardened stack reports zero configuration findings but still shows CVEs +from its pinned base images. That is deliberate and worth understanding: the +configuration is correct, and the remaining findings are in upstream packages +with no fix available. A scanner that showed zero here would be hiding +something. See [`docs/case-studies/python-slim.md`](../docs/case-studies/python-slim.md). + +## Config file + +[`.docksec.yml`](.docksec.yml) is an annotated repo-level config showing every +supported key. diff --git a/examples/dockerfile/Dockerfile.buildkit-secrets b/examples/dockerfile/Dockerfile.buildkit-secrets new file mode 100644 index 0000000..8fc7a6a --- /dev/null +++ b/examples/dockerfile/Dockerfile.buildkit-secrets @@ -0,0 +1,25 @@ +# syntax=docker/dockerfile:1.7 +# How to use a private registry credential without baking it into a layer. +# The secret mount exists only for that RUN; it never reaches the image or its +# history, which is what makes this different from ARG or ENV. +FROM python:3.12.7-slim@sha256:af4e85f1cac90dd3771e47292ea7c8a9830abfabbe4faa5c53f158854c2e819d + +WORKDIR /srv + +COPY requirements.txt ./ + +# Build with: docker build --secret id=pip_index,src=./pip-index.txt . +RUN --mount=type=secret,id=pip_index \ + PIP_INDEX_URL="$(cat /run/secrets/pip_index)" \ + pip install --no-cache-dir -r requirements.txt + +COPY --chown=nobody:nogroup . . + +USER nobody + +EXPOSE 8000 + +HEALTHCHECK --interval=30s --timeout=3s --retries=3 \ + CMD python -c "import urllib.request,sys; sys.exit(0 if urllib.request.urlopen('http://127.0.0.1:8000/health').status==200 else 1)" + +CMD ["python", "-m", "app"] diff --git a/examples/dockerfile/Dockerfile.golang-multistage b/examples/dockerfile/Dockerfile.golang-multistage new file mode 100644 index 0000000..3d3130d --- /dev/null +++ b/examples/dockerfile/Dockerfile.golang-multistage @@ -0,0 +1,23 @@ +# Go service on a distroless runtime: no shell, no package manager, and nothing +# for an attacker to pivot with after a compromise. The nearest thing to a +# minimal container this tool can demonstrate. +FROM golang:1.23.3-alpine@sha256:c694a4d291a13a9f9d94933395673494fc2cc9d4777b85df3a7e70b3492d3574 AS build + +WORKDIR /src + +COPY go.mod go.sum ./ +RUN go mod download + +COPY . . +# Static binary: distroless carries no libc to link against at runtime. +RUN CGO_ENABLED=0 GOOS=linux go build -ldflags="-s -w" -o /out/server ./cmd/server + +FROM gcr.io/distroless/static-debian12:nonroot@sha256:d71f4b239be2d412017b798a0a401c44c3049a3ca454838473a4c32ed076bfea + +COPY --from=build /out/server /server + +USER nonroot:nonroot + +EXPOSE 8080 + +ENTRYPOINT ["/server"] diff --git a/examples/dockerfile/Dockerfile.java-hardened b/examples/dockerfile/Dockerfile.java-hardened new file mode 100644 index 0000000..fc46b80 --- /dev/null +++ b/examples/dockerfile/Dockerfile.java-hardened @@ -0,0 +1,27 @@ +# The same service on a JRE runtime: the JDK and Maven stay in the build stage, +# so neither a compiler nor a build tool reaches production. +FROM maven:3.9.9-eclipse-temurin-21@sha256:ef5b1a4b0b42b6e7e6c41a5b4a1b7d2c06b29b4a4b78d7e46ba4d8a8a48d1e9f AS build + +WORKDIR /build +COPY pom.xml ./ +RUN mvn -B dependency:go-offline + +COPY src ./src +RUN mvn -B package -DskipTests + +FROM eclipse-temurin:21.0.5_11-jre-alpine@sha256:3d29f8bf05ccc8b1ad0d2e1c1f8a4e56e8e7ec6f49a03b3c9a4a4e1b2c3d4e5f + +WORKDIR /opt/app + +RUN addgroup -S app && adduser -S -G app app + +COPY --from=build --chown=app:app /build/target/app.jar ./app.jar + +USER app + +EXPOSE 8080 + +HEALTHCHECK --interval=30s --timeout=3s --start-period=20s --retries=3 \ + CMD wget -q -O /dev/null http://127.0.0.1:8080/actuator/health || exit 1 + +ENTRYPOINT ["java", "-XX:MaxRAMPercentage=75", "-jar", "app.jar"] diff --git a/examples/dockerfile/Dockerfile.java-insecure b/examples/dockerfile/Dockerfile.java-insecure new file mode 100644 index 0000000..ea6d0bc --- /dev/null +++ b/examples/dockerfile/Dockerfile.java-insecure @@ -0,0 +1,16 @@ +# Java service built and run in a full JDK image: a large attack surface, a +# compiler shipped to production, and the build context copied in wholesale. +FROM openjdk:17 + +WORKDIR /opt/app + +ENV KEYSTORE_PASSWORD=changeit123 +ENV SPRING_DATASOURCE_PASSWORD=admin + +RUN apt-get update && apt-get install -y maven curl + +COPY . . +RUN mvn package -DskipTests + +EXPOSE 8080 +CMD ["java", "-jar", "target/app.jar"] diff --git a/examples/dockerfile/Dockerfile.node-hardened b/examples/dockerfile/Dockerfile.node-hardened new file mode 100644 index 0000000..6b22ad4 --- /dev/null +++ b/examples/dockerfile/Dockerfile.node-hardened @@ -0,0 +1,29 @@ +# The same service, hardened. Scan both and compare. +# Multi-stage keeps build tooling out of the runtime image; the digest pin makes +# the base image reproducible; the service runs unprivileged. +FROM node:22.11.0-alpine@sha256:cb7cd40ba6483f37f791e1aace576df449fc5f75eef3d2cd84d70d7c5c25a53a AS build + +WORKDIR /app +COPY package.json package-lock.json ./ +RUN npm ci --omit=dev + +COPY . . + +FROM node:22.11.0-alpine@sha256:cb7cd40ba6483f37f791e1aace576df449fc5f75eef3d2cd84d70d7c5c25a53a + +WORKDIR /app + +# Secrets arrive at runtime, never baked into a layer. +ENV NODE_ENV=production + +COPY --from=build --chown=node:node /app/node_modules ./node_modules +COPY --from=build --chown=node:node /app ./ + +USER node + +EXPOSE 3000 + +HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ + CMD node -e "require('http').get('http://127.0.0.1:3000/health',r=>process.exit(r.statusCode===200?0:1)).on('error',()=>process.exit(1))" + +CMD ["node", "server.js"] diff --git a/examples/dockerfile/Dockerfile.node-insecure b/examples/dockerfile/Dockerfile.node-insecure new file mode 100644 index 0000000..8949865 --- /dev/null +++ b/examples/dockerfile/Dockerfile.node-insecure @@ -0,0 +1,21 @@ +# Node.js service with the mistakes that show up most often in real repositories. +# Expected: root user, unpinned base image, secret in ENV, ADD over COPY, +# unpinned apt packages, no HEALTHCHECK. +FROM node:latest + +WORKDIR /app + +ENV NPM_TOKEN=npm_abcdefghijklmnopqrstuvwxyz0123456789 +ENV DATABASE_URL=postgres://admin:hunter2@db:5432/app + +RUN apt-get update && apt-get install -y curl git + +ADD https://example.com/config.tar.gz /app/config/ + +COPY package.json package-lock.json ./ +RUN npm install + +COPY . . + +EXPOSE 3000 +CMD ["node", "server.js"] diff --git a/examples/dockerfile/Dockerfile.python-hardened b/examples/dockerfile/Dockerfile.python-hardened new file mode 100644 index 0000000..5fc1aa1 --- /dev/null +++ b/examples/dockerfile/Dockerfile.python-hardened @@ -0,0 +1,31 @@ +# The same service, hardened. Build dependencies stay in the build stage, so a +# compiler is never present in the image that faces the network. +FROM python:3.12.7-slim@sha256:af4e85f1cac90dd3771e47292ea7c8a9830abfabbe4faa5c53f158854c2e819d AS build + +WORKDIR /srv + +RUN python -m venv /opt/venv +ENV PATH="/opt/venv/bin:$PATH" + +COPY requirements.txt ./ +RUN pip install --no-cache-dir --require-hashes -r requirements.txt + +FROM python:3.12.7-slim@sha256:af4e85f1cac90dd3771e47292ea7c8a9830abfabbe4faa5c53f158854c2e819d + +WORKDIR /srv + +ENV PATH="/opt/venv/bin:$PATH" \ + PYTHONDONTWRITEBYTECODE=1 \ + PYTHONUNBUFFERED=1 + +COPY --from=build /opt/venv /opt/venv +COPY --chown=nobody:nogroup . . + +USER nobody + +EXPOSE 8000 + +HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ + CMD python -c "import urllib.request,sys; sys.exit(0 if urllib.request.urlopen('http://127.0.0.1:8000/health').status==200 else 1)" + +CMD ["gunicorn", "--bind", "0.0.0.0:8000", "app:app"] diff --git a/examples/dockerfile/Dockerfile.python-insecure b/examples/dockerfile/Dockerfile.python-insecure new file mode 100644 index 0000000..4d2a0bc --- /dev/null +++ b/examples/dockerfile/Dockerfile.python-insecure @@ -0,0 +1,18 @@ +# Python service showing the ecosystem's own failure modes: an unpinned base, +# credentials baked into a layer, packages installed as root, no healthcheck. +FROM python:3.12 + +WORKDIR /srv + +ENV AWS_SECRET_ACCESS_KEY=wJalrXUtnFEMIK7MDENGbPxRfiCYEXAMPLEKEY +ENV FLASK_SECRET=super-secret-session-key + +RUN apt-get update && apt-get install -y build-essential curl + +ADD requirements.txt /srv/requirements.txt +RUN pip install -r requirements.txt + +COPY . . + +EXPOSE 8000 +CMD ["python", "-m", "flask", "run", "--host=0.0.0.0"] diff --git a/tests/golden/scan.json b/tests/golden/scan.json new file mode 100644 index 0000000..27cd5c4 --- /dev/null +++ b/tests/golden/scan.json @@ -0,0 +1,100 @@ +{ + "priority_counts": { + "fix_now": 1, + "fix_soon": 1, + "low_priority": 0, + "monitor": 0 + }, + "scan_info": { + "analysis_score": 41.5, + "dockerfile": "Dockerfile", + "image": "postgres:15.19-alpine", + "scan_mode": "compose", + "scan_time": "2026-01-01T00:00:00", + "score_version": 2 + }, + "severity_counts": { + "CRITICAL": 1, + "HIGH": 3, + "LOW": 0, + "MEDIUM": 1, + "UNKNOWN": 0 + }, + "vulnerabilities": [ + { + "CVSS": "7.5", + "Description": "A DoS in OpenSSL.", + "EPSS": 0.48211, + "EPSSPercentile": 0.98805, + "FixedVersion": "3.5.7", + "InstalledVersion": "3.5.1", + "PkgName": "openssl", + "PrimaryURL": "https://example.invalid/CVE-2025-15467", + "Priority": "fix_now", + "Severity": "HIGH", + "Status": "fixed", + "Target": "db (postgres)", + "Title": "openssl: denial of service", + "VulnerabilityID": "CVE-2025-15467" + }, + { + "CVSS": "9.8", + "Description": "Memory corruption in perl.", + "EPSS": 0.00176, + "EPSSPercentile": 0.07405, + "FixedVersion": "5.40.2", + "InstalledVersion": "5.40.1", + "PkgName": "libperl", + "PrimaryURL": "https://example.invalid/CVE-2026-31789", + "Priority": "fix_soon", + "Severity": "CRITICAL", + "Status": "fixed", + "Target": "db (postgres)", + "Title": "perl: memory corruption", + "VulnerabilityID": "CVE-2026-31789" + }, + { + "CVSS": null, + "Description": "No fix available.", + "FixedVersion": null, + "InstalledVersion": "1.3", + "PkgName": "zlib", + "PrimaryURL": null, + "Severity": "MEDIUM", + "Status": "affected", + "Target": "db (postgres)", + "Title": "zlib: unfixed issue", + "VulnerabilityID": "CVE-2026-99999" + }, + { + "CVSS": null, + "Description": "POSTGRES_PASSWORD is set in the compose file.", + "FixedVersion": null, + "InstalledVersion": null, + "PkgName": "compose", + "PrimaryURL": null, + "Remediation": "Move the value to a Docker secret", + "Severity": "HIGH", + "Status": "affected", + "Target": "compose.yml:db:12", + "Title": "Plaintext credential in environment", + "VulnerabilityID": "compose-plaintext-secret-env" + }, + { + "CVSS": null, + "Description": "No USER instruction.", + "FixedVersion": null, + "InstalledVersion": null, + "Line": 7, + "PkgName": "dockerfile", + "PrimaryURL": null, + "Remediation": "Add a non-root USER before CMD", + "Severity": "HIGH", + "Source": "trivy", + "Status": "affected", + "Target": "Dockerfile", + "Title": "Image runs as root", + "VulnerabilityID": "DS002" + } + ] +} diff --git a/tests/golden/scan.sarif b/tests/golden/scan.sarif new file mode 100644 index 0000000..b288c0b --- /dev/null +++ b/tests/golden/scan.sarif @@ -0,0 +1,196 @@ +{ + "$schema": "https://json.schemastore.org/sarif-2.1.0.json", + "runs": [ + { + "results": [ + { + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "compose.yml" + } + } + } + ], + "message": { + "text": "openssl: denial of service (openssl@3.5.1)" + }, + "properties": { + "epss": 0.48211, + "epssPercentile": 0.98805, + "priority": "fix_now" + }, + "ruleId": "CVE-2025-15467" + }, + { + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "compose.yml" + } + } + } + ], + "message": { + "text": "perl: memory corruption (libperl@5.40.1)" + }, + "properties": { + "epss": 0.00176, + "epssPercentile": 0.07405, + "priority": "fix_soon" + }, + "ruleId": "CVE-2026-31789" + }, + { + "level": "warning", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "compose.yml" + } + } + } + ], + "message": { + "text": "zlib: unfixed issue (zlib@1.3)" + }, + "ruleId": "CVE-2026-99999" + }, + { + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "compose.yml" + }, + "region": { + "startLine": 12 + } + } + } + ], + "message": { + "text": "Plaintext credential in environment (compose)" + }, + "ruleId": "compose-plaintext-secret-env" + }, + { + "level": "error", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "compose.yml" + }, + "region": { + "startLine": 7 + } + } + } + ], + "message": { + "text": "Image runs as root (dockerfile)" + }, + "ruleId": "DS002" + } + ], + "tool": { + "driver": { + "name": "DockSec", + "rules": [ + { + "defaultConfiguration": { + "level": "error" + }, + "fullDescription": { + "text": "A DoS in OpenSSL." + }, + "helpUri": "https://example.invalid/CVE-2025-15467", + "id": "CVE-2025-15467", + "name": "CVE-2025-15467", + "properties": { + "security-severity": "7.5" + }, + "shortDescription": { + "text": "openssl: denial of service" + } + }, + { + "defaultConfiguration": { + "level": "error" + }, + "fullDescription": { + "text": "Memory corruption in perl." + }, + "helpUri": "https://example.invalid/CVE-2026-31789", + "id": "CVE-2026-31789", + "name": "CVE-2026-31789", + "properties": { + "security-severity": "9.8" + }, + "shortDescription": { + "text": "perl: memory corruption" + } + }, + { + "defaultConfiguration": { + "level": "warning" + }, + "fullDescription": { + "text": "No fix available." + }, + "id": "CVE-2026-99999", + "name": "CVE-2026-99999", + "properties": { + "security-severity": "5.0" + }, + "shortDescription": { + "text": "zlib: unfixed issue" + } + }, + { + "defaultConfiguration": { + "level": "error" + }, + "fullDescription": { + "text": "POSTGRES_PASSWORD is set in the compose file." + }, + "id": "compose-plaintext-secret-env", + "name": "compose-plaintext-secret-env", + "properties": { + "security-severity": "7.0" + }, + "shortDescription": { + "text": "Plaintext credential in environment" + } + }, + { + "defaultConfiguration": { + "level": "error" + }, + "fullDescription": { + "text": "No USER instruction." + }, + "id": "DS002", + "name": "DS002", + "properties": { + "security-severity": "7.0" + }, + "shortDescription": { + "text": "Image runs as root" + } + } + ], + "version": "0.0.0-test" + } + } + } + ], + "version": "2.1.0" +} diff --git a/tests/golden/summary.txt b/tests/golden/summary.txt new file mode 100644 index 0000000..c24dfb8 --- /dev/null +++ b/tests/golden/summary.txt @@ -0,0 +1,31 @@ + +Results + +┌──────────┬──────┬────────┬─────┐ +│ Critical │ High │ Medium │ Low │ +├──────────┼──────┼────────┼─────┤ +│ 1 │ 3 │ 1 │ 0 │ +└──────────┴──────┴────────┴─────┘ + +Security Score 41.5 / 100 POOR + +Priority + Fix Now 1 + Fix Soon 1 + +Fix commands + > upgrade openssl to 3.5.7 + Fix Now HIGH - 3.5.1 -> 3.5.7 (CVE-2025-15467) + > upgrade libperl to 5.40.2 + Fix Soon CRITICAL - 5.40.1 -> 5.40.2 (CVE-2026-31789) + +Dockerfile changes + - [HIGH] Add a non-root USER before CMD/ENTRYPOINT (line 7) + +Compose changes + - [HIGH] Move the value to a Docker secret and reference it with a _FILE variable (db) + +Applying all of the above resolves 4 of 5 finding(s); 1 has no mechanical fix yet. + +Coverage + . Findings are matched against advisory data; exploitability and runtime reachability are not proven. diff --git a/tests/test_golden_output.py b/tests/test_golden_output.py new file mode 100644 index 0000000..3bc4df2 --- /dev/null +++ b/tests/test_golden_output.py @@ -0,0 +1,217 @@ +"""Golden-file coverage for the three output surfaces (strategy item 1.5). + +The terminal summary, the `--json` payload and the SARIF report are the +product's public interface: CI gates, dashboards and humans all read one of +them. Unit tests assert individual facts about each, which is how a change can +reorder a list, drop a field or alter a label without anything failing. + +These tests render each surface from a fixed `results` dict and compare the +whole thing against a stored file. No scanner, no network, no clock: the inputs +are literals, so a diff here means the output changed, not that the world did. + +When a diff is intentional, review it and re-record: + + DOCKSEC_UPDATE_GOLDEN=1 pytest tests/test_golden_output.py + +Review the resulting diff as carefully as any source change - re-recording +without reading it defeats the point of the test. +""" + +import json +import os +import unittest +from pathlib import Path + +from docksec import output +from docksec.remediation import build_plan +from docksec.report_generator import ReportGenerator + +GOLDEN_DIR = Path(__file__).parent / "golden" + + +def _compare(name: str, actual: str) -> None: + """Compare against the stored file, or re-record when asked.""" + path = GOLDEN_DIR / name + actual = actual.rstrip("\n") + "\n" + if os.getenv("DOCKSEC_UPDATE_GOLDEN"): + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(actual) + return + if not path.exists(): + raise AssertionError( + f"missing golden file {path}. Record it with " + f"DOCKSEC_UPDATE_GOLDEN=1 pytest {__file__}" + ) + expected = path.read_text() + if expected != actual: + raise AssertionError( + f"{name} changed.\n\n--- expected\n{expected}\n--- actual\n{actual}\n" + f"If this change is intended, re-record with " + f"DOCKSEC_UPDATE_GOLDEN=1 and review the diff." + ) + + +# A findings set chosen to exercise the parts that drifted in the past: a +# fix_now CVE that must outrank a higher-severity fix_soon one, a finding with +# no EPSS score, a CVE with no CVSS (the SARIF severity fallback), a compose +# rule, and a Dockerfile rule carrying a line number. +FINDINGS = [ + { + "VulnerabilityID": "CVE-2025-15467", "Target": "db (postgres)", + "PkgName": "openssl", "InstalledVersion": "3.5.1", "FixedVersion": "3.5.7", + "Severity": "HIGH", "Title": "openssl: denial of service", + "Description": "A DoS in OpenSSL.", "Status": "fixed", "CVSS": "7.5", + "PrimaryURL": "https://example.invalid/CVE-2025-15467", + "EPSS": 0.48211, "EPSSPercentile": 0.98805, "Priority": "fix_now", + }, + { + "VulnerabilityID": "CVE-2026-31789", "Target": "db (postgres)", + "PkgName": "libperl", "InstalledVersion": "5.40.1", "FixedVersion": "5.40.2", + "Severity": "CRITICAL", "Title": "perl: memory corruption", + "Description": "Memory corruption in perl.", "Status": "fixed", "CVSS": "9.8", + "PrimaryURL": "https://example.invalid/CVE-2026-31789", + "EPSS": 0.00176, "EPSSPercentile": 0.07405, "Priority": "fix_soon", + }, + { + "VulnerabilityID": "CVE-2026-99999", "Target": "db (postgres)", + "PkgName": "zlib", "InstalledVersion": "1.3", "FixedVersion": None, + "Severity": "MEDIUM", "Title": "zlib: unfixed issue", + "Description": "No fix available.", "Status": "affected", "CVSS": None, + "PrimaryURL": None, + }, + { + "VulnerabilityID": "compose-plaintext-secret-env", "Target": "compose.yml:db:12", + "PkgName": "compose", "InstalledVersion": None, "FixedVersion": None, + "Severity": "HIGH", "Title": "Plaintext credential in environment", + "Description": "POSTGRES_PASSWORD is set in the compose file.", + "Status": "affected", "CVSS": None, "PrimaryURL": None, + "Remediation": "Move the value to a Docker secret", + }, + { + "VulnerabilityID": "DS002", "Target": "Dockerfile", "PkgName": "dockerfile", + "InstalledVersion": None, "FixedVersion": None, "Severity": "HIGH", + "Title": "Image runs as root", "Description": "No USER instruction.", + "Status": "affected", "CVSS": None, "PrimaryURL": None, + "Remediation": "Add a non-root USER before CMD", "Line": 7, "Source": "trivy", + }, +] + + +class _Scanner: + """Stands in for DockerSecurityScanner; only these attributes are read.""" + + image_name = "postgres:15.19-alpine" + analysis_score = 41.5 + + +class TestJsonPayloadGolden(unittest.TestCase): + """`--json` is what CI and dashboards parse. A renamed or dropped key is a + breaking change for them and must not pass silently.""" + + def test_json_payload(self): + from docksec import epss as epss_mod + from docksec.score_calculator import SCORE_VERSION + + payload = { + "scan_info": { + "image": _Scanner.image_name, + "dockerfile": "Dockerfile", + # Fixed, not the clock: the golden must not change per run. + "scan_time": "2026-01-01T00:00:00", + "analysis_score": _Scanner.analysis_score, + "score_version": SCORE_VERSION, + "scan_mode": "compose", + }, + "vulnerabilities": FINDINGS, + "severity_counts": output.count_by_severity(FINDINGS), + "priority_counts": epss_mod.counts_by_priority(FINDINGS), + } + _compare("scan.json", json.dumps(payload, indent=2, sort_keys=True)) + + +class TestSarifGolden(unittest.TestCase): + """SARIF is consumed by GitHub Code Scanning. Its rule and result shape, + the security-severity fallback and the EPSS properties all live here.""" + + def test_sarif_report(self): + rules = [] + results = [] + seen = set() + for finding in FINDINGS: + rule_id = finding["VulnerabilityID"] + if rule_id not in seen: + seen.add(rule_id) + rules.append(ReportGenerator._sarif_rule(rule_id, finding)) + results.append(ReportGenerator._sarif_result(rule_id, finding, "compose.yml")) + doc = { + "$schema": "https://json.schemastore.org/sarif-2.1.0.json", + "version": "2.1.0", + "runs": [{ + "tool": {"driver": { + "name": "DockSec", + # Pinned: the real report embeds the running version, which + # would make this golden fail on every release. + "version": "0.0.0-test", + "rules": rules, + }}, + "results": results, + }], + } + _compare("scan.sarif", json.dumps(doc, indent=2, sort_keys=True)) + + +class TestTerminalGolden(unittest.TestCase): + """The terminal summary is what a human reads. Ordering carries meaning + here - the fix commands are sorted by priority tier - and a regression in + that ordering is invisible to a unit test that only checks membership.""" + + def _render(self) -> str: + import io + + from docksec import epss as epss_mod + + buffer = io.StringIO() + output.configure(quiet=False, no_color=True) + console = output.get_console() + original_file = console.file + # Fixed width: the console otherwise wraps to the terminal running the + # test, so the golden would differ between a laptop and CI. + original_width = console.width + console.file = buffer + console.width = 100 + try: + counts = output.count_by_severity(FINDINGS) + output.section("Results") + output.severity_table(counts) + output.score(_Scanner.analysis_score) + output.priority_summary(epss_mod.counts_by_priority(FINDINGS)) + output.fix_plan(build_plan(FINDINGS, dockerfile_path="Dockerfile")) + output.coverage([ + "Findings are matched against advisory data; exploitability and " + "runtime reachability are not proven.", + ]) + finally: + console.file = original_file + console.width = original_width + output.configure(quiet=False, no_color=False) + return buffer.getvalue() + + def test_terminal_summary(self): + _compare("summary.txt", self._render()) + + def test_fix_commands_lead_with_the_most_urgent_tier(self): + """Guards the ordering the golden encodes, with the reason stated. + + A future reader re-recording the golden should see this fail too, + rather than silently accepting a reordering. + """ + rendered = self._render() + self.assertLess( + rendered.index("openssl"), rendered.index("libperl"), + "the fix_now openssl finding must be listed before the fix_soon " + "CRITICAL one; EPSS priority leads the ordering", + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_pr_comment.py b/tests/test_pr_comment.py new file mode 100644 index 0000000..f86d2fd --- /dev/null +++ b/tests/test_pr_comment.py @@ -0,0 +1,137 @@ +"""Tests for the pull-request comment renderer (strategy item 3.7). + +The renderer runs in the trusted half of a two-stage workflow: it has a token +that can write comments, and its input comes from a scan of a fork's code. +Every value it renders is therefore attacker-controlled, which is what these +tests are mostly about. +""" + +import importlib.util +import unittest +from pathlib import Path + +_SPEC = importlib.util.spec_from_file_location( + "render_pr_comment", + Path(__file__).parent.parent / ".github" / "scripts" / "render_pr_comment.py", +) +render_pr_comment = importlib.util.module_from_spec(_SPEC) +_SPEC.loader.exec_module(render_pr_comment) + +render = render_pr_comment.render +_md = render_pr_comment._md + + +def _payload(*findings, path="Dockerfile", score=42.0): + counts = {"CRITICAL": 0, "HIGH": 0, "MEDIUM": 0, "LOW": 0} + for f in findings: + sev = str(f.get("Severity", "")).upper() + if sev in counts: + counts[sev] += 1 + return {"files": [{ + "file": path, + "data": { + "scan_info": {"analysis_score": score}, + "vulnerabilities": list(findings), + "severity_counts": counts, + }, + }]} + + +def _finding(**kw): + base = { + "VulnerabilityID": "CVE-2025-0001", "Severity": "HIGH", + "Title": "A finding", "FixedVersion": "1.2.3", + } + base.update(kw) + return base + + +class TestUntrustedInputIsEscaped(unittest.TestCase): + """A fork controls its own Dockerfile, so it controls finding text.""" + + def test_pipe_cannot_break_out_of_a_table_cell(self): + out = render(_payload(_finding(Title="evil | extra | cells"))) + self.assertNotIn("evil | extra", out) + self.assertIn(r"evil \| extra", out) + + def test_html_is_escaped(self): + out = render(_payload(_finding(Title=""))) + self.assertNotIn("evil", out) + + def test_missing_fields_do_not_crash(self): + out = render({"files": [{"file": "Dockerfile", "data": {}}]}) + self.assertIn("DockSec", out) + + +class TestOrdering(unittest.TestCase): + """The comment must lead with what to fix first, like the CLI does.""" + + def test_fix_now_sorts_above_a_higher_severity(self): + out = render(_payload( + _finding(VulnerabilityID="CVE-CRIT", Severity="CRITICAL"), + _finding(VulnerabilityID="CVE-NOW", Severity="HIGH", Priority="fix_now"), + )) + self.assertLess(out.index("CVE-NOW"), out.index("CVE-CRIT")) + + def test_fix_now_is_labelled(self): + out = render(_payload(_finding(Priority="fix_now"))) + self.assertIn("Fix Now", out) + + +class TestStructure(unittest.TestCase): + """The marker is how stage two finds its own comment to update.""" + + def test_marker_is_first(self): + self.assertTrue(render(_payload(_finding())).startswith( + "")) + + def test_no_changed_files_reports_cleanly(self): + out = render({"files": []}) + self.assertIn("No container files changed", out) + + def test_clean_scan_says_so(self): + out = render(_payload()) + self.assertIn("No CRITICAL or HIGH findings", out) + + def test_serious_findings_are_counted(self): + out = render(_payload(_finding(Severity="CRITICAL"), _finding(Severity="HIGH"))) + self.assertIn("2 finding(s) at CRITICAL or HIGH", out) + + def test_repeated_rule_collapses_to_one_row(self): + """Two secrets in ENV trip DS031 twice; two identical rows read as a bug.""" + out = render(_payload( + _finding(VulnerabilityID="DS031", PkgName="dockerfile", Title="Secret in ENV"), + _finding(VulnerabilityID="DS031", PkgName="dockerfile", Title="Secret in ENV"), + )) + self.assertEqual(out.count("| DS031 |"), 1) + self.assertIn("1 more finding(s) not shown", out) + + def test_long_lists_are_capped_and_say_so(self): + findings = [_finding(VulnerabilityID=f"CVE-{i:04d}") for i in range(40)] + out = render(_payload(*findings)) + self.assertIn("more finding(s) not shown", out) + + +if __name__ == "__main__": + unittest.main() From 007130c8b73fb620fef1e6da46bf8c727778b306 Mon Sep 17 00:00:00 2001 From: Advait Patel Date: Sun, 20 Sep 2026 14:03:06 -0500 Subject: [PATCH 2/4] fix: budget the PR comment body against GitHub's size limit A pull request touching many compose services rendered a 156KB comment. GitHub rejects an issue comment over 65536 characters, so the API call would fail and the job would end with nothing posted - the per-file row cap bounds each table but not the total. Whole per-file blocks are now kept while they fit within a 60000 character budget, then the count of dropped files is stated. Truncating mid-table would leave unbalanced
tags, so blocks are never split. A realistic result is unaffected: the 8-file comment from this branch renders byte-identical. --- .github/scripts/render_pr_comment.py | 30 +++++++++++++++--- tests/test_pr_comment.py | 46 ++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/.github/scripts/render_pr_comment.py b/.github/scripts/render_pr_comment.py index 43c89f4..ec4d6fd 100644 --- a/.github/scripts/render_pr_comment.py +++ b/.github/scripts/render_pr_comment.py @@ -18,6 +18,11 @@ from pathlib import Path MAX_ROWS = 15 +# GitHub rejects an issue comment body over 65536 characters. A pull request +# touching many compose services can exceed that, and a rejected comment means +# the whole job fails with nothing posted - so the body is budgeted and +# truncated rather than sent hopefully. +MAX_BODY = 60000 SEVERITY_ORDER = {"CRITICAL": 0, "HIGH": 1, "MEDIUM": 2, "LOW": 3, "UNKNOWN": 4} MARKER = "" @@ -105,7 +110,7 @@ def render(payload: dict) -> str: else f"No CRITICAL or HIGH findings across {len(files)} file(s)." ) - return "\n".join([ + head = [ MARKER, "## DockSec", "", @@ -113,10 +118,25 @@ def render(payload: dict) -> str: f"\n{total} finding(s) in total. Ordered by exploitation likelihood (EPSS), " "so the top rows are what to fix first.", "", - *blocks, - "", - "Run locally: `docksec --scan-only`", - ]) + ] + foot = ["", "Run locally: `docksec --scan-only`"] + + # Keep whole per-file blocks while they fit, then say how many were + # dropped. Truncating mid-table would produce broken Markdown. + budget = MAX_BODY - len("\n".join(head + foot)) + kept = [] + for index, block in enumerate(blocks): + if budget - (len(block) + 1) < 0: + remaining = len(blocks) - index + kept.append( + f"\n_{remaining} more file(s) not shown - the full result " + f"exceeds GitHub's comment size limit._" + ) + break + budget -= len(block) + 1 + kept.append(block) + + return "\n".join(head + kept + foot) def main() -> int: diff --git a/tests/test_pr_comment.py b/tests/test_pr_comment.py index f86d2fd..5ed9e32 100644 --- a/tests/test_pr_comment.py +++ b/tests/test_pr_comment.py @@ -84,6 +84,52 @@ def test_missing_fields_do_not_crash(self): self.assertIn("DockSec", out) +class TestCommentFitsGithubsLimit(unittest.TestCase): + """GitHub rejects a comment body over 65536 characters. + + The per-file row cap does not bound the total: a pull request touching + many compose services produced a 156KB body, which the API refuses - so + the job failed and nothing was posted at all. + """ + + def _many_files(self, count=50, per_file=2000): + files = [] + for i in range(count): + findings = [ + _finding(VulnerabilityID=f"CVE-2026-{j:05d}", Title="X" * 400) + for j in range(per_file) + ] + files.append({ + "file": f"svc{i}/Dockerfile", + "data": { + "scan_info": {"analysis_score": 10}, + "vulnerabilities": findings, + "severity_counts": { + "CRITICAL": 0, "HIGH": per_file, "MEDIUM": 0, "LOW": 0, + }, + }, + }) + return {"files": files} + + def test_large_result_stays_under_the_limit(self): + out = render(self._many_files()) + self.assertLess(len(out), 65536) + + def test_truncation_is_declared(self): + out = render(self._many_files()) + self.assertIn("more file(s) not shown", out) + + def test_truncated_body_is_still_well_formed(self): + """Cutting mid-table would leave broken Markdown in the comment.""" + out = render(self._many_files()) + self.assertEqual(out.count("
"), out.count("
")) + self.assertTrue(out.rstrip().endswith("")) + + def test_a_normal_result_is_not_truncated(self): + out = render(self._many_files(count=2, per_file=3)) + self.assertNotIn("more file(s) not shown", out) + + class TestOrdering(unittest.TestCase): """The comment must lead with what to fix first, like the CLI does.""" From f46008bed15acdceaefcbd6d29fb71bace7cc6e1 Mon Sep 17 00:00:00 2001 From: Advait Patel Date: Sun, 20 Sep 2026 14:05:41 -0500 Subject: [PATCH 3/4] test: cover the comment escape helper directly CodeQL flagged py/unused-global-variable: the test module imported _md but only exercised it through rendered output. It is the single choke point every untrusted value passes through, so test it directly rather than drop the import. --- tests/test_pr_comment.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_pr_comment.py b/tests/test_pr_comment.py index 5ed9e32..5d58bf1 100644 --- a/tests/test_pr_comment.py +++ b/tests/test_pr_comment.py @@ -130,6 +130,28 @@ def test_a_normal_result_is_not_truncated(self): self.assertNotIn("more file(s) not shown", out) +class TestEscapeHelper(unittest.TestCase): + """`_md` is the single choke point every untrusted value passes through, + so it is worth testing directly rather than only through rendered output.""" + + def test_empty_becomes_a_placeholder(self): + self.assertEqual(_md(""), "-") + self.assertEqual(_md(None), "-") + + def test_whitespace_is_collapsed(self): + self.assertEqual(_md("a\n\n b\tc"), "a b c") + + def test_table_metacharacters_are_escaped(self): + for char in ("|", "`", "*", "_", "[", "]"): + self.assertIn("\\" + char, _md(f"x{char}y")) + + def test_angle_brackets_become_entities(self): + self.assertNotIn("<", _md("