From 606b4423f11fb7b0d5dc6862d3add37c262b3b24 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 2 Aug 2026 13:17:50 +0000 Subject: [PATCH] Authorize comparison replacement at commit time --- internal/comparison/publish.go | 60 ++++-- internal/comparison/publish_test.go | 278 +++++++++++++++++++++++++++- 2 files changed, 323 insertions(+), 15 deletions(-) diff --git a/internal/comparison/publish.go b/internal/comparison/publish.go index ade9dea..694b5c4 100644 --- a/internal/comparison/publish.go +++ b/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) - 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) + return restoreMovedDestination(operations, backupDirectory, currentPlan.Target, 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 diff --git a/internal/comparison/publish_test.go b/internal/comparison/publish_test.go index c24b23c..60d31fe 100644 --- a/internal/comparison/publish_test.go +++ b/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()