fix(devcontainer): preserve remote user ownership

cmd/snapshot/create.go

@@ -120,6 +120,7 @@ func (cmd *CreateCmd) Run(ctx context.Context, devsyConfig *config.Config, args
 		MountPrefix:             vols.MountPrefix,
 		RunArgs:                 vols.RunArgs,
 		ContainerEnv:            vols.ContainerEnv,
+		RemoteUser:              vols.RemoteUser,
 		ContainerImageMediaType: img.MediaType,
 		ContainerImageDigest:    img.Digest,
 		ContainerImageSize:      img.Size,
@@ -301,6 +302,7 @@ type pushedVolumes struct {
 	MountPrefix  string
 	RunArgs      []string
 	ContainerEnv map[string]string
+	RemoteUser   string
 }
 
 // The volumes RPC (StreamSnapshotVolumes) is served by a tunnelServer reading
@@ -341,6 +343,7 @@ func (cmd *CreateCmd) pushVolumes(
 		MountPrefix:  mountPrefix,
 		RunArgs:      result.MergedConfig.RunArgs,
 		ContainerEnv: redactedContainerEnv(result.MergedConfig.ContainerEnv),
+		RemoteUser:   devcontainerconfig.GetRemoteUser(result),
 	}, nil
 }
 

cmd/snapshot/restore.go

@@ -94,6 +94,7 @@ func (cmd *RestoreCmd) Run(
 	if err != nil {
 		return fmt.Errorf("read snapshot container env: %w", err)
 	}
+	remoteUser := manifest.RemoteUser()
 
 	log.Infof("restoring snapshot: ref=%s workspaceId=%s", snapshotRef, ws.ID)
 
@@ -105,6 +106,7 @@ func (cmd *RestoreCmd) Run(
 		DevContainerSource: ws.DevContainerSource,
 		RunArgs:            runArgs,
 		ContainerEnv:       containerEnv,
+		RemoteUser:         remoteUser,
 	})
 }
 

cmd/workspace/up/up.go

