diff --git a/backend/Dockerfile b/backend/Dockerfile index c3aab1a..c04e054 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -10,5 +10,8 @@ CMD ["npm", "run", "dev"] FROM base AS prod COPY . . +# Generate Prisma client in the production image RUN npx prisma generate -CMD ["npm", "start"] +# Run as non-root user +USER node +CMD ["node", "src/index.js"] diff --git a/backend/docker-entrypoint.sh b/backend/docker-entrypoint.sh new file mode 100644 index 0000000..8291c9c --- /dev/null +++ b/backend/docker-entrypoint.sh @@ -0,0 +1,8 @@ +#!/bin/sh +set -e + +echo "Running Prisma migrations..." +npx prisma migrate deploy + +echo "Starting server..." +exec node src/index.js diff --git a/backend/package-lock.json b/backend/package-lock.json index c235cac..60f8c54 100644 --- a/backend/package-lock.json +++ b/backend/package-lock.json @@ -11,6 +11,7 @@ "@prisma/client": "^5.14.0", "cors": "^2.8.5", "express": "^4.19.2", + "express-rate-limit": "^8.3.2", "express-validator": "^7.1.0", "helmet": "^7.1.0", "morgan": "^1.10.0", @@ -2064,6 +2065,23 @@ "url": "https://opencollective.com/express" } }, + "node_modules/express-rate-limit": { + "version": "8.3.2", + "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-8.3.2.tgz", + "integrity": "sha512-77VmFeJkO0/rvimEDuUC5H30oqUC4EyOhyGccfqoLebB0oiEYfM7nwPrsDsBL1gsTpwfzX8SFy2MT3TDyRq+bg==", + "dependencies": { + "ip-address": "10.1.0" + }, + "engines": { + "node": ">= 16" + }, + "funding": { + "url": "https://github.com/sponsors/express-rate-limit" + }, + "peerDependencies": { + "express": ">= 4.11" + } + }, "node_modules/express-validator": { "version": "7.3.2", "resolved": "https://registry.npmjs.org/express-validator/-/express-validator-7.3.2.tgz", @@ -2489,6 +2507,14 @@ "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz", "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==" }, + "node_modules/ip-address": { + "version": "10.1.0", + "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.1.0.tgz", + "integrity": "sha512-XXADHxXmvT9+CRxhXg56LJovE+bmWnEWB78LB83VZTprKTmaC5QfruXocxzTZ2Kl0DNwKuBdlIhjL8LeY8Sf8Q==", + "engines": { + "node": ">= 12" + } + }, "node_modules/ipaddr.js": { "version": "1.9.1", "resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-1.9.1.tgz", @@ -6396,6 +6422,14 @@ "vary": "~1.1.2" } }, + "express-rate-limit": { + "version": "8.3.2", + "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-8.3.2.tgz", + "integrity": "sha512-77VmFeJkO0/rvimEDuUC5H30oqUC4EyOhyGccfqoLebB0oiEYfM7nwPrsDsBL1gsTpwfzX8SFy2MT3TDyRq+bg==", + "requires": { + "ip-address": "10.1.0" + } + }, "express-validator": { "version": "7.3.2", "resolved": "https://registry.npmjs.org/express-validator/-/express-validator-7.3.2.tgz", @@ -6697,6 +6731,11 @@ "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz", "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==" }, + "ip-address": { + "version": "10.1.0", + "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.1.0.tgz", + "integrity": "sha512-XXADHxXmvT9+CRxhXg56LJovE+bmWnEWB78LB83VZTprKTmaC5QfruXocxzTZ2Kl0DNwKuBdlIhjL8LeY8Sf8Q==" + }, "ipaddr.js": { "version": "1.9.1", "resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-1.9.1.tgz", diff --git a/backend/package.json b/backend/package.json index b6ef087..5276223 100644 --- a/backend/package.json +++ b/backend/package.json @@ -16,6 +16,7 @@ "@prisma/client": "^5.14.0", "cors": "^2.8.5", "express": "^4.19.2", + "express-rate-limit": "^8.3.2", "express-validator": "^7.1.0", "helmet": "^7.1.0", "morgan": "^1.10.0", @@ -29,7 +30,9 @@ }, "jest": { "testEnvironment": "node", - "testMatch": ["**/tests/**/*.test.js"], + "testMatch": [ + "**/tests/**/*.test.js" + ], "globalSetup": "./tests/setup.js", "globalTeardown": "./tests/teardown.js" } diff --git a/backend/src/app.js b/backend/src/app.js index 61dc932..4a410db 100644 --- a/backend/src/app.js +++ b/backend/src/app.js @@ -2,6 +2,7 @@ const express = require('express'); const cors = require('cors'); const helmet = require('helmet'); const morgan = require('morgan'); +const rateLimit = require('express-rate-limit'); const { globalErrorHandler } = require('./middleware/errorHandler'); const experimentsRouter = require('./routes/experiments'); @@ -11,19 +12,40 @@ const auditLogsRouter = require('./routes/auditLogs'); const app = express(); -app.use(helmet()); +// Security headers +app.use(helmet({ + crossOriginResourcePolicy: { policy: 'cross-origin' }, +})); + +// CORS — restrict to the configured frontend origin in production +const allowedOrigin = process.env.CORS_ORIGIN || '*'; app.use(cors({ - origin: process.env.CORS_ORIGIN || '*', + origin: allowedOrigin, methods: ['GET', 'POST', 'PUT', 'DELETE', 'OPTIONS'], allowedHeaders: ['Content-Type', 'Authorization'], })); + +// Rate limiting — 100 requests per minute per IP on all API routes +const apiLimiter = rateLimit({ + windowMs: 60 * 1000, + max: 100, + standardHeaders: true, + legacyHeaders: false, + message: { error: 'Too many requests, please try again later.' }, + skip: () => process.env.NODE_ENV === 'test', +}); + app.use(express.json({ limit: '1mb' })); app.use(morgan(process.env.NODE_ENV === 'test' ? 'silent' : process.env.NODE_ENV === 'production' ? 'combined' : 'dev')); +// Health check (no rate limit) app.get('/health', (req, res) => { res.json({ status: 'ok', timestamp: new Date().toISOString() }); }); +// Apply rate limiter to all API routes +app.use('/api', apiLimiter); + app.use('/api/experiments', experimentsRouter); app.use('/api/animals', animalsRouter); app.use('/api/daily-statuses', dailyStatusesRouter); diff --git a/backend/src/index.js b/backend/src/index.js index 2718b5c..663228b 100644 --- a/backend/src/index.js +++ b/backend/src/index.js @@ -7,12 +7,7 @@ async function startServer() { try { await prisma.$connect(); console.log('Database connected successfully'); - - if (process.env.NODE_ENV === 'production') { - const { execSync } = require('child_process'); - execSync('npx prisma migrate deploy', { stdio: 'inherit' }); - } - + // Note: migrations are run by docker-entrypoint.sh before this process starts app.listen(PORT, '0.0.0.0', () => { console.log(`Server running on port ${PORT}`); }); diff --git a/backend/src/routes/auditLogs.js b/backend/src/routes/auditLogs.js index 572850d..a484c43 100644 --- a/backend/src/routes/auditLogs.js +++ b/backend/src/routes/auditLogs.js @@ -7,7 +7,7 @@ const { validateRequest } = require('../middleware/errorHandler'); router.get( '/', [ - query('tableName').optional().isString(), + query('tableName').optional().isIn(['experiments', 'animals', 'daily_statuses', 'audit_logs']).withMessage('tableName must be one of: experiments, animals, daily_statuses, audit_logs'), query('recordId').optional().matches(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i).withMessage('recordId must be a valid UUID'), query('limit').optional().isInt({ min: 1, max: 200 }).withMessage('limit must be 1–200').toInt(), query('offset').optional().isInt({ min: 0 }).withMessage('offset must be ≥0').toInt(), diff --git a/backend/tests/auditLogs.test.js b/backend/tests/auditLogs.test.js index 750cfe3..94eca2e 100644 --- a/backend/tests/auditLogs.test.js +++ b/backend/tests/auditLogs.test.js @@ -38,6 +38,11 @@ describe('GET /api/audit-logs', () => { ); }); + it('returns 422 for invalid tableName (not in allowlist)', async () => { + const res = await request(app).get('/api/audit-logs?tableName=users'); + expect(res.status).toBe(422); + }); + it('returns 422 for invalid recordId', async () => { const res = await request(app).get('/api/audit-logs?recordId=not-a-uuid'); expect(res.status).toBe(422); diff --git a/docker-compose.yml b/docker-compose.yml index 30bfcaa..e78733a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -11,8 +11,9 @@ services: POSTGRES_DB: ${POSTGRES_DB:-experiments_db} volumes: - postgres_data:/var/lib/postgresql/data - ports: - - "5432:5432" + # Port NOT exposed to host — only reachable by backend via internal Docker network + expose: + - "5432" healthcheck: test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER:-expuser} -d ${POSTGRES_DB:-experiments_db}"] interval: 10s @@ -32,6 +33,10 @@ services: DATABASE_URL: postgresql://${POSTGRES_USER:-expuser}:${POSTGRES_PASSWORD:-exppassword}@postgres:5432/${POSTGRES_DB:-experiments_db} NODE_ENV: production PORT: 3001 + CORS_ORIGIN: "http://localhost:52867" + # Run as non-root + user: "node" + entrypoint: ["/bin/sh", "/app/docker-entrypoint.sh"] ports: - "3001:3001" healthcheck: diff --git a/frontend/src/pages/Dashboard.jsx b/frontend/src/pages/Dashboard.jsx index 9ee60e8..80ce9e0 100644 --- a/frontend/src/pages/Dashboard.jsx +++ b/frontend/src/pages/Dashboard.jsx @@ -83,7 +83,7 @@ export default function Dashboard() { {error && setError(null)} />} {loading && ( -
Loading experiments…
+
Loading experiments…
)} {!loading && experiments.length === 0 && ( diff --git a/memory.md b/memory.md index bf39f90..726c35c 100644 --- a/memory.md +++ b/memory.md @@ -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 +