fix(output): truncate progress item on a rune boundary

internal/notify/notify_test.go

@@ -159,6 +159,41 @@ func TestWebhookPayloadShape(t *testing.T) {
 	}
 }
 
+// a finding title carrying a triple-backtick run (e.g. a scraped html <title>)
+// must not terminate the code fence early: if it does, the trailing text lands
+// outside the block and a "@everyone" rides through as a live mention.
+func TestCodeBlockFindingCannotBreakFence(t *testing.T) {
+	for _, title := range []string{"``` @everyone", "````", "a``````b"} {
+		findings := []finding.Finding{
+			{Target: "https://evil.test", Module: "cms", Severity: finding.SeverityInfo, Key: "cms:t", Title: title},
+		}
+		content := codeBlock(renderFindings(findings))
+		// the only ``` runs are the wrapper's opener and closer; any surviving in
+		// the body would split the fence open and spill the tail as live markdown.
+		if n := strings.Count(content, "```"); n != 2 {
+			t.Fatalf("title %q broke the fence: found %d ``` runs, want 2\n%s", title, n, content)
+		}
+	}
+}
+
+// slack parses its angle-bracket entities (broadcast mentions <!channel> and
+// user mentions <@U..>) even inside a code fence, so the fence cannot contain
+// them. the slack sink must html-escape &, < and > first: a title of
+// "<!channel>" must not survive as a live entity in the payload.
+func TestSlackEscapesBroadcastMention(t *testing.T) {
+	for _, title := range []string{"<!channel>", "<!everyone>", "<@U0>", "<!here> a&b"} {
+		findings := []finding.Finding{
+			{Target: "https://evil.test", Module: "cms", Severity: finding.SeverityInfo, Key: "cms:t", Title: title},
+		}
+		text := slackText(findings)
+		// codeBlock adds only backticks, so any raw < or > left in the payload is
+		// an unescaped entity that slack would parse as a live mention.
+		if strings.ContainsAny(text, "<>") {
+			t.Fatalf("title %q left a live angle bracket in slack payload:\n%s", title, text)
+		}
+	}
+}
+
 func TestProviderNon2xxIsError(t *testing.T) {
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.WriteHeader(http.StatusForbidden)

internal/notify/slack.go

@@ -15,6 +15,7 @@ package notify
 import (
 	"context"
 	"net/http"
+	"strings"
 
 	"github.com/vmfunc/sif/internal/finding"
 )
@@ -34,12 +35,42 @@ type slackPayload struct {
 }
 
 func (s *slackProvider) send(ctx context.Context, client *http.Client, findings []finding.Finding) error {
-	payload := slackPayload{Text: codeBlock(renderFindings(findings))}
+	payload := slackPayload{Text: slackText(findings)}
 	return postJSON(ctx, client, s.webhook, payload)
 }
 
+// slackText builds the slack payload text so the escape-then-fence order has a
+// single home the caller and tests share; skip either step and untrusted title
+// text reaches the channel unneutralized.
+func slackText(findings []finding.Finding) string {
+	return codeBlock(slackEscape(renderFindings(findings)))
+}
+
+// slackEscape converts slack's three mrkdwn control characters to html entities.
+// slack parses its angle-bracket entities - broadcast mentions <!channel>,
+// <!here>, <!everyone> and user mentions <@U..> - even inside a code fence, so
+// fencing alone does not contain an untrusted title: a title of "<!channel>"
+// would still ping the channel. neutralizing &, < and > blocks every such entity.
+// order matters: & is escaped first or a real < would become &amp;lt;.
+func slackEscape(s string) string {
+	s = strings.ReplaceAll(s, "&", "&amp;")
+	s = strings.ReplaceAll(s, "<", "&lt;")
+	s = strings.ReplaceAll(s, ">", "&gt;")
+	return s
+}
+
 // codeBlock wraps body in a triple-backtick fence; both slack and discord render
 // it fixed-width, which preserves the column-aligned finding lines.
 func codeBlock(body string) string {
-	return "```\n" + body + "```"
+	return "```\n" + fenceGuard(body) + "```"
+}
+
+// fenceGuard breaks every run of three backticks in s with zero-width spaces so
+// no ``` survives to terminate the surrounding fence: finding text is untrusted
+// (a scraped html title can carry its own ``` run), and a run of three would
+// close the fence early and spill the tail out as live markdown. the inserted
+// U+200B is invisible in both slack and discord, so the finding lines still
+// read cleanly.
+func fenceGuard(s string) string {
+	return strings.ReplaceAll(s, "```", "`\u200b`\u200b`")
 }

internal/output/progress.go

@@ -16,6 +16,7 @@ import (
 	"fmt"
 	"sync"
 	"sync/atomic"
+	"unicode/utf8"
 )
 
 // Progress bar configuration
@@ -174,11 +175,7 @@ func (p *Progress) render() {
 		}
 	}
 
