Add per-hook HTTP method restrictions

docs/Hook-Definition.md

@@ -10,6 +10,7 @@ Hooks are defined as JSON objects. Please note that in order to be considered va
  * `response-headers` - specifies the list of headers in format `{"name": "X-Example-Header", "value": "it works"}` that will be returned in HTTP response for the hook
  * `success-http-response-code` - specifies the HTTP status code to be returned upon success
  * `incoming-payload-content-type` - sets the `Content-Type` of the incoming HTTP request (ie. `application/json`); useful when the request lacks a `Content-Type` or sends an erroneous value
+ * `http-methods` - a list of allowed HTTP methods, such as `POST` and `GET`
  * `include-command-output-in-response` - boolean whether webhook should wait for the command to finish and return the raw output as a response to the hook initiator. If the command fails to execute or encounters any errors while executing the response will result in 500 Internal Server Error HTTP status code, otherwise the 200 OK status code will be returned.
  * `include-command-output-in-response-on-error` - boolean whether webhook should include command stdout & stderror as a response in failed executions. It only works if `include-command-output-in-response` is set to `true`.
  * `parse-parameters-as-json` - specifies the list of arguments that contain JSON strings. These parameters will be decoded by webhook and you can access them like regular objects in rules and `pass-arguments-to-command`.

internal/hook/hook.go

@@ -460,6 +460,7 @@ type Hook struct {
 	TriggerRuleMismatchHttpResponseCode int             `json:"trigger-rule-mismatch-http-response-code,omitempty"`
 	IncomingPayloadContentType          string          `json:"incoming-payload-content-type,omitempty"`
 	SuccessHttpResponseCode             int             `json:"success-http-response-code,omitempty"`
+	HTTPMethods                         []string        `json:"http-methods"`
 }
 
 // ParseJSONParameters decodes specified arguments to JSON objects and replaces the

test/hooks.json.tmpl

@@ -3,6 +3,7 @@
     "id": "github",
     "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":

test/hooks.yaml.tmpl

@@ -1,4 +1,6 @@
 - id: github
+  http-methods:
+  - POST
   trigger-rule:
     and:
     - match:

webhook.go

@@ -253,218 +253,236 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
 
 	log.Printf("[%s] incoming HTTP request from %s\n", rid, r.RemoteAddr)
 
+	id := mux.Vars(r)["id"]
+
+	matchedHook := matchLoadedHook(id)
+	if matchedHook == nil {
+		w.WriteHeader(http.StatusNotFound)
+		fmt.Fprint(w, "Hook not found.")
+		return
+	}
+
+	// Check for allowed methods
+	if len(matchedHook.HTTPMethods) != 0 {
+		var allowed bool
+		for i := range matchedHook.HTTPMethods {
+			if matchedHook.HTTPMethods[i] == r.Method {
+				allowed = true
+				break
+			}
+		}
+
+		if !allowed {
+			w.WriteHeader(http.StatusMethodNotAllowed)
+			return
+		}
+	}
+
+	log.Printf("[%s] %s got matched\n", rid, id)
+
 	for _, responseHeader := range responseHeaders {
 		w.Header().Set(responseHeader.Name, responseHeader.Value)
 	}
 
-	id := mux.Vars(r)["id"]
+	var (
+		body []byte
+		err  error
+	)
 
