fix: close PR allowlist mutation gaps

Author: cbartz

README.md

@@ -1512,7 +1512,7 @@ GITHUB_ALLOWED_PR_AUTHORS='renovate[bot],github-actions[bot]' ./github-mcp-serve
 
 When set, tools such as `merge_pull_request`, `update_pull_request`, review-write tools, and PR branch updates fetch the target PR and reject the call unless `pr.User.Login` is in the allowlist. Read-only PR tools and `create_pull_request` are not restricted. `actions_run_trigger` is not gated by this setting because it targets a ref rather than a PR number.
 
-In HTTP mode, `GITHUB_PERSONAL_ACCESS_TOKEN` can also be used as a server-side default token for trusted local deployments. Requests with an `Authorization` header still use the request token; requests without one fall back to the configured server token.
+In HTTP mode, `GITHUB_PERSONAL_ACCESS_TOKEN` can also be used as a server-side default token for trusted local deployments. Requests with an `Authorization` header still use the request token; requests without one fall back to the configured server token. This means the server's GitHub identity is used for any unauthenticated HTTP request, so only enable this when the HTTP endpoint is on a trusted network.
 
 ## i18n / Overriding Descriptions
 

docs/server-configuration.md

@@ -413,7 +413,7 @@ When set, mutating pull request tools first fetch the target pull request and ch
 }
 ```
 
-Known limitations: `actions_run_trigger` operates on refs, not pull request numbers, so it is not gated by this setting. The allowlist checks `pr.User.Login`; PRs from forks authored by allowed bots still pass. Enabling the allowlist adds one API call before a mutating PR operation when the handler does not already have the pull request.
+Known limitations: `actions_run_trigger` operates on refs, not pull request numbers, so it is not gated by this setting. Review-thread resolve and unresolve tools take only opaque thread IDs and are not gated by the PR author allowlist. The allowlist checks `pr.User.Login`; PRs from forks authored by allowed bots still pass. Enabling the allowlist adds one API call before a mutating PR operation when the handler does not already have the pull request.
 
 ---
 

docs/streamable-http.md

@@ -101,3 +101,5 @@ GITHUB_PERSONAL_ACCESS_TOKEN=ghp_yourtokenhere github-mcp-server http
 ```
 
 If a request includes `Authorization: Bearer ...`, that request token takes precedence. If no request token is provided and no server-side token is configured, the server returns `401 Unauthorized`.
+
+When this fallback is enabled, the server's GitHub identity is used for every HTTP request without an `Authorization` header. Only expose the endpoint on a trusted network.

internal/ghmcp/server.go

@@ -260,6 +260,9 @@ func RunStdioServer(cfg StdioServerConfig) error {
 	}
 	logger := slog.New(slogHandler)
 	logger.Info("starting server", "version", cfg.Version, "host", cfg.Host, "dynamicToolsets", cfg.DynamicToolsets, "readOnly", cfg.ReadOnly, "lockdownEnabled", cfg.LockdownMode)
+	if len(cfg.AllowedPRAuthors) > 0 {
+		logger.Info("PR author allowlist enforced", "authors", cfg.AllowedPRAuthors)
+	}
 
 	// Fetch token scopes for scope-based tool filtering (PAT tokens only)
 	// Only classic PATs (ghp_ prefix) return OAuth scopes via X-OAuth-Scopes header.

pkg/github/copilot.go

@@ -507,6 +507,10 @@ func RequestCopilotReview(t translations.TranslationHelperFunc) inventory.Server
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
 
+			if result, err := enforcePRAuthorAllowlist(ctx, deps, owner, repo, pullNumber, nil); result != nil || err != nil {
+				return result, nil, err
+			}
+
 			client, err := deps.GetClient(ctx)
 			if err != nil {
 				return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil

pkg/github/copilot_test.go

@@ -961,3 +961,31 @@ func Test_RequestCopilotReview(t *testing.T) {
 		})
 	}
 }
+
+func Test_RequestCopilotReview_PRAuthorDenied(t *testing.T) {
+	serverTool := RequestCopilotReview(translations.NullTranslationHelper)
+	client := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+		GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, &github.PullRequest{
+			User: &github.User{Login: github.Ptr("alice")},
+		}),
+		PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: func(w http.ResponseWriter, _ *http.Request) {
+			t.Fatal("reviewer request endpoint should not be called when PR author is denied")
+		},
+	}))
+	deps := BaseDeps{
+		Client:           client,
+		allowedPRAuthors: buildPRAuthorAllowlist([]string{"renovate[bot]"}),
+	}
+	handler := serverTool.Handler(deps)
+	request := createMCPRequest(map[string]any{
+		"owner":      "owner",
+		"repo":       "repo",
+		"pullNumber": float64(42),
+	})
+
+	result, err := handler(ContextWithDeps(context.Background(), deps), &request)
+
+	require.NoError(t, err)
+	require.True(t, result.IsError)
+	assert.Contains(t, getErrorResult(t, result).Text, `pull request author "alice" is not in --allowed-pr-authors`)
+}

pkg/github/issues.go

@@ -663,6 +663,10 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool
 			if err != nil {
 				return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil
 			}
