fix(lockdown): check bot accounts with [bot] suffix in review comments

pkg/github/lockdown.go

@@ -3,6 +3,7 @@ package github
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/modelcontextprotocol/go-sdk/mcp"
 
@@ -36,3 +37,15 @@ func authorLockdownResult(ctx context.Context, cache *lockdown.RepoAccessCache,
 	}
 	return nil, nil
 }
+
+// actorLoginWithBotSuffix returns the canonical login for a GitHub actor.
+// GraphQL actor queries return bot logins without the "[bot]" suffix (e.g. "my-bot"),
+// while GitHub's REST API and permission endpoints expect the canonical "[bot]" suffix
+// (e.g. "my-bot[bot]"). Built-in trusted identities such as "copilot" are preserved
+// without modification.
+func actorLoginWithBotSuffix(login, typename string) string {
+	if login != "" && typename == "Bot" && !strings.EqualFold(login, "copilot") && !strings.HasSuffix(login, "[bot]") {
+		return login + "[bot]"
+	}
+	return login
+}

pkg/github/pullrequests.go

@@ -508,7 +508,8 @@ type reviewCommentNode struct {
 	StartLine         *githubv4.Int
 	OriginalStartLine *githubv4.Int
 	Author            struct {
-		Login githubv4.String
+		Login    githubv4.String
+		Typename githubv4.String `graphql:"__typename"`
 	}
 	CreatedAt githubv4.DateTime
 	UpdatedAt githubv4.DateTime
@@ -577,7 +578,7 @@ func getPullRequestReviewComments(ctx context.Context, gqlClient *githubv4.Clien
 			filteredComments := make([]reviewCommentNode, 0, len(thread.Comments.Nodes))
 
 			for _, comment := range thread.Comments.Nodes {
-				login := string(comment.Author.Login)
+				login := actorLoginWithBotSuffix(string(comment.Author.Login), string(comment.Author.Typename))
 				if login != "" {
 					isSafeContent, err := cache.IsSafeContent(ctx, login, owner, repo)
 					if err != nil {

pkg/github/pullrequests_test.go

@@ -2238,6 +2238,7 @@ func Test_GetPullRequestComments(t *testing.T) {
 		expectError     bool
 		expectedErrMsg  string
 		lockdownEnabled bool
+		restOverrides   map[string]string
 		validateResult  func(t *testing.T, textContent string)
 	}{
 		{
@@ -2575,6 +2576,143 @@ func Test_GetPullRequestComments(t *testing.T) {
 				assert.Equal(t, "Maintainer review comment", comment.Body)
 			},
 		},
+		{
+			name: "lockdown enabled verifies permissions of bot accounts",
+			gqlHTTPClient: githubv4mock.NewMockedHTTPClient(
+				githubv4mock.NewQueryMatcher(
+					reviewThreadsQuery{},
+					map[string]any{
+						"owner":             githubv4.String("owner"),
+						"repo":              githubv4.String("repo"),
+						"prNum":             githubv4.Int(42),
+						"first":             githubv4.Int(30),
+						"commentsPerThread": githubv4.Int(100),
+						"after":             (*githubv4.String)(nil),
+					},
+					githubv4mock.DataResponse(map[string]any{
+						"repository": map[string]any{
+							"pullRequest": map[string]any{
+								"reviewThreads": map[string]any{
+									"nodes": []map[string]any{
+										{
+											"id":          "RT_kwDOA0xdyM4AX1Yz",
+											"isResolved":  false,
+											"isOutdated":  false,
+											"isCollapsed": false,
+											"comments": map[string]any{
+												"totalCount": 4,
+												"nodes": []map[string]any{
+													{
+														"id":   "PRRC_1",
+														"body": "Maintainer comment",
+														"path": "file1.go",
+														"line": 5,
+														"author": map[string]any{
+															"login":      "maintainer",
+															"__typename": "User",
+														},
+														"createdAt": "2024-01-01T12:00:00Z",
+														"updatedAt": "2024-01-01T12:00:00Z",
+														"url":       "https://github.com/owner/repo/pull/42#discussion_r1",
+													},
+													{
+														"id":   "PRRC_2",
+														"body": "Custom bot comment with write access",
+														"path": "file1.go",
+														"line": 10,
+														"author": map[string]any{
+															"login":      "custom-bot",
+															"__typename": "Bot",
+														},
+														"createdAt": "2024-01-01T13:00:00Z",
+														"updatedAt": "2024-01-01T13:00:00Z",
+														"url":       "https://github.com/owner/repo/pull/42#discussion_r2",
+													},
+													{
+														"id":   "PRRC_3",
+														"body": "Bot comment without write access",
+														"path": "file1.go",
+														"line": 15,
+														"author": map[string]any{
+															"login":      "read-only-bot",
+															"__typename": "Bot",
+														},
+														"createdAt": "2024-01-01T14:00:00Z",
+														"updatedAt": "2024-01-01T14:00:00Z",
+														"url":       "https://github.com/owner/repo/pull/42#discussion_r3",
+													},
+													{
+														"id":   "PRRC_4",
+														"body": "Trusted bot comment",
+														"path": "file1.go",
+														"line": 20,
+														"author": map[string]any{
+															"login":      "github-actions",
+															"__typename": "Bot",
+														},
+														"createdAt": "2024-01-01T15:00:00Z",
+														"updatedAt": "2024-01-01T15:00:00Z",
+														"url":       "https://github.com/owner/repo/pull/42#discussion_r4",
+													},
+													{
+														"id":   "PRRC_5",
+														"body": "Trusted copilot comment",
+														"path": "file1.go",
+														"line": 25,
+														"author": map[string]any{
+															"login":      "copilot",
+															"__typename": "Bot",
+														},
+														"createdAt": "2024-01-01T16:00:00Z",
+														"updatedAt": "2024-01-01T16:00:00Z",
+														"url":       "https://github.com/owner/repo/pull/42#discussion_r5",
+													},
+												},
+											},
+										},
+									},
+									"pageInfo": map[string]any{
+										"hasNextPage":     false,
+										"hasPreviousPage": false,
+										"startCursor":     "cursor1",
+										"endCursor":       "cursor2",
+									},
+									"totalCount": 1,
+								},
+							},
+						},
+					}),
+				),
+			),
+			requestArgs: map[string]any{
+				"method":     "get_review_comments",
+				"owner":      "owner",
+				"repo":       "repo",
+				"pullNumber": float64(42),
+			},
+			expectError:     false,
+			lockdownEnabled: true,
+			restOverrides: map[string]string{
+				"maintainer":         "write",
+				"custom-bot[bot]":    "write",
+				"read-only-bot[bot]": "read",
+			},
+			validateResult: func(t *testing.T, textContent string) {
+				var result MinimalReviewThreadsResponse
+				err := json.Unmarshal([]byte(textContent), &result)
+				require.NoError(t, err)
+
+				assert.Len(t, result.ReviewThreads, 1)
+				thread := result.ReviewThreads[0]
+				assert.Equal(t, 4, thread.TotalCount)
+				assert.Len(t, thread.Comments, 4)
+
+				assert.Equal(t, "maintainer", thread.Comments[0].Author)
+				assert.Equal(t, "custom-bot", thread.Comments[1].Author)
+				assert.Equal(t, "github-actions", thread.Comments[2].Author)
+				assert.Equal(t, "copilot", thread.Comments[3].Author)
+			},
+		},
 	}
 
 	for _, tc := range tests {
@@ -2590,11 +2728,15 @@ func Test_GetPullRequestComments(t *testing.T) {
 			// Setup cache for lockdown mode
 			var restClient *github.Client
 			if tc.lockdownEnabled {
-				restClient = mockRESTPermissionServer(t, "read", map[string]string{
+				overrides := map[string]string{
 					"maintainer":    "write",
 					"external-user": "read",
 					"testuser":      "read",
-				})
+				}
+				if tc.restOverrides != nil {
+					overrides = tc.restOverrides
+				}
+				restClient = mockRESTPermissionServer(t, "read", overrides)
 			}
 			cache := stubRepoAccessCache(restClient, 5*time.Minute)
 

pkg/lockdown/lockdown_test.go

@@ -302,3 +302,55 @@ func TestRepoAccessCacheRejectsEmptyViewerLogin(t *testing.T) {
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "empty")
 }
+
+func TestRepoAccessCache_BotPermissions(t *testing.T) {
+	ctx := t.Context()
+
+	// Server that returns 404 for "custom-bot" but 200 with "write" for "custom-bot[bot]"
+	restServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if strings.Contains(r.URL.Path, "/collaborators/custom-bot/") {
+			http.Error(w, `{"message":"custom-bot is not a user"}`, http.StatusNotFound)
+			return
+		}
+		if strings.Contains(r.URL.Path, "/collaborators/custom-bot[bot]/") {
+			resp := gogithub.RepositoryPermissionLevel{Permission: new("write")}
+			w.Header().Set("Content-Type", "application/json")
+			_ = json.NewEncoder(w).Encode(resp)
+			return
+		}
+		if strings.Contains(r.URL.Path, "/collaborators/read-only-bot[bot]/") {
+			resp := gogithub.RepositoryPermissionLevel{Permission: new("read")}
+			w.Header().Set("Content-Type", "application/json")
+			_ = json.NewEncoder(w).Encode(resp)
+			return
+		}
+		http.Error(w, `{"message":"not found"}`, http.StatusNotFound)
+	}))
+	t.Cleanup(restServer.Close)
+
+	restClient, err := gogithub.NewClient(gogithub.WithEnterpriseURLs(restServer.URL+"/", restServer.URL+"/"))
+	require.NoError(t, err)
+
+	gqlClient, _ := newMockGQLClient("viewer", false)
+	cache := NewRepoAccessCache(gqlClient, restClient, WithCacheName(t.Name()))
+
+	// Trusted bot with [bot] suffix directly succeeds
+	safe, err := cache.IsSafeContent(ctx, "github-actions[bot]", testOwner, testRepo)
+	require.NoError(t, err)
+	require.True(t, safe)
+
+	// Bot with [bot] suffix that has write permission succeeds
+	safe, err = cache.IsSafeContent(ctx, "custom-bot[bot]", testOwner, testRepo)
+	require.NoError(t, err)
+	require.True(t, safe)
+
+	// Bot with [bot] suffix that only has read permission is rejected
+	safe, err = cache.IsSafeContent(ctx, "read-only-bot[bot]", testOwner, testRepo)
+	require.NoError(t, err)
+	require.False(t, safe)
+
+	// Built-in trusted bot "copilot" directly succeeds
+	safe, err = cache.IsSafeContent(ctx, "copilot", testOwner, testRepo)
+	require.NoError(t, err)
+	require.True(t, safe)
+}