httpcaddyfile: Validates TLS DNS challenge options (#7099)

* httpcaddyfile: Validates TLS DNS challenge options

Adds validation to the TLS Caddyfile adapter to ensure that when DNS challenge options (such as propagation_delay or dns_ttl) are specified, a DNS provider is also configured.

Adds new integration tests to verify this validation logic, and implements a new mechanism for adapt tests to assert a config adapt error.

* Add some more AI-generated tests asserting config errors

* Parallel doesn't work here, we use global variables

* Windows fix

caddyconfig/caddyfile/lexer.go

@@ -323,7 +323,8 @@ func (l *lexer) finalizeHeredoc(val []rune, marker string) ([]rune, error) {
 
 		// if the padding doesn't match exactly at the start then we can't safely strip
 		if index != 0 {
-			return nil, fmt.Errorf("mismatched leading whitespace in heredoc <<%s on line #%d [%s], expected whitespace [%s] to match the closing marker", marker, l.line+lineNum+1, lineText, paddingToStrip)
+			cleanLineText := strings.TrimRight(lineText, "\r\n")
+			return nil, fmt.Errorf("mismatched leading whitespace in heredoc <<%s on line #%d [%s], expected whitespace [%s] to match the closing marker", marker, l.line+lineNum+1, cleanLineText, paddingToStrip)
 		}
 
 		// strip, then append the line, with the newline, to the output.

caddyconfig/httpcaddyfile/builtins.go

@@ -130,6 +130,9 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 	var reusePrivateKeys bool
 	var forceAutomate bool
 
+	// Track which DNS challenge options are set
+	var dnsOptionsSet []string
+
 	firstLine := h.RemainingArgs()
 	switch len(firstLine) {
 	case 0:
@@ -350,6 +353,7 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 			if acmeIssuer.Challenges.DNS == nil {
 				acmeIssuer.Challenges.DNS = new(caddytls.DNSChallengeConfig)
 			}
+			dnsOptionsSet = append(dnsOptionsSet, "resolvers")
 			acmeIssuer.Challenges.DNS.Resolvers = args
 
 		case "propagation_delay":
@@ -371,6 +375,7 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 			if acmeIssuer.Challenges.DNS == nil {
 				acmeIssuer.Challenges.DNS = new(caddytls.DNSChallengeConfig)
 			}
+			dnsOptionsSet = append(dnsOptionsSet, "propagation_delay")
 			acmeIssuer.Challenges.DNS.PropagationDelay = caddy.Duration(delay)
 
 		case "propagation_timeout":
@@ -398,6 +403,7 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 			if acmeIssuer.Challenges.DNS == nil {
 				acmeIssuer.Challenges.DNS = new(caddytls.DNSChallengeConfig)
 			}
+			dnsOptionsSet = append(dnsOptionsSet, "propagation_timeout")
 			acmeIssuer.Challenges.DNS.PropagationTimeout = caddy.Duration(timeout)
 
 		case "dns_ttl":
@@ -419,6 +425,7 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 			if acmeIssuer.Challenges.DNS == nil {
 				acmeIssuer.Challenges.DNS = new(caddytls.DNSChallengeConfig)
 			}
+			dnsOptionsSet = append(dnsOptionsSet, "dns_ttl")
 			acmeIssuer.Challenges.DNS.TTL = caddy.Duration(ttl)
 
 		case "dns_challenge_override_domain":
@@ -435,6 +442,7 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 			if acmeIssuer.Challenges.DNS == nil {
 				acmeIssuer.Challenges.DNS = new(caddytls.DNSChallengeConfig)
 			}
+			dnsOptionsSet = append(dnsOptionsSet, "dns_challenge_override_domain")
 			acmeIssuer.Challenges.DNS.OverrideDomain = arg[0]
 
 		case "ca_root":
@@ -470,6 +478,18 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
 		}
 	}
 
+	// Validate DNS challenge config: any DNS challenge option except "dns" requires a DNS provider
+	if acmeIssuer != nil && acmeIssuer.Challenges != nil && acmeIssuer.Challenges.DNS != nil {
+		dnsCfg := acmeIssuer.Challenges.DNS
+		providerSet := dnsCfg.ProviderRaw != nil || h.Option("dns") != nil
+		if len(dnsOptionsSet) > 0 && !providerSet {
+			return nil, h.Errf(
+				"setting DNS challenge options [%s] requires a DNS provider (set with the 'dns' subdirective or 'acme_dns' global option)",
+				strings.Join(dnsOptionsSet, ", "),
+			)
+		}
+	}
+
 	// a naked tls directive is not allowed
 	if len(firstLine) == 0 && !hasBlock {
 		return nil, h.ArgErr()

caddytest/integration/caddyfile_adapt/ambiguous_site_definition.caddyfiletest

@@ -0,0 +1,12 @@
+example.com
+handle {
+	respond "one"
+}
+
+example.com
+handle {
+	respond "two"
+}
+----------
+Caddyfile:6: unrecognized directive: example.com
+Did you mean to define a second site? If so, you must use curly braces around each site to separate their configurations.

caddytest/integration/caddyfile_adapt/ambiguous_site_definition_duplicate_key.caddyfiletest

@@ -0,0 +1,9 @@
+:8080 {
+	respond "one"
+}
+
+:8080 {
+	respond "two"
+}
+----------
+ambiguous site definition: :8080

caddytest/integration/caddyfile_adapt/directive_as_site_address.caddyfiletest

@@ -0,0 +1,5 @@
+handle
+
+respond "should not work"
+----------
+Caddyfile:1: parsed 'handle' as a site address, but it is a known directive; directives must appear in a site block

caddytest/integration/caddyfile_adapt/duplicate_listener_address_global.caddyfiletest

@@ -0,0 +1,12 @@
+{
+	servers {
+		srv0 {
+			listen :8080
+		}
+		srv1 {
+			listen :8080
+		}
+	}
+}
+----------
+parsing caddyfile tokens for 'servers': unrecognized servers option 'srv0', at Caddyfile:3

caddytest/integration/caddyfile_adapt/heredoc_extra_indentation.caddyfiletest

@@ -0,0 +1,41 @@
+:80
+
+handle {
+	respond <<END
+        line1
+        line2
+  END
+}
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":80"
+					],
+					"routes": [
+						{
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"handle": [
+												{
+													"body": "      line1\n      line2",
+													"handler": "static_response"
+												}
+											]
+										}
+									]
+								}
+							]
+						}
+					]
+				}
+			}
+		}
+	}
+}

