diff --git a/TODO.md b/TODO.md index 4320581..7f6394d 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,13 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-07: Made a symlink whose target cannot be read stop the backup + ([issue #269](https://git.eeqj.de/sneak/vaultik/issues/269)). It was + left out of the snapshot with only a debug log line, even without + `--skip-errors`. It now aborts the run, or with `--skip-errors` is + skipped with the `Failed to access` error line that any other entry + the scan cannot read gets. + - 2026-10-07: Made an interrupted command exit 130 and say so ([issue #267](https://git.eeqj.de/sneak/vaultik/issues/267)). Ctrl-C or SIGTERM during `snapshot create`, `snapshot restore` or `snapshot diff --git a/internal/snapshot/scanner.go b/internal/snapshot/scanner.go index b611857..46765b2 100644 --- a/internal/snapshot/scanner.go +++ b/internal/snapshot/scanner.go @@ -889,9 +889,10 @@ func (s *Scanner) scanPhase( } // Handle symlinks and directories - if handled := s.recordSpecialEntry( - filePath, info, existingFiles, collector, result); handled { - return nil + handled, err := s.recordSpecialEntry( + filePath, info, existingFiles, collector, result) + if handled { + return err } // Skip other non-regular files (devices, sockets, etc.) @@ -933,22 +934,25 @@ func (s *Scanner) scanPhase( } // recordSpecialEntry records symlinks and directories (which have no -// data to chunk) and reports whether it handled the entry. +// data to chunk) and reports whether it handled the entry. For a symlink +// whose target cannot be read it returns handleWalkError's result. func (s *Scanner) recordSpecialEntry( filePath string, info os.FileInfo, existingFiles map[string]struct{}, collector *scanCollector, result *ScanResult, -) bool { +) (bool, error) { // Handle symlinks if info.Mode()&os.ModeSymlink != 0 { - file := s.buildSymlinkEntry(filePath, info) - if file != nil { - existingFiles[filePath] = struct{}{} - collector.addToProcess(filePath, info, file) - s.updateScanEntryStats(result, true, info) + file, err := s.buildSymlinkEntry(filePath, info) + if err != nil { + return true, s.handleWalkError(filePath, err) } - return true + existingFiles[filePath] = struct{}{} + collector.addToProcess(filePath, info, file) + s.updateScanEntryStats(result, true, info) + + return true, nil } // Handle directories (record for permission/ownership preservation @@ -958,10 +962,10 @@ func (s *Scanner) recordSpecialEntry( existingFiles[filePath] = struct{}{} collector.addToProcess(filePath, info, file) - return true + return true, nil } - return false + return false, nil } // handleWalkError deals with a filesystem error surfaced by the walk: @@ -1111,13 +1115,12 @@ func (s *Scanner) printScanProgressLine( } // buildSymlinkEntry creates a File record for a symlink. -// Returns nil if the link target cannot be read. -func (s *Scanner) buildSymlinkEntry(path string, info os.FileInfo) *database.File { +func (s *Scanner) buildSymlinkEntry( + path string, info os.FileInfo, +) (*database.File, error) { target, err := os.Readlink(path) if err != nil { - log.Debug("Cannot read symlink target", "path", path, "error", err) - - return nil + return nil, err } var uid, gid uint32 @@ -1136,7 +1139,7 @@ func (s *Scanner) buildSymlinkEntry(path string, info os.FileInfo) *database.Fil UID: uid, GID: gid, LinkTarget: types.FilePath(target), - } + }, nil } // buildDirectoryEntry creates a File record for a directory. diff --git a/internal/snapshot/skip_errors_test.go b/internal/snapshot/skip_errors_test.go index 9ae128a..8325372 100644 --- a/internal/snapshot/skip_errors_test.go +++ b/internal/snapshot/skip_errors_test.go @@ -3,6 +3,7 @@ package snapshot_test import ( "context" "errors" + "io" "os" "path/filepath" "strings" @@ -13,6 +14,7 @@ import ( "github.com/spf13/afero" "sneak.berlin/go/vaultik/internal/database" "sneak.berlin/go/vaultik/internal/snapshot" + "sneak.berlin/go/vaultik/internal/ui" ) // errSimTempFail is the one-time temp-file creation failure blobTempFailFs @@ -80,6 +82,30 @@ func (f *readFailFs) Open(name string) (afero.File, error) { return file, nil } +// linkRemovedAfterLstatFs is the real filesystem, except that the symlink at +// target is removed right after the walk lstats it, as happens when a link is +// deleted during a backup. The scanner's readlink of it then fails. +type linkRemovedAfterLstatFs struct { + afero.OsFs + + t *testing.T + target string +} + +func (f *linkRemovedAfterLstatFs) LstatIfPossible( + name string, +) (os.FileInfo, bool, error) { + info, lstatCalled, err := f.OsFs.LstatIfPossible(name) + if err == nil && name == f.target { + rmErr := os.Remove(name) + if rmErr != nil { + f.t.Errorf("removing %s: %v", name, rmErr) + } + } + + return info, lstatCalled, err +} + // writeSkipErrorTestFile writes one file into fs with a fixed mtime. func writeSkipErrorTestFile(t *testing.T, fs afero.Fs, path, content string) { t.Helper() @@ -102,10 +128,11 @@ func writeSkipErrorTestFile(t *testing.T, fs afero.Fs, path, content string) { } } -// runSkipErrorScan scans /source on fs with the given skip-errors setting and -// returns the repositories (for inspection) and the scan error. +// runSkipErrorScan scans source on fs with the given skip-errors setting, +// printing user-facing messages to uiw (nil discards them), and returns the +// repositories (for inspection) and the scan error. func runSkipErrorScan( - t *testing.T, fs afero.Fs, skipErrors bool, + t *testing.T, fs afero.Fs, source string, skipErrors bool, uiw *ui.Writer, ) (*database.Repositories, error) { t.Helper() @@ -130,6 +157,7 @@ func runSkipErrorScan( MaxBlobSize: int64(1024 * 1024), CompressionLevel: 3, AgeRecipients: []string{testAgePublicKey}, + UI: uiw, SkipErrors: skipErrors, }) @@ -137,7 +165,7 @@ func runSkipErrorScan( snapshotID := "test-snapshot-skip-errors" createTestSnapshotRecord(ctx, t, repos, snapshotID) - _, err = scanner.Scan(ctx, "/source", snapshotID) + _, err = scanner.Scan(ctx, source, snapshotID) return repos, err } @@ -157,7 +185,7 @@ func TestScannerPackingFailureAbortsUnderSkipErrors(t *testing.T) { writeSkipErrorTestFile(t, fs, "/source/file1.txt", "first file content") writeSkipErrorTestFile(t, fs, "/source/file2.txt", "second file content") - repos, err := runSkipErrorScan(t, fs, true) + repos, err := runSkipErrorScan(t, fs, "/source", true, nil) if err == nil { t.Fatal("expected scan to abort on the packer error, got nil") } @@ -184,7 +212,7 @@ func TestScannerReadErrorAbortsWithoutSkipErrors(t *testing.T) { fs := &readFailFs{Fs: afero.NewMemMapFs(), target: target} writeSkipErrorTestFile(t, fs, target, "content that cannot be read") - _, err := runSkipErrorScan(t, fs, false) + _, err := runSkipErrorScan(t, fs, "/source", false, nil) if err == nil { t.Fatal("expected scan to fail on the read error, got nil") } @@ -200,7 +228,7 @@ func TestScannerReadErrorSkippedWithSkipErrors(t *testing.T) { fs := &readFailFs{Fs: afero.NewMemMapFs(), target: target} writeSkipErrorTestFile(t, fs, target, "content that cannot be read") - repos, err := runSkipErrorScan(t, fs, true) + repos, err := runSkipErrorScan(t, fs, "/source", true, nil) if err != nil { t.Fatalf("expected scan to complete with --skip-errors, got %v", err) } @@ -214,3 +242,63 @@ func TestScannerReadErrorSkippedWithSkipErrors(t *testing.T) { t.Fatalf("expected unreadable file skipped, got %d chunks", len(chunks)) } } + +// writeSymlinkSource creates a source directory on disk holding one symlink +// and returns the directory and the symlink's path. +func writeSymlinkSource(t *testing.T) (string, string) { + t.Helper() + + sourceDir := t.TempDir() + linkPath := filepath.Join(sourceDir, "link") + + err := os.Symlink("target.txt", linkPath) + if err != nil { + t.Fatalf("creating symlink: %v", err) + } + + return sourceDir, linkPath +} + +// TestScannerUnreadableSymlinkAbortsWithoutSkipErrors checks that a symlink +// whose target cannot be read aborts the run when --skip-errors is not set. +func TestScannerUnreadableSymlinkAbortsWithoutSkipErrors(t *testing.T) { + t.Parallel() + + sourceDir, linkPath := writeSymlinkSource(t) + fs := &linkRemovedAfterLstatFs{t: t, target: linkPath} + + _, err := runSkipErrorScan(t, fs, sourceDir, false, nil) + if !errors.Is(err, os.ErrNotExist) { + t.Fatalf("expected scan to fail on the removed symlink, got %v", err) + } +} + +// TestScannerUnreadableSymlinkSkippedWithSkipErrors checks that a symlink +// whose target cannot be read is skipped with an error line, and the run +// completes, when --skip-errors is set. +func TestScannerUnreadableSymlinkSkippedWithSkipErrors(t *testing.T) { + t.Parallel() + + sourceDir, linkPath := writeSymlinkSource(t) + fs := &linkRemovedAfterLstatFs{t: t, target: linkPath} + uiw := ui.NewWithColor(io.Discard, false) + + repos, err := runSkipErrorScan(t, fs, sourceDir, true, uiw) + if err != nil { + t.Fatalf("expected scan to complete with --skip-errors, got %v", err) + } + + if uiw.ErrorCount() != 1 { + t.Fatalf("expected one error line for the symlink, got %d", + uiw.ErrorCount()) + } + + file, err := repos.Files.GetByPath(context.Background(), linkPath) + if err != nil { + t.Fatalf("getting %s: %v", linkPath, err) + } + + if file != nil { + t.Fatalf("expected %s not to be recorded", linkPath) + } +}