Skip to content

Commit 59803bb

Browse files
ulrikandersenulrikandersen
authored
Merge pull request #426 from shapehq/only-allow-accessing-repos-with-correct-suffix
Prevent loading files from repos with incorrect names
2 parents 5c2d285 + 1807f18 commit 59803bb

File tree

3 files changed

+202
-2
lines changed

3 files changed

+202
-2
lines changed

‎__test__/common/github/RepoRestrictedGitHubClient.test.ts‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
import { RepoRestrictedGitHubClient } from '@/common/github/RepoRestrictedGitHubClient';
2+
import {
3+
IGitHubClient,
4+
AddCommentToPullRequestRequest,
5+
GetPullRequestCommentsRequest,
6+
GetPullRequestFilesRequest,
7+
GetRepositoryContentRequest,
8+
GraphQLQueryRequest,
9+
UpdatePullRequestCommentRequest,
10+
GitHubClient
11+
} from "@/common";
12+
import { jest } from '@jest/globals';
13+
14+
describe('RepoRestrictedGitHubClient', () => {
15+
let client: RepoRestrictedGitHubClient;
16+
const repositoryNameSuffix = '-suffix';
17+
18+
const gitHubClient: jest.Mocked<IGitHubClient> = {
19+
graphql: jest.fn(),
20+
getRepositoryContent: jest.fn(),
21+
getPullRequestFiles: jest.fn(),
22+
getPullRequestComments: jest.fn(),
23+
addCommentToPullRequest: jest.fn(),
24+
updatePullRequestComment: jest.fn(),
25+
};
26+
27+
beforeEach(() => {
28+
client = new RepoRestrictedGitHubClient({
29+
repositoryNameSuffix,
30+
gitHubClient
31+
});
32+
});
33+
34+
it('should delegate graphql request to the underlying client', async () => {
35+
const request: GraphQLQueryRequest = { query: '' };
36+
await client.graphql(request);
37+
expect(gitHubClient.graphql).toHaveBeenCalledWith(request);
38+
});
39+
40+
it('should check suffix for getRepositoryContent', async () => {
41+
const request: GetRepositoryContentRequest = {
42+
repositoryName: 'repo-suffix', path: '',
43+
repositoryOwner: '',
44+
ref: undefined
45+
};
46+
await client.getRepositoryContent(request);
47+
expect(gitHubClient.getRepositoryContent).toHaveBeenCalledWith(request);
48+
});
49+
50+
it('should throw error if suffix is invalid for getRepositoryContent', async () => {
51+
const request: GetRepositoryContentRequest = {
52+
repositoryName: 'repo', path: '',
53+
repositoryOwner: '',
54+
ref: undefined
55+
};
56+
await expect(client.getRepositoryContent(request)).rejects.toThrow("Invalid repository name");
57+
});
58+
59+
it('should check suffix for getPullRequestFiles', async () => {
60+
const request: GetPullRequestFilesRequest = {
61+
repositoryName: 'repo-suffix', pullRequestNumber: 1,
62+
appInstallationId: 0,
63+
repositoryOwner: ''
64+
};
65+
await client.getPullRequestFiles(request);
66+
expect(gitHubClient.getPullRequestFiles).toHaveBeenCalledWith(request);
67+
});
68+
69+
it('should throw error if suffix is invalid for getPullRequestFiles', async () => {
70+
const request: GetPullRequestFilesRequest = {
71+
repositoryName: 'repo', pullRequestNumber: 1,
72+
appInstallationId: 0,
73+
repositoryOwner: ''
74+
};
75+
await expect(client.getPullRequestFiles(request)).rejects.toThrow("Invalid repository name");
76+
});
77+
78+
it('should check suffix for getPullRequestComments', async () => {
79+
const request: GetPullRequestCommentsRequest = {
80+
repositoryName: 'repo-suffix', pullRequestNumber: 1,
81+
appInstallationId: 0,
82+
repositoryOwner: ''
83+
};
84+
await client.getPullRequestComments(request);
85+
expect(gitHubClient.getPullRequestComments).toHaveBeenCalledWith(request);
86+
});
87+
88+
it('should throw error if suffix is invalid for getPullRequestComments', async () => {
89+
const request: GetPullRequestCommentsRequest = {
90+
repositoryName: 'repo', pullRequestNumber: 1,
91+
appInstallationId: 0,
92+
repositoryOwner: ''
93+
};
94+
await expect(client.getPullRequestComments(request)).rejects.toThrow("Invalid repository name");
95+
});
96+
97+
it('should check suffix for addCommentToPullRequest', async () => {
98+
const request: AddCommentToPullRequestRequest = {
99+
repositoryName: 'repo-suffix', pullRequestNumber: 1, body: '',
100+
appInstallationId: 0,
101+
repositoryOwner: ''
102+
};
103+
await client.addCommentToPullRequest(request);
104+
expect(gitHubClient.addCommentToPullRequest).toHaveBeenCalledWith(request);
105+
});
106+
107+
it('should throw error if suffix is invalid for addCommentToPullRequest', async () => {
108+
const request: AddCommentToPullRequestRequest = {
109+
repositoryName: 'repo', pullRequestNumber: 1, body: '',
110+
appInstallationId: 0,
111+
repositoryOwner: ''
112+
};
113+
await expect(client.addCommentToPullRequest(request)).rejects.toThrow("Invalid repository name");
114+
});
115+
116+
it('should check suffix for updatePullRequestComment', async () => {
117+
const request: UpdatePullRequestCommentRequest = {
118+
repositoryName: 'repo-suffix', commentId: 1, body: '',
119+
appInstallationId: 0,
120+
repositoryOwner: ''
121+
};
122+
await client.updatePullRequestComment(request);
123+
expect(gitHubClient.updatePullRequestComment).toHaveBeenCalledWith(request);
124+
});
125+
126+
it('should throw error if suffix is invalid for updatePullRequestComment', async () => {
127+
const request: UpdatePullRequestCommentRequest = {
128+
repositoryName: 'repo', commentId: 1, body: '',
129+
appInstallationId: 0,
130+
repositoryOwner: ''
131+
};
132+
await expect(client.updatePullRequestComment(request)).rejects.toThrow("Invalid repository name");
133+
});
134+
});

