diff --git a/CHANGELOG.md b/CHANGELOG.md index e45f66f..997042d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,10 @@ `/mnt/`, where the controller and the Docker daemon see the same paths: a path on the controller's own data volume differs for the daemon, and a capped `--delete` there would mirror an EMPTY folder over the copy. Those paths, a box whose image cannot be read and a container that cannot start (exit 125) keep the plain in-process rsync; an rsync - failure inside the container is an error. Tests `TestR1214_*` (3, Docker-free seams), red-proved both ways. + failure inside the container is an error. A path with a SYMLINK anywhere in it never takes the container either + (a link under /mnt pointing outside resolves elsewhere for the daemon — found by the commit's security review the + same night; `filepath.EvalSymlinks` must return the path itself). Tests `TestR1214_*` (4, Docker-free seams), + red-proved each way. ## Unreleased — two data-loss candidates fixed (R-1187 c, R-1189 d), 2026-10-22 night diff --git a/controller/internal/backup/r1214_capped_copy_test.go b/controller/internal/backup/r1214_capped_copy_test.go index 296434c..2ca74a9 100644 --- a/controller/internal/backup/r1214_capped_copy_test.go +++ b/controller/internal/backup/r1214_capped_copy_test.go @@ -91,3 +91,24 @@ func TestR1214_ANotStartedContainerFallsBackAnRsyncErrorDoesNot(t *testing.T) { t.Fatalf("no readable image → the plain copy: %v", err) } } + +// A symlink under the capped root that points elsewhere must never reach the container (the daemon would resolve it +// somewhere else, and `--delete` would mirror that over the copy) — the security review's case. +// RED-PROOF: with the EvalSymlinks check removed, this fails — seen failing 2026-10-22. +func TestR1214_ASymlinkedPathNeverGoesToDocker(t *testing.T) { + root := t.TempDir() + "/" + elsewhere := t.TempDir() + os.WriteFile(filepath.Join(elsewhere, "f"), []byte("x"), 0644) + link := filepath.Join(root, "drive1", "unit") + os.MkdirAll(filepath.Dir(link), 0755) + if err := os.Symlink(elsewhere, link); err != nil { + t.Fatal(err) + } + rec := withSeams(t, root, "img", func([]string) ([]byte, int, error) { return nil, 0, nil }) + if err := rsyncMirror(link, filepath.Join(root, "drive2", "unit")); err != nil { + t.Fatal(err) + } + if len(rec.calls) != 0 { + t.Fatalf("a symlinked path went to the container: %v", rec.calls) + } +} diff --git a/controller/internal/backup/tier2.go b/controller/internal/backup/tier2.go index 66160cd..11e3f97 100644 --- a/controller/internal/backup/tier2.go +++ b/controller/internal/backup/tier2.go @@ -818,9 +818,15 @@ func rsyncMirror(src, dst string) error { const tier2CopyMemCap = "512m" // tier2CappedPath: a path the controller and the Docker daemon see identically (the guest's /mnt is bind-mounted into -// the controller at /mnt). Never a relative path, never `..`. +// the controller at /mnt). Never a relative path, never `..`, and NO SYMLINK anywhere in it: a link under /mnt that +// points outside /mnt resolves to a different place for the daemon (found by the commit's security review, +// 2026-10-22) — the path must resolve to itself. The path must exist (rsyncMirror creates dst first). func tier2CappedPath(p string) bool { - return strings.HasPrefix(p, tier2CappedRoot) && filepath.Clean(p) == p + if !strings.HasPrefix(p, tier2CappedRoot) || filepath.Clean(p) != p { + return false + } + real, err := filepath.EvalSymlinks(p) + return err == nil && real == p } // tier2CappedRoot is "/mnt/" in production (a test seam: tests point it at a temp dir).