fix: harden IMDb IRC output

config.yaml

@@ -83,6 +83,7 @@ plugins:
   acronym: {enabled: true, data_file: "data/acronyms.txt", max_length: 320}
   weather: {enabled: true, default_units: imperial}
   steam: {enabled: true, timeout_seconds: 10, max_length: 360}
+  # IMDb max_length is UTF-8 response bytes; the IRC 512-byte wire cap is also enforced.
   imdb: {enabled: true, timeout_seconds: 8, max_length: 320, max_results: 3, cooldown_seconds: 5}
   news: {enabled: true, api_key: "", max_results: 3, max_length: 360}
   # Search Assist and bounded public-result excerpts; Instant Answers/Wikidata remain fallbacks.

docs/configuration.md

@@ -105,6 +105,7 @@ plugins:
   attack: {enabled: true}
   weapons: {enabled: true, data_file: "data/weapons.txt", max_length: 240}
   github: {enabled: true, timeout_seconds: 8, max_length: 360, token: ""}
+  # IMDb max_length is UTF-8 response bytes; the IRC 512-byte wire cap is also enforced.
   imdb: {enabled: true, timeout_seconds: 8, max_length: 320, max_results: 3, cooldown_seconds: 5}
   reddit: {enabled: true, timeout_seconds: 8, max_length: 360}
   daily: {enabled: true, bonus_xp: 25}

docs/plugins.md

@@ -673,10 +673,13 @@ an API key:
 GoBot uses IMDb's keyless JSON suggestion endpoint to return up to
 `plugins.imdb.max_results` matches with the title, year, type, principal cast
 when available, and a direct IMDb link. Results are kept to one IRC line and
-bounded by `plugins.imdb.max_length`. The `🎥` marker is deliberately a single
-Unicode code point without a U+FE0F variation selector, which avoids an
-avoidable source of width differences in some terminal clients, including
-older Mosh terminal combinations.
+bounded by `plugins.imdb.max_length` UTF-8 bytes and the IRC protocol's
+512-byte wire-line limit, whichever is smaller. All response paths use the
+same byte-aware limit. The `🎥` marker is deliberately a single Unicode code
+point without a U+FE0F variation selector. Variation selectors, invisible
+format controls, IRC controls, and line breaks are removed from returned IMDb
+metadata to avoid formatting injection and an avoidable source of width
+differences in terminal clients, including older Mosh terminal combinations.
 
 This is not HTML scraping and it is not IMDb's official GraphQL API. IMDb's
 official non-commercial datasets are bulk downloads, while the official API

plugins/imdb.go

