Refuse an exclude pattern that does not compile at config load (closes #455)

An exclude pattern that did not compile was skipped with a warning during the backup, which then stored what the user meant to leave out. Config.Validate now refuses it, naming the setting and quoting the pattern, for example exclude[2]: "build[" is not a valid exclude pattern: ...

The compile step moves to a new package, internal/exclude, as exclude.Compile; Validate and the scanner both call it. The scanner's skip-and-warn path is gone; a pattern that does not compile there now fails the scanner.

Model: opus-5-5

internal/config/config.go

@@ -396,6 +396,8 @@ func Parse(data []byte) (*Config, error) {
 //     never the value. An empty list is accepted, because only snapshot
 //     create needs a recipient and it checks for one itself
 //   - At least one snapshot must be configured with at least one path
+//   - Every exclude pattern, global or in a snapshot, must compile; errors
+//     name the setting and quote the pattern
 //   - Storage must be configured (either storage_url or s3.* fields)
 //   - The index path must be absolute: a relative one would resolve against
 //     the working directory, so each directory would get its own index
@@ -416,18 +418,18 @@ func (c *Config) Validate() error {
 		}
 	}
 
-	if len(c.Snapshots) == 0 {
-		return errNoSnapshots
+	err := c.validateSnapshots()
+	if err != nil {
+		return err
 	}
 
-	for name, snap := range c.Snapshots {
-		if len(snap.Paths) == 0 {
-			return fmt.Errorf("%w: %q", errSnapshotNoPaths, name)
-		}
+	err = c.validateExcludes()
+	if err != nil {
+		return err
 	}
 
 	// Validate storage configuration
-	err := c.validateStorage()
+	err = c.validateStorage()
 	if err != nil {
 		return err
 	}
@@ -466,6 +468,22 @@ func (c *Config) Validate() error {
 	return nil
 }
 
+// validateSnapshots refuses a config with no snapshot or with a snapshot
+// that has no path.
+func (c *Config) validateSnapshots() error {
+	if len(c.Snapshots) == 0 {
+		return errNoSnapshots
+	}
+
+	for name, snap := range c.Snapshots {
+		if len(snap.Paths) == 0 {
+			return fmt.Errorf("%w: %q", errSnapshotNoPaths, name)
+		}
+	}
+
+	return nil
+}
+
 // validateAgeRecipient parses one age_recipients entry with the age library
 // and returns a value-free error on failure. A recipient string can be
 // sensitive (an operator may paste a secret key by mistake), so neither the

internal/config/exclude.go

@@ -0,0 +1,39 @@
+package config
+
+import (
+	"fmt"
+
+	"sneak.berlin/go/vaultik/internal/exclude"
+)
+
+// validateExcludes refuses an exclude pattern, global or in a snapshot,
+// that does not compile, naming the setting and quoting the pattern.
+func (c *Config) validateExcludes() error {
+	err := validateExcludeList("exclude", c.Exclude)
+	if err != nil {
+		return err
+	}
+
+	for _, name := range c.SnapshotNames() {
+		err = validateExcludeList("snapshots."+name+".exclude",
+			c.Snapshots[name].Exclude)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// validateExcludeList refuses the first pattern in patterns that does not
+// compile. setting is the name of the list, as in "exclude".
+func validateExcludeList(setting string, patterns []string) error {
+	for i, pattern := range patterns {
+		_, _, err := exclude.Compile(pattern)
+		if err != nil {
+			return fmt.Errorf("%s[%d]: %q %w", setting, i, pattern, err)
+		}
+	}
+
+	return nil
+}

internal/config/exclude_test.go

@@ -0,0 +1,65 @@
+package config_test
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"sneak.berlin/go/vaultik/internal/config"
+)
+
+// TestLoadRefusesBadExcludePattern checks that an exclude pattern that does
+// not compile, global or in a snapshot, fails at load with the setting and
+// the pattern named, rather than the backup storing what it meant to leave
+// out.
+func TestLoadRefusesBadExcludePattern(t *testing.T) {
+	t.Parallel()
+
+	const storage = "storage_url: file:///tmp/vaultik-test-storage\n"
+
+	tests := []struct {
+		name    string
+		config  string
+		wantErr string
+	}{
+		{
+			name: "global",
+			config: "exclude: ['*.tmp', '.cache', 'build[']\n" +
+				"snapshots:\n" +
+				"  home:\n" +
+				"    paths: [/tmp/vaultik-test-source]\n" + storage,
+			wantErr: `exclude[2]: "build[" is not a valid exclude pattern`,
+		},
+		{
+			name: "snapshot",
+			config: "snapshots:\n" +
+				"  home:\n" +
+				"    paths: [/tmp/vaultik-test-source]\n" +
+				"    exclude: ['/a/**/x[']\n" + storage,
+			wantErr: `snapshots.home.exclude[0]: "/a/**/x[" is not a valid exclude pattern`,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			path := filepath.Join(t.TempDir(), "config.yml")
+
+			err := os.WriteFile(path, []byte(tt.config), 0o600)
+			if err != nil {
+				t.Fatalf("write config: %v", err)
+			}
+
+			_, err = config.Load(path)
+			if err == nil {
+				t.Fatalf("Load() returned nil, want an error containing %q", tt.wantErr)
+			}
+
+			if !strings.Contains(err.Error(), tt.wantErr) {
+				t.Fatalf("Load() error = %q, want it to contain %q", err, tt.wantErr)
+			}
+		})
+	}
+}

internal/exclude/exclude.go

@@ -0,0 +1,76 @@
+// Package exclude compiles exclude patterns into globs.
+package exclude
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+
+	"github.com/gobwas/glob"
+)
+
+var errBadExcludePattern = errors.New("is not a valid exclude pattern")
+
+// Compile compiles one exclude pattern into the globs that match it.
+// anchored reports whether the pattern matches only from the root of the
+// source directory. An empty pattern yields no globs. Config.Validate and
+// the scanner both compile patterns with it, so a pattern that loads is one
+// the scanner can use.
+func Compile(pattern string) ([]glob.Glob, bool, error) {
+	if pattern == "" {
+		return nil, false, nil
+	}
+
+	// A leading "/" anchors the pattern to the source directory.
+	trimmed, anchored := strings.CutPrefix(pattern, "/")
+
+	// A trailing "/" marks a directory and is not matched.
+	trimmed = strings.TrimSuffix(trimmed, "/")
+
+	// A leading "**/" demands a "/" before the name and so misses an
+	// entry directly under the source path. It means "at every depth",
+	// which is what an unanchored pattern already does, so drop the
+	// prefix and match unanchored: as in gitignore, "**/x" and
+	// "/**/x" then also match a top-level "x".
+	if rest, found := strings.CutPrefix(trimmed, "**/"); found {
+		trimmed = rest
+		anchored = false
+	}
+
+	// Likewise "/**/" inside a pattern demands two slashes, so
+	// "a/**/*.log" misses "a/x.log". As in gitignore it stands for
+	// zero or more directories, so also compile the pattern with it
+	// made a single "/". The glob library's {a,b} is no help: with a
+	// "**" inside it in mid-pattern, it misses paths it should match.
+	var globs []glob.Glob
+
+	for _, variant := range doubleStarVariants(trimmed) {
+		g, err := glob.Compile(variant, '/')
+		if err != nil {
+			return nil, false, fmt.Errorf("%w: %w", errBadExcludePattern, err)
+		}
+
+		globs = append(globs, g)
+	}
+
+	return globs, anchored, nil
+}
+
+// doubleStarVariants returns pattern once for every way of keeping each
+// "/**/" in it or making it a single "/".
+func doubleStarVariants(pattern string) []string {
+	const doubleStar = "/**/"
+
+	before, after, found := strings.Cut(pattern, doubleStar)
+	if !found {
+		return []string{pattern}
+	}
+
+	var variants []string
+
+	for _, rest := range doubleStarVariants(after) {
+		variants = append(variants, before+doubleStar+rest, before+"/"+rest)
+	}
+
+	return variants
+}

internal/snapshot/scanner.go

@@ -20,6 +20,7 @@ import (
 	"sneak.berlin/go/vaultik/internal/blob"
 	"sneak.berlin/go/vaultik/internal/chunker"
 	"sneak.berlin/go/vaultik/internal/database"
+	"sneak.berlin/go/vaultik/internal/exclude"
 	"sneak.berlin/go/vaultik/internal/log"
 	"sneak.berlin/go/vaultik/internal/storage"
 	"sneak.berlin/go/vaultik/internal/types"
@@ -221,8 +222,12 @@ func NewScanner(cfg ScannerConfig) *Scanner {
 		progress = NewProgressReporter()
 	}
 
-	// Compile exclude patterns
-	compiledExclude := compileExcludePatterns(cfg.Exclude)
+	compiledExclude, err := compileExcludePatterns(cfg.Exclude)
+	if err != nil {
+		log.Error("Failed to compile exclude patterns", "error", err)
+
+		return nil
+	}
 
 	uiw := cfg.UI
 	if uiw == nil {
@@ -2318,51 +2323,18 @@ func wrapPermissionError(path string, err error) error {
 		"with --skip-errors to continue past unreadable files)", path, err)
 }
 
-// compileExcludePatterns compiles the exclude patterns into glob matchers
-func compileExcludePatterns(patterns []string) []compiledPattern {
+// compileExcludePatterns compiles the exclude patterns into glob matchers.
+// Config.Validate has already refused a pattern that does not compile.
+func compileExcludePatterns(patterns []string) ([]compiledPattern, error) {
 	var compiled []compiledPattern
 
 	for _, p := range patterns {
-		if p == "" {
-			continue
+		globs, anchored, err := exclude.Compile(p)
+		if err != nil {
+			return nil, fmt.Errorf("%q %w", p, err)
 		}
 
-		// Check if pattern is anchored (starts with /)
-		anchored := strings.HasPrefix(p, "/")
-
-		pattern := p
-		if anchored {
-			pattern = p[1:] // Remove leading /
-		}
-
-		// Remove trailing slash if present (directory indicator)
-		pattern = strings.TrimSuffix(pattern, "/")
-
-		// A leading "**/" demands a "/" before the name and so misses an
-		// entry directly under the source path. It means "at every depth",
-		// which is what an unanchored pattern already does, so drop the
-		// prefix and match unanchored: as in gitignore, "**/x" and
-		// "/**/x" then also match a top-level "x".
-		if rest, found := strings.CutPrefix(pattern, "**/"); found {
-			pattern = rest
-			anchored = false
-		}
-
-		// Likewise "/**/" inside a pattern demands two slashes, so
-		// "a/**/*.log" misses "a/x.log". As in gitignore it stands for
-		// zero or more directories, so also compile the pattern with it
-		// made a single "/". The glob library's {a,b} is no help: with a
-		// "**" inside it in mid-pattern, it misses paths it should match.
-		for _, variant := range doubleStarVariants(pattern) {
-			// For patterns without path separators, we need to match them as components
-			// e.g., ".git" should match ".git" anywhere in the path
-			g, err := glob.Compile(variant, '/')
-			if err != nil {
-				log.Warn("Invalid exclude pattern, skipping", "pattern", p, "error", err)
-
-				break
-			}
-
+		for _, g := range globs {
 			compiled = append(compiled, compiledPattern{
 				pattern:  g,
 				anchored: anchored,
@@ -2371,26 +2343,7 @@ func compileExcludePatterns(patterns []string) []compiledPattern {
 		}
 	}
 
-	return compiled
-}
-
-// doubleStarVariants returns pattern once for every way of keeping each
-// "/**/" in it or making it a single "/".
-func doubleStarVariants(pattern string) []string {
-	const doubleStar = "/**/"
-
-	before, after, found := strings.Cut(pattern, doubleStar)
-	if !found {
-		return []string{pattern}
-	}
-
-	var variants []string
-
-	for _, rest := range doubleStarVariants(after) {
-		variants = append(variants, before+doubleStar+rest, before+"/"+rest)
-	}
-
-	return variants
+	return compiled, nil
 }
 
 // shouldExclude checks if a path should be excluded based on exclude patterns