buildstep: keep a check ask's cache layer out of every step's reach, move through an os.Root, and layer the build cache alone

Review of 62a50eb: the layer lay in the workspace, which every step binds read-write,
so a change's command could swap a kept directory for a symlink and the move, run
outside the sandbox, would write into the shared cache (ADR 0541 §2). The layer now
lies beside the workspace (Workspaces.CacheLayer, removed by Release and Sweep),
Command refuses a layer within reach of the workspace, a writable directory or a
cache, and every move goes through an os.Root on the layer and keeps no symlink. Only
LayerCaches (the build cache) are layered; a module cache keeps its throwaway layer
(issue 1921).

go/buildstep/isolate.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"slices"
 	"strconv"
 	"strings"
 	"sync"
@@ -33,20 +34,26 @@ const Bubblewrap = "bwrap"
 //     downloaded) read-only;
 //   - the shared warm Caches read-only under a throwaway writable layer (S18), or read-write for a trunk build alone
 //     (WriteCaches), so a candidate's code never changes what a later build reads;
-//   - with a CacheLayer, what one command writes into the caches kept for the later commands of the same ask (issue 1921,
-//     the try speed-up): the vet, the unit tests and the integration compile of one ask compile each package once.
+//   - with a CacheLayer, what one command writes into the LayerCaches kept for the later commands of the same ask (issue
+//     1921, the try speed-up): the vet, the unit tests and the integration compile of one ask compile each package once.
 type Isolation struct {
 	Workspace   string
 	Writable    []string
 	ReadOnly    []string
 	Caches      []string
 	WriteCaches bool
-	// CacheLayer is a directory of the ask's own, in its workspace and gone with it: each command writes the caches into
-	// a layer of its own beneath it (overlayfs refuses an upper layer two mounts share, and an ask's commands run side by
-	// side), which Isolated moves into the ask's kept layer when the command ends; every later command reads the kept
-	// layer above the shared cache. The shared cache stays unchanged. Empty: what a command writes is thrown away.
+	// CacheLayer is a directory of the ask's own, gone with its workspace (Workspaces.CacheLayer): each command writes
+	// the LayerCaches into a layer of its own beneath it (overlayfs refuses an upper layer two mounts share, and an
+	// ask's commands run side by side), which Isolated moves into the ask's kept layer when the command ends; every
+	// later command reads the kept layer above the shared cache. The shared cache stays unchanged. It must lie outside
+	// the Workspace and every Writable directory, so no command reaches it (a command that could would make the move,
+	// which runs outside the sandbox, write where it points). Empty: what a command writes is thrown away.
 	CacheLayer string
-	Env        []string
+	// LayerCaches are the Caches kept in the CacheLayer: a build cache, whose entries are whole files written by
+	// rename. Every other cache (a module cache, with read-only directories and half-written modules) keeps a
+	// throwaway layer.
+	LayerCaches []string
+	Env         []string
 	// Hosts is the file bound as /etc/hosts; Isolated writes one naming only the loopback.
 	Hosts string
 	// run is the directory of this command's own writable layers under CacheLayer; Isolated makes one per command.
@@ -110,6 +117,9 @@ func (iso Isolation) command(env []string, name string, args ...string) ([]strin
 			}
 		}
 	}
