From 346c761ef6f504362bb0869eae5a894ad39f2f1f Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Fri, 20 Nov 2020 16:32:55 -0600 Subject: [PATCH 1/2] Add request source Add "request" source with support for "method" and "remote-addr" parameters. Both values are taken from the raw http.Request object. Fixes #312 --- docs/Referencing-Request-Values.md | 22 +++++++-- internal/hook/hook.go | 21 +++++++++ internal/hook/hook_test.go | 26 ++++++----- test/hooks.json.tmpl | 15 +++++++ test/hooks.yaml.tmpl | 9 ++++ webhook_test.go | 71 ++++++++++++++++++++++++------ 6 files changed, 136 insertions(+), 28 deletions(-) diff --git a/docs/Referencing-Request-Values.md b/docs/Referencing-Request-Values.md index 2cc0860..ba35363 100644 --- a/docs/Referencing-Request-Values.md +++ b/docs/Referencing-Request-Values.md @@ -1,5 +1,5 @@ # Referencing request values -There are three types of request values: +There are four types of request values: 1. HTTP Request Header values @@ -19,7 +19,23 @@ There are three types of request values: } ``` -3. Payload (JSON or form-value encoded) +3. HTTP Request parameters + + ```json + { + "source": "request", + "name": "method" + } + ``` + + ```json + { + "source": "request", + "name": "remote-addr" + } + ``` + +4. Payload (JSON or form-value encoded) ```json { "source": "payload", @@ -57,7 +73,7 @@ There are three types of request values: If the payload contains a key with the specified name "commits.0.commit.id", then the value of that key has priority over the dot-notation referencing. -3. XML Payload +4. XML Payload Referencing XML payload parameters is much like the JSON examples above, but XML is more complex. Element attributes are prefixed by a hyphen (`-`). diff --git a/internal/hook/hook.go b/internal/hook/hook.go index 34a429d..738e623 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -35,6 +35,7 @@ const ( SourceQuery string = "url" SourceQueryAlias string = "query" SourcePayload string = "payload" + SourceRequest string = "request" SourceString string = "string" SourceEntirePayload string = "entire-payload" SourceEntireQuery string = "entire-query" @@ -438,12 +439,30 @@ func (ha *Argument) Get(r *Request) (string, error) { case SourceHeader: source = &r.Headers key = textproto.CanonicalMIMEHeaderKey(ha.Name) + case SourceQuery, SourceQueryAlias: source = &r.Query + case SourcePayload: source = &r.Payload + case SourceString: return ha.Name, nil + + case SourceRequest: + if r == nil || r.RawRequest == nil { + return "", errors.New("request is nil") + } + + switch ha.Name { + case "remote-addr": + return r.RawRequest.RemoteAddr, nil + case "method": + return r.RawRequest.Method, nil + default: + return "", fmt.Errorf("unsupported request key: %q", ha.Name) + } + case SourceEntirePayload: res, err := json.Marshal(&r.Payload) if err != nil { @@ -451,6 +470,7 @@ func (ha *Argument) Get(r *Request) (string, error) { } return string(res), nil + case SourceEntireHeaders: res, err := json.Marshal(&r.Headers) if err != nil { @@ -458,6 +478,7 @@ func (ha *Argument) Get(r *Request) (string, error) { } return string(res), nil + case SourceEntireQuery: res, err := json.Marshal(&r.Query) if err != nil { diff --git a/internal/hook/hook_test.go b/internal/hook/hook_test.go index 9a548d7..184243b 100644 --- a/internal/hook/hook_test.go +++ b/internal/hook/hook_test.go @@ -255,27 +255,31 @@ func TestExtractParameter(t *testing.T) { var argumentGetTests = []struct { source, name string headers, query, payload map[string]interface{} + request *http.Request value string ok bool }{ - {"header", "a", map[string]interface{}{"A": "z"}, nil, nil, "z", true}, - {"url", "a", nil, map[string]interface{}{"a": "z"}, nil, "z", true}, - {"payload", "a", nil, nil, map[string]interface{}{"a": "z"}, "z", true}, - {"string", "a", nil, nil, map[string]interface{}{"a": "z"}, "a", true}, + {"header", "a", map[string]interface{}{"A": "z"}, nil, nil, nil, "z", true}, + {"url", "a", nil, map[string]interface{}{"a": "z"}, nil, nil, "z", true}, + {"payload", "a", nil, nil, map[string]interface{}{"a": "z"}, nil, "z", true}, + {"request", "method", nil, nil, map[string]interface{}{"a": "z"}, &http.Request{Method: "POST", RemoteAddr: "127.0.0.1:1234"}, "POST", true}, + {"request", "remote-addr", nil, nil, map[string]interface{}{"a": "z"}, &http.Request{Method: "POST", RemoteAddr: "127.0.0.1:1234"}, "127.0.0.1:1234", true}, + {"string", "a", nil, nil, map[string]interface{}{"a": "z"}, nil, "a", true}, // failures - {"header", "a", nil, map[string]interface{}{"a": "z"}, map[string]interface{}{"a": "z"}, "", false}, // nil headers - {"url", "a", map[string]interface{}{"A": "z"}, nil, map[string]interface{}{"a": "z"}, "", false}, // nil query - {"payload", "a", map[string]interface{}{"A": "z"}, map[string]interface{}{"a": "z"}, nil, "", false}, // nil payload - {"foo", "a", map[string]interface{}{"A": "z"}, nil, nil, "", false}, // invalid source + {"header", "a", nil, map[string]interface{}{"a": "z"}, map[string]interface{}{"a": "z"}, nil, "", false}, // nil headers + {"url", "a", map[string]interface{}{"A": "z"}, nil, map[string]interface{}{"a": "z"}, nil, "", false}, // nil query + {"payload", "a", map[string]interface{}{"A": "z"}, map[string]interface{}{"a": "z"}, nil, nil, "", false}, // nil payload + {"foo", "a", map[string]interface{}{"A": "z"}, nil, nil, nil, "", false}, // invalid source } func TestArgumentGet(t *testing.T) { for _, tt := range argumentGetTests { a := Argument{tt.source, tt.name, "", false} r := &Request{ - Headers: tt.headers, - Query: tt.query, - Payload: tt.payload, + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + RawRequest: tt.request, } value, err := a.Get(r) if (err == nil) != tt.ok || value != tt.value { diff --git a/test/hooks.json.tmpl b/test/hooks.json.tmpl index b94ed1a..fd5cb83 100644 --- a/test/hooks.json.tmpl +++ b/test/hooks.json.tmpl @@ -252,6 +252,21 @@ "include-command-output-in-response": true, "include-command-output-in-response-on-error": true }, + { + "id": "request-source", + "pass-arguments-to-command": [ + { + "source": "request", + "name": "method" + }, + { + "source": "request", + "name": "remote-addr" + } + ], + "execute-command": "{{ .Hookecho }}", + "include-command-output-in-response": true + }, { "id": "static-params-ok", "execute-command": "{{ .Hookecho }}", diff --git a/test/hooks.yaml.tmpl b/test/hooks.yaml.tmpl index 325ef27..82833dc 100644 --- a/test/hooks.yaml.tmpl +++ b/test/hooks.yaml.tmpl @@ -152,6 +152,15 @@ include-command-output-in-response: true include-command-output-in-response-on-error: true +- id: request-source + pass-arguments-to-command: + - source: request + name: method + - source: request + name: remote-addr + execute-command: '{{ .Hookecho }}' + include-command-output-in-response: true + - id: static-params-ok execute-command: '{{ .Hookecho }}' include-command-output-in-response: true diff --git a/webhook_test.go b/webhook_test.go index d12826d..885efb6 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -129,8 +129,24 @@ func TestWebhook(t *testing.T) { t.Errorf("POST %q: failed to ready body: %s", tt.desc, err) } - if res.StatusCode != tt.respStatus || string(body) != tt.respBody { - t.Errorf("failed %q (id: %s):\nexpected status: %#v, response: %s\ngot status: %#v, response: %s\ncommand output:\n%s\n", tt.desc, tt.id, tt.respStatus, tt.respBody, res.StatusCode, body, b) + // Test body + { + var bodyFailed bool + + if res.StatusCode != tt.respStatus { + bodyFailed = true + } + + if tt.bodyIsRE { + bodyFailed = string(body) == tt.respBody + } else { + r := regexp.MustCompile(tt.respBody) + bodyFailed = !r.Match(body) + } + + if bodyFailed { + t.Errorf("failed %q (id: %s):\nexpected status: %#v, response: %s\ngot status: %#v, response: %s\ncommand output:\n%s\n", tt.desc, tt.id, tt.respStatus, tt.respBody, res.StatusCode, body, b) + } } if tt.logMatch == "" { @@ -303,6 +319,7 @@ var hookHandlerTests = []struct { headers map[string]string contentType string body string + bodyIsRE bool respStatus int respBody string @@ -459,6 +476,7 @@ var hookHandlerTests = []struct { "watchers":1 } }`, + false, http.StatusOK, `arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 @@ -473,6 +491,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 nil, "application/x-www-form-urlencoded", `payload={"canon_url": "https://bitbucket.org","commits": [{"author": "marcus","branch": "master","files": [{"file": "somefile.py","type": "modified"}],"message": "Added some more things to somefile.py\n","node": "620ade18607a","parents": ["702c70160afc"],"raw_author": "Marcus Bertrand ","raw_node": "620ade18607ac42d872b568bb92acaa9a28620e9","revision": null,"size": -1,"timestamp": "2012-05-30 05:58:56","utctimestamp": "2014-11-07 15:19:02+00:00"}],"repository": {"absolute_url": "/webhook/testing/","fork": false,"is_private": true,"name": "Project X","owner": "marcus","scm": "git","slug": "project-x","website": "https://atlassian.com/"},"user": "marcus"}`, + false, http.StatusOK, `success`, ``, @@ -526,6 +545,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 ], "total_commits_count": 4 }`, + false, http.StatusOK, `arg: b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327 John Smith john@example.com `, @@ -547,6 +567,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 Hello!! `, + false, http.StatusOK, `success`, ``, @@ -569,6 +590,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 "sg_message_id": "sg_message_id" } ]`, + false, http.StatusOK, `success`, ``, @@ -601,6 +623,7 @@ Content-Transfer-Encoding: binary binary data --xxx--`, + false, http.StatusOK, `success`, ``, @@ -614,6 +637,7 @@ binary data nil, "application/json", `{"exists": 1}`, + false, http.StatusOK, `success`, ``, @@ -627,6 +651,7 @@ binary data nil, "application/json", `{"exists": 1}`, + false, http.StatusOK, `Hook rules were not satisfied.`, `parameter node not found`, @@ -668,6 +693,7 @@ binary data }, "ref":"refs/heads/master" }`, + false, http.StatusOK, `arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 @@ -710,6 +736,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 }, "ref":"refs/heads/master" }`, + false, http.StatusOK, `arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz `, @@ -724,34 +751,50 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 map[string]string{"X-Hub-Signature": "33f9d709782f62b8b4a0178586c65ab098a39fe2"}, "application/json", ``, + false, http.StatusOK, ``, ``, }, + { + "request-source", + "request-source", + nil, + "POST", + map[string]string{"X-Hub-Signature": "33f9d709782f62b8b4a0178586c65ab098a39fe2"}, + "application/json", + `{}`, + true, + http.StatusOK, + `arg: POST 127.0.0.1:.* +`, + ``, + }, + // test with disallowed global HTTP method - {"global disallowed method", "bitbucket", []string{"Post "}, "GET", nil, `{}`, "application/json", http.StatusMethodNotAllowed, ``, ``}, + {"global disallowed method", "bitbucket", []string{"Post "}, "GET", nil, `{}`, "application/json", false, http.StatusMethodNotAllowed, ``, ``}, // test with disallowed HTTP method - {"disallowed method", "github", nil, "Get", nil, `{}`, "application/json", http.StatusMethodNotAllowed, ``, ``}, + {"disallowed method", "github", nil, "Get", nil, `{}`, "application/json", false, http.StatusMethodNotAllowed, ``, ``}, // test with custom return code - {"empty payload", "github", nil, "POST", nil, "application/json", `{}`, http.StatusBadRequest, `Hook rules were not satisfied.`, ``}, + {"empty payload", "github", nil, "POST", nil, "application/json", `{}`, false, http.StatusBadRequest, `Hook rules were not satisfied.`, ``}, // test with custom invalid http code, should default to 200 OK - {"empty payload", "bitbucket", nil, "POST", nil, "application/json", `{}`, http.StatusOK, `Hook rules were not satisfied.`, ``}, + {"empty payload", "bitbucket", nil, "POST", nil, "application/json", `{}`, false, http.StatusOK, `Hook rules were not satisfied.`, ``}, // test with no configured http return code, should default to 200 OK - {"empty payload", "gitlab", nil, "POST", nil, "application/json", `{}`, http.StatusOK, `Hook rules were not satisfied.`, ``}, + {"empty payload", "gitlab", nil, "POST", nil, "application/json", `{}`, false, http.StatusOK, `Hook rules were not satisfied.`, ``}, // test capturing command output - {"don't capture output on success by default", "capture-command-output-on-success-not-by-default", nil, "POST", nil, "application/json", `{}`, http.StatusOK, ``, ``}, - {"capture output on success with flag set", "capture-command-output-on-success-yes-with-flag", nil, "POST", nil, "application/json", `{}`, http.StatusOK, `arg: exit=0 + {"don't capture output on success by default", "capture-command-output-on-success-not-by-default", nil, "POST", nil, "application/json", `{}`, false, http.StatusOK, ``, ``}, + {"capture output on success with flag set", "capture-command-output-on-success-yes-with-flag", nil, "POST", nil, "application/json", `{}`, false, http.StatusOK, `arg: exit=0 `, ``}, - {"don't capture output on error by default", "capture-command-output-on-error-not-by-default", nil, "POST", nil, "application/json", `{}`, http.StatusInternalServerError, `Error occurred while executing the hook's command. Please check your logs for more details.`, ``}, - {"capture output on error with extra flag set", "capture-command-output-on-error-yes-with-extra-flag", nil, "POST", nil, "application/json", `{}`, http.StatusInternalServerError, `arg: exit=1 + {"don't capture output on error by default", "capture-command-output-on-error-not-by-default", nil, "POST", nil, "application/json", `{}`, false, http.StatusInternalServerError, `Error occurred while executing the hook's command. Please check your logs for more details.`, ``}, + {"capture output on error with extra flag set", "capture-command-output-on-error-yes-with-extra-flag", nil, "POST", nil, "application/json", `{}`, false, http.StatusInternalServerError, `arg: exit=1 `, ``}, // Check logs - {"static params should pass", "static-params-ok", nil, "POST", nil, "application/json", `{}`, http.StatusOK, "arg: passed\n", `(?s)command output: arg: passed`}, - {"command with space logs warning", "warn-on-space", nil, "POST", nil, "application/json", `{}`, http.StatusInternalServerError, "Error occurred while executing the hook's command. Please check your logs for more details.", `(?s)error in exec:.*use 'pass[-]arguments[-]to[-]command' to specify args`}, - {"unsupported content type error", "github", nil, "POST", map[string]string{"Content-Type": "nonexistent/format"}, "application/json", `{}`, http.StatusBadRequest, `Hook rules were not satisfied.`, `(?s)error parsing body payload due to unsupported content type header:`}, + {"static params should pass", "static-params-ok", nil, "POST", nil, "application/json", `{}`, false, http.StatusOK, "arg: passed\n", `(?s)command output: arg: passed`}, + {"command with space logs warning", "warn-on-space", nil, "POST", nil, "application/json", `{}`, false, http.StatusInternalServerError, "Error occurred while executing the hook's command. Please check your logs for more details.", `(?s)error in exec:.*use 'pass[-]arguments[-]to[-]command' to specify args`}, + {"unsupported content type error", "github", nil, "POST", map[string]string{"Content-Type": "nonexistent/format"}, "application/json", `{}`, false, http.StatusBadRequest, `Hook rules were not satisfied.`, `(?s)error parsing body payload due to unsupported content type header:`}, } // buffer provides a concurrency-safe bytes.Buffer to tests above. From 6f5962f8f2592b75ccf55900eb7d27512f99c74a Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Sat, 21 Nov 2020 10:00:03 -0600 Subject: [PATCH 2/2] Use strings.ToLower on source name parameters --- internal/hook/hook.go | 2 +- internal/hook/hook_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/hook/hook.go b/internal/hook/hook.go index 738e623..19809e7 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -454,7 +454,7 @@ func (ha *Argument) Get(r *Request) (string, error) { return "", errors.New("request is nil") } - switch ha.Name { + switch strings.ToLower(ha.Name) { case "remote-addr": return r.RawRequest.RemoteAddr, nil case "method": diff --git a/internal/hook/hook_test.go b/internal/hook/hook_test.go index 184243b..e9f29e4 100644 --- a/internal/hook/hook_test.go +++ b/internal/hook/hook_test.go @@ -262,7 +262,7 @@ var argumentGetTests = []struct { {"header", "a", map[string]interface{}{"A": "z"}, nil, nil, nil, "z", true}, {"url", "a", nil, map[string]interface{}{"a": "z"}, nil, nil, "z", true}, {"payload", "a", nil, nil, map[string]interface{}{"a": "z"}, nil, "z", true}, - {"request", "method", nil, nil, map[string]interface{}{"a": "z"}, &http.Request{Method: "POST", RemoteAddr: "127.0.0.1:1234"}, "POST", true}, + {"request", "METHOD", nil, nil, map[string]interface{}{"a": "z"}, &http.Request{Method: "POST", RemoteAddr: "127.0.0.1:1234"}, "POST", true}, {"request", "remote-addr", nil, nil, map[string]interface{}{"a": "z"}, &http.Request{Method: "POST", RemoteAddr: "127.0.0.1:1234"}, "127.0.0.1:1234", true}, {"string", "a", nil, nil, map[string]interface{}{"a": "z"}, nil, "a", true}, // failures