From a542011cdce8fb579089a0e6a8284526a19d21d3 Mon Sep 17 00:00:00 2001 From: ChandranshuRao14 Date: Tue, 24 Dec 2024 12:59:42 -0600 Subject: [PATCH 1/4] Feat: Allow setting security_and_analysis settings in gh repo edit --- pkg/cmd/repo/edit/edit.go | 92 ++++++++++++++++++++++++++++------ pkg/cmd/repo/edit/edit_test.go | 30 +++++++++++ 2 files changed, 106 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index 29a149cf9..80e460186 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -66,22 +66,37 @@ type EditOptions struct { } type EditRepositoryInput struct { - AllowForking *bool `json:"allow_forking,omitempty"` - AllowUpdateBranch *bool `json:"allow_update_branch,omitempty"` - DefaultBranch *string `json:"default_branch,omitempty"` - DeleteBranchOnMerge *bool `json:"delete_branch_on_merge,omitempty"` - Description *string `json:"description,omitempty"` - EnableAutoMerge *bool `json:"allow_auto_merge,omitempty"` - EnableIssues *bool `json:"has_issues,omitempty"` - EnableMergeCommit *bool `json:"allow_merge_commit,omitempty"` - EnableProjects *bool `json:"has_projects,omitempty"` - EnableDiscussions *bool `json:"has_discussions,omitempty"` - EnableRebaseMerge *bool `json:"allow_rebase_merge,omitempty"` - EnableSquashMerge *bool `json:"allow_squash_merge,omitempty"` - EnableWiki *bool `json:"has_wiki,omitempty"` - Homepage *string `json:"homepage,omitempty"` - IsTemplate *bool `json:"is_template,omitempty"` - Visibility *string `json:"visibility,omitempty"` + enableAdvancedSecurity *bool + enableSecretScanning *bool + enableSecretScanningPushProtection *bool + + AllowForking *bool `json:"allow_forking,omitempty"` + AllowUpdateBranch *bool `json:"allow_update_branch,omitempty"` + DefaultBranch *string `json:"default_branch,omitempty"` + DeleteBranchOnMerge *bool `json:"delete_branch_on_merge,omitempty"` + Description *string `json:"description,omitempty"` + EnableAutoMerge *bool `json:"allow_auto_merge,omitempty"` + EnableIssues *bool `json:"has_issues,omitempty"` + EnableMergeCommit *bool `json:"allow_merge_commit,omitempty"` + EnableProjects *bool `json:"has_projects,omitempty"` + EnableDiscussions *bool `json:"has_discussions,omitempty"` + EnableRebaseMerge *bool `json:"allow_rebase_merge,omitempty"` + EnableSquashMerge *bool `json:"allow_squash_merge,omitempty"` + EnableWiki *bool `json:"has_wiki,omitempty"` + Homepage *string `json:"homepage,omitempty"` + IsTemplate *bool `json:"is_template,omitempty"` + SecurityAndAnalysis *SecurityAndAnalysisInput `json:"security_and_analysis,omitempty"` + Visibility *string `json:"visibility,omitempty"` +} + +type SecurityAndAnalysisInput struct { + EnableAdvancedSecurity *SecurityAndAnalysisStatus `json:"advanced_security,omitempty"` + EnableSecretScanning *SecurityAndAnalysisStatus `json:"secret_scanning,omitempty"` + EnableSecretScanningPushProtection *SecurityAndAnalysisStatus `json:"secret_scanning_push_protection,omitempty"` +} + +type SecurityAndAnalysisStatus struct { + Status *string `json:"status,omitempty"` } func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobra.Command { @@ -177,6 +192,9 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr cmdutil.NilBoolFlag(cmd, &opts.Edits.EnableSquashMerge, "enable-squash-merge", "", "Enable merging pull requests via squashed commit") cmdutil.NilBoolFlag(cmd, &opts.Edits.EnableRebaseMerge, "enable-rebase-merge", "", "Enable merging pull requests via rebase") cmdutil.NilBoolFlag(cmd, &opts.Edits.EnableAutoMerge, "enable-auto-merge", "", "Enable auto-merge functionality") + cmdutil.NilBoolFlag(cmd, &opts.Edits.enableAdvancedSecurity, "enable-advanced-security", "", "Enable advanced security in the repository") + cmdutil.NilBoolFlag(cmd, &opts.Edits.enableSecretScanning, "enable-secret-scanning", "", "Enable secret scanning in the repository") + cmdutil.NilBoolFlag(cmd, &opts.Edits.enableSecretScanningPushProtection, "enable-secret-scanning-push-protection", "", "Enable secret scanning push protection in the repository. Secret scanning must be enabled first") cmdutil.NilBoolFlag(cmd, &opts.Edits.DeleteBranchOnMerge, "delete-branch-on-merge", "", "Delete head branch when pull requests are merged") cmdutil.NilBoolFlag(cmd, &opts.Edits.AllowForking, "allow-forking", "", "Allow forking of an organization repository") cmdutil.NilBoolFlag(cmd, &opts.Edits.AllowUpdateBranch, "allow-update-branch", "", "Allow a pull request head branch that is behind its base branch to be updated") @@ -240,6 +258,34 @@ func editRun(ctx context.Context, opts *EditOptions) error { } } + if hasSecurityEdits(opts.Edits) { + apiClient := api.NewClientFromHTTP(opts.HTTPClient) + repo, err := api.FetchRepository(apiClient, opts.Repository, []string{"viewerCanAdminister"}) + if err != nil { + return err + } + if !repo.ViewerCanAdminister { + return fmt.Errorf("you do not have sufficient permissions to edit repository security and analysis features") + } + + opts.Edits.SecurityAndAnalysis = &SecurityAndAnalysisInput{} + if opts.Edits.enableAdvancedSecurity != nil { + opts.Edits.SecurityAndAnalysis.EnableAdvancedSecurity = &SecurityAndAnalysisStatus{ + Status: boolToStatus(*opts.Edits.enableAdvancedSecurity), + } + } + if opts.Edits.enableSecretScanning != nil { + opts.Edits.SecurityAndAnalysis.EnableSecretScanning = &SecurityAndAnalysisStatus{ + Status: boolToStatus(*opts.Edits.enableSecretScanning), + } + } + if opts.Edits.enableSecretScanningPushProtection != nil { + opts.Edits.SecurityAndAnalysis.EnableSecretScanningPushProtection = &SecurityAndAnalysisStatus{ + Status: boolToStatus(*opts.Edits.enableSecretScanningPushProtection), + } + } + } + apiPath := fmt.Sprintf("repos/%s/%s", repo.RepoOwner(), repo.RepoName()) body := &bytes.Buffer{} @@ -560,3 +606,17 @@ func isIncluded(value string, opts []string) bool { } return false } + +func boolToStatus(status bool) *string { + var result string + if status { + result = "enabled" + } else { + result = "disabled" + } + return &result +} + +func hasSecurityEdits(edits EditRepositoryInput) bool { + return edits.enableAdvancedSecurity != nil || edits.enableSecretScanning != nil || edits.enableSecretScanningPushProtection != nil +} diff --git a/pkg/cmd/repo/edit/edit_test.go b/pkg/cmd/repo/edit/edit_test.go index 217c1dce4..93d44788d 100644 --- a/pkg/cmd/repo/edit/edit_test.go +++ b/pkg/cmd/repo/edit/edit_test.go @@ -201,6 +201,36 @@ func Test_editRun(t *testing.T) { })) }, }, + { + name: "enable/disable security and analysis settings", + opts: EditOptions{ + Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"), + Edits: EditRepositoryInput{ + SecurityAndAnalysis: &SecurityAndAnalysisInput{ + EnableAdvancedSecurity: &SecurityAndAnalysisStatus{ + Status: sp("enabled"), + }, + EnableSecretScanning: &SecurityAndAnalysisStatus{ + Status: sp("enabled"), + }, + EnableSecretScanningPushProtection: &SecurityAndAnalysisStatus{ + Status: sp("disabled"), + }, + }, + }, + }, + httpStubs: func(t *testing.T, r *httpmock.Registry) { + r.Register( + httpmock.REST("PATCH", "repos/OWNER/REPO"), + httpmock.RESTPayload(200, `{}`, func(payload map[string]interface{}) { + assert.Equal(t, 1, len(payload)) + securityAndAnalysis := payload["security_and_analysis"].(map[string]interface{}) + assert.Equal(t, "enabled", securityAndAnalysis["advanced_security"].(map[string]interface{})["status"]) + assert.Equal(t, "enabled", securityAndAnalysis["secret_scanning"].(map[string]interface{})["status"]) + assert.Equal(t, "disabled", securityAndAnalysis["secret_scanning_push_protection"].(map[string]interface{})["status"]) + })) + }, + }, } for _, tt := range tests { From 29371e949126db53cc8d93132d23812a06fc72fd Mon Sep 17 00:00:00 2001 From: ChandranshuRao14 Date: Thu, 26 Dec 2024 22:52:32 -0500 Subject: [PATCH 2/4] Extract logic into helper function --- pkg/cmd/repo/edit/edit.go | 82 ++++++++++++++++--------------- pkg/cmd/repo/edit/edit_test.go | 89 ++++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 38 deletions(-) diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index 80e460186..8efcea11d 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -89,16 +89,6 @@ type EditRepositoryInput struct { Visibility *string `json:"visibility,omitempty"` } -type SecurityAndAnalysisInput struct { - EnableAdvancedSecurity *SecurityAndAnalysisStatus `json:"advanced_security,omitempty"` - EnableSecretScanning *SecurityAndAnalysisStatus `json:"secret_scanning,omitempty"` - EnableSecretScanningPushProtection *SecurityAndAnalysisStatus `json:"secret_scanning_push_protection,omitempty"` -} - -type SecurityAndAnalysisStatus struct { - Status *string `json:"status,omitempty"` -} - func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobra.Command { opts := &EditOptions{ IO: f.IOStreams, @@ -172,6 +162,18 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr return cmdutil.FlagErrorf("use of --visibility flag requires --accept-visibility-change-consequences flag") } + if hasSecurityEdits(opts.Edits) { + apiClient := api.NewClientFromHTTP(opts.HTTPClient) + repo, err := api.FetchRepository(apiClient, opts.Repository, []string{"viewerCanAdminister"}) + if err != nil { + return err + } + if !repo.ViewerCanAdminister { + return fmt.Errorf("you do not have sufficient permissions to edit repository security and analysis features") + } + opts.Edits.SecurityAndAnalysis = transformSecurityAndAnalysisOpts(opts) + } + if runF != nil { return runF(opts) } @@ -258,34 +260,6 @@ func editRun(ctx context.Context, opts *EditOptions) error { } } - if hasSecurityEdits(opts.Edits) { - apiClient := api.NewClientFromHTTP(opts.HTTPClient) - repo, err := api.FetchRepository(apiClient, opts.Repository, []string{"viewerCanAdminister"}) - if err != nil { - return err - } - if !repo.ViewerCanAdminister { - return fmt.Errorf("you do not have sufficient permissions to edit repository security and analysis features") - } - - opts.Edits.SecurityAndAnalysis = &SecurityAndAnalysisInput{} - if opts.Edits.enableAdvancedSecurity != nil { - opts.Edits.SecurityAndAnalysis.EnableAdvancedSecurity = &SecurityAndAnalysisStatus{ - Status: boolToStatus(*opts.Edits.enableAdvancedSecurity), - } - } - if opts.Edits.enableSecretScanning != nil { - opts.Edits.SecurityAndAnalysis.EnableSecretScanning = &SecurityAndAnalysisStatus{ - Status: boolToStatus(*opts.Edits.enableSecretScanning), - } - } - if opts.Edits.enableSecretScanningPushProtection != nil { - opts.Edits.SecurityAndAnalysis.EnableSecretScanningPushProtection = &SecurityAndAnalysisStatus{ - Status: boolToStatus(*opts.Edits.enableSecretScanningPushProtection), - } - } - } - apiPath := fmt.Sprintf("repos/%s/%s", repo.RepoOwner(), repo.RepoName()) body := &bytes.Buffer{} @@ -620,3 +594,35 @@ func boolToStatus(status bool) *string { func hasSecurityEdits(edits EditRepositoryInput) bool { return edits.enableAdvancedSecurity != nil || edits.enableSecretScanning != nil || edits.enableSecretScanningPushProtection != nil } + +type SecurityAndAnalysisInput struct { + EnableAdvancedSecurity *SecurityAndAnalysisStatus `json:"advanced_security,omitempty"` + EnableSecretScanning *SecurityAndAnalysisStatus `json:"secret_scanning,omitempty"` + EnableSecretScanningPushProtection *SecurityAndAnalysisStatus `json:"secret_scanning_push_protection,omitempty"` +} + +type SecurityAndAnalysisStatus struct { + Status *string `json:"status,omitempty"` +} + +// Transform security and analysis parameters to properly serialize EditRepositoryInput +// See API Docs: https://docs.github.com/en/rest/repos/repos?apiVersion=2022-11-28#update-a-repository +func transformSecurityAndAnalysisOpts(opts *EditOptions) *SecurityAndAnalysisInput { + securityOptions := &SecurityAndAnalysisInput{} + if opts.Edits.enableAdvancedSecurity != nil { + securityOptions.EnableAdvancedSecurity = &SecurityAndAnalysisStatus{ + Status: boolToStatus(*opts.Edits.enableAdvancedSecurity), + } + } + if opts.Edits.enableSecretScanning != nil { + securityOptions.EnableSecretScanning = &SecurityAndAnalysisStatus{ + Status: boolToStatus(*opts.Edits.enableSecretScanning), + } + } + if opts.Edits.enableSecretScanningPushProtection != nil { + securityOptions.EnableSecretScanningPushProtection = &SecurityAndAnalysisStatus{ + Status: boolToStatus(*opts.Edits.enableSecretScanningPushProtection), + } + } + return securityOptions +} diff --git a/pkg/cmd/repo/edit/edit_test.go b/pkg/cmd/repo/edit/edit_test.go index 93d44788d..93b256465 100644 --- a/pkg/cmd/repo/edit/edit_test.go +++ b/pkg/cmd/repo/edit/edit_test.go @@ -700,6 +700,95 @@ func Test_editRun_interactive(t *testing.T) { } } +func Test_transformSecurityAndAnalysisOpts(t *testing.T) { + tests := []struct { + name string + opts EditOptions + want *SecurityAndAnalysisInput + }{ + { + name: "Enable all security and analysis settings", + opts: EditOptions{ + Edits: EditRepositoryInput{ + enableAdvancedSecurity: bp(true), + enableSecretScanning: bp(true), + enableSecretScanningPushProtection: bp(true), + }, + }, + want: &SecurityAndAnalysisInput{ + EnableAdvancedSecurity: &SecurityAndAnalysisStatus{ + Status: sp("enabled"), + }, + EnableSecretScanning: &SecurityAndAnalysisStatus{ + Status: sp("enabled"), + }, + EnableSecretScanningPushProtection: &SecurityAndAnalysisStatus{ + Status: sp("enabled"), + }, + }, + }, + { + name: "Disable all security and analysis settings", + opts: EditOptions{ + Edits: EditRepositoryInput{ + enableAdvancedSecurity: bp(false), + enableSecretScanning: bp(false), + enableSecretScanningPushProtection: bp(false), + }, + }, + want: &SecurityAndAnalysisInput{ + EnableAdvancedSecurity: &SecurityAndAnalysisStatus{ + Status: sp("disabled"), + }, + EnableSecretScanning: &SecurityAndAnalysisStatus{ + Status: sp("disabled"), + }, + EnableSecretScanningPushProtection: &SecurityAndAnalysisStatus{ + Status: sp("disabled"), + }, + }, + }, + { + name: "Enable only advanced security", + opts: EditOptions{ + Edits: EditRepositoryInput{ + enableAdvancedSecurity: bp(true), + }, + }, + want: &SecurityAndAnalysisInput{ + EnableAdvancedSecurity: &SecurityAndAnalysisStatus{ + Status: sp("enabled"), + }, + EnableSecretScanning: nil, + EnableSecretScanningPushProtection: nil, + }, + }, + { + name: "Disable only secret scanning", + opts: EditOptions{ + Edits: EditRepositoryInput{ + enableSecretScanning: bp(false), + }, + }, + want: &SecurityAndAnalysisInput{ + EnableAdvancedSecurity: nil, + EnableSecretScanning: &SecurityAndAnalysisStatus{ + Status: sp("disabled"), + }, + EnableSecretScanningPushProtection: nil, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + opts := &tt.opts + transformed := transformSecurityAndAnalysisOpts(opts) + assert.Equal(t, tt.want, transformed) + }) + } +} + func sp(v string) *string { return &v } From 079719f9230cd7ccdc26c23375016272a5d13ac3 Mon Sep 17 00:00:00 2001 From: ChandranshuRao14 Date: Tue, 31 Dec 2024 00:47:05 -0500 Subject: [PATCH 3/4] Move api call to editRun --- pkg/cmd/repo/edit/edit.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index 8efcea11d..cacc9420d 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -163,14 +163,6 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr } if hasSecurityEdits(opts.Edits) { - apiClient := api.NewClientFromHTTP(opts.HTTPClient) - repo, err := api.FetchRepository(apiClient, opts.Repository, []string{"viewerCanAdminister"}) - if err != nil { - return err - } - if !repo.ViewerCanAdminister { - return fmt.Errorf("you do not have sufficient permissions to edit repository security and analysis features") - } opts.Edits.SecurityAndAnalysis = transformSecurityAndAnalysisOpts(opts) } @@ -260,6 +252,17 @@ func editRun(ctx context.Context, opts *EditOptions) error { } } + if hasSecurityEdits(opts.Edits) { + apiClient := api.NewClientFromHTTP(opts.HTTPClient) + repo, err := api.FetchRepository(apiClient, opts.Repository, []string{"viewerCanAdminister"}) + if err != nil { + return err + } + if !repo.ViewerCanAdminister { + return fmt.Errorf("you do not have sufficient permissions to edit repository security and analysis features") + } + } + apiPath := fmt.Sprintf("repos/%s/%s", repo.RepoOwner(), repo.RepoName()) body := &bytes.Buffer{} From 576fa8a3bc97169f24fd5264dc4f4114027308c6 Mon Sep 17 00:00:00 2001 From: Tyler McGoffin Date: Thu, 2 Jan 2025 22:29:45 -0800 Subject: [PATCH 4/4] Add test for permissions check for security and analysis edits (#1) --- pkg/cmd/repo/edit/edit.go | 2 +- pkg/cmd/repo/edit/edit_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index cacc9420d..daa700cfb 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -252,7 +252,7 @@ func editRun(ctx context.Context, opts *EditOptions) error { } } - if hasSecurityEdits(opts.Edits) { + if opts.Edits.SecurityAndAnalysis != nil { apiClient := api.NewClientFromHTTP(opts.HTTPClient) repo, err := api.FetchRepository(apiClient, opts.Repository, []string{"viewerCanAdminister"}) if err != nil { diff --git a/pkg/cmd/repo/edit/edit_test.go b/pkg/cmd/repo/edit/edit_test.go index 93b256465..868e300fa 100644 --- a/pkg/cmd/repo/edit/edit_test.go +++ b/pkg/cmd/repo/edit/edit_test.go @@ -220,6 +220,10 @@ func Test_editRun(t *testing.T) { }, }, httpStubs: func(t *testing.T, r *httpmock.Registry) { + r.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(`{"data": { "repository": { "viewerCanAdminister": true } } }`)) + r.Register( httpmock.REST("PATCH", "repos/OWNER/REPO"), httpmock.RESTPayload(200, `{}`, func(payload map[string]interface{}) { @@ -231,6 +235,31 @@ func Test_editRun(t *testing.T) { })) }, }, + { + name: "does not have sufficient permissions for security edits", + opts: EditOptions{ + Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"), + Edits: EditRepositoryInput{ + SecurityAndAnalysis: &SecurityAndAnalysisInput{ + EnableAdvancedSecurity: &SecurityAndAnalysisStatus{ + Status: sp("enabled"), + }, + EnableSecretScanning: &SecurityAndAnalysisStatus{ + Status: sp("enabled"), + }, + EnableSecretScanningPushProtection: &SecurityAndAnalysisStatus{ + Status: sp("disabled"), + }, + }, + }, + }, + httpStubs: func(t *testing.T, r *httpmock.Registry) { + r.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(`{"data": { "repository": { "viewerCanAdminister": false } } }`)) + }, + wantsErr: "you do not have sufficient permissions to edit repository security and analysis features", + }, } for _, tt := range tests {