caddytest/integration/caddyfile_adapt/heredoc_incomplete.caddyfiletest

@@ -0,0 +1,9 @@
+:80
+
+handle {
+    respond <<EOF
+    Hello
+# missing EOF marker
+}
+----------
+mismatched leading whitespace in heredoc <<EOF on line #5 [    Hello], expected whitespace [# missing ] to match the closing marker

caddytest/integration/caddyfile_adapt/heredoc_invalid_marker.caddyfiletest

@@ -0,0 +1,9 @@
+:80
+
+handle {
+    respond <<END!
+    Hello
+    END!
+}
+----------
+heredoc marker on line #4 must contain only alpha-numeric characters, dashes and underscores; got 'END!'

caddytest/integration/caddyfile_adapt/heredoc_mismatched_whitespace.caddyfiletest

@@ -0,0 +1,10 @@
+:80
+
+handle {
+	respond <<END
+	line1
+	line2
+  END
+}
+----------
+mismatched leading whitespace in heredoc <<END on line #5 [	line1], expected whitespace [  ] to match the closing marker

caddytest/integration/caddyfile_adapt/heredoc_missing_marker.caddyfiletest

@@ -0,0 +1,9 @@
+:80
+
+handle {
+    respond << 
+    Hello
+    END
+}
+----------
+parsing caddyfile tokens for 'handle': unrecognized directive: Hello - are you sure your Caddyfile structure (nesting and braces) is correct?, at Caddyfile:7

caddytest/integration/caddyfile_adapt/heredoc_too_many_angle_brackets.caddyfiletest

@@ -0,0 +1,9 @@
+:80
+
+handle {
+    respond <<<END
+    Hello
+    END
+}
+----------
+too many '<' for heredoc on line #4; only use two, for example <<END

caddytest/integration/caddyfile_adapt/import_cycle.caddyfiletest

@@ -0,0 +1,12 @@
+(import1) {
+	import import2
+}
+
+(import2) {
+	import import1
+}
+
+import import1
+
+----------
+a cycle of imports exists between Caddyfile:import2 and Caddyfile:import1

caddytest/integration/caddyfile_adapt/invoke_undefined_named_route.caddyfiletest

@@ -0,0 +1,5 @@
+example.com {
+	invoke foo
+}
+----------
+cannot invoke named route 'foo', which was not defined

caddytest/integration/caddyfile_adapt/matcher_outside_site_block.caddyfiletest

@@ -0,0 +1,9 @@
+@foo {
+	path /foo
+}
+
+handle {
+	respond "should not work"
+}
+----------
+request matchers may not be defined globally, they must be in a site block; found @foo, at Caddyfile:1

caddytest/integration/caddyfile_adapt/site_address_invalid_port.caddyfiletest

@@ -0,0 +1,7 @@
+:70000
+
+handle {
+	respond "should not work"
+}
+----------
+port 70000 is out of range

caddytest/integration/caddyfile_adapt/site_address_negative_port.caddyfiletest

@@ -0,0 +1,7 @@
+:-1
+
+handle {
+	respond "should not work"
+}
+----------
+port -1 is out of range

caddytest/integration/caddyfile_adapt/site_address_unsupported_scheme.caddyfiletest

@@ -0,0 +1,7 @@
+foo://example.com
+
+handle {
+	respond "hello"
+}
+----------
+unsupported URL scheme foo://

caddytest/integration/caddyfile_adapt/site_address_wss_invalid_port.caddyfiletest

@@ -0,0 +1,7 @@
+wss://example.com:70000
+
+handle {
+	respond "should not work"
+}
+----------
+port 70000 is out of range

caddytest/integration/caddyfile_adapt/site_address_wss_scheme.caddyfiletest

@@ -0,0 +1,7 @@
+wss://example.com
+
+handle {
+	respond "hello"
+}
+----------
+the scheme wss:// is only supported in browsers; use https:// instead

caddytest/integration/caddyfile_adapt/tls_dns_multiple_options_without_provider.caddyfiletest

@@ -0,0 +1,9 @@
+localhost
+
+tls {
+	propagation_delay 10s
+	dns_ttl 5m
+}
+
+----------
+parsing caddyfile tokens for 'tls': setting DNS challenge options [propagation_delay, dns_ttl] requires a DNS provider (set with the 'dns' subdirective or 'acme_dns' global option), at Caddyfile:6

caddytest/integration/caddyfile_adapt/tls_dns_propagation_timeout_without_provider.caddyfiletest

@@ -0,0 +1,7 @@
+:443 {
+	tls {
+		propagation_timeout 30s
+	}
+}
+----------
+parsing caddyfile tokens for 'tls': setting DNS challenge options [propagation_timeout] requires a DNS provider (set with the 'dns' subdirective or 'acme_dns' global option), at Caddyfile:4

caddytest/integration/caddyfile_adapt/tls_dns_propagation_without_provider.caddyfiletest

@@ -0,0 +1,7 @@
+:443 {
+	tls {
+		propagation_delay 30s
+	}
+}
+----------
+parsing caddyfile tokens for 'tls': setting DNS challenge options [propagation_delay] requires a DNS provider (set with the 'dns' subdirective or 'acme_dns' global option), at Caddyfile:4

caddytest/integration/caddyfile_adapt/tls_dns_ttl.caddyfiletest

