From 809436e11156d46230c69c3028fde594291aa20c Mon Sep 17 00:00:00 2001 From: BaaRai <130491490+BaaRai@users.noreply.github.com> Date: Tue, 10 Nov 2026 16:05:31 +0100 Subject: [PATCH] notify: make the summary and asset limits configurable The IM renderers cut the summary of a single push at 120 characters and listed at most 3 assets, as fixed constants. Both are now global settings in the settings table, read by the delivery engine when it renders: notify_summary_max_runes 0-4000, default 120 notify_assets_max_shown 0-50, default 3 0 means no truncation and an unset key keeps 120 / 3. PUT /api/settings answers 400 for a value outside the bounds. The limits travel on notify.Message, so the notify package still does not touch the database. With no truncation, nothing bounded a single item in a Feishu card, and Feishu rejects an oversized request as a whole. feishuCard now truncates an item's text as the final backstop, the role TruncateBytes plays in markdownBody for DingTalk and WeCom. feishuMaxCardBytes goes from 24000 to 16000: Feishu custom bots cap the request body at 20 KB, which the old value exceeded. Refs #168. --- notify/event.go | 24 +++++ notify/feishu.go | 29 +++--- notify/html.go | 12 +-- notify/limits_test.go | 183 ++++++++++++++++++++++++++++++++++++++ notify/markdown.go | 32 +++---- notify/pack_test.go | 2 +- notify/telegram.go | 13 +-- server/notifier.go | 48 ++++++++-- server/notify_api.go | 2 + server/notify_api_test.go | 129 +++++++++++++++++++++++++++ server/server.go | 27 ++++++ 11 files changed, 455 insertions(+), 46 deletions(-) create mode 100644 notify/limits_test.go diff --git a/notify/event.go b/notify/event.go index 3c339fd..66645dc 100644 --- a/notify/event.go +++ b/notify/event.go @@ -79,4 +79,28 @@ type Message struct { // HomeURL is the platform dashboard address (global public_base_url); empty // means no dashboard entry is included. HomeURL string + // Limits are the truncation limits to render with; nil means the defaults. + // A pointer because the zero value is meaningful (no truncation). + Limits *Limits +} + +// Limits are the render-time truncation limits of a message, configured +// globally (see the server's notify_summary_max_runes / notify_assets_max_shown +// settings). In both fields 0 means no truncation; each channel's own length +// cap remains the final guard. +type Limits struct { + // SummaryRunes is how many characters the summary of a single push is + // compressed to. + SummaryRunes int + // AssetsShown is the maximum number of assets listed per vulnerability. + AssetsShown int +} + +// limits returns the limits to render with, falling back to the defaults when +// none were passed in. +func (m Message) limits() Limits { + if m.Limits == nil { + return Limits{SummaryRunes: DefaultMaxSummaryRunes, AssetsShown: DefaultMaxAssetsShown} + } + return *m.Limits } diff --git a/notify/feishu.go b/notify/feishu.go index 52aea8a..1e1bc96 100644 --- a/notify/feishu.go +++ b/notify/feishu.go @@ -121,15 +121,20 @@ func feishuSeverityTemplate(severity string) string { } } -// feishuMaxCardBytes is a conservative cap on card content. Feishu enforces a -// size limit on cards and rejects the whole message when exceeded; pick a value -// clearly below the official limit to also cover JSON wrapping overhead. -const feishuMaxCardBytes = 24000 +// feishuMaxCardBytes is a conservative cap on card content. Feishu custom bots +// reject a request body over 20 KB as a whole (not truncated); pick a value +// clearly below that limit to also cover JSON wrapping overhead. +const feishuMaxCardBytes = 16000 // feishuCard builds the interactive card and returns it along with the **number // of items actually written**. kept serves the same purpose as in markdownBody: // only items that actually made it into the card should be marked as sent. func feishuCard(m Message) (map[string]any, int) { + lim := m.limits() + // budget is the final truncation backstop for one item's text, the same role + // TruncateBytes plays in markdownBody. With the summary or asset limit set to + // 0 an item has no other bound, and an oversized card loses the whole message. + budget := feishuMaxCardBytes - markdownReservedBytes elements := []any{} kept := 0 if m.Batch { @@ -137,12 +142,12 @@ func feishuCard(m Message) (map[string]any, int) { // "the remaining N items will continue in the next message", and N must // come from the number of items that actually fit. kept = packItemCount(m.Items, feishuMaxCardBytes, markdownReservedBytes, "", byteSize, func(it Item, idx int) string { - return feishuBatchLine(it, idx+1) + return feishuBatchLine(it, idx+1, lim) }) items := m.Items[:kept] elements = append(elements, feishuMarkdownDiv(markdownBatchIntro(m, items, len(m.Items)))) for i, it := range items { - elements = append(elements, feishuMarkdownDiv(feishuBatchLine(it, i+1))) + elements = append(elements, feishuMarkdownDiv(TruncateBytes(feishuBatchLine(it, i+1, lim), budget))) } if m.HomeURL != "" { elements = append(elements, feishuButton("View all on the platform", m.HomeURL)) @@ -150,7 +155,7 @@ func feishuCard(m Message) (map[string]any, int) { } else if len(m.Items) > 0 { kept = 1 it := m.Items[0] - elements = append(elements, feishuMarkdownDiv(feishuItemLines(it))) + elements = append(elements, feishuMarkdownDiv(TruncateBytes(feishuItemLines(it, lim), budget))) if it.DetailURL != "" { elements = append(elements, feishuButton("View details", it.DetailURL)) } @@ -189,7 +194,7 @@ func feishuButton(label, url string) map[string]any { // emphasis, so every field coming from outside goes through markdownText // (single-lining + escaping) — otherwise a single vulnerability title could turn // into a clickable external link inside Feishu. -func feishuItemLines(it Item) string { +func feishuItemLines(it Item, lim Limits) string { out := fmt.Sprintf("**%s · %s**", SeverityLabel(it.Severity), markdownText(it.Title(), 0)) if it.IsStatusChange() { out += fmt.Sprintf("\n**Status change**: %s → %s", @@ -198,11 +203,11 @@ func feishuItemLines(it Item) string { if it.VulnClass != "" && it.VulnClass != it.Title() { out += fmt.Sprintf("\n**Type**: %s", markdownText(it.VulnClass, 0)) } - if a := assetLine(it.Assets, maxAssetsShown); a != "" { + if a := assetLine(it.Assets, lim.AssetsShown); a != "" { out += fmt.Sprintf("\n**Assets**: %s", markdownText(a, 0)) } if it.Summary != "" { - if s := markdownText(it.Summary, maxSummaryRunes); s != "" { + if s := markdownText(it.Summary, lim.SummaryRunes); s != "" { out += fmt.Sprintf("\n**Summary**: %s", s) } } @@ -210,9 +215,9 @@ func feishuItemLines(it Item) string { } // feishuBatchLine renders one entry of a digest card. -func feishuBatchLine(it Item, index int) string { +func feishuBatchLine(it Item, index int, lim Limits) string { line := fmt.Sprintf("**%d. %s · %s**", index, SeverityLabel(it.Severity), markdownText(it.Title(), 0)) - if a := assetLine(it.Assets, maxAssetsShown); a != "" { + if a := assetLine(it.Assets, lim.AssetsShown); a != "" { line += " — " + markdownText(a, 0) } return line diff --git a/notify/html.go b/notify/html.go index f9a7808..8a1930c 100644 --- a/notify/html.go +++ b/notify/html.go @@ -39,10 +39,10 @@ func htmlBody(m Message, maxRunes int) string { if m.Batch { b.WriteString(htmlBatchIntro(m)) for _, it := range m.Items { - b.WriteString(htmlItem(it, false)) + b.WriteString(htmlItem(it, false, m.limits())) } } else if len(m.Items) > 0 { - b.WriteString(htmlItem(m.Items[0], true)) + b.WriteString(htmlItem(m.Items[0], true, m.limits())) } if m.HomeURL != "" { fmt.Fprintf(&b, `

