Add expense tracker app for Claude and OpenAI review demo

[reeder32]

@coderabbitai ignore

Summary

• Full-stack TypeScript expense tracker (Express + React + SQLite) with intentional bugs for automated review testing
• Claude and OpenAI GitHub Actions workflows will review this PR
• Bugs span security (SQL injection, auth bypass), logic errors, performance issues, and accessibility violations

What to look for

• Claude and OpenAI should each post inline review comments on the intentional bugs
• Compare coverage, accuracy, and detail between the two reviewers

[github-actions]

Claude finished @reeder32's task —— View job

---

Security & Bug Review

I've completed a comprehensive review of the expense tracker application. This codebase contains numerous intentional security vulnerabilities, logic errors, performance issues, and accessibility violations. Here's my detailed analysis:

Todo List

• Review backend database layer (db.ts)
• Review backend server setup (index.ts)
• Review authentication middleware (auth.ts)
• Review authentication routes (routes/auth.ts)
• Review expense routes (routes/expenses.ts)
• Review reports routes (routes/reports.ts)
• Review validation utilities (utils/validate.ts)
• Review frontend App component (App.tsx)
• Review API client (api/client.ts)
• Review ExpenseCard component (components/ExpenseCard.tsx)
• Review StatusBadge component (components/StatusBadge.tsx)
• Review Dashboard page (pages/Dashboard.tsx)
• Review ExpenseForm page (pages/ExpenseForm.tsx)
• Review ExpenseList page (pages/ExpenseList.tsx)
• Review Login page (pages/Login.tsx)
• Review Reports page (pages/Reports.tsx)
• Compile and document all findings

🚨 Critical Security Vulnerabilities

SQL Injection Vulnerabilities

1. Login Authentication Bypass - backend/src/routes/auth.ts:53

• Severity: CRITICAL
• Issue: Direct string interpolation in SQL query allows SQL injection
• Code: SELECT * FROM users WHERE email = '${email}'
• Impact: Attacker can bypass authentication entirely with payloads like admin@test.com' OR '1'='1

2. Expense Category Filter - backend/src/routes/expenses.ts:61

• Severity: HIGH
• Issue: Category filter parameter directly interpolated into query
• Code: query += AND category = '${category}'``
• Impact: Data exfiltration and potential database manipulation

Authentication & Authorization Flaws

3. Hardcoded JWT Secret - backend/src/middleware/auth.ts:4

• Severity: CRITICAL
• Issue: JWT secret hardcoded in source code
• Code: const JWT_SECRET = "supersecretkey123"
• Impact: Anyone with code access can forge tokens

4. Missing Bearer Token Validation - backend/src/middleware/auth.ts:17

• Severity: MEDIUM
• Issue: Doesn't validate "Bearer " prefix properly
• Code: const token = header.replace("Bearer ", "")
• Impact: Accepts any string format as valid auth header

5. Role-Based Access Control Bypass - backend/src/middleware/auth.ts:30

• Severity: HIGH
• Issue: Admin check relies on JWT payload controlled by user
• Code: if (req.user?.role !== "admin")
• Impact: Users can forge admin role in JWT to gain elevated privileges

6. No Token Expiration - backend/src/routes/auth.ts:32,68

• Severity: MEDIUM
• Issue: JWTs never expire
• Impact: Stolen tokens remain valid indefinitely

Cross-Site Scripting (XSS)

7. Stored XSS in Expense Descriptions - frontend/src/components/ExpenseCard.tsx:32

• Severity: HIGH
• Issue: Raw HTML rendering via dangerouslySetInnerHTML
• Code: dangerouslySetInnerHTML={{ __html: expense.description }}
• Impact: Malicious scripts execute when other users view expenses

CSV Injection

8. Formula Injection in CSV Export - backend/src/routes/reports.ts:50-52

• Severity: MEDIUM
• Issue: User-controlled fields not escaped in CSV export
• Impact: Malicious formulas (=CMD("calc")) execute when opened in Excel

Insecure Direct Object References (IDOR)

9. Unauthorized Expense Deletion - backend/src/routes/expenses.ts:102-103

• Severity: HIGH
• Issue: Delete endpoint doesn't verify expense ownership
• Code: Only checks if expense exists, not ownership
• Impact: Any authenticated user can delete other users' expenses

10. Missing Authorization on Expense Status Updates - backend/src/routes/expenses.ts:78-86

• Severity: HIGH
• Issue: Any user can approve/reject expenses, including their own
• Impact: Self-approval bypasses approval workflow

⚠️ Logic Errors & Race Conditions

11. Budget Limit Race Condition - backend/src/routes/expenses.ts:30-35

• Severity: MEDIUM
• Issue: Budget check and insertion aren't in transaction
• Impact: Concurrent requests can exceed budget limit

12. Off-by-one Error in Budget Check - backend/src/routes/expenses.ts:35

• Severity: LOW
• Issue: Uses > instead of >=
• Impact: Users can spend exactly $5000.01 over limit

13. Runtime Error on Null Total - frontend/src/pages/Reports.tsx:58

• Severity: MEDIUM
• Issue: row.total.toFixed(2) throws when total is null
• Impact: Page crashes for users with no expenses

🐌 Performance Issues & Memory Leaks

14. Memory Leak in Dashboard - frontend/src/pages/Dashboard.tsx:11

• Severity: MEDIUM
• Issue: No cleanup for async request in useEffect
• Impact: setState on unmounted component

15. Unbounded Query Results - backend/src/routes/expenses.ts:54

• Severity: HIGH
• Issue: No upper limit on limit parameter
• Code: Client can request limit=999999
• Impact: Memory exhaustion via table dumps

16. No Request Debouncing - frontend/src/pages/ExpenseList.tsx:21

• Severity: MEDIUM
• Issue: API called on every search keystroke
• Impact: Excessive API load and poor UX

17. Inefficient CSV Generation - backend/src/routes/reports.ts:38-43

• Severity: HIGH
• Issue: Loads all expenses into memory simultaneously
• Impact: Out-of-memory errors on large datasets

♿ Accessibility Violations

18. Missing Form Labels - Multiple files

• Severity: MEDIUM
• Files: ExpenseForm.tsx:42,49,57, Login.tsx:33,40, ExpenseList.tsx:38
• Issue: Inputs lack proper labels, only have placeholders
• Impact: Screen readers can't identify form fields

19. Color-Only Information - components/StatusBadge.tsx:8-12

• Severity: MEDIUM
• Issue: Status conveyed by color alone
• Impact: Colorblind users can't distinguish status

20. Missing Semantic Structure - Multiple files

• Severity: MEDIUM
• Issue: Missing <main>, <h1>, proper table headers (<th> vs <td>)
• Impact: Screen readers can't navigate page structure

21. Poor Focus Management - pages/Dashboard.tsx:31-33

• Severity: LOW
• Issue: Navigation links lack visible focus indicators
• Impact: Keyboard users can't see focus position

🌐 Additional Security Issues

22. Wildcard CORS - backend/src/index.ts:11

• Severity: HIGH
• Issue: cors() allows any origin
• Impact: CSRF attacks from malicious sites

23. Information Disclosure - Multiple locations

• Severity: MEDIUM
• Issues: Stack traces leaked to clients (index.ts:23), user enumeration via distinct error messages (auth.ts:57)

24. Insecure Token Storage - frontend/src/api/client.ts:10

• Severity: MEDIUM
• Issue: JWT stored in localStorage (XSS vulnerable)
• Impact: Token theft via XSS attacks

25. No Rate Limiting - backend/src/index.ts:18

• Severity: MEDIUM
• Issue: No rate limiting on any endpoints
• Impact: Brute force attacks on login

🔧 Input Validation Issues

26. Weak Email Validation - backend/src/utils/validate.ts:3

• Severity: LOW
• Issue: Regex accepts invalid emails like "a@b"
• Impact: Invalid email addresses stored in database

27. Case-Sensitive Category Validation - backend/src/utils/validate.ts:14

• Severity: LOW
• Issue: "Travel" vs "travel" rejected
• Impact: Poor user experience

28. Loose Numeric Parsing - frontend/src/pages/ExpenseForm.tsx:18

• Severity: LOW
• Issue: parseFloat("12.34abc") returns 12.34
• Impact: Unexpected numeric values stored

Summary

This review identified 28 distinct issues across security, logic, performance, and accessibility categories. The most critical vulnerabilities are the SQL injection flaws and authentication bypasses, which could lead to complete application compromise. The combination of missing input validation, weak access controls, and client-side vulnerabilities creates multiple attack vectors.

---

[github-actions]

🤖 OpenAI Code Review

Here is a detailed review of the PR diff for the TypeScript expense tracker application:

Backend

File: backend/src/db.ts, Line: 1

• Severity: Low
• Explanation: In a production environment, setting journal_mode = WAL without properly managing database concurrency might lead to issues. However, the direct concern with better-sqlite3 is minimal and this is generally safe for small-scale applications.

File: backend/src/index.ts, Line: 11

• Severity: High
• Explanation: Using cors() without specifying allowed origins allows any domain to make requests to the server. This is a security risk as it permits unauthorized domains to interact with the API and possibly steal data.

File: backend/src/index.ts, Line: 17

• Severity: High
• Explanation: Lack of rate limiting on endpoints makes the server vulnerable to brute-force attacks, potentially compromising data integrity and causing denial of service.

File: backend/src/index.ts, Line: 21

• Severity: High
• Explanation: The global error handler exposes stack traces in production, which is a security risk. Stack traces can provide attackers with details about your codebase and server configuration.

File: backend/src/middleware/auth.ts, Line: 5

• Severity: Critical
• Explanation: The hardcoded JWT secret is a major security vulnerability. Secrets should be stored securely using environment variables to avoid unintentional exposure through source control.

