From 377fa205ea0aca09ccbdf75773f38d43dae52647 Mon Sep 17 00:00:00 2001 From: Keith Jackson Date: Sun, 13 Sep 2026 08:51:58 -0500 Subject: [PATCH 1/2] WIP: catch envsubst collapsing a doubled dollar in substituted paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit INCOMPLETE — preserved by the PM after the authoring agent hit the session rate limit mid-task. Do not merge without finishing and gating it. Closes the inverse of what this check already covered. It asserted that every ${...} in a substituted path is defined or escaped; it did NOT catch content containing a literal doubled dollar that the author needs delivered verbatim and that envsubst silently halves. That shipped a broken CronJob (fixed in 16b7fcd): Postgres dollar-quoting became a single dollar and every Flux-reconciled run died with a syntax error, while passing review because verification used kubectl apply, which runs no envsubst. Introduces a dollar-collapse.allow allowlist so each legitimate use is a reviewed decision rather than an accident. WHAT REMAINS: the agent's own next step was proving it catches the CLASS and not just this instance, via a scratch manifest under a substituted path. Its scratch fixture (apps/wave-2/zz-scratch-class.yaml and the wave-2 kustomization edit) was deliberately temporary and has been reverted here, so the fail-proof is NOT yet demonstrated. Also unanswered: brief point 3, on whether the check can say anything about a manifest verified only through a non-substituting path. Co-Authored-By: Claude Opus 5 --- .../chainsaw-test.yaml | 39 ++- .../flux-postbuild-substitution/check.sh | 272 +++++++++++++++++- .../dollar-collapse.allow | 50 ++++ 3 files changed, 353 insertions(+), 8 deletions(-) create mode 100644 conformance/tests/l0-static/flux-postbuild-substitution/dollar-collapse.allow diff --git a/conformance/tests/l0-static/flux-postbuild-substitution/chainsaw-test.yaml b/conformance/tests/l0-static/flux-postbuild-substitution/chainsaw-test.yaml index ab928a3..aada01e 100644 --- a/conformance/tests/l0-static/flux-postbuild-substitution/chainsaw-test.yaml +++ b/conformance/tests/l0-static/flux-postbuild-substitution/chainsaw-test.yaml @@ -8,6 +8,18 @@ # the empty string while the Kustomization still reports Ready — which emptied Cilium's # k8sServiceHost and broke the CNI on 2/3 dev nodes (DECISIONS-LOG 2026-09-01). # +# It is also LOSSY IN THE OTHER DIRECTION, and that is the second incident on this record. +# envsubst consumes dollars in PAIRS and emits one per pair, unconditionally and everywhere — +# so no substituted path can carry a literal `$$` at all. On 2026-09-13 a ConfigMap under +# apps-wave-0 shipped a dollar-quoted PL/pgSQL `DO $$ ... $$;` self-check; Postgres received +# a half-quoted `DO $` and every Flux-reconciled run of the nextcloud-db grant reconciler died +# with a syntax error, nine Error pods deep before anyone looked (commit 16b7fcd). It passed +# review because it was hand-applied with `kubectl apply`, which runs no envsubst: the defect +# existed ONLY on the rendered path, and it landed on the assertion whose whole job was to +# make a silent regression visible. Nothing here failed — the manifest rendered +# "successfully" into something broken, because the check treated any doubled dollar as a +# deliberate escape and stopped looking. +# # GIVEN every Flux Kustomization under clusters/ that declares postBuild.substituteFrom, # WHEN each `spec.path` is resolved to the exact file set it renders (following # kustomization.yaml `resources:` transitively, not globbing the repo), @@ -15,7 +27,12 @@ # or deliberately ESCAPED `$${...}`; no unescaped expansion carries a default # operator on an undefined name (the silent-empty class strict mode does NOT catch); # no unescaped expansion is unparseable by envsubst (which fails the WHOLE -# Kustomization); every path rendering a knob actually substitutes cluster-vars; +# Kustomization); NO RUN OF TWO OR MORE DOLLARS IS SILENTLY REWRITTEN — the only shape +# that passes unremarked is exactly `$${`, the documented total escape, and every other +# run (a bare `$$` that cannot survive verbatim, an ODD `$$${...}` that only LOOKS +# escaped and in fact expands, an over-counted `$$$${...}`) is named and justified in +# `dollar-collapse.allow`, which is itself held to being exact and non-stale; +# every path rendering a knob actually substitutes cluster-vars; # every overlay-patched knob has a base default; and the check is wired into the # merge gate as a blocking, unconditionally-run stage. # @@ -26,6 +43,19 @@ # only); conformance/run.sh --layer l0 and ci/l0.sh run it directly, and this Chainsaw # Test wraps the SAME script. # +# NOT proven, and deliberately NOT proxied: the wider lesson of 2026-09-13 is that the bug +# survived review because verification used a path (`kubectl apply`) that runs no +# substitution. That property is NOT statically decidable from this repo — which command +# someone ran is a fact about a shell session, and git stores the manifest, never the +# invocation. Grepping commit prose or runbooks for `kubectl apply` would be trivially +# satisfiable by rewording and would fire on the break-glass runbooks whose whole content is +# that command, so no such proxy is attempted. The dollar-run assertion is the honest answer +# instead: it moves the proof off the verification PATH and onto the SOURCE, where it IS +# decidable, so the mangling is caught by reading the manifest however a human chose to try +# it. An end-to-end `kustomize build | flux envsubst --strict` per substituted path would be +# the real thing, and it is how the arithmetic in check.sh's header was measured — but the L0 +# CI toolchain installs neither tool, so as a gate stage it would SKIP permanently. +# # NOT proven (named in check.sh's header): this is a STATIC scan of the SOURCE manifests # a path includes — a deliberate superset of the post-`kustomize build` stream. It does # not render, does not prove the feature gate is enabled on the live kustomize-controller, @@ -63,13 +93,16 @@ metadata: conformance.home.k3s/intent: >- Every variable a substituted Flux path renders is defined in cluster-vars or escaped, so post-build substitution cannot silently empty a value and enabling - StrictPostBuildSubstitutions cannot fail a legitimate manifest. + StrictPostBuildSubstitutions cannot fail a legitimate manifest; and no dollar run + envsubst rewrites reaches the cluster unreviewed, so content that must arrive + verbatim cannot be silently mangled on the Flux-rendered path (16b7fcd). spec: steps: - name: audit-every-substituted-flux-path description: >- Resolve each postBuild.substituteFrom Kustomization to the files it renders and - assert every ${...} is defined or escaped, then assert the gate wires it blocking. + assert every ${...} is defined or escaped and no dollar run is silently collapsed, + then assert the gate wires it blocking. try: - script: content: bash ./check.sh diff --git a/conformance/tests/l0-static/flux-postbuild-substitution/check.sh b/conformance/tests/l0-static/flux-postbuild-substitution/check.sh index a0000eb..b75754a 100755 --- a/conformance/tests/l0-static/flux-postbuild-substitution/check.sh +++ b/conformance/tests/l0-static/flux-postbuild-substitution/check.sh @@ -18,6 +18,32 @@ # substituted path renders is either DEFINED or deliberately ESCAPED — and nothing in # the repo asserted that. This check does. # +# SECOND INCIDENT, 2026-09-13 — THE INVERSE HAZARD (commit 16b7fcd) +# ------------------------------------------------------------------ +# The first incident was a variable the author wanted EXPANDED that rendered EMPTY. This one +# is its mirror: content the author wanted to reach the workload UNTOUCHED that substitution +# silently REWROTE. A CronJob reconciler in apps/wave-0/nextcloud-db.yaml asserted its own +# convergence in a dollar-quoted PL/pgSQL block — +# +# DO BEGIN ... END ; where is two literal dollars: standard Postgres +# +# — inside a ConfigMap under apps-wave-0, which declares postBuild.substituteFrom. envsubst +# collapses a doubled dollar into ONE dollar, ALWAYS, so Postgres received a half-quoted +# `DO ` and died with `syntax error at or near "$"`. Three scheduled runs left +# nine Error pods in nextcloud-db before anyone looked. +# +# It PASSED verification because it was hand-applied with `kubectl apply`, which runs NO +# envsubst. The defect existed ONLY on the Flux-rendered path. And the damage landed +# precisely on the self-check: the GRANT on line 1 still ran, so the privilege stayed +# correct and the only thing broken was the assertion meant to make a silent regression +# visible — the part whose whole job is to stop a blind spot being quiet. +# +# THE ASSERTION GAP THIS CLOSES: assertions (1)-(3) below ask whether every `${...}` is +# defined, escaped, or parseable. They said NOTHING about a doubled dollar that is not an +# escape, because the old classifier treated a `$` run of length >= 2 as "escaped, therefore +# fine" and stopped looking. Nothing failed the gate; the manifest rendered "successfully" +# into something broken. Assertion (7) is that hole, closed. +# # ENVSUBST BEHAVIOUR ASSERTED HERE (empirically verified against the pinned toolchain, # `flux envsubst`, flux 2.4.0 — this is measured behaviour, not assumption): # ${DEFINED} -> substituted with the cluster-vars value. @@ -33,6 +59,47 @@ # shell/cloud-init consumer. Escaping is total: it neutralises # even the indexed form. # $VAR, $(cmd) -> left alone by envsubst; they need no escaping. +# +# DOLLAR-RUN ARITHMETIC — the exact rule, MEASURED, not inferred (see assertion (7)) +# --------------------------------------------------------------------------------- +# envsubst here is `github.com/drone/envsubst`, vendored by fluxcd/pkg/kustomize and exposed +# by the `flux envsubst` CLI ("replicate the behavior of the Flux Kustomization post-build +# substitutions" — its own --help). Its scanner walks the input left to right; on a `$` it +# looks at the NEXT byte: +# * another `$` -> BOTH are consumed and ONE `$` is emitted. This is UNCONDITIONAL. What +# follows the pair is irrelevant — `{`, a letter, a digit, a semicolon, +# a newline, end of input. `$$` is not "the escape for `${`"; it is the +# escape for `$` itself, everywhere. +# * `{` -> the braced expression is parsed and EXPANDED. +# * anything else -> the `$` is emitted literally (hence `$VAR` and `$(cmd)` survive). +# So a run of N consecutive dollars emits floor(N/2) literal dollars, and when N is ODD the +# final dollar is still LIVE. Measured with flux 2.4.0 `flux envsubst`, DEFINED=VALUE — the +# left column is authored bytes, the right is what reaches the cluster: +# +# $ -> $ ${DEFINED} -> VALUE +# $$ -> $ $${DEFINED} -> ${DEFINED} <- the escape +# $$$ -> $$ $$${DEFINED} -> $VALUE <- ODD: brace LIVE +# $$$$ -> $$ $$$${DEFINED} -> $${DEFINED} +# $$$$$ -> $$$ $$$$${DEFINED} -> $$VALUE +# DO $$ .. $$; -> DO $ .. $; $$${NOPE:-d} -> $d +# awk '{print $$2}' -> awk '{print $2}' $${ARR[0]} -> ${ARR[0]} (safe) +# $$@ $$< $$^ -> $@ $< $^ $$${ARR[0]} -> HARD PARSE ERROR, whole +# Kustomization dies +# +# TWO CONSEQUENCES THIS CHECK IS BUILT ON: +# (a) THERE IS NO WAY TO SHIP A LITERAL `$$` THROUGH A SUBSTITUTED PATH. To make two +# dollars arrive you must author FOUR. So any authored `$$` that is meant to reach the +# workload verbatim is a BUG, full stop — that is the 2026-09-13 incident, and Postgres +# dollar-quoting is its commonest shape (Makefile `$$@`, awk `$$2` and docker-compose +# `$${}` are the same hazard arriving from the other direction: there the collapse is +# what the author wanted). +# (b) `$$$` + brace is NOT "more escaped" than `$$` + brace — it is NOT escaped at all. An +# odd run leaves one live dollar, so the brace expands, or, for an inner envsubst +# cannot parse, fails the ENTIRE Kustomization (proven above with $$${ARR[0]}, which +# aborted the whole stream while $${ARR[0]} on the line before it was fine). +# `--strict` reports NONE of this: every row of that table except the parse error was +# accepted SILENTLY by `flux envsubst --strict`, exit 0. Strict mode cannot see this class, +# so a source-level assertion is the only available guard. # ${...}, ${a b}, ${ARR[0]} (UNESCAPED) -> HARD PARSE ERROR ("unable to parse # variable name" / "missing closing brace") which fails the # WHOLE Kustomization — every resource in the path, not just @@ -55,6 +122,12 @@ # (5) every knob an overlay patches (`/data/KEY`) exists in the BASE ConfigMap — # the "add the knob with a base default BEFORE referencing it" rule that the # incident's interim discipline depends on; +# (7) NO DOLLAR RUN IS SILENTLY REWRITTEN: the only run of length >= 2 that passes +# unremarked is exactly `$$` immediately followed by `{` — the documented, total, +# already-inventoried escape. EVERY other run of >= 2 dollars (a bare `$$`, an odd +# `$$$`, an over-counted `$$$$`) is rewritten by envsubst between git and the +# container, so it must be named and justified in `dollar-collapse.allow`, a file +# this check also holds to being exact and non-stale; # AND (6) this check is LOAD-BEARING: it DECLARES `conformance.home.k3s/scope: whole-repo`, # so ci/l0.sh's `--include-scope whole-repo` runs it on every gate run, and # ci/gate.sh sets the merge block when the l0 stage fails. @@ -79,6 +152,21 @@ # kustomize-controller. Both are cluster facts (L2), untested here. Tagging # R-GITOPS-01 claims ONLY the static slice: what git says a Kustomization renders # is fully determined, with no variable silently emptied on the way in. +# - It CANNOT prove the thing that actually let 16b7fcd's bug reach a cluster: that a +# change was VERIFIED through a substituting path. "It was hand-applied with kubectl" is +# a fact about a shell session, not about any tracked file — git records the manifest, +# never the command someone ran to try it, so the property is not statically decidable +# here and no honest formulation of it exists. Nor is a proxy worth having: grepping +# commit prose or runbooks for `kubectl apply` would be trivially satisfiable by +# rewording, and would fire on the many legitimate break-glass runbooks whose whole +# content is that command. So this check does not attempt it, and does not pretend to. +# Assertion (7) answers that incident a different way: it moves the proof off the +# verification PATH, which is unknowable, and onto the SOURCE, which is not — the +# mangling is now caught by reading the manifest, whichever way a human chose to try it. +# (The one decidable half-measure, `kustomize build | flux envsubst --strict` per +# substituted path in CI, is already ruled out above: the L0 toolchain installs neither +# tool, so the stage would SKIP permanently. The dev image has both, and it is how the +# arithmetic table above was measured.) # - It does NOT validate cluster-vars VALUES (that GATEWAY_VIP is inside the LB pool, # that an overlay's LAN block is self-consistent). Presence, not correctness. # - A cluster-vars key nothing references is reported as an ADVISORY, never a failure: @@ -108,6 +196,7 @@ definition of "what a Flux path actually renders" in the harness. """ from __future__ import annotations +import bisect import os import re import sys @@ -231,9 +320,18 @@ LEN_RE = re.compile(r"^#([A-Za-z_][A-Za-z0-9_]*)$") def classify(dollars: int, inner: str) -> tuple[str, str] | None: - """Return (severity-tag, remediation) for a hazardous expansion, else None.""" - if dollars >= 2: - return None # escaped: emitted as a literal + """Return (severity-tag, remediation) for a hazardous expansion, else None. + + PARITY, not ">= 2" (the 2026-09-13 correction). envsubst consumes dollars in PAIRS and + emits one per pair, so a run of N dollars in front of a brace leaves the brace ESCAPED + only when N is EVEN. An ODD run emits floor(N/2) literal dollars and its last dollar is + still LIVE, so `$$${X}` expands exactly as `${X}` does — and `$$${ARR[0]}` fails the + whole Kustomization where `$${ARR[0]}` is harmless. The old `dollars >= 2 -> return None` + called every such run "escaped" and stopped looking. Assertion (7) flags the run itself; + this function additionally classifies the still-live expansion an odd run carries. + """ + if dollars % 2 == 0: + return None # fully escaped: emitted as a literal if NAME_RE.match(inner): if inner in KEYS: return None # defined knob — the happy path @@ -268,6 +366,9 @@ def rendered_files(path_spec: str) -> tuple[list[str], bool]: findings: list[str] = [] referenced: dict[str, set[str]] = {} inventory: list[str] = [] +# rel path -> the Kustomization name(s) that render it, for assertion (7)'s second pass. +# A file can be rendered by more than one Kustomization; all of them substitute. +subst_files: dict[str, set[str]] = {} for k in kustomizations: files, exists = rendered_files(k["path"]) @@ -281,13 +382,14 @@ for k in kustomizations: text = f.read_text() except (OSError, UnicodeDecodeError): continue + subst_files.setdefault(rel, set()).add(k["name"]) for lineno, line in enumerate(text.splitlines(), 1): for m in TOKEN_RE.finditer(line): dollars, inner = len(m.group(1)), m.group(2) verdict = classify(dollars, inner) if verdict is None: - if dollars >= 2: - n_esc += 1 + if dollars % 2 == 0: # PARITY, see classify(): an odd run is + n_esc += 1 # live, not escaped else: n_def += 1 referenced.setdefault(re.split(r"[:/#%^,]", inner)[0] or inner, @@ -313,6 +415,156 @@ for k in kustomizations: check(not findings, f"{len(findings)} unescaped, undefined or unparseable expansion(s) in substituted paths") +# ----------------------------------- (7) no dollar run is silently rewritten on the way out +# THE 2026-09-13 CLASS. envsubst consumes dollars in PAIRS and emits one per pair, +# unconditionally and everywhere (header: DOLLAR-RUN ARITHMETIC, measured against flux 2.4.0). +# So every run of >= 2 dollars in a substituted manifest is REWRITTEN between git and the +# container, and from the text alone the two intents are indistinguishable: +# +# (a) a deliberate escape the author WANTS collapsed — `$${VAR}` -> literal `${VAR}`; +# (b) content that must arrive VERBATIM — `DO $$ ... $$;` -> `DO $ ... $;` +# i.e. the incident. A BUG. +# +# THE RULE CHOSEN, AND WHY IT IS NEITHER NOISY NOR VACUOUS. +# Exactly ONE shape passes unremarked: a run of EXACTLY TWO dollars immediately followed by +# `{`. That is not a guess about intent, it is the escape contract this repo already +# documents in clusters/base/apps-vms.yaml, that assertions (1)-(3) already validate the +# inside of, and that 18 reviewed occurrences already rely on (unifi cloud-init, the gitea +# and nextcloud OIDC bootstrap scripts, two longhorn comments). Failing those would be a +# permanently-firing alert on documented, correct intent, and this repo has already been +# bitten by one of those. +# +# EVERY OTHER run of >= 2 dollars FAILS unless it is named in `dollar-collapse.allow`: +# * DOLLAR-COLLAPSE — a run not followed by `{`. Cannot survive verbatim: `$$` -> `$`. +# This is the incident's exact shape and a blanket bug by default. +# * DOLLAR-HALF-ESCAPED — an ODD run >= 3 before `{`. Looks escaped, is not: the last +# dollar is live. Occasionally legitimate (`$$${PRICE}` to render a +# literal `$` before a value), so it is allowlistable, not banned. +# * DOLLAR-OVER-ESCAPED — an EVEN run >= 4 before `{`. Emits N/2 dollars then a literal +# brace; nearly always a miscount, sometimes deliberate. +# +# WHY AN ALLOWLIST AND NOT A BLANKET BAN: a blanket ban was the first choice and was +# rejected on evidence, not taste — main carries two real non-braced `$$` occurrences today +# (prose inside embedded shell comments), so a blanket ban would have shipped RED. Nor is +# the allowlist a hole: an entry is a hand-written edit to a file inside the conformance +# test, which is loud in review; it must quote a >= 24-character VERBATIM context, so it can +# only ever cover the occurrence it was written for and never a future one that drifts into +# the same file; it must carry a >= 40-character reason; and every entry must still MATCH +# something, so the allowlist cannot rot into a blanket exemption after the code it excused +# is deleted. A wildcard, a bare `$$` context and a stale entry each FAIL. +ALLOW_FILE = (ROOT / "conformance" / "tests" / "l0-static" + / "flux-postbuild-substitution" / "dollar-collapse.allow") +MIN_CONTEXT = 24 +MIN_REASON = 40 +DOLLAR_RUN_RE = re.compile(r"\$+") + +allow_entries: list[dict] = [] +check(ALLOW_FILE.is_file(), + f"{ALLOW_FILE.relative_to(ROOT)} is missing — assertion (7) needs its allowlist to " + f"exist even when empty, because an absent file and an empty file are the same green " + f"and the next author would not know the mechanism is there") +if ALLOW_FILE.is_file(): + for lineno, raw in enumerate(ALLOW_FILE.read_text().splitlines(), 1): + if not raw.strip() or raw.lstrip().startswith("#"): + continue + parts = [x.strip() for x in raw.split("|")] + if len(parts) != 3: + check(False, f"dollar-collapse.allow:{lineno}: expected exactly 3 '|'-separated " + f"fields (path | verbatim-context | reason), got {len(parts)}") + continue + allow_entries.append({"line": lineno, "path": parts[0], "ctx": parts[1], + "reason": parts[2], "used": 0}) + +for e in allow_entries: + tag = f"dollar-collapse.allow:{e['line']}" + check((ROOT / e["path"]).is_file(), + f"{tag}: names '{e['path']}', which is not a file in this repo — a stale entry is " + f"a standing exemption for a path someone could recreate") + check(len(e["ctx"]) >= MIN_CONTEXT, + f"{tag}: context {e['ctx']!r} is {len(e['ctx'])} chars; at least {MIN_CONTEXT} are " + f"required so an entry cannot be a near-wildcard that swallows future occurrences") + check(DOLLAR_RUN_RE.search(e["ctx"]) is not None + and max((len(m.group(0)) for m in DOLLAR_RUN_RE.finditer(e["ctx"])), default=0) >= 2, + f"{tag}: context {e['ctx']!r} contains no run of two or more dollars, so it excuses " + f"nothing and is either a typo or a decoy") + check(len(e["reason"]) >= MIN_REASON, + f"{tag}: reason is {len(e['reason'])} chars; at least {MIN_REASON} are required. " + f"State what envsubst will actually emit and why that is acceptable — an entry " + f"nobody had to explain is an accident wearing an approval") + +RUN_TAGS = { + "collapse": ("DOLLAR-COLLAPSE", + "envsubst collapses each PAIR of dollars into one, everywhere and " + "unconditionally, so this CANNOT reach the workload verbatim (a literal $$ " + "needs $$$$ in source). This is the 2026-09-13 nextcloud-db shape: a " + "dollar-quoted PL/pgSQL DO block arrived half-quoted and every Flux-" + "reconciled run died. Rewrite without dollars, double them again if the " + "collapse is what you want, or justify it in dollar-collapse.allow"), + "half": ("DOLLAR-HALF-ESCAPED", + "an ODD run of dollars before a brace is NOT escaped: floor(N/2) literal " + "dollars are emitted and the LAST dollar is still live, so this expansion " + "EVALUATES at reconcile time (and if its inner is unparseable it fails the " + "entire Kustomization). Use an EVEN run to escape, or justify it in " + "dollar-collapse.allow if a literal dollar before a substituted value is " + "really what you meant"), + "over": ("DOLLAR-OVER-ESCAPED", + "an EVEN run of four or more dollars emits N/2 literal dollars followed by a " + "LITERAL brace — the value is never substituted and extra dollars appear in " + "the rendered manifest. Almost always a miscount of the escape; justify it in " + "dollar-collapse.allow if it is not"), +} + +run_findings: list[str] = [] +n_runs_ok = 0 +for rel in sorted(subst_files): + text = (ROOT / rel).read_text(errors="replace") + # Absolute offsets, not per-line: the gitea/nextcloud bootstrap scripts are single + # 4000-character folded scalars, so "the line" is not a reviewable unit of context here. + nl = [i for i, ch in enumerate(text) if ch == "\n"] + spans = [] # allowlisted [start, end) regions + for e in allow_entries: + if e["path"] != rel: + continue + start = text.find(e["ctx"]) + while start >= 0: + spans.append((start, start + len(e["ctx"]), e)) + start = text.find(e["ctx"], start + 1) + for m in DOLLAR_RUN_RE.finditer(text): + n = len(m.group(0)) + if n < 2: + continue + braced = text[m.end():m.end() + 1] == "{" + if n == 2 and braced: + n_runs_ok += 1 # the documented escape; (1)-(3) own it + continue + covering = [e for (a, b, e) in spans if a <= m.start() and m.end() <= b] + if covering: + for e in covering: + e["used"] += 1 + continue + kind = "collapse" if not braced else ("half" if n % 2 else "over") + tag, fix = RUN_TAGS[kind] + lineno = bisect.bisect_right(nl, m.start()) + 1 + ctx = text[max(0, m.start() - 60):m.end() + 60].replace("\n", "\\n") + run_findings.append( + f" [{tag}] {rel}:{lineno} a run of {n} dollar(s)" + f"{' before a brace' if braced else ''}\n" + f" rendered by Flux Kustomization(s) {', '.join(sorted(subst_files[rel]))}" + f" — postBuild substitution REWRITES it\n" + f" {fix}\n" + f" | ...{ctx}...") + +check(not run_findings, + f"{len(run_findings)} dollar run(s) in substituted paths that envsubst rewrites and " + f"nothing justifies (the 2026-09-13 nextcloud-db class)") + +for e in allow_entries: + check(e["used"] > 0, + f"dollar-collapse.allow:{e['line']}: STALE — its context is not present in " + f"{e['path']} any more (or no longer contains a rewritten dollar run), so the entry " + f"now excuses nothing. Delete it. A stale allowlist is how an opt-in decays into a " + f"blanket exemption for whatever lands in that file next") + # --------------------------------------------- (5) overlay knobs exist in the base # Parse contract: overlays patch cluster-vars with JSON6902 ops (`- op: replace` / # `path: /data/KEY`) inside an inline `patch: |-` block. The repo rule (cluster-vars.yaml @@ -363,6 +615,10 @@ print("Flux postBuild substitution inventory (per Kustomization):") for line in inventory: print(line) print(f"\ncluster-vars knobs defined: {len(KEYS)} referenced: {len(referenced)}") +print(f"dollar runs >=2 in substituted files: {n_runs_ok} documented $${{...}} escapes, " + f"{sum(e['used'] for e in allow_entries)} justified by dollar-collapse.allow " + f"({len(allow_entries)} entr{'y' if len(allow_entries) == 1 else 'ies'}), " + f"{len(run_findings)} unjustified") if ADVISORIES: print("\nADVISORIES (not failures):") @@ -374,6 +630,12 @@ if findings: for f in findings: print(f, file=sys.stderr) +if run_findings: + print("\nHAZARDS — dollar runs envsubst rewrites between git and the container:", + file=sys.stderr) + for f in run_findings: + print(f, file=sys.stderr) + print() if not FAILURES: print(f"check OK: flux-postbuild-substitution — {CHECKS} assertions " diff --git a/conformance/tests/l0-static/flux-postbuild-substitution/dollar-collapse.allow b/conformance/tests/l0-static/flux-postbuild-substitution/dollar-collapse.allow new file mode 100644 index 0000000..c2905af --- /dev/null +++ b/conformance/tests/l0-static/flux-postbuild-substitution/dollar-collapse.allow @@ -0,0 +1,50 @@ +# ============================================================================= +# dollar-collapse.allow — the reviewed exceptions to assertion (7) of +# conformance/tests/l0-static/flux-postbuild-substitution/check.sh. +# +# WHY THIS FILE EXISTS +# -------------------- +# Flux's postBuild substitution runs github.com/drone/envsubst over everything a +# substituted Kustomization renders, and that scanner consumes dollars in PAIRS, +# emitting ONE dollar per pair — unconditionally, wherever they appear, not only in +# front of a brace. A run of N dollars becomes floor(N/2) dollars, and an ODD run +# leaves one live dollar behind (so `$$${X}` is NOT escaped; it expands). +# +# Consequence: there is NO way to ship a literal `$$` through a substituted path. +# On 2026-09-13 that broke the nextcloud-db grant reconciler — a dollar-quoted +# PL/pgSQL `DO $$ ... $$;` block reached Postgres as `DO $ ... $;` and every +# Flux-reconciled run died with `syntax error at or near "$"`. It had passed review +# because it was hand-applied with `kubectl apply`, which runs no envsubst at all. +# See commit 16b7fcd and check.sh's DOLLAR-RUN ARITHMETIC table (measured against +# flux 2.4.0, not inferred). +# +# From the source text alone, a doubled dollar the author WANTS collapsed and one +# that must arrive verbatim are indistinguishable. check.sh therefore accepts exactly +# one shape without comment — a run of EXACTLY TWO dollars immediately followed by +# `{`, the escape contract clusters/base/apps-vms.yaml already documents and that 18 +# reviewed occurrences already depend on — and FAILS every other run of two or more +# dollars unless it is named here. +# +# ADDING AN ENTRY IS A DECISION, NOT A MUTE BUTTON +# ----------------------------------------------- +# One entry per line, exactly three `|`-separated fields: +# +# | | +# +# * the context must be a VERBATIM substring of the file AS STORED IN GIT (for a +# folded/JSON-ish scalar that means the escaped bytes, e.g. `\n` and `—` +# spelled literally), at least 24 characters long, containing the dollar run it +# excuses, and containing no `|` — shift the window if a pipe is in the way; +# * the reason must be at least 40 characters and should say what envsubst will +# actually EMIT and why that is acceptable; +# * an entry covers only the dollar runs inside the region its context matches, so +# it can never silently cover a new occurrence that lands elsewhere in the file; +# * check.sh FAILS a stale entry — one whose context no longer matches, or which +# excuses nothing any more. Delete it rather than leaving it to rot into a +# blanket exemption for whatever lands in that file next. +# +# If the collapse would change BEHAVIOUR, this file is the wrong answer. Rewrite the +# content without dollars (16b7fcd) or double them again (`$$$$` -> `$$`). +# ============================================================================= +apps/wave-0/gitea.yaml | # NOTE the `$$` escape below \u2014 it is load-bearing. apps-wave-0 declares postBuild. | Prose, not code: this is a comment INSIDE the gitea OIDC bootstrap shell script, and the two dollars are the sentence quoting the escape idiom it goes on to explain. envsubst renders it as "the `$` escape below", which is cosmetically wrong and behaviourally inert — a shell comment. Cannot be written correctly at all: `$$$$` would render `$$` but would then be wrong in the source a reader sees in git, and the file has no non-dollar way to name the idiom. Reviewed and accepted as cosmetic. +apps/wave-1/nextcloud.yaml | # NOTE the `$$` escape below \u2014 it is load-bearing. apps-wave-1 declares postBuild. | Identical case to the gitea entry above, in the nextcloud OIDC bootstrap script: a shell comment quoting the escape idiom, rendered by envsubst as "the `$` escape below". Cosmetic only — the collapse happens inside a `#` comment line that no interpreter reads, and the escaped expansion the comment is describing is on a later line and is unaffected. Reviewed and accepted as cosmetic. From 830983b791b7ff3427813b7b339999e6521dce43 Mon Sep 17 00:00:00 2001 From: Keith Jackson Date: Sun, 13 Sep 2026 16:17:51 -0500 Subject: [PATCH 2/2] =?UTF-8?q?test(l0):=20prove=20the=20dollar-collapse?= =?UTF-8?q?=20guard=20fires=20=E2=80=94=20and=20name=20the=20real=20envsub?= =?UTF-8?q?st?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The implementation landed in the preceding WIP commit had never been executed. This finishes it: the arithmetic is re-measured, the guard and its allowlist are attacked, and the header's one factual error is corrected. WHAT ENVSUBST ACTUALLY IS. The header credited `github.com/drone/envsubst`. The pinned flux 2.4.0 binary in the dev image embeds `github.com/fluxcd/pkg/envsubst@v1.1.0` — Flux's own fork of drone/envsubst ("This package is a fork of drone/envsubst", its README) — read straight out of the binary with `grep -ao`. Same behaviour, different module; the header now says so and says how to check, because a claim about a dependency that nobody can re-derive is how the last wrong assumption survived. RE-MEASURED, NOT ASSERTED. Every row of the DOLLAR-RUN ARITHMETIC table was re-run through `flux envsubst` (flux 2.4.0, DEFINED=VALUE) and every row holds: `$$`->`$`, `$$$`->`$$`, `$${D}`->`${D}`, `$$${D}`->`$VALUE` (odd run: the brace is LIVE), `$$$${D}`->`$${D}`, `DO $$ x $$;`->`DO $ x $;`, and `$$${ARR[0]}` dies with "missing closing brace" while `$${ARR[0]}` is fine. `--strict` output was byte-identical to non-strict, exit 0 both ways (`cmp`) — strict mode is blind to this whole class, which is why a source assertion is the only guard available. The header now carries the 30-second reproduction, flagged to be re-measured on any FLUX_VERSION bump. END TO END ON THE REAL PATH. With the pre-16b7fcd manifest restored, `kubectl kustomize apps/wave-0` emits `DO $$` — valid SQL, and exactly what the `kubectl apply` verification saw — while the same stream through `flux envsubst --strict` emits `DO $` and `$;` and exits 0. That two-command contrast IS the incident. THE GUARD IS NOT TRIVIALLY SATISFIABLE — 16 adversarial cases, all as intended, run against a throwaway copy of the tree so nothing could be left behind: * an unmarked `$$` under a substituted path FAILS (DOLLAR-COLLAPSE); so do an odd `$$${...}` (DOLLAR-HALF-ESCAPED) and an even `$$$${...}` (OVER-ESCAPED); * a stale entry FAILS three ways — context no longer in the file, context that only covers an already-accepted `$${` escape and so excuses nothing, and a path that is not a file in the repo; * the allowlist cannot blanket-exempt a path: a 2-char context is rejected (<24), a `.*` wildcard is matched VERBATIM and therefore rejected as stale, a 24-char context carrying no dollar run is rejected, a 4-char reason is rejected (<40) — and, the central case, a genuine entry covers only the span its context matches, so a SECOND `$$` elsewhere in the same file still fails. Two genuine entries for two real occurrences do pass, so the mechanism is usable rather than merely unsatisfiable; * the CLASS, not the instance: the pre-fix `DO $$ ... $$;` reconstructed in a NEW ConfigMap under apps/wave-2 fails at both dollar runs, and restoring the real pre-16b7fcd apps/wave-0/nextcloud-db.yaml verbatim fails the check at lines 384 and 391. The guard rejects the exact commit that broke production. The scratch fixtures lived only in the throwaway copy; `git status` on this branch stayed clean throughout, which is the other half of the last attempt's unfinished business. LEGITIMATE USES EXIST AND STAY GREEN. 18 `$${...}` escapes (unifi cloud-init, the gitea and nextcloud OIDC bootstrap scripts, two longhorn comments) pass unremarked, and the 2 non-braced `$$` occurrences are the two allowlist entries. Those two are shell comments inside block scalars, and `kubectl kustomize` confirms the comment text really does survive into the stream envsubst reads — so they are genuine, reaching content whose collapse is cosmetic, not a hypothetical the allowlist was invented to excuse. STILL NOT PROVEN, AND STILL NOT PROXIED: that a change was verified through a substituting path. Which command a human ran is a fact about a shell session; git stores the manifest, never the invocation, so it is not statically decidable here and no honest formulation exists. Grepping commit prose for `kubectl apply` would be satisfiable by rewording and would fire on the break-glass runbooks whose whole content is that command. The answer is to move the proof off the verification path and onto the source, which is what assertion (7) does. No ci/l0.sh stage: the `conformance.home.k3s/scope: "whole-repo"` annotation is what wires it, and whole-repo-invariants-wired fails the gate on a hand-written stage. Co-Authored-By: Claude Opus 5 --- .../flux-postbuild-substitution/check.sh | 31 +++++++++++++++---- .../dollar-collapse.allow | 4 ++- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/conformance/tests/l0-static/flux-postbuild-substitution/check.sh b/conformance/tests/l0-static/flux-postbuild-substitution/check.sh index b75754a..84f2e10 100755 --- a/conformance/tests/l0-static/flux-postbuild-substitution/check.sh +++ b/conformance/tests/l0-static/flux-postbuild-substitution/check.sh @@ -62,10 +62,14 @@ # # DOLLAR-RUN ARITHMETIC — the exact rule, MEASURED, not inferred (see assertion (7)) # --------------------------------------------------------------------------------- -# envsubst here is `github.com/drone/envsubst`, vendored by fluxcd/pkg/kustomize and exposed -# by the `flux envsubst` CLI ("replicate the behavior of the Flux Kustomization post-build -# substitutions" — its own --help). Its scanner walks the input left to right; on a `$` it -# looks at the NEXT byte: +# envsubst here is `github.com/fluxcd/pkg/envsubst` v1.1.0 — Flux's own maintained FORK of +# `github.com/drone/envsubst` ("This package is a fork of drone/envsubst", its README) — and it +# is exposed by the `flux envsubst` CLI ("replicate the behavior of the Flux Kustomization +# post-build substitutions", its own --help). Named from the binary, not from memory: the +# pinned flux 2.4.0 in the dev image embeds the Go module path +# `/home/runner/go/pkg/mod/github.com/fluxcd/pkg/envsubst@v1.1.0/parse` +# (`grep -ao 'github.com/fluxcd/pkg/envsubst[^ ]*' "$(command -v flux)"`). Its scanner walks +# the input left to right; on a `$` it looks at the NEXT byte: # * another `$` -> BOTH are consumed and ONE `$` is emitted. This is UNCONDITIONAL. What # follows the pair is irrelevant — `{`, a letter, a digit, a semicolon, # a newline, end of input. `$$` is not "the escape for `${`"; it is the @@ -98,8 +102,23 @@ # cannot parse, fails the ENTIRE Kustomization (proven above with $$${ARR[0]}, which # aborted the whole stream while $${ARR[0]} on the line before it was fine). # `--strict` reports NONE of this: every row of that table except the parse error was -# accepted SILENTLY by `flux envsubst --strict`, exit 0. Strict mode cannot see this class, -# so a source-level assertion is the only available guard. +# accepted SILENTLY by `flux envsubst --strict`, exit 0, and its output was BYTE-IDENTICAL to +# non-strict (`cmp`). Strict mode cannot see this class, so a source-level assertion is the +# only available guard. +# +# TO REPRODUCE THE TABLE (30 seconds, no cluster) — it is a measurement, so re-measure it +# after any FLUX_VERSION bump in infra/devcontainer/versions.env. Open the toolbox with +# `./infra/devcontainer/dev.sh` and run these INSIDE it (do NOT pipe into `dev.sh run` from +# the host: it starts the container with `-it`, so a piped stdin hangs): +# printf '[$$] [$$$] [$${D}] [$$${D}] [DO $$ x $$;]\n' | D=VALUE flux envsubst +# -> [$] [$$] [${D}] [$VALUE] [DO $ x $;] +# And end-to-end, on the actual delivery path, with the pre-16b7fcd manifest checked out +# (`git show 16b7fcd^:apps/wave-0/nextcloud-db.yaml > apps/wave-0/nextcloud-db.yaml`): +# kubectl kustomize apps/wave-0 | grep 'DO \$' -> "DO $$" (the kubectl path) +# kubectl kustomize apps/wave-0 | flux envsubst --strict | grep 'DO \$' +# -> "DO $" (the FLUX path), exit 0 +# (`kubectl kustomize` IS the kustomize build here; the image ships no standalone binary.) +# That two-command contrast IS the 2026-09-13 incident, and is why review missed it. # ${...}, ${a b}, ${ARR[0]} (UNESCAPED) -> HARD PARSE ERROR ("unable to parse # variable name" / "missing closing brace") which fails the # WHOLE Kustomization — every resource in the path, not just diff --git a/conformance/tests/l0-static/flux-postbuild-substitution/dollar-collapse.allow b/conformance/tests/l0-static/flux-postbuild-substitution/dollar-collapse.allow index c2905af..28bcf02 100644 --- a/conformance/tests/l0-static/flux-postbuild-substitution/dollar-collapse.allow +++ b/conformance/tests/l0-static/flux-postbuild-substitution/dollar-collapse.allow @@ -4,7 +4,9 @@ # # WHY THIS FILE EXISTS # -------------------- -# Flux's postBuild substitution runs github.com/drone/envsubst over everything a +# Flux's postBuild substitution runs github.com/fluxcd/pkg/envsubst (Flux's fork of +# github.com/drone/envsubst; the module path is embedded in the pinned flux 2.4.0 +# binary, and check.sh's header says how to read it back out) over everything a # substituted Kustomization renders, and that scanner consumes dollars in PAIRS, # emitting ONE dollar per pair — unconditionally, wherever they appear, not only in # front of a brace. A run of N dollars becomes floor(N/2) dollars, and an ODD run