Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions TASK.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# TASK: Multi-role permissions (admin + moderator)

Branch: `feat/multi-role-permissions`
Repo: VolvoxLLC/volvox-bot
Work in: `/home/bill/worktrees/volvox-bot-multi-roles`

## Goal
Allow configuring multiple Discord roles as admin and multiple roles as moderator, instead of a single role each.

## Schema change
- `config.permissions.adminRoleId: string | null` → `config.permissions.adminRoleIds: string[]`
- `config.permissions.moderatorRoleId: string | null` → `config.permissions.moderatorRoleIds: string[]`

## Files to change

### 1. `src/config.json` (or wherever defaults live)
- Find the permissions defaults
- Change `adminRoleId: null` → `adminRoleIds: []`
- Change `moderatorRoleId: null` → `moderatorRoleIds: []`

### 2. `web/src/types/config.ts`
- Line ~191: Change `adminRoleId: string | null` → `adminRoleIds: string[]`
- Line ~192: Change `moderatorRoleId: string | null` → `moderatorRoleIds: string[]`

### 3. `src/utils/permissions.js`
- Line 66: `config.permissions?.adminRoleId` check → check if member has ANY role in `config.permissions?.adminRoleIds ?? []`
```js
const adminRoleIds = config.permissions?.adminRoleIds ?? [];
if (adminRoleIds.length > 0) {
return adminRoleIds.some(id => member.roles.cache.has(id));
}
```
- Line 162-169: Same pattern for both admin and moderator checks
- Keep backward compat: if `adminRoleId` (singular) exists in config, treat it as `[adminRoleId]` so old configs still work

### 4. `src/utils/modExempt.js`
- Update to check array: `(config.permissions?.adminRoleIds ?? []).some(id => member.roles.cache.has(id))`
- Same for moderatorRoleIds
- Keep backward compat for old singular field

### 5. `src/modules/moderation.js`
- Lines 589-594: Spread the arrays for protect roles:
```js
...(protectRoles.includeAdmins ? (config.permissions?.adminRoleIds ?? []) : []),
...(protectRoles.includeModerators ? (config.permissions?.moderatorRoleIds ?? []) : []),
```

### 6. `web/src/components/dashboard/config-editor.tsx`
- The RoleSelector for admin is currently single-select (wraps value in array, takes `selected[0]`). Change to true multi-select:
- `selected={draftConfig.permissions?.adminRoleIds ?? []}` (no wrapping)
- `onChange={(selected) => updatePermissionsField('adminRoleIds', selected)}`
- Remove `maxSelections={1}` if present
- Same for moderator:
- `selected={draftConfig.permissions?.moderatorRoleIds ?? []}`
- `onChange={(selected) => updatePermissionsField('moderatorRoleIds', selected)}`

### 7. Check for any other references to old singular fields
```bash
grep -rn "adminRoleId\b\|moderatorRoleId\b" src/ web/src/ --include="*.js" --include="*.ts" --include="*.tsx"
```
Fix any remaining references.

### 8. Update tests
- `tests/utils/permissions.test.js` — update mocks and assertions to use arrays
- `tests/utils/modExempt.test.js` — same

## Backward compat pattern
In permissions.js and modExempt.js, support old configs that have the singular field:
```js
const adminRoleIds = config.permissions?.adminRoleIds
?? (config.permissions?.adminRoleId ? [config.permissions.adminRoleId] : []);
```

