fix: cross-check live BlockDeviceMappings before DeleteVolume (#440)

The daemon-side in-use gate trusts persisted volume State/AttachedInstance,
which the attach/detach lifecycle can leave desynced: a running root volume
was observed with State:"" and AttachedInstance:"" while the instance's
BlockDeviceMappings still listed it, so DeleteVolume passed the gate and
removed the durable backing under a live QEMU.

Add an authoritative guard at the gateway DeleteVolume path: fan out the
strict DescribeInstances survey and refuse with VolumeInUse when any
non-terminated instance's BlockDeviceMappings reference the volume. Fail
closed on an incomplete cluster view rather than delete against a partial
survey. Terminate-time cleanup uses the daemon volume service directly and is
unaffected.

spinifex/gateway/ec2.go

@@ -244,7 +244,7 @@ var ec2Actions = map[string]EC2Handler{
 		return gateway_ec2_volume.CreateVolume(input, gw.NATSConn, accountID)
 	}),
 	"DeleteVolume": ec2Handler(func(input *ec2.DeleteVolumeInput, gw *GatewayConfig, accountID string) (any, error) {
-		return gateway_ec2_volume.DeleteVolume(input, gw.NATSConn, accountID)
+		return gateway_ec2_volume.DeleteVolume(input, gw.NATSConn, gw.DiscoverActiveNodes(), accountID)
 	}),
 	"AttachVolume": ec2Handler(func(input *ec2.AttachVolumeInput, gw *GatewayConfig, accountID string) (any, error) {
 		return gateway_ec2_volume.AttachVolume(input, gw.NATSConn, accountID)

spinifex/gateway/ec2/volume/DeleteVolume.go

@@ -2,10 +2,13 @@ package gateway_ec2_volume
 
 import (
 	"errors"
+	"log/slog"
 	"strings"
 
+	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/mulgadc/spinifex/spinifex/awserrors"
+	gateway_ec2_instance "github.com/mulgadc/spinifex/spinifex/gateway/ec2/instance"
 	handlers_ec2_volume "github.com/mulgadc/spinifex/spinifex/handlers/ec2/volume"
 	"github.com/nats-io/nats.go"
 )
@@ -22,8 +25,41 @@ func ValidateDeleteVolumeInput(input *ec2.DeleteVolumeInput) error {
 	return nil
 }
 
-// DeleteVolume handles the DeleteVolume API call
-func DeleteVolume(input *ec2.DeleteVolumeInput, natsConn *nats.Conn, accountID string) (ec2.DeleteVolumeOutput, error) {
+// volumeHeldByInstance returns the id of a non-terminated instance whose
+// BlockDeviceMappings reference volumeID, or "" if none does. A terminated
+// instance's mappings are stale (its volumes are released), so they do not
+// block a delete. This is the authoritative in-use signal: persisted volume
+// State/AttachedInstance can drift out of sync with the real attachment.
+func volumeHeldByInstance(reservations []*ec2.Reservation, volumeID string) string {
+	for _, res := range reservations {
+		if res == nil {
+			continue
+		}
+		for _, inst := range res.Instances {
+			if inst == nil {
+				continue
+			}
+			if inst.State != nil && aws.StringValue(inst.State.Name) == ec2.InstanceStateNameTerminated {
+				continue
+			}
+			for _, bdm := range inst.BlockDeviceMappings {
+				if bdm != nil && bdm.Ebs != nil && aws.StringValue(bdm.Ebs.VolumeId) == volumeID {
+					return aws.StringValue(inst.InstanceId)
+				}
+			}
+		}
+	}
+	return ""
+}
+
+// DeleteVolume handles the DeleteVolume API call.
+//
+// Before destroying the backing it cross-checks live cluster-wide instance
+// BlockDeviceMappings: the daemon-side gate trusts persisted volume metadata,
+// which the attach/detach lifecycle can leave desynced (a running root with
+// State:"" / AttachedInstance:""), so a drifted-but-live volume could be
+// deleted under a running QEMU. The check fails closed on an incomplete view.
+func DeleteVolume(input *ec2.DeleteVolumeInput, natsConn *nats.Conn, expectedNodes int, accountID string) (ec2.DeleteVolumeOutput, error) {
 	var output ec2.DeleteVolumeOutput
 
 	err := ValidateDeleteVolumeInput(input)
@@ -31,6 +67,28 @@ func DeleteVolume(input *ec2.DeleteVolumeInput, natsConn *nats.Conn, accountID s
 		return output, err
 	}
 
+	volumeID := aws.StringValue(input.VolumeId)
+
+	// Strict fan-out: complete is true only when every active node answered and
+	// both instance buckets queried, so a node that owns the attachment cannot be
+	// silently missing from the survey. Refuse rather than delete against a
+	// partial view of the cluster.
+	reservations, complete, err := gateway_ec2_instance.DescribeInstancesForReconcile(
+		&ec2.DescribeInstancesInput{}, natsConn, expectedNodes, accountID)
+	if err != nil {
+		slog.Error("DeleteVolume: in-use precheck fan-out failed", "volumeId", volumeID, "err", err)
+		return output, errors.New(awserrors.ErrorServerInternal)
+	}
+	if !complete {
+		slog.Error("DeleteVolume: refusing delete on incomplete instance view", "volumeId", volumeID)
+		return output, errors.New(awserrors.ErrorServerInternal)
+	}
+	if holder := volumeHeldByInstance(reservations, volumeID); holder != "" {
+		slog.Error("DeleteVolume: volume still attached to a live instance",
+			"volumeId", volumeID, "instanceId", holder)
+		return output, errors.New(awserrors.ErrorVolumeInUse)
+	}
+
 	volumeService := handlers_ec2_volume.NewNATSVolumeService(natsConn)
 	result, err := volumeService.DeleteVolume(input, accountID)
 

spinifex/gateway/ec2/volume/DeleteVolume_test.go

@@ -97,3 +97,80 @@ func TestValidateDeleteVolumeInput(t *testing.T) {
 		})
 	}
 }
+
+func instanceWithVolume(instanceID, state, volumeID string) *ec2.Instance {
+	return &ec2.Instance{
+		InstanceId: aws.String(instanceID),
+		State:      &ec2.InstanceState{Name: aws.String(state)},
+		BlockDeviceMappings: []*ec2.InstanceBlockDeviceMapping{
+			{DeviceName: aws.String("/dev/vda"), Ebs: &ec2.EbsInstanceBlockDevice{VolumeId: aws.String(volumeID)}},
+		},
+	}
+}
+
+func reservations(instances ...*ec2.Instance) []*ec2.Reservation {
+	return []*ec2.Reservation{{Instances: instances}}
+}
+
+func TestVolumeHeldByInstance(t *testing.T) {
+	const vol = "vol-target"
+
+	tests := []struct {
+		name         string
+		reservations []*ec2.Reservation
+		want         string
+	}{
+		{
+			name:         "RunningInstanceHoldsVolume",
+			reservations: reservations(instanceWithVolume("i-running", ec2.InstanceStateNameRunning, vol)),
+			want:         "i-running",
+		},
+		{
+			name:         "StoppedInstanceHoldsVolume",
+			reservations: reservations(instanceWithVolume("i-stopped", ec2.InstanceStateNameStopped, vol)),
+			want:         "i-stopped",
+		},
+		{
+			name:         "TerminatedInstanceIgnored",
+			reservations: reservations(instanceWithVolume("i-dead", ec2.InstanceStateNameTerminated, vol)),
+			want:         "",
+		},
+		{
+			name:         "NoInstanceReferencesVolume",
+			reservations: reservations(instanceWithVolume("i-other", ec2.InstanceStateNameRunning, "vol-different")),
+			want:         "",
+		},
+		{
+			name: "RunningWinsOverTerminatedDuplicate",
+			reservations: reservations(
+				instanceWithVolume("i-dead", ec2.InstanceStateNameTerminated, vol),
+				instanceWithVolume("i-live", ec2.InstanceStateNameRunning, vol),
+			),
+			want: "i-live",
+		},
+		{
+			name:         "EmptyReservations",
+			reservations: nil,
+			want:         "",
+		},
+		{
+			name: "NilSafety",
+			reservations: []*ec2.Reservation{
+				nil,
+				{Instances: []*ec2.Instance{nil, {InstanceId: aws.String("i-nobdm")}}},
+				{Instances: []*ec2.Instance{{
+					InstanceId:          aws.String("i-nilebs"),
+					State:               &ec2.InstanceState{Name: aws.String(ec2.InstanceStateNameRunning)},
+					BlockDeviceMappings: []*ec2.InstanceBlockDeviceMapping{nil, {Ebs: nil}},
+				}}},
+			},
+			want: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, volumeHeldByInstance(tt.reservations, vol))
+		})
+	}
+}

spinifex/gateway/ec2/volume/handler_test.go

@@ -28,17 +28,17 @@ func TestCreateVolume_NilNATS(t *testing.T) {
 }
 
 func TestDeleteVolume_ValidationErrors(t *testing.T) {
-	_, err := DeleteVolume(nil, nil, "")
+	_, err := DeleteVolume(nil, nil, 1, "")
 	assert.EqualError(t, err, awserrors.ErrorInvalidParameterValue)
 
-	_, err = DeleteVolume(&ec2.DeleteVolumeInput{VolumeId: aws.String("bad")}, nil, "")
+	_, err = DeleteVolume(&ec2.DeleteVolumeInput{VolumeId: aws.String("bad")}, nil, 1, "")
 	assert.EqualError(t, err, awserrors.ErrorInvalidVolumeIDMalformed)
 }
 
 func TestDeleteVolume_NilNATS(t *testing.T) {
 	_, err := DeleteVolume(&ec2.DeleteVolumeInput{
 		VolumeId: aws.String("vol-1234567890abcdef0"),
-	}, nil, "acct-123")
+	}, nil, 1, "acct-123")
 	assert.Error(t, err)
 }