Skip to content

Commit 28b868d

Browse files
JoannaaKLCopilot
andauthored
Add in memory cache for lockdown mode (#1416)
* Apply lockdown mode to issues and pull requests * Add cache * Unlock in defer * Add muesli/cache2go * [WIP] Replace custom cache in lockdown.go with cache2go struct (#1425) * Initial plan * Replace custom cache with cache2go library - Added github.com/muesli/cache2go dependency - Replaced custom map-based cache with cache2go.CacheTable - Removed manual timer management (scheduleExpiry, ensureEntry methods) - Removed timer field from repoAccessCacheEntry struct - Updated GetRepoAccessInfo to use cache2go's Value() and Add() methods - Updated SetTTL to flush and re-add entries with new TTL - Used unique cache names per instance to avoid test interference - All existing tests pass with the new implementation Co-authored-by: JoannaaKL <[email protected]> * Final verification complete Co-authored-by: JoannaaKL <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: JoannaaKL <[email protected]> * Use muesli for cache * Make RepoAccessCache a singleton (#1426) * Initial plan * Implement RepoAccessCache as a singleton pattern Co-authored-by: JoannaaKL <[email protected]> * Complete singleton implementation and verification Co-authored-by: JoannaaKL <[email protected]> * Remove cacheIDCounter as requested Co-authored-by: JoannaaKL <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: JoannaaKL <[email protected]> * Update mutexes * . * Reuse cache * . * . * Fix logic after vibe coding * Update docs * . * Refactoring to make the code pretty * Hide lockdown logic behind shouldFilter function * . * Tests --------- Co-authored-by: Copilot <[email protected]>
1 parent ec6afa7 commit 28b868d

File tree

18 files changed

+846
-107
lines changed

18 files changed

+846
-107
lines changed

README.md

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,7 +1264,7 @@ docker run -i --rm \
12641264

12651265
## Lockdown Mode
12661266

1267-
Lockdown mode limits the content that the server will surface from public repositories. When enabled, requests that fetch issue details will return an error if the issue was created by someone who does not have push access to the repository. Private repositories are unaffected, and collaborators can still access their own issues.
1267+
Lockdown mode limits the content that the server will surface from public repositories. When enabled, the server checks whether the author of each item has push access to the repository. Private repositories are unaffected, and collaborators keep full access to their own content.
12681268

12691269
```bash
12701270
./github-mcp-server --lockdown-mode
@@ -1279,7 +1279,20 @@ docker run -i --rm \
12791279
ghcr.io/github/github-mcp-server
12801280
```
12811281

1282-
At the moment lockdown mode applies to the issue read toolset, but it is designed to extend to additional data surfaces over time.
1282+
The behavior of lockdown mode depends on the tool invoked.
1283+
1284+
Following tools will return an error when the author lacks the push access:
1285+
1286+
- `issue_read:get`
1287+
- `pull_request_read:get`
1288+
1289+
Following tools will filter out content from users lacking the push access:
1290+
1291+
- `issue_read:get_comments`
1292+
- `issue_read:get_sub_issues`
1293+
- `pull_request_read:get_comments`
1294+
- `pull_request_read:get_review_comments`
1295+
- `pull_request_read:get_reviews`
12831296

12841297
## i18n / Overriding Descriptions
12851298

cmd/github-mcp-server/generate_docs.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"strings"
1111

1212
"github.com/github/github-mcp-server/pkg/github"
13+
"github.com/github/github-mcp-server/pkg/lockdown"
1314
"github.com/github/github-mcp-server/pkg/raw"
1415
"github.com/github/github-mcp-server/pkg/toolsets"
1516
"github.com/github/github-mcp-server/pkg/translations"
@@ -64,7 +65,8 @@ func generateReadmeDocs(readmePath string) error {
6465
t, _ := translations.TranslationHelper()
6566

6667
// Create toolset group with mock clients
67-
tsg := github.DefaultToolsetGroup(false, mockGetClient, mockGetGQLClient, mockGetRawClient, t, 5000, github.FeatureFlags{})
68+
repoAccessCache := lockdown.GetInstance(nil)
69+
tsg := github.DefaultToolsetGroup(false, mockGetClient, mockGetGQLClient, mockGetRawClient, t, 5000, github.FeatureFlags{}, repoAccessCache)
6870

6971
// Generate toolsets documentation
7072
toolsetsDoc := generateToolsetsDoc(tsg)
@@ -302,7 +304,8 @@ func generateRemoteToolsetsDoc() string {
302304
t, _ := translations.TranslationHelper()
303305

304306
// Create toolset group with mock clients
305-
tsg := github.DefaultToolsetGroup(false, mockGetClient, mockGetGQLClient, mockGetRawClient, t, 5000, github.FeatureFlags{})
307+
repoAccessCache := lockdown.GetInstance(nil)
308+
tsg := github.DefaultToolsetGroup(false, mockGetClient, mockGetGQLClient, mockGetRawClient, t, 5000, github.FeatureFlags{}, repoAccessCache)
306309

307310
// Generate table header
308311
buf.WriteString("| Name | Description | API URL | 1-Click Install (VS Code) | Read-only Link | 1-Click Read-only Install (VS Code) |\n")

cmd/github-mcp-server/main.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"os"
77
"strings"
8+
"time"
89

910
"github.com/github/github-mcp-server/internal/ghmcp"
1011
"github.com/github/github-mcp-server/pkg/github"
@@ -50,6 +51,7 @@ var (
5051
enabledToolsets = []string{github.ToolsetMetadataDefault.ID}
5152
}
5253

54+
ttl := viper.GetDuration("repo-access-cache-ttl")
5355
stdioServerConfig := ghmcp.StdioServerConfig{
5456
Version: version,
5557
Host: viper.GetString("host"),
@@ -62,6 +64,7 @@ var (
6264
LogFilePath: viper.GetString("log-file"),
6365
ContentWindowSize: viper.GetInt("content-window-size"),
6466
LockdownMode: viper.GetBool("lockdown-mode"),
67+
RepoAccessCacheTTL: &ttl,
6568
}
6669
return ghmcp.RunStdioServer(stdioServerConfig)
6770
},
@@ -84,6 +87,7 @@ func init() {
8487
rootCmd.PersistentFlags().String("gh-host", "", "Specify the GitHub hostname (for GitHub Enterprise etc.)")
8588
rootCmd.PersistentFlags().Int("content-window-size", 5000, "Specify the content window size")
8689
rootCmd.PersistentFlags().Bool("lockdown-mode", false, "Enable lockdown mode")
90+
rootCmd.PersistentFlags().Duration("repo-access-cache-ttl", 5*time.Minute, "Override the repo access cache TTL (e.g. 1m, 0s to disable)")
8791

8892
// Bind flag to viper
8993
_ = viper.BindPFlag("toolsets", rootCmd.PersistentFlags().Lookup("toolsets"))
@@ -95,6 +99,7 @@ func init() {
9599
_ = viper.BindPFlag("host", rootCmd.PersistentFlags().Lookup("gh-host"))
96100
_ = viper.BindPFlag("content-window-size", rootCmd.PersistentFlags().Lookup("content-window-size"))
97101
_ = viper.BindPFlag("lockdown-mode", rootCmd.PersistentFlags().Lookup("lockdown-mode"))
102+
_ = viper.BindPFlag("repo-access-cache-ttl", rootCmd.PersistentFlags().Lookup("repo-access-cache-ttl"))
98103

99104
// Add subcommands
100105
rootCmd.AddCommand(stdioCmd)

go.mod

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ require (
88
github.com/mark3labs/mcp-go v0.36.0
99
github.com/microcosm-cc/bluemonday v1.0.27
1010
github.com/migueleliasweb/go-github-mock v1.3.0
11+
github.com/muesli/cache2go v0.0.0-20221011235721-518229cd8021
1112
github.com/spf13/cobra v1.10.1
1213
github.com/spf13/viper v1.21.0
1314
github.com/stretchr/testify v1.11.1
@@ -37,7 +38,7 @@ require (
3738
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
3839
github.com/fsnotify/fsnotify v1.9.0 // indirect
3940
github.com/go-viper/mapstructure/v2 v2.4.0
40-
github.com/google/go-querystring v1.1.0
41+
github.com/google/go-querystring v1.1.0 // indirect
4142
github.com/google/uuid v1.6.0 // indirect
4243
github.com/inconshreveable/mousetrap v1.1.0 // indirect
4344
github.com/pelletier/go-toml/v2 v2.2.4 // indirect

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ github.com/microcosm-cc/bluemonday v1.0.27 h1:MpEUotklkwCSLeH+Qdx1VJgNqLlpY2KXwX
6363
github.com/microcosm-cc/bluemonday v1.0.27/go.mod h1:jFi9vgW+H7c3V0lb6nR74Ib/DIB5OBs92Dimizgw2cA=
6464
github.com/migueleliasweb/go-github-mock v1.3.0 h1:2sVP9JEMB2ubQw1IKto3/fzF51oFC6eVWOOFDgQoq88=
6565
github.com/migueleliasweb/go-github-mock v1.3.0/go.mod h1:ipQhV8fTcj/G6m7BKzin08GaJ/3B5/SonRAkgrk0zCY=
66+
github.com/muesli/cache2go v0.0.0-20221011235721-518229cd8021 h1:31Y+Yu373ymebRdJN1cWLLooHH8xAr0MhKTEJGV/87g=
67+
github.com/muesli/cache2go v0.0.0-20221011235721-518229cd8021/go.mod h1:WERUkUryfUWlrHnFSO/BEUZ+7Ns8aZy7iVOGewxKzcc=
6668
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
6769
github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4=
6870
github.com/pelletier/go-toml/v2 v2.2.4/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY=

internal/ghmcp/server.go

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
"github.com/github/github-mcp-server/pkg/errors"
1818
"github.com/github/github-mcp-server/pkg/github"
19+
"github.com/github/github-mcp-server/pkg/lockdown"
1920
mcplog "github.com/github/github-mcp-server/pkg/log"
2021
"github.com/github/github-mcp-server/pkg/raw"
2122
"github.com/github/github-mcp-server/pkg/translations"
@@ -54,6 +55,9 @@ type MCPServerConfig struct {
5455

5556
// LockdownMode indicates if we should enable lockdown mode
5657
LockdownMode bool
58+
59+
// RepoAccessTTL overrides the default TTL for repository access cache entries.
60+
RepoAccessTTL *time.Duration
5761
}
5862

5963
const stdioServerLogPrefix = "stdioserver"
@@ -80,6 +84,14 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
8084
},
8185
} // We're going to wrap the Transport later in beforeInit
8286
gqlClient := githubv4.NewEnterpriseClient(apiHost.graphqlURL.String(), gqlHTTPClient)
87+
repoAccessOpts := []lockdown.RepoAccessOption{}
88+
if cfg.RepoAccessTTL != nil {
89+
repoAccessOpts = append(repoAccessOpts, lockdown.WithTTL(*cfg.RepoAccessTTL))
90+
}
91+
var repoAccessCache *lockdown.RepoAccessCache
92+
if cfg.LockdownMode {
93+
repoAccessCache = lockdown.GetInstance(gqlClient, repoAccessOpts...)
94+
}
8395

8496
// When a client send an initialize request, update the user agent to include the client info.
8597
beforeInit := func(_ context.Context, _ any, message *mcp.InitializeRequest) {
@@ -165,6 +177,7 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
165177
cfg.Translator,
166178
cfg.ContentWindowSize,
167179
github.FeatureFlags{LockdownMode: cfg.LockdownMode},
180+
repoAccessCache,
168181
)
169182
err = tsg.EnableToolsets(enabledToolsets, nil)
170183

@@ -219,6 +232,9 @@ type StdioServerConfig struct {
219232

220233
// LockdownMode indicates if we should enable lockdown mode
221234
LockdownMode bool
235+
236+
// RepoAccessCacheTTL overrides the default TTL for repository access cache entries.
237+
RepoAccessCacheTTL *time.Duration
222238
}
223239

224240
// RunStdioServer is not concurrent safe.
@@ -229,23 +245,6 @@ func RunStdioServer(cfg StdioServerConfig) error {
229245

230246
t, dumpTranslations := translations.TranslationHelper()
231247

232-
ghServer, err := NewMCPServer(MCPServerConfig{
233-
Version: cfg.Version,
234-
Host: cfg.Host,
235-
Token: cfg.Token,
236-
EnabledToolsets: cfg.EnabledToolsets,
237-
DynamicToolsets: cfg.DynamicToolsets,
238-
ReadOnly: cfg.ReadOnly,
239-
Translator: t,
240-
ContentWindowSize: cfg.ContentWindowSize,
241-
LockdownMode: cfg.LockdownMode,
242-
})
243-
if err != nil {
244-
return fmt.Errorf("failed to create MCP server: %w", err)
245-
}
246-
247-
stdioServer := server.NewStdioServer(ghServer)
248-
249248
var slogHandler slog.Handler
250249
var logOutput io.Writer
251250
if cfg.LogFilePath != "" {
@@ -262,6 +261,24 @@ func RunStdioServer(cfg StdioServerConfig) error {
262261
logger := slog.New(slogHandler)
263262
logger.Info("starting server", "version", cfg.Version, "host", cfg.Host, "dynamicToolsets", cfg.DynamicToolsets, "readOnly", cfg.ReadOnly, "lockdownEnabled", cfg.LockdownMode)
264263
stdLogger := log.New(logOutput, stdioServerLogPrefix, 0)
264+
265+
ghServer, err := NewMCPServer(MCPServerConfig{
266+
Version: cfg.Version,
267+
Host: cfg.Host,
268+
Token: cfg.Token,
269+
EnabledToolsets: cfg.EnabledToolsets,
270+
DynamicToolsets: cfg.DynamicToolsets,
271+
ReadOnly: cfg.ReadOnly,
272+
Translator: t,
273+
ContentWindowSize: cfg.ContentWindowSize,
274+
LockdownMode: cfg.LockdownMode,
275+
RepoAccessTTL: cfg.RepoAccessCacheTTL,
276+
})
277+
if err != nil {
278+
return fmt.Errorf("failed to create MCP server: %w", err)
279+
}
280+
281+
stdioServer := server.NewStdioServer(ghServer)
265282
stdioServer.SetErrorLogger(stdLogger)
266283

267284
if cfg.ExportTranslations {

pkg/github/issues.go

Lines changed: 65 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ func fragmentToIssue(fragment IssueFragment) *github.Issue {
228228
}
229229

230230
// GetIssue creates a tool to get details of a specific issue in a GitHub repository.
231-
func IssueRead(getClient GetClientFn, getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc, flags FeatureFlags) (tool mcp.Tool, handler server.ToolHandlerFunc) {
231+
func IssueRead(getClient GetClientFn, getGQLClient GetGQLClientFn, cache *lockdown.RepoAccessCache, t translations.TranslationHelperFunc, flags FeatureFlags) (tool mcp.Tool, handler server.ToolHandlerFunc) {
232232
return mcp.NewTool("issue_read",
233233
mcp.WithDescription(t("TOOL_ISSUE_READ_DESCRIPTION", "Get information about a specific issue in a GitHub repository.")),
234234
mcp.WithToolAnnotation(mcp.ToolAnnotation{
@@ -297,20 +297,20 @@ Options are:
297297

298298
switch method {
299299
case "get":
300-
return GetIssue(ctx, client, gqlClient, owner, repo, issueNumber, flags)
300+
return GetIssue(ctx, client, cache, owner, repo, issueNumber, flags)
301301
case "get_comments":
302-
return GetIssueComments(ctx, client, owner, repo, issueNumber, pagination, flags)
302+
return GetIssueComments(ctx, client, cache, owner, repo, issueNumber, pagination, flags)
303303
case "get_sub_issues":
304-
return GetSubIssues(ctx, client, owner, repo, issueNumber, pagination, flags)
304+
return GetSubIssues(ctx, client, cache, owner, repo, issueNumber, pagination, flags)
305305
case "get_labels":
306-
return GetIssueLabels(ctx, gqlClient, owner, repo, issueNumber, flags)
306+
return GetIssueLabels(ctx, gqlClient, owner, repo, issueNumber)
307307
default:
308308
return mcp.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil
309309
}
310310
}
311311
}
312312

313-
func GetIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, owner string, repo string, issueNumber int, flags FeatureFlags) (*mcp.CallToolResult, error) {
313+
func GetIssue(ctx context.Context, client *github.Client, cache *lockdown.RepoAccessCache, owner string, repo string, issueNumber int, flags FeatureFlags) (*mcp.CallToolResult, error) {
314314
issue, resp, err := client.Issues.Get(ctx, owner, repo, issueNumber)
315315
if err != nil {
316316
return nil, fmt.Errorf("failed to get issue: %w", err)
@@ -326,12 +326,16 @@ func GetIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Cl
326326
}
327327

328328
if flags.LockdownMode {
329-
if issue.User != nil {
330-
shouldRemoveContent, err := lockdown.ShouldRemoveContent(ctx, gqlClient, *issue.User.Login, owner, repo)
329+
if cache == nil {
330+
return nil, fmt.Errorf("lockdown cache is not configured")
331+
}
332+
login := issue.GetUser().GetLogin()
333+
if login != "" {
334+
isSafeContent, err := cache.IsSafeContent(ctx, login, owner, repo)
331335
if err != nil {
332336
return mcp.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil
333337
}
334-
if shouldRemoveContent {
338+
if !isSafeContent {
335339
return mcp.NewToolResultError("access to issue details is restricted by lockdown mode"), nil
336340
}
337341
}
@@ -355,7 +359,7 @@ func GetIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Cl
355359
return mcp.NewToolResultText(string(r)), nil
356360
}
357361

358-
func GetIssueComments(ctx context.Context, client *github.Client, owner string, repo string, issueNumber int, pagination PaginationParams, _ FeatureFlags) (*mcp.CallToolResult, error) {
362+
func GetIssueComments(ctx context.Context, client *github.Client, cache *lockdown.RepoAccessCache, owner string, repo string, issueNumber int, pagination PaginationParams, flags FeatureFlags) (*mcp.CallToolResult, error) {
359363
opts := &github.IssueListCommentsOptions{
360364
ListOptions: github.ListOptions{
361365
Page: pagination.Page,
@@ -376,6 +380,30 @@ func GetIssueComments(ctx context.Context, client *github.Client, owner string,
376380
}
377381
return mcp.NewToolResultError(fmt.Sprintf("failed to get issue comments: %s", string(body))), nil
378382
}
383+
if flags.LockdownMode {
384+
if cache == nil {
385+
return nil, fmt.Errorf("lockdown cache is not configured")
386+
}
387+
filteredComments := make([]*github.IssueComment, 0, len(comments))
388+
for _, comment := range comments {
389+
user := comment.User
390+
if user == nil {
391+
continue
392+
}
393+
login := user.GetLogin()
394+
if login == "" {
395+
continue
396+
}
397+
isSafeContent, err := cache.IsSafeContent(ctx, login, owner, repo)
398+
if err != nil {
399+
return mcp.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil
400+
}
401+
if isSafeContent {
402+
filteredComments = append(filteredComments, comment)
403+
}
404+
}
405+
comments = filteredComments
406+
}
379407

380408
r, err := json.Marshal(comments)
381409
if err != nil {
@@ -385,7 +413,7 @@ func GetIssueComments(ctx context.Context, client *github.Client, owner string,
385413
return mcp.NewToolResultText(string(r)), nil
386414
}
387415

388-
func GetSubIssues(ctx context.Context, client *github.Client, owner string, repo string, issueNumber int, pagination PaginationParams, _ FeatureFlags) (*mcp.CallToolResult, error) {
416+
func GetSubIssues(ctx context.Context, client *github.Client, cache *lockdown.RepoAccessCache, owner string, repo string, issueNumber int, pagination PaginationParams, featureFlags FeatureFlags) (*mcp.CallToolResult, error) {
389417
opts := &github.IssueListOptions{
390418
ListOptions: github.ListOptions{
391419
Page: pagination.Page,
@@ -412,6 +440,31 @@ func GetSubIssues(ctx context.Context, client *github.Client, owner string, repo
412440
return mcp.NewToolResultError(fmt.Sprintf("failed to list sub-issues: %s", string(body))), nil
413441
}
414442

443+
if featureFlags.LockdownMode {
444+
if cache == nil {
445+
return nil, fmt.Errorf("lockdown cache is not configured")
446+
}
447+
filteredSubIssues := make([]*github.SubIssue, 0, len(subIssues))
448+
for _, subIssue := range subIssues {
449+
user := subIssue.User
450+
if user == nil {
451+
continue
452+
}
453+
login := user.GetLogin()
454+
if login == "" {
455+
continue
456+
}
457+
isSafeContent, err := cache.IsSafeContent(ctx, login, owner, repo)
458+
if err != nil {
459+
return mcp.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil
460+
}
461+
if isSafeContent {
462+
filteredSubIssues = append(filteredSubIssues, subIssue)
463+
}
464+
}
465+
subIssues = filteredSubIssues
466+
}
467+
415468
r, err := json.Marshal(subIssues)
416469
if err != nil {
417470
return nil, fmt.Errorf("failed to marshal response: %w", err)
@@ -420,7 +473,7 @@ func GetSubIssues(ctx context.Context, client *github.Client, owner string, repo
420473
return mcp.NewToolResultText(string(r)), nil
421474
}
422475

423-
func GetIssueLabels(ctx context.Context, client *githubv4.Client, owner string, repo string, issueNumber int, _ FeatureFlags) (*mcp.CallToolResult, error) {
476+
func GetIssueLabels(ctx context.Context, client *githubv4.Client, owner string, repo string, issueNumber int) (*mcp.CallToolResult, error) {
424477
// Get current labels on the issue using GraphQL
425478
var query struct {
426479
Repository struct {

0 commit comments

Comments
 (0)