diff --git a/internal/cli/app.go b/internal/cli/app.go index bd54f1d..71d1159 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -7,11 +7,9 @@ import ( "context" "errors" "fmt" - "path/filepath" "sync" "time" - "github.com/adrg/xdg" "github.com/spf13/cobra" "go.uber.org/dig" "go.uber.org/fx" @@ -351,8 +349,7 @@ func runVaultikApp( // only one runs at a time; a read-only command runs without it and is not // blocked while a mutator holds the lock (opts.Mode). func RunWithApp(ctx context.Context, opts AppOptions) error { - release, err := acquireLockIfMutating(opts.Mode, - filepath.Join(xdg.DataHome, "vaultik")) + release, err := acquireLockIfMutating(opts.Mode, pidlock.Dir()) if err != nil { return err } diff --git a/internal/cli/entry_snapshot_create_own_files_test.go b/internal/cli/entry_snapshot_create_own_files_test.go new file mode 100644 index 0000000..c448039 --- /dev/null +++ b/internal/cli/entry_snapshot_create_own_files_test.go @@ -0,0 +1,69 @@ +package cli //nolint:testpackage // shares runEntry and the hermetic config helpers + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/adrg/xdg" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestEntrySnapshotCreateSkipsOwnFiles backs up a directory that holds +// vaultik's index and PID file beside one other file, with no exclude +// pattern set. The summary counts only the other file. See +// https://git.eeqj.de/sneak/vaultik/issues/421. +// +// Not parallel: it replaces os.Args, os.Stdout, os.Stderr and the xdg +// globals. +func TestEntrySnapshotCreateSkipsOwnFiles(t *testing.T) { + for _, testCase := range []struct { + name string + // linked sets index_path to a symlink outside the backed-up + // directory that points to the index inside it. + linked bool + }{ + {name: "index in the backed-up directory"}, + {name: "index_path links into the backed-up directory", linked: true}, + } { + t.Run(testCase.name, func(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.yml") + sourceDir := filepath.Join(dir, "source") + storeDir := filepath.Join(dir, "store") + + indexPath := filepath.Join(sourceDir, "index.sqlite") + + if testCase.linked { + link := filepath.Join(dir, "index.sqlite") + require.NoError(t, os.Symlink(indexPath, link)) + + indexPath = link + } + + contents := fmt.Sprintf(hermeticConfig, + sourceDir, storeDir, indexPath) + require.NoError(t, + os.WriteFile(configPath, []byte(contents), configFileMode)) + require.NoError(t, os.Mkdir(storeDir, 0o750)) + require.NoError(t, os.Mkdir(sourceDir, 0o750)) + require.NoError(t, os.WriteFile( + filepath.Join(sourceDir, "file.txt"), []byte("hello"), 0o600)) + + // The PID file goes in a directory under XDG_DATA_HOME, which + // xdg reads at package init. + t.Setenv("XDG_DATA_HOME", filepath.Join(sourceDir, "data")) + xdg.Reload() + t.Cleanup(xdg.Reload) + + code, stdout, _ := runEntry(t, + flagConfig, configPath, cmdSnapshot, cmdCreate) + + require.Equal(t, 0, code) + assert.Contains(t, stdout, + "Files: 1 examined, 1 backed up, 0 unchanged.") + }) + } +} diff --git a/internal/database/module.go b/internal/database/module.go index 65b04b0..b4035be 100644 --- a/internal/database/module.go +++ b/internal/database/module.go @@ -20,6 +20,17 @@ const indexDirPerm = 0o700 // backed-up path and chunk hash and must stay private. const indexFilePerm = 0o600 +// IndexFiles returns the path of the index at indexPath and the paths of +// the side files SQLite keeps beside it while it writes. +func IndexFiles(indexPath string) []string { + return []string{ + indexPath, + indexPath + "-wal", + indexPath + "-shm", + indexPath + "-journal", + } +} + // ensureIndexFileMode makes the index file owner-only before the SQLite // driver opens it: it creates the file 0600 if absent, or chmods an existing // one to 0600. Doing this first matters because SQLite creates its -wal and diff --git a/internal/pidlock/pidlock.go b/internal/pidlock/pidlock.go index 279b233..1d7145b 100644 --- a/internal/pidlock/pidlock.go +++ b/internal/pidlock/pidlock.go @@ -11,6 +11,7 @@ import ( "strconv" "strings" + "github.com/adrg/xdg" "golang.org/x/sys/unix" ) @@ -29,8 +30,20 @@ const ( lockDirPerm = 0o700 // pidFilePerm is the mode for the PID file (owner-only). pidFilePerm = 0o600 + // pidFileName is the name of the PID file in the lock directory. + pidFileName = "vaultik.pid" ) +// Dir returns the directory that holds vaultik's PID file. +func Dir() string { + return filepath.Join(xdg.DataHome, "vaultik") +} + +// Path returns the path of the PID file that Acquire uses in lockDir. +func Path(lockDir string) string { + return filepath.Join(lockDir, pidFileName) +} + // Acquire attempts to acquire a PID lock in the specified directory. // If another process holds the lock, it returns ErrAlreadyRunning with // that process's PID. On success, it writes the current PID to the lock @@ -42,7 +55,7 @@ func Acquire(lockDir string) (*Lock, error) { return nil, fmt.Errorf("creating lock directory: %w", err) } - lockPath := filepath.Join(lockDir, "vaultik.pid") + lockPath := Path(lockDir) // No O_TRUNC: the file may hold the PID of the process that has the // lock, which the error below reports. diff --git a/internal/snapshot/module.go b/internal/snapshot/module.go index f83ac07..16bde51 100644 --- a/internal/snapshot/module.go +++ b/internal/snapshot/module.go @@ -2,11 +2,13 @@ package snapshot import ( "os" + "path/filepath" "github.com/spf13/afero" "go.uber.org/fx" "sneak.berlin/go/vaultik/internal/config" "sneak.berlin/go/vaultik/internal/database" + "sneak.berlin/go/vaultik/internal/pidlock" "sneak.berlin/go/vaultik/internal/storage" "sneak.berlin/go/vaultik/internal/ui" ) @@ -62,9 +64,51 @@ func provideScannerFactory( EnableProgress: params.EnableProgress, UI: params.UI, Exclude: excludes, + OwnFiles: ownFiles(cfg.IndexPath, pidlock.Dir()), SkipErrors: params.SkipErrors, PreviousSnapshotID: params.PreviousSnapshotID, DeviceNumber: params.DeviceNumber, }) } } + +// ownFiles returns the paths of the index at indexPath, its side files +// and the PID file in lockDir. Symlinks in their directories are resolved, +// as they are in the snapshot's source paths, so the scan finds them under +// the same names. If indexPath is a symlink, the file it points to and +// that file's side files are returned too. +func ownFiles(indexPath, lockDir string) []string { + paths := append(database.IndexFiles(indexPath), pidlock.Path(lockDir)) + for i, p := range paths { + paths[i] = resolveDir(p) + } + + // SQLite writes the file a symlinked index points to and keeps the + // side files beside that file. The database package creates the index + // before the scanner is built, so the link resolves. + index := resolveDir(indexPath) + + target, err := filepath.EvalSymlinks(index) + if err == nil && target != index { + paths = append(paths, database.IndexFiles(target)...) + } + + return paths +} + +// resolveDir returns the absolute form of path with the symlinks in its +// directory resolved. If the directory cannot be resolved, it returns the +// absolute form unresolved. If path has no absolute form, it returns path. +func resolveDir(path string) string { + abs, err := filepath.Abs(path) + if err != nil { + return path + } + + dir, err := filepath.EvalSymlinks(filepath.Dir(abs)) + if err != nil { + return abs + } + + return filepath.Join(dir, filepath.Base(abs)) +} diff --git a/internal/snapshot/module_test.go b/internal/snapshot/module_test.go new file mode 100644 index 0000000..dc143aa --- /dev/null +++ b/internal/snapshot/module_test.go @@ -0,0 +1,32 @@ +//nolint:testpackage // exercises the unexported ownFiles helper +package snapshot + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +// The scan walks source paths with symlinks resolved, so vaultik's own +// files are named the same way, or a symlinked home would back them up. +func TestOwnFilesResolvesSymlinkedDirectory(t *testing.T) { + t.Parallel() + + realDir, err := filepath.EvalSymlinks(t.TempDir()) + require.NoError(t, err) + + link := filepath.Join(t.TempDir(), "link") + require.NoError(t, os.Symlink(realDir, link)) + + paths := ownFiles(filepath.Join(link, "index.sqlite"), link) + + require.Equal(t, []string{ + filepath.Join(realDir, "index.sqlite"), + filepath.Join(realDir, "index.sqlite-wal"), + filepath.Join(realDir, "index.sqlite-shm"), + filepath.Join(realDir, "index.sqlite-journal"), + filepath.Join(realDir, "vaultik.pid"), + }, paths) +} diff --git a/internal/snapshot/own_files_test.go b/internal/snapshot/own_files_test.go new file mode 100644 index 0000000..769e6d8 --- /dev/null +++ b/internal/snapshot/own_files_test.go @@ -0,0 +1,65 @@ +package snapshot_test + +import ( + "context" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/database" + "sneak.berlin/go/vaultik/internal/snapshot" +) + +// A backup of the directory that holds vaultik's index and PID file +// stores neither, with no exclude pattern set, and still stores the +// files beside them. See https://git.eeqj.de/sneak/vaultik/issues/421. +func TestScanSkipsOwnFiles(t *testing.T) { + t.Parallel() + + const dataDir = "/home/user/.local/share/vaultik" + + ownFiles := append(database.IndexFiles(dataDir+"/index.sqlite"), + dataDir+"/vaultik.pid") + + fs := afero.NewMemMapFs() + for _, p := range append(ownFiles, dataDir+"/notes.txt") { + require.NoError(t, afero.WriteFile(fs, p, []byte("data"), 0o600)) + } + + db, err := database.NewTestDB() + require.NoError(t, err) + + defer func() { _ = db.Close() }() + + repos := database.NewRepositories(db) + + scanner := snapshot.NewScanner(snapshot.ScannerConfig{ + FS: fs, + ChunkSize: 64 * 1024, + Repositories: repos, + MaxBlobSize: 1024 * 1024, + CompressionLevel: 3, + AgeRecipients: []string{ + "age1ql3z7hjy54pw3hyww5ayyfg7zqgvc7w3j2elw8zmrj2kg5sfn9aqmcac8p"}, + OwnFiles: ownFiles, + }) + require.NotNil(t, scanner) + + ctx := context.Background() + createSnapshotRecord(ctx, t, repos, "test-snapshot") + + result, err := scanner.Scan(ctx, "/home/user", "test-snapshot") + require.NoError(t, err) + + require.Equal(t, 1, result.FilesScanned) + + stored, err := repos.Files.GetByPath(ctx, dataDir+"/notes.txt") + require.NoError(t, err) + require.NotNil(t, stored, "a file beside the index is stored") + + for _, p := range ownFiles { + f, err := repos.Files.GetByPath(ctx, p) + require.NoError(t, err) + require.Nil(t, f, "%s is not stored", p) + } +} diff --git a/internal/snapshot/scanner.go b/internal/snapshot/scanner.go index de8db7e..2f8094c 100644 --- a/internal/snapshot/scanner.go +++ b/internal/snapshot/scanner.go @@ -67,7 +67,10 @@ type Scanner struct { currentSourcePath string exclude []string // Glob patterns for files/directories to exclude compiledExclude []compiledPattern // Compiled glob patterns - progress *ProgressReporter + // ownFiles holds the paths of vaultik's own working files, which the + // scan treats as excluded (ScannerConfig.OwnFiles). + ownFiles map[string]struct{} + progress *ProgressReporter // skipErrors skips files that cannot be opened or read (logged loudly); // packer, database, encryption, and upload errors still abort the run. skipErrors bool @@ -133,6 +136,11 @@ type ScannerConfig struct { EnableProgress bool // Enable the live progress reporter (ETAs, throughput) UI *ui.Writer // Where user-facing scanner messages go; nil = discard Exclude []string // Glob patterns for files/directories to exclude + // OwnFiles are the absolute paths of vaultik's own working files: the + // index, its side files and the PID file. The scan treats them as + // excluded, whatever Exclude says, since a copy taken while they are + // being written is torn. + OwnFiles []string // SkipErrors skips files that cannot be opened or read (log loudly but // continue); packer, database, encryption, and upload errors still abort. SkipErrors bool @@ -226,6 +234,11 @@ func NewScanner(cfg ScannerConfig) *Scanner { deviceNumber = deviceNumberFromStat } + ownFiles := make(map[string]struct{}, len(cfg.OwnFiles)) + for _, p := range cfg.OwnFiles { + ownFiles[filepath.Clean(p)] = struct{}{} + } + return &Scanner{ fs: cfg.FS, chunker: chunker.NewChunker(cfg.ChunkSize), @@ -237,6 +250,7 @@ func NewScanner(cfg ScannerConfig) *Scanner { ageRecipient: strings.Join(cfg.AgeRecipients, ","), exclude: cfg.Exclude, compiledExclude: compiledExclude, + ownFiles: ownFiles, progress: progress, skipErrors: cfg.SkipErrors, previousSnapshotID: cfg.PreviousSnapshotID, @@ -2248,8 +2262,9 @@ func (s *Scanner) detectDeletedFiles( // excludedOrInExcludedDir reports whether filePath, which is at or // beneath rootPath, or any directory between rootPath and it, matches an -// exclude pattern. A pattern such as */.local/share/Trash matches the -// directory but not the files inside it, which the walk skips too. +// exclude pattern or is one of vaultik's own files. A pattern such as +// */.local/share/Trash matches the directory but not the files inside it, +// which the walk skips too. func (s *Scanner) excludedOrInExcludedDir(filePath, rootPath string) bool { for p := filePath; len(p) > len(rootPath); p = filepath.Dir(p) { if s.shouldExclude(p, rootPath) { @@ -2373,9 +2388,14 @@ func doubleStarVariants(pattern string) []string { } // shouldExclude checks if a path should be excluded based on exclude patterns +// or because it is one of vaultik's own working files. // filePath is the full path to the file // rootPath is the root of the backup source directory func (s *Scanner) shouldExclude(filePath, rootPath string) bool { + if _, own := s.ownFiles[filePath]; own { + return true + } + if len(s.compiledExclude) == 0 { return false }