CRITICAL: Incomplete user deletion - orphaned data risk #70

New Issue

Closed

opened 2026-05-31 12:02:46 -05:00 by null · 1 comment

null commented

2026-05-31 12:02:46 -05:00

Owner

Bug Description

The DELETE /api/admin/users/:id route (routes/admin.js:317-325) only explicitly deletes from 4 tables: import_sessions, import_history, sessions, and users. It then relies on ON DELETE CASCADE foreign keys to clean up the remaining 25+ tables.

The problem: the code manually deletes import_sessions and import_history BEFORE deleting the user row. If foreign_keys pragma is ever disabled (e.g., during migration, or in a test environment), the cascade wont fire, leaving orphaned data across bills, payments, categories, transactions, monthly_bill_state, bill_history_ranges, bill_templates, data_sources, financial_accounts, and more.

Even with foreign_keys ON, the explicit deletes before the user delete are redundant and potentially confusing -- the cascade would handle those anyway.

Affected Code

routes/admin.js:317-325

db.transaction(() => {
db.prepare("DELETE FROM import_sessions WHERE user_id = ?").run(user.id);
db.prepare("DELETE FROM import_history WHERE user_id = ?").run(user.id);
db.prepare("DELETE FROM sessions WHERE user_id = ?").run(user.id);
db.prepare("DELETE FROM users WHERE id = ?").run(user.id);
});

Impact

If PRAGMA foreign_keys is ever OFF, user deletion leaves orphaned data across the entire database. This could leak data across user boundaries or cause referential integrity errors.

Recommended Fix

Remove the redundant explicit deletes (let CASCADE handle everything)
Or: add explicit deletes for ALL user-owned tables in the transaction
Add an assertion after the transaction that no orphaned rows remain
Consider a hard-delete utility that works even without foreign keys enabled

## Bug Description The DELETE /api/admin/users/:id route (routes/admin.js:317-325) only explicitly deletes from 4 tables: import_sessions, import_history, sessions, and users. It then relies on ON DELETE CASCADE foreign keys to clean up the remaining 25+ tables. The problem: the code manually deletes import_sessions and import_history BEFORE deleting the user row. If foreign_keys pragma is ever disabled (e.g., during migration, or in a test environment), the cascade wont fire, leaving orphaned data across bills, payments, categories, transactions, monthly_bill_state, bill_history_ranges, bill_templates, data_sources, financial_accounts, and more. Even with foreign_keys ON, the explicit deletes before the user delete are redundant and potentially confusing -- the cascade would handle those anyway. ## Affected Code routes/admin.js:317-325 db.transaction(() => { db.prepare("DELETE FROM import_sessions WHERE user_id = ?").run(user.id); db.prepare("DELETE FROM import_history WHERE user_id = ?").run(user.id); db.prepare("DELETE FROM sessions WHERE user_id = ?").run(user.id); db.prepare("DELETE FROM users WHERE id = ?").run(user.id); }); ## Impact If PRAGMA foreign_keys is ever OFF, user deletion leaves orphaned data across the entire database. This could leak data across user boundaries or cause referential integrity errors. ## Recommended Fix 1. Remove the redundant explicit deletes (let CASCADE handle everything) 2. Or: add explicit deletes for ALL user-owned tables in the transaction 3. Add an assertion after the transaction that no orphaned rows remain 4. Consider a hard-delete utility that works even without foreign keys enabled