From 496496e3efd1c10b3358f46df7f310ac6d0815ab Mon Sep 17 00:00:00 2001 From: detailswes Date: Fri, 16 Jan 2026 17:30:21 +0530 Subject: [PATCH] Consider workflow scope or user-escalation pattern for merging PRs that modify workflow files --- pkg/github/pullrequests.go | 52 ++++++++++++++++++++++++++++++--- pkg/github/pullrequests_test.go | 44 ++++++++++++++++++++++++++++ pkg/scopes/scopes.go | 3 ++ 3 files changed, 95 insertions(+), 4 deletions(-) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 62952783e..5b49d289b 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "strings" "github.com/go-viper/mapstructure/v2" "github.com/google/go-github/v79/github" @@ -1045,6 +1046,27 @@ func ListPullRequests(t translations.TranslationHelperFunc) inventory.ServerTool }) } +// workflowProtectedPaths contains the directory prefixes that require the workflow scope +var workflowProtectedPaths = []string{ + ".github/workflows/", + ".github/workflows-lab/", +} + +// containsWorkflowFiles checks if any of the given commit files are in workflow-protected directories +func containsWorkflowFiles(files []*github.CommitFile) bool { + for _, file := range files { + if file == nil || file.Filename == nil { + continue + } + for _, protectedPath := range workflowProtectedPaths { + if strings.HasPrefix(*file.Filename, protectedPath) { + return true + } + } + } + return false +} + // MergePullRequest creates a tool to merge a pull request. func MergePullRequest(t translations.TranslationHelperFunc) inventory.ServerTool { schema := &jsonschema.Schema{ @@ -1118,15 +1140,37 @@ func MergePullRequest(t translations.TranslationHelperFunc) inventory.ServerTool return utils.NewToolResultError(err.Error()), nil, nil } + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + } + + // Pre-flight check: detect workflow files that require the 'workflow' scope + files, resp, err := client.PullRequests.ListFiles(ctx, owner, repo, pullNumber, &github.ListOptions{PerPage: 100}) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to list pull request files for workflow scope check", + resp, + err, + ), nil, nil + } + if resp != nil && resp.Body != nil { + _ = resp.Body.Close() + } + + if containsWorkflowFiles(files) { + return utils.NewToolResultError( + "This pull request modifies GitHub Actions workflow files (.github/workflows/). " + + "Merging requires the 'workflow' OAuth scope, which is not included by default. " + + "Please use a Personal Access Token with the 'workflow' scope, or merge manually via the GitHub UI.", + ), nil, nil + } + options := &github.PullRequestOptions{ CommitTitle: commitTitle, MergeMethod: mergeMethod, } - client, err := deps.GetClient(ctx) - if err != nil { - return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil - } result, resp, err := client.PullRequests.Merge(ctx, owner, repo, pullNumber, commitMessage, options) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index d2664479d..3d5819061 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -709,6 +709,18 @@ func Test_MergePullRequest(t *testing.T) { SHA: github.Ptr("abcd1234efgh5678"), } + // Setup mock files - no workflow files + mockNonWorkflowFiles := []*github.CommitFile{ + {Filename: github.Ptr("src/main.go")}, + {Filename: github.Ptr("README.md")}, + } + + // Setup mock files - with workflow files + mockWorkflowFiles := []*github.CommitFile{ + {Filename: github.Ptr("src/main.go")}, + {Filename: github.Ptr(".github/workflows/ci.yml")}, + } + tests := []struct { name string mockedClient *http.Client @@ -720,6 +732,7 @@ func Test_MergePullRequest(t *testing.T) { { name: "successful merge", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposPullsFilesByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockNonWorkflowFiles), PutReposPullsMergeByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]interface{}{ "commit_title": "Merge PR #42", "commit_message": "Merging awesome feature", @@ -742,6 +755,7 @@ func Test_MergePullRequest(t *testing.T) { { name: "merge fails", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposPullsFilesByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockNonWorkflowFiles), PutReposPullsMergeByOwnerByRepoByPullNumber: func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusMethodNotAllowed) _, _ = w.Write([]byte(`{"message": "Pull request cannot be merged"}`)) @@ -755,6 +769,36 @@ func Test_MergePullRequest(t *testing.T) { expectError: true, expectedErrMsg: "failed to merge pull request", }, + { + name: "merge blocked by workflow files", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposPullsFilesByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockWorkflowFiles), + }), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + }, + expectError: true, + expectedErrMsg: "workflow", + }, + { + name: "merge with .github but not workflows directory proceeds", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposPullsFilesByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, []*github.CommitFile{ + {Filename: github.Ptr(".github/CODEOWNERS")}, + {Filename: github.Ptr(".github/pull_request_template.md")}, + }), + PutReposPullsMergeByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockMergeResult), + }), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + }, + expectError: false, + expectedMergeResult: mockMergeResult, + }, } for _, tc := range tests { diff --git a/pkg/scopes/scopes.go b/pkg/scopes/scopes.go index a9b06e988..afc77d566 100644 --- a/pkg/scopes/scopes.go +++ b/pkg/scopes/scopes.go @@ -55,6 +55,9 @@ const ( // WritePackages grants write access to packages WritePackages Scope = "write:packages" + + // Workflow grants read and write access to GitHub Actions workflow files + Workflow Scope = "workflow" ) // ScopeHierarchy defines parent-child relationships between scopes.