Fix #4952: Allow custom SMTP test email via _APP_SMTP_TEST_EMAIL #11019
Conversation
📝 WalkthroughWalkthroughThis pull request makes the SMTP test email address configurable through environment variables. A new configuration variable Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/config/variables.phpsrc/Appwrite/Platform/Tasks/Doctor.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: scan
🔇 Additional comments (1)
app/config/variables.php (1)
571-579: LGTM!The configuration variable is properly structured and placed in the correct SMTP category. The description clearly explains its purpose, and the default value matches the usage in
Doctor.php.
| $testEmail = App::getEnv('_APP_SMTP_TEST_EMAIL', 'demo@example.com'); | ||
| $mail->addAddress($testEmail, 'Appwrite Doctor'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the usage pattern of getEnv methods in Doctor.php and check if App::getEnv is used elsewhere in Platform Tasks
# Check all getEnv usage in Doctor.php
echo "=== All getEnv usage in Doctor.php ==="
rg -n "getEnv\(" src/Appwrite/Platform/Tasks/Doctor.php
# Check if App::getEnv is used in other Platform Tasks
echo -e "\n=== App::getEnv usage in other Platform Tasks ==="
rg -n "App::getEnv" --type=php src/Appwrite/Platform/Tasks/
# Check if System::getEnv is the standard pattern
echo -e "\n=== System::getEnv usage count in Platform Tasks ==="
rg "System::getEnv" --type=php src/Appwrite/Platform/Tasks/ | wc -l
# Check the imports at the top of Doctor.php
echo -e "\n=== Imports in Doctor.php ==="
head -30 src/Appwrite/Platform/Tasks/Doctor.php | grep -E "use |App::|System::"Repository: appwrite/appwrite
Length of output: 3224
🏁 Script executed:
#!/bin/bash
# Check the implementations of App::getEnv and System::getEnv
echo "=== Checking Utopia\App class for getEnv method ==="
find . -type f -name "*.php" -path "*/Utopia/App*" | head -5
echo -e "\n=== Searching for App::getEnv definition ==="
rg "function getEnv" --type=php | grep -i "app\|getenv"
echo -e "\n=== Searching for System::getEnv definition ==="
rg "function getEnv" --type=php --type-add 'php:include' -A 3 | head -20
echo -e "\n=== Check if Utopia\App is used elsewhere in Doctor.php ==="
rg "App::" src/Appwrite/Platform/Tasks/Doctor.phpRepository: appwrite/appwrite
Length of output: 731
🏁 Script executed:
#!/bin/bash
# Look for Utopia library in vendor or check if we can find getEnv method definitions
echo "=== Check vendor directory for Utopia ==="
find vendor -type f -name "*.php" -path "*Utopia*" 2>/dev/null | grep -E "(App|System)" | head -10
echo -e "\n=== Search for getEnv in vendor (limited) ==="
rg "public.*function getEnv" vendor/ --type=php 2>/dev/null | head -5
echo -e "\n=== Check composer.json for Utopia version ==="
grep -A 5 "utopia" composer.json 2>/dev/null || grep -i "utopia" composer.json 2>/dev/null | head -5
echo -e "\n=== Look for any documentation or comments about getEnv in Doctor.php context ==="
sed -n '210,225p' src/Appwrite/Platform/Tasks/Doctor.phpRepository: appwrite/appwrite
Length of output: 2733
🏁 Script executed:
#!/bin/bash
# Find Utopia App and System classes with getEnv methods
echo "=== Find Utopia\App class with getEnv ==="
find vendor/utopia-php -name "*.php" -type f | xargs grep -l "class App" | head -5
echo -e "\n=== Find Utopia\System\System class ==="
find vendor/utopia-php/system -name "*.php" -type f | head -10
echo -e "\n=== Check App class in framework ==="
cat vendor/utopia-php/framework/src/App.php | head -100 | grep -A 5 "function getEnv"
echo -e "\n=== Check System class ==="
cat vendor/utopia-php/system/src/System.php | head -150 | grep -A 5 "function getEnv"Repository: appwrite/appwrite
Length of output: 957
🏁 Script executed:
#!/bin/bash
# Read the actual getEnv implementations
echo "=== Utopia\App::getEnv method ==="
grep -A 10 "public.*function getEnv" vendor/utopia-php/framework/src/App.php
echo -e "\n=== Utopia\System\System::getEnv method ==="
grep -A 10 "public.*function getEnv" vendor/utopia-php/system/src/System/System.php
echo -e "\n=== Full context: check if App::getEnv is static or instance method ==="
grep -B 5 "function getEnv" vendor/utopia-php/framework/src/App.php | head -15
echo -e "\n=== Full context: System::getEnv ==="
grep -B 5 "function getEnv" vendor/utopia-php/system/src/System/System.php | head -15Repository: appwrite/appwrite
Length of output: 1142
🏁 Script executed:
#!/bin/bash
# Verify Doctor.php is a CLI task and check the actual runtime context
echo "=== Check Doctor class definition ==="
head -50 src/Appwrite/Platform/Tasks/Doctor.php | grep -E "class|extends|Action"
echo -e "\n=== Check if Platform/Action is a CLI task base class ==="
grep -r "class Action" vendor/utopia-php/platform/src/ | head -3
echo -e "\n=== Check how Platform Tasks are invoked ==="
rg "Doctor" src/ --type=php | grep -E "new Doctor|Doctor::|execute" | head -5
echo -e "\n=== Verify if _APP_SMTP_TEST_EMAIL is an actual environment variable ==="
rg "_APP_SMTP_TEST_EMAIL" . --type=php | grep -v "Doctor.php" | head -5Repository: appwrite/appwrite
Length of output: 581
Replace App::getEnv with System::getEnv on line 217.
Line 217 uses App::getEnv, which reads from the $_SERVER superglobal. However, in CLI context (Platform Tasks), $_SERVER is not reliably populated with environment variables. The System::getEnv method correctly uses PHP's getenv() function, which is the proper way to access environment variables in CLI tasks.
All other uses in Doctor.php (40 instances) correctly use System::getEnv. This inconsistency will cause _APP_SMTP_TEST_EMAIL to always fall back to the default value 'demo@example.com' instead of reading the configured environment variable, breaking SMTP test functionality.
Change:
$testEmail = App::getEnv('_APP_SMTP_TEST_EMAIL', 'demo@example.com');to:
$testEmail = System::getEnv('_APP_SMTP_TEST_EMAIL', 'demo@example.com');🤖 Prompt for AI Agents
In src/Appwrite/Platform/Tasks/Doctor.php around lines 217 to 218, replace the
call to App::getEnv with System::getEnv so the SMTP test email is read from the
actual environment (getenv) in CLI context; update the single line that assigns
$testEmail to call System::getEnv('_APP_SMTP_TEST_EMAIL', 'demo@example.com')
instead of App::getEnv to match the rest of the file and ensure the configured
value is used.
What does this PR do?
This PR allows users to configure a custom test email address for the
doctorcommand using the_APP_SMTP_TEST_EMAILenvironment variable.Why is this necessary?
The current hardcoded
demo@example.comaddress is often rejected by SMTP providers (like SendGrid or AWS SES), causing the doctor command to report a "disconnected" status even when the configuration is correct.Which issue does it fix?
Fixes #4952
Checklist
Doctor.phpto useApp::getEnv.app/config/variables.php.