Repository navigation
BMC redfish fails closed on HTTP status + argv-anemia dissolution plan + srv4 sccache access - #7138
Merged
Merged
Conversation
…execution vs srv4) Every redfish.Http.* op derived `success: Bool from "exit_success"` but ran `curl -sS -k` WITHOUT --fail — and curl exits 0 on 401/403/404/5xx. So `success` read any non-2xx as TRUE: a §5 fail-open across the whole BMC transport. Found live against srv4: bmc_probe_credential_phase reported FactoryCredentialActive for an already-rotated BMC, because the factory-login GetSystem returned HTTP 401 (0-byte body) and curl's exit 0 made success=true. Left unfixed, the full converge would take the FactoryCredentialActive -> mint+store+rotate arm against a healthy host; and SetAccountPassword.success is exit-based too, so a rotation PATCH that 401s would report success while changing nothing (the "secret populated, server never rotated" state the srv3 onboarding doc flagged as a mystery). Fix: --fail-with-body on all 10 redfish argv rows so exit_success reflects HTTP status (fail-with-body, not bare --fail, keeps the OpenBMC error body in `body` for diagnosis — the rotation debugging needed it). Also -fsL on the busybox tarball download so a 404 fails at the download, not later at `tar xf`. Proven by execution: with the fix the srv4 probe no longer short-circuits on the factory 401 (it proceeds past FactoryCredentialActive). Carrier note redfish_http_success_semantics_note pins why the flag is load-bearing. Audit: the fail-open was confined to extdeps/bmc/http.dag; extdeps/http/client.dag already uses -fsS/-fL. The other 90 exit_success ops use tools where exit code IS the success signal (git, cargo, systemctl, apt, ...). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The prior commit claimed the fail-open was "confined to extdeps/bmc/http.dag" and that http/client.dag "already uses -fsS/-fL". That was wrong: http.Client.Get and DownloadToFile use -fsS/-fL, but PostJsonFromFile used `-sk` with no --fail — the single-line grep missed it because the argv array is multiline. A rigorous multiline-aware census (python over every `argv: [...]` containing "curl") confirms this was the last curl-without-fail site after the redfish fix. Add --fail-with-body so a POST that 4xx/5xx's is a failure, not a silent success. (Also anemic here, tracked for the dissolution plan: `-k` insecure-TLS as a hardcoded default on the general client, and `readonly` on a POST.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The redfish fail-open was a symptom; the disease is argv anemia — extdeps shell transports hand-spell tool invocations as bare argv String arrays where each flag (-s, -k, -f, -X, -H, --data @file, ...) is an opaque String standing where a modeled request axis belonged (§2). Because --fail is a hand-typed flag and not a modeled success property, it was omittable — and omitted in 11 sites. Registers gunbc.plans.transport_argv_anemia_dissolution: - Decodes the flags into their grounded axes (method/url/header/body/tls/auth/success). - Names the dissolution target that ALREADY EXISTS (§3, don't re-coin): `transport rest` (extdeps.transports.rest), the declared declarative peer handler that 28 files use and that both the redfish and http-client carrier notes already point to. - Lane A (HTTP): extend transport rest with the 3 axes the hand-argv carries (tls-verify, auth modes, body-from-file) + MODELED SUCCESS (HTTP-status, not exit_success) → convert redfish + http.client → sweep. Kills the curl-fail-open class by construction and DELETES the interim --fail-with-body patches. §5: bad state becomes unwritable. - Lane B (non-HTTP tool argv): busybox build + the 262 argv-lines/37 files → modeled extdeps/tools/* ops, riding the shell -> intent lane, JIT per consumer. Compiles + evaluates clean (body fn renders the full MarkdownBlock tree). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Build-cache provisioning (P1b, ProvisionBuildCache on the standup spine) is srvN-agnostic, but host_toolchain_ensure_access() only had srv1/srv2/srv3 arms — srv4 fell through to `none`, so provisioning could never actuate on srv4 even though it's enrolled in fleet_intent/offers/runner_deployment. Add host_identity_srv4_access (mirrors srv2: SSH to operator_host_srv4, fleet operator principal) and the srv4 arm. Proven by execution: host_toolchain_ensure_access(srv4) now resolves Present (was Absent). (host_identity_access.dag carries no import list — the import-strip state; the srv4 rows follow the existing srv1/srv2 pattern exactly, resolved globally by the interpreter. The --target dag "unlisted import" diagnostics are pre-existing and tree-wide, not introduced here.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
The release-build EAGAIN retry escalated in two steps: level-1 (CARGO_BUILD_JOBS=1, KEEPS sccache) then level-2 (EnvUnset RUSTC_WRAPPER — DROPS the cache to force a pass). Level-2 is the §5 absorbing fallback the design named "must never ship": when sccache is misconfigured under load it silently discards the cache and the build goes green, so the deficit's frequency is zeroed by construction and the corpus-denominated cost surfaces later as a timeout, never as a diagnostic. - Delete ci_retry_escalation_level2; the retry now escalates to level-1 only, then fails loud: level-1's build isn't cache-dropped, and if it fails the bins don't exist so `claim_executor --verify-build-artifacts` fails the step. No silent widen. - Regenerate .github/workflows/ci.yml from the authority (expected_ci_yml); the level-2 nested retry block collapses to the single level-1 line. Byte-verified: trailing bytes identical, 9 lines removed (the level-2 block), no other drift. - Flip witness_gunbc_ci_step_projects_run_script_markers: it asserted `env -u RUSTC_WRAPPER` PRESENT (enshrining the fallback in the contract); now asserts it ABSENT from yaml AND build_script AND floor_script. - Delete ci_cargo_eagain_fallback_level2_scaffold (its dissolution trigger — level-2's removal — fired) and re-point provision_build_cache_widen_sketch_note (the RED-control fixture stays, guarding the verdict fold against re-introduction). Verified: ci_yaml_serializer_keystone_holds → true. REMAINING (coupled, operator-sequenced, NOT in this commit): the `if sccache --show-stats` opportunistic guard still silently skips caching when the daemon is down — removing it makes sccache mandatory, which requires the service to be provisioned first. Now unblocked (srv4 access wired; provisioning is srvN-agnostic): `gunbc` run the build-cache provision on each host, verify `sccache --show-stats` reads back, then drop the guard. That live re-provision is the ctrl-sccache.service config fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
First concrete argv-anemia dissolution (gunbc.plans.transport_argv_anemia_dissolution Lane B). busybox_build_commands hand-spelled 8 bare argv arrays where every flag was an opaque string — ["curl","-fsL","-o",..], ["tar","xf",..,"--strip-components=1"], ["mkdir","-p",..], ["make","-C",..], ["sed","-i",..], ["cp",..]. Add 6 typed builders to extdeps.exec.command (the ArgvCommand home, beside the existing scp_command precedent): curl_download_command / mkdir_p_command / tar_extract_command / make_command / sed_in_place_command / cp_command. Each names the operation and DERIVES the argv, so the flags (-f, -p, --strip-components, ...) live in ONE place, not hand-spelled per call site (§2). busybox_build_commands now reads as named operations: curl_download_command(url:.., destination:..) tar_extract_command(archive:.., into_directory:.., strip_components: 1) make_command(directory:.., arguments: [arch, cross, "defconfig"]) ... Proven by execution (dag/test/claim/busybox_argv_builders_test.dag): - builders derive byte-identical argv (List<String> == the expected literal), - RED control: a wrong flag (mkdir without -p, strip_components: 2, curl without -f) is != the expected — the equality discriminates, - busybox_build_commands still yields the same 8-command sequence (integration probe). Side finding, NOT fixed here (pre-existing, confirmed by stashing my changes): command_runner.dag:32 uses shell.Exec.Run without importing extdeps.shell, so it only resolves in broad closures (the import-strip Class-B fragility) — a busybox-importing witness needs extdeps.shell.exec dragged into the pool to compile. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
B (remove ci_retry_escalation_level2) changed the shared release-build retry authority, which BOTH .github/workflows/ci.yml AND .github/workflows/falsifier.yml project. I hand-collapsed ci.yml (byte-correct — main_wet leaves it unchanged) but missed falsifier.yml, so generated_artifact_drift_gate_passes reded and the heal job regenerated it but couldn't push (App lacks `workflows` permission). Regenerated falsifier.yml from the authority (generated_artifact_gate main_wet): the same level-2 `env -u RUSTC_WRAPPER` block collapses to the single level-1 line. Verified drift-free: a re-run of main_wet is a no-op (byte-idempotent fixed point = what the drift gate checks). Lesson: regenerate generated artifacts from the authority, never hand-edit them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nsus refuse) The standalone busybox_argv_builders_test.dag was a NEW *_test.dag, so the floor refused it: "no census TSV row for entry ... (regenerate the census)". Enrolling it would drag 3 unrelated, pre-existing-stale main entries into the census count (876 declared vs 880 on disk) — scope creep + a main-level staleness this PR shouldn't own. Instead, drop the standalone witness and fold its flag-level RED control into the already-enrolled busybox_bmc_build_witness_test: assert the rendered commands contain '-fsL' (curl download fail-close), '--strip-components=1' (tar), and 'mkdir' '-p' — so a builder that drops a derived flag reds the existing witness. Verified true by execution (broadened closure past the pre-existing command_runner shell.Exec resolution bug). Net: same flag-level coverage, zero census churn. The 6 typed builders + the busybox rewrite (previous commit) are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… busybox flags)
Batch-3 caught the witnesses that still asserted the removed -u RUSTC_WRAPPER
level-2 fallback (I flipped ci_yaml_serializer in B but missed these):
- ci_spec_witness_test: witness_release_build_eagain_cold_retry now asserts
`env -u RUSTC_WRAPPER` / `without RUSTC_WRAPPER` ABSENT (was present); the
tiers-wrap witness asserts the level-1 line as the plain `CARGO_BUILD_JOBS=1 <cmd>`
(no longer nested in `) 2>&1`) and no level-2 line.
- orchestration_retry_emit_test: ci_cargo_eagain_retry_core_golden — the
HAND-AUTHORED oracle — collapses the nested level-2 retry to the single level-1
line, matching the production seam. Its note requires updating BOTH sides
deliberately (a perturbation can't green by moving both at once); orch_retry_perturbed_level2
stays as the RED control (adds level2 back → must differ from the golden).
Verified: byte_identical_to_hand_authored + production_seam_matches_golden both true.
Also lands the busybox flag assertions ('-fsL' / '--strip-components=1' / 'mkdir' '-p')
in busybox_bmc_build_witness_test — they were dropped in 91d7ec1 because the
`git add` aborted on the already-deleted standalone pathspec.
All fixed witnesses green by execution locally.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
briansrls
added a commit
that referenced
this pull request
Jul 24, 2026
…t (config is a language too) (#7159) * shell→dag census §4.I: the CI foreign-executor sites the census missed A 2026-07-24 three-facet CI audit found five CI `run:` shell sites that landed after the census's #78f43c38 grounding and are tracked only at the site (rich in-code notes) but never rolled up into §4/§5: ci_spec.dag · ci_fmt_gate_line (→ cargo.Build.Fmt, §5.B call-the-op) ci_spec.dag · gunbc_ci_deploy_invoke (E-emit) ci_spec.dag · gunbc_ci_heal_regen_invoke (E-emit) ci_spec.dag · ci_heal_git_add_lines + commit/push (E-emit → typed git.Core.*) ci_materialization.dag · ci_sccache_provider_shell_injection (E-emit + a §5 ABSORBING-FALLBACK: the `if sccache --show-stats` guard silently skips caching when the daemon is down — dissolves to a mandatory provisioning-ensure, the sibling of the level-2 `-u RUSTC_WRAPPER` fallback removed on #7138) Also records the §5 absorbing-fallbacks INSIDE the CI scripts that the census tracked the scripts but not the widens for (git rev-parse … || pwd ×17; git fetch … || true; the toolchain-unresolved cache-poison slug; the CARGO_BUILD_JOBS=1 fleet-pressure mask), each owed a modeled outcome on migration. And cross-references the ADJACENT (not shell-string) model↔realization forks the same audit found — toolchain isolation (ToolchainEnvIsolation modeled but the workflow hand-spells per-job isolation → the rustup ETXTBSY race in a prose note), GHA cache key, artifact upload/download, permissions — which belong to the model↔realization-fork thread / roadmap 2-emit-partition, noted here only so they aren't lost across the seam. Keyed on file+symbol per the §4 convention; no code changes, doc-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * shell→intent design: generalize the invariant — config is a language too The doc's thesis is "the intent names no language" (shell strings). The 2026-07-24 CI-workflow audit found the same §3/§4 violation in a second sink that isn't a string — hand-spelled CONFIG (toolchain isolation, GHA cache key, action version/components, permissions) whose modeled authority already exists (ToolchainEnvIsolation, gha_actions_cache_facts, ToolchainCapability, std.effect_grant) but which the workflow emit hand-picks instead of deriving. Adds a principle-only section that sharpens the invariant to "the EMITTER names no concept": - the second sink (config, not strings) — same disease, one sink over; - the litmus "opaque to emission" — if the emitter special-cases a concept (it has an "isolate this job" branch), the concept is forked emitter logic, not a fact; - the bespoke ci_workflow/ci_yaml_emit is itself the fork — there should be ONE emit(intent, target); a second workflow emitter is the N×M trap at workflow scale (roadmap 2-emit-partition already targets the general-emit end-state); - config-opacity + the multi-target Python litmus are the same property (target is a parameter); - the wall generalized: a lens that reds config hand-written where a model exists. Respects the doc's no-second-census discipline — states the principle, references census §4.I for the sites. Doc-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Brian Searls <briansearls1@gmail.com> Co-authored-by: Claude Opus 4.8 (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.
Found by execution while testing #7027's srv4 onboarding against the live BMC. Three related threads, all proven by execution.
1. Redfish transport fails closed (§5 fail-open, found live vs srv4)
Every
redfish.Http.*op derivedsuccess: Bool from "exit_success"but rancurl -sS -kwith no--fail— and curl exits 0 on 401/403/404/5xx. Sosuccessread any non-2xx as true: a fail-open across the whole BMC transport.bmc_probe_credential_phasereportedFactoryCredentialActivefor an already-rotated srv4 BMC, because the factory-loginGetSystemreturned HTTP 401 (0-byte body) and curl's exit 0 madesuccess=true. Left unfixed, the converge would take theFactoryCredentialActive → mint+store+rotatearm against a healthy host; andSetAccountPassword.successis exit-based too, so a rotation PATCH that 401s would report success while changing nothing (the "secret populated, server never rotated" state the srv3 doc flagged as a mystery — likely this bug).--fail-with-bodyon all 10 redfish argv rows (fail-with-body keeps the OpenBMC error body for diagnosis). Carrier noteredfish_http_success_semantics_notepins why the flag is load-bearing.curl: (22) ... 401, correctly failing). Removing the fail-open also exposed a downstreamdata_b64credential-decoding suspect (classic §5 — it was masked).2. http.Client.PostJsonFromFile fails closed too (+ audit correction)
A rigorous multiline-aware census found one more curl-without-
--failsite (my first-pass single-line grep missed the multiline argv). Fixed. Everything else uses tools where exit code is the success signal.3. Argv-anemia dissolution plan (roadmap artifact)
Registers
gunbc.plans.transport_argv_anemia_dissolution. The fail-open's root cause is argv anemia:--failis a hand-typed flag, not a modeled success property, so it was omittable. The dissolution target already exists (§3):transport rest(28 files use it; both carrier notes name it), which models success by HTTP status — making the class unwritable. Redfish is the named "PR B"; its live path couples to the emit-execution lane (interpreter refuses insecure-TLS per the 2026-07-16 ruling). Lane B = the non-HTTPArgvCommandargv, JIT.4. sccache: srv4 wired into the access dispatch
host_toolchain_ensure_access()had no srv4 arm — it fell through tonone, so build-cache provisioning could never actuate on srv4 despite srv4 being enrolled in fleet_intent/offers/runner_deployment. Addedhost_identity_srv4_access+ the arm. Proven:host_toolchain_ensure_access(srv4)now resolvesPresent.Follow-up commit incoming on this branch (operator-sequenced): remove the CI
-u RUSTC_WRAPPERabsorbing fallback (§5) + fix thectrl-sccache.serviceconfig, now that srv4 can be provisioned.🤖 Generated with Claude Code