Skip to content

Comments

Check if email was sent with TLS#495

Merged
ahmetskilinc merged 6 commits intoMail-0:stagingfrom
danteissaias:feat/tls
Mar 20, 2025
Merged

Check if email was sent with TLS#495
ahmetskilinc merged 6 commits intoMail-0:stagingfrom
danteissaias:feat/tls

Conversation

@danteissaias
Copy link
Contributor

@danteissaias danteissaias commented Mar 20, 2025

Description

Only show the sent with TLS message if the email was actually sent with TLS.


Type of Change

Please delete options that are not relevant.

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature with breaking changes)
  • 📝 Documentation update
  • 🎨 UI/UX improvement
  • 🔒 Security enhancement
  • ⚡ Performance improvement

Areas Affected

Please check all that apply:

  • Email Integration (Gmail, IMAP, etc.)
  • User Interface/Experience
  • Authentication/Authorization
  • Data Storage/Management
  • API Endpoints
  • Documentation
  • Testing Infrastructure
  • Development Workflow
  • Deployment/Infrastructure

Testing Done

Describe the tests you've done:

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • Cross-browser testing (if UI changes)
  • Mobile responsiveness verified (if UI changes)

Security Considerations

For changes involving data or authentication:

  • No sensitive data is exposed
  • Authentication checks are in place
  • Input validation is implemented
  • Rate limiting is considered (if applicable)

Checklist

  • I have read the CONTRIBUTING document
  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in complex areas
  • I have updated the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix/feature works
  • All tests pass locally
  • Any dependent changes are merged and published

By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.

Summary by CodeRabbit

  • New Features

    • Enhanced email parsing now detects TLS encryption, marking emails with a security status.
    • The mail display has been updated to show a security indicator when TLS is used, giving users clearer insight into email safety.
    • A new property indicating TLS usage has been added to the email message structure.
  • Bug Fixes

    • Simplified the unsubscribe functionality for a more streamlined user experience.

@vercel
Copy link

vercel bot commented Mar 20, 2025

@danteissaias is attempting to deploy a commit to the Zero Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2025

Walkthrough

The changes enhance the email processing pipeline by adding TLS detection. The parser now collects header data to determine if an email was sent with TLS encryption using a new utility function. This new logic updates the email's parsed object with a tls property. Additionally, the UI component is updated to conditionally display security information based on the TLS flag. The parsed message type is also extended to include this new property.

Changes

Files Change Summary
apps/mail/app/.../driver/google.ts, apps/mail/lib/.../email-utils.ts, apps/mail/types/index.ts Enhanced email parsing: added header extraction for TLS detection, introduced wasSentWithTLS utility function, and updated the ParsedMessage interface with a new tls boolean property.
apps/mail/components/mail/.../mail-display.tsx Updated rendering logic to conditionally display security information based on the tls property.

Suggested reviewers

  • MrgSub

Poem

Hop along the code, I’m a rabbit so free,
TLS now lights up our email decree.
Headers inspected with a curious glance,
Secure messages now get their chance.
With carrots and code, I joyfully bound,
In a world where TLS security is found!
🥕🐇


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 126de63 and eb6583d.

📒 Files selected for processing (1)
  • apps/mail/components/mail/mail-display.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • apps/mail/components/mail/mail-display.tsx

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (2)
apps/mail/lib/email-utils.ts (1)

162-180: TLS detection logic implemented correctly.

The wasSentWithTLS function correctly implements the detection of TLS usage by checking for common TLS indicators in email headers. The implementation:

  1. Uses appropriate regex patterns to match various TLS indicators
  2. Processes headers in reverse order (newest to oldest) which is correct for email delivery chains
  3. Returns early once a match is found for efficiency

Consider adding JSDoc comments to explain the function's purpose and the indicators being checked. Also, creating a copy of the array before reversing it would prevent modifying the original array:

-export const wasSentWithTLS = (receivedHeaders: string[])  => {
+/**
+ * Determines if an email was sent with TLS encryption by examining its received headers
+ * @param receivedHeaders - Array of 'Received' headers from the email
+ * @returns true if any header indicates TLS usage, false otherwise
+ */
+export const wasSentWithTLS = (receivedHeaders: string[])  => {
   const tlsIndicators = [
     /using\s+TLS/i,
     /with\s+ESMTPS/i,
     /version=TLS[0-9_.]+/i,
     /TLSv[0-9.]+/i,
     /cipher=[A-Z0-9-]+/i
   ];
   
-  for (const header of receivedHeaders.reverse()) {
+  for (const header of [...receivedHeaders].reverse()) {
     for (const indicator of tlsIndicators) {
       if (indicator.test(header)) {
         return true;
       }
     }
   }

   return false;
}
apps/mail/app/api/driver/google.ts (1)

132-134: TLS detection header extraction implementation.

The code extracts 'received' headers and checks for the presence of a 'tls-report' header, which is a good approach for determining if TLS was used.

Consider adding parentheses around payload?.headers for a more defensive approach to handle the case when headers might be undefined:

-const receivedHeaders = payload?.headers?.filter(header => header.name?.toLowerCase() === 'received')
+const receivedHeaders = (payload?.headers || []).filter(header => header.name?.toLowerCase() === 'received')
   .map(header => header.value || '') || [];
-const hasTLSReport = payload?.headers?.some(header => header.name?.toLowerCase() === 'tls-report');
+const hasTLSReport = (payload?.headers || []).some(header => header.name?.toLowerCase() === 'tls-report');
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d52347a and 2bda96e.

📒 Files selected for processing (4)
  • apps/mail/app/api/driver/google.ts (2 hunks)
  • apps/mail/components/mail/mail-display.tsx (1 hunks)
  • apps/mail/lib/email-utils.ts (1 hunks)
  • apps/mail/types/index.ts (1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
apps/mail/app/api/driver/google.ts (1)
apps/mail/lib/email-utils.ts (1) (1)
  • wasSentWithTLS (162-180)
🔇 Additional comments (5)
apps/mail/types/index.ts (1)

38-38: Type definition properly updated.

The tls property is correctly added to the ParsedMessage interface with the appropriate boolean type. This change enables the interface to represent whether an email was sent with TLS encryption.

apps/mail/lib/email-utils.ts (1)

160-160: Fixed missing closing brace.

The syntactic error with the missing closing brace has been properly fixed.

apps/mail/components/mail/mail-display.tsx (1)

339-347: Properly implemented conditional security information display.

The security information now correctly displays only when the email was actually sent with TLS. This is a good UX improvement that prevents showing misleading security indicators for emails that weren't sent securely.

apps/mail/app/api/driver/google.ts (2)

6-6: Properly imported the new TLS utility function.

The wasSentWithTLS function is correctly imported from the email utils module.


140-140: TLS status properly added to returned object.

The TLS status is correctly determined by checking both the received headers (using the new utility function) and the presence of a TLS report header. This comprehensive approach ensures accurate TLS detection.

@ahmetskilinc
Copy link
Contributor

lgtm, resolve conflicts and ill merge

@ahmetskilinc ahmetskilinc merged commit b701799 into Mail-0:staging Mar 20, 2025
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants