Skip to content

Commit 96f6fd3

Browse files
authored
Merge pull request #1 from cbartz/feature/pr-author-allowlist
feat: pr author allowlist
2 parents e2ff518 + 3fa67ab commit 96f6fd3

21 files changed

Lines changed: 567 additions & 5 deletions

README.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1502,6 +1502,18 @@ Following tools will filter out content from users lacking the push access:
15021502
- `pull_request_read:get_review_comments`
15031503
- `pull_request_read:get_reviews`
15041504

1505+
## Pull Request Author Allowlist
1506+
1507+
To restrict mutating pull request tools to bot-authored PRs, use `--allowed-pr-authors` or `GITHUB_ALLOWED_PR_AUTHORS` with a comma-separated list of GitHub logins:
1508+
1509+
```bash
1510+
GITHUB_ALLOWED_PR_AUTHORS='renovate[bot],github-actions[bot]' ./github-mcp-server stdio --toolsets=pull_requests,actions
1511+
```
1512+
1513+
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.
1514+
1515+
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.
1516+
15051517
## i18n / Overriding Descriptions
15061518

15071519
The descriptions of the tools can be overridden by creating a

cmd/github-mcp-server/main.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,13 @@ var (
6969
}
7070
}
7171

72+
var allowedPRAuthors []string
73+
if viper.IsSet("allowed_pr_authors") {
74+
if err := viper.UnmarshalKey("allowed_pr_authors", &allowedPRAuthors); err != nil {
75+
return fmt.Errorf("failed to unmarshal allowed-pr-authors: %w", err)
76+
}
77+
}
78+
7279
// Parse enabled features (similar to toolsets)
7380
var enabledFeatures []string
7481
if viper.IsSet("features") {
@@ -92,6 +99,7 @@ var (
9299
LogFilePath: viper.GetString("log-file"),
93100
ContentWindowSize: viper.GetInt("content-window-size"),
94101
LockdownMode: viper.GetBool("lockdown-mode"),
102+
AllowedPRAuthors: allowedPRAuthors,
95103
InsidersMode: viper.GetBool("insiders"),
96104
ExcludeTools: excludeTools,
97105
RepoAccessCacheTTL: &ttl,
@@ -127,10 +135,18 @@ var (
127135
}
128136
}
129137

138+
var allowedPRAuthors []string
139+
if viper.IsSet("allowed_pr_authors") {
140+
if err := viper.UnmarshalKey("allowed_pr_authors", &allowedPRAuthors); err != nil {
141+
return fmt.Errorf("failed to unmarshal allowed-pr-authors: %w", err)
142+
}
143+
}
144+
130145
ttl := viper.GetDuration("repo-access-cache-ttl")
131146
httpConfig := ghhttp.ServerConfig{
132147
Version: version,
133148
Host: viper.GetString("host"),
149+
Token: viper.GetString("personal_access_token"),
134150
Port: viper.GetInt("port"),
135151
BaseURL: viper.GetString("base-url"),
136152
ResourcePath: viper.GetString("base-path"),
@@ -139,6 +155,7 @@ var (
139155
LogFilePath: viper.GetString("log-file"),
140156
ContentWindowSize: viper.GetInt("content-window-size"),
141157
LockdownMode: viper.GetBool("lockdown-mode"),
158+
AllowedPRAuthors: allowedPRAuthors,
142159
RepoAccessCacheTTL: &ttl,
143160
ScopeChallenge: viper.GetBool("scope-challenge"),
144161
ReadOnly: viper.GetBool("read-only"),
@@ -173,6 +190,7 @@ func init() {
173190
rootCmd.PersistentFlags().String("gh-host", "", "Specify the GitHub hostname (for GitHub Enterprise etc.)")
174191
rootCmd.PersistentFlags().Int("content-window-size", 5000, "Specify the content window size")
175192
rootCmd.PersistentFlags().Bool("lockdown-mode", false, "Enable lockdown mode")
193+
rootCmd.PersistentFlags().StringSlice("allowed-pr-authors", nil, "Comma-separated list of pull request author logins allowed for mutating pull request tools")
176194
rootCmd.PersistentFlags().Bool("insiders", false, "Enable insiders features")
177195
rootCmd.PersistentFlags().Duration("repo-access-cache-ttl", 5*time.Minute, "Override the repo access cache TTL (e.g. 1m, 0s to disable)")
178196

@@ -195,6 +213,7 @@ func init() {
195213
_ = viper.BindPFlag("host", rootCmd.PersistentFlags().Lookup("gh-host"))
196214
_ = viper.BindPFlag("content-window-size", rootCmd.PersistentFlags().Lookup("content-window-size"))
197215
_ = viper.BindPFlag("lockdown-mode", rootCmd.PersistentFlags().Lookup("lockdown-mode"))
216+
_ = viper.BindPFlag("allowed_pr_authors", rootCmd.PersistentFlags().Lookup("allowed-pr-authors"))
198217
_ = viper.BindPFlag("insiders", rootCmd.PersistentFlags().Lookup("insiders"))
199218
_ = viper.BindPFlag("repo-access-cache-ttl", rootCmd.PersistentFlags().Lookup("repo-access-cache-ttl"))
200219
_ = viper.BindPFlag("port", httpCmd.Flags().Lookup("port"))

docs/server-configuration.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ We currently support the following ways in which the GitHub MCP Server can be co
1313
| Read-Only Mode | `X-MCP-Readonly` header or `/readonly` URL | `--read-only` flag or `GITHUB_READ_ONLY` env var |
1414
| Dynamic Mode | Not available | `--dynamic-toolsets` flag or `GITHUB_DYNAMIC_TOOLSETS` env var |
1515
| Lockdown Mode | `X-MCP-Lockdown` header | `--lockdown-mode` flag or `GITHUB_LOCKDOWN_MODE` env var |
16+
| PR Author Allowlist | Server `--allowed-pr-authors` flag or `GITHUB_ALLOWED_PR_AUTHORS` env var | `--allowed-pr-authors` flag or `GITHUB_ALLOWED_PR_AUTHORS` env var |
1617
| Insiders Mode | `X-MCP-Insiders` header or `/insiders` URL | `--insiders` flag or `GITHUB_INSIDERS` env var |
1718
| Feature Flags | `X-MCP-Features` header | `--features` flag |
1819
| Scope Filtering | Always enabled | Always enabled |
@@ -30,6 +31,8 @@ Note: **read-only** mode acts as a strict security filter that takes precedence
3031

3132
Note: **excluded tools** takes precedence over toolsets and individual tools — listed tools are always excluded, even if their toolset is enabled or they are explicitly added via `--tools` / `X-MCP-Tools`.
3233

34+
Note: **PR author allowlist** restricts mutating pull request tools to existing pull requests authored by the configured GitHub logins. Read-only PR tools and `create_pull_request` are not restricted. `actions_run_trigger` is not restricted by this setting because it targets a ref rather than a pull request number.
35+
3336
---
3437

3538
## Configuration Examples
@@ -387,6 +390,33 @@ Lockdown mode ensures the server only surfaces content in public repositories fr
387390

388391
---
389392

393+
### PR Author Allowlist
394+
395+
**Best for:** Automation workflows that may mutate bot-authored pull requests but should never mutate human-authored pull requests.
396+
397+
When set, mutating pull request tools first fetch the target pull request and check `pr.User.Login`. If the author is not in the allowlist, the tool returns an error before making the mutation. Empty or unset means unrestricted behavior.
398+
399+
```json
400+
{
401+
"type": "stdio",
402+
"command": "go",
403+
"args": [
404+
"run",
405+
"./cmd/github-mcp-server",
406+
"stdio",
407+
"--toolsets=pull_requests,actions",
408+
"--allowed-pr-authors=renovate[bot],github-actions[bot]"
409+
],
410+
"env": {
411+
"GITHUB_PERSONAL_ACCESS_TOKEN": "${input:github_token}"
412+
}
413+
}
414+
```
415+
416+
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.
417+
418+
---
419+
390420
### Insiders Mode
391421

392422
**Best for:** Users who want early access to experimental features and new tools before they reach general availability.

docs/streamable-http.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,3 +91,15 @@ To provide PAT credentials, or to customize server behavior preferences, you can
9191
```
9292

9393
See [Remote Server](./remote-server.md) documentation for more details on client configuration options.
94+
95+
### Using a Server-Side Default Token
96+
97+
For trusted local deployments, HTTP mode can use `GITHUB_PERSONAL_ACCESS_TOKEN` as a fallback when a request does not include an `Authorization` header:
98+
99+
```bash
100+
GITHUB_PERSONAL_ACCESS_TOKEN=ghp_yourtokenhere github-mcp-server http
101+
```
102+
103+
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`.
104+
105+
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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se
135135
cfg.ContentWindowSize,
136136
featureChecker,
137137
obs,
138+
cfg.AllowedPRAuthors,
138139
)
139140
// Build and register the tool/resource/prompt inventory
140141
inventoryBuilder := github.NewInventory(cfg.Translator).
@@ -220,6 +221,10 @@ type StdioServerConfig struct {
220221
// LockdownMode indicates if we should enable lockdown mode
221222
LockdownMode bool
222223

224+
// AllowedPRAuthors restricts mutating pull request tools to PRs authored by
225+
// one of these GitHub logins. Empty means unrestricted.
226+
AllowedPRAuthors []string
227+
223228
// InsidersMode indicates if we should enable experimental features
224229
InsidersMode bool
225230

@@ -255,6 +260,9 @@ func RunStdioServer(cfg StdioServerConfig) error {
255260
}
256261
logger := slog.New(slogHandler)
257262
logger.Info("starting server", "version", cfg.Version, "host", cfg.Host, "dynamicToolsets", cfg.DynamicToolsets, "readOnly", cfg.ReadOnly, "lockdownEnabled", cfg.LockdownMode)
263+
if len(cfg.AllowedPRAuthors) > 0 {
264+
logger.Info("PR author allowlist enforced", "authors", cfg.AllowedPRAuthors)
265+
}
258266

259267
// Fetch token scopes for scope-based tool filtering (PAT tokens only)
260268
// Only classic PATs (ghp_ prefix) return OAuth scopes via X-OAuth-Scopes header.
@@ -284,6 +292,7 @@ func RunStdioServer(cfg StdioServerConfig) error {
284292
Translator: t,
285293
ContentWindowSize: cfg.ContentWindowSize,
286294
LockdownMode: cfg.LockdownMode,
295+
AllowedPRAuthors: cfg.AllowedPRAuthors,
287296
InsidersMode: cfg.InsidersMode,
288297
ExcludeTools: cfg.ExcludeTools,
289298
Logger: logger,

pkg/github/copilot.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,10 @@ func RequestCopilotReview(t translations.TranslationHelperFunc) inventory.Server
507507
return utils.NewToolResultError(err.Error()), nil, nil
508508
}
509509

510+
if result, err := enforcePRAuthorAllowlist(ctx, deps, owner, repo, pullNumber, nil); result != nil || err != nil {
511+
return result, nil, err
512+
}
513+
510514
client, err := deps.GetClient(ctx)
511515
if err != nil {
512516
return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil

pkg/github/copilot_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -961,3 +961,31 @@ func Test_RequestCopilotReview(t *testing.T) {
961961
})
962962
}
963963
}
964+
965+
func Test_RequestCopilotReview_PRAuthorDenied(t *testing.T) {
966+
serverTool := RequestCopilotReview(translations.NullTranslationHelper)
967+
client := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
968+
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, &github.PullRequest{
969+
User: &github.User{Login: github.Ptr("alice")},
970+
}),
971+
PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: func(w http.ResponseWriter, _ *http.Request) {
972+
t.Fatal("reviewer request endpoint should not be called when PR author is denied")
973+
},
974+
}))
975+
deps := BaseDeps{
976+
Client: client,
977+
allowedPRAuthors: buildPRAuthorAllowlist([]string{"renovate[bot]"}),
978+
}
979+
handler := serverTool.Handler(deps)
980+
request := createMCPRequest(map[string]any{
981+
"owner": "owner",
982+
"repo": "repo",
983+
"pullNumber": float64(42),
984+
})
985+
986+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
987+
988+
require.NoError(t, err)
989+
require.True(t, result.IsError)
990+
assert.Contains(t, getErrorResult(t, result).Text, `pull request author "alice" is not in --allowed-pr-authors`)
991+
}

