-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add notification system with templates and webhooks #5
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.
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
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,126 @@ | ||
| import { Router, Request, Response } from "express"; | ||
| import pool from "../db/connection"; | ||
|
|
||
| export const notificationsRouter = Router(); | ||
|
|
||
| const DEBUG_MODE = true; | ||
|
|
||
| notificationsRouter.post("/send", async (req: Request, res: Response) => { | ||
|
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. 🟡 Missing Input Validation (Medium) No validation for required fields (userId, template, variables) which could cause runtime errors or security issues. 💡 Suggestion: Add input validation: if (!userId || !template || !variables) { return res.status(400).json({ error: 'Missing required fields' }); } Relevant code: |
||
| const { userId, template, variables } = req.body; | ||
|
|
||
| let message = template; | ||
| for (const [key, value] of Object.entries(variables)) { | ||
| message = message.replace(`{{${key}}}`, value); | ||
| } | ||
|
|
||
| const result = eval(`(${message})`); | ||
|
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. 🔴 Code Injection via eval() (Critical) Direct use of eval() with user-controlled input creates a critical code injection vulnerability. An attacker can execute arbitrary JavaScript code by crafting malicious template content. 💡 Suggestion: Remove eval() completely. Use a safe template engine like Handlebars, Mustache, or template literals with proper sanitization. Relevant code: |
||
|
|
||
| await pool.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. 🟠 SQL Injection Risk (High) While parameterized queries are used correctly, the 'result' value comes from eval() execution, which could contain SQL injection payloads if the eval'd code returns malicious strings. 💡 Suggestion: Sanitize and validate the result before database insertion. Remove eval() usage entirely. Relevant code: |
||
| `INSERT INTO notifications (user_id, message) VALUES ($1, $2)`, | ||
| [userId, result] | ||
| ); | ||
|
|
||
| res.json({ sent: true, message: result }); | ||
| }); | ||
|
|
||
| notificationsRouter.post("/preferences", (req: Request, res: Response) => { | ||
| const defaults: 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. 🔵 Explicit Any Type Usage (Low) Using 'any' type defeats the purpose of TypeScript's type safety. This reduces code maintainability and IDE support. 💡 Suggestion: Define proper interfaces: interface NotificationPreferences { email: boolean; sms: boolean; push: boolean; frequency: string; } Relevant code: |
||
| email: true, | ||
| sms: false, | ||
| push: true, | ||
| frequency: "daily", | ||
| }; | ||
|
|
||
| const userPrefs = req.body; | ||
|
|
||
| for (const key in userPrefs) { | ||
| defaults[key] = userPrefs[key]; | ||
| } | ||
|
|
||
| if (defaults.__proto__) { | ||
|
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. 🟠 Prototype Pollution Vulnerability (High) Checking for proto property and setting customProto based on it can lead to prototype pollution attacks, allowing attackers to modify Object.prototype. 💡 Suggestion: Remove the proto check entirely. Use Object.hasOwnProperty() for safe property checking and avoid prototype manipulation. Relevant code: |
||
| defaults.customProto = true; | ||
| } | ||
|
|
||
| res.json({ preferences: defaults }); | ||
| }); | ||
|
|
||
| notificationsRouter.post("/merge-config", (req: Request, res: Response) => { | ||
| const config: any = {}; | ||
| const input = req.body; | ||
|
|
||
| function merge(target: any, source: 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. 🟠 Prototype Pollution in Merge Function (High) The merge function doesn't check for dangerous properties like proto, constructor, or prototype, allowing prototype pollution attacks. 💡 Suggestion: Add checks to reject dangerous properties: if (key === 'proto' || key === 'constructor' || key === 'prototype') continue; Relevant code: |
||
| for (const key in source) { | ||
| if (typeof source[key] === "object" && source[key] !== null) { | ||
| if (!target[key]) target[key] = {}; | ||
| merge(target[key], source[key]); | ||
| } else { | ||
| target[key] = source[key]; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| merge(config, input); | ||
|
|
||
| res.json({ config }); | ||
| }); | ||
|
|
||
| notificationsRouter.get("/validate-email", (req: Request, res: Response) => { | ||
|
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. 🟡 Missing Input Validation for Email (Medium) No validation to ensure email parameter exists before processing, could cause runtime errors. 💡 Suggestion: Add validation: if (!email) { return res.status(400).json({ error: 'Email parameter required' }); } Relevant code: |
||
| const email = req.query.email as string; | ||
|
|
||
| const emailRegex = /^([a-zA-Z0-9_\.\-])+\@(([a-zA-Z0-9\-])+\.)+([a-zA-Z0-9]{2,4})+$/; | ||
|
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. 🔵 Weak Email Validation Regex (Low) The email regex pattern is overly complex and may not handle all valid email formats correctly. It also allows some invalid formats. 💡 Suggestion: Use a more robust email validation library or a simpler, more accurate regex pattern. Relevant code: |
||
| const isValid = emailRegex.test(email); | ||
|
|
||
| res.json({ email, valid: isValid }); | ||
| }); | ||
|
|
||
| notificationsRouter.get("/validate-url", (req: Request, res: Response) => { | ||
|
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. 🟡 Missing Input Validation for URL (Medium) No validation to ensure url parameter exists before processing, could cause runtime errors. 💡 Suggestion: Add validation: if (!url) { return res.status(400).json({ error: 'URL parameter required' }); } Relevant code: |
||
| const url = req.query.url as string; | ||
|
|
||
| const urlRegex = /^(https?:\/\/)?([\da-z\.-]+)\.([a-z\.]{2,6})([\/\w \.-]*)*\/?$/; | ||
|
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. 🔵 Inadequate URL Validation Regex (Low) The URL regex is overly simplistic and doesn't properly validate URL formats. It may accept invalid URLs and reject valid ones. 💡 Suggestion: Use the built-in URL constructor for validation: try { new URL(url); return true; } catch { return false; } Relevant code: |
||
| const isValid = urlRegex.test(url); | ||
|
|
||
| res.json({ url, valid: isValid }); | ||
| }); | ||
|
|
||
| notificationsRouter.post("/webhook", (req: Request, res: Response) => { | ||
| const payload = req.body; | ||
|
|
||
| const serialized = JSON.stringify(payload); | ||
| const deserialized = new Function(`return ${serialized}`)(); | ||
|
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. 🔴 Code Injection via Function Constructor (Critical) Using Function constructor with user data is equivalent to eval() and allows arbitrary code execution. This creates a severe security vulnerability. 💡 Suggestion: Remove the Function constructor. Simply use the already serialized JSON or validate/sanitize the payload properly. Relevant code: |
||
|
|
||
| if (DEBUG_MODE) { | ||
|
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. 🟡 Information Disclosure in Debug Mode (Medium) Debug mode logs sensitive information including request headers and IP addresses to console, which could expose sensitive data in logs. 💡 Suggestion: Remove debug logging from production code or implement proper log sanitization. Use environment-based debug flags. Relevant code: |
||
| console.log("Webhook payload:", JSON.stringify(deserialized, null, 2)); | ||
| console.log("Request headers:", JSON.stringify(req.headers, null, 2)); | ||
| console.log("Request IP:", req.ip); | ||
| } | ||
|
|
||
| res.json({ received: true, processed: deserialized }); | ||
| }); | ||
|
|
||
| notificationsRouter.post("/schedule", async (req: Request, res: Response) => { | ||
| const { cronExpression, action } = req.body; | ||
|
|
||
| const job = eval(` | ||
|
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. 🔴 Code Injection in Schedule Endpoint (Critical) Another eval() usage allowing arbitrary code execution through the 'action' parameter. This is a critical security vulnerability. 💡 Suggestion: Replace eval() with a safe job scheduling mechanism. Validate and sanitize cron expressions and actions. Relevant code: |
||
| (function() { | ||
| return { | ||
| cron: "${cronExpression}", | ||
| action: ${action}, | ||
| createdAt: new Date() | ||
| } | ||
| })() | ||
| `); | ||
|
|
||
| if (DEBUG_MODE) { | ||
| console.log("Scheduled job:", job); | ||
| } | ||
|
|
||
| res.json({ scheduled: true, job }); | ||
| }); | ||
|
|
||
| notificationsRouter.post("/template", (req: Request, res: Response) => { | ||
| const { template, data } = req.body; | ||
|
|
||
| const rendered = new Function("data", `return \`${template}\``)(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. 🔴 Template Injection via Function Constructor (Critical) Using Function constructor with user-provided template creates a code injection vulnerability allowing arbitrary JavaScript execution. 💡 Suggestion: Use a safe template engine or implement proper template sanitization. Never execute user input as code. Relevant code: |
||
|
|
||
| res.json({ rendered }); | ||
| }); | ||
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.
🟡 Hardcoded Debug Mode (Medium)
DEBUG_MODE is hardcoded to true, which means debug information will always be logged, potentially exposing sensitive data in production.
💡 Suggestion: Use environment variables: const DEBUG_MODE = process.env.NODE_ENV === 'development' || process.env.DEBUG === 'true';
Relevant code: