test(perf): stop hashing test passwords at production cost (#242)
Linting / lint (pull_request) Successful in 2m10s
SonarQube Analysis / sonarqube (pull_request) Successful in 19m15s

The integration suite registers around thirty-five customers and asserts nothing about any of their hashes, yet paid bcrypt cost 12 for every one. bcryptjs is a pure-JS implementation, so it pays that cost several times over compared with a native build, and hashing was most of the suite's wall clock. On a contended runner it pushed adminInventory.integration.test.ts past its twenty-second timeout, which then surfaced as a foreign key violation somewhere else entirely — the test timed out, jest moved on, beforeEach truncated, and the still-in-flight registration wrote a token for a customer that had just been deleted.

Measured rather than asserted, warm run against warm run with only the constant changed: 34.5s at cost 12, 9.8s at cost 4. Three and a half times faster, about twenty-five seconds off every integration run, with all 263 tests passing either way.

The first attempt at that measurement was wrong and worth recording. Comparing a cold run at cost 4 against a warm run at cost 12 made the change look like a 36% regression-shaped improvement of the wrong size; the difference was ts-jest and Postgres warming up, not the cost factor. Both numbers above are second runs, and the cost-12 figure was taken twice — 34.3s and 34.5s — before being believed.

Deliberately not configurable. An environment variable here would be a way to weaken password hashing in production by misconfiguration, and nothing needs to tune it. The only route to the cheap cost is NODE_ENV=test, which a deployed container would announce anyway by refusing to serve the built frontend, since app.ts gates static serving on the same value. A setting that quietly degrades a security property should be unreachable rather than warned about, which is the reasoning that already made DEMO_MODE strict.

`hashRoundsFor` is pure and separately tested because the failure it guards against is silent: only the exact string 'test' earns the cheap cost, and an unset NODE_ENV gets the strong one, so the dangerous direction has to be asked for explicitly. Both constants are pinned by assertions too — without that the branch tests pass while the numbers drift to something useless.

Closes #242
This commit is contained in:
2026-08-30 12:29:19 -05:00
parent 2ba35f8732
commit d7dacffa11
3 changed files with 92 additions and 3 deletions
+4 -3
View File
@@ -1,5 +1,6 @@
import { Router, Request, Response } from 'express';
import bcrypt from 'bcryptjs';
import { PASSWORD_HASH_ROUNDS } from '../passwordHashing';
import crypto from 'node:crypto';
import { pool, requireRow } from '../db';
import { requireCustomer } from '../middleware/customerAuth';
@@ -207,7 +208,7 @@ router.post('/register', asyncRoute(async (req: Request, res: Response) => {
const { rows: existing } = await pool.query<IdRow>(`SELECT id FROM customers WHERE email = $1`, [normalizedEmail]);
if (existing.length) return res.status(409).json({ error: 'an account with this email already exists' });
const passwordHash = await bcrypt.hash(password, 12);
const passwordHash = await bcrypt.hash(password, PASSWORD_HASH_ROUNDS);
const unsubscribeToken = crypto.randomBytes(16).toString('hex');
const consent = !!marketingConsent;
@@ -336,7 +337,7 @@ router.post('/reset-password', asyncRoute(async (req: Request, res: Response) =>
return res.status(403).json({ error: 'this account has been disabled' });
}
const passwordHash = await bcrypt.hash(String(password), 12);
const passwordHash = await bcrypt.hash(String(password), PASSWORD_HASH_ROUNDS);
const client = await pool.connect();
try {
@@ -475,7 +476,7 @@ router.post('/change-password', requireCustomer, asyncRoute(async (req: Request,
if (!(await bcrypt.compare(currentPassword || '', customer.password_hash))) {
return res.status(401).json({ error: 'current password is incorrect' });
}
const newHash = await bcrypt.hash(newPassword, 12);
const newHash = await bcrypt.hash(newPassword, PASSWORD_HASH_ROUNDS);
await pool.query(`UPDATE customers SET password_hash = $1 WHERE id = $2`, [newHash, req.customerId]);
// Password reset already ends every session, on the reasoning that a password