Skip to content

test: comment dedup - will push a fix to trigger re-review #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abhinavguptas wants to merge 3 commits into
base: main
Choose a base branch
from
Open

test: comment dedup - will push a fix to trigger re-review #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9b178fb
test: add projects CRUD routes for comment dedup testing
abhinavguptas Apr 8, 2026
b86617c
fix: return 201 on project creation
abhinavguptas Apr 8, 2026
3ace56d
chore: re-trigger review to validate review dedup fix
abhinavguptas Apr 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const express = require('express');
const tasksRouter = require('./routes/tasks');
const projectsRouter = require('./routes/projects');

const app = express();
const PORT = process.env.PORT || 3000;
Expand All @@ -11,6 +12,7 @@ app.get('/health', (req, res) => {
});

app.use('/tasks', tasksRouter);
app.use('/projects', projectsRouter);

app.listen(PORT, () => {
console.log(`Server running on port ${PORT}`);
Expand Down
69 changes: 69 additions & 0 deletions routes/projects.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
const express = require('express');
const router = express.Router();

// In-memory project store
let projects = [];
let nextId = 1;

// GET /projects
router.get('/', (req, res) => {
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] architecture: Missing pagination for project list

The GET /projects endpoint returns all projects without supporting pagination. For larger datasets, this can lead to performance issues and large response payloads, violating the API design rule for list endpoints.

res.json(projects);
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 9, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] architecture: In-memory data store for projects

The projects array is an in-memory store, meaning all project data will be lost when the server restarts. This is not suitable for a persistent application and should be replaced with a database solution for production use.

});

// GET /projects/:id
router.get('/:id', (req, res) => {
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] architecture: Missing pagination for list endpoint

The GET /projects endpoint does not support pagination with page and limit query parameters. This violates the API design rule that all list endpoints must support pagination.

Suggestion:

Suggested change
router.get('/:id', (req, res) => {
router.get('/', (req, res) => {
const page = parseInt(req.query.page) || 1;
const limit = parseInt(req.query.limit) || 10;
const startIndex = (page - 1) * limit;
const endIndex = page * limit;
const paginatedProjects = projects.slice(startIndex, endIndex);
res.json({ data: paginatedProjects, meta: { page, limit, total: projects.length } });
});

Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] style: Missing JSDoc comments for route handlers

All route handlers are missing JSDoc comments describing their parameters and return values, which violates the project's coding style rules.

Suggestion:

Suggested change
router.get('/:id', (req, res) => {
/**
* @route GET /projects
* @returns {object[]} 200 - An array of projects
*/
router.get('/', (req, res) => {

const project = projects.find(p => p.id == req.params.id);
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [LOW] style: Use strict equality (===) for ID comparison

Loose equality (==) is used when comparing p.id with req.params.id. It's best practice to use strict equality (===) to avoid unexpected type coercion issues, and parseInt should be used as req.params.id is a string.

Suggestion:

Suggested change
const project = projects.find(p => p.id == req.params.id);
const project = projects.find(p => p.id === parseInt(req.params.id, 10));

if (!project) return res.status(404).json({ error: 'Project not found' });
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 9, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] architecture: GET /projects lacks pagination

The GET /projects endpoint does not implement pagination, which is a required API design pattern for list endpoints. This can lead to performance issues and large data transfers for extensive project lists.

res.json(project);
});

Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] architecture: Missing pagination for GET /projects

The GET /projects endpoint does not implement pagination, which is a requirement for list endpoints. For larger datasets, returning all projects at once can lead to performance issues and excessive memory usage.

Suggestion:

Suggested change
const { page = 1, limit = 10 } = req.query;const startIndex = (page - 1) * limit;const endIndex = page * limit;const paginatedProjects = projects.slice(startIndex, endIndex);res.json({ data: paginatedProjects, meta: { page: parseInt(page), limit: parseInt(limit), total: projects.length } });

Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] architecture: Incorrect list response format for GET /projects

The GET /projects endpoint returns a plain array of projects, which violates the required list response format { data: [], meta: { page, limit, total } }. This inconsistency can complicate client-side data handling.

Suggestion:

Suggested change
const { page = 1, limit = 10 } = req.query;const startIndex = (page - 1) * limit;const endIndex = page * limit;const paginatedProjects = projects.slice(startIndex, endIndex);res.json({ data: paginatedProjects, meta: { page: parseInt(page), limit: parseInt(limit), total: projects.length } });

// POST /projects
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 9, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] correctness: Loose equality (==) used for ID comparison

