feat: implement custom tracking domain CNAME integration (#485) - #673
feat: implement custom tracking domain CNAME integration (#485)#673Bheemeswari497 wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughChangesCustom tracking domains are added to organizations with DNS CNAME validation. Matching hosts are authorized and routed through restricted tracking middleware, while campaign email links and unsubscribe URLs use the organization-specific domain when available. Custom tracking domain
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant DynamicAllowedHosts
participant CustomDomainMiddleware
participant Organization
participant TrackingEndpoint
Client->>DynamicAllowedHosts: Check request host
DynamicAllowedHosts->>Organization: Find custom_tracking_domain
Organization-->>DynamicAllowedHosts: Host authorization
Client->>CustomDomainMiddleware: Request with custom Host header
CustomDomainMiddleware->>Organization: Resolve matching tenant
Organization-->>CustomDomainMiddleware: Matching organization
CustomDomainMiddleware->>TrackingEndpoint: Allow tracking endpoint
TrackingEndpoint-->>Client: Tracking response
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
backend/backend/middleware.py (2)
52-52: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueUnsanitized request-derived values in log message.
host/request.pathare interpolated directly into the warning log; flagged by static analysis as a log-forging risk (CWE-117). Actual exploitability is low here sincehostalready passed Django's host validation, but stripping newlines fromrequest.pathbefore logging is a cheap defensive measure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/backend/middleware.py` at line 52, Sanitize the request-derived request.path value before the logger.warning call in the custom-domain access handling, removing newline characters to prevent log forging; preserve the existing host and path context and warning behavior.Source: Linters/SAST tools
35-36: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider caching the custom-domain → organization lookup.
Every request whose host doesn't match
default_host/127.0.0.1/localhosttriggers a DB query, including from bots/scanners probing arbitrary hosts. The unique index oncustom_tracking_domainkeeps this cheap, but a short-TTL cache (e.g. Django's cache framework) would remove the DB round trip from this hot path entirely, which matters more as tracking-link click volume grows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/backend/middleware.py` around lines 35 - 36, The custom-domain lookup in the middleware currently queries the database on every non-default host request. Update the host-resolution flow around Organization.objects.get to use Django’s cache framework with a short TTL, caching both successful organization resolutions and missing domains, while preserving the existing default-host and localhost behavior.backend/campaigns/tasks.py (1)
526-528: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent "is this localhost" detection vs.
tenants/models.py.Here it's a substring check (
'localhost' in organization.custom_tracking_domain), whileOrganization.clean()usesself.custom_tracking_domain.startswith('localhost'). Both work for realistic inputs, but the divergence is easy to lose track of if either check is ever tightened. Consider extracting a shared helper (e.g.is_local_tracking_domain(domain)) used by both files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/campaigns/tasks.py` around lines 526 - 528, Unify localhost detection for the tracking domain by introducing a shared helper such as is_local_tracking_domain(domain). Update the URL construction logic near organization.custom_tracking_domain and Organization.clean() to use this helper instead of their separate substring and startswith checks, preserving the existing DEBUG-gated HTTP behavior.backend/tenants/models.py (1)
43-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueException handling nits flagged by Ruff.
raise ValidationError(...)inside theexcept dns.resolver.NXDOMAIN/NoAnswer/Timeoutblocks (lines 44, 46, 48) and the catch-all (line 52) drop the original traceback context; and line 51 catches a blindException. Chain withfromto preserve debuggability, and consider{e!s}per RUF010.♻️ Proposed fix
except dns.resolver.NXDOMAIN: - raise ValidationError({'custom_tracking_domain': 'Domain does not exist.'}) + raise ValidationError({'custom_tracking_domain': 'Domain does not exist.'}) from None except dns.resolver.NoAnswer: - raise ValidationError({'custom_tracking_domain': 'No CNAME record found for this domain.'}) + raise ValidationError({'custom_tracking_domain': 'No CNAME record found for this domain.'}) from None except dns.resolver.Timeout: - raise ValidationError({'custom_tracking_domain': 'DNS query timed out.'}) + raise ValidationError({'custom_tracking_domain': 'DNS query timed out.'}) from None except ValidationError: raise except Exception as e: - raise ValidationError({'custom_tracking_domain': f'DNS validation failed: {str(e)}'}) + raise ValidationError({'custom_tracking_domain': f'DNS validation failed: {e!s}'}) from e🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tenants/models.py` around lines 43 - 52, Update the DNS exception handlers in the model’s validation method: chain each newly raised ValidationError from the caught exception, including NXDOMAIN, NoAnswer, Timeout, and the catch-all handler, and format the catch-all message with the exception’s string conversion per Ruff guidance. Preserve the existing ValidationError re-raise behavior and messages.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/backend/middleware.py`:
- Around line 24-39: Update the ALLOWED_HOSTS configuration in backend.settings
to remove the wildcard and include only the canonical application host and
approved tenant custom-tracking domains. Preserve support for the default host
and localhost behavior used by process_request, and source tenant domains from
the existing approved configuration rather than allowing arbitrary Host headers.
In `@backend/campaigns/tasks.py`:
- Line 604: Update build_unsubscribe_url in gmail_service.py to accept the
organization keyword argument used by both email branches, while preserving
existing behavior for callers that provide only lead. Ensure the function
handles the organization value consistently when constructing the unsubscribe
URL.
In `@backend/tenants/models.py`:
- Around line 35-39: Update the CNAME target comparison in the answer-validation
loop to be case-insensitive, while continuing to strip the trailing dot from
rdata.target before comparison with target_domain. Preserve the existing
valid=True and break behavior when the normalized values match.
- Around line 27-32: Update the DNS lookup in the model’s clean validation flow
to pass explicit timeout and lifetime values to dns.resolver.resolve for
self.custom_tracking_domain. Keep the existing CNAME resolution and surrounding
URL parsing behavior unchanged, using bounded values that prevent validation
from waiting on resolver defaults.
---
Nitpick comments:
In `@backend/backend/middleware.py`:
- Line 52: Sanitize the request-derived request.path value before the
logger.warning call in the custom-domain access handling, removing newline
characters to prevent log forging; preserve the existing host and path context
and warning behavior.
- Around line 35-36: The custom-domain lookup in the middleware currently
queries the database on every non-default host request. Update the
host-resolution flow around Organization.objects.get to use Django’s cache
framework with a short TTL, caching both successful organization resolutions and
missing domains, while preserving the existing default-host and localhost
behavior.
In `@backend/campaigns/tasks.py`:
- Around line 526-528: Unify localhost detection for the tracking domain by
introducing a shared helper such as is_local_tracking_domain(domain). Update the
URL construction logic near organization.custom_tracking_domain and
Organization.clean() to use this helper instead of their separate substring and
startswith checks, preserving the existing DEBUG-gated HTTP behavior.
In `@backend/tenants/models.py`:
- Around line 43-52: Update the DNS exception handlers in the model’s validation
method: chain each newly raised ValidationError from the caught exception,
including NXDOMAIN, NoAnswer, Timeout, and the catch-all handler, and format the
catch-all message with the exception’s string conversion per Ruff guidance.
Preserve the existing ValidationError re-raise behavior and messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d75e4bb7-b511-47d3-b69b-f7417b50447c
📒 Files selected for processing (5)
backend/backend/middleware.pybackend/campaigns/tasks.pybackend/campaigns/test_custom_domains.pybackend/tenants/migrations/0003_organization_custom_tracking_domain.pybackend/tenants/models.py
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/backend/settings.py (1)
105-118: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winMIDDLEWARE order lets
CustomDomainMiddlewarebypass rate limiting and security headers.
RateLimitMiddlewareandSecurityHeadersMiddleware(lines 114-115) sit afterCustomDomainMiddleware(line 113) in the list, making them "inner" layers in Django's middleware chain. WheneverCustomDomainMiddleware.process_requestshort-circuits — raisingDisallowedHostfor an unmatched host, or returningHttpResponseNotFoundfor a non-tracking path on a matched custom domain (seebackend/backend/middleware.pylines 41/48/56/71) —get_response()is never called, so neitherRateLimitMiddleware's request-side throttling norSecurityHeadersMiddleware's response headers ever run for that request. This means unauthenticated probing of custom-domain hosts/paths is completely unthrottled, and blocked responses ship without your standard security headers.Reordering so
RateLimitMiddleware/SecurityHeadersMiddlewarerun beforeCustomDomainMiddlewarewould let them wrap around whatever response it produces. Note this needs to be reconciled with theALLOWED_HOSTS=['*']fix above (that fix wants host validation to run as early as possible so no other middleware trusts an unvalidated Host) — consider splitting cheap host validation (very first) from the heavier org-resolution/tenant/path-restriction logic (after rate limiting/security headers).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/backend/settings.py` around lines 105 - 118, Update the MIDDLEWARE ordering so cheap Host validation remains first, while RateLimitMiddleware and SecurityHeadersMiddleware wrap CustomDomainMiddleware and therefore apply to its short-circuit responses. Split host validation from CustomDomainMiddleware’s heavier tenant/domain/path-resolution logic if necessary, preserving the existing tenant isolation behavior and ensuring unvalidated Host values are rejected before other middleware trusts them.
🧹 Nitpick comments (3)
backend/backend/middleware.py (2)
44-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd exception chaining on re-raised
DisallowedHost.Static analysis flags both
exceptblocks: re-raising withoutfrom e/from Noneobscures the original cause in tracebacks/logs when debugging host-header issues in production.🐛 Suggested fix
try: org = Organization.objects.get(id=org_id) except Organization.DoesNotExist: cache.delete(cache_key) - raise DisallowedHost(f"Invalid HTTP_HOST header: {host}") + raise DisallowedHost(f"Invalid HTTP_HOST header: {host}") from None else: try: org = Organization.objects.get(custom_tracking_domain=host) cache.set(cache_key, org.id, 300) except Organization.DoesNotExist: cache.set(cache_key, "MISSING", 300) # If no matching organization exists, block the request - raise DisallowedHost(f"Invalid HTTP_HOST header: {host}") + raise DisallowedHost(f"Invalid HTTP_HOST header: {host}") from None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/backend/middleware.py` around lines 44 - 56, Update both Organization.DoesNotExist handlers in the middleware host-resolution flow to explicitly chain the raised DisallowedHost exception using the appropriate caught-exception or suppressed-context form. Preserve the existing cache invalidation, MISSING caching, and rejection behavior.Source: Linters/SAST tools
43-56: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant org lookup duplicates
settings.py'sDynamicAllowedHostscheck.
settings.py'sDynamicAllowedHosts.__contains__(once actually invoked — see the critical issue flagged in that file) already performs a cached existence check againstOrganization.custom_tracking_domainfor the exact same host beforeget_host()returns. This middleware then performs a second, independent cache lookup +Organization.objects.get(...)for the same host, doubling cache/DB round-trips per custom-domain request. Consider consolidating into a single shared lookup/cache (e.g. a helper that returns theOrganizationobject itself, reused by both the host-validation layer and this middleware).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/backend/middleware.py` around lines 43 - 56, Consolidate the custom-domain organization lookup shared by DynamicAllowedHosts.__contains__ and the middleware’s host-resolution path so each request performs one cached lookup. Introduce or reuse a helper that returns the Organization object (or a missing result) and use it in both validation and middleware, preserving the existing invalid-host rejection and cache behavior.backend/tenants/utils.py (1)
3-10: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winLoosen substring match on
'localhost'.
domain.startswith('localhost') or 'localhost' in domainmatches any domain merely containing the substring (e.g.notlocalhost.example.com), not just actual local hosts. Combined with theDEBUGgate, this could unexpectedly bypass CNAME validation (inOrganization.clean()) or forcehttpscheme (inbuild_unsubscribe_url/rewrite_email_links) for a legitimately-named customer domain in a dev/staging environment whereDEBUG=True.♻️ Suggested tightening
def is_local_tracking_domain(domain): """ Returns True if the domain should be treated as a local tracking domain. """ if not getattr(settings, 'DEBUG', False): return False domain = domain.lower() - return domain.startswith('localhost') or 'localhost' in domain + return domain in ('localhost', '127.0.0.1', '[::1]') or domain.startswith('localhost:')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tenants/utils.py` around lines 3 - 10, Update is_local_tracking_domain to recognize only actual localhost domains, removing the broad substring match that accepts values such as notlocalhost.example.com. Preserve the DEBUG gate and ensure valid localhost host forms, including an optional port if supported by existing callers, continue returning True while unrelated customer domains return False.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/backend/settings.py`:
- Around line 68-80: Update the custom-host resolution try/except block to
replace the broad silent Exception handler with targeted exception handling for
the expected cache and database failures, and log each handled failure with the
relevant host and exception details before returning False. Preserve the
existing successful cache lookup, database query, and fail-closed behavior;
avoid a bare broad catch so Ruff BLE001 is satisfied.
- Around line 59-82: Fix DynamicAllowedHosts so Django host validation actually
consults the dynamic allowlist: either configure ALLOWED_HOSTS as ['*'] and
ensure the first-position middleware performs equivalent validation, or
implement iteration over the current allowed hosts so validate_host() can match
them. Preserve the existing localhost, canonical BACKEND_BASE_URL, and cached
Organization.custom_tracking_domain checks.
In `@backend/campaigns/gmail_service.py`:
- Around line 164-178: Update build_unsubscribe_url to access BACKEND_BASE_URL
with the same fallback used by rewrite_email_links: default to
http://127.0.0.1:8000 when the setting is absent, then continue stripping the
trailing slash before constructing the URL.
---
Outside diff comments:
In `@backend/backend/settings.py`:
- Around line 105-118: Update the MIDDLEWARE ordering so cheap Host validation
remains first, while RateLimitMiddleware and SecurityHeadersMiddleware wrap
CustomDomainMiddleware and therefore apply to its short-circuit responses. Split
host validation from CustomDomainMiddleware’s heavier
tenant/domain/path-resolution logic if necessary, preserving the existing tenant
isolation behavior and ensuring unvalidated Host values are rejected before
other middleware trusts them.
---
Nitpick comments:
In `@backend/backend/middleware.py`:
- Around line 44-56: Update both Organization.DoesNotExist handlers in the
middleware host-resolution flow to explicitly chain the raised DisallowedHost
exception using the appropriate caught-exception or suppressed-context form.
Preserve the existing cache invalidation, MISSING caching, and rejection
behavior.
- Around line 43-56: Consolidate the custom-domain organization lookup shared by
DynamicAllowedHosts.__contains__ and the middleware’s host-resolution path so
each request performs one cached lookup. Introduce or reuse a helper that
returns the Organization object (or a missing result) and use it in both
validation and middleware, preserving the existing invalid-host rejection and
cache behavior.
In `@backend/tenants/utils.py`:
- Around line 3-10: Update is_local_tracking_domain to recognize only actual
localhost domains, removing the broad substring match that accepts values such
as notlocalhost.example.com. Preserve the DEBUG gate and ensure valid localhost
host forms, including an optional port if supported by existing callers,
continue returning True while unrelated customer domains return False.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 34adaefd-a99f-42a7-8881-1b64a27e841a
⛔ Files ignored due to path filters (41)
backend/backend/__pycache__/__init__.cpython-314.pycis excluded by!**/*.pycbackend/backend/__pycache__/celery.cpython-314.pycis excluded by!**/*.pycbackend/backend/__pycache__/settings.cpython-314.pycis excluded by!**/*.pycbackend/backend/__pycache__/urls.cpython-314.pycis excluded by!**/*.pycbackend/campaigns/__pycache__/__init__.cpython-314.pycis excluded by!**/*.pycbackend/campaigns/__pycache__/ai.cpython-314.pycis excluded by!**/*.pycbackend/campaigns/__pycache__/apps.cpython-314.pycis excluded by!**/*.pycbackend/campaigns/__pycache__/gmail_service.cpython-314.pycis excluded by!**/*.pycbackend/campaigns/__pycache__/google_auth_views.cpython-314.pycis excluded by!**/*.pycbackend/campaigns/__pycache__/models.cpython-314.pycis excluded by!**/*.pycbackend/campaigns/__pycache__/serializers.cpython-314.pycis excluded by!**/*.pycbackend/campaigns/__pycache__/tasks.cpython-314.pycis excluded by!**/*.pycbackend/campaigns/__pycache__/views.cpython-314.pycis excluded by!**/*.pycbackend/campaigns/migrations/__pycache__/0001_initial.cpython-314.pycis excluded by!**/*.pycbackend/campaigns/migrations/__pycache__/0002_campaignlead_last_sent_message_id_and_more.cpython-314.pycis excluded by!**/*.pycbackend/campaigns/migrations/__pycache__/0003_alter_sequencestep_channel_type.cpython-314.pycis excluded by!**/*.pycbackend/campaigns/migrations/__pycache__/0004_connectedemailaccount_connected_by.cpython-314.pycis excluded by!**/*.pycbackend/campaigns/migrations/__pycache__/__init__.cpython-314.pycis excluded by!**/*.pycbackend/leads/__pycache__/__init__.cpython-314.pycis excluded by!**/*.pycbackend/leads/__pycache__/apps.cpython-314.pycis excluded by!**/*.pycbackend/leads/__pycache__/models.cpython-314.pycis excluded by!**/*.pycbackend/leads/__pycache__/serializers.cpython-314.pycis excluded by!**/*.pycbackend/leads/__pycache__/views.cpython-314.pycis excluded by!**/*.pycbackend/leads/migrations/__pycache__/0001_initial.cpython-314.pycis excluded by!**/*.pycbackend/leads/migrations/__pycache__/__init__.cpython-314.pycis excluded by!**/*.pycbackend/tenants/__pycache__/__init__.cpython-314.pycis excluded by!**/*.pycbackend/tenants/__pycache__/admin.cpython-314.pycis excluded by!**/*.pycbackend/tenants/__pycache__/apps.cpython-314.pycis excluded by!**/*.pycbackend/tenants/__pycache__/middleware.cpython-314.pycis excluded by!**/*.pycbackend/tenants/__pycache__/models.cpython-314.pycis excluded by!**/*.pycbackend/tenants/__pycache__/security.cpython-314.pycis excluded by!**/*.pycbackend/tenants/migrations/__pycache__/0001_initial.cpython-314.pycis excluded by!**/*.pycbackend/tenants/migrations/__pycache__/__init__.cpython-314.pycis excluded by!**/*.pycbackend/users/__pycache__/__init__.cpython-314.pycis excluded by!**/*.pycbackend/users/__pycache__/apps.cpython-314.pycis excluded by!**/*.pycbackend/users/__pycache__/jwt.cpython-314.pycis excluded by!**/*.pycbackend/users/__pycache__/models.cpython-314.pycis excluded by!**/*.pycbackend/users/__pycache__/serializers.cpython-314.pycis excluded by!**/*.pycbackend/users/__pycache__/views.cpython-314.pycis excluded by!**/*.pycbackend/users/migrations/__pycache__/0001_initial.cpython-314.pycis excluded by!**/*.pycbackend/users/migrations/__pycache__/__init__.cpython-314.pycis excluded by!**/*.pyc
📒 Files selected for processing (7)
backend/backend/middleware.pybackend/backend/settings.pybackend/campaigns/gmail_service.pybackend/campaigns/tasks.pybackend/campaigns/test_custom_domains.pybackend/tenants/models.pybackend/tenants/utils.py
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/tenants/models.py
- backend/campaigns/test_custom_domains.py
| class DynamicAllowedHosts(list): | ||
| def __contains__(self, host): | ||
| from urllib.parse import urlparse | ||
| import os | ||
|
|
||
| canonical_host = urlparse(os.getenv('BACKEND_BASE_URL', 'https://leadorbit.onrender.com')).hostname or 'leadorbit.onrender.com' | ||
| if host in ['localhost', '127.0.0.1', '[::1]', canonical_host]: | ||
| return True | ||
|
|
||
| try: | ||
| from django.core.cache import cache | ||
| cache_key = f"allowed_host_{host}" | ||
| is_allowed = cache.get(cache_key) | ||
| if is_allowed is not None: | ||
| return is_allowed | ||
|
|
||
| from tenants.models import Organization | ||
| is_allowed = Organization.objects.filter(custom_tracking_domain=host).exists() | ||
| cache.set(cache_key, is_allowed, 300) | ||
| return is_allowed | ||
| except Exception: | ||
| return False | ||
|
|
||
| ALLOWED_HOSTS = DynamicAllowedHosts() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backend/backend/settings.py =="
nl -ba backend/backend/settings.py | sed -n '50,130p'
echo
echo "== search for ALLOWED_HOSTS overrides and get_host usage =="
rg -n "override_settings\(ALLOWED_HOSTS|request\.get_host\(|ALLOWED_HOSTS\s*=" backend -g '!**/node_modules/**' || true
echo
echo "== Django host validation implementation (installed package, if available) =="
python3 - <<'PY'
import inspect
try:
import django
from django.http.request import HttpRequest, validate_host
print("django_version:", django.get_version())
print("\nvalidate_host source:")
print(inspect.getsource(validate_host))
print("\nHttpRequest.get_host source:")
print(inspect.getsource(HttpRequest.get_host))
except Exception as e:
print("ERROR:", type(e).__name__, e)
PYRepository: Kuldeeep18/LeadOrbit
Length of output: 2029
🌐 Web query:
Django 5.0 validate_host HttpRequest.get_host source any(pattern == '*' or is_same_domain(host, pattern) for pattern in allowed_hosts)
💡 Result:
In Django 5.0, the validate_host function is used by HttpRequest.get_host to ensure that the request's Host header is safe, preventing HTTP Host header attacks [1][2]. The validation logic is defined in django.http.request as follows [3][4]: return any( pattern == "" or is_same_domain(host, pattern) for pattern in allowed_hosts) When ALLOWED_HOSTS contains the wildcard '', the validation process short-circuits to return True for any provided host header [1][4]. This effectively disables Django's built-in host validation [1][5]. If you use '', you are responsible for implementing your own security validation for the Host header, typically by placing custom middleware at the beginning of the MIDDLEWARE setting [1][5]. When the list does not contain '', Django checks the host against each pattern in ALLOWED_HOSTS using is_same_domain [2][4][6]. Patterns starting with a period (e.g., '.example.com') match the domain and all its subdomains, while other patterns must match exactly [1][6][5]. Note that this security check is only applied when using HttpRequest.get_host [1][5]. If your application accesses the Host header directly from request.META, this protection is bypassed [1][5].
Citations:
- 1: https://django.readthedocs.io/en/5.0.x/ref/settings.html
- 2: https://github.com/django/django/blob/stable/5.1.x/django/http/request.py
- 3: https://docs.djangoproject.com/en/5.0/_modules/django/http/request/
- 4: https://github.com/django/django/blob/main/django/http/request.py
- 5: https://github.com/django/django/blob/main/docs/ref/settings.txt
- 6: https://github.com/django/django/blob/stable/6.0.x/django/utils/http.py
Critical: DynamicAllowedHosts never participates in Django host validation
Django iterates ALLOWED_HOSTS in validate_host(), so overriding __contains__ here has no effect. Since ALLOWED_HOSTS = DynamicAllowedHosts() starts empty, request.get_host() will reject every host and break requests as soon as middleware calls it. Use ALLOWED_HOSTS = ['*'] with first-position middleware validation, or make this object actually iterable over the allowed hosts.
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 79-79: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/backend/settings.py` around lines 59 - 82, Fix DynamicAllowedHosts so
Django host validation actually consults the dynamic allowlist: either configure
ALLOWED_HOSTS as ['*'] and ensure the first-position middleware performs
equivalent validation, or implement iteration over the current allowed hosts so
validate_host() can match them. Preserve the existing localhost, canonical
BACKEND_BASE_URL, and cached Organization.custom_tracking_domain checks.
| try: | ||
| from django.core.cache import cache | ||
| cache_key = f"allowed_host_{host}" | ||
| is_allowed = cache.get(cache_key) | ||
| if is_allowed is not None: | ||
| return is_allowed | ||
|
|
||
| from tenants.models import Organization | ||
| is_allowed = Organization.objects.filter(custom_tracking_domain=host).exists() | ||
| cache.set(cache_key, is_allowed, 300) | ||
| return is_allowed | ||
| except Exception: | ||
| return False |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Blind except Exception fails closed silently, with no logging.
Any transient cache/DB error (connection pool exhaustion, cache backend blip) causes is_allowed resolution to silently return False for every non-canonical host, with no log trace to diagnose why custom-domain traffic suddenly stopped resolving. Ruff also flags this (BLE001).
🩹 Suggested fix
- except Exception:
- return False
+ except Exception:
+ logger.exception("Failed to validate dynamic host %r", host)
+ return False📝 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.
| try: | |
| from django.core.cache import cache | |
| cache_key = f"allowed_host_{host}" | |
| is_allowed = cache.get(cache_key) | |
| if is_allowed is not None: | |
| return is_allowed | |
| from tenants.models import Organization | |
| is_allowed = Organization.objects.filter(custom_tracking_domain=host).exists() | |
| cache.set(cache_key, is_allowed, 300) | |
| return is_allowed | |
| except Exception: | |
| return False | |
| try: | |
| from django.core.cache import cache | |
| cache_key = f"allowed_host_{host}" | |
| is_allowed = cache.get(cache_key) | |
| if is_allowed is not None: | |
| return is_allowed | |
| from tenants.models import Organization | |
| is_allowed = Organization.objects.filter(custom_tracking_domain=host).exists() | |
| cache.set(cache_key, is_allowed, 300) | |
| return is_allowed | |
| except Exception: | |
| logger.exception("Failed to validate dynamic host %r", host) | |
| return False |
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 79-79: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/backend/settings.py` around lines 68 - 80, Update the custom-host
resolution try/except block to replace the broad silent Exception handler with
targeted exception handling for the expected cache and database failures, and
log each handled failure with the relevant host and exception details before
returning False. Preserve the existing successful cache lookup, database query,
and fail-closed behavior; avoid a bare broad catch so Ruff BLE001 is satisfied.
Source: Linters/SAST tools
| def build_unsubscribe_url(lead, organization=None): | ||
| """ | ||
| Build a signed unsubscribe URL for a lead using the backend base URL. | ||
| Build a signed unsubscribe URL for a lead using the backend base URL or custom tracking domain. | ||
| """ | ||
| from .utils import generate_unsubscribe_token | ||
| from django.conf import settings | ||
|
|
||
| token = generate_unsubscribe_token(lead.id) | ||
| return f"{settings.BACKEND_BASE_URL}/api/v1/unsubscribe/{lead.id}/{token}/" | ||
| if organization and organization.custom_tracking_domain: | ||
| from tenants.utils import is_local_tracking_domain | ||
| scheme = 'http' if is_local_tracking_domain(organization.custom_tracking_domain) else 'https' | ||
| base_url = f"{scheme}://{organization.custom_tracking_domain}" | ||
| else: | ||
| base_url = settings.BACKEND_BASE_URL.rstrip('/') | ||
| return f"{base_url}/api/v1/unsubscribe/{lead.id}/{token}/" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Inconsistent BACKEND_BASE_URL access vs. sibling rewrite_email_links.
This falls back to settings.BACKEND_BASE_URL.rstrip('/') with no default, while rewrite_email_links in tasks.py (called on the same clead right before this) uses getattr(django_settings, 'BACKEND_BASE_URL', 'http://127.0.0.1:8000'). If BACKEND_BASE_URL is ever missing in a given environment, this function raises AttributeError and breaks email sending entirely, whereas the other falls back gracefully — worth confirming BACKEND_BASE_URL is guaranteed to always be defined, and aligning the two for consistency/resilience.
🛡️ Suggested fix
else:
- base_url = settings.BACKEND_BASE_URL.rstrip('/')
+ base_url = getattr(settings, 'BACKEND_BASE_URL', 'http://127.0.0.1:8000').rstrip('/')📝 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 build_unsubscribe_url(lead, organization=None): | |
| """ | |
| Build a signed unsubscribe URL for a lead using the backend base URL. | |
| Build a signed unsubscribe URL for a lead using the backend base URL or custom tracking domain. | |
| """ | |
| from .utils import generate_unsubscribe_token | |
| from django.conf import settings | |
| token = generate_unsubscribe_token(lead.id) | |
| return f"{settings.BACKEND_BASE_URL}/api/v1/unsubscribe/{lead.id}/{token}/" | |
| if organization and organization.custom_tracking_domain: | |
| from tenants.utils import is_local_tracking_domain | |
| scheme = 'http' if is_local_tracking_domain(organization.custom_tracking_domain) else 'https' | |
| base_url = f"{scheme}://{organization.custom_tracking_domain}" | |
| else: | |
| base_url = settings.BACKEND_BASE_URL.rstrip('/') | |
| return f"{base_url}/api/v1/unsubscribe/{lead.id}/{token}/" | |
| def build_unsubscribe_url(lead, organization=None): | |
| """ | |
| Build a signed unsubscribe URL for a lead using the backend base URL or custom tracking domain. | |
| """ | |
| from .utils import generate_unsubscribe_token | |
| from django.conf import settings | |
| token = generate_unsubscribe_token(lead.id) | |
| if organization and organization.custom_tracking_domain: | |
| from tenants.utils import is_local_tracking_domain | |
| scheme = 'http' if is_local_tracking_domain(organization.custom_tracking_domain) else 'https' | |
| base_url = f"{scheme}://{organization.custom_tracking_domain}" | |
| else: | |
| base_url = getattr(settings, 'BACKEND_BASE_URL', 'http://127.0.0.1:8000').rstrip('/') | |
| return f"{base_url}/api/v1/unsubscribe/{lead.id}/{token}/" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/campaigns/gmail_service.py` around lines 164 - 178, Update
build_unsubscribe_url to access BACKEND_BASE_URL with the same fallback used by
rewrite_email_links: default to http://127.0.0.1:8000 when the setting is
absent, then continue stripping the trailing slash before constructing the URL.
1 participant
Related Issue
Closes #485
Summary
Implemented support for custom tracking domain (CNAME) integration for organizations.
Changes
custom_tracking_domainsupport for organizations.dnspython.Type of Change
Testing
Checklist
Summary by CodeRabbit