merge: red team security fixes into main
This commit is contained in:
+4
-1
@@ -10,5 +10,8 @@ CMD ["npm", "run", "dev"]
|
|||||||
|
|
||||||
FROM base AS prod
|
FROM base AS prod
|
||||||
COPY . .
|
COPY . .
|
||||||
|
# Generate Prisma client in the production image
|
||||||
RUN npx prisma generate
|
RUN npx prisma generate
|
||||||
CMD ["npm", "start"]
|
# Run as non-root user
|
||||||
|
USER node
|
||||||
|
CMD ["node", "src/index.js"]
|
||||||
|
|||||||
@@ -0,0 +1,8 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
set -e
|
||||||
|
|
||||||
|
echo "Running Prisma migrations..."
|
||||||
|
npx prisma migrate deploy
|
||||||
|
|
||||||
|
echo "Starting server..."
|
||||||
|
exec node src/index.js
|
||||||
Generated
+39
@@ -11,6 +11,7 @@
|
|||||||
"@prisma/client": "^5.14.0",
|
"@prisma/client": "^5.14.0",
|
||||||
"cors": "^2.8.5",
|
"cors": "^2.8.5",
|
||||||
"express": "^4.19.2",
|
"express": "^4.19.2",
|
||||||
|
"express-rate-limit": "^8.3.2",
|
||||||
"express-validator": "^7.1.0",
|
"express-validator": "^7.1.0",
|
||||||
"helmet": "^7.1.0",
|
"helmet": "^7.1.0",
|
||||||
"morgan": "^1.10.0",
|
"morgan": "^1.10.0",
|
||||||
@@ -2064,6 +2065,23 @@
|
|||||||
"url": "https://opencollective.com/express"
|
"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": {
|
"node_modules/express-validator": {
|
||||||
"version": "7.3.2",
|
"version": "7.3.2",
|
||||||
"resolved": "https://registry.npmjs.org/express-validator/-/express-validator-7.3.2.tgz",
|
"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",
|
"resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz",
|
||||||
"integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ=="
|
"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": {
|
"node_modules/ipaddr.js": {
|
||||||
"version": "1.9.1",
|
"version": "1.9.1",
|
||||||
"resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-1.9.1.tgz",
|
"resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-1.9.1.tgz",
|
||||||
@@ -6396,6 +6422,14 @@
|
|||||||
"vary": "~1.1.2"
|
"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": {
|
"express-validator": {
|
||||||
"version": "7.3.2",
|
"version": "7.3.2",
|
||||||
"resolved": "https://registry.npmjs.org/express-validator/-/express-validator-7.3.2.tgz",
|
"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",
|
"resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz",
|
||||||
"integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ=="
|
"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": {
|
"ipaddr.js": {
|
||||||
"version": "1.9.1",
|
"version": "1.9.1",
|
||||||
"resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-1.9.1.tgz",
|
"resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-1.9.1.tgz",
|
||||||
|
|||||||
@@ -16,6 +16,7 @@
|
|||||||
"@prisma/client": "^5.14.0",
|
"@prisma/client": "^5.14.0",
|
||||||
"cors": "^2.8.5",
|
"cors": "^2.8.5",
|
||||||
"express": "^4.19.2",
|
"express": "^4.19.2",
|
||||||
|
"express-rate-limit": "^8.3.2",
|
||||||
"express-validator": "^7.1.0",
|
"express-validator": "^7.1.0",
|
||||||
"helmet": "^7.1.0",
|
"helmet": "^7.1.0",
|
||||||
"morgan": "^1.10.0",
|
"morgan": "^1.10.0",
|
||||||
@@ -29,7 +30,9 @@
|
|||||||
},
|
},
|
||||||
"jest": {
|
"jest": {
|
||||||
"testEnvironment": "node",
|
"testEnvironment": "node",
|
||||||
"testMatch": ["**/tests/**/*.test.js"],
|
"testMatch": [
|
||||||
|
"**/tests/**/*.test.js"
|
||||||
|
],
|
||||||
"globalSetup": "./tests/setup.js",
|
"globalSetup": "./tests/setup.js",
|
||||||
"globalTeardown": "./tests/teardown.js"
|
"globalTeardown": "./tests/teardown.js"
|
||||||
}
|
}
|
||||||
|
|||||||
+24
-2
@@ -2,6 +2,7 @@ const express = require('express');
|
|||||||
const cors = require('cors');
|
const cors = require('cors');
|
||||||
const helmet = require('helmet');
|
const helmet = require('helmet');
|
||||||
const morgan = require('morgan');
|
const morgan = require('morgan');
|
||||||
|
const rateLimit = require('express-rate-limit');
|
||||||
const { globalErrorHandler } = require('./middleware/errorHandler');
|
const { globalErrorHandler } = require('./middleware/errorHandler');
|
||||||
|
|
||||||
const experimentsRouter = require('./routes/experiments');
|
const experimentsRouter = require('./routes/experiments');
|
||||||
@@ -11,19 +12,40 @@ const auditLogsRouter = require('./routes/auditLogs');
|
|||||||
|
|
||||||
const app = express();
|
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({
|
app.use(cors({
|
||||||
origin: process.env.CORS_ORIGIN || '*',
|
origin: allowedOrigin,
|
||||||
methods: ['GET', 'POST', 'PUT', 'DELETE', 'OPTIONS'],
|
methods: ['GET', 'POST', 'PUT', 'DELETE', 'OPTIONS'],
|
||||||
allowedHeaders: ['Content-Type', 'Authorization'],
|
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(express.json({ limit: '1mb' }));
|
||||||
app.use(morgan(process.env.NODE_ENV === 'test' ? 'silent' : process.env.NODE_ENV === 'production' ? 'combined' : 'dev'));
|
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) => {
|
app.get('/health', (req, res) => {
|
||||||
res.json({ status: 'ok', timestamp: new Date().toISOString() });
|
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/experiments', experimentsRouter);
|
||||||
app.use('/api/animals', animalsRouter);
|
app.use('/api/animals', animalsRouter);
|
||||||
app.use('/api/daily-statuses', dailyStatusesRouter);
|
app.use('/api/daily-statuses', dailyStatusesRouter);
|
||||||
|
|||||||
@@ -7,12 +7,7 @@ async function startServer() {
|
|||||||
try {
|
try {
|
||||||
await prisma.$connect();
|
await prisma.$connect();
|
||||||
console.log('Database connected successfully');
|
console.log('Database connected successfully');
|
||||||
|
// Note: migrations are run by docker-entrypoint.sh before this process starts
|
||||||
if (process.env.NODE_ENV === 'production') {
|
|
||||||
const { execSync } = require('child_process');
|
|
||||||
execSync('npx prisma migrate deploy', { stdio: 'inherit' });
|
|
||||||
}
|
|
||||||
|
|
||||||
app.listen(PORT, '0.0.0.0', () => {
|
app.listen(PORT, '0.0.0.0', () => {
|
||||||
console.log(`Server running on port ${PORT}`);
|
console.log(`Server running on port ${PORT}`);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -7,7 +7,7 @@ const { validateRequest } = require('../middleware/errorHandler');
|
|||||||
router.get(
|
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('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('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(),
|
query('offset').optional().isInt({ min: 0 }).withMessage('offset must be ≥0').toInt(),
|
||||||
|
|||||||
@@ -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 () => {
|
it('returns 422 for invalid recordId', async () => {
|
||||||
const res = await request(app).get('/api/audit-logs?recordId=not-a-uuid');
|
const res = await request(app).get('/api/audit-logs?recordId=not-a-uuid');
|
||||||
expect(res.status).toBe(422);
|
expect(res.status).toBe(422);
|
||||||
|
|||||||
+7
-2
@@ -11,8 +11,9 @@ services:
|
|||||||
POSTGRES_DB: ${POSTGRES_DB:-experiments_db}
|
POSTGRES_DB: ${POSTGRES_DB:-experiments_db}
|
||||||
volumes:
|
volumes:
|
||||||
- postgres_data:/var/lib/postgresql/data
|
- postgres_data:/var/lib/postgresql/data
|
||||||
ports:
|
# Port NOT exposed to host — only reachable by backend via internal Docker network
|
||||||
- "5432:5432"
|
expose:
|
||||||
|
- "5432"
|
||||||
healthcheck:
|
healthcheck:
|
||||||
test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER:-expuser} -d ${POSTGRES_DB:-experiments_db}"]
|
test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER:-expuser} -d ${POSTGRES_DB:-experiments_db}"]
|
||||||
interval: 10s
|
interval: 10s
|
||||||
@@ -32,6 +33,10 @@ services:
|
|||||||
DATABASE_URL: postgresql://${POSTGRES_USER:-expuser}:${POSTGRES_PASSWORD:-exppassword}@postgres:5432/${POSTGRES_DB:-experiments_db}
|
DATABASE_URL: postgresql://${POSTGRES_USER:-expuser}:${POSTGRES_PASSWORD:-exppassword}@postgres:5432/${POSTGRES_DB:-experiments_db}
|
||||||
NODE_ENV: production
|
NODE_ENV: production
|
||||||
PORT: 3001
|
PORT: 3001
|
||||||
|
CORS_ORIGIN: "http://localhost:52867"
|
||||||
|
# Run as non-root
|
||||||
|
user: "node"
|
||||||
|
entrypoint: ["/bin/sh", "/app/docker-entrypoint.sh"]
|
||||||
ports:
|
ports:
|
||||||
- "3001:3001"
|
- "3001:3001"
|
||||||
healthcheck:
|
healthcheck:
|
||||||
|
|||||||
@@ -83,7 +83,7 @@ export default function Dashboard() {
|
|||||||
{error && <Alert type="error" message={error} onDismiss={() => setError(null)} />}
|
{error && <Alert type="error" message={error} onDismiss={() => setError(null)} />}
|
||||||
|
|
||||||
{loading && (
|
{loading && (
|
||||||
<div className="text-center py-16 text-gray-400">Loading experiments…</div>
|
<div className="text-center py-16 text-gray-400" aria-live="polite" aria-busy="true">Loading experiments…</div>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
{!loading && experiments.length === 0 && (
|
{!loading && experiments.length === 0 && (
|
||||||
|
|||||||
@@ -123,4 +123,55 @@ All PUT and DELETE routes pass through `auditMiddleware` which:
|
|||||||
- All tests run inside Docker via `docker-compose.test.yml`
|
- All tests run inside Docker via `docker-compose.test.yml`
|
||||||
|
|
||||||
## Red Team Audit
|
## 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