centralise lockdown checks and default fail closed on empty author

pkg/github/issues.go

@@ -734,18 +734,8 @@ func GetIssue(ctx context.Context, client *github.Client, deps ToolDependencies,
 	}
 
 	if flags.LockdownMode {
-		if cache == nil {
-			return nil, fmt.Errorf("lockdown cache is not configured")
-		}
-		login := issue.GetUser().GetLogin()
-		if login != "" {
-			isSafeContent, err := cache.IsSafeContent(ctx, login, owner, repo)
-			if err != nil {
-				return utils.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil
-			}
-			if !isSafeContent {
-				return utils.NewToolResultError("access to issue details is restricted by lockdown mode"), nil
-			}
+		if restricted, err := authorLockdownResult(ctx, cache, owner, repo, issue.GetUser().GetLogin(), lockdownIssueRestrictedMessage); restricted != nil || err != nil {
+			return restricted, err
 		}
 	}
 

pkg/github/lockdown.go

@@ -0,0 +1,38 @@
+package github
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/modelcontextprotocol/go-sdk/mcp"
+
+	"github.com/github/github-mcp-server/pkg/lockdown"
+	"github.com/github/github-mcp-server/pkg/utils"
+)
+
+// Restriction messages returned when lockdown mode withholds content from a read tool.
+const (
+	lockdownPullRequestRestrictedMessage = "access to pull request is restricted by lockdown mode"
+	lockdownIssueRestrictedMessage       = "access to issue details is restricted by lockdown mode"
+)
+
+// authorLockdownResult returns a restricted tool result when content authored by
+// authorLogin cannot be surfaced for owner/repo under lockdown mode, and (nil, nil)
+// when access is permitted. It should only be called when lockdown mode is enabled.
+// It fails closed: a missing cache, an empty author, or a lookup error denies access.
+func authorLockdownResult(ctx context.Context, cache *lockdown.RepoAccessCache, owner, repo, authorLogin, restrictedMessage string) (*mcp.CallToolResult, error) {
+	if cache == nil {
+		return nil, fmt.Errorf("lockdown cache is not configured")
+	}
+	if authorLogin == "" {
+		return utils.NewToolResultError(restrictedMessage), nil
+	}
+	isSafeContent, err := cache.IsSafeContent(ctx, authorLogin, owner, repo)
+	if err != nil {
+		return utils.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil
+	}
+	if !isSafeContent {
+		return utils.NewToolResultError(restrictedMessage), nil
+	}
+	return nil, nil
+}

pkg/github/lockdown_test.go

@@ -0,0 +1,29 @@
+package github
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func Test_authorLockdownResult(t *testing.T) {
+	t.Parallel()
+
+	t.Run("missing cache returns error", func(t *testing.T) {
+		result, err := authorLockdownResult(context.Background(), nil, "owner", "repo", "author", lockdownIssueRestrictedMessage)
+		require.Error(t, err)
+		assert.Nil(t, result)
+	})
+
+	t.Run("empty author fails closed", func(t *testing.T) {
+		cache := stubRepoAccessCache(nil, time.Minute)
+		result, err := authorLockdownResult(context.Background(), cache, "owner", "repo", "", lockdownIssueRestrictedMessage)
+		require.NoError(t, err)
+		require.NotNil(t, result)
+		assert.True(t, result.IsError)
+		assert.Contains(t, getErrorResult(t, result).Text, lockdownIssueRestrictedMessage)
+	})
+}

pkg/github/pullrequests.go

@@ -196,19 +196,8 @@ func GetPullRequest(ctx context.Context, client *github.Client, deps ToolDepende
 	}
 
 	if ff.LockdownMode {
-		if cache == nil {
-			return nil, fmt.Errorf("lockdown cache is not configured")
-		}
-		login := pr.GetUser().GetLogin()
-		if login != "" {
-			isSafeContent, err := cache.IsSafeContent(ctx, login, owner, repo)
-			if err != nil {
-				return nil, fmt.Errorf("failed to check content removal: %w", err)
-			}
-
-			if !isSafeContent {
-				return utils.NewToolResultError("access to pull request is restricted by lockdown mode"), nil
-			}
+		if restricted, err := authorLockdownResult(ctx, cache, owner, repo, pr.GetUser().GetLogin(), lockdownPullRequestRestrictedMessage); restricted != nil || err != nil {
+			return restricted, err
 		}
 	}
 

pkg/github/pullrequests_test.go

@@ -53,12 +53,14 @@ func Test_GetPullRequest(t *testing.T) {
 	}
 
 	tests := []struct {
-		name           string
-		mockedClient   *http.Client
-		requestArgs    map[string]any
-		expectError    bool
-		expectedPR     *github.PullRequest
-		expectedErrMsg string
+		name            string
+		mockedClient    *http.Client
+		requestArgs     map[string]any
+		expectError     bool
+		expectedPR      *github.PullRequest
+		expectedErrMsg  string
+		lockdownEnabled bool
+		restPermission  string
 	}{
 		{
 			name: "successful PR fetch",
@@ -91,6 +93,38 @@ func Test_GetPullRequest(t *testing.T) {
 			expectError:    true,
 			expectedErrMsg: "failed to get pull request",
 		},
+		{
+			name: "lockdown enabled - user lacks push access",
+			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+				GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPR),
+			}),
+			requestArgs: map[string]any{
+				"method":     "get",
+				"owner":      "owner",
+				"repo":       "repo",
+				"pullNumber": float64(42),
+			},
+			expectError:     true,
+			expectedErrMsg:  "access to pull request is restricted by lockdown mode",
+			lockdownEnabled: true,
+			restPermission:  "read",
+		},
+		{
+			name: "lockdown enabled - private repository",
+			mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+				GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPR),
+			}),
+			requestArgs: map[string]any{
+				"method":     "get",
+				"owner":      "owner2",
+				"repo":       "repo2",
+				"pullNumber": float64(42),
+			},
+			expectError:     false,
+			expectedPR:      mockPR,
+			lockdownEnabled: true,
+			restPermission:  "none",
+		},
 	}
 
 	for _, tc := range tests {
@@ -98,11 +132,17 @@ func Test_GetPullRequest(t *testing.T) {
 			// Setup client with mock
 			client := mustNewGHClient(t, tc.mockedClient)
 			gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient())
+
+			var restClient *github.Client
+			if tc.restPermission != "" {
+				restClient = mockRESTPermissionServer(t, tc.restPermission, nil)
+			}
+
 			deps := BaseDeps{
 				Client:          client,
 				GQLClient:       gqlClient,
-				RepoAccessCache: stubRepoAccessCache(nil, 5*time.Minute),
-				Flags:           stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
+				RepoAccessCache: stubRepoAccessCache(restClient, 5*time.Minute),
+				Flags:           stubFeatureFlags(map[string]bool{"lockdown-mode": tc.lockdownEnabled}),
 			}
 			handler := serverTool.Handler(deps)