fix: harden input validation, redirect safety, and error resilience

This commit is contained in:
RhenCloud 2026-08-02 23:56:58 +08:00
parent d419b9c940
commit bd33fa6835
No known key found for this signature in database
GPG key ID: A574A617378C4E0B
8 changed files with 208 additions and 97 deletions

View file

@ -2,6 +2,7 @@ import { Hono } from "hono";
import { getUserOctokit } from "./github-oauth"; import { getUserOctokit } from "./github-oauth";
import { findUserIdByToken } from "./token-store"; import { findUserIdByToken } from "./token-store";
import type { Env } from "./types"; import type { Env } from "./types";
import { log } from "./log";
function extractBearerToken(c: { function extractBearerToken(c: {
req: { header: (name: string) => string | undefined }; req: { header: (name: string) => string | undefined };
@ -11,6 +12,24 @@ function extractBearerToken(c: {
return auth.slice(7); 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: <T>() => Promise<T> };
}): Promise<Record<string, unknown> | null> {
try {
return (await c.req.json()) as Record<string, unknown>;
} catch {
return null;
}
}
export function createActionRoutes(): Hono<{ Bindings: Env }> { export function createActionRoutes(): Hono<{ Bindings: Env }> {
const app = new 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); const userId = await findUserIdByToken(c.env.KV, token);
if (!userId) return c.json({ error: "Invalid or expired token" }, 401); if (!userId) return c.json({ error: "Invalid or expired token" }, 401);
const body = await c.req.json<{ const body = await readJson(c);
owner: string; if (
repo: string; !body ||
issueNumber: number; !isNonEmptyString(body.owner) ||
body: string; !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); const octokit = await getUserOctokit(userId, c.env.KV);
if (!octokit) return c.json({ error: "Not authorized" }, 401); if (!octokit) return c.json({ error: "Not authorized" }, 401);
await octokit.rest.issues.createComment({ try {
owner: body.owner, await octokit.rest.issues.createComment({
repo: body.repo, owner: body.owner,
issue_number: body.issueNumber, repo: body.repo,
body: body.body, 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 }); return c.json({ ok: true });
}); });
@ -45,21 +73,33 @@ export function createActionRoutes(): Hono<{ Bindings: Env }> {
const userId = await findUserIdByToken(c.env.KV, token); const userId = await findUserIdByToken(c.env.KV, token);
if (!userId) return c.json({ error: "Invalid or expired token" }, 401); if (!userId) return c.json({ error: "Invalid or expired token" }, 401);
const body = await c.req.json<{ const body = await readJson(c);
owner: string; if (
repo: string; !body ||
pullNumber: number; !isNonEmptyString(body.owner) ||
method?: "merge" | "squash" | "rebase"; !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); const octokit = await getUserOctokit(userId, c.env.KV);
if (!octokit) return c.json({ error: "Not authorized" }, 401); if (!octokit) return c.json({ error: "Not authorized" }, 401);
await octokit.rest.pulls.merge({ try {
owner: body.owner, await octokit.rest.pulls.merge({
repo: body.repo, owner: body.owner,
pull_number: body.pullNumber, repo: body.repo,
merge_method: body.method ?? "squash", 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 }); return c.json({ ok: true });
}); });
@ -70,20 +110,29 @@ export function createActionRoutes(): Hono<{ Bindings: Env }> {
const userId = await findUserIdByToken(c.env.KV, token); const userId = await findUserIdByToken(c.env.KV, token);
if (!userId) return c.json({ error: "Invalid or expired token" }, 401); if (!userId) return c.json({ error: "Invalid or expired token" }, 401);
const body = await c.req.json<{ const body = await readJson(c);
owner: string; if (
repo: string; !body ||
pullNumber: number; !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); const octokit = await getUserOctokit(userId, c.env.KV);
if (!octokit) return c.json({ error: "Not authorized" }, 401); if (!octokit) return c.json({ error: "Not authorized" }, 401);
await octokit.rest.pulls.update({ try {
owner: body.owner, await octokit.rest.pulls.update({
repo: body.repo, owner: body.owner,
pull_number: body.pullNumber, repo: body.repo,
state: "closed", 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 }); return c.json({ ok: true });
}); });
@ -94,22 +143,40 @@ export function createActionRoutes(): Hono<{ Bindings: Env }> {
const userId = await findUserIdByToken(c.env.KV, token); const userId = await findUserIdByToken(c.env.KV, token);
if (!userId) return c.json({ error: "Invalid or expired token" }, 401); if (!userId) return c.json({ error: "Invalid or expired token" }, 401);
const body = await c.req.json<{ const body = await readJson(c);
owner: string; const reactions = ["+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes"] as const;
repo: string; if (
issueNumber: number; !body ||
reaction: string; !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); const octokit = await getUserOctokit(userId, c.env.KV);
if (!octokit) return c.json({ error: "Not authorized" }, 401); if (!octokit) return c.json({ error: "Not authorized" }, 401);
await octokit.rest.reactions.createForIssue({ try {
owner: body.owner, await octokit.rest.reactions.createForIssue({
repo: body.repo, owner: body.owner,
issue_number: body.issueNumber, repo: body.repo,
content: body.reaction as issue_number: body.issueNumber,
"+1" | "-1" | "laugh" | "confused" | "heart" | "hooray" | "rocket" | "eyes", 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 }); return c.json({ ok: true });
}); });

View file

@ -39,11 +39,11 @@ export async function createAdminSession(
} }
export function adminCookie(sessionId: string): string { 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 { 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<string, string> { function parseCookies(header: string | undefined): Record<string, string> {

View file

@ -205,12 +205,18 @@ export class DiscordGateway {
} }
private handleMessage(data: string): void { private handleMessage(data: string): void {
const msg = JSON.parse(data) as { let msg: { op: number; d: unknown; s: number | null; t: string | null };
op: number; try {
d: unknown; msg = JSON.parse(data) as {
s: number | null; op: number;
t: string | null; d: unknown;
}; s: number | null;
t: string | null;
};
} catch {
log.warn("Gateway received malformed frame");
return;
}
if (msg.s !== null) this.lastSequence = msg.s; if (msg.s !== null) this.lastSequence = msg.s;

View file

@ -33,6 +33,12 @@ export async function sendMessage(
if (!res.ok) { if (!res.ok) {
const err = await res.text(); 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"); log.error({ status: res.status, err, channelId }, "Discord API error");
return { ok: false, error: err }; return { ok: false, error: err };
} }

View file

@ -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 matched = config.routes.filter((route) => matchRoute(route, event) && accepted(route));
const anyRegularMatched = matched.some((route) => !route.fallback); const anyRegularMatched = matched.some((route) => !route.fallback);
for (const route of matched) { const tasks = matched
// A fallback route only fires when no regular route matched the event. .filter((route) => !(route.fallback && anyRegularMatched))
if (route.fallback && anyRegularMatched) continue; .map(async (route) => {
const target = route.target.threadId
? `${route.target.channelId}/${route.target.threadId}`
: route.target.channelId;
const target = route.target.threadId try {
? `${route.target.channelId}/${route.target.threadId}` const tr = trMap.get(route.lang ?? "en")!;
: route.target.channelId; 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 { await Promise.allSettled(tasks);
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");
}
}
} }
async function sendToChannel( async function sendToChannel(

View file

@ -23,11 +23,19 @@ function generateRandomHex(length: number): string {
.join(""); .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 }> { export function createOAuthRoutes(): Hono<{ Bindings: Env }> {
const app = new Hono<{ Bindings: Env }>(); const app = new Hono<{ Bindings: Env }>();
app.get("/github", async (c) => { app.get("/github", async (c) => {
const redirectTo = c.req.query("redirect") ?? "/"; const redirectTo = safeRedirectPath(c.req.query("redirect"));
const state = generateRandomHex(16); const state = generateRandomHex(16);
const pending: PendingState = { const pending: PendingState = {

View file

@ -7,6 +7,7 @@ import { createActionRoutes } from "./action-routes";
import { createAdminRoutes } from "./admin-routes"; import { createAdminRoutes } from "./admin-routes";
import { createLegalRoutes } from "./legal-routes"; import { createLegalRoutes } from "./legal-routes";
import { createHomeRoutes } from "./home-routes"; import { createHomeRoutes } from "./home-routes";
import { loadConfig } from "./config";
import { log } from "./log"; import { log } from "./log";
const MAX_BODY_SIZE = 1024 * 1024; 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 }); await c.env.KV.put(`delivery:${delivery}`, "1", { expirationTtl: 300 });
} }
const { loadConfig } = await import("./config");
const config = await loadConfig(c.env); const config = await loadConfig(c.env);
const dispatch = dispatchEvent(config, event, c.env).catch((err) => const dispatch = dispatchEvent(config, event, c.env).catch((err) =>
log.error(err, "Dispatch failed"), log.error(err, "Dispatch failed"),

View file

@ -1,6 +1,30 @@
import type { WebhookEvent, Route, Filter } from "./types"; import type { WebhookEvent, Route, Filter } from "./types";
const keyCache = new Map<string, CryptoKey>(); const keyCache = new Map<string, CryptoKey>();
const regexCache = new Map<string, RegExp>();
const keywordBodyCache = new WeakMap<WebhookEvent, string>();
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<CryptoKey> { async function getHmacKey(secret: string): Promise<CryptoKey> {
const cached = keyCache.get(secret); const cached = keyCache.get(secret);
@ -109,15 +133,14 @@ function matchFilter(filter: Filter, event: WebhookEvent, keywordBody?: string):
value = extractBranch(event); value = extractBranch(event);
break; break;
case "keyword": { 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]; const patterns = Array.isArray(filter.match) ? filter.match : [filter.match];
return patterns.some((p) => { const matches = patterns.some((p) => {
try { const re = compileKeywordRegex(p);
return new RegExp(p, "i").test(body); if (!re) return body.includes(p.toLowerCase());
} catch { return re.test(body);
return body.includes(p.toLowerCase());
}
}); });
return filter.exclude ? !matches : matches;
} }
default: default:
return false; return false;
@ -145,6 +168,6 @@ export function eventOwners(event: WebhookEvent): string[] {
export function matchRoute(route: Route, event: WebhookEvent): boolean { export function matchRoute(route: Route, event: WebhookEvent): boolean {
if (!route.enabled) return false; if (!route.enabled) return false;
const hasKeyword = route.filters.some((f) => f.type === "keyword"); 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)); return route.filters.every((f) => matchFilter(f, event, keywordBody));
} }