diff --git a/internal/cli/seal.go b/internal/cli/seal.go index df9cacd..f27c214 100644 --- a/internal/cli/seal.go +++ b/internal/cli/seal.go @@ -93,8 +93,10 @@ func runSealCheck(ctx context.Context, p *ui.Printer, opts cluster.KubeconfigOpt hooks, herr := listTestHooksFn(ctx, tt) if herr != nil { // Ctrl-C while enumerating is a cancelled run, not an enumeration - // failure — exit quietly, matching the per-check loop below (Bugbot). - if ctx.Err() != nil { + // failure — exit quietly, matching the per-check loop below. Reuse + // installerRunInterrupted so a terminal Ctrl-C where helm exits 130 before + // NotifyContext flips ctx.Err() is caught here too (Bugbot #397). + if installerRunInterrupted(ctx, herr) { return &exitError{code: exitInterrupted} } // Can't even enumerate the checks — that's an error, not a verdict: @@ -123,7 +125,12 @@ func runSealCheck(ctx context.Context, p *ui.Printer, opts cluster.KubeconfigOpt // Ctrl-C (or a parent deadline) mid-suite is a cancelled run, not a // verdict: every remaining check would "fail" on the dead context and // render a fake Unsealed. Exit quietly, the way `status --wait` does. - if ctx.Err() != nil { + // Reuse installerRunInterrupted (shared with upgrade/prepare-host): it + // also catches the race where a terminal Ctrl-C makes the helm child exit + // 130 BEFORE NotifyContext flips ctx.Err() — otherwise this check reads as + // a real failure and prints a false Unsealed (exit 2), breaking the "never + // claim a verdict you didn't verify" rule (Bugbot #397). + if installerRunInterrupted(ctx, terr) { return &exitError{code: exitInterrupted} } check := sealCheck{name: name, passed: terr == nil} diff --git a/internal/cli/seal_test.go b/internal/cli/seal_test.go index 308dbed..f486097 100644 --- a/internal/cli/seal_test.go +++ b/internal/cli/seal_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "os/exec" "strings" "testing" "time" @@ -385,6 +386,51 @@ func TestSeal_CancelledMidSuite_NoVerdict(t *testing.T) { } } +// The Ctrl-C race the ctx.Err()-only check missed: on a terminal Ctrl-C the helm +// child can exit 130 (128+SIGINT) BEFORE NotifyContext flips ctx.Err(). The +// cancel site must still treat that as a quiet interrupt (exit 130, no verdict) +// and never let the failed check render a false Unsealed. It reuses the same +// installerRunInterrupted helper as upgrade/prepare-host (Bugbot #397). +func TestSeal_CtrlCExit130BeforeCtxFlips_NoFalseUnsealed(t *testing.T) { + if _, err := exec.LookPath("bash"); err != nil { + t.Skip("bash not available to synthesize an exit-130 ExitError") + } + // A real *exec.ExitError with code 130 — exactly what helm.Runner surfaces + // (CombinedOutput returns the exec error unwrapped) when the child dies on SIGINT. + exit130 := exec.Command("bash", "-c", "exit 130").Run() + + stubSealTarget(t) + origList, origRun := listTestHooksFn, runHelmTestFn + listTestHooksFn = func(_ context.Context, _ helm.TestTarget) ([]helm.TestHook, error) { + return sealHooks(), nil + } + ran := 0 + runHelmTestFn = func(_ context.Context, _ helm.TestTarget, _ []string, _ time.Duration) (string, error) { + ran++ + // ctx is deliberately NOT cancelled — this is the race window where the + // child already exited 130 but NotifyContext hasn't flipped ctx.Err() yet. + return "", exit130 + } + t.Cleanup(func() { listTestHooksFn, runHelmTestFn = origList, origRun }) + + var out bytes.Buffer + err := runSealCheck(context.Background(), ui.New(&out), cluster.KubeconfigOptions{}, 0) + if got := ExitCodeFromError(err); got != exitInterrupted { + t.Fatalf("exit code = %d, want %d — a Ctrl-C (helm exit 130) must be a quiet interrupt, not a verdict: %v", got, exitInterrupted, err) + } + if !IsSilentError(err) { + t.Fatalf("Ctrl-C must exit quietly, got: %v", err) + } + if ran != 1 { + t.Errorf("no further checks may run after the interrupt, ran %d", ran) + } + for _, verdict := range []string{"Sealed", "Unsealed", "Seal unknown"} { + if strings.Contains(out.String(), verdict) { + t.Errorf("no verdict (esp. a false Unsealed) may render on a Ctrl-C run; got %q in:\n%s", verdict, out.String()) + } + } +} + // helmFailureDetail distills helm's combined output into the one-line reason: // the specific hook bullet first, then the Error: line, then the raw error — // and never an empty string for a failure.