Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions backend/src/db.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import Database from "better-sqlite3";
import path from "path";

const db = new Database(path.join(__dirname, "..", "teamexpense.db"));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check whether the DB file or *.db pattern is excluded in .gitignore
fd -g ".gitignore" --exec grep -n "\.db\|teamexpense" {} +

Repository: reeder32/teamexpense

Length of output: 271


🏁 Script executed:

#!/bin/bash
# Check if teamexpense.db file exists in the repository and if it's tracked by git
echo "=== Checking for database files in repository ==="
find . -name "teamexpense.db" -o -name "*.db" 2>/dev/null | head -20

echo -e "\n=== Checking git status for tracked db files ==="
git ls-files | grep -i "\.db\|teamexpense" || echo "No .db files found in git index"

echo -e "\n=== Check .gitignore at repository root ==="
if [ -f ".gitignore" ]; then
  cat .gitignore | grep -n "\.db\|teamexpense" || echo "No db patterns found in root .gitignore"
else
  echo ".gitignore not found at repository root"
fi

echo -e "\n=== Check for backend/.gitignore ==="
if [ -f "backend/.gitignore" ]; then
  cat backend/.gitignore | grep -n "\.db\|teamexpense" || echo "No db patterns found in backend/.gitignore"
else
  echo "backend/.gitignore not found"
fi

Repository: reeder32/teamexpense

Length of output: 336


🏁 Script executed:

#!/bin/bash
# Verify the actual file structure and path resolution context
echo "=== Backend directory structure ==="
ls -la backend/ 2>/dev/null | grep -E "\.db|\.gitignore|src" | head -20

echo -e "\n=== Check if backend/src/db.ts exists ==="
if [ -f "backend/src/db.ts" ]; then
  echo "File found. Content around line 4:"
  sed -n '1,10p' backend/src/db.ts
else
  echo "File not found"
fi

Repository: reeder32/teamexpense

Length of output: 629


Add .db-shm and .db-wal patterns to .gitignore.

The root .gitignore already excludes *.db, but SQLite's WAL (Write-Ahead Logging) support—enabled on line 6 (db.pragma("journal_mode = WAL"))—creates .db-shm and .db-wal files that are currently tracked in git (backend/teamexpense.db-shm and backend/teamexpense.db-wal). These are environment-specific database state files and should be excluded. Update .gitignore to also exclude *.db-shm and *.db-wal.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/db.ts` at line 4, The repository currently ignores *.db but not
SQLite WAL files; because backend/src/db.ts creates the Database instance (const
db = new Database(...)) and enables WAL with db.pragma("journal_mode = WAL"),
add patterns to the root .gitignore to exclude environment-specific WAL files by
adding entries for *.db-shm and *.db-wal so backend/teamexpense.db-shm and
backend/teamexpense.db-wal are not tracked.


db.pragma("journal_mode = WAL");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Foreign key enforcement is disabled — the REFERENCES users(id) constraint is silently ignored.

SQLite disables FK enforcement by default. Without PRAGMA foreign_keys = ON, the FOREIGN KEY (user_id) REFERENCES users(id) declaration has no effect: rows with dangling user_id values can be inserted and users can be deleted leaving orphaned expense rows.

🔧 Proposed fix
 db.pragma("journal_mode = WAL");
+db.pragma("foreign_keys = ON");

Note: this pragma must be set on every connection opening (it is not persisted). Since better-sqlite3 uses a single synchronous connection, adding it here is sufficient.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
db.pragma("journal_mode = WAL");
db.pragma("journal_mode = WAL");
db.pragma("foreign_keys = ON");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/db.ts` at line 6, The database connection enables WAL but not
foreign key enforcement, so the FOREIGN KEY constraints are ignored; update the
connection initialization where the sqlite connection object db is created (the
same place calling db.pragma("journal_mode = WAL")) to also run
db.pragma("foreign_keys = ON") so foreign key checks are enabled for this
connection (remember this pragma must be set on each connection open).


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;
23 changes: 23 additions & 0 deletions backend/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
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 = 3001;

app.use(cors());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CORS allows all origins - attackers can make requests from malicious sites

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Restrict CORS to known origins.

cors() with no configuration emits Access-Control-Allow-Origin: *, allowing any website to call your authenticated API endpoints. To avoid exposing sensitive data to unintended origins, specify origins in an allowlist rather than using the allow-all wildcard. Even though the wildcard blocks credentialed requests by spec, it still permits reading any unauthenticated response body from any origin.

🛡️ Proposed fix
-app.use(cors());
+app.use(cors({
+  origin: process.env.CORS_ORIGIN || "http://localhost:3000",
+  methods: ["GET", "POST", "PUT", "PATCH", "DELETE"],
+}));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
app.use(cors());
app.use(cors({
origin: process.env.CORS_ORIGIN || "http://localhost:3000",
methods: ["GET", "POST", "PUT", "PATCH", "DELETE"],
}));
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/index.ts` at line 10, The current call app.use(cors()) allows all
origins; restrict CORS by replacing the wildcard with a configured allowlist:
define an array of allowedOrigins and pass a cors options object (or origin
callback) to cors() that checks the incoming Origin against that list and either
allows or rejects it, and set credentials: true if you need to support
cookies/auth; update the middleware instantiation where app.use(cors()) appears
to use this allowlist-based options so only known origins can access
authenticated endpoints.

app.use(express.json());

app.use("/api/auth", authRoutes);
app.use("/api/expenses", expenseRoutes);
app.use("/api/reports", reportRoutes);

app.get("/api/health", (_, res) => {
res.json({ status: "ok" });
});

app.listen(PORT, () => {
console.log(`Server running on http://localhost:${PORT}`);
});
32 changes: 32 additions & 0 deletions backend/src/middleware/auth.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { Request, Response, NextFunction } from "express";
import jwt from "jsonwebtoken";

const JWT_SECRET = "secret123";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded JWT secret exposes all tokens to compromise - must use environment variable

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

6. Hardcoded jwt secret 🐞 Bug ⛨ Security

JWT signing/verification uses a hardcoded secret (secret123). If the code leaks or is guessable,
attackers can forge tokens and become any user/admin.
Agent Prompt
### Issue description
JWT secret is hardcoded, enabling token forgery and blocking secret rotation.

### Issue Context
Both signing and verification depend on `JWT_SECRET`.

### Fix Focus Areas
- backend/src/middleware/auth.ts[1-32]
  - Replace hardcoded constant with `process.env.JWT_SECRET`.
  - Throw on startup if missing.
  - Avoid exporting the secret; instead export a helper (e.g., `signToken(payload)`), or keep secret private to the module.
- backend/src/routes/auth.ts[1-65]
  - Update token signing to use the centralized helper or imported config.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Hardcoded JWT_SECRET is a critical security vulnerability.

"secret123" is committed to source control and trivially guessable. Any attacker who knows this value can forge arbitrary tokens for any user ID and role (including admin), bypassing all authentication.

Load it from an environment variable and fail fast at startup if it is absent:

🛡️ Proposed fix
-const JWT_SECRET = "secret123";
+const JWT_SECRET = process.env.JWT_SECRET;
+if (!JWT_SECRET) {
+  throw new Error("JWT_SECRET environment variable is not set");
+}

Update the export type accordingly (export const JWT_SECRET: string), since after the guard TypeScript will narrow it to string.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const JWT_SECRET = "secret123";
const JWT_SECRET = process.env.JWT_SECRET;
if (!JWT_SECRET) {
throw new Error("JWT_SECRET environment variable is not set");
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/middleware/auth.ts` at line 4, Replace the hardcoded JWT_SECRET
with a value loaded from process.env and fail fast if missing: read
process.env.JWT_SECRET into the exported JWT_SECRET constant (export const
JWT_SECRET: string) and throw or exit during module initialization when it's
undefined or empty so the app won't start with an insecure default; update any
code that imported the previous value to use the new exported JWT_SECRET string.


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 || !header.startsWith("Bearer ")) {
return res.status(401).json({ error: "No token provided" });
}

const token = header.split(" ")[1];

try {
const decoded = jwt.verify(token, JWT_SECRET) as {
id: number;
email: string;
role: string;
};
req.user = decoded;
next();
} catch {
return res.status(401).json({ error: "Invalid token" });
}
}

export { JWT_SECRET };
66 changes: 66 additions & 0 deletions backend/src/routes/auth.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
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();

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 format" });
}

const existing = db.prepare("SELECT id FROM users WHERE email = ?").get(email);
if (existing) {
return res.status(409).json({ error: "Email already registered" });
}

const hash = await bcrypt.hash(password, 10);
const result = db.prepare(
"INSERT INTO users (email, password, name) VALUES (?, ?, ?)"
).run(email, hash, name);

const token = jwt.sign(
{ id: result.lastInsertRowid, email, role: "member" },
JWT_SECRET,
{ expiresIn: "24h" }
);

res.status(201).json({ token, user: { id: result.lastInsertRowid, email, name, role: "member" } });
Comment on lines +27 to +37
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's find and examine the auth.ts file to see the code context
if [ -f "backend/src/routes/auth.ts" ]; then
  echo "=== Auth.ts file (lines 20-40) ==="
  sed -n '20,40p' backend/src/routes/auth.ts
fi

# Check what version of better-sqlite3 is installed
echo -e "\n=== Package.json dependencies ==="
if [ -f "package.json" ]; then
  grep -A 10 '"dependencies"' package.json | grep -E '(better-sqlite3|@types/better-sqlite3)'
fi

if [ -f "backend/package.json" ]; then
  grep -A 10 '"dependencies"' backend/package.json | grep -E '(better-sqlite3|@types/better-sqlite3)'
fi

# Search for lastInsertRowid usage in the codebase
echo -e "\n=== lastInsertRowid usage ==="
rg -n "lastInsertRowid" --type ts --type js

Repository: reeder32/teamexpense

Length of output: 1249


🏁 Script executed:

#!/bin/bash
# Check if node_modules has the type definitions
echo "=== Checking for better-sqlite3 types in node_modules ==="
if [ -d "node_modules/@types/better-sqlite3" ]; then
  find node_modules/@types/better-sqlite3 -name "*.d.ts" -type f | head -5
  echo ""
  echo "=== Type definition for RunResult ==="
  grep -A 5 "RunResult" node_modules/@types/better-sqlite3/index.d.ts 2>/dev/null || echo "File not found in expected location"
fi

# Also try to find the declaration file another way
echo -e "\n=== Searching for RunResult type definition ==="
find . -path ./node_modules -prune -o -name "*.d.ts" -type f -print 2>/dev/null | xargs grep -l "RunResult" 2>/dev/null || echo "No RunResult definitions found"

# Check if the type definitions are available
echo -e "\n=== Checking available package.json in node_modules ==="
if [ -f "node_modules/better-sqlite3/package.json" ]; then
  grep -E '"(types|typings)"' node_modules/better-sqlite3/package.json
fi

Repository: reeder32/teamexpense

Length of output: 258


🌐 Web query:

better-sqlite3 RunResult lastInsertRowid type definition

💡 Result:

In better-sqlite3, Statement#run() returns a RunResult (often called the “info” object) whose lastInsertRowid is typed as:

interface RunResult {
  changes: number;
  lastInsertRowid: number | bigint;
}

That number | bigint comes from the TypeScript definitions (@types/better-sqlite3) and reflects whether the statement/database is configured to return “safe integers” (BigInt) or normal JS numbers. [1], [2]

Note: the API docs describe the same field (spelled lastInsertROWID in the docs) as “the rowid of the last row inserted,” and say it should be ignored if the statement didn’t insert rows. [2]

Sources: [1] [2]


🏁 Script executed:

#!/bin/bash
# Check TypeScript configuration
echo "=== TypeScript Config ==="
if [ -f "tsconfig.json" ]; then
  jq '.' tsconfig.json 2>/dev/null || cat tsconfig.json
fi

if [ -f "backend/tsconfig.json" ]; then
  jq '.' backend/tsconfig.json 2>/dev/null || cat backend/tsconfig.json
fi

# Check if there's any type casting or narrowing around lastInsertRowid
echo -e "\n=== Context around lastInsertRowid usage in auth.ts ==="
sed -n '25,40p' backend/src/routes/auth.ts

# Check if jwt.sign has any special handling or type requirements
echo -e "\n=== Checking jwt import and type ==="
grep -n "import.*jwt" backend/src/routes/auth.ts

# Verify the actual behavior of JSON.stringify with bigint
echo -e "\n=== Testing JSON.stringify with bigint behavior ==="
node <<'EOF'
try {
  const result = JSON.stringify({ id: BigInt(1), email: "test" });
  console.log("Result:", result);
} catch (e) {
  console.log("Error:", e.message);
}
EOF

Repository: reeder32/teamexpense

Length of output: 1140


Handle the number | bigint union type for lastInsertRowid.

better-sqlite3's RunResult.lastInsertRowid is typed as number | bigint depending on database configuration. Using it directly in JWT payloads and JSON responses without type narrowing is unsafe—JSON.stringify() will throw "Do not know how to serialize a BigInt" if the value is a bigint. Either assert the type at the database initialization level, explicitly narrow the type before use (e.g., Number(result.lastInsertRowid)), or ensure your database configuration consistently returns number values.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/routes/auth.ts` around lines 27 - 37, result.lastInsertRowid can
be number | bigint which will break JSON/JWT serialization; before using it in
jwt.sign and the JSON response convert/narrow it to a safe number type (e.g.,
coerce with Number(result.lastInsertRowid) or validate and toString() for stable
string ids) and use that converted value in jwt.sign(...) and in the user object
returned by res.status(201).json(...); update uses of result.lastInsertRowid in
this block to reference the new narrowed variable.

});

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 are required" });
}

