OCPNODE-4526: Add '..' block, max-length, and cross-store uniqueness

pkg/controller/container-runtime-config/helpers.go

@@ -728,27 +728,60 @@ func validateUserContainerRuntimeConfig(cfg *mcfgv1.ContainerRuntimeConfig) erro
 		}
 	}
 
+	// Cross-store-type path uniqueness: the same path must not appear in
+	// multiple store type lists since they have different directory
+	// structure expectations and lockfile locations.
+	layerPaths := make(map[string]bool, len(ctrcfg.AdditionalLayerStores))
+	for _, s := range ctrcfg.AdditionalLayerStores {
+		layerPaths[filepath.Clean(string(s.Path))] = true
+	}
+	imagePaths := make(map[string]bool, len(ctrcfg.AdditionalImageStores))
+	for _, s := range ctrcfg.AdditionalImageStores {
+		cleaned := filepath.Clean(string(s.Path))
+		imagePaths[cleaned] = true
+		if layerPaths[cleaned] {
+			errs = append(errs, fmt.Errorf("path %q is used in both AdditionalLayerStores and AdditionalImageStores", s.Path))
+		}
+	}
+	for _, s := range ctrcfg.AdditionalArtifactStores {
+		cleaned := filepath.Clean(string(s.Path))
+		if layerPaths[cleaned] {
+			errs = append(errs, fmt.Errorf("path %q is used in both AdditionalLayerStores and AdditionalArtifactStores", s.Path))
+		}
+		if imagePaths[cleaned] {
+			errs = append(errs, fmt.Errorf("path %q is used in both AdditionalImageStores and AdditionalArtifactStores", s.Path))
+		}
+	}
+
 	return errors.Join(errs...)
 }
 
 // storePathRegexp matches the CRD CEL rule: ^/[a-zA-Z0-9/._-]+$
 var storePathRegexp = regexp.MustCompile(`^/[a-zA-Z0-9/._-]+$`)
 
-// validateStorePath validates that a storage path is absolute, only contains
-// allowed characters (a-z, A-Z, 0-9, '/', '.', '_', '-'), and does not
-// contain consecutive forward slashes. This mirrors the CRD-level CEL
-// validation so that invalid config is caught early.
+// storePathMaxLength mirrors the CRD MaxLength=256 for StorePath.
+const storePathMaxLength = 256
+
+// validateStorePath mirrors the CRD-level CEL validation for StorePath.
 func validateStorePath(p mcfgv1.StorePath, field string) error {
 	path := string(p)
 	if path == "" {
 		return fmt.Errorf("invalid %s: path must not be empty", field)
 	}
+	if len(path) > storePathMaxLength {
+		return fmt.Errorf("invalid %s path %q: must not exceed %d characters", field, path, storePathMaxLength)
+	}
 	if !storePathRegexp.MatchString(path) {
 		return fmt.Errorf("invalid %s path %q: must be an absolute path containing only alphanumeric characters, '/', '.', '_', and '-'", field, path)
 	}
 	if strings.Contains(path, "//") {
 		return fmt.Errorf("invalid %s path %q: must not contain consecutive forward slashes", field, path)
 	}
+	for _, component := range strings.Split(path, "/") {
+		if component == ".." {
+			return fmt.Errorf("invalid %s path %q: must not contain '..' components", field, path)
+		}
+	}
 	return nil
 }
 

pkg/controller/container-runtime-config/helpers_test.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"os"
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
@@ -1548,6 +1549,10 @@ func TestValidateStorePath(t *testing.T) {
 			name: "valid path with dots and dashes",
 			path: "/mnt/nfs-images/cache_v1.0",
 		},
+		{
+			name: "double dots within filename",
+			path: "/var/lib/foo..bar",
+		},
 		{
 			name:    "empty path",
 			path:    "",
@@ -1578,6 +1583,35 @@ func TestValidateStorePath(t *testing.T) {
 			wantErr: true,
 			errMsg:  "must not contain consecutive forward slashes",
 		},
+		{
+			name:    "dot-dot traversal",
+			path:    "/var/lib/../../etc",
+			wantErr: true,
+			errMsg:  "must not contain '..' components",
+		},
+		{
+			name:    "dot-dot at end",
+			path:    "/mnt/store/..",
+			wantErr: true,
+			errMsg:  "must not contain '..' components",
+		},
+		{
+			name:    "path with colon",
+			path:    "/var/lib/store:ref",
+			wantErr: true,
+			errMsg:  "must be an absolute path",
+		},
+		{
+			name:    "path at max length",
+			path:    mcfgv1.StorePath("/" + strings.Repeat("a", 255)),
+			wantErr: false,
+		},
+		{
+			name:    "path exceeds max length",
+			path:    mcfgv1.StorePath("/" + strings.Repeat("a", 256)),
+			wantErr: true,
+			errMsg:  "must not exceed 256 characters",
+		},
 	}
 
 	for _, test := range tests {
@@ -1656,6 +1690,70 @@ func TestValidateUserContainerRuntimeConfigAdditionalStores(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "cross-store duplicate layer and image",
+			cfg: &mcfgv1.ContainerRuntimeConfig{
+				Spec: mcfgv1.ContainerRuntimeConfigSpec{
+					ContainerRuntimeConfig: &mcfgv1.ContainerRuntimeConfiguration{
+						AdditionalLayerStores: []mcfgv1.AdditionalLayerStore{
+							{Path: "/mnt/shared"},
+						},
+						AdditionalImageStores: []mcfgv1.AdditionalImageStore{
+							{Path: "/mnt/shared"},
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "cross-store duplicate layer and artifact",
+			cfg: &mcfgv1.ContainerRuntimeConfig{
+				Spec: mcfgv1.ContainerRuntimeConfigSpec{
+					ContainerRuntimeConfig: &mcfgv1.ContainerRuntimeConfiguration{
+						AdditionalLayerStores: []mcfgv1.AdditionalLayerStore{
+							{Path: "/mnt/shared"},
+						},
+						AdditionalArtifactStores: []mcfgv1.AdditionalArtifactStore{
+							{Path: "/mnt/shared"},
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "cross-store duplicate image and artifact",
+			cfg: &mcfgv1.ContainerRuntimeConfig{
+				Spec: mcfgv1.ContainerRuntimeConfigSpec{
+					ContainerRuntimeConfig: &mcfgv1.ContainerRuntimeConfiguration{
+						AdditionalImageStores: []mcfgv1.AdditionalImageStore{
+							{Path: "/mnt/shared"},
+						},
+						AdditionalArtifactStores: []mcfgv1.AdditionalArtifactStore{
+							{Path: "/mnt/shared"},
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "cross-store duplicate with trailing slash normalization",
+			cfg: &mcfgv1.ContainerRuntimeConfig{
+				Spec: mcfgv1.ContainerRuntimeConfigSpec{
+					ContainerRuntimeConfig: &mcfgv1.ContainerRuntimeConfiguration{
+						AdditionalLayerStores: []mcfgv1.AdditionalLayerStore{
+							{Path: "/mnt/shared"},
+						},
+						AdditionalImageStores: []mcfgv1.AdditionalImageStore{
+							{Path: "/mnt/shared/"},
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
 	}
 
 	for _, test := range tests {