-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Changement des var database #735
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
Conversation
Erreur import {config} from './config.js'
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughThe pull request updates the MongoDB connection configuration from a local instance to MongoDB Atlas SRV and revises default application settings in the config file, including appId and masterKey values. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.env.example(1 hunks)config.js(1 hunks)
🔇 Additional comments (1)
config.js (1)
8-8: LGTM: AppId default updated appropriately.The change to use
'App-gym'as the default appId is acceptable and more descriptive than the generic'myAppId'. The environment variable pattern is correctly maintained.
| APP_ID=myAppId | ||
| MASTER_KEY=myMasterKey | ||
| MONGODB_URI=mongodb://localhost:27017/parse | ||
| MONGODB_URI=mongodb+srv://odensutibun_db_user:<db_password>@gymproject.d2bedmi.mongodb.net/?appName=GymProject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This environment variable is no longer used by the application.
The MONGODB_URI environment variable defined here is ignored because config.js now uses a hardcoded database URI instead of reading from process.env.MONGODB_URI. This makes the .env.example misleading.
This should be addressed by fixing the critical issue in config.js (see comments on that file).
🤖 Prompt for AI Agents
In .env.example around line 3 the MONGODB_URI entry is misleading because
config.js currently ignores process.env.MONGODB_URI and uses a hardcoded URI;
fix by updating config.js to read the Mongo URL from process.env.MONGODB_URI
(with the current hardcoded URI as a safe fallback) so the .env variable is
actually used and keep the .env.example entry, or alternatively remove the entry
here if you intentionally want a hardcoded value—prefer the former (read env var
with fallback) and ensure any tests/startup use the env variable.
| var databaseURI = 'mongodb+srv://odensutibun_db_user:<db_password>@gymproject.d2bedmi.mongodb.net/?appName=GymProject'; | ||
|
|
||
| export const config = { | ||
| databaseURI: | ||
| process.env.DATABASE_URI || process.env.MONGODB_URI || 'mongodb://localhost:27017/dev', | ||
| databaseURI: databaseURI, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Remove hardcoded database credentials from source code.
Hardcoding the database URI directly in the source code is a serious security vulnerability:
- The database username (
odensutibun_db_user) and cluster endpoint are exposed in version control - This defeats the purpose of using environment variables for configuration
- The
MONGODB_URIenvironment variable defined in.env.exampleis now completely ignored - This violates 12-factor app configuration principles
Even with a placeholder password, exposing the username and cluster details makes your database more vulnerable to attacks.
Apply this diff to restore environment variable usage:
-var databaseURI = 'mongodb+srv://odensutibun_db_user:<db_password>@gymproject.d2bedmi.mongodb.net/?appName=GymProject';
-
export const config = {
- databaseURI: databaseURI,
+ databaseURI: process.env.MONGODB_URI || 'mongodb://localhost:27017/parse',
cloud: () => import('./cloud/main.js'),This allows you to:
- Keep credentials out of version control
- Use the
MONGODB_URIenvironment variable from.env.example - Maintain a sensible local development fallback
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var databaseURI = 'mongodb+srv://odensutibun_db_user:<db_password>@gymproject.d2bedmi.mongodb.net/?appName=GymProject'; | |
| export const config = { | |
| databaseURI: | |
| process.env.DATABASE_URI || process.env.MONGODB_URI || 'mongodb://localhost:27017/dev', | |
| databaseURI: databaseURI, | |
| export const config = { | |
| databaseURI: process.env.MONGODB_URI || 'mongodb://localhost:27017/parse', |
🤖 Prompt for AI Agents
In config.js around lines 3–6, remove the hardcoded MongoDB connection string
and replace it with logic that reads the connection URI from
process.env.MONGODB_URI (with a sensible local fallback for development) and
export that value; ensure no credentials remain in source, update .env.example
to document MONGODB_URI, and fail fast or log a clear error if neither env var
nor fallback is available so credentials are never committed.
| appId: process.env.APP_ID || 'myAppId', | ||
| masterKey: process.env.MASTER_KEY || '', //Add your master key here. Keep it secret! | ||
| appId: process.env.APP_ID || 'App-gym', | ||
| masterKey: process.env.MASTER_KEY || 'master_root', //Add your master key here. Keep it secret! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Major security risk: Weak default master key exposes the application.
Setting a hardcoded default master key ('master_root') is a significant security vulnerability:
- If the
MASTER_KEYenvironment variable is not set, the application will use this publicly-known weak key - The master key grants full administrative access to your Parse Server
- The previous empty string default was safer as it would cause configuration errors, forcing proper setup
Apply this diff to remove the weak default:
- masterKey: process.env.MASTER_KEY || 'master_root', //Add your master key here. Keep it secret!
+ masterKey: process.env.MASTER_KEY || '', //Add your master key here. Keep it secret!Alternatively, make the master key required and fail early if not provided:
masterKey: process.env.MASTER_KEY, // Required - must be set in environment🤖 Prompt for AI Agents
config.js around line 9: the current line provides a hardcoded weak default
masterKey ('master_root'), which is a major security risk; change the assignment
to use only process.env.MASTER_KEY (remove the default) and add an early runtime
validation immediately after loading config that checks if
process.env.MASTER_KEY is set, and if not logs a clear error and exits or throws
so the process fails to start; update any comments to indicate the key is
required and document setting MASTER_KEY in deployment.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.