[bug] plugin env PATH fix (#2418)

## Summary When a plugin modifies PATH and also config in devbox.json modifies PATH via `env:{}`, the env overwrites the plugin and so PATH modification from the plugin is lost. This is because we do `maps.Copy()` to merge the env from plugin to the one from config. The copy is fine for all other env variables but for PATH we need to handle merging the two rather than overwriting. Fixes #2138 ## How was it tested? bug recreate: - devbox init - devbox add ruby (ruby plugin modifies PATH) - devbox shell (see PATH is prepended with ruby plugin) - modify devbox.json and add `"env": {"PATH": "/my/config/bin:$PATH"}` - re-enter devbox shell and see PATH prepend from ruby plugin is lost, only `"/my/config/bin"` is prepended. fix recreate: - follow same steps, but in last step both PATH prepends from devbox.json and from plugin are present.

Author: mohsenari

internal/devconfig/config.go

@@ -346,9 +346,11 @@ func (c *Config) NixPkgsCommitHash() string {
 func (c *Config) Env() map[string]string {
 env := map[string]string{}
 for _, i := range c.included {
-maps.Copy(env, i.Env())
+expandedEnvFromPlugin := OSExpandIfPossible(i.Env(), env)
+maps.Copy(env, expandedEnvFromPlugin)
 }
-maps.Copy(env, c.Root.Env)
+rootConfigEnv := OSExpandIfPossible(c.Root.Env, env)
+maps.Copy(env, rootConfigEnv)
 return env
 }
 
@@ -406,3 +408,19 @@ func createIncludableFromPluginConfig(pluginConfig *plugin.Config) *Config {
 }
 return includable
 }
+
+func OSExpandIfPossible(env, existingEnv map[string]string) map[string]string {
+mapping := func(value string) string {
+// If the value is not set in existingEnv, return the value wrapped in ${...}
+if existingEnv == nil || existingEnv[value] == "" {
+return fmt.Sprintf("${%s}", value)
+}
+return existingEnv[value]
+}
+
+res := map[string]string{}
+for k, v := range env {
+res[k] = os.Expand(v, mapping)
+}
+return res
+}

internal/devconfig/config_test.go

@@ -327,3 +327,103 @@ func TestDefault(t *testing.T) {
 t.Errorf("got different JSON after load/save/load:\ninput:\n%s\noutput:\n%s", inBytes, outBytes)
 }
 }
+
+func TestOSExpandIfPossible(t *testing.T) {
+tests := []struct {
+name string
+env map[string]string
+existingEnv map[string]string
+want map[string]string
+}{
+{
+name: "basic expansion",
+env: map[string]string{
+"FOO": "$BAR",
+"BAZ": "${QUX}",
+},
+existingEnv: map[string]string{
+"BAR": "bar_value",
+"QUX": "qux_value",
+},
+want: map[string]string{
+"FOO": "bar_value",
+"BAZ": "qux_value",
+},
+},
+{
+name: "missing values remain as template",
+env: map[string]string{
+"FOO": "$BAR",
+"BAZ": "${QUX}",
+},
+existingEnv: map[string]string{
+"BAR": "bar_value",
+// QUX is missing
+},
+want: map[string]string{
+"FOO": "bar_value",
+"BAZ": "${QUX}",
+},
+},
+{
+name: "nil existing env",
+env: map[string]string{
+"FOO": "$BAR",
+"BAZ": "${QUX}",
+},
+existingEnv: nil,
+want: map[string]string{
+"FOO": "${BAR}",
+"BAZ": "${QUX}",
+},
+},
+{
+name: "empty existing env",
+env: map[string]string{
+"FOO": "$BAR",
+},
+existingEnv: map[string]string{},
+want: map[string]string{
+"FOO": "${BAR}",
+},
+},
+{
+name: "mixed literal and variable",
+env: map[string]string{
+"FOO": "prefix_${BAR}_suffix",
+},
+existingEnv: map[string]string{
+"BAR": "bar_value",
+},
+want: map[string]string{
+"FOO": "prefix_bar_value_suffix",
+},
+},
+{
+name: "path special case",
+env: map[string]string{
+"FOO": "/my/config:$FOO",
+},
+existingEnv: map[string]string{
+"FOO": "/my/plugin:$FOO",
+},
+want: map[string]string{
+"FOO": "/my/config:/my/plugin:$FOO",
+},
+},
+}
+
+for _, tt := range tests {
+t.Run(tt.name, func(t *testing.T) {
+got := OSExpandIfPossible(tt.env, tt.existingEnv)
+if len(got) != len(tt.want) {
+t.Errorf("OSExpandIfPossible() got %v entries, want %v entries", len(got), len(tt.want))
+}
+for k, v := range tt.want {
+if got[k] != v {
+t.Errorf("OSExpandIfPossible() for key %q = %q, want %q", k, got[k], v)
+}
+}
+})
+}
+}