diff --git a/internal/config/config.go b/internal/config/config.go index 4f97544..ba8b259 100644 --- a/internal/config/config.go +++ b/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 diff --git a/internal/config/exclude.go b/internal/config/exclude.go new file mode 100644 index 0000000..ac9a58c --- /dev/null +++ b/internal/config/exclude.go @@ -0,0 +1,107 @@ +package config + +import ( + "errors" + "fmt" + "strings" + + "github.com/gobwas/glob" +) + +var errBadExcludePattern = errors.New("is not a valid exclude pattern") + +// CompileExcludePattern 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 CompileExcludePattern(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 +} + +// 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 := CompileExcludePattern(pattern) + if err != nil { + return fmt.Errorf("%s[%d]: %q %w", setting, i, pattern, err) + } + } + + return nil +} diff --git a/internal/config/exclude_test.go b/internal/config/exclude_test.go new file mode 100644 index 0000000..8a3e0d4 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/internal/snapshot/scanner.go b/internal/snapshot/scanner.go index 9bd36b7..fcd18ba 100644 --- a/internal/snapshot/scanner.go +++ b/internal/snapshot/scanner.go @@ -19,6 +19,7 @@ import ( "github.com/spf13/afero" "sneak.berlin/go/vaultik/internal/blob" "sneak.berlin/go/vaultik/internal/chunker" + "sneak.berlin/go/vaultik/internal/config" "sneak.berlin/go/vaultik/internal/database" "sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/storage" @@ -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 := config.CompileExcludePattern(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