fix: close the storage-exhaustion recovery gaps (durable terminate, per-service memory caps, local volume leak) (#653)

* fix(vm): durably record terminate even when local state persistence fails

finalizeTerminated bailed out of WriteTerminatedInstance whenever the
transition to terminated hit a persistence-only failure (e.g. ENOSPC writing
local state), even though the in-memory status had already reached
terminated. That left the instance owned by nothing: StuckTerminateReaper
skips it (no longer shutting-down) and TerminatedTeardownReaper never sees
it (never reached the terminated KV bucket). Only an operator daemon restart
recovered it.

WriteTerminatedInstance is a JetStream KV write, independent of local disk
space, so keep going into it whenever the in-memory status did reach
terminated, distinguishing that case from a genuine invalid/raced
transition. Once the disk has space again, teardown now completes on its
own via the existing idempotent TerminatedTeardownReaper.

* fix(systemd): cap per-service memory under spinifex-system.slice

Every service under spinifex.slice ran with MemoryMax=infinity /
MemoryHigh=infinity, so one runaway service could exhaust the whole slice
budget and the slice-level OOM would then reap an arbitrary victim instead
of the offender.

Add per-unit MemoryHigh/MemoryMax as percentages of physical RAM (so the
same units scale from a dev box to bare metal). spinifex-daemon.service
still hosts every guest QEMU process in its own cgroup (no per-instance
slice yet), so it gets by far the largest allowance, just under the slice's
own 80%/90%, to avoid starving VM launches. predastore is the legitimately
memory-hungry service and gets generous headroom above its idle footprint.
The remaining infra services are small, single-purpose Go processes and get
a shared conservative bound.

* fix(viperblockd): remove local volume directory on ebs.delete

DeleteVolume deleted the S3 data and notified viperblockd, but the ebs.delete
handler only stopped the volume's goroutines, killed nbdkit, and removed the
NBD socket — it never removed the local WAL/checkpoint cache directory under
BaseDir. The only path that removed it (VB.Close's RemoveLocalFiles) only
ran from the unmount handler's seal step, which -efi volumes never reach
(they carry no durable data to seal) and which a main volume also skips
whenever a prior seal failed (e.g. disk full, deliberately keeping local
files for recovery). Since terminate always unmounts before deleting, and
delete is permanent, retention had no remaining purpose by the time
ebs.delete ran.

Unconditionally remove BaseDir/<volume> in the ebs.delete handler regardless
of mount-tracking state, closing the leak for both the root volume and its
-efi companion.

* fix(viperblockd): validate volume names before removing local state

ebs.delete unmarshalled the volume name straight off the wire and
handed it to os.RemoveAll: an empty name collapsed to BaseDir itself,
and ".." components could escape it entirely. localVolumeDir now
rejects an empty/invalid name or one that resolves outside BaseDir
before any removal, and ebs.delete returns Success: false with an
error instead of silently skipping. Applied the same gate to
ebs.mount and ebs.config, the other two handlers that turn a
wire-supplied volume name into a BaseDir-relative path.

* chore: pin viperblock to the merged concurrent-open corruption fix

The ansible build role builds with GOWORK=off, so the deployed binary links
whatever go.mod pins rather than the go.work overlay. Move the pin to the
revision carrying the per-volume flock and the WAL/state write fixes.

build/systemd/spinifex-awsgw.service

@@ -19,6 +19,10 @@ RestartSec=5
 TimeoutStopSec=10
 
 OOMScoreAdjust=-500
+# Per-service ceiling bounds a leak in this service alone rather than
+# tripping the slice-wide cap, which would pick an arbitrary victim.
+MemoryHigh=10%
+MemoryMax=20%
 Environment=SPINIFEX_CONFIG_PATH=/etc/spinifex/spinifex.toml
 Environment=SPINIFEX_BASE_DIR=/var/lib/spinifex/awsgw/
 Environment=SPINIFEX_AWSGW_TLS_CERT=/etc/spinifex/server.pem

build/systemd/spinifex-daemon.service

@@ -19,6 +19,10 @@ KillMode=process
 TimeoutStopSec=15
 # RG-4: infra tier — reaped only after every guest
 OOMScoreAdjust=-500
+# Guest QEMU still runs inside this cgroup, so the ceiling tracks the slice
+# budget (just under 80%/90%), not just daemon overhead.
+MemoryHigh=75%
+MemoryMax=85%
 Environment=SPINIFEX_CONFIG_PATH=/etc/spinifex/spinifex.toml
 Environment=SPINIFEX_BASE_DIR=/var/lib/spinifex/spinifex/
 Environment=SPINIFEX_WAL_DIR=/var/lib/spinifex/spinifex/

build/systemd/spinifex-nats.service

@@ -17,6 +17,10 @@ RestartSec=5
 # stops before nats does, so size this for nats's own graceful-stop needs.
 TimeoutStopSec=20
 OOMScoreAdjust=-500
+# Per-service ceiling bounds a leak in this service alone rather than
+# tripping the slice-wide cap, which would pick an arbitrary victim.
+MemoryHigh=10%
+MemoryMax=20%
 Environment=SPINIFEX_CONFIG_PATH=/etc/spinifex/nats/nats.conf
 
 # Filesystem access

build/systemd/spinifex-northstar.service

@@ -17,6 +17,10 @@ RestartSec=5
 # NATS unsubscribe/close and process exit that follow it.
 TimeoutStopSec=10
 OOMScoreAdjust=-500
+# Per-service ceiling bounds a leak in this service alone rather than
+# tripping the slice-wide cap, which would pick an arbitrary victim.
+MemoryHigh=10%
+MemoryMax=20%
 Environment=SPINIFEX_CONFIG_PATH=/etc/spinifex/spinifex.toml
 Environment=SPINIFEX_BASE_DIR=/var/lib/spinifex/northstar/
 

build/systemd/spinifex-predastore.service

@@ -22,6 +22,11 @@ RestartSec=5
 # backstop if that ordering is ever bypassed (direct stop, future edit).
 TimeoutStopSec=30
 OOMScoreAdjust=-500
+# Legitimately the most memory-hungry infra service (S3 buffering/caching);
+# High/Max stay well above idle so normal load isn't throttled, while still
+# bounding a leak short of the slice-wide cap.
+MemoryHigh=40%
+MemoryMax=55%
 Environment=SPINIFEX_PREDASTORE_CONFIG_PATH=/etc/spinifex/predastore/predastore.toml
 Environment=SPINIFEX_PREDASTORE_BASE_PATH=/var/lib/spinifex/predastore/
 Environment=SPINIFEX_PREDASTORE_TLS_CERT=/etc/spinifex/server.pem

build/systemd/spinifex-qmp-collector.service

@@ -19,6 +19,10 @@ RestartSec=5
 # Poll loop selects on ctx.Done() directly and closes NATS locally; shutdown
 # is near-instant, so this only bounds an unexpectedly wedged process.
 TimeoutStopSec=10
+# Per-service ceiling bounds a leak in this service alone rather than
+# tripping the slice-wide cap, which would pick an arbitrary victim.
+MemoryHigh=10%
+MemoryMax=20%
 Environment=SPINIFEX_CONFIG_PATH=/etc/spinifex/spinifex.toml
 Environment=SPINIFEX_BASE_DIR=/var/lib/spinifex/spinifex/
 # Same runtime dir as the daemon: telemetry sockets + metadata live there

build/systemd/spinifex-ui.service

@@ -16,6 +16,10 @@ RestartSec=5
 # long streaming proxy responses, e.g. console passthrough); add margin above it.
 TimeoutStopSec=40
 OOMScoreAdjust=-500
+# Per-service ceiling bounds a leak in this service alone rather than
+# tripping the slice-wide cap, which would pick an arbitrary victim.
+MemoryHigh=10%
+MemoryMax=20%
 Environment=SPINIFEX_UI_TLS_CERT=/etc/spinifex/server.pem
 Environment=SPINIFEX_UI_TLS_KEY=/etc/spinifex/server.key
 Environment=SPINIFEX_UI_BASE_DIR=/var/lib/spinifex/spinifex-ui/

build/systemd/spinifex-viperblock.service

@@ -15,6 +15,10 @@ ExecStart=/usr/local/bin/spx service viperblock start
 Restart=on-failure
 RestartSec=5
 OOMScoreAdjust=-500
+# Per-service ceiling bounds a leak in this service alone rather than
+# tripping the slice-wide cap, which would pick an arbitrary victim.
+MemoryHigh=15%
+MemoryMax=25%
 # KillMode=process: nbdkit still serving a guest survives a restart here, the
 # way guest QEMU survives a daemon restart. Tearing one out mid-write corrupts
 # that guest's filesystem; idle nbdkit is reaped explicitly on shutdown.

build/systemd/spinifex-vpcd.service

@@ -18,6 +18,9 @@ RestartSec=5
 # no sub-timeout bounds that call today, so this covers a normal pass.
 TimeoutStopSec=20
 OOMScoreAdjust=-500
+# Per-service ceiling: bounds a leak here instead of the slice-wide cap.
+MemoryHigh=10%
+MemoryMax=20%
 Environment=SPINIFEX_CONFIG_PATH=/etc/spinifex/spinifex.toml
 Environment=SPINIFEX_BASE_DIR=/var/lib/spinifex/vpcd/
 

go.mod

@@ -22,7 +22,7 @@ require (
 	github.com/miekg/dns v1.1.72
 	github.com/mulgadc/northstar v1.14.0
 	github.com/mulgadc/predastore v1.14.1-0.20260730221229-608939ef2bb4
-	github.com/mulgadc/viperblock v1.14.1-0.20260801223811-8a4787ade83a
+	github.com/mulgadc/viperblock v1.14.1-0.20260802084514-ed52f6a0e4c0
 	github.com/nats-io/nats-server/v2 v2.14.3
 	github.com/nats-io/nats.go v1.52.0
 	github.com/opencontainers/runtime-spec v1.3.0

go.sum

@@ -1321,8 +1321,8 @@ github.com/mulgadc/northstar v1.14.0 h1:8JzDcS4k3SN5PF3jBdB8x9FOWC24eITCClmOdwpo
 github.com/mulgadc/northstar v1.14.0/go.mod h1:6EArbWIQUrCFQ+XZ5ZJrGS/ep/u/rF6l+dKZh5vVIRs=
 github.com/mulgadc/predastore v1.14.1-0.20260730221229-608939ef2bb4 h1:Q4/NEryYTMiALkFwbPPOrKpNgLir+d7T2hbyRpAFnWc=
 github.com/mulgadc/predastore v1.14.1-0.20260730221229-608939ef2bb4/go.mod h1:na0aNXABZF03/HfFWCrAPHrId/2FIEIUQfB82vlu1xg=
-github.com/mulgadc/viperblock v1.14.1-0.20260801223811-8a4787ade83a h1:7HZ8K0hpA0tuExkWgcNmUuK1ZH7ZvjNpVsFvKLvOMrE=
-github.com/mulgadc/viperblock v1.14.1-0.20260801223811-8a4787ade83a/go.mod h1:5ENQb0OrjTplIRiLShRW/oNWOzbFvSCFgo+PkNH4858=
+github.com/mulgadc/viperblock v1.14.1-0.20260802084514-ed52f6a0e4c0 h1:etjtOa6Sq21CtX+8RU4vYb5JBz+dExPLLym6xpXD4Mw=
+github.com/mulgadc/viperblock v1.14.1-0.20260802084514-ed52f6a0e4c0/go.mod h1:5ENQb0OrjTplIRiLShRW/oNWOzbFvSCFgo+PkNH4858=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
 github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=

spinifex/services/viperblockd/viperblockd.go

@@ -253,12 +253,51 @@ func isAuxVolume(volumeName string) bool {
 	return strings.HasSuffix(volumeName, "-efi")
 }
 
+// validVolumeName reports whether volume is safe to use as a single path
+// component under BaseDir: non-empty, no separator, and not "." or "..".
+func validVolumeName(volume string) bool {
+	return volume != "" && volume != "." && volume != ".." && filepath.Base(volume) == volume
+}
+
+// localVolumeDir validates volume and baseDir, then returns baseDir/volume.
+// Every handler that turns a wire-supplied volume name into a filesystem path
+// (mount, unmount, delete, config) must go through this: the name arrives
+// unmarshalled straight from a NATS message, so an empty or ".."-laden value
+// must never reach a filesystem call. The character checks in validVolumeName
+// already rule out an escape, but the path is still resolved and checked
+// against baseDir as a second, independent guard.
+func localVolumeDir(baseDir, volume string) (string, error) {
+	if baseDir == "" {
+		return "", fmt.Errorf("empty base directory")
+	}
+	if !validVolumeName(volume) {
+		return "", fmt.Errorf("invalid volume name %q", volume)
+	}
+
+	absBase, err := filepath.Abs(baseDir)
+	if err != nil {
+		return "", fmt.Errorf("resolve base directory: %w", err)
+	}
+	dir := filepath.Join(absBase, volume)
+
+	rel, err := filepath.Rel(absBase, dir)
+	if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
+		return "", fmt.Errorf("volume name %q escapes base directory", volume)
+	}
+	return dir, nil
+}
+
 // volumeNeedsSeal reports whether an unmounted volume has local viperblock
 // state under baseDir/<volume> to flush. A node that never held the local
 // WAL has nothing to seal. Callers handle auxiliary volumes separately (see
 // isAuxVolume).
 func volumeNeedsSeal(volumeName, baseDir string) bool {
-	_, err := os.Stat(filepath.Join(baseDir, volumeName))
+	dir, err := localVolumeDir(baseDir, volumeName)
+	if err != nil {
+		slog.Warn("volumeNeedsSeal: rejecting invalid volume name", "volume", volumeName, "err", err)
+		return false
+	}
+	_, err = os.Stat(dir)
 	return err == nil
 }
 
@@ -469,6 +508,17 @@ func launchService(cfg *Config) (err error) {
 			return
 		}
 
+		// Reject before touching anything: an empty or path-traversing name
+		// must never reach the RemoveAll below, which is otherwise wide
+		// enough to wipe BaseDir itself or escape it entirely.
+		localPath, err := localVolumeDir(cfg.BaseDir, ebsRequest.Volume)
+		if err != nil {
+			slog.ErrorContext(ctx, "ebs.delete: refusing invalid volume name", "volume", ebsRequest.Volume, "err", err)
+			utils.MarkSpanError(span, err)
+			respondJSON(msg, types.EBSDeleteResponse{Volume: ebsRequest.Volume, Error: fmt.Sprintf("invalid volume name: %v", err)})
+			return
+		}
+
 		response := types.EBSDeleteResponse{Volume: ebsRequest.Volume, Success: true}
 
 		// Find and clean up the mounted volume if it exists
@@ -515,6 +565,20 @@ func launchService(cfg *Config) (err error) {
 			slog.InfoContext(ctx, "ebs.delete: volume not mounted (expected for available volumes)", "volume", ebsRequest.Volume)
 		}
 
+		// Delete is permanent: remove the on-disk WAL/checkpoint cache
+		// regardless of mount-tracking state. -efi volumes never go through
+		// the unmount seal (isAuxVolume skips it, they carry no durable
+		// data), and a main volume's seal can have been skipped after a
+		// failed flush (e.g. disk full), so this is the only guaranteed
+		// cleanup point for both. localPath was validated above.
+		if err := os.RemoveAll(localPath); err != nil {
+			slog.ErrorContext(ctx, "ebs.delete: failed to remove local volume directory",
+				"volume", ebsRequest.Volume, "path", localPath, "err", err)
+		} else {
+			slog.InfoContext(ctx, "ebs.delete: removed local volume directory",
+				"volume", ebsRequest.Volume, "path", localPath)
+		}
+
 		respondJSON(msg, response)
 	}); err != nil {
 		return fmt.Errorf("failed to subscribe to ebs.delete: %w", err)
@@ -718,6 +782,16 @@ func launchService(cfg *Config) (err error) {
 			return
 		}
 
+		// Reject before the not-live branch below can hand this name to
+		// openLoadedVolumeVB, which derives a BaseDir/<volume> path.
+		if !validVolumeName(req.Volume) {
+			err := fmt.Errorf("invalid volume name %q", req.Volume)
+			slog.ErrorContext(ctx, "ebs.config: refusing invalid volume name", "volume", req.Volume)
+			utils.MarkSpanError(span, err)
+			respondJSON(msg, types.EBSConfigUpdateResponse{Volume: req.Volume, Error: err.Error()})
+			return
+		}
+
 		cfg.mu.Lock()
 		var live *viperblock.VB
 		for _, volume := range cfg.MountedVolumes {
@@ -793,6 +867,18 @@ func launchService(cfg *Config) (err error) {
 
 		slog.InfoContext(ctx, "ebs.mount", "request", ebsRequest)
 
+		// Reject before any local path is derived from the name: this is the
+		// first point an unvalidated wire name would otherwise reach the
+		// filesystem (clearStaleSealReceipt) and, further down, viperblock's
+		// own BaseDir/<volume> layout.
+		if !validVolumeName(ebsRequest.Name) {
+			err := fmt.Errorf("invalid volume name %q", ebsRequest.Name)
+			slog.ErrorContext(ctx, "ebs.mount: refusing invalid volume name", "volume", ebsRequest.Name)
+			utils.MarkSpanError(span, err)
+			respondJSON(msg, types.EBSMountResponse{Error: err.Error()})
+			return
+		}
+
 		// Clear any receipt left by a previous mount before anything else can
 		// return early, so a stale receipt can never survive into this mount.
 		clearStaleSealReceipt(cfg.BaseDir, ebsRequest.Name)

spinifex/services/viperblockd/viperblockd_handlers_test.go

@@ -125,6 +125,166 @@ func TestIntegration_EBSDeleteUnmountedVolume(t *testing.T) {
 	assert.True(t, resp.Success)
 }
 
+// TestIntegration_EBSDeleteRemovesLocalVolumeDirectory covers the residual
+// leak: terminateCleanup always unmounts before DeleteVolumes sends
+// ebs.delete, so the common case is an already-unmounted volume. Before the
+// fix, ebs.delete only cleaned up a still-mounted volume's nbdkit
+// process/socket and never touched cfg.BaseDir/<volume>, so the
+// WAL/checkpoint directory a prior mount (or a failed unmount seal) left
+// behind survived every DeleteVolume call — including for -efi companion
+// volumes, which never go through the unmount seal at all (isAuxVolume skips
+// it), so they leaked unconditionally rather than only on a failed seal.
+func TestIntegration_EBSDeleteRemovesLocalVolumeDirectory(t *testing.T) {
+	t.Parallel()
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+
+	ns, natsURL := setupEmbeddedNATS(t)
+	defer ns.Shutdown()
+
+	cfg := setupTestConfig(t, natsURL)
+	// No mounted volumes: matches the terminate path where Unmount already
+	// ran and left local state behind (a failed seal for the main volume, or
+	// unconditionally for the -efi companion) before ebs.delete fires.
+	createMockVolumeState(t, cfg.BaseDir, "vol-residual-test")
+	createMockVolumeState(t, cfg.BaseDir, "vol-residual-test-efi")
+
+	go func() { launchService(cfg) }()
+	time.Sleep(500 * time.Millisecond)
+
+	nc, err := nats.Connect(natsURL)
+	require.NoError(t, err)
+	defer nc.Close()
+
+	for _, volume := range []string{"vol-residual-test", "vol-residual-test-efi"} {
+		reqData, _ := json.Marshal(types.EBSDeleteRequest{Volume: volume})
+		msg, err := nc.Request("ebs.delete", reqData, 3*time.Second)
+		require.NoError(t, err)
+
+		var resp types.EBSDeleteResponse
+		require.NoError(t, json.Unmarshal(msg.Data, &resp))
+		assert.True(t, resp.Success)
+
+		assert.False(t, fileExistsCheck(filepath.Join(cfg.BaseDir, volume)),
+			"ebs.delete must remove the local volume directory for %s", volume)
+	}
+}
+
+// TestIntegration_EBSDeleteEmptyVolumeNameDoesNotWipeBaseDir is the
+// regression test for the data-destruction hazard: before validation,
+// filepath.Join(cfg.BaseDir, "") collapsed to cfg.BaseDir itself, so an empty
+// Volume made ebs.delete os.RemoveAll the node's entire local WAL/checkpoint
+// cache. This must be rejected, and BaseDir must survive.
+func TestIntegration_EBSDeleteEmptyVolumeNameDoesNotWipeBaseDir(t *testing.T) {
+	t.Parallel()
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+
+	ns, natsURL := setupEmbeddedNATS(t)
+	defer ns.Shutdown()
+
+	cfg := setupTestConfig(t, natsURL)
+	createMockVolumeState(t, cfg.BaseDir, "vol-survivor")
+
+	go func() { launchService(cfg) }()
+	time.Sleep(500 * time.Millisecond)
+
+	nc, err := nats.Connect(natsURL)
+	require.NoError(t, err)
+	defer nc.Close()
+
+	reqData, _ := json.Marshal(types.EBSDeleteRequest{Volume: ""})
+	msg, err := nc.Request("ebs.delete", reqData, 3*time.Second)
+	require.NoError(t, err)
+
+	var resp types.EBSDeleteResponse
+	require.NoError(t, json.Unmarshal(msg.Data, &resp))
+	assert.False(t, resp.Success, "an empty volume name must not report success")
+	assert.NotEmpty(t, resp.Error)
+
+	assert.True(t, fileExistsCheck(cfg.BaseDir), "BaseDir itself must survive an empty volume name")
+	assert.True(t, fileExistsCheck(filepath.Join(cfg.BaseDir, "vol-survivor")),
+		"an unrelated volume directory must survive an empty volume name")
+}
+
+// TestIntegration_EBSDeleteRejectsPathTraversal covers names that would
+// otherwise let ebs.delete escape BaseDir via filepath.Join's ".." cleaning.
+// Both cases must be refused before any RemoveAll runs, and Success must not
+// come back true for a request that was refused.
+func TestIntegration_EBSDeleteRejectsPathTraversal(t *testing.T) {
+	t.Parallel()
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+
+	ns, natsURL := setupEmbeddedNATS(t)
+	defer ns.Shutdown()
+
+	cfg := setupTestConfig(t, natsURL)
+	createMockVolumeState(t, cfg.BaseDir, "vol-survivor")
+
+	go func() { launchService(cfg) }()
+	time.Sleep(500 * time.Millisecond)
+
+	nc, err := nats.Connect(natsURL)
+	require.NoError(t, err)
+	defer nc.Close()
+
+	for _, name := range []string{"../..", "a/b"} {
+		reqData, _ := json.Marshal(types.EBSDeleteRequest{Volume: name})
+		msg, err := nc.Request("ebs.delete", reqData, 3*time.Second)
+		require.NoError(t, err)
+
+		var resp types.EBSDeleteResponse
+		require.NoError(t, json.Unmarshal(msg.Data, &resp))
+		assert.False(t, resp.Success, "volume name %q must not report success", name)
+		assert.NotEmpty(t, resp.Error, "volume name %q must return an error", name)
+	}
+
+	assert.True(t, fileExistsCheck(filepath.Join(cfg.BaseDir, "vol-survivor")),
+		"an unrelated volume directory must survive a path-traversal attempt")
+}
+
+// TestIntegration_EBSDeleteValidNameLeavesSiblingsUntouched is the essential
+// non-regression check for the new validation: a legitimate delete must still
+// remove exactly its own directory, and nothing else under BaseDir.
+func TestIntegration_EBSDeleteValidNameLeavesSiblingsUntouched(t *testing.T) {
+	t.Parallel()
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+
+	ns, natsURL := setupEmbeddedNATS(t)
+	defer ns.Shutdown()
+
+	cfg := setupTestConfig(t, natsURL)
+	createMockVolumeState(t, cfg.BaseDir, "vol-target")
+	createMockVolumeState(t, cfg.BaseDir, "vol-sibling")
+
+	go func() { launchService(cfg) }()
+	time.Sleep(500 * time.Millisecond)
+
+	nc, err := nats.Connect(natsURL)
+	require.NoError(t, err)
+	defer nc.Close()
+
+	reqData, _ := json.Marshal(types.EBSDeleteRequest{Volume: "vol-target"})
+	msg, err := nc.Request("ebs.delete", reqData, 3*time.Second)
+	require.NoError(t, err)
+
+	var resp types.EBSDeleteResponse
+	require.NoError(t, json.Unmarshal(msg.Data, &resp))
+	assert.True(t, resp.Success)
+	assert.Empty(t, resp.Error)
+
+	assert.False(t, fileExistsCheck(filepath.Join(cfg.BaseDir, "vol-target")),
+		"the targeted volume directory must be removed")
+	assert.True(t, fileExistsCheck(filepath.Join(cfg.BaseDir, "vol-sibling")),
+		"a sibling volume directory must survive deleting a different volume")
+}
+
 func TestIntegration_EBSDeleteInvalidJSON(t *testing.T) {
 	t.Parallel()
 	if testing.Short() {

spinifex/services/viperblockd/viperblockd_test.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"path/filepath"
 	"sync"
 	"testing"
 	"time"
@@ -512,3 +513,62 @@ func TestRetryLoadState(t *testing.T) {
 		assert.Equal(t, 225*time.Millisecond, lastDelay)
 	})
 }
+
+// TestValidVolumeName covers the character-level gate every handler that
+// turns a wire volume name into a filesystem path must pass first.
+func TestValidVolumeName(t *testing.T) {
+	valid := []string{"vol-0123456789abcdef0", "vol-0123456789abcdef0-efi", "a"}
+	for _, name := range valid {
+		assert.True(t, validVolumeName(name), "expected %q to be valid", name)
+	}
+
+	invalid := []string{"", ".", "..", "../..", "a/b", "/etc/passwd", "vol/../.."}
+	for _, name := range invalid {
+		assert.False(t, validVolumeName(name), "expected %q to be rejected", name)
+	}
+}
+
+// TestLocalVolumeDir is the regression test for the ebs.delete data-destruction
+// hazard: an unvalidated volume name reaching os.RemoveAll could wipe BaseDir
+// itself (empty name) or escape it entirely ("../.."). Every case here must be
+// rejected before a caller ever gets a path back to remove.
+func TestLocalVolumeDir(t *testing.T) {
+	baseDir := t.TempDir()
+
+	t.Run("empty_volume_rejected", func(t *testing.T) {
+		_, err := localVolumeDir(baseDir, "")
+		require.Error(t, err, "an empty volume must never resolve to BaseDir itself")
+	})
+
+	t.Run("dot_rejected", func(t *testing.T) {
+		_, err := localVolumeDir(baseDir, ".")
+		require.Error(t, err)
+	})
+
+	t.Run("parent_traversal_rejected", func(t *testing.T) {
+		for _, name := range []string{"..", "../..", "../../etc"} {
+			_, err := localVolumeDir(baseDir, name)
+			require.Error(t, err, "volume %q must not escape BaseDir", name)
+		}
+	})
+
+	t.Run("embedded_separator_rejected", func(t *testing.T) {
+		for _, name := range []string{"a/b", "a/../../b", "/abs/path"} {
+			_, err := localVolumeDir(baseDir, name)
+			require.Error(t, err, "volume %q must not contain a path separator", name)
+		}
+	})
+
+	t.Run("empty_base_dir_rejected", func(t *testing.T) {
+		_, err := localVolumeDir("", "vol-abc123")
+		require.Error(t, err)
+	})
+
+	t.Run("valid_name_resolves_under_base_dir", func(t *testing.T) {
+		dir, err := localVolumeDir(baseDir, "vol-abc123")
+		require.NoError(t, err)
+		absBase, err := filepath.Abs(baseDir)
+		require.NoError(t, err)
+		assert.Equal(t, filepath.Join(absBase, "vol-abc123"), dir)
+	})
+}

spinifex/vm/shutdown.go

@@ -240,8 +240,21 @@ func (m *Manager) finalizeTerminated(instance *VM) error {
 	// instance that was never inserted into the local map.
 	m.Inspect(instance, func(v *VM) { v.LastNode = m.deps.NodeID })
 
-	if err := m.transitionWithPrecheck(instance, StateTerminated); err != nil {
-		return fmt.Errorf("transition to terminated: %w", err)
+	transitionErr := m.transitionWithPrecheck(instance, StateTerminated)
+	if transitionErr != nil && errors.Is(transitionErr, ErrInvalidTransition) {
+		// A genuine invalid/raced transition: in-memory status never reached
+		// terminated, so there is nothing durable to record yet.
+		return fmt.Errorf("transition to terminated: %w", transitionErr)
+	}
+	if transitionErr != nil {
+		// In-memory status reached terminated even though local persistence
+		// failed (e.g. ENOSPC writing the local state file). Keep going:
+		// WriteTerminatedInstance below is a JetStream KV write independent
+		// of local disk space, so the durable record can still land and
+		// TerminatedTeardownReaper picks it up on its next sweep without
+		// requiring an operator restart.
+		slog.Warn("Local state persistence failed on terminate, continuing to durable KV write",
+			"instanceId", instance.ID, "err", transitionErr)
 	}
 
 	// Stamp the termination time so the GC backstop can preserve a
@@ -254,6 +267,9 @@ func (m *Manager) finalizeTerminated(instance *VM) error {
 		if err := m.deps.StateStore.WriteTerminatedInstance(instance.ID, instance); err != nil {
 			slog.Error("Failed to write terminated instance to KV, keeping in local state for retry",
 				"instanceId", instance.ID, "err", err)
+			if transitionErr != nil {
+				return transitionErr
+			}
 			return err
 		}
 	}
@@ -276,7 +292,7 @@ func (m *Manager) finalizeTerminated(instance *VM) error {
 	}
 	slog.Info("Released instance ownership to KV",
 		"instanceId", instance.ID, "state", string(StateTerminated), "lastNode", m.deps.NodeID)
-	return nil
+	return transitionErr
 }
 
 // reconcileVanishedQEMU finalizes a shutting-down instance whose QEMU process

spinifex/vm/shutdown_test.go

@@ -1018,6 +1018,55 @@ func TestTerminate_WriteTerminatedFailure_PropagatesAndKeepsLocal(t *testing.T)
 	assert.True(t, stillInMap, "instance must remain in local map for retry on the next daemon restart")
 }
 
+// TestTerminate_LocalPersistenceFailure_StillDurablyRecordsTerminated covers
+// the ENOSPC scenario: TransitionState fails to persist locally (e.g. the
+// local state file write hits a full disk) but the in-memory status still
+// reaches StateTerminated. Before the fix, finalizeTerminated bailed out on
+// that error before ever calling WriteTerminatedInstance, so the durable
+// terminated record TerminatedTeardownReaper scans was never written and
+// nothing retried teardown without an operator restart. The durable KV write
+// (a JetStream call independent of local disk space) must still happen.
+func TestTerminate_LocalPersistenceFailure_StillDurablyRecordsTerminated(t *testing.T) {
+	t.Setenv("XDG_RUNTIME_DIR", t.TempDir())
+	persistErr := errors.New("no space left on device")
+	store := newFakeStateStore()
+	cleaner := &recordingInstanceCleaner{}
+	down := &atomic.Int64{}
+	m := NewManager()
+	m.SetDeps(Deps{
+		NodeID:          "test-node",
+		StateStore:      store,
+		VolumeMounter:   &fakeVolumeMounter{},
+		InstanceCleaner: cleaner,
+		TransitionState: func(v *VM, target InstanceState) error {
+			// Mirrors daemon.TransitionState: memory mutation always
+			// succeeds; only the local file persistence fails, matching a
+			// full disk on the terminated transition specifically.
+			m.Inspect(v, func(vv *VM) { vv.Status = target })
+			if target == StateTerminated {
+				return persistErr
+			}
+			return nil
+		},
+		ShutdownSignal: func() bool { return false },
+		Hooks: ManagerHooks{
+			OnInstanceDown: func(id string) { down.Add(1) },
+		},
+	})
+
+	v := &VM{ID: "i-local-persist-fail", Status: StateRunning, InstanceType: "t3.micro", Instance: &ec2.Instance{}}
+	m.Insert(v)
+
+	err := m.Terminate(v.ID)
+
+	require.ErrorIs(t, err, persistErr, "the local persistence failure must still surface to the caller")
+	require.NotNil(t, store.terminated[v.ID],
+		"WriteTerminatedInstance must still run so TerminatedTeardownReaper can find and finish this instance")
+	assert.Equal(t, int64(1), down.Load(), "OnInstanceDown must still fire once the durable record lands")
+	_, stillInMap := m.Get(v.ID)
+	assert.False(t, stillInMap, "instance must leave the local running map once terminated is durable")
+}
+
 // TestTerminate_DeleteIfReclaimed_NoHook covers the slot-reclaim branch
 // in finalizeTerminated (shutdown.go:197-201): a concurrent handler
 // inserts a fresh VM under the same id between WriteTerminatedInstance