review: whole-repo static audit, 16 fixes, cuts, docs against the code

.github/workflows/build-os-images.yml

@@ -5,7 +5,6 @@ on:
     branches: [master]
     paths:
       - "os-image/**"
-      - "!os-image/windows/**"
       - "!os-image/**/*.md"
   workflow_dispatch:
     inputs:
@@ -43,7 +42,6 @@ jobs:
             declare -A names
             while IFS= read -r dockerfile; do
               [[ -z "$dockerfile" ]] && continue
-              [[ -f "$(dirname "$dockerfile")/.skip-generic-build" ]] && continue
               name=$(echo "$dockerfile" | cut -d'/' -f2)
               names[$name]=1
             done <<< "$(find os-image -name Dockerfile -type f)"
@@ -55,7 +53,6 @@ jobs:
               declare -A names
               while IFS= read -r dockerfile; do
                 [[ -z "$dockerfile" ]] && continue
-                [[ -f "$(dirname "$dockerfile")/.skip-generic-build" ]] && continue
                 name=$(echo "$dockerfile" | cut -d'/' -f2)
                 names[$name]=1
               done <<< "$(find os-image -name Dockerfile -type f)"
@@ -65,7 +62,6 @@ jobs:
               declare -A names
               for f in $all_changed; do
                 tag_dir=$(echo "$f" | cut -d'/' -f1-3)
-                [[ -f "$tag_dir/.skip-generic-build" ]] && continue
                 name=$(echo "$f" | cut -d'/' -f2)
                 if [[ -n "$name" ]]; then
                   names[$name]=1
@@ -104,7 +100,6 @@ jobs:
             # Find all Dockerfiles for this name
             while IFS= read -r dockerfile; do
               [[ -z "$dockerfile" ]] && continue
-              [[ -f "$(dirname "$dockerfile")/.skip-generic-build" ]] && continue
               tag=$(echo "$dockerfile" | cut -d'/' -f3)
               [[ -n "$tag" ]] || continue
               tag_no_cache="$needs_no_cache"

.github/workflows/build.yml

@@ -7,6 +7,9 @@ on:
     paths-ignore:
       - 'os-image/**'
 
+permissions:
+  contents: read
+
 jobs:
   build:
     runs-on: ubuntu-latest

.github/workflows/lint.yml

@@ -12,6 +12,9 @@ on:
     paths-ignore:
       - 'os-image/**'
 
+permissions:
+  contents: read
+
 jobs:
   lint:
     runs-on: ubuntu-latest

.github/workflows/test.yml

@@ -12,6 +12,9 @@ on:
     paths-ignore:
       - 'os-image/**'
 
+permissions:
+  contents: read
+
 jobs:
   test:
     runs-on: ubuntu-latest

README.md

