Plugins: Backend: Skip host environment variables (#77858)

* Add pluginsSkipHostEnvVars feature flag

* Set go-plugin's SkipHostEnvVar depending on feature flags

* add missing file

* Re-generate feature flags

* Add allowedHostEnvVarNames

* Fix feature toggles not being passed to plugin context service's plugin env vars

* Fix tests

* PR review feedback: Use cfg.Features

* Fix tests

* PR review feedback: removed DefaultProviderWithFeatures

* merge with master

* fix tests

* use features.IsEnabledGlobally

pkg/plugins/envvars/envvars.go

@@ -17,12 +17,26 @@ import (
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/plugins/auth"
 	"github.com/grafana/grafana/pkg/plugins/config"
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
 )
 
 const (
 	customConfigPrefix = "GF_PLUGIN"
 )
 
+// allowedHostEnvVarNames is the list of environment variables that can be passed from Grafana's process to the
+// plugin's process
+var allowedHostEnvVarNames = []string{
+	// Env vars used by net/http (Go stdlib) for http/https proxy
+	// https://github.com/golang/net/blob/fbaf41277f28102c36926d1368dafbe2b54b4c1d/http/httpproxy/proxy.go#L91-L93
+	"HTTP_PROXY",
+	"http_proxy",
+	"HTTPS_PROXY",
+	"https_proxy",
+	"NO_PROXY",
+	"no_proxy",
+}
+
 type Provider interface {
 	Get(ctx context.Context, p *plugins.Plugin) []string
 }
@@ -72,7 +86,15 @@ func (s *Service) Get(ctx context.Context, p *plugins.Plugin) []string {
 	hostEnv = append(hostEnv, azsettings.WriteToEnvStr(s.cfg.Azure)...)
 	hostEnv = append(hostEnv, s.tracingEnvVars(p)...)
 
+	// If FlagPluginsSkipHostEnvVars is enabled, get some allowed variables from the current process and pass
+	// them down to the plugin. If the feature flag is not enabled, do not add anything else because ALL env vars
+	// from the current process (os.Environ()) will be forwarded to the plugin's process by go-plugin
+	if s.cfg.Features != nil && s.cfg.Features.IsEnabledGlobally(featuremgmt.FlagPluginsSkipHostEnvVars) {
+		hostEnv = append(hostEnv, s.allowedHostEnvVars()...)
+	}
+
 	ev := getPluginSettings(p.ID, s.cfg).asEnvVar(customConfigPrefix, hostEnv...)
+
 	return ev
 }
 
@@ -238,6 +260,19 @@ func (s *Service) secureSocksProxyEnvVars() []string {
 	return nil
 }
 
+// allowedHostEnvVars returns the variables that can be passed from Grafana's process
+// (current process, also known as: "host") to the plugin process.
+// A string in format "k=v" is returned for each variable in allowedHostEnvVarNames, if it's set.
+func (s *Service) allowedHostEnvVars() []string {
+	var r []string
+	for _, envVarName := range allowedHostEnvVarNames {
+		if envVarValue, ok := os.LookupEnv(envVarName); ok {
+			r = append(r, envVarName+"="+envVarValue)
+		}
+	}
+	return r
+}
+
 type pluginSettings map[string]string
 
 func getPluginSettings(pluginID string, cfg *config.Cfg) pluginSettings {

pkg/plugins/envvars/envvars_test.go

@@ -38,6 +38,7 @@ func TestInitializer_envVars(t *testing.T) {
 					"custom_env_var": "customVal",
 				},
 			},
+			Features: featuremgmt.WithFeatures(),
 		}, licensing)
 
 		envVars := envVarsProvider.Get(context.Background(), p)
@@ -51,6 +52,76 @@ func TestInitializer_envVars(t *testing.T) {
 	})
 }
 
+func TestInitializer_skipHostEnvVars(t *testing.T) {
+	const (
+		envVarName  = "HTTP_PROXY"
+		envVarValue = "lorem ipsum"
+	)
+
+	t.Setenv(envVarName, envVarValue)
+
+	p := &plugins.Plugin{
+		JSONData: plugins.JSONData{
+			ID: "test",
+		},
+	}
+
+	t.Run("without FlagPluginsSkipHostEnvVars should not populate host env vars", func(t *testing.T) {
+		envVarsProvider := NewProvider(&config.Cfg{Features: featuremgmt.WithFeatures()}, nil)
+		envVars := envVarsProvider.Get(context.Background(), p)
+
+		// We want to test that the envvars.Provider does not add any of the host env vars.
+		// When starting the plugin via go-plugin, ALL host env vars will be added by go-plugin,
+		// but we are testing the envvars.Provider here, so that's outside the scope of this test.
+		_, ok := getEnvVarWithExists(envVars, envVarName)
+		require.False(t, ok, "host env var should not be present")
+	})
+
+	t.Run("with FlagPluginsSkipHostEnvVars", func(t *testing.T) {
+		envVarsProvider := NewProvider(&config.Cfg{
+			Features: featuremgmt.WithFeatures(featuremgmt.FlagPluginsSkipHostEnvVars),
+		}, nil)
+
+		t.Run("should populate allowed host env vars", func(t *testing.T) {
+			// Set all allowed variables
+			for _, ev := range allowedHostEnvVarNames {
+				t.Setenv(ev, envVarValue)
+			}
+			envVars := envVarsProvider.Get(context.Background(), p)
+
+			// Test against each variable
+			for _, expEvName := range allowedHostEnvVarNames {
+				gotEvValue, ok := getEnvVarWithExists(envVars, expEvName)
+				require.True(t, ok, "host env var should be present")
+				require.Equal(t, envVarValue, gotEvValue)
+			}
+		})
+
+		t.Run("should not populate host env vars that aren't allowed", func(t *testing.T) {
+			// Set all allowed variables
+			for _, ev := range allowedHostEnvVarNames {
+				t.Setenv(ev, envVarValue)
+			}
+			// ...and an extra one, which should not leak
+			const superSecretEnvVariableName = "SUPER_SECRET_VALUE"
+			t.Setenv(superSecretEnvVariableName, "01189998819991197253")
+			envVars := envVarsProvider.Get(context.Background(), p)
+
+			// Super secret should not leak
+			_, ok := getEnvVarWithExists(envVars, superSecretEnvVariableName)
+			require.False(t, ok, "super secret env var should not be leaked")
+
+			// Everything else should be present
+			for _, expEvName := range allowedHostEnvVarNames {
+				var gotEvValue string
+				gotEvValue, ok = getEnvVarWithExists(envVars, expEvName)
+				require.True(t, ok, "host env var should be present")
+				require.Equal(t, envVarValue, gotEvValue)
+			}
+		})
+	})
+}
+
 func TestInitializer_tracingEnvironmentVariables(t *testing.T) {
 	const pluginID = "plugin_id"