Files
gravl/.planning/codebase/CONCERNS.md
T
2026-02-15 21:49:31 +01:00

18 KiB
Raw Blame History

Codebase Concerns

Analysis Date: 2026-02-15

Tech Debt

Hardcoded Program ID in Backend:

  • Issue: Multiple API endpoints hardcode program_id = 1 in queries, preventing multi-program support
  • Files: backend/src/index.js (lines 27, 198, 386, 410)
  • Impact: Cannot support multiple training programs; system is locked to single PPL program. Future features requiring program selection will require significant refactoring
  • Fix approach: Add program_id parameter to endpoints; refactor to accept program ID from request or user preferences

Hardcoded User ID Default:

  • Issue: Backend defaults to user_id = 1 when not provided; frontend also uses fallback user?.id || 1
  • Files: backend/src/index.js (line 290), frontend/src/App.jsx (line 21), frontend/src/pages/ProfilePage.jsx (lines 25-27, 48)
  • Impact: Multi-user isolation broken; all users can see/modify each other's data if API auth fails. Critical security concern
  • Fix approach: Remove all fallback user IDs; enforce auth token verification; validate user ownership on all endpoints

Single Backend File Architecture:

  • Issue: All 425 lines of API logic in one file (backend/src/index.js); no separation into routes, controllers, middleware
  • Files: backend/src/index.js
  • Impact: Difficult to maintain, test, or extend. Mixed concerns (auth, database, business logic) in same file. No clear patterns for new endpoints
  • Fix approach: Refactor into: routes/, controllers/, middleware/, services/ directories

No Request Validation:

  • Issue: No input validation on any API endpoints; accepts any data and passes to database
  • Files: backend/src/index.js (all POST/PUT routes: lines 35-50, 103-118, 121-136, 153-168, 299-329)
  • Impact: SQL injection risk (mitigated by parameterized queries but semantic validation missing), malformed data in database, inconsistent state
  • Fix approach: Add validation library (e.g., joi, zod); validate types, ranges, required fields before database operations

Weak Default JWT Secret:

  • Issue: JWT secret defaults to plain string 'gravl-secret-key-change-in-production' if env var not set
  • Files: backend/src/index.js (line 9)
  • Impact: If deployment forgets to set JWT_SECRET env var, all tokens can be forged. Auth completely broken in that scenario
  • Fix approach: Require JWT_SECRET as mandatory env var; fail at startup if not set; remove default

Exposed Database Password in Docker Compose:

  • Issue: Database password hardcoded in plaintext in docker-compose.yml
  • Files: docker-compose.yml (line 12: DB_PASSWORD=homelab_postgres_2026)
  • Impact: Secret visible in git history and version control. Deployed as plain text to running containers
  • Fix approach: Use .env file (gitignored) with env var substitution; never commit secrets

Hardcoded Database Connection Defaults:

  • Issue: Database credentials have weak defaults if env vars missing
  • Files: backend/src/index.js (lines 11-17)
  • Impact: If env vars not set, connects with default user/password/database
  • Fix approach: Require critical env vars (DB_HOST, DB_PORT, DB_USER, DB_PASSWORD, DB_NAME) as mandatory at startup

No Error Handling for Database Failures:

  • Issue: Database errors logged to console but no graceful degradation or retry logic
  • Files: backend/src/index.js (lines 46-50, 64-67, 97-100, 114-117, 132-135, 164-167, 189-191, 227-230, 245-248, 275-278, 292-295, 325-328, 379-382, 416-419)
  • Impact: Client gets generic "Database error" message; no way to debug; connection pool exhaustion not handled
  • Fix approach: Add structured error logging; implement connection retry logic; differentiate error messages (auth vs DB vs validation)

Vague Error Messages:

  • Issue: Many endpoints return generic { error: 'Server error' } without details
  • Files: backend/src/index.js (lines 49, 66, 116, 134, 166, 191, 229, 248, 278, 295, 328, 381, 418)
  • Impact: Frontend cannot distinguish between different failure modes; users see unhelpful messages; debugging impossible
  • Fix approach: Use structured error codes (e.g., ERR_EMAIL_EXISTS, ERR_INVALID_CREDENTIALS, ERR_DB_UNAVAILABLE)

Known Bugs

User ID Fallback Breaks Multi-User:

  • Symptoms: Any endpoint called without proper auth gets user ID 1; all data accessible to wrong users
  • Files: backend/src/index.js (line 290: user_id || 1)
  • Trigger: Call /api/logs/last/...?program_exercise_id=X without user_id parameter, or auth fails silently
  • Workaround: Frontend always provides user ID; but if auth token expires mid-session, falls back to user 1

