Add path validation and deny app-root from minion

Author: AlexFenlon

internal/configs/annotations.go

@@ -75,6 +75,7 @@ var minionDenylist = map[string]bool{
 	"nginx.org/server-snippets":                         true,
 	"nginx.org/ssl-ciphers":                             true,
 	"nginx.org/ssl-prefer-server-ciphers":               true,
+	"nginx.org/app-root":                                true,
 	"appprotect.f5.com/app_protect_enable":              true,
 	"appprotect.f5.com/app_protect_policy":              true,
 	"appprotect.f5.com/app_protect_security_log_enable": true,
@@ -504,7 +505,11 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool
 	}
 
 	if appRoot, exists := ingEx.Ingress.Annotations["nginx.org/app-root"]; exists {
-		cfgParams.AppRoot = appRoot
+		if !VerifyPath(appRoot) {
+			nl.Errorf(l, "Ingress %s/%s: Invalid value nginx.org/app-root: got %q. Must start with '/'", ingEx.Ingress.GetNamespace(), ingEx.Ingress.GetName(), appRoot)
+		} else {
+			cfgParams.AppRoot = appRoot
+		}
 	}
 
 	if useClusterIP, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, UseClusterIPAnnotation, ingEx.Ingress); exists {

internal/configs/annotations_test.go

@@ -1011,3 +1011,113 @@ func TestSSLRedirectAnnotations(t *testing.T) {
 		})
 	}
 }
+
+func TestAppRootAnnotation(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		annotations map[string]string
+		expected    string
+	}{
+		{
+			name: "valid app-root - coffee path",
+			annotations: map[string]string{
+				"nginx.org/app-root": "/coffee",
+			},
+			expected: "/coffee",
+		},
+		{
+			name: "valid app-root - nested path with mocha",
+			annotations: map[string]string{
+				"nginx.org/app-root": "/coffee/mocha",
+			},
+			expected: "/coffee/mocha",
+		},
+		{
+			name: "valid app-root - tea path",
+			annotations: map[string]string{
+				"nginx.org/app-root": "/tea",
+			},
+			expected: "/tea",
+		},
+		{
+			name: "valid app-root - nested tea path",
+			annotations: map[string]string{
+				"nginx.org/app-root": "/tea/green-tea",
+			},
+			expected: "/tea/green-tea",
+		},
+		{
+			name: "valid app-root - cafe path",
+			annotations: map[string]string{
+				"nginx.org/app-root": "/cafe",
+			},
+			expected: "/cafe",
+		},
+		{
+			name: "invalid app-root - does not start with slash",
+			annotations: map[string]string{
+				"nginx.org/app-root": "coffee",
+			},
+			expected: "", // Should remain empty due to invalid path
+		},
+		{
+			name: "invalid app-root - contains invalid characters",
+			annotations: map[string]string{
+				"nginx.org/app-root": "/tea$mocha",
+			},
+			expected: "", // Should remain empty due to invalid characters
+		},
+		{
+			name: "invalid app-root - contains curly braces",
+			annotations: map[string]string{
+				"nginx.org/app-root": "/coffee{test}",
+			},
+			expected: "", // Should remain empty due to invalid characters
+		},
+		{
+			name: "invalid app-root - contains semicolon",
+			annotations: map[string]string{
+				"nginx.org/app-root": "/tea;chai",
+			},
+			expected: "", // Should remain empty due to invalid characters
+		},
+		{
+			name: "invalid app-root - contains whitespace",
+			annotations: map[string]string{
+				"nginx.org/app-root": "/tea chai",
+			},
+			expected: "", // Should remain empty due to invalid characters
+		},
+		{
+			name:        "no app-root annotation",
+			annotations: map[string]string{},
+			expected:    "", // Should remain empty when annotation is missing
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			ingEx := &IngressEx{
+				Ingress: &networking.Ingress{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:        "test-ingress",
+						Namespace:   "default",
+						Annotations: tt.annotations,
+					},
+				},
+			}
+
+			baseCfgParams := NewDefaultConfigParams(context.Background(), false)
+			result := parseAnnotations(ingEx, baseCfgParams, false, false, false, false, false)
+
+			if result.AppRoot != tt.expected {
+				t.Errorf("Test %q: expected AppRoot %q, got %q", tt.name, tt.expected, result.AppRoot)
+			}
+		})
+	}
+}

internal/k8s/validation.go

@@ -394,11 +394,8 @@ func validateAppRootAnnotation(context *annotationValidationContext) field.Error
 		return allErrs
 	}
 
-	// Validate that the path doesn't contain invalid characters
-	// Allow alphanumeric, hyphens, underscores, dots, and forward slashes
-	validPath := regexp.MustCompile(`^/[a-zA-Z0-9\-_./]*$`)
-	if !validPath.MatchString(path) {
-		allErrs = append(allErrs, field.Invalid(context.fieldPath, path, "contains invalid characters, only alphanumeric, hyphens, underscores, dots, and forward slashes are allowed"))
+	if !configs.VerifyPath(path) {
+		allErrs = append(allErrs, field.Invalid(context.fieldPath, path, "path must start with '/' and must not include any special character, '{', '}', ';' or '$'"))
 		return allErrs
 	}
 

internal/k8s/validation_test.go

@@ -3485,7 +3485,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) {
 			appProtectDosEnabled:  false,
 			internalRoutesEnabled: false,
 			expectedErrors: []string{
-				`annotations.nginx.org/app-root: Invalid value: "/tea$1": contains invalid characters, only alphanumeric, hyphens, underscores, dots, and forward slashes are allowed`,
+				`annotations.nginx.org/app-root: Invalid value: "/tea$1": path must start with '/' and must not include any special character, '{', '}', ';' or '$'`,
 			},
 			msg: "invalid nginx.org/app-root annotation, invalid characters",
 		},