diff options
-rw-r--r-- | app/plugin.go | 73 | ||||
-rw-r--r-- | i18n/en.json | 6 | ||||
-rw-r--r-- | mlog/log.go | 1 | ||||
-rw-r--r-- | model/manifest.go | 6 | ||||
-rw-r--r-- | plugin/pluginenv/environment.go | 6 | ||||
-rw-r--r-- | plugin/pluginenv/environment_test.go | 2 | ||||
-rw-r--r-- | plugin/rpcplugin/hooks.go | 10 | ||||
-rw-r--r-- | plugin/rpcplugin/hooks_test.go | 2 | ||||
-rw-r--r-- | plugin/rpcplugin/main.go | 4 | ||||
-rw-r--r-- | plugin/rpcplugin/main_test.go | 12 | ||||
-rw-r--r-- | plugin/rpcplugin/rpcplugintest/supervisor.go | 83 | ||||
-rw-r--r-- | plugin/rpcplugin/sandbox/main_test.go | 18 | ||||
-rw-r--r-- | plugin/rpcplugin/supervisor.go | 26 | ||||
-rw-r--r-- | plugin/valid.go | 32 | ||||
-rw-r--r-- | plugin/valid_test.go | 32 |
15 files changed, 256 insertions, 57 deletions
diff --git a/app/plugin.go b/app/plugin.go index 3da9cea40..0aaa8d1d4 100644 --- a/app/plugin.go +++ b/app/plugin.go @@ -15,7 +15,6 @@ import ( "os" "path/filepath" "strings" - "unicode/utf8" "github.com/gorilla/mux" "github.com/mattermost/mattermost-server/mlog" @@ -33,10 +32,6 @@ import ( "github.com/mattermost/mattermost-server/plugin/rpcplugin/sandbox" ) -const ( - PLUGIN_MAX_ID_LENGTH = 190 -) - var prepackagedPlugins map[string]func(string) ([]byte, error) = map[string]func(string) ([]byte, error){ "jira": jira.Asset, "zoom": zoom.Asset, @@ -47,7 +42,7 @@ func (a *App) initBuiltInPlugins() { "ldapextras": &ldapextras.Plugin{}, } for id, p := range plugins { - mlog.Debug("Initializing built-in plugin: " + id) + mlog.Debug("Initializing built-in plugin", mlog.String("plugin_id", id)) api := &BuiltInPluginAPI{ id: id, router: a.Srv.Router.PathPrefix("/plugins/" + id).Subrouter(), @@ -65,21 +60,23 @@ func (a *App) initBuiltInPlugins() { } } -// ActivatePlugins will activate any plugins enabled in the config -// and deactivate all other plugins. -func (a *App) ActivatePlugins() { +func (a *App) setPluginsActive(activate bool) { if a.PluginEnv == nil { - mlog.Error("plugin env not initialized") + mlog.Error(fmt.Sprintf("Cannot setPluginsActive(%t): plugin env not initialized", activate)) return } plugins, err := a.PluginEnv.Plugins() if err != nil { - mlog.Error("failed to activate plugins: " + err.Error()) + mlog.Error(fmt.Sprintf("Cannot setPluginsActive(%t)", activate), mlog.Err(err)) return } for _, plugin := range plugins { + if plugin.Manifest == nil { + continue + } + id := plugin.Manifest.Id pluginState := &model.PluginState{Enable: false} @@ -89,15 +86,14 @@ func (a *App) ActivatePlugins() { active := a.PluginEnv.IsPluginActive(id) - if pluginState.Enable && !active { + if activate && pluginState.Enable && !active { if err := a.activatePlugin(plugin.Manifest); err != nil { - mlog.Error(fmt.Sprintf("%v plugin enabled in config.json but failing to activate err=%v", plugin.Manifest.Id, err.DetailedError)) - continue + mlog.Error("Plugin failed to activate", mlog.String("plugin_id", plugin.Manifest.Id), mlog.String("err", err.DetailedError)) } - } else if !pluginState.Enable && active { + } else if (!activate || !pluginState.Enable) && active { if err := a.deactivatePlugin(plugin.Manifest); err != nil { - mlog.Error(err.Error()) + mlog.Error("Plugin failed to deactivate", mlog.String("plugin_id", plugin.Manifest.Id), mlog.String("err", err.DetailedError)) } } } @@ -114,13 +110,13 @@ func (a *App) activatePlugin(manifest *model.Manifest) *model.AppError { a.Publish(message) } - mlog.Info(fmt.Sprintf("Activated %v plugin", manifest.Id)) + mlog.Info("Activated plugin", mlog.String("plugin_id", manifest.Id)) return nil } func (a *App) deactivatePlugin(manifest *model.Manifest) *model.AppError { if err := a.PluginEnv.DeactivatePlugin(manifest.Id); err != nil { - return model.NewAppError("removePlugin", "app.plugin.deactivate.app_error", nil, err.Error(), http.StatusBadRequest) + return model.NewAppError("deactivatePlugin", "app.plugin.deactivate.app_error", nil, err.Error(), http.StatusBadRequest) } a.UnregisterPluginCommands(manifest.Id) @@ -131,7 +127,7 @@ func (a *App) deactivatePlugin(manifest *model.Manifest) *model.AppError { a.Publish(message) } - mlog.Info(fmt.Sprintf("Deactivated %v plugin", manifest.Id)) + mlog.Info("Deactivated plugin", mlog.String("plugin_id", manifest.Id)) return nil } @@ -174,8 +170,8 @@ func (a *App) installPlugin(pluginFile io.Reader, allowPrepackaged bool) (*model return nil, model.NewAppError("installPlugin", "app.plugin.prepackaged.app_error", nil, "", http.StatusBadRequest) } - if utf8.RuneCountInString(manifest.Id) > PLUGIN_MAX_ID_LENGTH { - return nil, model.NewAppError("installPlugin", "app.plugin.id_length.app_error", map[string]interface{}{"Max": PLUGIN_MAX_ID_LENGTH}, err.Error(), http.StatusBadRequest) + if !plugin.IsValidId(manifest.Id) { + return nil, model.NewAppError("installPlugin", "app.plugin.invalid_id.app_error", map[string]interface{}{"Min": plugin.MinIdLength, "Max": plugin.MaxIdLength, "Regex": plugin.ValidId.String()}, "", http.StatusBadRequest) } bundles, err := a.PluginEnv.Plugins() @@ -184,7 +180,7 @@ func (a *App) installPlugin(pluginFile io.Reader, allowPrepackaged bool) (*model } for _, bundle := range bundles { - if bundle.Manifest.Id == manifest.Id { + if bundle.Manifest != nil && bundle.Manifest.Id == manifest.Id { return nil, model.NewAppError("installPlugin", "app.plugin.install_id.app_error", nil, "", http.StatusBadRequest) } } @@ -211,6 +207,10 @@ func (a *App) GetPlugins() (*model.PluginsResponse, *model.AppError) { resp := &model.PluginsResponse{Active: []*model.PluginInfo{}, Inactive: []*model.PluginInfo{}} for _, plugin := range plugins { + if plugin.Manifest == nil { + continue + } + info := &model.PluginInfo{ Manifest: *plugin.Manifest, } @@ -259,9 +259,11 @@ func (a *App) removePlugin(id string, allowPrepackaged bool) *model.AppError { } var manifest *model.Manifest + var pluginPath string for _, p := range plugins { - if p.Manifest.Id == id { + if p.Manifest != nil && p.Manifest.Id == id { manifest = p.Manifest + pluginPath = filepath.Dir(p.ManifestPath) break } } @@ -277,7 +279,7 @@ func (a *App) removePlugin(id string, allowPrepackaged bool) *model.AppError { } } - err = os.RemoveAll(filepath.Join(a.PluginEnv.SearchPath(), id)) + err = os.RemoveAll(pluginPath) if err != nil { return model.NewAppError("removePlugin", "app.plugin.remove.app_error", nil, err.Error(), http.StatusInternalServerError) } @@ -372,12 +374,12 @@ func (a *App) InitPlugins(pluginPath, webappPath string, supervisorOverride plug mlog.Info("Starting up plugins") if err := os.Mkdir(pluginPath, 0744); err != nil && !os.IsExist(err) { - mlog.Error("failed to start up plugins: " + err.Error()) + mlog.Error("Failed to start up plugins", mlog.Err(err)) return } if err := os.Mkdir(webappPath, 0744); err != nil && !os.IsExist(err) { - mlog.Error("failed to start up plugins: " + err.Error()) + mlog.Error("Failed to start up plugins", mlog.Err(err)) return } @@ -399,15 +401,14 @@ func (a *App) InitPlugins(pluginPath, webappPath string, supervisorOverride plug if supervisorOverride != nil { options = append(options, pluginenv.SupervisorProvider(supervisorOverride)) } else if err := sandbox.CheckSupport(); err != nil { - mlog.Warn(err.Error()) - mlog.Warn("plugin sandboxing is not supported. plugins will run with the same access level as the server. See documentation to learn more: https://developers.mattermost.com/extend/plugins/security/") + mlog.Warn("plugin sandboxing is not supported. plugins will run with the same access level as the server. See documentation to learn more: https://developers.mattermost.com/extend/plugins/security/", mlog.Err(err)) options = append(options, pluginenv.SupervisorProvider(rpcplugin.SupervisorProvider)) } else { options = append(options, pluginenv.SupervisorProvider(sandbox.SupervisorProvider)) } if env, err := pluginenv.New(options...); err != nil { - mlog.Error("failed to start up plugins: " + err.Error()) + mlog.Error("Failed to start up plugins", mlog.Err(err)) return } else { a.PluginEnv = env @@ -415,36 +416,34 @@ func (a *App) InitPlugins(pluginPath, webappPath string, supervisorOverride plug for id, asset := range prepackagedPlugins { if tarball, err := asset("plugin.tar.gz"); err != nil { - mlog.Error("failed to install prepackaged plugin: " + err.Error()) + mlog.Error("Failed to install prepackaged plugin", mlog.Err(err)) } else if tarball != nil { a.removePlugin(id, true) if _, err := a.installPlugin(bytes.NewReader(tarball), true); err != nil { - mlog.Error("failed to install prepackaged plugin: " + err.Error()) + mlog.Error("Failed to install prepackaged plugin", mlog.Err(err)) } if _, ok := a.Config().PluginSettings.PluginStates[id]; !ok && id != "zoom" { if err := a.EnablePlugin(id); err != nil { - mlog.Error("failed to enable prepackaged plugin: " + err.Error()) + mlog.Error("Failed to enable prepackaged plugin", mlog.Err(err)) } } } } a.RemoveConfigListener(a.PluginConfigListenerId) - a.PluginConfigListenerId = a.AddConfigListener(func(prevCfg, cfg *model.Config) { + a.PluginConfigListenerId = a.AddConfigListener(func(_, cfg *model.Config) { if a.PluginEnv == nil { return } - if *prevCfg.PluginSettings.Enable && *cfg.PluginSettings.Enable { - a.ActivatePlugins() - } + a.setPluginsActive(*cfg.PluginSettings.Enable) for _, err := range a.PluginEnv.Hooks().OnConfigurationChange() { mlog.Error(err.Error()) } }) - a.ActivatePlugins() + a.setPluginsActive(true) } func (a *App) ServePluginRequest(w http.ResponseWriter, r *http.Request) { diff --git a/i18n/en.json b/i18n/en.json index 0afaa326c..d4a08b07a 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -3812,7 +3812,7 @@ }, { "id": "app.plugin.activate.app_error", - "translation": "Unable to activate extracted plugin. Plugin may already exist and be activated." + "translation": "Unable to activate extracted plugin." }, { "id": "app.plugin.cluster.save_config.app_error", @@ -3847,8 +3847,8 @@ "translation": "Unable to get active plugins" }, { - "id": "app.plugin.id_length.app_error", - "translation": "Plugin Id must be less than {{.Max}} characters." + "id": "app.plugin.invalid_id.app_error", + "translation": "Plugin Id must be at least {{.Min}} characters, at most {{.Max}} characters and match {{.Regex}}." }, { "id": "app.plugin.install.app_error", diff --git a/mlog/log.go b/mlog/log.go index 22db0c7b1..ad537a11d 100644 --- a/mlog/log.go +++ b/mlog/log.go @@ -29,6 +29,7 @@ type Field = zapcore.Field var Int64 = zap.Int64 var Int = zap.Int var String = zap.String +var Err = zap.Error type LoggerConfiguration struct { EnableConsole bool diff --git a/model/manifest.go b/model/manifest.go index 32d4341cd..d6a064d4e 100644 --- a/model/manifest.go +++ b/model/manifest.go @@ -93,9 +93,9 @@ type PluginSettingsSchema struct { // help_text: When true, an extra thing will be enabled! // default: false type Manifest struct { - // The id is a globally unique identifier that represents your plugin. Ids are limited - // to 190 characters. Reverse-DNS notation using a name you control is a good option. - // For example, "com.mycompany.myplugin". + // The id is a globally unique identifier that represents your plugin. Ids must be at least + // 3 characters, at most 190 characters and must match ^[a-zA-Z0-9-_\.]+$. + // Reverse-DNS notation using a name you control is a good option, e.g. "com.mycompany.myplugin". Id string `json:"id" yaml:"id"` // The name to be displayed for the plugin. diff --git a/plugin/pluginenv/environment.go b/plugin/pluginenv/environment.go index adc9ddbde..adc02e885 100644 --- a/plugin/pluginenv/environment.go +++ b/plugin/pluginenv/environment.go @@ -112,8 +112,12 @@ func (env *Environment) ActivatePlugin(id string) error { env.mutex.Lock() defer env.mutex.Unlock() + if !plugin.IsValidId(id) { + return fmt.Errorf("invalid plugin id: %s", id) + } + if _, ok := env.activePlugins[id]; ok { - return fmt.Errorf("plugin already active: %v", id) + return nil } plugins, err := ScanSearchPath(env.searchPath) if err != nil { diff --git a/plugin/pluginenv/environment_test.go b/plugin/pluginenv/environment_test.go index 2a52b3830..91d639f69 100644 --- a/plugin/pluginenv/environment_test.go +++ b/plugin/pluginenv/environment_test.go @@ -149,7 +149,7 @@ func TestEnvironment(t *testing.T) { assert.Equal(t, env.ActivePluginIds(), []string{"foo"}) activePlugins = env.ActivePlugins() assert.Len(t, activePlugins, 1) - assert.Error(t, env.ActivatePlugin("foo")) + assert.NoError(t, env.ActivatePlugin("foo")) assert.True(t, env.IsPluginActive("foo")) hooks.On("OnDeactivate").Return(nil) diff --git a/plugin/rpcplugin/hooks.go b/plugin/rpcplugin/hooks.go index 7b44d0de7..90734fd1c 100644 --- a/plugin/rpcplugin/hooks.go +++ b/plugin/rpcplugin/hooks.go @@ -11,6 +11,7 @@ import ( "net/rpc" "reflect" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/plugin" ) @@ -165,6 +166,7 @@ type RemoteHooks struct { muxer *Muxer apiCloser io.Closer implemented [maxRemoteHookCount]bool + pluginId string } var _ plugin.Hooks = (*RemoteHooks)(nil) @@ -237,6 +239,7 @@ func (h *RemoteHooks) ServeHTTP(w http.ResponseWriter, r *http.Request) { Request: forwardedRequest, RequestBodyStream: requestBodyStream, }, nil); err != nil { + mlog.Error("Plugin failed to ServeHTTP", mlog.String("plugin_id", h.pluginId), mlog.Err(err)) http.Error(w, "500 internal server error", http.StatusInternalServerError) } } @@ -260,10 +263,11 @@ func (h *RemoteHooks) Close() error { return h.client.Close() } -func ConnectHooks(conn io.ReadWriteCloser, muxer *Muxer) (*RemoteHooks, error) { +func ConnectHooks(conn io.ReadWriteCloser, muxer *Muxer, pluginId string) (*RemoteHooks, error) { remote := &RemoteHooks{ - client: rpc.NewClient(conn), - muxer: muxer, + client: rpc.NewClient(conn), + muxer: muxer, + pluginId: pluginId, } implemented, err := remote.Implemented() if err != nil { diff --git a/plugin/rpcplugin/hooks_test.go b/plugin/rpcplugin/hooks_test.go index 116038dae..c404442b7 100644 --- a/plugin/rpcplugin/hooks_test.go +++ b/plugin/rpcplugin/hooks_test.go @@ -31,7 +31,7 @@ func testHooksRPC(hooks interface{}, f func(*RemoteHooks)) error { id, server := c1.Serve() go ServeHooks(hooks, server, c1) - remote, err := ConnectHooks(c2.Connect(id), c2) + remote, err := ConnectHooks(c2.Connect(id), c2, "plugin_id") if err != nil { return err } diff --git a/plugin/rpcplugin/main.go b/plugin/rpcplugin/main.go index 96a61c068..efb880605 100644 --- a/plugin/rpcplugin/main.go +++ b/plugin/rpcplugin/main.go @@ -30,7 +30,7 @@ func Main(hooks interface{}) { } // Returns the hooks being served by a call to Main. -func ConnectMain(muxer *Muxer) (*RemoteHooks, error) { +func ConnectMain(muxer *Muxer, pluginId string) (*RemoteHooks, error) { buf := make([]byte, 1) if _, err := muxer.Read(buf); err != nil { return nil, err @@ -43,5 +43,5 @@ func ConnectMain(muxer *Muxer) (*RemoteHooks, error) { return nil, err } - return ConnectHooks(muxer.Connect(id), muxer) + return ConnectHooks(muxer.Connect(id), muxer, pluginId) } diff --git a/plugin/rpcplugin/main_test.go b/plugin/rpcplugin/main_test.go index 6cdd46df0..06423106c 100644 --- a/plugin/rpcplugin/main_test.go +++ b/plugin/rpcplugin/main_test.go @@ -10,11 +10,21 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/plugin/plugintest" "github.com/mattermost/mattermost-server/plugin/rpcplugin/rpcplugintest" ) func TestMain(t *testing.T) { + // Setup a global logger to catch tests logging outside of app context + // The global logger will be stomped by apps initalizing but that's fine for testing. Ideally this won't happen. + mlog.InitGlobalLogger(mlog.NewLogger(&mlog.LoggerConfiguration{ + EnableConsole: true, + ConsoleJson: true, + ConsoleLevel: "error", + EnableFile: false, + })) + dir, err := ioutil.TempDir("", "") require.NoError(t, err) defer os.RemoveAll(dir) @@ -46,7 +56,7 @@ func TestMain(t *testing.T) { var api plugintest.API - hooks, err := ConnectMain(muxer) + hooks, err := ConnectMain(muxer, "plugin_id") require.NoError(t, err) assert.NoError(t, hooks.OnActivate(&api)) assert.NoError(t, hooks.OnDeactivate()) diff --git a/plugin/rpcplugin/rpcplugintest/supervisor.go b/plugin/rpcplugin/rpcplugintest/supervisor.go index 05dc8ed8f..2ae065621 100644 --- a/plugin/rpcplugin/rpcplugintest/supervisor.go +++ b/plugin/rpcplugin/rpcplugintest/supervisor.go @@ -7,6 +7,8 @@ import ( "encoding/json" "fmt" "io/ioutil" + "net/http" + "net/http/httptest" "os" "path/filepath" "testing" @@ -30,6 +32,7 @@ func TestSupervisorProvider(t *testing.T, sp SupervisorProviderFunc) { "Supervisor_NonExistentExecutablePath": testSupervisor_NonExistentExecutablePath, "Supervisor_StartTimeout": testSupervisor_StartTimeout, "Supervisor_PluginCrash": testSupervisor_PluginCrash, + "Supervisor_PluginRepeatedlyCrash": testSupervisor_PluginRepeatedlyCrash, } { t.Run(name, func(t *testing.T) { f(t, sp) }) } @@ -188,3 +191,83 @@ func testSupervisor_PluginCrash(t *testing.T, sp SupervisorProviderFunc) { assert.True(t, recovered) require.NoError(t, supervisor.Stop()) } + +// Crashed plugins should be relaunched at most three times. +func testSupervisor_PluginRepeatedlyCrash(t *testing.T, sp SupervisorProviderFunc) { + dir, err := ioutil.TempDir("", "") + require.NoError(t, err) + defer os.RemoveAll(dir) + + backend := filepath.Join(dir, "backend.exe") + CompileGo(t, ` + package main + + import ( + "net/http" + "os" + + "github.com/mattermost/mattermost-server/plugin/rpcplugin" + ) + + type MyPlugin struct { + crashing bool + } + + func (p *MyPlugin) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost { + p.crashing = true + go func() { + os.Exit(1) + }() + } + + if p.crashing { + w.WriteHeader(http.StatusInternalServerError) + } else { + w.WriteHeader(http.StatusOK) + } + } + + func main() { + rpcplugin.Main(&MyPlugin{}) + } + `, backend) + + ioutil.WriteFile(filepath.Join(dir, "plugin.json"), []byte(`{"id": "foo", "backend": {"executable": "backend.exe"}}`), 0600) + + var api plugintest.API + bundle := model.BundleInfoForPath(dir) + supervisor, err := sp(bundle) + require.NoError(t, err) + require.NoError(t, supervisor.Start(&api)) + + for attempt := 1; attempt <= 4; attempt++ { + // Verify that the plugin is operational + response := httptest.NewRecorder() + supervisor.Hooks().ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/plugins/id", nil)) + require.Equal(t, http.StatusOK, response.Result().StatusCode) + + // Crash the plugin + supervisor.Hooks().ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodPost, "/plugins/id", nil)) + + // Wait for it to potentially recover + recovered := false + for i := 0; i < 125; i++ { + response := httptest.NewRecorder() + supervisor.Hooks().ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/plugins/id", nil)) + if response.Result().StatusCode == http.StatusOK { + recovered = true + break + } + + time.Sleep(time.Millisecond * 100) + } + + if attempt < 4 { + require.True(t, recovered, "failed to recover after attempt %d", attempt) + } else { + require.False(t, recovered, "unexpectedly recovered after attempt %d", attempt) + } + } + require.NoError(t, supervisor.Stop()) +} diff --git a/plugin/rpcplugin/sandbox/main_test.go b/plugin/rpcplugin/sandbox/main_test.go new file mode 100644 index 000000000..4be4a42af --- /dev/null +++ b/plugin/rpcplugin/sandbox/main_test.go @@ -0,0 +1,18 @@ +package sandbox + +import ( + "testing" + + "github.com/mattermost/mattermost-server/mlog" +) + +func TestMain(t *testing.T) { + // Setup a global logger to catch tests logging outside of app context + // The global logger will be stomped by apps initalizing but that's fine for testing. Ideally this won't happen. + mlog.InitGlobalLogger(mlog.NewLogger(&mlog.LoggerConfiguration{ + EnableConsole: true, + ConsoleJson: true, + ConsoleLevel: "error", + EnableFile: false, + })) +} diff --git a/plugin/rpcplugin/supervisor.go b/plugin/rpcplugin/supervisor.go index 6a48cb5e8..6e26d5682 100644 --- a/plugin/rpcplugin/supervisor.go +++ b/plugin/rpcplugin/supervisor.go @@ -12,19 +12,26 @@ import ( "sync/atomic" "time" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/plugin" ) +const ( + MaxProcessRestarts = 3 +) + // Supervisor implements a plugin.Supervisor that launches the plugin in a separate process and // communicates via RPC. // -// If the plugin unexpectedly exists, the supervisor will relaunch it after a short delay. +// If the plugin unexpectedly exits, the supervisor will relaunch it after a short delay, but will +// only restart a plugin at most three times. type Supervisor struct { hooks atomic.Value done chan bool cancel context.CancelFunc newProcess func(context.Context) (Process, io.ReadWriteCloser, error) + pluginId string } var _ plugin.Supervisor = (*Supervisor)(nil) @@ -66,19 +73,28 @@ func (s *Supervisor) run(ctx context.Context, start chan<- error, api plugin.API s.done <- true }() done := ctx.Done() - for { + for i := 0; i <= MaxProcessRestarts; i++ { s.runPlugin(ctx, start, api) select { case <-done: return default: start = nil - time.Sleep(time.Second) + if i < MaxProcessRestarts { + mlog.Debug("Plugin terminated unexpectedly", mlog.String("plugin_id", s.pluginId)) + time.Sleep(time.Duration((1 + i*i)) * time.Second) + } else { + mlog.Debug("Plugin terminated unexpectedly too many times", mlog.String("plugin_id", s.pluginId), mlog.Int("max_process_restarts", MaxProcessRestarts)) + } } } } func (s *Supervisor) runPlugin(ctx context.Context, start chan<- error, api plugin.API) error { + if start == nil { + mlog.Debug("Restarting plugin", mlog.String("plugin_id", s.pluginId)) + } + p, ipc, err := s.newProcess(ctx) if err != nil { if start != nil { @@ -100,7 +116,7 @@ func (s *Supervisor) runPlugin(ctx context.Context, start chan<- error, api plug muxerClosed <- muxer.Close() }() - hooks, err := ConnectMain(muxer) + hooks, err := ConnectMain(muxer, s.pluginId) if err == nil { err = hooks.OnActivate(api) } @@ -147,5 +163,5 @@ func SupervisorWithNewProcessFunc(bundle *model.BundleInfo, newProcess func(cont if strings.HasPrefix(executable, "..") { return nil, fmt.Errorf("invalid backend executable") } - return &Supervisor{newProcess: newProcess}, nil + return &Supervisor{pluginId: bundle.Manifest.Id, newProcess: newProcess}, nil } diff --git a/plugin/valid.go b/plugin/valid.go new file mode 100644 index 000000000..62c594a1a --- /dev/null +++ b/plugin/valid.go @@ -0,0 +1,32 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package plugin + +import ( + "regexp" + "unicode/utf8" +) + +const ( + MinIdLength = 3 + MaxIdLength = 190 +) + +var ValidId *regexp.Regexp + +func init() { + ValidId = regexp.MustCompile(`^[a-zA-Z0-9-_\.]+$`) +} + +func IsValidId(id string) bool { + if utf8.RuneCountInString(id) < MinIdLength { + return false + } + + if utf8.RuneCountInString(id) > MaxIdLength { + return false + } + + return ValidId.MatchString(id) +} diff --git a/plugin/valid_test.go b/plugin/valid_test.go new file mode 100644 index 000000000..d47eeb58b --- /dev/null +++ b/plugin/valid_test.go @@ -0,0 +1,32 @@ +package plugin_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/mattermost/mattermost-server/plugin" +) + +func TestIsValid(t *testing.T) { + t.Parallel() + + testCases := map[string]bool{ + "": false, + "a": false, + "ab": false, + "abc": true, + "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij": true, + "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij1": false, + "../path": false, + "/etc/passwd": false, + "com.mattermost.plugin_with_features-0.9": true, + "PLUGINS-THAT-YELL-ARE-OK-2": true, + } + + for id, valid := range testCases { + t.Run(id, func(t *testing.T) { + assert.Equal(t, valid, plugin.IsValidId(id)) + }) + } +} |