From d277868488747e778320a2d6f1856df832ab1478 Mon Sep 17 00:00:00 2001 From: Memplethee Lab Date: Sun, 11 Oct 2026 00:18:51 +0100 Subject: [PATCH 1/4] fix: bind authenticated user, add MIME type filter, and fix CI artifact paths Three security and CI fixes: Issue #156 - Replace hardcoded userId=system with authenticated user identity - FileUploadController: use @CurrentUser() decorator to extract user.id - WebhookController: same pattern for all 8 endpoints - Both controllers now throw BadRequestException if user identity missing Issue #157 - Add fileFilter allowlist to multer interceptor - Added FILE_ALLOWED_MIME_TYPES env var with comprehensive default - fileFilter rejects disallowed MIME types with 400 before storage - Configurable via environment without code changes Issue #158 - Fix CI artifact upload paths - Changed TypeScript client path from client/ to sdks/typescript - Added Python client artifact upload step for sdks/python - All three artifacts now upload correctly Closes #156 Closes #157 Closes #158 --- .env.example | 4 + .github/workflows/build-check.yml | 9 +- .../file-upload/file-upload.controller.ts | 109 +++++++++++++++++- .../webhooks/webhook.controller.ts | 56 ++++++--- 4 files changed, 158 insertions(+), 20 deletions(-) diff --git a/.env.example b/.env.example index a5e8daa..4ac4c30 100644 --- a/.env.example +++ b/.env.example @@ -201,6 +201,10 @@ FILE_CLEANUP_ENABLED=true FILE_ORPHAN_RETENTION_DAYS=7 FILE_EXPIRED_RETENTION_DAYS=1 FILE_DEFAULT_EXPIRY_HOURS=720 +# Comma-separated allowlist of MIME types accepted at the upload boundary. +# Requests with any other MIME type are rejected with 400 before storage. +# Default covers images, documents, video, audio and archives. +FILE_ALLOWED_MIME_TYPES=image/jpeg,image/png,image/gif,image/webp,image/svg+xml,image/tiff,image/bmp,image/avif,application/pdf,application/msword,application/vnd.openxmlformats-officedocument.wordprocessingml.document,application/vnd.ms-excel,application/vnd.openxmlformats-officedocument.spreadsheetml.sheet,application/vnd.ms-powerpoint,application/vnd.openxmlformats-officedocument.presentationml.presentation,text/plain,text/csv,application/json,application/xml,video/mp4,video/webm,video/ogg,video/quicktime,video/x-msvideo,video/x-matroska,audio/mpeg,audio/wav,audio/ogg,audio/webm,audio/aac,audio/flac,audio/mp4,application/zip,application/x-tar,application/gzip,application/x-rar-compressed,application/x-7z-compressed,application/x-bzip2 # S3 Configuration (when FILE_STORAGE_BACKEND=s3) S3_BUCKET=alian-structure-files diff --git a/.github/workflows/build-check.yml b/.github/workflows/build-check.yml index 16cfc02..4dd7ee0 100644 --- a/.github/workflows/build-check.yml +++ b/.github/workflows/build-check.yml @@ -159,5 +159,12 @@ jobs: uses: actions/upload-artifact@v4 with: name: typescript-client - path: client/ + path: sdks/typescript + retention-days: 30 + + - name: Upload Python client artifact + uses: actions/upload-artifact@v4 + with: + name: python-client + path: sdks/python retention-days: 30 diff --git a/src/infrastructure/file-upload/file-upload.controller.ts b/src/infrastructure/file-upload/file-upload.controller.ts index a455b1e..27d8ba4 100644 --- a/src/infrastructure/file-upload/file-upload.controller.ts +++ b/src/infrastructure/file-upload/file-upload.controller.ts @@ -39,6 +39,59 @@ import { FileSearchDto, FileCleanupDto, } from "./dto/file-upload.dto"; +import { CurrentUser } from "src/core/auth/decorators/current-user.decorator"; +import { AuthUser } from "src/core/auth/strategies/interfaces/auth-strategy.interface"; +import { BadRequestException, ConfigService } from "@nestjs/common"; + +/** + * Default MIME type allowlist used when FILE_ALLOWED_MIME_TYPES is not set. + * Mirrors the FileValidationService default categories. + */ +const DEFAULT_ALLOWED_MIME_TYPES = [ + // Images + "image/jpeg", + "image/png", + "image/gif", + "image/webp", + "image/svg+xml", + "image/tiff", + "image/bmp", + "image/avif", + // Documents + "application/pdf", + "application/msword", + "application/vnd.openxmlformats-officedocument.wordprocessingml.document", + "application/vnd.ms-excel", + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + "application/vnd.ms-powerpoint", + "application/vnd.openxmlformats-officedocument.presentationml.presentation", + "text/plain", + "text/csv", + "application/json", + "application/xml", + // Video + "video/mp4", + "video/webm", + "video/ogg", + "video/quicktime", + "video/x-msvideo", + "video/x-matroska", + // Audio + "audio/mpeg", + "audio/wav", + "audio/ogg", + "audio/webm", + "audio/aac", + "audio/flac", + "audio/mp4", + // Archives + "application/zip", + "application/x-tar", + "application/gzip", + "application/x-rar-compressed", + "application/x-7z-compressed", + "application/x-bzip2", +]; @ApiTags("Files") @ApiBearerAuth() @@ -51,8 +104,25 @@ export class FileUploadController { private readonly cleanupService: FileCleanupService, private readonly scanService: FileScanService, private readonly metadataService: FileMetadataService, + private readonly configService: ConfigService, ) {} + /** + * Resolves the MIME type allowlist from FILE_ALLOWED_MIME_TYPES env var. + * Falls back to the default list when unset or empty. + */ + private getAllowedMimeTypes(): Set { + const raw = this.configService.get("FILE_ALLOWED_MIME_TYPES"); + if (!raw) { + return new Set(DEFAULT_ALLOWED_MIME_TYPES); + } + const parsed = raw + .split(",") + .map((t) => t.trim().toLowerCase()) + .filter(Boolean); + return parsed.length > 0 ? new Set(parsed) : new Set(DEFAULT_ALLOWED_MIME_TYPES); + } + // ── Upload ──────────────────────────────────────────────────────── @Post("upload") @@ -62,6 +132,19 @@ export class FileUploadController { limits: { fileSize: 50 * 1024 * 1024, // 50MB }, + fileFilter: (req, file, cb) => { + const allowed = this.getAllowedMimeTypes(); + if (allowed.has(file.mimetype)) { + cb(null, true); + } else { + cb( + new BadRequestException( + `Unsupported file type: ${file.mimetype}`, + ), + false, + ); + } + }, }), ) @ApiOperation({ summary: "Upload a file" }) @@ -85,8 +168,12 @@ export class FileUploadController { async uploadFile( @UploadedFile() file: Express.Multer.File, @Body() dto: UploadFileDto, + @CurrentUser() user: AuthUser, ) { - const userId = "system"; + if (!user?.id) { + throw new BadRequestException("User identity not found in request"); + } + const userId = user.id; const result = await this.storageService.upload(file, userId, dto); return { @@ -118,8 +205,12 @@ export class FileUploadController { @Query("category") category?: string, @Query("limit") limit?: number, @Query("offset") offset?: number, + @CurrentUser() user: AuthUser, ) { - const userId = "system"; + if (!user?.id) { + throw new BadRequestException("User identity not found in request"); + } + const userId = user.id; const { files, total } = await this.storageService.search( { name, @@ -148,8 +239,11 @@ export class FileUploadController { @Get("search") @ApiOperation({ summary: "Search files with filters" }) - async searchFiles(@Query() dto: FileSearchDto) { - const userId = "system"; + async searchFiles(@Query() dto: FileSearchDto, @CurrentUser() user: AuthUser) { + if (!user?.id) { + throw new BadRequestException("User identity not found in request"); + } + const userId = user.id; const { files, total } = await this.storageService.search(dto, userId); return { @@ -262,8 +356,11 @@ export class FileUploadController { @Get("stats/overview") @ApiOperation({ summary: "Get file storage statistics" }) - async getStats() { - const userId = "system"; + async getStats(@CurrentUser() user: AuthUser) { + if (!user?.id) { + throw new BadRequestException("User identity not found in request"); + } + const userId = user.id; const stats = await this.storageService.getStorageStats(userId); return { success: true, stats }; } diff --git a/src/infrastructure/webhooks/webhook.controller.ts b/src/infrastructure/webhooks/webhook.controller.ts index a90b783..3b65602 100644 --- a/src/infrastructure/webhooks/webhook.controller.ts +++ b/src/infrastructure/webhooks/webhook.controller.ts @@ -27,6 +27,9 @@ import { UpdateWebhookSubscriptionDto, PublishWebhookEventDto, } from "./dto/webhook.dto"; +import { CurrentUser } from "src/core/auth/decorators/current-user.decorator"; +import { AuthUser } from "src/core/auth/strategies/interfaces/auth-strategy.interface"; +import { BadRequestException } from "@nestjs/common"; @ApiTags("Webhooks") @ApiBearerAuth() @@ -48,9 +51,12 @@ export class WebhookController { @ApiResponse({ status: 201, description: "Subscription created with signing key" }) async createSubscription( @Body() dto: CreateWebhookSubscriptionDto, + @CurrentUser() user: AuthUser, ) { - // TODO: extract userId from auth guard - const userId = "system"; + if (!user?.id) { + throw new BadRequestException("User identity not found in request"); + } + const userId = user.id; const sub = await this.subscriptionService.create(userId, dto); return { success: true, @@ -61,8 +67,11 @@ export class WebhookController { @Get("subscriptions") @ApiOperation({ summary: "List all webhook subscriptions" }) - async listSubscriptions() { - const userId = "system"; + async listSubscriptions(@CurrentUser() user: AuthUser) { + if (!user?.id) { + throw new BadRequestException("User identity not found in request"); + } + const userId = user.id; const subscriptions = await this.subscriptionService.findAll(userId); // Strip signing keys from response const safe = subscriptions.map(({ signingKey, ...rest }) => rest); @@ -72,8 +81,11 @@ export class WebhookController { @Get("subscriptions/:id") @ApiOperation({ summary: "Get a webhook subscription by ID" }) @ApiParam({ name: "id" }) - async getSubscription(@Param("id") id: string) { - const userId = "system"; + async getSubscription(@Param("id") id: string, @CurrentUser() user: AuthUser) { + if (!user?.id) { + throw new BadRequestException("User identity not found in request"); + } + const userId = user.id; const sub = await this.subscriptionService.findOne(id, userId); const { signingKey, ...rest } = sub; return { success: true, subscription: rest }; @@ -85,8 +97,12 @@ export class WebhookController { async updateSubscription( @Param("id") id: string, @Body() dto: UpdateWebhookSubscriptionDto, + @CurrentUser() user: AuthUser, ) { - const userId = "system"; + if (!user?.id) { + throw new BadRequestException("User identity not found in request"); + } + const userId = user.id; const sub = await this.subscriptionService.update(id, userId, dto); const { signingKey, ...rest } = sub; return { success: true, subscription: rest }; @@ -96,8 +112,11 @@ export class WebhookController { @HttpCode(HttpStatus.OK) @ApiOperation({ summary: "Disable a webhook subscription" }) @ApiParam({ name: "id" }) - async deleteSubscription(@Param("id") id: string) { - const userId = "system"; + async deleteSubscription(@Param("id") id: string, @CurrentUser() user: AuthUser) { + if (!user?.id) { + throw new BadRequestException("User identity not found in request"); + } + const userId = user.id; await this.subscriptionService.remove(id, userId); return { success: true, message: "Subscription disabled" }; } @@ -106,8 +125,11 @@ export class WebhookController { @HttpCode(HttpStatus.OK) @ApiOperation({ summary: "Rotate the signing key for a subscription" }) @ApiParam({ name: "id" }) - async rotateKey(@Param("id") id: string) { - const userId = "system"; + async rotateKey(@Param("id") id: string, @CurrentUser() user: AuthUser) { + if (!user?.id) { + throw new BadRequestException("User identity not found in request"); + } + const userId = user.id; const result = await this.subscriptionService.rotateKey(id, userId); return { success: true, @@ -123,8 +145,12 @@ export class WebhookController { async toggleStatus( @Param("id") id: string, @Query("status") status: "active" | "paused" | "disabled", + @CurrentUser() user: AuthUser, ) { - const userId = "system"; + if (!user?.id) { + throw new BadRequestException("User identity not found in request"); + } + const userId = user.id; const sub = await this.subscriptionService.toggleStatus(id, userId, status as any); const { signingKey, ...rest } = sub; return { success: true, subscription: rest }; @@ -192,8 +218,12 @@ export class WebhookController { async listDeadLetters( @Query("limit") limit?: number, @Query("offset") offset?: number, + @CurrentUser() user: AuthUser, ) { - const userId = "system"; + if (!user?.id) { + throw new BadRequestException("User identity not found in request"); + } + const userId = user.id; const deadLetters = await this.deliveryService.getDeadLetters( userId, limit || 50, From 177b29f917d74e33b1dc9e65a2aaeac8cd07f8e0 Mon Sep 17 00:00:00 2001 From: Memplethee Lab Date: Sun, 11 Oct 2026 00:29:08 +0100 Subject: [PATCH 2/4] fix(types): resolve TypeScript compilation errors - Import ConfigService from @nestjs/config instead of @nestjs/common - Add null guard for file object in fileFilter callback - Reorder parameters so required @CurrentUser comes before optional query params --- src/infrastructure/file-upload/file-upload.controller.ts | 9 +++++++-- src/infrastructure/webhooks/webhook.controller.ts | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/infrastructure/file-upload/file-upload.controller.ts b/src/infrastructure/file-upload/file-upload.controller.ts index 27d8ba4..c7e308d 100644 --- a/src/infrastructure/file-upload/file-upload.controller.ts +++ b/src/infrastructure/file-upload/file-upload.controller.ts @@ -41,7 +41,8 @@ import { } from "./dto/file-upload.dto"; import { CurrentUser } from "src/core/auth/decorators/current-user.decorator"; import { AuthUser } from "src/core/auth/strategies/interfaces/auth-strategy.interface"; -import { BadRequestException, ConfigService } from "@nestjs/common"; +import { BadRequestException } from "@nestjs/common"; +import { ConfigService } from "@nestjs/config"; /** * Default MIME type allowlist used when FILE_ALLOWED_MIME_TYPES is not set. @@ -133,6 +134,10 @@ export class FileUploadController { fileSize: 50 * 1024 * 1024, // 50MB }, fileFilter: (req, file, cb) => { + if (!file) { + cb(new BadRequestException("No file provided"), false); + return; + } const allowed = this.getAllowedMimeTypes(); if (allowed.has(file.mimetype)) { cb(null, true); @@ -201,11 +206,11 @@ export class FileUploadController { @ApiQuery({ name: "limit", required: false }) @ApiQuery({ name: "offset", required: false }) async listFiles( + @CurrentUser() user: AuthUser, @Query("name") name?: string, @Query("category") category?: string, @Query("limit") limit?: number, @Query("offset") offset?: number, - @CurrentUser() user: AuthUser, ) { if (!user?.id) { throw new BadRequestException("User identity not found in request"); diff --git a/src/infrastructure/webhooks/webhook.controller.ts b/src/infrastructure/webhooks/webhook.controller.ts index 3b65602..a6ab69f 100644 --- a/src/infrastructure/webhooks/webhook.controller.ts +++ b/src/infrastructure/webhooks/webhook.controller.ts @@ -216,9 +216,9 @@ export class WebhookController { @ApiQuery({ name: "limit", required: false }) @ApiQuery({ name: "offset", required: false }) async listDeadLetters( + @CurrentUser() user: AuthUser, @Query("limit") limit?: number, @Query("offset") offset?: number, - @CurrentUser() user: AuthUser, ) { if (!user?.id) { throw new BadRequestException("User identity not found in request"); From 717eb6cf339efa7822a0907c4c0525c5decb3ff3 Mon Sep 17 00:00:00 2001 From: Memplethee Lab Date: Sun, 11 Oct 2026 00:33:51 +0100 Subject: [PATCH 3/4] fix(types): use non-null assertion for file in fileFilter after guard TypeScript control flow cannot narrow file to non-null across cb() call boundaries. The guard clause already ensures file is non-null, so the assertion is safe. --- src/infrastructure/file-upload/file-upload.controller.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/infrastructure/file-upload/file-upload.controller.ts b/src/infrastructure/file-upload/file-upload.controller.ts index c7e308d..8cb67b5 100644 --- a/src/infrastructure/file-upload/file-upload.controller.ts +++ b/src/infrastructure/file-upload/file-upload.controller.ts @@ -139,12 +139,12 @@ export class FileUploadController { return; } const allowed = this.getAllowedMimeTypes(); - if (allowed.has(file.mimetype)) { + if (allowed.has(file!.mimetype)) { cb(null, true); } else { cb( new BadRequestException( - `Unsupported file type: ${file.mimetype}`, + `Unsupported file type: ${file!.mimetype}`, ), false, ); From 8b376370eac61b588353c4534a48b1edbc7df689 Mon Sep 17 00:00:00 2001 From: Memplethee Lab Date: Sun, 11 Oct 2026 00:38:51 +0100 Subject: [PATCH 4/4] fix(types): extract file to local const for proper type narrowing TypeScript could not narrow file to non-null across cb() boundaries. Extracting to a local const f ensures proper control flow narrowing. --- src/infrastructure/file-upload/file-upload.controller.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/infrastructure/file-upload/file-upload.controller.ts b/src/infrastructure/file-upload/file-upload.controller.ts index 8cb67b5..5fc797d 100644 --- a/src/infrastructure/file-upload/file-upload.controller.ts +++ b/src/infrastructure/file-upload/file-upload.controller.ts @@ -134,17 +134,18 @@ export class FileUploadController { fileSize: 50 * 1024 * 1024, // 50MB }, fileFilter: (req, file, cb) => { - if (!file) { + const f = file; + if (!f) { cb(new BadRequestException("No file provided"), false); return; } const allowed = this.getAllowedMimeTypes(); - if (allowed.has(file!.mimetype)) { + if (allowed.has(f.mimetype)) { cb(null, true); } else { cb( new BadRequestException( - `Unsupported file type: ${file!.mimetype}`, + `Unsupported file type: ${f.mimetype}`, ), false, );