From 135c636516ddfd67ceeb45ed487213a2941f26d7 Mon Sep 17 00:00:00 2001 From: Till JS Date: Thu, 19 Mar 2026 11:12:20 +0100 Subject: [PATCH] fix(calendar,contacts,todo): second round of pre-launch audit fixes Calendar: validate startTime < endTime on event create/update, verify share invitation recipient matches accepting user, add @MaxLength on search DTO, use ConfigService for FRONTEND_URL, fix Docker default port. Contacts: replace Error with NotFoundException in tag controller, verify contact ownership before tag operations, add @ArrayMaxSize(100) on batch DTOs, add unique constraint on contact tags (userId, name), add @MaxLength(10000) on note content, reorder photo upload for safety. Todo: add self-referencing FK on parentTaskId with cascade delete, validate parent task ownership on create, add @Min/@Max on query limit/offset, add @MaxLength(500) on search, add error handling to quick add in web app. Co-Authored-By: Claude Opus 4.6 --- .../backend/src/event/dto/query-events.dto.ts | 2 ++ .../backend/src/event/event.service.spec.ts | 6 ++--- .../apps/backend/src/event/event.service.ts | 23 +++++++++++++++- .../backend/src/share/share.controller.ts | 2 +- .../backend/src/share/share.service.spec.ts | 19 +++++++++----- .../apps/backend/src/share/share.service.ts | 17 +++++++++--- apps/calendar/apps/web/Dockerfile | 2 +- .../backend/src/batch/batch.controller.ts | 11 +++++++- .../apps/backend/src/db/schema/tags.schema.ts | 3 ++- .../apps/backend/src/note/note.controller.ts | 4 ++- .../apps/backend/src/photo/photo.service.ts | 26 +++++++++---------- .../apps/backend/src/tag/tag.controller.ts | 21 ++++++++++++--- .../apps/backend/src/tag/tag.module.ts | 2 ++ .../backend/src/db/schema/tasks.schema.ts | 5 ++++ .../backend/src/task/dto/query-tasks.dto.ts | 7 +++++ .../apps/backend/src/task/task.service.ts | 5 ++++ .../apps/web/src/routes/(app)/+layout.svelte | 22 +++++++++------- 17 files changed, 133 insertions(+), 44 deletions(-) diff --git a/apps/calendar/apps/backend/src/event/dto/query-events.dto.ts b/apps/calendar/apps/backend/src/event/dto/query-events.dto.ts index a4610b248..543cdf954 100644 --- a/apps/calendar/apps/backend/src/event/dto/query-events.dto.ts +++ b/apps/calendar/apps/backend/src/event/dto/query-events.dto.ts @@ -7,6 +7,7 @@ import { IsInt, Min, Max, + MaxLength, } from 'class-validator'; import { Transform, Type } from 'class-transformer'; @@ -32,6 +33,7 @@ export class QueryEventsDto { @IsOptional() @IsString() + @MaxLength(500) search?: string; @IsOptional() diff --git a/apps/calendar/apps/backend/src/event/event.service.spec.ts b/apps/calendar/apps/backend/src/event/event.service.spec.ts index bc6a62285..188ca0f74 100644 --- a/apps/calendar/apps/backend/src/event/event.service.spec.ts +++ b/apps/calendar/apps/backend/src/event/event.service.spec.ts @@ -152,7 +152,7 @@ describe('EventService', () => { calendarId: calendar.id, title: 'New Event', startTime: new Date().toISOString(), - endTime: new Date().toISOString(), + endTime: new Date(Date.now() + 3600000).toISOString(), }); expect(result).toEqual(newEvent); @@ -169,7 +169,7 @@ describe('EventService', () => { const result = await service.create(TEST_USER_ID, { title: 'New Event', startTime: new Date().toISOString(), - endTime: new Date().toISOString(), + endTime: new Date(Date.now() + 3600000).toISOString(), }); expect(result).toEqual(newEvent); @@ -188,7 +188,7 @@ describe('EventService', () => { calendarId: calendar.id, title: 'New Event', startTime: new Date().toISOString(), - endTime: new Date().toISOString(), + endTime: new Date(Date.now() + 3600000).toISOString(), tagIds, }); diff --git a/apps/calendar/apps/backend/src/event/event.service.ts b/apps/calendar/apps/backend/src/event/event.service.ts index 366c8ae8d..62fbf9afd 100644 --- a/apps/calendar/apps/backend/src/event/event.service.ts +++ b/apps/calendar/apps/backend/src/event/event.service.ts @@ -1,4 +1,10 @@ -import { Injectable, Inject, NotFoundException, ForbiddenException } from '@nestjs/common'; +import { + Injectable, + Inject, + NotFoundException, + ForbiddenException, + BadRequestException, +} from '@nestjs/common'; import { eq, and, gte, lte, inArray, or, ilike } from 'drizzle-orm'; import { DATABASE_CONNECTION } from '../db/database.module'; import { Database } from '../db/connection'; @@ -95,7 +101,15 @@ export class EventService { }); } + private validateTimeRange(startTime: string, endTime: string): void { + if (new Date(startTime) >= new Date(endTime)) { + throw new BadRequestException('startTime must be before endTime'); + } + } + async create(userId: string, dto: CreateEventDto): Promise { + this.validateTimeRange(dto.startTime, dto.endTime); + let calendarId = dto.calendarId; let calendar; @@ -138,6 +152,13 @@ export class EventService { async update(id: string, userId: string, dto: UpdateEventDto): Promise { const existingEvent = await this.findByIdOrThrow(id, userId); + // Validate time range when either startTime or endTime is provided + const effectiveStart = dto.startTime ?? existingEvent.startTime.toISOString(); + const effectiveEnd = dto.endTime ?? existingEvent.endTime.toISOString(); + if (dto.startTime || dto.endTime) { + this.validateTimeRange(effectiveStart, effectiveEnd); + } + // If changing calendar, verify user owns the new calendar if (dto.calendarId && dto.calendarId !== existingEvent.calendarId) { await this.calendarService.findByIdOrThrow(dto.calendarId, userId); diff --git a/apps/calendar/apps/backend/src/share/share.controller.ts b/apps/calendar/apps/backend/src/share/share.controller.ts index 6b5ba3610..9e01b29cc 100644 --- a/apps/calendar/apps/backend/src/share/share.controller.ts +++ b/apps/calendar/apps/backend/src/share/share.controller.ts @@ -57,7 +57,7 @@ export class ShareController { @Post('shares/:shareId/accept') async acceptInvitation(@CurrentUser() user: CurrentUserData, @Param('shareId') shareId: string) { - const share = await this.shareService.acceptInvitation(shareId, user.userId); + const share = await this.shareService.acceptInvitation(shareId, user.userId, user.email); return { share }; } diff --git a/apps/calendar/apps/backend/src/share/share.service.spec.ts b/apps/calendar/apps/backend/src/share/share.service.spec.ts index 9d745cade..6e9902d08 100644 --- a/apps/calendar/apps/backend/src/share/share.service.spec.ts +++ b/apps/calendar/apps/backend/src/share/share.service.spec.ts @@ -1,5 +1,6 @@ import { Test, TestingModule } from '@nestjs/testing'; import { NotFoundException, ForbiddenException } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; import { ShareService } from './share.service'; import { CalendarService } from '../calendar/calendar.service'; import { EmailService } from '../email/email.service'; @@ -53,6 +54,10 @@ describe('ShareService', () => { provide: EmailService, useValue: mockEmailService, }, + { + provide: ConfigService, + useValue: { get: jest.fn().mockReturnValue('http://localhost:5179') }, + }, ], }).compile(); @@ -241,7 +246,7 @@ describe('ShareService', () => { mockDb.where.mockResolvedValueOnce([share]); mockDb.returning.mockResolvedValueOnce([acceptedShare]); - const result = await service.acceptInvitation(share.id, TEST_USER_ID); + const result = await service.acceptInvitation(share.id, TEST_USER_ID, 'shared@example.com'); expect(result.status).toBe('accepted'); expect(result.sharedWithUserId).toBe(TEST_USER_ID); @@ -250,18 +255,18 @@ describe('ShareService', () => { it('should throw NotFoundException when invitation not found', async () => { mockDb.where.mockResolvedValueOnce([]); - await expect(service.acceptInvitation('non-existent-id', TEST_USER_ID)).rejects.toThrow( - NotFoundException - ); + await expect( + service.acceptInvitation('non-existent-id', TEST_USER_ID, 'shared@example.com') + ).rejects.toThrow(NotFoundException); }); it('should throw ForbiddenException when invitation already processed', async () => { const share = createMockCalendarShare({ status: 'accepted' }); mockDb.where.mockResolvedValueOnce([share]); - await expect(service.acceptInvitation(share.id, TEST_USER_ID)).rejects.toThrow( - ForbiddenException - ); + await expect( + service.acceptInvitation(share.id, TEST_USER_ID, 'shared@example.com') + ).rejects.toThrow(ForbiddenException); }); }); diff --git a/apps/calendar/apps/backend/src/share/share.service.ts b/apps/calendar/apps/backend/src/share/share.service.ts index 31b5eed99..de5afbd35 100644 --- a/apps/calendar/apps/backend/src/share/share.service.ts +++ b/apps/calendar/apps/backend/src/share/share.service.ts @@ -1,4 +1,5 @@ import { Injectable, Inject, NotFoundException, ForbiddenException, Logger } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; import { eq, and, or } from 'drizzle-orm'; import { randomBytes } from 'crypto'; import { DATABASE_CONNECTION } from '../db/database.module'; @@ -19,7 +20,8 @@ export class ShareService { constructor( @Inject(DATABASE_CONNECTION) private db: Database, private calendarService: CalendarService, - private emailService: EmailService + private emailService: EmailService, + private configService: ConfigService ) {} async findByCalendar(calendarId: string, userId: string): Promise { @@ -76,7 +78,7 @@ export class ShareService { // Send invitation email if sharing with specific email if (dto.email && !dto.createLink) { const inviterName = inviterEmail.split('@')[0]; - const baseUrl = process.env.FRONTEND_URL || 'http://localhost:5179'; + const baseUrl = this.configService.get('FRONTEND_URL', 'http://localhost:5179'); const acceptUrl = `${baseUrl}/shares/${created.id}/accept`; try { @@ -131,7 +133,7 @@ export class ShareService { await this.db.delete(calendarShares).where(eq(calendarShares.id, id)); } - async acceptInvitation(shareId: string, userId: string): Promise { + async acceptInvitation(shareId: string, userId: string, email: string): Promise { const share = await this.findById(shareId); if (!share) { throw new NotFoundException(`Invitation not found`); @@ -141,6 +143,15 @@ export class ShareService { throw new ForbiddenException('Invitation has already been processed'); } + // Validate that the accepting user matches the invitation target + const matchesUserId = share.sharedWithUserId && share.sharedWithUserId === userId; + const matchesEmail = share.sharedWithEmail && share.sharedWithEmail === email; + if (share.sharedWithUserId || share.sharedWithEmail) { + if (!matchesUserId && !matchesEmail) { + throw new ForbiddenException('You are not the intended recipient of this invitation'); + } + } + const [updated] = await this.db .update(calendarShares) .set({ diff --git a/apps/calendar/apps/web/Dockerfile b/apps/calendar/apps/web/Dockerfile index dc7554079..82c77d02b 100644 --- a/apps/calendar/apps/web/Dockerfile +++ b/apps/calendar/apps/web/Dockerfile @@ -2,7 +2,7 @@ FROM node:20-alpine AS builder # Build arguments for SvelteKit static env vars -ARG PUBLIC_BACKEND_URL=http://calendar-backend:3016 +ARG PUBLIC_BACKEND_URL=http://calendar-backend:3014 ARG PUBLIC_MANA_CORE_AUTH_URL=http://mana-core-auth:3001 # Set as environment variables for build diff --git a/apps/contacts/apps/backend/src/batch/batch.controller.ts b/apps/contacts/apps/backend/src/batch/batch.controller.ts index 80633add1..6c5eab7ed 100644 --- a/apps/contacts/apps/backend/src/batch/batch.controller.ts +++ b/apps/contacts/apps/backend/src/batch/batch.controller.ts @@ -1,12 +1,20 @@ import { Controller, Post, Body, UseGuards } from '@nestjs/common'; import { JwtAuthGuard, CurrentUser, CurrentUserData } from '@manacore/shared-nestjs-auth'; import { BatchService } from './batch.service'; -import { IsArray, IsString, IsBoolean, IsOptional, ArrayMinSize } from 'class-validator'; +import { + IsArray, + IsString, + IsBoolean, + IsOptional, + ArrayMinSize, + ArrayMaxSize, +} from 'class-validator'; class BatchContactIdsDto { @IsArray() @IsString({ each: true }) @ArrayMinSize(1) + @ArrayMaxSize(100) contactIds: string[]; } @@ -26,6 +34,7 @@ class BatchTagsDto extends BatchContactIdsDto { @IsArray() @IsString({ each: true }) @ArrayMinSize(1) + @ArrayMaxSize(100) tagIds: string[]; } diff --git a/apps/contacts/apps/backend/src/db/schema/tags.schema.ts b/apps/contacts/apps/backend/src/db/schema/tags.schema.ts index ef9d24a62..4dffa07fa 100644 --- a/apps/contacts/apps/backend/src/db/schema/tags.schema.ts +++ b/apps/contacts/apps/backend/src/db/schema/tags.schema.ts @@ -1,4 +1,4 @@ -import { pgTable, uuid, timestamp, varchar, primaryKey, index } from 'drizzle-orm/pg-core'; +import { pgTable, uuid, timestamp, varchar, primaryKey, index, unique } from 'drizzle-orm/pg-core'; import { contacts } from './contacts.schema'; export const contactTags = pgTable( @@ -12,6 +12,7 @@ export const contactTags = pgTable( }, (table) => ({ userIdx: index('contact_tags_user_idx').on(table.userId), + userNameUnique: unique('contact_tags_user_name_unique').on(table.userId, table.name), }) ); diff --git a/apps/contacts/apps/backend/src/note/note.controller.ts b/apps/contacts/apps/backend/src/note/note.controller.ts index 08e6b09ac..0bdbf8d2b 100644 --- a/apps/contacts/apps/backend/src/note/note.controller.ts +++ b/apps/contacts/apps/backend/src/note/note.controller.ts @@ -11,10 +11,11 @@ import { } from '@nestjs/common'; import { JwtAuthGuard, CurrentUser, CurrentUserData } from '@manacore/shared-nestjs-auth'; import { NoteService } from './note.service'; -import { IsString, IsOptional, IsBoolean } from 'class-validator'; +import { IsString, IsOptional, IsBoolean, MaxLength } from 'class-validator'; class CreateNoteDto { @IsString() + @MaxLength(10000) content!: string; @IsBoolean() @@ -25,6 +26,7 @@ class CreateNoteDto { class UpdateNoteDto { @IsString() @IsOptional() + @MaxLength(10000) content?: string; @IsBoolean() diff --git a/apps/contacts/apps/backend/src/photo/photo.service.ts b/apps/contacts/apps/backend/src/photo/photo.service.ts index 08dee06d2..f6c0ac11a 100644 --- a/apps/contacts/apps/backend/src/photo/photo.service.ts +++ b/apps/contacts/apps/backend/src/photo/photo.service.ts @@ -56,23 +56,11 @@ export class PhotoService { throw new BadRequestException('Contact not found'); } - // Delete old photo if exists - if (contact.photoUrl) { - try { - const oldKey = this.extractKeyFromUrl(contact.photoUrl); - if (oldKey) { - await this.storage.delete(oldKey); - } - } catch { - // Ignore deletion errors - } - } - // Generate unique key for the new photo const filename = `${contactId}.${extension}`; const key = generateUserFileKey(userId, filename); - // Upload to S3 + // Upload new photo to S3 first (safe: if this fails, nothing is lost) const contentType = getContentType(filename); await this.storage.upload(key, file.buffer, { contentType, @@ -89,6 +77,18 @@ export class PhotoService { .set({ photoUrl, updatedAt: new Date() }) .where(eq(contacts.id, contactId)); + // Delete old photo if exists (after successful upload + DB update) + if (contact.photoUrl) { + try { + const oldKey = this.extractKeyFromUrl(contact.photoUrl); + if (oldKey) { + await this.storage.delete(oldKey); + } + } catch { + // Ignore deletion errors - orphaned old file is harmless + } + } + return { photoUrl }; } diff --git a/apps/contacts/apps/backend/src/tag/tag.controller.ts b/apps/contacts/apps/backend/src/tag/tag.controller.ts index d77db0efa..b962b306e 100644 --- a/apps/contacts/apps/backend/src/tag/tag.controller.ts +++ b/apps/contacts/apps/backend/src/tag/tag.controller.ts @@ -8,9 +8,11 @@ import { Param, UseGuards, ParseUUIDPipe, + NotFoundException, } from '@nestjs/common'; import { JwtAuthGuard, CurrentUser, CurrentUserData } from '@manacore/shared-nestjs-auth'; import { TagService } from './tag.service'; +import { ContactService } from '../contact/contact.service'; import { IsString, IsOptional, MaxLength } from 'class-validator'; class CreateTagDto { @@ -39,7 +41,10 @@ class UpdateTagDto { @Controller('tags') @UseGuards(JwtAuthGuard) export class TagController { - constructor(private readonly tagService: TagService) {} + constructor( + private readonly tagService: TagService, + private readonly contactService: ContactService + ) {} @Get() async findAll(@CurrentUser() user: CurrentUserData) { @@ -81,7 +86,12 @@ export class TagController { // Verify tag belongs to user const tag = await this.tagService.findById(tagId, user.userId); if (!tag) { - throw new Error('Tag not found'); + throw new NotFoundException('Tag not found'); + } + // Verify contact belongs to user + const contact = await this.contactService.findById(contactId, user.userId); + if (!contact) { + throw new NotFoundException('Contact not found'); } await this.tagService.addTagToContact(contactId, tagId); return { success: true }; @@ -96,7 +106,12 @@ export class TagController { // Verify tag belongs to user const tag = await this.tagService.findById(tagId, user.userId); if (!tag) { - throw new Error('Tag not found'); + throw new NotFoundException('Tag not found'); + } + // Verify contact belongs to user + const contact = await this.contactService.findById(contactId, user.userId); + if (!contact) { + throw new NotFoundException('Contact not found'); } await this.tagService.removeTagFromContact(contactId, tagId); return { success: true }; diff --git a/apps/contacts/apps/backend/src/tag/tag.module.ts b/apps/contacts/apps/backend/src/tag/tag.module.ts index 223049f55..edf883146 100644 --- a/apps/contacts/apps/backend/src/tag/tag.module.ts +++ b/apps/contacts/apps/backend/src/tag/tag.module.ts @@ -1,8 +1,10 @@ import { Module } from '@nestjs/common'; import { TagController } from './tag.controller'; import { TagService } from './tag.service'; +import { ContactModule } from '../contact/contact.module'; @Module({ + imports: [ContactModule], controllers: [TagController], providers: [TagService], exports: [TagService], diff --git a/apps/todo/apps/backend/src/db/schema/tasks.schema.ts b/apps/todo/apps/backend/src/db/schema/tasks.schema.ts index 5969058d9..2f94b9b1e 100644 --- a/apps/todo/apps/backend/src/db/schema/tasks.schema.ts +++ b/apps/todo/apps/backend/src/db/schema/tasks.schema.ts @@ -8,6 +8,7 @@ import { integer, jsonb, index, + foreignKey, } from 'drizzle-orm/pg-core'; import { projects } from './projects.schema'; import { kanbanColumns } from './kanban-columns.schema'; @@ -110,6 +111,10 @@ export const tasks = pgTable( table.userId, table.scheduledDate ), + parentTaskFk: foreignKey({ + columns: [table.parentTaskId], + foreignColumns: [table.id], + }).onDelete('cascade'), }) ); diff --git a/apps/todo/apps/backend/src/task/dto/query-tasks.dto.ts b/apps/todo/apps/backend/src/task/dto/query-tasks.dto.ts index cea00a0a3..7f0dc3317 100644 --- a/apps/todo/apps/backend/src/task/dto/query-tasks.dto.ts +++ b/apps/todo/apps/backend/src/task/dto/query-tasks.dto.ts @@ -6,6 +6,9 @@ import { IsString, IsNumber, IsDateString, + Min, + Max, + MaxLength, } from 'class-validator'; import { Transform } from 'class-transformer'; import type { TaskPriority, TaskStatus } from '../../db/schema/tasks.schema'; @@ -46,6 +49,7 @@ export class QueryTasksDto { @IsOptional() @IsString() + @MaxLength(500) search?: string; @IsOptional() @@ -59,10 +63,13 @@ export class QueryTasksDto { @IsOptional() @Transform(({ value }) => parseInt(value, 10)) @IsNumber() + @Min(0) + @Max(100) limit?: number; @IsOptional() @Transform(({ value }) => parseInt(value, 10)) @IsNumber() + @Min(0) offset?: number; } diff --git a/apps/todo/apps/backend/src/task/task.service.ts b/apps/todo/apps/backend/src/task/task.service.ts index 6df72c534..843c7a877 100644 --- a/apps/todo/apps/backend/src/task/task.service.ts +++ b/apps/todo/apps/backend/src/task/task.service.ts @@ -106,6 +106,11 @@ export class TaskService { await this.projectService.findByIdOrThrow(dto.projectId, userId); } + // Verify parent task belongs to user if provided + if (dto.parentTaskId) { + await this.findByIdOrThrow(dto.parentTaskId, userId); + } + // Get the highest order value for the project const existingTasks = await this.findAll(userId, { projectId: dto.projectId ?? undefined }); const maxOrder = existingTasks.reduce((max, t) => Math.max(max, t.order ?? 0), -1); diff --git a/apps/todo/apps/web/src/routes/(app)/+layout.svelte b/apps/todo/apps/web/src/routes/(app)/+layout.svelte index 78e7d733d..5abc8636f 100644 --- a/apps/todo/apps/web/src/routes/(app)/+layout.svelte +++ b/apps/todo/apps/web/src/routes/(app)/+layout.svelte @@ -89,16 +89,20 @@ async function handleCreate(query: string): Promise { if (!query.trim()) return; - const parsed = parseTaskInput(query); - const resolved = resolveTaskIds(parsed, projectsStore.projects, labelsStore.labels); + try { + const parsed = parseTaskInput(query); + const resolved = resolveTaskIds(parsed, projectsStore.projects, labelsStore.labels); - const result = await tasksStore.createTask({ - title: resolved.title, - dueDate: resolved.dueDate, - priority: resolved.priority, - projectId: resolved.projectId, - labelIds: resolved.labelIds, - }); + await tasksStore.createTask({ + title: resolved.title, + dueDate: resolved.dueDate, + priority: resolved.priority, + projectId: resolved.projectId, + labelIds: resolved.labelIds, + }); + } catch (error) { + console.error('Failed to create task:', error); + } } // PillNav collapsed state (controlled by FAB)