diff options
| author | Dmitry Vyukov <dvyukov@google.com> | 2020-09-14 14:16:02 +0200 |
|---|---|---|
| committer | Dmitry Vyukov <dvyukov@google.com> | 2020-09-14 15:00:02 +0200 |
| commit | 3e84253bf41d63c55f92679b1aab9102f2f4949a (patch) | |
| tree | 53603d090d5d58a7b4b57bad6e86a07c3f2dd013 | |
| parent | 9eff3337ee5c407d4156eccb7bdea4d666e492fc (diff) | |
dashboard/app: fix up machine info handling
Assorted local fixes fixes, like dedup machine info in the database,
fix up HTML table markup, enforce and check access to the machine info
entities, etc.
Follow up to #2085
Fixes #466
| -rw-r--r-- | dashboard/app/access.go | 5 | ||||
| -rw-r--r-- | dashboard/app/access_test.go | 35 | ||||
| -rw-r--r-- | dashboard/app/api.go | 2 | ||||
| -rw-r--r-- | dashboard/app/app_test.go | 9 | ||||
| -rw-r--r-- | dashboard/app/main.go | 5 | ||||
| -rw-r--r-- | dashboard/app/reporting_test.go | 5 | ||||
| -rw-r--r-- | dashboard/app/templates.html | 3 |
7 files changed, 45 insertions, 19 deletions
diff --git a/dashboard/app/access.go b/dashboard/app/access.go index a58de77aa..9517cfad0 100644 --- a/dashboard/app/access.go +++ b/dashboard/app/access.go @@ -98,6 +98,11 @@ func checkTextAccess(c context.Context, r *http.Request, tag string, id int64) ( return checkCrashTextAccess(c, r, "ReproSyz", id) case textReproC: return checkCrashTextAccess(c, r, "ReproC", id) + case textMachineInfo: + // MachineInfo is deduplicated, so we can't find the exact crash/bug. + // But since machine info is usually the same for all bugs and is not secret, + // it's fine to check based on the namespace. + return nil, nil, nil } } diff --git a/dashboard/app/access_test.go b/dashboard/app/access_test.go index d83178c9f..bef53b7bb 100644 --- a/dashboard/app/access_test.go +++ b/dashboard/app/access_test.go @@ -108,7 +108,7 @@ func TestAccess(t *testing.T) { } // noteBugAccessLevel collects all entities associated with the extID bug. - noteBugAccessLevel := func(extID string, level AccessLevel) { + noteBugAccessLevel := func(extID string, level, nsLevel AccessLevel) { bug, _, err := findBugByReportingID(c.ctx, extID) c.expectOK(err) crash, _, err := findCrashForBug(c.ctx, bug) @@ -174,11 +174,22 @@ func TestAccess(t *testing.T) { url: fmt.Sprintf("/text?tag=ReproSyz&x=%v", strconv.FormatUint(uint64(crash.ReproSyz), 16)), }, + { + level: nsLevel, + ref: fmt.Sprint(crash.MachineInfo), + url: fmt.Sprintf("/text?tag=MachineInfo&id=%v", crash.MachineInfo), + }, + { + level: nsLevel, + ref: fmt.Sprint(crash.MachineInfo), + url: fmt.Sprintf("/text?tag=MachineInfo&x=%v", + strconv.FormatUint(uint64(crash.MachineInfo), 16)), + }, }...) } - // noteBuildccessLevel collects all entities associated with the kernel build buildID. - noteBuildccessLevel := func(ns, buildID string) { + // noteBuildAccessLevel collects all entities associated with the kernel build buildID. + noteBuildAccessLevel := func(ns, buildID string) { build, err := loadBuild(c.ctx, ns, buildID) c.expectOK(err) entities = append(entities, entity{ @@ -211,12 +222,13 @@ func TestAccess(t *testing.T) { for k, v := range config.Namespaces[ns].Clients { clientName, clientKey = k, v } - namespaceAccessPrefix := accessLevelPrefix(config.Namespaces[ns].AccessLevel) + nsLevel := config.Namespaces[ns].AccessLevel + namespaceAccessPrefix := accessLevelPrefix(nsLevel) client := c.makeClient(clientName, clientKey, true) build := testBuild(1) build.KernelConfig = []byte(namespaceAccessPrefix + "build") client.UploadBuild(build) - noteBuildccessLevel(ns, build.ID) + noteBuildAccessLevel(ns, build.ID) for reportingIdx := 0; reportingIdx < 2; reportingIdx++ { accessLevel := config.Namespaces[ns].Reporting[reportingIdx].AccessLevel @@ -233,7 +245,7 @@ func TestAccess(t *testing.T) { // Invalid bugs become visible up to the last reporting. finalLevel := config.Namespaces[ns]. Reporting[len(config.Namespaces[ns].Reporting)-1].AccessLevel - noteBugAccessLevel(repInvalid.ID, finalLevel) + noteBugAccessLevel(repInvalid.ID, finalLevel, nsLevel) crashFixed := testCrashWithRepro(build, reportingIdx*10+0) client.ReportCrash(crashFixed) @@ -254,22 +266,23 @@ func TestAccess(t *testing.T) { buildFixing.Manager = build.Manager buildFixing.Commits = []string{ns + "-patch0"} client.UploadBuild(buildFixing) - noteBuildccessLevel(ns, buildFixing.ID) + noteBuildAccessLevel(ns, buildFixing.ID) // Fixed bugs are also visible up to the last reporting. - noteBugAccessLevel(repFixed.ID, finalLevel) + noteBugAccessLevel(repFixed.ID, finalLevel, nsLevel) crashOpen := testCrashWithRepro(build, reportingIdx*10+0) crashOpen.Log = []byte(accessPrefix + "log") crashOpen.Report = []byte(accessPrefix + "report") crashOpen.ReproC = []byte(accessPrefix + "repro c") crashOpen.ReproSyz = []byte(accessPrefix + "repro syz") + crashOpen.MachineInfo = []byte(ns + "machine info") client.ReportCrash(crashOpen) repOpen := client.pollBug() if reportingIdx != 0 { client.updateBug(repOpen.ID, dashapi.BugStatusUpstream, "") repOpen = client.pollBug() } - noteBugAccessLevel(repOpen.ID, accessLevel) + noteBugAccessLevel(repOpen.ID, accessLevel, nsLevel) crashPatched := testCrashWithRepro(build, reportingIdx*10+1) client.ReportCrash(crashPatched) @@ -287,7 +300,7 @@ func TestAccess(t *testing.T) { }) c.expectEQ(reply.OK, true) // Patched bugs are also visible up to the last reporting. - noteBugAccessLevel(repPatched.ID, finalLevel) + noteBugAccessLevel(repPatched.ID, finalLevel, nsLevel) crashDup := testCrashWithRepro(build, reportingIdx*10+2) client.ReportCrash(crashDup) @@ -297,7 +310,7 @@ func TestAccess(t *testing.T) { repDup = client.pollBug() } client.updateBug(repDup.ID, dashapi.BugStatusDup, repOpen.ID) - noteBugAccessLevel(repDup.ID, accessLevel) + noteBugAccessLevel(repDup.ID, accessLevel, nsLevel) } } diff --git a/dashboard/app/api.go b/dashboard/app/api.go index 12f3c7e1c..6492f1d98 100644 --- a/dashboard/app/api.go +++ b/dashboard/app/api.go @@ -810,7 +810,7 @@ func saveCrash(c context.Context, ns string, req *dashapi.Crash, bugKey *db.Key, if crash.ReproC, err = putText(c, ns, textReproC, req.ReproC, false); err != nil { return err } - if crash.MachineInfo, err = putText(c, ns, textMachineInfo, req.MachineInfo, false); err != nil { + if crash.MachineInfo, err = putText(c, ns, textMachineInfo, req.MachineInfo, true); err != nil { return err } crashKey := db.NewIncompleteKey(c, "Crash", bugKey) diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go index 2931a30b7..95b313306 100644 --- a/dashboard/app/app_test.go +++ b/dashboard/app/app_test.go @@ -305,10 +305,11 @@ var buildCommitDate = time.Date(1, 2, 3, 4, 5, 6, 0, time.UTC) func testCrash(build *dashapi.Build, id int) *dashapi.Crash { return &dashapi.Crash{ - BuildID: build.ID, - Title: fmt.Sprintf("title%v", id), - Log: []byte(fmt.Sprintf("log%v", id)), - Report: []byte(fmt.Sprintf("report%v", id)), + BuildID: build.ID, + Title: fmt.Sprintf("title%v", id), + Log: []byte(fmt.Sprintf("log%v", id)), + Report: []byte(fmt.Sprintf("report%v", id)), + MachineInfo: []byte(fmt.Sprintf("machine info %v", id)), } } diff --git a/dashboard/app/main.go b/dashboard/app/main.go index 422cebd9c..a5b39f023 100644 --- a/dashboard/app/main.go +++ b/dashboard/app/main.go @@ -37,6 +37,7 @@ func initHTTPHandlers() { http.Handle("/x/patch.diff", handlerWrapper(handleTextX(textPatch))) http.Handle("/x/bisect.txt", handlerWrapper(handleTextX(textLog))) http.Handle("/x/error.txt", handlerWrapper(handleTextX(textError))) + http.Handle("/x/minfo.txt", handlerWrapper(handleTextX(textMachineInfo))) for ns := range config.Namespaces { http.Handle("/"+ns, handlerWrapper(handleMain)) http.Handle("/"+ns+"/fixed", handlerWrapper(handleFixed)) @@ -548,8 +549,10 @@ func textFilename(tag string) string { return "bisect.txt" case textError: return "error.txt" + case textMachineInfo: + return "minfo.txt" default: - return "text.txt" + panic(fmt.Sprintf("unknown tag %v", tag)) } } diff --git a/dashboard/app/reporting_test.go b/dashboard/app/reporting_test.go index 1bb985b55..59523b6ca 100644 --- a/dashboard/app/reporting_test.go +++ b/dashboard/app/reporting_test.go @@ -25,6 +25,7 @@ func TestReportBug(t *testing.T) { Maintainers: []string{`"Foo Bar" <foo@bar.com>`, `bar@foo.com`}, Log: []byte("log1"), Report: []byte("report1"), + MachineInfo: []byte("machine info 1"), } c.client.ReportCrash(crash1) @@ -61,6 +62,8 @@ func TestReportBug(t *testing.T) { KernelCommitDate: buildCommitDate, KernelConfig: []byte("config1"), KernelConfigLink: externalLink(c.ctx, textKernelConfig, dbBuild.KernelConfig), + MachineInfo: []byte("machine info 1"), + MachineInfoLink: externalLink(c.ctx, textMachineInfo, dbCrash.MachineInfo), Log: []byte("log1"), LogLink: externalLink(c.ctx, textCrashLog, dbCrash.Log), Report: []byte("report1"), @@ -496,7 +499,7 @@ func TestMachineInfo(t *testing.T) { bugPage, err := c.AuthGET(AccessAdmin, bugURL) c.expectOK(err) - infoLinkRegex := regexp.MustCompile(`<a href="(/text\?tag=MachineInfo[^"]+)">machine info</a>`) + infoLinkRegex := regexp.MustCompile(`<a href="(/text\?tag=MachineInfo[^"]+)">info</a>`) infoLinkSubmatch := infoLinkRegex.FindSubmatch(bugPage) c.expectEQ(len(infoLinkSubmatch), 2) infoURL := string(infoLinkSubmatch[1]) diff --git a/dashboard/app/templates.html b/dashboard/app/templates.html index c07ef4cfb..12a9b524c 100644 --- a/dashboard/app/templates.html +++ b/dashboard/app/templates.html @@ -323,6 +323,7 @@ Use of this source code is governed by Apache 2 LICENSE that can be found in the <th><a onclick="return sortTable(this, 'Report', reproSort)" href="#">Report</a></th> <th><a onclick="return sortTable(this, 'Syz repro', reproSort)" href="#">Syz repro</a></th> <th><a onclick="return sortTable(this, 'C repro', textSort)" href="#">C repro</a></th> + <th><a onclick="return sortTable(this, 'machine info', textSort)" href="#">VM info</a></th> {{if .HasMaintainers}} <th><a onclick="return sortTable(this, 'Maintainers', textSort)" href="#">Maintainers</a></th> {{end}} @@ -342,7 +343,7 @@ Use of this source code is governed by Apache 2 LICENSE that can be found in the <td class="repro">{{if $b.ReportLink}}<a href="{{$b.ReportLink}}">report</a>{{end}}</td> <td class="repro">{{if $b.ReproSyzLink}}<a href="{{$b.ReproSyzLink}}">syz</a>{{end}}</td> <td class="repro">{{if $b.ReproCLink}}<a href="{{$b.ReproCLink}}">C</a>{{end}}</td> - <td class="repro">{{if $b.MachineInfoLink}}<a href="{{$b.MachineInfoLink}}">machine info</a>{{end}}</td> + <td class="repro">{{if $b.MachineInfoLink}}<a href="{{$b.MachineInfoLink}}">info</a>{{end}}</td> {{if $.HasMaintainers}} <td class="maintainers" title="{{$b.Maintainers}}">{{$b.Maintainers}}</td> {{end}} |
