From 5a5a1beefd814f32c75feca0f46d67071ca2930f Mon Sep 17 00:00:00 2001 From: Nick Reeder Date: Tue, 3 Mar 2026 15:00:42 -0800 Subject: [PATCH 1/3] Add app source code with intentional bugs for AI code review comparison Adds full backend (Express/SQLite) and frontend (React) source files with annotated bugs across all review categories: SQL injection, auth vulnerabilities, logic errors, performance/memory leaks, and accessibility violations. Also aligns OpenAI workflow prompt with Claude workflow for fair comparison. Co-Authored-By: Claude Opus 4.6 --- .github/workflows/openai-code-review.yml | 21 +++-- backend/src/db.ts | 30 +++++++ backend/src/index.ts | 30 +++++++ backend/src/middleware/auth.ts | 36 ++++++++ backend/src/routes/auth.ts | 74 ++++++++++++++++ backend/src/routes/expenses.ts | 107 +++++++++++++++++++++++ backend/src/routes/reports.ts | 58 ++++++++++++ backend/src/utils/validate.ts | 15 ++++ frontend/src/App.tsx | 29 ++++++ frontend/src/api/client.ts | 19 ++++ frontend/src/components/ExpenseCard.tsx | 49 +++++++++++ frontend/src/components/StatusBadge.tsx | 31 +++++++ frontend/src/pages/Dashboard.tsx | 61 +++++++++++++ frontend/src/pages/ExpenseForm.tsx | 72 +++++++++++++++ frontend/src/pages/ExpenseList.tsx | 69 +++++++++++++++ frontend/src/pages/Login.tsx | 57 ++++++++++++ frontend/src/pages/Reports.tsx | 67 ++++++++++++++ 17 files changed, 820 insertions(+), 5 deletions(-) create mode 100644 backend/src/db.ts create mode 100644 backend/src/index.ts create mode 100644 backend/src/middleware/auth.ts create mode 100644 backend/src/routes/auth.ts create mode 100644 backend/src/routes/expenses.ts create mode 100644 backend/src/routes/reports.ts create mode 100644 backend/src/utils/validate.ts create mode 100644 frontend/src/App.tsx create mode 100644 frontend/src/api/client.ts create mode 100644 frontend/src/components/ExpenseCard.tsx create mode 100644 frontend/src/components/StatusBadge.tsx create mode 100644 frontend/src/pages/Dashboard.tsx create mode 100644 frontend/src/pages/ExpenseForm.tsx create mode 100644 frontend/src/pages/ExpenseList.tsx create mode 100644 frontend/src/pages/Login.tsx create mode 100644 frontend/src/pages/Reports.tsx diff --git a/.github/workflows/openai-code-review.yml b/.github/workflows/openai-code-review.yml index 6a39009..9304de0 100644 --- a/.github/workflows/openai-code-review.yml +++ b/.github/workflows/openai-code-review.yml @@ -41,13 +41,24 @@ jobs: { "role": "system", "content": ( - "You are a senior code reviewer. Review this PR diff for a TypeScript expense tracker " - "(Node/Express + React + SQLite). Find ALL bugs in these categories: SQL injection/auth " - "vulnerabilities, logic errors, performance/memory leaks, and accessibility issues. " - "For each bug: state the file, approximate line, severity (critical/high/medium/low), and explanation." + "You are reviewing a TypeScript expense tracker app (Node/Express + React + SQLite).\n" + "Focus on:\n" + "1. SQL injection and authentication vulnerabilities\n" + "2. Logic errors and off-by-one bugs\n" + "3. Performance issues and memory leaks\n" + "4. UI/UX and accessibility violations\n" + "Be thorough — flag every issue you find with file, line, severity, and explanation." ) }, - {"role": "user", "content": f"PR Diff:\n\n{diff}"} + { + "role": "user", + "content": ( + "Please review this entire PR for bugs. Focus on SQL injection and auth vulnerabilities, " + "logic errors, performance issues and memory leaks, and UI/UX accessibility violations. " + "Flag every issue with file, line number, severity, and explanation.\n\n" + f"PR Diff:\n\n{diff}" + ) + } ] }).encode() diff --git a/backend/src/db.ts b/backend/src/db.ts new file mode 100644 index 0000000..d103ac5 --- /dev/null +++ b/backend/src/db.ts @@ -0,0 +1,30 @@ +import Database from "better-sqlite3"; +import path from "path"; + +const dbPath = path.join(__dirname, "..", "teamexpense.db"); +const db = new Database(dbPath); + +db.pragma("journal_mode = WAL"); + +db.exec(` + CREATE TABLE IF NOT EXISTS users ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + email TEXT UNIQUE NOT NULL, + password TEXT NOT NULL, + name TEXT NOT NULL, + role TEXT NOT NULL DEFAULT 'member' + ); + + CREATE TABLE IF NOT EXISTS expenses ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + user_id INTEGER NOT NULL, + amount REAL NOT NULL, + description TEXT NOT NULL, + category TEXT NOT NULL, + status TEXT NOT NULL DEFAULT 'pending', + created_at TEXT NOT NULL DEFAULT (datetime('now')), + FOREIGN KEY (user_id) REFERENCES users(id) + ); +`); + +export default db; diff --git a/backend/src/index.ts b/backend/src/index.ts new file mode 100644 index 0000000..a183c70 --- /dev/null +++ b/backend/src/index.ts @@ -0,0 +1,30 @@ +import express from "express"; +import cors from "cors"; +import authRoutes from "./routes/auth"; +import expenseRoutes from "./routes/expenses"; +import reportRoutes from "./routes/reports"; + +const app = express(); +const PORT = process.env.PORT || 3001; + +// BUG: Wildcard CORS — allows any origin to make authenticated requests +app.use(cors()); +app.use(express.json()); + +app.use("/api/auth", authRoutes); +app.use("/api/expenses", expenseRoutes); +app.use("/api/reports", reportRoutes); + +// BUG: No rate limiting on any endpoints — susceptible to brute-force attacks + +// BUG: Global error handler leaks stack traces to the client in production +app.use((err: any, _req: express.Request, res: express.Response, _next: express.NextFunction) => { + console.error(err); + res.status(500).json({ error: err.message, stack: err.stack }); +}); + +app.listen(PORT, () => { + console.log(`Server running on port ${PORT}`); +}); + +export default app; diff --git a/backend/src/middleware/auth.ts b/backend/src/middleware/auth.ts new file mode 100644 index 0000000..1a93055 --- /dev/null +++ b/backend/src/middleware/auth.ts @@ -0,0 +1,36 @@ +import { Request, Response, NextFunction } from "express"; +import jwt from "jsonwebtoken"; + +const JWT_SECRET = "supersecretkey123"; // BUG: Hardcoded JWT secret — should use environment variable + +export interface AuthRequest extends Request { + user?: { id: number; email: string; role: string }; +} + +export function authenticate(req: AuthRequest, res: Response, next: NextFunction) { + const header = req.headers.authorization; + if (!header) { + return res.status(401).json({ error: "No token provided" }); + } + + // BUG: Does not validate "Bearer " prefix — any string with a valid JWT payload passes + const token = header.replace("Bearer ", ""); + + try { + const payload = jwt.verify(token, JWT_SECRET) as any; + req.user = payload; + next(); + } catch { + return res.status(401).json({ error: "Invalid token" }); + } +} + +export function requireAdmin(req: AuthRequest, res: Response, next: NextFunction) { + // BUG: Checks role from the JWT payload which the user controls — should verify role from DB + if (req.user?.role !== "admin") { + return res.status(403).json({ error: "Admin access required" }); + } + next(); +} + +export { JWT_SECRET }; diff --git a/backend/src/routes/auth.ts b/backend/src/routes/auth.ts new file mode 100644 index 0000000..b3cda04 --- /dev/null +++ b/backend/src/routes/auth.ts @@ -0,0 +1,74 @@ +import { Router, Request, Response } from "express"; +import bcrypt from "bcryptjs"; +import jwt from "jsonwebtoken"; +import db from "../db"; +import { JWT_SECRET } from "../middleware/auth"; +import { isValidEmail } from "../utils/validate"; + +const router = Router(); + +// POST /api/auth/register +router.post("/register", async (req: Request, res: Response) => { + const { email, password, name } = req.body; + + if (!email || !password || !name) { + return res.status(400).json({ error: "All fields are required" }); + } + + if (!isValidEmail(email)) { + return res.status(400).json({ error: "Invalid email" }); + } + + // BUG: No minimum password length or complexity check + const hashed = await bcrypt.hash(password, 10); + + try { + const stmt = db.prepare("INSERT INTO users (email, password, name) VALUES (?, ?, ?)"); + const result = stmt.run(email, hashed, name); + + const token = jwt.sign( + { id: result.lastInsertRowid, email, role: "member" }, + JWT_SECRET + // BUG: No token expiration — JWTs are valid forever + ); + + return res.status(201).json({ token, user: { id: result.lastInsertRowid, email, name, role: "member" } }); + } catch (err: any) { + if (err.code === "SQLITE_CONSTRAINT_UNIQUE") { + return res.status(409).json({ error: "Email already registered" }); + } + return res.status(500).json({ error: "Registration failed" }); + } +}); + +// POST /api/auth/login +router.post("/login", async (req: Request, res: Response) => { + const { email, password } = req.body; + + if (!email || !password) { + return res.status(400).json({ error: "Email and password required" }); + } + + // BUG: SQL injection — user input interpolated directly into query + const user = db.prepare(`SELECT * FROM users WHERE email = '${email}'`).get() as any; + + if (!user) { + // BUG: Leaks whether an email exists — allows user enumeration + return res.status(401).json({ error: "No account found with that email" }); + } + + const valid = await bcrypt.compare(password, user.password); + if (!valid) { + return res.status(401).json({ error: "Incorrect password" }); + } + + const token = jwt.sign( + { id: user.id, email: user.email, role: user.role }, + JWT_SECRET + // BUG: No token expiration here either + ); + + return res.json({ token, user: { id: user.id, email: user.email, name: user.name, role: user.role } }); +}); + +export default router; diff --git a/backend/src/routes/expenses.ts b/backend/src/routes/expenses.ts new file mode 100644 index 0000000..05e9561 --- /dev/null +++ b/backend/src/routes/expenses.ts @@ -0,0 +1,107 @@ +import { Router, Response } from "express"; +import db from "../db"; +import { authenticate, AuthRequest } from "../middleware/auth"; +import { isPositiveNumber, isValidCategory } from "../utils/validate"; + +const router = Router(); +router.use(authenticate); + +const BUDGET_LIMIT = 5000; + +// POST /api/expenses +router.post("/", (req: AuthRequest, res: Response) => { + const { amount, description, category } = req.body; + const userId = req.user!.id; + + if (!isPositiveNumber(amount)) { + return res.status(400).json({ error: "Amount must be a positive number" }); + } + + if (!description || typeof description !== "string") { + return res.status(400).json({ error: "Description is required" }); + } + + if (!isValidCategory(category)) { + return res.status(400).json({ error: "Invalid category" }); + } + + // BUG: Race condition — checking total and inserting are not in a transaction, + // so concurrent requests can exceed the budget limit + const row = db.prepare( + "SELECT COALESCE(SUM(amount), 0) as total FROM expenses WHERE user_id = ?" + ).get(userId) as any; + + // BUG: Off-by-one — uses > instead of >= so a user can hit exactly $5000.01 over limit + if (row.total + amount > BUDGET_LIMIT) { + return res.status(400).json({ + error: `Budget limit of $${BUDGET_LIMIT} exceeded. Current total: $${row.total}`, + }); + } + + const stmt = db.prepare( + "INSERT INTO expenses (user_id, amount, description, category) VALUES (?, ?, ?, ?)" + ); + const result = stmt.run(userId, amount, description, category); + + return res.status(201).json({ id: result.lastInsertRowid, amount, description, category, status: "pending" }); +}); + +// GET /api/expenses +router.get("/", (req: AuthRequest, res: Response) => { + const userId = req.user!.id; + const page = parseInt(req.query.page as string) || 1; + const limit = parseInt(req.query.limit as string) || 20; + // BUG: No upper bound on limit — a client can request limit=999999 and dump the entire table + const offset = (page - 1) * limit; + + // BUG: SQL injection — category filter is interpolated directly into the query string + const category = req.query.category as string; + let query = `SELECT * FROM expenses WHERE user_id = ?`; + if (category) { + query += ` AND category = '${category}'`; + } + query += ` ORDER BY created_at DESC LIMIT ${limit} OFFSET ${offset}`; + + const expenses = db.prepare(query).all(userId); + return res.json({ expenses, page, limit }); +}); + +// PATCH /api/expenses/:id/status +router.patch("/:id/status", (req: AuthRequest, res: Response) => { + const { status } = req.body; + const expenseId = req.params.id; + + if (!["approved", "rejected"].includes(status)) { + return res.status(400).json({ error: "Status must be 'approved' or 'rejected'" }); + } + + // BUG: No admin check — any authenticated user can approve/reject any expense + const expense = db.prepare("SELECT * FROM expenses WHERE id = ?").get(expenseId) as any; + + if (!expense) { + return res.status(404).json({ error: "Expense not found" }); + } + + // BUG: Users can approve their own expenses — no self-approval guard + db.prepare("UPDATE expenses SET status = ? WHERE id = ?").run(status, expenseId); + return res.json({ ...expense, status }); +}); + +// DELETE /api/expenses/:id +router.delete("/:id", (req: AuthRequest, res: Response) => { + const expenseId = req.params.id; + const userId = req.user!.id; + + const expense = db.prepare("SELECT * FROM expenses WHERE id = ?").get(expenseId) as any; + + if (!expense) { + return res.status(404).json({ error: "Expense not found" }); + } + + // BUG: IDOR — only checks if expense exists, not if it belongs to the requesting user + // Any authenticated user can delete any other user's expense + db.prepare("DELETE FROM expenses WHERE id = ?").run(expenseId); + return res.json({ message: "Expense deleted" }); +}); + +export default router; diff --git a/backend/src/routes/reports.ts b/backend/src/routes/reports.ts new file mode 100644 index 0000000..61e207a --- /dev/null +++ b/backend/src/routes/reports.ts @@ -0,0 +1,58 @@ +import { Router, Response } from "express"; +import db from "../db"; +import { authenticate, requireAdmin, AuthRequest } from "../middleware/auth"; + +const router = Router(); +router.use(authenticate); + +// GET /api/reports/my-summary +router.get("/my-summary", (req: AuthRequest, res: Response) => { + const userId = req.user!.id; + + const rows = db.prepare(` + SELECT category, COUNT(*) as count, SUM(amount) as total + FROM expenses + WHERE user_id = ? + GROUP BY category + `).all(userId); + + return res.json({ summary: rows }); +}); + +// GET /api/reports/team-summary +router.get("/team-summary", requireAdmin, (req: AuthRequest, res: Response) => { + const rows = db.prepare(` + SELECT u.name, u.email, COUNT(e.id) as count, SUM(e.amount) as total + FROM users u + LEFT JOIN expenses e ON u.id = e.user_id + GROUP BY u.id + `).all(); + + return res.json({ summary: rows }); +}); + +// GET /api/reports/export +router.get("/export", requireAdmin, (req: AuthRequest, res: Response) => { + // BUG: No pagination or streaming — loads ALL expenses into memory at once + // Will cause OOM on large datasets + const expenses = db.prepare(` + SELECT e.*, u.name as user_name, u.email as user_email + FROM expenses e + JOIN users u ON e.user_id = u.id + ORDER BY e.created_at DESC + `).all(); + + // BUG: No Content-Disposition header — browser won't prompt download + res.setHeader("Content-Type", "text/csv"); + + // BUG: CSV injection — user-controlled fields (description, name) are not escaped. + // A description like '=CMD("calc")' will execute in Excel when opened. + let csv = "ID,User,Email,Amount,Description,Category,Status,Date\n"; + for (const e of expenses as any[]) { + csv += `${e.id},${e.user_name},${e.user_email},${e.amount},${e.description},${e.category},${e.status},${e.created_at}\n`; + } + + return res.send(csv); +}); + +export default router; diff --git a/backend/src/utils/validate.ts b/backend/src/utils/validate.ts new file mode 100644 index 0000000..20b5d40 --- /dev/null +++ b/backend/src/utils/validate.ts @@ -0,0 +1,15 @@ +export function isValidEmail(email: string): boolean { + // BUG: Overly permissive regex — accepts strings like "a@b" without a TLD + return /^[^\s@]+@[^\s@]+$/.test(email); +} + +export function isPositiveNumber(value: unknown): value is number { + return typeof value === "number" && value > 0; +} + +const VALID_CATEGORIES = ["travel", "meals", "supplies", "software", "other"]; + +export function isValidCategory(category: string): boolean { + // BUG: Case-sensitive comparison — "Travel" or "MEALS" will be rejected + return VALID_CATEGORIES.includes(category); +} diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx new file mode 100644 index 0000000..e88ce34 --- /dev/null +++ b/frontend/src/App.tsx @@ -0,0 +1,29 @@ +import React from "react"; +import { BrowserRouter, Routes, Route, Navigate } from "react-router-dom"; +import Login from "./pages/Login"; +import Dashboard from "./pages/Dashboard"; +import ExpenseForm from "./pages/ExpenseForm"; +import ExpenseList from "./pages/ExpenseList"; +import Reports from "./pages/Reports"; + +function PrivateRoute({ children }: { children: React.ReactNode }) { + // BUG: Only checks if token exists, not if it's valid or expired + // An expired or tampered token will pass this check + const token = localStorage.getItem("token"); + return token ? <>{children} : ; +} + +export default function App() { + return ( + + + } /> + } /> + } /> + } /> + } /> + } /> + + + ); +} diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts new file mode 100644 index 0000000..584fafe --- /dev/null +++ b/frontend/src/api/client.ts @@ -0,0 +1,19 @@ +import axios from "axios"; + +const client = axios.create({ + baseURL: "http://localhost:3001/api", +}); + +client.interceptors.request.use((config) => { + // BUG: Reads token from localStorage which is vulnerable to XSS attacks + // Should use httpOnly cookies instead + const token = localStorage.getItem("token"); + if (token) { + config.headers.Authorization = `Bearer ${token}`; + } + return config; +}); + +// BUG: No response interceptor to handle 401s — if the token expires, +// the user sees raw errors instead of being redirected to login +export default client; diff --git a/frontend/src/components/ExpenseCard.tsx b/frontend/src/components/ExpenseCard.tsx new file mode 100644 index 0000000..4202cf1 --- /dev/null +++ b/frontend/src/components/ExpenseCard.tsx @@ -0,0 +1,49 @@ +import React from "react"; +import StatusBadge from "./StatusBadge"; + +interface Props { + expense: { + id: number; + amount: number; + description: string; + category: string; + status: string; + created_at: string; + }; + onDelete: (id: number) => void; +} + +export default function ExpenseCard({ expense, onDelete }: Props) { + return ( + // BUG (a11y): Card is not an
or list item — no semantic structure +
+
+ {/* BUG (a11y): Amount displayed with color alone to indicate high values — no text/icon alternative */} +
500 ? "red" : "green" }}> + ${expense.amount.toFixed(2)} +
+ +
+ + {/* BUG: Renders description as raw HTML via dangerouslySetInnerHTML — XSS vulnerability + If a user submits '' as a description, it executes */} +
+ +
+ {expense.category} + {expense.created_at} +
+ + {/* BUG (a11y): Delete button has no aria-label — screen reader just says "X" with no context */} + +
+ ); +} diff --git a/frontend/src/components/StatusBadge.tsx b/frontend/src/components/StatusBadge.tsx new file mode 100644 index 0000000..5fcb772 --- /dev/null +++ b/frontend/src/components/StatusBadge.tsx @@ -0,0 +1,31 @@ +import React from "react"; + +interface Props { + status: string; +} + +// BUG (a11y): Status conveyed by color alone — no icon or text pattern for colorblind users +const STATUS_COLORS: Record = { + pending: "#f0ad4e", + approved: "#5cb85c", + rejected: "#d9534f", +}; + +export default function StatusBadge({ status }: Props) { + return ( + // BUG (a11y): No role="status" or aria-label — screen readers get no context + + {status} + + ); +} diff --git a/frontend/src/pages/Dashboard.tsx b/frontend/src/pages/Dashboard.tsx new file mode 100644 index 0000000..35ed570 --- /dev/null +++ b/frontend/src/pages/Dashboard.tsx @@ -0,0 +1,61 @@ +import React, { useEffect, useState } from "react"; +import { Link } from "react-router-dom"; +import client from "../api/client"; + +export default function Dashboard() { + const [summary, setSummary] = useState([]); + + useEffect(() => { + // BUG: No cleanup/abort — if the component unmounts before the request completes, + // this will attempt to setState on an unmounted component (memory leak) + client.get("/reports/my-summary").then((res) => { + setSummary(res.data.summary); + }); + }, []); + + const total = summary.reduce((sum, row) => sum + row.total, 0); + + return ( + // BUG (a11y): No
landmark wrapping page content +
+
Dashboard
+ +
+ {/* BUG (a11y): Important status info not in a landmark or live region */} +
Total Spending: ${total.toFixed(2)}
+
Budget Remaining: ${(5000 - total).toFixed(2)}
+
+ + {/* BUG (a11y): Navigation links styled as plain text — no visual focus indicator */} +
+ New Expense + View All + Reports +
+ + {summary.length > 0 && ( + // BUG (a11y): Table has no or aria-label describing its purpose + + + + {/* BUG (a11y): Uses + + + + + + {summary.map((row: any) => ( + // BUG: Using array index as key implicitly — category could be a better key + + + + + + ))} + +
instead of for header cells — no scope attribute */} + CategoryCountTotal
{row.category}{row.count}${row.total.toFixed(2)}
+ )} +
+ ); +} diff --git a/frontend/src/pages/ExpenseForm.tsx b/frontend/src/pages/ExpenseForm.tsx new file mode 100644 index 0000000..f3f622a --- /dev/null +++ b/frontend/src/pages/ExpenseForm.tsx @@ -0,0 +1,72 @@ +import React, { useState } from "react"; +import { useNavigate } from "react-router-dom"; +import client from "../api/client"; + +const CATEGORIES = ["travel", "meals", "supplies", "software", "other"]; + +export default function ExpenseForm() { + const [amount, setAmount] = useState(""); + const [description, setDescription] = useState(""); + const [category, setCategory] = useState(CATEGORIES[0]); + const [error, setError] = useState(""); + const navigate = useNavigate(); + + const handleSubmit = async (e: React.FormEvent) => { + e.preventDefault(); + + // BUG: parseFloat("12.34abc") returns 12.34 — no strict numeric validation + const parsed = parseFloat(amount); + if (isNaN(parsed) || parsed <= 0) { + setError("Enter a valid positive amount"); + return; + } + + // BUG: No client-side XSS sanitization on description — stored XSS if server also lacks sanitization + try { + await client.post("/expenses", { amount: parsed, description, category }); + navigate("/expenses"); + } catch (err: any) { + setError(err.response?.data?.error || "Failed to create expense"); + } + }; + + return ( + // BUG (a11y): No
landmark, no page heading +
+
New Expense
+ + {error &&
{error}
} + +
+ {/* BUG (a11y): No
+ ); +} diff --git a/frontend/src/pages/ExpenseList.tsx b/frontend/src/pages/ExpenseList.tsx new file mode 100644 index 0000000..defed48 --- /dev/null +++ b/frontend/src/pages/ExpenseList.tsx @@ -0,0 +1,69 @@ +import React, { useEffect, useState } from "react"; +import client from "../api/client"; +import ExpenseCard from "../components/ExpenseCard"; + +interface Expense { + id: number; + amount: number; + description: string; + category: string; + status: string; + created_at: string; +} + +export default function ExpenseList() { + const [expenses, setExpenses] = useState([]); + const [page, setPage] = useState(1); + const [search, setSearch] = useState(""); + + useEffect(() => { + // BUG: Fetches on every keystroke in search — no debounce, hammers the API + client.get("/expenses", { params: { page, category: search || undefined } }).then((res) => { + setExpenses(res.data.expenses); + }); + }, [page, search]); + + const handleDelete = async (id: number) => { + // BUG: No confirmation dialog before delete — one click permanently removes data + await client.delete(`/expenses/${id}`); + setExpenses(expenses.filter((e) => e.id !== id)); + }; + + return ( + // BUG (a11y): No
landmark +
+
Expenses
+ + {/* BUG (a11y): Search input has no label or aria-label */} + setSearch(e.target.value)} + style={{ width: "100%", padding: 8, marginBottom: 20 }} + /> + + {expenses.length === 0 ? ( +
No expenses found.
+ ) : ( + // BUG (a11y): List of expenses not wrapped in a
    /
      — screen readers can't navigate as a list +
      + {expenses.map((expense) => ( + + ))} +
      + )} + + {/* BUG (a11y): Pagination buttons have no aria-label describing their action */} +
      + + Page {page} + +
      +
+ ); +} diff --git a/frontend/src/pages/Login.tsx b/frontend/src/pages/Login.tsx new file mode 100644 index 0000000..b6d90cd --- /dev/null +++ b/frontend/src/pages/Login.tsx @@ -0,0 +1,57 @@ +import React, { useState } from "react"; +import { useNavigate } from "react-router-dom"; +import client from "../api/client"; + +export default function Login() { + const [email, setEmail] = useState(""); + const [password, setPassword] = useState(""); + const [error, setError] = useState(""); + const navigate = useNavigate(); + + const handleSubmit = async (e: React.FormEvent) => { + e.preventDefault(); + try { + const { data } = await client.post("/auth/login", { email, password }); + localStorage.setItem("token", data.token); + navigate("/dashboard"); + } catch (err: any) { + // BUG: Displays raw server error message to user — may leak internal details + setError(err.response?.data?.error || "Login failed"); + } + }; + + return ( + // BUG (a11y): No
landmark, no

heading, no skip-navigation link +
+
Team Expense Login
+ + {/* BUG (a11y): Error message has no role="alert" or aria-live — screen readers won't announce it */} + {error &&
{error}
} + +
+ {/* BUG (a11y): Inputs have no associated
+ ); +} diff --git a/frontend/src/pages/Reports.tsx b/frontend/src/pages/Reports.tsx new file mode 100644 index 0000000..707ebf5 --- /dev/null +++ b/frontend/src/pages/Reports.tsx @@ -0,0 +1,67 @@ +import React, { useEffect, useState } from "react"; +import client from "../api/client"; + +export default function Reports() { + const [mySummary, setMySummary] = useState([]); + const [teamSummary, setTeamSummary] = useState([]); + + useEffect(() => { + // BUG: Both requests fire even if user is not admin — team-summary will 403 + // and the error is silently swallowed + client.get("/reports/my-summary").then((res) => setMySummary(res.data.summary)); + client.get("/reports/team-summary").then((res) => setTeamSummary(res.data.summary)).catch(() => {}); + }, []); + + return ( +
+
Reports
+ + {/* BUG (a11y): Section has no heading hierarchy — goes from page title to table directly */} +
+
My Spending
+ + + + + + + + + {mySummary.map((row: any) => ( + + + + + ))} + +
CategoryTotal
{row.category}${row.total?.toFixed(2)}
+
+ + {teamSummary.length > 0 && ( +
+
Team Spending
+ {/* BUG (a11y): Table has no caption and uses for headers */} + + + + + + + + + + {teamSummary.map((row: any) => ( + + + + {/* BUG: row.total can be null (user with no expenses) — toFixed() will throw */} + + + ))} + +
NameExpensesTotal
{row.name}{row.count}${row.total.toFixed(2)}
+
+ )} +
+ ); +} From ee514fe1ad3831bf856d6450b07075e89e5498d5 Mon Sep 17 00:00:00 2001 From: Nick Reeder Date: Tue, 3 Mar 2026 15:13:43 -0800 Subject: [PATCH 2/3] Update both review workflows to post inline comments - Claude: instruct action to post inline review comments per finding - OpenAI: request structured JSON, use pulls.createReview for inline comments with fallback to table format if line mapping fails --- .github/workflows/claude-code-review.yml | 11 ++- .github/workflows/openai-code-review.yml | 97 +++++++++++++++++++----- 2 files changed, 86 insertions(+), 22 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 550277b..b05a3e3 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -13,7 +13,7 @@ jobs: contents: read pull-requests: write issues: write - id-token: write # ← was missing, required by the action + id-token: write steps: - name: Checkout uses: actions/checkout@v4 @@ -24,11 +24,17 @@ jobs: uses: anthropics/claude-code-action@beta with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} - github_token: ${{ secrets.GITHUB_TOKEN }} # ← was missing + github_token: ${{ secrets.GITHUB_TOKEN }} direct_prompt: | Please review this entire PR for bugs. Focus on SQL injection and auth vulnerabilities, logic errors, performance issues and memory leaks, and UI/UX accessibility violations. Flag every issue with file, line number, severity, and explanation. + + IMPORTANT: For each issue you find, post an inline review comment on the specific line + of the file where the issue occurs. Do NOT put all findings in a single comment — use + individual inline comments so each issue appears directly on the relevant line in the diff. + After posting all inline comments, post a brief summary comment listing the total count + of issues found by severity. trigger_phrase: "@claude" custom_instructions: | You are reviewing a TypeScript expense tracker app (Node/Express + React + SQLite). @@ -38,3 +44,4 @@ jobs: 3. Performance issues and memory leaks 4. UI/UX and accessibility violations Be thorough — flag every issue you find with file, line, severity, and explanation. + Always post findings as inline review comments on the exact lines where the issues are. diff --git a/.github/workflows/openai-code-review.yml b/.github/workflows/openai-code-review.yml index 9304de0..bbedfd9 100644 --- a/.github/workflows/openai-code-review.yml +++ b/.github/workflows/openai-code-review.yml @@ -22,13 +22,9 @@ jobs: git diff origin/${{ github.base_ref }}...HEAD > pr_diff.txt - name: OpenAI Review via API - id: review env: OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} run: | - DIFF=$(cat pr_diff.txt | head -c 20000) - - # Write request body to a temp file to avoid shell escaping issues python3 - <<'PYEOF' import os, json, urllib.request, sys @@ -37,6 +33,7 @@ jobs: payload = json.dumps({ "model": "gpt-4o", + "response_format": {"type": "json_object"}, "messages": [ { "role": "system", @@ -47,7 +44,12 @@ jobs: "2. Logic errors and off-by-one bugs\n" "3. Performance issues and memory leaks\n" "4. UI/UX and accessibility violations\n" - "Be thorough — flag every issue you find with file, line, severity, and explanation." + "Be thorough — flag every issue you find with file, line, severity, and explanation.\n\n" + "You MUST respond with a JSON object in this exact format:\n" + '{"comments": [{"path": "relative/file/path.ts", "line": 42, "severity": "critical|high|medium|low", "body": "explanation of the issue"}]}\n' + "The path must match the file path in the diff (e.g. backend/src/routes/auth.ts).\n" + "The line number must be a line that exists in the NEW version of the file in the diff.\n" + "Return ONLY the JSON object, no other text." ) }, { @@ -74,28 +76,83 @@ jobs: try: with urllib.request.urlopen(req) as resp: data = json.loads(resp.read()) - review = data["choices"][0]["message"]["content"] + review_json = data["choices"][0]["message"]["content"] except urllib.error.HTTPError as e: body = e.read().decode() print(f"OpenAI API error {e.code}: {body}", file=sys.stderr) - review = f"⚠️ OpenAI API error {e.code} — check that OPENAI_API_KEY secret is set correctly.\n\n{body}" + with open("review_result.json", "w") as f: + json.dump({"error": f"OpenAI API error {e.code}: {body}"}, f) + sys.exit(1) - with open(os.environ["GITHUB_ENV"], "a") as f: - f.write("OPENAI_REVIEW< ({ + path: c.path, + line: c.line, + body: `**[${c.severity.toUpperCase()}]** ${c.body}` + })); + + // Count by severity for summary + const counts = {}; + comments.forEach(c => { + const s = c.severity.toLowerCase(); + counts[s] = (counts[s] || 0) + 1; }); - env: - OPENAI_REVIEW: ${{ env.OPENAI_REVIEW }} + const summary = Object.entries(counts) + .map(([s, n]) => `- **${s}**: ${n}`) + .join('\n'); + + try { + await github.rest.pulls.createReview({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.issue.number, + event: 'COMMENT', + body: `## OpenAI Code Review\n\nFound **${comments.length}** issues:\n${summary}`, + comments: reviewComments + }); + } catch (err) { + // If inline comments fail (e.g. line not in diff), fall back to single comment + let body = `## OpenAI Code Review\n\nFound **${comments.length}** issues:\n${summary}\n\n`; + body += '| File | Line | Severity | Issue |\n|------|------|----------|-------|\n'; + comments.forEach(c => { + body += `| \`${c.path}\` | ${c.line} | ${c.severity} | ${c.body} |\n`; + }); + await github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body + }); + } From 33e03d53adf5c1c5fc3f850794b0605e57b5626d Mon Sep 17 00:00:00 2001 From: Nick Reeder Date: Wed, 4 Mar 2026 13:21:31 -0800 Subject: [PATCH 3/3] Add CodeRabbit configuration for automated code reviews Configure assertive review profile with path-specific instructions, security/performance quality gates, sequence diagrams, and auto-review for the expense tracker app. --- .coderabbit.yaml | 152 ++++++++++++++++++++++ .github/workflows/claude-code-review.yml | 47 ------- .github/workflows/openai-code-review.yml | 158 ----------------------- 3 files changed, 152 insertions(+), 205 deletions(-) create mode 100644 .coderabbit.yaml delete mode 100644 .github/workflows/claude-code-review.yml delete mode 100644 .github/workflows/openai-code-review.yml diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 0000000..ab1184f --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,152 @@ +# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json + +language: en-US +tone_instructions: > + Be thorough and professional. Provide actionable feedback with clear explanations + of why a change is recommended. When identifying security or performance issues, + include the potential impact and a suggested fix. Use a constructive, mentor-like tone. + +early_access: true + +reviews: + profile: assertive + + # Summaries & visuals + high_level_summary: true + high_level_summary_instructions: > + Provide a concise executive summary covering: (1) what changed and why, + (2) architectural impact, (3) security or performance concerns, + and (4) any breaking changes. Include a risk assessment (low/medium/high). + high_level_summary_in_walkthrough: true + changed_files_summary: true + sequence_diagrams: true + collapse_walkthrough: false + review_status: true + + # Effort & collaboration + estimate_code_review_effort: true + assess_linked_issues: true + related_issues: true + related_prs: true + suggested_labels: true + suggested_reviewers: true + + # Disable fluff + poem: false + in_progress_fortune: false + + # Auto-review settings + auto_review: + enabled: true + drafts: false + auto_incremental_review: true + base_branches: + - main + - develop + ignore_title_keywords: + - WIP + - DO NOT MERGE + - DRAFT + + # Path filters — focus on source code + path_filters: + include: + - "backend/src/**" + - "frontend/src/**" + - "package.json" + - "tsconfig.json" + exclude: + - "node_modules/**" + - "dist/**" + - "build/**" + - "coverage/**" + - "*.min.js" + - "*.lock" + - "*.map" + + # Path-specific review instructions + path_instructions: + - path: "backend/src/routes/**" + instructions: > + API routes: Verify input validation, proper HTTP status codes, + authentication/authorization checks, and SQL injection prevention. + Flag any raw SQL queries that don't use parameterized statements. + - path: "backend/src/middleware/**" + instructions: > + Middleware: Check JWT validation logic, error handling, and + ensure auth bypass is not possible. Verify token expiry handling. + - path: "backend/src/db.ts" + instructions: > + Database layer: Check for SQL injection, proper connection handling, + and data integrity constraints. Verify migrations are safe. + - path: "backend/src/utils/**" + instructions: > + Utility functions: Verify input sanitization, edge case handling, + and that validation logic is comprehensive and consistent. + - path: "frontend/src/pages/**" + instructions: > + Pages: Check for proper state management, error boundaries, + loading states, and accessibility (ARIA labels, keyboard navigation). + - path: "frontend/src/components/**" + instructions: > + Components: Verify React best practices — proper hook usage, + memoization where needed, prop validation, and XSS prevention. + - path: "frontend/src/api/**" + instructions: > + API client: Check error handling, request/response interceptors, + auth token attachment, and that sensitive data is not logged. + + # Pre-merge quality gates + pre_merge_checks: + title: + mode: warning + requirements: "Use conventional commits format (feat:, fix:, refactor:, docs:, test:, chore:)" + description: + mode: warning + docstrings: + mode: warning + threshold: 70 + custom_checks: + - name: "Security" + mode: error + instructions: > + Check for hardcoded secrets, credentials, API keys, or tokens. + Verify no sensitive data is exposed in logs or error messages. + Flag any use of eval(), innerHTML, or dangerouslySetInnerHTML. + - name: "Performance" + mode: warning + instructions: > + Identify potential performance issues: N+1 queries, missing + database indexes, unbounded queries, memory leaks in React + components (missing cleanup in useEffect), and unnecessary re-renders. + + # Finishing touches — automated improvements + finishing_touches: + docstrings: + enabled: true + unit_tests: + enabled: true + + # Linting & analysis tools + tools: + eslint: + enabled: true + biome: + enabled: true + semgrep: + enabled: true + github-checks: + enabled: true + timeout_ms: 90000 + +chat: + auto_reply: true + +knowledge_base: + opt_out: false + web_search: + enabled: true + learnings: + scope: auto + issues: + scope: auto diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml deleted file mode 100644 index b05a3e3..0000000 --- a/.github/workflows/claude-code-review.yml +++ /dev/null @@ -1,47 +0,0 @@ -name: Claude Code Review - -on: - pull_request: - types: [opened, synchronize, reopened] - issue_comment: - types: [created] - -jobs: - claude-review: - runs-on: ubuntu-latest - permissions: - contents: read - pull-requests: write - issues: write - id-token: write - steps: - - name: Checkout - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - - name: Claude Code Review - uses: anthropics/claude-code-action@beta - with: - anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} - github_token: ${{ secrets.GITHUB_TOKEN }} - direct_prompt: | - Please review this entire PR for bugs. Focus on SQL injection and auth vulnerabilities, - logic errors, performance issues and memory leaks, and UI/UX accessibility violations. - Flag every issue with file, line number, severity, and explanation. - - IMPORTANT: For each issue you find, post an inline review comment on the specific line - of the file where the issue occurs. Do NOT put all findings in a single comment — use - individual inline comments so each issue appears directly on the relevant line in the diff. - After posting all inline comments, post a brief summary comment listing the total count - of issues found by severity. - trigger_phrase: "@claude" - custom_instructions: | - You are reviewing a TypeScript expense tracker app (Node/Express + React + SQLite). - Focus on: - 1. SQL injection and authentication vulnerabilities - 2. Logic errors and off-by-one bugs - 3. Performance issues and memory leaks - 4. UI/UX and accessibility violations - Be thorough — flag every issue you find with file, line, severity, and explanation. - Always post findings as inline review comments on the exact lines where the issues are. diff --git a/.github/workflows/openai-code-review.yml b/.github/workflows/openai-code-review.yml deleted file mode 100644 index bbedfd9..0000000 --- a/.github/workflows/openai-code-review.yml +++ /dev/null @@ -1,158 +0,0 @@ -name: OpenAI Code Review - -on: - pull_request: - types: [opened, synchronize, reopened] - -jobs: - openai-review: - runs-on: ubuntu-latest - permissions: - contents: read - pull-requests: write - - steps: - - name: Checkout - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - - name: Get PR Diff - run: | - git diff origin/${{ github.base_ref }}...HEAD > pr_diff.txt - - - name: OpenAI Review via API - env: - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} - run: | - python3 - <<'PYEOF' - import os, json, urllib.request, sys - - diff = open("pr_diff.txt").read()[:20000] - api_key = os.environ["OPENAI_API_KEY"] - - payload = json.dumps({ - "model": "gpt-4o", - "response_format": {"type": "json_object"}, - "messages": [ - { - "role": "system", - "content": ( - "You are reviewing a TypeScript expense tracker app (Node/Express + React + SQLite).\n" - "Focus on:\n" - "1. SQL injection and authentication vulnerabilities\n" - "2. Logic errors and off-by-one bugs\n" - "3. Performance issues and memory leaks\n" - "4. UI/UX and accessibility violations\n" - "Be thorough — flag every issue you find with file, line, severity, and explanation.\n\n" - "You MUST respond with a JSON object in this exact format:\n" - '{"comments": [{"path": "relative/file/path.ts", "line": 42, "severity": "critical|high|medium|low", "body": "explanation of the issue"}]}\n' - "The path must match the file path in the diff (e.g. backend/src/routes/auth.ts).\n" - "The line number must be a line that exists in the NEW version of the file in the diff.\n" - "Return ONLY the JSON object, no other text." - ) - }, - { - "role": "user", - "content": ( - "Please review this entire PR for bugs. Focus on SQL injection and auth vulnerabilities, " - "logic errors, performance issues and memory leaks, and UI/UX accessibility violations. " - "Flag every issue with file, line number, severity, and explanation.\n\n" - f"PR Diff:\n\n{diff}" - ) - } - ] - }).encode() - - req = urllib.request.Request( - "https://api.openai.com/v1/chat/completions", - data=payload, - headers={ - "Authorization": f"Bearer {api_key}", - "Content-Type": "application/json" - } - ) - - try: - with urllib.request.urlopen(req) as resp: - data = json.loads(resp.read()) - review_json = data["choices"][0]["message"]["content"] - except urllib.error.HTTPError as e: - body = e.read().decode() - print(f"OpenAI API error {e.code}: {body}", file=sys.stderr) - with open("review_result.json", "w") as f: - json.dump({"error": f"OpenAI API error {e.code}: {body}"}, f) - sys.exit(1) - - with open("review_result.json", "w") as f: - f.write(review_json) - PYEOF - - - name: Post Inline Review Comments - uses: actions/github-script@v7 - with: - script: | - const fs = require('fs'); - const result = JSON.parse(fs.readFileSync('review_result.json', 'utf8')); - - if (result.error) { - await github.rest.issues.createComment({ - issue_number: context.issue.number, - owner: context.repo.owner, - repo: context.repo.repo, - body: `## OpenAI Code Review\n\n⚠️ ${result.error}` - }); - return; - } - - const comments = result.comments || []; - if (comments.length === 0) { - await github.rest.issues.createComment({ - issue_number: context.issue.number, - owner: context.repo.owner, - repo: context.repo.repo, - body: '## OpenAI Code Review\n\nNo issues found.' - }); - return; - } - - // Build inline comments for the review - const reviewComments = comments.map(c => ({ - path: c.path, - line: c.line, - body: `**[${c.severity.toUpperCase()}]** ${c.body}` - })); - - // Count by severity for summary - const counts = {}; - comments.forEach(c => { - const s = c.severity.toLowerCase(); - counts[s] = (counts[s] || 0) + 1; - }); - const summary = Object.entries(counts) - .map(([s, n]) => `- **${s}**: ${n}`) - .join('\n'); - - try { - await github.rest.pulls.createReview({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.issue.number, - event: 'COMMENT', - body: `## OpenAI Code Review\n\nFound **${comments.length}** issues:\n${summary}`, - comments: reviewComments - }); - } catch (err) { - // If inline comments fail (e.g. line not in diff), fall back to single comment - let body = `## OpenAI Code Review\n\nFound **${comments.length}** issues:\n${summary}\n\n`; - body += '| File | Line | Severity | Issue |\n|------|------|----------|-------|\n'; - comments.forEach(c => { - body += `| \`${c.path}\` | ${c.line} | ${c.severity} | ${c.body} |\n`; - }); - await github.rest.issues.createComment({ - issue_number: context.issue.number, - owner: context.repo.owner, - repo: context.repo.repo, - body - }); - }