Merge 6786a77075 into 511504c2eb

internal/execuser/execuser.go

@@ -0,0 +1,68 @@
+// Package execuser resolves user overrides for guest exec processes.
+package execuser
+
+import (
+	"fmt"
+	userpkg "os/user"
+	"strconv"
+	"strings"
+	"syscall"
+)
+
+// Resolve parses a user override in user[:group] form.
+//
+// User and group components may be names or numeric IDs.
+func Resolve(spec string) (*syscall.Credential, error) {
+	// Split the override into user and optional group
+	userPart, groupPart, _ := strings.Cut(spec, ":")
+	if userPart == "" {
+		return nil, fmt.Errorf("invalid user override %q", spec)
+	}
+
+	// Resolve the user by numeric ID or name
+	var user *userpkg.User
+	var err error
+
+	if _, err = strconv.ParseUint(userPart, 10, 32); err == nil {
+		user, err = userpkg.LookupId(userPart)
+	} else {
+		user, err = userpkg.Lookup(userPart)
+	}
+	if err != nil {
+		return nil, fmt.Errorf("failed to resolve user %q: %w", userPart, err)
+	}
+
+	// User resolution yields strings, so we need to parse them first
+	uid, err := strconv.ParseUint(user.Uid, 10, 32)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse UID %q: %w", user.Uid, err)
+	}
+
+	gid, err := strconv.ParseUint(user.Gid, 10, 32)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse GID %q: %w", user.Gid, err)
+	}
+
+	// Keep the user's primary GID when no group override is provided
+	if groupPart == "" {
+		return &syscall.Credential{Uid: uint32(uid), Gid: uint32(gid)}, nil
+	}
+
+	// When group override is provided, and it's numeric, use it directly
+	if groupGID, err := strconv.ParseUint(groupPart, 10, 32); err == nil {
+		return &syscall.Credential{Uid: uint32(uid), Gid: uint32(groupGID)}, nil
+	}
+
+	// Otherwise, resolve named group override through the system user database
+	group, err := userpkg.LookupGroup(groupPart)
+	if err != nil {
+		return nil, fmt.Errorf("failed to resolve group %q: %w", groupPart, err)
+	}
+
+	groupGID, err := strconv.ParseUint(group.Gid, 10, 32)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse GID %q: %w", group.Gid, err)
+	}
+
+	return &syscall.Credential{Uid: uint32(uid), Gid: uint32(groupGID)}, nil
+}

internal/execuser/execuser_test.go

@@ -0,0 +1,69 @@
+package execuser_test
+
+import (
+	userpkg "os/user"
+	"strconv"
+	"syscall"
+	"testing"
+
+	"github.com/cirruslabs/tart-guest-agent/internal/execuser"
+	"github.com/stretchr/testify/require"
+)
+
+func TestResolve(t *testing.T) {
+	currentUser, err := userpkg.Current()
+	require.NoError(t, err)
+
+	currentGroup, err := userpkg.LookupGroupId(currentUser.Gid)
+	require.NoError(t, err)
+
+	currentUID, err := strconv.ParseUint(currentUser.Uid, 10, 32)
+	require.NoError(t, err)
+
+	currentGID, err := strconv.ParseUint(currentUser.Gid, 10, 32)
+	require.NoError(t, err)
+
+	tests := []struct {
+		name string
+		spec string
+		want *syscall.Credential
+	}{
+		{
+			name: "named user",
+			spec: currentUser.Username,
+			want: &syscall.Credential{Uid: uint32(currentUID), Gid: uint32(currentGID)},
+		},
+		{
+			name: "numeric user",
+			spec: currentUser.Uid,
+			want: &syscall.Credential{Uid: uint32(currentUID), Gid: uint32(currentGID)},
+		},
+		{
+			name: "named user and group",
+			spec: currentUser.Username + ":" + currentGroup.Name,
+			want: &syscall.Credential{Uid: uint32(currentUID), Gid: uint32(currentGID)},
+		},
+		{
+			name: "named user and numeric group",
+			spec: currentUser.Username + ":31337",
+			want: &syscall.Credential{Uid: uint32(currentUID), Gid: 31337},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got, err := execuser.Resolve(test.spec)
+			require.NoError(t, err)
+			require.Equal(t, test.want, got)
+		})
+	}
+}
+
+func TestResolveRejectsMissingUser(t *testing.T) {
+	for _, spec := range []string{"", ":staff"} {
+		t.Run(spec, func(t *testing.T) {
+			_, err := execuser.Resolve(spec)
+			require.ErrorContains(t, err, "invalid user override")
+		})
+	}
+}

internal/rpc/exec.go

@@ -7,13 +7,12 @@ import (
 	"io"
 	"os"
 	"os/exec"
-	userpkg "os/user"
 	"slices"
-	"strconv"
 	"strings"
 	"sync"
 	"syscall"
 
+	"github.com/cirruslabs/tart-guest-agent/internal/execuser"
 	"github.com/creack/pty"
 	"github.com/google/uuid"
 	"github.com/samber/lo"
@@ -460,32 +459,17 @@ func applyExecOverrides(cmd *exec.Cmd, command *ExecRequest_Command) error {
 	}
 
 	if user := command.GetUser(); user != "" {
-		selectedUser, err := userpkg.Lookup(user)
+		credential, err := execuser.Resolve(user)
 		if err != nil {
-			return fmt.Errorf("failed to resolve user %q: %w", user, err)
+			return fmt.Errorf("failed to apply user override %q: %w", user, err)
 		}
 
-		uid, err := strconv.ParseUint(selectedUser.Uid, 10, 32)
-		if err != nil {
-			return fmt.Errorf("failed to parse UID %q for user %q: %w",
-				selectedUser.Uid, user, err)
-		}
-
-		gid, err := strconv.ParseUint(selectedUser.Gid, 10, 32)
-		if err != nil {
-			return fmt.Errorf("failed to parse GID %q for user %q: %w",
-				selectedUser.Gid, user, err)
-		}
-
-		if uint32(uid) == uint32(os.Geteuid()) && uint32(gid) == uint32(os.Getegid()) {
+		// Avoid changing credentials when the requested user is the guest agent user
+		if credential.Uid == uint32(os.Geteuid()) && credential.Gid == uint32(os.Getegid()) {
 			return nil
 		}
 
-		// Avoid changing credentials when the requested user is the same as guest agen't user
-		cmd.SysProcAttr.Credential = &syscall.Credential{
-			Uid: uint32(uid),
-			Gid: uint32(gid),
-		}
+		cmd.SysProcAttr.Credential = credential
 	}
 
 	return nil