Fix SSRF check bypass, rate-limit !lyrics, and clean up

README.md

@@ -283,8 +283,9 @@ off, so the page exists on one host only.
   and search terms are passed to yt-dlp after `--`, so they are never read as
   options.
 - **Rate limiting:** `!play`, which starts a yt-dlp process each time, takes
-  at most 3 calls per 5 seconds and one at a time per user
-  (`tests/test_rate_limits.py` triggers both limits). Effect changes are
+  at most 3 calls per 5 seconds and one at a time per user, and `!lyrics`,
+  which calls LRCLIB and Genius, at most 3 per 10 seconds per user
+  (`tests/test_rate_limits.py` triggers each limit). Effect changes are
   throttled to one every three seconds per server, and `/` commands are only
   registered through the owner-only `!sync`, because Discord rate-limits
   syncing.

cogs/effects.py

@@ -58,6 +58,8 @@ class Effect:
 # `ctx.invoked_with` says which name the user actually typed.
 _EFFECT_NAMES = list(EFFECTS)
 
+_NO_EFFECT = Effect("", "Audio effects removed ✅")
+
 
 class Effects(commands.Cog, name="🎛️ Audio Effects"):
     def __init__(self, bot: commands.Bot) -> None:
@@ -71,17 +73,16 @@ def _throttled(self, ctx: GuildContext) -> bool:
         self._last_change[ctx.guild.id] = now
         return False
 
-    async def _switch_to(self, ctx: GuildContext, name: str | None, filter_str: str, label: str,
-                         rate: float = 1.0) -> None:
+    async def _switch_to(self, ctx: GuildContext, name: Optional[str], effect: Effect) -> None:
         """Throttle, apply, and report — the whole path every effect command takes."""
         if self._throttled(ctx):
             await ctx.send(embed=error_embed(
                 f"Easy — wait {_EFFECT_COOLDOWN:.0f}s between effect changes."
             ), ephemeral=True)
             return
         player = players.get(ctx.guild.id)
-        if player and player.apply_effect(name, filter_str, rate):
-            await ctx.send(embed=success_embed(label))
+        if player and player.apply_effect(name, effect.filter, effect.rate):
+            await ctx.send(embed=success_embed(effect.label))
         else:
             await ctx.send(embed=error_embed("Nothing is playing."), ephemeral=True)
 
@@ -92,14 +93,13 @@ async def _switch_to(self, ctx: GuildContext, name: str | None, filter_str: str,
     async def apply_effect(self, ctx: GuildContext) -> None:
         # Always set here: this callback only runs when invoked by one of its names.
         name = (ctx.invoked_with or _EFFECT_NAMES[0]).lower()
-        effect = EFFECTS[name]
-        await self._switch_to(ctx, name, effect.filter, effect.label, effect.rate)
+        await self._switch_to(ctx, name, EFFECTS[name])
 
     @hybrid_command(name="reset", aliases=["fxreset", "noeffect"])
     @same_voice_channel()
     async def reset_effect(self, ctx: GuildContext) -> None:
         """Remove all audio effects."""
-        await self._switch_to(ctx, None, "", "Audio effects removed ✅")
+        await self._switch_to(ctx, None, _NO_EFFECT)
 
     @hybrid_command(name="effect")
     @app_commands.describe(name="The effect to apply; leave it out to see the active one")
@@ -118,7 +118,7 @@ async def effect(self, ctx: GuildContext, name: Optional[str] = None) -> None:
                 f"Unknown effect `{name}`. See `{ctx.clean_prefix}effects`."), ephemeral=True)
             return
         if await in_same_voice_channel(ctx):
-            await self._switch_to(ctx, name, chosen.filter, chosen.label, chosen.rate)
+            await self._switch_to(ctx, name, chosen)
 
     @hybrid_command(name="effects")
     async def list_effects(self, ctx: GuildContext) -> None:

cogs/lyrics.py