@@ -2,6 +2,7 @@ localhost
 
 respond "hello from localhost"
 tls {
+	dns mock
 	dns_ttl 5m10s
 }
 ----------
@@ -54,6 +55,9 @@ tls {
 							{
 								"challenges": {
 									"dns": {
+										"provider": {
+											"name": "mock"
+										},
 										"ttl": 310000000000
 									}
 								},

caddytest/integration/caddyfile_adapt/tls_propagation_options.caddyfiletest

@@ -2,6 +2,7 @@ localhost
 
 respond "hello from localhost"
 tls {
+	dns mock
 	propagation_delay 5m10s
 	propagation_timeout 10m20s
 }
@@ -56,7 +57,10 @@ tls {
 								"challenges": {
 									"dns": {
 										"propagation_delay": 310000000000,
-										"propagation_timeout": 620000000000
+										"propagation_timeout": 620000000000,
+										"provider": {
+											"name": "mock"
+										}
 									}
 								},
 								"module": "acme"

caddytest/integration/caddyfile_adapt_test.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/caddyserver/caddy/v2/caddyconfig"
 	"github.com/caddyserver/caddy/v2/caddytest"
 	_ "github.com/caddyserver/caddy/v2/internal/testmocks"
 )
@@ -27,9 +28,11 @@ func TestCaddyfileAdaptToJSON(t *testing.T) {
 		if f.IsDir() {
 			continue
 		}
-
-		// read the test file
 		filename := f.Name()
+
+		// run each file as a subtest, so that we can see which one fails more easily
+		t.Run(filename, func(t *testing.T) {
+			// read the test file
 			data, err := os.ReadFile("./caddyfile_adapt/" + filename)
 			if err != nil {
 				t.Errorf("failed to read %s dir: %s", filename, err)
@@ -38,19 +41,35 @@ func TestCaddyfileAdaptToJSON(t *testing.T) {
 			// split the Caddyfile (first) and JSON (second) parts
 			// (append newline to Caddyfile to match formatter expectations)
 			parts := strings.Split(string(data), "----------")
-		caddyfile, json := strings.TrimSpace(parts[0])+"\n", strings.TrimSpace(parts[1])
+			caddyfile, expected := strings.TrimSpace(parts[0])+"\n", strings.TrimSpace(parts[1])
 
 			// replace windows newlines in the json with unix newlines
-		json = winNewlines.ReplaceAllString(json, "\n")
+			expected = winNewlines.ReplaceAllString(expected, "\n")
 
 			// replace os-specific default path for file_server's hide field
 			replacePath, _ := jsonMod.Marshal(fmt.Sprint(".", string(filepath.Separator), "Caddyfile"))
-		json = strings.ReplaceAll(json, `"./Caddyfile"`, string(replacePath))
+			expected = strings.ReplaceAll(expected, `"./Caddyfile"`, string(replacePath))
 
-		// run the test
-		ok := caddytest.CompareAdapt(t, filename, caddyfile, "caddyfile", json)
+			// if the expected output is JSON, compare it
+			if len(expected) > 0 && expected[0] == '{' {
+				ok := caddytest.CompareAdapt(t, filename, caddyfile, "caddyfile", expected)
 				if !ok {
 					t.Errorf("failed to adapt %s", filename)
 				}
+				return
+			}
+
+			// otherwise, adapt the Caddyfile and check for errors
+			cfgAdapter := caddyconfig.GetAdapter("caddyfile")
+			_, _, err = cfgAdapter.Adapt([]byte(caddyfile), nil)
+			if err == nil {
+				t.Errorf("expected error for %s but got none", filename)
+			} else {
+				normalizedErr := winNewlines.ReplaceAllString(err.Error(), "\n")
+				if !strings.Contains(normalizedErr, expected) {
+					t.Errorf("expected error for %s to contain:\n%s\nbut got:\n%s", filename, expected, normalizedErr)
+				}
+			}
+		})
 	}
 }