pkg/github/dependencies.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,10 @@ type ToolDependencies interface {
105105

106106
// Metrics returns the metrics client
107107
Metrics(ctx context.Context) metrics.Metrics
108+
109+
// IsPRAuthorAllowed checks whether a pull request author is allowed for
110+
// mutating pull request tools. enforced is false when no allowlist is set.
111+
IsPRAuthorAllowed(login string) (allowed bool, enforced bool)
108112
}
109113

110114
// BaseDeps is the standard implementation of ToolDependencies for the local server.
@@ -127,6 +131,8 @@ type BaseDeps struct {
127131

128132
// Observability exporters (includes logger)
129133
Obsv observability.Exporters
134+
135+
allowedPRAuthors map[string]struct{}
130136
}
131137

132138
// Compile-time assertion to verify that BaseDeps implements the ToolDependencies interface.
@@ -143,6 +149,7 @@ func NewBaseDeps(
143149
contentWindowSize int,
144150
featureChecker inventory.FeatureFlagChecker,
145151
obsv observability.Exporters,
152+
allowedPRAuthors ...[]string,
146153
) *BaseDeps {
147154
return &BaseDeps{
148155
Client: client,
@@ -154,6 +161,7 @@ func NewBaseDeps(
154161
ContentWindowSize: contentWindowSize,
155162
featureChecker: featureChecker,
156163
Obsv: obsv,
164+
allowedPRAuthors: buildPRAuthorAllowlist(firstStringSlice(allowedPRAuthors)),
157165
}
158166
}
159167

@@ -196,6 +204,11 @@ func (d BaseDeps) Metrics(ctx context.Context) metrics.Metrics {
196204
return d.Obsv.Metrics(ctx)
197205
}
198206

207+
// IsPRAuthorAllowed implements ToolDependencies.
208+
func (d BaseDeps) IsPRAuthorAllowed(login string) (bool, bool) {
209+
return isPRAuthorAllowed(d.allowedPRAuthors, login)
210+
}
211+
199212
// IsFeatureEnabled checks if a feature flag is enabled.
200213
// Returns false if the feature checker is nil, flag name is empty, or an error occurs.
201214
// This allows tools to conditionally change behavior based on feature flags.
@@ -276,6 +289,8 @@ type RequestDeps struct {
276289

277290
// Observability exporters (includes logger)
278291
obsv observability.Exporters
292+
293+
allowedPRAuthors map[string]struct{}
279294
}
280295

281296
// NewRequestDeps creates a RequestDeps with the provided clients and configuration.
@@ -288,6 +303,7 @@ func NewRequestDeps(
288303
contentWindowSize int,
289304
featureChecker inventory.FeatureFlagChecker,
290305
obsv observability.Exporters,
306+
allowedPRAuthors ...[]string,
291307
) *RequestDeps {
292308
return &RequestDeps{
293309
apiHosts: apiHosts,
@@ -298,6 +314,7 @@ func NewRequestDeps(
298314
ContentWindowSize: contentWindowSize,
299315
featureChecker: featureChecker,
300316
obsv: obsv,
317+
allowedPRAuthors: buildPRAuthorAllowlist(firstStringSlice(allowedPRAuthors)),
301318
}
302319
}
303320

@@ -420,6 +437,11 @@ func (d *RequestDeps) Metrics(ctx context.Context) metrics.Metrics {
420437
return d.obsv.Metrics(ctx)
421438
}
422439

440+
// IsPRAuthorAllowed implements ToolDependencies.
441+
func (d *RequestDeps) IsPRAuthorAllowed(login string) (bool, bool) {
442+
return isPRAuthorAllowed(d.allowedPRAuthors, login)
443+
}
444+
423445
// IsFeatureEnabled checks if a feature flag is enabled.
424446
func (d *RequestDeps) IsFeatureEnabled(ctx context.Context, flagName string) bool {
425447
if d.featureChecker == nil || flagName == "" {

pkg/github/issues.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,10 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool
663663
if err != nil {
664664
return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil
665665
}
666+
if result, err := enforceIssueCommentPRAuthorAllowlist(ctx, deps, client, owner, repo, issueNumber); result != nil || err != nil {
667+
return result, nil, err
668+
}
669+
666670
createdComment, resp, err := client.Issues.CreateComment(ctx, owner, repo, issueNumber, comment)
667671
if err != nil {
668672
return utils.NewToolResultErrorFromErr("failed to create comment", err), nil, nil

pkg/github/issues_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,41 @@ func Test_AddIssueComment(t *testing.T) {
386386
}
387387
}
388388

389+
func Test_AddIssueComment_PRAuthorDenied(t *testing.T) {
390+
serverTool := AddIssueComment(translations.NullTranslationHelper)
391+
client := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
392+
GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, &github.Issue{
393+
Number: github.Ptr(42),
394+
PullRequestLinks: &github.PullRequestLinks{
395+
URL: github.Ptr("https://api.github.com/repos/owner/repo/pulls/42"),
396+
},
397+
}),
398+
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, &github.PullRequest{
399+
User: &github.User{Login: github.Ptr("alice")},
400+
}),
401+
PostReposIssuesCommentsByOwnerByRepoByIssueNumber: func(w http.ResponseWriter, _ *http.Request) {
402+
t.Fatal("issue comment endpoint should not be called when PR author is denied")
403+
},
404+
}))
405+
deps := BaseDeps{
406+
Client: client,
407+
allowedPRAuthors: buildPRAuthorAllowlist([]string{"renovate[bot]"}),
408+
}
409+
handler := serverTool.Handler(deps)
410+
request := createMCPRequest(map[string]any{
411+
"owner": "owner",
412+
"repo": "repo",
413+
"issue_number": float64(42),
414+
"body": "comment",
415+
})
416+
417+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
418+
419+
require.NoError(t, err)
420+
require.True(t, result.IsError)
421+
assert.Contains(t, getErrorResult(t, result).Text, `pull request author "alice" is not in --allowed-pr-authors`)
422+
}
423+
389424
func Test_SearchIssues(t *testing.T) {
390425
// Verify tool definition once
391426
serverTool := SearchIssues(translations.NullTranslationHelper)

0 commit comments

Comments
 (0)