fix: protect admins/mods/owner from moderation actions (#174)#186
fix: protect admins/mods/owner from moderation actions (#174)#186BillChirico merged 9 commits intomainfrom
Conversation
- Add moderation.protectRoles config (enabled, includeAdmins, includeModerators, includeServerOwner, roleIds) - Export isProtectedTarget() from moderation.js — checks server owner, adminRoleId, moderatorRoleId, and custom roleIds - Guard executeModAction with protection check before hierarchy check; add skipProtection flag for lift-punishment commands - Add skipProtection: true to untimeout (removing punishment) - Extend configValidation.js schema with protectRoles shape - Add ModerationProtectRoles TypeScript interface to web types - Add isProtectedTarget unit tests (9 cases) in moderation.test.js - Add executeModAction protection path tests in modAction.test.js - Update moderation.js mock in all affected command tests
Biome enforces LF line endings. Without this file, Git's core.autocrlf=true on Windows converts LF->CRLF on checkout causing 373+ format errors locally. * text=auto eol=lf overrides autocrlf and ensures all text files are checked out with LF regardless of OS or Git config.
This reverts commit 2b6262d.
Fixes 373 Biome format errors on Windows caused by core.autocrlf=true converting LF->CRLF on checkout. * text=auto eol=lf ensures all text files are checked out with LF regardless of OS or local Git config.
- Self-moderation: prevent mods from moderating themselves (both self-mod + protectRoles checks now share skipProtection flag so untimeout can still target self) - Audit log: warn() when protection blocks an action with action/targetId/moderatorId/guildId context - Triage: sendModerationLog skips embed when any flagged user is a protected role (admin/mod/owner); logs warn with userId/channelId - Dashboard UI: Protect Roles fieldset added to Moderation Card in config-editor.tsx (toggle + 3 checkboxes + role IDs text input) - ModerationSection component updated with onProtectRolesChange prop and matching UI - Tests: +5 tests (self-mod, warn log, skipProtection bypass, triage skip-protected, triage send-when-not-protected)
Original PR by Brian Munene Mwirigi (@brian-mwirigi) Changes: - isProtectedTarget() exported from moderation.js — checks server owner, adminRoleId, moderatorRoleId, and custom roleIds - All mod commands blocked from targeting protected users via executeModAction - Self-moderation prevention — mods cannot moderate themselves - skipProtection: true on /untimeout (removing a punishment should never be blocked) - Audit log: warn() fires with context whenever protection blocks an action - Triage: sendModerationLog skips the moderation embed if a flagged user is protected - Web dashboard: Protect Roles config added to Moderation card - Config validation schema updated - TypeScript types added (ModerationProtectRoles interface)
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (19)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements role protection for moderation commands, preventing the bot from applying moderation actions to users with admin/moderator roles, the server owner, or any custom configured protected roles. It addresses issue #174 where the bot could warn or moderate admin users.
Changes:
- New
isProtectedTarget()function exported fromsrc/modules/moderation.jsthat checks if a target is protected, with sensible defaults - Protection logic integrated into
executeModActionhelper (blocks all mod commands), withskipProtection: trueon/untimeout; self-moderation prevention also added - Web dashboard "Protect Roles from Moderation" fieldset added to the Moderation config card, plus TypeScript types, config validation schema, and
config.jsondefaults
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/modules/moderation.js |
Adds isProtectedTarget() with opt-in defaults and role/owner checks |
src/utils/modAction.js |
Integrates self-moderation prevention and isProtectedTarget check via skipProtection flag |
src/modules/triage-respond.js |
Skips moderation log embed for protected users in auto-triage |
src/commands/untimeout.js |
Adds skipProtection: true so removing a timeout is never blocked |
src/api/utils/configValidation.js |
Schema updated with protectRoles object definition |
config.json |
Default protectRoles config block added under moderation |
web/src/types/config.ts |
New ModerationProtectRoles TypeScript interface |
web/src/components/dashboard/config-editor.tsx |
Protect Roles fieldset with toggles and role ID input added to Moderation card |
tests/modules/moderation.test.js |
Comprehensive unit tests for isProtectedTarget |
tests/utils/modAction.test.js |
Tests for self-moderation and protection-block paths |
tests/modules/triage-respond.test.js |
Tests for protected-target skip in triage log |
tests/commands/kick.test.js |
Adds protection test at the command level |
tests/commands/ban.test.js, softban.test.js, tempban.test.js, timeout.test.js, warn.test.js |
isProtectedTarget mock added |
README.md |
protectRoles config keys documented in Moderation section |
.gitattributes |
New file enforcing LF line endings |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('returns false when protectRoles config is absent', () => { | ||
| const target = makeTarget('user1'); | ||
| const guild = makeGuild('owner1'); | ||
| const config = { moderation: {} }; | ||
| expect(isProtectedTarget(target, guild, config)).toBe(false); | ||
| }); |
There was a problem hiding this comment.
The test description "returns false when protectRoles config is absent" is misleading. The function uses enabled: true as a default when protectRoles is absent from config. The test only passes false because the test user is not the server owner and there are no permissions roleIds configured — not because protection is disabled.
If the same config ({ moderation: {} }) were used with a target who IS the server owner, isProtectedTarget would return true (because includeServerOwner defaults to true). The test should be renamed to reflect the actual scenario tested, e.g., "returns false for non-owner user when no roleIds are configured and protectRoles is absent", or a separate test should be added to document the defaulting behaviour for the server owner.
| const defaultProtectRoles = { | ||
| enabled: true, | ||
| includeAdmins: true, | ||
| includeModerators: true, | ||
| includeServerOwner: true, | ||
| roleIds: [], | ||
| }; | ||
|
|
||
| // Deep-merge defaults so a partial persisted object (e.g. only roleIds set) | ||
| // never leaves enabled/include* as undefined/falsy. | ||
| const protectRoles = { ...defaultProtectRoles, ...config.moderation?.protectRoles }; |
There was a problem hiding this comment.
The isProtectedTarget function defaults enabled: true when protectRoles is absent from the persisted config. This means all existing guilds without an explicit protectRoles config block will silently have protection enabled after this update, which is a breaking behavior change.
Specifically, if config.permissions.adminRoleId or config.permissions.moderatorRoleId is set on a guild that has not yet configured protectRoles, admins and moderators will immediately become un-moderatable. The PR description does not mention this as intentional for existing deployments.
Consider defaulting to enabled: false when the key is absent (opt-in semantics), or at minimum document this behavior prominently in the migration notes. The config.json already ships the block, so new deployments are unaffected, but existing guilds whose config was persisted before this change will be silently affected.
|
| Filename | Overview |
|---|---|
| src/modules/moderation.js | Adds isProtectedTarget() function with proper default handling and role checking logic |
| src/utils/modAction.js | Integrates protection checks into executeModAction with self-moderation blocking and audit logging |
| src/modules/triage-respond.js | Skips moderation logs when AI flags protected users, with proper error handling |
| src/commands/untimeout.js | Adds skipProtection: true flag for untimeout command (removing punishment should never be blocked) |
| tests/modules/moderation.test.js | Comprehensive test coverage for isProtectedTarget() with all edge cases |
| tests/utils/modAction.test.js | Tests protection blocking, self-moderation prevention, and skipProtection flag behavior |
| web/src/components/dashboard/config-editor.tsx | Adds UI fieldset for role protection configuration with toggle switches and role ID input |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Moderation Action Initiated] --> B{Is target<br/>same as moderator?}
B -->|Yes| C[❌ Block: Cannot moderate yourself]
B -->|No| D{skipProtection<br/>flag set?}
D -->|Yes| E[Skip to Hierarchy Check]
D -->|No| F{Is protection<br/>enabled in config?}
F -->|No| E
F -->|Yes| G{Is target<br/>server owner?}
G -->|Yes & includeServerOwner| C2[❌ Block: Protected user]
G -->|No| H{Does target have<br/>protected role?}
H -->|Yes| C2
H -->|No| E
E --> I[Continue with hierarchy<br/>and action execution]
T[Triage AI Flags User] --> T1{Is flagged user<br/>protected?}
T1 -->|Yes| T2[Skip moderation log]
T1 -->|No| T3[Send moderation log]
Last reviewed commit: 466d929
Closes #174
Original PR by Brian Munene Mwirigi (@brian-mwirigi) - #181
This is a replacement PR with merge conflicts resolved.
What changed
isProtectedTarget()exported frommoderation.js— checks server owner, adminRoleId, moderatorRoleId, and custom roleIds/warn,/kick,/ban,/tempban,/softban,/timeout) blocked from targeting protected users viaexecuteModActionskipProtection: trueon/untimeout(removing a punishment should never be blocked)warn()fires with context whenever protection blocks an actionsendModerationLogskips the moderation embed if a flagged user is a protected roleconfigValidation.jsschema updatedModerationProtectRolesinterface)