fix(features): keep host variables raw in the metadata label

docs/parity/parity-matrix.yaml

@@ -1093,6 +1093,48 @@ initial_cases:
     current_status: match
     notes: "The compose service's Features Dockerfile (appended to `FROM <image> AS dev_container_auto_added_stage_label`) has the same structure as the image build's: builtin env, consecutiveId folders, install wrapper, the Feature's containerEnv and, like TS getFeaturesBuildOptions, the config's containerEnv as ENV lines (CFG_VAR). Go used the _dev_container_feature_<i> layout and left the config's containerEnv out of the compose image."
 
+  - id: build.feature-host-variables-raw-label-success
+    lane: runtime
+    command: build
+    priority: p1
+    docker_required: true
+    setup_cmd: "WORKSPACE=/tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf $WORKSPACE; mkdir -p $WORKSPACE/.devcontainer/f; printf '%s' '{\"id\":\"f\",\"version\":\"1.0.0\",\"mounts\":[{\"source\":\"${localEnv:HOME}\",\"target\":\"/parity-home\",\"type\":\"bind\"}],\"postCreateCommand\":\"echo ${localEnv:HOME}\"}' > $WORKSPACE/.devcontainer/f/devcontainer-feature.json; printf '#!/bin/sh\\ntrue\\n' > $WORKSPACE/.devcontainer/f/install.sh; chmod +x $WORKSPACE/.devcontainer/f/install.sh; printf '%s' '{\"image\":\"alpine:3\",\"features\":{\"./f\":{}}}' > $WORKSPACE/.devcontainer/devcontainer.json; echo WORKSPACE=$WORKSPACE"
+    ts_cmd: "build --workspace-folder ${WORKSPACE} --image-name parity-${PARITY_CASE_ID}-${PARITY_SIDE}"
+    verify_cmd: "docker image inspect -f '{{index .Config.Labels \"devcontainer.metadata\"}}' ${IMAGE_NAME}"
+    cleanup_cmd: "docker rmi -f parity-${PARITY_CASE_ID}-${PARITY_SIDE} >/dev/null 2>&1; rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; true"
+    asserts: [exit_code, stdout_normalized]
+    class: build-runtime
+    current_status: match
+    notes: "TS getDevcontainerMetadata writes the Feature's raw values to the devcontainer.metadata label (imageMetadata.ts uses .raw), so a Feature mount and lifecycle command keep ${localEnv:HOME} and are resolved on the host that runs `up`. Go resolved the Feature's mounts before building the label, baking the build host's HOME into the image."
+
+  - id: up.compose-feature-host-variables-raw-label-success
+    lane: runtime
+    command: up
+    priority: p1
+    docker_required: true
+    setup_cmd: "WORKSPACE=/tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf $WORKSPACE; mkdir -p $WORKSPACE/.devcontainer/f; printf '%s' '{\"id\":\"f\",\"version\":\"1.0.0\",\"mounts\":[{\"source\":\"${localEnv:HOME}\",\"target\":\"/parity-home\",\"type\":\"bind\"}],\"postCreateCommand\":\"echo ${localEnv:HOME}\"}' > $WORKSPACE/.devcontainer/f/devcontainer-feature.json; printf '#!/bin/sh\\ntrue\\n' > $WORKSPACE/.devcontainer/f/install.sh; chmod +x $WORKSPACE/.devcontainer/f/install.sh; printf 'services:\\n  app:\\n    image: alpine:3\\n    command: sleep infinity\\n' > $WORKSPACE/.devcontainer/docker-compose.yml; printf '%s' '{\"dockerComposeFile\":\"docker-compose.yml\",\"service\":\"app\",\"workspaceFolder\":\"/\",\"features\":{\"./f\":{}}}' > $WORKSPACE/.devcontainer/devcontainer.json; echo WORKSPACE=$WORKSPACE"
+    ts_cmd: "up --workspace-folder ${WORKSPACE} --skip-post-create --update-remote-user-uid-default never --id-label parity.case=${PARITY_CASE_ID}-${PARITY_SIDE}"
+    verify_cmd: "docker inspect -f '{{index .Config.Labels \"devcontainer.metadata\"}}' ${CONTAINER_ID}; docker image inspect -f '{{index .Config.Labels \"devcontainer.metadata\"}}' $(docker inspect -f '{{.Image}}' ${CONTAINER_ID}); docker inspect -f '{{range .Mounts}}{{.Source}}->{{.Destination}} {{end}}' ${CONTAINER_ID}"
+    cleanup_cmd: "docker compose --project-name ${COMPOSE_PROJECT_NAME} -f ${WORKSPACE}/.devcontainer/docker-compose.yml down -t 0 >/dev/null 2>&1; rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; true"
+    asserts: [exit_code, stdout_normalized]
+    class: up-runtime
+    current_status: match
+    notes: "The compose service's image and container labels keep the Feature's ${localEnv:HOME} raw, as TS writes them; the bind mount the container gets is resolved on this host (TS getDevcontainerMetadata's featureRaw.map(substitute)). Go resolved the Feature's mounts before building the label."
+
+  - id: read-configuration.feature-host-variables-merged-success
+    lane: runtime
+    command: read-configuration
+    priority: p1
+    docker_required: true
+    setup_cmd: "WORKSPACE=/tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf $WORKSPACE; mkdir -p $WORKSPACE/.devcontainer/f; printf '%s' '{\"id\":\"f\",\"version\":\"1.0.0\",\"mounts\":[{\"source\":\"${localEnv:HOME}\",\"target\":\"/parity-home\",\"type\":\"bind\"}],\"postCreateCommand\":\"echo ${localEnv:HOME}\"}' > $WORKSPACE/.devcontainer/f/devcontainer-feature.json; printf '#!/bin/sh\\ntrue\\n' > $WORKSPACE/.devcontainer/f/install.sh; chmod +x $WORKSPACE/.devcontainer/f/install.sh; printf '%s' '{\"image\":\"alpine:3\",\"features\":{\"./f\":{}}}' > $WORKSPACE/.devcontainer/devcontainer.json; echo WORKSPACE=$WORKSPACE"
+    ts_cmd: "read-configuration --workspace-folder ${WORKSPACE} --include-merged-configuration"
+    verify_cmd: "true"
+    cleanup_cmd: "rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}"
+    asserts: [exit_code, stdout_normalized]
+    class: merged-configuration
+    current_status: match
+    notes: "Without a container, TS getDevcontainerMetadata resolves the Features' raw entries with the configuration's host substitution, so mergedConfiguration lists the Feature's mount and postCreateCommand with ${localEnv:HOME} resolved. Go listed them raw."
+
   - id: read-configuration.container-id-missing-error
     lane: runtime
     command: read-configuration