@@ -10,6 +10,8 @@ import (
 	"net/url"
 	"strings"
 	"time"
+	"unicode"
+	"unicode/utf8"
 
 	"github.com/variablenix/GoBot/bot"
 	"github.com/variablenix/GoBot/storage"
@@ -23,6 +25,7 @@ const imdbPrefix = "🎥"
 const (
 	defaultIMDbMaxLength = 320
 	defaultIMDbResults   = 3
+	imdbIRCMaxLineBytes  = 512
 )
 
 type IMDb struct {
@@ -64,13 +67,13 @@ func (p *IMDb) Handle(b *bot.Bot, m bot.Message) bool {
 
 	query := strings.TrimSpace(arg)
 	if !validIMDbQuery(query) {
-		b.Send(m.ReplyTarget(), "usage: !imdb <movie or film>")
+		p.send(b, m.ReplyTarget(), "usage: !imdb <movie or film>")
 		return true
 	}
 
 	key := scopedKey(b.Config.NetworkName, m.ReplyTarget(), pluginIdentity(m))
 	if !p.cooldown.allow(key) {
-		b.Send(m.ReplyTarget(), "IMDb search is cooling down — please wait a moment")
+		p.send(b, m.ReplyTarget(), "IMDb search is cooling down — please wait a moment")
 		return true
 	}
 
@@ -79,22 +82,34 @@ func (p *IMDb) Handle(b *bot.Bot, m bot.Message) bool {
 	titles, err := lookupIMDbTitles(ctx, query)
 	if err != nil {
 		if err == errIMDbNotFound {
-			b.Send(m.ReplyTarget(), fmt.Sprintf("%s IMDb: no movie or film found for %q", imdbPrefix, cleanExternalText(query)))
+			p.send(b, m.ReplyTarget(), fmt.Sprintf("%s IMDb: no movie or film found for %q", imdbPrefix, query))
 		} else {
-			b.Send(m.ReplyTarget(), "IMDb search is temporarily unavailable")
+			p.send(b, m.ReplyTarget(), "IMDb search is temporarily unavailable")
 		}
 		return true
 	}
 
 	result := formatIMDbResults(titles, imdbMaxResults(p.cfg))
-	b.Send(m.ReplyTarget(), truncateRunes(result, imdbMaxLength(p.cfg)))
+	p.send(b, m.ReplyTarget(), result)
 	return true
 }
 
+func (p *IMDb) send(b *bot.Bot, target, text string) {
+	b.Send(target, boundIMDbReply(target, text, imdbMaxLength(p.cfg)))
+}
+
 var errIMDbNotFound = errors.New("IMDb title not found")
 
 func validIMDbQuery(query string) bool {
-	return query != "" && len([]rune(query)) <= 120 && !strings.ContainsAny(query, "\r\n\t")
+	if query == "" || len([]rune(query)) > 120 {
+		return false
+	}
+	for _, r := range query {
+		if unsafeIMDbRune(r) {
+			return false
+		}
+	}
+	return true
 }
 
 func imdbTimeout(c bot.PluginConfig) time.Duration {
@@ -121,6 +136,58 @@ func imdbMaxResults(c bot.PluginConfig) int {
 	return max
 }
 
+func boundIMDbReply(target, text string, configuredMax int) string {
+	text = cleanIMDbText(text)
+	// gopkg.in/irc.v3 writes messages as-is. Reserve the exact bytes used by
+	// "PRIVMSG <target> :" and CRLF so the complete wire line stays at or
+	// below IRC's 512-byte limit.
+	wireLimit := imdbIRCMaxLineBytes - len("PRIVMSG ") - len([]byte(target)) - len(" :") - len("\r\n")
+	if wireLimit < 1 {
+		wireLimit = 1
+	}
+	if configuredMax < 1 || configuredMax > wireLimit {
+		configuredMax = wireLimit
+	}
+	return truncateUTF8Bytes(text, configuredMax)
+}
+
+func truncateUTF8Bytes(text string, maxBytes int) string {
+	if maxBytes <= 0 {
+		return ""
+	}
+	if len(text) <= maxBytes {
+		return text
+	}
+	const suffix = "…"
+	if maxBytes < len(suffix) {
+		return strings.Repeat(".", maxBytes)
+	}
+	cut := maxBytes - len(suffix)
+	for cut > 0 && !utf8.RuneStart(text[cut]) {
+		cut--
+	}
+	return text[:cut] + suffix
+}
+
+func cleanIMDbText(text string) string {
+	text = cleanExternalText(text)
+	var cleaned strings.Builder
+	cleaned.Grow(len(text))
+	for _, r := range text {
+		if unsafeIMDbRune(r) {
+			continue
+		}
+		cleaned.WriteRune(r)
+	}
+	return strings.Join(strings.Fields(cleaned.String()), " ")
+}
+
+func unsafeIMDbRune(r rune) bool {
+	return unicode.IsControl(r) || unicode.Is(unicode.Cf, r) ||
+		(r >= 0xFE00 && r <= 0xFE0F) ||
+		(r >= 0xE0100 && r <= 0xE01EF)
+}
+
 func lookupIMDbTitles(ctx context.Context, query string) ([]imdbTitle, error) {
 	if !validIMDbQuery(query) {
 		return nil, errIMDbNotFound
@@ -192,21 +259,21 @@ func formatIMDbResults(titles []imdbTitle, maxResults int) string {
 	}
 	items := make([]string, 0, maxResults)
 	for _, title := range titles[:maxResults] {
-		label := cleanExternalText(title.Label)
+		label := cleanIMDbText(title.Label)
 		details := make([]string, 0, 3)
 		if year := imdbYear(title); year != "" {
 			details = append(details, year)
 		}
 		if kind := imdbKind(title); kind != "" {
 			details = append(details, kind)
 		}
-		if stars := cleanExternalText(title.Stars); stars != "" {
+		if stars := cleanIMDbText(title.Stars); stars != "" {
 			details = append(details, stars)
 		}
 		if len(details) > 0 {
 			label += " (" + strings.Join(details, "; ") + ")"
 		}
-		id := cleanExternalText(title.ID)
+		id := strings.TrimSpace(title.ID)
 		items = append(items, label+" | https://www.imdb.com/title/"+id+"/")
 	}
 	result := imdbPrefix + " IMDb: " + strings.Join(items, " ; ")
@@ -220,7 +287,7 @@ func imdbYear(title imdbTitle) string {
 	if title.Year > 0 {
 		return fmt.Sprintf("%d", title.Year)
 	}
-	return cleanExternalText(title.YearRaw)
+	return cleanIMDbText(title.YearRaw)
 }
 
 func imdbKind(title imdbTitle) string {
@@ -242,6 +309,6 @@ func imdbKind(title imdbTitle) string {
 	case "videogame":
 		return "game"
 	default:
-		return cleanExternalText(title.Kind)
+		return cleanIMDbText(title.Kind)
 	}
 }

plugins/imdb_test.go

@@ -1,9 +1,15 @@
 package plugins
 
 import (
+	"context"
+	"errors"
 	"net/http"
 	"strings"
 	"testing"
+	"time"
+	"unicode/utf8"
+
+	"github.com/variablenix/GoBot/bot"
 )
 
 func TestIMDbCommandsAndHelp(t *testing.T) {
@@ -26,13 +32,47 @@ func TestValidIMDbQuery(t *testing.T) {
 	if !validIMDbQuery("The Matrix") {
 		t.Fatal("expected a normal title query to be valid")
 	}
-	for _, query := range []string{"", "hello\nworld", strings.Repeat("x", 121)} {
+	for _, query := range []string{"", "hello\nworld", "hidden\u200djoiner", "emoji\ufe0f", strings.Repeat("x", 121)} {
 		if validIMDbQuery(query) {
 			t.Fatalf("query %q should be invalid", query)
 		}
 	}
 }
 
+func TestCleanIMDbTextRemovesInvisibleFormatting(t *testing.T) {
+	got := cleanIMDbText("The\ufe0f \u200dMovie\u202e\x03\x034")
+	if got != "The Movie" {
+		t.Fatalf("cleanIMDbText() = %q, want %q", got, "The Movie")
+	}
+	for _, forbidden := range []string{"\ufe0f", "\u200d", "\u202e", "\x03"} {
+		if strings.Contains(got, forbidden) {
+			t.Fatalf("cleaned IMDb text still contains %q: %q", forbidden, got)
+		}
+	}
+}
+
+func TestBoundIMDbReplyUsesUTF8WireByteLimit(t *testing.T) {
+	target := "#international-movies"
+	reply := boundIMDbReply(target, imdbPrefix+" IMDb: "+strings.Repeat("界", 300)+"\r\nsecond line", 500)
+	wire := "PRIVMSG " + target + " :" + reply + "\r\n"
+	if len([]byte(wire)) > imdbIRCMaxLineBytes {
+		t.Fatalf("wire line is %d bytes, want at most %d", len([]byte(wire)), imdbIRCMaxLineBytes)
+	}
+	if !utf8.ValidString(reply) {
+		t.Fatalf("reply is not valid UTF-8: %q", reply)
+	}
+	if strings.ContainsAny(reply, "\r\n") {
+		t.Fatalf("reply contains a line break: %q", reply)
+	}
+	if !strings.HasSuffix(reply, "…") {
+		t.Fatalf("truncated reply does not end with an ellipsis: %q", reply)
+	}
+	configuredReply := boundIMDbReply(target, strings.Repeat("界", 100), 120)
+	if len([]byte(configuredReply)) > 120 {
+		t.Fatalf("configured reply is %d bytes, want at most 120", len([]byte(configuredReply)))
+	}
+}
+
 func TestLookupIMDbTitlesFiltersPeople(t *testing.T) {
 	old := apiHTTPClient
 	t.Cleanup(func() { apiHTTPClient = old })
@@ -61,7 +101,7 @@ func TestValidIMDbID(t *testing.T) {
 	}
 }
 
-func TestFormatIMDbResultsIsBoundedAndIncludesMoreCount(t *testing.T) {
+func TestFormatIMDbResultsIncludesMoreCount(t *testing.T) {
 	titles := []imdbTitle{
 		{ID: "tt1375666", Label: "Inception", KindID: "movie", Year: 2010, Stars: "Leonardo DiCaprio"},
 		{ID: "tt0133093", Label: "The Matrix", KindID: "movie", Year: 1999},
@@ -75,3 +115,109 @@ func TestFormatIMDbResultsIsBoundedAndIncludesMoreCount(t *testing.T) {
 		}
 	}
 }
+
+func TestLookupIMDbTitlesHandlesUpstreamFailures(t *testing.T) {
+	tests := []struct {
+		name   string
+		status int
+		body   string
+	}{
+		{name: "server error", status: http.StatusInternalServerError, body: `{}`},
+		{name: "malformed JSON", status: http.StatusOK, body: `{"d":[`},
+		{name: "no usable titles", status: http.StatusOK, body: `{"d":[{"id":"nm0000138","l":"Person","qid":"name"}]}`},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			old := apiHTTPClient
+			t.Cleanup(func() { apiHTTPClient = old })
+			apiHTTPClient = &http.Client{Transport: newPluginRoundTripper(func(*http.Request) (*http.Response, error) {
+				return newPluginResponse(test.status, test.body), nil
+			})}
+			if _, err := lookupIMDbTitles(t.Context(), "inception"); err == nil {
+				t.Fatal("lookupIMDbTitles() error = nil")
+			}
+		})
+	}
+}
+
+func TestLookupIMDbTitlesHonorsContextCancellation(t *testing.T) {
+	old := apiHTTPClient
+	t.Cleanup(func() { apiHTTPClient = old })
+	apiHTTPClient = &http.Client{Transport: newPluginRoundTripper(func(request *http.Request) (*http.Response, error) {
+		<-request.Context().Done()
+		return nil, request.Context().Err()
+	})}
+	ctx, cancel := context.WithTimeout(t.Context(), time.Millisecond)
+	defer cancel()
+	if _, err := lookupIMDbTitles(ctx, "inception"); !errors.Is(err, context.DeadlineExceeded) {
+		t.Fatalf("lookupIMDbTitles() error = %v, want context deadline exceeded", err)
+	}
+}
+
+func TestIMDbHandleAlwaysSendsOneSafeWireLine(t *testing.T) {
+	tests := []struct {
+		name     string
+		query    string
+		status   int
+		body     string
+		contains string
+	}{
+		{
+			name:     "successful lookup sanitizes third-party text",
+			query:    "international",
+			status:   http.StatusOK,
+			body:     `{"d":[{"id":"tt1234567","l":"` + strings.Repeat("界", 180) + `\ufe0f\u200d","q":"feature","qid":"movie","s":"Actor\u202e Name","y":2026}]}`,
+			contains: imdbPrefix + " IMDb:",
+		},
+		{
+			name:     "not found bounds multibyte query",
+			query:    strings.Repeat("界", 120),
+			status:   http.StatusNotFound,
+			body:     `{}`,
+			contains: "no movie or film found",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			old := apiHTTPClient
+			t.Cleanup(func() { apiHTTPClient = old })
+			apiHTTPClient = &http.Client{Transport: newPluginRoundTripper(func(*http.Request) (*http.Response, error) {
+				return newPluginResponse(test.status, test.body), nil
+			})}
+
+			sent := make(chan bot.Outgoing, 2)
+			cfg := bot.Config{NetworkName: "test", CommandPrefix: "!"}
+			b := &bot.Bot{Config: cfg, Queue: bot.NewQueue(1000, 20, func(message bot.Outgoing) { sent <- message })}
+			plugin := &IMDb{}
+			if err := plugin.Init(bot.PluginConfig{"max_length": 500, "max_results": 5, "cooldown_seconds": 1}, nil); err != nil {
+				t.Fatal(err)
+			}
+			message := bot.Message{Nick: "Alice", Target: "#movies", IsChannel: true, Text: "!imdb " + test.query}
+			if !plugin.Handle(b, message) {
+				t.Fatal("IMDb command was not consumed")
+			}
+			b.Queue.Drain(context.Background())
+
+			if len(sent) != 1 {
+				t.Fatalf("IMDb sent %d messages, want exactly one", len(sent))
+			}
+			outgoing := <-sent
+			if !strings.Contains(outgoing.Text, test.contains) {
+				t.Fatalf("reply %q does not contain %q", outgoing.Text, test.contains)
+			}
+			wire := "PRIVMSG " + outgoing.Target + " :" + outgoing.Text + "\r\n"
+			if len([]byte(wire)) > imdbIRCMaxLineBytes {
+				t.Fatalf("wire line is %d bytes, want at most %d", len([]byte(wire)), imdbIRCMaxLineBytes)
+			}
+			if !utf8.ValidString(outgoing.Text) || strings.ContainsAny(outgoing.Text, "\r\n") {
+				t.Fatalf("reply is not a valid single UTF-8 line: %q", outgoing.Text)
+			}
+			for _, forbidden := range []string{"\ufe0f", "\u200d", "\u202e"} {
+				if strings.Contains(outgoing.Text, forbidden) {
+					t.Fatalf("reply contains unsafe formatting rune %q: %q", forbidden, outgoing.Text)
+				}
+			}
+		})
+	}
+}