diff --git a/docs/plugins.md b/docs/plugins.md index d05ba16..ae5d467 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -726,6 +726,9 @@ invalid URLs, and malformed third-party text each produce a clear single-line response. Queries and returned metadata reject control characters, invisible formatting characters, variation selectors, and line breaks. Responses are bounded by `plugins.lyrics.max_length` and the IRC 512-byte wire-line limit. +Metadata is shortened before the link; if an unusually long canonical URL +would not fit, GoBot uses Genius's short song URL so the single link remains +complete and usable. Optional settings: diff --git a/docs/security.md b/docs/security.md index dedf1e2..e4a53c7 100644 --- a/docs/security.md +++ b/docs/security.md @@ -43,6 +43,10 @@ dependencies, and deployment configuration maintained. - The paste token is sent only to the configured Opengist base URL and should use HTTPS. `BOT_PASTE_TOKEN` and `BOT_PASTE_BASE_URL` belong in the service environment, not in Git. +- The Genius client access token is sent only to the fixed + `https://api.genius.com/search` endpoint. Lyrics lookup rejects redirects so + the bearer token is not forwarded, and `BOT_GENIUS_ACCESS_TOKEN` belongs in + the service environment rather than Git. - IRC invitations, command handling, and cooldown warnings are rate-limited. - The Docker image runs as a non-root user. - Protect the BoltDB data file and its containing directory with filesystem diff --git a/plugins/duckhunt.go b/plugins/duckhunt.go index 9f118c2..af6b50f 100644 --- a/plugins/duckhunt.go +++ b/plugins/duckhunt.go @@ -1128,10 +1128,7 @@ func duckAchievementCondition(key string, player duckPlayer, kills, friends uint } func formatDuckAchievement(nick string, achievement duckAchievementDefinition) string { - description := achievement.Description - if strings.Contains(description, goldenDuckLabel) { - description = strings.Replace(description, goldenDuckLabel, ircColor(ircGold, goldenDuckLabel), 1) - } + description := strings.Replace(achievement.Description, goldenDuckLabel, ircColor(ircGold, goldenDuckLabel), 1) return fmt.Sprintf("%s %s unlocked: %s - %s", ircColor(ircGreen, "[Achievement]"), ircColor(ircCyan, nick), ircColor(ircYellow, achievement.Name), description) } diff --git a/plugins/lyrics.go b/plugins/lyrics.go index f40a9f7..224a242 100644 --- a/plugins/lyrics.go +++ b/plugins/lyrics.go @@ -9,6 +9,7 @@ import ( "net/http" "net/url" "os" + "strconv" "strings" "time" "unicode" @@ -24,6 +25,15 @@ const ( geniusSearchEndpoint = "https://api.genius.com/search" ) +var lyricsHTTPClient = &http.Client{ + Timeout: 10 * time.Second, + // The bearer token is only intended for api.genius.com. A redirect is an + // upstream failure here, not permission to forward credentials elsewhere. + CheckRedirect: func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + }, +} + type Lyrics struct { cfg bot.PluginConfig cooldown scopedCooldown @@ -43,6 +53,7 @@ type geniusSearchHit struct { } type geniusSong struct { + ID int64 `json:"id"` Title string `json:"title"` FullTitle string `json:"full_title"` ArtistNames string `json:"artist_names"` @@ -103,7 +114,12 @@ func (p *Lyrics) Handle(b *bot.Bot, m bot.Message) bool { return true } - p.send(b, m.ReplyTarget(), formatLyricsResult(song)) + reply, ok := formatLyricsResult(song, m.ReplyTarget(), lyricsMaxLength(p.cfg)) + if !ok { + p.send(b, m.ReplyTarget(), "lyrics result could not be formatted safely") + return true + } + b.Send(m.ReplyTarget(), reply) return true } @@ -112,8 +128,8 @@ func (p *Lyrics) send(b *bot.Bot, target, text string) { } var ( - errGeniusNotFound = errors.New("Genius song not found") - errGeniusUnauthorized = errors.New("Genius authentication failed") + errGeniusNotFound = errors.New("genius song not found") + errGeniusUnauthorized = errors.New("genius authentication failed") ) func isLyricsCommand(command string) bool { @@ -169,6 +185,10 @@ func lyricsMaxLength(c bot.PluginConfig) int { func boundLyricsReply(target, text string, configuredMax int) string { text = cleanLyricsText(text) + return truncateUTF8Bytes(text, lyricsReplyByteLimit(target, configuredMax)) +} + +func lyricsReplyByteLimit(target string, configuredMax int) int { wireLimit := lyricsIRCMaxLineBytes - len("PRIVMSG ") - len([]byte(target)) - len(" :") - len("\r\n") if wireLimit < 1 { wireLimit = 1 @@ -176,7 +196,7 @@ func boundLyricsReply(target, text string, configuredMax int) string { if configuredMax < 1 || configuredMax > wireLimit { configuredMax = wireLimit } - return truncateUTF8Bytes(text, configuredMax) + return configuredMax } func cleanLyricsText(text string) string { @@ -214,7 +234,7 @@ func lookupGeniusSong(ctx context.Context, query, token string) (geniusSong, err req.Header.Set("Authorization", "Bearer "+token) req.Header.Set("User-Agent", "GoBot/1.0 (IRC bot; Genius lyrics link lookup)") - res, err := apiHTTPClient.Do(req) + res, err := lyricsHTTPClient.Do(req) if err != nil { return geniusSong{}, err } @@ -226,7 +246,7 @@ func lookupGeniusSong(ctx context.Context, query, token string) (geniusSong, err return geniusSong{}, errGeniusNotFound } if res.StatusCode < 200 || res.StatusCode >= 300 { - return geniusSong{}, fmt.Errorf("Genius returned HTTP %d", res.StatusCode) + return geniusSong{}, fmt.Errorf("genius returned HTTP %d", res.StatusCode) } var payload geniusSearchResponse @@ -246,29 +266,63 @@ func lookupGeniusSong(ctx context.Context, query, token string) (geniusSong, err } func validGeniusSongURL(raw string) bool { - parsed, err := url.Parse(strings.TrimSpace(raw)) - if err != nil || !strings.EqualFold(parsed.Scheme, "https") || parsed.User != nil || parsed.Path == "" { + raw = strings.TrimSpace(raw) + if raw == "" || cleanLyricsText(raw) != raw { + return false + } + parsed, err := url.Parse(raw) + if err != nil || !strings.EqualFold(parsed.Scheme, "https") || parsed.User != nil || + parsed.Path == "" || parsed.Path == "/" || parsed.RawQuery != "" || parsed.Fragment != "" { return false } - return strings.EqualFold(parsed.Hostname(), "genius.com") || - strings.EqualFold(parsed.Hostname(), "www.genius.com") + host := strings.ToLower(parsed.Host) + return host == "genius.com" || host == "www.genius.com" } -func formatLyricsResult(song geniusSong) string { - artist := cleanLyricsText(song.ArtistNames) +func formatLyricsResult(song geniusSong, target string, configuredMax int) (string, bool) { + link := strings.TrimSpace(song.URL) + if !validGeniusSongURL(link) { + return "", false + } + limit := lyricsReplyByteLimit(target, configuredMax) + prefix := "[lyrics] " + if len(prefix)+len(link) > limit && song.ID > 0 { + link = "https://genius.com/songs/" + strconv.FormatInt(song.ID, 10) + } + if len(prefix)+len(link) > limit { + return "", false + } + + artist := cleanLyricsLabel(song.ArtistNames) if artist == "" { - artist = cleanLyricsText(song.PrimaryArtist.Name) + artist = cleanLyricsLabel(song.PrimaryArtist.Name) } - title := cleanLyricsText(song.Title) + title := cleanLyricsLabel(song.Title) if title == "" { - title = cleanLyricsText(song.FullTitle) + title = cleanLyricsLabel(song.FullTitle) } - link := strings.TrimSpace(song.URL) + label := "Genius song" if artist != "" && title != "" { - return fmt.Sprintf("[lyrics] %s - %s | %s", artist, title, link) + label = artist + " - " + title + } else if title != "" { + label = title } - if title != "" { - return fmt.Sprintf("[lyrics] %s | %s", title, link) + + suffix := " | " + link + available := limit - len(prefix) - len(suffix) + if available < 1 { + return prefix + link, true + } + label = truncateUTF8Bytes(label, available) + return prefix + label + suffix, true +} + +func cleanLyricsLabel(text string) string { + cleaned := cleanLyricsText(text) + lower := strings.ToLower(cleaned) + if strings.Contains(lower, "http://") || strings.Contains(lower, "https://") || + strings.Contains(lower, "www.") { + return "" } - return "[lyrics] Genius song | " + link + return cleaned } diff --git a/plugins/lyrics_test.go b/plugins/lyrics_test.go index a2057c8..7b11e46 100644 --- a/plugins/lyrics_test.go +++ b/plugins/lyrics_test.go @@ -35,6 +35,29 @@ func TestValidLyricsQuery(t *testing.T) { } } +func TestValidGeniusToken(t *testing.T) { + for _, token := range []string{"client-access-token", strings.Repeat("x", 4096)} { + if !validGeniusToken(token) { + t.Errorf("validGeniusToken(%q) = false", token) + } + } + for _, token := range []string{"", "token\nheader", strings.Repeat("x", 4097), string([]byte{0xff})} { + if validGeniusToken(token) { + t.Errorf("validGeniusToken(%q) = true", token) + } + } +} + +func TestLyricsHTTPClientRejectsRedirects(t *testing.T) { + request, err := http.NewRequest(http.MethodGet, "https://redirect.example", nil) + if err != nil { + t.Fatal(err) + } + if err := lyricsHTTPClient.CheckRedirect(request, nil); !errors.Is(err, http.ErrUseLastResponse) { + t.Fatalf("CheckRedirect() error = %v, want http.ErrUseLastResponse", err) + } +} + func TestValidGeniusSongURL(t *testing.T) { for _, raw := range []string{ "https://genius.com/Radiohead-paranoid-android-lyrics", @@ -50,6 +73,12 @@ func TestValidGeniusSongURL(t *testing.T) { "https://genius.com.evil.example/song", "https://user:pass@genius.com/song", "https://genius.com", + "https://genius.com/", + "https://genius.com:8443/song", + "https://genius.com/song?tracking=1", + "https://genius.com/song#lyrics", + "https://genius.com/song\u202e", + "https://genius.com/song\ufe0f", } { if validGeniusSongURL(raw) { t.Errorf("validGeniusSongURL(%q) = true", raw) @@ -58,9 +87,9 @@ func TestValidGeniusSongURL(t *testing.T) { } func TestLookupGeniusSongUsesTokenAndSelectsSong(t *testing.T) { - old := apiHTTPClient - t.Cleanup(func() { apiHTTPClient = old }) - apiHTTPClient = &http.Client{Transport: newPluginRoundTripper(func(r *http.Request) (*http.Response, error) { + old := lyricsHTTPClient + t.Cleanup(func() { lyricsHTTPClient = old }) + lyricsHTTPClient = &http.Client{Transport: newPluginRoundTripper(func(r *http.Request) (*http.Response, error) { if r.URL.Path != "/search" || r.URL.Query().Get("q") != `artist - "song"` { t.Fatalf("unexpected Genius request: %s", r.URL) } @@ -69,7 +98,7 @@ func TestLookupGeniusSongUsesTokenAndSelectsSong(t *testing.T) { } return newPluginResponse(http.StatusOK, `{"response":{"hits":[ {"type":"artist","result":{"title":"Artist","url":"https://genius.com/artists/artist"}}, - {"type":"song","result":{"title":"Song","artist_names":"Artist","url":"https://genius.com/Artist-song-lyrics"}} + {"type":"song","result":{"id":123,"title":"Song","artist_names":"Artist","url":"https://genius.com/Artist-song-lyrics"}} ]}}`), nil })} @@ -82,10 +111,48 @@ func TestLookupGeniusSongUsesTokenAndSelectsSong(t *testing.T) { } } +func TestLookupGeniusSongAcceptsScreenshotQueries(t *testing.T) { + queries := []string{ + "electric wizard L.S.D.", + "electric wizard lsd", + "electric wizard", + "electric wizard Dopethrone", + "Snoop Dogg - Smoke Weed Everyday", + "Smoke Weed Everyday", + } + old := lyricsHTTPClient + t.Cleanup(func() { lyricsHTTPClient = old }) + lyricsHTTPClient = &http.Client{Transport: newPluginRoundTripper(func(r *http.Request) (*http.Response, error) { + query := r.URL.Query().Get("q") + found := false + for _, candidate := range queries { + if query == candidate { + found = true + break + } + } + if !found { + t.Fatalf("unexpected screenshot query: %q", query) + } + return newPluginResponse(http.StatusOK, `{"response":{"hits":[{"type":"song","result":{"id":321,"title":"Matched song","artist_names":"Matched artist","url":"https://genius.com/Matched-artist-matched-song-lyrics"}}]}}`), nil + })} + + for _, query := range queries { + song, err := lookupGeniusSong(t.Context(), query, "test-token") + if err != nil { + t.Errorf("lookupGeniusSong(%q) error = %v", query, err) + continue + } + if song.ID != 321 || song.URL == "" { + t.Errorf("lookupGeniusSong(%q) = %#v", query, song) + } + } +} + func TestLookupGeniusSongRejectsInvalidResults(t *testing.T) { - old := apiHTTPClient - t.Cleanup(func() { apiHTTPClient = old }) - apiHTTPClient = &http.Client{Transport: newPluginRoundTripper(func(*http.Request) (*http.Response, error) { + old := lyricsHTTPClient + t.Cleanup(func() { lyricsHTTPClient = old }) + lyricsHTTPClient = &http.Client{Transport: newPluginRoundTripper(func(*http.Request) (*http.Response, error) { return newPluginResponse(http.StatusOK, `{"response":{"hits":[ {"type":"artist","result":{"title":"Artist","url":"https://genius.com/artists/artist"}}, {"type":"song","result":{"title":"Bad host","url":"https://evil.example/song"}} @@ -96,12 +163,49 @@ func TestLookupGeniusSongRejectsInvalidResults(t *testing.T) { } } +func TestLookupGeniusSongHandlesUpstreamFailures(t *testing.T) { + tests := []struct { + name string + status int + body string + want error + }{ + {name: "unauthorized", status: http.StatusUnauthorized, body: `{}`, want: errGeniusUnauthorized}, + {name: "forbidden", status: http.StatusForbidden, body: `{}`, want: errGeniusUnauthorized}, + {name: "not found", status: http.StatusNotFound, body: `{}`, want: errGeniusNotFound}, + {name: "no hits", status: http.StatusOK, body: `{"response":{"hits":[]}}`, want: errGeniusNotFound}, + {name: "malformed JSON", status: http.StatusOK, body: `{"response":`, want: nil}, + {name: "server error", status: http.StatusInternalServerError, body: `{}`, want: nil}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + old := lyricsHTTPClient + t.Cleanup(func() { lyricsHTTPClient = old }) + lyricsHTTPClient = &http.Client{Transport: newPluginRoundTripper(func(*http.Request) (*http.Response, error) { + return newPluginResponse(test.status, test.body), nil + })} + _, err := lookupGeniusSong(t.Context(), "song", "token") + if test.want != nil { + if !errors.Is(err, test.want) { + t.Fatalf("lookupGeniusSong() error = %v, want %v", err, test.want) + } + } else if err == nil { + t.Fatal("lookupGeniusSong() error = nil") + } + }) + } +} + func TestFormatLyricsResultCleansMetadata(t *testing.T) { - got := formatLyricsResult(geniusSong{ + got, ok := formatLyricsResult(geniusSong{ + ID: 123, Title: "Song\ufe0f\u200d\u202e", ArtistNames: "Artist\x03 Name", URL: "https://genius.com/Artist-song-lyrics", - }) + }, "#music", 320) + if !ok { + t.Fatal("formatLyricsResult() rejected a valid song") + } if got != "[lyrics] Artist Name - Song | https://genius.com/Artist-song-lyrics" { t.Fatalf("formatLyricsResult() = %q", got) } @@ -110,6 +214,28 @@ func TestFormatLyricsResultCleansMetadata(t *testing.T) { } } +func TestFormatLyricsResultPreservesExactlyOneCompleteLink(t *testing.T) { + canonical := "https://genius.com/Artist-" + strings.Repeat("very-long-title-", 20) + "lyrics" + got, ok := formatLyricsResult(geniusSong{ + ID: 456, + Title: strings.Repeat("界", 200), + ArtistNames: "Artist https://untrusted.example", + URL: canonical, + }, "#music", 120) + if !ok { + t.Fatal("formatLyricsResult() rejected a song with a usable short link") + } + if got != "[lyrics] 界界界界界界界界界界界界界界界界界界界界界界界界界… | https://genius.com/songs/456" { + t.Fatalf("formatLyricsResult() = %q", got) + } + if strings.Count(got, "https://") != 1 || !strings.HasSuffix(got, "https://genius.com/songs/456") { + t.Fatalf("reply does not preserve exactly one complete Genius link: %q", got) + } + if len([]byte(got)) > 120 { + t.Fatalf("reply is %d bytes, want at most 120: %q", len([]byte(got)), got) + } +} + func TestBoundLyricsReplyUsesUTF8WireByteLimit(t *testing.T) { target := "#international-music" reply := boundLyricsReply(target, "[lyrics] "+strings.Repeat("界", 300)+"\r\nsecond line", 500) @@ -123,11 +249,11 @@ func TestBoundLyricsReplyUsesUTF8WireByteLimit(t *testing.T) { } func TestLyricsHandleReturnsOneBoundedLine(t *testing.T) { - old := apiHTTPClient - t.Cleanup(func() { apiHTTPClient = old }) + old := lyricsHTTPClient + t.Cleanup(func() { lyricsHTTPClient = old }) t.Setenv("BOT_GENIUS_ACCESS_TOKEN", "test-token") - apiHTTPClient = &http.Client{Transport: newPluginRoundTripper(func(*http.Request) (*http.Response, error) { - return newPluginResponse(http.StatusOK, `{"response":{"hits":[{"type":"song","result":{"title":"Song\ufe0f","artist_names":"Artist\u200d","url":"https://genius.com/Artist-song-lyrics"}}]}}`), nil + lyricsHTTPClient = &http.Client{Transport: newPluginRoundTripper(func(*http.Request) (*http.Response, error) { + return newPluginResponse(http.StatusOK, `{"response":{"hits":[{"type":"song","result":{"id":123,"title":"Song\ufe0f","artist_names":"Artist\u200d","url":"https://genius.com/Artist-song-lyrics"}}]}}`), nil })} sent := make(chan bot.Outgoing, 2) diff --git a/plugins/paste.go b/plugins/paste.go index 0a3286b..e8b60b6 100644 --- a/plugins/paste.go +++ b/plugins/paste.go @@ -252,7 +252,7 @@ func (p *Paste) createPaste(parent context.Context, content string) (string, err result = p.baseURL + "/" + strings.TrimSpace(response.ID) } if result == "" { - return "", fmt.Errorf("Opengist response did not include a URL") + return "", fmt.Errorf("opengist response did not include a URL") } return result, nil } diff --git a/plugins/reddit.go b/plugins/reddit.go index a1bd292..4c8f3ef 100644 --- a/plugins/reddit.go +++ b/plugins/reddit.go @@ -30,6 +30,10 @@ func (p *Reddit) Handle(b *bot.Bot, m bot.Message) bool { return false } target, sort, ok := parseRedditLookupArg(arg) + if !ok { + b.Send(m.ReplyTarget(), ircColor(ircYellow, "usage: !reddit [best|hot|new|top|rising] ")) + return true + } postURL, endpoint, ok := redditLookupEndpointWithSort(target, sort) if !ok { b.Send(m.ReplyTarget(), ircColor(ircYellow, "usage: !reddit [best|hot|new|top|rising] ")) diff --git a/plugins/seen.go b/plugins/seen.go index ae4f28b..cc1cfca 100644 --- a/plugins/seen.go +++ b/plugins/seen.go @@ -24,7 +24,7 @@ func (p *Seen) Help() string { return "!seen = len(labels) { + return '?' + } + return labels[index] +} + var triviaFallbacks = []triviaQuestion{ {Category: "Science", Text: "What planet is known as the Red Planet", Options: []string{"Mars", "Venus", "Jupiter", "Mercury"}, Correct: 0}, {Category: "Geography", Text: "What is the capital of Japan", Options: []string{"Tokyo", "Kyoto", "Osaka", "Sapporo"}, Correct: 0},