refactor(provider/env): document env var contract and validate reserved names

pkg/provider/env.go

@@ -22,6 +22,23 @@ func combineOptions(
 	return options
 }
 
+// ToEnvironment builds the full environment for provider subprocess execution.
+// It combines the current process environment (os.Environ) with provider-specific
+// variables from three sources:
+//
+//   - ToOptions: merged provider config options (uppercased key=value pairs),
+//     plus workspace identity vars (WORKSPACE_ID, WORKSPACE_PROVIDER, etc.),
+//     machine identity vars (MACHINE_ID, MACHINE_PROVIDER, etc.),
+//     and base vars (PROVIDER_ID, PROVIDER_CONTEXT, PROVIDER_FOLDER, DEVPOD, DEVPOD_OS, DEVPOD_ARCH).
+//   - extraEnv: caller-supplied additional variables.
+//
+// The provider name is available to subprocesses as PROVIDER_ID (set by GetBaseEnvironment),
+// WORKSPACE_PROVIDER (set by ToOptionsWorkspace), and MACHINE_PROVIDER (set by ToOptionsMachine).
+//
+// Note: DEVPOD_PROVIDER is reserved by the --provider CLI flag via inheritFlagsFromEnvironment
+// in cmd/root.go. It is inherited from the parent process via os.Environ() but must not be
+// explicitly set here, as doing so would override the global provider selection for child
+// devpod processes.
 func ToEnvironment(
 	workspace *Workspace,
 	machine *Machine,
@@ -143,6 +160,11 @@ func Merge(m1 map[string]string, m2 map[string]string) map[string]string {
 	return retMap
 }
 
+// GetBaseEnvironment returns system-level env vars set for every provider subprocess:
+// DEVPOD (binary path), DEVPOD_OS, DEVPOD_ARCH, PROVIDER_ID (provider name),
+// PROVIDER_CONTEXT, PROVIDER_FOLDER, and DEVPOD_LOG_LEVEL.
+//
+// PROVIDER_ID is the canonical way for provider scripts to discover their own name.
 func GetBaseEnvironment(context, provider string) map[string]string {
 	retVars := map[string]string{}
 

pkg/provider/env_test.go

@@ -0,0 +1,67 @@
+package provider
+
+import (
+	"slices"
+	"strings"
+	"testing"
+)
+
+func TestToEnvironment_ContainsProviderID(t *testing.T) {
+	ws := &Workspace{
+		ID:      "test-workspace",
+		Context: "default",
+		Provider: WorkspaceProviderConfig{
+			Name: "test-provider",
+		},
+		Source: WorkspaceSource{},
+	}
+
+	environ := ToEnvironment(ws, nil, nil, nil)
+
+	assertEnvContains(t, environ, "PROVIDER_ID", "test-provider")
+	assertEnvContains(t, environ, "WORKSPACE_PROVIDER", "test-provider")
+	assertEnvContains(t, environ, "WORKSPACE_ID", "test-workspace")
+}
+
+func TestToEnvironment_DoesNotDuplicateDevpodProvider(t *testing.T) {
+	ws := &Workspace{
+		ID:      "test-workspace",
+		Context: "default",
+		Provider: WorkspaceProviderConfig{
+			Name: "test-provider",
+		},
+		Source: WorkspaceSource{},
+	}
+
+	environ := ToEnvironment(ws, nil, nil, nil)
+
+	// DEVPOD_PROVIDER is reserved by the --provider CLI flag.
+	// It may appear from os.Environ() but must not be explicitly added.
+	count := 0
+	for _, entry := range environ {
+		if strings.HasPrefix(entry, "DEVPOD_PROVIDER=") {
+			count++
+		}
+	}
+	if count > 1 {
+		t.Errorf(
+			"found %d DEVPOD_PROVIDER entries; expected at most 1 (from os.Environ)",
+			count,
+		)
+	}
+}
+
+func TestToEnvironment_IncludesExtraEnv(t *testing.T) {
+	extra := map[string]string{"CUSTOM_VAR": "custom_value"}
+	environ := ToEnvironment(nil, nil, nil, extra)
+
+	assertEnvContains(t, environ, "CUSTOM_VAR", "custom_value")
+}
+
+func assertEnvContains(t *testing.T, environ []string, key, value string) {
+	t.Helper()
+	expected := key + "=" + value
+	if !slices.Contains(environ, expected) {
+		t.Errorf("expected %s in environment, not found", expected)
+	}
+}

pkg/provider/parse.go

@@ -18,6 +18,29 @@ var ProviderNameRegEx = regexp.MustCompile(`[^a-z0-9\-]+`)
 
 var optionNameRegEx = regexp.MustCompile(`[^A-Z0-9_]+`)
 
+// reservedOptionNames are env var names set by the DevPod runtime
+// (via GetBaseEnvironment, ToOptionsWorkspace, ToOptionsMachine).
+// Provider options must not use these names as they would silently
+// overwrite system-set values in the subprocess environment.
+var reservedOptionNames = map[string]bool{
+	"PROVIDER_ID":        true,
+	"PROVIDER_CONTEXT":   true,
+	"PROVIDER_FOLDER":    true,
+	"WORKSPACE_ID":       true,
+	"WORKSPACE_UID":      true,
+	"WORKSPACE_PROVIDER": true,
+	"WORKSPACE_CONTEXT":  true,
+	"WORKSPACE_FOLDER":   true,
+	"WORKSPACE_SOURCE":   true,
+	"WORKSPACE_ORIGIN":   true,
+	"WORKSPACE_PICTURE":  true,
+	"MACHINE_ID":         true,
+	"MACHINE_CONTEXT":    true,
+	"MACHINE_FOLDER":     true,
+	"MACHINE_PROVIDER":   true,
+	"LOFT_PROJECT":       true,
+}
+
 var allowedTypes = []string{
 	"string",
 	"multiline",
@@ -75,6 +98,14 @@ func validate(config *ProviderConfig) error {
 			)
 		}
 
+		if reservedOptionNames[optionName] {
+			return fmt.Errorf(
+				"provider option '%s' uses a reserved environment variable name; "+
+					"choose a different name to avoid overwriting system-set values",
+				optionName,
+			)
+		}
+
 		// validate option validation
 		if optionValue.ValidationPattern != "" {
 			_, err := regexp.Compile(optionValue.ValidationPattern)

pkg/provider/parse_test.go

@@ -0,0 +1,58 @@
+package provider
+
+import (
+	"testing"
+
+	"github.com/skevetter/devpod/pkg/types"
+)
+
+func TestValidate_RejectsReservedOptionNames(t *testing.T) {
+	reserved := []string{
+		"PROVIDER_ID",
+		"PROVIDER_CONTEXT",
+		"PROVIDER_FOLDER",
+		"WORKSPACE_ID",
+		"WORKSPACE_UID",
+		"WORKSPACE_PROVIDER",
+		"WORKSPACE_CONTEXT",
+		"WORKSPACE_FOLDER",
+		"WORKSPACE_SOURCE",
+		"WORKSPACE_ORIGIN",
+		"WORKSPACE_PICTURE",
+		"MACHINE_ID",
+		"MACHINE_CONTEXT",
+		"MACHINE_FOLDER",
+		"MACHINE_PROVIDER",
+		"LOFT_PROJECT",
+	}
+
+	for _, name := range reserved {
+		cfg := &ProviderConfig{
+			Name: "test-provider",
+			Options: map[string]*types.Option{
+				name: {Description: "test"},
+			},
+		}
+		err := validate(cfg)
+		if err == nil {
+			t.Errorf("expected error for reserved option name %q, got nil", name)
+		}
+	}
+}
+
+func TestValidate_AllowsNonReservedOptionNames(t *testing.T) {
+	cfg := &ProviderConfig{
+		Name: "test-provider",
+		Options: map[string]*types.Option{
+			"MY_CUSTOM_OPTION": {Description: "test"},
+			"AWS_REGION":       {Description: "test"},
+		},
+		Exec: ProviderCommands{
+			Command: []string{"echo hello"},
+		},
+	}
+	err := validate(cfg)
+	if err != nil {
+		t.Errorf("expected no error for non-reserved names, got: %v", err)
+	}
+}