-	// Truncate item if too long
-	maxItemLen := 30
-	if len(lastItem) > maxItemLen {
-		lastItem = lastItem[:maxItemLen-3] + "..."
-	}
+	lastItem = truncateItem(lastItem, 30)
 
 	// Format: [========>          ] 45% (4500/10000) /admin
 	line := fmt.Sprintf("    [%s] %3d%% (%d/%d) %s",
@@ -192,3 +189,22 @@ func (p *Progress) render() {
 	ClearLine()
 	fmt.Fprint(sink, line)
 }
+
+// truncateItem shortens item to at most limit columns for the progress line,
+// cutting on a rune boundary so a multibyte path never leaves a half-rune that
+// renders as a replacement glyph. width is counted in runes, not bytes.
+func truncateItem(item string, limit int) string {
+	if utf8.RuneCountInString(item) <= limit {
+		return item
+	}
+	runes := []rune(item)
+	// the ellipsis is 3 columns; a smaller cap can't hold it, so cut plainly and
+	// never let limit-3 go negative (a negative slice bound panics).
+	if limit < 3 {
+		if limit < 0 {
+			limit = 0
+		}
+		return string(runes[:limit])
+	}
+	return string(runes[:limit-3]) + "..."
+}

internal/output/progress_test.go

@@ -17,8 +17,40 @@ import (
 	"strings"
 	"sync"
 	"testing"
+	"unicode/utf8"
 )
 
+// see truncateItem's doc comment (progress.go) for why this must hold.
+func TestTruncateItemStaysValidUTF8(t *testing.T) {
+	item := strings.Repeat("é", 40) // 40 runes, 80 bytes, well over the cap
+	got := truncateItem(item, 30)
+	if !utf8.ValidString(got) {
+		t.Fatalf("truncateItem produced invalid utf-8: %q", got)
+	}
+	if !strings.HasSuffix(got, "...") {
+		t.Errorf("truncateItem dropped the ellipsis: %q", got)
+	}
+}
+
+// a cap smaller than the 3-column ellipsis must not panic on the limit-3 slice
+// bound; it falls back to a plain rune cut and honours the cap.
+func TestTruncateItemSmallLimit(t *testing.T) {
+	for _, tc := range []struct {
+		item  string
+		limit int
+		want  string
+	}{
+		{"abcdef", 2, "ab"},
+		{"abcdef", 0, ""},
+		{"héllo", 1, "h"},
+		{"abcdef", -1, ""},
+	} {
+		if got := truncateItem(tc.item, tc.limit); got != tc.want {
+			t.Errorf("truncateItem(%q, %d) = %q, want %q", tc.item, tc.limit, got, tc.want)
+		}
+	}
+}
+
 // the non-tty milestone path divides current*100/total, so a zero-total bar
 // used to panic with integer divide-by-zero when piped or redirected.
 func TestProgressZeroTotalNoPanic(t *testing.T) {