From 63f3e96f69b9884d1078175008b63e366382774d Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sun, 5 Apr 2026 02:18:10 -0500 Subject: [PATCH 1/3] feat(provider): validate option names against reserved env var names Provider options are uppercased and injected as subprocess env vars. If an option collides with a system-set name like PROVIDER_ID or WORKSPACE_ID, it silently overwrites the system value. This adds validation to reject reserved names at provider config parse time. --- pkg/provider/parse.go | 31 ++++++++++++++++++++ pkg/provider/parse_test.go | 58 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 pkg/provider/parse_test.go diff --git a/pkg/provider/parse.go b/pkg/provider/parse.go index cff7d5edb..13aab4f2f 100644 --- a/pkg/provider/parse.go +++ b/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) diff --git a/pkg/provider/parse_test.go b/pkg/provider/parse_test.go new file mode 100644 index 000000000..b36f5748d --- /dev/null +++ b/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) + } +} From 7fbddf520c807089a8dfcfd556b71831e96b88c4 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sun, 5 Apr 2026 02:19:41 -0500 Subject: [PATCH 2/3] docs(provider/env): document subprocess environment contract Adds comprehensive doc comments to ToEnvironment and GetBaseEnvironment explaining which env vars are set, where they come from, and why DEVPOD_PROVIDER must not be explicitly added to the subprocess env. --- pkg/provider/env.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/pkg/provider/env.go b/pkg/provider/env.go index 09db44a45..d468e4b5c 100644 --- a/pkg/provider/env.go +++ b/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{} From b5efd735cd04af196e502b7dbfc9488b9487b1cd Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sun, 5 Apr 2026 02:19:44 -0500 Subject: [PATCH 3/3] test(provider/env): add contract tests for subprocess environment Verifies that PROVIDER_ID and WORKSPACE_PROVIDER carry the provider name, that DEVPOD_PROVIDER is not duplicated, and that extraEnv vars are included. --- pkg/provider/env_test.go | 67 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 pkg/provider/env_test.go diff --git a/pkg/provider/env_test.go b/pkg/provider/env_test.go new file mode 100644 index 000000000..d0a9b6649 --- /dev/null +++ b/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) + } +}