From 526c9a20ac8ae1dbe309db3a59b86b6476946398 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Thu, 21 May 2020 17:47:55 -0500 Subject: [PATCH 01/26] Fix request dumper The existing code had a bug in printing request params. Simplify the request logger by using httputil.DumpRequest. Also print the request before handing it downstream. Fixes #425 --- internal/middleware/dumper.go | 79 +++++++++++++---------------------- 1 file changed, 30 insertions(+), 49 deletions(-) diff --git a/internal/middleware/dumper.go b/internal/middleware/dumper.go index 582c163..ae0e110 100644 --- a/internal/middleware/dumper.go +++ b/internal/middleware/dumper.go @@ -8,13 +8,11 @@ import ( "bytes" "fmt" "io" - "io/ioutil" "net" "net/http" + "net/http/httputil" "sort" "strings" - - "github.com/gorilla/mux" ) // responseDupper tees the response to a buffer and a response writer. @@ -34,70 +32,53 @@ func Dumper(w io.Writer) func(http.Handler) http.Handler { // Request ID rid := r.Context().Value(RequestIDKey) - // Request URL - buf.WriteString(fmt.Sprintf("> [%s] %s %s", rid, r.Method, r.URL.String())) + // Dump request - // Request Headers - keys := make([]string, len(r.Header)) - i := 0 - for k := range r.Header { - keys[i] = k - i++ - } - sort.Strings(keys) - for _, k := range keys { - buf.WriteString(fmt.Sprintf("\n> [%s] %s: %s", rid, k, strings.Join(r.Header[k], ", "))) - } - - // Request parameters - params := mux.Vars(r) - keys = make([]string, len(params)) - i = 0 - for k := range params { - keys[i] = k - i++ - } - sort.Strings(keys) - for _, k := range keys { - buf.WriteString(fmt.Sprintf("\n> [%s] %s: %s", rid, k, strings.Join(r.Header[k], ", "))) - } - - // Request body - b, err := ioutil.ReadAll(r.Body) + bd, err := httputil.DumpRequest(r, true) if err != nil { - b = []byte("failed to read body: " + err.Error()) + buf.WriteString(fmt.Sprintf("[%s] Error dumping request for debugging: %s\n", rid, err)) } - if len(b) > 0 { - buf.WriteByte('\n') - lines := strings.Split(string(b), "\n") - for _, line := range lines { - buf.WriteString(fmt.Sprintf("> [%s] %s\n", rid, line)) - } + + sc := bufio.NewScanner(bytes.NewBuffer(bd)) + sc.Split(bufio.ScanLines) + for sc.Scan() { + buf.WriteString(fmt.Sprintf("> [%s] ", rid)) + buf.WriteString(sc.Text() + "\n") } - r.Body = ioutil.NopCloser(bytes.NewBuffer(b)) + + w.Write(buf.Bytes()) + buf.Reset() + + // Dump Response dupper := &responseDupper{ResponseWriter: rw, Buffer: &bytes.Buffer{}} h.ServeHTTP(dupper, r) - buf.WriteString(fmt.Sprintf("\n< [%s] %s", rid, http.StatusText(dupper.Status))) - keys = make([]string, len(dupper.Header())) - i = 0 + // Response Status + buf.WriteString(fmt.Sprintf("< [%s] %d %s\n", rid, dupper.Status, http.StatusText(dupper.Status))) + + // Response Headers + keys := make([]string, len(dupper.Header())) + i := 0 for k := range dupper.Header() { keys[i] = k i++ } sort.Strings(keys) for _, k := range keys { - buf.WriteString(fmt.Sprintf("\n< [%s] %s: %s", rid, k, strings.Join(dupper.Header()[k], ", "))) + buf.WriteString(fmt.Sprintf("< [%s] %s: %s\n", rid, k, strings.Join(dupper.Header()[k], ", "))) } + + // Response Body if dupper.Buffer.Len() > 0 { - buf.WriteByte('\n') - lines := strings.Split(dupper.Buffer.String(), "\n") - for _, line := range lines { - buf.WriteString(fmt.Sprintf("< [%s] %s\n", rid, line)) + buf.WriteString(fmt.Sprintf("< [%s]\n", rid)) + sc = bufio.NewScanner(dupper.Buffer) + sc.Split(bufio.ScanLines) + for sc.Scan() { + buf.WriteString(fmt.Sprintf("< [%s] ", rid)) + buf.WriteString(sc.Text() + "\n") } } - buf.WriteByte('\n') w.Write(buf.Bytes()) }) } From 41ac427a895a80b744e1ab698350277121fd0e1f Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Thu, 21 May 2020 18:02:52 -0500 Subject: [PATCH 02/26] Warn on failed validate of empty payload signature If signature validation fails on an empty payload, append a note to the end of the error message. Updates #423 --- internal/hook/hook.go | 36 +++++++++++++++++++++++------------- internal/hook/hook_test.go | 27 ++++++++++++++++++++------- test/hooks.json.tmpl | 24 ++++++++++++++++++++++++ test/hooks.yaml.tmpl | 13 +++++++++++++ webhook_test.go | 16 +++++++++++++++- 5 files changed, 95 insertions(+), 21 deletions(-) diff --git a/internal/hook/hook.go b/internal/hook/hook.go index a9b4774..dd5546c 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -73,6 +73,8 @@ func IsParameterNodeError(err error) bool { type SignatureError struct { Signature string Signatures []string + + emptyPayload bool } func (e *SignatureError) Error() string { @@ -80,11 +82,16 @@ func (e *SignatureError) Error() string { return "" } - if e.Signatures != nil { - return fmt.Sprintf("invalid payload signatures %s", e.Signatures) + var empty string + if e.emptyPayload { + empty = " on empty payload" } - return fmt.Sprintf("invalid payload signature %s", e.Signature) + if e.Signatures != nil { + return fmt.Sprintf("invalid payload signatures %s%s", e.Signatures, empty) + } + + return fmt.Sprintf("invalid payload signature %s%s", e.Signature, empty) } // ArgumentError describes an invalid argument passed to Hook. @@ -160,21 +167,24 @@ func ValidateMAC(payload []byte, mac hash.Hash, signatures []string) (string, er return "", err } - expectedMAC := hex.EncodeToString(mac.Sum(nil)) + actualMAC := hex.EncodeToString(mac.Sum(nil)) for _, signature := range signatures { - if hmac.Equal([]byte(signature), []byte(expectedMAC)) { - return expectedMAC, err + if hmac.Equal([]byte(signature), []byte(actualMAC)) { + return actualMAC, err } } - return expectedMAC, &SignatureError{ - Signatures: signatures, + e := &SignatureError{Signatures: signatures} + if len(payload) == 0 { + e.emptyPayload = true } + + return actualMAC, e } // CheckPayloadSignature calculates and verifies SHA1 signature of the given payload -func CheckPayloadSignature(payload []byte, secret string, signature string) (string, error) { +func CheckPayloadSignature(payload []byte, secret, signature string) (string, error) { if secret == "" { return "", errors.New("signature validation secret can not be empty") } @@ -187,7 +197,7 @@ func CheckPayloadSignature(payload []byte, secret string, signature string) (str } // CheckPayloadSignature256 calculates and verifies SHA256 signature of the given payload -func CheckPayloadSignature256(payload []byte, secret string, signature string) (string, error) { +func CheckPayloadSignature256(payload []byte, secret, signature string) (string, error) { if secret == "" { return "", errors.New("signature validation secret can not be empty") } @@ -200,7 +210,7 @@ func CheckPayloadSignature256(payload []byte, secret string, signature string) ( } // CheckPayloadSignature512 calculates and verifies SHA512 signature of the given payload -func CheckPayloadSignature512(payload []byte, secret string, signature string) (string, error) { +func CheckPayloadSignature512(payload []byte, secret, signature string) (string, error) { if secret == "" { return "", errors.New("signature validation secret can not be empty") } @@ -254,7 +264,7 @@ func CheckScalrSignature(headers map[string]interface{}, body []byte, signingKey // CheckIPWhitelist makes sure the provided remote address (of the form IP:port) falls within the provided IP range // (in CIDR form or a single IP address). -func CheckIPWhitelist(remoteAddr string, ipRange string) (bool, error) { +func CheckIPWhitelist(remoteAddr, ipRange string) (bool, error) { // Extract IP address from remote address. // IPv6 addresses will likely be surrounded by []. @@ -293,7 +303,7 @@ func CheckIPWhitelist(remoteAddr string, ipRange string) (bool, error) { // ReplaceParameter replaces parameter value with the passed value in the passed map // (please note you should pass pointer to the map, because we're modifying it) // based on the passed string -func ReplaceParameter(s string, params interface{}, value interface{}) bool { +func ReplaceParameter(s string, params, value interface{}) bool { if params == nil { return false } diff --git a/internal/hook/hook_test.go b/internal/hook/hook_test.go index 489fde2..e348930 100644 --- a/internal/hook/hook_test.go +++ b/internal/hook/hook_test.go @@ -49,12 +49,14 @@ var checkPayloadSignatureTests = []struct { {[]byte(`{"a": "z"}`), "secret", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", true}, {[]byte(`{"a": "z"}`), "secret", "sha1=b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", true}, {[]byte(`{"a": "z"}`), "secret", "sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e,sha1=b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", true}, + {[]byte(``), "secret", "25af6174a0fcecc4d346680a72b7ce644b9a88e8", "25af6174a0fcecc4d346680a72b7ce644b9a88e8", true}, // failures {[]byte(`{"a": "z"}`), "secret", "XXXe04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", false}, {[]byte(`{"a": "z"}`), "secret", "sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", false}, {[]byte(`{"a": "z"}`), "secret", "sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e,sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", false}, {[]byte(`{"a": "z"}`), "secreX", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "900225703e9342328db7307692736e2f7cc7b36e", false}, {[]byte(`{"a": "z"}`), "", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "", false}, + {[]byte(``), "secret", "XXXf6174a0fcecc4d346680a72b7ce644b9a88e8", "25af6174a0fcecc4d346680a72b7ce644b9a88e8", false}, } func TestCheckPayloadSignature(t *testing.T) { @@ -80,11 +82,13 @@ var checkPayloadSignature256Tests = []struct { {[]byte(`{"a": "z"}`), "secret", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", true}, {[]byte(`{"a": "z"}`), "secret", "sha256=f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", true}, {[]byte(`{"a": "z"}`), "secret", "sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89,sha256=f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", true}, + {[]byte(``), "secret", "f9e66e179b6747ae54108f82f8ade8b3c25d76fd30afde6c395822c530196169", "f9e66e179b6747ae54108f82f8ade8b3c25d76fd30afde6c395822c530196169", true}, // failures {[]byte(`{"a": "z"}`), "secret", "XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", false}, {[]byte(`{"a": "z"}`), "secret", "sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", false}, {[]byte(`{"a": "z"}`), "secret", "sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89,sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", false}, {[]byte(`{"a": "z"}`), "", "XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "", false}, + {[]byte(``), "secret", "XXX66e179b6747ae54108f82f8ade8b3c25d76fd30afde6c395822c530196169", "f9e66e179b6747ae54108f82f8ade8b3c25d76fd30afde6c395822c530196169", false}, } func TestCheckPayloadSignature256(t *testing.T) { @@ -109,9 +113,11 @@ var checkPayloadSignature512Tests = []struct { }{ {[]byte(`{"a": "z"}`), "secret", "4ab17cc8ec668ead8bf498f87f8f32848c04d5ca3c9bcfcd3db9363f0deb44e580b329502a7fdff633d4d8fca301cc5c94a55a2fec458c675fb0ff2655898324", "4ab17cc8ec668ead8bf498f87f8f32848c04d5ca3c9bcfcd3db9363f0deb44e580b329502a7fdff633d4d8fca301cc5c94a55a2fec458c675fb0ff2655898324", true}, {[]byte(`{"a": "z"}`), "secret", "sha512=4ab17cc8ec668ead8bf498f87f8f32848c04d5ca3c9bcfcd3db9363f0deb44e580b329502a7fdff633d4d8fca301cc5c94a55a2fec458c675fb0ff2655898324", "4ab17cc8ec668ead8bf498f87f8f32848c04d5ca3c9bcfcd3db9363f0deb44e580b329502a7fdff633d4d8fca301cc5c94a55a2fec458c675fb0ff2655898324", true}, + {[]byte(``), "secret", "b0e9650c5faf9cd8ae02276671545424104589b3656731ec193b25d01b07561c27637c2d4d68389d6cf5007a8632c26ec89ba80a01c77a6cdd389ec28db43901", "b0e9650c5faf9cd8ae02276671545424104589b3656731ec193b25d01b07561c27637c2d4d68389d6cf5007a8632c26ec89ba80a01c77a6cdd389ec28db43901", true}, // failures {[]byte(`{"a": "z"}`), "secret", "74a0081f5b5988f4f3e8b8dd34dadc6291611f2e6260635a7e1535f8e95edb97ff520ba8b152e8ca5760ac42639854f3242e29efc81be73a8bf52d474d31ffea", "4ab17cc8ec668ead8bf498f87f8f32848c04d5ca3c9bcfcd3db9363f0deb44e580b329502a7fdff633d4d8fca301cc5c94a55a2fec458c675fb0ff2655898324", false}, {[]byte(`{"a": "z"}`), "", "74a0081f5b5988f4f3e8b8dd34dadc6291611f2e6260635a7e1535f8e95edb97ff520ba8b152e8ca5760ac42639854f3242e29efc81be73a8bf52d474d31ffea", "", false}, + {[]byte(``), "secret", "XXX9650c5faf9cd8ae02276671545424104589b3656731ec193b25d01b07561c27637c2d4d68389d6cf5007a8632c26ec89ba80a01c77a6cdd389ec28db43901", "b0e9650c5faf9cd8ae02276671545424104589b3656731ec193b25d01b07561c27637c2d4d68389d6cf5007a8632c26ec89ba80a01c77a6cdd389ec28db43901", false}, } func TestCheckPayloadSignature512(t *testing.T) { @@ -502,7 +508,8 @@ var andRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "z", "B": "y"}, nil, nil, []byte{}, + &map[string]interface{}{"A": "z", "B": "y"}, nil, nil, + []byte{}, true, false, }, { @@ -511,7 +518,8 @@ var andRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "z", "B": "Y"}, nil, nil, []byte{}, + &map[string]interface{}{"A": "z", "B": "Y"}, nil, nil, + []byte{}, false, false, }, // Complex test to cover Rules.Evaluate @@ -537,7 +545,8 @@ var andRuleTests = []struct { }, }, }, - &map[string]interface{}{"A": "z", "B": "y", "C": "x", "D": "w", "E": "X", "F": "X"}, nil, nil, []byte{}, + &map[string]interface{}{"A": "z", "B": "y", "C": "x", "D": "w", "E": "X", "F": "X"}, nil, nil, + []byte{}, true, false, }, {"empty rule", AndRule{{}}, nil, nil, nil, nil, false, false}, @@ -573,7 +582,8 @@ var orRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "z", "B": "X"}, nil, nil, []byte{}, + &map[string]interface{}{"A": "z", "B": "X"}, nil, nil, + []byte{}, true, false, }, { @@ -582,7 +592,8 @@ var orRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "X", "B": "y"}, nil, nil, []byte{}, + &map[string]interface{}{"A": "X", "B": "y"}, nil, nil, + []byte{}, true, false, }, { @@ -591,7 +602,8 @@ var orRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "Z", "B": "Y"}, nil, nil, []byte{}, + &map[string]interface{}{"A": "Z", "B": "Y"}, nil, nil, + []byte{}, false, false, }, // failures @@ -600,7 +612,8 @@ var orRuleTests = []struct { OrRule{ {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, }, - &map[string]interface{}{"Y": "Z"}, nil, nil, []byte{}, + &map[string]interface{}{"Y": "Z"}, nil, nil, + []byte{}, false, true, }, } diff --git a/test/hooks.json.tmpl b/test/hooks.json.tmpl index 0c7fc0c..71673ce 100644 --- a/test/hooks.json.tmpl +++ b/test/hooks.json.tmpl @@ -259,5 +259,29 @@ "name": "passed" } ], + }, + { + "id": "empty-payload-signature", + "execute-command": "{{ .Hookecho }}", + "command-working-directory": "/", + "include-command-output-in-response": true, + "trigger-rule": + { + "and": + [ + { + "match": + { + "type": "payload-hash-sha1", + "secret": "mysecret", + "parameter": + { + "source": "header", + "name": "X-Hub-Signature" + } + } + } + ] + } } ] diff --git a/test/hooks.yaml.tmpl b/test/hooks.yaml.tmpl index f1e1204..7c6e201 100644 --- a/test/hooks.yaml.tmpl +++ b/test/hooks.yaml.tmpl @@ -150,3 +150,16 @@ - id: warn-on-space execute-command: '{{ .Hookecho }} foo' include-command-output-in-response: true + +- id: empty-payload-signature + include-command-output-in-response: true + execute-command: '{{ .Hookecho }}' + command-working-directory: / + trigger-rule: + and: + - match: + parameter: + source: header + name: X-Hub-Signature + secret: mysecret + type: payload-hash-sha1 diff --git a/webhook_test.go b/webhook_test.go index ebf350d..6dda780 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -77,7 +77,7 @@ func TestWebhook(t *testing.T) { for _, tt := range hookHandlerTests { t.Run(tt.desc+"@"+hookTmpl, func(t *testing.T) { ip, port := serverAddress(t) - args := []string{fmt.Sprintf("-hooks=%s", configPath), fmt.Sprintf("-ip=%s", ip), fmt.Sprintf("-port=%s", port), "-verbose"} + args := []string{fmt.Sprintf("-hooks=%s", configPath), fmt.Sprintf("-ip=%s", ip), fmt.Sprintf("-port=%s", port), "-debug"} if len(tt.cliMethods) != 0 { args = append(args, "-http-methods="+strings.Join(tt.cliMethods, ",")) @@ -111,6 +111,7 @@ func TestWebhook(t *testing.T) { var res *http.Response req.Header.Add("Content-Type", tt.contentType) + req.ContentLength = int64(len(tt.body)) client := &http.Client{} res, err = client.Do(req) @@ -663,6 +664,19 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 ``, }, + { + "empty-payload-signature", // allow empty payload signature validation + "empty-payload-signature", + nil, + "POST", + map[string]string{"X-Hub-Signature": "33f9d709782f62b8b4a0178586c65ab098a39fe2"}, + "application/json", + ``, + http.StatusOK, + ``, + ``, + }, + // test with disallowed global HTTP method {"global disallowed method", "bitbucket", []string{"Post "}, "GET", nil, `{}`, "application/json", http.StatusMethodNotAllowed, ``, ``}, // test with disallowed HTTP method From cc98de88ce28775136e66fbdeba7a0af4fa33d14 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Thu, 28 May 2020 18:17:41 -0500 Subject: [PATCH 03/26] Fix godoc comment on LogEntry.Panic --- internal/middleware/logger.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/middleware/logger.go b/internal/middleware/logger.go index 50c6a44..5e74360 100644 --- a/internal/middleware/logger.go +++ b/internal/middleware/logger.go @@ -50,7 +50,7 @@ func (l *LogEntry) Write(status, bytes int, elapsed time.Duration) { log.Print(l.buf.String()) } -/// Panic prints the call stack for a panic. +// Panic prints the call stack for a panic. func (l *LogEntry) Panic(v interface{}, stack []byte) { e := l.NewLogEntry(l.req).(*LogEntry) fmt.Fprintf(e.buf, "panic: %#v", v) From 3d824b47b74547df7bfbe681c5e4250bec9405bf Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Thu, 28 May 2020 18:20:07 -0500 Subject: [PATCH 04/26] Rename var to avoid shadowing bytes package importShadow: shadow of imported package 'bytes' (gocritic) --- internal/middleware/logger.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/middleware/logger.go b/internal/middleware/logger.go index 5e74360..899dd1c 100644 --- a/internal/middleware/logger.go +++ b/internal/middleware/logger.go @@ -39,13 +39,13 @@ type LogEntry struct { } // Write constructs and writes the final log entry. -func (l *LogEntry) Write(status, bytes int, elapsed time.Duration) { +func (l *LogEntry) Write(status, totalBytes int, elapsed time.Duration) { rid := GetReqID(l.req.Context()) if rid != "" { fmt.Fprintf(l.buf, "[%s] ", rid) } - fmt.Fprintf(l.buf, "%03d | %s | %s | ", status, humanize.IBytes(uint64(bytes)), elapsed) + fmt.Fprintf(l.buf, "%03d | %s | %s | ", status, humanize.IBytes(uint64(totalBytes)), elapsed) l.buf.WriteString(l.req.Host + " | " + l.req.Method + " " + l.req.RequestURI) log.Print(l.buf.String()) } From c9199d62e46eabd03ce6c55a04746e0f8b0cb427 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Thu, 28 May 2020 18:13:28 -0500 Subject: [PATCH 05/26] Tighten file permissions on pidfile creation Fixes report from gosec: "G306: Expect WriteFile permissions to be 0600 or less." Also, use new octal number formatting. --- internal/pidfile/pidfile.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/pidfile/pidfile.go b/internal/pidfile/pidfile.go index c88c159..e602dcb 100644 --- a/internal/pidfile/pidfile.go +++ b/internal/pidfile/pidfile.go @@ -35,10 +35,10 @@ func New(path string) (*PIDFile, error) { return nil, err } // Note MkdirAll returns nil if a directory already exists - if err := MkdirAll(filepath.Dir(path), os.FileMode(0755)); err != nil { + if err := MkdirAll(filepath.Dir(path), os.FileMode(0o755)); err != nil { return nil, err } - if err := ioutil.WriteFile(path, []byte(fmt.Sprintf("%d", os.Getpid())), 0644); err != nil { + if err := ioutil.WriteFile(path, []byte(fmt.Sprintf("%d", os.Getpid())), 0o600); err != nil { return nil, err } From eefcd7f7d5be1407142f0e6713484d162588c3b3 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Mon, 27 Jul 2020 12:59:58 -0500 Subject: [PATCH 06/26] Require Go 1.14 When go.mod specifies go 1.14 or higher, the go tools now verify that vendor/modules.txt is consistent with go.mod. Fixed by running `go mod vendor`. --- .travis.yml | 2 +- go.mod | 2 +- vendor/modules.txt | 17 +++++++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 86f29d5..54e4652 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ language: go go: - - 1.13.x + - 1.14.x - master os: diff --git a/go.mod b/go.mod index bc7efb7..48f350c 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/adnanh/webhook -go 1.13 +go 1.14 require ( github.com/clbanning/mxj v1.8.4 diff --git a/vendor/modules.txt b/vendor/modules.txt index 06d8a6a..8c121cf 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1,20 +1,37 @@ # github.com/clbanning/mxj v1.8.4 +## explicit github.com/clbanning/mxj # github.com/dustin/go-humanize v1.0.0 +## explicit github.com/dustin/go-humanize +# github.com/fsnotify/fsnotify v1.4.7 +## explicit # github.com/ghodss/yaml v1.0.0 +## explicit github.com/ghodss/yaml # github.com/go-chi/chi v4.0.2+incompatible +## explicit github.com/go-chi/chi github.com/go-chi/chi/middleware # github.com/gofrs/uuid v3.2.0+incompatible +## explicit github.com/gofrs/uuid # github.com/gorilla/mux v1.7.3 +## explicit github.com/gorilla/mux +# github.com/kr/pretty v0.1.0 +## explicit +# golang.org/x/net v0.0.0-20191209160850-c0dbc17a3553 +## explicit # golang.org/x/sys v0.0.0-20191228213918-04cbcbbfeed8 +## explicit golang.org/x/sys/unix golang.org/x/sys/windows +# gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 +## explicit # gopkg.in/fsnotify.v1 v1.4.2 +## explicit gopkg.in/fsnotify.v1 # gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7 +## explicit gopkg.in/yaml.v2 From fb9b22a118d68675ac061e100ac89b633df40005 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabe=20Ga=C5=82=C4=85zka?= <38294951+flawedworld@users.noreply.github.com> Date: Wed, 22 Jul 2020 16:37:22 +0100 Subject: [PATCH 07/26] Change minimum golang version to 1.14 in README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9527262..1706b79 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ If you don't have time to waste configuring, hosting, debugging and maintaining # Getting started ## Installation ### Building from source -To get started, first make sure you've properly set up your [Go](http://golang.org/doc/install) 1.12 or newer environment and then run +To get started, first make sure you've properly set up your [Go](http://golang.org/doc/install) 1.14 or newer environment and then run ```bash $ go build github.com/adnanh/webhook ``` From 0e90ccb441288b3068d0f77cdd9bef37824ffaab Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Wed, 29 Jul 2020 09:49:02 -0500 Subject: [PATCH 08/26] Add support for top-level JSON array in payload Detect if leading character in JSON payload is an array bracket. If found, decode payload into an interface{} and then save the results into payload["root"]. References to payload values would need to reference the leading, "virtual" root node (i.e. "root.0.name"). Fixes #215 --- docs/Hook-Examples.md | 51 +++++++++++++++++++++++++++++++++++++++++++ test/hooks.json.tmpl | 16 ++++++++++++++ test/hooks.yaml.tmpl | 12 ++++++++++ webhook.go | 31 +++++++++++++++++++++----- webhook_test.go | 22 +++++++++++++++++++ 5 files changed, 127 insertions(+), 5 deletions(-) diff --git a/docs/Hook-Examples.md b/docs/Hook-Examples.md index 23896a7..cb4145e 100644 --- a/docs/Hook-Examples.md +++ b/docs/Hook-Examples.md @@ -425,6 +425,57 @@ Travis sends webhooks as `payload=`, so the payload needs to be par ] ``` +## JSON Array Payload + +If the JSON payload is an array instead of an object, `webhook` will process the payload and place it into a "root" object. +Therefore, references to payload values must begin with `root.`. + +For example, given the following payload (taken from the Sendgrid Event Webhook documentation): +```json +[ + { + "email": "example@test.com", + "timestamp": 1513299569, + "smtp-id": "<14c5d75ce93.dfd.64b469@ismtpd-555>", + "event": "processed", + "category": "cat facts", + "sg_event_id": "sg_event_id", + "sg_message_id": "sg_message_id" + }, + { + "email": "example@test.com", + "timestamp": 1513299569, + "smtp-id": "<14c5d75ce93.dfd.64b469@ismtpd-555>", + "event": "deferred", + "category": "cat facts", + "sg_event_id": "sg_event_id", + "sg_message_id": "sg_message_id", + "response": "400 try again later", + "attempt": "5" + } +] +``` + +A reference to the second item in the array would look like this: +```json +[ + { + "id": "sendgrid", + "execute-command": "{{ .Hookecho }}", + "trigger-rule": { + "match": { + "type": "value", + "parameter": { + "source": "payload", + "name": "root.1.event" + }, + "value": "deferred" + } + } + } +] +``` + ## XML Payload Given the following payload: diff --git a/test/hooks.json.tmpl b/test/hooks.json.tmpl index 71673ce..7fe7c8c 100644 --- a/test/hooks.json.tmpl +++ b/test/hooks.json.tmpl @@ -168,6 +168,22 @@ ], } }, + { + "id": "sendgrid", + "execute-command": "{{ .Hookecho }}", + "command-working-directory": "/", + "response-message": "success", + "trigger-rule": { + "match": { + "type": "value", + "parameter": { + "source": "payload", + "name": "root.0.event" + }, + "value": "processed" + } + } + }, { "id": "plex", "execute-command": "{{ .Hookecho }}", diff --git a/test/hooks.yaml.tmpl b/test/hooks.yaml.tmpl index 7c6e201..ace4277 100644 --- a/test/hooks.yaml.tmpl +++ b/test/hooks.yaml.tmpl @@ -97,6 +97,18 @@ name: "app.messages.message.#text" value: "Hello!!" +- id: sendgrid + execute-command: '{{ .Hookecho }}' + command-working-directory: / + response-message: success + trigger-rule: + match: + type: value + parameter: + source: payload + name: root.0.event + value: processed + - id: plex trigger-rule: match: diff --git a/webhook.go b/webhook.go index 1af2ce5..6a258c4 100644 --- a/webhook.go +++ b/webhook.go @@ -16,6 +16,7 @@ import ( "path/filepath" "strings" "time" + "unicode" "github.com/adnanh/webhook/internal/hook" "github.com/adnanh/webhook/internal/middleware" @@ -141,7 +142,7 @@ func main() { } if *logPath != "" { - file, err := os.OpenFile(*logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666) + file, err := os.OpenFile(*logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o666) if err != nil { logQueue = append(logQueue, fmt.Sprintf("error opening log file %q: %v", *logPath, err)) // we'll bail out below @@ -385,9 +386,29 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { decoder := json.NewDecoder(bytes.NewReader(body)) decoder.UseNumber() - err := decoder.Decode(&payload) - if err != nil { - log.Printf("[%s] error parsing JSON payload %+v\n", rid, err) + var firstChar byte + for i := 0; i < len(body); i++ { + if unicode.IsSpace(rune(body[i])) { + continue + } + firstChar = body[i] + break + } + + if firstChar == byte('[') { + var arrayPayload interface{} + err := decoder.Decode(&arrayPayload) + if err != nil { + log.Printf("[%s] error parsing JSON array payload %+v\n", rid, err) + } + + payload = make(map[string]interface{}, 1) + payload["root"] = arrayPayload + } else { + err := decoder.Decode(&payload) + if err != nil { + log.Printf("[%s] error parsing JSON payload %+v\n", rid, err) + } } case strings.Contains(contentType, "x-www-form-urlencoded"): @@ -648,7 +669,7 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in return string(out), err } -func writeHttpResponseCode(w http.ResponseWriter, rid string, hookId string, responseCode int) { +func writeHttpResponseCode(w http.ResponseWriter, rid, hookId string, responseCode int) { // Check if the given return code is supported by the http package // by testing if there is a StatusText for this code. if len(http.StatusText(responseCode)) > 0 { diff --git a/webhook_test.go b/webhook_test.go index 6dda780..4671c24 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -547,6 +547,28 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 `success`, ``, }, + { + "payload-json-array", + "sendgrid", + nil, + "POST", + nil, + "application/json", + `[ + { + "email": "example@test.com", + "timestamp": 1513299569, + "smtp-id": "<14c5d75ce93.dfd.64b469@ismtpd-555>", + "event": "processed", + "category": "cat facts", + "sg_event_id": "sg_event_id", + "sg_message_id": "sg_message_id" + } +]`, + http.StatusOK, + `success`, + ``, + }, { "multipart", "plex", From 534e99bf131e9a331761eceaeb45a8e9ef1ee776 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Wed, 29 Jul 2020 17:13:33 -0500 Subject: [PATCH 09/26] Add a table of contents to some of the docs --- docs/Hook-Examples.md | 16 ++++++++++++++++ docs/Hook-Rules.md | 17 ++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/docs/Hook-Examples.md b/docs/Hook-Examples.md index 23896a7..48ad38b 100644 --- a/docs/Hook-Examples.md +++ b/docs/Hook-Examples.md @@ -1,6 +1,22 @@ # Hook examples This page is still work in progress. Feel free to contribute! +### Table of Contents + +* [Incoming Github webhook](#incoming-github-webhook) +* [Incoming Bitbucket webhook](#incoming-bitbucket-webhook) +* [Incoming Gitlab webhook](#incoming-gitlab-webhook) +* [Incoming Gogs webhook](#incoming-gogs-webhook) +* [Incoming Gitea webhook](#incoming-gitea-webhook) +* [Slack slash command](#slack-slash-command) +* [A simple webhook with a secret key in GET query](#a-simple-webhook-with-a-secret-key-in-get-query) +* [JIRA Webhooks](#jira-webhooks) +* [Pass File-to-command sample](#pass-file-to-command-sample) +* [Incoming Scalr Webhook](#incoming-scalr-webhook) +* [Travis CI webhook](#travis-ci-webhook) +* [XML Payload](#xml-payload) +* [Multipart Form Data](#multipart-form-data) + ## Incoming Github webhook ```json [ diff --git a/docs/Hook-Rules.md b/docs/Hook-Rules.md index e1f92f6..0ff30b3 100644 --- a/docs/Hook-Rules.md +++ b/docs/Hook-Rules.md @@ -1,5 +1,20 @@ # Hook rules +### Table of Contents + +* [And](#and) +* [Or](#or) +* [Not](#not) +* [Multi-level](#multi-level) +* [Match](#match) + * [Match value](#match-value) + * [Match regex](#match-regex) + * [Match payload-hash-sha1](#match-payload-hash-sha1) + * [Match payload-hash-sha256](#match-payload-hash-sha256) + * [Match payload-hash-sha512](#match-payload-hash-sha512) + * [Match Whitelisted IP range](#match-whitelisted-ip-range) + * [Match scalr-signature](#match-scalr-signature) + ## And *And rule* will evaluate to _true_, if and only if all of the sub rules evaluate to _true_. ```json @@ -267,4 +282,4 @@ Given the time check make sure that NTP is enabled on both your Scalr and webhoo "secret": "Scalr-provided signing key" } } -``` \ No newline at end of file +``` From ae5e9e7894fee973b40ad66d28ead6fb7d368e20 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Fri, 31 Jul 2020 11:58:12 -0500 Subject: [PATCH 10/26] Update ExtractParameterAsString to return JSON on complex types Fixes #448 --- internal/hook/hook.go | 17 +++++++++++++++-- internal/hook/hook_test.go | 7 +++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/internal/hook/hook.go b/internal/hook/hook.go index dd5546c..2c2d787 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -392,14 +392,27 @@ func GetParameter(s string, params interface{}) (interface{}, error) { return nil, &ParameterNodeError{s} } -// ExtractParameterAsString extracts value from interface{} as string based on the passed string +// ExtractParameterAsString extracts value from interface{} as string based on +// the passed string. Complex data types are rendered as JSON instead of the Go +// Stringer format. func ExtractParameterAsString(s string, params interface{}) (string, error) { pValue, err := GetParameter(s, params) if err != nil { return "", err } - return fmt.Sprintf("%v", pValue), nil + switch v := reflect.ValueOf(pValue); v.Kind() { + case reflect.Array, reflect.Map, reflect.Slice: + r, err := json.Marshal(pValue) + if err != nil { + return "", err + } + + return string(r), nil + + default: + return fmt.Sprintf("%v", pValue), nil + } } // Argument type specifies the parameter key name and the source it should diff --git a/internal/hook/hook_test.go b/internal/hook/hook_test.go index e348930..46bd444 100644 --- a/internal/hook/hook_test.go +++ b/internal/hook/hook_test.go @@ -223,6 +223,9 @@ var extractParameterTests = []struct { {"a.b.0", map[string]interface{}{"a": map[string]interface{}{"b": []interface{}{"x", "y", "z"}}}, "x", true}, {"a.1.b", map[string]interface{}{"a": []interface{}{map[string]interface{}{"b": "y"}, map[string]interface{}{"b": "z"}}}, "z", true}, {"a.1.b.c", map[string]interface{}{"a": []interface{}{map[string]interface{}{"b": map[string]interface{}{"c": "y"}}, map[string]interface{}{"b": map[string]interface{}{"c": "z"}}}}, "z", true}, + {"b", map[string]interface{}{"b": map[string]interface{}{"z": 1}}, `{"z":1}`, true}, + {"c", map[string]interface{}{"c": []interface{}{"y", "z"}}, `["y","z"]`, true}, + {"d", map[string]interface{}{"d": [2]interface{}{"y", "z"}}, `["y","z"]`, true}, // failures {"check_nil", nil, "", false}, {"a.X", map[string]interface{}{"a": map[string]interface{}{"b": "z"}}, "", false}, // non-existent parameter reference @@ -239,7 +242,7 @@ func TestExtractParameter(t *testing.T) { for _, tt := range extractParameterTests { value, err := ExtractParameterAsString(tt.s, tt.params) if (err == nil) != tt.ok || value != tt.value { - t.Errorf("failed to extract parameter %q:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, err:%s}", tt.s, tt.value, tt.ok, value, err) + t.Errorf("failed to extract parameter %q:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, err:%v}", tt.s, tt.value, tt.ok, value, err) } } } @@ -266,7 +269,7 @@ func TestArgumentGet(t *testing.T) { a := Argument{tt.source, tt.name, "", false} value, err := a.Get(tt.headers, tt.query, tt.payload) if (err == nil) != tt.ok || value != tt.value { - t.Errorf("failed to get {%q, %q}:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, err:%s}", tt.source, tt.name, tt.value, tt.ok, value, err) + t.Errorf("failed to get {%q, %q}:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, err:%v}", tt.source, tt.name, tt.value, tt.ok, value, err) } } } From dd5fa204157215aa9f38fc56929f284cadf321a5 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Tue, 15 Sep 2020 13:05:24 -0500 Subject: [PATCH 11/26] Log stdlib error on failed exec.LookPath The error returned by exec.LookPath was never surfaced to the user. Without that detail, the user can't tell the difference between a non-existent path and a permissions issue. Additionally, when ExecuteCommand is an absolute path, we were still attempting to prepend the CommandWorkingDirectory if the ExecuteCommand was not found, which made it difficult to know which path the user intended to execute. This commit simplifies the logic to avoid multiple attempts with ExecuteCommand is an absolute path and changes the error message from: error locating command: '/path/to/file' to: error in exec: "/path/to/file": stat /path/to/file: no such file or directory error in exec: "/path/to/file": permission denied Fixes #457 --- webhook.go | 14 +++++++------- webhook_test.go | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/webhook.go b/webhook.go index 6a258c4..26872ed 100644 --- a/webhook.go +++ b/webhook.go @@ -581,16 +581,16 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in var errors []error // check the command exists - cmdPath, err := exec.LookPath(h.ExecuteCommand) - if err != nil { - // give a last chance, maybe is a relative path - relativeToCwd := filepath.Join(h.CommandWorkingDirectory, h.ExecuteCommand) - // check the command exists - cmdPath, err = exec.LookPath(relativeToCwd) + var lookpath string + if filepath.IsAbs(h.ExecuteCommand) || h.CommandWorkingDirectory == "" { + lookpath = h.ExecuteCommand + } else { + lookpath = filepath.Join(h.CommandWorkingDirectory, h.ExecuteCommand) } + cmdPath, err := exec.LookPath(lookpath) if err != nil { - log.Printf("[%s] error locating command: '%s'", rid, h.ExecuteCommand) + log.Printf("[%s] error in %s", rid, err) // check if parameters specified in execute-command by mistake if strings.IndexByte(h.ExecuteCommand, ' ') != -1 { diff --git a/webhook_test.go b/webhook_test.go index 4671c24..353d22d 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -720,7 +720,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 // 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 locating command.*use 'pass[-]arguments[-]to[-]command' to specify args`}, + {"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:`}, } From c6603894c1215ded1e717594b05dfb7cdd2dd81b Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Fri, 25 Sep 2020 19:46:06 -0500 Subject: [PATCH 12/26] Add Request object to hook package to simplify API To avoid having to pass around so many parameters to the hook package, create a Request object to store all request-specific data. Update APIs accordingly. --- internal/hook/hook.go | 121 ++++++++++++++++---------- internal/hook/hook_test.go | 172 ++++++++++++++++++++++++------------- webhook.go | 153 +++++++++++++++++---------------- webhook_test.go | 6 +- 4 files changed, 269 insertions(+), 183 deletions(-) diff --git a/internal/hook/hook.go b/internal/hook/hook.go index 2c2d787..e6a9272 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -17,6 +17,7 @@ import ( "log" "math" "net" + "net/http" "net/textproto" "os" "reflect" @@ -47,6 +48,30 @@ const ( EnvNamespace string = "HOOK_" ) +// Request represents a webhook request. +type Request struct { + // The request ID set by the RequestID middleware. + ID string + + // The Content-Type of the request. + ContentType string + + // The raw request body. + Body []byte + + // Headers is a map of the parsed headers. + Headers map[string]interface{} + + // Query is a map of the parsed URL query values. + Query map[string]interface{} + + // Payload is a map of the parsed payload. + Payload map[string]interface{} + + // The underlying HTTP request. + RawRequest *http.Request +} + // ParameterNodeError describes an error walking a parameter node. type ParameterNodeError struct { key string @@ -222,22 +247,26 @@ func CheckPayloadSignature512(payload []byte, secret, signature string) (string, return ValidateMAC(payload, hmac.New(sha512.New, []byte(secret)), signatures) } -func CheckScalrSignature(headers map[string]interface{}, body []byte, signingKey string, checkDate bool) (bool, error) { - // Check for the signature and date headers - if _, ok := headers["X-Signature"]; !ok { +func CheckScalrSignature(r *Request, signingKey string, checkDate bool) (bool, error) { + if r.Headers == nil { return false, nil } - if _, ok := headers["Date"]; !ok { + + // Check for the signature and date headers + if _, ok := r.Headers["X-Signature"]; !ok { + return false, nil + } + if _, ok := r.Headers["Date"]; !ok { return false, nil } if signingKey == "" { return false, errors.New("signature validation signing key can not be empty") } - providedSignature := headers["X-Signature"].(string) - dateHeader := headers["Date"].(string) + providedSignature := r.Headers["X-Signature"].(string) + dateHeader := r.Headers["Date"].(string) mac := hmac.New(sha1.New, []byte(signingKey)) - mac.Write(body) + mac.Write(r.Body) mac.Write([]byte(dateHeader)) expectedSignature := hex.EncodeToString(mac.Sum(nil)) @@ -426,41 +455,41 @@ type Argument struct { // Get Argument method returns the value for the Argument's key name // based on the Argument's source -func (ha *Argument) Get(headers, query, payload *map[string]interface{}) (string, error) { +func (ha *Argument) Get(r *Request) (string, error) { var source *map[string]interface{} key := ha.Name switch ha.Source { case SourceHeader: - source = headers + source = &r.Headers key = textproto.CanonicalMIMEHeaderKey(ha.Name) case SourceQuery, SourceQueryAlias: - source = query + source = &r.Query case SourcePayload: - source = payload + source = &r.Payload case SourceString: return ha.Name, nil case SourceEntirePayload: - r, err := json.Marshal(payload) + res, err := json.Marshal(&r.Payload) if err != nil { return "", err } - return string(r), nil + return string(res), nil case SourceEntireHeaders: - r, err := json.Marshal(headers) + res, err := json.Marshal(&r.Headers) if err != nil { return "", err } - return string(r), nil + return string(res), nil case SourceEntireQuery: - r, err := json.Marshal(query) + res, err := json.Marshal(&r.Query) if err != nil { return "", err } - return string(r), nil + return string(res), nil } if source != nil { @@ -545,11 +574,11 @@ type Hook struct { // ParseJSONParameters decodes specified arguments to JSON objects and replaces the // string with the newly created object -func (h *Hook) ParseJSONParameters(headers, query, payload *map[string]interface{}) []error { +func (h *Hook) ParseJSONParameters(r *Request) []error { errors := make([]error, 0) for i := range h.JSONStringParameters { - arg, err := h.JSONStringParameters[i].Get(headers, query, payload) + arg, err := h.JSONStringParameters[i].Get(r) if err != nil { errors = append(errors, &ArgumentError{h.JSONStringParameters[i]}) } else { @@ -568,11 +597,11 @@ func (h *Hook) ParseJSONParameters(headers, query, payload *map[string]interface switch h.JSONStringParameters[i].Source { case SourceHeader: - source = headers + source = &r.Headers case SourcePayload: - source = payload + source = &r.Payload case SourceQuery, SourceQueryAlias: - source = query + source = &r.Query } if source != nil { @@ -598,14 +627,14 @@ func (h *Hook) ParseJSONParameters(headers, query, payload *map[string]interface // ExtractCommandArguments creates a list of arguments, based on the // PassArgumentsToCommand property that is ready to be used with exec.Command() -func (h *Hook) ExtractCommandArguments(headers, query, payload *map[string]interface{}) ([]string, []error) { +func (h *Hook) ExtractCommandArguments(r *Request) ([]string, []error) { args := make([]string, 0) errors := make([]error, 0) args = append(args, h.ExecuteCommand) for i := range h.PassArgumentsToCommand { - arg, err := h.PassArgumentsToCommand[i].Get(headers, query, payload) + arg, err := h.PassArgumentsToCommand[i].Get(r) if err != nil { args = append(args, "") errors = append(errors, &ArgumentError{h.PassArgumentsToCommand[i]}) @@ -625,11 +654,11 @@ func (h *Hook) ExtractCommandArguments(headers, query, payload *map[string]inter // ExtractCommandArgumentsForEnv creates a list of arguments in key=value // format, based on the PassEnvironmentToCommand property that is ready to be used // with exec.Command(). -func (h *Hook) ExtractCommandArgumentsForEnv(headers, query, payload *map[string]interface{}) ([]string, []error) { +func (h *Hook) ExtractCommandArgumentsForEnv(r *Request) ([]string, []error) { args := make([]string, 0) errors := make([]error, 0) for i := range h.PassEnvironmentToCommand { - arg, err := h.PassEnvironmentToCommand[i].Get(headers, query, payload) + arg, err := h.PassEnvironmentToCommand[i].Get(r) if err != nil { errors = append(errors, &ArgumentError{h.PassEnvironmentToCommand[i]}) continue @@ -661,11 +690,11 @@ type FileParameter struct { // ExtractCommandArgumentsForFile creates a list of arguments in key=value // format, based on the PassFileToCommand property that is ready to be used // with exec.Command(). -func (h *Hook) ExtractCommandArgumentsForFile(headers, query, payload *map[string]interface{}) ([]FileParameter, []error) { +func (h *Hook) ExtractCommandArgumentsForFile(r *Request) ([]FileParameter, []error) { args := make([]FileParameter, 0) errors := make([]error, 0) for i := range h.PassFileToCommand { - arg, err := h.PassFileToCommand[i].Get(headers, query, payload) + arg, err := h.PassFileToCommand[i].Get(r) if err != nil { errors = append(errors, &ArgumentError{h.PassFileToCommand[i]}) continue @@ -772,16 +801,16 @@ type Rules struct { // Evaluate finds the first rule property that is not nil and returns the value // it evaluates to -func (r Rules) Evaluate(headers, query, payload *map[string]interface{}, body *[]byte, remoteAddr string) (bool, error) { +func (r Rules) Evaluate(req *Request) (bool, error) { switch { case r.And != nil: - return r.And.Evaluate(headers, query, payload, body, remoteAddr) + return r.And.Evaluate(req) case r.Or != nil: - return r.Or.Evaluate(headers, query, payload, body, remoteAddr) + return r.Or.Evaluate(req) case r.Not != nil: - return r.Not.Evaluate(headers, query, payload, body, remoteAddr) + return r.Not.Evaluate(req) case r.Match != nil: - return r.Match.Evaluate(headers, query, payload, body, remoteAddr) + return r.Match.Evaluate(req) } return false, nil @@ -791,11 +820,11 @@ func (r Rules) Evaluate(headers, query, payload *map[string]interface{}, body *[ type AndRule []Rules // Evaluate AndRule will return true if and only if all of ChildRules evaluate to true -func (r AndRule) Evaluate(headers, query, payload *map[string]interface{}, body *[]byte, remoteAddr string) (bool, error) { +func (r AndRule) Evaluate(req *Request) (bool, error) { res := true for _, v := range r { - rv, err := v.Evaluate(headers, query, payload, body, remoteAddr) + rv, err := v.Evaluate(req) if err != nil { return false, err } @@ -813,11 +842,11 @@ func (r AndRule) Evaluate(headers, query, payload *map[string]interface{}, body type OrRule []Rules // Evaluate OrRule will return true if any of ChildRules evaluate to true -func (r OrRule) Evaluate(headers, query, payload *map[string]interface{}, body *[]byte, remoteAddr string) (bool, error) { +func (r OrRule) Evaluate(req *Request) (bool, error) { res := false for _, v := range r { - rv, err := v.Evaluate(headers, query, payload, body, remoteAddr) + rv, err := v.Evaluate(req) if err != nil { return false, err } @@ -835,8 +864,8 @@ func (r OrRule) Evaluate(headers, query, payload *map[string]interface{}, body * type NotRule Rules // Evaluate NotRule will return true if and only if ChildRule evaluates to false -func (r NotRule) Evaluate(headers, query, payload *map[string]interface{}, body *[]byte, remoteAddr string) (bool, error) { - rv, err := Rules(r).Evaluate(headers, query, payload, body, remoteAddr) +func (r NotRule) Evaluate(req *Request) (bool, error) { + rv, err := Rules(r).Evaluate(req) return !rv, err } @@ -862,15 +891,15 @@ const ( ) // Evaluate MatchRule will return based on the type -func (r MatchRule) Evaluate(headers, query, payload *map[string]interface{}, body *[]byte, remoteAddr string) (bool, error) { +func (r MatchRule) Evaluate(req *Request) (bool, error) { if r.Type == IPWhitelist { - return CheckIPWhitelist(remoteAddr, r.IPRange) + return CheckIPWhitelist(req.RawRequest.RemoteAddr, r.IPRange) } if r.Type == ScalrSignature { - return CheckScalrSignature(*headers, *body, r.Secret, true) + return CheckScalrSignature(req, r.Secret, true) } - arg, err := r.Parameter.Get(headers, query, payload) + arg, err := r.Parameter.Get(req) if err == nil { switch r.Type { case MatchValue: @@ -878,13 +907,13 @@ func (r MatchRule) Evaluate(headers, query, payload *map[string]interface{}, bod case MatchRegex: return regexp.MatchString(r.Regex, arg) case MatchHashSHA1: - _, err := CheckPayloadSignature(*body, r.Secret, arg) + _, err := CheckPayloadSignature(req.Body, r.Secret, arg) return err == nil, err case MatchHashSHA256: - _, err := CheckPayloadSignature256(*body, r.Secret, arg) + _, err := CheckPayloadSignature256(req.Body, r.Secret, arg) return err == nil, err case MatchHashSHA512: - _, err := CheckPayloadSignature512(*body, r.Secret, arg) + _, err := CheckPayloadSignature512(req.Body, r.Secret, arg) return err == nil, err } } diff --git a/internal/hook/hook_test.go b/internal/hook/hook_test.go index 46bd444..c99edfb 100644 --- a/internal/hook/hook_test.go +++ b/internal/hook/hook_test.go @@ -1,6 +1,7 @@ package hook import ( + "net/http" "os" "reflect" "strings" @@ -136,7 +137,7 @@ func TestCheckPayloadSignature512(t *testing.T) { var checkScalrSignatureTests = []struct { description string headers map[string]interface{} - payload []byte + body []byte secret string expectedSignature string ok bool @@ -175,7 +176,11 @@ var checkScalrSignatureTests = []struct { func TestCheckScalrSignature(t *testing.T) { for _, testCase := range checkScalrSignatureTests { - valid, err := CheckScalrSignature(testCase.headers, testCase.payload, testCase.secret, false) + r := &Request{ + Headers: testCase.headers, + Body: testCase.body, + } + valid, err := CheckScalrSignature(r, testCase.secret, false) if valid != testCase.ok { t.Errorf("failed to check scalr signature fot test case: %s\nexpected ok:%#v, got ok:%#v}", testCase.description, testCase.ok, valid) @@ -249,25 +254,30 @@ func TestExtractParameter(t *testing.T) { var argumentGetTests = []struct { source, name string - headers, query, payload *map[string]interface{} + headers, query, payload map[string]interface{} 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, "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}, // 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"}, "", 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 } func TestArgumentGet(t *testing.T) { for _, tt := range argumentGetTests { a := Argument{tt.source, tt.name, "", false} - value, err := a.Get(tt.headers, tt.query, tt.payload) + r := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + } + value, err := a.Get(r) if (err == nil) != tt.ok || value != tt.value { t.Errorf("failed to get {%q, %q}:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, err:%v}", tt.source, tt.name, tt.value, tt.ok, value, err) } @@ -276,26 +286,31 @@ func TestArgumentGet(t *testing.T) { var hookParseJSONParametersTests = []struct { params []Argument - headers, query, payload *map[string]interface{} - rheaders, rquery, rpayload *map[string]interface{} + headers, query, payload map[string]interface{} + rheaders, rquery, rpayload map[string]interface{} ok bool }{ - {[]Argument{Argument{"header", "a", "", false}}, &map[string]interface{}{"A": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"A": map[string]interface{}{"b": "y"}}, nil, nil, true}, - {[]Argument{Argument{"url", "a", "", false}}, nil, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, nil, true}, - {[]Argument{Argument{"payload", "a", "", false}}, nil, nil, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, true}, - {[]Argument{Argument{"header", "z", "", false}}, &map[string]interface{}{"Z": `{}`}, nil, nil, &map[string]interface{}{"Z": map[string]interface{}{}}, nil, nil, true}, + {[]Argument{Argument{"header", "a", "", false}}, map[string]interface{}{"A": `{"b": "y"}`}, nil, nil, map[string]interface{}{"A": map[string]interface{}{"b": "y"}}, nil, nil, true}, + {[]Argument{Argument{"url", "a", "", false}}, nil, map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, nil, true}, + {[]Argument{Argument{"payload", "a", "", false}}, nil, nil, map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, true}, + {[]Argument{Argument{"header", "z", "", false}}, map[string]interface{}{"Z": `{}`}, nil, nil, map[string]interface{}{"Z": map[string]interface{}{}}, nil, nil, true}, // failures - {[]Argument{Argument{"header", "z", "", false}}, &map[string]interface{}{"Z": ``}, nil, nil, &map[string]interface{}{"Z": ``}, nil, nil, false}, // empty string - {[]Argument{Argument{"header", "y", "", false}}, &map[string]interface{}{"X": `{}`}, nil, nil, &map[string]interface{}{"X": `{}`}, nil, nil, false}, // missing parameter - {[]Argument{Argument{"string", "z", "", false}}, &map[string]interface{}{"Z": ``}, nil, nil, &map[string]interface{}{"Z": ``}, nil, nil, false}, // invalid argument source + {[]Argument{Argument{"header", "z", "", false}}, map[string]interface{}{"Z": ``}, nil, nil, map[string]interface{}{"Z": ``}, nil, nil, false}, // empty string + {[]Argument{Argument{"header", "y", "", false}}, map[string]interface{}{"X": `{}`}, nil, nil, map[string]interface{}{"X": `{}`}, nil, nil, false}, // missing parameter + {[]Argument{Argument{"string", "z", "", false}}, map[string]interface{}{"Z": ``}, nil, nil, map[string]interface{}{"Z": ``}, nil, nil, false}, // invalid argument source } func TestHookParseJSONParameters(t *testing.T) { for _, tt := range hookParseJSONParametersTests { h := &Hook{JSONStringParameters: tt.params} - err := h.ParseJSONParameters(tt.headers, tt.query, tt.payload) + r := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + } + err := h.ParseJSONParameters(r) if (err == nil) != tt.ok || !reflect.DeepEqual(tt.headers, tt.rheaders) { - t.Errorf("failed to parse %v:\nexpected %#v, ok: %v\ngot %#v, ok: %v", tt.params, *tt.rheaders, tt.ok, *tt.headers, (err == nil)) + t.Errorf("failed to parse %v:\nexpected %#v, ok: %v\ngot %#v, ok: %v", tt.params, tt.rheaders, tt.ok, tt.headers, (err == nil)) } } } @@ -303,19 +318,24 @@ func TestHookParseJSONParameters(t *testing.T) { var hookExtractCommandArgumentsTests = []struct { exec string args []Argument - headers, query, payload *map[string]interface{} + headers, query, payload map[string]interface{} value []string ok bool }{ - {"test", []Argument{Argument{"header", "a", "", false}}, &map[string]interface{}{"A": "z"}, nil, nil, []string{"test", "z"}, true}, + {"test", []Argument{Argument{"header", "a", "", false}}, map[string]interface{}{"A": "z"}, nil, nil, []string{"test", "z"}, true}, // failures - {"fail", []Argument{Argument{"payload", "a", "", false}}, &map[string]interface{}{"A": "z"}, nil, nil, []string{"fail", ""}, false}, + {"fail", []Argument{Argument{"payload", "a", "", false}}, map[string]interface{}{"A": "z"}, nil, nil, []string{"fail", ""}, false}, } func TestHookExtractCommandArguments(t *testing.T) { for _, tt := range hookExtractCommandArgumentsTests { h := &Hook{ExecuteCommand: tt.exec, PassArgumentsToCommand: tt.args} - value, err := h.ExtractCommandArguments(tt.headers, tt.query, tt.payload) + r := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + } + value, err := h.ExtractCommandArguments(r) if (err == nil) != tt.ok || !reflect.DeepEqual(value, tt.value) { t.Errorf("failed to extract args {cmd=%q, args=%v}:\nexpected %#v, ok: %v\ngot %#v, ok: %v", tt.exec, tt.args, tt.value, tt.ok, value, (err == nil)) } @@ -344,7 +364,7 @@ func TestHookExtractCommandArguments(t *testing.T) { var hookExtractCommandArgumentsForEnvTests = []struct { exec string args []Argument - headers, query, payload *map[string]interface{} + headers, query, payload map[string]interface{} value []string ok bool }{ @@ -352,14 +372,14 @@ var hookExtractCommandArgumentsForEnvTests = []struct { { "test", []Argument{Argument{"header", "a", "", false}}, - &map[string]interface{}{"A": "z"}, nil, nil, + map[string]interface{}{"A": "z"}, nil, nil, []string{"HOOK_a=z"}, true, }, { "test", []Argument{Argument{"header", "a", "MYKEY", false}}, - &map[string]interface{}{"A": "z"}, nil, nil, + map[string]interface{}{"A": "z"}, nil, nil, []string{"MYKEY=z"}, true, }, @@ -367,7 +387,7 @@ var hookExtractCommandArgumentsForEnvTests = []struct { { "fail", []Argument{Argument{"payload", "a", "", false}}, - &map[string]interface{}{"A": "z"}, nil, nil, + map[string]interface{}{"A": "z"}, nil, nil, []string{}, false, }, @@ -376,7 +396,12 @@ var hookExtractCommandArgumentsForEnvTests = []struct { func TestHookExtractCommandArgumentsForEnv(t *testing.T) { for _, tt := range hookExtractCommandArgumentsForEnvTests { h := &Hook{ExecuteCommand: tt.exec, PassEnvironmentToCommand: tt.args} - value, err := h.ExtractCommandArgumentsForEnv(tt.headers, tt.query, tt.payload) + r := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + } + value, err := h.ExtractCommandArgumentsForEnv(r) if (err == nil) != tt.ok || !reflect.DeepEqual(value, tt.value) { t.Errorf("failed to extract args for env {cmd=%q, args=%v}:\nexpected %#v, ok: %v\ngot %#v, ok: %v", tt.exec, tt.args, tt.value, tt.ok, value, (err == nil)) } @@ -454,24 +479,24 @@ func TestHooksMatch(t *testing.T) { var matchRuleTests = []struct { typ, regex, secret, value, ipRange string param Argument - headers, query, payload *map[string]interface{} + headers, query, payload map[string]interface{} body []byte remoteAddr string ok bool err bool }{ - {"value", "", "", "z", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", true, false}, - {"regex", "^z", "", "z", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", true, false}, - {"payload-hash-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": "b17e04cbb22afa8ffbff8796fc1894ed27badd9e"}, nil, nil, []byte(`{"a": "z"}`), "", true, false}, - {"payload-hash-sha256", "", "secret", "", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89"}, nil, nil, []byte(`{"a": "z"}`), "", true, false}, + {"value", "", "", "z", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", true, false}, + {"regex", "^z", "", "z", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", true, false}, + {"payload-hash-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "b17e04cbb22afa8ffbff8796fc1894ed27badd9e"}, nil, nil, []byte(`{"a": "z"}`), "", true, false}, + {"payload-hash-sha256", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89"}, nil, nil, []byte(`{"a": "z"}`), "", true, false}, // failures - {"value", "", "", "X", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, false}, - {"regex", "^X", "", "", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, false}, - {"value", "", "2", "X", "", Argument{"header", "a", "", false}, &map[string]interface{}{"Y": "z"}, nil, nil, []byte{}, "", false, true}, // reference invalid header + {"value", "", "", "X", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, false}, + {"regex", "^X", "", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, false}, + {"value", "", "2", "X", "", Argument{"header", "a", "", false}, map[string]interface{}{"Y": "z"}, nil, nil, []byte{}, "", false, true}, // reference invalid header // errors - {"regex", "*", "", "", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, true}, // invalid regex - {"payload-hash-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true}, // invalid hmac - {"payload-hash-sha256", "", "secret", "", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true}, // invalid hmac + {"regex", "*", "", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, true}, // invalid regex + {"payload-hash-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true}, // invalid hmac + {"payload-hash-sha256", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true}, // invalid hmac // IP whitelisting, valid cases {"ip-whitelist", "", "", "", "192.168.0.1/24", Argument{}, nil, nil, nil, []byte{}, "192.168.0.2:9000", true, false}, // valid IPv4, with range {"ip-whitelist", "", "", "", "192.168.0.1/24", Argument{}, nil, nil, nil, []byte{}, "192.168.0.2:9000", true, false}, // valid IPv4, with range @@ -490,7 +515,16 @@ var matchRuleTests = []struct { func TestMatchRule(t *testing.T) { for i, tt := range matchRuleTests { r := MatchRule{tt.typ, tt.regex, tt.secret, tt.value, tt.param, tt.ipRange} - ok, err := r.Evaluate(tt.headers, tt.query, tt.payload, &tt.body, tt.remoteAddr) + req := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + Body: tt.body, + RawRequest: &http.Request{ + RemoteAddr: tt.remoteAddr, + }, + } + ok, err := r.Evaluate(req) if ok != tt.ok || (err != nil) != tt.err { t.Errorf("%d failed to match %#v:\nexpected ok: %#v, err: %v\ngot ok: %#v, err: %v", i, r, tt.ok, tt.err, ok, err) } @@ -500,7 +534,7 @@ func TestMatchRule(t *testing.T) { var andRuleTests = []struct { desc string // description of the test case rule AndRule - headers, query, payload *map[string]interface{} + headers, query, payload map[string]interface{} body []byte ok bool err bool @@ -511,7 +545,7 @@ var andRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "z", "B": "y"}, nil, nil, + map[string]interface{}{"A": "z", "B": "y"}, nil, nil, []byte{}, true, false, }, @@ -521,7 +555,7 @@ var andRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "z", "B": "Y"}, nil, nil, + map[string]interface{}{"A": "z", "B": "Y"}, nil, nil, []byte{}, false, false, }, @@ -548,7 +582,7 @@ var andRuleTests = []struct { }, }, }, - &map[string]interface{}{"A": "z", "B": "y", "C": "x", "D": "w", "E": "X", "F": "X"}, nil, nil, + map[string]interface{}{"A": "z", "B": "y", "C": "x", "D": "w", "E": "X", "F": "X"}, nil, nil, []byte{}, true, false, }, @@ -557,14 +591,20 @@ var andRuleTests = []struct { { "invalid rule", AndRule{{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a", "", false}, ""}}}, - &map[string]interface{}{"Y": "z"}, nil, nil, nil, + map[string]interface{}{"Y": "z"}, nil, nil, nil, false, true, }, } func TestAndRule(t *testing.T) { for _, tt := range andRuleTests { - ok, err := tt.rule.Evaluate(tt.headers, tt.query, tt.payload, &tt.body, "") + r := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + Body: tt.body, + } + ok, err := tt.rule.Evaluate(r) if ok != tt.ok || (err != nil) != tt.err { t.Errorf("failed to match %#v:\nexpected ok: %#v, err: %v\ngot ok: %#v, err: %v", tt.desc, tt.ok, tt.err, ok, err) } @@ -574,7 +614,7 @@ func TestAndRule(t *testing.T) { var orRuleTests = []struct { desc string // description of the test case rule OrRule - headers, query, payload *map[string]interface{} + headers, query, payload map[string]interface{} body []byte ok bool err bool @@ -585,7 +625,7 @@ var orRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "z", "B": "X"}, nil, nil, + map[string]interface{}{"A": "z", "B": "X"}, nil, nil, []byte{}, true, false, }, @@ -595,7 +635,7 @@ var orRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "X", "B": "y"}, nil, nil, + map[string]interface{}{"A": "X", "B": "y"}, nil, nil, []byte{}, true, false, }, @@ -605,7 +645,7 @@ var orRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "Z", "B": "Y"}, nil, nil, + map[string]interface{}{"A": "Z", "B": "Y"}, nil, nil, []byte{}, false, false, }, @@ -615,7 +655,7 @@ var orRuleTests = []struct { OrRule{ {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, }, - &map[string]interface{}{"Y": "Z"}, nil, nil, + map[string]interface{}{"Y": "Z"}, nil, nil, []byte{}, false, true, }, @@ -623,7 +663,13 @@ var orRuleTests = []struct { func TestOrRule(t *testing.T) { for _, tt := range orRuleTests { - ok, err := tt.rule.Evaluate(tt.headers, tt.query, tt.payload, &tt.body, "") + r := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + Body: tt.body, + } + ok, err := tt.rule.Evaluate(r) if ok != tt.ok || (err != nil) != tt.err { t.Errorf("%#v:\nexpected ok: %#v, err: %v\ngot ok: %#v err: %v", tt.desc, tt.ok, tt.err, ok, err) } @@ -633,18 +679,24 @@ func TestOrRule(t *testing.T) { var notRuleTests = []struct { desc string // description of the test case rule NotRule - headers, query, payload *map[string]interface{} + headers, query, payload map[string]interface{} body []byte ok bool err bool }{ - {"(a=z): !a=X", NotRule{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a", "", false}, ""}}, &map[string]interface{}{"A": "z"}, nil, nil, []byte{}, true, false}, - {"(a=z): !a=z", NotRule{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, &map[string]interface{}{"A": "z"}, nil, nil, []byte{}, false, false}, + {"(a=z): !a=X", NotRule{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a", "", false}, ""}}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, true, false}, + {"(a=z): !a=z", NotRule{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, false, false}, } func TestNotRule(t *testing.T) { for _, tt := range notRuleTests { - ok, err := tt.rule.Evaluate(tt.headers, tt.query, tt.payload, &tt.body, "") + r := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + Body: tt.body, + } + ok, err := tt.rule.Evaluate(r) if ok != tt.ok || (err != nil) != tt.err { t.Errorf("failed to match %#v:\nexpected ok: %#v, err: %v\ngot ok: %#v, err: %v", tt.rule, tt.ok, tt.err, ok, err) } diff --git a/webhook.go b/webhook.go index 26872ed..f220d4c 100644 --- a/webhook.go +++ b/webhook.go @@ -302,10 +302,14 @@ func main() { } func hookHandler(w http.ResponseWriter, r *http.Request) { - rid := middleware.GetReqID(r.Context()) + req := &hook.Request{ + ID: middleware.GetReqID(r.Context()), + RawRequest: r, + } - log.Printf("[%s] incoming HTTP %s request from %s\n", rid, r.Method, r.RemoteAddr) + log.Printf("[%s] incoming HTTP %s request from %s\n", req.ID, r.Method, r.RemoteAddr) + // TODO: rename this to avoid confusion with Request.ID id := mux.Vars(r)["id"] matchedHook := matchLoadedHook(id) @@ -341,57 +345,54 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { if !allowedMethod { w.WriteHeader(http.StatusMethodNotAllowed) - log.Printf("[%s] HTTP %s method not allowed for hook %q", rid, r.Method, id) + log.Printf("[%s] HTTP %s method not allowed for hook %q", req.ID, r.Method, id) return } - log.Printf("[%s] %s got matched\n", rid, id) + log.Printf("[%s] %s got matched\n", req.ID, id) for _, responseHeader := range responseHeaders { w.Header().Set(responseHeader.Name, responseHeader.Value) } - var ( - body []byte - err error - ) + var err error // set contentType to IncomingPayloadContentType or header value - contentType := r.Header.Get("Content-Type") + req.ContentType = r.Header.Get("Content-Type") if len(matchedHook.IncomingPayloadContentType) != 0 { - contentType = matchedHook.IncomingPayloadContentType + req.ContentType = matchedHook.IncomingPayloadContentType } - isMultipart := strings.HasPrefix(contentType, "multipart/form-data;") + isMultipart := strings.HasPrefix(req.ContentType, "multipart/form-data;") if !isMultipart { - body, err = ioutil.ReadAll(r.Body) + req.Body, err = ioutil.ReadAll(r.Body) if err != nil { - log.Printf("[%s] error reading the request body: %+v\n", rid, err) + log.Printf("[%s] error reading the request body: %+v\n", req.ID, err) } } // parse headers - headers := valuesToMap(r.Header) + req.Headers = valuesToMap(r.Header) // parse query variables - query := valuesToMap(r.URL.Query()) + req.Query = valuesToMap(r.URL.Query()) // parse body - var payload map[string]interface{} + // var payload map[string]interface{} switch { - case strings.Contains(contentType, "json"): - decoder := json.NewDecoder(bytes.NewReader(body)) + case strings.Contains(req.ContentType, "json"): + decoder := json.NewDecoder(bytes.NewReader(req.Body)) decoder.UseNumber() var firstChar byte - for i := 0; i < len(body); i++ { - if unicode.IsSpace(rune(body[i])) { + for i := 0; i < len(req.Body); i++ { + if unicode.IsSpace(rune(req.Body[i])) { continue } - firstChar = body[i] + firstChar = req.Body[i] break } @@ -399,36 +400,36 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { var arrayPayload interface{} err := decoder.Decode(&arrayPayload) if err != nil { - log.Printf("[%s] error parsing JSON array payload %+v\n", rid, err) + log.Printf("[%s] error parsing JSON array payload %+v\n", req.ID, err) } - payload = make(map[string]interface{}, 1) - payload["root"] = arrayPayload + req.Payload = make(map[string]interface{}, 1) + req.Payload["root"] = arrayPayload } else { - err := decoder.Decode(&payload) + err := decoder.Decode(&req.Payload) if err != nil { - log.Printf("[%s] error parsing JSON payload %+v\n", rid, err) + log.Printf("[%s] error parsing JSON payload %+v\n", req.ID, err) } } - case strings.Contains(contentType, "x-www-form-urlencoded"): - fd, err := url.ParseQuery(string(body)) + case strings.Contains(req.ContentType, "x-www-form-urlencoded"): + fd, err := url.ParseQuery(string(req.Body)) if err != nil { - log.Printf("[%s] error parsing form payload %+v\n", rid, err) + log.Printf("[%s] error parsing form payload %+v\n", req.ID, err) } else { - payload = valuesToMap(fd) + req.Payload = valuesToMap(fd) } - case strings.Contains(contentType, "xml"): - payload, err = mxj.NewMapXmlReader(bytes.NewReader(body)) + case strings.Contains(req.ContentType, "xml"): + req.Payload, err = mxj.NewMapXmlReader(bytes.NewReader(req.Body)) if err != nil { - log.Printf("[%s] error parsing XML payload: %+v\n", rid, err) + log.Printf("[%s] error parsing XML payload: %+v\n", req.ID, err) } case isMultipart: err = r.ParseMultipartForm(*maxMultipartMem) if err != nil { - msg := fmt.Sprintf("[%s] error parsing multipart form: %+v\n", rid, err) + msg := fmt.Sprintf("[%s] error parsing multipart form: %+v\n", req.ID, err) log.Println(msg) w.WriteHeader(http.StatusInternalServerError) fmt.Fprint(w, "Error occurred while parsing multipart form.") @@ -436,14 +437,14 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { } for k, v := range r.MultipartForm.Value { - log.Printf("[%s] found multipart form value %q", rid, k) + log.Printf("[%s] found multipart form value %q", req.ID, k) - if payload == nil { - payload = make(map[string]interface{}) + if req.Payload == nil { + req.Payload = make(map[string]interface{}) } // TODO(moorereason): support duplicate, named values - payload[k] = v[0] + req.Payload[k] = v[0] } for k, v := range r.MultipartForm.File { @@ -472,11 +473,11 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { } if parseAsJSON { - log.Printf("[%s] parsing multipart form file %q as JSON\n", rid, k) + log.Printf("[%s] parsing multipart form file %q as JSON\n", req.ID, k) f, err := v[0].Open() if err != nil { - msg := fmt.Sprintf("[%s] error parsing multipart form file: %+v\n", rid, err) + msg := fmt.Sprintf("[%s] error parsing multipart form file: %+v\n", req.ID, err) log.Println(msg) w.WriteHeader(http.StatusInternalServerError) fmt.Fprint(w, "Error occurred while parsing multipart form file.") @@ -489,24 +490,24 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { var part map[string]interface{} err = decoder.Decode(&part) if err != nil { - log.Printf("[%s] error parsing JSON payload file: %+v\n", rid, err) + log.Printf("[%s] error parsing JSON payload file: %+v\n", req.ID, err) } - if payload == nil { - payload = make(map[string]interface{}) + if req.Payload == nil { + req.Payload = make(map[string]interface{}) } - payload[k] = part + req.Payload[k] = part } } default: - log.Printf("[%s] error parsing body payload due to unsupported content type header: %s\n", rid, contentType) + log.Printf("[%s] error parsing body payload due to unsupported content type header: %s\n", req.ID, req.ContentType) } // handle hook - errors := matchedHook.ParseJSONParameters(&headers, &query, &payload) + errors := matchedHook.ParseJSONParameters(req) for _, err := range errors { - log.Printf("[%s] error parsing JSON parameters: %s\n", rid, err) + log.Printf("[%s] error parsing JSON parameters: %s\n", req.ID, err) } var ok bool @@ -514,29 +515,29 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { if matchedHook.TriggerRule == nil { ok = true } else { - ok, err = matchedHook.TriggerRule.Evaluate(&headers, &query, &payload, &body, r.RemoteAddr) + ok, err = matchedHook.TriggerRule.Evaluate(req) if err != nil { if !hook.IsParameterNodeError(err) { - msg := fmt.Sprintf("[%s] error evaluating hook: %s", rid, err) + msg := fmt.Sprintf("[%s] error evaluating hook: %s", req.ID, err) log.Println(msg) w.WriteHeader(http.StatusInternalServerError) fmt.Fprint(w, "Error occurred while evaluating hook rules.") return } - log.Printf("[%s] %v", rid, err) + log.Printf("[%s] %v", req.ID, err) } } if ok { - log.Printf("[%s] %s hook triggered successfully\n", rid, matchedHook.ID) + log.Printf("[%s] %s hook triggered successfully\n", req.ID, matchedHook.ID) for _, responseHeader := range matchedHook.ResponseHeaders { w.Header().Set(responseHeader.Name, responseHeader.Value) } if matchedHook.CaptureCommandOutput { - response, err := handleHook(matchedHook, rid, &headers, &query, &payload, &body) + response, err := handleHook(matchedHook, req) if err != nil { w.WriteHeader(http.StatusInternalServerError) @@ -549,16 +550,16 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { } else { // Check if a success return code is configured for the hook if matchedHook.SuccessHttpResponseCode != 0 { - writeHttpResponseCode(w, rid, matchedHook.ID, matchedHook.SuccessHttpResponseCode) + writeHttpResponseCode(w, req.ID, matchedHook.ID, matchedHook.SuccessHttpResponseCode) } fmt.Fprint(w, response) } } else { - go handleHook(matchedHook, rid, &headers, &query, &payload, &body) + go handleHook(matchedHook, req) // Check if a success return code is configured for the hook if matchedHook.SuccessHttpResponseCode != 0 { - writeHttpResponseCode(w, rid, matchedHook.ID, matchedHook.SuccessHttpResponseCode) + writeHttpResponseCode(w, req.ID, matchedHook.ID, matchedHook.SuccessHttpResponseCode) } fmt.Fprint(w, matchedHook.ResponseMessage) @@ -568,16 +569,16 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { // Check if a return code is configured for the hook if matchedHook.TriggerRuleMismatchHttpResponseCode != 0 { - writeHttpResponseCode(w, rid, matchedHook.ID, matchedHook.TriggerRuleMismatchHttpResponseCode) + writeHttpResponseCode(w, req.ID, matchedHook.ID, matchedHook.TriggerRuleMismatchHttpResponseCode) } // if none of the hooks got triggered - log.Printf("[%s] %s got matched, but didn't get triggered because the trigger rules were not satisfied\n", rid, matchedHook.ID) + log.Printf("[%s] %s got matched, but didn't get triggered because the trigger rules were not satisfied\n", req.ID, matchedHook.ID) fmt.Fprint(w, "Hook rules were not satisfied.") } -func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]interface{}, body *[]byte) (string, error) { +func handleHook(h *hook.Hook, r *hook.Request) (string, error) { var errors []error // check the command exists @@ -590,12 +591,12 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in cmdPath, err := exec.LookPath(lookpath) if err != nil { - log.Printf("[%s] error in %s", rid, err) + log.Printf("[%s] error in %s", r.ID, err) // check if parameters specified in execute-command by mistake if strings.IndexByte(h.ExecuteCommand, ' ') != -1 { s := strings.Fields(h.ExecuteCommand)[0] - log.Printf("[%s] use 'pass-arguments-to-command' to specify args for '%s'", rid, s) + log.Printf("[%s] use 'pass-arguments-to-command' to specify args for '%s'", r.ID, s) } return "", err @@ -604,37 +605,37 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in cmd := exec.Command(cmdPath) cmd.Dir = h.CommandWorkingDirectory - cmd.Args, errors = h.ExtractCommandArguments(headers, query, payload) + cmd.Args, errors = h.ExtractCommandArguments(r) for _, err := range errors { - log.Printf("[%s] error extracting command arguments: %s\n", rid, err) + log.Printf("[%s] error extracting command arguments: %s\n", r.ID, err) } var envs []string - envs, errors = h.ExtractCommandArgumentsForEnv(headers, query, payload) + envs, errors = h.ExtractCommandArgumentsForEnv(r) for _, err := range errors { - log.Printf("[%s] error extracting command arguments for environment: %s\n", rid, err) + log.Printf("[%s] error extracting command arguments for environment: %s\n", r.ID, err) } - files, errors := h.ExtractCommandArgumentsForFile(headers, query, payload) + files, errors := h.ExtractCommandArgumentsForFile(r) for _, err := range errors { - log.Printf("[%s] error extracting command arguments for file: %s\n", rid, err) + log.Printf("[%s] error extracting command arguments for file: %s\n", r.ID, err) } for i := range files { tmpfile, err := ioutil.TempFile(h.CommandWorkingDirectory, files[i].EnvName) if err != nil { - log.Printf("[%s] error creating temp file [%s]", rid, err) + log.Printf("[%s] error creating temp file [%s]", r.ID, err) continue } - log.Printf("[%s] writing env %s file %s", rid, files[i].EnvName, tmpfile.Name()) + log.Printf("[%s] writing env %s file %s", r.ID, files[i].EnvName, tmpfile.Name()) if _, err := tmpfile.Write(files[i].Data); err != nil { - log.Printf("[%s] error writing file %s [%s]", rid, tmpfile.Name(), err) + log.Printf("[%s] error writing file %s [%s]", r.ID, tmpfile.Name(), err) continue } if err := tmpfile.Close(); err != nil { - log.Printf("[%s] error closing file %s [%s]", rid, tmpfile.Name(), err) + log.Printf("[%s] error closing file %s [%s]", r.ID, tmpfile.Name(), err) continue } @@ -644,27 +645,27 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in cmd.Env = append(os.Environ(), envs...) - log.Printf("[%s] executing %s (%s) with arguments %q and environment %s using %s as cwd\n", rid, h.ExecuteCommand, cmd.Path, cmd.Args, envs, cmd.Dir) + log.Printf("[%s] executing %s (%s) with arguments %q and environment %s using %s as cwd\n", r.ID, h.ExecuteCommand, cmd.Path, cmd.Args, envs, cmd.Dir) out, err := cmd.CombinedOutput() - log.Printf("[%s] command output: %s\n", rid, out) + log.Printf("[%s] command output: %s\n", r.ID, out) if err != nil { - log.Printf("[%s] error occurred: %+v\n", rid, err) + log.Printf("[%s] error occurred: %+v\n", r.ID, err) } for i := range files { if files[i].File != nil { - log.Printf("[%s] removing file %s\n", rid, files[i].File.Name()) + log.Printf("[%s] removing file %s\n", r.ID, files[i].File.Name()) err := os.Remove(files[i].File.Name()) if err != nil { - log.Printf("[%s] error removing file %s [%s]", rid, files[i].File.Name(), err) + log.Printf("[%s] error removing file %s [%s]", r.ID, files[i].File.Name(), err) } } } - log.Printf("[%s] finished handling %s\n", rid, h.ID) + log.Printf("[%s] finished handling %s\n", r.ID, h.ID) return string(out), err } diff --git a/webhook_test.go b/webhook_test.go index 353d22d..c8a46cc 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -53,7 +53,11 @@ func TestStaticParams(t *testing.T) { b := &bytes.Buffer{} log.SetOutput(b) - _, err = handleHook(spHook, "test", &spHeaders, &map[string]interface{}{}, &map[string]interface{}{}, &[]byte{}) + r := &hook.Request{ + ID: "test", + Headers: spHeaders, + } + _, err = handleHook(spHook, r) if err != nil { t.Fatalf("Unexpected error: %v\n", err) } From 6bbf14f7d9fc0f354ef0a313e76e69e93f83a6d2 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Thu, 24 Sep 2020 21:37:49 -0500 Subject: [PATCH 13/26] Transition payload hash option names to hmac The payload-hash-* options are imprecisely named. Clarify their function as HMAC validations by renaming them. The existing options will continue to work but are deprecated. Log a warning if the old options are used. All tests, examples, and documentation are updated. Fixes #289 --- docs/Hook-Examples.md | 4 ++-- docs/Hook-Rules.md | 33 +++++++++++++++++---------------- docs/Templates.md | 4 ++-- hooks.json.example | 2 +- hooks.json.tmpl.example | 2 +- hooks.yaml.example | 2 +- hooks.yaml.tmpl.example | 2 +- internal/hook/hook.go | 12 ++++++++++++ internal/hook/hook_test.go | 6 ++++++ test/hooks.json.tmpl | 4 ++-- test/hooks.yaml.tmpl | 4 ++-- 11 files changed, 47 insertions(+), 28 deletions(-) diff --git a/docs/Hook-Examples.md b/docs/Hook-Examples.md index e02eee7..7ca2600 100644 --- a/docs/Hook-Examples.md +++ b/docs/Hook-Examples.md @@ -46,7 +46,7 @@ This page is still work in progress. Feel free to contribute! { "match": { - "type": "payload-hash-sha1", + "type": "payload-hmac-sha1", "secret": "mysecret", "parameter": { @@ -166,7 +166,7 @@ Values in the request body can be accessed in the command or to the match rule b { "match": { - "type": "payload-hash-sha256", + "type": "payload-hmac-sha256", "secret": "mysecret", "parameter": { diff --git a/docs/Hook-Rules.md b/docs/Hook-Rules.md index 0ff30b3..2e5550b 100644 --- a/docs/Hook-Rules.md +++ b/docs/Hook-Rules.md @@ -9,9 +9,9 @@ * [Match](#match) * [Match value](#match-value) * [Match regex](#match-regex) - * [Match payload-hash-sha1](#match-payload-hash-sha1) - * [Match payload-hash-sha256](#match-payload-hash-sha256) - * [Match payload-hash-sha512](#match-payload-hash-sha512) + * [Match payload-hmac-sha1](#match-payload-hmac-sha1) + * [Match payload-hmac-sha256](#match-payload-hmac-sha256) + * [Match payload-hmac-sha512](#match-payload-hmac-sha512) * [Match Whitelisted IP range](#match-whitelisted-ip-range) * [Match scalr-signature](#match-scalr-signature) @@ -110,7 +110,7 @@ "source": "header", "name": "X-Hub-Signature" }, - "type": "payload-hash-sha1", + "type": "payload-hmac-sha1", "secret": "mysecret" } }, @@ -150,9 +150,7 @@ *Please note:* Due to technical reasons, _number_ and _boolean_ values in the _match rule_ must be wrapped around with a pair of quotes. -There are three different match rules: - -### 1. Match value +### Match value ```json { "match": @@ -168,7 +166,7 @@ There are three different match rules: } ``` -### 2. Match regex +### Match regex For the regex syntax, check out ```json { @@ -185,12 +183,13 @@ For the regex syntax, check out } ``` -### 3. Match payload-hash-sha1 +### Match payload-hmac-sha1 +Validate the HMAC of the payload using the SHA1 hash and the given *secret*. ```json { "match": { - "type": "payload-hash-sha1", + "type": "payload-hmac-sha1", "secret": "yoursecret", "parameter": { @@ -208,12 +207,13 @@ will be tried unless a match is found. For example: X-Hub-Signature: sha1=the-first-signature,sha1=the-second-signature ``` -### 4. Match payload-hash-sha256 +### Match payload-hmac-sha256 +Validate the HMAC of the payload using the SHA256 hash and the given *secret*. ```json { "match": { - "type": "payload-hash-sha256", + "type": "payload-hmac-sha256", "secret": "yoursecret", "parameter": { @@ -231,12 +231,13 @@ will be tried unless a match is found. For example: X-Hub-Signature: sha256=the-first-signature,sha256=the-second-signature ``` -### 5. Match payload-hash-sha512 +### Match payload-hmac-sha512 +Validate the HMAC of the payload using the SHA512 hash and the given *secret*. ```json { "match": { - "type": "payload-hash-sha512", + "type": "payload-hmac-sha512", "secret": "yoursecret", "parameter": { @@ -254,7 +255,7 @@ will be tried unless a match is found. For example: X-Hub-Signature: sha512=the-first-signature,sha512=the-second-signature ``` -### 6. Match Whitelisted IP range +### Match Whitelisted IP range The IP can be IPv4- or IPv6-formatted, using [CIDR notation](https://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing#CIDR_blocks). To match a single IP address only, use `/32`. @@ -268,7 +269,7 @@ The IP can be IPv4- or IPv6-formatted, using [CIDR notation](https://en.wikipedi } ``` -### 7. Match scalr-signature +### Match scalr-signature The trigger rule checks the scalr signature and also checks that the request was signed less than 5 minutes before it was received. A unqiue signing key is generated for each webhook endpoint URL you register in Scalr. diff --git a/docs/Templates.md b/docs/Templates.md index 2adeab3..b7cb972 100644 --- a/docs/Templates.md +++ b/docs/Templates.md @@ -6,7 +6,7 @@ In additional to the [built-in Go template functions and features][tt], `webhook ## Example Usage -In the example `hooks.json` file below, the `payload-hash-sha1` matching rule looks up the secret hash from the environment using the `getenv` template function. +In the example `hooks.json` file below, the `payload-hmac-sha1` matching rule looks up the HMAC secret from the environment using the `getenv` template function. Additionally, the result is piped through the built-in Go template function `js` to ensure that the result is a well-formed Javascript/JSON string. ``` @@ -44,7 +44,7 @@ Additionally, the result is piped through the built-in Go template function `js` { "match": { - "type": "payload-hash-sha1", + "type": "payload-hmac-sha1", "secret": "{{ getenv "XXXTEST_SECRET" | js }}", "parameter": { diff --git a/hooks.json.example b/hooks.json.example index 00f6ff3..90cd275 100644 --- a/hooks.json.example +++ b/hooks.json.example @@ -33,7 +33,7 @@ { "match": { - "type": "payload-hash-sha1", + "type": "payload-hmac-sha1", "secret": "mysecret", "parameter": { diff --git a/hooks.json.tmpl.example b/hooks.json.tmpl.example index 70e05c8..5c00643 100644 --- a/hooks.json.tmpl.example +++ b/hooks.json.tmpl.example @@ -33,7 +33,7 @@ { "match": { - "type": "payload-hash-sha1", + "type": "payload-hmac-sha1", "secret": "{{ getenv "XXXTEST_SECRET" | js }}", "parameter": { diff --git a/hooks.yaml.example b/hooks.yaml.example index 74713e0..ba9d0b7 100644 --- a/hooks.yaml.example +++ b/hooks.yaml.example @@ -15,7 +15,7 @@ trigger-rule: and: - match: - type: payload-hash-sha1 + type: payload-hmac-sha1 secret: mysecret parameter: source: header diff --git a/hooks.yaml.tmpl.example b/hooks.yaml.tmpl.example index 2bcfbd6..672c8d0 100644 --- a/hooks.yaml.tmpl.example +++ b/hooks.yaml.tmpl.example @@ -15,7 +15,7 @@ trigger-rule: and: - match: - type: payload-hash-sha1 + type: payload-hmac-sha1 secret: "{{ getenv "XXXTEST_SECRET" | js }}" parameter: source: header diff --git a/internal/hook/hook.go b/internal/hook/hook.go index e6a9272..157c457 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -883,6 +883,9 @@ type MatchRule struct { const ( MatchValue string = "value" MatchRegex string = "regex" + MatchHMACSHA1 string = "payload-hmac-sha1" + MatchHMACSHA256 string = "payload-hmac-sha256" + MatchHMACSHA512 string = "payload-hmac-sha512" MatchHashSHA1 string = "payload-hash-sha1" MatchHashSHA256 string = "payload-hash-sha256" MatchHashSHA512 string = "payload-hash-sha512" @@ -907,12 +910,21 @@ func (r MatchRule) Evaluate(req *Request) (bool, error) { case MatchRegex: return regexp.MatchString(r.Regex, arg) case MatchHashSHA1: + log.Print(`warn: use of deprecated option payload-hash-sha1; use payload-hmac-sha1 instead`) + fallthrough + case MatchHMACSHA1: _, err := CheckPayloadSignature(req.Body, r.Secret, arg) return err == nil, err case MatchHashSHA256: + log.Print(`warn: use of deprecated option payload-hash-sha256: use payload-hmac-sha256 instead`) + fallthrough + case MatchHMACSHA256: _, err := CheckPayloadSignature256(req.Body, r.Secret, arg) return err == nil, err case MatchHashSHA512: + log.Print(`warn: use of deprecated option payload-hash-sha512: use payload-hmac-sha512 instead`) + fallthrough + case MatchHMACSHA512: _, err := CheckPayloadSignature512(req.Body, r.Secret, arg) return err == nil, err } diff --git a/internal/hook/hook_test.go b/internal/hook/hook_test.go index c99edfb..32bc848 100644 --- a/internal/hook/hook_test.go +++ b/internal/hook/hook_test.go @@ -487,7 +487,9 @@ var matchRuleTests = []struct { }{ {"value", "", "", "z", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", true, false}, {"regex", "^z", "", "z", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", true, false}, + {"payload-hmac-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "b17e04cbb22afa8ffbff8796fc1894ed27badd9e"}, nil, nil, []byte(`{"a": "z"}`), "", true, false}, {"payload-hash-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "b17e04cbb22afa8ffbff8796fc1894ed27badd9e"}, nil, nil, []byte(`{"a": "z"}`), "", true, false}, + {"payload-hmac-sha256", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89"}, nil, nil, []byte(`{"a": "z"}`), "", true, false}, {"payload-hash-sha256", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89"}, nil, nil, []byte(`{"a": "z"}`), "", true, false}, // failures {"value", "", "", "X", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, false}, @@ -495,8 +497,12 @@ var matchRuleTests = []struct { {"value", "", "2", "X", "", Argument{"header", "a", "", false}, map[string]interface{}{"Y": "z"}, nil, nil, []byte{}, "", false, true}, // reference invalid header // errors {"regex", "*", "", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, true}, // invalid regex + {"payload-hmac-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true}, // invalid hmac {"payload-hash-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true}, // invalid hmac + {"payload-hmac-sha256", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true}, // invalid hmac {"payload-hash-sha256", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true}, // invalid hmac + {"payload-hmac-sha512", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true}, // invalid hmac + {"payload-hash-sha512", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true}, // invalid hmac // IP whitelisting, valid cases {"ip-whitelist", "", "", "", "192.168.0.1/24", Argument{}, nil, nil, nil, []byte{}, "192.168.0.2:9000", true, false}, // valid IPv4, with range {"ip-whitelist", "", "", "", "192.168.0.1/24", Argument{}, nil, nil, nil, []byte{}, "192.168.0.2:9000", true, false}, // valid IPv4, with range diff --git a/test/hooks.json.tmpl b/test/hooks.json.tmpl index 7fe7c8c..3075762 100644 --- a/test/hooks.json.tmpl +++ b/test/hooks.json.tmpl @@ -31,7 +31,7 @@ { "match": { - "type": "payload-hash-sha1", + "type": "payload-hmac-sha1", "secret": "mysecret", "parameter": { @@ -288,7 +288,7 @@ { "match": { - "type": "payload-hash-sha1", + "type": "payload-hmac-sha1", "secret": "mysecret", "parameter": { diff --git a/test/hooks.yaml.tmpl b/test/hooks.yaml.tmpl index ace4277..16aa8c1 100644 --- a/test/hooks.yaml.tmpl +++ b/test/hooks.yaml.tmpl @@ -8,7 +8,7 @@ source: header name: X-Hub-Signature secret: mysecret - type: payload-hash-sha1 + type: payload-hmac-sha1 - match: parameter: source: payload @@ -174,4 +174,4 @@ source: header name: X-Hub-Signature secret: mysecret - type: payload-hash-sha1 + type: payload-hmac-sha1 From d2795059303521cac3fdd0a1c4feeec406512514 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Mon, 28 Sep 2020 14:23:56 -0500 Subject: [PATCH 14/26] Document YAML support Fixes #400 Updates #288 --- README.md | 13 +++++++++++-- docs/Hook-Definition.md | 3 ++- docs/Hook-Examples.md | 8 ++++++-- docs/Templates.md | 6 +++--- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 1706b79..001835c 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,9 @@ If you are using Debian linux ("stretch" or later), you can install webhook usin Prebuilt binaries for different architectures are available at [GitHub Releases](https://github.com/adnanh/webhook/releases). ## Configuration -Next step is to define some hooks you want [webhook][w] to serve. Begin by creating an empty file named `hooks.json`. This file will contain an array of hooks the [webhook][w] will serve. Check [Hook definition page](docs/Hook-Definition.md) to see the detailed description of what properties a hook can contain, and how to use them. +Next step is to define some hooks you want [webhook][w] to serve. +[webhook][w] supports JSON or YAML configuration files, but we'll focus primarily on JSON in the following example. +Begin by creating an empty file named `hooks.json`. This file will contain an array of hooks the [webhook][w] will serve. Check [Hook definition page](docs/Hook-Definition.md) to see the detailed description of what properties a hook can contain, and how to use them. Let's define a simple hook named `redeploy-webhook` that will run a redeploy script located in `/var/scripts/redeploy.sh`. Make sure that your bash script has `#!/bin/sh` shebang on top. @@ -61,6 +63,13 @@ Our `hooks.json` file will now look like this: ] ``` +**NOTE:** If you prefer YAML, the equivalent `hooks.yaml` file would be: +```yaml +- id: redeploy-webhook + execute-command: "/var/scripts/redeploy.sh" + command-working-directory: "/var/webhook" +``` + You can now run [webhook][w] using ```bash $ /path/to/webhook -hooks hooks.json -verbose @@ -90,7 +99,7 @@ All files are ignored unless they match one of the following criteria: In either case, the given file part will be parsed as JSON and added to the `payload` map. ## Templates -[webhook][w] can parse the `hooks.json` input file as a Go template when given the `-template` [CLI parameter](docs/Webhook-Parameters.md). See the [Templates page](docs/Templates.md) for more details on template usage. +[webhook][w] can parse the hooks configuration file as a Go template when given the `-template` [CLI parameter](docs/Webhook-Parameters.md). See the [Templates page](docs/Templates.md) for more details on template usage. ## Using HTTPS [webhook][w] by default serves hooks using http. If you want [webhook][w] to serve secure content using https, you can use the `-secure` flag while starting [webhook][w]. Files containing a certificate and matching private key for the server must be provided using the `-cert /path/to/cert.pem` and `-key /path/to/key.pem` flags. If the certificate is signed by a certificate authority, the cert file should be the concatenation of the server's certificate followed by the CA's certificate. diff --git a/docs/Hook-Definition.md b/docs/Hook-Definition.md index 7c54a20..925bea1 100644 --- a/docs/Hook-Definition.md +++ b/docs/Hook-Definition.md @@ -1,5 +1,6 @@ # Hook definition -Hooks are defined as JSON objects. Please note that in order to be considered valid, a hook object must contain the `id` and `execute-command` properties. All other properties are considered optional. + +Hooks are defined as objects in the JSON or YAML hooks configuration file. Please note that in order to be considered valid, a hook object must contain the `id` and `execute-command` properties. All other properties are considered optional. ## Properties (keys) diff --git a/docs/Hook-Examples.md b/docs/Hook-Examples.md index 7ca2600..ba581eb 100644 --- a/docs/Hook-Examples.md +++ b/docs/Hook-Examples.md @@ -1,5 +1,9 @@ -# Hook examples -This page is still work in progress. Feel free to contribute! +# Hook Examples + +Hooks are defined in a hooks configuration file in either JSON or YAML format, +although the examples on this page all use the JSON format. + +🌱 This page is still a work in progress. Feel free to contribute! ### Table of Contents diff --git a/docs/Templates.md b/docs/Templates.md index b7cb972..12798f4 100644 --- a/docs/Templates.md +++ b/docs/Templates.md @@ -1,12 +1,12 @@ # Templates in Webhook -[`webhook`][w] can parse the `hooks.json` input file as a Go template when given the `-template` [CLI parameter](Webhook-Parameters.md). +[`webhook`][w] can parse a hooks configuration file as a Go template when given the `-template` [CLI parameter](Webhook-Parameters.md). -In additional to the [built-in Go template functions and features][tt], `webhook` provides a `getenv` template function for inserting environment variables into a `hooks.json` file. +In additional to the [built-in Go template functions and features][tt], `webhook` provides a `getenv` template function for inserting environment variables into a templated configuration file. ## Example Usage -In the example `hooks.json` file below, the `payload-hmac-sha1` matching rule looks up the HMAC secret from the environment using the `getenv` template function. +In the example JSON template file below (YAML is also supported), the `payload-hmac-sha1` matching rule looks up the HMAC secret from the environment using the `getenv` template function. Additionally, the result is piped through the built-in Go template function `js` to ensure that the result is a well-formed Javascript/JSON string. ``` From 0814b10a16f8e53280d9e2138ae3a7b95381514a Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Mon, 28 Sep 2020 17:20:22 -0500 Subject: [PATCH 15/26] Add Github Action to build & run tests --- .github/workflows/build.yml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 .github/workflows/build.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 0000000..af26c40 --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,29 @@ +name: build +on: [push, pull_request] +jobs: + build: + strategy: + matrix: + go-version: [1.14.x, 1.15.x] + os: [ubuntu-latest, macos-latest, windows-latest] + + runs-on: ${{ matrix.os }} + + steps: + - name: Setup Go + uses: actions/setup-go@v2 + with: + go-version: ${{ matrix.go-version }} + id: go + + - name: Checkout code + uses: actions/checkout@v2 + + - name: Download dependencies + run: go mod download + + - name: Build + run: go build -v + + - name: Test + run: go test -v ./... From a904537367e0c7d36976e723f1c1a596c3498670 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Mon, 28 Sep 2020 20:41:24 -0500 Subject: [PATCH 16/26] Add build badge to README.md --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 1706b79..35dfce9 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# What is webhook? +# What is webhook? ![build-status][badge] Webhook @@ -202,3 +202,4 @@ THE SOFTWARE. [w]: https://github.com/adnanh/webhook [wc]: https://github.com/adnanh/webhook-contrib +[badge]: https://github.com/adnanh/webhook/workflows/build/badge.svg From f007fa52809edc645cddb27ce688ce73f58aad92 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Mon, 28 Sep 2020 20:51:09 -0500 Subject: [PATCH 17/26] Simplify build workflow --- .github/workflows/build.yml | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index af26c40..75f347d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -10,18 +10,12 @@ jobs: runs-on: ${{ matrix.os }} steps: - - name: Setup Go - uses: actions/setup-go@v2 + - uses: actions/checkout@v2 + - uses: actions/setup-go@v2 with: go-version: ${{ matrix.go-version }} id: go - - name: Checkout code - uses: actions/checkout@v2 - - - name: Download dependencies - run: go mod download - - name: Build run: go build -v From dc184d2737ff15e76a953cb76b34576e055e3ee9 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Sat, 24 Oct 2020 11:40:27 -0500 Subject: [PATCH 18/26] Fix OrRule logic on parameter lookup failures Fixes #471 --- internal/hook/hook.go | 4 ++- internal/hook/hook_test.go | 4 +-- test/hooks.json.tmpl | 70 ++++++++++++++++++++++++++++++++++++++ test/hooks.yaml.tmpl | 36 ++++++++++++++++++++ webhook_test.go | 26 ++++++++++++++ 5 files changed, 137 insertions(+), 3 deletions(-) diff --git a/internal/hook/hook.go b/internal/hook/hook.go index 157c457..c446aee 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -848,7 +848,9 @@ func (r OrRule) Evaluate(req *Request) (bool, error) { for _, v := range r { rv, err := v.Evaluate(req) if err != nil { - return false, err + if !IsParameterNodeError(err) { + return false, err + } } res = res || rv diff --git a/internal/hook/hook_test.go b/internal/hook/hook_test.go index 32bc848..9a548d7 100644 --- a/internal/hook/hook_test.go +++ b/internal/hook/hook_test.go @@ -657,13 +657,13 @@ var orRuleTests = []struct { }, // failures { - "invalid rule", + "missing parameter node", OrRule{ {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, }, map[string]interface{}{"Y": "Z"}, nil, nil, []byte{}, - false, true, + false, false, }, } diff --git a/test/hooks.json.tmpl b/test/hooks.json.tmpl index 3075762..b94ed1a 100644 --- a/test/hooks.json.tmpl +++ b/test/hooks.json.tmpl @@ -276,6 +276,76 @@ } ], }, + { + "id": "issue-471", + "execute-command": "{{ .Hookecho }}", + "response-message": "success", + "trigger-rule": + { + "or": + [ + { + "match": + { + "parameter": + { + "source": "payload", + "name": "foo" + }, + "type": "value", + "value": "bar" + } + }, + { + "match": + { + "parameter": + { + "source": "payload", + "name": "exists" + }, + "type": "value", + "value": 1 + } + } + ] + } + }, + { + "id": "issue-471-and", + "execute-command": "{{ .Hookecho }}", + "response-message": "success", + "trigger-rule": + { + "and": + [ + { + "match": + { + "parameter": + { + "source": "payload", + "name": "foo" + }, + "type": "value", + "value": "bar" + } + }, + { + "match": + { + "parameter": + { + "source": "payload", + "name": "exists" + }, + "type": "value", + "value": 1 + } + } + ] + } + }, { "id": "empty-payload-signature", "execute-command": "{{ .Hookecho }}", diff --git a/test/hooks.yaml.tmpl b/test/hooks.yaml.tmpl index 16aa8c1..325ef27 100644 --- a/test/hooks.yaml.tmpl +++ b/test/hooks.yaml.tmpl @@ -163,6 +163,42 @@ execute-command: '{{ .Hookecho }} foo' include-command-output-in-response: true +- id: issue-471 + execute-command: '{{ .Hookecho }}' + response-message: success + trigger-rule: + or: + - match: + parameter: + source: payload + name: foo + type: value + value: bar + - match: + parameter: + source: payload + name: exists + type: value + value: 1 + +- id: issue-471-and + execute-command: '{{ .Hookecho }}' + response-message: success + trigger-rule: + and: + - match: + parameter: + source: payload + name: foo + type: value + value: bar + - match: + parameter: + source: payload + name: exists + type: value + value: 1 + - id: empty-payload-signature include-command-output-in-response: true execute-command: '{{ .Hookecho }}' diff --git a/webhook_test.go b/webhook_test.go index c8a46cc..d12826d 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -606,6 +606,32 @@ binary data ``, }, + { + "issue-471", + "issue-471", + nil, + "POST", + nil, + "application/json", + `{"exists": 1}`, + http.StatusOK, + `success`, + ``, + }, + + { + "issue-471-and", + "issue-471-and", + nil, + "POST", + nil, + "application/json", + `{"exists": 1}`, + http.StatusOK, + `Hook rules were not satisfied.`, + `parameter node not found`, + }, + { "missing-cmd-arg", // missing head_commit.author.email "github", From 22c8a1670b92a879580eebe48397edd343dab12c Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Tue, 17 Nov 2020 15:00:58 -0600 Subject: [PATCH 19/26] Move some request parsing into hook package Trying to simplify hookHandler. No functional changes introduced. --- internal/hook/hook.go | 25 --------- internal/hook/request.go | 116 +++++++++++++++++++++++++++++++++++++++ webhook.go | 53 +++--------------- 3 files changed, 125 insertions(+), 69 deletions(-) create mode 100644 internal/hook/request.go diff --git a/internal/hook/hook.go b/internal/hook/hook.go index c446aee..34a429d 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -17,7 +17,6 @@ import ( "log" "math" "net" - "net/http" "net/textproto" "os" "reflect" @@ -48,30 +47,6 @@ const ( EnvNamespace string = "HOOK_" ) -// Request represents a webhook request. -type Request struct { - // The request ID set by the RequestID middleware. - ID string - - // The Content-Type of the request. - ContentType string - - // The raw request body. - Body []byte - - // Headers is a map of the parsed headers. - Headers map[string]interface{} - - // Query is a map of the parsed URL query values. - Query map[string]interface{} - - // Payload is a map of the parsed payload. - Payload map[string]interface{} - - // The underlying HTTP request. - RawRequest *http.Request -} - // ParameterNodeError describes an error walking a parameter node. type ParameterNodeError struct { key string diff --git a/internal/hook/request.go b/internal/hook/request.go new file mode 100644 index 0000000..ef7c949 --- /dev/null +++ b/internal/hook/request.go @@ -0,0 +1,116 @@ +package hook + +import ( + "bytes" + "encoding/json" + "fmt" + "net/http" + "net/url" + "unicode" + + "github.com/clbanning/mxj" +) + +// Request represents a webhook request. +type Request struct { + // The request ID set by the RequestID middleware. + ID string + + // The Content-Type of the request. + ContentType string + + // The raw request body. + Body []byte + + // Headers is a map of the parsed headers. + Headers map[string]interface{} + + // Query is a map of the parsed URL query values. + Query map[string]interface{} + + // Payload is a map of the parsed payload. + Payload map[string]interface{} + + // The underlying HTTP request. + RawRequest *http.Request +} + +func (r *Request) ParseJSONPayload() error { + decoder := json.NewDecoder(bytes.NewReader(r.Body)) + decoder.UseNumber() + + var firstChar byte + for i := 0; i < len(r.Body); i++ { + if unicode.IsSpace(rune(r.Body[i])) { + continue + } + firstChar = r.Body[i] + break + } + + if firstChar == byte('[') { + var arrayPayload interface{} + err := decoder.Decode(&arrayPayload) + if err != nil { + return fmt.Errorf("error parsing JSON array payload %+v", err) + } + + r.Payload = make(map[string]interface{}, 1) + r.Payload["root"] = arrayPayload + } else { + err := decoder.Decode(&r.Payload) + if err != nil { + return fmt.Errorf("error parsing JSON payload %+v", err) + } + } + + return nil +} + +func (r *Request) ParseHeaders(headers map[string][]string) { + r.Headers = make(map[string]interface{}, len(headers)) + + for k, v := range headers { + if len(v) > 0 { + r.Headers[k] = v[0] + } + } +} + +func (r *Request) ParseQuery(query map[string][]string) { + r.Query = make(map[string]interface{}, len(query)) + + for k, v := range query { + if len(v) > 0 { + r.Query[k] = v[0] + } + } +} + +func (r *Request) ParseFormPayload() error { + fd, err := url.ParseQuery(string(r.Body)) + if err != nil { + return fmt.Errorf("error parsing form payload %+v", err) + } + + r.Payload = make(map[string]interface{}, len(fd)) + + for k, v := range fd { + if len(v) > 0 { + r.Payload[k] = v[0] + } + } + + return nil +} + +func (r *Request) ParseXMLPayload() error { + var err error + + r.Payload, err = mxj.NewMapXmlReader(bytes.NewReader(r.Body)) + if err != nil { + return fmt.Errorf("error parsing XML payload: %+v", err) + } + + return nil +} diff --git a/webhook.go b/webhook.go index f220d4c..ea6a9f2 100644 --- a/webhook.go +++ b/webhook.go @@ -1,7 +1,6 @@ package main import ( - "bytes" "crypto/tls" "encoding/json" "flag" @@ -10,19 +9,16 @@ import ( "log" "net" "net/http" - "net/url" "os" "os/exec" "path/filepath" "strings" "time" - "unicode" "github.com/adnanh/webhook/internal/hook" "github.com/adnanh/webhook/internal/middleware" "github.com/adnanh/webhook/internal/pidfile" - "github.com/clbanning/mxj" chimiddleware "github.com/go-chi/chi/middleware" "github.com/gorilla/mux" fsnotify "gopkg.in/fsnotify.v1" @@ -373,57 +369,26 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { } } - // parse headers - req.Headers = valuesToMap(r.Header) - - // parse query variables - req.Query = valuesToMap(r.URL.Query()) - - // parse body - // var payload map[string]interface{} + req.ParseHeaders(r.Header) + req.ParseQuery(r.URL.Query()) switch { case strings.Contains(req.ContentType, "json"): - decoder := json.NewDecoder(bytes.NewReader(req.Body)) - decoder.UseNumber() - - var firstChar byte - for i := 0; i < len(req.Body); i++ { - if unicode.IsSpace(rune(req.Body[i])) { - continue - } - firstChar = req.Body[i] - break - } - - if firstChar == byte('[') { - var arrayPayload interface{} - err := decoder.Decode(&arrayPayload) - if err != nil { - log.Printf("[%s] error parsing JSON array payload %+v\n", req.ID, err) - } - - req.Payload = make(map[string]interface{}, 1) - req.Payload["root"] = arrayPayload - } else { - err := decoder.Decode(&req.Payload) - if err != nil { - log.Printf("[%s] error parsing JSON payload %+v\n", req.ID, err) - } + err = req.ParseJSONPayload() + if err != nil { + log.Printf("[%s] %s", req.ID, err) } case strings.Contains(req.ContentType, "x-www-form-urlencoded"): - fd, err := url.ParseQuery(string(req.Body)) + err = req.ParseFormPayload() if err != nil { - log.Printf("[%s] error parsing form payload %+v\n", req.ID, err) - } else { - req.Payload = valuesToMap(fd) + log.Printf("[%s] %s", req.ID, err) } case strings.Contains(req.ContentType, "xml"): - req.Payload, err = mxj.NewMapXmlReader(bytes.NewReader(req.Body)) + err = req.ParseXMLPayload() if err != nil { - log.Printf("[%s] error parsing XML payload: %+v\n", req.ID, err) + log.Printf("[%s] %s", req.ID, err) } case isMultipart: From 346c761ef6f504362bb0869eae5a894ad39f2f1f Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Fri, 20 Nov 2020 16:32:55 -0600 Subject: [PATCH 20/26] 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 21/26] 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 From c2ffd465c4db38bb256606bcb8b955ec9392a442 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Tue, 24 Nov 2020 16:45:54 -0600 Subject: [PATCH 22/26] Add support for slashes in hook IDs When matching variables in routes, gorilla/mux uses a default pattern of "[^/]+", thereby prohibiting slashes in variable matching. Override the default pattern to remove this restriction. See https://github.com/gorilla/mux/blob/v1.8.0/regexp.go#L50 Fixes #421 --- test/hooks.json.tmpl | 16 ++++++++++++++++ test/hooks.yaml.tmpl | 12 ++++++++++++ webhook.go | 29 ++++++++++++++++++++--------- webhook_test.go | 23 +++++++++++++++++++++++ 4 files changed, 71 insertions(+), 9 deletions(-) diff --git a/test/hooks.json.tmpl b/test/hooks.json.tmpl index fd5cb83..4c40eba 100644 --- a/test/hooks.json.tmpl +++ b/test/hooks.json.tmpl @@ -184,6 +184,22 @@ } } }, + { + "id": "sendgrid/dir", + "execute-command": "{{ .Hookecho }}", + "command-working-directory": "/", + "response-message": "success", + "trigger-rule": { + "match": { + "type": "value", + "parameter": { + "source": "payload", + "name": "root.0.event" + }, + "value": "it worked!" + } + } + }, { "id": "plex", "execute-command": "{{ .Hookecho }}", diff --git a/test/hooks.yaml.tmpl b/test/hooks.yaml.tmpl index 82833dc..127016e 100644 --- a/test/hooks.yaml.tmpl +++ b/test/hooks.yaml.tmpl @@ -109,6 +109,18 @@ name: root.0.event value: processed +- id: sendgrid/dir + execute-command: '{{ .Hookecho }}' + command-working-directory: / + response-message: success + trigger-rule: + match: + type: value + parameter: + source: payload + name: root.0.event + value: it worked! + - id: plex trigger-rule: match: diff --git a/webhook.go b/webhook.go index ea6a9f2..922e326 100644 --- a/webhook.go +++ b/webhook.go @@ -262,7 +262,7 @@ func main() { // Clean up input *httpMethods = strings.ToUpper(strings.ReplaceAll(*httpMethods, " ", "")) - hooksURL := makeURL(hooksURLPrefix) + hooksURL := makeRoutePattern(hooksURLPrefix) r.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) { fmt.Fprint(w, "OK") @@ -278,7 +278,7 @@ func main() { // Serve HTTP if !*secure { - log.Printf("serving hooks on http://%s%s", addr, hooksURL) + log.Printf("serving hooks on http://%s%s", addr, makeHumanPattern(hooksURLPrefix)) log.Print(svr.Serve(ln)) return @@ -293,7 +293,7 @@ func main() { } svr.TLSNextProto = make(map[string]func(*http.Server, *tls.Conn, http.Handler)) // disable http/2 - log.Printf("serving hooks on https://%s%s", addr, hooksURL) + log.Printf("serving hooks on https://%s%s", addr, makeHumanPattern(hooksURLPrefix)) log.Print(svr.ServeTLS(ln, *cert, *key)) } @@ -763,10 +763,21 @@ func valuesToMap(values map[string][]string) map[string]interface{} { return ret } -// makeURL builds a hook URL with or without a prefix. -func makeURL(prefix *string) string { - if prefix == nil || *prefix == "" { - return "/{id}" - } - return "/" + *prefix + "/{id}" +// makeRoutePattern builds a pattern matching URL for the mux. +func makeRoutePattern(prefix *string) string { + return makeBaseURL(prefix) + "/{id:.*}" +} + +// makeHumanPattern builds a human-friendly URL for display. +func makeHumanPattern(prefix *string) string { + return makeBaseURL(prefix) + "/{id}" +} + +// makeBaseURL creates the base URL before any mux pattern matching. +func makeBaseURL(prefix *string) string { + if prefix == nil || *prefix == "" { + return "" + } + + return "/" + *prefix } diff --git a/webhook_test.go b/webhook_test.go index 885efb6..6364f1a 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -589,6 +589,29 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 "sg_event_id": "sg_event_id", "sg_message_id": "sg_message_id" } +]`, + false, + http.StatusOK, + `success`, + ``, + }, + { + "slash-in-hook-id", + "sendgrid/dir", + nil, + "POST", + nil, + "application/json", + `[ + { + "email": "example@test.com", + "timestamp": 1513299569, + "smtp-id": "<14c5d75ce93.dfd.64b469@ismtpd-555>", + "event": "it worked!", + "category": "cat facts", + "sg_event_id": "sg_event_id", + "sg_message_id": "sg_message_id" + } ]`, false, http.StatusOK, From 6d2f26d95283a77a97722adb57a2c6a2574576b3 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Tue, 24 Nov 2020 21:11:45 -0600 Subject: [PATCH 23/26] Add soft signature failure support Add a new trigger-signature-soft-failures option to allow soft signature failures in Or rules. Fixes #234 --- docs/Hook-Definition.md | 1 + internal/hook/hook.go | 15 +- internal/hook/request.go | 3 + test/hooks.json.tmpl | 143 +++++++++++++++++ test/hooks.yaml.tmpl | 75 +++++++++ webhook.go | 3 + webhook_test.go | 320 ++++++++++++++++++++++++++++++++++++++- 7 files changed, 554 insertions(+), 6 deletions(-) diff --git a/docs/Hook-Definition.md b/docs/Hook-Definition.md index 925bea1..8a7e744 100644 --- a/docs/Hook-Definition.md +++ b/docs/Hook-Definition.md @@ -22,6 +22,7 @@ Hooks are defined as objects in the JSON or YAML hooks configuration file. Pleas * `pass-file-to-command` - specifies a list of entries that will be serialized as a file. Incoming [data](Referencing-Request-Values.md) will be serialized in a request-temporary-file (otherwise parallel calls of the hook would lead to concurrent overwritings of the file). The filename to be addressed within the subsequent script is provided via an environment variable. Use `envname` to specify the name of the environment variable. If `envname` is not provided `HOOK_` and the name used to reference the request value are used. Defining `command-working-directory` will store the file relative to this location, if not provided, the systems temporary file directory will be used. If `base64decode` is true, the incoming binary data will be base 64 decoded prior to storing it into the file. By default the corresponding file will be removed after the webhook exited. * `trigger-rule` - specifies the rule that will be evaluated in order to determine should the hook be triggered. Check [Hook rules page](Hook-Rules.md) to see the list of valid rules and their usage * `trigger-rule-mismatch-http-response-code` - specifies the HTTP status code to be returned when the trigger rule is not satisfied + * `trigger-signature-soft-failures` - allow signature validation failures within Or rules; by default, signature failures are treated as errors. ## Examples Check out [Hook examples page](Hook-Examples.md) for more complex examples of hooks. diff --git a/internal/hook/hook.go b/internal/hook/hook.go index 19809e7..c425179 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -95,6 +95,16 @@ func (e *SignatureError) Error() string { return fmt.Sprintf("invalid payload signature %s%s", e.Signature, empty) } +// IsSignatureError returns whether err is of type SignatureError. +func IsSignatureError(err error) bool { + switch err.(type) { + case *SignatureError: + return true + default: + return false + } +} + // ArgumentError describes an invalid argument passed to Hook. type ArgumentError struct { Argument Argument @@ -563,6 +573,7 @@ type Hook struct { JSONStringParameters []Argument `json:"parse-parameters-as-json,omitempty"` TriggerRule *Rules `json:"trigger-rule,omitempty"` TriggerRuleMismatchHttpResponseCode int `json:"trigger-rule-mismatch-http-response-code,omitempty"` + TriggerSignatureSoftFailures bool `json:"trigger-signature-soft-failures,omitempty"` IncomingPayloadContentType string `json:"incoming-payload-content-type,omitempty"` SuccessHttpResponseCode int `json:"success-http-response-code,omitempty"` HTTPMethods []string `json:"http-methods"` @@ -845,7 +856,9 @@ func (r OrRule) Evaluate(req *Request) (bool, error) { rv, err := v.Evaluate(req) if err != nil { if !IsParameterNodeError(err) { - return false, err + if !req.AllowSignatureErrors || (req.AllowSignatureErrors && !IsSignatureError(err)) { + return false, err + } } } diff --git a/internal/hook/request.go b/internal/hook/request.go index ef7c949..643d5a8 100644 --- a/internal/hook/request.go +++ b/internal/hook/request.go @@ -33,6 +33,9 @@ type Request struct { // The underlying HTTP request. RawRequest *http.Request + + // Treat signature errors as simple validate failures. + AllowSignatureErrors bool } func (r *Request) ParseJSONPayload() error { diff --git a/test/hooks.json.tmpl b/test/hooks.json.tmpl index fd5cb83..08d181a 100644 --- a/test/hooks.json.tmpl +++ b/test/hooks.json.tmpl @@ -55,6 +55,149 @@ ] } }, + { + "id": "github-multi-sig", + "execute-command": "{{ .Hookecho }}", + "command-working-directory": "/", + "http-methods": ["Post "], + "include-command-output-in-response": true, + "trigger-rule-mismatch-http-response-code": 400, + "trigger-signature-soft-failures": true, + "pass-environment-to-command": + [ + { + "source": "payload", + "name": "head_commit.timestamp" + } + ], + "pass-arguments-to-command": + [ + { + "source": "payload", + "name": "head_commit.id" + }, + { + "source": "payload", + "name": "head_commit.author.email" + } + ], + "trigger-rule": + { + "and": + [ + "or": + [ + { + "match": + { + "type": "payload-hmac-sha1", + "secret": "mysecretFAIL", + "parameter": + { + "source": "header", + "name": "X-Hub-Signature" + } + } + }, + { + "match": + { + "type": "payload-hmac-sha1", + "secret": "mysecret", + "parameter": + { + "source": "header", + "name": "X-Hub-Signature" + } + } + } + ], + { + "match": + { + "type": "value", + "value": "refs/heads/master", + "parameter": + { + "source": "payload", + "name": "ref" + } + } + } + ] + } + }, + { + "id": "github-multi-sig-fail", + "execute-command": "{{ .Hookecho }}", + "command-working-directory": "/", + "http-methods": ["Post "], + "include-command-output-in-response": true, + "trigger-rule-mismatch-http-response-code": 400, + "pass-environment-to-command": + [ + { + "source": "payload", + "name": "head_commit.timestamp" + } + ], + "pass-arguments-to-command": + [ + { + "source": "payload", + "name": "head_commit.id" + }, + { + "source": "payload", + "name": "head_commit.author.email" + } + ], + "trigger-rule": + { + "and": + [ + "or": + [ + { + "match": + { + "type": "payload-hmac-sha1", + "secret": "mysecretFAIL", + "parameter": + { + "source": "header", + "name": "X-Hub-Signature" + } + } + }, + { + "match": + { + "type": "payload-hmac-sha1", + "secret": "mysecret", + "parameter": + { + "source": "header", + "name": "X-Hub-Signature" + } + } + } + ], + { + "match": + { + "type": "value", + "value": "refs/heads/master", + "parameter": + { + "source": "payload", + "name": "ref" + } + } + } + ] + } + }, { "id": "bitbucket", "execute-command": "{{ .Hookecho }}", diff --git a/test/hooks.yaml.tmpl b/test/hooks.yaml.tmpl index 82833dc..163dc77 100644 --- a/test/hooks.yaml.tmpl +++ b/test/hooks.yaml.tmpl @@ -28,6 +28,81 @@ name: head_commit.timestamp command-working-directory: / +- id: github-multi-sig + http-methods: + - "Post " + trigger-rule: + and: + - or: + - match: + parameter: + source: header + name: X-Hub-Signature + secret: mysecretFAIL + type: payload-hmac-sha1 + - match: + parameter: + source: header + name: X-Hub-Signature + secret: mysecret + type: payload-hmac-sha1 + - match: + parameter: + source: payload + name: ref + type: value + value: refs/heads/master + include-command-output-in-response: true + trigger-rule-mismatch-http-response-code: 400 + trigger-signature-soft-failures: true + execute-command: '{{ .Hookecho }}' + pass-arguments-to-command: + - source: payload + name: head_commit.id + - source: payload + name: head_commit.author.email + pass-environment-to-command: + - source: payload + name: head_commit.timestamp + command-working-directory: / + +- id: github-multi-sig-fail + http-methods: + - "Post " + trigger-rule: + and: + - or: + - match: + parameter: + source: header + name: X-Hub-Signature + secret: mysecretFAIL + type: payload-hmac-sha1 + - match: + parameter: + source: header + name: X-Hub-Signature + secret: mysecret + type: payload-hmac-sha1 + - match: + parameter: + source: payload + name: ref + type: value + value: refs/heads/master + include-command-output-in-response: true + trigger-rule-mismatch-http-response-code: 400 + execute-command: '{{ .Hookecho }}' + pass-arguments-to-command: + - source: payload + name: head_commit.id + - source: payload + name: head_commit.author.email + pass-environment-to-command: + - source: payload + name: head_commit.timestamp + command-working-directory: / + - id: bitbucket trigger-rule: and: diff --git a/webhook.go b/webhook.go index ea6a9f2..ffbae4d 100644 --- a/webhook.go +++ b/webhook.go @@ -480,6 +480,9 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { if matchedHook.TriggerRule == nil { ok = true } else { + // Save signature soft failures option in request for evaluators + req.AllowSignatureErrors = matchedHook.TriggerSignatureSoftFailures + ok, err = matchedHook.TriggerRule.Evaluate(req) if err != nil { if !hook.IsParameterNodeError(err) { diff --git a/webhook_test.go b/webhook_test.go index 885efb6..7518300 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -133,10 +133,6 @@ func TestWebhook(t *testing.T) { { var bodyFailed bool - if res.StatusCode != tt.respStatus { - bodyFailed = true - } - if tt.bodyIsRE { bodyFailed = string(body) == tt.respBody } else { @@ -144,7 +140,7 @@ func TestWebhook(t *testing.T) { bodyFailed = !r.Match(body) } - if bodyFailed { + if res.StatusCode != tt.respStatus || 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) } } @@ -483,6 +479,320 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 `, ``, }, + { + "github-multi-sig", + "github-multi-sig", + nil, + "POST", + map[string]string{"X-Hub-Signature": "f68df0375d7b03e3eb29b4cf9f9ec12e08f42ff8"}, + "application/json", + `{ + "after":"1481a2de7b2a7d02428ad93446ab166be7793fbb", + "before":"17c497ccc7cca9c2f735aa07e9e3813060ce9a6a", + "commits":[ + { + "added":[ + + ], + "author":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "committer":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "distinct":true, + "id":"c441029cf673f84c8b7db52d0a5944ee5c52ff89", + "message":"Test", + "modified":[ + "README.md" + ], + "removed":[ + + ], + "timestamp":"2013-02-22T13:50:07-08:00", + "url":"https://github.com/octokitty/testing/commit/c441029cf673f84c8b7db52d0a5944ee5c52ff89" + }, + { + "added":[ + + ], + "author":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "committer":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "distinct":true, + "id":"36c5f2243ed24de58284a96f2a643bed8c028658", + "message":"This is me testing the windows client.", + "modified":[ + "README.md" + ], + "removed":[ + + ], + "timestamp":"2013-02-22T14:07:13-08:00", + "url":"https://github.com/octokitty/testing/commit/36c5f2243ed24de58284a96f2a643bed8c028658" + }, + { + "added":[ + "words/madame-bovary.txt" + ], + "author":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "committer":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "distinct":true, + "id":"1481a2de7b2a7d02428ad93446ab166be7793fbb", + "message":"Rename madame-bovary.txt to words/madame-bovary.txt", + "modified":[ + + ], + "removed":[ + "madame-bovary.txt" + ], + "timestamp":"2013-03-12T08:14:29-07:00", + "url":"https://github.com/octokitty/testing/commit/1481a2de7b2a7d02428ad93446ab166be7793fbb" + } + ], + "compare":"https://github.com/octokitty/testing/compare/17c497ccc7cc...1481a2de7b2a", + "created":false, + "deleted":false, + "forced":false, + "head_commit":{ + "added":[ + "words/madame-bovary.txt" + ], + "author":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "committer":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "distinct":true, + "id":"1481a2de7b2a7d02428ad93446ab166be7793fbb", + "message":"Rename madame-bovary.txt to words/madame-bovary.txt", + "modified":[ + + ], + "removed":[ + "madame-bovary.txt" + ], + "timestamp":"2013-03-12T08:14:29-07:00", + "url":"https://github.com/octokitty/testing/commit/1481a2de7b2a7d02428ad93446ab166be7793fbb" + }, + "pusher":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian" + }, + "ref":"refs/heads/master", + "repository":{ + "created_at":1332977768, + "description":"", + "fork":false, + "forks":0, + "has_downloads":true, + "has_issues":true, + "has_wiki":true, + "homepage":"", + "id":3860742, + "language":"Ruby", + "master_branch":"master", + "name":"testing", + "open_issues":2, + "owner":{ + "email":"lolwut@noway.biz", + "name":"octokitty" + }, + "private":false, + "pushed_at":1363295520, + "size":2156, + "stargazers":1, + "url":"https://github.com/octokitty/testing", + "watchers":1 + } + }`, + false, + http.StatusOK, + `arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz +env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 +`, + ``, + }, + { + "github-multi-sig-fail", + "github-multi-sig-fail", + nil, + "POST", + map[string]string{"X-Hub-Signature": "f68df0375d7b03e3eb29b4cf9f9ec12e08f42ff8"}, + "application/json", + `{ + "after":"1481a2de7b2a7d02428ad93446ab166be7793fbb", + "before":"17c497ccc7cca9c2f735aa07e9e3813060ce9a6a", + "commits":[ + { + "added":[ + + ], + "author":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "committer":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "distinct":true, + "id":"c441029cf673f84c8b7db52d0a5944ee5c52ff89", + "message":"Test", + "modified":[ + "README.md" + ], + "removed":[ + + ], + "timestamp":"2013-02-22T13:50:07-08:00", + "url":"https://github.com/octokitty/testing/commit/c441029cf673f84c8b7db52d0a5944ee5c52ff89" + }, + { + "added":[ + + ], + "author":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "committer":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "distinct":true, + "id":"36c5f2243ed24de58284a96f2a643bed8c028658", + "message":"This is me testing the windows client.", + "modified":[ + "README.md" + ], + "removed":[ + + ], + "timestamp":"2013-02-22T14:07:13-08:00", + "url":"https://github.com/octokitty/testing/commit/36c5f2243ed24de58284a96f2a643bed8c028658" + }, + { + "added":[ + "words/madame-bovary.txt" + ], + "author":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "committer":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "distinct":true, + "id":"1481a2de7b2a7d02428ad93446ab166be7793fbb", + "message":"Rename madame-bovary.txt to words/madame-bovary.txt", + "modified":[ + + ], + "removed":[ + "madame-bovary.txt" + ], + "timestamp":"2013-03-12T08:14:29-07:00", + "url":"https://github.com/octokitty/testing/commit/1481a2de7b2a7d02428ad93446ab166be7793fbb" + } + ], + "compare":"https://github.com/octokitty/testing/compare/17c497ccc7cc...1481a2de7b2a", + "created":false, + "deleted":false, + "forced":false, + "head_commit":{ + "added":[ + "words/madame-bovary.txt" + ], + "author":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "committer":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian", + "username":"octokitty" + }, + "distinct":true, + "id":"1481a2de7b2a7d02428ad93446ab166be7793fbb", + "message":"Rename madame-bovary.txt to words/madame-bovary.txt", + "modified":[ + + ], + "removed":[ + "madame-bovary.txt" + ], + "timestamp":"2013-03-12T08:14:29-07:00", + "url":"https://github.com/octokitty/testing/commit/1481a2de7b2a7d02428ad93446ab166be7793fbb" + }, + "pusher":{ + "email":"lolwut@noway.biz", + "name":"Garen Torikian" + }, + "ref":"refs/heads/master", + "repository":{ + "created_at":1332977768, + "description":"", + "fork":false, + "forks":0, + "has_downloads":true, + "has_issues":true, + "has_wiki":true, + "homepage":"", + "id":3860742, + "language":"Ruby", + "master_branch":"master", + "name":"testing", + "open_issues":2, + "owner":{ + "email":"lolwut@noway.biz", + "name":"octokitty" + }, + "private":false, + "pushed_at":1363295520, + "size":2156, + "stargazers":1, + "url":"https://github.com/octokitty/testing", + "watchers":1 + } + }`, + false, + http.StatusInternalServerError, + `Error occurred while evaluating hook rules.`, + ``, + }, { "bitbucket", // bitbucket sends their payload using uriencoded params. "bitbucket", From 62f9c01cab97598969456faa1148f3cc32c4f817 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Wed, 25 Nov 2020 10:06:41 -0600 Subject: [PATCH 24/26] Add option to send raw request body to command The existing `entire-payload` option sends a JSON representation of the parsed request body. Add a new `raw-request-body` source to send the raw request body. Fixes #439 --- internal/hook/hook.go | 22 +++++++++++++--------- test/hooks.json.tmpl | 11 +++++++++++ test/hooks.yaml.tmpl | 7 +++++++ webhook_test.go | 19 +++++++++++++++++++ 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/internal/hook/hook.go b/internal/hook/hook.go index 19809e7..987f324 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -31,15 +31,16 @@ import ( // Constants used to specify the parameter source const ( - SourceHeader string = "header" - SourceQuery string = "url" - SourceQueryAlias string = "query" - SourcePayload string = "payload" - SourceRequest string = "request" - SourceString string = "string" - SourceEntirePayload string = "entire-payload" - SourceEntireQuery string = "entire-query" - SourceEntireHeaders string = "entire-headers" + SourceHeader string = "header" + SourceQuery string = "url" + SourceQueryAlias string = "query" + SourcePayload string = "payload" + SourceRawRequestBody string = "raw-request-body" + SourceRequest string = "request" + SourceString string = "string" + SourceEntirePayload string = "entire-payload" + SourceEntireQuery string = "entire-query" + SourceEntireHeaders string = "entire-headers" ) const ( @@ -449,6 +450,9 @@ func (ha *Argument) Get(r *Request) (string, error) { case SourceString: return ha.Name, nil + case SourceRawRequestBody: + return string(r.Body), nil + case SourceRequest: if r == nil || r.RawRequest == nil { return "", errors.New("request is nil") diff --git a/test/hooks.json.tmpl b/test/hooks.json.tmpl index fd5cb83..032d331 100644 --- a/test/hooks.json.tmpl +++ b/test/hooks.json.tmpl @@ -168,6 +168,17 @@ ], } }, + { + "id": "txt-raw", + "execute-command": "{{ .Hookecho }}", + "command-working-directory": "/", + "include-command-output-in-response": true, + "pass-arguments-to-command": [ + { + "source": "raw-request-body" + } + ] + }, { "id": "sendgrid", "execute-command": "{{ .Hookecho }}", diff --git a/test/hooks.yaml.tmpl b/test/hooks.yaml.tmpl index 82833dc..9d73d8b 100644 --- a/test/hooks.yaml.tmpl +++ b/test/hooks.yaml.tmpl @@ -97,6 +97,13 @@ name: "app.messages.message.#text" value: "Hello!!" +- id: txt-raw + execute-command: '{{ .Hookecho }}' + command-working-directory: / + include-command-output-in-response: true + pass-arguments-to-command: + - source: raw-request-body + - id: sendgrid execute-command: '{{ .Hookecho }}' command-working-directory: / diff --git a/webhook_test.go b/webhook_test.go index 885efb6..539cdd4 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -572,6 +572,25 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 `success`, ``, }, + { + "txt-raw", + "txt-raw", + nil, + "POST", + map[string]string{"Content-Type": "text/plain"}, + "text/plain", + `# FOO + +blah +blah`, + false, + http.StatusOK, + `# FOO + +blah +blah`, + ``, + }, { "payload-json-array", "sendgrid", From f2b536dbad4a7cb8da1fbef6f506b226203c6997 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Sat, 5 Dec 2020 16:33:04 -0600 Subject: [PATCH 25/26] Add string parameter example to docs Fixes #487 --- docs/Hook-Examples.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/docs/Hook-Examples.md b/docs/Hook-Examples.md index ba581eb..36e4ddc 100644 --- a/docs/Hook-Examples.md +++ b/docs/Hook-Examples.md @@ -20,6 +20,7 @@ although the examples on this page all use the JSON format. * [Travis CI webhook](#travis-ci-webhook) * [XML Payload](#xml-payload) * [Multipart Form Data](#multipart-form-data) +* [Pass string arguments to command](#pass-string-arguments-to-command) ## Incoming Github webhook ```json @@ -589,3 +590,34 @@ Content-Disposition: form-data; name="thumb"; filename="thumb.jpg" ``` We key off of the `name` attribute in the `Content-Disposition` value. + +## Pass string arguments to command + +To pass simple string arguments to a command, use the `string` parameter source. +The following example will pass two static string parameters ("-e 123123") to the +`execute-command` before appending the `pusher.email` value from the payload: + +```json +[ + { + "id": "webhook", + "execute-command": "/home/adnan/redeploy-go-webhook.sh", + "command-working-directory": "/home/adnan/go", + "pass-arguments-to-command": + [ + { + "source": "string", + "name": "-e" + }, + { + "source": "string", + "name": "123123" + }, + { + "source": "payload", + "name": "pusher.email" + } + ] + } +] +``` From 159cb4a911d6499b70a9bab0d4ab16d142bf2af9 Mon Sep 17 00:00:00 2001 From: Adnan Hajdarevic Date: Sun, 6 Dec 2020 08:42:09 +0100 Subject: [PATCH 26/26] bump version to 2.8.0 --- webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webhook.go b/webhook.go index 1c62aab..9ea45aa 100644 --- a/webhook.go +++ b/webhook.go @@ -25,7 +25,7 @@ import ( ) const ( - version = "2.7.0" + version = "2.8.0" ) var (