Progression Calculation Assumes User Context:

  • Symptoms: /api/progression/:id endpoint without auth header returns data for user_id=1, not current user
  • Files: backend/src/index.js (line 334: user_id || 1)
  • Trigger: Make unauthenticated request to progression endpoint
  • Workaround: Frontend includes user_id in query param, but no validation that it matches auth token

Missing Auth Validation on Read Endpoints:

  • Symptoms: /api/logs, /api/logs/last/..., /api/progression/... do not require auth; anyone can see anyone's data
  • Files: backend/src/index.js (lines 252-279, 282-296, 332-383 — none have authMiddleware)
  • Trigger: Unauthenticated request to any of these endpoints returns full data
  • Workaround: None; endpoint is truly public

Profile Fetch Endpoints Bypass Auth:

  • Symptoms: /api/user/profile GET and other profile endpoints sometimes called without token
  • Files: frontend/src/pages/ProfilePage.jsx (lines 25-27 make calls with optional header)
  • Trigger: Token expires or not in localStorage; frontend still tries to fetch profile
  • Workaround: Redirect on 401, but data may be partially loaded

Onboarding Does Not Validate Strength Input:

  • Symptoms: Can enter non-numeric strength values; API accepts and stores as invalid data
  • Files: frontend/src/pages/OnboardingWizard.jsx (lines 150-153); backend/src/index.js (no validation)
  • Trigger: Enter "abc" in 1RM field; submit saves to database
  • Workaround: None; data is corrupted

Security Considerations

Authentication Not Required on Data Endpoints:

  • Risk: Public endpoints /api/logs, /api/progression/..., /api/logs/last/... expose all user workout data without auth
  • Files: backend/src/index.js (routes at lines 252, 282, 332)
  • Current mitigation: None; these routes are public
  • Recommendations: Add authMiddleware to all endpoints that return user data; validate user_id from request matches decoded token

User ID Not Validated on Update Operations:

  • Risk: Client sends PUT request with any user_id; no validation that it matches auth token
  • Files: backend/src/index.js (lines 103, 121, 153, 299)
  • Current mitigation: Database constraint on user_id, but not enforced at API level
  • Recommendations: Extract user_id from JWT token (req.user.id); never accept from request body; validate ownership

Password Hashing Strength Acceptable But Not Tested:

  • Risk: Uses bcryptjs with rounds=10 (line 39); no test for hash strength
  • Files: backend/src/index.js (line 39)
  • Current mitigation: bcryptjs with 10 rounds is secure
  • Recommendations: Increase to 12+ rounds; add integration test for password hashing

JWT Token Expiry Too Long:

  • Risk: 30-day token expiry is long; stolen token has extended window
  • Files: backend/src/index.js (lines 44, 61)
  • Current mitigation: Token stored in localStorage; vulnerable to XSS
  • Recommendations: Reduce to 1-7 days; implement refresh token rotation; consider httpOnly cookies

No CORS Validation:

  • Risk: CORS enabled for all origins (app.use(cors()))
  • Files: backend/src/index.js (line 19)
  • Current mitigation: None; frontend is localhost during dev, but prod deployment may not restrict
  • Recommendations: Add whitelist: cors({ origin: process.env.FRONTEND_URL })

Database Connection Not SSL in Docker:

  • Risk: PostgreSQL connection from Docker unencrypted if over network
  • Files: backend/src/index.js (lines 11-17); docker-compose.yml
  • Current mitigation: On internal homelab network, but not encrypted
  • Recommendations: Add ssl: true to pool config if connecting over untrusted network

Performance Bottlenecks

N+1 Query Problem in Program Endpoints:

  • Problem: /api/programs/:id loads program, then for each day, joins exercises separately
  • Files: backend/src/index.js (lines 196-231)
  • Cause: Single query with complex LEFT JOINs and json_agg; works but could be optimized with batching
  • Improvement path: Already optimized with single query; no issue here. Performance is acceptable

No Database Indexes on Common Queries:

  • Problem: workout_logs queries filter by (user_id, date, program_exercise_id) but indexes only on two columns
  • Files: db/init.sql (line 77); backend/src/index.js (lines 252-279)
  • Cause: Missing composite index on (user_id, date) and separate on program_exercise_id
  • Improvement path: Add CREATE INDEX idx_workout_logs_user_date_exercise ON workout_logs(user_id, date, program_exercise_id)

Measurements Fetch Not Limited:

  • Problem: /api/user/measurements returns up to 100 rows without pagination
  • Files: backend/src/index.js (line 142)
  • Cause: LIMIT 100 hardcoded; if user has years of data, transfers unnecessary payload
  • Improvement path: Add limit and offset query params; default to last 30 records

