From defe7e07e8cafe773e8d6f3974ec1367eeb658ba Mon Sep 17 00:00:00 2001 From: Aikins Laryea Date: Mon, 3 Aug 2026 12:26:35 +0000 Subject: [PATCH 1/2] cleanup: pin source network identity --- internal/cli/cleanup.go | 130 ++++++++++++-- internal/cli/cleanup_test.go | 131 +++++++++++++- internal/safepath/safepath.go | 72 ++++++++ internal/safepath/safepath_other.go | 144 +++++++++++++++ internal/safepath/safepath_unix.go | 29 +++ internal/source/localdocker/scanner.go | 4 +- internal/source/localdocker/scanner_test.go | 11 +- internal/target/dokploy/cleanup.go | 150 ++++++++++++++-- internal/target/dokploy/cleanup_test.go | 187 ++++++++++++++++++-- 9 files changed, 804 insertions(+), 54 deletions(-) diff --git a/internal/cli/cleanup.go b/internal/cli/cleanup.go index 6dc5daa..28f4abb 100644 --- a/internal/cli/cleanup.go +++ b/internal/cli/cleanup.go @@ -14,6 +14,7 @@ import ( "time" "github.com/aikins01/bort/internal/analyzer" + "github.com/aikins01/bort/internal/manifest" "github.com/aikins01/bort/internal/planutil" "github.com/aikins01/bort/internal/preparer" "github.com/aikins01/bort/internal/safepath" @@ -86,11 +87,12 @@ type cleanupSourceVolume struct { } type cleanupSourceNetwork struct { - App string `json:"app"` - Name string `json:"name"` - ExpectedIdentity string `json:"expectedIdentity,omitempty"` - ExpectedAbsent bool `json:"expectedAbsent,omitempty"` - Action string `json:"action"` + App string `json:"app"` + Name string `json:"name"` + DiscoveredIdentity string `json:"discoveredIdentity,omitempty"` + ExpectedIdentity string `json:"expectedIdentity,omitempty"` + ExpectedAbsent bool `json:"expectedAbsent,omitempty"` + Action string `json:"action"` } type cleanupTargetArtifact struct { @@ -444,7 +446,11 @@ func planCleanupPurge(run loadedMigrationRun, target string, filters cleanupPurg return cleanupPurgeResult{}, err } selectedRefs := cleanupSelectedAppRefs(selected) - owners, err := cleanupPurgeResourceOwners(run) + networkIdentities, err := cleanupManifestNetworkIdentities(run) + if err != nil { + return cleanupPurgeResult{}, err + } + owners, err := cleanupPurgeResourceOwners(run, networkIdentities) if err != nil { return cleanupPurgeResult{}, err } @@ -457,6 +463,7 @@ func planCleanupPurge(run loadedMigrationRun, target string, filters cleanupPurg CompletesLifecycle: cleanupPurgeCoversAllRunApps(run, filters), Filters: filters, } + manualNetworkCompletion := false for _, app := range selected { if control := cleanupSourceControlForApp(app); control != nil { @@ -522,7 +529,7 @@ func planCleanupPurge(run loadedMigrationRun, target string, filters cleanupPurg } result.SourceVolumes = append(result.SourceVolumes, volume) } - networks, err := cleanupNetworksForApp(run, app) + networks, err := cleanupNetworksForApp(run, app, networkIdentities) if err != nil { return cleanupPurgeResult{}, fmt.Errorf("inspect source networks for %s before purge: %w", app.Name, err) } @@ -541,7 +548,13 @@ func planCleanupPurge(run loadedMigrationRun, target string, filters cleanupPurg result.Warnings = append(result.Warnings, fmt.Sprintf("source network %s for %s is also referenced by unselected app(s): %s", network.Name, app.Name, strings.Join(sharedWith, ", "))) continue } - network.Action = "remove_on_purge_apply" + if identity := strings.TrimSpace(network.DiscoveredIdentity); len(identity) != 64 || !dokploy.ValidSourcePurgeNetworkID(identity) { + manualNetworkCompletion = true + network.Action = "require_absent_before_purge_apply" + result.Warnings = append(result.Warnings, fmt.Sprintf("source network %s for %s has no canonical recorded ID and must be removed manually before cleanup purge --apply; Bort will verify it remains absent", network.Name, app.Name)) + } else { + network.Action = "remove_on_purge_apply" + } result.SourceNetworks = append(result.SourceNetworks, network) } } @@ -563,7 +576,7 @@ func planCleanupPurge(run loadedMigrationRun, target string, filters cleanupPurg if filters.AllApps && !cleanupPurgeCoversAllRunApps(run, filters) { result.Warnings = append(result.Warnings, "--all-apps excludes platform-role apps unless --include-platform is set; this purge will not complete the migration lifecycle") } - if len(cleanupPurgeVolumeChecks(result.SourceVolumes)) > 0 || len(result.SourcePaths) > 0 { + if len(cleanupPurgeVolumeChecks(result.SourceVolumes)) > 0 || len(result.SourcePaths) > 0 || manualNetworkCompletion { setCleanupPurgeManualCompletion(&result) } if filters.AllApps && !result.CompletesLifecycle { @@ -629,7 +642,7 @@ func cleanupPurgeAppRef(app preparer.AppPlan) string { return "" } -func cleanupPurgeResourceOwners(run loadedMigrationRun) (cleanupPurgeOwners, error) { +func cleanupPurgeResourceOwners(run loadedMigrationRun, networkIdentities map[string]string) (cleanupPurgeOwners, error) { owners := cleanupPurgeOwners{ ContainerIDs: map[string][]string{}, ContainerNames: map[string][]string{}, @@ -667,7 +680,7 @@ func cleanupPurgeResourceOwners(run loadedMigrationRun) (cleanupPurgeOwners, err } } } - networks, err := cleanupNetworksForApp(run, app) + networks, err := cleanupNetworksForApp(run, app, networkIdentities) if err != nil { return cleanupPurgeOwners{}, fmt.Errorf("inspect source networks for %s before purge: %w", app.Name, err) } @@ -1032,7 +1045,7 @@ func cleanupPurgeOptions(result cleanupPurgeResult) dokploy.SourcePurgeOptions { opts.Volumes = append(opts.Volumes, dokploy.SourcePurgeVolume{App: volume.App, Service: volume.Service, Name: volume.Name, ExpectedAbsent: volume.ExpectedAbsent}) } for _, network := range result.SourceNetworks { - opts.Networks = append(opts.Networks, dokploy.SourcePurgeNetwork{App: network.App, Name: network.Name, ExpectedIdentity: network.ExpectedIdentity, ExpectedAbsent: network.ExpectedAbsent}) + opts.Networks = append(opts.Networks, dokploy.SourcePurgeNetwork{App: network.App, Name: network.Name, DiscoveredIdentity: network.DiscoveredIdentity, ExpectedIdentity: network.ExpectedIdentity, ExpectedAbsent: network.ExpectedAbsent}) } for _, path := range result.SourcePaths { opts.Paths = append(opts.Paths, dokploy.SourcePurgePath{App: path.App, Source: path.Source, Path: path.Path, AllowPlatform: path.AllowPlatform, ExpectedAbsent: path.ExpectedAbsent}) @@ -1300,6 +1313,10 @@ func planCleanup(ctx context.Context, run loadedMigrationRun, target string) cle DryRun: true, } result.StalePlatformRecords, result.Warnings = inspectStalePlatformRecords(ctx, target) + networkIdentities, networkIdentityErr := cleanupManifestNetworkIdentities(run) + if networkIdentityErr != nil { + result.Warnings = append(result.Warnings, fmt.Sprintf("inspect source network identities: %v", networkIdentityErr)) + } for _, record := range result.StalePlatformRecords { status := "planned" message := "remove Dokploy metadata only if the project still exists and has zero domains" @@ -1327,10 +1344,13 @@ func planCleanup(ctx context.Context, run loadedMigrationRun, target string) cle result.SourceContainers = append(result.SourceContainers, containers...) } result.SourceVolumes = append(result.SourceVolumes, cleanupVolumesForApp(app)...) - networks, err := cleanupNetworksForApp(run, app) + networks, err := cleanupNetworksForApp(run, app, networkIdentities) if err != nil { result.Warnings = append(result.Warnings, fmt.Sprintf("inspect source networks for %s: %v", app.Name, err)) } else { + for i := range networks { + networks[i].Action = "preserve_until_manual_purge_after_acceptance" + } result.SourceNetworks = append(result.SourceNetworks, networks...) } result.TargetArtifacts = append(result.TargetArtifacts, cleanupTargetArtifactsForApp(app)...) @@ -1570,7 +1590,7 @@ func cleanupVolumesForApp(app preparer.AppPlan) []cleanupSourceVolume { return volumes } -func cleanupNetworksForApp(run loadedMigrationRun, app preparer.AppPlan) ([]cleanupSourceNetwork, error) { +func cleanupNetworksForApp(run loadedMigrationRun, app preparer.AppPlan, identities map[string]string) ([]cleanupSourceNetwork, error) { topology, err := cleanupReadTopology(run, app) if err != nil { return nil, err @@ -1580,14 +1600,86 @@ func cleanupNetworksForApp(run loadedMigrationRun, app preparer.AppPlan) ([]clea } networks := make([]cleanupSourceNetwork, 0, len(topology.Networks)) for _, network := range topology.Networks { - if strings.TrimSpace(network) == "" { + network = strings.TrimSpace(network) + if network == "" { continue } - networks = append(networks, cleanupSourceNetwork{App: app.Name, Name: network, Action: "preserve_until_manual_purge_after_acceptance"}) + networks = append(networks, cleanupSourceNetwork{App: app.Name, Name: network, DiscoveredIdentity: identities[network]}) } return networks, nil } +func cleanupManifestNetworkIdentities(run loadedMigrationRun) (map[string]string, error) { + identities := map[string]string{} + manifestPath := strings.TrimSpace(run.Run.ManifestPath) + if manifestPath == "" { + return identities, nil + } + runDir := filepath.FromSlash(run.Run.RunDir) + path := filepath.FromSlash(manifestPath) + name := filepath.Base(path) + if path != name { + manifestAbsolute, err := filepath.Abs(path) + if err != nil { + return nil, err + } + expectedAbsolute, err := filepath.Abs(filepath.Join(runDir, name)) + if err != nil { + return nil, err + } + if manifestAbsolute != expectedAbsolute { + return nil, fmt.Errorf("source manifest path %s is outside migration run %s", manifestPath, run.Run.RunDir) + } + } + contents, err := safepath.ReadPrivateFile(runDir, name) + if err != nil { + return nil, fmt.Errorf("read source manifest network identities: %w", err) + } + var sourceManifest manifest.Manifest + if err := json.Unmarshal(contents, &sourceManifest); err != nil { + return nil, fmt.Errorf("decode source manifest network identities: %w", err) + } + conflicts := map[string]struct{}{} + record := func(name, identity string) { + name = strings.TrimSpace(name) + identity = strings.TrimSpace(identity) + if name == "" || identity == "" { + return + } + if !dokploy.ValidSourcePurgeNetworkID(identity) { + delete(identities, name) + conflicts[name] = struct{}{} + return + } + if _, conflict := conflicts[name]; conflict { + return + } + if current, ok := identities[name]; ok { + if current != identity && !dokploy.SourcePurgeNetworkIDsEquivalent(current, identity) { + delete(identities, name) + conflicts[name] = struct{}{} + return + } + if len(identity) > len(current) { + identities[name] = identity + } + return + } + identities[name] = identity + } + for _, network := range sourceManifest.Networks { + record(network.Name, network.ID) + } + for _, app := range sourceManifest.Apps { + for _, service := range app.Services { + for _, network := range service.Networks { + record(network.Name, network.NetworkID) + } + } + } + return identities, nil +} + func cleanupReadTopology(run loadedMigrationRun, app preparer.AppPlan) (analyzer.Topology, error) { bundleDir := run.Prepare.BundleDir appDir := filepath.Join(bundleDir, filepath.FromSlash(app.Directory)) @@ -1812,11 +1904,15 @@ func writeCleanupPurgeResourceResults(w io.Writer, label string, items []dokploy } fmt.Fprintf(w, " %s:\n", label) for _, item := range items { + identity := "" + if strings.TrimSpace(item.Identity) != "" { + identity = " [id: " + strings.TrimSpace(item.Identity) + "]" + } message := item.Message if message != "" { message = ": " + message } - fmt.Fprintf(w, " [%s] %s%s\n", item.Status, item.Ref, message) + fmt.Fprintf(w, " [%s] %s%s%s\n", item.Status, item.Ref, identity, message) } } diff --git a/internal/cli/cleanup_test.go b/internal/cli/cleanup_test.go index 51960de..2e3af54 100644 --- a/internal/cli/cleanup_test.go +++ b/internal/cli/cleanup_test.go @@ -10,6 +10,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "runtime" "strings" "testing" "time" @@ -289,6 +290,127 @@ func TestRunCleanupPurgeInventoriesDestructiveSourceResources(t *testing.T) { } } +func TestPlanCleanupPurgePinsDiscoveredSourceNetworkIdentity(t *testing.T) { + workDir := t.TempDir() + t.Chdir(workDir) + fullID := "123456789abc" + strings.Repeat("d", 52) + manifestPath := filepath.Join(workDir, "manifest.json") + if err := writeJSONArtifact(manifestPath, manifest.Manifest{ + Source: manifest.Source{Platform: "coolify-local"}, + Networks: []manifest.Network{{ID: fullID, Name: "api-net"}, {ID: "abcdef123456", Name: "legacy-net"}}, + Apps: []manifest.App{{ + Name: "api", + Services: []manifest.Service{{ + ID: "cid123", + Name: "web", + Image: "example/api:latest", + Networks: []manifest.ServiceNetwork{{Name: " api-net ", NetworkID: fullID}, {Name: "legacy-net", NetworkID: "abcdef123456"}}, + }}, + }}, + }); err != nil { + t.Fatal(err) + } + runCommand(t, runMigrate, []string{"--manifest", manifestPath, "--run", "network-identity"}) + run, err := loadMigrationRun("network-identity") + if err != nil { + t.Fatal(err) + } + result, err := planCleanupPurge(run, "dokploy", cleanupPurgeFilters{AllApps: true}, nil) + if err != nil { + t.Fatal(err) + } + if len(result.SourceNetworks) != 2 { + t.Fatalf("expected immutable source network identity in purge plan, got %#v", result.SourceNetworks) + } + networks := map[string]cleanupSourceNetwork{} + for _, network := range result.SourceNetworks { + networks[network.Name] = network + } + if networks["api-net"].DiscoveredIdentity != fullID || networks["api-net"].Action != "require_absent_before_purge_apply" { + t.Fatalf("expected manual completion to require canonical network absence too, got %#v", networks["api-net"]) + } + if networks["legacy-net"].DiscoveredIdentity != "abcdef123456" || networks["legacy-net"].Action != "require_absent_before_purge_apply" { + t.Fatalf("expected legacy network identity to require manual removal, got %#v", networks["legacy-net"]) + } + if !result.ManualCompletion || result.CompletesLifecycle { + t.Fatalf("expected legacy network identity to force manual completion, got %#v", result) + } +} + +func TestCleanupManifestNetworkIdentitiesRejectsPersistentConflict(t *testing.T) { + workDir := t.TempDir() + t.Chdir(workDir) + runDir := filepath.Join(".bort", "runs", "network-conflict") + if err := os.MkdirAll(runDir, 0o700); err != nil { + t.Fatal(err) + } + manifestPath := filepath.Join(runDir, "manifest.json") + if err := writeJSONArtifact(manifestPath, manifest.Manifest{ + Networks: []manifest.Network{ + {ID: strings.Repeat("a", 64), Name: "api-net"}, + {ID: strings.Repeat("b", 64), Name: "api-net"}, + {ID: strings.Repeat("a", 64), Name: "api-net"}, + {ID: strings.Repeat("c", 64), Name: "stable-net"}, + {ID: strings.Repeat("d", 65), Name: "invalid-net"}, + {ID: strings.Repeat("d", 64), Name: "invalid-net"}, + {ID: "eeeeeeeeeeee", Name: "legacy-net"}, + {ID: strings.Repeat("e", 64), Name: "legacy-net"}, + {ID: "ffffffffffff", Name: "legacy-only-net"}, + }, + }); err != nil { + t.Fatal(err) + } + identities, err := cleanupManifestNetworkIdentities(loadedMigrationRun{Run: migrationRun{ + RunDir: filepath.ToSlash(runDir), + ManifestPath: filepath.ToSlash(manifestPath), + }}) + if err != nil { + t.Fatal(err) + } + if _, ok := identities["api-net"]; ok { + t.Fatalf("conflicting network identity was restored by a later duplicate: %#v", identities) + } + if identities["stable-net"] != strings.Repeat("c", 64) { + t.Fatalf("stable network identity was lost: %#v", identities) + } + if _, ok := identities["invalid-net"]; ok { + t.Fatalf("invalid network identity was restored by a later valid record: %#v", identities) + } + if _, ok := identities["legacy-net"]; ok { + t.Fatalf("legacy short network identity was restored by a later full record: %#v", identities) + } + if identities["legacy-only-net"] != "ffffffffffff" { + t.Fatalf("legacy short network identity was not preserved for manual cleanup: %#v", identities) + } +} + +func TestCleanupManifestNetworkIdentitiesRejectsManifestSymlink(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink test") + } + workDir := t.TempDir() + t.Chdir(workDir) + runDir := filepath.Join(".bort", "runs", "manifest-link") + if err := os.MkdirAll(runDir, 0o700); err != nil { + t.Fatal(err) + } + outsidePath := filepath.Join(workDir, "outside-manifest.json") + if err := writeJSONArtifact(outsidePath, manifest.Manifest{Networks: []manifest.Network{{ID: "aaaaaaaaaaaa", Name: "api-net"}}}); err != nil { + t.Fatal(err) + } + manifestPath := filepath.Join(runDir, "manifest.json") + if err := os.Symlink(outsidePath, manifestPath); err != nil { + t.Fatal(err) + } + _, err := cleanupManifestNetworkIdentities(loadedMigrationRun{Run: migrationRun{ + RunDir: filepath.ToSlash(runDir), + ManifestPath: filepath.ToSlash(manifestPath), + }}) + if err == nil { + t.Fatal("expected source manifest symlink to be rejected") + } +} + func TestRunCleanupPurgeApplyRequiresExplicitScopeAndConfirmation(t *testing.T) { workDir := t.TempDir() t.Chdir(workDir) @@ -704,8 +826,8 @@ func TestCleanupPurgePlatformNetworkRequiresExplicitInclusion(t *testing.T) { if err != nil { t.Fatal(err) } - if !withPlatform.CompletesLifecycle || len(withPlatform.SourceNetworks) != 1 || withPlatform.SourceNetworks[0].Name != "coolify" { - t.Fatalf("expected included platform network to be scheduled, got %#v", withPlatform) + if withPlatform.CompletesLifecycle || !withPlatform.ManualCompletion || len(withPlatform.SourceNetworks) != 1 || withPlatform.SourceNetworks[0].Name != "coolify" || withPlatform.SourceNetworks[0].Action != "require_absent_before_purge_apply" { + t.Fatalf("expected included platform network without a recorded identity to require manual absence, got %#v", withPlatform) } } @@ -891,6 +1013,7 @@ func TestCleanupRecoveryCommandsPreserveExternalRunAndPurgeScope(t *testing.T) { } func TestWriteCleanupPurgeTextWarnsAfterPartialApply(t *testing.T) { + networkID := strings.Repeat("a", 64) result := cleanupPurgeResult{ RunName: "partial", RunDir: filepath.Join(".bort", "runs", "partial"), @@ -898,11 +1021,11 @@ func TestWriteCleanupPurgeTextWarnsAfterPartialApply(t *testing.T) { PurgeResult: &dokploy.SourcePurgeResult{Containers: []dokploy.SourcePurgeResourceResult{ {Ref: "web", Status: "removed"}, {Ref: "worker", Status: "error"}, - }}, + }, Networks: []dokploy.SourcePurgeResourceResult{{Ref: "api-net", Identity: networkID, Status: "removed"}}}, } var output strings.Builder writeCleanupPurgeText(&output, result) - for _, want := range []string{"Mode: incomplete", "[removed] web", "[error] worker", "Earlier resources may already have been removed"} { + for _, want := range []string{"Mode: incomplete", "[removed] web", "[error] worker", "[removed] api-net [id: " + networkID + "]", "Earlier resources may already have been removed"} { if !strings.Contains(output.String(), want) { t.Fatalf("expected partial purge output to contain %q, got:\n%s", want, output.String()) } diff --git a/internal/safepath/safepath.go b/internal/safepath/safepath.go index 4aff6f4..d29f791 100644 --- a/internal/safepath/safepath.go +++ b/internal/safepath/safepath.go @@ -1,6 +1,7 @@ package safepath import ( + "bytes" "crypto/rand" "encoding/hex" "errors" @@ -88,6 +89,77 @@ func (d *PrivateDir) CreateFile(name string, mode os.FileMode) (*os.File, error) return createPrivateFileNoFollow(dir, name, mode) } +func (d *PrivateDir) ReadFile(name string) ([]byte, error) { + if err := validatePrivateFileName(name); err != nil { + return nil, err + } + dir, err := d.openFile() + if err != nil { + return nil, err + } + file, err := openPrivateFileNoFollow(dir, name) + if err != nil { + return nil, err + } + defer file.Close() + info, err := file.Stat() + if err != nil { + return nil, err + } + if !info.Mode().IsRegular() { + return nil, fmt.Errorf("private file %s is not regular", name) + } + contents, err := io.ReadAll(file) + if err != nil { + return nil, err + } + afterRead, err := file.Stat() + if err != nil { + return nil, err + } + if !samePrivateFileState(info, afterRead) { + return nil, fmt.Errorf("private file %s changed during read", name) + } + if _, err := file.Seek(0, io.SeekStart); err != nil { + return nil, err + } + verification, err := io.ReadAll(file) + if err != nil { + return nil, err + } + afterVerification, err := file.Stat() + if err != nil { + return nil, err + } + if !samePrivateFileState(afterRead, afterVerification) || !bytes.Equal(contents, verification) { + return nil, fmt.Errorf("private file %s changed during read", name) + } + current, err := openPrivateFileNoFollow(dir, name) + if err != nil { + return nil, err + } + defer current.Close() + currentInfo, err := current.Stat() + if err != nil { + return nil, err + } + if !currentInfo.Mode().IsRegular() || !samePrivateFileState(afterVerification, currentInfo) { + return nil, fmt.Errorf("private file %s changed during read", name) + } + return contents, nil +} + +func samePrivateFileState(a, b os.FileInfo) bool { + return os.SameFile(a, b) && a.Size() == b.Size() && a.Mode() == b.Mode() && a.ModTime().Equal(b.ModTime()) +} + +func ReadPrivateFile(path, name string) ([]byte, error) { + if err := validatePrivateFileName(name); err != nil { + return nil, err + } + return readPrivateFile(path, name) +} + func (d *PrivateDir) WriteFileAtomic(name string, data []byte, mode os.FileMode) error { if err := validatePrivateFileName(name); err != nil { return err diff --git a/internal/safepath/safepath_other.go b/internal/safepath/safepath_other.go index 4af5f92..7cab423 100644 --- a/internal/safepath/safepath_other.go +++ b/internal/safepath/safepath_other.go @@ -3,8 +3,12 @@ package safepath import ( + "bytes" "fmt" + "io" "os" + "path/filepath" + "strings" ) func openPrivateDirNoFollow(path string, _ bool) (*os.File, error) { @@ -15,6 +19,146 @@ func createPrivateFileNoFollow(*os.File, string, os.FileMode) (*os.File, error) return nil, fmt.Errorf("secure private file creation is unavailable on Windows") } +func openPrivateFileNoFollow(*os.File, string) (*os.File, error) { + return nil, fmt.Errorf("secure private file reading is unavailable on Windows") +} + +func readPrivateFile(path, name string) ([]byte, error) { + root, held, err := openPrivateRootNoFollow(path) + if err != nil { + return nil, err + } + defer root.Close() + before, err := root.Lstat(name) + if err != nil { + return nil, err + } + if !before.Mode().IsRegular() { + return nil, fmt.Errorf("private file %s is not regular", name) + } + file, err := root.Open(name) + if err != nil { + return nil, err + } + defer file.Close() + opened, err := file.Stat() + if err != nil { + return nil, err + } + if !opened.Mode().IsRegular() || !os.SameFile(before, opened) { + return nil, fmt.Errorf("private file %s changed during read", name) + } + contents, err := io.ReadAll(file) + if err != nil { + return nil, err + } + afterRead, err := file.Stat() + if err != nil { + return nil, err + } + if !samePrivateFileState(opened, afterRead) { + return nil, fmt.Errorf("private file %s changed during read", name) + } + if _, err := file.Seek(0, io.SeekStart); err != nil { + return nil, err + } + verification, err := io.ReadAll(file) + if err != nil { + return nil, err + } + afterVerification, err := file.Stat() + if err != nil { + return nil, err + } + if !samePrivateFileState(afterRead, afterVerification) || !bytes.Equal(contents, verification) { + return nil, fmt.Errorf("private file %s changed during read", name) + } + currentFile, err := root.Lstat(name) + if err != nil { + return nil, err + } + if !currentFile.Mode().IsRegular() || !samePrivateFileState(afterVerification, currentFile) { + return nil, fmt.Errorf("private file %s changed during read", name) + } + currentRoot, current, err := openPrivateRootNoFollow(path) + if err != nil { + return nil, err + } + defer currentRoot.Close() + if !os.SameFile(held, current) { + return nil, fmt.Errorf("private directory path changed during operation") + } + return contents, nil +} + +func openPrivateRootNoFollow(path string) (*os.Root, os.FileInfo, error) { + abs, err := filepath.Abs(path) + if err != nil { + return nil, nil, err + } + volume := filepath.VolumeName(abs) + if volume == "" { + return nil, nil, fmt.Errorf("private directory path %s has no volume", path) + } + rootPath := volume + string(os.PathSeparator) + rel, err := filepath.Rel(rootPath, abs) + if err != nil { + return nil, nil, err + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { + return nil, nil, fmt.Errorf("private directory path %s escapes volume root", path) + } + root, err := os.OpenRoot(rootPath) + if err != nil { + return nil, nil, err + } + held, err := root.Stat(".") + if err != nil { + root.Close() + return nil, nil, err + } + if rel == "." { + return root, held, nil + } + for _, part := range strings.Split(rel, string(os.PathSeparator)) { + if part == "" || part == "." { + continue + } + before, err := root.Lstat(part) + if err != nil { + root.Close() + return nil, nil, err + } + if !before.IsDir() || before.Mode()&os.ModeSymlink != 0 { + root.Close() + return nil, nil, fmt.Errorf("private directory component %s is not a regular directory", part) + } + next, err := root.OpenRoot(part) + if err != nil { + root.Close() + return nil, nil, err + } + opened, err := next.Stat(".") + if err != nil { + next.Close() + root.Close() + return nil, nil, err + } + if !os.SameFile(before, opened) { + next.Close() + root.Close() + return nil, nil, fmt.Errorf("private directory component %s changed while opening", part) + } + if err := root.Close(); err != nil { + next.Close() + return nil, nil, err + } + root = next + held = opened + } + return root, held, nil +} + func writePrivateFileAtomicNoFollow(*os.File, string, []byte, os.FileMode) error { return fmt.Errorf("secure private file replacement is unavailable on Windows") } diff --git a/internal/safepath/safepath_unix.go b/internal/safepath/safepath_unix.go index 6b95cba..3628e17 100644 --- a/internal/safepath/safepath_unix.go +++ b/internal/safepath/safepath_unix.go @@ -108,6 +108,35 @@ func createPrivateFileNoFollow(dir *os.File, name string, mode os.FileMode) (*os return file, nil } +func openPrivateFileNoFollow(dir *os.File, name string) (*os.File, error) { + fd, err := unix.Openat(int(dir.Fd()), name, unix.O_RDONLY|unix.O_CLOEXEC|unix.O_NOFOLLOW, 0) + if err != nil { + return nil, err + } + file := os.NewFile(uintptr(fd), filepath.Join(dir.Name(), name)) + if file == nil { + _ = unix.Close(fd) + return nil, fmt.Errorf("open private file %s", name) + } + return file, nil +} + +func readPrivateFile(path, name string) ([]byte, error) { + dir, err := OpenPrivateDirNoFollow(path) + if err != nil { + return nil, err + } + defer dir.Close() + contents, err := dir.ReadFile(name) + if err != nil { + return nil, err + } + if err := dir.ValidatePath(); err != nil { + return nil, err + } + return contents, nil +} + func writePrivateFileAtomicNoFollow(dir *os.File, name string, data []byte, mode os.FileMode) error { tmpName, err := privateTempName() if err != nil { diff --git a/internal/source/localdocker/scanner.go b/internal/source/localdocker/scanner.go index 7d1e729..6b8efd1 100644 --- a/internal/source/localdocker/scanner.go +++ b/internal/source/localdocker/scanner.go @@ -234,7 +234,7 @@ func (s *Scanner) inspectNetworks(ctx context.Context) ([]manifest.Network, erro networks := make([]manifest.Network, 0, len(inspected)) for _, network := range inspected { networks = append(networks, manifest.Network{ - ID: shortID(network.ID), + ID: network.ID, Name: network.Name, Driver: network.Driver, Scope: network.Scope, @@ -602,7 +602,7 @@ func ports(raw map[string][]portBinding) []manifest.Port { func networks(raw map[string]networkAttachInspect) []manifest.ServiceNetwork { networks := make([]manifest.ServiceNetwork, 0, len(raw)) for name, network := range raw { - networks = append(networks, manifest.ServiceNetwork{Name: name, NetworkID: shortID(network.NetworkID), IPAddress: network.IPAddress}) + networks = append(networks, manifest.ServiceNetwork{Name: name, NetworkID: network.NetworkID, IPAddress: network.IPAddress}) } sort.Slice(networks, func(i, j int) bool { return networks[i].Name < networks[j].Name }) return networks diff --git a/internal/source/localdocker/scanner_test.go b/internal/source/localdocker/scanner_test.go index 82e3946..91d8af6 100644 --- a/internal/source/localdocker/scanner_test.go +++ b/internal/source/localdocker/scanner_test.go @@ -231,6 +231,7 @@ func TestParseDuBytesAndInt64(t *testing.T) { } func TestScanPopulatesNewServiceFields(t *testing.T) { + networkID := strings.Repeat("a", 64) scanner := &Scanner{ Now: func() time.Time { return time.Unix(0, 0).UTC() }, runCommand: func(_ context.Context, args ...string) ([]byte, error) { @@ -243,7 +244,7 @@ func TestScanPopulatesNewServiceFields(t *testing.T) { "Config":{"Image":"app:1","Env":["PORT=80"],"Labels":{},"Healthcheck":{"Test":["CMD","ok"],"Interval":1000000000,"Retries":2}}, "State":{"Status":"running"}, "Mounts":[{"Type":"volume","Name":"data","Source":"/var/lib/docker/volumes/data/_data","Destination":"/data","RW":true}], - "NetworkSettings":{"Ports":{},"Networks":{}} + "NetworkSettings":{"Ports":{},"Networks":{"net":{"NetworkID":"` + networkID + `","IPAddress":"172.18.0.2"}}} }]`), nil case len(args) >= 2 && args[0] == "image" && args[1] == "inspect": return []byte(`[{"Id":"sha256:abc","RepoDigests":["registry/app@sha256:dead"]}]`), nil @@ -254,7 +255,7 @@ func TestScanPopulatesNewServiceFields(t *testing.T) { case len(args) == 3 && args[0] == "network" && args[1] == "ls" && args[2] == "-q": return []byte("n1\n"), nil case len(args) >= 2 && args[0] == "network" && args[1] == "inspect": - return []byte(`[{"Id":"n1","Name":"net","Driver":"bridge","Scope":"local"}]`), nil + return []byte(`[{"Id":"` + networkID + `","Name":"net","Driver":"bridge","Scope":"local"}]`), nil } return nil, fmt.Errorf("unexpected docker args: %v", args) }, @@ -283,6 +284,12 @@ func TestScanPopulatesNewServiceFields(t *testing.T) { if svc.Healthcheck == nil || svc.Healthcheck.Interval != "1s" || svc.Healthcheck.Retries != 2 { t.Fatalf("unexpected healthcheck: %+v", svc.Healthcheck) } + if len(svc.Networks) != 1 || svc.Networks[0].NetworkID != networkID { + t.Fatalf("expected full service network id %q, got %+v", networkID, svc.Networks) + } + if len(m.Networks) != 1 || m.Networks[0].ID != networkID { + t.Fatalf("expected full top-level network id %q, got %+v", networkID, m.Networks) + } if len(m.Volumes) != 1 { t.Fatalf("expected 1 volume, got %d", len(m.Volumes)) } diff --git a/internal/target/dokploy/cleanup.go b/internal/target/dokploy/cleanup.go index 98fe847..119da09 100644 --- a/internal/target/dokploy/cleanup.go +++ b/internal/target/dokploy/cleanup.go @@ -47,10 +47,11 @@ type SourcePurgeVolume struct { } type SourcePurgeNetwork struct { - App string `json:"app,omitempty"` - Name string `json:"name"` - ExpectedIdentity string `json:"expectedIdentity,omitempty"` - ExpectedAbsent bool `json:"expectedAbsent,omitempty"` + App string `json:"app,omitempty"` + Name string `json:"name"` + DiscoveredIdentity string `json:"discoveredIdentity,omitempty"` + ExpectedIdentity string `json:"expectedIdentity,omitempty"` + ExpectedAbsent bool `json:"expectedAbsent,omitempty"` } type SourcePurgePath struct { @@ -77,10 +78,11 @@ type SourcePurgeResult struct { } type SourcePurgeResourceResult struct { - App string `json:"app,omitempty"` - Ref string `json:"ref"` - Status string `json:"status"` - Message string `json:"message,omitempty"` + App string `json:"app,omitempty"` + Ref string `json:"ref"` + Identity string `json:"identity,omitempty"` + Status string `json:"status"` + Message string `json:"message,omitempty"` } func (c *Client) CleanupStalePlatformProjects(ctx context.Context, opts StalePlatformCleanupOptions) (StalePlatformCleanupResult, error) { @@ -114,7 +116,13 @@ func (c *Client) PurgeSourceResources(ctx context.Context, opts SourcePurgeOptio } volumes := cleanupSourcePurgeVolumes(opts.Volumes) paths := cleanupSourcePurgePaths(opts.Paths) - networks := cleanupSourcePurgeNetworks(opts.Networks) + networks, err := cleanupSourcePurgeNetworks(opts.Networks) + if err != nil { + return result, err + } + if err := validateSourcePurgeNetworksForExecution(networks); err != nil { + return result, err + } if sourcePurgeRequiresManualCompletion(volumes, paths, networks) { for _, volume := range volumes { if err := runSourcePurgeResource(&result, &result.Volumes, opts.OnProgress, SourcePurgeResourceResult{App: volume.App, Ref: volume.Name}, func() (SourcePurgeResourceResult, error) { @@ -138,7 +146,7 @@ func (c *Client) PurgeSourceResources(ctx context.Context, opts SourcePurgeOptio } } for _, network := range networks { - if err := runSourcePurgeResource(&result, &result.Networks, opts.OnProgress, SourcePurgeResourceResult{App: network.App, Ref: network.Name}, func() (SourcePurgeResourceResult, error) { + if err := runSourcePurgeResource(&result, &result.Networks, opts.OnProgress, SourcePurgeResourceResult{App: network.App, Ref: network.Name, Identity: network.ExpectedIdentity}, func() (SourcePurgeResourceResult, error) { return verifySourcePurgeNetworkAbsent(ctx, runner, network) }); err != nil { return result, err @@ -164,7 +172,7 @@ func (c *Client) PurgeSourceResources(ctx context.Context, opts SourcePurgeOptio if !network.ExpectedAbsent { continue } - if err := runSourcePurgeResource(&result, &result.Networks, opts.OnProgress, SourcePurgeResourceResult{App: network.App, Ref: network.Name}, func() (SourcePurgeResourceResult, error) { + if err := runSourcePurgeResource(&result, &result.Networks, opts.OnProgress, SourcePurgeResourceResult{App: network.App, Ref: network.Name, Identity: network.ExpectedIdentity}, func() (SourcePurgeResourceResult, error) { return purgeSourceNetwork(ctx, runner, network) }); err != nil { return result, err @@ -181,7 +189,7 @@ func (c *Client) PurgeSourceResources(ctx context.Context, opts SourcePurgeOptio if network.ExpectedAbsent { continue } - if err := runSourcePurgeResource(&result, &result.Networks, opts.OnProgress, SourcePurgeResourceResult{App: network.App, Ref: network.Name}, func() (SourcePurgeResourceResult, error) { + if err := runSourcePurgeResource(&result, &result.Networks, opts.OnProgress, SourcePurgeResourceResult{App: network.App, Ref: network.Name, Identity: network.ExpectedIdentity}, func() (SourcePurgeResourceResult, error) { return purgeSourceNetwork(ctx, runner, network) }); err != nil { return result, err @@ -264,11 +272,28 @@ func (c *Client) IdentifySourcePurgeResources(ctx context.Context, opts SourcePu identified.Volumes = append(identified.Volumes, volume) } identified.Networks = nil - for _, network := range cleanupSourcePurgeNetworks(opts.Networks) { + networks, err := cleanupSourcePurgeNetworks(opts.Networks) + if err != nil { + return SourcePurgeOptions{}, err + } + for _, network := range networks { identity, absent, err := inspectSourcePurgeNetworkIdentity(ctx, runner, network.Name) if err != nil { return SourcePurgeOptions{}, err } + discoveredIdentity := strings.TrimSpace(network.DiscoveredIdentity) + if !absent && discoveredIdentity == "" { + return SourcePurgeOptions{}, fmt.Errorf("source network %q has no stable ID from discovery; remove it manually before cleanup purge --apply", network.Name) + } + if !absent && !sourcePurgeCanonicalNetworkID(discoveredIdentity) { + return SourcePurgeOptions{}, fmt.Errorf("source network %q has non-canonical discovered ID %q; remove it manually before cleanup purge --apply", network.Name, discoveredIdentity) + } + if !absent && !sourcePurgeCanonicalNetworkID(identity) { + return SourcePurgeOptions{}, fmt.Errorf("docker returned non-canonical ID %q for source network %q", identity, network.Name) + } + if !absent && !SourcePurgeNetworkIDsEquivalent(identity, discoveredIdentity) { + return SourcePurgeOptions{}, fmt.Errorf("refusing source network %q because current ID %s does not match discovered ID %s", network.Name, identity, discoveredIdentity) + } network.ExpectedIdentity = identity network.ExpectedAbsent = absent identified.Networks = append(identified.Networks, network) @@ -349,6 +374,31 @@ func sourcePurgeContainerIDsEquivalent(a, b string) bool { return len(a) >= 12 && strings.HasPrefix(b, a) } +func ValidSourcePurgeNetworkID(identity string) bool { + identity = strings.TrimSpace(identity) + if len(identity) < 12 || len(identity) > 64 { + return false + } + for _, r := range identity { + if r < '0' || r > '9' { + if r < 'a' || r > 'f' { + return false + } + } + } + return true +} + +func sourcePurgeCanonicalNetworkID(identity string) bool { + return len(strings.TrimSpace(identity)) == 64 && ValidSourcePurgeNetworkID(identity) +} + +func SourcePurgeNetworkIDsEquivalent(a, b string) bool { + a = strings.TrimSpace(a) + b = strings.TrimSpace(b) + return sourcePurgeCanonicalNetworkID(a) && sourcePurgeCanonicalNetworkID(b) && a == b +} + func cleanupSourcePurgeContainers(containers []SourcePurgeContainer) ([]SourcePurgeContainer, error) { idsByName := map[string]string{} for _, container := range containers { @@ -434,22 +484,66 @@ func cleanupSourcePurgeVolumes(volumes []SourcePurgeVolume) []SourcePurgeVolume return cleaned } -func cleanupSourcePurgeNetworks(networks []SourcePurgeNetwork) []SourcePurgeNetwork { - seen := map[string]struct{}{} +func cleanupSourcePurgeNetworks(networks []SourcePurgeNetwork) ([]SourcePurgeNetwork, error) { + seen := map[string]int{} cleaned := []SourcePurgeNetwork{} for _, network := range networks { network.Name = strings.TrimSpace(network.Name) + network.DiscoveredIdentity = strings.TrimSpace(network.DiscoveredIdentity) + network.ExpectedIdentity = strings.TrimSpace(network.ExpectedIdentity) if network.Name == "" { continue } - if _, ok := seen[network.Name]; ok { + if network.ExpectedAbsent && network.ExpectedIdentity != "" { + return nil, fmt.Errorf("source network %q has conflicting expected identity and absence state", network.Name) + } + if index, ok := seen[network.Name]; ok { + current := &cleaned[index] + if current.DiscoveredIdentity != "" && network.DiscoveredIdentity != "" && current.DiscoveredIdentity != network.DiscoveredIdentity { + return nil, fmt.Errorf("source network %q has conflicting discovered identities", network.Name) + } + if current.ExpectedIdentity != "" && network.ExpectedIdentity != "" && current.ExpectedIdentity != network.ExpectedIdentity { + return nil, fmt.Errorf("source network %q has conflicting expected identities", network.Name) + } + if current.ExpectedAbsent != network.ExpectedAbsent { + return nil, fmt.Errorf("source network %q has conflicting expected absence state", network.Name) + } + if current.DiscoveredIdentity == "" { + current.DiscoveredIdentity = network.DiscoveredIdentity + } + if current.ExpectedIdentity == "" { + current.ExpectedIdentity = network.ExpectedIdentity + } continue } - seen[network.Name] = struct{}{} + seen[network.Name] = len(cleaned) cleaned = append(cleaned, network) } + for _, network := range cleaned { + if network.ExpectedIdentity != "" && !sourcePurgeCanonicalNetworkID(network.ExpectedIdentity) { + return nil, fmt.Errorf("source network %q has non-canonical confirmed ID %q", network.Name, network.ExpectedIdentity) + } + } sort.Slice(cleaned, func(i, j int) bool { return cleaned[i].Name < cleaned[j].Name }) - return cleaned + return cleaned, nil +} + +func validateSourcePurgeNetworksForExecution(networks []SourcePurgeNetwork) error { + for _, network := range networks { + if network.ExpectedAbsent { + continue + } + if !sourcePurgeCanonicalNetworkID(network.ExpectedIdentity) { + return fmt.Errorf("source network %q has no canonical confirmed ID", network.Name) + } + if !sourcePurgeCanonicalNetworkID(network.DiscoveredIdentity) { + return fmt.Errorf("source network %q has non-canonical discovered ID %q", network.Name, network.DiscoveredIdentity) + } + if !SourcePurgeNetworkIDsEquivalent(network.ExpectedIdentity, network.DiscoveredIdentity) { + return fmt.Errorf("source network %q has conflicting discovered and confirmed IDs", network.Name) + } + } + return nil } func cleanupSourcePurgePaths(paths []SourcePurgePath) []SourcePurgePath { @@ -614,6 +708,24 @@ func purgeSourceNetwork(ctx context.Context, runner dockerRunner, item SourcePur result.Status = "blocked" return result, fmt.Errorf("refusing to remove docker network %q without a pre-confirmation identity", name) } + if !sourcePurgeCanonicalNetworkID(identity) { + result.Status = "blocked" + return result, fmt.Errorf("refusing to remove docker network %q with non-canonical confirmed ID %q", name, identity) + } + discoveredIdentity := strings.TrimSpace(item.DiscoveredIdentity) + if discoveredIdentity == "" { + result.Status = "blocked" + return result, fmt.Errorf("refusing to remove docker network %q without a stable ID from discovery", name) + } + if !sourcePurgeCanonicalNetworkID(discoveredIdentity) { + result.Status = "blocked" + return result, fmt.Errorf("refusing to remove docker network %q with non-canonical discovered ID %q", name, discoveredIdentity) + } + if !SourcePurgeNetworkIDsEquivalent(identity, discoveredIdentity) { + result.Status = "blocked" + return result, fmt.Errorf("refusing to remove docker network %q because confirmed ID %s does not match discovered ID %s", name, identity, discoveredIdentity) + } + result.Identity = identity if _, err := runner.Output(ctx, "network", "rm", identity); err != nil { if isDockerVolumeOrNetworkMissingErr(err) { _, absent, inspectErr := inspectSourcePurgeNetworkIdentity(ctx, runner, name) @@ -638,7 +750,7 @@ func purgeSourceNetwork(ctx context.Context, runner dockerRunner, item SourcePur func verifySourcePurgeNetworkAbsent(ctx context.Context, runner dockerRunner, item SourcePurgeNetwork) (SourcePurgeResourceResult, error) { name := strings.TrimSpace(item.Name) - result := SourcePurgeResourceResult{App: item.App, Ref: name} + result := SourcePurgeResourceResult{App: item.App, Ref: name, Identity: strings.TrimSpace(item.ExpectedIdentity)} if err := validateDockerPurgeName("network", name); err != nil { result.Status = "blocked" return result, err diff --git a/internal/target/dokploy/cleanup_test.go b/internal/target/dokploy/cleanup_test.go index 70230b7..5bd3b51 100644 --- a/internal/target/dokploy/cleanup_test.go +++ b/internal/target/dokploy/cleanup_test.go @@ -144,17 +144,18 @@ func dokployBackupTestDir(t *testing.T) string { } func TestPurgeSourceResourcesRemovesDockerResources(t *testing.T) { + fullNetworkID := "123456789abc" + strings.Repeat("d", 52) runner := &fakeDockerRunner{outputs: map[string][]byte{ "inspect --type container cid123": []byte(`[{"Id":"cid123","Name":"/web","Config":{"Labels":{"coolify.managed":"true"}},"State":{"Running":false,"Status":"exited"}}]`), "rm -f cid123": []byte("cid123\n"), - "network inspect api-net": []byte(`[{"Id":"network-id","Name":"api-net"}]`), - "network rm network-id": []byte("network-id\n"), + "network inspect api-net": []byte(`[{"Id":"` + fullNetworkID + `","Name":"api-net"}]`), + "network rm " + fullNetworkID: []byte(fullNetworkID + "\n"), }} client := &Client{Docker: runner} options, err := client.IdentifySourcePurgeResources(context.Background(), SourcePurgeOptions{ Containers: []SourcePurgeContainer{{App: "api", Service: "web", ContainerID: "cid123", ContainerName: "web"}}, - Networks: []SourcePurgeNetwork{{App: "api", Name: "api-net"}}, + Networks: []SourcePurgeNetwork{{App: "api", Name: "api-net", DiscoveredIdentity: fullNetworkID}}, }) if err != nil { t.Fatalf("IdentifySourcePurgeResources: %v", err) @@ -169,11 +170,14 @@ func TestPurgeSourceResourcesRemovesDockerResources(t *testing.T) { if len(result.Networks) != 1 || result.Networks[0].Status != "removed" { t.Fatalf("unexpected network result: %#v", result.Networks) } + if result.Networks[0].Identity != fullNetworkID { + t.Fatalf("expected canonical network identity in result, got %#v", result.Networks[0]) + } gotArgs := []string{} for _, args := range runner.outputArgs { gotArgs = append(gotArgs, strings.Join(args, " ")) } - for _, want := range []string{"inspect --type container cid123", "rm -f cid123", "network inspect api-net", "network rm network-id"} { + for _, want := range []string{"inspect --type container cid123", "rm -f cid123", "network inspect api-net", "network rm " + fullNetworkID} { found := false for _, got := range gotArgs { if got == want { @@ -245,6 +249,51 @@ func TestCleanupSourcePurgeContainersDeduplicatesShortAndCanonicalIDs(t *testing } } +func TestCleanupSourcePurgeNetworksRejectsConflictingIdentityState(t *testing.T) { + for name, networks := range map[string][]SourcePurgeNetwork{ + "discovered identity": { + {Name: "api-net", DiscoveredIdentity: strings.Repeat("a", 64)}, + {Name: "api-net", DiscoveredIdentity: strings.Repeat("b", 64)}, + }, + "expected identity": { + {Name: "api-net", ExpectedIdentity: strings.Repeat("a", 64)}, + {Name: "api-net", ExpectedIdentity: strings.Repeat("b", 64)}, + }, + "expected absence": { + {Name: "api-net", ExpectedAbsent: true}, + {Name: "api-net"}, + }, + } { + t.Run(name, func(t *testing.T) { + if _, err := cleanupSourcePurgeNetworks(networks); err == nil || !strings.Contains(err.Error(), "conflicting") { + t.Fatalf("expected conflicting duplicate networks to fail closed, got %v", err) + } + }) + } + legacyID := "123456789abc" + networks, err := cleanupSourcePurgeNetworks([]SourcePurgeNetwork{ + {App: "api", Name: "api-net", DiscoveredIdentity: legacyID}, + {App: "worker", Name: "api-net", DiscoveredIdentity: legacyID}, + }) + if err != nil { + t.Fatal(err) + } + if len(networks) != 1 || networks[0].DiscoveredIdentity != legacyID { + t.Fatalf("identical legacy network IDs were not deduplicated: %#v", networks) + } + fullID := "123456789abc" + strings.Repeat("d", 52) + networks, err = cleanupSourcePurgeNetworks([]SourcePurgeNetwork{ + {App: "api", Name: "api-net", DiscoveredIdentity: fullID, ExpectedIdentity: fullID}, + {App: "worker", Name: "api-net", DiscoveredIdentity: fullID, ExpectedIdentity: fullID}, + }) + if err != nil { + t.Fatal(err) + } + if len(networks) != 1 || networks[0].DiscoveredIdentity != fullID || networks[0].ExpectedIdentity != fullID { + t.Fatalf("equivalent duplicate network IDs were not canonicalized: %#v", networks) + } +} + func TestPurgeSourceResourcesSkipsAbsentAllowedPath(t *testing.T) { client := &Client{Docker: &fakeDockerRunner{}} options, err := client.IdentifySourcePurgeResources(context.Background(), SourcePurgeOptions{Paths: []SourcePurgePath{{Path: "/data/coolify/applications/app-1"}}}) @@ -320,38 +369,134 @@ func TestPurgeSourceResourcesRejectsMismatchedInspectedContainerID(t *testing.T) } func TestPurgeSourceNetworkPreservesReplacementAfterReviewedIDDisappears(t *testing.T) { + reviewedID := "aaaaaaaaaaaa" + strings.Repeat("a", 52) + replacementID := strings.Repeat("b", 64) runner := &cleanupDockerRunner{ fakeDockerRunner: &fakeDockerRunner{outputs: map[string][]byte{ - "network inspect api-net": []byte(`[{"Id":"replacement-id","Name":"api-net"}]`), + "network inspect api-net": []byte(`[{"Id":"` + replacementID + `","Name":"api-net"}]`), }}, outputErrors: map[string]error{ - "network rm reviewed-id": errors.New("Error response from daemon: network reviewed-id not found"), + "network rm " + reviewedID: errors.New("Error response from daemon: network not found"), }, } client := &Client{Docker: runner} result, err := client.PurgeSourceResources(context.Background(), SourcePurgeOptions{ - Networks: []SourcePurgeNetwork{{Name: "api-net", ExpectedIdentity: "reviewed-id"}}, + Networks: []SourcePurgeNetwork{{Name: "api-net", DiscoveredIdentity: reviewedID, ExpectedIdentity: reviewedID}}, }) if err == nil || !strings.Contains(err.Error(), "appeared after confirmation") { t.Fatalf("expected replacement network to be preserved, got result=%#v err=%v", result, err) } for _, args := range runner.outputArgs { - if strings.Join(args, " ") == "network rm replacement-id" { + if strings.Join(args, " ") == "network rm "+replacementID { + t.Fatalf("replacement network was removed: %#v", runner.outputArgs) + } + } +} + +func TestIdentifySourcePurgeResourcesRejectsNetworkReplacementSinceDiscovery(t *testing.T) { + discoveredID := "aaaaaaaaaaaa" + strings.Repeat("a", 52) + replacementID := "aaaaaaaaaaaa" + strings.Repeat("b", 52) + runner := &fakeDockerRunner{outputs: map[string][]byte{ + "network inspect api-net": []byte(`[{"Id":"` + replacementID + `","Name":"api-net"}]`), + }} + client := &Client{Docker: runner} + _, err := client.IdentifySourcePurgeResources(context.Background(), SourcePurgeOptions{ + Networks: []SourcePurgeNetwork{{Name: "api-net", DiscoveredIdentity: discoveredID}}, + }) + if err == nil || !strings.Contains(err.Error(), "does not match discovered ID") { + t.Fatalf("expected replacement network to be rejected, got %v", err) + } + for _, args := range runner.outputArgs { + if len(args) > 1 && args[0] == "network" && args[1] == "rm" { t.Fatalf("replacement network was removed: %#v", runner.outputArgs) } } } +func TestIdentifySourcePurgeResourcesRequiresDiscoveredNetworkIdentity(t *testing.T) { + runner := &fakeDockerRunner{outputs: map[string][]byte{ + "network inspect api-net": []byte(`[{"Id":"` + strings.Repeat("a", 64) + `","Name":"api-net"}]`), + }} + client := &Client{Docker: runner} + for name, test := range map[string]struct { + identity string + want string + }{ + "missing": {want: "has no stable ID from discovery"}, + "legacy short": { + identity: "aaaaaaaaaaaa", + want: "has non-canonical discovered ID", + }, + } { + t.Run(name, func(t *testing.T) { + _, err := client.IdentifySourcePurgeResources(context.Background(), SourcePurgeOptions{ + Networks: []SourcePurgeNetwork{{Name: "api-net", DiscoveredIdentity: test.identity}}, + }) + if err == nil || !strings.Contains(err.Error(), test.want) || !strings.Contains(err.Error(), "remove it manually") { + t.Fatalf("expected discovered identity to require manual removal, got %v", err) + } + }) + } +} + +func TestPurgeSourceNetworkRejectsShortConfirmedIdentity(t *testing.T) { + client := &Client{Docker: &fakeDockerRunner{}} + progressCalls := 0 + result, err := client.PurgeSourceResources(context.Background(), SourcePurgeOptions{ + Networks: []SourcePurgeNetwork{{Name: "api-net", DiscoveredIdentity: "123456789abc", ExpectedIdentity: "123456789abc"}}, + OnProgress: func(SourcePurgeResult) error { + progressCalls++ + return nil + }, + }) + if err == nil || !strings.Contains(err.Error(), "non-canonical confirmed ID") { + t.Fatalf("expected short confirmed identity to block mutation, got result=%#v err=%v", result, err) + } + if progressCalls != 0 { + t.Fatalf("invalid confirmed identity was published before rejection: %d progress calls", progressCalls) + } +} + +func TestPurgeSourceNetworkRejectsInvalidIdentityTupleBeforeProgress(t *testing.T) { + confirmedID := strings.Repeat("a", 64) + for name, network := range map[string]SourcePurgeNetwork{ + "missing confirmed": {Name: "api-net", DiscoveredIdentity: confirmedID}, + "missing discovered": {Name: "api-net", ExpectedIdentity: confirmedID}, + "invalid discovered": {Name: "api-net", DiscoveredIdentity: "abcdef", ExpectedIdentity: confirmedID}, + "short discovered": {Name: "api-net", DiscoveredIdentity: confirmedID[:12], ExpectedIdentity: confirmedID}, + "mismatched IDs": {Name: "api-net", DiscoveredIdentity: strings.Repeat("b", 64), ExpectedIdentity: confirmedID}, + } { + t.Run(name, func(t *testing.T) { + runner := &fakeDockerRunner{} + progressCalls := 0 + result, err := (&Client{Docker: runner}).PurgeSourceResources(context.Background(), SourcePurgeOptions{ + Networks: []SourcePurgeNetwork{network}, + OnProgress: func(SourcePurgeResult) error { + progressCalls++ + return nil + }, + }) + if err == nil { + t.Fatalf("expected invalid identity tuple to fail, got %#v", result) + } + if progressCalls != 0 || len(runner.outputArgs) != 0 { + t.Fatalf("invalid identity tuple reached progress or Docker: progress=%d args=%#v", progressCalls, runner.outputArgs) + } + }) + } +} + func TestPurgeSourceResourcesReturnsPartialResults(t *testing.T) { + canonicalID := "aaaaaaaaaaaa" + strings.Repeat("a", 52) runner := &fakeDockerRunner{outputs: map[string][]byte{ "inspect --type container cid123": []byte(`[{"Id":"cid123","Name":"/web","State":{"Running":false,"Status":"exited"}}]`), "rm -f cid123": []byte("cid123\n"), - "network inspect api-net": []byte(`[{"Id":"network-id","Name":"api-net"}]`), + "network inspect api-net": []byte(`[{"Id":"` + canonicalID + `","Name":"api-net"}]`), }} client := &Client{Docker: runner} options, err := client.IdentifySourcePurgeResources(context.Background(), SourcePurgeOptions{ Containers: []SourcePurgeContainer{{App: "api", ContainerID: "cid123", ContainerName: "web"}}, - Networks: []SourcePurgeNetwork{{App: "api", Name: "api-net"}}, + Networks: []SourcePurgeNetwork{{App: "api", Name: "api-net", DiscoveredIdentity: canonicalID}}, }) if err != nil { t.Fatalf("IdentifySourcePurgeResources: %v", err) @@ -554,6 +699,28 @@ func TestSourcePurgeAcceptsNamedVolumeThatRemainsAbsent(t *testing.T) { } } +func TestSourcePurgeManualNetworkResultRetainsConfirmedIdentity(t *testing.T) { + canonicalID := strings.Repeat("a", 64) + runner := &cleanupDockerRunner{ + fakeDockerRunner: &fakeDockerRunner{}, + outputErrors: map[string]error{ + "volume inspect api-data": errors.New("Error response from daemon: get api-data: no such volume"), + "network inspect api-net": errors.New("Error response from daemon: network api-net not found"), + }, + } + client := &Client{Docker: runner} + result, err := client.PurgeSourceResources(context.Background(), SourcePurgeOptions{ + Volumes: []SourcePurgeVolume{{Name: "api-data", ExpectedAbsent: true}}, + Networks: []SourcePurgeNetwork{{Name: "api-net", DiscoveredIdentity: canonicalID, ExpectedIdentity: canonicalID}}, + }) + if err != nil { + t.Fatalf("PurgeSourceResources: %v", err) + } + if len(result.Networks) != 1 || result.Networks[0].Status != "skipped" || result.Networks[0].Identity != canonicalID { + t.Fatalf("confirmed network identity was lost from manual result: %#v", result.Networks) + } +} + type cleanupDockerRunner struct { *fakeDockerRunner outputErrors map[string]error From fa6281d91399ac07ca0fb2c7f246a290ab7c23ac Mon Sep 17 00:00:00 2001 From: Aikins Laryea Date: Mon, 3 Aug 2026 12:36:25 +0000 Subject: [PATCH 2/2] tests: use private cleanup directories --- internal/cli/cleanup_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/cli/cleanup_test.go b/internal/cli/cleanup_test.go index 2e3af54..3026160 100644 --- a/internal/cli/cleanup_test.go +++ b/internal/cli/cleanup_test.go @@ -291,7 +291,7 @@ func TestRunCleanupPurgeInventoriesDestructiveSourceResources(t *testing.T) { } func TestPlanCleanupPurgePinsDiscoveredSourceNetworkIdentity(t *testing.T) { - workDir := t.TempDir() + workDir := cleanupBackupTestDir(t) t.Chdir(workDir) fullID := "123456789abc" + strings.Repeat("d", 52) manifestPath := filepath.Join(workDir, "manifest.json") @@ -338,7 +338,7 @@ func TestPlanCleanupPurgePinsDiscoveredSourceNetworkIdentity(t *testing.T) { } func TestCleanupManifestNetworkIdentitiesRejectsPersistentConflict(t *testing.T) { - workDir := t.TempDir() + workDir := cleanupBackupTestDir(t) t.Chdir(workDir) runDir := filepath.Join(".bort", "runs", "network-conflict") if err := os.MkdirAll(runDir, 0o700); err != nil { @@ -388,7 +388,7 @@ func TestCleanupManifestNetworkIdentitiesRejectsManifestSymlink(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("symlink test") } - workDir := t.TempDir() + workDir := cleanupBackupTestDir(t) t.Chdir(workDir) runDir := filepath.Join(".bort", "runs", "manifest-link") if err := os.MkdirAll(runDir, 0o700); err != nil {