110 lines
5.6 KiB
Markdown
110 lines
5.6 KiB
Markdown
# DocFast Codebase Audit — 2026-02-16
|
|
|
|
## Priority 1: HIGH IMPACT (Do Now)
|
|
|
|
### 1. Structured Logging + Request IDs
|
|
- **Current:** `console.log/error` everywhere — no request correlation, no structured format
|
|
- **Impact:** Debugging production issues is nearly impossible. Can't trace a request through the system.
|
|
- **Fix:** Add request ID middleware, use structured JSON logging (pino)
|
|
|
|
### 2. Graceful Error Handling in Browser Pool
|
|
- **Current:** `acquirePage()` hangs indefinitely if all pages are busy and queue is full — the QUEUE_FULL error only fires if `pdfQueue.length >= MAX_QUEUE_SIZE`, but the queue size check happens AFTER the rate limit check (which is in middleware), so the request already passed rate limiting
|
|
- **Current:** No timeout on page operations — if Puppeteer hangs, the request hangs forever
|
|
- **Fix:** Add timeout to `acquirePage()`, add overall request timeout for PDF generation
|
|
|
|
### 3. Missing Error Handling in Async Operations
|
|
- **Current:** Several fire-and-forget patterns that silently swallow errors:
|
|
- `sendVerificationEmail().catch(err => console.error(...))` — email failure is silent to user
|
|
- `pool.query(...).catch(console.error)` in verifyTokenSync — DB write failures silently ignored
|
|
- `saveUsageEntry().catch(console.error)` — usage tracking silently fails
|
|
- **Impact:** Users could sign up but never receive verification email, with no feedback
|
|
|
|
### 4. Input Validation Gaps
|
|
- **Current:** Convert endpoints accept any JSON body. No max length on HTML input (only 2MB express limit). No validation on PDF options (format, margin values)
|
|
- **Current:** Template rendering passes user data directly to template render functions — no field validation against the schema
|
|
- **Fix:** Validate format values (A4, Letter, etc.), margin format, reject obviously malicious HTML sizes
|
|
|
|
### 5. Memory Leak Risk in Verification Cache
|
|
- **Current:** `verificationsCache` array grows unbounded — every verification ever created stays in memory
|
|
- **Current:** `rateLimitStore` map never gets cleaned up proactively (only on check)
|
|
- **Fix:** Add periodic cleanup, TTL-based eviction
|
|
|
|
### 6. SEO Improvements
|
|
- **Current:** Basic meta description, no Open Graph tags, no structured data, no sitemap.xml, no robots.txt
|
|
- **Fix:** Add OG tags, Twitter cards, JSON-LD structured data, sitemap, robots.txt
|
|
|
|
### 7. Response Headers & Security
|
|
- **Current:** API responses don't include `X-Request-Id` header
|
|
- **Current:** No `Cache-Control` headers on static assets
|
|
- **Current:** CSP policy from helmet is default — could be tighter
|
|
- **Fix:** Add proper caching headers, request ID, tighten CSP
|
|
|
|
## Priority 2: MEDIUM IMPACT
|
|
|
|
### 8. API Response Consistency
|
|
- **Current:** Mixed response formats:
|
|
- Signup returns `{status, message}` or `{status, apiKey, tier, message}`
|
|
- Errors sometimes `{error}`, sometimes `{error, detail}`
|
|
- Recovery returns key directly in response (security concern — should match signup UX)
|
|
- **Fix:** Standardize error/success envelope format
|
|
|
|
### 9. Database Connection Resilience
|
|
- **Current:** No connection retry logic. If PostgreSQL is temporarily down on startup, the app crashes.
|
|
- **Current:** No health check interval on pool connections
|
|
- **Fix:** Add retry with backoff on startup, add `connectionTimeoutMillis`
|
|
|
|
### 10. Test Coverage
|
|
- **Current:** Only 1 test file (136 lines) with basic tests. No integration tests, no template tests.
|
|
- **Fix:** Add proper test suite covering all routes, error cases, rate limiting
|
|
|
|
### 11. Version Mismatch
|
|
- **Current:** package.json says `0.1.0`, health endpoint returns `0.2.1` (hardcoded string)
|
|
- **Fix:** Read version from package.json or env
|
|
|
|
### 12. Docker Compose Version
|
|
- **Current:** Uses deprecated `version: "3.8"` key
|
|
- **Fix:** Remove it (modern compose doesn't need it)
|
|
|
|
### 13. Accessibility
|
|
- **Current:** Landing page has no skip-to-content link, no focus indicators, modal not keyboard-trapable, no aria-labels on interactive elements
|
|
- **Fix:** Add ARIA attributes, focus management, keyboard navigation
|
|
|
|
### 14. Performance: Static Asset Optimization
|
|
- **Current:** Landing page loads Inter font from Google Fonts (render-blocking). No asset fingerprinting, no compression middleware.
|
|
- **Fix:** Add compression middleware, preload fonts, add cache headers
|
|
|
|
## Priority 3: NICE TO HAVE
|
|
|
|
### 15. OpenAPI Spec Accuracy
|
|
- Verify /openapi.json matches actual API behavior (email-change route not documented)
|
|
|
|
### 16. Unused Code
|
|
- `verifications` table + token-based verification seems legacy (superseded by code-based verification). The `GET /verify` route and `verifyToken/verifyTokenSync` functions may be dead code.
|
|
|
|
### 17. Usage Tracking Race Condition
|
|
- In-memory usage map with async DB saves — concurrent requests could cause count drift
|
|
|
|
### 18. Template XSS via Currency
|
|
- Template `esc()` function escapes output, but `cur` (currency) is injected without escaping in the template HTML. `d.currency` could contain XSS payload.
|
|
|
|
### 19. Double `!important` Overflow CSS
|
|
- Landing page has aggressive `!important` overrides for mobile that could cause issues with new components
|
|
|
|
---
|
|
|
|
## Execution Plan (CEO Decisions)
|
|
|
|
**Batch 1 — Backend Hardening (spawn backend dev):**
|
|
- Structured logging with pino + request IDs (#1)
|
|
- PDF generation timeout (#2)
|
|
- Memory leak fixes — verification cache cleanup, bounded rateLimitStore (#5)
|
|
- Version from env/package.json (#11)
|
|
- Compression middleware (#14)
|
|
- Template currency XSS fix (#18)
|
|
|
|
**Batch 2 — Frontend/SEO (spawn UI/UX dev):**
|
|
- SEO: OG tags, Twitter cards, sitemap.xml, robots.txt (#6)
|
|
- Accessibility: ARIA labels, focus management, keyboard nav (#13)
|
|
- Static asset caching headers (#7, #14)
|
|
|
|
**Batch 3 — QA verification of all changes**
|