refactor: whole-project /code audit round 2 (fixes + comment tightening)

cmd/core/ensure_image_test.go

@@ -42,8 +42,7 @@ func (f *fakeImageBackend) Config(context.Context, []*types.VMConfig) ([][]*type
 	return nil, nil, nil
 }
 
-// Regression guard for issue 37: pinned digest with no local hit must force-pull
-// to bypass cloudimg's URL-keyed cache.
+// Issue 37 regression guard: a pinned digest with no local hit must force-pull past cloudimg's URL-keyed cache.
 func TestEnsureImage_ForceWhenDigestPinned(t *testing.T) {
 	const (
 		url    = "https://epoch.example/dl/simular/win11"
@@ -98,8 +97,7 @@ func TestEnsureImage_ForceWhenDigestPinned(t *testing.T) {
 	}
 }
 
-// A cloudimg ref without an http(s) scheme reaching Pull surfaces as
-// `unsupported protocol scheme` from http.Get; the shape guard short-circuits.
+// A non-http(s) cloudimg ref reaching Pull would surface http.Get's `unsupported protocol scheme`; the shape guard short-circuits first.
 func TestEnsureImage_SkipsBadShape(t *testing.T) {
 	tests := []struct {
 		name      string

cmd/core/helpers.go

@@ -98,8 +98,7 @@ func EnsureImage(ctx context.Context, backends []imagebackend.Images, vmCfg *typ
 		if img != nil {
 			return // exact image version exists locally
 		}
-		// Pull by digest reference when available — ensures we get the exact
-		// version recorded at snapshot time, not whatever the tag points to now.
+		// Pull by digest when available — the tag may point at a different manifest than at snapshot time.
 		pullRef := digestPullRef(vmCfg.Image, vmCfg.ImageDigest, vmCfg.ImageType)
 		if shapeErr := validateRefShape(pullRef, vmCfg.ImageType); shapeErr != nil {
 			logger.Warnf(ctx, "skipping auto-pull of %s: %v — pre-pull manually if missing", pullRef, shapeErr)

cmd/core/metering.go

@@ -41,7 +41,7 @@ func buildRecorder(ctx context.Context, conf *config.Config) metering.Recorder {
 	case "stderr":
 		return meteringstderr.New()
 	default:
-		log.WithFunc("core.MeteringRecorder").Warnf(ctx, "unknown metering backend %q; using nop", conf.Metering.Backend)
+		log.WithFunc("core.buildRecorder").Warnf(ctx, "unknown metering backend %q; using nop", conf.Metering.Backend)
 		return metering.NopRecorder{}
 	}
 }

cmd/images/handler_test.go

@@ -8,9 +8,9 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
-)
 
-var qcow2Magic = []byte{'Q', 'F', 'I', 0xfb}
+	"github.com/cocoonstack/cocoon/utils"
+)
 
 func gzipWrap(t *testing.T, data []byte) []byte {
 	t.Helper()
@@ -55,7 +55,7 @@ func writeTempFile(t *testing.T, dir, name string, data []byte) string {
 }
 
 func TestDetectReader(t *testing.T) {
-	qcow2Data := append(qcow2Magic, make([]byte, 100)...)
+	qcow2Data := append(utils.Qcow2Magic, make([]byte, 100)...)
 	tarData := []byte("this is a tar-like stream of data, not really tar but not qcow2 either")
 
 	tests := []struct {
@@ -155,7 +155,7 @@ func TestDetectReader_GzipPreservesFullContent(t *testing.T) {
 
 func TestDetectLocalImportSource(t *testing.T) {
 	dir := t.TempDir()
-	qcow2Path := writeTempFile(t, dir, "image.qcow2", append(qcow2Magic, make([]byte, 32)...))
+	qcow2Path := writeTempFile(t, dir, "image.qcow2", append(utils.Qcow2Magic, make([]byte, 32)...))
 	tarPath := writeTempFile(t, dir, "image.tar", tarWrap(t, "payload.txt", []byte("payload")))
 	gzipTarPath := writeTempFile(t, dir, "image.tar.gz", gzipWrap(t, tarWrap(t, "payload.txt", []byte("payload"))))
 	invalidPath := writeTempFile(t, dir, "invalid.bin", []byte("not an archive"))
@@ -193,7 +193,7 @@ func TestDetectLocalImportSource(t *testing.T) {
 
 func TestPlanLocalImportPreservesAllFiles(t *testing.T) {
 	dir := t.TempDir()
-	part1 := writeTempFile(t, dir, "part-1.qcow2", append(qcow2Magic, make([]byte, 8)...))
+	part1 := writeTempFile(t, dir, "part-1.qcow2", append(utils.Qcow2Magic, make([]byte, 8)...))
 	part2 := writeTempFile(t, dir, "part-2.qcow2", []byte("second-part"))
 
 	plan, err := planLocalImport([]string{part1, part2})

cmd/images/import.go

@@ -20,6 +20,7 @@ import (
 	"github.com/cocoonstack/cocoon/progress"
 	cloudimgProgress "github.com/cocoonstack/cocoon/progress/cloudimg"
 	ociProgress "github.com/cocoonstack/cocoon/progress/oci"
+	"github.com/cocoonstack/cocoon/utils"
 )
 
 func (h Handler) Import(cmd *cobra.Command, args []string) error {
@@ -226,10 +227,10 @@ func detectLocalImportSource(filePath string) (importSourceKind, error) {
 		return 0, fmt.Errorf("peek %s: %w", filePath, readErr)
 	}
 
-	if n >= 2 && bytes.Equal(magic[:2], []byte{0x1f, 0x8b}) {
+	if n >= 2 && bytes.Equal(magic[:2], utils.GzipMagic) {
 		return importSourceStream, nil
 	}
-	if n >= 4 && bytes.Equal(magic[:4], []byte{'Q', 'F', 'I', 0xfb}) {
+	if n >= 4 && bytes.Equal(magic[:4], utils.Qcow2Magic) {
 		return importSourceQcow2, nil
 	}
 
@@ -270,7 +271,7 @@ func detectReader(r io.Reader) (io.Reader, imageType, func(), error) {
 		return nil, 0, nil, fmt.Errorf("peek content: %w", err)
 	}
 
-	if cpeek[0] == 'Q' && cpeek[1] == 'F' && cpeek[2] == 'I' && cpeek[3] == 0xfb {
+	if bytes.HasPrefix(cpeek, utils.Qcow2Magic) {
 		return inner, imageTypeQcow2, cleanup, nil
 	}
 

cmd/snapshot/commands.go

@@ -61,7 +61,7 @@ func Command(h Actions) *cobra.Command {
 	}
 	exportCmd.Flags().StringP("output", "o", "", "output file path (default: <name-or-id>.tar)")
 	exportCmd.Flags().Bool("gzip", false, "compress output with gzip")
-	exportCmd.Flags().String("to-dir", "", "export into a directory (must be empty/absent) instead of a tar; pairs with `vm clone --from-dir`")
+	exportCmd.Flags().String("to-dir", "", "export into a directory (must be empty/absent) instead of a tar; pairs with 'vm clone --from-dir'")
 	exportCmd.MarkFlagsMutuallyExclusive("to-dir", "output")
 	exportCmd.MarkFlagsMutuallyExclusive("to-dir", "gzip")
 

cmd/snapshot/handler.go

@@ -110,13 +110,10 @@ func (h Handler) List(cmd *cobra.Command, _ []string) error {
 	}
 
 	if filterIDs != nil {
-		filtered := snapshots[:0]
-		for _, s := range snapshots {
-			if _, ok := filterIDs[s.ID]; ok {
-				filtered = append(filtered, s)
-			}
-		}
-		snapshots = filtered
+		snapshots = slices.DeleteFunc(snapshots, func(s *types.Snapshot) bool {
+			_, ok := filterIDs[s.ID]
+			return !ok
+		})
 	}
 
 	if len(snapshots) == 0 {

cmd/vm/debug.go

@@ -109,48 +109,46 @@ func printFCDebug(configs []*types.StorageConfig, boot *types.BootConfig, vmCfg
 	fmt.Println("# Configure via REST API (use curl or similar):")
 	sock := fmt.Sprintf("/tmp/fc-%s.sock", vmCfg.Name)
 
-	fmt.Printf("# 1. Machine config\n")
+	fmt.Println("# 1. Machine config")
 	fmt.Printf("curl --unix-socket %s -X PUT http://localhost/machine-config \\\n", sock)
 	fmt.Printf("  -d '{\"vcpu_count\": %d, \"mem_size_mib\": %d}'\n", vmCfg.CPU, memMiB)
 	fmt.Println()
 
-	fmt.Printf("# 2. Boot source\n")
+	fmt.Println("# 2. Boot source")
 	fmt.Printf("curl --unix-socket %s -X PUT http://localhost/boot-source \\\n", sock)
 	fmt.Printf("  -d '{\"kernel_image_path\": \"%s\", \"initrd_path\": \"%s\", \"boot_args\": \"%s\"}'\n",
 		boot.KernelPath, boot.InitrdPath, cmdline)
 	fmt.Println()
 
-	fmt.Printf("# 3. Drives\n")
+	fmt.Println("# 3. Drives")
 	for i, sc := range configs {
 		fmt.Printf("curl --unix-socket %s -X PUT http://localhost/drives/drive_%d \\\n", sock, i)
 		fmt.Printf("  -d '{\"drive_id\": \"drive_%d\", \"path_on_host\": \"%s\", \"is_root_device\": false, \"is_read_only\": %t}'\n",
 			i, sc.Path, sc.RO)
 	}
-	// COW drive
 	fmt.Printf("curl --unix-socket %s -X PUT http://localhost/drives/drive_%d \\\n", sock, len(configs))
 	fmt.Printf("  -d '{\"drive_id\": \"drive_%d\", \"path_on_host\": \"%s\", \"is_root_device\": false, \"is_read_only\": false}'\n",
 		len(configs), cowPath)
 	fmt.Println()
 
 	if size, ok := hypervisor.BalloonSize(vmCfg.Memory, vmCfg.Windows); ok {
-		fmt.Printf("# 4. Balloon\n")
+		fmt.Println("# 4. Balloon")
 		fmt.Printf("curl --unix-socket %s -X PUT http://localhost/balloon \\\n", sock)
 		fmt.Printf("  -d '{\"amount_mib\": %d, \"deflate_on_oom\": true, \"free_page_reporting\": true}'\n", size>>20) //nolint:mnd
 		fmt.Println()
 	}
 
-	fmt.Printf("# 5. Start\n")
+	fmt.Println("# 5. Start")
 	fmt.Printf("curl --unix-socket %s -X PUT http://localhost/actions \\\n", sock)
-	fmt.Printf("  -d '{\"action_type\": \"InstanceStart\"}'\n")
+	fmt.Println("  -d '{\"action_type\": \"InstanceStart\"}'")
 }
 
 func buildCHDebugSpec(cmd *cobra.Command, storageConfigs []*types.StorageConfig, boot *types.BootConfig, vmCfg *types.VMConfig) chDebugSpec {
 	maxCPU, _ := cmd.Flags().GetInt("max-cpu")
 	balloon, _ := cmd.Flags().GetInt("balloon")
 	cowPath, _ := cmd.Flags().GetString("cow")
 	chBin, _ := cmd.Flags().GetString("ch")
-	// Mirror runtime gating: Windows / sub-MinBalloon VMs never get balloon,
-	// even if the user passed --balloon, so debug output stays truthful.
+	// Mirror runtime gating: Windows / sub-MinBalloon VMs never get balloon even with --balloon, so debug output stays truthful.
 	size, ok := hypervisor.BalloonSize(vmCfg.Memory, vmCfg.Windows)
 	switch {
 	case !ok:
@@ -196,11 +194,11 @@ func printCHDebug(s chDebugSpec) {
 		fmt.Printf("%s \\\n", s.CHBin)
 		fmt.Printf("  --kernel %s \\\n", s.Boot.KernelPath)
 		fmt.Printf("  --initramfs %s \\\n", s.Boot.InitrdPath)
-		fmt.Printf("  --disk")
+		fmt.Print("  --disk")
 		for _, d := range diskArgs {
 			fmt.Printf(" \\\n    \"%s\"", d)
 		}
-		fmt.Printf(" \\\n")
+		fmt.Print(" \\\n")
 		fmt.Printf("  --cmdline \"%s\" \\\n", cmdline)
 	} else {
 		if s.CowPath == "" {
@@ -216,7 +214,7 @@ func printCHDebug(s chDebugSpec) {
 		fmt.Printf("# Launch VM: %s (image: %s, boot: UEFI firmware)\n", s.VMCfg.Name, s.VMCfg.Image)
 		fmt.Printf("%s \\\n", s.CHBin)
 		fmt.Printf("  --firmware %s \\\n", s.Boot.FirmwarePath)
-		fmt.Printf("  --disk \\\n")
+		fmt.Print("  --disk \\\n")
 		diskArgs := cloudhypervisor.DebugDiskCLIArgs([]*types.StorageConfig{{Path: s.CowPath, RO: false}}, cpu, diskQueueSize, noDirectIO)
 		fmt.Printf("    \"%s\" \\\n", diskArgs[0])
 	}
@@ -234,10 +232,10 @@ func printCommonCHArgs(s chDebugSpec) {
 	}
 	fmt.Printf("  --cpus boot=%d,max=%d%s \\\n", s.VMCfg.CPU, s.MaxCPU, cpuExtra)
 	fmt.Printf("  --memory size=%dM%s \\\n", s.VMCfg.Memory>>20, memExtra) //nolint:mnd
-	fmt.Printf("  --rng src=/dev/urandom \\\n")
+	fmt.Print("  --rng src=/dev/urandom \\\n")
 	if s.Balloon > 0 {
 		fmt.Printf("  --balloon size=%dM,deflate_on_oom=on,free_page_reporting=on \\\n", s.Balloon)
 	}
-	fmt.Printf("  --watchdog \\\n")
-	fmt.Printf("  --serial tty --console off\n")
+	fmt.Print("  --watchdog \\\n")
+	fmt.Println("  --serial tty --console off")
 }

cmd/vm/exec_test.go

@@ -77,8 +77,7 @@ func TestReadHybridVsockReply(t *testing.T) {
 	}
 }
 
-// TestDialHybridVsock_ConnectHandshake spins up an in-process listener that
-// speaks the CH/FC hybrid vsock dialect (CONNECT <port>\n → OK <port>\n).
+// TestDialHybridVsock_ConnectHandshake drives the CH/FC hybrid vsock dialect (CONNECT <port>\n → OK <port>\n) against an in-process listener.
 func TestDialHybridVsock_ConnectHandshake(t *testing.T) {
 	// macOS caps unix socket paths at ~104 bytes, so t.TempDir() (long
 	// /var/folders/... path) can overflow. Use os.CreateTemp + immediate unlink.

cmd/vm/lifecycle.go

@@ -343,7 +343,7 @@ func finishRoutedCmd(ctx context.Context, cmd *cobra.Command, logTag, name, past
 }
 
 func batchRoutedCmd(ctx context.Context, cmd *cobra.Command, name, pastTense string, routed map[hypervisor.Hypervisor][]string, fn func(hypervisor.Hypervisor, []string) ([]string, error)) error {
-	logTag := "cmd." + name
+	logTag := "cmd.vm." + name
 	allDone, lastErr := runRoutedLoop(ctx, logTag, pastTense, cmdcore.WantJSON(cmd), routed, fn)
 	return finishRoutedCmd(ctx, cmd, logTag, name, pastTense, allDone, lastErr)
 }

cmd/vm/run.go

@@ -251,6 +251,9 @@ func (h Handler) cloneFromDir(ctx context.Context, cmd *cobra.Command, conf *con
 	if err != nil {
 		return fmt.Errorf("load envelope: %w", err)
 	}
+	if conf == nil {
+		return fmt.Errorf("nil config")
+	}
 	// Local copy keeps backend flip from leaking to the caller's shared *config.Config.
 	localConf := *conf
 	if cfg.Hypervisor != "" {

cmd/vm/status.go

@@ -242,6 +242,9 @@ func statusEventDiffLoop(ctx context.Context, hypers []hypervisor.Hypervisor, fi
 		vms := listAndFilter(ctx, hypers, filters)
 		curr := make(map[string]entry, len(vms))
 		for _, vm := range vms {
+			if vm == nil {
+				continue
+			}
 			state := cmdcore.ReconcileState(vm)
 			vmCopy := *vm
 			vmCopy.State = types.VMState(state)
@@ -315,15 +318,9 @@ func applyFilters(vms []*types.VM, filters []string) []*types.VM {
 }
 
 func matchesFilter(vm *types.VM, filters []string) bool {
-	for _, f := range filters {
-		if vm.ID == f || vm.Config.Name == f {
-			return true
-		}
-		if len(f) >= 3 && strings.HasPrefix(vm.ID, f) {
-			return true
-		}
-	}
-	return false
+	return slices.ContainsFunc(filters, func(f string) bool {
+		return vm.ID == f || vm.Config.Name == f || (len(f) >= 3 && strings.HasPrefix(vm.ID, f))
+	})
 }
 
 func snapshotAll(vms []*types.VM) []vmSnapshot {

config/config.go

@@ -19,44 +19,33 @@ type HypervisorType string
 
 // Config holds global Cocoon configuration.
 type Config struct {
-	// RootDir is the base directory for persistent data (images, firmware, VM DB).
-	// Env: COCOON_ROOT_DIR. Default: /var/lib/cocoon.
+	// RootDir: persistent data (images, firmware, VM DB). Env: COCOON_ROOT_DIR. Default: /var/lib/cocoon.
 	RootDir string `json:"root_dir" mapstructure:"root_dir"`
 	// RunDir: ephemeral runtime state (PID files, sockets). Env: COCOON_RUN_DIR. Default: /var/lib/cocoon/run.
 	RunDir string `json:"run_dir" mapstructure:"run_dir"`
-	// LogDir is the base directory for VM and process logs.
-	// Env: COCOON_LOG_DIR. Default: /var/log/cocoon.
+	// LogDir: VM and process logs. Env: COCOON_LOG_DIR. Default: /var/log/cocoon.
 	LogDir string `json:"log_dir" mapstructure:"log_dir"`
-	// CHBinary is the path or name of the cloud-hypervisor executable.
-	// Default: "cloud-hypervisor".
+	// CHBinary: path or name of the cloud-hypervisor executable. Default: "cloud-hypervisor".
 	CHBinary string `json:"ch_binary" mapstructure:"ch_binary"`
-	// FCBinary is the path or name of the firecracker executable.
-	// Default: "firecracker".
+	// FCBinary: path or name of the firecracker executable. Default: "firecracker".
 	FCBinary string `json:"fc_binary" mapstructure:"fc_binary"`
-	// UseFirecracker selects Firecracker as the hypervisor backend.
-	// Set via --fc flag. Default: false (use Cloud Hypervisor).
+	// UseFirecracker selects the Firecracker backend (--fc flag). Default: false (Cloud Hypervisor).
 	UseFirecracker bool `json:"use_firecracker,omitempty" mapstructure:"use_firecracker"`
 	// StopTimeoutSeconds: guest ACPI grace before SIGTERM/SIGKILL escalation. Default: 30.
 	StopTimeoutSeconds int `json:"stop_timeout_seconds" mapstructure:"stop_timeout_seconds"`
-	// PoolSize is the goroutine pool size for concurrent operations.
-	// Defaults to runtime.NumCPU() if zero.
+	// PoolSize: goroutine pool size for concurrent operations; 0 = runtime.NumCPU().
 	PoolSize int `json:"pool_size" mapstructure:"pool_size"`
-	// CNIConfDir is the directory for CNI plugin configuration files.
-	// Default: /etc/cni/net.d.
+	// CNIConfDir: CNI plugin configuration dir. Default: /etc/cni/net.d.
 	CNIConfDir string `json:"cni_conf_dir" mapstructure:"cni_conf_dir"`
-	// CNIBinDir is the directory for CNI plugin binaries.
-	// Default: /opt/cni/bin.
+	// CNIBinDir: CNI plugin binary dir. Default: /opt/cni/bin.
 	CNIBinDir string `json:"cni_bin_dir" mapstructure:"cni_bin_dir"`
 	// DNS: comma/semicolon-separated DNS servers injected into VM net config. Env: COCOON_DNS. Default: "8.8.8.8,1.1.1.1".
 	DNS string `json:"dns" mapstructure:"dns"`
-	// SocketWaitTimeoutSeconds is how long to wait for the CH API socket
-	// after process start. Default: 5. Increase for slow storage.
+	// SocketWaitTimeoutSeconds: wait for the CH API socket after start. Default: 5; increase for slow storage.
 	SocketWaitTimeoutSeconds int `json:"socket_wait_timeout_seconds" mapstructure:"socket_wait_timeout_seconds"`
-	// TerminateGracePeriodSeconds is the SIGTERM→SIGKILL window when
-	// force-killing a CH process. Default: 5.
-	TerminateGracePeriodSeconds int `json:"terminate_grace_period_seconds" mapstructure:"terminate_grace_period_seconds"`
-	// Log configuration, uses eru core's ServerLogConfig.
-	Log *coretypes.ServerLogConfig `json:"log" mapstructure:"log"`
+	// TerminateGracePeriodSeconds: SIGTERM→SIGKILL window when force-killing CH. Default: 5.
+	TerminateGracePeriodSeconds int                        `json:"terminate_grace_period_seconds" mapstructure:"terminate_grace_period_seconds"`
+	Log                         *coretypes.ServerLogConfig `json:"log" mapstructure:"log"`
 	// Metering selects the lifecycle-event recorder backend.
 	Metering MeteringConfig `json:"metering,omitzero" mapstructure:"metering"`
 }

extend/fs/fs.go

@@ -21,8 +21,7 @@ var (
 	// (cocoon-fs-<tag>) and safe for shell quoting and guest mount commands.
 	validTagRe = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_-]{0,35}$`)
 
-	// ErrUnsupportedBackend signals the resolved hypervisor backend cannot
-	// hot-plug vhost-user-fs (e.g. Firecracker).
+	// ErrUnsupportedBackend signals the backend cannot hot-plug vhost-user-fs (e.g. Firecracker).
 	ErrUnsupportedBackend = errors.New("backend does not support fs attach")
 )
 

extend/vfio/vfio.go

@@ -24,8 +24,7 @@ var (
 	// "cocoon-" is reserved so cocoon-derived ids never collide.
 	validIDRe = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_.-]{0,63}$`)
 
-	// ErrUnsupportedBackend signals the resolved hypervisor backend cannot
-	// hot-plug VFIO devices (e.g. Firecracker).
+	// ErrUnsupportedBackend signals the backend cannot hot-plug VFIO devices (e.g. Firecracker).
 	ErrUnsupportedBackend = errors.New("backend does not support device attach")
 )
 

gc/module.go

@@ -21,7 +21,7 @@ type Module[S any] struct {
 	Collect func(ctx context.Context, ids []string, snap S) error
 }
 
-// Module[S] implements runner — internal to the gc package.
+// Module[S] implements runner.
 func (m Module[S]) getName() string        { return m.Name }
 func (m Module[S]) getLocker() lock.Locker { return m.Locker }