fix(security): rate limiting, CORS restriction, tableName allowlist, non-root Docker, migration entrypoint, aria-live
This commit is contained in:
@@ -123,4 +123,55 @@ All PUT and DELETE routes pass through `auditMiddleware` which:
|
||||
- All tests run inside Docker via `docker-compose.test.yml`
|
||||
|
||||
## Red Team Audit
|
||||
_To be filled after Phase 3 testing is complete._
|
||||
|
||||
### Findings
|
||||
|
||||
#### 🔴 Critical
|
||||
|
||||
1. **CORS wildcard (`origin: '*'`)** — The backend accepts requests from any origin. An attacker on a different domain could make authenticated browser requests to the API if any auth is added later, or exfiltrate data via CSRF.
|
||||
- **Fix**: Restrict `CORS_ORIGIN` to the frontend's origin in production.
|
||||
|
||||
2. **Prisma `migrate deploy` via `execSync` on server start** — Running migrations inline at startup means a crashing migration kills the entire service process. Also, injecting `DATABASE_URL` with special characters could theoretically escape the shell via `child_process.execSync`.
|
||||
- **Fix**: Run migrations as a separate Docker entrypoint step, not inside the Node process.
|
||||
|
||||
3. **Unparameterized `table_name` in audit logs route** — `table_name` is accepted as a plain string from query params and passed directly to Prisma. Prisma ORM prevents SQL injection here, but the field is never validated against a known allowlist — an attacker can write arbitrary strings to query params and pollute logs.
|
||||
- **Fix**: Validate `tableName` against an allowlist: `['experiments', 'animals', 'daily_statuses', 'audit_logs']`.
|
||||
|
||||
#### 🟡 Medium
|
||||
|
||||
4. **No rate limiting** — All API endpoints are open to brute-force or denial-of-service via request flooding. Express has no rate limiter middleware.
|
||||
- **Fix**: Add `express-rate-limit` on all `/api/*` routes.
|
||||
|
||||
5. **Helmet defaults only** — `helmet()` is enabled but with defaults; notably `Content-Security-Policy` is not tuned for the SPA. The nginx proxy serves the frontend without explicit CSP headers either.
|
||||
- **Fix**: Add a CSP header in nginx and strengthen helmet config.
|
||||
|
||||
6. **Docker: no `read_only` filesystem or user restriction** — The backend and frontend containers run as root by default.
|
||||
- **Fix**: Add `user: node` and `read_only: true` (with tmpfs for writable paths) in `docker-compose.yml`.
|
||||
|
||||
7. **Postgres port exposed on host** — `docker-compose.yml` exposes port 5432 externally. In production, only the backend container needs DB access.
|
||||
- **Fix**: Remove host-side port mapping for postgres; use internal Docker networking only.
|
||||
|
||||
8. **`NODE_ENV=production` skip for Prisma generate** — `npx prisma generate` is not called in the production Dockerfile before `npm ci --only=production`, so the generated client may be missing.
|
||||
- **Fix**: Run `prisma generate` in the builder stage.
|
||||
|
||||
#### 🟢 UX / Low
|
||||
|
||||
9. **Audit log section on Dashboard shows ALL experiments audit logs** (no `recordId` filter) — this is intentional but confusing; a large table will render hundreds of rows without pagination controls.
|
||||
- **Fix**: Add pagination controls to `AuditLogSection`.
|
||||
|
||||
10. **No loading skeleton on list pages** — a "Loading…" text string isn't accessible; screen readers get no meaningful feedback.
|
||||
- **Fix**: Add `aria-live="polite"` region for loading state.
|
||||
|
||||
11. **Form submission does not disable all interactive elements** — While the submit button shows a spinner, the Cancel button remains active and can unmount the form mid-submission, causing a React state update on an unmounted component.
|
||||
- **Fix**: Disable Cancel button while `loading` is true (already done for inputs, extend to Cancel).
|
||||
|
||||
### Patches Applied (feature/red-team-fixes)
|
||||
- Restricted CORS to env-configurable origin
|
||||
- Moved migration out of `startServer()` into Docker entrypoint script
|
||||
- Added `tableName` allowlist validation on audit logs route
|
||||
- Added `express-rate-limit` (100 req/min per IP on all API routes)
|
||||
- Removed exposed postgres port from docker-compose
|
||||
- Added `user: node` to backend docker-compose service
|
||||
- Fixed Prisma generate in production Dockerfile
|
||||
- Disabled Cancel button during form submission
|
||||
|
||||
|
||||
Reference in New Issue
Block a user