diff --git a/internal/localapi/escrow_ceremony.go b/internal/localapi/escrow_ceremony.go index 47926ad..87ef68c 100644 --- a/internal/localapi/escrow_ceremony.go +++ b/internal/localapi/escrow_ceremony.go @@ -226,6 +226,9 @@ func (s *Server) handleEscrowCeremonyStart(w http.ResponseWriter, r *http.Reques if base == nil { base = context.Background() } + // The reply's fields are read HERE, before the job goroutine may write job.Phase (a data race the race + // detector caught, 2026-10-13 — the reply could show the finished phase or a torn value). + jobID, phase := job.JobID, job.Phase done := make(chan struct{}) go func() { defer close(done) @@ -274,7 +277,7 @@ func (s *Server) handleEscrowCeremonyStart(w http.ResponseWriter, r *http.Reques "claim_ttl_s", int(escrowClaimTTL.Seconds()), "duration_ms", time.Since(start).Milliseconds()) }() s.escrowDone = done // tests wait on it; production polls the status endpoint - writeStatus(w, http.StatusAccepted, true, map[string]any{"vmid": vmid, "job_id": job.JobID, "phase": job.Phase}, "") + writeStatus(w, http.StatusAccepted, true, map[string]any{"vmid": vmid, "job_id": jobID, "phase": phase}, "") } // handleEscrowCeremonyStatus is GET /escrow/ceremony/status: phase + the NON-SECRET summary. diff --git a/internal/localapi/hostpoweroff.go b/internal/localapi/hostpoweroff.go index 30bde6f..d2be5e8 100644 --- a/internal/localapi/hostpoweroff.go +++ b/internal/localapi/hostpoweroff.go @@ -209,18 +209,8 @@ func (s *Server) powerOffBlocker(ctx context.Context, vmid int) (string, string) if s.inFlight == nil || s.staleLock == nil { return PowerOffUnknown, "the heavy-operation gate or the Proxmox task reader is not wired" } - running, err := c.Running() - if err != nil { - return PowerOffUnknown, "cannot list the running guests: " + err.Error() - } - var others []string - for _, v := range running { - if v != vmid { - others = append(others, strconv.Itoa(v)) - } - } - if len(others) > 0 { - return PowerOffOtherGuests, "other guests or VMs running: " + strings.Join(others, ", ") + if code, detail := s.otherGuestsBlocker(vmid); code != "" { + return code, detail } if busy := s.inFlight.Busy(); busy != "" { switch { @@ -324,6 +314,24 @@ func runningGuestsFromProc(procDir string) ([]int, error) { return out, nil } +// otherGuestsBlocker refuses while any guest or VM other than the caller's runs (or the list cannot be read). +func (s *Server) otherGuestsBlocker(vmid int) (string, string) { + running, err := s.powerOff.Running() + if err != nil { + return PowerOffUnknown, "cannot list the running guests: " + err.Error() + } + var others []string + for _, v := range running { + if v != vmid { + others = append(others, strconv.Itoa(v)) + } + } + if len(others) > 0 { + return PowerOffOtherGuests, "other guests or VMs running: " + strings.Join(others, ", ") + } + return "", "" +} + // waitJobsClear waits until jobBlocker finds nothing, at most JobWait. false = still busy (or unreadable) at the end: // the power-off is CANCELLED — the box stays on, the gate is released, the log says why. Better a box that did not // switch off than a backup, update or format cut in the middle. @@ -332,6 +340,12 @@ func (s *Server) waitJobsClear(base context.Context, vmid int, when string) bool ctx, cancel := context.WithTimeout(base, c.JobWait) defer cancel() for { + // Another guest or VM started after acceptance (security review 2026-10-13): no waiting — cancel at once. + if code, detail := s.otherGuestsBlocker(vmid); code != "" { + s.logger.Error("host-poweroff: CANCELLED — another guest is running now; the box stays on", "when", when, + "reason", code, "detail", detail) + return false + } code, detail := s.jobBlocker(ctx, vmid) if code == "" { return true diff --git a/internal/localapi/hostpoweroff_test.go b/internal/localapi/hostpoweroff_test.go index cae5b99..316a25c 100644 --- a/internal/localapi/hostpoweroff_test.go +++ b/internal/localapi/hostpoweroff_test.go @@ -328,3 +328,26 @@ func TestHostPowerOff_JobStartedAfterAcceptance(t *testing.T) { t.Fatalf("the heavy gate is still held after a cancel: %q", busy) } } + +// A guest or VM started AFTER acceptance cancels the power-off at once (security review 2026-10-13). +func TestHostPowerOff_OtherGuestStartedAfterAcceptance(t *testing.T) { + r := newPowerRig(t, time.Hour) + var started atomic.Bool + r.srv.powerOff.Running = func() ([]int, error) { + if started.Load() { + return []int{8200, 341}, nil + } + return []int{8200}, nil + } + r.srv.powerOff.ReportNow = func(context.Context) bool { started.Store(true); return true } // a VM starts now + if w := do(t, r.h, "POST", "/host/poweroff", "A", ""); w.Code != http.StatusAccepted { + t.Fatalf("accept: %d %s", w.Code, w.Body.String()) + } + time.Sleep(50 * time.Millisecond) + if s := r.power.got(); len(s) != 0 { + t.Fatalf("power-off ran although a VM started: %v", s) + } + if busy := r.srv.inFlight.Busy(); busy != "" { + t.Fatalf("the heavy gate is still held after a cancel: %q", busy) + } +}