fix(vm): stop leaking the VNC password; unify the CNI-VNC password gate

- VNCPass is launch-scoped and set from the flag on every start, but it
  was persisted to vm.json (json tag) and echoed by inspect/list — a
  plaintext password at rest. Mark it json:"-".
- setVNCPassword embedded the full HMP dialogue in its error, which
  echoes the typed 'set_password vnc <pw>' line to stderr/CI logs.
  Return a static error instead.
- startVNCProxy left an orphan proxy holding the host port if WritePIDFile
  failed after the child forked; kill it on that path.
- The mandatory-password gate was hand-duplicated at create/clone/launch;
  extract requireCNIVNCPassword and add a regression test.

Author: CMGS

cmd/vm/clone.go

@@ -30,8 +30,8 @@ func (h *Handler) Clone(cmd *cobra.Command, args []string) error {
 	netMode, _ := cmd.Flags().GetString("net")
 	vnc, _ := cmd.Flags().GetInt("vnc")
 	vncPass, _ := cmd.Flags().GetString("vnc-password")
-	if netMode == netCNI && vnc >= 0 && vncPass == "" { // same pre-scaffold check as create
-		return errCNIVNCPassRequired
+	if err = requireCNIVNCPassword(netMode == netCNI, vnc, vncPass); err != nil { // same pre-scaffold check as create
+		return err
 	}
 	// the clone inherits SRC's data disks by name; extra --data-disk specs must not collide with them
 	// and the combined count still honors the AHCI cap. Parse before scaffolding to fail fast.

cmd/vm/handler.go

@@ -37,7 +37,7 @@ type record struct {
 	CPUs      int      `json:"cpus"`
 	Memory    string   `json:"memory"`
 	VNCDisp   int      `json:"vnc"`
-	VNCPass   string   `json:"vnc_password,omitempty"`
+	VNCPass   string   `json:"-"` // launch-scoped, set from the flag each start; never persisted (would leak at rest)
 	SSHPort   int      `json:"ssh_port"`
 	NetMode   string   `json:"net_mode,omitempty"`
 	Hugepages bool     `json:"hugepages,omitempty"`

cmd/vm/helper.go

@@ -227,7 +227,8 @@ func setVNCPassword(ctx context.Context, monSock, pw string) error {
 	_ = conn.SetReadDeadline(time.Now().Add(5 * time.Second))
 	out, _ := readUntil(conn, "(qemu)")
 	if strings.Contains(out, "Could not") {
-		return fmt.Errorf("qemu: %s", strings.TrimSpace(out))
+		// out echoes the typed "set_password vnc <pw>" line — never surface it.
+		return errors.New("qemu rejected set_password (vnc display not active?)")
 	}
 	return nil
 }

cmd/vm/lifecycle.go

@@ -142,8 +142,8 @@ func (h *Handler) create(cmd *cobra.Command, image string) (*record, error) {
 	vnc, _ := cmd.Flags().GetInt("vnc")
 	vncPass, _ := cmd.Flags().GetString("vnc-password")
 	netMode, _ := cmd.Flags().GetString("net")
-	if netMode == netCNI && vnc >= 0 && vncPass == "" { // fail before scaffolding leaves a half-made VM
-		return nil, errCNIVNCPassRequired
+	if err = requireCNIVNCPassword(netMode == netCNI, vnc, vncPass); err != nil { // fail before scaffolding leaves a half-made VM
+		return nil, err
 	}
 	oc, code, varsTmpl, err := resolveFirmware(cmd)
 	if err != nil {
@@ -182,8 +182,8 @@ func (h *Handler) create(cmd *cobra.Command, image string) (*record, error) {
 func (h *Handler) launch(cmd *cobra.Command, dir string, r *record) error {
 	ctx := cliutil.CommandContext(cmd)
 	logger := log.WithFunc("cmd.vm.launch")
-	if r.Netns != "" && r.VNCDisp >= 0 && r.VNCPass == "" {
-		return errCNIVNCPassRequired
+	if err := requireCNIVNCPassword(r.Netns != "", r.VNCDisp, r.VNCPass); err != nil {
+		return err
 	}
 	if hostIsAMD() {
 		// macOS reads MSRs an AMD host lacks; without kvm.ignore_msrs KVM injects #GP. Best-effort,

cmd/vm/vnc.go

@@ -30,6 +30,16 @@ const (
 // never expose that unauthenticated.
 var errCNIVNCPassRequired = errors.New("--vnc with --net cni serves VNC on a host port reachable off-box; --vnc-password is required")
 
+// requireCNIVNCPassword rejects an unauthenticated VNC display on a CNI VM whose
+// host port is reachable off-box. isCNI is the flag intent at create/clone
+// (pre-scaffold) or the resolved Netns at launch (post-scaffold).
+func requireCNIVNCPassword(isCNI bool, vncDisp int, vncPass string) error {
+	if isCNI && vncDisp >= 0 && vncPass == "" {
+		return errCNIVNCPassRequired
+	}
+	return nil
+}
+
 // vncProxyCommand is the hidden re-exec target that runs the forwarder for its lifetime, accepting
 // on the TCP listener inherited as fd 3 (bound by the parent so bind errors fail the launch).
 func vncProxyCommand() *cobra.Command {
@@ -72,7 +82,11 @@ func startVNCProxy(ctx context.Context, dir string, disp int) error {
 	if err := c.Start(); err != nil {
 		return err
 	}
-	return utils.WritePIDFile(filepath.Join(dir, vncProxyPID), c.Process.Pid)
+	if err := utils.WritePIDFile(filepath.Join(dir, vncProxyPID), c.Process.Pid); err != nil {
+		_ = c.Process.Kill() // no pidfile -> stopVNCProxy can't reap it; don't orphan the proxy
+		return err
+	}
+	return nil
 }
 
 // stopVNCProxy kills a running proxy (best-effort) and removes its pidfile. Zero grace: the CLI's

cmd/vm/vnc_test.go

@@ -0,0 +1,32 @@
+package vm
+
+import (
+	"errors"
+	"testing"
+)
+
+func TestRequireCNIVNCPassword(t *testing.T) {
+	tests := []struct {
+		name    string
+		isCNI   bool
+		vncDisp int
+		vncPass string
+		wantErr bool
+	}{
+		{"cni vnc no password rejected", true, 0, "", true},
+		{"cni vnc with password ok", true, 0, "s3cret", false},
+		{"cni vnc disabled ok", true, -1, "", false},
+		{"non-cni vnc no password ok", false, 0, "", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := requireCNIVNCPassword(tt.isCNI, tt.vncDisp, tt.vncPass)
+			if tt.wantErr && !errors.Is(err, errCNIVNCPassRequired) {
+				t.Errorf("got %v, want errCNIVNCPassRequired", err)
+			}
+			if !tt.wantErr && err != nil {
+				t.Errorf("got %v, want nil", err)
+			}
+		})
+	}
+}