fix(api): only require permissions a channel overwrite grants (#2867)

This commit is contained in:
Hampus
2026-09-20 17:56:22 +02:00
committed by GitHub
parent 416af4bec4
commit eedfd9275f
4 changed files with 112 additions and 22 deletions
@@ -24,6 +24,7 @@ import {deleteChannelMessageSearchDocuments} from '@app/api/search/MessageSearch
import type {IUserRepository} from '@app/api/user/IUserRepository';
import {serializeChannelForAudit} from '@app/api/utils/AuditSerializationUtils';
import {applyProtectedOverwriteBits} from '@app/api/utils/featureUtils';
import {overwriteGrantedBits} from '@app/api/utils/PermissionUtils';
import type {VoiceAvailabilityService} from '@app/api/voice/VoiceAvailabilityService';
import type {VoiceRegionAvailability} from '@app/api/voice/VoiceModel';
import type {IWebhookRepository} from '@app/api/webhook/IWebhookRepository';
@@ -208,25 +209,6 @@ export class ChannelOperationsService {
userId,
channelId: channel.id,
});
if (!isOwner) {
for (const overwrite of data.permission_overwrites ?? []) {
const allowPerms = (overwrite.allow ? BigInt(overwrite.allow) : 0n) & ALL_PERMISSIONS;
if ((allowPerms & ~channelPermissions) !== 0n) {
throw new MissingPermissionsError();
}
}
const nextDeny = new Map<RoleID | UserID, bigint>();
for (const overwrite of data.permission_overwrites ?? []) {
const targetKey = overwrite.type === 0 ? createRoleID(overwrite.id) : createUserID(overwrite.id);
nextDeny.set(targetKey, (overwrite.deny ? BigInt(overwrite.deny) : 0n) & ALL_PERMISSIONS);
}
for (const [targetId, existing] of previousPermissionOverwrites ?? []) {
const removedDeny = existing.deny & ~(nextDeny.get(targetId) ?? 0n);
if ((removedDeny & ~channelPermissions) !== 0n) {
throw new MissingPermissionsError();
}
}
}
permissionOverwrites = new Map();
for (const overwrite of data.permission_overwrites ?? []) {
const targetId = overwrite.type === 0 ? createRoleID(overwrite.id) : createUserID(overwrite.id);
@@ -251,6 +233,18 @@ export class ChannelOperationsService {
}),
);
}
if (!isOwner) {
const targetIds = new Set([...(previousPermissionOverwrites?.keys() ?? []), ...permissionOverwrites.keys()]);
for (const targetId of targetIds) {
const grantedBits = overwriteGrantedBits(
previousPermissionOverwrites?.get(targetId),
permissionOverwrites.get(targetId),
);
if ((grantedBits & ~channelPermissions) !== 0n) {
throw new MissingPermissionsError();
}
}
}
}
const requestedParentId =
data.parent_id !== undefined ? (data.parent_id ? createChannelID(data.parent_id) : null) : channel.parentId;
@@ -646,9 +640,8 @@ export class ChannelOperationsService {
const sanitizedAllow = protectedBits.allow;
const sanitizedDeny = protectedBits.deny;
const hasAdministrator = (userPermissions & Permissions.ADMINISTRATOR) !== 0n;
if (!hasAdministrator && (sanitizedAllow & ~userPermissions) !== 0n) throw new MissingPermissionsError();
const removedDeny = (existing?.deny ?? 0n) & ~sanitizedDeny;
if (!hasAdministrator && (removedDeny & ~userPermissions) !== 0n) throw new MissingPermissionsError();
const grantedBits = overwriteGrantedBits(existing, {allow: sanitizedAllow, deny: sanitizedDeny});
if (!hasAdministrator && (grantedBits & ~userPermissions) !== 0n) throw new MissingPermissionsError();
const previousPermissionOverwrites = channel.permissionOverwrites;
const nextOverwrite = new ChannelPermissionOverwrite({
type: params.overwrite.type,
@@ -294,6 +294,48 @@ describe('Channel Permission Overwrites', () => {
expect(overwrite?.allow).toBe(Permissions.VIEW_CHANNEL.toString());
expect(overwrite?.deny).toBe(Permissions.MANAGE_MESSAGES.toString());
});
test('should let an editor change an overwrite that already allows a permission they lack', async () => {
const {owner, members, guild, systemChannel} = await setupTestGuildWithMembers(harness, 1);
const manager = members[0];
const managerRole = await createRole(harness, owner.token, guild.id, {
name: 'Queue Manager',
permissions: Permissions.MANAGE_ROLES.toString(),
});
const botRole = await createRole(harness, owner.token, guild.id, {name: 'Bot'});
await addMemberRole(harness, owner.token, guild.id, manager.userId, managerRole.id);
await createPermissionOverwrite(harness, owner.token, systemChannel.id, botRole.id, {
type: 0,
allow: Permissions.PIN_MESSAGES.toString(),
deny: '0',
});
await createBuilder(harness, manager.token)
.put(`/channels/${systemChannel.id}/permissions/${botRole.id}`)
.body({
type: 0,
allow: (Permissions.PIN_MESSAGES | Permissions.SEND_MESSAGES).toString(),
deny: '0',
})
.expect(HTTP_STATUS.NO_CONTENT)
.execute();
const updated = await getChannel(harness, owner.token, systemChannel.id);
const botOverwrite = updated.permission_overwrites?.find((o) => o.id === botRole.id);
expect(botOverwrite?.allow).toBe((Permissions.PIN_MESSAGES | Permissions.SEND_MESSAGES).toString());
});
test('should reject an editor granting a permission they lack', async () => {
const {owner, members, guild, systemChannel} = await setupTestGuildWithMembers(harness, 1);
const manager = members[0];
const managerRole = await createRole(harness, owner.token, guild.id, {
name: 'Queue Manager',
permissions: Permissions.MANAGE_ROLES.toString(),
});
const botRole = await createRole(harness, owner.token, guild.id, {name: 'Bot'});
await addMemberRole(harness, owner.token, guild.id, manager.userId, managerRole.id);
await createBuilder(harness, manager.token)
.put(`/channels/${systemChannel.id}/permissions/${botRole.id}`)
.body({type: 0, allow: Permissions.PIN_MESSAGES.toString(), deny: '0'})
.expect(HTTP_STATUS.FORBIDDEN)
.execute();
});
test('should propagate category permission patches only to children that were synced when the category changed', async () => {
const {owner, guild} = await setupTestGuildWithMembers(harness, 0);
const targetRole = await createRole(harness, owner.token, guild.id, {name: 'Readers'});
@@ -0,0 +1,44 @@
// SPDX-License-Identifier: AGPL-3.0-or-later
import {overwriteGrantedBits} from '@app/api/utils/PermissionUtils';
import {Permissions} from '@fluxer/constants/src/ChannelConstants';
import {describe, expect, it} from 'vitest';
const BOT_OVERWRITE = {
allow: Permissions.ADD_REACTIONS | Permissions.SEND_MESSAGES | Permissions.MANAGE_MESSAGES | Permissions.PIN_MESSAGES,
deny: 0n,
};
describe('overwriteGrantedBits', () => {
it('grants nothing when an overwrite is resubmitted unmodified', () => {
expect(overwriteGrantedBits(BOT_OVERWRITE, {...BOT_OVERWRITE})).toBe(0n);
});
it('ignores allow bits that were already set when another bit is flipped', () => {
const before = {allow: Permissions.PIN_MESSAGES, deny: 0n};
const after = {allow: Permissions.PIN_MESSAGES | Permissions.SEND_MESSAGES, deny: 0n};
expect(overwriteGrantedBits(before, after)).toBe(Permissions.SEND_MESSAGES);
});
it('grants nothing when an allow bit is withdrawn', () => {
const before = {allow: Permissions.SEND_MESSAGES | Permissions.MANAGE_MESSAGES, deny: 0n};
const after = {allow: Permissions.SEND_MESSAGES, deny: 0n};
expect(overwriteGrantedBits(before, after)).toBe(0n);
});
it('grants nothing when a deny is added for a permission the editor lacks', () => {
const after = {allow: Permissions.VIEW_CHANNEL, deny: Permissions.MANAGE_MESSAGES};
expect(overwriteGrantedBits(null, after)).toBe(Permissions.VIEW_CHANNEL);
});
it('grants the bits lifted out of deny', () => {
const before = {allow: 0n, deny: Permissions.ADD_REACTIONS | Permissions.SEND_MESSAGES};
const after = {allow: 0n, deny: Permissions.ADD_REACTIONS};
expect(overwriteGrantedBits(before, after)).toBe(Permissions.SEND_MESSAGES);
});
it('treats a removed overwrite as granting everything it denied', () => {
const before = {allow: Permissions.SEND_MESSAGES, deny: Permissions.ADD_REACTIONS};
expect(overwriteGrantedBits(before, undefined)).toBe(Permissions.ADD_REACTIONS);
});
});
@@ -51,3 +51,14 @@ export async function hasPermission(
): Promise<boolean> {
return await gatewayService.checkPermission(params);
}
export function overwriteGrantedBits(
before: {allow: bigint; deny: bigint} | null | undefined,
after: {allow: bigint; deny: bigint} | null | undefined,
): bigint {
const beforeAllow = before?.allow ?? 0n;
const beforeDeny = before?.deny ?? 0n;
const afterAllow = after?.allow ?? 0n;
const afterDeny = after?.deny ?? 0n;
return (afterAllow & ~beforeAllow) | (beforeDeny & ~afterDeny);
}