diff --git a/src/action-routes.ts b/src/action-routes.ts index 7a522ad..9766f96 100644 --- a/src/action-routes.ts +++ b/src/action-routes.ts @@ -2,6 +2,7 @@ import { Hono } from "hono"; import { getUserOctokit } from "./github-oauth"; import { findUserIdByToken } from "./token-store"; import type { Env } from "./types"; +import { log } from "./log"; function extractBearerToken(c: { req: { header: (name: string) => string | undefined }; @@ -11,6 +12,24 @@ function extractBearerToken(c: { return auth.slice(7); } +function isNonEmptyString(value: unknown): value is string { + return typeof value === "string" && value.length > 0; +} + +function isValidId(value: unknown): value is number { + return typeof value === "number" && Number.isFinite(value) && value > 0; +} + +async function readJson(c: { + req: { json: () => Promise }; +}): Promise | null> { + try { + return (await c.req.json()) as Record; + } catch { + return null; + } +} + export function createActionRoutes(): Hono<{ Bindings: Env }> { const app = new Hono<{ Bindings: Env }>(); @@ -20,21 +39,30 @@ export function createActionRoutes(): Hono<{ Bindings: Env }> { const userId = await findUserIdByToken(c.env.KV, token); if (!userId) return c.json({ error: "Invalid or expired token" }, 401); - const body = await c.req.json<{ - owner: string; - repo: string; - issueNumber: number; - body: string; - }>(); + const body = await readJson(c); + if ( + !body || + !isNonEmptyString(body.owner) || + !isNonEmptyString(body.repo) || + !isValidId(body.issueNumber) || + !isNonEmptyString(body.body) + ) { + return c.json({ error: "Invalid request body" }, 400); + } const octokit = await getUserOctokit(userId, c.env.KV); if (!octokit) return c.json({ error: "Not authorized" }, 401); - await octokit.rest.issues.createComment({ - owner: body.owner, - repo: body.repo, - issue_number: body.issueNumber, - body: body.body, - }); + try { + await octokit.rest.issues.createComment({ + owner: body.owner, + repo: body.repo, + issue_number: body.issueNumber, + body: body.body, + }); + } catch (err) { + log.error({ err }, "Failed to create comment"); + return c.json({ error: "GitHub API error" }, 500); + } return c.json({ ok: true }); }); @@ -45,21 +73,33 @@ export function createActionRoutes(): Hono<{ Bindings: Env }> { const userId = await findUserIdByToken(c.env.KV, token); if (!userId) return c.json({ error: "Invalid or expired token" }, 401); - const body = await c.req.json<{ - owner: string; - repo: string; - pullNumber: number; - method?: "merge" | "squash" | "rebase"; - }>(); + const body = await readJson(c); + if ( + !body || + !isNonEmptyString(body.owner) || + !isNonEmptyString(body.repo) || + !isValidId(body.pullNumber) + ) { + return c.json({ error: "Invalid request body" }, 400); + } + const method = body.method === undefined ? "squash" : body.method; + if (method !== "merge" && method !== "squash" && method !== "rebase") { + return c.json({ error: "Invalid request body" }, 400); + } const octokit = await getUserOctokit(userId, c.env.KV); if (!octokit) return c.json({ error: "Not authorized" }, 401); - await octokit.rest.pulls.merge({ - owner: body.owner, - repo: body.repo, - pull_number: body.pullNumber, - merge_method: body.method ?? "squash", - }); + try { + await octokit.rest.pulls.merge({ + owner: body.owner, + repo: body.repo, + pull_number: body.pullNumber, + merge_method: method, + }); + } catch (err) { + log.error({ err }, "Failed to merge pull request"); + return c.json({ error: "GitHub API error" }, 500); + } return c.json({ ok: true }); }); @@ -70,20 +110,29 @@ export function createActionRoutes(): Hono<{ Bindings: Env }> { const userId = await findUserIdByToken(c.env.KV, token); if (!userId) return c.json({ error: "Invalid or expired token" }, 401); - const body = await c.req.json<{ - owner: string; - repo: string; - pullNumber: number; - }>(); + const body = await readJson(c); + if ( + !body || + !isNonEmptyString(body.owner) || + !isNonEmptyString(body.repo) || + !isValidId(body.pullNumber) + ) { + return c.json({ error: "Invalid request body" }, 400); + } const octokit = await getUserOctokit(userId, c.env.KV); if (!octokit) return c.json({ error: "Not authorized" }, 401); - await octokit.rest.pulls.update({ - owner: body.owner, - repo: body.repo, - pull_number: body.pullNumber, - state: "closed", - }); + try { + await octokit.rest.pulls.update({ + owner: body.owner, + repo: body.repo, + pull_number: body.pullNumber, + state: "closed", + }); + } catch (err) { + log.error({ err }, "Failed to close pull request"); + return c.json({ error: "GitHub API error" }, 500); + } return c.json({ ok: true }); }); @@ -94,22 +143,40 @@ export function createActionRoutes(): Hono<{ Bindings: Env }> { const userId = await findUserIdByToken(c.env.KV, token); if (!userId) return c.json({ error: "Invalid or expired token" }, 401); - const body = await c.req.json<{ - owner: string; - repo: string; - issueNumber: number; - reaction: string; - }>(); + const body = await readJson(c); + const reactions = ["+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes"] as const; + if ( + !body || + !isNonEmptyString(body.owner) || + !isNonEmptyString(body.repo) || + !isValidId(body.issueNumber) || + !isNonEmptyString(body.reaction) || + !(reactions as readonly string[]).includes(body.reaction) + ) { + return c.json({ error: "Invalid request body" }, 400); + } const octokit = await getUserOctokit(userId, c.env.KV); if (!octokit) return c.json({ error: "Not authorized" }, 401); - await octokit.rest.reactions.createForIssue({ - owner: body.owner, - repo: body.repo, - issue_number: body.issueNumber, - content: body.reaction as - "+1" | "-1" | "laugh" | "confused" | "heart" | "hooray" | "rocket" | "eyes", - }); + try { + await octokit.rest.reactions.createForIssue({ + owner: body.owner, + repo: body.repo, + issue_number: body.issueNumber, + content: body.reaction as + | "+1" + | "-1" + | "laugh" + | "confused" + | "heart" + | "hooray" + | "rocket" + | "eyes", + }); + } catch (err) { + log.error({ err }, "Failed to create reaction"); + return c.json({ error: "GitHub API error" }, 500); + } return c.json({ ok: true }); }); diff --git a/src/admin-session.ts b/src/admin-session.ts index 8fb186f..2ed1aff 100644 --- a/src/admin-session.ts +++ b/src/admin-session.ts @@ -39,11 +39,11 @@ export async function createAdminSession( } export function adminCookie(sessionId: string): string { - return `${SESSION_COOKIE}=${sessionId}; Path=/; HttpOnly; SameSite=Lax; Max-Age=${SESSION_TTL}`; + return `${SESSION_COOKIE}=${sessionId}; Path=/; HttpOnly; Secure; SameSite=Lax; Max-Age=${SESSION_TTL}`; } export function clearAdminCookie(): string { - return `${SESSION_COOKIE}=; Path=/; HttpOnly; SameSite=Lax; Max-Age=0`; + return `${SESSION_COOKIE}=; Path=/; HttpOnly; Secure; SameSite=Lax; Max-Age=0`; } function parseCookies(header: string | undefined): Record { diff --git a/src/discord-gateway.ts b/src/discord-gateway.ts index 4ded903..bee902b 100644 --- a/src/discord-gateway.ts +++ b/src/discord-gateway.ts @@ -205,12 +205,18 @@ export class DiscordGateway { } private handleMessage(data: string): void { - const msg = JSON.parse(data) as { - op: number; - d: unknown; - s: number | null; - t: string | null; - }; + let msg: { op: number; d: unknown; s: number | null; t: string | null }; + try { + msg = JSON.parse(data) as { + op: number; + d: unknown; + s: number | null; + t: string | null; + }; + } catch { + log.warn("Gateway received malformed frame"); + return; + } if (msg.s !== null) this.lastSequence = msg.s; diff --git a/src/discord-rest.ts b/src/discord-rest.ts index d940b91..17aacf5 100644 --- a/src/discord-rest.ts +++ b/src/discord-rest.ts @@ -33,6 +33,12 @@ export async function sendMessage( if (!res.ok) { const err = await res.text(); + if (res.status >= 500) { + log.error({ status: res.status, err, attempt, channelId }, "Discord API 5xx"); + if (attempt === 2) return { ok: false, error: err }; + await new Promise((r) => setTimeout(r, 1000 * (attempt + 1))); + continue; + } log.error({ status: res.status, err, channelId }, "Discord API error"); return { ok: false, error: err }; } diff --git a/src/discord.ts b/src/discord.ts index d11432f..37ab3b2 100644 --- a/src/discord.ts +++ b/src/discord.ts @@ -53,39 +53,40 @@ export async function dispatchEvent(config: Config, event: WebhookEvent, env: En const matched = config.routes.filter((route) => matchRoute(route, event) && accepted(route)); const anyRegularMatched = matched.some((route) => !route.fallback); - for (const route of matched) { - // A fallback route only fires when no regular route matched the event. - if (route.fallback && anyRegularMatched) continue; + const tasks = matched + .filter((route) => !(route.fallback && anyRegularMatched)) + .map(async (route) => { + const target = route.target.threadId + ? `${route.target.channelId}/${route.target.threadId}` + : route.target.channelId; - const target = route.target.threadId - ? `${route.target.channelId}/${route.target.threadId}` - : route.target.channelId; + try { + const tr = trMap.get(route.lang ?? "en")!; + const message = formatEvent(route, event, tr); + await sendToChannel(route.target.channelId, message, env, route.target.threadId); + await recordSend(env.KV, { + ts: Date.now(), + routeId: route.id, + event: event.event, + repo: (event.payload.repository as { full_name?: string } | undefined)?.full_name, + target, + ok: true, + }); + } catch (err) { + await recordSend(env.KV, { + ts: Date.now(), + routeId: route.id, + event: event.event, + repo: (event.payload.repository as { full_name?: string } | undefined)?.full_name, + target, + ok: false, + error: err instanceof Error ? err.message : String(err), + }); + log.error({ routeId: route.id, err }, "Route failed"); + } + }); - try { - const tr = trMap.get(route.lang ?? "en")!; - const message = formatEvent(route, event, tr); - await sendToChannel(route.target.channelId, message, env, route.target.threadId); - await recordSend(env.KV, { - ts: Date.now(), - routeId: route.id, - event: event.event, - repo: (event.payload.repository as { full_name?: string } | undefined)?.full_name, - target, - ok: true, - }); - } catch (err) { - await recordSend(env.KV, { - ts: Date.now(), - routeId: route.id, - event: event.event, - repo: (event.payload.repository as { full_name?: string } | undefined)?.full_name, - target, - ok: false, - error: err instanceof Error ? err.message : String(err), - }); - log.error({ routeId: route.id, err }, "Route failed"); - } - } + await Promise.allSettled(tasks); } async function sendToChannel( diff --git a/src/oauth-routes.ts b/src/oauth-routes.ts index 3ae2c77..167f60e 100644 --- a/src/oauth-routes.ts +++ b/src/oauth-routes.ts @@ -23,11 +23,19 @@ function generateRandomHex(length: number): string { .join(""); } +function safeRedirectPath(value: string | undefined): string { + if (!value) return "/"; + if (!value.startsWith("/")) return "/"; + if (value.startsWith("//")) return "/"; + if (/^\/\\/.test(value)) return "/"; + return value; +} + export function createOAuthRoutes(): Hono<{ Bindings: Env }> { const app = new Hono<{ Bindings: Env }>(); app.get("/github", async (c) => { - const redirectTo = c.req.query("redirect") ?? "/"; + const redirectTo = safeRedirectPath(c.req.query("redirect")); const state = generateRandomHex(16); const pending: PendingState = { diff --git a/src/server.ts b/src/server.ts index 8ef9cb6..b4a2955 100644 --- a/src/server.ts +++ b/src/server.ts @@ -7,6 +7,7 @@ import { createActionRoutes } from "./action-routes"; import { createAdminRoutes } from "./admin-routes"; import { createLegalRoutes } from "./legal-routes"; import { createHomeRoutes } from "./home-routes"; +import { loadConfig } from "./config"; import { log } from "./log"; const MAX_BODY_SIZE = 1024 * 1024; @@ -58,7 +59,6 @@ export function createServer(): Hono<{ Bindings: Env }> { await c.env.KV.put(`delivery:${delivery}`, "1", { expirationTtl: 300 }); } - const { loadConfig } = await import("./config"); const config = await loadConfig(c.env); const dispatch = dispatchEvent(config, event, c.env).catch((err) => log.error(err, "Dispatch failed"), diff --git a/src/webhook.ts b/src/webhook.ts index 03cf02b..de79a95 100644 --- a/src/webhook.ts +++ b/src/webhook.ts @@ -1,6 +1,30 @@ import type { WebhookEvent, Route, Filter } from "./types"; const keyCache = new Map(); +const regexCache = new Map(); +const keywordBodyCache = new WeakMap(); +const MAX_PATTERN_LENGTH = 200; + +function compileKeywordRegex(pattern: string): RegExp | null { + if (pattern.length > MAX_PATTERN_LENGTH) return null; + const cached = regexCache.get(pattern); + if (cached) return cached; + try { + const re = new RegExp(pattern, "i"); + regexCache.set(pattern, re); + return re; + } catch { + return null; + } +} + +function getKeywordBody(event: WebhookEvent): string { + const cached = keywordBodyCache.get(event); + if (cached !== undefined) return cached; + const body = JSON.stringify(event.payload).toLowerCase(); + keywordBodyCache.set(event, body); + return body; +} async function getHmacKey(secret: string): Promise { const cached = keyCache.get(secret); @@ -109,15 +133,14 @@ function matchFilter(filter: Filter, event: WebhookEvent, keywordBody?: string): value = extractBranch(event); break; case "keyword": { - const body = keywordBody ?? JSON.stringify(event.payload).toLowerCase(); + const body = keywordBody ?? getKeywordBody(event); const patterns = Array.isArray(filter.match) ? filter.match : [filter.match]; - return patterns.some((p) => { - try { - return new RegExp(p, "i").test(body); - } catch { - return body.includes(p.toLowerCase()); - } + const matches = patterns.some((p) => { + const re = compileKeywordRegex(p); + if (!re) return body.includes(p.toLowerCase()); + return re.test(body); }); + return filter.exclude ? !matches : matches; } default: return false; @@ -145,6 +168,6 @@ export function eventOwners(event: WebhookEvent): string[] { export function matchRoute(route: Route, event: WebhookEvent): boolean { if (!route.enabled) return false; const hasKeyword = route.filters.some((f) => f.type === "keyword"); - const keywordBody = hasKeyword ? JSON.stringify(event.payload).toLowerCase() : undefined; + const keywordBody = hasKeyword ? getKeywordBody(event) : undefined; return route.filters.every((f) => matchFilter(f, event, keywordBody)); }