Repository: kopia/kopia
Pull request #4720 (2025-07-15 20:39:02)

refactor(general): cleanup clock bounds check
- Followup to #4719


julio-lopez: FYI @abhishekm2610

Patch:
diff --git a/repo/content/sessions.go b/repo/content/sessions.go
index e4cf5877..150d1547 100644
--- a/repo/content/sessions.go
+++ b/repo/content/sessions.go
@@ -6,6 +6,7 @@
 	"encoding/json"
 	"fmt"
 	"os"
+	"strconv"
 	"strings"
 	"time"
 
@@ -57,6 +58,23 @@ func checkClockSkewBounds(localTime, modTime time.Time) error {
 	return nil
 }
 
+func maybeCheckClockSkewBounds(localTime, modTime time.Time) error {
+	v, found := os.LookupEnv("KOPIA_ENABLE_CLOCK_SKEW_CHECK")
+	if !found {
+		return nil
+	}
+
+	if enabled, err := strconv.ParseBool(v); err == nil && !enabled {
+		// err was nil and the value explicitly disabled the check, for example
+		// KOPIA_ENABLE_CLOCK_SKEW_CHECK=false
+		return nil
+	}
+
+	// Perform the check by default when the environment variable is set and
+	// is not a boolean, for example KOPIA_ENABLE_CLOCK_SKEW_CHECK=foo
+	return checkClockSkewBounds(localTime, modTime)
+}
+
 // generateSessionID generates a random session identifier.
 func generateSessionID(now time.Time) (SessionID, error) {
 	// generate session ID as {random-64-bit}{epoch-number}
@@ -142,12 +160,8 @@ func (bm *WriteManager) writeSessionMarkerLocked(ctx context.Context) error {
 		return errors.Wrapf(err, "unable to write session marker: %v", string(sessionBlobID))
 	}
 
-	if v, found := os.LookupEnv("KOPIA_ENABLE_CLOCK_SKEW_CHECK"); found {
-		if !strings.EqualFold(v, "false") && v != "0" {
-			if skewError := checkClockSkewBounds(bm.timeNow(), modTime); skewError != nil {
-				return errors.Wrap(skewError, "while writing session marker")
-			}
-		}
+	if err := maybeCheckClockSkewBounds(bm.timeNow(), modTime); err != nil {
+		return errors.Wrap(err, "unable to check for clock skew after writing session marker")
 	}
 
 	bm.sessionMarkerBlobIDs = append(bm.sessionMarkerBlobIDs, sessionBlobID)


Test patch:
diff --git a/repo/content/sessions_test.go b/repo/content/sessions_test.go
index 73d3e57a..0e2bbd9d 100644
--- a/repo/content/sessions_test.go
+++ b/repo/content/sessions_test.go
@@ -76,6 +76,45 @@ func TestCheckClockSkewBounds_Negative(t *testing.T) {
 	require.Contains(t, err.Error(), "clock skew detected")
 }
 
+func TestMaybeCheckClockSkewBounds_Disabled(t *testing.T) {
+	now := clock.Now()
+
+	for _, tc := range []time.Time{
+		now.Add(-maxClockSkew - time.Hour),
+		now.Add(-maxClockSkew),
+		now,
+		now.Add(maxClockSkew),
+		now.Add(maxClockSkew + 10*time.Hour),
+	} {
+		t.Run(tc.String(), func(t *testing.T) {
+			// KOPIA_ENABLE_CLOCK_SKEW_CHECK is not set
+			err := maybeCheckClockSkewBounds(now, tc)
+			require.NoError(t, err)
+
+			t.Setenv("KOPIA_ENABLE_CLOCK_SKEW_CHECK", "false")
+
+			err = maybeCheckClockSkewBounds(now, tc)
+			require.NoError(t, err)
+		})
+	}
+}
+
+func TestMaybeCheckClockSkewBounds_Enabled(t *testing.T) {
+	t.Setenv("KOPIA_ENABLE_CLOCK_SKEW_CHECK", "true")
+
+	now := clock.Now()
+
+	for _, tc := range []time.Time{
+		now.Add(-maxClockSkew - time.Hour),
+		now.Add(maxClockSkew + 10*time.Hour),
+	} {
+		t.Run(tc.String(), func(t *testing.T) {
+			err := maybeCheckClockSkewBounds(now, tc)
+			require.Error(t, err)
+		})
+	}
+}
+
 func TestWriteSessionMarkerLockedWithoutClockSkew(t *testing.T) {
 	t.Setenv("KOPIA_ENABLE_CLOCK_SKEW_CHECK", "1")
 
@@ -115,7 +154,7 @@ func TestWriteSessionMarkerLockedWithClockSkew(t *testing.T) {
 	keyTime := map[blob.ID]time.Time{}
 
 	bmTime := faketime.NewTimeAdvance(time.Date(2018, time.January, 1, 0, 0, 0, 0, time.UTC))
-	stTime := faketime.NewTimeAdvance(time.Date(2018, time.January, 1, 0, int(maxClockSkew), 0, 0, time.UTC))
+	stTime := faketime.NewTimeAdvance(bmTime.NowFunc()().Add(maxClockSkew + time.Nanosecond))
 	st := blobtesting.NewMapStorage(data, keyTime, stTime.NowFunc())
 
 	bm, err := NewManagerForTesting(testlogging.Context(t), st, mustCreateFormatProvider(t, &format.ContentFormat{