fix: swap to hashicorp/cronexpr for backend cronexpr parsing which supports more syntax

docs/src/docs/operations.md

@@ -40,6 +40,25 @@ Backrest provides flexible scheduling options for all operations through policie
 | Interval Days  | Run every N days                | Regular daily+ intervals                                  |
 | Interval Hours | Run every N hours               | Regular sub-daily intervals                               |
 
+### Cron Expression Syntax
+
+Cron schedules accept the standard five fields (minute, hour, day of month, month, day of week) with the usual `*`, `,`, `-`, and `/` operators. Day of week accepts `0`-`7` (both `0` and `7` mean Sunday) or the names `SUN`-`SAT`.
+
+The following extensions are also supported:
+
+| Syntax      | Meaning                                    | Example                                           |
+| ----------- | ------------------------------------------ | ------------------------------------------------- |
+| `#`         | Nth weekday of the month                   | `0 0 * * 1#1` runs on the first Monday             |
+| `L`         | Last day of the month                      | `0 0 L * *` runs on the last day of each month     |
+| `<dow>L`    | Last given weekday of the month            | `0 0 * * 5L` runs on the last Friday               |
+| `<dom>W`    | Nearest weekday to a day of the month      | `0 0 15W * *` runs on the weekday nearest the 15th |
+| `?`         | Same as `*`                                | `0 0 * * ?`                                        |
+| `@` aliases | `@yearly`, `@monthly`, `@weekly`, `@daily`, `@hourly` | `@daily` runs at midnight              |
+
+Longer expressions are also accepted. Six fields adds a trailing year (`minute hour day-of-month month day-of-week year`), and seven fields adds both a leading seconds field and a trailing year (`second minute hour day-of-month month day-of-week year`).
+
+Expressions that are syntactically valid but can never match a real date, such as `0 0 30 2 *`, are rejected when the configuration is saved rather than silently never running.
+
 ### Schedule Clocks
 
 | Clock         | Description                    | Best For                                |

go.mod

@@ -8,12 +8,12 @@ require (
 	fyne.io/systray v1.12.2
 	github.com/djherbis/buffer v1.2.0
 	github.com/djherbis/nio/v3 v3.0.1
-	github.com/gitploy-io/cronexpr v0.2.2
 	github.com/gofrs/flock v0.13.0
 	github.com/golang-jwt/jwt/v5 v5.3.1
 	github.com/google/go-cmp v0.7.0
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
 	github.com/google/uuid v1.6.0
+	github.com/hashicorp/cronexpr v1.1.3
 	github.com/hashicorp/go-multierror v1.1.1
 	github.com/hashicorp/golang-lru/v2 v2.0.7
 	github.com/hectane/go-acl v1.0.0

go.sum

@@ -27,8 +27,6 @@ github.com/djherbis/nio/v3 v3.0.1 h1:6wxhnuppteMa6RHA4L81Dq7ThkZH8SwnDzXDYy95vB4
 github.com/djherbis/nio/v3 v3.0.1/go.mod h1:Ng4h80pbZFMla1yKzm61cF0tqqilXZYrogmWgZxOcmg=
 github.com/eclipse/paho.golang v0.23.0 h1:KHgl2wz6EJo7cMBmkuhpt7C576vP+kpPv7jjvSyR6Mk=
 github.com/eclipse/paho.golang v0.23.0/go.mod h1:nQRhTkoZv8EAiNs5UU0/WdQIx2NrnWUpL9nsGJTQN04=
-github.com/gitploy-io/cronexpr v0.2.2 h1:Au+wK6FqmOLAF7AkW6q4gnrNXTe3rEW97XFZ4chy0xs=
-github.com/gitploy-io/cronexpr v0.2.2/go.mod h1:Uep5sbzUSocMZvJ1s0lNI9zi37s5iUI1llkw3vRGK9M=
 github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI=
 github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
 github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag=
@@ -53,6 +51,8 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
 github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg=
 github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
+github.com/hashicorp/cronexpr v1.1.3 h1:rl5IkxXN2m681EfivTlccqIryzYJSXRGRNa0xeG7NA4=
+github.com/hashicorp/cronexpr v1.1.3/go.mod h1:P4wA0KBl9C5q2hABiMO7cp6jcIg96CDh1Efb3g1PWA4=
 github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
 github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I=
 github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=

internal/orchestrator/tasks/scheduling_test.go

@@ -255,7 +255,10 @@ func TestScheduling(t *testing.T) {
 					UnixTimeEndMs:   farFuture.UnixMilli(),
 				},
 			},
