feat(#7): implementing the email to sms using imap#8
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements email-to-SMS forwarding functionality using IMAP, allowing users to configure rules that monitor email accounts and forward incoming emails as SMS messages. This addresses issue #7 by adding a new redirect_emails rule type.
Key Changes:
- Added new
redirect_emailsrule type with IMAP-based email monitoring - Implemented background goroutines to poll email accounts at configurable intervals
- Added email parsing logic to extract sender, subject, and body for SMS forwarding
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
| rules.example.yaml | Added example configuration for email-to-SMS forwarding with IMAP server settings and updated Netflix sender to uppercase |
| main.go | Implemented email monitoring system with IMAP client integration, email parsing, SMS forwarding logic, and graceful shutdown handling for email monitors |
| go.mod | Added dependencies for go-imap v2 (beta), go-message, and go-sasl libraries |
| go.sum | Added checksums for new IMAP-related dependencies and their transitive dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for name, stopChan := range emailMonitors { | ||
| log.Printf("INFO: Stopping email monitor: %s", name) | ||
| stopChan <- true | ||
| close(stopChan) |
There was a problem hiding this comment.
The shutdown logic sends to the stop channel and then immediately closes it. If a goroutine is in the middle of processing emails (not waiting on the channel), closing the channel could cause a panic when the goroutine eventually tries to check the channel. Consider using a buffered channel or implementing a graceful shutdown with a timeout to ensure goroutines have time to complete their current operation.
| close(stopChan) |
| for { | ||
| select { | ||
| case <-stopChan: | ||
| log.Printf("INFO: Stopping email monitor for '%s'", rule.Name) | ||
| return | ||
| case <-ticker.C: | ||
| checkForNewEmails(rule, lastCheckTime) | ||
| lastCheckTime = time.Now() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func checkForNewEmails(rule Rule, since time.Time) { | ||
| // Connect to IMAP server | ||
| client, err := imapclient.DialTLS(rule.EmailServer, nil) | ||
| if err != nil { | ||
| log.Printf("ERROR: Failed to connect to email server %s: %v", rule.EmailServer, err) | ||
| return | ||
| } | ||
| defer client.Close() | ||
|
|
||
| // Login | ||
| if err := client.Login(rule.Username, rule.Password).Wait(); err != nil { | ||
| log.Printf("ERROR: Failed to login to email account %s: %v", rule.Username, err) | ||
| return | ||
| } | ||
| defer client.Logout() | ||
|
|
There was a problem hiding this comment.
Each call to checkForNewEmails creates a new IMAP connection and immediately closes it after fetching messages. For high-frequency checking (e.g., every 5 seconds as shown in the example), this could be inefficient and may trigger connection limits. Consider implementing connection pooling or reusing connections with periodic reconnection to improve performance and reduce overhead.
| for { | |
| select { | |
| case <-stopChan: | |
| log.Printf("INFO: Stopping email monitor for '%s'", rule.Name) | |
| return | |
| case <-ticker.C: | |
| checkForNewEmails(rule, lastCheckTime) | |
| lastCheckTime = time.Now() | |
| } | |
| } | |
| } | |
| func checkForNewEmails(rule Rule, since time.Time) { | |
| // Connect to IMAP server | |
| client, err := imapclient.DialTLS(rule.EmailServer, nil) | |
| if err != nil { | |
| log.Printf("ERROR: Failed to connect to email server %s: %v", rule.EmailServer, err) | |
| return | |
| } | |
| defer client.Close() | |
| // Login | |
| if err := client.Login(rule.Username, rule.Password).Wait(); err != nil { | |
| log.Printf("ERROR: Failed to login to email account %s: %v", rule.Username, err) | |
| return | |
| } | |
| defer client.Logout() | |
| // Maintain a persistent IMAP client for this rule to avoid reconnecting on every tick. | |
| var client *imapclient.Client | |
| // Helper to establish a new IMAP connection and log in. | |
| connect := func() { | |
| if client != nil { | |
| return | |
| } | |
| c, err := imapclient.DialTLS(rule.EmailServer, nil) | |
| if err != nil { | |
| log.Printf("ERROR: Failed to connect to email server %s: %v", rule.EmailServer, err) | |
| return | |
| } | |
| if err := c.Login(rule.Username, rule.Password).Wait(); err != nil { | |
| log.Printf("ERROR: Failed to login to email account %s: %v", rule.Username, err) | |
| _ = c.Close() | |
| return | |
| } | |
| client = c | |
| } | |
| // Attempt initial connection; if it fails, we'll keep retrying on each tick. | |
| connect() | |
| for { | |
| select { | |
| case <-stopChan: | |
| log.Printf("INFO: Stopping email monitor for '%s'", rule.Name) | |
| if client != nil { | |
| _ = client.Logout() | |
| _ = client.Close() | |
| } | |
| return | |
| case <-ticker.C: | |
| // Ensure we have a connected client; if not, try to reconnect. | |
| if client == nil { | |
| connect() | |
| } | |
| if client != nil { | |
| checkForNewEmails(rule, client, lastCheckTime) | |
| lastCheckTime = time.Now() | |
| } | |
| } | |
| } | |
| } | |
| func checkForNewEmails(rule Rule, client *imapclient.Client, since time.Time) { | |
| // Assume the IMAP client is already connected and logged in. |
| // Fetch the messages (without Peek so they get marked as read) | ||
| fetchOptions := &imap.FetchOptions{ | ||
| Envelope: true, | ||
| BodySection: []*imap.FetchItemBodySection{ | ||
| {}, // Not using Peek, so messages will be marked as read |
There was a problem hiding this comment.
The code searches for unseen messages and then marks them as read during fetch (by not using Peek). However, if an error occurs during processing or sending the SMS, the email will still be marked as read and won't be retried. Consider implementing error handling that keeps messages unread if SMS delivery fails, or implement a separate retry mechanism.
| // Fetch the messages (without Peek so they get marked as read) | |
| fetchOptions := &imap.FetchOptions{ | |
| Envelope: true, | |
| BodySection: []*imap.FetchItemBodySection{ | |
| {}, // Not using Peek, so messages will be marked as read | |
| // Fetch the messages using PEEK so they are not automatically marked as read | |
| fetchOptions := &imap.FetchOptions{ | |
| Envelope: true, | |
| BodySection: []*imap.FetchItemBodySection{ | |
| {Peek: true}, // Use Peek to avoid setting the \Seen flag during fetch |
|
|
||
| if err := fetchCmd.Close(); err != nil { | ||
| log.Printf("ERROR: Fetch error: %v", err) | ||
| } |
There was a problem hiding this comment.
The fetchCmd.Close() is called twice: once with defer at line 247 and again explicitly at line 265. This could cause issues or unexpected behavior. Remove the explicit call at line 265-267 since the deferred call will handle cleanup.
| if err := fetchCmd.Close(); err != nil { | |
| log.Printf("ERROR: Fetch error: %v", err) | |
| } |
| Attachments: []interface{}{}, | ||
| } | ||
|
|
||
| sendMessage(outMsg) |
There was a problem hiding this comment.
The error from sendMessage is not being checked when forwarding emails to SMS. If the SMS sending fails (e.g., due to network issues or server problems), the user won't be aware that the email wasn't delivered. Consider checking the return value or at least logging if the send operation fails to ensure visibility of delivery issues.
| sendMessage(outMsg) | |
| if err := sendMessage(outMsg); err != nil { | |
| log.Printf("ERROR: Failed to forward email to %s: %v", receiver, err) | |
| continue | |
| } |
| emailCount++ | ||
|
|
||
| // Small delay to avoid overwhelming the server | ||
| time.Sleep(time.Duration(idx) * time.Second) |
There was a problem hiding this comment.
The delay calculation time.Duration(idx) * time.Second uses the array index to stagger the startup of email monitors. This means the first rule (idx=0) has no delay, the second has 1 second, third has 2 seconds, etc. This could cause unexpected delays for rules later in the list. Consider using a fixed small delay (e.g., 100-500ms) for all rules instead, or only delay if idx > 0.
| emailCount++ | |
| // Small delay to avoid overwhelming the server | |
| time.Sleep(time.Duration(idx) * time.Second) | |
| // Small fixed delay between starting monitors to avoid overwhelming the server. | |
| // Apply only after the first monitor has been started. | |
| if emailCount > 0 { | |
| time.Sleep(200 * time.Millisecond) | |
| } | |
| emailCount++ |
| type: redirect_emails | ||
| email_server: "mail.my-server.com:993" # IMAP server with port (993 for SSL) | ||
| username: "postmaster@my-server.com" # Your email address | ||
| password: "change_me" # Your email password or app password |
There was a problem hiding this comment.
The email password is stored in plain text in the YAML configuration file. Consider documenting the security implications in the example configuration, and potentially supporting environment variable substitution for sensitive fields like passwords (e.g., using ${ENV_VAR} syntax).
| password: "change_me" # Your email password or app password | |
| # SECURITY NOTE: | |
| # Avoid storing real email passwords directly in rules.yaml or in version control. | |
| # Prefer providing the password via an environment variable or a secrets manager. | |
| # If your application supports it, you can use an environment-variable placeholder | |
| # like ${EMAIL_ACCOUNT_PASSWORD} and have it resolved at runtime. | |
| password: "${EMAIL_ACCOUNT_PASSWORD}" # Example: resolved from EMAIL_ACCOUNT_PASSWORD env var |
| to_receivers: | ||
| - "+33755442211" | ||
| - "+33611223344" | ||
| check_interval_secs: 5 # Check every 5 seconds (default) |
There was a problem hiding this comment.
The check interval in the example is set to 5 seconds, which could cause excessive load on email servers and may violate rate limits for many IMAP providers. Consider using a more conservative default value (e.g., 60-300 seconds) in the example to prevent users from inadvertently triggering rate limits or getting their accounts blocked.
| check_interval_secs: 5 # Check every 5 seconds (default) | |
| check_interval_secs: 60 # Check every 60 seconds (recommended default: 60–300) |
| if len(emailBody) > maxBodyLength { | ||
| emailBody = emailBody[:maxBodyLength] + "..." |
There was a problem hiding this comment.
The email body truncation is performed using simple byte slicing emailBody[:maxBodyLength]. This could split multi-byte UTF-8 characters, resulting in invalid UTF-8 sequences. Consider using rune-based truncation or a library that handles UTF-8 properly to avoid corrupting international characters.
| if len(emailBody) > maxBodyLength { | |
| emailBody = emailBody[:maxBodyLength] + "..." | |
| bodyRunes := []rune(emailBody) | |
| if len(bodyRunes) > maxBodyLength { | |
| emailBody = string(bodyRunes[:maxBodyLength]) + "..." |
| github.com/emersion/go-imap/v2 v2.0.0-beta.7 // indirect | ||
| github.com/emersion/go-message v0.18.1 // indirect | ||
| github.com/emersion/go-sasl v0.0.0-20231106173351-e73c9f7bad43 // indirect |
There was a problem hiding this comment.
The new IMAP-related dependencies are marked as // indirect in go.mod, but they are directly imported and used in main.go. These should be direct dependencies, not indirect. Run go mod tidy to properly classify these dependencies as direct imports.
| github.com/emersion/go-imap/v2 v2.0.0-beta.7 // indirect | |
| github.com/emersion/go-message v0.18.1 // indirect | |
| github.com/emersion/go-sasl v0.0.0-20231106173351-e73c9f7bad43 // indirect | |
| github.com/emersion/go-imap/v2 v2.0.0-beta.7 | |
| github.com/emersion/go-message v0.18.1 | |
| github.com/emersion/go-sasl v0.0.0-20231106173351-e73c9f7bad43 |
No description provided.