Forum teuk.org

🛡️ Safer Command Wrappers and Chanset Permission Fixes

in Mediabot · started by TeuK · 1w ago

TeuK · 1w ago

🛡️ Safer Command Wrappers and Chanset Permission Fixes

Article

This update is a maintenance and reliability pass focused on command wrappers, malformed context handling, and a permission bug that affected chanset.

The theme is simple: commands should not crash or deny valid users because of fragile assumptions in wrapper code.

Defensive command arguments

Several command wrappers were hardened to avoid blindly dereferencing:

@{ $ctx->args }

or the slightly safer but still incomplete form:

@{ $ctx->args // [] }

The second form protects only against undef. It still dies if args is present but not an array reference.

The wrappers now use the defensive pattern:

my @args = (ref($ctx->args) eq 'ARRAY') ? @{ $ctx->args } : ();

This keeps normal behavior unchanged while avoiding Not an ARRAY reference crashes when a malformed command context is ever passed through the dispatcher.

Modules covered

The cleanup touched several command areas:

Mediabot/External.pm
Mediabot/AdminCommands.pm
Mediabot/ChannelCommands.pm
Mediabot/LoginCommands.pm
Mediabot/UserCommands.pm
Mediabot/Helpers.pm

The affected commands include:

chatGPT
tmdb
debug
user topic
tmdblangset
pass
ident
login
adduser
deluser
echo
resolve

The goal was not to refactor the commands themselves. It was only to make their context argument handling safer and more consistent.

TMDB JSON robustness

TMDB response handling was hardened.

The code already skipped malformed individual results, but it still assumed that the top-level results field was always an array.

Now the code checks that:

ref($data) eq 'HASH'
ref($data->{results}) eq 'ARRAY'

before dereferencing it.

That prevents crashes if TMDB ever returns valid JSON with an unexpected structure, such as:

{ "results": {} }

or:

{ "results": "oops" }

In those cases the command safely returns no result instead of dying.

DNS resolve timeout safety

The resolve helper already used a child process to avoid blocking the IRC loop during DNS lookups.

However, the timeout path still had a dangerous pattern:

waitpid($child_pid, 0)

If the child process was still stuck, the timeout handler itself could block.

That path now uses a safer sequence:

non-blocking waitpid(WNOHANG)
TERM
short wait
KILL if still alive

This makes the timeout path actually non-blocking.

ChatGPT response parsing guard

The chatGPT() helper decoded JSON under eval, but then assumed the decoded structure always contained:

choices[0].message.content

That is fine for a successful API response, but not for error-shaped JSON such as:

{ "error": { "message": "..." } }

The parser now checks each level before dereferencing:

HASH response
ARRAY choices
HASH choices[0]
HASH message
defined content

If the response is valid JSON but not in the expected shape, the bot reports:

Could not read API response.

without crashing.

Chanset permission bug

A real permission bug was fixed in channelSet_ctx().

The code was calling:

$user->has_level($self, 'Administrator')

but has_level() expects the required level as its argument. Passing the bot object first made the global admin check fail.

This caused an Owner/Admin user to be recognized correctly, but still get:

Your level does not allow you to use this command.

The call now correctly uses:

$user->has_level('Administrator')

The command still also allows per-channel access via the existing channel-level check.

UrlTitle reminder

For X/Twitter, Facebook, Instagram, Spotify, YouTube previews, and generic title extraction, the relevant channel setting is:

UrlTitle

So to enable title fetching on a channel:

u chanset #channel +UrlTitle

Twitter is not the flag used by the current URL title handler.

Regression tests added

New regression tests were added for:

  • TMDB results being required to be an array;
  • defensive context arguments in External.pm;
  • defensive context arguments in AdminCommands.pm;
  • defensive context arguments in ChannelCommands.pm;
  • defensive context arguments in LoginCommands.pm;
  • defensive context arguments in UserCommands.pm;
  • defensive context arguments in Helpers.pm;
  • non-blocking resolve timeout cleanup;
  • guarded ChatGPT JSON structure parsing;
  • fixed chanset Owner/Admin permission check.

Why this matters

This update is about resilience.

It does not change how commands are supposed to behave when everything is normal. Instead, it prevents avoidable crashes and false denials when inputs, API responses, or command contexts are not exactly what the code expected.

That is the kind of work that makes the bot less fragile over time.

Suggested commit message

🛡️ Protego Contextum: guard command wrappers and restore chanset authority

Validation command

cd /home/mediabot/mediabot_v3 || exit 1

find Mediabot -name '*.pm' -print0 | xargs -0 -n1 runuser -u mediabot -- perl -I. -c

runuser -u mediabot -- perl -c mediabot.pl

runuser -u mediabot -- perl t/test_commands.pl --filter '158|159|160|161|162|163|164|165|166|167|168'

Suggested smoke tests

From IRC:

u chanset #macintosh +UrlTitle
https://x.com/EYakoby/status/2052086640558977311?s=20
m tmdb piège de cristal
m resolve teuk.org

Expected behavior:

  • Owner/Admin can use chanset again.
  • UrlTitle enables X/Twitter title handling.
  • TMDB still works normally.
  • resolve remains responsive and does not block the bot.

You must be logged in to reply.