pepepper/fluxer
1
0
Fork 0
mirror of https://github.com/fluxerapp/fluxer synced 2026-10-07 19:22:14 +09:00
Code Issues Packages Projects Releases Wiki Activity

fix(webhook): stop gating webhook file uploads on creator perms (#2576)

Browse Source
This commit is contained in:
Hampus
2026-09-08 14:25:59 +02:00
committed by GitHub
parent 8a65832a65
commit c506d6d5e3
7 changed files with 123 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
fluxer_api/src/api/channel/services/AttachmentUploadService.ts
+60 -13
View File
@@ -11,6 +11,7 @@ import {
} from '@fluxer/constants/src/LimitConstants';
import {ValidationErrorCodes} from '@fluxer/constants/src/ValidationErrorCodes';
import {CannotSendMessageToNonTextChannelError} from '@fluxer/errors/src/domains/channel/CannotSendMessageToNonTextChannelError';
import {UnknownChannelError} from '@fluxer/errors/src/domains/channel/UnknownChannelError';
import {UnknownMessageError} from '@fluxer/errors/src/domains/channel/UnknownMessageError';
import {FeatureTemporarilyDisabledError} from '@fluxer/errors/src/domains/core/FeatureTemporarilyDisabledError';
import {FileSizeTooLargeError} from '@fluxer/errors/src/domains/core/FileSizeTooLargeError';
@@ -18,6 +19,7 @@ import {InputValidationError} from '@fluxer/errors/src/domains/core/InputValidat
import {MissingPermissionsError} from '@fluxer/errors/src/domains/core/MissingPermissionsError';
import {UnknownUserError} from '@fluxer/errors/src/domains/user/UnknownUserError';
import {ServiceUnavailableError} from '@fluxer/errors/src/HttpErrors';
import type {GuildResponse} from '@fluxer/schema/src/domains/guild/GuildResponseSchemas';
import type {
CompleteMultipartAttachmentUploadItem,
CompleteMultipartAttachmentUploadResult,
@@ -26,7 +28,9 @@ import type {
} from '@fluxer/schema/src/domains/message/AttachmentUploadSchemas';
import type {AttachmentID, ChannelID, MessageID, UserID} from '../../BrandedTypes';
import {Config} from '../../Config';
import {SYSTEM_USER_ID} from '../../constants/Core';
import type {IPurgeQueue} from '../../infrastructure/BunnyPurgeQueue';
import type {IGatewayService} from '../../infrastructure/IGatewayService';
import type {IStorageService} from '../../infrastructure/IStorageService';
import type {LimitConfigService} from '../../limits/LimitConfigService';
import {resolveLimitSafe} from '../../limits/LimitConfigUtils';
@@ -64,6 +68,8 @@ interface DeleteAttachmentParams {
requestCache: RequestCache;
}
type UploadActor = 'member' | 'webhook';
interface UploadFormDataAttachmentsParams {
userId: UserID;
channelId: ChannelID;
@@ -76,6 +82,7 @@ interface UploadFormDataAttachmentsParams {
id: number;
filename: string;
}>;
actor?: UploadActor;
}
interface RequestPresignedAttachmentUploadUrlsParams {
@@ -104,6 +111,7 @@ export class AttachmentUploadService {
private messageInteractionService: MessageInteractionService,
private messageService: MessageService,
private limitConfigService: LimitConfigService,
private gatewayService: IGatewayService,
) {}
async uploadFormDataAttachments({
@@ -112,8 +120,9 @@ export class AttachmentUploadService {
clientIp,
files,
attachmentMetadata,
actor = 'member',
}: UploadFormDataAttachmentsParams): Promise<Array<UploadedAttachment>> {
const {maxFileSize} = await this.getUploadPermissionAndLimit({userId, channelId});
const {maxFileSize} = await this.getUploadPermissionAndLimit({userId, channelId, actor});
assertAttachmentFileSizesWithinLimit(
files.map(({file}) => file.size),
maxFileSize,
@@ -168,7 +177,7 @@ export class AttachmentUploadService {
if (!Config.presignedAttachmentUploadsEnabled) {
throw new FeatureTemporarilyDisabledError();
}
const {maxFileSize} = await this.getUploadPermissionAndLimit({userId, channelId});
const {maxFileSize} = await this.getUploadPermissionAndLimit({userId, channelId, actor: 'member'});
assertAttachmentFileSizesWithinLimit(
attachments.map(({file_size}) => file_size),
maxFileSize,
@@ -275,7 +284,7 @@ export class AttachmentUploadService {
if (!Config.presignedAttachmentUploadsEnabled) {
throw new FeatureTemporarilyDisabledError();
}
const {maxFileSize} = await this.getUploadPermissionAndLimit({userId, channelId});
const {maxFileSize} = await this.getUploadPermissionAndLimit({userId, channelId, actor: 'member'});
const bucket = Config.s3.buckets.uploads;
return Promise.all(
uploads.map(async ({upload_filename, upload_id}, index) => {
@@ -412,21 +421,24 @@ export class AttachmentUploadService {
}
}
private async getUploadPermissionAndLimit({userId, channelId}: {userId: UserID; channelId: ChannelID}): Promise<{
private async getUploadPermissionAndLimit({
userId,
channelId,
actor,
}: {
userId: UserID;
channelId: ChannelID;
actor: UploadActor;
}): Promise<{
maxFileSize: number;
}> {
const {channel, guild, checkPermission, member} =
await this.messageInteractionService.authService.getChannelAuthenticated({
userId,
channelId,
});
const {channel, guild} =
actor === 'webhook'
? await this.getWebhookUploadChannel(channelId)
: await this.getMemberUploadChannel({userId, channelId});
if (!TEXT_BASED_CHANNEL_TYPES.has(channel.type)) {
throw new CannotSendMessageToNonTextChannelError();
}
if (guild) {
await checkPermission(Permissions.SEND_MESSAGES | Permissions.ATTACH_FILES);
assertGuildMemberCanCommunicate(member);
}
const user = await this.userRepository.findUnique(userId);
if (!user) {
throw new UnknownUserError();
@@ -439,6 +451,41 @@ export class AttachmentUploadService {
const maxFileSize = user.isBot ? Math.min(resolvedMaxFileSize, ATTACHMENT_MAX_SIZE_BOT) : resolvedMaxFileSize;
return {maxFileSize};
}
private async getMemberUploadChannel({userId, channelId}: {userId: UserID; channelId: ChannelID}): Promise<{
channel: Channel;
guild: GuildResponse | null;
}> {
const {channel, guild, checkPermission, member} =
await this.messageInteractionService.authService.getChannelAuthenticated({
userId,
channelId,
});
if (guild) {
await checkPermission(Permissions.SEND_MESSAGES | Permissions.ATTACH_FILES);
assertGuildMemberCanCommunicate(member);
}
return {channel, guild};
}
private async getWebhookUploadChannel(channelId: ChannelID): Promise<{
channel: Channel;
guild: GuildResponse | null;
}> {
const channel = await this.channelRepository.channelData.findUnique(channelId);
if (!channel) {
throw new UnknownChannelError();
}
if (!channel.guildId) {
return {channel, guild: null};
}
const guild = await this.gatewayService.getGuildData({
guildId: channel.guildId,
userId: SYSTEM_USER_ID,
skipMembershipCheck: true,
});
return {channel, guild};
}
}
async function mapWithConcurrency<T, TResult>(
fluxer_api/src/api/channel/services/ChannelService.ts
+1
View File
@@ -162,6 +162,7 @@ export class ChannelService {
this.interactions,
this.messages,
limitConfigService,
gatewayService,
);
this.groupDms = new GroupDmOperationsService(
channelRepository,
fluxer_api/src/api/channel/services/message/MessageRequestParser.ts
+2
View File
@@ -34,6 +34,7 @@ type AttachmentMetadata = ClientAttachmentRequest | ClientUploadedAttachmentRequ
interface ParseMultipartMessageDataOptions {
onPayloadParsed?: (payload: unknown) => void;
actor?: 'member' | 'webhook';
}
export async function parseMultipartMessageData(
@@ -158,6 +159,7 @@ export async function parseMultipartMessageData(
clientIp,
files: filesWithIndices,
attachmentMetadata: inlineNewAttachments,
actor: options?.actor,
});
const uploadedMap = new Map(uploadedAttachments.map((attachment) => [attachment.id, attachment]));
const processedInlineAttachments = inlineNewAttachments.map((clientData) => {
fluxer_api/src/api/channel/services/message/MessageSendService.ts
+17 -9
View File
@@ -9,7 +9,12 @@ import {
SENDABLE_MESSAGE_FLAGS,
} from '@fluxer/constants/src/ChannelConstants';
import {GuildNSFWLevel, GuildOperations} from '@fluxer/constants/src/GuildConstants';
import {RelationshipTypes, SensitiveMediaFilterLevel, UserFlags} from '@fluxer/constants/src/UserConstants';
import {
DELETED_USER_ID,
RelationshipTypes,
SensitiveMediaFilterLevel,
UserFlags,
} from '@fluxer/constants/src/UserConstants';
import {ValidationErrorCodes} from '@fluxer/constants/src/ValidationErrorCodes';
import {UnknownChannelError} from '@fluxer/errors/src/domains/channel/UnknownChannelError';
import {UnknownMessageError} from '@fluxer/errors/src/domains/channel/UnknownMessageError';
@@ -211,15 +216,18 @@ export class MessageSendService {
return processed.length > 0 ? processed : undefined;
}
private resolveWebhookAttachmentUploadUserId(
private async resolveWebhookAttachmentUploadUserId(
webhook: Webhook,
attachments?: Array<AttachmentRequestData>,
): UserID | undefined {
const uploadUserId = webhook.creatorId ?? undefined;
if (uploadUserId === undefined && this.attachmentsToProcess(attachments) !== undefined) {
throw InputValidationError.fromCode('attachments', ValidationErrorCodes.INVALID_MESSAGE_DATA);
): Promise<UserID | undefined> {
if (this.attachmentsToProcess(attachments) === undefined) {
return webhook.creatorId ?? undefined;
}
return uploadUserId;
if (!webhook.creatorId) {
return createUserID(DELETED_USER_ID);
}
const creator = await this.deps.userRepository.findUnique(webhook.creatorId);
return creator ? webhook.creatorId : createUserID(DELETED_USER_ID);
}
private getOneToOneDmRecipientId(channel: Channel, senderId: UserID): UserID | null {
@@ -1184,7 +1192,7 @@ export class MessageSendService {
flags: this.deps.validationService.calculateMessageFlags(data),
embeds: data.embeds,
attachments: this.attachmentsToProcess(data.attachments),
attachmentUploadUserId: this.resolveWebhookAttachmentUploadUserId(webhook, data.attachments),
attachmentUploadUserId: await this.resolveWebhookAttachmentUploadUserId(webhook, data.attachments),
stickerIds: data.sticker_ids ? data.sticker_ids.flatMap((stickerId) => createStickerID(stickerId)) : undefined,
messageReference,
messageSnapshots,
@@ -1269,7 +1277,7 @@ export class MessageSendService {
data,
channel,
guild,
attachmentUploadUserId: this.resolveWebhookAttachmentUploadUserId(webhook, data.attachments),
attachmentUploadUserId: await this.resolveWebhookAttachmentUploadUserId(webhook, data.attachments),
allowEmbeds: true,
});
await this.deps.dispatchService.dispatchMessageUpdate({channel, message: updatedMessage, requestCache});
fluxer_api/src/api/webhook/WebhookController.ts
+1
View File
@@ -102,6 +102,7 @@ async function parseWebhookMultipartMessageData(
onPayloadParsed(payload) {
parsedPayload = payload;
},
actor: 'webhook',
},
);
if (!parsedPayload) {
fluxer_api/src/api/webhook/tests/WebhookMultipartAttachmentUploads.test.ts
+34 -2
View File
@@ -1,12 +1,14 @@
// SPDX-License-Identifier: AGPL-3.0-or-later
import {Permissions} from '@fluxer/constants/src/ChannelConstants';
import {afterEach, beforeEach, describe, expect, it} from 'vitest';
import {createTestAccount} from '../../auth/tests/AuthTestUtils';
import {loadFixture} from '../../channel/tests/AttachmentTestUtils';
import {createGuild} from '../../guild/tests/GuildTestUtils';
import {createPermissionOverwrite} from '../../channel/tests/ChannelTestUtils';
import {acceptInvite, addMemberRole, createGuild, createRole} from '../../guild/tests/GuildTestUtils';
import {type ApiTestHarness, createApiTestHarness} from '../../test/ApiTestHarness';
import {HTTP_STATUS} from '../../test/TestConstants';
import {createWebhook, deleteWebhook, executeWebhookWithAttachments} from './WebhookTestUtils';
import {createChannelInvite, createWebhook, deleteWebhook, executeWebhookWithAttachments} from './WebhookTestUtils';
describe('Webhook multipart attachment uploads', () => {
let harness: ApiTestHarness;
@@ -92,6 +94,36 @@ describe('Webhook multipart attachment uploads', () => {
expect(response.status).toBe(HTTP_STATUS.BAD_REQUEST);
await deleteWebhook(harness, webhook.id, owner.token);
});
it('executes multipart webhook requests when the creator is denied attach files', async () => {
const owner = await createTestAccount(harness);
const creator = await createTestAccount(harness);
const guild = await createGuild(harness, owner.token, 'Webhook creator denied attach files guild');
const channelId = guild.system_channel_id!;
const invite = await createChannelInvite(harness, owner.token, channelId);
await acceptInvite(harness, creator.token, invite.code);
const webhookManagerRole = await createRole(harness, owner.token, guild.id, {
name: 'Webhook Manager',
permissions: Permissions.MANAGE_WEBHOOKS.toString(),
});
await addMemberRole(harness, owner.token, guild.id, creator.userId, webhookManagerRole.id);
const webhook = await createWebhook(harness, channelId, creator.token, 'Denied Creator Webhook');
await createPermissionOverwrite(harness, owner.token, channelId, creator.userId, {
type: 1,
allow: '0',
deny: Permissions.ATTACH_FILES.toString(),
});
const {response, json} = await executeWebhookWithAttachments(harness, {
webhookId: webhook.id,
webhookToken: webhook.token,
payload: {
attachments: [{id: 0, filename: 'denied_creator.png'}],
},
files: [{index: 0, filename: 'denied_creator.png', data: loadFixture('yeah.png')}],
});
expect(response.status).toBe(HTTP_STATUS.OK);
expect(json?.attachments?.length).toBe(1);
await deleteWebhook(harness, webhook.id, owner.token);
});
it('rejects multipart webhook requests when file indices do not match metadata IDs', async () => {
const owner = await createTestAccount(harness);
const guild = await createGuild(harness, owner.token, 'Webhook multipart id mismatch guild');
fluxer_api/src/api/webhook/tests/WebhookOrphanedCreator.test.ts
+8 -13
View File
@@ -1,6 +1,5 @@
// SPDX-License-Identifier: AGPL-3.0-or-later
import {APIErrorCodes} from '@fluxer/constants/src/ApiErrorCodes';
import {DELETED_USER_ID, DELETED_USER_USERNAME} from '@fluxer/constants/src/UserConstants';
import {afterEach, beforeEach, describe, expect, it} from 'vitest';
import {createTestAccount} from '../../auth/tests/AuthTestUtils';
@@ -13,10 +12,6 @@ import {createWebhook, executeWebhook, executeWebhookWithAttachments, getChannel
const VANISHED_CREATOR_ID = createUserID(999999999999999997n);
function parseErrorCode(text: string): string | undefined {
return (JSON.parse(text) as {code?: string}).code;
}
describe('Webhook whose creating account cannot be resolved', () => {
let harness: ApiTestHarness;
beforeEach(async () => {
@@ -49,13 +44,13 @@ describe('Webhook whose creating account cannot be resolved', () => {
expect(listed?.user.id).toBe(VANISHED_CREATOR_ID.toString());
expect(listed?.user.username).toBe(DELETED_USER_USERNAME);
});
it('answers a multipart execution for a webhook with no creator id with an access decision', async () => {
it('executes a multipart payload for a webhook with no creator id', async () => {
const owner = await createTestAccount(harness);
const guild = await createGuild(harness, owner.token, 'Null creator multipart guild');
const channelId = guild.system_channel_id!;
const webhook = await createWebhook(harness, channelId, owner.token, 'Null Creator Multipart Webhook');
await new WebhookRepository().update(createWebhookID(BigInt(webhook.id)), {creatorId: null});
const {response, text} = await executeWebhookWithAttachments(harness, {
const {response, json} = await executeWebhookWithAttachments(harness, {
webhookId: webhook.id,
webhookToken: webhook.token,
payload: {
@@ -63,16 +58,16 @@ describe('Webhook whose creating account cannot be resolved', () => {
},
files: [{index: 0, filename: 'orphaned.txt', data: Buffer.from('uploaded by an orphaned webhook')}],
});
expect(response.status).toBe(HTTP_STATUS.FORBIDDEN);
expect(parseErrorCode(text)).toBe(APIErrorCodes.ACCESS_DENIED);
expect(response.status).toBe(HTTP_STATUS.OK);
expect(json?.attachments?.[0].filename).toBe('orphaned.txt');
});
it('answers a multipart execution for a webhook whose creator row is gone with an access decision', async () => {
it('executes a multipart payload for a webhook whose creator row is gone', async () => {
const owner = await createTestAccount(harness);
const guild = await createGuild(harness, owner.token, 'Vanished creator multipart guild');
const channelId = guild.system_channel_id!;
const webhook = await createWebhook(harness, channelId, owner.token, 'Vanished Creator Multipart Webhook');
await new WebhookRepository().update(createWebhookID(BigInt(webhook.id)), {creatorId: VANISHED_CREATOR_ID});
const {response, text} = await executeWebhookWithAttachments(harness, {
const {response, json} = await executeWebhookWithAttachments(harness, {
webhookId: webhook.id,
webhookToken: webhook.token,
payload: {
@@ -80,8 +75,8 @@ describe('Webhook whose creating account cannot be resolved', () => {
},
files: [{index: 0, filename: 'vanished.txt', data: Buffer.from('uploaded by a vanished creator')}],
});
expect(response.status).toBe(HTTP_STATUS.FORBIDDEN);
expect(parseErrorCode(text)).toBe(APIErrorCodes.ACCESS_DENIED);
expect(response.status).toBe(HTTP_STATUS.OK);
expect(json?.attachments?.[0].filename).toBe('vanished.txt');
});
it('executes a json payload for a webhook with no creator id', async () => {
const owner = await createTestAccount(harness);