Enforce suggestion permissions in GraphQL mutations
swadhinbiswas committed 8 months ago
dfec2b1
1 parent a14e474
commit dfec2b1
2 files changed
+174 -5
Showing 2 changed files+174-5
▼src/lib/graphql/resolvers.ts
+83-5
| @@ -5,7 +5,7 @@ | ||
5 | 5 | |
6 | 6 | import { getDatabase, schema } from "@/db"; |
7 | 7 | import type { NodePgDatabase } from "drizzle-orm/node-postgres"; |
8 | -import { eq, and, like, desc, sql, count } from "drizzle-orm"; | |
8 | +import { eq, and, like, desc, sql, count, inArray } from "drizzle-orm"; | |
9 | 9 | import { logger } from "@/lib/logger"; |
10 | 10 | import { compareBranches, getCommit, mergeBranch } from "@/lib/git"; |
11 | 11 | import { resolveRepoPath } from "@/lib/git-storage"; |
| @@ -669,18 +669,51 @@ export const resolvers = { | ||
669 | 669 | ) => { |
670 | 670 | if (!ctx.userId) throw new Error("Authentication required"); |
671 | 671 | |
672 | - const { applySuggestion } = await import("@/lib/suggested-changes"); | |
672 | + const comment = await ctx.db.query.pullRequestComments.findFirst({ | |
673 | + where: eq(schema.pullRequestComments.id, input.commentId), | |
674 | + with: { | |
675 | + pullRequest: { | |
676 | + columns: { | |
677 | + id: true, | |
678 | + repositoryId: true, | |
679 | + }, | |
680 | + }, | |
681 | + }, | |
682 | + }); | |
683 | + if (!comment || !comment.pullRequest) { | |
684 | + throw new Error("Comment not found"); | |
685 | + } | |
686 | + | |
687 | + const { canApplySuggestions, applySuggestion } = await import("@/lib/suggestions"); | |
688 | + const canApply = await canApplySuggestions(ctx.userId, comment.pullRequest.id); | |
689 | + if (!canApply) { | |
690 | + throw new Error("You don't have permission to apply suggestions"); | |
691 | + } | |
692 | + | |
693 | + if (comment.path) { | |
694 | + const { checkPathPermissions } = await import("@/lib/path-scoping"); | |
695 | + const permission = await checkPathPermissions( | |
696 | + ctx.userId, | |
697 | + comment.pullRequest.repositoryId, | |
698 | + [comment.path], | |
699 | + "write" | |
700 | + ); | |
701 | + if (!permission.allowed) { | |
702 | + throw new Error(permission.reason || "Insufficient path permissions"); | |
703 | + } | |
704 | + } | |
705 | + | |
673 | 706 | const result = await applySuggestion(input.commentId, ctx.userId); |
674 | 707 | |
675 | 708 | if (!result.success) { |
676 | 709 | throw new Error(result.error || "Failed to apply suggestion"); |
677 | 710 | } |
678 | 711 | |
679 | - const comment = await ctx.db.query.pullRequestComments.findFirst({ | |
712 | + const updatedComment = await ctx.db.query.pullRequestComments.findFirst({ | |
680 | 713 | where: eq(schema.pullRequestComments.id, input.commentId), |
681 | 714 | }); |
682 | 715 | |
683 | - return { success: true, comment }; | |
716 | + return { success: true, comment: updatedComment }; | |
684 | 717 | }, |
685 | 718 | |
686 | 719 | batchApplySuggestions: async ( |
| @@ -690,7 +723,52 @@ export const resolvers = { | ||
690 | 723 | ) => { |
691 | 724 | if (!ctx.userId) throw new Error("Authentication required"); |
692 | 725 | |
693 | - const { batchApplySuggestions } = await import("@/lib/suggested-changes"); | |
726 | + const comments = await ctx.db.query.pullRequestComments.findMany({ | |
727 | + where: inArray(schema.pullRequestComments.id, input.commentIds), | |
728 | + with: { | |
729 | + pullRequest: { | |
730 | + columns: { | |
731 | + id: true, | |
732 | + repositoryId: true, | |
733 | + }, | |
734 | + }, | |
735 | + }, | |
736 | + }); | |
737 | + if (comments.length !== input.commentIds.length) { | |
738 | + throw new Error("One or more comments were not found"); | |
739 | + } | |
740 | + | |
741 | + const prIds = new Set(comments.map(c => c.pullRequest?.id).filter(Boolean)); | |
742 | + if (prIds.size !== 1) { | |
743 | + throw new Error("All suggestions in a batch must belong to the same pull request"); | |
744 | + } | |
745 | + | |
746 | + const [pullRequestId] = [...prIds] as string[]; | |
747 | + const repositoryId = comments[0].pullRequest?.repositoryId; | |
748 | + if (!repositoryId) { | |
749 | + throw new Error("Repository not found for suggestions"); | |
750 | + } | |
751 | + | |
752 | + const { canApplySuggestions, batchApplySuggestions } = await import("@/lib/suggestions"); | |
753 | + const canApply = await canApplySuggestions(ctx.userId, pullRequestId); | |
754 | + if (!canApply) { | |
755 | + throw new Error("You don't have permission to apply suggestions"); | |
756 | + } | |
757 | + | |
758 | + const scopedPaths = [...new Set(comments.map(c => c.path).filter(Boolean) as string[])]; | |
759 | + if (scopedPaths.length > 0) { | |
760 | + const { checkPathPermissions } = await import("@/lib/path-scoping"); | |
761 | + const permission = await checkPathPermissions( | |
762 | + ctx.userId, | |
763 | + repositoryId, | |
764 | + scopedPaths, | |
765 | + "write" | |
766 | + ); | |
767 | + if (!permission.allowed) { | |
768 | + throw new Error(permission.reason || "Insufficient path permissions"); | |
769 | + } | |
770 | + } | |
771 | + | |
694 | 772 | const result = await batchApplySuggestions(input.commentIds, ctx.userId); |
695 | 773 | |
696 | 774 | const appliedComments = await ctx.db.query.pullRequestComments.findMany({ |
697 | 775 | |
▼tests/unit/graphql-suggestion-permissions.test.ts
+91-0
| @@ -0,0 +1,91 @@ | ||
1 | +import { beforeEach, describe, expect, it, vi } from "vitest"; | |
2 | + | |
3 | +const { | |
4 | + canApplySuggestionsMock, | |
5 | + applySuggestionMock, | |
6 | + batchApplySuggestionsMock, | |
7 | + checkPathPermissionsMock, | |
8 | +} = vi.hoisted(() => ({ | |
9 | + canApplySuggestionsMock: vi.fn(async () => true), | |
10 | + applySuggestionMock: vi.fn(async () => ({ success: true })), | |
11 | + batchApplySuggestionsMock: vi.fn(async () => ({ applied: [], failed: [] })), | |
12 | + checkPathPermissionsMock: vi.fn(async () => ({ allowed: true, deniedPaths: [] })), | |
13 | +})); | |
14 | + | |
15 | +vi.mock("@/lib/suggestions", () => ({ | |
16 | + canApplySuggestions: canApplySuggestionsMock, | |
17 | + applySuggestion: applySuggestionMock, | |
18 | + batchApplySuggestions: batchApplySuggestionsMock, | |
19 | +})); | |
20 | + | |
21 | +vi.mock("@/lib/path-scoping", () => ({ | |
22 | + checkPathPermissions: checkPathPermissionsMock, | |
23 | +})); | |
24 | + | |
25 | +import { resolvers } from "@/lib/graphql/resolvers"; | |
26 | + | |
27 | +function makeContext() { | |
28 | + const comment = { | |
29 | + id: "comment-1", | |
30 | + path: "secure/config.yml", | |
31 | + pullRequest: { id: "pr-1", repositoryId: "repo-1" }, | |
32 | + }; | |
33 | + | |
34 | + return { | |
35 | + db: { | |
36 | + query: { | |
37 | + pullRequestComments: { | |
38 | + findFirst: vi.fn(async () => comment), | |
39 | + findMany: vi.fn(async (_args?: any) => [comment]), | |
40 | + }, | |
41 | + }, | |
42 | + }, | |
43 | + userId: "user-1", | |
44 | + } as any; | |
45 | +} | |
46 | + | |
47 | +describe("GraphQL suggestion permission enforcement", () => { | |
48 | + beforeEach(() => { | |
49 | + canApplySuggestionsMock.mockResolvedValue(true); | |
50 | + applySuggestionMock.mockResolvedValue({ success: true }); | |
51 | + batchApplySuggestionsMock.mockResolvedValue({ applied: ["comment-1"], failed: [] }); | |
52 | + checkPathPermissionsMock.mockResolvedValue({ allowed: true, deniedPaths: [] }); | |
53 | + }); | |
54 | + | |
55 | + it("blocks applySuggestion when path permission is denied", async () => { | |
56 | + const ctx = makeContext(); | |
57 | + checkPathPermissionsMock.mockResolvedValue({ | |
58 | + allowed: false, | |
59 | + deniedPaths: ["secure/config.yml"], | |
60 | + reason: "Insufficient permissions for paths: secure/config.yml", | |
61 | + }); | |
62 | + | |
63 | + await expect( | |
64 | + resolvers.Mutation.applySuggestion( | |
65 | + {}, | |
66 | + { input: { commentId: "comment-1" } }, | |
67 | + ctx | |
68 | + ) | |
69 | + ).rejects.toThrow("Insufficient permissions for paths: secure/config.yml"); | |
70 | + | |
71 | + expect(applySuggestionMock).not.toHaveBeenCalled(); | |
72 | + }); | |
73 | + | |
74 | + it("blocks batchApplySuggestions when comments span multiple PRs", async () => { | |
75 | + const ctx = makeContext(); | |
76 | + ctx.db.query.pullRequestComments.findMany.mockResolvedValueOnce([ | |
77 | + { id: "comment-1", path: "a.ts", pullRequest: { id: "pr-1", repositoryId: "repo-1" } }, | |
78 | + { id: "comment-2", path: "b.ts", pullRequest: { id: "pr-2", repositoryId: "repo-1" } }, | |
79 | + ]); | |
80 | + | |
81 | + await expect( | |
82 | + resolvers.Mutation.batchApplySuggestions( | |
83 | + {}, | |
84 | + { input: { commentIds: ["comment-1", "comment-2"] } }, | |
85 | + ctx | |
86 | + ) | |
87 | + ).rejects.toThrow("All suggestions in a batch must belong to the same pull request"); | |
88 | + | |
89 | + expect(batchApplySuggestionsMock).not.toHaveBeenCalled(); | |
90 | + }); | |
91 | +}); | |
0 | 92 | |