From c8d89e5075ab768c3aeaf13f5ae582a7d2e71121 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 5 May 2026 06:36:50 +0000 Subject: [PATCH 1/2] feat(api): add email-only notifier for high/medium API errors (#809) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Epic #616 Phase 3 / sub-issue #809。重要 API エラー (severity=high/medium) の メール通知サービス `services/notifier.ts` を新設し、AI 解析コールバック (`PUT /api/webhooks/github/ai-result/:id`) の成功時に 1 回だけ呼ぶよう配線する。 Slack はこのリポジトリでは使わないため非対応。`MONITORING_NOTIFY_EMAIL` 未設定 / severity が low/unknown の場合は no-op。本文には sentry_issue_id / severity / title / 管理画面 URL のみ載せ、Authorization / Cookie / 内部 URL など PII は混入させない。 Phase 3 / #809: introduce `notifier.ts` for email-only alerts on high/medium severity API errors. Slack is intentionally unsupported. The notifier no-ops when `MONITORING_NOTIFY_EMAIL` is unset or severity is low/unknown. The AI callback is the single call site so duplicate alerts can't happen, and sends are fire-and-forget so the webhook response isn't delayed by Resend. --- server/api/.env.example | 19 ++ .../routes/webhooks/githubAiCallback.test.ts | 48 +++++ .../src/routes/webhooks/githubAiCallback.ts | 16 ++ server/api/src/services/notifier.test.ts | 193 ++++++++++++++++++ server/api/src/services/notifier.ts | 191 +++++++++++++++++ 5 files changed, 467 insertions(+) create mode 100644 server/api/src/services/notifier.test.ts create mode 100644 server/api/src/services/notifier.ts diff --git a/server/api/.env.example b/server/api/.env.example index cc08b087..b6ca5eec 100644 --- a/server/api/.env.example +++ b/server/api/.env.example @@ -27,3 +27,22 @@ GITHUB_APP_ID= GITHUB_APP_PRIVATE_KEY= GITHUB_APP_INSTALLATION_ID= GITHUB_DISPATCH_REPOSITORY= + +# Monitoring alert email + admin URL for Epic #616 Phase 3 (issue #809). +# - MONITORING_NOTIFY_EMAIL: recipient address for `notifyApiErrorAlert`. When +# unset the notifier is a no-op so the AI callback path works in unconfigured +# staging environments. +# - ADMIN_BASE_URL: public origin of the admin panel (e.g. `https://admin.zedi-note.app`). +# Used to build a `/errors/` link inside alert emails. +# Internal/private URLs must NOT be set here. When unset the alert email +# omits the link line and falls back to sentry_issue_id only. +# +# Epic #616 Phase 3（issue #809）用の通知設定。 +# - MONITORING_NOTIFY_EMAIL: severity が `high` / `medium` の API エラー通知の +# 宛先メール。未設定時は notifier が no-op となり、AI コールバックは正常に +# 動作する（ステージング段階で通知のみ無効化できる）。 +# - ADMIN_BASE_URL: 管理画面の公開オリジン（例: `https://admin.zedi-note.app`）。 +# 通知メール本文に `/errors/` のリンクを差し込む際に使う。 +# 内部 URL を設定してはいけない。未設定時はリンク行を省略する。 +MONITORING_NOTIFY_EMAIL= +ADMIN_BASE_URL= diff --git a/server/api/src/__tests__/routes/webhooks/githubAiCallback.test.ts b/server/api/src/__tests__/routes/webhooks/githubAiCallback.test.ts index 3156cd10..358e392f 100644 --- a/server/api/src/__tests__/routes/webhooks/githubAiCallback.test.ts +++ b/server/api/src/__tests__/routes/webhooks/githubAiCallback.test.ts @@ -67,6 +67,7 @@ describe("PUT /api/webhooks/github/ai-result/:id", () => { beforeEach(() => { vi.spyOn(console, "error").mockImplementation(() => undefined); vi.spyOn(console, "log").mockImplementation(() => undefined); + vi.spyOn(console, "warn").mockImplementation(() => undefined); process.env.GITHUB_APP_ID = "123"; process.env.GITHUB_APP_INSTALLATION_ID = "456"; process.env.GITHUB_APP_PRIVATE_KEY = "stub"; @@ -317,4 +318,51 @@ describe("PUT /api/webhooks/github/ai-result/:id", () => { }); expect(res.status).toBe(404); }); + + it("invokes notifyApiErrorAlert exactly once with the post-update row (Phase 3 / #809)", async () => { + // 通知は AI コールバックの 1 箇所からのみ起動する（二重通知防止）。 + // notifyApiErrorAlert が成功 200 のたびに 1 回だけ呼ばれることを確認する。 + // + // The notifier is called from this single site to prevent duplicate + // alerts. Verify that a successful 200 response invokes it exactly once + // with the post-update row's fields. + vi.resetModules(); + const notifySpy = vi.fn().mockResolvedValue({ email: { sent: true, id: "e1" } }); + await vi.doMock("../../../services/notifier.js", () => ({ + notifyApiErrorAlert: notifySpy, + })); + await stubVerifyInstallationToken(true); + const { default: routes } = await import("../../../routes/webhooks/githubAiCallback.js"); + const updated = { + id: VALID_UUID, + sentryIssueId: "sentry-xyz", + severity: "high", + title: "TypeError", + }; + const { db } = createMockDb([[updated]]); + const app = new Hono(); + app.onError(errorHandler); + app.use("*", async (c, next) => { + c.set("db", db as unknown as AppEnv["Variables"]["db"]); + await next(); + }); + app.route("/api/webhooks/github/ai-result", routes); + + const res = await app.request(`/api/webhooks/github/ai-result/${VALID_UUID}`, { + method: "PUT", + headers: { "Content-Type": "application/json", Authorization: "Bearer ghs_ok" }, + body: JSON.stringify({ severity: "high" }), + }); + expect(res.status).toBe(200); + // fire-and-forget の microtask が消化されるのを待つ。 + // Drain any pending microtasks queued by the fire-and-forget call. + await new Promise((resolve) => setImmediate(resolve)); + expect(notifySpy).toHaveBeenCalledTimes(1); + expect(notifySpy).toHaveBeenCalledWith({ + apiErrorId: VALID_UUID, + sentryIssueId: "sentry-xyz", + severity: "high", + title: "TypeError", + }); + }); }); diff --git a/server/api/src/routes/webhooks/githubAiCallback.ts b/server/api/src/routes/webhooks/githubAiCallback.ts index fc30f8c2..ba65c45a 100644 --- a/server/api/src/routes/webhooks/githubAiCallback.ts +++ b/server/api/src/routes/webhooks/githubAiCallback.ts @@ -25,6 +25,7 @@ import { type UpdateAiAnalysisInput, } from "../../services/apiErrorService.js"; import { publishApiErrorUpdate } from "../../services/apiErrorBroadcaster.js"; +import { notifyApiErrorAlert } from "../../services/notifier.js"; import { verifyInstallationToken } from "../../lib/githubAppAuth.js"; import type { AppEnv } from "../../types/index.js"; @@ -161,6 +162,21 @@ app.put("/:id", async (c) => { // SSE 購読者へ AI 解析結果を配信 (Phase 2 / issue #807)。 // Notify SSE subscribers so the admin UI updates without a page reload. publishApiErrorUpdate(updated); + // 重要エラーのメール通知 (Phase 3 / issue #809)。severity が `high` / + // `medium` 以外、もしくは `MONITORING_NOTIFY_EMAIL` 未設定の場合は no-op。 + // ここを唯一の呼び出し元にして二重通知を防ぐ。fire-and-forget で + // webhook 応答を遅らせない（notifier 側でエラーは swallow 済み）。 + // + // Email alert for high-impact errors (Phase 3 / #809). The notifier is a + // no-op when severity is `low`/`unknown` or `MONITORING_NOTIFY_EMAIL` is + // unset. This is the single call site to prevent duplicate alerts; we + // fire-and-forget so the webhook response doesn't await Resend. + void notifyApiErrorAlert({ + apiErrorId: updated.id, + sentryIssueId: updated.sentryIssueId, + severity: updated.severity, + title: updated.title, + }); // 外部 (GitHub Actions) 向けの webhook なので、`error` キーを成功時に流用する // 内部 admin API の慣習ではなく、`data` キーで返して "error 有無で失敗判定" // できる素直な形にする（admin/src/api/admin.ts と異なり消費者がまだ存在しない）。 diff --git a/server/api/src/services/notifier.test.ts b/server/api/src/services/notifier.test.ts new file mode 100644 index 00000000..99818002 --- /dev/null +++ b/server/api/src/services/notifier.test.ts @@ -0,0 +1,193 @@ +/** + * `notifier.ts` の単体テスト (Epic #616 Phase 3 / sub-issue #809)。 + * + * 本プロジェクトでは外部通知をメール経由のみに限定する（Slack は使わない）。 + * テストは `emailService` を mock して、severity と環境変数の組み合わせで + * 1 通だけ送られる / 送られないことを担保する。PII (Authorization / Cookie / + * raw email) が本文に混入しないことも検証する。 + * + * Unit tests for `notifier.ts` (Epic #616 Phase 3 / sub-issue #809). External + * alerting is intentionally email-only (no Slack). Tests mock `emailService` + * and assert send count for each (severity, env) combination, plus PII-safe + * body content (no Authorization / Cookie / raw email leakage). + */ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +const mockSendEmail = vi.fn(); + +vi.mock("./emailService.js", () => ({ + sendEmail: mockSendEmail, +})); + +// モック設定後にインポート / Import after mock setup +const { notifyApiErrorAlert } = await import("./notifier.js"); + +describe("notifyApiErrorAlert", () => { + const originalEnv = process.env; + + beforeEach(() => { + process.env = { ...originalEnv }; + vi.clearAllMocks(); + mockSendEmail.mockResolvedValue({ success: true, id: "email_id_1" }); + vi.spyOn(console, "warn").mockImplementation(() => undefined); + vi.spyOn(console, "error").mockImplementation(() => undefined); + vi.spyOn(console, "log").mockImplementation(() => undefined); + }); + + afterEach(() => { + process.env = originalEnv; + vi.restoreAllMocks(); + }); + + it("MONITORING_NOTIFY_EMAIL 未設定なら送信しない (no-op)", async () => { + delete process.env.MONITORING_NOTIFY_EMAIL; + + const result = await notifyApiErrorAlert({ + apiErrorId: "00000000-0000-0000-0000-000000000001", + sentryIssueId: "sentry-abc", + severity: "high", + title: "TypeError: cannot read property", + }); + + expect(result.email.sent).toBe(false); + expect(mockSendEmail).not.toHaveBeenCalled(); + }); + + it("severity が low / unknown のときは送信しない", async () => { + process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com"; + + const lowResult = await notifyApiErrorAlert({ + apiErrorId: "00000000-0000-0000-0000-000000000001", + sentryIssueId: "sentry-low", + severity: "low", + title: "minor", + }); + const unknownResult = await notifyApiErrorAlert({ + apiErrorId: "00000000-0000-0000-0000-000000000002", + sentryIssueId: "sentry-unknown", + severity: "unknown", + title: "unclassified", + }); + + expect(lowResult.email.sent).toBe(false); + expect(unknownResult.email.sent).toBe(false); + expect(mockSendEmail).not.toHaveBeenCalled(); + }); + + it("severity=high かつ MONITORING_NOTIFY_EMAIL 設定済みで 1 通だけ送る", async () => { + process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com"; + + const result = await notifyApiErrorAlert({ + apiErrorId: "00000000-0000-0000-0000-000000000001", + sentryIssueId: "sentry-abc", + severity: "high", + title: "TypeError: cannot read property", + }); + + expect(result.email.sent).toBe(true); + expect(mockSendEmail).toHaveBeenCalledTimes(1); + const args = mockSendEmail.mock.calls[0]?.[0]; + expect(args.to).toBe("ops@example.com"); + expect(args.subject).toContain("high"); + expect(args.subject).toContain("sentry-abc"); + }); + + it("severity=medium でも送る", async () => { + process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com"; + + const result = await notifyApiErrorAlert({ + apiErrorId: "00000000-0000-0000-0000-000000000003", + sentryIssueId: "sentry-med", + severity: "medium", + title: "Latency spike", + }); + + expect(result.email.sent).toBe(true); + expect(mockSendEmail).toHaveBeenCalledTimes(1); + }); + + it("ADMIN_BASE_URL 設定時は管理画面 URL を本文に含める", async () => { + process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com"; + process.env.ADMIN_BASE_URL = "https://admin.zedi-note.app"; + + await notifyApiErrorAlert({ + apiErrorId: "00000000-0000-0000-0000-000000000001", + sentryIssueId: "sentry-abc", + severity: "high", + title: "TypeError", + }); + + const args = mockSendEmail.mock.calls[0]?.[0]; + expect(args.html).toContain( + "https://admin.zedi-note.app/errors/00000000-0000-0000-0000-000000000001", + ); + }); + + it("ADMIN_BASE_URL 未設定時は URL を本文に含めない（no-op フォールバック）", async () => { + process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com"; + delete process.env.ADMIN_BASE_URL; + + await notifyApiErrorAlert({ + apiErrorId: "00000000-0000-0000-0000-000000000001", + sentryIssueId: "sentry-abc", + severity: "high", + title: "TypeError", + }); + + const args = mockSendEmail.mock.calls[0]?.[0]; + // 管理画面 URL は載せないが、最低限 sentry_issue_id と severity は本文にある。 + // No admin URL is rendered, but sentry_issue_id and severity are still + // present so an on-call can pivot to the Sentry issue directly. + expect(args.html).not.toContain("http"); + expect(args.html).toContain("sentry-abc"); + expect(args.html).toContain("high"); + }); + + it("PII (Authorization / Cookie / raw email) を本文に含めない", async () => { + process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com"; + process.env.ADMIN_BASE_URL = "https://admin.zedi-note.app"; + + await notifyApiErrorAlert({ + apiErrorId: "00000000-0000-0000-0000-000000000001", + sentryIssueId: "sentry-abc", + severity: "high", + title: "TypeError", + }); + + const args = mockSendEmail.mock.calls[0]?.[0]; + const haystack = `${args.subject}\n${args.html}`.toLowerCase(); + expect(haystack).not.toContain("authorization"); + expect(haystack).not.toContain("cookie"); + expect(haystack).not.toContain("bearer "); + }); + + it("emailService が失敗しても throw せず success=false を返す", async () => { + process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com"; + mockSendEmail.mockResolvedValueOnce({ success: false, error: "Rate limit" }); + + const result = await notifyApiErrorAlert({ + apiErrorId: "00000000-0000-0000-0000-000000000001", + sentryIssueId: "sentry-abc", + severity: "high", + title: "TypeError", + }); + + expect(result.email.sent).toBe(false); + expect(result.email.error).toBe("Rate limit"); + }); + + it("emailService が例外を投げても throw せず success=false を返す", async () => { + process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com"; + mockSendEmail.mockRejectedValueOnce(new Error("Network down")); + + const result = await notifyApiErrorAlert({ + apiErrorId: "00000000-0000-0000-0000-000000000001", + sentryIssueId: "sentry-abc", + severity: "high", + title: "TypeError", + }); + + expect(result.email.sent).toBe(false); + expect(result.email.error).toContain("Network down"); + }); +}); diff --git a/server/api/src/services/notifier.ts b/server/api/src/services/notifier.ts new file mode 100644 index 00000000..b45cfc56 --- /dev/null +++ b/server/api/src/services/notifier.ts @@ -0,0 +1,191 @@ +/** + * 重要 API エラー通知サービス (Epic #616 Phase 3 / sub-issue #809)。 + * + * AI 解析後に severity が `high` / `medium` と判定された `api_errors` 行に対し、 + * 運用担当の通知メールアドレスへ薄いサマリ（sentry_issue_id, severity, 管理画面 + * URL）をメールで送る。Slack は本リポジトリでは使わないためサポートしない。 + * + * 設計方針: + * - 環境変数（`MONITORING_NOTIFY_EMAIL`）が未設定なら no-op。本番投入前のステー + * ジング段階でも例外を出さずに動かせる。 + * - severity が `high` / `medium` 以外（`low`, `unknown`）は no-op。Phase 3 の + * 通知は P1/P2 と整合する重大度のみ対象。 + * - 本文に Authorization / Cookie / raw email など PII を載せない。本サービスに + * 渡すのは集約済みサマリ (`title`, `sentryIssueId`, `severity`) と管理画面 URL + * 構築に必要な `apiErrorId` のみ。 + * - 内部 URL（API ホスト等）は本文に含めない。`ADMIN_BASE_URL` のみ参照する。 + * - 二重通知防止のため呼び出し側で 1 箇所に集約する想定（githubAiCallback の + * `updateAiAnalysis` 成功直後）。 + * + * Email-only alerter for high-impact API errors (Epic #616 Phase 3 / #809). + * Slack is intentionally unsupported in this repo. The notifier is no-op when + * `MONITORING_NOTIFY_EMAIL` is unset or when severity is not `high`/`medium`, + * so it can ship before staging configuration is finalized. The body contains + * only summary fields (sentry_issue_id, severity, title) and the public admin + * URL — Authorization / Cookie / raw email values are never propagated here. + * + * @see ./emailService.ts + * @see ../routes/webhooks/githubAiCallback.ts + * @see https://github.com/otomatty/zedi/issues/616 + * @see https://github.com/otomatty/zedi/issues/809 + */ +import type { ApiErrorSeverity } from "../schema/apiErrors.js"; +import { sendEmail } from "./emailService.js"; + +/** + * 通知対象とする severity。Phase 3 では `high` と `medium` のみ。 + * Severities that trigger an alert. Phase 3 covers `high` and `medium` only. + */ +const NOTIFIABLE_SEVERITIES: readonly ApiErrorSeverity[] = ["high", "medium"]; + +/** + * `notifyApiErrorAlert` の入力。呼び出し側 (githubAiCallback) は + * `updateAiAnalysis` の戻り行から必要なフィールドだけを抜き出して渡す。 + * + * Input payload for `notifyApiErrorAlert`. Callers (githubAiCallback) extract + * only the fields they need from the post-update `api_errors` row. Internal + * URLs / capability tokens / raw user data must not be passed in. + */ +export interface NotifyApiErrorAlertPayload { + /** `api_errors.id`（管理画面 URL 構築に使う） / api_errors row id */ + apiErrorId: string; + /** Sentry issue id（本文と件名に出す） / Sentry issue id (rendered in body) */ + sentryIssueId: string; + /** AI 解析後の severity / AI-derived severity */ + severity: ApiErrorSeverity; + /** 短いタイトル（PII を含まない要約） / Short, PII-free error title */ + title: string; +} + +/** + * 通知結果。メール経路の send 結果のみ返す（Slack は未対応）。 + * + * Result of a `notifyApiErrorAlert` call. Only the email channel is reported + * because Slack is intentionally unsupported in this repo. `sent: false` with + * no `error` indicates an intentional skip (no-op path). + */ +export interface NotifyApiErrorAlertResult { + email: { + /** 送信を実行したか / Whether an email was actually sent */ + sent: boolean; + /** Resend が返したメール ID / Email id returned by Resend (on success) */ + id?: string; + /** 失敗時のエラーメッセージ / Error message when send failed */ + error?: string; + }; +} + +/** + * HTML エスケープ。件名・本文に挿入する値はすべてここを通す。 + * Escape user-controlled text before injecting into the HTML body. + */ +function escapeHtml(value: string): string { + return value + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); +} + +/** + * 末尾スラッシュを 1 つだけに正規化する（`https://x/` も `https://x` も同じ + * 結果）。空文字なら null。 + * + * Normalize a base URL by stripping trailing slashes. Returns null for empty + * inputs so the caller can distinguish "configured" from "absent" cleanly. + */ +function normalizeBaseUrl(raw: string | undefined): string | null { + if (!raw) return null; + const trimmed = raw.trim().replace(/\/+$/, ""); + return trimmed.length > 0 ? trimmed : null; +} + +/** + * 件名と HTML 本文を組み立てる。本文に含めるのは + * - sentry_issue_id + * - severity + * - title + * - 管理画面 URL（ADMIN_BASE_URL 設定時のみ） + * のみ。Authorization / Cookie / raw email / 内部 URL は呼び出し側でも + * 当サービス内でも参照しない。 + * + * Build subject + HTML body for the alert email. Only sentry_issue_id, + * severity, title, and the public admin URL (when `ADMIN_BASE_URL` is set) + * are rendered. Authorization, Cookie, raw email, and internal URLs are + * never referenced — by contract this function takes only the fields it + * renders. + */ +function buildEmail(payload: NotifyApiErrorAlertPayload): { subject: string; html: string } { + const adminBase = normalizeBaseUrl(process.env.ADMIN_BASE_URL); + const adminUrl = adminBase ? `${adminBase}/errors/${payload.apiErrorId}` : null; + + const subject = `[zedi:${payload.severity}] API error ${payload.sentryIssueId}`; + + const lines: string[] = [ + `

severity: ${escapeHtml(payload.severity)}

`, + `

sentry_issue_id: ${escapeHtml(payload.sentryIssueId)}

`, + `

title: ${escapeHtml(payload.title)}

`, + ]; + if (adminUrl) { + const safeUrl = escapeHtml(adminUrl); + lines.push(`

${safeUrl}

`); + } + return { subject, html: lines.join("\n") }; +} + +/** + * 重要 API エラーをメール通知する。Slack は使わない。 + * + * - severity が `high` / `medium` 以外なら no-op。 + * - `MONITORING_NOTIFY_EMAIL` が未設定なら no-op。 + * - emailService 側のエラーは握りつぶし、`result.email.error` で返す + * （通知失敗で呼び出し元（webhook 応答）を 500 にしないため）。 + * + * Send an email alert for a high-impact API error. Slack is not supported. + * The notifier short-circuits to a no-op when severity is `low`/`unknown` or + * when `MONITORING_NOTIFY_EMAIL` is unset. Email failures are swallowed and + * surfaced via `result.email.error` so a notification outage does not turn the + * upstream webhook response into a 500. + * + * @param payload - 通知に必要なサマリ / Alert summary fields + * @returns 送信結果 / Per-channel delivery result + */ +export async function notifyApiErrorAlert( + payload: NotifyApiErrorAlertPayload, +): Promise { + if (!NOTIFIABLE_SEVERITIES.includes(payload.severity)) { + return { email: { sent: false } }; + } + + const to = process.env.MONITORING_NOTIFY_EMAIL?.trim(); + if (!to) { + console.warn( + "[notifier] MONITORING_NOTIFY_EMAIL is not set; skipping alert for sentry_issue_id=", + payload.sentryIssueId, + ); + return { email: { sent: false } }; + } + + const { subject, html } = buildEmail(payload); + + try { + const sendResult = await sendEmail({ to, subject, html }); + if (!sendResult.success) { + console.error( + `[notifier] email send failed for sentry_issue_id=${payload.sentryIssueId}: ${sendResult.error ?? "unknown error"}`, + ); + return { email: { sent: false, error: sendResult.error } }; + } + console.log( + `[notifier] alert email sent for sentry_issue_id=${payload.sentryIssueId} severity=${payload.severity}`, + ); + return { email: { sent: true, id: sendResult.id } }; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + console.error( + `[notifier] unexpected error sending alert for sentry_issue_id=${payload.sentryIssueId}: ${message}`, + ); + return { email: { sent: false, error: message } }; + } +} From 96da9b663af485dcd3d2b38c5be080d9d476e866 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 5 May 2026 06:51:04 +0000 Subject: [PATCH 2/2] fix(api): address review feedback on notifier (#809) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - gate `notifyApiErrorAlert` on severity transitions so retries / partial callbacks no longer re-send (Codex P1). Route now reads the row before `updateAiAnalysis` and only fires when severity transitions from a non-notifiable value into `high`/`medium`. - validate `ADMIN_BASE_URL` scheme (`http:` / `https:` only) and drop malformed values to prevent dangerous URLs in alert HTML (CodeRabbit). - reorder `.env.example` so `ADMIN_BASE_URL` precedes `MONITORING_NOTIFY_EMAIL` to satisfy dotenv-linter (CodeRabbit). - adopt `getOptionalEnv` for env access for parity with `emailService.ts` and strip CR/LF from email subject as defense-in-depth against header injection (gemini-code-assist). Tests cover: first-sight escalation fires once; retry (high→high), partial callback (no severity), and downgrade do NOT fire; non-HTTP schemes never produce an `href`; CR/LF never survives in the subject. --- server/api/.env.example | 2 +- .../routes/webhooks/githubAiCallback.test.ts | 160 ++++++++++++++++-- .../src/routes/webhooks/githubAiCallback.ts | 76 +++++++-- server/api/src/services/notifier.test.ts | 58 +++++++ server/api/src/services/notifier.ts | 46 +++-- 5 files changed, 303 insertions(+), 39 deletions(-) diff --git a/server/api/.env.example b/server/api/.env.example index b6ca5eec..061ff561 100644 --- a/server/api/.env.example +++ b/server/api/.env.example @@ -44,5 +44,5 @@ GITHUB_DISPATCH_REPOSITORY= # - ADMIN_BASE_URL: 管理画面の公開オリジン（例: `https://admin.zedi-note.app`）。 # 通知メール本文に `/errors/` のリンクを差し込む際に使う。 # 内部 URL を設定してはいけない。未設定時はリンク行を省略する。 -MONITORING_NOTIFY_EMAIL= ADMIN_BASE_URL= +MONITORING_NOTIFY_EMAIL= diff --git a/server/api/src/__tests__/routes/webhooks/githubAiCallback.test.ts b/server/api/src/__tests__/routes/webhooks/githubAiCallback.test.ts index 358e392f..bc36450d 100644 --- a/server/api/src/__tests__/routes/webhooks/githubAiCallback.test.ts +++ b/server/api/src/__tests__/routes/webhooks/githubAiCallback.test.ts @@ -217,17 +217,26 @@ describe("PUT /api/webhooks/github/ai-result/:id", () => { vi.resetModules(); await stubVerifyInstallationToken(true); const { default: routes } = await import("../../../routes/webhooks/githubAiCallback.js"); + const pre = { + id: VALID_UUID, + sentryIssueId: "abc", + severity: "unknown", + title: "TypeError", + }; const updated = { id: VALID_UUID, sentryIssueId: "abc", severity: "high", + title: "TypeError", aiSummary: "ヌルポインタ参照", aiSuspectedFiles: [{ path: "src/a.ts", line: 12 }], aiRootCause: "X が undefined", aiSuggestedFix: "guard 追加", }; - // updateAiAnalysis issues a single update chain that resolves to [row]. - const { db, chains } = createMockDb([[updated]]); + // The route now does a pre-read (`getApiErrorById`) before the UPDATE so + // it can compare pre/post severity for the Phase 3 notifier. Mock both + // chains: select then update. + const { db, chains } = createMockDb([[pre], [updated]]); const app = new Hono(); app.onError(errorHandler); app.use("*", async (c, next) => { @@ -259,7 +268,10 @@ describe("PUT /api/webhooks/github/ai-result/:id", () => { vi.resetModules(); await stubVerifyInstallationToken(true); const { default: routes } = await import("../../../routes/webhooks/githubAiCallback.js"); - const { db } = createMockDb([]); + // Pre-read returns the existing row; updateAiAnalysis then throws + // ApiErrorAiAnalysisValidationError on the bad severity → 400. + const pre = { id: VALID_UUID, sentryIssueId: "abc", severity: "unknown", title: "x" }; + const { db } = createMockDb([[pre]]); const app = new Hono(); app.onError(errorHandler); app.use("*", async (c, next) => { @@ -280,7 +292,8 @@ describe("PUT /api/webhooks/github/ai-result/:id", () => { vi.resetModules(); await stubVerifyInstallationToken(true); const { default: routes } = await import("../../../routes/webhooks/githubAiCallback.js"); - const { db } = createMockDb([]); + const pre = { id: VALID_UUID, sentryIssueId: "abc", severity: "unknown", title: "x" }; + const { db } = createMockDb([[pre]]); const app = new Hono(); app.onError(errorHandler); app.use("*", async (c, next) => { @@ -301,7 +314,8 @@ describe("PUT /api/webhooks/github/ai-result/:id", () => { vi.resetModules(); await stubVerifyInstallationToken(true); const { default: routes } = await import("../../../routes/webhooks/githubAiCallback.js"); - // updateAiAnalysis returns null when the UPDATE returns no rows. + // The pre-read short-circuits to 404 when the row is missing, so the + // route never issues the UPDATE. const { db } = createMockDb([[]]); const app = new Hono(); app.onError(errorHandler); @@ -319,13 +333,12 @@ describe("PUT /api/webhooks/github/ai-result/:id", () => { expect(res.status).toBe(404); }); - it("invokes notifyApiErrorAlert exactly once with the post-update row (Phase 3 / #809)", async () => { - // 通知は AI コールバックの 1 箇所からのみ起動する（二重通知防止）。 - // notifyApiErrorAlert が成功 200 のたびに 1 回だけ呼ばれることを確認する。 + it("notifies once when severity transitions from unknown into high (Phase 3 / #809)", async () => { + // severity が `unknown` から `high` へ初めて昇格したケース。1 回だけ + // notifier に渡すことを担保する。 // - // The notifier is called from this single site to prevent duplicate - // alerts. Verify that a successful 200 response invokes it exactly once - // with the post-update row's fields. + // First-sight escalation from `unknown` → `high` must invoke the + // notifier exactly once with the post-update row's fields. vi.resetModules(); const notifySpy = vi.fn().mockResolvedValue({ email: { sent: true, id: "e1" } }); await vi.doMock("../../../services/notifier.js", () => ({ @@ -333,13 +346,14 @@ describe("PUT /api/webhooks/github/ai-result/:id", () => { })); await stubVerifyInstallationToken(true); const { default: routes } = await import("../../../routes/webhooks/githubAiCallback.js"); - const updated = { + const pre = { id: VALID_UUID, sentryIssueId: "sentry-xyz", - severity: "high", + severity: "unknown", title: "TypeError", }; - const { db } = createMockDb([[updated]]); + const updated = { ...pre, severity: "high" }; + const { db } = createMockDb([[pre], [updated]]); const app = new Hono(); app.onError(errorHandler); app.use("*", async (c, next) => { @@ -365,4 +379,122 @@ describe("PUT /api/webhooks/github/ai-result/:id", () => { title: "TypeError", }); }); + + it("does NOT notify on idempotent retry (high → high)", async () => { + // GitHub Actions が同じ severity でリトライした場合、行はすでに + // 通知済みなので再送しない。 + // + // GitHub Actions retries the AI workflow occasionally. If the row was + // already escalated and the callback re-asserts the same severity, we + // must NOT resend the alert. + vi.resetModules(); + const notifySpy = vi.fn().mockResolvedValue({ email: { sent: true } }); + await vi.doMock("../../../services/notifier.js", () => ({ + notifyApiErrorAlert: notifySpy, + })); + await stubVerifyInstallationToken(true); + const { default: routes } = await import("../../../routes/webhooks/githubAiCallback.js"); + const pre = { + id: VALID_UUID, + sentryIssueId: "sentry-xyz", + severity: "high", + title: "TypeError", + }; + const updated = { ...pre }; + const { db } = createMockDb([[pre], [updated]]); + const app = new Hono(); + app.onError(errorHandler); + app.use("*", async (c, next) => { + c.set("db", db as unknown as AppEnv["Variables"]["db"]); + await next(); + }); + app.route("/api/webhooks/github/ai-result", routes); + + const res = await app.request(`/api/webhooks/github/ai-result/${VALID_UUID}`, { + method: "PUT", + headers: { "Content-Type": "application/json", Authorization: "Bearer ghs_ok" }, + body: JSON.stringify({ severity: "high" }), + }); + expect(res.status).toBe(200); + await new Promise((resolve) => setImmediate(resolve)); + expect(notifySpy).not.toHaveBeenCalled(); + }); + + it("does NOT notify on partial callback that omits severity (already-escalated row)", async () => { + // severity を含まない部分更新（例: ai_summary だけ refresh）。pre が + // すでに `high` の行に対して再送が発生してはいけない。 + // + // Partial callback that only refreshes a non-severity AI field on a row + // already at `high`. Pre and post severity match, so the notifier must + // not fire — even though the post-update severity is notifiable. + vi.resetModules(); + const notifySpy = vi.fn().mockResolvedValue({ email: { sent: true } }); + await vi.doMock("../../../services/notifier.js", () => ({ + notifyApiErrorAlert: notifySpy, + })); + await stubVerifyInstallationToken(true); + const { default: routes } = await import("../../../routes/webhooks/githubAiCallback.js"); + const pre = { + id: VALID_UUID, + sentryIssueId: "sentry-xyz", + severity: "high", + title: "TypeError", + }; + const updated = { ...pre, aiSummary: "更新後の要約" }; + const { db } = createMockDb([[pre], [updated]]); + const app = new Hono(); + app.onError(errorHandler); + app.use("*", async (c, next) => { + c.set("db", db as unknown as AppEnv["Variables"]["db"]); + await next(); + }); + app.route("/api/webhooks/github/ai-result", routes); + + const res = await app.request(`/api/webhooks/github/ai-result/${VALID_UUID}`, { + method: "PUT", + headers: { "Content-Type": "application/json", Authorization: "Bearer ghs_ok" }, + body: JSON.stringify({ ai_summary: "更新後の要約" }), + }); + expect(res.status).toBe(200); + await new Promise((resolve) => setImmediate(resolve)); + expect(notifySpy).not.toHaveBeenCalled(); + }); + + it("does NOT notify when severity downgrades into low/unknown", async () => { + // 通知済み行 (`high`) を `low` に下げるケースは通知しない。 + // + // De-escalations from a notifiable severity back to `low`/`unknown` + // must not produce a fresh alert. + vi.resetModules(); + const notifySpy = vi.fn().mockResolvedValue({ email: { sent: true } }); + await vi.doMock("../../../services/notifier.js", () => ({ + notifyApiErrorAlert: notifySpy, + })); + await stubVerifyInstallationToken(true); + const { default: routes } = await import("../../../routes/webhooks/githubAiCallback.js"); + const pre = { + id: VALID_UUID, + sentryIssueId: "sentry-xyz", + severity: "high", + title: "TypeError", + }; + const updated = { ...pre, severity: "low" }; + const { db } = createMockDb([[pre], [updated]]); + const app = new Hono(); + app.onError(errorHandler); + app.use("*", async (c, next) => { + c.set("db", db as unknown as AppEnv["Variables"]["db"]); + await next(); + }); + app.route("/api/webhooks/github/ai-result", routes); + + const res = await app.request(`/api/webhooks/github/ai-result/${VALID_UUID}`, { + method: "PUT", + headers: { "Content-Type": "application/json", Authorization: "Bearer ghs_ok" }, + body: JSON.stringify({ severity: "low" }), + }); + expect(res.status).toBe(200); + await new Promise((resolve) => setImmediate(resolve)); + expect(notifySpy).not.toHaveBeenCalled(); + }); }); diff --git a/server/api/src/routes/webhooks/githubAiCallback.ts b/server/api/src/routes/webhooks/githubAiCallback.ts index ba65c45a..8a43a61d 100644 --- a/server/api/src/routes/webhooks/githubAiCallback.ts +++ b/server/api/src/routes/webhooks/githubAiCallback.ts @@ -21,9 +21,11 @@ import { Hono } from "hono"; import { ApiErrorAiAnalysisValidationError, + getApiErrorById, updateAiAnalysis, type UpdateAiAnalysisInput, } from "../../services/apiErrorService.js"; +import type { ApiErrorSeverity } from "../../schema/apiErrors.js"; import { publishApiErrorUpdate } from "../../services/apiErrorBroadcaster.js"; import { notifyApiErrorAlert } from "../../services/notifier.js"; import { verifyInstallationToken } from "../../lib/githubAppAuth.js"; @@ -40,6 +42,29 @@ const app = new Hono(); */ const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; +/** + * Phase 3 / #809 で通知対象となる severity の集合。 + * Severity values that warrant an email alert (Phase 3 / #809). + */ +const NOTIFIABLE_SEVERITIES: ReadonlySet = new Set(["high", "medium"]); + +/** + * 1 行に対する通知を「冪等な再送 / 部分更新では再発火させない」ためのガード。 + * `prev` が notifiable で `next` も notifiable のとき（例: high→high の冪等 + * 再送、medium→high のエスカレート）は false を返す。`prev` が notifiable + * でなく `next` が notifiable のときだけ true。これにより 1 行あたり最大 + * 1 通の運用通知に揃える。 + * + * Returns true only when severity transitions from a non-notifiable value + * (`low` / `unknown`) into `high` or `medium`. Idempotent retries + * (high → high) and lateral moves between notifiable levels (medium → high) + * deliberately return false so each row produces at most one operational + * email over its lifetime. + */ +export function severityBecameNotifiable(prev: ApiErrorSeverity, next: ApiErrorSeverity): boolean { + return !NOTIFIABLE_SEVERITIES.has(prev) && NOTIFIABLE_SEVERITIES.has(next); +} + /** * `Authorization: Bearer ...` ヘッダから token を抜き出す。 * Extract the bearer token from an `Authorization` header value, or null when @@ -151,6 +176,22 @@ app.put("/:id", async (c) => { } const db = c.get("db"); + // Phase 3 / #809 の通知は「severity が初めて high/medium に到達したとき」 + // のみ発火させる。`updateAiAnalysis` は冪等なので、リトライや severity を + // 含まない部分更新のたびに通知すると重複アラートが発生する。よって + // UPDATE 前に行を読み、pre.severity と updated.severity を比較する。 + // 行が無ければここで 404 を返し、後続の UPDATE を発行しない。 + // + // Phase 3 / #809: only alert when severity *transitions* into high/medium + // for the first time. `updateAiAnalysis` is idempotent — retries or partial + // callbacks (e.g. an empty body or a callback that only refreshes + // `ai_summary`) would otherwise resend the alert. We read the row first so + // we can compare pre vs post severity, and so a missing row short-circuits + // before the UPDATE. + const pre = await getApiErrorById(db, id); + if (!pre) { + return c.json({ error: "Not found" }, 404); + } try { const updated = await updateAiAnalysis(db, { id, ...normalized }); if (!updated) { @@ -162,21 +203,28 @@ app.put("/:id", async (c) => { // SSE 購読者へ AI 解析結果を配信 (Phase 2 / issue #807)。 // Notify SSE subscribers so the admin UI updates without a page reload. publishApiErrorUpdate(updated); - // 重要エラーのメール通知 (Phase 3 / issue #809)。severity が `high` / - // `medium` 以外、もしくは `MONITORING_NOTIFY_EMAIL` 未設定の場合は no-op。 - // ここを唯一の呼び出し元にして二重通知を防ぐ。fire-and-forget で - // webhook 応答を遅らせない（notifier 側でエラーは swallow 済み）。 + // 重要エラーのメール通知 (Phase 3 / issue #809)。`pre.severity` が + // notifiable でなく、かつ `updated.severity` が notifiable のときだけ + // 1 回だけ発火する。冪等な再送やエスカレート済み行への部分更新では + // 発火しない。`MONITORING_NOTIFY_EMAIL` 未設定時は notifier 側で no-op。 // - // Email alert for high-impact errors (Phase 3 / #809). The notifier is a - // no-op when severity is `low`/`unknown` or `MONITORING_NOTIFY_EMAIL` is - // unset. This is the single call site to prevent duplicate alerts; we - // fire-and-forget so the webhook response doesn't await Resend. - void notifyApiErrorAlert({ - apiErrorId: updated.id, - sentryIssueId: updated.sentryIssueId, - severity: updated.severity, - title: updated.title, - }); + // Email alert for high-impact errors (Phase 3 / #809). Fires exactly when + // severity transitions from a non-notifiable value (`low` / `unknown` / + // null) into `high` or `medium`. Idempotent retries and partial updates + // on already-escalated rows are deliberate no-ops here. The notifier + // itself further no-ops when `MONITORING_NOTIFY_EMAIL` is unset. + if (severityBecameNotifiable(pre.severity, updated.severity)) { + // fire-and-forget: notifier 側でエラーは swallow 済み。webhook 応答を + // Resend 呼び出しで遅延させない。 + // fire-and-forget so the webhook response doesn't await Resend; the + // notifier itself swallows transport errors. + void notifyApiErrorAlert({ + apiErrorId: updated.id, + sentryIssueId: updated.sentryIssueId, + severity: updated.severity, + title: updated.title, + }); + } // 外部 (GitHub Actions) 向けの webhook なので、`error` キーを成功時に流用する // 内部 admin API の慣習ではなく、`data` キーで返して "error 有無で失敗判定" // できる素直な形にする（admin/src/api/admin.ts と異なり消費者がまだ存在しない）。 diff --git a/server/api/src/services/notifier.test.ts b/server/api/src/services/notifier.test.ts index 99818002..d53b01a6 100644 --- a/server/api/src/services/notifier.test.ts +++ b/server/api/src/services/notifier.test.ts @@ -143,6 +143,64 @@ describe("notifyApiErrorAlert", () => { expect(args.html).toContain("high"); }); + it("ADMIN_BASE_URL が http:/https: 以外なら URL を載せない（防御的）", async () => { + // operator が javascript:alert(1) や data: URL を誤設定しても、本文の + // クリック可能リンクには絶対に流れ込まないことを確認する。 + // + // Defense-in-depth against an operator typo: non-HTTP(S) schemes must + // never make it into the alert HTML as a clickable link. + process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com"; + + for (const bad of [ + "javascript:alert(1)", + "data:text/html,", + "file:///etc/passwd", + "not-a-url", + " ", + ]) { + vi.clearAllMocks(); + mockSendEmail.mockResolvedValue({ success: true, id: "x" }); + process.env.ADMIN_BASE_URL = bad; + await notifyApiErrorAlert({ + apiErrorId: "00000000-0000-0000-0000-000000000001", + sentryIssueId: "sentry-abc", + severity: "high", + title: "TypeError", + }); + const args = mockSendEmail.mock.calls[0]?.[0]; + expect(args.html).not.toContain("javascript:"); + expect(args.html).not.toContain("data:"); + expect(args.html).not.toContain("file:"); + expect(args.html).not.toContain("href="); + } + }); + + it("件名から CR/LF を取り除く（header-injection 防御）", async () => { + // sentryIssueId は実運用では英数記号のみだが、防御的に CR/LF を除去 + // した結果が件名に反映されることを確認する。 + // + // Defense-in-depth: even if a malformed sentry_issue_id slips through, + // CR/LF must not survive into the subject so SMTP transports can never + // be tricked into header injection. + process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com"; + + await notifyApiErrorAlert({ + apiErrorId: "00000000-0000-0000-0000-000000000001", + sentryIssueId: "sentry\r\nBcc:attacker@example.com", + severity: "high", + title: "TypeError", + }); + + const args = mockSendEmail.mock.calls[0]?.[0]; + // 件名に CR/LF が残っていなければヘッダ折り返しによるヘッダ注入は不可能。 + // 文字列としての "Bcc:" は残るが、改行が挟まらないので別ヘッダにはならない。 + // + // No CR/LF in the subject means an SMTP transport can't fold the line + // into a new header. The literal "Bcc:" substring is harmless without a + // preceding line break. + expect(args.subject).not.toMatch(/[\r\n]/); + }); + it("PII (Authorization / Cookie / raw email) を本文に含めない", async () => { process.env.MONITORING_NOTIFY_EMAIL = "ops@example.com"; process.env.ADMIN_BASE_URL = "https://admin.zedi-note.app"; diff --git a/server/api/src/services/notifier.ts b/server/api/src/services/notifier.ts index b45cfc56..bc043824 100644 --- a/server/api/src/services/notifier.ts +++ b/server/api/src/services/notifier.ts @@ -29,6 +29,7 @@ * @see https://github.com/otomatty/zedi/issues/616 * @see https://github.com/otomatty/zedi/issues/809 */ +import { getOptionalEnv } from "../lib/env.js"; import type { ApiErrorSeverity } from "../schema/apiErrors.js"; import { sendEmail } from "./emailService.js"; @@ -89,16 +90,30 @@ function escapeHtml(value: string): string { } /** - * 末尾スラッシュを 1 つだけに正規化する（`https://x/` も `https://x` も同じ - * 結果）。空文字なら null。 + * 末尾スラッシュを取り除き、`http(s)` スキームのみを許可した正規化済み + * base URL を返す。`javascript:` 等の危険なスキームや URL として解析不能な + * 値は null にフォールバックして、本文に埋め込まれないようにする。 * - * Normalize a base URL by stripping trailing slashes. Returns null for empty - * inputs so the caller can distinguish "configured" from "absent" cleanly. + * Normalize a base URL: strip trailing slashes and only accept the `http:` / + * `https:` schemes. Anything else (`javascript:`, `data:`, malformed input, + * etc.) returns null so it never lands in alert HTML — operator-controlled + * env var, but defense in depth keeps a config typo from producing a clickable + * non-HTTP link in an email body. */ -function normalizeBaseUrl(raw: string | undefined): string | null { - if (!raw) return null; +function normalizeBaseUrl(raw: string): string | null { const trimmed = raw.trim().replace(/\/+$/, ""); - return trimmed.length > 0 ? trimmed : null; + if (trimmed.length === 0) return null; + let parsed: URL; + try { + parsed = new URL(trimmed); + } catch { + return null; + } + if (parsed.protocol !== "https:" && parsed.protocol !== "http:") return null; + // 正規化済み URL を再合成。pathname が "/" のときは省略する。 + // Reassemble from parsed components; drop a bare `/` pathname. + const path = parsed.pathname === "/" ? "" : parsed.pathname.replace(/\/+$/, ""); + return `${parsed.origin}${path}`; } /** @@ -117,10 +132,21 @@ function normalizeBaseUrl(raw: string | undefined): string | null { * renders. */ function buildEmail(payload: NotifyApiErrorAlertPayload): { subject: string; html: string } { - const adminBase = normalizeBaseUrl(process.env.ADMIN_BASE_URL); + const adminBase = normalizeBaseUrl(getOptionalEnv("ADMIN_BASE_URL")); const adminUrl = adminBase ? `${adminBase}/errors/${payload.apiErrorId}` : null; - const subject = `[zedi:${payload.severity}] API error ${payload.sentryIssueId}`; + // 件名から CR/LF を取り除いて、メール送信ライブラリが SMTP ヘッダ化する + // 際の header injection (RFC 5322 folding 悪用) を防ぐ。Resend は JSON で + // 受けるため現状は発火しないが、防御を 1 段噛ませてもコストはほぼゼロ。 + // + // Strip CR/LF from the subject as defense-in-depth against SMTP header + // injection. Resend takes the subject as a JSON field today so this isn't + // exploitable now, but the cost is one regex and the behavior holds even + // if we ever swap mail transports. + const subject = `[zedi:${payload.severity}] API error ${payload.sentryIssueId}`.replace( + /[\r\n]+/g, + " ", + ); const lines: string[] = [ `

severity: ${escapeHtml(payload.severity)}

`, @@ -158,7 +184,7 @@ export async function notifyApiErrorAlert( return { email: { sent: false } }; } - const to = process.env.MONITORING_NOTIFY_EMAIL?.trim(); + const to = getOptionalEnv("MONITORING_NOTIFY_EMAIL").trim(); if (!to) { console.warn( "[notifier] MONITORING_NOTIFY_EMAIL is not set; skipping alert for sentry_issue_id=",