diff --git a/scripts/test-config.ts b/scripts/test-config.ts index 5efc114f..15307b14 100644 --- a/scripts/test-config.ts +++ b/scripts/test-config.ts @@ -35,6 +35,12 @@ import { type DiscordChannelConfig, type DiscordNewChannelConfig, } from '../src/config/channels'; +import { + DISCORD_ROLE_PERMISSION_BITS, + discordPermissionBitfield, + getDiscordRolePermissionErrors, + mergeDiscordRolePermissions, +} from '../src/config/discordPermissions'; let passed = 0; let failed = 0; @@ -672,6 +678,42 @@ test('Tag drift ignores Discord tag IDs and ordering but catches changed tags', ); }); +// Discord role permissions +console.log('\nTesting Discord role permissions...\n'); +const EMOJI = (1n << 18n).toString(); // USE_EXTERNAL_EMOJIS +const HAND_SET = ((1n << 10n) | (1n << 11n)).toString(); // VIEW_CHANNEL | SEND_MESSAGES +test('USE_EXTERNAL_EMOJIS is bit 18 (262144)', () => + discordPermissionBitfield(['USE_EXTERNAL_EMOJIS']) === '262144' && EMOJI === '262144'); +test('No permissions bitfield is 0', () => discordPermissionBitfield([]) === '0'); +test('Granting keeps permissions set by hand on the role', () => + mergeDiscordRolePermissions(HAND_SET, undefined, EMOJI) === + (BigInt(HAND_SET) | BigInt(EMOJI)).toString()); +test('Re-applying the same permissions is a no-op', () => { + const live = (BigInt(HAND_SET) | BigInt(EMOJI)).toString(); + return mergeDiscordRolePermissions(live, EMOJI, EMOJI) === live; +}); +test('Removing a permission from config clears only that bit', () => { + const live = (BigInt(HAND_SET) | BigInt(EMOJI)).toString(); + return mergeDiscordRolePermissions(live, EMOJI, undefined) === HAND_SET; +}); +test('Permission allowlist has no administrative permissions', () => { + const dangerous = [0, 1, 2, 3, 4, 5, 13, 17, 22, 23, 24, 27, 28, 29, 30, 34, 40].map( + (bit) => 1n << BigInt(bit) + ); // CREATE_INVITE..ADMINISTRATOR, MANAGE_*, MENTION_EVERYONE, MUTE/DEAFEN/MOVE, timeouts + return Object.values(DISCORD_ROLE_PERMISSION_BITS).every((bits) => !dangerous.includes(bits)); +}); +test('Unknown and duplicate role permissions are rejected', () => + getDiscordRolePermissionErrors('x', ['ADMINISTRATOR']).length === 1 && + getDiscordRolePermissionErrors('x', ['USE_EXTERNAL_EMOJIS', 'USE_EXTERNAL_EMOJIS']).length === + 1 && + getDiscordRolePermissionErrors('x', ['USE_EXTERNAL_EMOJIS']).length === 0); +test('Configured Discord role permissions are all valid', () => + ROLES.every( + (r) => + !r.discord?.permissions || + getDiscordRolePermissionErrors(r.discord.role, r.discord.permissions).length === 0 + )); + // Summary console.log(`\n${passed} passed, ${failed} failed`); process.exit(failed > 0 ? 1 : 0); diff --git a/scripts/validate-config.ts b/scripts/validate-config.ts index c818dacb..e75ccad6 100644 --- a/scripts/validate-config.ts +++ b/scripts/validate-config.ts @@ -16,6 +16,7 @@ import { import { MEMBERS } from '../src/config/users'; import { ACCESS_POLICIES } from '../src/config/accessPolicies'; import { DISCORD_CHANNELS, validateDiscordChannels } from '../src/config/channels'; +import { getDiscordRolePermissionErrors } from '../src/config/discordPermissions'; import { hasProvisionUserRole, resolveGoogleMemberEmail } from '../src/config/utils'; import type { RoleId } from '../src/config/roleIds'; @@ -414,6 +415,17 @@ console.log('Validating Discord channel entries in channels.ts...'); } } +// Validate Discord role permissions in roles.ts +// Only allowlisted (low-risk) permissions may be granted through config. +console.log('Validating Discord role permissions in roles.ts...'); +for (const role of ROLES) { + if (!role.discord?.permissions) continue; + for (const error of getDiscordRolePermissionErrors(role.discord.role, role.discord.permissions)) { + console.error(`ERROR: ${error}`); + hasErrors = true; + } +} + // Validate parent role references in roles.ts console.log('Validating parent role references in roles.ts...'); for (const role of ROLES) { diff --git a/src/config/discordPermissions.ts b/src/config/discordPermissions.ts new file mode 100644 index 00000000..250cc105 --- /dev/null +++ b/src/config/discordPermissions.ts @@ -0,0 +1,71 @@ +/** + * Discord role permissions that may be granted through roles.ts. + * + * Deliberately an allowlist of low-risk, non-moderation permissions: anything that + * could administer the server (ADMINISTRATOR, MANAGE_*, KICK/BAN, MENTION_EVERYONE...) + * stays a by-hand decision in Discord. Extend this list in a reviewed PR when needed. + * + * Bit values: https://discord.com/developers/docs/topics/permissions#permissions-bitwise-permission-flags + */ +export const DISCORD_ROLE_PERMISSION_BITS = { + ADD_REACTIONS: 1n << 6n, + EMBED_LINKS: 1n << 14n, + ATTACH_FILES: 1n << 15n, + USE_EXTERNAL_EMOJIS: 1n << 18n, + USE_EXTERNAL_STICKERS: 1n << 37n, +} as const; + +export type DiscordRolePermission = keyof typeof DISCORD_ROLE_PERMISSION_BITS; + +export function isDiscordRolePermission(value: string): value is DiscordRolePermission { + return Object.prototype.hasOwnProperty.call(DISCORD_ROLE_PERMISSION_BITS, value); +} + +/** Bitfield for a list of permission names, as the decimal string the Discord API uses. */ +export function discordPermissionBitfield(permissions: readonly DiscordRolePermission[]): string { + return permissions + .reduce((bits, name) => bits | DISCORD_ROLE_PERMISSION_BITS[name], 0n) + .toString(); +} + +/** + * The permissions to write to a role so that Pulumi owns only the bits it declared. + * + * - Bits Pulumi did not manage before and does not manage now are left exactly as they + * are live (permissions set by hand in Discord survive). + * - Bits declared now are set. + * - Bits Pulumi declared previously but no longer declares are cleared. + * + * All values are Discord's decimal-string bitfields. + */ +export function mergeDiscordRolePermissions( + live: string, + previouslyManaged: string | undefined, + managed: string | undefined +): string { + const previous = BigInt(previouslyManaged ?? '0'); + const next = BigInt(managed ?? '0'); + return ((BigInt(live) & ~previous) | next).toString(); +} + +/** Errors for permission names that are duplicated or not on the allowlist. */ +export function getDiscordRolePermissionErrors( + roleName: string, + permissions: readonly string[] +): string[] { + const errors: string[] = []; + const seen = new Set(); + for (const permission of permissions) { + if (!isDiscordRolePermission(permission)) { + errors.push( + `Discord role "${roleName}": permission "${permission}" is not allowed ` + + `(allowed: ${Object.keys(DISCORD_ROLE_PERMISSION_BITS).join(', ')})` + ); + } + if (seen.has(permission)) { + errors.push(`Discord role "${roleName}": permission "${permission}" is listed twice`); + } + seen.add(permission); + } + return errors; +} diff --git a/src/config/roles.ts b/src/config/roles.ts index 1d5f7e69..b06c32d4 100644 --- a/src/config/roles.ts +++ b/src/config/roles.ts @@ -1,4 +1,5 @@ import { ROLE_IDS, type RoleId } from './roleIds'; +import type { DiscordRolePermission } from './discordPermissions'; /** * GitHub team configuration @@ -16,6 +17,13 @@ export interface GitHubConfig { export interface DiscordConfig { /** Display name in Discord (can have spaces) */ role: string; + /** + * Permissions Pulumi grants on this role (allowlist in discordPermissions.ts). + * Omit to leave the role's permissions entirely untouched. Pulumi only owns the + * bits listed here: other permissions set by hand in Discord are preserved, and + * removing an entry later revokes just that permission. + */ + permissions?: readonly DiscordRolePermission[]; } /** @@ -151,7 +159,7 @@ export const ROLES: readonly Role[] = [ description: 'General maintainers', // No parent team: must not inherit steering-committee repo permissions github: { team: 'maintainers' }, - discord: { role: 'maintainers (synced)' }, + discord: { role: 'maintainers (synced)', permissions: ['USE_EXTERNAL_EMOJIS'] }, // GWS user accounts are opt-in: maintainers add firstName/lastName/googleEmailPrefix // to their entry in users.ts via PR to get an @modelcontextprotocol.io account google: { group: 'maintainers', allowExternalMembers: true }, diff --git a/src/discord.ts b/src/discord.ts index 8eccc9c5..a3903d88 100644 --- a/src/discord.ts +++ b/src/discord.ts @@ -16,6 +16,10 @@ import { type DiscordChannelConfig, type DiscordChannelState, } from './config/channels'; +import { + discordPermissionBitfield, + mergeDiscordRolePermissions, +} from './config/discordPermissions'; const config = new pulumi.Config('discord'); // Discord integration is optional - only enabled if botToken and guildId are configured @@ -141,6 +145,11 @@ interface DiscordRoleInputs { guildId: string; roleName: string; token: string; + /** + * Bitfield (decimal string) of the permissions Pulumi owns on this role. Absent for + * roles that declare no permissions: their permissions are never read or written. + */ + permissions?: string; } interface DiscordRoleOutputs extends DiscordRoleInputs { @@ -158,7 +167,8 @@ const discordRoleProvider: pulumi.dynamic.ResourceProvider = { method: 'POST', body: JSON.stringify({ name: inputs.roleName, - permissions: '0', // No special permissions - roles are for organization only + // Only the declared permissions; '0' (organization only) when none are declared + permissions: inputs.permissions ?? '0', mentionable: false, hoist: false, }), @@ -205,14 +215,33 @@ const discordRoleProvider: pulumi.dynamic.ResourceProvider = { async update( id: string, - _olds: DiscordRoleOutputs, + olds: DiscordRoleOutputs, news: DiscordRoleInputs ): Promise> { + const body: { name: string; permissions?: string } = { name: news.roleName }; + + // Touch permissions only if this role declares them now or did before; otherwise the + // PATCH stays name-only, exactly as for every role prior to permission support. + if (news.permissions !== undefined || olds.permissions !== undefined) { + const liveRoles = await discordFetch( + news.token, + `/guilds/${news.guildId}/roles` + ); + const live = liveRoles.find((r) => r.id === id); + if (!live) { + throw new Error(`Role ${id} not found; refusing to write its permissions`); + } + // Keep every bit Pulumi does not own; set/clear only the declared ones + body.permissions = mergeDiscordRolePermissions( + live.permissions, + olds.permissions, + news.permissions + ); + } + await discordFetch(news.token, `/guilds/${news.guildId}/roles/${id}`, { method: 'PATCH', - body: JSON.stringify({ - name: news.roleName, - }), + body: JSON.stringify(body), }); return { @@ -246,6 +275,7 @@ class DiscordRole extends pulumi.dynamic.Resource { guildId: pulumi.Input; roleName: pulumi.Input; token: pulumi.Input; + permissions?: pulumi.Input; }, opts?: pulumi.CustomResourceOptions ) { @@ -788,6 +818,11 @@ if (DISCORD_ENABLED) { guildId, roleName: role.discord.role, token: botToken, + // Omitted entirely (not undefined-valued) for roles without permissions, so their + // resource inputs are byte-for-byte what they were before this field existed + ...(role.discord.permissions && { + permissions: discordPermissionBitfield(role.discord.permissions), + }), }); });