From c6b48d8f95cb9e6452029507e0faf13b8b9916a5 Mon Sep 17 00:00:00 2001 From: Till-JS <101404291+Till-JS@users.noreply.github.com> Date: Wed, 10 Dec 2025 14:25:24 +0100 Subject: [PATCH] fix(todo): add input validation, N+1 fix, and RRULE bounds checking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security & Validation: - Add @IsNotEmpty and @MinLength(1) validators to prevent empty inputs - CreateTaskDto.title - CreateLabelDto.name - CreateProjectDto.name - Add German error messages for validation failures Performance: - Fix N+1 query in network.service.ts getGraph() - Batch load all task-label relationships in single query - Reduces queries from O(n) to O(1) for label fetching Security: - Add validateRRule() to prevent DoS via malicious recurrence rules - Reject rules > 500 chars - Reject rules with > 5000 occurrences in 10 years - Prevents hourly/minutely abuse while allowing daily tasks Cleanup: - Remove debug console.log from tasks.svelte.ts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../backend/src/label/dto/create-label.dto.ts | 6 ++- .../backend/src/network/network.service.ts | 29 ++++++++---- .../src/project/dto/create-project.dto.ts | 14 +++++- .../backend/src/task/dto/create-task.dto.ts | 6 ++- .../apps/backend/src/task/task.service.ts | 42 +++++++++++++++++ .../apps/web/src/lib/stores/tasks.svelte.ts | 47 ++++++++++++++++++- 6 files changed, 129 insertions(+), 15 deletions(-) diff --git a/apps/todo/apps/backend/src/label/dto/create-label.dto.ts b/apps/todo/apps/backend/src/label/dto/create-label.dto.ts index 5efeb87f3..920537e9a 100644 --- a/apps/todo/apps/backend/src/label/dto/create-label.dto.ts +++ b/apps/todo/apps/backend/src/label/dto/create-label.dto.ts @@ -1,8 +1,10 @@ -import { IsString, IsOptional, MaxLength } from 'class-validator'; +import { IsString, IsOptional, MaxLength, MinLength, IsNotEmpty } from 'class-validator'; export class CreateLabelDto { @IsString() - @MaxLength(100) + @IsNotEmpty({ message: 'Name darf nicht leer sein' }) + @MinLength(1, { message: 'Name muss mindestens 1 Zeichen haben' }) + @MaxLength(100, { message: 'Name darf maximal 100 Zeichen haben' }) name: string; @IsOptional() diff --git a/apps/todo/apps/backend/src/network/network.service.ts b/apps/todo/apps/backend/src/network/network.service.ts index 8b5d26524..1c67c19f0 100644 --- a/apps/todo/apps/backend/src/network/network.service.ts +++ b/apps/todo/apps/backend/src/network/network.service.ts @@ -1,5 +1,5 @@ import { Injectable, Inject } from '@nestjs/common'; -import { eq } from 'drizzle-orm'; +import { eq, inArray } from 'drizzle-orm'; import { DATABASE_CONNECTION } from '../db/database.module'; import { Database } from '../db/connection'; import { tasks, labels, taskLabels, projects } from '../db/schema'; @@ -54,21 +54,32 @@ export class NetworkService { const projectMap = new Map(userProjects.map((p) => [p.id, p.name])); - // 3. Get labels for each task + // 3. Get all labels for all tasks in a single batch query (fix N+1) + const taskIds = userTasks.map(({ task }) => task.id); const taskLabelsMap = new Map(); - for (const { task } of userTasks) { - const taskLabelRows = await this.db + if (taskIds.length > 0) { + const allTaskLabels = await this.db .select({ - id: labels.id, - name: labels.name, - color: labels.color, + taskId: taskLabels.taskId, + labelId: labels.id, + labelName: labels.name, + labelColor: labels.color, }) .from(taskLabels) .innerJoin(labels, eq(taskLabels.labelId, labels.id)) - .where(eq(taskLabels.taskId, task.id)); + .where(inArray(taskLabels.taskId, taskIds)); - taskLabelsMap.set(task.id, taskLabelRows); + // Group labels by taskId + for (const row of allTaskLabels) { + const existing = taskLabelsMap.get(row.taskId) || []; + existing.push({ + id: row.labelId, + name: row.labelName, + color: row.labelColor, + }); + taskLabelsMap.set(row.taskId, existing); + } } // 4. Filter tasks that have at least one label diff --git a/apps/todo/apps/backend/src/project/dto/create-project.dto.ts b/apps/todo/apps/backend/src/project/dto/create-project.dto.ts index cf13948b0..266bd7f00 100644 --- a/apps/todo/apps/backend/src/project/dto/create-project.dto.ts +++ b/apps/todo/apps/backend/src/project/dto/create-project.dto.ts @@ -1,9 +1,19 @@ -import { IsString, IsOptional, IsBoolean, MaxLength, IsObject } from 'class-validator'; +import { + IsString, + IsOptional, + IsBoolean, + MaxLength, + MinLength, + IsObject, + IsNotEmpty, +} from 'class-validator'; import type { ProjectSettings } from '../../db/schema/projects.schema'; export class CreateProjectDto { @IsString() - @MaxLength(255) + @IsNotEmpty({ message: 'Name darf nicht leer sein' }) + @MinLength(1, { message: 'Name muss mindestens 1 Zeichen haben' }) + @MaxLength(255, { message: 'Name darf maximal 255 Zeichen haben' }) name: string; @IsOptional() diff --git a/apps/todo/apps/backend/src/task/dto/create-task.dto.ts b/apps/todo/apps/backend/src/task/dto/create-task.dto.ts index 32e7fdd22..f266923d4 100644 --- a/apps/todo/apps/backend/src/task/dto/create-task.dto.ts +++ b/apps/todo/apps/backend/src/task/dto/create-task.dto.ts @@ -6,13 +6,17 @@ import { IsArray, IsObject, MaxLength, + MinLength, IsDateString, + IsNotEmpty, } from 'class-validator'; import type { TaskPriority, Subtask, TaskMetadata } from '../../db/schema/tasks.schema'; export class CreateTaskDto { @IsString() - @MaxLength(500) + @IsNotEmpty({ message: 'Titel darf nicht leer sein' }) + @MinLength(1, { message: 'Titel muss mindestens 1 Zeichen haben' }) + @MaxLength(500, { message: 'Titel darf maximal 500 Zeichen haben' }) title: string; @IsOptional() diff --git a/apps/todo/apps/backend/src/task/task.service.ts b/apps/todo/apps/backend/src/task/task.service.ts index 5dfc7d825..cbb420d1e 100644 --- a/apps/todo/apps/backend/src/task/task.service.ts +++ b/apps/todo/apps/backend/src/task/task.service.ts @@ -219,6 +219,42 @@ export class TaskService { }); } + /** + * Validates an RRULE string to prevent abuse (DoS, excessive occurrences). + * Returns true if valid, false if invalid or too complex. + */ + private validateRRule(rruleString: string): boolean { + // Basic length check + if (!rruleString || rruleString.length > 500) { + return false; + } + + try { + const rule = rrulestr(rruleString); + + // Get occurrences for the next 10 years with a limit + // Daily tasks = ~3650/10yrs, hourly would be ~87600 (reject) + const maxOccurrences = 5000; + const tenYearsFromNow = new Date(); + tenYearsFromNow.setFullYear(tenYearsFromNow.getFullYear() + 10); + + const occurrences = rule.between(new Date(), tenYearsFromNow, true, (_, count) => { + // Stop iteration early if we exceed limit + return count < maxOccurrences; + }); + + // Reject if too many occurrences (prevents hourly/minutely abuse) + if (occurrences.length >= maxOccurrences) { + console.warn(`RRULE rejected: too many occurrences (${occurrences.length})`); + return false; + } + + return true; + } catch { + return false; + } + } + /** * Creates the next occurrence of a recurring task based on its RRULE. * Returns the newly created task, or null if no more occurrences should be created. @@ -229,6 +265,12 @@ export class TaskService { ): Promise { if (!task.recurrenceRule) return null; + // Validate RRULE complexity before parsing + if (!this.validateRRule(task.recurrenceRule)) { + console.warn(`Invalid or too complex RRULE for task ${task.id}`); + return null; + } + try { // Parse the RRULE string const rule = rrulestr(task.recurrenceRule); diff --git a/apps/todo/apps/web/src/lib/stores/tasks.svelte.ts b/apps/todo/apps/web/src/lib/stores/tasks.svelte.ts index 483b3a1c1..435057133 100644 --- a/apps/todo/apps/web/src/lib/stores/tasks.svelte.ts +++ b/apps/todo/apps/web/src/lib/stores/tasks.svelte.ts @@ -121,7 +121,6 @@ export const tasksStore = { try { // Fetch all tasks without filter - let frontend handle filtering const allTasks = await tasksApi.getTasks({}); - console.log('API response - all tasks:', allTasks.length); tasks = allTasks; } catch (e) { error = e instanceof Error ? e.message : 'Failed to fetch all tasks'; @@ -239,6 +238,52 @@ export const tasksStore = { } }, + /** + * Update task optimistically (for drag and drop) + * Updates local state immediately, then syncs with server + */ + updateTaskOptimistic( + id: string, + data: { + dueDate?: string | null; + isCompleted?: boolean; + } + ) { + // Optimistic update - immediately update local state + const originalTask = tasks.find((t) => t.id === id); + if (!originalTask) return; + + tasks = tasks.map((t) => (t.id === id ? { ...t, ...data } : t)); + + // Sync with server in background + if (data.isCompleted !== undefined) { + const apiCall = data.isCompleted ? tasksApi.completeTask(id) : tasksApi.uncompleteTask(id); + + apiCall + .then((updatedTask) => { + tasks = tasks.map((t) => (t.id === id ? updatedTask : t)); + }) + .catch((e) => { + // Rollback on error + console.error('Failed to update task:', e); + tasks = tasks.map((t) => (t.id === id ? originalTask : t)); + }); + } + + if (data.dueDate !== undefined) { + tasksApi + .updateTask(id, { dueDate: data.dueDate }) + .then((updatedTask) => { + tasks = tasks.map((t) => (t.id === id ? updatedTask : t)); + }) + .catch((e) => { + // Rollback on error + console.error('Failed to update task:', e); + tasks = tasks.map((t) => (t.id === id ? originalTask : t)); + }); + } + }, + /** * Delete a task */