diff --git a/.github/workflows/validate-charts.yml b/.github/workflows/validate-charts.yml index 1a86d43..e61ecda 100644 --- a/.github/workflows/validate-charts.yml +++ b/.github/workflows/validate-charts.yml @@ -9,6 +9,8 @@ jobs: outputs: charts: ${{ steps.detect.outputs.charts }} any-chart-changed: ${{ steps.detect.outputs.any-chart-changed }} + scan-charts: ${{ steps.detect.outputs.scan-charts }} + any-scan: ${{ steps.detect.outputs.any-scan }} steps: - name: "Checkout Repository" uses: actions/checkout@v4 @@ -17,8 +19,9 @@ jobs: - name: "Detect changed charts" id: detect run: | + CHANGED_FILES=$(git diff --name-only "origin/${{ github.base_ref }}...HEAD") CHARTS="[]" - CHANGED_DIRS=$(git diff --name-only "origin/${{ github.base_ref }}...HEAD" | cut -d/ -f1 | sort -u) + CHANGED_DIRS=$(echo "$CHANGED_FILES" | cut -d/ -f1 | sort -u) for DIR in $CHANGED_DIRS; do if [ -f "$DIR/Chart.yaml" ]; then CHARTS=$(echo "$CHARTS" | jq -c --arg dir "$DIR" '. + [$dir]') @@ -33,6 +36,24 @@ jobs: echo "Changed charts: $CHARTS" fi + # The Trivy scan normally follows the changed charts. But a PR that + # changes the scanner itself touches no chart at all, and would leave + # the change untested. In that case scan a fixed sample instead, one + # chart per interesting case: a documented deviation (samba), a fully + # hardened one (cyberchef), and one with two workloads + # (matrix-synapse). + SCAN_CHARTS="$CHARTS" + if [ "$CHARTS" = "[]" ] && echo "$CHANGED_FILES" | grep -qE '^(ci/(run-trivy-config\.sh|trivy/)|\.github/workflows/validate-charts\.yml)'; then + SCAN_CHARTS='["samba","cyberchef","matrix-synapse"]' + echo "Scanner changed without a chart change - scanning the sample: $SCAN_CHARTS" + fi + echo "scan-charts=$SCAN_CHARTS" >> "$GITHUB_OUTPUT" + if [ "$SCAN_CHARTS" = "[]" ]; then + echo "any-scan=false" >> "$GITHUB_OUTPUT" + else + echo "any-scan=true" >> "$GITHUB_OUTPUT" + fi + build-chart: name: "Build ${{ matrix.chart }}" needs: detect-changed-charts @@ -68,6 +89,43 @@ jobs: echo "Version bump OK: $BASE_VERSION -> $NEW_VERSION" fi + trivy-config: + name: "Trivy config ${{ matrix.chart }}" + needs: detect-changed-charts + if: needs.detect-changed-charts.outputs.any-scan == 'true' + runs-on: ubuntu-latest + # Reporting only (issue #84): the scan renders each changed chart and lists + # what `trivy config` finds, but it never fails the job - see + # TRIVY_CONFIG_MODE below. Deliberately not a gate yet: the repo has several + # documented deviations, and a check that goes red on those every time is one + # people learn to scroll past. Depends only on detect-changed-charts, so it + # runs beside build-chart and adds no wall-clock time to the pipeline. + strategy: + fail-fast: false + matrix: + chart: ${{ fromJSON(needs.detect-changed-charts.outputs.scan-charts) }} + env: + TRIVY_VERSION: v0.74.0 + # "report" always exits 0. Switch to "gate" to make a finding that no + # documented exception covers fail the job - the intended step once the + # seccomp gap (#84) is closed. + TRIVY_CONFIG_MODE: report + steps: + - name: "Checkout Repository" + uses: actions/checkout@v4 + - name: "Install Helm" + uses: azure/setup-helm@v4 + - name: "Install Trivy" + uses: aquasecurity/setup-trivy@v0.3.1 + with: + version: ${{ env.TRIVY_VERSION }} + cache: true + # Findings land in the job summary, not only in the log: exceptions with + # their reasons come from ci/trivy/trivyignore.yaml, the trusted-registry + # list from ci/trivy/data/ksv0125.yaml. + - name: "Scan rendered manifests" + run: ci/run-trivy-config.sh "${{ matrix.chart }}" + install-test: name: "Install-test ${{ matrix.chart }}" needs: [detect-changed-charts, build-chart] diff --git a/ci/run-trivy-config.sh b/ci/run-trivy-config.sh new file mode 100755 index 0000000..f12b640 --- /dev/null +++ b/ci/run-trivy-config.sh @@ -0,0 +1,204 @@ +#!/usr/bin/env bash +# Misconfiguration scan for one chart: renders the chart with Helm and runs +# `trivy config` over the *rendered* manifests. +# +# Usage: ci/run-trivy-config.sh +# +# Why the manifests and not the chart directory: `trivy config ` +# renders the chart itself, with default values only. Every chart here has +# `required()` values, so that render fails - and Trivy does not fail with it, +# it logs "WARN Skipping chart" and prints an empty, green report. A scan that +# silently reports "clean" for every chart in the repo is worse than no scan. +# Rendering with Helm puts the failure where it belongs: this script exits +# non-zero when the render breaks. +# +# Which values (see README "Install-test"): the same committed fixtures the +# install-test uses, ci//test-values.yaml. They exist, they are +# maintained, and they are proven to produce a running workload. They also do +# not touch the security posture - no test-values file overrides +# securityContext, images or capabilities (only minecraft-server sets +# resources.requests), so what Trivy sees is the chart's own hardening, not a +# CI-specific one. +# +# Charts without install-test fixtures (ci/excluded-charts.txt) are still +# scanned - rendering costs nothing, and the reason for excluding them is the +# image size, which a static scan never touches. They get their values from +# ci/trivy/values/.yaml, or none at all if the chart renders on defaults. +# +# Exceptions live in ci/trivy/trivyignore.yaml in Trivy's native ignore-file +# schema, one `statement` per entry (same rule as ci/excluded-charts.txt: no +# entry without a reason). Findings covered by it are reported separately, not +# hidden. The trusted-registry check is configured rather than excepted, via +# ci/trivy/data/ksv0125.yaml - see the comment in that file. +# +# Mode (TRIVY_CONFIG_MODE, default "report"): +# report always exits 0 - findings are reported, the PR is not blocked. +# gate exits 1 when a finding is not covered by ci/trivy/trivyignore.yaml. +# Promotion to a gate is this one variable, once the seccomp gap (#84 part 2) +# is closed and the actionable set is genuinely empty. +set -euo pipefail + +CHART="${1:?usage: $0 }" +MODE="${TRIVY_CONFIG_MODE:-report}" +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +CI_DIR="$ROOT/ci" +IGNORE_FILE="$CI_DIR/trivy/trivyignore.yaml" +DATA_DIR="$CI_DIR/trivy/data" +RELEASE="scan" + +if [ ! -f "$ROOT/$CHART/Chart.yaml" ]; then + echo "ERROR: '$CHART' is not a chart directory" >&2 + exit 1 +fi +if [ ! -f "$IGNORE_FILE" ]; then + echo "ERROR: missing $IGNORE_FILE" >&2 + exit 1 +fi + +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT +# The rendered file is named after the chart: ignore entries scope themselves to +# a single chart via `paths: ["**/.yaml"]`, which is how a chart-specific +# deviation (samba's writable root filesystem) stays an exception for that chart +# and keeps being reported for every other one. +RENDERED="$WORK/$CHART.yaml" + +# --- 1. Pick the values ------------------------------------------------------ +VALUES_ARGS=() +VALUES_LABEL="chart defaults (no values file)" +if [ -f "$CI_DIR/$CHART/test-values.yaml" ]; then + VALUES_ARGS=(-f "$CI_DIR/$CHART/test-values.yaml") + VALUES_LABEL="ci/$CHART/test-values.yaml" +elif [ -f "$CI_DIR/trivy/values/$CHART.yaml" ]; then + VALUES_ARGS=(-f "$CI_DIR/trivy/values/$CHART.yaml") + VALUES_LABEL="ci/trivy/values/$CHART.yaml" +fi + +# --- 2. Every ignore entry needs a reason ------------------------------------ +python3 - "$IGNORE_FILE" <<'PY' +import re, sys +# Deliberately not a YAML parser: runners have python3 but not pyyaml, and the +# file is a flat list. Trivy is the one that must parse it; this only enforces +# the repo rule that no exception exists without a written reason. +text = open(sys.argv[1]).read() +entries = re.findall(r'^\s*-\s+id:\s*(\S+)(.*?)(?=^\s*-\s+id:|\Z)', text, re.M | re.S) +if not entries: + print("NOTE: no exceptions declared in the ignore file") +bad = [i for i, body in entries if not re.search(r'^\s+statement:\s*\S', body, re.M)] +if bad: + print(f"ERROR: ignore entries without a statement: {', '.join(bad)}", file=sys.stderr) + print("Every exception needs a written reason (same rule as ci/excluded-charts.txt).", file=sys.stderr) + sys.exit(1) +PY + +# --- 3. Render --------------------------------------------------------------- +echo "::group::helm template $CHART ($VALUES_LABEL)" +# ${a[@]+"${a[@]}"} - an empty array under `set -u` is an error on bash 3.2 +# (the macOS default), where this script also runs locally. +if ! helm template "$RELEASE" "$ROOT/$CHART" ${VALUES_ARGS[@]+"${VALUES_ARGS[@]}"} > "$RENDERED"; then + echo "::endgroup::" + echo "ERROR: rendering '$CHART' failed - nothing was scanned." >&2 + echo "Add the values the chart needs to ci/trivy/values/$CHART.yaml." >&2 + exit 1 +fi +DOCS="$(grep -c '^kind:' "$RENDERED" || true)" +echo "Rendered $DOCS manifests." +echo "::endgroup::" + +# --- 4. Scan ----------------------------------------------------------------- +# Two passes so the split between "actionable" and "covered by an exception" is +# made by Trivy's own matching (including the path globs), not re-implemented +# here: one pass with the ignore file, one without. A suppressed finding is +# dropped from the report entirely - `trivy config` has no --show-suppressed. +echo "::group::trivy config $CHART" +TRIVY=(trivy config --quiet --disable-telemetry --config-data "$DATA_DIR") +"${TRIVY[@]}" -f json -o "$WORK/all.json" "$RENDERED" +"${TRIVY[@]}" -f json -o "$WORK/open.json" --ignorefile "$IGNORE_FILE" "$RENDERED" +"${TRIVY[@]}" --ignorefile "$IGNORE_FILE" "$RENDERED" || true +echo "::endgroup::" + +# --- 5. Report --------------------------------------------------------------- +SUMMARY="${GITHUB_STEP_SUMMARY:-/dev/null}" +export CHART VALUES_LABEL DOCS MODE +python3 - "$WORK/all.json" "$WORK/open.json" "$IGNORE_FILE" "$SUMMARY" <<'PY' +import json, os, re, sys +from collections import Counter + +all_json, open_json, ignore_file, summary_path = sys.argv[1:5] +chart = os.environ["CHART"] + +def findings(path): + doc = json.load(open(path)) + out = [] + for res in doc.get("Results", []): + for m in res.get("Misconfigurations", []): + if m.get("Status") != "FAIL": + continue + resource = (m.get("CauseMetadata") or {}).get("Resource") or "" + out.append((m["ID"], m.get("Severity", "UNKNOWN"), m.get("Title", ""), resource)) + return out + +every, still_open = findings(all_json), findings(open_json) +open_counter = Counter(f[0] for f in still_open) +suppressed = Counter(f[0] for f in every) - open_counter + +# The same ID appears several times, scoped to different charts by `paths`. +# Trivy decides what is suppressed; this only picks the matching reason to +# print, preferring an entry scoped to this chart over a repo-wide one. +statements = {} +text = open(ignore_file).read() +for ident, body in re.findall(r'^\s*-\s+id:\s*(\S+)(.*?)(?=^\s*-\s+id:|\Z)', text, re.M | re.S): + key = ident.strip('"\'').removeprefix("AVD-") + st = re.search(r'^\s+statement:\s*(.+?)\s*$', body, re.M) + reason = st.group(1).strip('"\'') if st else "" + paths = re.search(r'^\s+paths:\s*(.+?)\s*$', body, re.M) + if paths: + if f"{chart}.yaml" not in paths.group(1): + continue # scoped to a different chart + statements[key] = reason + else: + statements.setdefault(key, reason) + +meta = {f[0]: (f[1], f[2]) for f in every} +RANK = {"CRITICAL": 0, "HIGH": 1, "MEDIUM": 2, "LOW": 3, "UNKNOWN": 4} +def order(item): + return (RANK.get(meta[item[0]][0], 9), item[0]) + +lines = [f"### `{chart}`", ""] +lines.append( + f"{len(still_open)} finding(s) to act on, {sum(suppressed.values())} covered by a " + f"documented exception - from {os.environ['DOCS']} manifests rendered with " + f"`{os.environ['VALUES_LABEL']}`." +) +lines.append("") + +if still_open: + lines += ["| ID | Severity | Occurrences | Check |", "|---|---|---|---|"] + for ident, count in sorted(open_counter.items(), key=order): + sev, title = meta[ident] + lines.append(f"| [{ident}](https://avd.aquasec.com/misconfig/{ident.lower()}) " + f"| {sev} | {count} | {title} |") +else: + lines.append("No findings outside the documented exceptions.") +lines.append("") + +if suppressed: + lines += ["
Covered by a documented exception " + f"({sum(suppressed.values())})", "", + "| ID | Occurrences | Why |", "|---|---|---|"] + for ident, count in sorted(suppressed.items(), key=order): + lines.append(f"| {ident} | {count} | {statements.get(ident, '(no statement)')} |") + lines += ["", "
", ""] + +with open(summary_path, "a") as fh: + fh.write("\n".join(lines) + "\n") + +# Annotations, capped: GitHub shows only the first handful, and a wall of them +# trains people to scroll past. +for ident, count in sorted(open_counter.items(), key=order)[:5]: + sev, title = meta[ident] + print(f"::warning title=trivy {chart}::{ident} ({sev}, x{count}): {title}") + +print(f"{chart}: {len(still_open)} to act on, {sum(suppressed.values())} excepted") +sys.exit(1 if (still_open and os.environ["MODE"] == "gate") else 0) +PY diff --git a/ci/trivy/data/ksv0125.yaml b/ci/trivy/data/ksv0125.yaml new file mode 100644 index 0000000..11e4a07 --- /dev/null +++ b/ci/trivy/data/ksv0125.yaml @@ -0,0 +1,22 @@ +# Trusted registries for AVD-KSV-0125 ("Restrict container images to trusted +# registries"), loaded via `trivy config --config-data ci/trivy/data`. +# +# Deliberately configured instead of ignored. An ignored check also stays quiet +# when an image from a genuinely foreign source shows up; a configured list only +# removes the noise and leaves the check able to say something. +# +# How the check reads this (builtin.kubernetes.KSV0125): it takes the part of +# the image reference before the first "/" and requires it to end with one of +# these entries. Setting the list *replaces* Trivy's built-in default (ACR, ECR +# and GCR domains) - none of which this fleet uses, so nothing is lost. +# +# Note what this does NOT silence: an unqualified Docker Hub image such as +# "apache/tika:3.2.3" puts "apache" in the registry position, which matches +# nothing here and is still reported. That is a real finding, not noise - the +# manifest does not say where the image comes from. It goes away by writing +# such images as "docker.io/apache/tika", which is why docker.io is listed. +ksv0125: + trusted_registries: + - cr.jk-dev.de + - ghcr.io + - docker.io diff --git a/ci/trivy/trivyignore.yaml b/ci/trivy/trivyignore.yaml new file mode 100644 index 0000000..4d61c72 --- /dev/null +++ b/ci/trivy/trivyignore.yaml @@ -0,0 +1,110 @@ +# Documented exceptions for `trivy config` (ci/run-trivy-config.sh). +# +# Trivy's native ignore-file schema, so the same file drives the future gate +# (`--ignorefile` + a non-zero exit) without being rewritten. +# +# House rule, same as ci/excluded-charts.txt: no entry without a `statement`. +# An entry without a reason reads as negligence a year from now, an entry with +# one reads as a decision. ci/run-trivy-config.sh refuses to run if a statement +# is missing, and it prints every suppressed finding together with its reason +# instead of hiding it. +# +# `paths` scopes an entry to a single chart: the scan renders each chart to +# .yaml, so "**/samba.yaml" means "samba only" and every other chart +# keeps being reported for that check. +# +# Not in here on purpose: AVD-KSV-0125 (untrusted registry). Suppressing it +# would also stay silent when an image from a genuinely foreign source appears. +# Instead the trusted-registry list is configured in ci/trivy/data/ksv0125.yaml +# and the check keeps its meaning. + +misconfigurations: + # --- repo-wide conventions ------------------------------------------------ + - id: AVD-KSV-0011 + statement: "No CPU limits, on purpose: CPU is compressible, so a limit throttles where the missing limit merely shares - see README 'Resources (auto-limits)'. The chart helper renders a CPU limit only when one is set explicitly." + + - id: AVD-KSV-0020 + statement: "UIDs are pinned to the value the image expects, not raised above 10000: minecraft-server 1000 (gosu default), teamspeak-server 9987, voucher-vault 33, the matrix-synapse delegation nginx 101. calibre-web-automated shows the risk - a runtime uid moving from 1000 to 911 makes existing volume data inaccessible, and every chart with a PVC carries that same risk class." + + - id: AVD-KSV-0021 + statement: "GIDs follow the pinned UIDs above; the same image expectation and the same PVC ownership risk apply." + + - id: AVD-KSV-0110 + statement: "Artifact of scanning rendered manifests: no chart pins a namespace (the release does), so Trivy reads every workload as living in 'default'. Nothing to fix in the charts." + + # --- samba: deviations forced by the ghcr.io/dockur/samba entrypoint ------ + # All four are justified in samba/values.yaml above its securityContext. + - id: AVD-KSV-0012 + paths: ["**/samba.yaml"] + statement: "samba starts as root: the entrypoint creates the Unix users from users.conf and smbd impersonates the authenticated share user per session, both of which require uid 0." + + - id: AVD-KSV-0014 + paths: ["**/samba.yaml"] + statement: "samba writes to its root filesystem at startup (/etc/passwd, /etc/group, the /etc/samba.conf healthcheck symlink, the TDB databases under /var/lib/samba), so readOnlyRootFilesystem cannot be true." + + - id: AVD-KSV-0022 + paths: ["**/samba.yaml"] + statement: "samba drops ALL and adds back only what a multi-user file server needs: NET_BIND_SERVICE (445/139), SETUID/SETGID (per-session impersonation), CHOWN/FOWNER/DAC_OVERRIDE (share files owned by the users)." + + - id: AVD-KSV-0106 + paths: ["**/samba.yaml"] + statement: "Same added capability set as AVD-KSV-0022; the check wants NET_BIND_SERVICE alone, which a file server that impersonates users cannot do." + + - id: AVD-KSV-0117 + paths: ["**/samba.yaml"] + statement: "SMB is served on its registered ports 445 and 139, which are privileged by definition - that is why NET_BIND_SERVICE is added." + + # --- calibre-web-automated: deviations forced by the s6-overlay image ----- + # All three are justified in calibre-web-automated/values.yaml. + - id: AVD-KSV-0012 + paths: ["**/calibre-web-automated.yaml"] + statement: "The CWA image boots via s6-overlay, whose stage-1/2 setup runs as uid 0 before dropping the application to PUID/PGID 1000; the supervisors stay root." + + - id: AVD-KSV-0014 + paths: ["**/calibre-web-automated.yaml"] + statement: "readOnlyRootFilesystem must stay false: in read-only mode the image disables its PUID/PGID remapping, the runtime uid silently moves from 1000 to the built-in 911 and existing volume data becomes inaccessible." + + - id: AVD-KSV-0022 + paths: ["**/calibre-web-automated.yaml"] + statement: "ALL dropped except the minimal s6 set verified against v4.0.6: CHOWN/DAC_OVERRIDE/FOWNER/SETGID/SETUID for the ownership fix-up and the root->PUID drop, KILL so the root supervisor can signal the uid-1000 services." + + - id: AVD-KSV-0106 + paths: ["**/calibre-web-automated.yaml"] + statement: "Same added capability set as AVD-KSV-0022; restricting it to NET_BIND_SERVICE would break the s6 init." + + # --- paperless-ngx: the root init container ------------------------------ + # Justified in paperless-ngx/values.yaml above its securityContext. The pod + # itself runs rootless as uid/gid 1000; only the init container is root. + - id: AVD-KSV-0105 + paths: ["**/paperless-ngx.yaml"] + statement: "Only the init-run-ownership init container runs as uid 0: s6-overlay requires /run to be owned by the runtime uid (group-write is not enough), so it chowns the /run emptyDir before the hardened main container starts. The main container runs as 1000 with runAsNonRoot." + + - id: AVD-KSV-0022 + paths: ["**/paperless-ngx.yaml"] + statement: "The same init container drops ALL and adds back only CHOWN, which is the one capability its single chown call needs; it also keeps a read-only root filesystem and no privilege escalation." + + - id: AVD-KSV-0106 + paths: ["**/paperless-ngx.yaml"] + statement: "Same CHOWN-only capability set as AVD-KSV-0022; the check wants NET_BIND_SERVICE alone, which an init container that fixes file ownership cannot use." + + # --- homeassistant: deviations forced by the upstream image --------------- + # All justified in homeassistant/values.yaml above its securityContext. + - id: AVD-KSV-0012 + paths: ["**/homeassistant.yaml"] + statement: "The official Home Assistant container boots via s6-overlay as uid 0 and supports no non-root user, so runAsNonRoot/runAsUser are deliberately unset." + + - id: AVD-KSV-0118 + paths: ["**/homeassistant.yaml"] + statement: "The empty pod securityContext is the deliberate form of the same deviation: the image must start as root. The container context is hardened (no privilege escalation, read-only root filesystem, ALL dropped)." + + - id: AVD-KSV-0022 + paths: ["**/homeassistant.yaml"] + statement: "ALL dropped, NET_RAW added back because Home Assistant's device discovery (DHCP watcher, ping/arp integrations) uses raw sockets and logs 'Operation not permitted' without it." + + - id: AVD-KSV-0106 + paths: ["**/homeassistant.yaml"] + statement: "Same NET_RAW addition as AVD-KSV-0022." + + - id: AVD-KSV-0119 + paths: ["**/homeassistant.yaml"] + statement: "NET_RAW is the capability in question and is added on purpose for device discovery; see AVD-KSV-0022." diff --git a/ci/trivy/values/template-chart.yaml b/ci/trivy/values/template-chart.yaml new file mode 100644 index 0000000..a17bb32 --- /dev/null +++ b/ci/trivy/values/template-chart.yaml @@ -0,0 +1,15 @@ +# Render values for the misconfiguration scan only (ci/run-trivy-config.sh). +# +# Used for charts that have no ci//test-values.yaml because they are +# excluded from the install-test. template-chart is excluded there for its +# placeholder image, which a static scan never pulls - so it is still scanned: +# it is the scaffold every new chart is copied from, and a finding in it is +# inherited by every chart that follows. +# +# Only the values the chart declares `required` - nothing that would change the +# security posture the scan is meant to look at. +ingress: + domain: scan.example.com + clusterIssuer: letsencrypt +secrets: + xxxSecretRef: op://Vault/scan/placeholder