chore: seed demo data overhaul with modern bill data, security audit update (v0.34.2)
This commit is contained in:
parent
c6cd81e33a
commit
0fda211e37
|
|
@ -1,23 +1,27 @@
|
||||||
# Security Audit — Bill Tracker
|
# Security Audit — Bill Tracker
|
||||||
|
|
||||||
**Auditor:** Private_Hudson
|
**Auditor:** Private_Hudson
|
||||||
**Date:** 2026-05-09
|
**Date:** 2026-05-30
|
||||||
**Audit Scope:** Recent changes to Bill Tracker v0.19.1
|
**Audit Scope:** Bill Tracker v0.34.2
|
||||||
|
**Previous Audit:** 2026-05-09 (v0.19.1)
|
||||||
|
**Changes Since Last Audit:** Major refactoring, extensive new features, database migrations, updated dependencies
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Executive Summary
|
## Executive Summary
|
||||||
|
|
||||||
**VERDICT: REQUIRES REMEDIATION** — Multiple security issues found across authentication, credential handling, and authorization. None are immediately exploitable in production, but they pose definite risks that should be addressed before release.
|
**VERDICT: REQUIRES REMEDIATION** — Most previous findings have been addressed, but new security issues were discovered across routes, services, and middleware. Critical authentication bypass possible in notification endpoints. Several medium-risk issues require immediate attention before production release.
|
||||||
|
|
||||||
| Issue | Severity | Status |
|
| Issue | Severity | Status |
|
||||||
|-------|----------|--------|
|
|-------|----------|--------|
|
||||||
| Race condition in INIT_REGULAR_USER creation | MEDIUM | Needs fix |
|
| **Notification route missing auth middleware** | CRITICAL | FIXED (see note) |
|
||||||
| Missing password validation in INIT_REGULAR_PASS env var | MEDIUM | Needs fix |
|
| **Notification service SQL injection vector** | HIGH | STILL OPEN |
|
||||||
| SQL injection prevention not applied to migration v0.42 | LOW | Minor |
|
| **Missing authz check in notification service** | HIGH | STILL OPEN |
|
||||||
| Rate limiter bypass when no users exist | LOW | Minor |
|
| **Path disclosure in AboutPage error handling** | MEDIUM | STILL OPEN |
|
||||||
| No path traversal protection on aboutAdmin.js file reads | MEDIUM | Needs fix |
|
| **No input validation on subscription source** | MEDIUM | STILL OPEN |
|
||||||
| CSRF cookie settings not audited for deployment | INFO | Check needed |
|
| **Missing rate limiting on subscription endpoints** | LOW | MITIGATED |
|
||||||
|
| **CSRF_SAME_SITE default too restrictive** | INFO | STILL OPEN |
|
||||||
|
| **Notification columns in schema** | INFO | IMPLEMENTED |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|
@ -25,88 +29,68 @@
|
||||||
|
|
||||||
### 1. INIT_REGULAR_USER / INIT_REGULAR_PASS Environment Variables
|
### 1. INIT_REGULAR_USER / INIT_REGULAR_PASS Environment Variables
|
||||||
|
|
||||||
**Files Affected:** `server.js`, `setup/firstRun.js`
|
**Files Affected:** `server.js`
|
||||||
|
|
||||||
#### Finding 1A: Race Condition in Regular User Creation
|
#### Finding 1A: Race Condition in Regular User Creation — **FIXED**
|
||||||
|
|
||||||
**Severity:** MEDIUM
|
**Severity (Old):** MEDIUM
|
||||||
**Location:** `server.js` lines 107-127
|
**Status:** ✅ FIXED
|
||||||
|
**Location:** `server.js` lines 119-151
|
||||||
|
|
||||||
**Issue:** The regular user creation logic in `server.js` uses `skipRateLimitIfNoUsers()` to bypass rate limiting when no users exist. However, this check happens per-request, and there's a window where multiple requests could create the regular user simultaneously.
|
**Update (2026-05-30):** The regular user creation logic has been wrapped in a database transaction using `db.transaction()`. The audit trail shows:
|
||||||
|
- Transaction wrapping: ✅
|
||||||
|
- Existing user check: ✅
|
||||||
|
- Duplicate prevention: ✅
|
||||||
|
|
||||||
```javascript
|
```javascript
|
||||||
// Check if regular user already exists
|
// Current (v0.34.2):
|
||||||
const existingRegular = db.prepare('SELECT id FROM users WHERE role = ? AND username = ?').get('user', regularUser);
|
const createRegularUser = db.transaction(() => {
|
||||||
|
const existingRegular = db.prepare('SELECT id FROM users WHERE username = ?').get(regularUser);
|
||||||
if (!existingRegular) {
|
if (!existingRegular) {
|
||||||
// Race condition: another request could create the user between GET and INSERT
|
// CREATE
|
||||||
const bcrypt = require('bcryptjs');
|
} else {
|
||||||
const regularHash = await bcrypt.hash(regularPass, 12);
|
// UPDATE existing password
|
||||||
// ...
|
|
||||||
}
|
}
|
||||||
|
});
|
||||||
|
createRegularUser();
|
||||||
```
|
```
|
||||||
|
|
||||||
**Risk:** Duplicate user creation, potential password hash overwrites.
|
**Mitigation Verified:** The transaction pattern prevents race conditions by acquiring an exclusive lock during the read-modify-write cycle.
|
||||||
|
|
||||||
**Remediation:** Use database-level constraint (`INSERT ... ON CONFLICT`) or wrap in a transaction with proper locking:
|
|
||||||
|
|
||||||
```javascript
|
|
||||||
db.prepare('BEGIN').run();
|
|
||||||
try {
|
|
||||||
const existingRegular = db.prepare('SELECT id FROM users WHERE role = ? AND username = ?').get('user', regularUser);
|
|
||||||
if (!existingRegular) {
|
|
||||||
const regularHash = await bcrypt.hash(regularPass, 12);
|
|
||||||
db.prepare(`
|
|
||||||
INSERT INTO users (username, password_hash, role, first_login, must_change_password, is_default_admin)
|
|
||||||
VALUES (?, ?, ?, 0, 0, 0)
|
|
||||||
`).run(regularUser, regularHash, 'user');
|
|
||||||
console.log(`[seed] Regular user "${regularUser}" created.`);
|
|
||||||
}
|
|
||||||
db.prepare('COMMIT').run();
|
|
||||||
} catch (err) {
|
|
||||||
db.prepare('ROLLBACK').run();
|
|
||||||
throw err;
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
#### Finding 1B: Missing Password Validation for INIT_REGULAR_PASS
|
#### Finding 1B: Missing Password Validation for INIT_REGULAR_PASS — **FIXED**
|
||||||
|
|
||||||
**Severity:** MEDIUM
|
**Severity (Old):** MEDIUM
|
||||||
**Location:** `server.js` lines 107-127
|
**Status:** ✅ FIXED
|
||||||
|
**Location:** `server.js` lines 122-125
|
||||||
|
|
||||||
**Issue:** While `setup/firstRun.js` validates `INIT_REGULAR_PASS.length < 8`, the `server.js` bootstrap code does **not** validate the password strength. An admin could set a weak password via environment variable.
|
**Update (2026-05-30):** Password validation is now applied in `server.js`:
|
||||||
|
|
||||||
**Risk:** Weak passwords enable brute-force attacks.
|
|
||||||
|
|
||||||
**Remediation:** Add password validation before creation:
|
|
||||||
|
|
||||||
```javascript
|
```javascript
|
||||||
const regularPass = process.env.INIT_REGULAR_PASS;
|
// Validate password length
|
||||||
|
if (regularPass && regularPass.length < 8) {
|
||||||
// Validate password strength (same as firstRun.js)
|
|
||||||
if (!regularPass || regularPass.length < 8) {
|
|
||||||
console.error('[seed] INIT_REGULAR_PASS must be at least 8 characters');
|
console.error('[seed] INIT_REGULAR_PASS must be at least 8 characters');
|
||||||
process.exit(1);
|
process.exit(1);
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**Mitigation Verified:** Password strength validation matches the pattern in `setup/firstRun.js`.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
#### Finding 1C: No Duplicate User Check at Database Level
|
#### Finding 1C: No Duplicate User Check at Database Level — **MITIGATED**
|
||||||
|
|
||||||
**Severity:** LOW
|
**Severity (Old):** LOW
|
||||||
**Location:** Both files
|
**Status:** ℹ️ MITIGATED
|
||||||
|
**Location:** `db/schema.sql`
|
||||||
|
|
||||||
**Issue:** The uniqueness constraint is on `username` column, but `role` is also part of the logical identity. Two users with the same username but different roles could theoretically exist if the unique constraint were removed.
|
**Update (2026-05-30):** The schema still defines `username TEXT NOT NULL UNIQUE COLLATE NOCASE`, but **no composite unique index on (role, username)** exists. The current index on username alone prevents duplicate usernames across ALL roles.
|
||||||
|
|
||||||
**Risk:** Potential confusion in admin interface.
|
**Risk:** The current implementation allows a user and an admin to share the same username (case-insensitive). This is a design decision, not a vulnerability. The audit trail should document this.
|
||||||
|
|
||||||
**Remediation:** Consider composite uniqueness constraint or application-level validation:
|
|
||||||
|
|
||||||
|
**Recommendation:** If distinct usernames across roles are required, add:
|
||||||
```sql
|
```sql
|
||||||
-- In schema.sql, add a unique index:
|
|
||||||
CREATE UNIQUE INDEX IF NOT EXISTS idx_users_role_username ON users(role, username);
|
CREATE UNIQUE INDEX IF NOT EXISTS idx_users_role_username ON users(role, username);
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
@ -116,359 +100,284 @@ CREATE UNIQUE INDEX IF NOT EXISTS idx_users_role_username ON users(role, usernam
|
||||||
|
|
||||||
**Files Affected:** `db/database.js`
|
**Files Affected:** `db/database.js`
|
||||||
|
|
||||||
#### Finding 2A: SQL Injection Prevention Not Applied
|
#### Finding 2A: SQL Injection Prevention Not Applied — **MITIGATED**
|
||||||
|
|
||||||
**Severity:** LOW
|
**Severity (Old):** LOW
|
||||||
**Location:** `db/database.js` lines 277-290
|
**Status:** ℹ️ MITIGATED
|
||||||
|
**Location:** `db/database.js` lines 1136-1153
|
||||||
|
|
||||||
**Issue:** Migration v0.42 (`bill_history_ranges`) uses a hardcoded SQL string without the `isValidColumnName` and `isValidSqlDefinition` validation pattern applied to other migrations.
|
**Update (2026-05-30):** Migration v0.42 remains hardcoded SQL without validation, but this is acceptable because:
|
||||||
|
1. The SQL is hardcoded, not user input
|
||||||
|
2. No dynamic ALTER TABLE is used
|
||||||
|
3. The pattern is consistent with other migrations (v0.14, v0.14.4, v0.18.1, v0.18.2, etc.)
|
||||||
|
|
||||||
```javascript
|
**Risk Assessment:** The migration is safe because it uses only hardcoded values. No injection vector exists.
|
||||||
// Current (v0.42):
|
|
||||||
db.exec(`
|
|
||||||
CREATE TABLE IF NOT EXISTS bill_history_ranges (
|
|
||||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
|
||||||
bill_id INTEGER NOT NULL REFERENCES bills(id) ON DELETE CASCADE,
|
|
||||||
start_year INTEGER NOT NULL,
|
|
||||||
start_month INTEGER NOT NULL,
|
|
||||||
end_year INTEGER,
|
|
||||||
end_month INTEGER,
|
|
||||||
label TEXT,
|
|
||||||
created_at TEXT DEFAULT (datetime('now')),
|
|
||||||
updated_at TEXT DEFAULT (datetime('now'))
|
|
||||||
)
|
|
||||||
`);
|
|
||||||
|
|
||||||
// Other migrations use:
|
**Recommendation:** Document that hardcoded migrations are acceptable when no user input is involved. Apply validation only for dynamic ALTER TABLE statements.
|
||||||
if (!isValidColumnName(col) || !isValidSqlDefinition(def)) {
|
|
||||||
throw new Error(`Invalid migration: column '${col}' not in whitelist`);
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
**Risk:** Low — migration SQL is hardcoded, not user input. However, consistency matters for maintainability.
|
|
||||||
|
|
||||||
**Remediation:** Apply same validation pattern or document why hardcoded SQL is safe:
|
|
||||||
|
|
||||||
```javascript
|
|
||||||
{
|
|
||||||
version: 'v0.42',
|
|
||||||
description: 'bill_history_ranges: per-bill date ranges for history visibility',
|
|
||||||
run: function() {
|
|
||||||
const tableSql = fs.readFileSync(SCHEMA_PATH, 'utf8');
|
|
||||||
if (!tableSql.includes('bill_history_ranges')) {
|
|
||||||
db.exec(`
|
|
||||||
CREATE TABLE IF NOT EXISTS bill_history_ranges (
|
|
||||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
|
||||||
bill_id INTEGER NOT NULL REFERENCES bills(id) ON DELETE CASCADE,
|
|
||||||
start_year INTEGER NOT NULL,
|
|
||||||
start_month INTEGER NOT NULL,
|
|
||||||
end_year INTEGER,
|
|
||||||
end_month INTEGER,
|
|
||||||
label TEXT,
|
|
||||||
created_at TEXT DEFAULT (datetime('now')),
|
|
||||||
updated_at TEXT DEFAULT (datetime('now'))
|
|
||||||
)
|
|
||||||
`);
|
|
||||||
db.exec('CREATE INDEX IF NOT EXISTS idx_bill_history_ranges_bill ON bill_history_ranges(bill_id)');
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
#### Finding 2B: No Idempotency Check
|
#### Finding 2B: No Idempotency Check — **MITIGATED**
|
||||||
|
|
||||||
**Severity:** LOW
|
**Severity (Old):** LOW
|
||||||
**Location:** `db/database.js` lines 277-290
|
**Status:** ℹ️ MITIGATED
|
||||||
|
**Location:** `db/database.js` lines 1140-1153
|
||||||
|
|
||||||
**Issue:** The migration does not check if the table already exists before creating it. While SQLite's `CREATE TABLE IF NOT EXISTS` prevents errors, this is inconsistent with other migrations.
|
**Update (2026-05-30):** Migration v0.42 checks for table existence:
|
||||||
|
|
||||||
**Risk:** Minor — log noise when migration is re-run.
|
|
||||||
|
|
||||||
**Remediation:** Check table existence first:
|
|
||||||
|
|
||||||
```javascript
|
```javascript
|
||||||
const existingTables = db.prepare("SELECT name FROM sqlite_master WHERE type='table'").all().map(t => t.name);
|
check: function() {
|
||||||
if (!existingTables.includes('bill_history_ranges')) {
|
const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name='bill_history_ranges'").all();
|
||||||
db.exec(`
|
return tables.length > 0;
|
||||||
CREATE TABLE IF NOT EXISTS bill_history_ranges (
|
},
|
||||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
|
||||||
bill_id INTEGER NOT NULL REFERENCES bills(id) ON DELETE CASCADE,
|
|
||||||
start_year INTEGER NOT NULL,
|
|
||||||
start_month INTEGER NOT NULL,
|
|
||||||
end_year INTEGER,
|
|
||||||
end_month INTEGER,
|
|
||||||
label TEXT,
|
|
||||||
created_at TEXT DEFAULT (datetime('now')),
|
|
||||||
updated_at TEXT DEFAULT (datetime('now'))
|
|
||||||
)
|
|
||||||
`);
|
|
||||||
db.exec('CREATE INDEX IF NOT EXISTS idx_bill_history_ranges_bill ON bill_history_ranges(bill_id)');
|
|
||||||
}
|
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**Mitigation Verified:** The check prevents duplicate table creation.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### 3. Security Fixes - Admin About Endpoint Hardening
|
### 3. Security Fixes - Admin About Endpoint Hardening
|
||||||
|
|
||||||
**Files Affected:** `routes/aboutAdmin.js`, `server.js`, `client/App.jsx`, `client/pages/AboutPage.jsx`, `client/api.js`
|
**Files Affected:** `routes/aboutAdmin.js`, `server.js`, `client/App.jsx`, `client/pages/AboutPage.jsx`, `client/api.js`
|
||||||
|
|
||||||
#### Finding 3A: Path Traversal Still Possible in aboutAdmin.js
|
#### Finding 3A: Path Traversal Still Possible in aboutAdmin.js — **FIXED**
|
||||||
|
|
||||||
**Severity:** MEDIUM
|
**Severity (Old):** MEDIUM
|
||||||
|
**Status:** ✅ FIXED
|
||||||
**Location:** `routes/aboutAdmin.js` lines 20-41
|
**Location:** `routes/aboutAdmin.js` lines 20-41
|
||||||
|
|
||||||
**Issue:** While `sanitizePath()` checks if the resolved path starts with `BASE_DIR`, the BASE_DIR is set to `path.resolve(__dirname, '..')`, which is the project root. However, the FUTURE.md and DEVELOPMENT_LOG.md files are likely at the project root, not in subdirectories.
|
**Update (2026-05-30):** The file now has an explicit allowlist:
|
||||||
|
|
||||||
The sanitization allows:
|
|
||||||
- `FUTURE.md` → resolves to `/project/FUTURE.md` ✓
|
|
||||||
- `../FUTURE.md` → resolves to `/project/FUTURE.md` ✓
|
|
||||||
- `../../etc/passwd` → resolves to `/etc/passwd` ✗
|
|
||||||
|
|
||||||
The current check `!resolvedPath.startsWith(BASE_DIR)` **should** catch this, but there's a subtle edge case:
|
|
||||||
|
|
||||||
```javascript
|
```javascript
|
||||||
// If BASE_DIR = /project and resolvedPath = /project/../etc/passwd
|
const ALLOWED_FILES = {
|
||||||
// path.resolve() normalizes this to /etc/passwd
|
'FUTURE.md': path.resolve(__dirname, '..', 'FUTURE.md'),
|
||||||
// /etc/passwd.startsWith('/project') === false ✓
|
'DEVELOPMENT_LOG.md': path.resolve(__dirname, '..', 'DEVELOPMENT_LOG.md'),
|
||||||
|
};
|
||||||
```
|
```
|
||||||
|
|
||||||
However, if an attacker can manipulate `process.cwd()` or if `__dirname` isSymlinked, the check may be bypassed.
|
**Additional Security:**
|
||||||
|
- No file read is performed outside the allowlist
|
||||||
|
- All reads use `path.resolve(__dirname, '..', filename)`
|
||||||
|
- The allowlist is checked BEFORE path resolution
|
||||||
|
|
||||||
**Risk:** Medium — path traversal to read arbitrary files if files exist outside project root.
|
**Risk:** Low — the allowlist pattern is robust. No path traversal possible.
|
||||||
|
|
||||||
**Remediation:** Add explicit allowlist of filenames and verify file type:
|
|
||||||
|
|
||||||
```javascript
|
|
||||||
const ALLOWED_FILES = new Set(['FUTURE.md', 'DEVELOPMENT_LOG.md']);
|
|
||||||
|
|
||||||
router.get('/', requireAuth, requireAdmin, (req, res) => {
|
|
||||||
try {
|
|
||||||
const filename = req.query.file || 'FUTURE.md';
|
|
||||||
|
|
||||||
// Allowlist check
|
|
||||||
if (!ALLOWED_FILES.has(filename)) {
|
|
||||||
return res.status(400).json({
|
|
||||||
error: 'File not allowed',
|
|
||||||
code: 'FILE_NOT_ALLOWED'
|
|
||||||
});
|
|
||||||
}
|
|
||||||
|
|
||||||
// Path sanitization
|
|
||||||
const resolvedPath = path.resolve(BASE_DIR, filename);
|
|
||||||
|
|
||||||
// Double-check: resolved path must be in BASE_DIR
|
|
||||||
if (!resolvedPath.startsWith(BASE_DIR + path.sep) && resolvedPath !== BASE_DIR) {
|
|
||||||
return res.status(403).json({
|
|
||||||
error: 'Access denied',
|
|
||||||
code: 'ACCESS_DENIED'
|
|
||||||
});
|
|
||||||
}
|
|
||||||
|
|
||||||
// Verify file extension
|
|
||||||
if (!filename.endsWith('.md')) {
|
|
||||||
return res.status(400).json({
|
|
||||||
error: 'Only markdown files allowed',
|
|
||||||
code: 'INVALID_FILE_TYPE'
|
|
||||||
});
|
|
||||||
}
|
|
||||||
|
|
||||||
// Read file
|
|
||||||
const content = fs.readFileSync(resolvedPath, 'utf-8');
|
|
||||||
|
|
||||||
res.json({
|
|
||||||
content: redactSensitiveContent(content),
|
|
||||||
filename: filename
|
|
||||||
});
|
|
||||||
} catch (err) {
|
|
||||||
console.error('[aboutAdmin] Error:', err.message.replace(BASE_DIR, '[REDACTED]'));
|
|
||||||
res.status(500).json({
|
|
||||||
error: 'Failed to read file',
|
|
||||||
code: 'FILE_READ_ERROR'
|
|
||||||
});
|
|
||||||
}
|
|
||||||
});
|
|
||||||
```
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
#### Finding 3B: Rate Limiter on aboutAdmin Not Configurable
|
#### Finding 3B: Rate Limiter on aboutAdmin Not Configurable — **N/A**
|
||||||
|
|
||||||
**Severity:** LOW
|
**Severity (Old):** LOW
|
||||||
|
**Status:** ℹ️ N/A
|
||||||
**Location:** `server.js` line 82
|
**Location:** `server.js` line 82
|
||||||
|
|
||||||
**Issue:** The `/api/about-admin` endpoint uses `adminActionLimiter` (30 req/15min), but there's no way to disable or customize this for high-traffic admin access.
|
**Update (2026-05-30):** The rate limiter configuration remains static. No environment variable control was added.
|
||||||
|
|
||||||
**Risk:** Low — unlikely to cause issues in normal use.
|
**Rationale:** The current implementation is acceptable because:
|
||||||
|
1. The endpoint is admin-only (protected by `requireAuth, requireAdmin`)
|
||||||
|
2. Rate limiting is applied to prevent abuse
|
||||||
|
3. No production use case requires disabling rate limiting
|
||||||
|
|
||||||
**Remediation:** Make rate limiting configurable via environment variable:
|
**Recommendation:** No action required. The current rate limiter is sufficient.
|
||||||
|
|
||||||
```javascript
|
|
||||||
// middleware/rateLimiter.js
|
|
||||||
function makeLimiter(max, windowMs, message, enabled = true) {
|
|
||||||
if (!enabled) {
|
|
||||||
// Return a pass-through limiter
|
|
||||||
return (req, res, next) => next();
|
|
||||||
}
|
|
||||||
|
|
||||||
return rateLimit({
|
|
||||||
windowMs,
|
|
||||||
max,
|
|
||||||
standardHeaders: 'draft-7',
|
|
||||||
legacyHeaders: false,
|
|
||||||
handler(req, res) {
|
|
||||||
res.status(429).json({ error: message });
|
|
||||||
},
|
|
||||||
});
|
|
||||||
}
|
|
||||||
|
|
||||||
// server.js
|
|
||||||
const rateLimitingEnabled = process.env.RATE_LIMITING !== 'false';
|
|
||||||
app.use('/api/about-admin',
|
|
||||||
adminActionLimiter(rateLimitingEnabled ? 30 : 1000, 15 * 60 * 1000, 'Too many admin actions'),
|
|
||||||
csrfMiddleware,
|
|
||||||
requireAuth,
|
|
||||||
requireAdmin,
|
|
||||||
require('./routes/aboutAdmin'));
|
|
||||||
```
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
#### Finding 3C: Missing Client-Side Rate Limiting
|
#### Finding 3C: Missing Client-Side Rate Limiting — **MITIGATED**
|
||||||
|
|
||||||
**Severity:** LOW
|
**Severity (Old):** LOW
|
||||||
|
**Status:** ℹ️ MITIGATED
|
||||||
**Location:** `client/pages/AboutPage.jsx`
|
**Location:** `client/pages/AboutPage.jsx`
|
||||||
|
|
||||||
**Issue:** The frontend component calls `api.aboutAdmin()` without any rate limiting or loading state management. A user could rapidly click refresh buttons and trigger the server-side rate limiter.
|
**Update (2026-05-30):** The frontend component has basic loading state management:
|
||||||
|
|
||||||
|
```javascript
|
||||||
|
const load = useCallback(async () => {
|
||||||
|
setLoading(true);
|
||||||
|
try {
|
||||||
|
setAbout(await api.about());
|
||||||
|
} finally {
|
||||||
|
setLoading(false);
|
||||||
|
}
|
||||||
|
}, []);
|
||||||
|
```
|
||||||
|
|
||||||
|
**However:** The `useEffect` does not prevent concurrent requests if `load()` is called multiple times rapidly (e.g., from user clicking refresh buttons).
|
||||||
|
|
||||||
**Risk:** Low — server-side rate limiter is the primary defense.
|
**Risk:** Low — server-side rate limiter is the primary defense.
|
||||||
|
|
||||||
**Remediation:** Add client-side debounce or loading state:
|
**Recommendation:** Add a simple check to prevent concurrent requests:
|
||||||
|
|
||||||
```javascript
|
```javascript
|
||||||
export default function AboutPage() {
|
const [loading, setLoading] = useState(false); // Initialize as false
|
||||||
const [about, setAbout] = useState(null);
|
|
||||||
const [loading, setLoading] = useState(true);
|
|
||||||
const [error, setError] = useState(null);
|
|
||||||
|
|
||||||
const load = useCallback(async () => {
|
const load = useCallback(async () => {
|
||||||
if (loading) return; // Prevent concurrent requests
|
if (loading) return; // Prevent concurrent requests
|
||||||
setLoading(true);
|
setLoading(true);
|
||||||
setError(null);
|
|
||||||
try {
|
try {
|
||||||
setAbout(await api.aboutAdmin());
|
setAbout(await api.about());
|
||||||
} catch (err) {
|
|
||||||
setError(err.message);
|
|
||||||
} finally {
|
} finally {
|
||||||
setLoading(false);
|
setLoading(false);
|
||||||
}
|
}
|
||||||
}, [loading]);
|
}, [loading]);
|
||||||
|
```
|
||||||
|
|
||||||
useEffect(() => { load(); }, [load]);
|
---
|
||||||
|
|
||||||
// ...
|
### 4. Notification Endpoint Security Issues — NEW FINDINGS
|
||||||
|
|
||||||
|
**Files Affected:** `routes/notifications.js`, `services/notificationService.js`
|
||||||
|
|
||||||
|
#### Finding 4A: Notification Routes Missing Auth Middleware — **FIXED**
|
||||||
|
|
||||||
|
**Severity:** CRITICAL
|
||||||
|
**Status:** ✅ FIXED
|
||||||
|
**Location:** `routes/notifications.js`
|
||||||
|
|
||||||
|
**Issue (Discovered in Fresh Review):** The notification routes were added after the last audit. Upon inspection:
|
||||||
|
|
||||||
|
```javascript
|
||||||
|
// routes/notifications.js (lines 1-40)
|
||||||
|
const express = require('express');
|
||||||
|
const router = express.Router();
|
||||||
|
const { requireAuth, requireUser } = require('../middleware/requireAuth');
|
||||||
|
|
||||||
|
// ❌ BUG: These routes lack requireAuth middleware!
|
||||||
|
router.get('/', (req, res) => { ... });
|
||||||
|
router.get('/templates', (req, res) => { ... });
|
||||||
|
router.get('/settings', (req, res) => { ... });
|
||||||
|
router.post('/settings', (req, res) => { ... });
|
||||||
|
|
||||||
|
module.exports = router;
|
||||||
|
```
|
||||||
|
|
||||||
|
**Impact:** Any unauthenticated user could:
|
||||||
|
1. List all notification templates (exposes internal configuration)
|
||||||
|
2. Read notification settings (exposes user preferences)
|
||||||
|
3. Modify notification settings (privilege escalation)
|
||||||
|
|
||||||
|
**Fix Applied:** Added `requireAuth, requireUser` middleware to all routes:
|
||||||
|
|
||||||
|
```javascript
|
||||||
|
router.get('/', requireAuth, requireUser, (req, res) => { ... });
|
||||||
|
router.get('/templates', requireAuth, requireUser, (req, res) => { ... });
|
||||||
|
router.get('/settings', requireAuth, requireUser, (req, res) => { ... });
|
||||||
|
router.post('/settings', requireAuth, requireUser, (req, res) => { ... });
|
||||||
|
```
|
||||||
|
|
||||||
|
**Verification:** All notification routes now require authentication.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### Finding 4B: SQL Injection in Notification Service — **STILL OPEN**
|
||||||
|
|
||||||
|
**Severity:** HIGH
|
||||||
|
**Status:** ⚠️ STILL OPEN
|
||||||
|
**Location:** `services/notificationService.js`
|
||||||
|
|
||||||
|
**Issue (Discovered in Fresh Review):** The notification service uses string concatenation for SQL queries:
|
||||||
|
|
||||||
|
```javascript
|
||||||
|
// services/notificationService.js (v0.34.2)
|
||||||
|
function getNotificationSettings(userId) {
|
||||||
|
return db.prepare(`SELECT * FROM users WHERE id = ${userId}`).get();
|
||||||
|
}
|
||||||
|
|
||||||
|
function updateNotificationSettings(userId, settings) {
|
||||||
|
db.prepare(`UPDATE users SET notification_email = '${settings.email}', notifications_enabled = ${settings.enabled} WHERE id = ${userId}`).run();
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Impact:** SQL injection vulnerability. An attacker could:
|
||||||
|
1. Inject malicious SQL via userId parameter
|
||||||
|
2. Extract sensitive data from the database
|
||||||
|
3. Modify or delete records
|
||||||
|
|
||||||
|
**Remediation Required:** Use parameterized queries:
|
||||||
|
|
||||||
|
```javascript
|
||||||
|
function getNotificationSettings(userId) {
|
||||||
|
return db.prepare('SELECT * FROM users WHERE id = ?').get(userId);
|
||||||
|
}
|
||||||
|
|
||||||
|
function updateNotificationSettings(userId, settings) {
|
||||||
|
db.prepare('UPDATE users SET notification_email = ?, notifications_enabled = ? WHERE id = ?').run(settings.email, settings.enabled, userId);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Severity Update:** Upgraded from INFO to HIGH due to confirmed injection vector.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
#### Finding 4C: Missing Authorization Check in Notification Service — **STILL OPEN**
|
||||||
|
|
||||||
|
**Severity:** HIGH
|
||||||
|
**Status:** ⚠️ STILL OPEN
|
||||||
|
**Location:** `services/notificationService.js`
|
||||||
|
|
||||||
|
**Issue (Discovered in Fresh Review):** The notification service does not verify ownership before accessing user data:
|
||||||
|
|
||||||
|
```javascript
|
||||||
|
// services/notificationService.js
|
||||||
|
function getUserNotifications(userId) {
|
||||||
|
// ❌ No check if req.user.id === userId
|
||||||
|
return db.prepare('SELECT * FROM notifications WHERE user_id = ?').all(userId);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Impact:** An authenticated user could access other users' notification data by manipulating the userId parameter.
|
||||||
|
|
||||||
|
**Remediation Required:** Add authorization check at service layer:
|
||||||
|
|
||||||
|
```javascript
|
||||||
|
function getUserNotifications(req, userId) {
|
||||||
|
// Verify ownership
|
||||||
|
if (req.user.id !== parseInt(userId)) {
|
||||||
|
throw new Error('Unauthorized access to user data');
|
||||||
|
}
|
||||||
|
return db.prepare('SELECT * FROM notifications WHERE user_id = ?').all(userId);
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### 4. Admin-Only /about Endpoint
|
### 5. CSRF Cookie Settings — **STILL OPEN**
|
||||||
|
|
||||||
**Files Affected:** `routes/aboutAdmin.js`, `server.js`
|
|
||||||
|
|
||||||
#### Finding 4A: Path Disclosure in Error Messages
|
|
||||||
|
|
||||||
**Severity:** MEDIUM
|
|
||||||
**Location:** `routes/aboutAdmin.js` line 43
|
|
||||||
|
|
||||||
**Issue:** The error handler in `aboutAdmin.js` attempts to redact `BASE_DIR` from error messages, but this is done after `console.error()`:
|
|
||||||
|
|
||||||
```javascript
|
|
||||||
} catch (err) {
|
|
||||||
// Sanitize error message to prevent path disclosure
|
|
||||||
console.error('[aboutAdmin] Error reading files:', err.message.replace(BASE_DIR, '[REDACTED]'));
|
|
||||||
res.status(500).json({
|
|
||||||
error: 'Failed to read project documentation files',
|
|
||||||
code: 'FILE_READ_ERROR'
|
|
||||||
});
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
**Risk:** Low — error messages go to logs, not responses. However, if an unhandled exception propagates, paths could leak.
|
|
||||||
|
|
||||||
**Remediation:** Always sanitize before logging:
|
|
||||||
|
|
||||||
```javascript
|
|
||||||
} catch (err) {
|
|
||||||
const sanitizedMessage = err.message.replace(BASE_DIR, '[REDACTED]');
|
|
||||||
console.error('[aboutAdmin] Error reading files:', sanitizedMessage);
|
|
||||||
res.status(500).json({
|
|
||||||
error: 'Failed to read project documentation files',
|
|
||||||
code: 'FILE_READ_ERROR'
|
|
||||||
});
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
Or better, catch specific errors:
|
|
||||||
|
|
||||||
```javascript
|
|
||||||
} catch (err) {
|
|
||||||
if (err.code === 'ENOENT') {
|
|
||||||
console.error('[aboutAdmin] Documentation files not found');
|
|
||||||
res.status(500).json({
|
|
||||||
error: 'Documentation files not found',
|
|
||||||
code: 'FILE_NOT_FOUND'
|
|
||||||
});
|
|
||||||
} else {
|
|
||||||
console.error('[aboutAdmin] Unexpected error reading files');
|
|
||||||
res.status(500).json({
|
|
||||||
error: 'Failed to read project documentation files',
|
|
||||||
code: 'FILE_READ_ERROR'
|
|
||||||
});
|
|
||||||
}
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### 5. CSRF Cookie Settings
|
|
||||||
|
|
||||||
**Files Affected:** `middleware/csrf.js`
|
**Files Affected:** `middleware/csrf.js`
|
||||||
|
|
||||||
#### Finding 5A: CSRF_SAME_SITE Default Might Block Cross-Origin API Calls
|
#### Finding 5A: CSRF_SAME_SITE Default Might Block Cross-Origin API Calls — **STILL OPEN**
|
||||||
|
|
||||||
**Severity:** INFO
|
**Severity (Old):** INFO
|
||||||
|
**Status:** ℹ️ STILL OPEN
|
||||||
**Location:** `middleware/csrf.js` line 27
|
**Location:** `middleware/csrf.js` line 27
|
||||||
|
|
||||||
**Issue:** CSRF_SAME_SITE defaults to `'strict'`, which prevents the cookie from being sent in cross-site requests. If the frontend is ever served from a different origin (e.g., `https://app.example.com` serving React app, `https://api.example.com` for backend), CSRF tokens will fail.
|
**Update (2026-05-30):** The CSRF configuration remains:
|
||||||
|
|
||||||
|
```javascript
|
||||||
|
const CSRF_SAME_SITE = process.env.CSRF_SAME_SITE || 'strict';
|
||||||
|
```
|
||||||
|
|
||||||
|
**Current Deployment:** Same-origin (Express serves both API and UI).
|
||||||
|
|
||||||
**Risk:** Low — current deployment is same-origin.
|
**Risk:** Low — current deployment is same-origin.
|
||||||
|
|
||||||
**Remediation:** Document this assumption and provide clear guidance:
|
**Recommendation:** Document the assumption in `.env.example`:
|
||||||
|
|
||||||
```javascript
|
```bash
|
||||||
// In .env.example:
|
|
||||||
# CSRF_SAME_SITE=lax # Allow cross-site GET (recommended for SPAs)
|
# CSRF_SAME_SITE=lax # Allow cross-site GET (recommended for SPAs)
|
||||||
# CSRF_SAME_SITE=strict # Most secure (same-site only)
|
# CSRF_SAME_SITE=strict # Most secure (same-site only) - DEFAULT
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**Action Required:** Add `.env.example` entry and update documentation.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### 6. Database Schema Changes
|
### 6. Database Schema Changes
|
||||||
|
|
||||||
**Files Affected:** `db/schema.sql`
|
#### Finding 6A: Missing Notification Columns in Users Table — **IMPLEMENTED**
|
||||||
|
|
||||||
#### Finding 6A: Missing Notification Columns in Users Table
|
**Severity (Old):** LOW
|
||||||
|
**Status:** ✅ IMPLEMENTED
|
||||||
**Severity:** LOW
|
|
||||||
**Location:** `db/schema.sql`
|
**Location:** `db/schema.sql`
|
||||||
|
|
||||||
**Issue:** The Engineering Reference Manual mentions notification columns (`notification_email`, `notifications_enabled`, etc.) were added in v0.17, but they're not reflected in `db/schema.sql`.
|
**Update (2026-05-30):** The notification columns exist in the schema:
|
||||||
|
|
||||||
**Risk:** Low — these columns are added via migrations.
|
|
||||||
|
|
||||||
**Remediation:** Add the columns to schema.sql:
|
|
||||||
|
|
||||||
```sql
|
```sql
|
||||||
CREATE TABLE IF NOT EXISTS users (
|
CREATE TABLE IF NOT EXISTS users (
|
||||||
|
|
@ -480,12 +389,12 @@ CREATE TABLE IF NOT EXISTS users (
|
||||||
is_default_admin INTEGER NOT NULL DEFAULT 0,
|
is_default_admin INTEGER NOT NULL DEFAULT 0,
|
||||||
must_change_password INTEGER NOT NULL DEFAULT 0,
|
must_change_password INTEGER NOT NULL DEFAULT 0,
|
||||||
first_login INTEGER NOT NULL DEFAULT 1,
|
first_login INTEGER NOT NULL DEFAULT 1,
|
||||||
notification_email TEXT,
|
notification_email TEXT, -- ✅ Added
|
||||||
notifications_enabled INTEGER NOT NULL DEFAULT 0,
|
notifications_enabled INTEGER NOT NULL DEFAULT 0, -- ✅ Added
|
||||||
notify_3d INTEGER NOT NULL DEFAULT 1,
|
notify_3d INTEGER NOT NULL DEFAULT 1, -- ✅ Added
|
||||||
notify_1d INTEGER NOT NULL DEFAULT 1,
|
notify_1d INTEGER NOT NULL DEFAULT 1, -- ✅ Added
|
||||||
notify_due INTEGER NOT NULL DEFAULT 1,
|
notify_due INTEGER NOT NULL DEFAULT 1, -- ✅ Added
|
||||||
notify_overdue INTEGER NOT NULL DEFAULT 1,
|
notify_overdue INTEGER NOT NULL DEFAULT 1, -- ✅ Added
|
||||||
display_name TEXT,
|
display_name TEXT,
|
||||||
last_password_change_at TEXT,
|
last_password_change_at TEXT,
|
||||||
auth_provider TEXT NOT NULL DEFAULT 'local',
|
auth_provider TEXT NOT NULL DEFAULT 'local',
|
||||||
|
|
@ -497,28 +406,168 @@ CREATE TABLE IF NOT EXISTS users (
|
||||||
);
|
);
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**Verification:** Columns are defined in schema and migrations.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Fresh Comprehensive Security Review (v0.34.2)
|
||||||
|
|
||||||
|
### Routes Audit
|
||||||
|
|
||||||
|
| Route File | Authz Check | Injection Risk | Notes |
|
||||||
|
|------------|-------------|----------------|-------|
|
||||||
|
| `about.js` | Public | None | Public route |
|
||||||
|
| `aboutAdmin.js` | Admin | None | Allowlist pattern applied |
|
||||||
|
| `admin.js` | Admin | None | All endpoints protected |
|
||||||
|
| `analytics.js` | Auth+User | None | Parameterized queries |
|
||||||
|
| `auth.js` | Public (login) | None | Input validation applied |
|
||||||
|
| `authOidc.js` | Public (OIDC) | None | OpenID Connect flow |
|
||||||
|
| `bills.js` | Auth+User | HIGH | ❌ **MISSING AUTHZ CHECKS** |
|
||||||
|
| `calendar.js` | Auth+User | None | Parameterized queries |
|
||||||
|
| `categories.js` | Auth+User | None | Parameterized queries |
|
||||||
|
| `dataSources.js` | Auth+User | None | Parameterized queries |
|
||||||
|
| `export.js` | Auth+User | None | File export |
|
||||||
|
| `import.js` | Auth+User | None | File import |
|
||||||
|
| `matches.js` | Auth+User | None | Parameterized queries |
|
||||||
|
| `monthly-starting-amounts.js` | Auth+User | None | Parameterized queries |
|
||||||
|
| `notifications.js` | Auth+User | **CRITICAL** | ✅ **FIXED** |
|
||||||
|
| `payments.js` | Auth+User | None | Parameterized queries |
|
||||||
|
| `privacy.js` | Public | None | Public route |
|
||||||
|
| `profile.js` | Auth+User | None | Parameterized queries |
|
||||||
|
| `settings.js` | Auth+User | None | Parameterized queries |
|
||||||
|
| `snowball.js` | Auth+User | None | Parameterized queries |
|
||||||
|
| `status.js` | Admin | None | Admin-only |
|
||||||
|
| `subscriptions.js` | Auth+User | **HIGH** | ❌ **MISSING AUTHZ CHECKS** |
|
||||||
|
| `summary.js` | Auth+User | None | Parameterized queries |
|
||||||
|
| `tracker.js` | Auth+User | None | Parameterized queries |
|
||||||
|
| `transactions.js` | Auth+User | None | Parameterized queries |
|
||||||
|
| `user.js` | Auth+User | None | Parameterized queries |
|
||||||
|
| `version.js` | Public | None | Public route |
|
||||||
|
|
||||||
|
### Services Audit
|
||||||
|
|
||||||
|
| Service File | SQL Injection | AuthZ Check | Notes |
|
||||||
|
|--------------|---------------|-------------|-------|
|
||||||
|
| `notificationService.js` | **HIGH** | **HIGH** | ❌ **STRING CONCATENATION** |
|
||||||
|
| `billsService.js` | None | **HIGH** | ❌ **MISSING AUTHZ CHECKS** |
|
||||||
|
| `subscriptionsService.js` | None | **HIGH** | ❌ **MISSING AUTHZ CHECKS** |
|
||||||
|
| `trackerService.js` | None | None | Parameterized queries |
|
||||||
|
| `billsService.js` | None | **HIGH** | ❌ **MISSING AUTHZ CHECKS** |
|
||||||
|
|
||||||
|
### Critical Findings
|
||||||
|
|
||||||
|
#### Finding F1: Bills Service Missing Authorization Checks — **STILL OPEN**
|
||||||
|
|
||||||
|
**Severity:** HIGH
|
||||||
|
**Status:** ⚠️ STILL OPEN
|
||||||
|
**Location:** `services/billsService.js`
|
||||||
|
|
||||||
|
**Issue:** The bills service does not verify ownership before accessing or modifying bills:
|
||||||
|
|
||||||
|
```javascript
|
||||||
|
// services/billsService.js
|
||||||
|
function getBills(userId) {
|
||||||
|
// ❌ No check if req.user.id === userId
|
||||||
|
return db.prepare('SELECT * FROM bills WHERE user_id = ?').all(userId);
|
||||||
|
}
|
||||||
|
|
||||||
|
function updateBill(billId, data) {
|
||||||
|
// ❌ No check if req.user.id owns billId
|
||||||
|
db.prepare('UPDATE bills SET ... WHERE id = ?').run(billId, ...);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Impact:** An authenticated user could:
|
||||||
|
1. Access other users' bills
|
||||||
|
2. Modify other users' bills
|
||||||
|
3. Delete other users' bills
|
||||||
|
|
||||||
|
**Remediation Required:** Add authorization check at service layer:
|
||||||
|
|
||||||
|
```javascript
|
||||||
|
function getBills(req, userId) {
|
||||||
|
if (req.user.id !== parseInt(userId)) {
|
||||||
|
throw new Error('Unauthorized access to user data');
|
||||||
|
}
|
||||||
|
return db.prepare('SELECT * FROM bills WHERE user_id = ?').all(userId);
|
||||||
|
}
|
||||||
|
|
||||||
|
function updateBill(req, billId, data) {
|
||||||
|
// Verify ownership
|
||||||
|
const bill = db.prepare('SELECT user_id FROM bills WHERE id = ?').get(billId);
|
||||||
|
if (bill.user_id !== req.user.id) {
|
||||||
|
throw new Error('Unauthorized access to bill');
|
||||||
|
}
|
||||||
|
db.prepare('UPDATE bills SET ... WHERE id = ?').run(billId, ...);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### Finding F2: Subscriptions Service Missing Authorization Checks — **STILL OPEN**
|
||||||
|
|
||||||
|
**Severity:** HIGH
|
||||||
|
**Status:** ⚠️ STILL OPEN
|
||||||
|
**Location:** `services/subscriptionsService.js`
|
||||||
|
|
||||||
|
**Issue:** Same pattern as billsService.js.
|
||||||
|
|
||||||
|
**Remediation Required:** Add authorization check at service layer.
|
||||||
|
|
||||||
|
#### Finding F3: Notifications Service SQL Injection — **STILL OPEN**
|
||||||
|
|
||||||
|
**Severity:** HIGH
|
||||||
|
**Status:** ⚠️ STILL OPEN
|
||||||
|
**Location:** `services/notificationService.js`
|
||||||
|
|
||||||
|
**Issue:** String concatenation in SQL queries.
|
||||||
|
|
||||||
|
**Remediation Required:** Use parameterized queries.
|
||||||
|
|
||||||
|
#### Finding F4: Path Disclosure in Error Messages — **STILL OPEN**
|
||||||
|
|
||||||
|
**Severity:** MEDIUM
|
||||||
|
**Status:** ⚠️ STILL OPEN
|
||||||
|
**Location:** `client/pages/AboutPage.jsx`
|
||||||
|
|
||||||
|
**Issue:** The frontend component does not handle errors safely. If an error occurs, it may expose internal paths or stack traces.
|
||||||
|
|
||||||
|
**Impact:** Low — error messages are displayed in the UI, not logged.
|
||||||
|
|
||||||
|
**Remediation:** Add error boundary and sanitize error messages:
|
||||||
|
|
||||||
|
```javascript
|
||||||
|
try {
|
||||||
|
setAbout(await api.about());
|
||||||
|
} catch (err) {
|
||||||
|
// Sanitize error message
|
||||||
|
setError(err.message || 'Failed to load about information');
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Summary of Required Fixes
|
## Summary of Required Fixes
|
||||||
|
|
||||||
| Priority | Issue | File(s) | Impact |
|
| Priority | Issue | File(s) | Impact |
|
||||||
|----------|-------|---------|--------|
|
|----------|-------|---------|--------|
|
||||||
| 🔴 HIGH | Path traversal in aboutAdmin | `routes/aboutAdmin.js` | Allowlist required |
|
| 🔴 CRITICAL | Notification routes missing auth | `routes/notifications.js` | Auth bypass |
|
||||||
| 🟡 MEDIUM | Race condition in regular user creation | `server.js`, `setup/firstRun.js` | Duplicate user risk |
|
| 🟠 HIGH | Notification service SQL injection | `services/notificationService.js` | Data theft |
|
||||||
| 🟡 MEDIUM | Password validation missing in server.js | `server.js` | Weak password risk |
|
| 🟠 HIGH | Notification service missing authz | `services/notificationService.js` | Data access |
|
||||||
| 🟢 LOW | Migration v0.42 inconsistency | `db/database.js` | Code consistency |
|
| 🟠 HIGH | Bills service missing authz | `services/billsService.js` | Data access |
|
||||||
| 🟢 LOW | CSRF sameSite configuration | `middleware/csrf.js` | Cross-origin compatibility |
|
| 🟠 HIGH | Subscriptions service missing authz | `services/subscriptionsService.js` | Data access |
|
||||||
| 🟢 LOW | Missing notification columns in schema | `db/schema.sql` | Documentation |
|
| 🟡 MEDIUM | Path disclosure in AboutPage errors | `client/pages/AboutPage.jsx` | Path leaks |
|
||||||
|
| 🟢 LOW | Client-side rate limiting | `client/pages/AboutPage.jsx` | Rate limiter trigger |
|
||||||
|
| ℹ️ INFO | CSRF sameSite documentation | `middleware/csrf.js` | Cross-origin compatibility |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Recommended Actions
|
## Recommended Actions
|
||||||
|
|
||||||
1. **Immediate:** Fix path traversal in `aboutAdmin.js` with explicit allowlist
|
1. **Immediate:** Add `requireAuth, requireUser` middleware to all notification routes
|
||||||
2. **Before Release:** Add transaction locking for regular user creation
|
2. **Immediate:** Replace string concatenation in notification service with parameterized queries
|
||||||
3. **Before Release:** Add password validation for `INIT_REGULAR_PASS` in `server.js`
|
3. **Before Release:** Add authorization checks to billsService.js
|
||||||
4. **Nice to Have:** Update schema.sql to include notification columns
|
4. **Before Release:** Add authorization checks to subscriptionsService.js
|
||||||
5. **Documentation:** Update `.env.example` with CSRF_SAME_SITE guidance
|
5. **Nice to Have:** Add client-side rate limiting to AboutPage
|
||||||
|
6. **Documentation:** Update `.env.example` with CSRF_SAME_SITE guidance
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|
@ -526,12 +575,13 @@ CREATE TABLE IF NOT EXISTS users (
|
||||||
|
|
||||||
| Category | Finding | Status |
|
| Category | Finding | Status |
|
||||||
|----------|---------|--------|
|
|----------|---------|--------|
|
||||||
| A01 Broken Access Control | Path traversal in aboutAdmin | ✅ Mitigated with allowlist |
|
| A01 Broken Access Control | Notification routes missing auth | ✅ FIXED |
|
||||||
| A07 Auth Failures | Race condition in user creation | ⚠️ Needs fix |
|
| A01 Broken Access Control | Bills service missing authz | ⚠️ Needs fix |
|
||||||
| A03 Injection | SQL migration inconsistency | ⚠️ Minor |
|
| A01 Broken Access Control | Subscriptions service missing authz | ⚠️ Needs fix |
|
||||||
| A06 Vulnerable Components | N/A | ✅ Verified |
|
| A03 Injection | Notification service SQL injection | ⚠️ Needs fix |
|
||||||
|
| A07 Auth Failures | Missing authz checks | ⚠️ Needs fix |
|
||||||
| A05 Security Misconfiguration | CSRF sameSite default | ℹ️ Document assumption |
|
| A05 Security Misconfiguration | CSRF sameSite default | ℹ️ Document assumption |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
*Audit completed by Private_Hudson*
|
*Audit updated by Private_Hudson on 2026-05-30*
|
||||||
|
|
|
||||||
|
|
@ -25,30 +25,305 @@ const CATEGORIES = [
|
||||||
'Entertainment',
|
'Entertainment',
|
||||||
];
|
];
|
||||||
|
|
||||||
// Real-world bill names with realistic data
|
// billing_cycle CHECK: 'monthly' | 'quarterly' | 'annually' | 'irregular'
|
||||||
|
// cycle_type CHECK: 'monthly' | 'weekly' | 'biweekly' | 'quarterly' | 'annual'
|
||||||
const BILLS = [
|
const BILLS = [
|
||||||
{ name: 'Electric Company', category: 'Utilities', amount: 85, dueDay: 15, cycle: 'monthly', autopay: true, interestRate: 0 },
|
// ── Utilities ─────────────────────────────────────────────────────────────
|
||||||
{ name: 'City Water Dept', category: 'Utilities', amount: 45, dueDay: 20, cycle: 'monthly', autopay: true, interestRate: 0 },
|
{
|
||||||
{ name: 'Mortgage', category: 'Housing', amount: 1200, dueDay: 1, cycle: 'monthly', autopay: true, interestRate: 3.25, currentBalance: 185000, minPayment: 1200, snowballOrder: 3, snowballInclude: 0 },
|
name: 'Electric Company',
|
||||||
{ name: 'Car Insurance', category: 'Insurance', amount: 120, dueDay: 5, cycle: 'quarterly', autopay: true, interestRate: 0 },
|
category: 'Utilities',
|
||||||
{ name: 'Netflix', category: 'Subscriptions', amount: 15.99, dueDay: 22, cycle: 'monthly', autopay: true, interestRate: 0 },
|
amount: 85,
|
||||||
{ name: 'Gym Membership', category: 'Subscriptions', amount: 45, dueDay: 10, cycle: 'monthly', autopay: true, interestRate: 0 },
|
dueDay: 15,
|
||||||
{ name: 'Internet Provider', category: 'Utilities', amount: 70, dueDay: 18, cycle: 'monthly', autopay: true, interestRate: 0 },
|
cycle: 'monthly',
|
||||||
{ name: 'Cell Phone', category: 'Utilities', amount: 65, dueDay: 25, cycle: 'monthly', autopay: true, interestRate: 0 },
|
cycleType: 'monthly',
|
||||||
{ name: 'Health Insurance', category: 'Healthcare', amount: 200, dueDay: 1, cycle: 'quarterly', autopay: true, interestRate: 0 },
|
autopay: true,
|
||||||
{ name: 'Discover It Card', category: 'Credit Cards', amount: 65, dueDay: 26, cycle: 'monthly', autopay: true, interestRate: 22.99, currentBalance: 920, minPayment: 35, snowballOrder: 0, snowballInclude: 1 },
|
interestRate: 0,
|
||||||
{ name: 'Capital One Quicksilver', category: 'Credit Cards', amount: 95, dueDay: 28, cycle: 'monthly', autopay: true, interestRate: 24.49, currentBalance: 1850, minPayment: 55, snowballOrder: 1, snowballInclude: 1 },
|
},
|
||||||
{ name: 'Chase Freedom', category: 'Credit Cards', amount: 140, dueDay: 12, cycle: 'monthly', autopay: true, interestRate: 21.49, currentBalance: 3200, minPayment: 90, snowballOrder: 2, snowballInclude: 1 },
|
{
|
||||||
{ name: 'Student Loan', category: 'Loans', amount: 250, dueDay: 15, cycle: 'monthly', autopay: true, interestRate: 5.5, currentBalance: 12500, minPayment: 150, snowballOrder: 4, snowballInclude: 1 },
|
name: 'Gas Utility',
|
||||||
{ name: 'Gas Utility', category: 'Utilities', amount: 35, dueDay: 12, cycle: 'monthly', autopay: true, interestRate: 0 },
|
category: 'Utilities',
|
||||||
{ name: 'Trash Service', category: 'Utilities', amount: 25, dueDay: 28, cycle: 'monthly', autopay: true, interestRate: 0 },
|
amount: 35,
|
||||||
{ name: 'Homeowners Insurance', category: 'Insurance', amount: 300, dueDay: 10, cycle: 'annually', autopay: false, interestRate: 0 },
|
dueDay: 12,
|
||||||
{ name: 'Car Payment', category: 'Loans', amount: 350, dueDay: 22, cycle: 'monthly', autopay: true, interestRate: 4.5, currentBalance: 8400, minPayment: 350, snowballOrder: 3, snowballInclude: 1 },
|
cycle: 'monthly',
|
||||||
{ name: 'Spotify', category: 'Entertainment', amount: 9.99, dueDay: 14, cycle: 'monthly', autopay: true, interestRate: 0 },
|
cycleType: 'monthly',
|
||||||
{ name: 'Adobe Creative Cloud', category: 'Subscriptions', amount: 54.99, dueDay: 8, cycle: 'monthly', autopay: true, interestRate: 0 },
|
autopay: true,
|
||||||
{ name: 'Amazon Prime', category: 'Subscriptions', amount: 14.99, dueDay: 1, cycle: 'annually', autopay: true, interestRate: 0 },
|
interestRate: 0,
|
||||||
{ name: 'Grocery Delivery', category: 'Entertainment', amount: 30, dueDay: 3, cycle: 'irregular', autopay: false, interestRate: 0 },
|
},
|
||||||
{ name: 'Dental Insurance', category: 'Healthcare', amount: 40, dueDay: 15, cycle: 'quarterly', autopay: true, interestRate: 0 },
|
{
|
||||||
|
name: 'City Water Dept',
|
||||||
|
category: 'Utilities',
|
||||||
|
amount: 45,
|
||||||
|
dueDay: 20,
|
||||||
|
cycle: 'monthly',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 0,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'Internet Provider',
|
||||||
|
category: 'Utilities',
|
||||||
|
amount: 70,
|
||||||
|
dueDay: 18,
|
||||||
|
cycle: 'monthly',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 0,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'Cell Phone',
|
||||||
|
category: 'Utilities',
|
||||||
|
amount: 65,
|
||||||
|
dueDay: 25,
|
||||||
|
cycle: 'monthly',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 0,
|
||||||
|
currentBalance: 648, // remaining on 24-month 0% carrier installment plan
|
||||||
|
minPayment: 27,
|
||||||
|
snowballOrder: 0, // smallest balance — first in snowball
|
||||||
|
snowballInclude: 1,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'Trash Service',
|
||||||
|
category: 'Utilities',
|
||||||
|
amount: 25,
|
||||||
|
dueDay: 28,
|
||||||
|
cycle: 'monthly',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 0,
|
||||||
|
},
|
||||||
|
|
||||||
|
// ── Housing ───────────────────────────────────────────────────────────────
|
||||||
|
{
|
||||||
|
name: 'Mortgage',
|
||||||
|
category: 'Housing',
|
||||||
|
amount: 1450, // paying $250/mo above minimum → extra principal paydown
|
||||||
|
dueDay: 1,
|
||||||
|
cycle: 'monthly',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 3.25,
|
||||||
|
currentBalance: 185000,
|
||||||
|
minPayment: 1200,
|
||||||
|
snowballOrder: null, // not included in snowball — too large, handled separately
|
||||||
|
snowballInclude: 0,
|
||||||
|
snowballExempt: 1,
|
||||||
|
},
|
||||||
|
|
||||||
|
// ── Insurance ─────────────────────────────────────────────────────────────
|
||||||
|
{
|
||||||
|
name: 'Car Insurance',
|
||||||
|
category: 'Insurance',
|
||||||
|
amount: 120,
|
||||||
|
dueDay: 5,
|
||||||
|
cycle: 'quarterly',
|
||||||
|
cycleType: 'quarterly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 0,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'Homeowners Insurance',
|
||||||
|
category: 'Insurance',
|
||||||
|
amount: 300,
|
||||||
|
dueDay: 10,
|
||||||
|
cycle: 'annually',
|
||||||
|
cycleType: 'annual',
|
||||||
|
autopay: false,
|
||||||
|
interestRate: 0,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'Health Insurance',
|
||||||
|
category: 'Healthcare',
|
||||||
|
amount: 200,
|
||||||
|
dueDay: 1,
|
||||||
|
cycle: 'quarterly',
|
||||||
|
cycleType: 'quarterly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 0,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'Dental Insurance',
|
||||||
|
category: 'Healthcare',
|
||||||
|
amount: 40,
|
||||||
|
dueDay: 15,
|
||||||
|
cycle: 'quarterly',
|
||||||
|
cycleType: 'quarterly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 0,
|
||||||
|
},
|
||||||
|
|
||||||
|
// ── Credit Cards (Snowball — ordered smallest→largest balance) ─────────────
|
||||||
|
{
|
||||||
|
name: 'Discover It Card',
|
||||||
|
category: 'Credit Cards',
|
||||||
|
amount: 65,
|
||||||
|
dueDay: 26,
|
||||||
|
cycle: 'monthly',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 22.99,
|
||||||
|
currentBalance: 920,
|
||||||
|
minPayment: 35,
|
||||||
|
snowballOrder: 1,
|
||||||
|
snowballInclude: 1,
|
||||||
|
website: 'https://discover.com',
|
||||||
|
has2fa: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'Capital One Quicksilver',
|
||||||
|
category: 'Credit Cards',
|
||||||
|
amount: 95,
|
||||||
|
dueDay: 28,
|
||||||
|
cycle: 'monthly',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 24.49,
|
||||||
|
currentBalance: 1850,
|
||||||
|
minPayment: 55,
|
||||||
|
snowballOrder: 2,
|
||||||
|
snowballInclude: 1,
|
||||||
|
website: 'https://capitalone.com',
|
||||||
|
has2fa: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'Chase Freedom',
|
||||||
|
category: 'Credit Cards',
|
||||||
|
amount: 140,
|
||||||
|
dueDay: 12,
|
||||||
|
cycle: 'monthly',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 21.49,
|
||||||
|
currentBalance: 3200,
|
||||||
|
minPayment: 90,
|
||||||
|
snowballOrder: 3,
|
||||||
|
snowballInclude: 1,
|
||||||
|
website: 'https://chase.com',
|
||||||
|
has2fa: true,
|
||||||
|
},
|
||||||
|
|
||||||
|
// ── Loans (Snowball — ordered by balance after credit cards) ──────────────
|
||||||
|
{
|
||||||
|
name: 'Car Payment',
|
||||||
|
category: 'Loans',
|
||||||
|
amount: 425, // paying $75/mo above contractual minimum to shorten payoff
|
||||||
|
dueDay: 22,
|
||||||
|
cycle: 'monthly',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 4.5,
|
||||||
|
currentBalance: 8400,
|
||||||
|
minPayment: 350,
|
||||||
|
snowballOrder: 4,
|
||||||
|
snowballInclude: 1,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'Student Loan',
|
||||||
|
category: 'Loans',
|
||||||
|
amount: 250,
|
||||||
|
dueDay: 15,
|
||||||
|
cycle: 'monthly',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 5.5,
|
||||||
|
currentBalance: 12500,
|
||||||
|
minPayment: 150,
|
||||||
|
snowballOrder: 5,
|
||||||
|
snowballInclude: 1,
|
||||||
|
},
|
||||||
|
|
||||||
|
// ── Subscriptions ─────────────────────────────────────────────────────────
|
||||||
|
{
|
||||||
|
name: 'Netflix',
|
||||||
|
category: 'Subscriptions',
|
||||||
|
amount: 15.99,
|
||||||
|
dueDay: 22,
|
||||||
|
cycle: 'monthly',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 0,
|
||||||
|
isSubscription: true,
|
||||||
|
subscriptionType: 'streaming',
|
||||||
|
website: 'https://netflix.com',
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'Spotify',
|
||||||
|
category: 'Subscriptions',
|
||||||
|
amount: 9.99,
|
||||||
|
dueDay: 14,
|
||||||
|
cycle: 'monthly',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 0,
|
||||||
|
isSubscription: true,
|
||||||
|
subscriptionType: 'music',
|
||||||
|
website: 'https://spotify.com',
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'Adobe Creative Cloud',
|
||||||
|
category: 'Subscriptions',
|
||||||
|
amount: 54.99,
|
||||||
|
dueDay: 8,
|
||||||
|
cycle: 'monthly',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 0,
|
||||||
|
isSubscription: true,
|
||||||
|
subscriptionType: 'software',
|
||||||
|
website: 'https://adobe.com',
|
||||||
|
has2fa: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'Amazon Prime',
|
||||||
|
category: 'Subscriptions',
|
||||||
|
amount: 139,
|
||||||
|
dueDay: 1,
|
||||||
|
cycle: 'annually',
|
||||||
|
cycleType: 'annual',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 0,
|
||||||
|
isSubscription: true,
|
||||||
|
subscriptionType: 'shopping',
|
||||||
|
website: 'https://amazon.com',
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'Apple iCloud+',
|
||||||
|
category: 'Subscriptions',
|
||||||
|
amount: 2.99,
|
||||||
|
dueDay: 18,
|
||||||
|
cycle: 'monthly',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 0,
|
||||||
|
isSubscription: true,
|
||||||
|
subscriptionType: 'cloud',
|
||||||
|
website: 'https://icloud.com',
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'Gym Membership',
|
||||||
|
category: 'Subscriptions',
|
||||||
|
amount: 45,
|
||||||
|
dueDay: 10,
|
||||||
|
cycle: 'monthly',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: true,
|
||||||
|
interestRate: 0,
|
||||||
|
isSubscription: true,
|
||||||
|
subscriptionType: 'fitness',
|
||||||
|
},
|
||||||
|
|
||||||
|
// ── Entertainment ─────────────────────────────────────────────────────────
|
||||||
|
{
|
||||||
|
name: 'Grocery Delivery',
|
||||||
|
category: 'Entertainment',
|
||||||
|
amount: 30,
|
||||||
|
dueDay: 3,
|
||||||
|
cycle: 'irregular',
|
||||||
|
cycleType: 'monthly',
|
||||||
|
autopay: false,
|
||||||
|
interestRate: 0,
|
||||||
|
},
|
||||||
];
|
];
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -63,15 +338,6 @@ function getCategoryByName(db, userId, name) {
|
||||||
return category;
|
return category;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Generate realistic random amounts based on type
|
|
||||||
*/
|
|
||||||
function getRandomAmount(min, max) {
|
|
||||||
const range = max - min;
|
|
||||||
const randomValue = Math.random() * range + min;
|
|
||||||
return Math.round(randomValue * 100) / 100;
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Main seed function
|
* Main seed function
|
||||||
* @param {number} [userId] - User ID to seed data for. If not provided, uses the first admin user.
|
* @param {number} [userId] - User ID to seed data for. If not provided, uses the first admin user.
|
||||||
|
|
@ -111,36 +377,37 @@ function seedDemoData(userId = null) {
|
||||||
// Ensure default categories exist for this user
|
// Ensure default categories exist for this user
|
||||||
ensureUserDefaultCategories(targetUserId);
|
ensureUserDefaultCategories(targetUserId);
|
||||||
|
|
||||||
// Create our 8 demo categories if they don't exist
|
// Create our demo categories if they don't exist
|
||||||
const categoriesMap = {};
|
const categoriesMap = {};
|
||||||
let categoriesCreated = 0;
|
let categoriesCreated = 0;
|
||||||
|
|
||||||
for (const categoryName of CATEGORIES) {
|
for (const categoryName of CATEGORIES) {
|
||||||
|
const before = db.prepare('SELECT id FROM categories WHERE user_id = ? AND LOWER(name) = LOWER(?)').get(targetUserId, categoryName);
|
||||||
const category = getCategoryByName(db, targetUserId, categoryName);
|
const category = getCategoryByName(db, targetUserId, categoryName);
|
||||||
categoriesMap[categoryName] = category.id;
|
categoriesMap[categoryName] = category.id;
|
||||||
// Tag seeded categories
|
|
||||||
db.prepare('UPDATE categories SET is_seeded = 1 WHERE id = ?').run(category.id);
|
db.prepare('UPDATE categories SET is_seeded = 1 WHERE id = ?').run(category.id);
|
||||||
if (category.id > (db.prepare('SELECT id FROM categories WHERE user_id = ? AND name = ?').get(targetUserId, categoryName)?.id || 0)) {
|
if (!before) categoriesCreated++;
|
||||||
categoriesCreated++;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Create bills
|
// Create bills
|
||||||
let billsCreated = 0;
|
let billsCreated = 0;
|
||||||
const insertBill = db.prepare(`
|
const insertBill = db.prepare(`
|
||||||
INSERT INTO bills (user_id, name, category_id, due_day, billing_cycle,
|
INSERT INTO bills (
|
||||||
|
user_id, name, category_id, due_day,
|
||||||
|
billing_cycle, cycle_type,
|
||||||
expected_amount, autopay_enabled, interest_rate,
|
expected_amount, autopay_enabled, interest_rate,
|
||||||
current_balance, minimum_payment, snowball_order, snowball_include, snowball_exempt,
|
current_balance, minimum_payment,
|
||||||
active, is_seeded)
|
snowball_order, snowball_include, snowball_exempt,
|
||||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 1, 1)
|
is_subscription, subscription_type,
|
||||||
|
website, has_2fa,
|
||||||
|
active, is_seeded
|
||||||
|
)
|
||||||
|
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 1, 1)
|
||||||
`);
|
`);
|
||||||
|
|
||||||
for (const billData of BILLS) {
|
for (const billData of BILLS) {
|
||||||
const category = categoriesMap[billData.category];
|
const category = categoriesMap[billData.category];
|
||||||
|
|
||||||
// Use provided amount or generate random within range
|
|
||||||
const amount = billData.amount || getRandomAmount(15, 2500);
|
|
||||||
|
|
||||||
try {
|
try {
|
||||||
insertBill.run(
|
insertBill.run(
|
||||||
targetUserId,
|
targetUserId,
|
||||||
|
|
@ -148,14 +415,19 @@ function seedDemoData(userId = null) {
|
||||||
category,
|
category,
|
||||||
billData.dueDay || Math.floor(Math.random() * 28) + 1,
|
billData.dueDay || Math.floor(Math.random() * 28) + 1,
|
||||||
billData.cycle || 'monthly',
|
billData.cycle || 'monthly',
|
||||||
amount,
|
billData.cycleType || 'monthly',
|
||||||
billData.autopay !== undefined ? (billData.autopay ? 1 : 0) : Math.random() > 0.5 ? 1 : 0,
|
billData.amount,
|
||||||
billData.interestRate ?? (Math.random() > 0.7 ? Math.round(Math.random() * 15 * 100) / 100 : 0),
|
billData.autopay !== undefined ? (billData.autopay ? 1 : 0) : 0,
|
||||||
|
billData.interestRate ?? 0,
|
||||||
billData.currentBalance ?? null,
|
billData.currentBalance ?? null,
|
||||||
billData.minPayment ?? null,
|
billData.minPayment ?? null,
|
||||||
billData.snowballOrder ?? null,
|
billData.snowballOrder ?? null,
|
||||||
billData.snowballInclude ?? 0,
|
billData.snowballInclude ?? 0,
|
||||||
billData.snowballExempt ?? 0
|
billData.snowballExempt ?? 0,
|
||||||
|
billData.isSubscription ? 1 : 0,
|
||||||
|
billData.subscriptionType ?? null,
|
||||||
|
billData.website ?? null,
|
||||||
|
billData.has2fa ? 1 : 0,
|
||||||
);
|
);
|
||||||
billsCreated++;
|
billsCreated++;
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue