fix: fallback to weak defaults when JWT secrets not initialized#5990
fix: fallback to weak defaults when JWT secrets not initialized#5990haroldfabla2-hue wants to merge 1 commit intoFlowiseAI:mainfrom
Conversation
When running docker-compose without setting JWT env vars, initAuthSecrets() may fail to initialize secrets, causing 'JwtStrategy requires a secret or key' error on startup. This fix adds fallback to weak default values (same as WEAK_DEFAULTS) instead of throwing an error, making docker-compose work out of the box. Fixes FlowiseAI#4563
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical startup issue encountered when running the application via Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request aims to improve the developer experience by providing fallback JWT secrets for easier setup with Docker. However, the current implementation introduces a critical security vulnerability by silently using weak, predictable secrets if they are not configured. This could lead to severe security breaches if deployed to production.
My review includes suggestions to make this feature secure by default. Specifically, I recommend checking the environment (NODE_ENV) and throwing an error if the secrets are not set in production, while logging a prominent warning and falling back to defaults only in development environments. This approach maintains the ease of local setup without compromising production security.
| if (jwtAuthTokenSecret === undefined) { | ||
| // Fallback to weak default if initialization failed (e.g., docker with default .env) | ||
| return WEAK_DEFAULTS.JWT_AUTH_TOKEN_SECRET | ||
| } |
There was a problem hiding this comment.
While this change improves the developer experience for local setup, it introduces a critical security risk by silently falling back to weak, predictable JWT secrets. If this configuration is accidentally deployed to production, it would allow attackers to easily forge authentication tokens. A much safer approach is to explicitly prevent the application from starting with weak secrets in a production environment, while still allowing it for development with a clear warning.
You will also need to import the logger at the top of the file:
import logger from '../../utils/logger'| if (jwtAuthTokenSecret === undefined) { | |
| // Fallback to weak default if initialization failed (e.g., docker with default .env) | |
| return WEAK_DEFAULTS.JWT_AUTH_TOKEN_SECRET | |
| } | |
| if (jwtAuthTokenSecret === undefined) { | |
| if (process.env.NODE_ENV === 'production') { | |
| throw new Error('FATAL: JWT_AUTH_TOKEN_SECRET must be set in production. The application will not start with an insecure default secret.'); | |
| } | |
| // Fallback to weak default for development | |
| logger.warn('WARNING: JWT_AUTH_TOKEN_SECRET is not set. Falling back to a weak, insecure default secret. This is for development only and MUST NOT be used in production.'); | |
| return WEAK_DEFAULTS.JWT_AUTH_TOKEN_SECRET | |
| } |
| if (jwtRefreshTokenSecret === undefined) { | ||
| return WEAK_DEFAULTS.JWT_REFRESH_TOKEN_SECRET | ||
| } |
There was a problem hiding this comment.
For the same security reasons as with the auth token secret, the refresh token secret should not fall back to a weak default in a production environment. This change ensures the application fails safe by refusing to start with an insecure configuration in production.
| if (jwtRefreshTokenSecret === undefined) { | |
| return WEAK_DEFAULTS.JWT_REFRESH_TOKEN_SECRET | |
| } | |
| if (jwtRefreshTokenSecret === undefined) { | |
| if (process.env.NODE_ENV === 'production') { | |
| throw new Error('FATAL: JWT_REFRESH_TOKEN_SECRET must be set in production. The application will not start with an insecure default secret.'); | |
| } | |
| // Fallback to weak default for development | |
| logger.warn('WARNING: JWT_REFRESH_TOKEN_SECRET is not set. Falling back to a weak, insecure default secret. This is for development only and MUST NOT be used in production.'); | |
| return WEAK_DEFAULTS.JWT_REFRESH_TOKEN_SECRET | |
| } |
|
@0xi4o is this still a valid issue? any solution? |
|
@HenryHengZJ The linked issue is nearly a year old and was fixed in PR #4606. The issue wasn't closed though. In addition to that, these changes reintroduce the weak defaults and thus weakens our security posture which we do not want. The only reason we still have a |
Description
When running docker-compose without setting JWT env vars,
initAuthSecrets()may fail to initialize secrets, causingJwtStrategy requires a secret or keyerror on startup.Fix
This PR adds fallback to weak default values (same as WEAK_DEFAULTS) instead of throwing an error, making docker-compose work out of the box.
Changes
getJWTAuthTokenSecret(): ReturnsWEAK_DEFAULTS.JWT_AUTH_TOKEN_SECRETinstead of throwing when undefinedgetJWTRefreshTokenSecret(): ReturnsWEAK_DEFAULTS.JWT_REFRESH_TOKEN_SECRETinstead of throwing when undefinedFixes
Fixes #4563
Testing
This fix was tested by attempting to run
docker compose up -dwithout JWT env vars set, confirming the server starts successfully.