From 017b9831f9b18728f3e47639576d0065d5f669d9 Mon Sep 17 00:00:00 2001 From: Masonlet Date: Mon, 23 Mar 2026 13:47:29 -0400 Subject: [PATCH 1/2] refactor: reorder handler, move checks before rate limiting --- api/contact/index.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/api/contact/index.ts b/api/contact/index.ts index 22b5faf..51dbf38 100644 --- a/api/contact/index.ts +++ b/api/contact/index.ts @@ -13,12 +13,6 @@ export default async (req: VercelRequest, res: VercelResponse): Promise => return; } - const { rateLimited } = await checkRateLimit("contact-form-limit"); - if (rateLimited) { - res.status(429).json({ error: "Too many requests. Please try again later." }); - return; - } - if (req.method !== "POST") { res.status(405).json({ error: "Method not allowed" }); return; @@ -29,20 +23,26 @@ export default async (req: VercelRequest, res: VercelResponse): Promise => return; } + if(req.body["fax_number"]) { + console.warn("Honeypot triggered:", req.headers["x-forwarded-for"] ?? "unknown"); + res.json({ success: true, message: "Message sent successfully" }); + return; + } + const emailConfig = getEmailConfig(config); if (!emailConfig) { res.status(503).json({ error: "Service temporarily unavailable" }); return; } - if(req.body["fax_number"]) { - console.warn("Honeypot triggered:", req.headers["x-forwarded-for"] ?? "unknown"); - res.json({ success: true, message: "Message sent successfully" }); + if (!isValidBody(req.body)) { + res.status(400).json({ error: "Invalid or missing fields" }); return; } - if (!isValidBody(req.body)) { - res.status(400).json({ error: "Invalid or missing fields" }); + const { rateLimited } = await checkRateLimit("contact-form-limit"); + if (rateLimited) { + res.status(429).json({ error: "Too many requests. Please try again later." }); return; } From 110b3941459ca0c0e0bbe630013930baae1b7f13 Mon Sep 17 00:00:00 2001 From: Masonlet Date: Mon, 23 Mar 2026 13:58:01 -0400 Subject: [PATCH 2/2] refactor: unify origin check into setCorsHeaders --- api/contact/cors.ts | 31 +++++++++++++------------ api/contact/index.ts | 8 +++---- tests/contact/cors.test.ts | 46 ++++++++++++++++--------------------- tests/contact/index.test.ts | 18 +++++++-------- 4 files changed, 48 insertions(+), 55 deletions(-) diff --git a/api/contact/cors.ts b/api/contact/cors.ts index ced3901..dc496f2 100644 --- a/api/contact/cors.ts +++ b/api/contact/cors.ts @@ -1,29 +1,30 @@ import type { VercelRequest, VercelResponse } from "@vercel/node"; -export function isOriginAllowed( - origin: string | undefined, - allowedOrigins: string[] -): boolean { - if (allowedOrigins.length === 0) return false; - return origin !== undefined && allowedOrigins.includes(origin); -} export function setCorsHeaders( req: VercelRequest, res: VercelResponse, allowedOrigins: string[] -): boolean { +): "preflight" | "forbidden" | "ok" { res.setHeader("X-Content-Type-Options", "nosniff"); const origin = req.headers["origin"]; - - if (allowedOrigins.length > 0 && allowedOrigins.includes(origin ?? "")) { - res.setHeader("Access-Control-Allow-Origin", origin!); - res.setHeader("Access-Control-Allow-Methods", "POST, OPTIONS"); - res.setHeader("Access-Control-Allow-Headers", "Content-Type"); + const isAllowed = origin && allowedOrigins.includes(origin); + + if (!isAllowed) { + if (req.method === "OPTIONS") { + res.status(403).end(); + return "preflight"; + } + return "forbidden"; } + res.setHeader("Access-Control-Allow-Origin", origin); + res.setHeader("Access-Control-Allow-Methods", "POST, OPTIONS"); + res.setHeader("Access-Control-Allow-Headers", "Content-Type"); + if (req.method === "OPTIONS") { res.status(204).end(); - return true; + return "preflight"; } - return false; + + return "ok"; } diff --git a/api/contact/index.ts b/api/contact/index.ts index 51dbf38..240e608 100644 --- a/api/contact/index.ts +++ b/api/contact/index.ts @@ -1,14 +1,14 @@ import type { VercelRequest, VercelResponse } from "@vercel/node"; import { checkRateLimit } from "@vercel/firewall" -import { setCorsHeaders, isOriginAllowed } from "./cors.js"; +import { setCorsHeaders } from "./cors.js"; import { isValidBody } from "./validation.js"; import { getEmailConfig, sendEmail } from "./email.js"; import { config } from "./config.js"; export default async (req: VercelRequest, res: VercelResponse): Promise => { - if (setCorsHeaders(req, res, config.allowedOrigins)) return; - - if (!isOriginAllowed(req.headers["origin"], config.allowedOrigins)) { + const cors = setCorsHeaders(req, res, config.allowedOrigins); + if (cors === "preflight") return; + if (cors === "forbidden") { res.status(403).json({ error: "Forbidden" }); return; } diff --git a/tests/contact/cors.test.ts b/tests/contact/cors.test.ts index 93f8ee8..23aab78 100644 --- a/tests/contact/cors.test.ts +++ b/tests/contact/cors.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from "vitest"; -import { isOriginAllowed, setCorsHeaders } from "@/api/contact/cors.js"; +import { setCorsHeaders } from "@/api/contact/cors.js"; import type { VercelRequest, VercelResponse } from "@vercel/node"; const makeReq = (origin?: string, method = "POST") => ({ @@ -17,24 +17,6 @@ const makeRes = () => { } as unknown as VercelResponse; }; -describe("isOriginAllowed", () => { - it("should return false when allowedOrigins is empty", () => { - expect(isOriginAllowed("https://example.com", [])).toBe(false); - }); - it("should return false when origin is undefined", () => { - expect(isOriginAllowed(undefined, ["https://example.com"])).toBe(false); - }); - it("should return false when origin is not in the list", () => { - expect(isOriginAllowed("https://other.com", ["https://example.com"])).toBe(false); - }); - it("should return true when origin is in the list", () => { - expect(isOriginAllowed("https://example.com", ["https://example.com"])).toBe(true); - }); - it("should return true when origin is one of many in the list", () => { - expect(isOriginAllowed("https://b.com", ["https://a.com", "https://b.com"])).toBe(true); - }); -}); - describe("setCorsHeaders", () => { it("should always set X-Content-Type-Options", () => { const req = makeReq(); @@ -66,28 +48,40 @@ describe("setCorsHeaders", () => { expect(res.setHeader).not.toHaveBeenCalledWith("Access-Control-Allow-Origin", expect.anything()); }); - it("should return true and end response on OPTIONS", () => { + it("should return 'preflight' and 204 on OPTIONS from allowed origin", () => { const req = makeReq("https://example.com", "OPTIONS"); const res = makeRes(); const result = setCorsHeaders(req, res, ["https://example.com"]); expect(res.status).toHaveBeenCalledWith(204); expect(res.end).toHaveBeenCalled(); - expect(result).toBe(true); + expect(result).toBe("preflight"); }); - it("should return true and end response on OPTIONS from a disallowed origin" , () => { + it("should return 'preflight' and 403 on OPTIONS from a disallowed origin" , () => { const req = makeReq("https://other.com", "OPTIONS"); const res = makeRes(); const result = setCorsHeaders(req, res, ["https://example.com"]); - expect(res.status).toHaveBeenCalledWith(204); + expect(res.status).toHaveBeenCalledWith(403); expect(res.setHeader).not.toHaveBeenCalledWith("Access-Control-Allow-Origin", expect.anything()); expect(res.end).toHaveBeenCalled(); - expect(result).toBe(true); + expect(result).toBe("preflight"); + }); + + it("should return 'forbidden' on non-OPTIONS requests from disallowed origin", () => { + const req = makeReq("https://example.com", "POST"); + const res = makeRes(); + expect(setCorsHeaders(req, res, ["https://other.com"])).toBe("forbidden"); + }); + + it("should return 'ok' on allowed POST request", () => { + const req = makeReq("https://example.com", "POST"); + const res = makeRes(); + expect(setCorsHeaders(req, res, ["https://example.com"])).toBe("ok"); }); - it("should return false on non-OPTIONS requests", () => { + it("should return 'forbidden' when allowedOrigins is empty", () => { const req = makeReq("https://example.com", "POST"); const res = makeRes(); - expect(setCorsHeaders(req, res, ["https://example.com"])).toBe(false); + expect(setCorsHeaders(req, res, [])).toBe("forbidden"); }); }); diff --git a/tests/contact/index.test.ts b/tests/contact/index.test.ts index ffe7de1..355c861 100644 --- a/tests/contact/index.test.ts +++ b/tests/contact/index.test.ts @@ -2,13 +2,13 @@ import { vi, describe, it, expect, beforeEach } from "vitest"; import type { VercelRequest, VercelResponse } from "@vercel/node"; vi.mock("@vercel/firewall", () => ({ checkRateLimit: vi.fn() })); -vi.mock("@/api/contact/cors.js", () => ({ setCorsHeaders: vi.fn(), isOriginAllowed: vi.fn() })); +vi.mock("@/api/contact/cors.js", () => ({ setCorsHeaders: vi.fn() })); vi.mock("@/api/contact/validation.js", () => ({ isValidBody: vi.fn() })); vi.mock("@/api/contact/email.js", () => ({ getEmailConfig: vi.fn(), sendEmail: vi.fn() })); vi.mock("@/api/contact/config.js", () => ({ config: { allowedOrigins: ["https://example.com"] } })); import { checkRateLimit } from "@vercel/firewall"; -import { setCorsHeaders, isOriginAllowed } from "@/api/contact/cors.js"; +import { setCorsHeaders } from "@/api/contact/cors.js"; import { isValidBody } from "@/api/contact/validation.js"; import { getEmailConfig, sendEmail } from "@/api/contact/email.js"; import handler from "@/api/contact/index.js"; @@ -32,16 +32,15 @@ const makeRes = (): VercelResponse => { describe("contact handler (index.ts)", () => { beforeEach(() => { vi.clearAllMocks(); - vi.mocked(setCorsHeaders).mockReturnValue(false); - vi.mocked(isOriginAllowed).mockReturnValue(true); + vi.mocked(setCorsHeaders).mockReturnValue("ok"); vi.mocked(checkRateLimit).mockResolvedValue({ rateLimited: false } as any); - vi.mocked(getEmailConfig).mockReturnValue({ client: {} as any, from: "from@test.com", to: "to@test.com" }); + vi.mocked(getEmailConfig).mockReturnValue({ client: {} as any, from: "from@test.com", to: ["to@test.com"] }); vi.mocked(isValidBody).mockReturnValue(true); vi.mocked(sendEmail).mockResolvedValue(undefined); }); - it("returns early when setCorsHeaders returns true (OPTIONS preflight)", async () => { - vi.mocked(setCorsHeaders).mockReturnValue(true); + it("returns early when setCorsHeaders returns 'preflight'", async () => { + vi.mocked(setCorsHeaders).mockReturnValue("preflight"); const req = makeReq({ method: "OPTIONS" }); const res = makeRes(); await handler(req, res); @@ -49,8 +48,8 @@ describe("contact handler (index.ts)", () => { expect(sendEmail).not.toHaveBeenCalled(); }); - it("returns 403 when origin is not allowed", async () => { - vi.mocked(isOriginAllowed).mockReturnValue(false); + it("returns 403 when setCorsHeaders returns 'forbidden'", async () => { + vi.mocked(setCorsHeaders).mockReturnValue("forbidden"); const res = makeRes(); await handler(makeReq(), res); expect(res.status).toHaveBeenCalledWith(403); @@ -58,7 +57,6 @@ describe("contact handler (index.ts)", () => { }); it("returns 429 when rate limited", async () => { - vi.mocked(isOriginAllowed).mockReturnValue(true); vi.mocked(checkRateLimit).mockResolvedValue({ rateLimited: true } as any); const res = makeRes(); await handler(makeReq(), res);