Skip to content

Commit 3109915

Browse files
committed
use REST API for permission checks
1 parent efcaead commit 3109915

3 files changed

Lines changed: 62 additions & 45 deletions

File tree

internal/ghmcp/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func createGitHubClients(cfg github.MCPServerConfig, apiHost utils.APIHostResolv
9191
if cfg.RepoAccessTTL != nil {
9292
opts = append(opts, lockdown.WithTTL(*cfg.RepoAccessTTL))
9393
}
94-
repoAccessCache = lockdown.GetInstance(gqlClient, opts...)
94+
repoAccessCache = lockdown.GetInstance(gqlClient, restClient, opts...)
9595
}
9696

9797
return &githubClients{

pkg/github/dependencies.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,8 +386,13 @@ func (d *RequestDeps) GetRepoAccessCache(ctx context.Context) (*lockdown.RepoAcc
386386
return nil, err
387387
}
388388

389+
restClient, err := d.GetClient(ctx)
390+
if err != nil {
391+
return nil, err
392+
}
393+
389394
// Create repo access cache
390-
instance := lockdown.GetInstance(gqlClient, d.RepoAccessOpts...)
395+
instance := lockdown.GetInstance(gqlClient, restClient, d.RepoAccessOpts...)
391396
return instance, nil
392397
}
393398

pkg/lockdown/lockdown.go

Lines changed: 55 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"sync"
99
"time"
1010

