Merge pull request #3 from adnanh/development

Merging latest updates
2018-02-26 17:03:15 -08:00 · 2018-02-26 17:03:15 -08:00 · d2c35576c0
commit d2c35576c0
parent b595694658 ae54669c02
11 changed files with 410 additions and 255 deletions
--- a/.travis.yml
+++ b/.travis.yml
@ -0,0 +1,21 @@
+language: go
+
+go:
+  - 1.8.x
+  - 1.9.x
+  - tip
+
+os:
+  - linux
+  - osx
+
+matrix:
+  fast_finish: true
+  allow_failures:
+    - go: tip
+
+install:
+  - go get -d -v -t ./...
+
+script:
+  - go test -v -race ./...
--- a/appveyor.yml
+++ b/appveyor.yml
@ -0,0 +1,15 @@
+clone_folder: C:\gopath\src\github.com\adnanh\webhook
+
+environment:
+  GOPATH: c:\gopath
+
+install:
+  - set PATH=%PATH%;C:\MinGW\bin;%GOPATH%\bin
+  - echo %PATH%
+  - echo %GOPATH%
+  - go version
+  - go env
+  - go get -d -v -t ./...
+
+build_script:
+  - go test -v -race ./...
--- a/docs/Hook-Examples.md
+++ b/docs/Hook-Examples.md
@ -281,3 +281,44 @@ or in a single line, using https://github.com/jpmens/jo to generate the JSON cod
 jo binary=%filename.zip | curl -H "Content-Type:application/json" -X POST -d @- \
 http://localhost:9000/hooks/test-file-webhook
 </pre>
