diff --git a/changelog/changelog-github.tmpl b/changelog/changelog-github.tmpl index d19788daf..9936da8e6 100644 --- a/changelog/changelog-github.tmpl +++ b/changelog/changelog-github.tmpl @@ -15,7 +15,7 @@ Details {{ range $entry := .Entries }}{{ with $entry }} * {{ .Type }} #{{ .PrimaryID }}: {{ .Title }} {{ range $par := .Paragraphs }} - {{ $par }} +{{ indent 3 $par }} {{ end }} {{ range $id := .Issues -}} {{ ` ` }}[#{{ $id }}](https://github.com/restic/restic/issues/{{ $id -}}) diff --git a/changelog/unreleased/issue-4004 b/changelog/unreleased/issue-4004 new file mode 100644 index 000000000..ca23af26f --- /dev/null +++ b/changelog/unreleased/issue-4004 @@ -0,0 +1,12 @@ +Bugfix: Allow use of container level SAS/SAT tokens with Azure backend + +When using a SAS/SAT token for authentication with Azure, restic was expecting +the provided token to be generated at the account level, granting permissions +to the storage account and all its containers. This caused an error that did +not allow tokens that were generated at the container level to be used to +initalize a repository. +Restic now allows SAS/SAT tokens that were generated at the account or +container level to be used to initalize a repository. + +https://github.com/restic/restic/issues/4004 +https://github.com/restic/restic/pull/5093 diff --git a/changelog/unreleased/issue-5050 b/changelog/unreleased/issue-5050 new file mode 100644 index 000000000..9604fc857 --- /dev/null +++ b/changelog/unreleased/issue-5050 @@ -0,0 +1,7 @@ +Bugfix: Missing error if `tag` fails to lock repository + +Since restic 0.17.0, the `tag` command did not return an error if it failed to +open or lock the repository. This has been fixed. + +https://github.com/restic/restic/issues/5050 +https://github.com/restic/restic/pull/5056 diff --git a/changelog/unreleased/pull-5047 b/changelog/unreleased/pull-5047 new file mode 100644 index 000000000..ee50c6ec7 --- /dev/null +++ b/changelog/unreleased/pull-5047 @@ -0,0 +1,7 @@ +Bugfix: Fix possible error on concurrent cache cleanup + +Fix for multiple restic processes executing concurrently and racing to +remove obsolete snapshots from the local backend cache. Restic now suppresses the `no +such file or directory` error. + +https://github.com/restic/restic/pull/5047 diff --git a/changelog/unreleased/pull-5057 b/changelog/unreleased/pull-5057 new file mode 100644 index 000000000..c34436044 --- /dev/null +++ b/changelog/unreleased/pull-5057 @@ -0,0 +1,21 @@ +Bugfix: Do not include irregular files in backup + +Since restic 0.17.1, files with type `irregular` could incorrectly be included +in snapshots. This is most likely to occur when backing up special file types +on Windows that cannot be handled by restic. + +This has been fixed. + +When running the `check` command this bug resulted in an error like the +following: + +``` + tree 12345678[...]: node "example.zip" with invalid type "irregular" +``` + +Repairing the affected snapshots requires upgrading to restic 0.17.2 and then +manually running `restic repair snapshots --forget`. This will remove the +`irregular` files from the snapshots. + +https://github.com/restic/restic/pull/5057 +https://forum.restic.net/t/errors-found-by-check-1-invalid-type-irregular-2-ciphertext-verification-failed/8447/2 diff --git a/cmd/restic/cmd_list.go b/cmd/restic/cmd_list.go index 1a4791e31..fcbed4440 100644 --- a/cmd/restic/cmd_list.go +++ b/cmd/restic/cmd_list.go @@ -2,6 +2,7 @@ package main import ( "context" + "strings" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/repository/index" @@ -10,8 +11,11 @@ import ( "github.com/spf13/cobra" ) +var listAllowedArgs = []string{"blobs", "packs", "index", "snapshots", "keys", "locks"} +var listAllowedArgsUseString = strings.Join(listAllowedArgs, "|") + var cmdList = &cobra.Command{ - Use: "list [flags] [blobs|packs|index|snapshots|keys|locks]", + Use: "list [flags] [" + listAllowedArgsUseString + "]", Short: "List objects in the repository", Long: ` The "list" command allows listing objects in the repository based on type. @@ -30,6 +34,8 @@ Exit status is 12 if the password is incorrect. RunE: func(cmd *cobra.Command, args []string) error { return runList(cmd.Context(), globalOptions, args) }, + ValidArgs: listAllowedArgs, + Args: cobra.MatchAll(cobra.ExactArgs(1), cobra.OnlyValidArgs), } func init() { diff --git a/cmd/restic/cmd_repair_snapshots.go b/cmd/restic/cmd_repair_snapshots.go index 385854312..01281cf3e 100644 --- a/cmd/restic/cmd_repair_snapshots.go +++ b/cmd/restic/cmd_repair_snapshots.go @@ -92,6 +92,10 @@ func runRepairSnapshots(ctx context.Context, gopts GlobalOptions, opts RepairOpt // - files whose contents are not fully available (-> file will be modified) rewriter := walker.NewTreeRewriter(walker.RewriteOpts{ RewriteNode: func(node *restic.Node, path string) *restic.Node { + if node.Type == "irregular" || node.Type == "" { + Verbosef(" file %q: removed node with invalid type %q\n", path, node.Type) + return nil + } if node.Type != "file" { return node } diff --git a/cmd/restic/cmd_tag.go b/cmd/restic/cmd_tag.go index c7bf725e9..8a2a83678 100644 --- a/cmd/restic/cmd_tag.go +++ b/cmd/restic/cmd_tag.go @@ -110,7 +110,7 @@ func runTag(ctx context.Context, opts TagOptions, gopts GlobalOptions, args []st Verbosef("create exclusive lock for repository\n") ctx, repo, unlock, err := openWithExclusiveLock(ctx, gopts, false) if err != nil { - return nil + return err } defer unlock() diff --git a/doc/030_preparing_a_new_repo.rst b/doc/030_preparing_a_new_repo.rst index fd5b31127..5826ffacf 100644 --- a/doc/030_preparing_a_new_repo.rst +++ b/doc/030_preparing_a_new_repo.rst @@ -455,9 +455,11 @@ Backblaze B2 than using the Backblaze B2 backend directly. Different from the B2 backend, restic's S3 backend will only hide no longer - necessary files. Thus, make sure to setup lifecycle rules to eventually - delete hidden files. The lifecycle setting "Keep only the last version of the file" - will keep only the most current version of a file. Read the [Backblaze documentation](https://www.backblaze.com/docs/cloud-storage-lifecycle-rules). + necessary files. By default, Backblaze B2 retains all of the different versions of the + files and "hides" the older versions. Thus, to free space occupied by hidden files, + it is **recommended** to use the B2 lifecycle "Keep only the last version of the file". + The previous version of the file is "hidden" for one day and then deleted automatically + by B2. More details at the [Backblaze documentation](https://www.backblaze.com/docs/cloud-storage-lifecycle-rules). Restic can backup data to any Backblaze B2 bucket. You need to first setup the following environment variables with the credentials you can find in the diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index e7c346d3a..839320816 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -262,7 +262,8 @@ func (arch *Archiver) nodeFromFileInfo(snPath, filename string, fi os.FileInfo, } // overwrite name to match that within the snapshot node.Name = path.Base(snPath) - if err != nil { + // do not filter error for nodes of irregular or invalid type + if node.Type != "irregular" && node.Type != "" && err != nil { err = fmt.Errorf("incomplete metadata for %v: %w", filename, err) return node, arch.error(filename, err) } diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index c54f9ea33..5ecfd4bc4 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -2423,4 +2423,47 @@ func TestMetadataBackupErrorFiltering(t *testing.T) { rtest.Assert(t, node != nil, "node is missing") rtest.Assert(t, err == replacementErr, "expected %v got %v", replacementErr, err) rtest.Assert(t, filteredErr != nil, "missing inner error") + + // check that errors from reading irregular file are not filtered + filteredErr = nil + node, err = arch.nodeFromFileInfo("file", filename, wrapIrregularFileInfo(fi), false) + rtest.Assert(t, node != nil, "node is missing") + rtest.Assert(t, filteredErr == nil, "error for irregular node should not have been filtered") + rtest.Assert(t, strings.Contains(err.Error(), "irregular"), "unexpected error %q does not warn about irregular file mode", err) +} + +func TestIrregularFile(t *testing.T) { + files := TestDir{ + "testfile": TestFile{ + Content: "foo bar test file", + }, + } + tempdir, repo := prepareTempdirRepoSrc(t, files) + + back := rtest.Chdir(t, tempdir) + defer back() + + tempfile := filepath.Join(tempdir, "testfile") + fi := lstat(t, "testfile") + + statfs := &StatFS{ + FS: fs.Local{}, + OverrideLstat: map[string]os.FileInfo{ + tempfile: wrapIrregularFileInfo(fi), + }, + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + arch := New(repo, fs.Track{FS: statfs}, Options{}) + _, excluded, err := arch.save(ctx, "/", tempfile, nil) + if err == nil { + t.Fatalf("Save() should have failed") + } + rtest.Assert(t, strings.Contains(err.Error(), "irregular"), "unexpected error %q does not warn about irregular file mode", err) + + if excluded { + t.Errorf("Save() excluded the node, that's unexpected") + } } diff --git a/internal/archiver/archiver_unix_test.go b/internal/archiver/archiver_unix_test.go index 4a380dff8..bc64a1047 100644 --- a/internal/archiver/archiver_unix_test.go +++ b/internal/archiver/archiver_unix_test.go @@ -46,6 +46,16 @@ func wrapFileInfo(fi os.FileInfo) os.FileInfo { return res } +// wrapIrregularFileInfo returns a new os.FileInfo with the mode changed to irregular file +func wrapIrregularFileInfo(fi os.FileInfo) os.FileInfo { + // wrap the os.FileInfo so we can return a modified stat_t + return wrappedFileInfo{ + FileInfo: fi, + sys: fi.Sys().(*syscall.Stat_t), + mode: (fi.Mode() &^ os.ModeType) | os.ModeIrregular, + } +} + func statAndSnapshot(t *testing.T, repo archiverRepo, name string) (*restic.Node, *restic.Node) { fi := lstat(t, name) want, err := restic.NodeFromFileInfo(name, fi, false) diff --git a/internal/archiver/archiver_windows_test.go b/internal/archiver/archiver_windows_test.go index e1195030f..ac8a67f2b 100644 --- a/internal/archiver/archiver_windows_test.go +++ b/internal/archiver/archiver_windows_test.go @@ -26,3 +26,11 @@ func wrapFileInfo(fi os.FileInfo) os.FileInfo { return res } + +// wrapIrregularFileInfo returns a new os.FileInfo with the mode changed to irregular file +func wrapIrregularFileInfo(fi os.FileInfo) os.FileInfo { + return wrappedFileInfo{ + FileInfo: fi, + mode: (fi.Mode() &^ os.ModeType) | os.ModeIrregular, + } +} diff --git a/internal/backend/azure/azure.go b/internal/backend/azure/azure.go index 737cf0e14..76c8d755a 100644 --- a/internal/backend/azure/azure.go +++ b/internal/backend/azure/azure.go @@ -160,6 +160,12 @@ func Create(ctx context.Context, cfg Config, rt http.RoundTripper) (*Backend, er if err != nil { return nil, errors.Wrap(err, "container.Create") } + } else if err != nil && bloberror.HasCode(err, bloberror.AuthorizationFailure) { + // We ignore this Auth. Failure, as the failure is related to the type + // of SAS/SAT, not an actual real failure. If the token is invalid, we + // fail later on anyway. + // For details see Issue #4004. + debug.Log("Ignoring AuthorizationFailure when calling GetProperties") } else if err != nil { return be, errors.Wrap(err, "container.GetProperties") } diff --git a/internal/backend/azure/azure_test.go b/internal/backend/azure/azure_test.go index 7df27d325..adafb6b03 100644 --- a/internal/backend/azure/azure_test.go +++ b/internal/backend/azure/azure_test.go @@ -80,6 +80,91 @@ func BenchmarkBackendAzure(t *testing.B) { newAzureTestSuite().RunBenchmarks(t) } +// TestBackendAzureAccountToken tests that a Storage Account SAS/SAT token can authorize. +// This test ensures that restic can use a token that was generated using the storage +// account keys can be used to authorize the azure connection. +// Requires the RESTIC_TEST_AZURE_ACCOUNT_NAME, RESTIC_TEST_AZURE_REPOSITORY, and the +// RESTIC_TEST_AZURE_ACCOUNT_SAS environment variables to be set, otherwise this test +// will be skipped. +func TestBackendAzureAccountToken(t *testing.T) { + vars := []string{ + "RESTIC_TEST_AZURE_ACCOUNT_NAME", + "RESTIC_TEST_AZURE_REPOSITORY", + "RESTIC_TEST_AZURE_ACCOUNT_SAS", + } + + for _, v := range vars { + if os.Getenv(v) == "" { + t.Skipf("set %v to test SAS/SAT Token Authentication", v) + return + } + } + + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + cfg, err := azure.ParseConfig(os.Getenv("RESTIC_TEST_AZURE_REPOSITORY")) + if err != nil { + t.Fatal(err) + } + + cfg.AccountName = os.Getenv("RESTIC_TEST_AZURE_ACCOUNT_NAME") + cfg.AccountSAS = options.NewSecretString(os.Getenv("RESTIC_TEST_AZURE_ACCOUNT_SAS")) + + tr, err := backend.Transport(backend.TransportOptions{}) + if err != nil { + t.Fatal(err) + } + + _, err = azure.Create(ctx, *cfg, tr) + if err != nil { + t.Fatal(err) + } +} + +// TestBackendAzureContainerToken tests that a container SAS/SAT token can authorize. +// This test ensures that restic can use a token that was generated using a user +// delegation key against the container we are storing data in can be used to +// authorize the azure connection. +// Requires the RESTIC_TEST_AZURE_ACCOUNT_NAME, RESTIC_TEST_AZURE_REPOSITORY, and the +// RESTIC_TEST_AZURE_CONTAINER_SAS environment variables to be set, otherwise this test +// will be skipped. +func TestBackendAzureContainerToken(t *testing.T) { + vars := []string{ + "RESTIC_TEST_AZURE_ACCOUNT_NAME", + "RESTIC_TEST_AZURE_REPOSITORY", + "RESTIC_TEST_AZURE_CONTAINER_SAS", + } + + for _, v := range vars { + if os.Getenv(v) == "" { + t.Skipf("set %v to test SAS/SAT Token Authentication", v) + return + } + } + + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + cfg, err := azure.ParseConfig(os.Getenv("RESTIC_TEST_AZURE_REPOSITORY")) + if err != nil { + t.Fatal(err) + } + + cfg.AccountName = os.Getenv("RESTIC_TEST_AZURE_ACCOUNT_NAME") + cfg.AccountSAS = options.NewSecretString(os.Getenv("RESTIC_TEST_AZURE_CONTAINER_SAS")) + + tr, err := backend.Transport(backend.TransportOptions{}) + if err != nil { + t.Fatal(err) + } + + _, err = azure.Create(ctx, *cfg, tr) + if err != nil { + t.Fatal(err) + } +} + func TestUploadLargeFile(t *testing.T) { if os.Getenv("RESTIC_AZURE_TEST_LARGE_UPLOAD") == "" { t.Skip("set RESTIC_AZURE_TEST_LARGE_UPLOAD=1 to test large uploads") diff --git a/internal/backend/cache/file.go b/internal/backend/cache/file.go index 12f5f23c5..adc39d687 100644 --- a/internal/backend/cache/file.go +++ b/internal/backend/cache/file.go @@ -211,6 +211,10 @@ func (c *Cache) list(t restic.FileType) (restic.IDSet, error) { dir := filepath.Join(c.path, cacheLayoutPaths[t]) err := filepath.Walk(dir, func(name string, fi os.FileInfo, err error) error { if err != nil { + // ignore ErrNotExist to gracefully handle multiple processes clearing the cache + if errors.Is(err, os.ErrNotExist) { + return nil + } return errors.Wrap(err, "Walk") } diff --git a/internal/fs/preallocate_linux.go b/internal/fs/preallocate_linux.go index 30b9e4644..7b0449507 100644 --- a/internal/fs/preallocate_linux.go +++ b/internal/fs/preallocate_linux.go @@ -2,6 +2,7 @@ package fs import ( "os" + "syscall" "golang.org/x/sys/unix" ) @@ -12,5 +13,17 @@ func PreallocateFile(wr *os.File, size int64) error { } // int fallocate(int fd, int mode, off_t offset, off_t len) // use mode = 0 to also change the file size - return unix.Fallocate(int(wr.Fd()), 0, 0, size) + return ignoringEINTR(func() error { return unix.Fallocate(int(wr.Fd()), 0, 0, size) }) +} + +// ignoringEINTR makes a function call and repeats it if it returns +// an EINTR error. +// copied from /usr/lib/go/src/internal/poll/fd_posix.go of go 1.23.1 +func ignoringEINTR(fn func() error) error { + for { + err := fn() + if err != syscall.EINTR { + return err + } + } } diff --git a/internal/restic/mknod_unix.go b/internal/restic/mknod_unix.go index 7dd6c60d0..b9a71bdf6 100644 --- a/internal/restic/mknod_unix.go +++ b/internal/restic/mknod_unix.go @@ -3,8 +3,16 @@ package restic -import "golang.org/x/sys/unix" +import ( + "os" -func mknod(path string, mode uint32, dev uint64) (err error) { - return unix.Mknod(path, mode, int(dev)) + "golang.org/x/sys/unix" +) + +func mknod(path string, mode uint32, dev uint64) error { + err := unix.Mknod(path, mode, int(dev)) + if err != nil { + err = &os.PathError{Op: "mknod", Path: path, Err: err} + } + return err } diff --git a/internal/restic/node_freebsd.go b/internal/restic/node_freebsd.go index 34d5b272c..6a2d04f36 100644 --- a/internal/restic/node_freebsd.go +++ b/internal/restic/node_freebsd.go @@ -3,14 +3,21 @@ package restic -import "syscall" +import ( + "os" + "syscall" +) func (node Node) restoreSymlinkTimestamps(path string, utimes [2]syscall.Timespec) error { return nil } -func mknod(path string, mode uint32, dev uint64) (err error) { - return syscall.Mknod(path, mode, dev) +func mknod(path string, mode uint32, dev uint64) error { + err := syscall.Mknod(path, mode, dev) + if err != nil { + err = &os.PathError{Op: "mknod", Path: path, Err: err} + } + return err } func (s statT) atim() syscall.Timespec { return s.Atimespec } diff --git a/internal/restic/node_unix_test.go b/internal/restic/node_unix_test.go index 9ea7b1725..b3927de22 100644 --- a/internal/restic/node_unix_test.go +++ b/internal/restic/node_unix_test.go @@ -7,10 +7,12 @@ import ( "os" "path/filepath" "runtime" + "strings" "syscall" "testing" "time" + "github.com/restic/restic/internal/errors" rtest "github.com/restic/restic/internal/test" ) @@ -145,3 +147,12 @@ func TestNodeFromFileInfo(t *testing.T) { }) } } + +func TestMknodError(t *testing.T) { + d := t.TempDir() + // Call mkfifo, which calls mknod, as mknod may give + // "operation not permitted" on Mac. + err := mkfifo(d, 0) + rtest.Assert(t, errors.Is(err, os.ErrExist), "want ErrExist, got %q", err) + rtest.Assert(t, strings.Contains(err.Error(), d), "filename not in %q", err) +}