fix(cli): eliminate update-check "send on closed channel" race

startupHooks runs once per cobra Execute; its background goroutine used the
package-level updateCheckResult directly for both close() and send. A process
running multiple commands (the test binary) reassigns that pointer each Execute,
so one goroutine could close another's channel -> panic: send on closed channel,
plus an unguarded pointer data race.

Each invocation now owns its own channel (startUpdateAdvisory): the goroutine
sends/closes only its local channel, never the shared pointer, and the pointer
handoff is mutex-guarded. Extracted consumeUpdateAdvisory + checkForUpdateMessage
so PersistentPostRun and vdb.go share one safe consumer. Adds concurrency,
delivery and nil-safety regression tests (race-clean under -race).

Author: 0x73746F66

cmd/root.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/spf13/cobra"
@@ -34,8 +35,80 @@ var (
 	buildDate = "unknown" // -X github.com/vulnetix/cli/v3/cmd.buildDate=
 )
 
-// updateCheckResult receives the update advisory message from the background goroutine.
-var updateCheckResult chan string
+// updateCheckResult receives the update advisory message from the background
+// goroutine started by the most recent startupHooks invocation. It is guarded
+// by updateCheckMu because cobra.OnInitialize fires startupHooks once per
+// Execute — a process that runs multiple commands (notably the test binary)
+// reassigns this pointer while prior background goroutines and the PostRun
+// consumer may still touch it.
+var (
+	updateCheckMu     sync.Mutex
+	updateCheckResult chan string
+)
+
+// startUpdateAdvisory installs a fresh advisory channel for the current command
+// and runs check in the background, delivering a non-empty advisory message (if
+// any) exactly once. Each call owns its own channel: the goroutine sends to and
+// closes only that local channel, never the shared pointer, so overlapping
+// invocations can never send on or close another call's channel (the historical
+// "send on closed channel" panic). The channel is buffered (cap 1) so the send
+// never blocks even when no consumer is waiting.
+func startUpdateAdvisory(check func() (msg string, ok bool)) {
+	ch := make(chan string, 1)
+	updateCheckMu.Lock()
+	updateCheckResult = ch
+	updateCheckMu.Unlock()
+	go func() {
+		defer close(ch)
+		if msg, ok := check(); ok && msg != "" {
+			ch <- msg
+		}
+	}()
+}
+
+// consumeUpdateAdvisory non-blockingly reads the pending advisory message for
+// the current command, returning "" when none is ready. Safe to call from any
+// goroutine.
+func consumeUpdateAdvisory() string {
+	updateCheckMu.Lock()
+	ch := updateCheckResult
+	updateCheckMu.Unlock()
+	if ch == nil {
+		return ""
+	}
+	select {
+	case msg := <-ch:
+		return msg
+	default:
+		return ""
+	}
+}
+
+// checkForUpdateMessage performs the network update check and returns an
+// advisory string when a newer release is available. It records that a check
+// happened so ShouldCheckForUpdate throttles subsequent runs.
+func checkForUpdateMessage() (string, bool) {
+	release, err := update.CheckLatest()
+	if err != nil {
+		return "", false
+	}
+	update.RecordUpdateCheck()
+	latest, err := update.ParseVersion(release.TagName)
+	if err != nil {
+		return "", false
+	}
+	current, err := update.ParseVersion(version)
+	if err != nil {
+		return "", false
+	}
+	if !latest.IsNewerThan(current) {
+		return "", false
+	}
+	return fmt.Sprintf(
+		"\nUpdate available: v%s → v%s\nRun 'vulnetix update' to update.\n",
+		current, latest,
+	), true
+}
 
 // rootCmd represents the base command when called without any subcommands
 var rootCmd = &cobra.Command{
@@ -53,15 +126,8 @@ vulnerabilities efficiently.`,
 		})
 	},
 	PersistentPostRun: func(cmd *cobra.Command, args []string) {
-		if updateCheckResult == nil {
-			return
-		}
-		select {
-		case msg := <-updateCheckResult:
-			if msg != "" {
-				fmt.Fprint(os.Stderr, msg)
-			}
-		default:
+		if msg := consumeUpdateAdvisory(); msg != "" {
+			fmt.Fprint(os.Stderr, msg)
 		}
 	},
 	RunE: func(cmd *cobra.Command, args []string) error {
@@ -327,29 +393,7 @@ func startupHooks() {
 		return
 	}
 
-	updateCheckResult = make(chan string, 1)
-	go func() {
-		defer close(updateCheckResult)
-		release, err := update.CheckLatest()
-		if err != nil {
-			return
-		}
-		update.RecordUpdateCheck()
-		latest, err := update.ParseVersion(release.TagName)
-		if err != nil {
-			return
-		}
-		current, err := update.ParseVersion(version)
-		if err != nil {
-			return
-		}
-		if latest.IsNewerThan(current) {
-			updateCheckResult <- fmt.Sprintf(
-				"\nUpdate available: v%s → v%s\nRun 'vulnetix update' to update.\n",
-				current, latest,
-			)
-		}
-	}()
+	startUpdateAdvisory(checkForUpdateMessage)
 }
 
 func init() {

cmd/update_advisory_test.go

@@ -0,0 +1,74 @@
+package cmd
+
+import (
+	"sync"
+	"testing"
+	"time"
+)
+
+// TestUpdateAdvisoryConcurrentInvocations reproduces the historical
+// "send on closed channel" panic: cobra.OnInitialize fires startupHooks once
+// per Execute, so a process running many commands (the test binary itself)
+// repeatedly reassigns updateCheckResult while prior background goroutines and
+// the PostRun consumer still touch it. The pre-fix code had each goroutine
+// close/send the shared package var, so one goroutine could close another's
+// channel. Each invocation now owns its own channel, so this must run cleanly
+// (and race-clean under `go test -race`).
+func TestUpdateAdvisoryConcurrentInvocations(t *testing.T) {
+	const workers = 300
+	var wg sync.WaitGroup
+	wg.Add(workers)
+	for i := 0; i < workers; i++ {
+		go func(i int) {
+			defer wg.Done()
+			// Half the invocations produce an advisory, half don't — exercising
+			// both the send and the close-only paths.
+			startUpdateAdvisory(func() (string, bool) {
+				return "update available", i%2 == 0
+			})
+			// Concurrently consume, as PersistentPostRun / vdb.go do.
+			_ = consumeUpdateAdvisory()
+		}(i)
+	}
+	wg.Wait()
+	// Drain whatever the last invocation left, exercising the consumer once more.
+	_ = consumeUpdateAdvisory()
+}
+
+// TestUpdateAdvisoryDelivery verifies a produced message is delivered exactly
+// once and that a subsequent read returns empty (channel drained/closed).
+func TestUpdateAdvisoryDelivery(t *testing.T) {
+	startUpdateAdvisory(func() (string, bool) {
+		return "hello", true
+	})
+	// The background goroutine may not have sent yet; poll with a yield until
+	// the message arrives or a deadline elapses. (consumeUpdateAdvisory is
+	// non-blocking by contract, so the test does the waiting.)
+	var got string
+	deadline := time.Now().Add(2 * time.Second)
+	for time.Now().Before(deadline) {
+		if msg := consumeUpdateAdvisory(); msg != "" {
+			got = msg
+			break
+		}
+		time.Sleep(time.Millisecond)
+	}
+	if got != "hello" {
+		t.Fatalf("expected advisory %q to be delivered, got %q", "hello", got)
+	}
+	// After consumption the buffered value is gone; a further read is empty.
+	if msg := consumeUpdateAdvisory(); msg != "" {
+		t.Errorf("expected no further advisory, got %q", msg)
+	}
+}
+
+// TestConsumeUpdateAdvisoryNilChannel confirms the consumer is safe before any
+// advisory channel has been installed.
+func TestConsumeUpdateAdvisoryNilChannel(t *testing.T) {
+	updateCheckMu.Lock()
+	updateCheckResult = nil
+	updateCheckMu.Unlock()
+	if msg := consumeUpdateAdvisory(); msg != "" {
+		t.Errorf("expected empty advisory for nil channel, got %q", msg)
+	}
+}

cmd/vdb.go

@@ -170,14 +170,8 @@ Examples:
 		}
 
 		// Replicate rootCmd's PersistentPostRun (update check notification)
-		if updateCheckResult != nil {
-			select {
-			case msg := <-updateCheckResult:
-				if msg != "" {
-					fmt.Fprint(os.Stderr, msg)
-				}
-			default:
-			}
+		if msg := consumeUpdateAdvisory(); msg != "" {
+			fmt.Fprint(os.Stderr, msg)
 		}
 
 		return nil