restart

.planning/codebase/CONCERNS.md

@@ -1,182 +0,0 @@
-# Codebase Concerns
-
-**Analysis Date:** 2026-03-11
-
-## Tech Debt
-
-**Date parsing without validation:**
-- Issue: In `UpdateBudget` handler, date parsing errors are silently ignored with blank identifiers
-- Files: `backend/internal/api/handlers.go:322-323`
-- Impact: Invalid dates passed to the server may silently become zero values, leading to incorrect budget date ranges. The API accepts malformed dates without reporting errors.
-- Fix approach: Check errors from `time.Parse()` and return HTTP 400 with a descriptive error message before updating the budget.
-
-**Silent error suppression in API client:**
-- Issue: In the fetch error handler, JSON parse errors are silently swallowed with `.catch(() => ({}))`
-- Files: `frontend/src/lib/api.ts:14`
-- Impact: If the server returns non-JSON error responses (e.g., plain text HTML error pages), the error message is lost and users see a generic error. Network/server issues become indistinguishable from API errors.
-- Fix approach: Log the raw response or preserve error details. Return a meaningful error message when JSON parsing fails.
-
-**Hardcoded default database URL with plaintext credentials:**
-- Issue: Default database URL includes plaintext username and password in code
-- Files: `backend/cmd/server/main.go:33`
-- Impact: Credentials appear in logs, version control history, and error messages. The `sslmode=disable` allows unencrypted database connections by default.
-- Fix approach: Remove the default fallback, require `DATABASE_URL` to be explicitly set in production. Use `sslmode=require` and implement proper secrets management.
-
-**Placeholder default session secret:**
-- Issue: Session secret defaults to `"change-me-in-production"` if not set via environment
-- Files: `backend/cmd/server/main.go:34`
-- Impact: If `SESSION_SECRET` is not configured in deployment, all sessions use a hardcoded insecure secret, allowing anyone with the code to forge tokens.
-- Fix approach: Fail startup with a clear error if `SESSION_SECRET` is not set. Make it required for production builds.
-
-## Security Considerations
-
-**Hardcoded CORS whitelist for localhost only:**
-- Risk: CORS only allows `http://localhost:5173` and `http://localhost:8080`, but this is production code that will be deployed. The hardcoded origins require code changes to deploy.
-- Files: `backend/internal/api/router.go:20-21`
-- Current mitigation: None (will break in production unless modified)
-- Recommendations: Make CORS origins configurable via environment variables. Use a secure default (e.g., `AllowedOrigins: []string{}` or read from env). Document required configuration.
-
-**Insufficient cookie security flags:**
-- Risk: Session cookies lack the `Secure` flag, which is critical for HTTPS environments
-- Files: `backend/internal/auth/auth.go:91-98`
-- Current mitigation: `HttpOnly` and `SameSite=Lax` are set, but `Secure` flag is missing
-- Recommendations: Add `Secure: true` to `SetSessionCookie()`. Make this configurable for development vs. production.
-
-**No CSRF protection:**
-- Risk: API endpoints accept POST/PUT/DELETE from any origin that passes CORS check. No CSRF tokens in use.
-- Files: `backend/internal/api/router.go` (all mutation endpoints)
-- Current mitigation: SameSite=Lax provides some protection for browser-based attacks, but is insufficient for API security
-- Recommendations: Implement CSRF token validation for state-changing operations, or use SameSite=Strict if browser support permits.
-
-**Unencrypted database connections by default:**
-- Risk: `sslmode=disable` in default DATABASE_URL allows plaintext communication with PostgreSQL
-- Files: `backend/cmd/server/main.go:33`
-- Current mitigation: None
-- Recommendations: Change default to `sslmode=require`. Document that production must use encrypted connections.
-
-**No input validation on budget/category names:**
-- Risk: User-provided text fields (name, notes) have no length limits or content validation
-- Files: `backend/internal/api/handlers.go` (CreateCategory, CreateBudget, CreateBudgetItem accept raw strings)
-- Current mitigation: Database has TEXT columns but no constraints
-- Recommendations: Add max length validation (e.g., 255 chars for names, 5000 for notes) before persisting. Return 422 for validation failures.
-
-**No rate limiting:**
-- Risk: No rate limiting on authentication endpoints. Brute force attacks on login/register are not mitigated.
-- Files: `backend/internal/api/router.go` (auth routes have no middleware)
-- Current mitigation: None
-- Recommendations: Implement rate limiting middleware per IP or per email address for auth endpoints.
-
-## Performance Bottlenecks
-
-**No pagination on category/budget listing:**
-- Problem: `ListCategories` and `ListBudgets` fetch all records without limits
-- Files: `backend/internal/db/queries.go:105-124, 163-181`
-- Cause: Simple SELECT without LIMIT. With thousands of categories/budgets, this becomes slow.
-- Improvement path: Add LIMIT/OFFSET parameters, return paginated results. Frontend should request by page.
-
-**N+1 query problem in budget copy:**
-- Problem: `CopyBudgetItems` calls `GetBudget()` twice (for verification), then does one bulk INSERT. If called frequently, verification queries add unnecessary overhead.
-- Files: `backend/internal/db/queries.go:304-319`
-- Cause: Two separate queries to verify ownership before the copy operation
-- Improvement path: Combine budget verification with the INSERT in a single transaction. Use CHECK constraints or triggers instead of application-level verification.
-
-**Totals computed in application code every time:**
-- Problem: `computeTotals()` is called for every `GetBudgetWithItems()` call, doing arithmetic on all items in memory
-- Files: `backend/internal/db/queries.go:269-301`
-- Cause: No caching, no database-side aggregation
-- Improvement path: Pre-compute totals and store in `budget` table, or use PostgreSQL aggregation functions (GROUP BY, SUM) in the query.
-
-## Fragile Areas
-
-**Date parsing without type safety:**
-- Files: `backend/internal/api/handlers.go:260-269, 322-323`
-- Why fragile: Dates are parsed as strings and silently fail. Different handlers handle errors differently (one validates, one ignores).
-- Safe modification: Extract date parsing into a helper function that validates and returns errors. Use typed request structs with custom JSON unmarshalers.
-- Test coverage: No tests visible for invalid date inputs.
-
-**Budget item deletion cascades (foreign key constraint):**
-- Files: `backend/migrations/001_initial.sql` (budget_items.category_id has ON DELETE RESTRICT)
-- Why fragile: Categories cannot be deleted if they have budget items referencing them. Users see opaque "foreign key violation" errors.
-- Safe modification: Change to ON DELETE SET NULL if nullability is acceptable, or implement soft deletes. Add validation to prevent orphaned categories.
-- Test coverage: No tests for category deletion scenarios.
-
-**Async state management in useBudgets hook:**
-- Files: `frontend/src/hooks/useBudgets.ts`
-- Why fragile: `selectBudget()` sets loading=true but fetchList doesn't. Race conditions if budget is selected while list is being fetched. No error boundaries.
-- Safe modification: Unify loading states, cancel pending requests when switching budgets, handle errors explicitly.
-- Test coverage: No tests for concurrent operations.
-
-**JSON error handling in API client:**
-- Files: `frontend/src/lib/api.ts:14`
-- Why fragile: `.catch(() => ({}))` returns empty object on any error. Downstream code treats empty object as valid response.
-- Safe modification: Check `res.ok` before calling `.json()`. Return typed error with details preserved.
-- Test coverage: No tests for malformed response bodies.
-
-## Missing Critical Features
-
-**No input validation framework:**
-- Problem: Handlers validate data individually with ad-hoc checks (empty email/password)
-- Blocks: Harder to enforce consistent validation across endpoints. SQL injection is mitigated by parameterized queries, but semantic validation is manual.
-- Impact: Each new endpoint requires defensive programming. Easy to miss validation cases.
-
-**No password strength requirements:**
-- Problem: Registration accepts any non-empty password
-- Blocks: Users can set weak passwords; no minimum length, complexity checks
-- Impact: Security of accounts depends entirely on user discipline.
-
-**OIDC not implemented:**
-- Problem: OIDC endpoints exist but return "not implemented"
-- Blocks: Alternative authentication methods unavailable
-- Impact: Feature is advertised in code but non-functional.
-
-**No email verification:**
-- Problem: Users can register with any email address; no verification step
-- Blocks: Typos create invalid accounts. Email communication impossible.
-- Impact: Poor UX for password resets or account recovery.
-
-**No transaction support in budget copy:**
-- Problem: `CopyBudgetItems` is not atomic. If INSERT fails partway, database is left in inconsistent state.
-- Blocks: Reliable bulk operations
-- Impact: Data corruption possible during concurrent operations.
-
-## Test Coverage Gaps
-
-**No backend unit tests:**
-- What's not tested: API handlers, database queries, auth logic, business logic
-- Files: `backend/internal/api/handlers.go`, `backend/internal/db/queries.go`, `backend/internal/auth/auth.go`
-- Risk: Regressions in critical paths go undetected. Date parsing bugs, permission checks, decimal arithmetic errors are not caught.
-- Priority: High
-
-**No frontend component tests:**
-- What's not tested: useBudgets hook, useAuth hook, form validation, error handling
-- Files: `frontend/src/hooks/`, `frontend/src/pages/`
-- Risk: User interactions fail silently. State management bugs are caught only by manual testing.
-- Priority: High
-
-**No integration tests:**
-- What's not tested: Complete workflows (register → create budget → add items → view totals)
-- Files: All
-- Risk: Multiple components may work individually but fail together. Database constraints aren't exercised.
-- Priority: Medium
-
-**No E2E tests:**
-- What's not tested: Full user flows in a real browser
-- Files: All
-- Risk: Frontend-specific issues (layout, accessibility, form submission) are missed.
-- Priority: Medium
-
-## Dependencies at Risk
-
-**No version pinning in Go modules:**
-- Risk: Transitive dependencies can break (jwt library major version bumps, pgx changes)
-- Impact: Uncontrolled upgrades break API compatibility
-- Migration plan: Use `go.mod` with explicit versions, run `go mod tidy` in CI. Test with vulnerable versions detected by `govulncheck`.
-
-**Frontend uses `bun` as package manager:**
-- Risk: If bun project stalls or has unresolved bugs, switching to npm/pnpm requires rewriting lockfiles
-- Impact: Vendor lock-in to a newer, less battle-tested tool
-- Migration plan: Keep package.json syntax compatible. Use `bun add`/`bun remove` instead of editing package.json manually.
-
----
-
-*Concerns audit: 2026-03-11*