enhance: return err if notification has no plugin type (#3638)

* enhance: return err if plugin has no type

* fix + reduce error verbosity

---------

Co-authored-by: marco <[email protected]>

Author: LaurenceJJones

cmd/crowdsec/api.go

@@ -38,7 +38,7 @@ func initAPIServer(ctx context.Context, cConfig *csconfig.Config) (*apiserver.AP
 
 		err = pluginBroker.Init(ctx, cConfig.PluginConfig, cConfig.API.Server.Profiles, cConfig.ConfigPaths)
 		if err != nil {
-			return nil, fmt.Errorf("unable to run plugin broker: %w", err)
+			return nil, fmt.Errorf("plugin broker: %w", err)
 		}
 
 		log.Info("initiated plugin broker")

pkg/csplugin/broker.go

@@ -84,11 +84,11 @@ func (pb *PluginBroker) Init(ctx context.Context, pluginCfg *csconfig.PluginCfg,
 	pb.pluginsTypesToDispatch = make(map[string]struct{})
 
 	if err := pb.loadConfig(configPaths.NotificationDir); err != nil {
-		return fmt.Errorf("while loading plugin config: %w", err)
+		return fmt.Errorf("loading config: %w", err)
 	}
 
 	if err := pb.loadPlugins(ctx, configPaths.PluginDir); err != nil {
-		return fmt.Errorf("while loading plugin: %w", err)
+		return fmt.Errorf("loading plugin: %w", err)
 	}
 
 	pb.watcher = PluginWatcher{}
@@ -409,8 +409,12 @@ func ParsePluginConfigFile(path string) ([]PluginConfig, error) {
 	dec := yaml.NewDecoder(yamlFile)
 	dec.SetStrict(true)
 
+	idx := -1
+
 	for {
-		pc := PluginConfig{}
+		var pc PluginConfig
+
+		idx += 1
 
 		err = dec.Decode(&pc)
 		if err != nil {
@@ -420,11 +424,17 @@ func ParsePluginConfigFile(path string) ([]PluginConfig, error) {
 
 			return nil, fmt.Errorf("while decoding %s got error %s", path, err)
 		}
+
 		// if the yaml document is empty, skip
 		if reflect.DeepEqual(pc, PluginConfig{}) {
 			continue
 		}
 
+		if pc.Type == "" {
+			return nil, fmt.Errorf("field 'type' missing in %s (position %d)", path, idx)
+
+		}
+
 		parsedConfigs = append(parsedConfigs, pc)
 	}
 

test/bats/70_plugin_http.bats

@@ -19,6 +19,7 @@ setup_file() {
 
     # https://mikefarah.gitbook.io/yq/operators/env-variable-operators
     config_set "$(config_get '.config_paths.notification_dir')/http.yaml" '
+        .type="http" |
         .url=strenv(MOCK_URL) |
         .group_wait="5s" |
         .group_threshold=2

test/bats/71_plugin_dummy.bats

@@ -17,6 +17,7 @@ setup_file() {
     DUMMY_YAML="$(config_get '.config_paths.notification_dir')/dummy.yaml"
 
     config_set "$DUMMY_YAML" '
+       .type="dummy" |
        .group_wait="5s" |
        .group_threshold=2 |
        .output_file=strenv(tempfile) |

test/bats/72_plugin_badconfig.bats

@@ -38,39 +38,39 @@ teardown() {
     config_set '.plugin_config.user="" | .plugin_config.group="nogroup"'
     config_set "$PROFILES_PATH" '.notifications=["http_default"]'
     rune -0 wait-for \
-        --err "api server init: unable to run plugin broker: while loading plugin: while getting process attributes: both plugin user and group must be set" \
+        --err "api server init: plugin broker: loading plugin: while getting process attributes: both plugin user and group must be set" \
         "$CROWDSEC"
 }
 
 @test "misconfigured plugin, only group is empty" {
     config_set '(.plugin_config.user="nobody") | (.plugin_config.group="")'
     config_set "$PROFILES_PATH" '.notifications=["http_default"]'
     rune -0 wait-for \
-        --err "api server init: unable to run plugin broker: while loading plugin: while getting process attributes: both plugin user and group must be set" \
+        --err "api server init: plugin broker: loading plugin: while getting process attributes: both plugin user and group must be set" \
         "$CROWDSEC"
 }
 
 @test "misconfigured plugin, user does not exist" {
     config_set '(.plugin_config.user="userdoesnotexist") | (.plugin_config.group="groupdoesnotexist")'
     config_set "$PROFILES_PATH" '.notifications=["http_default"]'
     rune -0 wait-for \
-        --err "api server init: unable to run plugin broker: while loading plugin: while getting process attributes: user: unknown user userdoesnotexist" \
+        --err "api server init: plugin broker: loading plugin: while getting process attributes: user: unknown user userdoesnotexist" \
         "$CROWDSEC"
 }
 
 @test "misconfigured plugin, group does not exist" {
     config_set '(.plugin_config.user=strenv(USER)) | (.plugin_config.group="groupdoesnotexist")'
     config_set "$PROFILES_PATH" '.notifications=["http_default"]'
     rune -0 wait-for \
-        --err "api server init: unable to run plugin broker: while loading plugin: while getting process attributes: group: unknown group groupdoesnotexist" \
+        --err "api server init: plugin broker: loading plugin: while getting process attributes: group: unknown group groupdoesnotexist" \
         "$CROWDSEC"
 }
 
 @test "bad plugin name" {
     config_set "$PROFILES_PATH" '.notifications=["http_default"]'
     cp "$PLUGIN_DIR"/notification-http "$PLUGIN_DIR"/badname
     rune -0 wait-for \
-        --err "api server init: unable to run plugin broker: while loading plugin: plugin name ${PLUGIN_DIR}/badname is invalid. Name should be like {type-name}" \
+        --err "api server init: plugin broker: loading plugin: plugin name ${PLUGIN_DIR}/badname is invalid. Name should be like {type-name}" \
         "$CROWDSEC"
 }
 
@@ -90,15 +90,15 @@ teardown() {
     config_set "$PROFILES_PATH" '.notifications=["http_default"]'
     chmod g+w "$PLUGIN_DIR"/notification-http
     rune -0 wait-for \
-        --err "api server init: unable to run plugin broker: while loading plugin: plugin at ${PLUGIN_DIR}/notification-http is group writable, group writable plugins are invalid" \
+        --err "api server init: plugin broker: loading plugin: plugin at ${PLUGIN_DIR}/notification-http is group writable, group writable plugins are invalid" \
         "$CROWDSEC"
 }
 
 @test "bad plugin permission (world writable)" {
     config_set "$PROFILES_PATH" '.notifications=["http_default"]'
     chmod o+w "$PLUGIN_DIR"/notification-http
     rune -0 wait-for \
-        --err "api server init: unable to run plugin broker: while loading plugin: plugin at ${PLUGIN_DIR}/notification-http is world writable, world writable plugins are invalid" \
+        --err "api server init: plugin broker: loading plugin: plugin at ${PLUGIN_DIR}/notification-http is world writable, world writable plugins are invalid" \
         "$CROWDSEC"
 }
 
@@ -124,10 +124,22 @@ teardown() {
         "$CROWDSEC"
 }
 
-@test "unable to run plugin broker: while reading plugin config" {
+@test "plugin broker: missing notification dir" {
     config_set '.config_paths.notification_dir="/this/path/does/not/exist"'
     config_set "$PROFILES_PATH" '.notifications=["http_default"]'
     rune -0 wait-for \
-        --err "api server init: unable to run plugin broker: while loading plugin config: open /this/path/does/not/exist: no such file or directory" \
+        --err "api server init: plugin broker: loading config: open /this/path/does/not/exist: no such file or directory" \
         "$CROWDSEC"
 }
+
+@test "misconfigured notification: missing plugin type" {
+    rune -0 yq -i 'del(.type)' "$CONFIG_DIR/notifications/http.yaml"
+    # enable a notification, otherwise plugins are ignored
+    config_set "$PROFILES_PATH" '.notifications=["http_default"]'
+    # the slack plugin may fail or not, but we just need the logs
+    config_set '.common.log_media="stdout"'
+    rune wait-for \
+        --err "api server init: plugin broker: loading plugin config" \
+        "$CROWDSEC"
+    assert_stderr --partial "field 'type' missing in $CONFIG_DIR/notifications/http.yaml (position 0)"
+}

test/bats/73_plugin_formatting.bats

@@ -17,6 +17,7 @@ setup_file() {
     # the $alert is not a shell variable
     # shellcheck disable=SC2016
     config_set "$DUMMY_YAML" '
+       .type="dummy" |
        .group_wait="5s" |
        .group_threshold=2 |
        .output_file=strenv(tempfile) |