Harden API validation throttling and error handling

src/backend/src/app.module.ts

@@ -13,6 +13,7 @@ import { EnvironmentEnum } from './environmentEnum';
 import { BullModule } from '@nestjs/bullmq';
 import { APP_GUARD } from '@nestjs/core';
 import { AuthGuard } from './shared/auth.guard';
+import { ThrottlerGuard, ThrottlerModule } from '@nestjs/throttler';
 
 @Module({
   imports: [
@@ -44,6 +45,12 @@ import { AuthGuard } from './shared/auth.guard';
       ],
       inject: [ConfigService],
     }),
+    ThrottlerModule.forRoot([
+      {
+        ttl: 60000,
+        limit: 60,
+      },
+    ]),
     BullModule.forRootAsync({
       imports: [ConfigModule],
       useFactory: async (configService: ConfigService) => ({
@@ -66,6 +73,10 @@ import { AuthGuard } from './shared/auth.guard';
       provide: APP_GUARD,
       useClass: AuthGuard,
     },
+    {
+      provide: APP_GUARD,
+      useClass: ThrottlerGuard,
+    },
   ],
 })
 export class AppModule {}

src/backend/src/main.ts

@@ -1,4 +1,6 @@
 import { NestFactory } from '@nestjs/core';
+import { ValidationPipe } from '@nestjs/common';
+import { HttpExceptionFilter } from './shared/filters/http-exception.filter';
 import { AppModule } from './app.module';
 import * as fs from 'fs';
 import { resolve } from 'path';
@@ -31,7 +33,19 @@ async function bootstrap() {
     }
   }
 
-  const app = await NestFactory.create(AppModule);
+  const app = await NestFactory.create(AppModule, {
+    bodyParser: true,
+  });
+
+  app.useGlobalPipes(
+    new ValidationPipe({
+      whitelist: true,
+      forbidNonWhitelisted: true,
+      transform: true,
+    }),
+  );
+
+  app.useGlobalFilters(new HttpExceptionFilter());
   app.setGlobalPrefix('api');
   await app.listen(process.env.PORT || 3000);
 }

src/backend/src/playlist/dto/create-playlist.dto.ts

@@ -0,0 +1,11 @@
+import { IsString, IsUrl, MaxLength } from 'class-validator';
+
+export class CreatePlaylistDto {
+  @IsString()
+  @MaxLength(2048)
+  @IsUrl({
+    require_protocol: true,
+    protocols: ['https'],
+  })
+  spotifyUrl: string;
+}

src/backend/src/playlist/dto/update-playlist.dto.ts

@@ -0,0 +1,7 @@
+import { IsBoolean, IsOptional } from 'class-validator';
+
+export class UpdatePlaylistDto {
+  @IsOptional()
+  @IsBoolean()
+  active?: boolean;
+}

src/backend/src/playlist/playlist.controller.ts

@@ -4,11 +4,14 @@ import {
   Delete,
   Get,
   Param,
+  ParseIntPipe,
   Post,
   Put,
 } from '@nestjs/common';
 import { PlaylistService } from './playlist.service';
 import { PlaylistEntity } from './playlist.entity';