## Rules
- Conventional commits
- Run `pnpm format && pnpm lint && pnpm test` and `pnpm --prefix web lint && pnpm --prefix web typecheck`
- Do NOT push
4 changes: 2 additions & 2 deletions config.json
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@
},
"permissions": {
"enabled": true,
"adminRoleId": null,
"moderatorRoleId": null,
"adminRoleIds": [],
"moderatorRoleIds": [],
"botOwners": [],
"usePermissions": true,
"allowedCommands": {
Expand Down
27 changes: 15 additions & 12 deletions src/modules/moderation.js
Original file line number Diff line number Diff line change
Expand Up @@ -550,12 +550,11 @@ export function stopTempbanScheduler() {
}

/**
* Check if a target member is protected from moderation actions.
* Protected members include the server owner, admins, moderators, and any custom role IDs
* configured under `moderation.protectRoles`.
* @param {import('discord.js').GuildMember} target - Target member to check
* @param {import('discord.js').Guild} guild - Discord guild
* @returns {boolean} True if the target should not be moderated
* Determine whether a guild member is protected from moderation actions.
* Protection is driven by the guild's live moderation.protectRoles settings (server owner, admin/moderator roles, and explicit role IDs).
* @param {import('discord.js').GuildMember} target - Member to evaluate.
* @param {import('discord.js').Guild} guild - Guild containing the member.
* @returns {boolean} `true` if the member is protected from moderation actions, `false` otherwise.
*/
export function isProtectedTarget(target, guild) {
// Fetch config per-invocation so live config edits take effect immediately.
Expand Down Expand Up @@ -585,13 +584,17 @@ export function isProtectedTarget(target, guild) {
return true;
}

// Resolve admin/moderator role ID arrays with backward compat for old singular fields
const adminRoleIds =
config.permissions?.adminRoleIds ??
(config.permissions?.adminRoleId ? [config.permissions.adminRoleId] : []);
const moderatorRoleIds =
config.permissions?.moderatorRoleIds ??
(config.permissions?.moderatorRoleId ? [config.permissions.moderatorRoleId] : []);

const protectedRoleIds = [
...(protectRoles.includeAdmins && config.permissions?.adminRoleId
? [config.permissions.adminRoleId]
: []),
...(protectRoles.includeModerators && config.permissions?.moderatorRoleId
? [config.permissions.moderatorRoleId]
: []),
...(protectRoles.includeAdmins ? adminRoleIds : []),
...(protectRoles.includeModerators ? moderatorRoleIds : []),
...(Array.isArray(protectRoles.roleIds) ? protectRoles.roleIds : []),
].filter(Boolean);

Expand Down
2 changes: 1 addition & 1 deletion src/utils/dbMaintenance.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
*/

import { info, error as logError, warn } from '../logger.js';
import { getConfig } from '../modules/config.js';
import { purgeOldAuditLogs } from '../modules/auditLogger.js';
import { getConfig } from '../modules/config.js';

/** Track optional tables we've already warned about to avoid hourly log spam */
const warnedMissingOptionalTables = new Set();
Expand Down
23 changes: 16 additions & 7 deletions src/utils/modExempt.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@ import { PermissionFlagsBits } from 'discord.js';
*
* Exempt if the member:
* - has the ADMINISTRATOR Discord permission, OR
* - holds the role at `config.permissions.adminRoleId` (singular ID), OR
* - holds the role at `config.permissions.moderatorRoleId` (singular ID), OR
* - holds any role in `config.permissions.adminRoleIds` (array), OR
* - holds any role in `config.permissions.moderatorRoleIds` (array), OR
* - holds any role ID or name listed in `config.permissions.modRoles` (array)
*
* Backward compat: also checks singular `adminRoleId` / `moderatorRoleId` fields
* so old configs continue to work without migration.
*
* @param {import('discord.js').Message} message
* @param {Object} config - Merged guild config
* @returns {boolean}
Expand All @@ -27,11 +30,17 @@ export function isExempt(message, config) {
// ADMINISTRATOR permission bypasses everything
if (member.permissions.has(PermissionFlagsBits.Administrator)) return true;

// Singular role IDs — the actual config schema (permissions.adminRoleId / moderatorRoleId)
const adminRoleId = config.permissions?.adminRoleId;
const moderatorRoleId = config.permissions?.moderatorRoleId;
if (adminRoleId && member.roles.cache.has(adminRoleId)) return true;
if (moderatorRoleId && member.roles.cache.has(moderatorRoleId)) return true;
// Array role IDs — new schema (permissions.adminRoleIds / moderatorRoleIds)
// Backward compat: fall back to singular field if array field is absent
const adminRoleIds =
config.permissions?.adminRoleIds ??
(config.permissions?.adminRoleId ? [config.permissions.adminRoleId] : []);
if (adminRoleIds.some((id) => member.roles.cache.has(id))) return true;

const moderatorRoleIds =
config.permissions?.moderatorRoleIds ??
(config.permissions?.moderatorRoleId ? [config.permissions.moderatorRoleId] : []);
if (moderatorRoleIds.some((id) => member.roles.cache.has(id))) return true;

// Legacy / test-facing array of role IDs or names (permissions.modRoles)
const modRoles = config.permissions?.modRoles ?? [];
Expand Down
48 changes: 30 additions & 18 deletions src/utils/permissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ export function isBotOwner(member, config) {
}

/**
* Check if a member is an admin
* Determine whether a guild member has administrative privileges.
*
* @param {GuildMember} member - Discord guild member
* @param {Object} config - Bot configuration
* @returns {boolean} True if member is admin
* @param {GuildMember} member - The guild member to check.
* @param {Object} config - Bot configuration containing permission role IDs.
* @returns {boolean} `true` if the member is an admin, `false` otherwise.
*/
export function isAdmin(member, config) {
if (!member) return false;
Expand All @@ -62,9 +62,13 @@ export function isAdmin(member, config) {
return true;
}

// Check if member has the configured admin role
if (config.permissions?.adminRoleId) {
return member.roles.cache.has(config.permissions.adminRoleId);
// Check if member has any of the configured admin roles
// Backward compat: support old singular adminRoleId field
const adminRoleIds =
config.permissions?.adminRoleIds ??
(config.permissions?.adminRoleId ? [config.permissions.adminRoleId] : []);
if (adminRoleIds.length > 0) {
return adminRoleIds.some((id) => member.roles.cache.has(id));
}

return false;
Expand Down Expand Up @@ -134,11 +138,12 @@ export function isGuildAdmin(member, config) {
}

/**
* Check if a member is a moderator (has MANAGE_GUILD permission or bot admin role)
* Determine whether a guild member is considered a moderator.
*
* @param {GuildMember} member - Discord guild member
* @param {Object} config - Bot configuration
* @returns {boolean} True if member is a moderator
* Considers bot owners, members with the Administrator or Manage Guild permission, and members with any configured admin or moderator role IDs (supports legacy singular role ID fields).
* @param {GuildMember} member - Discord guild member to check.
* @param {Object} config - Bot configuration containing permission role settings (e.g., permissions.adminRoleIds, permissions.moderatorRoleIds or legacy adminRoleId/moderatorRoleId).
* @returns {boolean} `true` if the member is a moderator, `false` otherwise.
*/
export function isModerator(member, config) {
if (!member) return false;
Expand All @@ -158,15 +163,22 @@ export function isModerator(member, config) {
return true;
}

// Check bot admin role from config
if (config.permissions?.adminRoleId) {
if (member.roles.cache.has(config.permissions.adminRoleId)) {
return true;
}
// Check bot admin roles from config
// Backward compat: support old singular adminRoleId field
const adminRoleIds =
config.permissions?.adminRoleIds ??
(config.permissions?.adminRoleId ? [config.permissions.adminRoleId] : []);
if (adminRoleIds.some((id) => member.roles.cache.has(id))) {
return true;
}

if (config.permissions?.moderatorRoleId) {
return member.roles.cache.has(config.permissions.moderatorRoleId);
// Check bot moderator roles from config
// Backward compat: support old singular moderatorRoleId field
const moderatorRoleIds =
config.permissions?.moderatorRoleIds ??
(config.permissions?.moderatorRoleId ? [config.permissions.moderatorRoleId] : []);
if (moderatorRoleIds.some((id) => member.roles.cache.has(id))) {
return true;
}

return false;
Expand Down
40 changes: 32 additions & 8 deletions tests/utils/modExempt.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,30 +48,54 @@ describe('isExempt', () => {
expect(isExempt(msg, {})).toBe(false);
});

it('should return true when member has adminRoleId', () => {
it('should return true when member has a role in adminRoleIds array', () => {
const msg = makeMessage({ isAdmin: false, roleIds: ['admin-role-id'] });
const config = { permissions: { adminRoleId: 'admin-role-id' } };
const config = { permissions: { adminRoleIds: ['admin-role-id'] } };
expect(isExempt(msg, config)).toBe(true);
});

it('should return false when adminRoleId is set but member does not have it', () => {
it('should return true when member has any of multiple adminRoleIds', () => {
const msg = makeMessage({ isAdmin: false, roleIds: ['admin-role-2'] });
const config = { permissions: { adminRoleIds: ['admin-role-1', 'admin-role-2'] } };
expect(isExempt(msg, config)).toBe(true);
});

it('should return false when adminRoleIds is set but member does not have any', () => {
const msg = makeMessage({ isAdmin: false, roleIds: ['other-role'] });
const config = { permissions: { adminRoleId: 'admin-role-id' } };
const config = { permissions: { adminRoleIds: ['admin-role-id'] } };
expect(isExempt(msg, config)).toBe(false);
});

it('should return true when member has moderatorRoleId', () => {
it('should return true when member has a role in moderatorRoleIds array', () => {
const msg = makeMessage({ isAdmin: false, roleIds: ['mod-role-id'] });
const config = { permissions: { moderatorRoleId: 'mod-role-id' } };
const config = { permissions: { moderatorRoleIds: ['mod-role-id'] } };
expect(isExempt(msg, config)).toBe(true);
});

it('should return false when moderatorRoleId is set but member does not have it', () => {
it('should return true when member has any of multiple moderatorRoleIds', () => {
const msg = makeMessage({ isAdmin: false, roleIds: ['mod-role-2'] });
const config = { permissions: { moderatorRoleIds: ['mod-role-1', 'mod-role-2'] } };
expect(isExempt(msg, config)).toBe(true);
});

it('should return false when moderatorRoleIds is set but member does not have any', () => {
const msg = makeMessage({ isAdmin: false, roleIds: [] });
const config = { permissions: { moderatorRoleId: 'mod-role-id' } };
const config = { permissions: { moderatorRoleIds: ['mod-role-id'] } };
expect(isExempt(msg, config)).toBe(false);
});

it('should support backward compat: singular adminRoleId still grants exemption', () => {
const msg = makeMessage({ isAdmin: false, roleIds: ['admin-role-id'] });
const config = { permissions: { adminRoleId: 'admin-role-id' } };
expect(isExempt(msg, config)).toBe(true);
});

it('should support backward compat: singular moderatorRoleId still grants exemption', () => {
const msg = makeMessage({ isAdmin: false, roleIds: ['mod-role-id'] });
const config = { permissions: { moderatorRoleId: 'mod-role-id' } };
expect(isExempt(msg, config)).toBe(true);
});

it('should return true when member has a role ID in modRoles array', () => {
const msg = makeMessage({ isAdmin: false, roleIds: ['custom-mod'] });
const config = { permissions: { modRoles: ['custom-mod'] } };
Expand Down
Loading
Loading