MM-51786: Sentry crash while installing a blocked plugin (#22804)

We would return "nil, nil" if a plugin was on the blocklist.
This would cause a nil dereference panic while trying to add
struct fields from manifest to the audit logs.

To avoid it, we return an error explicitly and ignore the
error ids to unnecessarily have it log an error.

https://mattermost.atlassian.net/browse/MM-51786

```release-note
NONE
```

server/channels/app/plugin.go

@@ -352,7 +352,7 @@ func (ch *Channels) syncPlugins() *model.AppError {
 			}
 
 			mlog.Info("Syncing plugin from file store", mlog.String("bundle", plugin.path))
-			if _, err := ch.installPluginLocally(reader, signature, installPluginLocallyAlways); err != nil {
+			if _, err := ch.installPluginLocally(reader, signature, installPluginLocallyAlways); err != nil && err.Id != "app.plugin.blocked.app_error" && err.Id != "app.plugin.skip_installation.app_error" {
 				mlog.Error("Failed to sync plugin from file store", mlog.String("bundle", plugin.path), mlog.Err(err))
 			}
 		}(plugin)
@@ -952,6 +952,11 @@ func (ch *Channels) processPrepackagedPlugins(pluginsDir string) []*plugin.Prepa
 			defer wg.Done()
 			p, err := ch.processPrepackagedPlugin(psPath)
 			if err != nil {
+				var appErr *model.AppError
+				// A log line already appears if the plugin is on the blocklist
+				if errors.As(err, &appErr) && (appErr.Id == "app.plugin.blocked.app_error" || appErr.Id == "app.plugin.skip_installation.app_error") {
+					return
+				}
 				mlog.Error("Failed to install prepackaged plugin", mlog.String("path", psPath.path), mlog.Err(err))
 				return
 			}

server/channels/app/plugin_install.go

@@ -92,7 +92,10 @@ func (ch *Channels) installPluginFromData(data model.PluginEventData) {
 
 	manifest, appErr := ch.installPluginLocally(reader, signature, installPluginLocallyAlways)
 	if appErr != nil {
-		mlog.Error("Failed to sync plugin from file store", mlog.String("bundle", plugin.path), mlog.Err(appErr))
+		// A log line already appears if the plugin is on the blocklist or skipped
+		if appErr.Id != "app.plugin.blocked.app_error" && appErr.Id != "app.plugin.skip_installation.app_error" {
+			mlog.Error("Failed to sync plugin from file store", mlog.String("bundle", plugin.path), mlog.Err(appErr))
+		}
 		return
 	}
 
@@ -330,8 +333,8 @@ func (ch *Channels) installExtractedPlugin(manifest *model.Manifest, fromPluginD
 
 	// Check plugin id is not blocked
 	if plugin.PluginIDIsBlocked(manifest.Id) {
-		mlog.Debug("Skipping installation of plugin since plugin is on blocklist", mlog.String("plugin_id", manifest.Id))
-		return nil, nil
+		mlog.Debug("Skipping installation of plugin since plugin is on blocklist. Some plugins are blocked because they are built into this version of Mattermost.", mlog.String("plugin_id", manifest.Id))
+		return nil, model.NewAppError("installExtractedPlugin", "app.plugin.blocked.app_error", map[string]any{"Id": manifest.Id}, "", http.StatusInternalServerError)
 	}
 
 	// Check for plugins installed with the same ID.
@@ -365,7 +368,7 @@ func (ch *Channels) installExtractedPlugin(manifest *model.Manifest, fromPluginD
 
 			if version.LTE(existingVersion) {
 				mlog.Debug("Skipping local installation of plugin since existing version is newer", mlog.String("plugin_id", manifest.Id))
-				return nil, nil
+				return nil, model.NewAppError("installExtractedPlugin", "app.plugin.skip_installation.app_error", map[string]any{"Id": manifest.Id}, "", http.StatusInternalServerError)
 			}
 		}
 

server/channels/app/plugin_install_test.go

@@ -172,10 +172,9 @@ func TestInstallPluginLocally(t *testing.T) {
 		defer th.TearDown()
 		cleanExistingBundles(t, th)
 
-		manifest, appErr := installPlugin(t, th, "playbooks", "0.0.1", installPluginLocallyAlways)
-		require.Nil(t, appErr)
-		require.Nil(t, manifest)
-
+		_, appErr := installPlugin(t, th, "playbooks", "0.0.1", installPluginLocallyAlways)
+		require.NotNil(t, appErr)
+		require.Equal(t, "app.plugin.blocked.app_error", appErr.Id)
 		assertBundleInfoManifests(t, th, []*model.Manifest{})
 	})
 
@@ -222,9 +221,9 @@ func TestInstallPluginLocally(t *testing.T) {
 			require.Nil(t, appErr)
 			require.NotNil(t, existingManifest)
 
-			manifest, appErr := installPlugin(t, th, "valid", "0.0.1", installPluginLocallyOnlyIfNewOrUpgrade)
-			require.Nil(t, appErr)
-			require.Nil(t, manifest)
+			_, appErr = installPlugin(t, th, "valid", "0.0.1", installPluginLocallyOnlyIfNewOrUpgrade)
+			require.NotNil(t, appErr)
+			require.Equal(t, "app.plugin.skip_installation.app_error", appErr.Id)
 
 			assertBundleInfoManifests(t, th, []*model.Manifest{existingManifest})
 		})
@@ -238,9 +237,9 @@ func TestInstallPluginLocally(t *testing.T) {
 			require.Nil(t, appErr)
 			require.NotNil(t, existingManifest)
 
-			manifest, appErr := installPlugin(t, th, "valid", "0.0.2", installPluginLocallyOnlyIfNewOrUpgrade)
-			require.Nil(t, appErr)
-			require.Nil(t, manifest)
+			_, appErr = installPlugin(t, th, "valid", "0.0.2", installPluginLocallyOnlyIfNewOrUpgrade)
+			require.NotNil(t, appErr)
+			require.Equal(t, "app.plugin.skip_installation.app_error", appErr.Id)
 
 			assertBundleInfoManifests(t, th, []*model.Manifest{existingManifest})
 		})

server/i18n/en.json

@@ -5931,6 +5931,10 @@
     "id": "app.oauth.update_app.updating.app_error",
     "translation": "We encountered an error updating the app."
   },
+  {
+    "id": "app.plugin.blocked.app_error",
+    "translation": "Plugin {{.Id}} is on the block list. Some plugins are blocked because they are built into this version of Mattermost."
+  },
   {
     "id": "app.plugin.cluster.save_config.app_error",
     "translation": "The plugin configuration in your config.json file must be updated manually when using ReadOnlyConfig with clustering enabled."
@@ -6063,6 +6067,10 @@
     "id": "app.plugin.signature_decode.app_error",
     "translation": "Unable to decode base64 signature."
   },
+  {
+    "id": "app.plugin.skip_installation.app_error",
+    "translation": "Skipping installation of plugin {{.Id}} since existing version is equal or newer."
+  },
   {
     "id": "app.plugin.store_bundle.app_error",
     "translation": "Unable to store the plugin to the configured file store."