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: <label> (remote rc=N)" diagnostic, used for all six phase-06 CAPI-stack sub-steps, was dead code; a real remote failure just aborted with ssh's raw exit code and no labeled context. Fixed with the verified if pipeline; then :; fi idiom (see the process note above for why the more obvious || true fix was wrong).

3. scripts/juju-spaces-check.sh -- one unguarded display-only pipe

echo "$SPJSON" | jq -r '.spaces[].name' | sort | sed 's/^/ /' was the SOLE exception to this file's otherwise-consistent || true-guarding of every other juju/jq call (the file's own DOCFIX-115 comment documents awareness of exactly this hazard). If .spaces is null/absent -- a genuinely empty freshly-added model, the exact scenario this script is meant to run against -- jq fails, pipefail propagates it, and the bare pipeline kills the script before the SPACES6 FATAL-accumulation loop runs. Fixed with || true (safe here, unlike case #2 above: this line is PURELY for display, and the HAVE array used by the actual pass/fail logic is built by its own, separate, independent jq call on the next line -- nothing needs this line's PIPESTATUS).

4. scripts/osd-blank-check.sh -- same class, lower likelihood

echo "$info" | grep -E 'virtual size|disk size' | sed 's/^/ /', unguarded, same set -euo pipefail hazard as #3. Low likelihood in practice (qemu-img info's standard fields make a total match-miss unlikely) but the same bug class as three other confirmed instances in this same sweep -- fixed with || true for consistency/defense in depth (same safety reasoning as #3: nothing downstream needs this line's exit status).

5. scripts/ledger-scan.sh -- DOCFIX/BUNDLEFIX next-free regex, dormant landmine

nextfree_mentions 'DOCFIX-[0-9]{3}' ... and the BUNDLEFIX equivalent used a FIXED {3} quantifier, not {3,} -- the exact same bug class this repo already got bitten by once for the D-series (a fixed-width pattern going blind at a digit-count boundary; that incident's own fix, {3,}, was never mirrored onto the DOCFIX/BUNDLEFIX patterns in this file). Reproduced: echo "DOCFIX-1004" | grep -oE 'DOCFIX-[0-9]{3}' | grep -oE '[0-9]+' yields 100, truncating a 4-digit number and silently proposing an already-used next-free number. Not yet reachable (repo is at DOCFIX-17x as of this fix) but a real, confirmed-via-reproduction dormant landmine. Fixed to {3,}, matching repo_lint.py's own L5 check convention. Re-verified bash scripts/ledger-scan.sh still reports the correct current next-free numbers after the fix.

6. scripts/dc-dc-rbd-mirror.sh -- bootstrap token file hardening (security)

Flagged by the security-audit agent: the default rbd-mirror bootstrap token path (TOKEN_OUT) was a fixed, predictable filename in world- writable /tmp on the remote unit, with no chmod applied after creation -- inheriting the remote user's default umask (commonly 0644, world-readable). This token is genuinely secret-adjacent material (it "grants cluster access" per the script's own header comment). Fixed: the default path now includes 4 random bytes from /dev/urandom (falling back to the script's own PID if /dev/urandom/od are unavailable), and the remote command now chmod 600s the file immediately after creation, in the SAME command (via &&), closing the window where it could be briefly world-readable. Updated the script's own "transfer OUT OF BAND" reminder to describe the hardened posture and to remind the operator to remove the file from the source unit after transfer.

Verification

bash scripts/repo-lint.sh: 0 fail, 1 documented legacy warn. bash tests/dc-dc-rbd-mirror/run-tests.sh: 19/19 PASS (unaffected by the token- path hardening -- no test hardcodes the exact old path/command string). Every fix above was independently reproduced via an isolated bash -c snippet BEFORE being applied to the real file, confirming both that the bug is real and that the chosen fix actually resolves it (not just "looks right") -- full reproduction transcripts are in this session's own working history, not restated here. Full test gauntlet re-run: 23/51 FAILED, identical pre-existing environment gaps as every prior check tonight (jq missing on this workstation) -- zero regressions, including phase-06-capi-stack's own harness (ALL PASS).

REVERT: git checkout HEAD~ -- scripts/carve-host-interfaces.sh scripts/phase-06-capi-stack.sh scripts/juju-spaces-check.sh scripts/osd-blank-check.sh scripts/ledger-scan.sh scripts/dc-dc-rbd-mirror.sh (safe -- all six are logic/hardening fixes with no behavior change for the success path).