fix(up): pull the Dockerfile base image before reading its metadata

up's Dockerfile path computed devcontainer metadata from the FROM image BEFORE
the build, but (unlike the `build` command) never pulled it when absent. On a
fresh run the base image is not cached yet, so InspectImage failed and the
base's baked metadata — e.g. `remoteUser=node` on the mcr devcontainers images —
was silently dropped. The CLI then stamped a metadata label without a remoteUser,
so exec/lifecycle fell back to root instead of node.

Extract the inspect→pull→inspect fallback into
EngineClient.ImageLabelsEnsuringPresent (shared by up and build, DRYing the two
paths) and unit-test all three branches (present / absent-then-pulled /
unpullable) against a full ImagePullResponse fake that streams like the real
client. End-to-end this fixes
exec.workspace-dockerfile-with-features-hello-success (TS `howdy, node!` vs Go
`howdy, root!`).

Also adds the previously-missing unit tests for parsePlatform (#1241) and
buildSecretIDs (#1078).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: aledbf

internal/cli/build.go

@@ -338,14 +338,7 @@ func (r *buildRunner) buildDockerfile(ctx context.Context, cfg *config.DevContai
 
 	// Generate base-image metadata. Ensure the base image is available locally so
 	// metadata from the published image label is preserved.
-	var baseLabels map[string]string
-	if baseInspect, inspErr := engine.InspectImage(ctx, baseImage); inspErr == nil && baseInspect.Config != nil {
-		baseLabels = baseInspect.Config.Labels
-	} else if pullErr := engine.PullImage(ctx, baseImage); pullErr == nil {
-		if baseInspect, inspErr := engine.InspectImage(ctx, baseImage); inspErr == nil && baseInspect.Config != nil {
-			baseLabels = baseInspect.Config.Labels
-		}
-	}
+	baseLabels := engine.ImageLabelsEnsuringPresent(ctx, baseImage)
 	baseMetadata := imagemeta.ReadMetadataFromLabels(baseLabels, logger)
 	metadata := append([]imagemeta.Entry{}, baseMetadata...)
 	// Preserve a `LABEL devcontainer.metadata` declared in the user's Dockerfile

internal/cli/pure_helpers_test.go

@@ -444,3 +444,21 @@ func TestFindPlatformArg(t *testing.T) {
 		}
 	}
 }
+
+// TestBuildSecretIDs covers #1078: extracting the KEY from each "KEY=VALUE"
+// build secret for mounting as --mount=type=secret,id=KEY.
+func TestBuildSecretIDs(t *testing.T) {
+	got := buildSecretIDs([]string{"NPM_TOKEN=abc", "GH_TOKEN=xyz=with=eq", "=novalue", "noeq"})
+	want := []string{"NPM_TOKEN", "GH_TOKEN"}
+	if len(got) != len(want) {
+		t.Fatalf("buildSecretIDs = %v, want %v", got, want)
+	}
+	for i := range want {
+		if got[i] != want[i] {
+			t.Errorf("id[%d] = %q, want %q", i, got[i], want[i])
+		}
+	}
+	if len(buildSecretIDs(nil)) != 0 {
+		t.Error("buildSecretIDs(nil) should be empty")
+	}
+}

internal/cli/up.go

@@ -763,9 +763,12 @@ func (r *upRunner) fromDockerfile(ctx context.Context, cfg *config.DevContainer,
 	baseImage := docker.FindBaseImage(prep.Parsed, buildArgsFromConfig(cfg), findTarget)
 
 	metadata := []imagemeta.Entry{}
-	if baseInspect, inspErr := engine.InspectImage(ctx, baseImage); inspErr == nil && baseInspect.Config != nil {
-		metadata = append(metadata, imagemeta.ReadMetadataFromLabels(baseInspect.Config.Labels, logger)...)
-	}
+	// Read the FROM image's baked devcontainer.metadata (e.g. remoteUser=node on
+	// the mcr devcontainers images). The metadata is computed before the build, so
+	// on a fresh run the base image is otherwise absent and its metadata would be
+	// silently lost — making exec/lifecycle fall back to root.
+	baseLabels := engine.ImageLabelsEnsuringPresent(ctx, baseImage)
+	metadata = append(metadata, imagemeta.ReadMetadataFromLabels(baseLabels, logger)...)
 	// Preserve a `LABEL devcontainer.metadata` from the user's Dockerfile (#1225).
 	metadata = append(metadata, imagemeta.ReadMetadataFromLabels(prep.Parsed.StageLabels(stageName), logger)...)
 	metadata = append(metadata, imagemeta.Entry{RemoteUser: cfg.RemoteUser, ContainerUser: cfg.ContainerUser})

internal/docker/engine.go

@@ -223,6 +223,23 @@ func (e *EngineClient) PullImage(ctx context.Context, ref string) error {
 	return e.PullImagePlatform(ctx, ref, "")
 }
 
+// ImageLabelsEnsuringPresent returns ref's image labels, pulling ref first when
+// it is not present locally so its baked metadata (e.g. the devcontainer.metadata
+// label carrying remoteUser) is not lost when read before a build. Returns nil
+// when the image can't be inspected even after a pull.
+func (e *EngineClient) ImageLabelsEnsuringPresent(ctx context.Context, ref string) map[string]string {
+	if info, err := e.InspectImage(ctx, ref); err == nil && info.Config != nil {
+		return info.Config.Labels
+	}
+	if err := e.PullImage(ctx, ref); err != nil {
+		return nil
+	}
+	if info, err := e.InspectImage(ctx, ref); err == nil && info.Config != nil {
+		return info.Config.Labels
+	}
+	return nil
+}
+
 // parsePlatform splits an "os/arch[/variant]" platform string into an OCI
 // platform. Malformed input yields a best-effort value (the daemon rejects it).
 func parsePlatform(s string) ocispec.Platform {

internal/docker/engine_test.go

@@ -5,10 +5,16 @@ import (
 	"errors"
 	"testing"
 
+	dockerspec "github.com/moby/docker-image-spec/specs-go/v1"
 	"github.com/moby/moby/api/types/container"
 	"github.com/moby/moby/api/types/events"
 	"github.com/moby/moby/api/types/image"
+	"github.com/moby/moby/api/types/jsonstream"
 	mobyclient "github.com/moby/moby/client"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"io"
+	"iter"
+	"strings"
 
 	"github.com/devcontainers/cli/internal/log"
 )
@@ -84,7 +90,7 @@ func (m *mockAPI) ImagePull(ctx context.Context, ref string, opts mobyclient.Ima
 	if m.imagePullFn != nil {
 		return m.imagePullFn(ctx, ref, opts)
 	}
-	return nil, nil
+	return fakePullResponse{strings.NewReader("")}, nil
 }
 
 func (m *mockAPI) ContainerInspect(ctx context.Context, id string, _ mobyclient.ContainerInspectOptions) (mobyclient.ContainerInspectResult, error) {
@@ -395,3 +401,107 @@ func TestEvents(t *testing.T) {
 		t.Errorf("got %d messages, want 2", len(msgs))
 	}
 }
+
+// TestImageLabelsEnsuringPresent guards the inspect→pull→inspect fallback: the
+// base image's baked metadata (e.g. devcontainer.metadata remoteUser) must be
+// read even on a fresh run where the image is not cached yet. Without the pull
+// the metadata is lost and exec/lifecycle fall back to root.
+func TestImageLabelsEnsuringPresent(t *testing.T) {
+	labels := map[string]string{"devcontainer.metadata": `[{"remoteUser":"node"}]`}
+	withLabels := func() (image.InspectResponse, error) {
+		return image.InspectResponse{Config: &dockerspec.DockerOCIImageConfig{
+			ImageConfig: ocispec.ImageConfig{Labels: labels},
+		}}, nil
+	}
+	notFound := func() (image.InspectResponse, error) {
+		return image.InspectResponse{}, errors.New("no such image")
+	}
+
+	t.Run("present: returns labels without pulling", func(t *testing.T) {
+		pulled := false
+		api := &mockAPI{
+			imageInspectFn: func(context.Context, string, ...mobyclient.ImageInspectOption) (image.InspectResponse, error) {
+				return withLabels()
+			},
+			imagePullFn: func(context.Context, string, mobyclient.ImagePullOptions) (mobyclient.ImagePullResponse, error) {
+				pulled = true
+				return fakePullResponse{strings.NewReader("pulling\n")}, nil
+			},
+		}
+		got := newTestEngine(api).ImageLabelsEnsuringPresent(t.Context(), "img")
+		if got["devcontainer.metadata"] != labels["devcontainer.metadata"] {
+			t.Errorf("labels = %v", got)
+		}
+		if pulled {
+			t.Error("pulled despite the image already being present")
+		}
+	})
+
+	t.Run("absent: pulls then returns labels", func(t *testing.T) {
+		n, pulled := 0, false
+		api := &mockAPI{
+			imageInspectFn: func(context.Context, string, ...mobyclient.ImageInspectOption) (image.InspectResponse, error) {
+				n++
+				if n == 1 {
+					return notFound()
+				}
+				return withLabels()
+			},
+			imagePullFn: func(context.Context, string, mobyclient.ImagePullOptions) (mobyclient.ImagePullResponse, error) {
+				pulled = true
+				return fakePullResponse{strings.NewReader("pulling\n")}, nil
+			},
+		}
+		got := newTestEngine(api).ImageLabelsEnsuringPresent(t.Context(), "img")
+		if !pulled {
+			t.Error("did not pull the missing image")
+		}
+		if got["devcontainer.metadata"] != labels["devcontainer.metadata"] {
+			t.Errorf("labels after pull = %v", got)
+		}
+	})
+
+	t.Run("absent and unpullable: nil", func(t *testing.T) {
+		api := &mockAPI{
+			imageInspectFn: func(context.Context, string, ...mobyclient.ImageInspectOption) (image.InspectResponse, error) {
+				return notFound()
+			},
+			imagePullFn: func(context.Context, string, mobyclient.ImagePullOptions) (mobyclient.ImagePullResponse, error) {
+				return nil, errors.New("pull failed")
+			},
+		}
+		if got := newTestEngine(api).ImageLabelsEnsuringPresent(t.Context(), "img"); got != nil {
+			t.Errorf("labels = %v, want nil", got)
+		}
+	})
+}
+
+// TestParsePlatform covers #1241 platform parsing into an OCI platform.
+func TestParsePlatform(t *testing.T) {
+	cases := []struct {
+		in             string
+		os, arch, var_ string
+	}{
+		{"linux/amd64", "linux", "amd64", ""},
+		{"linux/arm64/v8", "linux", "arm64", "v8"},
+		{"linux", "linux", "", ""},
+	}
+	for _, c := range cases {
+		p := parsePlatform(c.in)
+		if p.OS != c.os || p.Architecture != c.arch || p.Variant != c.var_ {
+			t.Errorf("parsePlatform(%q) = %+v", c.in, p)
+		}
+	}
+}
+
+// fakePullResponse is a minimal but complete mobyclient.ImagePullResponse for
+// tests: it streams the wrapped reader (so io.Copy drains it exactly like the
+// real client) and satisfies the JSONMessages/Wait/Close surface.
+type fakePullResponse struct{ r io.Reader }
+
+func (f fakePullResponse) Read(p []byte) (int, error) { return f.r.Read(p) }
+func (f fakePullResponse) Close() error               { return nil }
+func (f fakePullResponse) Wait(context.Context) error { return nil }
+func (f fakePullResponse) JSONMessages(context.Context) iter.Seq2[jsonstream.Message, error] {
+	return func(func(jsonstream.Message, error) bool) {}
+}