@@ -86,6 +86,9 @@ type Options struct {
 	// the same suppressed-discovery circumstances as RunArgs. Used by
 	// snapshot restore to replay the original devcontainer.json's containerEnv.
 	ContainerEnv map[string]string
+	// RemoteUser is the remoteUser to replay under the same
+	// suppressed-discovery circumstances as RunArgs. Used by snapshot restore.
+	RemoteUser string
 }
 
 type HeadlessOptions struct {
@@ -202,6 +205,7 @@ func buildUpCmd(g *flags.GlobalFlags, opts Options) *UpCmd {
 	cmd.DevContainerSource = opts.DevContainerSource
 	cmd.RunArgs = opts.RunArgs
 	cmd.ContainerEnv = opts.ContainerEnv
+	cmd.RemoteUser = opts.RemoteUser
 	if opts.Name != "" {
 		cmd.ID = opts.Name
 	}

cmd/workspace/up/up_client.go

@@ -557,8 +557,9 @@ func (cmd *UpCmd) validateFromSnapshot(ctx context.Context, args []string) error
 }
 
 // applyFromSnapshotOverrides replays the create-time devcontainer.json
-// settings the snapshot's manifest carries (runArgs, containerEnv) onto cmd,
-// so the image-sourced restored container behaves like the original did.
+// settings the snapshot's manifest carries (runArgs, containerEnv,
+// remoteUser) onto cmd, so the image-sourced restored container behaves like
+// the original did.
 func (cmd *UpCmd) applyFromSnapshotOverrides(manifest *snapshotpkg.Manifest) error {
 	runArgs, err := manifest.RunArgs()
 	if err != nil {
@@ -571,6 +572,10 @@ func (cmd *UpCmd) applyFromSnapshotOverrides(manifest *snapshotpkg.Manifest) err
 		return fmt.Errorf("read --from-snapshot container env: %w", err)
 	}
 	cmd.ContainerEnv = containerEnv
+
+	if cmd.RemoteUser == "" {
+		cmd.RemoteUser = manifest.RemoteUser()
+	}
 	return nil
 }
 

cmd/workspace/up/up_test.go

@@ -10,6 +10,8 @@ import (
 	"github.com/devsy-org/devsy/pkg/config"
 	"github.com/devsy-org/devsy/pkg/flags/names"
 	"github.com/devsy-org/devsy/pkg/ide/opener"
+	provider2 "github.com/devsy-org/devsy/pkg/provider"
+	snapshotpkg "github.com/devsy-org/devsy/pkg/snapshot"
 	"github.com/google/go-containerregistry/pkg/registry"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -423,6 +425,36 @@ func TestUpCmd_ValidateFromSnapshotFailsOnMissingManifest(t *testing.T) {
 	assert.Contains(t, err.Error(), "validate --from-snapshot ref")
 }
 
+func TestUpCmd_ApplyFromSnapshotOverridesKeepsExplicitRemoteUserFlag(t *testing.T) {
+	cmd := &UpCmd{CLIOptions: provider2.CLIOptions{RemoteUser: "explicit-user"}}
+	manifest := mustBuildManifest(t, "snapshot-user")
+
+	require.NoError(t, cmd.applyFromSnapshotOverrides(manifest))
+
+	assert.Equal(t, "explicit-user", cmd.RemoteUser)
+}
+
+func TestUpCmd_ApplyFromSnapshotOverridesUsesSnapshotRemoteUserWhenUnset(t *testing.T) {
+	cmd := &UpCmd{}
+	manifest := mustBuildManifest(t, "snapshot-user")
+
+	require.NoError(t, cmd.applyFromSnapshotOverrides(manifest))
+
+	assert.Equal(t, "snapshot-user", cmd.RemoteUser)
+}
+
+func mustBuildManifest(t *testing.T, remoteUser string) *snapshotpkg.Manifest {
+	t.Helper()
+	manifest, err := snapshotpkg.BuildManifest(snapshotpkg.BuildManifestOptions{
+		WorkspaceUID:         "ws-uid",
+		ContainerImageDigest: "sha256:" + strings.Repeat("a", 64),
+		VolumesDigest:        "sha256:" + strings.Repeat("b", 64),
+		RemoteUser:           remoteUser,
+	})
+	require.NoError(t, err)
+	return manifest
+}
+
 func TestBuildUpCmd_DoesNotMutateCallerGlobalFlags(t *testing.T) {
 	g := &flags.GlobalFlags{Provider: "default-provider", ResultFormat: ""}
 

e2e/tests/snapshot/snapshot.go

@@ -21,6 +21,7 @@ const (
 	snapshotCmd         = "snapshot"
 	snapshotVerbCreate  = "create"
 	snapshotVerbRestore = "restore"
+	nonRootRemoteUser   = "devsyuser"
 )
 
 var _ = ginkgo.Describe("devsy snapshot", ginkgo.Label("snapshot"), func() {
@@ -430,11 +431,6 @@ var _ = ginkgo.Describe("devsy snapshot", ginkgo.Label("snapshot"), func() {
 		restoredWorkspace, err := f.FindWorkspace(ctx, restoredID)
 		framework.ExpectNoError(err)
 
-		// The custom --label runArg only exists in this fixture's
-		// devcontainer.json, not in the base image or --add-host (which the
-		// registry fixture itself already depends on to function at all): its
-		// presence on the restored container proves restore replays the
-		// original runArgs generally, not just the one the test harness needs.
 		containerIDs, err := dockerHelper.FindContainer(ctx, []string{
 			fmt.Sprintf("%s=%s", pkgconfig.DevcontainerIDLabel, restoredWorkspace.UID),
 			"devsy-e2e-snapshot-runargs=true",
@@ -445,4 +441,57 @@ var _ = ginkgo.Describe("devsy snapshot", ginkgo.Label("snapshot"), func() {
 			"restored container should carry the original devcontainer.json's custom runArg label",
 		)
 	}, ginkgo.SpecTimeout(framework.TimeoutLong()))
+
+	ginkgo.It("restores files owned by the remote user when reusing the original id", func(
+		ctx context.Context,
+	) {
+		initialDir, err := os.Getwd()
+		framework.ExpectNoError(err)
+
+		tempDir, err := framework.CopyToTempDir("tests/snapshot/testdata/docker-nonroot")
+		framework.ExpectNoError(err)
+		ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir)
+		ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, tempDir)
+		framework.ExpectNoError(f.DevsyUp(ctx, tempDir))
+
+		workspaceFolder, err := f.DevsySSH(ctx, tempDir, "pwd")
+		framework.ExpectNoError(err)
+		workspaceFolder = strings.TrimSpace(workspaceFolder)
+
+		markerCmd := fmt.Sprintf("echo mutated > %s/marker.txt", workspaceFolder)
+		_, err = f.DevsySSH(ctx, tempDir, markerCmd)
+		framework.ExpectNoError(err)
+
+		out, _, err := f.ExecCommandCapture(ctx, []string{
+			snapshotCmd, snapshotVerbCreate, tempDir, registryFlag, registryHost + "/e2e/snapshots",
+			debugFlag,
+		})
+		framework.ExpectNoError(err)
+		snapshotRef := strings.TrimSpace(out)
+
+		framework.ExpectNoError(f.DevsyWorkspaceDelete(ctx, tempDir))
+
+		_, _, err = f.ExecCommandCapture(ctx, []string{
+			snapshotCmd, snapshotVerbRestore, snapshotRef, debugFlag,
+		})
+		framework.ExpectNoError(err)
+
+		restoredWorkspaceFolder, err := f.DevsySSH(ctx, tempDir, "pwd")
+		framework.ExpectNoError(err)
+		restoredWorkspaceFolder = strings.TrimSpace(restoredWorkspaceFolder)
+
+		content, err := f.DevsySSH(
+			ctx, tempDir, fmt.Sprintf("cat %s/marker.txt", restoredWorkspaceFolder),
+		)
+		framework.ExpectNoError(err)
+		gomega.Expect(content).To(gomega.ContainSubstring("mutated"))
+
+		ownerCmd := fmt.Sprintf(
+			`test "$(stat -c %%U %s/marker.txt)" = %q && echo OWNER_OK || echo OWNER_MISMATCH`,
+			restoredWorkspaceFolder, nonRootRemoteUser,
+		)
+		ownerOut, err := f.DevsySSH(ctx, tempDir, ownerCmd)
+		framework.ExpectNoError(err)
+		gomega.Expect(ownerOut).To(gomega.ContainSubstring("OWNER_OK"))
+	}, ginkgo.SpecTimeout(framework.TimeoutLong()))
 })

e2e/tests/snapshot/testdata/docker-nonroot/.devcontainer.json

@@ -0,0 +1,11 @@
+{
+  "name": "snapshot non-root",
+  "build": {
+    "dockerfile": "Dockerfile"
+  },
+  "remoteUser": "devsyuser",
+  "runArgs": ["--add-host=host.docker.internal:host-gateway"],
+  "containerEnv": {
+    "DEVSY_INSECURE_DOCKER_INTERNAL": "true"
+  }
+}

e2e/tests/snapshot/testdata/docker-nonroot/Dockerfile

@@ -0,0 +1,3 @@
+FROM ghcr.io/devsy-org/test-images/base:ubuntu
+
+RUN useradd --create-home --shell /bin/bash devsyuser

pkg/agent/snapshot/restore.go

@@ -70,7 +70,10 @@ func RestoreVolumes(
 	}
 
 	levels := len(strings.Split(layer.MountPrefix, "/"))
-	if err := extract.Extract(rc, target, extract.StripLevels(levels)); err != nil {
+	if err := extract.Extract(
+		rc, target,
+		extract.StripLevels(levels), extract.PreserveHeaderOwnership(),
+	); err != nil {
 		return fmt.Errorf("extract snapshot volumes into %s: %w", target, err)
 	}
 	return nil

pkg/copy/copy.go

@@ -1,7 +1,6 @@
 package copy
 
 import (
-	"errors"
 	"fmt"
 	"io"
 	"io/fs"
@@ -28,6 +27,43 @@ func Chown(path string, userName string) error {
 	return os.Lchown(path, uidInt, gidInt)
 }
 
+// ChownFailure is one entry a recursive chown could not reassign.
+type ChownFailure struct {
+	Path string
+	Err  error
+}
+
+func (f ChownFailure) Error() string { return fmt.Sprintf("%s: %v", f.Path, f.Err) }
+
+func (f ChownFailure) Unwrap() error { return f.Err }
+
+// ChownFailures aggregates the entries ChownR could not chown.
+type ChownFailures []ChownFailure
+
+func (fs ChownFailures) Error() string {
+	return fmt.Sprintf("%d entries could not be chowned, first: %v", len(fs), fs[0])
+}
+
+func (fs ChownFailures) Unwrap() []error {
+	errs := make([]error, len(fs))
+	for i, f := range fs {
+		errs[i] = f
+	}
+	return errs
+}
+
+// AllDenied reports whether every failure was refused by the filesystem
+// (permission denied or read-only share); the expected case for entries on
+// virtiofs shares such as read-only .git pack files.
+func (fs ChownFailures) AllDenied() bool {
+	for _, f := range fs {
+		if !DeniedByFilesystem(f.Err) {
+			return false
+		}
+	}
+	return len(fs) > 0
+}
+
 func ChownR(path string, userName string) error {
 	if userName == "" {
 		return nil
@@ -44,28 +80,30 @@ func ChownR(path string, userName string) error {
 	// #nosec G115 -- a resolved system uid is non-negative and fits uint32.
 	uidU32 := uint32(uidInt)
 
-	// A single un-chownable entry (e.g. a read-only file on a virtiofs share)
-	// must not abort the walk and leave the rest of the tree unowned.
-	var errs []error
+	var failures ChownFailures
 	_ = filepath.WalkDir(path, func(name string, dirEntry fs.DirEntry, err error) error {
 		if err != nil {
-			errs = append(errs, err)
+			failures = append(failures, ChownFailure{Path: name, Err: err})
 			return nil
 		}
 		info, err := dirEntry.Info()
 		if err != nil {
+			failures = append(failures, ChownFailure{Path: name, Err: err})
 			return nil
 		}
 		if IsUID(info, uidU32) {
 			return nil
 		}
 		// #nosec G122 -- best-effort chown of a freshly provisioned tree we own; WalkDir yields real paths.
-		if err := os.Lchown(name, uidInt, gidInt); err != nil {
-			errs = append(errs, err)
+		if lerr := os.Lchown(name, uidInt, gidInt); lerr != nil {
+			failures = append(failures, ChownFailure{Path: name, Err: lerr})
 		}
 		return nil
 	})
-	return errors.Join(errs...)
+	if len(failures) == 0 {
+		return nil
+	}
+	return failures
 }
 
 func MkdirAllChown(path string, perm os.FileMode, userName string) error {

pkg/copy/copy_supported.go

@@ -3,6 +3,7 @@
 package copy
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"syscall"
@@ -13,6 +14,19 @@ func IsUID(info os.FileInfo, uid uint32) bool {
 	return ok && stat.Uid == uid
 }
 
+// DeniedByFilesystem reports whether err means the filesystem refused the
+// reassignment (insufficient privilege or a read-only share).
+func DeniedByFilesystem(err error) bool {
+	return errors.Is(err, os.ErrPermission) || errors.Is(err, syscall.EROFS)
+}
+
+// Unsupported reports whether err means chown itself is not a meaningful
+// operation on this platform (Windows only). On unix, chown is always a
+// real operation, so any failure here is a genuine denial, never this.
+func Unsupported(error) bool {
+	return false
+}
+
 func Lchown(info os.FileInfo, sourcePath, destPath string) error {
 	stat, ok := info.Sys().(*syscall.Stat_t)
 	if !ok {