Merge pull request #129 from jk-cluster/feat/trivy-config-ci

CI: report Trivy misconfiguration findings for changed charts

.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]

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 <chart-name>
+#
+# Why the manifests and not the chart directory: `trivy config <chart-dir>`
+# 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/<chart>/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/<chart>.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 <chart-name>}"
+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: ["**/<chart>.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 += ["<details><summary>Covered by a documented exception "
+              f"({sum(suppressed.values())})</summary>", "",
+              "| ID | Occurrences | Why |", "|---|---|---|"]
+    for ident, count in sorted(suppressed.items(), key=order):
+        lines.append(f"| {ident} | {count} | {statements.get(ident, '(no statement)')} |")
+    lines += ["", "</details>", ""]
+
+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

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

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
+# <chart>.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."

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/<chart>/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