Merge pull request #101 from adnanh/development

webhook 2.5.0

.gitignore

@@ -0,0 +1,4 @@
+.idea
+.cover
+coverage
+webhook

Makefile

@@ -0,0 +1,29 @@
+OS = darwin freebsd linux openbsd windows
+ARCHS = 386 arm amd64 arm64
+
+all: build release
+
+build: deps
+	go build
+
+release: clean deps
+	@for arch in $(ARCHS);\
+	do \
+		for os in $(OS);\
+		do \
+			echo "Building $$os-$$arch"; \
+			mkdir -p build/webhook-$$os-$$arch/; \
+			GOOS=$$os GOARCH=$$arch go build -o build/webhook-$$os-$$arch/webhook; \
+			tar cz -C build -f build/webhook-$$os-$$arch.tar.gz webhook-$$os-$$arch; \
+		done \
+	done
+
+test: deps
+	go test ./...
+
+deps:
+	go get -d -v -t ./...
+
+clean:
+	rm -rf build
+	rm -f webhook

hook/hook.go

@@ -93,7 +93,7 @@ func CheckPayloadSignature(payload []byte, secret string, signature string) (str
 	expectedMAC := hex.EncodeToString(mac.Sum(nil))
 
 	if !hmac.Equal([]byte(signature), []byte(expectedMAC)) {
-		return expectedMAC, &SignatureError{expectedMAC}
+		return expectedMAC, &SignatureError{signature}
 	}
 	return expectedMAC, err
 }
