Skip to content

Commit 5ceccf2

Browse files
Andrew Bakerclaude
Andrew Baker
and
claude
committed
fix(hot-standby): address all code review findings (C1–L6)
Critical: - C1 install-standby-sync: ExecStart used NVMe mount-prefix path; strip SD_ROOT_MNT so the path is correct when the SD card boots - C2 standby-restore-agent: replace `source "${TRIGGER}"` with safe explicit field parsing + shell-metacharacter rejection (injection guard) - C3 hot-standby-sync + standby-restore-agent: sync state now lives on the SD boot partition (.pi2s3-last-synced on FAT); NVMe-side state file was wiped on every restore, causing infinite reboot loops High: - H1 pi-image-backup: guard standby_failback() against empty STANDBY_FAILBACK_CMD; sync marker is never written without a real failback - H2 cf-dns-swap: wrap CF API calls in 3-attempt retry with exponential backoff; transient 429/503 no longer leaves DNS permanently on standby - H3 standby-restore-agent: add error handling on both sudo reboot calls - H4 route53-swap: capture CHANGE_ID stdout-only; stderr goes to temp file so warnings no longer corrupt the change ID Medium: - M1 hot-standby-sync: register cleanup trap before mktemp -d (closes window where a crash left SD card mounted + temp dirs unremoved) - M2 hot-standby-sync: add flock single-instance guard (prevents concurrent cron runs racing on the SD card) - M3 hot-standby-sync: distinguish NoSuchKey from real S3 errors; auth failures now surface as ERROR + ntfy instead of silent exit 0 - M4 cf-dns-swap: missing CNAME now exits non-zero (partial failover = failure) - M5 install-standby-sync: remove dead SD_BOOT_MNT / _SD_BOOT_MOUNTED code - M6 standby-restore-agent: _NTFY_SITE now sourced from config.env (CF_SITE_HOSTNAME) after removing the injected trigger source Low: - L1 hot-standby-sync: remove unused HOST_SHORT variable - L2 hot-standby-sync: move exec>> log redirect to before config loading - L3 install-standby-sync: chmod fallback 666 → 640 - L4 hot-standby-sync: remove NTFY_URL from trigger file; restore agent reads NTFY_URL from its own config.env on the SD card - L5 route53-swap: fix trailing comma trim with sed (heredoc appends newline after comma; string suffix operator did nothing) - L6 standby-restore-agent: capture restore exit code before error message Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 70c0a82 commit 5ceccf2

6 files changed

Lines changed: 237 additions & 131 deletions

File tree

‎extras/failover/cf-dns-swap.sh‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,32 +60,54 @@ cf_api() {
6060
curl "${args[@]}"
6161
}
6262

63+
# Wrap CF API calls with up to 3 attempts (exponential backoff).
64+
# Transient 429/503 from CF must not leave DNS stuck on standby.
65+
cf_api_retry() {
66+
local _attempt _resp _success _err
67+
for _attempt in 1 2 3; do
68+
_resp=$(cf_api "$@")
69+
_success=$(echo "${_resp}" | grep -o '"success":[a-z]*' | cut -d: -f2 || true)
70+
[[ "${_success}" == "true" ]] && { echo "${_resp}"; return 0; }
71+
_err=$(echo "${_resp}" | grep -o '"message":"[^"]*"' | head -1 | cut -d'"' -f4 || true)
72+
echo " CF API attempt ${_attempt}/3 failed: ${_err:-${_resp}}" >&2
73+
[[ ${_attempt} -lt 3 ]] && sleep $(( _attempt * 10 ))
74+
done
75+
echo "${_resp}"
76+
return 1
77+
}
78+
6379
echo "CF DNS swap: pointing to ${LABEL} (${TARGET_UUID})..."
6480

81+
_SKIP_COUNT=0
6582
IFS=',' read -ra DOMAINS <<< "${CF_FAILOVER_DOMAINS}"
6683
for domain in "${DOMAINS[@]}"; do
6784
domain="${domain// /}"
6885
[[ -z "${domain}" ]] && continue
6986

