Skip to content

fix(bot): fail closed on invalid role lists and localize the deny reply - #142

Merged
retardgerman merged 3 commits into
devfrom
fix/role-check-hardening
Nov 6, 2026
Merged

retardgerman merged 3 commits into
devfrom
fix/role-check-hardening

Conversation

@retardgerman

Copy link
Copy Markdown
Contributor

Builds on #141. Merge that first; this PR then only shows its own commit.

Problem

  • If ROLE_ALLOWLIST or ROLE_BLOCKLIST contained invalid JSON (e.g. after hand-editing config.json), the list was ignored and everyone was allowed. Values that parse but aren't a list of role IDs behaved inconsistently.
  • The "no permission" reply was hardcoded in English, and in DMs it was misleading: the user may have the right role, the bot just can't check it there.
  • Denied interactions weren't logged, so admins couldn't tell why users reported being blocked.

Fix

  • An invalid role list now denies role-restricted interactions and logs an error. The error is logged when it appears, not on every interaction, and again if it comes back after being fixed.
  • The deny reply comes from locales/ (English and German included). In DMs it tells users to use the command in the server.
  • Denials are logged with the user ID and whether it was a DM or which server.

checkRolePermission() returned true whenever there was no guild member,
which is the case for interactions in a DM with the bot. Since slash
commands are registered globally and available in DMs, a user missing
from ROLE_ALLOWLIST or listed in ROLE_BLOCKLIST could still use /request
and /search by messaging the bot directly. The check also ignored which
guild an interaction came from, so a blocklisted user could use the bot
in any other server it is in, where the blocked role doesn't exist.

When a role list is set, interactions are now only allowed from members
of the configured GUILD_ID (denied and logged if GUILD_ID is empty).
Autocomplete is gated as well (empty suggestions instead of TMDB/Seerr
lookups), and two select menu prefixes that no handler uses are no
longer exempt from the check. Without role lists, nothing changes.
An unparseable ROLE_ALLOWLIST or ROLE_BLOCKLIST was treated as an empty
list, which silently turned access control off for everyone. Values that
parse but aren't an array of role ID strings behaved inconsistently
between guild members and DMs. Both cases now deny role-restricted
interactions and log the reason.

The deny reply now comes from locales/ and tells DM users to use the
command in the server instead of claiming they lack permission. Denials
are logged with the user ID and whether it was a DM or which guild.
@retardgerman
retardgerman merged commit 5b13c1e into dev Nov 6, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant