mirror of
https://github.com/Memo-2023/mana-monorepo.git
synced 2026-05-14 20:01:09 +02:00
fix(todo): add input validation, N+1 fix, and RRULE bounds checking
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 <noreply@anthropic.com>
This commit is contained in:
parent
8b9337ac72
commit
4197b61622
6 changed files with 129 additions and 15 deletions
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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<string, { id: string; name: string; color: string | null }[]>();
|
||||
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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<TaskWithLabels | null> {
|
||||
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);
|
||||
|
|
|
|||
|
|
@ -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
|
||||
*/
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue