c327ba3370
Changes: - Updated docker-compose.yml MLflow service port binding from 5000:5000 to 127.0.0.1:5000:5000 to restrict access to localhost only for security - Marked task 1.7 as complete in tasks.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
92 lines
5.2 KiB
Markdown
92 lines
5.2 KiB
Markdown
## Why
|
|
|
|
A comprehensive code review identified **93 issues** (13 Critical, 35 Major, 45 Minor) across the entire codebase. The application has zero authentication, hardcoded credentials in committed files, path traversal vectors, unsafe deserialization, frontend race conditions/memory leaks, and zero test files. These must be fixed before any production deployment.
|
|
|
|
## What Changes
|
|
|
|
### Security (Critical)
|
|
- Add authentication middleware to all API endpoints (Next.js + FastAPI)
|
|
- Fix `.env` gitignore and remove credentials from git history
|
|
- Replace all hardcoded database credentials with env var interpolation
|
|
- Bind PostgreSQL port to localhost only
|
|
- Validate `run_id` parameters to prevent SSRF/path traversal
|
|
- Add file upload size limits
|
|
- Fix CORS wildcard on ML service
|
|
- Add model file integrity checks (SHA256) for joblib/pickle files
|
|
|
|
### Frontend Stability (Critical + Major)
|
|
- Fix stale closures in `fetchPredictions`, CandleChart click handler, SpanAnnotationManager keyboard handler
|
|
- Add AbortController to cancel stale prediction requests
|
|
- Fix SpanAnnotationManager preview primitive memory leak
|
|
- Add `response.ok` checks on all fetch calls
|
|
- Add React Error Boundary
|
|
- Fix hardcoded 1-minute candle interval assumption
|
|
- Use `chart.applyOptions()` for theme changes instead of re-creating chart
|
|
- Fix `fitContent()` called on every span change
|
|
|
|
### API Hardening (Major)
|
|
- Add Zod validation on all proxy routes
|
|
- Wrap chart cascade delete in transaction + add span annotation deletion
|
|
- Stop leaking error details to clients (frontend + backend)
|
|
- Require `chartId` for bulk annotation deletes
|
|
- Add input size limits on batch prediction
|
|
|
|
### Code Quality (Major + Minor)
|
|
- Centralize duplicate interface definitions into `src/types/`
|
|
- Replace pervasive `any` types with proper interfaces
|
|
- Move dev dependencies to `devDependencies`
|
|
- Remove dead code (migrate.ts, get_db_session, dead filter)
|
|
- Replace deprecated Python APIs (on_event, declarative_base, utcnow)
|
|
|
|
### Infrastructure (Major + Minor)
|
|
- Add security response headers (CSP, X-Frame-Options)
|
|
- ML Dockerfile: run as non-root user
|
|
- Create `.dockerignore`
|
|
- Add rate limiting middleware
|
|
- Pin Docker base images to digests
|
|
- Implement real health checks on ML service
|
|
- Use `next/font` instead of CSS `@import`
|
|
|
|
### UX Polish (Minor)
|
|
- Add confirmation dialog for delete-all annotations
|
|
- Fix accessibility (ARIA labels, focus trapping, keyboard nav)
|
|
- Add CSV injection protection to exports
|
|
- Fix dark theme on settings pages
|
|
- Add debounce to confidence slider
|
|
- Add click-outside handler to chart dropdown
|
|
|
|
## Capabilities
|
|
|
|
### New Capabilities
|
|
- `api-authentication`: API key or session-based auth middleware for Next.js routes and FastAPI endpoints
|
|
- `input-validation`: Zod schemas for all proxy routes, file upload limits, run_id format validation, batch size limits
|
|
- `security-headers`: CSP, X-Frame-Options, and other security response headers via Next.js config
|
|
- `error-boundary`: React Error Boundary wrapping critical component subtrees
|
|
- `shared-types`: Centralized TypeScript interfaces in `src/types/` replacing duplicate definitions across 6+ files
|
|
|
|
### Modified Capabilities
|
|
- `docker-deployment`: Bind ports to localhost, add `.dockerignore`, run ML as non-root, pin images, fix healthcheck, TA-Lib HTTPS download
|
|
- `backend-api`: Transaction-wrapped cascade deletes, generic error responses, scoped bulk delete, rate limiting
|
|
- `ml-inference`: CORS fix, model integrity checks, path traversal prevention, thread-safe model reads, input size limits
|
|
- `ml-training`: run_id validation, resource limits, path traversal prevention
|
|
- `chart-canvas`: Fix stale closures, theme change via applyOptions, fix 1-min interval assumption, extract magic numbers, use refs for click handler
|
|
- `span-annotation`: Fix keyboard handler stale closure, preview primitive memory leak, fitContent removal, primitive recreation optimization
|
|
- `prediction-ui`: AbortController for requests, bounded prediction cache, response.ok checks
|
|
- `postgres-data-layer`: Remove `.env` from git, env var interpolation for credentials, SSL/TLS support
|
|
- `ui-shell`: Error boundary integration, confirmation dialogs, accessibility improvements, dark theme fixes, next/font migration
|
|
- `label-management`: (no spec-level changes, only implementation cleanup)
|
|
- `feature-engineering`: (no spec-level changes, deprecated API cleanup only)
|
|
|
|
## Impact
|
|
|
|
- **All 27 API route files** in `src/app/api/` — auth middleware, error handling, validation
|
|
- **`src/app/page.tsx`** — AbortController, response checks, cache bounds, confirmation dialog
|
|
- **`src/components/CandleChart.tsx`** — stale closures, theme, interval fix, refs
|
|
- **`src/components/SpanAnnotationManager.tsx`** — memory leak, keyboard handler, fitContent, primitives
|
|
- **`services/ml/app/main.py`** — CORS, auth, error messages, model integrity, path validation, deprecated APIs
|
|
- **`services/ml/Dockerfile`** — non-root user, HTTPS download
|
|
- **`docker-compose.yml`** — port bindings, credential interpolation
|
|
- **`next.config.js`** — security headers
|
|
- **`package.json`** — devDependencies reorganization
|
|
- **New files**: `src/middleware.ts`, `src/types/*.ts`, `src/components/ErrorBoundary.tsx`, `.dockerignore`
|
|
- **No breaking API changes** — all fixes are additive or internal
|