-			wantTime: mustParseTime(t, "1970-01-02T08:00:00Z"),
+			// now is 1970-01-02T03:46:40Z, so the next UTC midnight is the 3rd.
+			// This must differ from the CLOCK_LOCAL case above: under TZ=America/Los_Angeles
+			// a UTC-clock schedule fires on UTC wall-clock, not local wall-clock.
+			wantTime: mustParseTime(t, "1970-01-03T00:00:00Z"),
 		},
 		{
 			name: "backup schedule cron since last run",

internal/protoutil/schedule.go

@@ -6,7 +6,7 @@ import (
 	"time"
 
 	v1 "github.com/garethgeorge/backrest/gen/go/v1"
-	"github.com/gitploy-io/cronexpr"
+	"github.com/hashicorp/cronexpr"
 )
 
 var ErrScheduleDisabled = errors.New("never")
@@ -51,14 +51,18 @@ func ResolveSchedule(sched *v1.Schedule, lastRan time.Time, curTime time.Time) (
 	case *v1.Schedule_MaxFrequencyHours:
 		return t.Add(time.Duration(s.MaxFrequencyHours) * time.Hour), nil
 	case *v1.Schedule_Cron:
-		cron, err := cronexpr.ParseInLocation(s.Cron, time.Now().Location().String())
+		cron, err := cronexpr.Parse(s.Cron)
 		if err != nil {
 			return time.Time{}, fmt.Errorf("parse cron %q: %w", s.Cron, err)
 		}
-		if cron.Next(t).IsZero() || cron.Next(t).Before(t) {
+		// Next evaluates the expression against t's own location, so the clock
+		// selection above (local / UTC / last run time) determines the zone the
+		// cron's wall-clock fields are interpreted in.
+		next := cron.Next(t)
+		if next.IsZero() || next.Before(t) {
 			return time.Time{}, fmt.Errorf("cron %q may be malformed, next scheduled time is in the past %v", s.Cron, t)
 		}
-		return cron.Next(t), nil
+		return next, nil
 	default:
 		return time.Time{}, fmt.Errorf("unknown schedule type: %T", s)
 	}
@@ -80,7 +84,7 @@ func NominalPeriod(sched *v1.Schedule, from time.Time) (time.Duration, error) {
 	case *v1.Schedule_MaxFrequencyHours:
 		return time.Duration(s.MaxFrequencyHours) * time.Hour, nil
 	case *v1.Schedule_Cron:
-		cron, err := cronexpr.ParseInLocation(s.Cron, from.Location().String())
+		cron, err := cronexpr.Parse(s.Cron)
 		if err != nil {
 			return 0, fmt.Errorf("parse cron %q: %w", s.Cron, err)
 		}
@@ -116,12 +120,15 @@ func ValidateSchedule(sched *v1.Schedule) error {
 		if s.Cron == "" {
 			return errors.New("empty cron expression")
 		}
-		cron, err := cronexpr.ParseInLocation(s.Cron, time.Now().Location().String())
+		cron, err := cronexpr.Parse(s.Cron)
 		if err != nil {
 			return fmt.Errorf("invalid cron %q: %w", s.Cron, err)
 		}
+		// A syntactically valid expression can still describe a date that never
+		// occurs (e.g. "0 0 30 2 *"); Next reports that as the zero time rather
+		// than an error, and such a schedule would silently never run.
 		if next := cron.Next(time.Now()); next.IsZero() || next.Year() < 2000 {
-			return fmt.Errorf("invalid cron %q: next scheduled time is invalid (check for DOW=7 usage)", s.Cron)
+			return fmt.Errorf("invalid cron %q: expression never matches a real date", s.Cron)
 		}
 	case nil:
 		return nil

internal/protoutil/schedule_test.go

@@ -11,8 +11,8 @@ import (
 )
 
 func TestResolveSchedule(t *testing.T) {
-	// Use Local time to match ResolveSchedule's ParseInLocation(time.Now().Location())
-	// ensuring cron expression and reference time are in same zone.
+	// Cron expressions are evaluated in the reference time's own zone, so use
+	// Local here to match the CLOCK_LOCAL cases below.
 	now := time.Date(2023, 10, 1, 10, 0, 0, 0, time.Local) // Sunday, Oct 1st 2023 10:00 Local
 
 	tests := []struct {
@@ -62,6 +62,31 @@ func TestResolveSchedule(t *testing.T) {
 			// Let's assume next slot is next week.
 			expected: now.Add(7 * 24 * time.Hour),
 		},
+		{
+			name: "Cron - First Monday of the month (#)",
+			schedule: &v1.Schedule{
+				Clock: v1.Schedule_CLOCK_LOCAL,
+				Schedule: &v1.Schedule_Cron{
+					Cron: "0 0 * * 1#1",
+				},
+			},
+			lastRan: now,
+			curTime: now,
+			// Oct 1st 2023 is a Sunday, so the first Monday is Oct 2nd.
+			expected: time.Date(2023, 10, 2, 0, 0, 0, 0, time.Local),
+		},
+		{
+			name: "Cron - Last day of the month (L)",
+			schedule: &v1.Schedule{
+				Clock: v1.Schedule_CLOCK_LOCAL,
+				Schedule: &v1.Schedule_Cron{
+					Cron: "0 0 L * *",
+				},
+			},
+			lastRan:  now,
+			curTime:  now,
+			expected: time.Date(2023, 10, 31, 0, 0, 0, 0, time.Local),
+		},
 	}
 
 	for _, tt := range tests {
@@ -92,12 +117,11 @@ func TestValidateSchedule(t *testing.T) {
 			expectError: false,
 		},
 		{
-			name: "Invalid Cron (7) - Validation Error",
+			name: "Valid Cron (7) - Sunday, same as 0",
 			schedule: &v1.Schedule{
 				Schedule: &v1.Schedule_Cron{Cron: "0 10 * * 7"},
 			},
-			expectError:   true,
-			errorContains: "check for DOW=7 usage",
+			expectError: false,
 		},
 		{
 			name: "Valid Frequency",
@@ -106,6 +130,68 @@ func TestValidateSchedule(t *testing.T) {
 			},
 			expectError: false,
 		},
+		// Extended (Quartz-style) syntax. cronstrue renders these in the WebUI, so
+		// the backend must accept everything the schedule form is willing to show.
+		{
+			name:        "Nth weekday of month (#)",
+			schedule:    &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 * * 1#1"}},
+			expectError: false,
+		},
+		{
+			name:        "Nth weekday of month by name",
+			schedule:    &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 * * MON#1"}},
+			expectError: false,
+		},
+		{
+			name:        "Last weekday of month (L)",
+			schedule:    &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 * * 5L"}},
+			expectError: false,
+		},
+		{
+			name:        "Last day of month (L)",
+			schedule:    &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 L * *"}},
+			expectError: false,
+		},
+		{
+			name:        "Nearest weekday (W)",
+			schedule:    &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 15W * *"}},
+			expectError: false,
+		},
+		{
+			name:        "Descriptor",
+			schedule:    &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "@daily"}},
+			expectError: false,
+		},
+		// Day-of-week ranges ending in 7 parse but never fire under the upstream
+		// (archived) gorhill parser; the hashicorp fork resolves 7 to Sunday.
+		{
+			name:        "Day of week range ending in 7",
+			schedule:    &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 * * 1-7"}},
+			expectError: false,
+		},
+		{
+			name:        "Day of week range MON-SUN",
+			schedule:    &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 * * MON-SUN"}},
+			expectError: false,
+		},
+		{
+			name:          "Nth weekday out of range",
+			schedule:      &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 * * 1#6"}},
+			expectError:   true,
+			errorContains: "invalid cron",
+		},
+		{
+			name:          "Expression that never matches a real date",
+			schedule:      &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 30 2 *"}},
+			expectError:   true,
+			errorContains: "never matches a real date",
+		},
+		{
+			name:          "Empty cron",
+			schedule:      &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: ""}},
+			expectError:   true,
+			errorContains: "empty cron expression",
+		},
 	}
 
 	for _, tt := range tests {
@@ -123,6 +209,37 @@ func TestValidateSchedule(t *testing.T) {
 	}
 }
 
+// TestResolveScheduleClockZone pins down which zone a cron expression's
+// wall-clock fields are interpreted in. The parser evaluates against the
+// reference time's own location, and ResolveSchedule picks that location from
+// the schedule's clock, so CLOCK_UTC fires on UTC wall-clock rather than the
+// host's local wall-clock.
+func TestResolveScheduleClockZone(t *testing.T) {
+	curTime := time.Date(2023, 10, 1, 10, 0, 0, 0, time.UTC)
+
+	tests := []struct {
+		name  string
+		clock v1.Schedule_Clock
+		zone  *time.Location
+	}{
+		{name: "UTC clock fires on UTC wall-clock", clock: v1.Schedule_CLOCK_UTC, zone: time.UTC},
+		{name: "Local clock fires on local wall-clock", clock: v1.Schedule_CLOCK_LOCAL, zone: time.Local},
+		{name: "Default clock fires on local wall-clock", clock: v1.Schedule_CLOCK_DEFAULT, zone: time.Local},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := protoutil.ResolveSchedule(&v1.Schedule{
+				Clock:    tt.clock,
+				Schedule: &v1.Schedule_Cron{Cron: "0 3 * * *"}, // 03:00 daily
+			}, curTime, curTime)
+			require.NoError(t, err)
+			assert.Equal(t, 3, got.In(tt.zone).Hour(), "expected 03:00 in %v, got %v", tt.zone, got)
+			assert.Equal(t, 0, got.In(tt.zone).Minute())
+		})
+	}
+}
+
 func transactionalTimeEqual(t1, t2 time.Time) bool {
 	return t1.Equal(t2)
 }
@@ -163,6 +280,12 @@ func TestNominalPeriod(t *testing.T) {
 			schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 9 * * 1-5"}},
 			expected: 72 * time.Hour,
 		},
+		{
+			name: "Cron first Monday of month uses widest gap",
+			// First Mondays sit 28 or 35 days apart depending on the month.
+			schedule: &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: "0 0 * * 1#1"}},
+			expected: 35 * 24 * time.Hour,
+		},
 		{
 			name:        "Disabled",
 			schedule:    &v1.Schedule{Schedule: &v1.Schedule_Disabled{Disabled: true}},
@@ -192,3 +315,24 @@ func TestNominalPeriod(t *testing.T) {
 		})
 	}
 }
+
+// FuzzValidateSchedule guards the cron parser against panics on arbitrary
+// expressions. Schedules come from the config file, so a panic here would take
+// down the server rather than surface as a validation error.
+func FuzzValidateSchedule(f *testing.F) {
+	for _, seed := range []string{
+		"* * * * *", "0 0 * * 1#1", "0 0 L * *", "0 0 15W * *", "@daily",
+		"0 0 * * 1-7", "not a cron", "", "* * * * * * *", "0 0 30 2 *",
+	} {
+		f.Add(seed)
+	}
+	f.Fuzz(func(t *testing.T, cron string) {
+		sched := &v1.Schedule{Schedule: &v1.Schedule_Cron{Cron: cron}}
+		if err := protoutil.ValidateSchedule(sched); err != nil {
+			return
+		}
+		// Anything that validates must also resolve and report a period without panicking.
+		_, _ = protoutil.ResolveSchedule(sched, time.Now(), time.Now())
+		_, _ = protoutil.NominalPeriod(sched, time.Now())
+	})
+}