From bc658d339f58ed496c63bd0c628e6afdcbae25d4 Mon Sep 17 00:00:00 2001
From: "devsy-app[bot]" <277138668+devsy-app[bot]@users.noreply.github.com>
Date: Sun, 9 Aug 2026 12:15:56 +0000
Subject: [PATCH] feat(devcontainer): require id in feature parse

The devcontainer Feature schema marks `id` as a required property, but ParseDevContainerFeature silently accepted manifests with an empty/missing id. The info and generatedocs commands would therefore process malformed features without error.

Add id validation to ParseDevContainerFeature so a missing or empty id returns an error referencing devcontainer-feature.json. Remove the now-redundant id-presence check in cmd/feature validatePublishTarget (the parser enforces it) and update its test to assert the parser-level message. Add unit tests covering the missing-id, empty-id, and valid-id cases.
---
 cmd/feature/publish.go                  |  4 ---
 cmd/feature/publish_test.go             |  2 +-
 pkg/devcontainer/config/feature_test.go | 42 +++++++++++++++++++++++++
 pkg/devcontainer/config/parse.go        |  7 +++++
 4 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/cmd/feature/publish.go b/cmd/feature/publish.go
index c82873b287..1dc9f33c64 100644
--- a/cmd/feature/publish.go
+++ b/cmd/feature/publish.go
@@ -143,10 +143,6 @@ func validatePublishTarget(target string) (*config.FeatureConfig, error) {
 		return nil, fmt.Errorf("parse feature metadata: %w", err)
 	}
 
-	if featureCfg.ID == "" {
-		return nil, fmt.Errorf("feature metadata missing required 'id' field")
-	}
-
 	return featureCfg, nil
 }
 
diff --git a/cmd/feature/publish_test.go b/cmd/feature/publish_test.go
index aa258917b8..7f5050aa8f 100644
--- a/cmd/feature/publish_test.go
+++ b/cmd/feature/publish_test.go
@@ -126,7 +126,7 @@ func TestValidatePublishTarget_MissingID(t *testing.T) {
 
 	_, err := validatePublishTarget(tmpDir)
 	require.Error(t, err)
-	assert.Contains(t, err.Error(), "missing required 'id' field")
+	assert.Contains(t, err.Error(), "missing required property \"id\"")
 }
 
 func TestValidatePublishTarget_Valid(t *testing.T) {
diff --git a/pkg/devcontainer/config/feature_test.go b/pkg/devcontainer/config/feature_test.go
index 48565250ef..3733a6adbc 100644
--- a/pkg/devcontainer/config/feature_test.go
+++ b/pkg/devcontainer/config/feature_test.go
@@ -2,6 +2,8 @@ package config
 
 import (
 	"encoding/json"
+	"os"
+	"path/filepath"
 	"reflect"
 	"testing"
 )
@@ -69,3 +71,43 @@ func TestFeatureConfig_DependsOnKeysEmpty(t *testing.T) {
 		t.Errorf("DependsOnKeys() = %v, want empty", got)
 	}
 }
+
+func TestParseDevContainerFeatureRequiresID(t *testing.T) {
+	cases := []struct {
+		name string
+		raw  string
+	}{
+		{"missing id", `{"version":"1.0.0","name":"Go"}`},
+		{"empty id", `{"id":"","version":"1.0.0","name":"Go"}`},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			dir := t.TempDir()
+			if err := os.WriteFile(
+				filepath.Join(dir, DevContainerFeatureFileName), []byte(tc.raw), 0o600,
+			); err != nil {
+				t.Fatalf("write feature file: %v", err)
+			}
+			if _, err := ParseDevContainerFeature(dir); err == nil {
+				t.Fatalf("ParseDevContainerFeature expected error for %s, got nil", tc.name)
+			}
+		})
+	}
+}
+
+func TestParseDevContainerFeatureAcceptsID(t *testing.T) {
+	dir := t.TempDir()
+	raw := `{"id":"go","version":"1.0.0","name":"Go"}`
+	if err := os.WriteFile(
+		filepath.Join(dir, DevContainerFeatureFileName), []byte(raw), 0o600,
+	); err != nil {
+		t.Fatalf("write feature file: %v", err)
+	}
+	cfg, err := ParseDevContainerFeature(dir)
+	if err != nil {
+		t.Fatalf("ParseDevContainerFeature: %v", err)
+	}
+	if cfg.ID != "go" {
+		t.Errorf("ID = %q, want %q", cfg.ID, "go")
+	}
+}
diff --git a/pkg/devcontainer/config/parse.go b/pkg/devcontainer/config/parse.go
index 612340c466..33b7e4a505 100644
--- a/pkg/devcontainer/config/parse.go
+++ b/pkg/devcontainer/config/parse.go
@@ -51,6 +51,13 @@ func ParseDevContainerFeature(folder string) (*FeatureConfig, error) {
 		return nil, err
 	}
 
+	if featureConfig.ID == "" {
+		// "id" is required by the devcontainer Feature schema.
+		return nil, fmt.Errorf(
+			"%s is missing required property \"id\"", DevContainerFeatureFileName,
+		)
+	}
+
 	featureConfig.Origin = path
 	return featureConfig, nil
 }