fix(security): fail-closed JWT secret validation in non-dev envs#3791
Conversation
In production/staging environments, throw (crash loud) when the JWT secret is empty, shorter than 32 chars, or matches any known default placeholder (devkit + all downstream projects). Dev/test/local keep the existing console.log warn so local and CI boots are unaffected. Also tightens the home-service readiness check to the same predicate (< 32 chars + known defaults) for a consistent security status page. Closes #3737
|
Warning Review limit reached
More reviews will be available in 15 minutes and 46 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Hardens JWT secret validation to fail closed in non-dev environments, preventing deployments from starting with empty/weak/default JWT signing secrets (per #3737).
Changes:
- Updated
validateJwtSecretto throw in non-dev/test/local envs when the secret is empty, < 32 chars, or matches known default placeholders. - Tightened the Home readiness JWT check to flag short secrets and known defaults.
- Added unit tests covering the environment/secret-strength behavior matrix for
validateJwtSecret.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| modules/home/services/home.service.js | Updates readiness JWT secret weakness detection (length + known defaults). |
| lib/helpers/config.js | Implements fail-closed JWT secret validation with environment-based behavior. |
| lib/helpers/tests/config.validateJwtSecret.unit.tests.js | Adds unit coverage for the new validation rules across environments. |
| const secret = config.jwt?.secret; | ||
| const env = process.env.NODE_ENV ?? 'development'; | ||
|
|
||
| const isWeak = !secret || secret.trim() === '' || secret.length < 32 || JWT_DEFAULT_SECRETS.has(secret); | ||
|
|
||
| if (!isWeak) return; // strong secret — nothing to do |
| const jwtSecret = config.jwt?.secret; | ||
| const jwtInsecure = !jwtSecret || jwtSecret.trim() === '' || jwtSecret === 'WaosSecretKeyExampleToChnageAbsolutely'; | ||
| const jwtInsecure = !jwtSecret || jwtSecret.trim() === '' || jwtSecret.length < 32 || JWT_DEFAULTS.has(jwtSecret); | ||
| checks.push({ |
| const JWT_DEFAULTS = new Set([ | ||
| 'WaosSecretKeyExampleToChnageAbsolutely', | ||
| 'TrawlNodeDevSecret', | ||
| 'ComesNodeDevSecret', | ||
| 'MontaineNodeDevSecret', | ||
| 'PierrebNodeDevSecret', | ||
| 'IsmNodeDevSecret', | ||
| ]); |
| afterEach(() => { | ||
| consoleLogSpy.mockRestore(); | ||
| process.env.NODE_ENV = originalEnv; | ||
| }); |
… new fail-closed threshold)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3791 +/- ##
==========================================
+ Coverage 90.19% 90.25% +0.05%
==========================================
Files 152 152
Lines 5020 5030 +10
Branches 1598 1601 +3
==========================================
+ Hits 4528 4540 +12
+ Misses 387 386 -1
+ Partials 105 104 -1
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Closes #3737
validateJwtSecret now throws (fail-closed) in non-dev/test envs when the JWT secret is empty, under 32 chars, or matches a known default/placeholder. Dev/test keep warn-only behavior (so local + CI still boot).
WARNING — DO NOT MERGE until downstream secret injection is confirmed: every deployed downstream must inject a strong (>=32 char, non-default) JWT secret, else this fail-closed check crashes prod boot.
Phase 0 critical-review: OK with nits (1 medium — JWT_DEFAULT_SECRETS Set duplicated in home.service.js; dedupe or follow-up).