Reject a retention duration longer than about 292 years (closes #289)
check / check (push) Successful in 15m42s

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
This commit is contained in:
2026-10-12 22:58:09 +00:00
parent fa56165fe4
commit 99e1cd4b2c
4 changed files with 68 additions and 8 deletions
+10
View File
@@ -22,6 +22,16 @@ the tag exists and is exercised; what is left is merging `next` to
# Completed Steps # 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 - 2026-10-08: Made a backup cancelled under `--skip-errors` stop at once
([issue #286](https://git.eeqj.de/sneak/vaultik/issues/286)). After ([issue #286](https://git.eeqj.de/sneak/vaultik/issues/286)). After
Ctrl-C or SIGTERM, phase 2 treated the cancellation error from each Ctrl-C or SIGTERM, phase 2 treated the cancellation error from each
+24 -8
View File
@@ -3,6 +3,7 @@ package vaultik
import ( import (
"errors" "errors"
"fmt" "fmt"
"math"
"regexp" "regexp"
"strconv" "strconv"
"strings" "strings"
@@ -47,6 +48,10 @@ const (
year = 365 * day 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 // A snapshot ID split on "_" has at least a hostname and a trailing
// timestamp. // timestamp.
const minSnapshotIDParts = 2 const minSnapshotIDParts = 2
@@ -145,10 +150,10 @@ func parseSnapshotName(snapshotID, hostname string) string {
// parseDuration parses a duration string with support for human-friendly units: // 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 // 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, // 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 // an unknown unit, a negative value, and a total longer than maxDuration are
// units the input must be whole numbers each followed directly by a unit // all rejected. Outside the Go units the input must be whole numbers each
// (2w3d), so 1.0y and 30 days are errors; decimals work only in Go units // followed directly by a unit (2w3d), so 1.0y and 30 days are errors;
// (1.5h). // decimals work only in Go units (1.5h).
func parseDuration(s string) (time.Duration, error) { func parseDuration(s string) (time.Duration, error) {
if strings.HasPrefix(strings.TrimSpace(s), "-") { if strings.HasPrefix(strings.TrimSpace(s), "-") {
return 0, errNegativeDuration 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) return 0, fmt.Errorf("invalid number %q: %w", match[1], err)
} }
var unitLength time.Duration
unit := strings.ToLower(match[2]) unit := strings.ToLower(match[2])
switch unit { switch unit {
case "d", "day", "days": case "d", "day", "days":
total += time.Duration(n) * day unitLength = day
case "w", "week", "weeks": case "w", "week", "weeks":
total += time.Duration(n) * week unitLength = week
case "mo", "month", "months": case "mo", "month", "months":
total += time.Duration(n) * month unitLength = month
case "y", "year", "years": case "y", "year", "years":
total += time.Duration(n) * year unitLength = year
default: default:
return 0, fmt.Errorf("%w %q", errUnknownTimeUnit, unit) 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 return total, nil
+11
View File
@@ -111,6 +111,17 @@ func TestParseDuration(t *testing.T) {
{"30 days", 0, true}, {"30 days", 0, true},
{"30 days ago", 0, true}, {"30 days ago", 0, true},
{"x7d", 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 { for _, tt := range tests {
+23
View File
@@ -267,6 +267,29 @@ func TestPurgeOlderThan_WithNameFilter(t *testing.T) {
assert.Contains(t, remaining, snapHomeT0) 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) { func TestPurgeKeepLatest_ThreeNames(t *testing.T) {
log.Initialize(log.Config{}) log.Initialize(log.Config{})
t.Parallel() t.Parallel()