From 39f27708bb399ebd3ba5d8c9558d4bc4bfdcf7a0 Mon Sep 17 00:00:00 2001 From: Aleksandr Nogikh Date: Wed, 2 Jul 2025 17:38:27 +0200 Subject: dashboard: forward emails from designated inboxes For some specified inboxes, forward the emails that contain syz commands. Add tests to verify the behavior. --- dashboard/app/config.go | 22 ++++++++ dashboard/app/email_test.go | 65 +++++++++++++++++++++++- dashboard/app/reporting_email.go | 106 ++++++++++++++++++++++++++++++--------- dashboard/app/util_test.go | 2 +- 4 files changed, 168 insertions(+), 27 deletions(-) (limited to 'dashboard') diff --git a/dashboard/app/config.go b/dashboard/app/config.go index 96f631e0a..409b99ce5 100644 --- a/dashboard/app/config.go +++ b/dashboard/app/config.go @@ -55,6 +55,8 @@ type GlobalConfig struct { // List of email addresses which are considered app's own email addresses. // All emails sent from one of these email addresses shall be ignored by the app on reception. ExtraOwnEmailAddresses []string + // Emails sent to these addresses are not to be processed like replies to syzbot emails. + MonitoredInboxes []*PerInboxConfig // Main part of the URL at which the app is reachable. // This URL is used e.g. to construct HTML links contained in the emails sent by the app. AppURL string @@ -71,6 +73,13 @@ type GlobalConfig struct { UploadBucket string } +type PerInboxConfig struct { + // Regexp of the inbox which received the message. + InboxRe string + // The mailing lists that must be also Cc'd for all emails that contain syz commands. + ForwardTo []string +} + // Per-namespace config. type Config struct { // See GlobalConfig.AccessLevel. @@ -496,6 +505,7 @@ func checkConfig(cfg *GlobalConfig) { checkNamespace(ns, cfg, namespaces, clientNames) } checkDiscussionEmails(cfg.DiscussionEmails) + checkMonitoredInboxes(cfg.MonitoredInboxes) checkACL(cfg.ACL) } @@ -517,6 +527,18 @@ func checkACL(acls []*ACLItem) { } } +func checkMonitoredInboxes(list []*PerInboxConfig) { + for _, item := range list { + _, err := regexp.Compile(item.InboxRe) + if err != nil { + panic(fmt.Sprintf("invalid InboxRe: %v", err)) + } + if len(item.ForwardTo) == 0 { + panic("PerInboxConfig with an empty ForwardTo") + } + } +} + func checkDiscussionEmails(list []DiscussionEmailConfig) { dup := map[string]struct{}{} for _, item := range list { diff --git a/dashboard/app/email_test.go b/dashboard/app/email_test.go index 18dd51993..219c7795e 100644 --- a/dashboard/app/email_test.go +++ b/dashboard/app/email_test.go @@ -14,6 +14,7 @@ import ( "github.com/google/syzkaller/pkg/email" "github.com/google/syzkaller/sys/targets" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // nolint: funlen @@ -1348,7 +1349,7 @@ func TestSingleListForward(t *testing.T) { EmailOptCC([]string{"some@list.com"}), EmailOptSubject("fix bug title")) forwarded := c.pollEmailBug() - c.expectEQ(forwarded.Subject, "[syzbot] fix bug title") + c.expectEQ(forwarded.Subject, "fix bug title") c.expectEQ(forwarded.Sender, sender) c.expectEQ(forwarded.To, []string{"test@syzkaller.com"}) c.expectEQ(len(forwarded.Cc), 0) @@ -1387,7 +1388,7 @@ func TestTwoListsForward(t *testing.T) { EmailOptCC(nil), EmailOptSubject("fix bug title")) forwarded := c.pollEmailBug() - c.expectEQ(forwarded.Subject, "[syzbot] fix bug title") + c.expectEQ(forwarded.Subject, "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) @@ -1402,3 +1403,63 @@ Author: default@sender.com #syz fix: some: commit title `) } + +func TestForwardEmailInbox(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + c.transformContext = func(c context.Context) context.Context { + newConfig := *getConfig(c) + newConfig.MonitoredInboxes = []*PerInboxConfig{ + { + InboxRe: `^syzbot\+prefix.*@testapp\.appspotmail\.com$`, + ForwardTo: []string{`forward@a.com`, `forward@b.com`}, + }, + } + return contextWithConfig(c, &newConfig) + } + + t.Run("forwarded", func(t *testing.T) { + c.incomingEmail("syzbot+prefixABCD@testapp.appspotmail.com", + "#syz invalid", + EmailOptMessageID(1), + EmailOptFrom("someone@mail.com"), + EmailOptCC([]string{"some@list.com"})) + msg := c.pollEmailBug() + require.NotNil(t, msg) + assert.Equal(t, `"syzbot" `, msg.Sender) + assert.ElementsMatch(t, []string{"forward@a.com", "forward@b.com", "someone@mail.com"}, + msg.To, "must be sent to the author and the missing lists") + assert.Equal(t, "<1>", msg.Headers.Get("In-Reply-To")) + assert.Equal(t, `For archival purposes, forwarding an incoming command email to +forward@a.com, forward@b.com. + +*** + +Subject: crash1 +Author: someone@mail.com + +#syz invalid +`, msg.Body) + }) + + t.Run("no command", func(t *testing.T) { + c.incomingEmail("syzbot+prefixABCD@testapp.appspotmail.com", + "Some spam message", + EmailOptMessageID(1), + EmailOptFrom("someone@mail.com")) + c.expectNoEmail() + }) + + t.Run("unrelated", func(t *testing.T) { + // It will react as if the email targeted the bug ABCD. + c.incomingEmail("syzbot+ABCD@testapp.appspotmail.com", + "#syz invalid", + EmailOptMessageID(1), + EmailOptFrom("someone@mail.com"), + EmailOptCC([]string{"some@list.com"})) + msg := c.pollEmailBug() + require.NotNil(t, msg) + assert.Contains(t, msg.Body, "I see the command but can't find the corresponding bug") + }) +} diff --git a/dashboard/app/reporting_email.go b/dashboard/app/reporting_email.go index d75b86a6a..a6942a952 100644 --- a/dashboard/app/reporting_email.go +++ b/dashboard/app/reporting_email.go @@ -10,9 +10,11 @@ import ( "errors" "fmt" "io" + "maps" "net/http" "net/mail" "regexp" + "slices" "sort" "strconv" "strings" @@ -106,6 +108,13 @@ func (cfg *EmailConfig) Validate() error { return nil } +func (cfg *EmailConfig) getSubject(title string) string { + if cfg.SubjectPrefix != "" { + return cfg.SubjectPrefix + " " + title + } + return title +} + // handleCoverageReports sends a coverage report for the two full months preceding the current one. // Assuming it is called June 15, the monthly report will cover April-May diff. func handleCoverageReports(w http.ResponseWriter, r *http.Request) { @@ -375,7 +384,7 @@ func emailSendBugNotif(c context.Context, notif *dashapi.BugNotification) error return err } log.Infof(c, "sending notif %v for %q to %q: %v", notif.Type, notif.Title, to, body) - if err := sendMailText(c, cfg, notif.Title, from, to, notif.ExtID, body); err != nil { + if err := sendMailText(c, cfg.getSubject(notif.Title), from, to, notif.ExtID, body); err != nil { return err } cmd := &dashapi.BugUpdate{ @@ -554,7 +563,7 @@ func sendMailTemplate(c context.Context, params *mailSendParams) error { return fmt.Errorf("failed to execute %v template: %w", params.templateName, err) } log.Infof(c, "sending email %q to %q", params.title, to) - return sendMailText(c, params.cfg, params.title, from, to, params.replyTo, body.String()) + return sendMailText(c, params.cfg.getSubject(params.title), from, to, params.replyTo, body.String()) } func generateEmailBugTitle(rep *dashapi.BugReport, emailConfig *EmailConfig) string { title := "" @@ -580,14 +589,6 @@ func handleIncomingMail(w http.ResponseWriter, r *http.Request) { log.Errorf(c, "invalid email handler URL: %s", url) return } - source := dashapi.NoDiscussion - for _, item := range getConfig(c).DiscussionEmails { - if item.ReceiveAddress != myEmail { - continue - } - source = item.Source - break - } msg, err := email.Parse(r.Body, ownEmails(c), ownMailingLists(c), []string{ appURL(c), }) @@ -598,23 +599,80 @@ func handleIncomingMail(w http.ResponseWriter, r *http.Request) { log.Warningf(c, "failed to parse email: %v", err) return } - log.Infof(c, "received email at %q, source %q", myEmail, source) - if source == dashapi.NoDiscussion { + source := matchDiscussionEmail(c, myEmail) + inbox := matchInbox(c, myEmail) + log.Infof(c, "received email at %q, source %q, matched ignored inbox=%v", + myEmail, source, inbox != nil) + if inbox != nil { + err = processInboxEmail(c, msg, inbox) + } else if source != dashapi.NoDiscussion { + // Discussions are safe to handle even during an emergency stop. + err = processDiscussionEmail(c, msg, source) + } else { if stop, err := emergentlyStopped(c); err != nil || stop { log.Errorf(c, "abort email processing due to emergency stop (stop %v, err %v)", stop, err) return } err = processIncomingEmail(c, msg) - } else { - // Discussions are safe to handle even during an emergency stop. - err = processDiscussionEmail(c, msg, source) } if err != nil { log.Errorf(c, "email processing failed: %s", err) } } +func matchDiscussionEmail(c context.Context, myEmail string) dashapi.DiscussionSource { + for _, item := range getConfig(c).DiscussionEmails { + if item.ReceiveAddress != myEmail { + continue + } + return item.Source + } + return dashapi.NoDiscussion +} + +func matchInbox(c context.Context, myEmail string) *PerInboxConfig { + for _, item := range getConfig(c).MonitoredInboxes { + if ok, _ := regexp.MatchString(item.InboxRe, myEmail); ok { + return item + } + } + return nil +} + +func processInboxEmail(c context.Context, msg *email.Email, inbox *PerInboxConfig) error { + if len(msg.Commands) == 0 || len(msg.BugIDs) == 0 { + // Do not forward emails with no commands. + // Also, we don't care about the emails that don't include any BugIDs. + return nil + } + needForwardTo := map[string]bool{} + for _, cc := range inbox.ForwardTo { + needForwardTo[cc] = true + } + for _, email := range msg.Cc { + delete(needForwardTo, email) + } + missing := slices.Collect(maps.Keys(needForwardTo)) + sort.Strings(missing) + if len(missing) == 0 { + // Everything's OK. + return nil + } + // We don't want to forward from a name+hash@domain address because + // the automation could confuse that with bug reports and not react to the commamnds in there. + // So we forward just from name@domain, but Cc name+hash@domain to still identify the email + // as related to the bug identified by the hash. + cc, err := email.AddAddrContext(fromAddr(c), msg.BugIDs[0]) + if err != nil { + return err + } + if !stringInList(msg.Cc, cc) { + msg.Cc = append(msg.Cc, cc) + } + return forwardEmail(c, msg, missing, "", msg.MessageID, true) +} + // nolint: gocyclo func processIncomingEmail(c context.Context, msg *email.Email) error { // Ignore any incoming emails from syzbot itself. @@ -676,7 +734,7 @@ func processIncomingEmail(c context.Context, msg *email.Email) error { } reply := groupEmailReplies(replies) if reply == "" && len(msg.Commands) > 0 && len(missingLists) > 0 && !unCc { - return forwardEmail(c, emailConfig, msg, bugInfo, missingLists) + return forwardEmail(c, msg, missingLists, bugInfo.bugReporting.ID, bugInfo.bugReporting.ExtID, false) } if reply != "" { return replyTo(c, msg, bugInfo.bugReporting.ID, reply) @@ -1363,8 +1421,8 @@ func missingMailingLists(c context.Context, msg *email.Email, emailConfig *Email return missing } -func forwardEmail(c context.Context, cfg *EmailConfig, msg *email.Email, - info *bugInfoResult, mailingLists []string) error { +func forwardEmail(c context.Context, msg *email.Email, mailingLists []string, + bugID, inReplyTo string, includeAuthor bool) 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. @@ -1375,23 +1433,23 @@ Subject: %s Author: %s %s`, strings.Join(mailingLists, ", "), msg.Subject, msg.Author, msg.Body) - from, err := email.AddAddrContext(fromAddr(c), info.bugReporting.ID) + from, err := email.AddAddrContext(fromAddr(c), bugID) if err != nil { return err } - return sendMailText(c, cfg, msg.Subject, from, mailingLists, info.bugReporting.ExtID, body) + if includeAuthor { + mailingLists = append([]string{msg.Author}, mailingLists...) + } + return sendMailText(c, msg.Subject, from, mailingLists, inReplyTo, body) } -func sendMailText(c context.Context, cfg *EmailConfig, subject, from string, to []string, replyTo, body string) error { +func sendMailText(c context.Context, subject, from string, to []string, replyTo, body string) error { msg := &aemail.Message{ Sender: from, To: to, Subject: subject, Body: body, } - if cfg.SubjectPrefix != "" { - msg.Subject = cfg.SubjectPrefix + " " + msg.Subject - } if replyTo != "" { msg.Headers = mail.Header{"In-Reply-To": []string{replyTo}} msg.Subject = replySubject(msg.Subject) diff --git a/dashboard/app/util_test.go b/dashboard/app/util_test.go index 014e34a57..f9e2ad660 100644 --- a/dashboard/app/util_test.go +++ b/dashboard/app/util_test.go @@ -669,7 +669,7 @@ Content-Type: text/plain %v `, sender, id, subject, from, strings.Join(cc, ","), to, origFrom, body) log.Infof(c.ctx, "sending %s", email) - _, err := c.POST("/_ah/mail/email@server.com", email) + _, err := c.POST("/_ah/mail/"+to, email) c.expectOK(err) } -- cgit mrf-deployment