Uh oh!
There was an error while loading. Please reload this page.
sec(1528): a machine verdict for the edgeuser DROP-readiness gate (backend#1528) - #896
Conversation
…ckend#1528) The gate is three criteria that must each hold under a DRIVEN workload, and it has to be evaluated three times per fleet (pre-revoke, post-revoke, post-drop) across the two fleets still carrying edgeuser. By hand that is six judgement calls on prose, and the criterion that matters most -- nothing resolves to edgeuser -- is the easiest to get wrong, because the interesting answer is a variable that is NOT there. The criterion this gate replaced could not fail: a point-in-time information_schema.processlist sample showed no edgeuser on a fleet where edgeuser was still root-equivalent and in active use, because consumers connect per-operation and disconnect. It would have passed before any of the work started. This is the machine form of the replacement, so the same shape of mistake cannot recur silently. The processlist query survives only as a labelled smoke test whose empty result is recorded as NOT evidence of absence. The tool is read-only -- no REVOKE, no DROP, no writes -- and never prints a password value; for every *_PASSWORD it asserts only SET or UNSET. Three design points that are the substance: * The identity variable list is DERIVED from each deployed pod%s own resolved environment, not from a list held in the script, so a credential variable added to the chart later is covered the day it ships. A hand-written list would agree with itself and disagree with the cluster -- which is what the superseded criterion actually did. * Criterion 3 counts as tb_ingest / tb_meta, NOT as root. information_schema is privilege-filtered, so a count read as root is the total and can never reveal a shrink. Hence --baseline-identity is mandatory: a root count and a tb_ingest count are different measurements, and #1528%s recorded staging baseline disagrees with dev%s almost certainly for exactly that reason. * The baselines have no defaults. A hardcoded per-fleet table of measurements is the backend#1729 defect this ticket keeps tripping over, so the script refuses to run without them. FAILS CLOSED throughout: an absent pod, a refused exec, unavailable logs, a missing driven cycle, an unreachable cluster and an unreadable count are each counted as findings and reported as "cannot tell", never as a pass. Verified: 22 verdict cases green via a stubbed kubectl (every refusal asserts the SPECIFIC finding, not a bare non-zero exit); registered in the single DRIFT_GUARDS declaration so it runs in the already-required drift job; drift 28/28. Mutation-proven with every anchor asserted applied -- forcing the tool to always say DROP-READY reddens 14 cases; dropping the fail-closed accounting reddens 3; disabling the edgeuser scan, the bootstrap-identity check, the log scan, the shrink comparison, or the mandatory baseline each redden their own; and counting as root instead of tb_ingest reddens 4. Documented as SECURITY.md 7.7, cross-referenced from 8.10 step 3. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…uard (backend#1528) The required `quality / pipefail early-close` job flagged three lines. Under errexit+pipefail a reader that closes early (grep -q on first match, head after N lines) can make the upstream SIGPIPE the pipelines status, so the validation or the excerpt fails for a reason unrelated to its input. edgeuser-drop-readiness.sh:77,78 printf | grep -qE -> case edgeuser-drop-readiness-verdicts.sh:140 printf | sed | head -30 -> capture The two validations use `case` rather than a here-string: no pipe and no subprocess at all, and it matches the two `case` validations immediately above instead of introducing a second idiom for the same job. Verified with the REAL guard, not a proxy: scripts/pipefail-early-close.sh from tracebloc/.github reports all three before and exits 0 after. AND THE VALIDATION WAS UNTESTED. The suite covered MISSING baselines and an unknown identity, but never a present-and-malformed count -- so the numeric guard this change rewrote had no assertion behind it. Adds abc / 8.7 / empty / 12x / -4 across both keys, plus a control that a plain integer still passes, since a `case` pattern rejecting everything would satisfy the refusals alone. verdicts 22 -> 33 passed, 0 failed. Mutation-proven: deleting the numeric guard reddens 8 of the 10 new cases (the two empty ones correctly still refuse via the earlier is-required guard) and leaves the control green. shellcheck -S warning -x and bash -n clean on both files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
# Conflicts: # Makefile
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
saqlainsyed007
left a comment
There was a problem hiding this comment.
Requested changes — and note that green CI here is a symptom, not a clearance. I verified the main finding against the chart.
HIGH — the DROP-readiness gate is structurally always-red on a real fleet.check_workload (edgeuser-drop-readiness.sh, the requests-proxy call at ~:176 and the assertions at :159-166) asserts DB_BOOTSTRAP_USER=root, SERVICE_DB_ACCOUNTS, and PER_EXPERIMENT_DB_CREDS on every workload. But client/templates/requests-proxy-deployment.yaml at this head renders only SERVICE_DB_ACCOUNTS and TB_META_USER — it has no DB_BOOTSTRAP_USER and no PER_EXPERIMENT_DB_CREDS (those govern the mint/bootstrap DDL, which only jobs-manager runs). So on a correctly-retired production fleet, requests-proxy trips two bad findings → FAILURES>0 → NOT DROP-READY, in every phase, forever. The tool can never authorize the DROP it exists to gate, and it prints a factually wrong reason ("the mint falls back to edgeuser") for a pod that never mints. Fix: assert the bootstrap/per-experiment gates only on the workload that actually carries them (jobs-manager); on a consumer like requests-proxy, check only what's relevant (SERVICE_DB_ACCOUNTS, and that nothing resolves to edgeuser).
MEDIUM — the happy-path test is vacuous and is what's hiding the above.scripts/tests/edgeuser-drop-readiness-verdicts.sh:103 (clean_fleet) writes the identical $common env — including DB_BOOTSTRAP_USER=root and PER_EXPERIMENT_DB_CREDS=true — to BOTH the jobs-manager and requests-proxy fixtures. Real requests-proxy renders neither, so case 1 ("a clean retired fleet is DROP-READY") passes only against a fleet shape that cannot exist, and every mutation case is differenced against that fiction. A fixture that mirrors the real chart (requests-proxy without those two vars) flips case 1 to NOT-READY and reddens on the real bug. That's why CI is green.
Good news on the safety direction: the failure is fail-closed (it under-reports READY, never falsely authorizes a DROP), and the fail-closed handling elsewhere is thorough (unreachable cluster → exit 2; missing pod/exec/count/log/baseline → counted failure; empty processlist treated as non-evidence). Two low notes are inline (a bare USER= env can match the *USER scan; Criterion 1 only catches edgeuser in *USER-named vars with exact value, so a DSN/URL cred would evade) — optional. Fix the per-workload expectations and the fixture and I'll re-review; the rest of the tool is well-built.
# Conflicts: # Makefile
…d it (backend#1528) Saqlain on #896, both findings confirmed against the chart: HIGH -- check_workload asserted DB_BOOTSTRAP_USER and PER_EXPERIMENT_DB_CREDS on EVERY workload, but requests-proxy-deployment.yaml renders neither (they govern the mint DDL, which only jobs-manager runs). On a correctly-retired fleet requests-proxy tripped two findings in every phase, forever, so the tool could never authorize the DROP it exists to gate -- while printing a wrong reason about a pod that never mints. The gates are now asserted per workload. MEDIUM -- the fixture wrote the SAME env to both pods, so the happy path was differenced against a fleet shape that cannot exist. That is why CI was green. The requests-proxy fixture now mirrors the chart, and the case that DEMANDED the defect ("a finding on requests-proxy too") is replaced by the two true assertions. Restoring the unconditional check now reddens 4 cases including "a clean retired fleet is DROP-READY". Before, it was green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 28, 2026
@saqlainsyed007 — both findings confirmed against the chart and fixed in HIGH — verified before fixing. Measured the two templates at this head: So requests-proxy tripped two MEDIUM — and this is the part worth dwelling on. The fixture wrote the identical Fixing that surfaced something sharper than a stale fixture: the test case that should have caught this was the case demanding it.
Mutation proof, which is the real answer to "why was CI green". Restoring the unconditional check: Four cases, including the happy path. Under the old fixture that same mutation was green — it was the shipped code. Anchor asserted to match exactly once before applying, so the mutation is not inert. Suite: 34 passed, 0 failed (was 33). The two low notes are untouched, as you marked them optional. Ready for re-review. |
…atching bare USER (backend#1528) Builds on the fix already on this branch rather than replacing it. A co-driver session landed the same correction for @saqlainsyed007%s HIGH while I was working it -- their `mint` / `consumer` role parameter and their per-workload fixture are both correct, and my push was rightly refused as non-fast-forward. This is the delta, not a re-do. THE ROLE IS NOW DERIVED AS WELL AS DECLARED. `mint`/`consumer` at the call site is a written-down copy of which workload renders which gate -- and a copy is precisely what caused the bug the parameter exists to fix: it agrees with itself while the chart moves underneath it. If a gate is added to requests-proxy, or the mint moves to a third workload, the label keeps asserting yesterday%s shape in silence. assert_role_matches_chart derives the role from the template and makes a DISAGREEMENT a finding. The explicit label is kept on purpose: it stays readable at the call site, and the derivation is what keeps it honest. Neither alone is enough -- a bare derivation is unreadable at the point of use, a bare label is a copy. FAILS CLOSED: an unreadable template is "cannot tell", never "the label is fine". A copy of the tool without the chart beside it therefore refuses rather than silently trusting the label, and a test runs the tool from a bare directory to prove it. BARE `USER` IS NOT A DATABASE IDENTITY. `^[A-Z0-9_]*USER=` matched the plain Unix `USER`, so a container with `USER=nobody` had an "identity" whose PASSWORD counterpart was absent -- a false finding on a healthy pod (Saqlain low note). Now `^[A-Z0-9][A-Z0-9_]*_USER=`, which still matches TB_META_USER. My first attempt was `[A-Z0-9]+_USER`, which excludes underscores in the PREFIX and so quietly matched nothing at all; the harness caught it as "the env declares no *_USER variable". Five cases added, and two of them exist because a mutation was GREEN first: * a DB_BOOTSTRAP_USER that is neither root nor empty -- the `*)` arm could be turned into an `ok` with every previous case still passing; * a bare USER= env is not an identity; * the chart being absent is a finding, not a trusted label; * a declared role that disagrees with the chart is a finding; * THE FIXTURE MIRRORS EACH TEMPLATE, derived from the templates. The fixture is correct now but nothing held it that way, and a fixture that drifts back re-hides the exact bug this suite was extended for. This replaces a claim in a comment with a check, which is the same defect one level up. Verified: verdict harness 39 passed / 0 failed; drift 30/30; helm unittest 600/600; shellcheck -S warning + bash -n clean. Mutation-proven, every anchor asserted applied: the cross-check removed -> RED; the derivation always answering "consumer" -> RED; an unreadable template silently trusting the label -> RED; bare USER matched again (both sites) -> RED; the disagreement no longer reported -> RED. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 28, 2026
@saqlainsyed007 — both findings are fixed, and you were right that the green CI was the symptom rather than the clearance. HIGH — fixed. Confirmed against the chart before touching anything: Worth admitting how I missed it: I had the evidence. My own check was MEDIUM — fixed. The fixture now mirrors each template, so the happy-path case is no longer differenced against a fleet shape that cannot exist. Both low notes taken, not declined:
One addition beyond your list, and it's the part I'd most like your eye on. A co-driver session landed the same HIGH fix while I was working it, using a So the role is now derived from the template as well as declared, and a disagreement is a finding. The explicit label stays because it's readable at the point of use; the derivation is what keeps it honest. It fails closed — an unreadable template is "cannot tell", never "the label is fine" — and a test runs the tool from a directory without the chart to prove that. Two of the new cases exist because a mutation came back GREEN first: nothing covered a Verified: verdict harness 39 passed / 0 failed, drift 30/30, Ready for re-review. |
aptracebloc
left a comment
There was a problem hiding this comment.
This is the right shape for the gate — deriving "nothing resolves to edgeuser" from each pod's own resolved env instead of a hand-written list, demoting the structurally-unfailable processlist query to a labelled non-evidence smoke test, and failing closed on absent-pod / refused-exec / unreadable-count / missing-driven-cycle. The test suite is strong: I reproduced the always-PASS mutation and 14 cases redden, the assertions key off specific finding substrings, and the fleet stubs are synthetic (derived, not restating the tool). SECURITY.md §7.7/§8.10 is accurate against the code.
Requesting changes on one reproduced false-PASS on the most safety-critical criterion — because a gate that authorizes an irreversible DROP USER edgeuser must never render "couldn't tell" as ready.
🔴 Blocker 1 — ingestion DB_USER absent renders [ok], not fail-closed (edgeuser-drop-readiness.sh:213).
ivars=$(K exec"$ing" -- env … | grep -E '^DB_USER=|^DB_PASSWORD='|| true)if [ -z"$ivars" ];then untold …
elif grep -q '^DB_USER=edgeuser$'<<<"$ivars";then bad …
else ok "ingestion ($ing): DB_USER=$(…)"# <-- reached when DB_PASSWORD is present but DB_USER is ABSENTfiIf DB_PASSWORD is set but DB_USER is entirely absent from env, ivars is non-empty (so the -z guard passes), the edgeuser grep doesn't match, and control falls to else → ok "…DB_USER=" with an empty value, no FAILURES++ → the tool prints DROP-READY / exit 0. I reproduced it. This is exactly the "the interesting answer is a variable that isn't there" failure the tool exists to kill, and it's not hypothetical: prod is digest-pinned to the 0.7 ingestor whose edgeuser DB_USER default is applied inside the container and never appears in env (see _helpers.tpl ~L611/716-720, backend#1752) — so this pod would authenticate as edgeuser while the gate greenlights the DROP. Note the asymmetry: check_workload's criterion-1(a) already treats "no *_USER at all" as untold (fail-closed) — the ingestion check must do the same. Fix: guard on the DB_USER line specifically (e.g. require a non-empty ^DB_USER= match to reach ok; DB_PASSWORD-present-but-DB_USER-absent → untold/bad).
🔴 Blocker 2 — no test covers the DB_USER-absent path (edgeuser-drop-readiness-verdicts.sh:208). The ingestion cases only exercise DB_USER=edgeuser and no-ingestion-pod, so Blocker 1 ships green. Add a case: clean fleet, ingest pod env = DB_PASSWORD=x only, expect non-zero + the specific "could not read DB_USER" finding. (This is what would have caught #1 — and it's the whole point of a mutation-proof gate here.)
🟡 Blocker 3 — empty-but-successful log scored clean (edgeuser-drop-readiness.sh:241). In scan_logs, a kubectl logs that succeeds with empty output sets any=1 and scores both checks [ok]. So "cycle aged out of --since 2h", "pod restarted and its buffer reset", or a stale pod all read as "criterion 2 clean" with no evidence the driven cycle was actually observed — "could not look" rendered as "looked and clean", against the tool's own doctrine. The any=0 guard doesn't catch the per-pod empty case. Criterion 2 is corroboration so this is softer than #1, but it also means SECURITY.md §7.7 slightly overstates the log-scan fail-closed guarantee (only a failedkubectl logs is a finding today).
Minor / non-blocking (worth handling while in here): the ingestion pod selector at :203 lacks the $2=="Running" filter pod_of() uses; criterion 1(a) at :155 only matches *_USER=edgeuser exactly, so an edgeuser inside a DATABASE_URL= DSN would go undetected (not in today's chart, but a gap against the "nothing resolves to edgeuser" guarantee); secret_val re-fetches all Secrets 5× (:300); and --baseline-identity is mandatory but inert (compare() always reads the live count) — a one-line "assertion-of-intent, not load-bearing" comment would prevent a maintainer misreading it.
— drafted with Claude Code
… not a clean one (backend#1528) Saqlain reproduced a false-PASS on the criterion that authorizes an irreversible DROP USER edgeuser. Both blockers confirmed and fixed: 1. $ivars matched DB_USER *or* DB_PASSWORD, so a pod with a password and no DB_USER cleared the -z guard, missed the edgeuser grep, and fell to ok with an EMPTY value -- DROP-READY, exit 0. Prod is digest-pinned to the 0.7 ingestor whose edgeuser DB_USER default is applied inside the container and never appears in env, so this is the shape that would actually occur. check_workload already treats "no *_USER at all" as untold; the asymmetry was the defect. 3. scan_logs scored a SUCCESSFUL but empty kubectl logs as [ok] on both checks, so an aged-out cycle or a restarted pod read as "criterion 2 clean". Empty is now untold. 2. Both paths now have cases. clean_fleet also had to stop writing EMPTY logs as its happy path -- the same "differenced against a state that cannot occur" shape as the requests-proxy fixture. 41 passed, 0 failed (was 34). Both mutations redden their own case at exit 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 28, 2026
@saqlainsyed007 — all three fixed in 🔴 1 — the DB_USER false-PASS. Confirmed: Your point about the asymmetry is the one I'd have missed: 🔴 2 — the missing case. Added, and it reproduces exactly what you described: under the mutation the tool exits 0. 🟡 3 — the empty log. Fixed, and it forced a fixture correction worth flagging: Verification. 41 passed / 0 failed (was 34). Two mutations, each reddening only its own case, anchors asserted to match exactly once:
The four minors are untouched as you marked them non-blocking — the Running-filter on the ingest selector, the DSN-embedded edgeuser gap, the 5× Secret re-fetch, and the inert |
…edgeuser (backend#1528) @aptracebloc%s four "non-blocking, worth handling while in here" items. His three blockers were already fixed on this branch by bbb3e7a; this is the remainder, and the first one turned out to be more than a nit. THE INGEST SELECTOR: not just a missing Running filter. `kubectl exec` needs a running container, so an ingestion Job pod that has already SUCCEEDED cannot be exec%d -- picking one yields an empty env and an unexplained "cannot tell". With the filter the message can name the real precondition instead: this check has to run WHILE the ingestion is in flight. The old advice, "drive one, then re-run", could not be followed, because a completed run leaves no readable env. THE DSN GAP, and the fix is where the env is READ, not where it is checked. The DB_USER checks match that name exactly, so a connection string under any other name walked past them. The env was being filtered to DB_USER/DB_PASSWORD at read time -- which is what made the gap structural rather than incidental. The full env is now kept and scanned for the substring, reported BY NAME with the value withheld (a matched value can carry a password, and a test asserts the secret never reaches the output). It runs regardless of the DB_USER verdict, because a pod can hold both a correct DB_USER and a stale DSN. secret_val re-pulled the namespace%s entire Secret set once per key. Fetched once now; same data, one call. --baseline-identity is mandatory and NOT load-bearing -- compare() always reads the live count. Said so in a comment, because the next maintainer would otherwise read it as a switch and "fix" it by wiring it into the query. It exists to make the operator state which identity produced the baseline, since a root count and a tb_ingest count are different measurements. SECURITY.md 7.7 said the log scan fails closed on "an unreadable log". After bbb3e7a that claim is TRUE but imprecise -- it did not cover an empty-but- successful read, which is the case Arturo actually caught. It now names every fail-closed condition the code implements, including the empty window, no RUNNING ingestion pod, and a DB_USER that is absent rather than wrong. Verified: verdict harness 45 passed / 0 failed (5 new cases: a Succeeded ingest pod is refused, an ingestion DSN is caught by name, its value is withheld, and a good DB_USER does not excuse a stale DSN beside it); drift 30/30; helm unittest 600/600; shellcheck -S warning + bash -n clean. Mutation-proven, every anchor asserted applied: the Running filter dropped -> RED; the DSN scan disabled -> RED; the env re-filtered before the scan (the original hiding place) -> RED; the DB_USER-absent guard removed -> RED. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 28, 2026
@aptracebloc — all four minors done. Your three blockers were already closed on this branch by The ingest selector. You flagged the missing
The old advice — "drive one, then re-run" — could not be followed, because a completed run leaves no readable env. That's a runbook correction, not a filter. The DSN gap — fixed where the env is read, not where it's checked. This is the part I'd point at: the env was being filtered to
And your Blocker 3 had a documentation tail I closed too. You noted §7.7 "slightly overstates the log-scan fail-closed guarantee". After Verified: 45 passed / 0 failed (5 new cases), drift 30/30, Ready for re-review. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Bugbot Medium, and it inverts the safety direction of the one check standing in
front of an irreversible DROP.
The verdict grepped a GROUP_CONCAT of every non-system account.
group_concat_max_len defaults to 1024 BYTES, so on a fleet carrying
per-experiment users -- required before this phase is even reachable -- the list
truncates and edgeuser falls off the END. A truncated-but-non-empty string was
then reported as "edgeuser is absent": the tool saying gone about an account it
never saw. The empty case WAS failed closed via ${remaining:-edgeuser}, which is
why only the truncated shape got through -- the guard was there and could not
see this.
Now a COUNT(*) WHERE user=edgeuser, which cannot truncate and answers the
question asked instead of a broader one that has to be searched. Non-numeric or
empty fails closed and is explicitly NOT reported as absent. The inventory stays
for the log, as rows rather than one concatenated string, and nothing decides on
it.
THE TEST TOOK THREE ATTEMPTS AND THE FIRST TWO WERE VACUOUS, which is the part
worth recording:
1. stub answered every mysql.user query with one canned string, so the COUNT
received a comma-joined list and failed closed -- looked like a harness bug.
Fixed by teaching the stub the two distinct queries.
2. stub still never truncated, so the mutation restoring GROUP_CONCAT passed.
Fixed by making the stub cut a GROUP_CONCAT answer at 1024 bytes, like the
real server.
3. fixture had 60 users = ~860 bytes, UNDER the cut, so nothing truncated.
Raised to 100 = ~1520 bytes with edgeuser at offset 1500.
Only then did the mutation redden, and with the right symptom: exit 0 wanted 1,
the old code declaring the DROP clean while edgeuser is present.
45 -> 47 passing, plus a control that a long CLEAN fleet still passes, so a check
that merely failed on long lists would not satisfy the new case.
shellcheck -S warning -x clean, make drift all 30 guards green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>…ckend#1528) Bugbot High. scan_logs matched on pod NAME alone, so completed ingestion Jobs from earlier cycles were scanned too. Their logs fall outside --since (default 2h), the empty-log rule scores that untold -- a cannot-tell, counted as a failure -- and a fleet that had EVER ingested could therefore never print DROP-READY. The tool could not authorise the DROP it exists to gate. Worth naming the shape: the empty-log rule is CORRECT and is kept. This is that rule meeting a population it was not written for -- Job pods that finished before the window. A Succeeded Job from a previous cycle is not evidence about THIS cycle in either direction, so it is skipped and counted rather than judged. If nothing is Running that IS a cannot-tell and is still reported as one, so the doctrine that "could not look" must never render as "looked and clean" is intact. 47 -> 49 passing. Two mutations, each anchor asserted applied: match on NAME only again -> the stale-Jobs case reddens, exit 1 wanted 0, which is the gate refusing a clean fleet skip EVERY ingest pod -> 3 red, so the over-broad direction is caught too The second mutation is why the control exists: implementing the skip as "ignore ingest pods" would have satisfied the new case while blinding criterion 2 entirely, so a dirty RUNNING pod beside finished Jobs is pinned separately. shellcheck -S warning -x clean, make drift all 30 guards green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
commented
Aug 28, 2026
bugbot run |
There was a problem hiding this comment.
✅ 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 dd21c08. Configure here.
…ckend#1528)
Bugbot High, and it had been RESOLVED WITH NO REPLY -- a bare blind-resolve. The
bugbot gate refused it on exactly that basis ("fix, or reply with the ticket and
resolve"), which is the gate working: it listed the finding as outstanding even
though the thread showed resolved.
The finding is real and I reproduced it before touching anything. pod_of piped
kubectl get pods into awk with an early exit; the exit closes the pipe, kubectl
takes SIGPIPE, and under this script set -euo pipefail the pipeline status is
141 -- so pod=$(pod_of x), a bare assignment, aborted the WHOLE script. No
finding, no cannot-tell, no verdict at all: the one output this tool must never
produce silently. Demonstrated with a producer long enough for SIGPIPE to land:
rc=141 and the next line never runs; after the fix rc=0.
Fixed in depth -- awk drains the input via !seen++ instead of exiting, and both
bare-assignment call sites carry || true. count_as gets the same guard: it is
reached today through "$(count_as ...)" where errexit does not fire, but that is
a property of the call site and a future direct assignment would inherit the
abort.
WHAT THE SUITE CANNOT TEST, said out loud rather than papered over. The kubectl
stub ends its get-pods branch with an unconditional exit 0, which discards
whatever status cat died of, so SIGPIPE is masked before the script sees it:
stub ending in exit 0 -> rc=0, script continues (what this suite sees)
stub without the exit 0 -> rc=141, script aborts (what a cluster does)
Four mutation attempts on the fix stayed green for that reason. The new case is
therefore labelled for what it actually pins -- a large namespace still reaches a
verdict -- and the fix evidence is the out-of-suite measurement. Making exit
status observable means changing the shared stub for every case in the file, so
it is called out rather than done quietly.
49 -> 50 passing, shellcheck -S warning -x clean, make drift all 30 guards green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>LukasWodka
commented
Aug 28, 2026
bugbot run |
Uh oh!
There was an error while loading. Please reload this page.
…the cycle (backend#1528)
Bugbot Medium, and the THIRD instance in this file of one shape: a check that a
real fleet can never satisfy.
The picker took any Running pod whose name contains ingest. Pod names are
{release}-{workload}-{hash}, so a release or namespace containing ingest --
tracebloc-ingest, say -- makes ...-jobs-manager-... and ...-requests-proxy-...
match too. kubectl get pods has no guaranteed order, so criterion 1 could pick a
control-plane pod, find no DB_USER, and report cannot-tell forever.
The exclusion is DERIVED from the workloads this script already checks -- exactly
the two names passed to check_workload -- so it cannot drift away from what the
chart renders without that call site changing too.
Also swept the class rather than the instance, which I should have done when I
fixed pod_of: this picker had the same awk `exit` shape. Stated precisely because
it is tempting to overclaim -- here the pipeline is already `|| true`-guarded, so
it was NOT an abort, unlike the bare assignment pod_of fed. It is now `!seen++`
for consistency, because leaving one of two spellings behind is how the next
reader concludes the early exit is fine.
50 -> 51 passing. The fixture puts the control-plane pods FIRST, which is the
order that produced the bug. Mutation-proven, anchor asserted applied: dropping
the exclusion reddens that case with exit 1 wanted 0 -- the gate refusing a clean
fleet. shellcheck -S warning -x clean, make drift all 30 guards green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>LukasWodka
commented
Aug 28, 2026
bugbot run |
There was a problem hiding this comment.
✅ 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 467a0a2. Configure here.
aptracebloc
left a comment
There was a problem hiding this comment.
Re-reviewed at 467a0a2. Both blocking findings from my earlier pass are addressed, and I verified each by mutation rather than by eye — this gate authorizes an irreversible DROP, so "the guard is there" is not enough on its own.
Ingestion DB_USER absent now fails closed. With DB_PASSWORD present and DB_USER absent,
ivarsis non-empty so the earlier branches don't fire, and the newelif [ -z "$iuser" ]rendersuntold(cannot-tell → failure), not[ok]. Neutralising that guard flips the "PASSWORD but no DB_USER" case to exit 0 / DROP-READY and reddens the test — so the case is real, not vacuous.Empty-but-successful
kubectl logsisuntold.scan_logstreats an empty log as cannot-tell andcontinues beforeany=1; neutralising it reddens the empty-log case.
The four minors (Running-only ingest selector, DSN/URL edgeuser scanned by name with the value withheld, secrets fetched once, --baseline-identity mandatory-but-inert) are all still handled, each with a test. The four commits since my last pass (COUNT-not-GROUP_CONCAT truncation fix, stale-Job skip, pod_of SIGPIPE, control-plane name exclusion) each ship a green case plus a control, and I inverted the post-drop arm to confirm "edgeuser still present" cannot read as clean.
Ran the verdict suite locally against stubbed kubectl: 51 passed, 0 failed. shellcheck -S warning -x clean. CI and Bugbot green at head, no open threads.
Approving. Nice work threading the fail-closed doctrine through every new population the checks now meet.
— drafted with Claude Code
LukasWodka
commented
Aug 28, 2026
Both blocking findings are fixed at HIGH — per-workload expectations. That is your "structurally always-red on a real fleet", reproduced on demand. MEDIUM — the vacuous fixture. Separately: Both low notes are covered too, not deferred — Suite at Your |
saqlainsyed007
left a comment
There was a problem hiding this comment.
Re-reviewed at 467a0a2e. Both my findings are genuinely fixed in code, not just resolved.
Finding 1 (structurally always-red) — fixed.check_workload now takes a mint|consumer role ($3); the DB_BOOTSTRAP_USER=root and PER_EXPERIMENT_DB_CREDS assertions run only for mint, and requests-proxy is called as consumer. That matches the chart at this head (requests-proxy renders only SERVICE_DB_ACCOUNTS + TB_META_USER; jobs-manager carries both mint gates), so a correctly-retired requests-proxy is no longer flagged NOT-READY. A real leak is still caught — the *_USER=edgeuser and DSN-value scans run on every workload — and assert_role_matches_chart derives the role from the template and makes a label/chart mismatch its own finding, failing closed on an unreadable template.
Finding 2 (vacuous happy-path) — fixed and mutation-proof.clean_fleet's requests-proxy fixture now mirrors the chart (consumer env, no mint gates). Confirmed reverting the code fix reddens case 1 and the new gate2b regression case, and reverting the fixture trips the new "fixture mirrors template" assertion. Suite 51/51.
Nice work on the extra Bugbot fixes too (SIGPIPE abort, control-plane exclusion, empty-log = cannot-tell, group_concat truncation → COUNT). No new defects, all 12 threads resolved, CI green, no conflicts. Approving.
Uh oh!
There was an error while loading. Please reload this page.
Makefile conflict: both sides appended to DRIFT_GUARDS -- develop gained edgeuser-drop-readiness-verdicts.sh (client#896), this branch adds gate-default-prose-agreement.sh. Resolved as the UNION (31 guards), asserting no develop guard was dropped rather than eyeballing it. The two edgeuser-drop-readiness files showed as deletions against develop only because #896 added them after this branch point; the merge restores them. drift: 31/31 green. helm-unittest: 36 suites / 622 tests. edgeuser-drop-readiness-verdicts: 51 passed. Chart 1.9.85 = develop 1.9.84 + 1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Closes tracebloc/backend#2760 — the tooling leg, carved out as its own
closeable slice so this PR has a reference it can legitimately close.
Part of tracebloc/backend#1528, which stays open: the ops legs (driving
the three phases, then the irreversible DROP USER) are not in this PR.
Why
#1528's DROP-readiness gate is three criteria that must each hold under a driven workload, and it has to be evaluated three times per fleet — pre-revoke, post-revoke, post-drop — across the two fleets still carrying
edgeuser. By hand that's six judgement calls on prose, immediately before an irreversibleDROP USERon a customer-facing database.And the criterion that matters most — "nothing resolves to
edgeuser" — is the easiest to get wrong, because the interesting answer is a variable that isn't there.The criterion this gate replaced could not fail. A point-in-time
information_schema.processlistsample showed noedgeuseron a fleet whereedgeuserwas still root-equivalent and in active use, because consumers connect per-operation and disconnect — it would have passed at every moment in #1528's life, including before any of the work started. This PR is the machine form of the replacement, so that shape of mistake can't recur silently. Theprocesslistquery survives only as a labelled smoke test whose empty result is printed as not evidence of absence.What it is
docs/migration-tools/edgeuser-drop-readiness.sh— read-only: noREVOKE, noDROP, no writes of any kind. It never prints a password value; for every*_PASSWORDit asserts only SET or UNSET, and passwords reach the client viaMYSQL_PWDso they never appear in a pod's argv.Three design points are the actual substance:
1. The variable list is derived from the deployed surface. Criterion 1 reads each pod's own resolved environment (
kubectl exec … env, which includesenvFrom/secretKeyRefvalues a manifest read can't resolve) rather than a list held in the script. A credential variable added to the chart later is covered the day it ships. A hand-written list would agree with itself and disagree with the cluster — which is precisely what the superseded criterion did.2. Criterion 3 counts as
tb_ingest/tb_meta, never asroot.information_schemais privilege-filtered, so a count read asrootis the total and can never reveal a shrink — root sees everything by definition. That's why--baseline-identityis mandatory: a root count and atb_ingestcount are different measurements, and comparing them silently is how a shrink hides.(An earlier version of this paragraph blamed #1528's 3-vs-4
metadatabaseline disagreement on the reading identity. That was wrong, and the answer was already on the ticket:metadatawent 3 → 4 whenperExperimentDbCredswas enabled, because that createsexperiment_db_creds. The identity rule above stands on its own — root genuinely cannot reveal a shrink — but it was not the cause of that particular discrepancy. The practical consequence for prod is the opposite of what I implied: expectmetadatato grow by exactly one table when the flag goes on, which the script reports asgrew by 1 (not a shrink).)3. The baselines have no defaults. A hardcoded per-fleet table of measurements is the backend#1729 defect this ticket keeps tripping over. The script refuses to run without them.
Fails closed throughout. An absent pod, a refused
exec, unavailable logs, a missing driven cycle, an unreachable cluster, an unreadable count — each is a finding, reported as[????] … cannot telland counted as a failure. "We could not look" must never read the same as "we looked and it was clean."Verification
scripts/tests/edgeuser-drop-readiness-verdicts.sh— 22 verdict cases, all green, driven by a stubbedkubectlrendering a synthetic fleet. Every refusal case asserts the specific finding, never a bare non-zero exit: any failure satisfies that, and a test that can't say which refusal fired is a coin toss reporting success.Registered in the single
DRIFT_GUARDSdeclaration, so it runs inside the already-required drift job rather than in a job nobody must wait for.Mutation-proven, every anchor asserted applied:
DROP-READYrootinstead oftb_ingest*_USER == edgeuserscan disabledDB_BOOTSTRAP_USERtreated as fineBuilding the harness also found four real bugs — three in the stub's dispatch order (the mysql queries are themselves invoked as
-- env MYSQL_PWD=… mysql …, so a-- envcase tried first swallowed all of them) and one in the test's owngrep, which read the expected substring as an option. None in the tool, but that is what the harness is for.Also:
drift28/28,shellcheck -S warning -x+bash -nclean on both files.Docs
New
SECURITY.md§7.7 (in the verification section, where a verifier belongs), cross-referenced from §8.10 step 3 so theREVOKE/DROPstep points at the tool instead of at prose.No chart content changes, so no
Chart.yamlbump.🤖 Generated with Claude Code
Note
Medium Risk
Read-only tooling, but a false DROP-READY verdict would gate an irreversible MySQL DROP on prod; impact depends on verifier correctness, mitigated by extensive stubbed verdict tests in drift CI.
Overview
Adds read-only, fail-closed ops tooling for backend#1528’s three-criteria gate before irreversible
edgeuserREVOKE/DROP, plus CI and docs so fleets use it instead of hand checks or the old processlist heuristic.docs/migration-tools/edgeuser-drop-readiness.shprints a single DROP-READY / NOT DROP-READY verdict against a live cluster (kubectlexec/logs, mandatory fleet baselines,--phase pre-revoke|post-revoke|post-drop). It checks: (1) no consumer env resolves toedgeuser(derived from pod env, mint vs consumer gates aligned to chart templates, running ingestion only, DSN scan); (2) no legacy-identity warnings or 1045 in logs over--since; (3) privilege-filtered table counts vs S0 baselines astb_ingest/tb_meta, with post-dropmysql.userverified via COUNT (not truncatingGROUP_CONCAT). Passwords are never printed.scripts/tests/edgeuser-drop-readiness-verdicts.shexercises happy path and every refusal path via a stubkubectl; it is wired intoDRIFT_GUARDSin the Makefile so required drift runs it.docs/SECURITY.mdadds §7.7 (how to run and trust the verifier) and points §8.10 S3 at that script instead of eyeballing readiness.Reviewed by Cursor Bugbot for commit 467a0a2. Bugbot is set up for automated code reviews on this repo. Configure here.