+import { CreatePlaylistDto } from './dto/create-playlist.dto';
+import { UpdatePlaylistDto } from './dto/update-playlist.dto';
 
 @Controller('playlist')
 export class PlaylistController {
@@ -20,25 +23,27 @@ export class PlaylistController {
   }
 
   @Post()
-  async create(@Body() playlist: PlaylistEntity): Promise<void> {
-    await this.service.create(playlist);
+  async create(@Body() playlist: CreatePlaylistDto): Promise<void> {
+    await this.service.create(playlist as PlaylistEntity);
   }
 
   @Put(':id')
   update(
-    @Param('id') id: number,
-    @Body() playlist: Partial<PlaylistEntity>,
+    @Param('id', ParseIntPipe) id: number,
+    @Body() playlist: UpdatePlaylistDto,
   ): Promise<void> {
     return this.service.update(id, playlist);
   }
 
   @Delete(':id')
-  remove(@Param('id') id: number): Promise<void> {
+  remove(@Param('id', ParseIntPipe) id: number): Promise<void> {
     return this.service.remove(id);
   }
 
   @Get('retry/:id')
-  retryFailedOfPlaylist(@Param('id') id: number): Promise<void> {
+  retryFailedOfPlaylist(
+    @Param('id', ParseIntPipe) id: number,
+  ): Promise<void> {
     return this.service.retryFailedOfPlaylist(id);
   }
 }

src/backend/src/playlist/playlist.service.ts

@@ -10,6 +10,7 @@ import { Interval } from '@nestjs/schedule';
 import { TrackStatusEnum } from '../track/track.entity';
 import { UtilsService } from '../shared/utils.service';
 import { SpotifyService } from '../shared/spotify.service';
+import { toSafeErrorMessage } from '../shared/errors/safe-error';
 
 enum WsPlaylistOperation {
   New = 'playlistNew',
@@ -77,7 +78,11 @@ export class PlaylistService {
       // Don't create folder structure for individual tracks - they go in root
     } catch (err) {
       this.logger.error(`Error getting track details: ${err}`);
-      playlist2Save = { ...playlist, error: String(err), isTrack: true };
+      playlist2Save = {
+        ...playlist,
+        error: toSafeErrorMessage(err),
+        isTrack: true,
+      };
     }
     const savedPlaylist = await this.save(playlist2Save);
 
@@ -117,7 +122,10 @@ export class PlaylistService {
       this.createPlaylistFolderStructure(playlist2Save.name);
     } catch (err) {
       this.logger.error(`Error getting playlist details: ${err}`);
-      playlist2Save = { ...playlist, error: String(err) };
+      playlist2Save = {
+        ...playlist,
+        error: toSafeErrorMessage(err),
+      };
     }
     const savedPlaylist = await this.save(playlist2Save);
 
@@ -224,7 +232,10 @@ export class PlaylistService {
         );
         this.createPlaylistFolderStructure(playlist.name);
       } catch (err) {
-        await this.update(playlist.id, { ...playlist, error: String(err) });
+        await this.update(playlist.id, {
+          ...playlist,
+          error: toSafeErrorMessage(err),
+        });
       }
       for (const track of tracks ?? []) {
         const track2Save = {

src/backend/src/shared/errors/safe-error.ts

@@ -0,0 +1,20 @@
+export function toSafeErrorMessage(error: unknown): string {
+  if (error instanceof Error) {
+    return sanitize(error.message);
+  }
+
+  if (typeof error === 'string') {
+    return sanitize(error);
+  }
+
+  return 'Unknown error';
+}
+
+function sanitize(text: string): string {
+  return text
+    .replace(/\/home\/[^\s]+/g, '[path]')
+    .replace(/\/root\/[^\s]+/g, '[path]')
+    .replace(/token=[^\s&]+/gi, 'token=[redacted]')
+    .replace(/authorization:[^\n]+/gi, 'authorization:[redacted]')
+    .slice(0, 500);
+}

src/backend/src/shared/filters/http-exception.filter.ts

@@ -0,0 +1,42 @@
+import {
+  ArgumentsHost,
+  Catch,
+  ExceptionFilter,
+  HttpException,
+  HttpStatus,
+  Logger,
+} from '@nestjs/common';
+import type { Request, Response } from 'express';
+
+@Catch()
+export class HttpExceptionFilter implements ExceptionFilter {
+  private readonly logger = new Logger(HttpExceptionFilter.name);
+
+  catch(exception: unknown, host: ArgumentsHost): void {
+    const ctx = host.switchToHttp();
+
+    const response = ctx.getResponse<Response>();
+    const request = ctx.getRequest<Request>();
+
+    const status =
+      exception instanceof HttpException
+        ? exception.getStatus()
+        : HttpStatus.INTERNAL_SERVER_ERROR;
+
+    const message =
+      exception instanceof HttpException
+        ? exception.message
+        : 'Internal server error';
+
+    this.logger.error(
+      `${request.method} ${request.url} -> ${status}: ${String(exception)}`,
+    );
+
+    response.status(status).json({
+      statusCode: status,
+      message,
+      timestamp: new Date().toISOString(),
+      path: request.url,
+    });
+  }
+}

src/backend/src/track/track.controller.ts

@@ -2,12 +2,14 @@ import {
   Controller,
   Delete,
   Get,
+  NotFoundException,
   Param,
+  ParseIntPipe,
   Res,
   StreamableFile,
 } from '@nestjs/common';
 import { TrackService } from './track.service';
-import { createReadStream } from 'fs';
+import { createReadStream, existsSync } from 'fs';
 import type { Response } from 'express';
 import { ConfigService } from '@nestjs/config';
 import { TrackEntity } from './track.entity';
@@ -20,33 +22,54 @@ export class TrackController {
   ) {}
 
   @Get('playlist/:id')
-  getAllByPlaylist(@Param('id') playlistId: number): Promise<TrackEntity[]> {
+  getAllByPlaylist(
+    @Param('id', ParseIntPipe) playlistId: number,
+  ): Promise<TrackEntity[]> {
     return this.service.getAllByPlaylist(playlistId);
   }
 
   @Get('download/:id')
   async getFile(
     @Res({ passthrough: true }) res: Response,
-    @Param('id') id: number,
+    @Param('id', ParseIntPipe) id: number,
   ): Promise<StreamableFile> {
     const track = await this.service.get(id);
+
+    if (!track || !track.playlist) {
+      throw new NotFoundException('Track not found');
+    }
+
+    const filePath = this.service.getFolderName(track, track.playlist);
+
+    if (!existsSync(filePath)) {
+      throw new NotFoundException('Track file not found');
+    }
+
     const fileName = this.service.getTrackFileName(track);
-    const readStream = createReadStream(
-      this.service.getFolderName(track, track.playlist),
-    );
+
+    const readStream = createReadStream(filePath);
+
     res.set({
-      'Content-Disposition': `attachment; filename="${encodeURIComponent(fileName)}`,
+      'Content-Disposition':
+        `attachment; filename="${encodeURIComponent(fileName)}"`,
+      'Content-Type': 'application/octet-stream',
+      'X-Content-Type-Options': 'nosniff',
     });
+
     return new StreamableFile(readStream);
   }
 
   @Delete(':id')
-  remove(@Param('id') id: number): Promise<void> {
+  remove(
+    @Param('id', ParseIntPipe) id: number,
+  ): Promise<void> {
     return this.service.remove(id);
   }
 
   @Get('retry/:id')
-  retry(@Param('id') id: number): Promise<void> {
+  retry(
+    @Param('id', ParseIntPipe) id: number,
+  ): Promise<void> {
     return this.service.retry(id);
   }
 }

src/backend/src/track/track.service.ts

@@ -12,6 +12,7 @@ import { UtilsService } from '../shared/utils.service';
 import { Queue } from 'bullmq';
 import { InjectQueue } from '@nestjs/bullmq';
 import { YoutubeService } from '../shared/youtube.service';
+import { toSafeErrorMessage } from '../shared/errors/safe-error';
 
 enum WsTrackOperation {
   New = 'trackNew',
@@ -73,7 +74,16 @@ export class TrackService {
 
   async retry(id: number): Promise<void> {
     const track = await this.get(id);
-    await this.trackSearchQueue.add('', track, { jobId: `id-${id}` });
+    const existingJob = await this.trackSearchQueue.getJob(`id-${id}`);
+
+    if (existingJob) {
+      this.logger.warn(`Search job already exists for track ${id}`);
+      return;
+    }
+
+    await this.trackSearchQueue.add('', track, {
+      jobId: `id-${id}`,
+    });
     await this.update(id, { ...track, status: TrackStatusEnum.New });
   }
 
@@ -96,7 +106,7 @@ export class TrackService {
       this.logger.error(err);
       updatedTrack = {
         ...track,
-        error: String(err),
+        error: toSafeErrorMessage(err),
         status: TrackStatusEnum.Error,
       };
       await this.update(track.id, updatedTrack);
@@ -150,7 +160,7 @@ export class TrackService {
       }
     } catch (err) {
       this.logger.error(err);
-      error = String(err);
+      error = toSafeErrorMessage(err);
     }
 
     const updatedTrack = {