-	if matchedHook := matchLoadedHook(id); matchedHook != nil {
+	// set contentType to IncomingPayloadContentType or header value
-		log.Printf("[%s] %s got matched\n", rid, id)
+	contentType := r.Header.Get("Content-Type")
+	if len(matchedHook.IncomingPayloadContentType) != 0 {
+		contentType = matchedHook.IncomingPayloadContentType
+	}
 
-		var (
+	isMultipart := strings.HasPrefix(contentType, "multipart/form-data;")
-			body []byte
-			err  error
-		)
 
-		// set contentType to IncomingPayloadContentType or header value
+	if !isMultipart {
-		contentType := r.Header.Get("Content-Type")
+		body, err = ioutil.ReadAll(r.Body)
-		if len(matchedHook.IncomingPayloadContentType) != 0 {
+		if err != nil {
-			contentType = matchedHook.IncomingPayloadContentType
+			log.Printf("[%s] error reading the request body: %+v\n", rid, err)
+		}
+	}
+
+	// parse headers
+	headers := valuesToMap(r.Header)
+
+	// parse query variables
+	query := valuesToMap(r.URL.Query())
+
+	// parse body
+	var payload map[string]interface{}
+
+	switch {
+	case strings.Contains(contentType, "json"):
+		decoder := json.NewDecoder(bytes.NewReader(body))
+		decoder.UseNumber()
+
+		err := decoder.Decode(&payload)
+		if err != nil {
+			log.Printf("[%s] error parsing JSON payload %+v\n", rid, err)
 		}
 
-		isMultipart := strings.HasPrefix(contentType, "multipart/form-data;")
+	case strings.Contains(contentType, "x-www-form-urlencoded"):
-
+		fd, err := url.ParseQuery(string(body))
-		if !isMultipart {
+		if err != nil {
-			body, err = ioutil.ReadAll(r.Body)
+			log.Printf("[%s] error parsing form payload %+v\n", rid, err)
-			if err != nil {
+		} else {
-				log.Printf("[%s] error reading the request body: %+v\n", rid, err)
+			payload = valuesToMap(fd)
-			}
 		}
 
-		// parse headers
+	case strings.Contains(contentType, "xml"):
-		headers := valuesToMap(r.Header)
+		payload, err = mxj.NewMapXmlReader(bytes.NewReader(body))
+		if err != nil {
+			log.Printf("[%s] error parsing XML payload: %+v\n", rid, err)
+		}
 
-		// parse query variables
+	case isMultipart:
-		query := valuesToMap(r.URL.Query())
+		err = r.ParseMultipartForm(*maxMultipartMem)
+		if err != nil {
+			msg := fmt.Sprintf("[%s] error parsing multipart form: %+v\n", rid, err)
+			log.Println(msg)
+			w.WriteHeader(http.StatusInternalServerError)
+			fmt.Fprint(w, "Error occurred while parsing multipart form.")
+			return
+		}
 
-		// parse body
+		for k, v := range r.MultipartForm.Value {
-		var payload map[string]interface{}
+			log.Printf("[%s] found multipart form value %q", rid, k)
 
-		switch {
+			if payload == nil {
-		case strings.Contains(contentType, "json"):
+				payload = make(map[string]interface{})
-			decoder := json.NewDecoder(bytes.NewReader(body))
-			decoder.UseNumber()
-
-			err := decoder.Decode(&payload)
-			if err != nil {
-				log.Printf("[%s] error parsing JSON payload %+v\n", rid, err)
 			}
 
-		case strings.Contains(contentType, "x-www-form-urlencoded"):
+			// TODO(moorereason): support duplicate, named values
-			fd, err := url.ParseQuery(string(body))
+			payload[k] = v[0]
-			if err != nil {
+		}
-				log.Printf("[%s] error parsing form payload %+v\n", rid, err)
-			} else {
-				payload = valuesToMap(fd)
-			}
 
-		case strings.Contains(contentType, "xml"):
+		for k, v := range r.MultipartForm.File {
-			payload, err = mxj.NewMapXmlReader(bytes.NewReader(body))
+			// Force parsing as JSON regardless of Content-Type.
-			if err != nil {
+			var parseAsJSON bool
-				log.Printf("[%s] error parsing XML payload: %+v\n", rid, err)
+			for _, j := range matchedHook.JSONStringParameters {
-			}
+				if j.Source == "payload" && j.Name == k {
-
+					parseAsJSON = true
-		case isMultipart:
+					break
-			err = r.ParseMultipartForm(*maxMultipartMem)
-			if err != nil {
-				msg := fmt.Sprintf("[%s] error parsing multipart form: %+v\n", rid, err)
-				log.Println(msg)
-				w.WriteHeader(http.StatusInternalServerError)
-				fmt.Fprint(w, "Error occurred while parsing multipart form.")
-				return
-			}
-
-			for k, v := range r.MultipartForm.Value {
-				log.Printf("[%s] found multipart form value %q", rid, k)
-
-				if payload == nil {
-					payload = make(map[string]interface{})
 				}
-
-				// TODO(moorereason): support duplicate, named values
-				payload[k] = v[0]
 			}
 
-			for k, v := range r.MultipartForm.File {
+			// TODO(moorereason): we need to support multiple parts
-				// Force parsing as JSON regardless of Content-Type.
+			// with the same name instead of just processing the first
-				var parseAsJSON bool
+			// one. Will need #215 resolved first.
-				for _, j := range matchedHook.JSONStringParameters {
+
-					if j.Source == "payload" && j.Name == k {
+			// MIME encoding can contain duplicate headers, so check them
+			// all.
+			if !parseAsJSON && len(v[0].Header["Content-Type"]) > 0 {
+				for _, j := range v[0].Header["Content-Type"] {
+					if j == "application/json" {
 						parseAsJSON = true
 						break
 					}
 				}
-
-				// TODO(moorereason): we need to support multiple parts
-				// with the same name instead of just processing the first
-				// one. Will need #215 resolved first.
-
-				// MIME encoding can contain duplicate headers, so check them
-				// all.
-				if !parseAsJSON && len(v[0].Header["Content-Type"]) > 0 {
-					for _, j := range v[0].Header["Content-Type"] {
-						if j == "application/json" {
-							parseAsJSON = true
-							break
-						}
-					}
-				}
-
-				if parseAsJSON {
-					log.Printf("[%s] parsing multipart form file %q as JSON\n", rid, k)
-
-					f, err := v[0].Open()
-					if err != nil {
-						msg := fmt.Sprintf("[%s] error parsing multipart form file: %+v\n", rid, err)
-						log.Println(msg)
-						w.WriteHeader(http.StatusInternalServerError)
-						fmt.Fprint(w, "Error occurred while parsing multipart form file.")
-						return
-					}
-
-					decoder := json.NewDecoder(f)
-					decoder.UseNumber()
-
-					var part map[string]interface{}
-					err = decoder.Decode(&part)
-					if err != nil {
-						log.Printf("[%s] error parsing JSON payload file: %+v\n", rid, err)
-					}
-
-					if payload == nil {
-						payload = make(map[string]interface{})
-					}
-					payload[k] = part
-				}
 			}
 
-		default:
+			if parseAsJSON {
-			log.Printf("[%s] error parsing body payload due to unsupported content type header: %s\n", rid, contentType)
+				log.Printf("[%s] parsing multipart form file %q as JSON\n", rid, k)
-		}
-
-		// handle hook
-		errors := matchedHook.ParseJSONParameters(&headers, &query, &payload)
-		for _, err := range errors {
-			log.Printf("[%s] error parsing JSON parameters: %s\n", rid, err)
-		}
-
-		var ok bool
-
-		if matchedHook.TriggerRule == nil {
-			ok = true
-		} else {
-			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.Println(msg)
-				w.WriteHeader(http.StatusInternalServerError)
-				fmt.Fprint(w, "Error occurred while evaluating hook rules.")
-				return
-			}
-		}
-
-		if ok {
-			log.Printf("[%s] %s hook triggered successfully\n", rid, matchedHook.ID)
-
-			for _, responseHeader := range matchedHook.ResponseHeaders {
-				w.Header().Set(responseHeader.Name, responseHeader.Value)
-			}
-
-			if matchedHook.CaptureCommandOutput {
-				response, err := handleHook(matchedHook, rid, &headers, &query, &payload, &body)
 
+				f, err := v[0].Open()
 				if err != nil {
+					msg := fmt.Sprintf("[%s] error parsing multipart form file: %+v\n", rid, err)
+					log.Println(msg)
 					w.WriteHeader(http.StatusInternalServerError)
-					if matchedHook.CaptureCommandOutputOnError {
+					fmt.Fprint(w, "Error occurred while parsing multipart form file.")
-						fmt.Fprint(w, response)
+					return
-					} else {
+				}
-						w.Header().Set("Content-Type", "text/plain; charset=utf-8")
+
-						fmt.Fprint(w, "Error occurred while executing the hook's command. Please check your logs for more details.")
+				decoder := json.NewDecoder(f)
-					}
+				decoder.UseNumber()
-				} else {
+
-					// Check if a success return code is configured for the hook
+				var part map[string]interface{}
-					if matchedHook.SuccessHttpResponseCode != 0 {
+				err = decoder.Decode(&part)
-						writeHttpResponseCode(w, rid, matchedHook.ID, matchedHook.SuccessHttpResponseCode)
+				if err != nil {
-					}
+					log.Printf("[%s] error parsing JSON payload file: %+v\n", rid, err)
+				}
+
+				if payload == nil {
+					payload = make(map[string]interface{})
+				}
+				payload[k] = part
+			}
+		}
+
+	default:
+		log.Printf("[%s] error parsing body payload due to unsupported content type header: %s\n", rid, contentType)
+	}
+
+	// handle hook
+	errors := matchedHook.ParseJSONParameters(&headers, &query, &payload)
+	for _, err := range errors {
+		log.Printf("[%s] error parsing JSON parameters: %s\n", rid, err)
+	}
+
+	var ok bool
+
+	if matchedHook.TriggerRule == nil {
+		ok = true
+	} else {
+		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.Println(msg)
+			w.WriteHeader(http.StatusInternalServerError)
+			fmt.Fprint(w, "Error occurred while evaluating hook rules.")
+			return
+		}
+	}
+
+	if ok {
+		log.Printf("[%s] %s hook triggered successfully\n", rid, matchedHook.ID)
+
+		for _, responseHeader := range matchedHook.ResponseHeaders {
+			w.Header().Set(responseHeader.Name, responseHeader.Value)
+		}
+
+		if matchedHook.CaptureCommandOutput {
+			response, err := handleHook(matchedHook, rid, &headers, &query, &payload, &body)
+
+			if err != nil {
+				w.WriteHeader(http.StatusInternalServerError)
+				if matchedHook.CaptureCommandOutputOnError {
 					fmt.Fprint(w, response)
+				} else {
+					w.Header().Set("Content-Type", "text/plain; charset=utf-8")
+					fmt.Fprint(w, "Error occurred while executing the hook's command. Please check your logs for more details.")
 				}
 			} else {
-				go handleHook(matchedHook, rid, &headers, &query, &payload, &body)
-
 				// Check if a success return code is configured for the hook
 				if matchedHook.SuccessHttpResponseCode != 0 {
 					writeHttpResponseCode(w, rid, matchedHook.ID, matchedHook.SuccessHttpResponseCode)
 				}
-
+				fmt.Fprint(w, response)
-				fmt.Fprint(w, matchedHook.ResponseMessage)
 			}
-			return
+		} else {
+			go handleHook(matchedHook, rid, &headers, &query, &payload, &body)
+
+			// Check if a success return code is configured for the hook
+			if matchedHook.SuccessHttpResponseCode != 0 {
+				writeHttpResponseCode(w, rid, matchedHook.ID, matchedHook.SuccessHttpResponseCode)
+			}
+
+			fmt.Fprint(w, matchedHook.ResponseMessage)
 		}
-
+		return
-		// Check if a return code is configured for the hook
-		if matchedHook.TriggerRuleMismatchHttpResponseCode != 0 {
-			writeHttpResponseCode(w, rid, matchedHook.ID, matchedHook.TriggerRuleMismatchHttpResponseCode)
-		}
-
-		// if none of the hooks got triggered
-		log.Printf("[%s] %s got matched, but didn't get triggered because the trigger rules were not satisfied\n", rid, matchedHook.ID)
-
-		fmt.Fprint(w, "Hook rules were not satisfied.")
-	} else {
-		w.WriteHeader(http.StatusNotFound)
-		fmt.Fprint(w, "Hook not found.")
 	}
+
+	// Check if a return code is configured for the hook
+	if matchedHook.TriggerRuleMismatchHttpResponseCode != 0 {
+		writeHttpResponseCode(w, rid, matchedHook.ID, matchedHook.TriggerRuleMismatchHttpResponseCode)
+	}
+
+	// if none of the hooks got triggered
+	log.Printf("[%s] %s got matched, but didn't get triggered because the trigger rules were not satisfied\n", rid, matchedHook.ID)
+
+	fmt.Fprint(w, "Hook rules were not satisfied.")
 }
 
 func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]interface{}, body *[]byte) (string, error) {

webhook_test.go

@@ -95,7 +95,7 @@ func TestWebhook(t *testing.T) {
 
 				url := fmt.Sprintf("http://%s:%s/hooks/%s", ip, port, tt.id)
 
-				req, err := http.NewRequest("POST", url, ioutil.NopCloser(strings.NewReader(tt.body)))
+				req, err := http.NewRequest(tt.method, url, ioutil.NopCloser(strings.NewReader(tt.body)))
 				if err != nil {
 					t.Errorf("New request failed: %s", err)
 				}
@@ -289,6 +289,7 @@ func webhookEnv() (env []string) {
 var hookHandlerTests = []struct {
 	desc        string
 	id          string
+	method      string
 	headers     map[string]string
 	contentType string
 	body        string
@@ -300,6 +301,7 @@ var hookHandlerTests = []struct {
 	{
 		"github",
 		"github",
+		"POST",
 		map[string]string{"X-Hub-Signature": "f68df0375d7b03e3eb29b4cf9f9ec12e08f42ff8"},
 		"application/json",
 		`{
@@ -455,6 +457,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 	{
 		"bitbucket", // bitbucket sends their payload using uriencoded params.
 		"bitbucket",
+		"POST",
 		nil,
 		"application/x-www-form-urlencoded",
 		`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"}`,
@@ -465,6 +468,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 	{
 		"gitlab",
 		"gitlab",
+		"POST",
 		map[string]string{"X-Gitlab-Event": "Push Hook"},
 		"application/json",
 		`{
@@ -517,6 +521,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 	{
 		"xml",
 		"xml",
+		"POST",
 		map[string]string{"Content-Type": "application/xml"},
 		"application/xml",
 		`<app>
@@ -535,6 +540,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 	{
 		"multipart",
 		"plex",
+		"POST",
 		nil,
 		"multipart/form-data; boundary=xxx",
 		`--xxx
@@ -566,6 +572,7 @@ binary data
 	{
 		"missing-cmd-arg", // missing head_commit.author.email
 		"github",
+		"POST",
 		map[string]string{"X-Hub-Signature": "ab03955b9377f530aa298b1b6d273ae9a47e1e40"},
 		"application/json",
 		`{
@@ -607,6 +614,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 	{
 		"missing-env-arg", // missing head_commit.timestamp
 		"github",
+		"POST",
 		map[string]string{"X-Hub-Signature": "2cf8b878cb6b74a25090a140fa4a474be04b97fa"},
 		"application/json",
 		`{
@@ -643,25 +651,27 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 		``,
 	},
 
+	// test with disallowed HTTP method
+	{"disallowed method", "github", "GET", nil, `{}`, "application/json", http.StatusMethodNotAllowed, ``, ``},
 	// test with custom return code
-	{"empty payload", "github", nil, "application/json", `{}`, http.StatusBadRequest, `Hook rules were not satisfied.`, ``},
+	{"empty payload", "github", "POST", nil, "application/json", `{}`, http.StatusBadRequest, `Hook rules were not satisfied.`, ``},
 	// test with custom invalid http code, should default to 200 OK
-	{"empty payload", "bitbucket", nil, "application/json", `{}`, http.StatusOK, `Hook rules were not satisfied.`, ``},
+	{"empty payload", "bitbucket", "POST", nil, "application/json", `{}`, http.StatusOK, `Hook rules were not satisfied.`, ``},
 	// test with no configured http return code, should default to 200 OK
-	{"empty payload", "gitlab", nil, "application/json", `{}`, http.StatusOK, `Hook rules were not satisfied.`, ``},
+	{"empty payload", "gitlab", "POST", nil, "application/json", `{}`, 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, "application/json", `{}`, http.StatusOK, ``, ``},
+	{"don't capture output on success by default", "capture-command-output-on-success-not-by-default", "POST", nil, "application/json", `{}`, http.StatusOK, ``, ``},
-	{"capture output on success with flag set", "capture-command-output-on-success-yes-with-flag", nil, "application/json", `{}`, http.StatusOK, `arg: exit=0
+	{"capture output on success with flag set", "capture-command-output-on-success-yes-with-flag", "POST", nil, "application/json", `{}`, http.StatusOK, `arg: exit=0
 `, ``},
-	{"don't capture output on error by default", "capture-command-output-on-error-not-by-default", nil, "application/json", `{}`, 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", "POST", nil, "application/json", `{}`, 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, "application/json", `{}`, http.StatusInternalServerError, `arg: exit=1
+	{"capture output on error with extra flag set", "capture-command-output-on-error-yes-with-extra-flag", "POST", nil, "application/json", `{}`, http.StatusInternalServerError, `arg: exit=1
 `, ``},
 
 	// Check logs
-	{"static params should pass", "static-params-ok", nil, "application/json", `{}`, http.StatusOK, "arg: passed\n", `(?s)command output: arg: passed`},
+	{"static params should pass", "static-params-ok", "POST", nil, "application/json", `{}`, http.StatusOK, "arg: passed\n", `(?s)command output: arg: passed`},
-	{"command with space logs warning", "warn-on-space", nil, "application/json", `{}`, 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`},
+	{"command with space logs warning", "warn-on-space", "POST", nil, "application/json", `{}`, 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`},
-	{"unsupported content type error", "github", map[string]string{"Content-Type": "nonexistent/format"}, "application/json", `{}`, http.StatusBadRequest, `Hook rules were not satisfied.`, `(?s)error parsing body payload due to unsupported content type header:`},
+	{"unsupported content type error", "github", "POST", map[string]string{"Content-Type": "nonexistent/format"}, "application/json", `{}`, http.StatusBadRequest, `Hook rules were not satisfied.`, `(?s)error parsing body payload due to unsupported content type header:`},
 }
 
 // buffer provides a concurrency-safe bytes.Buffer to tests above.