Skip to content

Commit dd99942

Browse files
fix(security): validate node labels and IPs before shell interpolation (#656)
User-controlled label keys/values from YAML config were directly interpolated into kubectl commands via fmt.Sprintf, enabling command injection. Add label validation against Kubernetes label pattern. Also validate PrivateIP with net.ParseIP before grep interpolation. Audit findings #17 (MEDIUM), #18 (MEDIUM). Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
1 parent 7aa5bce commit dd99942

File tree

2 files changed

+33
-0
lines changed

2 files changed

+33
-0
lines changed

api/holodeck/v1alpha1/validation.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,23 @@ package v1alpha1
1818

1919
import (
2020
"fmt"
21+
"regexp"
2122
)
2223

24+
var k8sLabelPattern = regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9._\-/]*[a-zA-Z0-9])?$`)
25+
26+
func validateLabels(labels map[string]string) error {
27+
for k, v := range labels {
28+
if !k8sLabelPattern.MatchString(k) {
29+
return fmt.Errorf("invalid label key %q: contains disallowed characters", k)
30+
}
31+
if v != "" && !k8sLabelPattern.MatchString(v) {
32+
return fmt.Errorf("invalid label value %q for key %q: contains disallowed characters", v, k)
33+
}
34+
}
35+
return nil
36+
}
37+
2338
// Validate validates the ClusterSpec configuration.
2439
func (c *ClusterSpec) Validate() error {
2540
if c == nil {
@@ -43,6 +58,16 @@ func (c *ClusterSpec) Validate() error {
4358
}
4459
}
4560

61+
// Validate labels for shell-injection safety
62+
if err := validateLabels(c.ControlPlane.Labels); err != nil {
63+
return fmt.Errorf("control-plane labels: %w", err)
64+
}
65+
if c.Workers != nil {
66+
if err := validateLabels(c.Workers.Labels); err != nil {
67+
return fmt.Errorf("worker labels: %w", err)
68+
}
69+
}
70+
4671
// Validate HA configuration
4772
if c.HighAvailability != nil {
4873
if err := c.HighAvailability.Validate(c.ControlPlane.Count); err != nil {

pkg/provisioner/cluster.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"bytes"
2121
"fmt"
2222
"io"
23+
"net"
2324
"os"
2425
"strings"
2526
"sync"
@@ -441,6 +442,13 @@ func (cp *ClusterProvisioner) configureNodes(firstCP NodeInfo, nodes []NodeInfo)
441442
}
442443
defer provisioner.Client.Close() // nolint: errcheck
443444

445+
// Validate all node IPs before interpolating into shell commands
446+
for _, node := range nodes {
447+
if net.ParseIP(node.PrivateIP) == nil {
448+
return fmt.Errorf("invalid private IP for node %s: %q", node.Name, node.PrivateIP)
449+
}
450+
}
451+
444452
// Build the node configuration script
445453
// Note: Use sudo -E to preserve KUBECONFIG environment variable, or use --kubeconfig flag
446454
var script strings.Builder

0 commit comments

Comments
 (0)