Uh oh!
There was an error while loading. Please reload this page.
Match DNS records by exact name, not substring - #52
Conversation
A deploy could delete or repoint another service's DNS record. Both lookups passed the bare subdomain to Cloudflare's `search` parameter and then filtered with `contains`, and both are substring matches, so a subdomain that is a prefix of another selected the other service's record too. Live example: the portal deploys with subdomain `pepper`, which matched both `pepper.river.red` and `pepper-mcp.river.red`. The lookup returned two ids into an unquoted variable, and the request URL that got built from it deleted the MCP service's record. That host then went NXDOMAIN, and because the zone's SOA minimum is 1800s, clients that looked it up during the gap stayed broken for up to half an hour after the record came back. Lookups now filter on the full `$subdomain.$CF_API_DOMAIN`, in the query and again in jq, and a duplicate stops the script rather than expanding several ids into a URL. The zone lookup had the same flaw - `river.red` also matched `myriver.red` - and is exact now too. Two things came with it: - Both credential styles are accepted. Auth moved to API tokens in 0.0.21 but the charts still pin 0.0.20 and still pass CF_API_EMAIL, so a chart bump would have broken every DNS job in the estate until the credential was migrated. CF_API_EMAIL now selects the Global API Key headers. - Deleting a record that does not exist is a no-op rather than a DELETE against an empty id. Verified against a stubbed API: `create pepper` and `delete pepper` touch only pepper.river.red, `create pepper-mcp` touches only pepper-mcp.river.red, an unknown subdomain creates rather than patches, and the zone lookup picks river.red over myriver.red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes a production-impacting Cloudflare DNS record lookup bug by switching from substring-based matching to exact FQDN matching, preventing one service from modifying or deleting another service’s DNS record when names share prefixes (e.g., pepper vs pepper-mcp). Also adds compatibility for both Cloudflare API token auth and Global API Key auth to keep older Helm charts working during rollout.
Changes:
- Switch DNS record and zone lookups to exact-name matching (
name=$fqdn+ exactjqfilter), and abort on duplicates. - Add dual authentication support: Bearer token by default, Global API Key headers when
CF_API_EMAILis present. - Make delete a no-op when the target record does not exist.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| readme.md | Documents the new authentication behavior and exact-record matching rationale. |
| k8s-tools.sh | Implements exact DNS record lookup via shared helper, adds dual auth header selection, and makes delete idempotent when record is missing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| lookup_record_id() { | ||
| local ids | ||
| ids=$(curl -s -G "https://api.cloudflare.com/client/v4/zones/$zone_id/dns_records" \ | ||
| --data-urlencode "name=$fqdn" \ | ||
| --data-urlencode "type=$record_type" \ | ||
| "${auth[@]}" | jq -r --arg fqdn "$fqdn" '.result[] | select(.name == $fqdn) | .id') | ||
| if [ "$(printf '%s' "$ids" | grep -c .)" -gt 1 ]; then | ||
| echo "found more than one $record_type record named $fqdn - refusing to guess" >&2 | ||
| exit 1 | ||
| fi | ||
| printf '%s' "$ids" | ||
| } |
There was a problem hiding this comment.
Good catch on the location — the guard was broken. The stated mechanism isn't the cause though, so recording the real one.
Command substitution preserves embedded newlines on assignment; collapsing to spaces happens on unquoted expansion, which this doesn't do:
$ ids=$(printf 'AAA\nBBB\n'); printf '%s' "$ids" | od -c
0000000 A A A \n B B B
$ printf '%s' "$ids" | grep -c . # quoted
2
$ printf '%s' $ids | grep -c . # unquoted, for contrast
1
So grep -c . counted correctly. The actual defect was the call site: cloudflare_record_id=$(lookup_record_id) runs the function in a subshell, so exit 1 ended only the subshell. The caller received empty output, read it as "no record exists", and took the create branch — so a duplicate quietly added a third record instead of aborting. Worse than building a bad URL.
Confirmed by running it against a stubbed API returning two records of one name: it printed creating for first time... and exited 0.
Fixed by having the helper assign to the global and calling it plainly, so the exit ends the script. Same stub now gives:
create DUPLICATE : exit=1 (no PATCH/POST/DELETE issued)
delete DUPLICATE : exit=1 (no PATCH/POST/DELETE issued)
Full matrix re-run and unchanged otherwise: create pepper → .../AAA only, create pepper-mcp → .../BBB only, delete pepper → DELETE .../AAA, unknown subdomain → create path, delete-missing → no request.
The guard was called as cloudflare_record_id=$(lookup_record_id), which runs the function in a subshell, so its `exit 1` ended only that subshell. The caller got empty output, read it as "no record exists", and created another record - the opposite of stopping. The helper now assigns to the global and is called plainly, so the exit ends the script. Verified against the stubbed API: a zone holding two records of the same name exits 1 on both create and delete having issued no PATCH, POST or DELETE. The other cases are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kierenj
commented
Aug 19, 2026
Verified against the live Cloudflare APIRe-tested with the real credential (the Global API Key from the cluster secret, i.e. exactly what the charts pass today) rather than a stub. The bug is worse than describedThe old lookup for subdomain So the blast radius includes cert-manager's DNS-01 challenge records, not just sibling services — The exact lookup returns precisely one: End-to-end, real APIRan the actual script against
The "both exist" listing was produced with the old query ( AuthThe Global API Key path is exercised by all of the above, confirming the dual-auth change: a Bearer-only build would have failed against this credential, which is what would have broken the estate on a naive chart bump. All scratch records removed. Live |
The bug
Both record lookups passed the bare
$subdomainto Cloudflare'ssearchparameter and then filtered withcontains. Both are substring matches, so any subdomain that is a prefix of another selected the other service's record as well.$cloudflare_record_idis also unquoted, so a multi-match expands several ids into the request URL.What it did in production
The Red Pepper portal deploys to live with subdomain
pepper. That matched bothpepper.river.redandpepper-mcp.river.red, and the deploy removed the MCP service's record:pepper-mcp.river.redwent NXDOMAIN while its pod stayed healthy. The zone's SOA minimum is 1800s, so clients that resolved it during the gap kept failing for up to 30 minutes after the record was restored — which is what made it look intermittent.Reproduced against the real response shape:
The fix
Both lookups now match the full
$subdomain.$CF_API_DOMAIN, in the API query and again injq, via a sharedlookup_record_id. A duplicate aborts rather than building a URL from several ids. The zone lookup had the same flaw —river.redalso matchedmyriver.red— and is exact now too.Two changes came with it:
CF_API_EMAIL. Bumping them to a Bearer-only version would have broken every DNS job in the estate until the credential was migrated, soCF_API_EMAILnow selects the Global API Key headers. This is what makes the chart bump safe to merge on its own.DELETEagainst an empty id.Verification
bash -nclean, and run end to end against a stubbed Cloudflare API andkubectl:create pepperPATCH .../AAAonly —pepper-mcpuntouchedcreate pepper-mcpPATCH .../BBBonlydelete pepperDELETE .../AAAonlycreate brand-newriver.red, notmyriver.redRollout
This needs an image publish before the chart bump lands:
az acr login -n RedRiver docker build . --tag redriver.azurecr.io/cloudflare-cli:0.0.24 --pushCompanion PR bumping
saffron-app-helm3andcinnamon-app-helm3from0.0.20to0.0.24follows inRedRiverSoftware/k8s.🤖 Generated with Claude Code