feat: enhance restic permission validation and add unit tests

Author: caffeinated92

config/config.go

@@ -3788,10 +3788,24 @@ func (conf *Config) CheckKeepWithin() error {
 	return nil
 }
 
+func isValidResticMode(value int) bool {
+	if value == 0 {
+		return true
+	}
+	if value < 600 || value > 777 {
+		return false
+	}
+	_, err := strconv.ParseUint(strconv.Itoa(value), 8, 32)
+	return err == nil
+}
+
 func parseResticMode(value int, defaultMode os.FileMode) os.FileMode {
 	if value <= 0 {
 		return defaultMode
 	}
+	if !isValidResticMode(value) {
+		return defaultMode
+	}
 
 	parsed, err := strconv.ParseUint(strconv.Itoa(value), 8, 32)
 	if err != nil {
@@ -3802,21 +3816,11 @@ func parseResticMode(value int, defaultMode os.FileMode) os.FileMode {
 }
 
 func (conf *Config) ValidateResticPermissions() error {
-	if conf.BackupResticDirMode < 0 {
-		return NewValidationError("backup-restic-dir-mode", conf.BackupResticDirMode, "expected octal value like 700")
-	}
-	if conf.BackupResticDirMode != 0 {
-		if _, err := strconv.ParseUint(strconv.Itoa(conf.BackupResticDirMode), 8, 32); err != nil {
-			return NewValidationError("backup-restic-dir-mode", conf.BackupResticDirMode, "expected octal value like 700")
-		}
-	}
-	if conf.BackupResticFileMode < 0 {
-		return NewValidationError("backup-restic-file-mode", conf.BackupResticFileMode, "expected octal value like 600")
+	if !isValidResticMode(conf.BackupResticDirMode) {
+		return NewValidationError("backup-restic-dir-mode", conf.BackupResticDirMode, "expected octal value in 6xx/7xx range, like 700")
 	}
-	if conf.BackupResticFileMode != 0 {
-		if _, err := strconv.ParseUint(strconv.Itoa(conf.BackupResticFileMode), 8, 32); err != nil {
-			return NewValidationError("backup-restic-file-mode", conf.BackupResticFileMode, "expected octal value like 600")
-		}
+	if !isValidResticMode(conf.BackupResticFileMode) {
+		return NewValidationError("backup-restic-file-mode", conf.BackupResticFileMode, "expected octal value in 6xx/7xx range, like 600")
 	}
 	return nil
 }

config/restic_permissions_test.go

@@ -0,0 +1,54 @@
+package config
+
+import (
+	"os"
+	"testing"
+)
+
+func TestParseResticMode(t *testing.T) {
+	defaultMode := os.FileMode(0700)
+
+	if got := parseResticMode(700, defaultMode); got != 0700 {
+		t.Fatalf("expected 700 to parse as 0700, got %#o", got)
+	}
+	if got := parseResticMode(600, defaultMode); got != 0600 {
+		t.Fatalf("expected 600 to parse as 0600, got %#o", got)
+	}
+	if got := parseResticMode(755, defaultMode); got != 0755 {
+		t.Fatalf("expected 755 to parse as 0755, got %#o", got)
+	}
+	if got := parseResticMode(400, defaultMode); got != defaultMode {
+		t.Fatalf("expected 400 to fallback to default, got %#o", got)
+	}
+	if got := parseResticMode(888, defaultMode); got != defaultMode {
+		t.Fatalf("expected 888 to fallback to default, got %#o", got)
+	}
+}
+
+func TestValidateResticPermissions(t *testing.T) {
+	conf := &Config{}
+
+	conf.BackupResticDirMode = 700
+	conf.BackupResticFileMode = 600
+	if err := conf.ValidateResticPermissions(); err != nil {
+		t.Fatalf("expected valid permissions, got error: %v", err)
+	}
+
+	conf.BackupResticDirMode = 400
+	conf.BackupResticFileMode = 600
+	if err := conf.ValidateResticPermissions(); err == nil {
+		t.Fatalf("expected error for invalid dir mode 400")
+	}
+
+	conf.BackupResticDirMode = 700
+	conf.BackupResticFileMode = 888
+	if err := conf.ValidateResticPermissions(); err == nil {
+		t.Fatalf("expected error for invalid file mode 888")
+	}
+
+	conf.BackupResticDirMode = 0
+	conf.BackupResticFileMode = 0
+	if err := conf.ValidateResticPermissions(); err != nil {
+		t.Fatalf("expected zero values to be valid, got error: %v", err)
+	}
+}

doc/implementation/config/RESTIC_PERMISSION_VALIDATION.md

@@ -0,0 +1,49 @@
+# Restic Permission Validation
+
+## Summary
+
+Restic permission modes accept only octal digit values in the 6xx or 7xx range
+(e.g., 600, 700, 750, 755). This keeps configuration consistent with OpenSVC's
+permission inputs (string values like "700"/"600").
+
+## Entry Points
+
+- config/config.go
+  - parseResticMode() converts octal digit values to os.FileMode.
+  - ValidateResticPermissions() enforces allowed ranges and returns
+    configuration validation errors.
+- server/api_cluster.go
+  - API updates for backup-restic-dir-mode and backup-restic-file-mode
+    validate inputs and return errors if invalid.
+- cluster/cluster_bck.go
+  - StartResticManager() calls ValidateResticPermissions() and logs a warning
+    on invalid inputs.
+
+## Rules
+
+- Valid values: 600-777 (octal digits only)
+- Zero value: 0 means "use defaults" (0700 for dirs, 0600 for files)
+- Invalid values (examples): 400, 888, -1, 0o700 (parsed to 448 and rejected)
+
+## Behavior
+
+- API updates with invalid values return errors and do not apply changes.
+- Invalid values from config files are rejected by validation; defaults are
+  used when values are zero or invalid.
+
+## Rationale
+
+- Aligns with OpenSVC permission formatting (string octal digits).
+- Avoids ambiguous interpretation of decimal values.
+- Ensures stronger defaults for security (owner-only permissions).
+
+## Examples
+
+Valid:
+- backup-restic-dir-mode = 700
+- backup-restic-file-mode = 600
+
+Invalid:
+- backup-restic-dir-mode = 400
+- backup-restic-file-mode = 888
+

server/api_cluster.go

@@ -2804,20 +2804,39 @@ func (repman *ReplicationManager) setClusterSetting(mycluster *cluster.Cluster,
 		val, _ := strconv.Atoi(value)
 		mycluster.Conf.BackupResticPurgeOldestOnDiskThreshold = val
 	case "backup-restic-timeout":
-		val, _ := strconv.Atoi(value)
+		val, err := strconv.Atoi(value)
+		if err != nil {
+			return fmt.Errorf("invalid value for backup-restic-timeout: %q", value)
+		}
 		mycluster.Conf.BackupResticTimeout = val
 		if mycluster.ResticManager != nil {
 			mycluster.ResticManager.SetOperationTimeout(mycluster.Conf.GetResticTimeout())
 		}
 	case "backup-restic-dir-mode":
-		val, _ := strconv.Atoi(value)
+		val, err := strconv.Atoi(value)
+		if err != nil {
+			return fmt.Errorf("invalid value for backup-restic-dir-mode: %q", value)
+		}
+		oldValue := mycluster.Conf.BackupResticDirMode
 		mycluster.Conf.BackupResticDirMode = val
+		if err := mycluster.Conf.ValidateResticPermissions(); err != nil {
+			mycluster.Conf.BackupResticDirMode = oldValue
+			return err
+		}
 		if mycluster.ResticManager != nil {
 			mycluster.ResticManager.SetPermissions(mycluster.Conf.GetResticDirMode(), mycluster.Conf.GetResticFileMode())
 		}
 	case "backup-restic-file-mode":
-		val, _ := strconv.Atoi(value)
+		val, err := strconv.Atoi(value)
+		if err != nil {
+			return fmt.Errorf("invalid value for backup-restic-file-mode: %q", value)
+		}
+		oldValue := mycluster.Conf.BackupResticFileMode
 		mycluster.Conf.BackupResticFileMode = val
+		if err := mycluster.Conf.ValidateResticPermissions(); err != nil {
+			mycluster.Conf.BackupResticFileMode = oldValue
+			return err
+		}
 		if mycluster.ResticManager != nil {
 			mycluster.ResticManager.SetPermissions(mycluster.Conf.GetResticDirMode(), mycluster.Conf.GetResticFileMode())
 		}