docs: map existing codebase

This commit is contained in:
2026-03-11 18:20:14 +01:00
parent 04cbb846d1
commit b9e4d82d1a
7 changed files with 1121 additions and 0 deletions

View File

@@ -0,0 +1,182 @@
# 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*