Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions .github/scripts/collect_pr_scan.sh
Original file line number Diff line number Diff line change
@@ -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"
29 changes: 29 additions & 0 deletions .github/scripts/merge_pr_scan.py
Original file line number Diff line number Diff line change
@@ -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())
159 changes: 159 additions & 0 deletions .github/scripts/render_pr_comment.py
Original file line number Diff line number Diff line change
@@ -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 = "<!-- docksec-pr-comment -->"


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"<details>\n<summary><strong>{_md(path)}</strong> - "
f"{counts.get('CRITICAL', 0)} critical, {counts.get('HIGH', 0)} high"
+ (f", score {score}" if score is not None else "")
+ "</summary>\n"
)

if not findings:
blocks.append(header + "\nNo findings.\n</details>")
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</details>")

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 = ["", "<sub>Run locally: `docksec <file> --scan-only`</sub>"]

# 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())
105 changes: 105 additions & 0 deletions .github/workflows/pr-comment.yml
Original file line number Diff line number Diff line change
@@ -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("<!-- docksec-pr-comment -->")) | .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
Loading
Loading