dashboard/app: remember job links

Remember link for jobs.
Check that mailing list is in CC when we accept commands.

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: <abcdef>
-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 @@
 		</tr>
 		{{range $job := $.Jobs}}
 			<tr>
-				<td class="time">{{formatTime $job.Created}}</td>
+				<td class="time">{{if $job.Link}}<a href="{{$job.Link}}">{{formatTime $job.Created}}</a>{{else}}{{formatTime $job.Created}}{{end}}</td>
 				<td class="time">{{formatTime $job.Started}}{{if gt $job.Attempts 1}} ({{$job.Attempts}}){{end}}</td>
 				<td class="time">{{formatTime $job.Finished}}</td>
 				<td>{{$job.User}}</td>
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