Skip to content

Commit ad957ce

Browse files
a-nogikha-nogikh
committed
syz-cluster: basic support for finding invalidation
Add some initial #syz invalid support to syz-cluster. For now, mark all findings as invalid and don't display that such series have findings on the web dashboard.
1 parent e833134 commit ad957ce

File tree

18 files changed

+181
-21
lines changed

18 files changed

+181
-21
lines changed

‎syz-cluster/dashboard/templates/series.html‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ <h3>Session {{.CreatedAt.Format "2006-01-02"}}</h3>
148148
{{range .Findings}}
149149
<tr>
150150
<td>
151+
{{if not .InvalidatedAt.IsNull}}<b>[invalidated]</b>{{end}}
151152
{{if .ReportURI}}
152153
<a href="/findings/{{.ID}}/report" class="modal-link-raw">{{.Title}}</a>
153154
{{else}}

‎syz-cluster/email-reporter/handler.go‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,19 +126,23 @@ func (h *Handler) IncomingEmail(ctx context.Context, msg *email.Email) error {
126126

127127
var reply string
128128
for _, command := range msg.Commands {
129+
var err error
129130
switch command.Command {
130131
case email.CmdUpstream:
131-
err := h.apiClient.UpstreamReport(ctx, reportID, &api.UpstreamReportReq{
132+
// Reply nothing on success.
133+
err = h.apiClient.UpstreamReport(ctx, reportID, &api.UpstreamReportReq{
132134
User: msg.Author,
133135
})
134-
if err != nil {
135-
reply = fmt.Sprintf("Failed to process the command. Contact %s.",
136-
h.emailConfig.SupportEmail)
137-
}
136+
case email.CmdInvalid:
138137
// Reply nothing on success.
138+
err = h.apiClient.InvalidateReport(ctx, reportID)
139139
default:
140140
reply = "Unknown command"
141141
}
142+
if err != nil {
143+
reply = fmt.Sprintf("Failed to process the command. Contact %s.",
144+
h.emailConfig.SupportEmail)
145+
}
142146
}
143147

144148
if reply == "" {

‎syz-cluster/email-reporter/handler_test.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/google/syzkaller/syz-cluster/pkg/emailclient"
1515
"github.com/google/syzkaller/syz-cluster/pkg/reporter"
1616
"github.com/stretchr/testify/assert"
17+
"github.com/stretchr/testify/require"
1718
)
1819

1920
var testEmailConfig = emailclient.TestEmailConfig()
@@ -64,6 +65,38 @@ func TestModerationReportFlow(t *testing.T) {
6465
}, receivedEmail)
6566
}
6667

68+
func TestReportInvalidationFlow(t *testing.T) {
69+
env, ctx := app.TestEnvironment(t)
70+
testSeries := controller.DummySeries()
71+
handler, _, emailServer := setupHandlerTest(t, env, ctx, testSeries)
72+
73+
report, err := handler.PollAndReport(ctx)
74+
require.NoError(t, err)
75+
76+
receivedEmail := emailServer.email()
77+
require.NotNil(t, receivedEmail, "a moderation email must be sent")
78+
receivedEmail.Body = nil // for now don't validate the body
79+
80+
// Emulate an "upstream" command.
81+
err = handler.IncomingEmail(ctx, &email.Email{
82+
BugIDs: []string{report.ID},
83+
Commands: []*email.SingleCommand{
84+
{
85+
Command: email.CmdInvalid,
86+
},
87+
},
88+
})
89+
require.NoError(t, err)
90+
91+
// The report must be not sent upstream.
92+
report, err = handler.PollAndReport(ctx)
93+
require.NoError(t, err)
94+
assert.Nil(t, report)
95+
96+
receivedEmail = emailServer.email()
97+
assert.Nil(t, receivedEmail, "an email must not be sent upstream")
98+
}
99+
67100
func TestInvalidReply(t *testing.T) {
68101
env, ctx := app.TestEnvironment(t)
69102
testSeries := controller.DummySeries()

‎syz-cluster/pkg/api/api.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ type Finding struct {
174174
Build BuildInfo `json:"build"`
175175
LinkCRepro string `json:"c_repro"`
176176
LinkSyzRepro string `json:"syz_repro"`
177+
Invalidated bool `json:"invalidated"`
177178
}
178179

179180
type BuildInfo struct {

‎syz-cluster/pkg/api/reporter.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@ func (client ReporterClient) UpstreamReport(ctx context.Context, id string, req
4545
return err
4646
}
4747

48+
func (client ReporterClient) InvalidateReport(ctx context.Context, id string) error {
49+
_, err := postJSON[any, any](ctx, client.baseURL+"/reports/"+id+"/invalidate", nil)
50+
return err
51+
}
52+
4853
type RecordReplyReq struct {
4954
MessageID string `json:"message_id"`
5055
ReportID string `json:"report_id"`

‎syz-cluster/pkg/db/entities.go‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -133,15 +133,20 @@ type SessionTest struct {
133133
}
134134

135135
type Finding struct {
136-
ID string `spanner:"ID"`
137-
SessionID string `spanner:"SessionID"`
138-
TestName string `spanner:"TestName"`
139-
Title string `spanner:"Title"`
140-
ReportURI string `spanner:"ReportURI"`
141-
LogURI string `spanner:"LogURI"`
142-
SyzReproURI string `spanner:"SyzReproURI"`
143-
SyzReproOptsURI string `spanner:"SyzReproOptsURI"`
144-
CReproURI string `spanner:"CReproURI"`
136+
ID string `spanner:"ID"`
137+
SessionID string `spanner:"SessionID"`
138+
TestName string `spanner:"TestName"`
139+
Title string `spanner:"Title"`
140+
ReportURI string `spanner:"ReportURI"`
141+
LogURI string `spanner:"LogURI"`
142+
SyzReproURI string `spanner:"SyzReproURI"`
143+
SyzReproOptsURI string `spanner:"SyzReproOptsURI"`
144+
CReproURI string `spanner:"CReproURI"`
145+
InvalidatedAt spanner.NullTime `spanner:"InvalidatedAt"`
146+
}
147+
148+
func (f *Finding) SetInvalidatedAt(t time.Time) {
149+
f.InvalidatedAt = spanner.NullTime{Time: t, Valid: true}
145150
}
146151

147152
type SessionReport struct {

‎syz-cluster/pkg/db/migrations/7_invalidated_findings.down.sql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE Findings DROP COLUMN InvalidatedAt;

‎syz-cluster/pkg/db/migrations/7_invalidated_findings.up.sql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE Findings ADD COLUMN InvalidatedAt TIMESTAMP DEFAULT(NULL);

‎syz-cluster/pkg/db/series_repo.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,9 @@ func (repo *SeriesRepository) ListLatest(ctx context.Context, filter SeriesFilte
173173
stmt.SQL += ")"
174174
}
175175
if filter.WithFindings {
176-
stmt.SQL += " AND Series.LatestSessionID IS NOT NULL " +
177-
"AND EXISTS(SELECT 1 FROM Findings WHERE Findings.SessionID = Series.LatestSessionID)"
176+
stmt.SQL += " AND Series.LatestSessionID IS NOT NULL AND EXISTS(" +
177+
"SELECT 1 FROM Findings WHERE " +
178+
"Findings.SessionID = Series.LatestSessionID AND Findings.InvalidatedAt IS NULL)"
178179
}
179180
stmt.SQL += " ORDER BY PublishedAt DESC, ID"
180181
if filter.Limit > 0 {
@@ -262,7 +263,8 @@ func (repo *SeriesRepository) queryFindingCounts(ctx context.Context, ro *spanne
262263
}
263264
list, err := readEntities[findingCount](ctx, repo.client.Single(), spanner.Statement{
264265
SQL: "SELECT `SessionID`, COUNT(`ID`) as `Count` FROM `Findings` " +
265-
"WHERE `SessionID` IN UNNEST(@ids) GROUP BY `SessionID`",
266+
"WHERE `SessionID` IN UNNEST(@ids) AND `Findings`.`InvalidatedAt` IS NULL " +
267+
"GROUP BY `SessionID`",
266268
Params: map[string]interface{}{
267269
"ids": keys,
268270
},

‎syz-cluster/pkg/db/series_repo_test.go‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ func TestSeriesRepositoryList(t *testing.T) {
145145
})
146146

147147
dtd.addSessionTest(session, "test")
148-
dtd.addFinding(session, "title", "test")
148+
finding := dtd.addFinding(session, "title", "test")
149149
dtd.finishSession(session)
150150
t.Run("query_finding_count", func(t *testing.T) {
151151
list, err := repo.ListLatest(ctx, SeriesFilter{Status: SessionStatusFinished}, time.Time{})
@@ -160,6 +160,18 @@ func TestSeriesRepositoryList(t *testing.T) {
160160
assert.Len(t, list, 1)
161161
assert.Equal(t, "Series 2", list[0].Series.Title)
162162
})
163+
164+
dtd.invalidateFinding(finding)
165+
t.Run("invalidated_findings", func(t *testing.T) {
166+
list, err := repo.ListLatest(ctx, SeriesFilter{WithFindings: true}, time.Time{})
167+
assert.NoError(t, err)
168+
assert.Len(t, list, 0)
169+
// When not filtered, ensure invalidated findings are not counted in.
170+
list, err = repo.ListLatest(ctx, SeriesFilter{Status: SessionStatusFinished}, time.Time{})
171+
assert.NoError(t, err)
172+
assert.Len(t, list, 1)
173+
assert.Equal(t, 0, list[0].Findings)
174+
})
163175
}
164176

165177
func TestSeriesRepositoryUpdate(t *testing.T) {

0 commit comments

Comments
 (0)