dashboard: forward incoming commands emails

Instead of reminding users to mention our mailing lists, forward their
emails there automatically.

Closes #4260.

6 files changed, 181 insertions, 48 deletions

diff --git a/dashboard/app/email_test.go b/dashboard/app/email_test.go
index 50da765eb..ae40d9e8a 100644
--- a/dashboard/app/email_test.go
+++ b/dashboard/app/email_test.go
@@ -374,14 +374,9 @@ unknown command "bad-command"
 	}
 
 	// Now mark the bug as fixed.
-	c.incomingEmail(sender1, "#syz fix: some: commit title", EmailOptCC(nil))
-	reply := c.pollEmailBug().Body
-	// nolint: lll
-	c.expectEQ(reply, `> #syz fix: some: commit title
-
-Your commands are accepted, but please keep bugs@syzkaller.com mailing list in CC next time. It serves as a history of what happened with each bug report. Thank you.
-
-`)
+	c.incomingEmail(sender1, "#syz fix: some: commit title",
+		EmailOptCC([]string{"bugs@syzkaller.com", "default@maintainers.com"}),
+		EmailOptSubject("fix bug title"))
 
 	// Check that the commit is now passed to builders.
 	builderPollResp, _ := c.client2.BuilderPoll(build.Manager)
@@ -502,15 +497,16 @@ func TestEmailDup2(t *testing.T) {
 			msg2 = c.pollEmailBug()
 			c.expectEQ(msg2.Subject, "[syzbot] KASAN: another bad thing")
 
+			cc := EmailOptCC([]string{"bugs@syzkaller.com", "default@maintainers.com"})
 			switch i {
 			case 0:
-				c.incomingEmail(msg2.Sender, "#syz dup: BUG: something bad")
+				c.incomingEmail(msg2.Sender, "#syz dup: BUG: something bad", cc)
 			case 1:
-				c.incomingEmail(msg2.Sender, "#syz dup: [syzbot] BUG: something bad")
+				c.incomingEmail(msg2.Sender, "#syz dup: [syzbot] BUG: something bad", cc)
 			case 2:
-				c.incomingEmail(msg2.Sender, "#syz dup: [syzbot] [subsystemA?] BUG: something bad")
+				c.incomingEmail(msg2.Sender, "#syz dup: [syzbot] [subsystemA?] BUG: something bad", cc)
 			default:
-				c.incomingEmail(msg2.Sender, "#syz dup: syzbot: BUG: something bad")
+				c.incomingEmail(msg2.Sender, "#syz dup: syzbot: BUG: something bad", cc)
 				reply := c.pollEmailBug()
 				c.expectTrue(strings.Contains(reply.Body, "can't find the dup bug"))
 			}
@@ -579,8 +575,10 @@ func TestEmailCrossReportingDup(t *testing.T) {
 		crash1.Title = fmt.Sprintf("bug_%v", i)
 		c.client2.ReportCrash(crash1)
 		bugSender := c.pollEmailBug().Sender
+		cc := EmailOptCC([]string{"default@maintainers.com", "test@syzkaller.com",
+			"bugs@syzkaller.com", "default2@maintainers.com", "bugs2@syzkaller.com"})
 		for j := 0; j < test.bug; j++ {
-			c.incomingEmail(bugSender, "#syz upstream")
+			c.incomingEmail(bugSender, "#syz upstream", cc)
 			bugSender = c.pollEmailBug().Sender
 		}
 
@@ -589,11 +587,11 @@ func TestEmailCrossReportingDup(t *testing.T) {
 		c.client2.ReportCrash(crash2)
 		dupSender := c.pollEmailBug().Sender
 		for j := 0; j < test.dup; j++ {
-			c.incomingEmail(dupSender, "#syz upstream")
+			c.incomingEmail(dupSender, "#syz upstream", cc)
 			dupSender = c.pollEmailBug().Sender
 		}
 
-		c.incomingEmail(bugSender, "#syz dup: "+crash2.Title)
+		c.incomingEmail(bugSender, "#syz dup: "+crash2.Title, cc)
 		if test.result {
 			c.expectNoEmail()
 		} else {
@@ -1321,3 +1319,89 @@ func expectLabels(t *testing.T, client *apiClient, extID string, labels ...strin
 	}
 	assert.ElementsMatch(t, names, labels)
 }
+
+var forwardEmailConfig = EmailConfig{
+	Email:              "test@syzkaller.com",
+	HandleListEmails:   true,
+	SubjectPrefix:      "[syzbot]",
+	MailMaintainers:    true,
+	DefaultMaintainers: []string{"some@list.com"},
+}
+
+func TestSingleListForward(t *testing.T) {
+	c := NewCtx(t)
+	defer c.Close()
+
+	client := c.makeClient(clientPublicEmail, keyPublicEmail, true)
+	c.updateReporting("access-public-email", "access-public-email-reporting1",
+		func(r Reporting) Reporting {
+			r.Config = &forwardEmailConfig
+			return r
+		})
+
+	build := testBuild(1)
+	client.UploadBuild(build)
+
+	crash := testCrash(build, 1)
+	client.ReportCrash(crash)
+
+	sender := c.pollEmailBug().Sender
+
+	c.incomingEmail(sender, "#syz fix: some: commit title",
+		EmailOptCC([]string{"some@list.com"}), EmailOptSubject("fix bug title"))
+
+	forwarded := c.pollEmailBug()
+	c.expectEQ(forwarded.Subject, "[syzbot] fix bug title")
+	c.expectEQ(forwarded.Sender, sender)
+	c.expectEQ(forwarded.To, []string{"test@syzkaller.com"})
+	c.expectEQ(len(forwarded.Cc), 0)
+	c.expectEQ(forwarded.Body, `For archival purposes, forwarding an incoming command email to
+test@syzkaller.com.
+
+***
+
+Subject: fix bug title
+Author: default@sender.com
+
+#syz fix: some: commit title
+`)
+}
+
+func TestTwoListsForward(t *testing.T) {
+	c := NewCtx(t)
+	defer c.Close()
+
+	client := c.makeClient(clientPublicEmail, keyPublicEmail, true)
+	c.updateReporting("access-public-email", "access-public-email-reporting1",
+		func(r Reporting) Reporting {
+			r.Config = &forwardEmailConfig
+			return r
+		})
+
+	build := testBuild(1)
+	client.UploadBuild(build)
+
+	crash := testCrash(build, 1)
+	client.ReportCrash(crash)
+
+	sender := c.pollEmailBug().Sender
+
+	c.incomingEmail(sender, "#syz fix: some: commit title",
+		EmailOptCC(nil), EmailOptSubject("fix bug title"))
+
+	forwarded := c.pollEmailBug()
+	c.expectEQ(forwarded.Subject, "[syzbot] fix bug title")
+	c.expectEQ(forwarded.Sender, sender)
+	c.expectEQ(forwarded.To, []string{"some@list.com", "test@syzkaller.com"})
+	c.expectEQ(len(forwarded.Cc), 0)
+	c.expectEQ(forwarded.Body, `For archival purposes, forwarding an incoming command email to
+some@list.com, test@syzkaller.com.
+
+***
+
+Subject: fix bug title
+Author: default@sender.com
+
+#syz fix: some: commit title
+`)
+}
diff --git a/dashboard/app/jobs_test.go b/dashboard/app/jobs_test.go
index 43e393c77..7d2251f37 100644
--- a/dashboard/app/jobs_test.go
+++ b/dashboard/app/jobs_test.go
@@ -292,13 +292,12 @@ func TestBootErrorPatch(t *testing.T) {
 	crash.Title = "riscv/fixes boot error: can't ssh into the instance"
 	c.client2.ReportCrash(crash)
 
-	sender := c.pollEmailBug().Sender
-	c.incomingEmail(sender, "#syz upstream\n")
-	sender = c.pollEmailBug().Sender
-	mailingList := c.config().Namespaces["test2"].Reporting[1].Config.(*EmailConfig).Email
+	report := c.pollEmailBug()
+	c.incomingEmail(report.Sender, "#syz upstream\n", EmailOptCC(report.To))
+	report = c.pollEmailBug()
 
-	c.incomingEmail(sender, syzTestGitBranchSamplePatch,
-		EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList}))
+	c.incomingEmail(report.Sender, syzTestGitBranchSamplePatch,
+		EmailOptFrom("test@requester.com"), EmailOptCC(report.To))
 	c.expectNoEmail()
 	pollResp := c.client2.pollJobs(build.Manager)
 	c.expectEQ(pollResp.Type, dashapi.JobTestPatch)
@@ -319,11 +318,10 @@ func TestTestErrorPatch(t *testing.T) {
 
 	sender := c.pollEmailBug().Sender
 	c.incomingEmail(sender, "#syz upstream\n")
-	sender = c.pollEmailBug().Sender
-	mailingList := c.config().Namespaces["test2"].Reporting[1].Config.(*EmailConfig).Email
+	report := c.pollEmailBug()
 
-	c.incomingEmail(sender, syzTestGitBranchSamplePatch,
-		EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList}))
+	c.incomingEmail(report.Sender, syzTestGitBranchSamplePatch,
+		EmailOptFrom("test@requester.com"), EmailOptCC(report.To))
 	c.expectNoEmail()
 	pollResp := c.client2.pollJobs(build.Manager)
 	c.expectEQ(pollResp.Type, dashapi.JobTestPatch)
diff --git a/dashboard/app/notifications_test.go b/dashboard/app/notifications_test.go
index c9093c5e2..4e680baba 100644
--- a/dashboard/app/notifications_test.go
+++ b/dashboard/app/notifications_test.go
@@ -257,16 +257,18 @@ func TestEmailNotifObsoleted(t *testing.T) {
 	if !strings.Contains(notif.Body, "Auto-closing this bug as obsolete") {
 		t.Fatalf("bad notification text: %q", notif.Body)
 	}
-	c.expectEQ(notif.To, []string{"bugs@syzkaller.com", "default@sender.com", "somebody@else.com"})
+	c.expectEQ(notif.To, []string{"bugs@syzkaller.com", "default@maintainers.com",
+		"default@sender.com", "somebody@else.com"})
 
 	// New crash must create new bug.
 	c.client2.ReportCrash(crash)
 	report = c.pollEmailBug()
 	c.expectEQ(report.Subject, "title1 (2)")
 	// Now the same, but for the last reporting (must have smaller CC list).
-	c.incomingEmail(report.Sender, "#syz upstream")
+	c.incomingEmail(report.Sender, "#syz upstream", EmailOptCC([]string{"test@syzkaller.com"}))
 	report = c.pollEmailBug()
-	c.incomingEmail(report.Sender, "#syz upstream")
+	c.incomingEmail(report.Sender, "#syz upstream",
+		EmailOptCC([]string{"bugs@syzkaller.com", "default@maintainers.com"}))
 	report = c.pollEmailBug()
 	_ = report
 
diff --git a/dashboard/app/reporting_email.go b/dashboard/app/reporting_email.go
index 289f5ad64..42fffb6f3 100644
--- a/dashboard/app/reporting_email.go
+++ b/dashboard/app/reporting_email.go
@@ -12,6 +12,7 @@ import (
 	"net/http"
 	"net/mail"
 	"regexp"
+	"sort"
 	"strconv"
 	"strings"
 	"sync"
@@ -501,9 +502,8 @@ func processIncomingEmail(c context.Context, msg *email.Email) error {
 	// 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.
 	fromMailingList := msg.MailingList != ""
-	mailingList := email.CanonicalEmail(emailConfig.Email)
-	mailingListInCC := checkMailingListInCC(c, msg, mailingList)
-	log.Infof(c, "from/cc mailing list: %v/%v", fromMailingList, mailingListInCC)
+	missingLists := missingMailingLists(c, msg, emailConfig)
+	log.Infof(c, "from/cc mailing list: %v (missing: %v)", fromMailingList, missingLists)
 	if fromMailingList && len(msg.BugIDs) > 0 && len(msg.Commands) > 0 {
 		// Note that if syzbot was not directly mentioned in To or Cc, this is not really
 		// a duplicate message, so it must be processed. We detect it by looking at BugID.
@@ -545,8 +545,8 @@ func processIncomingEmail(c context.Context, msg *email.Email) error {
 			replies = append(replies, handleBugCommand(c, bugInfo, msg, nil))
 		}
 		reply := groupEmailReplies(replies)
-		if reply == "" && len(msg.Commands) > 0 && !mailingListInCC && !unCc {
-			reply = warnMailingListInCC(c, msg, mailingList)
+		if reply == "" && len(msg.Commands) > 0 && len(missingLists) > 0 && !unCc {
+			return forwardEmail(c, emailConfig, msg, bugInfo, missingLists)
 		}
 		if reply != "" {
 			return replyTo(c, msg, bugInfo.bugReporting.ID, reply)
@@ -1205,23 +1205,51 @@ func (p *subjectTitleParser) parseFullTitle(subject string) (string, error) {
 	return parts[len(parts)-1], nil
 }
 
-func checkMailingListInCC(c context.Context, msg *email.Email, mailingList string) bool {
-	if msg.MailingList == mailingList {
-		return true
+func missingMailingLists(c context.Context, msg *email.Email, emailConfig *EmailConfig) []string {
+	// We want to ensure that the incoming message is recorded on both our mailing list
+	// and the archive mailing list (in case of Linux -- linux-kernel@vger.kernel.org).
+	mailingLists := []string{
+		email.CanonicalEmail(emailConfig.Email),
 	}
-	for _, cc := range msg.Cc {
-		if cc == mailingList {
-			return true
+	if emailConfig.MailMaintainers {
+		mailingLists = append(mailingLists, emailConfig.DefaultMaintainers...)
+	}
+	// Consider all recipients.
+	exists := map[string]struct{}{}
+	if msg.MailingList != "" {
+		exists[msg.MailingList] = struct{}{}
+	}
+	for _, email := range msg.Cc {
+		exists[email] = struct{}{}
+	}
+	var missing []string
+	for _, list := range mailingLists {
+		if _, ok := exists[list]; !ok {
+			missing = append(missing, list)
 		}
 	}
-	msg.Cc = append(msg.Cc, mailingList)
-	return false
+	sort.Strings(missing)
+	msg.Cc = append(msg.Cc, missing...)
+	return missing
 }
 
-func warnMailingListInCC(c context.Context, msg *email.Email, mailingList string) string {
-	return fmt.Sprintf("Your commands are accepted, but please keep %v mailing list"+
-		" in CC next time. It serves as a history of what happened with each bug report."+
-		" Thank you.", mailingList)
+func forwardEmail(c context.Context, cfg *EmailConfig, msg *email.Email,
+	info *bugInfoResult, mailingLists []string) error {
+	log.Infof(c, "forwarding email: id=%q from=%q to=%q", msg.MessageID, msg.Author, mailingLists)
+	body := fmt.Sprintf(`For archival purposes, forwarding an incoming command email to
+%v.
+
+***
+
+Subject: %s
+Author: %s
+
+%s`, strings.Join(mailingLists, ", "), msg.Subject, msg.Author, msg.Body)
+	from, err := email.AddAddrContext(fromAddr(c), info.bugReporting.ID)
+	if err != nil {
+		return err
+	}
+	return sendMailText(c, cfg, msg.Subject, from, mailingLists, info.bugReporting.ExtID, body)
 }
 
 func sendMailText(c context.Context, cfg *EmailConfig, subject, from string, to []string, replyTo, body string) error {
diff --git a/dashboard/app/subsystem_test.go b/dashboard/app/subsystem_test.go
index 3462a471f..8bb560d32 100644
--- a/dashboard/app/subsystem_test.go
+++ b/dashboard/app/subsystem_test.go
@@ -1005,7 +1005,7 @@ func TestRemindersPriority(t *testing.T) {
 	defer c.Close()
 
 	client := c.makeClient(clientSubsystemRemind, keySubsystemRemind, true)
-	mailingList := c.config().Namespaces["subsystem-reminders"].Reporting[1].Config.(*EmailConfig).Email
+	cc := EmailOptCC([]string{"bugs@syzkaller.com", "default@maintainers.com"})
 	build := testBuild(1)
 	client.UploadBuild(build)
 
@@ -1018,7 +1018,7 @@ func TestRemindersPriority(t *testing.T) {
 	client.ReportCrash(aFirst)
 	sender, firstExtID := client.pollEmailAndExtID()
 	c.incomingEmail(sender, "#syz set prio: low\n",
-		EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList}))
+		EmailOptFrom("test@requester.com"), cc)
 	c.advanceTime(time.Hour)
 
 	// WARNING: a second, normal prio
@@ -1036,7 +1036,7 @@ func TestRemindersPriority(t *testing.T) {
 	client.ReportCrash(aThird)
 	sender, thirdExtID := client.pollEmailAndExtID()
 	c.incomingEmail(sender, "#syz set prio: high\n",
-		EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList}))
+		EmailOptFrom("test@requester.com"), cc)
 	c.advanceTime(time.Hour)
 
 	// Report bugs once more to pretend they're still valid.
diff --git a/dashboard/app/util_test.go b/dashboard/app/util_test.go
index 171ec8029..ba4a953d3 100644
--- a/dashboard/app/util_test.go
+++ b/dashboard/app/util_test.go
@@ -246,6 +246,12 @@ func (c *Ctx) setNoObsoletions() {
 	}
 }
 
+func (c *Ctx) updateReporting(ns, name string, f func(Reporting) Reporting) {
+	c.transformContext = func(c context.Context) context.Context {
+		return contextWithConfig(c, replaceReporting(c, ns, name, f))
+	}
+}
+
 func (c *Ctx) decommissionManager(ns, oldManager, newManager string) {
 	c.transformContext = func(c context.Context) context.Context {
 		newConfig := replaceManagerConfig(c, ns, oldManager, func(cfg ConfigManager) ConfigManager {
@@ -729,3 +735,18 @@ func replaceManagerConfig(c context.Context, ns, mgr string, f func(ConfigManage
 		return &ret
 	})
 }
+
+func replaceReporting(c context.Context, ns, name string, f func(Reporting) Reporting) *GlobalConfig {
+	return replaceNamespaceConfig(c, ns, func(cfg *Config) *Config {
+		ret := *cfg
+		var newReporting []Reporting
+		for _, cfg := range ret.Reporting {
+			if cfg.Name == name {
+				cfg = f(cfg)
+			}
+			newReporting = append(newReporting, cfg)
+		}
+		ret.Reporting = newReporting
+		return &ret
+	})
+}