From 99e1cd4b2cdf2801fc40916627f349bb6359eec5 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 12 Oct 2026 22:58:09 +0000 Subject: [PATCH] Reject a retention duration longer than about 292 years (closes #289) parseDuration multiplied each number by the length of its unit with no limit, so from 293y (or 15251w, 3559mo, 106752d, or a sum such as 200y200y) the result wrapped to a negative duration. The purge cutoff then lay in the future and every snapshot of the name was selected, for --older-than and --keep-newer-than alike. Each part is now checked before it is added, and a total past the largest time.Duration is rejected as an invalid duration, so nothing is deleted. Go units were already safe: time.ParseDuration rejects an overflow. With --prune the backup still runs before the duration is read; only the prune step fails. Model: opus-5-5 --- TODO.md | 10 ++++++++ internal/vaultik/helpers.go | 32 ++++++++++++++++++------- internal/vaultik/helpers_test.go | 11 +++++++++ internal/vaultik/purge_per_name_test.go | 23 ++++++++++++++++++ 4 files changed, 68 insertions(+), 8 deletions(-) diff --git a/TODO.md b/TODO.md index aab8ceb..018c80c 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,16 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 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 diff --git a/internal/vaultik/helpers.go b/internal/vaultik/helpers.go index ff5fe19..325d8f2 100644 --- a/internal/vaultik/helpers.go +++ b/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 diff --git a/internal/vaultik/helpers_test.go b/internal/vaultik/helpers_test.go index e279319..f3b2f04 100644 --- a/internal/vaultik/helpers_test.go +++ b/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 { diff --git a/internal/vaultik/purge_per_name_test.go b/internal/vaultik/purge_per_name_test.go index 800480c..f895f61 100644 --- a/internal/vaultik/purge_per_name_test.go +++ b/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() -- 2.54.0