Limit and retry the module download in the Docker build (closes #351)

Each `go mod download` in the Dockerfile now runs under a 180-second
time limit and is tried up to three times. Without the limit, a stalled
connection to the module proxy held the step, and every build on the
host sharing it, until the kernel dropped the connection. When all
three attempts fail, the step prints "module download failed" and
exits 1, so the build stops there and does not look like a test or
lint failure. The loop is repeated in each of the three stages because
a Dockerfile cannot share shell code between stages. A test checks
every download step for the limit, the retries, the message and the
final `exit 1`.

Judgement call: 180 seconds and three attempts, against a normal
download of 13 to 24 seconds.
Deviation: REPO_POLICIES.md's model Go Dockerfile still shows a bare
`RUN go mod download`.

Model: opus-5-5

Dockerfile

@@ -5,7 +5,16 @@
 FROM golangci/golangci-lint:v2.14.0@sha256:ad862ba6b3798cbe0fd9fd7408d498fd74fbd2623a92406b2fd3898faf0bf98f AS lint
 WORKDIR /src
 COPY go.mod go.sum ./
-RUN go mod download
+# Without the time limit a stalled connection to the module proxy holds
+# this step, and every build on the host sharing it, until the kernel
+# drops the connection, which can take an hour. The download normally
+# takes under half a minute.
+RUN for attempt in 1 2 3; do \
+        timeout 180 go mod download && exit 0; \
+        echo "go mod download attempt $attempt failed or ran over 180s" >&2; \
+    done; \
+    echo "module download failed: giving up after 3 attempts" >&2; \
+    exit 1
 COPY . .
 # `golangci-lint run` silently ignores an unknown top-level key in
 # .golangci.yml, such as a misspelt `linters:`; `config verify` fails on it.
@@ -18,7 +27,13 @@ RUN golangci-lint run --config .golangci.yml ./...
 FROM golang:1.26.1@sha256:cd78d88e00afadbedd272f977d375a6247455f3a4b1178f8ae8bbcb201743a8a AS test
 WORKDIR /src
 COPY go.mod go.sum ./
-RUN go mod download
+# Limited and retried as in the lint phase above.
+RUN for attempt in 1 2 3; do \
+        timeout 180 go mod download && exit 0; \
+        echo "go mod download attempt $attempt failed or ran over 180s" >&2; \
+    done; \
+    echo "module download failed: giving up after 3 attempts" >&2; \
+    exit 1
 COPY . .
 RUN go test -timeout 90s -race -cover ./... || \
     { echo "--- Rerunning with -v for details ---"; \
@@ -36,7 +51,13 @@ RUN apk add --no-cache git
 RUN git config --system --add safe.directory /src
 WORKDIR /src
 COPY go.mod go.sum ./
-RUN go mod download
+# Limited and retried as in the lint phase above.
+RUN for attempt in 1 2 3; do \
+        timeout 180 go mod download && exit 0; \
+        echo "go mod download attempt $attempt failed or ran over 180s" >&2; \
+    done; \
+    echo "module download failed: giving up after 3 attempts" >&2; \
+    exit 1
 COPY . .
 
 # The VERSION build arg when one is given, otherwise

README.md

@@ -372,9 +372,7 @@ on the destination in one go, use `vaultik remote nuke --force`.
   store — its remote key (or an unambiguous leading part of it) as shown
   by `snapshot list`. See
   [restoring on another machine](#restoring-on-another-machine).
-* Optional path arguments to restore specific files/directories (default: all).
-  A path that matches nothing in the snapshot is named in a warning, and the
-  command exits non-zero after restoring the paths that match
+* Optional path arguments to restore specific files/directories (default: all)
 * Preserves file permissions, timestamps, ownership (ownership requires root),
   symlinks, and empty directories
 * `--verify`: After restoring, verify every file's chunk hashes match
@@ -435,7 +433,10 @@ object store. Credentials are read from `s3.access_key_id` and
 `s3.secret_access_key` in the config file.
 
 **Local filesystem** (`file:///path/to/backup`): Stores blobs and metadata on
-a local or mounted filesystem. Useful for testing or backing up to a NAS.
+a local or mounted filesystem. Useful for testing or backing up to a NAS. The
+path must be absolute; a relative one is refused when the config loads.
+Percent-escapes in the path are decoded, so `my%20stick` and `my stick` name
+the same directory.
 
 **Rclone** (`rclone://remote/path`): Uses rclone's 70+ supported cloud
 providers. Requires rclone to be configured separately (`rclone config`).

cmd/vaultik/dockermoddownload_test.go

@@ -0,0 +1,45 @@
+package main_test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestDockerfileBoundsEveryModuleDownload fails unless every
+// `go mod download` in the Dockerfile runs under a time limit, is tried
+// a fixed number of times, and then fails the build with a message that
+// names the module download (issue #351). Like the version tests, this
+// parses the Dockerfile; that a stalled download really is cut off and
+// retried was checked by hand.
+func TestDockerfileBoundsEveryModuleDownload(t *testing.T) {
+	t.Parallel()
+
+	var downloads []string
+
+	for _, instruction := range instructions(t, productDockerfile) {
+		if strings.Contains(instruction, "go mod download") {
+			downloads = append(downloads, instruction)
+		}
+	}
+
+	require.NotEmpty(t, downloads, "%s must download modules",
+		productDockerfile)
+
+	for _, download := range downloads {
+		assert.Contains(t, download, "timeout 180 go mod download",
+			"%s: a module download must have a time limit: %s",
+			productDockerfile, download)
+		assert.Contains(t, download, "for attempt in 1 2 3",
+			"%s: a module download must be retried: %s",
+			productDockerfile, download)
+		assert.Contains(t, download, "module download failed",
+			"%s: a module download that never succeeds must say so: %s",
+			productDockerfile, download)
+		assert.True(t, strings.HasSuffix(download, "exit 1"),
+			"%s: a module download that never succeeds must fail the"+
+				" build: %s", productDockerfile, download)
+	}
+}

internal/blob/packer.go

@@ -245,6 +245,17 @@ func (p *Packer) Flush(ctx context.Context) error {
 	return nil
 }
 
+// DiscardCurrentBlob closes and removes the temp file of the blob being
+// packed, if any, and drops that blob. A caller that stops before Flush
+// must call it, or the temp file is left behind. Thread-safe.
+func (p *Packer) DiscardCurrentBlob() {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+
+	p.cleanupTempFile()
+	p.currentBlob = nil
+}
+
 // FinalizeBlob finalizes the current blob being assembled.
 // This compresses the accumulated chunks, encrypts the result, and computes
 // the content-addressed hash. The finalized blob is either passed to the

internal/cli/config.go

@@ -251,6 +251,12 @@ func NewConfigCommand() *cobra.Command {
 		Use:   "config",
 		Short: "Manage the configuration file",
 		Long:  "Commands for creating, editing, and querying the vaultik config file.",
+		// cobra checks Args only on a command that has a Run: without
+		// one, a mistyped subcommand printed help and exited 0.
+		Args: cobra.NoArgs,
+		Run: func(cmd *cobra.Command, _ []string) {
+			_ = cmd.Help()
+		},
 	}
 
 	cmd.AddCommand(newConfigInitCommand())

internal/cli/database.go

@@ -15,6 +15,12 @@ func NewDatabaseCommand() *cobra.Command {
 		Use:   "database",
 		Short: "Manage the local state database",
 		Long:  `Commands for managing the local SQLite state database.`,
+		// cobra checks Args only on a command that has a Run: without
+		// one, a mistyped subcommand printed help and exited 0.
+		Args: cobra.NoArgs,
+		Run: func(cmd *cobra.Command, _ []string) {
+			_ = cmd.Help()
+		},
 	}
 
 	cmd.AddCommand(

internal/cli/entry_unknown_subcommand_test.go

@@ -0,0 +1,43 @@
+package cli //nolint:testpackage // shares runEntry and the command-word constants
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestEntryGroupRejectsUnknownSubcommand runs each group command with a
+// mistyped subcommand, as a crontab line might carry one. It must fail
+// with exit 1 and name the word, as `vaultik bogus` does at the top
+// level, instead of printing the group's help and exiting 0 having done
+// nothing. The group name alone still prints its help.
+//
+//nolint:paralleltest // replaces os.Args, os.Stdout and os.Stderr
+func TestEntryGroupRejectsUnknownSubcommand(t *testing.T) {
+	for _, testCase := range []struct {
+		group    string
+		mistyped string
+	}{
+		{group: cmdSnapshot, mistyped: "craete"},
+		{group: "config", mistyped: "inti"},
+		{group: cmdRemote, mistyped: "ifno"},
+		{group: "database", mistyped: "delte"},
+		{group: "completion", mistyped: "nosuchshell"},
+	} {
+		t.Run(testCase.group, func(t *testing.T) {
+			code, stdout, stderr := runEntry(t,
+				testCase.group, testCase.mistyped)
+
+			assert.Equal(t, 1, code)
+			assert.Empty(t, stdout)
+			assert.Contains(t, stderr,
+				`unknown command "`+testCase.mistyped+`"`)
+
+			code, stdout, _ = runEntry(t, testCase.group)
+
+			assert.Equal(t, 0, code)
+			assert.Contains(t, stdout,
+				programName+" "+testCase.group+" [command]")
+		})
+	}
+}

internal/cli/remote.go

@@ -18,6 +18,12 @@ func NewRemoteCommand() *cobra.Command {
 		Use:   "remote",
 		Short: "Remote storage management commands",
 		Long:  "Commands for inspecting and managing remote storage",
+		// cobra checks Args only on a command that has a Run: without
+		// one, a mistyped subcommand printed help and exited 0.
+		Args: cobra.NoArgs,
+		Run: func(cmd *cobra.Command, _ []string) {
+			_ = cmd.Help()
+		},
 	}
 
 	// Add subcommands

internal/cli/root.go

@@ -74,6 +74,21 @@ on the source system.`,
 		NewDatabaseCommand(),
 	)
 
+	// cobra adds the completion command only when Execute runs, and with
+	// no Run, so it skipped its own cobra.NoArgs and a mistyped shell name
+	// printed help and exited 0. Adding it here lets it have a Run. Its
+	// scripts go to os.Stdout as it is now: a later SetOut does not
+	// redirect them.
+	cmd.InitDefaultCompletionCmd()
+
+	for _, sub := range cmd.Commands() {
+		if sub.Name() == "completion" {
+			sub.Run = func(completion *cobra.Command, _ []string) {
+				_ = completion.Help()
+			}
+		}
+	}
+
 	return cmd
 }
 

internal/cli/snapshot.go

@@ -41,6 +41,12 @@ func NewSnapshotCommand() *cobra.Command {
 		Use:   "snapshot",
 		Short: "Snapshot management commands",
 		Long:  "Commands for creating, listing, and managing snapshots",
+		// cobra checks Args only on a command that has a Run: without
+		// one, a mistyped subcommand printed help and exited 0.
+		Args: cobra.NoArgs,
+		Run: func(cmd *cobra.Command, _ []string) {
+			_ = cmd.Help()
+		},
 	}
 
 	// Add subcommands

internal/cli/snapshot_restore.go

@@ -30,8 +30,6 @@ This command will restore files from the specified snapshot to the
 target directory.
 If no paths are specified, all files are restored.
 If paths are specified, only matching files/directories are restored.
-A path that matches nothing in the snapshot is named in a warning, and
-the command exits non-zero after restoring the paths that match.
 
 The snapshot may be named by its ID or, when restoring on a host with no
 local index, by the remote key that 'snapshot list' prints for a

internal/config/config.go

@@ -61,6 +61,9 @@ var (
 	errBadS3PartSize    = errors.New("s3.part_size must be between 5MiB and 5GiB")
 	errBadStorageScheme = errors.New(
 		"storage_url must start with s3://, file://, or rclone://")
+	errFileURLNotAbsolute = errors.New(
+		"a file:// storage_url needs an absolute path, " +
+			"as in file:///path/to/backup")
 	errStorageNotConfigured = errors.New(
 		"storage not configured; set storage_url or provide s3.endpoint + " +
 			"s3.bucket + credentials")
@@ -461,11 +464,18 @@ func (c *Config) validateStorage() error {
 
 // validateStorageURL validates URL-based storage configuration. File and
 // rclone URLs need no credentials; S3 URLs require the legacy s3.*
-// credential fields.
+// credential fields. A file URL must name an absolute path: a relative one
+// would resolve against the working directory, so the destination would
+// move with it.
 func (c *Config) validateStorageURL() error {
 	switch {
 	case strings.HasPrefix(c.StorageURL, "file://"):
-		// File storage doesn't need S3 credentials
+		// storage.ParseStorageURL applies the same rule, but config
+		// cannot import storage, which imports config.
+		if !strings.HasPrefix(c.StorageURL, "file:///") {
+			return errFileURLNotAbsolute
+		}
+
 		return nil
 	case strings.HasPrefix(c.StorageURL, "rclone://"):
 		// Rclone storage uses rclone's own config

internal/config/config_test.go

@@ -337,6 +337,35 @@ func TestLoadS3PartSize(t *testing.T) {
 	})
 }
 
+// TestLoadRejectsRelativeFileURL checks that a file:// storage_url with a
+// relative path fails at load. Accepted, it would resolve against the
+// working directory, so a backup run from another directory would write
+// to a different destination and still succeed.
+func TestLoadRejectsRelativeFileURL(t *testing.T) {
+	t.Parallel()
+
+	const relativeConfig = "snapshots:\n" +
+		"  test:\n" +
+		"    paths: [/tmp/vaultik-test-source]\n" +
+		"storage_url: file://dest\n"
+
+	path := filepath.Join(t.TempDir(), "config.yml")
+
+	err := os.WriteFile(path, []byte(relativeConfig), 0o600)
+	if err != nil {
+		t.Fatalf("write config: %v", err)
+	}
+
+	_, err = Load(path)
+	if !errors.Is(err, errFileURLNotAbsolute) {
+		t.Fatalf("Load() error = %v, want errFileURLNotAbsolute", err)
+	}
+
+	if !strings.Contains(err.Error(), "absolute path") {
+		t.Errorf("error %q does not say to use an absolute path", err.Error())
+	}
+}
+
 // TestValidateAgeRecipients checks that recipients are parsed at config load
 // (a bad entry fails immediately, not mid-backup) and that no invalid entry —
 // least of all a pasted secret key — is echoed in the error. An empty list

internal/snapshot/exclude_test.go

@@ -531,3 +531,50 @@ func TestExcludePatterns_UnanchoredPatternFile(t *testing.T) {
 	require.Equal(t, 1, result.FilesScanned,
 		"Unanchored pattern for file should match anywhere")
 }
+
+func TestExcludePatterns_DoubleStarDirectoryAtEveryDepth(t *testing.T) {
+	t.Parallel()
+
+	// As in gitignore, a leading **/ also matches directly under the source dir
+	fs := setupAnchoredTestFS(t)
+
+	scanner, repos, cleanup := createTestScanner(t, fs, []string{"**/projectname"})
+	defer cleanup()
+
+	ctx := context.Background()
+	createSnapshotRecord(ctx, t, repos, "test-snapshot")
+
+	result, err := scanner.Scan(ctx, "/backup", "test-snapshot")
+	require.NoError(t, err)
+
+	// **/projectname should exclude BOTH:
+	// - /backup/projectname/file.txt
+	// - /backup/otherproject/projectname/file.txt
+	// Total files: 4, excluded: 2
+	require.Equal(t, 2, result.FilesScanned,
+		"**/ directory pattern should match at the top level and nested")
+}
+
+func TestExcludePatterns_DoubleStarFileAtEveryDepth(t *testing.T) {
+	t.Parallel()
+
+	// As in gitignore, a leading **/ also matches directly under the source dir
+	fs := setupAnchoredTestFS(t)
+
+	scanner, repos, cleanup := createTestScanner(t, fs, []string{"**/*.txt"})
+	defer cleanup()
+
+	ctx := context.Background()
+	createSnapshotRecord(ctx, t, repos, "test-snapshot")
+
+	result, err := scanner.Scan(ctx, "/backup", "test-snapshot")
+	require.NoError(t, err)
+
+	// **/*.txt should exclude ALL .txt files:
+	// - /backup/file.txt
+	// - /backup/projectname/file.txt
+	// - /backup/otherproject/projectname/file.txt
+	// Total files: 4, excluded: 3
+	require.Equal(t, 1, result.FilesScanned,
+		"**/ file pattern should match at the top level and nested")
+}

internal/snapshot/scanner.go

@@ -1295,6 +1295,11 @@ func (s *Scanner) batchAddFilesToSnapshot(
 func (s *Scanner) processPhase(
 	ctx context.Context, filesToProcess []*FileToProcess, result *ScanResult,
 ) error {
+	// On an error or cancellation the blob being packed is never
+	// finalized, so its temp file must be removed here. After a
+	// successful flush there is no such blob and this does nothing.
+	defer s.packer.DiscardCurrentBlob()
+
 	// Calculate total bytes to process
 	var totalBytes int64
 	for _, f := range filesToProcess {
@@ -2013,6 +2018,14 @@ func compileExcludePatterns(patterns []string) []compiledPattern {
 		// 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. An unanchored pattern
+		// already matches at every depth, so drop the prefix: as in
+		// gitignore, "**/x" then also matches a top-level "x".
+		if !anchored {
+			pattern = strings.TrimPrefix(pattern, "**/")
+		}
+
 		// Compile the glob pattern
 		// For patterns without path separators, we need to match them as components
 		// e.g., ".git" should match ".git" anywhere in the path

internal/snapshot/skip_errors_test.go

@@ -381,3 +381,60 @@ func TestScannerCancelStopsSkipErrorsRun(t *testing.T) {
 		})
 	}
 }
+
+// TestScannerRemovesBlobTempFileWhenRunStops checks that a run stopped
+// part-way, by a file it cannot read or by a cancellation, removes the
+// temp file of the blob it was packing.
+func TestScannerRemovesBlobTempFileWhenRunStops(t *testing.T) {
+	t.Parallel()
+
+	// a.txt is backed up first and starts a blob; the run stops at b.txt.
+	const target = "/source/b.txt"
+
+	tests := []struct {
+		name  string
+		newFs func(cancel context.CancelFunc) afero.Fs
+	}{
+		{
+			name: "file cannot be read",
+			newFs: func(context.CancelFunc) afero.Fs {
+				return &readFailFs{Fs: afero.NewMemMapFs(), target: target}
+			},
+		},
+		{
+			name: "cancelled",
+			newFs: func(cancel context.CancelFunc) afero.Fs {
+				return &cancelOnOpenFs{
+					Fs: afero.NewMemMapFs(), target: target, cancel: cancel,
+				}
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+
+			fs := tt.newFs(cancel)
+			writeSkipErrorTestFile(t, fs, "/source/a.txt", "first file content")
+			writeSkipErrorTestFile(t, fs, target, "second file content")
+
+			_, err := runSkipErrorScan(ctx, t, fs, "/source", false, nil)
+			if err == nil {
+				t.Fatal("expected the scan to stop with an error, got nil")
+			}
+
+			left, err := afero.Glob(fs, filepath.Join(os.TempDir(), "vaultik-blob-*"))
+			if err != nil {
+				t.Fatalf("listing blob temp files: %v", err)
+			}
+
+			if len(left) != 0 {
+				t.Fatalf("expected no blob temp file left, got %v", left)
+			}
+		})
+	}
+}

internal/storage/url.go

@@ -17,8 +17,10 @@ const (
 
 // Sentinel errors for storage URL parsing.
 var (
-	ErrEmptyStorageURL   = errors.New("storage URL is empty")
-	ErrEmptyFilePath     = errors.New("file URL path is empty")
+	ErrEmptyStorageURL  = errors.New("storage URL is empty")
+	ErrEmptyFilePath    = errors.New("file URL path is empty")
+	ErrRelativeFilePath = errors.New(
+		"file URL needs an absolute path, as in file:///path/to/backup")
 	ErrMissingBucket     = errors.New("s3 URL missing bucket name")
 	ErrMissingRemote     = errors.New("rclone URL missing remote name")
 	ErrUnsupportedScheme = errors.New(
@@ -51,17 +53,8 @@ func ParseStorageURL(rawURL string) (*URL, error) {
 		return nil, ErrEmptyStorageURL
 	}
 
-	// Handle file:// URLs
 	if after, ok := strings.CutPrefix(rawURL, "file://"); ok {
-		path := after
-		if path == "" {
-			return nil, ErrEmptyFilePath
-		}
-
-		return &URL{
-			Scheme: schemeFile,
-			Prefix: path,
-		}, nil
+		return parseFileURL(after)
 	}
 
 	if strings.HasPrefix(rawURL, "s3://") {
@@ -75,6 +68,27 @@ func ParseStorageURL(rawURL string) (*URL, error) {
 	return nil, ErrUnsupportedScheme
 }
 
+// parseFileURL parses the path that follows file://. The path must be
+// absolute: a relative one would resolve against the working directory,
+// so the destination would move with it. Percent-escapes are decoded, so
+// my%20stick names the directory "my stick"; a literal space is kept.
+func parseFileURL(rawPath string) (*URL, error) {
+	if rawPath == "" {
+		return nil, ErrEmptyFilePath
+	}
+
+	if !strings.HasPrefix(rawPath, "/") {
+		return nil, ErrRelativeFilePath
+	}
+
+	path, err := url.PathUnescape(rawPath)
+	if err != nil {
+		return nil, fmt.Errorf("invalid file URL path: %w", err)
+	}
+
+	return &URL{Scheme: schemeFile, Prefix: path}, nil
+}
+
 // parseS3URL parses an s3://bucket/prefix URL. It rejects credentials in
 // the userinfo and any query parameter other than endpoint, region and
 // ssl, so a credential-bearing URL is never stored or echoed.

internal/storage/url_parse_test.go

@@ -14,7 +14,10 @@ import (
 func TestParseStorageURLValid(t *testing.T) {
 	t.Parallel()
 
-	const bucket = "mybucket"
+	const (
+		bucket     = "mybucket"
+		fileScheme = "file"
+	)
 
 	cases := []struct {
 		name string
@@ -24,7 +27,17 @@ func TestParseStorageURLValid(t *testing.T) {
 		{
 			name: "file absolute path",
 			raw:  "file:///var/backups/vaultik",
-			want: &storage.URL{Scheme: "file", Prefix: "/var/backups/vaultik"},
+			want: &storage.URL{Scheme: fileScheme, Prefix: "/var/backups/vaultik"},
+		},
+		{
+			name: "file percent-escaped path is decoded",
+			raw:  "file:///Volumes/my%20stick/backup",
+			want: &storage.URL{Scheme: fileScheme, Prefix: "/Volumes/my stick/backup"},
+		},
+		{
+			name: "file path with a literal space",
+			raw:  "file:///Volumes/my stick/backup",
+			want: &storage.URL{Scheme: fileScheme, Prefix: "/Volumes/my stick/backup"},
 		},
 		{
 			name: "s3 bucket and prefix, ssl defaults on",
@@ -90,6 +103,7 @@ func TestParseStorageURLErrors(t *testing.T) {
 	}{
 		{"empty url", "", storage.ErrEmptyStorageURL},
 		{"file empty path", "file://", storage.ErrEmptyFilePath},
+		{"file relative path", "file://dest", storage.ErrRelativeFilePath},
 		{"s3 missing bucket", "s3://", storage.ErrMissingBucket},
 		{"s3 missing bucket with path", "s3:///justprefix", storage.ErrMissingBucket},
 		{"rclone missing remote", "rclone://", storage.ErrMissingRemote},

internal/vaultik/restore.go

@@ -27,7 +27,6 @@ import (
 // Sentinel errors for restore failures.
 var (
 	errFilesFailedRestore    = errors.New("file(s) failed to restore")
-	errPathsNotInSnapshot    = errors.New("requested path(s) not in the snapshot")
 	errFilesFailedVerify     = errors.New("files failed verification")
 	errDecryptionKeyRequired = errors.New(
 		"decryption key required for restore\n\n" +
@@ -150,7 +149,7 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
 	repos := database.NewRepositories(tempDB)
 
 	// Step 2: Get list of files to restore
-	files, unmatched, err := v.getFilesToRestore(v.ctx, repos, opts.Paths)
+	files, err := v.getFilesToRestore(v.ctx, repos, opts.Paths)
 	if err != nil {
 		return fmt.Errorf("getting files to restore: %w", err)
 	}
@@ -159,7 +158,7 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
 		log.Warn("No files found to restore")
 		v.UI.Warningf("No files found to restore.")
 
-		return v.reportUnmatchedPaths(unmatched)
+		return nil
 	}
 
 	log.Info("Found files to restore", "count", len(files))
@@ -199,16 +198,14 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
 		v.UI.Duration(result.Duration),
 	)
 
-	return v.finishRestore(repos, files, unmatched, opts, result)
+	return v.finishRestore(repos, files, opts, result)
 }
 
 // finishRestore emits the post-restore warnings, runs optional
-// verification, and converts any failed-file count or requested path
-// that matched nothing into an error.
+// verification, and converts any failed-file count into an error.
 func (v *Vaultik) finishRestore(
 	repos *database.Repositories,
 	files []*database.File,
-	unmatched []string,
 	opts *RestoreOptions,
 	result *RestoreResult,
 ) error {
@@ -226,8 +223,6 @@ func (v *Vaultik) finishRestore(
 		}
 	}
 
-	unmatchedErr := v.reportUnmatchedPaths(unmatched)
-
 	// Run verification if requested
 	if opts.Verify {
 		err := v.handleRestoreVerification(repos, files, opts, result)
@@ -240,23 +235,7 @@ func (v *Vaultik) finishRestore(
 		return fmt.Errorf("%d %w", result.FilesFailed, errFilesFailedRestore)
 	}
 
-	return unmatchedErr
-}
-
-// reportUnmatchedPaths warns, by name, about each requested path that
-// matched nothing in the snapshot, and returns an error counting them,
-// or nil when there are none.
-func (v *Vaultik) reportUnmatchedPaths(unmatched []string) error {
-	if len(unmatched) == 0 {
-		return nil
-	}
-
-	for _, path := range unmatched {
-		log.Warn("Requested path matches nothing in the snapshot", "path", path)
-		v.UI.Warningf("%s matches nothing in the snapshot.", v.UI.Path(path))
-	}
-
-	return fmt.Errorf("%d %w", len(unmatched), errPathsNotInSnapshot)
+	return nil
 }
 
 // restoreIdentities parses the configured age secret key once into every
@@ -833,34 +812,27 @@ func (v *Vaultik) materializeSnapshotDB(
 	return db, tempDir, nil
 }
 
-// getFilesToRestore returns the list of files to restore based on path
-// filters, and the filters, as given, that matched no file.
+// getFilesToRestore returns the list of files to restore based on path filters
 func (v *Vaultik) getFilesToRestore(
 	ctx context.Context, repos *database.Repositories, pathFilters []string,
-) ([]*database.File, []string, error) {
+) ([]*database.File, error) {
 	// If no filters, get all files
 	if len(pathFilters) == 0 {
-		files, err := repos.Files.ListAll(ctx)
-
-		return files, nil, err
+		return repos.Files.ListAll(ctx)
 	}
 
 	// Get files matching the path filters
-	var (
-		result    []*database.File
-		unmatched []string
-	)
+	var result []*database.File
 
 	seen := make(map[string]bool)
 
 	for _, filter := range pathFilters {
-		files, err := repos.Files.ListUnderPath(ctx, filepath.Clean(filter))
-		if err != nil {
-			return nil, nil, fmt.Errorf("listing files under %s: %w", filter, err)
-		}
+		// Normalize the filter path
+		filter = filepath.Clean(filter)
 
-		if len(files) == 0 {
-			unmatched = append(unmatched, filter)
+		files, err := repos.Files.ListUnderPath(ctx, filter)
+		if err != nil {
+			return nil, fmt.Errorf("listing files under %s: %w", filter, err)
 		}
 
 		for _, file := range files {
@@ -871,7 +843,7 @@ func (v *Vaultik) getFilesToRestore(
 		}
 	}
 
-	return result, unmatched, nil
+	return result, nil
 }
 
 // buildChunkToBlobMap creates a mapping from chunk hash to blob information

internal/vaultik/restore_unmatched_path_test.go

@@ -1,142 +0,0 @@
-package vaultik_test
-
-import (
-	"bytes"
-	"context"
-	"os"
-	"path/filepath"
-	"strings"
-	"testing"
-
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/config"
-	"sneak.berlin/go/vaultik/internal/database"
-	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/storage"
-	"sneak.berlin/go/vaultik/internal/ui"
-	"sneak.berlin/go/vaultik/internal/vaultik"
-)
-
-// These tests cover https://git.eeqj.de/sneak/vaultik/issues/309: a path
-// given to restore that matches nothing in the snapshot is named in a
-// warning and makes the restore fail, after the paths that do match are
-// restored.
-
-func TestRestoreNamesUnmatchedPathAndRestoresTheMatchingOne(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	dataDir := filepath.Join(tempDir, "src")
-	storeDir := filepath.Join(tempDir, "remote")
-	restoreDir := filepath.Join(tempDir, "restored")
-	dbPath := filepath.Join(tempDir, "index.sqlite")
-
-	ctx := context.Background()
-	cfg := faultTestConfig()
-	files := writeFaultSourceTree(t, fs, dataDir)
-
-	store, err := storage.NewFileStorer(storeDir)
-	require.NoError(t, err)
-
-	db, err := database.New(ctx, dbPath)
-	require.NoError(t, err)
-
-	id := fullFaultBackup(ctx, t, fs, store, cfg,
-		database.NewRepositories(db), dataDir, dbPath, "unmatched")
-	require.NoError(t, db.Close())
-
-	matched := filepath.Join(dataDir, "small.txt")
-	unmatched := filepath.Join(dataDir, "typo.txt")
-
-	var out bytes.Buffer
-
-	v := newReaderVaultik(ctx, cfg, store, nil, fs)
-	v.UI = ui.NewWithColor(&out, false)
-
-	err = v.Restore(&vaultik.RestoreOptions{
-		SnapshotID: id,
-		TargetDir:  restoreDir,
-		Paths:      []string{matched, unmatched},
-	})
-
-	require.Error(t, err, "a path that matches nothing must fail the restore")
-	assert.Contains(t, out.String(), unmatched,
-		"the path that matches nothing must be named")
-	assert.NotContains(t, out.String(), matched,
-		"the path that matches must not be reported")
-
-	got, err := afero.ReadFile(fs, filepath.Join(restoreDir, matched))
-	require.NoError(t, err, "the path that matches must still be restored")
-	assert.Equal(t, files[matched], got)
-}
-
-// When no requested path matches, nothing is restored and each path is
-// named. One path is relative. The other goes through the symlink that
-// is the snapshot's configured path, while the entries are stored under
-// the directory the symlink points to.
-func TestRestoreNamesEveryPathWhenNothingMatches(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	const snapshotName = "linked"
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-	dataDir := filepath.Join(tempDir, "src")
-	linkDir := filepath.Join(tempDir, "srclink")
-	storeDir := filepath.Join(tempDir, "remote")
-	restoreDir := filepath.Join(tempDir, "restored")
-	dbPath := filepath.Join(tempDir, "index.sqlite")
-
-	ctx := context.Background()
-	writeFaultSourceTree(t, fs, dataDir)
-	require.NoError(t, os.Symlink(dataDir, linkDir))
-
-	cfg := faultTestConfig()
-	cfg.IndexPath = dbPath
-	cfg.ChunkSize = config.Size(faultChunkSize)
-	cfg.Snapshots = map[string]config.SnapshotConfig{
-		snapshotName: {Paths: []string{linkDir}},
-	}
-
-	store, err := storage.NewFileStorer(storeDir)
-	require.NoError(t, err)
-
-	db, err := database.New(ctx, dbPath)
-	require.NoError(t, err)
-
-	repos := database.NewRepositories(db)
-	v := newBackupVaultik(ctx, cfg, store, repos, db, fs)
-
-	require.NoError(t, backUp(v, snapshotName))
-
-	id := localSnapshotID(ctx, t, repos, snapshotName)
-	require.NoError(t, db.Close())
-
-	throughLink := filepath.Join(linkDir, "sub")
-	relative := strings.TrimPrefix(filepath.Join(dataDir, "small.txt"), "/")
-
-	var out bytes.Buffer
-
-	reader := newReaderVaultik(ctx, cfg, store, nil, fs)
-	reader.UI = ui.NewWithColor(&out, false)
-
-	err = reader.Restore(&vaultik.RestoreOptions{
-		SnapshotID: id,
-		TargetDir:  restoreDir,
-		Paths:      []string{throughLink, relative},
-	})
-
-	require.Error(t, err, "a restore that matches nothing must fail")
-	assert.Contains(t, out.String(), throughLink,
-		"the path through the symlink must be named")
-	assert.Contains(t, out.String(), relative,
-		"the relative path must be named")
-
-	_, err = fs.Stat(restoreDir)
-	assert.True(t, os.IsNotExist(err), "nothing must be restored")
-}