Skip to content

Commit edf5b93

Browse files
fix(config): gate production hardening on isProd predicate (env-gate defect class) (#3874)
* fix(config): export isProd/isDevEnv predicate * fix(responses): stop leaking error objects in non-dev envs * fix(config): activate api+billingPlans rate limiters via base layer * fix(config): gate swagger docs + mongoose debug off non-dev envs * test(config): harden env-gate test isolation + cover local env Capture NODE_ENV in beforeEach/afterEach (safe restore even if a sibling test mutates it before the file loads). Document the inline DEV_ENVS set in express.docsEnvGate must stay in sync with lib/helpers/config.js. Add local-env positive cases to docsEnvGate (mounts docs) and debugGate (debug true) — mirrors the existing development cases. * docs(config): align JSDoc defaults + add @returns to test helpers (CR pass-1) - Clarify that isDevEnv/isProd default to 'development' when NODE_ENV is unset - Add @returns {void} to expectUsableProfile in rateLimiter.baseLayer tests - Add JSDoc to buildMockApp helper in express.docsEnvGate tests
1 parent 351ed34 commit edf5b93

14 files changed

Lines changed: 537 additions & 9 deletions

config/defaults/production.config.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@ const config = {
22
app: {
33
title: 'Devkit Node - Production Environment',
44
},
5+
// Secure-by-default: keep the unauthenticated API docs surface off in production.
6+
// The runtime gate in lib/services/express.js (isProd) already prevents mounting
7+
// docs in any non-dev env; this flag makes the intent explicit at the config layer.
8+
swagger: {
9+
enable: false,
10+
},
511
api: {
612
host: '0.0.0.0',
713
port: 4200,

lib/helpers/config.js

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,31 @@ const isJwtSecretWeak = (secret) => !secret || secret.trim() === '' || secret.le
8080

8181
/**
8282
* Safe envs where a weak / default secret is tolerated (warn only).
83+
* Also the single source of truth for the dev-vs-prod hardening predicate:
84+
* any NODE_ENV outside this set is treated as a production-grade deployment.
8385
*/
8486
const DEV_ENVS = new Set(['development', 'test', 'local']);
8587

88+
/**
89+
* @desc Predicate — is the given env a known development-grade env?
90+
* Used to gate production hardening. The deployment model runs apps under
91+
* arbitrary NODE_ENV labels, so "dev" is an explicit allow-list (DEV_ENVS),
92+
* never a literal `=== 'development'` check.
93+
* @param {string} [env=process.env.NODE_ENV??'development'] - environment name (read at call time); defaults to 'development' when NODE_ENV is unset
94+
* @returns {boolean} true when env is one of development/test/local
95+
*/
96+
const isDevEnv = (env = process.env.NODE_ENV ?? 'development') => DEV_ENVS.has(env);
97+
98+
/**
99+
* @desc Predicate — should production hardening apply to the given env?
100+
* Inverse of {@link isDevEnv}: true for `production` AND for any non-dev label
101+
* (e.g. a deployment env name), so hardening is secure-by-default off any
102+
* non-dev env, not just the literal `production`.
103+
* @param {string} [env=process.env.NODE_ENV??'development'] - environment name (read at call time); defaults to 'development' when NODE_ENV is unset
104+
* @returns {boolean} true when env is NOT one of development/test/local
105+
*/
106+
const isProd = (env = process.env.NODE_ENV ?? 'development') => !DEV_ENVS.has(env);
107+
86108
/**
87109
* @desc Validate JWT secret strength.
88110
* - In non-dev/non-test environments: throw (fail-closed) when the secret is
@@ -102,7 +124,7 @@ const validateJwtSecret = (config) => {
102124

103125
const message = '+ Important warning: JWT secret is empty, too short (< 32 chars), or set to a known default placeholder. Set a strong secret via DEVKIT_NODE_jwt_secret.';
104126

105-
if (DEV_ENVS.has(env)) {
127+
if (isDevEnv(env)) {
106128
console.log(chalk.red(message));
107129
return;
108130
}
@@ -213,6 +235,8 @@ export default {
213235
validateDomainIsSet,
214236
JWT_DEFAULT_SECRETS,
215237
isJwtSecretWeak,
238+
isDevEnv,
239+
isProd,
216240
validateJwtSecret,
217241
initSecureMode,
218242
initGlobalConfigFiles,

lib/helpers/responses.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import configHelper from './config.js';
2+
13
/**
24
* @desc Function res success
35
* @param {Object} res - Express response object
@@ -97,7 +99,10 @@ const error = (res, httpStatus, message, description) => (error = {}) => {
9799
errorCode: getErrorCode(error),
98100
description: getDescription(description, error),
99101
};
100-
if (process.env.NODE_ENV !== 'production') result.error = safeStringify(error);
102+
// Only expose the serialized raw error in dev-grade envs (development/test/local).
103+
// Any other NODE_ENV (production or a deployment env label) gets the generic
104+
// envelope only — prevents internal detail leaks under the downstream run model.
105+
if (!configHelper.isProd()) result.error = safeStringify(error);
101106
res.status(status).json(result);
102107
return result;
103108
};
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* Module dependencies.
3+
*/
4+
import { describe, test, expect } from '@jest/globals';
5+
import configHelper from '../config.js';
6+
7+
/**
8+
* Unit tests for the environment predicate — isDevEnv / isProd.
9+
*
10+
* These are the shared production-hardening predicate. The deployment model runs
11+
* downstream apps as NODE_ENV={projectName} (any non-dev label), so hardening must
12+
* key off "is this NOT a known dev env" rather than the literal "production".
13+
*/
14+
describe('config helper — environment predicate (isDevEnv / isProd):', () => {
15+
describe('isDevEnv', () => {
16+
test('returns true for development', () => {
17+
expect(configHelper.isDevEnv('development')).toBe(true);
18+
});
19+
20+
test('returns true for test', () => {
21+
expect(configHelper.isDevEnv('test')).toBe(true);
22+
});
23+
24+
test('returns true for local', () => {
25+
expect(configHelper.isDevEnv('local')).toBe(true);
26+
});
27+
28+
test('returns false for production', () => {
29+
expect(configHelper.isDevEnv('production')).toBe(false);
30+
});
31+
32+
test('returns false for an arbitrary project env label', () => {
33+
expect(configHelper.isDevEnv('someproject')).toBe(false);
34+
});
35+
});
36+
37+
describe('isProd', () => {
38+
test('returns true for production', () => {
39+
expect(configHelper.isProd('production')).toBe(true);
40+
});
41+
42+
test('returns true for an arbitrary project env label (downstream deployment model)', () => {
43+
expect(configHelper.isProd('someproject')).toBe(true);
44+
});
45+
46+
test('returns false for development', () => {
47+
expect(configHelper.isProd('development')).toBe(false);
48+
});
49+
50+
test('returns false for test', () => {
51+
expect(configHelper.isProd('test')).toBe(false);
52+
});
53+
54+
test('returns false for local', () => {
55+
expect(configHelper.isProd('local')).toBe(false);
56+
});
57+
});
58+
59+
describe('default argument reads process.env.NODE_ENV at call time', () => {
60+
test('isDevEnv() honors NODE_ENV mutated after import', () => {
61+
const original = process.env.NODE_ENV;
62+
try {
63+
process.env.NODE_ENV = 'production';
64+
expect(configHelper.isDevEnv()).toBe(false);
65+
expect(configHelper.isProd()).toBe(true);
66+
process.env.NODE_ENV = 'development';
67+
expect(configHelper.isDevEnv()).toBe(true);
68+
expect(configHelper.isProd()).toBe(false);
69+
} finally {
70+
process.env.NODE_ENV = original;
71+
}
72+
});
73+
74+
test('isProd() defaults to development (dev) when NODE_ENV is unset', () => {
75+
const original = process.env.NODE_ENV;
76+
try {
77+
delete process.env.NODE_ENV;
78+
expect(configHelper.isProd()).toBe(false);
79+
expect(configHelper.isDevEnv()).toBe(true);
80+
} finally {
81+
process.env.NODE_ENV = original;
82+
}
83+
});
84+
});
85+
});
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/**
2+
* Module dependencies.
3+
*/
4+
import { describe, test, expect, beforeEach, afterEach } from '@jest/globals';
5+
import responses from '../responses.js';
6+
7+
/**
8+
* Build a minimal Express response double that captures status + json body.
9+
* @returns {{status: Function, json: Function, _status: number, _body: object}}
10+
*/
11+
const buildRes = () => {
12+
const res = {
13+
_status: undefined,
14+
_body: undefined,
15+
status(code) { this._status = code; return this; },
16+
json(body) { this._body = body; return this; },
17+
};
18+
return res;
19+
};
20+
21+
/**
22+
* Unit tests — responses.error must NOT serialize the raw error object into the
23+
* client payload outside of dev/test/local. The deployment model runs apps under
24+
* arbitrary NODE_ENV labels, so the leak gate keys off the dev-env predicate, not
25+
* the literal `production`.
26+
*/
27+
describe('responses.error — error-object leak gating:', () => {
28+
let originalNodeEnv;
29+
30+
beforeEach(() => {
31+
originalNodeEnv = process.env.NODE_ENV;
32+
});
33+
34+
afterEach(() => {
35+
process.env.NODE_ENV = originalNodeEnv;
36+
});
37+
38+
test('does NOT include serialized error in body under a project (non-dev) env', () => {
39+
process.env.NODE_ENV = 'someproject';
40+
const res = buildRes();
41+
responses.error(res, 500, undefined, undefined)(new Error('secret internal detail'));
42+
expect(res._body).toBeDefined();
43+
expect(res._body.error).toBeUndefined();
44+
// The generic envelope fields stay present.
45+
expect(res._body.type).toBe('error');
46+
expect(res._body.status).toBe(500);
47+
});
48+
49+
test('does NOT include serialized error in body under production', () => {
50+
process.env.NODE_ENV = 'production';
51+
const res = buildRes();
52+
responses.error(res, 500)(new Error('secret internal detail'));
53+
expect(res._body.error).toBeUndefined();
54+
});
55+
56+
test('DOES include serialized error in body under development (debugging aid)', () => {
57+
process.env.NODE_ENV = 'development';
58+
const res = buildRes();
59+
responses.error(res, 500)(new Error('debuggable detail'));
60+
expect(typeof res._body.error).toBe('string');
61+
});
62+
63+
test('DOES include serialized error in body under test', () => {
64+
process.env.NODE_ENV = 'test';
65+
const res = buildRes();
66+
responses.error(res, 500)(new Error('debuggable detail'));
67+
expect(typeof res._body.error).toBe('string');
68+
});
69+
});
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/**
2+
* Module dependencies.
3+
*/
4+
import { describe, test, expect } from '@jest/globals';
5+
import organizationsDevConfig from '../../../modules/organizations/config/organizations.development.config.js';
6+
import billingDevConfig from '../../../modules/billing/config/billing.development.config.js';
7+
import authDevConfig from '../../../modules/auth/config/auth.development.config.js';
8+
9+
/**
10+
* Config-layering regression guard for the rate-limiter env-gate defect.
11+
*
12+
* The rate-limiter middleware is presence-driven: `limiters.<name>` returns a real
13+
* limiter iff `config.rateLimit.<name>` exists in merged config, else a no-op.
14+
* Previously `rateLimit.api` and `rateLimit.billingPlans` existed ONLY in
15+
* `config/defaults/production.config.js` (literal `NODE_ENV=production`) and
16+
* `test.config.js`, so under the downstream run model (NODE_ENV=<project>) they
17+
* were undefined → no-op → the public Stripe-fanout and membership-request routes
18+
* ran unthrottled.
19+
*
20+
* Fix: each profile lives in its owning module's *.development.config.js — a base
21+
* (Layer 1) layer that ALWAYS merges regardless of NODE_ENV, mirroring how the
22+
* `auth` profile is provided. Stricter caps stay as production-config overrides.
23+
*
24+
* A base-layer profile is a structural contract; assert its presence + shape so a
25+
* future refactor that drops it (re-opening the defect) fails loudly.
26+
*/
27+
describe('rate-limiter base-layer profiles (env-gate config-layering):', () => {
28+
/**
29+
* Assert a profile is a usable express-rate-limit options object.
30+
* @param {object} profile - a config.rateLimit.<name> profile
31+
* @returns {void}
32+
*/
33+
const expectUsableProfile = (profile) => {
34+
expect(profile).toBeDefined();
35+
expect(typeof profile).toBe('object');
36+
expect(Number.isInteger(profile.windowMs)).toBe(true);
37+
expect(profile.windowMs).toBeGreaterThan(0);
38+
expect(Number.isInteger(profile.max)).toBe(true);
39+
expect(profile.max).toBeGreaterThan(0);
40+
};
41+
42+
test('auth profile already lives in the auth base layer (reference pattern)', () => {
43+
expectUsableProfile(authDevConfig.rateLimit.auth);
44+
});
45+
46+
test('api profile lives in the organizations base layer (always merges)', () => {
47+
expectUsableProfile(organizationsDevConfig.rateLimit.api);
48+
});
49+
50+
test('billingPlans profile lives in the billing base layer (always merges)', () => {
51+
expectUsableProfile(billingDevConfig.rateLimit.billingPlans);
52+
});
53+
});

lib/services/express.js

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import YAML from 'js-yaml';
1818
import redoc from 'redoc-express';
1919

2020
import config from '../../config/index.js';
21+
import configHelper from '../helpers/config.js';
2122
import guidesHelper from '../helpers/guides.js';
2223
import redactUrl from '../helpers/redactUrl.js';
2324
import logger from './logger.js';
@@ -75,7 +76,11 @@ const redocCustomCss = `
7576
* @returns {void}
7677
*/
7778
const initSwagger = (app) => {
78-
if (config.swagger.enable) {
79+
// Secure-by-default: the API docs surface (/api/spec.json + /api/docs) is
80+
// UNAUTHENTICATED, so it is only mounted in dev-grade envs. Any production-grade
81+
// env (the literal `production` OR a deployment env label) skips it even when
82+
// config.swagger.enable is still truthy — opt-OUT by default in non-dev.
83+
if (config.swagger.enable && !configHelper.isProd()) {
7984
if (!config.files.swagger || config.files.swagger.length === 0) {
8085
logger.warn('[swagger] no swagger files configured — skipping API docs');
8186
return;
@@ -332,8 +337,11 @@ const initModulesServerRoutes = async (app) => {
332337
const initErrorRoutes = (app) => {
333338
/**
334339
* Express 4-arity error handler.
335-
* In production: returns a generic 500 to prevent leaking internal error details (#3726).
336-
* In non-production: includes err.message and err.code for debugging.
340+
* In production-grade envs: returns a generic 500 to prevent leaking internal
341+
* error details (#3726). In dev-grade envs (development/test/local): includes
342+
* err.message and err.code for debugging. "Production-grade" is any env outside
343+
* the dev allow-list, covering the downstream run model where apps boot under a
344+
* deployment env label rather than the literal `production`.
337345
* @param {Error} err - The error object caught by Express
338346
* @param {import('express').Request} req - Express request object
339347
* @param {import('express').Response} res - Express response object
@@ -343,9 +351,9 @@ const initErrorRoutes = (app) => {
343351
app.use((err, req, res, next) => {
344352
if (!err) return next();
345353
logger.error('Unhandled express error', { error: err });
346-
// In production never leak internal error details (message, code) to clients.
347-
// Detailed errors are still logged above for observability.
348-
if (process.env.NODE_ENV === 'production') {
354+
// In any production-grade env never leak internal error details (message, code)
355+
// to clients. Detailed errors are still logged above for observability.
356+
if (configHelper.isProd()) {
349357
return res.status(err.status || 500).send({ message: 'Internal Server Error' });
350358
}
351359
res.status(err.status || 500).send({

lib/services/mongoose.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,20 @@ import chalk from 'chalk';
66
import mongoose from 'mongoose';
77
import path from 'path';
88
import config from '../../config/index.js';
9+
import configHelper from '../helpers/config.js';
910
import logger from './logger.js';
1011

12+
/**
13+
* @desc Resolve the effective mongoose `debug` flag.
14+
* Query logging is enabled only when BOTH the config opt-in is set AND the env is
15+
* dev-grade (development/test/local). Any production-grade env (the literal
16+
* `production` OR a deployment env label) forces it off so verbose query logs —
17+
* which can include collection/field/value detail — never run in production.
18+
* @param {object} [cfg=config] - application configuration object
19+
* @returns {boolean} effective debug flag
20+
*/
21+
const resolveDebug = (cfg = config) => Boolean(cfg?.db?.debug) && configHelper.isDevEnv();
22+
1123
/**
1224
* Load all mongoose related models
1325
*/
@@ -38,7 +50,7 @@ const connect = async () => {
3850
if (mongoOptions.sslKey) mongoOptions.sslKey = path.resolve(mongoOptions.sslKey);
3951

4052
await mongoose.connect(config.db.uri, mongoOptions);
41-
mongoose.set('debug', config.db.debug);
53+
mongoose.set('debug', resolveDebug(config));
4254
logger.info(chalk.yellow('Connected to MongoDB.'));
4355

4456
return mongoose;
@@ -62,4 +74,5 @@ export default {
6274
loadModels,
6375
connect,
6476
disconnect,
77+
resolveDebug,
6578
};

0 commit comments

Comments
 (0)