From d95adecef55ba01ea442bef2ff5ff238eed24220 Mon Sep 17 00:00:00 2001 From: Kieren Johnstone Date: Wed, 19 Aug 2026 06:56:39 +0100 Subject: [PATCH 1/2] Match DNS records by exact name, not substring 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) --- k8s-tools.sh | 84 ++++++++++++++++++++++++++++++++++++++-------------- readme.md | 13 ++++++++ 2 files changed, 75 insertions(+), 22 deletions(-) diff --git a/k8s-tools.sh b/k8s-tools.sh index b44abe4..e6d192b 100644 --- a/k8s-tools.sh +++ b/k8s-tools.sh @@ -1,6 +1,6 @@ #!/bin/bash -echo cloudflare-cli: k8s-tools v0.0.23 +echo cloudflare-cli: k8s-tools v0.0.24 bad=0 if [ -z "$action" ]; then echo "variable 'action' is not set"; bad=1; fi @@ -26,10 +26,47 @@ fi record_type=${CF_DNS_TYPE:="A"} bad=1 -zone_id=$(curl https://api.cloudflare.com/client/v4/zones?name=$CF_API_DOMAIN \ --H "Authorization: Bearer $CF_API_KEY" | jq -r ".result[] | select(.name | contains(\"$CF_API_DOMAIN\")) | .id") +# An API token authenticates as a bearer; a Global API Key needs the account email alongside it. +# Both are accepted, so a chart still supplying CF_API_EMAIL keeps working on this version. +if [ -n "$CF_API_EMAIL" ]; then + auth=(-H "X-Auth-Email: $CF_API_EMAIL" -H "X-Auth-Key: $CF_API_KEY") +else + auth=(-H "Authorization: Bearer $CF_API_KEY") +fi + +# The record this invocation owns, as Cloudflare names it. Every lookup below matches it exactly. +case "$subdomain" in + *".$CF_API_DOMAIN") fqdn="$subdomain" ;; + *) fqdn="$subdomain.$CF_API_DOMAIN" ;; +esac + +zone_id=$(curl -s -G https://api.cloudflare.com/client/v4/zones \ +--data-urlencode "name=$CF_API_DOMAIN" \ +"${auth[@]}" | jq -r --arg zone "$CF_API_DOMAIN" '.result[] | select(.name == $zone) | .id') if [ -z "$zone_id" ]; then echo "zone not found"; exit 1; fi +# Resolves the id of the record named $fqdn, or the empty string. +# +# Matching is exact in both the query and the filter. The API's `search` parameter is a substring +# filter, so looking up `pepper` also returns `pepper-mcp`, and a deploy of one service would then +# rewrite or delete another service's record. +# +# Refuses to continue on a duplicate rather than expanding several ids into a request URL. +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" +} + if [ $action = "create" ]; then bad=0 @@ -81,32 +118,31 @@ if [ $action = "create" ]; then echo public Host Name: $dns_record_value fi - echo looking up existing record to delete... - cloudflare_record_id=$(curl https://api.cloudflare.com/client/v4/zones/$zone_id/dns_records?search=$subdomain \ - -H "Authorization: Bearer $CF_API_KEY" | jq -r ".result[] | select(.name | contains(\"$subdomain\")) | .id") + echo "looking up existing $record_type record for $fqdn..." + cloudflare_record_id=$(lookup_record_id) if [ -z "$cloudflare_record_id" ] then echo creating for first time... - curl https://api.cloudflare.com/client/v4/zones/$zone_id/dns_records \ + curl -s https://api.cloudflare.com/client/v4/zones/$zone_id/dns_records \ -H 'Content-Type: application/json' \ - -H "Authorization: Bearer $CF_API_KEY" \ + "${auth[@]}" \ -d '{ "content": "'$dns_record_value'", - "name": "'$subdomain'", + "name": "'$fqdn'", "proxied": '$use_proxy', "type": "'$record_type'" }' retVal=$? else echo updating... - curl https://api.cloudflare.com/client/v4/zones/$zone_id/dns_records/$cloudflare_record_id \ + curl -s "https://api.cloudflare.com/client/v4/zones/$zone_id/dns_records/$cloudflare_record_id" \ -X PATCH \ -H 'Content-Type: application/json' \ - -H "Authorization: Bearer $CF_API_KEY" \ + "${auth[@]}" \ -d '{ "content": "'$dns_record_value'", - "name": "'$subdomain'", + "name": "'$fqdn'", "proxied": '$use_proxy', "type": "'$record_type'" }' @@ -115,16 +151,20 @@ if [ $action = "create" ]; then fi if [ $action = "delete" ]; then bad=0 - echo deleting... - echo looking up existing record to delete... - cloudflare_record_id=$(curl https://api.cloudflare.com/client/v4/zones/$zone_id/dns_records?search=$subdomain \ - -H "Authorization: Bearer $CF_API_KEY" | jq -r ".result[] | select(.name | contains(\"$subdomain\")) | .id") - - echo deleting... - curl https://api.cloudflare.com/client/v4/zones/$zone_id/dns_records/$cloudflare_record_id \ - -X DELETE \ - -H "Authorization: Bearer $CF_API_KEY" - retVal=$? + echo "looking up existing $record_type record for $fqdn..." + cloudflare_record_id=$(lookup_record_id) + + if [ -z "$cloudflare_record_id" ] + then + echo "no $record_type record named $fqdn - nothing to delete" + retVal=0 + else + echo deleting... + curl -s "https://api.cloudflare.com/client/v4/zones/$zone_id/dns_records/$cloudflare_record_id" \ + -X DELETE \ + "${auth[@]}" + retVal=$? + fi fi if [ $bad -eq 1 ]; then echo "unknown action - use create or delete"; exit 1; fi diff --git a/readme.md b/readme.md index d449c34..97c9989 100644 --- a/readme.md +++ b/readme.md @@ -2,6 +2,19 @@ This tool is used as aprt of our [helm charts](https://github.com/RedRiverSoftware/k8s/tree/master/helm3-charts) to manage Cloudflare DNS records automatically post deployment. +## Authentication + +Set `CF_API_KEY` to a Cloudflare API token and leave `CF_API_EMAIL` unset, or set `CF_API_KEY` to a +Global API Key and `CF_API_EMAIL` to the account address. The presence of `CF_API_EMAIL` selects +between the two, so a chart still passing it keeps working. + +## Record matching + +A record is matched by its exact name, `$subdomain.$CF_API_DOMAIN`. Cloudflare's `search` parameter +is a substring filter, so matching on the subdomain alone lets one service act on another's record +whenever one name is a prefix of another — `pepper` against `pepper-mcp`, for instance. If a zone +somehow holds more than one matching record, the script stops rather than guessing. + ## Update the Docker image Authenticate to the Azure Container Registry: From 26cf3eb47fc1486513e755b0dd3e5eaee4abc4c5 Mon Sep 17 00:00:00 2001 From: Kieren Johnstone Date: Wed, 19 Aug 2026 07:01:56 +0100 Subject: [PATCH 2/2] Make the duplicate-record guard actually stop the script 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) --- k8s-tools.sh | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/k8s-tools.sh b/k8s-tools.sh index e6d192b..14a09f0 100644 --- a/k8s-tools.sh +++ b/k8s-tools.sh @@ -45,26 +45,25 @@ zone_id=$(curl -s -G https://api.cloudflare.com/client/v4/zones \ "${auth[@]}" | jq -r --arg zone "$CF_API_DOMAIN" '.result[] | select(.name == $zone) | .id') if [ -z "$zone_id" ]; then echo "zone not found"; exit 1; fi -# Resolves the id of the record named $fqdn, or the empty string. +# Sets $cloudflare_record_id to the id of the record named $fqdn, or to the empty string. # # Matching is exact in both the query and the filter. The API's `search` parameter is a substring # filter, so looking up `pepper` also returns `pepper-mcp`, and a deploy of one service would then # rewrite or delete another service's record. # -# Refuses to continue on a duplicate rather than expanding several ids into a request URL. +# Assigns to a global rather than printing: called through $(...) it would run in a subshell, where +# the exit below would end only that subshell and hand the caller an empty id - which reads as "no +# record exists", so a duplicate would quietly add another instead of stopping. lookup_record_id() { - local ids - ids=$(curl -s -G "https://api.cloudflare.com/client/v4/zones/$zone_id/dns_records" \ + cloudflare_record_id=$(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 + if [ "$(printf '%s' "$cloudflare_record_id" | 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" } if [ $action = "create" ]; then @@ -119,7 +118,7 @@ if [ $action = "create" ]; then fi echo "looking up existing $record_type record for $fqdn..." - cloudflare_record_id=$(lookup_record_id) + lookup_record_id if [ -z "$cloudflare_record_id" ] then @@ -152,7 +151,7 @@ fi if [ $action = "delete" ]; then bad=0 echo "looking up existing $record_type record for $fqdn..." - cloudflare_record_id=$(lookup_record_id) + lookup_record_id if [ -z "$cloudflare_record_id" ] then