# 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 600`s 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).