diff --git a/internal/build/proxy_restore.go b/internal/build/proxy_restore.go index 0dcf5798a..3564f49b3 100644 --- a/internal/build/proxy_restore.go +++ b/internal/build/proxy_restore.go @@ -76,7 +76,7 @@ func cleanupOutputDir(sink Sink, workdir, dir string) error { if why := unwritableIn(filepath.Dir(dir), dir); why != "" { return proxyKeep(dir, "it is owned by root (the traefik-config container wrote it): "+why+" is not writable; run `sudo rm -rf "+dir+"` or chown it to your user, then build again") } - err = removeTree(sink, dir) + err = removeTree(sink, dir, info) if errors.Is(err, fs.ErrPermission) { // The one-shot runs as root and Docker creates its bind directories // root-owned, so an unprivileged build cannot unlink inside them. The @@ -151,10 +151,25 @@ func proxyOwnedDir(workdir, dir string) (bool, string) { return true, "" } +// errTreeSwapped is returned when a directory being emptied stopped being the +// directory the walk found: something replaced it (for example with a symlink) +// between the check and the deletion. +var errTreeSwapped = errors.New("a directory was replaced while it was being removed") + +// stillSameDir reports whether dir is still the real directory identified by want. +// An Lstat never follows a final symlink, so a link swapped in for dir fails. +func stillSameDir(dir string, want fs.FileInfo) bool { + fi, err := os.Lstat(dir) + return err == nil && fi.IsDir() && os.SameFile(want, fi) +} + // removeTree deletes dir's files and then dir, through the sink. A symlink // entry is removed as a link, never followed. The marker goes last, so an // interrupted cleanup still leaves a directory the next build recognises. -func removeTree(sink Sink, dir string) error { +// want is dir's Lstat from the caller's check: the walk re-checks it after the +// listing and before every removal, so a directory swapped for a symlink to +// somewhere else stops the cleanup instead of deleting through the link. +func removeTree(sink Sink, dir string, want fs.FileInfo) error { entries, err := sink.ReadDir(dir) if err != nil { return err @@ -164,9 +179,12 @@ func removeTree(sink Sink, dir string) error { if e.Name() == proxyGeneratedMarker { continue } + if !stillSameDir(dir, want) { + return fmt.Errorf("%s: %w", dir, errTreeSwapped) + } // The listing may be stale: only a directory that is still one is entered. if fi, err := os.Lstat(p); e.IsDir() && err == nil && fi.IsDir() { - if err := removeTree(sink, p); err != nil { + if err := removeTree(sink, p, fi); err != nil { return err } continue @@ -176,6 +194,9 @@ func removeTree(sink Sink, dir string) error { return err } } + if !stillSameDir(dir, want) { + return fmt.Errorf("%s: %w", dir, errTreeSwapped) + } if err := sink.Remove(filepath.Join(dir, proxyGeneratedMarker)); err != nil && !errors.Is(err, fs.ErrNotExist) { return err } diff --git a/internal/build/proxy_restore_test.go b/internal/build/proxy_restore_test.go index 0ae521766..7c343cc29 100644 --- a/internal/build/proxy_restore_test.go +++ b/internal/build/proxy_restore_test.go @@ -4,6 +4,8 @@ package build // wrote (EPIC DEPL D4) and never anything else. import ( + "errors" + "io/fs" "os" "path/filepath" "runtime" @@ -197,3 +199,47 @@ func TestProxyRestoreRootOwnedPlansNoRemoval(t *testing.T) { t.Fatalf("plan recorded removals for a directory apply cannot clean: %v", sink.removed) } } + +// swapSink replaces a directory with a symlink to victim the moment the walk +// lists it, the window between the walk's Lstat and its deletions. +type swapSink struct { + Sink + target, victim string + done bool +} + +func (s *swapSink) ReadDir(p string) ([]fs.DirEntry, error) { + if p == s.target && !s.done { + s.done = true + entries, err := s.Sink.ReadDir(p) + if err != nil { + return nil, err + } + if err := os.RemoveAll(p); err != nil { + return nil, err + } + if err := os.Symlink(s.victim, p); err != nil { + return nil, err + } + return entries, nil + } + return s.Sink.ReadDir(p) +} + +func TestProxyRestoreDirSwappedForSymlinkMidWalk(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlinks need privileges on windows") + } + workdir, dir := restoreProject(t, testRendererMarker) + victim := t.TempDir() + writeFixtureFile(t, filepath.Join(victim, "static.yml"), "precious\n", 0644) + writeFixtureFile(t, filepath.Join(victim, proxyGeneratedMarker), testRendererMarker, 0644) + sink := &swapSink{Sink: newDiskSink(workdir), target: filepath.Join(dir, "sub"), victim: victim} + err := cleanupProviderOutput(sink, workdir, ProviderNginx) + if !errors.Is(err, errTreeSwapped) { + t.Fatalf("a swapped directory must stop the cleanup, got %v", err) + } + if !exists(filepath.Join(victim, "static.yml")) || !exists(filepath.Join(victim, proxyGeneratedMarker)) { + t.Fatal("the cleanup deleted through a symlink swapped in mid-walk") + } +}