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 6a39009..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,17 +33,34 @@ jobs: payload = json.dumps({ "model": "gpt-4o", + "response_format": {"type": "json_object"}, "messages": [ { "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.\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": 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() @@ -63,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 + }); + } 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)}
+
+ )} +
+ ); +}