From f62c54a4f2b53e048fa537262572e9b02f3b0828 Mon Sep 17 00:00:00 2001
From: JackDoan <me@jackdoan.com>
Date: Thu, 1 Oct 2026 10:21:53 -0500
Subject: [PATCH] Fix firewall config parsing panics on empty group, null
 groups, and non-string groups

---
 CHANGELOG.md     |  3 +++
 firewall.go      |  9 +++++++--
 firewall_test.go | 20 ++++++++++++++++++++
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4f27bc82..3539a3ab 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   down with it. A hostinfo returned by StartHandshake had no RemoteList until the handshake loop's first
   pass attached one, and `create-tunnel -address` set the remote before that. The list is now attached when
   the handshake starts, so a pending hostinfo never exists without one.
+- A firewall rule with `group: []`, `groups:` with no value, or a `groups` entry that isn't a string, such as
+  `groups: [1, 2]`, no longer crashes Nebula on startup or reload. An empty `group` array is rejected, a null
+  `groups` means no groups, and non-string entries are read as strings.
 
 ## [1.11.1] - 2026-08-21
 
diff --git a/firewall.go b/firewall.go
index 5fb2d6de..b1950123 100644
--- a/firewall.go
+++ b/firewall.go
@@ -972,6 +972,9 @@ func convertRule(l *slog.Logger, p any, table string, i int) (rule, error) {
 		if len(v) > 1 {
 			return r, errors.New("group should contain a single value, an array with more than one entry was provided")
 		}
+		if len(v) == 0 {
+			return r, errors.New("group should contain a single value, an empty array was provided")
+		}
 
 		l.Warn("group was an array with a single value, converting to simple value",
 			"table", table,
@@ -982,13 +985,15 @@ func convertRule(l *slog.Logger, p any, table string, i int) (rule, error) {
 
 	singleGroup := toString("group", m)
 
-	if rg, ok := m["groups"]; ok {
+	// A null value, `groups:` with nothing after it, is the same as no groups
+	if rg, ok := m["groups"]; ok && rg != nil {
 		switch reflect.TypeOf(rg).Kind() {
 		case reflect.Slice:
+			// Every entry is read as a string, as the other fields are, so `groups: [1, 2]` is the groups "1" and "2"
 			v := reflect.ValueOf(rg)
 			r.Groups = make([]string, v.Len())
 			for i := 0; i < v.Len(); i++ {
-				r.Groups[i] = v.Index(i).Interface().(string)
+				r.Groups[i] = fmt.Sprintf("%v", v.Index(i).Interface())
 			}
 		case reflect.String:
 			r.Groups = []string{rg.(string)}
diff --git a/firewall_test.go b/firewall_test.go
index b8727915..5915e6e5 100644
--- a/firewall_test.go
+++ b/firewall_test.go
@@ -1439,6 +1439,26 @@ func TestAddFirewallRulesFromConfig(t *testing.T) {
 	require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf))
 	assert.Equal(t, addRuleCall{incoming: true, proto: firewall.ProtoAny, startPort: 1, endPort: 1, groups: []string{"a", "b"}, ip: "", localIp: ""}, mf.lastCall)
 
+	// Test group and groups values that used to crash: an empty group array is rejected, groups entries that
+	// aren't strings are read as strings, and a null groups is no groups
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "group": []any{}}}}
+	require.EqualError(t, AddFirewallRulesFromConfig(l, true, conf, mf), "firewall.inbound rule #0; group should contain a single value, an empty array was provided")
+
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "groups": []any{1, 2}}}}
+	require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf))
+	assert.Equal(t, []string{"1", "2"}, mf.lastCall.groups)
+
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "groups": nil, "host": "a"}}}
+	require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf))
+	assert.Empty(t, mf.lastCall.groups)
+	assert.Equal(t, "a", mf.lastCall.host)
+
 	// Test Add error
 	conf = config.NewC(test.NewLogger())
 	mf = &mockFirewall{}