Update HTTP methods to sanitize user input

internal/hook/hook.go

@@ -649,8 +649,7 @@ func (h *Hooks) LoadFromFile(path string, asTemplate bool) error {
 		file = buf.Bytes()
 	}
 
-	e = yaml.Unmarshal(file, h)
+	return yaml.Unmarshal(file, h)
-	return e
 }
 
 // Append appends hooks unless the new hooks contain a hook with an ID that already exists

test/hooks.json.tmpl

@@ -3,7 +3,7 @@
     "id": "github",
     "execute-command": "{{ .Hookecho }}",
     "command-working-directory": "/",
-    "http-methods": ["POST"],
+    "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,6 +1,6 @@
 - id: github
   http-methods:
-  - POST
+  - "Post "
   trigger-rule:
     and:
     - match:

webhook.go

@@ -208,6 +208,10 @@ func main() {
 		r.HandleFunc(hooksURL, hookHandler)
 	} else {
 		allowed := strings.Split(*httpMethods, ",")
+		for i := range allowed {
+			allowed[i] = strings.TrimSpace(allowed[i])
+		}
+
 		r.HandleFunc(hooksURL, hookHandler).Methods(allowed...)
 	}
 
@@ -257,7 +261,7 @@ func main() {
 func hookHandler(w http.ResponseWriter, r *http.Request) {
 	rid := middleware.GetReqID(r.Context())
 
-	log.Printf("[%s] incoming HTTP request from %s\n", rid, r.RemoteAddr)
+	log.Printf("[%s] incoming HTTP %s request from %s\n", rid, r.Method, r.RemoteAddr)
 
 	id := mux.Vars(r)["id"]
 
@@ -272,6 +276,10 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
 	if len(matchedHook.HTTPMethods) != 0 {
 		var allowed bool
 		for i := range matchedHook.HTTPMethods {
+			// TODO(moorereason): refactor config loading and reloading to
+			// sanitize these methods once at load time.
+			matchedHook.HTTPMethods[i] = strings.ToUpper(strings.TrimSpace(matchedHook.HTTPMethods[i]))
+
 			if matchedHook.HTTPMethods[i] == r.Method {
 				allowed = true
 				break
@@ -280,6 +288,7 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
 
 		if !allowed {
 			w.WriteHeader(http.StatusMethodNotAllowed)
+			log.Printf("[%s] HTTP %s method not implemented for hook %q", rid, r.Method, id)
 			return
 		}
 	}

webhook_test.go

@@ -664,9 +664,9 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00
 	},
 
 	// test with disallowed global HTTP method
-	{"global disallowed method", "bitbucket", []string{"POST"}, "GET", nil, `{}`, "application/json", http.StatusMethodNotAllowed, ``, ``},
+	{"global disallowed method", "bitbucket", []string{"Post "}, "GET", nil, `{}`, "application/json", http.StatusMethodNotAllowed, ``, ``},
 	// test with disallowed HTTP method
-	{"disallowed method", "github", nil, "GET", nil, `{}`, "application/json", http.StatusMethodNotAllowed, ``, ``},
+	{"disallowed method", "github", nil, "Get", nil, `{}`, "application/json", http.StatusMethodNotAllowed, ``, ``},
 	// test with custom return code
 	{"empty payload", "github", nil, "POST", nil, "application/json", `{}`, http.StatusBadRequest, `Hook rules were not satisfied.`, ``},
 	// test with custom invalid http code, should default to 200 OK