From 3666edfeb55080ebe138d77417fa96fe2555d6bb Mon Sep 17 00:00:00 2001
From: Aleksandr Nogikh <nogikh@google.com>
Date: Mon, 30 May 2022 13:17:24 +0000
Subject: dashboard: never report unreliable bisections

Currently the dashboard does not report unreliable bisections
themselves, but there was a way in which they could still leak. The
recently detected case is when the bisection was done using a syz repro,
no reporting was made, but with a message about a newly found C repro
syzbot included the bisection info and Cc'd bisection-sourced emails.

Fix that behavior and introduce one new test.
---
 dashboard/app/bisect_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++++
 dashboard/app/reporting.go   | 15 +++++++++-----
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/dashboard/app/bisect_test.go b/dashboard/app/bisect_test.go
index 624f8f876..71a86198c 100644
--- a/dashboard/app/bisect_test.go
+++ b/dashboard/app/bisect_test.go
@@ -545,6 +545,54 @@ https://goo.gl/tpsmEJ#testing-patches`,
 	}
 }
 
+func TestUnreliableBisect(t *testing.T) {
+	c := NewCtx(t)
+	defer c.Close()
+
+	build := testBuild(1)
+	c.client2.UploadBuild(build)
+	// Upload a crash that has only a syz repro.
+	crash := testCrashWithRepro(build, 1)
+	crash.ReproC = nil
+	c.client2.ReportCrash(crash)
+	_ = c.client2.pollEmailBug()
+
+	pollResp := c.client2.pollJobs(build.Manager)
+	jobID := pollResp.ID
+	done := &dashapi.JobDoneReq{
+		ID:    jobID,
+		Build: *build,
+		Log:   []byte("bisect log"),
+		Flags: dashapi.BisectResultRelease,
+		Commits: []dashapi.Commit{
+			{
+				Hash:       "111111111111111111111111",
+				Title:      "Linux 4.10",
+				Author:     "abcd@kernel.org",
+				AuthorName: "Abcd Efgh",
+				CC:         []string{"reviewer1@kernel.org", "reviewer2@kernel.org"},
+				Date:       time.Date(2000, 2, 9, 4, 5, 6, 7, time.UTC),
+			},
+		},
+	}
+	done.Build.ID = jobID
+	c.expectOK(c.client2.JobDone(done))
+
+	// The bisection result is unreliable - it shouldn't be reported.
+	c.expectNoEmail()
+
+	// Upload a crash with a C repro.
+	crash2 := testCrashWithRepro(build, 1)
+	c.client2.ReportCrash(crash2)
+
+	// Make sure it doesn't mention bisection and doesn't include the emails from it.
+	msg := c.pollEmailBug()
+	c.expectEQ(msg.To, []string{"test@syzkaller.com"})
+	c.expectEQ(msg.Subject, crash.Title)
+	c.expectTrue(strings.Contains(msg.Body, "syzbot has found a reproducer for the following issue"))
+	c.expectTrue(!strings.Contains(msg.Body, "bisection"))
+}
+
 func TestBisectWrong(t *testing.T) {
 	// Test bisection results with BisectResultMerge/BisectResultNoop flags set.
 	// If any of these set, the result must not be reported separately,
diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go
index 5bd890fb2..c9556c34c 100644
--- a/dashboard/app/reporting.go
+++ b/dashboard/app/reporting.go
@@ -379,11 +379,16 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *db.Key
 		if err != nil {
 			return nil, err
 		}
-		job = job1
-		if crash1.ReproC != 0 || crash.ReproC == 0 {
-			// Don't override the crash in this case,
-			// otherwise we will always think that we haven't reported the C repro.
-			crash, crashKey = crash1, crashKey1
+		// If we didn't check whether the bisect is unreliable, even though it would not be
+		// reported anyway, we could still eventually Cc people from those commits later
+		// (e.g. when we did bisected with a syz repro and then notified about a C repro).
+		if !job1.isUnreliableBisect() {
+			job = job1
+			if crash1.ReproC != 0 || crash.ReproC == 0 {
+				// Don't override the crash in this case,
+				// otherwise we will always think that we haven't reported the C repro.
+				crash, crashKey = crash1, crashKey1
+			}
 		}
 	}
 	crashLog, _, err := getText(c, textCrashLog, crash.Log)
-- 
cgit mrf-deployment