11+
"github.com/google/go-github/v82/github"
1112
"github.com/muesli/cache2go"
1213
"github.com/shurcooL/githubv4"
1314
)
@@ -16,6 +17,7 @@ import (
1617
// multiple tools can reuse the same access information safely across goroutines.
1718
type RepoAccessCache struct {
1819
client *githubv4.Client
20+
restClient *github.Client
1921
mu sync.Mutex
2022
cache *cache2go.CacheTable
2123
ttl time.Duration
@@ -78,27 +80,38 @@ func WithCacheName(name string) RepoAccessOption {
7880
// It initializes the instance on first call with the provided client and options.
7981
// Subsequent calls ignore the client and options parameters and return the existing instance.
8082
// This is the preferred way to access the cache in production code.
81-
func GetInstance(client *githubv4.Client, opts ...RepoAccessOption) *RepoAccessCache {
83+
func GetInstance(client *githubv4.Client, restClient *github.Client, opts ...RepoAccessOption) *RepoAccessCache {
8284
instanceMu.Lock()
8385
defer instanceMu.Unlock()
8486
if instance == nil {
85-
instance = &RepoAccessCache{
86-
client: client,
87-
cache: cache2go.Cache(defaultRepoAccessCacheKey),
88-
ttl: defaultRepoAccessTTL,
89-
trustedBotLogins: map[string]struct{}{
90-
"copilot": {},
91-
},
92-
}
93-
for _, opt := range opts {
94-
if opt != nil {
95-
opt(instance)
96-
}
97-
}
87+
instance = newRepoAccessCache(client, restClient, opts...)
9888
}
9989
return instance
10090
}
10191

92+
// NewRepoAccessCache creates a standalone cache instance, used for tests.
93+
func NewRepoAccessCache(client *githubv4.Client, restClient *github.Client, opts ...RepoAccessOption) *RepoAccessCache {
94+
return newRepoAccessCache(client, restClient, opts...)
95+
}
96+
97+
func newRepoAccessCache(client *githubv4.Client, restClient *github.Client, opts ...RepoAccessOption) *RepoAccessCache {
98+
c := &RepoAccessCache{
99+
client: client,
100+
restClient: restClient,
101+
cache: cache2go.Cache(defaultRepoAccessCacheKey),
102+
ttl: defaultRepoAccessTTL,
103+
trustedBotLogins: map[string]struct{}{
104+
"copilot": {},
105+
},
106+
}
107+
for _, opt := range opts {
108+
if opt != nil {
109+
opt(c)
110+
}
111+
}
112+
return c
113+
}
114+
102115
// SetLogger updates the logger used for cache diagnostics.
103116
func (c *RepoAccessCache) SetLogger(logger *slog.Logger) {
104117
c.mu.Lock()
@@ -157,21 +170,19 @@ func (c *RepoAccessCache) getRepoAccessInfo(ctx context.Context, username, owner
157170
}, nil
158171
}
159172

160-
c.logDebug(ctx, "known users cache miss, fetching from graphql API")
173+
c.logDebug(ctx, "known users cache miss, fetching permission")
161174

162-
info, queryErr := c.queryRepoAccessInfo(ctx, username, owner, repo)
163-
if queryErr != nil {
164-
return RepoAccessInfo{}, queryErr
175+
hasPush, pushErr := c.checkPushAccess(ctx, username, owner, repo)
176+
if pushErr != nil {
177+
return RepoAccessInfo{}, pushErr
165178
}
166179

167-
entry.knownUsers[userKey] = info.HasPushAccess
168-
entry.viewerLogin = info.ViewerLogin
169-
entry.isPrivate = info.IsPrivate
180+
entry.knownUsers[userKey] = hasPush
170181
c.cache.Add(key, c.ttl, entry)
171182

172183
return RepoAccessInfo{
173184
IsPrivate: entry.isPrivate,
174-
HasPushAccess: entry.knownUsers[userKey],
185+
HasPushAccess: hasPush,
175186
ViewerLogin: entry.viewerLogin,
176187
}, nil
177188
}
@@ -208,36 +219,22 @@ func (c *RepoAccessCache) queryRepoAccessInfo(ctx context.Context, username, own
208219
Login githubv4.String
209220
}
210221
Repository struct {
211-
IsPrivate githubv4.Boolean
212-
Collaborators struct {
213-
Edges []struct {
214-
Permission githubv4.String
215-
Node struct {
216-
Login githubv4.String
217-
}
218-
}
219-
} `graphql:"collaborators(query: $username, first: 1)"`
222+
IsPrivate githubv4.Boolean
220223
} `graphql:"repository(owner: $owner, name: $name)"`
221224
}
222225

223226
variables := map[string]any{
224-
"owner": githubv4.String(owner),
225-
"name": githubv4.String(repo),
226-
"username": githubv4.String(username),
227+
"owner": githubv4.String(owner),
228+
"name": githubv4.String(repo),
227229
}
228230

229231
if err := c.client.Query(ctx, &query, variables); err != nil {
230-
return RepoAccessInfo{}, fmt.Errorf("failed to query repository access info: %w", err)
232+
return RepoAccessInfo{}, fmt.Errorf("failed to query repository metadata: %w", err)
231233
}
232234

233-
hasPush := false
234-
for _, edge := range query.Repository.Collaborators.Edges {
235-
login := string(edge.Node.Login)
236-
if strings.EqualFold(login, username) {
237-
permission := string(edge.Permission)
238-
hasPush = permission == "WRITE" || permission == "ADMIN" || permission == "MAINTAIN"
239-
break
240-
}
235+
hasPush, err := c.checkPushAccess(ctx, username, owner, repo)
236+
if err != nil {
237+
return RepoAccessInfo{}, err
241238
}
242239

243240
c.logDebug(ctx, fmt.Sprintf("queried repo access info for user %s to %s/%s: isPrivate=%t, hasPushAccess=%t, viewerLogin=%s",
@@ -250,6 +247,21 @@ func (c *RepoAccessCache) queryRepoAccessInfo(ctx context.Context, username, own
250247
}, nil
251248
}
252249

250+
// checkPushAccess checks if the user has push access to the repository via the REST permission endpoint.
251+
func (c *RepoAccessCache) checkPushAccess(ctx context.Context, username, owner, repo string) (bool, error) {
252+
if c.restClient == nil {
253+
return false, fmt.Errorf("nil REST client")
254+
}
255+
256+
permLevel, _, err := c.restClient.Repositories.GetPermissionLevel(ctx, owner, repo, username)
257+
if err != nil {
258+
return false, fmt.Errorf("failed to get user permission level: %w", err)
259+
}
260+
261+
permission := permLevel.GetPermission()
262+
return permission == "admin" || permission == "write", nil
263+
}
264+
253265
func (c *RepoAccessCache) log(ctx context.Context, level slog.Level, msg string, attrs ...slog.Attr) {
254266
if c == nil || c.logger == nil {
255267
return

0 commit comments

Comments
 (0)