diff --git a/changelog/unreleased/issue-3709 b/changelog/unreleased/issue-3709 new file mode 100644 index 000000000..195b4b502 --- /dev/null +++ b/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 diff --git a/cmd/restic/cmd_backup.go b/cmd/restic/cmd_backup.go index 7ce4f52de..bf1e17a47 100644 --- a/cmd/restic/cmd_backup.go +++ b/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)) } diff --git a/cmd/restic/integration_filter_pattern_test.go b/cmd/restic/integration_filter_pattern_test.go new file mode 100644 index 000000000..e1534d841 --- /dev/null +++ b/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()) +} diff --git a/internal/filter/filter.go b/internal/filter/filter.go index 3e16c9316..cfacb8cc5 100644 --- a/internal/filter/filter.go +++ b/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) diff --git a/internal/filter/filter_patterns_test.go b/internal/filter/filter_patterns_test.go new file mode 100644 index 000000000..215471500 --- /dev/null +++ b/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")) + } + }) +}