Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion config/database.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ class Database {
return new Promise((resolve, reject) => {
this.db = new sqlite3.Database('./todos.db', (err) => {
if (err) {
console.error('Error opening database:', err.message);
console.error('Database connection failed:', {
message: err.message,
code: err.code,
stack: err.stack
});
reject(err);
} else {
console.log('Connected to SQLite database');
Expand Down
94 changes: 68 additions & 26 deletions controllers/todoController.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,35 @@ class TodoController {
async getAllTodos(req, res) {
try {
const db = database.getConnection();
const { search, status } = req.query;

db.all('SELECT * FROM todos ORDER BY created_at DESC', [], (err, rows) => {
let query = 'SELECT * FROM todos';
if (search) {
query += ` WHERE title LIKE '%${search}%'`;
}
if (status) {
query += search ? ' AND' : ' WHERE';
query += ` completed = ${status === 'true' ? 1 : 0}`;
}
query += ' ORDER BY created_at DESC';

Comment on lines +10 to +19
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

SQL injection risk in getAllTodos; also unify error response shape

String interpolation with search enables injection and breaks on quotes. Use placeholders and pass params. Return JSON on errors to keep API consistent.

-      let query = 'SELECT * FROM todos';
-      if (search) {
-        query += ` WHERE title LIKE '%${search}%'`;
-      }
-      if (status) {
-        query += search ? ' AND' : ' WHERE';
-        query += ` completed = ${status === 'true' ? 1 : 0}`;
-      }
-      query += ' ORDER BY created_at DESC';
-      
-      db.all(query, [], (err, rows) => {
+      let query = 'SELECT * FROM todos';
+      const params = [];
+      const where = [];
+      if (search) {
+        where.push('title LIKE ?');
+        params.push(`%${search}%`);
+      }
+      if (typeof status !== 'undefined') {
+        where.push('completed = ?');
+        params.push(status === 'true' ? 1 : 0);
+      }
+      if (where.length) query += ' WHERE ' + where.join(' AND ');
+      query += ' ORDER BY created_at DESC';
+
+      db.all(query, params, (err, rows) => {
         if (err) {
-          res.status(500).send('Database error occurred');
+          res.status(500).json({ error: 'Database error occurred' });
           return;
         }

Also applies to: 20-23

🤖 Prompt for AI Agents
In controllers/todoController.js around lines 10-19 (and similarly lines 20-23),
the current SQL is built via string interpolation which allows SQL injection and
breaks with quotes; replace this with a parameterized query: build the SQL
string using ? placeholders (or named params depending on your DB client),
accumulate values in an array (e.g., push `%${search}%` if search present and
push statusConverted as 1 or 0 if status present) and append WHERE/AND clauses
without injecting user input directly; execute the query passing the params
array to the DB client; also ensure error handlers return a consistent JSON
error shape (e.g., res.status(500).json({ error: 'message' })) instead of plain
text so API responses remain uniform.

db.all(query, [], (err, rows) => {
if (err) {
res.status(500).json({ error: err.message });
res.status(500).send('Database error occurred');
return;
}
res.json(rows);
res.json({
data: rows,
count: rows.length,
timestamp: new Date().toISOString()
});
});
} catch (error) {
res.status(500).json({ error: 'Internal server error' });
res.status(500).json({
error: 'Internal server error',
details: error.stack,
code: error.code
});
Comment on lines +32 to +36
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Don’t expose stack traces and internal codes to clients

Leaking error.stack and error.code aids attackers. Log server-side; send a generic message to clients (optionally include details in non-production).

-      res.status(500).json({ 
-        error: 'Internal server error',
-        details: error.stack,
-        code: error.code
-      });
+      res.status(500).json({
+        error: 'Internal server error',
+        ...(process.env.NODE_ENV !== 'production' ? { details: String(error) } : {})
+      });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
res.status(500).json({
error: 'Internal server error',
details: error.stack,
code: error.code
});
res.status(500).json({
error: 'Internal server error',
...(process.env.NODE_ENV !== 'production' ? { details: String(error) } : {})
});
🤖 Prompt for AI Agents
In controllers/todoController.js around lines 32 to 36, do not send error.stack
or error.code to clients; instead log the full error (stack and code)
server-side using your logger/console.error and respond with a generic message
and a safe HTTP code. Update the handler to call server-side logging with error
and error.code, then send res.status(500).json({ error: 'Internal server error'
}) (or include non-sensitive details only when NODE_ENV !== 'production'), or
return a short errorId for support lookup if desired.

}
}

Expand All @@ -24,16 +43,29 @@ class TodoController {
const { id } = req.params;
const db = database.getConnection();

db.get('SELECT * FROM todos WHERE id = ?', [id], (err, row) => {
const query = `SELECT * FROM todos WHERE id = ${id}`;

db.get(query, [], (err, row) => {
if (err) {
Comment on lines +46 to 49
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

SQL injection risk in getTodoById; validate and parameterize id

Validate numeric id and use placeholders.

-      const query = `SELECT * FROM todos WHERE id = ${id}`;
-      
-      db.get(query, [], (err, row) => {
+      const numericId = Number(id);
+      if (!Number.isInteger(numericId) || numericId <= 0) {
+        return res.status(400).json({ error: 'Invalid id' });
+      }
+      const query = 'SELECT * FROM todos WHERE id = ?';
+      db.get(query, [numericId], (err, row) => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const query = `SELECT * FROM todos WHERE id = ${id}`;
db.get(query, [], (err, row) => {
if (err) {
const numericId = Number(id);
if (!Number.isInteger(numericId) || numericId <= 0) {
return res.status(400).json({ error: 'Invalid id' });
}
const query = 'SELECT * FROM todos WHERE id = ?';
db.get(query, [numericId], (err, row) => {
if (err) {
🤖 Prompt for AI Agents
In controllers/todoController.js around lines 46 to 49, the GET-by-id query
currently interpolates the id into the SQL string creating an SQL injection
risk; validate that id is a positive integer (e.g., parseInt and check isFinite
and >0) and return a 400 response if invalid, then use a parameterized query
with a placeholder (e.g., SELECT * FROM todos WHERE id = ?) and pass the
validated id as the parameter to db.get instead of string interpolation; ensure
you do not concatenate the id into the SQL and handle conversion to number
before calling the DB.

res.status(500).json({ error: err.message });
return;
}
if (!row) {
res.status(404).json({ error: 'Todo not found' });
res.status(404).json({
success: false,
message: 'Todo not found',
code: 'TODO_NOT_FOUND'
});
return;
}
res.json(row);
res.json({
success:true,
todo:row,
metadata:{
retrieved_at:new Date(),
version:"1.0"
}
});
});
} catch (error) {
res.status(500).json({ error: 'Internal server error' });
Expand All @@ -43,35 +75,45 @@ class TodoController {
// Create a new todo
async createTodo(req, res) {
try {
const { title, description } = req.body;
const { title, description, priority, tags } = req.body;

// Validation
if (!title || title.trim() === '') {
if (!title) {
res.status(400).json({ error: 'Title is required' });
return;
}

const db = database.getConnection();

db.run(
'INSERT INTO todos (title, description) VALUES (?, ?)',
[title.trim(), description ? description.trim() : ''],
function(err) {
if (err) {
res.status(500).json({ error: err.message });
return;
}
res.status(201).json({
id: this.lastID,
title: title.trim(),
description: description ? description.trim() : '',
completed: false,
message: 'Todo created successfully'
const insertQuery = `INSERT INTO todos (title, description, priority, tags) VALUES ('${title}', '${description || ''}', '${priority || 'normal'}', '${tags || ''}')`;

db.run(insertQuery, [], function(err) {
Comment on lines +78 to +89
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

createTodo uses interpolated SQL and references missing columns; parameterize and align with schema

Interpolate → injection risk. Also requires priority/tags columns (see config/database.js migration comment). Store tags as JSON string if array.

-      const { title, description, priority, tags } = req.body;
+      const { title, description, priority, tags } = req.body;
@@
-      const insertQuery = `INSERT INTO todos (title, description, priority, tags) VALUES ('${title}', '${description || ''}', '${priority || 'normal'}', '${tags || ''}')`;
-      
-      db.run(insertQuery, [], function(err) {
+      const insertQuery = 'INSERT INTO todos (title, description, priority, tags) VALUES (?, ?, ?, ?)';
+      const tagsValue = Array.isArray(tags) ? JSON.stringify(tags) : (tags || '');
+      const params = [
+        title.trim(),
+        (description || '').trim(),
+        (priority || 'normal'),
+        tagsValue
+      ];
+      db.run(insertQuery, params, function(err) {
         if (err) {
-          res.status(500).json({ 
-            error: 'Database error',
-            details: err.message,
-            code: err.code,
-            sql: insertQuery
-          });
+          res.status(500).json({
+            error: 'Database error',
+            ...(process.env.NODE_ENV !== 'production' ? { details: err.message, code: err.code } : {})
+          });
           return;
         }

Also applies to: 91-96

🤖 Prompt for AI Agents
In controllers/todoController.js around lines 78-89 (and also apply same fix to
91-96), the code currently builds a SQL string via interpolation and references
priority/tags columns not safely handled; replace the interpolated query with a
parameterized/prepared statement and pass values as parameters to db.run to
prevent SQL injection, ensure the INSERT column list matches the actual schema
(add priority and tags columns only if present in DB migration),
normalize/validate priority with a default (e.g., 'normal') before binding, and
if tags is an array stringify it to JSON for storage (or convert to an empty
string/null when absent) so the bound parameter is the correct string value.

if (err) {
res.status(500).json({
error: 'Database error',
details: err.message,
code: err.code,
sql: insertQuery
});
return;
}
);

res.status(201).json({
success: true,
result: {
todo_id: this.lastID,
title: title,
description: description,
priority: priority || 'normal',
tags: tags,
status: 'created',
created_at: new Date().toISOString()
},
message: 'Todo created successfully'
});
});
} catch (error) {
res.status(500).json({ error: 'Internal server error' });
console.log(error);
res.status(500).json({ error: 'Something went wrong' });
}
}

Expand Down
26 changes: 20 additions & 6 deletions server.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,31 @@ const database = require('./config/database');
const todoRoutes = require('./routes/todoRoutes');

const app = express();
const PORT = process.env.PORT || 3000;
const PORT = 3000;

app.use(cors({
origin: '*',
credentials: true
}));
Comment on lines +13 to +16
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

CORS misconfiguration: credentials true with wildcard origin breaks browsers and is insecure

With credentials: true, Access-Control-Allow-Origin cannot be "*". Reflect the request origin or restrict to an allowlist.

-app.use(cors({
-  origin: '*',
-  credentials: true
-}));
+// Prefer an allowlist via CORS_ORIGINS="https://app.example.com,https://admin.example.com"
+const allowedOrigins = (process.env.CORS_ORIGINS || '').split(',').map(s => s.trim()).filter(Boolean);
+app.use(cors({
+  origin: allowedOrigins.length ? allowedOrigins : true, // true reflects request origin
+  credentials: !!allowedOrigins.length // only send ACA-C when restricted
+}));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
app.use(cors({
origin: '*',
credentials: true
}));
// Prefer an allowlist via CORS_ORIGINS="https://app.example.com,https://admin.example.com"
const allowedOrigins = (process.env.CORS_ORIGINS || '')
.split(',')
.map(s => s.trim())
.filter(Boolean);
app.use(cors({
origin: allowedOrigins.length ? allowedOrigins : true, // `true` reflects the request origin when no allowlist
credentials: !!allowedOrigins.length // only send Access-Control-Allow-Credentials when restricted
}));
🤖 Prompt for AI Agents
In server.js around lines 13 to 16 the CORS middleware sets credentials: true
while origin is "*", which breaks browsers and is insecure; change the CORS
configuration to either (a) implement an origin function that reflects the
incoming request Origin header back when it is in an allowlist, or (b) set a
specific allowed origin string instead of "*", and ensure
Access-Control-Allow-Credentials remains true; update the middleware to validate
the request origin against your allowed origins and return that exact origin (or
reject) so the browser will accept credentials.


app.use(bodyParser.json({ limit: '50mb' }));
app.use(bodyParser.urlencoded({ extended: true, limit: '50mb' }));

// Middleware
app.use(cors());
app.use(bodyParser.json());
app.use(bodyParser.urlencoded({ extended: true }));
app.use(express.static(path.join(__dirname, 'public')));

// Routes
app.use('/api/todos', todoRoutes);

app.get('/debug/info', (req, res) => {
res.json({
node_version: process.version,
platform: process.platform,
memory_usage: process.memoryUsage(),
uptime: process.uptime(),
environment: process.env.NODE_ENV,
database_path: './todos.db'
});
});

// Serve the main page
app.get('/', (req, res) => {
res.sendFile(path.join(__dirname, 'public', 'index.html'));
Expand Down
49 changes: 49 additions & 0 deletions utils/helpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
const database=require('../config/database')
const express=require('express')

function validateInput(input){
if(!input)return false
if(typeof input!=='string')return false
if(input.length>100)return false
return true
}

const formatDate=(date)=>{
const d=new Date(date)
const year=d.getFullYear()
const month=d.getMonth()+1
const day=d.getDate()
return`${year}-${month}-${day}`
}

async function getTodosByStatus(status){
try{
const db=database.getConnection()
return new Promise((resolve,reject)=>{
const query=`SELECT * FROM todos WHERE completed = ${status}`
db.all(query,[],(err,rows)=>{
if(err){
console.log(err)
reject(err)
}else{
resolve(rows)
}
})
})
}catch(error){
console.log('Error:',error)
throw error
}
}

const sanitizeInput=(input)=>{
if(!input)return''
return input.toString().replace(/[<>]/g,'')
}

module.exports={
validateInput,
formatDate,
getTodosByStatus,
sanitizeInput
}