refactor(config): phased VariableResolver with an inline ${...} scanner

Collapse the SubstituteHost/SubstituteContainer/SubstituteDevContainerID/
SubstituteTemplateOptions free functions into a single VariableResolver that
applies substitutions one explicit phase at a time (host, container, identity,
template) over a shared SubstitutionContext, and returns an error instead of
silently swallowing malformed input.

${...} tags are scanned by a ~25-line substituteTags helper: text/template is
the wrong tool here (its {{action}} model can't parse the opaque ${var:arg}
syntax, and it can't leave unknown tags verbatim for a later phase), and a
vendored engine was 200+ lines of dead API for one call site.

Callers migrate to the resolver: the up flow runs the full pre-container
pipeline (BeforeContainerInto) before consuming config fields and derives
id-labels up front; container-phase call sites (exec, read-configuration,
setup, up) share a resolveContainerVariables helper; mounts, initializeCommand
and feature host-var substitution thread the typed context and surface
resolution errors.

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

Author: aledbf

internal/cli/env.go

@@ -3,8 +3,17 @@ package cli
 import (
 	"os"
 	"strings"
+
+	"github.com/devcontainers/cli/internal/config"
 )
 
+func resolveContainerVariables(containerEnv map[string]string, value interface{}) (interface{}, error) {
+	return config.NewVariableResolver().AfterContainer(config.SubstitutionContext{
+		HostSubContext: config.HostSubContext{Platform: "linux"},
+		ContainerEnv:   containerEnv,
+	}, value)
+}
+
 // osEnvMap returns the current process environment as a map.
 func osEnvMap() map[string]string {
 	env := make(map[string]string)

internal/cli/exec.go

@@ -262,7 +262,10 @@ func runExec(ctx context.Context, opts *execOpts, cmdArgs []string) error {
 
 	// 8. Substitute ${containerEnv:X} in resolved remoteEnv
 	for k, v := range resolvedRemoteEnv {
-		substituted := config.SubstituteContainer("linux", containerEnv, v)
+		substituted, err := resolveContainerVariables(containerEnv, v)
+		if err != nil {
+			return fmt.Errorf("resolve container variables: %w", err)
+		}
 		if s, ok := substituted.(string); ok {
 			resolvedRemoteEnv[k] = s
 		}

internal/cli/feature_hostsub_test.go

@@ -22,7 +22,9 @@ func TestSubstituteFeatureHostVars(t *testing.T) {
 		}},
 	}}
 
-	substituteFeatureHostVars(sets, hs)
+	if err := substituteFeatureHostVars(sets, hs); err != nil {
+		t.Fatal(err)
+	}
 
 	f := sets[0].Features[0]
 	if got := f.ContainerEnv["CACHE"]; got != "/home/me/.cache" {

internal/cli/feature_install.go

@@ -469,22 +469,33 @@ func realignFeatureDirs(tmpDir string, sets []*features.Set) error {
 // 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).
-func substituteFeatureHostVars(sets []*features.Set, hs config.HostSubContext) {
+func substituteFeatureHostVars(sets []*features.Set, hs config.HostSubContext) error {
+	resolver := config.NewVariableResolver()
+	ctx := config.SubstitutionContext{HostSubContext: hs}
 	for _, fs := range sets {
 		for i := range fs.Features {
 			f := &fs.Features[i]
 			for k, v := range f.ContainerEnv {
-				if s, ok := config.SubstituteHost(hs, v).(string); ok {
+				resolved, err := resolver.Resolve(ctx, config.PhaseHost, v)
+				if err != nil {
+					return fmt.Errorf("resolve Feature %s containerEnv %s: %w", f.ID, k, err)
+				}
+				if s, ok := resolved.(string); ok {
 					f.ContainerEnv[k] = s
 				}
 			}
 			if len(f.Mounts) > 0 {
-				if m, ok := config.SubstituteHost(hs, f.Mounts).([]interface{}); ok {
+				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.([]interface{}); ok {
 					f.Mounts = m
 				}
 			}
 		}
 	}
+	return nil
 }
 
 func extendImageWithFeatures(
@@ -526,7 +537,9 @@ func extendImageWithFeatures(
 	// Resolve ${localEnv:…} / ${localWorkspaceFolder} / … in each Feature's
 	// containerEnv and mounts, like the config's own values (upstream #308).
 	if fbOpts != nil && fbOpts.HostSub != nil {
-		substituteFeatureHostVars(featureSets, *fbOpts.HostSub)
+		if err := substituteFeatureHostVars(featureSets, *fbOpts.HostSub); err != nil {
+			return nil, err
+		}
 	}
 
 	// Write/validate the features lockfile when requested (opt-in via the

internal/cli/mounts.go

@@ -102,22 +102,44 @@ func composeVolumeSpecsFromMetadata(entries []interface{}, devcontainerID string
 }
 
 func mountFromMetadata(entry interface{}, devcontainerID string) (dockermount.Mount, error) {
+	resolve := func(value string) (string, error) {
+		resolved, err := config.NewVariableResolver().Resolve(config.SubstitutionContext{
+			DevContainerID: devcontainerID,
+		}, config.PhaseIdentity, value)
+		if err != nil {
+			return "", err
+		}
+		result, ok := resolved.(string)
+		if !ok {
+			return "", fmt.Errorf("identity substitution returned %T for string", resolved)
+		}
+		return result, nil
+	}
 	switch raw := entry.(type) {
 	case string:
-		spec := config.SubstituteDevContainerIDString(devcontainerID, raw)
+		spec, err := resolve(raw)
+		if err != nil {
+			return dockermount.Mount{}, err
+		}
 		return docker.ParseMountSpec(spec)
 	case map[string]interface{}:
 		target, _ := raw["target"].(string)
 		// ${devcontainerId} must resolve in target too, not only source — the
 		// string form substitutes the whole spec, so the object form has to match
 		// (e.g. target "/cache/${devcontainerId}").
-		target = config.SubstituteDevContainerIDString(devcontainerID, target)
+		target, err := resolve(target)
+		if err != nil {
+			return dockermount.Mount{}, err
+		}
 		if target == "" {
 			return dockermount.Mount{}, fmt.Errorf("mount requires a target/destination")
 		}
 
 		source, _ := raw["source"].(string)
-		source = config.SubstituteDevContainerIDString(devcontainerID, source)
+		source, err = resolve(source)
+		if err != nil {
+			return dockermount.Mount{}, err
+		}
 		mountType, _ := raw["type"].(string)
 		if mountType == "" {
 			mountType = string(dockermount.TypeBind)

internal/cli/read_configuration.go

@@ -228,7 +228,10 @@ func runReadConfiguration(ctx context.Context, out Output, opts *readConfigOpts)
 		inspect, inspectErr := engine.InspectContainer(ctx, containerID)
 		if inspectErr == nil && inspect.Config != nil {
 			cEnv := envSliceToMap(inspect.Config.Env)
-			substituted := config.SubstituteContainer("linux", cEnv, cfgMap)
+			substituted, subErr := resolveContainerVariables(cEnv, cfgMap)
+			if subErr != nil {
+				return writeErrorResult(out, fmt.Sprintf("resolve configuration container variables: %v", subErr))
+			}
 			if subMap, ok := substituted.(map[string]interface{}); ok {
 				cfgMap = subMap
 			}
@@ -339,7 +342,10 @@ func runReadConfiguration(ctx context.Context, out Output, opts *readConfigOpts)
 				mergedJSON, _ := json.Marshal(merged)
 				var mergedGeneric interface{}
 				json.Unmarshal(mergedJSON, &mergedGeneric)
-				substituted := config.SubstituteContainer("linux", cEnv, mergedGeneric)
+				substituted, subErr := resolveContainerVariables(cEnv, mergedGeneric)
+				if subErr != nil {
+					return subErr
+				}
 				subJSON, _ := json.Marshal(substituted)
 				json.Unmarshal(subJSON, merged)
 			}

internal/cli/setup.go

@@ -183,7 +183,10 @@ func newSetUpCmd() *cobra.Command {
 					cfgJSON, _ := json.Marshal(cfg)
 					var cfgGeneric interface{}
 					json.Unmarshal(cfgJSON, &cfgGeneric)
-					substituted := config.SubstituteContainer("linux", containerEnv, cfgGeneric)
+					substituted, subErr := resolveContainerVariables(containerEnv, cfgGeneric)
+					if subErr != nil {
+						return writeErrorResult(out, fmt.Sprintf("resolve configuration container variables: %v", subErr))
+					}
 					// Clean null values and internal fields
 					if subMap, ok := substituted.(map[string]interface{}); ok {
 						for k, v := range subMap {
@@ -225,7 +228,10 @@ func newSetUpCmd() *cobra.Command {
 				mergedJSON, _ := json.Marshal(merged)
 				var mergedGeneric interface{}
 				json.Unmarshal(mergedJSON, &mergedGeneric)
-				substituted := config.SubstituteContainer("linux", containerEnv, mergedGeneric)
+				substituted, subErr := resolveContainerVariables(containerEnv, mergedGeneric)
+				if subErr != nil {
+					return writeErrorResult(out, fmt.Sprintf("resolve merged container variables: %v", subErr))
+				}
 				result["mergedConfiguration"] = substituted
 			}
 

internal/cli/up.go

@@ -321,25 +321,38 @@ func runUp(ctx context.Context, out Output, opts *upOpts) error {
 	}
 	cfg := loadResult.Config
 
+	// Derive id labels before consuming configuration fields so the complete
+	// pre-container pipeline can run first.
+	if len(idLabels) == 0 {
+		idLabels = []string{
+			fmt.Sprintf("devcontainer.local_folder=%s", workspaceFolder),
+		}
+		if loadResult.Config.ConfigFilePath != "" {
+			idLabels = append(idLabels, fmt.Sprintf("devcontainer.config_file=%s", loadResult.Config.ConfigFilePath))
+		}
+	}
+
 	// Merge --additional-features into config (config features have priority)
 	var mergeErr error
 	opts.lockfileExcludeIDs, mergeErr = mergeAdditionalFeatures(cfg, opts.additionalFeatures)
 	if mergeErr != nil {
 		return writeErrorResult(out, mergeErr.Error())
 	}
 
-	if derr := enforceDisallowedFeatures(ctx, cfg, logger); derr != nil {
-		return writeErrorJSON(out, coreerrors.ToErrorOutput(derr))
+	idCtx := config.SubstitutionContext{
+		HostSubContext: loadResult.HostSub,
+		DevContainerID: config.ComputeDevContainerID(idLabelsMap(idLabels)),
+	}
+	resolver := config.NewVariableResolver()
+	if err := resolver.BeforeContainerInto(idCtx, cfg); err != nil {
+		return writeErrorResult(out, fmt.Sprintf("apply pre-container substitutions: %v", err))
+	}
+	if err := resolver.BeforeContainerInto(idCtx, loadResult.WorkspaceConfig); err != nil {
+		return writeErrorResult(out, fmt.Sprintf("apply workspace pre-container substitutions: %v", err))
 	}
 
-	// Derive id labels from workspace if not provided via --id-label.
-	if len(idLabels) == 0 {
-		idLabels = []string{
-			fmt.Sprintf("devcontainer.local_folder=%s", workspaceFolder),
-		}
-		if loadResult.Config.ConfigFilePath != "" {
-			idLabels = append(idLabels, fmt.Sprintf("devcontainer.config_file=%s", loadResult.Config.ConfigFilePath))
-		}
+	if derr := enforceDisallowedFeatures(ctx, cfg, logger); derr != nil {
+		return writeErrorJSON(out, coreerrors.ToErrorOutput(derr))
 	}
 
 	// Apply --remove-existing-container up-front, BEFORE initializeCommand, so a
@@ -356,7 +369,7 @@ func runUp(ctx context.Context, out Output, opts *upOpts) error {
 	// A failure aborts `up` with an error outcome, matching the TS CLI (which
 	// throws a ContainerError). Swallowing it produced silently-broken setups.
 	if !cfg.InitializeCommand.IsEmpty() {
-		if err := lifecycle.RunInitializeCommand(ctx, logger, &cfg.InitializeCommand, workspaceFolder); err != nil {
+		if err := lifecycle.RunInitializeCommand(ctx, logger, &cfg.InitializeCommand, loadResult.HostSub); err != nil {
 			return writeErrorJSON(out, coreerrors.ToErrorOutput(&coreerrors.ContainerError{
 				Description: "The initializeCommand in the devcontainer.json failed.",
 			}))
@@ -492,7 +505,10 @@ func runUp(ctx context.Context, out Output, opts *upOpts) error {
 			cfgJSON, _ := json.Marshal(cfg)
 			var cfgGeneric interface{}
 			json.Unmarshal(cfgJSON, &cfgGeneric)
-			substituted := config.SubstituteContainer("linux", containerEnv, cfgGeneric)
+			substituted, subErr := resolveContainerVariables(containerEnv, cfgGeneric)
+			if subErr != nil {
+				return writeErrorResult(out, fmt.Sprintf("resolve configuration container variables: %v", subErr))
+			}
 			if subMap, ok := substituted.(map[string]interface{}); ok {
 				for k, v := range subMap {
 					if v == nil {
@@ -550,7 +566,10 @@ func runUp(ctx context.Context, out Output, opts *upOpts) error {
 			mergedJSON, _ := json.Marshal(merged)
 			var mergedGeneric interface{}
 			json.Unmarshal(mergedJSON, &mergedGeneric)
-			mergedSub := config.SubstituteContainer("linux", containerEnv, mergedGeneric)
+			mergedSub, subErr := resolveContainerVariables(containerEnv, mergedGeneric)
+			if subErr != nil {
+				return writeErrorResult(out, fmt.Sprintf("resolve merged container variables: %v", subErr))
+			}
 			// Carry over config-only properties spread into mergedConfiguration by
 			// the TS CLI that the typed MergedConfig does not model.
 			if mm, ok := mergedSub.(map[string]interface{}); ok {
@@ -569,6 +588,16 @@ func runUp(ctx context.Context, out Output, opts *upOpts) error {
 	return writeSuccessJSON(out, result)
 }
 
+func idLabelsMap(labels []string) map[string]string {
+	result := make(map[string]string, len(labels))
+	for _, label := range labels {
+		if i := strings.IndexByte(label, '='); i >= 0 {
+			result[label[:i]] = label[i+1:]
+		}
+	}
+	return result
+}
+
 // finishUp produces the JSON result for an existing container found via --id-label
 // (no config/loadResult available). cfg and loadResult may be nil.
 func (r *upRunner) finishUp(ctx context.Context, containerID string, cfg *config.DevContainer, loadResult *config.LoadResult) error {

internal/config/loader.go

@@ -169,7 +169,10 @@ func LoadDevContainerConfig(workspaceFolder, configPath, overrideConfigPath stri
 		ConfigFilePath:           config.ConfigFilePath,
 	}
 
-	substituted := SubstituteHost(ctx, raw)
+	substituted, err := NewVariableResolver().Resolve(SubstitutionContext{HostSubContext: ctx}, PhaseHost, raw)
+	if err != nil {
+		return nil, fmt.Errorf("apply host substitutions: %w", err)
+	}
 	if m, ok := substituted.(map[string]interface{}); ok {
 		raw = m
 	}