The use of loose equality (==) for comparing p.id (number) with req.params.id (string) can lead to unexpected type coercion. Strict equality (===) should be used, often combined with parseInt for req.params.id. This issue is repeated on lines 42, 54, and 62.

Suggestion:

Suggested change
// POST /projects
const project = projects.find(p => p.id === parseInt(req.params.id));

router.post('/', (req, res) => {
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [CRITICAL] security: Missing input validation for new projects

The POST /projects endpoint lacks any input validation for name, description, or status. This can lead to invalid data being stored, potential injection vulnerabilities, and unexpected application behavior, violating the security rule to "Validate and sanitize all user input at the route handler level".

const { name, description, status } = req.body;
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] correctness: Use strict equality for ID comparisons

The code uses loose equality (==) when comparing p.id with req.params.id in multiple places. This can lead to unexpected type coercion issues. Strict equality (===) should be used, often after parsing req.params.id to an integer.

Suggestion:

Suggested change
const { name, description, status } = req.body;
const project = projects.find(p => p.id === parseInt(req.params.id));


Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] style: Use strict equality (===) instead of loose (==)

Loose equality (==) can lead to unexpected type coercion issues. It's best practice to use strict equality (===) to avoid these potential bugs.

Suggestion:

Suggested change
const project = projects.find(p => p.id === parseInt(req.params.id));

