diff --git a/docs/changelog-20260710-sweep-script-fixes.md b/docs/changelog-20260710-sweep-script-fixes.md new file mode 100644 index 0000000..3efadf6 --- /dev/null +++ b/docs/changelog-20260710-sweep-script-fixes.md @@ -0,0 +1,117 @@ +# Changelog 2026-07-10 -- Full-project sweep: script logic bugs + security hardening (DOCFIX-173) + +No live infrastructure touched. Part of the full-project sweep (see +DOCFIX-170 for overall scope). A dedicated agent read every `.sh` file +under `scripts/` and found 4 confirmed logic bugs, all independently +reproduced in isolation before being trusted -- the same discipline that +caught the `dc-dc-prefixes-import` test bug earlier tonight. A separate +security-audit agent found 1 real hardening gap in a script built +earlier tonight. + +**Notable process point:** the FIRST proposed fix for one of these bugs +(`scripts/phase-06-capi-stack.sh`) was itself WRONG, caught before +committing by reproducing it in isolation: `... | cmd || true` after a +failing pipeline, under `pipefail`, silently DISCARDS the pipeline's real +`PIPESTATUS` array and replaces it with `true`'s own trivial exit status +(0) -- meaning the fix would have made the script's own "GATE FAIL" check +never fire again, silently treating every remote failure as success. That +is WORSE than the original bug (a loud-but-blind abort vs. a silent +false-pass). The correct fix uses the `if pipeline; then :; fi` idiom +instead, verified via the same reproduction technique before committing. + +## Items (all confirmed via isolated `bash -c` reproduction, not asserted) + +### 1. `scripts/carve-host-interfaces.sh` -- `emit()`'s `return 1` killed apply-mode runs on the first MAAS error +Every call site invokes `emit` as a bare, unguarded statement under this +file's `set -euo pipefail`. `emit()`'s own `fail()` call already does the +FATAL-count accounting its caller needs; the subsequent `return 1` was +never consumed by any caller, and under `set -e` it terminated the WHOLE +SCRIPT immediately on the first real MAAS mutation failure -- before +`carve_raw`'s later plane carves, the "resulting interface tree" +verification, or the final "Summary: N fatal" line could run, even though +the script's own design (a FATAL accumulator + end-of-run summary) clearly +intends to report the full picture after a partial failure. Fixed by +removing the redundant `return 1` (with a comment explaining why, to +prevent reintroduction). + +### 2. `scripts/phase-06-capi-stack.sh` -- `run_step`'s `PIPESTATUS` read was dead code +`ssh ... | sed 's/^/ /'` as a bare statement under `set -euo pipefail`: +a real ssh failure makes the pipeline itself fail (via `pipefail`), which +triggers `set -e` and kills the script at THAT line, before +`local rc=${PIPESTATUS[0]}` on the next line is ever reached -- so the +function's own "GATE FAIL: `