fix: update ownership of user homedir (#238)

Fixes #229

If a user mounts a Docker volume into /home/$USER, Docker will automatically assign permissions root:root to it as the envbuilder container runs as root by default. The resulting container will then have /home/$USER owned by root:root. The user will be unable to write any files there until they manually fix the permissions, which would require root privileges.

This PR adds a step to fix ownership of /home/$USER to the uid:gid we get from UserInfo.

Author: johnstcn

envbuilder.go

@@ -759,13 +759,34 @@ func Run(ctx context.Context, options Options) error {
 		//
 		// We need to change the ownership of the files to the user that will
 		// be running the init script.
-		filepath.Walk(options.WorkspaceFolder, func(path string, info os.FileInfo, err error) error {
+		if chownErr := filepath.Walk(options.WorkspaceFolder, func(path string, _ os.FileInfo, err error) error {
 			if err != nil {
 				return err
 			}
 			return os.Chown(path, userInfo.uid, userInfo.gid)
-		})
-		endStage("👤 Updated the ownership of the workspace!")
+		}); chownErr != nil {
+			options.Logger(notcodersdk.LogLevelError, "chown %q: %s", userInfo.user.HomeDir, chownErr.Error())
+			endStage("⚠️ Failed to the ownership of the workspace, you may need to fix this manually!")
+		} else {
+			endStage("👤 Updated the ownership of the workspace!")
+		}
+	}
+
+	// We may also need to update the ownership of the user homedir.
+	// Skip this step if the user is root.
+	if userInfo.uid != 0 {
+		endStage := startStage("🔄 Updating ownership of %s...", userInfo.user.HomeDir)
+		if chownErr := filepath.Walk(userInfo.user.HomeDir, func(path string, _ fs.FileInfo, err error) error {
+			if err != nil {
+				return err
+			}
+			return os.Chown(path, userInfo.uid, userInfo.gid)
+		}); chownErr != nil {
+			options.Logger(notcodersdk.LogLevelError, "chown %q: %s", userInfo.user.HomeDir, chownErr.Error())
+			endStage("⚠️ Failed to update ownership of %s, you may need to fix this manually!", userInfo.user.HomeDir)
+		} else {
+			endStage("🏡 Updated ownership of %s!", userInfo.user.HomeDir)
+		}
 	}
 
 	err = os.MkdirAll(options.WorkspaceFolder, 0o755)

integration/integration_test.go

@@ -30,6 +30,8 @@ import (
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/filters"
+	"github.com/docker/docker/api/types/mount"
+	"github.com/docker/docker/api/types/volume"
 	"github.com/docker/docker/client"
 	"github.com/docker/docker/pkg/stdcopy"
 	"github.com/go-git/go-billy/v5/memfs"
@@ -1354,6 +1356,40 @@ COPY --from=a /root/date.txt /date.txt`, testImageAlpine, testImageAlpine),
 	})
 }
 
+func TestChownHomedir(t *testing.T) {
+	t.Parallel()
+
+	// Ensures that a Git repository with a devcontainer.json is cloned and built.
+	srv := createGitServer(t, gitServerOptions{
+		files: map[string]string{
+			".devcontainer/devcontainer.json": `{
+				"name": "Test",
+				"build": {
+					"dockerfile": "Dockerfile"
+				},
+			}`,
+			".devcontainer/Dockerfile": fmt.Sprintf(`FROM %s
+RUN useradd test \
+  --create-home \
+  --shell=/bin/bash \
+  --uid=1001 \
+  --user-group
+USER test
+`, testImageUbuntu), // Note: this isn't reproducible with Alpine for some reason.
+		},
+	})
+
+	// Run envbuilder with a Docker volume mounted to homedir
+	volName := fmt.Sprintf("%s%d-home", t.Name(), time.Now().Unix())
+	ctr, err := runEnvbuilder(t, options{env: []string{
+		envbuilderEnv("GIT_URL", srv.URL),
+	}, volumes: map[string]string{volName: "/home/test"}})
+	require.NoError(t, err)
+
+	output := execContainer(t, ctr, "stat -c %u:%g /home/test/")
+	require.Equal(t, "1001:1001", strings.TrimSpace(output))
+}
+
 type setupInMemoryRegistryOpts struct {
 	Username string
 	Password string
@@ -1465,8 +1501,9 @@ func cleanOldEnvbuilders() {
 }
 
 type options struct {
-	binds []string
-	env   []string
+	binds   []string
+	env     []string
+	volumes map[string]string
 }
 
 // runEnvbuilder starts the envbuilder container with the given environment
@@ -1479,6 +1516,21 @@ func runEnvbuilder(t *testing.T, options options) (string, error) {
 	t.Cleanup(func() {
 		cli.Close()
 	})
+	mounts := make([]mount.Mount, 0)
+	for volName, volPath := range options.volumes {
+		mounts = append(mounts, mount.Mount{
+			Type:   mount.TypeVolume,
+			Source: volName,
+			Target: volPath,
+		})
+		_, err = cli.VolumeCreate(ctx, volume.CreateOptions{
+			Name: volName,
+		})
+		require.NoError(t, err)
+		t.Cleanup(func() {
+			_ = cli.VolumeRemove(ctx, volName, true)
+		})
+	}
 	ctr, err := cli.ContainerCreate(ctx, &container.Config{
 		Image: "envbuilder:latest",
 		Env:   options.env,
@@ -1488,10 +1540,11 @@ func runEnvbuilder(t *testing.T, options options) (string, error) {
 	}, &container.HostConfig{
 		NetworkMode: container.NetworkMode("host"),
 		Binds:       options.binds,
+		Mounts:      mounts,
 	}, nil, nil, "")
 	require.NoError(t, err)
 	t.Cleanup(func() {
-		cli.ContainerRemove(ctx, ctr.ID, container.RemoveOptions{
+		_ = cli.ContainerRemove(ctx, ctr.ID, container.RemoveOptions{
 			RemoveVolumes: true,
 			Force:         true,
 		})