-
Notifications
You must be signed in to change notification settings - Fork 24
feat: medusa cloud #53
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughA new environment variable, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MedusaApp
participant EnvFile
participant Database
User->>EnvFile: Sets DATABASE_SSL_DISABLED in .env
MedusaApp->>EnvFile: Reads DATABASE_SSL_DISABLED
MedusaApp->>Database: Configures connection (SSL enabled/disabled based on env var)
Possibly related PRs
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
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: 1
🔭 Outside diff range comments (1)
apps/medusa/medusa-config.ts (1)
39-87
: 💡 Verification agent🧩 Analysis chain
Verify the usage of DATABASE_SSL_DISABLED variable.
While you've correctly defined the
DATABASE_SSL_DISABLED
variable, I don't see where it's actually used in the configuration to control SSL settings. There should be a configuration property in the database section that uses this variable.
🏁 Script executed:
#!/bin/bash # Search for previous occurrences of SSL configuration git show HEAD~1:apps/medusa/medusa-config.ts | grep -A 10 -B 10 "ssl:" # Check if there are any other files that might use database configuration find apps/medusa -type f -name "*.ts" -o -name "*.js" | xargs grep -l "databaseDriverOptions\|ssl:" | head -n 5Length of output: 745
Use DATABASE_SSL_DISABLED to configure database SSL
The
DATABASE_SSL_DISABLED
variable is defined but not applied in your database settings. Please updateapps/medusa/medusa-config.ts
to include adatabaseDriverOptions.ssl
property underprojectConfig
, for example:• File: apps/medusa/medusa-config.ts
Location: within theprojectConfig
objectSuggested diff:
projectConfig: { databaseUrl: process.env.DATABASE_URL, + databaseDriverOptions: { + ssl: process.env.DATABASE_SSL_DISABLED !== 'true', + }, redisUrl: REDIS_URL, redisPrefix: process.env.REDIS_PREFIX, http: { /* … */ }, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/medusa/.env.template
(1 hunks)apps/medusa/medusa-config.ts
(1 hunks)
🔇 Additional comments (1)
apps/medusa/medusa-config.ts (1)
8-8
: Good addition of environment variable parsing.Using the environment variable to control SSL configuration provides flexibility across different environments.
@@ -13,3 +13,5 @@ ADMIN_BACKEND_URL=http://localhost:9000 | |||
|
|||
# Add your own Stripe secret key here | |||
STRIPE_API_KEY= | |||
|
|||
DATABASE_SSL_DISABLED=true |
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.
💡 Verification agent
❓ Verification inconclusive
Carefully consider this default for production environments.
Adding the DATABASE_SSL_DISABLED=true
variable is a good way to make SSL configuration explicit and customizable. However, setting the default to disable SSL in the template file could lead to insecure configurations if this template is used as a starting point for production environments.
Consider either:
- Setting the default to
false
to prioritize security in production - Adding a comment that clearly indicates this setting should be changed for production environments
🏁 Script executed:
#!/bin/bash
# Check if there are any security guidelines or documentation about database connections
find . -type f -name "*.md" | xargs grep -l "database security\|SSL connection\|production configuration" | head -n 5
Length of output: 119
Enforce SSL by default for production
Disabling SSL by default in your template risks insecure DB connections if this file is copied into prod. Please update apps/medusa/.env.template
at line 17:
- File:
apps/medusa/.env.template
- Line: 17
Suggested change (pick one):
- DATABASE_SSL_DISABLED=true
+ # For production, leave SSL enabled. Set this to `true` only in local/dev.
+ DATABASE_SSL_DISABLED=false
Or, if you prefer to keep the default as true
for dev, add a clear comment above it:
+ # ⚠️ In production ensure this is set to `false` to enforce SSL
DATABASE_SSL_DISABLED=true
📝 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.
DATABASE_SSL_DISABLED=true | |
# For production, leave SSL enabled. Set this to `true` only in local/dev. | |
DATABASE_SSL_DISABLED=false |
Summary by CodeRabbit