From 0759f5c80147cb9fbcd6a98b3361eeade1e6af15 Mon Sep 17 00:00:00 2001
From: naiba <hi@nai.ba>
Date: Fri, 5 Jun 2026 02:42:49 +0000
Subject: [PATCH] fix(alert): prevent divide-by-zero panic and unbounded growth
 on zero-duration rule
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A general alert rule with Duration:0 (accepted by the API, no minimum
validation) drove fail*100/total with total==0, panicking with an integer
divide-by-zero inside checkStatus, which has no recover and would take down the
whole alert goroutine — a config-reachable DoS by any user able to create alert
rules. Skip duration<=0 rules in Check via continue (not boundCheck, which would
pollute hasPassedRule and skip sibling valid rules). Also trim the sample slice
when max<=0 in alertsentinel, otherwise it appended every tick without ever
trimming. Adds regression tests for zero-only and mixed rule sets.
---
 model/alertrule.go                 |  7 +++++
 model/alertrule_test.go            | 45 ++++++++++++++++++++++++++++++
 service/singleton/alertsentinel.go | 13 ++++++---
 3 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/model/alertrule.go b/model/alertrule.go
index 82bf2bbd..29819900 100644
--- a/model/alertrule.go
+++ b/model/alertrule.go
@@ -166,6 +166,13 @@ func (r *AlertRule) Check(points [][]bool) (int, bool) {
 			continue
 		} else {
 			// 常规报警
+			// duration<=0 是无意义的规则（持续 0 秒）：直接跳过该规则，
+			// 既不污染 hasPassedRule（否则会连带跳过同一 alert 里其它有效
+			// 规则），也避免下方 fail*100/total 在 total=0 时整数除零 panic
+			// —— checkStatus 无 recover，一次 panic 会拖垮整个告警 goroutine。
+			if duration <= 0 {
+				continue
+			}
 			if hasPassedRule = boundCheck(len(points), duration, hasPassedRule); hasPassedRule {
 				continue
 			}
diff --git a/model/alertrule_test.go b/model/alertrule_test.go
index 8ec971b7..5b0c1b35 100644
--- a/model/alertrule_test.go
+++ b/model/alertrule_test.go
@@ -315,3 +315,48 @@ func assertEq(t *testing.T, msg string, exp, act any) {
 		t.Fatalf("failed to test for %s. exp=[%v] but act=[%v]", msg, exp, act)
 	}
 }
+
+// TestAlertRule_ZeroDurationGeneralRule guards against a config-reachable DoS:
+// a general rule with Duration:0 (the API validates Duration as "optional" with
+// no minimum) previously hit fail*100/total with total==0, panicking with an
+// integer divide-by-zero inside checkStatus — which has no recover and would
+// take down the whole alert goroutine. boundCheck now treats duration<=0 as a
+// passed (no-op) rule, so Check must return without panicking.
+func TestAlertRule_ZeroDurationGeneralRule(t *testing.T) {
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatalf("Check panicked on a Duration:0 general rule (config-reachable DoS): %v", r)
+		}
+	}()
+
+	rule := &AlertRule{
+		Rules: []*Rule{{Type: "cpu", Duration: 0}},
+	}
+	// The only contract here is "do not panic". A zero-duration rule is skipped,
+	// so it contributes nothing to the verdict and max stays 0.
+	maxD, _ := rule.Check([][]bool{{true}, {false}})
+	if maxD != 0 {
+		t.Fatalf("a skipped Duration:0 rule must not contribute to max, got %d", maxD)
+	}
+}
+
+// Mixing a valid rule with a zero-duration rule must also be safe: the zero
+// rule is skipped, the real rule still drives the verdict.
+func TestAlertRule_ZeroDurationMixedWithValidRule(t *testing.T) {
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatalf("Check panicked on a mixed zero/valid rule set: %v", r)
+		}
+	}()
+
+	rule := &AlertRule{
+		Rules: []*Rule{
+			{Type: "cpu", Duration: 0},
+			{Type: "cpu", Duration: 3},
+		},
+	}
+	maxD, _ := rule.Check([][]bool{{true, false}, {true, false}, {true, false}})
+	if maxD != 3 {
+		t.Fatalf("the valid Duration:3 rule must still set max=3, got %d", maxD)
+	}
+}
diff --git a/service/singleton/alertsentinel.go b/service/singleton/alertsentinel.go
index e2c1d0de..6af6c6fe 100644
--- a/service/singleton/alertsentinel.go
+++ b/service/singleton/alertsentinel.go
@@ -184,10 +184,15 @@ func checkStatus() {
 				}
 				alertsPrevState[alert.ID][server.ID] = _RuleCheckPass
 			}
-			// 清理旧数据
-			if max > 0 && max < len(alertsStore[alert.ID][server.ID]) {
-				index := len(alertsStore[alert.ID][server.ID]) - max
-				alertsStore[alert.ID][server.ID] = alertsStore[alert.ID][server.ID][index:]
+			// 清理旧数据：只需保留最近 max 个采样点（各规则 Duration 的最大值）。
+			// max==0 表示该 alert 没有任何有效规则会回看历史（例如全部
+			// Duration<=0 被跳过），此时历史采样无用，必须清空——否则每个
+			// tick 都 append 而永不裁剪，切片会无限增长成内存泄漏。
+			samples := alertsStore[alert.ID][server.ID]
+			if max <= 0 {
+				alertsStore[alert.ID][server.ID] = samples[:0]
+			} else if max < len(samples) {
+				alertsStore[alert.ID][server.ID] = samples[len(samples)-max:]
 			}
 		}
 	}