test: harden the CEL guard and name the Job generation in its pin #172

Merged
binjovi-bot merged 1 commit from test/vap-cel-balance-hardening into trunk 2026-09-02 03:27:08 +00:00
Owner

Why

An adversarial review of #168 (three independent reviewers, each finding verified by a refuter against cel-go v0.26.0 / k8s.io/apiserver v0.36.0, the parser the apiserver runs) found holes in tests/vap-cel-balance.sh. The guard was fail-closed on today's tree, but it could miss the class it was written for and it could go red on valid CEL. Every hole is now a fixture the self-test must catch before the guard judges the repository.

Lexer (false negatives first)

  • Line break inside a '…', "…" or r'…' string. cel-go excludes \n/\r from single-quoted strings; only the triple-quoted forms span lines. The lexer accepted it. Reachable today: the >- style every policy under kyverno-policies/ uses puts a line break inside the string as soon as a continuation line is more-indented. Verified: rewrapping one messageExpression in binjovi-image-digest-pinned that way was green before and is string opened with " has a line break in it now.
  • // comments. They are CEL (COMMENT : '//' (~'\n')* -> channel(HIDDEN)). Un-skipped, a ) in a comment masked a missing ) (verified in the incident's own variable, publisherCredential: comment added, one real ) removed, guard green; now red with 1 unclosed: (), and an apostrophe in a comment went red on a valid policy (now green).
  • br'…' / bR"…" raw bytes literals were read as escaped strings.

Discovery and extraction

  • A policy inside a kind: List was discovered by grep and dropped by yq, so it was never judged; kustomize flattens a List, so its items are applied. extract now opens Lists.
  • A quoted kind, a trailing comment, or a .yml file escaped the grep. Loosened to what yq's select accepts.
  • A YAML file yq cannot read printed ok: 64 … and then the bracket advice (pipefail made the pipeline red after judge had already spoken). yq now runs to completion into a file first and is named as the failure: FAIL: yq could not read an admission policy file named above; repair that YAML first.
  • A map-shaped variables: crashed the judge with a TypeError traceback instead of a FAIL line.

Self-test

