From 0bfc1202cebeedf826d470296424f85dee4fe842 Mon Sep 17 00:00:00 2001 From: Aleksandr Nogikh Date: Fri, 11 Jul 2025 12:23:09 +0200 Subject: syz-cluster: make reports more detailed Fill in build details per each finding and display that information in the report email. Extend the test that verifies how api.SessionReport is filled. --- syz-cluster/pkg/api/api.go | 4 ++-- syz-cluster/pkg/controller/testutil.go | 13 ++++++----- syz-cluster/pkg/db/finding_repo.go | 3 ++- syz-cluster/pkg/db/finding_repo_test.go | 2 +- syz-cluster/pkg/db/spanner.go | 4 +++- syz-cluster/pkg/report/template.txt | 11 +++++----- syz-cluster/pkg/report/testdata/1.in.json | 6 +++-- syz-cluster/pkg/report/testdata/1.moderation.txt | 10 ++++----- syz-cluster/pkg/report/testdata/1.upstream.txt | 10 ++++----- syz-cluster/pkg/reporter/api_test.go | 11 ++++++++++ syz-cluster/pkg/service/build.go | 9 ++++++++ syz-cluster/pkg/service/finding.go | 28 +++++++++++++++++++----- syz-cluster/pkg/service/report.go | 4 +++- syz-cluster/pkg/service/series.go | 1 + 14 files changed, 81 insertions(+), 35 deletions(-) (limited to 'syz-cluster/pkg') diff --git a/syz-cluster/pkg/api/api.go b/syz-cluster/pkg/api/api.go index 3f3a6b089..5fcd0e797 100644 --- a/syz-cluster/pkg/api/api.go +++ b/syz-cluster/pkg/api/api.go @@ -128,8 +128,6 @@ type SessionReport struct { ID string `json:"id"` Cc []string `json:"cc"` Moderation bool `json:"moderation"` - BaseRepo string `json:"base_repo"` - BaseCommit string `json:"base_commit"` Series *Series `json:"series"` Findings []*Finding `json:"findings"` Link string `json:"link"` // URL to the web dashboard. @@ -145,6 +143,8 @@ type Finding struct { } type BuildInfo struct { + Repo string `json:"repo"` + BaseCommit string `json:"base_commit"` Arch string `json:"arch"` Compiler string `json:"compiler"` ConfigLink string `json:"config_link"` diff --git a/syz-cluster/pkg/controller/testutil.go b/syz-cluster/pkg/controller/testutil.go index 5ed01418c..f4d813e2f 100644 --- a/syz-cluster/pkg/controller/testutil.go +++ b/syz-cluster/pkg/controller/testutil.go @@ -49,6 +49,7 @@ func DummySeries() *api.Series { return &api.Series{ ExtID: "ext-id", Title: "test series name", + Link: "http://link/to/series", Patches: []api.SeriesPatch{ { Seq: 1, @@ -83,12 +84,14 @@ func DummyFindings() []*api.NewFinding { func FakeSeriesWithFindings(t *testing.T, ctx context.Context, env *app.AppEnvironment, client *api.Client, series *api.Series) { _, sessionID := UploadTestSeries(t, ctx, client, series) - buildResp := UploadTestBuild(t, ctx, client, DummyBuild()) + baseBuild := UploadTestBuild(t, ctx, client, DummyBuild()) + patchedBuild := UploadTestBuild(t, ctx, client, DummyBuild()) err := client.UploadTestResult(ctx, &api.TestResult{ - SessionID: sessionID, - BaseBuildID: buildResp.ID, - TestName: "test", - Result: api.TestRunning, + SessionID: sessionID, + BaseBuildID: baseBuild.ID, + PatchedBuildID: patchedBuild.ID, + TestName: "test", + Result: api.TestRunning, }) assert.NoError(t, err) diff --git a/syz-cluster/pkg/db/finding_repo.go b/syz-cluster/pkg/db/finding_repo.go index 4a474263e..b5ce94534 100644 --- a/syz-cluster/pkg/db/finding_repo.go +++ b/syz-cluster/pkg/db/finding_repo.go @@ -65,11 +65,12 @@ func (repo *FindingRepository) Save(ctx context.Context, finding *Finding) error } // nolint: dupl -func (repo *FindingRepository) ListForSession(ctx context.Context, sessionID string) ([]*Finding, error) { +func (repo *FindingRepository) ListForSession(ctx context.Context, sessionID string, limit int) ([]*Finding, error) { stmt := spanner.Statement{ SQL: "SELECT * FROM `Findings` WHERE `SessionID` = @session ORDER BY `TestName`, `Title`", Params: map[string]interface{}{"session": sessionID}, } + addLimit(&stmt, limit) iter := repo.client.Single().Query(ctx, stmt) defer iter.Stop() return readEntities[Finding](iter) diff --git a/syz-cluster/pkg/db/finding_repo_test.go b/syz-cluster/pkg/db/finding_repo_test.go index 40ca385ac..291bb0129 100644 --- a/syz-cluster/pkg/db/finding_repo_test.go +++ b/syz-cluster/pkg/db/finding_repo_test.go @@ -53,7 +53,7 @@ func TestFindingRepo(t *testing.T) { assert.ErrorIs(t, err, ErrFindingExists) } - list, err := findingRepo.ListForSession(ctx, session.ID) + list, err := findingRepo.ListForSession(ctx, session.ID, NoLimit) assert.NoError(t, err) assert.Equal(t, toInsert, list) } diff --git a/syz-cluster/pkg/db/spanner.go b/syz-cluster/pkg/db/spanner.go index 1c973ab46..8defc9541 100644 --- a/syz-cluster/pkg/db/spanner.go +++ b/syz-cluster/pkg/db/spanner.go @@ -271,8 +271,10 @@ func readEntities[T any](iter *spanner.RowIterator) ([]*T, error) { return ret, nil } +const NoLimit = 0 + func addLimit(stmt *spanner.Statement, limit int) { - if limit > 0 { + if limit != NoLimit { stmt.SQL += " LIMIT @limit" stmt.Params["limit"] = limit } diff --git a/syz-cluster/pkg/report/template.txt b/syz-cluster/pkg/report/template.txt index 6a4723771..955fd2444 100644 --- a/syz-cluster/pkg/report/template.txt +++ b/syz-cluster/pkg/report/template.txt @@ -1,4 +1,4 @@ -{{.Config.Name}} has processed the following series +{{.Config.Name}} has tested the following series [v{{.Report.Series.Version}}] {{.Report.Series.Title}} {{.Report.Series.Link}} @@ -11,10 +11,6 @@ and found the following issues: * {{.Title}} {{- end}} -The series was applied to the following base tree: -* Tree: {{.Report.BaseRepo}} -* Commit: {{.Report.BaseCommit}} - Full report is available here: {{.Report.Link}} @@ -23,7 +19,10 @@ Full report is available here: *** {{.Title}} -{{if .Build.Arch}} + +tree: {{.Build.Repo}} +base: {{.Build.BaseCommit}} +{{- if .Build.Arch}} arch: {{.Build.Arch}} {{- end}} {{- if .Build.Compiler}} diff --git a/syz-cluster/pkg/report/testdata/1.in.json b/syz-cluster/pkg/report/testdata/1.in.json index e6ff0696e..68dea8d69 100644 --- a/syz-cluster/pkg/report/testdata/1.in.json +++ b/syz-cluster/pkg/report/testdata/1.in.json @@ -1,7 +1,5 @@ { "id": "abcd", - "base_repo": "git://repo", - "base_commit": "abcd0123", "series": { "title": "Series title", "version": 2, @@ -22,6 +20,8 @@ "c_repro": "http://link/to/c/repro", "syz_repro": "http://link/to/syz/repro", "build": { + "repo": "http://kernel/repo1", + "base_commit": "base_commit1", "arch": "amd64", "config_link": "http://link/to/config/1", "compiler": "clang" @@ -32,6 +32,8 @@ "report": "Report Line D\nReport Line E\nReport Line F", "syz_repro": "http://link/to/syz/repro2", "build": { + "repo": "http://kernel/repo1", + "base_commit": "base_commit1", "arch": "arm64", "config_link": "http://link/to/config/2", "compiler": "clang" diff --git a/syz-cluster/pkg/report/testdata/1.moderation.txt b/syz-cluster/pkg/report/testdata/1.moderation.txt index 7f5a98f2b..c90ad82a6 100644 --- a/syz-cluster/pkg/report/testdata/1.moderation.txt +++ b/syz-cluster/pkg/report/testdata/1.moderation.txt @@ -1,4 +1,4 @@ -syzbot has processed the following series +syzbot has tested the following series [v2] Series title http://link/to/series @@ -9,10 +9,6 @@ and found the following issues: * WARNING in abcd * KASAN: use-after-free Write in abcd -The series was applied to the following base tree: -* Tree: git://repo -* Commit: abcd0123 - Full report is available here: http://some/link/to/report @@ -20,6 +16,8 @@ http://some/link/to/report WARNING in abcd +tree: http://kernel/repo1 +base: base_commit1 arch: amd64 compiler: clang config: http://link/to/config/1 @@ -34,6 +32,8 @@ Report Line C KASAN: use-after-free Write in abcd +tree: http://kernel/repo1 +base: base_commit1 arch: arm64 compiler: clang config: http://link/to/config/2 diff --git a/syz-cluster/pkg/report/testdata/1.upstream.txt b/syz-cluster/pkg/report/testdata/1.upstream.txt index 510e82cd2..66c5b91a2 100644 --- a/syz-cluster/pkg/report/testdata/1.upstream.txt +++ b/syz-cluster/pkg/report/testdata/1.upstream.txt @@ -1,4 +1,4 @@ -syzbot has processed the following series +syzbot has tested the following series [v2] Series title http://link/to/series @@ -9,10 +9,6 @@ and found the following issues: * WARNING in abcd * KASAN: use-after-free Write in abcd -The series was applied to the following base tree: -* Tree: git://repo -* Commit: abcd0123 - Full report is available here: http://some/link/to/report @@ -20,6 +16,8 @@ http://some/link/to/report WARNING in abcd +tree: http://kernel/repo1 +base: base_commit1 arch: amd64 compiler: clang config: http://link/to/config/1 @@ -34,6 +32,8 @@ Report Line C KASAN: use-after-free Write in abcd +tree: http://kernel/repo1 +base: base_commit1 arch: arm64 compiler: clang config: http://link/to/config/2 diff --git a/syz-cluster/pkg/reporter/api_test.go b/syz-cluster/pkg/reporter/api_test.go index 14c61f5bc..b7170bc68 100644 --- a/syz-cluster/pkg/reporter/api_test.go +++ b/syz-cluster/pkg/reporter/api_test.go @@ -40,6 +40,7 @@ func TestAPIReportFlow(t *testing.T) { Series: &api.Series{ ExtID: testSeries.ExtID, Title: testSeries.Title, + Link: "http://link/to/series", Patches: []api.SeriesPatch{ { Seq: 1, @@ -54,11 +55,21 @@ func TestAPIReportFlow(t *testing.T) { Title: "finding 0", Report: "report 0", LogURL: "TODO", // TODO + Build: api.BuildInfo{ + Repo: "mainline", + BaseCommit: "abcd", + Arch: "amd64", + }, }, { Title: "finding 1", Report: "report 1", LogURL: "TODO", // TODO + Build: api.BuildInfo{ + Repo: "mainline", + BaseCommit: "abcd", + Arch: "amd64", + }, }, }, }, nextResp.Report) diff --git a/syz-cluster/pkg/service/build.go b/syz-cluster/pkg/service/build.go index 070f3e20d..40d716472 100644 --- a/syz-cluster/pkg/service/build.go +++ b/syz-cluster/pkg/service/build.go @@ -71,3 +71,12 @@ func (s *BuildService) LastBuild(ctx context.Context, req *api.LastBuildReq) (*a } return resp, nil } + +func makeBuildInfo(build *db.Build) api.BuildInfo { + return api.BuildInfo{ + Repo: build.TreeName, // TODO: we actually want to use repo URI here. + BaseCommit: build.CommitHash, + Arch: build.Arch, + ConfigLink: "", + } +} diff --git a/syz-cluster/pkg/service/finding.go b/syz-cluster/pkg/service/finding.go index fb6e2e22e..5c059f8a8 100644 --- a/syz-cluster/pkg/service/finding.go +++ b/syz-cluster/pkg/service/finding.go @@ -16,14 +16,18 @@ import ( ) type FindingService struct { - findingRepo *db.FindingRepository - blobStorage blob.Storage + findingRepo *db.FindingRepository + sessionTestRepo *db.SessionTestRepository + buildRepo *db.BuildRepository + blobStorage blob.Storage } func NewFindingService(env *app.AppEnvironment) *FindingService { return &FindingService{ - findingRepo: db.NewFindingRepository(env.Spanner), - blobStorage: env.BlobStorage, + findingRepo: db.NewFindingRepository(env.Spanner), + blobStorage: env.BlobStorage, + buildRepo: db.NewBuildRepository(env.Spanner), + sessionTestRepo: db.NewSessionTestRepository(env.Spanner), } } @@ -67,17 +71,29 @@ func (s *FindingService) Save(ctx context.Context, req *api.NewFinding) error { return err } -func (s *FindingService) List(ctx context.Context, sessionID string) ([]*api.Finding, error) { - list, err := s.findingRepo.ListForSession(ctx, sessionID) +func (s *FindingService) List(ctx context.Context, sessionID string, limit int) ([]*api.Finding, error) { + list, err := s.findingRepo.ListForSession(ctx, sessionID, limit) if err != nil { return nil, fmt.Errorf("failed to query the list: %w", err) } + tests, err := s.sessionTestRepo.BySession(ctx, sessionID) + if err != nil { + return nil, fmt.Errorf("failed to query session tests: %w", err) + } + testPerName := map[string]*db.FullSessionTest{} + for _, test := range tests { + testPerName[test.TestName] = test + } var ret []*api.Finding for _, item := range list { finding := &api.Finding{ Title: item.Title, LogURL: "TODO", // TODO: where to take it from? } + build := testPerName[item.TestName].PatchedBuild + if build != nil { + finding.Build = makeBuildInfo(build) + } bytes, err := blob.ReadAllBytes(s.blobStorage, item.ReportURI) if err != nil { return nil, fmt.Errorf("failed to read the report: %w", err) diff --git a/syz-cluster/pkg/service/report.go b/syz-cluster/pkg/service/report.go index 4faef5b1e..4b23d32f5 100644 --- a/syz-cluster/pkg/service/report.go +++ b/syz-cluster/pkg/service/report.go @@ -66,6 +66,8 @@ func (rs *ReportService) Upstream(ctx context.Context, id string, req *api.Upstr return nil } +const maxFindingsPerReport = 5 + func (rs *ReportService) Next(ctx context.Context, reporter string) (*api.NextReportResp, error) { list, err := rs.reportRepo.ListNotReported(ctx, reporter, 1) if err != nil { @@ -78,7 +80,7 @@ func (rs *ReportService) Next(ctx context.Context, reporter string) (*api.NextRe if err != nil { return nil, fmt.Errorf("failed to query series: %w", err) } - findings, err := rs.findingService.List(ctx, report.SessionID) + findings, err := rs.findingService.List(ctx, report.SessionID, maxFindingsPerReport) if err != nil { return nil, fmt.Errorf("failed to query findings: %w", err) } diff --git a/syz-cluster/pkg/service/series.go b/syz-cluster/pkg/service/series.go index 7628ec48b..6583306ce 100644 --- a/syz-cluster/pkg/service/series.go +++ b/syz-cluster/pkg/service/series.go @@ -120,6 +120,7 @@ func (s *SeriesService) getSeries(ctx context.Context, Version: int(series.Version), Cc: series.Cc, PublishedAt: series.PublishedAt, + Link: series.Link, } for _, patch := range patches { var body []byte -- cgit mrf-deployment