Merge pull request #3734 from lbausch/validate-patterns

Validate exclude patterns

changelog/unreleased/issue-3709

@@ -0,0 +1,9 @@
+Enhancement: Validate exclude patterns before backing up
+
+Exclude patterns provided via `--exclude`, `--iexclude`, `--exclude-file` or `--iexclude-file`
+previously weren't validated. As a consequence, invalid patterns resulted in all files being backed up.
+restic now validates all patterns before running the backup and aborts with a fatal error
+if an invalid pattern is detected.
+
+https://github.com/restic/restic/issues/3709
+https://github.com/restic/restic/pull/3734

cmd/restic/cmd_backup.go

@@ -20,6 +20,7 @@ import (
 	"github.com/restic/restic/internal/archiver"
 	"github.com/restic/restic/internal/debug"
 	"github.com/restic/restic/internal/errors"
+	"github.com/restic/restic/internal/filter"
 	"github.com/restic/restic/internal/fs"
 	"github.com/restic/restic/internal/repository"
 	"github.com/restic/restic/internal/restic"
@@ -298,6 +299,11 @@ func collectRejectByNameFuncs(opts BackupOptions, repo *repository.Repository, t
 		if err != nil {
 			return nil, err
 		}
+
+		if valid, invalidPatterns := filter.ValidatePatterns(excludes); !valid {
+			return nil, errors.Fatalf("--exclude-file: invalid pattern(s) provided:\n%s", strings.Join(invalidPatterns, "\n"))
+		}
+
 		opts.Excludes = append(opts.Excludes, excludes...)
 	}
 
@@ -306,14 +312,27 @@ func collectRejectByNameFuncs(opts BackupOptions, repo *repository.Repository, t
 		if err != nil {
 			return nil, err
 		}
+
+		if valid, invalidPatterns := filter.ValidatePatterns(excludes); !valid {
+			return nil, errors.Fatalf("--iexclude-file: invalid pattern(s) provided:\n%s", strings.Join(invalidPatterns, "\n"))
+		}
+
 		opts.InsensitiveExcludes = append(opts.InsensitiveExcludes, excludes...)
 	}
 
 	if len(opts.InsensitiveExcludes) > 0 {
+		if valid, invalidPatterns := filter.ValidatePatterns(opts.InsensitiveExcludes); !valid {
+			return nil, errors.Fatalf("--iexclude: invalid pattern(s) provided:\n%s", strings.Join(invalidPatterns, "\n"))
+		}
+
 		fs = append(fs, rejectByInsensitivePattern(opts.InsensitiveExcludes))
 	}
 
 	if len(opts.Excludes) > 0 {
+		if valid, invalidPatterns := filter.ValidatePatterns(opts.Excludes); !valid {
+			return nil, errors.Fatalf("--exclude: invalid pattern(s) provided:\n%s", strings.Join(invalidPatterns, "\n"))
+		}
+
 		fs = append(fs, rejectByPattern(opts.Excludes))
 	}
 

cmd/restic/integration_filter_pattern_test.go

@@ -0,0 +1,69 @@
+//go:build go1.16
+// +build go1.16
+
+// Before Go 1.16 filepath.Match returned early on a failed match,
+// and thus did not report any later syntax error in the pattern.
+// https://go.dev/doc/go1.16#path/filepath
+
+package main
+
+import (
+	"io/ioutil"
+	"path/filepath"
+	"testing"
+
+	rtest "github.com/restic/restic/internal/test"
+)
+
+func TestBackupFailsWhenUsingInvalidPatterns(t *testing.T) {
+	env, cleanup := withTestEnvironment(t)
+	defer cleanup()
+
+	testRunInit(t, env.gopts)
+
+	var err error
+
+	// Test --exclude
+	err = testRunBackupAssumeFailure(t, filepath.Dir(env.testdata), []string{"testdata"}, BackupOptions{Excludes: []string{"*[._]log[.-][0-9]", "!*[._]log[.-][0-9]"}}, env.gopts)
+
+	rtest.Equals(t, `Fatal: --exclude: invalid pattern(s) provided:
+*[._]log[.-][0-9]
+!*[._]log[.-][0-9]`, err.Error())
+
+	// Test --iexclude
+	err = testRunBackupAssumeFailure(t, filepath.Dir(env.testdata), []string{"testdata"}, BackupOptions{InsensitiveExcludes: []string{"*[._]log[.-][0-9]", "!*[._]log[.-][0-9]"}}, env.gopts)
+
+	rtest.Equals(t, `Fatal: --iexclude: invalid pattern(s) provided:
+*[._]log[.-][0-9]
+!*[._]log[.-][0-9]`, err.Error())
+}
+
+func TestBackupFailsWhenUsingInvalidPatternsFromFile(t *testing.T) {
+	env, cleanup := withTestEnvironment(t)
+	defer cleanup()
+
+	testRunInit(t, env.gopts)
+
+	// Create an exclude file with some invalid patterns
+	excludeFile := env.base + "/excludefile"
+	fileErr := ioutil.WriteFile(excludeFile, []byte("*.go\n*[._]log[.-][0-9]\n!*[._]log[.-][0-9]"), 0644)
+	if fileErr != nil {
+		t.Fatalf("Could not write exclude file: %v", fileErr)
+	}
+
+	var err error
+
+	// Test --exclude-file:
+	err = testRunBackupAssumeFailure(t, filepath.Dir(env.testdata), []string{"testdata"}, BackupOptions{ExcludeFiles: []string{excludeFile}}, env.gopts)
+
+	rtest.Equals(t, `Fatal: --exclude-file: invalid pattern(s) provided:
+*[._]log[.-][0-9]
+!*[._]log[.-][0-9]`, err.Error())
+
+	// Test --iexclude-file
+	err = testRunBackupAssumeFailure(t, filepath.Dir(env.testdata), []string{"testdata"}, BackupOptions{InsensitiveExcludeFiles: []string{excludeFile}}, env.gopts)
+
+	rtest.Equals(t, `Fatal: --iexclude-file: invalid pattern(s) provided:
+*[._]log[.-][0-9]
+!*[._]log[.-][0-9]`, err.Error())
+}

internal/filter/filter.go

@@ -18,6 +18,7 @@ type patternPart struct {
 
 // Pattern represents a preparsed filter pattern
 type Pattern struct {
+	original  string
 	parts     []patternPart
 	isNegated bool
 }
@@ -31,6 +32,9 @@ func prepareStr(str string) ([]string, error) {
 
 func preparePattern(patternStr string) Pattern {
 	var negate bool
+
+	originalPattern := patternStr
+
 	if patternStr[0] == '!' {
 		negate = true
 		patternStr = patternStr[1:]
@@ -48,7 +52,7 @@ func preparePattern(patternStr string) Pattern {
 		parts[i] = patternPart{part, isSimple}
 	}
 
-	return Pattern{parts, negate}
+	return Pattern{originalPattern, parts, negate}
 }
 
 // Split p into path components. Assuming p has been Cleaned, no component
@@ -130,7 +134,7 @@ func childMatch(pattern Pattern, strs []string) (matched bool, err error) {
 	} else {
 		l = len(strs)
 	}
-	return match(Pattern{pattern.parts[0:l], pattern.isNegated}, strs)
+	return match(Pattern{pattern.original, pattern.parts[0:l], pattern.isNegated}, strs)
 }
 
 func hasDoubleWildcard(list Pattern) (ok bool, pos int) {
@@ -158,7 +162,7 @@ func match(pattern Pattern, strs []string) (matched bool, err error) {
 			}
 			newPat = append(newPat, pattern.parts[pos+1:]...)
 
-			matched, err := match(Pattern{newPat, pattern.isNegated}, strs)
+			matched, err := match(Pattern{pattern.original, newPat, pattern.isNegated}, strs)
 			if err != nil {
 				return false, err
 			}
@@ -216,6 +220,27 @@ func match(pattern Pattern, strs []string) (matched bool, err error) {
 	return false, nil
 }
 
+// ValidatePatterns validates a slice of patterns.
+// Returns true if all patterns are valid - false otherwise, along with the invalid patterns.
+func ValidatePatterns(patterns []string) (allValid bool, invalidPatterns []string) {
+	invalidPatterns = make([]string, 0)
+
+	for _, Pattern := range ParsePatterns(patterns) {
+		// Validate all pattern parts
+		for _, part := range Pattern.parts {
+			// Validate the pattern part by trying to match it against itself
+			if _, validErr := filepath.Match(part.pattern, part.pattern); validErr != nil {
+				invalidPatterns = append(invalidPatterns, Pattern.original)
+
+				// If a single part is invalid, stop processing this pattern
+				continue
+			}
+		}
+	}
+
+	return len(invalidPatterns) == 0, invalidPatterns
+}
+
 // ParsePatterns prepares a list of patterns for use with List.
 func ParsePatterns(pattern []string) []Pattern {
 	patpat := make([]Pattern, 0)

internal/filter/filter_patterns_test.go

@@ -0,0 +1,57 @@
+//go:build go1.16
+// +build go1.16
+
+// Before Go 1.16 filepath.Match returned early on a failed match,
+// and thus did not report any later syntax error in the pattern.
+// https://go.dev/doc/go1.16#path/filepath
+
+package filter_test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/restic/restic/internal/filter"
+	rtest "github.com/restic/restic/internal/test"
+)
+
+func TestValidPatterns(t *testing.T) {
+	// Test invalid patterns are detected and returned
+	t.Run("detect-invalid-patterns", func(t *testing.T) {
+		allValid, invalidPatterns := filter.ValidatePatterns([]string{"*.foo", "*[._]log[.-][0-9]", "!*[._]log[.-][0-9]"})
+
+		rtest.Assert(t, allValid == false, "Expected invalid patterns to be detected")
+
+		rtest.Equals(t, invalidPatterns, []string{"*[._]log[.-][0-9]", "!*[._]log[.-][0-9]"})
+	})
+
+	// Test all patterns defined in matchTests are valid
+	patterns := make([]string, 0)
+
+	for _, data := range matchTests {
+		patterns = append(patterns, data.pattern)
+	}
+
+	t.Run("validate-patterns", func(t *testing.T) {
+		allValid, invalidPatterns := filter.ValidatePatterns(patterns)
+
+		if !allValid {
+			t.Errorf("Found invalid pattern(s):\n%s", strings.Join(invalidPatterns, "\n"))
+		}
+	})
+
+	// Test all patterns defined in childMatchTests are valid
+	childPatterns := make([]string, 0)
+
+	for _, data := range childMatchTests {
+		childPatterns = append(childPatterns, data.pattern)
+	}
+
+	t.Run("validate-child-patterns", func(t *testing.T) {
+		allValid, invalidPatterns := filter.ValidatePatterns(childPatterns)
+
+		if !allValid {
+			t.Errorf("Found invalid child pattern(s):\n%s", strings.Join(invalidPatterns, "\n"))
+		}
+	})
+}