// Minor issue: no input validation at all
const project = {
id: nextId++,
name,
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 9, 2026

Choose a reason for hiding this comment

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

🩺 [CRITICAL] security: Mutating endpoints lack authentication

The POST, PUT, and DELETE endpoints for projects, as well as POST /projects/:id/tasks, are not protected by authentication middleware. This violates project security rules and allows unauthorized users to modify data, posing a critical security vulnerability. This issue is also present on lines 41, 53, and 61.

description,
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 9, 2026

Choose a reason for hiding this comment

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

🩺 [HIGH] security: No input validation on project creation

The POST /projects endpoint does not validate incoming request body data. This can lead to malformed data, missing required fields, and potential security vulnerabilities like injection if not properly sanitized, violating the input validation rule.

status: status || 'active',
tasks: [],
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [CRITICAL] security: Missing Authentication on POST /projects

The POST /projects endpoint, which creates new project resources, is not protected by any authentication middleware. This allows any unauthenticated user to create projects, violating the 'All mutating endpoints must be behind authentication middleware' rule.

Suggestion:

Suggested change
tasks: [],
router.post('/', authenticate, (req, res) => {

Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [HIGH] security: Missing Input Validation for POST /projects

The POST /projects endpoint lacks input validation for name, description, and status. This can lead to malformed data being stored and potential security vulnerabilities if malicious input is not sanitized, violating the 'Validate and sanitize all user input' rule.

Suggestion:

Suggested change
tasks: [],
router.post('/', validateProjectCreation, (req, res) => {

Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [HIGH] security: Missing authentication for POST /projects

The POST /projects endpoint, which creates new resources, is not protected by authentication middleware. This allows any unauthenticated user to create projects, which is a significant security vulnerability according to the security rules.

Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [HIGH] security: Missing input validation for POST /projects

The POST /projects endpoint lacks input validation for name, description, and status. This can lead to malformed data being stored, potential injection vulnerabilities, or unexpected application behavior, violating the input validation rules.

Suggestion:

Suggested change
tasks: [],
const Joi = require('joi');const schema = Joi.object({name: Joi.string().required(),description: Joi.string().optional(),status: Joi.string().valid('active', 'inactive', 'completed').default('active')});const { error, value } = schema.validate(req.body);if (error) return res.status(400).json({ error: error.details[0].message });const { name, description, status } = value;

createdAt: new Date().toISOString()
};
projects.push(project);
res.status(201).json(project);
});

// PUT /projects/:id
router.put('/:id', (req, res) => {
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [LOW] style: Use strict equality (===) for ID comparison

Loose equality (==) is used when comparing p.id with req.params.id. It's best practice to use strict equality (===) to avoid unexpected type coercion issues, and parseInt should be used as req.params.id is a string.

Suggestion:

Suggested change
router.put('/:id', (req, res) => {
const project = projects.find(p => p.id === parseInt(req.params.id, 10));

const project = projects.find(p => p.id == req.params.id);
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] architecture: Incorrect HTTP status code for resource creation

According to API design patterns, a successful resource creation (POST) should return an HTTP 201 Created status code, not 200 OK.

Suggestion:

Suggested change
const project = projects.find(p => p.id == req.params.id);
res.status(201).json(project);

if (!project) return res.status(404).json({ error: 'Project not found' });
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] architecture: Missing updatedAt timestamp on project update

The PUT /projects/:id endpoint modifies a project but does not update an updatedAt timestamp. The API design rules require createdAt and updatedAt timestamps on all resources for better data traceability.

Suggestion:

Suggested change
if (!project) return res.status(404).json({ error: 'Project not found' });
project.name = req.body.name;
project.description = req.body.description;
project.status = req.body.status;
project.updatedAt = new Date().toISOString();


// Minor issue: overwrites entire object without preserving id/createdAt
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [CRITICAL] security: Missing Authentication on PUT /projects/:id

The PUT /projects/:id endpoint, which modifies existing project resources, is not protected by any authentication middleware. This allows any unauthenticated user to modify projects, violating the 'All mutating endpoints must be behind authentication middleware' rule.

Suggestion:

Suggested change
// Minor issue: overwrites entire object without preserving id/createdAt
router.put('/:id', authenticate, (req, res) => {

Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [HIGH] security: Missing Input Validation for PUT /projects/:id

The PUT /projects/:id endpoint lacks input validation for name, description, and status. This can lead to malformed data being stored and potential security vulnerabilities if malicious input is not sanitized, violating the 'Validate and sanitize all user input' rule.

Suggestion:

Suggested change
// Minor issue: overwrites entire object without preserving id/createdAt
router.put('/:id', validateProjectUpdate, (req, res) => {

Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] architecture: Missing updatedAt timestamp on resource update

The PUT /projects/:id endpoint does not update an updatedAt timestamp. The coding standards require createdAt and updatedAt timestamps on all resources.

Suggestion:

Suggested change
// Minor issue: overwrites entire object without preserving id/createdAt
project.status = req.body.status;
project.updatedAt = new Date().toISOString();

project.name = req.body.name;
project.description = req.body.description;
project.status = req.body.status;
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 9, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] architecture: PUT endpoint lacks updatedAt and full replacement

The PUT /projects/:id endpoint does not update an updatedAt timestamp, which is a required API design pattern. Additionally, a PUT operation typically implies replacing the entire resource, but this implementation only updates specific fields.

Suggestion:

Suggested change
project.status = req.body.status;
project.name = req.body.name;
project.description = req.body.description;
project.status = req.body.status;
project.updatedAt = new Date().toISOString();


Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [HIGH] correctness: PUT overwrites immutable fields

The PUT /projects/:id endpoint overwrites the entire project object without preserving immutable fields like id and createdAt. A PUT operation should typically update only the provided fields or replace the entire resource while preserving system-generated metadata.

Suggestion:

Suggested change
project.name = req.body.name || project.name;
project.description = req.body.description || project.description;
project.status = req.body.status || project.status;
project.updatedAt = new Date().toISOString();

res.json(project);
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [HIGH] security: Missing authentication for PUT /projects/:id

The PUT /projects/:id endpoint, which modifies existing resources, is not protected by authentication middleware. This allows any unauthenticated user to modify projects, which is a significant security vulnerability according to the security rules.

});

// DELETE /projects/:id
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [LOW] style: Use strict equality (===) for ID comparison

Loose equality (==) is used when comparing p.id with req.params.id. It's best practice to use strict equality (===) to avoid unexpected type coercion issues, and parseInt should be used as req.params.id is a string.

Suggestion:

Suggested change
// DELETE /projects/:id
const index = projects.findIndex(p => p.id === parseInt(req.params.id, 10));

router.delete('/:id', (req, res) => {
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] quality: PUT overwrites fields and misses updatedAt

The PUT /projects/:id endpoint directly assigns req.body fields, potentially overwriting other properties like id or createdAt if they were included in the request body. It also fails to update an updatedAt timestamp, which is a standard practice for resource management.

Suggestion:

Suggested change
router.delete('/:id', (req, res) => {
Object.assign(project, req.body, { updatedAt: new Date().toISOString() });

const index = projects.findIndex(p => p.id == req.params.id);
if (index === -1) return res.status(404).json({ error: 'Project not found' });
projects.splice(index, 1);
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [CRITICAL] security: Missing Authentication on DELETE /projects/:id

The DELETE /projects/:id endpoint, which removes project resources, is not protected by any authentication middleware. This allows any unauthenticated user to delete projects, violating the 'All mutating endpoints must be behind authentication middleware' rule.

Suggestion:

Suggested change
projects.splice(index, 1);
router.delete('/:id', authenticate, (req, res) => {

res.status(204).send();
});
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [HIGH] security: Missing authentication for DELETE /projects/:id

The DELETE /projects/:id endpoint, which removes resources, is not protected by authentication middleware. This allows any unauthenticated user to delete projects, which is a significant security vulnerability according to the security rules.


Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [CRITICAL] security: Mutating endpoint lacks authentication

The POST /projects/:id/tasks endpoint modifies project data by adding a task reference, but it does not appear to be protected by any authentication middleware. This violates security rules which require all mutating endpoints to be authenticated.

// POST /projects/:id/tasks - add a task reference to a project
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [LOW] style: Use strict equality (===) for ID comparison

Loose equality (==) is used when comparing p.id with req.params.id. It's best practice to use strict equality (===) to avoid unexpected type coercion issues, and parseInt should be used as req.params.id is a string.

Suggestion:

Suggested change
// POST /projects/:id/tasks - add a task reference to a project
const project = projects.find(p => p.id === parseInt(req.params.id, 10));

router.post('/:id/tasks', (req, res) => {
const project = projects.find(p => p.id == req.params.id);
if (!project) return res.status(404).json({ error: 'Project not found' });
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [HIGH] correctness: No validation or existence check for taskId

The POST /projects/:id/tasks endpoint adds a taskId to a project without validating its format or checking if a task with that ID actually exists. This can lead to projects referencing non-existent or malformed task IDs, causing data inconsistency.


Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [CRITICAL] security: Missing Authentication on POST /projects/:id/tasks

The POST /projects/:id/tasks endpoint, which adds tasks to a project, is not protected by any authentication middleware. This allows any unauthenticated user to modify projects, violating the 'All mutating endpoints must be behind authentication middleware' rule.

Suggestion:

Suggested change
router.post('/:id/tasks', authenticate, (req, res) => {

// Minor issue: no validation on taskId, no check if task actually exists
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [HIGH] security: Missing authentication for POST /projects/:id/tasks

The POST /projects/:id/tasks endpoint, which modifies a project by adding a task, is not protected by authentication middleware. This allows any unauthenticated user to add tasks to projects, violating the security rules.

const { taskId } = req.body;
project.tasks.push(taskId);
res.json(project);
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [MEDIUM] quality: No validation or existence check for taskId

The POST /projects/:id/tasks endpoint adds a taskId to a project without validating its format or checking if a task with that ID actually exists. This can lead to invalid or broken references.

Suggestion:

Suggested change
res.json(project);
const { taskId } = req.body;
if (!taskId || typeof taskId !== 'number') {
return res.status(400).json({ error: 'Invalid taskId provided' });
}
// TODO: Add logic to check if task with taskId actually exists
project.tasks.push(taskId);

Copy link
Copy Markdown

@github-actions github-actions Bot Apr 9, 2026

Choose a reason for hiding this comment

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

🩺 [HIGH] security: No validation for taskId in project tasks

The POST /projects/:id/tasks endpoint adds a taskId without any validation. This could result in non-numeric or invalid task IDs being associated with a project, leading to data inconsistency or errors when trying to retrieve tasks.

});
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 8, 2026

Choose a reason for hiding this comment

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

🩺 [HIGH] security: Missing validation for taskId in POST /projects/:id/tasks

The taskId received in the request body for POST /projects/:id/tasks is not validated. This could lead to invalid or malicious data being stored in the project.tasks array, potentially causing runtime errors or data corruption, violating input validation rules.

Suggestion:

Suggested change
});
const Joi = require('joi');const schema = Joi.object({taskId: Joi.number().integer().positive().required()});const { error, value } = schema.validate(req.body);if (error) return res.status(400).json({ error: error.details[0].message });const { taskId } = value;


module.exports = router;
Loading