fix: reject overflowing backup and PostgreSQL durations

internal/app/backup_schema.go

@@ -214,6 +214,9 @@ func PositiveDuration(value string) (time.Duration, error) {
 		if err != nil || days <= 0 {
 			return 0, fmt.Errorf("%q is not positive", value)
 		}
+		if days > maxDurationDays {
+			return 0, fmt.Errorf("%q exceeds the maximum representable duration", value)
+		}
 		return time.Duration(days) * 24 * time.Hour, nil
 	}
 	d, err := time.ParseDuration(value)

internal/app/duration_limits_test.go

@@ -0,0 +1,67 @@
+package app
+
+import (
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestPositiveDurationRejectsOverflow(t *testing.T) {
+	for _, value := range []string{"106752d", "213504d", "2147483647d", "9223372036854775807d"} {
+		if got, err := PositiveDuration(value); err == nil || got != 0 {
+			t.Errorf("PositiveDuration(%q) = %v, %v; want refusal", value, got, err)
+		}
+	}
+	for value, want := range map[string]time.Duration{
+		"1d":      24 * time.Hour,
+		"106751d": 106751 * 24 * time.Hour,
+		"15m":     15 * time.Minute,
+	} {
+		if got, err := PositiveDuration(value); err != nil || got != want {
+			t.Errorf("PositiveDuration(%q) = %v, %v; want %v", value, got, err, want)
+		}
+	}
+}
+
+func TestPostgresDurationsRejectOverflowInEveryUnit(t *testing.T) {
+	for _, value := range []string{
+		"9223372037", "9223372037s", "9223372036855ms",
+		"153722868min", "2562048h", "106752d", "213504d",
+		"9223372036854775807s", "9223372036854775808ms",
+	} {
+		if got, ok := ParsePostgresDuration(value); ok || got != 0 {
+			t.Errorf("ParsePostgresDuration(%q) = %v, %v; want refusal", value, got, ok)
+		}
+	}
+	for value, want := range map[string]time.Duration{
+		"9223372036":      9223372036 * time.Second,
+		"9223372036s":     9223372036 * time.Second,
+		"9223372036854ms": 9223372036854 * time.Millisecond,
+		"153722867min":    153722867 * time.Minute,
+		"2562047h":        2562047 * time.Hour,
+		"106751d":         106751 * 24 * time.Hour,
+		" 1 MIN ":         time.Minute,
+		"0ms":             0,
+	} {
+		if got, ok := ParsePostgresDuration(value); !ok || got != want {
+			t.Errorf("ParsePostgresDuration(%q) = %v, %v; want %v", value, got, ok, want)
+		}
+	}
+}
+
+func TestBackupPolicyRejectsOverflowBeforeItCanBecomeAShortWindow(t *testing.T) {
+	for name, tc := range map[string]struct {
+		policy string
+		code   string
+	}{
+		"data loss": {"        maxDataLoss: 213504d\n", "project_invalid"},
+		"retention": {"        maxDataLoss: 15m\n        retention: {window: 213504d}\n", "backup_retention_unsupported"},
+		"drill age": {"        maxDataLoss: 15m\n        drill: {maxAge: 213504d}\n", "project_invalid"},
+	} {
+		t.Run(name, func(t *testing.T) {
+			project := strings.Replace(validBackupProject, "        maxDataLoss: 15m\n", tc.policy, 1)
+			_, err := loadFixtureBytes([]byte(project), "ob.yml")
+			assertAppErrorCode(t, err, tc.code)
+		})
+	}
+}

internal/app/runtime.go

@@ -441,24 +441,32 @@ func ParsePostgresDuration(value string) (time.Duration, bool) {
 	if digits == 0 {
 		return 0, false
 	}
-	count, err := strconv.Atoi(trimmed[:digits])
+	count, err := strconv.ParseInt(trimmed[:digits], 10, 64)
 	if err != nil || count < 0 {
 		return 0, false
 	}
 	unit := strings.ToLower(strings.TrimSpace(trimmed[digits:]))
+	var scale time.Duration
 	switch unit {
 	case "", "s":
-		return time.Duration(count) * time.Second, true
+		scale = time.Second
 	case "ms":
-		return time.Duration(count) * time.Millisecond, true
+		scale = time.Millisecond
 	case "min":
-		return time.Duration(count) * time.Minute, true
+		scale = time.Minute
 	case "h":
-		return time.Duration(count) * time.Hour, true
+		scale = time.Hour
 	case "d":
-		return time.Duration(count) * 24 * time.Hour, true
+		scale = 24 * time.Hour
+	default:
+		return 0, false
+	}
+	// Check before multiplying: overflow can wrap to a plausible positive
+	// duration and make an unsafe server setting satisfy a backup objective.
+	if count > int64((1<<63-1)/scale) {
+		return 0, false
 	}
-	return 0, false
+	return time.Duration(count) * scale, true
 }
 
 // maxDurationDays is the largest whole-day count that fits in int64

internal/engine/backup_archiving_test.go

@@ -0,0 +1,66 @@
+package engine
+
+import (
+	"io"
+	"strings"
+	"testing"
+
+	"github.com/labstack/onebox/internal/app"
+	"github.com/labstack/onebox/internal/transport"
+)
+
+func TestArchivingIssuesDoNotTreatUnusableTimeoutsAsWithinPolicy(t *testing.T) {
+	for _, tc := range []struct {
+		timeout string
+		issue   string
+		policy  string
+	}{
+		{timeout: "900s"},
+		{timeout: "15min"},
+		{timeout: "14min"},
+		{timeout: "16min", issue: "closes a write-ahead log segment"},
+		{timeout: "106752d", issue: "cannot determine"},
+		{timeout: "213504d", issue: "cannot determine"},
+		{timeout: "9223372037s", issue: "cannot determine"},
+		{timeout: "9223372036855ms", issue: "cannot determine"},
+		{timeout: "153722868min", issue: "cannot determine"},
+		{timeout: "2562048h", issue: "cannot determine"},
+		{timeout: "soon", issue: "cannot determine"},
+		{timeout: "0", issue: "disabled"},
+		{timeout: "900s", issue: "cannot determine", policy: "213504d"},
+		{timeout: "900s", issue: "cannot determine", policy: "0"},
+		{timeout: "900s", issue: "cannot determine", policy: "soon"},
+	} {
+		t.Run(tc.timeout+"/"+tc.policy, func(t *testing.T) {
+			policy := tc.policy
+			if policy == "" {
+				policy = "15m"
+			}
+			fake := &transport.Fake{Dynamic: func(command string) (transport.Result, bool) {
+				if strings.Contains(command, "show archive_timeout;") {
+					return transport.Result{Stdout: "on\n" + app.WalgBinary + " wal-push %p\n" + tc.timeout + "\n"}, true
+				}
+				return transport.Result{}, false
+			}}
+			spec := &app.Spec{
+				Name: "shop",
+				Services: map[string]app.Service{
+					"database": {Driver: "postgres", Version: "18", Backup: &app.BackupPolicy{Target: "offsite", MaxDataLoss: policy}},
+				},
+				BackupTargets: map[string]app.BackupTarget{"offsite": {}},
+			}
+			e := New(&app.Resolved{Spec: spec, Env: "production"}, nil, fake, Options{Out: io.Discard})
+			issues, err := e.archivingIssues(t.Context(), "database")
+			if err != nil {
+				t.Fatal(err)
+			}
+			if tc.issue == "" {
+				if len(issues) != 0 {
+					t.Fatalf("valid timeout raised issues: %v", issues)
+				}
+			} else if len(issues) != 1 || !strings.Contains(issues[0], tc.issue) {
+				t.Fatalf("timeout %q silently accepted or misreported: %v", tc.timeout, issues)
+			}
+		})
+	}
+}

internal/engine/backup_postgres.go

@@ -556,8 +556,22 @@ func (e *Engine) archivingIssues(ctx context.Context, service string) ([]string,
 		issues = append(issues, fmt.Sprintf(
 			"the server's archive_command is not the one Onebox installed, so where the write-ahead log goes is not what this project describes; re-run `ob backup enable %s`", service))
 	}
-	if declared, ok := app.ParseDuration(projection.Policy.MaxDataLoss); ok {
-		if observed, parsed := app.ParsePostgresDuration(timeout); parsed && observed > declared {
+	declared, policyErr := app.PositiveDuration(projection.Policy.MaxDataLoss)
+	if policyErr != nil {
+		issues = append(issues, fmt.Sprintf(
+			"cannot determine whether archiving satisfies the maximum data loss policy %q: %v",
+			projection.Policy.MaxDataLoss, policyErr))
+	} else {
+		observed, parsed := app.ParsePostgresDuration(timeout)
+		switch {
+		case !parsed:
+			issues = append(issues, fmt.Sprintf(
+				"cannot determine whether archive_timeout %q satisfies the maximum data loss policy %s; the server value is invalid or exceeds the maximum representable duration",
+				timeout, projection.Policy.MaxDataLoss))
+		case observed == 0:
+			issues = append(issues, fmt.Sprintf(
+				"the server has archive_timeout disabled, so an idle database can lose more than the policy permits; re-run `ob backup enable %s`", service))
+		case observed > declared:
 			issues = append(issues, fmt.Sprintf(
 				"the server closes a write-ahead log segment every %s, but the policy tolerates losing at most %s; an idle database can lose more than the policy permits",
 				timeout, projection.Policy.MaxDataLoss))