@@ -310,6 +310,9 @@ async def show_current(self, dest: discord.abc.Messageable,
     # -- The command ---------------------------------------------------
 
     @hybrid_command(aliases=["ly"])
+    # One lookup is up to two LRCLIB and two Genius requests; spamming it would
+    # get the bot's address rate-limited by both.
+    @commands.cooldown(rate=3, per=10.0, type=commands.BucketType.user)
     async def lyrics(self, ctx: GuildContext, *,
                      query: Optional[str] = None) -> None:
         """Follow the lyrics of the current song, or look a song up."""

cogs/music.py

@@ -5,6 +5,7 @@
 import aiohttp
 import discord
 from discord import app_commands
+from discord.channel import VocalGuildChannel
 from discord.ext import commands
 
 from services import media, suggestions
@@ -30,14 +31,18 @@
 
 def _is_playlist_url(query: str) -> bool:
     """Detect playlist/set/album URLs across supported sites."""
-    q = query.lower()
-    if not q.startswith("http"):
+    if not media.is_url(query):
         return False
+    q = query.lower()
     if "list=" in q and "watch?v=" not in q:
         return True                        # YouTube playlist
     return "/sets/" in q or "/album/" in q  # SoundCloud set / Bandcamp album
 
 
+def _has_listeners(channel: VocalGuildChannel) -> bool:
+    return any(not member.bot for member in channel.members)
+
+
 async def _reply(ctx: GuildContext, outcome: controls.Outcome) -> None:
     """Report an action's outcome; under `/`, a failure only to whoever asked."""
     embed = success_embed(outcome.message) if outcome.ok else error_embed(outcome.message)
@@ -110,7 +115,7 @@ async def play(self, ctx: GuildContext, *, query: str) -> None:
         if len(query) > MAX_QUERY_LEN:
             await ctx.send(embed=error_embed("That query is too long."))
             return
-        if query.startswith("http") and not await media.is_public_url(query):
+        if media.is_url(query) and not await media.is_public_url(query):
             await ctx.send(embed=error_embed(
                 "I can only play links to public websites."))
             return
@@ -120,13 +125,13 @@ async def play(self, ctx: GuildContext, *, query: str) -> None:
 
         async with ctx.typing():
             if _is_playlist_url(query):
-                tracks = await media.get_playlist(query, loop=self.bot.loop)
+                tracks = await media.get_playlist(query)
                 if not tracks:
                     await ctx.send(embed=error_embed("Couldn't load that playlist."))
                     return
                 return await self._enqueue(ctx, player, tracks, "playlist")
 
-            track = await media.search(query, loop=self.bot.loop)
+            track = await media.search(query)
             if not track:
                 await ctx.send(embed=error_embed(f"No results found for `{query}`."))
                 return
@@ -224,7 +229,9 @@ async def nowplaying(self, ctx: GuildContext) -> None:
         if not player or not player.current:
             await ctx.send(embed=error_embed("Nothing is playing right now."))
             return
-        await ctx.send(embed=now_playing_embed(player.current, ctx.author, loop_mode=player.loop_mode))
+        requester = player.current.get("requester") or ctx.me   # autoplay picks have none
+        await ctx.send(embed=now_playing_embed(player.current, requester,
+                                               loop_mode=player.loop_mode))
 
     @hybrid_command()
     @same_voice_channel()
@@ -320,9 +327,9 @@ async def on_voice_state_update(self, member: discord.Member,
             return
         if before.channel != vc.channel:
             return
-        if len([m for m in vc.channel.members if not m.bot]) == 0:
+        if not _has_listeners(vc.channel):
             await asyncio.sleep(60)
-            if vc.is_connected() and len([m for m in vc.channel.members if not m.bot]) == 0:
+            if vc.is_connected() and not _has_listeners(vc.channel):
                 player = players.get(member.guild.id)
                 if player:
                     player.destroy()

services/media.py

@@ -118,18 +118,28 @@ def _build_track(info: dict[str, Any], *, query: str = "") -> Track:
     }
 
 
+def is_url(query: str) -> bool:
+    """
+    Whether ``query`` is a link rather than search terms.
+
+    The one definition every caller shares: it decides whether the private
+    network check runs, so any second, stricter copy of it is a way around that
+    check. Case-insensitive, because yt-dlp accepts ``HTTP://`` as readily.
+    """
+    return query.lower().startswith(("http://", "https://"))
+
+
 def _first_thumb(info: dict[str, Any]) -> Optional[str]:
     thumbs = info.get("thumbnails") or []
     return thumbs[-1]["url"] if thumbs else None
 
 
-def _run(opts: dict[str, Any], target: str, *,
-         loop: asyncio.AbstractEventLoop) -> asyncio.Future[Any]:
+def _run(opts: dict[str, Any], target: str) -> asyncio.Future[Any]:
     """Run yt-dlp's blocking extract_info in a thread executor."""
     def _extract() -> Any:
         with yt_dlp.YoutubeDL(opts) as ydl:
             return ydl.extract_info(target, download=False)
-    return loop.run_in_executor(None, _extract)
+    return asyncio.get_running_loop().run_in_executor(None, _extract)
 
 
 def _search_target(query: str) -> tuple[str, bool]:
@@ -143,63 +153,56 @@ def _search_target(query: str) -> tuple[str, bool]:
     for prefix, engine in _SEARCH_PREFIXES.items():
         if lowered.startswith(prefix):
             return engine + query[len(prefix):].strip(), True
-    if query.startswith("http"):
-        return query, False          # a URL — extract it directly
+    if is_url(query):
+        return query, False          # extract it directly
     return f"ytsearch1:{query}", True
 
 
 # ── Public API ────────────────────────────────────────────────────────
 
-async def search(query: str, *,
-                 loop: Optional[asyncio.AbstractEventLoop] = None) -> Optional[Track]:
+async def search(query: str) -> Optional[Track]:
     """Resolve a single track from a search term or any supported URL."""
-    loop = loop or asyncio.get_event_loop()
     target, flat_ok = _search_target(query)
     opts = {**YTDL_OPTIONS, "extract_flat": flat_ok}
     try:
-        info = await _run(opts, target, loop=loop)
+        info = await _run(opts, target)
         info = _first_entry(info)
         return _build_track(info, query=query) if info else None
     except Exception as e:
         log.warning("Search failed for %r: %s", query, e)
         return None
 
 
-async def search_many(query: str, limit: int = 5, *,
-                      loop: Optional[asyncio.AbstractEventLoop] = None) -> list[Track]:
+async def search_many(query: str, limit: int = 5) -> list[Track]:
     """Return up to ``limit`` YouTube search results (metadata only)."""
-    loop = loop or asyncio.get_event_loop()
     opts = {**YTDL_OPTIONS, "extract_flat": True}
     try:
-        info = await _run(opts, f"ytsearch{limit}:{query}", loop=loop)
+        info = await _run(opts, f"ytsearch{limit}:{query}")
         entries = (info or {}).get("entries", [])
         return [_build_track(e, query=query) for e in entries if e]
     except Exception as e:
         log.warning("Multi-search failed for %r: %s", query, e)
         return []
 
 
-async def get_playlist(url: str, *,
-                       loop: Optional[asyncio.AbstractEventLoop] = None) -> list[Track]:
+async def get_playlist(url: str) -> list[Track]:
     """Extract every track from a playlist/set/album URL (metadata only)."""
-    loop = loop or asyncio.get_event_loop()
     opts = {**YTDL_OPTIONS, "noplaylist": False, "extract_flat": True}
     try:
-        info = await _run(opts, url, loop=loop)
+        info = await _run(opts, url)
         entries = (info or {}).get("entries", [])
         return [_build_track(e) for e in entries if e]
     except Exception as e:
         log.warning("Playlist load failed for %r: %s", url, e)
         return []
 
 
-async def related(track: Track, *,
-                  loop: Optional[asyncio.AbstractEventLoop] = None) -> Optional[Track]:
+async def related(track: Track) -> Optional[Track]:
     """Approximate a 'related' track for autoplay via a themed search."""
     seed = track.get("uploader") or track.get("title") or ""
     if not seed:
         return None
-    for cand in await search_many(f"{seed} mix", limit=5, loop=loop):
+    for cand in await search_many(f"{seed} mix", limit=5):
         if cand.get("url") and cand["url"] != track.get("url"):
             return cand
     return None

services/suggestions.py

@@ -12,6 +12,8 @@
 
 import aiohttp
 
+from services.media import is_url
+
 log = logging.getLogger("loopify.suggestions")
 
 SUGGEST_URL = "https://suggestqueries.google.com/complete/search"
@@ -23,7 +25,7 @@
 async def fetch(session: aiohttp.ClientSession, query: str) -> list[str]:
     """Up to MAX_SUGGESTIONS search terms completing ``query``."""
     query = query.strip()
-    if not query or query.startswith("http"):
+    if not query or is_url(query):
         return []
     try:
         async with session.get(

tests/test_commands.py

@@ -155,13 +155,28 @@ async def test_play_rejects_an_over_length_query(music_cog, ctx):
 @pytest.mark.parametrize("url", [
     "http://192.168.1.1/admin",
     "http://127.0.0.1:8080/",
+    # yt-dlp lowercases the scheme itself, so case must not skip the check —
+    # least of all on the playlist path, whose own detection ignores case.
+    "HTTP://192.168.1.1/admin",
+    "Http://10.0.0.1/playlist?list=PL123",
+    "HTTPS://10.0.0.1/sets/x",
 ])
 async def test_play_refuses_links_into_private_networks(music_cog, ctx, url):
     ctx.author.voice.channel = MagicMock()
-    with patch("cogs.music.media.search", new=AsyncMock()) as search:
+    with patch("cogs.music.media.search", new=AsyncMock()) as search, \
+         patch("cogs.music.media.get_playlist", new=AsyncMock()) as playlist:
         await Music.play.callback(music_cog, ctx, query=url)
     assert "public" in sent_text(ctx)
     search.assert_not_awaited()
+    playlist.assert_not_awaited()
+
+
+async def test_play_does_not_mistake_a_search_for_a_link(music_cog, ctx):
+    music_cog._ensure_voice = AsyncMock(return_value=False)
+    with patch("cogs.music.media.is_public_url", new=AsyncMock()) as check:
+        await Music.play.callback(music_cog, ctx, query="httpster live")
+    check.assert_not_awaited()
+    music_cog._ensure_voice.assert_awaited_once()
 
 
 async def test_shuffle_on_an_empty_queue(music_cog, ctx):
@@ -366,9 +381,8 @@ async def test_effect_by_name_applies_it(effects_cog, ctx):
     ctx.author.voice.channel = ctx.voice_client
     with patch.object(effects_cog, "_switch_to", new=AsyncMock()) as switch:
         await Effects.effect.callback(effects_cog, ctx, "Nightcore")
-    name, flt, _label, rate = switch.await_args.args[1:]
-    assert name == "nightcore" and flt == EFFECTS["nightcore"].filter
-    assert rate == EFFECTS["nightcore"].rate
+    name, effect = switch.await_args.args[1:]
+    assert name == "nightcore" and effect is EFFECTS["nightcore"]
 
 
 async def test_an_unknown_effect_name_is_refused(effects_cog, ctx):
@@ -390,3 +404,31 @@ async def test_effects_list_under_slash_points_at_effect(effects_cog, ctx):
     await Effects.list_effects.callback(effects_cog, ctx)
     text = sent_text(ctx)
     assert "/effect" in text and "`/bass`" not in text
+
+
+# -- what the read-only commands show -----------------------------------
+
+async def test_nowplaying_credits_whoever_queued_the_track(music_cog, ctx, fake_bot,
+                                                           fake_guild, track_factory):
+    queued_by = MagicMock(mention="<@queued-by>")
+    player = players.get_or_create(fake_bot, ctx.guild, ctx.channel)
+    try:
+        player.current = track_factory("Song", requester=queued_by)
+        ctx.author.mention = "<@asked-for-np>"
+        await Music.nowplaying.callback(music_cog, ctx)
+    finally:
+        players.discard(fake_guild.id)
+    embed = ctx.send.await_args.kwargs["embed"]
+    requested = next(f.value for f in embed.fields if "Requested" in f.name)
+    assert requested == "<@queued-by>"
+
+
+@pytest.mark.parametrize("page,shown", [(0, 1), (-1, 1), (99, 3)])
+def test_queue_pages_out_of_range_show_the_nearest_real_one(track_factory, page, shown):
+    from utils.embeds import queue_embed
+
+    queue = [track_factory(f"T{i}") for i in range(1, 26)]       # three pages
+    embed = queue_embed(queue, None, page=page)
+    assert embed.footer.text.startswith(f"Page {shown}/3")
+    first = (shown - 1) * 10 + 1
+    assert embed.fields[0].value.startswith(f"`{first}.` **T{first}**")

tests/test_errors.py

@@ -149,6 +149,8 @@ async def test_an_unexpected_error_is_acknowledged_and_logged(ctx, caplog):
 
     assert "went wrong" in sent_text(ctx).lower()
     assert "something broke internally" in caplog.text, "the operator still needs it"
+    # A bug, not a user mistake: it must stand out from the warnings around it.
+    assert any(r.levelname == "ERROR" and r.exc_info for r in caplog.records)
 
 
 async def test_the_acknowledgement_leaks_no_internals(ctx):

tests/test_media_helpers.py

@@ -45,6 +45,16 @@ def test_urls_are_passed_through_untouched(url):
     assert flat_ok is False, "a URL must be extracted directly, not flat-listed"
 
 
+def test_url_detection_ignores_the_case_of_the_scheme():
+    target, flat_ok = media._search_target("HTTPS://www.youtube.com/watch?v=abc")
+    assert target == "HTTPS://www.youtube.com/watch?v=abc"
+    assert flat_ok is False
+
+
+def test_a_word_starting_with_http_is_a_search_not_a_link():
+    assert media._search_target("httpster live") == ("ytsearch1:httpster live", True)
+
+
 def test_a_prefix_wins_over_url_detection():
     """`sc: https://...` should search SoundCloud, not extract the URL."""
     target, _ = media._search_target("sc: https://example.invalid/x")