The broken fixture now carries eight defects (short bracket, open string, crossed brackets, split string, comment-masked bracket, messageExpression, auditAnnotations, a List-wrapped MutatingAdmissionPolicy) and the whole fixture twelve valid expressions across a VAP and a MutatingAdmissionPolicy (nested, escaped, raw, raw bytes, triple-quoted over two lines, // comment with a quote and a bracket in it, map/list literals, auditAnnotations, applyConfiguration and jsonPatch). The judge must also refuse the whole fixture at a minimum of 13. Ten lexer/extractor mutants each turn the self-test red (no comment handling, no line-break check, no bytes prefix, auditAnnotations/mutations/matchConditions/messageExpression branch dropped, minimum ignored, List not opened, lexer always None).

Pin (tests/binjovi.sh)

The review showed the fingerprint pin is bypassable in exactly the shape of 18db1c1f: change the template, keep v15, re-pin the new checksum. A snapshot test cannot see history, so the pin now carries the generation name with the checksum (binjovi-forgejo-integration-v15 1955159150 12636), refuses an empty template (a renamed kind used to fingerprint as 4294967295 0), and its message says not to re-pin under the old name. The step still rests on the editor; the durable alternative is kustomize.toolkit.fluxcd.io/force: enabled on the Job, as binjovi-release-cli-oauth-v2 in the same component already carries — a design call for a Job with PGUSER=forgejo power, so not made here.

Verified

  • Clean tree: ok: 64 CEL expressions in 6 admission policies close what they open, 1 s in the admission image; tests/binjovi.sh green locally and in the image; full tests/check.sh green in the image.
  • Red on: the 18db1c1f credential guard, the split-string rewrap, the comment-masked bracket, a quoted/commented kind in a .yml, a List-wrapped policy, a tab-indented file, a map-shaped variables:, a one-character template edit without a rename.
  • Not checked by this guard, by design (header says so): invalid escape sequences and a stray backslash outside a string. It is not a CEL compiler.

https://claude.ai/code/session_01LeVatedQ4sCA1u6LHjo7B5

## Why An adversarial review of #168 (three independent reviewers, each finding verified by a refuter against cel-go v0.26.0 / k8s.io/apiserver v0.36.0, the parser the apiserver runs) found holes in `tests/vap-cel-balance.sh`. The guard was fail-closed on today's tree, but it could miss the class it was written for and it could go red on valid CEL. Every hole is now a fixture the self-test must catch before the guard judges the repository. ## Lexer (false negatives first) - **Line break inside a `'…'`, `"…"` or `r'…'` string.** cel-go excludes `\n`/`\r` from single-quoted strings; only the triple-quoted forms span lines. The lexer accepted it. Reachable today: the `>-` style every policy under `kyverno-policies/` uses puts a line break inside the string as soon as a continuation line is more-indented. Verified: rewrapping one `messageExpression` in `binjovi-image-digest-pinned` that way was green before and is `string opened with " has a line break in it` now. - **`//` comments.** They are CEL (`COMMENT : '//' (~'\n')* -> channel(HIDDEN)`). Un-skipped, a `)` in a comment masked a missing `)` (verified in the incident's own variable, `publisherCredential`: comment added, one real `)` removed, guard green; now red with `1 unclosed: (`), and an apostrophe in a comment went red on a valid policy (now green). - **`br'…'` / `bR"…"` raw bytes literals** were read as escaped strings. ## Discovery and extraction - A policy inside a **`kind: List`** was discovered by grep and dropped by yq, so it was never judged; kustomize flattens a List, so its items are applied. `extract` now opens Lists. - A **quoted kind, a trailing comment, or a `.yml` file** escaped the grep. Loosened to what yq's `select` accepts. - A **YAML file yq cannot read** printed `ok: 64 …` and then the bracket advice (pipefail made the pipeline red after judge had already spoken). yq now runs to completion into a file first and is named as the failure: `FAIL: yq could not read an admission policy file named above; repair that YAML first`. - A **map-shaped `variables:`** crashed the judge with a `TypeError` traceback instead of a FAIL line. ## Self-test The broken fixture now carries eight defects (short bracket, open string, crossed brackets, split string, comment-masked bracket, messageExpression, auditAnnotations, a List-wrapped MutatingAdmissionPolicy) and the whole fixture twelve valid expressions across a VAP and a MutatingAdmissionPolicy (nested, escaped, raw, raw bytes, triple-quoted over two lines, `//` comment with a quote and a bracket in it, map/list literals, auditAnnotations, applyConfiguration and jsonPatch). The judge must also refuse the whole fixture at a minimum of 13. Ten lexer/extractor mutants each turn the self-test red (no comment handling, no line-break check, no bytes prefix, auditAnnotations/mutations/matchConditions/messageExpression branch dropped, minimum ignored, List not opened, lexer always None). ## Pin (`tests/binjovi.sh`) The review showed the fingerprint pin is bypassable in exactly the shape of 18db1c1f: change the template, keep `v15`, re-pin the new checksum. A snapshot test cannot see history, so the pin now carries the generation name with the checksum (`binjovi-forgejo-integration-v15 1955159150 12636`), refuses an empty template (a renamed kind used to fingerprint as `4294967295 0`), and its message says not to re-pin under the old name. The step still rests on the editor; the durable alternative is `kustomize.toolkit.fluxcd.io/force: enabled` on the Job, as `binjovi-release-cli-oauth-v2` in the same component already carries — a design call for a Job with `PGUSER=forgejo` power, so not made here. ## Verified - Clean tree: `ok: 64 CEL expressions in 6 admission policies close what they open`, 1 s in the admission image; `tests/binjovi.sh` green locally and in the image; full `tests/check.sh` green in the image. - Red on: the 18db1c1f credential guard, the split-string rewrap, the comment-masked bracket, a quoted/commented kind in a `.yml`, a List-wrapped policy, a tab-indented file, a map-shaped `variables:`, a one-character template edit without a rename. - Not checked by this guard, by design (header says so): invalid escape sequences and a stray backslash outside a string. It is not a CEL compiler. https://claude.ai/code/session_01LeVatedQ4sCA1u6LHjo7B5
test: harden the CEL guard and name the Job generation in its pin
All checks were successful
binjovi/ci Binjovi completed the frozen plan
12d5fcc05d
Adversarial review of #168 found holes in tests/vap-cel-balance.sh. Each one
is now a fixture the self-test must catch before the guard judges the tree:

- a line break inside a '…', "…" or r'…' string passed the lexer; the apiserver
  refuses it, and the repository's own `>-` style with a more-indented line
  produces exactly that (false negative, reachable today)
- `//` comments were lexed as code: a ')' in a comment masked a missing ')'
  (false negative) and an apostrophe in a comment went red (false positive)
- br'…' / bR"…" raw bytes literals were read as escaped strings
- a policy inside a `kind: List` was discovered by grep and dropped by yq,
  so it was never judged; kustomize flattens a List, so its items are applied
- a quoted kind, a trailing comment or a .yml file escaped discovery
- a YAML file yq could not read printed an `ok:` line and then the bracket
  advice; yq now runs to completion first and is named as the failure
- a map-shaped `variables:` crashed the judge with a traceback
- the self-test never exercised auditAnnotations, mutations or the minimum

tests/binjovi.sh pins the Job generation name together with the template
fingerprint, refuses an empty template, and says not to re-pin under the old
name: a snapshot test cannot see history, so that step rests on the editor.

Claude-Session: https://claude.ai/code/session_01LeVatedQ4sCA1u6LHjo7B5
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
sean/seanfarm!172
No description provided.