Refuse a relative file:// storage_url and decode its escapes (closes #299)

The path after file:// was used as written. file://dest resolved
against the working directory, so a backup run from another directory
wrote elsewhere, exited 0 and left a snapshot that could not be
restored; my%20stick created a directory with that literal name.

Config load now refuses a file:// storage_url whose path is not
absolute. ParseStorageURL refuses it too and decodes percent-escapes;
a literal space still works. The rule is written in both packages
because config cannot import storage, which imports config.

A % not followed by two hex digits now fails when the storage is
opened, not at config load.

Model: opus-5-5

README.md

@@ -426,7 +426,10 @@ 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.
+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.
 
 **Rclone** (`rclone://remote/path`): Uses rclone's 70+ supported cloud
 providers. Requires rclone to be configured separately (`rclone config`).

TODO.md

@@ -22,6 +22,27 @@ 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-12: Rejected a retention duration longer than about 292 years
+  ([issue #289](https://git.eeqj.de/sneak/vaultik/issues/289)). The
+  number given to `--older-than` or `--keep-newer-than` was multiplied
+  by the length of its unit with no limit, so from `293y`, `15251w`,
+  `3559mo` or `106752d`, or a sum of parts such as `200y200y`, the
+  result wrapped to a negative duration. The cutoff then lay in the
+  future, every snapshot of the name was deleted, the one just made
+  included, and the command exited 0. Such a duration is now rejected
+  as invalid and nothing is deleted.
+
 - 2026-10-08: Made a backup cancelled under `--skip-errors` stop at once
   ([issue #286](https://git.eeqj.de/sneak/vaultik/issues/286)). After
   Ctrl-C or SIGTERM, phase 2 treated the cancellation error from each

internal/config/config.go

@@ -61,6 +61,9 @@ 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")
@@ -461,11 +464,18 @@ 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.
+// 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.
 func (c *Config) validateStorageURL() error {
 	switch {
 	case strings.HasPrefix(c.StorageURL, "file://"):
-		// File storage doesn't need S3 credentials
+		// storage.ParseStorageURL applies the same rule, but config
+		// cannot import storage, which imports config.
+		if !strings.HasPrefix(c.StorageURL, "file:///") {
+			return errFileURLNotAbsolute
+		}
+
 		return nil
 	case strings.HasPrefix(c.StorageURL, "rclone://"):
 		// Rclone storage uses rclone's own config

internal/config/config_test.go

@@ -337,6 +337,35 @@ 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

internal/storage/url.go

@@ -17,8 +17,10 @@ const (
 
 // Sentinel errors for storage URL parsing.
 var (
-	ErrEmptyStorageURL   = errors.New("storage URL is empty")
-	ErrEmptyFilePath     = errors.New("file URL path is empty")
+	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(
@@ -51,17 +53,8 @@ func ParseStorageURL(rawURL string) (*URL, error) {
 		return nil, ErrEmptyStorageURL
 	}
 
-	// Handle file:// URLs
 	if after, ok := strings.CutPrefix(rawURL, "file://"); ok {
-		path := after
-		if path == "" {
-			return nil, ErrEmptyFilePath
-		}
-
-		return &URL{
-			Scheme: schemeFile,
-			Prefix: path,
-		}, nil
+		return parseFileURL(after)
 	}
 
 	if strings.HasPrefix(rawURL, "s3://") {
@@ -75,6 +68,27 @@ 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.

internal/storage/url_parse_test.go

@@ -14,7 +14,10 @@ import (
 func TestParseStorageURLValid(t *testing.T) {
 	t.Parallel()
 
-	const bucket = "mybucket"
+	const (
+		bucket     = "mybucket"
+		fileScheme = "file"
+	)
 
 	cases := []struct {
 		name string
@@ -24,7 +27,17 @@ func TestParseStorageURLValid(t *testing.T) {
 		{
 			name: "file absolute path",
 			raw:  "file:///var/backups/vaultik",
-			want: &storage.URL{Scheme: "file", Prefix: "/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"},
 		},
 		{
 			name: "s3 bucket and prefix, ssl defaults on",
@@ -90,6 +103,7 @@ 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},

internal/vaultik/helpers.go

@@ -3,6 +3,7 @@ package vaultik
 import (
 	"errors"
 	"fmt"
+	"math"
 	"regexp"
 	"strconv"
 	"strings"
@@ -47,6 +48,10 @@ const (
 	year  = 365 * day
 )
 
+// maxDuration is the largest value a time.Duration can hold, about 292
+// years.
+const maxDuration = time.Duration(math.MaxInt64)
+
 // A snapshot ID split on "_" has at least a hostname and a trailing
 // timestamp.
 const minSnapshotIDParts = 2
@@ -145,10 +150,10 @@ func parseSnapshotName(snapshotID, hostname string) string {
 // parseDuration parses a duration string with support for human-friendly units:
 // d/day/days, w/week/weeks, mo/month/months, y/year/years, plus standard Go
 // duration units. Following Go, m is minutes and mo is months. A bare number,
-// an unknown unit, and a negative value are all rejected. Outside the Go
-// units the input must be whole numbers each followed directly by a unit
-// (2w3d), so 1.0y and 30 days are errors; decimals work only in Go units
-// (1.5h).
+// an unknown unit, a negative value, and a total longer than maxDuration are
+// all rejected. Outside the Go units the input must be whole numbers each
+// followed directly by a unit (2w3d), so 1.0y and 30 days are errors;
+// decimals work only in Go units (1.5h).
 func parseDuration(s string) (time.Duration, error) {
 	if strings.HasPrefix(strings.TrimSpace(s), "-") {
 		return 0, errNegativeDuration
@@ -181,19 +186,30 @@ func parseDuration(s string) (time.Duration, error) {
 			return 0, fmt.Errorf("invalid number %q: %w", match[1], err)
 		}
 
+		var unitLength time.Duration
+
 		unit := strings.ToLower(match[2])
 		switch unit {
 		case "d", "day", "days":
-			total += time.Duration(n) * day
+			unitLength = day
 		case "w", "week", "weeks":
-			total += time.Duration(n) * week
+			unitLength = week
 		case "mo", "month", "months":
-			total += time.Duration(n) * month
+			unitLength = month
 		case "y", "year", "years":
-			total += time.Duration(n) * year
+			unitLength = year
 		default:
 			return 0, fmt.Errorf("%w %q", errUnknownTimeUnit, unit)
 		}
+
+		// Past maxDuration the sum wraps to a negative value, which would
+		// put the purge cutoff in the future.
+		if time.Duration(n) > (maxDuration-total)/unitLength {
+			return 0, fmt.Errorf("%w: %q is longer than about 292 years",
+				errInvalidDuration, s)
+		}
+
+		total += time.Duration(n) * unitLength
 	}
 
 	return total, nil

internal/vaultik/helpers_test.go

@@ -111,6 +111,17 @@ func TestParseDuration(t *testing.T) {
 		{"30 days", 0, true},
 		{"30 days ago", 0, true},
 		{"x7d", 0, true},
+		// The largest time.Duration is about 292 years. A larger value
+		// must be rejected, not wrapped to a negative one, which would
+		// put the cutoff in the future and select every snapshot.
+		{"292y", 292 * 365 * 24 * time.Hour, false},
+		{"293y", 0, true},
+		{"3559mo", 0, true},
+		{"15251w", 0, true},
+		{"106752d", 0, true},
+		{"2562048h", 0, true},
+		{"200y200y", 0, true},
+		{"292y6mo", 0, true},
 	}
 
 	for _, tt := range tests {

internal/vaultik/purge_per_name_test.go

@@ -267,6 +267,29 @@ func TestPurgeOlderThan_WithNameFilter(t *testing.T) {
 	assert.Contains(t, remaining, snapHomeT0)
 }
 
+func TestPurgeOlderThan_TooLargeDeletesNothing(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	snapshotIDs := []string{snapHomeT0, snapHomeT1}
+
+	v := setupPurgeTest(t, snapHostname, snapshotIDs)
+
+	err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{
+		OlderThan: "1000y",
+		Force:     true,
+	})
+	require.Error(t, err)
+
+	err = v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{
+		OlderThan: "292y",
+		Force:     true,
+	})
+	require.NoError(t, err)
+
+	assert.ElementsMatch(t, snapshotIDs, listRemainingSnapshots(t, v))
+}
+
 func TestPurgeKeepLatest_ThreeNames(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()