Merge pull request #25 from spapa013/spapadop/plt-881-volumemount-path-validation-incorrectly-uses-filepath

Spapadop/plt 881 volumemount path validation incorrectly uses filepath

Author: spapa013

internal/config/types.go

@@ -94,8 +94,8 @@ type ResourceConfig struct {
 // VolumeMount represents a volume mount configuration
 type VolumeMount struct {
 	Name          string `yaml:"name" validate:"required,min=1,max=63,alphanum"`
-	LocalPath     string `yaml:"localPath" validate:"required,min=1,filepath"`
-	ContainerPath string `yaml:"containerPath" validate:"required,min=1,filepath"`
+	LocalPath     string `yaml:"localPath" validate:"required,mount_path"`
+	ContainerPath string `yaml:"containerPath" validate:"required,mount_path"`
 }
 
 // RefreshConfig represents auto-refresh settings

internal/config/validation.go

@@ -57,6 +57,9 @@ func init() {
 	if err := validate.RegisterValidation("k8s_memory", validateKubernetesMemory); err != nil {
 		panic(fmt.Errorf("register validator k8s_memory: %w", err))
 	}
+	if err := validate.RegisterValidation("mount_path", validateMountPath); err != nil {
+		panic(fmt.Errorf("register validator mount_path: %w", err))
+	}
 	validate.RegisterStructValidation(validateGitRepo, GitRepo{})
 }
 
@@ -188,6 +191,33 @@ func validateKubernetesMemory(fl validator.FieldLevel) bool {
 	}
 }
 
+// validateMountPath implements the "mount_path" tag.
+// It validates mount paths using syntax only (no filesystem checks).
+func validateMountPath(fl validator.FieldLevel) bool {
+	p, ok := fl.Field().Interface().(string)
+	if !ok {
+		return false
+	}
+
+	p = strings.TrimSpace(p)
+	if p == "" {
+		return false
+	}
+
+	// Kubernetes-style mount paths are absolute, slash-separated paths.
+	if !path.IsAbs(p) {
+		return false
+	}
+
+	// Reject NUL bytes; otherwise rely on lexical cleaning only.
+	if strings.ContainsRune(p, '\x00') {
+		return false
+	}
+
+	clean := path.Clean(p)
+	return clean != "" && clean != "."
+}
+
 // ValidateDevEnvConfig runs tag-based validation and then applies
 // additional semantic checks that are easier to express in code.
 func ValidateDevEnvConfig(config *DevEnvConfig) error {
@@ -282,6 +312,8 @@ func formatFieldError(fieldError validator.FieldError) string {
 		return fmt.Sprintf("'%s' must be a valid URL, got '%v'", fieldName, value)
 	case "filepath":
 		return fmt.Sprintf("'%s' must be a valid file path, got '%v'", fieldName, value)
+	case "mount_path":
+		return fmt.Sprintf("'%s' must be a valid absolute mount path, got '%v'", fieldName, value)
 	case "cron":
 		return fmt.Sprintf("'%s' must be a valid cron expression, got '%v'", fieldName, value)
 

internal/config/validation_test.go

@@ -279,3 +279,71 @@ func TestValidateDevEnvConfig_PythonBinPathMustBeAbsolute(t *testing.T) {
 	assert.Contains(t, err.Error(), "pythonBinPath")
 	assert.Contains(t, err.Error(), "absolute path")
 }
+
+func TestValidator_MountPath(t *testing.T) {
+	type S struct {
+		Path string `validate:"mount_path"`
+	}
+
+	cases := []struct {
+		name string
+		val  string
+		ok   bool
+	}{
+		{name: "root mount dir", val: "/mnt", ok: true},
+		{name: "mount subpath", val: "/mnt/data", ok: true},
+		{name: "root", val: "/", ok: true},
+		{name: "empty", val: "", ok: false},
+		{name: "whitespace", val: "   ", ok: false},
+		{name: "relative path", val: "mnt/data", ok: false},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := validate.Struct(&S{Path: tc.val})
+			if tc.ok {
+				require.NoError(t, err)
+			} else {
+				require.Error(t, err)
+			}
+		})
+	}
+}
+
+func TestValidateDevEnvConfig_VolumeMountPaths(t *testing.T) {
+	newCfg := func(localPath, containerPath string) *DevEnvConfig {
+		return &DevEnvConfig{
+			Name: "alice",
+			BaseConfig: BaseConfig{
+				SSHPublicKey: "ssh-ed25519 AAAAB3NzaC1lZDI1NTE5AAAA user@host",
+				Volumes: []VolumeMount{
+					{
+						Name:          "mnt",
+						LocalPath:     localPath,
+						ContainerPath: containerPath,
+					},
+				},
+			},
+		}
+	}
+
+	t.Run("accepts root directory mounts", func(t *testing.T) {
+		require.NoError(t, ValidateDevEnvConfig(newCfg("/mnt", "/mnt")))
+	})
+
+	t.Run("accepts mount subpaths", func(t *testing.T) {
+		require.NoError(t, ValidateDevEnvConfig(newCfg("/mnt/data", "/mnt/data")))
+	})
+
+	t.Run("rejects empty localPath", func(t *testing.T) {
+		err := ValidateDevEnvConfig(newCfg("", "/mnt"))
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "LocalPath")
+	})
+
+	t.Run("rejects empty containerPath", func(t *testing.T) {
+		err := ValidateDevEnvConfig(newCfg("/mnt", ""))
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "ContainerPath")
+	})
+}