From 713be502bdb4038332e95959e8e32f2b52424cb1 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 6 Oct 2026 06:12:07 +0200 Subject: [PATCH] Reject a duration with characters outside its parts (closes #215) parseDuration fell back to an unanchored search for number-and-unit pieces when time.ParseDuration failed, and skipped everything in between. 1.0y became 0, so `snapshot create --prune --keep-newer-than 1.0y` deleted every snapshot of the backed-up names, the new one included. 2.1w became one week and 1,5y five years. The fallback now requires the whole input to be whole-number-and-unit parts with nothing between them. A bare number is rejected before time.ParseDuration sees it, since Go reads 0 and +0 as zero with no unit. Judgement call: a space between number and unit (`30 days`) was accepted and is now an error, matching Go's own units. Model: opus-5-5 --- TODO.md | 9 +++++++++ internal/vaultik/helpers.go | 20 +++++++++++++++----- internal/vaultik/helpers_test.go | 11 +++++++++++ 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/TODO.md b/TODO.md index fef4869..dbf60b3 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-06: Made `--older-than` and `--keep-newer-than` reject a + duration with characters outside its number-and-unit parts + ([issue #215](https://git.eeqj.de/sneak/vaultik/issues/215)). The + parser picked out the parts it recognised and skipped the rest, so + `1.0y` became zero and `--prune --keep-newer-than 1.0y` deleted every + snapshot of the backed-up names, the new one included. `1.0y`, + `1,5y`, `30 days`, `x7d` and a bare `0` are now errors; decimals still + work in Go units such as `1.5h`. + - 2026-10-06: Made a backup re-chunk a known file that lists a chunk no uploaded blob holds ([issue #214](https://git.eeqj.de/sneak/vaultik/issues/214)). File diff --git a/internal/vaultik/helpers.go b/internal/vaultik/helpers.go index 64a1d85..9ca5045 100644 --- a/internal/vaultik/helpers.go +++ b/internal/vaultik/helpers.go @@ -140,24 +140,34 @@ func parseSnapshotName(snapshotID 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. +// 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). func parseDuration(s string) (time.Duration, error) { if strings.HasPrefix(strings.TrimSpace(s), "-") { return 0, errNegativeDuration } + // A bare number has no unit, but time.ParseDuration accepts 0, which + // would put the cutoff at now and select every snapshot. + _, err := strconv.ParseFloat(s, 64) + if err == nil { + return 0, fmt.Errorf("%w: %q", errInvalidDuration, s) + } + d, err := time.ParseDuration(s) if err == nil { return d, nil } - re := regexp.MustCompile(`(\d+)\s*([a-zA-Z]+)`) - - matches := re.FindAllStringSubmatch(s, -1) - if len(matches) == 0 { + if !regexp.MustCompile(`^(\d+[a-zA-Z]+)+$`).MatchString(s) { return 0, fmt.Errorf("%w: %q", errInvalidDuration, s) } + re := regexp.MustCompile(`(\d+)([a-zA-Z]+)`) + matches := re.FindAllStringSubmatch(s, -1) + var total time.Duration for _, match := range matches { diff --git a/internal/vaultik/helpers_test.go b/internal/vaultik/helpers_test.go index eb58781..b77b51d 100644 --- a/internal/vaultik/helpers_test.go +++ b/internal/vaultik/helpers_test.go @@ -59,6 +59,7 @@ func TestParseDuration(t *testing.T) { {"30s", 30 * time.Second, false}, {"6m", 6 * time.Minute, false}, {"1h", time.Hour, false}, + {"1.5h", 90 * time.Minute, false}, // Extended calendar units. {"30d", 30 * 24 * time.Hour, false}, {"3days", 3 * 24 * time.Hour, false}, @@ -73,11 +74,21 @@ func TestParseDuration(t *testing.T) { {"1y6mo", 365*24*time.Hour + 180*24*time.Hour, false}, // Rejected inputs. {"6", 0, true}, // bare number, no unit + {"0", 0, true}, // bare number; time.ParseDuration accepts it + {"+0", 0, true}, // bare number; time.ParseDuration accepts it {"5x", 0, true}, // unknown unit {"-5d", 0, true}, // negative, extended unit {"-5h", 0, true}, // negative, Go unit {"", 0, true}, // empty {"garbage", 0, true}, + // Characters outside the number-and-unit parts must not be + // skipped: 1.0y read as 0y would select every snapshot. + {"1.0y", 0, true}, + {"2.1w", 0, true}, + {"1,5y", 0, true}, + {"30 days", 0, true}, + {"30 days ago", 0, true}, + {"x7d", 0, true}, } for _, tt := range tests {