70-
# Look up the DNS record ID
71-
resp=$(cf_api GET "/zones/${CF_ZONE_ID}/dns_records?type=CNAME&name=${domain}")
87+
# Look up the DNS record ID (with retry)
88+
resp=$(cf_api_retry GET "/zones/${CF_ZONE_ID}/dns_records?type=CNAME&name=${domain}") || {
89+
echo " FAIL ${domain}: could not look up DNS record after 3 attempts"
90+
exit 1
91+
}
7292
record_id=$(echo "${resp}" | grep -o '"id":"[^"]*"' | head -1 | cut -d'"' -f4 || true)
7393
if [[ -z "${record_id}" ]]; then
74-
echo " WARN: no CNAME record found for ${domain} — skipping"
94+
echo " FAIL ${domain}: no CNAME record found — cannot swap (partial failover is unsafe)"
95+
(( _SKIP_COUNT++ )) || true
7596
continue
7697
fi
7798

7899
new_content="${TARGET_UUID}.cfargotunnel.com"
79-
resp=$(cf_api PATCH "/zones/${CF_ZONE_ID}/dns_records/${record_id}" \
80-
"{\"content\":\"${new_content}\"}")
81-
success=$(echo "${resp}" | grep -o '"success":[a-z]*' | cut -d: -f2 || true)
82-
if [[ "${success}" == "true" ]]; then
83-
echo " OK ${domain} → ${new_content}"
84-
else
85-
err=$(echo "${resp}" | grep -o '"message":"[^"]*"' | head -1 | cut -d'"' -f4 || true)
86-
echo " FAIL ${domain}: ${err:-unknown error}"
100+
resp=$(cf_api_retry PATCH "/zones/${CF_ZONE_ID}/dns_records/${record_id}" \
101+
"{\"content\":\"${new_content}\"}") || {
102+
echo " FAIL ${domain}: PATCH failed after 3 attempts"
87103
exit 1
88-
fi
104+
}
105+
echo " OK ${domain} → ${new_content}"
89106
done
90107

108+
if [[ ${_SKIP_COUNT} -gt 0 ]]; then
109+
echo " ERROR: ${_SKIP_COUNT} domain(s) could not be swapped — aborting to avoid partial failover"
110+
exit 1
111+
fi
112+
91113
echo "CF DNS swap complete → ${LABEL}"

‎extras/failover/route53-swap.sh‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ for domain in "${DOMAINS[@]}"; do
8181
EOF
8282
)
8383
done
84-
CHANGES_JSON="${CHANGES_JSON%,}" # trim trailing comma
84+
CHANGES_JSON=$(printf '%s' "${CHANGES_JSON}" | sed 's/,[[:space:]]*$//')
8585

