Author SHA1 Message Date
clawbot 35eed79b1a Check --keep-newer-than before the backup starts (closes #307)
check / check (push) Waiting to run
snapshot create read --keep-newer-than only in the post-backup prune,
so a value the duration parser rejects failed the run after the
snapshot was uploaded: a nightly cron line added a snapshot each night,
reported it as failed, and never deleted any. Without --prune the flag
was ignored. The command now checks the flag before it starts and fails
on an invalid duration or a missing --prune.

A failed post-backup prune is now wrapped in ErrPostBackupPruneFailed
and reported as "Backup completed, but post-backup prune failed: ..."
rather than as a failed snapshot creation. The exit status stays 1.

Model: opus-5-5
2026-10-13 09:44:44 +02:00
clawbot 94412d9ef3 Stop remote info counting a non-blob file under blobs/ (closes #295)
check / check (push) Waiting to run
remote info counted every key under blobs/ with four or more path parts
as a blob, while prune skips a name that is not a blob hash. A stray
file such as .DS_Store was reported as an orphaned blob, with advice to
run prune, which then deleted nothing. remote info now takes its blob
list from prune's listAllRemoteBlobs, so its blob and orphan figures,
in the table and in --json, cover exactly what prune would consider.

blobKeyParts moves to prune.go, its only remaining user.

Model: opus-5-5
2026-10-13 08:33:27 +02:00
12 changed files with 222 additions and 119 deletions
+7 -6
View File
@@ -286,7 +286,9 @@ needed.
remove orphaned blobs from remote storage. By default keeps only the latest
snapshot per name; use `--keep-newer-than` for a rolling window.
* `--keep-newer-than <duration>`: With `--prune`, keep snapshots newer than
this duration instead of only the latest (e.g. `4w`, `30d`, `6mo`, `1y`)
this duration instead of only the latest (e.g. `4w`, `30d`, `6mo`, `1y`).
Without `--prune`, or with a value that is not a valid duration, the
command fails before the backup starts.
**`snapshot list`**: Show every snapshot known to this host — the union
of the local index and the backup destination store — with timestamps
@@ -401,7 +403,9 @@ orphaned blob figures are reported as unknown; `--json` gives them as
`null`, lists the remote key of each unreadable manifest in
`unreadable_manifests` and counts the manifests under skipped names in
`skipped_manifest_count`. An unreadable manifest also leaves its
snapshot's blob count and blob size unknown, `null` in `--json`.
snapshot's blob count and blob size unknown, `null` in `--json`. A file
under `blobs/` whose name is not a blob hash is not counted as a blob,
since `prune` never deletes it.
* `--json`: Output as JSON
**`remote nuke`**: Delete every snapshot's metadata and every blob from the
@@ -429,10 +433,7 @@ object store. Credentials are read from `s3.access_key_id` and
`s3.secret_access_key` in the config file.
**Local filesystem** (`file:///path/to/backup`): Stores blobs and metadata on
a local or mounted filesystem. Useful for testing or backing up to a NAS. The
path must be absolute; a relative one is refused when the config loads.
Percent-escapes in the path are decoded, so `my%20stick` and `my stick` name
the same directory.
a local or mounted filesystem. Useful for testing or backing up to a NAS.
**Rclone** (`rclone://remote/path`): Uses rclone's 70+ supported cloud
providers. Requires rclone to be configured separately (`rclone config`).
+7 -10
View File
@@ -22,16 +22,13 @@ the tag exists and is exercised; what is left is merging `next` to
# Completed Steps
- 2026-10-13: Refused a `file://` `storage_url` whose path is relative
([issue #299](https://git.eeqj.de/sneak/vaultik/issues/299)). The text
after `file://` was used as written, so `file://dest` resolved against
the working directory: a backup run from another directory wrote to a
different destination, still exited 0, and left a snapshot that could
not be restored. Such a URL now fails when the config loads, with an
error asking for an absolute path. Percent-escapes in the path are now
decoded, so `my%20stick` names the directory `my stick` instead of
creating one named `my%20stick`; a path written with a literal space
works as before.
- 2026-10-13: Stopped `remote info` counting a file under `blobs/` that
is not a blob as an orphaned blob
([issue #295](https://git.eeqj.de/sneak/vaultik/issues/295)). A stray
file such as `.DS_Store` was reported as an orphaned blob and the
report advised running `prune`, which skips such a file and deleted
nothing. `remote info` now takes its blob list from the same listing
`prune` uses, in the table and in `--json`.
- 2026-10-13: Made `snapshot remove` fail for a snapshot that does not
exist ([issue #293](https://git.eeqj.de/sneak/vaultik/issues/293)).
@@ -0,0 +1,108 @@
package cli //nolint:testpackage // shares runEntry and the hermetic config helpers
import (
"os"
"path/filepath"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// Flags of `snapshot create` used to build argument vectors below.
const (
flagPrune = "--prune"
flagKeepNewerThan = "--keep-newer-than"
)
// TestEntrySnapshotCreateRejectsKeepNewerThanBeforeBackup checks that a
// --keep-newer-than the post-backup prune could not use stops `snapshot
// create` before the backup: the command fails and the destination
// store stays empty. It used to be read only by the prune, after the
// snapshot was uploaded, and without --prune it was ignored.
//
//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
func TestEntrySnapshotCreateRejectsKeepNewerThanBeforeBackup(t *testing.T) {
for _, testCase := range []struct {
name string
args []string
}{
{
name: "decimal in a day count",
args: []string{flagPrune, flagKeepNewerThan, "1.5d"},
},
{
name: "unknown unit",
args: []string{flagPrune, flagKeepNewerThan, "4x"},
},
{
name: "without --prune",
args: []string{flagKeepNewerThan, "4w"},
},
} {
t.Run(testCase.name, func(t *testing.T) {
configPath, storeDir := writeBackupConfig(t)
code, _, _ := runEntry(t, append(
[]string{flagConfig, configPath, cmdSnapshot, cmdCreate},
testCase.args...)...)
assert.Equal(t, 1, code)
stored, err := os.ReadDir(storeDir)
require.NoError(t, err)
assert.Empty(t, stored, "nothing may be uploaded")
})
}
}
// TestEntrySnapshotCreateReportsPostBackupPruneFailure makes the
// post-backup prune fail after a good backup, by planting a manifest
// that cannot be decoded, and checks that the failure is reported as the
// prune's and not as a failed snapshot creation.
//
//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
func TestEntrySnapshotCreateReportsPostBackupPruneFailure(t *testing.T) {
configPath, storeDir := writeBackupConfig(t)
// A remote key is 64 lowercase hex characters. The prune reads every
// manifest on the store and fails on one it cannot decode.
plantedDir := filepath.Join(storeDir, "metadata", strings.Repeat("a", 64))
require.NoError(t, os.MkdirAll(plantedDir, 0o750))
require.NoError(t, os.WriteFile(
filepath.Join(plantedDir, "manifest.json.zst"),
[]byte("not a valid manifest"), 0o600))
code, _, stderr := runEntry(t,
flagConfig, configPath, cmdSnapshot, cmdCreate, flagPrune)
assert.Equal(t, 1, code)
assert.Contains(t, stderr, "Backup completed, but post-backup prune failed")
assert.NotContains(t, stderr, "Snapshot creation failed")
snapshots, err := os.ReadDir(filepath.Join(storeDir, "metadata"))
require.NoError(t, err)
assert.Len(t, snapshots, 2,
"the new snapshot is on the store beside the planted one")
}
// writeBackupConfig builds a hermetic config with one small file in its
// source directory and an empty destination store directory. Returns the
// config path and the store directory.
func writeBackupConfig(t *testing.T) (string, string) {
t.Helper()
configPath := writeHermeticPruneConfig(t, false)
// writeHermeticPruneConfig puts the source and store directories
// beside the config file.
dir := filepath.Dir(configPath)
sourceDir := filepath.Join(dir, "source")
require.NoError(t, os.Mkdir(sourceDir, 0o750))
require.NoError(t, os.WriteFile(
filepath.Join(sourceDir, "file.txt"), []byte("hello"), 0o600))
return configPath, filepath.Join(dir, "store")
}
+12
View File
@@ -71,6 +71,11 @@ otherwise from the platform config directory (~/.config/vaultik/config.yml
on Linux), then /etc/vaultik/config.yml.`,
Args: cobra.ArbitraryArgs,
RunE: func(cmd *cobra.Command, args []string) error {
err := opts.Validate()
if err != nil {
return err
}
// Pass snapshot names from args
opts.Snapshots = args
// --skip-errors is a global flag on the root command.
@@ -97,6 +102,13 @@ on Linux), then /etc/vaultik/config.yml.`,
}, func(v *vaultik.Vaultik) error {
return v.CreateSnapshot(opts)
}, func(err error) {
if errors.Is(err, vaultik.ErrPostBackupPruneFailed) {
log.Error("Post-backup prune failed", "error", err)
ReportErrorf("Backup completed, but %v", err)
return
}
log.Error("Snapshot creation failed", "error", err)
ReportErrorf("Snapshot creation failed: %v", err)
})
+2 -12
View File
@@ -61,9 +61,6 @@ var (
errBadS3PartSize = errors.New("s3.part_size must be between 5MiB and 5GiB")
errBadStorageScheme = errors.New(
"storage_url must start with s3://, file://, or rclone://")
errFileURLNotAbsolute = errors.New(
"a file:// storage_url needs an absolute path, " +
"as in file:///path/to/backup")
errStorageNotConfigured = errors.New(
"storage not configured; set storage_url or provide s3.endpoint + " +
"s3.bucket + credentials")
@@ -464,18 +461,11 @@ func (c *Config) validateStorage() error {
// validateStorageURL validates URL-based storage configuration. File and
// rclone URLs need no credentials; S3 URLs require the legacy s3.*
// credential fields. A file URL must name an absolute path: a relative one
// would resolve against the working directory, so the destination would
// move with it.
// credential fields.
func (c *Config) validateStorageURL() error {
switch {
case strings.HasPrefix(c.StorageURL, "file://"):
// storage.ParseStorageURL applies the same rule, but config
// cannot import storage, which imports config.
if !strings.HasPrefix(c.StorageURL, "file:///") {
return errFileURLNotAbsolute
}
// File storage doesn't need S3 credentials
return nil
case strings.HasPrefix(c.StorageURL, "rclone://"):
// Rclone storage uses rclone's own config
-29
View File
@@ -337,35 +337,6 @@ func TestLoadS3PartSize(t *testing.T) {
})
}
// TestLoadRejectsRelativeFileURL checks that a file:// storage_url with a
// relative path fails at load. Accepted, it would resolve against the
// working directory, so a backup run from another directory would write
// to a different destination and still succeed.
func TestLoadRejectsRelativeFileURL(t *testing.T) {
t.Parallel()
const relativeConfig = "snapshots:\n" +
" test:\n" +
" paths: [/tmp/vaultik-test-source]\n" +
"storage_url: file://dest\n"
path := filepath.Join(t.TempDir(), "config.yml")
err := os.WriteFile(path, []byte(relativeConfig), 0o600)
if err != nil {
t.Fatalf("write config: %v", err)
}
_, err = Load(path)
if !errors.Is(err, errFileURLNotAbsolute) {
t.Fatalf("Load() error = %v, want errFileURLNotAbsolute", err)
}
if !strings.Contains(err.Error(), "absolute path") {
t.Errorf("error %q does not say to use an absolute path", err.Error())
}
}
// TestValidateAgeRecipients checks that recipients are parsed at config load
// (a bad entry fails immediately, not mid-backup) and that no invalid entry —
// least of all a pasted secret key — is echoed in the error. An empty list
+10 -24
View File
@@ -19,8 +19,6 @@ const (
var (
ErrEmptyStorageURL = errors.New("storage URL is empty")
ErrEmptyFilePath = errors.New("file URL path is empty")
ErrRelativeFilePath = errors.New(
"file URL needs an absolute path, as in file:///path/to/backup")
ErrMissingBucket = errors.New("s3 URL missing bucket name")
ErrMissingRemote = errors.New("rclone URL missing remote name")
ErrUnsupportedScheme = errors.New(
@@ -53,8 +51,17 @@ func ParseStorageURL(rawURL string) (*URL, error) {
return nil, ErrEmptyStorageURL
}
// Handle file:// URLs
if after, ok := strings.CutPrefix(rawURL, "file://"); ok {
return parseFileURL(after)
path := after
if path == "" {
return nil, ErrEmptyFilePath
}
return &URL{
Scheme: schemeFile,
Prefix: path,
}, nil
}
if strings.HasPrefix(rawURL, "s3://") {
@@ -68,27 +75,6 @@ func ParseStorageURL(rawURL string) (*URL, error) {
return nil, ErrUnsupportedScheme
}
// parseFileURL parses the path that follows file://. The path must be
// absolute: a relative one would resolve against the working directory,
// so the destination would move with it. Percent-escapes are decoded, so
// my%20stick names the directory "my stick"; a literal space is kept.
func parseFileURL(rawPath string) (*URL, error) {
if rawPath == "" {
return nil, ErrEmptyFilePath
}
if !strings.HasPrefix(rawPath, "/") {
return nil, ErrRelativeFilePath
}
path, err := url.PathUnescape(rawPath)
if err != nil {
return nil, fmt.Errorf("invalid file URL path: %w", err)
}
return &URL{Scheme: schemeFile, Prefix: path}, nil
}
// parseS3URL parses an s3://bucket/prefix URL. It rejects credentials in
// the userinfo and any query parameter other than endpoint, region and
// ssl, so a credential-bearing URL is never stored or echoed.
+2 -16
View File
@@ -14,10 +14,7 @@ import (
func TestParseStorageURLValid(t *testing.T) {
t.Parallel()
const (
bucket = "mybucket"
fileScheme = "file"
)
const bucket = "mybucket"
cases := []struct {
name string
@@ -27,17 +24,7 @@ func TestParseStorageURLValid(t *testing.T) {
{
name: "file absolute path",
raw: "file:///var/backups/vaultik",
want: &storage.URL{Scheme: fileScheme, Prefix: "/var/backups/vaultik"},
},
{
name: "file percent-escaped path is decoded",
raw: "file:///Volumes/my%20stick/backup",
want: &storage.URL{Scheme: fileScheme, Prefix: "/Volumes/my stick/backup"},
},
{
name: "file path with a literal space",
raw: "file:///Volumes/my stick/backup",
want: &storage.URL{Scheme: fileScheme, Prefix: "/Volumes/my stick/backup"},
want: &storage.URL{Scheme: "file", Prefix: "/var/backups/vaultik"},
},
{
name: "s3 bucket and prefix, ssl defaults on",
@@ -103,7 +90,6 @@ func TestParseStorageURLErrors(t *testing.T) {
}{
{"empty url", "", storage.ErrEmptyStorageURL},
{"file empty path", "file://", storage.ErrEmptyFilePath},
{"file relative path", "file://dest", storage.ErrRelativeFilePath},
{"s3 missing bucket", "s3://", storage.ErrMissingBucket},
{"s3 missing bucket with path", "s3:///justprefix", storage.ErrMissingBucket},
{"rclone missing remote", "rclone://", storage.ErrMissingRemote},
+7 -18
View File
@@ -167,10 +167,6 @@ const (
// metadataKeyParts is the minimum "/"-separated segment count of a
// metadata object key (metadata/<snapshot-id>/<filename>).
metadataKeyParts = 3
// blobKeyParts is the minimum "/"-separated segment count of a blob
// object key (blobs/<aa>/<bb>/<hash>).
blobKeyParts = 4
)
// SnapshotMetadataInfo contains information about a single snapshot's metadata
@@ -454,23 +450,16 @@ func (v *Vaultik) scanRemoteBlobStorage(
v.stdoutf("Scanning blobs...\n")
}
blobCh := v.Storage.ListStream(v.ctx, "blobs/")
allBlobs := make(map[string]int64)
for obj := range blobCh {
if obj.Err != nil {
return fmt.Errorf("listing blobs: %w", obj.Err)
// Use prune's listing, so a file under blobs/ that prune would never
// delete is not counted as a blob, and so not as an orphaned one.
allBlobs, err := v.listAllRemoteBlobs()
if err != nil {
return err
}
parts := strings.Split(obj.Key, "/")
if len(parts) < blobKeyParts {
continue
}
hash := parts[3]
allBlobs[hash] = obj.Size
for _, size := range allBlobs {
result.TotalBlobCount++
result.TotalBlobSize += obj.Size
result.TotalBlobSize += size
}
// A blob named only by a manifest that could not be read, or by one
+4
View File
@@ -23,6 +23,10 @@ var errNukeRequiresForce = errors.New(
// metadata.
const metadataDirName = "metadata"
// blobKeyParts is the "/"-separated segment count of a blob object key
// (blobs/<aa>/<bb>/<hash>).
const blobKeyParts = 4
// NukeRemote deletes every snapshot's metadata and every blob from remote
// storage, along with any object an upload cut off part-way left under a
// temporary `.partial` name. After this returns successfully the bucket
+33
View File
@@ -185,6 +185,39 @@ func TestRemoteInfo_SkipsNonConformingMetadataName(t *testing.T) {
assert.NotContains(t, doc, "unreadable_manifests")
}
// TestRemoteInfo_DoesNotCountNonBlobFileUnderBlobs checks that a file
// under blobs/ whose name is not a blob hash is not counted as a blob.
// prune never deletes such a file, so counting it as an orphaned blob
// would advise a prune that removes nothing.
func TestRemoteInfo_DoesNotCountNonBlobFileUnderBlobs(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newListEnv(t)
env.addRemote(t, listRemoteID, time.Date(2026, 3, 2, 0, 0, 0, 0, time.UTC))
addBlob(t, env.store.testStorer, testBlobHashA)
require.NoError(t, env.store.Put(context.Background(),
"blobs/"+testBlobHashA[:2]+"/"+testBlobHashA[2:4]+"/.DS_Store",
bytes.NewReader([]byte("junk"))))
require.NoError(t, env.v.RemoteInfo(false))
text := env.stdout.String()
assert.Contains(t, text, "Total blobs on remote: 1 (")
assert.Contains(t, text, "Orphaned (unreferenced): 0 (")
assert.NotContains(t, text, "vaultik prune")
env.stdout.Reset()
require.NoError(t, env.v.RemoteInfo(true))
var doc map[string]any
require.NoError(t, json.Unmarshal(env.stdout.Bytes(), &doc))
assert.InDelta(t, 1, doc["total_blob_count"], 0)
assert.InDelta(t, 0, doc["orphaned_blob_count"], 0)
assert.InDelta(t, 0, doc["orphaned_blob_size"], 0)
}
// TestRemoteInfo_DirectoryWithoutManifestLeavesOrphansKnown checks that
// a directory under metadata/ holding no manifest.json.zst, such as one
// left by a backup interrupted before its manifest upload, leaves the
+27 -1
View File
@@ -28,8 +28,14 @@ var (
errNoAgeRecipients = errors.New(
"creating a snapshot needs at least one public key in " +
"age_recipients (generate a keypair with: age-keygen)")
errKeepNewerThanNeedsPrune = errors.New("--keep-newer-than requires --prune")
)
// ErrPostBackupPruneFailed wraps a failure of the post-backup prune,
// which runs only after every snapshot was created and uploaded, so the
// caller can report it apart from a failed backup.
var ErrPostBackupPruneFailed = errors.New("post-backup prune failed")
// listRecentLimit caps how many snapshot rows are fetched from the
// local index for listing and purge operations.
const listRecentLimit = 10000
@@ -45,6 +51,26 @@ type SnapshotCreateOptions struct {
Snapshots []string // Optional list of snapshot names to process (empty = all)
}
// Validate fails when KeepNewerThan is set without Prune or is not a
// valid duration. Only the post-backup prune reads KeepNewerThan, after
// the backup, so the caller checks it before starting one.
func (opts *SnapshotCreateOptions) Validate() error {
if opts.KeepNewerThan == "" {
return nil
}
if !opts.Prune {
return errKeepNewerThanNeedsPrune
}
_, err := parseDuration(opts.KeepNewerThan)
if err != nil {
return fmt.Errorf("--keep-newer-than: %w", err)
}
return nil
}
// CreateSnapshot executes the snapshot creation operation
func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error {
// config.Load accepts an empty list, since listing, verifying and
@@ -123,7 +149,7 @@ func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error {
if opts.Prune {
err = v.runPostBackupPrune(snapshotNames, opts.KeepNewerThan)
if err != nil {
return fmt.Errorf("post-backup prune: %w", err)
return fmt.Errorf("%w: %w", ErrPostBackupPruneFailed, err)
}
}