Merge pull request #5098 from MichaelEischer/prepare-patch-release

Prepare patch release

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 -}})

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

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

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

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

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() {

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
 			}

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()
 

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
+   necessary files. By default, Backblaze B2 retains all of the different versions of the
-   delete hidden files. The lifecycle setting "Keep only the last version of the file" 
+   files and "hides" the older versions. Thus, to free space occupied by hidden files,
-   will keep only the most current version of a file. Read the [Backblaze documentation](https://www.backblaze.com/docs/cloud-storage-lifecycle-rules).
+   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

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)
 	}

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")
+	}
 }

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)

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,
+	}
+}

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")
 	}

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")

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")
 		}
 

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
+		}
+	}
 }

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) {
+	"golang.org/x/sys/unix"
-	return unix.Mknod(path, mode, int(dev))
+)
+
+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
 }

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) {
+func mknod(path string, mode uint32, dev uint64) error {
-	return syscall.Mknod(path, mode, dev)
+	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 }

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)
+}