internal/cli/compose_up.go

@@ -399,11 +399,11 @@ func (r *upRunner) buildAndExtendCompose(ctx context.Context, s composeStart, pe
 		featureEntries = append(featureEntries, featureMetadataEntry(fs, s.skipPersistCustoms))
 	}
 	labelMetadata := slices.Concat(baseEntries, featureEntries, []imagemeta.Entry{r.configLabel})
-	metadata := slices.Clone(baseEntries)
+	metadata := slices.Concat(baseEntries, featureEntries)
 	if err := r.substitute(&metadata); err != nil {
 		return nil, fmt.Errorf("substitute the image's metadata: %w", err)
 	}
-	metadata = slices.Concat(metadata, featureEntries, []imagemeta.Entry{configMetadataEntry(cfg, false)})
+	metadata = append(metadata, configMetadataEntry(cfg, false))
 
 	result := &composeExtendResult{imageMetadata: metadata}
 	userSyntax := ""

internal/cli/compose_up_held_test.go

@@ -1278,3 +1278,32 @@ func TestBuildAndExtendComposeForTheBuildCommand(t *testing.T) {
 		}
 	}
 }
+
+func TestBuildAndExtendComposeKeepsFeatureHostVariablesRawInTheLabel(t *testing.T) {
+	h := newComposeHarness(t)
+	h.write("vars/devcontainer-feature.json", `{"id":"vars","version":"1.0.0","containerEnv":{"FE":"${localEnv:SRC}/env"},"mounts":[{"source":"${localEnv:SRC}","target":"/m","type":"bind"}],"postCreateCommand":"echo ${localEnv:SRC}"}`)
+	h.write("vars/install.sh", "#!/bin/sh\n")
+	s := h.start(&config.DevContainer{Features: map[string]any{"./vars": map[string]any{}}}, map[string]any{"image": "alpine:3"})
+	s.loadResult.HostSub = config.HostSubContext{Env: map[string]string{"SRC": "/host/src"}}
+	h.r.substitute = hostMetadataSubstitution(s.loadResult.HostSub, s.idLabels)
+	res, err := h.r.buildAndExtendCompose(context.Background(), s, h.persisted, "", true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	content := readFile(t, overrideDockerfile(t, readFile(t, res.overrideFiles[0])))
+	for _, want := range []string{`\"source\":\"\${localEnv:SRC}\"`, `\"postCreateCommand\":\"echo \${localEnv:SRC}\"`, `ENV FE="/host/src/env"`} {
+		if !strings.Contains(content, want) {
+			t.Errorf("Dockerfile lacks %s:\n%s", want, content)
+		}
+	}
+	if strings.Contains(content, `\"source\":\"/host/src\"`) {
+		t.Errorf("the label carries the build host's mount source:\n%s", content)
+	}
+	feature := res.imageMetadata[1]
+	if got := feature.Mounts[0].(map[string]any)["source"]; got != "/host/src" {
+		t.Errorf("the container's Feature mount source = %v, want /host/src", got)
+	}
+	if feature.PostCreateCommand != "echo /host/src" {
+		t.Errorf("the container's Feature postCreateCommand = %v, want echo /host/src", feature.PostCreateCommand)
+	}
+}

internal/cli/e2e_fixes_test.go

@@ -44,6 +44,11 @@ func TestE2E_FeatureUserHomeAndLocalEnv(t *testing.T) {
 		t.Skip("Docker not available")
 	}
 	t.Setenv("E2E_LOCALENV_MARKER", "resolved-localenv-1234")
+	mountSrc := t.TempDir()
+	if err := os.WriteFile(filepath.Join(mountSrc, "marker"), []byte("mounted-from-localenv"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv("E2E_LOCALENV_MOUNT", mountSrc)
 
 	ws := writeFixture(t, map[string]string{
 		".devcontainer/devcontainer.json": `{
@@ -55,7 +60,8 @@ func TestE2E_FeatureUserHomeAndLocalEnv(t *testing.T) {
 			"id": "localfeat",
 			"version": "1.0.0",
 			"name": "localfeat",
-			"containerEnv": { "MY_LOCAL_ENV": "${localEnv:E2E_LOCALENV_MARKER}" }
+			"containerEnv": { "MY_LOCAL_ENV": "${localEnv:E2E_LOCALENV_MARKER}" },
+			"mounts": [{ "source": "${localEnv:E2E_LOCALENV_MOUNT}", "target": "/e2e-localenv-mount", "type": "bind" }]
 		}`,
 		".devcontainer/localfeat/install.sh": "#!/bin/sh\nset -e\n" +
 			`echo "REMOTE_USER_HOME=[$_REMOTE_USER_HOME]" > /e2e-feature-marker` + "\n",
@@ -75,6 +81,16 @@ func TestE2E_FeatureUserHomeAndLocalEnv(t *testing.T) {
 	if got := dockerExec(t, id, "printenv MY_LOCAL_ENV"); got != "resolved-localenv-1234" {
 		t.Errorf("#308 Feature containerEnv ${localEnv} = %q, want resolved-localenv-1234", got)
 	}
+	if got := dockerExec(t, id, "cat /e2e-localenv-mount/marker"); got != "mounted-from-localenv" {
+		t.Errorf("Feature mount ${localEnv} not resolved on the host running up: %q", got)
+	}
+	out, err := exec.Command("sh", "-c", `docker image inspect -f '{{index .Config.Labels "devcontainer.metadata"}}' "$(docker inspect -f '{{.Image}}' `+id+`)"`).CombinedOutput()
+	if err != nil {
+		t.Fatalf("inspect the container's image: %v\n%s", err, out)
+	}
+	if label := string(out); !strings.Contains(label, `"source":"${localEnv:E2E_LOCALENV_MOUNT}"`) || strings.Contains(label, mountSrc) {
+		t.Errorf("image label = %s, want the Feature's mount source raw", label)
+	}
 }
 
 func TestE2E_RemoveExistingBeforeInitialize(t *testing.T) {

internal/cli/feature_hostsub_test.go

@@ -31,7 +31,7 @@ func TestSubstituteFeatureHostVars(t *testing.T) {
 	if got := f.ContainerEnv["WS"]; got != "/home/me/proj/x" {
 		t.Errorf("containerEnv WS = %q, want /home/me/proj/x", got)
 	}
-	if got := f.Mounts[0].(string); got != "source=/home/me/.ssh,target=/root/.ssh,type=bind" {
-		t.Errorf("mount = %q, want resolved localEnv:HOME", got)
+	if got := f.Mounts[0].(string); got != "source=${localEnv:HOME}/.ssh,target=/root/.ssh,type=bind" {
+		t.Errorf("mount = %q, want it raw", got)
 	}
 }

internal/cli/feature_install.go

@@ -548,10 +548,8 @@ func engineAtLeast23(ctx context.Context, engine *docker.EngineClient) bool {
 }
 
 // substituteFeatureHostVars resolves host-side variables (${localEnv:…},
-// ${localWorkspaceFolder}, …) in each Feature's containerEnv values and mounts,
-// so a Feature mount like "${localEnv:HOME}/.cfg" reaches Docker resolved —
-// matching how the config's own containerEnv/mounts are substituted (upstream
-// #308, which only substituted devcontainer.json's own values).
+// ${localWorkspaceFolder}, …) in each Feature's containerEnv, which the
+// Features Dockerfile writes as ENV lines (upstream #308).
 func substituteFeatureHostVars(sets []*features.Set, hs config.HostSubContext) error {
 	resolver := config.NewVariableResolver()
 	ctx := config.SubstitutionContext{HostSubContext: hs}
@@ -567,15 +565,6 @@ func substituteFeatureHostVars(sets []*features.Set, hs config.HostSubContext) e
 					f.ContainerEnv[k] = s
 				}
 			}
-			if len(f.Mounts) > 0 {
-				resolved, err := resolver.Resolve(ctx, config.PhaseHost, f.Mounts)
-				if err != nil {
-					return fmt.Errorf("resolve Feature %s mounts: %w", f.ID, err)
-				}
-				if m, ok := resolved.([]any); ok {
-					f.Mounts = m
-				}
-			}
 		}
 	}
 	return nil

internal/cli/feature_install_nodocker_held_test.go

@@ -21,7 +21,7 @@ func TestExtendImageWithFeaturesResolvesFeatureHostVars(t *testing.T) {
 	if err := os.MkdirAll(dir, 0o755); err != nil {
 		t.Fatal(err)
 	}
-	if err := os.WriteFile(filepath.Join(dir, "devcontainer-feature.json"), []byte(`{"id":"envy","version":"1.0.0","containerEnv":{"FROM_HOST":"${localEnv:HOST_VALUE}"}}`), 0o644); err != nil {
+	if err := os.WriteFile(filepath.Join(dir, "devcontainer-feature.json"), []byte(`{"id":"envy","version":"1.0.0","containerEnv":{"FROM_HOST":"${localEnv:HOST_VALUE}"},"mounts":[{"source":"${localEnv:HOST_VALUE}","target":"/m","type":"bind"}]}`), 0o644); err != nil {
 		t.Fatal(err)
 	}
 	if err := os.WriteFile(filepath.Join(dir, "install.sh"), []byte("#!/bin/sh\n"), 0o755); err != nil {
@@ -53,9 +53,12 @@ func TestExtendImageWithFeaturesResolvesFeatureHostVars(t *testing.T) {
 	if err != nil {
 		t.Fatalf("extended Dockerfile: %v (calls %v)", err, r.calls)
 	}
-	if !strings.Contains(string(data), "resolved-on-host") || strings.Contains(string(data), "localEnv") {
+	if !strings.Contains(string(data), `ENV FROM_HOST="resolved-on-host"`) {
 		t.Errorf("extended Dockerfile = %s, want FROM_HOST resolved from the host", data)
 	}
+	if !strings.Contains(string(data), `\"source\":\"\${localEnv:HOST_VALUE}\"`) || strings.Contains(string(data), `\"source\":\"resolved-on-host\"`) {
+		t.Errorf("extended Dockerfile = %s, want the label's Feature mount raw", data)
+	}
 }
 
 func TestExtendImageWithFeaturesBuildContext(t *testing.T) {

internal/cli/metadata_label.go

@@ -53,13 +53,30 @@ type metadataSubstitution func(entries *[]imagemeta.Entry) error
 
 func hostMetadataSubstitution(hostSub config.HostSubContext, idLabels []string) metadataSubstitution {
 	return func(entries *[]imagemeta.Entry) error {
+		// Resolve a copy: decoding into *entries would write through the
+		// mounts, commands and maps they share with the raw entries that
+		// become the devcontainer.metadata label.
+		data, err := json.Marshal(*entries)
+		if err != nil {
+			return fmt.Errorf("copy metadata entries: %w", err)
+		}
+		var resolved []imagemeta.Entry
+		if err := json.Unmarshal(data, &resolved); err != nil {
+			return fmt.Errorf("copy metadata entries: %w", err)
+		}
 		resolver := config.NewVariableResolver()
 		ctx := config.SubstitutionContext{HostSubContext: hostSub}
 		if idLabels == nil {
-			return resolver.ResolveInto(ctx, config.PhaseHost, entries)
+			err = resolver.ResolveInto(ctx, config.PhaseHost, &resolved)
+		} else {
+			ctx.DevContainerID = config.ComputeDevContainerID(config.EnvMap(idLabels))
+			err = resolver.BeforeContainerInto(ctx, &resolved)
+		}
+		if err != nil {
+			return err
 		}
-		ctx.DevContainerID = config.ComputeDevContainerID(config.EnvMap(idLabels))
-		return resolver.BeforeContainerInto(ctx, entries)
+		*entries = resolved
+		return nil
 	}
 }
 

internal/cli/metadata_label_test.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"errors"
 	"reflect"
+	"slices"
 	"testing"
 
 	"github.com/devcontainers/cli/internal/config"
@@ -141,3 +142,25 @@ func TestHostMetadataSubstitution(t *testing.T) {
 		t.Errorf("with id labels: ID = %q, want %q", entries[0].ContainerEnv["ID"], want)
 	}
 }
+
+func TestHostMetadataSubstitutionLeavesTheRawEntriesRaw(t *testing.T) {
+	raw := []imagemeta.Entry{{
+		ID:                "f",
+		Mounts:            []any{map[string]any{"source": "${localEnv:SRC}", "target": "/m", "type": "bind"}},
+		PostCreateCommand: []any{"echo", "${localEnv:SRC}"},
+		Customizations:    map[string]any{"x": "${localEnv:SRC}"},
+	}}
+	label := imagemeta.GenerateMetadataLabel(raw)
+	resolved := slices.Clone(raw)
+	for _, idLabels := range [][]string{nil, {"devcontainer.local_folder=/ws"}} {
+		if err := hostMetadataSubstitution(config.HostSubContext{Env: map[string]string{"SRC": "/host/src"}}, idLabels)(&resolved); err != nil {
+			t.Fatal(err)
+		}
+		if got := resolved[0].Mounts[0].(map[string]any)["source"]; got != "/host/src" {
+			t.Errorf("idLabels %v: resolved mount source = %v, want /host/src", idLabels, got)
+		}
+		if got := imagemeta.GenerateMetadataLabel(raw); got != label {
+			t.Errorf("idLabels %v: the raw entries changed:\n got %s\nwant %s", idLabels, got, label)
+		}
+	}
+}

internal/cli/read_configuration.go

@@ -210,6 +210,9 @@ func runReadConfiguration(ctx context.Context, out Output, opts *readConfigOpts)
 			for _, fs := range featResult.FeatureSets {
 				featureEntries = append(featureEntries, featureMetadataEntry(fs, false))
 			}
+			if err := hostMetadataSubstitution(result.HostSub, nil)(&featureEntries); err != nil {
+				return readConfigurationFailure(lgr, fmt.Errorf("substitute the Features' metadata: %w", err))
+			}
 		}
 		entries, entriesErr := readConfigurationMetadata(append(base, featureEntries...), featureEntries, containerCfg, result, opts.idLabels, workspaceFolder, lgr)
 		if entriesErr != nil {

internal/cli/up.go

@@ -305,7 +305,7 @@ func runUp(ctx context.Context, out Output, opts *upOpts) error {
 		return writeErrorResult(out, fmt.Sprintf("apply workspace pre-container substitutions: %v", err))
 	}
 	run.idLabels = idLabels
-	run.substitute = func(entries *[]imagemeta.Entry) error { return resolver.BeforeContainerInto(idCtx, entries) }
+	run.substitute = hostMetadataSubstitution(loadResult.HostSub, idLabels)
 	if run.configLabel, err = configLabelEntry(loadResult.RawConfig, opts.omitConfigRemoteEnvFromMeta); err != nil {
 		return writeErrorResult(out, err.Error())
 	}