amadan

carlos / platform Public

edge: harden DNSimple zone discovery + tighten backend tests

Paul Campbell 7e8ba911ac00135295313268971945aa50037638 parent 78fa1e3
2 files changed, +76 −12
  • internal/edge/dns.go +18 −1
  • internal/edge/dns_test.go +58 −11
diff --git a/internal/edge/dns.go b/internal/edge/dns.go
index 4a5c621ae..88f6d8033 100644
--- a/internal/edge/dns.go
+++ b/internal/edge/dns.go
@@ -144,7 +144,13 @@ func (d *dnsimpleDNS) zoneExists(ctx context.Context, zone string) (bool, error)
}
err = d.do(ctx, "GET", "/v2/"+acct+"/zones/"+zone, nil, nil)
if err != nil {
- if errors.Is(err, errDNSimpleNotFound) {
+ // 404 (zone genuinely absent) and 403 (token's account doesn't own
+ // this candidate zone — DNSimple returns Forbidden, not NotFound, for
+ // zones outside the authenticated account) both mean "not this zone,
+ // keep walking". A 401 (bad token) is neither and falls through to
+ // the generic error below, so it aborts loudly instead of reading as
+ // "no zone found".
+ if errors.Is(err, errDNSimpleNotFound) || errors.Is(err, errDNSimpleForbidden) {
return false, nil
}
return false, err
@@ -159,6 +165,14 @@ func (d *dnsimpleDNS) zoneExists(ctx context.Context, zone string) (bool, error)
// walking suffixes) from transport or auth failures (stop and report).
var errDNSimpleNotFound = errors.New("dnsimple: not found")
+// errDNSimpleForbidden distinguishes a 403 (zone exists but isn't owned by
+// this account's token — DNSimple's zones API answers Forbidden rather than
+// NotFound for zones outside the account) from other failures. At the
+// zoneExists call site it is treated the same as errDNSimpleNotFound: keep
+// walking suffixes. A 401 (bad token) is deliberately NOT folded into this —
+// it must abort the walk with a clear diagnostic.
+var errDNSimpleForbidden = errors.New("dnsimple: forbidden")
+
func (d *dnsimpleDNS) do(ctx context.Context, method, path string, body, out any) error {
var rdr io.Reader
if body != nil {
@@ -186,6 +200,9 @@ func (d *dnsimpleDNS) do(ctx context.Context, method, path string, body, out any
if resp.StatusCode == http.StatusNotFound {
return fmt.Errorf("%s %s: %w", method, path, errDNSimpleNotFound)
}
+ if resp.StatusCode == http.StatusForbidden {
+ return fmt.Errorf("%s %s: %w", method, path, errDNSimpleForbidden)
+ }
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
msg, _ := io.ReadAll(io.LimitReader(resp.Body, 4096))
return fmt.Errorf("%s %s: %s: %s", method, path, resp.Status, strings.TrimSpace(string(msg)))
diff --git a/internal/edge/dns_test.go b/internal/edge/dns_test.go
index 13b1b0b31..2cc0e3dc3 100644
--- a/internal/edge/dns_test.go
+++ b/internal/edge/dns_test.go
@@ -49,26 +49,33 @@ func TestDNSProviderSelection(t *testing.T) {
// fakeDNSimple is a minimal DNSimple v2 API: whoami, per-zone existence,
// record create/delete. It records mutations for assertions.
type fakeDNSimple struct {
- zones map[string]bool // zone name → exists
- created []string // "zone|name|content"
- deleted []int64
- nextID int64
+ zones map[string]bool // zone name → exists (200)
+ forbidden map[string]bool // zone name → exists but not owned by this account (403)
+ created []string // "zone|name|content"
+ deleted []int64
+ nextID int64
}
+// handler wires the mux and wraps it in an auth-checking middleware so the
+// Authorization header is asserted on every request — whoami, zone lookup,
+// record create, AND record delete — not just whoami.
func (f *fakeDNSimple) handler(t *testing.T) http.Handler {
mux := http.NewServeMux()
mux.HandleFunc("GET /v2/whoami", func(w http.ResponseWriter, r *http.Request) {
- if r.Header.Get("Authorization") != "Bearer tok" {
- t.Errorf("whoami auth header = %q", r.Header.Get("Authorization"))
- }
fmt.Fprint(w, `{"data":{"account":{"id":42}}}`)
})
mux.HandleFunc("GET /v2/42/zones/{zone}", func(w http.ResponseWriter, r *http.Request) {
- if !f.zones[r.PathValue("zone")] {
+ zone := r.PathValue("zone")
+ switch {
+ case f.zones[zone]:
+ fmt.Fprintf(w, `{"data":{"name":%q}}`, zone)
+ case f.forbidden[zone]:
+ // A zone that exists but isn't owned by this account's token:
+ // DNSimple answers 403, not 404.
+ http.Error(w, `{"message":"forbidden"}`, 403)
+ default:
http.Error(w, `{"message":"not found"}`, 404)
- return
}
- fmt.Fprintf(w, `{"data":{"name":%q}}`, r.PathValue("zone"))
})
mux.HandleFunc("POST /v2/42/zones/{zone}/records", func(w http.ResponseWriter, r *http.Request) {
var body struct {
@@ -78,6 +85,12 @@ func (f *fakeDNSimple) handler(t *testing.T) http.Handler {
if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
t.Errorf("create record body: %v", err)
}
+ if body.Type != "TXT" {
+ t.Errorf("create record type = %q, want TXT", body.Type)
+ }
+ if body.TTL != 60 {
+ t.Errorf("create record ttl = %d, want 60", body.TTL)
+ }
f.nextID++
f.created = append(f.created, fmt.Sprintf("%s|%s|%s", r.PathValue("zone"), body.Name, body.Content))
w.WriteHeader(201)
@@ -89,7 +102,12 @@ func (f *fakeDNSimple) handler(t *testing.T) http.Handler {
f.deleted = append(f.deleted, id)
w.WriteHeader(204)
})
- return mux
+ return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ if r.Header.Get("Authorization") != "Bearer tok" {
+ t.Errorf("%s %s auth header = %q", r.Method, r.URL.Path, r.Header.Get("Authorization"))
+ }
+ mux.ServeHTTP(w, r)
+ })
}
// The per-account adaptation: the zone is discovered by suffix-walking fqdn
@@ -118,6 +136,35 @@ func TestDNSimplePresentDiscoversZone(t *testing.T) {
}
}
+// A 403 means "this zone exists but your token doesn't own it" — DNSimple's
+// real API answers Forbidden, not NotFound, for zones outside the account.
+// The walk must treat that the same as a 404 (keep walking) rather than
+// aborting, or flagship would never discover oncarlos.com if an intermediate
+// suffix candidate happened to be a zone owned by a different account.
+func TestDNSimplePresentSkipsForbiddenAndNotFoundZones(t *testing.T) {
+ fake := &fakeDNSimple{
+ zones: map[string]bool{"oncarlos.com": true},
+ forbidden: map[string]bool{"x.abs.oncarlos.com": true},
+ // "abs.oncarlos.com" is in neither map, so it 404s (not found).
+ }
+ srv := httptest.NewServer(fake.handler(t))
+ defer srv.Close()
+ t.Setenv("CARLOS_DNSIMPLE_BASE", srv.URL)
+
+ d := newDNSimpleDNS("tok")
+ cleanup, err := d.present(context.Background(), "_acme-challenge.x.abs.oncarlos.com", "txtvalue")
+ if err != nil {
+ t.Fatalf("present: %v", err)
+ }
+ want := []string{"oncarlos.com|_acme-challenge.x.abs|txtvalue"}
+ if !reflect.DeepEqual(fake.created, want) {
+ t.Fatalf("created = %v, want %v", fake.created, want)
+ }
+ if err := cleanup(context.Background()); err != nil {
+ t.Fatalf("cleanup: %v", err)
+ }
+}
+
func TestDNSimplePresentNoZone(t *testing.T) {
fake := &fakeDNSimple{zones: map[string]bool{}}
srv := httptest.NewServer(fake.handler(t))