WIP: catch envsubst collapsing a doubled dollar in substituted paths
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 <[email protected]>
This commit is contained in:
1 parent
f4f6e5f7a3
commit
377fa205ea
3 files changed
+353
-8
No files matched your search
@@ -8,6 +8,18 @@
|
|||||||
# the empty string while the Kustomization still reports Ready — which emptied Cilium's
|
# 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).
|
# 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,
|
# GIVEN every Flux Kustomization under clusters/ that declares postBuild.substituteFrom,
|
||||||
# WHEN each `spec.path` is resolved to the exact file set it renders (following
|
# WHEN each `spec.path` is resolved to the exact file set it renders (following
|
||||||
# kustomization.yaml `resources:` transitively, not globbing the repo),
|
# kustomization.yaml `resources:` transitively, not globbing the repo),
|
||||||
@@ -15,7 +27,12 @@
|
|||||||
# or deliberately ESCAPED `$${...}`; no unescaped expansion carries a default
|
# or deliberately ESCAPED `$${...}`; no unescaped expansion carries a default
|
||||||
# operator on an undefined name (the silent-empty class strict mode does NOT catch);
|
# 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
|
# 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
|
# every overlay-patched knob has a base default; and the check is wired into the
|
||||||
# merge gate as a blocking, unconditionally-run stage.
|
# 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
|
# only); conformance/run.sh --layer l0 and ci/l0.sh run it directly, and this Chainsaw
|
||||||
# Test wraps the SAME script.
|
# 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
|
# 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
|
# 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,
|
# not render, does not prove the feature gate is enabled on the live kustomize-controller,
|
||||||
@@ -63,13 +93,16 @@ metadata:
|
|||||||
conformance.home.k3s/intent: >-
|
conformance.home.k3s/intent: >-
|
||||||
Every variable a substituted Flux path renders is defined in cluster-vars or
|
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
|
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:
|
spec:
|
||||||
steps:
|
steps:
|
||||||
- name: audit-every-substituted-flux-path
|
- name: audit-every-substituted-flux-path
|
||||||
description: >-
|
description: >-
|
||||||
Resolve each postBuild.substituteFrom Kustomization to the files it renders and
|
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:
|
try:
|
||||||
- script:
|
- script:
|
||||||
content: bash ./check.sh
|
content: bash ./check.sh
|
||||||
@@ -18,6 +18,32 @@
|
|||||||
# substituted path renders is either DEFINED or deliberately ESCAPED — and nothing in
|
# substituted path renders is either DEFINED or deliberately ESCAPED — and nothing in
|
||||||
# the repo asserted that. This check does.
|
# 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 <dq> BEGIN ... END <dq>; where <dq> 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 <one dollar>` 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,
|
# ENVSUBST BEHAVIOUR ASSERTED HERE (empirically verified against the pinned toolchain,
|
||||||
# `flux envsubst`, flux 2.4.0 — this is measured behaviour, not assumption):
|
# `flux envsubst`, flux 2.4.0 — this is measured behaviour, not assumption):
|
||||||
# ${DEFINED} -> substituted with the cluster-vars value.
|
# ${DEFINED} -> substituted with the cluster-vars value.
|
||||||
@@ -33,6 +59,47 @@
|
|||||||
# shell/cloud-init consumer. Escaping is total: it neutralises
|
# shell/cloud-init consumer. Escaping is total: it neutralises
|
||||||
# even the indexed form.
|
# even the indexed form.
|
||||||
# $VAR, $(cmd) -> left alone by envsubst; they need no escaping.
|
# $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
|
# ${...}, ${a b}, ${ARR[0]} (UNESCAPED) -> HARD PARSE ERROR ("unable to parse
|
||||||
# variable name" / "missing closing brace") which fails the
|
# variable name" / "missing closing brace") which fails the
|
||||||
# WHOLE Kustomization — every resource in the path, not just
|
# 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 —
|
# (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
|
# the "add the knob with a base default BEFORE referencing it" rule that the
|
||||||
# incident's interim discipline depends on;
|
# 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`,
|
# 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
|
# 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.
|
# 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
|
# kustomize-controller. Both are cluster facts (L2), untested here. Tagging
|
||||||
# R-GITOPS-01 claims ONLY the static slice: what git says a Kustomization renders
|
# 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.
|
# 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 <path> | 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,
|
# - 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.
|
# 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:
|
# - 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
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import bisect
|
||||||
import os
|
import os
|
||||||
import re
|
import re
|
||||||
import sys
|
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:
|
def classify(dollars: int, inner: str) -> tuple[str, str] | None:
|
||||||
"""Return (severity-tag, remediation) for a hazardous expansion, else None."""
|
"""Return (severity-tag, remediation) for a hazardous expansion, else None.
|
||||||
if dollars >= 2:
|
|
||||||
return None # escaped: emitted as a literal
|
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 NAME_RE.match(inner):
|
||||||
if inner in KEYS:
|
if inner in KEYS:
|
||||||
return None # defined knob — the happy path
|
return None # defined knob — the happy path
|
||||||
@@ -268,6 +366,9 @@ def rendered_files(path_spec: str) -> tuple[list[str], bool]:
|
|||||||
findings: list[str] = []
|
findings: list[str] = []
|
||||||
referenced: dict[str, set[str]] = {}
|
referenced: dict[str, set[str]] = {}
|
||||||
inventory: list[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:
|
for k in kustomizations:
|
||||||
files, exists = rendered_files(k["path"])
|
files, exists = rendered_files(k["path"])
|
||||||
@@ -281,13 +382,14 @@ for k in kustomizations:
|
|||||||
text = f.read_text()
|
text = f.read_text()
|
||||||
except (OSError, UnicodeDecodeError):
|
except (OSError, UnicodeDecodeError):
|
||||||
continue
|
continue
|
||||||
|
subst_files.setdefault(rel, set()).add(k["name"])
|
||||||
for lineno, line in enumerate(text.splitlines(), 1):
|
for lineno, line in enumerate(text.splitlines(), 1):
|
||||||
for m in TOKEN_RE.finditer(line):
|
for m in TOKEN_RE.finditer(line):
|
||||||
dollars, inner = len(m.group(1)), m.group(2)
|
dollars, inner = len(m.group(1)), m.group(2)
|
||||||
verdict = classify(dollars, inner)
|
verdict = classify(dollars, inner)
|
||||||
if verdict is None:
|
if verdict is None:
|
||||||
if dollars >= 2:
|
if dollars % 2 == 0: # PARITY, see classify(): an odd run is
|
||||||
n_esc += 1
|
n_esc += 1 # live, not escaped
|
||||||
else:
|
else:
|
||||||
n_def += 1
|
n_def += 1
|
||||||
referenced.setdefault(re.split(r"[:/#%^,]", inner)[0] or inner,
|
referenced.setdefault(re.split(r"[:/#%^,]", inner)[0] or inner,
|
||||||
@@ -313,6 +415,156 @@ for k in kustomizations:
|
|||||||
check(not findings,
|
check(not findings,
|
||||||
f"{len(findings)} unescaped, undefined or unparseable expansion(s) in substituted paths")
|
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
|
# --------------------------------------------- (5) overlay knobs exist in the base
|
||||||
# Parse contract: overlays patch cluster-vars with JSON6902 ops (`- op: replace` /
|
# 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
|
# `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:
|
for line in inventory:
|
||||||
print(line)
|
print(line)
|
||||||
print(f"\ncluster-vars knobs defined: {len(KEYS)} referenced: {len(referenced)}")
|
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:
|
if ADVISORIES:
|
||||||
print("\nADVISORIES (not failures):")
|
print("\nADVISORIES (not failures):")
|
||||||
@@ -374,6 +630,12 @@ if findings:
|
|||||||
for f in findings:
|
for f in findings:
|
||||||
print(f, file=sys.stderr)
|
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()
|
print()
|
||||||
if not FAILURES:
|
if not FAILURES:
|
||||||
print(f"check OK: flux-postbuild-substitution — {CHECKS} assertions "
|
print(f"check OK: flux-postbuild-substitution — {CHECKS} assertions "
|
||||||
|
|||||||
@@ -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:
|
||||||
|
#
|
||||||
|
# <repo-relative path> | <verbatim context> | <reason>
|
||||||
|
#
|
||||||
|
# * 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.
|
||||||
Reference in new issue
Block a user