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 4c40eba..d04a9f2 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 127016e..b2f2de6 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 922e326..1c62aab 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 6364f1a..bd63dd5 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",