fix(alert): prevent divide-by-zero panic and unbounded growth on zero-duration rule

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.
This commit is contained in:
naiba
2026-06-05 02:42:49 +00:00
parent 36240f5888
commit 0759f5c801
3 changed files with 61 additions and 4 deletions
+7
View File
@@ -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
}
+45
View File
@@ -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)
}
}