Overview

The wand was raised. Seven bugs disarmed, four quality improvements landed, and the codebase emerges cleaner than ever. No new features this time — just the quiet, necessary work of making everything that already exists work correctly.

“Expelliarmus Maxima! Every bug yields. Every edge case surrenders.” 🏰🔍

🔴 B1 — Partyline .bcast: Wrong Session Hash

Module: Partyline.pm — _cmd_bcast

_cmd_bcast was reading $self->{sessions}{$id} for the operator’s level. Auth data is stored in $self->{users}{$id}. The level resolved to undef // 0 — which meant the <= 1 check always passed, and any authenticated Partyline user could broadcast to all channels, regardless of actual level.

# Before — always 0, check always passes
my $session = $self->{sessions}{$id} // {};
my $level   = $session->{level} // 0;

# After — correct hash, safe default
my $session = $self->{users}{$id} // {};
my $level   = $session->{level} // 99;

The log line also showed undef for the operator nick — fixed implicitly by reading from the correct hash.

🟠 B2 — !define: HTML Entities Not Decoded

Module: UserCommands.pm — mbDefine_ctx

Wiktionary definitions strip HTML tags correctly (<[^>]+>), but the content also contains HTML entities like &, ', <. These were passed through raw to IRC.

# Before
serendipity: An unsought & unexpected, yet fortunate discovery...

# After
serendipity: An unsought & unexpected, yet fortunate discovery...

Fix: added HTML::Entities::decode_entities($first_def) after the tag strip — consistent with how mbTrivia_ctx already handled its Wiktionary content.

🟡 B3 — checkTriviaAnswer: No eval Guard

Module: UserCommands.pm — checkTriviaAnswer

The regex lc($text) =~ /\Q$trivia->{answer}\E/ had no eval {} wrapper and no defined check on $trivia->{answer}. While \Q...\E prevents ReDoS, an undef answer could still emit a Perl warning on every public message during an active trivia.

# After
return unless defined $trivia->{answer};
my $matched = eval {
    lc($text) eq $trivia->{answer}
    || lc($text) =~ /\Q$trivia->{answer}\E/
};
return unless $matched;

🟠 B4 — !streak: Float Comparison on ->days

Module: UserCommands.pm — mbStreak_ctx

Time::Piece’s subtraction operator returns a Time::Seconds object. Calling ->days on it yields a float, not an integer. The comparison ($d1 - $d2)->days == 1 could silently fail on some platforms.

# Before — fragile float comparison
last unless ($d1 - $d2)->days == 1;

# After — safe rounding
last unless int(($d1 - $d2)->days + 0.5) == 1;  # B4/fix

🟠 B5 — !active: No Output Limit

Module: UserCommands.pm — mbActive_ctx

!active 24h on a busy channel could return 50+ nicks in a single JOIN on CHANNEL_LOG. The resulting IRC line would exceed 512 bytes and be silently truncated by the server.

Two-part fix:

-- SQL cap
ORDER BY cl.nick LIMIT 30

# Output truncation
if (length($list) > 350) { $list = substr($list, 0, 347) . '...'; }

🟡 B6 — !alias: Cache Never Loaded at Boot

Module: UserCommands.pm — mbAlias_ctx

mbAlias_ctx maintains an in-memory _alias_cache hash written on every !alias set or !alias del. But on bot restart, the cache starts empty — aliases stored in BOT_ALIAS were never loaded back from DB.

Fix: lazy-load on first !alias call via a _alias_cache_loaded flag:

unless ($self->{_alias_cache_loaded}) {
    my $sth = $self->{dbh}->prepare('SELECT alias, command FROM BOT_ALIAS');
    if ($sth && $sth->execute()) {
        while (my $r = $sth->fetchrow_hashref) {
            $self->{_alias_cache}{ $r->{alias} } = $r->{command};
        }
        $sth->finish;
    }
    $self->{_alias_cache_loaded} = 1;
}

🟡 B7 — !poll: No Automatic Expiry

Module: UserCommands.pm — mbPoll_ctx / mbVote_ctx

A poll stayed active indefinitely if !pollstop was forgotten. On the next !vote, the bot would still accept votes on a days-old poll.

Fix: deadline = time() + 300 (5 minutes) set at poll creation. mbVote_ctx now checks the deadline before accepting a vote:

if ($poll && $poll->{active} && time() > ($poll->{deadline} // 0)) {
    $poll->{active} = 0;
    botPrivmsg($self, $channel, 'Poll expired. Use !pollresult to see results.');
    return;
}

🟠 C1 — !calc: Inner Subs Redefined on Every Call

Module: DBCommands.pm — mbCalc_ctx

sub round, sub deg2rad, and sub rad2deg are defined inside the eval block that runs on every !calc call. Perl emits a Subroutine ... redefined warning on each invocation after the first, flooding the logs.

Fix: one line, zero risk.

no warnings 'redefine';
sub round   { int($_[0] + 0.5 * ($_[0] >= 0 ? 1 : -1)) }
sub deg2rad { $_[0] * 3.14159265358979 / 180 }
sub rad2deg { $_[0] * 180 / 3.14159265358979 }

🟠 C2 — processKarma: No Per-Message Limit

Module: UserCommands.pm — processKarma

A single message like a++ b++ c++ d++ e++ f++ would trigger 6 DB updates and 6 bot responses — a trivial flood vector.

Fix: cap at 3 karma changes per message.

my $karma_hits = 0;
while ($text =~ /([^\s+\-]{2,32})(\+\+|--)/g) {
    last if ++$karma_hits > 3;
    ...
}

🟡 C3 — !stats Karma Block: Direct dbh Access

Module: UserCommands.pm — mbStats_ctx (S7 karma block)

The karma lookup added in the previous session used $self->{dbh} directly. If the DB connection had dropped between the main query and this block, the karma queries would fail silently. Fixed to use ensure_connected:

my $dbh_k = eval { $self->{db}->ensure_connected } // $self->{dbh};

📝 C4 — !note: time() as Note ID

Module: UserCommands.pm — mbNote_ctx

Notes used id => time() — two notes added in the same second would have the same display ID. Cosmetic only (notes are stored in an array, not a DB), but confusing for the user.

# Before
push @notes, { id => time(), text => $text };

# After
my $note_id = scalar(@notes) + 1;
push @notes, { id => $note_id, text => $text };

✨ Small Gains: S2, S3, S6, S7

📦 Files Changed