Redesign email templates with dark theme#443
Open
beng360 wants to merge 7 commits into
Open
Conversation
…mplate Replace the white-background email templates with a modern dark theme design (#0a0a0a background, #141414 card, accent color bars). All 4 template types: - Investigation Started: red accent, severity badge, analysis status indicator - Investigation Completed: green accent, RCA summary box, resolution metrics - Email Verification: purple accent, monospace code display - Action Completed: green/red accent based on success/failure, error details All links point to aurora-ai.net. Removes old helper methods (_email_header_html, _cta_button_html, etc.) in favor of a single _base_html shell. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRefactors EmailService to produce dark-themed HTML emails, consolidates severity/color and timestamp/URL helpers, simplifies SMTP multipart assembly, rewrites investigation/verification emails with escaping, adds EmailService.send_action_completed_email, and adds docs for SMTP and notification setup. ChangesEmail Service Dark Theme & Action Notifications
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@server/utils/notifications/email_service.py`:
- Around line 33-39: The code fails to validate SMTP_FROM_EMAIL so
self.from_email can be None; update EmailService initialization to treat
SMTP_FROM_EMAIL as required like the others by adding it to the validation loop
(or explicitly checking self.from_email) and raise a ValueError with a clear
message if missing (reference SMTP_FROM_EMAIL, self.from_email, and the
EmailService constructor/initializer where SMTP_HOST/SMTP_USER/SMTP_PASSWORD are
validated).
- Around line 159-239: Import the stdlib html module (import html), then escape
all user-controlled values before interpolating into the content string: create
safe_* variables (e.g. safe_alert_title = html.escape(alert_title),
safe_source_type = html.escape(source_type), safe_severity =
html.escape(severity), safe_service = html.escape(service), safe_started_str =
html.escape(started_str), safe_incident_url = html.escape(incident_url,
quote=True)) and replace the direct uses of alert_title, source_type, severity,
service, started_str, incident_url in the content variable with these safe_*
names; apply the same safe_* pattern for any other user-controlled fields in
this method and other email-building methods in the class to prevent HTML
injection.
- Line 452: The session URL construction in email_service.py should URL-encode
session_id before interpolation to avoid malformed URLs; import
urllib.parse.quote and replace the string interpolation that sets session_url
(which references self.frontend_url and session_id) with one that encodes
session_id (e.g., using quote(session_id, safe='')) so the generated URL is safe
when session_id contains special characters.
- Around line 464-475: The template directly interpolates error_msg into
error_section which risks HTML injection; before constructing error_section
escape error_msg using html.escape (import html if missing) and use the escaped
value when formatting the HTML string (look for error_msg and error_section in
this block). Ensure you only escape the content (not the surrounding HTML) so
the message renders safely.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4838ddc1-a67a-4794-ada2-5215935d371f
📒 Files selected for processing (1)
server/utils/notifications/email_service.py
…de session_id - Add SMTP_FROM_EMAIL to required env var validation - HTML-escape all user-controlled values before template interpolation - URL-encode session_id in action completion email URLs - Escape error_msg in action failure error section Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
server/utils/notifications/email_service.py (1)
470-470:⚠️ Potential issue | 🟠 Major | ⚡ Quick winserver/utils/notifications/email_service.py: cast
session_idtostrbeforequote()
send_action_completed_emailbuilds the action link withquote(session_id, ...)whensession_idis truthy.urllib.parse.quote()raisesTypeErrorfor non-string values (e.g.,int,UUID), which can break email generation—cast tostrbefore encoding.Proposed fix
- session_url = f"{self.frontend_url}/actions?session={quote(session_id, safe='')}" if session_id else f"{self.frontend_url}/actions" + session_url = f"{self.frontend_url}/actions?session={quote(str(session_id), safe='')}" if session_id else f"{self.frontend_url}/actions"🤖 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 `@server/utils/notifications/email_service.py` at line 470, In send_action_completed_email, the session_url construction calls urllib.parse.quote(session_id, ...) which can TypeError if session_id isn't a str; fix by casting session_id to str before quoting (e.g., use quote(str(session_id), safe='')) so session_url always receives a string; update the line that builds session_url in send_action_completed_email to wrap session_id with str() while keeping the existing frontend_url and conditional logic.
🤖 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.
Duplicate comments:
In `@server/utils/notifications/email_service.py`:
- Line 470: In send_action_completed_email, the session_url construction calls
urllib.parse.quote(session_id, ...) which can TypeError if session_id isn't a
str; fix by casting session_id to str before quoting (e.g., use
quote(str(session_id), safe='')) so session_url always receives a string; update
the line that builds session_url in send_action_completed_email to wrap
session_id with str() while keeping the existing frontend_url and conditional
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fd1c42b0-3fdc-4735-a7b6-ae657882c6ab
📒 Files selected for processing (1)
server/utils/notifications/email_service.py
Resolve conflict in email_service.py: keep html_mod alias used throughout the branch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@server/utils/notifications/email_service.py`:
- Around line 561-657: Remove the duplicate send_action_completed_email method
block that overrides the correct implementation (the second definition that
calls _get_logo_url, _email_header_html, _status_banner_html,
_alert_title_card_html, _detail_field_html, _cta_button_html, and
_email_footer_html); delete this entire duplicate method so the original
send_action_completed_email (the one that uses existing helpers) remains and no
AttributeError occurs at runtime.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 73b8a589-3493-4afc-9ed9-532ff87352e2
📒 Files selected for processing (1)
server/utils/notifications/email_service.py
- Remove duplicate send_action_completed_email method from merge - Replace hardcoded aurora-ai.net with FRONTEND_URL env var throughout - Fix action email link path (/chat?sessionId= not /actions?session=) - Remove SendGrid-specific references from docstring - Add docs/email-notifications.md with SMTP setup guide for self-hosters Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/utils/notifications/email_service.py (1)
419-423: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
verification_codeis not HTML-escaped, unlike other user-controlled values.For consistency with the escaping pattern applied in other email methods,
verification_codeshould be escaped before HTML interpolation. While verification codes are typically system-generated alphanumeric strings, applying escaping provides defense-in-depth.Proposed fix
+ safe_code = html_mod.escape(verification_code) + content = f""" <!-- Header --> ... <div style="font-size: 36px; font-weight: 700; letter-spacing: 12px; color: `#ffffff`; font-family: 'SF Mono', 'Fira Code', 'Courier New', monospace;"> - {verification_code} + {safe_code} </div>🤖 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 `@server/utils/notifications/email_service.py` around lines 419 - 423, The verification_code is interpolated into the HTML template without HTML-escaping; locate the email-building function (e.g., the method that renders the verification email in EmailService — look for send_verification_email or the template-building function that declares verification_code) and HTML-escape the value before interpolation (use the same escape utility used elsewhere in this file/module for user-controlled values) so the template inserts escaped_verification_code instead of raw verification_code to maintain consistent escaping and defense-in-depth.
🤖 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 `@server/utils/notifications/email_service.py`:
- Around line 470-471: The URL-quoting call quote(session_id, safe='') in the
session_url construction is redundant because session_id is already typed as a
str along the call path (set in _send_action_completion_notification and passed
to send_action_completed_email); remove the quote(...) call and use session_id
directly (or if you prefer defensive coding, cast to str(session_id)) when
building session_url in send_action_completed_email so the code no longer risks
a TypeError while preserving the existing HTML escaping later.
---
Outside diff comments:
In `@server/utils/notifications/email_service.py`:
- Around line 419-423: The verification_code is interpolated into the HTML
template without HTML-escaping; locate the email-building function (e.g., the
method that renders the verification email in EmailService — look for
send_verification_email or the template-building function that declares
verification_code) and HTML-escape the value before interpolation (use the same
escape utility used elsewhere in this file/module for user-controlled values) so
the template inserts escaped_verification_code instead of raw verification_code
to maintain consistent escaping and defense-in-depth.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9e8d9922-f6dd-48bb-861b-9f0e06271da9
📒 Files selected for processing (2)
docs/email-notifications.mdserver/utils/notifications/email_service.py
Pass ssl.create_default_context() to starttls() so certificate verification is explicit rather than relying on Python version defaults. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
create_default_context() alone doesn't satisfy the rule — need explicit PROTOCOL_TLS_CLIENT with minimum_version set to TLSv1_2. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
SonarCloud flags smtplib.SMTP regardless of STARTTLS because the initial connection is plaintext. Support both: SMTP_SSL on port 465 (default), STARTTLS on port 587 when explicitly configured. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
send_action_completed_emailtemplate (success/failure variants with error details)Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Style
Improvements
Documentation