Forum teuk.org

Runtime Safety and Timer Consistency Pass

in Mediabot · started by TeuK · 1w ago

TeuK · 1w ago

Mediabot v3 — Runtime Safety and Timer Consistency Pass

Context

This pass continued the defensive cleanup of Mediabot v3, with a focus on the paths that run continuously or during important runtime events:

  • authentication checks;
  • autologin updates;
  • channel object database methods;
  • public command statistics;
  • timer listing;
  • timer add/remove consistency.

The goal was not to change user-facing behavior dramatically. The goal was to make the bot safer when the database misbehaves, avoid runtime/DB divergence, and prevent a few subtle correctness bugs from coming back later.


Suggested commit message

🧙 Protego Temporis: harden auth, channel spells, and timer consistency

Alternative shorter version:

🧙 Protego Temporis: harden runtime DB paths and timers

1. Auth credential lookup hardening

Mediabot::Auth::verify_credentials() still had DB calls wrapped in eval with unchecked prepare() / execute() calls.

That meant failures were caught, but not handled as cleanly as the rest of the codebase now expects.

The function now:

  • checks the DB handle;
  • rejects empty lookup keys;
  • checks prepare();
  • checks execute();
  • logs clear DB errors;
  • finishes the statement before continuing;
  • keeps the existing password verification behavior.

Files changed:

Mediabot/Auth.pm
t/cases/64_auth_credentials_update_db_safety.t

2. Autologin update hardening

maybe_autologin() also had an unchecked update path:

UPDATE USER SET auth=1, last_login=NOW() WHERE id_user=?

The update is now handled explicitly:

  • DB handle is checked;
  • prepare() failure returns a clear reason;
  • execute() failure returns a clear reason;
  • statement handles are finished properly;
  • in-memory session state is only updated after the DB update succeeds.

This avoids misleading runtime auth state if the database update fails.

Files changed:

Mediabot/Auth.pm
t/cases/64_auth_credentials_update_db_safety.t

3. Mediabot::Channel object DB safety

Mediabot::Channel is becoming a stable base for the ongoing channel refactoring, so it needed the same DB safety treatment as the helpers.

Several methods previously called:

my $sth = $self->{dbh}->prepare(...);
$sth->execute(...);

without checking whether prepare() succeeded.

A shared helper was introduced:

_execute_update()

It handles:

  • prepare failure;
  • execute failure;
  • logging;
  • statement cleanup;
  • explicit success/failure return.

The following methods now use safer DB handling:

get_user_level()
get_user_info()
set_topic()
set_tmdb_lang()
set_key()
set_description()
set_chanmode()
set_auto_join()
exists_in_db()
create_in_db()

The setters now return a clear boolean result.

create_in_db() also retrieves last_insert_id from the DB handle after finishing the statement.

Files changed:

Mediabot/Channel.pm
t/cases/65_channel_object_db_safety.t

4. popcmd exact handle lookup

popcmd <nickhandle> was still using a LIKE pattern against the user handle.

That was unnecessary because popcmd is not a wildcard search command. It targets one exact handle.

Before:

WHERE U.nickname LIKE ? ESCAPE '!'

After:

WHERE U.nickname = ?

The function was also hardened:

  • prepare() checked;
  • execute() checked;
  • SQL errors reported to the user;
  • statement handle finished before output processing;
  • existing pagination preserved.

Files changed:

Mediabot/DBCommands.pm
t/cases/66_popcmd_exact_lookup_db_safety.t

5. .timers count bug fixed

A real correctness bug was found in mbTimers_ctx().

The timer counter was incremented twice per DB timer:

$count++;
...
$count++;

So one timer could be counted as two, three timers as six, and so on.

The new version derives the count from collected timer lines:

my $count = scalar(@timer_lines);

No more manual counter drift.

Files changed:

Mediabot/DBCommands.pm
t/cases/67_timers_count_pagination.t

6. .timers output pagination

The .timers command was also improved to produce cleaner output.

It now sends:

DB timers: X result(s)
timer[01]: ...
timer[02]: ...

And if the runtime scheduler is available:

Scheduler tasks: X result(s)
schedule[01]: ...
schedule[02]: ...

This makes the command readable even when there are several DB timers and several scheduler tasks.

It also now checks:

  • prepare();
  • execute();
  • statement cleanup on failure.

Files changed:

Mediabot/DBCommands.pm
t/cases/67_timers_count_pagination.t

7. addtimer DB/runtime consistency

addtimer previously used dbh->do() inside eval.

It now uses explicit prepared statements and a safer order of operations.

Important behavior changes:

  • validates timer name;
  • validates interval range;
  • validates command length;
  • validates IRC verb;
  • checks runtime duplicate;
  • checks DB duplicate;
  • inserts into DB before starting the runtime timer.

That last point matters: if the database insert fails, Mediabot no longer creates a memory-only timer that would disappear at restart.

Files changed:

Mediabot/DBCommands.pm
t/cases/68_timer_add_remove_db_safety.t

8. remtimer DB/runtime consistency

remtimer now also avoids dbh->do() inside eval.

It deletes from the database first, then removes the runtime timer.

This prevents the opposite inconsistency: a timer being removed from memory while still present in the database and therefore coming back after a restart.

It now:

  • checks prepare();
  • checks execute();
  • checks affected rows;
  • reports runtime/DB divergence;
  • only removes the runtime timer after a successful DB delete.

Files changed:

Mediabot/DBCommands.pm
t/cases/68_timer_add_remove_db_safety.t

9. Regression tests added

This pass added tests covering the new behavior:

t/cases/64_auth_credentials_update_db_safety.t
t/cases/65_channel_object_db_safety.t
t/cases/66_popcmd_exact_lookup_db_safety.t
t/cases/67_timers_count_pagination.t
t/cases/68_timer_add_remove_db_safety.t

These tests check for:

  • missing prepare() guards;
  • missing execute() guards;
  • missing statement cleanup;
  • old eval/dbh->do() timer writes;
  • accidental SQL LIKE in popcmd;
  • duplicate timer count regressions;
  • DB-before-runtime consistency for timers.

Suggested validation

Syntax checks:

perl -I. -c Mediabot/Auth.pm
perl -I. -c Mediabot/Channel.pm
perl -I. -c Mediabot/DBCommands.pm
perl -I. -c mediabot.pl

Full regression suite:

env LANG=C.UTF-8 LC_ALL=C.UTF-8 perl t/test_commands.pl --verbose

Useful live checks:

m login teuk <password>
m whoami
m popcmd teuk
m popcmd %
m timers
m addtimer testtimer 60 PRIVMSG #teuk timer-test
m timers
m remtimer testtimer
m timers

Expected behavior:

  • login still works normally;
  • autologin still works normally;
  • popcmd % no longer behaves like a wildcard lookup;
  • .timers no longer doubles the timer count;
  • timer add/remove keeps runtime and database state aligned.

Suggested Git commands

cd /home/mediabot/mediabot_v3

git status --short
git diff --stat

Stage the intended files:

git add   Mediabot/Auth.pm   Mediabot/Channel.pm   Mediabot/DBCommands.pm   t/cases/64_auth_credentials_update_db_safety.t   t/cases/65_channel_object_db_safety.t   t/cases/66_popcmd_exact_lookup_db_safety.t   t/cases/67_timers_count_pagination.t   t/cases/68_timer_add_remove_db_safety.t

Check staged content:

git diff --cached --stat
git diff --cached --name-only

Commit:

git commit -m "🧙 Protego Temporis: harden auth, channel spells, and timer consistency"

Push:

git push

Closing note

This was another defensive pass, but an important one.

Authentication, channel state, and timers are the kind of code paths that must stay boring and predictable. With this work, Mediabot is less likely to drift between memory and database state, less likely to trip over a database hiccup, and better protected against old fragile patterns returning later.

A few more protective charms, and the bot keeps getting sturdier.

You must be logged in to reply.