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/database.go b/internal/cli/database.go index 93b330c..93c7911 100644 --- a/internal/cli/database.go +++ b/internal/cli/database.go @@ -6,6 +6,7 @@ import ( "github.com/spf13/cobra" "sneak.berlin/go/vaultik/internal/config" + "sneak.berlin/go/vaultik/internal/database" "sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/vaultik" ) @@ -100,18 +101,16 @@ Use --force to skip the confirmation prompt.`, } } - // Delete the database file - err = os.Remove(dbPath) - if err != nil { - return fmt.Errorf("failed to delete database: %w", err) + // Delete the database file and its side files. A side file + // may not exist, so only failing to delete the database file + // is an error. + for _, indexFile := range database.IndexFiles(dbPath) { + err = os.Remove(indexFile) + if err != nil && indexFile == dbPath { + return fmt.Errorf("failed to delete database: %w", err) + } } - // Also delete WAL and SHM files if they exist - walPath := dbPath + "-wal" - shmPath := dbPath + "-shm" - _ = os.Remove(walPath) // Ignore errors - files may not exist - _ = os.Remove(shmPath) - out.Infof("Local state database deleted: %s.", dbPath) log.Info("Local state database deleted", "path", dbPath) diff --git a/internal/cli/database_test.go b/internal/cli/database_test.go index be88674..fcb1ba5 100644 --- a/internal/cli/database_test.go +++ b/internal/cli/database_test.go @@ -5,9 +5,11 @@ import ( "errors" "io" "os" + "path/filepath" "strings" "testing" + "sneak.berlin/go/vaultik/internal/database" "sneak.berlin/go/vaultik/internal/vaultik" ) @@ -46,3 +48,28 @@ func TestDatabaseDeleteDeclinedIsError(t *testing.T) { out.String()) } } + +// TestDatabaseDeleteRemovesSideFiles checks that the side files SQLite +// keeps beside the database file are deleted with it. +// +//nolint:paralleltest // mutates the process-global rootFlags +func TestDatabaseDeleteRemovesSideFiles(t *testing.T) { + dir := t.TempDir() + dbPath, cfgPath := seedDatabaseDeleteConfig(t, dir) + + indexFiles := database.IndexFiles(dbPath) + for _, indexFile := range indexFiles { + seedFile(t, dir, filepath.Base(indexFile), "data") + } + + setRootFlags(t, RootFlags{ConfigPath: cfgPath}) + + mustExecute(t, newDatabaseDeleteCommand(), "--force") + + for _, indexFile := range indexFiles { + _, err := os.Stat(indexFile) + if !os.IsNotExist(err) { + t.Errorf("database delete left %s: stat err = %v", indexFile, err) + } + } +} diff --git a/internal/cli/entry_snapshot_create_index_and_lock_files_test.go b/internal/cli/entry_snapshot_create_index_and_lock_files_test.go new file mode 100644 index 0000000..8cf32cc --- /dev/null +++ b/internal/cli/entry_snapshot_create_index_and_lock_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" +) + +// TestEntrySnapshotCreateSkipsIndexAndLockFiles 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 TestEntrySnapshotCreateSkipsIndexAndLockFiles(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/index_and_lock_files_test.go b/internal/snapshot/index_and_lock_files_test.go new file mode 100644 index 0000000..3e07045 --- /dev/null +++ b/internal/snapshot/index_and_lock_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 TestScanSkipsIndexAndLockFiles(t *testing.T) { + t.Parallel() + + const dataDir = "/home/user/.local/share/vaultik" + + indexAndLockFiles := append(database.IndexFiles(dataDir+"/index.sqlite"), + dataDir+"/vaultik.pid") + + fs := afero.NewMemMapFs() + for _, p := range append(indexAndLockFiles, 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"}, + IndexAndLockFiles: indexAndLockFiles, + }) + 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 indexAndLockFiles { + 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/module.go b/internal/snapshot/module.go index f83ac07..9edd50b 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,52 @@ func provideScannerFactory( EnableProgress: params.EnableProgress, UI: params.UI, Exclude: excludes, + IndexAndLockFiles: indexAndLockFiles(cfg.IndexPath, pidlock.Dir()), SkipErrors: params.SkipErrors, PreviousSnapshotID: params.PreviousSnapshotID, DeviceNumber: params.DeviceNumber, }) } } + +// indexAndLockFiles 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 indexAndLockFiles(indexPath, lockDir string) []string { + paths := append(database.IndexFiles(indexPath), pidlock.Path(lockDir)) + for i, p := range paths { + paths[i] = resolveParentDir(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 := resolveParentDir(indexPath) + + target, err := filepath.EvalSymlinks(index) + if err == nil && target != index { + paths = append(paths, database.IndexFiles(target)...) + } + + return paths +} + +// resolveParentDir returns the absolute form of path with the symlinks in +// its parent directory resolved. If the parent directory cannot be +// resolved, it returns the absolute form unresolved. If path has no +// absolute form, it returns path. +func resolveParentDir(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..dff9b7f --- /dev/null +++ b/internal/snapshot/module_test.go @@ -0,0 +1,32 @@ +//nolint:testpackage // exercises the unexported indexAndLockFiles 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 TestIndexAndLockFilesResolvesSymlinkedDirectory(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 := indexAndLockFiles(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/scanner.go b/internal/snapshot/scanner.go index de8db7e..57eba64 100644 --- a/internal/snapshot/scanner.go +++ b/internal/snapshot/scanner.go @@ -67,6 +67,9 @@ type Scanner struct { currentSourcePath string exclude []string // Glob patterns for files/directories to exclude compiledExclude []compiledPattern // Compiled glob patterns + // indexAndLockFiles holds the paths of vaultik's own working files, + // which the scan treats as excluded (ScannerConfig.IndexAndLockFiles). + indexAndLockFiles 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. @@ -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 + // IndexAndLockFiles 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. + IndexAndLockFiles []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 } + indexAndLockFiles := make(map[string]struct{}, len(cfg.IndexAndLockFiles)) + for _, p := range cfg.IndexAndLockFiles { + indexAndLockFiles[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, + indexAndLockFiles: indexAndLockFiles, 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 _, ok := s.indexAndLockFiles[filePath]; ok { + return true + } + if len(s.compiledExclude) == 0 { return false }