Repository: openshift-pipelines/pipelines-as-code
Pull request #53 (2021-05-12 10:03:29)

Add event_type in created pod
* Refactor code to use github.ParsePayload to parse the event type. This is the premice works to get proper separation of all event type.
    
* We address a minor hijack probability where a rogued user would be able to use the recheck event to DDOS a CI.
    
* Triggers is stil buggy with a full payload, we probably will need to change the
technique of passing information in there unless we start to redo everything as
a Webhook interceptor and just not deal with triggers anymore :\

* Add a python script to help test and replay json payloads 


Patch:
diff --git a/config/400-triggers.yaml b/config/400-triggers.yaml
index 187cb340..84dd9301 100644
--- a/config/400-triggers.yaml
+++ b/config/400-triggers.yaml
@@ -79,6 +79,7 @@ spec:
   params:
     - name: installation_id
     - name: payload
+    - name: event_type
   resourcetemplates:
     - apiVersion: tekton.dev/v1beta1
       kind: PipelineRun
@@ -93,11 +94,13 @@ spec:
             value: $(tt.params.installation_id)
           - name: payload
             value: $(tt.params.payload)
-
+          - name: event_type
+            value: $(tt.params.event_type)
         pipelineSpec:
           params:
             - name: installation_id
             - name: payload
+            - name: event_type
           workspaces:
             - name: secrets
           tasks:
@@ -116,6 +119,8 @@ spec:
                 params:
                   - name: payload
                     type: string
+                  - name: event_type
+                    type: string
                   - name: token
                     type: string
                 steps:
@@ -127,6 +132,8 @@ spec:
                             fieldPath: metadata.labels['tekton.dev/pipelineRun']
                       - name: PAC_PAYLOAD
                         value: "$(params.payload)"
+                      - name: PAC_EVENT_TYPE
+                        value: "$(params.event_type)"
                       - name: PAC_TOKEN
                         value: "$(params.token)"
                     image: "ko://github.com/openshift-pipelines/pipelines-as-code/cmd/pipelines-as-code"
@@ -135,6 +142,8 @@ spec:
                   value: "$(tasks.get-token.results.token)"
                 - name: payload
                   value: "$(params.payload)"
+                - name: event_type
+                  value: "$(params.event_type)"
         workspaces:
           - name: secrets
             secret:
@@ -152,5 +161,7 @@ spec:
   params:
     - name: payload
       value: $(body)
+    - name: event_type
+      value: $(header.X-GitHub-Event)
     - name: installation_id
       value: $(body.installation.id)