8686
CHANGE_BATCH=$(cat <<EOF
8787
{
@@ -91,13 +91,17 @@ CHANGE_BATCH=$(cat <<EOF
9191
EOF
9292
)
9393

94+
_R53_ERR_TMP=$(mktemp)
9495
CHANGE_ID=$(aws route53 change-resource-record-sets \
9596
--hosted-zone-id "${R53_HOSTED_ZONE_ID}" \
9697
--change-batch "${CHANGE_BATCH}" \
97-
--query 'ChangeInfo.Id' --output text 2>&1) || {
98-
echo " ERROR: Route53 change failed: ${CHANGE_ID}"
98+
--query 'ChangeInfo.Id' --output text 2>"${_R53_ERR_TMP}") || {
99+
echo " ERROR: Route53 change failed:"
100+
cat "${_R53_ERR_TMP}"
101+
rm -f "${_R53_ERR_TMP}"
99102
exit 1
100103
}
104+
rm -f "${_R53_ERR_TMP}"
101105

102106
echo " Change submitted: ${CHANGE_ID}"
103107

‎extras/firstboot/standby-restore-agent.sh‎

Lines changed: 96 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,17 @@
33
# extras/firstboot/standby-restore-agent.sh
44
#
55
# Installed on the SD card by install-standby-sync.sh.
6-
# Runs automatically on every SD boot (via /etc/rc.local or
7-
# a systemd service wired by install-standby-sync.sh).
6+
# Runs automatically on every SD boot (via a systemd oneshot
7+
# service wired by install-standby-sync.sh).
88
#
99
# On each SD boot this script checks for a trigger file written
1010
# by hot-standby-sync.sh on the NVMe Pi. If found:
11-
# 1. Reads restore parameters from the trigger
11+
# 1. Safely reads restore parameters from the trigger
1212
# 2. Runs pi-image-restore.sh → overwrites NVMe from S3
1313
# 3. Runs the post-restore script (tunnel swap, hostname, etc.)
14-
# 4. Removes the trigger so the restore doesn't repeat
15-
# 5. Reboots into the freshly restored NVMe
14+
# 4. Writes last-synced date to SD boot partition (FAT, persists)
15+
# 5. Removes the trigger so the restore doesn't repeat on next boot
16+
# 6. Reboots into the freshly restored NVMe
1617
#
1718
# If no trigger file is found, this script exits immediately
1819
# and the Pi boots normally (allows normal SD use).
@@ -34,15 +35,6 @@ echo "========================================================"
3435
echo " pi2s3 standby restore agent — $(date)"
3536
echo "========================================================"
3637

37-
# ── Load trigger parameters ───────────────────────────────────────────────────
38-
# shellcheck disable=SC1090
39-
source "${TRIGGER}"
40-
41-
echo " Restore date: ${RESTORE_DATE:-latest}"
42-
echo " Restore host: ${RESTORE_HOST:-}"
43-
echo " Restore device: ${RESTORE_DEVICE:-/dev/nvme0n1}"
44-
echo " Post-restore: ${POST_RESTORE_SCRIPT:-none}"
45-
4638
# ── Locate pi2s3 ─────────────────────────────────────────────────────────────
4739
PI2S3_DIR=""
4840
for _candidate in \
@@ -60,12 +52,11 @@ done
6052
if [[ -z "${PI2S3_DIR}" ]]; then
6153
echo " ERROR: pi2s3 tools not found on SD card."
6254
echo " Run install-standby-sync.sh on the standby Pi to install them."
63-
# Remove trigger so we don't loop on reboot
6455
rm -f "${TRIGGER}"
6556
exit 1
6657
fi
6758

68-
# ── Load config (AWS creds, bucket, etc.) ─────────────────────────────────────
59+
# ── Load config (AWS creds, NTFY_URL, etc.) ───────────────────────────────────
6960
CONFIG_FILE="${PI2S3_DIR}/config.env"
7061
if [[ ! -f "${CONFIG_FILE}" ]]; then
7162
echo " ERROR: config.env not found at ${CONFIG_FILE}"
@@ -75,6 +66,8 @@ fi
7566
# shellcheck disable=SC1090
7667
source "${CONFIG_FILE}"
7768

69+
_NTFY_SITE="${CF_SITE_HOSTNAME:-$(hostname -s)}"
70+
7871
ntfy() {
7972
[[ -z "${NTFY_URL:-}" ]] && return 0
8073
curl -s --max-time 10 \
@@ -85,77 +78,137 @@ ntfy() {
8578
"${NTFY_URL}" > /dev/null 2>&1 || true
8679
}
8780

88-
_NTFY_SITE="${NTFY_SITE:-$(hostname -s)}"
81+
# ── Safely parse trigger file (no source — avoids code injection) ─────────────
82+
# The trigger was written by hot-standby-sync.sh on the NVMe, but the NVMe
83+
# reads its S3 marker from a potentially compromised bucket. Never source
84+
# the trigger file directly; parse each field explicitly and validate.
85+
_read_trigger() {
86+
local field="$1" default="${2:-}"
87+
local val
88+
val=$(grep -m1 "^${field}=" "${TRIGGER}" 2>/dev/null \
89+
| cut -d= -f2- \
90+
| sed 's/^"//;s/"$//' \
91+
|| true)
92+
[[ -z "${val}" ]] && { echo "${default}"; return 0; }
93+
if echo "${val}" | grep -qE '[`$;|&<>()\]'; then
94+
echo " WARN: trigger field ${field} contains unsafe characters — using default" >&2
95+
echo "${default}"
96+
return 0
97+
fi
98+
echo "${val}"
99+
}
100+
101+
RESTORE_DATE=$(_read_trigger RESTORE_DATE "latest")
102+
RESTORE_HOST=$(_read_trigger RESTORE_HOST "")
103+
RESTORE_DEVICE=$(_read_trigger RESTORE_DEVICE "/dev/nvme0n1")
104+
POST_RESTORE_SCRIPT=$(_read_trigger POST_RESTORE_SCRIPT "")
105+
106+
# Validate RESTORE_DATE: must be YYYY-MM-DD or "latest"
107+
if [[ "${RESTORE_DATE}" != "latest" ]]; then
108+
if ! [[ "${RESTORE_DATE}" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}$ ]]; then
109+
echo " ERROR: invalid RESTORE_DATE in trigger: '${RESTORE_DATE}'"
110+
rm -f "${TRIGGER}"
111+
exit 1
112+
fi
113+
fi
114+
115+
# Validate RESTORE_DEVICE: must be a block device path (/dev/...)
116+
if ! [[ "${RESTORE_DEVICE}" =~ ^/dev/[a-zA-Z0-9]+$ ]]; then
117+
echo " ERROR: invalid RESTORE_DEVICE in trigger: '${RESTORE_DEVICE}'"
118+
rm -f "${TRIGGER}"
119+
exit 1
120+
fi
121+
122+
echo " Restore date: ${RESTORE_DATE}"
123+
echo " Restore host: ${RESTORE_HOST}"
124+
echo " Restore device: ${RESTORE_DEVICE}"
125+
echo " Post-restore: ${POST_RESTORE_SCRIPT:-none}"
89126

90127
ntfy "S3 > PI: ${_NTFY_SITE}: Restore Running" \
91-
"$(hostname): restore from ${RESTORE_DATE:-latest} started.
92-
Target: ${RESTORE_DEVICE:-/dev/nvme0n1}
128+
"$(hostname): restore from ${RESTORE_DATE} started.
129+
Target: ${RESTORE_DEVICE}
93130
Check log: ${LOG}" \
94131
"low" "arrows_counterclockwise"
95132

96133
# ── Run the restore ───────────────────────────────────────────────────────────
97134
RESTORE_ARGS=(
98-
--device "${RESTORE_DEVICE:-/dev/nvme0n1}"
99-
--date "${RESTORE_DATE:-latest}"
135+
--device "${RESTORE_DEVICE}"
136+
--date "${RESTORE_DATE}"
100137
--resize
101138
--yes
102139
)
103-
[[ -n "${RESTORE_HOST:-}" ]] && RESTORE_ARGS+=(--host "${RESTORE_HOST}")
140+
[[ -n "${RESTORE_HOST}" ]] && RESTORE_ARGS+=(--host "${RESTORE_HOST}")
104141

105-
# Post-restore script: use value from trigger, then fall back to config.env
142+
# Post-restore script: use trigger value, fall back to config.env
106143
_PR_SCRIPT="${POST_RESTORE_SCRIPT:-${STANDBY_POST_RESTORE_SCRIPT:-}}"
107144
if [[ -n "${_PR_SCRIPT}" && -f "${_PR_SCRIPT}" ]]; then
108145
RESTORE_ARGS+=(--post-restore "${_PR_SCRIPT}")
109146
elif [[ -n "${_PR_SCRIPT}" ]]; then
110-
echo " WARN: STANDBY_POST_RESTORE_SCRIPT not found: ${_PR_SCRIPT} — skipping"
147+
echo " WARN: post-restore script not found: ${_PR_SCRIPT} — skipping"
111148
fi
112149

113150
echo " Running: bash ${PI2S3_DIR}/pi-image-restore.sh ${RESTORE_ARGS[*]}"
114151
echo ""
115152

116-
if bash "${PI2S3_DIR}/pi-image-restore.sh" "${RESTORE_ARGS[@]}"; then
153+
_restore_rc=0
154+
bash "${PI2S3_DIR}/pi-image-restore.sh" "${RESTORE_ARGS[@]}" || _restore_rc=$?
155+
156+
if [[ ${_restore_rc} -eq 0 ]]; then
117157
echo ""
118158
echo " Restore complete."
119159
RESTORE_OK=true
120160
else
121161
echo ""
122-
echo " ERROR: pi-image-restore.sh failed (exit $?)"
162+
echo " ERROR: pi-image-restore.sh failed (exit ${_restore_rc})"
123163
RESTORE_OK=false
124164
fi
125165

126-
# ── Clean up trigger (always, even on failure, to avoid boot loops) ───────────
166+
# ── Write last-synced state to SD boot partition ──────────────────────────────
167+
# Stored on the FAT SD boot partition so hot-standby-sync.sh can read it
168+
# on the next cron run — NVMe data (and any state file on NVMe) is wiped
169+
# by each restore, but the SD boot partition persists.
170+
if [[ "${RESTORE_OK}" == "true" ]]; then
171+
if [[ -d "/boot/firmware" ]]; then
172+
_BOOT_STATE="/boot/firmware/.pi2s3-last-synced"
173+
else
174+
_BOOT_STATE="/boot/.pi2s3-last-synced"
175+
fi
176+
echo "${RESTORE_DATE}" > "${_BOOT_STATE}" 2>/dev/null \
177+
&& echo " State written: ${_BOOT_STATE} = ${RESTORE_DATE}" \
178+
|| echo " WARN: could not write sync state to ${_BOOT_STATE}"
179+
fi
180+
181+
# ── Clean up trigger (always — avoids boot loops even on failure) ─────────────
127182
rm -f "${TRIGGER}"
128183

129184
if [[ "${RESTORE_OK}" != "true" ]]; then
130185
ntfy "S3 > PI: ${_NTFY_SITE}: Restore Failed" \
131-
"$(hostname): restore from ${RESTORE_DATE:-latest} FAILED.
186+
"$(hostname): restore from ${RESTORE_DATE} FAILED (exit ${_restore_rc}).
132187
The NVMe may be in a partial state. Manual intervention required.
133188
Check log: ${LOG}" \
134189
"urgent" "sos"
135190
echo " Rebooting (NVMe may be partial — investigate before relying on standby)."
136191
sleep 5
137-
sudo reboot
192+
sudo reboot || {
193+
echo " FATAL: sudo reboot failed — manual reboot required."
194+
exit 1
195+
}
138196
exit 1
139197
fi
140198

141-
# ── Write last-synced state ───────────────────────────────────────────────────
142-
# The NVMe root is mounted at RESTORE_ROOT after restore.
143-
# Write the state file there so hot-standby-sync.sh can read it after reboot.
144-
_STATE_DEST=""
145-
if [[ -n "${STANDBY_SYNC_STATE_FILE:-}" && -n "${RESTORE_ROOT:-}" ]]; then
146-
# Map the state file path into the restored NVMe root
147-
_STATE_DEST="${RESTORE_ROOT}${STANDBY_SYNC_STATE_FILE}"
148-
sudo mkdir -p "$(dirname "${_STATE_DEST}")" 2>/dev/null || true
149-
echo "${RESTORE_DATE:-}" | sudo tee "${_STATE_DEST}" > /dev/null
150-
echo " State written: ${_STATE_DEST} = ${RESTORE_DATE:-}"
151-
fi
152-
153199
ntfy "S3 > PI: ${_NTFY_SITE}: Sync Complete" \
154-
"$(hostname): synced to ${RESTORE_DATE:-latest} backup.
200+
"$(hostname): synced to ${RESTORE_DATE} backup.
155201
Rebooting to NVMe — standby back up in ~2 min." \
156202
"low" "white_check_mark,floppy_disk"
157203

158204
echo " Rebooting to NVMe with fresh data..."
159205
echo "========================================================"
160206
sleep 2
161-
sudo reboot
207+
sudo reboot || {
208+
echo " FATAL: sudo reboot failed — manual reboot required."
209+
ntfy "S3 > PI: ${_NTFY_SITE}: Reboot Failed" \
210+
"$(hostname): restore complete but reboot failed. Pi is stuck on SD card.
211+
Manual reboot required to boot into restored NVMe." \
212+
"urgent" "sos"
213+
exit 1
214+
}

0 commit comments

Comments
 (0)