swadhin / opencodehub public

Harden workflow state endpoints with auth and safety checks

swadhinbiswas committed 8 months ago
7a069c9
1 parent 93f6489
commit 7a069c9
3 files changed
+233 -66
Showing 3 changed files+233-66
src/pages/api/repos/[owner]/[repo]/workflow/states.tssrc/pages/api/repos/[owner]/[repo]/workflow/states/[id].tstests/integration/workflow-states-route.test.ts
▼src/pages/api/repos/[owner]/[repo]/workflow/states.ts
+72-51
@@ -2,78 +2,99 @@
2
2
import type { APIRoute } from "astro";
3
3
import { getDatabase, schema } from "@/db";
4
4
import { eq, and } from "drizzle-orm";
5
-import { canAdminRepo } from "@/lib/permissions";
5
+import { canAdminRepo, canReadRepo } from "@/lib/permissions";
6
6
import { generateId } from "@/lib/utils";
7
7
import { z } from "zod";
8
+import { badRequest, forbidden, notFound, success, unauthorized, withErrorHandler } from "@/lib/api";
8
9
9
-export const GET: APIRoute = async ({ params, request, locals }) => {
10
- const { owner, repo } = params;
11
- const db = getDatabase();
10
+const createStateSchema = z.object({
11
+ name: z.string().min(1),
12
+ description: z.string().optional(),
13
+});
14
+
15
+function normalizeStateName(name: string): string {
16
+ return name.toLowerCase().trim().replace(/\s+/g, "_");
17
+}
12
18
13
- const repository = await db.query.repositories.findFirst({
19
+async function resolveRepository(db: ReturnType<typeof getDatabase>, owner: string, repo: string) {
20
+ const ownerUser = await db.query.users.findFirst({
21
+ where: eq(schema.users.username, owner),
22
+ });
23
+ if (!ownerUser) return null;
24
+ return db.query.repositories.findFirst({
14
25
where: and(
15
- eq(schema.repositories.name, repo!),
16
- eq(schema.repositories.ownerId, (await db.query.users.findFirst({
17
- where: eq(schema.users.username, owner!)
18
- }))?.id || "")
19
- )
26
+ eq(schema.repositories.name, repo),
27
+ eq(schema.repositories.ownerId, ownerUser.id)
28
+ ),
20
29
});
30
+}
31
+
32
+export const GET: APIRoute = withErrorHandler(async ({ params, locals }) => {
33
+ const { owner, repo } = params;
34
+ const currentUser = locals.user;
35
+ const db = getDatabase();
21
36
22
- if (!repository) return new Response("Repository not found", { status: 404 });
37
+ if (!currentUser) return unauthorized();
38
+ if (!owner || !repo) return badRequest("Missing parameters");
39
+
40
+ const repository = await resolveRepository(db, owner, repo);
41
+ if (!repository) return notFound("Repository not found");
42
+ if (!(await canReadRepo(currentUser.id, repository, { isAdmin: currentUser.isAdmin ?? undefined }))) {
43
+ return notFound("Repository not found");
44
+ }
23
45
24
46
const states = await db.query.prStateDefinitions.findMany({
25
47
where: eq(schema.prStateDefinitions.repositoryId, repository.id),
26
48
orderBy: (states, { asc }) => [asc(states.order)]
27
49
});
28
50
29
- return new Response(JSON.stringify(states), {
30
- headers: { "Content-Type": "application/json" }
31
- });
32
-};
51
+ return success(states);
52
+});
33
53
34
-export const POST: APIRoute = async ({ params, request, locals }) => {
54
+export const POST: APIRoute = withErrorHandler(async ({ params, request, locals }) => {
35
55
const { owner, repo } = params;
36
56
const db = getDatabase();
37
57
const currentUser = locals.user;
38
58
39
- if (!currentUser) return new Response("Unauthorized", { status: 401 });
59
+ if (!currentUser) return unauthorized();
60
+ if (!owner || !repo) return badRequest("Missing parameters");
61
+ const repository = await resolveRepository(db, owner, repo);
62
+ if (!repository) return notFound("Repository not found");
63
+
64
+ if (!await canAdminRepo(currentUser.id, repository)) {
65
+ return forbidden();
66
+ }
67
+
68
+ const body = await request.json().catch(() => null);
69
+ const parsed = createStateSchema.safeParse(body);
70
+ if (!parsed.success) {
71
+ return badRequest(parsed.error.issues[0]?.message || "Invalid state payload");
72
+ }
40
73
41
- const repository = await db.query.repositories.findFirst({
74
+ const normalizedName = normalizeStateName(parsed.data.name);
75
+ const existingByName = await db.query.prStateDefinitions.findFirst({
42
76
where: and(
43
- eq(schema.repositories.name, repo!),
44
- eq(schema.repositories.ownerId, (await db.query.users.findFirst({
45
- where: eq(schema.users.username, owner!)
46
- }))?.id || "")
47
- )
77
+ eq(schema.prStateDefinitions.repositoryId, repository.id),
78
+ eq(schema.prStateDefinitions.name, normalizedName)
79
+ ),
48
80
});
81
+ if (existingByName) {
82
+ return badRequest(`State '${normalizedName}' already exists`);
83
+ }
49
84
50
- if (!repository) return new Response("Repository not found", { status: 404 });
85
+ const id = generateId("prstate");
86
+ const count = await db.$count(schema.prStateDefinitions, eq(schema.prStateDefinitions.repositoryId, repository.id));
51
87
52
- if (!await canAdminRepo(currentUser.id, repository)) {
53
- return new Response("Forbidden", { status: 403 });
54
- }
88
+ // @ts-expect-error - Drizzle union type mismatch
89
+ const [newState] = await db.insert(schema.prStateDefinitions).values({
90
+ id,
91
+ repositoryId: repository.id,
92
+ name: normalizedName,
93
+ displayName: parsed.data.name.trim(),
94
+ description: parsed.data.description,
95
+ order: count,
96
+ color: "#808080",
97
+ }).returning();
55
98
56
- try {
57
- const body = await request.json();
58
- const name = z.string().min(1).parse(body.name);
59
- const description = z.string().optional().parse(body.description);
60
-
61
- const id = generateId("prstate");
62
- const count = await db.$count(schema.prStateDefinitions, eq(schema.prStateDefinitions.repositoryId, repository.id));
63
-
64
- // @ts-expect-error - Drizzle union type mismatch
65
- const [newState] = await db.insert(schema.prStateDefinitions).values({
66
- id,
67
- repositoryId: repository.id,
68
- name: name.toLowerCase().replace(/ /g, "_"),
69
- displayName: name,
70
- description,
71
- order: count,
72
- color: "#808080", // Default gray
73
- }).returning();
74
-
75
- return new Response(JSON.stringify(newState), { status: 201 });
76
- } catch (e: any) {
77
- return new Response(JSON.stringify({ error: e.message }), { status: 400 });
78
- }
79
-};
99
+ return success(newState);
100
+});
80
101
▼src/pages/api/repos/[owner]/[repo]/workflow/states/[id].ts
+37-15
@@ -2,35 +2,57 @@ import type { APIRoute } from "astro";
2
2
import { getDatabase, schema } from "@/db";
3
3
import { eq, and } from "drizzle-orm";
4
4
import { canAdminRepo } from "@/lib/permissions";
5
+import { badRequest, forbidden, notFound, success, unauthorized, withErrorHandler } from "@/lib/api";
5
6
6
-export const DELETE: APIRoute = async ({ params, request, locals }) => {
7
+async function resolveRepository(db: ReturnType<typeof getDatabase>, owner: string, repo: string) {
8
+ const ownerUser = await db.query.users.findFirst({
9
+ where: eq(schema.users.username, owner),
10
+ });
11
+ if (!ownerUser) return null;
12
+ return db.query.repositories.findFirst({
13
+ where: and(
14
+ eq(schema.repositories.name, repo),
15
+ eq(schema.repositories.ownerId, ownerUser.id)
16
+ ),
17
+ });
18
+}
19
+
20
+export const DELETE: APIRoute = withErrorHandler(async ({ params, locals }) => {
7
21
const { owner, repo, id } = params;
8
22
const db = getDatabase();
9
23
const currentUser = locals.user;
10
24
11
- if (!currentUser) return new Response("Unauthorized", { status: 401 });
25
+ if (!currentUser) return unauthorized();
26
+ if (!owner || !repo || !id) return badRequest("Missing parameters");
27
+ const repository = await resolveRepository(db, owner, repo);
28
+ if (!repository) return notFound("Repository not found");
12
29
13
- const repository = await db.query.repositories.findFirst({
30
+ if (!await canAdminRepo(currentUser.id, repository)) {
31
+ return forbidden();
32
+ }
33
+
34
+ const existing = await db.query.prStateDefinitions.findFirst({
14
35
where: and(
15
- eq(schema.repositories.name, repo!),
16
- eq(schema.repositories.ownerId, (await db.query.users.findFirst({
17
- where: eq(schema.users.username, owner!)
18
- }))?.id || "")
19
- )
36
+ eq(schema.prStateDefinitions.id, id),
37
+ eq(schema.prStateDefinitions.repositoryId, repository.id)
38
+ ),
20
39
});
40
+ if (!existing) return notFound("State not found");
21
41
22
- if (!repository) return new Response("Repository not found", { status: 404 });
23
-
24
- if (!await canAdminRepo(currentUser.id, repository)) {
25
- return new Response("Forbidden", { status: 403 });
42
+ const inUse = await db.query.pullRequests.findFirst({
43
+ where: eq(schema.pullRequests.stateId, id),
44
+ columns: { id: true },
45
+ });
46
+ if (inUse) {
47
+ return badRequest("Cannot delete state that is currently assigned to pull requests");
26
48
}
27
49
28
50
// @ts-expect-error - Drizzle union type mismatch
29
51
await db.delete(schema.prStateDefinitions)
30
52
.where(and(
31
- eq(schema.prStateDefinitions.id, id!),
53
+ eq(schema.prStateDefinitions.id, id),
32
54
eq(schema.prStateDefinitions.repositoryId, repository.id)
33
55
));
34
56
35
- return new Response(null, { status: 204 });
36
-};
57
+ return success({ deleted: true });
58
+});
37
59
▼tests/integration/workflow-states-route.test.ts
+124-0
@@ -0,0 +1,124 @@
1
+import { beforeEach, describe, expect, it, vi } from "vitest";
2
+
3
+const { canReadRepoMock, canAdminRepoMock, fakeSchema } = vi.hoisted(() => ({
4
+ canReadRepoMock: vi.fn(async () => true),
5
+ canAdminRepoMock: vi.fn(async () => true),
6
+ fakeSchema: {
7
+ users: { username: {} },
8
+ repositories: { name: {}, ownerId: {}, id: {} },
9
+ prStateDefinitions: { repositoryId: {}, name: {}, id: {} },
10
+ pullRequests: { stateId: {}, id: {} },
11
+ } as any,
12
+}));
13
+
14
+let mockDb: any;
15
+
16
+vi.mock("@/db", () => ({
17
+ getDatabase: () => mockDb,
18
+ schema: fakeSchema,
19
+}));
20
+
21
+vi.mock("@/lib/permissions", () => ({
22
+ canReadRepo: canReadRepoMock,
23
+ canAdminRepo: canAdminRepoMock,
24
+}));
25
+
26
+import { GET as getStates, POST as createState } from "@/pages/api/repos/[owner]/[repo]/workflow/states";
27
+import { DELETE as deleteState } from "@/pages/api/repos/[owner]/[repo]/workflow/states/[id]";
28
+
29
+function makeDb() {
30
+ const owner = { id: "owner-1" };
31
+ const repository = { id: "repo-1", ownerId: "owner-1", name: "demo" };
32
+ const existingState = { id: "state-1", repositoryId: "repo-1", name: "in_review" };
33
+
34
+ const insertCalls: unknown[] = [];
35
+ const deleteCalls: unknown[] = [];
36
+
37
+ return {
38
+ query: {
39
+ users: {
40
+ findFirst: vi.fn(async () => owner),
41
+ },
42
+ repositories: {
43
+ findFirst: vi.fn(async () => repository),
44
+ },
45
+ prStateDefinitions: {
46
+ findMany: vi.fn(async () => [existingState]),
47
+ findFirst: vi.fn(async (args?: any) => {
48
+ if (args?.where) {
49
+ // Existence checks in route; default to found.
50
+ return existingState;
51
+ }
52
+ return existingState;
53
+ }),
54
+ },
55
+ pullRequests: {
56
+ findFirst: vi.fn(async () => null),
57
+ },
58
+ },
59
+ $count: vi.fn(async () => 1),
60
+ insert: vi.fn(() => ({
61
+ values: vi.fn(async (value: unknown) => {
62
+ insertCalls.push(value);
63
+ return [{ id: "new-state", name: "qa", repositoryId: "repo-1" }];
64
+ }),
65
+ })),
66
+ delete: vi.fn(() => ({
67
+ where: vi.fn(async (value: unknown) => {
68
+ deleteCalls.push(value);
69
+ }),
70
+ })),
71
+ __state: { insertCalls, deleteCalls },
72
+ };
73
+}
74
+
75
+async function readJson(response: Response): Promise<any> {
76
+ return response.json();
77
+}
78
+
79
+describe("workflow states route", () => {
80
+ beforeEach(() => {
81
+ mockDb = makeDb();
82
+ canReadRepoMock.mockResolvedValue(true);
83
+ canAdminRepoMock.mockResolvedValue(true);
84
+ });
85
+
86
+ it("rejects GET when unauthenticated", async () => {
87
+ const response = await getStates({
88
+ params: { owner: "owner-1", repo: "demo" },
89
+ locals: {},
90
+ } as any);
91
+
92
+ expect(response.status).toBe(401);
93
+ });
94
+
95
+ it("rejects duplicate state names on POST", async () => {
96
+ const response = await createState({
97
+ params: { owner: "owner-1", repo: "demo" },
98
+ locals: { user: { id: "admin-1", isAdmin: false } },
99
+ request: new Request("http://localhost/api/repos/owner-1/demo/workflow/states", {
100
+ method: "POST",
101
+ headers: { "content-type": "application/json" },
102
+ body: JSON.stringify({ name: "In Review" }),
103
+ }),
104
+ } as any);
105
+
106
+ const body = await readJson(response);
107
+ expect(response.status).toBe(400);
108
+ expect(body?.error?.code).toBe("BAD_REQUEST");
109
+ });
110
+
111
+ it("blocks DELETE when state is assigned to pull requests", async () => {
112
+ mockDb.query.pullRequests.findFirst.mockResolvedValueOnce({ id: "pr-1" });
113
+
114
+ const response = await deleteState({
115
+ params: { owner: "owner-1", repo: "demo", id: "state-1" },
116
+ locals: { user: { id: "admin-1", isAdmin: false } },
117
+ } as any);
118
+
119
+ const body = await readJson(response);
120
+ expect(response.status).toBe(400);
121
+ expect(body?.error?.code).toBe("BAD_REQUEST");
122
+ expect(mockDb.__state.deleteCalls).toHaveLength(0);
123
+ });
124
+});
0
125