Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pkg/devcontainer/config/extends.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,9 @@ func parseDevContainerJSONFileWithVisited(
}
devContainer.Origin = absPath

// Recursively resolve extends
// Recursively resolve extends — substitute variables in refs first
if !devContainer.Extends.IsEmpty() {
devContainer.Extends = substituteExtendsRefs(devContainer.Extends, absPath)
declaringDir := filepath.Dir(absPath)
parent, err := resolveExtendsArray(ctx, devContainer.Extends, declaringDir, visited)
if err != nil {
Expand Down
90 changes: 90 additions & 0 deletions pkg/devcontainer/config/extends_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,96 @@
}
}

func TestExtends_VarSub_LocalEnv(t *testing.T) {
tmpDir := t.TempDir()
subDir := filepath.Join(tmpDir, ".devcontainer")
// #nosec G301 -- test directory
if err := os.MkdirAll(subDir, 0o750); err != nil {
t.Fatal(err)
}

writeJSON(t, subDir, "parent.json", `{
"image": "ubuntu:22.04",
"remoteUser": "dev"
}`)

t.Setenv("DEVSY_TEST_EXTENDS_DIR", subDir)

childPath := writeJSON(t, subDir, "devcontainer.json", `{
"extends": "${localEnv:DEVSY_TEST_EXTENDS_DIR}/parent.json",
"name": "child-with-env"
}`)

cfg, err := ParseDevContainerJSONFile(childPath)
if err != nil {
t.Fatal(err)
}
if cfg.Name != "child-with-env" {
t.Errorf("expected name 'child-with-env', got %q", cfg.Name)
}
if cfg.Image != "ubuntu:22.04" {

Check failure on line 708 in pkg/devcontainer/config/extends_test.go

View workflow job for this annotation

GitHub Actions / Lint

string `ubuntu:22.04` has 3 occurrences, make it a constant (goconst)

Check failure on line 708 in pkg/devcontainer/config/extends_test.go

View workflow job for this annotation

GitHub Actions / Lint Gate

string `ubuntu:22.04` has 3 occurrences, make it a constant (goconst)
t.Errorf("expected image inherited from parent, got %q", cfg.Image)
}
if cfg.RemoteUser != "dev" {
t.Errorf("expected remoteUser 'dev', got %q", cfg.RemoteUser)
}
}

func TestExtends_VarSub_LocalWorkspaceFolder(t *testing.T) {
tmpDir := t.TempDir()
subDir := filepath.Join(tmpDir, ".devcontainer")
// #nosec G301 -- test directory
if err := os.MkdirAll(subDir, 0o750); err != nil {
t.Fatal(err)
}

writeJSON(t, tmpDir, "base-config.json", `{
"image": "node:20",
"remoteUser": "node"
}`)

// ${localWorkspaceFolder} should resolve to the parent of .devcontainer
childPath := writeJSON(t, subDir, "devcontainer.json", `{
"extends": "${localWorkspaceFolder}/base-config.json",
"name": "workspace-ref"
}`)

cfg, err := ParseDevContainerJSONFile(childPath)
if err != nil {
t.Fatal(err)
}
if cfg.Name != "workspace-ref" {
t.Errorf("expected name 'workspace-ref', got %q", cfg.Name)
}
if cfg.Image != "node:20" {
t.Errorf("expected image 'node:20', got %q", cfg.Image)
}
}

func TestExtends_VarSub_MissingEnvResolvesToEmpty(t *testing.T) {
tmpDir := t.TempDir()
subDir := filepath.Join(tmpDir, ".devcontainer")
// #nosec G301 -- test directory
if err := os.MkdirAll(subDir, 0o750); err != nil {
t.Fatal(err)
}

// Ensure the env var is unset
t.Setenv("DEVSY_TEST_NONEXISTENT_VAR", "")
_ = os.Unsetenv("DEVSY_TEST_NONEXISTENT_VAR")

childPath := writeJSON(t, subDir, "devcontainer.json", `{
"extends": "${localEnv:DEVSY_TEST_NONEXISTENT_VAR}/parent.json",
"name": "missing-env"
}`)

// Missing env var resolves to empty string, resulting in an invalid path
_, err := ParseDevContainerJSONFile(childPath)
if err == nil {
t.Fatal("expected error when env var resolves to empty and path is invalid")
}
}

func strPtr(s string) *string {
return &s
}
35 changes: 34 additions & 1 deletion pkg/devcontainer/config/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"os"
path2 "path"
"path/filepath"
"runtime"
"strings"
"unicode/utf8"

Expand Down Expand Up @@ -97,8 +98,11 @@ func ParseDevContainerJSONFile(jsonFilePath string) (*DevContainerConfig, error)
}
devContainer.Origin = path

