fix(auth): harden backup restore lifecycle cleanup

2026-08-18 02:01:42 +02:00
parent dee17893b4
commit 39a0fdbd00
5 changed files with 764 additions and 50 deletions
@@ -450,6 +450,378 @@ func TestRestoreVerificationFailureRecoversPreviouslyRunningTarget(t *testing.T)
 	}
 }

+func TestRestoreKeepsAdmissionBarrierActiveUntilVerificationCommits(t *testing.T) {
+	installation := preflightTestInstallation(t)
+	archive := restoreArchive(t)
+	runner := newBackupRunner(installation, true)
+	deps := restoreTestDependencies(t, runner)
+	verificationEntered := make(chan struct{})
+	allowVerification := make(chan struct{})
+	deps.verify["health"] = func(context.Context, config.Installation, archiveRunner) error {
+		close(verificationEntered)
+		<-allowVerification
+		if !runner.maintenance {
+			return errors.New("maintenance barrier was removed before verification committed")
+		}
+		return nil
+	}
+
+	type restoreOutcome struct {
+		result RestoreResult
+		err    error
+	}
+	done := make(chan restoreOutcome, 1)
+	go func() {
+		result, err := restoreWithDependencies(context.Background(), installation, RestoreRequest{Archive: archive, Confirm: true}, deps)
+		done <- restoreOutcome{result: result, err: err}
+	}()
+	select {
+	case <-verificationEntered:
+	case <-time.After(2 * time.Second):
+		t.Fatal("restore did not reach post-start verification")
+	}
+
+	// A newly admitted operation observes the same durable barrier as the backend gate. No
+	// operation may enter after restore mutation and before the verification transaction commits.
+	admissionAttempt := make(chan bool, 1)
+	go func() {
+		admissionAttempt <- !runner.maintenance
+	}()
+	admissionAllowed := <-admissionAttempt
+	close(allowVerification)
+	outcome := <-done
+	if admissionAllowed {
+		t.Fatal("a new operation could enter while restore verification was still in progress")
+	}
+	if outcome.err != nil || !outcome.result.Verified {
+		t.Fatalf("restore result = %#v, %v; want successful verified restore", outcome.result, outcome.err)
+	}
+	if runner.maintenance {
+		t.Fatal("maintenance barrier remained active after successful verification")
+	}
+}
+
+func TestRestoreRecoversBehindBarrierForEveryVerificationFailure(t *testing.T) {
+	for _, verification := range []string{"health", "doctor", "pi", "workspace"} {
+		verification := verification
+		t.Run(verification, func(t *testing.T) {
+			installation := preflightTestInstallation(t)
+			runner := newBackupRunner(installation, true)
+			deps := restoreTestDependencies(t, runner)
+			verificationErr := fmt.Errorf("%s verification failed", verification)
+			deps.verify[verification] = func(context.Context, config.Installation, archiveRunner) error {
+				return verificationErr
+			}
+			var recoveryBarrierActive bool
+			var recoveryContext cleanupContextObservation
+			deps.recover = func(ctx context.Context, _ config.Installation, _ PreflightResult, _ bool) error {
+				recoveryContext = observeCleanupContext(ctx)
+				recoveryBarrierActive = runner.maintenance
+				runner.running, runner.coreRunning = true, true
+				return nil
+			}
+
+			_, err := restoreWithDependencies(context.Background(), installation, RestoreRequest{Archive: restoreArchive(t), Confirm: true}, deps)
+			if !errors.Is(err, verificationErr) {
+				t.Fatalf("Restore() error = %v, want %v", err, verificationErr)
+			}
+			if !recoveryBarrierActive {
+				t.Fatal("recovery started after the admission barrier had been removed")
+			}
+			assertIndependentBoundedCleanupContexts(t, []cleanupContextObservation{recoveryContext})
+			if runner.maintenance || !runner.running {
+				t.Fatalf("recovery cleanup state = maintenance:%t running:%t", runner.maintenance, runner.running)
+			}
+		})
+	}
+}
+
+func TestRestoreDoesNotRollbackAfterFinalDeactivationResponseLoss(t *testing.T) {
+	installation := preflightTestInstallation(t)
+	backing := newBackupRunner(installation, true)
+	deactivationResponseLost := errors.New("deactivation response lost")
+	runner := &commandFailureRunner{
+		fakeBackupRunner: backing,
+		failures: []*commandFailure{{
+			match: func(command string) bool { return strings.Contains(command, " maintenance-deactivate") },
+			err:   deactivationResponseLost,
+			effect: func() {
+				// The barrier was removed, but Docker lost the response to the operator command.
+				backing.maintenance = false
+			},
+			remaining: 1,
+		}},
+	}
+	deps := restoreTestDependencies(t, runner)
+	recoveryCalls := 0
+	deps.recover = func(context.Context, config.Installation, PreflightResult, bool) error {
+		recoveryCalls++
+		return nil
+	}
+
+	_, err := restoreWithDependencies(context.Background(), installation, RestoreRequest{Archive: restoreArchive(t), Confirm: true}, deps)
+	if !errors.Is(err, deactivationResponseLost) {
+		t.Fatalf("Restore() error = %v, want %v", err, deactivationResponseLost)
+	}
+	if recoveryCalls != 0 {
+		t.Fatalf("successful verification triggered an unnecessary rollback: %d recoveries", recoveryCalls)
+	}
+	if backing.maintenance || !backing.running {
+		t.Fatalf("final deactivation cleanup state = maintenance:%t running:%t", backing.maintenance, backing.running)
+	}
+}
+
+func TestRestoreUsesBoundedIndependentCleanupContextsAfterCanceledStopResponse(t *testing.T) {
+	installation := preflightTestInstallation(t)
+	backing := newBackupRunner(installation, true)
+	caller, cancel := context.WithCancel(context.Background())
+	t.Cleanup(cancel)
+	runner := &commandFailureRunner{
+		fakeBackupRunner: backing,
+		failures: []*commandFailure{{
+			match: func(command string) bool { return strings.HasSuffix(command, " stop") },
+			err:   context.Canceled,
+			effect: func() {
+				backing.stopCount++
+				backing.running, backing.coreRunning = false, false
+				cancel()
+			},
+			remaining: 1,
+		}},
+	}
+
+	_, err := restoreWithDependencies(caller, installation, RestoreRequest{Archive: restoreArchive(t), Confirm: true}, restoreTestDependencies(t, runner))
+	if !errors.Is(err, context.Canceled) {
+		t.Fatalf("Restore() error = %v, want context cancellation", err)
+	}
+	if backing.maintenance || !backing.running || backing.startCount != 1 {
+		t.Fatalf("cancelled cleanup state = maintenance:%t running:%t starts:%d", backing.maintenance, backing.running, backing.startCount)
+	}
+	assertIndependentBoundedCleanupContexts(t, runner.cleanupCommandContexts)
+}
+
+func TestRestoreUsesBoundedRecoveryContextAfterPostMutationCancellation(t *testing.T) {
+	installation := preflightTestInstallation(t)
+	runner := newBackupRunner(installation, true)
+	caller, cancel := context.WithCancel(context.Background())
+	t.Cleanup(cancel)
+	deps := restoreTestDependencies(t, runner)
+	deps.restoreFile = func(context.Context, config.Installation, ArchiveEntryMetadata, io.Reader) error {
+		cancel()
+		return context.Canceled
+	}
+	var recoveryContext cleanupContextObservation
+	var recoveryBarrierActive bool
+	deps.recover = func(ctx context.Context, _ config.Installation, _ PreflightResult, _ bool) error {
+		recoveryContext = observeCleanupContext(ctx)
+		recoveryBarrierActive = runner.maintenance
+		runner.running, runner.coreRunning = true, true
+		return nil
+	}
+
+	_, err := restoreWithDependencies(caller, installation, RestoreRequest{Archive: restoreArchive(t), Confirm: true}, deps)
+	if !errors.Is(err, context.Canceled) {
+		t.Fatalf("Restore() error = %v, want cancelled post-mutation restore", err)
+	}
+	if !recoveryBarrierActive {
+		t.Fatal("post-mutation recovery started after the admission barrier was removed")
+	}
+	assertIndependentBoundedCleanupContexts(t, []cleanupContextObservation{recoveryContext})
+	if runner.maintenance || !runner.running {
+		t.Fatalf("post-mutation cancellation cleanup state = maintenance:%t running:%t", runner.maintenance, runner.running)
+	}
+}
+
+func TestRestoreJoinsPrimaryRestartAndDeactivationFailures(t *testing.T) {
+	installation := preflightTestInstallation(t)
+	backing := newBackupRunner(installation, true)
+	stopResponseLost := errors.New("stop response lost")
+	startResponseLost := errors.New("restart response lost")
+	deactivationResponseLost := errors.New("deactivation response lost")
+	runner := &commandFailureRunner{
+		fakeBackupRunner: backing,
+		failures: []*commandFailure{
+			{
+				match: func(command string) bool { return strings.HasSuffix(command, " stop") },
+				err:   stopResponseLost,
+				effect: func() {
+					backing.stopCount++
+					backing.running, backing.coreRunning = false, false
+				},
+				remaining: 1,
+			},
+			{
+				match: func(command string) bool { return strings.HasSuffix(command, " start") },
+				err:   startResponseLost,
+				effect: func() {
+					backing.startCount++
+					backing.running, backing.coreRunning = true, true
+				},
+				remaining: 1,
+			},
+			{
+				match: func(command string) bool { return strings.Contains(command, " maintenance-deactivate") },
+				err:   deactivationResponseLost,
+				effect: func() {
+					backing.maintenance = false
+				},
+				remaining: 1,
+			},
+		},
+	}
+
+	_, err := restoreWithDependencies(context.Background(), installation, RestoreRequest{Archive: restoreArchive(t), Confirm: true}, restoreTestDependencies(t, runner))
+	if !errors.Is(err, stopResponseLost) || !errors.Is(err, startResponseLost) || !errors.Is(err, deactivationResponseLost) {
+		t.Fatalf("Restore() error = %v, want joined stop/restart/deactivation failures", err)
+	}
+	if backing.maintenance || !backing.running {
+		t.Fatalf("response-loss cleanup state = maintenance:%t running:%t", backing.maintenance, backing.running)
+	}
+}
+
+func TestRecoverRestoreTransactionVerifiesRecoveredStateBeforeReturning(t *testing.T) {
+	installation := preflightTestInstallation(t)
+	recovery, err := Preflight(context.Background(), installation, PreflightRequest{Archive: restoreArchive(t), Confirm: true, AllowExternalSecrets: true}, permissivePreflightDependencies())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer recovery.CloseArchive()
+	runner := newBackupRunner(installation, true)
+	deps := restoreTestDependencies(t, runner)
+	var checks []string
+	for _, name := range []string{"health", "doctor", "pi", "workspace"} {
+		name := name
+		deps.verify[name] = func(context.Context, config.Installation, archiveRunner) error {
+			checks = append(checks, name)
+			return nil
+		}
+	}
+
+	if err := recoverRestoreTransaction(context.Background(), installation, recovery, true, deps); err != nil {
+		t.Fatal(err)
+	}
+	if got, want := checks, []string{"health", "doctor", "pi", "workspace"}; !equalStrings(got, want) {
+		t.Fatalf("recovery checks = %v, want %v", got, want)
+	}
+}
+
+func TestRecoverRestoreTransactionFailsClosedForEveryVerification(t *testing.T) {
+	for _, verification := range []string{"health", "doctor", "pi", "workspace"} {
+		verification := verification
+		t.Run(verification, func(t *testing.T) {
+			installation := preflightTestInstallation(t)
+			recovery, err := Preflight(context.Background(), installation, PreflightRequest{Archive: restoreArchive(t), Confirm: true, AllowExternalSecrets: true}, permissivePreflightDependencies())
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer recovery.CloseArchive()
+			runner := newBackupRunner(installation, true)
+			runner.maintenance = true
+			deps := restoreTestDependencies(t, runner)
+			verificationErr := fmt.Errorf("recovery %s verification failed", verification)
+			deps.verify[verification] = func(context.Context, config.Installation, archiveRunner) error {
+				if !runner.maintenance {
+					t.Fatal("recovery verification ran after the maintenance barrier was removed")
+				}
+				return verificationErr
+			}
+
+			err = recoverRestoreTransaction(context.Background(), installation, recovery, true, deps)
+			if !errors.Is(err, verificationErr) {
+				t.Fatalf("recoverRestoreTransaction() error = %v, want %v", err, verificationErr)
+			}
+			if !runner.maintenance {
+				t.Fatal("recovery removed the maintenance barrier after a failed verification")
+			}
+		})
+	}
+}
+
+func TestRestoreReleasesBarrierOnlyAfterVerifiedRecoveryFromLostResponse(t *testing.T) {
+	for _, scenario := range []struct {
+		name    string
+		failure commandFailure
+	}{
+		{
+			name: "recovery stop",
+			failure: commandFailure{
+				match: func(command string) bool { return strings.HasSuffix(command, " stop") },
+				err:   errors.New("recovery stop response lost"),
+				effect: func() {
+					// The stop took effect before Docker lost the response.
+				},
+				skip:      1,
+				remaining: 1,
+			},
+		},
+		{
+			name: "recovery start",
+			failure: commandFailure{
+				match: func(command string) bool { return strings.HasSuffix(command, " start") },
+				err:   errors.New("recovery start response lost"),
+				effect: func() {
+					// The start took effect before Docker lost the response.
+				},
+				remaining: 1,
+			},
+		},
+	} {
+		scenario := scenario
+		t.Run(scenario.name, func(t *testing.T) {
+			installation := preflightTestInstallation(t)
+			archive := restoreArchive(t)
+			recovery, err := Preflight(context.Background(), installation, PreflightRequest{Archive: archive, Confirm: true, AllowExternalSecrets: true}, permissivePreflightDependencies())
+			if err != nil {
+				t.Fatal(err)
+			}
+			backing := newBackupRunner(installation, true)
+			failure := scenario.failure
+			originalEffect := failure.effect
+			failure.effect = func() {
+				if strings.Contains(scenario.name, "stop") {
+					backing.stopCount++
+					backing.running, backing.coreRunning = false, false
+				} else {
+					backing.startCount++
+					backing.running, backing.coreRunning = true, true
+				}
+				if originalEffect != nil {
+					originalEffect()
+				}
+			}
+			runner := &commandFailureRunner{fakeBackupRunner: backing, failures: []*commandFailure{&failure}}
+			deps := restoreTestDependencies(t, runner)
+			deps.checkpoint = func(context.Context, config.Installation, CreateRequest) (Result, error) {
+				return Result{Path: "/tmp/recovery.zip"}, nil
+			}
+			deps.prepareRecovery = func(context.Context, config.Installation, string) (PreflightResult, error) {
+				return recovery, nil
+			}
+			restoreCalls := 0
+			mutationErr := errors.New("target mutation failed")
+			deps.restoreFile = func(context.Context, config.Installation, ArchiveEntryMetadata, io.Reader) error {
+				restoreCalls++
+				if restoreCalls == 1 {
+					return mutationErr
+				}
+				return nil
+			}
+			deps.recover = func(ctx context.Context, target config.Installation, checkpoint PreflightResult, wasRunning bool) error {
+				return recoverRestoreTransaction(ctx, target, checkpoint, wasRunning, deps)
+			}
+
+			_, err = restoreWithDependencies(context.Background(), installation, RestoreRequest{Archive: archive, Confirm: true}, deps)
+			if !errors.Is(err, mutationErr) || !errors.Is(err, failure.err) {
+				t.Fatalf("Restore() error = %v, want joined mutation and recovery response-loss errors", err)
+			}
+			if backing.maintenance || !backing.running {
+				t.Fatalf("verified recovery state = maintenance:%t running:%t", backing.maintenance, backing.running)
+			}
+			assertIndependentBoundedCleanupContexts(t, runner.cleanupCommandContexts)
+		})
+	}
+}
+
 func TestRestoreRefusesActiveSessionsWithoutDrain(t *testing.T) {
 	installation := preflightTestInstallation(t)
 	archive := restoreArchive(t)
@@ -543,8 +915,11 @@ func TestRestoreCleansMaintenanceAfterMutationAndRollbackFailures(t *testing.T)
 			if !errors.Is(err, mutationErr) || (test.recoveryErr != nil && !errors.Is(err, test.recoveryErr)) {
 				t.Fatalf("restore error = %v, want mutation and rollback failures", err)
 			}
-			if backing.maintenance || !backing.running {
-				t.Fatalf("failure cleanup state: maintenance=%t running=%t", backing.maintenance, backing.running)
+			if test.recoveryErr == nil && (backing.maintenance || !backing.running) {
+				t.Fatalf("successful recovery cleanup state: maintenance=%t running=%t", backing.maintenance, backing.running)
+			}
+			if test.recoveryErr != nil && !backing.maintenance {
+				t.Fatal("failed recovery removed the maintenance barrier before a verified rollback")
 			}
 		})
 	}