+
+
+## Incoming Scalr Webhook
+[Guide by @hassanbabaie]
+Scalr makes webhook calls based on an event to a configured webhook endpoint (for example Host Down, Host Up). Webhook endpoints are URLs where Scalr will deliver Webhook notifications.  
+Scalr assigns a unique signing key for every configured webhook endpoint.
+Refer to this URL for information on how to setup the webhook call on the Scalr side: [Scalr Wiki Webhooks](https://scalr-wiki.atlassian.net/wiki/spaces/docs/pages/6193173/Webhooks)
+In order to leverage the Signing Key for addtional authentication/security you must configure the trigger rule with a match type of "scalr-signature".
+
+```json
+[
+    {
+        "id": "redeploy-webhook",
+        "execute-command": "/home/adnan/redeploy-go-webhook.sh",
+        "command-working-directory": "/home/adnan/go",
+        "include-command-output-in-response": true,
+        "trigger-rule": 
+		{
+            "match": 
+			{
+                "type": "scalr-signature",
+                "secret": "Scalr-provided signing key"
+            }
+        },
+        "pass-environment-to-command": 
+		[
+            {
+                "envname": "EVENT_NAME",
+                "source": "payload",
+                "name": "eventName"
+            },
+            {
+                "envname": "SERVER_HOSTNAME",
+                "source": "payload",
+                "name": "data.SCALR_SERVER_HOSTNAME"
+            }
+        ]
+    }
+]
+
+```
--- a/docs/Hook-Rules.md
+++ b/docs/Hook-Rules.md
@ -199,3 +199,19 @@ The IP can be IPv4- or IPv6-formatted, using [CIDR notation](https://en.wikipedi
  }
 }
 ```
+
+### 5. Match scalr-signature
+
+The trigger rule checks the scalr signature and also checks that the request was signed less than 5 minutes before it was received. 
+A unqiue signing key is generated for each webhook endpoint URL you register in Scalr.
+Given the time check make sure that NTP is enabled on both your Scalr and webhook server to prevent any issues
+
+```json
+{
+  "match":
+  {
+    "type": "scalr-signature",
+    "secret": "Scalr-provided signing key"
+  }
+}
+```
--- a/hook/hook.go
+++ b/hook/hook.go
@ -11,8 +11,8 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
-	"math"
 	"log"
+	"math"
 	"net"
 	"net/textproto"
 	"os"
@ -94,9 +94,7 @@ func (e *ParseError) Error() string {

 // CheckPayloadSignature calculates and verifies SHA1 signature of the given payload
 func CheckPayloadSignature(payload []byte, secret string, signature string) (string, error) {
-	if strings.HasPrefix(signature, "sha1=") {
-		signature = signature[5:]
-	}
+	signature = strings.TrimPrefix(signature, "sha1=")

 	mac := hmac.New(sha1.New, []byte(secret))
 	_, err := mac.Write(payload)
@ -113,9 +111,7 @@ func CheckPayloadSignature(payload []byte, secret string, signature string) (str

 // CheckPayloadSignature256 calculates and verifies SHA256 signature of the given payload
 func CheckPayloadSignature256(payload []byte, secret string, signature string) (string, error) {
-	if strings.HasPrefix(signature, "sha256=") {
-		signature = signature[7:]
-	}
+	signature = strings.TrimPrefix(signature, "sha256=")

 	mac := hmac.New(sha256.New, []byte(secret))
 	_, err := mac.Write(payload)
@ -196,7 +192,7 @@ func CheckIPWhitelist(remoteAddr string, ipRange string) (bool, error) {

 	ipRange = strings.TrimSpace(ipRange)

-	if strings.Index(ipRange, "/") == -1 {
+	if !strings.Contains(ipRange, "/") {
 		ipRange = ipRange + "/32"
 	}

@ -687,7 +683,7 @@ func (r AndRule) Evaluate(headers, query, payload *map[string]interface{}, body
 		}

 		res = res && rv
-		if res == false {
+		if !res {
 			return res, nil
 		}
 	}
@ -709,7 +705,7 @@ func (r OrRule) Evaluate(headers, query, payload *map[string]interface{}, body *
 		}

 		res = res || rv
-		if res == true {
+		if res {
 			return res, nil
 		}
 	}
--- a/test/hookecho.go
+++ b/test/hookecho.go
@ -5,8 +5,8 @@ package main
 import (
 	"fmt"
 	"os"
-	"strings"
 	"strconv"
+	"strings"
 )

 func main() {
--- a/test/hooks.json.tmpl
+++ b/test/hooks.json.tmpl
@ -180,5 +180,29 @@
    "execute-command": "{{ .Hookecho }}",
    "include-command-output-in-response": true,
    "include-command-output-in-response-on-error": true
+  },
+  {
+    "id": "static-params-ok",
+		"execute-command": "{{ .Hookecho }}",
+		"response-message": "success",
+    "include-command-output-in-response": true,
+		"pass-arguments-to-command": [
+      {
+        "source": "string",
+        "name": "passed"
+      }
+    ],
+  },
+  {
+    "id": "warn-on-space",
+		"execute-command": "{{ .Hookecho }} foo",
+		"response-message": "success",
+    "include-command-output-in-response": true,
+		"pass-arguments-to-command": [
+      {
+        "source": "string",
+        "name": "passed"
+      }
+    ],
  }
 ]
--- a/test/hooks.yaml.tmpl
+++ b/test/hooks.yaml.tmpl
@ -1,4 +1,5 @@
- trigger-rule:
+- id: github
+  trigger-rule:
    and:
    - match:
        parameter:
@ -23,9 +24,10 @@
  pass-environment-to-command:
  - source: payload
    name: head_commit.timestamp
-  id: github
  command-working-directory: /
- trigger-rule:
+
+- id: bitbucket
+  trigger-rule:
    and:
    - match:
        parameter:
@ -52,9 +54,10 @@
  execute-command: '{{ .Hookecho }}'
  response-message: success
  include-command-output-in-response: false
-  id: bitbucket
  command-working-directory: /
- trigger-rule:
+
+- id: gitlab
+  trigger-rule:
    match:
      parameter:
        source: payload
@ -71,25 +74,28 @@
  execute-command: '{{ .Hookecho }}'
  response-message: success
  include-command-output-in-response: true
-  id: gitlab
  command-working-directory: /
+
 - id: capture-command-output-on-success-not-by-default
  pass-arguments-to-command:
  - source: string
    name: exit=0
  execute-command: '{{ .Hookecho }}'
+
 - id: capture-command-output-on-success-yes-with-flag
  pass-arguments-to-command:
  - source: string
    name: exit=0
  execute-command: '{{ .Hookecho }}'
  include-command-output-in-response: true
+
 - id: capture-command-output-on-error-not-by-default
  pass-arguments-to-command:
  - source: string
    name: exit=1
  execute-command: '{{ .Hookecho }}'
  include-command-output-in-response: true
+
 - id: capture-command-output-on-error-yes-with-extra-flag
  pass-arguments-to-command:
  - source: string
@ -97,3 +103,14 @@
  execute-command: '{{ .Hookecho }}'
  include-command-output-in-response: true
  include-command-output-in-response-on-error: true
+
+- id: static-params-ok
+  execute-command: '{{ .Hookecho }}'
+  include-command-output-in-response: true
+  pass-arguments-to-command:
+  - source: string
+    name: passed
+
+- id: warn-on-space
+  execute-command: '{{ .Hookecho }} foo'
+  include-command-output-in-response: true
--- a/webhook.go
+++ b/webhook.go
@ -23,7 +23,7 @@ import (
 )

 const (
-	version = "2.6.7"
+	version = "2.6.8"
 )

 var (
@ -120,7 +120,7 @@ func main() {

 	newHooksFiles := hooksFiles[:0]
 	for _, filePath := range hooksFiles {
-		if _, ok := loadedHooksFromFiles[filePath]; ok == true {
+		if _, ok := loadedHooksFromFiles[filePath]; ok {
 			newHooksFiles = append(newHooksFiles, filePath)
 		}
 	}
@ -250,11 +250,10 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
 		}

 		// handle hook
-		if errors := matchedHook.ParseJSONParameters(&headers, &query, &payload); errors != nil {
+		errors := matchedHook.ParseJSONParameters(&headers, &query, &payload)
 		for _, err := range errors {
 			log.Printf("[%s] error parsing JSON parameters: %s\n", rid, err)
 		}
-		}

 		var ok bool

@ -264,7 +263,7 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
 			ok, err = matchedHook.TriggerRule.Evaluate(&headers, &query, &payload, &body, r.RemoteAddr)
 			if err != nil {
 				msg := fmt.Sprintf("[%s] error evaluating hook: %s", rid, err)
-				log.Printf(msg)
+				log.Print(msg)
 				w.WriteHeader(http.StatusInternalServerError)
 				fmt.Fprintf(w, "Error occurred while evaluating hook rules.")
 				return
@ -341,40 +340,37 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in
 	cmd.Dir = h.CommandWorkingDirectory

 	cmd.Args, errors = h.ExtractCommandArguments(headers, query, payload)
-	if errors != nil {
 	for _, err := range errors {
 		log.Printf("[%s] error extracting command arguments: %s\n", rid, err)
 	}
-	}

 	var envs []string
 	envs, errors = h.ExtractCommandArgumentsForEnv(headers, query, payload)

-	if errors != nil {
 	for _, err := range errors {
 		log.Printf("[%s] error extracting command arguments for environment: %s\n", rid, err)
 	}
-	}

 	files, errors := h.ExtractCommandArgumentsForFile(headers, query, payload)

-	if errors != nil {
 	for _, err := range errors {
 		log.Printf("[%s] error extracting command arguments for file: %s\n", rid, err)
 	}
-	}

 	for i := range files {
 		tmpfile, err := ioutil.TempFile(h.CommandWorkingDirectory, files[i].EnvName)
 		if err != nil {
 			log.Printf("[%s] error creating temp file [%s]", rid, err)
+			continue
 		}
 		log.Printf("[%s] writing env %s file %s", rid, files[i].EnvName, tmpfile.Name())
 		if _, err := tmpfile.Write(files[i].Data); err != nil {
 			log.Printf("[%s] error writing file %s [%s]", rid, tmpfile.Name(), err)
+			continue
 		}
 		if err := tmpfile.Close(); err != nil {
 			log.Printf("[%s] error closing file %s [%s]", rid, tmpfile.Name(), err)
+			continue
 		}

 		files[i].File = tmpfile
@ -394,12 +390,14 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in
 	}

 	for i := range files {
+		if files[i].File != nil {
 			log.Printf("[%s] removing file %s\n", rid, files[i].File.Name())
 			err := os.Remove(files[i].File.Name())
 			if err != nil {
 				log.Printf("[%s] error removing file %s [%s]", rid, files[i].File.Name(), err)
 			}
 		}
+	}

 	log.Printf("[%s] finished handling %s\n", rid, h.ID)

@ -431,7 +429,7 @@ func reloadHooks(hooksFilePath string) {
 				}
 			}

-			if (matchLoadedHook(hook.ID) != nil && !wasHookIDAlreadyLoaded) || seenHooksIds[hook.ID] == true {
+			if (matchLoadedHook(hook.ID) != nil && !wasHookIDAlreadyLoaded) || seenHooksIds[hook.ID] {
 				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
--- a/webhook_test.go
+++ b/webhook_test.go
@ -13,6 +13,7 @@ import (
 	"regexp"
 	"runtime"
 	"strings"
+	"sync"
 	"testing"
 	"text/template"
 	"time"
@ -21,15 +22,26 @@ import (
 )

 func TestStaticParams(t *testing.T) {
+	// FIXME(moorereason): incorporate this test into TestWebhook.
+	//   Need to be able to execute a binary with a space in the filename.
+	if runtime.GOOS == "windows" {
+		t.Skip("Skipping on Windows")
+	}

 	spHeaders := make(map[string]interface{})
 	spHeaders["User-Agent"] = "curl/7.54.0"
 	spHeaders["Accept"] = "*/*"

-	// case 1: correct entry
+	// case 2: binary with spaces in its name
+	err := os.Symlink("/bin/echo", "/tmp/with space")
+	if err != nil {
+		t.Fatalf("%v", err)
+	}
+	defer os.Remove("/tmp/with space")
+
 	spHook := &hook.Hook{
-		ID:                      "static-params-ok",
-		ExecuteCommand:          "/bin/echo",
+		ID:                      "static-params-name-space",
+		ExecuteCommand:          "/tmp/with space",
 		CommandWorkingDirectory: "/tmp",
 		ResponseMessage:         "success",
 		CaptureCommandOutput:    true,
@ -41,86 +53,38 @@ func TestStaticParams(t *testing.T) {
 	b := &bytes.Buffer{}
 	log.SetOutput(b)

-	s, err := handleHook(spHook, "test", &spHeaders, &map[string]interface{}{}, &map[string]interface{}{}, &[]byte{})
+	_, err = handleHook(spHook, "test", &spHeaders, &map[string]interface{}{}, &map[string]interface{}{}, &[]byte{})
 	if err != nil {
 		t.Fatalf("Unexpected error: %v\n", err)
 	}
-	matched, _ := regexp.MatchString("(?s).*command output: passed.*static-params-ok", b.String())
-	if !matched {
-		t.Fatalf("Unexpected log output:\n%s", b)
-	}
-
-	// case 2: binary with spaces in its name
-	err = os.Symlink("/bin/echo", "/tmp/with space")
-	if err != nil {
-		t.Fatalf("%v", err)
-	}
-	defer os.Remove("/tmp/with space")
-
-	spHook = &hook.Hook{
-		ID:                      "static-params-name-space",
-		ExecuteCommand:          "/tmp/with space",
-		CommandWorkingDirectory: "/tmp",
-		ResponseMessage:         "success",
-		CaptureCommandOutput:    true,
-		PassArgumentsToCommand: []hook.Argument{
-			hook.Argument{Source: "string", Name: "passed"},
-		},
-	}
-
-	b = &bytes.Buffer{}
-	log.SetOutput(b)
-
-	s, err = handleHook(spHook, "test", &spHeaders, &map[string]interface{}{}, &map[string]interface{}{}, &[]byte{})
-	if err != nil {
-		t.Fatalf("Unexpected error: %v\n", err)
-	}
-	matched, _ = regexp.MatchString("(?s)command output: .*static-params-name-space", b.String())
+	matched, _ := regexp.MatchString("(?s)command output: .*static-params-name-space", b.String())
 	if !matched {
 		t.Fatalf("Unexpected log output:\n%sn", b)
 	}
-
-	// case 3: parameters specified in execute-command
-	spHook = &hook.Hook{
-		ID:                      "static-params-bad",
-		ExecuteCommand:          "/bin/echo success",
-		CommandWorkingDirectory: "/tmp",
-		ResponseMessage:         "success",
-		CaptureCommandOutput:    true,
-		PassArgumentsToCommand: []hook.Argument{
-			hook.Argument{Source: "string", Name: "failed"},
-		},
-	}
-
-	b = &bytes.Buffer{}
-	log.SetOutput(b)
-
-	s, err = handleHook(spHook, "test", &spHeaders, &map[string]interface{}{}, &map[string]interface{}{}, &[]byte{})
-	if err == nil {
-		t.Fatalf("Error expected, but none returned: %s\n", s)
-	}
-	matched, _ = regexp.MatchString("(?s)unable to locate command: ..bin.echo success.*use 'pass-arguments-to-command'", b.String())
-	if !matched {
-		t.Fatalf("Unexpected log output:\n%s\n", b)
-	}
 }

 func TestWebhook(t *testing.T) {
 	hookecho, cleanupHookecho := buildHookecho(t)
 	defer cleanupHookecho()

-	for _, hookTmpl := range []string{"test/hooks.json.tmpl", "test/hooks.yaml.tmpl"} {
-		config, cleanupConfig := genConfig(t, hookecho, hookTmpl)
-		defer cleanupConfig()
+	webhook, cleanupWebhookFn := buildWebhook(t)
+	defer cleanupWebhookFn()

-		webhook, cleanupWebhook := buildWebhook(t)
-		defer cleanupWebhook()
+	for _, hookTmpl := range []string{"test/hooks.json.tmpl", "test/hooks.yaml.tmpl"} {
+		configPath, cleanupConfigFn := genConfig(t, hookecho, hookTmpl)
+		defer cleanupConfigFn()
+
+		for _, tt := range hookHandlerTests {
+			t.Run(tt.desc, func(t *testing.T) {

 				ip, port := serverAddress(t)
-		args := []string{fmt.Sprintf("-hooks=%s", config), fmt.Sprintf("-ip=%s", ip), fmt.Sprintf("-port=%s", port), "-verbose"}
+				args := []string{fmt.Sprintf("-hooks=%s", configPath), fmt.Sprintf("-ip=%s", ip), fmt.Sprintf("-port=%s", port), "-verbose"}
+
+				// Setup a buffer for capturing webhook logs for later evaluation
+				b := &buffer{}

 				cmd := exec.Command(webhook, args...)
-		//cmd.Stderr = os.Stderr // uncomment to see verbose output
+				cmd.Stderr = b
 				cmd.Env = webhookEnv()
 				cmd.Args[0] = "webhook"
 				if err := cmd.Start(); err != nil {
@ -130,7 +94,6 @@ func TestWebhook(t *testing.T) {

 				waitForServerReady(t, ip, port)

-		for _, tt := range hookHandlerTests {
 				url := fmt.Sprintf("http://%s:%s/hooks/%s", ip, port, tt.id)

 				req, err := http.NewRequest("POST", url, ioutil.NopCloser(strings.NewReader(tt.body)))
@ -165,35 +128,51 @@ func TestWebhook(t *testing.T) {
 				if res.StatusCode != tt.respStatus || string(body) != tt.respBody {
 					t.Errorf("failed %q (id: %s):\nexpected status: %#v, response: %s\ngot status: %#v, response: %s", tt.desc, tt.id, tt.respStatus, tt.respBody, res.StatusCode, body)
 				}
+
+				if tt.logMatch == "" {
+					return
+				}
+
+				// There's the potential for a race condition below where we
+				// try to read the logs buffer b before the logs have been
+				// flushed by the webhook process. Kill the process to flush
+				// the logs.
+				killAndWait(cmd)
+
+				matched, _ := regexp.MatchString(tt.logMatch, b.String())
+				if !matched {
+					t.Errorf("failed log match for %q (id: %s):\nmatch pattern: %q\ngot:\n%s", tt.desc, tt.id, tt.logMatch, b)
+				}
+			})
 		}
 	}
 }

-func buildHookecho(t *testing.T) (bin string, cleanup func()) {
+func buildHookecho(t *testing.T) (binPath string, cleanupFn func()) {
 	tmp, err := ioutil.TempDir("", "hookecho-test-")
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer func() {
-		if cleanup == nil {
+		if cleanupFn == nil {
 			os.RemoveAll(tmp)
 		}
 	}()

-	bin = filepath.Join(tmp, "hookecho")
+	binPath = filepath.Join(tmp, "hookecho")
 	if runtime.GOOS == "windows" {
-		bin += ".exe"
+		binPath += ".exe"
 	}

-	cmd := exec.Command("go", "build", "-o", bin, "test/hookecho.go")
+	cmd := exec.Command("go", "build", "-o", binPath, "test/hookecho.go")
 	if err := cmd.Run(); err != nil {
 		t.Fatalf("Building hookecho: %v", err)
 	}

-	return bin, func() { os.RemoveAll(tmp) }
+	return binPath, func() { os.RemoveAll(tmp) }
 }

-func genConfig(t *testing.T, bin string, hookTemplate string) (config string, cleanup func()) {
+func genConfig(t *testing.T, bin string, hookTemplate string) (configPath string, cleanupFn func()) {
 	tmpl := template.Must(template.ParseFiles(hookTemplate))

 	tmp, err := ioutil.TempDir("", "webhook-config-")
@ -201,7 +180,7 @@ func genConfig(t *testing.T, bin string, hookTemplate string) (config string, cl
 		t.Fatal(err)
 	}
 	defer func() {
-		if cleanup == nil {
+		if cleanupFn == nil {
 			os.RemoveAll(tmp)
 		}
 	}()
@ -215,7 +194,11 @@ func genConfig(t *testing.T, bin string, hookTemplate string) (config string, cl
 	}
 	defer file.Close()

-	data := struct{ Hookecho string }{filepath.ToSlash(bin)}
+	data := struct{ Hookecho string }{filepath.FromSlash(bin)}
+	if runtime.GOOS == "windows" {
+		// Simulate escaped backslashes on Windows.
+		data.Hookecho = strings.Replace(data.Hookecho, `\`, `\\`, -1)
+	}
 	if err := tmpl.Execute(file, data); err != nil {
 		t.Fatalf("Executing template: %v", err)
 	}
@ -223,28 +206,28 @@ func genConfig(t *testing.T, bin string, hookTemplate string) (config string, cl
 	return path, func() { os.RemoveAll(tmp) }
 }

-func buildWebhook(t *testing.T) (bin string, cleanup func()) {
+func buildWebhook(t *testing.T) (binPath string, cleanupFn func()) {
 	tmp, err := ioutil.TempDir("", "webhook-test-")
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer func() {
-		if cleanup == nil {
+		if cleanupFn == nil {
 			os.RemoveAll(tmp)
 		}
 	}()

-	bin = filepath.Join(tmp, "webhook")
+	binPath = filepath.Join(tmp, "webhook")
 	if runtime.GOOS == "windows" {
-		bin += ".exe"
+		binPath += ".exe"
 	}

-	cmd := exec.Command("go", "build", "-o", bin)
+	cmd := exec.Command("go", "build", "-o", binPath)
 	if err := cmd.Run(); err != nil {
 		t.Fatalf("Building webhook: %v", err)
 	}

-	return bin, func() { os.RemoveAll(tmp) }
+	return binPath, func() { os.RemoveAll(tmp) }
 }

 func serverAddress(t *testing.T) (string, string) {
@ -288,6 +271,10 @@ func waitForServer(t *testing.T, url string, status int, timeout time.Duration)
 }

 func killAndWait(cmd *exec.Cmd) {
+	if cmd == nil || cmd.ProcessState != nil && cmd.ProcessState.Exited() {
+		return
+	}
+
 	cmd.Process.Kill()
 	cmd.Wait()
 }
@ -313,6 +300,7 @@ var hookHandlerTests = []struct {

 	respStatus int
 	respBody   string
+	logMatch   string
 }{
 	{
 		"github",
@ -467,6 +455,7 @@ var hookHandlerTests = []struct {
 		`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.
