refactor(telemetry): generate the redaction floor from one declaration (backend#2378)

[LukasWodka]

Closes tracebloc/backend#2378 — the consumer half; tracebloc/client-runtime#376 is the owner half and merges first, so the keyword lives here.

MERGE ORDER — read first. client-runtime#376 must merge first. scripts/.client-runtime-ref here is pinned at #376's head commit (98f5ada), which is what makes the cross-repo job green on this PR — pinning develop's tip instead would have landed this with a red gate, and a red gate teaches people to skip the tier. Once #376 lands, re-point that pin at client-runtime's develop merge SHA; the bytes are identical either way, so it is tidiness rather than correctness. The pin file says so in place.

Landing this before #376 would ship a chart whose declaration exists nowhere upstream. Don't.

For the record, because it is the best evidence in this PR: the job first ran red on purpose, with client-runtime/log_redactions.json is missing at 6293d15… — the pin predated the declaration, and the step reported that as a finding rather than as agreement. The App token minted, the private repo checked out, and the fail-closed branch fired. Then it was armed green.

What was wrong

The six OTTL replace_pattern calls in telemetry-collector-configmap.yaml were a hand port of client-runtime's _LOG_REDACTIONS, and the comment above them said so in as many words: "THIS IS A CROSS-REPO SECOND COPY AND IT CANNOT BE MACHINE-CHECKED FROM HERE."

backend#1908 is what that cost. The generic pattern anchored its keyword with a word boundary on both sides, and _ is a word character, so every environment-variable-shaped secret — AZURE_CLIENT_SECRET=, MYSQL_PASSWORD=, DJANGO_SECRET_KEY=, AWS_SECRET_ACCESS_KEY= — fell straight through. In both copies. For as long as they existed. With passing tests on both sides.

What this does

The rule is declared once, in client-runtime/log_redactions.json. This chart vendors those bytes at client/log_redactions.json and generates the statements from them with .Files.Get + mustFromJson. The hand port is gone.

The render is byte-identical to develop's — same six patterns, same six replacements, same order — verified by diffing the rendered ConfigMap. helm unittest needed no assertion changes.

Why vendored and not fetched

client-runtime is private and GITHUB_TOKEN is repo-scoped, so no lint job here can read it without a credential — and a network read fails on exactly the day the network is what broke. So the bytes are vendored with their sha256 and checked locally, offline, in the required drift job.

Where the checks live, and what each can see

check	question	network?	required?
scripts/tests/collector-redaction-derived.sh (new, in make drift → Source-of-truth drift)	are the vendored bytes the ones scripts/.log-redactions.sha256 pins, and is what the chart renders the declaration — pair for pair, in order?	no	yes
scripts/tests/collector-redaction-floor.sh (existing, from #799)	do those patterns actually scrub real secrets, and leave the diagnosis alone?	no	yes
Cross-repo contract drift (existing job, extended)	has upstream moved on without this copy following?	yes (App token, already provisioned)	no — weekly + path-filtered

The two local guards are complementary, and this was measured, not assumed: dropping a redaction from the declaration and fixing the digest leaves the new guard green (the local pair is self-consistent) and reddens the floor guard on its count floor. A hand-written statement in the template does the opposite. Neither subsumes the other, so collector-redaction-floor.sh was extended in prose only — its behaviour and its independently-written specimens are untouched.

Derived, never restated (backend#1729 rule 1, and rule 9's corollary): the new guard holds no pattern text of its own. It compares a file against the real template's real output. That is precisely what copy (3) failed to do — it greped the render for six substrings it held itself, agreed with itself, and reported that as agreement with a file in another repository.

Fails closed everywhere

• Missing vendored file → the template itself fails, so the chart cannot render a Collector with an empty statements list (which would report Ready and export every secret it read). Verified: Error: … client/log_redactions.json is missing from the chart ….
• Missing vendored file, missing pin, empty pin, digest mismatch, unparseable render, a statement that is not a two-argument replace_pattern, or zero patterns parsed → all findings.
• The provenance half runs without helm, so it never skips; only the render comparison degrades on a box with no helm, and it says so.
• The cross-repo job treats an absent upstream file as a finding, not as agreement.

Mutation proof

Anchors asserted before each run:

mutation	anchor	result
hand-edit the vendored declaration (7th redaction), leave the pin	7 entries	🔴 client/log_redactions.json is not the file scripts/.log-redactions.sha256 pins
hand-write an extra replace_pattern into the template	1 match for HANDWRITTEN	🔴 rendered but NOT declared: '\bHANDWRITTEN[0-9]+\b'
drop a redaction and fix the digest (a "clean" local edit)	5 entries	🟢 derived guard (self-consistent) / 🔴 floor guard: "the redaction floor dropped: 5 pattern(s), floor is 6"
delete client/log_redactions.json	No such file	🔴 helm render refuses; guard errors
delete scripts/.log-redactions.sha256	file gone	🔴 "the only local witness of WHICH version … is missing"
upstream adopts a 7th redaction, this repo does not re-vendor	digests differ	🔴 both cross-repo steps; local guard green — the honest limit, and why that job exists

The ritual cost, stated rather than discovered

A bump is now a two-PR sequence across repos: land it in client-runtime (its digest tripwire refuses to let that half land quietly), then here — copy the file, copy the digest, bump scripts/.client-runtime-ref, bump Chart.yaml. That is the price of removing the network and the credential from the required path, and it is written into scripts/.log-redactions.sha256 and into the cross-repo job's failure message so nobody has to rediscover it.

Verification

helm unittest client 550 passed, 34 suites (unchanged assertions)
make drift all 17 guards green (16 on develop + the new one)
make lint 53 scripts parse; shellcheck 60 files, severity=error, clean
shellcheck -S warning -x scripts/tests/collector-redaction-derived.sh clean
gitleaks detect -c .gitleaks.toml no leaks found
rendered replace_pattern lines: byte-identical to develop

No .gitleaks.toml entry was needed, and none was added: the vendored declaration holds regexes, not secret-shaped values. The existing path-scoped entry for collector-redaction-floor.sh is untouched.

Chart.yamlversion + appVersion → 1.9.66 (equal, one patch above develop), since chart content changed.

---

Note

Medium Risk
Touches the Collector’s secret-scrubbing pipeline (what leaves a customer cluster), but the six patterns themselves are unchanged; risk is mainly fail-closed template/CI wiring and a temporary pin to an unmerged client-runtime commit.

Overview
The Collector’s redaction floor is no longer a hand-copied set of OTTL replace_pattern calls. The chart vendors client/log_redactions.json (from client-runtime) and generates the statements via .Files.Get / mustFromJson. A missing or empty declaration fails the render so a Collector cannot start with no scrubbing.

A new required drift guard (collector-redaction-derived.sh) checks the vendored bytes against scripts/.log-redactions.sha256 and that the rendered pairs match the declaration in order. The weekly cross-repo job now diffs that file and digest against the same pinned client-runtime ref as the envelope contract. Chart version is 1.9.67.

Note:scripts/.client-runtime-ref currently points at an unmerged client-runtime#376 SHA on purpose; re-point it after that PR lands. Adopting an upstream redaction change is a two-PR bump (runtime first, then this repo).

^{Reviewed by Cursor Bugbot for commit 42e65a2. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit b350035. Configure here.}

[saqlainsyed007]

Reviewed at high effort. The core is correct — the template generation fails closed, the escaping round-trips (mustFromJson → quote/%q → OTTL literal → RE2), and collector-redaction-derived.sh is a genuine provenance+render check (digest against the pin, then rendered pairs vs. the declaration, order-sensitive, fail-closed on CI-without-helm). The Chart.yaml bump follows the repo's per-content-change convention, and the unmerged .client-runtime-ref pin is a documented post-#376 tidy-up (#376 is now approved).

Requesting changes on one confirmed defect — Bugbot's open "Unpinned Helm version" thread is valid, verified against the sibling files:

• The new Set up Helm step in .github/workflows/envelope-contract-drift.yml uses azure/setup-helm@…v4.3.1 with no with: version:.
• drift-checks.yaml (version: v3.15.4) and helm-ci.yaml (three steps, all version: v3.15.4) pin it. azure/setup-helm defaults version to latest, so this job installs Helm 4.x while the required Source-of-truth drift path — which runs the samecollector-redaction-derived.sh — runs v3.15.4.
• That is exactly the "green having proved less than its name claims" risk this step was added to close: the render comparison could pass under a Helm the required path never uses (or Helm 4.x could change .Files.Get/mustFromJson/templating outright). The step's comment "Same action, same pin as drift-checks.yaml and helm-ci.yaml" is now inaccurate.

Fix: add

with:
version: v3.15.4

to that step so all three jobs render under the same pinned Helm. Everything else is good to go.

[LukasWodka]

@saqlainsyed007 Fixed in a81023c — exactly the diff you specified:

uses: azure/setup-helm@1a275c3b69536ee54be43f2070a358922e12c8d4 # v4.3.1with:
version: v3.15.4

You and Bugbot were right on every step of the reasoning, including the part I'd have been most likely to wave off: the step's own comment claimed "same action, same pin as drift-checks.yaml and helm-ci.yaml" while carrying no version:, so the sentence documenting the invariant was the thing hiding its absence. azure/setup-helm defaults to latest, so this job would have rendered under Helm 4.x while the required Source-of-truth drift path ran the samecollector-redaction-derived.sh under v3.15.4 — a green proving less than its name claims, which is the precise failure this workflow exists to close.

Verified all eight setup-helm steps in the repo now report v3.15.4 (drift-checks, installer-tests, envelope-contract-drift, helm-ci ×3, standard-checks, release-helm-chart) — no unpinned outlier left. I also rewrote the comment so it states why the pin is load-bearing rather than just asserting the pin exists.

Re-requesting your review.

[saadqbal]

I verified this one end to end rather than reading it, because the whole PR is a provenance claim and
a provenance claim you haven't checked is just a longer comment.

The chain holds. Upstream client-runtime/log_redactions.json at the pinned ref, the vendored
client/log_redactions.json, and the digest in scripts/.log-redactions.sha256 are all
5a804b4c13a5ab05… — same bytes, same hash, three independent reads. And the render really is
unchanged: I rendered the chart at this head and at origin/develop and diffed the six extracted
replace_pattern statements — identical, patterns and replacements and order.

The new required guard is mutation-proof. Four mutations, all of them reddened, each with a
message that names the actual defect:

mutation	result
hand-write a 7th replace_pattern in the template	rendered but NOT declared: '\\bSNEAKY[0-9]{4}\\b'
range reverse $redactions	same pairs, different ORDER — redaction is applied in sequence
hand-edit the vendored JSON	digest mismatch, both hashes printed
delete the vendored JSON	caught in step 1, before helm runs

Also checked the thing that would have made all of the above moot: client/.helmignore does not
exclude log_redactions.json, and helm package puts it in the tgz — so the released artifact
carries the declaration and won't fail on a customer's install.

One finding, inline and non-blocking: 2>/dev/null on the helm template call discards the
template's fail message, so the .helmignore case — the exclusion your own comment names — reports
"expected exactly one Collector ConfigMap, found 0" one line after step 1 printed ok for that same
file. Fails closed, just points at the wrong thing.

Holding the approval on the gate, not on that: bats (bash unit, mocked) and Cursor Bugbot are
both still pending on this head, and @saqlainsyed007's change-request from b350035 is still
standing — the helm version pin they asked for is in and credited in the workflow comment, so
that's probably just a re-review away rather than anything outstanding.

Two notes for whoever does the post-merge tidy-up. The body says the pin is at #376's head
98f5ada, but scripts/.client-runtime-ref actually holds e4c4813e… and #376's head is now
7ea3886c — three SHAs. Harmless, and I confirmed why: the only commit between e4c4813 and
7ea3886 touches tests/test_log_redaction_contract.py, and log_redactions.json is the identical
blob at both. So "the bytes are identical either way" is true. Just fix the SHA in the body so the
re-point instruction points somewhere real.

[saqlainsyed007]

Re-review. My original blocker is fixed — the Set up Helm step in envelope-contract-drift.yml now pins with: version: v3.15.4, so the drift job renders under the same Helm as the required path. Thanks.

Holding on @saadqbal's open thread, which I verified against the code and agree is a real defect (scripts/tests/collector-redaction-derived.sh:230):

helm template t "$CHART" ... 2>/dev/null | python3 "$CHK" "$DECLARATION"

The 2>/dev/null discards stderr, which is exactly where the template's fail writes. So when the fail-closed path fires — a missing/.helmignored log_redactions.json — the carefully-worded message that names the cause and the fix (the whole point of those fail strings) never reaches the operator; the guard sees empty stdin and reports its own generic error instead. saadqbal reproduced it via .helmignore. Drop the 2>/dev/null (or capture stderr and surface it on non-zero exit) so the actionable message survives. Everything else is good — happy to approve once that lands.

[LukasWodka]

Fixed in 42e65a2 — took your suggestion essentially as written. I reproduced it first, and it behaves exactly as you described.

Before (.helmignore entry added, the exclusion the template comment names):

 ok: client/log_redactions.json matches scripts/.log-redactions.sha256 (6 redaction(s), sha256 5a804b4c13a5…)
[ERROR] expected exactly one Collector ConfigMap, found 0 — cannot check what cannot be located

After, same input — the template's own fail string now survives:

 ok: client/log_redactions.json matches scripts/.log-redactions.sha256 (6 redaction(s), sha256 5a804b4c13a5…)
[ERROR] expected exactly one Collector ConfigMap, found 0 — cannot check what cannot be located
Error: execution error at (client/templates/telemetry-collector-daemonset.yaml:98:28):
client/log_redactions.json is missing from the chart, so the Collector's redaction floor
cannot be generated. It is vendored from tracebloc/client-runtime; restore it and check
scripts/.log-redactions.sha256.

Your framing of why it matters is the part I'd have got wrong on my own: the ok is not merely unhelpful, it actively contradicts the failure one line above it, because step 1 checks the file on disk and the template could not read it. Those are different questions and the guard was reporting them as if they were one.

Verified three paths rather than just the reported one, since a stderr change can mask a different failure:

path	result
A — nothing broken	green, exit 0 (unchanged)
B — helm cannot read the declaration	exit 1, cause + fix now named
C — real digest drift, helm fine	exit 1, drift message unchanged, helm stderr does not intrude

Kept your grep -v for the kubeconfig noise, with || true — grep exits 1 on no match, so without it an empty stderr would have turned this into a different failure than the real one. shellcheck -S warning -x clean, bash -n clean.

One thing I did not fix, so it's a known limit rather than an oversight: the misdirecting found 0 ConfigMaps line still prints above the real cause. The reader now ends on the right message, but ordering it properly means teaching the python checker that an empty render is a distinct condition from a wrong one. Happy to do that here if you'd rather — it felt like scope creep on a diagnosability fix.

You were right that this was the only thing worth blocking on.

[saqlainsyed007]

Approving. Both blockers are resolved and verified:

• Helm version pin (my original) — the Set up Helm step now pins v3.15.4, so the drift job renders under the same Helm as the required path.
• 2>/dev/null swallowing the fail message (@saadqbal's) — the helm render now captures stderr to $HELM_ERR and, on failure, surfaces it (grep -v filters only the benign kubeconfig WARNING, || true so an empty stderr doesn't change the failure), then exit 1. So the template's actionable fail strings reach the operator instead of a generic "found 0 ConfigMaps". Still fail-closed (pipefail exits 1) — purely a which-file-the-2am-reader-is-sent-to fix.

The core refactor was sound throughout (one vendored declaration, generated OTTL statements, digest tripwire, the render-vs-declaration guard). CI green, no open threads.

[saadqbal]

Approving. My finding is fixed and I re-ran the reproduction rather than reading the diff: with
log_redactions.json added to .helmignore, the guard now surfaces helm's own message —
"client/log_redactions.json is missing from the chart … restore it and check
scripts/.log-redactions.sha256" — so the operator is sent to the right file. Still exits 1, and the
mutation anchor still reddens on a hand-written 7th replace_pattern, so nothing was traded away for
the diagnosability.

The comment you left above the call is better than the fix — it records the misdirection, not just
the swallow, which is the part that would have cost someone an hour.

Worth one non-blocking note since it's the same line of thinking, and I'm explicitly not asking for
more here: python's found 0 ConfigMaps still prints before helm's captured stderr, so the first
[ERROR] an operator reads remains the misleading one — the real cause is just two lines below it
now instead of absent. Inherent to the pipeline ordering, and vastly better than it was.

For the record, the earlier verification stands on this head too: upstream bytes, vendored copy and
pinned digest all agree, the render is byte-identical to develop's, and the declaration ships inside
the packaged tgz.