Strength History Not Limited:

  • Problem: /api/user/strength also LIMIT 100
  • Files: backend/src/index.js (line 174)
  • Cause: Same as measurements
  • Improvement path: Add pagination; default to last 12 records (1 year monthly checks)

Frontend Fetches All Logs for All Exercises at Once:

  • Problem: App.jsx fetchLogs() makes one request per exercise (loop)
  • Files: frontend/src/App.jsx (lines 35-51)
  • Cause: Not batched; if 6 exercises, makes 6 API calls sequentially
  • Improvement path: Batch into single endpoint; return all day's logs in one query

Progression Calculation Fetches Last 10 Logs Per Request:

  • Problem: /api/progression/:id fetches last 10 logs for every exercise opened
  • Files: backend/src/index.js (line 354)
  • Cause: Called on component expand; if user expands 6 exercises, 6 queries
  • Improvement path: Batch progression calculations; include in WorkoutPage's initial fetch

Fragile Areas

AuthContext Token Refresh Not Automatic:

  • Files: frontend/src/context/AuthContext.jsx
  • Why fragile: 30-day token expiry means users logged in for <30d get sudden 401. No refresh token mechanism. Token stored in localStorage (XSS vulnerable)
  • Safe modification: Add refresh token endpoint; implement automatic refresh-before-expiry; consider httpOnly cookies
  • Test coverage: No tests; AuthContext has no test file

Profile Fetch Without Error Boundaries:

  • Files: frontend/src/pages/ProfilePage.jsx (lines 22-42)
  • Why fragile: Fetch errors caught in console but no UI feedback; Promise.all() fails if one fetch fails, but all three fetches (profile, measurements, strength) are separate queries
  • Safe modification: Wrap each fetch in try-catch separately; show partial data if some fail; add error toast
  • Test coverage: No tests

Onboarding State Not Persisted During Request:

  • Files: frontend/src/pages/OnboardingWizard.jsx (lines 24-72)
  • Why fragile: If user fills all 4 steps and network fails during save, form clears but data lost. No draft save
  • Safe modification: Auto-save to localStorage after each step; restore on remount
  • Test coverage: No tests; body fat calculation not tested

Warmup Completion State Lost on Navigation:

  • Files: frontend/src/pages/WorkoutPage.jsx (lines 51-53, 79-87)
  • Why fragile: completedWarmups is local state in component; navigating away loses progress. No persistence
  • Safe modification: Save to localStorage keyed by date+day
  • Test coverage: No tests

Exercise Progression Display Race Condition:

  • Files: frontend/src/pages/WorkoutPage.jsx (lines 48-67)
  • Why fragile: loadProgressions() called in useEffect with [day] dependency; if day changes rapidly, multiple requests in flight; setState after unmount possible
  • Safe modification: Add cleanup function; abort controller for fetch; cache by day ID
  • Test coverage: No tests

Database Schema Missing User FK in Measurements:

  • Files: db/init.sql (lines 64-74)
  • Why fragile: user_measurements table has no explicit FOREIGN KEY to users table; can create orphaned records; cascading delete not enforced
  • Safe modification: Add user_id INTEGER REFERENCES users(id) ON DELETE CASCADE NOT NULL
  • Test coverage: Schema not tested

Hardcoded Warmup Exercises Not Database-Driven:

  • Files: frontend/src/pages/WorkoutPage.jsx (lines 5-35)
  • Why fragile: Warmup data hardcoded in component; if new muscle group added to exercises, no warmups for it; mapping is manual and fragile
  • Safe modification: Move to database table warmup_exercises(muscle_group, name, duration, type); fetch on page load
  • Test coverage: No tests; mapping logic untested

Scaling Limits

Single Program Per User:

  • Current capacity: System assumes all users follow program_id=1
  • Limit: Cannot support multiple programs or user switching between programs
  • Scaling path: Refactor endpoints to accept program_id; add user_programs join table; allow user to select active program

No Pagination on History Endpoints:

  • Current capacity: /api/user/measurements returns LIMIT 100; /api/user/strength returns LIMIT 100
  • Limit: If user has >100 measurements (100+ days of data), response grows indefinitely
  • Scaling path: Implement cursor-based pagination; return 20-30 records per page; add date range filters

Database Connection Pool Not Configured:

  • Current capacity: pg module defaults to pool size 10
  • Limit: 10 concurrent connections; 11th request queues
  • Scaling path: Add explicit pool configuration: max: 20, min: 5 adjusted per load; monitor with pg_stat_activity

