Don't alert on missing -all on included SPF records

pkg/analyzer/dns_spf.go

@@ -33,11 +33,12 @@ import (
 // checkSPFRecords looks up and validates SPF records for a domain, including resolving include: directives
 func (d *DNSAnalyzer) checkSPFRecords(domain string) *[]api.SPFRecord {
 	visited := make(map[string]bool)
-	return d.resolveSPFRecords(domain, visited, 0)
+	return d.resolveSPFRecords(domain, visited, 0, true)
 }
 
 // resolveSPFRecords recursively resolves SPF records including include: directives
-func (d *DNSAnalyzer) resolveSPFRecords(domain string, visited map[string]bool, depth int) *[]api.SPFRecord {
+// isMainRecord indicates if this is the primary domain's record (not an included one)
+func (d *DNSAnalyzer) resolveSPFRecords(domain string, visited map[string]bool, depth int, isMainRecord bool) *[]api.SPFRecord {
 	const maxDepth = 10 // Prevent infinite recursion
 
 	if depth > maxDepth {
@@ -103,7 +104,7 @@ func (d *DNSAnalyzer) resolveSPFRecords(domain string, visited map[string]bool,
 	}
 
 	// Basic validation
-	validationErr := d.validateSPF(spfRecord)
+	validationErr := d.validateSPF(spfRecord, isMainRecord)
 
 	// Extract the "all" mechanism qualifier
 	var allQualifier *api.SPFRecordAllQualifier
@@ -140,7 +141,7 @@ func (d *DNSAnalyzer) resolveSPFRecords(domain string, visited map[string]bool,
 	if redirectDomain != "" {
 		// redirect= replaces the current domain's policy entirely
 		// Only follow if no other mechanisms matched (per RFC 7208)
-		redirectRecords := d.resolveSPFRecords(redirectDomain, visited, depth+1)
+		redirectRecords := d.resolveSPFRecords(redirectDomain, visited, depth+1, false)
 		if redirectRecords != nil {
 			results = append(results, *redirectRecords...)
 		}
@@ -150,7 +151,7 @@ func (d *DNSAnalyzer) resolveSPFRecords(domain string, visited map[string]bool,
 	// Extract and resolve include: directives
 	includes := d.extractSPFIncludes(spfRecord)
 	for _, includeDomain := range includes {
-		includedRecords := d.resolveSPFRecords(includeDomain, visited, depth+1)
+		includedRecords := d.resolveSPFRecords(includeDomain, visited, depth+1, false)
 		if includedRecords != nil {
 			results = append(results, *includedRecords...)
 		}
@@ -236,7 +237,8 @@ func (d *DNSAnalyzer) isValidSPFMechanism(token string) error {
 }
 
 // validateSPF performs basic SPF record validation
-func (d *DNSAnalyzer) validateSPF(record string) error {
+// isMainRecord indicates if this is the primary domain's record (not an included one)
+func (d *DNSAnalyzer) validateSPF(record string, isMainRecord bool) error {
 	// Must start with v=spf1
 	if !strings.HasPrefix(record, "v=spf1") {
 		return fmt.Errorf("SPF record must start with 'v=spf1'")
@@ -269,19 +271,22 @@ func (d *DNSAnalyzer) validateSPF(record string) error {
 		return nil
 	}
 
-	// Check for common syntax issues
+	// Only check for 'all' mechanism on the main record, not on included records
-	// Should have a final mechanism (all, +all, -all, ~all, ?all)
+	if isMainRecord {
-	validEndings := []string{" all", " +all", " -all", " ~all", " ?all"}
+		// Check for common syntax issues
-	hasValidEnding := false
+		// Should have a final mechanism (all, +all, -all, ~all, ?all)
-	for _, ending := range validEndings {
+		validEndings := []string{" all", " +all", " -all", " ~all", " ?all"}
-		if strings.HasSuffix(record, ending) {
+		hasValidEnding := false
-			hasValidEnding = true
+		for _, ending := range validEndings {
-			break
+			if strings.HasSuffix(record, ending) {
+				hasValidEnding = true
+				break
+			}
 		}
-	}
 
-	if !hasValidEnding {
+		if !hasValidEnding {
-		return fmt.Errorf("SPF record should end with an 'all' mechanism (e.g., '-all', '~all') or have a 'redirect=' modifier")
+			return fmt.Errorf("SPF record should end with an 'all' mechanism (e.g., '-all', '~all') or have a 'redirect=' modifier")
+		}
 	}
 
 	return nil

pkg/analyzer/dns_spf_test.go

@@ -128,7 +128,8 @@ func TestValidateSPF(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			err := analyzer.validateSPF(tt.record)
+			// Test as main record (isMainRecord = true) since these tests check overall SPF validity
+			err := analyzer.validateSPF(tt.record, true)
 			if tt.expectError {
 				if err == nil {
 					t.Errorf("validateSPF(%q) expected error but got nil", tt.record)
@@ -144,6 +145,74 @@ func TestValidateSPF(t *testing.T) {
 	}
 }
 
+func TestValidateSPF_IncludedRecords(t *testing.T) {
+	tests := []struct {
+		name         string
+		record       string
+		isMainRecord bool
+		expectError  bool
+		errorMsg     string
+	}{
+		{
+			name:         "Main record without 'all' - should error",
+			record:       "v=spf1 include:_spf.example.com",
+			isMainRecord: true,
+			expectError:  true,
+			errorMsg:     "should end with an 'all' mechanism",
+		},
+		{
+			name:         "Included record without 'all' - should NOT error",
+			record:       "v=spf1 include:_spf.example.com",
+			isMainRecord: false,
+			expectError:  false,
+		},
+		{
+			name:         "Included record with only mechanisms - should NOT error",
+			record:       "v=spf1 ip4:192.0.2.0/24 mx",
+			isMainRecord: false,
+			expectError:  false,
+		},
+		{
+			name:         "Main record with only mechanisms - should error",
+			record:       "v=spf1 ip4:192.0.2.0/24 mx",
+			isMainRecord: true,
+			expectError:  true,
+			errorMsg:     "should end with an 'all' mechanism",
+		},
+		{
+			name:         "Included record with 'all' - valid",
+			record:       "v=spf1 ip4:192.0.2.0/24 -all",
+			isMainRecord: false,
+			expectError:  false,
+		},
+		{
+			name:         "Main record with 'all' - valid",
+			record:       "v=spf1 ip4:192.0.2.0/24 -all",
+			isMainRecord: true,
+			expectError:  false,
+		},
+	}
+
+	analyzer := NewDNSAnalyzer(5 * time.Second)
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := analyzer.validateSPF(tt.record, tt.isMainRecord)
+			if tt.expectError {
+				if err == nil {
+					t.Errorf("validateSPF(%q, isMainRecord=%v) expected error but got nil", tt.record, tt.isMainRecord)
+				} else if tt.errorMsg != "" && !strings.Contains(err.Error(), tt.errorMsg) {
+					t.Errorf("validateSPF(%q, isMainRecord=%v) error = %q, want error containing %q", tt.record, tt.isMainRecord, err.Error(), tt.errorMsg)
+				}
+			} else {
+				if err != nil {
+					t.Errorf("validateSPF(%q, isMainRecord=%v) unexpected error: %v", tt.record, tt.isMainRecord, err)
+				}
+			}
+		})
+	}
+}
+
 func TestExtractSPFRedirect(t *testing.T) {
 	tests := []struct {
 		name             string