From ff7c7be61e82bd063e5c9d2a2266bb31960db63b Mon Sep 17 00:00:00 2001 From: Chmouel Boudjnah Date: Wed, 18 Dec 2024 17:42:19 +0100 Subject: [PATCH] Fix crash with bad payload We were getting this crash on the controller: ``` panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x1f99bc7] pipelines-as-code/pkg/provider/github/parse_payload.go:277 ``` TestSkippedEvent shows it but it wasn't detected that the controller was crashing and hapily returned ok Signed-off-by: Chmouel Boudjnah --- pkg/provider/github/parse_payload.go | 15 +++++++++++++++ pkg/provider/github/parse_payload_test.go | 18 ++++++++++++++++++ test/invalid_event_test.go | 2 +- 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/pkg/provider/github/parse_payload.go b/pkg/provider/github/parse_payload.go index b6a6e3353..7623f25d4 100644 --- a/pkg/provider/github/parse_payload.go +++ b/pkg/provider/github/parse_payload.go @@ -254,6 +254,9 @@ func (v *Provider) processEvent(ctx context.Context, event *info.Event, eventInt return nil, err } case *github.PushEvent: + if gitEvent.GetRepo() == nil { + return nil, errors.New("error parsing payload the repository should not be nil") + } processedEvent.Organization = gitEvent.GetRepo().GetOwner().GetLogin() processedEvent.Repository = gitEvent.GetRepo().GetName() processedEvent.DefaultBranch = gitEvent.GetRepo().GetDefaultBranch() @@ -274,6 +277,9 @@ func (v *Provider) processEvent(ctx context.Context, event *info.Event, eventInt processedEvent.HeadURL = processedEvent.BaseURL // in push events Head URL is the same as BaseURL case *github.PullRequestEvent: processedEvent.Repository = gitEvent.GetRepo().GetName() + if gitEvent.GetRepo() == nil { + return nil, errors.New("error parsing payload the repository should not be nil") + } processedEvent.Organization = gitEvent.GetRepo().Owner.GetLogin() processedEvent.DefaultBranch = gitEvent.GetRepo().GetDefaultBranch() processedEvent.SHA = gitEvent.GetPullRequest().Head.GetSHA() @@ -309,6 +315,9 @@ func (v *Provider) processEvent(ctx context.Context, event *info.Event, eventInt func (v *Provider) handleReRequestEvent(ctx context.Context, event *github.CheckRunEvent) (*info.Event, error) { runevent := info.NewEvent() + if event.GetRepo() == nil { + return nil, errors.New("error parsing payload the repository should not be nil") + } runevent.Organization = event.GetRepo().GetOwner().GetLogin() runevent.Repository = event.GetRepo().GetName() runevent.URL = event.GetRepo().GetHTMLURL() @@ -334,6 +343,9 @@ func (v *Provider) handleReRequestEvent(ctx context.Context, event *github.Check func (v *Provider) handleCheckSuites(ctx context.Context, event *github.CheckSuiteEvent) (*info.Event, error) { runevent := info.NewEvent() + if event.GetRepo() == nil { + return nil, errors.New("error parsing payload the repository should not be nil") + } runevent.Organization = event.GetRepo().GetOwner().GetLogin() runevent.Repository = event.GetRepo().GetName() runevent.URL = event.GetRepo().GetHTMLURL() @@ -396,6 +408,9 @@ func (v *Provider) handleIssueCommentEvent(ctx context.Context, event *github.Is func (v *Provider) handleCommitCommentEvent(ctx context.Context, event *github.CommitCommentEvent) (*info.Event, error) { action := "push" runevent := info.NewEvent() + if event.GetRepo() == nil { + return nil, errors.New("error parsing payload the repository should not be nil") + } runevent.Organization = event.GetRepo().GetOwner().GetLogin() runevent.Repository = event.GetRepo().GetName() runevent.Sender = event.GetSender().GetLogin() diff --git a/pkg/provider/github/parse_payload_test.go b/pkg/provider/github/parse_payload_test.go index 496e42796..997e80c17 100644 --- a/pkg/provider/github/parse_payload_test.go +++ b/pkg/provider/github/parse_payload_test.go @@ -91,6 +91,10 @@ var samplePRAnother = github.PullRequest{ func TestParsePayLoad(t *testing.T) { samplePrEventClosed := samplePRevent samplePrEventClosed.Action = github.String("closed") + + samplePRNoRepo := samplePRevent + samplePRNoRepo.Repo = nil + tests := []struct { name string wantErrString string @@ -188,6 +192,20 @@ func TestParsePayLoad(t *testing.T) { }, shaRet: "samplePRsha", }, + { + name: "bad/pull request", + eventType: "pull_request", + triggerTarget: triggertype.PullRequest.String(), + payloadEventStruct: samplePRNoRepo, + wantErrString: "error parsing payload the repository should not be nil", + }, + { + name: "bad/push", + eventType: "push", + triggerTarget: triggertype.Push.String(), + payloadEventStruct: github.PushEvent{}, + wantErrString: "error parsing payload the repository should not be nil", + }, { // specific run from a check_suite name: "good/rerequest check_run on pull request", diff --git a/test/invalid_event_test.go b/test/invalid_event_test.go index 0de2ff53e..4a3714377 100644 --- a/test/invalid_event_test.go +++ b/test/invalid_event_test.go @@ -82,7 +82,7 @@ func TestSkippedEvent(t *testing.T) { assert.NilError(t, err) defer resp.Body.Close() - assert.Equal(t, resp.StatusCode, http.StatusOK, "%s reply expected 200 OK", elURL) + assert.Equal(t, resp.StatusCode, http.StatusAccepted, "%s reply expected 202 OK", elURL) } func TestGETCall(t *testing.T) {