diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index f57a802..cdab3d1 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -69,10 +69,29 @@ on: repositories whose lint runs gosec set "" to avoid duplicate findings. type: string default: p/default + semgrep-exclude-rules: + description: >- + Rule ids semgrep skips (space-separated), for a rule that cannot + apply to the repository at all (e.g. a JavaScript rule in a + repository without JavaScript); the caller gives the reason next to + it. A single false positive gets `nosemgrep` instead. + type: string + default: "" config-scan: description: trivy misconfiguration scan (Dockerfile, Kubernetes, Helm). type: boolean default: true + config-setup-recipe: + description: >- + Recipe run in working-directory before the configuration scan, e.g. + one that fetches Helm chart dependencies: trivy skips a chart whose + dependencies are missing from charts/, and the scan then fails. + type: string + default: "" + config-tools: + description: Tools from tools.lock that config-setup-recipe needs (space-separated), e.g. helm. + type: string + default: "" licenses: type: boolean default: true @@ -306,9 +325,11 @@ jobs: env: SCAN_PATH: ${{ inputs.scan-path }} RULESETS: ${{ inputs.semgrep-rulesets }} + EXCLUDE_RULES: ${{ inputs.semgrep-exclude-rules }} run: | configs=(--config "$HARNESS/semgrep/ironflock.yml") for ruleset in $RULESETS; do configs+=(--config "$ruleset"); done + for rule in $EXCLUDE_RULES; do configs+=(--exclude-rule "$rule"); done semgrep scan --metrics=off --disable-version-check "${configs[@]}" \ --sarif --output "$RUNNER_TEMP/findings/semgrep.sarif" "$SCAN_PATH" - uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 @@ -414,15 +435,25 @@ jobs: mv .harness "$RUNNER_TEMP/harness" echo "HARNESS=$RUNNER_TEMP/harness" >> "$GITHUB_ENV" mkdir -p "$RUNNER_TEMP/findings" + - name: install just and the setup tools (tools.lock) + if: inputs.config-setup-recipe != '' + env: + TOOLS: ${{ inputs.config-tools }} + run: | + # shellcheck disable=SC2086 # a space-separated list of tool names + "$HARNESS/scripts/install-tool.sh" just $TOOLS + - name: prepare the scan (config-setup-recipe) + if: inputs.config-setup-recipe != '' + working-directory: ${{ inputs.working-directory }} + env: + RECIPE: ${{ inputs.config-setup-recipe }} + run: just "$RECIPE" - run: '"$HARNESS/scripts/install-tool.sh" trivy' - name: trivy config (Dockerfile, Kubernetes, Helm) env: SCAN_PATH: ${{ inputs.scan-path }} TRIVYIGNORE: ${{ inputs.trivyignore }} - run: | - args=(--quiet --format sarif --output "$RUNNER_TEMP/findings/trivy-config.sarif") - if [ -f "$TRIVYIGNORE" ]; then args+=(--ignorefile "$TRIVYIGNORE"); fi - trivy config "${args[@]}" "$SCAN_PATH" + run: '"$HARNESS/scripts/trivy-config.sh" "$SCAN_PATH" "$RUNNER_TEMP/findings/trivy-config.sarif"' - uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 with: name: ${{ inputs.artifact-prefix }}-findings-config diff --git a/.github/workflows/selftest.yml b/.github/workflows/selftest.yml index d47f771..03450a3 100644 --- a/.github/workflows/selftest.yml +++ b/.github/workflows/selftest.yml @@ -7,7 +7,8 @@ # negative controls planted findings (fake key, reachable vulnerable call, # secret logged to the console, root container, unpinned # action, failing Go, Node and Python tests, a second or missing -# lockfile) are each reported +# lockfile, a privileged container in a Helm chart) are each +# reported; a Helm chart trivy skips fails the scan # Negative controls run report-only and are asserted from the outputs, so the # self-test is green exactly when every planted finding is detected. # `v1` is advanced only to a commit whose self-test is green. @@ -252,6 +253,50 @@ jobs: code-scanning: false harness-ref: ${{ github.sha }} + # A chart whose dependency is not committed: trivy skips the whole chart, + # so the configuration scan must fail instead of reporting it clean. + config-scan-skipped-chart: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - run: scripts/install-tool.sh trivy + # Output captured: the expected ::error:: lines are not annotations here. + - name: the skipped chart must fail trivy-config.sh + run: | + if out=$(scripts/trivy-config.sh tests/fixtures/helm "$RUNNER_TEMP/skipped.sarif" 2>&1); then + echo "::error::trivy-config.sh passed although trivy skipped the chart" + exit 1 + fi + if ! grep -q 'trivy skipped a Helm chart' <<<"$out"; then + echo "$out" + echo "::error::trivy-config.sh failed, but not on the skipped chart" + exit 1 + fi + echo "trivy-config.sh failed on the skipped chart as expected" + + # The same chart with config-setup-recipe fetching the dependency: the + # planted privileged container, which only the rendered chart shows, must be + # reported. + security-helm: + uses: ./.github/workflows/security.yml + permissions: + contents: read + security-events: write + with: + working-directory: tests/fixtures/helm + scan-path: tests/fixtures/helm + artifact-prefix: helm + gitleaks: false + semgrep: false + licenses: false + config-setup-recipe: deps + config-tools: helm + code-scanning: false + harness-ref: ${{ github.sha }} + # Dry run of the release workflow (not the merge of a release pull request: # builds both platforms natively, pushes nothing, needs no credentials). # The decisions of a real release (merge title, existing tag, re-run) are @@ -352,7 +397,7 @@ jobs: harness-ref: ${{ github.sha }} assert-planted: - needs: [security-planted, lint-planted] + needs: [security-planted, security-helm, lint-planted] # Also when a scanner job broke: the counts then show what is missing. if: ${{ !cancelled() }} runs-on: ubuntu-latest @@ -361,15 +406,20 @@ jobs: - name: every planted finding was reported env: FINDINGS: ${{ needs.security-planted.outputs.findings }} + HELM: ${{ needs.security-helm.outputs.findings }} LINT: ${{ needs.lint-planted.outputs.findings }} run: | echo "security: $FINDINGS" + echo "security (helm): $HELM" echo "workflow lint: $LINT" fail=0 for cat in secrets vuln-high misconfig-high semgrep-error; do n=$(jq -r --arg c "$cat" '.[$c] // 0' <<<"$FINDINGS") if [ "$n" -lt 1 ]; then echo "::error::no $cat finding in the planted fixture"; fail=1; fi done + if [ "$(jq -r '.["misconfig-high"] // 0' <<<"$HELM")" -lt 1 ]; then + echo "::error::no misconfig-high finding in the planted Helm chart"; fail=1 + fi for tool in zizmor pinact; do if [ "$(jq -r --arg t "$tool" '.[$t] // 0' <<<"$LINT")" != 1 ]; then echo "::error::$tool did not flag the planted workflow"; fail=1 @@ -381,7 +431,7 @@ jobs: # above succeeded (skipped or cancelled counts as failed). selftest-ok: if: ${{ always() }} - needs: [lint-self, lint-templates, lint-planted, scripts, gate-pass, gate-fail, node-gate-npm, node-gate-npm-tools, node-gate-bun, node-gate-fail, python-gate-pass, python-gate-fail, security-clean, security-clean-node, security-planted, release-dry-run, release-dry-run-package-json, build-context-artifact, release-dry-run-build-context, release-dry-run-image-tag, assert-image-tag, assert-planted] + needs: [lint-self, lint-templates, lint-planted, scripts, gate-pass, gate-fail, node-gate-npm, node-gate-npm-tools, node-gate-bun, node-gate-fail, python-gate-pass, python-gate-fail, security-clean, security-clean-node, security-planted, config-scan-skipped-chart, security-helm, release-dry-run, release-dry-run-package-json, build-context-artifact, release-dry-run-build-context, release-dry-run-image-tag, assert-image-tag, assert-planted] runs-on: ubuntu-latest timeout-minutes: 5 steps: diff --git a/README.md b/README.md index 53a58e6..c095ff7 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,13 @@ repository-local `integration` job next to the gate. vulnerable code is reachable. - A scanner job that fails or is cancelled (rather than finds something) always fails `report`: an incomplete scan is not a pass. +- Helm charts are scanned rendered. trivy skips a chart it cannot load (a + dependency missing from `charts/`, a required value without a default) with + only a warning, subcharts included, so a skipped chart fails the `config` job. + A repository whose chart dependencies are not committed fetches them in + `config-setup-recipe` (run in `working-directory`, with the tools from + `tools.lock` listed in `config-tools`, e.g. `helm`); values a chart requires + go into `misconfiguration.helm.set` of a `trivy.yaml` in the repository root. - semgrep runs the IronFlock rules plus `semgrep-rulesets` (default `p/default`). Go repositories, whose `just lint` runs gosec, set `semgrep-rulesets: ""` so each finding is reported (and annotated) once. @@ -212,6 +219,7 @@ request that adds it. Templates are in [templates/repo](templates/repo). | `.gitleaks.toml` | reviewed secret false positives | review with each change | | `.gitleaks-baseline.json` | historical findings that cannot be removed from history | — | | `// nosemgrep: ` | a reviewed semgrep false positive, with the reason | — | +| `semgrep-exclude-rules` (caller input) | a semgrep rule that cannot apply to the repository at all, with the reason next to it | review with each change | The summary lists exceptions that expire within 30 days. @@ -225,8 +233,10 @@ The summary lists exceptions that expire within 30 days. pull request and weekly. The fixtures plant one finding per scanner (a fake key, a reachable call into a vulnerable `golang.org/x/text`, a secret written to the console, a root container on an outdated base image, an unpinned action, - a failing test); the self-test is green only when each one is reported and - the clean fixture passes with every `fail-on` category set. Alerts that + a failing test, a privileged container in a Helm chart whose dependency is + fetched first); the self-test is green only when each one is reported, the + same chart without its dependency fails the scan, and the clean fixture + passes with every `fail-on` category set. Alerts that GitHub raises for the fixtures are expected. - **Releases.** Callers use the moving tag `v1`, so a change reaches every repository when `v1` moves. The harness has the same recipes as every diff --git a/scripts/sarif-summary.py b/scripts/sarif-summary.py index 1902fb4..36bc238 100755 --- a/scripts/sarif-summary.py +++ b/scripts/sarif-summary.py @@ -7,9 +7,10 @@ `report` reads every *.sarif and licenses*.json below DIR, sorts each finding into one category, drops findings an OpenVEX statement marks not_affected or -fixed, prints a Markdown summary (also appended to --summary) and writes -`findings` (JSON counts) and `blocking` (count) to --output. It exits 1 when a -category listed in --fail-on has findings, or when a scanner job failed. +fixed and results suppressed in the source (`nosemgrep`), prints a Markdown +summary (also appended to --summary) and writes `findings` (JSON counts) and +`blocking` (count) to --output. It exits 1 when a category listed in --fail-on +has findings, or when a scanner job failed. Categories: secrets, vuln-critical, vuln-high, vuln-other, misconfig-high, misconfig-other, license-forbidden, license-restricted, semgrep-error, @@ -196,6 +197,10 @@ def collect(directory, vex): for run in doc.get("runs") or []: tool, rules, rule_list = rule_index(run) for result in run.get("results") or []: + # In-source suppressions (`nosemgrep`) stay in the SARIF; one + # without a status, or accepted, takes the result out. + if any((s or {}).get("status", "accepted") == "accepted" for s in result.get("suppressions") or []): + continue rule = rules.get(result.get("ruleId")) if rule is None and isinstance(result.get("ruleIndex"), int) and result["ruleIndex"] < len(rule_list): rule = rule_list[result["ruleIndex"]] diff --git a/scripts/test_sarif_summary.py b/scripts/test_sarif_summary.py index 4e2fc16..a01e5f9 100644 --- a/scripts/test_sarif_summary.py +++ b/scripts/test_sarif_summary.py @@ -144,6 +144,18 @@ def test_semgrep_level_from_rule_default(self): self.assertEqual((findings["semgrep-error"], findings["semgrep-other"]), (1, 1)) self.assertEqual(proc.returncode, 1) + def test_in_source_suppression_is_not_a_finding(self): + c = self.case() + suppressed = result("r1") + suppressed["suppressions"] = [{"kind": "inSource"}] + rejected = result("r1", line=2) + rejected["suppressions"] = [{"kind": "inSource", "status": "rejected"}] + c.add("s.sarif", sarif("Semgrep OSS", [{"id": "r1", "defaultConfiguration": {"level": "error"}}], + [suppressed, rejected])) + proc, findings = c.report("semgrep-error") + self.assertEqual(findings["semgrep-error"], 1) + self.assertEqual(proc.returncode, 1) + def test_licenses(self): c = self.case() c.add("licenses.json", {"Results": [{"Target": "go.mod", "Licenses": [ diff --git a/scripts/trivy-config.sh b/scripts/trivy-config.sh new file mode 100755 index 0000000..0a08dfc --- /dev/null +++ b/scripts/trivy-config.sh @@ -0,0 +1,34 @@ +#!/usr/bin/env bash +# Run the trivy misconfiguration scan (Dockerfile, Kubernetes, Helm) and write +# a SARIF report. Never fails on findings; the report job decides. Fails when +# trivy skipped a Helm chart: a chart it cannot load (a dependency missing from +# charts/, a required value without a default) is skipped with a warning, +# subcharts included, and the report would look clean. +# trivy-config.sh +# Optional env: TRIVYIGNORE (used when the file exists). +set -euo pipefail + +path=$1 +out=$2 + +args=(--format sarif --output "$out") +if [ -n "${TRIVYIGNORE:-}" ] && [ -f "$TRIVYIGNORE" ]; then + args+=(--ignorefile "$TRIVYIGNORE") +fi + +# Not --quiet: a skipped chart shows only in the log. +log=$(mktemp) +trap 'rm -f "$log"' EXIT +status=0 +trivy config "${args[@]}" "$path" 2>"$log" || status=$? +cat "$log" >&2 +[ "$status" -eq 0 ] || exit "$status" + +skipped=$(grep -F 'Skipping chart' "$log" | sed 's/.*Skipping chart[[:space:]]*//' || true) +if [ -n "$skipped" ]; then + while IFS= read -r line; do + echo "::error::trivy skipped a Helm chart, so the scan does not cover it or its subcharts: $line" + done <<<"$skipped" + echo "::error::Make the chart load for the scan: fetch its dependencies first (config-setup-recipe, e.g. helm dependency build) or set the values it requires (misconfiguration.helm.set in trivy.yaml)." + exit 1 +fi diff --git a/templates/workflows/release.yml b/templates/workflows/release.yml index 7e49963..5f7fc79 100644 --- a/templates/workflows/release.yml +++ b/templates/workflows/release.yml @@ -51,6 +51,6 @@ jobs: contents: read # The App key is a secret of the `release` environment; environment # secrets reach a reusable workflow only through `secrets: inherit`. - secrets: inherit # zizmor: ignore[secrets-inherit] + secrets: inherit # zizmor: ignore[secrets-inherit] # nosemgrep: yaml.github-actions.security.secrets-inherit.secrets-inherit with: version-file: VERSION diff --git a/tests/fixtures/helm/Justfile b/tests/fixtures/helm/Justfile new file mode 100644 index 0000000..2328e33 --- /dev/null +++ b/tests/fixtures/helm/Justfile @@ -0,0 +1,6 @@ +# A chart whose dependency is not committed, like charts that pull their +# dependencies from a Helm repository: `deps` fetches it, as a repository's +# config-setup-recipe does before the configuration scan. + +deps: + helm dependency build chart diff --git a/tests/fixtures/helm/chart/.gitignore b/tests/fixtures/helm/chart/.gitignore new file mode 100644 index 0000000..742d091 --- /dev/null +++ b/tests/fixtures/helm/chart/.gitignore @@ -0,0 +1,3 @@ +# Fetched by `just deps`, never committed: the self-test needs the chart +# without it. +/charts/ diff --git a/tests/fixtures/helm/chart/Chart.lock b/tests/fixtures/helm/chart/Chart.lock new file mode 100644 index 0000000..cd7335a --- /dev/null +++ b/tests/fixtures/helm/chart/Chart.lock @@ -0,0 +1,6 @@ +dependencies: +- name: sub + repository: file://../sub + version: 0.1.0 +digest: sha256:da78b009933aa7e175d98d822bcf1735d3fa0f46623dfbde83c224af9de5f952 +generated: "2026-10-29T02:16:36.545217+01:00" diff --git a/tests/fixtures/helm/chart/Chart.yaml b/tests/fixtures/helm/chart/Chart.yaml new file mode 100644 index 0000000..f33979d --- /dev/null +++ b/tests/fixtures/helm/chart/Chart.yaml @@ -0,0 +1,8 @@ +apiVersion: v2 +name: planted +description: Planted for the harness self-test. Never install this chart. +version: 0.1.0 +dependencies: + - name: sub + version: 0.1.0 + repository: file://../sub diff --git a/tests/fixtures/helm/chart/templates/deployment.yaml b/tests/fixtures/helm/chart/templates/deployment.yaml new file mode 100644 index 0000000..8018ab5 --- /dev/null +++ b/tests/fixtures/helm/chart/templates/deployment.yaml @@ -0,0 +1,25 @@ +{{- /* +Planted for the harness self-test: a privileged container. The file is not +plain YAML, so only the rendered chart shows it, and trivy renders the chart +only when its dependency is in charts/. +*/}} +{{- if .Values.enabled }} +apiVersion: apps/v1 +kind: Deployment +metadata: + name: {{ .Release.Name }}-planted +spec: + selector: + matchLabels: + app: planted + template: + metadata: + labels: + app: planted + spec: + containers: + - name: planted + image: alpine:3.12 + securityContext: + privileged: true +{{- end }} diff --git a/tests/fixtures/helm/chart/values.yaml b/tests/fixtures/helm/chart/values.yaml new file mode 100644 index 0000000..d4ca941 --- /dev/null +++ b/tests/fixtures/helm/chart/values.yaml @@ -0,0 +1 @@ +enabled: true diff --git a/tests/fixtures/helm/sub/Chart.yaml b/tests/fixtures/helm/sub/Chart.yaml new file mode 100644 index 0000000..8a18044 --- /dev/null +++ b/tests/fixtures/helm/sub/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v2 +name: sub +description: The dependency of the planted chart (no templates). +version: 0.1.0