@@ -198,6 +198,7 @@ func ExtractParameterAsString(s string, params interface{}) (string, bool) {
 type Argument struct {
 	Source  string `json:"source,omitempty"`
 	Name    string `json:"name,omitempty"`
+	EnvName string `json:"envname,omitempty"`
 }
 
 // Get Argument method returns the value for the Argument's key name
@@ -364,7 +365,13 @@ func (h *Hook) ExtractCommandArgumentsForEnv(headers, query, payload *map[string
 
 	for i := range h.PassEnvironmentToCommand {
 		if arg, ok := h.PassEnvironmentToCommand[i].Get(headers, query, payload); ok {
+			if h.PassEnvironmentToCommand[i].EnvName != "" {
+				// first try to use the EnvName if specified
+				args = append(args, h.PassEnvironmentToCommand[i].EnvName+"="+arg)
+			} else {
+				// then fallback on the name
 				args = append(args, EnvNamespace+h.PassEnvironmentToCommand[i].Name+"="+arg)
+			}
 		} else {
 			return args, &ArgumentError{h.PassEnvironmentToCommand[i]}
 		}
@@ -405,23 +412,6 @@ func (h *Hooks) Match(id string) *Hook {
 	return nil
 }
 
-// MatchAll iterates through Hooks and returns all of the hooks that match the
-// given ID, if no hook matches the given ID, nil is returned
-func (h *Hooks) MatchAll(id string) []*Hook {
-	var matchedHooks []*Hook
-	for i := range *h {
-		if (*h)[i].ID == id {
-			matchedHooks = append(matchedHooks, &(*h)[i])
-		}
-	}
-
-	if len(matchedHooks) > 0 {
-		return matchedHooks
-	}
-
-	return nil
-}
-
 // Rules is a structure that contains one of the valid rule types
 type Rules struct {
 	And   *AndRule   `json:"and,omitempty"`
@@ -531,10 +521,3 @@ func (r MatchRule) Evaluate(headers, query, payload *map[string]interface{}, bod
 	}
 	return false, nil
 }
-
-// CommandStatusResponse type encapsulates the executed command exit code, message, stdout and stderr
-type CommandStatusResponse struct {
-	ResponseMessage string `json:"message,omitempty"`
-	Output          string `json:"output,omitempty"`
-	Error           string `json:"error,omitempty"`
-}

hook/hook_test.go

@@ -2,6 +2,7 @@ package hook
 
 import (
 	"reflect"
+	"strings"
 	"testing"
 )
 
@@ -25,6 +26,10 @@ func TestCheckPayloadSignature(t *testing.T) {
 		if (err == nil) != tt.ok || mac != tt.mac {
 			t.Errorf("failed to check payload signature {%q, %q, %q}:\nexpected {mac:%#v, ok:%#v},\ngot {mac:%#v, ok:%#v}", tt.payload, tt.secret, tt.signature, tt.mac, tt.ok, mac, (err == nil))
 		}
+
+		if err != nil && strings.Contains(err.Error(), tt.mac) {
+			t.Errorf("error message should not disclose expected mac: %s", err)
+		}
 	}
 }
 
@@ -80,7 +85,7 @@ var argumentGetTests = []struct {
 
 func TestArgumentGet(t *testing.T) {
 	for _, tt := range argumentGetTests {
-		a := Argument{tt.source, tt.name}
+		a := Argument{tt.source, tt.name, ""}
 		value, ok := a.Get(tt.headers, tt.query, tt.payload)
 		if ok != tt.ok || value != tt.value {
 			t.Errorf("failed to get {%q, %q}:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, ok:%#v}", tt.source, tt.name, tt.value, tt.ok, value, ok)
@@ -94,14 +99,14 @@ var hookParseJSONParametersTests = []struct {
 	rheaders, rquery, rpayload *map[string]interface{}
 	ok                         bool
 }{
-	{[]Argument{Argument{"header", "a"}}, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, nil, nil, true},
-	{[]Argument{Argument{"url", "a"}}, nil, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, nil, true},
-	{[]Argument{Argument{"payload", "a"}}, nil, nil, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, true},
-	{[]Argument{Argument{"header", "z"}}, &map[string]interface{}{"z": `{}`}, nil, nil, &map[string]interface{}{"z": map[string]interface{}{}}, nil, nil, true},
+	{[]Argument{Argument{"header", "a", ""}}, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, nil, nil, true},
+	{[]Argument{Argument{"url", "a", ""}}, nil, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, nil, true},
+	{[]Argument{Argument{"payload", "a", ""}}, nil, nil, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, true},
+	{[]Argument{Argument{"header", "z", ""}}, &map[string]interface{}{"z": `{}`}, nil, nil, &map[string]interface{}{"z": map[string]interface{}{}}, nil, nil, true},
 	// failures
-	{[]Argument{Argument{"header", "z"}}, &map[string]interface{}{"z": ``}, nil, nil, &map[string]interface{}{"z": ``}, nil, nil, false},     // empty string
-	{[]Argument{Argument{"header", "y"}}, &map[string]interface{}{"X": `{}`}, nil, nil, &map[string]interface{}{"X": `{}`}, nil, nil, false}, // missing parameter
-	{[]Argument{Argument{"string", "z"}}, &map[string]interface{}{"z": ``}, nil, nil, &map[string]interface{}{"z": ``}, nil, nil, false},     // invalid argument source
+	{[]Argument{Argument{"header", "z", ""}}, &map[string]interface{}{"z": ``}, nil, nil, &map[string]interface{}{"z": ``}, nil, nil, false},     // empty string
+	{[]Argument{Argument{"header", "y", ""}}, &map[string]interface{}{"X": `{}`}, nil, nil, &map[string]interface{}{"X": `{}`}, nil, nil, false}, // missing parameter
+	{[]Argument{Argument{"string", "z", ""}}, &map[string]interface{}{"z": ``}, nil, nil, &map[string]interface{}{"z": ``}, nil, nil, false},     // invalid argument source
 }
 
 func TestHookParseJSONParameters(t *testing.T) {
@@ -121,9 +126,9 @@ var hookExtractCommandArgumentsTests = []struct {
 	value                   []string
 	ok                      bool
 }{
-	{"test", []Argument{Argument{"header", "a"}}, &map[string]interface{}{"a": "z"}, nil, nil, []string{"test", "z"}, true},
+	{"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) {
@@ -136,6 +141,67 @@ func TestHookExtractCommandArguments(t *testing.T) {
 	}
 }
 
+// Here we test the extraction of env variables when the user defined a hook
+// with the "pass-environment-to-command" directive
+// we test both cases where the name of the data is used as the name of the
+// env key & the case where the hook definition sets the env var name to a
+// fixed value using the envname construct like so::
+//    [
+//      {
+//        "id": "push",
+//        "execute-command": "bb2mm",
+//        "command-working-directory": "/tmp",
+//        "pass-environment-to-command":
+//        [
+//          {
+//            "source": "entire-payload",
+//            "envname": "PAYLOAD"
+//          },
+//        ]
+//      }
+//    ]
+var hookExtractCommandArgumentsForEnvTests = []struct {
+	exec                    string
+	args                    []Argument
+	headers, query, payload *map[string]interface{}
+	value                   []string
+	ok                      bool
+}{
+	// successes
+	{
+		"test",
+		[]Argument{Argument{"header", "a", ""}},
+		&map[string]interface{}{"a": "z"}, nil, nil,
+		[]string{"HOOK_a=z"},
+		true,
+	},
+	{
+		"test",
+		[]Argument{Argument{"header", "a", "MYKEY"}},
+		&map[string]interface{}{"a": "z"}, nil, nil,
+		[]string{"MYKEY=z"},
+		true,
+	},
+	// failures
+	{
+		"fail",
+		[]Argument{Argument{"payload", "a", ""}},
+		&map[string]interface{}{"a": "z"}, nil, nil,
+		[]string{},
+		false,
+	},
+}
+
+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)
+		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))
+		}
+	}
+}
+
 var hooksLoadFromFileTests = []struct {
 	path string
 	ok   bool
@@ -182,16 +248,16 @@ var matchRuleTests = []struct {
 	ok                        bool
 	err                       bool
 }{
-	{"value", "", "", "z", Argument{"header", "a"}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, true, false},
-	{"regex", "^z", "", "z", Argument{"header", "a"}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, true, false},
-	{"payload-hash-sha1", "", "secret", "", Argument{"header", "a"}, &map[string]interface{}{"a": "b17e04cbb22afa8ffbff8796fc1894ed27badd9e"}, nil, nil, []byte(`{"a": "z"}`), true, false},
+	{"value", "", "", "z", Argument{"header", "a", ""}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, true, false},
+	{"regex", "^z", "", "z", Argument{"header", "a", ""}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, true, false},
+	{"payload-hash-sha1", "", "secret", "", Argument{"header", "a", ""}, &map[string]interface{}{"a": "b17e04cbb22afa8ffbff8796fc1894ed27badd9e"}, nil, nil, []byte(`{"a": "z"}`), true, false},
 	// failures
-	{"value", "", "", "X", Argument{"header", "a"}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, false},
-	{"regex", "^X", "", "", Argument{"header", "a"}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, false},
-	{"value", "", "2", "X", Argument{"header", "a"}, &map[string]interface{}{"y": "z"}, nil, nil, []byte{}, false, false}, // reference invalid header
+	{"value", "", "", "X", Argument{"header", "a", ""}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, false},
+	{"regex", "^X", "", "", Argument{"header", "a", ""}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, false},
+	{"value", "", "2", "X", Argument{"header", "a", ""}, &map[string]interface{}{"y": "z"}, nil, nil, []byte{}, false, false}, // reference invalid header
 	// errors
-	{"regex", "*", "", "", Argument{"header", "a"}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, true},                 // invalid regex
-	{"payload-hash-sha1", "", "secret", "", Argument{"header", "a"}, &map[string]interface{}{"a": ""}, nil, nil, []byte{}, false, true}, // invalid hmac
+	{"regex", "*", "", "", Argument{"header", "a", ""}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, true},                 // invalid regex
+	{"payload-hash-sha1", "", "secret", "", Argument{"header", "a", ""}, &map[string]interface{}{"a": ""}, nil, nil, []byte{}, false, true}, // invalid hmac
 }
 
 func TestMatchRule(t *testing.T) {
@@ -215,8 +281,8 @@ var andRuleTests = []struct {
 	{
 		"(a=z, b=y): a=z && b=y",
 		AndRule{
-			{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}},
-			{Match: &MatchRule{"value", "", "", "y", Argument{"header", "b"}}},
+			{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}},
+			{Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", ""}}},
 		},
 		&map[string]interface{}{"a": "z", "b": "y"}, nil, nil, []byte{},
 		true, false,
@@ -224,8 +290,8 @@ var andRuleTests = []struct {
 	{
 		"(a=z, b=Y): a=z && b=y",
 		AndRule{
-			{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}},
-			{Match: &MatchRule{"value", "", "", "y", Argument{"header", "b"}}},
+			{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}},
+			{Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", ""}}},
 		},
 		&map[string]interface{}{"a": "z", "b": "Y"}, nil, nil, []byte{},
 		false, false,
@@ -234,22 +300,22 @@ var andRuleTests = []struct {
 	{
 		"(a=z, b=y, c=x, d=w=, e=X, f=X): a=z && (b=y && c=x) && (d=w || e=v) && !f=u",
 		AndRule{
-			{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}},
+			{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}},
 			{
 				And: &AndRule{
-					{Match: &MatchRule{"value", "", "", "y", Argument{"header", "b"}}},
-					{Match: &MatchRule{"value", "", "", "x", Argument{"header", "c"}}},
+					{Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", ""}}},
+					{Match: &MatchRule{"value", "", "", "x", Argument{"header", "c", ""}}},
 				},
 			},
 			{
 				Or: &OrRule{
-					{Match: &MatchRule{"value", "", "", "w", Argument{"header", "d"}}},
-					{Match: &MatchRule{"value", "", "", "v", Argument{"header", "e"}}},
+					{Match: &MatchRule{"value", "", "", "w", Argument{"header", "d", ""}}},
+					{Match: &MatchRule{"value", "", "", "v", Argument{"header", "e", ""}}},
 				},
 			},
 			{
 				Not: &NotRule{
-					Match: &MatchRule{"value", "", "", "u", Argument{"header", "f"}},
+					Match: &MatchRule{"value", "", "", "u", Argument{"header", "f", ""}},
 				},
 			},
 		},
@@ -260,7 +326,7 @@ var andRuleTests = []struct {
 	// failures
 	{
 		"invalid rule",
-		AndRule{{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a"}}}},
+		AndRule{{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a", ""}}}},
 		&map[string]interface{}{"y": "z"}, nil, nil, nil,
 		false, false,
 	},
@@ -286,8 +352,8 @@ var orRuleTests = []struct {
 	{
 		"(a=z, b=X): a=z || b=y",
 		OrRule{
-			{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}},
-			{Match: &MatchRule{"value", "", "", "y", Argument{"header", "b"}}},
+			{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}},
+			{Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", ""}}},
 		},
 		&map[string]interface{}{"a": "z", "b": "X"}, nil, nil, []byte{},
 		true, false,
@@ -295,8 +361,8 @@ var orRuleTests = []struct {
 	{
 		"(a=X, b=y): a=z || b=y",
 		OrRule{
-			{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}},
-			{Match: &MatchRule{"value", "", "", "y", Argument{"header", "b"}}},
+			{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}},
+			{Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", ""}}},
 		},
 		&map[string]interface{}{"a": "X", "b": "y"}, nil, nil, []byte{},
 		true, false,
@@ -304,8 +370,8 @@ var orRuleTests = []struct {
 	{
 		"(a=Z, b=Y): a=z || b=y",
 		OrRule{
-			{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}},
-			{Match: &MatchRule{"value", "", "", "y", Argument{"header", "b"}}},
+			{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}},
+			{Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", ""}}},
 		},
 		&map[string]interface{}{"a": "Z", "b": "Y"}, nil, nil, []byte{},
 		false, false,
@@ -314,7 +380,7 @@ var orRuleTests = []struct {
 	{
 		"invalid rule",
 		OrRule{
-			{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}},
+			{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}},
 		},
 		&map[string]interface{}{"y": "Z"}, nil, nil, []byte{},
 		false, false,
@@ -338,8 +404,8 @@ var notRuleTests = []struct {
 	ok                      bool
 	err                     bool
 }{
-	{"(a=z): !a=X", NotRule{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a"}}}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, true, false},
-	{"(a=z): !a=z", NotRule{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, false},
+	{"(a=z): !a=X", NotRule{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a", ""}}}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, true, false},
+	{"(a=z): !a=z", NotRule{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, false},
 }
 
 func TestNotRule(t *testing.T) {

test/hooks.json.tmpl

@@ -57,7 +57,7 @@
     "id": "bitbucket",
     "execute-command": "{{ .Hookecho }}",
     "command-working-directory": "/",
-    "include-command-output-in-response": true,
+    "include-command-output-in-response": false,
     "response-message": "success",
     "parse-parameters-as-json": [
       {
@@ -136,4 +136,3 @@
     }
   }
 ]
-

webhook.go

@@ -21,11 +21,11 @@ import (
 )
 
 const (
-	version = "2.3.8"
+	version = "2.5.0"
 )
 
 var (
-	ip             = flag.String("ip", "", "ip the webhook should serve hooks on")
+	ip             = flag.String("ip", "0.0.0.0", "ip the webhook should serve hooks on")
 	port           = flag.Int("port", 9000, "port the webhook should serve hooks on")
 	verbose        = flag.Bool("verbose", false, "show verbose output")
 	noPanic        = flag.Bool("nopanic", false, "do not panic if hooks cannot be loaded when webhook is not running in verbose mode")
@@ -76,10 +76,16 @@ func main() {
 
 		log.Printf("couldn't load hooks from file! %+v\n", err)
 	} else {
-		log.Printf("loaded %d hook(s) from file\n", len(hooks))
+		seenHooksIds := make(map[string]bool)
+
+		log.Printf("found %d hook(s) in file\n", len(hooks))
 
 		for _, hook := range hooks {
-			log.Printf("\t> %s\n", hook.ID)
+			if seenHooksIds[hook.ID] == true {
+				log.Fatalf("error: hook with the id %s has already been loaded!\nplease check your hooks file for duplicate hooks ids!\n", hook.ID)
+			}
+			seenHooksIds[hook.ID] = true
+			log.Printf("\tloaded: %s\n", hook.ID)
 		}
 	}
 
@@ -105,10 +111,10 @@ func main() {
 	}
 
 	l := negroni.NewLogger()
-	l.Logger = log.New(os.Stderr, "[webhook] ", log.Ldate|log.Ltime)
+	l.ALogger = log.New(os.Stderr, "[webhook] ", log.Ldate|log.Ltime)
 
 	negroniRecovery := &negroni.Recovery{
-		Logger:     l.Logger,
+		Logger:     l.ALogger,
 		PrintStack: true,
 		StackAll:   false,
 		StackSize:  1024 * 8,
@@ -131,10 +137,10 @@ func main() {
 	n.UseHandler(router)
 
 	if *secure {
-		log.Printf("starting secure (https) webhook on %s:%d", *ip, *port)
+		log.Printf("serving hooks on https://%s:%d%s", *ip, *port, hooksURL)
 		log.Fatal(http.ListenAndServeTLS(fmt.Sprintf("%s:%d", *ip, *port), *cert, *key, n))
 	} else {
-		log.Printf("starting insecure (http) webhook on %s:%d", *ip, *port)
+		log.Printf("serving hooks on http://%s:%d%s", *ip, *port, hooksURL)
 		log.Fatal(http.ListenAndServe(fmt.Sprintf("%s:%d", *ip, *port), n))
 	}
 
@@ -147,10 +153,8 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
 
 	id := mux.Vars(r)["id"]
 
-	matchedHooks := hooks.MatchAll(id)
-
-	if matchedHooks != nil {
-		log.Printf("%s got matched (%d time(s))\n", id, len(matchedHooks))
+	if matchedHook := hooks.Match(id); matchedHook != nil {
+		log.Printf("%s got matched\n", id)
 
 		body, err := ioutil.ReadAll(r.Body)
 		if err != nil {
@@ -187,53 +191,55 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
 		}
 
 		// handle hook
-		for _, h := range matchedHooks {
-
-			err := h.ParseJSONParameters(&headers, &query, &payload)
-			if err != nil {
-				msg := fmt.Sprintf("error parsing JSON: %s", err)
+		if err = matchedHook.ParseJSONParameters(&headers, &query, &payload); err != nil {
+			msg := fmt.Sprintf("error parsing JSON parameters: %s", err)
 			log.Printf(msg)
 			w.WriteHeader(http.StatusBadRequest)
-				fmt.Fprintf(w, msg)
+			fmt.Fprintf(w, "Unable to parse JSON parameters.")
 			return
 		}
 
 		var ok bool
 
-			if h.TriggerRule == nil {
+		if matchedHook.TriggerRule == nil {
 			ok = true
 		} else {
-				ok, err = h.TriggerRule.Evaluate(&headers, &query, &payload, &body)
+			ok, err = matchedHook.TriggerRule.Evaluate(&headers, &query, &payload, &body)
 			if err != nil {
 				msg := fmt.Sprintf("error evaluating hook: %s", err)
 				log.Printf(msg)
 				w.WriteHeader(http.StatusInternalServerError)
-					fmt.Fprintf(w, msg)
+				fmt.Fprintf(w, "Error occurred while evaluating hook rules.")
 				return
 			}
 		}
 
 		if ok {
-				log.Printf("%s hook triggered successfully\n", h.ID)
+			log.Printf("%s hook triggered successfully\n", matchedHook.ID)
 
-				for _, responseHeader := range h.ResponseHeaders {
+			for _, responseHeader := range matchedHook.ResponseHeaders {
 				w.Header().Set(responseHeader.Name, responseHeader.Value)
 			}
 
-				if h.CaptureCommandOutput {
-					response := handleHook(h, &headers, &query, &payload, &body)
-					fmt.Fprintf(w, response)
-				} else {
-					go handleHook(h, &headers, &query, &payload, &body)
-					fmt.Fprintf(w, h.ResponseMessage)
-				}
+			if matchedHook.CaptureCommandOutput {
+				response, err := handleHook(matchedHook, &headers, &query, &payload, &body)
 
-				return
+				if err != nil {
+					w.Header().Set("Content-Type", "text/plain; charset=utf-8")
+					w.WriteHeader(http.StatusInternalServerError)
+					fmt.Fprintf(w, "Error occurred while executing the hook's command. Please check your logs for more details.")
+				} else {
+					fmt.Fprintf(w, response)
 				}
+			} else {
+				go handleHook(matchedHook, &headers, &query, &payload, &body)
+				fmt.Fprintf(w, matchedHook.ResponseMessage)
+			}
+			return
 		}
 
 		// if none of the hooks got triggered
-		log.Printf("%s got matched (%d time(s)), but didn't get triggered because the trigger rules were not satisfied\n", matchedHooks[0].ID, len(matchedHooks))
+		log.Printf("%s got matched, but didn't get triggered because the trigger rules were not satisfied\n", matchedHook.ID)
 
 		fmt.Fprintf(w, "Hook rules were not satisfied.")
 	} else {
@@ -242,7 +248,7 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-func handleHook(h *hook.Hook, headers, query, payload *map[string]interface{}, body *[]byte) string {
+func handleHook(h *hook.Hook, headers, query, payload *map[string]interface{}, body *[]byte) (string, error) {
 	var err error
 
 	cmd := exec.Command(h.ExecuteCommand)
@@ -262,28 +268,17 @@ func handleHook(h *hook.Hook, headers, query, payload *map[string]interface{}, b
 
 	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()
+	out, err := cmd.Output()
 
 	log.Printf("command output: %s\n", out)
 
-	var errorResponse string
-
 	if err != nil {
 		log.Printf("error occurred: %+v\n", err)
-		errorResponse = fmt.Sprintf("%+v", err)
 	}
 
 	log.Printf("finished handling %s\n", h.ID)
 
-	var response []byte
-	response, err = json.Marshal(&hook.CommandStatusResponse{ResponseMessage: h.ResponseMessage, Output: string(out), Error: errorResponse})
-
-	if err != nil {
-		log.Printf("error marshalling response: %+v", err)
-		return h.ResponseMessage
-	}
-
-	return string(response)
+	return string(out), err
 }
 
 func reloadHooks() {
@@ -297,10 +292,18 @@ func reloadHooks() {
 	if err != nil {
 		log.Printf("couldn't load hooks from file! %+v\n", err)
 	} else {
-		log.Printf("loaded %d hook(s) from file\n", len(hooks))
+		seenHooksIds := make(map[string]bool)
 
-		for _, hook := range hooks {
-			log.Printf("\t> %s\n", hook.ID)
+		log.Printf("found %d hook(s) in file\n", len(newHooks))
+
+		for _, hook := range newHooks {
+			if seenHooksIds[hook.ID] == true {
+				log.Printf("error: hook with the id %s has already been loaded!\nplease check your hooks file for duplicate hooks ids!", hook.ID)
+				log.Println("reverting hooks back to the previous configuration")
+				return
+			}
+			seenHooksIds[hook.ID] = true
+			log.Printf("\tloaded: %s\n", hook.ID)
 		}
 
 		hooks = newHooks

webhook_test.go

@@ -372,7 +372,9 @@ var hookHandlerTests = []struct {
 		}`,
 		false,
 		http.StatusOK,
-		`{"output":"arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz\nenv: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00\n"}`,
+		`arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz
+env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
+`,
 	},
 	{
 		"bitbucket", // bitbucket sends their payload using uriencoded params.
@@ -381,7 +383,7 @@ var hookHandlerTests = []struct {
 		`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 <marcus@somedomain.com>","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"}`,
 		true,
 		http.StatusOK,
-		`{"message":"success"}`,
+		`success`,
 	},
 	{
 		"gitlab",
@@ -431,7 +433,8 @@ var hookHandlerTests = []struct {
 		}`,
 		false,
 		http.StatusOK,
-		`{"message":"success","output":"arg: b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327 John Smith john@example.com\n"}`,
+		`arg: b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327 John Smith john@example.com
+`,
 	},
 
 	{
@@ -469,7 +472,9 @@ var hookHandlerTests = []struct {
 		}`,
 		false,
 		http.StatusOK,
-		`{"output":"arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz\nenv: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00\n"}`,
+		`arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz
+env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
+`,
 	},
 
 	{
@@ -506,7 +511,8 @@ var hookHandlerTests = []struct {
 		}`,
 		false,
 		http.StatusOK,
-		`{"output":"arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz\n"}`,
+		`arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz
+`,
 	},
 
 	{"empty payload", "github", nil, `{}`, false, http.StatusOK, `Hook rules were not satisfied.`},