fix: never create a switch port in zero port groups

applyPorts dropped any security group whose OVN port group was missing and
carried on. When that removed the last one, the ENI's logical switch port was
created belonging to no port group, so no SG ACL applied — including the
priority 900/800 default-denies, which are attached per port group — and the
port was fully unrestricted until a later pass healed it.

Fail closed instead: a port whose policy cannot be programmed in full is
refused and reported unconverged, so the drift loop retries it once applySGs
catches up. The update path is skipped too, leaving a live guest on a stale
policy rather than stripping it bare.

EnsurePort and SetPortSecurityGroups get the same guard for an empty SG list.
Every ENI record written by CreateNetworkInterface has at least one SG, but the
field is omitempty with no migration, so a legacy row would otherwise yield an
unrestricted port.

spinifex/network/reconcile/apply.go

@@ -2,6 +2,7 @@ package reconcile
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log/slog"
 	"maps"
@@ -271,6 +272,30 @@ func (r *reconciler) applySGs(ctx context.Context, intent IntentState, actual Ac
 	}
 }
 
+// desiredPortGroups maps an ENI's SGs to OVN port group names, erroring unless
+// every one of them exists. An empty result is never valid: a port in no port
+// group matches no SG ACL, including the per-group default-denies, so it would
+// come up unrestricted.
+func desiredPortGroups(spec topology.PortSpec, actual ActualState) ([]string, error) {
+	if len(spec.SGIDs) == 0 {
+		return nil, errors.New("ENI has no security groups")
+	}
+	pgNames := make([]string, 0, len(spec.SGIDs))
+	var missing []string
+	for _, sgID := range spec.SGIDs {
+		pgName := topology.SecurityGroupPortGroup(sgID)
+		if _, ok := actual.PortGroups[pgName]; !ok {
+			missing = append(missing, pgName)
+			continue
+		}
+		pgNames = append(pgNames, pgName)
+	}
+	if len(missing) > 0 {
+		return nil, fmt.Errorf("security group port groups missing in OVN: %s", strings.Join(missing, ", "))
+	}
+	return pgNames, nil
+}
+
 // applyPorts ensures each intent ENI has an LSP with PG memberships matching its
 // SGIDs. Existing ports use diff-based UpdatePortGroupMemberships to avoid gaps.
 // When pruneOrphans is true, ENI LSPs with no matching intent ENI are torn down;
