feat(media): serve uploads from local filesystem with permission check

Previously, the permission checks of the /media/:uuid route could be
bypassed for the local filesystem as a media backend, because the
/media/:uuid route performed a redirect to the target after checking
the permissions. Since the target was always /uploads/:uuid.ext where
ext was the extension of the uploaded file, you could simply try to
guess the file extension and have access to the file since the
/uploads endpoint was simply the complete uploads folder mounted.
Now, media uploads from the local filesystem backend are served
under the /media route directly instead of a redirect.

Signed-off-by: Erik Michelson <github@erik.michelson.eu>
This commit is contained in:
Erik Michelson
2026-07-05 02:57:56 +02:00
parent c2ccf48c39
commit 6c5b44eaec
14 changed files with 161 additions and 72 deletions
@@ -6,6 +6,7 @@
import { MediaUploadSchema, PermissionLevel } from '@hedgedoc/commons';
import { FieldNameMediaUpload } from '@hedgedoc/database';
import {
BadRequestException,
Controller,
Delete,
Get,
@@ -14,7 +15,6 @@ import {
Put,
UseGuards,
UseInterceptors,
BadRequestException,
} from '@nestjs/common';
import { ApiBody, ApiConsumes, ApiHeader, ApiTags } from '@nestjs/swagger';
@@ -128,7 +128,7 @@ export class MediaController {
@Param('uuid') uuid: string,
): Promise<MediaUploadDto> {
if (!(await this.mediaService.canUserAccessUpload(userId, uuid))) {
throw new PermissionError('You do not have permission to access this media upload');
throw new PermissionError('You do not have permission to access this media upload.');
}
return await this.mediaService.getMediaUploadDtoByUuid(uuid);
}
@@ -9,7 +9,8 @@ import { createParamDecorator, ExecutionContext, UnauthorizedException } from '@
import { CompleteRequest } from '../request.type';
type RequestUserIdParameter = {
forbidGuests: boolean;
forbidGuests?: boolean;
allowAnonymous?: boolean;
};
/**
@@ -18,17 +19,26 @@ type RequestUserIdParameter = {
* If a user is present in the request, returns the user object.
* If no user is present and guests are allowed, returns `null`.
* If no user is present and guests are not allowed, throws {@link UnauthorizedException}.
* If no user is present and `allowAnonymous` is true, returns `null` without throwing.
*/
// oxlint-disable-next-line @typescript-eslint/naming-convention
export const RequestUserId = createParamDecorator(
(data: RequestUserIdParameter = { forbidGuests: false }, ctx: ExecutionContext) => {
(
data: RequestUserIdParameter = { forbidGuests: false, allowAnonymous: false },
ctx: ExecutionContext,
) => {
const request: CompleteRequest = ctx.switchToHttp().getRequest();
if (
!request.authProviderType ||
(request.authProviderType === AuthProviderType.GUEST && data.forbidGuests)
) {
// The session is always present (the session middleware runs for every
// request); fall back to it so that this decorator works even when no
// guard has populated `request.userId` yet.
const userId = request.userId ?? request.session?.userId ?? null;
const authProviderType = request.authProviderType ?? request.session?.loginAuthProviderType;
if (!authProviderType || (authProviderType === AuthProviderType.GUEST && data.forbidGuests)) {
if (data.allowAnonymous) {
return null;
}
throw new UnauthorizedException("You're not logged in");
}
return request.userId;
return userId;
},
);
-12
View File
@@ -3,7 +3,6 @@
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import { MediaBackendType } from '@hedgedoc/commons';
import { HttpAdapterHost } from '@nestjs/core';
import { NestFastifyApplication } from '@nestjs/platform-fastify';
import { WsAdapter } from '@nestjs/platform-ws';
@@ -111,17 +110,6 @@ export async function setupApp(
app.useGlobalPipes(setupValidationPipe(logger));
// Map URL paths to directories
if (mediaConfig.backend.type === MediaBackendType.FILESYSTEM) {
logger.log(
`Serving the local folder '${mediaConfig.backend.filesystem.uploadPath}' under '/uploads'`,
'AppBootstrap',
);
const path = await import('path');
await app.register(import('@fastify/static'), {
root: path.resolve(mediaConfig.backend.filesystem.uploadPath),
prefix: '/uploads/',
});
}
logger.log(`Serving the local folder 'public' under '/public'`, 'AppBootstrap');
const path = await import('path');
await app.register(import('@fastify/static'), {
+1 -1
View File
@@ -35,8 +35,8 @@ import { FrontendConfigService } from './frontend-config/frontend-config.service
import { GroupsModule } from './groups/groups.module';
import { KnexLoggerService } from './logger/knex-logger.service';
import { LoggerModule } from './logger/logger.module';
import { MediaRedirectModule } from './media-redirect/media-redirect.module';
import { MediaModule } from './media/media.module';
import { MediaRedirectModule } from './media-redirect/media-redirect.module';
import { MessageModule } from './message/message.module';
import { MonitoringModule } from './monitoring/monitoring.module';
import { PermissionsModule } from './permissions/permissions.module';
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file)
* SPDX-FileCopyrightText: 2026 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
@@ -8,9 +8,9 @@ import { ApiTags } from '@nestjs/swagger';
import { FastifyReply } from 'fastify';
import { OpenApi } from '../api/utils/decorators/openapi.decorator';
import { RequestUserId } from '../api/utils/decorators/request-user-id.decorator';
import { ConsoleLoggerService } from '../logger/console-logger.service';
import { MediaService } from '../media/media.service';
import { RequestUserId } from '../api/utils/decorators/request-user-id.decorator';
import { PermissionError } from '../errors/errors';
@OpenApi()
@@ -25,16 +25,25 @@ export class MediaRedirectController {
}
@Get(':uuid')
@OpenApi(302, 404, 500)
@OpenApi(200, 302, 404, 500)
async getMedia(
@RequestUserId() userId: number,
@RequestUserId({ allowAnonymous: true }) userId: number | null,
@Param('uuid') uuid: string,
@Res() response: FastifyReply,
): Promise<void> {
if (!(await this.mediaService.canUserAccessUpload(userId, uuid))) {
throw new PermissionError('You do not have permission to access this media upload.');
}
const url = await this.mediaService.getFileUrl(uuid);
await response.redirect(url);
const fileResponse = await this.mediaService.getFileResponse(uuid);
if (fileResponse.type === 'redirect') {
await response.redirect(fileResponse.url);
return;
}
await response
.header('Content-Type', fileResponse.contentType)
.header('Content-Length', fileResponse.buffer.byteLength)
.header('Content-Disposition', `attachment; filename="${fileResponse.fileName}"`)
.status(200)
.send(fileResponse.buffer);
}
}
@@ -13,6 +13,7 @@ import mediaConfiguration, { MediaConfig } from '../../config/media.config';
import { MediaBackendError } from '../../errors/errors';
import { ConsoleLoggerService } from '../../logger/console-logger.service';
import { MediaBackend } from '../media-backend.interface';
import { MediaFileResponse } from '../media-response.interface'
@Injectable()
export class FilesystemBackend implements MediaBackend {
@@ -39,7 +40,7 @@ export class FilesystemBackend implements MediaBackend {
await this.ensureDirectory();
try {
await fs.writeFile(filePath, buffer, null);
return JSON.stringify({ ext: fileType.ext });
return JSON.stringify({ ext: fileType.ext, mime: fileType.mime });
} catch (e) {
this.logger.error((e as Error).message, (e as Error).stack, 'saveFile');
throw new MediaBackendError(`Could not save file '${filePath}'`);
@@ -63,15 +64,33 @@ export class FilesystemBackend implements MediaBackend {
}
}
getFileUrl(uuid: string, backendData: string): Promise<string> {
getFileUrl(uuid: string, _: string): Promise<string> {
return Promise.resolve(`/media/${uuid}`);
}
/**
* Reads the file from the local filesystem and returns its content together
* with the detected MIME type..
*
* @param uuid Unique identifier of the uploaded file
* @param backendData Internal backend data
* @returns Object containing the file buffer and the detected MIME type
*/
async getFileResponse(
uuid: string,
backendData: string | null,
): Promise<Omit<MediaFileResponse, 'type'>> {
if (!backendData) {
throw new MediaBackendError('No backend data provided');
}
const { ext } = JSON.parse(backendData) as { ext: string };
const { ext, mime } = JSON.parse(backendData) as { ext: string, mime: string };
if (!ext) {
throw new MediaBackendError('No file extension in backend data');
}
return Promise.resolve(`/uploads/${uuid}.${ext}`);
const filePath = this.getFilePath(uuid, ext);
const buffer = await fs.readFile(filePath);
const contentType = mime ?? 'application/octet-stream';
return { buffer, contentType, fileName: `${uuid}.${ext}` };
}
private getFilePath(fileName: string, extension: string): string {
@@ -0,0 +1,18 @@
/*
* SPDX-FileCopyrightText: 2026 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
export interface MediaRedirectResponse {
type: 'redirect';
url: string;
}
export interface MediaFileResponse {
type: 'file';
contentType: string;
fileName: string;
buffer: Buffer;
}
export type MediaResponse = MediaRedirectResponse | MediaFileResponse;
+20 -8
View File
@@ -189,33 +189,45 @@ describe('MediaService', () => {
});
});
describe('getFileUrl', () => {
it('returns file url if found', async () => {
describe('getFileResponse', () => {
it('returns the file content for the filesystem backend', async () => {
mockSelect(
tracker,
[
FieldNameMediaUpload.backendType,
FieldNameMediaUpload.backendData],
FieldNameMediaUpload.backendData,
FieldNameMediaUpload.fileName,
],
TableMediaUpload,
FieldNameMediaUpload.uuid,
{
[FieldNameMediaUpload.backendType]: backendType,
[FieldNameMediaUpload.backendData]: backendData,
[FieldNameMediaUpload.fileName]: fileName,
},
);
// As the media service loads the used backend dynamically, we need to
// spy on fileSystemBackend here instead of service.mediaBackend
jest
.spyOn(fileSystemBackend, 'getFileUrl')
.spyOn(fileSystemBackend, 'getFileResponse')
.mockImplementationOnce(
async (givenUuid: string, givenBackendData: string | null): Promise<string> => {
async (
givenUuid: string,
givenBackendData: string | null,
): Promise<{ buffer: Buffer; contentType: string; fileName: string }> => {
expect(givenUuid).toBe(uuid);
expect(givenBackendData).toBe(backendData);
return `http://example.com/${fileName}`;
return { buffer: fileBuffer, contentType: 'image/png', fileName: `${uuid}.png` };
},
);
const result = await service.getFileUrl(uuid);
expect(result).toBe(`http://example.com/${fileName}`);
const result = await service.getFileResponse(uuid);
expect(result).toEqual({
kind: 'file',
buffer: fileBuffer,
contentType: 'image/png',
fileName,
});
expect(tracker.history.select).toHaveLength(1);
expect(tracker.history.select[0].bindings).toEqual([uuid, 1]);
});
+6 -4
View File
@@ -40,6 +40,7 @@ import { ImgurBackend } from './backends/imgur-backend';
import { S3Backend } from './backends/s3-backend';
import { WebdavBackend } from './backends/webdav-backend';
import { MediaBackend } from './media-backend.interface';
import { MediaResponse } from './media-response.interface'
@Injectable()
export class MediaService {
@@ -292,7 +293,7 @@ export class MediaService {
* @param uuid The UUID of the media upload to check against
* @returns true if the user has access, false otherwise
*/
async canUserAccessUpload(userId: number, uuid: string): Promise<boolean> {
async canUserAccessUpload(userId: number | null, uuid: string): Promise<boolean> {
const mediaUpload = await this.knex(TableMediaUpload)
.select(FieldNameMediaUpload.userId)
.where(FieldNameMediaUpload.uuid, uuid)
@@ -302,13 +303,14 @@ export class MediaService {
}
const linkedNoteIds = await this.getLinkedNoteIds(uuid);
if (linkedNoteIds.length === 0) {
return mediaUpload[FieldNameMediaUpload.userId] === userId;
}
if (userId === null) {
return false;
}
if (linkedNoteIds.length === 0) {
return mediaUpload[FieldNameMediaUpload.userId] === userId;
}
for (const noteId of linkedNoteIds) {
const linkedNotePermission = await this.permissionService.determinePermission(userId, noteId);
if (linkedNotePermission >= PermissionLevel.READ) {
@@ -64,9 +64,15 @@ describe('Media', () => {
.set('HedgeDoc-Note', noteAlias1)
.expect(201);
uuid = uploadResponse.body.uuid;
const apiResponse = await agentUser1.get(`${PRIVATE_API_PREFIX}/media/${uuid}`);
expect(apiResponse.statusCode).toEqual(200);
const downloadResponse = await agentUser1.get(`/uploads/${uuid}.png`);
const downloadResponse = await agentUser1
.get(`/media/${uuid}`)
.buffer(true)
.parse((response, callback) => {
const chunks: Buffer[] = [];
response.on('data', (chunk: Buffer) => chunks.push(chunk));
response.on('end', () => callback(null, Buffer.concat(chunks)));
});
expect(downloadResponse.statusCode).toEqual(200);
expect(downloadResponse.body).toEqual(testImage);
});
it('with user and uppercase note alias', async () => {
@@ -76,9 +82,15 @@ describe('Media', () => {
.set('HedgeDoc-Note', noteAlias1.toUpperCase())
.expect(201);
uuid = uploadResponse.body.uuid;
const apiResponse = await agentUser1.get(`${PRIVATE_API_PREFIX}/media/${uuid}`);
expect(apiResponse.statusCode).toEqual(200);
const downloadResponse = await agentUser1.get(`/uploads/${uuid}.png`);
const downloadResponse = await agentUser1
.get(`/media/${uuid}`)
.buffer(true)
.parse((response, callback) => {
const chunks: Buffer[] = [];
response.on('data', (chunk: Buffer) => chunks.push(chunk));
response.on('end', () => callback(null, Buffer.concat(chunks)));
});
expect(downloadResponse.statusCode).toEqual(200);
expect(downloadResponse.body).toEqual(testImage);
});
it('with guest user', async () => {
@@ -94,9 +106,15 @@ describe('Media', () => {
.set('HedgeDoc-Note', noteDtoResponse.body.metadata.primaryAlias)
.expect(201);
uuid = uploadResponse.body.uuid;
const apiResponse = await agentGuestUser.get(`${PRIVATE_API_PREFIX}/media/${uuid}`);
expect(apiResponse.statusCode).toEqual(200);
const downloadResponse = await agentGuestUser.get(`/uploads/${uuid}.png`);
const downloadResponse = await agentGuestUser
.get(`/media/${uuid}`)
.buffer(true)
.parse((response, callback) => {
const chunks: Buffer[] = [];
response.on('data', (chunk: Buffer) => chunks.push(chunk));
response.on('end', () => callback(null, Buffer.concat(chunks)));
});
expect(downloadResponse.statusCode).toEqual(200);
expect(downloadResponse.body).toEqual(testImage);
});
it('with guest user and uppercase note alias', async () => {
@@ -112,9 +130,15 @@ describe('Media', () => {
.set('HedgeDoc-Note', noteDtoResponse.body.metadata.primaryAlias.toUpperCase())
.expect(201);
uuid = uploadResponse.body.uuid;
const apiResponse = await agentGuestUser.get(`${PRIVATE_API_PREFIX}/media/${uuid}`);
expect(apiResponse.statusCode).toEqual(200);
const downloadResponse = await agentGuestUser.get(`/uploads/${uuid}.png`);
const downloadResponse = await agentGuestUser
.get(`/media/${uuid}`)
.buffer(true)
.parse((response, callback) => {
const chunks: Buffer[] = [];
response.on('data', (chunk: Buffer) => chunks.push(chunk));
response.on('end', () => callback(null, Buffer.concat(chunks)));
});
expect(downloadResponse.statusCode).toEqual(200);
expect(downloadResponse.body).toEqual(testImage);
});
});
@@ -219,11 +243,11 @@ describe('Media', () => {
testSetup.ownedNoteIds[0],
);
await agentUser1.get(`/uploads/${uuid}.png`).expect(200);
await agentUser1.get(`/media/${uuid}`).expect(200);
await agentUser1.delete(`${PRIVATE_API_PREFIX}/media/${uuid}`).expect(204);
await agentUser1.get(`/uploads/${uuid}.png`).expect(404);
await agentUser1.get(`/media/${uuid}`).expect(404);
});
it('allowed if user is owner of note', async () => {
const uuid = await testSetup.mediaService.saveFile(
@@ -233,11 +257,11 @@ describe('Media', () => {
testSetup.ownedNoteIds[0],
);
await agentUser1.get(`/uploads/${uuid}.png`).expect(200);
await agentUser1.get(`/media/${uuid}`).expect(200);
await agentUser1.delete(`${PRIVATE_API_PREFIX}/media/${uuid}`).expect(204);
await agentUser1.get(`/uploads/${uuid}.png`).expect(404);
await agentUser1.get(`/media/${uuid}`).expect(404);
});
it("other user can't delete", async () => {
const uuid = await testSetup.mediaService.saveFile(
@@ -180,10 +180,12 @@ describe('Media', () => {
.set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`)
.expect(403);
// The second user has no access to the file at all, neither as the
// uploader nor via a linked note, so the read endpoint also rejects.
await agent
.get(`/uploads/${upload}.png`)
.get(`/media/${upload}`)
.set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`)
.expect(200);
.expect(403);
// delete upload for real
await agent
@@ -193,7 +195,7 @@ describe('Media', () => {
// Test if file is really deleted
await agent
.get(`/uploads/${upload}.png`)
.get(`/media/${upload}`)
.set('Authorization', `Bearer ${testSetup.authTokens[0].secret}`)
.expect(404);
});
@@ -217,10 +219,12 @@ describe('Media', () => {
.set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`)
.expect(403);
// The second user has no access to the file at all, neither as the
// uploader nor via a linked note, so the read endpoint also rejects.
await agent
.get(`/uploads/${upload}.png`)
.get(`/media/${upload}`)
.set('Authorization', `Bearer ${testSetup.authTokens[1].secret}`)
.expect(200);
.expect(403);
// delete upload for real
await agent
@@ -230,7 +234,7 @@ describe('Media', () => {
// Test if file is really deleted
await agent
.get(`/uploads/${upload}.png`)
.get(`/media/${upload}`)
.set('Authorization', `Bearer ${testSetup.authTokens[2].secret}`)
.expect(404);
});
+6
View File
@@ -68,6 +68,7 @@ import { ConsoleLoggerService } from '../src/logger/console-logger.service';
import { LoggerModule } from '../src/logger/logger.module';
import { FilesystemBackend } from '../src/media/backends/filesystem-backend';
import { MediaModule } from '../src/media/media.module';
import { MediaRedirectModule } from '../src/media-redirect/media-redirect.module';
import { MediaService } from '../src/media/media.service';
import { MonitoringModule } from '../src/monitoring/monitoring.module';
import { NoteService } from '../src/notes/note.service';
@@ -262,6 +263,10 @@ export class TestSetupBuilder {
path: PRIVATE_API_PREFIX,
module: PrivateApiModule,
},
{
path: '/media',
module: MediaRedirectModule,
},
];
process.env.HD_BASE_URL = `https://${testId}.example.com`;
@@ -308,6 +313,7 @@ export class TestSetupBuilder {
FrontendConfigModule,
AuthModule,
SessionModule,
MediaRedirectModule,
EventEmitterModule.forRoot(eventModuleConfig),
],
providers: [
-1
View File
@@ -25,7 +25,6 @@
reverse_proxy /realtime http://localhost:{$HD_BACKEND_PORT:3000}
reverse_proxy /api/* http://localhost:{$HD_BACKEND_PORT:3000}
reverse_proxy /public/* http://localhost:{$HD_BACKEND_PORT:3000}
reverse_proxy /uploads/* http://localhost:{$HD_BACKEND_PORT:3000}
reverse_proxy /media/* http://localhost:{$HD_BACKEND_PORT:3000}
reverse_proxy /* http://localhost:{$HD_FRONTEND_PORT:3001}
}
+2 -4
View File
@@ -31,7 +31,7 @@ in your `docker-compose.yml`:
- hedgedoc_uploads:/usr/src/app/backend/uploads
labels:
traefik.enable: "true"
traefik.http.routers.hedgedoc_2_backend.rule: "Host(`md.example.com`) && (PathPrefix(`/realtime`) || PathPrefix(`/api`) || PathPrefix(`/public`) || PathPrefix(`/uploads`) || PathPrefix(`/media`))"
traefik.http.routers.hedgedoc_2_backend.rule: "Host(`md.example.com`) && (PathPrefix(`/realtime`) || PathPrefix(`/api`) || PathPrefix(`/public`) || PathPrefix(`/media`))"
traefik.http.routers.hedgedoc_2_backend.tls: "true"
traefik.http.routers.hedgedoc_2_backend.tls.certresolver: "letsencrypt"
traefik.http.services.hedgedoc_2_backend.loadbalancer.server.port: "3000"
@@ -113,7 +113,7 @@ Here is an example configuration for [nginx][nginx].
server {
server_name md.example.com;
location ~ ^/(api|public|uploads|media)/ {
location ~ ^/(api|public|media)/ {
proxy_pass http://127.0.0.1:3000;
proxy_set_header X-Forwarded-Host $host;
proxy_set_header X-Real-IP $remote_addr;
@@ -173,7 +173,6 @@ Here is an example config snippet for [Apache][apache]:
ProxyPassReverse /api http://127.0.0.1:3000/
ProxyPassReverse /public http://127.0.0.1:3000/
ProxyPassReverse /uploads http://127.0.0.1:3000/
ProxyPassReverse /media http://127.0.0.1:3000/
ProxyPassReverse /realtime http://127.0.0.1:3000/
@@ -201,7 +200,6 @@ Here is a list of things your reverse proxy needs to do to let HedgeDoc work:
- Passing `/realtime` to <http://localhost:3000>
- Passing `/api/*` to <http://localhost:3000>
- Passing `/public/*` to <http://localhost:3000>
- Passing `/uploads/*` to <http://localhost:3000>
- Passing `/media/*` to <http://localhost:3000>
- Passing `/*` to <http://localhost:3001>
- Set the `X-Forwarded-Proto` header