‎src/common/github/RepoRestrictedGitHubClient.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import {
2+
IGitHubClient,
3+
AddCommentToPullRequestRequest,
4+
GetPullRequestCommentsRequest,
5+
GetPullRequestFilesRequest,
6+
GetRepositoryContentRequest,
7+
GraphQLQueryRequest,
8+
GraphQlQueryResponse,
9+
PullRequestComment,
10+
PullRequestFile,
11+
RepositoryContent,
12+
UpdatePullRequestCommentRequest
13+
} from "@/common";
14+
15+
export class RepoRestrictedGitHubClient implements IGitHubClient {
16+
17+
private gitHubClient: IGitHubClient;
18+
private repositoryNameSuffix: string;
19+
20+
constructor(config: {
21+
repositoryNameSuffix: string;
22+
gitHubClient: IGitHubClient
23+
}) {
24+
this.gitHubClient = config.gitHubClient;
25+
this.repositoryNameSuffix = config.repositoryNameSuffix;
26+
}
27+
28+
graphql(request: GraphQLQueryRequest): Promise<GraphQlQueryResponse> {
29+
return this.gitHubClient.graphql(request);
30+
}
31+
32+
getRepositoryContent(request: GetRepositoryContentRequest): Promise<RepositoryContent> {
33+
if (!this.isRepositoryNameValid(request.repositoryName)) return Promise.reject(new Error("Invalid repository name"));
34+
return this.gitHubClient.getRepositoryContent(request);
35+
}
36+
37+
getPullRequestFiles(request: GetPullRequestFilesRequest): Promise<PullRequestFile[]> {
38+
if (!this.isRepositoryNameValid(request.repositoryName)) return Promise.reject(new Error("Invalid repository name"));
39+
return this.gitHubClient.getPullRequestFiles(request);
40+
}
41+
42+
getPullRequestComments(request: GetPullRequestCommentsRequest): Promise<PullRequestComment[]> {
43+
if (!this.isRepositoryNameValid(request.repositoryName)) return Promise.reject(new Error("Invalid repository name"));
44+
return this.gitHubClient.getPullRequestComments(request);
45+
}
46+
47+
addCommentToPullRequest(request: AddCommentToPullRequestRequest): Promise<void> {
48+
if (!this.isRepositoryNameValid(request.repositoryName)) return Promise.reject(new Error("Invalid repository name"));
49+
return this.gitHubClient.addCommentToPullRequest(request);
50+
}
51+
52+
updatePullRequestComment(request: UpdatePullRequestCommentRequest): Promise<void> {
53+
if (!this.isRepositoryNameValid(request.repositoryName)) return Promise.reject(new Error("Invalid repository name"));
54+
return this.gitHubClient.updatePullRequestComment(request);
55+
}
56+
57+
private isRepositoryNameValid(repositoryName: string): boolean {
58+
return repositoryName.endsWith(this.repositoryNameSuffix);
59+
}
60+
}

‎src/composition.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import {
5050
RepositoryNameEventFilter,
5151
PullRequestCommenter
5252
} from "@/features/hooks/domain"
53+
import { RepoRestrictedGitHubClient } from "./common/github/RepoRestrictedGitHubClient"
5354

5455
const gitHubAppCredentials = {
5556
appId: env.getOrThrow("GITHUB_APP_ID"),
@@ -135,13 +136,18 @@ const oauthTokenRefresher = new LockingOAuthTokenRefresher({
135136
})
136137
})
137138

138-
export const gitHubClient = new GitHubClient({
139+
const gitHubClient = new GitHubClient({
139140
...gitHubAppCredentials,
140141
oauthTokenDataSource
141142
})
142143

144+
const repoRestrictedGitHubClient = new RepoRestrictedGitHubClient({
145+
repositoryNameSuffix: env.getOrThrow("REPOSITORY_NAME_SUFFIX"),
146+
gitHubClient
147+
})
148+
143149
export const userGitHubClient = new OAuthTokenRefreshingGitHubClient({
144-
gitHubClient,
150+
gitHubClient: repoRestrictedGitHubClient,
145151
oauthTokenDataSource,
146152
oauthTokenRefresher
147153
})

0 commit comments

Comments
 (0)