@@ -279,15 +304,13 @@ func (r *reconciler) applyPorts(ctx context.Context, intent IntentState, actual
 	for portID, spec := range intent.Ports {
 		portName := topology.Port(portID)
 		switchName := topology.SubnetSwitch(spec.SubnetID)
-		desiredPGs := make([]string, 0, len(spec.SGIDs))
-		for _, sgID := range spec.SGIDs {
-			pgName := topology.SecurityGroupPortGroup(sgID)
-			if _, ok := actual.PortGroups[pgName]; !ok {
-				slog.Warn("reconcile/apply: skipping port SG membership — port group missing in OVN",
-					"port", portName, "sg", sgID, "pg", pgName)
-				continue
-			}
-			desiredPGs = append(desiredPGs, pgName)
+		desiredPGs, err := desiredPortGroups(spec, actual)
+		if err != nil {
+			// Fail closed. Every SG ACL, including the default-denies, hangs off
+			// a port group, so a port in none of them is unrestricted.
+			slog.Error("reconcile/apply: refusing port with unprogrammable policy", "port", portName, "err", err)
+			res.fail(classPort, portName, err)
+			continue
 		}
 
 		if _, err := r.ovn.GetLogicalSwitchPort(ctx, portName); err != nil {

spinifex/network/reconcile/apply_test.go

@@ -90,7 +90,7 @@ func TestReconcile_PortSuppressDHCP(t *testing.T) {
 	intent.Ports["eni-static"] = topology.PortSpec{
 		PortID: "eni-static", SubnetID: "subnet-a", VPCID: "vpc-a",
 		PrivateIP: netip.MustParseAddr("10.0.1.11"), MAC: mac,
-		SuppressDHCP: true,
+		SGIDs: []string{"sg-a"}, SuppressDHCP: true,
 	}
 
 	if err := rec.Reconcile(ctx, intent); err != nil {
@@ -486,6 +486,125 @@ func TestReconcile_PortMembershipDriftCorrected(t *testing.T) {
 	}
 }
 
+// Every SG ACL, including the priority 900/800 default-denies, hangs off a port
+// group, so a port in none of them is unrestricted. When a port's policy cannot
+// be programmed in full, applyPorts must refuse the port rather than create it.
+func TestReconcile_UnprogrammablePolicyDoesNotCreatePort(t *testing.T) {
+	ctx := context.Background()
+
+	tests := []struct {
+		name   string
+		mutate func(*IntentState)
+	}{
+		{
+			// The OVN-recreate / cold-chassis window: applySGs has not yet
+			// built the port group this pass.
+			name: "every port group missing",
+			mutate: func(in *IntentState) {
+				delete(in.SGs, "sg-a")
+			},
+		},
+		{
+			// A partial miss must not land the port in the subset that exists:
+			// the absent SG's rules would silently not apply.
+			name: "one port group missing",
+			mutate: func(in *IntentState) {
+				port := in.Ports["eni-a"]
+				port.SGIDs = append(port.SGIDs, "sg-b")
+				in.Ports["eni-a"] = port
+			},
+		},
+		{
+			// A legacy ENI record written before SG defaulting: the field is
+			// omitempty and unmarshals to nil with no migration to backfill it.
+			name: "no security groups at all",
+			mutate: func(in *IntentState) {
+				port := in.Ports["eni-a"]
+				port.SGIDs = nil
+				in.Ports["eni-a"] = port
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			rec, m := newTestReconciler(t)
+			intent := freshIntent(t)
+			tc.mutate(&intent)
+
+			err := rec.Reconcile(ctx, intent)
+			if !errors.Is(err, ErrPassIncomplete) {
+				t.Errorf("Reconcile err = %v, want ErrPassIncomplete: the port must be reported unconverged", err)
+			}
+			if _, ok := m.Ports["port-eni-a"]; ok {
+				t.Error("port created with an unprogrammable policy — it would be unrestricted")
+			}
+		})
+	}
+}
+
+// The same rule on the update path: an existing port whose desired groups have
+// gone missing keeps the memberships it has. Applying the diff would strip a
+// live guest bare, which is worse than leaving it on a stale policy.
+func TestReconcile_UnprogrammablePolicyKeepsExistingMemberships(t *testing.T) {
+	ctx := context.Background()
+	rec, m := newTestReconciler(t)
+
+	intent := freshIntent(t)
+	if err := rec.Reconcile(ctx, intent); err != nil {
+		t.Fatalf("Reconcile #1: %v", err)
+	}
+	if m.Ports["port-eni-a"] == nil {
+		t.Fatal("ENI port not created")
+	}
+
+	// A pass whose EnsureSGPortGroup failed sees the port group as absent while
+	// the LSP is still joined to it in OVN.
+	actual := newActualState()
+	var res passResult
+	rec.applyPorts(ctx, intent, actual, false, &res)
+	if len(res.failures) == 0 {
+		t.Error("applyPorts reported no failure for an unprogrammable policy")
+	}
+
+	names, err := m.ListPortGroupsForPort(ctx, "port-eni-a")
+	if err != nil {
+		t.Fatalf("ListPortGroupsForPort: %v", err)
+	}
+	if len(names) == 0 {
+		t.Error("live port stripped of every port group — it is now unrestricted")
+	}
+}
+
+// Once applySGs catches up, the next pass creates the port and joins it: the
+// refusal is a window that heals, not a permanent block.
+func TestReconcile_RefusedPortHealsOncePortGroupExists(t *testing.T) {
+	ctx := context.Background()
+	rec, m := newTestReconciler(t)
+
+	intent := freshIntent(t)
+	withoutSG := intent
+	withoutSG.SGs = map[string]policy.SGSpec{}
+	if err := rec.Reconcile(ctx, withoutSG); !errors.Is(err, ErrPassIncomplete) {
+		t.Fatalf("Reconcile #1 err = %v, want ErrPassIncomplete", err)
+	}
+	if _, ok := m.Ports["port-eni-a"]; ok {
+		t.Fatal("port created before its port group existed")
+	}
+
+	if err := rec.Reconcile(ctx, intent); err != nil {
+		t.Fatalf("Reconcile #2: %v", err)
+	}
+	storedPort := m.Ports["port-eni-a"]
+	if storedPort == nil {
+		t.Fatal("port not created once its port group existed")
+	}
+	pg := m.PortGroups[topology.SecurityGroupPortGroup("sg-a")]
+	if pg == nil || !slices.Contains(pg.Ports, storedPort.UUID) {
+		t.Error("healed port not joined to its SG port group")
+	}
+}
+
 // TestReconcile_PublicInstanceExemptFromDropGate locks the post-reboot regression: a
 // reconcile that drop-gates an IGW-attached subnet with no 0.0.0.0/0 route must also
 // install the /32 reroute above the gate for every public-IP instance in that subnet,

spinifex/network/reconcile/invariants_test.go

@@ -31,6 +31,7 @@ func TestI1_GuestLSPMustHavePortSecurity(t *testing.T) {
 		port := topology.PortSpec{
 			PortID: "eni-a", SubnetID: "subnet-a", VPCID: "vpc-a",
 			PrivateIP: netip.MustParseAddr("10.0.1.5"), MAC: mac,
+			SGIDs: []string{"sg-a"},
 		}
 		if err := mgr.EnsureVPC(ctx, vpc); err != nil {
 			t.Fatalf("EnsureVPC: %v", err)
@@ -38,6 +39,9 @@ func TestI1_GuestLSPMustHavePortSecurity(t *testing.T) {
 		if err := mgr.EnsureSubnet(ctx, sub); err != nil {
 			t.Fatalf("EnsureSubnet: %v", err)
 		}
+		if err := mgr.EnsureSGPortGroup(ctx, "sg-a"); err != nil {
+			t.Fatalf("EnsureSGPortGroup: %v", err)
+		}
 		if err := mgr.EnsurePort(ctx, port); err != nil {
 			t.Fatalf("EnsurePort: %v", err)
 		}

spinifex/network/subscribers/subscribers_test.go

@@ -655,10 +655,12 @@ func TestHandlePort_FlushesStaleMACBinding(t *testing.T) {
 
 	require.NoError(t, topo.EnsureVPC(ctx, topology.VPCSpec{VPCID: "vpc-a", CIDR: netip.MustParsePrefix("172.31.0.0/16")}))
 	require.NoError(t, topo.EnsureSubnet(ctx, topology.SubnetSpec{SubnetID: "subnet-a", VPCID: "vpc-a", CIDR: netip.MustParsePrefix("172.31.0.0/24")}))
+	require.NoError(t, topo.EnsureSGPortGroup(ctx, "sg-a"))
 
 	evt := PortEvent{
 		NetworkInterfaceId: "eni-new", SubnetId: "subnet-a", VpcId: "vpc-a",
 		PrivateIpAddress: "172.31.0.4", MacAddress: "02:00:00:00:00:aa",
+		SecurityGroupIds: []string{"sg-a"},
 	}
 
 	sub.handleCreatePort(&nats.Msg{Data: mustJSON(t, evt)})

spinifex/network/topology/manager_live.go

@@ -301,6 +301,11 @@ func (m *liveManager) EnsurePort(ctx context.Context, spec PortSpec) error {
 	if spec.PortID == "" || spec.SubnetID == "" {
 		return fmt.Errorf("EnsurePort: PortID/SubnetID required")
 	}
+	// A port in no port group matches no SG ACL, including the per-group
+	// default-denies, so it would come up unrestricted.
+	if len(spec.SGIDs) == 0 {
+		return fmt.Errorf("EnsurePort: %q has no security groups", spec.PortID)
+	}
 	portName := Port(spec.PortID)
 	switchName := SubnetSwitch(spec.SubnetID)
 
@@ -437,6 +442,11 @@ func (m *liveManager) SetPortSecurityGroups(ctx context.Context, portID string,
 	if m.ovn == nil {
 		return fmt.Errorf("OVN client not connected")
 	}
+	// Stripping the last group would leave a live port unrestricted; teardown
+	// goes through DeletePort, which clears memberships and removes the LSP.
+	if len(sgIDs) == 0 {
+		return fmt.Errorf("SetPortSecurityGroups: %q would be left in no port group", portID)
+	}
 	portName := Port(portID)
 	if _, err := m.reconcilePortSGs(ctx, portName, sgIDs); err != nil {
 		return fmt.Errorf("reconcile SGs for port %q: %w", portName, err)

spinifex/network/topology/manager_live_test.go

@@ -16,6 +16,17 @@ func newLiveManagerForTest(t *testing.T) (Manager, *mock.Client) {
 	return NewLiveManager(m), m
 }
 
+// seedSG creates the SG's port group and returns the ID as a PortSpec.SGIDs
+// value. Every ENI carries at least one SG, and EnsurePort now refuses a port
+// that would land in no port group.
+func seedSG(ctx context.Context, t *testing.T, mgr Manager, sgID string) []string {
+	t.Helper()
+	if err := mgr.EnsureSGPortGroup(ctx, sgID); err != nil {
+		t.Fatalf("EnsureSGPortGroup(%s): %v", sgID, err)
+	}
+	return []string{sgID}
+}
+
 func TestLiveManager_EnsureVPC(t *testing.T) {
 	mgr, mockClient := newLiveManagerForTest(t)
 	ctx := context.Background()
@@ -94,6 +105,7 @@ func TestLiveManager_EnsurePort(t *testing.T) {
 		VPCID:     vpc.VPCID,
 		PrivateIP: netip.MustParseAddr("10.2.1.10"),
 		MAC:       mac,
+		SGIDs:     seedSG(ctx, t, mgr, "sg-port"),
 	}
 	if err := mgr.EnsurePort(ctx, port); err != nil {
 		t.Fatalf("EnsurePort: %v", err)
@@ -130,10 +142,12 @@ func TestLiveManager_EnsurePort_SuppressDHCP(t *testing.T) {
 		t.Fatalf("EnsureSubnet: %v", err)
 	}
 
+	sgIDs := seedSG(ctx, t, mgr, "sg-nodhcp")
 	dhcpMAC, _ := net.ParseMAC("02:00:00:00:02:01")
 	dhcpPort := PortSpec{
 		PortID: "eni-dhcp", SubnetID: sub.SubnetID, VPCID: vpc.VPCID,
 		PrivateIP: netip.MustParseAddr("10.7.1.10"), MAC: dhcpMAC,
+		SGIDs: sgIDs,
 	}
 	if err := mgr.EnsurePort(ctx, dhcpPort); err != nil {
 		t.Fatalf("EnsurePort(dhcp): %v", err)
@@ -150,7 +164,7 @@ func TestLiveManager_EnsurePort_SuppressDHCP(t *testing.T) {
 	staticPort := PortSpec{
 		PortID: "eni-static", SubnetID: sub.SubnetID, VPCID: vpc.VPCID,
 		PrivateIP: netip.MustParseAddr("10.7.1.11"), MAC: staticMAC,
-		SuppressDHCP: true,
+		SGIDs: sgIDs, SuppressDHCP: true,
 	}
 	if err := mgr.EnsurePort(ctx, staticPort); err != nil {
 		t.Fatalf("EnsurePort(static): %v", err)
@@ -175,6 +189,7 @@ func TestLiveManager_DeletePort(t *testing.T) {
 	port := PortSpec{
 		PortID: "eni-DP", SubnetID: sub.SubnetID, VPCID: vpc.VPCID,
 		PrivateIP: netip.MustParseAddr("10.3.1.5"), MAC: mac,
+		SGIDs: seedSG(ctx, t, mgr, "sg-dp"),
 	}
 	if err := mgr.EnsurePort(ctx, port); err != nil {
 		t.Fatalf("EnsurePort: %v", err)
@@ -209,6 +224,7 @@ func TestLiveManager_DeletePort_AlreadyAbsentIsNoop(t *testing.T) {
 	port2 := PortSpec{
 		PortID: "eni-DP2", SubnetID: sub.SubnetID, VPCID: vpc.VPCID,
 		PrivateIP: netip.MustParseAddr("10.5.1.5"), MAC: mac,
+		SGIDs: seedSG(ctx, t, mgr, "sg-dp2"),
 	}
 	if err := mgr.EnsurePort(ctx, port2); err != nil {
 		t.Fatalf("EnsurePort: %v", err)
@@ -233,10 +249,12 @@ func TestLiveManager_DeletePort_ThenRecreateSameIP_NoDuplicateLSP(t *testing.T)
 	_ = mgr.EnsureVPC(ctx, vpc)
 	_ = mgr.EnsureSubnet(ctx, sub)
 
+	sgIDs := seedSG(ctx, t, mgr, "sg-reuse")
 	oldMAC, _ := net.ParseMAC("02:00:00:00:01:01")
 	oldPort := PortSpec{
 		PortID: "eni-old", SubnetID: sub.SubnetID, VPCID: vpc.VPCID,
 		PrivateIP: netip.MustParseAddr("172.31.0.4"), MAC: oldMAC,
+		SGIDs: sgIDs,
 	}
 	if err := mgr.EnsurePort(ctx, oldPort); err != nil {
 		t.Fatalf("EnsurePort(old): %v", err)
@@ -249,6 +267,7 @@ func TestLiveManager_DeletePort_ThenRecreateSameIP_NoDuplicateLSP(t *testing.T)
 	newPort := PortSpec{
 		PortID: "eni-new", SubnetID: sub.SubnetID, VPCID: vpc.VPCID,
 		PrivateIP: netip.MustParseAddr("172.31.0.4"), MAC: newMAC,
+		SGIDs: sgIDs,
 	}
 	if err := mgr.EnsurePort(ctx, newPort); err != nil {
 		t.Fatalf("EnsurePort(new): %v", err)
@@ -404,6 +423,49 @@ func TestLiveManager_SetPortSecurityGroups(t *testing.T) {
 	}
 }
 
+// Every SG ACL, including the priority 900/800 default-denies, is attached to a
+// port group, so a port in none of them is unrestricted. EnsurePort must refuse
+// to create one and SetPortSecurityGroups must refuse to strip a live one bare.
+func TestLiveManager_NoSecurityGroupsIsRefused(t *testing.T) {
+	mgr, mockClient := newLiveManagerForTest(t)
+	ctx := context.Background()
+	vpc := VPCSpec{VPCID: "vpc-nosg", CIDR: netip.MustParsePrefix("10.9.0.0/16")}
+	sub := SubnetSpec{SubnetID: "subnet-nosg", VPCID: vpc.VPCID, CIDR: netip.MustParsePrefix("10.9.1.0/24")}
+	_ = mgr.EnsureVPC(ctx, vpc)
+	_ = mgr.EnsureSubnet(ctx, sub)
+
+	mac, _ := net.ParseMAC("02:00:00:00:09:01")
+	bare := PortSpec{
+		PortID: "eni-bare", SubnetID: sub.SubnetID, VPCID: vpc.VPCID,
+		PrivateIP: netip.MustParseAddr("10.9.1.5"), MAC: mac,
+	}
+	if err := mgr.EnsurePort(ctx, bare); err == nil {
+		t.Error("EnsurePort with no SGs must fail: the LSP would match no SG ACL")
+	}
+	if _, err := mockClient.GetLogicalSwitchPort(ctx, Port(bare.PortID)); err == nil {
+		t.Error("refused EnsurePort must leave no LSP behind")
+	}
+
+	live := PortSpec{
+		PortID: "eni-live", SubnetID: sub.SubnetID, VPCID: vpc.VPCID,
+		PrivateIP: netip.MustParseAddr("10.9.1.6"), MAC: mac,
+		SGIDs: seedSG(ctx, t, mgr, "sg-nosg"),
+	}
+	if err := mgr.EnsurePort(ctx, live); err != nil {
+		t.Fatalf("EnsurePort: %v", err)
+	}
+	if err := mgr.SetPortSecurityGroups(ctx, live.PortID, nil); err == nil {
+		t.Error("SetPortSecurityGroups with no SGs must fail: it would strip the port bare")
+	}
+	names, err := mockClient.ListPortGroupsForPort(ctx, Port(live.PortID))
+	if err != nil {
+		t.Fatalf("ListPortGroupsForPort: %v", err)
+	}
+	if len(names) != 1 || names[0] != SecurityGroupPortGroup("sg-nosg") {
+		t.Errorf("live port must keep its membership, got %v", names)
+	}
+}
+
 func TestLiveManager_WithDNSServer(t *testing.T) {
 	mockClient := mock.New()
 	_ = mockClient.Connect(context.Background())