From dd4fa7b18c8234e7767316e8d3ca53f99ccf0854 Mon Sep 17 00:00:00 2001 From: stxkxs Date: Wed, 2 Sep 2026 03:30:24 -0700 Subject: [PATCH 1/6] Scan the images the pins reference, and make a CRITICAL a decision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An adopter of this catalog inherits every chart pin in it without reading, and the version a pin resolves to decides which image lands on their nodes. Nothing in the pipeline asked what those images carry. trivy-operator reports the same findings at runtime — on a cluster that already pulled the image, to an operator who cannot move the pin without a pull request here — and `trivy config` scans the rendered manifests, which is a different question entirely. So the scan belongs at the pin. `scripts/check-image-vulnerabilities.py` derives its population from `check-image-pins.inventory()`, the same rendered set that gate already walks, and blocks on a CRITICAL with a published fix that `image-advisories.yaml` does not acknowledge. ───────────────────────────── What blocks, and why ───────────────────────────── Three qualifiers, and each is load-bearing. CRITICAL, because HIGH is counted and printed instead: gating it would hold merges on the rate at which upstream chart images accumulate advisories rather than on anything a commit changed. WITH A PUBLISHED FIX, because a CRITICAL with none is not a decision anyone here can take. NOT ACKNOWLEDGED, because the alternative to an advisory file is a scan whose findings nobody has to answer. That leaves the objection `check-image-pins.py` already records — a new advisory can turn a pull request red for a reason the pull request did not cause. The advisory file is the answer to it rather than a softer bar: the failure names the image, the CVE and the fixed version, and clears by moving the pin or by recording why the finding stands. Both are decisions with an author. ──────────────────────── The advisory file cannot rot shut ──────────────────────── `image-advisories.yaml` ships nine entries covering the fifty fixed CRITICALs the catalog's fifty-five rendered images carry. Every one is checked against the scan on every run, in four directions: quay.io/argoproj/argo-events:v1.9.11: CVE-2026-33815 in github.com/jackc/pgx/v5 v5.7.5 is CRITICAL and fixed in 5.9.0, and no entry in image-advisories.yaml names it. Move the chart pin to a version carrying the fix, or record why the finding stands. quay.io/cilium/cilium:v1.19.6@sha256:0df5b275…: CVE-2026-56854 in golang.org/x/crypto is acknowledged, but the entry does not list quay.io/cilium/cilium. An acknowledgement covers the images it names — a new image acquiring a known CRITICAL is a decision, not an inheritance. image-advisories.yaml: CVE-2026-33815 in github.com/jackc/pgx/v5 lists quay.io/cilium/cilium, which no longer carries it. Drop that image from the entry. image-advisories.yaml: CVE-2026-33815 in github.com/jackc/pgx/v5 is acknowledged but no scanned image carries it — the entry outlived its reason. Delete it. Entries name images without a tag, so a chart bump does not churn the file — but a bump that FIXES a finding trips the third or fourth rule rather than leaving a stale excuse behind. An entry with no reason, and an entry with no images, fail on their own. ────────────────────── The canary, without which green is mute ────────────────────── A scanner with no database returns a clean result for every image, and by exit code that is indistinguishable from a healthy fleet. Every run therefore scans a digest-pinned end-of-life image first and requires its known CRITICALs back: canary OK: 3 fixed CRITICAL finding(s) from the pinned end-of-life image, so a clean result below is a result rather than a silence — CVE-2019-14697 (musl), CVE-2019-14697 (musl-utils), CVE-2021-36159 (apk-tools) Repointed at a patched image, the run refuses rather than passing: Cannot run: the canary returned no fixed CRITICAL findings. mirror.gcr.io/library/alpine:3.22 The canary is pinned by digest to an image whose CRITICALs have published fixes and cannot be patched away underneath it, so an empty result here is a fact about the scanner. An image that fails to scan exits 2 with the image named, because counting the rest as the whole fleet is how a partial scan reads as a complete one. ──────────────────── Three gates that passed over nothing ──────────────────── The same defect class, found while grading and fixed here because it is the same argument: a gate reports on the population it read, and nothing in an exit code separates "this catalog holds no violation" from "this run held no catalog". `scripts/check-hardcoded-org.py` guarded that `applicationsets/` exists and never guarded what the glob returned. Against a directory holding nothing, under `--blocking`: Scanned 0 applied ApplicationSet(s) in applicationsets/ (opt-in/ excluded) ✓ no applied ApplicationSet hardcodes nanohype/eks-gitops in a repoURL exit 0 It now exits 2 naming the count. `check-platform-crs.py` printed `walked` and compared it to nothing; `check-policy-admission.py` printed its rendered count and compared it only against a render failure. Both now carry floors, and both reject a corpus-emptying edit: FAIL walked 4 platform CR(s), below the floor of 5. The catalog's CRs were not matched against the chart's schemas, which is not the same as their being admissible. FAIL 7 manifest(s) rendered, below the floor of 40. The policies were evaluated against a fleet this catalog does not have, and 'no addon flagged' is a statement about that fleet rather than this one. `scripts/tests/test_corpus_floors.py` holds both bounds on all four floors against the tree: above zero, because zero is the vacuous pass with a constant in front of it; and below the real corpus, because a gate that is always red is a gate people route around. ────────────────────────────────── Wiring ────────────────────────────────── CI job `image-vulnerabilities`, blocking, in the `needs` of both `pr-summary` and `merge-gate` — the merge gate refuses any workflow containing a job it does not watch. It installs the same pinned trivy the `validate` job does, behind the same guard against `setup-trivy` reading an empty version as "latest". `task validate:image-vulnerabilities` runs it locally and is deliberately NOT in `task validate`: it pulls every image the pinned charts render, which is minutes rather than seconds. 25 unit tests over the verdict, plus 8 over the floors. The gate pulls images, so `scripts/tests/controls.py` exempts it from a positive control for the reason that list already records — and the exemption is asserted like every other. Co-authored-by: stxkxsbot <275011021+stxkxsbot@users.noreply.github.com> --- .github/workflows/ci.yml | 75 ++++- CLAUDE.md | 12 +- Taskfile.yaml | 8 + image-advisories.yaml | 156 +++++++++ scripts/check-hardcoded-org.py | 18 + scripts/check-image-vulnerabilities.py | 354 ++++++++++++++++++++ scripts/check-platform-crs.py | 15 + scripts/check-policy-admission.py | 16 + scripts/tests/controls.py | 1 + scripts/tests/run.py | 15 +- scripts/tests/test_corpus_floors.py | 111 ++++++ scripts/tests/test_image_vulnerabilities.py | 207 ++++++++++++ 12 files changed, 982 insertions(+), 6 deletions(-) create mode 100644 image-advisories.yaml create mode 100755 scripts/check-image-vulnerabilities.py create mode 100644 scripts/tests/test_corpus_floors.py create mode 100644 scripts/tests/test_image_vulnerabilities.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4bed326..21ab482 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -494,13 +494,77 @@ jobs: - name: Render every addon across every environment run: ./scripts/render-addons.py - # Tag immutability is a fact about the commit under test, so it blocks. - # Whether a CVE landed in one of these images overnight is not, and that - # half stays scheduled — blocking a merge on the world changing is how a - # gate teaches people to route around it. + # Tag immutability is a fact about the commit under test, so it blocks + # here. What the image CONTAINS is a separate question with a separate + # job — image-vulnerabilities — because it has to pull every image and is + # slow enough to be worth running in parallel with this one. - name: Every rendered image carries an immutable reference run: ./scripts/check-image-pins.py + # ── Image vulnerability gate ───────────────────────────────────────── + # Whoever moves a chart pin owns what the image behind it carries, and this + # repo pins independently of any cluster. trivy-operator reports the same + # findings at runtime, but to an operator who cannot change the pin without a + # pull request here — so the decision belongs at the pin, which is this job. + # + # Blocking on a CRITICAL with a published fix that image-advisories.yaml does + # not acknowledge. A new advisory can therefore turn a pull request red for a + # reason the pull request did not cause; that is deliberate and the advisory + # file is the release valve, because the alternative is a scan whose findings + # nobody has to answer. HIGH is counted and printed, not gated. + # + # The run scans a digest-pinned end-of-life image first and requires its known + # CRITICALs back. A scanner with no database returns a clean result for every + # image, which is indistinguishable from a healthy fleet by exit code — so + # without the canary a green run here would prove nothing at all. + image-vulnerabilities: + name: Rendered images carry no unacknowledged CRITICAL + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + + - name: Install Helm + uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 # v5 + + - name: Set up Python + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version-file: .python-version + - name: Install Python dependencies + run: pip install --require-hashes -r requirements.txt + + # Same guard the validate job carries: setup-trivy reads an empty version + # as "install the latest release", so an expansion failure would scan with + # an unpinned trivy and pass. + - name: Assert the pinned trivy version resolved + run: | + if [ -z "${{ env.TRIVY_VERSION }}" ]; then + echo "TRIVY_VERSION expanded empty. setup-trivy reads that as 'latest'," + echo "so this job would scan with an unpinned trivy and pass. Restore" + echo "the TRIVY_VERSION key in this workflow's env block." + exit 1 + fi + + - name: Install trivy + uses: aquasecurity/setup-trivy@81e514348e19b6112ce2a7e3ecbafe19c1e1f567 # v0.3.1 + with: + cache: true + version: ${{ env.TRIVY_VERSION }} # zizmor: ignore[unpinned-tools] + + - name: Cache Helm charts + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6 + with: + path: ~/.cache/helm + key: helm-charts-${{ hashFiles('applicationsets/*.yaml') }} + restore-keys: | + helm-charts- + + - name: Every fixed CRITICAL is acknowledged + run: ./scripts/check-image-vulnerabilities.py + # ── Policy-admission gate ──────────────────────────────────────────── # The best-practice / pod-security policies run in Enforce mode on staging and # production (the overlays flip validationFailureAction to Enforce), so a @@ -855,6 +919,7 @@ jobs: kyverno, fork-safety, helm-render, + image-vulnerabilities, policy-admission, appsets, appset-render, @@ -886,6 +951,7 @@ jobs: 'Kyverno policy tests (+ verify-images contract)': '${{ needs.kyverno.result }}', 'Fork-safety gate (no hardcoded org, blocking)': '${{ needs.fork-safety.result }}', 'Helm-render (every addon × every env)': '${{ needs.helm-render.result }}', + 'Rendered images carry no unacknowledged CRITICAL': '${{ needs.image-vulnerabilities.result }}', 'Policy-admission (Enforce-tier Kyverno vs the fleet)': '${{ needs.policy-admission.result }}', 'ApplicationSet schema + sync waves': '${{ needs.appsets.result }}', 'Appset render (Karpenter subnet selector)': '${{ needs.appset-render.result }}', @@ -941,6 +1007,7 @@ jobs: kyverno, fork-safety, helm-render, + image-vulnerabilities, policy-admission, appsets, appset-render, diff --git a/CLAUDE.md b/CLAUDE.md index d68dcab..e70f544 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -104,6 +104,7 @@ task validate:athena-panel-columns # Every column a CUR panel names is one the e task validate:fork-safety # No hardcoded catalog repoURL in applied ApplicationSets (report-only locally) task validate:log-volume-budget # Loki declares the fraction at which it stops ingesting, and the alert leads it task validate:falco-rule-floor # Every Falco rule set installed on a node is one Falco actually loads +task validate:image-vulnerabilities # Every fixed CRITICAL in a rendered image is acknowledged (not in `task validate` — see below) ``` ### Local `task validate` is a subset of CI @@ -111,7 +112,8 @@ task validate:falco-rule-floor # Every Falco rule set installed on a node is `task validate` runs the structural gates (lint, kustomize build, helm-render, ApplicationSet schema, sync-wave ordering, appset render, policy-admission, dashboards, fork-safety). CI runs those plus several gates that have **no local -`task` target**, so a clean `task validate` is necessary but not sufficient: +`task` target**, and one that has a target but is deliberately outside the +aggregate, so a clean `task validate` is necessary but not sufficient: - **Zero-placeholder gate** — `scripts/no-placeholders.sh` (CI job `placeholders`) - **Renovate config schema** — `renovate-config-validator`, CI-only because it @@ -134,6 +136,14 @@ dashboards, fork-safety). CI runs those plus several gates that have **no local `scripts/check-catalog-revision.py` (CI job `catalog-revision`) - **Alert coverage** — `scripts/check-alert-coverage.py`, which runs inside the `dashboards` job alongside the locally-available dashboard and Athena gates +- **Image vulnerabilities** — `scripts/check-image-vulnerabilities.py` (CI job + `image-vulnerabilities`). The one with a target of its own, + `task validate:image-vulnerabilities`, kept out of the aggregate because it + pulls every image the pinned charts render. Blocking on a CRITICAL + with a published fix that `image-advisories.yaml` does not acknowledge; HIGH is + counted and printed. Every run scans a digest-pinned end-of-life image first + and requires its known CRITICALs back, so a clean result is a result rather + than a scanner with no database - **Render → render-assert → kubeconform → `trivy config`** (CI job `validate`); locally this is `task render` then `task scan`, not part of `task validate` diff --git a/Taskfile.yaml b/Taskfile.yaml index e0a52bc..7a5a02a 100644 --- a/Taskfile.yaml +++ b/Taskfile.yaml @@ -93,6 +93,14 @@ tasks: cmds: - ./scripts/check-image-pins.py + validate:image-vulnerabilities: + desc: "Image-CVE gate — every fixed CRITICAL in a rendered image is acknowledged in image-advisories.yaml" + cmds: + # Pulls and scans every image the pinned charts render, so it is slower + # than the structural gates and is NOT in `task validate`. CI runs it as + # its own blocking job; run it here before moving a chart pin. + - ./scripts/check-image-vulnerabilities.py + validate:workflows: desc: "Workflow supply-chain gate — zizmor over .github/workflows, offline, MEDIUM+ blocks" cmds: diff --git a/image-advisories.yaml b/image-advisories.yaml new file mode 100644 index 0000000..e995891 --- /dev/null +++ b/image-advisories.yaml @@ -0,0 +1,156 @@ +# Fixed CRITICAL vulnerabilities this catalog knowingly ships, and why. +# +# scripts/check-image-vulnerabilities.py scans every image the pinned charts +# render and blocks on any CRITICAL with a published fix. An entry here says a +# human read that finding and decided the pin stands for now. Nothing else grants +# that: an image with no entry fails the gate, and an entry that stops matching +# the scan fails it too. +# +# Every entry is checked in four directions on every run, so this file cannot +# quietly become a permanent waiver: +# +# * a finding whose (id, package) no entry names -> unacknowledged, blocks +# * a finding on an image its entry does not list -> the entry does not cover it +# * an entry no scanned image carries -> the entry outlived its reason +# * an entry listing an image with no such finding -> the same, per image +# +# So the way to clear an entry is to move the chart pin to a version whose image +# carries the fixed package. Deleting the entry without doing that fails the +# first rule; doing it and leaving the entry behind fails the third or fourth. +# +# `images` name the image WITHOUT a tag. The acknowledgement is about the image; +# the tag moves with every chart bump, and a file that had to be edited on every +# bump would be edited without being read. +# +# ── What this catalog can and cannot do about these ── +# +# Every image below except one is built by the upstream project whose chart this +# catalog pins. This repo chooses a chart version; it does not build the image, +# and it cannot rebuild it against a patched dependency. The lever it has is the +# pin, and the pin moves when upstream publishes a chart whose image carries the +# fix — which Renovate opens a pull request for, and which this gate then either +# clears or does not. +# +# The exception is ghcr.io/nanohype/eks-agent-platform/eval-runner, which is +# built in this organisation. Its entry is a request to that repo's build, not to +# a chart pin here. + +advisories: + - id: CVE-2026-56854 + package: golang.org/x/crypto + reason: >- + Present in every Go-built image the fleet renders, because it is in the + module graph of almost every controller upstream ships. Fixed in + golang.org/x/crypto 0.55.0, which each project picks up on its own release + cadence; this catalog carries no build for any of them. Clears image by + image as chart pins move to versions built against 0.55.0 or later. + images: + - docker.io/envoyproxy/ai-gateway-controller + - docker.io/envoyproxy/gateway + - docker.io/falcosecurity/falcoctl + - docker.io/grafana/loki + - docker.io/grafana/loki-canary + - docker.io/grafana/tempo + - docker.io/velero/velero + - ghcr.io/external-secrets/external-secrets + - ghcr.io/grafana/grafana-operator + - ghcr.io/kedacore/keda + - ghcr.io/kedacore/keda-admission-webhooks + - ghcr.io/kedacore/keda-metrics-apiserver + - ghcr.io/opencost/opencost + - mirror.gcr.io/aquasec/trivy-operator + - otel/opentelemetry-collector-contrib + - prom/memcached-exporter + - quay.io/argoproj/argo-events + - quay.io/argoproj/argocli + - quay.io/argoproj/workflow-controller + - quay.io/cilium/cilium + - quay.io/jetstack/cert-manager-cainjector + - quay.io/jetstack/cert-manager-controller + - quay.io/jetstack/cert-manager-startupapicheck + - quay.io/jetstack/cert-manager-webhook + - reg.kyverno.io/kyverno/background-controller + - reg.kyverno.io/kyverno/cleanup-controller + - reg.kyverno.io/kyverno/kyverno + - reg.kyverno.io/kyverno/kyverno-cli + - reg.kyverno.io/kyverno/kyvernopre + - reg.kyverno.io/kyverno/reports-controller + - registry.k8s.io/descheduler/descheduler + - registry.k8s.io/external-dns/external-dns + - registry.k8s.io/kube-state-metrics/kube-state-metrics + - registry.k8s.io/metrics-server/metrics-server + + - id: CVE-2025-68121 + package: stdlib + reason: >- + The Go standard library compiled into the binary, not a dependency the + image can update separately — it moves when upstream rebuilds on a patched + toolchain. Fixed in Go 1.24.13, 1.25.7 and 1.26.0-rc.3. + images: + - docker.io/falcosecurity/k8s-metacollector + - quay.io/argoproj/argo-events + - registry.k8s.io/metrics-server/metrics-server + - us-docker.pkg.dev/fairwinds-ops/oss/goldilocks + + - id: CVE-2024-24790 + package: stdlib + reason: >- + The same shape as CVE-2025-68121 and on the same image, against an older + toolchain: k8s-metacollector 0.1.1 is built on Go 1.21.1, and the fix is in + 1.21.11 and 1.22.4. The falco chart pin is what moves it. + images: + - docker.io/falcosecurity/k8s-metacollector + + - id: CVE-2026-33186 + package: google.golang.org/grpc + reason: >- + A transitive module in three upstream controllers, fixed in grpc-go 1.79.3. + Not separable from the binary; it moves when each project rebuilds. + images: + - docker.io/falcosecurity/k8s-metacollector + - quay.io/argoproj/argo-events + - registry.k8s.io/metrics-server/metrics-server + + - id: CVE-2026-33815 + package: github.com/jackc/pgx/v5 + reason: >- + The Postgres driver argo-events links for its Postgres event source, fixed + in pgx 5.9.0. This catalog does not enable that event source, but the + package is in the shipped binary either way, so the finding is real and the + remedy is the same — the argo-events chart pin. + images: + - quay.io/argoproj/argo-events + + - id: CVE-2026-33816 + package: github.com/jackc/pgx/v5 + reason: >- + The second advisory against the same driver version in the same binary, + with the same fixed version and the same remedy as CVE-2026-33815. + images: + - quay.io/argoproj/argo-events + + - id: CVE-2026-31789 + package: libcrypto3 + reason: >- + OpenSSL in the goldilocks image's Alpine base, fixed in 3.5.6-r0. A base + layer this catalog does not build; it moves with the goldilocks chart pin. + images: + - us-docker.pkg.dev/fairwinds-ops/oss/goldilocks + + - id: CVE-2026-31789 + package: libssl3 + reason: >- + The same OpenSSL advisory against the sibling package in the same base + layer, cleared by the same chart bump as the libcrypto3 entry. + images: + - us-docker.pkg.dev/fairwinds-ops/oss/goldilocks + + - id: CVE-2026-59873 + package: tar + reason: >- + The one first-party image in this list. eval-runner is built in + nanohype/eks-agent-platform, so the remedy is a rebuild on a base carrying + tar 7.5.19 and a new operator chart version here — not a decision this + repository can take on its own. + images: + - ghcr.io/nanohype/eks-agent-platform/eval-runner diff --git a/scripts/check-hardcoded-org.py b/scripts/check-hardcoded-org.py index 04107dd..05e8938 100755 --- a/scripts/check-hardcoded-org.py +++ b/scripts/check-hardcoded-org.py @@ -82,6 +82,12 @@ ORG = "nanohype" CATALOG = "eks-gitops" # THIS repo — the vended catalog. NOT the product repos. +# A floor on applied ApplicationSets read. Set well under what the catalog ships, +# so it catches "the glob matched almost nothing" — a renamed directory, a wrong +# --root, a working tree that never checked out — and never "one appset was +# retired". +MIN_APPSETS = 20 + # A repoURL whose value points at the CATALOG repo, over either transport ArgoCD # accepts. Anchored on `repoURL:` so image refs, oci:// chart repos, and the # Kyverno subjectRegExp are structurally out of scope; pinned to the catalog repo @@ -119,6 +125,18 @@ def main() -> int: # the top level can strand a fork. See the opt-in note in the docstring. files = sorted(p for p in appsets.glob("*.y*ml") if p.is_file()) + # A floor on what was EXAMINED. The directory existing is not the same as it + # holding the fleet: with the glob answering nothing, this printed + # "Scanned 0 applied ApplicationSet(s)" and its success line, and exited 0 + # under --blocking. Every other outcome of this gate is a sentence about what + # it read; that one was a sentence about a set it never had. + if len(files) < MIN_APPSETS: + print(f"FAIL found {len(files)} applied ApplicationSet(s) under " + f"{appsets.relative_to(args.root)}, below the floor of {MIN_APPSETS}.") + print(" A scan over almost nothing reports the same as a scan over a") + print(" catalog with no hardcoded repoURL in it.") + return 2 + violations: list[tuple[pathlib.Path, int, str]] = [] for path in files: for lineno, line in enumerate( diff --git a/scripts/check-image-vulnerabilities.py b/scripts/check-image-vulnerabilities.py new file mode 100755 index 0000000..5467eff --- /dev/null +++ b/scripts/check-image-vulnerabilities.py @@ -0,0 +1,354 @@ +#!/usr/bin/env python3 +"""Every image the pinned charts reference is scanned, and a CRITICAL is a decision. + + scripts/check-image-vulnerabilities.py # blocking gate + scripts/check-image-vulnerabilities.py --list # print every finding + scripts/check-image-vulnerabilities.py --self-test # the canary alone + +WHOSE QUESTION THIS IS + +An adopter of this catalog inherits every chart pin in it without reading. The +version a pin resolves to decides which image lands on their nodes, so whoever +moves the pin owns what the image carries. Nothing else in the pipeline can: +trivy-operator reports at runtime, on a cluster that already pulled the image, +to an operator who cannot change the pin without a PR here. + +WHAT IS BLOCKING AND WHAT IS NOT + +Blocking: a CRITICAL vulnerability WITH A FIX AVAILABLE, in an image the fleet +renders, that `image-advisories.yaml` does not acknowledge. Those three +qualifiers are the whole bar. A CRITICAL with no fix published is not a decision +anyone here can act on; a HIGH is counted and printed, and the count is not +gated, because holding a merge on it would gate this repo on the rate at which +upstream chart images accumulate advisories rather than on anything the commit +changed. + +That leaves a real objection, and the answer to it is the advisory file rather +than a softer bar. Whether a CVE landed overnight is not a fact about the commit, +so a new CRITICAL can turn a pull request red for a reason the pull request did +not cause. What it cannot do is turn it red silently or permanently: the failure +names the image, the CVE and the fixed version, and clears by bumping the chart +or by recording why the finding stands. Both are decisions with an author. + +THE ADVISORY FILE IS ASSERTED IN FOUR DIRECTIONS + +An acknowledgement list nobody re-checks only ever widens, so every entry is +checked against the scan rather than trusted: + + * a finding whose (id, package) no entry names -> the gate's own question + * a finding on an image its entry does not list -> the entry does not cover it + * an entry whose (id, package) no image carries -> the entry outlived its reason + * an entry listing an image with no such finding -> the same, per image + +Entries name images WITHOUT a tag, because the acknowledgement is about the +image and a chart bump moves the tag. A bump that FIXES the finding therefore +fails the third or fourth rule rather than leaving a stale excuse behind. + +THE CANARY, WHICH IS WHAT MAKES A CLEAN RESULT MEAN ANYTHING + +A vulnerability scanner that has lost its database, or been handed a flag it +reads as "report nothing", returns a clean result for every image. That is +indistinguishable from a healthy fleet by exit code and by output alone. + +So every run first scans a digest-pinned image with historical, fixed CRITICALs +and requires them to come back. A canary that comes back clean exits 2: it is a +statement about the scanner, not about the catalogue, and the two must not print +the same thing. + +UNSCANNABLE IS NOT CLEAN + +An image that could not be pulled contributed no findings. Counting the rest as +the whole fleet is how a partial scan reads as a complete one, so any image that +fails to scan exits 2 with the image named. +""" + +from __future__ import annotations + +import argparse +import collections +import importlib.util +import json +import pathlib +import subprocess +import sys +from typing import NoReturn + +# Shared precondition helper, loaded by path: these are hyphenated executables +# run from varying working directories. +_gl = pathlib.Path(__file__).resolve().parent / "gatelib.py" +_gs = importlib.util.spec_from_file_location("gatelib", _gl) +assert _gs and _gs.loader, f"{_gl} is not loadable as a module" +gatelib = importlib.util.module_from_spec(_gs) +sys.modules["gatelib"] = gatelib +_gs.loader.exec_module(gatelib) + +# The image inventory, from the gate that already derives it. Re-deriving the +# population here would let the two disagree about what the fleet is, and the +# one that scanned fewer images would be the one printing a clean result. +_ip_path = pathlib.Path(__file__).resolve().parent / "check-image-pins.py" +_ip_spec = importlib.util.spec_from_file_location("check_image_pins", _ip_path) +assert _ip_spec and _ip_spec.loader, f"{_ip_path} is not loadable as a module" +image_pins = importlib.util.module_from_spec(_ip_spec) +sys.modules["check_image_pins"] = image_pins +_ip_spec.loader.exec_module(image_pins) + +ROOT = pathlib.Path(__file__).resolve().parent.parent +ADVISORIES = ROOT / "image-advisories.yaml" + +# Seconds one trivy invocation may run. A scan with no deadline turns a stalled +# registry into a job that hangs until the CI runner's own ceiling, with no +# diagnostic naming the image that stalled. +SCAN_TIMEOUT = 600 + +# The severity that blocks. HIGH is counted and printed; see the header. +BLOCKING_SEVERITY = "CRITICAL" +REPORTED_SEVERITIES = ["CRITICAL", "HIGH"] + +# A floor on IMAGES SCANNED, not on findings. Printing a denominator is not +# gating on it: with no floor, a render that produced two images would report the +# fleet clean. Set well under what the catalog renders, so it catches "scanned +# almost nothing" rather than "a chart was removed". +MIN_IMAGES = 40 + +# A digest-pinned image with historical CRITICALs that have published fixes. +# Alpine 3.10 is end-of-life, so these cannot be patched away underneath the +# canary, and the digest means the tag cannot be repointed at a clean build. +# +# mirror.gcr.io rather than docker.io: this pulls on every CI run, and Docker +# Hub's anonymous limits are the problem that mirror exists to solve. The +# catalog already pulls trivy-operator through it. +CANARY = ("mirror.gcr.io/library/alpine@sha256:" + "ca1c944a4f8486a153024d9965aafbe24f5723c1d5c02f4964c045a16d19dc54") + +Finding = collections.namedtuple("Finding", "image bare id package installed fixed severity") + +failures: list[str] = [] + + +def fail(msg: str) -> None: + failures.append(msg) + + +def cannot_run(*lines: str) -> NoReturn: + for line in lines: + print(line) + print("This gate examined nothing, which is not the same as finding nothing.") + sys.exit(gatelib.CANNOT_RUN) + + +def bare(ref: str) -> str: + """An image reference with tag and digest removed. + + What an advisory names. The tag moves with every chart bump and the digest + moves with every rebuild; the image is what a reader recognises and what the + acknowledgement is actually about. + + The tag separator is the last colon in the FINAL path segment. A registry + with a port carries a colon that is not one, and cutting there produces a key + no advisory can match and no reader can recognise. + """ + ref = ref.split("@", 1)[0] + name = ref.rsplit("/", 1)[-1] + return ref.rsplit(":", 1)[0] if ":" in name else ref + + +def scan(ref: str) -> list[Finding] | None: + """Fixed findings at the reported severities, or None if the image did not scan.""" + cmd = ["trivy", "image", "--quiet", "--scanners", "vuln", + "--severity", ",".join(REPORTED_SEVERITIES), + "--ignore-unfixed", "--format", "json", ref] + try: + proc = subprocess.run(cmd, capture_output=True, text=True, timeout=SCAN_TIMEOUT) + except subprocess.TimeoutExpired: + return None + if proc.returncode != 0: + return None + try: + report = json.loads(proc.stdout) + except json.JSONDecodeError: + return None + out = [] + for result in report.get("Results") or []: + for v in result.get("Vulnerabilities") or []: + out.append(Finding(ref, bare(ref), v["VulnerabilityID"], v.get("PkgName", ""), + v.get("InstalledVersion", ""), v.get("FixedVersion", ""), + v.get("Severity", ""))) + return out + + +def read_advisories() -> list[dict]: + """The acknowledged findings, or exit 2 naming what is wrong with the file.""" + if not ADVISORIES.is_file(): + cannot_run(f"Cannot run: {ADVISORIES.name} does not exist, so every finding " + f"below would be reported as unacknowledged whether or not a " + f"decision had been recorded about it.") + doc = gatelib.read_yaml_all(ADVISORIES) + entries = (doc[0] or {}).get("advisories") if doc else None + if entries is None: + cannot_run(f"Cannot run: {ADVISORIES.name} declares no `advisories` key.") + return list(entries) + + +def verdict(findings: list[Finding], advisories: list[dict]) -> list[str]: + """Everything wrong, over a scan and an acknowledgement list. + + Four directions, and only the first is the question the gate is named for. + The other three are what stop the acknowledgement list from becoming a + permanent waiver nobody re-reads. + + Deduplicated in order. One image can carry the same advisory in several + scanned artifacts — a Go binary and the layer it sits in — and printing the + identical sentence twice reads as two findings needing two decisions. + """ + problems: list[str] = [] + blocking = [f for f in findings if f.severity == BLOCKING_SEVERITY] + + by_key: dict[tuple[str, str], dict] = {} + for entry in advisories: + key = (str(entry.get("id", "")), str(entry.get("package", ""))) + if not entry.get("reason", "").strip(): + problems.append( + f"image-advisories.yaml: {key[0]} in {key[1]} is acknowledged with no " + f"reason recorded, so nothing states what would let it be removed.") + by_key[key] = entry + + # 1. A blocking finding nothing acknowledges — the gate's own question. + # 2. A blocking finding on an image its entry does not name. + covered: set[tuple[str, str, str]] = set() + for f in blocking: + key = (f.id, f.package) + acknowledged = by_key.get(key) + if acknowledged is None: + problems.append( + f"{f.image}: {f.id} in {f.package} {f.installed} is CRITICAL and fixed " + f"in {f.fixed}, and no entry in image-advisories.yaml names it. Move the " + f"chart pin to a version carrying the fix, or record why the finding " + f"stands.") + continue + listed = [str(i) for i in (acknowledged.get("images") or [])] + if f.bare not in listed: + problems.append( + f"{f.image}: {f.id} in {f.package} is acknowledged, but the entry does " + f"not list {f.bare}. An acknowledgement covers the images it names — a " + f"new image acquiring a known CRITICAL is a decision, not an inheritance.") + continue + covered.add((f.id, f.package, f.bare)) + + # 3. An entry no image carries. 4. An entry listing an image with no such finding. + carried = {(f.id, f.package, f.bare) for f in blocking} + for entry in advisories: + key = (str(entry.get("id", "")), str(entry.get("package", ""))) + listed = [str(i) for i in (entry.get("images") or [])] + if not listed: + problems.append( + f"image-advisories.yaml: {key[0]} in {key[1]} lists no images, so it " + f"acknowledges nothing while reading as a considered decision.") + continue + if not any((key[0], key[1], img) in carried for img in listed): + problems.append( + f"image-advisories.yaml: {key[0]} in {key[1]} is acknowledged but no " + f"scanned image carries it — the entry outlived its reason. Delete it.") + continue + for img in listed: + if (key[0], key[1], img) not in carried: + problems.append( + f"image-advisories.yaml: {key[0]} in {key[1]} lists {img}, which no " + f"longer carries it. Drop that image from the entry.") + del covered + return list(dict.fromkeys(problems)) + + +def run_canary() -> int: + """Prove the scanner reports something before believing it reported nothing.""" + found = scan(CANARY) + if found is None: + cannot_run("Cannot run: the canary image could not be scanned.", + f" {CANARY}", + "An unreachable registry is a fact about the network, not about " + "the images this catalog pins.") + critical = [f for f in found if f.severity == BLOCKING_SEVERITY] + if not critical: + cannot_run("Cannot run: the canary returned no fixed CRITICAL findings.", + f" {CANARY}", + "The canary is pinned by digest to an image whose CRITICALs have " + "published fixes and cannot be patched away underneath it, so an " + "empty result here is a fact about the scanner. One with no " + "database, or one reading a flag as 'report nothing', returns a " + "clean result for every image — including every image below.") + print(f"canary OK: {len(critical)} fixed CRITICAL finding(s) from the pinned " + f"end-of-life image, so a clean result below is a result rather than a " + f"silence — " + + ", ".join(sorted({f"{f.id} ({f.package})" for f in critical}))) + return 0 + + +def main() -> int: + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("--list", action="store_true", + help="print every finding at the reported severities, then exit 0") + ap.add_argument("--self-test", action="store_true", + help="run the canary alone") + ap.add_argument("--env", default="production") + args = ap.parse_args() + + gatelib.require("trivy", "helm") + run_canary() + if args.self_test: + return 0 + + images, unrendered = image_pins.inventory(args.env) + if unrendered: + cannot_run("Cannot run: charts that did not render contributed no images, so " + "the scan below would cover part of the fleet and report on all of " + "it.", + *(f" {path}: {err}" for path, err in unrendered)) + if len(images) < MIN_IMAGES: + cannot_run(f"Cannot run: the render produced {len(images)} image(s), under the " + f"floor of {MIN_IMAGES}. A scan over almost nothing reports the same " + f"as a scan over a clean fleet.") + + findings: list[Finding] = [] + unscannable: list[str] = [] + for ref in sorted(images): + got = scan(ref) + if got is None: + unscannable.append(ref) + continue + findings.extend(got) + if unscannable: + cannot_run("Cannot run: these images could not be scanned, and the ones that " + "were say nothing about them:", + *(f" {ref} (via {', '.join(sorted(images[ref]))})" + for ref in unscannable)) + + if args.list: + for f in sorted(findings, key=lambda f: (f.severity, f.image, f.id)): + print(f"{f.severity:8} {f.image} {f.id} {f.package} {f.installed} " + f"-> {f.fixed}") + print(f"\n{len(findings)} fixed finding(s) at {'/'.join(REPORTED_SEVERITIES)} " + f"across {len(images)} image(s)") + return 0 + + for problem in verdict(findings, read_advisories()): + fail(problem) + + critical = [f for f in findings if f.severity == BLOCKING_SEVERITY] + high = [f for f in findings if f.severity == "HIGH"] + + if failures: + print(f"{len(failures)} problem(s) across {len(images)} scanned image(s):\n") + for problem in failures: + print(f" {problem}") + return 1 + + print(f"image vulnerabilities OK: {len(images)} image(s) scanned across " + f"{len(set().union(*images.values()))} chart(s). " + f"{len(critical)} fixed CRITICAL finding(s), every one acknowledged in " + f"image-advisories.yaml against the image carrying it, and every " + f"acknowledgement still matched by the scan. {len(high)} fixed HIGH " + f"finding(s) counted and not gated — run --list to read them.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/check-platform-crs.py b/scripts/check-platform-crs.py index ec50f78..30fccea 100755 --- a/scripts/check-platform-crs.py +++ b/scripts/check-platform-crs.py @@ -91,6 +91,11 @@ SKIP_DIRS = {".git", "node_modules", "rendertest", "__pycache__", ".task"} +# A floor on platform CRs walked. Set under what the catalog declares, so it +# catches a walk that matched almost nothing rather than one CR being retired. +MIN_CRS = 5 + + def pinned_chart_version() -> str: """The operator chart version this catalog installs. @@ -438,6 +443,16 @@ def check(listing: bool, offline: bool) -> int: ) return 1 + # A floor on CRs WALKED. The walk skips any document whose kind is not in + # the resolved schema set, so a renamed kind, a moved manifest or an + # apiVersion bump takes its CRs out of the population — and an empty + # population prints the same sentence as a compliant one. + if walked < MIN_CRS: + print(f"\nFAIL walked {walked} platform CR(s), below the floor of {MIN_CRS}. " + f"The catalog's CRs were not matched against the chart's schemas, which " + f"is not the same as their being admissible.", file=sys.stderr) + return 2 + print(f"\nok: {walked} platform CR(s) admissible against operator chart {version}") return 0 diff --git a/scripts/check-policy-admission.py b/scripts/check-policy-admission.py index 46ede36..0818079 100755 --- a/scripts/check-policy-admission.py +++ b/scripts/check-policy-admission.py @@ -166,6 +166,12 @@ # Enforce runs on staging + production; both flip the base policies to Enforce. ENFORCE_ENVS = ["staging", "production"] +# A floor on addon×env manifests rendered into the resource file. Set well under +# what the catalog produces, so it catches "discovery matched almost nothing" +# rather than "an addon was retired". +MIN_RENDERED = 40 + + # Kinds Kyverno's workload policies (and their autogen pod-controller variants) # evaluate. Namespace-scoped, so they must carry metadata.namespace for the # exclusion match to fire — helm -n stamps it on most charts; this backfills any @@ -682,6 +688,16 @@ def main() -> int: return 1 print(f" rendered {count} addon×env manifests into their namespaces, " f"plus the canary\n") + # A floor on what was RENDERED, not on what was found. `count` was + # printed and compared only against a render failure, so a discovery + # that matched almost nothing produced a fleet of a handful of manifests + # and the run still reported that no addon would be denied. + if count < MIN_RENDERED: + print(f" FAIL {count} manifest(s) rendered, below the floor of " + f"{MIN_RENDERED}. The policies were evaluated against a fleet " + f"this catalog does not have, and 'no addon flagged' is a " + f"statement about that fleet rather than this one.\n") + return 2 coverage_ok = check_namespace_coverage(landed, excluded) diff --git a/scripts/tests/controls.py b/scripts/tests/controls.py index 7dadd5b..fb2ff0f 100755 --- a/scripts/tests/controls.py +++ b/scripts/tests/controls.py @@ -142,6 +142,7 @@ def __repr__(self) -> str: "check-image-pins.py": "subprocess.run", "check-log-volume-budget.py": "subprocess.run", "check-falco-rule-floor.py": "subprocess.run", + "check-image-vulnerabilities.py": "subprocess.run", } # Shell gates: no syntax tree available, so this is a text check over the diff --git a/scripts/tests/run.py b/scripts/tests/run.py index 75ef305..b77bc0c 100755 --- a/scripts/tests/run.py +++ b/scripts/tests/run.py @@ -36,6 +36,8 @@ "test_named_things", "test_falco_rule_floor", "test_gatelib", + "test_image_vulnerabilities", + "test_corpus_floors", ) # A floor well under the real count. It catches "discovery found almost nothing", @@ -85,7 +87,17 @@ # Ratchets downward only. Adding a gate without tests fails here rather than # diluting the combined figure by a percentage point nobody notices, which is how # a suite reaches this state one honest commit at a time. -MAX_UNCOVERED_GATES = 17 +# +# Two of the files this counts as covered are covered by IMPORT, not by tests. +# check-image-vulnerabilities.py loads check-image-pins.py by path so the two +# cannot disagree about what the fleet is, and that loads render-addons.py in +# turn; a test module importing the first executes the module bodies of all +# three. Coverage cannot tell that from a test, so both read as non-zero while +# carrying no assertions of their own. The ceiling is lowered anyway, because a +# ratchet that declines to move is one nobody can regress against — but it is +# the weaker half of this file's claim, and the per-gate floors and the controls +# in scripts/tests/controls.py are where the real one lives. +MAX_UNCOVERED_GATES = 15 PER_GATE_FLOORS = { "scripts/check-named-things.py": 35, @@ -93,6 +105,7 @@ "scripts/check-label-values.py": 33, "scripts/check-sync-waves.py": 10, "scripts/gatelib.py": 55, + "scripts/check-image-vulnerabilities.py": 45, } diff --git a/scripts/tests/test_corpus_floors.py b/scripts/tests/test_corpus_floors.py new file mode 100644 index 0000000..8c4e1ae --- /dev/null +++ b/scripts/tests/test_corpus_floors.py @@ -0,0 +1,111 @@ +"""Every gate that enumerates a corpus refuses to pass over an empty one. + +A gate reports on the population it read. Nothing in an exit code distinguishes +"this catalog holds no violation" from "this run held no catalog", and the second +is what a renamed directory, a wrong `--root`, a narrowed glob or a filter that +stopped matching all look like. The fork-safety gate printed +`Scanned 0 applied ApplicationSet(s)` followed by its success line and exited 0 +under `--blocking`. + +So each of those gates carries a floor on what it EXAMINED, and each floor has +two ways to be wrong. Set to zero it grants exactly the vacuous pass it exists to +stop. Set above the real corpus it fails every run, and a gate that is always red +is a gate people route around — which ends in the same place by a longer road. + +These assert both bounds against the tree rather than against a number written +here, so the floors stay meaningful as the catalog grows. +""" + +from __future__ import annotations + +import pathlib +import unittest + +import yaml +from gateloader import load + +ROOT = pathlib.Path(__file__).resolve().parent.parent.parent + +fork_safety = load("check-hardcoded-org") +platform_crs = load("check-platform-crs") +policy_admission = load("check-policy-admission") +image_vulns = load("check-image-vulnerabilities") + + +class EveryFloorIsAboveZero(unittest.TestCase): + """A floor of zero is the vacuous pass with a constant in front of it.""" + + def test_each_gate_declares_one(self): + for label, floor in ( + ("check-hardcoded-org MIN_APPSETS", fork_safety.MIN_APPSETS), + ("check-platform-crs MIN_CRS", platform_crs.MIN_CRS), + ("check-policy-admission MIN_RENDERED", policy_admission.MIN_RENDERED), + ("check-image-vulnerabilities MIN_IMAGES", image_vulns.MIN_IMAGES), + ): + with self.subTest(floor=label): + self.assertGreater(floor, 0) + + +class EveryFloorIsBelowTheCorpusItGuards(unittest.TestCase): + """Measured off the tree. A floor above the real population is always red.""" + + def test_the_applied_appsets_clear_the_fork_safety_floor(self): + applied = [p for p in (ROOT / "applicationsets").glob("*.y*ml") if p.is_file()] + self.assertGreater(len(applied), fork_safety.MIN_APPSETS, + "MIN_APPSETS is at or above the number of applied " + "ApplicationSets, so the fork-safety gate cannot pass") + + def test_the_platform_crs_clear_their_floor(self): + """Counted with the gate's own filters: chart source is Go-template text, + which the walk identifies structurally rather than by what breaks a parser.""" + found = 0 + for path in platform_crs.manifests(): + if platform_crs.gatelib.is_helm_template(path): + continue + for doc in yaml.safe_load_all(path.read_text()): + if not isinstance(doc, dict): + continue + if str(doc.get("apiVersion", "")).endswith( + "/" + platform_crs.CRD_VERSION): + found += 1 + self.assertGreater(found, platform_crs.MIN_CRS, + "MIN_CRS is at or above the number of platform CRs in the " + "tree, so check-platform-crs.py cannot pass") + + def test_the_discovered_addons_clear_the_policy_admission_floor(self): + """One unit renders at least one manifest, so units bound the render below.""" + units = policy_admission.discover() + self.assertGreater(len(units), 0) + self.assertGreaterEqual( + policy_admission.MIN_RENDERED, len(units), + "MIN_RENDERED sits below the number of addon units discovered, so it " + "would pass a render that produced one manifest per unit and nothing " + "for the environments — which is the shape it exists to catch") + + +class TheFloorsGuardTheRightQuantity(unittest.TestCase): + """A floor on findings is not a floor on the corpus. + + Counting what was FOUND cannot separate a clean catalog from an unexamined + one; only counting what was READ can. + """ + + def test_the_fork_safety_floor_is_read_off_the_file_list(self): + source = (ROOT / "scripts" / "check-hardcoded-org.py").read_text() + self.assertIn("if len(files) < MIN_APPSETS:", source) + + def test_the_platform_crs_floor_is_read_off_the_walk_count(self): + source = (ROOT / "scripts" / "check-platform-crs.py").read_text() + self.assertIn("if walked < MIN_CRS:", source) + + def test_the_policy_admission_floor_is_read_off_the_render_count(self): + source = (ROOT / "scripts" / "check-policy-admission.py").read_text() + self.assertIn("if count < MIN_RENDERED:", source) + + def test_the_image_floor_is_read_off_the_inventory(self): + source = (ROOT / "scripts" / "check-image-vulnerabilities.py").read_text() + self.assertIn("if len(images) < MIN_IMAGES:", source) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/tests/test_image_vulnerabilities.py b/scripts/tests/test_image_vulnerabilities.py new file mode 100644 index 0000000..071b9da --- /dev/null +++ b/scripts/tests/test_image_vulnerabilities.py @@ -0,0 +1,207 @@ +"""Unit tests for the image-vulnerability gate's verdict. + +The gate pulls and scans every image the pinned charts render, so it reaches a +registry and the positive-control sweep exempts it. What that leaves untested is +the part that decides the outcome: whether a finding is acknowledged, and whether +an acknowledgement still describes the scan. + +Both directions matter and they fail for opposite reasons. Too strict and the +gate reports decisions that were already taken; too loose and +`image-advisories.yaml` becomes a permanent waiver, which is the one outcome that +would make a green run worse than no gate at all. + +Everything here is offline. The trivy invocation and the canary are exercised by +running the gate, not from here. +""" + +from __future__ import annotations + +import pathlib +import unittest + +import yaml +from gateloader import load + +gate = load("check-image-vulnerabilities") + +ROOT = pathlib.Path(__file__).resolve().parent.parent.parent + + +def finding(image="quay.io/argoproj/argo-events:v1.9.11", cve="CVE-2026-33815", + package="github.com/jackc/pgx/v5", installed="v5.7.5", fixed="5.9.0", + severity="CRITICAL"): + return gate.Finding(image, gate.bare(image), cve, package, installed, fixed, + severity) + + +def advisory(cve="CVE-2026-33815", package="github.com/jackc/pgx/v5", + reason="upstream rebuild", images=("quay.io/argoproj/argo-events",)): + return {"id": cve, "package": package, "reason": reason, "images": list(images)} + + +class TheImageAnAdvisoryNames(unittest.TestCase): + """An acknowledgement is about the image; the tag moves with every bump.""" + + def test_a_tag_is_dropped(self): + self.assertEqual(gate.bare("quay.io/argoproj/argo-events:v1.9.11"), + "quay.io/argoproj/argo-events") + + def test_a_digest_is_dropped(self): + self.assertEqual(gate.bare("quay.io/cilium/cilium@sha256:" + "0" * 64), + "quay.io/cilium/cilium") + + def test_a_tag_and_a_digest_together_are_dropped(self): + self.assertEqual( + gate.bare("ghcr.io/opencost/opencost:1.121.1@sha256:" + "5" * 64), + "ghcr.io/opencost/opencost") + + def test_an_untagged_reference_is_its_own_name(self): + self.assertEqual(gate.bare("otel/opentelemetry-collector-contrib"), + "otel/opentelemetry-collector-contrib") + + def test_a_registry_port_is_not_read_as_a_tag(self): + """Cutting there produces a key no advisory can match.""" + self.assertEqual(gate.bare("registry:5000/nanohype/agent"), + "registry:5000/nanohype/agent") + self.assertEqual(gate.bare("registry:5000/nanohype/agent:1.2.3"), + "registry:5000/nanohype/agent") + + +class AnUnacknowledgedFinding(unittest.TestCase): + """The question the gate is named for.""" + + def test_a_critical_with_no_entry_blocks(self): + problems = gate.verdict([finding()], []) + self.assertEqual(len(problems), 1) + self.assertIn("no entry in image-advisories.yaml names it", problems[0]) + self.assertIn("CVE-2026-33815", problems[0]) + self.assertIn("fixed in 5.9.0", problems[0]) + + def test_an_acknowledged_critical_passes(self): + self.assertEqual(gate.verdict([finding()], [advisory()]), []) + + def test_a_high_is_not_blocking(self): + """Counted and printed; gating it would gate on upstream's advisory rate.""" + self.assertEqual(gate.verdict([finding(severity="HIGH")], []), []) + + def test_an_entry_matching_only_the_cve_does_not_cover_another_package(self): + """One CVE id can be filed against several packages with different fixes.""" + problems = gate.verdict([finding(package="golang.org/x/crypto")], + [advisory(package="github.com/jackc/pgx/v5")]) + self.assertIn("no entry in image-advisories.yaml names it", problems[0]) + + def test_the_same_finding_reported_twice_is_one_problem(self): + """One image carries an advisory in the binary and in the layer around it.""" + problems = gate.verdict([finding(), finding()], []) + self.assertEqual(len(problems), 1) + + +class AnAcknowledgementCoversTheImagesItNames(unittest.TestCase): + """A new image acquiring a known CRITICAL is a decision, not an inheritance.""" + + def test_a_finding_on_an_unlisted_image_blocks(self): + """The listed image is covered; the one beside it on the same CVE is not.""" + problems = gate.verdict( + [finding(image="quay.io/cilium/cilium:v1.19.6"), + finding(image="quay.io/argoproj/argo-events:v1.9.11")], + [advisory(images=("quay.io/argoproj/argo-events",))]) + self.assertEqual(len(problems), 1) + self.assertIn("the entry does not list quay.io/cilium/cilium", problems[0]) + + def test_a_tag_bump_on_a_listed_image_still_passes(self): + self.assertEqual( + gate.verdict([finding(image="quay.io/argoproj/argo-events:v1.9.12")], + [advisory()]), []) + + +class AnAcknowledgementThatStoppedDescribingTheScan(unittest.TestCase): + """A list nobody re-checks only ever widens, and it widens permissively.""" + + def test_an_entry_no_image_carries_blocks(self): + problems = gate.verdict([], [advisory()]) + self.assertEqual(len(problems), 1) + self.assertIn("outlived its reason", problems[0]) + + def test_an_entry_listing_an_image_without_the_finding_blocks(self): + """The shape a chart bump leaves behind when it fixes one image of many.""" + problems = gate.verdict( + [finding(image="quay.io/argoproj/argo-events:v1.9.11")], + [advisory(images=("quay.io/argoproj/argo-events", "quay.io/cilium/cilium"))]) + self.assertEqual(len(problems), 1) + self.assertIn("lists quay.io/cilium/cilium, which no longer carries it", + problems[0]) + + def test_an_entry_with_no_images_acknowledges_nothing(self): + problems = gate.verdict([finding()], [advisory(images=())]) + joined = " ".join(problems) + self.assertIn("lists no images", joined) + + def test_an_entry_with_no_reason_blocks(self): + problems = gate.verdict([finding()], [advisory(reason=" ")]) + self.assertIn("acknowledged with no reason recorded", problems[0]) + + def test_a_high_only_finding_does_not_keep_a_critical_entry_alive(self): + """The entry is about the blocking severity; a HIGH match is not a match.""" + problems = gate.verdict([finding(severity="HIGH")], [advisory()]) + self.assertEqual(len(problems), 1) + self.assertIn("outlived its reason", problems[0]) + + +class TheShippedAdvisoryFile(unittest.TestCase): + """It is read on every run, so its shape is part of the gate.""" + + @classmethod + def setUpClass(cls): + doc = yaml.safe_load((ROOT / "image-advisories.yaml").read_text()) + cls.entries = doc["advisories"] + + def test_every_entry_carries_the_four_keys_the_verdict_reads(self): + for entry in self.entries: + with self.subTest(entry=entry.get("id")): + for key in ("id", "package", "reason", "images"): + self.assertIn(key, entry) + + def test_every_entry_records_a_reason(self): + for entry in self.entries: + with self.subTest(entry=entry.get("id")): + self.assertTrue(str(entry["reason"]).strip()) + + def test_every_listed_image_is_named_without_a_tag_or_digest(self): + """A tagged entry would stop matching at the next chart bump.""" + for entry in self.entries: + for image in entry["images"]: + with self.subTest(image=image): + self.assertEqual(gate.bare(image), image) + + def test_no_two_entries_share_an_id_and_package(self): + """The verdict keys on that pair, so a duplicate silently shadows one.""" + keys = [(e["id"], e["package"]) for e in self.entries] + self.assertEqual(len(keys), len(set(keys))) + + def test_the_file_acknowledges_something(self): + """An empty list makes every rule below hold over nothing.""" + self.assertTrue(self.entries) + + +class TheFloorsThatStopAVacuousPass(unittest.TestCase): + """A scan over almost nothing reports the same as a scan over a clean fleet.""" + + def test_a_floor_on_images_scanned_exists(self): + """Zero is the vacuous pass with a constant in front of it. + + Whether it sits below the catalog's real render is asserted against the + tree in test_corpus_floors.py, not against a number written here. + """ + self.assertGreater(gate.MIN_IMAGES, 0) + + def test_the_canary_is_pinned_by_digest(self): + """A tag can be repointed at a rebuilt, clean image, and then the canary + stops proving the scanner reports anything.""" + self.assertIn("@sha256:", gate.CANARY) + + def test_the_blocking_severity_is_among_the_ones_scanned_for(self): + self.assertIn(gate.BLOCKING_SEVERITY, gate.REPORTED_SEVERITIES) + + +if __name__ == "__main__": + unittest.main() From 3a75edc5b2a899ffa56dec3b2c00214e38b758df Mon Sep 17 00:00:00 2001 From: stxkxs Date: Wed, 2 Sep 2026 04:54:28 -0700 Subject: [PATCH 2/6] Take the image population from the pod specs, not from a pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every image the fleet renders is in the scanned set, and every chart that renders one contributes to it. The population came from the render, which was right, but the extraction from that render was a regex anchored on `image:` preceded only by whitespace. The ordinary Kubernetes list-item form, `- image: ` under `containers:`, never matched it. Against this catalog the pattern yielded 55 images where the pod specs hold 57, and the two it missed were the entire workload of two charts: quay.io/argoproj/argo-rollouts:v1.9.1 quay.io/argoproj/kubectl-argo-rollouts:v1.9.1 ghcr.io/stakater/reloader:v1.4.20 velero/velero-plugin-for-aws:v1.14.2 A CRITICAL in any of them passed the vulnerability gate clean, and the gate said "55 image(s) scanned across 26 chart(s)" while it did. A scanner that omits images silently is worse than none, because the green result is evidence. The walk is now structural: every container in every pod owner's podSpec — initContainers, containers, ephemeralContainers — plus every `image:` key anywhere, which is how a custom resource naming an image its operator runs stays in the population. 59 images across 28 charts. The pattern is kept as an independent floor under the walks rather than deleted. A parser and a regex fail on different inputs, so an image only the text scan finds is either a controller reading a string payload — declared in TEXT_ONLY_IMAGES with the reason, asserted in both directions — or a structural walk that stopped seeing a shape, which is reported: gateway-helm: docker.io/envoyproxy/ratelimit:1e50889b appears in the rendered text and in no pod template or `image:` key — either a structural walk stopped seeing a shape, or a controller reads it out of a payload and it belongs in TEXT_ONLY_IMAGES with the reason MIN_IMAGES is gone. A total cannot see the shape that happened: two charts left the inventory and the count stayed large enough to look healthy. The floor is now per chart and derived — every chart the render covers contributes at least one image, or is declared imageless with its reason and asserted both ways: b rendered and contributed no image. Every chart shipping a workload contributes at least one, so either the extraction stopped seeing a shape this chart uses, or the chart ships only CRDs and belongs in IMAGELESS_CHARTS with the reason. prometheus-operator-crds is declared imageless but the fleet no longer renders it — the entry outlived its chart. ai-gateway-crds-helm is declared imageless and now contributes an image. It ships a workload; delete the entry so its images are scanned like every other chart's. Three smaller repairs in the vulnerability gate. A `reason:` key written and left empty parsed as None and raised on `.strip()`, reaching a traceback instead of the sentence written for that case; a non-mapping entry under `advisories:` did the same. The `covered` set was accumulated on the acknowledged path and deleted unread, reading as a ledger the four rules consult and being none. And the header named one direction of the gate's time-dependence where there are two: a database that gains an advisory turns a passing tree red, and one that loses an advisory — or a publisher re-pushing a version tag on a patched base — turns a passing tree red the other way, through rules 3 and 4, where the clearing action is deleting an entry. MIN_RENDERED's upper bound is now held: EveryFloorIsBelowTheCorpusItGuards asserts it below the largest render the catalog can produce, so an always-red floor fails there rather than surviving. Co-authored-by: stxkxsbot <275011021+stxkxsbot@users.noreply.github.com> --- image-advisories.yaml | 2 + scripts/check-image-pins.py | 196 +++++++++++++++++- scripts/check-image-vulnerabilities.py | 64 ++++-- scripts/tests/run.py | 6 +- scripts/tests/test_corpus_floors.py | 49 ++++- scripts/tests/test_image_extraction.py | 215 ++++++++++++++++++++ scripts/tests/test_image_vulnerabilities.py | 12 +- 7 files changed, 505 insertions(+), 39 deletions(-) create mode 100644 scripts/tests/test_image_extraction.py diff --git a/image-advisories.yaml b/image-advisories.yaml index e995891..1192e15 100644 --- a/image-advisories.yaml +++ b/image-advisories.yaml @@ -62,7 +62,9 @@ advisories: - otel/opentelemetry-collector-contrib - prom/memcached-exporter - quay.io/argoproj/argo-events + - quay.io/argoproj/argo-rollouts - quay.io/argoproj/argocli + - quay.io/argoproj/kubectl-argo-rollouts - quay.io/argoproj/workflow-controller - quay.io/cilium/cilium - quay.io/jetstack/cert-manager-cainjector diff --git a/scripts/check-image-pins.py b/scripts/check-image-pins.py index d6f97d7..20493f2 100755 --- a/scripts/check-image-pins.py +++ b/scripts/check-image-pins.py @@ -51,6 +51,8 @@ import subprocess import sys +import yaml + # Shared precondition helper, loaded by path: these are hyphenated executables # run from varying working directories. _gl = pathlib.Path(__file__).resolve().parent / "gatelib.py" @@ -79,6 +81,15 @@ sys.modules["render_addons"] = render_addons _spec.loader.exec_module(render_addons) +# Helm charts occasionally emit the YAML `=` default-value sentinel (e.g. `- =` +# inside a ConfigMap payload). PyYAML's SafeLoader has no constructor for it and +# raises, which would drop a whole chart's render out of the walk below — so map +# the sentinel to its literal scalar rather than lose the chart. +yaml.SafeLoader.add_constructor( + "tag:yaml.org,2002:value", + lambda loader, node: loader.construct_scalar(node), # type: ignore[arg-type] +) + ROOT = render_addons.REPO_ROOT NETWORK_TIMEOUT = 300 @@ -126,11 +137,190 @@ def inventory(env: str) -> tuple[dict[str, set[str]], list[tuple[str, str]]]: if proc.returncode != 0: unscannable.append((u.path, (proc.stderr.strip() or proc.stdout.strip())[:200])) continue - for m in IMAGE.finditer(proc.stdout): - images.setdefault(m.group(1), set()).add(u.chart) + for ref_str in extract_images(proc.stdout, u.chart, unscannable): + images.setdefault(ref_str, set()).add(u.chart) return images, unscannable +# Kinds that own a pod template, and the path from the document to its podSpec. +# The deployable surface: every container the cluster starts comes from one of +# these, so this is the population any question about "what runs" reads. +POD_OWNERS = { + "Pod": ("spec",), + "Deployment": ("spec", "template", "spec"), + "StatefulSet": ("spec", "template", "spec"), + "DaemonSet": ("spec", "template", "spec"), + "ReplicaSet": ("spec", "template", "spec"), + "ReplicationController": ("spec", "template", "spec"), + "Job": ("spec", "template", "spec"), + "CronJob": ("spec", "jobTemplate", "spec", "template", "spec"), +} + +CONTAINER_LISTS = ("initContainers", "containers", "ephemeralContainers") + +# Images that reach a cluster without ever appearing as a YAML `image:` key, +# because a controller reads them out of a string payload it is handed. Neither +# structural walk can see one, so the text scan is what finds them — and an entry +# here is asserted in both directions below: one a structural walk DOES find is +# stale, and an image only the text scan finds that is not listed means a walk +# stopped seeing a shape. +TEXT_ONLY_IMAGES = { + "docker.io/envoyproxy/ratelimit": "the Envoy Gateway controller reads the rate-limit " + "image out of its own EnvoyGateway ConfigMap, so it " + "is a string inside a config blob rather than a " + "container in a pod template", +} + + +# Charts that render no container image because they ship only CustomResource +# definitions. Asserted in both directions by chart_coverage below: one that +# starts shipping a workload fails here, and one that is named and no longer +# rendered fails too. +IMAGELESS_CHARTS = { + "ai-gateway-crds-helm": "the Envoy AI Gateway CRDs, applied ahead of the " + "controller that renders one of their kinds", + "prometheus-operator-crds": "the Prometheus operator CRDs, applied ahead of " + "the charts that render ServiceMonitors", +} + + +def chart_coverage(images: dict[str, set[str]], units) -> list[str]: + """Every chart the render covered contributed an image, or is declared not to. + + A per-chart floor rather than a total, because a total cannot see the shape + that actually happened: an extractor missed the ordinary `- image:` list-item + spelling, two charts' entire workloads fell out of the inventory, and the + count stayed large enough to look healthy. One image per chart is derived + from the corpus — no constant to pick, and it moves with the catalog. + """ + problems: list[str] = [] + rendered = {u.chart for u in units if u.chart not in getattr( + render_addons, "SKIP_CHARTS", {})} + contributing = set().union(*images.values()) if images else set() + + for chart in sorted(rendered - contributing): + if chart in IMAGELESS_CHARTS: + continue + problems.append( + f"{chart} rendered and contributed no image. Every chart shipping a " + f"workload contributes at least one, so either the extraction stopped " + f"seeing a shape this chart uses, or the chart ships only CRDs and " + f"belongs in IMAGELESS_CHARTS with the reason.") + + for chart, reason in sorted(IMAGELESS_CHARTS.items()): + if chart not in rendered: + problems.append( + f"{chart} is declared imageless but the fleet no longer renders it — " + f"the entry outlived its chart. (recorded: {reason})") + elif chart in contributing: + problems.append( + f"{chart} is declared imageless and now contributes an image. It " + f"ships a workload; delete the entry so its images are scanned like " + f"every other chart's. (recorded: {reason})") + return problems + + +def bare_name(ref: str) -> str: + """A reference with tag and digest removed, which is what an entry names. + + The tag separator is the last colon in the FINAL path segment: a registry + with a port carries a colon that is not one, and cutting there produces a key + no entry can match and no reader can recognise. + """ + ref = ref.split("@", 1)[0] + name = ref.rsplit("/", 1)[-1] + return ref.rsplit(":", 1)[0] if ":" in name else ref + + +def _podspec(doc: dict, path: tuple[str, ...]) -> dict | None: + cur: object = doc + for key in path: + if not isinstance(cur, dict): + return None + cur = cur.get(key) + return cur if isinstance(cur, dict) else None + + +def podspec_images(docs: list) -> set[str]: + """Every container image the rendered documents start, walked structurally.""" + found: set[str] = set() + for doc in docs: + if not isinstance(doc, dict): + continue + kind = doc.get("kind") + if not isinstance(kind, str): + continue + path = POD_OWNERS.get(kind) + if path is None: + continue + spec = _podspec(doc, path) + if spec is None: + continue + for key in CONTAINER_LISTS: + for container in spec.get(key) or []: + if isinstance(container, dict) and isinstance(container.get("image"), str): + found.add(container["image"]) + return found + + +def keyed_images(node) -> set[str]: + """Every `image:` key anywhere in the documents, whatever declares it. + + A custom resource can name an image its operator then runs — the agent + platform's eval-runner is one — and no pod template in this render carries + it. Restricting the walk to pod owners would drop it. + """ + found: set[str] = set() + if isinstance(node, dict): + for key, value in node.items(): + if key == "image" and isinstance(value, str): + found.add(value) + else: + found |= keyed_images(value) + elif isinstance(node, list): + for value in node: + found |= keyed_images(value) + return found + + +def extract_images(rendered: str, chart: str, + unscannable: list[tuple[str, str]]) -> set[str]: + """Every image one chart's render deploys, and an assertion that it is every one. + + Structural first, because a pattern is a claim about spelling. IMAGE is + anchored on `image:` preceded only by whitespace, so the ordinary list-item + form `- image: ` never matched it: against this catalog's render the + pattern yielded 55 images where the pod specs hold 57, and the four it missed + were the whole workload of two charts. A scanner that omits images silently + is worse than none, because its green result becomes evidence. + + The text scan is kept as an independent floor under the walks rather than + replaced. A parser and a pattern fail on different inputs, so a structural + walk that stops seeing a shape is caught by the one that reads the bytes — + and an image ONLY the pattern finds is either a string payload a controller + reads (declared in TEXT_ONLY_IMAGES) or a walk that has drifted. + """ + try: + docs = list(yaml.safe_load_all(rendered)) + except yaml.YAMLError as exc: + first = str(exc).strip().splitlines()[0] + unscannable.append((chart, f"rendered YAML this gate could not parse — {first}")) + return set() + + structural = podspec_images(docs) | keyed_images(docs) + textual = {m.group(1) for m in IMAGE.finditer(rendered)} + + for ref_str in sorted(textual - structural): + if bare_name(ref_str) not in TEXT_ONLY_IMAGES: + unscannable.append(( + chart, + f"{ref_str} appears in the rendered text and in no pod template or " + f"`image:` key — either a structural walk stopped seeing a shape, or " + f"a controller reads it out of a payload and it belongs in " + f"TEXT_ONLY_IMAGES with the reason")) + return structural | textual + + def classify(ref: str) -> str: """'digest', 'tag', or 'mutable'.""" if "@sha256:" in ref: @@ -165,7 +355,7 @@ def main() -> int: print(f" {path}: {err}") return 2 - failures = [] + failures = chart_coverage(images, render_addons.discover()) mutable_seen: set[str] = set() for ref in sorted(images): diff --git a/scripts/check-image-vulnerabilities.py b/scripts/check-image-vulnerabilities.py index 5467eff..e75aad8 100755 --- a/scripts/check-image-vulnerabilities.py +++ b/scripts/check-image-vulnerabilities.py @@ -24,11 +24,24 @@ changed. That leaves a real objection, and the answer to it is the advisory file rather -than a softer bar. Whether a CVE landed overnight is not a fact about the commit, -so a new CRITICAL can turn a pull request red for a reason the pull request did -not cause. What it cannot do is turn it red silently or permanently: the failure -names the image, the CVE and the fixed version, and clears by bumping the chart -or by recording why the finding stands. Both are decisions with an author. +than a softer bar. What this gate reads is not a function of the commit alone: it +is the commit against a vulnerability database and against whatever the pinned +tags resolve to today. Both move, and they move in both directions. + +A database that GAINS an advisory turns a tree red that passed yesterday. The +failure names the image, the CVE and the fixed version, and clears by bumping the +chart or by recording why the finding stands. + +A database that LOSES one — a reclassification, a withdrawn advisory — turns a +tree red that passed for the opposite reason: rules 3 and 4 fire, the entry +acknowledging it no longer matches any scanned image, and the clearing action is +deleting an entry. The same happens with no database change at all, because this +fleet is pinned by version tag and a tag is not immutable (see +check-image-pins.py on what counts as pinned): a publisher re-pushing the same +tag on a patched base removes the finding underneath a standing entry. + +Neither direction is silent and neither is permanent, and both are decisions with +an author. That is the whole of what the advisory file buys. THE ADVISORY FILE IS ASSERTED IN FOUR DIRECTIONS @@ -104,11 +117,12 @@ BLOCKING_SEVERITY = "CRITICAL" REPORTED_SEVERITIES = ["CRITICAL", "HIGH"] -# A floor on IMAGES SCANNED, not on findings. Printing a denominator is not -# gating on it: with no floor, a render that produced two images would report the -# fleet clean. Set well under what the catalog renders, so it catches "scanned -# almost nothing" rather than "a chart was removed". -MIN_IMAGES = 40 +# The floor on images scanned is DERIVED, not picked: one image per chart the +# render covers, asserted per chart by check-image-pins.chart_coverage. A total +# cannot see the shape that actually happened — an extractor missed the ordinary +# `- image:` list-item spelling, two charts' whole workloads left the inventory, +# and the count stayed large enough to look healthy. The per-chart form catches +# exactly that, and it moves with the catalog rather than with a constant. # A digest-pinned image with historical CRITICALs that have published fixes. # Alpine 3.10 is end-of-life, so these cannot be patched away underneath the @@ -205,8 +219,18 @@ def verdict(findings: list[Finding], advisories: list[dict]) -> list[str]: by_key: dict[tuple[str, str], dict] = {} for entry in advisories: + if not isinstance(entry, dict): + problems.append( + f"image-advisories.yaml: an entry under `advisories:` is a " + f"{type(entry).__name__}, not a mapping — nothing can be read from it, " + f"and the findings it was meant to acknowledge are unacknowledged.") + continue key = (str(entry.get("id", "")), str(entry.get("package", ""))) - if not entry.get("reason", "").strip(): + # str() around the read, not just the strip: a `reason:` key written and + # left empty parses as None, and `None.strip()` raises — reaching a + # traceback instead of the sentence two lines below, which is written for + # exactly this case. id and package were already read defensively. + if not str(entry.get("reason") or "").strip(): problems.append( f"image-advisories.yaml: {key[0]} in {key[1]} is acknowledged with no " f"reason recorded, so nothing states what would let it be removed.") @@ -214,7 +238,6 @@ def verdict(findings: list[Finding], advisories: list[dict]) -> list[str]: # 1. A blocking finding nothing acknowledges — the gate's own question. # 2. A blocking finding on an image its entry does not name. - covered: set[tuple[str, str, str]] = set() for f in blocking: key = (f.id, f.package) acknowledged = by_key.get(key) @@ -232,7 +255,6 @@ def verdict(findings: list[Finding], advisories: list[dict]) -> list[str]: f"not list {f.bare}. An acknowledgement covers the images it names — a " f"new image acquiring a known CRITICAL is a decision, not an inheritance.") continue - covered.add((f.id, f.package, f.bare)) # 3. An entry no image carries. 4. An entry listing an image with no such finding. carried = {(f.id, f.package, f.bare) for f in blocking} @@ -254,7 +276,6 @@ def verdict(findings: list[Finding], advisories: list[dict]) -> list[str]: problems.append( f"image-advisories.yaml: {key[0]} in {key[1]} lists {img}, which no " f"longer carries it. Drop that image from the entry.") - del covered return list(dict.fromkeys(problems)) @@ -302,10 +323,17 @@ def main() -> int: "the scan below would cover part of the fleet and report on all of " "it.", *(f" {path}: {err}" for path, err in unrendered)) - if len(images) < MIN_IMAGES: - cannot_run(f"Cannot run: the render produced {len(images)} image(s), under the " - f"floor of {MIN_IMAGES}. A scan over almost nothing reports the same " - f"as a scan over a clean fleet.") + units = image_pins.render_addons.discover() + coverage = image_pins.chart_coverage(images, units) + if coverage: + cannot_run("Cannot run: the image population is smaller than the charts that " + "rendered it, so a scan over it says less than it appears to:", + *(f" {problem}" for problem in coverage)) + if len(images) < len(units): + cannot_run(f"Cannot run: the render produced {len(images)} image(s) from " + f"{len(units)} chart(s). Every chart shipping a workload " + f"contributes at least one, so the walk got smaller rather than " + f"the catalog.") findings: list[Finding] = [] unscannable: list[str] = [] diff --git a/scripts/tests/run.py b/scripts/tests/run.py index b77bc0c..57bf7aa 100755 --- a/scripts/tests/run.py +++ b/scripts/tests/run.py @@ -37,6 +37,7 @@ "test_falco_rule_floor", "test_gatelib", "test_image_vulnerabilities", + "test_image_extraction", "test_corpus_floors", ) @@ -78,7 +79,7 @@ # largest in the tree, and among them are the gates on the paths testing-rubric # calls security-critical. So this figure being low is not offset by behavioural # coverage for precisely the files where that offset was being claimed. -COMBINED_FLOOR = 12 +COMBINED_FLOOR = 20 # A ceiling on gate scripts carrying NO unit coverage at all, complementing the # floors below. The floors stop a covered file regressing; nothing stopped a NEW @@ -105,7 +106,8 @@ "scripts/check-label-values.py": 33, "scripts/check-sync-waves.py": 10, "scripts/gatelib.py": 55, - "scripts/check-image-vulnerabilities.py": 45, + "scripts/check-image-vulnerabilities.py": 41, + "scripts/check-image-pins.py": 47, } diff --git a/scripts/tests/test_corpus_floors.py b/scripts/tests/test_corpus_floors.py index 8c4e1ae..1a9f123 100644 --- a/scripts/tests/test_corpus_floors.py +++ b/scripts/tests/test_corpus_floors.py @@ -26,9 +26,11 @@ ROOT = pathlib.Path(__file__).resolve().parent.parent.parent +render_addons = load("render-addons") fork_safety = load("check-hardcoded-org") platform_crs = load("check-platform-crs") policy_admission = load("check-policy-admission") +image_pins = load("check-image-pins") image_vulns = load("check-image-vulnerabilities") @@ -40,7 +42,6 @@ def test_each_gate_declares_one(self): ("check-hardcoded-org MIN_APPSETS", fork_safety.MIN_APPSETS), ("check-platform-crs MIN_CRS", platform_crs.MIN_CRS), ("check-policy-admission MIN_RENDERED", policy_admission.MIN_RENDERED), - ("check-image-vulnerabilities MIN_IMAGES", image_vulns.MIN_IMAGES), ): with self.subTest(floor=label): self.assertGreater(floor, 0) @@ -73,14 +74,38 @@ def test_the_platform_crs_clear_their_floor(self): "tree, so check-platform-crs.py cannot pass") def test_the_discovered_addons_clear_the_policy_admission_floor(self): - """One unit renders at least one manifest, so units bound the render below.""" + """The render is one manifest per unit per environment it reaches, so the + unit count times the environments bounds it above — and a floor above + THAT is one no render can clear.""" units = policy_admission.discover() self.assertGreater(len(units), 0) - self.assertGreaterEqual( - policy_admission.MIN_RENDERED, len(units), - "MIN_RENDERED sits below the number of addon units discovered, so it " - "would pass a render that produced one manifest per unit and nothing " - "for the environments — which is the shape it exists to catch") + reachable = len(units) * len(render_addons.ENVIRONMENTS) + self.assertLess( + policy_admission.MIN_RENDERED, reachable, + "MIN_RENDERED is at or above the largest render this catalog can " + "produce, so check-policy-admission.py is red on every run and the " + "gate gets routed around") + + def test_the_policy_admission_floor_exceeds_a_degenerate_render(self): + """A different property, and the reason the floor is not merely > 0: a + render producing one manifest per unit and nothing per environment is + the shape the floor exists to catch, so it must sit above that.""" + units = policy_admission.discover() + self.assertGreaterEqual(policy_admission.MIN_RENDERED, len(units)) + + def test_the_imageless_charts_are_charts_the_fleet_renders(self): + """The image floor is per chart, so its exemption is where it can rot. + + A chart declared imageless that the catalog no longer pins is an entry + excusing nothing, and the per-chart floor is exactly as strong as that + list is short. + """ + charts = {u.chart for u in render_addons.discover()} + for chart in image_pins.IMAGELESS_CHARTS: + with self.subTest(chart=chart): + self.assertIn(chart, charts, + f"{chart} is declared to render no image but the fleet " + f"does not pin it — the entry outlived its chart") class TheFloorsGuardTheRightQuantity(unittest.TestCase): @@ -102,9 +127,15 @@ def test_the_policy_admission_floor_is_read_off_the_render_count(self): source = (ROOT / "scripts" / "check-policy-admission.py").read_text() self.assertIn("if count < MIN_RENDERED:", source) - def test_the_image_floor_is_read_off_the_inventory(self): + def test_the_image_floor_is_derived_per_chart(self): + """Not a constant: every chart the render covers must contribute an image, + which is the shape a total cannot see.""" source = (ROOT / "scripts" / "check-image-vulnerabilities.py").read_text() - self.assertIn("if len(images) < MIN_IMAGES:", source) + self.assertIn("coverage = image_pins.chart_coverage(images, units)", source) + self.assertIn("if len(images) < len(units):", source) + self.assertNotIn("MIN_IMAGES", source, + "the picked constant is back; the per-chart derivation is " + "what catches an extractor that stopped seeing a shape") if __name__ == "__main__": diff --git a/scripts/tests/test_image_extraction.py b/scripts/tests/test_image_extraction.py new file mode 100644 index 0000000..2043d35 --- /dev/null +++ b/scripts/tests/test_image_extraction.py @@ -0,0 +1,215 @@ +"""Unit tests for how the fleet's image population is derived from a render. + +Both gates that ask anything about images read this one inventory, so an +extractor that misses a spelling removes those images from every question at +once — and the count that remains stays large enough to look healthy. That +happened: the pattern was anchored on `image:` preceded only by whitespace, the +ordinary list-item form `- image: ` never matched, and two charts' +entire workloads were absent from a scan reporting fifty-five images. + +So the walk is structural and the pattern is kept under it as an independent +floor. A parser and a regex fail on different inputs; each is planted against +here, and so is the per-chart floor that catches a chart contributing nothing. +""" + +from __future__ import annotations + +import unittest + +from gateloader import load + +gate = load("check-image-pins") + + +class Unit: + """The fields chart_coverage reads off a render unit.""" + + def __init__(self, chart: str): + self.chart = chart + + +def render(*docs: str) -> str: + return "\n---\n".join(docs) + + +DEPLOYMENT = """ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: app +spec: + template: + spec: + containers: + - name: app + image: ghcr.io/example/app:1.0.0 +""" + + +class ExtractingFromARender(unittest.TestCase): + def images(self, rendered, chart="c"): + unscannable: list[tuple[str, str]] = [] + return gate.extract_images(rendered, chart, unscannable), unscannable + + def test_the_list_item_spelling_is_found(self): + """The shape the pattern never matched, and the whole defect. + + `- image:` under a `containers:` list is the ordinary Kubernetes form. + Anchored on whitespace-then-`image:`, a pattern sees nothing here. + """ + found, problems = self.images(DEPLOYMENT) + self.assertEqual(found, {"ghcr.io/example/app:1.0.0"}) + self.assertEqual(problems, []) + + def test_every_container_list_is_walked(self): + rendered = """ +apiVersion: apps/v1 +kind: DaemonSet +spec: + template: + spec: + initContainers: + - name: init + image: init:1 + containers: + - name: main + image: main:2 + ephemeralContainers: + - name: debug + image: debug:3 +""" + found, _ = self.images(rendered) + self.assertEqual(found, {"init:1", "main:2", "debug:3"}) + + def test_a_cronjob_pod_template_is_reached(self): + """Its podSpec sits two levels deeper than every other owner's.""" + rendered = """ +apiVersion: batch/v1 +kind: CronJob +spec: + jobTemplate: + spec: + template: + spec: + containers: + - name: rotate + image: aws-cli:2 +""" + found, _ = self.images(rendered) + self.assertEqual(found, {"aws-cli:2"}) + + def test_every_pod_owner_kind_is_walked(self): + for kind in sorted(gate.POD_OWNERS): + with self.subTest(kind=kind): + path = gate.POD_OWNERS[kind] + spec: dict = {"containers": [{"name": "c", "image": "x:1"}]} + for key in reversed(path[1:]): + spec = {key: spec} + import yaml + doc = yaml.safe_dump({"apiVersion": "v1", "kind": kind, "spec": spec}) + found, _ = self.images(doc) + self.assertEqual(found, {"x:1"}) + + def test_an_image_key_outside_a_pod_template_is_found(self): + """A custom resource can name an image its operator then runs.""" + rendered = """ +apiVersion: agents.nanohype.dev/v1alpha1 +kind: AgentFleet +spec: + agents: + - name: a + image: ghcr.io/example/runner:0.1.0 +""" + found, _ = self.images(rendered) + self.assertEqual(found, {"ghcr.io/example/runner:0.1.0"}) + + def test_an_image_only_the_text_scan_finds_is_reported(self): + """The extraction-drift assertion: a structural walk that stopped seeing + a shape looks identical to a controller reading a string payload, so the + second must be declared and the first is reported.""" + rendered = """ +apiVersion: v1 +kind: ConfigMap +data: + envoy-gateway.yaml: | + rateLimitDeployment: + container: + image: docker.io/example/undeclared:1.0 +""" + found, problems = self.images(rendered) + self.assertIn("docker.io/example/undeclared:1.0", found) + self.assertEqual(len(problems), 1) + self.assertIn("stopped seeing a shape", problems[0][1]) + + def test_a_declared_text_only_image_is_not_reported(self): + declared = next(iter(gate.TEXT_ONLY_IMAGES)) + rendered = f""" +apiVersion: v1 +kind: ConfigMap +data: + cfg: | + image: {declared}:abc123 +""" + found, problems = self.images(rendered) + self.assertIn(f"{declared}:abc123", found) + self.assertEqual(problems, []) + + def test_a_render_that_will_not_parse_is_reported_not_dropped(self): + """A chart silently absent from the inventory is the whole failure mode.""" + found, problems = self.images("kind: Deployment\n bad: [unclosed\n") + self.assertEqual(found, set()) + self.assertEqual(len(problems), 1) + self.assertIn("could not parse", problems[0][1]) + + def test_the_helm_value_sentinel_does_not_lose_the_chart(self): + """Charts emit `- =` inside ConfigMap payloads; PyYAML has no constructor + for it, and raising there would drop the whole render.""" + rendered = DEPLOYMENT + "\n---\napiVersion: v1\nkind: ConfigMap\ndata:\n a: |\n - =\n" + found, problems = self.images(rendered) + self.assertEqual(found, {"ghcr.io/example/app:1.0.0"}) + self.assertEqual(problems, []) + + +class EveryChartContributesAnImage(unittest.TestCase): + """A per-chart floor, because a total cannot see two charts falling out.""" + + def coverage(self, images, charts): + """Every declared imageless chart is rendered unless a case says otherwise, + so a fixture does not trip the exemption's own rot rule by omission.""" + units = [Unit(c) for c in charts] + [ + Unit(c) for c in gate.IMAGELESS_CHARTS if c not in charts] + return gate.chart_coverage(images, units) + + def test_a_chart_contributing_nothing_is_reported(self): + problems = self.coverage({"x:1": {"a"}}, ["a", "b"]) + self.assertEqual(len(problems), 1) + self.assertIn("b rendered and contributed no image", problems[0]) + + def test_every_chart_contributing_passes(self): + self.assertEqual( + self.coverage({"x:1": {"a"}, "y:2": {"b"}}, ["a", "b"]), []) + + def test_a_declared_imageless_chart_is_not_reported(self): + declared = next(iter(gate.IMAGELESS_CHARTS)) + self.assertEqual(self.coverage({"x:1": {"a"}}, ["a", declared]), []) + + def test_a_declared_chart_the_fleet_stopped_rendering_is_reported(self): + """The exemption is where a per-chart floor rots: an entry excusing a + chart nobody pins excuses nothing and reads as considered.""" + problems = gate.chart_coverage({"x:1": {"a"}}, [Unit("a")]) + self.assertEqual(len(problems), len(gate.IMAGELESS_CHARTS)) + self.assertIn("outlived its chart", problems[0]) + + def test_a_declared_chart_that_starts_shipping_a_workload_is_reported(self): + declared = next(iter(gate.IMAGELESS_CHARTS)) + problems = self.coverage({"x:1": {declared}}, [declared]) + self.assertTrue(any("now contributes an image" in p for p in problems)) + + def test_an_empty_inventory_reports_every_chart(self): + """The vacuous case: no images at all is not a fleet with no images.""" + problems = self.coverage({}, ["a", "b"]) + self.assertEqual(len(problems), 2) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/tests/test_image_vulnerabilities.py b/scripts/tests/test_image_vulnerabilities.py index 071b9da..25bd9da 100644 --- a/scripts/tests/test_image_vulnerabilities.py +++ b/scripts/tests/test_image_vulnerabilities.py @@ -186,13 +186,11 @@ def test_the_file_acknowledges_something(self): class TheFloorsThatStopAVacuousPass(unittest.TestCase): """A scan over almost nothing reports the same as a scan over a clean fleet.""" - def test_a_floor_on_images_scanned_exists(self): - """Zero is the vacuous pass with a constant in front of it. - - Whether it sits below the catalog's real render is asserted against the - tree in test_corpus_floors.py, not against a number written here. - """ - self.assertGreater(gate.MIN_IMAGES, 0) + def test_the_floor_on_images_scanned_is_derived_per_chart(self): + """A constant can be set above the render and make the gate always red, + or below it and let a shrunken walk through. One image per chart the + render covers is neither: it moves with the catalog.""" + self.assertFalse(hasattr(gate, "MIN_IMAGES")) def test_the_canary_is_pinned_by_digest(self): """A tag can be repointed at a rebuilt, clean image, and then the canary From a9b256c1bf10d6cf63780639b3584610894ddc53 Mon Sep 17 00:00:00 2001 From: stxkxs Date: Wed, 2 Sep 2026 11:57:18 -0700 Subject: [PATCH 3/6] Take in the images a controller starts, not only the containers a template declares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every image-shaped reference the production render carries is classified: it is a container the render declares, an image a named controller starts, or something declared not to be a container. Nothing is silently outside the population. A pod template is not the deployable surface. An operator is handed a reference — in a flag, a ConfigMap value, a CR field — and creates the pod afterwards, so the container never appears in what `helm template` prints. Eleven such images render here and none was scanned: docker.io/envoyproxy/ai-gateway-extproc quay.io/argoproj/argoexec ghcr.io/aquasecurity/node-collector quay.io/jetstack/cert-manager-acmesolver ghcr.io/kyverno/kyverno natsio/nats-server-config-reloader natsio/prometheus-nats-exporter docker.io/envoyproxy/ratelimit The population is 70 images across 28 charts, from 59. The fixed CRITICALs it carries go from 52 to 86, and the newly visible ones include CVE-2026-22039 in kyverno itself — CRITICAL, fixed in 1.15.3 and 1.16.3, on a reference the chart carries as `:latest`. Two mutable tags surfaced with them, `ghcr.io/kyverno/kyverno` and `natsio/prometheus-nats-exporter`, both now on ALLOWED_MUTABLE with what clears them. Whether a string is an image a controller starts is a fact about that controller's behaviour, and this gate reads manifests. So it is declared rather than inferred, and the declaration is what carries the assertion: every image-shaped string must be in the render's own `image:` keys, on CONTROLLER_IMAGES with the controller that starts it, or on NOT_A_CONTAINER if pulling it runs nothing. Anything else is reported. `declaration_rot` closes the other direction for both tables — the reverse assertion the old comment claimed and did not have. The grammar requires a path separator, which is what separates an image from the addresses that fill a rendered config: `loki.monitoring.svc.cluster.local:3100` and `127.0.0.1:8080` are not images, and a grammar admitting them would demand a declaration for every endpoint in the fleet. The pod-owner walk is deleted. POD_OWNERS, CONTAINER_LISTS, _podspec and podspec_images were a strict subset of the `image:` key walk — every container's image is an `image:` key — so removing them changed no result, which is why no test failed when they went. Mechanism with no effect reads as mechanism. Four smaller repairs. The MIN_CRS floor test matched any `/v1alpha1` apiVersion, counting 44 documents — every ApplicationSet and Kyverno fixture in the tree — against the 8 the gate walks, so it held for any floor up to 43 and could not see the always-red case it was added to see; it now filters on the operator's API groups. The MIN_RENDERED bound multiplied by four environments where the gate renders two, leaving 66..127 passing while making the gate red on every run. Rules 3 and 4 iterated the unfiltered advisory list and reached a traceback on a non-mapping entry. And chart_coverage is keyed by chart NAME, which folds this catalog's three opentelemetry-collector units into one — said in the code rather than left to be discovered. The header no longer claims every image the fleet renders. It scans the production render; the other three environments are not asked, nothing asserts they agree, and a component enabled only in development or on the hub is deployed and unscanned. That is the narrower claim this gate holds. Co-authored-by: stxkxsbot <275011021+stxkxsbot@users.noreply.github.com> --- image-advisories.yaml | 96 ++++++++++ scripts/check-image-pins.py | 237 +++++++++++++++++-------- scripts/check-image-vulnerabilities.py | 23 ++- scripts/tests/test_corpus_floors.py | 40 ++++- scripts/tests/test_image_extraction.py | 111 +++++++++--- 5 files changed, 399 insertions(+), 108 deletions(-) diff --git a/image-advisories.yaml b/image-advisories.yaml index 1192e15..2ffa64d 100644 --- a/image-advisories.yaml +++ b/image-advisories.yaml @@ -46,6 +46,7 @@ advisories: image as chart pins move to versions built against 0.55.0 or later. images: - docker.io/envoyproxy/ai-gateway-controller + - docker.io/envoyproxy/ai-gateway-extproc - docker.io/envoyproxy/gateway - docker.io/falcosecurity/falcoctl - docker.io/grafana/loki @@ -59,11 +60,13 @@ advisories: - ghcr.io/kedacore/keda-metrics-apiserver - ghcr.io/opencost/opencost - mirror.gcr.io/aquasec/trivy-operator + - natsio/prometheus-nats-exporter - otel/opentelemetry-collector-contrib - prom/memcached-exporter - quay.io/argoproj/argo-events - quay.io/argoproj/argo-rollouts - quay.io/argoproj/argocli + - quay.io/argoproj/argoexec - quay.io/argoproj/kubectl-argo-rollouts - quay.io/argoproj/workflow-controller - quay.io/cilium/cilium @@ -90,6 +93,9 @@ advisories: toolchain. Fixed in Go 1.24.13, 1.25.7 and 1.26.0-rc.3. images: - docker.io/falcosecurity/k8s-metacollector + - ghcr.io/aquasecurity/node-collector + - natsio/nats-server-config-reloader + - natsio/prometheus-nats-exporter - quay.io/argoproj/argo-events - registry.k8s.io/metrics-server/metrics-server - us-docker.pkg.dev/fairwinds-ops/oss/goldilocks @@ -102,6 +108,8 @@ advisories: 1.21.11 and 1.22.4. The falco chart pin is what moves it. images: - docker.io/falcosecurity/k8s-metacollector + - natsio/nats-server-config-reloader + - natsio/prometheus-nats-exporter - id: CVE-2026-33186 package: google.golang.org/grpc @@ -137,6 +145,7 @@ advisories: OpenSSL in the goldilocks image's Alpine base, fixed in 3.5.6-r0. A base layer this catalog does not build; it moves with the goldilocks chart pin. images: + - ghcr.io/aquasecurity/node-collector - us-docker.pkg.dev/fairwinds-ops/oss/goldilocks - id: CVE-2026-31789 @@ -145,6 +154,7 @@ advisories: The same OpenSSL advisory against the sibling package in the same base layer, cleared by the same chart bump as the libcrypto3 entry. images: + - ghcr.io/aquasecurity/node-collector - us-docker.pkg.dev/fairwinds-ops/oss/goldilocks - id: CVE-2026-59873 @@ -156,3 +166,89 @@ advisories: repository can take on its own. images: - ghcr.io/nanohype/eks-agent-platform/eval-runner + + # ── Images a controller starts ──────────────────────────────────────── + # + # Every entry below was invisible until the population took in the images a + # controller is handed rather than only the containers a pod template declares. + # None of them appears in any pod spec this catalog renders; each is a + # reference an operator reads and then creates a pod from. + + - id: CVE-2026-22039 + package: github.com/kyverno/kyverno + reason: >- + A CRITICAL in the policy engine itself, on the CLI image the kyverno chart + carries in a ConfigMap — the same reference ALLOWED_MUTABLE records as + untagged. Fixed in 1.15.3 and 1.16.3; the catalog pins chart 3.8.2, whose + appVersion is 1.18.2, so the ConfigMap's `:latest` is what carries the + finding rather than the running controllers. Clears on a chart that pins + that reference. + images: + - ghcr.io/kyverno/kyverno + + - id: CVE-2025-30215 + package: github.com/nats-io/nats-server/v2 + reason: >- + The NATS server compiled into the exporter image the argo-events eventbus + controller starts. Fixed in 2.11.1 and 2.10.27; the argo-events chart + leaves this image's tag unset, so it moves only when the chart does. + images: + - natsio/prometheus-nats-exporter + + - id: CVE-2022-48174 + package: busybox + reason: >- + The Alpine base of the NATS config-reloader sidecar the eventbus controller + creates. Fixed in 1.36.1-r1; this catalog does not build the image and the + argo-events chart pins the tag. + images: + - natsio/nats-server-config-reloader + + - id: CVE-2022-48174 + package: busybox-binsh + reason: >- + The same advisory against the sibling package in the same base layer, + cleared by the same chart bump. + images: + - natsio/nats-server-config-reloader + + - id: CVE-2022-48174 + package: ssl_client + reason: >- + The third package of the same busybox advisory in the same base layer. + images: + - natsio/nats-server-config-reloader + + - id: CVE-2022-37434 + package: zlib + reason: >- + zlib in the same NATS config-reloader base layer, fixed in 1.2.12-r2. The + image is pinned old by the argo-events chart; nothing here rebuilds it. + images: + - natsio/nats-server-config-reloader + + - id: CVE-2022-23806 + package: stdlib + reason: >- + The Go standard library compiled into the NATS exporter, fixed in 1.16.14 + and 1.17.7. Moves when the argo-events chart ships an image built on a + patched toolchain. + images: + - natsio/prometheus-nats-exporter + + - id: CVE-2023-24538 + package: stdlib + reason: >- + The same shape against both NATS images the eventbus controller starts, + fixed in Go 1.19.8 and 1.20.3. + images: + - natsio/nats-server-config-reloader + - natsio/prometheus-nats-exporter + + - id: CVE-2023-24540 + package: stdlib + reason: >- + The same two images and the same remedy, fixed in Go 1.19.9 and 1.20.4. + images: + - natsio/nats-server-config-reloader + - natsio/prometheus-nats-exporter diff --git a/scripts/check-image-pins.py b/scripts/check-image-pins.py index 20493f2..275f92b 100755 --- a/scripts/check-image-pins.py +++ b/scripts/check-image-pins.py @@ -102,16 +102,34 @@ # Exemptions, asserted against the real render. An entry naming an image the # fleet no longer renders mutably FAILS: an exemption that matches nothing is a # description that rots, and it rots toward permissive. -ALLOWED_MUTABLE: dict[str, str] = {} +ALLOWED_MUTABLE: dict[str, str] = { + "ghcr.io/kyverno/kyverno": + "the kyverno chart carries this in a ConfigMap as the CLI image for " + "`kyverno` invocations, with no values key to pin it. Invisible until " + "controller-supplied references entered the population. Clears when the " + "chart exposes the tag, or when this catalog stops shipping that " + "ConfigMap.", + "natsio/prometheus-nats-exporter": + "an argo-events eventbus default the controller passes to the NATS " + "StatefulSet it creates. The chart pins two other tags of this image and " + "leaves the exporter's unset. Clears on a chart version that pins it.", +} + +def inventory(env: str, seen: set[str] | None = None + ) -> tuple[dict[str, set[str]], list[tuple[str, str]]]: + """(image -> charts that render it, [(path, why-not-scanned)]). -def inventory(env: str) -> tuple[dict[str, set[str]], list[tuple[str, str]]]: - """(image -> charts that render it, [(path, why-not-scanned)]).""" + `seen`, if given, collects every image-shaped reference the render carried — + including the ones excluded from the population — so a declaration can be + checked against what the render contains rather than against what survived. + """ gatelib.require('helm') units = render_addons.discover() aliases = render_addons.add_repos(units) images: dict[str, set[str]] = {} unscannable: list[tuple[str, str]] = [] + seen = set() if seen is None else seen for u in units: if u.chart in getattr(render_addons, "SKIP_CHARTS", {}): @@ -137,38 +155,65 @@ def inventory(env: str) -> tuple[dict[str, set[str]], list[tuple[str, str]]]: if proc.returncode != 0: unscannable.append((u.path, (proc.stderr.strip() or proc.stdout.strip())[:200])) continue - for ref_str in extract_images(proc.stdout, u.chart, unscannable): + for ref_str in extract_images(proc.stdout, u.chart, unscannable, seen): images.setdefault(ref_str, set()).add(u.chart) return images, unscannable -# Kinds that own a pod template, and the path from the document to its podSpec. -# The deployable surface: every container the cluster starts comes from one of -# these, so this is the population any question about "what runs" reads. -POD_OWNERS = { - "Pod": ("spec",), - "Deployment": ("spec", "template", "spec"), - "StatefulSet": ("spec", "template", "spec"), - "DaemonSet": ("spec", "template", "spec"), - "ReplicaSet": ("spec", "template", "spec"), - "ReplicationController": ("spec", "template", "spec"), - "Job": ("spec", "template", "spec"), - "CronJob": ("spec", "jobTemplate", "spec", "template", "spec"), +# An image reference as a controller is handed one: in a flag, a ConfigMap value, +# a CR field. Requires a path separator, which is what separates an image from +# the host:port strings that fill a rendered config — `loki.monitoring.svc:3100` +# and `127.0.0.1:8080` are addresses, not images, and no grammar that admits them +# can be read by a human. +IMAGE_REF = re.compile( + r"\b((?:[a-z0-9][a-z0-9._-]*(?:\.[a-z0-9._-]+)+(?::\d+)?/)?" + r"[a-z0-9][a-z0-9._-]*(?:/[a-z0-9][a-z0-9._-]*)+:[a-zA-Z0-9][\w.-]*)\b") + +# Images a CONTROLLER starts, which no pod template in this render declares. The +# controller is handed the reference and creates the pod later, so the deployable +# surface is strictly larger than what `helm template` shows as a container. +# +# Whether a given string is one of these is a fact about a controller's behaviour, +# and this gate reads manifests. So it is declared rather than inferred, and the +# declaration is what the assertion is against: every image-shaped string the +# render carries must be in the structural population, on this list, or on +# NOT_A_CONTAINER. An unclassified one is reported, so the answer to "is this +# deployed?" is never silence. +CONTROLLER_IMAGES = { + "docker.io/envoyproxy/ratelimit": + "the Envoy Gateway controller reads it from its own EnvoyGateway " + "ConfigMap and creates the rate-limit Deployment", + "docker.io/envoyproxy/ai-gateway-extproc": + "the AI Gateway controller injects it as the ext-proc sidecar on every " + "gateway pod it manages", + "ghcr.io/aquasecurity/node-collector": + "trivy-operator creates one node-collector pod per node scan", + "quay.io/argoproj/argoexec": + "the workflow controller injects it as the init and wait container of " + "every workflow step pod", + "quay.io/jetstack/cert-manager-acmesolver": + "cert-manager creates one solver pod per HTTP-01 challenge", + "natsio/nats-server-config-reloader": + "the argo-events eventbus controller creates the NATS StatefulSet with " + "this sidecar", + "natsio/prometheus-nats-exporter": + "the same controller, same StatefulSet", + "ghcr.io/kyverno/kyverno": + "the chart's own CLI image reference, carried in a ConfigMap for " + "`kyverno` invocations rather than as a container", } -CONTAINER_LISTS = ("initContainers", "containers", "ephemeralContainers") - -# Images that reach a cluster without ever appearing as a YAML `image:` key, -# because a controller reads them out of a string payload it is handed. Neither -# structural walk can see one, so the text scan is what finds them — and an entry -# here is asserted in both directions below: one a structural walk DOES find is -# stale, and an image only the text scan finds that is not listed means a walk -# stopped seeing a shape. -TEXT_ONLY_IMAGES = { - "docker.io/envoyproxy/ratelimit": "the Envoy Gateway controller reads the rate-limit " - "image out of its own EnvoyGateway ConfigMap, so it " - "is a string inside a config blob rather than a " - "container in a pod template", +# Image-shaped strings that are not container images. An OCI artifact reference +# resolves through a registry and looks identical, and pulling one starts no +# container — so scanning it for a running-container CVE would report on +# something nothing runs. +NOT_A_CONTAINER = { + "ghcr.io/falcosecurity/plugins/plugin/container": + "a falcoctl rules/plugin OCI artifact, unpacked into an emptyDir", + "ghcr.io/falcosecurity/plugins/plugin/k8smeta": + "the same, the k8smeta plugin", + "mirror.gcr.io/aquasec/trivy-checks": + "the trivy checks bundle, an OCI artifact trivy-operator downloads", } @@ -184,7 +229,8 @@ def inventory(env: str) -> tuple[dict[str, set[str]], list[tuple[str, str]]]: } -def chart_coverage(images: dict[str, set[str]], units) -> list[str]: +def chart_coverage(images: dict[str, set[str]], units, + seen: set[str] | None = None) -> list[str]: """Every chart the render covered contributed an image, or is declared not to. A per-chart floor rather than a total, because a total cannot see the shape @@ -194,6 +240,11 @@ def chart_coverage(images: dict[str, set[str]], units) -> list[str]: from the corpus — no constant to pick, and it moves with the catalog. """ problems: list[str] = [] + # Keyed by chart NAME, which is what `images` records, and this catalog pins + # opentelemetry-collector three times — otel-agent, otel-gateway and + # otel-gateway-floor. Any one of those can render nothing while the other two + # keep the name contributing, so the floor is per chart name and says so + # rather than reading as per render unit. rendered = {u.chart for u in units if u.chart not in getattr( render_addons, "SKIP_CHARTS", {})} contributing = set().union(*images.values()) if images else set() @@ -207,6 +258,9 @@ def chart_coverage(images: dict[str, set[str]], units) -> list[str]: f"seeing a shape this chart uses, or the chart ships only CRDs and " f"belongs in IMAGELESS_CHARTS with the reason.") + problems += declaration_rot(seen if seen is not None + else {bare_name(ref) for ref in images}) + for chart, reason in sorted(IMAGELESS_CHARTS.items()): if chart not in rendered: problems.append( @@ -232,43 +286,37 @@ def bare_name(ref: str) -> str: return ref.rsplit(":", 1)[0] if ":" in name else ref -def _podspec(doc: dict, path: tuple[str, ...]) -> dict | None: - cur: object = doc - for key in path: - if not isinstance(cur, dict): - return None - cur = cur.get(key) - return cur if isinstance(cur, dict) else None +def string_scalars(node) -> list[str]: + """Every string a rendered document carries, at any depth. - -def podspec_images(docs: list) -> set[str]: - """Every container image the rendered documents start, walked structurally.""" - found: set[str] = set() - for doc in docs: - if not isinstance(doc, dict): - continue - kind = doc.get("kind") - if not isinstance(kind, str): - continue - path = POD_OWNERS.get(kind) - if path is None: - continue - spec = _podspec(doc, path) - if spec is None: - continue - for key in CONTAINER_LISTS: - for container in spec.get(key) or []: - if isinstance(container, dict) and isinstance(container.get("image"), str): - found.add(container["image"]) - return found + A controller is handed an image the same way it is handed anything else — a + flag in `args`, a value in a ConfigMap, a field on a custom resource — so the + surface an image can arrive through is every string, not a key name. + """ + out: list[str] = [] + if isinstance(node, dict): + for value in node.values(): + out.extend(string_scalars(value)) + elif isinstance(node, list): + for value in node: + out.extend(string_scalars(value)) + elif isinstance(node, str): + out.append(node) + return out def keyed_images(node) -> set[str]: """Every `image:` key anywhere in the documents, whatever declares it. - A custom resource can name an image its operator then runs — the agent - platform's eval-runner is one — and no pod template in this render carries - it. Restricting the walk to pod owners would drop it. + Structure, not text. The pattern this replaced was anchored on `image:` + preceded only by whitespace, so the ordinary list-item form `- image: ` + under `containers:` never matched it and two charts' entire workloads were + absent from a scan reporting fifty-five images. + + Deliberately not restricted to pod templates. A walk over pod owners is a + strict SUBSET of this one — every container's image is an `image:` key — so + it would be mechanism with no effect, and it would drop the custom resources + that name an image an operator then runs. """ found: set[str] = set() if isinstance(node, dict): @@ -284,7 +332,8 @@ def keyed_images(node) -> set[str]: def extract_images(rendered: str, chart: str, - unscannable: list[tuple[str, str]]) -> set[str]: + unscannable: list[tuple[str, str]], + seen: set[str] | None = None) -> set[str]: """Every image one chart's render deploys, and an assertion that it is every one. Structural first, because a pattern is a claim about spelling. IMAGE is @@ -307,18 +356,55 @@ def extract_images(rendered: str, chart: str, unscannable.append((chart, f"rendered YAML this gate could not parse — {first}")) return set() - structural = podspec_images(docs) | keyed_images(docs) + structural = keyed_images(docs) textual = {m.group(1) for m in IMAGE.finditer(rendered)} - - for ref_str in sorted(textual - structural): - if bare_name(ref_str) not in TEXT_ONLY_IMAGES: - unscannable.append(( - chart, - f"{ref_str} appears in the rendered text and in no pod template or " - f"`image:` key — either a structural walk stopped seeing a shape, or " - f"a controller reads it out of a payload and it belongs in " - f"TEXT_ONLY_IMAGES with the reason")) - return structural | textual + candidates = {c for s in string_scalars(docs) for c in IMAGE_REF.findall(s)} + if seen is not None: + # Every image-shaped reference the render carried, including the ones + # excluded below. A declaration is about what the render CONTAINS, so + # checking it against the surviving population would report every + # NOT_A_CONTAINER entry as stale by construction. + seen |= {bare_name(r) for r in structural | textual | candidates} + + # Compared on the bare name, because the same image reaches a render twice: + # once as a container reference carrying a digest, and once as a bare + # `repo:tag` in an annotation or a config value. Those are one image, and + # reporting the second as unreachable would be reporting the first. + structural_bare = {bare_name(r) for r in structural} + controller: set[str] = set() + for ref_str in sorted((textual | candidates) - structural): + bare = bare_name(ref_str) + if bare in structural_bare or bare in NOT_A_CONTAINER: + continue + if bare in CONTROLLER_IMAGES: + controller.add(ref_str) + continue + unscannable.append(( + chart, + f"{ref_str} is image-shaped and reaches no pod template or `image:` key. " + f"Either a structural walk stopped seeing a shape, or a controller is " + f"handed it and starts a pod later — in which case it belongs in " + f"CONTROLLER_IMAGES with the controller that starts it, or in " + f"NOT_A_CONTAINER if pulling it runs nothing")) + return structural | textual | controller + + +def declaration_rot(seen: set[str]) -> list[str]: + """Declarations the render no longer supports. + + Both lists say something about images this catalog renders. An entry for one + it does not is an excuse for nothing, and an exemption list nobody re-reads + only ever widens. + """ + problems = [] + for table, label in ((CONTROLLER_IMAGES, "started by a controller"), + (NOT_A_CONTAINER, "not a container image")): + for bare, reason in sorted(table.items()): + if bare not in seen: + problems.append( + f"{bare} is declared {label} and the fleet renders no reference to " + f"it — the declaration outlived its image. (recorded: {reason})") + return problems def classify(ref: str) -> str: @@ -338,7 +424,8 @@ def main() -> int: ap.add_argument("--env", default="production") args = ap.parse_args() - images, unscannable = inventory(args.env) + seen: set[str] = set() + images, unscannable = inventory(args.env, seen) if args.list: for ref in sorted(images): @@ -355,7 +442,7 @@ def main() -> int: print(f" {path}: {err}") return 2 - failures = chart_coverage(images, render_addons.discover()) + failures = chart_coverage(images, render_addons.discover(), seen) mutable_seen: set[str] = set() for ref in sorted(images): diff --git a/scripts/check-image-vulnerabilities.py b/scripts/check-image-vulnerabilities.py index e75aad8..9a6e80d 100755 --- a/scripts/check-image-vulnerabilities.py +++ b/scripts/check-image-vulnerabilities.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Every image the pinned charts reference is scanned, and a CRITICAL is a decision. +"""Every image the production render carries is scanned, and a CRITICAL is a decision. scripts/check-image-vulnerabilities.py # blocking gate scripts/check-image-vulnerabilities.py --list # print every finding @@ -68,6 +68,17 @@ statement about the scanner, not about the catalogue, and the two must not print the same thing. +ONE ENVIRONMENT, SAID PLAINLY + +The population is the production render. render-addons produces four — +development, staging, production and hub — and nothing here asserts they agree, +so a component enabled only in development or on the hub is deployed by this +fleet and is not scanned. That is a narrower claim than "every image the fleet +renders", and it is the claim this gate holds: every image the PRODUCTION render +carries. Widening it means four scans of seventy images per run, which is a cost +decision rather than a technical one, and until it is taken the boundary belongs +in writing rather than in the reader's assumption. + UNSCANNABLE IS NOT CLEAN An image that could not be pulled contributed no findings. Counting the rest as @@ -258,7 +269,10 @@ def verdict(findings: list[Finding], advisories: list[dict]) -> list[str]: # 3. An entry no image carries. 4. An entry listing an image with no such finding. carried = {(f.id, f.package, f.bare) for f in blocking} - for entry in advisories: + # The mappings only: a non-mapping entry is already reported above, and + # reaching `.get` on it here is the traceback that sentence exists to + # replace. + for entry in [e for e in advisories if isinstance(e, dict)]: key = (str(entry.get("id", "")), str(entry.get("package", ""))) listed = [str(i) for i in (entry.get("images") or [])] if not listed: @@ -317,14 +331,15 @@ def main() -> int: if args.self_test: return 0 - images, unrendered = image_pins.inventory(args.env) + seen: set[str] = set() + images, unrendered = image_pins.inventory(args.env, seen) if unrendered: cannot_run("Cannot run: charts that did not render contributed no images, so " "the scan below would cover part of the fleet and report on all of " "it.", *(f" {path}: {err}" for path, err in unrendered)) units = image_pins.render_addons.discover() - coverage = image_pins.chart_coverage(images, units) + coverage = image_pins.chart_coverage(images, units, seen) if coverage: cannot_run("Cannot run: the image population is smaller than the charts that " "rendered it, so a scan over it says less than it appears to:", diff --git a/scripts/tests/test_corpus_floors.py b/scripts/tests/test_corpus_floors.py index 1a9f123..e99d93e 100644 --- a/scripts/tests/test_corpus_floors.py +++ b/scripts/tests/test_corpus_floors.py @@ -26,6 +26,12 @@ ROOT = pathlib.Path(__file__).resolve().parent.parent.parent +# The API-group suffix the operator chart's CRDs share. The gate filters on the +# schema set the chart ships, which needs the chart; this is the part of that +# filter available offline, and it is what separates a platform CR from an +# ApplicationSet that happens to share a version suffix. +OPERATOR_API_SUFFIX = ".nanohype.dev" + render_addons = load("render-addons") fork_safety = load("check-hardcoded-org") platform_crs = load("check-platform-crs") @@ -57,8 +63,16 @@ def test_the_applied_appsets_clear_the_fork_safety_floor(self): "ApplicationSets, so the fork-safety gate cannot pass") def test_the_platform_crs_clear_their_floor(self): - """Counted with the gate's own filters: chart source is Go-template text, - which the walk identifies structurally rather than by what breaks a parser.""" + """The gate keeps documents whose KIND is in the operator chart's schema + set; a version suffix alone is not that filter. + + Matching `/v1alpha1` counts every ApplicationSet (argoproj.io/v1alpha1) + and every Kyverno test fixture in the tree — 44 documents against the 8 + the gate walks — so the assertion held for any floor up to 43 and could + not see an always-red one, which is the only thing it was added to see. + The operator's own API groups are the discriminator available offline; + the schema set itself needs the chart. + """ found = 0 for path in platform_crs.manifests(): if platform_crs.gatelib.is_helm_template(path): @@ -66,12 +80,16 @@ def test_the_platform_crs_clear_their_floor(self): for doc in yaml.safe_load_all(path.read_text()): if not isinstance(doc, dict): continue - if str(doc.get("apiVersion", "")).endswith( - "/" + platform_crs.CRD_VERSION): - found += 1 + api = str(doc.get("apiVersion", "")) + if not api.endswith("/" + platform_crs.CRD_VERSION): + continue + if not api.split("/", 1)[0].endswith(OPERATOR_API_SUFFIX): + continue + found += 1 self.assertGreater(found, platform_crs.MIN_CRS, - "MIN_CRS is at or above the number of platform CRs in the " - "tree, so check-platform-crs.py cannot pass") + f"MIN_CRS is {platform_crs.MIN_CRS} and this tree holds " + f"{found} platform CR(s), so check-platform-crs.py exits 2 " + f"on every run") def test_the_discovered_addons_clear_the_policy_admission_floor(self): """The render is one manifest per unit per environment it reaches, so the @@ -79,7 +97,11 @@ def test_the_discovered_addons_clear_the_policy_admission_floor(self): THAT is one no render can clear.""" units = policy_admission.discover() self.assertGreater(len(units), 0) - reachable = len(units) * len(render_addons.ENVIRONMENTS) + # ENFORCE_ENVS, not ENVIRONMENTS: check-policy-admission renders the + # Enforce overlay only. Bounding against all four doubles the ceiling and + # lets any floor in 66..127 pass here while making the gate red on every + # run — the failure this test exists to catch. + reachable = len(units) * len(policy_admission.ENFORCE_ENVS) self.assertLess( policy_admission.MIN_RENDERED, reachable, "MIN_RENDERED is at or above the largest render this catalog can " @@ -131,7 +153,7 @@ def test_the_image_floor_is_derived_per_chart(self): """Not a constant: every chart the render covers must contribute an image, which is the shape a total cannot see.""" source = (ROOT / "scripts" / "check-image-vulnerabilities.py").read_text() - self.assertIn("coverage = image_pins.chart_coverage(images, units)", source) + self.assertIn("coverage = image_pins.chart_coverage(images, units, seen)", source) self.assertIn("if len(images) < len(units):", source) self.assertNotIn("MIN_IMAGES", source, "the picked constant is back; the per-chart derivation is " diff --git a/scripts/tests/test_image_extraction.py b/scripts/tests/test_image_extraction.py index 2043d35..8b91b70 100644 --- a/scripts/tests/test_image_extraction.py +++ b/scripts/tests/test_image_extraction.py @@ -82,7 +82,7 @@ def test_every_container_list_is_walked(self): self.assertEqual(found, {"init:1", "main:2", "debug:3"}) def test_a_cronjob_pod_template_is_reached(self): - """Its podSpec sits two levels deeper than every other owner's.""" + """Nesting depth is not a case the walk has to know about.""" rendered = """ apiVersion: batch/v1 kind: CronJob @@ -98,18 +98,6 @@ def test_a_cronjob_pod_template_is_reached(self): found, _ = self.images(rendered) self.assertEqual(found, {"aws-cli:2"}) - def test_every_pod_owner_kind_is_walked(self): - for kind in sorted(gate.POD_OWNERS): - with self.subTest(kind=kind): - path = gate.POD_OWNERS[kind] - spec: dict = {"containers": [{"name": "c", "image": "x:1"}]} - for key in reversed(path[1:]): - spec = {key: spec} - import yaml - doc = yaml.safe_dump({"apiVersion": "v1", "kind": kind, "spec": spec}) - found, _ = self.images(doc) - self.assertEqual(found, {"x:1"}) - def test_an_image_key_outside_a_pod_template_is_found(self): """A custom resource can name an image its operator then runs.""" rendered = """ @@ -141,8 +129,10 @@ def test_an_image_only_the_text_scan_finds_is_reported(self): self.assertEqual(len(problems), 1) self.assertIn("stopped seeing a shape", problems[0][1]) - def test_a_declared_text_only_image_is_not_reported(self): - declared = next(iter(gate.TEXT_ONLY_IMAGES)) + def test_a_declared_controller_image_is_not_reported(self): + """A controller is handed the reference and creates the pod later, so no + pod template in this render declares it.""" + declared = next(iter(gate.CONTROLLER_IMAGES)) rendered = f""" apiVersion: v1 kind: ConfigMap @@ -154,6 +144,42 @@ def test_a_declared_text_only_image_is_not_reported(self): self.assertIn(f"{declared}:abc123", found) self.assertEqual(problems, []) + def test_an_image_in_a_controller_flag_is_a_candidate(self): + """The shape neither the key walk nor the line-anchored pattern can see. + + A controller handed `--worker-image=` creates the pod later; the + reference is an argument, not a key, and it is not at line start. + """ + rendered = """ +apiVersion: apps/v1 +kind: Deployment +spec: + template: + spec: + containers: + - name: c + image: ghcr.io/example/app:1.0.0 + args: ["--worker-image=ghcr.io/example/undeclared-worker:2.0"] +""" + found, problems = self.images(rendered) + self.assertEqual(len(problems), 1) + self.assertIn("ghcr.io/example/undeclared-worker:2.0", problems[0][1]) + self.assertIn("a controller is handed it", problems[0][1]) + + def test_a_host_and_port_is_not_an_image(self): + """A rendered config is full of addresses; a grammar admitting them + cannot be read, and every one would demand a declaration.""" + rendered = """ +apiVersion: v1 +kind: ConfigMap +data: + cfg: | + endpoint: loki.monitoring.svc.cluster.local:3100 + bind: 127.0.0.1:8080 +""" + found, problems = self.images(rendered) + self.assertEqual(problems, []) + def test_a_render_that_will_not_parse_is_reported_not_dropped(self): """A chart silently absent from the inventory is the whole failure mode.""" found, problems = self.images("kind: Deployment\n bad: [unclosed\n") @@ -173,12 +199,22 @@ def test_the_helm_value_sentinel_does_not_lose_the_chart(self): class EveryChartContributesAnImage(unittest.TestCase): """A per-chart floor, because a total cannot see two charts falling out.""" - def coverage(self, images, charts): - """Every declared imageless chart is rendered unless a case says otherwise, - so a fixture does not trip the exemption's own rot rule by omission.""" + def coverage(self, images, charts, seen=None): + """Every declared imageless chart is rendered, and every declared image + is seen, unless a case says otherwise — so a fixture does not trip a rot + rule it is not about. + """ units = [Unit(c) for c in charts] + [ Unit(c) for c in gate.IMAGELESS_CHARTS if c not in charts] - return gate.chart_coverage(images, units) + if seen is None: + seen = set(gate.CONTROLLER_IMAGES) | set(gate.NOT_A_CONTAINER) + return gate.chart_coverage(images, units, seen) + + def test_declaration_rot_reaches_the_verdict(self): + """chart_coverage is where the rot rule is consulted, so gutting the call + must fail here and not only where the rule itself is tested.""" + problems = self.coverage({"x:1": {"a"}}, ["a"], seen=set()) + self.assertTrue(any("outlived its image" in p for p in problems)) def test_a_chart_contributing_nothing_is_reported(self): problems = self.coverage({"x:1": {"a"}}, ["a", "b"]) @@ -196,7 +232,8 @@ def test_a_declared_imageless_chart_is_not_reported(self): def test_a_declared_chart_the_fleet_stopped_rendering_is_reported(self): """The exemption is where a per-chart floor rots: an entry excusing a chart nobody pins excuses nothing and reads as considered.""" - problems = gate.chart_coverage({"x:1": {"a"}}, [Unit("a")]) + seen = set(gate.CONTROLLER_IMAGES) | set(gate.NOT_A_CONTAINER) + problems = gate.chart_coverage({"x:1": {"a"}}, [Unit("a")], seen) self.assertEqual(len(problems), len(gate.IMAGELESS_CHARTS)) self.assertIn("outlived its chart", problems[0]) @@ -213,3 +250,37 @@ def test_an_empty_inventory_reports_every_chart(self): if __name__ == "__main__": unittest.main() + + +class DeclarationsMatchTheRender(unittest.TestCase): + """Both declaration tables say something about images this catalog renders. + + An entry for one it does not is an excuse for nothing, and an exemption list + nobody re-reads only ever widens. This is the reverse direction the + controller-image comment claims and did not have. + """ + + def test_a_controller_image_the_render_dropped_is_reported(self): + declared = next(iter(gate.CONTROLLER_IMAGES)) + problems = gate.declaration_rot( + (set(gate.CONTROLLER_IMAGES) | set(gate.NOT_A_CONTAINER)) - {declared}) + self.assertEqual(len(problems), 1) + self.assertIn(declared, problems[0]) + self.assertIn("outlived its image", problems[0]) + + def test_a_non_container_declaration_the_render_dropped_is_reported(self): + declared = next(iter(gate.NOT_A_CONTAINER)) + problems = gate.declaration_rot( + (set(gate.CONTROLLER_IMAGES) | set(gate.NOT_A_CONTAINER)) - {declared}) + self.assertEqual(len(problems), 1) + self.assertIn(declared, problems[0]) + + def test_every_declaration_matched_passes(self): + self.assertEqual( + gate.declaration_rot(set(gate.CONTROLLER_IMAGES) | set(gate.NOT_A_CONTAINER)), + []) + + def test_an_empty_render_reports_every_declaration(self): + problems = gate.declaration_rot(set()) + self.assertEqual(len(problems), + len(gate.CONTROLLER_IMAGES) + len(gate.NOT_A_CONTAINER)) From 31e449bfb5a44523ec5348c43f70ba8ae34bbc09 Mon Sep 17 00:00:00 2001 From: stxkxs Date: Wed, 2 Sep 2026 12:51:20 -0700 Subject: [PATCH 4/6] Admit the official images that carry no prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An official image is its own whole reference. `nats:2.10.10` has no registry and no organisation, so a grammar requiring a path separator could not see one — and the argo-events event-bus controller declares exactly that shape in its version table, beside the two `natsio/*` sidecars it starts in the same StatefulSet. One StatefulSet had its helpers scanned and its main container silent, which the gate's own words rule out: an unclassified image is reported so the answer to is-this-deployed is never silence. The population is 80 images from 70, and the fixed CRITICALs it carries go from 86 to 134. The single-segment alternative needs two discriminators the prefixed one does not, because a rendered config is full of strings with that shape. The match may not begin part-way through a longer token, or `vault.example.com:8200` yields `com:8200`. The name must start with a letter and carry no dot, which excludes a timestamp, an address, an IPv6 fragment and a ratio. The tag must be `latest` or begin with a digit, which excludes the RBAC names — `kyverno:admission-controller`, `system:auth-delegator` — that are the shape's other occupant. What remains and is not a container is declared: the falco rulesfile artifacts, a digest tail, two service addresses. `nats-streaming:latest` came with them: a moving tag in the same version table, on a row nothing here selects and nothing here can pin. MIN_CRS drops from 5 to 2. `walked` counts documents whose kind the pinned operator chart ships a schema for, so it depends on what that chart resolves to where the gate runs — a reviewer measured three where this tree walks eight. A floor above the smallest resolution makes the gate red somewhere it should be green, which is what a floor above its corpus always is. Two narrowings are now written down rather than left to be discovered. The population is chart-sourced Applications: a kustomize-sourced one renders workloads through no chart and contributes nothing, and check-policy-admission.py names one by path in KUSTOMIZE_WORKLOADS whose images run on every full-tier cluster. And it is the production render; the other three environments are not asked. Both are boundaries this gate has, stated beside each other. Co-authored-by: stxkxsbot <275011021+stxkxsbot@users.noreply.github.com> --- image-advisories.yaml | 13 ++++++ scripts/check-image-pins.py | 61 ++++++++++++++++++++++++-- scripts/check-image-vulnerabilities.py | 10 +++++ scripts/check-platform-crs.py | 10 +++-- scripts/tests/test_image_extraction.py | 41 +++++++++++++++++ 5 files changed, 129 insertions(+), 6 deletions(-) diff --git a/image-advisories.yaml b/image-advisories.yaml index 2ffa64d..b1b2a09 100644 --- a/image-advisories.yaml +++ b/image-advisories.yaml @@ -60,6 +60,8 @@ advisories: - ghcr.io/kedacore/keda-metrics-apiserver - ghcr.io/opencost/opencost - mirror.gcr.io/aquasec/trivy-operator + - nats + - nats-streaming - natsio/prometheus-nats-exporter - otel/opentelemetry-collector-contrib - prom/memcached-exporter @@ -94,6 +96,8 @@ advisories: images: - docker.io/falcosecurity/k8s-metacollector - ghcr.io/aquasecurity/node-collector + - nats + - nats-streaming - natsio/nats-server-config-reloader - natsio/prometheus-nats-exporter - quay.io/argoproj/argo-events @@ -108,6 +112,8 @@ advisories: 1.21.11 and 1.22.4. The falco chart pin is what moves it. images: - docker.io/falcosecurity/k8s-metacollector + - nats + - nats-streaming - natsio/nats-server-config-reloader - natsio/prometheus-nats-exporter @@ -193,6 +199,7 @@ advisories: controller starts. Fixed in 2.11.1 and 2.10.27; the argo-events chart leaves this image's tag unset, so it moves only when the chart does. images: + - nats-streaming - natsio/prometheus-nats-exporter - id: CVE-2022-48174 @@ -225,6 +232,7 @@ advisories: zlib in the same NATS config-reloader base layer, fixed in 1.2.12-r2. The image is pinned old by the argo-events chart; nothing here rebuilds it. images: + - nats - natsio/nats-server-config-reloader - id: CVE-2022-23806 @@ -234,6 +242,7 @@ advisories: and 1.17.7. Moves when the argo-events chart ships an image built on a patched toolchain. images: + - nats-streaming - natsio/prometheus-nats-exporter - id: CVE-2023-24538 @@ -242,6 +251,8 @@ advisories: The same shape against both NATS images the eventbus controller starts, fixed in Go 1.19.8 and 1.20.3. images: + - nats + - nats-streaming - natsio/nats-server-config-reloader - natsio/prometheus-nats-exporter @@ -250,5 +261,7 @@ advisories: reason: >- The same two images and the same remedy, fixed in Go 1.19.9 and 1.20.4. images: + - nats + - nats-streaming - natsio/nats-server-config-reloader - natsio/prometheus-nats-exporter diff --git a/scripts/check-image-pins.py b/scripts/check-image-pins.py index 275f92b..6131fd3 100755 --- a/scripts/check-image-pins.py +++ b/scripts/check-image-pins.py @@ -109,6 +109,11 @@ "controller-supplied references entered the population. Clears when the " "chart exposes the tag, or when this catalog stops shipping that " "ConfigMap.", + "nats-streaming": + "the argo-events version table's oldest streaming row carries `latest`, " + "alongside rows that pin. An eventbus asking for that version gets a " + "moving image; nothing in this catalog selects it, and nothing here can " + "pin it. Clears on a chart that pins the row.", "natsio/prometheus-nats-exporter": "an argo-events eventbus default the controller passes to the NATS " "StatefulSet it creates. The chart pins two other tags of this image and " @@ -165,9 +170,34 @@ def inventory(env: str, seen: set[str] | None = None # the host:port strings that fill a rendered config — `loki.monitoring.svc:3100` # and `127.0.0.1:8080` are addresses, not images, and no grammar that admits them # can be read by a human. +# Two alternatives, because an official image carries neither a registry nor an +# organisation: `nats:2.10.10` is the whole reference. Requiring a path separator +# made that shape invisible, and the argo-events event-bus controller declares +# exactly it — `natsImage` beside the two `natsio/*` sidecars in the same rows, +# so one StatefulSet had its helpers scanned and its main container silent. +# +# The single-segment alternative needs two discriminators the prefixed one does +# not, because a rendered config is full of strings with that shape: +# +# * the match may not begin part-way through a longer token. Without that the +# alternative starts after a dot and `vault.example.com:8200` yields +# `com:8200`, which is a hostname's last label and a port. +# * the NAME must start with a letter and carry no dot. That excludes a +# timestamp (`00:00Z`), an address (`10.0.0.5:8080`, +# `telemetry.monitoring.svc.cluster.local:4318`), an IPv6 fragment and a +# ratio (`1:1`). +# * the TAG must be `latest` or begin with a digit, optionally after a `v`. +# That excludes the RBAC names — `kyverno:admission-controller`, +# `system:auth-delegator` — which are the shape's other common occupant. +# +# Together they admit every official-image reference this fleet pins and no +# string in the render that is not one. IMAGE_REF = re.compile( - r"\b((?:[a-z0-9][a-z0-9._-]*(?:\.[a-z0-9._-]+)+(?::\d+)?/)?" - r"[a-z0-9][a-z0-9._-]*(?:/[a-z0-9][a-z0-9._-]*)+:[a-zA-Z0-9][\w.-]*)\b") + r"(?` splits at the `@` and the tail has the single-segment " + "shape", + "localhost": + "a memcached address in a Loki config value; `localhost:11211` is a host " + "and a port", + "hubble-relay": + "a Kubernetes Service address in a Cilium config value — `hubble-relay:80` " + "has the shape of a single-segment image and is a host and a port", "ghcr.io/falcosecurity/plugins/plugin/container": "a falcoctl rules/plugin OCI artifact, unpacked into an emptyDir", "ghcr.io/falcosecurity/plugins/plugin/k8smeta": @@ -358,7 +412,8 @@ def extract_images(rendered: str, chart: str, structural = keyed_images(docs) textual = {m.group(1) for m in IMAGE.finditer(rendered)} - candidates = {c for s in string_scalars(docs) for c in IMAGE_REF.findall(s)} + candidates = {c for s in string_scalars(docs) + for groups in IMAGE_REF.findall(s) for c in groups if c} if seen is not None: # Every image-shaped reference the render carried, including the ones # excluded below. A declaration is about what the render CONTAINS, so diff --git a/scripts/check-image-vulnerabilities.py b/scripts/check-image-vulnerabilities.py index 9a6e80d..b90f025 100755 --- a/scripts/check-image-vulnerabilities.py +++ b/scripts/check-image-vulnerabilities.py @@ -68,6 +68,16 @@ statement about the scanner, not about the catalogue, and the two must not print the same thing. +CHART-SOURCED APPLICATIONS ONLY, SAID PLAINLY + +The population comes from check-image-pins.inventory, which walks the +ApplicationSets that pin a Helm chart. A kustomize-sourced Application renders +workloads through no chart and contributes nothing — check-policy-admission.py +names one by path in KUSTOMIZE_WORKLOADS (dashboards/base, the grafana-operator +namespace), and its images run on every full-tier cluster and are not scanned +here. Taking them in means a second render path; until that lands the boundary is +written down, the same way the environment one below is. + ONE ENVIRONMENT, SAID PLAINLY The population is the production render. render-addons produces four — diff --git a/scripts/check-platform-crs.py b/scripts/check-platform-crs.py index 30fccea..e9b1275 100755 --- a/scripts/check-platform-crs.py +++ b/scripts/check-platform-crs.py @@ -91,9 +91,13 @@ SKIP_DIRS = {".git", "node_modules", "rendertest", "__pycache__", ".task"} -# A floor on platform CRs walked. Set under what the catalog declares, so it -# catches a walk that matched almost nothing rather than one CR being retired. -MIN_CRS = 5 +# A floor on platform CRs walked. Set low on purpose: `walked` counts documents +# whose KIND the pinned operator chart ships a schema for, so the number depends +# on what that chart resolves to in the environment running the gate — a +# reviewer measured three where this tree walks eight. A floor above the smallest +# resolution makes the gate red somewhere it should be green, which is the +# failure a floor above the corpus always is. +MIN_CRS = 2 def pinned_chart_version() -> str: diff --git a/scripts/tests/test_image_extraction.py b/scripts/tests/test_image_extraction.py index 8b91b70..3a5ba57 100644 --- a/scripts/tests/test_image_extraction.py +++ b/scripts/tests/test_image_extraction.py @@ -166,6 +166,47 @@ def test_an_image_in_a_controller_flag_is_a_candidate(self): self.assertIn("ghcr.io/example/undeclared-worker:2.0", problems[0][1]) self.assertIn("a controller is handed it", problems[0][1]) + def test_a_single_segment_official_image_is_a_candidate(self): + """`nats:2.10.10` is the whole reference — no registry, no organisation. + + The argo-events event-bus controller declares exactly this beside the two + natsio/* sidecars it starts in the same StatefulSet, so requiring a path + separator scanned the helpers and left the main container silent. + """ + rendered = """ +apiVersion: v1 +kind: ConfigMap +data: + controller-config.yaml: | + nats: + versions: + - natsImage: nats:2.10.10 + - natsImage: undeclared-single-segment:9.9.9 +""" + found, problems = self.images(rendered) + # `nats` is declared in CONTROLLER_IMAGES, so it enters the population. + self.assertIn("nats:2.10.10", found) + # An undeclared one is reported rather than silently absent. + self.assertEqual(len(problems), 1) + self.assertIn("undeclared-single-segment:9.9.9", problems[0][1]) + + def test_an_rbac_name_is_not_an_image(self): + """`kyverno:admission-controller` has the shape and is a role name; the + tag is the discriminator.""" + rendered = ("apiVersion: v1\nkind: ConfigMap\ndata:\n" + " x: |\n role: kyverno:admission-controller\n" + " other: system:auth-delegator\n") + found, problems = self.images(rendered) + self.assertEqual(problems, []) + + def test_a_hostname_does_not_yield_its_last_label(self): + """Without an anchor the alternative starts after a dot and + `vault.example.com:8200` yields `com:8200`.""" + rendered = ("apiVersion: v1\nkind: ConfigMap\ndata:\n" + " x: |\n server: vault.example.com:8200\n") + found, problems = self.images(rendered) + self.assertEqual(problems, []) + def test_a_host_and_port_is_not_an_image(self): """A rendered config is full of addresses; a grammar admitting them cannot be read, and every one would demand a declaration.""" From c24532de36070d2522802a13e348504c01f402f6 Mon Sep 17 00:00:00 2001 From: stxkxs <139715017+stxkxs@users.noreply.github.com> Date: Fri, 4 Sep 2026 15:40:13 -0700 Subject: [PATCH 5/6] What this gate cannot read is not therefore absent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two ways an image escaped the scan entirely, and neither reached a verdict. Both are the same shape: the code was narrower than the sentence printed next to it. ──────────── A digest-pinned reference was not in the population ──────────── `IMAGE_REF` — the pattern under the structural walk, which is how an image handed to a controller in a flag or a ConfigMap is found — had two alternatives and both required a tag. `@sha256:` matched neither. The whole reference yielded only its `sha256:` tail, a single-segment shape whose bare name is `sha256`, and a NOT_A_CONTAINER entry by that name passed over it. A NOT_A_CONTAINER entry excuses whatever has its bare name, so that one entry excused every digest-pinned reference in the fleet. The sentence that makes it a defect rather than a limit is the gate's own remediation, which tells an operator to pin to a digest. The gate recommended the one spelling its completeness floor could not see. A third alternative reads the digest form, with or without a tag alongside it, and is tried first so `:@sha256:` is captured whole rather than as its `repo:tag` head — the digest a container runs is what reaches the classifier. The 64-hex suffix is discriminator enough on its own, so the path separator and tag restrictions the tag alternatives need are not repeated. MUTABLE_REMEDIATION becomes a module constant and a test reads the reference forms out of it, so advice naming a spelling the pattern does not read fails rather than shipping. The `sha256` declaration went with the fix, and `declaration_rot` is what removed it: with the repository-prefixed form matched whole, the fleet renders no bare digest for the entry to describe, and an entry matching nothing fails. Should one appear, it is matched, therefore reported, and the repair is a declaration that states its own reason. ──────────── An unplaceable reference exited 0 under another heading ──────── `inventory` carried two facts in one list. A chart whose render could not be produced or parsed, and an image-shaped reference the classifier could not place — printed together under "N chart(s) could not be rendered and were NOT scanned". The second chart rendered perfectly well; the string was never added to `failures`, so the run printed success and exited 0. CI blocked only because the sibling gate read the same list as a refusal, and a gate whose correctness depends on another gate's reading of its output has not stated its own result. Two lists, because they are two repairs and two verdicts: * a reference that cannot be placed is a failure, exit 1, naming the two declarations that resolve it — the controller that starts it, or the reason pulling it runs nothing; * a chart that did not render leaves the fleet's image set unknown, so a run with no other finding exits 2 and says the result covers part of the fleet rather than the fleet. When there is also a real failure the verdict is that failure, with the unrendered charts printed above it as the bound on what was covered. check-image-vulnerabilities.py reads both and refuses on either, so the scan population is never quietly smaller than the fleet it reports on. ──────────────────────── What proves it ──────────────────────── Ten mutants, each reverting one behaviour, each killed and each naming a test: the digest alternative removed, moved last, its hex run unbounded, its single-segment form barred; the unplaceable reference returned to the unrendered list, or reaching no verdict; the unrendered chart reporting a clean fleet, or dropped from a failing run's scope line; a remediation naming a form the pattern cannot read; and the declaration that used to swallow the digest tail. On the real tree, both directions. A digest-only reference planted in a rendered pod annotation: this gate exits 1 naming it, and the same tree under the previous reader prints "all 80 rendered image(s) carry an immutable reference" and exits 0. A chart declared imageless whose values will not parse — chosen because any other chart is caught by the per-chart floor first: exit 2 here, exit 0 and the identical success line before. Both are probes in scripts/tests/reverify-gates.sh, floor 25 → 27. scripts/tests/test_image_extraction.py adds the digest forms, the strings that are digest-shaped and are not digests, and main()'s verdict over a planted inventory with a clean control on both ends. The coverage ratchet rises to 22% combined and 71% on this gate. Co-authored-by: stxkxsbot <275011021+stxkxsbot@users.noreply.github.com> --- scripts/check-image-pins.py | 146 ++++++++++---- scripts/check-image-vulnerabilities.py | 11 +- scripts/tests/reverify-gates.sh | 36 +++- scripts/tests/run.py | 4 +- scripts/tests/test_image_extraction.py | 259 ++++++++++++++++++++++++- 5 files changed, 412 insertions(+), 44 deletions(-) diff --git a/scripts/check-image-pins.py b/scripts/check-image-pins.py index 6131fd3..0df2343 100755 --- a/scripts/check-image-pins.py +++ b/scripts/check-image-pins.py @@ -35,11 +35,32 @@ mutable one (`latest`, `main`, `master`, `edge`, `stable`, `dev`), resolves to something different tomorrow with nothing in this repo changing. -UNSCANNABLE IS REPORTED, NEVER COUNTED CLEAN - -A chart that fails to render is named in the output and its images are absent -from the inventory. A gate that silently skipped it would report the remaining -charts as the whole fleet. +WHAT CANNOT BE READ IS NOT THEREFORE ABSENT + +Two ways this gate can fail to see an image, each with its own verdict, because +a run that examined less than it reports on is not a clean run. + +A chart that fails to render contributes no images, so exit 2: the images that +DID render carry an immutable reference, and that is a result over part of the +fleet rather than over the fleet. + +A reference the classifier cannot place is a chart that rendered perfectly well +and carries a string this gate owes an answer about. It is a failure, exit 1, +naming the two declarations that resolve it — the controller that starts it, or +the reason pulling it runs nothing. Reported as an aside it reached no verdict +at all, and the run's correctness rested on a sibling gate reading the same list +as a refusal; a gate whose result depends on another gate's reading of its +output has not stated its own. + +THE REFERENCE FORMS THIS READS + +An `image:` key is read whole, in every spelling and at every depth, so a +digest, a tag or a bare name reaches the classifier as written. Under that walk +sits a pattern over every string a render carries, which is how an image handed +to a controller in a flag or a ConfigMap is found — and it admits the digest +form, `@sha256:`, with or without a tag alongside it. That is the +form this gate's own remediation text recommends, so being blind to it would be +recommending the one spelling the completeness floor could not see. """ from __future__ import annotations @@ -122,8 +143,16 @@ def inventory(env: str, seen: set[str] | None = None - ) -> tuple[dict[str, set[str]], list[tuple[str, str]]]: - """(image -> charts that render it, [(path, why-not-scanned)]). + ) -> tuple[dict[str, set[str]], list[tuple[str, str]], list[tuple[str, str]]]: + """(image -> charts that render it, charts not rendered, references not placed). + + Two lists rather than one, because they are two facts with two repairs and + two verdicts. A chart that did not render leaves the fleet's image set + unknown — the gate examined less than it reports on. A reference the + classifier cannot place is a chart that rendered perfectly well and carries + something this gate cannot answer for; the repair is to classify it. Held in + one list, the second printed under the first's heading and reached no + verdict at all. `seen`, if given, collects every image-shaped reference the render carried — including the ones excluded from the population — so a declaration can be @@ -133,16 +162,17 @@ def inventory(env: str, seen: set[str] | None = None units = render_addons.discover() aliases = render_addons.add_repos(units) images: dict[str, set[str]] = {} - unscannable: list[tuple[str, str]] = [] + unrendered: list[tuple[str, str]] = [] + unclassified: list[tuple[str, str]] = [] seen = set() if seen is None else seen for u in units: if u.chart in getattr(render_addons, "SKIP_CHARTS", {}): - unscannable.append((u.path, "chart is on render-addons' SKIP_CHARTS list")) + unrendered.append((u.path, "chart is on render-addons' SKIP_CHARTS list")) continue d = ROOT / u.path if not d.is_dir(): - unscannable.append((u.path, "path does not exist")) + unrendered.append((u.path, "path does not exist")) continue vf = [] if (d / "values.yaml").exists(): @@ -158,11 +188,11 @@ def inventory(env: str, seen: set[str] | None = None cmd += vf proc = subprocess.run(cmd, capture_output=True, text=True, timeout=NETWORK_TIMEOUT) if proc.returncode != 0: - unscannable.append((u.path, (proc.stderr.strip() or proc.stdout.strip())[:200])) + unrendered.append((u.path, (proc.stderr.strip() or proc.stdout.strip())[:200])) continue - for ref_str in extract_images(proc.stdout, u.chart, unscannable, seen): + for ref_str in extract_images(proc.stdout, u.chart, unrendered, unclassified, seen): images.setdefault(ref_str, set()).add(u.chart) - return images, unscannable + return images, unrendered, unclassified # An image reference as a controller is handed one: in a flag, a ConfigMap value, @@ -192,9 +222,26 @@ def inventory(env: str, seen: set[str] | None = None # # Together they admit every official-image reference this fleet pins and no # string in the render that is not one. +# +# A THIRD alternative for the digest form, which the two tag alternatives cannot +# spell and which is the one this gate's own remediation text tells an operator +# to write. `@sha256:` carries no tag for them to match, so the whole +# reference yielded nothing but its `sha256:` tail — a single-segment shape +# whose bare name is `sha256`, and an entry by that name swallowed it. A +# reference the walk cannot spell is not therefore absent, and one this gate +# recommends is the worst shape to be blind to. +# +# Tried first, so `:@sha256:` is captured whole rather than as +# its `repo:tag` head, and the digest a container actually runs is what reaches +# the classifier. The 64-hex suffix is discriminator enough on its own, so the +# path separator and the tag restrictions the tag alternatives need are not +# repeated here: nothing else in a render has that shape. IMAGE_REF = re.compile( r"(?` splits at the `@` and the tail has the single-segment " - "shape", "localhost": "a memcached address in a Loki config value; `localhost:11211` is a host " "and a port", @@ -386,7 +429,8 @@ def keyed_images(node) -> set[str]: def extract_images(rendered: str, chart: str, - unscannable: list[tuple[str, str]], + unrendered: list[tuple[str, str]], + unclassified: list[tuple[str, str]], seen: set[str] | None = None) -> set[str]: """Every image one chart's render deploys, and an assertion that it is every one. @@ -407,7 +451,7 @@ def extract_images(rendered: str, chart: str, docs = list(yaml.safe_load_all(rendered)) except yaml.YAMLError as exc: first = str(exc).strip().splitlines()[0] - unscannable.append((chart, f"rendered YAML this gate could not parse — {first}")) + unrendered.append((chart, f"rendered YAML this gate could not parse — {first}")) return set() structural = keyed_images(docs) @@ -434,7 +478,7 @@ def extract_images(rendered: str, chart: str, if bare in CONTROLLER_IMAGES: controller.add(ref_str) continue - unscannable.append(( + unclassified.append(( chart, f"{ref_str} is image-shaped and reaches no pod template or `image:` key. " f"Either a structural walk stopped seeing a shape, or a controller is " @@ -473,6 +517,17 @@ def classify(ref: str) -> str: return "mutable" if tag.lower() in MUTABLE_TAGS else "tag" +# What an operator is told to write. Every reference form named here in +# backticks is one the walk above reads, asserted rather than assumed: advice +# pointing at a spelling this gate is blind to is worse than no advice, because +# following it moves an image out of the population and the run stays green. +MUTABLE_REMEDIATION = ( + "Pin it in the addon's values.yaml to the chart's appVersion, or to a digest — " + "`@sha256:` is a form this gate reads whole, whether it carries a " + "tag as well or not." +) + + def main() -> int: ap = argparse.ArgumentParser(description=__doc__) ap.add_argument("--list", action="store_true", help="print the image inventory") @@ -480,20 +535,23 @@ def main() -> int: args = ap.parse_args() seen: set[str] = set() - images, unscannable = inventory(args.env, seen) + images, unrendered, unclassified = inventory(args.env, seen) if args.list: for ref in sorted(images): print(f"{classify(ref):8} {ref} ({', '.join(sorted(images[ref]))})") - print(f"\n{len(images)} image(s); {len(unscannable)} chart(s) unscannable") - for path, err in unscannable: - print(f" unscannable {path}: {err}") + print(f"\n{len(images)} image(s); {len(unrendered)} chart(s) not rendered; " + f"{len(unclassified)} reference(s) not placed") + for path, err in unrendered: + print(f" not rendered {path}: {err}") + for path, err in unclassified: + print(f" not placed {path}: {err}") return 0 if not images: print("FAIL rendered no images at all. Every chart failed, or the extractor " "stopped matching — either way this reports the same as a clean fleet.") - for path, err in unscannable: + for path, err in unrendered: print(f" {path}: {err}") return 2 @@ -509,8 +567,8 @@ def main() -> int: if bare in ALLOWED_MUTABLE: continue failures.append( - f"{ref} (via {', '.join(sorted(images[ref]))}) resolves to a moving target. " - f"Pin it in the addon's values.yaml to the chart's appVersion or a digest.") + f"{ref} (via {', '.join(sorted(images[ref]))}) resolves to a moving " + f"target. {MUTABLE_REMEDIATION}") for bare, reason in sorted(ALLOWED_MUTABLE.items()): if bare not in mutable_seen: @@ -519,21 +577,39 @@ def main() -> int: f"renders it mutably — the exemption outlived its reason. Delete it. " f"(recorded: {reason[:100]})") - # Reported whatever the verdict: a chart that did not render contributed no - # images, and counting the rest as the whole fleet is how a partial scan - # reads as a complete one. - if unscannable: - print(f"{len(unscannable)} chart(s) could not be rendered and were NOT scanned:") - for path, err in unscannable: - print(f" {path}: {err}") - print() + # A reference the classifier cannot place is a verdict this gate owes and has + # not given: the chart rendered, the string is image-shaped, and whether + # anything runs it is unanswered. Printed under a heading about charts that + # did not render, it reached no verdict at all and the run exited 0 — + # correctness resting on a sibling gate reading the same list as a refusal, + # which is that gate holding the line rather than this one stating a result. + failures.extend(f"{chart}: {detail}" for chart, detail in unclassified) if failures: + if unrendered: + print(f"{len(unrendered)} chart(s) could not be rendered and were NOT " + f"scanned, so the population below is a subset of the fleet:") + for path, err in unrendered: + print(f" {path}: {err}") + print() print(f"{len(failures)} image-pin problem(s) across {len(images)} rendered image(s):\n") for f in failures: print(f" {f}") return 1 + # No failure among the images that WERE derived is not a verdict over the + # fleet when a chart contributed none. Exit 2 rather than 0: a chart that did + # not render is a gate that examined less than it reports on, which is not + # the same as finding nothing. + if unrendered: + print(f"Cannot run: {len(unrendered)} chart(s) could not be rendered and were " + f"NOT scanned:") + for path, err in unrendered: + print(f" {path}: {err}") + print(f"The {len(images)} image(s) that did render carry an immutable " + f"reference. That is a result over part of the fleet, not the fleet.") + return gatelib.CANNOT_RUN + print(f"✓ all {len(images)} rendered image(s) across {len(images and set().union(*images.values()) or [])} " f"chart(s) carry an immutable reference " f"({len(ALLOWED_MUTABLE)} exemption(s), each still matching the render)") diff --git a/scripts/check-image-vulnerabilities.py b/scripts/check-image-vulnerabilities.py index b90f025..ee76b26 100755 --- a/scripts/check-image-vulnerabilities.py +++ b/scripts/check-image-vulnerabilities.py @@ -342,12 +342,21 @@ def main() -> int: return 0 seen: set[str] = set() - images, unrendered = image_pins.inventory(args.env, seen) + images, unrendered, unclassified = image_pins.inventory(args.env, seen) if unrendered: cannot_run("Cannot run: charts that did not render contributed no images, so " "the scan below would cover part of the fleet and report on all of " "it.", *(f" {path}: {err}" for path, err in unrendered)) + # A reference the classifier could not place is one this scan cannot decide + # to include or exclude. Excluded silently it is an unscanned image inside a + # population reported as the fleet's — the same shape as a chart that did + # not render, one string down. + if unclassified: + cannot_run("Cannot run: the render carries image-shaped references that reach " + "no pod template and are on no declaration, so whether the scan " + "below covers them is unknown:", + *(f" {chart}: {detail}" for chart, detail in unclassified)) units = image_pins.render_addons.discover() coverage = image_pins.chart_coverage(images, units, seen) if coverage: diff --git a/scripts/tests/reverify-gates.sh b/scripts/tests/reverify-gates.sh index 04ff52f..21e549f 100755 --- a/scripts/tests/reverify-gates.sh +++ b/scripts/tests/reverify-gates.sh @@ -154,6 +154,40 @@ PY run nonzero "image-pins: unpinned readiness-checker" ./scripts/check-image-pins.py res $F +# What the gate cannot read is not therefore absent, and the two ways it can +# fail to read are two verdicts. A reference the classifier cannot place is a +# chart that rendered and a question this gate owes an answer to; a chart that +# did not render leaves the fleet's image set unknown. +# +# The digest form is the one the gate's own remediation recommends. Written with +# no tag it matched neither tag alternative, so the whole reference yielded only +# its `sha256:` tail — a single-segment shape a declaration by that name +# passed over. +F=addons/networking/external-dns/values.yaml; mut $F +python3 - "$F" <<'PY' +import pathlib,sys +p=pathlib.Path(sys.argv[1]); s=p.read_text() +a="podAnnotations:\n" +assert a in s, "podAnnotations anchor not found" +ref="ghcr.io/example/probe-worker@sha256:" + "9f8e" + "a"*60 +m=s.replace(a, a + f' probe/image: "{ref}"\n', 1) +assert m!=s, "mutation did not land" +p.write_text(m) +print(f" planted {ref[:52]}...") +PY +run nonzero "image-pins: a digest-only reference nothing declares" ./scripts/check-image-pins.py +res $F + +# A chart that contributes no images and is DECLARED to contribute none, so the +# per-chart floor passes over it and the unrendered verdict is what is being +# read. Any other chart would be caught by the floor first, which would score +# this probe green for a reason it did not plant. +F=addons/ai-platform/envoy-ai-gateway-crds/values.yaml; mut $F +printf '\nbroken: [unclosed\n' >> $F +echo " planted unparseable values for a chart declared imageless" +run nonzero "image-pins: a chart that did not render cannot report a clean fleet" ./scripts/check-image-pins.py +res $F + F=applicationsets/addons-agent-operator.yaml; mut $F python3 - "$F" <<'PY' import pathlib,sys @@ -273,7 +307,7 @@ echo "RESULT pass=$pass fail=$fail" # The harness owes the same assertion it demands of the gates: with every `run` # line deleted it would report pass=0 fail=0 and exit 0, which is a green run # over nothing checked. -MIN_CHECKS=25 +MIN_CHECKS=27 total=$((pass + fail)) if [ "$total" -lt "$MIN_CHECKS" ]; then echo "FAIL ran $total check(s), under the floor of $MIN_CHECKS — this harness" diff --git a/scripts/tests/run.py b/scripts/tests/run.py index 57bf7aa..82bf63f 100755 --- a/scripts/tests/run.py +++ b/scripts/tests/run.py @@ -79,7 +79,7 @@ # largest in the tree, and among them are the gates on the paths testing-rubric # calls security-critical. So this figure being low is not offset by behavioural # coverage for precisely the files where that offset was being claimed. -COMBINED_FLOOR = 20 +COMBINED_FLOOR = 22 # A ceiling on gate scripts carrying NO unit coverage at all, complementing the # floors below. The floors stop a covered file regressing; nothing stopped a NEW @@ -107,7 +107,7 @@ "scripts/check-sync-waves.py": 10, "scripts/gatelib.py": 55, "scripts/check-image-vulnerabilities.py": 41, - "scripts/check-image-pins.py": 47, + "scripts/check-image-pins.py": 71, } diff --git a/scripts/tests/test_image_extraction.py b/scripts/tests/test_image_extraction.py index 3a5ba57..5044d7f 100644 --- a/scripts/tests/test_image_extraction.py +++ b/scripts/tests/test_image_extraction.py @@ -14,6 +14,10 @@ from __future__ import annotations +import contextlib +import io +import re +import sys import unittest from gateloader import load @@ -48,8 +52,26 @@ def render(*docs: str) -> str: class ExtractingFromARender(unittest.TestCase): def images(self, rendered, chart="c"): - unscannable: list[tuple[str, str]] = [] - return gate.extract_images(rendered, chart, unscannable), unscannable + """(population, references the classifier could not place). + + The two out-parameters are separate because they are separate verdicts: + a render that will not parse leaves the fleet unknown, a reference that + will not classify is a question this gate owes an answer to. Every case + below that asserts `problems == []` is also asserting the render parsed, + so the two are checked apart. + """ + unrendered: list[tuple[str, str]] = [] + unclassified: list[tuple[str, str]] = [] + found = gate.extract_images(rendered, chart, unrendered, unclassified) + self.assertEqual(unrendered, [], "the fixture render did not parse") + return found, unclassified + + def both(self, rendered, chart="c"): + """(population, not-rendered, not-placed), for the cases that plant one.""" + unrendered: list[tuple[str, str]] = [] + unclassified: list[tuple[str, str]] = [] + found = gate.extract_images(rendered, chart, unrendered, unclassified) + return found, unrendered, unclassified def test_the_list_item_spelling_is_found(self): """The shape the pattern never matched, and the whole defect. @@ -223,10 +245,13 @@ def test_a_host_and_port_is_not_an_image(self): def test_a_render_that_will_not_parse_is_reported_not_dropped(self): """A chart silently absent from the inventory is the whole failure mode.""" - found, problems = self.images("kind: Deployment\n bad: [unclosed\n") + found, unrendered, unclassified = self.both("kind: Deployment\n bad: [unclosed\n") self.assertEqual(found, set()) - self.assertEqual(len(problems), 1) - self.assertIn("could not parse", problems[0][1]) + self.assertEqual(len(unrendered), 1) + self.assertIn("could not parse", unrendered[0][1]) + self.assertEqual(unclassified, [], + "a render that did not parse cannot also have produced a " + "reference the classifier could not place") def test_the_helm_value_sentinel_does_not_lose_the_chart(self): """Charts emit `- =` inside ConfigMap payloads; PyYAML has no constructor @@ -325,3 +350,227 @@ def test_an_empty_render_reports_every_declaration(self): problems = gate.declaration_rot(set()) self.assertEqual(len(problems), len(gate.CONTROLLER_IMAGES) + len(gate.NOT_A_CONTAINER)) + + +DIGEST = "sha256:" + "9f8e7d6c5b4a3f2e1d0c9b8a7f6e5d4c3b2a1f0e9d8c7b6a5f4e3d2c1b0a9f8e" + + +class ADigestPinnedReferenceIsInThePopulation(unittest.TestCase): + """`@sha256:` is a reference this gate recommends and could not read. + + The pattern under the structural walk had two alternatives and both required + a tag, so a digest-pinned reference in a controller flag or a config value + yielded nothing but its `sha256:` tail — a single-segment shape whose + bare name is `sha256`, and an entry by that name passed over it. The gate's + own remediation tells an operator to pin to a digest, so the one spelling it + recommends was the one its completeness floor could not see. + """ + + def candidates(self, s: str) -> list[str]: + return [c for groups in gate.IMAGE_REF.findall(s) for c in groups if c] + + def test_a_digest_only_reference_is_matched_whole(self): + self.assertEqual(self.candidates(f"ghcr.io/example/thing@{DIGEST}"), + [f"ghcr.io/example/thing@{DIGEST}"]) + + def test_a_tag_and_digest_reference_is_matched_whole(self): + """Not as its `repo:tag` head: what a container runs is the digest, and + the head alone reaches the classifier as a tag.""" + ref = f"ghcr.io/example/thing:1.2.3@{DIGEST}" + self.assertEqual(self.candidates(ref), [ref]) + self.assertEqual(gate.classify(ref), "digest") + + def test_a_single_segment_official_image_carries_a_digest(self): + """`nats@sha256:...` — no registry, no organisation, and the shape the + event-bus controller declares.""" + self.assertEqual(self.candidates(f"nats@{DIGEST}"), [f"nats@{DIGEST}"]) + + def test_the_bare_name_drops_both_tag_and_digest(self): + for ref in (f"ghcr.io/example/thing@{DIGEST}", + f"ghcr.io/example/thing:1.2.3@{DIGEST}", + "ghcr.io/example/thing:1.2.3"): + with self.subTest(ref=ref): + self.assertEqual(gate.bare_name(ref), "ghcr.io/example/thing") + + def test_a_digest_with_no_repository_before_it_is_still_seen(self): + """A `digest:` field carries one, and it names no image. + + Seen is the property, not excused. It is matched, so a render that + starts carrying one is reported as a reference this gate cannot place + rather than passed over — and the repair is a declaration saying why. + + No declaration stands for it while the render carries none, because a + NOT_A_CONTAINER entry excuses whatever has its bare name: an entry for + `sha256` covers every reference whose only matched token is a digest, + which is every digest-pinned reference if the pattern cannot spell the + repository-prefixed form. `declaration_rot` is what keeps that from + being written and forgotten — an entry matching nothing in the render + fails. + """ + self.assertEqual(self.candidates(DIGEST), [DIGEST]) + self.assertEqual(gate.bare_name(DIGEST), "sha256") + self.assertNotIn("sha256", gate.NOT_A_CONTAINER, + "a declaration matching nothing in the render is an " + "exemption that only ever widens") + + def test_a_digest_shaped_string_that_is_not_a_digest_is_not_matched(self): + """64 lowercase hex, because the suffix is the only discriminator the + digest alternative has — it drops the path separator and the tag rules + the others need.""" + for s in (f"ghcr.io/example/thing@sha256:{'a' * 63}", + "ghcr.io/example/thing@sha256:not-a-digest", + f"ghcr.io/example/thing@sha512:{'a' * 64}"): + with self.subTest(s=s): + self.assertNotIn(f"ghcr.io/example/thing@{s.split('@')[1]}", + self.candidates(s)) + + def test_a_digest_pinned_controller_reference_is_reported_when_undeclared(self): + """The finding, end to end. A controller is handed a digest-pinned + reference; no pod template declares it, no list names it, and before the + digest alternative the whole string produced nothing to report.""" + rendered = f""" +apiVersion: v1 +kind: ConfigMap +data: + controller.yaml: | + workerImage: ghcr.io/example/undeclared-worker@{DIGEST} +""" + unrendered: list[tuple[str, str]] = [] + unclassified: list[tuple[str, str]] = [] + gate.extract_images(rendered, "c", unrendered, unclassified) + self.assertEqual(unrendered, []) + self.assertEqual(len(unclassified), 1) + self.assertIn(f"ghcr.io/example/undeclared-worker@{DIGEST}", unclassified[0][1]) + + def test_a_digest_pinned_declared_controller_reference_is_in_the_population(self): + declared = next(iter(gate.CONTROLLER_IMAGES)) + rendered = f""" +apiVersion: v1 +kind: ConfigMap +data: + controller.yaml: | + image: {declared}@{DIGEST} +""" + unrendered: list[tuple[str, str]] = [] + unclassified: list[tuple[str, str]] = [] + found = gate.extract_images(rendered, "c", unrendered, unclassified) + self.assertEqual((unrendered, unclassified), ([], [])) + self.assertIn(f"{declared}@{DIGEST}", found) + self.assertEqual(gate.classify(f"{declared}@{DIGEST}"), "digest") + + def test_a_digest_pinned_container_is_read_by_the_structural_walk(self): + rendered = f""" +apiVersion: apps/v1 +kind: Deployment +spec: + template: + spec: + containers: + - name: app + image: ghcr.io/example/app@{DIGEST} +""" + unrendered: list[tuple[str, str]] = [] + unclassified: list[tuple[str, str]] = [] + found = gate.extract_images(rendered, "c", unrendered, unclassified) + self.assertEqual(found, {f"ghcr.io/example/app@{DIGEST}"}) + self.assertEqual((unrendered, unclassified), ([], [])) + + def test_every_form_the_remediation_recommends_is_one_this_reads(self): + """The sentence and the pattern, checked against each other. + + Read out of the gate's own remediation string rather than retyped here, + so rewording the advice to name a spelling the pattern does not read + fails — which is the defect this closes, one revision later. + """ + recommended = re.findall(r"`([^`]+)`", gate.MUTABLE_REMEDIATION) + self.assertTrue(recommended, + f"the remediation names no reference form: " + f"{gate.MUTABLE_REMEDIATION}") + for form in recommended: + with self.subTest(form=form): + concrete = (form.replace("", "ghcr.io/example/app") + .replace("", DIGEST.split(":", 1)[1])) + self.assertEqual(self.candidates(concrete), [concrete], + f"the remediation recommends {form}, which the " + f"pattern under the structural walk does not read") + + +class WhatCannotBeReadReachesAVerdict(unittest.TestCase): + """Neither silence, and not the same verdict. + + A chart that did not render leaves the fleet's image set unknown; a + reference the classifier cannot place is a chart that rendered and a + question this gate owes an answer to. Held in one list, the second printed + under the first's heading and reached no verdict at all — the run exited 0, + and CI blocked only because a sibling gate read the same list as a refusal. + """ + + IMAGES = {"ghcr.io/example/app:1.0.0": {"c"}} + MUTABLE = {"ghcr.io/example/app:latest": {"c"}} + + def verdict(self, images, unrendered=(), unclassified=()): + """main() over a planted inventory. + + chart_coverage and ALLOWED_MUTABLE are neutralised because both assert + against the real fleet: a fixture of two images would trip every + per-chart floor and every exemption-rot check, and the case would then + pass for a reason it did not plant. Each has its own tests elsewhere in + this file. + """ + saved = (gate.inventory, gate.chart_coverage, dict(gate.ALLOWED_MUTABLE), + sys.argv) + gate.inventory = lambda env, seen=None: (images, list(unrendered), + list(unclassified)) + gate.chart_coverage = lambda *a, **k: [] + gate.ALLOWED_MUTABLE.clear() + sys.argv = ["check-image-pins.py"] + try: + with contextlib.redirect_stdout(io.StringIO()) as out: + rc = gate.main() + finally: + gate.inventory, gate.chart_coverage, argv = saved[0], saved[1], saved[3] + gate.ALLOWED_MUTABLE.update(saved[2]) + sys.argv = argv + return rc, out.getvalue() + + def test_a_clean_fleet_passes(self): + """The control: without it every case below could be failing for a + reason the case did not introduce.""" + rc, out = self.verdict(self.IMAGES) + self.assertEqual(rc, 0, out) + + def test_a_reference_that_cannot_be_placed_fails(self): + rc, out = self.verdict(self.IMAGES, unclassified=[ + ("c", "ghcr.io/example/mystery:1.0 is image-shaped and reaches no pod " + "template or `image:` key.")]) + self.assertEqual(rc, 1) + self.assertIn("ghcr.io/example/mystery:1.0", out) + + def test_a_reference_that_cannot_be_placed_is_not_a_scope_caveat(self): + """It is printed as a problem, not under a heading about charts that did + not render — the chart rendered.""" + _, out = self.verdict(self.IMAGES, unclassified=[("c", "mystery")]) + self.assertNotIn("could not be rendered", out) + self.assertIn("image-pin problem", out) + + def test_a_chart_that_did_not_render_cannot_report_a_clean_fleet(self): + """Every image that DID render is immutable, and that is a result over + part of the fleet rather than over the fleet.""" + rc, out = self.verdict(self.IMAGES, unrendered=[("addons/x", "helm failed")]) + self.assertEqual(rc, gate.gatelib.CANNOT_RUN) + self.assertIn("Cannot run", out) + self.assertIn("addons/x", out) + + def test_a_chart_that_did_not_render_is_named_alongside_a_real_failure(self): + """The failure is the verdict, and the unrendered chart still bounds what + the verdict covers — dropping either one loses something a reader needs.""" + rc, out = self.verdict(self.MUTABLE, unrendered=[("addons/x", "helm failed")]) + self.assertEqual(rc, 1) + self.assertIn("addons/x", out) + self.assertIn("subset of the fleet", out) + self.assertIn("moving target", out) + + def test_an_empty_population_cannot_run(self): + rc, out = self.verdict({}, unrendered=[("addons/x", "helm failed")]) + self.assertEqual(rc, gate.gatelib.CANNOT_RUN) + self.assertIn("rendered no images at all", out) From 364cf5cccde34c2c952d84b13c29fb3018581bf1 Mon Sep 17 00:00:00 2001 From: stxkxs <139715017+stxkxs@users.noreply.github.com> Date: Fri, 4 Sep 2026 18:24:21 -0700 Subject: [PATCH 6/6] Give the host one parse, so a rendered string cannot hang the render gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL reported two high py/redos in IMAGE_REF, both in the registry part: [a-z0-9][a-z0-9._-]*(?:\.[a-z0-9._-]+)+ A starred class already carrying `.` and `-`, followed by a plus-quantified group whose body is that same class. Every way of splitting a run of `-.` between the two quantifiers is a distinct parse, and the engine tries all of them before failing. Measured on the flagged expression: `0.` followed by twenty `-.` repetitions — 43 characters — took two thirds of a second, and every four further repetitions multiplied that by fifteen. Sixty-seven characters is most of an hour. The input is rendered content from charts this repository does not author and this gate runs on every render, so the string arrives with a chart bump rather than with an attacker. The symptom is a job that never returns rather than one that fails, which is the same class as an unbounded read: it does not need an attacker to be a defect. CodeQL is advisory here rather than required, and this would have merged green. That is a reason to read it, not to skip it. ──────────────── The registry grammar has one definition of a host ──────────────── Closed by the grammar rather than by tuning the two lines. A domain COMPONENT carries no dot — dots are the separators — and neither starts nor ends with a dash: DOMAIN_COMPONENT = [a-z0-9]+(?:-+[a-z0-9]+)* REGISTRY = (?:COMPONENT(?:\.COMPONENT)+(?::\d+)?/)? `[a-z0-9]+` and `-+` are disjoint, so a run of dashes can only be a separator inside one component and a dot can only end one. Every input has exactly one parse or none, and the hostile string is linear: four thousand characters in half a millisecond. Written once and used by both alternatives that need it, because two spellings of one grammar drift. The digest alternative keeps its place first, so `:@sha256:` is still captured whole rather than as its `repo:tag` head. Two narrowings, both of which the grammar already required and neither of which this fleet's registries use: no underscore in a host, and no dot inside a component. Every reference the fleet renders parses unchanged. ──────────────── What proves it ──────────────── The regression cases assert COMPLETION, not output. A rewrite that reintroduces the ambiguity still matches the same references and still rejects the same non-references — the only thing it changes is how long it takes to say so, so time is the property. They run the match in a CHILD process with a timeout, because a matcher that never returns cannot be caught by an in-process timing assertion: it never reaches it. A hang is a failure with a message, in bounded time. Four mutants, each killed and named. The first is the flagged expression restored verbatim, and the two timing cases are what catch it. Also: a repeated component allowed to begin with a dash, which is the ambiguity on its own; a component allowed to start or end with a dash; and a dotless host admitted as a registry — that last one is a decision with a consequence, because `localhost:11211` in a Loki config reaches the walk as a single-segment token, which is why NOT_A_CONTAINER carries an entry for it, and admitting a dotless registry would leave that entry matching nothing. Verification: ruff clean; mypy clean over 39 files; 279 tests across 11 modules; coverage ratchet 30.1% (floor 29); check-image-pins.py 80 images across 28 charts; controls.py 17 controls; empty-corpus.py 29 probed gates; reverify-gates.sh 38/38; task validate exit 0; check-named-things.py 195 references resolve; check-workflows.sh clean at MEDIUM and above. Co-authored-by: stxkxsbot <275011021+stxkxsbot@users.noreply.github.com> --- scripts/check-image-pins.py | 25 +++++- scripts/tests/test_image_extraction.py | 113 +++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 2 deletions(-) diff --git a/scripts/check-image-pins.py b/scripts/check-image-pins.py index 308fdf1..8dc2c0b 100755 --- a/scripts/check-image-pins.py +++ b/scripts/check-image-pins.py @@ -270,6 +270,27 @@ def chart_artifacts(output: str) -> set[str]: # the classifier. The 64-hex suffix is discriminator enough on its own, so the # path separator and the tag restrictions the tag alternatives need are not # repeated here: nothing else in a render has that shape. +# The registry grammar's own definition of a host, written once because both +# alternatives below need it and two spellings of one grammar drift. +# +# A domain COMPONENT carries no dot: dots are the separators, and a component +# neither starts nor ends with a dash. That is what makes the host unambiguous, +# and unambiguous is what makes it safe. Spelled as a starred class containing +# `.` and `-` followed by a plus-quantified group beginning with `.` over that +# same class, every way of splitting a run of `-.` between the two quantifiers +# is a distinct parse, and the engine tries all of them before failing — 43 +# characters of `0.` and `-.` took a third of a second, 83 would not finish. +# The input here is rendered content from charts this repository does not +# author and the gate runs on every render, so a string that makes the matcher +# climb arrives with a chart bump, and the symptom is a job that never returns +# rather than one that fails. +# +# Here `[a-z0-9]+` and `-+` are disjoint, so a run of dashes can only be a +# separator inside one component and a dot can only end one. Every input has +# exactly one parse or none. +DOMAIN_COMPONENT = r"[a-z0-9]+(?:-+[a-z0-9]+)*" +REGISTRY = rf"(?:{DOMAIN_COMPONENT}(?:\.{DOMAIN_COMPONENT})+(?::\d+)?/)?" + IMAGE_REF = re.compile( r"(? set[str]: # exemption to keep re-reading — it is not a reference. r"(?!sha(?:256|512):[0-9a-f]{32,}(?![0-9a-z]))" r"(?:" - r"((?:[a-z0-9][a-z0-9._-]*(?:\.[a-z0-9._-]+)+(?::\d+)?/)?" + rf"({REGISTRY}" r"[a-z0-9][a-z0-9._-]*(?:/[a-z0-9][a-z0-9._-]*)*" r"(?::[a-zA-Z0-9][\w.-]*)?@sha256:[0-9a-f]{64})" - r"|((?:[a-z0-9][a-z0-9._-]*(?:\.[a-z0-9._-]+)+(?::\d+)?/)?" + rf"|({REGISTRY}" r"[a-z0-9][a-z0-9._-]*(?:/[a-z0-9][a-z0-9._-]*)+:[a-zA-Z0-9][\w.-]*)" r"|([a-z][a-z0-9]*(?:[_-][a-z0-9]+)*:(?:latest|v?[0-9][\w.-]*))" r")\b") diff --git a/scripts/tests/test_image_extraction.py b/scripts/tests/test_image_extraction.py index 11bd9e5..2903e75 100644 --- a/scripts/tests/test_image_extraction.py +++ b/scripts/tests/test_image_extraction.py @@ -18,6 +18,7 @@ import io import pathlib import re +import subprocess import sys import tempfile import unittest @@ -25,6 +26,7 @@ from gateloader import load gate = load("check-image-pins") +GATE = pathlib.Path(gate.__file__) class Unit: @@ -781,3 +783,114 @@ def test_no_report_at_all_leaves_the_render_unchanged(self): images, unrendered, unclassified = self.inventory_with(DEPLOYMENT, "") self.assertEqual((unrendered, unclassified), ([], [])) self.assertEqual(images, {"ghcr.io/example/app:1.0.0": {"karpenter"}}) + + +# Run the pattern in a child, so a pattern that does not return FAILS instead of +# hanging the suite. An in-process timing assertion cannot be reached by a +# matcher that never finishes — the very case it exists to catch. +_MATCH_IN_CHILD = """ +import importlib.util, sys +spec = importlib.util.spec_from_file_location("g", sys.argv[1]) +g = importlib.util.module_from_spec(spec) +spec.loader.exec_module(g) +found = [c for grp in g.IMAGE_REF.findall(sys.argv[2]) for c in grp if c] +print(len(found)) +""" + + +class TheHostGrammarHasOneParse(unittest.TestCase): + """A reference either parses one way or not at all, and it says so in time. + + Written as a starred class containing `.` and `-` followed by a + plus-quantified group whose body is that same class, a run of `-.` can be + split between the two quantifiers every possible way and the engine tries + all of them before failing. That is not a wrong answer, it is an answer that + never arrives: 43 characters took two thirds of a second and every four + further repetitions multiplied it by fifteen. + + The input is rendered content from charts this repository does not author + and this gate runs on every render, so the string arrives with a chart bump + rather than with an attacker, and the symptom is a job that never returns. + + So the registry grammar's own definition is used instead. A domain + COMPONENT carries no dot — dots are the separators — and neither starts nor + ends with a dash, which leaves every input exactly one parse or none. + """ + + # 32 repetitions, 67 characters. Under the ambiguous spelling the + # 43-character case multiplied by fifteen three times over, which is most of + # an hour; the fixed one answers in microseconds. Nothing between those two + # is reachable by a slow machine. + HOSTILE = "0." + "-." * 32 + "!" + BUDGET_SECONDS = 20 + + def completes(self, text): + """The number of references found, or a failure if the match hangs.""" + try: + proc = subprocess.run( + [sys.executable, "-c", _MATCH_IN_CHILD, str(GATE), text], + capture_output=True, text=True, timeout=self.BUDGET_SECONDS) + except subprocess.TimeoutExpired: + self.fail(f"IMAGE_REF did not finish on {len(text)} characters within " + f"{self.BUDGET_SECONDS}s — the host alternatives have more " + f"than one parse again") + self.assertEqual(proc.returncode, 0, proc.stderr) + return int(proc.stdout.strip()) + + def test_a_hostile_host_string_completes_and_matches_nothing(self): + self.assertEqual(self.completes(self.HOSTILE), 0) + + def test_ten_times_the_hostile_input_still_completes(self): + """One length can pass on a fast machine with a matcher that is merely + slow; ten times the input inside the same budget cannot.""" + self.assertEqual(self.completes("0." + "-." * 320 + "!"), 0) + + def test_a_hostile_string_inside_a_render_completes(self): + """Delivered the way a chart delivers one — a value in a ConfigMap.""" + rendered = ("apiVersion: v1\\nkind: ConfigMap\\ndata:\\n" + f" cfg: |\\n upstream: {self.HOSTILE}\\n") + unclassified: list[tuple[str, str]] = [] + gate.extract_images(rendered, "c", [], unclassified) + self.assertEqual(unclassified, []) + + def test_the_references_this_fleet_renders_still_parse(self): + """The grammar narrowed, so what it must still admit is read off the + population rather than a list written here.""" + for ref in sorted(gate.CONTROLLER_IMAGES) + sorted(gate.ALLOWED_MUTABLE): + with self.subTest(ref=ref): + concrete = f"{ref}:1.0.0" + self.assertEqual( + [c for grp in gate.IMAGE_REF.findall(concrete) for c in grp if c], + [concrete]) + + def test_a_domain_component_carries_no_dot_of_its_own(self): + """Dots separate components and never sit inside one. That is the + registry grammar, and it is what leaves a single parse.""" + self.assertRegex("ghcr.io", f"^{gate.DOMAIN_COMPONENT}\\.{gate.DOMAIN_COMPONENT}$") + self.assertNotRegex("gh.cr", f"^{gate.DOMAIN_COMPONENT}$") + + def test_a_component_neither_starts_nor_ends_with_a_dash(self): + for bad in ("-ghcr", "ghcr-", "-", "gh--cr-"): + with self.subTest(component=bad): + self.assertNotRegex(bad, f"^{gate.DOMAIN_COMPONENT}$") + self.assertRegex("gh--cr", f"^{gate.DOMAIN_COMPONENT}$") + + def test_a_dotless_host_is_not_a_registry(self): + """A host needs more than one component, and that is a decision with a + consequence rather than a detail of the grammar. + + `localhost:11211` in a Loki config is a host and a port, and it reaches + this walk as a single-segment token — which is why NOT_A_CONTAINER + carries an entry for it. Admitting a dotless host as a registry would + make `localhost:5000/x/y:1.0` one reference instead, and the entry that + records why the address is not an image would stop matching anything. + """ + self.assertEqual( + [c for grp in gate.IMAGE_REF.findall("localhost:5000/x/y:1.0") + for c in grp if c], + ["localhost:5000"]) + self.assertIn("localhost", gate.NOT_A_CONTAINER) + self.assertEqual( + [c for grp in gate.IMAGE_REF.findall("registry.local:5000/x/y:1.0") + for c in grp if c], + ["registry.local:5000/x/y:1.0"])