diff --git a/cmd/atelet/main.go b/cmd/atelet/main.go index 801454c67..b7291369d 100644 --- a/cmd/atelet/main.go +++ b/cmd/atelet/main.go @@ -947,8 +947,11 @@ func writeFileAtomic(path string, data []byte, perm os.FileMode) error { func resetActorDirs(actorTemplateNamespace, actorTemplateName, actorID string) error { // Explicitly leave runsc logs dir untouched. + // removeAllWritable, not os.RemoveAll: the bundle holds unpacked actor-image + // rootfs whose directories keep the image's (possibly read-only) modes, which + // atelet can't remove as plain root without first making them writable. bundleDir := ateompath.OCIBundleDir(actorTemplateNamespace, actorTemplateName, actorID) - if err := os.RemoveAll(bundleDir); err != nil { + if err := removeAllWritable(bundleDir); err != nil { return fmt.Errorf("while deleting bundle dir: %w", err) } if err := os.MkdirAll(bundleDir, 0o700); err != nil { diff --git a/cmd/atelet/oci.go b/cmd/atelet/oci.go index a00df99e9..928d6540e 100644 --- a/cmd/atelet/oci.go +++ b/cmd/atelet/oci.go @@ -21,10 +21,12 @@ import ( "errors" "fmt" "io" + "io/fs" "log/slog" "os" "path" "path/filepath" + "sort" "strings" "github.com/agent-substrate/substrate/cmd/atelet/internal/memorypullcache" @@ -63,7 +65,7 @@ func prepareOCIDirectory(ctx context.Context, pullCache *memorypullcache.MemoryP bundlePath := ateompath.OCIBundlePath(actorTemplateNamespace, actorTemplateName, actorID, containerName) rootPath := path.Join(bundlePath, "rootfs") - if err := os.RemoveAll(rootPath); err != nil { + if err := removeAllWritable(rootPath); err != nil { return fmt.Errorf("while clearing rootfs %q: %w", rootPath, err) } @@ -281,6 +283,13 @@ func untar(ctx context.Context, tarData io.Reader, rootPath string) error { } defer root.Close() + // Directories are created owner-writable during extraction (so their children + // can be written even when the image marks them read-only, e.g. ko ships + // /ko-app as 0555) and their real modes are restored afterwards. This lets + // atelet, running as plain root, unpack arbitrary actor images without + // CAP_DAC_OVERRIDE. Keyed by name so a repeated dir entry's last mode wins. + dirModes := map[string]os.FileMode{} + tarReader := tar.NewReader(tarData) for { hdr, err := tarReader.Next() @@ -332,12 +341,18 @@ func untar(ctx context.Context, tarData io.Reader, rootPath string) error { } case tar.TypeDir: - err := root.Mkdir(name, mode) + // Create owner-writable so children can be written even when the image + // marks the dir read-only; the real mode is restored after extraction + // (see dirModes / the restore pass below). + err := root.Mkdir(name, mode|0o700) if errors.Is(err, os.ErrExist) { - // Ignore --- real images produced by ko seem to have directory entries placed multiple times? + // OCI layers can repeat a directory entry (real ko images do); the + // existing dir is already owner-writable, so let the later entry's + // mode win at restore time. } else if err != nil { return fmt.Errorf("while creating directory=%q, mode=%v: %w", name, mode, err) } + dirModes[name] = mode case tar.TypeSymlink: // OCI image layers may re-define the same path across layers (e.g. @@ -393,5 +408,39 @@ func untar(ctx context.Context, tarData io.Reader, rootPath string) error { } + // Restore the image's intended directory modes now that every child exists. + // Deepest paths first: a child's path is always longer than its parent's, so + // length-descending order guarantees a directory is restored before any of its + // ancestors — restoring a parent to a non-traversable mode then can't block + // restoring its children. + dirs := make([]string, 0, len(dirModes)) + for name := range dirModes { + dirs = append(dirs, name) + } + sort.Slice(dirs, func(i, j int) bool { return len(dirs[i]) > len(dirs[j]) }) + for _, name := range dirs { + if err := root.Chmod(name, dirModes[name]); err != nil { + return fmt.Errorf("while restoring mode %v on directory %q: %w", dirModes[name], name, err) + } + } + return nil } + +// removeAllWritable removes path and everything under it, first making every +// directory owner-writable so its children can be unlinked. atelet runs as plain +// root (no CAP_DAC_OVERRIDE), so it cannot remove entries inside an image-defined +// read-only directory (e.g. ko's restored 0555 /ko-app) — os.RemoveAll alone +// fails there with EACCES. atelet owns these files, so chmod needs no capability. +func removeAllWritable(path string) error { + // Make dirs traversable/writable top-down (WalkDir visits a directory before + // reading it, so chmod here lets the walk descend into otherwise-unreadable + // dirs). Best-effort: ignore errors and let os.RemoveAll surface real ones. + _ = filepath.WalkDir(path, func(p string, d fs.DirEntry, err error) error { + if err == nil && d.IsDir() { + _ = os.Chmod(p, 0o700) + } + return nil + }) + return os.RemoveAll(path) +} diff --git a/cmd/atelet/oci_test.go b/cmd/atelet/oci_test.go index a12c0e637..6241f02a8 100644 --- a/cmd/atelet/oci_test.go +++ b/cmd/atelet/oci_test.go @@ -450,6 +450,37 @@ func TestUntar_LaterEntryWins(t *testing.T) { }) } +// A read-only directory in the image (e.g. ko ships /ko-app as 0555) must still +// get its child written AND keep the image's mode, so atelet can unpack arbitrary +// actor images as plain root without CAP_DAC_OVERRIDE. removeAllWritable must +// then still be able to delete the restored read-only tree. (Meaningful as a +// non-root test run; as root the dir-permission checks are bypassed.) +func TestUntar_ReadOnlyDir(t *testing.T) { + entries := []tarEntry{ + {name: "ko-app", typeflag: tar.TypeDir, mode: 0o555}, + {name: "ko-app/counter", typeflag: tar.TypeReg, mode: 0o755, body: "bin"}, + } + dir, err := runUntar(t, entries) + if err != nil { + t.Fatalf("untar into read-only dir: %v", err) + } + if got, _ := os.ReadFile(filepath.Join(dir, "ko-app/counter")); string(got) != "bin" { + t.Errorf("ko-app/counter = %q, want %q", got, "bin") + } + info, err := os.Stat(filepath.Join(dir, "ko-app")) + if err != nil { + t.Fatalf("stat ko-app: %v", err) + } + if info.Mode().Perm() != 0o555 { + t.Errorf("ko-app mode = %v, want the image's 0555 preserved", info.Mode().Perm()) + } + // atelet must be able to delete the restored read-only tree (this also lets + // t.TempDir's cleanup succeed, which plain os.RemoveAll could not on 0555). + if err := removeAllWritable(filepath.Join(dir, "ko-app")); err != nil { + t.Errorf("removeAllWritable on restored read-only dir: %v", err) + } +} + func TestUntar_PathTraversal(t *testing.T) { tests := []struct { name string diff --git a/manifests/ate-install/atelet.yaml b/manifests/ate-install/atelet.yaml index c9564c40d..cfe73900c 100644 --- a/manifests/ate-install/atelet.yaml +++ b/manifests/ate-install/atelet.yaml @@ -69,8 +69,21 @@ spec: image: ko://github.com/agent-substrate/substrate/cmd/atelet args: - --gcp-auth-for-image-pulls=true + # atelet does no mounts, netlink, device, or namespace operations (those + # live in the ateom worker pod) — it only reads/writes the + # /var/lib/ateom-gvisor hostPath as root, so it needs no Linux + # capabilities. (Unpacking arbitrary actor images can require writing into + # image-defined read-only dirs; the untar in cmd/atelet/oci.go extracts + # directories writable then restores the image's modes, and removal makes + # them writable again first, so root suffices without CAP_DAC_OVERRIDE.) + # runAsUser is pinned explicitly rather than relying on the base image's + # default user. securityContext: - privileged: true + runAsUser: 0 + runAsGroup: 0 + capabilities: + drop: + - ALL env: - name: MY_NODE_NAME valueFrom: