Plugins: Forbid loading Angular plugins when Angular is disabled (#69225)

* Plugins: Forbid loading Angular plugins when Angular is disabled

* Plugins: Made angulardetector a service, add tests for angular loader cases

* Fix missing import

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)

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,
 	}
 }

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.
+// Inspector can inspect a module.js and determine if it's an Angular plugin or not.
-// They are executed in the specified order.
+type Inspector interface {
-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")},
-
-	&regexDetector{regex: regexp.MustCompile(`['"](app/core/utils/promiseToDigest)|(app/plugins/.*?)|(app/core/core_module)['"]`)},
-	&regexDetector{regex: regexp.MustCompile(`from\s+['"]grafana\/app\/`)},
-	&regexDetector{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) {
+	Inspect(p *plugins.Plugin) (bool, 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
 }

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)
+	})
+}

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
+		},
+	}
+)

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")},
+
+	&regexDetector{regex: regexp.MustCompile(`['"](app/core/utils/promiseToDigest)|(app/plugins/.*?)|(app/core/core_module)['"]`)},
+	&regexDetector{regex: regexp.MustCompile(`from\s+['"]grafana\/app\/`)},
+	&regexDetector{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
+}

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,
 	}
 }
 
@@ -162,15 +167,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 +181,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)

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)

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)

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
 }
 

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)),