diff --git a/hack/replay-gh-events.py b/hack/replay-gh-events.py
new file mode 100755
index 00000000..b17c8915
--- /dev/null
+++ b/hack/replay-gh-events.py
@@ -0,0 +1,96 @@
+#!/usr/bin/env python3
+# -*- coding: utf-8 -*-
+# Author: Chmouel Boudjnah <chmouel@chmouel.com>
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+"""Will replay a json file in Eventlistenner, it automatically detects the
+EventListenner, the webhook secret from github-app-secret secret name
+require requests library"""
+import argparse
+import base64
+import hashlib
+import hmac
+import json
+import subprocess
+
+import requests
+
+NAMESPACE = "pipelines-ascode"
+SECRET_NAME = "github-app-secret"
+ELNAME = "openshift-pipelines-ascode"
+
+
+def get_el_route():
+    elroute = subprocess.run(
+        f"oc get route -n {NAMESPACE} -l eventlistener={ELNAME}-interceptor -o json",
+        shell=True,
+        check=True,
+        capture_output=True)
+    return "https://" + \
+        json.loads(elroute.stdout)["items"][0]["status"]["ingress"][0]["host"]
+
+
+def get_installation_id_and_webhook_secret():
+    secret = subprocess.run(
+        f"kubectl get secret {SECRET_NAME} -n{NAMESPACE} -o json",
+        shell=True,
+        check=True,
+        capture_output=True)
+    jeez = json.loads(secret.stdout)
+    return (jeez["data"]["application_id"],
+            base64.b64decode(jeez["data"]["webhook.secret"]).decode())
+
+
+def main(args):
+    application_id, secret = get_installation_id_and_webhook_secret()
+    el = get_el_route()
+    text = open(args.json_file).read()
+    jeez = json.loads(text)
+    esha256 = hmac.new(secret.encode("utf-8"),
+                       msg=text.encode("utf-8"),
+                       digestmod=hashlib.sha256).hexdigest()
+    esha1 = hmac.new(secret.encode("utf-8"),
+                     msg=text.encode("utf-8"),
+                     digestmod=hashlib.sha1).hexdigest()
+
+    print("Replay event for repo " + jeez["repository"]["full_name"])
+    if jeez["action"] in ("opened", "synchronize") and "pull_request" in jeez:
+        event_type = "pull_request"
+    elif jeez["action"] in ("rerequested") and "check_run" in jeez:
+        event_type = "check_run"
+    else:
+        raise Exception("Unknown event_type")
+
+    headers = {
+        "content-type": "application/json",
+        "X-GitHub-Event": event_type,
+        "X-GitHub-Hook-Installation-Target-ID": application_id,
+        "X-GitHub-Hook-Installation-Target-Type": "integration",
+        "X-Hub-Signature": "sha1=" + esha1,
+        "X-Hub-Signature-256": "sha256=" + esha256,
+    }
+    r = requests.request("POST",
+                         el,
+                         data=text.encode("utf-8"),
+                         headers=headers)
+    print(r.content.decode())
+
+
+def parse_args():
+    parser = argparse.ArgumentParser(description='Replay a webhook')
+    parser.add_argument("json_file", help="json file name")
+    return parser.parse_args()
+
+
+if __name__ == '__main__':
+    main(parse_args())
diff --git a/pkg/cmd/pipelineascode/pipelineascode.go b/pkg/cmd/pipelineascode/pipelineascode.go
index f07d33e8..4b6c18eb 100644
--- a/pkg/cmd/pipelineascode/pipelineascode.go
+++ b/pkg/cmd/pipelineascode/pipelineascode.go
@@ -62,9 +62,10 @@ func Command(p cli.Params) *cobra.Command {
 	cmd.Flags().StringVarP(&opts.RunInfo.Branch, "webhook-target-branch", "", os.Getenv("PAC_BRANCH"), "Target branch of the repository to test")
 	cmd.Flags().StringVarP(&opts.RunInfo.Sender, "webhook-sender", "", os.Getenv("PAC_Sender"), "Sender for the commit/pr")
 	cmd.Flags().StringVarP(&opts.RunInfo.URL, "webhook-url", "", os.Getenv("PAC_URL"), "URL of the repository to test")
+	cmd.Flags().StringVarP(&opts.RunInfo.EventType, "webhook-type", "", os.Getenv("PAC_EVENT_TYPE"), "Payload event type as set from Github")
 
-	cmd.Flags().StringVarP(&opts.Payload, "payload", "", os.Getenv("PAC_PAYLOAD"), "The payload from webhook")
-	cmd.Flags().StringVarP(&opts.PayloadFile, "payload-file", "", os.Getenv("PAC_PAYLOAD_FILE"), "A file which contains the webhook payload")
+	cmd.Flags().StringVarP(&opts.Payload, "payload", "", os.Getenv("PAC_PAYLOAD"), "The payload from webhook as string")
+	cmd.Flags().StringVarP(&opts.PayloadFile, "payload-file", "", os.Getenv("PAC_PAYLOAD_FILE"), "A file containing the webhook payload")
 	return cmd
 }
 