+	if err := iso.checkLayer(); err != nil {
+		return nil, err
+	}
 	words := append(append([]string{}, iso.Env...), env...)
 	home := false
 	for _, w := range words {
@@ -146,16 +156,16 @@ func (iso Isolation) command(env []string, name string, args ...string) ([]strin
 
 // cacheArgs binds the shared warm caches (S18): for a trunk build read-write, so the next build is warm; for any other
 // read-only beneath an overlay whose writes go to a tmpfs of the step's own, so the step writes as a build does and
-// nothing it wrote outlives it (bubblewrap's --tmp-overlay, an overlay mount in the step's user namespace). With a
-// CacheLayer the ask's kept layer lies above the shared cache and the command's writes go to its own layer on disk
-// (--overlay), which Isolated keeps for the ask when the command ends.
+// nothing it wrote outlives it (bubblewrap's --tmp-overlay, an overlay mount in the step's user namespace). A cache of
+// LayerCaches, with a CacheLayer, has the ask's kept layer above the shared cache and the command's writes go to its own
+// layer on disk (--overlay), which Isolated keeps for the ask when the command ends.
 func (iso Isolation) cacheArgs() []string {
 	var argv []string
 	for i, c := range iso.Caches {
 		switch {
 		case iso.WriteCaches:
 			argv = append(argv, "--bind", c, c)
-		case iso.CacheLayer != "":
+		case iso.layered(c):
 			upper, work := iso.runLayer(i)
 			argv = append(argv, "--overlay-src", c, "--overlay-src", keptLayer(iso.CacheLayer, i), "--overlay", upper, work, c)
 		default:
@@ -165,6 +175,33 @@ func (iso Isolation) cacheArgs() []string {
 	return argv
 }
 
+// layered says whether the cache c is kept in the ask's cache layer.
+func (iso Isolation) layered(c string) bool {
+	return iso.CacheLayer != "" && !iso.WriteCaches && slices.Contains(iso.LayerCaches, c)
+}
+
+// checkLayer refuses a cache layer a command could reach (in or above the workspace or a writable directory, or a
+// cache), and a layered cache that is not one of the caches.
+func (iso Isolation) checkLayer() error {
+	if iso.CacheLayer == "" {
+		return nil
+	}
+	for _, d := range append(append([]string{iso.Workspace}, iso.Writable...), iso.Caches...) {
+		if within(iso.CacheLayer, d) || within(d, iso.CacheLayer) {
+			return fmt.Errorf("buildstep: the cache layer %s is within reach of the step's %s", iso.CacheLayer, d)
+		}
+	}
+	for _, c := range iso.LayerCaches {
+		if !slices.Contains(iso.Caches, c) {
+			return fmt.Errorf("buildstep: the layered cache %s is not one of the caches", c)
+		}
+	}
+	return nil
+}
+
+// within says whether p is dir or lies beneath it.
+func within(p, dir string) bool { return p == dir || strings.HasPrefix(p, dir+"/") }
+
 func layerDirs(layer string) []string {
 	if layer == "" {
 		return nil
@@ -172,8 +209,9 @@ func layerDirs(layer string) []string {
 	return []string{layer}
 }
 
-// keptLayer is where the ask keeps what its commands wrote into the cache i.
-func keptLayer(layer string, i int) string { return filepath.Join(layer, "kept", strconv.Itoa(i)) }
+// keptRel is where, in the cache layer, the ask keeps what its commands wrote into the cache i; keptLayer is its path.
+func keptRel(i int) string                 { return filepath.Join("kept", strconv.Itoa(i)) }
+func keptLayer(layer string, i int) string { return filepath.Join(layer, keptRel(i)) }
 
 // runLayer is the upper and work directory of this command's writes into the cache i. Command, which makes no
 // directory, names the layer "run"; Isolated gives each command one of its own.
@@ -190,9 +228,12 @@ var layerLocks sync.Map
 
 // prepareLayer makes the kept layers and a run directory of the command's own under the cache layer.
 func (iso *Isolation) prepareLayer() error {
-	if iso.CacheLayer == "" || iso.WriteCaches || len(iso.Caches) == 0 {
+	if iso.CacheLayer == "" || iso.WriteCaches || len(iso.LayerCaches) == 0 {
 		return nil
 	}
+	if err := iso.checkLayer(); err != nil {
+		return err
+	}
 	if err := os.MkdirAll(filepath.Join(iso.CacheLayer, "runs"), 0o700); err != nil {
 		return err
 	}
@@ -201,7 +242,10 @@ func (iso *Isolation) prepareLayer() error {
 		return err
 	}
 	iso.run = run
-	for i := range iso.Caches {
+	for i, c := range iso.Caches {
+		if !iso.layered(c) {
+			continue
+		}
 		upper, work := iso.runLayer(i)
 		for _, d := range []string{keptLayer(iso.CacheLayer, i), upper, work} {
 			if err := os.MkdirAll(d, 0o700); err != nil {
@@ -212,10 +256,11 @@ func (iso *Isolation) prepareLayer() error {
 	return nil
 }
 
-// keepLayer moves what the command wrote into the caches into the ask's kept layers, and removes its run directory.
-// A file the command wrote replaces the kept one (a cache's newest write wins); a whiteout (a file the command
-// deleted) is not kept, since a cache entry deleted by one command is no fault of a later one. A write that cannot be
-// kept only means a later command builds it again.
+// keepLayer moves what the command wrote into the layered caches into the ask's kept layers, and removes its run
+// directory. A file the command wrote replaces the kept one (a cache's newest write wins); a whiteout (a file the
+// command deleted), a symlink or any other entry that is not a directory or a regular file is not kept. Every move goes
+// through an os.Root on the cache layer, so no symlink leads it outside the layer. A write that cannot be kept only
+// means a later command builds it again.
 func (iso Isolation) keepLayer() {
 	if iso.run == "" {
 		return
@@ -224,23 +269,37 @@ func (iso Isolation) keepLayer() {
 	mu, _ := layerLocks.LoadOrStore(iso.CacheLayer, &sync.Mutex{})
 	mu.(*sync.Mutex).Lock()
 	defer mu.(*sync.Mutex).Unlock()
-	for i := range iso.Caches {
-		upper, _ := iso.runLayer(i)
-		kept := keptLayer(iso.CacheLayer, i)
-		filepath.WalkDir(upper, func(p string, d fs.DirEntry, err error) error {
+	root, err := os.OpenRoot(iso.CacheLayer)
+	if err != nil {
+		return
+	}
+	defer root.Close()
+	runRel, err := filepath.Rel(iso.CacheLayer, iso.run)
+	if err != nil {
+		return
+	}
+	for i, c := range iso.Caches {
+		if !iso.layered(c) {
+			continue
+		}
+		upper := filepath.Join(runRel, strconv.Itoa(i), "upper")
+		fs.WalkDir(root.FS(), upper, func(p string, d fs.DirEntry, err error) error {
 			if err != nil || p == upper {
 				return nil
 			}
 			rel, _ := filepath.Rel(upper, p)
-			to := filepath.Join(kept, rel)
+			to := filepath.Join(keptRel(i), rel)
 			switch {
 			case d.IsDir():
-				if fi, err := os.Lstat(to); err == nil && !fi.IsDir() {
-					os.Remove(to)
+				if fi, err := root.Lstat(to); err == nil && !fi.IsDir() {
+					root.Remove(to)
 				}
-				os.MkdirAll(to, 0o700)
+				root.MkdirAll(to, 0o700)
 			case d.Type().IsRegular():
-				os.Rename(p, to)
+				if fi, err := root.Lstat(to); err == nil && !fi.Mode().IsRegular() {
+					return nil // never replace what is not a file the layer kept
+				}
+				root.Rename(p, to)
 			}
 			return nil
 		})

go/buildstep/isolate_caches_test.go

@@ -32,24 +32,35 @@ func TestACandidatesStepWritesTheCachesOnlyIntoAThrowawayLayer(t *testing.T) {
 	}
 	// 1921/S2: with a cache layer, a command reads the ask's kept layer above the shared cache and writes into a layer
 	// on disk of the ask's own, never into the shared cache and never into a throwaway tmpfs.
-	iso = Isolation{Workspace: "/w", Caches: []string{"/cache/gomod", "/cache/gobuild"}, CacheLayer: "/w/cachelayer"}
+	// Only the layered caches (the build cache) are; the module cache keeps its throwaway layer.
+	iso = Isolation{Workspace: "/w/ws1", Caches: []string{"/cache/gomod", "/cache/gobuild"}, CacheLayer: "/w/ws1.cachelayer",
+		LayerCaches: []string{"/cache/gobuild"}}
 	argv, err = iso.Command("go", "vet")
 	if err != nil {
 		t.Fatal(err)
 	}
-	for i, c := range iso.Caches {
-		if !holds(argv, "--overlay-src", c, "--overlay-src", keptLayer(iso.CacheLayer, i), "--overlay") {
-			t.Errorf("the cache %s is not read beneath the ask's kept layer: %v", c, argv)
-		}
-		if up := overlayOf(argv, c); up == "" || !strings.HasPrefix(up, "/w/cachelayer/") {
-			t.Errorf("the cache %s is written into %q, not a layer of the ask's own: %v", c, up, argv)
-		}
-		if holds(argv, "--tmp-overlay", c) || holds(argv, "--bind", c, c) {
-			t.Errorf("the cache %s is thrown away or written: %v", c, argv)
-		}
+	if !holds(argv, "--overlay-src", "/cache/gobuild", "--overlay-src", keptLayer(iso.CacheLayer, 1), "--overlay") {
+		t.Errorf("the build cache is not read beneath the ask's kept layer: %v", argv)
 	}
-	if _, err := (Isolation{Workspace: "/w", CacheLayer: "cachelayer"}).Command("go"); err == nil {
-		t.Error("a relative cache layer was taken")
+	if up := overlayOf(argv, "/cache/gobuild"); up == "" || !strings.HasPrefix(up, "/w/ws1.cachelayer/") {
+		t.Errorf("the build cache is written into %q, not a layer of the ask's own: %v", up, argv)
+	}
+	if !holds(argv, "--overlay-src", "/cache/gomod", "--tmp-overlay", "/cache/gomod") || overlayOf(argv, "/cache/gomod") != "" {
+		t.Errorf("the module cache is not under a throwaway layer: %v", argv)
+	}
+	// A layer a command could reach is refused: in the workspace (bound read-write), in a writable directory, in a
+	// cache, or above one of them; so is a relative one, and a layered cache that is no cache.
+	for _, bad := range []Isolation{
+		{Workspace: "/w/ws1", CacheLayer: "/w/ws1/cachelayer"},
+		{Workspace: "/w/ws1", CacheLayer: "/w"},
+		{Workspace: "/w/ws1", Writable: []string{"/out"}, CacheLayer: "/out/layer"},
+		{Workspace: "/w/ws1", Caches: []string{"/cache/gobuild"}, CacheLayer: "/cache/gobuild/layer"},
+		{Workspace: "/w/ws1", CacheLayer: "cachelayer"},
+		{Workspace: "/w/ws1", Caches: []string{"/cache/gobuild"}, CacheLayer: "/w/ws1.cachelayer", LayerCaches: []string{"/cache/other"}},
+	} {
+		if _, err := bad.Command("go"); err == nil {
+			t.Errorf("the cache layer %s of %+v was taken", bad.CacheLayer, bad)
+		}
 	}
 }
 

go/buildstep/isolate_integration_test.go

@@ -66,8 +66,9 @@ func TestARunStepReachesNothingAndLeavesTheCachesUnchanged(t *testing.T) {
 
 	// 1921/S2: with the ask's cache layer, what one command writes into the cache the next command of the ask reads,
 	// above what the shared cache holds, and the shared cache stays unchanged.
+	layer := filepath.Join(filepath.Dir(workspace), "ws1.cachelayer")
 	asked := Isolated(forgekey.ExecRunnerFor(0), Isolation{Workspace: workspace, Caches: []string{cacheDir},
-		CacheLayer: filepath.Join(workspace, "cachelayer")})
+		CacheLayer: layer, LayerCaches: []string{cacheDir}})
 	r = asked(context.Background(), []string{"PATH=/usr/bin:/bin"}, "/bin/sh", "-c", "cat "+filepath.Join(cacheDir, "warm")+" && mkdir -p "+filepath.Join(cacheDir, "ab")+" && echo compiled > "+filepath.Join(cacheDir, "ab", "one-d"))
 	if r.Status != 0 || r.Err != "" || strings.TrimSpace(r.Stdout) != "warm" {
 		t.Fatalf("the first command of the ask did not read the shared cache and write its own (status %d, %s): %s%s", r.Status, r.Err, r.Stdout, r.Stderr)
@@ -76,10 +77,23 @@ func TestARunStepReachesNothingAndLeavesTheCachesUnchanged(t *testing.T) {
 	if r.Status != 0 || r.Err != "" || r.Stdout != "compiled\nwarm\n" {
 		t.Fatalf("the next command of the ask did not find what the first wrote (status %d, %s): %q %s", r.Status, r.Err, r.Stdout, r.Stderr)
 	}
-	if _, err := os.Stat(filepath.Join(cacheDir, "ab")); !os.IsNotExist(err) {
-		t.Fatalf("the ask's commands wrote the shared cache: %v", err)
+	// A command cannot reach the layer (it lies outside every directory the step binds), and a symlink it writes into
+	// the cache is not kept, so nothing it does makes the keeping write outside the layer.
+	r = asked(context.Background(), []string{"PATH=/usr/bin:/bin"}, "/bin/sh", "-c",
+		"ls "+layer+" >/dev/null 2>&1 && echo reached; ln -s "+cacheDir+" "+filepath.Join(cacheDir, "esc")+" && mkdir "+filepath.Join(cacheDir, "cd")+" && echo x > "+filepath.Join(cacheDir, "cd", "f"))
+	if r.Status != 0 || r.Err != "" || strings.Contains(r.Stdout, "reached") {
+		t.Fatalf("the command reached the cache layer or could not write the cache (status %d, %s): %q %s", r.Status, r.Err, r.Stdout, r.Stderr)
 	}
-	if err := os.RemoveAll(filepath.Join(workspace, "cachelayer")); err != nil {
-		t.Fatalf("the ask's cache layer cannot be removed with its workspace: %v", err)
+	if fi, err := os.Lstat(filepath.Join(keptLayer(layer, 0), "esc")); err == nil {
+		t.Fatalf("a symlink the command wrote was kept: %v", fi.Mode())
+	}
+	if b, err := os.ReadFile(filepath.Join(keptLayer(layer, 0), "cd", "f")); err != nil || string(b) != "x\n" {
+		t.Fatalf("the command's file was not kept: %q %v", b, err)
+	}
+	if entries, _ := os.ReadDir(cacheDir); len(entries) != 1 || entries[0].Name() != "warm" {
+		t.Fatalf("the ask's commands wrote the shared cache: %v", entries)
+	}
+	if err := os.RemoveAll(layer); err != nil {
+		t.Fatalf("the ask's cache layer cannot be removed: %v", err)
 	}
 }

go/buildstep/isolate_test.go

@@ -124,7 +124,7 @@ func TestIsolatedRunsEachCommandThroughBubblewrap(t *testing.T) {
 	if err := os.MkdirAll(shared, 0o700); err != nil {
 		t.Fatal(err)
 	}
-	layer := filepath.Join(root, "ws", "cachelayer")
+	layer := filepath.Join(root, "ws1.cachelayer")
 	var mu sync.Mutex
 	uppers := map[string]bool{}
 	gate := make(chan struct{})
@@ -141,7 +141,15 @@ func TestIsolatedRunsEachCommandThroughBubblewrap(t *testing.T) {
 		os.WriteFile(filepath.Join(up, "ab", args[len(args)-1]+"-d"), []byte("compiled"), 0o600)
 		return forgekey.Ran{}
 	}
-	run := Isolated(writer, Isolation{Workspace: filepath.Join(root, "ws"), Caches: []string{shared}, CacheLayer: layer})
+	// A symlink in the kept layer (none is ever kept; planted here) leads no move outside the layer: the kept directory
+	// replaces it, and the shared cache it points to stays unchanged.
+	if err := os.MkdirAll(keptLayer(layer, 0), 0o700); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink(shared, filepath.Join(keptLayer(layer, 0), "ab")); err != nil {
+		t.Fatal(err)
+	}
+	run := Isolated(writer, Isolation{Workspace: filepath.Join(root, "ws1"), Caches: []string{shared}, CacheLayer: layer, LayerCaches: []string{shared}})
 	var wg sync.WaitGroup
 	for _, pkg := range []string{"one", "two"} {
 		wg.Add(1)

go/buildstep/workspaces.go

@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"sort"
+	"strings"
 	"sync"
 	"syscall"
 	"time"
@@ -116,6 +117,14 @@ func (w *Workspaces) touch(id string) {
 	_ = os.Chtimes(filepath.Join(w.Dir, id), now, now) // a workspace not made yet has nothing to mark
 }
 
+// cacheLayerSuffix names a workspace's cache layer beside it (Isolation.CacheLayer): never inside it, which every step
+// of the workspace binds read-write.
+const cacheLayerSuffix = ".cachelayer"
+
+// CacheLayer is the directory of the workspace id's cache layer: beside the workspace, out of every step's reach, and
+// removed with it by Release and Sweep.
+func (w *Workspaces) CacheLayer(id string) string { return filepath.Join(w.Dir, id+cacheLayerSuffix) }
+
 // Release removes the workspace at once: its caller's chain of steps has ended. One already gone is no error; one a
 // running step holds is refused and kept; an id that is not a workspace's is refused.
 func (w *Workspaces) Release(id string) error {
@@ -130,6 +139,9 @@ func (w *Workspaces) Release(id string) error {
 	if err := os.RemoveAll(filepath.Join(w.Dir, id)); err != nil {
 		return fmt.Errorf("the workspace %s could not be removed: %w", id, err)
 	}
+	if err := os.RemoveAll(w.CacheLayer(id)); err != nil {
+		return fmt.Errorf("the cache layer of the workspace %s could not be removed: %w", id, err)
+	}
 	return nil
 }
 
@@ -144,6 +156,13 @@ func (w *Workspaces) Sweep(now time.Time, bound time.Duration) []string {
 	w.mu.Lock()
 	defer w.mu.Unlock()
 	for _, e := range entries {
+		if id, ok := strings.CutSuffix(e.Name(), cacheLayerSuffix); ok && workspaceID.MatchString(id) && w.held[id] == 0 {
+			// a cache layer whose workspace is gone (its removal failed or was cut short) goes too
+			if _, err := os.Stat(filepath.Join(w.Dir, id)); os.IsNotExist(err) {
+				os.RemoveAll(filepath.Join(w.Dir, e.Name()))
+			}
+			continue
+		}
 		if !e.IsDir() || !workspaceID.MatchString(e.Name()) || w.held[e.Name()] > 0 {
 			continue
 		}
@@ -152,6 +171,7 @@ func (w *Workspaces) Sweep(now time.Time, bound time.Duration) []string {
 			continue
 		}
 		if os.RemoveAll(filepath.Join(w.Dir, e.Name())) == nil {
+			os.RemoveAll(w.CacheLayer(e.Name()))
 			removed = append(removed, e.Name())
 		}
 	}

go/buildstep/workspaces_test.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 )
@@ -68,8 +69,15 @@ func gone(p string) bool { _, err := os.Stat(p); return os.IsNotExist(err) }
 func TestReleaseRemovesAWorkspaceAtOnce(t *testing.T) {
 	w := &Workspaces{Dir: t.TempDir()}
 	p := makeWorkspace(t, w, "abc1", 0)
-	if err := w.Release("abc1"); err != nil || !gone(p) {
-		t.Fatalf("Release = %v, gone %v", err, gone(p))
+	layer := w.CacheLayer("abc1") // issue 1921/S2: the cache layer lies beside the workspace and goes with it
+	if filepath.Dir(layer) != w.Dir || strings.HasPrefix(layer, p+"/") {
+		t.Fatalf("the cache layer %s does not lie beside the workspace %s", layer, p)
+	}
+	if err := os.MkdirAll(filepath.Join(layer, "kept", "0"), 0o700); err != nil {
+		t.Fatal(err)
+	}
+	if err := w.Release("abc1"); err != nil || !gone(p) || !gone(layer) {
+		t.Fatalf("Release = %v, gone %v, its cache layer gone %v", err, gone(p), gone(layer))
 	}
 	if err := w.Release("abc1"); err != nil {
 		t.Errorf("a second release = %v, want nil", err)
@@ -98,11 +106,21 @@ func TestSweepRemovesUnheldWorkspacesPastTheBound(t *testing.T) {
 	if err := os.WriteFile(filepath.Join(w.Dir, "not-a-workspace.txt"), nil, 0o600); err != nil {
 		t.Fatal(err)
 	}
+	// issue 1921/S2: a swept workspace's cache layer goes with it, and so does a layer whose workspace is gone; a held
+	// workspace's stays
+	for _, id := range []string{"old1", "held1", "orphan1"} {
+		if err := os.MkdirAll(w.CacheLayer(id), 0o700); err != nil {
+			t.Fatal(err)
+		}
+	}
 	done := w.Hold("held1")
 	removed := w.Sweep(time.Now(), 2*time.Hour)
 	if !gone(old) || gone(heldOld) || gone(recent) {
 		t.Errorf("after the sweep: old gone %v, held kept %v, recent kept %v", gone(old), !gone(heldOld), !gone(recent))
 	}
+	if !gone(w.CacheLayer("old1")) || !gone(w.CacheLayer("orphan1")) || gone(w.CacheLayer("held1")) {
+		t.Errorf("after the sweep: old1's layer gone %v, orphan1's gone %v, held1's kept %v", gone(w.CacheLayer("old1")), gone(w.CacheLayer("orphan1")), !gone(w.CacheLayer("held1")))
+	}
 	if len(removed) != 1 || removed[0] != "old1" {
 		t.Errorf("Sweep answered %v, want [old1]", removed)
 	}