Restrict request params of CreateHook for repos and orgs to known sub…

…set (#1018)

Fixes #1015.

github/orgs_hooks.go

@@ -51,10 +51,21 @@ func (s *OrganizationsService) GetHook(ctx context.Context, org string, id int64
 // CreateHook creates a Hook for the specified org.
 // Name and Config are required fields.
 //
+// Note that only a subset of the hook fields are used and hook must
+// not be nil.
+//
 // GitHub API docs: https://developer.github.com/v3/orgs/hooks/#create-a-hook
 func (s *OrganizationsService) CreateHook(ctx context.Context, org string, hook *Hook) (*Hook, *Response, error) {
 	u := fmt.Sprintf("orgs/%v/hooks", org)
-	req, err := s.client.NewRequest("POST", u, hook)
+
+	hookReq := &createHookRequest{
+		Name:   hook.Name,
+		Events: hook.Events,
+		Active: hook.Active,
+		Config: hook.Config,
+	}
+
+	req, err := s.client.NewRequest("POST", u, hookReq)
 	if err != nil {
 		return nil, nil, err
 	}

github/orgs_hooks_test.go

@@ -45,6 +45,36 @@ func TestOrganizationsService_ListHooks_invalidOrg(t *testing.T) {
 	testURLParseError(t, err)
 }
 
+func TestOrganizationsService_CreateHook(t *testing.T) {
+	client, mux, _, teardown := setup()
+	defer teardown()
+
+	input := &Hook{Name: String("t"), CreatedAt: &referenceTime}
+
+	mux.HandleFunc("/orgs/o/hooks", func(w http.ResponseWriter, r *http.Request) {
+		v := new(createHookRequest)
+		json.NewDecoder(r.Body).Decode(v)
+
+		testMethod(t, r, "POST")
+		want := &createHookRequest{Name: String("t")}
+		if !reflect.DeepEqual(v, want) {
+			t.Errorf("Request body = %+v, want %+v", v, want)
+		}
+
+		fmt.Fprint(w, `{"id":1}`)
+	})
+
+	hook, _, err := client.Organizations.CreateHook(context.Background(), "o", input)
+	if err != nil {
+		t.Errorf("Organizations.CreateHook returned error: %v", err)
+	}
+
+	want := &Hook{ID: Int64(1)}
+	if !reflect.DeepEqual(hook, want) {
+		t.Errorf("Organizations.CreateHook returned %+v, want %+v", hook, want)
+	}
+}
+
 func TestOrganizationsService_GetHook(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()

github/repos_hooks.go

@@ -69,27 +69,55 @@ func (w WebHookAuthor) String() string {
 
 // Hook represents a GitHub (web and service) hook for a repository.
 type Hook struct {
-	CreatedAt *time.Time             `json:"created_at,omitempty"`
-	UpdatedAt *time.Time             `json:"updated_at,omitempty"`
-	Name      *string                `json:"name,omitempty"`
-	URL       *string                `json:"url,omitempty"`
-	Events    []string               `json:"events,omitempty"`
-	Active    *bool                  `json:"active,omitempty"`
-	Config    map[string]interface{} `json:"config,omitempty"`
-	ID        *int64                 `json:"id,omitempty"`
+	CreatedAt *time.Time `json:"created_at,omitempty"`
+	UpdatedAt *time.Time `json:"updated_at,omitempty"`
+	URL       *string    `json:"url,omitempty"`
+	ID        *int64     `json:"id,omitempty"`
+
+	// Only the following fields are used when creating a hook.
+	// Name and Config are required.
+	Name   *string                `json:"name,omitempty"`
+	Config map[string]interface{} `json:"config,omitempty"`
+	Events []string               `json:"events,omitempty"`
+	Active *bool                  `json:"active,omitempty"`
 }
 
 func (h Hook) String() string {
 	return Stringify(h)
 }
 
+// createHookRequest is a subset of Hook and is used internally
+// by CreateHook to pass only the known fields for the endpoint.
+//
+// See https://github.com/google/go-github/issues/1015 for more
+// information.
+type createHookRequest struct {
+	// Name and Config are required.
+	// Name must be passed as "web".
+	Name   *string                `json:"name,omitempty"`
+	Config map[string]interface{} `json:"config,omitempty"`
+	Events []string               `json:"events,omitempty"`
+	Active *bool                  `json:"active,omitempty"`
+}
+
 // CreateHook creates a Hook for the specified repository.
 // Name and Config are required fields.
 //
+// Note that only a subset of the hook fields are used and hook must
+// not be nil.
+//
 // GitHub API docs: https://developer.github.com/v3/repos/hooks/#create-a-hook
 func (s *RepositoriesService) CreateHook(ctx context.Context, owner, repo string, hook *Hook) (*Hook, *Response, error) {
 	u := fmt.Sprintf("repos/%v/%v/hooks", owner, repo)
-	req, err := s.client.NewRequest("POST", u, hook)
+
+	hookReq := &createHookRequest{
+		Name:   hook.Name,
+		Events: hook.Events,
+		Active: hook.Active,
+		Config: hook.Config,
+	}
+
+	req, err := s.client.NewRequest("POST", u, hookReq)
 	if err != nil {
 		return nil, nil, err
 	}

github/repos_hooks_test.go

@@ -18,15 +18,16 @@ func TestRepositoriesService_CreateHook(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()
 
-	input := &Hook{Name: String("t")}
+	input := &Hook{Name: String("t"), CreatedAt: &referenceTime}
 
 	mux.HandleFunc("/repos/o/r/hooks", func(w http.ResponseWriter, r *http.Request) {
-		v := new(Hook)
+		v := new(createHookRequest)
 		json.NewDecoder(r.Body).Decode(v)
 
 		testMethod(t, r, "POST")
-		if !reflect.DeepEqual(v, input) {
-			t.Errorf("Request body = %+v, want %+v", v, input)
+		want := &createHookRequest{Name: String("t")}
+		if !reflect.DeepEqual(v, want) {
+			t.Errorf("Request body = %+v, want %+v", v, want)
 		}
 
 		fmt.Fprint(w, `{"id":1}`)
@@ -43,14 +44,6 @@ func TestRepositoriesService_CreateHook(t *testing.T) {
 	}
 }
 
-func TestRepositoriesService_CreateHook_invalidOwner(t *testing.T) {
-	client, _, _, teardown := setup()
-	defer teardown()
-
-	_, _, err := client.Repositories.CreateHook(context.Background(), "%", "%", nil)
-	testURLParseError(t, err)
-}
-
 func TestRepositoriesService_ListHooks(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()

github/strings_test.go

@@ -101,7 +101,7 @@ func TestString(t *testing.T) {
 		{Gist{ID: String("1")}, `github.Gist{ID:"1", Files:map[]}`},
 		{GitObject{SHA: String("s")}, `github.GitObject{SHA:"s"}`},
 		{Gitignore{Name: String("n")}, `github.Gitignore{Name:"n"}`},
-		{Hook{ID: Int64(1)}, `github.Hook{Config:map[], ID:1}`},
+		{Hook{ID: Int64(1)}, `github.Hook{ID:1, Config:map[]}`},
 		{IssueComment{ID: Int64(1)}, `github.IssueComment{ID:1}`},
 		{Issue{Number: Int(1)}, `github.Issue{Number:1}`},
 		{Key{ID: Int64(1)}, `github.Key{ID:1}`},