Logs Stored Flat Without Aggregate Summary:

  • Current capacity: Every set logged individually; querying 100 workouts × 6 exercises × 3 sets = 1800 rows
  • Limit: As user history grows, workout fetches slow down
  • Scaling path: Add workout_sessions table with aggregate stats; denormalize common queries

Frontend Loads Entire Program Structure:

  • Current capacity: /api/programs/:id returns all days + all exercises in one response
  • Limit: With 12+ exercises per day, response grows; not a problem now but scales poorly
  • Scaling path: Lazy-load exercises per day; separate endpoint for day details; cache aggressively

Dependencies at Risk

Express 4.x Minor Versions:

  • Risk: No automatic security updates; express vulnerabilities not patched unless manually updated
  • Impact: Known CVEs in middleware could be exploited
  • Migration plan: Upgrade to Express 5.x (breaking changes); or add npm audit fix to CI pipeline

bcryptjs No Longer Maintained:

  • Risk: bcryptjs is unmaintained library; use native Node.js crypto instead
  • Impact: Security bugs in bcryptjs won't be fixed; but practical risk is low (bcrypt algorithm is solid)
  • Migration plan: Switch to bcrypt (native binding) or Node.js crypto.scrypt() + built-in functions

jsonwebtoken Known Vulns:

  • Risk: jsonwebtoken has history of algorithm confusion vulnerabilities (prior versions)
  • Impact: Current version 9.0.2 (in package.json) is recent; likely patched
  • Migration plan: Keep up with minor/patch updates; add npm audit to CI

pg Library Version:

  • Risk: pg 8.11.3 is from 2023; no known critical issues but check advisories
  • Impact: Low; PostgreSQL driver is stable
  • Migration plan: Keep updated; monitor npm advisories

Missing Critical Features

No Input Sanitization:

  • Problem: User inputs not sanitized before database storage; e.g., XSS in exercise names, injection in auth fields
  • Blocks: Any user-generated content features (notes, comments); social features
  • Fix approach: Add input sanitization library (e.g., DOMPurify for frontend, xss for backend); validate at both layers

No Rate Limiting:

  • Problem: No rate limits on auth endpoints; brute force attack possible on /api/auth/login
  • Blocks: Public deployment; production security
  • Fix approach: Add express-rate-limit middleware; 5 attempts per 15 min per IP

No Audit Logging:

  • Problem: No record of who did what when; can't detect unauthorized access or data changes
  • Blocks: Compliance requirements; forensics
  • Fix approach: Add audit_logs table; log all create/update/delete with user_id, timestamp, action

No Soft Deletes:

  • Problem: No way to recover deleted data; hard deletes cascade immediately
  • Blocks: Undo features; data recovery
  • Fix approach: Add deleted_at column to tables; use soft deletes; implement undelete mechanism

No API Versioning:

  • Problem: No v1, v2 paths; breaking changes would affect all clients
  • Blocks: Safe API evolution
  • Fix approach: Add /api/v1/... prefix; maintain backward compatibility when possible

Test Coverage Gaps

No Backend Tests:

  • What's not tested: All 425 lines of backend/src/index.js have zero test coverage
  • Files: backend/src/index.js
  • Risk: Auth logic not verified; SQL injection prevention untested; progression calculation not validated; error paths not covered
  • Priority: High

No Frontend Unit Tests:

  • What's not tested: No Jest/Vitest config; components not tested
  • Files: frontend/src/**/*.jsx (all files)
  • Risk: UI bugs not caught; hooks logic untested; state transitions not verified
  • Priority: High

No Integration Tests:

  • What's not tested: API-to-database flow untested; full workout logging flow untested
  • Risk: Database schema changes break endpoints; race conditions in concurrent requests not caught
  • Priority: Medium

No E2E Tests:

  • What's not tested: User flows untested (register → onboard → log workout → progress)
  • Risk: Broken onboarding, broken login, navigation issues in production not caught
  • Priority: Medium

Auth Logic Not Tested:

  • What's not tested: Token verification, expiry, malformed tokens, missing auth headers
  • Files: backend/src/index.js (lines 22-29); frontend/src/context/AuthContext.jsx
  • Risk: Auth bypass vulnerabilities not detected
  • Priority: Critical

Progression Calculation Not Tested:

  • What's not tested: Algorithm for suggesting weight increases
  • Files: backend/src/index.js (lines 332-383)
  • Risk: Incorrect progression logic goes unnoticed; users stay on same weight or jump too much
  • Priority: High

Database Schema Not Validated:

  • What's not tested: Foreign key constraints, cascading deletes, data types
  • Files: db/init.sql
  • Risk: Invalid data created; orphaned records; type mismatches
  • Priority: Medium

Concerns audit: 2026-02-15