Authorize comparison replacement at commit time

internal/comparison/publish.go

@@ -51,9 +51,17 @@ type DestinationPlan struct {
 	WorkingDirectory string
 	Target           string
 	Replace          bool
-	Exists           bool
+	state            destinationState
 }
 
+type destinationState uint8
+
+const (
+	destinationAbsent destinationState = iota
+	destinationEmpty
+	destinationBundle
+)
+
 // ErrUnrecognizedBundle marks a directory that is not a valid current-schema
 // Weatherreporter comparison bundle.
 var ErrUnrecognizedBundle = errors.New("unrecognized comparison bundle")
@@ -98,7 +106,7 @@ func PlanDestination(workingDirectory, target string, replace bool) (Destination
 		return DestinationPlan{}, newDestinationError(DestinationInspection, target, err)
 	}
 	if len(entries) == 0 {
-		plan.Exists = true
+		plan.state = destinationEmpty
 		return plan, nil
 	}
 	if !replace {
@@ -107,7 +115,7 @@ func PlanDestination(workingDirectory, target string, replace bool) (Destination
 	if _, err := RecognizeBundle(target); err != nil {
 		return DestinationPlan{}, newDestinationError(DestinationUnrecognized, target, err)
 	}
-	plan.Exists = true
+	plan.state = destinationBundle
 	return plan, nil
 }
 
@@ -312,7 +320,7 @@ func publish(ctx context.Context, plan DestinationPlan, bundle LogicalBundle, op
 	if err := ctx.Err(); err != nil {
 		return err
 	}
-	if !currentPlan.Exists {
+	if currentPlan.state == destinationAbsent {
 		if err := operations.rename(temporaryDirectory, currentPlan.Target); err != nil {
 			return fmt.Errorf("publish comparison bundle to %q: %w", currentPlan.Target, err)
 		}
@@ -327,15 +335,11 @@ func publish(ctx context.Context, plan DestinationPlan, bundle LogicalBundle, op
 	if err := operations.rename(currentPlan.Target, backupDirectory); err != nil {
 		return fmt.Errorf("back up comparison destination %q: %w", currentPlan.Target, err)
 	}
+	if err := authorizeMovedDestination(currentPlan, backupDirectory); err != nil {
+		return restoreMovedDestination(operations, backupDirectory, currentPlan.Target, err)
+	}
 	if err := operations.rename(temporaryDirectory, currentPlan.Target); err != nil {
-		restoreErr := operations.rename(backupDirectory, currentPlan.Target)
+		return restoreMovedDestination(operations, backupDirectory, currentPlan.Target, fmt.Errorf("replace comparison destination %q: %w", currentPlan.Target, err))
-		if restoreErr != nil {
-			return errors.Join(
-				fmt.Errorf("replace comparison destination %q: %w", currentPlan.Target, err),
-				fmt.Errorf("restore prior comparison destination from %q: %w", backupDirectory, restoreErr),
-			)
-		}
-		return fmt.Errorf("replace comparison destination %q: %w", currentPlan.Target, err)
 	}
 	temporaryDirectory = ""
 	if err := os.RemoveAll(backupDirectory); err != nil {
@@ -344,6 +348,38 @@ func publish(ctx context.Context, plan DestinationPlan, bundle LogicalBundle, op
 	return nil
 }
 
+func authorizeMovedDestination(plan DestinationPlan, backupDirectory string) error {
+	backupPlan, err := PlanDestination(plan.WorkingDirectory, backupDirectory, plan.Replace)
+	if err != nil {
+		return fmt.Errorf("authorize moved comparison destination %q: %w", backupDirectory, err)
+	}
+	if backupPlan.state != destinationEmpty && backupPlan.state != destinationBundle {
+		return fmt.Errorf("authorize moved comparison destination %q: destination disappeared", backupDirectory)
+	}
+	return nil
+}
+
+func restoreMovedDestination(operations publishOperations, backupDirectory, target string, cause error) error {
+	if _, err := os.Lstat(target); err == nil {
+		return errors.Join(
+			cause,
+			fmt.Errorf("restore prior comparison destination from %q: destination %q reappeared", backupDirectory, target),
+		)
+	} else if !errors.Is(err, os.ErrNotExist) {
+		return errors.Join(
+			cause,
+			fmt.Errorf("inspect comparison destination %q before restoring from %q: %w", target, backupDirectory, err),
+		)
+	}
+	if restoreErr := operations.rename(backupDirectory, target); restoreErr != nil {
+		return errors.Join(
+			cause,
+			fmt.Errorf("restore prior comparison destination from %q: %w", backupDirectory, restoreErr),
+		)
+	}
+	return cause
+}
+
 func writeLogicalBundle(ctx context.Context, directory string, bundle LogicalBundle, manifestData []byte) error {
 	if err := writeBundleFile(ctx, filepath.Join(directory, DataPackageFilename), bundle.DataPackage); err != nil {
 		return err

internal/comparison/publish_test.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 )
@@ -17,7 +18,7 @@ func TestPlanDestination(t *testing.T) {
 	if err != nil {
 		t.Fatalf("PlanDestination() error = %v", err)
 	}
-	if plan.Target != absent || plan.Exists {
+	if plan.Target != absent || plan.state != destinationAbsent {
 		t.Fatalf("PlanDestination() = %#v, want absent target", plan)
 	}
 	nested := filepath.Join(workingDirectory, "missing-parent", "comparison")
@@ -33,7 +34,7 @@ func TestPlanDestination(t *testing.T) {
 		t.Fatal(err)
 	}
 	plan, err = PlanDestination(workingDirectory, empty, false)
-	if err != nil || !plan.Exists {
+	if err != nil || plan.state != destinationEmpty {
 		t.Fatalf("PlanDestination(empty) = %#v, %v", plan, err)
 	}
 
@@ -176,11 +177,235 @@ func TestPlanDestinationAcceptsRecognizedReplacement(t *testing.T) {
 	if err != nil {
 		t.Fatalf("PlanDestination() error = %v", err)
 	}
-	if !plan.Exists {
+	if plan.state != destinationBundle {
 		t.Fatal("PlanDestination() did not record recognized existing destination")
 	}
 }
 
+func TestPublishReauthorizesMovedDestination(t *testing.T) {
+	tests := []struct {
+		name            string
+		replace         bool
+		requiresSymlink bool
+		prepare         func(t *testing.T, workingDirectory, target string)
+		mutate          func(t *testing.T, target string)
+		verify          func(t *testing.T, target string)
+	}{
+		{
+			name: "regular file after absent preflight",
+			mutate: func(t *testing.T, target string) {
+				t.Helper()
+				if err := os.WriteFile(target, []byte("unrelated file"), 0o600); err != nil {
+					t.Fatal(err)
+				}
+			},
+			verify: func(t *testing.T, target string) {
+				t.Helper()
+				if data := readFile(t, target); string(data) != "unrelated file" {
+					t.Fatalf("unrelated file data = %q", data)
+				}
+			},
+		},
+		{
+			name: "nonempty directory after empty preflight",
+			prepare: func(t *testing.T, _, target string) {
+				t.Helper()
+				if err := os.Mkdir(target, 0o700); err != nil {
+					t.Fatal(err)
+				}
+			},
+			mutate: writeUnrecognizedDirectory,
+			verify: func(t *testing.T, target string) {
+				t.Helper()
+				if data := readFile(t, filepath.Join(target, "unrelated")); string(data) != "unrelated" {
+					t.Fatalf("unrelated data = %q", data)
+				}
+			},
+		},
+		{
+			name: "regular file after empty preflight",
+			prepare: func(t *testing.T, _, target string) {
+				t.Helper()
+				if err := os.Mkdir(target, 0o700); err != nil {
+					t.Fatal(err)
+				}
+			},
+			mutate: func(t *testing.T, target string) {
+				t.Helper()
+				if err := os.WriteFile(target, []byte("unrelated file"), 0o600); err != nil {
+					t.Fatal(err)
+				}
+			},
+			verify: func(t *testing.T, target string) {
+				t.Helper()
+				if data := readFile(t, target); string(data) != "unrelated file" {
+					t.Fatalf("unrelated file data = %q", data)
+				}
+			},
+		},
+		{
+			name:            "symlink after empty preflight",
+			requiresSymlink: true,
+			prepare: func(t *testing.T, _, target string) {
+				t.Helper()
+				if err := os.Mkdir(target, 0o700); err != nil {
+					t.Fatal(err)
+				}
+			},
+			mutate: func(t *testing.T, target string) {
+				t.Helper()
+				if err := os.Symlink("unrelated-target", target); err != nil {
+					t.Fatal(err)
+				}
+			},
+			verify: func(t *testing.T, target string) {
+				t.Helper()
+				info, err := os.Lstat(target)
+				if err != nil || info.Mode()&os.ModeSymlink == 0 {
+					t.Fatalf("symlink stat = %v, %v", info, err)
+				}
+			},
+		},
+		{
+			name:    "unrecognized directory after bundle preflight",
+			replace: true,
+			prepare: func(t *testing.T, workingDirectory, target string) {
+				t.Helper()
+				plan, err := PlanDestination(workingDirectory, target, false)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if err := Publish(context.Background(), plan, testBundle()); err != nil {
+					t.Fatal(err)
+				}
+			},
+			mutate: writeUnrecognizedDirectory,
+			verify: func(t *testing.T, target string) {
+				t.Helper()
+				if data := readFile(t, filepath.Join(target, "unrelated")); string(data) != "unrelated" {
+					t.Fatalf("unrelated data = %q", data)
+				}
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if test.requiresSymlink && runtime.GOOS == "windows" {
+				t.Skip("symlink replacement coverage requires Unix symlink semantics")
+			}
+			workingDirectory := t.TempDir()
+			target := filepath.Join(workingDirectory, "comparison-daily")
+			if test.prepare != nil {
+				test.prepare(t, workingDirectory, target)
+			}
+			plan, err := PlanDestination(workingDirectory, target, test.replace)
+			if err != nil {
+				t.Fatal(err)
+			}
+			err = publish(context.Background(), plan, testBundle(), publishOperations{
+				rename: os.Rename,
+				beforeCommit: func() {
+					if err := os.RemoveAll(target); err != nil {
+						t.Fatal(err)
+					}
+					test.mutate(t, target)
+				},
+			})
+			if err == nil {
+				t.Fatal("publish() succeeded despite unauthorized replacement")
+			}
+			test.verify(t, target)
+			assertOnlyDestinationEntry(t, workingDirectory, filepath.Base(target))
+		})
+	}
+}
+
+func TestPublishRetainsUnauthorizedMovedDestinationWhenRestoreFails(t *testing.T) {
+	workingDirectory := t.TempDir()
+	target := filepath.Join(workingDirectory, "comparison-daily")
+	if err := os.Mkdir(target, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	plan, err := PlanDestination(workingDirectory, target, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var backupPath string
+	calls := 0
+	err = publish(context.Background(), plan, testBundle(), publishOperations{
+		rename: func(oldPath, newPath string) error {
+			calls++
+			if calls == 1 {
+				backupPath = newPath
+			}
+			if calls == 2 {
+				return errors.New("restore failed")
+			}
+			return os.Rename(oldPath, newPath)
+		},
+		beforeCommit: func() {
+			if err := os.RemoveAll(target); err != nil {
+				t.Fatal(err)
+			}
+			if err := os.WriteFile(target, []byte("unrelated file"), 0o600); err != nil {
+				t.Fatal(err)
+			}
+		},
+	})
+	if err == nil || !strings.Contains(err.Error(), backupPath) {
+		t.Fatalf("publish() error = %v, want retained backup path %q", err, backupPath)
+	}
+	if data := readFile(t, backupPath); string(data) != "unrelated file" {
+		t.Fatalf("retained backup data = %q", data)
+	}
+	if _, err := os.Lstat(target); !errors.Is(err, os.ErrNotExist) {
+		t.Fatalf("target stat error = %v, want not exist", err)
+	}
+	assertOnlyDestinationEntry(t, workingDirectory, filepath.Base(backupPath))
+}
+
+func TestPublishRetainsMovedDestinationWhenTargetReappears(t *testing.T) {
+	workingDirectory := t.TempDir()
+	target := filepath.Join(workingDirectory, "comparison-daily")
+	if err := os.Mkdir(target, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	plan, err := PlanDestination(workingDirectory, target, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var backupPath string
+	err = publish(context.Background(), plan, testBundle(), publishOperations{
+		rename: func(oldPath, newPath string) error {
+			if backupPath != "" {
+				return os.Rename(oldPath, newPath)
+			}
+			backupPath = newPath
+			if err := os.Rename(oldPath, newPath); err != nil {
+				return err
+			}
+			if err := os.WriteFile(filepath.Join(backupPath, "unrelated"), []byte("unrelated"), 0o600); err != nil {
+				t.Fatal(err)
+			}
+			if err := os.WriteFile(target, []byte("reappeared"), 0o600); err != nil {
+				t.Fatal(err)
+			}
+			return nil
+		},
+	})
+	if err == nil || !strings.Contains(err.Error(), "reappeared") || !strings.Contains(err.Error(), backupPath) {
+		t.Fatalf("publish() error = %v, want reappeared target and retained backup path %q", err, backupPath)
+	}
+	if data := readFile(t, target); string(data) != "reappeared" {
+		t.Fatalf("reappeared target data = %q", data)
+	}
+	if data := readFile(t, filepath.Join(backupPath, "unrelated")); string(data) != "unrelated" {
+		t.Fatalf("retained backup data = %q", data)
+	}
+	assertDestinationEntries(t, workingDirectory, filepath.Base(target), filepath.Base(backupPath))
+}
+
 func TestPublishWritesAndReplacesBundle(t *testing.T) {
 	workingDirectory := t.TempDir()
 	target := filepath.Join(workingDirectory, "comparison-daily")
@@ -353,6 +578,53 @@ func publishTestBundle(t *testing.T, bundle LogicalBundle) string {
 	return target
 }
 
+func writeUnrecognizedDirectory(t *testing.T, target string) {
+	t.Helper()
+	if err := os.Mkdir(target, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(target, "unrelated"), []byte("unrelated"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func readFile(t *testing.T, path string) []byte {
+	t.Helper()
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return data
+}
+
+func assertOnlyDestinationEntry(t *testing.T, directory, want string) {
+	t.Helper()
+	assertDestinationEntries(t, directory, want)
+}
+
+func assertDestinationEntries(t *testing.T, directory string, wants ...string) {
+	t.Helper()
+	entries, err := os.ReadDir(directory)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(entries) != len(wants) {
+		t.Fatalf("directory entries = %#v, want %#v", entries, wants)
+	}
+	for _, want := range wants {
+		found := false
+		for _, entry := range entries {
+			if entry.Name() == want {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Fatalf("directory entries = %#v, missing %q", entries, want)
+		}
+	}
+}
+
 func testBundle() LogicalBundle {
 	dataPackage := []byte("report: daily\n")
 	manifest := validManifest()