Fix YouTube search failures and audit plugin safety

bot/bot.go

@@ -180,12 +180,27 @@ func clonePluginOverrides(overrides map[string]map[string]bool) map[string]map[s
 	return clone
 }
 func (b *Bot) Send(target, text string) {
-	if !b.Queue.Enqueue(Outgoing{target, text}) {
-		b.Stats.dropped.Add(1)
-		if b.networkStats != nil {
-			b.networkStats.dropped.Add(1)
+	parts := outgoingMessageParts(target, text)
+	if len(parts) == 0 {
+		b.recordDroppedOutput()
+		return
+	}
+	for _, part := range parts {
+		if !b.Queue.Enqueue(Outgoing{target, part}) {
+			b.recordDroppedOutput()
+			if b.Log != nil {
+				b.Log.Warn("outgoing queue full", zap.String("target", target))
+			}
 		}
-		b.Log.Warn("outgoing queue full", zap.String("target", target))
+	}
+}
+
+func (b *Bot) recordDroppedOutput() {
+	if b.Stats != nil {
+		b.Stats.dropped.Add(1)
+	}
+	if b.networkStats != nil {
+		b.networkStats.dropped.Add(1)
 	}
 }
 func (b *Bot) sendNow(target, text string) {

bot/outgoing.go

@@ -0,0 +1,65 @@
+package bot
+
+import (
+	"strings"
+	"unicode/utf8"
+)
+
+// The IRC writer does not enforce the base protocol's 512-byte limit or
+// remove line terminators. Enforce these once for every plugin's PRIVMSG.
+// Split large results instead of silently dropping their trailing links.
+func outgoingMessageParts(target, text string) []string {
+	if target == "" || strings.ContainsAny(target, " \t\r\n\x00,") || strings.HasPrefix(target, ":") || !utf8.ValidString(target) {
+		return nil
+	}
+	limit := 512 - len("PRIVMSG "+target+" :\r\n")
+	if limit < 16 {
+		return nil
+	}
+	text = strings.NewReplacer("\r", " ", "\n", " ", "\x00", "").Replace(strings.ToValidUTF8(text, ""))
+	if len(text) <= limit {
+		return []string{text}
+	}
+	var parts []string
+	// Bound amplification even if an upstream returns an unexpectedly huge field.
+	for len(text) > 0 && len(parts) < 10 {
+		if len(text) <= limit {
+			parts = append(parts, text)
+			break
+		}
+		end := 0
+		// Keep UTF-8 and mIRC color parameters intact. Reserve a reset byte so
+		// a split formatted reply cannot bleed its style into client UI text.
+		for end < len(text) {
+			_, size := utf8.DecodeRuneInString(text[end:])
+			if text[end] == '\x03' {
+				size = outgoingColorSize(text[end:])
+			}
+			if end+size > limit-1 {
+				break
+			}
+			end += size
+		}
+		part := text[:end]
+		if strings.ContainsAny(part, "\x02\x03\x16\x1d\x1e\x1f") {
+			part += "\x0f"
+		}
+		parts = append(parts, part)
+		text = text[end:]
+	}
+	return parts
+}
+
+func outgoingColorSize(text string) int {
+	i := 1
+	for count := 0; i < len(text) && count < 2 && text[i] >= '0' && text[i] <= '9'; count++ {
+		i++
+	}
+	if i+1 < len(text) && text[i] == ',' && text[i+1] >= '0' && text[i+1] <= '9' {
+		i++
+		for count := 0; i < len(text) && count < 2 && text[i] >= '0' && text[i] <= '9'; count++ {
+			i++
+		}
+	}
+	return i
+}

bot/outgoing_test.go

@@ -0,0 +1,67 @@
+package bot
+
+import (
+	"context"
+	"strings"
+	"testing"
+	"unicode/utf8"
+)
+
+func TestOutgoingPluginTextIsBoundedWithoutLosingSuffix(t *testing.T) {
+	for _, target := range []string{"#test", "#" + strings.Repeat("c", 180)} {
+		text := strings.Repeat("界🎥", 250) + " https://example.com/result"
+		parts := outgoingMessageParts(target, text)
+		if len(parts) < 2 || strings.Join(parts, "") != text {
+			t.Fatal("large output lost content or its trailing link")
+		}
+		for _, part := range parts {
+			if !utf8.ValidString(part) || len("PRIVMSG "+target+" :"+part+"\r\n") > 512 {
+				t.Fatalf("invalid UTF-8 or oversized IRC line: %d bytes", len(part))
+			}
+		}
+	}
+}
+
+func TestOutgoingTextPreservesShortFormattingAndBlocksInjection(t *testing.T) {
+	text := "\x0308GOLDEN DUCK\x0f 🎥"
+	if parts := outgoingMessageParts("#test", text); len(parts) != 1 || parts[0] != text {
+		t.Fatalf("short formatted output changed: %q", parts)
+	}
+	for _, target := range []string{"", "#test\r\nOPER injected", "#test other", "#a,#b", ":bad", strings.Repeat("c", 600)} {
+		if parts := outgoingMessageParts(target, "hello"); len(parts) != 0 {
+			t.Errorf("invalid target %q accepted", target)
+		}
+	}
+	parts := outgoingMessageParts("#test", "hello\r\nQUIT :injected\x00\xff")
+	if len(parts) != 1 || parts[0] != "hello  QUIT :injected" {
+		t.Fatalf("unsafe characters survived: %q", parts)
+	}
+}
+
+func TestOutgoingColorSequenceIsNotCut(t *testing.T) {
+	limit := 512 - len("PRIVMSG #test :\r\n")
+	text := strings.Repeat("x", limit-3) + "\x0308,04" + strings.Repeat("gold", 150)
+	parts := outgoingMessageParts("#test", text)
+	if !strings.HasPrefix(parts[1], "\x0308,04") {
+		t.Fatalf("color sequence split: %q", parts[:2])
+	}
+	if len(outgoingMessageParts("#test", strings.Repeat("x", 100000))) != 10 {
+		t.Fatal("output amplification is not bounded")
+	}
+}
+
+func TestSendAppliesSafetyBeforeQueueing(t *testing.T) {
+	sent := make(chan Outgoing, 10)
+	b := &Bot{Queue: NewQueue(1, 1, func(message Outgoing) { sent <- message })}
+	b.Send("#test", strings.Repeat("x", 700)+"\r\n")
+	b.Queue.Drain(context.Background())
+	if len(sent) != 2 {
+		t.Fatalf("expected two safe messages, got %d", len(sent))
+	}
+	for len(sent) > 0 {
+		message := <-sent
+		if len(message.Text) > 497 || strings.ContainsAny(message.Text, "\r\n") {
+			t.Fatal("Send queued an unsafe message")
+		}
+	}
+}

docs/plugins.md

@@ -430,16 +430,26 @@ short link:
 !youtube Linux server setup
 ~~~
 
-The response is labeled `[YouTube]`, includes the channel, title, and a
-`https://youtu.be/...` link. When `BOT_YOUTUBE_API_KEY` is configured, GoBot
+The response is labeled `[YouTube]`, includes the title, channel when available,
+and a `https://youtu.be/...` link. When `BOT_YOUTUBE_API_KEY` is configured, GoBot
 also adds the video's public view and like totals when YouTube exposes them.
 The statistics are best-effort: a missing like count, an API limitation, or a
 temporary statistics lookup failure does not prevent the search result from
 being returned. The command searches video results, which includes music
 videos and other YouTube video content. GoBot uses the key for the official
 Data API search and statistics lookup, then falls back to YouTube's public
-results page when the key is unavailable or the API cannot be used. Configure
-`plugins.youtube.max_length` and `plugins.youtube.timeout_seconds` as needed.
+results page when the key is unavailable or the API cannot be used. If that
+page provides no usable video results (including consent or age-confirmation
+pages), a public web search restricted to YouTube watch links supplies an
+indexed video title and short link. Only validated YouTube video URLs are
+accepted; optional oEmbed metadata adds the channel when available. GoBot
+also retries the explicit title portion of `artist - title` searches when the
+full indexed query has no usable match (for example, a misspelled artist).
+This does not bypass sign-in or age restrictions for viewing a video. Each earlier
+provider has a bounded time budget so it cannot consume the entire fallback
+deadline. Titles, bylines, and statistics are shortened or omitted as needed
+to preserve the video link and IRC byte limit, including Unicode replies.
+Configure `plugins.youtube.max_length` and `plugins.youtube.timeout_seconds` as needed.
 The API key is optional, but improves search reliability and avoids depending
 on changes to YouTube's public results HTML.
 
@@ -452,6 +462,9 @@ The `cve` plugin queries the NVD's public CVE 2.0 API and requires no API key:
 !vuln CVE-2024-3094
 ~~~
 
+The shared `!vuln` alias routes CVE identifiers here and package queries such as
+`!vuln npm lodash` to the package-audit plugin when that plugin is enabled.
+
 GoBot returns the CVE ID, the best available CVSS score and severity, up to
 three affected vendor/product/version labels, and the NVD detail link. NVD
 records may not have a score yet, and CPE applicability data can be broad, so
@@ -1469,8 +1482,12 @@ project--
 !reddit https://www.reddit.com/r/example/comments/abc123/post/
 ~~~
 
-- seen reports where and when a nickname last spoke. Records are stored in
-  BoltDB.
+- seen reports where and when a nickname last spoke in the current channel.
+  Records are scoped to both network and channel in BoltDB; private messages
+  are never stored, and lookups in private messages or other channels cannot
+  disclose channel history. Legacy unscoped records are not displayed because
+  their origin cannot be verified. A new channel message establishes a safely
+  scoped record after upgrading.
 - tell queues a message and delivers it when the addressed nickname next speaks.
 - karma tracks case-insensitive thing++ and thing-- changes.
 - luv awards the named nickname one persistent blue-heart point with `!luv

plugins/ask.go

@@ -1259,9 +1259,12 @@ func unwrapBingResultURL(raw string) string {
 	if !strings.HasPrefix(encoded, "a1") {
 		return raw
 	}
-	decoded, err := base64.RawStdEncoding.DecodeString(encoded[2:])
-	if err != nil {
-		decoded, err = base64.StdEncoding.DecodeString(encoded[2:])
+	var decoded []byte
+	for _, encoding := range []*base64.Encoding{base64.RawURLEncoding, base64.URLEncoding, base64.RawStdEncoding, base64.StdEncoding} {
+		decoded, err = encoding.DecodeString(encoded[2:])
+		if err == nil {
+			break
+		}
 	}
 	if err != nil || !validPublicHTTPURL(string(decoded)) {
 		return raw

plugins/cve.go

@@ -68,6 +68,11 @@ func (p *CVE) Handle(b *bot.Bot, m bot.Message) bool {
 		return false
 	}
 	cveID := strings.ToUpper(strings.TrimSpace(arg))
+	// The package-audit plugin shares !vuln. Preserve CVE lookups while letting
+	// non-CVE arguments reach that plugin instead of consuming them as errors.
+	if cmd == "vuln" && !strings.HasPrefix(cveID, "CVE-") {
+		return false
+	}
 	if !cveIDPattern.MatchString(cveID) {
 		b.Send(m.ReplyTarget(), ircColor(ircYellow, "usage: !cve CVE-YYYY-NNNN; browse/search CVEs: "+nvdSearchURL))
 		return true

plugins/cve_test.go

@@ -1,12 +1,49 @@
 package plugins
 
 import (
+	"context"
 	"io"
 	"net/http"
 	"strings"
 	"testing"
+
+	"github.com/variablenix/GoBot/bot"
 )
 
+func TestVulnRoutesCVEAndPackageQueries(t *testing.T) {
+	oldCVE, oldAPI := cveHTTPClient, apiHTTPClient
+	t.Cleanup(func() { cveHTTPClient, apiHTTPClient = oldCVE, oldAPI })
+	cveHTTPClient = &http.Client{Transport: cveRoundTripper(func(*http.Request) (*http.Response, error) {
+		return cveTestResponse(200, `{"vulnerabilities":[{"cve":{"id":"CVE-2024-1234","descriptions":[{"lang":"en","value":"Example issue"}]}}]}`), nil
+	})}
+	apiHTTPClient = &http.Client{Transport: newPluginRoundTripper(func(r *http.Request) (*http.Response, error) {
+		if r.URL.Host == "api.osv.dev" {
+			return newPluginResponse(200, `{"vulns":[]}`), nil
+		}
+		return newPluginResponse(200, `{"name":"example","version":"1.0.0"}`), nil
+	})}
+	cve, audit := &CVE{}, &Audit{}
+	cve.Init(nil, nil)
+	audit.Init(nil, nil)
+	sent := make(chan string, 4)
+	b := &bot.Bot{Config: bot.Config{CommandPrefix: "!"}, Queue: bot.NewQueue(1, 1, func(m bot.Outgoing) { sent <- m.Text })}
+	defer b.Queue.Drain(context.Background())
+	for _, command := range []string{"!vuln CVE-2024-1234", "!vuln npm example"} {
+		m := bot.Message{Nick: "tester", Target: "Echo", Text: command}
+		if command == "!vuln npm example" {
+			if cve.Handle(b, m) || !audit.Handle(b, m) {
+				t.Fatal("package query was not routed to audit")
+			}
+		} else if !cve.Handle(b, m) {
+			t.Fatal("CVE alias stopped working")
+		}
+	}
+	b.Queue.Drain(context.Background())
+	if len(sent) != 2 || !strings.Contains(<-sent, "CVE-2024-1234") || !strings.Contains(<-sent, "no known vulnerabilities") {
+		t.Fatal("shared alias did not produce the expected replies")
+	}
+}
+
 type cveRoundTripper func(*http.Request) (*http.Response, error)
 
 func (f cveRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) }

plugins/github.go

@@ -423,7 +423,7 @@ func (p *GitHub) getJSON(ctx context.Context, path string, destination any) erro
 	if token := strings.TrimSpace(p.cfg.String("token", "")); token != "" {
 		req.Header.Set("Authorization", "Bearer "+token)
 	}
-	res, err := apiHTTPClient.Do(req)
+	res, err := authenticatedAPIRequest(req)
 	if err != nil {
 		return err
 	}

plugins/ipinfo.go

@@ -100,6 +100,7 @@ func lookupIP(ctx context.Context, query string) (ipLookup, error) {
 		select {
 		case <-ctx.Done():
 			timer.Stop()
+			ipRequestMu.Unlock()
 			return ipLookup{}, ctx.Err()
 		case <-timer.C:
 		}

plugins/ipinfo_test.go

@@ -1,6 +1,7 @@
 package plugins
 
 import (
+	"context"
 	"io"
 	"net/http"
 	"strings"
@@ -59,3 +60,26 @@ func TestValidIPQueryRejectsURLLikeInput(t *testing.T) {
 		}
 	}
 }
+
+func TestIPRateLimitCancellationReleasesLock(t *testing.T) {
+	oldClient, oldLast := ipHTTPClient, ipLastRequest
+	t.Cleanup(func() { ipHTTPClient, ipLastRequest = oldClient, oldLast })
+	ipLastRequest = time.Now()
+	ctx, cancel := context.WithTimeout(t.Context(), time.Millisecond)
+	defer cancel()
+	if _, err := lookupIP(ctx, "8.8.8.8"); err == nil {
+		t.Fatal("cancelled rate-limit wait succeeded")
+	}
+	if !ipRequestMu.TryLock() {
+		ipRequestMu.Unlock() // Leave subsequent tests usable on regression.
+		t.Fatal("cancelled wait left all future IP lookups locked")
+	}
+	ipLastRequest = time.Time{}
+	ipRequestMu.Unlock()
+	ipHTTPClient = &http.Client{Transport: ipRoundTripper(func(r *http.Request) (*http.Response, error) {
+		return ipTestResponse(200, `{"status":"success","query":"8.8.8.8"}`), nil
+	})}
+	if _, err := lookupIP(t.Context(), "8.8.8.8"); err != nil {
+		t.Fatalf("next lookup did not recover: %v", err)
+	}
+}

plugins/lastfm.go

@@ -52,7 +52,7 @@ func (p *LastFM) Handle(b *bot.Bot, m bot.Message) bool {
 	values := url.Values{"method": {"user.getrecenttracks"}, "user": {username}, "api_key": {apiKey}, "format": {"json"}, "limit": {"1"}}
 	req, _ := http.NewRequestWithContext(ctx, http.MethodGet, "https://ws.audioscrobbler.com/2.0/?"+values.Encode(), nil)
 	req.Header.Set("User-Agent", "GoBot/1.0 (IRC bot)")
-	res, err := apiHTTPClient.Do(req)
+	res, err := authenticatedAPIRequest(req)
 	if err != nil || res.StatusCode != http.StatusOK {
 		if res != nil {
 			res.Body.Close()

plugins/newplugin_helpers.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"math/big"
+	"net/http"
 	"os"
 	"strings"
 	"sync"
@@ -14,6 +15,21 @@ import (
 	"github.com/variablenix/GoBot/bot"
 )
 
+// Do not let upstream redirects forward API keys or bearer tokens to another
+// origin (including a subdomain), or downgrade an authenticated HTTPS request.
+// A per-request copy preserves the shared transport and test injection without
+// mutating the client used concurrently by unrelated plugins.
+func authenticatedAPIRequest(req *http.Request) (*http.Response, error) {
+	client := *apiHTTPClient
+	client.CheckRedirect = func(next *http.Request, via []*http.Request) error {
+		if len(via) >= 10 || next.URL.Scheme != req.URL.Scheme || !strings.EqualFold(next.URL.Host, req.URL.Host) {
+			return http.ErrUseLastResponse
+		}
+		return nil
+	}
+	return client.Do(req)
+}
+
 // scopedCooldown is deliberately local to the plugin. Bot.AllowCommand gives
 // every command a sender cooldown, while these cooldowns express the tighter
 // channel/user scopes required by individual games.