Merge pull request #5084 from rspamd/vstakhov-negative-group-limits

[Feature] Allow to set negative group score limit via `min_score`

12 files changed, 162 insertions, 26 deletions

diff --git a/src/libmime/scan_result.c b/src/libmime/scan_result.c
index f15290b95..894ae4f9e 100644
--- a/src/libmime/scan_result.c
+++ b/src/libmime/scan_result.c
@@ -201,16 +201,34 @@ rspamd_check_group_score(struct rspamd_task *task,
 						 double *group_score,
 						 double w)
 {
-	if (gr != NULL && group_score && gr->max_score > 0.0 && w > 0.0) {
-		if (*group_score >= gr->max_score && w > 0) {
+	double group_limit = NAN;
+
+	if (gr != NULL && group_score) {
+		if ((*group_score + w) >= 0 && !isnan(gr->max_score) && gr->max_score > 0) {
+			group_limit = gr->max_score;
+		}
+		else if ((*group_score + w) < 0 && !isnan(gr->min_score) && gr->min_score < 0) {
+			group_limit = -gr->min_score;
+		}
+	}
+
+	if (gr != NULL && group_limit && !isnan(group_limit)) {
+		if (fabs(*group_score) >= group_limit && signbit(*group_score) == signbit(w)) {
+			/* Cannot add more to the group */
 			msg_info_task("maximum group score %.2f for group %s has been reached,"
 						  " ignoring symbol %s with weight %.2f",
-						  gr->max_score,
+						  group_limit,
 						  gr->name, symbol, w);
 			return NAN;
 		}
-		else if (*group_score + w > gr->max_score) {
-			w = gr->max_score - *group_score;
+		else if (fabs(*group_score + w) > group_limit) {
+			/* Reduce weight */
+			double new_w = signbit(w) ? -group_limit - *group_score : group_limit - *group_score;
+			msg_info_task("maximum group score %.2f for group %s has been reached,"
+						  " reduce weight of symbol %s from %.2f to %.2f",
+						  group_limit,
+						  gr->name, symbol, w, new_w);
+			w = new_w;
 		}
 	}
 
@@ -393,15 +411,7 @@ insert_metric_result(struct rspamd_task *task,
 					}
 					else if (gr_score) {
 						*gr_score += cur_diff;
-
-						if (cur_diff < diff) {
-							/* Reduce */
-							msg_debug_metric(
-								"group limit %.2f is reached for %s when inserting symbol %s;"
-								" reduce score %.2f - %.2f",
-								*gr_score, gr->name, symbol, diff, cur_diff);
-							diff = cur_diff;
-						}
+						diff = cur_diff;
 					}
 				}
 			}
@@ -461,15 +471,7 @@ insert_metric_result(struct rspamd_task *task,
 				}
 				else if (gr_score) {
 					*gr_score += cur_score;
-
-					if (cur_score < final_score) {
-						/* Reduce */
-						msg_debug_metric(
-							"group limit %.2f is reached for %s when inserting symbol %s;"
-							" reduce score %.2f - %.2f",
-							*gr_score, gr->name, symbol, final_score, cur_score);
-						final_score = cur_score;
-					}
+					final_score = cur_score;
 				}
 			}
 		}
diff --git a/src/libserver/cfg_file.h b/src/libserver/cfg_file.h
index 1ba1d84ad..fa784f2a2 100644
--- a/src/libserver/cfg_file.h
+++ b/src/libserver/cfg_file.h
@@ -102,6 +102,7 @@ struct rspamd_symbols_group {
 	char *description;
 	GHashTable *symbols;
 	double max_score;
+	double min_score;
 	unsigned int flags;
 };
 
diff --git a/src/libserver/cfg_rcl.cxx b/src/libserver/cfg_rcl.cxx
index 8a479fa6d..9b6e759bb 100644
--- a/src/libserver/cfg_rcl.cxx
+++ b/src/libserver/cfg_rcl.cxx
@@ -420,6 +420,18 @@ rspamd_rcl_group_handler(rspamd_mempool_t *pool, const ucl_object_t *obj,
 		return FALSE;
 	}
 