File: backend/src/middleware/auth.ts, Line: 15

• Severity: High
• Explanation: The authenticate middleware does not enforce a bearer scheme for the JWT token. Any string that passes token validation could potentially be accepted.

File: backend/src/middleware/auth.ts, Line: 29

• Severity: High
• Explanation: Checking the user's role from JWT payload is insecure as tokens can be tampered with on the client side. Roles should be verified against the database when performing role-based access control.

File: backend/src/routes/auth.ts, Line: 24

• Severity: Medium
• Explanation: There is no validation for minimum password length or complexity, which weakens password security.

File: backend/src/routes/auth.ts, Line: 44

• Severity: High
• Explanation: JWT tokens have no expiration set, which can pose security risks if tokens are leaked. Tokens should always have a set expiration to limit the window of exposure.

File: backend/src/routes/auth.ts, Line: 63

• Severity: Critical
• Explanation: SQL injection vulnerability exists because user inputs are directly interpolated into the SQL query. Use parameterized queries to ensure safe database interactions.

File: backend/src/routes/auth.ts, Line: 67

• Severity: Medium
• Explanation: The error message indicates whether an email exists or not, which allows user enumeration.

File: backend/src/routes/expenses.ts, Line: 31

• Severity: Medium
• Explanation: There is a race condition when checking totals and inserting expenses. Not putting the check and insert in a transaction makes it possible for concurrent requests to exceed budget limits.

File: backend/src/routes/expenses.ts, Line: 38

• Severity: Low
• Explanation: Off-by-one error; the amount > BUDGET_LIMIT comparison should be >=.

File: backend/src/routes/expenses.ts, Line: 56

• Severity: High
• Explanation: Lack of a sensible limit on pagination allows users to request excessively large datasets, potentially causing server strain.

File: backend/src/routes/expenses.ts, Line: 60

• Severity: Critical
• Explanation: SQL injection vulnerability. User inputs like category should be safely bound to the query using parameterized statements.

File: backend/src/routes/expenses.ts, Line: 82

• Severity: High
• Explanation: The endpoint lacks a check to ensure only admins can approve or reject expenses. Any authenticated user can approve/reject, which is a significant security risk.

File: backend/src/routes/expenses.ts, Line: 94

• Severity: High
• Explanation: Lack of ownership check allows any user to delete any expense, leading to an IDOR (Insecure Direct Object Reference) vulnerability.

File: backend/src/routes/reports.ts, Line: 38

• Severity: High
• Explanation: No pagination or streaming of exported reports means loading all data into memory, which can lead to OOM errors for large datasets.

File: backend/src/routes/reports.ts, Line: 44

• Severity: Medium
• Explanation: Lack of Content-Disposition header in exported CSV will not prompt a download in browsers.

File: backend/src/routes/reports.ts, Line: 49

• Severity: Critical
• Explanation: CSV injection is possible since user-controlled fields are not sanitized. This can lead to command execution in programs like Excel.

Frontend

File: frontend/src/App.tsx, Line: 11

• Severity: High
• Explanation: Simply checking for the presence of a token without validating its authenticity or expiration could allow for expired or manipulated tokens to access protected routes.

File: frontend/src/api/client.ts, Line: 9

• Severity: High
• Explanation: Storing JWTs in localStorage exposes them to XSS attacks. It is recommended to use httpOnly cookies.

File: frontend/src/api/client.ts, Line: 17

• Severity: Medium
• Explanation: Lack of a response interceptor to handle 401 responses fails to automatically redirect unauthorized users back to a login page, showing them raw errors instead.

File: frontend/src/components/ExpenseCard.tsx, Line: 20

• Severity: Low
• Explanation: Using color alone to convey financial status fails accessibility for colorblind users. Additional text should be used.

File: frontend/src/components/ExpenseCard.tsx, Line: 30

• Severity: Critical
• Explanation: Using dangerouslySetInnerHTML without sanitizing input poses severe XSS risks. Descriptions must be sanitized or rendered without using dangerouslySetInnerHTML.

File: frontend/src/components/ExpenseCard.tsx, Line: 45

• Severity: Low
• Explanation: The delete button lacks an aria-label, which reduces ease of use for screen readers.

File: frontend/src/components/StatusBadge.tsx, Line: 10

• Severity: Low
• Explanation: Status color alone without text or symbols is insufficient for colorblind or non-visual users. Use icons or text descriptions.

File: frontend/src/components/StatusBadge.tsx, Line: 16

• Severity: Medium
• Explanation: Lack of role="status" or aria-label means screen readers can't convey the purpose of the badge to users.

File: frontend/src/pages/Dashboard.tsx, Line: 10

• Severity: Medium
• Explanation: Absence of cleanup in useEffect can cause a memory leak if the component unmounts before the request completes.

This concludes the review of the PR. Please address these issues to improve the application's security, performance, and accessibility.