-
-
Notifications
You must be signed in to change notification settings - Fork 10
fix(config): gate production hardening on isProd predicate (env-gate defect class) #3874
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
683e784
fix(config): export isProd/isDevEnv predicate
PierreBrisorgueil 5688f5f
fix(responses): stop leaking error objects in non-dev envs
PierreBrisorgueil 3d07d3c
fix(config): activate api+billingPlans rate limiters via base layer
PierreBrisorgueil cc0ca33
fix(config): gate swagger docs + mongoose debug off non-dev envs
PierreBrisorgueil eeaeab3
test(config): harden env-gate test isolation + cover local env
PierreBrisorgueil 0c83b9b
docs(config): align JSDoc defaults + add @returns to test helpers (CR…
PierreBrisorgueil File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| /** | ||
| * Module dependencies. | ||
| */ | ||
| import { describe, test, expect } from '@jest/globals'; | ||
| import configHelper from '../config.js'; | ||
|
|
||
| /** | ||
| * Unit tests for the environment predicate — isDevEnv / isProd. | ||
| * | ||
| * These are the shared production-hardening predicate. The deployment model runs | ||
| * downstream apps as NODE_ENV={projectName} (any non-dev label), so hardening must | ||
| * key off "is this NOT a known dev env" rather than the literal "production". | ||
| */ | ||
| describe('config helper — environment predicate (isDevEnv / isProd):', () => { | ||
| describe('isDevEnv', () => { | ||
| test('returns true for development', () => { | ||
| expect(configHelper.isDevEnv('development')).toBe(true); | ||
| }); | ||
|
|
||
| test('returns true for test', () => { | ||
| expect(configHelper.isDevEnv('test')).toBe(true); | ||
| }); | ||
|
|
||
| test('returns true for local', () => { | ||
| expect(configHelper.isDevEnv('local')).toBe(true); | ||
| }); | ||
|
|
||
| test('returns false for production', () => { | ||
| expect(configHelper.isDevEnv('production')).toBe(false); | ||
| }); | ||
|
|
||
| test('returns false for an arbitrary project env label', () => { | ||
| expect(configHelper.isDevEnv('someproject')).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe('isProd', () => { | ||
| test('returns true for production', () => { | ||
| expect(configHelper.isProd('production')).toBe(true); | ||
| }); | ||
|
|
||
| test('returns true for an arbitrary project env label (downstream deployment model)', () => { | ||
| expect(configHelper.isProd('someproject')).toBe(true); | ||
| }); | ||
|
|
||
| test('returns false for development', () => { | ||
| expect(configHelper.isProd('development')).toBe(false); | ||
| }); | ||
|
|
||
| test('returns false for test', () => { | ||
| expect(configHelper.isProd('test')).toBe(false); | ||
| }); | ||
|
|
||
| test('returns false for local', () => { | ||
| expect(configHelper.isProd('local')).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe('default argument reads process.env.NODE_ENV at call time', () => { | ||
| test('isDevEnv() honors NODE_ENV mutated after import', () => { | ||
| const original = process.env.NODE_ENV; | ||
| try { | ||
| process.env.NODE_ENV = 'production'; | ||
| expect(configHelper.isDevEnv()).toBe(false); | ||
| expect(configHelper.isProd()).toBe(true); | ||
| process.env.NODE_ENV = 'development'; | ||
| expect(configHelper.isDevEnv()).toBe(true); | ||
| expect(configHelper.isProd()).toBe(false); | ||
| } finally { | ||
| process.env.NODE_ENV = original; | ||
| } | ||
| }); | ||
|
|
||
| test('isProd() defaults to development (dev) when NODE_ENV is unset', () => { | ||
| const original = process.env.NODE_ENV; | ||
| try { | ||
| delete process.env.NODE_ENV; | ||
| expect(configHelper.isProd()).toBe(false); | ||
| expect(configHelper.isDevEnv()).toBe(true); | ||
| } finally { | ||
| process.env.NODE_ENV = original; | ||
| } | ||
| }); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| /** | ||
| * Module dependencies. | ||
| */ | ||
| import { describe, test, expect, beforeEach, afterEach } from '@jest/globals'; | ||
| import responses from '../responses.js'; | ||
|
|
||
| /** | ||
| * Build a minimal Express response double that captures status + json body. | ||
| * @returns {{status: Function, json: Function, _status: number, _body: object}} | ||
| */ | ||
| const buildRes = () => { | ||
| const res = { | ||
| _status: undefined, | ||
| _body: undefined, | ||
| status(code) { this._status = code; return this; }, | ||
| json(body) { this._body = body; return this; }, | ||
| }; | ||
| return res; | ||
| }; | ||
|
|
||
| /** | ||
| * Unit tests — responses.error must NOT serialize the raw error object into the | ||
| * client payload outside of dev/test/local. The deployment model runs apps under | ||
| * arbitrary NODE_ENV labels, so the leak gate keys off the dev-env predicate, not | ||
| * the literal `production`. | ||
| */ | ||
| describe('responses.error — error-object leak gating:', () => { | ||
| let originalNodeEnv; | ||
|
|
||
| beforeEach(() => { | ||
| originalNodeEnv = process.env.NODE_ENV; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| process.env.NODE_ENV = originalNodeEnv; | ||
| }); | ||
|
|
||
| test('does NOT include serialized error in body under a project (non-dev) env', () => { | ||
| process.env.NODE_ENV = 'someproject'; | ||
| const res = buildRes(); | ||
| responses.error(res, 500, undefined, undefined)(new Error('secret internal detail')); | ||
| expect(res._body).toBeDefined(); | ||
| expect(res._body.error).toBeUndefined(); | ||
| // The generic envelope fields stay present. | ||
| expect(res._body.type).toBe('error'); | ||
| expect(res._body.status).toBe(500); | ||
| }); | ||
|
|
||
| test('does NOT include serialized error in body under production', () => { | ||
| process.env.NODE_ENV = 'production'; | ||
| const res = buildRes(); | ||
| responses.error(res, 500)(new Error('secret internal detail')); | ||
| expect(res._body.error).toBeUndefined(); | ||
| }); | ||
|
|
||
| test('DOES include serialized error in body under development (debugging aid)', () => { | ||
| process.env.NODE_ENV = 'development'; | ||
| const res = buildRes(); | ||
| responses.error(res, 500)(new Error('debuggable detail')); | ||
| expect(typeof res._body.error).toBe('string'); | ||
| }); | ||
|
|
||
| test('DOES include serialized error in body under test', () => { | ||
| process.env.NODE_ENV = 'test'; | ||
| const res = buildRes(); | ||
| responses.error(res, 500)(new Error('debuggable detail')); | ||
| expect(typeof res._body.error).toBe('string'); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| /** | ||
| * Module dependencies. | ||
| */ | ||
| import { describe, test, expect } from '@jest/globals'; | ||
| import organizationsDevConfig from '../../../modules/organizations/config/organizations.development.config.js'; | ||
| import billingDevConfig from '../../../modules/billing/config/billing.development.config.js'; | ||
| import authDevConfig from '../../../modules/auth/config/auth.development.config.js'; | ||
|
|
||
| /** | ||
| * Config-layering regression guard for the rate-limiter env-gate defect. | ||
| * | ||
| * The rate-limiter middleware is presence-driven: `limiters.<name>` returns a real | ||
| * limiter iff `config.rateLimit.<name>` exists in merged config, else a no-op. | ||
| * Previously `rateLimit.api` and `rateLimit.billingPlans` existed ONLY in | ||
| * `config/defaults/production.config.js` (literal `NODE_ENV=production`) and | ||
| * `test.config.js`, so under the downstream run model (NODE_ENV=<project>) they | ||
| * were undefined → no-op → the public Stripe-fanout and membership-request routes | ||
| * ran unthrottled. | ||
| * | ||
| * Fix: each profile lives in its owning module's *.development.config.js — a base | ||
| * (Layer 1) layer that ALWAYS merges regardless of NODE_ENV, mirroring how the | ||
| * `auth` profile is provided. Stricter caps stay as production-config overrides. | ||
| * | ||
| * A base-layer profile is a structural contract; assert its presence + shape so a | ||
| * future refactor that drops it (re-opening the defect) fails loudly. | ||
| */ | ||
| describe('rate-limiter base-layer profiles (env-gate config-layering):', () => { | ||
| /** | ||
| * Assert a profile is a usable express-rate-limit options object. | ||
| * @param {object} profile - a config.rateLimit.<name> profile | ||
| * @returns {void} | ||
| */ | ||
| const expectUsableProfile = (profile) => { | ||
| expect(profile).toBeDefined(); | ||
| expect(typeof profile).toBe('object'); | ||
| expect(Number.isInteger(profile.windowMs)).toBe(true); | ||
| expect(profile.windowMs).toBeGreaterThan(0); | ||
| expect(Number.isInteger(profile.max)).toBe(true); | ||
| expect(profile.max).toBeGreaterThan(0); | ||
| }; | ||
|
|
||
| test('auth profile already lives in the auth base layer (reference pattern)', () => { | ||
| expectUsableProfile(authDevConfig.rateLimit.auth); | ||
| }); | ||
|
|
||
| test('api profile lives in the organizations base layer (always merges)', () => { | ||
| expectUsableProfile(organizationsDevConfig.rateLimit.api); | ||
| }); | ||
|
|
||
| test('billingPlans profile lives in the billing base layer (always merges)', () => { | ||
| expectUsableProfile(billingDevConfig.rateLimit.billingPlans); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.