✨(webhooks) add new Webhooks Channel to create messages from API #383
Conversation
WalkthroughThis pull request introduces webhook-based inbound message delivery with header-based channel and API key authentication. Infrastructure changes include disabling a Docker user configuration and registering new webhook routes with corresponding tests. Changes
Sequence Diagram(s)sequenceDiagram
participant ExtClient as External Client
participant Webhook as InboundWebhookViewSet
participant Auth as WebhookAuthentication
participant DB as Database
participant Pipeline as Message Pipeline
ExtClient->>Webhook: POST /inbound/webhook<br/>(X-Channel-ID, X-API-KEY headers)
Webhook->>Auth: authenticate(request)
Auth->>DB: Lookup channel by ID
alt Invalid/Missing Channel
Auth-->>Webhook: AuthenticationFailed
Webhook-->>ExtClient: 401 Unauthorized
else Valid Channel
Auth->>Auth: Validate API key<br/>(constant-time comparison)
alt Invalid API Key
Auth-->>Webhook: AuthenticationFailed
Webhook-->>ExtClient: 401 Unauthorized
else Valid API Key
Auth-->>Webhook: User payload<br/>(channel, auth_method)
Webhook->>Webhook: Extract & validate<br/>sender, email, message, subject
Webhook->>DB: Verify mailbox configured
rect rgb(200, 220, 250)
note right of Webhook: Sanitize headers,<br/>enrich with webhook metadata
end
Webhook->>Pipeline: deliver_inbound_message()
Pipeline-->>Webhook: Success/Failure
Webhook-->>ExtClient: 200 JSON success<br/>or 500 Error
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The webhook implementation introduces new authentication logic and message delivery flow requiring careful validation of email handling, header sanitization, and security considerations (constant-time key comparison, header injection prevention). The diversity across authentication, viewset, routing, and test files adds moderate complexity, though patterns are consistent and well-tested. 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.0)src/backend/core/api/viewsets/inbound/webhook.pysrc/backend/core/tests/api/test_inbound_webhook.pysrc/backend/core/urls.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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
compose.yaml(1 hunks)src/backend/core/api/viewsets/inbound/webhook.py(1 hunks)src/backend/core/tests/api/test_inbound_webhook.py(1 hunks)src/backend/core/urls.py(2 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/core/urls.pysrc/backend/core/tests/api/test_inbound_webhook.pysrc/backend/core/api/viewsets/inbound/webhook.py
src/backend/**/urls.py
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
Define clear, RESTful URL patterns using Django’s URL dispatcher
Files:
src/backend/core/urls.py
src/backend/**/{tests.py,tests/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
src/backend/**/{tests.py,tests/**/*.py}: Use Django’s testing tools (pytest-django) to ensure code quality and reliability
Unit tests should focus on a single use case, keep assertions minimal, and cover all possible cases
Files:
src/backend/core/tests/api/test_inbound_webhook.py
🧠 Learnings (1)
📚 Learning: 2025-09-02T10:12:12.835Z
Learnt from: CR
PR: suitenumerique/messages#0
File: .cursor/rules/django-python.mdc:0-0
Timestamp: 2025-09-02T10:12:12.835Z
Learning: Applies to src/backend/**/views.py : Use Django REST Framework viewsets for API endpoints
Applied to files:
src/backend/core/urls.py
🧬 Code graph analysis (3)
src/backend/core/urls.py (1)
src/backend/core/api/viewsets/inbound/webhook.py (1)
InboundWebhookViewSet(73-210)
src/backend/core/tests/api/test_inbound_webhook.py (2)
src/backend/core/api/viewsets/inbound/webhook.py (3)
WebhookAuthentication(26-70)authenticate(33-50)authenticate_header(68-70)src/backend/core/factories.py (2)
MailboxFactory(60-96)ChannelFactory(249-258)
src/backend/core/api/viewsets/inbound/webhook.py (4)
src/backend/core/api/permissions.py (1)
IsAuthenticated(18-25)src/backend/core/mda/inbound.py (1)
deliver_inbound_message(332-736)src/backend/core/mda/rfc5322/composer.py (1)
compose_email(481-556)src/backend/core/models.py (1)
Channel(418-469)
⏰ 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). (6)
- GitHub Check: docker-publish-socks-proxy / docker-build-push
- GitHub Check: docker-publish-mta-in / docker-build-push
- GitHub Check: build-front
- GitHub Check: check-api-state
- GitHub Check: test-back
- GitHub Check: lint-back
| try: | ||
| channel = models.Channel.objects.get(id=channel_id) | ||
| except models.Channel.DoesNotExist as e: | ||
| raise AuthenticationFailed("Invalid channel ID") from e | ||
|
|
||
| # Get authentication method from channel settings | ||
| auth_method = (channel.settings or {}).get("auth_method", "api_key") | ||
|
|
||
| if auth_method == "api_key": | ||
| return self._authenticate_api_key(request, channel) | ||
|
|
||
| raise AuthenticationFailed(f"Unsupported authentication method: {auth_method}") | ||
|
|
||
| def _authenticate_api_key(self, request, channel): |
There was a problem hiding this comment.
Handle invalid channel IDs without 500s
Line 40 raises ValueError when X-Channel-ID isn’t a valid UUID, causing a 500 instead of an auth failure. Wrap the lookup to catch ValueError alongside DoesNotExist and re-raise AuthenticationFailed.
Apply this diff:
- try:
- channel = models.Channel.objects.get(id=channel_id)
- except models.Channel.DoesNotExist as e:
- raise AuthenticationFailed("Invalid channel ID") from e
+ try:
+ channel = models.Channel.objects.get(id=channel_id)
+ except (ValueError, models.Channel.DoesNotExist) as exc:
+ raise AuthenticationFailed("Invalid channel ID") from exc🤖 Prompt for AI Agents
In src/backend/core/api/viewsets/inbound/webhook.py around lines 39 to 52, the
Channel lookup currently only catches models.Channel.DoesNotExist, but when
X-Channel-ID is not a valid UUID the lookup raises ValueError and returns a 500;
update the exception handling to catch both models.Channel.DoesNotExist and
ValueError and re-raise AuthenticationFailed (preserving the original exception
via "from e") so invalid or malformed channel IDs result in an auth failure
instead of an internal server error.
| def sanitize_header(header: str) -> str: | ||
| return header.replace("\r", "").replace("\n", "")[0:1000] | ||
|
|
||
| # Add webhook-specific headers | ||
| prepend_headers = [("X-StMsg-Sender-Auth", "webhook")] | ||
|
|
||
| # Add source information | ||
| if request.META.get("HTTP_USER_AGENT"): | ||
| prepend_headers.append( | ||
| ( | ||
| "X-StMsg-Webhook-User-Agent", | ||
| sanitize_header(request.META.get("HTTP_USER_AGENT")), | ||
| ) | ||
| ) | ||
|
|
||
| if request.META.get("HTTP_REFERER"): | ||
| prepend_headers.append( | ||
| ( | ||
| "X-StMsg-Webhook-Referer", | ||
| sanitize_header(request.META.get("HTTP_REFERER")), | ||
| ) | ||
| ) | ||
|
|
||
| prepend_headers.append( | ||
| ( | ||
| "Received", | ||
| f"from webhook ({sanitize_header(request.META.get('REMOTE_ADDR'))})", | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Guard sanitize_header against missing values
sanitize_header(request.META.get("REMOTE_ADDR")) blows up when the header is absent (None has no replace), leading to a 500. Default to an empty string (or sentinel) before sanitizing.
Recommend:
- def sanitize_header(header: str) -> str:
- return header.replace("\r", "").replace("\n", "")[0:1000]
+ def sanitize_header(header: str | None) -> str:
+ sanitized = (header or "").replace("\r", "").replace("\n", "")
+ return sanitized[:1000]📝 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.
| def sanitize_header(header: str) -> str: | |
| return header.replace("\r", "").replace("\n", "")[0:1000] | |
| # Add webhook-specific headers | |
| prepend_headers = [("X-StMsg-Sender-Auth", "webhook")] | |
| # Add source information | |
| if request.META.get("HTTP_USER_AGENT"): | |
| prepend_headers.append( | |
| ( | |
| "X-StMsg-Webhook-User-Agent", | |
| sanitize_header(request.META.get("HTTP_USER_AGENT")), | |
| ) | |
| ) | |
| if request.META.get("HTTP_REFERER"): | |
| prepend_headers.append( | |
| ( | |
| "X-StMsg-Webhook-Referer", | |
| sanitize_header(request.META.get("HTTP_REFERER")), | |
| ) | |
| ) | |
| prepend_headers.append( | |
| ( | |
| "Received", | |
| f"from webhook ({sanitize_header(request.META.get('REMOTE_ADDR'))})", | |
| ) | |
| ) | |
| def sanitize_header(header: str | None) -> str: | |
| sanitized = (header or "").replace("\r", "").replace("\n", "") | |
| return sanitized[:1000] | |
| # Add webhook-specific headers | |
| prepend_headers = [("X-StMsg-Sender-Auth", "webhook")] | |
| # Add source information | |
| if request.META.get("HTTP_USER_AGENT"): | |
| prepend_headers.append( | |
| ( | |
| "X-StMsg-Webhook-User-Agent", | |
| sanitize_header(request.META.get("HTTP_USER_AGENT")), | |
| ) | |
| ) | |
| if request.META.get("HTTP_REFERER"): | |
| prepend_headers.append( | |
| ( | |
| "X-StMsg-Webhook-Referer", | |
| sanitize_header(request.META.get("HTTP_REFERER")), | |
| ) | |
| ) | |
| prepend_headers.append( | |
| ( | |
| "Received", | |
| f"from webhook ({sanitize_header(request.META.get('REMOTE_ADDR'))})", | |
| ) | |
| ) |
🤖 Prompt for AI Agents
In src/backend/core/api/viewsets/inbound/webhook.py around lines 145 to 173,
sanitize_header is called with request.META.get(...) values that may be None,
causing AttributeError; change the code to ensure sanitize_header always
receives a string (e.g., pass request.META.get('REMOTE_ADDR') or '' via or ""),
or update sanitize_header to accept Optional[str] and coerce None to '' before
calling replace and slicing; apply the same guarding to all places
sanitize_header is called (USER_AGENT, HTTP_REFERER, REMOTE_ADDR) so missing
headers no longer raise a 500.
1 participant
More generic version of the widget endpoint
Summary by CodeRabbit
New Features
Chores