Resolve TODOs and issues with allow_list

slackhq · Jul 5, 2024 · 9bc9d25 · 9bc9d25
1 parent 58e330f
commit 9bc9d25
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 10 deletions.
diff --git a/allow_list.go b/allow_list.go
@@ -122,18 +122,19 @@ func newAllowList(k string, raw interface{}, handleKey func(key string, value in
 		}
 
 		ipNet, err := netip.ParsePrefix(rawCIDR)
-		//TODO: better include the error
 		if err != nil {
-			return nil, fmt.Errorf("config `%s` has invalid CIDR: %s", k, rawCIDR)
+			return nil, fmt.Errorf("config `%s` has invalid CIDR: %s. %w", k, rawCIDR, err)
 		}
 
+		ipNet = netip.PrefixFrom(ipNet.Addr().Unmap(), ipNet.Bits())
+
 		// TODO: should we error on duplicate CIDRs in the config?
 		tree.Insert(ipNet, value)
 
 		maskBits := ipNet.Bits()
 
 		var rules *allowListRules
-		if ipNet.Masked().Addr().Is4() {
+		if ipNet.Addr().Is4() {
 			rules = &rules4
 		} else {
 			rules = &rules6
@@ -156,17 +157,15 @@ func newAllowList(k string, raw interface{}, handleKey func(key string, value in
 
 	if !rules4.defaultSet {
 		if rules4.allValuesMatch {
-			//TODO ensure this is a 0/0
-			tree.Insert(netip.Prefix{}, !rules4.allValues)
+			tree.Insert(netip.PrefixFrom(netip.IPv4Unspecified(), 0), !rules4.allValues)
 		} else {
 			return nil, fmt.Errorf("config `%s` contains both true and false rules, but no default set for 0.0.0.0/0", k)
 		}
 	}
 
 	if !rules6.defaultSet {
 		if rules6.allValuesMatch {
-			//TODO: ensure this is a ::/0
-			tree.Insert(netip.Prefix{}, !rules6.allValues)
+			tree.Insert(netip.PrefixFrom(netip.IPv6Unspecified(), 0), !rules6.allValues)
 		} else {
 			return nil, fmt.Errorf("config `%s` contains both true and false rules, but no default set for ::/0", k)
 		}
@@ -242,9 +241,8 @@ func getRemoteAllowRanges(c *config.C, k string) (*bart.Table[*AllowList], error
 		}
 
 		ipNet, err := netip.ParsePrefix(rawCIDR)
-		//TODO: better to include err
 		if err != nil {
-			return nil, fmt.Errorf("config `%s` has invalid CIDR: %s", k, rawCIDR)
+			return nil, fmt.Errorf("config `%s` has invalid CIDR: %s. %w", k, rawCIDR, err)
 		}
 
 		remoteAllowRanges.Insert(ipNet, allowList)

diff --git a/allow_list_test.go b/allow_list_test.go
@@ -18,7 +18,7 @@ func TestNewAllowListFromConfig(t *testing.T) {
 		"192.168.0.0": true,
 	}
 	r, err := newAllowListFromConfig(c, "allowlist", nil)
-	assert.EqualError(t, err, "config `allowlist` has invalid CIDR: 192.168.0.0")
+	assert.EqualError(t, err, "config `allowlist` has invalid CIDR: 192.168.0.0. netip.ParsePrefix(\"192.168.0.0\"): no '/'")
 	assert.Nil(t, r)
 
 	c.Settings["allowlist"] = map[interface{}]interface{}{