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 +8 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

Eliminate SQL injection in filtering (use placeholders, not string concat)

Current LIKE/status concatenation is injectable and breaks on quotes. Use parameterized queries.

-      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) => {
+      const base = 'SELECT * FROM todos';
+      const where = [];
+      const params = [];
+      if (search) {
+        where.push('title LIKE ?');
+        params.push(`%${search}%`);
+      }
+      if (typeof status === 'string') {
+        where.push('completed = ?');
+        params.push(status === 'true' ? 1 : 0);
+      }
+      const query = `${base}${where.length ? ' WHERE ' + where.join(' AND ') : ''} ORDER BY created_at DESC`;
+
+      db.all(query, params, (err, rows) => {

Also applies to: 20-30

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
});
}
}

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

Parameterize getTodoById (current code is injectable)

Interpolate-free query with numeric coercion.

-      const query = `SELECT * FROM todos WHERE id = ${id}`;
-      
-      db.get(query, [], (err, row) => {
+      const query = 'SELECT * FROM todos WHERE id = ?';
+      db.get(query, [Number(id)], (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 query = 'SELECT * FROM todos WHERE id = ?';
db.get(query, [Number(id)], (err, row) => {
if (err) {
🤖 Prompt for AI Agents
In controllers/todoController.js around lines 46 to 49, the SQL query currently
interpolates id directly which is vulnerable to injection; change to a
parameterized query and coerce id to a number before using it. Replace the
string interpolation with a placeholder (e.g., "SELECT * FROM todos WHERE id =
?"), call db.get using the id as a bound parameter array, and coerce/validate
the id (parseInt or Number and check for NaN) before passing it; handle invalid
id by returning an appropriate error response.

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) {
if (err) {
res.status(500).json({
error: 'Database error',
details: err.message,
code: err.code,
sql: insertQuery
});
return;
}
Comment on lines +87 to 98
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix INSERT: SQL injection + schema mismatch (priority,tags don’t exist yet)

  • Uses string interpolation → injectable and breaks on quotes.
  • Table schema lacks priority/tags, so this fails at runtime.
-      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 values = [
+        title.trim(),
+        (description || '').trim(),
+        (priority || 'normal'),
+        Array.isArray(tags) ? JSON.stringify(tags) : (tags || '')
+      ];
+      
+      db.run(insertQuery, values, function(err) {
-        if (err) {
-          res.status(500).json({ 
-            error: 'Database error',
-            details: err.message,
-            code: err.code,
-            sql: insertQuery
-          });
+        if (err) {
+          res.status(500).json({
+            error: 'Database error',
+            details: err.message,
+            code: err.code
+          });
           return;
         }

Additionally update the DB schema and migrate existing DBs. Example (in config/database.js initializeTables), after CREATE TABLE:

// Ensure new columns exist (idempotent)
this.db.all("PRAGMA table_info(todos)", [], (e, cols) => {
  if (e) return reject(e);
  const names = new Set(cols.map(c => c.name));
  const alters = [];
  if (!names.has('priority')) alters.push("ALTER TABLE todos ADD COLUMN priority TEXT DEFAULT 'normal'");
  if (!names.has('tags')) alters.push("ALTER TABLE todos ADD COLUMN tags TEXT DEFAULT ''");
  (function runNext() {
    if (!alters.length) return resolve();
    const sql = alters.shift();
    // Ignore 'duplicate column name' errors for safety
    this.db.run(sql, (err2) => runNext());
  }).call(this);
});
🤖 Prompt for AI Agents
In controllers/todoController.js around lines 87-98, the INSERT uses string
interpolation causing SQL injection and will break on quotes, and it expects
columns (priority, tags) that may not exist; fix by converting the INSERT to a
parameterized prepared statement (use placeholders and pass values as params)
and only include priority/tags when present or set safe defaults, and update the
DB initialization/migration (config/database.js initializeTables) to
idempotently add priority and tags columns if missing (run PRAGMA table_info,
build ALTER TABLE statements for missing columns with defaults, and execute them
while safely ignoring duplicate column errors) so existing DBs are migrated.

);

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 +11 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

Fix invalid CORS config ('*' + credentials) and restore env-driven PORT

Browsers reject Access-Control-Allow-Origin: * when Access-Control-Allow-Credentials: true. Also, hardcoding PORT breaks PaaS.

-const PORT = 3000;
-
-app.use(cors({
-  origin: '*',
-  credentials: true
-}));
+const PORT = process.env.PORT || 3000;
+
+const corsOptions = {
+  origin: process.env.CORS_ORIGIN || 'http://localhost:3000',
+  credentials: true
+};
+app.use(cors(corsOptions));
📝 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 PORT = 3000;
app.use(cors({
origin: '*',
credentials: true
}));
const PORT = process.env.PORT || 3000;
const corsOptions = {
origin: process.env.CORS_ORIGIN || 'http://localhost:3000',
credentials: true
};
app.use(cors(corsOptions));
🤖 Prompt for AI Agents
In server.js around lines 11–16, change the hardcoded PORT to use an environment
variable (e.g., process.env.PORT || 3000) so PaaS can control the port, and
replace the invalid CORS config (origin: '*', credentials: true) by configuring
a concrete origin or dynamic origin resolution—e.g., read allowed origin(s) from
an env var (CORS_ORIGIN) or implement a function that echoes/validates the
request Origin against an allowlist—while keeping credentials: true.


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'
});
});
Comment on lines +25 to +34
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Protect /debug/info (information disclosure)

This exposes environment and memory details. Gate it by environment (or auth).

-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'
-  });
-});
+if (process.env.NODE_ENV !== 'production') {
+  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'
+    });
+  });
+}
📝 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.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'
});
});
if (process.env.NODE_ENV !== 'production') {
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'
});
});
}
🤖 Prompt for AI Agents
In server.js around lines 25 to 34 the /debug/info route currently returns
sensitive process and environment details; gate this endpoint so it is not
exposed in production by checking process.env.NODE_ENV and only registering or
responding when NODE_ENV !== 'production' (or require a simple auth header/api
key when NODE_ENV === 'production'); if access is not allowed return a 403 or
404 and do not include any sensitive fields. Ensure the route either is
conditionally mounted or performs the environment/auth check at the top and
avoids sending process.env or full memory dumps in production.


// 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
}