From f148b5fb28ec1d89854ab025f689b92826f5c2de Mon Sep 17 00:00:00 2001 From: Giuseppe Guerra Date: Wed, 7 Jun 2023 11:08:01 +0200 Subject: [PATCH] Plugins: Forbid loading Angular plugins when Angular is disabled (#69679) * Plugins: Forbid loading Angular plugins when Angular is disabled * Plugins: Made angulardetector a service, add tests for angular loader cases * Fix missing import * Add nolint:gocyclo to loadPlugins --- pkg/api/plugin_resource_test.go | 4 +- pkg/plugins/config/config.go | 5 +- .../loader/angulardetector/angulardetector.go | 46 ++------------ .../angulardetector/angulardetector_test.go | 31 +++++++++- .../manager/loader/angulardetector/fakes.go | 29 +++++++++ .../manager/loader/angulardetector/service.go | 62 +++++++++++++++++++ pkg/plugins/manager/loader/loader.go | 36 +++++++---- pkg/plugins/manager/loader/loader_test.go | 55 +++++++++++++++- .../manager/manager_integration_test.go | 6 +- .../pluginsintegration/config/config.go | 1 + .../pluginsintegration/pluginsintegration.go | 2 + 11 files changed, 217 insertions(+), 60 deletions(-) create mode 100644 pkg/plugins/manager/loader/angulardetector/fakes.go create mode 100644 pkg/plugins/manager/loader/angulardetector/service.go diff --git a/pkg/api/plugin_resource_test.go b/pkg/api/plugin_resource_test.go index 2bb03754f07..f6dc6107ada 100644 --- a/pkg/api/plugin_resource_test.go +++ b/pkg/api/plugin_resource_test.go @@ -22,6 +22,7 @@ import ( pluginClient "github.com/grafana/grafana/pkg/plugins/manager/client" "github.com/grafana/grafana/pkg/plugins/manager/fakes" "github.com/grafana/grafana/pkg/plugins/manager/loader" + "github.com/grafana/grafana/pkg/plugins/manager/loader/angulardetector" "github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath" "github.com/grafana/grafana/pkg/plugins/manager/loader/finder" "github.com/grafana/grafana/pkg/plugins/manager/registry" @@ -67,7 +68,8 @@ func TestCallResource(t *testing.T) { reg := registry.ProvideService() l := loader.ProvideService(pCfg, fakes.NewFakeLicensingService(), signature.NewUnsignedAuthorizer(pCfg), reg, provider.ProvideService(coreRegistry), finder.NewLocalFinder(pCfg), fakes.NewFakeRoleRegistry(), - assetpath.ProvideService(pluginscdn.ProvideService(pCfg)), signature.ProvideService(pCfg, statickey.New())) + assetpath.ProvideService(pluginscdn.ProvideService(pCfg)), signature.ProvideService(pCfg, statickey.New()), + angulardetector.NewDefaultPatternsListInspector()) srcs := sources.ProvideService(cfg, pCfg) ps, err := store.ProvideService(reg, srcs, l) require.NoError(t, err) diff --git a/pkg/plugins/config/config.go b/pkg/plugins/config/config.go index 505425e6f2f..4a6b11ecd02 100644 --- a/pkg/plugins/config/config.go +++ b/pkg/plugins/config/config.go @@ -39,11 +39,13 @@ type Cfg struct { GrafanaComURL string Features plugins.FeatureToggles + + AngularSupportEnabled bool } func NewCfg(devMode bool, pluginsPath string, pluginSettings setting.PluginSettings, pluginsAllowUnsigned []string, awsAllowedAuthProviders []string, awsAssumeRoleEnabled bool, azure *azsettings.AzureSettings, secureSocksDSProxy setting.SecureSocksDSProxySettings, - grafanaVersion string, logDatasourceRequests bool, pluginsCDNURLTemplate string, tracing Tracing, features plugins.FeatureToggles) *Cfg { + grafanaVersion string, logDatasourceRequests bool, pluginsCDNURLTemplate string, tracing Tracing, features plugins.FeatureToggles, angularSupportEnabled bool) *Cfg { return &Cfg{ log: log.New("plugin.cfg"), PluginsPath: pluginsPath, @@ -60,5 +62,6 @@ func NewCfg(devMode bool, pluginsPath string, pluginSettings setting.PluginSetti Tracing: tracing, GrafanaComURL: "https://grafana.com", Features: features, + AngularSupportEnabled: angularSupportEnabled, } } diff --git a/pkg/plugins/manager/loader/angulardetector/angulardetector.go b/pkg/plugins/manager/loader/angulardetector/angulardetector.go index 4f13aa24007..6180a6b6585 100644 --- a/pkg/plugins/manager/loader/angulardetector/angulardetector.go +++ b/pkg/plugins/manager/loader/angulardetector/angulardetector.go @@ -2,8 +2,6 @@ package angulardetector import ( "bytes" - "fmt" - "io" "regexp" "github.com/grafana/grafana/pkg/plugins" @@ -40,43 +38,9 @@ func (d *regexDetector) Detect(moduleJs []byte) bool { return d.regex.Match(moduleJs) } -// angularDetectors contains all the detectors to detect Angular plugins. -// They are executed in the specified order. -var angularDetectors = []detector{ - &containsBytesDetector{pattern: []byte("PanelCtrl")}, - &containsBytesDetector{pattern: []byte("QueryCtrl")}, - &containsBytesDetector{pattern: []byte("app/plugins/sdk")}, - &containsBytesDetector{pattern: []byte("angular.isNumber(")}, - &containsBytesDetector{pattern: []byte("editor.html")}, - &containsBytesDetector{pattern: []byte("ctrl.annotation")}, - &containsBytesDetector{pattern: []byte("getLegacyAngularInjector")}, - - ®exDetector{regex: regexp.MustCompile(`['"](app/core/utils/promiseToDigest)|(app/plugins/.*?)|(app/core/core_module)['"]`)}, - ®exDetector{regex: regexp.MustCompile(`from\s+['"]grafana\/app\/`)}, - ®exDetector{regex: regexp.MustCompile(`System\.register\(`)}, -} - -// Inspect open module.js and checks if the plugin is using Angular by matching against its source code. -// It returns true if module.js matches against any of the detectors in angularDetectors. -func Inspect(p *plugins.Plugin) (isAngular bool, err error) { - f, err := p.FS.Open("module.js") - if err != nil { - return false, fmt.Errorf("open module.js: %w", err) - } - defer func() { - if closeErr := f.Close(); closeErr != nil && err == nil { - err = fmt.Errorf("close module.js: %w", closeErr) - } - }() - b, err := io.ReadAll(f) - if err != nil { - return false, fmt.Errorf("module.js readall: %w", err) - } - for _, d := range angularDetectors { - if d.Detect(b) { - isAngular = true - break - } - } - return +// Inspector can inspect a module.js and determine if it's an Angular plugin or not. +type Inspector interface { + // Inspect open module.js and checks if the plugin is using Angular by matching against its source code. + // It returns true if module.js matches against any of the detectors in angularDetectors. + Inspect(p *plugins.Plugin) (bool, error) } diff --git a/pkg/plugins/manager/loader/angulardetector/angulardetector_test.go b/pkg/plugins/manager/loader/angulardetector/angulardetector_test.go index ca595df1520..f77cae7f118 100644 --- a/pkg/plugins/manager/loader/angulardetector/angulardetector_test.go +++ b/pkg/plugins/manager/loader/angulardetector/angulardetector_test.go @@ -44,9 +44,10 @@ func TestAngularDetector_Inspect(t *testing.T) { }, exp: false, }) + inspector := NewDefaultPatternsListInspector() for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - isAngular, err := Inspect(tc.plugin) + isAngular, err := inspector.Inspect(tc.plugin) require.NoError(t, err) require.Equal(t, tc.exp, isAngular) }) @@ -54,7 +55,33 @@ func TestAngularDetector_Inspect(t *testing.T) { t.Run("no module.js", func(t *testing.T) { p := &plugins.Plugin{FS: plugins.NewInMemoryFS(map[string][]byte{})} - _, err := Inspect(p) + _, err := inspector.Inspect(p) require.ErrorIs(t, err, plugins.ErrFileNotExist) }) } + +func TestFakeInspector(t *testing.T) { + t.Run("FakeInspector", func(t *testing.T) { + var called bool + inspector := FakeInspector{InspectFunc: func(p *plugins.Plugin) (bool, error) { + called = true + return false, nil + }} + r, err := inspector.Inspect(&plugins.Plugin{}) + require.True(t, called) + require.NoError(t, err) + require.False(t, r) + }) + + t.Run("AlwaysAngularFakeInspector", func(t *testing.T) { + r, err := AlwaysAngularFakeInspector.Inspect(&plugins.Plugin{}) + require.NoError(t, err) + require.True(t, r) + }) + + t.Run("NeverAngularFakeInspector", func(t *testing.T) { + r, err := NeverAngularFakeInspector.Inspect(&plugins.Plugin{}) + require.NoError(t, err) + require.False(t, r) + }) +} diff --git a/pkg/plugins/manager/loader/angulardetector/fakes.go b/pkg/plugins/manager/loader/angulardetector/fakes.go new file mode 100644 index 00000000000..eac9e26a1d7 --- /dev/null +++ b/pkg/plugins/manager/loader/angulardetector/fakes.go @@ -0,0 +1,29 @@ +package angulardetector + +import "github.com/grafana/grafana/pkg/plugins" + +// FakeInspector is an inspector whose Inspect function can be set to any function. +type FakeInspector struct { + // InspectFunc is the function called when calling Inspect() + InspectFunc func(p *plugins.Plugin) (bool, error) +} + +func (i *FakeInspector) Inspect(p *plugins.Plugin) (bool, error) { + return i.InspectFunc(p) +} + +var ( + // AlwaysAngularFakeInspector is an inspector that always returns `true, nil` + AlwaysAngularFakeInspector = &FakeInspector{ + InspectFunc: func(p *plugins.Plugin) (bool, error) { + return true, nil + }, + } + + // NeverAngularFakeInspector is an inspector that always returns `false, nil` + NeverAngularFakeInspector = &FakeInspector{ + InspectFunc: func(p *plugins.Plugin) (bool, error) { + return false, nil + }, + } +) diff --git a/pkg/plugins/manager/loader/angulardetector/service.go b/pkg/plugins/manager/loader/angulardetector/service.go new file mode 100644 index 00000000000..6b0867d82de --- /dev/null +++ b/pkg/plugins/manager/loader/angulardetector/service.go @@ -0,0 +1,62 @@ +package angulardetector + +import ( + "fmt" + "io" + "regexp" + + "github.com/grafana/grafana/pkg/plugins" +) + +// defaultDetectors contains all the detectors to detect Angular plugins. +// They are executed in the specified order. +var defaultDetectors = []detector{ + &containsBytesDetector{pattern: []byte("PanelCtrl")}, + &containsBytesDetector{pattern: []byte("QueryCtrl")}, + &containsBytesDetector{pattern: []byte("app/plugins/sdk")}, + &containsBytesDetector{pattern: []byte("angular.isNumber(")}, + &containsBytesDetector{pattern: []byte("editor.html")}, + &containsBytesDetector{pattern: []byte("ctrl.annotation")}, + &containsBytesDetector{pattern: []byte("getLegacyAngularInjector")}, + + ®exDetector{regex: regexp.MustCompile(`['"](app/core/utils/promiseToDigest)|(app/plugins/.*?)|(app/core/core_module)['"]`)}, + ®exDetector{regex: regexp.MustCompile(`from\s+['"]grafana\/app\/`)}, + ®exDetector{regex: regexp.MustCompile(`System\.register\(`)}, +} + +// PatternsListInspector matches module.js against all the specified patterns, in sequence. +type PatternsListInspector struct { + detectors []detector +} + +// NewDefaultPatternsListInspector returns a new *PatternsListInspector using defaultDetectors as detectors. +func NewDefaultPatternsListInspector() *PatternsListInspector { + return &PatternsListInspector{detectors: defaultDetectors} +} + +func ProvideService() Inspector { + return NewDefaultPatternsListInspector() +} + +func (i *PatternsListInspector) Inspect(p *plugins.Plugin) (isAngular bool, err error) { + f, err := p.FS.Open("module.js") + if err != nil { + return false, fmt.Errorf("open module.js: %w", err) + } + defer func() { + if closeErr := f.Close(); closeErr != nil && err == nil { + err = fmt.Errorf("close module.js: %w", closeErr) + } + }() + b, err := io.ReadAll(f) + if err != nil { + return false, fmt.Errorf("module.js readall: %w", err) + } + for _, d := range i.detectors { + if d.Detect(b) { + isAngular = true + break + } + } + return +} diff --git a/pkg/plugins/manager/loader/loader.go b/pkg/plugins/manager/loader/loader.go index 7f1d2b9b114..ad4a2ace04d 100644 --- a/pkg/plugins/manager/loader/loader.go +++ b/pkg/plugins/manager/loader/loader.go @@ -36,20 +36,24 @@ type Loader struct { log log.Logger cfg *config.Cfg + angularInspector angulardetector.Inspector + errs map[string]*plugins.SignatureError } func ProvideService(cfg *config.Cfg, license plugins.Licensing, authorizer plugins.PluginLoaderAuthorizer, pluginRegistry registry.Service, backendProvider plugins.BackendFactoryProvider, pluginFinder finder.Finder, - roleRegistry plugins.RoleRegistry, assetPath *assetpath.Service, signatureCalculator plugins.SignatureCalculator) *Loader { + roleRegistry plugins.RoleRegistry, assetPath *assetpath.Service, signatureCalculator plugins.SignatureCalculator, + angularInspector angulardetector.Inspector) *Loader { return New(cfg, license, authorizer, pluginRegistry, backendProvider, process.NewManager(pluginRegistry), - roleRegistry, assetPath, pluginFinder, signatureCalculator) + roleRegistry, assetPath, pluginFinder, signatureCalculator, angularInspector) } func New(cfg *config.Cfg, license plugins.Licensing, authorizer plugins.PluginLoaderAuthorizer, pluginRegistry registry.Service, backendProvider plugins.BackendFactoryProvider, processManager process.Service, roleRegistry plugins.RoleRegistry, - assetPath *assetpath.Service, pluginFinder finder.Finder, signatureCalculator plugins.SignatureCalculator) *Loader { + assetPath *assetpath.Service, pluginFinder finder.Finder, signatureCalculator plugins.SignatureCalculator, + angularInspector angulardetector.Inspector) *Loader { return &Loader{ pluginFinder: pluginFinder, pluginRegistry: pluginRegistry, @@ -62,6 +66,7 @@ func New(cfg *config.Cfg, license plugins.Licensing, authorizer plugins.PluginLo roleRegistry: roleRegistry, cfg: cfg, assetPath: assetPath, + angularInspector: angularInspector, } } @@ -74,6 +79,7 @@ func (l *Loader) Load(ctx context.Context, src plugins.PluginSource) ([]*plugins return l.loadPlugins(ctx, src, found) } +// nolint:gocyclo func (l *Loader) loadPlugins(ctx context.Context, src plugins.PluginSource, found []*plugins.FoundBundle) ([]*plugins.Plugin, error) { var loadedPlugins []*plugins.Plugin @@ -162,15 +168,6 @@ func (l *Loader) loadPlugins(ctx context.Context, src plugins.PluginSource, foun } } - // Detect angular for external plugins - if plugin.IsExternalPlugin() { - var err error - plugin.AngularDetected, err = angulardetector.Inspect(plugin) - if err != nil { - l.log.Warn("could not inspect plugin for angular", "pluginID", plugin.ID, "err", err) - } - } - if plugin.IsApp() { setDefaultNavURL(plugin) } @@ -185,6 +182,21 @@ func (l *Loader) loadPlugins(ctx context.Context, src plugins.PluginSource, foun // initialize plugins initializedPlugins := make([]*plugins.Plugin, 0) for _, p := range verifiedPlugins { + // Detect angular for external plugins + if p.IsExternalPlugin() { + var err error + p.AngularDetected, err = l.angularInspector.Inspect(p) + if err != nil { + l.log.Warn("could not inspect plugin for angular", "pluginID", p.ID, "err", err) + } + + // Do not initialize plugins if they're using Angular and Angular support is disabled + if p.AngularDetected && !l.cfg.AngularSupportEnabled { + l.log.Error("Refusing to initialize plugin because it's using Angular, which has been disabled", "pluginID", p.ID) + continue + } + } + err := l.pluginInitializer.Initialize(ctx, p) if err != nil { l.log.Error("Could not initialize plugin", "pluginId", p.ID, "err", err) diff --git a/pkg/plugins/manager/loader/loader_test.go b/pkg/plugins/manager/loader/loader_test.go index ccb93246490..66745c476f5 100644 --- a/pkg/plugins/manager/loader/loader_test.go +++ b/pkg/plugins/manager/loader/loader_test.go @@ -16,6 +16,7 @@ import ( "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/log" "github.com/grafana/grafana/pkg/plugins/manager/fakes" + "github.com/grafana/grafana/pkg/plugins/manager/loader/angulardetector" "github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath" "github.com/grafana/grafana/pkg/plugins/manager/loader/finder" "github.com/grafana/grafana/pkg/plugins/manager/loader/initializer" @@ -1076,6 +1077,58 @@ func TestLoader_Load_SkipUninitializedPlugins(t *testing.T) { }) } +func TestLoader_Load_Angular(t *testing.T) { + fakePluginSource := &fakes.FakePluginSource{ + PluginClassFunc: func(ctx context.Context) plugins.Class { + return plugins.External + }, + PluginURIsFunc: func(ctx context.Context) []string { + return []string{"../testdata/valid-v2-signature"} + }, + } + for _, cfgTc := range []struct { + name string + cfg *config.Cfg + }{ + {name: "angular support enabled", cfg: &config.Cfg{AngularSupportEnabled: true}}, + {name: "angular support disabled", cfg: &config.Cfg{AngularSupportEnabled: false}}, + } { + t.Run(cfgTc.name, func(t *testing.T) { + for _, tc := range []struct { + name string + angularInspector angulardetector.Inspector + shouldLoad bool + }{ + { + name: "angular plugin", + angularInspector: angulardetector.AlwaysAngularFakeInspector, + // angular plugins should load only if allowed by the cfg + shouldLoad: cfgTc.cfg.AngularSupportEnabled, + }, + { + name: "non angular plugin", + angularInspector: angulardetector.NeverAngularFakeInspector, + // non-angular plugins should always load + shouldLoad: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + l := newLoader(cfgTc.cfg, func(l *Loader) { + l.angularInspector = tc.angularInspector + }) + p, err := l.Load(context.Background(), fakePluginSource) + require.NoError(t, err) + if tc.shouldLoad { + require.Len(t, p, 1, "plugin should have been loaded") + } else { + require.Empty(t, p, "plugin shouldn't have been loaded") + } + }) + } + }) + } +} + func TestLoader_Load_NestedPlugins(t *testing.T) { rootDir, err := filepath.Abs("../") if err != nil { @@ -1387,7 +1440,7 @@ func newLoader(cfg *config.Cfg, cbs ...func(loader *Loader)) *Loader { l := New(cfg, &fakes.FakeLicensingService{}, signature.NewUnsignedAuthorizer(cfg), fakes.NewFakePluginRegistry(), fakes.NewFakeBackendProcessProvider(), fakes.NewFakeProcessManager(), fakes.NewFakeRoleRegistry(), assetpath.ProvideService(pluginscdn.ProvideService(cfg)), finder.NewLocalFinder(cfg), - signature.ProvideService(cfg, statickey.New())) + signature.ProvideService(cfg, statickey.New()), angulardetector.NewDefaultPatternsListInspector()) for _, cb := range cbs { cb(l) diff --git a/pkg/plugins/manager/manager_integration_test.go b/pkg/plugins/manager/manager_integration_test.go index 7d648254cb1..deafc31f658 100644 --- a/pkg/plugins/manager/manager_integration_test.go +++ b/pkg/plugins/manager/manager_integration_test.go @@ -10,7 +10,6 @@ import ( "github.com/grafana/grafana-azure-sdk-go/azsettings" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" - "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" "github.com/stretchr/testify/require" "gopkg.in/ini.v1" @@ -22,6 +21,7 @@ import ( "github.com/grafana/grafana/pkg/plugins/manager/client" "github.com/grafana/grafana/pkg/plugins/manager/fakes" "github.com/grafana/grafana/pkg/plugins/manager/loader" + "github.com/grafana/grafana/pkg/plugins/manager/loader/angulardetector" "github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath" "github.com/grafana/grafana/pkg/plugins/manager/loader/finder" "github.com/grafana/grafana/pkg/plugins/manager/registry" @@ -30,6 +30,7 @@ import ( "github.com/grafana/grafana/pkg/plugins/manager/sources" "github.com/grafana/grafana/pkg/plugins/manager/store" "github.com/grafana/grafana/pkg/plugins/pluginscdn" + "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/licensing" "github.com/grafana/grafana/pkg/services/pluginsintegration/config" @@ -119,7 +120,8 @@ func TestIntegrationPluginManager(t *testing.T) { lic := plicensing.ProvideLicensing(cfg, &licensing.OSSLicensingService{Cfg: cfg}) l := loader.ProvideService(pCfg, lic, signature.NewUnsignedAuthorizer(pCfg), reg, provider.ProvideService(coreRegistry), finder.NewLocalFinder(pCfg), fakes.NewFakeRoleRegistry(), - assetpath.ProvideService(pluginscdn.ProvideService(pCfg)), signature.ProvideService(pCfg, statickey.New())) + assetpath.ProvideService(pluginscdn.ProvideService(pCfg)), signature.ProvideService(pCfg, statickey.New()), + angulardetector.NewDefaultPatternsListInspector()) srcs := sources.ProvideService(cfg, pCfg) ps, err := store.ProvideService(reg, srcs, l) require.NoError(t, err) diff --git a/pkg/services/pluginsintegration/config/config.go b/pkg/services/pluginsintegration/config/config.go index ced64a8895d..2e484c31104 100644 --- a/pkg/services/pluginsintegration/config/config.go +++ b/pkg/services/pluginsintegration/config/config.go @@ -41,6 +41,7 @@ func ProvideConfig(settingProvider setting.Provider, grafanaCfg *setting.Cfg, fe grafanaCfg.PluginsCDNURLTemplate, tracingCfg, featuremgmt.ProvideToggles(features), + grafanaCfg.AngularSupportEnabled, ), nil } diff --git a/pkg/services/pluginsintegration/pluginsintegration.go b/pkg/services/pluginsintegration/pluginsintegration.go index 2f6d18670a0..5d9b6d8e764 100644 --- a/pkg/services/pluginsintegration/pluginsintegration.go +++ b/pkg/services/pluginsintegration/pluginsintegration.go @@ -12,6 +12,7 @@ import ( "github.com/grafana/grafana/pkg/plugins/manager/client" "github.com/grafana/grafana/pkg/plugins/manager/filestore" "github.com/grafana/grafana/pkg/plugins/manager/loader" + "github.com/grafana/grafana/pkg/plugins/manager/loader/angulardetector" "github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath" "github.com/grafana/grafana/pkg/plugins/manager/loader/finder" "github.com/grafana/grafana/pkg/plugins/manager/process" @@ -51,6 +52,7 @@ var WireSet = wire.NewSet( coreplugin.ProvideCoreRegistry, pluginscdn.ProvideService, assetpath.ProvideService, + angulardetector.ProvideService, loader.ProvideService, wire.Bind(new(loader.Service), new(*loader.Loader)), wire.Bind(new(plugins.ErrorResolver), new(*loader.Loader)),