Uh oh!
There was an error while loading. Please reload this page.
fix(scripts): refuse a prod digest refresh past the edgeuser ceiling (backend#1528) - #692
Conversation
…(backend#1528) `resolve-ingestor-digest.sh --write` resolves the `channelTags.prod` float and knew nothing about the ordering ceiling that float sits above. While `serviceDbAccountsByEnv.prod` is false, prod still authenticates as the shared `edgeuser`, so the ingestor it runs must be a release that still HAS the edgeuser fallback. data-ingestors#468 removed that fallback. values.yaml pins `prodDigest` DELIBERATELY behind the float and explains why in prose — but it also tells you to refresh the pin "with the helper, never by hand", and the helper happily resolved straight past the ceiling. client#490 nearly shipped exactly that. Today the float resolves to a digest different from the pin, so the hazard is live, not theoretical. Fail closed in the helper instead of relying on prose: - Refuse `--write` unless `serviceDbAccountsByEnv.prod` is a definite `true`. Absent or unparseable reads also refuse — a chart edit must not be able to silently disarm the guard. - Refuse BEFORE the registry round-trip, so the reason isn't buried under network output and no call is wasted. - `INGESTOR_PIN_ALLOW_PRE_FLAG=1` overrides for a verified target release. - The guard reads the live flag rather than hardcoding a version, so it stops firing on its own once prod flips. Read-only resolution is untouched. Tests: scripts/tests/ingestor-pin-ceiling.bats — refusal, both escape hatches, pin left intact, no-registry-contact ordering, override, post-flip, read-only, sibling-`prod:`-key scoping, and both fail-closed reads. Removing the guard turns 7 of the 10 red.
shujaatTracebloc
left a comment
There was a problem hiding this comment.
Approving.
The awk reader is the part worth checking, and it fails closed on every shape I could think of, not just the ones the tests pin:
prod: "true"(quoted) → no match → refuseprod: true # comment→ no match → refuseserviceDbAccountsByEnv: # comment→ header regex misses, so rule 2 resetsin_block→ refuse
Every one of those is "not a definite true", and every one refuses. That's the correct polarity for a guard whose false-negative ships a broken pin to every prod edge.
Rule ordering is also correct and easy to get wrong: the header line itself starts with a non-space, so it would be caught by the in_block = 0 reset — it isn't, because rule 1 nexts first.
refusal happens BEFORE any registry round-trip earning its own test with a call-recording docker stub is the right call; that's the assertion that would rot silently if the guard were ever moved below resolution.
Assertions all carry || return 1 (#527), and the chart-version-guard is satisfied since this is scripts/ only.
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka
commented
Aug 12, 2026
Correction to the PR description, since it affects how you should weigh this. I originally wrote that refreshing the pin would move prod onto a build without the edgeuser fallback. That was wrong, and I have corrected the description. I checked every published tag: v0.8.2, v0.8.3 and v0.8.4 all predate data-ingestors#468. It is merged on The guard is therefore preventative, not a fix for a currently-broken path. Its value is that Everything in the test plan stands unchanged. Sorry for the churn in the description — I would rather you review it against an accurate risk statement. |
Summary
scripts/resolve-ingestor-digest.sh --writeresolves thechannelTags.prodfloat and knew nothing about the ordering ceiling that float sits above. This makes the ceiling executable instead of prose.While
serviceDbAccountsByEnv.prodisfalse, prod still authenticates as the sharededgeuser, so the ingestor it runs must be a release that still has the edgeuser fallback.data-ingestors#468removed that fallback.client/values.yamlpinsprodDigestdeliberately behind the float and says so in prose — but it also instructs you to refresh the pin "with the helper, never by hand", and the helper resolved straight past the ceiling.client#490nearly shipped exactly that.Being precise about the current risk
Running
--writetoday would in fact be safe, and it is worth saying so plainly rather than overselling this:channelTags.prod: "0.8"resolves tosha256:02da1eb…(v0.8.4)images.ingestor.prodDigestis pinned atsha256:05e1249…(v0.8.2, set by client#490)I checked every published tag: v0.8.2, v0.8.3 and v0.8.4 all predate #468. It is merged on
data-ingestorsdevelop/stagingbut is not onmain, so no released build carries it yet. The helper would currently move the pin from one safe build to another.The problem is what happens next.
channelTags.prodis a float, and the first 0.8.x release cut from that line ships #468. On that day the float silently crosses the ceiling and the helper's output changes meaning with no signal at all — same command, same chart, now a prod-breaking pin. Nobody re-reads a values.yaml comment before running a helper that the file itself tells them to run.So this guard is preventative, not a fix for something broken right now. It converts a comment that must be remembered into a check that cannot be forgotten, and it removes itself once prod flips.
What changed
--writeunlessserviceDbAccountsByEnv.prodis a definitetrue. Absent or unparseable reads also refuse — a chart edit must not be able to silently disarm the guard.INGESTOR_PIN_ALLOW_PRE_FLAG=1overrides for a target release you have verified still carries the fallback.The new
read_prod_service_db_accounts()reader follows the same block-scoping discipline as the two readers above it, so a siblingprod:leaf (channelTags.prod,imageTags.prod, …) can never be mistaken for the flag.Test plan
New
scripts/tests/ingestor-pin-ceiling.bats(10 tests), picked up automatically by CI'sbats scripts/tests/*.bats:--writewhile the flag isfalse; refusal names both escape hatchesINGESTOR_PIN_ALLOW_PRE_FLAG=1allows the refreshtrueprod:leaf cannot be mistaken for the flagEvidence, run locally on this branch's base (
6ed024a):bats scripts/tests/*.bats→ 956 ok / 0 failures (mine at 367–376)bats scripts/tests/bats-hygiene.bats→ passes (all 24 assertions hardened with|| return 1per the house rule)shellcheck --severity=error→ clean; full shellcheck identical to HEAD (4 pre-existing SC2295, on untouched lines)scripts/gen-manifest.sh --check→ up to date (this helper is not manifest-covered)Notes for the reviewer
Two bugs were caught during testing and are fixed here, mentioned so you know where to look hardest:
read_prod_service_db_accountsbefore definition and would have failed on every invocation.== "false", so deleting the key left it fail-open. It now requires a definitetrue, matchingfr-gate's fail-closed posture.Related: backend#1528 (ceiling), data-ingestors#468 (removed fallback), client#490 (near-miss).