const user = db.prepare("SELECT * FROM users WHERE email = ?").get(email) as any;
if (!user) {
return res.status(401).json({ error: "Invalid credentials" });
}

const valid = await bcrypt.compare(password, user.password);
if (!valid) {
return res.status(401).json({ error: "Invalid credentials" });
}

const token = jwt.sign(
{ id: user.id, email: user.email, role: user.role },
JWT_SECRET,
{ expiresIn: "24h" }
);

res.json({ token, user: { id: user.id, email: user.email, name: user.name, role: user.role } });
});
Comment on lines +40 to +64
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

/login has no rate limiting — brute-force attacks are trivial.

Without request rate limiting, an attacker can attempt unlimited password guesses against any known email address. Add per-IP or per-email rate limiting (e.g., express-rate-limit) in front of this endpoint.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/routes/auth.ts` around lines 40 - 64, Add a per-IP or per-email
rate limiter middleware (using express-rate-limit) to the login endpoint to
prevent brute-force attempts: create a limiter (e.g., loginLimiter) with
sensible windowMs and max attempts and a custom keyGenerator that uses req.ip or
req.body.email (or both) and a custom handler for blocked requests, then apply
that middleware to router.post("/login", ...) so the limiter runs before the
authentication logic in the login route; ensure the limiter is
exported/initialized where router is defined and tailored to block/slowdown
repeated requests while returning a clear 429 response.


export default router;
152 changes: 152 additions & 0 deletions backend/src/routes/expenses.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
import { Router, Response } from "express";
import db from "../db";
import { authenticate, AuthRequest } from "../middleware/auth";
import { isPositiveNumber, sanitizeString, CATEGORIES, BUDGET_LIMIT } from "../utils/validate";

const router = Router();

// Create expense
router.post("/", authenticate, (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 || !category) {
return res.status(400).json({ error: "Description and category are required" });
}

if (!CATEGORIES.includes(category)) {
return res.status(400).json({ error: "Invalid category" });
}

const existing = db.prepare(
"SELECT COALESCE(SUM(amount), 0) as total FROM expenses WHERE user_id = ? AND status != 'rejected'"
).get(userId) as any;

if (existing.total + amount > BUDGET_LIMIT) {
return res.status(400).json({ error: `Would exceed budget limit of $${BUDGET_LIMIT}` });
}

const result = db.prepare(
"INSERT INTO expenses (user_id, amount, description, category) VALUES (?, ?, ?, ?)"
).run(userId, amount, sanitizeString(description), category);

res.status(201).json({ id: result.lastInsertRowid, amount, description, category, status: "pending" });
});

// Get single expense
router.get("/:id", authenticate, (req: AuthRequest, res: Response) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, check if the file exists and its size
wc -l backend/src/routes/expenses.ts

Repository: reeder32/teamexpense

Length of output: 98


🏁 Script executed:

# Read the file to see all route handlers
cat -n backend/src/routes/expenses.ts

Repository: reeder32/teamexpense

Length of output: 5742


Constrain /:id to prevent capturing /search and /filter endpoints.

The /search and /filter route handlers (lines 69 and 84) are defined after the /:id handler (line 41). In Express, routes are matched in registration order, so requests to /search and /filter will be captured by /:id with "search" and "filter" as the id parameter, making those endpoints unreachable. Add a regex constraint to match only numeric IDs:

Suggested fix
-router.get("/:id", authenticate, (req: AuthRequest, res: Response) => {
+router.get("/:id(\\d+)", authenticate, (req: AuthRequest, res: Response) => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
router.get("/:id", authenticate, (req: AuthRequest, res: Response) => {
router.get("/:id(\\d+)", authenticate, (req: AuthRequest, res: Response) => {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/routes/expenses.ts` at line 41, The dynamic route
router.get("/:id", authenticate, ...) is currently too greedy and captures
requests intended for the later /search and /filter handlers; change the route
path to constrain id to numeric values (e.g., use a regex path like "/:id(\\d+)"
or an equivalent route parameter constraint) so only numeric IDs match, leaving
the /search and /filter routes to be reached; update the declaration that
registers the route (router.get("/:id", ...)) to the constrained form and keep
the same handler and middleware (authenticate, AuthRequest, Response) otherwise
unchanged.

