Add ability to verify webhook secret. Fixes #34

README.md

@@ -226,7 +226,7 @@ If installing on a single repository, navigate to the repository home page and c
 - Click **Add webhook**
 - set **Payload URL** to `http://$URL/events` where `$URL` is where Atlantis is hosted. **Be sure to add `/events`**
 - set **Content type** to `application/json`
-- leave **Secret** blank
+- leave **Secret** blank or set this to a random key (https://www.random.org/strings/). If you set it, you'll need to use the `--gh-webhook-secret` option when you start Atlantis
 - select **Let me select individual events**
 - check the boxes
 	- **Pull request review**
@@ -245,13 +245,14 @@ Once you've created the user (or have decided to use an existing user) you need
 ### Start Atlantis
 Now you're ready to start Atlantis! Run
 ```
-$ atlantis server --atlantis-url $URL --gh-user $USERNAME --gh-token $TOKEN
+$ atlantis server --atlantis-url $URL --gh-user $USERNAME --gh-token $TOKEN --gh-webhook-secret $SECRET
 2049/10/6 00:00:00 [WARN] server: Atlantis started - listening on port 4141
 ```
 
 - `$URL` is the URL that Atlantis can be reached at
 - `$USERNAME` is the GitHub username you generated the token for
 - `$TOKEN` is the access token you created. If you don't want this to be passed in as an argument for security reasons you can specify it in a config file (see [Configuration](#configuration)) or as an environment variable: `ATLANTIS_GH_TOKEN`
+- `$SECRET` is the random key you used for the webhook secret. If you left the secret blank then don't specify this flag. If you don't want this to be passed in as an argument for security reasons you can specify it in a config file (see [Configuration](#configuration)) or as an environment variable: `ATLANTIS_GH_WEBHOOK_SECRET`
 
 Atlantis is now running!
 **We recommend running it under something like Systemd or Supervisord.**

cmd/server.go

@@ -25,6 +25,7 @@ const (
 	ghHostnameFlag      = "gh-hostname"
 	ghTokenFlag         = "gh-token"
 	ghUserFlag          = "gh-user"
+	ghWebHookSecret     = "gh-webhook-secret"
 	logLevelFlag        = "log-level"
 	portFlag            = "port"
 	requireApprovalFlag = "require-approval"
@@ -67,6 +68,11 @@ var stringFlags = []stringFlag{
 		name:        ghUserFlag,
 		description: "[REQUIRED] GitHub username of API user.",
 	},
+	{
+		name:        ghWebHookSecret,
+		description: "Optional secret used for GitHub webhooks (see https://developer.github.com/webhooks/securing/). If not specified, Atlantis won't validate the incoming webhook call.",
+		env:         "ATLANTIS_GH_WEBHOOK_SECRET",
+	},
 	{
 		name:        logLevelFlag,
 		description: "Log level. Either debug, info, warn, or error.",

server/server.go

@@ -2,14 +2,16 @@ package server
 
 import (
 	"fmt"
-	"io/ioutil"
 	"log"
 	"net/http"
 	"net/url"
 	"os"
 	"strings"
 	"time"
 
+	"bytes"
+	"io/ioutil"
+
 	"github.com/elazarl/go-bindata-assetfs"
 	gh "github.com/google/go-github/github"
 	"github.com/gorilla/mux"
@@ -33,28 +35,30 @@ const (
 
 // Server listens for GitHub events and runs the necessary Atlantis command
 type Server struct {
-	router             *mux.Router
-	port               int
-	commandHandler     *CommandHandler
-	pullClosedExecutor *PullClosedExecutor
-	logger             *logging.SimpleLogger
-	eventParser        *EventParser
-	lockingClient      *locking.Client
-	atlantisURL        string
+	router              *mux.Router
+	port                int
+	commandHandler      *CommandHandler
+	pullClosedExecutor  *PullClosedExecutor
+	logger              *logging.SimpleLogger
+	eventParser         *EventParser
+	lockingClient       *locking.Client
+	atlantisURL         string
+	githubWebHookSecret []byte
 }
 
 // the mapstructure tags correspond to flags in cmd/server.go
 type ServerConfig struct {
-	AWSRegion       string `mapstructure:"aws-region"`
-	AssumeRole      string `mapstructure:"aws-assume-role-arn"`
-	AtlantisURL     string `mapstructure:"atlantis-url"`
-	DataDir         string `mapstructure:"data-dir"`
-	GithubHostname  string `mapstructure:"gh-hostname"`
-	GithubToken     string `mapstructure:"gh-token"`
-	GithubUser      string `mapstructure:"gh-user"`
-	LogLevel        string `mapstructure:"log-level"`
-	Port            int    `mapstructure:"port"`
-	RequireApproval bool   `mapstructure:"require-approval"`
+	AWSRegion           string `mapstructure:"aws-region"`
+	AssumeRole          string `mapstructure:"aws-assume-role-arn"`
+	AtlantisURL         string `mapstructure:"atlantis-url"`
+	DataDir             string `mapstructure:"data-dir"`
+	GithubHostname      string `mapstructure:"gh-hostname"`
+	GithubToken         string `mapstructure:"gh-token"`
+	GithubUser          string `mapstructure:"gh-user"`
+	GithubWebHookSecret string `mapstructure:"gh-webhook-secret"`
+	LogLevel            string `mapstructure:"log-level"`
+	Port                int    `mapstructure:"port"`
+	RequireApproval     bool   `mapstructure:"require-approval"`
 }
 
 type CommandContext struct {
@@ -156,14 +160,15 @@ func NewServer(config ServerConfig) (*Server, error) {
 	}
 	router := mux.NewRouter()
 	return &Server{
-		router:             router,
-		port:               config.Port,
-		commandHandler:     commandHandler,
-		pullClosedExecutor: pullClosedExecutor,
-		eventParser:        eventParser,
-		logger:             logger,
-		lockingClient:      lockingClient,
-		atlantisURL:        config.AtlantisURL,
+		router:              router,
+		port:                config.Port,
+		commandHandler:      commandHandler,
+		pullClosedExecutor:  pullClosedExecutor,
+		eventParser:         eventParser,
+		logger:              logger,
+		lockingClient:       lockingClient,
+		atlantisURL:         config.AtlantisURL,
+		githubWebHookSecret: []byte(config.GithubWebHookSecret),
 	}, nil
 }
 
@@ -300,23 +305,50 @@ func (s *Server) postEvents(w http.ResponseWriter, r *http.Request) {
 	githubReqID := "X-Github-Delivery=" + r.Header.Get("X-Github-Delivery")
 	var payload []byte
 
-	// webhook requests can either be application/json or application/x-www-form-urlencoded
+	// webhook requests can either be application/json or application/x-www-form-urlencoded.
+	// We accept both to make it easier on users that may choose x-www-form-urlencoded by mistake
 	if r.Header.Get("Content-Type") == "application/x-www-form-urlencoded" {
 		// GitHub stores the json payload as a form value
 		payloadForm := r.PostFormValue("payload")
 		if payloadForm == "" {
 			s.respond(w, logging.Warn, http.StatusBadRequest, "request did not contain expected 'payload' form value")
 			return
 		}
+		if len(s.githubWebHookSecret) != 0 {
+			// github calculates the signature based on the query escaped
+			// post body. In order to use go-github's ValidatePayload method
+			// that only accepts an http request we need to override r.Body
+			// with a value that was the original raw body before it was
+			// parsed.
+			rawPayload := fmt.Sprintf("payload=%s", url.QueryEscape(payloadForm))
+			r.Body = ioutil.NopCloser(bytes.NewBuffer([]byte(rawPayload)))
+			_, err := gh.ValidatePayload(r, s.githubWebHookSecret)
+			if err != nil {
+				s.respond(w, logging.Warn, http.StatusBadRequest, "webhook failed secret key validation")
+				return
+			}
+		}
 		payload = []byte(payloadForm)
 	} else {
 		// else read it as json
-		defer r.Body.Close()
-		var err error
-		payload, err = ioutil.ReadAll(r.Body)
-		if err != nil {
-			s.respond(w, logging.Warn, http.StatusBadRequest, "could not read body: %s", err)
-			return
+		if len(s.githubWebHookSecret) != 0 {
+			var err error
+			payload, err = gh.ValidatePayload(r, s.githubWebHookSecret)
+			if err != nil {
+				s.respond(w, logging.Warn, http.StatusBadRequest, "webhook failed secret key validation")
+				return
+			}
+		} else {
+			// if we're not validating against the webhook secret then
+			// we can't use the ValidatePayload method and need to read
+			// the request body ourselves.
+			defer r.Body.Close()
+			var err error
+			payload, err = ioutil.ReadAll(r.Body)
+			if err != nil {
+				s.respond(w, logging.Warn, http.StatusBadRequest, "could not read body: %s", err)
+				return
+			}
 		}
 	}