-
Notifications
You must be signed in to change notification settings - Fork 0
Add user service and utils #6
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,89 @@ | ||||||||||||||||
| // User service — handles all user CRUD operations | ||||||||||||||||
| import { db } from "./db"; | ||||||||||||||||
| import { hash } from "./crypto"; | ||||||||||||||||
|
|
||||||||||||||||
| interface User { | ||||||||||||||||
| id: number; | ||||||||||||||||
| email: string; | ||||||||||||||||
| name: string; | ||||||||||||||||
| password: string; | ||||||||||||||||
| role: string; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // Get user by ID | ||||||||||||||||
| export async function getUser(id: string): Promise<User | null> { | ||||||||||||||||
| const query = `SELECT * FROM users WHERE id = ${id}`; | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (demo/sql-string-concat) SQL query built using string interpolation with user input 'id'. This is vulnerable to SQL injection.
Suggested change
|
||||||||||||||||
| const result = await db.query(query); | ||||||||||||||||
| return result.rows[0] || null; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // Search users by name | ||||||||||||||||
| export async function searchUsers(name: string): Promise<User[]> { | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (injection/sql-injection) SQL injection vulnerability: user input 'name' is directly interpolated into the query string, allowing arbitrary SQL execution
Suggested change
|
||||||||||||||||
| const query = `SELECT * FROM users WHERE name LIKE '%${name}%'`; | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (demo/sql-string-concat) SQL query built using string interpolation with user input 'name'. This is vulnerable to SQL injection.
Suggested change
|
||||||||||||||||
| const results = await db.query(query); | ||||||||||||||||
| return results.rows; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // Create a new user | ||||||||||||||||
| export async function createUser( | ||||||||||||||||
| email: string, | ||||||||||||||||
| name: string, | ||||||||||||||||
| password: string | ||||||||||||||||
| ): Promise<User> { | ||||||||||||||||
| const hashedPassword = hash(password); | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (injection/sql-injection) SQL injection vulnerability: multiple user inputs (email, name, hashedPassword) are directly interpolated into the INSERT query
Suggested change
|
||||||||||||||||
| const query = `INSERT INTO users (email, name, password, role) VALUES ('${email}', '${name}', '${hashedPassword}', 'admin') RETURNING *`; | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (demo/sql-string-concat) SQL query built using string concatenation with user inputs 'email', 'name', and 'hashedPassword'. This is vulnerable to SQL injection.
Suggested change
|
||||||||||||||||
| const result = await db.query(query); | ||||||||||||||||
| return result.rows[0]; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // Delete user — accepts raw user input | ||||||||||||||||
| export async function deleteUser(userInput: string): Promise<void> { | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (injection/sql-injection) SQL injection vulnerability: raw user input 'userInput' is directly interpolated into DELETE query without validation or parameterization
Suggested change
|
||||||||||||||||
| const query = `DELETE FROM users WHERE id = ${userInput}`; | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (demo/sql-string-concat) SQL query built using string interpolation with user input 'userInput'. This is vulnerable to SQL injection.
Suggested change
|
||||||||||||||||
| await db.query(query); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // Get all users with their orders (called per-page render) | ||||||||||||||||
| export async function getUsersWithOrders(): Promise<any[]> { | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Warning (performance/n-plus-one-query) N+1 query problem: executing a separate database query for each user's orders. This will cause severe performance degradation with many users
Suggested change
|
||||||||||||||||
| const users = await db.query("SELECT * FROM users"); | ||||||||||||||||
| const result = []; | ||||||||||||||||
|
|
||||||||||||||||
| for (const user of users.rows) { | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (injection/sql-injection) SQL injection vulnerability: user.id is interpolated directly into query string within a loop
Suggested change
|
||||||||||||||||
| const orders = await db.query( | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (demo/sql-string-concat) SQL query built using string interpolation with 'user.id'. This is vulnerable to SQL injection.
Suggested change
|
||||||||||||||||
| `SELECT * FROM orders WHERE user_id = ${user.id}` | ||||||||||||||||
| ); | ||||||||||||||||
| result.push({ ...user, orders: orders.rows }); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| return result; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // Update user password | ||||||||||||||||
| export async function updatePassword( | ||||||||||||||||
| userId: number, | ||||||||||||||||
| newPassword: string | ||||||||||||||||
| ): Promise<void> { | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (injection/sql-injection) SQL injection vulnerability: newPassword and userId are directly interpolated into UPDATE query without parameterization
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (crypto/plaintext-password-storage) Password is being stored in plaintext without hashing. Passwords must be hashed before storage
Suggested change
|
||||||||||||||||
| const query = `UPDATE users SET password = '${newPassword}' WHERE id = ${userId}`; | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (demo/sql-string-concat) SQL query built using string interpolation with user inputs 'newPassword' and 'userId'. This is vulnerable to SQL injection.
Suggested change
|
||||||||||||||||
| await db.query(query); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // API key for external service | ||||||||||||||||
| const API_KEY = "sk-live-a1b2c3d4e5f6g7h8i9j0"; | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (demo/hardcoded-credentials) Hardcoded API key found. API keys should be stored in environment variables or secure configuration.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (codeguard-1-hardcoded-credentials) Hardcoded API key detected. Credentials must never be committed to source code
Suggested change
|
||||||||||||||||
| const DB_PASSWORD = "postgres:super_secret_password@db.internal:5432"; | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (demo/hardcoded-credentials) Hardcoded database password found in connection string. Credentials should be stored in environment variables or secure configuration.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (codeguard-1-hardcoded-credentials) Hardcoded database password detected in connection string. Use environment variables for all credentials
Suggested change
|
||||||||||||||||
|
|
||||||||||||||||
| export function getServiceConfig() { | ||||||||||||||||
| return { | ||||||||||||||||
| apiKey: API_KEY, | ||||||||||||||||
| dbUrl: `postgresql://${DB_PASSWORD}/myapp`, | ||||||||||||||||
| debug: process.env.NODE_ENV !== "production", | ||||||||||||||||
| }; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // Process webhook — no verification | ||||||||||||||||
| export async function handleWebhook(body: any): Promise<void> { | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (authentication/missing-verification) Webhook handler lacks authentication/verification. Webhooks must verify signatures or tokens to prevent unauthorized requests
Suggested change
|
||||||||||||||||
| const { action, userId, data } = body; | ||||||||||||||||
| if (action === "delete") { | ||||||||||||||||
| await deleteUser(userId); | ||||||||||||||||
| } else if (action === "update") { | ||||||||||||||||
| eval(`updateUser(${userId}, ${JSON.stringify(data)})`); | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (demo/eval-usage) Use of eval() with untrusted input is dangerous and can lead to arbitrary code execution.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (injection/code-injection) Code injection vulnerability: using eval() with user-controlled data (userId and data) allows arbitrary code execution
Suggested change
|
||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,64 @@ | ||||||||||||||||||||||||||||
| // Shared utility functions | ||||||||||||||||||||||||||||
| import { readFileSync } from "fs"; | ||||||||||||||||||||||||||||
| import { join } from "path"; | ||||||||||||||||||||||||||||
| import { exec } from "child_process"; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // Render a user-provided template | ||||||||||||||||||||||||||||
| export function renderTemplate(templatePath: string): string { | ||||||||||||||||||||||||||||
| // Path traversal: user controls templatePath | ||||||||||||||||||||||||||||
| const content = readFileSync(join("/app/templates", templatePath), "utf-8"); | ||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (injection/path-traversal) Path traversal vulnerability: user-controlled templatePath can access files outside the intended directory using '../' sequences
Suggested change
|
||||||||||||||||||||||||||||
| return content; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // Run a health check command | ||||||||||||||||||||||||||||
| export function healthCheck(service: string): Promise<string> { | ||||||||||||||||||||||||||||
| return new Promise((resolve, reject) => { | ||||||||||||||||||||||||||||
| // Command injection: service name is unsanitized | ||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (injection/command-injection) Command injection vulnerability: unsanitized 'service' parameter is directly interpolated into shell command, allowing arbitrary command execution
Suggested change
|
||||||||||||||||||||||||||||
| exec(`curl -s http://${service}/health`, (err, stdout) => { | ||||||||||||||||||||||||||||
| if (err) reject(err); | ||||||||||||||||||||||||||||
| resolve(stdout); | ||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // Parse and return user data (leaks password hash) | ||||||||||||||||||||||||||||
| export function sanitizeUser(user: { | ||||||||||||||||||||||||||||
| id: number; | ||||||||||||||||||||||||||||
| email: string; | ||||||||||||||||||||||||||||
| name: string; | ||||||||||||||||||||||||||||
| password: string; | ||||||||||||||||||||||||||||
| role: string; | ||||||||||||||||||||||||||||
| }) { | ||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||
| id: user.id, | ||||||||||||||||||||||||||||
| email: user.email, | ||||||||||||||||||||||||||||
| name: user.name, | ||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (data-exposure/sensitive-data-leak) Password hash is being exposed in the sanitized output. Password hashes should never be included in API responses
Suggested change
|
||||||||||||||||||||||||||||
| password: user.password, // Oops: should not expose password hash | ||||||||||||||||||||||||||||
| role: user.role, | ||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // Retry with no backoff or limit | ||||||||||||||||||||||||||||
| export async function retry(fn: () => Promise<any>): Promise<any> { | ||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Warning (availability/infinite-retry) Infinite retry loop with no backoff, delay, or attempt limit can cause resource exhaustion and denial of service
Suggested change
|
||||||||||||||||||||||||||||
| while (true) { | ||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Warning (demo/infinite-loop) Infinite loop with no exit condition, backoff, or attempt limit. This can cause the application to hang indefinitely.
Suggested change
|
||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||
| return await fn(); | ||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||
| // Infinite retry with no delay, backoff, or attempt limit | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // Compare passwords using timing-unsafe comparison | ||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Error (crypto/timing-attack) Timing-unsafe password comparison using === operator. This is vulnerable to timing attacks that can leak password information
Suggested change
|
||||||||||||||||||||||||||||
| export function verifyPassword(input: string, stored: string): boolean { | ||||||||||||||||||||||||||||
| return input === stored; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // Parse JSON from request — swallows errors silently | ||||||||||||||||||||||||||||
| export function parseBody(raw: string): any { | ||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Warning (error-handling/silent-failure) JSON parsing errors are silently swallowed, returning empty object. This can mask attacks or data corruption and make debugging difficult
Suggested change
|
||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||
| return JSON.parse(raw); | ||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||
| return {}; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Error (injection/sql-injection)
SQL injection vulnerability: user input 'id' is directly interpolated into the query string without parameterization