diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index 27ffaed281c..e701cb0c2f6 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -162,6 +162,7 @@ Experimental features might be changed or removed without prior notice. | `dashboardScene` | Enables dashboard rendering using scenes for all roles | | `logsInfiniteScrolling` | Enables infinite scrolling for the Logs panel in Explore and Dashboards | | `flameGraphItemCollapsing` | Allow collapsing of flame graph items | +| `pluginsSkipHostEnvVars` | Disables passing host environment variable to plugin processes | ## Development feature toggles diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 604876fd3de..7fbe94d26c1 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -162,4 +162,5 @@ export interface FeatureToggles { alertingDetailsViewV2?: boolean; datatrails?: boolean; alertingSimplifiedRouting?: boolean; + pluginsSkipHostEnvVars?: boolean; } diff --git a/pkg/expr/dataplane_test.go b/pkg/expr/dataplane_test.go index 762b2069396..3973dd8c86e 100644 --- a/pkg/expr/dataplane_test.go +++ b/pkg/expr/dataplane_test.go @@ -9,6 +9,8 @@ import ( "github.com/grafana/dataplane/examples" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/data" + "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/plugins" @@ -22,7 +24,6 @@ import ( "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" - "github.com/stretchr/testify/require" ) func TestPassThroughDataplaneExamples(t *testing.T) { diff --git a/pkg/plugins/backendplugin/grpcplugin/client.go b/pkg/plugins/backendplugin/grpcplugin/client.go index 6dc6aab3848..eacd1611e2b 100644 --- a/pkg/plugins/backendplugin/grpcplugin/client.go +++ b/pkg/plugins/backendplugin/grpcplugin/client.go @@ -39,7 +39,7 @@ var pluginSet = map[int]goplugin.PluginSet{ }, } -func newClientConfig(executablePath string, args []string, env []string, logger log.Logger, +func newClientConfig(executablePath string, args []string, env []string, skipHostEnvVars bool, logger log.Logger, versionedPlugins map[int]goplugin.PluginSet) *goplugin.ClientConfig { // We can ignore gosec G201 here, since the dynamic part of executablePath comes from the plugin definition // nolint:gosec @@ -50,6 +50,7 @@ func newClientConfig(executablePath string, args []string, env []string, logger Cmd: cmd, HandshakeConfig: handshake, VersionedPlugins: versionedPlugins, + SkipHostEnv: skipHostEnvVars, Logger: logWrapper{Logger: logger}, AllowedProtocols: []goplugin.Protocol{goplugin.ProtocolGRPC}, GRPCDialOptions: []grpc.DialOption{ @@ -75,6 +76,7 @@ type PluginDescriptor struct { pluginID string executablePath string executableArgs []string + skipHostEnvVars bool managed bool versionedPlugins map[int]goplugin.PluginSet startRendererFn StartRendererFunc @@ -82,21 +84,22 @@ type PluginDescriptor struct { } // NewBackendPlugin creates a new backend plugin factory used for registering a backend plugin. -func NewBackendPlugin(pluginID, executablePath string, executableArgs ...string) backendplugin.PluginFactoryFunc { - return newBackendPlugin(pluginID, executablePath, true, executableArgs...) +func NewBackendPlugin(pluginID, executablePath string, skipHostEnvVars bool, executableArgs ...string) backendplugin.PluginFactoryFunc { + return newBackendPlugin(pluginID, executablePath, true, skipHostEnvVars, executableArgs...) } // NewUnmanagedBackendPlugin creates a new backend plugin factory used for registering an unmanaged backend plugin. -func NewUnmanagedBackendPlugin(pluginID, executablePath string, executableArgs ...string) backendplugin.PluginFactoryFunc { - return newBackendPlugin(pluginID, executablePath, false, executableArgs...) +func NewUnmanagedBackendPlugin(pluginID, executablePath string, skipHostEnvVars bool, executableArgs ...string) backendplugin.PluginFactoryFunc { + return newBackendPlugin(pluginID, executablePath, false, skipHostEnvVars, executableArgs...) } // NewBackendPlugin creates a new backend plugin factory used for registering a backend plugin. -func newBackendPlugin(pluginID, executablePath string, managed bool, executableArgs ...string) backendplugin.PluginFactoryFunc { +func newBackendPlugin(pluginID, executablePath string, managed bool, skipHostEnvVars bool, executableArgs ...string) backendplugin.PluginFactoryFunc { return newPlugin(PluginDescriptor{ pluginID: pluginID, executablePath: executablePath, executableArgs: executableArgs, + skipHostEnvVars: skipHostEnvVars, managed: managed, versionedPlugins: pluginSet, }) diff --git a/pkg/plugins/backendplugin/grpcplugin/grpc_plugin.go b/pkg/plugins/backendplugin/grpcplugin/grpc_plugin.go index ffd723690f5..b25d601b65f 100644 --- a/pkg/plugins/backendplugin/grpcplugin/grpc_plugin.go +++ b/pkg/plugins/backendplugin/grpcplugin/grpc_plugin.go @@ -44,7 +44,7 @@ func newGrpcPlugin(descriptor PluginDescriptor, logger log.Logger, env func() [] descriptor: descriptor, logger: logger, clientFactory: func() *plugin.Client { - return plugin.NewClient(newClientConfig(descriptor.executablePath, descriptor.executableArgs, env(), logger, descriptor.versionedPlugins)) + return plugin.NewClient(newClientConfig(descriptor.executablePath, descriptor.executableArgs, env(), descriptor.skipHostEnvVars, logger, descriptor.versionedPlugins)) }, } } diff --git a/pkg/plugins/backendplugin/provider/provider.go b/pkg/plugins/backendplugin/provider/provider.go index 6afc4328861..82fbebdd15c 100644 --- a/pkg/plugins/backendplugin/provider/provider.go +++ b/pkg/plugins/backendplugin/provider/provider.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana/pkg/plugins/backendplugin/pluginextensionv2" "github.com/grafana/grafana/pkg/plugins/backendplugin/secretsmanagerplugin" "github.com/grafana/grafana/pkg/plugins/log" + "github.com/grafana/grafana/pkg/services/featuremgmt" ) // PluginBackendProvider is a function type for initializing a Plugin backend. @@ -17,19 +18,21 @@ type PluginBackendProvider func(_ context.Context, _ *plugins.Plugin) backendplu type Service struct { providerChain []PluginBackendProvider + features featuremgmt.FeatureToggles } -func New(providers ...PluginBackendProvider) *Service { +func New(features featuremgmt.FeatureToggles, providers ...PluginBackendProvider) *Service { if len(providers) == 0 { - return New(RendererProvider, SecretsManagerProvider, DefaultProvider) + return New(features, RendererProvider, SecretsManagerProvider, DefaultProvider(features)) } return &Service{ providerChain: providers, + features: features, } } -func ProvideService(coreRegistry *coreplugin.Registry) *Service { - return New(coreRegistry.BackendFactoryProvider(), RendererProvider, SecretsManagerProvider, DefaultProvider) +func ProvideService(features featuremgmt.FeatureToggles, coreRegistry *coreplugin.Registry) *Service { + return New(features, coreRegistry.BackendFactoryProvider(), RendererProvider, SecretsManagerProvider, DefaultProvider(features)) } func (s *Service) BackendFactory(ctx context.Context, p *plugins.Plugin) backendplugin.PluginFactoryFunc { @@ -65,6 +68,9 @@ var SecretsManagerProvider PluginBackendProvider = func(_ context.Context, p *pl ) } -var DefaultProvider PluginBackendProvider = func(_ context.Context, p *plugins.Plugin) backendplugin.PluginFactoryFunc { - return grpcplugin.NewBackendPlugin(p.ID, p.ExecutablePath()) +func DefaultProvider(features featuremgmt.FeatureToggles) PluginBackendProvider { + return func(_ context.Context, p *plugins.Plugin) backendplugin.PluginFactoryFunc { + skipEnvVars := features.IsEnabledGlobally(featuremgmt.FlagPluginsSkipHostEnvVars) + return grpcplugin.NewBackendPlugin(p.ID, p.ExecutablePath(), skipEnvVars) + } } diff --git a/pkg/plugins/envvars/envvars.go b/pkg/plugins/envvars/envvars.go index 184ac3f70a9..06a52918be3 100644 --- a/pkg/plugins/envvars/envvars.go +++ b/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 { diff --git a/pkg/plugins/envvars/envvars_test.go b/pkg/plugins/envvars/envvars_test.go index c158e1e2a0f..0cef94b2fad 100644 --- a/pkg/plugins/envvars/envvars_test.go +++ b/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" diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 3199e554ba9..ecddb76c76b 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1063,6 +1063,13 @@ var ( Owner: grafanaAlertingSquad, HideFromDocs: true, }, + { + Name: "pluginsSkipHostEnvVars", + Description: "Disables passing host environment variable to plugin processes", + Stage: FeatureStageExperimental, + FrontendOnly: false, + Owner: grafanaPluginsPlatformSquad, + }, } ) diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 20ad5e1c8e0..1fffede8694 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -143,3 +143,4 @@ flameGraphItemCollapsing,experimental,@grafana/observability-traces-and-profilin alertingDetailsViewV2,experimental,@grafana/alerting-squad,false,false,false,true datatrails,experimental,@grafana/dashboards-squad,false,false,false,true alertingSimplifiedRouting,experimental,@grafana/alerting-squad,false,false,false,false +pluginsSkipHostEnvVars,experimental,@grafana/plugins-platform-backend,false,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index db950be0fd4..943cc371f8d 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -582,4 +582,8 @@ const ( // FlagAlertingSimplifiedRouting // Enables the simplified routing for alerting FlagAlertingSimplifiedRouting = "alertingSimplifiedRouting" + + // FlagPluginsSkipHostEnvVars + // Disables passing host environment variable to plugin processes + FlagPluginsSkipHostEnvVars = "pluginsSkipHostEnvVars" ) diff --git a/pkg/services/pluginsintegration/test_helper.go b/pkg/services/pluginsintegration/test_helper.go index b958b05681c..fdc1fb14492 100644 --- a/pkg/services/pluginsintegration/test_helper.go +++ b/pkg/services/pluginsintegration/test_helper.go @@ -54,7 +54,7 @@ func CreateIntegrationTestCtx(t *testing.T, cfg *setting.Cfg, coreRegistry *core disc := pipeline.ProvideDiscoveryStage(pCfg, finder.NewLocalFinder(true), reg) boot := pipeline.ProvideBootstrapStage(pCfg, signature.ProvideService(pCfg, statickey.New()), assetpath.ProvideService(pCfg, cdn)) valid := pipeline.ProvideValidationStage(pCfg, signature.NewValidator(signature.NewUnsignedAuthorizer(pCfg)), angularInspector, errTracker) - init := pipeline.ProvideInitializationStage(pCfg, reg, fakes.NewFakeLicensingService(), provider.ProvideService(coreRegistry), proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry()) + init := pipeline.ProvideInitializationStage(pCfg, reg, fakes.NewFakeLicensingService(), provider.ProvideService(featuremgmt.WithFeatures(), coreRegistry), proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry()) term, err := pipeline.ProvideTerminationStage(pCfg, reg, proc) require.NoError(t, err) @@ -100,7 +100,7 @@ func CreateTestLoader(t *testing.T, cfg *pluginsCfg.Cfg, opts LoaderOpts) *loade if opts.Initializer == nil { reg := registry.ProvideService() coreRegistry := coreplugin.NewRegistry(make(map[string]backendplugin.PluginFactoryFunc)) - opts.Initializer = pipeline.ProvideInitializationStage(cfg, reg, fakes.NewFakeLicensingService(), provider.ProvideService(coreRegistry), process.ProvideService(), &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry()) + opts.Initializer = pipeline.ProvideInitializationStage(cfg, reg, fakes.NewFakeLicensingService(), provider.ProvideService(featuremgmt.WithFeatures(), coreRegistry), process.ProvideService(), &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry()) } if opts.Terminator == nil {