Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 48 additions & 4 deletions pkg/github/pullrequests.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io"
"net/http"
"strings"

"github.com/go-viper/mapstructure/v2"
"github.com/google/go-github/v79/github"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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,
Expand Down
44 changes: 44 additions & 0 deletions pkg/github/pullrequests_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",
Expand All @@ -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"}`))
Expand All @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions pkg/scopes/scopes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down