@@ -75,7 +76,7 @@ func getRunInfoFromArgsOrPayload(cs *cli.Clients, payload string, runinfo *webvc
 		return &webvcs.RunInfo{}, fmt.Errorf("no payload or not enough params set properly")
 	}
 
-	payloadinfo, err := cs.GithubClient.ParsePayload(cs.Log, payload)
+	payloadinfo, err := cs.GithubClient.ParsePayload(cs.Log, runinfo.EventType, payload)
 	if err != nil {
 		return &webvcs.RunInfo{}, err
 	}
diff --git a/pkg/webvcs/github.go b/pkg/webvcs/github.go
index 73365dda..7f51844b 100644
--- a/pkg/webvcs/github.go
+++ b/pkg/webvcs/github.go
@@ -21,21 +21,23 @@ type GithubVCS struct {
 }
 
 type RunInfo struct {
+	Branch        string
+	CheckRunID    *int64
+	DefaultBranch string
+	EventType     string
 	Owner         string
 	Repository    string
-	DefaultBranch string
+	Sender        string
 	SHA           string
 	URL           string
-	Branch        string
-	Sender        string
-	CheckRunID    *int64
 	WebConsoleURL string
 }
 
 func (r RunInfo) Check() error {
 	if r.SHA != "" && r.Branch != "" &&
 		r.Repository != "" && r.DefaultBranch != "" &&
-		r.Owner != "" && r.URL != "" && r.Sender != "" {
+		r.Owner != "" && r.URL != "" && r.Sender != "" &&
+		r.EventType != "" {
 		return nil
 	}
 	return fmt.Errorf("missing values in runInfo")
@@ -75,60 +77,65 @@ func payloadFix(payload string) string {
 	return replacer.Replace(payload)
 }
 
-func (v GithubVCS) ParsePayload(log *zap.SugaredLogger, payload string) (*RunInfo, error) {
-	runinfo := &RunInfo{}
-	payload = payloadFix(payload)
-	prMap := &github.PullRequestEvent{}
-	err := json.Unmarshal([]byte(payload), prMap)
-	if err != nil {
-		return runinfo, err
+func (v GithubVCS) handleReRequestEvent(log *zap.SugaredLogger, event *github.CheckRunEvent) (RunInfo, error) {
+	runinfo := RunInfo{
+		Owner:      event.GetRepo().GetOwner().GetLogin(),
+		Repository: event.GetRepo().GetName(),
 	}
-
-	checkrunEvent := new(github.CheckRunEvent)
-
-	err = json.Unmarshal([]byte(payload), &checkrunEvent)
+	prNumber := event.GetCheckRun().GetCheckSuite().PullRequests[0].GetNumber()
+	log.Infof("Recheck of PR %s/%s#%d has been requested", runinfo.Owner, runinfo.Repository, prNumber)
+	pr, _, err := v.Client.PullRequests.Get(context.Background(), runinfo.Owner, runinfo.Repository, prNumber)
 	if err != nil {
 		return runinfo, err
 	}
+	// Make sure to use the Base for Default Branch or there would be a potential hijack
+	runinfo.DefaultBranch = pr.GetBase().GetRepo().GetDefaultBranch()
+	runinfo.URL = pr.GetBase().GetRepo().GetHTMLURL()
+	runinfo.SHA = pr.GetHead().GetSHA()
+	// TODO: Maybe if we wanted to allow rerequest from non approved user we
+	// would use the CheckRun Sender instead of the rerequest sender, could it
+	// be a room for abuse? 🤔
+	runinfo.Sender = pr.GetUser().GetLogin()
+	runinfo.Branch = pr.GetHead().GetRef()
+	return runinfo, nil
+}
 
-	// If the user  has requested a recheck then fetch the pr link and use that.
-	if checkrunEvent.GetAction() == "rerequested" {
-		runinfo.Owner = checkrunEvent.GetRepo().Owner.GetLogin()
-		runinfo.Sender = checkrunEvent.GetSender().GetLogin()
-		runinfo.Repository = checkrunEvent.GetRepo().GetName()
-		prNumber := checkrunEvent.GetCheckRun().GetCheckSuite().PullRequests[0].GetNumber()
-
-		log.Info("Recheck of PR %s/%s#%s has been requested", runinfo.Owner, runinfo.Repository, prNumber)
-		// There should be only one pull_request, I am not quite sure how a
-		// checksuite with multiple PR can be done 🤔
-		pr, _, err := v.Client.PullRequests.Get(context.Background(), runinfo.Owner, runinfo.Repository, prNumber)
-		if err != nil {
-			return &RunInfo{}, err
-		}
-		prMap.PullRequest = pr
+// ParsePayload parse payload event
+func (v GithubVCS) ParsePayload(log *zap.SugaredLogger, eventType, payload string) (*RunInfo, error) {
+	var runinfo RunInfo
+	payload = payloadFix(payload)
+	event, err := github.ParseWebHook(eventType, []byte(payloadFix(payload)))
+	if err != nil {
+		return &runinfo, err
 	}
-
-	if prMap.PullRequest == nil {
-		return &RunInfo{}, errors.New("cannot parse payload as PR")
+	err = json.Unmarshal([]byte(payload), &event)
+	if err != nil {
+		return &runinfo, err
 	}
 
-	// This should already been set from the rerequest event
-	if runinfo.Sender == "" {
-		runinfo.Sender = prMap.GetPullRequest().GetUser().GetLogin()
-	}
-	if runinfo.Repository == "" {
-		runinfo.Repository = prMap.GetRepo().GetName()
-	}
-	if runinfo.Owner == "" {
-		runinfo.Owner = prMap.GetRepo().Owner.GetLogin()
+	switch event := event.(type) {
+	case *github.CheckRunEvent:
+		if event.GetAction() == "rerequested" {
+			runinfo, err = v.handleReRequestEvent(log, event)
+			if err != nil {
+				return &runinfo, err
+			}
+		}
+	case *github.PullRequestEvent:
+		runinfo = RunInfo{
+			Owner:         event.GetRepo().Owner.GetLogin(),
+			Repository:    event.GetRepo().GetName(),
+			DefaultBranch: event.GetRepo().GetDefaultBranch(),
+			SHA:           event.GetPullRequest().Head.GetSHA(),
+			URL:           event.GetRepo().GetHTMLURL(),
+			Branch:        event.GetPullRequest().Base.GetRef(),
+			Sender:        event.GetPullRequest().GetUser().GetLogin(),
+		}
+	default:
+		return &runinfo, errors.New("this event is not supported")
 	}
-
-	runinfo.DefaultBranch = prMap.GetRepo().GetDefaultBranch()
-	runinfo.URL = prMap.GetRepo().GetHTMLURL()
-	runinfo.SHA = prMap.PullRequest.Head.GetSHA()
-	runinfo.Branch = prMap.PullRequest.Base.GetRef()
-
-	return runinfo, nil
+	runinfo.EventType = eventType
+	return &runinfo, nil
 }
 
 // CheckSenderOrgMembership Get sender user's organization. We can


Test patch:
diff --git a/pkg/cmd/pipelineascode/pipelineascode_test.go b/pkg/cmd/pipelineascode/pipelineascode_test.go
index b94c53a7..782d499e 100644
--- a/pkg/cmd/pipelineascode/pipelineascode_test.go
+++ b/pkg/cmd/pipelineascode/pipelineascode_test.go
@@ -42,7 +42,11 @@ func TestRunWrapOld(t *testing.T) {
 		ConsoleURL: "https://console.url",
 	}
 
-	options := &pacpkg.Options{}
+	options := &pacpkg.Options{
+		RunInfo: webvcs.RunInfo{
+			EventType: "pull_request",
+		},
+	}
 	err := runWrap(options, cs, k8int)
 	assert.ErrorContains(t, err, "no payload")
 
@@ -71,6 +75,7 @@ func TestGetInfo(t *testing.T) {
 		URL:           "http://chmouel.com",
 		Branch:        "goodRuninfoBranch",
 		Sender:        "ElSender",
+		EventType:     "pull_request",
 	}
 
 	b, err := ioutil.ReadFile("testdata/pull_request.json")
@@ -103,16 +108,10 @@ func TestGetInfo(t *testing.T) {
 	}{
 		{
 			desc:    "Error on bad payload",
-			runinfo: webvcs.RunInfo{},
+			runinfo: webvcs.RunInfo{EventType: "pull_request"},
 			payload: "foo bar",
 			errmsg:  "invalid character",
 		},
-		{
-			desc:    "Good json but bad payload",
-			runinfo: webvcs.RunInfo{},
-			payload: "{}",
-			errmsg:  "cannot parse payload",
-		},
 		{
 			desc:    "No payload no runcheck",
 			runinfo: webvcs.RunInfo{},
@@ -122,14 +121,15 @@ func TestGetInfo(t *testing.T) {
 		{
 			desc: "Bad runinfo with missing infos",
 			runinfo: webvcs.RunInfo{
-				Owner: "foo",
+				Owner:     "foo",
+				EventType: "pull_request",
 			},
 			payload: "",
 			errmsg:  "no payload or not enough params",
 		},
 		{
 			desc:    "Missing values payload",
-			runinfo: webvcs.RunInfo{},
+			runinfo: webvcs.RunInfo{EventType: "pull_request"},
 			payload: missingValuesPayload,
 			errmsg:  "missing some values",
 		},
@@ -142,7 +142,7 @@ func TestGetInfo(t *testing.T) {
 		},
 		{
 			desc:           "Good payload",
-			runinfo:        webvcs.RunInfo{},
+			runinfo:        webvcs.RunInfo{EventType: "pull_request"},
 			payload:        goodPayload,
 			errmsg:         "",
 			branchShouldBe: "goodInfoBranch",
@@ -193,6 +193,7 @@ func TestRunWrap(t *testing.T) {
 		{
 			name: "good",
 			opts: &pacpkg.Options{
+				RunInfo:     webvcs.RunInfo{EventType: "pull_request"},
 				PayloadFile: "testdata/pull_request.json",
 			},
 		},
diff --git a/pkg/webvcs/github_test.go b/pkg/webvcs/github_test.go
index 138da23e..37de4b38 100644
--- a/pkg/webvcs/github_test.go
+++ b/pkg/webvcs/github_test.go
@@ -30,38 +30,39 @@ func TestPayLoadFix(t *testing.T) {
 	assert.NilError(t, err)
 	gvcs := NewGithubVCS("none")
 	logger, _ := getLogger()
-	_, err = gvcs.ParsePayload(logger, payloadFix(string(b)))
+	_, err = gvcs.ParsePayload(logger, "pull_request", payloadFix(string(b)))
 	// would bomb out on "assertion failed: error is not nil: invalid character
 	// '\n' in string literal" if we don't payloadfix
 	assert.NilError(t, err)
 }
 
 func TestParsePayloadRerequest(t *testing.T) {
-	repoSender := "jean-pierre"
-	repoOwner := "openshift"
+	checkrunSender := "jean-pierre"
+	prOwner := "openshift"
 	repoName := "pipelines"
 	prNumber := "123"
 	checkrunEvent := fmt.Sprintf(`{"action": "rerequested", 
 	"sender": {"login": "%s"},
 	"check_run": {"check_suite": {"pull_requests": [{"number": %s}]}}, 
 	"repository": {"name": "%s", "owner": {"login": "%s"}}}`,
-		repoSender, prNumber, repoName, repoOwner)
+		checkrunSender, prNumber, repoName, prOwner)
 	fakeclient, mux, _, teardown := ghtesthelper.SetupGH()
 	defer teardown()
-	mux.HandleFunc("/repos/"+repoOwner+"/"+repoName+"/pulls/"+prNumber, func(rw http.ResponseWriter, r *http.Request) {
-		fmt.Fprintf(rw, `{"repo": {"name": "%s", "owner": {"login": "%s"}}}`, repoName, repoOwner)
+	mux.HandleFunc("/repos/"+prOwner+"/"+repoName+"/pulls/"+prNumber, func(rw http.ResponseWriter, r *http.Request) {
+		fmt.Fprintf(rw, `{"head": {"ref": "123"}, "user": {"login": "%s"}}`, prOwner)
 	})
 	gvcs := GithubVCS{
 		Client:  fakeclient,
 		Context: context.Background(),
 	}
 	logger, observer := getLogger()
-	runinfo, err := gvcs.ParsePayload(logger, checkrunEvent)
+	runinfo, err := gvcs.ParsePayload(logger, "check_run", checkrunEvent)
 	assert.NilError(t, err)
 
-	assert.Equal(t, repoOwner, runinfo.Owner)
+	assert.Equal(t, prOwner, runinfo.Owner)
 	assert.Equal(t, repoName, runinfo.Repository)
-	assert.Equal(t, repoSender, runinfo.Sender)
+	assert.Assert(t, checkrunSender != runinfo.Sender)
+
 	assert.Assert(t, strings.Contains(observer.TakeAll()[0].Message, "Recheck of PR"))
 }
 
@@ -72,17 +73,33 @@ func TestParsePayload(t *testing.T) {
 	gvcs := NewGithubVCS("none")
 	logger, _ := getLogger()
 
-	runinfo, err := gvcs.ParsePayload(logger, string(b))
+	runinfo, err := gvcs.ParsePayload(logger, "pull_request", string(b))
 	assert.NilError(t, err)
 	assert.Assert(t, runinfo.Branch == "master")
 	assert.Assert(t, runinfo.Owner == "chmouel")
 	assert.Assert(t, runinfo.Repository == "scratchpad")
 	assert.Assert(t, runinfo.URL == "https://github.com/chmouel/scratchpad")
-	_, err = gvcs.ParsePayload(logger, "hello moto")
+}
+
+func TestParsePayloadInvalid(t *testing.T) {
+	gvcs := NewGithubVCS("none")
+	logger, _ := getLogger()
+	_, err := gvcs.ParsePayload(logger, "pull_request", "hello moto")
 	assert.ErrorContains(t, err, "invalid character")
+}
 
-	_, err = gvcs.ParsePayload(logger, "{\"hello\": \"moto\"}")
-	assert.Error(t, err, "cannot parse payload as PR")
+func TestParsePayloadUnkownEvent(t *testing.T) {
+	gvcs := NewGithubVCS("none")
+	logger, _ := getLogger()
+	_, err := gvcs.ParsePayload(logger, "foo", "{\"hello\": \"moto\"}")
+	assert.ErrorContains(t, err, "unknown X-Github-Event")
+}
+
+func TestParsePayCannotParse(t *testing.T) {
+	gvcs := NewGithubVCS("none")
+	logger, _ := getLogger()
+	_, err := gvcs.ParsePayload(logger, "gollum", "{}")
+	assert.Error(t, err, "this event is not supported")
 }
 
 func setupFakesURLS() (client GithubVCS, teardown func()) {
@@ -416,6 +433,7 @@ func TestRunInfoCheck(t *testing.T) {
 				Branch:        tt.fields.Branch,
 				CheckRunID:    tt.fields.CheckRunID,
 				Sender:        tt.fields.Sender,
+				EventType:     "pull_request",
 			}
 			if err := r.Check(); (err != nil) != tt.wantErr {
 				t.Errorf("RunInfo.Check() error = %v, wantErr %v", err, tt.wantErr)