audit week 3 [SEC-004 + SEC-005]: scope members.fetch + redact PII in debug logs
[SEC-004] services/staffThread.js — addRoleMembersToThread previously called
the unscoped guild.members.fetch() on every ticket creation, chunking every
member of the guild. With STAFF_THREAD_AUTO_ADD_ROLE on and a 50-member
staff role, the 300ms-per-add loop also blocked ticket creation for ~15s.
- Read role.members directly (computed from guild.members.cache, kept in
sync via the GuildMembers gateway intent set on the client). Skip the
explicit unscoped fetch in the hot path.
- Cache-cold fallback: one scoped guild.members.fetch({ withPresences:
false }) — irrelevant presence sync stays off the wire.
- createStaffThread no longer awaits the add-loop. Wraps the call in
setImmediate(...) so ticket creation returns immediately while the
rate-limited add-loop runs in the background.
[SEC-005] services/debugLog.js — stacks/messages posted to the debug
channel could leak customer email addresses (interpolated through ticket
errors) and Discord member/channel IDs. Add a redactPII helper applied to
both logError's message + stack and logWarn's body:
- Email regex /[\w.+-]+@[\w.-]+\.\w+/g → [EMAIL_REDACTED]
- Discord snowflake /\b\d{18,20}\b/g → [ID_REDACTED]
interaction.user.tag in the User: line is intentionally not redacted —
it's needed for triage and is not PII (Discord usernames are public).
This commit is contained in:
@@ -11,6 +11,24 @@ function setClient(c) {
|
|||||||
client = c;
|
client = c;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --- PII redaction ---
|
||||||
|
|
||||||
|
// Email addresses (loose regex — covers most RFC 5321 local parts that show up
|
||||||
|
// in support traffic) and Discord snowflakes (18–20 digit numeric IDs) get
|
||||||
|
// redacted before stack/message text reaches the debug channel. Both can land
|
||||||
|
// in error stacks via senderEmail interpolation, channel IDs in error
|
||||||
|
// messages, etc. — redacting at the boundary keeps the debug channel useful
|
||||||
|
// for triage without leaking customer addresses or staff member IDs.
|
||||||
|
const EMAIL_REDACT_RE = /[\w.+-]+@[\w.-]+\.\w+/g;
|
||||||
|
const SNOWFLAKE_REDACT_RE = /\b\d{18,20}\b/g;
|
||||||
|
|
||||||
|
function redactPII(s) {
|
||||||
|
if (s == null) return '';
|
||||||
|
return String(s)
|
||||||
|
.replace(EMAIL_REDACT_RE, '[EMAIL_REDACTED]')
|
||||||
|
.replace(SNOWFLAKE_REDACT_RE, '[ID_REDACTED]');
|
||||||
|
}
|
||||||
|
|
||||||
// --- Helpers ---
|
// --- Helpers ---
|
||||||
|
|
||||||
async function sendToChannel(channelId, embed, overrideClient) {
|
async function sendToChannel(channelId, embed, overrideClient) {
|
||||||
@@ -38,9 +56,10 @@ async function logError(context, error, interaction = null, overrideClient = nul
|
|||||||
const commandLine = (interaction?.commandName || interaction?.customId)
|
const commandLine = (interaction?.commandName || interaction?.customId)
|
||||||
? `Command/Button: ${interaction.commandName || interaction.customId}\n`
|
? `Command/Button: ${interaction.commandName || interaction.customId}\n`
|
||||||
: '';
|
: '';
|
||||||
const stack = (error.stack || error.message || String(error)).slice(0, 1500);
|
const message = redactPII(error.message || String(error));
|
||||||
|
const stack = redactPII(error.stack || error.message || String(error)).slice(0, 1500);
|
||||||
await channel.send({
|
await channel.send({
|
||||||
content: `\`[${context}]\` ${error.message || String(error)}\n${userLine}${commandLine}\n\`\`\`${stack}\`\`\``
|
content: `\`[${context}]\` ${message}\n${userLine}${commandLine}\n\`\`\`${stack}\`\`\``
|
||||||
});
|
});
|
||||||
} catch (_) {
|
} catch (_) {
|
||||||
// ignore send failures
|
// ignore send failures
|
||||||
@@ -52,7 +71,7 @@ async function logError(context, error, interaction = null, overrideClient = nul
|
|||||||
async function logWarn(context, message, overrideClient = null) {
|
async function logWarn(context, message, overrideClient = null) {
|
||||||
const embed = new EmbedBuilder()
|
const embed = new EmbedBuilder()
|
||||||
.setTitle(`Warning: ${context}`)
|
.setTitle(`Warning: ${context}`)
|
||||||
.setDescription(String(message).slice(0, 4000))
|
.setDescription(redactPII(String(message)).slice(0, 4000))
|
||||||
.setColor(0xFFFF00)
|
.setColor(0xFFFF00)
|
||||||
.setTimestamp();
|
.setTimestamp();
|
||||||
await sendToChannel(CONFIG.DEBUGGING_CHANNEL_ID, embed, overrideClient);
|
await sendToChannel(CONFIG.DEBUGGING_CHANNEL_ID, embed, overrideClient);
|
||||||
|
|||||||
@@ -11,9 +11,10 @@
|
|||||||
* is logged via logWarn.
|
* is logged via logWarn.
|
||||||
* - invitable: false means only staff with MANAGE_THREADS can add additional
|
* - invitable: false means only staff with MANAGE_THREADS can add additional
|
||||||
* members — this is intentional for privacy.
|
* members — this is intentional for privacy.
|
||||||
* - guild.members.fetch() in addRoleMembersToThread can be slow on large
|
* - addRoleMembersToThread reads from role.members (cache-derived) and only
|
||||||
* servers. The 300ms delay between adds avoids the thread member add rate
|
* falls back to a scoped guild.members.fetch on cache miss. The 300ms
|
||||||
* limit (approximately 5/second).
|
* delay between adds avoids the thread member add rate limit (~5/sec).
|
||||||
|
* It runs via setImmediate so it doesn't block ticket creation.
|
||||||
*/
|
*/
|
||||||
const { ChannelType } = require('discord.js');
|
const { ChannelType } = require('discord.js');
|
||||||
const { CONFIG } = require('../config');
|
const { CONFIG } = require('../config');
|
||||||
@@ -39,7 +40,11 @@ async function createStaffThread(channel, client) {
|
|||||||
});
|
});
|
||||||
|
|
||||||
if (CONFIG.STAFF_THREAD_AUTO_ADD_ROLE && CONFIG.STAFF_THREAD_ROLE_ID) {
|
if (CONFIG.STAFF_THREAD_AUTO_ADD_ROLE && CONFIG.STAFF_THREAD_ROLE_ID) {
|
||||||
await addRoleMembersToThread(thread, channel.guild, client);
|
// Run off the critical path — the add loop is rate-limited at 300ms per
|
||||||
|
// member and would block ticket creation for ~15s on a 50-member role.
|
||||||
|
setImmediate(() => {
|
||||||
|
addRoleMembersToThread(thread, channel.guild, client).catch(() => {});
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
return thread;
|
return thread;
|
||||||
@@ -55,16 +60,26 @@ async function createStaffThread(channel, client) {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Add all members of the staff role to the thread.
|
* Add all members of the staff role to the thread.
|
||||||
|
*
|
||||||
|
* Prefers role.members (computed from guild.members.cache, kept in sync via
|
||||||
|
* the GuildMembers gateway intent — see broccolini-discord.js intents). Only
|
||||||
|
* falls back to a scoped guild.members.fetch on cache miss (e.g. cold cache
|
||||||
|
* just after restart). Previously called the unscoped guild.members.fetch()
|
||||||
|
* on every ticket creation, which chunked all members of the guild — wasted
|
||||||
|
* gateway/REST budget and added ~15s to ticket creation on busy guilds.
|
||||||
*/
|
*/
|
||||||
async function addRoleMembersToThread(thread, guild, client) {
|
async function addRoleMembersToThread(thread, guild, client) {
|
||||||
try {
|
try {
|
||||||
const role = await guild.roles.fetch(CONFIG.STAFF_THREAD_ROLE_ID).catch(() => null);
|
const role = await guild.roles.fetch(CONFIG.STAFF_THREAD_ROLE_ID).catch(() => null);
|
||||||
if (!role) return;
|
if (!role) return;
|
||||||
|
|
||||||
await guild.members.fetch();
|
let members = role.members.filter(m => !m.user.bot);
|
||||||
const members = guild.members.cache.filter(m =>
|
if (members.size === 0) {
|
||||||
m.roles.cache.has(CONFIG.STAFF_THREAD_ROLE_ID) && !m.user.bot
|
// Cache cold (first ticket after restart). withPresences: false skips
|
||||||
);
|
// the presence sync, which is irrelevant for thread-add and expensive.
|
||||||
|
await guild.members.fetch({ withPresences: false }).catch(() => {});
|
||||||
|
members = role.members.filter(m => !m.user.bot);
|
||||||
|
}
|
||||||
|
|
||||||
for (const [, member] of members) {
|
for (const [, member] of members) {
|
||||||
await thread.members.add(member.id).catch(() => {});
|
await thread.members.add(member.id).catch(() => {});
|
||||||
|
|||||||
Reference in New Issue
Block a user