Repository: adnanh/webhook (go)

Issue:
Allow soft failures for signature validation
@adnanh @moorereason 

Heads up, I've spotted an error in the development branch it is only impacting the new Scalr Signature rule. 

This is is when using the OR match rule which seems to only work off the top one in the list for example in the below only A would work.:

```
"trigger-rule":{
                "or":[
                        {
                        "match":{
                                "type": "scalr-signature",
                                "secret": "secret **A** inserted here"
                                }
                        },
                        {
                        "match":{
                                "type": "scalr-signature",
                                "secret": "secret **B** inserted here"
                                }
                        }
                ]
        }
```

I think I need to make an update in the sections below related to 'scalr-signature' in order to be able to support the use of the OR and AND MATCH RULES.

I'm going to take a look now but please let me know if you think this might be the case.

@AloysAugustin please also let me know if you by chance spot this message and spot the missing line

**EXTRACT FROM hook.go**

```
// AndRule will evaluate to true if and only if all of the ChildRules evaluate to true
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) {
	res := true

	for _, v := range r {
		rv, err := v.Evaluate(headers, query, payload, body, remoteAddr)
		if err != nil {
			return false, err
		}

		res = res && rv
		if !res {
			return res, nil
		}
	}

	return res, nil
}

// OrRule will evaluate to true if any of the ChildRules evaluate to true
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) {
	res := false

	for _, v := range r {
		rv, err := v.Evaluate(headers, query, payload, body, remoteAddr)
		if err != nil {
			return false, err
		}

		res = res || rv
		if res {
			return res, nil
		}
	}

	return res, nil
}
```





Pull request description:
Add soft signature failure support
Add a new trigger-signature-soft-failures option to allow soft signature
failures in Or rules.

Fixes #234

Patch:
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/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) {


Test patch:
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_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",