amadan

carlos / platform Public

instancefs: directory operations beneath symlink-free directory fds

MkdirBeneath, RenameBetween, RemoveBeneath and ChownBeneath act on one
entry beneath a directory opened with openat2 RESOLVE_NO_SYMLINKS, so the
layout pass can create, move and clear entries in the raw instance
directories without a same-uid tenant redirecting them (design §3.2).
The !linux builds return errors.ErrUnsupported.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 16f31d10d5668c5424f62543e9d40e61d39c206d parent 9352093
4 files changed, +386 −0
  • internal/instancefs/dirs_linux.go +138 −0
  • internal/instancefs/dirs_linux_test.go +207 −0
  • internal/instancefs/instancefs.go +10 −0
  • internal/instancefs/instancefs_other.go +31 −0
diff --git a/internal/instancefs/dirs_linux.go b/internal/instancefs/dirs_linux.go
new file mode 100644
index 000000000..16d184efc
--- /dev/null
+++ b/internal/instancefs/dirs_linux.go
@@ -0,0 +1,138 @@
+//go:build linux
+
+package instancefs
+
+import (
+ "errors"
+ "fmt"
+ "os"
+ "path/filepath"
+
+ "golang.org/x/sys/unix"
+)
+
+// These are the directory operations the layout pass performs on the raw
+// directories behind the instance bind mounts (isolated-exec design §3.2).
+// A flat tenant shares the agent's uid during the transition and can plant
+// a symlink anywhere the agent can write, so every one of them works from a
+// directory fd that openDir opened without following any symlink, and names
+// exactly one entry beneath it. The *at calls do not follow a symlink at
+// that last component either: mkdirat answers EEXIST, renameat and unlinkat
+// act on the link itself, and fchownat is told not to.
+
+// MkdirBeneath makes sure name, directly under dir, is a real directory
+// with exactly perm (setgid and sticky included): it creates it when it is
+// absent and accepts it when it is already a directory, then sets the mode
+// through an fd on the directory itself. mkdir(2) masks the mode with the
+// umask and ignores setgid, so the explicit fchmod is what makes the mode
+// exact. Anything else at name, a symlink or a file, is refused and left
+// untouched.
+func MkdirBeneath(dir, name string, perm os.FileMode) error {
+ if err := bareName(name); err != nil {
+ return err
+ }
+ dfd, err := openDir(dir)
+ if err != nil {
+ return err
+ }
+ defer unix.Close(dfd)
+ path := filepath.Join(dir, name)
+ if err := unix.Mkdirat(dfd, name, uint32(perm.Perm())); err != nil && !errors.Is(err, unix.EEXIST) {
+ return &os.PathError{Op: "mkdirat", Path: path, Err: err}
+ }
+ how := unix.OpenHow{
+ Flags: unix.O_RDONLY | unix.O_DIRECTORY | unix.O_CLOEXEC,
+ Resolve: resolveFlags,
+ }
+ fd, err := unix.Openat2(dfd, name, &how)
+ if err != nil {
+ if errors.Is(err, unix.ELOOP) {
+ return fmt.Errorf("%w: %s", ErrSymlink, path)
+ }
+ return &os.PathError{Op: "openat2", Path: path, Err: err}
+ }
+ defer unix.Close(fd)
+ if err := unix.Fchmod(fd, unixMode(perm)); err != nil {
+ return &os.PathError{Op: "fchmod", Path: path, Err: err}
+ }
+ return nil
+}
+
+// RenameBetween renames srcName in srcDir to dstName in dstDir. Both
+// directories are opened without following a symlink at any component, so
+// neither side of the rename can be redirected; the entry itself moves as
+// whatever it is, a link included. An existing entry at the destination is
+// replaced, as rename(2) always does: callers that must not replace
+// something check for it first.
+func RenameBetween(srcDir, srcName, dstDir, dstName string) error {
+ if err := bareName(srcName); err != nil {
+ return err
+ }
+ if err := bareName(dstName); err != nil {
+ return err
+ }
+ sfd, err := openDir(srcDir)
+ if err != nil {
+ return err
+ }
+ defer unix.Close(sfd)
+ dfd, err := openDir(dstDir)
+ if err != nil {
+ return err
+ }
+ defer unix.Close(dfd)
+ if err := unix.Renameat(sfd, srcName, dfd, dstName); err != nil {
+ return &os.LinkError{Op: "renameat", Old: filepath.Join(srcDir, srcName), New: filepath.Join(dstDir, dstName), Err: err}
+ }
+ return nil
+}
+
+// RemoveBeneath unlinks name, a non-directory entry directly under dir. A
+// symlink there is removed, never its target.
+func RemoveBeneath(dir, name string) error {
+ if err := bareName(name); err != nil {
+ return err
+ }
+ dfd, err := openDir(dir)
+ if err != nil {
+ return err
+ }
+ defer unix.Close(dfd)
+ if err := unix.Unlinkat(dfd, name, 0); err != nil {
+ return &os.PathError{Op: "unlinkat", Path: filepath.Join(dir, name), Err: err}
+ }
+ return nil
+}
+
+// ChownBeneath changes the owner and group of name directly under dir
+// without following a symlink there. -1 leaves either unchanged.
+func ChownBeneath(dir, name string, uid, gid int) error {
+ if err := bareName(name); err != nil {
+ return err
+ }
+ dfd, err := openDir(dir)
+ if err != nil {
+ return err
+ }
+ defer unix.Close(dfd)
+ if err := unix.Fchownat(dfd, name, uid, gid, unix.AT_SYMLINK_NOFOLLOW); err != nil {
+ return &os.PathError{Op: "fchownat", Path: filepath.Join(dir, name), Err: err}
+ }
+ return nil
+}
+
+// unixMode converts an os.FileMode to the mode bits chmod(2) takes; Go's
+// setgid and sticky flags are not the kernel's bit positions.
+func unixMode(m os.FileMode) uint32 {
+ mode := uint32(m.Perm())
+ if m&os.ModeSetuid != 0 {
+ mode |= unix.S_ISUID
+ }
+ if m&os.ModeSetgid != 0 {
+ mode |= unix.S_ISGID
+ }
+ if m&os.ModeSticky != 0 {
+ mode |= unix.S_ISVTX
+ }
+ return mode
+}
diff --git a/internal/instancefs/dirs_linux_test.go b/internal/instancefs/dirs_linux_test.go
new file mode 100644
index 000000000..eb2a24364
--- /dev/null
+++ b/internal/instancefs/dirs_linux_test.go
@@ -0,0 +1,207 @@
+//go:build linux
+
+package instancefs
+
+import (
+ "errors"
+ "os"
+ "path/filepath"
+ "syscall"
+ "testing"
+)
+
+func TestMkdirBeneathCreatesWithTheExactMode(t *testing.T) {
+ dir := t.TempDir()
+ if err := MkdirBeneath(dir, "run", 0o750|os.ModeSetgid); err != nil {
+ t.Fatalf("MkdirBeneath: %v", err)
+ }
+ fi, err := os.Lstat(filepath.Join(dir, "run"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !fi.IsDir() || fi.Mode().Perm() != 0o750 || fi.Mode()&os.ModeSetgid == 0 {
+ t.Fatalf("mode = %v, want a 2750 directory", fi.Mode())
+ }
+}
+
+// An existing real directory is accepted and brought to the requested
+// mode: the pass runs again and again over the same host, and a directory
+// a same-uid process loosened must not stay loose.
+func TestMkdirBeneathAcceptsAnExistingDirectoryAndResetsItsMode(t *testing.T) {
+ dir := t.TempDir()
+ if err := os.Mkdir(filepath.Join(dir, "d"), 0o777); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Chmod(filepath.Join(dir, "d"), 0o777); err != nil {
+ t.Fatal(err)
+ }
+ if err := MkdirBeneath(dir, "d", 0o700); err != nil {
+ t.Fatalf("MkdirBeneath over an existing directory: %v", err)
+ }
+ fi, _ := os.Lstat(filepath.Join(dir, "d"))
+ if fi.Mode().Perm() != 0o700 {
+ t.Fatalf("mode = %v, want 0700", fi.Mode())
+ }
+}
+
+func TestMkdirBeneathRefusesASymlinkAndLeavesItsTargetAlone(t *testing.T) {
+ dir, elsewhere := t.TempDir(), t.TempDir()
+ if err := os.Chmod(elsewhere, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Symlink(elsewhere, filepath.Join(dir, "d")); err != nil {
+ t.Fatal(err)
+ }
+ if err := MkdirBeneath(dir, "d", 0o700); !errors.Is(err, ErrSymlink) {
+ t.Fatalf("err = %v, want ErrSymlink", err)
+ }
+ fi, _ := os.Stat(elsewhere)
+ if fi.Mode().Perm() != 0o755 {
+ t.Fatalf("the symlink's target was chmodded to %v", fi.Mode())
+ }
+}
+
+func TestMkdirBeneathRefusesAFileAndAPathName(t *testing.T) {
+ dir := t.TempDir()
+ if err := os.WriteFile(filepath.Join(dir, "f"), nil, 0o600); err != nil {
+ t.Fatal(err)
+ }
+ if err := MkdirBeneath(dir, "f", 0o700); err == nil {
+ t.Fatal("a regular file accepted as a directory")
+ }
+ if err := MkdirBeneath(dir, "a/b", 0o700); !errors.Is(err, ErrEscapes) {
+ t.Fatalf("a multi-component name: err = %v, want ErrEscapes", err)
+ }
+ if err := MkdirBeneath(dir, "..", 0o700); !errors.Is(err, ErrEscapes) {
+ t.Fatalf("..: err = %v, want ErrEscapes", err)
+ }
+}
+
+func TestMkdirBeneathRefusesASymlinkedParent(t *testing.T) {
+ base, real := t.TempDir(), t.TempDir()
+ link := filepath.Join(base, "link")
+ if err := os.Symlink(real, link); err != nil {
+ t.Fatal(err)
+ }
+ if err := MkdirBeneath(link, "d", 0o700); !errors.Is(err, ErrSymlink) {
+ t.Fatalf("err = %v, want ErrSymlink", err)
+ }
+ if _, err := os.Lstat(filepath.Join(real, "d")); !errors.Is(err, os.ErrNotExist) {
+ t.Fatal("a directory was created through the symlinked parent")
+ }
+}
+
+func TestRenameBetweenMovesFilesAndDirectories(t *testing.T) {
+ src, dst := t.TempDir(), t.TempDir()
+ if err := os.WriteFile(filepath.Join(src, "h.db"), []byte("db"), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Mkdir(filepath.Join(src, "shadow"), 0o700); err != nil {
+ t.Fatal(err)
+ }
+ if err := RenameBetween(src, "h.db", dst, "h.db"); err != nil {
+ t.Fatalf("file: %v", err)
+ }
+ if err := RenameBetween(src, "shadow", dst, "shadow"); err != nil {
+ t.Fatalf("directory: %v", err)
+ }
+ if b, err := os.ReadFile(filepath.Join(dst, "h.db")); err != nil || string(b) != "db" {
+ t.Fatalf("moved file: %q %v", b, err)
+ }
+ if fi, err := os.Lstat(filepath.Join(dst, "shadow")); err != nil || !fi.IsDir() {
+ t.Fatalf("moved directory: %v %v", fi, err)
+ }
+}
+
+// A symlink at the source name is moved as the link it is, never followed:
+// rename works on directory entries.
+func TestRenameBetweenMovesASymlinkAsALink(t *testing.T) {
+ src, dst, elsewhere := t.TempDir(), t.TempDir(), t.TempDir()
+ target := filepath.Join(elsewhere, "secret")
+ if err := os.WriteFile(target, []byte("s"), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Symlink(target, filepath.Join(src, "h.env")); err != nil {
+ t.Fatal(err)
+ }
+ if err := RenameBetween(src, "h.env", dst, "h.env"); err != nil {
+ t.Fatal(err)
+ }
+ if fi, err := os.Lstat(filepath.Join(dst, "h.env")); err != nil || fi.Mode()&os.ModeSymlink == 0 {
+ t.Fatalf("destination is not the link: %v %v", fi, err)
+ }
+ if _, err := os.Lstat(target); err != nil {
+ t.Fatalf("the link's target moved: %v", err)
+ }
+}
+
+func TestRenameBetweenRefusesASymlinkedDirectoryOnEitherSide(t *testing.T) {
+ base, real := t.TempDir(), t.TempDir()
+ link := filepath.Join(base, "link")
+ if err := os.Symlink(real, link); err != nil {
+ t.Fatal(err)
+ }
+ plain := t.TempDir()
+ if err := os.WriteFile(filepath.Join(plain, "h.db"), []byte("db"), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ if err := RenameBetween(plain, "h.db", link, "h.db"); !errors.Is(err, ErrSymlink) {
+ t.Fatalf("symlinked destination: err = %v, want ErrSymlink", err)
+ }
+ if _, err := os.Lstat(filepath.Join(real, "h.db")); !errors.Is(err, os.ErrNotExist) {
+ t.Fatal("the file landed through the symlinked destination")
+ }
+ if err := os.WriteFile(filepath.Join(real, "x"), nil, 0o600); err != nil {
+ t.Fatal(err)
+ }
+ if err := RenameBetween(link, "x", plain, "x"); !errors.Is(err, ErrSymlink) {
+ t.Fatalf("symlinked source: err = %v, want ErrSymlink", err)
+ }
+}
+
+func TestRenameBetweenRefusesPathNames(t *testing.T) {
+ a, b := t.TempDir(), t.TempDir()
+ for _, bad := range [][2]string{{"x/y", "y"}, {"y", "../y"}, {"", "y"}, {"y", "."}} {
+ if err := RenameBetween(a, bad[0], b, bad[1]); !errors.Is(err, ErrEscapes) {
+ t.Errorf("%q -> %q: err = %v, want ErrEscapes", bad[0], bad[1], err)
+ }
+ }
+}
+
+func TestRemoveBeneathRemovesTheEntryNotItsTarget(t *testing.T) {
+ dir, elsewhere := t.TempDir(), t.TempDir()
+ target := filepath.Join(elsewhere, "keep")
+ if err := os.WriteFile(target, []byte("k"), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Symlink(target, filepath.Join(dir, "link")); err != nil {
+ t.Fatal(err)
+ }
+ if err := RemoveBeneath(dir, "link"); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := os.Lstat(target); err != nil {
+ t.Fatalf("the link's target was removed: %v", err)
+ }
+ if err := RemoveBeneath(dir, "link"); !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("second remove: err = %v, want ErrNotExist", err)
+ }
+}
+
+func TestChownBeneathChangesTheGroupWithoutFollowing(t *testing.T) {
+ dir := t.TempDir()
+ if err := os.Mkdir(filepath.Join(dir, "d"), 0o700); err != nil {
+ t.Fatal(err)
+ }
+ gid := os.Getgid()
+ if err := ChownBeneath(dir, "d", -1, gid); err != nil {
+ t.Fatalf("ChownBeneath to our own group: %v", err)
+ }
+ fi, _ := os.Lstat(filepath.Join(dir, "d"))
+ if st := fi.Sys().(*syscall.Stat_t); int(st.Gid) != gid {
+ t.Fatalf("gid = %d, want %d", st.Gid, gid)
+ }
+ if err := ChownBeneath(dir, "a/b", -1, gid); !errors.Is(err, ErrEscapes) {
+ t.Fatalf("path name: err = %v, want ErrEscapes", err)
+ }
+}
diff --git a/internal/instancefs/instancefs.go b/internal/instancefs/instancefs.go
index 93cb41447..3e07bc427 100644
--- a/internal/instancefs/instancefs.go
+++ b/internal/instancefs/instancefs.go
@@ -52,6 +52,16 @@ func cleanRel(name string) (string, error) {
return c, nil
}
+// bareName validates name as exactly one path component: the directory
+// operations act on one entry beneath an fd, and a name carrying a slash
+// would resolve through intermediate components that fd never vetted.
+func bareName(name string) error {
+ if name == "" || name == "." || name == ".." || strings.Contains(name, "/") {
+ return ErrEscapes
+ }
+ return nil
+}
+
// ReadBeneath is OpenBeneath followed by reading the whole file.
func ReadBeneath(dir, name string) ([]byte, error) {
f, err := OpenBeneath(dir, name, os.O_RDONLY, 0)
diff --git a/internal/instancefs/instancefs_other.go b/internal/instancefs/instancefs_other.go
index 5df5e10d5..8ffe79b90 100644
--- a/internal/instancefs/instancefs_other.go
+++ b/internal/instancefs/instancefs_other.go
@@ -28,3 +28,34 @@ func MountNoSymfollow(mountpoint string) (bool, error) {
}
func mountinfoLineHas(line, mountpoint, option string) bool { return false }
+
+func MkdirBeneath(dir, name string, perm os.FileMode) error {
+ if err := bareName(name); err != nil {
+ return err
+ }
+ return errors.ErrUnsupported
+}
+
+func RenameBetween(srcDir, srcName, dstDir, dstName string) error {
+ if err := bareName(srcName); err != nil {
+ return err
+ }
+ if err := bareName(dstName); err != nil {
+ return err
+ }
+ return errors.ErrUnsupported
+}
+
+func RemoveBeneath(dir, name string) error {
+ if err := bareName(name); err != nil {
+ return err
+ }
+ return errors.ErrUnsupported
+}
+
+func ChownBeneath(dir, name string, uid, gid int) error {
+ if err := bareName(name); err != nil {
+ return err
+ }
+ return errors.ErrUnsupported
+}