Scope PR comment routes to repository owner and name
swadhinbiswas committed 8 months ago
710e522
1 parent dfec2b1
commit 710e522
2 files changed
+40 -4
Showing 2 changed files+40-4
▼src/pages/api/repos/[owner]/[repo]/pulls/[number]/comments.ts
+32-4
| @@ -33,6 +33,24 @@ import { logger } from "@/lib/logger"; | ||
33 | 33 | |
34 | 34 | // ... existing imports ... |
35 | 35 | |
36 | +async function getRepositoryByOwnerAndName( | |
37 | + db: NodePgDatabase<typeof schema>, | |
38 | + owner: string, | |
39 | + repo: string | |
40 | +) { | |
41 | + const repoOwner = await db.query.users.findFirst({ | |
42 | + where: eq(schema.users.username, owner), | |
43 | + }); | |
44 | + if (!repoOwner) return null; | |
45 | + | |
46 | + return db.query.repositories.findFirst({ | |
47 | + where: and( | |
48 | + eq(schema.repositories.ownerId, repoOwner.id), | |
49 | + eq(schema.repositories.name, repo) | |
50 | + ), | |
51 | + }); | |
52 | +} | |
53 | + | |
36 | 54 | // GET /api/repos/:owner/:repo/pulls/:number/comments - List comments |
37 | 55 | export const GET: APIRoute = withErrorHandler(async ({ params, request }) => { |
38 | 56 | const tokenPayload = await getUserFromRequest(request); |
| @@ -42,16 +60,19 @@ export const GET: APIRoute = withErrorHandler(async ({ params, request }) => { | ||
42 | 60 | |
43 | 61 | const { owner, repo, number } = params; |
44 | 62 | const db = getDatabase() as NodePgDatabase<typeof schema>; |
63 | + const repository = await getRepositoryByOwnerAndName(db, owner as string, repo as string); | |
64 | + if (!repository) return notFound("Repository not found"); | |
45 | 65 | |
46 | 66 | // Find PR |
47 | 67 | const pr = await db.query.pullRequests.findFirst({ |
48 | 68 | where: and( |
69 | + eq(schema.pullRequests.repositoryId, repository.id), | |
49 | 70 | eq(schema.pullRequests.number, parseInt(number as string)), |
50 | 71 | ), |
51 | 72 | with: { repository: true }, |
52 | 73 | }); |
53 | 74 | |
54 | - if (!pr || pr.repository?.ownerId !== owner) { | |
75 | + if (!pr) { | |
55 | 76 | return notFound("Pull request not found"); |
56 | 77 | } |
57 | 78 | |
| @@ -99,16 +120,19 @@ export const POST: APIRoute = withErrorHandler(async ({ params, request }) => { | ||
99 | 120 | |
100 | 121 | const { owner, repo, number } = params; |
101 | 122 | const db = getDatabase() as NodePgDatabase<typeof schema>; |
123 | + const repository = await getRepositoryByOwnerAndName(db, owner as string, repo as string); | |
124 | + if (!repository) return notFound("Repository not found"); | |
102 | 125 | |
103 | 126 | // Find PR |
104 | 127 | const pr = await db.query.pullRequests.findFirst({ |
105 | 128 | where: and( |
129 | + eq(schema.pullRequests.repositoryId, repository.id), | |
106 | 130 | eq(schema.pullRequests.number, parseInt(number as string)), |
107 | 131 | ), |
108 | 132 | with: { repository: true }, |
109 | 133 | }); |
110 | 134 | |
111 | - if (!pr || pr.repository?.ownerId !== owner) { | |
135 | + if (!pr) { | |
112 | 136 | return notFound("Pull request not found"); |
113 | 137 | } |
114 | 138 | |
| @@ -172,6 +196,8 @@ export const PATCH: APIRoute = withErrorHandler(async ({ params, request }) => { | ||
172 | 196 | |
173 | 197 | const { owner, repo, number, commentId } = params; |
174 | 198 | const db = getDatabase() as NodePgDatabase<typeof schema>; |
199 | + const repository = await getRepositoryByOwnerAndName(db, owner as string, repo as string); | |
200 | + if (!repository) return notFound("Repository not found"); | |
175 | 201 | |
176 | 202 | // Find comment |
177 | 203 | const comment = await db.query.pullRequestComments.findFirst({ |
| @@ -183,7 +209,7 @@ export const PATCH: APIRoute = withErrorHandler(async ({ params, request }) => { | ||
183 | 209 | }, |
184 | 210 | }); |
185 | 211 | |
186 | - if (!comment || comment.pullRequest?.repository?.ownerId !== owner) { | |
212 | + if (!comment || comment.pullRequest?.repositoryId !== repository.id) { | |
187 | 213 | return notFound("Comment not found"); |
188 | 214 | } |
189 | 215 | |
| @@ -228,6 +254,8 @@ export const DELETE: APIRoute = withErrorHandler(async ({ params, request }) => | ||
228 | 254 | |
229 | 255 | const { owner, repo, number, commentId } = params; |
230 | 256 | const db = getDatabase() as NodePgDatabase<typeof schema>; |
257 | + const repository = await getRepositoryByOwnerAndName(db, owner as string, repo as string); | |
258 | + if (!repository) return notFound("Repository not found"); | |
231 | 259 | |
232 | 260 | // Find comment |
233 | 261 | const comment = await db.query.pullRequestComments.findFirst({ |
| @@ -239,7 +267,7 @@ export const DELETE: APIRoute = withErrorHandler(async ({ params, request }) => | ||
239 | 267 | }, |
240 | 268 | }); |
241 | 269 | |
242 | - if (!comment || comment.pullRequest?.repository?.ownerId !== owner) { | |
270 | + if (!comment || comment.pullRequest?.repositoryId !== repository.id) { | |
243 | 271 | return notFound("Comment not found"); |
244 | 272 | } |
245 | 273 | |
246 | 274 | |
▼tests/integration/pr-comments-path-permissions.test.ts
+8-0
| @@ -4,6 +4,8 @@ const { getUserFromRequestMock, checkPathPermissionsMock, fakeSchema } = vi.hois | ||
4 | 4 | getUserFromRequestMock: vi.fn(async () => ({ userId: "user-1", isAdmin: false })), |
5 | 5 | checkPathPermissionsMock: vi.fn(async () => ({ allowed: true, deniedPaths: [] })), |
6 | 6 | fakeSchema: { |
7 | + users: { username: {}, id: {} }, | |
8 | + repositories: { ownerId: {}, name: {}, id: {} }, | |
7 | 9 | pullRequests: { number: {}, repositoryId: {} }, |
8 | 10 | pullRequestComments: { id: {}, pullRequestId: {} }, |
9 | 11 | } as any, |
| @@ -49,6 +51,12 @@ function makeDb() { | ||
49 | 51 | |
50 | 52 | return { |
51 | 53 | query: { |
54 | + users: { | |
55 | + findFirst: vi.fn(async () => ({ id: "owner-1" })), | |
56 | + }, | |
57 | + repositories: { | |
58 | + findFirst: vi.fn(async () => ({ id: "repo-1", ownerId: "owner-1", name: "demo" })), | |
59 | + }, | |
52 | 60 | pullRequests: { |
53 | 61 | findFirst: vi.fn(async () => pr), |
54 | 62 | }, |
55 | 63 | |