swadhin / opencodehub public

Enforce path scope for pull request templates

swadhinbiswas committed 8 months ago
c8e1d4f
1 parent 8c3e006
commit c8e1d4f
4 files changed
+115 -2
Showing 4 changed files+115-2
doc/feature_audit.mdsrc/lib/openapi.tssrc/pages/api/repos/[owner]/[repo]/pulls/templates.tstests/integration/pull-templates-path-permissions.test.ts
▼doc/feature_audit.md
+1-1
@@ -36,7 +36,7 @@
36
36
| Branch protection rules | ✅ | Schema + UI + API in place |
37
37
| Repository templates | ⚠️ | Template creation and listing are implemented with access-aware discovery filters (`/repos/templates?q=&owner=&visibility=`) including collaborator-visible private templates; dedicated template governance UX remains partial |
38
38
| Repository Wiki | ✅ | Implemented (Phase 8) |
39
-| File-level permissions | ⚠️ | Path-scoped enforcement now covers PR comment create/edit/delete/list filtering, PR review list comment filtering, AI review suggestion read filtering, single-review inline comments, batch-review comment submission, suggestion-apply, partial file-approval create/revoke and read-filter flows, PR rewrite operations, PR impact scan execution checks, and PR impact read-result file-path filtering; broader endpoint coverage still remains |
39
+| File-level permissions | ⚠️ | Path-scoped enforcement now covers PR comment create/edit/delete/list filtering, PR review list comment filtering, AI review suggestion read filtering, single-review inline comments, batch-review comment submission, suggestion-apply, partial file-approval create/revoke and read-filter flows, PR rewrite operations, PR impact scan execution checks, PR impact read-result file-path filtering, and PR template discovery path checks; broader endpoint coverage still remains |
40
40
| Commit signing (GPG) | ✅ | GPG key management UI exists |
41
41
| Commit signing (SSH) | ✅ | SSH key management UI exists |
42
42
43
43
▼src/lib/openapi.ts
+1-1
@@ -1234,7 +1234,7 @@ export const openApiSpec = {
1234
1234
get: {
1235
1235
tags: ["Pull Requests"],
1236
1236
summary: "Get pull request template",
1237
- description: "Loads PR template content from common repository template locations for a target branch.",
1237
+ description: "Loads PR template content from common repository template locations for a target branch, with path-scoped read permission checks per candidate path.",
1238
1238
security: [{ bearerAuth: [] }],
1239
1239
parameters: [
1240
1240
{ name: "owner", in: "path", required: true, schema: { type: "string" } },
1241
1241
▼src/pages/api/repos/[owner]/[repo]/pulls/templates.ts
+4-0
@@ -4,6 +4,7 @@ import { and, eq } from "drizzle-orm";
4
4
import { resolveRepoPath } from "@/lib/git-storage";
5
5
import { getFileContent } from "@/lib/git";
6
6
import { canReadRepo } from "@/lib/permissions";
7
+import { checkPathPermissions } from "@/lib/path-scoping";
7
8
import { withErrorHandler } from "@/lib/errors";
8
9
import { notFound, success } from "@/lib/api";
9
10
@@ -53,7 +54,10 @@ export const GET: APIRoute = withErrorHandler(async ({ params, request, locals }
53
54
"pull_request_template.txt"
54
55
];
55
56
57
+ const scopedUserId = locals.user?.id || "__anonymous__";
56
58
for (const path of candidates) {
59
+ const permission = await checkPathPermissions(scopedUserId, repo.id, [path], "read");
60
+ if (!permission.allowed) continue;
57
61
const file = await getFileContent(repoPath, path, targetBranch);
58
62
if (file && !file.isBinary && file.content) {
59
63
return success({ content: file.content });
60
64
▼tests/integration/pull-templates-path-permissions.test.ts
+109-0
@@ -0,0 +1,109 @@
1
+import { beforeEach, describe, expect, it, vi } from "vitest";
2
+
3
+const {
4
+ canReadRepoMock,
5
+ checkPathPermissionsMock,
6
+ getFileContentMock,
7
+ fakeSchema,
8
+} = vi.hoisted(() => ({
9
+ canReadRepoMock: vi.fn(async () => true),
10
+ checkPathPermissionsMock: vi.fn(async () => ({ allowed: true, deniedPaths: [] })),
11
+ getFileContentMock: vi.fn(async () => null),
12
+ fakeSchema: {
13
+ users: { username: {} },
14
+ repositories: { ownerId: {}, name: {}, id: {}, diskPath: {}, defaultBranch: {} },
15
+ } as any,
16
+}));
17
+
18
+let mockDb: any;
19
+
20
+vi.mock("@/db", () => ({
21
+ getDatabase: () => mockDb,
22
+ schema: fakeSchema,
23
+}));
24
+
25
+vi.mock("@/lib/permissions", () => ({
26
+ canReadRepo: canReadRepoMock,
27
+}));
28
+
29
+vi.mock("@/lib/path-scoping", () => ({
30
+ checkPathPermissions: checkPathPermissionsMock,
31
+}));
32
+
33
+vi.mock("@/lib/git-storage", () => ({
34
+ resolveRepoPath: vi.fn(async () => "/tmp/repo"),
35
+}));
36
+
37
+vi.mock("@/lib/git", () => ({
38
+ getFileContent: getFileContentMock,
39
+}));
40
+
41
+import { GET as templatesGet } from "@/pages/api/repos/[owner]/[repo]/pulls/templates";
42
+
43
+function makeDb() {
44
+ return {
45
+ query: {
46
+ users: {
47
+ findFirst: vi.fn(async () => ({ id: "owner-1", username: "owner-1" })),
48
+ },
49
+ repositories: {
50
+ findFirst: vi.fn(async () => ({
51
+ id: "repo-1",
52
+ ownerId: "owner-1",
53
+ name: "demo",
54
+ diskPath: "/tmp/repo",
55
+ defaultBranch: "main",
56
+ })),
57
+ },
58
+ },
59
+ };
60
+}
61
+
62
+describe("pull request template path permissions", () => {
63
+ beforeEach(() => {
64
+ vi.clearAllMocks();
65
+ mockDb = makeDb();
66
+ canReadRepoMock.mockResolvedValue(true);
67
+ checkPathPermissionsMock.mockResolvedValue({ allowed: true, deniedPaths: [] });
68
+ getFileContentMock.mockResolvedValue({
69
+ isBinary: false,
70
+ content: "## PR Template",
71
+ });
72
+ });
73
+
74
+ it("skips restricted template paths and returns readable template content", async () => {
75
+ checkPathPermissionsMock.mockImplementation(async (_userId: string, _repoId: string, paths: string[]) => {
76
+ if (paths[0] === ".github/pull_request_template.md") {
77
+ return {
78
+ allowed: false,
79
+ deniedPaths: [".github/pull_request_template.md"],
80
+ reason: "denied",
81
+ };
82
+ }
83
+ return { allowed: true, deniedPaths: [] };
84
+ });
85
+
86
+ getFileContentMock.mockImplementation(async (_repoPath: string, path: string) => {
87
+ if (path === ".github/PULL_REQUEST_TEMPLATE.md") {
88
+ return { isBinary: false, content: "## Secondary Template" };
89
+ }
90
+ return null;
91
+ });
92
+
93
+ const response = await templatesGet({
94
+ params: { owner: "owner-1", repo: "demo" },
95
+ locals: { user: { id: "user-1", isAdmin: false } },
96
+ request: new Request("http://localhost/api/repos/owner-1/demo/pulls/templates"),
97
+ } as any);
98
+
99
+ const body = await response.json();
100
+ expect(response.status).toBe(200);
101
+ expect(body?.data?.content).toBe("## Secondary Template");
102
+ expect(checkPathPermissionsMock).toHaveBeenCalledWith(
103
+ "user-1",
104
+ "repo-1",
105
+ [".github/pull_request_template.md"],
106
+ "read"
107
+ );
108
+ });
109
+});
0
110