Merge pull request #56 from adnanh/development

Bugfixes

Author: adnanh

hook/hook.go

@@ -5,7 +5,6 @@ import (
 	"crypto/sha1"
 	"encoding/hex"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io/ioutil"
 	"reflect"
@@ -31,8 +30,17 @@ const (
 	EnvNamespace string = "HOOK_"
 )
 
-// ErrInvalidPayloadSignature describes an invalid payload signature.
-var ErrInvalidPayloadSignature = errors.New("invalid payload signature")
+// SignatureError describes an invalid payload signature passed to Hook.
+type SignatureError struct {
+	Signature string
+}
+
+func (e *SignatureError) Error() string {
+	if e == nil {
+		return "<nil>"
+	}
+	return fmt.Sprintf("invalid payload signature %s", e.Signature)
+}
 
 // ArgumentError describes an invalid argument passed to Hook.
 type ArgumentError struct {
@@ -84,7 +92,7 @@ func CheckPayloadSignature(payload []byte, secret string, signature string) (str
 	expectedMAC := hex.EncodeToString(mac.Sum(nil))
 
 	if !hmac.Equal([]byte(signature), []byte(expectedMAC)) {
-		err = ErrInvalidPayloadSignature
+		return expectedMAC, &SignatureError{expectedMAC}
 	}
 	return expectedMAC, err
 }
@@ -302,6 +310,7 @@ func (h *Hook) ExtractCommandArguments(headers, query, payload *map[string]inter
 		if arg, ok := h.PassArgumentsToCommand[i].Get(headers, query, payload); ok {
 			args = append(args, arg)
 		} else {
+			args = append(args, "")
 			return args, &ArgumentError{h.PassArgumentsToCommand[i]}
 		}
 	}

hook/hook_test.go

@@ -123,7 +123,7 @@ var hookExtractCommandArgumentsTests = []struct {
 }{
 	{"test", []Argument{Argument{"header", "a"}}, &map[string]interface{}{"a": "z"}, nil, nil, []string{"test", "z"}, true},
 	// failures
-	{"fail", []Argument{Argument{"payload", "a"}}, &map[string]interface{}{"a": "z"}, nil, nil, []string{"fail"}, false},
+	{"fail", []Argument{Argument{"payload", "a"}}, &map[string]interface{}{"a": "z"}, nil, nil, []string{"fail", ""}, false},
 }
 
 func TestHookExtractCommandArguments(t *testing.T) {

test/hooks.json.tmpl

@@ -8,7 +8,7 @@
     [
       {
         "source": "payload",
-        "name": "pusher.email"
+        "name": "head_commit.timestamp"
       }
     ],
     "pass-arguments-to-command":
@@ -19,11 +19,7 @@
       },
       {
         "source": "payload",
-        "name": "pusher.name"
-      },
-      {
-        "source": "payload",
-        "name": "pusher.email"
+        "name": "head_commit.author.email"
       }
     ],
     "trigger-rule":

webhook.go

@@ -238,16 +238,16 @@ func handleHook(h *hook.Hook, headers, query, payload *map[string]interface{}, b
 	cmd.Args, err = h.ExtractCommandArguments(headers, query, payload)
 	if err != nil {
 		log.Printf("error extracting command arguments: %s", err)
-		return ""
 	}
 
-	cmd.Env, err = h.ExtractCommandArgumentsForEnv(headers, query, payload)
+	var envs []string
+	envs, err = h.ExtractCommandArgumentsForEnv(headers, query, payload)
 	if err != nil {
-		log.Printf("error extracting command arguments: %s", err)
-		return ""
+		log.Printf("error extracting command arguments for environment: %s", err)
 	}
+	cmd.Env = append(os.Environ(), envs...)
 
-	log.Printf("executing %s (%s) with arguments %s and environment %s using %s as cwd\n", h.ExecuteCommand, cmd.Path, cmd.Args, cmd.Env, cmd.Dir)
+	log.Printf("executing %s (%s) with arguments %q and environment %s using %s as cwd\n", h.ExecuteCommand, cmd.Path, cmd.Args, envs, cmd.Dir)
 
 	out, err := cmd.CombinedOutput()
 

webhook_test.go

@@ -372,7 +372,7 @@ var hookHandlerTests = []struct {
 		}`,
 		false,
 		http.StatusOK,
-		`{"output":"arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb Garen Torikian [email protected]\nenv: [email protected]\n"}`,
+		`{"output":"arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb [email protected]\nenv: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00\n"}`,
 	},
 	{
 		"bitbucket", // bitbucket sends their payload using uriencoded params.
@@ -434,5 +434,80 @@ var hookHandlerTests = []struct {
 		`{"message":"success","output":"arg: b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327 John Smith [email protected]\n"}`,
 	},
 
+	{
+		"missing-cmd-arg", // missing head_commit.author.email
+		"github",
+		map[string]string{"X-Hub-Signature": "ab03955b9377f530aa298b1b6d273ae9a47e1e40"},
+		`{
+			"head_commit":{
+				"added":[
+					"words/madame-bovary.txt"
+				],
+				"author":{
+					"email":"[email protected]",
+					"name":"Garen Torikian",
+					"username":"octokitty"
+				},
+				"committer":{
+					"email":"[email protected]",
+					"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"
+			},
+			"ref":"refs/heads/master"
+		}`,
+		false,
+		http.StatusOK,
+		`{"output":"arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb [email protected]\nenv: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00\n"}`,
+	},
+
+	{
+		"missing-env-arg", // missing head_commit.timestamp
+		"github",
+		map[string]string{"X-Hub-Signature": "2cf8b878cb6b74a25090a140fa4a474be04b97fa"},
+		`{
+			"head_commit":{
+				"added":[
+					"words/madame-bovary.txt"
+				],
+				"author":{
+					"email":"[email protected]",
+					"name":"Garen Torikian",
+					"username":"octokitty"
+				},
+				"committer":{
+					"email":"[email protected]",
+					"name":"Garen Torikian",
+					"username":"octokitty"
+				},
+				"distinct":true,
+				"id":"1481a2de7b2a7d02428ad93446ab166be7793fbb",
+				"message":"Rename madame-bovary.txt to words/madame-bovary.txt",
+				"modified":[
+
+				],
+				"removed":[
+					"madame-bovary.txt"
+				],
+				"url":"https://github.com/octokitty/testing/commit/1481a2de7b2a7d02428ad93446ab166be7793fbb"
+			},
+			"ref":"refs/heads/master"
+		}`,
+		false,
+		http.StatusOK,
+		`{"output":"arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb [email protected]\n"}`,
+	},
+
 	{"empty payload", "github", nil, `{}`, false, http.StatusOK, `Hook rules were not satisfied.`},
 }