Merge pull request #101 from adnanh/development

webhook 2.5.0

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
 }
@@ -367,7 +367,7 @@ func (h *Hook) ExtractCommandArgumentsForEnv(headers, query, payload *map[string
 		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, EnvNamespace+h.PassEnvironmentToCommand[i].EnvName+"="+arg)
+				args = append(args, h.PassEnvironmentToCommand[i].EnvName+"="+arg)
 			} else {
 				// then fallback on the name
 				args = append(args, EnvNamespace+h.PassEnvironmentToCommand[i].Name+"="+arg)
@@ -412,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"`
@@ -538,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)
+		}
 	}
 }
 
@@ -174,7 +179,7 @@ var hookExtractCommandArgumentsForEnvTests = []struct {
 		"test",
 		[]Argument{Argument{"header", "a", "MYKEY"}},
 		&map[string]interface{}{"a": "z"}, nil, nil,
-		[]string{"HOOK_MYKEY=z"},
+		[]string{"MYKEY=z"},
 		true,
 	},
 	// failures

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.9"
+	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.`},