Skip to content

feat: add input validation and filtering to tasks API#2

Open
abhinavguptas wants to merge 1 commit intomainfrom
feat/add-task-validation
Open

feat: add input validation and filtering to tasks API#2
abhinavguptas wants to merge 1 commit intomainfrom
feat/add-task-validation

Conversation

@abhinavguptas
Copy link
Copy Markdown
Contributor

Hardens the tasks routes with proper input validation, sanitization, and filtering.

Changes

  • Required field validation on title (create + update)
  • Max length enforcement (200 chars)
  • Inputs trimmed before storing
  • Integer parsing with NaN guard on all :id params
  • Optional ?completed filter on GET /tasks
  • updatedAt timestamp on PATCH

- Validate title (required, max 200 chars) on create and update
- Sanitize inputs with trim() before storing
- Parse :id as integer with NaN guard on all routes
- Add ?completed filter to GET /tasks
- Track updatedAt timestamp on PATCH
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

🩺 Dr. Concret.io — 🔴 Changes Requested

This pull request significantly improves the robustness of the tasks API by adding comprehensive input validation, sanitization, and filtering capabilities. The changes address several common API vulnerabilities and enhance data integrity. However, there are a couple of critical correctness issues related to type coercion in the PATCH endpoint that need to be addressed before merging.

Highlights

  • Robust input validation for the 'title' field, including presence, type, length, and trimming.
  • Consistent and safe parsing of ID parameters using parseInt with radix and isNaN checks across all routes.
  • Effective implementation of the completed query parameter for filtering tasks in the GET /tasks endpoint.
  • Inclusion of an updatedAt timestamp for PATCH operations, improving data traceability.

Findings (4)

Severity Count
High 2
Medium 2

Missing Coding Standards

This repository has no coding standards files. Consider creating:

  • review-rules.md: A review-rules.md file would establish clear project-specific coding standards, best practices, and review guidelines, ensuring consistency and quality across the codebase.
  • .eslintrc.js: An ESLint configuration file (.eslintrc.js) is essential for enforcing consistent code style, identifying potential errors, and maintaining code quality across the project. It helps catch issues early in the development cycle.
  • .prettierrc.js: A Prettier configuration file (.prettierrc.js) ensures consistent code formatting automatically, reducing bikeshedding during code reviews and improving developer productivity. It works well in conjunction with ESLint.

🩺 Dr. Concret.io · Model: gemini-2.5-flash · Tokens: 2749 in, 1692 out · Cost: ~$.0138

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review: see individual comments below.

Comment thread routes/tasks.js
const task = tasks.find(t => t.id === id);
if (!task) return res.status(404).json({ error: 'Task not found' });
Object.assign(task, req.body);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 [HIGH] correctness: Incorrect boolean coercion for 'completed' in PATCH

The Boolean(completed) conversion will incorrectly interpret string values like 'false' as true. If a client sends {"completed": "false"} in the request body, the task will be marked as completed. This can lead to incorrect task states.

Suggestion:

Suggested change
Explicitly check for string 'true'/'false' if string input is expected, or ensure the input is a proper boolean. For JSON bodies, `req.body.completed` should already be a boolean if sent correctly.

Comment thread routes/tasks.js
if (isNaN(id)) return res.status(400).json({ error: 'Invalid task ID' });

const task = tasks.find(t => t.id === id);
if (!task) return res.status(404).json({ error: 'Task not found' });
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 [HIGH] correctness: Incorrect handling of null for 'description' in PATCH

If description is null in the request body, String(description).trim() will convert it to the string literal 'null'. This prevents users from explicitly clearing a task's description.

Suggestion:

Suggested change
if (!task) return res.status(404).json({ error: 'Task not found' });
Modify the logic to `task.description = description === null ? null : String(description).trim();` to correctly handle `null` values.

Comment thread routes/tasks.js
if (!task) return res.status(404).json({ error: 'Task not found' });
res.json(task);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 [MEDIUM] quality: Missing validation for 'description' field

The description field is not validated for length or type in either POST or PATCH requests. While optional, an excessively long or non-string description could lead to data quality issues or unexpected behavior.

Suggestion:

Suggested change
Add validation for `description` (e.g., max length) within the `validateTask` function or a similar helper, applying it when `description` is provided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant