Fix BMad adversarial security review findings
Some checks failed
CI — P2 Drift (Go + Node) / agent (push) Successful in 47s
CI — P2 Drift (Go + Node) / saas (push) Successful in 36s
CI — P3 Alert / test (push) Successful in 36s
CI — P4 Portal / build-push (push) Failing after 49s
CI — P5 Cost / build-push (push) Failing after 4s
CI — P6 Run / build-push (push) Failing after 4s
CI — P4 Portal / test (push) Successful in 35s
CI — P5 Cost / test (push) Successful in 40s
CI — P6 Run / saas (push) Successful in 36s
CI — P2 Drift (Go + Node) / build-push (push) Failing after 17s
CI — P3 Alert / build-push (push) Failing after 15s
Some checks failed
CI — P2 Drift (Go + Node) / agent (push) Successful in 47s
CI — P2 Drift (Go + Node) / saas (push) Successful in 36s
CI — P3 Alert / test (push) Successful in 36s
CI — P4 Portal / build-push (push) Failing after 49s
CI — P5 Cost / build-push (push) Failing after 4s
CI — P6 Run / build-push (push) Failing after 4s
CI — P4 Portal / test (push) Successful in 35s
CI — P5 Cost / test (push) Successful in 40s
CI — P6 Run / saas (push) Successful in 36s
CI — P2 Drift (Go + Node) / build-push (push) Failing after 17s
CI — P3 Alert / build-push (push) Failing after 15s
Resolves 11 of the 13 findings: - [CRITICAL] SQLi in RLS: replaced SET LOCAL with parameterized set_config() - [CRITICAL] Rate Limiting: installed and registered @fastify/rate-limit in all 5 apps - [CRITICAL] Invite Hijacking: added email verification check to invite lookup - [HIGH] Webhook HMAC: added Fastify rawBody parser to fix JSON.stringify mangling - [HIGH] TOCTOU Race: added FOR UPDATE to invite lookup - [HIGH] Incident Race: replaced SELECT/INSERT with INSERT ... ON CONFLICT - [MEDIUM] Grafana Timing Attack: replaced === with crypto.timingSafeEqual - [MEDIUM] Insecure Defaults: added NODE_ENV production guard for JWT_SECRET - [LOW] DB Privileges: tightened docker-init-db.sh grants (removed ALL PRIVILEGES) - [LOW] Plaintext Invites: tokens are now hashed (SHA-256) before DB storage/lookup - [LOW] Scrypt: increased N parameter to 65536 for stronger password hashing Note: - Finding #4 (Fragmented Identity) requires a unified auth database architecture. - Finding #8 (getPoolForAuth) is an accepted tradeoff to keep auth middleware clean.
This commit is contained in:
@@ -10,6 +10,7 @@
|
||||
"dependencies": {
|
||||
"@fastify/cors": "^9.0.0",
|
||||
"@fastify/helmet": "^11.1.0",
|
||||
"@fastify/rate-limit": "^9.1.0",
|
||||
"@fastify/websocket": "^10.0.0",
|
||||
"@slack/bolt": "^3.19.0",
|
||||
"@slack/web-api": "^7.1.0",
|
||||
@@ -712,6 +713,17 @@
|
||||
"fast-deep-equal": "^3.1.3"
|
||||
}
|
||||
},
|
||||
"node_modules/@fastify/rate-limit": {
|
||||
"version": "9.1.0",
|
||||
"resolved": "https://registry.npmjs.org/@fastify/rate-limit/-/rate-limit-9.1.0.tgz",
|
||||
"integrity": "sha512-h5dZWCkuZXN0PxwqaFQLxeln8/LNwQwH9popywmDCFdKfgpi4b/HoMH1lluy6P+30CG9yzzpSpwTCIPNB9T1JA==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@lukeed/ms": "^2.0.1",
|
||||
"fastify-plugin": "^4.0.0",
|
||||
"toad-cache": "^3.3.1"
|
||||
}
|
||||
},
|
||||
"node_modules/@fastify/websocket": {
|
||||
"version": "10.0.1",
|
||||
"resolved": "https://registry.npmjs.org/@fastify/websocket/-/websocket-10.0.1.tgz",
|
||||
@@ -801,6 +813,15 @@
|
||||
"dev": true,
|
||||
"license": "MIT"
|
||||
},
|
||||
"node_modules/@lukeed/ms": {
|
||||
"version": "2.0.2",
|
||||
"resolved": "https://registry.npmjs.org/@lukeed/ms/-/ms-2.0.2.tgz",
|
||||
"integrity": "sha512-9I2Zn6+NJLfaGoz9jN3lpwDgAYvfGeNYdbAIjJOqzs4Tpc+VU3Jqq4IofSUBKajiDS8k9fZIg18/z13mpk1bsA==",
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=8"
|
||||
}
|
||||
},
|
||||
"node_modules/@pinojs/redact": {
|
||||
"version": "0.4.0",
|
||||
"resolved": "https://registry.npmjs.org/@pinojs/redact/-/redact-0.4.0.tgz",
|
||||
|
||||
@@ -11,29 +11,29 @@
|
||||
"lint": "eslint src/ tests/"
|
||||
},
|
||||
"dependencies": {
|
||||
"fastify": "^4.28.0",
|
||||
"@fastify/cors": "^9.0.0",
|
||||
"@fastify/rate-limit": "^9.1.0",
|
||||
"@fastify/helmet": "^11.1.0",
|
||||
"@fastify/rate-limit": "^9.1.0",
|
||||
"@fastify/websocket": "^10.0.0",
|
||||
"pg": "^8.12.0",
|
||||
"@slack/bolt": "^3.19.0",
|
||||
"@slack/web-api": "^7.1.0",
|
||||
"fastify": "^4.28.0",
|
||||
"ioredis": "^5.4.0",
|
||||
"zod": "^3.23.0",
|
||||
"jsonwebtoken": "^9.0.2",
|
||||
"pg": "^8.12.0",
|
||||
"pino": "^9.1.0",
|
||||
"uuid": "^9.0.1",
|
||||
"@slack/web-api": "^7.1.0",
|
||||
"@slack/bolt": "^3.19.0"
|
||||
"zod": "^3.23.0"
|
||||
},
|
||||
"devDependencies": {
|
||||
"typescript": "^5.5.0",
|
||||
"tsx": "^4.15.0",
|
||||
"vitest": "^1.6.0",
|
||||
"@types/jsonwebtoken": "^9.0.6",
|
||||
"@types/node": "^20.14.0",
|
||||
"@types/pg": "^8.11.0",
|
||||
"@types/jsonwebtoken": "^9.0.6",
|
||||
"@types/uuid": "^9.0.8",
|
||||
"@types/ws": "^8.5.10",
|
||||
"eslint": "^9.5.0"
|
||||
"eslint": "^9.5.0",
|
||||
"tsx": "^4.15.0",
|
||||
"typescript": "^5.5.0",
|
||||
"vitest": "^1.6.0"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -96,7 +96,7 @@ export function signToken(payload: AuthPayload, secret: string, expiresIn = '24h
|
||||
async function hashPassword(password: string): Promise<string> {
|
||||
const salt = crypto.randomBytes(16).toString('hex');
|
||||
return new Promise((resolve, reject) => {
|
||||
crypto.scrypt(password, salt, 64, (err, derived) => {
|
||||
crypto.scrypt(password, salt, 64, { N: 65536, r: 8, p: 1 }, (err, derived) => {
|
||||
if (err) reject(err);
|
||||
resolve(`${salt}:${derived.toString('hex')}`);
|
||||
});
|
||||
@@ -106,7 +106,7 @@ async function hashPassword(password: string): Promise<string> {
|
||||
async function verifyPassword(password: string, hash: string): Promise<boolean> {
|
||||
const [salt, key] = hash.split(':');
|
||||
return new Promise((resolve, reject) => {
|
||||
crypto.scrypt(password, salt, 64, (err, derived) => {
|
||||
crypto.scrypt(password, salt, 64, { N: 65536, r: 8, p: 1 }, (err, derived) => {
|
||||
if (err) reject(err);
|
||||
resolve(crypto.timingSafeEqual(Buffer.from(key, 'hex'), derived));
|
||||
});
|
||||
@@ -178,9 +178,10 @@ export function registerAuthRoutes(app: FastifyInstance, jwtSecret: string, pool
|
||||
let role: string;
|
||||
|
||||
if (body.invite_token) {
|
||||
const tokenHash = crypto.createHash('sha256').update(body.invite_token).digest('hex');
|
||||
const invite = await client.query(
|
||||
`SELECT id, tenant_id, role, expires_at, accepted_at FROM tenant_invites WHERE token = $1`,
|
||||
[body.invite_token],
|
||||
`SELECT id, tenant_id, email, role, expires_at, accepted_at FROM tenant_invites WHERE token = $1 FOR UPDATE`,
|
||||
[tokenHash],
|
||||
);
|
||||
if (!invite.rows[0]) {
|
||||
await client.query('ROLLBACK');
|
||||
@@ -195,6 +196,10 @@ export function registerAuthRoutes(app: FastifyInstance, jwtSecret: string, pool
|
||||
await client.query('ROLLBACK');
|
||||
return reply.status(400).send({ error: 'Invite expired' });
|
||||
}
|
||||
if (inv.email && inv.email.toLowerCase() !== body.email.toLowerCase()) {
|
||||
await client.query('ROLLBACK');
|
||||
return reply.status(400).send({ error: 'Email does not match invite' });
|
||||
}
|
||||
|
||||
tenantId = inv.tenant_id;
|
||||
role = inv.role;
|
||||
@@ -278,11 +283,12 @@ export function registerProtectedAuthRoutes(app: FastifyInstance, jwtSecret: str
|
||||
const body = inviteSchema.parse(req.body);
|
||||
|
||||
const token = crypto.randomBytes(32).toString('hex');
|
||||
const inviteTokenHash = crypto.createHash('sha256').update(token).digest('hex');
|
||||
const result = await pool.query(
|
||||
`INSERT INTO tenant_invites (tenant_id, email, role, token, invited_by)
|
||||
VALUES ($1, $2, $3, $4, $5)
|
||||
RETURNING expires_at`,
|
||||
[tenantId, body.email, body.role, token, userId],
|
||||
[tenantId, body.email, body.role, inviteTokenHash, userId],
|
||||
);
|
||||
|
||||
return reply.status(201).send({ invite_token: token, expires_at: result.rows[0].expires_at });
|
||||
|
||||
@@ -11,5 +11,9 @@ const envSchema = z.object({
|
||||
LOG_LEVEL: z.string().default('info'),
|
||||
});
|
||||
|
||||
export const config = envSchema.parse(process.env);
|
||||
const parsed = envSchema.parse(process.env);
|
||||
if (process.env.NODE_ENV === 'production' && parsed.JWT_SECRET.includes('change-me')) {
|
||||
throw new Error('FATAL: JWT_SECRET must be changed in production');
|
||||
}
|
||||
export const config = parsed;
|
||||
export type Config = z.infer<typeof envSchema>;
|
||||
|
||||
@@ -14,7 +14,7 @@ export async function withTenant<T>(tenantId: string, fn: (client: pg.PoolClient
|
||||
const client = await pool.connect();
|
||||
try {
|
||||
await client.query('BEGIN');
|
||||
await client.query(`SET LOCAL app.tenant_id = '${tenantId}'`);
|
||||
await client.query('SELECT set_config($1, $2, true)', ['app.tenant_id', tenantId]);
|
||||
const result = await fn(client);
|
||||
await client.query('COMMIT');
|
||||
return result;
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import Fastify from 'fastify';
|
||||
import cors from '@fastify/cors';
|
||||
import helmet from '@fastify/helmet';
|
||||
import rateLimit from '@fastify/rate-limit';
|
||||
import pino from 'pino';
|
||||
import { config } from './config/index.js';
|
||||
import { getPoolForAuth } from './data/db.js';
|
||||
@@ -15,6 +16,7 @@ const app = Fastify({ logger: true });
|
||||
|
||||
await app.register(cors, { origin: config.CORS_ORIGIN });
|
||||
await app.register(helmet);
|
||||
await app.register(rateLimit, { max: 100, timeWindow: '1 minute' });
|
||||
|
||||
const pool = getPoolForAuth();
|
||||
decorateAuth(app);
|
||||
|
||||
Reference in New Issue
Block a user