+	if (!std::isnan(gr->max_score) && gr->max_score < 0) {
+		msg_err_config("group %s has negative max_score which is broken, use min_score if required", gr->name);
+
+		return FALSE;
+	}
+	if (!std::isnan(gr->min_score) && gr->min_score > 0) {
+		msg_err_config("group %s has positive min_score which is broken, use max_score if required", gr->name);
+
+		return FALSE;
+	}
+
+
 	if (const auto *elt = ucl_object_lookup(obj, "one_shot"); elt != nullptr) {
 		if (ucl_object_type(elt) != UCL_BOOLEAN) {
 			g_set_error(err,
@@ -2349,6 +2361,12 @@ rspamd_rcl_config_init(struct rspamd_config *cfg, GHashTable *skip_sections)
 									   G_STRUCT_OFFSET(struct rspamd_symbols_group, max_score),
 									   0,
 									   "Maximum score that could be reached by this symbols group");
+		rspamd_rcl_add_default_handler(sub,
+									   "min_score",
+									   rspamd_rcl_parse_struct_double,
+									   G_STRUCT_OFFSET(struct rspamd_symbols_group, min_score),
+									   0,
+									   "Maximum negative score that could be reached by this symbols group");
 	}
 
 	if (!(skip_sections && g_hash_table_lookup(skip_sections, "worker"))) {
diff --git a/src/libserver/cfg_utils.cxx b/src/libserver/cfg_utils.cxx
index 1344bc4f9..d8696e72d 100644
--- a/src/libserver/cfg_utils.cxx
+++ b/src/libserver/cfg_utils.cxx
@@ -1052,6 +1052,8 @@ rspamd_config_new_group(struct rspamd_config *cfg, const char *name)
 	rspamd_mempool_add_destructor(cfg->cfg_pool,
 								  (rspamd_mempool_destruct_t) g_hash_table_unref, gr->symbols);
 	gr->name = rspamd_mempool_strdup(cfg->cfg_pool, name);
+	gr->max_score = NAN;
+	gr->min_score = NAN;
 
 	if (strcmp(gr->name, "ungrouped") == 0) {
 		gr->flags |= RSPAMD_SYMBOL_GROUP_UNGROUPED;
diff --git a/src/lua/lua_config.c b/src/lua/lua_config.c
index f9a79eef1..717aa81ce 100644
--- a/src/lua/lua_config.c
+++ b/src/lua/lua_config.c
@@ -3933,6 +3933,8 @@ lua_config_get_groups(lua_State *L)
 				lua_setfield(L, -2, "description");
 				lua_pushnumber(L, gr->max_score);
 				lua_setfield(L, -2, "max_score");
+				lua_pushnumber(L, gr->min_score);
+				lua_setfield(L, -2, "min_score");
 				lua_pushboolean(L, (gr->flags & RSPAMD_SYMBOL_GROUP_PUBLIC) != 0);
 				lua_setfield(L, -2, "is_public");
 				/* TODO: maybe push symbols as well */
diff --git a/src/plugins/fuzzy_check.c b/src/plugins/fuzzy_check.c
index a035eeaae..b92177f1b 100644
--- a/src/plugins/fuzzy_check.c
+++ b/src/plugins/fuzzy_check.c
@@ -2299,6 +2299,8 @@ fuzzy_insert_result(struct fuzzy_client_session *session,
 	 * Otherwise `value` means error code
 	 */
 
+	msg_debug_fuzzy_check("got reply with probability %.2f and value %.2f",
+						  (double) rep->v1.prob, (double) rep->v1.value);
 	nval = fuzzy_normalize(rep->v1.value, weight);
 
 	if (io) {
diff --git a/src/rspamadm/configdump.c b/src/rspamadm/configdump.c
index 167b4c891..456875cf2 100644
--- a/src/rspamadm/configdump.c
+++ b/src/rspamadm/configdump.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2023 Vsevolod Stakhov
+ * Copyright 2024 Vsevolod Stakhov
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -445,6 +445,9 @@ rspamadm_configdump(int argc, char **argv, const struct rspamadm_command *cmd)
 									  ucl_object_fromdouble(gr->max_score),
 									  "max_score", strlen("max_score"), false);
 				ucl_object_insert_key(gr_ucl,
+									  ucl_object_fromdouble(gr->min_score),
+									  "min_score", strlen("min_score"), false);
+				ucl_object_insert_key(gr_ucl,
 									  ucl_object_fromstring(gr->description),
 									  "description", strlen("description"), false);
 
diff --git a/test/functional/cases/001_merged/101_lua.robot b/test/functional/cases/001_merged/101_lua.robot
index 2cfc03677..51d5b4b23 100644
--- a/test/functional/cases/001_merged/101_lua.robot
+++ b/test/functional/cases/001_merged/101_lua.robot
@@ -52,4 +52,48 @@ External Maps Simple
 
 Task Inject Url
   Scan File  ${URL_ICS}  Settings={symbols_enabled = [TEST_INJECT_URL]}
-  Expect Symbol  TEST_INJECT_URL
\ No newline at end of file
+  Expect Symbol  TEST_INJECT_URL
+
+Group Score Positive
+  Scan File  ${MESSAGE}  Settings={symbols_enabled = [GR_POSITIVE1, GR_POSITIVE2, GR_POSITIVE4, GR_POSITIVE8, GR_POSITIVE16]}
+  Expect Symbol With Score  GR_POSITIVE1  1
+  Expect Symbol With Score  GR_POSITIVE2  2
+  Expect Symbol With Score  GR_POSITIVE4  4
+  Expect Symbol With Score  GR_POSITIVE8  3
+  Expect Symbol With Score  GR_POSITIVE16  0
+
+Group Score Negative
+  Scan File  ${MESSAGE}  Settings={symbols_enabled = [GR_NEGATIVE1, GR_NEGATIVE2, GR_NEGATIVE4, GR_NEGATIVE8]}
+  Expect Symbol With Score  GR_NEGATIVE1  -1
+  Expect Symbol With Score  GR_NEGATIVE2  -2
+  Expect Symbol With Score  GR_NEGATIVE4  -4
+  Expect Symbol With Score  GR_NEGATIVE8  -3
+
+Group Score Mix 1
+  Scan File  ${MESSAGE}  Settings={symbols_enabled = [GR_POSITIVE1, GR_POSITIVE2, GR_POSITIVE4, GR_POSITIVE8, GR_NEGATIVE1, GR_NEGATIVE2, GR_NEGATIVE4, GR_NEGATIVE8]}
+  Expect Symbol With Score  GR_POSITIVE1  1
+  Expect Symbol With Score  GR_POSITIVE2  2
+  Expect Symbol With Score  GR_POSITIVE4  4
+  Expect Symbol With Score  GR_POSITIVE8  3
+  Expect Symbol With Score  GR_NEGATIVE1  -1
+  Expect Symbol With Score  GR_NEGATIVE2  -2
+  Expect Symbol With Score  GR_NEGATIVE4  -4
+  Expect Symbol With Score  GR_NEGATIVE8  -8
+
+Group Score Mix 2
+  Scan File  ${MESSAGE}  Settings={symbols_enabled = [GR_POSITIVE1, GR_POSITIVE2, GR_POSITIVE4, GR_POSITIVE8, GR_NEGATIVE16]}
+  Expect Symbol With Score  GR_POSITIVE1  1
+  Expect Symbol With Score  GR_POSITIVE2  2
+  Expect Symbol With Score  GR_POSITIVE4  4
+  Expect Symbol With Score  GR_POSITIVE8  3
+  Expect Symbol With Score  GR_NEGATIVE16  -16
+
+Group Score Mix 3
+  Scan File  ${MESSAGE}  Settings={symbols_enabled = [GR_POSITIVE1, GR_NEGATIVE16]}
+  Expect Symbol With Score  GR_POSITIVE1  1
+  Expect Symbol With Score  GR_NEGATIVE16  -11
+
+Group Score Mix 4
+  Scan File  ${MESSAGE}  Settings={symbols_enabled = [GR_POSITIVE16, GR_NEGATIVE16]}
+  Expect Symbol With Score  GR_POSITIVE16  10
+  Expect Symbol With Score  GR_NEGATIVE16  -16
\ No newline at end of file
diff --git a/test/functional/configs/merged-local.conf b/test/functional/configs/merged-local.conf
index 2d914b075..2aef274c2 100644
--- a/test/functional/configs/merged-local.conf
+++ b/test/functional/configs/merged-local.conf
@@ -965,6 +965,45 @@ symbols {
   }
 }
 
+group "test" {
+  max_score = 10;
+  min_score = -10;
+
+  symbols = {
+    "GR_POSITIVE1" = {
+      score = 1.0;
+    },
+    "GR_POSITIVE2" = {
+      score = 2.0;
+    },
+    "GR_POSITIVE4" = {
+      score = 4.0;
+    },
+    "GR_POSITIVE8" = {
+      score = 8.0;
+    },
+    "GR_POSITIVE16" = {
+      score = 16.0;
+    },
+
+    "GR_NEGATIVE1" = {
+      score = -1.0;
+    },
+    "GR_NEGATIVE2" = {
+      score = -2.0;
+    },
+    "GR_NEGATIVE4" = {
+      score = -4.0;
+    },
+    "GR_NEGATIVE8" = {
+      score = -8.0;
+    },
+    "GR_NEGATIVE16" = {
+      score = -16.0;
+    },
+  }
+}
+
 worker "controller" {
     bind_socket = "{= env.LOCAL_ADDR =}:{= env.PORT_CONTROLLER =}";
     keypair {
diff --git a/test/functional/configs/merged.conf b/test/functional/configs/merged.conf
index 0ee224ceb..06a34308d 100644
--- a/test/functional/configs/merged.conf
+++ b/test/functional/configs/merged.conf
@@ -11,6 +11,7 @@ lua = "{= env.TESTDIR =}/lua/recipients.lua"
 lua = "{= env.TESTDIR =}/lua/remove_result.lua"
 lua = "{= env.TESTDIR =}/lua/tlds.lua"
 lua = "{= env.TESTDIR =}/lua/inject_url.lua"
+lua = "{= env.TESTDIR =}/lua/limits.lua"
 
 # 104_get_from
 lua = "{= env.TESTDIR =}/lua/get_from.lua"
diff --git a/test/functional/lua/deps.lua b/test/functional/lua/deps.lua
index 6171db699..b78d3abb7 100644
--- a/test/functional/lua/deps.lua
+++ b/test/functional/lua/deps.lua
@@ -24,7 +24,7 @@ rspamd_config:register_virtual_symbol('TOP', 1.0, id)
 rspamd_config:register_symbol('DEP1', 1.0, cb_dep1)
 rspamd_config:register_dependency('DEP1', 'TOP')
 
-for i = 2,10 do
+for i = 2, 10 do
   rspamd_config:register_symbol('DEP' .. tostring(i), 1.0, cb_gen(i - 1))
   rspamd_config:register_dependency('DEP' .. tostring(i), 'DEP' .. tostring(i - 1))
 end
diff --git a/test/functional/lua/limits.lua b/test/functional/lua/limits.lua
new file mode 100644
index 000000000..52fb47fb9
--- /dev/null
+++ b/test/functional/lua/limits.lua
@@ -0,0 +1,22 @@
+local true_cb_gen = function()
+  return function()
+    return true
+  end
+end
+
+local test_weights = { 1, 2, 4, 8, 16 }
+for _, i in ipairs(test_weights) do
+  rspamd_config:register_symbol('GR_POSITIVE' .. tostring(i), 1.0, true_cb_gen())
+
+  if i > 1 then
+    rspamd_config:register_dependency('GR_POSITIVE' .. tostring(i), 'GR_POSITIVE' .. tostring(i / 2))
+  end
+
+  rspamd_config:register_symbol('GR_NEGATIVE' .. tostring(i), 1.0, true_cb_gen())
+
+  if i > 1 then
+    rspamd_config:register_dependency('GR_NEGATIVE' .. tostring(i), 'GR_NEGATIVE' .. tostring(i / 2))
+  end
+end
+
+rspamd_config:register_dependency('GR_NEGATIVE1', 'GR_POSITIVE16')
\ No newline at end of file