From c5241ca184828fd890fa0651f355b4c3fe7fd8bb Mon Sep 17 00:00:00 2001 From: Samuel K Date: Tue, 7 Jul 2026 17:18:02 -0500 Subject: [PATCH] fix(docker): remap container UID when remote user comes from image metadata The container UID/GID remap gated on the raw devcontainer.json's remoteUser/containerUser, but those commonly come from image metadata (e.g. mcr.microsoft.com/devcontainers/* images set remoteUser=vscode). With the raw config empty, shouldUpdateUserUID returned false, the remap was skipped, and the in-container chown left the bind-mounted workspace owned by the container UID (1000). On the next up the host agent (a different UID) could no longer read the tree, reported "Couldn't find a devcontainer.json", and failed writing a default one with permission denied. Pass the merged config's resolved user identity to the remap on both the single-container and compose paths so it matches the host user. --- pkg/devcontainer/compose.go | 13 +++++++------ pkg/devcontainer/single.go | 17 ++++++++++++++++- pkg/devcontainer/single_test.go | 29 +++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/pkg/devcontainer/compose.go b/pkg/devcontainer/compose.go index 7d6f5eac3..3caf83fd1 100644 --- a/pkg/devcontainer/compose.go +++ b/pkg/devcontainer/compose.go @@ -331,10 +331,6 @@ func (r *runner) finalizeComposeContainer( return nil, fmt.Errorf("get image metadata from container: %w", err) } - if err := r.updateContainerUserUID(ctx, parsedConfig); err != nil { - return nil, err - } - mergedConfig, err := mergeImageMetadataConfig( ctx, parsedConfig, @@ -345,6 +341,11 @@ func (r *runner) finalizeComposeContainer( return nil, err } + resolvedConfig := withResolvedUser(parsedConfig.Config, mergedConfig) + if err := r.updateContainerUserUID(ctx, resolvedConfig); err != nil { + return nil, err + } + // expose the compose project name inside the container if mergedConfig.RemoteEnv == nil { mergedConfig.RemoteEnv = map[string]*string{} @@ -372,7 +373,7 @@ func (r *runner) finalizeComposeContainer( // updateContainerUserUID updates the container user's UID/GID on Docker drivers. func (r *runner) updateContainerUserUID( ctx context.Context, - parsedConfig *config.SubstitutedConfig, + parsedConfig *config.DevContainerConfig, ) error { dockerDriver, ok := r.Driver.(driver.DockerDriver) if !ok { @@ -383,7 +384,7 @@ func (r *runner) updateContainerUserUID( if err := dockerDriver.UpdateContainerUserUID( ctx, r.ID, - parsedConfig.Config, + parsedConfig, writer, ); err != nil { log.Errorf("failed to update container user UID/GID: error=%v", err) diff --git a/pkg/devcontainer/single.go b/pkg/devcontainer/single.go index d8350147d..990dcffd6 100644 --- a/pkg/devcontainer/single.go +++ b/pkg/devcontainer/single.go @@ -534,7 +534,7 @@ func (r *runner) runContainer( return dockerDriver.RunDockerDevContainer(ctx, &driver.RunDockerDevContainerParams{ WorkspaceID: r.ID, Options: runOptions, - ParsedConfig: p.parsedConfig.Config, + ParsedConfig: withResolvedUser(p.parsedConfig.Config, mergedConfig), IDE: r.WorkspaceConfig.Workspace.IDE.Name, IDEOptions: r.WorkspaceConfig.Workspace.IDE.Options, LocalWorkspaceFolder: r.LocalWorkspaceFolder, @@ -546,6 +546,21 @@ func (r *runner) runContainer( return r.Driver.RunDevContainer(ctx, r.ID, runOptions) } +// withResolvedUser returns a copy of parsedConfig carrying the effective user +// identity from the merged config. remoteUser/containerUser/updateRemoteUserUID +// often come from image metadata rather than the raw devcontainer.json, so the +// container UID/GID remap must see the merged values or it silently skips. +func withResolvedUser( + parsedConfig *config.DevContainerConfig, + mergedConfig *config.MergedDevContainerConfig, +) *config.DevContainerConfig { + resolved := config.CloneDevContainerConfig(parsedConfig) + resolved.RemoteUser = mergedConfig.RemoteUser + resolved.ContainerUser = mergedConfig.ContainerUser + resolved.UpdateRemoteUserUID = mergedConfig.UpdateRemoteUserUID + return resolved +} + // parseWorkspaceMount parses the substituted workspace mount, returning nil when // it has been suppressed via an empty workspaceMount. func parseWorkspaceMount(substitutionContext *config.SubstitutionContext) *config.Mount { diff --git a/pkg/devcontainer/single_test.go b/pkg/devcontainer/single_test.go index ae0227faf..6c6c70db6 100644 --- a/pkg/devcontainer/single_test.go +++ b/pkg/devcontainer/single_test.go @@ -91,3 +91,32 @@ func TestWorkspaceMountDestination(t *testing.T) { //nolint:funlen // table-driv }) } } + +func TestWithResolvedUser(t *testing.T) { + parsed := &config.DevContainerConfig{} + parsed.RunArgs = []string{"--cap-add=SYS_PTRACE"} + + uid := true + merged := &config.MergedDevContainerConfig{} + merged.RemoteUser = "vscode" + merged.ContainerUser = "node" + merged.UpdateRemoteUserUID = &uid + + got := withResolvedUser(parsed, merged) + + if got.RemoteUser != "vscode" { + t.Errorf("RemoteUser = %q, want vscode", got.RemoteUser) + } + if got.ContainerUser != "node" { + t.Errorf("ContainerUser = %q, want node", got.ContainerUser) + } + if got.UpdateRemoteUserUID == nil || !*got.UpdateRemoteUserUID { + t.Errorf("UpdateRemoteUserUID = %v, want true", got.UpdateRemoteUserUID) + } + if len(got.RunArgs) != 1 || got.RunArgs[0] != "--cap-add=SYS_PTRACE" { + t.Errorf("RunArgs not preserved: %v", got.RunArgs) + } + if parsed.RemoteUser != "" { + t.Error("source config must not be mutated") + } +}