Fix error handling and add namespace validation

- Handle ParseSelector errors in comparator.go instead of silently ignoring
- Collect and display selector parse errors in checker.go and build.go
- Use defer for cleanup in nca.go exportClusterState to ensure cleanup on all error paths
- Wrap WriteFile error in init.go writePolicyWithHeader with file path context
- Add namespace name validation (RFC 1123) in pkg/util/validate.go
- Integrate validation in init, scan, and gadget to reject invalid namespaces early

Author: fenio

pkg/build/checker.go

@@ -23,7 +23,8 @@ type SelectorWarning struct {
 
 // CheckResult holds all warnings from selector checking.
 type CheckResult struct {
-	Warnings []SelectorWarning
+	Warnings    []SelectorWarning
+	ParseErrors []string // Selectors that couldn't be parsed
 }
 
 // SelectorChecker validates policy selectors against live cluster state.
@@ -51,6 +52,7 @@ func (sc *SelectorChecker) Check(ctx context.Context, pol *policy.Policy) (*Chec
 		direction string
 	}
 	refs := make(map[dedupeKey]*selectorRef)
+	result := &CheckResult{}
 
 	for _, rule := range pol.Rules {
 		if rule.Action == policy.ActionDeny {
@@ -69,7 +71,12 @@ func (sc *SelectorChecker) Check(ctx context.Context, pol *policy.Policy) (*Chec
 			}
 
 			parsed, err := policy.ParseSelector(dir.raw)
-			if err != nil || parsed == nil {
+			if err != nil {
+				result.ParseErrors = append(result.ParseErrors,
+					fmt.Sprintf("rule %q: invalid %s selector %q: %v", rule.Name, dir.direction, dir.raw, err))
+				continue
+			}
+			if parsed == nil {
 				continue
 			}
 
@@ -104,9 +111,9 @@ func (sc *SelectorChecker) Check(ctx context.Context, pol *policy.Policy) (*Chec
 		namespacesNeeded[ns] = true
 	}
 
-	// If no namespaces to check, return early
+	// If no namespaces to check, return early (but preserve any parse errors)
 	if len(namespacesNeeded) == 0 {
-		return &CheckResult{}, nil
+		return result, nil
 	}
 
 	// Fetch cluster namespaces
@@ -217,7 +224,8 @@ func (sc *SelectorChecker) Check(ctx context.Context, pol *policy.Policy) (*Chec
 		}
 	}
 
-	return &CheckResult{Warnings: warnings}, nil
+	result.Warnings = warnings
+	return result, nil
 }
 
 // fetchNamespaces retrieves all namespace names from the cluster.

pkg/cli/build.go

@@ -94,13 +94,22 @@ func runBuild(cmd *cobra.Command, args []string) error {
 		checkResult, err := checker.Check(ctx, pol)
 		if err != nil {
 			fmt.Fprintf(os.Stderr, "%s Could not check selectors: %s\n\n", colorWarning("⚠"), err)
-		} else if len(checkResult.Warnings) > 0 {
-			fmt.Fprintf(os.Stderr, "%s %d selector warning(s):\n", colorWarning("⚠"), len(checkResult.Warnings))
-			for _, w := range checkResult.Warnings {
-				fmt.Fprintf(os.Stderr, "  %s rule %q %s selector %q: %s\n",
-					colorDim("•"), w.RuleName, w.Direction, w.Selector, w.Reason)
+		} else {
+			if len(checkResult.ParseErrors) > 0 {
+				fmt.Fprintf(os.Stderr, "%s Selector parse errors:\n", colorWarning("⚠"))
+				for _, e := range checkResult.ParseErrors {
+					fmt.Fprintf(os.Stderr, "  %s\n", e)
+				}
+				fmt.Fprintln(os.Stderr)
+			}
+			if len(checkResult.Warnings) > 0 {
+				fmt.Fprintf(os.Stderr, "%s %d selector warning(s):\n", colorWarning("⚠"), len(checkResult.Warnings))
+				for _, w := range checkResult.Warnings {
+					fmt.Fprintf(os.Stderr, "  %s rule %q %s selector %q: %s\n",
+						colorDim("•"), w.RuleName, w.Direction, w.Selector, w.Reason)
+				}
+				fmt.Fprintln(os.Stderr)
 			}
-			fmt.Fprintln(os.Stderr)
 		}
 	}
 

pkg/cli/init.go

@@ -226,7 +226,11 @@ func discoverNamespaces(ctx context.Context, filter string, h *heuristics.Config
 	if filter != "" {
 		wanted := make(map[string]bool)
 		for _, ns := range strings.Split(filter, ",") {
-			wanted[strings.TrimSpace(ns)] = true
+			ns = strings.TrimSpace(ns)
+			if err := util.ValidateNamespaceName(ns); err != nil {
+				return nil, fmt.Errorf("invalid namespace in filter: %w", err)
+			}
+			wanted[ns] = true
 		}
 		var filtered []string
 		for _, ns := range all {
@@ -906,5 +910,8 @@ func writePolicyWithHeader(pol *policy.Policy, path string) error {
 	header += "#\n"
 	header += "# Run: netalchemy verify -p policy.yaml\n\n"
 
-	return os.WriteFile(path, []byte(header+string(yamlData)), 0600)
+	if err := os.WriteFile(path, []byte(header+string(yamlData)), 0600); err != nil {
+		return fmt.Errorf("failed to write policy to %s: %w", path, err)
+	}
+	return nil
 }

pkg/cli/scan.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/fenio/netalchemy/pkg/policy"
 	"github.com/fenio/netalchemy/pkg/scan"
+	"github.com/fenio/netalchemy/pkg/util"
 	"github.com/spf13/cobra"
 )
 
@@ -109,8 +110,12 @@ func runScan(cmd *cobra.Command, args []string) error {
 	}
 
 	// Convert trace to rules
+	sourceNamespaces, err := splitNamespaces(scanNamespaces)
+	if err != nil {
+		return fmt.Errorf("invalid namespace: %w", err)
+	}
 	converter := &scan.TraceConverter{
-		SourceNamespaces: splitNamespaces(scanNamespaces),
+		SourceNamespaces: sourceNamespaces,
 	}
 
 	result, err := converter.ConvertTraceWithResult(trace)
@@ -186,16 +191,20 @@ func validateScanDuration(s string) (time.Duration, error) {
 	return d, nil
 }
 
-func splitNamespaces(ns string) []string {
+func splitNamespaces(ns string) ([]string, error) {
 	if ns == "" {
-		return nil
+		return nil, nil
 	}
 	var result []string
 	for _, n := range strings.Split(ns, ",") {
 		trimmed := strings.TrimSpace(n)
-		if trimmed != "" {
-			result = append(result, trimmed)
+		if trimmed == "" {
+			continue
+		}
+		if err := util.ValidateNamespaceName(trimmed); err != nil {
+			return nil, err
 		}
+		result = append(result, trimmed)
 	}
-	return result
+	return result, nil
 }

pkg/cli/scan_test.go

@@ -9,9 +9,10 @@ import (
 
 func TestSplitNamespaces(t *testing.T) {
 	tests := []struct {
-		name  string
-		input string
-		want  []string
+		name    string
+		input   string
+		want    []string
+		wantErr bool
 	}{
 		{
 			name:  "empty string",
@@ -43,12 +44,26 @@ func TestSplitNamespaces(t *testing.T) {
 			input: ",,",
 			want:  nil,
 		},
+		{
+			name:    "invalid namespace uppercase",
+			input:   "Default",
+			wantErr: true,
+		},
+		{
+			name:    "invalid namespace special chars",
+			input:   "ns;echo pwned",
+			wantErr: true,
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := splitNamespaces(tt.input)
-			if !reflect.DeepEqual(got, tt.want) {
+			got, err := splitNamespaces(tt.input)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("splitNamespaces(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr)
+				return
+			}
+			if !tt.wantErr && !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("splitNamespaces(%q) = %v, want %v", tt.input, got, tt.want)
 			}
 		})

pkg/scan/gadget.go

@@ -145,6 +145,19 @@ func (r *GadgetRunner) Run() (*GadgetResult, error) {
 
 // RunWithContext executes kubectl gadget with context support for cancellation.
 func (r *GadgetRunner) RunWithContext(ctx context.Context) (*GadgetResult, error) {
+	// Validate namespace names early
+	if r.Namespaces != "" {
+		for _, ns := range strings.Split(r.Namespaces, ",") {
+			ns = strings.TrimSpace(ns)
+			if ns == "" {
+				continue
+			}
+			if err := util.ValidateNamespaceName(ns); err != nil {
+				return nil, fmt.Errorf("invalid namespace: %w", err)
+			}
+		}
+	}
+
 	if err := r.checkInstalled(ctx); err != nil {
 		return nil, err
 	}

pkg/util/validate.go

@@ -0,0 +1,24 @@
+package util
+
+import (
+	"fmt"
+	"regexp"
+)
+
+// RFC 1123 DNS subdomain: lowercase alphanumeric, may contain hyphens,
+// must start and end with alphanumeric, max 63 chars
+var namespaceRegex = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`)
+
+// ValidateNamespaceName checks if a namespace name is valid per RFC 1123.
+func ValidateNamespaceName(name string) error {
+	if len(name) == 0 {
+		return fmt.Errorf("namespace name cannot be empty")
+	}
+	if len(name) > 63 {
+		return fmt.Errorf("namespace name %q exceeds 63 characters", name)
+	}
+	if !namespaceRegex.MatchString(name) {
+		return fmt.Errorf("namespace name %q is invalid: must be lowercase alphanumeric with optional hyphens, starting and ending with alphanumeric", name)
+	}
+	return nil
+}

pkg/util/validate_test.go

@@ -0,0 +1,39 @@
+package util
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestValidateNamespaceName(t *testing.T) {
+	tests := []struct {
+		name    string
+		input   string
+		wantErr bool
+	}{
+		{"valid simple", "default", false},
+		{"valid with hyphen", "kube-system", false},
+		{"valid with numbers", "app123", false},
+		{"valid single char", "a", false},
+		{"valid starts with number", "123app", false},
+		{"empty", "", true},
+		{"starts with hyphen", "-invalid", true},
+		{"ends with hyphen", "invalid-", true},
+		{"uppercase", "Default", true},
+		{"spaces", "my namespace", true},
+		{"special chars underscore", "my_namespace", true},
+		{"special chars dot", "my.namespace", true},
+		{"special chars semicolon", "ns;echo", true},
+		{"too long", strings.Repeat("a", 64), true},
+		{"max length valid", strings.Repeat("a", 63), false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidateNamespaceName(tt.input)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ValidateNamespaceName(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr)
+			}
+		})
+	}
+}

pkg/verify/comparator.go

@@ -125,8 +125,16 @@ func (c *Comparator) findMatchingRule(details string) string {
 // ruleMatchesConnection checks if a rule could have produced the given connection
 func (c *Comparator) ruleMatchesConnection(rule policy.Rule, details string) bool {
 	// Parse selectors from the rule
-	fromSelector, _ := policy.ParseSelector(rule.From)
-	toSelector, _ := policy.ParseSelector(rule.To)
+	fromSelector, fromErr := policy.ParseSelector(rule.From)
+	toSelector, toErr := policy.ParseSelector(rule.To)
+
+	// If selectors can't be parsed, we can't match this rule
+	if rule.From != "" && fromErr != nil {
+		return false
+	}
+	if rule.To != "" && toErr != nil {
+		return false
+	}
 
 	// Check if the "from" selector matches the source in details
 	if fromSelector != nil {

pkg/verify/nca.go

@@ -251,6 +251,14 @@ func (r *NCARunner) exportClusterState(ctx context.Context) (string, error) {
 		return "", fmt.Errorf("failed to create temp directory: %w", err)
 	}
 
+	// Cleanup on failure; caller is responsible for cleanup on success
+	success := false
+	defer func() {
+		if !success {
+			os.RemoveAll(clusterDir) // Best-effort cleanup
+		}
+	}()
+
 	resources := []string{"pods", "namespaces", "services", "networkpolicies"}
 	kubectlArgs := []string{"get", strings.Join(resources, ","), "-A", "-o", "yaml"}
 
@@ -261,16 +269,15 @@ func (r *NCARunner) exportClusterState(ctx context.Context) (string, error) {
 
 	result, err := util.RunCommandWithRetry(ctx, "kubectl", kubectlArgs, opts, util.DefaultRetryConfig())
 	if err != nil {
-		os.RemoveAll(clusterDir)
 		return "", fmt.Errorf("kubectl get failed: %w", err)
 	}
 
 	outPath := filepath.Join(clusterDir, "cluster-state.yaml")
 	if err := os.WriteFile(outPath, []byte(result.Stdout), 0600); err != nil {
-		os.RemoveAll(clusterDir)
 		return "", fmt.Errorf("failed to write cluster state: %w", err)
 	}
 
+	success = true
 	return clusterDir, nil
 }