From e93131fcce06b64b89ccf4fa166b82dec6e7480d Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Sat, 18 Nov 2017 18:54:17 +0100 Subject: dashboard/app: remember job links Remember link for jobs. Check that mailing list is in CC when we accept commands. --- dashboard/app/email_test.go | 52 ++---------------- dashboard/app/entities.go | 1 + dashboard/app/jobs.go | 33 ++++++++++-- dashboard/app/main.go | 2 + dashboard/app/main.html | 2 +- dashboard/app/reporting.go | 17 +++--- dashboard/app/reporting_email.go | 112 ++++++++++++++++++++++++++++++++------- dashboard/app/util_test.go | 1 + docs/syzbot.md | 2 + 9 files changed, 140 insertions(+), 82 deletions(-) diff --git a/dashboard/app/email_test.go b/dashboard/app/email_test.go index dcf3c4b51..0ebf41186 100644 --- a/dashboard/app/email_test.go +++ b/dashboard/app/email_test.go @@ -155,19 +155,7 @@ report1 } // Now upstream the bug and check that it reaches the next reporting. - incoming2 := fmt.Sprintf(`Sender: syzkaller@googlegroups.com -Date: Tue, 15 Aug 2017 14:59:00 -0700 -Message-ID: <1234> -Subject: crash1 -From: foo@bar.com -To: foo@bar.com -Cc: %v -Content-Type: text/plain - -#syz upstream -`, sender0) - - c.expectOK(c.POST("/_ah/mail/", incoming2)) + c.incomingEmail(sender0, "#syz upstream") sender1 := "" { @@ -317,18 +305,8 @@ unknown command "bad-command" } } - // Now mark the bug as invalid. - incoming5 := fmt.Sprintf(`Sender: syzkaller@googlegroups.com -Date: Tue, 15 Aug 2017 14:59:00 -0700 -Message-ID: -Subject: title1 -From: foo@bar.com -To: %v -Content-Type: text/plain - -#syz fix: some: commit title -`, sender1) - c.expectOK(c.POST("/_ah/mail/", incoming5)) + // Now mark the bug as fixed. + c.incomingEmail(sender1, "#syz fix: some: commit title") c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 0) @@ -410,17 +388,7 @@ func TestEmailDup(t *testing.T) { msg2 := <-c.emailSink // Dup crash2 to crash1. - incoming1 := fmt.Sprintf(`Sender: syzkaller@googlegroups.com -Date: Tue, 15 Aug 2017 14:59:00 -0700 -Message-ID: <12345> -Subject: title1 -From: foo@bar.com -To: %v -Content-Type: text/plain - -#syz dup: BUG: slightly more elaborate title -`, msg2.Sender) - c.expectOK(c.POST("/_ah/mail/", incoming1)) + c.incomingEmail(msg2.Sender, "#syz dup: BUG: slightly more elaborate title") c.expectOK(c.GET("/email_poll")) c.expectEQ(len(c.emailSink), 0) @@ -431,17 +399,7 @@ Content-Type: text/plain c.expectEQ(len(c.emailSink), 0) // Now close the original bug, and check that new bugs for dup are now created. - incoming2 := fmt.Sprintf(`Sender: syzkaller@googlegroups.com -Date: Tue, 15 Aug 2017 14:59:00 -0700 -Message-ID: <12345> -Subject: title1 -From: foo@bar.com -To: %v -Content-Type: text/plain - -#syz invalid -`, msg1.Sender) - c.expectOK(c.POST("/_ah/mail/", incoming2)) + c.incomingEmail(msg1.Sender, "#syz invalid") // New crash must produce new bug in the first reporting. c.expectOK(c.API(client2, key2, "report_crash", crash2, nil)) diff --git a/dashboard/app/entities.go b/dashboard/app/entities.go index 82b3c3003..0ece3deef 100644 --- a/dashboard/app/entities.go +++ b/dashboard/app/entities.go @@ -106,6 +106,7 @@ type Job struct { User string Reporting string ExtID string // email Message-ID + Link string // web link for the job (e.g. email in the group) Namespace string Manager string BugTitle string diff --git a/dashboard/app/jobs.go b/dashboard/app/jobs.go index 529421efe..94f1400d8 100644 --- a/dashboard/app/jobs.go +++ b/dashboard/app/jobs.go @@ -38,8 +38,7 @@ func addTestJob(c context.Context, bugID, user, extID, patch, repo, branch strin if err != nil { return "can't find associated bug", err } - now := timeNow(c) - bugReporting, _ := bugReportingByID(bug, bugID, now) + bugReporting, _ := bugReportingByID(bug, bugID) // TODO(dvyukov): find the exact crash that we reported. crash, crashKey, err := findCrashForBug(c, bug) @@ -81,7 +80,7 @@ func addTestJob(c context.Context, bugID, user, extID, patch, repo, branch strin } job := &Job{ - Created: now, + Created: timeNow(c), User: user, Reporting: bugReporting.Name, ExtID: extID, @@ -124,6 +123,32 @@ func addTestJob(c context.Context, bugID, user, extID, patch, repo, branch strin return "", nil } +func updateTestJob(c context.Context, extID, link string) error { + var jobs []*Job + keys, err := datastore.NewQuery("Job"). + Filter("ExtID=", extID). + GetAll(c, &jobs) + if len(jobs) != 1 || err != nil { + return fmt.Errorf("failed to query jobs: jobs=%v err=%v", len(jobs), err) + } + job, jobKey := jobs[0], keys[0] + if job.Link != "" { + return nil + } + tx := func(c context.Context) error { + job := new(Job) + if err := datastore.Get(c, jobKey, job); err != nil { + return err + } + job.Link = link + if _, err := datastore.Put(c, jobKey, job); err != nil { + return err + } + return nil + } + return datastore.RunInTransaction(c, tx, nil) +} + // pollPendingJobs returns the next job to execute for the provided list of managers. func pollPendingJobs(c context.Context, managers []string) (interface{}, error) { retry: @@ -318,7 +343,7 @@ func createBugReportForJob(c context.Context, job *Job, jobKey *datastore.Key, c Config: reportingConfig, ID: bugReporting.ID, JobID: extJobID(jobKey), - ExtID: bugReporting.ExtID, + ExtID: job.ExtID, Title: bug.displayTitle(), Log: crashLog, Report: report, diff --git a/dashboard/app/main.go b/dashboard/app/main.go index 7f93e7b02..68734f1ca 100644 --- a/dashboard/app/main.go +++ b/dashboard/app/main.go @@ -77,6 +77,7 @@ type uiCrash struct { type uiJob struct { Created time.Time + Link string User string Reporting string Namespace string @@ -298,6 +299,7 @@ func loadRecentJobs(c context.Context) ([]*uiJob, error) { for i, job := range jobs { ui := &uiJob{ Created: job.Created, + Link: job.Link, User: job.User, Reporting: job.Reporting, Namespace: job.Namespace, diff --git a/dashboard/app/main.html b/dashboard/app/main.html index 56ee285ee..ea1eec7c9 100644 --- a/dashboard/app/main.html +++ b/dashboard/app/main.html @@ -55,7 +55,7 @@ {{range $job := $.Jobs}} - {{formatTime $job.Created}} + {{if $job.Link}}{{formatTime $job.Created}}{{else}}{{formatTime $job.Created}}{{end}} {{formatTime $job.Started}}{{if gt $job.Attempts 1}} ({{$job.Attempts}}){{end}} {{formatTime $job.Finished}} {{$job.User}} diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go index 3e5c4a10b..bfb8a02d9 100644 --- a/dashboard/app/reporting.go +++ b/dashboard/app/reporting.go @@ -244,13 +244,9 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, bugReporting *Bu func incomingCommand(c context.Context, cmd *dashapi.BugUpdate) (bool, string, error) { log.Infof(c, "got command: %+q", cmd) ok, reason, err := incomingCommandImpl(c, cmd) - if !ok { - log.Errorf(c, "%v", reason) - } if err != nil { - log.Errorf(c, "%v", err) - } - if !ok { + log.Errorf(c, "%v (%v)", reason, err) + } else if !ok { log.Warningf(c, "invalid update: %v", reason) } return ok, reason, err @@ -264,7 +260,7 @@ func incomingCommandImpl(c context.Context, cmd *dashapi.BugUpdate) (bool, strin now := timeNow(c) dupHash := "" if cmd.Status == dashapi.BugStatusDup { - bugReporting, _ := bugReportingByID(bug, cmd.ID, now) + bugReporting, _ := bugReportingByID(bug, cmd.ID) dup, dupKey, err := findBugByReportingID(c, cmd.DupOf) if err != nil { // Email reporting passes bug title in cmd.DupOf, try to find bug by title. @@ -279,7 +275,7 @@ func incomingCommandImpl(c context.Context, cmd *dashapi.BugUpdate) (bool, strin } cmd.DupOf = dupReporting.ID } - dupReporting, _ := bugReportingByID(dup, cmd.DupOf, now) + dupReporting, _ := bugReportingByID(dup, cmd.DupOf) if bugReporting == nil || dupReporting == nil { return false, internalError, fmt.Errorf("can't find bug reporting") } @@ -356,7 +352,7 @@ func incomingCommandTx(c context.Context, now time.Time, cmd *dashapi.BugUpdate, default: return false, internalError, fmt.Errorf("unknown bug status %v", bug.Status) } - bugReporting, final := bugReportingByID(bug, cmd.ID, now) + bugReporting, final := bugReportingByID(bug, cmd.ID) if bugReporting == nil { return false, internalError, fmt.Errorf("can't find bug reporting") } @@ -492,12 +488,11 @@ func findDupByTitle(c context.Context, ns, title string) (*Bug, *datastore.Key, return bug, bugKey, nil } -func bugReportingByID(bug *Bug, id string, now time.Time) (*BugReporting, bool) { +func bugReportingByID(bug *Bug, id string) (*BugReporting, bool) { for i := range bug.Reporting { if bug.Reporting[i].ID == id { return &bug.Reporting[i], i == len(bug.Reporting)-1 } - bug.Reporting[i].Closed = now } return nil, false } diff --git a/dashboard/app/reporting_email.go b/dashboard/app/reporting_email.go index 3a3cb3437..fcafd1e9f 100644 --- a/dashboard/app/reporting_email.go +++ b/dashboard/app/reporting_email.go @@ -224,15 +224,45 @@ 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) - // 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)] { + bug, bugReporting, reporting := loadBugInfo(c, msg) + if bug == nil { + return nil // error was already logged + } + _ = bugReporting + emailConfig := reporting.Config.(*EmailConfig) + mailingList := email.CanonicalEmail(emailConfig.Email) + fromMailingList := email.CanonicalEmail(msg.From) == mailingList + mailingListInCC := checkMailingListInCC(c, msg, mailingList) + // 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. + if msg.Command == "test:" { + if fromMailingList { + if msg.Link != "" { + if err := updateTestJob(c, msg.MessageID, msg.Link); err != nil { + log.Errorf(c, "failed to update job: %v", err) + } + } + return nil + } + args := strings.Split(msg.CommandArgs, " ") + if len(args) != 2 { + return replyTo(c, msg, fmt.Sprintf("want 2 args (repo, branch), got %v", + len(args)), nil) + } + reply := handleTestRequest(c, msg.BugID, email.CanonicalEmail(msg.From), + msg.MessageID, msg.Patch, args[0], args[1]) + if reply != "" { + return replyTo(c, msg, reply, nil) + } + if !mailingListInCC { + warnMailingListInCC(c, msg, mailingList) + } + return nil + } + if fromMailingList && msg.Command != "" { log.Infof(c, "duplicate email from mailing list, ignoring") return nil } - // TODO(dvyukov): check that our mailing list is in CC - // (otherwise there will be no history of what hsppened with a bug). cmd := &dashapi.BugUpdate{ ID: msg.BugID, ExtID: msg.MessageID, @@ -258,19 +288,6 @@ func incomingMail(c context.Context, r *http.Request) error { } cmd.Status = dashapi.BugStatusDup cmd.DupOf = msg.CommandArgs - case "test:": - // TODO(dvyukov): remember email link for jobs. - args := strings.Split(msg.CommandArgs, " ") - if len(args) != 2 { - return replyTo(c, msg, fmt.Sprintf("want 2 args (repo, branch), got %v", - len(args)), nil) - } - reply := handleTestRequest(c, msg.BugID, email.CanonicalEmail(msg.From), - msg.MessageID, msg.Patch, args[0], args[1]) - if reply != "" { - return replyTo(c, msg, reply, nil) - } - return nil default: return replyTo(c, msg, fmt.Sprintf("unknown command %q", msg.Command), nil) } @@ -281,9 +298,66 @@ func incomingMail(c context.Context, r *http.Request) error { if !ok && reply != "" { return replyTo(c, msg, reply, nil) } + if !mailingListInCC && msg.Command != "" { + warnMailingListInCC(c, msg, mailingList) + } return nil } +func loadBugInfo(c context.Context, msg *email.Email) (bug *Bug, bugReporting *BugReporting, reporting *Reporting) { + if msg.BugID == "" { + log.Warningf(c, "no bug ID") + return nil, nil, nil + } + bug, _, err := findBugByReportingID(c, msg.BugID) + if err != nil { + log.Errorf(c, "can't find bug: %v", err) + replyTo(c, msg, "Can't find the corresponding bug.", nil) + return nil, nil, nil + } + bugReporting, _ = bugReportingByID(bug, msg.BugID) + if bugReporting == nil { + log.Errorf(c, "can't find bug reporting: %v", err) + replyTo(c, msg, "Can't find the corresponding bug.", nil) + return nil, nil, nil + } + reporting = config.Namespaces[bug.Namespace].ReportingByName(bugReporting.Name) + if reporting == nil { + log.Errorf(c, "can't find reporting for this bug: namespace=%q reporting=%q", + bug.Namespace, bugReporting.Name) + return nil, nil, nil + } + if reporting.Config.Type() != emailType { + log.Errorf(c, "reporting is not email: namespace=%q reporting=%q config=%q", + bug.Namespace, bugReporting.Name, reporting.Config.Type()) + return nil, nil, nil + } + return bug, bugReporting, reporting +} + +func checkMailingListInCC(c context.Context, msg *email.Email, mailingList string) bool { + if email.CanonicalEmail(msg.From) == mailingList { + return true + } + for _, cc := range msg.Cc { + if email.CanonicalEmail(cc) == mailingList { + return true + } + } + msg.Cc = append(msg.Cc, mailingList) + return false +} + +func warnMailingListInCC(c context.Context, msg *email.Email, mailingList string) { + reply := fmt.Sprintf("Your '%v' command is 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.", + msg.Command, mailingList) + if err := replyTo(c, msg, reply, nil); err != nil { + log.Errorf(c, "failed to send email reply: %v", err) + } +} + var mailTemplates = template.Must(template.New("").ParseGlob("mail_*.txt")) func sendMailTemplate(c context.Context, subject, from string, to []string, replyTo string, diff --git a/dashboard/app/util_test.go b/dashboard/app/util_test.go index 5bfd96119..5c868b1b5 100644 --- a/dashboard/app/util_test.go +++ b/dashboard/app/util_test.go @@ -175,6 +175,7 @@ Date: Tue, 15 Aug 2017 14:59:00 -0700 Message-ID: <1234> Subject: crash1 From: default@sender.com +Cc: test@syzkaller.com, bugs@syzkaller.com To: %v Content-Type: text/plain diff --git a/docs/syzbot.md b/docs/syzbot.md index 21b231887..3d532a8a8 100644 --- a/docs/syzbot.md +++ b/docs/syzbot.md @@ -41,6 +41,8 @@ reliable because of email clients splitting lines and messing with whitespaces. Note: if the crash happens again, it will cause creation of a new bug report. Note: all commands must start from beginning of the line. +Note: please keep `syzkaller-bugs@googlegroups.com` mailing list in CC. +It serves as a history of what happened with each bug report. ## syzkaller reproducers -- cgit mrf-deployment