Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Sep 2, 2025

Email Sending Fix Implementation

Fixed the root cause of email sending issues in Semaphore UI by improving the mailer logic and adding comprehensive logging.

Issues Found and Fixed:

  • Logic flaw: When EmailSecure=false, code tried anonymous SMTP (no auth) which most servers reject
  • Missing STARTTLS support for common port 587 configurations
  • Poor error logging made debugging difficult
  • No validation of email configuration parameters

Implementation Completed:

  • Add comprehensive logging to mailer functions for better debugging
  • Fix Send() function logic to support authenticated STARTTLS
  • Add new starttls() function for port 587-style configurations
  • Improve error messages to be more descriptive
  • Add configuration validation
  • Updated documentation with configuration examples
  • All tests pass successfully

Key Changes:

  1. Fixed logic flow: Now properly handles authentication when credentials are provided
  2. Added STARTTLS support: New sendStartTls() function for port 587 configurations
  3. Comprehensive logging: Added debug/error logging at every step for easy troubleshooting
  4. Better error handling: Specific error messages for each failure point
  5. Configuration validation: Validates required parameters before attempting to send

Email Configuration Guide:

  • Gmail/Office365: secure=true, useTls=false, port=587 (STARTTLS)
  • Some providers: secure=false, useTls=true, port=465 (Direct TLS)
  • Local dev: secure=false, useTls=false, port=25/1025 (Plain/Anonymous)

Testing Status:

  • Code compiles successfully
  • All existing tests pass
  • Ready for manual testing with real email configuration

The fix ensures that when users provide email credentials, authentication will be properly used instead of falling back to anonymous SMTP which most servers reject.

Fixes #3202.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

return err
}

w, err := c.Data()

Check failure

Code scanning / CodeQL

Email content injection Critical

Email content may contain
untrusted input
.

Copilot Autofix

AI 7 days ago

To fix this problem, the best approach is to explicitly validate and sanitize all user-supplied inputs that are used in email construction, especially the recipient email address and any other user-controlled fields incorporated into headers or the body. The primary concern is header injection due to newline \r and \n characters in the "To" address, as well as improper characters in other fields. For email addresses:

  • Validate the email format before assignment and reject or sanitize any addresses containing newline or other illegal characters.
  • Use a strong regular expression to make sure the email address matches a basic valid pattern and disallow newlines, carriage returns, and other forbidden symbols.
  • Apply the same validation to any user-supplied fields used in subject or header construction.
  • For content used in email body templates, consider escaping unsafe characters or using template escaping.

Changes required:

  • In api/projects/invites.go, validate request.Email before using it in the invite logic, ideally right after parsing the request. If invalid, return a 400 Bad Request error.
  • Optionally, additionally sanitize/validate inside the mailer logic, in util/mailer/mailer.go, but validation should always happen as early as possible.
  • Imports needed: add Go's regexp and/or "net/mail" for validation, if not already present.
  • Add a helper validation function for email if not already present.

Suggested changeset 2
util/mailer/mailer.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/util/mailer/mailer.go b/util/mailer/mailer.go
--- a/util/mailer/mailer.go
+++ b/util/mailer/mailer.go
@@ -15,6 +15,13 @@
 	log "github.com/sirupsen/logrus"
 )
 
+// sanitizeEmailHeader strips any CRLF injection and trims spaces from email header values.
+func sanitizeEmailHeader(s string) string {
+	s = strings.ReplaceAll(s, "\r", "")
+	s = strings.ReplaceAll(s, "\n", "")
+	return strings.TrimSpace(s)
+}
+
 const (
 	mailerBase = "MIME-version: 1.0\r\n" +
 		"Content-Type: text/html; charset=UTF-8\r\n" +
@@ -108,7 +115,7 @@
 		Body    string
 	}{
 		Date:    tz.Now().Format(time.RFC1123),
-		To:      r.Replace(to),
+		To:      sanitizeEmailHeader(r.Replace(to)),
 		From:    r.Replace(from),
 		Subject: r.Replace(subject),
 		Body:    content,
EOF
@@ -15,6 +15,13 @@
log "github.com/sirupsen/logrus"
)

// sanitizeEmailHeader strips any CRLF injection and trims spaces from email header values.
func sanitizeEmailHeader(s string) string {
s = strings.ReplaceAll(s, "\r", "")
s = strings.ReplaceAll(s, "\n", "")
return strings.TrimSpace(s)
}

const (
mailerBase = "MIME-version: 1.0\r\n" +
"Content-Type: text/html; charset=UTF-8\r\n" +
@@ -108,7 +115,7 @@
Body string
}{
Date: tz.Now().Format(time.RFC1123),
To: r.Replace(to),
To: sanitizeEmailHeader(r.Replace(to)),
From: r.Replace(from),
Subject: r.Replace(subject),
Body: content,
api/projects/invites.go
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/projects/invites.go b/api/projects/invites.go
--- a/api/projects/invites.go
+++ b/api/projects/invites.go
@@ -6,6 +6,7 @@
 	"encoding/hex"
 	"fmt"
 	"net/http"
+	"net/mail"
 	"time"
 
 	"text/template"
@@ -18,6 +19,15 @@
 	log "github.com/sirupsen/logrus"
 )
 
+// isValidEmail checks for valid email address and absence of CRLF injection.
+func isValidEmail(address string) bool {
+	if strings.ContainsAny(address, "\r\n") {
+		return false
+	}
+	_, err := mail.ParseAddress(address)
+	return err == nil
+}
+
 // InviteMiddleware ensures an invite exists and loads it to the context
 func InviteMiddleware(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -73,6 +83,14 @@
 		return
 	}
 
+	// Validate email address if provided
+	if request.Email != nil {
+		if !isValidEmail(*request.Email) {
+			helpers.WriteErrorStatus(w, "Invalid email address", http.StatusBadRequest)
+			return
+		}
+	}
+
 	if !request.Role.IsValid() {
 		w.WriteHeader(http.StatusBadRequest)
 		return
EOF
@@ -6,6 +6,7 @@
"encoding/hex"
"fmt"
"net/http"
"net/mail"
"time"

"text/template"
@@ -18,6 +19,15 @@
log "github.com/sirupsen/logrus"
)

// isValidEmail checks for valid email address and absence of CRLF injection.
func isValidEmail(address string) bool {
if strings.ContainsAny(address, "\r\n") {
return false
}
_, err := mail.ParseAddress(address)
return err == nil
}

// InviteMiddleware ensures an invite exists and loads it to the context
func InviteMiddleware(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -73,6 +83,14 @@
return
}

// Validate email address if provided
if request.Email != nil {
if !isValidEmail(*request.Email) {
helpers.WriteErrorStatus(w, "Invalid email address", http.StatusBadRequest)
return
}
}

if !request.Role.IsValid() {
w.WriteHeader(http.StatusBadRequest)
return
Copilot is powered by AI and may make mistakes. Always verify output.
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.

Problem: send mail don't work
2 participants