From e37e1ca86eb8d5c54d6ad04fe18b2185bb57fe11 Mon Sep 17 00:00:00 2001 From: ak Date: Thu, 1 Oct 2026 21:12:40 -0700 Subject: [PATCH 1/4] Fix YouTube search recovery and harden plugin reliability --- bot/bot.go | 25 +++- bot/outgoing.go | 65 +++++++++ bot/outgoing_test.go | 67 +++++++++ docs/plugins.md | 23 +++- plugins/ask.go | 9 +- plugins/cve.go | 5 + plugins/cve_test.go | 37 +++++ plugins/github.go | 2 +- plugins/ipinfo.go | 1 + plugins/ipinfo_test.go | 24 ++++ plugins/lastfm.go | 2 +- plugins/newplugin_helpers.go | 16 +++ plugins/news.go | 2 +- plugins/paste.go | 2 +- plugins/plugin_audit_test.go | 85 ++++++++++++ plugins/poll.go | 11 +- plugins/poll_test.go | 19 ++- plugins/provider_live_test.go | 81 +++++++++++ plugins/reminder.go | 5 +- plugins/reminder_test.go | 25 ++++ plugins/seen.go | 25 +++- plugins/seen_test.go | 48 +++++++ plugins/weather.go | 6 + plugins/weather_test.go | 41 ++++++ plugins/youtube.go | 248 ++++++++++++++++++++++++++-------- plugins/youtube_test.go | 169 +++++++++++++++++++++++ 26 files changed, 960 insertions(+), 83 deletions(-) create mode 100644 bot/outgoing.go create mode 100644 bot/outgoing_test.go create mode 100644 plugins/plugin_audit_test.go create mode 100644 plugins/provider_live_test.go diff --git a/bot/bot.go b/bot/bot.go index b3cf6b0..915f8d1 100644 --- a/bot/bot.go +++ b/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) { diff --git a/bot/outgoing.go b/bot/outgoing.go new file mode 100644 index 0000000..14b4e6c --- /dev/null +++ b/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 +} diff --git a/bot/outgoing_test.go b/bot/outgoing_test.go new file mode 100644 index 0000000..b5ee7bc --- /dev/null +++ b/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") + } + } +} diff --git a/docs/plugins.md b/docs/plugins.md index 6cc1ef2..848c1dd 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -430,7 +430,7 @@ short link: !youtube Linux server setup ~~~ -The response is labeled `[YouTube]`, includes the channel, title, and a +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 @@ -438,7 +438,16 @@ 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 +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. 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 +461,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 +1481,11 @@ 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 a channel on the current + IRC network. Records are stored in BoltDB; private messages are never stored + or disclosed. Legacy nickname-only records are not displayed because their + network and public/private 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 diff --git a/plugins/ask.go b/plugins/ask.go index df091cc..098e105 100644 --- a/plugins/ask.go +++ b/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 diff --git a/plugins/cve.go b/plugins/cve.go index a43b84d..3052589 100644 --- a/plugins/cve.go +++ b/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 diff --git a/plugins/cve_test.go b/plugins/cve_test.go index 9a45b8b..d32c355 100644 --- a/plugins/cve_test.go +++ b/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) } diff --git a/plugins/github.go b/plugins/github.go index b5b9c85..bc3d0ed 100644 --- a/plugins/github.go +++ b/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 } diff --git a/plugins/ipinfo.go b/plugins/ipinfo.go index 8d17097..367c3ae 100644 --- a/plugins/ipinfo.go +++ b/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: } diff --git a/plugins/ipinfo_test.go b/plugins/ipinfo_test.go index 2c5bc00..ced6ad7 100644 --- a/plugins/ipinfo_test.go +++ b/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) + } +} diff --git a/plugins/lastfm.go b/plugins/lastfm.go index 0f9c4c5..936ad1b 100644 --- a/plugins/lastfm.go +++ b/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() diff --git a/plugins/newplugin_helpers.go b/plugins/newplugin_helpers.go index 6eeebfa..3c4661e 100644 --- a/plugins/newplugin_helpers.go +++ b/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. diff --git a/plugins/news.go b/plugins/news.go index ee8f389..7965018 100644 --- a/plugins/news.go +++ b/plugins/news.go @@ -44,7 +44,7 @@ func (p *News) Handle(b *bot.Bot, m bot.Message) bool { return true } req.Header.Set("X-Api-Key", p.cfg.String("api_key", "")) - res, err := apiHTTPClient.Do(req) + res, err := authenticatedAPIRequest(req) if err != nil || res.StatusCode != http.StatusOK { if res != nil { res.Body.Close() diff --git a/plugins/paste.go b/plugins/paste.go index e8b60b6..302f9b0 100644 --- a/plugins/paste.go +++ b/plugins/paste.go @@ -228,7 +228,7 @@ func (p *Paste) createPaste(parent context.Context, content string) (string, err req.Header.Set("Authorization", "token "+p.token) req.Header.Set("Content-Type", "application/json") req.Header.Set("Accept", "application/json") - res, err := apiHTTPClient.Do(req) + res, err := authenticatedAPIRequest(req) if err != nil { return "", err } diff --git a/plugins/plugin_audit_test.go b/plugins/plugin_audit_test.go new file mode 100644 index 0000000..b01ae7d --- /dev/null +++ b/plugins/plugin_audit_test.go @@ -0,0 +1,85 @@ +package plugins + +import ( + "context" + "net/http" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/variablenix/GoBot/bot" + "github.com/variablenix/GoBot/storage" +) + +// Exercise every registered plugin's initialization and command boundary. +// Provider-specific and stateful behavior remains covered by its own tests. +func TestAllPluginsInitializeAndKeepUnknownCommandAvailable(t *testing.T) { + db, err := storage.Open(filepath.Join(t.TempDir(), "audit.db")) + if err != nil { + t.Fatal(err) + } + defer db.Close() + b := &bot.Bot{Config: bot.Config{CommandPrefix: "!", NetworkName: "test"}, Queue: bot.NewQueue(1, 1, func(bot.Outgoing) {})} + defer b.Queue.Drain(context.Background()) + names, commands := map[string]bool{}, map[string]string{} + for _, p := range All() { + t.Run(p.Name(), func(t *testing.T) { + if names[p.Name()] || p.Name() == "" || p.Help() == "" { + t.Fatal("missing/duplicate plugin identity or help") + } + names[p.Name()] = true + for _, command := range p.Commands() { + if owner := commands[command]; owner != "" && !(command == "vuln" && owner == "cve" && p.Name() == "audit") { + t.Fatalf("command %q conflicts with %s", command, owner) + } + commands[command] = p.Name() + } + var config bot.PluginConfig + if p.Name() == "wordle" { + config = bot.PluginConfig{"words_file": "../data/wordle/words.txt"} + } + if err := p.Init(config, db); err != nil { + t.Fatal(err) + } + if p.Handle(b, bot.Message{Command: "PRIVMSG", Nick: "tester", Target: "#test", IsChannel: true, Text: "!__unknown_command__", Timestamp: time.Now()}) { + t.Fatal("plugin consumed another command") + } + if stopper, ok := p.(bot.Stopper); ok { + stopper.Stop(b) + } + }) + } + t.Logf("audited %d registered plugins and %d command aliases", len(names), len(commands)) +} + +func TestAuthenticatedAPIRedirectsKeepCredentialsOnOrigin(t *testing.T) { + old := apiHTTPClient + t.Cleanup(func() { apiHTTPClient = old }) + for _, destination := range []string{"https://other.example/private", "https://sub.api.example/private", "http://api.example/private", "https://api.example/next"} { + requests := 0 + apiHTTPClient = &http.Client{Transport: newPluginRoundTripper(func(r *http.Request) (*http.Response, error) { + requests++ + if requests == 1 { + res := newPluginResponse(302, "") + res.Header.Set("Location", destination) + return res, nil + } + return newPluginResponse(200, `{}`), nil + })} + req, _ := http.NewRequestWithContext(t.Context(), "GET", "https://api.example/start", nil) + req.Header.Set("X-Api-Key", "synthetic-key") + res, err := authenticatedAPIRequest(req) + if err != nil { + t.Fatal(err) + } + res.Body.Close() + want := 1 + if strings.HasPrefix(destination, "https://api.example/") { + want = 2 + } + if requests != want { + t.Fatalf("redirect to %s issued %d requests, want %d", destination, requests, want) + } + } +} diff --git a/plugins/poll.go b/plugins/poll.go index 57fa9d0..974c8e6 100644 --- a/plugins/poll.go +++ b/plugins/poll.go @@ -123,6 +123,9 @@ func (p *Poll) vote(key, nick, rawOption string) string { if err != nil || option < 1 || option > len(current.Options) { return fmt.Sprintf("vote with a number from 1 to %d", len(current.Options)) } + if current.Votes == nil { + current.Votes = make(map[string]int) + } current.Votes[strings.ToLower(nick)] = option p.persist(key) return fmt.Sprintf("vote recorded for option %d", option) @@ -159,12 +162,16 @@ func mustList(db *storage.DB, bucket string) []string { func formatPoll(current *poll) string { counts := make([]int, len(current.Options)) + votes := 0 for _, option := range current.Votes { - counts[option-1]++ + if option >= 1 && option <= len(counts) { + counts[option-1]++ + votes++ + } } parts := make([]string, len(current.Options)) for i, option := range current.Options { parts[i] = fmt.Sprintf("%d) %s [%d]", i+1, option, counts[i]) } - return fmt.Sprintf("Poll: %s — %s (%d votes)", current.Question, strings.Join(parts, "; "), len(current.Votes)) + return fmt.Sprintf("Poll: %s — %s (%d votes)", current.Question, strings.Join(parts, "; "), votes) } diff --git a/plugins/poll_test.go b/plugins/poll_test.go index 6864c78..99d315e 100644 --- a/plugins/poll_test.go +++ b/plugins/poll_test.go @@ -1,6 +1,9 @@ package plugins -import "testing" +import ( + "strings" + "testing" +) func TestPollCreateAndVote(t *testing.T) { p := &Poll{} @@ -31,3 +34,17 @@ func TestPollRejectsInvalidOptions(t *testing.T) { t.Fatalf("got %q", got) } } + +func TestPollInvalidStoredVotesDoNotPanic(t *testing.T) { + current := &poll{Question: "Example?", Options: []string{"A", "B"}, Votes: map[string]int{"valid": 1, "zero": 0, "large": 999, "negative": -1}} + if result := formatPoll(current); !strings.Contains(result, "(1 votes)") { + t.Fatal("expected usable results despite invalid stored votes") + } +} + +func TestPollCanVoteWithMissingStoredVotes(t *testing.T) { + p := &Poll{active: map[string]*poll{"example": {Question: "Example?", Options: []string{"A", "B"}}}} + if response := p.vote("example", "tester", "1"); response != "vote recorded for option 1" { + t.Fatalf("vote failed: %s", response) + } +} diff --git a/plugins/provider_live_test.go b/plugins/provider_live_test.go new file mode 100644 index 0000000..5cf7fa6 --- /dev/null +++ b/plugins/provider_live_test.go @@ -0,0 +1,81 @@ +package plugins + +import ( + "context" + "os" + "strings" + "testing" + "unicode/utf8" + + "github.com/variablenix/GoBot/bot" +) + +// Run from the deployment host explicitly. These call providers through the +// real command handlers but never connect to IRC or send to live channels. +// Keep normal CI independent of API keys, rate limits, and upstream outages. +func TestLivePluginCommands(t *testing.T) { + if os.Getenv("GOBOT_LIVE_PLUGINS") != "1" { + t.Skip("opt-in live plugin smoke test") + } + cases := []struct { + plugin bot.Plugin + command, want, token string + }{ + {&Wikipedia{}, "!wiki Linux", "wikipedia.org", ""}, + {&Weather{}, "!weather London", "London", ""}, + {&IMDb{}, "!imdb The Matrix", "imdb.com/title/", ""}, + {&Linux{}, "!linux", "kernel", ""}, + {&Cats{}, "!cat", "Cat fact", ""}, + {&XKCD{}, "!xkcd 1", "xkcd.com/1", ""}, + {&Urban{}, "!urban hello", "urbandictionary.com", ""}, + {&Dadjoke{}, "!dadjoke", "", ""}, + {&GitHub{}, "!github golang/go", "github.com/golang/go", ""}, + {&Pkg{}, "!pkg npm lodash", "lodash", ""}, + {&Docker{}, "!docker alpine", "hub.docker.com", ""}, + {&CVE{}, "!cve CVE-2024-3094", "CVE-2024-3094", ""}, + {&Reddit{}, "!reddit linux", "reddit.com", ""}, + {&Steam{}, "!steam Portal 2", "steampowered.com", ""}, + {&Sports{}, "!sports", "Sports pick:", ""}, + {&News{}, "!news technology", "http", "BOT_NEWS_API_KEY"}, + {&Lyrics{}, "!lyrics electric wizard Dopethrone", "genius.com", "BOT_GENIUS_ACCESS_TOKEN"}, + } + for _, test := range cases { + t.Run(test.plugin.Name(), func(t *testing.T) { + if test.token != "" && strings.TrimSpace(os.Getenv(test.token)) == "" { + t.Skip("provider credential not configured") + } + cfg := bot.PluginConfig{"timeout_seconds": 8} + if test.token != "" { + cfg["api_key"] = os.Getenv(test.token) + } + if err := test.plugin.Init(cfg, nil); err != nil { + t.Fatal(err) + } + sent := make(chan string, 20) + b := &bot.Bot{Config: bot.Config{CommandPrefix: "!", NetworkName: "test"}, Queue: bot.NewQueue(1, 1, func(m bot.Outgoing) { sent <- m.Text })} + defer b.Queue.Drain(context.Background()) + if !test.plugin.Handle(b, bot.Message{Command: "PRIVMSG", Nick: "tester", Target: "#test", IsChannel: true, Text: test.command}) { + t.Fatal("command not handled") + } + b.Queue.Drain(context.Background()) + var replies []string + for len(sent) > 0 { + text := <-sent + if !utf8.ValidString(text) || len("PRIVMSG #test :"+text+"\r\n") > 512 || strings.ContainsAny(text, "\r\n\x00") { + t.Fatal("invalid IRC reply") + } + replies = append(replies, stripPluginIRC(text)) + } + joined := strings.Join(replies, " ") + lower := strings.ToLower(joined) + for _, failure := range []string{"unavailable", "not configured", "not found", "no results", "usage:", "failed"} { + if strings.Contains(lower, failure) { + t.Fatalf("provider did not return a result: %s", joined) + } + } + if joined == "" || !strings.Contains(lower, strings.ToLower(test.want)) { + t.Fatalf("expected result marker %q, got %s", test.want, joined) + } + }) + } +} diff --git a/plugins/reminder.go b/plugins/reminder.go index a8037bc..c062bf3 100644 --- a/plugins/reminder.go +++ b/plugins/reminder.go @@ -125,7 +125,10 @@ func (p *Reminder) Handle(b *bot.Bot, m bot.Message) bool { p.mu.Unlock() saved := reminderRecord{ID: fmt.Sprintf("%d", time.Now().UnixNano()), Network: b.Config.NetworkName, Target: m.ReplyTarget(), Nick: m.Nick, Message: message, DueAt: time.Now().Add(duration)} if p.db != nil { - _ = p.db.Set("reminders", saved.ID, saved) + if err := p.db.Set("reminders", saved.ID, saved); err != nil { + b.Send(m.ReplyTarget(), ircColor(ircRed, "could not save the reminder; please try again")) + return true + } } p.schedule(b, saved) b.Send(m.ReplyTarget(), ircColor(ircGreen, "reminder set for "+formatReminderDuration(duration))) diff --git a/plugins/reminder_test.go b/plugins/reminder_test.go index 91683c6..c972625 100644 --- a/plugins/reminder_test.go +++ b/plugins/reminder_test.go @@ -1,8 +1,14 @@ package plugins import ( + "context" + "path/filepath" + "strings" "testing" "time" + + "github.com/variablenix/GoBot/bot" + "github.com/variablenix/GoBot/storage" ) func TestFormatReminderDuration(t *testing.T) { @@ -13,3 +19,22 @@ func TestFormatReminderDuration(t *testing.T) { t.Fatalf("got %q", got) } } + +func TestReminderStorageFailureDoesNotClaimSuccess(t *testing.T) { + db, err := storage.Open(filepath.Join(t.TempDir(), "reminder.db")) + if err != nil { + t.Fatal(err) + } + p := &Reminder{} + p.Init(nil, db) + db.Close() + sent := make(chan string, 2) + b := &bot.Bot{Config: bot.Config{CommandPrefix: "!"}, Queue: bot.NewQueue(1, 1, func(message bot.Outgoing) { sent <- message.Text })} + if !p.Handle(b, bot.Message{Nick: "tester", Target: "Echo", Text: "!remind 30m check logs"}) { + t.Fatal("reminder command was not handled") + } + b.Queue.Drain(context.Background()) + if reply := <-sent; !strings.Contains(reply, "could not save") || len(p.items) != 0 { + t.Fatalf("failed storage still scheduled/confirmed reminder: %q", reply) + } +} diff --git a/plugins/seen.go b/plugins/seen.go index cc1cfca..b2ab7b2 100644 --- a/plugins/seen.go +++ b/plugins/seen.go @@ -18,25 +18,34 @@ type record struct { type Seen struct{ db *storage.DB } -func (p *Seen) Name() string { return "seen" } -func (p *Seen) Commands() []string { return []string{"seen"} } -func (p *Seen) Help() string { return "!seen — show when someone last spoke" } +func (p *Seen) Name() string { return "seen" } +func (p *Seen) Commands() []string { return []string{"seen"} } +func (p *Seen) Help() string { + return "!seen — show when someone last spoke in a channel on this network" +} func (p *Seen) Init(_ bot.PluginConfig, d *storage.DB) error { p.db = d; return nil } func (p *Seen) Handle(b *bot.Bot, m bot.Message) bool { - if m.Command == "PRIVMSG" && m.Nick != "" { - _ = p.db.Set("seen", strings.ToLower(m.Nick), record{m.Nick, m.Target, normalizeSeenText(m.Nick, m.Text), m.Timestamp}) + if m.Command == "PRIVMSG" && m.IsChannel && m.Nick != "" && p.db != nil { + _ = p.db.Set("seen", seenKey(b.Config.NetworkName, m.Nick), record{m.Nick, m.Target, cleanExternalText(normalizeSeenText(m.Nick, m.Text)), m.Timestamp}) } cmd, arg, ok := bot.IsCommand(m, b.Config.CommandPrefix) if !ok || cmd != "seen" { return false } - v, e := p.db.Get("seen", strings.ToLower(strings.TrimSpace(arg))) + if p.db == nil { + b.Send(m.ReplyTarget(), "Seen storage is unavailable.") + return true + } + // Legacy nickname-only records may contain private messages and cannot be + // assigned to a network safely. Leave them untouched, but never disclose them. + v, e := p.db.Get("seen", seenKey(b.Config.NetworkName, strings.TrimSpace(arg))) if e != nil { b.Send(m.ReplyTarget(), "I haven't seen that nick yet.") return true } var x record - if err := json.Unmarshal(v, &x); err != nil || strings.TrimSpace(x.Nick) == "" { + if err := json.Unmarshal(v, &x); err != nil || strings.TrimSpace(x.Nick) == "" || + (!strings.HasPrefix(x.Channel, "#") && !strings.HasPrefix(x.Channel, "&")) { b.Send(m.ReplyTarget(), "That seen record is unavailable.") return true } @@ -44,6 +53,8 @@ func (p *Seen) Handle(b *bot.Bot, m bot.Message) bool { return true } +func seenKey(network, nick string) string { return scopedKey(network, "", nick) } + func normalizeSeenText(nick, text string) string { if len(text) >= 2 && text[0] == '\x01' && text[len(text)-1] == '\x01' { content := text[1 : len(text)-1] diff --git a/plugins/seen_test.go b/plugins/seen_test.go index 09f819c..890e5da 100644 --- a/plugins/seen_test.go +++ b/plugins/seen_test.go @@ -1,8 +1,14 @@ package plugins import ( + "context" + "path/filepath" + "strings" "testing" "time" + + "github.com/variablenix/GoBot/bot" + "github.com/variablenix/GoBot/storage" ) func TestFormatSeenAge(t *testing.T) { @@ -27,6 +33,48 @@ func TestFormatSeenAge(t *testing.T) { } } +func TestSeenDoesNotExposePrivateOrCrossNetworkRecords(t *testing.T) { + db, err := storage.Open(filepath.Join(t.TempDir(), "seen.db")) + if err != nil { + t.Fatal(err) + } + defer db.Close() + sent := make(chan string, 10) + b := &bot.Bot{Config: bot.Config{CommandPrefix: "!", NetworkName: "first"}, Queue: bot.NewQueue(1, 1, func(message bot.Outgoing) { sent <- message.Text })} + defer b.Queue.Drain(context.Background()) + p := &Seen{} + p.Init(nil, db) + p.Handle(b, bot.Message{Command: "PRIVMSG", Nick: "Alice", Target: "Echo", Text: "private content", Timestamp: time.Now()}) + if _, err := db.Get("seen", seenKey("first", "Alice")); err != storage.ErrNotFound { + t.Fatal("private message was persisted") + } + p.Handle(b, bot.Message{Command: "PRIVMSG", Nick: "Alice", Target: "#test", IsChannel: true, Text: "public greeting", Timestamp: time.Now()}) + if !p.Handle(b, bot.Message{Nick: "tester", Target: "Echo", Text: "!seen Alice"}) { + t.Fatal("seen command was not handled") + } + b.Queue.Drain(context.Background()) + if len(sent) != 1 || !strings.Contains(<-sent, "public greeting") { + t.Fatal("same-network public record was not returned") + } + if _, err := db.Get("seen", seenKey("second", "Alice")); err != storage.ErrNotFound { + t.Fatal("network isolation failed") + } + // Legacy records cannot distinguish networks or prove a public origin. + db.Set("seen", "bob", record{Nick: "Bob", Channel: "Echo", Text: "legacy private content"}) + p.Handle(b, bot.Message{Nick: "tester", Target: "Echo", Text: "!seen Bob"}) + b.Config.NetworkName = "second" + p.Handle(b, bot.Message{Nick: "tester", Target: "Echo", Text: "!seen Alice"}) + b.Queue.Drain(context.Background()) + if len(sent) != 2 { + t.Fatalf("expected two lookup replies, got %d", len(sent)) + } + for len(sent) > 0 { + if reply := <-sent; strings.Contains(reply, "private content") || strings.Contains(reply, "public greeting") { + t.Fatalf("private or cross-network data leaked: %q", reply) + } + } +} + func TestNormalizeSeenTextForCTCPAction(t *testing.T) { tests := []struct { name string diff --git a/plugins/weather.go b/plugins/weather.go index 8822a44..8c15d7a 100644 --- a/plugins/weather.go +++ b/plugins/weather.go @@ -114,6 +114,9 @@ func (p *Weather) Handle(b *bot.Bot, m bot.Message) bool { req, _ := http.NewRequestWithContext(requestCtx, http.MethodGet, geocodeURL, nil) res, err := apiHTTPClient.Do(req) if err != nil || res.StatusCode != 200 { + if res != nil { + res.Body.Close() + } b.Send(m.ReplyTarget(), ircColor(ircRed, "I couldn't find weather for that city.")) return true } @@ -132,6 +135,9 @@ func (p *Weather) Handle(b *bot.Bot, m bot.Message) bool { req, _ = http.NewRequestWithContext(requestCtx, http.MethodGet, forecastURL, nil) res, err = apiHTTPClient.Do(req) if err != nil || res.StatusCode != 200 { + if res != nil { + res.Body.Close() + } b.Send(m.ReplyTarget(), ircColor(ircRed, "Weather data is temporarily unavailable.")) return true } diff --git a/plugins/weather_test.go b/plugins/weather_test.go index 78ea36e..a459734 100644 --- a/plugins/weather_test.go +++ b/plugins/weather_test.go @@ -1,6 +1,9 @@ package plugins import ( + "context" + "io" + "net/http" "path/filepath" "strings" "testing" @@ -105,3 +108,41 @@ func TestWeatherHelpDocumentsDefaults(t *testing.T) { } } } + +type weatherTrackingBody struct { + io.Reader + closed bool +} + +func (b *weatherTrackingBody) Close() error { b.closed = true; return nil } + +func TestWeatherClosesFailedProviderResponses(t *testing.T) { + old := apiHTTPClient + t.Cleanup(func() { apiHTTPClient = old }) + for _, failedHost := range []string{"geocoding-api.open-meteo.com", "api.open-meteo.com"} { + t.Run(failedHost, func(t *testing.T) { + var bodies []*weatherTrackingBody + apiHTTPClient = &http.Client{Transport: newPluginRoundTripper(func(r *http.Request) (*http.Response, error) { + status, payload := 200, `{"results":[{"name":"Example City","country_code":"US","latitude":1,"longitude":1}]}` + if r.URL.Host == failedHost { + status, payload = 503, `{}` + } + body := &weatherTrackingBody{Reader: strings.NewReader(payload)} + bodies = append(bodies, body) + return &http.Response{StatusCode: status, Header: http.Header{}, Body: body}, nil + })} + b := &bot.Bot{Config: bot.Config{CommandPrefix: "!"}, Queue: bot.NewQueue(1, 1, func(bot.Outgoing) {})} + defer b.Queue.Drain(context.Background()) + p := &Weather{} + p.Init(nil, nil) + if !p.Handle(b, bot.Message{Text: "!weather Example City", Nick: "tester", Target: "Echo"}) || len(bodies) == 0 { + t.Fatal("weather command did not exercise the provider") + } + for _, body := range bodies { + if !body.closed { + t.Fatal("provider error leaked its response body") + } + } + }) + } +} diff --git a/plugins/youtube.go b/plugins/youtube.go index 2ea2c61..4c4efcc 100644 --- a/plugins/youtube.go +++ b/plugins/youtube.go @@ -8,6 +8,8 @@ import ( "io" "net/http" "net/url" + "regexp" + "sort" "strconv" "strings" "time" @@ -23,7 +25,12 @@ const ( youtubeMaxQuery = 120 ) -var youtubeHTTPClient = &http.Client{Timeout: 10 * time.Second} +var youtubeHTTPClient = &http.Client{ + Timeout: 10 * time.Second, + CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }, +} + +var youtubeDataAssignment = regexp.MustCompile(`(?:\b(?:var|let|const)\s+ytInitialData|window\s*\[\s*["']ytInitialData["']\s*\]|\bytInitialData)\s*=\s*`) type YouTube struct { apiKey string @@ -79,17 +86,35 @@ func (p *YouTube) Handle(b *bot.Bot, m bot.Message) bool { b.Send(m.ReplyTarget(), ircColor(ircRed, "YouTube search is temporarily unavailable")) return true } - b.Send(m.ReplyTarget(), formatYouTubeSearchResult(result, p.maxLength)) + b.Send(m.ReplyTarget(), formatYouTubeSearchResultForTarget(result, p.maxLength, m.ReplyTarget())) return true } func (p *YouTube) search(ctx context.Context, query string) (youtubeSearchResult, error) { if p.apiKey != "" { - if result, err := p.searchAPI(ctx, query); err == nil { + step, cancel := youtubeStepContext(ctx, 2*time.Second) + result, err := p.searchAPI(step, query) + cancel() + if err == nil { return result, nil } } - return p.searchPage(ctx, query) + step, cancel := youtubeStepContext(ctx, 5*time.Second) + result, err := p.searchPage(step, query) + cancel() + if err == nil { + return result, nil + } + // Public search can return a consent/age-confirmation page with HTTP 200. + // Use indexed video metadata, not login or age-check circumvention. + return p.searchIndex(ctx, query) +} + +func youtubeStepContext(ctx context.Context, limit time.Duration) (context.Context, context.CancelFunc) { + if deadline, ok := ctx.Deadline(); ok && time.Until(deadline)/2 < limit { + limit = time.Until(deadline) / 2 + } + return context.WithTimeout(ctx, limit) } func (p *YouTube) searchAPI(ctx context.Context, query string) (youtubeSearchResult, error) { @@ -120,9 +145,11 @@ func (p *YouTube) searchAPI(ctx context.Context, query string) (youtubeSearchRes return youtubeSearchResult{}, err } for _, item := range response.Items { - result := youtubeSearchResult{VideoID: item.ID.VideoID, Title: cleanTitle(item.Snippet.Title), ChannelName: cleanTitle(item.Snippet.ChannelName)} + result := youtubeSearchResult{VideoID: item.ID.VideoID, Title: cleanYouTubeText(item.Snippet.Title), ChannelName: cleanYouTubeText(item.Snippet.ChannelName)} if validYouTubeSearchResult(result) { - p.addStatistics(ctx, result.VideoID, &result) + step, cancel := context.WithTimeout(ctx, time.Second) + p.addStatistics(step, result.VideoID, &result) + cancel() return result, nil } } @@ -179,26 +206,91 @@ func (p *YouTube) searchPage(ctx context.Context, query string) (youtubeSearchRe params := endpoint.Query() params.Set("search_query", query) endpoint.RawQuery = params.Encode() - req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint.String(), nil) + body, err := youtubeSearchHTML(ctx, endpoint.String()) if err != nil { return youtubeSearchResult{}, err } + return parseYouTubeInitialData(body) +} + +func youtubeSearchHTML(ctx context.Context, endpoint string) ([]byte, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) + if err != nil { + return nil, err + } req.Header.Set("Accept", "text/html,application/xhtml+xml") req.Header.Set("Accept-Language", "en-US,en;q=0.9") req.Header.Set("User-Agent", "Mozilla/5.0 (compatible; GoBot YouTube search)") res, err := youtubeHTTPClient.Do(req) if err != nil { - return youtubeSearchResult{}, err + return nil, err } defer res.Body.Close() if res.StatusCode != http.StatusOK { - return youtubeSearchResult{}, fmt.Errorf("YouTube returned HTTP %d", res.StatusCode) + return nil, fmt.Errorf("video search returned HTTP %d", res.StatusCode) } - body, err := io.ReadAll(io.LimitReader(res.Body, 4<<20)) + const maxBytes = 4 << 20 + body, err := io.ReadAll(io.LimitReader(res.Body, maxBytes+1)) + if err != nil { + return nil, err + } + if len(body) > maxBytes { + return nil, fmt.Errorf("video search response exceeds size limit") + } + return body, nil +} + +func (p *YouTube) searchIndex(ctx context.Context, query string) (youtubeSearchResult, error) { + endpoint := "https://www.bing.com/search?" + url.Values{ + "q": {"site:youtube.com/watch " + query}, "count": {"8"}, "setlang": {"en-US"}, + }.Encode() + body, err := youtubeSearchHTML(ctx, endpoint) if err != nil { return youtubeSearchResult{}, err } - return parseYouTubeInitialData(body) + for _, item := range parseBingSearchResults(body) { + id := youtubeIndexedVideoID(item.URL) + title := strings.TrimSuffix(strings.TrimSuffix(cleanYouTubeText(item.Title), " - YouTube Music"), " - YouTube") + result := youtubeSearchResult{VideoID: id, Title: title} + if !validYouTubeSearchResult(result) { + continue + } + // oEmbed can enrich the public result, but unavailable metadata must + // not discard an otherwise useful indexed video title and link. + step, cancel := context.WithTimeout(ctx, time.Second) + var metadata struct { + Title string `json:"title"` + Author string `json:"author_name"` + } + oembed := "https://www.youtube.com/oembed?" + url.Values{"url": {"https://www.youtube.com/watch?v=" + id}, "format": {"json"}}.Encode() + if p.getJSON(step, oembed, &metadata) == nil && cleanYouTubeText(metadata.Title) != "" { + result.Title = cleanYouTubeText(metadata.Title) + result.ChannelName = cleanYouTubeText(metadata.Author) + } + cancel() + return result, nil + } + return youtubeSearchResult{}, fmt.Errorf("no indexed YouTube video found") +} + +func youtubeIndexedVideoID(rawURL string) string { + u, err := url.Parse(rawURL) + if err != nil || u.Scheme != "https" || u.User != nil || u.Port() != "" { + return "" + } + var id string + switch strings.ToLower(u.Host) { + case "youtube.com", "www.youtube.com", "m.youtube.com", "music.youtube.com": + if u.Path == "/watch" && len(u.Query()["v"]) == 1 { + id = u.Query().Get("v") + } + case "youtu.be": + id = strings.TrimPrefix(u.Path, "/") + } + if !validYouTubeSearchResult(youtubeSearchResult{VideoID: id, Title: "video"}) { + return "" + } + return id } func (p *YouTube) getJSON(ctx context.Context, endpoint string, value interface{}) error { @@ -220,12 +312,11 @@ func (p *YouTube) getJSON(ctx context.Context, endpoint string, value interface{ } func parseYouTubeInitialData(body []byte) (youtubeSearchResult, error) { - marker := []byte("var ytInitialData = ") - start := bytes.Index(body, marker) - if start < 0 { + assignment := youtubeDataAssignment.FindIndex(body) + if assignment == nil { return youtubeSearchResult{}, fmt.Errorf("YouTube search data not found") } - jsonStart := start + len(marker) + jsonStart := assignment[1] decoder := json.NewDecoder(bytes.NewReader(body[jsonStart:])) var data interface{} if err := decoder.Decode(&data); err != nil { @@ -240,21 +331,46 @@ func parseYouTubeInitialData(body []byte) (youtubeSearchResult, error) { func findYouTubeVideo(value interface{}) (youtubeSearchResult, bool) { switch node := value.(type) { case map[string]interface{}: - if renderer, ok := node["videoRenderer"].(map[string]interface{}); ok { - result := youtubeSearchResult{ - VideoID: stringValue(renderer["videoId"]), - Title: cleanTitle(youtubeText(renderer["title"])), - ChannelName: cleanTitle(youtubeText(renderer["ownerText"])), + // Only search the primary results when the full page envelope exists. + // Sidebar recommendations must not win over the first organic result. + if primary, ok := youtubeObject(node, "contents", "twoColumnSearchResultsRenderer", "primaryContents"); ok { + return findYouTubeVideo(primary) + } + for _, kind := range []string{"videoRenderer", "videoWithContextRenderer"} { + if renderer, ok := node[kind].(map[string]interface{}); ok { + result := youtubeSearchResult{ + VideoID: stringValue(renderer["videoId"]), + Title: cleanYouTubeText(youtubeText(renderer["title"])), + ChannelName: cleanYouTubeText(youtubeText(renderer["ownerText"])), + } + if result.ChannelName == "" { + result.ChannelName = cleanYouTubeText(youtubeText(renderer["longBylineText"])) + } + if validYouTubeSearchResult(result) { + return result, true + } } - if result.ChannelName == "" { - result.ChannelName = cleanTitle(youtubeText(renderer["longBylineText"])) + } + if renderer, ok := node["lockupViewModel"].(map[string]interface{}); ok { + if stringValue(renderer["contentType"]) == "LOCKUP_CONTENT_TYPE_VIDEO" { + metadata, _ := youtubeObject(renderer, "metadata", "lockupMetadataViewModel") + result := youtubeSearchResult{VideoID: stringValue(renderer["contentId"]), Title: cleanYouTubeText(youtubeText(metadata["title"]))} + if validYouTubeSearchResult(result) { + return result, true + } } - if validYouTubeSearchResult(result) { - return result, true + return youtubeSearchResult{}, false // Do not mine playlist thumbnails. + } + keys := make([]string, 0, len(node)) + for key := range node { + if key == "adSlotRenderer" || key == "promotedSparklesWebRenderer" || key == "secondaryContents" { + continue } + keys = append(keys, key) } - for _, child := range node { - if result, ok := findYouTubeVideo(child); ok { + sort.Strings(keys) + for _, key := range keys { + if result, ok := findYouTubeVideo(node[key]); ok { return result, true } } @@ -268,6 +384,19 @@ func findYouTubeVideo(value interface{}) (youtubeSearchResult, bool) { return youtubeSearchResult{}, false } +func youtubeObject(node map[string]interface{}, path ...string) (map[string]interface{}, bool) { + for _, key := range path { + var ok bool + node, ok = node[key].(map[string]interface{}) + if !ok { + return nil, false + } + } + return node, true +} + +func cleanYouTubeText(text string) string { return cleanIMDbText(cleanTitle(text)) } + func youtubeText(value interface{}) string { node, ok := value.(map[string]interface{}) if !ok { @@ -276,6 +405,9 @@ func youtubeText(value interface{}) string { if text := stringValue(node["simpleText"]); text != "" { return text } + if text := stringValue(node["content"]); text != "" { + return text + } runs, ok := node["runs"].([]interface{}) if !ok { return "" @@ -297,7 +429,7 @@ func stringValue(value interface{}) string { } func validYouTubeSearchResult(result youtubeSearchResult) bool { - if result.Title == "" || result.VideoID == "" || len(result.VideoID) > 32 { + if result.Title == "" || len(result.VideoID) != 11 { return false } for _, r := range result.VideoID { @@ -309,31 +441,46 @@ func validYouTubeSearchResult(result youtubeSearchResult) bool { } func formatYouTubeSearchResult(result youtubeSearchResult, maxLength int) string { - link := "https://youtu.be/" + result.VideoID - prefix := "[YouTube]" - if result.ChannelName != "" { - prefix += " " + result.ChannelName + " —" - } - stats := youtubeStatsText(result) - fixedSuffix := " | " + link - if stats != "" { - fixedSuffix = " | " + stats + fixedSuffix - } - availableTitle := maxLength - len([]rune(prefix+" "+result.Title+fixedSuffix)) + len([]rune(result.Title)) - title := result.Title - if maxLength > 0 && availableTitle < len([]rune(title)) { - if availableTitle > 1 { - title = truncateRunes(title, availableTitle) - } else { - title = "" + return formatYouTubeSearchResultForTarget(result, maxLength, "") +} + +func formatYouTubeSearchResultForTarget(result youtubeSearchResult, maxLength int, target string) string { + result.Title = cleanYouTubeText(result.Title) + result.ChannelName = cleanYouTubeText(result.ChannelName) + // Include UTF-8 bytes, IRC color controls and the actual PRIVMSG envelope. + wireLimit := 512 - len("PRIVMSG "+target+" :\r\n") + if maxLength <= 0 || maxLength > wireLimit { + maxLength = wireLimit + } + for { + text := renderYouTubeSearchResult(result) + if len(text) <= maxLength { + return text + } + switch { + case result.ChannelName != "": + result.ChannelName = "" // Preserve title and link ahead of the byline. + case result.HasViewCount || result.HasLikeCount: + result.HasViewCount, result.HasLikeCount = false, false + case result.Title != "": + budget := len(result.Title) - (len(text) - maxLength) + result.Title = truncateUTF8Bytes(result.Title, budget) + default: + return "https://youtu.be/" + result.VideoID } } +} + +func renderYouTubeSearchResult(result youtubeSearchResult) string { + link := "https://youtu.be/" + result.VideoID header := ircColor(ircRed, "[YouTube]") if result.ChannelName != "" { header += " " + ircColor(ircYellow, result.ChannelName) + " —" } - header += " " + ircColor(ircCyan, title) + if result.Title != "" { + header += " " + ircColor(ircCyan, result.Title) + } parts := []string{header} if result.HasViewCount { parts = append(parts, ircColor(ircYellow, "👁 "+formatYouTubeCount(result.ViewCount)+" views")) @@ -345,17 +492,6 @@ func formatYouTubeSearchResult(result youtubeSearchResult, maxLength int) string return strings.Join(parts, " | ") } -func youtubeStatsText(result youtubeSearchResult) string { - var stats []string - if result.HasViewCount { - stats = append(stats, "👁 "+formatYouTubeCount(result.ViewCount)+" views") - } - if result.HasLikeCount { - stats = append(stats, "👍 "+formatYouTubeCount(result.LikeCount)+" likes") - } - return strings.Join(stats, " | ") -} - func formatYouTubeCount(count int64) string { value := strconv.FormatInt(count, 10) for i := len(value) - 3; i > 0; i -= 3 { diff --git a/plugins/youtube_test.go b/plugins/youtube_test.go index 5ab264c..a3af6f6 100644 --- a/plugins/youtube_test.go +++ b/plugins/youtube_test.go @@ -1,10 +1,16 @@ package plugins import ( + "context" + "encoding/base64" + "encoding/json" "io" "net/http" + "os" "strings" "testing" + "time" + "unicode/utf8" "github.com/variablenix/GoBot/bot" ) @@ -173,3 +179,166 @@ func TestYouTubeHelpDocumentsAliases(t *testing.T) { } } } + +func TestYouTubeAssignmentVariantsAndPrimaryOrder(t *testing.T) { + data := `{"contents":{"twoColumnSearchResultsRenderer":{"primaryContents":{"sectionListRenderer":{"contents":[{"adSlotRenderer":{"videoRenderer":{"videoId":"advert12345","title":{"simpleText":"Advertisement"}}}},{"videoRenderer":{"videoId":"first123456","title":{"simpleText":"First video"}}},{"videoRenderer":{"videoId":"later123456","title":{"simpleText":"Later video"}}}]}},"secondaryContents":{"videoRenderer":{"videoId":"other123456","title":{"simpleText":"Sidebar video"}}}}}}` + for _, assignment := range []string{"var ytInitialData = ", "let ytInitialData=", "const ytInitialData\n =\n", `window["ytInitialData"] = `, `window['ytInitialData']=`} { + t.Run(assignment, func(t *testing.T) { + for range 25 { + got, err := parseYouTubeInitialData([]byte(assignment + data + ";")) + if err != nil || got.VideoID != "first123456" { + t.Fatalf("wrong primary result: %+v, %v", got, err) + } + } + }) + } +} + +func TestYouTubeModernRenderersAndInvalidData(t *testing.T) { + for _, body := range []string{ + `{"videoWithContextRenderer":{"videoId":"first123456","title":{"simpleText":"Example video"},"longBylineText":{"simpleText":"Example channel"}}}`, + `{"lockupViewModel":{"contentId":"first123456","contentType":"LOCKUP_CONTENT_TYPE_VIDEO","metadata":{"lockupMetadataViewModel":{"title":{"content":"Example video"}}}}}`, + } { + got, err := parseYouTubeInitialData([]byte("var ytInitialData=" + body + ";")) + if err != nil || got.VideoID != "first123456" || got.Title != "Example video" { + t.Fatalf("renderer result: %+v, %v", got, err) + } + } + for _, body := range []string{ + `Confirm your age`, `var ytInitialData = broken;`, + `var ytInitialData={"videoRenderer":{"videoId":"../invalid","title":{"simpleText":"Unsafe"}}};`, + `var ytInitialData={"lockupViewModel":{"contentType":"LOCKUP_CONTENT_TYPE_PLAYLIST","videoRenderer":{"videoId":"first123456","title":{"simpleText":"Playlist thumbnail"}}}};`, + } { + if _, err := parseYouTubeInitialData([]byte(body)); err == nil { + t.Fatalf("accepted invalid video data: %s", body) + } + } +} + +func TestYouTubeIndexURLValidation(t *testing.T) { + for _, raw := range []string{"https://www.youtube.com/watch?v=first123456", "https://music.youtube.com/watch?v=first123456", "https://youtu.be/first123456"} { + if got := youtubeIndexedVideoID(raw); got != "first123456" { + t.Errorf("valid URL %q rejected", raw) + } + } + for _, raw := range []string{ + "https://youtube.com.evil.example/watch?v=first123456", "https://www.youtube.com@evil.example/watch?v=first123456", + "http://youtube.com/watch?v=first123456", "https://youtube.com:8443/watch?v=first123456", + "https://youtube.com/watch?v=short", "https://youtube.com/watch?v=first123456&v=later123456", + "https://youtube.com/channel/first123456", "https://youtu.be/first123456/extra", "http://127.0.0.1/private", + } { + if got := youtubeIndexedVideoID(raw); got != "" { + t.Errorf("unsafe/non-video URL %q accepted: %q", raw, got) + } + } +} + +func TestYouTubeRecoveryFromAPIAndPageFailures(t *testing.T) { + old := youtubeHTTPClient + t.Cleanup(func() { youtubeHTTPClient = old }) + for _, slow := range []bool{false, true} { + t.Run(map[bool]string{false: "quota-and-age-gate", true: "timeouts"}[slow], func(t *testing.T) { + var requests []string + youtubeHTTPClient = &http.Client{Transport: youtubeRoundTripper(func(r *http.Request) (*http.Response, error) { + requests = append(requests, r.URL.Host+r.URL.Path) + switch r.URL.Host + r.URL.Path { + case "www.googleapis.com/youtube/v3/search", "www.youtube.com/results": + if slow { + <-r.Context().Done() + return nil, r.Context().Err() + } + if r.URL.Host == "www.googleapis.com" { + return youtubeTestResponse(403, "application/json", `{}`), nil + } + return youtubeTestResponse(200, "text/html", `var ytInitialData={"contents":{"backgroundPromoRenderer":{"title":{"simpleText":"Confirm your age"}}}};`), nil + case "www.bing.com/search": + if r.Context().Err() != nil || r.URL.Query().Get("q") != "site:youtube.com/watch example music" { + t.Fatal("fallback lost query or inherited expired context") + } + // Ignore non-YouTube results, accept Bing's encoded destination, + // and never fetch a URL supplied by a search result directly. + encoded := "a1" + base64.RawURLEncoding.EncodeToString([]byte("https://www.youtube.com/watch?v=first123456")) + return youtubeTestResponse(200, "text/html", `

  • Wrong source

  • Example music - YouTube

    • `), nil + case "www.youtube.com/oembed": + return youtubeTestResponse(200, "application/json", `{"title":"Example music","author_name":"Example artist"}`), nil + default: + t.Fatalf("unexpected fallback request host/path: %s", r.URL.Host+r.URL.Path) + return nil, nil + } + })} + ctx, cancel := context.WithTimeout(t.Context(), 300*time.Millisecond) + defer cancel() + got, err := (&YouTube{apiKey: "test-key"}).search(ctx, "example music") + if err != nil || got.VideoID != "first123456" || got.ChannelName != "Example artist" || len(requests) != 4 { + t.Fatalf("recovery: %+v, %v, requests=%v", got, err, requests) + } + }) + } +} + +func TestYouTubeIndexedTitleSurvivesMetadataFailure(t *testing.T) { + old := youtubeHTTPClient + t.Cleanup(func() { youtubeHTTPClient = old }) + youtubeHTTPClient = &http.Client{Transport: youtubeRoundTripper(func(r *http.Request) (*http.Response, error) { + if r.URL.Host == "www.bing.com" { + return youtubeTestResponse(200, "text/html", `

  • Example music - YouTube

    • `), nil + } + return youtubeTestResponse(403, "text/html", "Unavailable"), nil + })} + got, err := (&YouTube{}).searchIndex(t.Context(), "example music") + if err != nil || got.Title != "Example music" || got.VideoID != "first123456" { + t.Fatalf("best-effort metadata discarded result: %+v, %v", got, err) + } +} + +func TestYouTubeReplyBoundsAndTerminalSafety(t *testing.T) { + for _, target := range []string{"#test", "#" + strings.Repeat("c", 180)} { + for _, limit := range []int{160, 320, 500} { + result := youtubeSearchResult{VideoID: "first123456", Title: strings.Repeat("音🎥", 200) + "\uFE0F\u202E\x03" + "04unsafe\r\n", ChannelName: strings.Repeat("界", 200), HasViewCount: true, ViewCount: 123456789} + got := formatYouTubeSearchResultForTarget(result, limit, target) + if len(got) > limit || len("PRIVMSG "+target+" :"+got+"\r\n") > 512 || !utf8.ValidString(got) { + t.Fatalf("reply exceeds UTF-8/wire/config bounds: %d bytes", len(got)) + } + if strings.ContainsAny(got, "\r\n\uFE0F\u202E") || !strings.HasSuffix(stripYouTubeIRC(got), "https://youtu.be/first123456") { + t.Fatalf("unsafe reply or lost link: %q", got) + } + } + } +} + +func TestYouTubeSearchHTMLSizeLimit(t *testing.T) { + old := youtubeHTTPClient + t.Cleanup(func() { youtubeHTTPClient = old }) + youtubeHTTPClient = &http.Client{Transport: youtubeRoundTripper(func(r *http.Request) (*http.Response, error) { + return youtubeTestResponse(200, "text/html", strings.Repeat("x", (4<<20)+1)), nil + })} + if _, err := youtubeSearchHTML(t.Context(), youtubeResultsURL); err == nil { + t.Fatal("oversized HTML accepted") + } +} + +// Live checks are opt-in; normal CI uses deterministic provider fixtures. +func TestLiveYouTubeSearch(t *testing.T) { + if os.Getenv("GOBOT_LIVE_YOUTUBE") != "1" { + t.Skip("opt-in live YouTube smoke test") + } + queries := []string{"Linux server setup", "classical piano music"} + if raw := os.Getenv("GOBOT_LIVE_YOUTUBE_QUERIES"); raw != "" { + if err := json.Unmarshal([]byte(raw), &queries); err != nil { + t.Fatal("invalid live query list") + } + } + p := &YouTube{} + p.Init(bot.PluginConfig{"api_key": os.Getenv("BOT_YOUTUBE_API_KEY")}, nil) + for index, query := range queries { + t.Run("query-"+string(rune('A'+index)), func(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), p.timeout) + defer cancel() + result, err := p.search(ctx, query) + if err != nil || !validYouTubeSearchResult(result) { + t.Fatalf("live lookup failed: %v", err) + } + t.Logf("title=%s video=%s", result.Title, result.VideoID) + }) + } +} From 1bffd2a73af5e9401f67a4167539ed4e0219eeee Mon Sep 17 00:00:00 2001 From: ak Date: Thu, 1 Oct 2026 21:15:25 -0700 Subject: [PATCH 2/4] Allow private deployment credentials for opt-in provider checks --- plugins/provider_live_test.go | 30 ++++++++++++++++++++++++++++++ plugins/youtube_test.go | 1 + 2 files changed, 31 insertions(+) diff --git a/plugins/provider_live_test.go b/plugins/provider_live_test.go index 5cf7fa6..1a48286 100644 --- a/plugins/provider_live_test.go +++ b/plugins/provider_live_test.go @@ -7,6 +7,7 @@ import ( "testing" "unicode/utf8" + "github.com/spf13/viper" "github.com/variablenix/GoBot/bot" ) @@ -17,6 +18,7 @@ func TestLivePluginCommands(t *testing.T) { if os.Getenv("GOBOT_LIVE_PLUGINS") != "1" { t.Skip("opt-in live plugin smoke test") } + loadLiveProviderCredentials(t) cases := []struct { plugin bot.Plugin command, want, token string @@ -45,6 +47,9 @@ func TestLivePluginCommands(t *testing.T) { t.Skip("provider credential not configured") } cfg := bot.PluginConfig{"timeout_seconds": 8} + if test.plugin.Name() == "github" { + cfg["token"] = os.Getenv("BOT_GITHUB_TOKEN") + } if test.token != "" { cfg["api_key"] = os.Getenv(test.token) } @@ -79,3 +84,28 @@ func TestLivePluginCommands(t *testing.T) { }) } } + +// Read only explicitly requested credential keys without evaluating a shell +// file, printing values, or changing the production configuration. +func loadLiveProviderCredentials(t *testing.T) { + t.Helper() + path := os.Getenv("GOBOT_LIVE_ENV_FILE") + if path == "" { + return + } + f, err := os.Open(path) + if err != nil { + t.Fatal("cannot open live provider environment file") + } + defer f.Close() + v := viper.New() + v.SetConfigType("env") + if v.ReadConfig(f) != nil { + t.Fatal("cannot parse live provider environment file") + } + for _, key := range []string{"BOT_YOUTUBE_API_KEY", "BOT_NEWS_API_KEY", "BOT_LASTFM_API_KEY", "BOT_GENIUS_ACCESS_TOKEN", "BOT_GITHUB_TOKEN"} { + if os.Getenv(key) == "" { + t.Setenv(key, v.GetString(key)) + } + } +} diff --git a/plugins/youtube_test.go b/plugins/youtube_test.go index a3af6f6..12c3171 100644 --- a/plugins/youtube_test.go +++ b/plugins/youtube_test.go @@ -322,6 +322,7 @@ func TestLiveYouTubeSearch(t *testing.T) { if os.Getenv("GOBOT_LIVE_YOUTUBE") != "1" { t.Skip("opt-in live YouTube smoke test") } + loadLiveProviderCredentials(t) queries := []string{"Linux server setup", "classical piano music"} if raw := os.Getenv("GOBOT_LIVE_YOUTUBE_QUERIES"); raw != "" { if err := json.Unmarshal([]byte(raw), &queries); err != nil { From b1d2eefe723ba9bebd6022f5477a7820cbb0130b Mon Sep 17 00:00:00 2001 From: ak Date: Thu, 1 Oct 2026 21:21:57 -0700 Subject: [PATCH 3/4] Recover explicit music titles when an artist query has no match --- docs/plugins.md | 13 ++++----- plugins/provider_live_test.go | 2 +- plugins/youtube.go | 28 ++++++++++++++++++++ plugins/youtube_test.go | 50 +++++++++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 7 deletions(-) diff --git a/docs/plugins.md b/docs/plugins.md index 848c1dd..f474b1c 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -430,8 +430,8 @@ short link: !youtube Linux server setup ~~~ -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 +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 @@ -442,13 +442,14 @@ 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. This does -not bypass sign-in or age restrictions for viewing a video. Each earlier +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. +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. diff --git a/plugins/provider_live_test.go b/plugins/provider_live_test.go index 1a48286..287d3fa 100644 --- a/plugins/provider_live_test.go +++ b/plugins/provider_live_test.go @@ -35,7 +35,7 @@ func TestLivePluginCommands(t *testing.T) { {&Pkg{}, "!pkg npm lodash", "lodash", ""}, {&Docker{}, "!docker alpine", "hub.docker.com", ""}, {&CVE{}, "!cve CVE-2024-3094", "CVE-2024-3094", ""}, - {&Reddit{}, "!reddit linux", "reddit.com", ""}, + {&Reddit{}, "!reddit r/linux", "reddit.com", ""}, {&Steam{}, "!steam Portal 2", "steampowered.com", ""}, {&Sports{}, "!sports", "Sports pick:", ""}, {&News{}, "!news technology", "http", "BOT_NEWS_API_KEY"}, diff --git a/plugins/youtube.go b/plugins/youtube.go index 4c4efcc..edb64af 100644 --- a/plugins/youtube.go +++ b/plugins/youtube.go @@ -241,6 +241,34 @@ func youtubeSearchHTML(ctx context.Context, endpoint string) ([]byte, error) { } func (p *YouTube) searchIndex(ctx context.Context, query string) (youtubeSearchResult, error) { + queries := []string{query} + // Artist - title is a common music-search format. If a misspelled artist + // prevents an indexed match, retry the explicit title, not a guessed spelling + // or an arbitrary first word. Keep the full query as the preferred lookup. + for _, separator := range []string{" - ", " — ", " – "} { + if _, title, ok := strings.Cut(query, separator); ok && strings.TrimSpace(title) != "" { + queries = append(queries, strings.TrimSpace(title)) + break + } + } + var lastErr error + for index, candidate := range queries { + step := ctx + cancel := func() {} + if index < len(queries)-1 { + step, cancel = youtubeStepContext(ctx, 3*time.Second) + } + result, err := p.searchIndexQuery(step, candidate) + cancel() + if err == nil { + return result, nil + } + lastErr = err + } + return youtubeSearchResult{}, lastErr +} + +func (p *YouTube) searchIndexQuery(ctx context.Context, query string) (youtubeSearchResult, error) { endpoint := "https://www.bing.com/search?" + url.Values{ "q": {"site:youtube.com/watch " + query}, "count": {"8"}, "setlang": {"en-US"}, }.Encode() diff --git a/plugins/youtube_test.go b/plugins/youtube_test.go index 12c3171..ca6f5e3 100644 --- a/plugins/youtube_test.go +++ b/plugins/youtube_test.go @@ -318,6 +318,56 @@ func TestYouTubeSearchHTMLSizeLimit(t *testing.T) { } // Live checks are opt-in; normal CI uses deterministic provider fixtures. +func TestYouTubeIndexRetriesExplicitMusicTitle(t *testing.T) { + old := youtubeHTTPClient + t.Cleanup(func() { youtubeHTTPClient = old }) + var queries []string + youtubeHTTPClient = &http.Client{Transport: youtubeRoundTripper(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/oembed" { + return youtubeTestResponse(403, "application/json", `{}`), nil + } + queries = append(queries, r.URL.Query().Get("q")) + if len(queries) == 1 { + return youtubeTestResponse(200, "text/html", `

  • Unrelated result

    • `), nil + } + return youtubeTestResponse(200, "text/html", `

  • Example music - YouTube

    • `), nil + })} + ctx, cancel := context.WithTimeout(t.Context(), time.Second) + defer cancel() + result, err := (&YouTube{}).searchIndex(ctx, "example artist - example music") + if err != nil || result.VideoID != "first123456" || len(queries) != 2 || queries[0] != "site:youtube.com/watch example artist - example music" || queries[1] != "site:youtube.com/watch example music" { + t.Fatalf("music recovery: result=%+v err=%v queries=%v", result, err, queries) + } +} + +func TestYouTubeAliasesReturnUsableIRCResult(t *testing.T) { + old := youtubeHTTPClient + t.Cleanup(func() { youtubeHTTPClient = old }) + youtubeHTTPClient = &http.Client{Transport: youtubeRoundTripper(func(*http.Request) (*http.Response, error) { + return youtubeTestResponse(200, "text/html", `var ytInitialData={"videoRenderer":{"videoId":"first123456","title":{"simpleText":"Example music"}}};`), nil + })} + p := &YouTube{} + p.Init(nil, nil) + sent := make(chan bot.Outgoing, 4) + b := &bot.Bot{Config: bot.Config{CommandPrefix: "!"}, Queue: bot.NewQueue(1, 1, func(m bot.Outgoing) { sent <- m })} + defer b.Queue.Drain(context.Background()) + for _, alias := range []string{"!yt", "!youtube"} { + if !p.Handle(b, bot.Message{Nick: "tester", Target: "#test", IsChannel: true, Text: alias + " example music"}) { + t.Fatal("YouTube alias not handled") + } + } + b.Queue.Drain(context.Background()) + if len(sent) != 2 { + t.Fatal("missing YouTube alias replies") + } + for len(sent) > 0 { + m := <-sent + if m.Target != "#test" || !strings.Contains(m.Text, "https://youtu.be/first123456") || len("PRIVMSG #test :"+m.Text+"\r\n") > 512 { + t.Fatal("invalid YouTube command reply") + } + } +} + func TestLiveYouTubeSearch(t *testing.T) { if os.Getenv("GOBOT_LIVE_YOUTUBE") != "1" { t.Skip("opt-in live YouTube smoke test") From 34cec5aef5a7aa838998eb2b29a170c67a4e5dc1 Mon Sep 17 00:00:00 2001 From: ak Date: Thu, 1 Oct 2026 21:26:09 -0700 Subject: [PATCH 4/4] Keep seen history private to its channel and expand live checks --- docs/plugins.md | 11 ++++++----- plugins/provider_live_test.go | 3 +++ plugins/seen.go | 18 +++++++++++------- plugins/seen_test.go | 16 +++++++++------- 4 files changed, 29 insertions(+), 19 deletions(-) diff --git a/docs/plugins.md b/docs/plugins.md index f474b1c..5976e4e 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -1482,11 +1482,12 @@ project-- !reddit https://www.reddit.com/r/example/comments/abc123/post/ ~~~ -- seen reports where and when a nickname last spoke in a channel on the current - IRC network. Records are stored in BoltDB; private messages are never stored - or disclosed. Legacy nickname-only records are not displayed because their - network and public/private origin cannot be verified. A new channel message - establishes a safely scoped record after upgrading. +- 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 diff --git a/plugins/provider_live_test.go b/plugins/provider_live_test.go index 287d3fa..c2bfb69 100644 --- a/plugins/provider_live_test.go +++ b/plugins/provider_live_test.go @@ -40,6 +40,9 @@ func TestLivePluginCommands(t *testing.T) { {&Sports{}, "!sports", "Sports pick:", ""}, {&News{}, "!news technology", "http", "BOT_NEWS_API_KEY"}, {&Lyrics{}, "!lyrics electric wizard Dopethrone", "genius.com", "BOT_GENIUS_ACCESS_TOKEN"}, + {&Horoscope{}, "!horoscope aries", "Aries:", ""}, + {&IPInfo{}, "!ip 8.8.8.8", "8.8.8.8", ""}, + {&LastFM{}, "!lastfm RJ", "last.fm", "BOT_LASTFM_API_KEY"}, } for _, test := range cases { t.Run(test.plugin.Name(), func(t *testing.T) { diff --git a/plugins/seen.go b/plugins/seen.go index b2ab7b2..0d77712 100644 --- a/plugins/seen.go +++ b/plugins/seen.go @@ -21,31 +21,35 @@ type Seen struct{ db *storage.DB } func (p *Seen) Name() string { return "seen" } func (p *Seen) Commands() []string { return []string{"seen"} } func (p *Seen) Help() string { - return "!seen — show when someone last spoke in a channel on this network" + return "!seen — show when someone last spoke in the current channel" } func (p *Seen) Init(_ bot.PluginConfig, d *storage.DB) error { p.db = d; return nil } func (p *Seen) Handle(b *bot.Bot, m bot.Message) bool { if m.Command == "PRIVMSG" && m.IsChannel && m.Nick != "" && p.db != nil { - _ = p.db.Set("seen", seenKey(b.Config.NetworkName, m.Nick), record{m.Nick, m.Target, cleanExternalText(normalizeSeenText(m.Nick, m.Text)), m.Timestamp}) + _ = p.db.Set("seen", seenKey(b.Config.NetworkName, m.Target, m.Nick), record{m.Nick, m.Target, cleanExternalText(normalizeSeenText(m.Nick, m.Text)), m.Timestamp}) } cmd, arg, ok := bot.IsCommand(m, b.Config.CommandPrefix) if !ok || cmd != "seen" { return false } + if !m.IsChannel { + b.Send(m.ReplyTarget(), "Use !seen in a channel; records are private to that channel.") + return true + } if p.db == nil { b.Send(m.ReplyTarget(), "Seen storage is unavailable.") return true } - // Legacy nickname-only records may contain private messages and cannot be - // assigned to a network safely. Leave them untouched, but never disclose them. - v, e := p.db.Get("seen", seenKey(b.Config.NetworkName, strings.TrimSpace(arg))) + // Legacy records may contain private messages or another channel's history. + // Leave them untouched, but never disclose them across channel boundaries. + v, e := p.db.Get("seen", seenKey(b.Config.NetworkName, m.Target, strings.TrimSpace(arg))) if e != nil { b.Send(m.ReplyTarget(), "I haven't seen that nick yet.") return true } var x record if err := json.Unmarshal(v, &x); err != nil || strings.TrimSpace(x.Nick) == "" || - (!strings.HasPrefix(x.Channel, "#") && !strings.HasPrefix(x.Channel, "&")) { + !strings.EqualFold(x.Channel, m.Target) { b.Send(m.ReplyTarget(), "That seen record is unavailable.") return true } @@ -53,7 +57,7 @@ func (p *Seen) Handle(b *bot.Bot, m bot.Message) bool { return true } -func seenKey(network, nick string) string { return scopedKey(network, "", nick) } +func seenKey(network, channel, nick string) string { return scopedKey(network, channel, nick) } func normalizeSeenText(nick, text string) string { if len(text) >= 2 && text[0] == '\x01' && text[len(text)-1] == '\x01' { diff --git a/plugins/seen_test.go b/plugins/seen_test.go index 890e5da..ecad0b9 100644 --- a/plugins/seen_test.go +++ b/plugins/seen_test.go @@ -45,28 +45,30 @@ func TestSeenDoesNotExposePrivateOrCrossNetworkRecords(t *testing.T) { p := &Seen{} p.Init(nil, db) p.Handle(b, bot.Message{Command: "PRIVMSG", Nick: "Alice", Target: "Echo", Text: "private content", Timestamp: time.Now()}) - if _, err := db.Get("seen", seenKey("first", "Alice")); err != storage.ErrNotFound { + if _, err := db.Get("seen", seenKey("first", "Echo", "Alice")); err != storage.ErrNotFound { t.Fatal("private message was persisted") } p.Handle(b, bot.Message{Command: "PRIVMSG", Nick: "Alice", Target: "#test", IsChannel: true, Text: "public greeting", Timestamp: time.Now()}) - if !p.Handle(b, bot.Message{Nick: "tester", Target: "Echo", Text: "!seen Alice"}) { + if !p.Handle(b, bot.Message{Nick: "tester", Target: "#test", IsChannel: true, Text: "!seen Alice"}) { t.Fatal("seen command was not handled") } b.Queue.Drain(context.Background()) if len(sent) != 1 || !strings.Contains(<-sent, "public greeting") { t.Fatal("same-network public record was not returned") } - if _, err := db.Get("seen", seenKey("second", "Alice")); err != storage.ErrNotFound { + if _, err := db.Get("seen", seenKey("second", "#test", "Alice")); err != storage.ErrNotFound { t.Fatal("network isolation failed") } // Legacy records cannot distinguish networks or prove a public origin. db.Set("seen", "bob", record{Nick: "Bob", Channel: "Echo", Text: "legacy private content"}) - p.Handle(b, bot.Message{Nick: "tester", Target: "Echo", Text: "!seen Bob"}) - b.Config.NetworkName = "second" + p.Handle(b, bot.Message{Nick: "tester", Target: "#test", IsChannel: true, Text: "!seen Bob"}) + p.Handle(b, bot.Message{Nick: "tester", Target: "#other", IsChannel: true, Text: "!seen Alice"}) p.Handle(b, bot.Message{Nick: "tester", Target: "Echo", Text: "!seen Alice"}) + b.Config.NetworkName = "second" + p.Handle(b, bot.Message{Nick: "tester", Target: "#test", IsChannel: true, Text: "!seen Alice"}) b.Queue.Drain(context.Background()) - if len(sent) != 2 { - t.Fatalf("expected two lookup replies, got %d", len(sent)) + if len(sent) != 4 { + t.Fatalf("expected four lookup replies, got %d", len(sent)) } for len(sent) > 0 { if reply := <-sent; strings.Contains(reply, "private content") || strings.Contains(reply, "public greeting") {