-
Notifications
You must be signed in to change notification settings - Fork 5
fix(backend): exempt inbound MTA endpoints from SSL redirect #388
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?
fix(backend): exempt inbound MTA endpoints from SSL redirect #388
Conversation
The SECURE_REDIRECT_EXEMPT setting was missing the inbound MTA router endpoints, causing internal HTTP calls from the MTA-in service to be redirected to HTTPS with a 301 response. This caused the milter to timeout when checking recipients and delivering mail, resulting in "451 4.7.1 Service unavailable" errors. The MTA-in service calls: - /api/v1.0/inbound/mta/check/ (for recipient validation) - /api/v1.0/inbound/mta/deliver/ (for mail delivery) These internal endpoints must accept HTTP requests and should not be redirected to HTTPS. Changed: - Added "^api/v1\.0/inbound/mta/" to SECURE_REDIRECT_EXEMPT This follows the existing pattern for the alias endpoints at /api/v1.0/mta/* which were already exempted. Fixes: MTA milter timeout and mail delivery failures
WalkthroughA new exempt path Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Pylint (4.0.1)src/backend/messages/settings.pyThanks 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: 0
🧹 Nitpick comments (1)
src/backend/messages/settings.py (1)
986-986
: LGTM with a suggested refactor for line length compliance.The addition of
"^api/v1\\.0/inbound/mta/"
correctly exempts the inbound MTA endpoints from SSL redirect, addressing the timeout and mail delivery failures described in the PR objectives. The pattern follows the existing convention used for MTA alias endpoints.However, the line exceeds the 100-character limit specified in the coding guidelines.
Consider wrapping the list across multiple lines for better readability and compliance:
- SECURE_REDIRECT_EXEMPT = ["^__lbheartbeat__", "^__heartbeat__", "^api/v1\\.0/mta/", "^api/v1\\.0/inbound/mta/"] + SECURE_REDIRECT_EXEMPT = [ + "^__lbheartbeat__", + "^__heartbeat__", + "^api/v1\\.0/mta/", + "^api/v1\\.0/inbound/mta/", + ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/backend/messages/settings.py
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/backend/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
src/backend/**/*.py
: Follow Django/PEP 8 style with a 100-character line limit
Use descriptive, snake_case names for variables and functions
Use Django ORM for database access; avoid raw SQL unless necessary for performance
Use Django’s built-in user model and authentication framework
Prefer try-except blocks to handle exceptions in business logic and views
Log expected and unexpected actions with appropriate log levels
Capture and report exceptions to Sentry; use capture_exception() for custom errors
Do not log sensitive information (tokens, passwords, financial/health data, PII)
Files:
src/backend/messages/settings.py
src/backend/**/{settings.py,middleware.py}
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
Use middleware judiciously for cross-cutting concerns (authentication, logging, caching)
Files:
src/backend/messages/settings.py
src/backend/**/settings.py
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
src/backend/**/settings.py
: Leverage Django’s caching framework (e.g., Redis/Memcached) where appropriate
Use Django’s cache framework with a backend like Redis or Memcached to reduce DB load
Optimize static file handling using Django’s staticfiles pipeline (e.g., WhiteNoise)
Files:
src/backend/messages/settings.py
⏰ 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). (5)
- GitHub Check: docker-publish-socks-proxy / docker-build-push
- GitHub Check: build-front
- GitHub Check: check-api-state
- GitHub Check: lint-back
- GitHub Check: test-back
🔇 Additional comments (1)
src/backend/messages/settings.py (1)
986-986
: Endpoints verified: both/api/v1.0/inbound/mta/check/
and/api/v1.0/inbound/mta/deliver/
are properly defined.The inbound MTA endpoints are correctly implemented in the codebase:
InboundMTAViewSet
is registered insrc/backend/core/urls.py
(line 84) via the inbound nested router- The
check
action is defined insrc/backend/core/api/viewsets/inbound/mta.py
(line 83)- The
deliver
action is defined insrc/backend/core/api/viewsets/inbound/mta.py
(line 104-107)- The
SECURE_REDIRECT_EXEMPT
regex pattern^api/v1\.0/inbound/mta/
correctly matches both endpoints
The SECURE_REDIRECT_EXEMPT setting was missing the inbound MTA router endpoints, causing internal HTTP calls from the MTA-in service to be redirected to HTTPS with a 301 response.
This caused the milter to timeout when checking recipients and delivering mail, resulting in "451 4.7.1 Service unavailable" errors.
The MTA-in service calls:
These internal endpoints must accept HTTP requests and should not be redirected to HTTPS.
Changed:
This follows the existing pattern for the alias endpoints at /api/v1.0/mta/* which were already exempted.
Fixes: MTA milter timeout and mail delivery failures
Summary by CodeRabbit