@@ -20,12 +20,12 @@ Lightweight MicroVM engine with dual hypervisor backends: [Cloud Hypervisor](htt
 ## Quick Start
 
 ```bash
-# Install the binary (Linux amd64; arm64: cocoon_0.7.6_Linux_arm64.tar.gz)
-curl -fsSL -o cocoon.tar.gz https://github.com/cocoonstack/cocoon/releases/download/v0.7.6/cocoon_0.7.6_Linux_x86_64.tar.gz
+# Install the binary (Linux amd64; arm64: cocoon_0.7.7_Linux_arm64.tar.gz)
+curl -fsSL -o cocoon.tar.gz https://github.com/cocoonstack/cocoon/releases/download/v0.7.7/cocoon_0.7.7_Linux_x86_64.tar.gz
 tar -xzf cocoon.tar.gz && install -m 0755 cocoon /usr/local/bin/
 
 # One-time environment setup (installs CH, firmware, CNI plugins)
-curl -fsSL -o cocoon-check https://raw.githubusercontent.com/cocoonstack/cocoon/refs/tags/v0.7.6/doctor/check.sh
+curl -fsSL -o cocoon-check https://raw.githubusercontent.com/cocoonstack/cocoon/refs/tags/v0.7.7/doctor/check.sh
 install -m 0755 cocoon-check /usr/local/bin/ && sudo cocoon-check --upgrade
 
 # Pull an image and run a VM

cgroup/cgroup.go

@@ -317,9 +317,6 @@ func placeScope(parentDir, dir, cpuset string) error {
 	if cpuset == "" {
 		return nil
 	}
-	if current, err := readControl(dir, cpusetName); err == nil && sameCPUList(current, cpuset) {
-		return nil
-	}
 	if err := checkSubset(cpuset, parentDir, "cpuset_cpus placement"); err != nil {
 		return err
 	}
@@ -384,7 +381,7 @@ func explainFenceWrite(parentDir, current, fence string, err error) error {
 	return fmt.Errorf("cgroup_cpus %q -> %q rejected with %d VM(s) running; the fence is machine-wide, drain them first: %w", current, fence, len(ids), err)
 }
 
-// enableControllers reads before writing: subtree_control writes take the kernel's hierarchy-wide cgroup_mutex, so steady-state launches must not contend on a no-op write. Missing controllers go in one combined write.
+// enableControllers reads before writing: subtree_control writes take the kernel's hierarchy-wide cgroup_mutex, so steady-state launches must not contend on a no-op write.
 func enableControllers(dir string, ctrls []string) error {
 	have, _ := readControl(dir, subtreeControlName)
 	enabled := strings.Fields(have)

cgroup/cgroup_test.go

@@ -278,20 +278,6 @@ func TestExplainFenceWriteNamesLiveVMs(t *testing.T) {
 	}
 }
 
-func TestPlaceScopeReadGate(t *testing.T) {
-	parent := t.TempDir()
-	dir := ScopeDir(parent, "X")
-	if err := os.Mkdir(dir, 0o755); err != nil {
-		t.Fatalf("setup: %v", err)
-	}
-	if err := os.WriteFile(filepath.Join(dir, "cpuset.cpus"), []byte("2-3\n"), 0o600); err != nil {
-		t.Fatalf("setup: %v", err)
-	}
-	if err := placeScope(parent, dir, "2,3"); err != nil {
-		t.Errorf("equal placement rewrote: %v", err)
-	}
-}
-
 func TestArmWritesQuotaAndBurst(t *testing.T) {
 	parent := t.TempDir()
 	dir := ScopeDir(parent, "A")

cmd/core/metastore.go

@@ -123,7 +123,7 @@ func MetaStore(conf *config.Config) (meta.Store, error) {
 		// Bootstrap owns its context: the store outlives any single caller, and a canceled first caller must not poison the Once for everyone.
 		ctx, cancel := context.WithTimeout(context.Background(), metaBootstrapTimeout)
 		defer cancel()
-		// Ordinary opens of EITHER engine refuse while a conversion is in flight (§6); the json engine cannot see the manifest itself.
+		// Ordinary opens of EITHER engine refuse while a conversion is in flight; the json engine cannot see the manifest itself.
 		dbPath := MetaDBPath(conf)
 		if err := metasqlite.RefuseManifest(dbPath); err != nil {
 			metaErr = err

cmd/core/utils.go

@@ -234,7 +234,6 @@ func resolveOwner[T typed](backends []T, ref string, found func(T) (bool, error)
 	}
 }
 
-// resolveVMOwner returns the owning hypervisor and the resolved VM.
 func resolveVMOwner(ctx context.Context, hypers []hypervisor.Hypervisor, ref string) (hypervisor.Hypervisor, *types.VM, error) {
 	var resolved *types.VM
 	owner, err := resolveOwner(

cmd/daemon/commands.go

@@ -19,6 +19,6 @@ func Command(h Handler) *cobra.Command {
 	cmd.Flags().Duration("gc-interval", 0, "sweep on this cadence in a background goroutine; 0 (default) leaves gc a CLI verb")
 	cmd.Flags().String("api-socket", "", "read-only API socket path (default: <run-dir>/cocoond.sock)")
 	cmd.Flags().Bool("no-api", false, "disable the read-only API")
-	cmd.Flags().Uint32("api-socket-mode", cocoond.DefaultAPISockMode, "read-only API socket permission bits")
+	cmd.Flags().Uint32("api-socket-mode", 0, "read-only API socket permission bits, octal with a leading 0 (default 0660)")
 	return cmd
 }

cmd/images/manage.go

@@ -68,18 +68,18 @@ func (h Handler) RM(cmd *cobra.Command, args []string) error {
 		refsByBackend[owner] = append(refsByBackend[owner], ref)
 	}
 
-	var allDeleted []string
+	total := 0
 	for backend, refs := range refsByBackend {
 		deleted, err := backend.Delete(ctx, refs)
 		if err != nil {
 			return fmt.Errorf("delete %s: %w", backend.Type(), err)
 		}
-		allDeleted = append(allDeleted, deleted...)
-	}
-	for _, ref := range allDeleted {
-		logger.Infof(ctx, "deleted: %s", ref)
+		for _, ref := range deleted {
+			logger.Infof(ctx, "deleted: %s", ref)
+		}
+		total += len(deleted)
 	}
-	if len(allDeleted) == 0 {
+	if total == 0 {
 		logger.Info(ctx, "no matching images found")
 	}
 	return nil

cmd/meta/convert/convert.go

@@ -1,4 +1,4 @@
-// Package convert runs the offline meta engine cutover (design §6).
+// Package convert runs the offline meta engine cutover.
 package convert
 
 import (
@@ -25,7 +25,7 @@ import (
 
 const asideSuffix = ".converted-"
 
-// testCrashStep injects crashes between protocol steps (§9 gate); nil in prod.
+// testCrashStep injects crashes between protocol steps; nil in prod.
 var testCrashStep func(step string) error
 
 // Spec carries both engines' declarations from the composition root.
@@ -121,7 +121,7 @@ func convertAll(ctx context.Context, spec Spec, target string, m *Manifest) erro
 		if m, err = newManifest(ctx, spec, target, src); err != nil {
 			return err
 		}
-		// The manifest is durable before the first target byte (§6).
+		// The manifest is durable before the first target byte.
 		if serr := saveManifest(spec.MetaRoot, m); serr != nil {
 			return serr
 		}
@@ -174,7 +174,7 @@ func newManifest(ctx context.Context, spec Spec, target string, src meta.Store)
 	return m, nil
 }
 
-// checkQuiesced refuses conversion while a live process holds the source (§6).
+// checkQuiesced refuses conversion while a live process holds the source.
 func checkQuiesced(ctx context.Context, spec Spec, target string, src meta.Store) error {
 	if target == config.MetaBackendJSON {
 		// sqlite source: an empty durable transaction fails ErrBusy while a writer is mid-flight.
@@ -238,7 +238,7 @@ func openTarget(ctx context.Context, spec Spec, target string) (meta.Store, erro
 }
 
 func convertNamespace(ctx context.Context, src, dst meta.Store, ns metasqlite.Namespace, rec *NSRecord, m *Manifest, spec Spec) error {
-	// A committed-but-unmarked target (crash window) is re-verified and claimed, never redone (§6).
+	// A committed-but-unmarked target (crash window) is re-verified and claimed, never redone.
 	dstDigest, dstCount, err := canonicalDigest(ctx, dst, ns)
 	if err != nil {
 		return err
@@ -268,7 +268,7 @@ func convertNamespace(ctx context.Context, src, dst meta.Store, ns metasqlite.Na
 	return crashStep("ns-done")
 }
 
-// duplicateGeneration writes the json target's .prev so imported data survives a torn main (§9).
+// duplicateGeneration writes the json target's .prev so imported data survives a torn main.
 func duplicateGeneration(spec Spec, nsName string) error {
 	jns, ok := findJSON(spec, nsName)
 	if !ok {
@@ -345,7 +345,7 @@ func copyNamespace(ctx context.Context, src, dst meta.Store, ns metasqlite.Names
 	})
 }
 
-// verifyNames checks every name entry points at an existing record (§6).
+// verifyNames checks every name entry points at an existing record.
 func verifyNames(ctx context.Context, s meta.Store, ns metasqlite.Namespace) error {
 	if !slices.Contains(ns.Tables, "names") {
 		return nil
@@ -368,7 +368,7 @@ func verifyNames(ctx context.Context, s meta.Store, ns metasqlite.Namespace) err
 	})
 }
 
-// canonicalDigest is the engine-neutral namespace fingerprint (§6): rows sorted by id per table, tables in declaration order; count is records only.
+// canonicalDigest is the engine-neutral namespace fingerprint: rows sorted by id per table, tables in declaration order; count is records only.
 func canonicalDigest(ctx context.Context, s meta.Store, ns metasqlite.Namespace) (string, int, error) {
 	h := sha256.New()
 	count := 0
@@ -431,7 +431,7 @@ func sourceFiles(spec Spec, target, nsName string) []string {
 		return []string{spec.DBPath}
 	}
 	if jns, ok := findJSON(spec, nsName); ok {
-		// Both json generations are authority (§6).
+		// Both json generations are authority.
 		return []string{jns.FilePath, jns.FilePath + ".prev"}
 	}
 	return nil

cmd/meta/handler.go

@@ -36,7 +36,7 @@ func (h Handler) InitStore(cmd *cobra.Command, _ []string) error {
 
 func (h Handler) Convert(cmd *cobra.Command, _ []string) error {
 	ctx, conf := h.Init(cmd)
-	// The configured backend is the sole target authority (§6): convert always moves the OTHER engine's data into the effective backend.
+	// The configured backend is the sole target authority: convert always moves the OTHER engine's data into the effective backend.
 	target := cmp.Or(conf.MetaBackend, config.MetaBackendSQLite)
 	dbPath := cmdcore.MetaDBPath(conf)
 	spec := convert.Spec{

cmd/others/commands.go

@@ -1,12 +1,7 @@
-// Package others defines the top-level gc, version, and completion commands.
+// Package others defines the top-level gc and version commands.
 package others
 
-import (
-	"fmt"
-	"os"
-
-	"github.com/spf13/cobra"
-)
+import "github.com/spf13/cobra"
 
 func Commands(h Handler) []*cobra.Command {
 	gcCmd := &cobra.Command{
@@ -28,26 +23,5 @@ func Commands(h Handler) []*cobra.Command {
 			Args:  cobra.NoArgs,
 			RunE:  h.Version,
 		},
-		{
-			Use:       "completion [bash|zsh|fish|powershell]",
-			Short:     "Generate shell completion script",
-			Args:      cobra.ExactArgs(1),
-			ValidArgs: []string{"bash", "zsh", "fish", "powershell"},
-			RunE: func(cmd *cobra.Command, args []string) error {
-				root := cmd.Root()
-				switch args[0] {
-				case "bash":
-					return root.GenBashCompletion(os.Stdout)
-				case "zsh":
-					return root.GenZshCompletion(os.Stdout)
-				case "fish":
-					return root.GenFishCompletion(os.Stdout, true)
-				case "powershell":
-					return root.GenPowerShellCompletionWithDesc(os.Stdout)
-				default:
-					return fmt.Errorf("unsupported shell: %s", args[0])
-				}
-			},
-		},
 	}
 }

cmd/root.go

@@ -115,7 +115,6 @@ func initConfig(ctx context.Context) error {
 		viper.SetConfigFile(cfgFile)
 	}
 	if err := viper.ReadInConfig(); err != nil {
-		// No config file is OK; a corrupt/unreadable one is not.
 		if _, ok := errors.AsType[viper.ConfigFileNotFoundError](err); !ok {
 			return fmt.Errorf("read config: %w", err)
 		}

cmd/snapshot/handler.go

@@ -216,7 +216,7 @@ func (h Handler) Import(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	var r io.Reader
+	var r *os.File
 	if len(args) > 0 {
 		f, openErr := os.Open(args[0])
 		if openErr != nil {
@@ -229,6 +229,7 @@ func (h Handler) Import(cmd *cobra.Command, args []string) error {
 		r = os.Stdin
 		logger.Info(ctx, "importing from stdin ...")
 	}
+	defer cmdcore.CloseOnCancel(ctx, r)()
 
 	snapID, err := snapBackend.Import(ctx, r, name, description)
 	if err != nil {

cmd/vm/commands.go

@@ -162,7 +162,7 @@ func Command(h Handler) *cobra.Command {
 	}
 	addVMFlags(debugCmd)
 	debugCmd.Flags().Int("max-cpu", 8, "max CPUs")
-	debugCmd.Flags().Int("balloon", 0, "balloon size in MB")
+	debugCmd.Flags().Int("balloon", 0, "balloon size in MB (0 = auto)")
 	debugCmd.Flags().String("cow", "", "COW disk path")
 	debugCmd.Flags().String("ch", "", "cloud-hypervisor binary path (default: ch_binary from the config)")
 
@@ -356,7 +356,7 @@ func addVMFlags(cmd *cobra.Command) {
 	cmd.Flags().String("memory", "1G", "memory size")
 	cmd.Flags().String("storage", "10G", "COW disk size")
 	cmd.Flags().Int("nics", 1, "number of network interfaces (0 = no network); OCI images up to v0.7.7 configure only the last NIC")
-	cmd.Flags().Int("queue-size", 0, "virtio-net ring depth per queue (0 = default 512; tradeoff: larger improves download throughput, smaller improves RPC latency)")
+	cmd.Flags().Int("queue-size", 0, "virtio-net ring depth per queue (0 = default 512; larger improves download throughput, smaller improves RPC latency; CH only, ignored by FC)")
 	cmd.Flags().Int("disk-queue-size", 0, "virtio-blk ring depth per device (0 = default 512; CH only, ignored by FC)")
 	cmd.Flags().Int("cpu-weight", 0, "cgroup cpu.weight, 1..10000 (0 = vCPU count)")
 	addCPUBandwidthFlags(cmd)

cmd/vm/lifecycle.go

@@ -108,8 +108,11 @@ func (h Handler) Console(cmd *cobra.Command, args []string) error {
 	fmt.Fprintf(os.Stderr, "Connected to %s (escape sequence: %s.)\r\n", ref, escapeDisplay)
 
 	if f, ok := conn.(*os.File); ok {
-		cleanup := console.HandleResize(inFd, f.Fd())
-		defer cleanup()
+		if rc, rcErr := f.SyscallConn(); rcErr == nil {
+			var ptyFd uintptr
+			_ = rc.Control(func(fd uintptr) { ptyFd = fd })
+			defer console.HandleResize(inFd, ptyFd)()
+		}
 	}
 
 	escapeKeys := []byte{escapeChar, '.'}

cmd/vm/logstream.go

@@ -64,12 +64,12 @@ func streamLog(ctx context.Context, path string, follow bool, tail int) error {
 			}
 			// Sig mismatch catches O_TRUNC re-opens.
 			newSig, _ := utils.FileHead(f, logHeadSigLen)
-			if !bytes.Equal(newSig, sig) {
+			if !bytes.HasPrefix(newSig, sig) {
 				if _, err := f.Seek(0, io.SeekStart); err != nil {
 					return fmt.Errorf("rewind log: %w", err)
 				}
-				sig = newSig
 			}
+			sig = newSig
 			if _, err := io.Copy(os.Stdout, f); err != nil {
 				return fmt.Errorf("read log: %w", err)
 			}
@@ -111,7 +111,6 @@ func seekToLastNLines(f *os.File, n int) error {
 	return err
 }
 
-// watchFile watches the parent dir because an atomic rename changes the inode.
 func watchFile(ctx context.Context, filePath string, debounce time.Duration) (<-chan struct{}, error) {
 	watcher, err := fsnotify.NewWatcher()
 	if err != nil {