// Resolve extends before applying legacy transforms
// Resolve extends before applying legacy transforms.
// Variable substitution must be applied to extends paths first so that
// ${localEnv:X}, ${localWorkspaceFolder}, etc. resolve before path lookup.
if !devContainer.Extends.IsEmpty() {
devContainer.Extends = substituteExtendsRefs(devContainer.Extends, path)
visited := map[string]bool{path: true}
declaringDir := filepath.Dir(path)
parent, err := resolveExtendsArray(
Expand Down Expand Up @@ -284,6 +288,35 @@ func Convert(from any, to any) error {
return json.Unmarshal(out, to)
}

// substituteExtendsRefs applies variable substitution to extends path strings
// so that ${localEnv:X}, ${localWorkspaceFolder}, etc. are resolved before the
// extends resolver attempts to open the referenced files.
func substituteExtendsRefs(refs ExtendsRef, configFilePath string) ExtendsRef {
localWorkspaceFolder := filepath.Dir(filepath.Dir(configFilePath))
Comment on lines +294 to +295

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

localWorkspaceFolder is computed incorrectly for non-.devcontainer/ config paths.

filepath.Dir(filepath.Dir(configFilePath)) only yields the correct workspace root when configFilePath lives inside a .devcontainer/ subdirectory (the canonical .devcontainer/devcontainer.json layout). For the other two standard layouts it is wrong:

Config path Dir(Dir(path)) Correct workspace
/ws/.devcontainer/devcontainer.json /ws /ws
/ws/.devcontainer.json (root-level) parent of /ws /ws
/ws/.devcontainer/myproj/devcontainer.json /ws/.devcontainer /ws

And when substituteExtendsRefs is called recursively from extends.go with a parent file that lives outside a .devcontainer/ directory (e.g., a shared base at /shared/base.json), the computed workspace is again wrong.

The proper fix is to thread the workspace folder through the call chain as a parameter. A lower-effort intermediate heuristic that handles the two most common layouts:

🛠️ Suggested heuristic fix
 func substituteExtendsRefs(refs ExtendsRef, configFilePath string) ExtendsRef {
-	localWorkspaceFolder := filepath.Dir(filepath.Dir(configFilePath))
+	// localWorkspaceFolder: parent of the .devcontainer/ dir when the config
+	// is inside one, otherwise the directory that contains the config file.
+	configDir := filepath.Dir(configFilePath)
+	localWorkspaceFolder := configDir
+	if filepath.Base(configDir) == ".devcontainer" {
+		localWorkspaceFolder = filepath.Dir(configDir)
+	}

The long-term fix is to accept workspaceFolder string as a parameter on ParseDevContainerJSONFile, parseDevContainerJSONFileWithVisited, and substituteExtendsRefs, matching the devcontainer spec where this value is provided by the host.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func substituteExtendsRefs(refs ExtendsRef, configFilePath string) ExtendsRef {
localWorkspaceFolder := filepath.Dir(filepath.Dir(configFilePath))
func substituteExtendsRefs(refs ExtendsRef, configFilePath string) ExtendsRef {
// localWorkspaceFolder: parent of the .devcontainer/ dir when the config
// is inside one, otherwise the directory that contains the config file.
configDir := filepath.Dir(configFilePath)
localWorkspaceFolder := configDir
if filepath.Base(configDir) == ".devcontainer" {
localWorkspaceFolder = filepath.Dir(configDir)
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/devcontainer/config/parse.go` around lines 294 - 295, The computed
localWorkspaceFolder in substituteExtendsRefs is wrong for non-.devcontainer
paths; pass the workspace folder through the call chain instead of deriving it
with filepath.Dir(filepath.Dir(configFilePath)). Add a workspaceFolder string
parameter to ParseDevContainerJSONFile, parseDevContainerJSONFileWithVisited,
and substituteExtendsRefs (and update callers in extends.go) so the correct
workspace root is used; alternatively, as a minimal interim fix implement the
suggested heuristic inside substituteExtendsRefs to detect root-level and nested
.devcontainer layouts rather than always using Dir(Dir(configFilePath)). Ensure
all function signatures and recursive calls are updated to accept and forward
workspaceFolder.

env := ListToObject(os.Environ())
isWindows := runtime.GOOS == "windows"
if isWindows {
newEnv := map[string]string{}
for k, v := range env {
newEnv[strings.ToLower(k)] = v
}
env = newEnv
}

subCtx := &SubstitutionContext{
LocalWorkspaceFolder: localWorkspaceFolder,
Env: env,
}

result := make(ExtendsRef, len(refs))
for i, ref := range refs {
result[i] = ResolveString(ref, func(match, variable string, args []string) string {
return replaceWithContext(isWindows, subCtx, match, variable, args)
})
}
return result
}

func ParseKeyValueFile(filename string) ([]string, error) {
f, err := os.Open(filename)
if err != nil {
Expand Down
Loading