Skip to content

fix(ci): watch every mutable label that points at a pinned digest (backend#1853) - #697

Merged
LukasWodka merged 5 commits into
developfrom
fix/1853-digest-drift-watcher
Aug 13, 2026
Merged

fix(ci): watch every mutable label that points at a pinned digest (backend#1853)#697
LukasWodka merged 5 commits into
developfrom
fix/1853-digest-drift-watcher

Conversation

@LukasWodka

@LukasWodka LukasWodka commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Fixes tracebloc/backend#1853. Built to @saadqbal's diagnosis rather than to the symptom.

The design, and why the DB_USER check is deliberately absent

"The disease is a moving label pointing at an immutable trust decision, with no watcher on the label. Anything about the build could have been the thing that changed — the fallback just got there first."

So this script asserts nothing about a build's contents. There is no DB_USER check, on purpose: an assertion about the fallback would go green the next time something else moves, which is the failure being watched for rather than a variant of it. It asks one property-agnostic question per pin:

does the float still resolve to the digest we decided to trust?

Where the trusted versions are registered: exactly where they already were — the digest: / prodDigest: fields of client/values.yaml. There is no second list to keep in sync, and adding a pin enrols it automatically.

Against the real chart

ok    ubuntu/squid:6.6-24.04_beta                    sha256:6a097f68bae7…
DRIFT: images.ingestor
UNWATCHABLE: images.mysqlClient is pinned to sha256:f546e47fb339…
2 finding(s): 3 pin(s) found, 2 resolvable and compared.

The UNWATCHABLE row is a real finding, not noise: mysqlClient carries a pin but declares no repository:, so nothing can tell you when its trust decision goes stale. That is a modelling gap worth surfacing — reported rather than skipped.

Two bugs of my own, both found by running it rather than reading it

  1. images:-scoped discovery watched 1 of the 3 pins. squid's pin lives outside images:, and an "empty field means skip" rule dropped mysqlClient without a word. A watcher silently covering a third of its subject is precisely the failure this exists to catch. Rewritten pin-driven over the whole file: a pin is either watched or reported, never skipped.

  2. IFS=$'\t' collapses runs of tabs, because tab is IFS whitespace. A record with an empty repository and an empty tag slid the pin into the wrong variable, left $pin empty, and the row was skipped in total silence. This is the same defect release-train's own parse-repos suite pins by name — "0x1f, not tab: bash collapses runs of IFS whitespace, so one empty field lets the next value slide into its place." I re-made a mistake my own test suite documents.

The two review items

client/values.yaml no longer states the float's version. It said "line at v0.8.4 as of 2026-08-12" while the float was already at v0.8.8. Beyond being stale, any version written there is stale on the next release — and a stale one reads as reassurance ("only two patches behind") for a gap that may be much larger. The comment now says the float moves without us and points at this watcher.

docs/SECURITY.md §4.1.1 is reframed as an explicit ceiling, with the evidence read from config.py at each tag:

release DB_USER resolution safe with the flag OFF
v0.8.0v0.8.4 os.environ.get("DB_USER", "edgeuser") yes — v0.8.4 is the ceiling
v0.8.8 and later self._require_env("DB_USER") — raises NO

It previously implied the unsafe release was a hypothetical "first 0.8.x release cut after 2026-08-11". It exists, and it is what the float points at today.

Tests

16 bats cases. The registry is stubbed through a documented seam that prints STUBBED RUN in the banner and the summary, so a stubbed log can never pass as a real audit.

The repo's own bats-hygiene guard caught a real defect in my tests: 31 assertions were bare [ ... ] on non-final lines, which bats treats as advisory — they could not fail their test. All are now || return 1. That guard earned its keep on this PR.

Mutation-verified after that fix:

mutation result
comparison always true 4 tests fail
discovery restricted to the images: block test 6 fails
a pin with no repository silently skipped tests 9 + 11 fail

One honest note on that third row: my first attempt at it was a sed whose pattern contained backticks and never matched, so it reported 0 failures. That was an inert mutation, not evidence of coverage — I re-did it with an asserted anchor. Worth recording, since "the mutation ran and nothing failed" and "the mutation never applied" look identical in a log.

bats scripts/tests/check-digest-drift.bats   16 passed
bats scripts/tests/bats-hygiene.bats          4 passed
shellcheck: clean    actionlint: clean    manifest.sha256: up to date

Not in make check

It needs the network and a docker daemon, and it is knowingly red today — the ingestor drift is the finding. A red target in the pre-push tier trains people to skip the tier. It runs daily via digest-drift.yml (unauthenticated: a watcher that needs a secret is a watcher that stops silently when the secret rotates). The bats suite is in make bats, which needs neither.

What this does not do

It does not advance any pin. Advancing is still scripts/resolve-ingestor-digest.sh --write, which carries the ordering ceiling from backend#1528. This watcher's only job is to make sure nobody finds out about drift from a manual sweep again.


Note

Medium Risk
Adds scheduled CI that depends on public registry/Docker availability and will report red on known ingestor float vs pin drift until pins are advanced deliberately; behavior is read-only and does not change deployed images.

Overview
Adds digest drift detection so registry float tags moving away from chart-pinned digests are caught without a manual sweep (backend#1853).

scripts/check-digest-drift.sh scans client/values.yaml for every digest / prodDigest, resolves the paired mutable label (e.g. channelTags.prod for ingestor prodDigest), and compares registry index digests via Docker. Outcomes are ok, DRIFT, UNRESOLVED (fail closed), or UNWATCHABLE (pin without resolvable repo/tag). Discovery is pin-driven across the whole values file—not only images:—with a DRIFT_RESOLVE_STUB seam for offline tests.

.github/workflows/digest-drift.yml runs the script daily (and on workflow_dispatch) with read-only permissions and no registry secrets.

make digest-drift is added but excluded from make check because it needs network/Docker and is expected to fail until someone deliberately advances pins.

Docs and chart comments stop embedding the ingestor float’s current semver (stale reassurance); docs/SECURITY.md §4.1.1 documents the v0.8.4 safe ceiling vs v0.8.8+ and points operators at the watcher instead of hand-maintained version lists.

scripts/tests/check-digest-drift.bats (16 cases) covers discovery edge cases and classification. Chart version bumps to 1.9.39.

Reviewed by Cursor Bugbot for commit 5deec84. Bugbot is set up for automated code reviews on this repo. Configure here.

…ckend#1853)

On 2026-08-12 the ingestor's `channelTags.prod: "0.8"` float moved to
v0.8.8, which removed the legacy edgeuser DB_USER fallback. Nothing
noticed; a manual sweep found it. Default prod edges were spared only
because prodDigest pins v0.8.2 and prodPin defaults to true.

DESIGNED TO SAADQBAL'S DIAGNOSIS, not to the symptom:

  "The disease is a moving label pointing at an immutable trust decision,
   with no watcher on the label. Anything about the build could have been
   the thing that changed -- the fallback just got there first."

So this script asserts NOTHING about a build's contents. There is no
DB_USER check, deliberately: such a check goes green the next time
something else moves, which is the failure this watches for rather than a
variant of it. It asks one property-agnostic question per pin -- does the
float still resolve to the digest we decided to trust?

The trusted versions are registered where they already were: the digest:/
prodDigest: fields of client/values.yaml. No second list to keep in sync;
adding a pin enrols it automatically.

Against the real chart: 3 pins found, squid agrees, the ingestor DRIFTS
(the real finding), and mysqlClient is reported UNWATCHABLE -- it carries
a pin but declares no repository, so nothing can tell you when its trust
decision goes stale. That is a genuine modelling gap, reported rather than
skipped.

TWO BUGS OF MY OWN, both found by running it, not reading it:

  1. images:-scoped discovery watched 1 of the 3 pins. squid's pin lives
     OUTSIDE images:, and an "empty field means skip" rule dropped
     mysqlClient without a word. Rewritten pin-driven over the whole file:
     a pin is watched or REPORTED, never skipped.

  2. IFS=$'\t' collapses runs of tabs, because tab is IFS whitespace. A
     record with an empty repository AND tag slid the pin into the wrong
     variable, leaving $pin empty, and the row was skipped in silence.
     This is the exact defect release-train's own parse-repos suite pins by
     name; 0x1f is not whitespace.

Also in this commit, per review:

  * client/values.yaml no longer states the float's version. It said "at
    v0.8.4" while the float was at v0.8.8 -- and any version written there
    is stale on the next release, reading as reassurance ("two patches
    behind") for a gap that may be far larger. The comment now says the
    float moves without us and points at this watcher.

  * docs/SECURITY.md 4.1.1 is reframed as an explicit CEILING with a table:
    v0.8.0-v0.8.4 safe with the flag off, v0.8.8+ NOT (config.py read at
    each tag). It previously implied the unsafe release was hypothetical;
    it exists and is what the float points at.

Tests: 16 bats cases, registry stubbed via a documented seam that prints
STUBBED on every run so a log cannot pass as a real audit. The repo's
bats-hygiene guard caught that 31 of my assertions were advisory -- a bare
[ ] on a non-final line cannot fail its test -- so all are now || return 1.

Mutation-verified after that fix:
  comparison always true                      -> 4 tests fail
  discovery restricted to the images: block   -> test 6 fails
  a pin with no repository silently skipped   -> tests 9 + 11 fail
(An earlier sed-based mutation of the third reported 0 failures; the
pattern contained backticks and never matched. Inert mutation, not
coverage -- re-done with an asserted anchor.)

NOT in `make check`: needs network + docker, and is knowingly red today
(the drift IS the finding). Runs daily via digest-drift.yml; the bats
suite is in `make bats`, which needs neither.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka
LukasWodka requested a review from saadqbal as a code owner August 13, 2026 07:23
@LukasWodka LukasWodka self-assigned this Aug 13, 2026
Comment thread scripts/check-digest-drift.sh
@LukasWodka
LukasWodka requested review from waqaskhanroghani and removed request for saqlainsyed007 and waqaskhanroghani August 13, 2026 07:27
… (backend#1853)

- resolve_index_digest ran docker buildx imagetools inspect / docker
  manifest inspect with no timeout, so a wedged daemon or stuck registry
  hung the daily job instead of failing closed as UNRESOLVED. Wrap them in
  _tmout (timeout/gtimeout, 30s) so a stuck call is non-zero -> UNRESOLVED.
- client/values.yaml changed, so the chart-content gate requires a
  Chart.yaml version bump: 1.9.34 -> 1.9.35.
@LukasWodka

Copy link
Copy Markdown
Contributor Author

bugbot run

Comment thread client/Chart.yaml Outdated
The version bump left appVersion at 1.9.34; app.kubernetes.io/version
follows appVersion, so installed objects would advertise the old chart
version. Keep them in lockstep as this chart does: appVersion 1.9.35.

@saadqbal saadqbal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Careful, well-argued PR — the pin-driven rewrite and the fail-closed paths (UNRESOLVED, zero-pins → exit 2, index-vs-per-arch, SIGPIPE capture-then-slice) all check out, and I reproduced the 3-pin discovery + 16 green bats + clean shellcheck locally. Two things below; not blocking, and the chart is clean today, but the first one pokes a hole in the "never skipped" invariant that's the whole point of this. Not approving yet — CI is still running.

Comment thread scripts/check-digest-drift.sh Outdated
Comment thread .github/workflows/digest-drift.yml Outdated
…eckout label

check-digest-drift.sh only matched a double-quoted sha256 at exactly
four-space indent, so a single-quoted (digest: SQ...SQ) or more-deeply
nested pin was dropped in silence -- and the PINS==0 guard cannot catch
that while any one conforming pin remains, so a run could print 'no drift'
with a pin unwatched. Make discovery quote- and indent-agnostic: a
canonical pin is watched as before; a pin off the structure is REPORTED
unwatchable, never skipped. Adds bats cases for the single-quoted repro,
the good-masks-sneaky case, and the off-structure -> UNWATCHABLE path.

Also correct digest-drift.yml's checkout pin comment: 11d5960a is v4.4.0
(repo-standard, on releases/v4), not v5.0.0; drop the stray double space.

addresses @saadqbal review, Bugbot, client#697.
@LukasWodka

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 5deec84. Configure here.

@LukasWodka
LukasWodka requested a review from aptracebloc August 13, 2026 11:00

@shujaatTracebloc shujaatTracebloc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran everything rather than reading it:

bats-hygiene.bats            18/18 ok, 0 failures
check-digest-drift.bats      19/19 ok
shellcheck check-digest-drift.sh   clean
Chart.yaml                   1.9.38 -> 1.9.39, version + appVersion in lockstep

The chart bump is there and paired correctly, which the version-bump gate requires for any client/values.yaml change — easy to forget on a PR whose subject is a script.

The design choice is the right one and I want to endorse it explicitly. Deliberately omitting a DB_USER assertion is correct: an assertion about the fallback goes green the next time something else about the build moves, which is the failure being watched for rather than a variant of it. "Does the float still resolve to the digest we decided to trust?" is property-agnostic and therefore doesn't rot. Registering trusted versions in the digest:/prodDigest: fields that already exist — no second list, new pins enrolled automatically — is what makes that hold up over time.

Confirmed both self-reported bugs are genuinely fixed, not just described:

  • IFS: line 210 uses IFS="$(printf '\037')", i.e. 0x1f, not tab. That's the exact lesson release-train's `parse-repos" suite pins by name — tab is IFS whitespace, so runs collapse and one empty field lets the next value slide into its place. Applied correctly here.
  • Pin-driven discovery: the images:-scoping is gone; a pin is watched or reported, never skipped (:148, :159). A watcher silently covering a third of its subject is precisely the class of bug this script exists to catch, so fixing it before merge matters more than usual.

UNWATCHABLE for mysqlClient is the right call — a pin with no repository: is a real modelling gap, and reporting it beats skipping it. Four finding classes (ok / DRIFT / UNRESOLVED / UNWATCHABLE) covers the space without collapsing "can't check" into "fine".

Nice detail: DRIFT_RESOLVE_STUB prints a loud *** STUBBED RUN — NOT a real audit. *** banner, and there's a test asserting it appears in both the banner and the summary. A test seam that can be mistaken for a real audit is how a green security check ends up meaning nothing.

Also good that "finding zero pins is an ERROR, not 'no drift'" is pinned — that's the vacuous-pass case.

LGTM.

@LukasWodka
LukasWodka merged commit 3d4a9ce into develop Aug 13, 2026
47 checks passed
@LukasWodka
LukasWodka deleted the fix/1853-digest-drift-watcher branch August 13, 2026 13:11

@saadqbal saadqbal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the fix — the discovery hole I held on is closed. Single- and double-quoted sha256: pins are now found at any indent, and a pin off the canonical 4-space structure is reported UNWATCHABLE rather than dropped. Reconstructed the exact case (one double-quoted + one single-quoted pin, the single-quoted one drifting): now 2 pin(s) found, the single-quoted DRIFT is reported, exit 1 — the old reader printed no drift / exit 0 on that same input. The three new bats cases fail against the old regex (confirmed by swapping it back in) and all 19 pass on head. Checkout label is right now too: 11d5960… really is v4.4.0 (v5.0.0 is 08c6903…), matching every other workflow here. Bugbot clean on this head.

One non-blocking heads-up: run against the real chart with both watchable floats agreeing, it still exits 1 because images.mysqlClient has no repository: and is reported UNWATCHABLE — so the daily watch is red from its first run until that pin gets a repo/tag or a recorded exemption. That's the watched-or-reported-never-skipped contract working as designed; just flagging so the first scheduled failure isn't a surprise. Approving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants