Skip to content

Commit 2de28f7

Browse files
JoannaaKLJoannaaKL
committed
Refactoring to make the code pretty
1 parent 60ce461 commit 2de28f7

File tree

4 files changed

+81
-41
lines changed

4 files changed

+81
-41
lines changed

‎pkg/github/issues.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -331,11 +331,11 @@ func GetIssue(ctx context.Context, client *github.Client, cache *lockdown.RepoAc
331331
}
332332
login := issue.GetUser().GetLogin()
333333
if login != "" {
334-
isPrivate, hasPushAccess, err := cache.GetRepoAccessInfo(ctx, login, owner, repo)
334+
info, err := cache.GetRepoAccessInfo(ctx, login, owner, repo)
335335
if err != nil {
336336
return mcp.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil
337337
}
338-
if !isPrivate && !hasPushAccess {
338+
if info.ViewerLogin != login && !info.IsPrivate && !info.HasPushAccess {
339339
return mcp.NewToolResultError("access to issue details is restricted by lockdown mode"), nil
340340
}
341341
}
@@ -394,16 +394,16 @@ func GetIssueComments(ctx context.Context, client *github.Client, cache *lockdow
394394
if login == "" {
395395
continue
396396
}
397-
isPrivate, hasPushAccess, err := cache.GetRepoAccessInfo(ctx, login, owner, repo)
397+
info, err := cache.GetRepoAccessInfo(ctx, login, owner, repo)
398398
if err != nil {
399399
return mcp.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil
400400
}
401-
// Do not filter content for private repositories
402-
if isPrivate {
401+
// Do not filter content for private repositories or if the comment author is the viewer
402+
if info.IsPrivate || info.ViewerLogin == login {
403403
filteredComments = comments
404404
break
405405
}
406-
if hasPushAccess {
406+
if info.HasPushAccess {
407407
filteredComments = append(filteredComments, comment)
408408
}
409409
}
@@ -459,16 +459,16 @@ func GetSubIssues(ctx context.Context, client *github.Client, cache *lockdown.Re
459459
if login == "" {
460460
continue
461461
}
462-
isPrivate, hasPushAccess, err := cache.GetRepoAccessInfo(ctx, login, owner, repo)
462+
info, err := cache.GetRepoAccessInfo(ctx, login, owner, repo)
463463
if err != nil {
464464
return mcp.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil
465465
}
466-
// Repo is private, do not filter content
467-
if isPrivate {
466+
// Repo is private or the comment author is the viewer, do not filter content
467+
if info.IsPrivate || info.ViewerLogin == login {
468468
filteredSubIssues = subIssues
469469
break
470470
}
471-
if hasPushAccess {
471+
if info.HasPushAccess {
472472
filteredSubIssues = append(filteredSubIssues, subIssue)
473473
}
474474
}

‎pkg/github/pullrequests.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -141,12 +141,12 @@ func GetPullRequest(ctx context.Context, client *github.Client, cache *lockdown.
141141
}
142142
login := pr.GetUser().GetLogin()
143143
if login != "" {
144-
isPrivate, hasPushAccess, err := cache.GetRepoAccessInfo(ctx, login, owner, repo)
144+
info, err := cache.GetRepoAccessInfo(ctx, login, owner, repo)
145145
if err != nil {
146146
return nil, fmt.Errorf("failed to check content removal: %w", err)
147147
}
148148

149-
if !isPrivate && !hasPushAccess {
149+
if info.ViewerLogin != login && !info.IsPrivate && !info.HasPushAccess {
150150
return mcp.NewToolResultError("access to pull request is restricted by lockdown mode"), nil
151151
}
152152
}
@@ -303,15 +303,16 @@ func GetPullRequestReviewComments(ctx context.Context, client *github.Client, ca
303303
if user == nil {
304304
continue
305305
}
306-
isPrivate, hasPushAccess, err := cache.GetRepoAccessInfo(ctx, user.GetLogin(), owner, repo)
306+
info, err := cache.GetRepoAccessInfo(ctx, user.GetLogin(), owner, repo)
307307
if err != nil {
308308
return mcp.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil
309309
}
310-
if isPrivate {
310+
// Do not filter content for private repositories or if the comment author is the viewer
311+
if info.IsPrivate || info.ViewerLogin == user.GetLogin() {
311312
filteredComments = comments
312313
break
313314
}
314-
if hasPushAccess {
315+
if info.HasPushAccess {
315316
filteredComments = append(filteredComments, comment)
316317
}
317318
}
@@ -353,14 +354,14 @@ func GetPullRequestReviews(ctx context.Context, client *github.Client, cache *lo
353354
for _, review := range reviews {
354355
login := review.GetUser().GetLogin()
355356
if login != "" {
356-
isPrivate, hasPushAccess, err := cache.GetRepoAccessInfo(ctx, login, owner, repo)
357+
info, err := cache.GetRepoAccessInfo(ctx, login, owner, repo)
357358
if err != nil {
358359
return nil, fmt.Errorf("failed to check lockdown mode: %w", err)
359360
}
360-
if isPrivate {
361+
if info.IsPrivate || info.ViewerLogin == login {
361362
filteredReviews = reviews
362363
}
363-
if hasPushAccess {
364+
if info.HasPushAccess {
364365
filteredReviews = append(filteredReviews, review)
365366
}
366367
reviews = filteredReviews

‎pkg/lockdown/lockdown.go‎

Lines changed: 50 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,16 @@ type RepoAccessCache struct {
2323
}
2424

2525
type repoAccessCacheEntry struct {
26-
isPrivate bool
27-
knownUsers map[string]bool // normalized login -> has push access
26+
isPrivate bool
27+
knownUsers map[string]bool // normalized login -> has push access
28+
viewerLogin string
29+
}
30+
31+
// RepoAccessInfo captures repository metadata needed for lockdown decisions.
32+
type RepoAccessInfo struct {
33+
IsPrivate bool
34+
HasPushAccess bool
35+
ViewerLogin string
2836
}
2937

3038
const (
@@ -101,12 +109,11 @@ type CacheStats struct {
101109
Evictions int64
102110
}
103111

104-
// GetRepoAccessInfo returns the repository's privacy status and whether the
105-
// specified user has push permissions. Results are cached per repository to
106-
// avoid repeated GraphQL round-trips.
107-
func (c *RepoAccessCache) GetRepoAccessInfo(ctx context.Context, username, owner, repo string) (bool, bool, error) {
112+
// GetRepoAccessInfo returns repository access metadata for the provided user.
113+
// Results are cached per repository to avoid repeated GraphQL round-trips.
114+
func (c *RepoAccessCache) GetRepoAccessInfo(ctx context.Context, username, owner, repo string) (RepoAccessInfo, error) {
108115
if c == nil {
109-
return false, false, fmt.Errorf("nil repo access cache")
116+
return RepoAccessInfo{}, fmt.Errorf("nil repo access cache")
110117
}
111118

112119
key := cacheKey(owner, repo)
@@ -120,41 +127,59 @@ func (c *RepoAccessCache) GetRepoAccessInfo(ctx context.Context, username, owner
120127
entry := cacheItem.Data().(*repoAccessCacheEntry)
121128
if cachedHasPush, known := entry.knownUsers[userKey]; known {
122129
c.logDebug("repo access cache hit", "owner", owner, "repo", repo, "user", username)
123-
return entry.isPrivate, cachedHasPush, nil
130+
return RepoAccessInfo{
131+
IsPrivate: entry.isPrivate,
132+
HasPushAccess: cachedHasPush,
133+
ViewerLogin: entry.viewerLogin,
134+
}, nil
124135
}
125136
c.logDebug("known users cache miss", "owner", owner, "repo", repo, "user", username)
126-
_, hasPush, queryErr := c.queryRepoAccessInfo(ctx, username, owner, repo)
137+
info, queryErr := c.queryRepoAccessInfo(ctx, username, owner, repo)
127138
if queryErr != nil {
128-
return false, false, queryErr
139+
return RepoAccessInfo{}, queryErr
129140
}
130-
entry.knownUsers[userKey] = hasPush
141+
entry.knownUsers[userKey] = info.HasPushAccess
142+
entry.viewerLogin = info.ViewerLogin
143+
entry.isPrivate = info.IsPrivate
131144
c.cache.Add(key, c.ttl, entry)
132-
return entry.isPrivate, entry.knownUsers[userKey], nil
145+
return RepoAccessInfo{
146+
IsPrivate: entry.isPrivate,
147+
HasPushAccess: entry.knownUsers[userKey],
148+
ViewerLogin: entry.viewerLogin,
149+
}, nil
133150
}
134151

135152
c.logDebug("repo access cache miss", "owner", owner, "repo", repo, "user", username)
136153

137-
isPrivate, hasPush, queryErr := c.queryRepoAccessInfo(ctx, username, owner, repo)
154+
info, queryErr := c.queryRepoAccessInfo(ctx, username, owner, repo)
138155
if queryErr != nil {
139-
return false, false, queryErr
156+
return RepoAccessInfo{}, queryErr
140157
}
141158

142159
// Create new entry
143160
entry := &repoAccessCacheEntry{
144-
knownUsers: map[string]bool{userKey: hasPush},
145-
isPrivate: isPrivate,
161+
knownUsers: map[string]bool{userKey: info.HasPushAccess},
162+
isPrivate: info.IsPrivate,
163+
viewerLogin: info.ViewerLogin,
146164
}
147165
c.cache.Add(key, c.ttl, entry)
148166

149-
return entry.isPrivate, entry.knownUsers[userKey], nil
167+
return RepoAccessInfo{
168+
IsPrivate: entry.isPrivate,
169+
HasPushAccess: entry.knownUsers[userKey],
170+
ViewerLogin: entry.viewerLogin,
171+
}, nil
150172
}
151173

152-
func (c *RepoAccessCache) queryRepoAccessInfo(ctx context.Context, username, owner, repo string) (bool, bool, error) {
174+
func (c *RepoAccessCache) queryRepoAccessInfo(ctx context.Context, username, owner, repo string) (RepoAccessInfo, error) {
153175
if c.client == nil {
154-
return false, false, fmt.Errorf("nil GraphQL client")
176+
return RepoAccessInfo{}, fmt.Errorf("nil GraphQL client")
155177
}
156178

157179
var query struct {
180+
Viewer struct {
181+
Login githubv4.String
182+
}
158183
Repository struct {
159184
IsPrivate githubv4.Boolean
160185
Collaborators struct {
@@ -175,7 +200,7 @@ func (c *RepoAccessCache) queryRepoAccessInfo(ctx context.Context, username, own
175200
}
176201

177202
if err := c.client.Query(ctx, &query, variables); err != nil {
178-
return false, false, fmt.Errorf("failed to query repository access info: %w", err)
203+
return RepoAccessInfo{}, fmt.Errorf("failed to query repository access info: %w", err)
179204
}
180205

181206
hasPush := false
@@ -188,7 +213,11 @@ func (c *RepoAccessCache) queryRepoAccessInfo(ctx context.Context, username, own
188213
}
189214
}
190215

191-
return bool(query.Repository.IsPrivate), hasPush, nil
216+
return RepoAccessInfo{
217+
IsPrivate: bool(query.Repository.IsPrivate),
218+
HasPushAccess: hasPush,
219+
ViewerLogin: string(query.Viewer.Login),
220+
}, nil
192221
}
193222

194223
func cacheKey(owner, repo string) string {

‎pkg/lockdown/lockdown_test.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ const (
1818
)
1919

2020
type repoAccessQuery struct {
21+
Viewer struct {
22+
Login githubv4.String
23+
}
2124
Repository struct {
2225
IsPrivate githubv4.Boolean
2326
Collaborators struct {
@@ -62,6 +65,9 @@ func newMockRepoAccessCache(t *testing.T, ttl time.Duration) (*RepoAccessCache,
6265
}
6366

6467
response := githubv4mock.DataResponse(map[string]any{
68+
"viewer": map[string]any{
69+
"login": testUser,
70+
},
6571
"repository": map[string]any{
6672
"isPrivate": false,
6773
"collaborators": map[string]any{
@@ -90,13 +96,17 @@ func TestRepoAccessCacheEvictsAfterTTL(t *testing.T) {
9096
ctx := t.Context()
9197

9298
cache, transport := newMockRepoAccessCache(t, 5*time.Millisecond)
93-
_, _, err := cache.GetRepoAccessInfo(ctx, testUser, testOwner, testRepo)
99+
info, err := cache.GetRepoAccessInfo(ctx, testUser, testOwner, testRepo)
94100
require.NoError(t, err)
101+
require.Equal(t, testUser, info.ViewerLogin)
102+
require.True(t, info.HasPushAccess)
95103
require.EqualValues(t, 1, transport.CallCount())
96104

97105
time.Sleep(20 * time.Millisecond)
98106

99-
_, _, err = cache.GetRepoAccessInfo(ctx, testUser, testOwner, testRepo)
107+
info, err = cache.GetRepoAccessInfo(ctx, testUser, testOwner, testRepo)
100108
require.NoError(t, err)
109+
require.Equal(t, testUser, info.ViewerLogin)
110+
require.True(t, info.HasPushAccess)
101111
require.EqualValues(t, 2, transport.CallCount())
102112
}

0 commit comments

Comments
 (0)