From e1f58bbf9111c9f94c7449f707a8e6c8fbbf0305 Mon Sep 17 00:00:00 2001 From: Chandarith Chea <45872539+chandarith-chea@users.noreply.github.com> Date: Sat, 14 Nov 2026 14:13:37 +0700 Subject: [PATCH 1/2] dockivad: remove, copy and archive trees with 0555 and 000 directories The engine is the Mac user, not root, so a guest's chmod 555 or chmod 000 of a directory (Go's module cache is 0555) is enforced against the engine too. With such a directory holding files: - docker rmi removed image.json, then failed on the image's files ("unlinkat ...: permission denied"). The files stayed, and every docker images after it answered "Host operation failed" (lstat image.json). - docker commit failed: "mkdir /.commit-*/rootfs/...: permission denied". Builds left their work directories behind for the same reason. - docker cp and docker export of a tree with a 000 directory or file sent a truncated tar. restricted.go has what the engine now does everywhere it removes, copies or archives guest files: the directory or file gets the owner bits it lacks for as long as they are needed, and its own mode back after, so what is stored and exported keeps the guest's modes. - Removal: every removal of guest files is removeTree. An image's directory leaves the cache in one rename (.removing-*) before its files go, so the image is listed whole or not at all; what a failed removal leaves goes at the next start. A cache directory an earlier engine left without its image.json is skipped by docker images and docker rmi, and removed at start. - Copies: cloneTree unlocks an unreadable tree for clonefile, which copies a tree whole or not at all (EACCES), and locks source and copy again. applyUpper (commit) keeps the directories it fills writable and sets their modes last. Layer diffs, the build cache's hash and size, and build exports walk with walkTree and read through 000 directories of the base. - Archives: docker cp out, docker export and docker diff read 000 directories and files. docker cp into a container, the engine's own files and mount points, and a build's COPY write into a 0555 directory as Docker's root does. Not changed: a path given to docker cp that itself runs through a 000 directory is still "not found"; unpacked image layers and populated volumes keep the owner bits the engine adds; bind mounts are read as they are. restricted_test.go: docker rmi of an image with such directories and docker images beside a half-removed one; a commit's copy and its layer; docker cp out, docker export and docker cp in. All three fail on main. --- dockivad/api.go | 11 +- dockivad/archive.go | 12 +- dockivad/build.go | 7 +- dockivad/buildcache.go | 12 +- dockivad/buildexport.go | 14 +-- dockivad/buildkit.go | 3 +- dockivad/buildkit_build.go | 2 +- dockivad/buildmount.go | 2 +- dockivad/image.go | 38 ++++-- dockivad/layer.go | 19 ++- dockivad/main.go | 1 + dockivad/overlay.go | 82 +++++++++--- dockivad/restricted.go | 164 ++++++++++++++++++++++++ dockivad/restricted_test.go | 245 ++++++++++++++++++++++++++++++++++++ dockivad/system.go | 3 +- dockivad/volumes.go | 4 +- dockivad/x86.go | 2 +- 17 files changed, 553 insertions(+), 68 deletions(-) create mode 100644 dockivad/restricted.go create mode 100644 dockivad/restricted_test.go diff --git a/dockivad/api.go b/dockivad/api.go index 41626fb..a132005 100644 --- a/dockivad/api.go +++ b/dockivad/api.go @@ -117,6 +117,9 @@ func (b *engine) removeImage(ref string, force bool) []obj { continue } p := filepath.Join(b.Cache, entry.Name()) + if !exists(filepath.Join(p, "image.json")) { // half removed by an earlier engine: goes at the next start (migrateNames) + continue + } meta := readObject(filepath.Join(p, "image.json")) d := str(meta["digest"]) if digest == "" && strings.HasPrefix(d, "sha256:"+strings.TrimPrefix(ref, "sha256:")) { @@ -149,8 +152,7 @@ func (b *engine) removeImage(ref string, force bool) []obj { } out := []obj{} for p, name := range dirs { - retainLowers(b.storeDir(), b.State, p) // containers keep running over its files - check(os.RemoveAll(p)) + removeImageDir(b.storeDir(), b.State, p) // containers keep running over its files out = append(out, obj{"Untagged": name}) b.events.publish(imageEvent("untag", digest, digest)) // Docker names both by the image ID } @@ -343,7 +345,7 @@ func (b *engine) createWith(name string, body obj, prepared *image) obj { if filesDone != nil { <-filesDone } - _ = os.RemoveAll(stage) + _ = removeTree(stage) } }() allowed := map[string]bool{} @@ -903,8 +905,7 @@ func (b *engine) api(authorized func(*http.Request) bool) http.Handler { check(e) rows := []any{} for _, entry := range entries { - if entry.IsDir() && len(entry.Name()) == 64 { - p := filepath.Join(b.Cache, entry.Name(), "image.json") + if p := filepath.Join(b.Cache, entry.Name(), "image.json"); entry.IsDir() && len(entry.Name()) == 64 && exists(p) { meta := readObject(p) if !imageFilter(str(meta["name"]), filters) { continue diff --git a/dockivad/archive.go b/dockivad/archive.go index 68df097..832b496 100644 --- a/dockivad/archive.go +++ b/dockivad/archive.go @@ -179,7 +179,7 @@ func tarEntry(out *tar.Writer, name, p string, info os.FileInfo) error { return e } if info.Mode().IsRegular() { - src, e := os.OpenFile(p, os.O_RDONLY|syscall.O_NOFOLLOW, 0) + src, e := openGuest(p) check(e) defer src.Close() _, e = io.CopyN(out, src, h.Size) @@ -239,7 +239,9 @@ func (b *engine) changes(name string) []obj { } var visit func(rel string) bool visit = func(rel string) bool { - entries, e := os.ReadDir(filepath.Join(r.upper, filepath.FromSlash(rel))) + dir := filepath.Join(r.upper, filepath.FromSlash(rel)) + defer unlock(dir, 0o500)() + entries, e := os.ReadDir(dir) check(e) changed := false for _, entry := range entries { @@ -311,7 +313,8 @@ func (b *engine) putArchive(name, requested string, body io.Reader, noOverwriteD if to.files != nil { to.files.remove(to.rel) } else { - check(os.RemoveAll(dest)) + defer unlock(filepath.Dir(dest), 0o700)() + check(removeTree(dest)) } exists = false } @@ -323,6 +326,9 @@ func (b *engine) putArchive(name, requested string, body io.Reader, noOverwriteD } defer to.files.settle(to.rel, h.Typeflag == tar.TypeDir) } + // Docker's root writes into a directory without write permission: it has it until + // the archive is in (restricted.go). + defer unlock(filepath.Dir(dest), 0o700)() mode := tarMode(h) switch h.Typeflag { case tar.TypeDir: diff --git a/dockivad/build.go b/dockivad/build.go index 16a5ea5..2232321 100644 --- a/dockivad/build.go +++ b/dockivad/build.go @@ -117,7 +117,7 @@ func (b *engine) build(w http.ResponseWriter, r *http.Request, q url.Values) { }() work, e := os.MkdirTemp(private(filepath.Join(b.storeDir(), "build")), "b-") check(e) - defer func() { go os.RemoveAll(work) }() + defer func() { go removeTree(work) }() quiet := q.Get("q") == "1" || q.Get("q") == "true" var memory int64 if raw := q.Get("memory"); raw != "" && raw != "0" { // docker build -m @@ -707,7 +707,7 @@ func (bd *builder) runStep(st *buildStage, o runOptions) { func (bd *builder) discardRoot(st *buildStage) { old := filepath.Join(bd.work, "old-"+token()[:12]) check(os.Rename(st.root, old)) - go os.RemoveAll(old) + go removeTree(old) } // The engine writes these into each container; a build keeps the stage's own. @@ -920,7 +920,7 @@ func isGuestDir(root, guest string) bool { // copyIntoGuest copies a directory's contents into a guest directory, merging. func copyIntoGuest(src, root, dest string, mode os.FileMode, chmod bool) { guestDir(root, dest) - check(filepath.WalkDir(src, func(p string, d fs.DirEntry, err error) error { + check(walkTree(src, func(p string, d fs.DirEntry, err error) error { check(err) rel, e := filepath.Rel(src, p) check(e) @@ -948,6 +948,7 @@ func copyFileTo(src, root, guest string, mode os.FileMode, chmod bool) { parent, base := path.Split(path.Clean("/" + guest)) reject(base != "", "Cannot copy a file onto /") dst := filepath.Join(guestDir(root, parent), base) + defer unlock(filepath.Dir(dst), 0o700)() if old, e := os.Lstat(dst); e == nil { reject(!old.IsDir(), "Cannot replace the directory "+guest+" with a file") check(os.Remove(dst)) diff --git a/dockivad/buildcache.go b/dockivad/buildcache.go index ea5c33b..fbc68c2 100644 --- a/dockivad/buildcache.go +++ b/dockivad/buildcache.go @@ -97,7 +97,7 @@ func (bd *builder) store(st *buildStage, in instruction, id, dir string) { _ = safeRun(func() { tmp, e := os.MkdirTemp(bd.work, "cache-") check(e) - defer os.RemoveAll(tmp) + defer removeTree(tmp) cloneTree(st.root, filepath.Join(tmp, "rootfs")) now := time.Now().UTC().Format(time.RFC3339Nano) save(filepath.Join(tmp, "config.json"), st.config) @@ -175,7 +175,7 @@ func (bd *builder) release() { // changed, so the sum overstates the disk used. Ask APFS for private sizes if it matters. func changedSize(root, base string) int64 { var n int64 - check(filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + check(walkTree(root, func(p string, d fs.DirEntry, err error) error { check(err) if !d.Type().IsRegular() { return nil @@ -185,7 +185,7 @@ func changedSize(root, base string) int64 { if base != "" { rel, e := filepath.Rel(root, p) check(e) - if old, e := os.Lstat(filepath.Join(base, rel)); e == nil && old.Mode().IsRegular() && old.Size() == info.Size() && old.ModTime().Equal(info.ModTime()) { + if old, e := lstatIn(base, rel); e == nil && old.Mode().IsRegular() && old.Size() == info.Size() && old.ModTime().Equal(info.ModTime()) { return nil } } @@ -241,7 +241,7 @@ func treeHash(root string) string { // hashTree writes each entry's name, mode, size and content or symlink target. func hashTree(h io.Writer, root string) { - check(filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + check(walkTree(root, func(p string, d fs.DirEntry, err error) error { check(err) rel, e := filepath.Rel(root, p) check(e) @@ -254,7 +254,7 @@ func hashTree(h io.Writer, root string) { check(e) fmt.Fprintf(h, "%q", target) case info.Mode().IsRegular(): - f, e := os.OpenFile(p, os.O_RDONLY|syscall.O_NOFOLLOW, 0) + f, e := openGuest(p) check(e) defer f.Close() _, e = io.CopyN(h, f, info.Size()) @@ -315,5 +315,5 @@ func (b *engine) discard(dir string) { trash, e := os.MkdirTemp(private(filepath.Join(b.storeDir(), "build")), "trash-") check(e) check(os.Rename(dir, filepath.Join(trash, "entry"))) - go os.RemoveAll(trash) + go removeTree(trash) } diff --git a/dockivad/buildexport.go b/dockivad/buildexport.go index be1003b..bec04de 100644 --- a/dockivad/buildexport.go +++ b/dockivad/buildexport.go @@ -52,7 +52,7 @@ func (w bytesWriter) Write(p []byte) (int, error) { func exportTar(c *grpcCall, root string) int64 { counted := &counter{} tw := tar.NewWriter(io.MultiWriter(bytesWriter{c}, counted)) - check(filepath.WalkDir(root, func(p string, d fs.DirEntry, e error) error { + check(walkTree(root, func(p string, d fs.DirEntry, e error) error { if e != nil || p == root { return e } @@ -78,7 +78,7 @@ func exportTree(c *grpcCall, root string) int64 { listed := make(chan error, 1) go func() { listed <- safeRun(func() { - check(filepath.WalkDir(root, func(p string, d fs.DirEntry, e error) error { + check(walkTree(root, func(p string, d fs.DirEntry, e error) error { if e != nil || p == root { return e } @@ -101,7 +101,7 @@ func exportTree(c *grpcCall, root string) int64 { case !mode.IsDir(): // devices, pipes and sockets stay behind return nil } - files = append(files, p) // read by the receive loop only after the listing (listed) + files = append(files, rel) // read by the receive loop only after the listing (listed) return c.send(pbuf{}.num(1, packetStat).msg(2, st)) })) check(c.send(pbuf{}.num(1, packetStat))) // the end of the listing @@ -132,14 +132,14 @@ func exportTree(c *grpcCall, root string) int64 { waitListing() // the files' numbers are only final once listed id := p.num(3) reject(id < uint64(len(files)), "The client asked for an unknown file") - total += sendFile(c, uint32(id), files[id]) + total += sendFile(c, uint32(id), root, files[id]) } } } -func sendFile(c *grpcCall, id uint32, p string) int64 { - f, e := os.Open(p) - check(e) +func sendFile(c *grpcCall, id uint32, root, rel string) int64 { + var f *os.File + check(through(root, rel, func(p string) (e error) { f, e = openGuest(p); return })) // restricted.go defer f.Close() buf := make([]byte, exportChunk) var n int64 diff --git a/dockivad/buildkit.go b/dockivad/buildkit.go index 179e561..8aeece4 100644 --- a/dockivad/buildkit.go +++ b/dockivad/buildkit.go @@ -25,7 +25,6 @@ import ( "net" "net/http" "net/url" - "os" "slices" "sort" "strconv" @@ -627,7 +626,7 @@ func (b *engine) bkControlSolve(ctx context.Context, req pbFields) pbuf { bkSolves.Unlock() st.mu.Lock() if st.llb.work != "" { - go os.RemoveAll(st.llb.work) + go removeTree(st.llb.work) } st.mu.Unlock() }) diff --git a/dockivad/buildkit_build.go b/dockivad/buildkit_build.go index 82de255..03fbaf2 100644 --- a/dockivad/buildkit_build.go +++ b/dockivad/buildkit_build.go @@ -36,7 +36,7 @@ func (b *engine) bkBuild(ctx context.Context, st *bkSolve, opts map[string]strin } work, e := os.MkdirTemp(private(filepath.Join(b.storeDir(), "build")), "b-") check(e) - defer func() { go os.RemoveAll(work) }() + defer func() { go removeTree(work) }() bd := &builder{b: b, ctx: ctx, work: work, context: filepath.Join(work, "context"), network: opts["force-network-mode"], args: map[string]string{}, used: map[string]bool{}, global: platformArgs(platform), platform: platform, nocache: nocache, named: map[string]namedContext{}} bd.secret = func(id string) (value []byte, e error) { // RUN --mount=type=secret: the session's Secrets service diff --git a/dockivad/buildmount.go b/dockivad/buildmount.go index dccf091..c296f84 100644 --- a/dockivad/buildmount.go +++ b/dockivad/buildmount.go @@ -301,7 +301,7 @@ func (bd *builder) mountSources(mounts []runMount) (binds []any, env []string, t scratch := func() string { d, e := os.MkdirTemp(bd.work, "mount-") check(e) - undo = append(undo, func() { os.RemoveAll(d) }) // at once: secrets are among them + undo = append(undo, func() { removeTree(d) }) // at once: secrets are among them return d } bind := func(source, target string, readonly bool) { diff --git a/dockivad/image.go b/dockivad/image.go index 2ee21fc..111289c 100644 --- a/dockivad/image.go +++ b/dockivad/image.go @@ -401,7 +401,10 @@ func layerDir(root, dir string, create bool) (string, bool) { if !create { return "", false } - check(mkdirGuest(p)) + func() { + defer unlock(filepath.Dir(p), 0o700)() + check(mkdirGuest(p)) + }() continue } check(e) @@ -416,7 +419,8 @@ func layerDir(root, dir string, create bool) (string, bool) { // cloneTree copies an unpacked root for a container: one APFS clone of the whole // tree (clonefileat), sharing blocks until written; a plain copy elsewhere. A clone // turns setuid and setgid off on files and directories (Debian's /var/mail is -// 2775), so they are set again. +// 2775), so they are set again. The copy has the modes of src, also where they keep +// the engine from reading it. func cloneTree(src, dst string) { s, e := syscall.BytePtrFromString(src) check(e) @@ -424,7 +428,18 @@ func cloneTree(src, dst string) { check(e) cwd := -2 // AT_FDCWD const sysClonefileat, cloneNofollow = 462, 1 - if _, _, errno := syscall.Syscall6(sysClonefileat, uintptr(cwd), uintptr(unsafe.Pointer(s)), uintptr(cwd), uintptr(unsafe.Pointer(d)), cloneNofollow, 0); errno == 0 { + clone := func() syscall.Errno { + _, _, errno := syscall.Syscall6(sysClonefileat, uintptr(cwd), uintptr(unsafe.Pointer(s)), uintptr(cwd), uintptr(unsafe.Pointer(d)), cloneNofollow, 0) + return errno + } + errno := clone() + var held []locked + defer func() { lockTree(dst, held); lockTree(src, held) }() + if errno == syscall.EACCES { // a directory or file the guest left unreadable (restricted.go): nothing is copied + unlockTree(src, &held) + errno = clone() + } + if errno == 0 { // ponytail: one lstat per entry (~100 ms for 25k files); a list of an image's // setuid files, kept at unpack, would spare it. check(filepath.WalkDir(src, func(p string, entry fs.DirEntry, err error) error { @@ -582,7 +597,7 @@ func commitImage(cache, name, layout string) string { } stage, e := os.MkdirTemp(cache, ".incoming-") check(e) - defer os.RemoveAll(stage) + defer removeTree(stage) dest := filepath.Join(stage, "layout") check(os.MkdirAll(filepath.Join(dest, "blobs/sha256"), 0700)) for _, d := range im.blobs { @@ -640,14 +655,20 @@ func (b *engine) commitPulled(name, layout string) string { // migrateNames moves images cached under a docker.io/ name (docker tag did, before // commitImage shortened names) to the familiar name imagePath looks up. One whose -// familiar name is taken was unreachable anyway, and goes. +// familiar name is taken was unreachable anyway, and goes. So does a cache directory +// without its image.json: docker rmi removed that first, then failed on the image's +// files where a directory lacked write permission, and listing images failed on it. func migrateNames(cache string) { entries, e := os.ReadDir(cache) check(e) for _, entry := range entries { _ = safeRun(func() { dir := filepath.Join(cache, entry.Name()) - if len(entry.Name()) != 64 || !exists(filepath.Join(dir, "image.json")) { + if len(entry.Name()) != 64 || !entry.IsDir() { + return + } + if !exists(filepath.Join(dir, "image.json")) { // left by a removal that failed half way + removeImageDir(filepath.Dir(cache), filepath.Join(filepath.Dir(cache), "containers"), dir) return } meta := readObject(filepath.Join(dir, "image.json")) @@ -657,8 +678,7 @@ func migrateNames(cache string) { } target := filepath.Join(cache, key(shortName(name))) if exists(target) { - retainLowers(filepath.Dir(cache), filepath.Join(filepath.Dir(cache), "containers"), dir) - check(os.RemoveAll(dir)) + removeImageDir(filepath.Dir(cache), filepath.Join(filepath.Dir(cache), "containers"), dir) return } meta["name"] = shortName(name) @@ -676,7 +696,7 @@ func ensureUnpacked(target string) { im := loadImage(noLinks(target, "layout")) stage, e := os.MkdirTemp(target, ".rootfs-") check(e) - defer os.RemoveAll(stage) + defer removeTree(stage) size := im.unpack(stage) if e := os.Rename(stage, filepath.Join(target, "rootfs")); e != nil && !exists(filepath.Join(target, "rootfs")) { check(e) diff --git a/dockivad/layer.go b/dockivad/layer.go index 1ecdac6..1c56010 100644 --- a/dockivad/layer.go +++ b/dockivad/layer.go @@ -40,18 +40,18 @@ func writeDiffLayer(root, base, blobs string) (digest string, size int64, change check(out.WriteHeader(h)) changed = true } - link := func(p string) string { target, e := os.Readlink(p); check(e); return target } + link := func(tree, rel string) string { target, e := readlinkIn(tree, rel); check(e); return target } if base != "" { // Whiteouts first. A directory root lacks, or holds as another type, is not // descended: its whiteout or replacement removes it whole. - check(filepath.WalkDir(base, func(p string, d fs.DirEntry, err error) error { + check(walkTree(base, func(p string, d fs.DirEntry, err error) error { check(err) rel, e := filepath.Rel(base, p) check(e) if rel == "." { return nil } - st, e := os.Lstat(filepath.Join(root, rel)) + st, e := lstatIn(root, rel) gone := os.IsNotExist(e) if gone { dir, name := filepath.Split(rel) @@ -68,7 +68,7 @@ func writeDiffLayer(root, base, blobs string) (digest string, size int64, change // fresh is a directory base lacks (or had as another type): all beneath it is new, // and base is not looked up there (its path may run through a file or symlink). fresh := "" - check(filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + check(walkTree(root, func(p string, d fs.DirEntry, err error) error { check(err) rel, e := filepath.Rel(root, p) check(e) @@ -79,11 +79,10 @@ func writeDiffLayer(root, base, blobs string) (digest string, size int64, change return nil // the root itself; sockets, FIFOs and devices } if base != "" && !strings.HasPrefix(rel, fresh+"/") { - was := filepath.Join(base, rel) - old, e := os.Lstat(was) + old, e := lstatIn(base, rel) if e == nil && old.Mode() == info.Mode() && (kind == fs.ModeDir || - kind == fs.ModeSymlink && link(was) == link(p) || + kind == fs.ModeSymlink && link(base, rel) == link(root, rel) || kind == 0 && old.Size() == info.Size() && old.ModTime().Equal(info.ModTime())) { return nil } @@ -99,13 +98,13 @@ func writeDiffLayer(root, base, blobs string) (digest string, size int64, change case fs.ModeDir: h.Name, h.Typeflag = rel+"/", tar.TypeDir case fs.ModeSymlink: - h.Typeflag, h.Linkname = tar.TypeSymlink, link(p) + h.Typeflag, h.Linkname = tar.TypeSymlink, link(root, rel) default: h.Size = info.Size() } add(h) if kind == 0 { - in, e := os.OpenFile(p, os.O_RDONLY|syscall.O_NOFOLLOW, 0) + in, e := openGuest(p) check(e) defer in.Close() _, e = io.CopyN(out, in, h.Size) @@ -137,7 +136,7 @@ func imageDocs(layout string) (manifest, config obj) { // treeSize is the total size of the regular files under root (docker images SIZE). func treeSize(root string) int64 { var n int64 - check(filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + check(walkTree(root, func(p string, d fs.DirEntry, err error) error { check(err) if d.Type().IsRegular() { info, e := d.Info() diff --git a/dockivad/main.go b/dockivad/main.go index 53addec..0c96a2d 100644 --- a/dockivad/main.go +++ b/dockivad/main.go @@ -95,6 +95,7 @@ func run() { moveToStore(b.Home, store) } cleanupUnpublished(b.State) + cleanupUnpublished(b.Cache) migrateNames(b.Cache) } if *importName != "" { diff --git a/dockivad/overlay.go b/dockivad/overlay.go index 4618e8e..c218cd9 100644 --- a/dockivad/overlay.go +++ b/dockivad/overlay.go @@ -158,9 +158,12 @@ func (r layers) upperDir(rel string, create bool) string { } else if !create { panic(apiError{404, "No such directory: /" + sofar}) } - if e := os.Mkdir(next, 0o700); e != nil && !errors.Is(e, fs.ErrExist) { - check(e) - } + func() { + defer unlock(p, 0o700)() + if e := os.Mkdir(next, 0o700); e != nil && !errors.Is(e, fs.ErrExist) { + check(e) + } + }() if from != nil { check(setMode(next, from.Mode()&permBits)) _ = os.Chtimes(next, from.ModTime(), from.ModTime()) @@ -194,6 +197,7 @@ func (r layers) settle(rel string, directory bool) { if !exists(wh) { return } + defer unlock(filepath.Dir(wh), 0o700)() if directory { f, e := os.OpenFile(filepath.Join(r.upper, dir, name, opaqueMarker), os.O_WRONLY|os.O_CREATE|syscall.O_NOFOLLOW, 0o600) check(e) @@ -210,12 +214,15 @@ func (r layers) remove(rel string) { reject(name != "", "Cannot remove the container root") up, low := r.lookup(rel) if low != "" { - f, e := os.OpenFile(filepath.Join(r.upperDir(dir, true), whiteoutPrefix+name), os.O_WRONLY|os.O_CREATE|syscall.O_NOFOLLOW, 0o600) + parent := r.upperDir(dir, true) + defer unlock(parent, 0o700)() + f, e := os.OpenFile(filepath.Join(parent, whiteoutPrefix+name), os.O_WRONLY|os.O_CREATE|syscall.O_NOFOLLOW, 0o600) check(e) f.Close() } if up != "" { - check(os.RemoveAll(up)) + defer unlock(filepath.Dir(up), 0o700)() + check(removeTree(up)) } } @@ -233,6 +240,7 @@ func (r layers) copyUp(rel string) string { check(e) reject(!st.IsDir(), "Cannot copy a directory up: /"+rel) dest := r.writable(rel) + defer unlock(filepath.Dir(dest), 0o700)() cloneTree(host, dest) if st.Mode().IsRegular() { check(setMode(dest, st.Mode()&permBits)) @@ -248,10 +256,32 @@ type guestEntry struct { // readDir lists directory rel as the guest sees it, sorted by name. func (r layers) readDir(rel string) []guestEntry { + out, lock := r.openDir(rel) + lock() + return out +} + +// openDir is readDir of a directory that stays readable (restricted.go) until lock +// is called: for a walk into it. +func (r layers) openDir(rel string) (out []guestEntry, lock func()) { up, low := r.lookup(rel) reject(up != "" || low != "", "No such directory: /"+rel) out, seen := []guestEntry{}, map[string]bool{} + locks := []func(){} + lock = func() { + for _, l := range locks { + l() + } + locks = nil + } + defer func() { + if v := recover(); v != nil { + lock() + panic(v) + } + }() add := func(dir string, upper bool) { + locks = append(locks, unlock(dir, 0o500)) entries, e := os.ReadDir(dir) check(e) for _, entry := range entries { @@ -280,7 +310,7 @@ func (r layers) readDir(rel string) []guestEntry { add(low, false) } sort.Slice(out, func(i, j int) bool { return out[i].name < out[j].name }) - return out + return out, lock } // walk visits rel and everything below it as the guest sees it, in lexical order @@ -304,7 +334,9 @@ func (r layers) walkFrom(rel, host string, info os.FileInfo, fn func(rel, host s } return e } - for _, entry := range r.readDir(rel) { + entries, lock := r.openDir(rel) + defer lock() + for _, entry := range entries { if e := r.walkFrom(path.Join(rel, entry.name), entry.host, entry.info, fn); e != nil { return e } @@ -321,6 +353,7 @@ func (r layers) writeFile(rel, content string) { reject(s.Mode().IsRegular() || s.Mode()&os.ModeSymlink != 0, "Cannot replace "+rel) } p := r.writable(rel) + defer unlock(filepath.Dir(p), 0o700)() if _, e := os.Lstat(p); e == nil { check(os.Remove(p)) } @@ -343,9 +376,12 @@ func (r layers) mountPoint(guest string, directory bool) { if !ok { if last && !directory { p := r.writable(sofar) - f, e := os.OpenFile(p, os.O_WRONLY|os.O_CREATE|os.O_EXCL|syscall.O_NOFOLLOW, 0o644) - check(e) - f.Close() + func() { + defer unlock(filepath.Dir(p), 0o700)() + f, e := os.OpenFile(p, os.O_WRONLY|os.O_CREATE|os.O_EXCL|syscall.O_NOFOLLOW, 0o644) + check(e) + f.Close() + }() r.settle(sofar, false) } else { r.upperDir(sofar, true) @@ -428,7 +464,7 @@ func (b *engine) commitContainer(name string, q url.Values) obj { } work, e := os.MkdirTemp(b.storeDir(), ".commit-") check(e) - defer os.RemoveAll(work) + defer removeTree(work) merged := filepath.Join(work, "rootfs") cloneTree(r.lower, merged) applyUpper(r.upper, merged) @@ -446,9 +482,16 @@ func (b *engine) commitContainer(name string, q url.Values) obj { // applyUpper lays an upper layer over a copy of its lower one: whiteouts remove, // opaque directories empty, the rest replaces (APFS clones). The engine's own files -// (engineFiles) stay out. +// (engineFiles) stay out. A directory the guest left without owner permissions +// (restricted.go) is writable while it is filled, and gets its mode after. func applyUpper(upper, merged string) { - check(filepath.WalkDir(upper, func(p string, d fs.DirEntry, err error) error { + type late struct { + path string + mode fs.FileMode + } + dirs := []late{} + defer unlock(merged, 0o700)() + check(walkTree(upper, func(p string, d fs.DirEntry, err error) error { check(err) rel, e := filepath.Rel(upper, p) check(e) @@ -461,12 +504,12 @@ func applyUpper(upper, merged string) { } case strings.HasPrefix(name, whiteoutPrefix+whiteoutPrefix): // the opaque marker (seen at its directory), a copy-up in flight case strings.HasPrefix(name, whiteoutPrefix): - check(os.RemoveAll(filepath.Join(filepath.Dir(dest), strings.TrimPrefix(name, whiteoutPrefix)))) + check(removeTree(filepath.Join(filepath.Dir(dest), strings.TrimPrefix(name, whiteoutPrefix)))) default: info, e := d.Info() check(e) if old, e := os.Lstat(dest); e == nil && !(old.IsDir() && d.IsDir()) || e == nil && exists(filepath.Join(p, opaqueMarker)) { - check(os.RemoveAll(dest)) + check(removeTree(dest)) } if !d.IsDir() { cloneTree(p, dest) @@ -478,8 +521,15 @@ func applyUpper(upper, merged string) { if e := os.Mkdir(dest, 0o700); e != nil && !errors.Is(e, fs.ErrExist) { check(e) } - check(setMode(dest, info.Mode()&permBits)) + mode := info.Mode() & permBits + check(setMode(dest, mode|0o700)) + if mode&0o700 != 0o700 { + dirs = append(dirs, late{dest, mode}) + } } return nil })) + for i := len(dirs) - 1; i >= 0; i-- { // children first + check(setMode(dirs[i].path, dirs[i].mode)) + } } diff --git a/dockivad/restricted.go b/dockivad/restricted.go new file mode 100644 index 0000000..0a4b70c --- /dev/null +++ b/dockivad/restricted.go @@ -0,0 +1,164 @@ +package main + +// Guest trees hold what the engine's own user may not read or change: guest root's +// chmod 555 of a directory (Go's module cache is made that way) or chmod 000 of a +// directory or file is the host mode too, and the engine is that host user, not root. +// To remove, copy or archive such a tree it gives the directory or file the owner +// bits it lacks for as long as it needs them, and its mode back after: what is stored +// and exported keeps the modes the guest set. A running container sees the wider mode +// for that moment, as it does while the runtime does the same for guest root +// (native/fs.h, fs_dac). +// +// ponytail: a path given to docker cp or a mount that itself runs through a 000 +// directory is still "not found" (layers.lookup); unlock on the way if it matters. + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" + "syscall" +) + +// unlock gives directory or file p the owner permission bits of need it lacks, and +// returns what puts its mode back: defer unlock(dir, 0o700)() around making or +// removing entries of dir, 0o500 around reading it. +func unlock(p string, need fs.FileMode) func() { + st, e := os.Lstat(p) + if e != nil || st.Mode()&fs.ModeSymlink != 0 { + return func() {} + } + return unlockAs(p, st, need) +} + +func unlockAs(p string, st fs.FileInfo, need fs.FileMode) func() { + mode := st.Mode() & permBits + if mode&need == need || setMode(p, mode|need) != nil { + return func() {} + } + return func() { _ = setMode(p, mode) } +} + +// walkTree is filepath.WalkDir for a guest tree. A directory without owner read and +// search permission is readable from before fn sees it (with its own mode in d) until +// the walk has left it, also when fn panics. +func walkTree(root string, fn fs.WalkDirFunc) error { + type held struct { + prefix string + lock func() + } + var open []held + leave := func(p string) { // lock again the directories p is not in + for len(open) > 0 && !strings.HasPrefix(p, open[len(open)-1].prefix) { + open[len(open)-1].lock() + open = open[:len(open)-1] + } + } + defer leave("") + return filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + leave(p) + if err != nil || !d.IsDir() { + return fn(p, d, err) + } + info, e := d.Info() + if e != nil { + return fn(p, d, e) + } + if info.Mode().Perm()&0o500 != 0o500 { + open = append(open, held{p + string(filepath.Separator), unlockAs(p, info, 0o500)}) + } + return fn(p, fs.FileInfoToDirEntry(info), nil) + }) +} + +// openGuest opens a guest's regular file to read it; one without read permission +// (Fedora's /etc/shadow is 000) has it for the open. +func openGuest(p string) (*os.File, error) { + f, e := os.OpenFile(p, os.O_RDONLY|syscall.O_NOFOLLOW, 0) + if !errors.Is(e, fs.ErrPermission) { + return f, e + } + if st, e := os.Lstat(p); e != nil || !st.Mode().IsRegular() { + return nil, &fs.PathError{Op: "open", Path: p, Err: fs.ErrPermission} + } + defer unlock(p, 0o400)() + return os.OpenFile(p, os.O_RDONLY|syscall.O_NOFOLLOW, 0) +} + +// through runs fn on rel of guest tree root; if a directory on the way has no search +// permission (fn answers EACCES), once more with them searchable. For a tree looked +// up beside the one being walked (a layer's base). +func through(root, rel string, fn func(p string) error) error { + p := filepath.Join(root, rel) + e := fn(p) + if !errors.Is(e, fs.ErrPermission) { + return e + } + dir := root + for _, part := range strings.Split(filepath.Dir(rel), string(filepath.Separator)) { + dir = filepath.Join(dir, part) + defer unlock(dir, 0o100)() + } + return fn(p) +} + +func lstatIn(root, rel string) (st os.FileInfo, e error) { + e = through(root, rel, func(p string) error { st, e = os.Lstat(p); return e }) + return +} + +func readlinkIn(root, rel string) (target string, e error) { + e = through(root, rel, func(p string) error { target, e = os.Readlink(p); return e }) + return +} + +// locked is an entry unlockTree made readable, with the mode it had. +type locked struct { + rel string + mode fs.FileMode +} + +// unlockTree makes every directory and regular file of a guest tree readable, for +// clonefile, which copies a tree whole or not at all; lockTree puts the modes back, +// in the tree or a copy of it. +func unlockTree(root string, held *[]locked) { + check(filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + check(err) + need := fs.FileMode(0o400) + if d.IsDir() { + need = 0o500 + } else if !d.Type().IsRegular() { + return nil + } + info, e := d.Info() + check(e) + if mode := info.Mode() & permBits; mode&need != need { + rel, e := filepath.Rel(root, p) + check(e) + check(setMode(p, mode|need)) // before WalkDir reads it + *held = append(*held, locked{rel, mode}) + } + return nil + })) +} + +func lockTree(root string, held []locked) { + for i := len(held) - 1; i >= 0; i-- { + _ = setMode(filepath.Join(root, held[i].rel), held[i].mode) + } +} + +// removeImageDir removes cached image directory dir, as docker rmi does. Its unpacked +// files stay for the containers running over them (retainLowers); the directory +// leaves the cache in one rename, so the image is listed whole or not at all, and what +// is left of a removal that fails goes at the next start (cleanupUnpublished). +func removeImageDir(store, state, dir string) { + retainLowers(store, state, dir) + gone := filepath.Join(filepath.Dir(dir), ".removing-"+token()[:16]) + check(os.Rename(dir, gone)) + if e := removeTree(gone); e != nil { + fmt.Fprintln(os.Stderr, "dockivad: files of a removed image stay until the next start:", e) + } +} diff --git a/dockivad/restricted_test.go b/dockivad/restricted_test.go new file mode 100644 index 0000000..5068450 --- /dev/null +++ b/dockivad/restricted_test.go @@ -0,0 +1,245 @@ +package main + +import ( + "archive/tar" + "bytes" + "fmt" + "io" + "io/fs" + "net/http/httptest" + "os" + "path/filepath" + "sort" + "strings" + "testing" +) + +// Guest trees with directories the engine's user may not change or read: 0555 (Go's +// module cache; guest root's chmod) and 000, each holding files. + +// restrictedTree makes, in dir (made if missing): +// +// m/a/f m and a 0555 +// z/k/f z 000, k 0555 +// s a 000 file +func restrictedTree(t *testing.T, dir string) { + t.Helper() + for _, d := range []string{"m/a", "z/k"} { + check(os.MkdirAll(filepath.Join(dir, d), 0o755)) + check(os.WriteFile(filepath.Join(dir, d, "f"), []byte(d), 0o644)) + } + check(os.WriteFile(filepath.Join(dir, "s"), []byte("secret"), 0o600)) + for _, rel := range []string{"m/a", "m", "z/k"} { // children first + check(os.Chmod(filepath.Join(dir, rel), 0o555)) + } + for _, rel := range []string{"z", "s"} { + check(os.Chmod(filepath.Join(dir, rel), 0)) + } +} + +// unlockLater lets the test's temporary directory go: t.TempDir's own removal stops +// at such directories. +func unlockLater(t *testing.T, dir string) { + t.Cleanup(func() { + _ = filepath.WalkDir(dir, func(p string, d fs.DirEntry, e error) error { + if e == nil && d.IsDir() { + _ = os.Chmod(p, 0o700) + } + return nil + }) + }) +} + +// treeModes lists a tree as "rel mode [content]" lines, reading into directories +// without permission (their modes are put back). +func treeModes(t *testing.T, root string) string { + t.Helper() + lines := []string{} + var visit func(rel string) + visit = func(rel string) { + p := filepath.Join(root, rel) + st, e := os.Lstat(p) + check(e) + line := fmt.Sprintf("%s %v", rel, st.Mode()) + switch { + case st.Mode().IsRegular(): + check(os.Chmod(p, 0o600)) + data, e := os.ReadFile(p) + check(e) + check(os.Chmod(p, st.Mode().Perm())) + line += " " + string(data) + case st.IsDir(): + check(os.Chmod(p, 0o700)) + entries, e := os.ReadDir(p) + check(e) + for _, entry := range entries { + visit(filepath.Join(rel, entry.Name())) + } + check(os.Chmod(p, st.Mode().Perm())) + } + lines = append(lines, line) + } + visit(".") + sort.Strings(lines) + return strings.Join(lines, "\n") +} + +const restrictedModes = `. drwxr-xr-x +m dr-xr-xr-x +m/a dr-xr-xr-x +m/a/f -rw-r--r-- m/a +s ---------- secret +z d--------- +z/k dr-xr-xr-x +z/k/f -rw-r--r-- z/k` + +// docker rmi of an image holding such directories removes all of it, and a cache +// directory an earlier engine left half removed (no image.json) breaks nothing. +func TestRemoveImageRestrictedDirs(t *testing.T) { + b := testEngine(t) + unlockLater(t, b.Home) + h := b.socketHandler() + dir := b.imagePath("dockiva/shell:demo") + restrictedTree(t, filepath.Join(dir, "rootfs", "data")) + half := filepath.Join(b.Cache, strings.Repeat("0", 64)) + restrictedTree(t, filepath.Join(half, "rootfs")) + if r := request(t, h, "GET", "/images/json", "", ""); r.Code != 200 || !strings.Contains(r.Body.String(), "dockiva/shell:demo") { + t.Fatalf("images beside a half-removed one: %d %s", r.Code, r.Body.String()) + } + if r := request(t, h, "DELETE", "/images/dockiva/shell:demo", "", ""); r.Code != 200 || !strings.Contains(r.Body.String(), "Deleted") { + t.Fatalf("rmi: %d %s", r.Code, r.Body.String()) + } + if r := request(t, h, "GET", "/images/json", "", ""); r.Code != 200 || strings.TrimSpace(r.Body.String()) != "[]" { + t.Fatalf("images after rmi: %d %s", r.Code, r.Body.String()) + } + cleanupUnpublished(b.Cache) + migrateNames(b.Cache) // the next start: the half-removed image goes + if entries, _ := os.ReadDir(b.Cache); len(entries) != 0 { + t.Fatalf("left in the image cache: %v", entries) + } +} + +// docker commit's copy: the image's files with the container's laid over them. Both +// hold such directories; the copy has their modes and files, the sources keep theirs, +// and the layer written from it lists them. +func TestCommitCopyRestrictedDirs(t *testing.T) { + dir := t.TempDir() + unlockLater(t, dir) + lower, upper, merged, blobs := filepath.Join(dir, "lower"), filepath.Join(dir, "upper"), filepath.Join(dir, "work", "rootfs"), filepath.Join(dir, "blobs") + for _, d := range []string{lower, filepath.Join(upper, "m/a"), filepath.Join(upper, "root"), filepath.Dir(merged), blobs} { + check(os.MkdirAll(d, 0o755)) + } + restrictedTree(t, lower) + // The container: a file added to, and one removed from, the image's 0555 and 000 + // directories, and directories of its own. + check(os.WriteFile(filepath.Join(upper, "m/a/new"), []byte("new"), 0o644)) + check(os.WriteFile(filepath.Join(upper, "m/a", whiteoutPrefix+"f"), nil, 0o600)) + check(os.MkdirAll(filepath.Join(upper, "z"), 0o755)) + check(os.WriteFile(filepath.Join(upper, "z", whiteoutPrefix+"k"), nil, 0o600)) + for _, rel := range []string{"m/a", "m"} { + check(os.Chmod(filepath.Join(upper, rel), 0o555)) + } + check(os.Chmod(filepath.Join(upper, "z"), 0)) + restrictedTree(t, filepath.Join(upper, "root")) + before := treeModes(t, lower) + "\n\n" + treeModes(t, upper) + + cloneTree(lower, merged) + applyUpper(upper, merged) + + own := strings.ReplaceAll(strings.Replace(restrictedModes, ". drwxr-xr-x\n", "", 1), "\n", "\nroot/") + want := ". drwxr-xr-x\nm dr-xr-xr-x\nm/a dr-xr-xr-x\nm/a/new -rw-r--r-- new\nroot drwxr-xr-x\nroot/" + own + "\ns ---------- secret\nz d---------" + if got := treeModes(t, merged); got != want { + t.Fatalf("merged:\n%s\nwant:\n%s", got, want) + } + if after := treeModes(t, lower) + "\n\n" + treeModes(t, upper); after != before { + t.Fatalf("the sources changed:\n%s\nwere:\n%s", after, before) + } + digest, size, changed := writeDiffLayer(merged, lower, blobs) + _, headers := layerHeaders(t, blobs, digest, size) + got := []string{} + for _, h := range headers { + got = append(got, fmt.Sprintf("%s %o", h.Name, h.Mode)) + } + sort.Strings(got) + wantLayer := "m/a/.wh.f 0, m/a/new 644, root/ 755, root/m/ 555, root/m/a/ 555, root/m/a/f 644, root/s 0, root/z/ 0, root/z/k/ 555, root/z/k/f 644, z/.wh.k 0" + if !changed || strings.Join(got, ", ") != wantLayer { + t.Fatalf("layer:\n%s\nwant:\n%s", strings.Join(got, ", "), wantLayer) + } + if size := treeSize(merged); size != int64(len("new")+2*len("secret")+2*len("m/a")) { + t.Fatalf("size: %d", size) + } + if e := removeTree(filepath.Dir(merged)); e != nil { + t.Fatal(e) + } +} + +// docker cp and docker export read such directories and files, with their modes, +// and docker cp into a 0555 directory works as it does for Docker's root. +func TestArchiveRestrictedDirs(t *testing.T) { + b := testEngine(t) + unlockLater(t, b.Home) + b.create("box", decode([]byte(`{"Image":"dockiva/shell:demo"}`), maxJSON)) + upper := filepath.Join(b.State, "box", "rootfs") + restrictedTree(t, filepath.Join(upper, "data")) + before := treeModes(t, filepath.Join(upper, "data")) + entries := func(r *httptest.ResponseRecorder, prefix string) string { + t.Helper() + lines := []string{} + for in := tar.NewReader(r.Body); ; { + h, e := in.Next() + if e == io.EOF { + break + } + if e != nil { + t.Fatalf("archive cut short after %v: %v", lines, e) + } + if name, ok := strings.CutPrefix(h.Name, prefix); ok { + data, e := io.ReadAll(in) + check(e) + lines = append(lines, strings.TrimSpace(fmt.Sprintf("%s %o %s", name, h.Mode, data))) + } + } + return strings.Join(lines, ", ") + } + want := "/ 755, /m/ 555, /m/a/ 555, /m/a/f 644 m/a, /s 0 secret, /z/ 0, /z/k/ 555, /z/k/f 644 z/k" + r := httptest.NewRecorder() + b.getArchive(r, "box", "/data", false) + if got := entries(r, "data"); got != want { + t.Fatalf("docker cp:\n%s\nwant:\n%s", got, want) + } + r = httptest.NewRecorder() + b.export(r, "box") + if got := entries(r, "data"); got != want { + t.Fatalf("docker export:\n%s", got) + } + if after := treeModes(t, filepath.Join(upper, "data")); after != before { + t.Fatalf("the container's files changed:\n%s\nwere:\n%s", after, before) + } + + // Into the container: a file beside m/a/f, a directory replacing it, and a file in + // a directory of the image the container has no copy of yet. + image := filepath.Join(b.imagePath("dockiva/shell:demo"), "rootfs") + check(os.Mkdir(filepath.Join(image, "ro"), 0o555)) + var in bytes.Buffer + w := tar.NewWriter(&in) + for _, h := range []*tar.Header{{Name: "data/m/a/new", Typeflag: tar.TypeReg, Mode: 0o644, Size: 2}, {Name: "data/m/a/f/", Typeflag: tar.TypeDir, Mode: 0o555}, + {Name: "data/m/a/f/in", Typeflag: tar.TypeReg, Mode: 0o600, Size: 2}, {Name: "ro/x", Typeflag: tar.TypeReg, Mode: 0o644, Size: 2}} { + check(w.WriteHeader(h)) + if h.Size > 0 { + _, _ = w.Write([]byte("hi")) + } + } + check(w.Close()) + b.putArchive("box", "/", &in, false) + want = strings.Replace(restrictedModes, "m/a/f -rw-r--r-- m/a", "m/a/f dr-xr-xr-x\nm/a/f/in -rw------- hi\nm/a/new -rw-r--r-- hi", 1) + if got := treeModes(t, filepath.Join(upper, "data")); got != want { + t.Fatalf("after docker cp into it:\n%s\nwant:\n%s", got, want) + } + if got := treeModes(t, filepath.Join(upper, "ro")); got != ". dr-xr-xr-x\nx -rw-r--r-- hi" { + t.Fatalf("copied-up directory:\n%s", got) + } + b.remove("box") + if exists(filepath.Join(b.State, "box")) { + t.Fatal("the container's files stay") + } +} diff --git a/dockivad/system.go b/dockivad/system.go index b8eff94..e1ac8ef 100644 --- a/dockivad/system.go +++ b/dockivad/system.go @@ -213,8 +213,7 @@ func (b *engine) pruneImages(danglingOnly bool, keep func(time.Time, obj) bool) if !keep(c.created, labels) { continue } - retainLowers(b.storeDir(), b.State, c.dir) - check(os.RemoveAll(c.dir)) + removeImageDir(b.storeDir(), b.State, c.dir) removed++ if !strings.HasPrefix(c.name, ":") { deleted = append(deleted, obj{"Untagged": c.name}) diff --git a/dockivad/volumes.go b/dockivad/volumes.go index d29ea4a..4015798 100644 --- a/dockivad/volumes.go +++ b/dockivad/volumes.go @@ -110,7 +110,7 @@ func (b *engine) removeVolume(name string) { if users := b.volumeUsers(name); len(users) > 0 { panic(apiError{409, "Volume " + name + " is in use by " + strings.Join(users, ", ")}) } - check(os.RemoveAll(filepath.Join(volumesDir(b.Home), name))) + check(removeTree(filepath.Join(volumesDir(b.Home), name))) } // removeAnonymousVolumes deletes the anonymous volumes a removed container used @@ -550,7 +550,7 @@ func copyTree(root layers, from, to string) { check(os.Mkdir(dest, 0o700)) check(setMode(dest, info.Mode()&permBits|0o700)) case info.Mode().IsRegular(): - in, e := os.Open(p) + in, e := openGuest(p) check(e) defer in.Close() out, e := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_EXCL|syscall.O_NOFOLLOW, info.Mode().Perm()) diff --git a/dockivad/x86.go b/dockivad/x86.go index 6223ae2..87882d2 100644 --- a/dockivad/x86.go +++ b/dockivad/x86.go @@ -120,7 +120,7 @@ func removeFexState(root string, made []string) { return } if path.Base(rel) == "fex-emu" { - check(os.RemoveAll(host)) + check(removeTree(host)) } else { _ = os.Remove(host) // a directory still holding files stays } From b28b50515c89efb15e66ae7dac4a540bd8d9cf9d Mon Sep 17 00:00:00 2001 From: Chandarith Chea <45872539+chandarith-chea@users.noreply.github.com> Date: Sat, 14 Nov 2026 15:01:19 +0700 Subject: [PATCH 2/2] dockivad: a start removes the work earlier engines left A build's work directory stayed in /build whenever one of its stages held a directory without write permission (os.RemoveAll in the background, its error dropped): 21 were left on an installed 0.6.10, and 5 more in /build from before the store. The previous commit stops new ones; nothing removed the old. At a start, before the API serves anything but /_ping, staleWork renames aside what an engine that has gone left in the store, and in the home before it: build, load, .commit-* and what an earlier sweep did not finish (.removing-*). They are removed in the background with removeTree, so a start does not wait for gigabytes to go; what an interrupted removal leaves goes at the next start. TestStaleWorkRestrictedDirs: such leftovers with 0555 and 000 directories go, images, containers, the build cache and volumes stay. --- dockivad/main.go | 33 +++++++++++++++++++++++++++++++++ dockivad/restricted_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/dockivad/main.go b/dockivad/main.go index 0c96a2d..80a661f 100644 --- a/dockivad/main.go +++ b/dockivad/main.go @@ -26,6 +26,28 @@ func cleanupUnpublished(state string) { } } +// staleWork moves aside what an engine that has gone left in dir, the store or the +// home before it: its builds' and loads' work (build, load) and its commits' (.commit-*), +// and what an earlier sweep did not finish. It returns what to remove. Builds left +// their work behind whenever a stage held a directory without write permission (Go's +// module cache; restricted.go), and nothing removed it. Called before the engine +// serves its API: nothing is building. +func staleWork(dir string) []string { + entries, _ := os.ReadDir(dir) + gone := []string{} + for _, entry := range entries { + name := entry.Name() + if !entry.IsDir() || name != "build" && name != "load" && !strings.HasPrefix(name, ".commit-") && !strings.HasPrefix(name, ".removing-") { + continue + } + aside := filepath.Join(dir, ".removing-"+token()[:16]) + if os.Rename(filepath.Join(dir, name), aside) == nil { + gone = append(gone, aside) + } + } + return gone +} + func run() { syscall.Umask(0077) if len(os.Args) > 1 && os.Args[1] == "_supervise" { @@ -97,6 +119,17 @@ func run() { cleanupUnpublished(b.State) cleanupUnpublished(b.Cache) migrateNames(b.Cache) + stale := staleWork(store) + if store != b.Home { + stale = append(stale, staleWork(b.Home)...) // from before the store + } + go func() { // gigabytes, perhaps: not in the way of the start + for _, p := range stale { + if e := removeTree(p); e != nil { + fmt.Fprintln(os.Stderr, "dockivad: an earlier engine's work stays until the next start:", e) + } + } + }() } if *importName != "" { reject(*layout != "", "Import requires --import-layout") diff --git a/dockivad/restricted_test.go b/dockivad/restricted_test.go index 5068450..ac1d1d8 100644 --- a/dockivad/restricted_test.go +++ b/dockivad/restricted_test.go @@ -119,6 +119,34 @@ func TestRemoveImageRestrictedDirs(t *testing.T) { } } +// The work earlier engines left (builds whose stages held such directories, an +// interrupted commit) goes at a start; images, containers and volumes stay. +func TestStaleWorkRestrictedDirs(t *testing.T) { + dir := t.TempDir() + unlockLater(t, dir) + for _, d := range []string{"build/b-1/root", "build/trash-2/entry/rootfs", "load/l-3", ".commit-4/rootfs", ".removing-5"} { + restrictedTree(t, filepath.Join(dir, d)) + } + for _, keep := range []string{"images", "containers", "build-cache", "volumes"} { + check(os.MkdirAll(filepath.Join(dir, keep, "x"), 0o700)) + } + check(os.WriteFile(filepath.Join(dir, "load.json"), nil, 0o600)) // a file of these names is not the engine's work + stale := staleWork(dir) + for _, p := range stale { + if e := removeTree(p); e != nil { + t.Fatal(e) + } + } + names := []string{} + entries, _ := os.ReadDir(dir) + for _, entry := range entries { + names = append(names, entry.Name()) + } + if got := strings.Join(names, " "); len(stale) != 4 || got != "build-cache containers images load.json volumes" { + t.Fatalf("%d removed, left: %s", len(stale), got) + } +} + // docker commit's copy: the image's files with the container's laid over them. Both // hold such directories; the copy has their modes and files, the sources keep theirs, // and the layer written from it lists them.