dashboard/app: add uncc command
Add "#syz uncc" command as a safety handle.
The command allows sender to unsubscribe from all future communication on the bug.
Linus mentioned possibility of saying "I'm not the right person for this report"
in the context of bug reminders:
https://groups.google.com/d/msg/syzkaller/zYlQ-b-QPHQ/AJzpeObcBAAJ
diff --git a/dashboard/app/email_test.go b/dashboard/app/email_test.go
index 4cb2548..07a242e 100644
--- a/dashboard/app/email_test.go
+++ b/dashboard/app/email_test.go
@@ -22,7 +22,7 @@
c.client2.UploadBuild(build)
crash := testCrash(build, 1)
- crash.Maintainers = []string{`"Foo Bar" <foo@bar.com>`, `bar@foo.com`}
+ crash.Maintainers = []string{`"Foo Bar" <foo@bar.com>`, `bar@foo.com`, `idont@want.EMAILS`}
c.client2.ReportCrash(crash)
// Report the crash over email and check all fields.
@@ -52,7 +52,7 @@
kernel config: %[3]v
dashboard link: https://testapp.appspot.com/bug?extid=%[1]v
compiler: compiler1
-CC: [bar@foo.com foo@bar.com]
+CC: [bar@foo.com foo@bar.com idont@want.EMAILS]
Unfortunately, I don't have any reproducer for this crash yet.
@@ -109,6 +109,14 @@
// We used to extract "#syz fix: exact-commit-title" from it.
c.incomingEmail(sender0, body0)
+ c.incomingEmail(sender0, "I don't want emails", EmailOptFrom(`"idont" <idont@WANT.emails>`))
+ c.expectNoEmail()
+
+ // This person sends an email and is listed as a maintainer, but opt-out of emails.
+ // We should not send anything else to them for this bug. Also don't warn about no mailing list in CC.
+ c.incomingEmail(sender0, "#syz uncc", EmailOptFrom(`"IDONT" <Idont@want.emails>`), EmailOptCC(nil))
+ c.expectNoEmail()
+
// Now report syz reproducer and check updated email.
build2 := testBuild(10)
build2.Arch = "386"
@@ -310,8 +318,7 @@
// 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)
+ c.expectNoEmail()
// Check that the commit is now passed to builders.
builderPollResp, _ := c.client2.BuilderPoll(build.Manager)
@@ -361,9 +368,6 @@
#syz upstream
`, sender)
c.expectOK(c.POST("/_ah/mail/", incoming1))
-
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
}
// Basic dup scenario: mark one bug as dup of another.
@@ -383,24 +387,25 @@
c.client2.ReportCrash(crash2)
c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 2)
- msg1 := <-c.emailSink
- msg2 := <-c.emailSink
+ msg1 := c.pollEmailBug()
+ msg2 := c.pollEmailBug()
// Dup crash2 to crash1.
c.incomingEmail(msg2.Sender, "#syz dup: BUG: slightly more elaborate title")
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
// Second crash happens again
crash2.ReproC = []byte("int main() {}")
c.client2.ReportCrash(crash2)
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
// Now close the original bug, and check that new bugs for dup are now created.
c.incomingEmail(msg1.Sender, "#syz invalid")
+ // uncc command must not trugger error reply even for closed bug.
+ c.incomingEmail(msg1.Sender, "#syz uncc", EmailOptCC(nil))
+ c.expectNoEmail()
+
// New crash must produce new bug in the first reporting.
c.client2.ReportCrash(crash2)
{
@@ -425,25 +430,21 @@
c.client2.ReportCrash(crash2)
c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 2)
- msg1 := <-c.emailSink
- msg2 := <-c.emailSink
+ msg1 := c.pollEmailBug()
+ msg2 := c.pollEmailBug()
// Dup crash2 to crash1.
c.incomingEmail(msg2.Sender, "#syz dup: BUG: slightly more elaborate title")
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
// Undup crash2.
c.incomingEmail(msg2.Sender, "#syz undup")
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
// Now close the original bug, and check that new crashes for the dup does not create bugs.
c.incomingEmail(msg1.Sender, "#syz invalid")
c.client2.ReportCrash(crash2)
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
}
func TestEmailCrossReportingDup(t *testing.T) {
@@ -491,13 +492,9 @@
c.incomingEmail(bugSender, "#syz dup: "+crash2.Title)
if test.result {
- if len(c.emailSink) != 0 {
- msg := <-c.emailSink
- t.Fatalf("unexpected reply: %s\n%s\n", msg.Subject, msg.Body)
- }
+ c.expectNoEmail()
} else {
- c.expectEQ(len(c.emailSink), 1)
- msg := <-c.emailSink
+ msg := c.pollEmailBug()
if !strings.Contains(msg.Body, "> #syz dup:") ||
!strings.Contains(msg.Body, "Can't dup bug to a bug in different reporting") {
c.t.Fatalf("bad reply body:\n%v", msg.Body)
@@ -512,8 +509,7 @@
// No reply for email without bug hash and no commands.
c.incomingEmail("syzbot@testapp.appspotmail.com", "Investment Proposal")
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
// If email contains a command we need to reply.
c.incomingEmail("syzbot@testapp.appspotmail.com", "#syz invalid")
diff --git a/dashboard/app/entities.go b/dashboard/app/entities.go
index 32cf379..a3ab5c7 100644
--- a/dashboard/app/entities.go
+++ b/dashboard/app/entities.go
@@ -89,6 +89,7 @@
CommitInfo []Commit // additional info for commits (for historical reasons parallel array to Commits)
HappenedOn []string `datastore:",noindex"` // list of managers
PatchedOn []string `datastore:",noindex"` // list of managers
+ UNCC []string // don't CC these emails on this bug
}
type Commit struct {
diff --git a/dashboard/app/jobs.go b/dashboard/app/jobs.go
index 1bb67a9..7bff92b 100644
--- a/dashboard/app/jobs.go
+++ b/dashboard/app/jobs.go
@@ -572,10 +572,6 @@
if err != nil {
return nil, err
}
- kernelConfig, _, err := getText(c, textKernelConfig, build.KernelConfig)
- if err != nil {
- return nil, err
- }
bugKey := jobKey.Parent()
crashKey := datastore.NewKey(c, "Crash", "", job.CrashID, bugKey)
crash := new(Crash)
@@ -590,10 +586,6 @@
if bugReporting == nil {
return nil, fmt.Errorf("job bug has no reporting %q", job.Reporting)
}
- creditEmail, err := email.AddAddrContext(ownEmail(c), bugReporting.ID)
- if err != nil {
- return nil, err
- }
var typ dashapi.ReportType
switch job.Type {
case JobTestPatch:
@@ -606,39 +598,21 @@
return nil, fmt.Errorf("unknown job type %v", job.Type)
}
rep := &dashapi.BugReport{
- Type: typ,
- Namespace: job.Namespace,
- Config: reportingConfig,
- ID: bugReporting.ID,
- JobID: extJobID(jobKey),
- ExtID: job.ExtID,
- Title: bug.displayTitle(),
- Link: fmt.Sprintf("%v/bug?extid=%v", appURL(c), bugReporting.ID),
- CreditEmail: creditEmail,
- CC: job.CC,
- Log: crashLog,
- LogLink: externalLink(c, textCrashLog, job.CrashLog),
- Report: report,
- ReportLink: externalLink(c, textCrashReport, job.CrashReport),
- OS: build.OS,
- Arch: build.Arch,
- VMArch: build.VMArch,
- UserSpaceArch: kernelArch(build.Arch),
- CompilerID: build.CompilerID,
- KernelRepo: build.KernelRepo,
- KernelRepoAlias: kernelRepoInfo(build).Alias,
- KernelBranch: build.KernelBranch,
- KernelCommit: build.KernelCommit,
- KernelCommitTitle: build.KernelCommitTitle,
- KernelCommitDate: build.KernelCommitDate,
- KernelConfig: kernelConfig,
- KernelConfigLink: externalLink(c, textKernelConfig, build.KernelConfig),
- ReproCLink: externalLink(c, textReproC, crash.ReproC),
- ReproSyzLink: externalLink(c, textReproSyz, crash.ReproSyz),
- CrashTitle: job.CrashTitle,
- Error: jobError,
- ErrorLink: externalLink(c, textError, job.Error),
- PatchLink: externalLink(c, textPatch, job.Patch),
+ Type: typ,
+ Config: reportingConfig,
+ JobID: extJobID(jobKey),
+ ExtID: job.ExtID,
+ CC: job.CC,
+ Log: crashLog,
+ LogLink: externalLink(c, textCrashLog, job.CrashLog),
+ Report: report,
+ ReportLink: externalLink(c, textCrashReport, job.CrashReport),
+ ReproCLink: externalLink(c, textReproC, crash.ReproC),
+ ReproSyzLink: externalLink(c, textReproSyz, crash.ReproSyz),
+ CrashTitle: job.CrashTitle,
+ Error: jobError,
+ ErrorLink: externalLink(c, textError, job.Error),
+ PatchLink: externalLink(c, textPatch, job.Patch),
}
if job.Type == JobBisectCause || job.Type == JobBisectFix {
kernelRepo := kernelRepoInfo(build)
@@ -659,6 +633,9 @@
rep.Error = rep.Error[len(rep.Error)-maxInlineError:]
rep.ErrorTruncated = true
}
+ if err := fillBugReport(c, rep, bug, bugReporting, build); err != nil {
+ return nil, err
+ }
return rep, nil
}
diff --git a/dashboard/app/jobs_test.go b/dashboard/app/jobs_test.go
index a5783b6..73e3247 100644
--- a/dashboard/app/jobs_test.go
+++ b/dashboard/app/jobs_test.go
@@ -74,8 +74,7 @@
c.incomingEmail(sender, "#syz test: git://git.git/git.git kernel-branch\n"+patch,
EmailOptFrom("\"foo\" <blAcklisteD@dOmain.COM>"))
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
pollResp := c.client2.pollJobs(build.Manager)
c.expectEQ(pollResp.ID, "")
@@ -83,15 +82,13 @@
c.incomingEmail(sender, "#syz test: git://git.git/git.git kernel-branch\n"+patch,
EmailOptMessageID(1), EmailOptFrom("test@requester.com"),
EmailOptCC([]string{"somebody@else.com"}))
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
// A dup of the same request with the same Message-ID.
c.incomingEmail(sender, "#syz test: git://git.git/git.git kernel-branch\n"+patch,
EmailOptMessageID(1), EmailOptFrom("test@requester.com"),
EmailOptCC([]string{"somebody@else.com"}))
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
pollResp = c.client2.pollJobs("foobar")
c.expectEQ(pollResp.ID, "")
diff --git a/dashboard/app/notifications_test.go b/dashboard/app/notifications_test.go
index d59465e..12e9aae 100644
--- a/dashboard/app/notifications_test.go
+++ b/dashboard/app/notifications_test.go
@@ -27,8 +27,7 @@
// Upstreaming happens after 14 days, so no emails yet.
c.advanceTime(13 * 24 * time.Hour)
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
// Now we should get notification about upstreaming and upstream report:
c.advanceTime(2 * 24 * time.Hour)
@@ -54,8 +53,7 @@
c.expectEQ(report.To, []string{"test@syzkaller.com"})
// No emails yet.
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
// Now upload repro and it should be auto-upstreamed.
crash.ReproOpts = []byte("repro opts")
@@ -82,16 +80,13 @@
c.expectEQ(report.To, []string{"test@syzkaller.com"})
c.incomingEmail(report.Sender, "#syz fix: some: commit title")
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
// Notification about bad fixing commit should be send after 90 days.
c.advanceTime(50 * 24 * time.Hour)
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
c.advanceTime(35 * 24 * time.Hour)
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
c.advanceTime(10 * 24 * time.Hour)
notif := c.pollEmailBug()
if !strings.Contains(notif.Body, "This bug is marked as fixed by commit:\nsome: commit title\n") {
@@ -99,8 +94,7 @@
}
// No notifications for another 14 days, then another one.
c.advanceTime(13 * 24 * time.Hour)
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
c.advanceTime(2 * 24 * time.Hour)
notif = c.pollEmailBug()
if !strings.Contains(notif.Body, "This bug is marked as fixed by commit:\nsome: commit title\n") {
@@ -127,13 +121,11 @@
// Bug is open, new crashes don't create new bug.
c.client2.ReportCrash(crash)
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
// Not yet.
c.advanceTime(179 * 24 * time.Hour)
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
// Now!
c.advanceTime(2 * 24 * time.Hour)
@@ -199,8 +191,7 @@
report4 = c.pollEmailBug()
c.advanceTime(179 * 24 * time.Hour)
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
c.client2.ReportCrash(crash2)
c.incomingEmail(report3.Sender, "I am looking at it")
@@ -209,8 +200,7 @@
// Only crash 4 is obsoleted.
notif := c.pollEmailBug()
c.expectEQ(notif.Sender, report4.Sender)
- c.expectOK(c.GET("/email_poll"))
- c.expectEQ(len(c.emailSink), 0)
+ c.expectNoEmail()
// Crash 3 also obsoleted after some time.
c.advanceTime(20 * 24 * time.Hour)
diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go
index 1467f63..31306b5 100644
--- a/dashboard/app/reporting.go
+++ b/dashboard/app/reporting.go
@@ -343,14 +343,6 @@
if err != nil {
return nil, err
}
- kernelConfig, _, err := getText(c, textKernelConfig, build.KernelConfig)
- if err != nil {
- return nil, err
- }
- creditEmail, err := email.AddAddrContext(ownEmail(c), bugReporting.ID)
- if err != nil {
- return nil, err
- }
typ := dashapi.ReportNew
if !bugReporting.Reported.IsZero() {
typ = dashapi.ReportRepro
@@ -358,41 +350,23 @@
kernelRepo := kernelRepoInfo(build)
rep := &dashapi.BugReport{
- Type: typ,
- Namespace: bug.Namespace,
- Config: reportingConfig,
- ID: bugReporting.ID,
- ExtID: bugReporting.ExtID,
- First: bugReporting.Reported.IsZero(),
- Moderation: reporting.moderation,
- Title: bug.displayTitle(),
- Link: fmt.Sprintf("%v/bug?extid=%v", appURL(c), bugReporting.ID),
- CreditEmail: creditEmail,
- Log: crashLog,
- LogLink: externalLink(c, textCrashLog, crash.Log),
- Report: report,
- ReportLink: externalLink(c, textCrashReport, crash.Report),
- Maintainers: append(crash.Maintainers, kernelRepo.CC...),
- OS: build.OS,
- Arch: build.Arch,
- VMArch: build.VMArch,
- UserSpaceArch: kernelArch(build.Arch),
- CompilerID: build.CompilerID,
- KernelRepo: build.KernelRepo,
- KernelRepoAlias: kernelRepo.Alias,
- KernelBranch: build.KernelBranch,
- KernelCommit: build.KernelCommit,
- KernelCommitTitle: build.KernelCommitTitle,
- KernelCommitDate: build.KernelCommitDate,
- KernelConfig: kernelConfig,
- KernelConfigLink: externalLink(c, textKernelConfig, build.KernelConfig),
- ReproC: reproC,
- ReproCLink: externalLink(c, textReproC, crash.ReproC),
- ReproSyz: reproSyz,
- ReproSyzLink: externalLink(c, textReproSyz, crash.ReproSyz),
- CrashID: crashKey.IntID(),
- NumCrashes: bug.NumCrashes,
- HappenedOn: managersToRepos(c, bug.Namespace, bug.HappenedOn),
+ Type: typ,
+ Config: reportingConfig,
+ ExtID: bugReporting.ExtID,
+ First: bugReporting.Reported.IsZero(),
+ Moderation: reporting.moderation,
+ Log: crashLog,
+ LogLink: externalLink(c, textCrashLog, crash.Log),
+ Report: report,
+ ReportLink: externalLink(c, textCrashReport, crash.Report),
+ Maintainers: append(crash.Maintainers, kernelRepo.CC...),
+ ReproC: reproC,
+ ReproCLink: externalLink(c, textReproC, crash.ReproC),
+ ReproSyz: reproSyz,
+ ReproSyzLink: externalLink(c, textReproSyz, crash.ReproSyz),
+ CrashID: crashKey.IntID(),
+ NumCrashes: bug.NumCrashes,
+ HappenedOn: managersToRepos(c, bug.Namespace, bug.HappenedOn),
}
if bugReporting.CC != "" {
rep.CC = strings.Split(bugReporting.CC, "|")
@@ -400,9 +374,48 @@
if bug.BisectCause == BisectYes {
rep.BisectCause = bisectFromJob(c, rep, job)
}
+ if err := fillBugReport(c, rep, bug, bugReporting, build); err != nil {
+ return nil, err
+ }
return rep, nil
}
+// fillBugReport fills common report fields for bug and job reports.
+func fillBugReport(c context.Context, rep *dashapi.BugReport, bug *Bug, bugReporting *BugReporting,
+ build *Build) error {
+ kernelConfig, _, err := getText(c, textKernelConfig, build.KernelConfig)
+ if err != nil {
+ return err
+ }
+ creditEmail, err := email.AddAddrContext(ownEmail(c), bugReporting.ID)
+ if err != nil {
+ return err
+ }
+ rep.Namespace = bug.Namespace
+ rep.ID = bugReporting.ID
+ rep.Title = bug.displayTitle()
+ rep.Link = fmt.Sprintf("%v/bug?extid=%v", appURL(c), bugReporting.ID)
+ rep.CreditEmail = creditEmail
+ rep.OS = build.OS
+ rep.Arch = build.Arch
+ rep.VMArch = build.VMArch
+ rep.UserSpaceArch = kernelArch(build.Arch)
+ rep.CompilerID = build.CompilerID
+ rep.KernelRepo = build.KernelRepo
+ rep.KernelRepoAlias = kernelRepoInfo(build).Alias
+ rep.KernelBranch = build.KernelBranch
+ rep.KernelCommit = build.KernelCommit
+ rep.KernelCommitTitle = build.KernelCommitTitle
+ rep.KernelCommitDate = build.KernelCommitDate
+ rep.KernelConfig = kernelConfig
+ rep.KernelConfigLink = externalLink(c, textKernelConfig, build.KernelConfig)
+ for _, addr := range bug.UNCC {
+ rep.CC = email.RemoveFromEmailList(rep.CC, addr)
+ rep.Maintainers = email.RemoveFromEmailList(rep.Maintainers, addr)
+ }
+ return nil
+}
+
func loadBisectJob(c context.Context, bug *Bug) (*Job, *Crash, *datastore.Key, *datastore.Key, error) {
bugKey := bug.key(c)
var jobs []*Job
@@ -679,7 +692,7 @@
if bugReporting.Link == "" {
bugReporting.Link = cmd.Link
}
- if len(cmd.CC) != 0 {
+ if len(cmd.CC) != 0 && cmd.Status != dashapi.BugStatusUnCC {
merged := email.MergeEmailLists(strings.Split(bugReporting.CC, "|"), cmd.CC)
bugReporting.CC = strings.Join(merged, "|")
}
@@ -755,6 +768,8 @@
bug.DupOf = dupHash
case dashapi.BugStatusUpdate:
// Just update Link, Commits, etc below.
+ case dashapi.BugStatusUnCC:
+ bug.UNCC = email.MergeEmailLists(bug.UNCC, cmd.CC)
default:
return false, internalError, fmt.Errorf("unknown bug status %v", cmd.Status)
}
diff --git a/dashboard/app/reporting_email.go b/dashboard/app/reporting_email.go
index 7d06eea..d05b0cd 100644
--- a/dashboard/app/reporting_email.go
+++ b/dashboard/app/reporting_email.go
@@ -326,6 +326,9 @@
}
cmd.Status = dashapi.BugStatusDup
cmd.DupOf = msg.CommandArgs
+ case "uncc", "uncc:":
+ cmd.Status = dashapi.BugStatusUnCC
+ cmd.CC = []string{email.CanonicalEmail(msg.From)}
default:
return replyTo(c, msg, fmt.Sprintf("unknown command %q", msg.Command), nil)
}
@@ -336,7 +339,7 @@
if !ok && reply != "" {
return replyTo(c, msg, reply, nil)
}
- if !mailingListInCC && msg.Command != "" {
+ if !mailingListInCC && msg.Command != "" && cmd.Status != dashapi.BugStatusUnCC {
warnMailingListInCC(c, msg, mailingList)
}
return nil
diff --git a/dashboard/app/util_test.go b/dashboard/app/util_test.go
index 74540d5..fbe11b0 100644
--- a/dashboard/app/util_test.go
+++ b/dashboard/app/util_test.go
@@ -252,6 +252,14 @@
return <-c.emailSink
}
+func (c *Ctx) expectNoEmail() {
+ c.expectOK(c.GET("/email_poll"))
+ if len(c.emailSink) != 0 {
+ msg := <-c.emailSink
+ c.t.Fatalf("\n%v: got expected email: %v\n%s", caller(0), msg.Subject, msg.Body)
+ }
+}
+
type apiClient struct {
*Ctx
*dashapi.Dashboard
diff --git a/dashboard/dashapi/dashapi.go b/dashboard/dashapi/dashapi.go
index 42200eb..d537864 100644
--- a/dashboard/dashapi/dashapi.go
+++ b/dashboard/dashapi/dashapi.go
@@ -469,6 +469,7 @@
BugStatusInvalid
BugStatusDup
BugStatusUpdate // aux info update (i.e. ExtID/Link/CC)
+ BugStatusUnCC // don't CC sender on any future communication
)
const (
diff --git a/pkg/email/parser.go b/pkg/email/parser.go
index 1b2dbc7..ce652c5 100644
--- a/pkg/email/parser.go
+++ b/pkg/email/parser.go
@@ -330,3 +330,14 @@
}
return result
}
+
+func RemoveFromEmailList(list []string, toRemove string) []string {
+ var result []string
+ toRemove = CanonicalEmail(toRemove)
+ for _, email := range list {
+ if CanonicalEmail(email) != toRemove {
+ result = append(result, email)
+ }
+ }
+ return result
+}
