From 17ab7b16ee632fd9c94966dd39e677713e42e5e2 Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 11 Nov 2026 00:22:57 +0000 Subject: [PATCH] Skip vaultik's own index and PID file in a backup (closes #421) A backup of a directory that holds the local index stored a torn copy of it, its -wal and -shm side files and vaultik.pid, all being written while they were read. The scan now treats these files as excluded, whatever the config says. Their paths come from index_path and the lock directory, with symlinks in their directories resolved the way source paths are. If index_path is itself a symlink, SQLite writes the file it points to, so that file and its side files are skipped too. pidlock now owns the lock directory and the PID file name, so the CLI and the scanner use the same path. database delete now removes the side files database.IndexFiles lists, the -journal file included. Model: opus-5-5 --- internal/cli/app.go | 5 +- internal/cli/database.go | 19 +++-- internal/cli/database_test.go | 27 ++++++++ ...apshot_create_index_and_lock_files_test.go | 69 +++++++++++++++++++ internal/database/module.go | 11 +++ internal/pidlock/pidlock.go | 15 +++- .../snapshot/index_and_lock_files_test.go | 65 +++++++++++++++++ internal/snapshot/module.go | 45 ++++++++++++ internal/snapshot/module_test.go | 32 +++++++++ internal/snapshot/scanner.go | 24 ++++++- 10 files changed, 295 insertions(+), 17 deletions(-) create mode 100644 internal/cli/entry_snapshot_create_index_and_lock_files_test.go create mode 100644 internal/snapshot/index_and_lock_files_test.go create mode 100644 internal/snapshot/module_test.go 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 }