+			if result, err := enforceIssueCommentPRAuthorAllowlist(ctx, deps, client, owner, repo, issueNumber); result != nil || err != nil {
+				return result, nil, err
+			}
+
 			createdComment, resp, err := client.Issues.CreateComment(ctx, owner, repo, issueNumber, comment)
 			if err != nil {
 				return utils.NewToolResultErrorFromErr("failed to create comment", err), nil, nil

pkg/github/issues_test.go

@@ -386,6 +386,41 @@ func Test_AddIssueComment(t *testing.T) {
 	}
 }
 
+func Test_AddIssueComment_PRAuthorDenied(t *testing.T) {
+	serverTool := AddIssueComment(translations.NullTranslationHelper)
+	client := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+		GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &github.Issue{
+			Number: github.Ptr(42),
+			PullRequestLinks: &github.PullRequestLinks{
+				URL: github.Ptr("https://api.github.com/repos/owner/repo/pulls/42"),
+			},
+		}),
+		GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, &github.PullRequest{
+			User: &github.User{Login: github.Ptr("alice")},
+		}),
+		PostReposIssuesCommentsByOwnerByRepoByIssueNumber: func(w http.ResponseWriter, _ *http.Request) {
+			t.Fatal("issue comment endpoint should not be called when PR author is denied")
+		},
+	}))
+	deps := BaseDeps{
+		Client:           client,
+		allowedPRAuthors: buildPRAuthorAllowlist([]string{"renovate[bot]"}),
+	}
+	handler := serverTool.Handler(deps)
+	request := createMCPRequest(map[string]any{
+		"owner":        "owner",
+		"repo":         "repo",
+		"issue_number": float64(42),
+		"body":         "comment",
+	})
+
+	result, err := handler(ContextWithDeps(context.Background(), deps), &request)
+
+	require.NoError(t, err)
+	require.True(t, result.IsError)
+	assert.Contains(t, getErrorResult(t, result).Text, `pull request author "alice" is not in --allowed-pr-authors`)
+}
+
 func Test_SearchIssues(t *testing.T) {
 	// Verify tool definition once
 	serverTool := SearchIssues(translations.NullTranslationHelper)

pkg/github/pr_author_allowlist.go

@@ -55,9 +55,7 @@ func enforcePRAuthorAllowlist(
 	pullNumber int,
 	pr *gogithub.PullRequest,
 ) (*mcp.CallToolResult, error) {
-	if allowed, enforced := deps.IsPRAuthorAllowed(""); !enforced {
-		return nil, nil
-	} else if allowed {
+	if _, enforced := deps.IsPRAuthorAllowed(""); !enforced {
 		return nil, nil
 	}
 
@@ -82,5 +80,50 @@ func enforcePRAuthorAllowlist(
 		return nil, nil
 	}
 
+	logPRAuthorAllowlistDenied(ctx, deps, owner, repo, pullNumber, login)
 	return utils.NewToolResultError(fmt.Sprintf("pull request author %q is not in --allowed-pr-authors", login)), nil
 }
+
+// enforceIssueCommentPRAuthorAllowlist applies the PR author allowlist when
+// an issue-comment target is actually a pull request.
+func enforceIssueCommentPRAuthorAllowlist(
+	ctx context.Context,
+	deps ToolDependencies,
+	client *gogithub.Client,
+	owner, repo string,
+	issueNumber int,
+) (*mcp.CallToolResult, error) {
+	if _, enforced := deps.IsPRAuthorAllowed(""); !enforced {
+		return nil, nil
+	}
+
+	issue, resp, err := client.Issues.Get(ctx, owner, repo, issueNumber)
+	if err != nil {
+		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get issue", resp, err), nil
+	}
+	if resp != nil && resp.Body != nil {
+		defer func() { _ = resp.Body.Close() }()
+	}
+
+	if issue.GetPullRequestLinks() == nil {
+		return nil, nil
+	}
+
+	return enforcePRAuthorAllowlist(ctx, deps, owner, repo, issueNumber, nil)
+}
+
+func logPRAuthorAllowlistDenied(ctx context.Context, deps ToolDependencies, owner, repo string, pullNumber int, login string) {
+	defer func() {
+		_ = recover()
+	}()
+
+	if logger := deps.Logger(ctx); logger != nil {
+		logger.Warn(
+			"PR mutation denied by allowlist",
+			"owner", owner,
+			"repo", repo,
+			"pr", pullNumber,
+			"author", login,
+		)
+	}
+}

pkg/http/server.go

@@ -119,6 +119,12 @@ func RunHTTPServer(cfg ServerConfig) error {
 	}
 	logger := slog.New(slogHandler)
 	logger.Info("starting server", "version", cfg.Version, "host", cfg.Host, "lockdownEnabled", cfg.LockdownMode, "readOnly", cfg.ReadOnly, "insidersMode", cfg.InsidersMode)
+	if len(cfg.AllowedPRAuthors) > 0 {
+		logger.Info("PR author allowlist enforced", "authors", cfg.AllowedPRAuthors)
+	}
+	if cfg.Token != "" {
+		logger.Warn("HTTP default token fallback enabled; requests without Authorization use the server token. Only expose this endpoint on a trusted network.")
+	}
 
 	apiHost, err := utils.NewAPIHost(cfg.Host)
 	if err != nil {