From 32afea12113afbd8d825c8feb5c9550ad393b8f2 Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 13 Oct 2026 01:57:44 +0000 Subject: [PATCH] Remove the blob temp file when a backup stops part-way (closes #302) When `snapshot create` stopped on a file error, a full temporary directory or Ctrl-C, the blob being packed was never finalized, and its `vaultik-blob-*.tmp` file (up to `blob_size_limit` of encrypted data) stayed in `$TMPDIR`. Phase 2 now calls the new `Packer.DiscardCurrentBlob` on the way out, which closes and removes that file. After a successful flush no blob is in progress, so the call then does nothing. The blob's row in the local index is left, as before, for the next run's repair of interrupted blobs to remove. Model: opus-5-5 --- TODO.md | 9 +++++ internal/blob/packer.go | 11 ++++++ internal/snapshot/scanner.go | 5 +++ internal/snapshot/skip_errors_test.go | 57 +++++++++++++++++++++++++++ 4 files changed, 82 insertions(+) diff --git a/TODO.md b/TODO.md index 61fdbc6..b45814f 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,15 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-13: Made a failed or interrupted backup remove the temporary + file of the blob it was packing + ([issue #302](https://git.eeqj.de/sneak/vaultik/issues/302)). When + `snapshot create` stopped on a file error, a full temporary directory + or Ctrl-C, that blob was never finalized and its `vaultik-blob-*.tmp` + file, up to `blob_size_limit` of encrypted data, stayed in `$TMPDIR`; + every such run left another. Phase 2 now removes it whenever it stops + before the last blob is finalized. + - 2026-10-12: Bound the local index to the destination an `s3.*` config names ([issue #298](https://git.eeqj.de/sneak/vaultik/issues/298)). Only `storage_url` was checked, so after `s3.bucket` or `s3.prefix` diff --git a/internal/blob/packer.go b/internal/blob/packer.go index 1c588cc..cee01de 100644 --- a/internal/blob/packer.go +++ b/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 diff --git a/internal/snapshot/scanner.go b/internal/snapshot/scanner.go index c810278..dc98d4f 100644 --- a/internal/snapshot/scanner.go +++ b/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 { diff --git a/internal/snapshot/skip_errors_test.go b/internal/snapshot/skip_errors_test.go index 04ad8de..2a88637 100644 --- a/internal/snapshot/skip_errors_test.go +++ b/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) + } + }) + } +}