Repository navigation
A credential in a command line is read by every process on the host, and on this fleet that is measured - #9226
Merged
Merged
Conversation
…and on this fleet that is measured
Three facts taken on srv1 on 2026-08-25 decide this class: /proc carries no
hidepid, /proc/PID/cmdline is mode 0444, and the CI runner slots are ordinary
host systemd services in the host pid namespace rather than containers. So an
argv word is not readable by "other processes owned by the same user" -- it is
readable by any job payload executing on a runner slot, in a repository whose
workflows a pull request can influence. The usual reason this class gets
deprioritized is an assumption that the reader must already be privileged; here
the reader is unprivileged and arrives through the front door.
The observation that opened the incident is NOT in this repository -- it is the
session container entrypoint projecting a forwarded credential roster into
`tmux new-session -e NAME=VALUE`. That is reported to its owner rather than
edited around, with a verified repair shape recorded in the new carrier. What
this commit does is ask the same question of our own tree, and it had three
answers.
extdeps.bmc.http declared `password: Secret` on GetSystem, GetPower and
GetThermal and then interpolated it into `-u {username}:{password}`.
bmc_onboard called GetSystem with the FRESHLY ROTATED credential, so the new
password entered argv at the moment the old one was retired. This was a
half-finished replacement migration rather than a new defect: every mutating
operation already used --netrc-file and a GetSystemWithNetrc sat directly beside
the leaking GetSystem answering the identical question. The correct shape was
present and understood; what kept the leak alive is that the old form was left
standing next to the new one, and five call sites duly reached for it. So the
cut is at the root -- the three argv-borne arms are deleted, and
GetSystemWithNetrc goes too, because once GetSystem takes no password the
`WithNetrc` suffix distinguishes nothing. `Secret` no longer appears anywhere in
that module, which is the receipt.
Moving every BMC call onto the netrc made that file's MODE load-bearing, so
write_netrc now uses WriteOwnerOnly -- 0600 at creation via O_EXCL, not
write-then-chmod -- with a shred first, since O_EXCL refuses an existing path and
this tool legitimately rewrites the netrc within one run. Otherwise the repair
would have moved the exposure from the process table to the filesystem, where
the audience is identical.
gunbc.ci_spec built `curl -H "Authorization: Bearer $WIF_ACCESS_TOKEN"` in two
runner preludes. The shell expands before exec, so the GCP access token was a
curl argv word on exactly the hosts measured above. The header is now written by
the `printf` BUILTIN -- which forks no process and so creates no second command
line to leak into -- and read back with `-H @FILE`. Executed both ways with a
planted canary: present in curl's argv under the old form, absent under the new,
so the detector is discriminating rather than merely quiet.
The third instance is DECLARED AND NOT REPAIRED, named in the new carrier rather
than left for rediscovery. nbd_proxy_serve concatenates a live BMCWEB session
token into a `--header=` argv word, and because that argv is launched via
systemd-run the value also reaches the unit's ExecStart and the journal. Its
repair is not the same move as the other two -- websocat has no @file header form
and systemd-run performs no shell expansion, so it needs an EnvironmentFile plus
an sh -c wrapper, on the srv3 OS-install path, and two witnesses that currently
ASSERT the token's presence in argv would have to be inverted with it. A probe
written to prove the value was materialized rather than left a dead shell
reference has, as a side effect, frozen the argv placement as a contract.
bmc_auth_runtime_secret_seam_test is deleted, and not as collateral: it asserts
that a Secret survives a locally-defined IDENTITY function. It has no red arm and
never had one, so it proved nothing about the transport, and with basic auth gone
its name would leave a reader grepping for coverage that does not exist.
RUNG, stated at the honest grain: mitigatable, not mechanically preventable.
Nothing refuses the next author who interpolates a credential into a command
string, and the reason is structural rather than an oversight --
v2.std.operation_argv already refuses undeclared names and a non-literal argv[0],
but ShellTransportOperationRow carries declared_inputs as NAMES WITHOUT TYPES, so
a Secret-typed input bound into an argv token is invisible to the binder. That
missing type on the row is the next-rung trigger, and it is the difference
between cannot-climb and can-climb-after-one-grounding.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…er than the committed file The repair in the previous commit is two call-site edits, and a call-site edit is exactly the kind of thing a later author undoes by copying a neighbouring curl invocation. This enrolls the property instead of trusting the diff. TWO ASSERTIONS, AND EITHER ALONE WOULD BE A DECORATION. The absence of the leaking spelling is the safety property; the presence of the file-backed one is what makes that absence mean the credential still ARRIVES rather than that the step quietly stopped authenticating. A prelude that had simply dropped the header satisfies an absence-only check perfectly -- an argv that is clean because it is broken. THE SUBJECT IS THE EMITTED SHELL, NOT THE COMMITTED WORKFLOW FILE, deliberately. The committed yml is a projection of these functions; asserting against it would test whether somebody remembered to regenerate, while asserting against the emission tests the authority that decides what any future regeneration produces. The third test is the one most likely to be lost in a later edit because it looks like a formatting preference. It is not. `printf` runs inside the shell and forks nothing, so the token exists only in the shell's memory and in the file; any EXTERNAL writer of the same bytes -- echo as a binary, a here-doc through tee, a python one-liner -- would leave the value in ITS OWN argv, relocating the exposure rather than removing it. Someone swapping printf for tee has not made a cosmetic change, and this test is what says so. One thing checked rather than assumed, because it would have made the whole witness non-discriminating: the emitted printf line contains the text "Authorization: Bearer" and the text "$WIF_ACCESS_TOKEN", so a naive forbidden pattern could have matched the REPAIR and gone red on correct code -- or, worse, been weakened until it matched nothing. It does not match: "%s\n' \"" sits between the two halves in the emitted line, while the old spelling was contiguous. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
Two updates since this was openedThe regen gap is upstream of this change, and I measured it rather than asserting itI said above that the emitted
The unique error sets are identical, and neither run writes anything. So the drift is real, but the sanctioned regeneration route produces no output on I did not hand-edit the generated file. Editing a generated artifact to match its authority is the laundering the drift gate exists to prevent, and it would have made the yml agree with the model for a reason unrelated to the model. Worth surfacing on its own: the documented regeneration recipe is currently non-functional on A correction to a measurement I reported earlier in this laneMy first version of the argv control returned 2 and 2 — both arms "leaking" — and I initially read that as the repair not working. It was the detector: Recording it because it is the same failure this PR is about, one level up: a value in a command line was read by something that had no business reading it — in that case, my own grep. |
…is scoped rather than started
TWO CORRECTIONS AND ONE SCOPING, none of which change the repairs already
landed.
FIRST, THE DECLARED-NOT-REPAIRED INSTANCE IS WORSE THAN THIS CARRIER SAID.
It recorded the BMCWEB session token as reaching the transient unit's ExecStart
and the journal "in addition to the process table", which undercounted by
reading only the websocat argv. extdeps.systemd.systemd_run RunTransient's own
argv is `systemd-run --unit={unit} --collect -- {command_argv}`, so the token
sits in THREE positions: systemd-run's own argv while it runs, websocat's argv
for the whole lifetime of the served ISO, and systemd's recorded ExecStart. The
middle one is the long-lived one and is the identical world-readable cmdline
surface as the incident that opened this carrier -- so the instance I deferred is
not merely a journal problem with a process-table footnote, it is the same
primary defect plus two further surfaces. The disposition does not change; the
severity statement did, and an understated severity on the one instance left open
is the wrong direction to be wrong in.
SECOND, THE QUERY THAT WOULD CONFIRM THE JOURNAL HALF IS RECORDED WITH ITS
CONTROL, because the obvious sweep does not terminate -- srv1's journal is ~3.4M
lines per 24h and a regex alternation over all of it exceeded a measurement
budget without returning. The unit name is derivable rather than guessable, so
the query is one glob and a fixed string. The control matters more than the
query: if no such unit has ever run on the host, a zero match means the path is
UNEXERCISED, not clean, and the leak is latent rather than absent. A zero against
no history distinguishes nothing -- which is the same non-result this lane has now
produced twice, once on my own curl measurement and once on a container-runtime
sweep whose control was also zero.
THIRD, THE CONSTRUCTION ITEM IS SCOPED AND NOT STARTED, on direction. What
landed is two repairs and a declaration; the class is held by review diligence,
which is exactly what failed for however long three instances stood. The missing
fact is specific: ShellTransportOperationRow carries declared_inputs as NAMES,
not types, so the refusal a binder would need to raise is not merely unwritten
but unexpressible against the row it would read. With the type present, Secret in
an argv token becomes a typed cause and instance 1 stops being a thing an author
rewrites.
The scope note also says what that trigger does NOT reach, so nobody picking it
up overclaims: ci_spec's hand-authored shell is not declared operation argv, so
no binder change touches it under any version. That half stays at diligence plus
the emission witness until shell command construction is itself modeled.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…he intuitive order is backwards A reader deciding what to pick up next reads the ranking line, and the natural assumption -- the two REPAIRED instances were the serious ones, the deferred one was the leftover -- is exactly wrong. Instances 1 and 2 were argv-only on SHORT-LIVED processes: a curl fetching one secret, a curl authenticating one Redfish call. Both closed. Instance 3 is argv PLUS A DURABLE RECORD, three surfaces with three lifetimes -- systemd-run's transient argv, websocat's argv FOR THE LIFETIME OF THE SERVED ISO, and the recorded ExecStart which persists in the journal and `systemctl show` after the process exits. The middle surface is the same one the incident was originally reported against, held for as long as an install is served. The third outlives the process, so revoking the session token stops the credential working without removing it from the record of what ran. This does not reverse the decision to declare rather than repair -- inverting two witnesses that ASSERT the token is in the argv still belongs with the repair, and doing it inside a security change on the srv3 install path is how a leak fix becomes an outage. It does mean the open instance outranks the binder trigger when someone picks up next work, and leaving that to be reconstructed from three separate paragraphs was the wrong way to record it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
Correction: the instance left open is the most severe of the threeI understated instance 3 in the original description, and the understatement pointed the wrong way — so it is corrected here and in the carrier rather than left as a detail. I described the BMCWEB session token as reaching the transient unit's
The ranking is the opposite of the intuitive one. Instances 1 and 2 were argv-only on short-lived processes — a curl fetching one secret, a curl authenticating one Redfish call — and they are closed. Instance 3 is argv plus a durable record. Its middle surface is the same one the incident was originally reported against, held open for as long as an install is being served. Its third surface outlives the process entirely, so revoking the session token stops the credential working without removing it from the record of what ran. So the instance still open outranks the binder trigger scoped below it. A reader choosing next work should not have to reconstruct that from three separate paragraphs. This does not reverse the decision to declare rather than repair. Inverting two witnesses that assert the token is in the argv belongs with the repair, and doing that inside a security change on the srv3 install path is how a leak fix becomes an outage. Scoped, not started: the construction answerOn direction, the next item is named in the carrier and deliberately not begun here.
Stated so nobody picking it up overclaims: that trigger does not reach the The journal half is unmeasured, and that is not a zeroThe bounded query exists and has not been run — the attempt failed on an expired GCP token, so it is an unreached measurement, not a clean result. Recorded in the carrier with the rule that matters: if no |
…he detector that nearly said otherwise Q3 ran on all four fleet hosts against the derivable unit glob `nbd-proxy-*-ws`: units=0, journal_lines=0, grep_journal=0, execstart_now=0, live websocat=0, everywhere. BOTH CONTROLS ARE ZERO, so the greps distinguish nothing. No such unit has ever existed and the journal holds no lines for that selection, which means the srv3 OS-install route has not run on this fleet. The verdict is UNEXERCISED PATH: the leak is LATENT, not absent. The code path is real and exposes the token on all three surfaces the moment it runs, and recording a zero-against-no-history as safety would be the same non-result this lane has produced repeatedly. THE RANKING NOW CARRIES TWO AXES, because collapsing them mis-ranks this in either direction. Instance 3 is worst by SURFACE COUNT AND LIFETIME and is the only one NOT CURRENTLY FIRING; instances 1 and 2 were LIVE and are closed. Severity alone over-ranks it against work leaking today; liveness alone under-ranks a three-surface exposure that fires the moment the install path runs. The latency also STRENGTHENS the declare-rather-than-repair call rather than merely permitting it: nothing is leaking right now, so no emergency forces a rushed change to the srv3 install path, and the repair can land properly with the two token-asserting witnesses inverted alongside it. AND THE DETECTOR RULE IS ENCODED, because the first pass of this very measurement reported live token-bearing processes on all four hosts and it was FALSE: every hit was `grep` matching its own command line, since the search pattern is itself an argv word that ps can see. Excluding the searcher returns zero. That is the third instance of this artifact in one day -- it also inflated an unrelated orphan census, and it made this module's own curl repair read as a non-repair until the probe was narrowed to the subject's own cmdline. None of the three was caught by reading. So: any ps- or /proc-based detector must exclude its own process, and the receipt for a ZERO must be a PLANTED POSITIVE rather than an absence of hits. That is not hygiene advice sitting in a security carrier by coincidence -- it is this carrier's own defect turned on the instrument: a value in a command line read by something with no business reading it, which here was the measurement itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
The journal half is now measured — and the answer is unexercised, not cleanRun on all four fleet hosts against the derivable unit glob
Both controls are zero, so the greps distinguish nothing. No such unit has ever existed and the journal holds no lines for that selection — the srv3 OS-install route has not run on this fleet. The leak is latent, not absent: the path is real and exposes the token on all three surfaces the moment it runs. A zero measured against no history is not evidence of safety. The ranking now carries two axesCollapsing them mis-ranks instance 3 in either direction:
Severity alone over-ranks it against work leaking today; liveness alone under-ranks a three-surface exposure that fires the moment the install path runs. The latency also strengthens the declare-rather-than-repair call rather than merely permitting it: nothing is leaking right now, so no emergency forces a rushed change to the srv3 install path, and the repair can land properly with the two token-asserting witnesses inverted alongside it. A detector rule, encoded because it nearly inverted this resultThe first pass of this exact measurement reported live token-bearing processes on all four hosts. It was false. Every hit was That is the third instance of this artifact in one day: it also inflated an unrelated orphan census (35 → 48), and it made this PR's own curl repair read as a non-repair until the probe was narrowed to the subject process's own So, now written into the carrier:
That rule is not hygiene advice sitting in a security carrier by coincidence — it is this carrier's own defect turned on the instrument: a value in a command line read by something with no business reading it, which here was the measurement itself. — sent from swift-ant-195 |
Contributor
Author
|
Re: review 55857 (REQUEST_CHANGES) — I checked this against the module rather than against the diff, and I'm declining the requested marker. The gate is real; it governs a different construction. Evidence below, and a residual I think the review is right about. The gate exists and I found its two instances
Both mark The prelude is on the far side of that migration already
— a typed Consistent with that, the prelude carries zero scaffold markers across its ~20 The lines cited as "further shell fragments at 772–780" are that prelude: Why adding the marker would be wrong, not merely unnecessaryMarking one Worth noting the direction: my function is a bare string literal with no Where the review is right, and what I changedTwo things I accept:
One constraint that decides why no modeled actuator was available: these preludes execute as raw shell in a foreign executor (GitHub Actions I have added a note at the function recording that boundary, so the next reader sees why this payload is shell and what would dissolve it — without claiming a scaffold verdict nobody granted. — sent from swift-ant-195 |
…it as debt it is not review 55857 asked for a bash-emit scaffold marker on the header-write command. Declining the marker and recording the reasoning where the next reader will hit it, because the marker would assert something false. The gate is real and this module has two instances of it -- but both mark `*_script` functions that CONCAT-BUILD a whole script as a String, and their own dissolution text names that shape: "medium-as-string concat scaffold", dissolving when bash-emit realizes the runner "through orchestration emit ... without a medium-as-string concat". gunbc_ci_fleet_key_agent_script does not concat-build. It emits a typed Pipeline of PipelineStep through v2.workflow.gunbc_invoke_step_emit -- which IS the terminal state those triggers name -- and gunbc.fleet_ssh_credential_verify's supersession note records that migration having already happened. The prelude carries zero scaffold markers across ~20 command steps for that reason. So marking one payload while its twenty peers carry none would claim this line is unmigrated debt when it sits in the migrated construction: a false rung on the carrier, and a scaffold verdict written by its own author to clear a review, which DESIGN forbids in the same breath as it says a dissolution condition does not authorize the debt it describes. What the review is right about is kept rather than argued away: a Run's `command` IS still shell text. That is corpus-wide and tracked by bash-emit, not by a per-line marker. And the reason no modeled actuator was available here is a BOUNDARY, not a preference -- this prelude runs as raw shell in a foreign executor before gunbc runs at all, so Filesystem.WriteOwnerOnly, which this same change uses for the netrc on the BMC path, is unreachable at that point in a job. printf is not a modeled route declined; it is the absence of one on that side of the boundary, which is what bash-emit would change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…es in prose Both are mine, both are the compiler correctly refusing, and both are error classes DESIGN names explicitly. Recording what they were rather than just fixing them, because I walked into each with the relevant rule already written down. FIRST, §4c. I put explanatory `//` blocks INSIDE func bodies in bmc_first_contact and bmc_onboard -- thirteen of the run's errors. §4c admits only standalone leading blocks attached to module-scope declarations and says trailing, body and unattached forms refuse; I wrote body forms. The content was worth keeping, so each block is hoisted above the declaration it describes and rewritten to read at that grain -- a note above `func bmc_probe_credential_phase` has to speak about the whole probe rather than about the statement it used to sit on, and the post-rotation one now names the func's own subject. This is the exact failure mode DESIGN records against #8976: an illegal annotation inside a match body made the compiler refuse at strict preparation while everything downstream kept producing valid-looking output. Here it refused before any witness executed, which is why the floor reported `modules_resolved` and then nothing about my witness at all. SECOND, brace interpolation in prose. I quoted two argv TEMPLATES inside `data` strings -- `-u {username}:{password}` and `systemd-run --unit={unit} --collect -- {command_argv}` -- and the compiler read the braces as interpolation and refused on four undefined variables. The escape is `\{`, already used in this repo for `%\{http_code}`. The irony is not lost: this carrier is about a value being interpolated into a string by a mechanism that had no business interpolating it, and its own prose was interpolated by a mechanism that had no business interpolating it either. I swept both classes across every file this branch touches rather than fixing only the lines the run named -- no indented `//` remains, and the only unescaped braces left are the argv templates in extdeps.bmc.http where interpolation is the point. WHAT I COULD NOT DO: validate this locally. The system gunbc predates annotation support and errors on line 6 of any file carrying a `//` block, including unmodified std/algebra.dag, std/decl_ref.dag and std/types.dag -- an older compiler against a newer corpus, so it cannot answer the question. The static sweep above is what I have; CI is the executor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gunbai-bot Bot
pushed a commit
that referenced
this pull request
Aug 26, 2026
…gen surfaces a security fix that never reached its workflow Review 55934 is correct and the finding is the one that matters most in a deletion: the code changed and the canonical authorities did not, so DESIGN and four carriers were left asserting that `--required-cited-symbol` is a live flag that runs the census standalone. After this PR that sentence returns `unknown argument` and exits 2. A knowingly-false recital in the canonical authority is premise contamination -- a reader planning against it budgets for an invocation that cannot run -- and it is exactly the drift these documents were rewritten to close on 2026-08-25. CORRECTED, all six, in the same PR as the deletion rather than after it: dag/gunbc/design_document.dag (the rung-drop row, and DESIGN.md regenerated) src/v2/lens/cited_symbol_resolution.dag (two rows) dag/gunbc/doc_graph_roots.dag dag/gunbc/fabric_capacity_class_gap.dag (past tense) dag/test/claim/build_cache_endpoint_observe_test.dag (past tense) THE RUNG DOES NOT DROP AGAIN, and every correction says so rather than quietly implying a second drop. The clause being replaced already argued that a mode no workflow invokes guards nothing; removing that mode therefore takes away no enforcement, and the class sat at mitigatable on review diligence before the deletion and sits there after it. What the deletion removes is the ON-DEMAND RE-DERIVATION ROUTE -- with the flag and its Rust census both gone there is now no way to run this census over the live corpus at all. That is a capability loss, not a rung change, and conflating the two would be the same 4b(1) inflation the original clause was written to avoid, run in reverse. THE REGEN SURFACED SOMETHING ELSE, AND IT IS NOT MINE. Regenerating the committed artifacts from their authorities also rewrote .github/workflows/fleet-converge.yml. That change belongs to #9226 -- "A credential in a command line is read by every process on the host, and on this fleet that is measured" -- which moved the Secret Manager bearer token out of the curl argv and into a header file, landed a witness asserting the header-file form, and NEVER REGENERATED THE WORKFLOW. So the model has said `-H @"$HDR_FILE"` since that merge while the emitted workflow that actually runs has kept `-H "Authorization: Bearer $WIF_ACCESS_TOKEN"` on the command line, readable from /proc by every process on the runner. The witness passes because it asserts against the authority, not against the artifact. It is included rather than reverted because the alternative is worse: running a generator and committing only the part of its output that suits this PR is cherry-picking, and it would leave a security repair inert while looking applied. The generated-artifact drift gate that would have caught this is in the set DESIGN's CI entry lists as UNGUARDED since the floor cut -- so nothing was going to catch it, and it was found only because this PR had an unrelated reason to regenerate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
briansrls
pushed a commit
that referenced
this pull request
Aug 27, 2026
… the test target main inherited (#9230) * Enrol the v2-emission selftest; delete two orphaned modes; repair the test target main inherited Rebuilt on current main rather than merged into it. The old branch shared three commits with #9228 and conflicted in twelve hunks where "ours" was a pre-deletion snapshot -- hand-resolving that risks silently restoring code #9228 had just deleted, so the two intentional changes were replayed onto main instead. Force-push is the honest vehicle here; a merge commit would have carried a resolution nobody could audit. THE ENROLMENT IS THE POINT, and it is a climb rather than a cut. run_required_v2_emission_selftest -- the red fixture that must be refused on the annotation cause and the green fixture that must emit -- was reachable ONLY through a standalone flag no workflow invoked. So the required run's v2-emission phase ran a producer whose REFUSAL had never once been shown to fire; a green established that the emitter emitted, not that it still refuses what it must. DESIGN 4b(4): a class's discriminating RED and positive control stay ENROLLED as the evidence the rung is real. Unenrolled evidence is not weaker evidence, it is none. It now runs inside the phase, ahead of the producer, and a failure lands in phase_failures like any other. Measured before enrolling: passes, about a second. The standalone flag goes in the same motion -- two routes to one fact, and only the enrolled one executes. TWO MODES DELETED, and only two: - --measure-cgroup-peak: its own comment names the `rust_tests` job as its caller. That job was deleted. An orphan whose stated consumer is gone. - --required-cited-symbol: DESIGN's 2026-08-23 row already declares this drop and states the surviving flag guards nothing, with a restoration trigger putting the wall at ingestion. Deleting the flag agrees with that row rather than leaving a mode that reads as coverage. KEPT AGAINST THIS PROGRAM'S OWN BIAS: --heads-reading-differential and the three --behavioral-receipt-* modes are INSTRUMENTS with entry points, not gates. DESIGN's 2026-08-24 ruling is that a measurement worth re-deriving is worth an entry point. The target-model lane confirms both shapes they need are expressible (addressable but absent from the //:required roster; a differential needs its own TestStanding arm rather than the Bazel status arm, which carries only PASSED/FAILED and would erase the divergence). They migrate; they do not get rescued. ALSO REPAIRS WHAT MAIN INHERITED. #9228 merged before its last commit landed, so main carries four errors in claim_executor's TEST target: an orphaned witness_walk_flags_tests module and a cfg(test) classify_witness_expectations helper over three deleted types, plus seven dead test helpers and the imports the cited-symbol deletion orphaned. CI cannot see these -- the required run compiles --bin, never --tests. EVIDENCE, both controls run rather than one: - transport: ctrl-build reports `forwarding env: RUSTFLAGS`, so -D warnings reached rustc. - subject: a planted `super::deliberately_absent_symbol_zzz()` produces its exact diagnostic under this branch's own --tests invocation, proving test modules entered the observed population. Clean is therefore a measurement, not a silence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Six authorities asserted a flag this PR deletes; correct them, and regen surfaces a security fix that never reached its workflow Review 55934 is correct and the finding is the one that matters most in a deletion: the code changed and the canonical authorities did not, so DESIGN and four carriers were left asserting that `--required-cited-symbol` is a live flag that runs the census standalone. After this PR that sentence returns `unknown argument` and exits 2. A knowingly-false recital in the canonical authority is premise contamination -- a reader planning against it budgets for an invocation that cannot run -- and it is exactly the drift these documents were rewritten to close on 2026-08-25. CORRECTED, all six, in the same PR as the deletion rather than after it: dag/gunbc/design_document.dag (the rung-drop row, and DESIGN.md regenerated) src/v2/lens/cited_symbol_resolution.dag (two rows) dag/gunbc/doc_graph_roots.dag dag/gunbc/fabric_capacity_class_gap.dag (past tense) dag/test/claim/build_cache_endpoint_observe_test.dag (past tense) THE RUNG DOES NOT DROP AGAIN, and every correction says so rather than quietly implying a second drop. The clause being replaced already argued that a mode no workflow invokes guards nothing; removing that mode therefore takes away no enforcement, and the class sat at mitigatable on review diligence before the deletion and sits there after it. What the deletion removes is the ON-DEMAND RE-DERIVATION ROUTE -- with the flag and its Rust census both gone there is now no way to run this census over the live corpus at all. That is a capability loss, not a rung change, and conflating the two would be the same 4b(1) inflation the original clause was written to avoid, run in reverse. THE REGEN SURFACED SOMETHING ELSE, AND IT IS NOT MINE. Regenerating the committed artifacts from their authorities also rewrote .github/workflows/fleet-converge.yml. That change belongs to #9226 -- "A credential in a command line is read by every process on the host, and on this fleet that is measured" -- which moved the Secret Manager bearer token out of the curl argv and into a header file, landed a witness asserting the header-file form, and NEVER REGENERATED THE WORKFLOW. So the model has said `-H @"$HDR_FILE"` since that merge while the emitted workflow that actually runs has kept `-H "Authorization: Bearer $WIF_ACCESS_TOKEN"` on the command line, readable from /proc by every process on the runner. The witness passes because it asserts against the authority, not against the artifact. It is included rather than reverted because the alternative is worse: running a generator and committing only the part of its output that suits this PR is cherry-picking, and it would leave a security repair inert while looking applied. The generated-artifact drift gate that would have caught this is in the set DESIGN's CI entry lists as UNGUARDED since the floor cut -- so nothing was going to catch it, and it was found only because this PR had an unrelated reason to regenerate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * The canonical authority cited a real module as the home of a symbol it does not define DESIGN's floor clause read `v2.workflow.required_floor` `run_required_floor`. The module is real; the symbol is not in it. `run_required_floor` is defined in `src/v1/stage0/src/cli_run.rs` and every other citation in the corpus names it as `v1_compiler.cli_run` `run_required_floor` -- this was the only occurrence of the wrong form, and it was in the one document every session reads first. The v2 module is not unrelated, which is why the repair names both rather than swapping one for the other: it owns the admission, cost-line and preparation-safety policy the fold consults. What it does not own is the fold. Citing the policy carrier as the driver is the same authority-substitution shape DESIGN already records -- both halves check out and only the arrow between them is invented. Worth stating plainly because it lands in a PR that deletes the census: this is precisely the defect `--required-cited-symbol` refused, and it was found by hand three days after that job was cut. The rung-drop row I corrected earlier in this PR predicted an unbounded future population; this is the first counted member of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Four authority rows asserted the gap this PR's own merge had already closed review 56048 (REQUEST_CHANGES) found three changed rows still saying cited-symbol checking is review diligence and cannot be re-derived, while DESIGN.md in the same tree records the rung restored. The finding is correct and the cause is mine: those edits were true when written, and taking main's retirement row in the merge made them false without touching them. A PR that contradicts itself across four files is the premise contamination this branch has spent the night correcting elsewhere. Corrected to describe the replacement rather than the drop: v1_compiler.declaration_index resolves every authored DeclarationRef inside the parse phase of --required-ci, at ingestion from the module's own source, strictly wider than what was dropped. The fourth row the review did not name is the worst of them and is fixed here too. fabric_capacity_class_gap asserted, as a heading and in the present tense, THE CITATIONS IN THIS MODULE ARE NOT CHECKED BY ANYTHING -- measured, correctly, against a census that enrolled five hand-named carriers. The declaration index enrolls by walking the Node tree for DeclarationRef literals, so a module cannot be outside it by omission, and those eight citations are now enrolled. The measurement is kept rather than deleted because it was taken rather than assumed; what changes is that it now reads as the account of a closed gap. Left standing, it would have told a future reader that a checked module was unchecked -- worse than the three flagged rows, which merely understated a rung. build_cache_endpoint_observe_test is deliberately NOT changed: its subject is annotation grain enforced in the floor's preparation, which is unaffected by which census runs beside it, and it already says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Moving the bearer token out of argv and leaving it on disk is the same exposure at a different address review 56110 (REQUEST_CHANGES) is correct and this is my defect. The fleet prelude DISARMS its EXIT trap so the ssh-agent outlives the step, and it did so after removing only KEY_FILE -- so the token I moved out of argv in this same PR sat in RUNNER_TEMP for the remainder of the job. The security repair was half a repair. FIXED BY LIFETIME, NOT BY CLEANUP. HDR_FILE is consumed by exactly one curl, so it is removed on the line after that curl rather than added to the later rm the review suggested. The credential ceases to exist when its only consumer is done with it, which is a shorter window than any exit path can offer and does not depend on which trap is armed when the step ends. The trap still covers the failure arm between creation and that removal, which is the only interval where the file can genuinely outlive the shell. The spark prelude is NOT affected and is not changed: it keeps its trap armed through step exit, so its header file is cleaned there. Only the fleet prelude disarms. The authority annotation asserted the thing that was false -- "Each prelude's existing trap removes HDR_FILE on exit" -- which is how the defect survived authoring and review of the original change. Corrected in place, naming which prelude it holds for and which it does not. WITNESS, PROVEN BOTH WAYS BY EXECUTION. The existing tests only ask HOW the header reaches curl, so they stayed green through the entire defect. The new one asserts ADJACENCY -- the removal on the line immediately after the consuming curl -- rather than presence, because presence stays green if the removal drifts below `trap - EXIT`, which is the defect itself. Green: exit 0. Red: delete the rm row from the authority and it exits 1 with "adjacency absent". Authority restored and verified byte-identical after. A position comparison would have been the more obvious spelling and I did not use it: this corpus has no index primitive, and minting one so a test can measure a shell string would be adding an authority to check a claim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * A comment cited a deleted flag as the precedent for keeping another mode's ordering review 56142 (APPROVE, non-blocking). The ordering rationale for --emit-partition-crates named --verify-build-artifacts AND --measure-cgroup-peak as its two co-examples; this PR deletes the second, so the sentence justified a real ordering by pointing at a mode that no longer exists. Small, and worth doing rather than deferring for the reason this branch has now hit four times: a stale citation does not announce itself, and the next reader takes the comment as the account of why the ordering is safe. The ordering IS safe and the remaining co-example carries the argument alone. Swept the corpus for the three deleted flags rather than fixing only the cited line. The only surviving matches are run_required_v2_emission_selftest's definition in cli_run and its call from the enrolled V2Emission phase -- that is the function this PR puts on the required path, not the flag it removes, so both are correct and stay. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Rehome the orphaned dichotomy-over-three-states paragraph into its authority before a regeneration deletes it DESIGN.md on main carried 1733 characters that `gunbc.recurring_failure_mode` did not: the second form of state-space conflation, its gunbc#9324 specimen, and its recognition rule. Measured by running the generated-artifact gate on a bare worktree at main 0aa09c1 -- `M DESIGN.md`, one line, one insertion one deletion -- and by grep: the text appears nowhere under dag/ or src/v2/. So the projection disagreed with its source in the document that defines the rule against exactly that, and the failure mode was not the disagreement but its remedy: ANY regeneration silently deletes prose no authority holds. This PR already regenerates DESIGN.md as part of resolving a merge-driver refusal, so it was itself the change that would have destroyed it -- caught only because a peer lane reported the drift while resolving a different PR. The repair is the one DESIGN section 3 prescribes: a fact's home is its layer, so the paragraph moves into the carrier that owns the recurring-failure-mode roster and the projection derives it. Verified by execution: the regenerated failure-mode line is now byte-identical to main's, and a second gate run is a no-op, so the tree is a fixed point of its own emitter rather than merely agreeing once. Nothing here is authored: the text is main's, moved to where it can survive. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: gunbc-ci-auto-heal <gunbc-ci-auto-heal@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
briansrls
pushed a commit
that referenced
this pull request
Aug 27, 2026
…at no longer exists (#9291) `operation_argv_previously_unbindable_operation_now_materializes` asserted `redfish.Http.GetSystem` materializes as `curl ... -u operator:secret https://srv4-bmc/redfish/v1/Systems/system`. That argv was DELETED by #9226, which cut the three password-in-argv arms of `extdeps.bmc.http` at the root and moved GetSystem onto `--netrc-file`. The witness kept asserting the deleted form, so it went red against the tree it reads -- a stale golden, not a defect in the binder. The repair points the witness at the operation as it is declared today: GetSystem binds `bmc_host` and `netrc_file`, and the expected argv is the netrc form. The fact the witness carries is unchanged -- an operation the seed's previous fixed-list materializer could not bind at all now materializes from its own declaration -- and it is still measured against a real corpus declaration rather than a fixture. One witness is ADDED, and it is the regression control #9226 did not leave behind: binding `password` on GetSystem is now refused as `UndeclaredInputBound:password`, located at `redfish.Http.GetSystem`. #9226's own module note says the class sits at mitigatable because `ShellTransportOperationRow` carries declared inputs as names without types, so nothing refuses a NEW `password: Secret` argv token. That remains true. What this control does hold is the cut itself: if the deleted arm is restored under its old name, the refusal disappears and the witness goes red -- so the credential cannot come back to THIS operation silently. Co-authored-by: gunbc-ci-auto-heal <gunbc-ci-auto-heal@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
An incident lane, not hygiene. The reported observation was live
GITHUB_TOKENandBUILDBUDDY_API_KEYvalues visible inpsargv on the fleet hosts.The root cause of the reported instance is not in this repository and this PR does not pretend to have fixed it — see Out of scope below. What this PR does is ask the same question of our own tree, and it had three answers.
Why this class is severe here specifically
Three facts measured on srv1, 2026-08-25:
/procis mounted with nohidepid/proc/PID/cmdlineis mode 0444So an argv word is not readable merely by processes of the same user. It is readable by any job payload executing on a runner slot, in a repository whose workflows a pull request can influence. The usual reason argv exposure gets deprioritized is an implicit assumption that the reader must already be privileged. Here the reader is unprivileged and arrives through the front door.
Instance 1 — BMC basic auth (repaired)
extdeps.bmc.httpdeclaredpassword: SecretonGetSystem,GetPowerandGetThermal, then interpolated it into the argv token-u {username}:{password}.bmc_onboardcalledGetSystemwith the freshly rotated credential, so the new password entered argv at the moment the old one was retired.This was a half-finished replacement migration, not a new defect. Every mutating operation in that module already used
--netrc-file, and aGetSystemWithNetrcsat directly beside the leakingGetSystemanswering the identical question. The correct shape was already present and already understood — what kept the leak alive is that the old form was left standing beside the new one, and five call sites duly reached for it.So the cut is at the root: the three argv-borne arms are deleted, not deprecated, and
GetSystemWithNetrcis deleted too — onceGetSystemtakes no password, theWithNetrcsuffix distinguishes nothing and is a second name for one concept.Secretno longer appears anywhere in that module, which is the receipt.Moving every BMC call onto the netrc made that file's mode load-bearing, so
write_netrcnow usesWriteOwnerOnly(0600 at creation viaO_EXCL, not write-then-chmod), preceded by a shred sinceO_EXCLrefuses an existing path and this tool legitimately rewrites the netrc within one run. Without that, the repair would have moved the exposure from the process table to the filesystem — where, on these hosts, the audience is identical.Instance 2 — Secret Manager bearer token on the runners (repaired)
gunbc.ci_specbuiltcurl -H "Authorization: Bearer $WIF_ACCESS_TOKEN"in two runner preludes. The shell expands beforeexec, so the GCP access token was a curl argv word on exactly the hosts measured above.The header is now written by the
printfbuiltin — which forks no process, and so creates no second command line for the value to leak into — and read back with-H @FILE. Both preludes already runumask 077, so the redirect creates the file 0600 at creation; each prelude's existing trap removes it.Discriminating control, executed:
-H "Authorization: Bearer $TOK"-H @"$HDR_FILE"The red arm is what makes the green arm mean something: the detector demonstrably finds a planted canary. (An earlier version of this measurement reported 2/2 — it was matching my own harness's command line rather than curl's, and was corrected by narrowing to the subject process's
/proc/PID/cmdline.)Instance 3 — BMCWEB session token (declared, NOT repaired)
Named here rather than left for rediscovery, because a partial repair that is silent about its remainder is how a class gets marked closed while an instance survives.
nbd_proxy_serve_websocat_argvconcatenates a live BMCWEB session token into a--header=argv word, and that argv is launched viasystemd-run --unit, so the value also reaches the transient unit's recordedExecStartand the journal — a wider surface than the other two.Its repair is not the same move: websocat has no
@fileheader form andsystemd-runperforms no shell expansion (which is exactly why an earlier change replaced a$BMCWEB_SESSION_TOKENreference with the materialized value — it was right about the mechanism). It needs anEnvironmentFileat 0600 plus ansh -cwrapper, on the srv3 OS-install path, and two witnesses that currently assert the token IS in the argv would have to be inverted with it. A probe written to prove the value was materialized rather than left a dead shell reference has, as a side effect, frozen the argv placement as a contract.Rushing that inside a security change risks breaking an install path in order to close a leak that is already declared. It is recorded in the new carrier with its mechanism and repair shape.
Deleted witness
bmc_auth_runtime_secret_seam_testasserts that aSecretsurvives a locally-defined identity function. It has no red arm and never had one, so it proved nothing about the transport; with basic auth gone, its name would leave a reader grepping for coverage that does not exist.Rung, at the honest grain
Mitigatable — not mechanically preventable. Nothing refuses the next author who interpolates a credential into a command string, and the reason is structural rather than an oversight:
v2.std.operation_argvalready refuses undeclared names and a non-literalargv[0], butShellTransportOperationRowcarriesdeclared_inputsas names without types, so aSecret-typed input bound into an argv token is invisible to the binder and could not be refused. The hand-authored shell inci_specis further out still — it is not declared operation argv at all.Next-rung trigger: carry the declared input's type on that row, then refuse Secret-into-argv-token as a typed cause beside
UndeclaredInputBound, at which point instance 1 becomes unwritable rather than merely rewritten.Out of scope — reported, not edited around
The reported observation comes from the session container entrypoint (session-dashboard infrastructure), which forwards a credential roster into
tmux new-session -e NAME=VALUE; those become the argv of the long-lived tmux server. No checkout of that source exists in this repository, so it cannot be repaired from here.The repair shape was established by execution and is recorded in the new carrier so the owning repo need not re-derive it: export in the parent shell and drop
-eentirely — the tmux server captures its own environment as the global environment and panes inherit it. Canary present in the server argv under-e, absent under export, pane reading the correct value in both.tmux setenvis not the fix; it relocates the value from a server argv to a client argv.Routing that change, and rotating the static BuildBuddy key, are with the operator.
Known gap in this PR
The emitted
.github/workflows/fleet-converge.ymlis not yet regenerated against theci_specchange. The whole-corpus emit did not complete in-session; it resolves 721 sources with no errors and then runs long. I did not hand-edit the generated artifact, since editing a generated file to match its authority is precisely the laundering the drift gate exists to prevent. The regen phase will refuse until it is regenerated, which is correct fail-closed behaviour — I will push the regenerated artifact as a follow-up commit on this branch.