@ -476,6 +465,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 		true,
 		http.StatusOK,
 		`success`,
+		``,
 	},
 	{
 		"gitlab",
@ -527,6 +517,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 		http.StatusOK,
 		`arg: b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327 John Smith john@example.com
 `,
+		``,
 	},

 	{
@ -567,6 +558,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 		`arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz
 env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 `,
+		``,
 	},

 	{
@ -605,20 +597,55 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 		http.StatusOK,
 		`arg: 1481a2de7b2a7d02428ad93446ab166be7793fbb lolwut@noway.biz
 `,
+		``,
 	},

 	// test with custom return code
-	{"empty payload", "github", nil, `{}`, false, http.StatusBadRequest, `Hook rules were not satisfied.`},
+	{"empty payload", "github", nil, `{}`, false, http.StatusBadRequest, `Hook rules were not satisfied.`, ``},
 	// test with custom invalid http code, should default to 200 OK
-	{"empty payload", "bitbucket", nil, `{}`, false, http.StatusOK, `Hook rules were not satisfied.`},
+	{"empty payload", "bitbucket", nil, `{}`, false, http.StatusOK, `Hook rules were not satisfied.`, ``},
 	// test with no configured http return code, should default to 200 OK
-	{"empty payload", "gitlab", nil, `{}`, false, http.StatusOK, `Hook rules were not satisfied.`},
+	{"empty payload", "gitlab", nil, `{}`, false, http.StatusOK, `Hook rules were not satisfied.`, ``},

 	// test capturing command output
