Fix Hetzner TXT quoting and Porkbun record IDs for DNS-01

internal/dnsprovider/providers.go

@@ -213,14 +213,14 @@ func (provider *porkbunProvider) Present(ctx context.Context, zone, name, value
 		"name": relative, "type": "TXT", "content": value, "ttl": "600",
 	}
 	var created struct {
-		Status string `json:"status"`
-		ID     any    `json:"id"`
+		Status string    `json:"status"`
+		ID     porkbunID `json:"id"`
 	}
 	if err := jsonRequest(ctx, provider.client, http.MethodPost, provider.baseURL+"/dns/create/"+url.PathEscape(zone), "", body, &created); err != nil {
 		return nil, err
 	}
-	id := fmt.Sprint(created.ID)
-	if created.Status != "SUCCESS" || id == "" || id == "<nil>" {
+	id := string(created.ID)
+	if created.Status != "SUCCESS" || id == "" {
 		return nil, errors.New("Porkbun did not return the created record ID")
 	}
 	return func(cleanupContext context.Context) error {
@@ -229,6 +229,29 @@ func (provider *porkbunProvider) Present(ctx context.Context, zone, name, value
 	}, nil
 }
 
+// porkbunID is a Porkbun record ID. The API documents it as a string but has
+// sent it as a JSON number, which decoding into any would turn into a float
+// such as 1.06926659e+08.
+type porkbunID string
+
+func (id *porkbunID) UnmarshalJSON(data []byte) error {
+	if string(data) == "null" {
+		*id = ""
+		return nil
+	}
+	var number json.Number
+	if err := json.Unmarshal(data, &number); err == nil {
+		*id = porkbunID(number.String())
+		return nil
+	}
+	var text string
+	if err := json.Unmarshal(data, &text); err != nil {
+		return fmt.Errorf("Porkbun record ID: %w", err)
+	}
+	*id = porkbunID(strings.TrimSpace(text))
+	return nil
+}
+
 type godaddyProvider struct {
 	credentials Credentials
 	client      *http.Client

internal/dnsprovider/providers_additional.go

@@ -61,7 +61,11 @@ func (provider *hetznerProvider) Present(ctx context.Context, zone, name, value
 	}
 	recordName := relativeName(name, zone)
 	endpoint := provider.baseURL + "/zones/" + url.PathEscape(zoneID) + "/rrsets/" + url.PathEscape(recordName) + "/TXT/actions/"
-	body := map[string]any{"records": []map[string]string{{"value": value}}}
+	// Hetzner takes record values in zone-file presentation form, so a TXT
+	// value must be a quoted character-string (RFC 1035 section 5.1). ACME
+	// key authorizations are base64url (RFC 8555 section 8.4), well under the
+	// 255-octet limit of one string.
+	body := map[string]any{"records": []map[string]string{{"value": strconv.Quote(value)}}}
 	if err := jsonRequest(ctx, provider.client, http.MethodPost, endpoint+"add_records", "Bearer "+provider.credentials.APIToken, body, nil); err != nil {
 		return nil, err
 	}

internal/dnsprovider/providers_test.go

@@ -138,10 +138,14 @@ func TestHetznerProviderAddsAndRemovesOnlyChallengeValue(t *testing.T) {
 		if request.Header.Get("Authorization") != "Bearer token" {
 			t.Errorf("authorization = %q", request.Header.Get("Authorization"))
 		}
-		return httpResponse(http.StatusCreated, `{}`), nil
+		body, _ := io.ReadAll(request.Body)
+		if got, want := string(body), `{"records":[{"value":"\"challenge-Xy_9\""}]}`; got != want {
+			t.Errorf("body = %s, want %s", got, want)
+		}
+		return httpResponse(http.StatusCreated, `{"action":{"id":1,"status":"running"}}`), nil
 	})}
 	provider := &hetznerProvider{credentials: Credentials{APIToken: "token", ZoneID: "zone-1"}, client: client, baseURL: "https://hetzner.test/v1"}
-	cleanup, err := provider.Present(context.Background(), "example.com", "_acme-challenge.example.com", "challenge")
+	cleanup, err := provider.Present(context.Background(), "example.com", "_acme-challenge.example.com", "challenge-Xy_9")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -332,6 +336,34 @@ func TestGoDaddyEnsureRecordReadsItemsResponse(t *testing.T) {
 	}
 }
 