const expense = db.prepare("SELECT * FROM expenses WHERE id = ?").get(req.params.id);

if (!expense) {
return res.status(404).json({ error: "Expense not found" });
}

res.json(expense);
Comment on lines +41 to +48
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users can view any expense by ID without checking ownership - should verify user_id matches req.user.id

Comment on lines +41 to +48
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

3. Expense idor read 🐞 Bug ⛨ Security

GET /api/expenses/:id returns an expense solely by ID without checking ownership, allowing any
authenticated user to fetch other users’ expenses by guessing IDs.
Agent Prompt
### Issue description
`GET /expenses/:id` does not enforce that the requested expense belongs to the authenticated user.

### Issue Context
Delete already enforces `id` + `user_id`, but GET does not.

### Fix Focus Areas
- backend/src/routes/expenses.ts[40-49]
  - Change query to `SELECT * FROM expenses WHERE id = ? AND user_id = ?`.
  - If admins should read any expense, implement explicit role-based branching (admin: id only; member: id+user_id).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +42 to +48
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Scope GET /:id to the authenticated user (IDOR).

Any authenticated user can fetch any expense by ID right now. This is an authorization bypass.

✅ Suggested fix
-router.get("/:id", authenticate, (req: AuthRequest, res: Response) => {
-  const expense = db.prepare("SELECT * FROM expenses WHERE id = ?").get(req.params.id);
+router.get("/:id", authenticate, (req: AuthRequest, res: Response) => {
+  const expense = db
+    .prepare("SELECT * FROM expenses WHERE id = ? AND user_id = ?")
+    .get(req.params.id, req.user!.id);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/routes/expenses.ts` around lines 42 - 48, The GET /:id handler
currently fetches an expense by id without checking ownership; update the DB
lookup to include the authenticated user's id so only the owner can fetch it:
ensure the auth middleware populates req.user.id, change the SELECT in the
db.prepare call that currently uses "SELECT * FROM expenses WHERE id = ?" to
also filter by the owner column (e.g. "AND user_id = ?"), pass req.params.id and
req.user.id in the proper order to .get(), and keep the existing 404 response
when no row is returned (operate on the existing expense variable).

});

// List expenses with pagination
router.get("/", authenticate, (req: AuthRequest, res: Response) => {
const page = parseInt(req.query.page as string) || 1;
const limit = parseInt(req.query.limit as string) || 20;
const userId = req.user!.id;

const expenses = db.prepare(
"SELECT * FROM expenses WHERE user_id = ? ORDER BY created_at DESC LIMIT ? OFFSET ?"
).all(userId, limit, page * limit);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pagination offset calculation is wrong - should be (page - 1) * limit

Suggested change
).all(userId, limit, page * limit);
).all(userId, limit, (page - 1) * limit);


Comment on lines +52 to +60
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

4. Pagination offset wrong 🐞 Bug ✓ Correctness

The list endpoint uses OFFSET page * limit which skips the first page (page=1 starts at
offset=20). Users will see missing/empty results on early pages.
Agent Prompt
### Issue description
Pagination offset is incorrect (`page * limit`).

### Issue Context
Default `page=1` should return the first `limit` rows with offset 0.

### Fix Focus Areas
- backend/src/routes/expenses.ts[52-66]
  - Change offset to `(page - 1) * limit`.
  - Add validation: clamp/reject negative or extremely large `limit` values.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +52 to +60
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix pagination offset and guard against negative values.

Offset uses page * limit, which skips the first page and allows negative offsets for page < 1.

✅ Suggested fix
-  const page = parseInt(req.query.page as string) || 1;
-  const limit = parseInt(req.query.limit as string) || 20;
+  const page = Math.max(parseInt(req.query.page as string) || 1, 1);
+  const limit = Math.max(parseInt(req.query.limit as string) || 20, 1);
@@
-  ).all(userId, limit, page * limit);
+  const offset = (page - 1) * limit;
+  ).all(userId, limit, offset);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/routes/expenses.ts` around lines 52 - 60, The pagination offset
logic in the router.get handler is wrong: it uses page * limit (which skips the
first page) and can produce negative/invalid offsets for page < 1; change the
calculation to compute offset = (Math.max(page, 1) - 1) * Math.max(limit, 1) (or
otherwise ensure page and limit are clamped to minimums) and pass that offset to
the prepared statement instead of page * limit; update the variables used in the
db.prepare(...).all(...) call in the router.get handler so the query uses limit
and the corrected offset and guard against non-positive page/limit values.

const count = db.prepare(
"SELECT COUNT(*) as total FROM expenses WHERE user_id = ?"
).get(userId) as any;

res.json({ data: expenses, total: count.total, page, limit });
});

// Search expenses
router.get("/search", authenticate, (req: AuthRequest, res: Response) => {
const query = req.query.q as string;

if (!query) {
return res.status(400).json({ error: "Search query is required" });
}

const results = db.prepare(
`SELECT * FROM expenses WHERE user_id = ${req.user!.id} AND description LIKE '%${query}%' ORDER BY created_at DESC`
).all();
Comment on lines +77 to +78
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical SQL injection vulnerability - user input query is directly interpolated into SQL

Suggested change
`SELECT * FROM expenses WHERE user_id = ${req.user!.id} AND description LIKE '%${query}%' ORDER BY created_at DESC`
).all();
const results = db.prepare(
"SELECT * FROM expenses WHERE user_id = ? AND description LIKE ? ORDER BY created_at DESC"
).all(req.user!.id, `%${query}%`);

Comment on lines +69 to +78
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Sql injection search 🐞 Bug ⛨ Security

The expenses search endpoint builds SQL with unescaped user input, enabling SQL injection and
potential cross-user data access/modification. This is exploitable via the q query parameter.
Agent Prompt
### Issue description
`GET /api/expenses/search` is vulnerable to SQL injection because it interpolates `req.query.q` directly into the SQL statement.

### Issue Context
The current implementation uses a template literal to build SQL, which allows crafted `q` values to alter the query.

### Fix Focus Areas
- backend/src/routes/expenses.ts[69-80]
  - Replace the template literal with a parameterized query, e.g.:
    - `SELECT * FROM expenses WHERE user_id = ? AND description LIKE ? ORDER BY created_at DESC`
    - params: `[req.user!.id, `%${query}%`]`
  - Consider trimming/length-limiting `query` similarly to `sanitizeString`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +68 to +78
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Parameterize search query to prevent SQL injection.

String interpolation in SQL allows injection via q. Use placeholders and a sanitized LIKE value.

✅ Suggested fix
-  const query = req.query.q as string;
+  const query = req.query.q;
 
-  if (!query) {
+  if (typeof query !== "string" || !query.trim()) {
     return res.status(400).json({ error: "Search query is required" });
   }
 
-  const results = db.prepare(
-    `SELECT * FROM expenses WHERE user_id = ${req.user!.id} AND description LIKE '%${query}%' ORDER BY created_at DESC`
-  ).all();
+  const like = `%${sanitizeString(query)}%`;
+  const results = db
+    .prepare(
+      "SELECT * FROM expenses WHERE user_id = ? AND description LIKE ? ORDER BY created_at DESC"
+    )
+    .all(req.user!.id, like);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/routes/expenses.ts` around lines 68 - 78, The SQL is built with
string interpolation in the router.get("/search") handler
(db.prepare(...).all()), which allows SQL injection via req.query.q; change it
to use a parameterized query and pass the sanitized LIKE pattern as a parameter
(e.g., "SELECT * FROM expenses WHERE user_id = ? AND description LIKE ? ORDER BY
created_at DESC") and supply [req.user!.id, `%${query}%`] (or named params) to
db.prepare().all(...) after validating/normalizing query to a string; ensure you
still return the results as before.


res.json(results);
});

// Filter expenses by date range
router.get("/filter", authenticate, (req: AuthRequest, res: Response) => {
const { startDate, endDate, category } = req.query;
Comment on lines +40 to +85
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Routes shadowed by :id 🐞 Bug ✓ Correctness

GET /:id is registered before /search and /filter, so requests to /search and /filter will
match the /:id handler first and never reach the intended endpoints. This breaks search and
filtering.
Agent Prompt
### Issue description
`/:id` is defined before `/search` and `/filter`, causing those literal routes to be shadowed.

### Issue Context
This prevents search and filtering endpoints from being reachable.

### Fix Focus Areas
- backend/src/routes/expenses.ts[40-110]
  - Register `/search` and `/filter` before `/:id`.
  - Optionally constrain `/:id` to numeric IDs only: `router.get('/:id(\\d+)', ...)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

const userId = req.user!.id;

let sql = "SELECT * FROM expenses WHERE user_id = ?";
const params: any[] = [userId];

if (startDate) {
sql += " AND created_at >= ?";
params.push(startDate);
}

if (endDate) {
sql += " AND created_at < ?";
params.push(endDate);
}

if (category) {
sql += " AND category = ?";
params.push(category);
}

sql += " ORDER BY created_at DESC";
const results = db.prepare(sql).all(...params);
res.json(results);
});

// Approve / reject expense (managers only)
router.patch("/:id/status", authenticate, (req: AuthRequest, res: Response) => {
if (req.user!.role !== "admin") {
return res.status(403).json({ error: "Only managers can approve/reject expenses" });
}

const { status } = req.body;
if (!["approved", "rejected"].includes(status)) {
return res.status(400).json({ error: "Status must be 'approved' or 'rejected'" });
}

const result = db.prepare(
"UPDATE expenses SET status = ? WHERE id = ?"
).run(status, req.params.id);

if (result.changes === 0) {
return res.status(404).json({ error: "Expense not found" });
}

res.json({ message: `Expense ${status}` });
});

// Delete expense
router.delete("/:id", authenticate, (req: AuthRequest, res: Response) => {
const expense = db.prepare("SELECT * FROM expenses WHERE id = ? AND user_id = ?").get(
req.params.id,
req.user!.id
) as any;

if (!expense) {
return res.status(404).json({ error: "Expense not found" });
}

if (expense.status !== "pending") {
return res.status(400).json({ error: "Can only delete pending expenses" });
}

db.prepare("DELETE FROM expenses WHERE id = ?").run(req.params.id);
res.json({ message: "Expense deleted" });
});

export default router;
56 changes: 56 additions & 0 deletions backend/src/routes/reports.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { Router, Request, Response } from "express";
import db from "../db";
import { authenticate, AuthRequest } from "../middleware/auth";

const router = Router();

// User spending summary
router.get("/my-summary", authenticate, (req: AuthRequest, res: Response) => {
const userId = req.user!.id;

const expenses = db.prepare(
"SELECT amount, category FROM expenses WHERE user_id = ? AND status = 'approved'"
).all(userId) as any[];

const byCategory: Record<string, number> = {};
let total = 0;

for (const exp of expenses) {
byCategory[exp.category] = (byCategory[exp.category] || 0) + exp.amount;
total += exp.amount;
}

res.json({ total, byCategory, count: expenses.length });
});

// Team summary — admin only
router.get("/team-summary", (req: Request, res: Response) => {
const expenses = db.prepare(
"SELECT * FROM expenses WHERE status = 'approved'"
).all() as any[];

const summary: Record<string, { name: string; total: number; count: number }> = {};

Comment on lines +26 to +33
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

5. Public team summary 🐞 Bug ⛨ Security

GET /api/reports/team-summary is labeled “admin only” but has no authentication/authorization
middleware, leaking organization-wide spending totals to unauthenticated callers.
Agent Prompt
### Issue description
`/reports/team-summary` is publicly accessible despite being intended for admins.

### Issue Context
Other report routes correctly use `authenticate` and role checks.

### Fix Focus Areas
- backend/src/routes/reports.ts[26-44]
  - Add `authenticate` middleware.
  - Add an admin role check and return 403 when unauthorized.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

for (const exp of expenses) {
if (!summary[exp.user_id]) {
const user = db.prepare("SELECT name FROM users WHERE id = ?").get(exp.user_id) as any;
summary[exp.user_id] = { name: user.name, total: 0, count: 0 };
}
summary[exp.user_id].total += exp.amount;
summary[exp.user_id].count += 1;
}
Comment on lines +34 to +41
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

N+1 query per unique user plus a missing null guard on the user lookup.

For every distinct user_id encountered in the expenses loop, a separate SELECT name FROM users WHERE id = ? is issued. This scales linearly with the number of distinct users. Additionally, if a user row has been deleted but their expenses remain, user will be null and user.name will throw a runtime error.

Replace with a single JOIN query:

♻️ Proposed fix
-  const expenses = db.prepare(
-    "SELECT * FROM expenses WHERE status = 'approved'"
-  ).all() as any[];
-
-  const summary: Record<string, { name: string; total: number; count: number }> = {};
-
-  for (const exp of expenses) {
-    if (!summary[exp.user_id]) {
-      const user = db.prepare("SELECT name FROM users WHERE id = ?").get(exp.user_id) as any;
-      summary[exp.user_id] = { name: user.name, total: 0, count: 0 };
-    }
-    summary[exp.user_id].total += exp.amount;
-    summary[exp.user_id].count += 1;
-  }
-
-  res.json(Object.values(summary));
+  const rows = db.prepare(`
+    SELECT u.name, SUM(e.amount) AS total, COUNT(*) AS count
+    FROM expenses e
+    JOIN users u ON u.id = e.user_id
+    WHERE e.status = 'approved'
+    GROUP BY e.user_id, u.name
+  `).all() as { name: string; total: number; count: number }[];
+
+  res.json(rows);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/routes/reports.ts` around lines 34 - 41, Current loop over
expenses causes N+1 queries via db.prepare("SELECT name FROM users WHERE id =
?") for each distinct exp.user_id and will throw if a user is missing; replace
this by querying expenses joined to users once (e.g., SELECT e.user_id, u.name,
e.amount FROM expenses e LEFT JOIN users u ON e.user_id = u.id) to return name
(nullable) and amount per row, then aggregate into the summary object (the
existing summary variable) in memory; ensure you handle null u.name by using a
fallback like "Unknown" or empty string before assigning
summary[exp.user_id].name so no runtime error occurs when a user row is missing.


res.json(Object.values(summary));
Comment on lines +27 to +43
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing authentication - anyone can access team summary without being logged in or having admin privileges

});
Comment on lines +27 to +44
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

/team-summary has no authentication or authorization — it is publicly accessible.

The comment says "admin only" but authenticate middleware is not applied and there is no req.user.role check. Any unauthenticated caller can retrieve the aggregated spending data for every user.

Compare with /export (Line 47–50) which correctly applies authenticate and checks the admin role.

🛡️ Proposed fix
-router.get("/team-summary", (req: Request, res: Response) => {
+router.get("/team-summary", authenticate, (req: AuthRequest, res: Response) => {
+  if (req.user!.role !== "admin") {
+    return res.status(403).json({ error: "Admin only" });
+  }
+
   const expenses = db.prepare(
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
router.get("/team-summary", (req: Request, res: Response) => {
const expenses = db.prepare(
"SELECT * FROM expenses WHERE status = 'approved'"
).all() as any[];
const summary: Record<string, { name: string; total: number; count: number }> = {};
for (const exp of expenses) {
if (!summary[exp.user_id]) {
const user = db.prepare("SELECT name FROM users WHERE id = ?").get(exp.user_id) as any;
summary[exp.user_id] = { name: user.name, total: 0, count: 0 };
}
summary[exp.user_id].total += exp.amount;
summary[exp.user_id].count += 1;
}
res.json(Object.values(summary));
});
router.get("/team-summary", authenticate, (req: AuthRequest, res: Response) => {
if (req.user!.role !== "admin") {
return res.status(403).json({ error: "Admin only" });
}
const expenses = db.prepare(
"SELECT * FROM expenses WHERE status = 'approved'"
).all() as any[];
const summary: Record<string, { name: string; total: number; count: number }> = {};
for (const exp of expenses) {
if (!summary[exp.user_id]) {
const user = db.prepare("SELECT name FROM users WHERE id = ?").get(exp.user_id) as any;
summary[exp.user_id] = { name: user.name, total: 0, count: 0 };
}
summary[exp.user_id].total += exp.amount;
summary[exp.user_id].count += 1;
}
res.json(Object.values(summary));
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/routes/reports.ts` around lines 27 - 44, The /team-summary route
(router.get("/team-summary")) is missing authentication and admin authorization;
update the route to apply the authenticate middleware and then enforce an admin
role check (use req.user.role or the same pattern used in the /export handler)
before querying the database so only authenticated admins can access the
aggregated data; return a 401/403 on missing auth or non-admin and keep the
existing summary logic intact.


// Export all expenses
router.get("/export", authenticate, (req: AuthRequest, res: Response) => {
if (req.user!.role !== "admin") {
return res.status(403).json({ error: "Admin only" });
}

const allExpenses = db.prepare("SELECT * FROM expenses").all();
res.json(allExpenses);
});

export default router;
22 changes: 22 additions & 0 deletions backend/src/utils/validate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
export function isValidEmail(email: string): boolean {
return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email);
}

export function isPositiveNumber(value: unknown): value is number {
return typeof value === "number" && value > 0;
}

export function sanitizeString(input: string): string {
return input.trim().slice(0, 500);
}

export const CATEGORIES = [
"travel",
"meals",
"supplies",
"software",
"equipment",
"other",
] as const;

export const BUDGET_LIMIT = 5000;
Loading
Loading