-	{"don't capture output on success by default", "capture-command-output-on-success-not-by-default", nil, `{}`, false, http.StatusOK, ``},
+	{"don't capture output on success by default", "capture-command-output-on-success-not-by-default", nil, `{}`, false, http.StatusOK, ``, ``},
 	{"capture output on success with flag set", "capture-command-output-on-success-yes-with-flag", nil, `{}`, false, http.StatusOK, `arg: exit=0
-`},
-	{"don't capture output on error by default", "capture-command-output-on-error-not-by-default", nil, `{}`, false, http.StatusInternalServerError, `Error occurred while executing the hook's command. Please check your logs for more details.`},
+`, ``},
+	{"don't capture output on error by default", "capture-command-output-on-error-not-by-default", nil, `{}`, false, http.StatusInternalServerError, `Error occurred while executing the hook's command. Please check your logs for more details.`, ``},
 	{"capture output on error with extra flag set", "capture-command-output-on-error-yes-with-extra-flag", nil, `{}`, false, http.StatusInternalServerError, `arg: exit=1
-`},
+`, ``},
+
+	// Check logs
+	{"static params should pass", "static-params-ok", nil, `{}`, false, http.StatusOK, "arg: passed\n", `(?s)command output: arg: passed`},
+	{"command with space logs warning", "warn-on-space", nil, `{}`, false, http.StatusInternalServerError, "Error occurred while executing the hook's command. Please check your logs for more details.", `(?s)unable to locate command.*use 'pass[-]arguments[-]to[-]command' to specify args`},
+}
+
+// buffer provides a concurrency-safe bytes.Buffer to tests above.
+type buffer struct {
+	b bytes.Buffer
+	m sync.Mutex
+}
+
+func (b *buffer) Read(p []byte) (n int, err error) {
+	b.m.Lock()
+	defer b.m.Unlock()
+	return b.b.Read(p)
+}
+
+func (b *buffer) Write(p []byte) (n int, err error) {
+	b.m.Lock()
+	defer b.m.Unlock()
+	return b.b.Write(p)
+}
+
+func (b *buffer) String() string {
+	b.m.Lock()
+	defer b.m.Unlock()
+	return b.b.String()
+}
+
+func (b *buffer) Reset() {
+	b.m.Lock()
+	defer b.m.Unlock()
+	b.b.Reset()
 }