From 22ae5830af1ab04e8042e534dd9baf750bc0d0ac Mon Sep 17 00:00:00 2001
From: Aleksandr Nogikh <nogikh@google.com>
Date: Thu, 6 Jul 2023 17:21:08 +0200
Subject: dashboard: don't overwrite user-set subsystems

Don't recalculate subsystems when a user has already specified them. Add
a test to verify this behavior.
---
 dashboard/app/api.go            |  9 ++++++---
 dashboard/app/subsystem_test.go | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/dashboard/app/api.go b/dashboard/app/api.go
index 54f1a04c4..6565b8690 100644
--- a/dashboard/app/api.go
+++ b/dashboard/app/api.go
@@ -770,9 +770,12 @@ func reportCrash(c context.Context, build *Build, req *dashapi.Crash) (*Bug, err
 	}
 
 	newSubsystems := []*subsystem.Subsystem{}
-	// Recalculate subsystems on the first saved crash and on the first saved repro.
-	calculateSubsystems := save && (bug.NumCrashes == 0 ||
-		bug.ReproLevel == ReproLevelNone && reproLevel != ReproLevelNone)
+	// Recalculate subsystems on the first saved crash and on the first saved repro,
+	// unless a user has already manually specified them.
+	calculateSubsystems := save &&
+		!bug.hasUserSubsystems() &&
+		(bug.NumCrashes == 0 ||
+			bug.ReproLevel == ReproLevelNone && reproLevel != ReproLevelNone)
 	if calculateSubsystems {
 		newSubsystems, err = inferSubsystems(c, bug, bugKey, &debugtracer.NullTracer{})
 		if err != nil {
diff --git a/dashboard/app/subsystem_test.go b/dashboard/app/subsystem_test.go
index 9eb6dde56..687ca35eb 100644
--- a/dashboard/app/subsystem_test.go
+++ b/dashboard/app/subsystem_test.go
@@ -208,6 +208,41 @@ func TestUserSubsystemsRefresh(t *testing.T) {
 	expectLabels(t, client, extID, "subsystems:subsystemB")
 }
 
+func TestNoUserSubsystemOverwrite(t *testing.T) {
+	c := NewCtx(t)
+	defer c.Close()
+
+	client := c.makeClient(clientPublicEmail, keyPublicEmail, true)
+
+	build := testBuild(1)
+	client.UploadBuild(build)
+
+	// Create a bug without subsystems.
+	crash := testCrash(build, 1)
+	client.ReportCrash(crash)
+	c.incomingEmail(c.pollEmailBug().Sender, "#syz upstream\n")
+
+	sender := c.pollEmailBug().Sender
+	_, extID, err := email.RemoveAddrContext(sender)
+	c.expectOK(err)
+
+	// Manually set subsystemA.
+	c.incomingEmail(sender, "#syz set subsystems: subsystemA\n",
+		EmailOptFrom("test@requester.com"))
+	expectLabels(t, client, extID, "subsystems:subsystemA")
+
+	// Now we find a reproducer that indicates it's subsystemB.
+
+	crash.GuiltyFiles = []string{"b.c"}
+	crash.ReproOpts = []byte("some opts")
+	crash.ReproSyz = []byte("getpid()")
+	client.ReportCrash(crash)
+	c.pollEmailBug()
+
+	// Make sure subsystem stayed unchanged.
+	expectLabels(t, client, extID, "subsystems:subsystemA")
+}
+
 // nolint: goconst
 func TestPeriodicSubsystemReminders(t *testing.T) {
 	c := NewCtx(t)
-- 
cgit mrf-deployment