From 03eef7ae8999350912e6bee0989b063ba6ade5c2 Mon Sep 17 00:00:00 2001 From: RhenCloud Date: Sat, 8 Aug 2026 23:00:15 +0800 Subject: [PATCH] fix: allow duplicate route IDs across groups and add group_id to send_logs - Change validateRoutes to per-group ID uniqueness instead of global - Remove post-merge cross-group route ID uniqueness checks - Include groupId in msg:* KV key to prevent collision - Add group_id column to send_logs table (migration 0004) - Record groupId in send_logs for accurate permission filtering - Use groupId from log entries for admin log access checks --- migrations/0004_add_group_id.sql | 1 + src/__tests__/send-log.test.ts | 1 + src/core/dispatch.ts | 5 ++++- src/lib/send-log.ts | 10 ++++++--- src/web/admin-routes.ts | 38 +++++++++++--------------------- 5 files changed, 26 insertions(+), 29 deletions(-) create mode 100644 migrations/0004_add_group_id.sql diff --git a/migrations/0004_add_group_id.sql b/migrations/0004_add_group_id.sql new file mode 100644 index 0000000..f7573c8 --- /dev/null +++ b/migrations/0004_add_group_id.sql @@ -0,0 +1 @@ +ALTER TABLE send_logs ADD COLUMN group_id TEXT; \ No newline at end of file diff --git a/src/__tests__/send-log.test.ts b/src/__tests__/send-log.test.ts index 0844e5a..ccfe567 100644 --- a/src/__tests__/send-log.test.ts +++ b/src/__tests__/send-log.test.ts @@ -6,6 +6,7 @@ function createMockDB(): D1Database { const insertCols = [ "ts", "route_id", + "group_id", "event", "repo", "target", diff --git a/src/core/dispatch.ts b/src/core/dispatch.ts index ac3c51a..8713625 100644 --- a/src/core/dispatch.ts +++ b/src/core/dispatch.ts @@ -69,6 +69,7 @@ export async function dispatchEvent(config: Config, event: WebhookEvent, env: En const base: { ts: number; routeId: string; + groupId: string | undefined; event: string; repo: string | undefined; target: string; @@ -78,6 +79,7 @@ export async function dispatchEvent(config: Config, event: WebhookEvent, env: En } = { ts: Date.now(), routeId: route.id, + groupId: route.groupId, event: event.event, repo: (event.payload.repository as { full_name?: string } | undefined)?.full_name, target: targetStr, @@ -91,7 +93,8 @@ export async function dispatchEvent(config: Config, event: WebhookEvent, env: En const driver = getDriver(target); let result: SendResult; if (message.updateKey) { - const kvKey = `msg:${route.id}:${message.updateKey}:${targetStr}`; + const groupPrefix = route.groupId ? `${route.groupId}:` : ""; + const kvKey = `msg:${groupPrefix}${route.id}:${message.updateKey}:${targetStr}`; const existingId = await env.KV.get(kvKey); if (existingId) { result = await driver.edit(message, target, env, existingId); diff --git a/src/lib/send-log.ts b/src/lib/send-log.ts index e23878f..18e8c63 100644 --- a/src/lib/send-log.ts +++ b/src/lib/send-log.ts @@ -4,6 +4,7 @@ export interface SendRecord { id?: number; ts: number; routeId: string; + groupId?: string; event: string; repo?: string; target: string; @@ -22,12 +23,13 @@ export interface SendRecord { } const COLUMNS = - "id, ts, route_id, event, repo, target, ok, error, status, message_id, delivery_id, platform, actor, action, duration_ms, error_code, attempts, detail"; + "id, ts, route_id, group_id, event, repo, target, ok, error, status, message_id, delivery_id, platform, actor, action, duration_ms, error_code, attempts, detail"; interface LogRow { id: number; ts: number; route_id: string; + group_id: string | null; event: string; repo: string | null; target: string; @@ -50,6 +52,7 @@ function toRecord(r: LogRow): SendRecord { id: r.id, ts: r.ts, routeId: r.route_id, + groupId: r.group_id ?? undefined, event: r.event, repo: r.repo ?? undefined, target: r.target, @@ -72,12 +75,13 @@ export async function recordSend(db: D1Database, record: SendRecord): Promise 200) return { ok: false, error: "too many routes" }; - const seen = new Set(); + const seenByGroup = new Map>(); for (let i = 0; i < routes.length; i++) { const r = routes[i] as Record; if (!r || typeof r !== "object") return { ok: false, error: `route[${i}] is not an object` }; if (typeof r.id !== "string" || !ID_RE.test(r.id)) { return { ok: false, error: `route[${i}].id is invalid` }; } - if (seen.has(r.id)) return { ok: false, error: `duplicate route id "${r.id}"` }; - seen.add(r.id); + const gid = (r.groupId as string) ?? "__nogroup__"; + let groupSeen = seenByGroup.get(gid); + if (!groupSeen) { + groupSeen = new Set(); + seenByGroup.set(gid, groupSeen); + } + if (groupSeen.has(r.id)) { + return { ok: false, error: `duplicate route id "${r.id}" in group "${gid}"` }; + } + groupSeen.add(r.id); // Skip full validation for routes that are unchanged from what is stored. const prev = unchanged?.get(r.id); if (prev && deepEqual(r, prev)) continue; @@ -282,12 +290,8 @@ export function createAdminRoutes(): Hono<{ Bindings: Env }> { if (s.scope.isSuper) { return c.json({ logs: await getSendLog(c.env.DB, limit) }); } - const all = await loadRoutes(c.env.KV); - const allowed = new Set( - all.filter((r) => r.groupId != null && s.scope.groupIds.has(r.groupId)).map((r) => r.id), - ); const logs = (await getSendLog(c.env.DB, 200)) - .filter((l) => allowed.has(l.routeId)) + .filter((l) => l.groupId != null && s.scope.groupIds.has(l.groupId)) .slice(0, limit); return c.json({ logs }); }); @@ -300,9 +304,7 @@ export function createAdminRoutes(): Hono<{ Bindings: Env }> { const entry = await getSendLogById(c.env.DB, id); if (!entry) return c.json({ error: "Log entry not found" }, 404); if (!s.scope.isSuper) { - const all = await loadRoutes(c.env.KV); - const route = all.find((r) => r.id === entry.routeId); - if (!route?.groupId || !s.scope.groupIds.has(route.groupId)) { + if (!entry.groupId || !s.scope.groupIds.has(entry.groupId)) { return c.json({ error: "Forbidden" }, 403); } } @@ -344,13 +346,6 @@ export function createAdminRoutes(): Hono<{ Bindings: Env }> { ]; } - // Guard against duplicate ids across the merged set. - const ids = new Set(); - for (const r of nextAll) { - if (ids.has(r.id)) return c.json({ error: `duplicate route id "${r.id}"` }, 400); - ids.add(r.id); - } - try { await saveRoutes(c.env.KV, nextAll); } catch (err) { @@ -422,13 +417,6 @@ export function createAdminRoutes(): Hono<{ Bindings: Env }> { const others = existing.filter((r) => r.groupId !== groupId); const nextAll = [...others, ...result.routes]; - // Guard against ids colliding with routes in other groups. - const ids = new Set(); - for (const r of nextAll) { - if (ids.has(r.id)) return c.json({ error: `duplicate route id "${r.id}"` }, 400); - ids.add(r.id); - } - try { await saveRoutes(c.env.KV, nextAll); } catch (err) {