Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
42 changes: 42 additions & 0 deletions scripts/test-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
12 changes: 12 additions & 0 deletions scripts/validate-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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) {
Expand Down
71 changes: 71 additions & 0 deletions src/config/discordPermissions.ts
Original file line number Diff line number Diff line change
@@ -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<string>();
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;
}
10 changes: 9 additions & 1 deletion src/config/roles.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ROLE_IDS, type RoleId } from './roleIds';
import type { DiscordRolePermission } from './discordPermissions';

/**
* GitHub team configuration
Expand All @@ -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[];
}

/**
Expand Down Expand Up @@ -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 },
Expand Down
45 changes: 40 additions & 5 deletions src/discord.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
}),
Expand Down Expand Up @@ -205,14 +215,33 @@ const discordRoleProvider: pulumi.dynamic.ResourceProvider = {

async update(
id: string,
_olds: DiscordRoleOutputs,
olds: DiscordRoleOutputs,
news: DiscordRoleInputs
): Promise<pulumi.dynamic.UpdateResult<DiscordRoleOutputs>> {
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<DiscordRoleApiResponse[]>(
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<DiscordRoleApiResponse>(news.token, `/guilds/${news.guildId}/roles/${id}`, {
method: 'PATCH',
body: JSON.stringify({
name: news.roleName,
}),
body: JSON.stringify(body),
});

return {
Expand Down Expand Up @@ -246,6 +275,7 @@ class DiscordRole extends pulumi.dynamic.Resource {
guildId: pulumi.Input<string>;
roleName: pulumi.Input<string>;
token: pulumi.Input<string>;
permissions?: pulumi.Input<string>;
},
opts?: pulumi.CustomResourceOptions
) {
Expand Down Expand Up @@ -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),
}),
});
});

Expand Down
Loading