+func TestPorkbunProviderCleansUpCreatedRecordByID(t *testing.T) {
+	for _, created := range []string{`"106926659"`, `106926659`} {
+		var paths []string
+		client := &http.Client{Transport: roundTripFunc(func(request *http.Request) (*http.Response, error) {
+			paths = append(paths, request.URL.Path)
+			if strings.Contains(request.URL.Path, "/dns/create/") {
+				return httpResponse(http.StatusOK, `{"status":"SUCCESS","id":`+created+`}`), nil
+			}
+			return httpResponse(http.StatusOK, `{"status":"SUCCESS"}`), nil
+		})}
+		provider := &porkbunProvider{credentials: Credentials{APIKey: "key", Secret: "secret"}, client: client, baseURL: "https://porkbun.test/api/json/v3"}
+		cleanup, err := provider.Present(context.Background(), "example.com", "_acme-challenge.example.com", "challenge")
+		if err != nil {
+			t.Fatalf("id %s: %v", created, err)
+		}
+		if err := cleanup(context.Background()); err != nil {
+			t.Fatalf("id %s: %v", created, err)
+		}
+		expected := []string{
+			"/api/json/v3/dns/create/example.com",
+			"/api/json/v3/dns/delete/example.com/106926659",
+		}
+		if strings.Join(paths, "\n") != strings.Join(expected, "\n") {
+			t.Fatalf("id %s: paths = %#v", created, paths)
+		}
+	}
+}
+
 func TestPorkbunEnsureApexRecordOmitsAtLabel(t *testing.T) {
 	var paths []string
 	client := &http.Client{Transport: roundTripFunc(func(request *http.Request) (*http.Response, error) {
@@ -358,7 +390,7 @@ func TestPorkbunEnsureRecordCollapsesDuplicateRecordsByID(t *testing.T) {
 	client := &http.Client{Transport: roundTripFunc(func(request *http.Request) (*http.Response, error) {
 		paths = append(paths, request.URL.Path)
 		if strings.Contains(request.URL.Path, "retrieveByNameType") {
-			return httpResponse(http.StatusOK, `{"status":"SUCCESS","records":[{"id":"10","content":"198.51.100.2","ttl":"600"},{"id":"11","content":"198.51.100.3","ttl":"600"}]}`), nil
+			return httpResponse(http.StatusOK, `{"status":"SUCCESS","records":[{"id":"10","content":"198.51.100.2","ttl":"600"},{"id":11,"content":"198.51.100.3","ttl":"600"}]}`), nil
 		}
 		return httpResponse(http.StatusOK, `{"status":"SUCCESS"}`), nil
 	})}

internal/dnsprovider/records.go

@@ -184,9 +184,9 @@ func (provider *porkbunProvider) EnsureRecord(ctx context.Context, record Record
 		Status  string `json:"status"`
 		Message string `json:"message"`
 		Records []struct {
-			ID      any    `json:"id"`
-			Content string `json:"content"`
-			TTL     any    `json:"ttl"`
+			ID      porkbunID `json:"id"`
+			Content string    `json:"content"`
+			TTL     any       `json:"ttl"`
 		} `json:"records"`
 	}
 	if err := jsonRequest(ctx, provider.client, http.MethodPost, provider.baseURL+lookupPath, "", authentication, &listed); err != nil {
@@ -219,17 +219,17 @@ func (provider *porkbunProvider) EnsureRecord(ctx context.Context, record Record
 		}
 		return true, nil
 	}
-	firstID := fmt.Sprint(listed.Records[0].ID)
-	if firstID == "" || firstID == "<nil>" {
+	firstID := string(listed.Records[0].ID)
+	if firstID == "" {
 		return false, errors.New("Porkbun did not return the existing record ID")
 	}
 	endpoint := provider.baseURL + "/dns/edit/" + url.PathEscape(record.Zone) + "/" + url.PathEscape(firstID)
 	if err := provider.porkbunWrite(ctx, endpoint, body); err != nil {
 		return false, err
 	}
 	for _, duplicate := range listed.Records[1:] {
-		duplicateID := fmt.Sprint(duplicate.ID)
-		if duplicateID == "" || duplicateID == "<nil>" {
+		duplicateID := string(duplicate.ID)
+		if duplicateID == "" {
 			return false, errors.New("Porkbun did not return a duplicate record ID")
 		}
 		deleteEndpoint := provider.baseURL + "/dns/delete/" + url.PathEscape(record.Zone) + "/" + url.PathEscape(duplicateID)