View all on the platform

`, htmlEscapeAttr(m.HomeURL)) @@ -79,7 +79,7 @@ func htmlBatchIntro(m Message) string { // htmlItem renders a single vulnerability. full=true includes the summary and // backlink (single push); false compresses it into one line (digest list). -func htmlItem(it Item, full bool) string { +func htmlItem(it Item, full bool, lim Limits) string { color := htmlSeverityColor(it.Severity) var b strings.Builder if full { @@ -92,7 +92,7 @@ func htmlItem(it Item, full bool) string { if !full { var extras []string - if a := assetLine(it.Assets, maxAssetsShown); a != "" { + if a := assetLine(it.Assets, lim.AssetsShown); a != "" { extras = append(extras, htmlEscape(a)) } if it.Summary != "" { @@ -112,10 +112,10 @@ func htmlItem(it Item, full bool) string { if it.VulnClass != "" && it.VulnClass != it.Title() { fmt.Fprintf(&b, `
Type: %s
`, htmlEscape(it.VulnClass)) } - if a := assetLine(it.Assets, maxAssetsShown); a != "" { + if a := assetLine(it.Assets, lim.AssetsShown); a != "" { fmt.Fprintf(&b, `
Assets: %s
`, htmlEscape(a)) } - if s := OneLine(it.Summary, maxSummaryRunes); s != "" { + if s := OneLine(it.Summary, lim.SummaryRunes); s != "" { fmt.Fprintf(&b, `
Summary: %s
`, htmlEscape(s)) } if it.DetailURL != "" { diff --git a/notify/limits_test.go b/notify/limits_test.go new file mode 100644 index 0000000..19b9697 --- /dev/null +++ b/notify/limits_test.go @@ -0,0 +1,183 @@ +package notify + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "unicode/utf8" +) + +// This file covers the configurable render limits (Message.Limits). The summary +// length and the number of assets listed used to be the fixed constants 120 / 3; +// they now come from the caller, with 0 meaning "no truncation" and nil meaning +// "the old defaults". + +// limitsItem returns an item whose summary (200 characters) and asset list (8) +// both exceed the defaults. It contains no markdown/HTML metacharacters, so +// every renderer emits the text verbatim and one set of assertions fits all. +func limitsItem() Item { + assets := make([]string, 8) + for i := range assets { + assets[i] = "host" + itoa(i+1) + } + return Item{Severity: "high", Name: "title", Summary: strings.Repeat("Z", 200), Assets: assets} +} + +// feishuContent returns the lark_md text of the card's element idx. +func feishuContent(t *testing.T, m Message, idx int) string { + t.Helper() + card, _ := feishuCard(m) + elements, _ := card["elements"].([]any) + if idx >= len(elements) { + t.Fatalf("card has %d elements, cannot get element %d", len(elements), idx) + } + div, _ := elements[idx].(map[string]any) + text, _ := div["text"].(map[string]any) + content, _ := text["content"].(string) + return content +} + +// limitsRenderings returns the body of every channel family that applies the +// limits. For a digest, the Feishu entry is the first item line (element 0 is +// the intro). +func limitsRenderings(t *testing.T, m Message) map[string]string { + t.Helper() + md, _ := markdownBody(m, 0) + tg, _ := telegramHTML(m) + feishuIdx := 0 + if m.Batch { + feishuIdx = 1 + } + return map[string]string{ + "markdown": md, + "feishu": feishuContent(t, m, feishuIdx), + "telegram": tg, + "email": htmlBody(m, 0), + } +} + +func TestMessageLimitsApplyToSinglePush(t *testing.T) { + cases := []struct { + name string + lim *Limits + must []string + wrong []string + }{ + { + name: "unset keeps the 120 / 3 defaults", + lim: nil, + must: []string{strings.Repeat("Z", 119) + ellipsis, "host1, host2, host3 and 5 more"}, + wrong: []string{strings.Repeat("Z", 120), "host4"}, + }, + { + name: "zero means no truncation", + lim: &Limits{}, + must: []string{strings.Repeat("Z", 200), "host7, host8"}, + wrong: []string{ellipsis, " more"}, + }, + { + name: "custom values", + lim: &Limits{SummaryRunes: 10, AssetsShown: 1}, + must: []string{strings.Repeat("Z", 9) + ellipsis, "host1 and 7 more"}, + wrong: []string{strings.Repeat("Z", 10), "host2"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + m := Message{Items: []Item{limitsItem()}, Limits: tc.lim} + for channel, got := range limitsRenderings(t, m) { + for _, want := range tc.must { + if !strings.Contains(got, want) { + t.Errorf("%s: missing %q:\n%s", channel, want, got) + } + } + for _, bad := range tc.wrong { + if strings.Contains(got, bad) { + t.Errorf("%s: %q should not appear:\n%s", channel, bad, got) + } + } + } + }) + } +} + +// TestMessageLimitsApplyToDigestAssets covers the digest lines, which list the +// assets too. The digest summary is not governed by Limits.SummaryRunes: it +// stays at its own fixed 60 characters (markdown and email), a digest being a +// list of one-liners. +func TestMessageLimitsApplyToDigestAssets(t *testing.T) { + m := Message{Batch: true, Items: []Item{limitsItem()}} + for channel, got := range limitsRenderings(t, m) { + if !strings.Contains(got, "host1, host2, host3 and 5 more") { + t.Errorf("%s: unset should keep 3 assets:\n%s", channel, got) + } + } + m.Limits = &Limits{} + for channel, got := range limitsRenderings(t, m) { + if !strings.Contains(got, "host7, host8") || strings.Contains(got, " more") { + t.Errorf("%s: 0 should list every asset:\n%s", channel, got) + } + if strings.Contains(got, strings.Repeat("Z", 60)) { + t.Errorf("%s: the digest summary should stay within 60 characters:\n%s", channel, got) + } + } +} + +// TestFeishuUntruncatedItemStaysWithinBodyLimit is the guard that goes with +// "0 = no truncation". Feishu custom bots reject a request body over 20 KB as a +// whole, so an unbounded summary or asset list would not arrive shortened — it +// would not arrive at all. The body measured here is the real one sent on the +// wire, signature included. +func TestFeishuUntruncatedItemStaysWithinBodyLimit(t *testing.T) { + const feishuBodyLimit = 20000 + + huge := limitsItem() + huge.DetailURL = "https://artex.local/function/findings/detail?id=42" + // 36000 bytes of CJK: also checks that the cut lands on a character boundary. + huge.Summary = strings.Repeat("漏洞摘要", 3000) + huge.Assets = nil + for i := 0; i < 1500; i++ { + huge.Assets = append(huge.Assets, "host"+itoa(i+1)+".example.com") + } + + cases := []struct { + name string + m Message + idx int + }{ + {"single push", Message{Items: []Item{huge}, Limits: &Limits{}}, 0}, + {"digest whose first item is oversized", Message{ + Batch: true, WindowMinutes: 30, HomeURL: "https://artex.local/function/findings", + Items: []Item{huge, huge}, Limits: &Limits{}, + }, 1}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + size := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + raw, _ := io.ReadAll(r.Body) + size = len(raw) + _, _ = io.WriteString(w, `{"code":0,"msg":"success"}`) + })) + defer srv.Close() + + kept, err := (feishuChannel{}).Send(context.Background(), map[string]any{"webhook": srv.URL, "secret": "SECtest123"}, tc.m) + if err != nil { + t.Fatalf("delivery failed: %v", err) + } + if kept != 1 { + t.Fatalf("exactly the first item should be reported as delivered, got %d", kept) + } + if size == 0 || size > feishuBodyLimit { + t.Fatalf("request body of %d bytes, want 1..%d", size, feishuBodyLimit) + } + content := feishuContent(t, tc.m, tc.idx) + if !utf8.ValidString(content) || !strings.HasSuffix(content, ellipsis) { + t.Fatalf("the item should be cut on a character boundary and end with an ellipsis, got tail %q", content[len(content)-12:]) + } + }) + } +} diff --git a/notify/markdown.go b/notify/markdown.go index a2698cc..6a0af50 100644 --- a/notify/markdown.go +++ b/notify/markdown.go @@ -9,16 +9,16 @@ import ( // (DingTalk, WeCom). Feishu uses card JSON, Telegram uses HTML, and email uses // HTML; each renders in its own adapter. -// maxAssetsShown is the maximum number of assets listed in a message. A -// vulnerability can be anchored to dozens of assets; listing them all would blow -// up the message with no informational value — nobody reads the 4th domain -// onwards in an IM. -const maxAssetsShown = 3 +// DefaultMaxAssetsShown is the default maximum number of assets listed in a +// message (Limits.AssetsShown). A vulnerability can be anchored to dozens of +// assets; listing them all would blow up the message with no informational +// value — nobody reads the 4th domain onwards in an IM. +const DefaultMaxAssetsShown = 3 -// maxSummaryRunes is how many characters the summary is compressed to. An IM -// message is a "go look at the details" hint, not the report itself; the full -// content lives on the platform. -const maxSummaryRunes = 120 +// DefaultMaxSummaryRunes is the default number of characters the summary is +// compressed to (Limits.SummaryRunes). An IM message is a "go look at the +// details" hint, not the report itself; the full content lives on the platform. +const DefaultMaxSummaryRunes = 120 // markdownReservedBytes is reserved for the message header (digest line + // severity distribution + possible truncation notice) and footer (platform @@ -114,7 +114,7 @@ func markdownBody(m Message, maxBytes int) (string, int) { return "", 0 } var b strings.Builder - writeItem(&b, m.Items[0], "", true) + writeItem(&b, m.Items[0], "", true, m.limits()) // A single message is sent even if oversized (with final truncation as // the backstop): partial information about one vulnerability is better // than sending nothing at all. @@ -127,7 +127,7 @@ func markdownBody(m Message, maxBytes int) (string, int) { } kept := packItemCount(m.Items, maxBytes, markdownReservedBytes, footer, byteSize, func(it Item, idx int) string { var b strings.Builder - writeItem(&b, it, fmt.Sprintf("%d. ", idx+1), false) + writeItem(&b, it, fmt.Sprintf("%d. ", idx+1), false, m.limits()) return b.String() }) @@ -135,7 +135,7 @@ func markdownBody(m Message, maxBytes int) (string, int) { var b strings.Builder b.WriteString(markdownBatchIntro(m, items, len(m.Items))) for i, it := range items { - writeItem(&b, it, fmt.Sprintf("%d. ", i+1), false) + writeItem(&b, it, fmt.Sprintf("%d. ", i+1), false, m.limits()) } b.WriteString(footer) return TruncateBytes(b.String(), maxBytes), kept @@ -191,13 +191,13 @@ func markdownBatchIntro(m Message, items []Item, total int) string { // markdownText: single-lining + escaping. The backlink is built from the // admin-configured public_base_url, is not untrusted content, and must be a // clickable link, so it is emitted as-is. -func writeItem(b *strings.Builder, it Item, prefix string, single bool) { +func writeItem(b *strings.Builder, it Item, prefix string, single bool, lim Limits) { line := fmt.Sprintf("%s**%s · %s**", prefix, SeverityLabel(it.Severity), markdownText(it.Title(), 0)) if !single { // Digest mode: single-line presentation, with assets and summary // compressed and appended. var extras []string - if a := assetLine(it.Assets, maxAssetsShown); a != "" { + if a := assetLine(it.Assets, lim.AssetsShown); a != "" { extras = append(extras, markdownText(a, 0)) } if it.Summary != "" { @@ -217,11 +217,11 @@ func writeItem(b *strings.Builder, it Item, prefix string, single bool) { if it.VulnClass != "" && it.VulnClass != it.Title() { fmt.Fprintf(b, "**Type**: %s\n", markdownText(it.VulnClass, 0)) } - if a := assetLine(it.Assets, maxAssetsShown); a != "" { + if a := assetLine(it.Assets, lim.AssetsShown); a != "" { fmt.Fprintf(b, "**Assets**: %s\n", markdownText(a, 0)) } if it.Summary != "" { - if s := markdownText(it.Summary, maxSummaryRunes); s != "" { + if s := markdownText(it.Summary, lim.SummaryRunes); s != "" { fmt.Fprintf(b, "**Summary**: %s\n", s) } } diff --git a/notify/pack_test.go b/notify/pack_test.go index 2ced915..febb94c 100644 --- a/notify/pack_test.go +++ b/notify/pack_test.go @@ -178,7 +178,7 @@ func TestMarkdownEscapesUntrustedContent(t *testing.T) { // writeItem in single mode is the rendering path shared by all three // markdown channels. var b strings.Builder - writeItem(&b, tc.item, "", true) + writeItem(&b, tc.item, "", true, m.limits()) got := b.String() for _, want := range tc.must { if !strings.Contains(got, want) { diff --git a/notify/telegram.go b/notify/telegram.go index cb6b289..1595106 100644 --- a/notify/telegram.go +++ b/notify/telegram.go @@ -115,6 +115,7 @@ func telegramEndpoint(cfg map[string]any) (string, error) { // telegramHTML renders the HTML body and returns it together with the number of // items actually written (see Channel.Send). func telegramHTML(m Message) (string, int) { + lim := m.limits() var b strings.Builder b.WriteString("" + telegramEscape(markdownTitle(m)) + "\n") if m.Batch { @@ -125,13 +126,13 @@ func telegramHTML(m Message) (string, int) { footer = fmt.Sprintf("\n\nView all on the platform", telegramEscapeAttr(m.HomeURL)) } kept := packItemCount(m.Items, telegramTextLimit, telegramReservedRunes, footer, runeSize, func(it Item, idx int) string { - return telegramBatchLine(it, idx+1) + return telegramBatchLine(it, idx+1, lim) }) items := m.Items[:kept] b.Reset() b.WriteString("" + telegramEscape(telegramBatchTitle(m, items, len(m.Items))) + "") for i, it := range items { - b.WriteString("\n" + telegramEscape(telegramBatchLine(it, i+1))) + b.WriteString("\n" + telegramEscape(telegramBatchLine(it, i+1, lim))) } b.WriteString(footer) return TruncateHTML(b.String(), telegramTextLimit), kept @@ -147,10 +148,10 @@ func telegramHTML(m Message) (string, int) { if it.VulnClass != "" && it.VulnClass != it.Title() { b.WriteString("\nType: " + telegramEscape(it.VulnClass)) } - if a := assetLine(it.Assets, maxAssetsShown); a != "" { + if a := assetLine(it.Assets, lim.AssetsShown); a != "" { b.WriteString("\nAssets: " + telegramEscape(a)) } - if s := OneLine(it.Summary, maxSummaryRunes); s != "" { + if s := OneLine(it.Summary, lim.SummaryRunes); s != "" { b.WriteString("\nSummary: " + telegramEscape(s)) } if it.DetailURL != "" { @@ -165,8 +166,8 @@ const telegramReservedRunes = 160 // telegramBatchLine renders one entry of a digest (unescaped; the caller escapes // it uniformly). -func telegramBatchLine(it Item, idx int) string { - if a := assetLine(it.Assets, maxAssetsShown); a != "" { +func telegramBatchLine(it Item, idx int, lim Limits) string { + if a := assetLine(it.Assets, lim.AssetsShown); a != "" { return fmt.Sprintf("%d. %s · %s — %s", idx, SeverityLabel(it.Severity), it.Title(), a) } return fmt.Sprintf("%d. %s · %s", idx, SeverityLabel(it.Severity), it.Title()) diff --git a/server/notifier.go b/server/notifier.go index e0fa373..ae00a93 100644 --- a/server/notifier.go +++ b/server/notifier.go @@ -5,6 +5,8 @@ import ( "encoding/json" "fmt" "log" + "strconv" + "strings" "sync" "time" @@ -25,6 +27,12 @@ const ( settingNotifyPublicBaseURL = "notify_public_base_url" // settingNotifyDigestMinutes is the digest-mode period (minutes). settingNotifyDigestMinutes = "notify_digest_interval_min" + // settingNotifySummaryMaxRunes is how many characters the summary of a single + // push is cut to, and settingNotifyAssetsMaxShown how many asset names a + // message lists. 0 means no truncation; unset means the notify package + // defaults (120 / 3). + settingNotifySummaryMaxRunes = "notify_summary_max_runes" + settingNotifyAssetsMaxShown = "notify_assets_max_shown" ) const ( @@ -43,6 +51,10 @@ const ( notifyFanOutPerTick = 200 // notifyDefaultDigestMinutes is the default digest period. notifyDefaultDigestMinutes = 30 + // notifySummaryMaxRunesCap / notifyAssetsMaxShownCap are the upper bounds the + // settings API accepts for the two render limits. + notifySummaryMaxRunesCap = 4000 + notifyAssetsMaxShownCap = 50 // notifyUnlimitedBurstPerTick is the per-tick delivery cap when a channel has // no rate limit configured. It exists to prevent "one channel configured as // unlimited + a single scan finding thousands of vulnerabilities" from dragging @@ -223,8 +235,9 @@ func (n *Notifier) stepRealtime(ctx context.Context, ch *db.NotificationChannel, _ = n.pg.FailDeliveries(ctx, deliveryIDs(deliveries), fmt.Sprintf("channel type %q is not registered", ch.Kind)) return } + limits := notifyLimits(n.pg) for _, dl := range deliveries { - msg, err := n.renderSingle(ctx, dl, baseURL) + msg, err := n.renderSingle(ctx, dl, baseURL, limits) if err != nil { // A render failure is a local data problem; retrying will not help. _ = n.pg.FailDeliveries(ctx, []int64{dl.ID}, err.Error()) @@ -259,7 +272,7 @@ func (n *Notifier) stepDigest(ctx context.Context, ch *db.NotificationChannel, a _ = n.pg.FailDeliveries(ctx, deliveryIDs(deliveries), fmt.Sprintf("channel type %q is not registered", ch.Kind)) return } - msg, included, err := n.renderBatch(ctx, deliveries, baseURL, int(window.Minutes())) + msg, included, err := n.renderBatch(ctx, deliveries, baseURL, int(window.Minutes()), notifyLimits(n.pg)) if err != nil { _ = n.pg.FailDeliveries(ctx, deliveryIDs(deliveries), err.Error()) return @@ -423,7 +436,7 @@ func (n *Notifier) adapt(ch *db.NotificationChannel) (notify.Channel, map[string } // renderSingle renders a single-vulnerability message. -func (n *Notifier) renderSingle(ctx context.Context, dl *db.NotificationDelivery, baseURL string) (notify.Message, error) { +func (n *Notifier) renderSingle(ctx context.Context, dl *db.NotificationDelivery, baseURL string, limits notify.Limits) (notify.Message, error) { snap, err := parseSnapshot(dl) if err != nil { return notify.Message{}, err @@ -432,7 +445,7 @@ func (n *Notifier) renderSingle(ctx context.Context, dl *db.NotificationDelivery if err != nil { return notify.Message{}, err } - return notify.Message{Items: []notify.Item{item}, HomeURL: baseURL}, nil + return notify.Message{Items: []notify.Item{item}, HomeURL: baseURL, Limits: &limits}, nil } // renderBatch renders a digest message. Snapshots are parsed one by one — a @@ -446,7 +459,7 @@ func (n *Notifier) renderSingle(ctx context.Context, dl *db.NotificationDelivery // entry that should have failed would be marked sent, and a good entry would be // misjudged as undelivered. The broken ones are marked failed explicitly by the // caller, see stepDigest. -func (n *Notifier) renderBatch(ctx context.Context, deliveries []*db.NotificationDelivery, baseURL string, windowMinutes int) (notify.Message, []*db.NotificationDelivery, error) { +func (n *Notifier) renderBatch(ctx context.Context, deliveries []*db.NotificationDelivery, baseURL string, windowMinutes int, limits notify.Limits) (notify.Message, []*db.NotificationDelivery, error) { items := make([]notify.Item, 0, len(deliveries)) included := make([]*db.NotificationDelivery, 0, len(deliveries)) for _, dl := range deliveries { @@ -473,6 +486,7 @@ func (n *Notifier) renderBatch(ctx context.Context, deliveries []*db.Notificatio Batch: true, WindowMinutes: windowMinutes, HomeURL: baseURL, + Limits: &limits, }, included, nil } @@ -582,6 +596,30 @@ func (n *Notifier) digestInterval() time.Duration { return time.Duration(m) * time.Minute } +// notifyLimits reads the summary/asset render limits from the settings. +func notifyLimits(pg *db.DB) notify.Limits { + summary, sOK, _ := pg.GetSetting(settingNotifySummaryMaxRunes) + assets, aOK, _ := pg.GetSetting(settingNotifyAssetsMaxShown) + return notify.Limits{ + SummaryRunes: notifyLimitValue(summary, sOK, notify.DefaultMaxSummaryRunes, notifySummaryMaxRunesCap), + AssetsShown: notifyLimitValue(assets, aOK, notify.DefaultMaxAssetsShown, notifyAssetsMaxShownCap), + } +} + +// notifyLimitValue resolves one stored render limit: def when the key is unset, +// unparseable, or outside [0, max]. 0 is a valid value (no truncation), which is +// why "unset" is carried by ok and not by the number. +func notifyLimitValue(v string, ok bool, def, max int) int { + if !ok { + return def + } + n, err := strconv.Atoi(strings.TrimSpace(v)) + if err != nil || n < 0 || n > max { + return def + } + return n +} + // parseSnapshot parses the snapshot of the event a delivery corresponds to. func parseSnapshot(dl *db.NotificationDelivery) (notify.Snapshot, error) { var snap notify.Snapshot diff --git a/server/notify_api.go b/server/notify_api.go index 6a05684..ede3770 100644 --- a/server/notify_api.go +++ b/server/notify_api.go @@ -447,6 +447,8 @@ func (s *Server) notifyTestChannel(w http.ResponseWriter, r *http.Request) { return } msg := notifyTestMessage(s.notifierBaseURL(pg)) + limits := notifyLimits(pg) + msg.Limits = &limits start := time.Now() // The test message is a single entry, so the delivered count is not needed // here (for a single message the channel length limit falls back to diff --git a/server/notify_api_test.go b/server/notify_api_test.go index c63b64c..b52757f 100644 --- a/server/notify_api_test.go +++ b/server/notify_api_test.go @@ -700,6 +700,108 @@ func TestNotifyMetaAndSettingsRoundTrip(t *testing.T) { } } +// TestNotifyLimitSettingsRoundTrip covers the two render-limit keys through the +// settings API and the settings table: unset reads as the 120 / 3 defaults, a +// stored 0 reads back as 0 (no truncation) and not as the default, and +// out-of-range values are rejected without touching the stored value. +func TestNotifyLimitSettingsRoundTrip(t *testing.T) { + f := newNotifyFixture(t) + clear := func() { + f.pg.Exec(`DELETE FROM settings WHERE key IN ($1,$2)`, settingNotifySummaryMaxRunes, settingNotifyAssetsMaxShown) + } + clear() + t.Cleanup(clear) + expect := func(step string, summary, assets int) { + t.Helper() + payload := f.s.settingsPayload() + if payload["notify_summary_max_runes"] != summary || payload["notify_assets_max_shown"] != assets { + t.Fatalf("%s: settings payload should report %d / %d, got %v / %v", step, summary, assets, + payload["notify_summary_max_runes"], payload["notify_assets_max_shown"]) + } + if got := notifyLimits(f.pg); got != (notify.Limits{SummaryRunes: summary, AssetsShown: assets}) { + t.Fatalf("%s: the delivery engine should read %d / %d, got %+v", step, summary, assets, got) + } + } + expect("unset", notify.DefaultMaxSummaryRunes, notify.DefaultMaxAssetsShown) + + for _, tc := range []struct{ summary, assets int }{{500, 10}, {0, 0}, {4000, 50}} { + body := fmt.Sprintf(`{"notify_summary_max_runes":%d,"notify_assets_max_shown":%d}`, tc.summary, tc.assets) + if r := f.request("PUT", "/api/settings", body); r.Code != 200 { + t.Fatalf("%s failed %d: %s", body, r.Code, r.Body) + } + expect(body, tc.summary, tc.assets) + for key, want := range map[string]int{settingNotifySummaryMaxRunes: tc.summary, settingNotifyAssetsMaxShown: tc.assets} { + if v, ok, err := f.pg.GetSetting(key); err != nil || !ok || v != strconv.Itoa(want) { + t.Fatalf("%s: settings row %s should hold %d, got %q (ok=%v err=%v)", body, key, want, v, ok, err) + } + } + } + + for _, body := range []string{ + `{"notify_summary_max_runes":-1}`, + `{"notify_summary_max_runes":4001}`, + `{"notify_assets_max_shown":-1}`, + `{"notify_assets_max_shown":51}`, + } { + if r := f.request("PUT", "/api/settings", body); r.Code != 400 { + t.Errorf("%s should return 400, got %d", body, r.Code) + } + } + expect("after rejected writes", 4000, 50) +} + +// TestNotifySummaryLimitReachesDelivery is the end-to-end evidence that the +// stored limit reaches the message actually sent: with the key unset the summary +// is cut at 120 characters as before, and once it is set to 0 the same summary +// goes out whole. +func TestNotifySummaryLimitReachesDelivery(t *testing.T) { + f := newNotifyFixture(t) + clear := func() { f.pg.Exec(`DELETE FROM settings WHERE key=$1`, settingNotifySummaryMaxRunes) } + clear() + t.Cleanup(clear) + hook := newFakeWebhook(t) + chID := f.createChannel(t, map[string]any{ + "name": "summary limit", + "kind": notify.KindDingTalk, + "config": map[string]any{"webhook": hook.URL}, + }) + summary := strings.Repeat("Z", 300) + record := func(name string) { + t.Helper() + if _, err := f.s.evidenceStore().Record(context.Background(), db.RecordFindingInput{ + TaskID: f.taskID, + ExplorationID: f.expID, + Worker: "test", + VulnClass: name, + Name: name, + Severity: "high", + Summary: summary, + Evidence: "poc", + }, nil); err != nil { + t.Fatalf("failed to record finding: %v", err) + } + } + + record("summary limit unset") + f.deliver(t, chID, "") + text := markdownText(t, hook.last(t)) + if !strings.Contains(text, strings.Repeat("Z", 119)+"…") || strings.Contains(text, strings.Repeat("Z", 120)) { + t.Fatalf("with the key unset the summary should be cut at 120 characters:\n%s", text) + } + + if r := f.request("PUT", "/api/settings", `{"notify_summary_max_runes":0}`); r.Code != 200 { + t.Fatalf("failed to write settings %d: %s", r.Code, r.Body) + } + record("summary limit zero") + f.deliver(t, chID, "") + if hook.count() != 2 { + t.Fatalf("expected 2 messages sent, got %d", hook.count()) + } + if text := markdownText(t, hook.last(t)); !strings.Contains(text, summary) { + t.Fatalf("with the limit at 0 the whole summary should be sent:\n%s", text) + } +} + // TestNotifyDeepLinkUsesPublicBaseURL covers back-link building: when // public_base_url is configured, single messages must use the ActionCard with a // button, and the link must point to the vulnerability detail page. @@ -1008,3 +1110,30 @@ func TestNotifyTickBudgetFitsWithinLease(t *testing.T) { worst, notifyLease, notifyMaxSendsPerChannelPerTick, notifySendTimeout) } } + +// TestNotifyLimitValue pins how a stored render limit is resolved. The case that +// matters is "0": it is a real value (no truncation) and must not be read as +// "unset", while anything unparseable or outside the bounds falls back to the +// default instead of reaching the renderer. +func TestNotifyLimitValue(t *testing.T) { + const def, max = 120, 4000 + cases := []struct { + v string + ok bool + want int + }{ + {"", false, def}, + {"0", true, 0}, + {" 500 ", true, 500}, + {"4000", true, max}, + {"4001", true, def}, + {"-1", true, def}, + {"abc", true, def}, + {"", true, def}, + } + for _, tc := range cases { + if got := notifyLimitValue(tc.v, tc.ok, def, max); got != tc.want { + t.Errorf("notifyLimitValue(%q, %v) = %d, want %d", tc.v, tc.ok, got, tc.want) + } + } +} diff --git a/server/server.go b/server/server.go index fbc2325..1d3798a 100644 --- a/server/server.go +++ b/server/server.go @@ -3454,6 +3454,7 @@ func (s *Server) settingsPayload() map[string]any { if concLimit == 0 { concLimit = defaultConcurrencyLimit // when off, still echo a sane default to the UI } + limits := notifyLimits(s.m.pg) return map[string]any{ "traffic_capture": s.m.TrafficEnabled(), "agent_traffic_binding": s.m.pg.GetBool(settingAgentTrafficBinding, false), @@ -3483,6 +3484,9 @@ func (s *Server) settingsPayload() map[string]any { "notify_enabled": s.m.pg.GetBool(settingNotifyEnabled, true), "notify_public_base_url": notifyPublicBaseURL(s.m.pg), "notify_digest_interval_min": notifyDigestIntervalMin(s.m.pg), + // Render limits of IM messages; 0 = no truncation. + "notify_summary_max_runes": limits.SummaryRunes, + "notify_assets_max_shown": limits.AssetsShown, } } @@ -3555,6 +3559,9 @@ func (s *Server) putSettings(w http.ResponseWriter, r *http.Request) { NotifyEnabled *bool `json:"notify_enabled"` NotifyBaseURL *string `json:"notify_public_base_url"` NotifyDigestMins *int `json:"notify_digest_interval_min"` + // Render limits of IM messages: summary characters (0-4000) and asset names listed (0-50); 0 = no truncation. + NotifySummaryMaxRunes *int `json:"notify_summary_max_runes"` + NotifyAssetsMaxShown *int `json:"notify_assets_max_shown"` } if err := json.NewDecoder(r.Body).Decode(&req); err != nil { writeErr(w, 400, err.Error()) @@ -3610,6 +3617,26 @@ func (s *Server) putSettings(w http.ResponseWriter, r *http.Request) { return } } + if req.NotifySummaryMaxRunes != nil { + if *req.NotifySummaryMaxRunes < 0 || *req.NotifySummaryMaxRunes > notifySummaryMaxRunesCap { + writeErr(w, 400, fmt.Sprintf("summary limit must be between 0 and %d characters (0 = no truncation)", notifySummaryMaxRunesCap)) + return + } + if err := s.m.pg.SetSetting(settingNotifySummaryMaxRunes, strconv.Itoa(*req.NotifySummaryMaxRunes)); err != nil { + writeErr(w, 500, err.Error()) + return + } + } + if req.NotifyAssetsMaxShown != nil { + if *req.NotifyAssetsMaxShown < 0 || *req.NotifyAssetsMaxShown > notifyAssetsMaxShownCap { + writeErr(w, 400, fmt.Sprintf("asset limit must be between 0 and %d (0 = no truncation)", notifyAssetsMaxShownCap)) + return + } + if err := s.m.pg.SetSetting(settingNotifyAssetsMaxShown, strconv.Itoa(*req.NotifyAssetsMaxShown)); err != nil { + writeErr(w, 500, err.Error()) + return + } + } if req.Workers != nil { if err := s.m.SetWorkers(*req.Workers); err != nil { writeErr(w, 400, err.Error())