Validate that all allowed network IPs are specified (#2150)

Author: dobrac

packages/api/internal/handlers/sandbox_create.go

@@ -513,7 +513,7 @@ func validateNetworkConfig(network *api.SandboxNetworkConfig) *api.APIError {
 // - when allowOut contains domains, denyOut must include 0.0.0.0/0
 func validateEgressRules(allowOut, denyOut []string) *api.APIError {
 	for _, cidr := range denyOut {
-		if !sandbox_network.IsIPOrCIDR(cidr) {
+		if !sandbox_network.IsSpecifiedIPOrCIDR(cidr) {
 			return &api.APIError{
 				Code:      http.StatusBadRequest,
 				Err:       fmt.Errorf("invalid denied CIDR %s", cidr),
@@ -523,7 +523,17 @@ func validateEgressRules(allowOut, denyOut []string) *api.APIError {
 	}
 
 	if len(allowOut) > 0 {
-		_, allowedDomains := sandbox_network.ParseAddressesAndDomains(allowOut)
+		allowedAddresses, allowedDomains := sandbox_network.ParseAddressesAndDomains(allowOut)
+
+		for _, addr := range allowedAddresses {
+			if !sandbox_network.IsSpecifiedIPOrCIDR(addr) {
+				return &api.APIError{
+					Code:      http.StatusBadRequest,
+					Err:       fmt.Errorf("invalid allowed address %s", addr),
+					ClientMsg: fmt.Sprintf("invalid allowed address %s", addr),
+				}
+			}
+		}
 		hasBlockAll := slices.Contains(denyOut, sandbox_network.AllInternetTrafficCIDR)
 
 		if len(allowedDomains) > 0 && !hasBlockAll {

packages/shared/pkg/sandbox-network/firewall.go

@@ -65,6 +65,32 @@ func IsIPOrCIDR(s string) bool {
 	return err == nil
 }
 
+// IsSpecifiedIPOrCIDR checks if a string is a valid IP address or CIDR notation
+// with a specified (non-zero) IP. It rejects unspecified addresses like 0.0.0.0
+// or :: (which cause errors in nftables), but allows 0.0.0.0/0 as a special case.
+func IsSpecifiedIPOrCIDR(s string) bool {
+	if !IsIPOrCIDR(s) {
+		return false
+	}
+
+	// Allow the special all-traffic CIDR
+	if s == AllInternetTrafficCIDR {
+		return true
+	}
+
+	// Extract the IP portion
+	if ip := net.ParseIP(s); ip != nil {
+		return !ip.IsUnspecified()
+	}
+
+	ip, _, err := net.ParseCIDR(s)
+	if err != nil {
+		return false
+	}
+
+	return !ip.IsUnspecified()
+}
+
 // ParseAddressesAndDomains separates a list of strings into IP addresses/CIDRs and domain names.
 func ParseAddressesAndDomains(entries []string) (addresses []string, domains []string) {
 	for _, entry := range entries {

packages/shared/pkg/sandbox-network/firewall_test.go

@@ -6,6 +6,35 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestIsSpecifiedIPOrCIDR(t *testing.T) {
+	t.Parallel()
+	testCases := []struct {
+		name     string
+		input    string
+		expected bool
+	}{
+		{"valid_ip", "1.2.3.4", true},
+		{"valid_cidr", "10.0.0.0/8", true},
+		{"valid_host_cidr", "192.168.1.1/32", true},
+		{"all_traffic_cidr", "0.0.0.0/0", true},
+		{"unspecified_ip", "0.0.0.0", false},
+		{"unspecified_cidr_32", "0.0.0.0/32", false},
+		{"unspecified_cidr_24", "0.0.0.0/24", false},
+		{"unspecified_ipv6", "::", false},
+		{"unspecified_ipv6_128", "::/128", false},
+		{"invalid_string", "not-an-ip", false},
+		{"empty_string", "", false},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			result := IsSpecifiedIPOrCIDR(tc.input)
+			require.Equal(t, tc.expected, result)
+		})
+	}
+}
+
 func TestAddressStringToCIDR(t *testing.T) {
 	t.Parallel()
 	testCases := []struct {