From 3ba067290916087282722ad2484766a2a7b37f98 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Wed, 8 Apr 2026 10:03:26 +0100 Subject: [PATCH] Add override_stat xattr support for virtiofs mounts On macOS, workspace directories mounted via virtiofs appear with the host user's UID inside the guest VM. This prevents the guest sandbox user from writing to files and nested directories. Add OverrideUID/OverrideGID fields to VirtioFSMount and a new SetOverrideStatTree function that walks the mount path and sets user.containers.override_stat xattrs on all files and directories before VM boot. This makes libkrun's virtiofs FUSE server report the desired UID/GID to the guest. Symlinks are skipped to prevent setting xattrs outside the mount boundary. Includes input validation for negative UIDs/GIDs and orphaned OverrideGID, build-tagged no-op stub for unsupported platforms, and performance optimization using DirEntry.Info() to avoid redundant Lstat syscalls during the tree walk. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/xattr/walk.go | 65 +++++++++++++++ internal/xattr/walk_other.go | 9 +++ internal/xattr/walk_test.go | 148 +++++++++++++++++++++++++++++++++++ microvm.go | 27 +++++++ options.go | 12 +++ 5 files changed, 261 insertions(+) create mode 100644 internal/xattr/walk.go create mode 100644 internal/xattr/walk_other.go create mode 100644 internal/xattr/walk_test.go diff --git a/internal/xattr/walk.go b/internal/xattr/walk.go new file mode 100644 index 0000000..b2e57e6 --- /dev/null +++ b/internal/xattr/walk.go @@ -0,0 +1,65 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//go:build darwin || linux + +package xattr + +import ( + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" +) + +// SetOverrideStatTree walks root and sets user.containers.override_stat +// on every file and directory. Each entry's real mode (from Lstat) is +// preserved in the xattr value. Symlinks are skipped — they cannot carry +// user.* xattrs on Linux, and skipping them prevents setting xattrs +// outside the mount boundary via symlink traversal. +// +// The root path is resolved via [filepath.EvalSymlinks] before walking, +// and every visited entry is verified to remain under the resolved root. +// +// Errors on individual entries are logged at debug level and skipped. +// Returns an error only if the root itself cannot be accessed. +// +// On platforms other than macOS and Linux a no-op stub is provided. +func SetOverrideStatTree(root string, uid, gid int) error { + if _, err := os.Lstat(root); err != nil { + return fmt.Errorf("access root %s: %w", root, err) + } + + realRoot, err := filepath.EvalSymlinks(root) + if err != nil { + return fmt.Errorf("resolve root: %w", err) + } + realRoot = filepath.Clean(realRoot) + rootPrefix := realRoot + string(filepath.Separator) + + return filepath.WalkDir(realRoot, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return nil // best-effort, skip inaccessible entries + } + // Skip symlinks: prevents setting xattrs outside mount boundary, + // and Linux rejects user.* xattrs on symlinks anyway. + if d.Type()&fs.ModeSymlink != 0 { + return nil + } + // Boundary check: verify path stays under resolved root. + cleanPath := filepath.Clean(path) + if cleanPath != realRoot && !strings.HasPrefix(cleanPath, rootPrefix) { + if d.IsDir() { + return fs.SkipDir + } + return nil + } + info, err := d.Info() + if err != nil { + return nil + } + SetOverrideStat(path, uid, gid, info.Mode()) + return nil + }) +} diff --git a/internal/xattr/walk_other.go b/internal/xattr/walk_other.go new file mode 100644 index 0000000..edd6e6c --- /dev/null +++ b/internal/xattr/walk_other.go @@ -0,0 +1,9 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//go:build !darwin && !linux + +package xattr + +// SetOverrideStatTree is a no-op on platforms without xattr support. +func SetOverrideStatTree(_ string, _, _ int) error { return nil } diff --git a/internal/xattr/walk_test.go b/internal/xattr/walk_test.go new file mode 100644 index 0000000..abb5bd9 --- /dev/null +++ b/internal/xattr/walk_test.go @@ -0,0 +1,148 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//go:build darwin || linux + +package xattr + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" +) + +func TestSetOverrideStatTree_NestedTree(t *testing.T) { + t.Parallel() + + root := t.TempDir() + sub1 := filepath.Join(root, "a") + sub2 := filepath.Join(root, "a", "b") + require.NoError(t, os.MkdirAll(sub2, 0o755)) + + // Create a regular file — it should also get the xattr. + filePath := filepath.Join(sub1, "file.txt") + require.NoError(t, os.WriteFile(filePath, []byte("hi"), 0o644)) + + require.NoError(t, SetOverrideStatTree(root, 1000, 1000)) + + // All directories should have the xattr set. + for _, dir := range []string{root, sub1, sub2} { + val := readXattrOpt(t, dir) + assert.Contains(t, val, "1000:1000:", "dir %s should have override xattr", dir) + } + + // Regular files should also have the xattr set. + val := readXattrOpt(t, filePath) + assert.Contains(t, val, "1000:1000:", "file should have override xattr") +} + +func TestSetOverrideStatTree_SymlinkToExternalDir(t *testing.T) { + t.Parallel() + + root := t.TempDir() + external := t.TempDir() + externalSub := filepath.Join(external, "secret") + require.NoError(t, os.Mkdir(externalSub, 0o755)) + + // Create a symlink inside root pointing to an external directory. + require.NoError(t, os.Symlink(external, filepath.Join(root, "escape"))) + + require.NoError(t, SetOverrideStatTree(root, 1000, 1000)) + + // The external directory must NOT have the xattr set. + _, err := unix.Lgetxattr(external, overrideKey, make([]byte, 256)) + assert.Error(t, err, "external dir should not have override xattr") + _, err = unix.Lgetxattr(externalSub, overrideKey, make([]byte, 256)) + assert.Error(t, err, "external subdir should not have override xattr") +} + +func TestSetOverrideStatTree_SymlinkToFile(t *testing.T) { + t.Parallel() + + root := t.TempDir() + target := filepath.Join(root, "real.txt") + require.NoError(t, os.WriteFile(target, []byte("data"), 0o644)) + require.NoError(t, os.Symlink(target, filepath.Join(root, "link.txt"))) + + require.NoError(t, SetOverrideStatTree(root, 1000, 1000)) + + // The real file gets the xattr (it's a regular file under root). + val := readXattrOpt(t, target) + assert.Contains(t, val, "1000:1000:", "real file should have override xattr") + + // The symlink itself should NOT have the xattr. + link := filepath.Join(root, "link.txt") + _, err := unix.Lgetxattr(link, overrideKey, make([]byte, 256)) + assert.Error(t, err, "symlink should not have override xattr") +} + +func TestSetOverrideStatTree_InaccessibleRoot(t *testing.T) { + t.Parallel() + + err := SetOverrideStatTree("/nonexistent/path/xattr-test", 1000, 1000) + assert.Error(t, err, "should fail on inaccessible root") +} + +func TestSetOverrideStatTree_EmptyDir(t *testing.T) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, SetOverrideStatTree(root, 1000, 1000)) + + // Root dir itself should have the xattr. + val := readXattrOpt(t, root) + assert.Contains(t, val, "1000:1000:", "root dir should have override xattr") +} + +func TestSetOverrideStatTree_RootIsSymlink(t *testing.T) { + t.Parallel() + + real := t.TempDir() + sub := filepath.Join(real, "child") + require.NoError(t, os.Mkdir(sub, 0o755)) + + // Create a symlink that points to real. The walk should resolve it + // and set xattrs on the real directory tree. + link := filepath.Join(t.TempDir(), "link") + require.NoError(t, os.Symlink(real, link)) + + require.NoError(t, SetOverrideStatTree(link, 1000, 1000)) + + val := readXattrOpt(t, real) + assert.Contains(t, val, "1000:1000:", "resolved root should have override xattr") + val = readXattrOpt(t, sub) + assert.Contains(t, val, "1000:1000:", "child dir should have override xattr") +} + +func TestSetOverrideStatTree_DifferentUIDGID(t *testing.T) { + t.Parallel() + + root := t.TempDir() + filePath := filepath.Join(root, "file.txt") + require.NoError(t, os.WriteFile(filePath, []byte("data"), 0o644)) + + // Use different UID and GID to verify both are written independently. + require.NoError(t, SetOverrideStatTree(root, 1000, 2000)) + + val := readXattrOpt(t, root) + assert.Contains(t, val, "1000:2000:", "dir should have uid=1000 gid=2000") + + val = readXattrOpt(t, filePath) + assert.Contains(t, val, "1000:2000:", "file should have uid=1000 gid=2000") +} + +// readXattrOpt reads the override_stat xattr and returns its value, or +// empty string if the xattr is not set. +func readXattrOpt(t *testing.T, path string) string { + t.Helper() + buf := make([]byte, 256) + n, err := unix.Lgetxattr(path, overrideKey, buf) + if err != nil { + return "" + } + return string(buf[:n]) +} diff --git a/microvm.go b/microvm.go index 32158cb..420c616 100644 --- a/microvm.go +++ b/microvm.go @@ -37,6 +37,7 @@ import ( "github.com/stacklok/go-microvm/hypervisor" "github.com/stacklok/go-microvm/hypervisor/libkrun" "github.com/stacklok/go-microvm/image" + "github.com/stacklok/go-microvm/internal/xattr" "github.com/stacklok/go-microvm/net/firewall" "github.com/stacklok/go-microvm/net/hosted" rootfspkg "github.com/stacklok/go-microvm/rootfs" @@ -235,6 +236,32 @@ func Run(ctx context.Context, imageRef string, opts ...Option) (*VM, error) { span.End() } + // 5b. Validate and set override_stat xattrs on virtiofs mount entries so + // the guest sees correct ownership (macOS + Linux; no-op on other platforms). + for _, m := range cfg.virtioFS { + if m.OverrideUID < 0 || m.OverrideGID < 0 { + return nil, fmt.Errorf("virtiofs mount %q: OverrideUID/OverrideGID must be non-negative", m.Tag) + } + if m.OverrideUID == 0 && m.OverrideGID > 0 { + return nil, fmt.Errorf("virtiofs mount %q: OverrideGID set without OverrideUID", m.Tag) + } + } + _, xattrSpan := tracer.Start(ctx, "microvm.VirtioFSOverrideStat") + for _, m := range cfg.virtioFS { + if m.OverrideUID > 0 && !m.ReadOnly { + gid := m.OverrideGID + if gid <= 0 { + gid = m.OverrideUID + } + if err := xattr.SetOverrideStatTree(m.HostPath, m.OverrideUID, gid); err != nil { + xattrSpan.RecordError(err) + slog.Warn("failed to set override_stat on virtiofs mount", + "tag", m.Tag, "path", m.HostPath, "error", err) + } + } + } + xattrSpan.End() + // 6. Start VM via backend. _, vmSpawnSpan := tracer.Start(ctx, "microvm.VMSpawn") slog.Debug("starting VM") diff --git a/options.go b/options.go index ed3495b..252be1c 100644 --- a/options.go +++ b/options.go @@ -47,6 +47,18 @@ type VirtioFSMount struct { // support host-side read-only virtiofs. A compromised guest kernel // could bypass this restriction. ReadOnly bool + // OverrideUID, when > 0, causes go-microvm to set the + // user.containers.override_stat xattr on every file and directory under + // HostPath before the VM starts. This makes libkrun's virtiofs FUSE + // server report the given UID/GID to the guest instead of the real + // host values. Symlinks are skipped for safety. + // A zero value means "no override." Since 0 is the zero value for int, + // overriding to UID 0 (root) is not supported through this field. + // Ignored for ReadOnly mounts. + OverrideUID int + // OverrideGID sets the group ID for the override_stat xattr. + // When 0 and OverrideUID > 0, defaults to OverrideUID. + OverrideGID int } // EgressPolicy restricts outbound VM traffic to specific DNS hostnames.