vh/esh-pfi-infrastructure
1
0
Fork 0
Code Issues 7 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(headscale-ddns): retry Cloudflare, validate every response, never write blind

Browse Source 
Three of the four failures in the week to 2026-09-23 (09-19, 09-21, 09-23)
had one signature. The Cloudflare zone lookup returned an empty body at its
15s cap, a bare json.load crashed with tracebacks instead of a cause, and the
script carried empty IDs on to a PATCH against zones//dns_records/.
Cloudflare rejected it, so there was no DNS impact, but only by accident.

- Cloudflare calls go through cf(): 3 announced tries, and a call counts
  only when the body says success:true.
- success:true is not trusted as shape. pick() validates every body and
  prints only the fields asked for, or one line saying why not. It requires
  exactly one zone named phasefinal.com and exactly one A record named
  headscale.phasefinal.com, each with a non-empty id and content. Two A
  records are refused rather than half-updated, and an empty id can no
  longer shift the content into the id slot.
- No write without both IDs. The run ends on a confirmation that the record
  now reads the new address. The previous final line was an echo whose exit
  status was always 0, even when the parse inside it failed.
- Only a global unicast IPv4 is published (python ipaddress is_global).
  Loopback, RFC1918, link-local, CGNAT and documentation ranges are retried
  and then refused.
- curl -q as the first argument ignores ~/.curlrc, so a verbose config can
  never log the bearer token. The vault CLI path is quoted. The empty
  data-array expansion is safe under set -u on bash < 4.4.

Tests: services/headscale-ddns/test_headscale_ddns.py, 12 cases with curl,
the vault CLI and sleep stubbed. Each failure case asserts the FATAL line's
stated reason, so a run that died earlier for an unrelated cause cannot pass
it. The documentation-range fixtures (203.0.113.x) were themselves rejected
by the new public-IP guard: a free positive control.

Live: a manual run and a unit run both printed "unchanged 70.230.226.88",
Result=success. Cross-model bug-hunt (heid "Talus", Gróa + seat): all 8
findings folded.
This commit is contained in:
vh
2026-09-23 16:27:58 -07:00
parent 466f7aa4e6
commit fedd4b6d95
3 changed files with 305 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
services/headscale-ddns/README.md
+30
View File
@@ -29,6 +29,36 @@ Two fixes, both verified by making them fail:
should not page a human, and a *silent* retry would hide a degrading
dependency.
## 2026-09-23: the Cloudflare half got the same treatment
Three of the four failures in the week to 09-23 (09-19, 09-21, 09-23) had one
signature: the **Cloudflare zone lookup returned an empty body at its 15s cap**.
The script parsed that with a bare `json.load`, crashed with tracebacks instead of
a cause, and carried empty IDs on to a PATCH against `zones//dns_records/`.
Cloudflare rejected it, so there was no DNS impact — fail-closed by accident.
Now, verified by `test_headscale_ddns.py` (curl, vault and sleep stubbed; 12
cases) and a live run:
- **Every Cloudflare call retries 3×**, announced, and only counts when the body
says `success: true`.
- **`success: true` is not trusted as shape.** Every body goes through one
validating extractor (`pick`): exactly one zone named `phasefinal.com`,
exactly one A record named `headscale.phasefinal.com`, non-empty `id` and
`content`. Two A records are refused rather than half-updated, and an empty
`id` can no longer shift the record's content into the id slot.
- **Nothing is written without both IDs**, and the run ends on a CONFIRMATION
that the record now reads the new address, not on an `echo` that always exits
0.
- **Only a global unicast IPv4 is published.** Loopback, RFC1918, link-local,
CGNAT 100.64/10 and documentation ranges are junk answers, retried and then
refused.
- `curl -q` (must be the first argument) ignores `~/.curlrc`, so a verbose config
there can never write the bearer token into the journal.
Cross-model bug-hunt (heid round "Talus") found the shape-trust class; all eight
findings folded.
## ⚠ The vault read is 17 of the script's 18 seconds
Measured 2026-09-22: `secret get` takes **~17s**, everything else ~1s. It runs
services/headscale-ddns/headscale-ddns.sh
+105 -9
View File
@@ -1,28 +1,124 @@
#!/bin/bash
# Keep headscale.phasefinal.com's A record on NH3's current WAN v4. Token from the vault at run time.
set -u
SECRET=/home/lkraven/development/eshpfi-management/services/secrets-broker/secret
SECRET=${HEADSCALE_DDNS_SECRET_CLI:-/home/lkraven/development/eshpfi-management/services/secrets-broker/secret}
# ⚠ SAY WHY. Both failure paths below used to `|| exit 1` in silence, and on
# 2026-09-22 15:28 this unit failed for real: the failed-START alarm fired
# correctly and carried NO CAUSE, because the script had printed nothing. An
# alarm you cannot act on costs the same triage as no alarm at all.
T=$($SECRET get nh3-dev/.config/cloudflare/infra-ops-dns-token 2>/dev/null) || {
T=$("$SECRET" get nh3-dev/.config/cloudflare/infra-ops-dns-token 2>/dev/null) || {
echo "FATAL: vault read failed for nh3-dev/.config/cloudflare/infra-ops-dns-token" >&2; exit 1; }
[ -n "$T" ] || { echo "FATAL: vault returned an EMPTY token (read succeeded, value blank)" >&2; exit 1; }
# The WAN lookup leans on a third party, so one blip should not page a human.
# Measured 2026-09-22: the whole script takes ~18s of which ~17s is the vault
# read, so three tries at a 10s cap is bounded and still well inside the 10-min
# timer. Retries are announced -- a silent retry hides a degrading dependency.
# A reachable-looking but non-public answer (127.x, 10.x, 169.254.x, CGNAT
# 100.64/10, 999.x) is junk too: Cloudflare would publish it. Only a global
# unicast IPv4 counts. `curl -q` must stay the FIRST argument everywhere: it
# makes curl ignore ~/.curlrc, so a verbose/trace config there can never write
# the Authorization header into the journal.
is_public_v4() { python3 -c 'import sys,ipaddress as i
try: a=i.ip_address(sys.argv[1])
except ValueError: sys.exit(1)
sys.exit(0 if a.version == 4 and a.is_global else 1)' "$1"; }
IP=""
for try in 1 2 3; do
IP=$(curl -s -m 10 -4 https://icanhazip.com | tr -d '[:space:]')
[[ "$IP" =~ ^[0-9.]+$ ]] && break
IP=$(curl -q -s -m 10 -4 https://icanhazip.com | tr -d '[:space:]')
is_public_v4 "$IP" && break
echo "WARN: WAN lookup attempt $try/3 returned [$IP]" >&2
sleep 2
done
[[ "$IP" =~ ^[0-9.]+$ ]] || {
echo "FATAL: could not determine WAN v4 after 3 attempts (icanhazip.com unreachable or returning junk); DNS left unchanged" >&2; exit 1; }
ZID=$(curl -s -m 15 -H "Authorization: Bearer $T" "https://api.cloudflare.com/client/v4/zones?name=phasefinal.com" | python3 -c 'import sys,json; print(json.load(sys.stdin)["result"][0]["id"])')
read -r RID CUR < <(curl -s -m 15 -H "Authorization: Bearer $T" "https://api.cloudflare.com/client/v4/zones/$ZID/dns_records?type=A&name=headscale.phasefinal.com" | python3 -c 'import sys,json; r=json.load(sys.stdin)["result"][0]; print(r["id"], r["content"])')
is_public_v4 "$IP" || {
echo "FATAL: could not determine a public WAN v4 after 3 attempts (icanhazip.com unreachable or returning junk: [$IP]); DNS left unchanged" >&2; exit 1; }
# Cloudflare gets the same treatment as the WAN lookup. Three of the four
# failures in the week to 2026-09-23 were the zone lookup returning an EMPTY
# body at its 15s cap; json.load raised, and the script then carried empty IDs
# all the way to a PATCH against zones//dns_records/. Now: a call only counts
# when Cloudflare says success:true, it is retried (announced), and nothing is
# written unless both IDs are in hand. PATCH sets an absolute value, so a retry
# of it is idempotent.
cf() { # cf METHOD URL [JSON] -> response body on stdout; non-zero after 3 failed tries
local body try data=()
[ -n "${3:-}" ] && data=(-H "Content-Type: application/json" -d "$3")
for try in 1 2 3; do
# ${data[@]+...}: an empty array under `set -u` is an unbound-variable
# error on bash < 4.4, which would fail every GET and blame Cloudflare.
body=$(curl -q -s -m 15 -X "$1" -H "Authorization: Bearer $T" ${data[@]+"${data[@]}"} "$2")
if python3 -c 'import sys,json; sys.exit(0 if json.loads(sys.argv[1]).get("success") is True else 1)' "$body" 2>/dev/null; then
printf '%s' "$body"; return 0
fi
echo "WARN: Cloudflare $1 attempt $try/3 failed: [${body:0:200}]" >&2
sleep 2
done
return 1
}
# success:true says the call worked, NOT that the body has the shape we are
# about to read. pick() validates the shape and prints exactly the fields
# asked for, one per line, or prints a one-line reason and exits non-zero.
# Every read of a Cloudflare body goes through it — a bare result[0]["id"]
# is how an empty id once shifted the record's CONTENT into the id slot.
pick() { # pick KIND BODY [EXPECT] ; KIND = zone | record | updated
python3 - "$@" <<'PY'
import sys, json
kind, body = sys.argv[1], sys.argv[2]
expect = sys.argv[3] if len(sys.argv) > 3 else None
def die(why):
print(why, file=sys.stderr); sys.exit(1)
try:
res = json.loads(body).get("result")
except Exception:
die("response is not a JSON object")
def one(name):
if not isinstance(res, list):
die(f"result is {type(res).__name__}, not a list")
if len(res) != 1:
die(f"expected exactly 1 {name}, got {len(res)}")
e = res[0]
if not isinstance(e, dict):
die(f"{name} entry is {type(e).__name__}, not an object")
return e
def text(e, key):
v = e.get(key)
if not isinstance(v, str) or not v.strip():
die(f"field {key!r} is missing or empty")
return v
if kind == "zone":
e = one("zone")
if e.get("name") != "phasefinal.com":
die(f"zone name is {e.get('name')!r}")
print(text(e, "id"))
elif kind == "record":
e = one("A record")
if e.get("type") != "A" or e.get("name") != "headscale.phasefinal.com":
die(f"record is {e.get('type')!r} {e.get('name')!r}")
print(text(e, "id")); print(text(e, "content"))
elif kind == "updated":
if not isinstance(res, dict):
die(f"result is {type(res).__name__}, not an object")
got = text(res, "content")
if got != expect:
die(f"record now reads {got!r}, not {expect!r}")
print(got)
else:
die(f"unknown kind {kind!r}")
PY
}
CF=https://api.cloudflare.com/client/v4
Z=$(cf GET "$CF/zones?name=phasefinal.com") || {
echo "FATAL: Cloudflare zone lookup for phasefinal.com failed after 3 attempts; DNS left unchanged" >&2; exit 1; }
ZID=$(pick zone "$Z" 2>&1) || {
echo "FATAL: Cloudflare zone lookup for phasefinal.com returned an unusable body ($ZID); DNS left unchanged" >&2; exit 1; }
R=$(cf GET "$CF/zones/$ZID/dns_records?type=A&name=headscale.phasefinal.com") || {
echo "FATAL: Cloudflare A record lookup for headscale.phasefinal.com failed after 3 attempts; DNS left unchanged" >&2; exit 1; }
F=$(pick record "$R" 2>&1) || {
echo "FATAL: Cloudflare A record for headscale.phasefinal.com is not usable ($F); refusing to guess; DNS left unchanged" >&2; exit 1; }
RID=$(sed -n 1p <<<"$F"); CUR=$(sed -n 2p <<<"$F")
[ "$CUR" = "$IP" ] && { echo "unchanged $IP"; exit 0; }
curl -s -m 15 -X PATCH -H "Authorization: Bearer $T" -H "Content-Type: application/json" "https://api.cloudflare.com/client/v4/zones/$ZID/dns_records/$RID" -d "{\"content\":\"$IP\"}" | python3 -c 'import sys,json; d=json.load(sys.stdin); print("updated", d["success"], d["result"]["content"])'
U=$(cf PATCH "$CF/zones/$ZID/dns_records/$RID" "{\"content\":\"$IP\"}") || {
echo "FATAL: Cloudflare update of headscale.phasefinal.com $CUR -> $IP failed after 3 attempts" >&2; exit 1; }
# The last word is the CONFIRMATION, not an echo that always exits 0: a
# success:true body we cannot read back as $IP fails the run.
NOW=$(pick updated "$U" "$IP" 2>&1) || {
echo "FATAL: Cloudflare accepted the update of headscale.phasefinal.com $CUR -> $IP but did not confirm it ($NOW)" >&2; exit 1; }
echo "updated $NOW (was $CUR)"
services/headscale-ddns/test_headscale_ddns.py
+170
View File
@@ -0,0 +1,170 @@
"""Tests for headscale-ddns.sh's Cloudflare calls (2026-09-23).
Three of the four failures in the week to 2026-09-23 (09-19, 09-21, 09-23) had
one signature: the Cloudflare zone lookup came back EMPTY after its 15s cap,
json.load raised, and the script carried on with empty IDs all the way to a
PATCH against `zones//dns_records/`. These pin the fix: Cloudflare calls retry
like the WAN lookup does, and no write is attempted without both IDs.
python3 -m unittest services/headscale-ddns/test_headscale_ddns.py
curl, the vault CLI and sleep are stubbed through PATH / env; nothing leaves
the box.
"""
import os
import pathlib
import subprocess
import tempfile
import unittest
SCRIPT = pathlib.Path(__file__).with_name("headscale-ddns.sh")
# The stub answers by URL. $CF_ZONE_FAILS makes the zone lookup return an empty
# body that many times first (the observed failure). Every call is logged.
CURL_STUB = r"""#!/bin/bash
url=""; for a in "$@"; do case "$a" in http*) url="$a";; esac; done
method=GET; prev=""; for a in "$@"; do [ "$prev" = "-X" ] && method="$a"; prev="$a"; done
echo "$method $url" >> "$STUB_DIR/calls"
echo "$1" >> "$STUB_DIR/first_args"
case "$url" in
*icanhazip.com*) echo "${WAN_IP:-1.1.1.1}";;
*"/zones?name="*)
n=$(cat "$STUB_DIR/zone_n" 2>/dev/null || echo 0); n=$((n + 1)); echo "$n" > "$STUB_DIR/zone_n"
[ "$n" -le "${CF_ZONE_FAILS:-0}" ] && exit 28 # empty body, curl timeout
if [ -n "${CF_ZONE_BODY:-}" ]; then echo "$CF_ZONE_BODY"
else echo '{"success":true,"result":[{"id":"Z1","name":"phasefinal.com"}]}'; fi;;
*"/dns_records?"*)
if [ -n "${CF_RECORDS_BODY:-}" ]; then echo "$CF_RECORDS_BODY"
elif [ -n "${CF_NO_RECORD:-}" ]; then echo '{"success":true,"result":[]}'
else echo '{"success":true,"result":[{"id":"R1","type":"A","name":"headscale.phasefinal.com","content":"'"${DNS_IP:-1.1.1.1}"'"}]}'; fi;;
*"/dns_records/"*)
if [ -n "${CF_PATCH_BODY:-}" ]; then echo "$CF_PATCH_BODY"
else echo '{"success":true,"result":{"content":"'"${WAN_IP:-1.1.1.1}"'"}}'; fi;;
*) echo '{"success":false,"errors":[{"message":"stub: unexpected url"}],"result":null}';;
esac
"""
class CloudflareCallTests(unittest.TestCase):
def setUp(self):
self.tmp = tempfile.TemporaryDirectory()
self.addCleanup(self.tmp.cleanup)
d = pathlib.Path(self.tmp.name)
bindir = d / "bin"
bindir.mkdir()
vault = d / "vault cli" # a space in the path, on purpose
vault.mkdir()
for p, body in ((bindir / "curl", CURL_STUB),
(bindir / "sleep", "#!/bin/sh\nexit 0\n"),
(vault / "secret", "#!/bin/sh\necho test-token\n")):
p.write_text(body)
p.chmod(0o755)
self.d = d
self.env = dict(os.environ, PATH=f"{bindir}:{os.environ['PATH']}",
STUB_DIR=str(d), HEADSCALE_DDNS_SECRET_CLI=str(vault / "secret"))
def run_script(self, **env):
return subprocess.run(["bash", str(SCRIPT)], env=dict(self.env, **env),
capture_output=True, text=True, timeout=60)
def calls(self):
f = self.d / "calls"
return f.read_text().splitlines() if f.exists() else []
def test_transient_empty_zone_lookup_is_retried(self):
r = self.run_script(CF_ZONE_FAILS="2")
self.assertEqual(r.returncode, 0, r.stderr)
self.assertIn("unchanged 1.1.1.1", r.stdout)
self.assertEqual(r.stderr.count("WARN: Cloudflare"), 2, r.stderr)
def test_persistent_empty_zone_lookup_fails_loudly_and_never_writes(self):
r = self.run_script(CF_ZONE_FAILS="99")
self.assert_clean_fatal(r, "zone")
self.assertFalse(any(c.startswith("PATCH") for c in self.calls()), self.calls())
self.assertFalse(any("/zones//" in c for c in self.calls()), self.calls())
def test_missing_a_record_fails_loudly_and_never_writes(self):
r = self.run_script(CF_NO_RECORD="1", DNS_IP="198.51.100.1")
self.assert_clean_fatal(r, "A record")
self.assertFalse(any(c.startswith("PATCH") for c in self.calls()), self.calls())
def test_changed_ip_is_patched_once(self):
r = self.run_script(DNS_IP="198.51.100.1", WAN_IP="1.1.1.1")
self.assertEqual(r.returncode, 0, r.stderr)
patches = [c for c in self.calls() if c.startswith("PATCH")]
self.assertEqual(patches, [
"PATCH https://api.cloudflare.com/client/v4/zones/Z1/dns_records/R1"])
self.assertIn("updated 1.1.1.1", r.stdout)
def patched(self):
return any(c.startswith("PATCH") for c in self.calls())
def assert_clean_fatal(self, r, why):
"""Failed, with a one-line FATAL naming `why` — so a test cannot pass
just because the run died earlier for some unrelated reason."""
self.assertEqual(r.returncode, 1, r.stdout + r.stderr)
self.assertNotIn("Traceback", r.stderr)
fatal = [l for l in r.stderr.splitlines() if l.startswith("FATAL")]
self.assertEqual(len(fatal), 1, r.stderr)
self.assertIn(why, fatal[0])
def test_success_true_patch_with_no_result_is_a_failure(self):
# The update must be confirmed from the response, and a confirmation
# that cannot be read must fail the run, not print "updated" and exit 0.
r = self.run_script(DNS_IP="198.51.100.1", CF_PATCH_BODY='{"success":true,"result":null}')
self.assert_clean_fatal(r, "update")
self.assertNotIn("updated", r.stdout)
def test_patch_response_showing_another_value_is_a_failure(self):
r = self.run_script(DNS_IP="198.51.100.1",
CF_PATCH_BODY='{"success":true,"result":{"content":"198.51.100.1"}}')
self.assert_clean_fatal(r, "update")
def test_two_a_records_are_refused_not_half_updated(self):
two = ('{"success":true,"result":['
'{"id":"R1","type":"A","name":"headscale.phasefinal.com","content":"198.51.100.1"},'
'{"id":"R2","type":"A","name":"headscale.phasefinal.com","content":"198.51.100.2"}]}')
r = self.run_script(CF_RECORDS_BODY=two)
self.assert_clean_fatal(r, "A record")
self.assertFalse(self.patched())
def test_empty_record_id_cannot_shift_content_into_the_id(self):
body = ('{"success":true,"result":[{"id":"","type":"A",'
'"name":"headscale.phasefinal.com","content":"198.51.100.1"}]}')
r = self.run_script(CF_RECORDS_BODY=body)
self.assert_clean_fatal(r, "A record")
self.assertFalse(self.patched())
def test_misshapen_success_bodies_fail_with_one_line_not_a_traceback(self):
for var, body in (("CF_ZONE_BODY", '{"success":true,"result":{"id":"Z1"}}'),
("CF_ZONE_BODY", '{"success":true,"result":[{}]}'),
("CF_RECORDS_BODY", '{"success":true,"result":[{}]}'),
("CF_RECORDS_BODY", '{"success":true,"result":"x"}')):
with self.subTest(var=var, body=body):
(self.d / "calls").unlink(missing_ok=True)
r = self.run_script(**{var: body})
self.assert_clean_fatal(r, "zone" if var == "CF_ZONE_BODY" else "A record")
self.assertFalse(self.patched())
def test_non_public_wan_answer_is_never_published(self):
for junk in ("127.0.0.1", "10.1.2.3", "192.168.1.5", "169.254.1.1",
"100.64.0.9", "999.1.1.1"):
with self.subTest(ip=junk):
(self.d / "calls").unlink(missing_ok=True)
r = self.run_script(WAN_IP=junk, DNS_IP="198.51.100.1")
self.assert_clean_fatal(r, "WAN")
self.assertFalse(self.patched())
def test_curl_ignores_curlrc_so_a_verbose_config_cannot_log_the_token(self):
self.run_script()
firsts = (self.d / "first_args").read_text().split()
self.assertTrue(firsts and all(a == "-q" for a in firsts), firsts)
def test_unchanged_ip_is_not_patched(self):
r = self.run_script()
self.assertEqual(r.returncode, 0, r.stderr)
self.assertFalse(any(c.startswith("PATCH") for c in self.calls()))
if __name__ == "__main__":
unittest.main()