421 lines
17 KiB
Markdown
421 lines
17 KiB
Markdown
# Bill Tracker — Future Improvements
|
|
|
|
**This document tracks potential future enhancements for Bill Tracker.**
|
|
|
|
**Last Updated:** 2026-05-10
|
|
**Current Version:** v0.23.2
|
|
|
|
## How to Use This Document
|
|
|
|
This file is a living document. Agents should:
|
|
1. Read this file before proposing changes
|
|
2. Add new recommendations with priority levels
|
|
3. Never add completed items — move those to HISTORY.md instead
|
|
4. Reference this file when dispatching improvement tasks
|
|
5. Only Ripley can remove items from this list.
|
|
|
|
### Priority Format
|
|
|
|
All items must include the priority emoji in their heading, matching the section they belong to:
|
|
|
|
| Priority | Emoji | Heading Format |
|
|
|----------|-------|---------------|
|
|
| CRITICAL | 🔴 | `### 🔴 Title — CRITICAL` |
|
|
| HIGH | 🟠 | `### 🟠 Title — HIGH` |
|
|
| MEDIUM | 🟡 | `### 🟡 Title — MEDIUM` |
|
|
| LOW | 🔵 | `### 🔵 Title — LOW` |
|
|
| NICE TO HAVE | 💭 | `### 💭 Title — NICE TO HAVE` |
|
|
|
|
Items are grouped under their priority section heading (`## 🔴 CRITICAL`, `## 🟠 HIGH`, etc.) and sorted most-impactful-first within each tier.
|
|
|
|
|
|
## Pending Recommendations
|
|
|
|
### 🔴 CRITICAL
|
|
|
|
|
|
### ~~🔴 Notification Runner Leaks Bill Details Across Users — CRITICAL~~ ✅ FIXED (v0.23.2)
|
|
**Moved to HISTORY.md**
|
|
|
|
|
|
### 🟠 HIGH
|
|
|
|
### 🟠 Admin Can Toggle Payments on Any User Bill — HIGH
|
|
**Priority:** HIGH
|
|
**Added:** 2026-05-10 by Prime (code review)
|
|
**Type:** SECURITY
|
|
|
|
**Description:**
|
|
`POST /api/bills/:id/toggle-paid` has a special admin branch that selects bills without `user_id = req.user.id`, then soft-deletes or creates payments on that bill. The surrounding router is mounted with `requireUser`, and `requireUser` allows `role='admin'`, so a non-default admin can modify another user's financial records through this endpoint. This contradicts the documented admin/privacy boundary used elsewhere.
|
|
|
|
**Affected Files:**
|
|
- `routes/bills.js:393-443`
|
|
- `middleware/requireAuth.js:38-45`
|
|
- `server.js:86-87`
|
|
|
|
**Potential Fix:**
|
|
Remove the admin-wide branch and require ownership for all bill payment mutations. If admin support access is intentional, put it behind a separate audited admin endpoint with explicit UI/permission labeling.
|
|
|
|
**Severity:** HIGH
|
|
|
|
### 🟠 Analytics Validation Errors Crash Instead of Returning 400 — HIGH
|
|
**Priority:** HIGH
|
|
**Added:** 2026-05-10 by Prime (code review)
|
|
**Type:** BUG / API_CONTRACT
|
|
|
|
**Description:**
|
|
`routes/analytics.js` calls `standardizeError()` on invalid query parameters, but never imports it. Valid analytics requests work, but invalid `year`, `month`, `months`, `category_id`, or `bill_id` input will throw `ReferenceError: standardizeError is not defined` and return a 500 instead of a standardized 400 response.
|
|
|
|
**Affected Files:**
|
|
- `routes/analytics.js:1-3`
|
|
- `routes/analytics.js:97-99`
|
|
|
|
**Potential Fix:**
|
|
Import `standardizeError` from `../middleware/errorFormatter` and add route tests for invalid analytics query parameters.
|
|
|
|
**Severity:** HIGH
|
|
|
|
### 🟠 User Export Drops Recurrence and History-Range Data — HIGH
|
|
**Priority:** HIGH
|
|
**Added:** 2026-05-10 by Prime (code review)
|
|
**Type:** DATA_INTEGRITY
|
|
|
|
**Description:**
|
|
`routes/export.js` does not include `bills.cycle_type`, `bills.cycle_day`, or `bill_history_ranges` in user Excel/SQLite exports. Users who rely on exports as portable backups can lose non-monthly recurrence settings and history visibility ranges after exporting and re-importing their data.
|
|
|
|
**Affected Files:**
|
|
- `routes/export.js`
|
|
- `db/schema.sql`
|
|
|
|
**Potential Fix:**
|
|
Add `cycle_type` and `cycle_day` to exported bill rows and the SQLite export schema. Export `bill_history_ranges` as its own Excel sheet/SQLite table and update the matching import path to restore those rows under the current user.
|
|
|
|
**Severity:** HIGH
|
|
|
|
### 🟠 Single-User Mode Can Lock Itself Out When Expired Sessions Exist — HIGH
|
|
**Priority:** HIGH
|
|
**Added:** 2026-05-10 by Prime (code review)
|
|
**Type:** BUG
|
|
|
|
**Description:**
|
|
`getSingleModeUser()` uses a `LEFT JOIN sessions` and requires `(s.expires_at > datetime('now') OR s.id IS NULL)`. If the configured default single-mode user has only expired session rows, the join returns expired rows, `s.id` is not NULL, and no user is returned. That defeats the intended session bypass for single-user mode until expired sessions are pruned.
|
|
|
|
**Affected Files:**
|
|
- `middleware/requireAuth.js`
|
|
- `services/authService.js`
|
|
|
|
**Potential Fix:**
|
|
Do not join sessions for single-user mode. Validate only that the configured default user exists, is active, and has role `user`, or explicitly prune expired sessions before checking.
|
|
|
|
**Severity:** HIGH
|
|
|
|
|
|
### 🟡 MEDIUM
|
|
|
|
|
|
### 🟡 Password Change Rate Limiter Applies to Every Profile Endpoint — MEDIUM
|
|
**Priority:** MEDIUM
|
|
**Added:** 2026-05-10 by Prime (code review)
|
|
**Type:** API_CONTRACT / CONFIG
|
|
|
|
**Description:**
|
|
`server.js` mounts `passwordLimiter` across all `/api/profile` routes, not only `/api/profile/change-password`. Five harmless profile reads/settings updates in 15 minutes can block profile access with a password-change rate-limit message, while also making profile pages fragile under normal UI polling or retries.
|
|
|
|
**Affected Files:**
|
|
- `server.js:102-103`
|
|
- `middleware/rateLimiter.js:18-23`
|
|
- `routes/profile.js`
|
|
|
|
**Potential Fix:**
|
|
Apply `passwordLimiter` only to `POST /api/profile/change-password` (or inside the route), leaving profile reads/settings on a normal API limiter if needed.
|
|
|
|
**Severity:** MEDIUM
|
|
|
|
### 🟡 Profile Password Change Does Not Invalidate Other Sessions — MEDIUM
|
|
**Priority:** MEDIUM
|
|
**Added:** 2026-05-10 by Prime (code review)
|
|
**Type:** SECURITY
|
|
|
|
**Description:**
|
|
`POST /api/profile/change-password` only invalidates other sessions and rotates the current session when `req.sessionId` exists, but `requireAuth()` never sets `req.sessionId`. Password changes through the profile endpoint can therefore leave all existing sessions active and skip current-session rotation. The separate `/api/auth/change-password` route uses the cookie value directly and does not have this specific gap.
|
|
|
|
**Affected Files:**
|
|
- `routes/profile.js:211-223`
|
|
- `middleware/requireAuth.js:22-31`
|
|
- `services/authService.js:175-194`
|
|
|
|
**Potential Fix:**
|
|
Set `req.sessionId` in `requireAuth()` from the session cookie after successful authentication, or make the profile route use `req.cookies?.[COOKIE_NAME]` consistently with `/api/auth/change-password`. Add a regression test that verifies other sessions are removed after profile password change.
|
|
|
|
**Severity:** MEDIUM
|
|
|
|
### 🟡 CSRF Defaults Conflict with SPA Token Loading — MEDIUM
|
|
**Priority:** MEDIUM
|
|
**Added:** 2026-05-10 by Prime (code review)
|
|
**Type:** CONFIG / SECURITY
|
|
|
|
**Description:**
|
|
The CSRF middleware defaults the CSRF cookie to `httpOnly: true`, but the SPA reads `bt_csrf_token` from `document.cookie` and sends it in `x-csrf-token`. With default settings, state-changing requests from the client will not include the token. `docker-compose.yml` compensates with `CSRF_HTTP_ONLY=false`, which means the secure default and the actual SPA contract disagree.
|
|
|
|
**Affected Files:**
|
|
- `middleware/csrf.js:12-16`
|
|
- `middleware/csrf.js:42-52`
|
|
- `client/api.js:1-16`
|
|
- `docker-compose.yml`
|
|
|
|
**Potential Fix:**
|
|
Choose one CSRF pattern and align defaults/docs: either intentionally use a readable double-submit token for the SPA, or provide a `/api/csrf-token` endpoint/header bootstrap so the cookie can stay HTTP-only.
|
|
|
|
**Severity:** MEDIUM
|
|
|
|
### 🟡 Change-Password Routes Are Globally Exempted from CSRF — MEDIUM
|
|
**Priority:** MEDIUM
|
|
**Added:** 2026-05-10 by Prime (code review)
|
|
**Type:** SECURITY
|
|
|
|
**Description:**
|
|
`server.js` sets `req.csrfSkip = true` for `/api/auth/change-password` and `/api/profile/change-password`. These routes still require the current password, so this is not an immediate account-takeover issue, but password-changing is a sensitive state mutation and should not be excluded from the same CSRF protection as other authenticated writes.
|
|
|
|
**Affected Files:**
|
|
- `server.js:70-71`
|
|
- `routes/auth.js:116-156`
|
|
- `routes/profile.js:177-230`
|
|
|
|
**Potential Fix:**
|
|
Remove the global CSRF skip for password-change routes once the SPA token flow is aligned, and keep only login/OIDC bootstrap routes exempt where no authenticated session exists yet.
|
|
|
|
**Severity:** MEDIUM
|
|
|
|
### 🟡 Notification Due-Day Math Can Miss Same-Day Reminders — MEDIUM
|
|
**Priority:** MEDIUM
|
|
**Added:** 2026-05-10 by Prime (code review)
|
|
**Type:** BUG
|
|
|
|
**Description:**
|
|
`runNotifications()` computes `diffDays` by subtracting the current timestamp from midnight on the due date and flooring the result. After midnight has passed on the due date, the difference becomes a small negative value and floors to `-1`, so due-today bills can be classified as overdue instead of `due_today`. Similar timezone/partial-day drift can affect 1-day and 3-day reminders.
|
|
|
|
**Affected Files:**
|
|
- `services/notificationService.js:172-175`
|
|
- `services/notificationService.js:213-222`
|
|
|
|
**Potential Fix:**
|
|
Normalize both dates to local date-only midnight or compare ISO date strings/calendar days before calculating reminder type. Add tests around morning/afternoon due-today execution.
|
|
|
|
**Severity:** MEDIUM
|
|
|
|
### 🟡 Upcoming Bills Allows Negative Day Windows — MEDIUM
|
|
**Priority:** MEDIUM
|
|
**Added:** 2026-05-10 by Prime (code review)
|
|
**Type:** BUG / API_CONTRACT
|
|
|
|
**Description:**
|
|
`GET /api/tracker/upcoming` caps `days` at 365 but does not enforce a lower bound or reject non-numeric input. `days=-30` creates a cutoff before today and returns no upcoming bills; `days=abc` leaves `cutoff` invalid. The endpoint contract says upcoming bills in the next N days, so invalid windows should be rejected or normalized.
|
|
|
|
**Affected Files:**
|
|
- `routes/tracker.js:245-253`
|
|
- `routes/tracker.js:284-291`
|
|
|
|
**Potential Fix:**
|
|
Parse `days` with integer validation, require `1 <= days <= 365`, and return standardized 400 errors for invalid input.
|
|
|
|
**Severity:** MEDIUM
|
|
|
|
|
|
### Architecture: Business Logic Mixed with Route Handlers
|
|
**Priority:** MEDIUM
|
|
**Added:** 2026-05-08 by Neo
|
|
|
|
**Description:**
|
|
Many routes contain business logic that should be extracted to service layers.
|
|
|
|
**Rationale:**
|
|
- `bills.js` contains `parseDueDay()`, `parseInterestRate()` — validation logic
|
|
- `tracker.js` contains date/range calculations that are reused across routes
|
|
- `admin.js` has complex OIDC config building mixed with routing
|
|
- `analytics.js` has complex date-building logic (`buildMonths`, `monthKey`, etc.)
|
|
|
|
**Implementation Notes:**
|
|
- Files to modify: Multiple route files + new service files in `/services/`
|
|
- Estimated effort: 8 hours
|
|
- Proposed structure:
|
|
```
|
|
/services/billsService.js
|
|
/services/trackerService.js
|
|
/services/analyticsService.js
|
|
/services/authService.js (existing)
|
|
/services/oidcService.js (existing)
|
|
/services/cleanupService.js (existing)
|
|
```
|
|
- Route handlers should call services, not contain business logic
|
|
|
|
### ~~Skip First-Login User Creation When ENV Seeds Users~~ ✅ COMPLETED (v0.22.3)
|
|
**Moved to HISTORY.md**
|
|
|
|
### ~~No Rollback Capability for Failed Migrations~~ ✅ COMPLETED (v0.23.1)
|
|
**Moved to HISTORY.md**
|
|
|
|
### ~~Limited Error Handling and Logging for Migrations~~ ✅ COMPLETED (v0.23.0)
|
|
**Moved to HISTORY.md**
|
|
|
|
**Rationale:**
|
|
- Migration errors are silent or unclear
|
|
- No logging of which migration failed or why
|
|
- No way to diagnose schema inconsistencies
|
|
- Risk: slow debugging on production issues
|
|
|
|
**Implementation Notes:**
|
|
- Add detailed logging: `[migration] Applying v0.20.0: Add user_groups table`
|
|
- Include timing: `[migration] v0.20.0 completed in 234ms`
|
|
- Log precondition checks: `[migration] Checking: table_exists('users')`
|
|
- Error log with context: `[migration-error] v0.20.0 failed: UNIQUE constraint failed on users.username`
|
|
|
|
---
|
|
|
|
### 🔵 LOW
|
|
|
|
|
|
### 🔵 Export Formats Include Sensitive Bill Credential Fields by Default — LOW
|
|
**Priority:** LOW
|
|
**Added:** 2026-05-10 by Prime (code review)
|
|
**Type:** SECURITY / PRIVACY
|
|
|
|
**Description:**
|
|
Full user exports include `website`, `username`, `account_info`, notes, and monthly notes by default. This may be intended for portability, but it turns every Excel/SQLite export into a high-sensitivity artifact and there is no lightweight/redacted export option.
|
|
|
|
**Affected Files:**
|
|
- `routes/export.js:88-153`
|
|
- `routes/export.js:156-199`
|
|
- `routes/profile.js:236-254`
|
|
|
|
**Potential Fix:**
|
|
Add explicit UI copy warning that exports may contain account metadata, and consider a redacted export mode that excludes credential/account fields and free-form notes.
|
|
|
|
**Severity:** LOW
|
|
|
|
### ~~🔵 Duplicate Local Login Route Increases Auth Drift Risk — LOW~~ ✅ FIXED (v0.23.2)
|
|
**Moved to HISTORY.md**
|
|
|
|
|
|
### Add comprehensive unit and integration tests
|
|
**Priority:** LOW
|
|
**Added:** 2026-05-08 by Scarlett
|
|
|
|
**Description:**
|
|
Currently no unit tests exist for components or hooks. The only testing appears to be functional tests in `test-functional.js`. Component-level testing is missing.
|
|
|
|
**Rationale:**
|
|
Code quality and maintainability. Unit tests catch regressions and document component behavior. Bill Tracker has complex business logic (bill calculations, monthly state, analytics) that should be tested.
|
|
|
|
**Implementation Notes:**
|
|
- Set up Jest + React Testing Library
|
|
- Test key components: BillModal, TrackerPage row, BillsTableInner
|
|
- Test hooks: useAuth, custom form hooks
|
|
- Test utility functions in `client/lib/utils.js`
|
|
- Consider vitest for faster test execution
|
|
- Add CI integration for test execution
|
|
- Files likely to be modified: Add `client/test/` directory, add `jest.config.cjs`
|
|
- Estimated effort: 8-12 hours for baseline coverage
|
|
|
|
### Features: Missing Export for User-Specific Reports
|
|
**Priority:** LOW
|
|
**Added:** 2026-05-08 by Neo
|
|
|
|
**Description:**
|
|
No built-in way to export filtered data (e.g., "all bills in category X for last 6 months").
|
|
|
|
**Rationale:**
|
|
- `/api/analytics/summary` exists but returns JSON only
|
|
- Users cannot generate Excel/PDF reports
|
|
- No programmatic way to get export links for specific filters
|
|
- `/api/export/user-excel` exports everything, not filtered views
|
|
|
|
**Implementation Notes:**
|
|
- Files to modify: `/home/kaspa/.openclaw/Projects/bill-tracker/routes/export.js`
|
|
- Estimated effort: 6 hours
|
|
- Add endpoints:
|
|
- `GET /api/export/user-excel?category_id=1&start=2026-01&end=2026-06`
|
|
- `GET /api/export/user-json?filter=bills&status=missed`
|
|
- Add report title/description to export metadata
|
|
|
|
### Features: Missing Bill Grouping and Reorganization API
|
|
**Priority:** LOW
|
|
**Added:** 2026-05-08 by Neo
|
|
|
|
**Description:**
|
|
No way to reorder bills, drag-and-drop, or group by custom criteria.
|
|
|
|
**Rationale:**
|
|
- `bills` table has `due_day` ordering but no manual sort order
|
|
- Frontend likely orders by `due_day` only
|
|
- Users cannot create bill groups or categories for bills
|
|
- No way to mark bills as "hidden" or "archived" without deactivating
|
|
|
|
**Implementation Notes:**
|
|
- Files to modify: `/home/kaspa/.openclaw/Projects/bill-tracker/db/schema.sql`, `/routes/bills.js`
|
|
- Estimated effort: 6 hours
|
|
- Add:
|
|
- `sort_order` column to bills table (default NULL, ordered first by sort_order then due_day)
|
|
- `PUT /api/bills/reorder` endpoint accepting `{bill_id: new_index}`
|
|
- `PUT /api/bills/:id/archived` to soft-dearchive (sets `archived` flag)
|
|
|
|
---
|
|
|
|
### 💭 NICE TO HAVE
|
|
|
|
### Add consistent form state management pattern
|
|
**Priority:** MEH
|
|
**Added:** 2026-05-08 by Scarlett
|
|
|
|
**Description:**
|
|
Form state management is inconsistent across components. Some use `useState` for each field, others use form libraries. Validation patterns vary.
|
|
|
|
**Rationale:**
|
|
Consistency and maintainability. A consistent pattern makes it easier to add new forms and reduce bugs.
|
|
|
|
**Implementation Notes:**
|
|
- Consider react-hook-form for complex forms
|
|
- Create reusable form field components (InputField, SelectField, etc.)
|
|
- Standardize validation approach
|
|
- Files likely to be modified: `client/components/*.jsx`
|
|
- Estimated effort: 4-6 hours for migration
|
|
|
|
---
|
|
|
|
## Template for New Recommendations
|
|
|
|
```markdown
|
|
### [Feature Name]
|
|
**Priority:** CRITICAL / HIGH / MEDIUM / LOW / MEH
|
|
**Added:** YYYY-MM-DD by [Agent]
|
|
|
|
**Description:**
|
|
Brief description of the improvement.
|
|
|
|
**Rationale:**
|
|
Why this matters.
|
|
|
|
**Implementation Notes:**
|
|
- Technical approach
|
|
- Files likely to be modified
|
|
- Estimated effort
|
|
|
|
**Depends On:**
|
|
Any prerequisites or blocking issues.
|
|
```
|
|
|
|
## Completed Items
|
|
|
|
### ✅ Security: Rate Limiting on /api/about-admin — MEDIUM
|
|
**Completed:** 2026-05-09 (v0.19.0)
|
|
**Fix:** `adminActionLimiter` (30 req/15min) applied to `/api/about-admin` route.
|
|
|
|
### ✅ Security: Markdown Sanitization in AboutPage — MEDIUM
|
|
**Completed:** 2026-05-09 (v0.19.0)
|
|
**Fix:** `rehype-sanitize` added to `AboutPage.jsx` ReactMarkdown component.
|
|
|
|
### ✅ Security: aboutAdmin() in API Client — LOW
|
|
**Completed:** 2026-05-09 (v0.19.0)
|
|
**Fix:** `aboutAdmin` endpoint function added to `client/api.js`.
|
|
|
|
---
|