diff --git a/pkg/api/plugins.go b/pkg/api/plugins.go index 141ef9df14e..67628695fc4 100644 --- a/pkg/api/plugins.go +++ b/pkg/api/plugins.go @@ -1,12 +1,13 @@ package api import ( + "bytes" "context" "encoding/json" "errors" "fmt" + "io" "net/http" - "os" "path" "path/filepath" "runtime" @@ -16,7 +17,6 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" - "github.com/grafana/grafana/pkg/infra/fs" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/backendplugin" @@ -28,6 +28,7 @@ import ( "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/pluginsettings" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/web" ) @@ -309,42 +310,25 @@ func (hs *HTTPServer) getPluginAssets(c *models.ReqContext) { } // prepend slash for cleaning relative paths - requestedFile := filepath.Clean(filepath.Join("/", web.Params(c.Req)["*"])) - rel, err := filepath.Rel("/", requestedFile) + requestedFile, err := util.CleanRelativePath(web.Params(c.Req)["*"]) if err != nil { // slash is prepended above therefore this is not expected to fail - c.JsonApiErr(500, "Failed to get the relative path", err) + c.JsonApiErr(500, "Failed to clean relative file path", err) return } - if !plugin.IncludedInSignature(rel) { - hs.log.Warn("Access to requested plugin file will be forbidden in upcoming Grafana versions as the file "+ - "is not included in the plugin signature", "file", requestedFile) - } - - absPluginDir, err := filepath.Abs(plugin.PluginDir) + f, err := plugin.File(requestedFile) if err != nil { - c.JsonApiErr(500, "Failed to get plugin absolute path", nil) - return - } - - pluginFilePath := filepath.Join(absPluginDir, rel) - - // It's safe to ignore gosec warning G304 since we already clean the requested file path and subsequently - // use this with a prefix of the plugin's directory, which is set during plugin loading - // nolint:gosec - f, err := os.Open(pluginFilePath) - if err != nil { - if os.IsNotExist(err) { - c.JsonApiErr(404, "Plugin file not found", err) + if errors.Is(err, plugins.ErrFileNotExist) { + c.JsonApiErr(404, "Plugin file not found", nil) return } c.JsonApiErr(500, "Could not open plugin file", err) return } defer func() { - if err := f.Close(); err != nil { - hs.log.Error("Failed to close file", "err", err) + if err = f.Close(); err != nil { + hs.log.Error("Failed to close plugin file", "err", err) } }() @@ -360,7 +344,16 @@ func (hs *HTTPServer) getPluginAssets(c *models.ReqContext) { c.Resp.Header().Set("Cache-Control", "public, max-age=3600") } - http.ServeContent(c.Resp, c.Req, pluginFilePath, fi.ModTime(), f) + if rs, ok := f.(io.ReadSeeker); ok { + http.ServeContent(c.Resp, c.Req, requestedFile, fi.ModTime(), rs) + } else { + b, err := io.ReadAll(f) + if err != nil { + c.JsonApiErr(500, "Plugin file exists but could not read", err) + return + } + http.ServeContent(c.Resp, c.Req, requestedFile, fi.ModTime(), bytes.NewReader(b)) + } } // CheckHealth returns the health of a plugin. @@ -496,34 +489,24 @@ func (hs *HTTPServer) pluginMarkdown(ctx context.Context, pluginId string, name return nil, plugins.NotFoundError{PluginID: pluginId} } - // nolint:gosec - // We can ignore the gosec G304 warning since we have cleaned the requested file path and subsequently - // use this with a prefix of the plugin's directory, which is set during plugin loading - path := filepath.Join(plugin.PluginDir, mdFilepath(strings.ToUpper(name))) - exists, err := fs.Exists(path) + md, err := plugin.File(mdFilepath(strings.ToUpper(name))) if err != nil { - return nil, err - } - if !exists { - path = filepath.Join(plugin.PluginDir, mdFilepath(strings.ToLower(name))) + md, err = plugin.File(mdFilepath(strings.ToUpper(name))) + if err != nil { + return make([]byte, 0), nil + } } + defer func() { + if err = md.Close(); err != nil { + hs.log.Error("Failed to close plugin markdown file", "err", err) + } + }() - exists, err = fs.Exists(path) + d, err := io.ReadAll(md) if err != nil { - return nil, err - } - if !exists { return make([]byte, 0), nil } - - // nolint:gosec - // We can ignore the gosec G304 warning since we have cleaned the requested file path and subsequently - // use this with a prefix of the plugin's directory, which is set during plugin loading - data, err := os.ReadFile(path) - if err != nil { - return nil, err - } - return data, nil + return d, nil } func mdFilepath(mdFilename string) string { diff --git a/pkg/api/plugins_test.go b/pkg/api/plugins_test.go index e579996bc41..b0c6dea7ded 100644 --- a/pkg/api/plugins_test.go +++ b/pkg/api/plugins_test.go @@ -12,7 +12,6 @@ import ( "strings" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/grafana/grafana-plugin-sdk-go/backend" @@ -135,13 +134,13 @@ func Test_PluginsInstallAndUninstall_AccessControl(t *testing.T) { t.Run(testName("Install", tc), func(t *testing.T) { input := strings.NewReader("{ \"version\": \"1.0.2\" }") response := callAPI(sc.server, http.MethodPost, "/api/plugins/test/install", input, t) - assert.Equal(t, tc.expectedCode, response.Code) + require.Equal(t, tc.expectedCode, response.Code) }) t.Run(testName("Uninstall", tc), func(t *testing.T) { input := strings.NewReader("{ }") response := callAPI(sc.server, http.MethodPost, "/api/plugins/test/uninstall", input, t) - assert.Equal(t, tc.expectedCode, response.Code) + require.Equal(t, tc.expectedCode, response.Code) }) } } @@ -155,56 +154,45 @@ func Test_GetPluginAssets(t *testing.T) { require.NoError(t, err) t.Cleanup(func() { err := os.RemoveAll(tmpFile.Name()) - assert.NoError(t, err) + require.NoError(t, err) err = os.RemoveAll(tmpFileInParentDir.Name()) - assert.NoError(t, err) + require.NoError(t, err) }) expectedBody := "Plugin test" _, err = tmpFile.WriteString(expectedBody) - assert.NoError(t, err) + require.NoError(t, err) requestedFile := filepath.Clean(tmpFile.Name()) - t.Run("Given a request for an existing plugin file that is listed as a signature covered file", func(t *testing.T) { - p := plugins.PluginDTO{ + t.Run("Given a request for an existing plugin file", func(t *testing.T) { + p := &plugins.Plugin{ JSONData: plugins.JSONData{ ID: pluginID, }, PluginDir: pluginDir, - SignedFiles: map[string]struct{}{ - requestedFile: {}, - }, } service := &plugins.FakePluginStore{ - PluginList: []plugins.PluginDTO{p}, + PluginList: []plugins.PluginDTO{p.ToDTO()}, } - l := &logtest.Fake{} url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile) - pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, l, + pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, func(sc *scenarioContext) { callGetPluginAsset(sc) require.Equal(t, 200, sc.resp.Code) - assert.Equal(t, expectedBody, sc.resp.Body.String()) - assert.Zero(t, l.WarnLogs.Calls) + require.Equal(t, expectedBody, sc.resp.Body.String()) }) }) t.Run("Given a request for a relative path", func(t *testing.T) { - p := plugins.PluginDTO{ - JSONData: plugins.JSONData{ - ID: pluginID, - }, - PluginDir: pluginDir, - } + p := createPluginDTO(plugins.JSONData{ID: pluginID}, plugins.External, pluginDir) service := &plugins.FakePluginStore{ PluginList: []plugins.PluginDTO{p}, } - l := &logtest.Fake{} url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, tmpFileInParentDir.Name()) - pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, l, + pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, func(sc *scenarioContext) { callGetPluginAsset(sc) @@ -212,44 +200,15 @@ func Test_GetPluginAssets(t *testing.T) { }) }) - t.Run("Given a request for an existing plugin file that is not listed as a signature covered file", func(t *testing.T) { - p := plugins.PluginDTO{ - JSONData: plugins.JSONData{ - ID: pluginID, - }, - PluginDir: pluginDir, - } - service := &plugins.FakePluginStore{ - PluginList: []plugins.PluginDTO{p}, - } - l := &logtest.Fake{} - - url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile) - pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, l, - func(sc *scenarioContext) { - callGetPluginAsset(sc) - - require.Equal(t, 200, sc.resp.Code) - assert.Equal(t, expectedBody, sc.resp.Body.String()) - assert.Zero(t, l.WarnLogs.Calls) - }) - }) - t.Run("Given a request for an non-existing plugin file", func(t *testing.T) { - p := plugins.PluginDTO{ - JSONData: plugins.JSONData{ - ID: pluginID, - }, - PluginDir: pluginDir, - } + p := createPluginDTO(plugins.JSONData{ID: pluginID}, plugins.External, pluginDir) service := &plugins.FakePluginStore{ PluginList: []plugins.PluginDTO{p}, } - l := &logtest.Fake{} requestedFile := "nonExistent" url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile) - pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, l, + pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, func(sc *scenarioContext) { callGetPluginAsset(sc) @@ -257,8 +216,7 @@ func Test_GetPluginAssets(t *testing.T) { err := json.NewDecoder(sc.resp.Body).Decode(&respJson) require.NoError(t, err) require.Equal(t, 404, sc.resp.Code) - assert.Equal(t, "Plugin file not found", respJson["message"]) - assert.Zero(t, l.WarnLogs.Calls) + require.Equal(t, "Plugin file not found", respJson["message"]) }) }) @@ -270,16 +228,16 @@ func Test_GetPluginAssets(t *testing.T) { requestedFile := "nonExistent" url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile) - pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, l, + pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, func(sc *scenarioContext) { callGetPluginAsset(sc) var respJson map[string]interface{} err := json.NewDecoder(sc.resp.Body).Decode(&respJson) require.NoError(t, err) - assert.Equal(t, 404, sc.resp.Code) - assert.Equal(t, "Plugin not found", respJson["message"]) - assert.Zero(t, l.WarnLogs.Calls) + require.Equal(t, 404, sc.resp.Code) + require.Equal(t, "Plugin not found", respJson["message"]) + require.Zero(t, l.WarnLogs.Calls) }) }) @@ -295,13 +253,13 @@ func Test_GetPluginAssets(t *testing.T) { l := &logtest.Fake{} url := fmt.Sprintf("/public/plugins/%s/%s", pluginID, requestedFile) - pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, l, + pluginAssetScenario(t, "When calling GET on", url, "/public/plugins/:pluginId/*", service, func(sc *scenarioContext) { callGetPluginAsset(sc) require.Equal(t, 200, sc.resp.Code) - assert.Equal(t, expectedBody, sc.resp.Body.String()) - assert.Zero(t, l.WarnLogs.Calls) + require.Equal(t, expectedBody, sc.resp.Body.String()) + require.Zero(t, l.WarnLogs.Calls) }) }) } @@ -348,7 +306,7 @@ func TestMakePluginResourceRequestSetCookieNotPresent(t *testing.T) { break } } - assert.Empty(t, resp.Header().Values("Set-Cookie"), "Set-Cookie header should not be present") + require.Empty(t, resp.Header().Values("Set-Cookie"), "Set-Cookie header should not be present") } func TestMakePluginResourceRequestContentTypeUnique(t *testing.T) { @@ -382,8 +340,8 @@ func TestMakePluginResourceRequestContentTypeUnique(t *testing.T) { break } } - assert.Len(t, resp.Header().Values("Content-Type"), 1, "should have 1 Content-Type header") - assert.Len(t, resp.Header().Values("x-another"), 1, "should have 1 X-Another header") + require.Len(t, resp.Header().Values("Content-Type"), 1, "should have 1 Content-Type header") + require.Len(t, resp.Header().Values("x-another"), 1, "should have 1 X-Another header") }) } } @@ -417,12 +375,11 @@ func callGetPluginAsset(sc *scenarioContext) { } func pluginAssetScenario(t *testing.T, desc string, url string, urlPattern string, pluginStore plugins.Store, - logger log.Logger, fn scenarioFunc) { + fn scenarioFunc) { t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) { hs := HTTPServer{ Cfg: setting.NewCfg(), pluginStore: pluginStore, - log: logger, } sc := setupScenarioContext(t, url) @@ -478,42 +435,40 @@ func (c *fakePluginClient) QueryData(ctx context.Context, req *backend.QueryData } func Test_PluginsList_AccessControl(t *testing.T) { - pluginStore := plugins.FakePluginStore{PluginList: []plugins.PluginDTO{ - { - PluginDir: "/grafana/plugins/test-app/dist", - Class: "external", - DefaultNavURL: "/plugins/test-app/page/test", - Pinned: false, - Signature: "unsigned", - Module: "plugins/test-app/module", - BaseURL: "public/plugins/test-app", - JSONData: plugins.JSONData{ - ID: "test-app", - Type: "app", - Name: "test-app", - Info: plugins.Info{ - Version: "1.0.0", - }, + p1 := &plugins.Plugin{ + PluginDir: "/grafana/plugins/test-app/dist", + Class: plugins.External, + DefaultNavURL: "/plugins/test-app/page/test", + Signature: plugins.SignatureUnsigned, + Module: "plugins/test-app/module", + BaseURL: "public/plugins/test-app", + JSONData: plugins.JSONData{ + ID: "test-app", + Type: plugins.App, + Name: "test-app", + Info: plugins.Info{ + Version: "1.0.0", }, }, - { - PluginDir: "/grafana/public/app/plugins/datasource/mysql", - Class: "core", - Pinned: false, - Signature: "internal", - Module: "app/plugins/datasource/mysql/module", - BaseURL: "public/app/plugins/datasource/mysql", - JSONData: plugins.JSONData{ - ID: "mysql", - Type: "datasource", - Name: "MySQL", - Info: plugins.Info{ - Author: plugins.InfoLink{Name: "Grafana Labs", URL: "https://grafana.com"}, - Description: "Data source for MySQL databases", - }, + } + p2 := &plugins.Plugin{ + PluginDir: "/grafana/public/app/plugins/datasource/mysql", + Class: plugins.Core, + Pinned: false, + Signature: plugins.SignatureInternal, + Module: "app/plugins/datasource/mysql/module", + BaseURL: "public/app/plugins/datasource/mysql", + JSONData: plugins.JSONData{ + ID: "mysql", + Type: plugins.DataSource, + Name: "MySQL", + Info: plugins.Info{ + Author: plugins.InfoLink{Name: "Grafana Labs", URL: "https://grafana.com"}, + Description: "Data source for MySQL databases", }, }, - }} + } + pluginStore := plugins.FakePluginStore{PluginList: []plugins.PluginDTO{p1.ToDTO(), p2.ToDTO()}} pluginSettings := pluginsettings.FakePluginSettings{Plugins: map[string]*pluginsettings.DTO{ "test-app": {ID: 0, OrgID: 1, PluginID: "test-app", PluginVersion: "1.0.0", Enabled: true}, @@ -574,3 +529,12 @@ func Test_PluginsList_AccessControl(t *testing.T) { }) } } + +func createPluginDTO(jd plugins.JSONData, class plugins.Class, pluginDir string) plugins.PluginDTO { + p := &plugins.Plugin{ + JSONData: jd, + Class: class, + PluginDir: pluginDir, + } + return p.ToDTO() +} diff --git a/pkg/plugins/backendplugin/provider/provider.go b/pkg/plugins/backendplugin/provider/provider.go index 218d89df5e1..a83f5a8fb8f 100644 --- a/pkg/plugins/backendplugin/provider/provider.go +++ b/pkg/plugins/backendplugin/provider/provider.go @@ -2,10 +2,6 @@ package provider import ( "context" - "fmt" - "path/filepath" - "runtime" - "strings" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/plugins" @@ -49,7 +45,7 @@ var RendererProvider PluginBackendProvider = func(_ context.Context, p *plugins. if !p.IsRenderer() { return nil } - return grpcplugin.NewRendererPlugin(p.ID, filepath.Join(p.PluginDir, rendererStartCmd()), + return grpcplugin.NewRendererPlugin(p.ID, p.ExecutablePath(), func(pluginID string, renderer pluginextensionv2.RendererPlugin, logger log.Logger) error { p.Renderer = renderer return nil @@ -61,7 +57,7 @@ var SecretsManagerProvider PluginBackendProvider = func(_ context.Context, p *pl if !p.IsSecretsManager() { return nil } - return grpcplugin.NewSecretsManagerPlugin(p.ID, filepath.Join(p.PluginDir, secretsManagerStartCmd()), + return grpcplugin.NewSecretsManagerPlugin(p.ID, p.ExecutablePath(), func(pluginID string, secretsmanager secretsmanagerplugin.SecretsManagerPlugin, logger log.Logger) error { p.SecretsManager = secretsmanager return nil @@ -70,42 +66,5 @@ var SecretsManagerProvider PluginBackendProvider = func(_ context.Context, p *pl } var DefaultProvider PluginBackendProvider = func(_ context.Context, p *plugins.Plugin) backendplugin.PluginFactoryFunc { - // TODO check for executable - return grpcplugin.NewBackendPlugin(p.ID, filepath.Join(p.PluginDir, pluginStartCmd(p.Executable))) -} - -func pluginStartCmd(executable string) string { - os := strings.ToLower(runtime.GOOS) - arch := runtime.GOARCH - extension := "" - - if os == "windows" { - extension = ".exe" - } - - return fmt.Sprintf("%s_%s_%s%s", executable, os, strings.ToLower(arch), extension) -} - -func rendererStartCmd() string { - os := strings.ToLower(runtime.GOOS) - arch := runtime.GOARCH - extension := "" - - if os == "windows" { - extension = ".exe" - } - - return fmt.Sprintf("%s_%s_%s%s", "plugin_start", os, strings.ToLower(arch), extension) -} - -func secretsManagerStartCmd() string { - os := strings.ToLower(runtime.GOOS) - arch := runtime.GOARCH - extension := "" - - if os == "windows" { - extension = ".exe" - } - - return fmt.Sprintf("%s_%s_%s%s", "secrets_plugin_start", os, strings.ToLower(arch), extension) + return grpcplugin.NewBackendPlugin(p.ID, p.ExecutablePath()) } diff --git a/pkg/plugins/manager/dashboards/filestore.go b/pkg/plugins/manager/dashboards/filestore.go index cb37225f043..80796cd448b 100644 --- a/pkg/plugins/manager/dashboards/filestore.go +++ b/pkg/plugins/manager/dashboards/filestore.go @@ -4,8 +4,6 @@ import ( "context" "fmt" "io/fs" - "os" - "path/filepath" "strings" "github.com/grafana/grafana/pkg/plugins" @@ -24,10 +22,8 @@ func ProvideFileStoreManager(pluginStore plugins.Store) *FileStoreManager { } } -var openDashboardFile = func(name string) (fs.File, error) { - // Wrapping in filepath.Clean to properly handle - // gosec G304 Potential file inclusion via variable rule. - return os.Open(filepath.Clean(name)) +var openDashboardFile = func(p plugins.PluginDTO, name string) (fs.File, error) { + return p.File(name) } func (m *FileStoreManager) ListPluginDashboardFiles(ctx context.Context, args *ListPluginDashboardFilesArgs) (*ListPluginDashboardFilesResult, error) { @@ -90,8 +86,7 @@ func (m *FileStoreManager) GetPluginDashboardFileContents(ctx context.Context, a return nil, err } - dashboardFilePath := filepath.Join(plugin.PluginDir, cleanPath) - file, err := openDashboardFile(dashboardFilePath) + file, err := openDashboardFile(plugin, cleanPath) if err != nil { return nil, err } diff --git a/pkg/plugins/manager/dashboards/filestore_test.go b/pkg/plugins/manager/dashboards/filestore_test.go index ae9697864bf..4ab977e4187 100644 --- a/pkg/plugins/manager/dashboards/filestore_test.go +++ b/pkg/plugins/manager/dashboards/filestore_test.go @@ -3,6 +3,7 @@ package dashboards import ( "context" "io" + "io/fs" "testing" "testing/fstest" @@ -117,20 +118,24 @@ func TestDashboardFileStore(t *testing.T) { t.Run("With filesystem", func(t *testing.T) { origOpenDashboardFile := openDashboardFile mapFs := fstest.MapFS{ - "plugins/plugin-id/dashboards/dash1.json": { + "dashboards/dash1.json": { Data: []byte("dash1"), }, - "plugins/plugin-id/dashboards/dash2.json": { + "dashboards/dash2.json": { Data: []byte("dash2"), }, - "plugins/plugin-id/dashboards/dash3.json": { + "dashboards/dash3.json": { Data: []byte("dash3"), }, - "plugins/plugin-id/dash2.json": { + "dash2.json": { Data: []byte("dash2"), }, } - openDashboardFile = mapFs.Open + openDashboardFile = func(p plugins.PluginDTO, name string) (fs.File, error) { + f, err := mapFs.Open(name) + require.NoError(t, err) + return f, nil + } t.Cleanup(func() { openDashboardFile = origOpenDashboardFile }) @@ -156,7 +161,6 @@ func TestDashboardFileStore(t *testing.T) { b, err := io.ReadAll(res.Content) require.NoError(t, err) require.Equal(t, "dash1", string(b)) - require.NoError(t, res.Content.Close()) }) t.Run("Should return file content for dashboards/dash2.json", func(t *testing.T) { @@ -170,7 +174,6 @@ func TestDashboardFileStore(t *testing.T) { b, err := io.ReadAll(res.Content) require.NoError(t, err) require.Equal(t, "dash2", string(b)) - require.NoError(t, res.Content.Close()) }) t.Run("Should return error when trying to read relative file", func(t *testing.T) { @@ -189,39 +192,39 @@ func TestDashboardFileStore(t *testing.T) { func setupPluginDashboardsForTest(t *testing.T) *FileStoreManager { t.Helper() - return &FileStoreManager{ - pluginStore: &plugins.FakePluginStore{ - PluginList: []plugins.PluginDTO{ + p1 := &plugins.Plugin{ + JSONData: plugins.JSONData{ + ID: "pluginWithoutDashboards", + Includes: []*plugins.Includes{ { - JSONData: plugins.JSONData{ - ID: "pluginWithoutDashboards", - Includes: []*plugins.Includes{ - { - Type: "page", - }, - }, - }, - }, - { - PluginDir: "plugins/plugin-id", - JSONData: plugins.JSONData{ - ID: "pluginWithDashboards", - Includes: []*plugins.Includes{ - { - Type: "page", - }, - { - Type: "dashboard", - Path: "dashboards/dash1.json", - }, - { - Type: "dashboard", - Path: "dashboards/dash2.json", - }, - }, - }, + Type: "page", }, }, }, } + + p2 := &plugins.Plugin{ + JSONData: plugins.JSONData{ + ID: "pluginWithDashboards", + Includes: []*plugins.Includes{ + { + Type: "page", + }, + { + Type: "dashboard", + Path: "dashboards/dash1.json", + }, + { + Type: "dashboard", + Path: "dashboards/dash2.json", + }, + }, + }, + } + + return &FileStoreManager{ + pluginStore: &plugins.FakePluginStore{ + PluginList: []plugins.PluginDTO{p1.ToDTO(), p2.ToDTO()}, + }, + } } diff --git a/pkg/plugins/manager/installer.go b/pkg/plugins/manager/installer.go index 0d82f6ee561..5f0e22b9697 100644 --- a/pkg/plugins/manager/installer.go +++ b/pkg/plugins/manager/installer.go @@ -51,8 +51,7 @@ func (m *PluginInstaller) Add(ctx context.Context, pluginID, version string, opt if plugin.Info.Version == version { return plugins.DuplicateError{ - PluginID: plugin.ID, - ExistingPluginDir: plugin.PluginDir, + PluginID: plugin.ID, } } @@ -65,8 +64,7 @@ func (m *PluginInstaller) Add(ctx context.Context, pluginID, version string, opt // if existing plugin version is the same as the target update version if dlOpts.Version == plugin.Info.Version { return plugins.DuplicateError{ - PluginID: plugin.ID, - ExistingPluginDir: plugin.PluginDir, + PluginID: plugin.ID, } } diff --git a/pkg/plugins/manager/installer_test.go b/pkg/plugins/manager/installer_test.go index fe4066f2163..02e394c396d 100644 --- a/pkg/plugins/manager/installer_test.go +++ b/pkg/plugins/manager/installer_test.go @@ -81,8 +81,7 @@ func TestPluginManager_Add_Remove(t *testing.T) { err = inst.Add(context.Background(), pluginID, v1, plugins.CompatOpts{}) require.Equal(t, plugins.DuplicateError{ - PluginID: pluginV1.ID, - ExistingPluginDir: pluginV1.PluginDir, + PluginID: pluginV1.ID, }, err) }) diff --git a/pkg/plugins/manager/loader/loader.go b/pkg/plugins/manager/loader/loader.go index 4a42e941f92..14d76ad985a 100644 --- a/pkg/plugins/manager/loader/loader.go +++ b/pkg/plugins/manager/loader/loader.go @@ -127,7 +127,6 @@ func (l *Loader) loadPlugins(ctx context.Context, class plugins.Class, pluginJSO plugin.Signature = sig.Status plugin.SignatureType = sig.Type plugin.SignatureOrg = sig.SigningOrg - plugin.SignedFiles = sig.Files loadedPlugins[plugin.PluginDir] = plugin } diff --git a/pkg/plugins/manager/manager_integration_test.go b/pkg/plugins/manager/manager_integration_test.go index 8bda9300e45..cfffbef2737 100644 --- a/pkg/plugins/manager/manager_integration_test.go +++ b/pkg/plugins/manager/manager_integration_test.go @@ -116,8 +116,8 @@ func TestIntegrationPluginManager(t *testing.T) { ctx := context.Background() verifyCorePluginCatalogue(t, ctx, ps) - verifyBundledPlugins(t, ctx, ps) - verifyPluginStaticRoutes(t, ctx, ps) + verifyBundledPlugins(t, ctx, ps, reg) + verifyPluginStaticRoutes(t, ctx, ps, reg) verifyBackendProcesses(t, reg.Plugins(ctx)) verifyPluginQuery(t, ctx, client.ProvideService(reg, pCfg)) } @@ -245,7 +245,7 @@ func verifyCorePluginCatalogue(t *testing.T, ctx context.Context, ps *store.Serv require.Equal(t, len(expPanels)+len(expDataSources)+len(expApps), len(ps.Plugins(ctx))) } -func verifyBundledPlugins(t *testing.T, ctx context.Context, ps *store.Service) { +func verifyBundledPlugins(t *testing.T, ctx context.Context, ps *store.Service, reg registry.Service) { t.Helper() dsPlugins := make(map[string]struct{}) @@ -258,6 +258,9 @@ func verifyBundledPlugins(t *testing.T, ctx context.Context, ps *store.Service) require.NotEqual(t, plugins.PluginDTO{}, inputPlugin) require.NotNil(t, dsPlugins["input"]) + intInputPlugin, exists := reg.Plugin(ctx, "input") + require.True(t, exists) + pluginRoutes := make(map[string]*plugins.StaticRoute) for _, r := range ps.Routes() { pluginRoutes[r.PluginID] = r @@ -265,23 +268,23 @@ func verifyBundledPlugins(t *testing.T, ctx context.Context, ps *store.Service) for _, pluginID := range []string{"input"} { require.Contains(t, pluginRoutes, pluginID) - require.True(t, strings.HasPrefix(pluginRoutes[pluginID].Directory, inputPlugin.PluginDir)) + require.True(t, strings.HasPrefix(pluginRoutes[pluginID].Directory, intInputPlugin.PluginDir)) } } -func verifyPluginStaticRoutes(t *testing.T, ctx context.Context, ps *store.Service) { +func verifyPluginStaticRoutes(t *testing.T, ctx context.Context, rr plugins.StaticRouteResolver, reg registry.Service) { routes := make(map[string]*plugins.StaticRoute) - for _, route := range ps.Routes() { + for _, route := range rr.Routes() { routes[route.PluginID] = route } require.Len(t, routes, 2) - inputPlugin, _ := ps.Plugin(ctx, "input") + inputPlugin, _ := reg.Plugin(ctx, "input") require.NotNil(t, routes["input"]) require.Equal(t, routes["input"].Directory, inputPlugin.PluginDir) - testAppPlugin, _ := ps.Plugin(ctx, "test-app") + testAppPlugin, _ := reg.Plugin(ctx, "test-app") require.Contains(t, routes, "test-app") require.Equal(t, routes["test-app"].Directory, testAppPlugin.PluginDir) } diff --git a/pkg/plugins/manager/signature/manifest.go b/pkg/plugins/manager/signature/manifest.go index 858ccf57910..8edf7f2d556 100644 --- a/pkg/plugins/manager/signature/manifest.go +++ b/pkg/plugins/manager/signature/manifest.go @@ -111,12 +111,7 @@ func Calculate(mlog log.Logger, plugin *plugins.Plugin) (plugins.Signature, erro }, err } - manifestPath := filepath.Join(plugin.PluginDir, "MANIFEST.txt") - - // nolint:gosec - // We can ignore the gosec G304 warning on this one because `manifestPath` is based - // on plugin the folder structure on disk and not user input. - byteValue, err := os.ReadFile(manifestPath) + byteValue := plugin.Manifest() if err != nil || len(byteValue) < 10 { mlog.Debug("Plugin is unsigned", "id", plugin.ID) return plugins.Signature{ diff --git a/pkg/plugins/manager/signature/signature.go b/pkg/plugins/manager/signature/signature.go index b0d3c6d3bbb..2e7262d7569 100644 --- a/pkg/plugins/manager/signature/signature.go +++ b/pkg/plugins/manager/signature/signature.go @@ -54,7 +54,7 @@ func (s *Validator) Validate(plugin *plugins.Plugin) *plugins.SignatureError { SignatureStatus: plugins.SignatureUnsigned, } } - s.log.Warn("Permitting unsigned plugin. This is not recommended", "pluginID", plugin.ID, "pluginDir", plugin.PluginDir) + s.log.Warn("Permitting unsigned plugin. This is not recommended", "pluginID", plugin.ID) return nil case plugins.SignatureInvalid: s.log.Debug("Plugin has an invalid signature", "pluginID", plugin.ID) diff --git a/pkg/plugins/models.go b/pkg/plugins/models.go index 3eed57d64ca..d60c7656314 100644 --- a/pkg/plugins/models.go +++ b/pkg/plugins/models.go @@ -26,12 +26,11 @@ func (e NotFoundError) Error() string { } type DuplicateError struct { - PluginID string - ExistingPluginDir string + PluginID string } func (e DuplicateError) Error() string { - return fmt.Sprintf("plugin with ID '%s' already exists in '%s'", e.PluginID, e.ExistingPluginDir) + return fmt.Sprintf("plugin with ID '%s' already exists", e.PluginID) } func (e DuplicateError) Is(err error) bool { @@ -195,13 +194,10 @@ func (s SignatureType) IsValid() bool { return false } -type PluginFiles map[string]struct{} - type Signature struct { Status SignatureStatus Type SignatureType SigningOrg string - Files PluginFiles } type PluginMetaDTO struct { diff --git a/pkg/plugins/plugins.go b/pkg/plugins/plugins.go index c0400d369c2..81bfbf11d6c 100644 --- a/pkg/plugins/plugins.go +++ b/pkg/plugins/plugins.go @@ -4,6 +4,11 @@ import ( "context" "encoding/json" "fmt" + "io/fs" + "os" + "path/filepath" + "runtime" + "strings" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana/pkg/infra/log" @@ -11,8 +16,11 @@ import ( "github.com/grafana/grafana/pkg/plugins/backendplugin/pluginextensionv2" "github.com/grafana/grafana/pkg/plugins/backendplugin/secretsmanagerplugin" "github.com/grafana/grafana/pkg/services/org" + "github.com/grafana/grafana/pkg/util" ) +var ErrFileNotExist = fmt.Errorf("file does not exist") + type Plugin struct { JSONData @@ -30,7 +38,6 @@ type Plugin struct { SignatureOrg string Parent *Plugin Children []*Plugin - SignedFiles PluginFiles SignatureError *SignatureError // SystemJS fields @@ -46,8 +53,10 @@ type Plugin struct { type PluginDTO struct { JSONData - PluginDir string - Class Class + logger log.Logger + pluginDir string + + Class Class // App fields IncludedInAppID string @@ -58,7 +67,6 @@ type PluginDTO struct { Signature SignatureStatus SignatureType SignatureType SignatureOrg string - SignedFiles PluginFiles SignatureError *SignatureError // SystemJS fields @@ -89,21 +97,29 @@ func (p PluginDTO) IsSecretsManager() bool { return p.JSONData.Type == SecretsManager } -func (p PluginDTO) IncludedInSignature(file string) bool { - // permit Core plugin files - if p.IsCorePlugin() { - return true +func (p PluginDTO) File(name string) (fs.File, error) { + cleanPath, err := util.CleanRelativePath(name) + if err != nil { + // CleanRelativePath should clean and make the path relative so this is not expected to fail + return nil, err } - // permit when no signed files (no MANIFEST) - if p.SignedFiles == nil { - return true + absPluginDir, err := filepath.Abs(p.pluginDir) + if err != nil { + return nil, err } - if _, exists := p.SignedFiles[file]; !exists { - return false + absFilePath := filepath.Join(absPluginDir, cleanPath) + // Wrapping in filepath.Clean to properly handle + // gosec G304 Potential file inclusion via variable rule. + f, err := os.Open(filepath.Clean(absFilePath)) + if err != nil { + if os.IsNotExist(err) { + return nil, ErrFileNotExist + } + return nil, err } - return true + return f, nil } // JSONData represents the plugin's plugin.json @@ -318,6 +334,25 @@ func (p *Plugin) Client() (PluginClient, bool) { return nil, false } +func (p *Plugin) ExecutablePath() string { + os := strings.ToLower(runtime.GOOS) + arch := runtime.GOARCH + extension := "" + + if os == "windows" { + extension = ".exe" + } + if p.IsRenderer() { + return filepath.Join(p.PluginDir, fmt.Sprintf("%s_%s_%s%s", "plugin_start", os, strings.ToLower(arch), extension)) + } + + if p.IsSecretsManager() { + return filepath.Join(p.PluginDir, fmt.Sprintf("%s_%s_%s%s", "secrets_plugin_start", os, strings.ToLower(arch), extension)) + } + + return filepath.Join(p.PluginDir, fmt.Sprintf("%s_%s_%s%s", p.Executable, os, strings.ToLower(arch), extension)) +} + type PluginClient interface { backend.QueryDataHandler backend.CollectMetricsHandler @@ -330,8 +365,9 @@ func (p *Plugin) ToDTO() PluginDTO { c, _ := p.Client() return PluginDTO{ + logger: p.Logger(), + pluginDir: p.PluginDir, JSONData: p.JSONData, - PluginDir: p.PluginDir, Class: p.Class, IncludedInAppID: p.IncludedInAppID, DefaultNavURL: p.DefaultNavURL, @@ -339,7 +375,6 @@ func (p *Plugin) ToDTO() PluginDTO { Signature: p.Signature, SignatureType: p.SignatureType, SignatureOrg: p.SignatureOrg, - SignedFiles: p.SignedFiles, SignatureError: p.SignatureError, Module: p.Module, BaseURL: p.BaseURL, @@ -387,6 +422,15 @@ func (p *Plugin) IsExternalPlugin() bool { return p.Class == External } +func (p *Plugin) Manifest() []byte { + d, err := os.ReadFile(filepath.Join(p.PluginDir, "MANIFEST.txt")) + if err != nil { + return []byte{} + } + + return d +} + type Class string const ( diff --git a/pkg/services/plugindashboards/service/service.go b/pkg/services/plugindashboards/service/service.go index a2934ce6ea9..ddb7e1697f5 100644 --- a/pkg/services/plugindashboards/service/service.go +++ b/pkg/services/plugindashboards/service/service.go @@ -116,7 +116,7 @@ func (s Service) LoadPluginDashboard(ctx context.Context, req *plugindashboards. } defer func() { - if err := resp.Content.Close(); err != nil { + if err = resp.Content.Close(); err != nil { s.logger.Warn("Failed to close plugin dashboard file", "reference", req.Reference, "err", err) } }() diff --git a/pkg/services/plugindashboards/service/service_test.go b/pkg/services/plugindashboards/service/service_test.go index f46af1556f3..ed367318b59 100644 --- a/pkg/services/plugindashboards/service/service_test.go +++ b/pkg/services/plugindashboards/service/service_test.go @@ -191,9 +191,8 @@ func (m pluginDashboardStoreMock) ListPluginDashboardFiles(ctx context.Context, func (m pluginDashboardStoreMock) GetPluginDashboardFileContents(ctx context.Context, args *dashboards.GetPluginDashboardFileContentsArgs) (*dashboards.GetPluginDashboardFileContentsResult, error) { if dashboardFiles, exists := m.pluginDashboardFiles[args.PluginID]; exists { if content, exists := dashboardFiles[args.FileReference]; exists { - r := bytes.NewReader(content) return &dashboards.GetPluginDashboardFileContentsResult{ - Content: io.NopCloser(r), + Content: io.NopCloser(bytes.NewReader(content)), }, nil } } else if !exists {