dashboard/app: email fixes

1. Allows sending emails upstream.
2. Filter out duplicate emails coming from our mailing lists.
3. Increase retry attempts for email commands
   (don't want them to fail due to concurrent crash reports from managers).

4 files changed, 63 insertions, 12 deletions

diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go
index d4e4d67c9..d9df7d708 100644
--- a/dashboard/app/reporting.go
+++ b/dashboard/app/reporting.go
@@ -153,7 +153,7 @@ func currentReporting(c context.Context, bug *Bug) (*Reporting, *BugReporting, i
 			continue
 		}
 		if reporting.Status == ReportingSuspended {
-			return nil, nil, 0, fmt.Sprintf("%v: reporting suspended"), nil
+			return nil, nil, 0, fmt.Sprintf("%v: reporting suspended", bugReporting.Name), nil
 		}
 		return reporting, bugReporting, i, "", nil
 	}
@@ -321,7 +321,13 @@ func incomingCommandImpl(c context.Context, cmd *dashapi.BugUpdate) (bool, strin
 		ok, reply, err = incomingCommandTx(c, now, cmd, bugKey, dupHash)
 		return err
 	}
-	err = datastore.RunInTransaction(c, tx, &datastore.TransactionOptions{XG: true})
+	err = datastore.RunInTransaction(c, tx, &datastore.TransactionOptions{
+		XG: true,
+		// Default is 3 which fails sometimes.
+		// We don't want incoming bug updates to fail,
+		// because for e.g. email we won't have an external retry.
+		Attempts: 30,
+	})
 	if err != nil {
 		return false, internalError, err
 	}
diff --git a/dashboard/app/reporting_email.go b/dashboard/app/reporting_email.go
index 0c6cb9f5b..00a601473 100644
--- a/dashboard/app/reporting_email.go
+++ b/dashboard/app/reporting_email.go
@@ -24,10 +24,21 @@ import (
 func init() {
 	http.HandleFunc("/email_poll", handleEmailPoll)
 	http.HandleFunc("/_ah/mail/", handleIncomingMail)
+
+	mailingLists = make(map[string]bool)
+	for _, cfg := range config.Namespaces {
+		for _, reporting := range cfg.Reporting {
+			if cfg, ok := reporting.Config.(*EmailConfig); ok {
+				mailingLists[email.CanonicalEmail(cfg.Email)] = true
+			}
+		}
+	}
 }
 
 const emailType = "email"
 
+var mailingLists map[string]bool
+
 type EmailConfig struct {
 	Email           string
 	Moderation      bool
@@ -80,9 +91,6 @@ func emailReport(c context.Context, rep *dashapi.BugReport) error {
 	}
 	to := []string{cfg.Email}
 	if cfg.MailMaintainers {
-		if !appengine.IsDevAppServer() {
-			panic("are you nuts?")
-		}
 		to = append(to, rep.Maintainers...)
 	}
 	to = email.MergeEmailLists(to, rep.CC)
@@ -144,6 +152,7 @@ func emailReport(c context.Context, rep *dashapi.BugReport) error {
 		ReproSyz:     len(rep.ReproSyz) != 0,
 		ReproC:       len(rep.ReproC) != 0,
 	}
+	log.Infof(c, "sending email %q to %q", rep.Title, to)
 	err = sendMailTemplate(c, rep.Title, from, to, rep.ExtID, attachments, "mail_bug.txt", data)
 	if err != nil {
 		return err
@@ -174,9 +183,13 @@ func incomingMail(c context.Context, r *http.Request) error {
 	}
 	log.Infof(c, "received email: subject %q, from %q, cc %q, msg %q, bug %q, cmd %q, link %q",
 		msg.Subject, msg.From, msg.Cc, msg.MessageID, msg.BugID, msg.Command, msg.Link)
-	// Don't send replies yet.
-	// It is not tested and it is unclear how verbose we want to be.
-	sendReply := false
+	// A mailing list can send us a duplicate email, to not process/reply to such duplicate emails,
+	// we ignore emails coming from our mailing lists. We could ignore only the mailing list
+	// associated with the bug, but we don't know the bug reporting yet (we only have bug ID).
+	if msg.Command != "" && mailingLists[email.CanonicalEmail(msg.From)] {
+		log.Infof(c, "duplicate email from mailing list, ignoring")
+		return nil
+	}
 	cmd := &dashapi.BugUpdate{
 		ID:    msg.BugID,
 		ExtID: msg.MessageID,
@@ -206,11 +219,13 @@ func incomingMail(c context.Context, r *http.Request) error {
 		return replyTo(c, msg, fmt.Sprintf("unknown command %q", msg.Command), nil)
 	}
 	ok, reply, err := incomingCommand(c, cmd)
-	_, _ = ok, err
-	if !sendReply {
-		return nil
+	if err != nil {
+		return nil // the error was already logged
 	}
-	return replyTo(c, msg, reply, nil)
+	if !ok {
+		return replyTo(c, msg, reply, nil)
+	}
+	return nil
 }
 
 var mailTemplates = template.Must(template.New("").ParseGlob("mail_*.txt"))
@@ -243,6 +258,8 @@ func replyTo(c context.Context, msg *email.Email, reply string, attachment *aema
 	if err != nil {
 		return err
 	}
+	log.Infof(c, "sending reply: to=%q cc=%q subject=%q reply=%q",
+		msg.From, msg.Cc, msg.Subject, reply)
 	replyMsg := &aemail.Message{
 		Sender:      from,
 		To:          []string{msg.From},
diff --git a/pkg/email/parser.go b/pkg/email/parser.go
index 56579157f..c000625b5 100644
--- a/pkg/email/parser.go
+++ b/pkg/email/parser.go
@@ -150,6 +150,21 @@ func RemoveAddrContext(email string) (string, string, error) {
 	return addr.String(), context, nil
 }
 
+func CanonicalEmail(email string) string {
+	addr, err := mail.ParseAddress(email)
+	if err != nil {
+		return email
+	}
+	at := strings.IndexByte(addr.Address, '@')
+	if at == -1 {
+		return email
+	}
+	if plus := strings.IndexByte(addr.Address[:at], '+'); plus != -1 {
+		addr.Address = addr.Address[:plus] + addr.Address[at:]
+	}
+	return strings.ToLower(addr.Address)
+}
+
 // extractCommand extracts command to syzbot from email body.
 // Commands are of the following form:
 // ^#syz cmd args...
diff --git a/pkg/email/parser_test.go b/pkg/email/parser_test.go
index 5156690d0..9c0a1ceec 100644
--- a/pkg/email/parser_test.go
+++ b/pkg/email/parser_test.go
@@ -76,6 +76,19 @@ func TestAddRemoveAddrContext(t *testing.T) {
 	}
 }
 
+func TestCanonicalEmail(t *testing.T) {
+	canonical := "foo@bar.com"
+	emails := []string{
+		"\"Foo Bar\" <foo+123+456@Bar.com>",
+		"<Foo@bar.com>",
+	}
+	for _, email := range emails {
+		if got := CanonicalEmail(email); got != canonical {
+			t.Errorf("got %q, want %q", got, canonical)
+		}
+	}
+}
+
 func TestParse(t *testing.T) {
 	for i, test := range parseTests {
 		body := func(t *testing.T, test ParseTest) {