Fix hardcoded JWT secrets in .env.example and constants.js (CWE-798)#178
Fix hardcoded JWT secrets in .env.example and constants.js (CWE-798)#178saaa99999999 wants to merge 1 commit into
Conversation
Replaced hardcoded JWT_SECRET values in .env.example with empty placeholders and generation instructions. Moved testConfig JWT_SECRET from hardcoded string to process.env.JWT_SECRET_TEST. Added startup validation that rejects empty, known-default, and short JWT secrets.
📝 WalkthroughWalkthroughThis PR strengthens JWT secret configuration by replacing hardcoded examples in ChangesJWT Secret Configuration and Validation
Sequence DiagramsequenceDiagram
participant Developer
participant EnvTemplate as .env.example
participant ConfigModule as constants.js
Developer->>EnvTemplate: Read setup instructions
EnvTemplate->>Developer: Generate secret via crypto.randomBytes(64)
Developer->>ConfigModule: Module loads with JWT_SECRET from env
ConfigModule->>ConfigModule: Validate secret non-empty
ConfigModule->>ConfigModule: Validate secret not weak/placeholder
ConfigModule->>ConfigModule: Validate secret length >= 32 chars
ConfigModule->>Developer: Export validated config or throw error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes hardcoded JWT secrets from the environment example and configuration files, transitioning to environment-variable-based configuration across all stages. It also introduces a startup validation check to ensure the JWT_SECRET is present, not a known weak value, and meets a minimum length requirement. Feedback suggests refining the validation logic to handle whitespace and case-insensitivity for weak secrets to prevent accidental bypasses.
| const KNOWN_WEAK_SECRETS = [ | ||
| '', 'ewtijwebgiuweg9w98u9283982t!!u1h28h1t1h89u9h@$$', | ||
| 'secret', 'changeme', 'jwt_secret', 'your-secret-key', | ||
| ]; | ||
| if (!config.JWT_SECRET || KNOWN_WEAK_SECRETS.includes(config.JWT_SECRET)) { | ||
| throw new Error( | ||
| 'JWT_SECRET is not set or uses a known default value. ' + | ||
| 'Set JWT_SECRET_DEV, JWT_SECRET_TEST, or JWT_SECRET_PROD in your .env file. ' + | ||
| 'Generate a secure key: node -e "console.log(require(\'crypto\').randomBytes(64).toString(\'hex\'))"' | ||
| ); | ||
| } | ||
| if (config.JWT_SECRET.length < 32) { | ||
| throw new Error( | ||
| `JWT_SECRET must be at least 32 characters. Current length: ${config.JWT_SECRET.length}.` | ||
| ); | ||
| } |
There was a problem hiding this comment.
The JWT secret validation can be improved by handling potential whitespace and performing case-insensitive checks for known weak secrets. This prevents accidental bypasses with leading/trailing spaces or variations like 'Secret'. Also, the empty string in the KNOWN_WEAK_SECRETS array is redundant as the !config.JWT_SECRET check already handles empty strings, null, and undefined values. The length check is also updated to use the trimmed secret for accuracy.
| const KNOWN_WEAK_SECRETS = [ | |
| '', 'ewtijwebgiuweg9w98u9283982t!!u1h28h1t1h89u9h@$$', | |
| 'secret', 'changeme', 'jwt_secret', 'your-secret-key', | |
| ]; | |
| if (!config.JWT_SECRET || KNOWN_WEAK_SECRETS.includes(config.JWT_SECRET)) { | |
| throw new Error( | |
| 'JWT_SECRET is not set or uses a known default value. ' + | |
| 'Set JWT_SECRET_DEV, JWT_SECRET_TEST, or JWT_SECRET_PROD in your .env file. ' + | |
| 'Generate a secure key: node -e "console.log(require(\'crypto\').randomBytes(64).toString(\'hex\'))"' | |
| ); | |
| } | |
| if (config.JWT_SECRET.length < 32) { | |
| throw new Error( | |
| `JWT_SECRET must be at least 32 characters. Current length: ${config.JWT_SECRET.length}.` | |
| ); | |
| } | |
| const KNOWN_WEAK_SECRETS = [ | |
| 'ewtijwebgiuweg9w98u9283982t!!u1h28h1t1h89u9h@$$', | |
| 'secret', 'changeme', 'jwt_secret', 'your-secret-key', | |
| ]; | |
| const secret = (config.JWT_SECRET || '').trim(); | |
| if (!secret || KNOWN_WEAK_SECRETS.some(weak => weak.toLowerCase() === secret.toLowerCase())) { | |
| throw new Error( | |
| 'JWT_SECRET is not set or uses a known default value. ' + | |
| 'Set JWT_SECRET_DEV, JWT_SECRET_TEST, or JWT_SECRET_PROD in your .env file. ' + | |
| 'Generate a secure key: node -e "console.log(require(\'crypto\').randomBytes(64).toString(\'hex\'))"' | |
| ); | |
| } | |
| if (secret.length < 32) { | |
| throw new Error( | |
| 'JWT_SECRET must be at least 32 characters (excluding whitespace). Current length: ' + secret.length + '.' | |
| ); | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/config/constants.js`:
- Around line 51-56: The denylist check for weak JWT secrets is brittle and
contains a typo in the KNOWN_WEAK_SECRETS entry and uses a direct includes which
can be bypassed; fix by correcting the compromised string in KNOWN_WEAK_SECRETS
(replace the incorrect "...h@$$" with the intended "...h@$") and change the
validation around config.JWT_SECRET to normalize the value (trim whitespace and
lower-case) before checking membership—e.g., compute a normalizedSecret =
config.JWT_SECRET.trim().toLowerCase() and compare against a pre-normalized
set/array of KNOWN_WEAK_SECRETS; apply the same normalization-based check
wherever config.JWT_SECRET is validated (the other check referenced around the
62-64 area).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1fbd801-2bc6-455c-beb2-df015e632ae5
📒 Files selected for processing (2)
.env.examplesrc/config/constants.js
| const KNOWN_WEAK_SECRETS = [ | ||
| '', 'ewtijwebgiuweg9w98u9283982t!!u1h28h1t1h89u9h@$$', | ||
| 'secret', 'changeme', 'jwt_secret', 'your-secret-key', | ||
| ]; | ||
| if (!config.JWT_SECRET || KNOWN_WEAK_SECRETS.includes(config.JWT_SECRET)) { | ||
| throw new Error( |
There was a problem hiding this comment.
Fix denylist mismatch and normalize JWT secret before validation.
Line 52 appears to have a typo (...h@$$) versus the compromised value described in this PR (...h@$), so the intended block may not trigger. Also, raw includes checks can be bypassed with whitespace/case variants (for example, " Secret ").
Suggested patch
-const KNOWN_WEAK_SECRETS = [
- '', 'ewtijwebgiuweg9w98u9283982t!!u1h28h1t1h89u9h@$$',
- 'secret', 'changeme', 'jwt_secret', 'your-secret-key',
-];
-if (!config.JWT_SECRET || KNOWN_WEAK_SECRETS.includes(config.JWT_SECRET)) {
+const KNOWN_WEAK_SECRETS = [
+ '',
+ 'ewtijwebgiuweg9w98u9283982t!!u1h28h1t1h89u9h@$',
+ 'secret',
+ 'changeme',
+ 'jwt_secret',
+ 'your-secret-key',
+];
+const normalizedSecret = String(config.JWT_SECRET || '').trim();
+const weakSecrets = new Set(KNOWN_WEAK_SECRETS.map((s) => s.trim().toLowerCase()));
+if (!normalizedSecret || weakSecrets.has(normalizedSecret.toLowerCase())) {
throw new Error(
'JWT_SECRET is not set or uses a known default value. ' +
'Set JWT_SECRET_DEV, JWT_SECRET_TEST, or JWT_SECRET_PROD in your .env file. ' +
'Generate a secure key: node -e "console.log(require(\'crypto\').randomBytes(64).toString(\'hex\'))"'
);
}
-if (config.JWT_SECRET.length < 32) {
+if (normalizedSecret.length < 32) {
throw new Error(
- `JWT_SECRET must be at least 32 characters. Current length: ${config.JWT_SECRET.length}.`
+ `JWT_SECRET must be at least 32 characters. Current length: ${normalizedSecret.length}.`
);
}Also applies to: 62-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/config/constants.js` around lines 51 - 56, The denylist check for weak
JWT secrets is brittle and contains a typo in the KNOWN_WEAK_SECRETS entry and
uses a direct includes which can be bypassed; fix by correcting the compromised
string in KNOWN_WEAK_SECRETS (replace the incorrect "...h@$$" with the intended
"...h@$") and change the validation around config.JWT_SECRET to normalize the
value (trim whitespace and lower-case) before checking membership—e.g., compute
a normalizedSecret = config.JWT_SECRET.trim().toLowerCase() and compare against
a pre-normalized set/array of KNOWN_WEAK_SECRETS; apply the same
normalization-based check wherever config.JWT_SECRET is validated (the other
check referenced around the 62-64 area).
CVE Request — Action Needed from MaintainerThis PR fixes security vulnerabilities. To assign a CVE number: GitHub only issues CVEs from the official upstream repository, not from forks. Please:
If you prefer, I can submit the CVE via MITRE (cveform.mitre.org) instead — just let me know. Thank you for reviewing this PR! |
1 participant
Summary
Replaced hardcoded JWT signing keys in .env.example and src/config/constants.js with environment variable references. Added startup validation that rejects empty, known-default, and short JWT secrets.
Details
Security Impact
This project uses symmetric JWT (HS256) where the same key signs and verifies tokens. The hardcoded key is visible in the public repository history. Anyone with this key can forge valid JWT tokens for any user ??including admin ??bypassing authentication completely (CWE-798).
Before/After
.env.example
`
Generate: node -e "console.log(require('crypto').randomBytes(64).toString('hex'))"
`
src/config/constants.js
`
`
Plus startup validation block (lines 50-66).
Summary by CodeRabbit