From 425503e095c5e70fe8ad18766e9dedeae01bba0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 28 May 2026 22:11:53 +0200 Subject: [PATCH] feat(security): safeFetch wrapper with manual redirect and timeout defaults Closes issue 218. Flips the convention from "remember to opt in to redirect: manual" to "remember to opt out". Every outbound fetch that doesn't pin a hard-coded host now routes through safeFetch and inherits AbortSignal.timeout(15_000) so a tar-pit upstream can no longer pin a worker indefinitely. The local AI client also gates on requirePublicHost: true since its host is user-supplied; operators running self-hosted Ollama / LM Studio on an RFC1918 address keep their escape hatch via ALLOW_LOCAL_AI_PRIVATE_HOSTS=true (also enforced at write-time in /api/user/ai-provider). 14 outbound call sites migrated. The four that already pinned manual redirect (the historic templates) keep their semantics; the wrapper preserves an explicit init.redirect. Adds undici as an explicit dependency so the connect-time pin landing alongside this can rely on the documented API rather than a transitive hop. Co-Authored-By: Claude Opus 4.7 (1M context) --- package.json | 1 + pnpm-lock.yaml | 3 + .../api/admin/monitoring/umami-test/route.ts | 3 +- src/app/api/bugreport/route.ts | 3 +- .../api/integrations/moodlog/test/route.ts | 30 ++-- .../api/integrations/withings/test/route.ts | 32 ++-- src/app/api/monitoring/umami-script/route.ts | 3 +- src/app/api/send/route.ts | 3 +- src/app/api/version/check-updates/route.ts | 3 +- src/lib/__tests__/safe-fetch.test.ts | 133 +++++++++++++++++ src/lib/ai/__tests__/local-client.test.ts | 10 +- .../ai/__tests__/provider-contract.test.ts | 10 +- src/lib/ai/anthropic-client.ts | 34 +++-- src/lib/ai/local-client.ts | 48 +++--- src/lib/ai/openai-client.ts | 43 +++--- src/lib/feedback/publish-github.ts | 3 +- src/lib/geo.ts | 30 ++-- src/lib/logging/transports.ts | 16 +- src/lib/monitoring/glitchtip.ts | 8 +- src/lib/moodlog/push.ts | 27 ++-- src/lib/moodlog/sync.ts | 25 ++-- src/lib/notifications/senders/ntfy.ts | 16 +- src/lib/safe-fetch.ts | 139 ++++++++++++++++++ src/lib/telegram.ts | 5 +- 24 files changed, 480 insertions(+), 148 deletions(-) create mode 100644 src/lib/__tests__/safe-fetch.test.ts create mode 100644 src/lib/safe-fetch.ts diff --git a/package.json b/package.json index 33bb183cd..b0f376490 100644 --- a/package.json +++ b/package.json @@ -83,6 +83,7 @@ "sax": "^1.6.0", "sonner": "^2.0.7", "tailwind-merge": "^3.6.0", + "undici": "^7.25.0", "web-push": "^3.6.7", "yaml": "^2.8.4", "zod": "^4.4.3", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 641898bfc..1f932612e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -110,6 +110,9 @@ importers: tailwind-merge: specifier: ^3.6.0 version: 3.6.0 + undici: + specifier: ^7.25.0 + version: 7.25.0 web-push: specifier: ^3.6.7 version: 3.6.7 diff --git a/src/app/api/admin/monitoring/umami-test/route.ts b/src/app/api/admin/monitoring/umami-test/route.ts index 330fd6041..660568ce5 100644 --- a/src/app/api/admin/monitoring/umami-test/route.ts +++ b/src/app/api/admin/monitoring/umami-test/route.ts @@ -3,6 +3,7 @@ import { apiHandler, requireAdmin } from "@/lib/api-handler"; import { annotate, getEvent } from "@/lib/logging/context"; import { apiError, apiSuccess } from "@/lib/api-response"; import { getPublicMonitoringSettings } from "@/lib/monitoring-settings"; +import { safeFetch } from "@/lib/safe-fetch"; export const dynamic = "force-dynamic"; @@ -88,7 +89,7 @@ export const POST = apiHandler(async (request: NextRequest) => { let lastDetails = ""; for (const targetUrl of targetUrls) { - const upstream = await fetch(targetUrl, { + const upstream = await safeFetch(targetUrl, { method: "POST", headers: { "content-type": "application/json", diff --git a/src/app/api/bugreport/route.ts b/src/app/api/bugreport/route.ts index 7ed289182..3f8ad4cc5 100644 --- a/src/app/api/bugreport/route.ts +++ b/src/app/api/bugreport/route.ts @@ -11,6 +11,7 @@ import { checkRateLimit } from "@/lib/rate-limit"; import { auditLog } from "@/lib/auth/audit"; import { prisma } from "@/lib/db"; import { decrypt } from "@/lib/crypto"; +import { safeFetch } from "@/lib/safe-fetch"; import { NextRequest } from "next/server"; import { z } from "zod/v4"; @@ -120,7 +121,7 @@ export const POST = apiHandler(async (request: NextRequest) => { } // Create the issue - const issueRes = await fetch( + const issueRes = await safeFetch( `https://api.github.com/repos/${ghRepo}/issues`, { method: "POST", diff --git a/src/app/api/integrations/moodlog/test/route.ts b/src/app/api/integrations/moodlog/test/route.ts index 9e9d68602..35eb80ed5 100644 --- a/src/app/api/integrations/moodlog/test/route.ts +++ b/src/app/api/integrations/moodlog/test/route.ts @@ -6,6 +6,7 @@ import { annotate } from "@/lib/logging/context"; import { prisma } from "@/lib/db"; import { decrypt } from "@/lib/crypto"; import { isPublicUrl } from "@/lib/validations/notifications"; +import { safeFetch, SafeFetchError } from "@/lib/safe-fetch"; export const dynamic = "force-dynamic"; @@ -110,20 +111,21 @@ export const POST = apiHandler(async (request: NextRequest) => { probeUrl.searchParams.set("from", yesterday); probeUrl.searchParams.set("to", today); - const controller = new AbortController(); - const timer = setTimeout(() => controller.abort(), TIMEOUT_MS); const start = performance.now(); try { - const res = await fetch(probeUrl.toString(), { - method: "GET", - headers: { Authorization: `Bearer ${apiKey}` }, - signal: controller.signal, - cache: "no-store", - // `manual` neutralises the redirect-follow SSRF where a public host - // serves a 302 to 169.254.169.254 (cloud metadata) or RFC1918 ranges. - redirect: "manual", - }); + // safeFetch keeps the manual-redirect guard (the apiKey would + // otherwise leak on a 302 to 169.254.169.254 / RFC1918) and adds + // the AbortSignal.timeout convention. + const res = await safeFetch( + probeUrl.toString(), + { + method: "GET", + headers: { Authorization: `Bearer ${apiKey}` }, + cache: "no-store", + }, + { timeoutMs: TIMEOUT_MS }, + ); const latencyMs = Math.round(performance.now() - start); // 3xx in `manual` redirect mode → we never followed; treat as success @@ -155,7 +157,9 @@ export const POST = apiHandler(async (request: NextRequest) => { }); } catch (e) { const err = e as Error; - const isAbort = err.name === "AbortError"; + const isAbort = + (e instanceof SafeFetchError && e.kind === "timeout") || + err.name === "AbortError"; const code = isAbort ? "timeout" : "connection_failed"; annotate({ meta: { @@ -168,7 +172,5 @@ export const POST = apiHandler(async (request: NextRequest) => { 502, { errorCode: code }, ); - } finally { - clearTimeout(timer); } }); diff --git a/src/app/api/integrations/withings/test/route.ts b/src/app/api/integrations/withings/test/route.ts index 4b226849c..0caab4298 100644 --- a/src/app/api/integrations/withings/test/route.ts +++ b/src/app/api/integrations/withings/test/route.ts @@ -5,6 +5,7 @@ import { checkRateLimit } from "@/lib/rate-limit"; import { annotate } from "@/lib/logging/context"; import { prisma } from "@/lib/db"; import { getValidToken } from "@/lib/withings/sync"; +import { safeFetch, SafeFetchError } from "@/lib/safe-fetch"; export const dynamic = "force-dynamic"; @@ -95,21 +96,21 @@ export const POST = apiHandler(async (request: NextRequest) => { lastupdate: Math.floor(Date.now() / 1000 - 60).toString(), }); - const controller = new AbortController(); - const timer = setTimeout(() => controller.abort(), TIMEOUT_MS); const start = performance.now(); try { - const res = await fetch(WITHINGS_MEASURE_URL, { - method: "POST", - headers: { - "Content-Type": "application/x-www-form-urlencoded", - Authorization: `Bearer ${tokenInfo.accessToken}`, + const res = await safeFetch( + WITHINGS_MEASURE_URL, + { + method: "POST", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + Authorization: `Bearer ${tokenInfo.accessToken}`, + }, + body: params.toString(), }, - body: params.toString(), - signal: controller.signal, - redirect: "manual", - }); + { timeoutMs: TIMEOUT_MS }, + ); const latencyMs = Math.round(performance.now() - start); if (!res.ok) { @@ -156,7 +157,12 @@ export const POST = apiHandler(async (request: NextRequest) => { }); } catch (e) { const err = e as Error; - const isAbort = err.name === "AbortError"; + // safeFetch wraps timeouts into SafeFetchError{kind:"timeout"}; + // legacy AbortError name is preserved for compatibility with any + // future direct-fetch path that still reaches this catch. + const isAbort = + (e instanceof SafeFetchError && e.kind === "timeout") || + err.name === "AbortError"; const code = isAbort ? "timeout" : "connection_failed"; annotate({ meta: { @@ -169,8 +175,6 @@ export const POST = apiHandler(async (request: NextRequest) => { 502, { errorCode: code }, ); - } finally { - clearTimeout(timer); } }); diff --git a/src/app/api/monitoring/umami-script/route.ts b/src/app/api/monitoring/umami-script/route.ts index fee29827b..4460c7f9c 100644 --- a/src/app/api/monitoring/umami-script/route.ts +++ b/src/app/api/monitoring/umami-script/route.ts @@ -2,6 +2,7 @@ import { NextResponse } from "next/server"; import { apiHandler } from "@/lib/api-handler"; import { annotate } from "@/lib/logging/context"; import { getPublicMonitoringSettings } from "@/lib/monitoring-settings"; +import { safeFetch } from "@/lib/safe-fetch"; export const dynamic = "force-dynamic"; @@ -25,7 +26,7 @@ export const GET = apiHandler(async () => { }); } - const response = await fetch(settings.umamiScriptUrl, { + const response = await safeFetch(settings.umamiScriptUrl, { next: { revalidate: 3600 }, }); if (!response.ok) { diff --git a/src/app/api/send/route.ts b/src/app/api/send/route.ts index fe00828d7..32e77efd5 100644 --- a/src/app/api/send/route.ts +++ b/src/app/api/send/route.ts @@ -5,6 +5,7 @@ import { annotate } from "@/lib/logging/context"; import { getPublicMonitoringSettings } from "@/lib/monitoring-settings"; import { checkRateLimit } from "@/lib/rate-limit"; import { isPublicUrl } from "@/lib/validations/notifications"; +import { safeFetch } from "@/lib/safe-fetch"; export const dynamic = "force-dynamic"; @@ -73,7 +74,7 @@ export const POST = apiHandler(async (request: NextRequest) => { let lastResponseStatus = 404; for (const targetUrl of targetUrls) { - const upstream = await fetch(targetUrl, { + const upstream = await safeFetch(targetUrl, { method: "POST", headers: { "content-type": diff --git a/src/app/api/version/check-updates/route.ts b/src/app/api/version/check-updates/route.ts index ae31afdfc..4ed7a645b 100644 --- a/src/app/api/version/check-updates/route.ts +++ b/src/app/api/version/check-updates/route.ts @@ -2,6 +2,7 @@ import { apiHandler, requireAuth } from "@/lib/api-handler"; import { apiError, apiSuccess } from "@/lib/api-response"; import { annotate } from "@/lib/logging/context"; import { getEvent } from "@/lib/logging/context"; +import { safeFetch } from "@/lib/safe-fetch"; import packageJson from "../../../../../package.json"; export const dynamic = "force-dynamic"; @@ -80,7 +81,7 @@ export const GET = apiHandler(async () => { let release: GithubReleaseShape | null = null; const callStart = Date.now(); try { - const res = await fetch(RELEASES_URL, { + const res = await safeFetch(RELEASES_URL, { headers: { Accept: "application/vnd.github+json" }, // GitHub anon rate limit is 60/hr per IP — tight but not zero. // We don't pass a token because the repo is public and the route diff --git a/src/lib/__tests__/safe-fetch.test.ts b/src/lib/__tests__/safe-fetch.test.ts new file mode 100644 index 000000000..cd9973c3f --- /dev/null +++ b/src/lib/__tests__/safe-fetch.test.ts @@ -0,0 +1,133 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { safeFetch, SafeFetchError } from "../safe-fetch"; + +describe("safeFetch", () => { + let fetchSpy: ReturnType; + + beforeEach(() => { + fetchSpy = vi.spyOn(globalThis, "fetch"); + }); + + afterEach(() => { + fetchSpy.mockRestore(); + }); + + it("defaults init.redirect to 'manual'", async () => { + fetchSpy.mockResolvedValueOnce(new Response("ok", { status: 200 })); + + await safeFetch("https://example.com"); + + const init = fetchSpy.mock.calls[0]?.[1] as RequestInit; + expect(init.redirect).toBe("manual"); + }); + + it("flips to 'follow' when opts.followRedirects is true", async () => { + fetchSpy.mockResolvedValueOnce(new Response("ok", { status: 200 })); + + await safeFetch("https://example.com", {}, { followRedirects: true }); + + const init = fetchSpy.mock.calls[0]?.[1] as RequestInit; + expect(init.redirect).toBe("follow"); + }); + + it("preserves an explicit init.redirect override", async () => { + fetchSpy.mockResolvedValueOnce(new Response("ok", { status: 200 })); + + await safeFetch("https://example.com", { redirect: "error" }); + + const init = fetchSpy.mock.calls[0]?.[1] as RequestInit; + expect(init.redirect).toBe("error"); + }); + + it("aborts on timeout (override to a tight window for the test)", async () => { + // Use a real, very short timeout. `AbortSignal.timeout` is a platform + // primitive that vi.useFakeTimers cannot reliably advance, so the + // assertion runs against actual wall-clock — but 25 ms is short + // enough that the suite stays fast. + fetchSpy.mockImplementationOnce( + (_url: unknown, init?: RequestInit) => + new Promise((_resolve, reject) => { + const signal = init?.signal; + signal?.addEventListener("abort", () => { + reject(signal.reason); + }); + }), + ); + + await expect( + safeFetch("https://example.com", {}, { timeoutMs: 25 }), + ).rejects.toMatchObject({ kind: "timeout" }); + }); + + it("respects a caller-supplied signal via AbortSignal.any", async () => { + const controller = new AbortController(); + fetchSpy.mockImplementationOnce( + (_url: unknown, init?: RequestInit) => + new Promise((_resolve, reject) => { + const signal = init?.signal; + signal?.addEventListener("abort", () => { + reject(signal.reason); + }); + }), + ); + + const pending = safeFetch("https://example.com", { + signal: controller.signal, + }); + controller.abort(); + + await expect(pending).rejects.toBeInstanceOf(SafeFetchError); + await expect(pending).rejects.toMatchObject({ kind: "timeout" }); + }); + + it("rejects a private host when requirePublicHost is true", async () => { + await expect( + safeFetch( + "http://169.254.169.254/latest/meta-data/", + {}, + { requirePublicHost: true }, + ), + ).rejects.toMatchObject({ + kind: "private_host", + }); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("rejects an RFC1918 host when requirePublicHost is true", async () => { + await expect( + safeFetch("http://10.0.0.5/admin", {}, { requirePublicHost: true }), + ).rejects.toMatchObject({ + kind: "private_host", + }); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("passes a public host through with requirePublicHost", async () => { + fetchSpy.mockResolvedValueOnce(new Response("ok", { status: 200 })); + + const res = await safeFetch( + "https://example.com/api", + {}, + { requirePublicHost: true }, + ); + + expect(res.status).toBe(200); + expect(fetchSpy).toHaveBeenCalledOnce(); + }); + + it("wraps a native network error as kind: 'network'", async () => { + fetchSpy.mockRejectedValueOnce(new TypeError("network down")); + + await expect(safeFetch("https://example.com")).rejects.toMatchObject({ + kind: "network", + }); + }); + + it("accepts a URL object as the target", async () => { + fetchSpy.mockResolvedValueOnce(new Response("ok", { status: 200 })); + + await safeFetch(new URL("https://example.com/path")); + + expect(fetchSpy.mock.calls[0]?.[0]).toBe("https://example.com/path"); + }); +}); diff --git a/src/lib/ai/__tests__/local-client.test.ts b/src/lib/ai/__tests__/local-client.test.ts index be5f2a489..e0f6f70be 100644 --- a/src/lib/ai/__tests__/local-client.test.ts +++ b/src/lib/ai/__tests__/local-client.test.ts @@ -1,9 +1,17 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { LocalOpenAICompatibleClient } from "../local-client"; describe("LocalOpenAICompatibleClient", () => { beforeEach(() => { vi.restoreAllMocks(); + // These tests cover the operator-opt-in path (LM Studio / Ollama on + // an RFC1918 host). The safeFetch wrapper enforces the SSRF guard + // unless the operator explicitly accepts a private host. + vi.stubEnv("ALLOW_LOCAL_AI_PRIVATE_HOSTS", "true"); + }); + + afterEach(() => { + vi.unstubAllEnvs(); }); it("targets the configured baseUrl + /chat/completions", async () => { diff --git a/src/lib/ai/__tests__/provider-contract.test.ts b/src/lib/ai/__tests__/provider-contract.test.ts index 1113622bd..470b1e63c 100644 --- a/src/lib/ai/__tests__/provider-contract.test.ts +++ b/src/lib/ai/__tests__/provider-contract.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { MockAIProvider } from "../mock-client"; import { OpenAIClient } from "../openai-client"; import { AnthropicClient } from "../anthropic-client"; @@ -32,6 +32,14 @@ function assertCompletionResult(result: CompletionResult): void { describe("AIProvider contract", () => { beforeEach(() => { vi.restoreAllMocks(); + // The local-provider contract test points at a localhost OpenAI- + // compatible server (Ollama / LM Studio). safeFetch enforces the + // SSRF guard unless the operator explicitly opts in. + vi.stubEnv("ALLOW_LOCAL_AI_PRIVATE_HOSTS", "true"); + }); + + afterEach(() => { + vi.unstubAllEnvs(); }); it("MockAIProvider — default response satisfies CompletionResult shape", async () => { diff --git a/src/lib/ai/anthropic-client.ts b/src/lib/ai/anthropic-client.ts index 5a110f1e9..903e3c88a 100644 --- a/src/lib/ai/anthropic-client.ts +++ b/src/lib/ai/anthropic-client.ts @@ -1,3 +1,4 @@ +import { safeFetch } from "@/lib/safe-fetch"; import type { AIProvider, CompletionParams, CompletionResult } from "./types"; interface AnthropicClientConfig { @@ -35,23 +36,26 @@ export class AnthropicClient implements AIProvider { ); const url = `${baseUrl}/messages`; - const res = await fetch(url, { - method: "POST", - headers: { - "Content-Type": "application/json", - "x-api-key": this.config.apiKey, - "anthropic-version": ANTHROPIC_VERSION, + const res = await safeFetch( + url, + { + method: "POST", + headers: { + "Content-Type": "application/json", + "x-api-key": this.config.apiKey, + "anthropic-version": ANTHROPIC_VERSION, + }, + body: JSON.stringify({ + model: this.config.model, + max_tokens: params.maxTokens ?? 1000, + temperature: params.temperature ?? 0.3, + system: params.systemPrompt, + messages: [{ role: "user", content: wrapForJson(params.userPrompt) }], + }), }, - body: JSON.stringify({ - model: this.config.model, - max_tokens: params.maxTokens ?? 1000, - temperature: params.temperature ?? 0.3, - system: params.systemPrompt, - messages: [{ role: "user", content: wrapForJson(params.userPrompt) }], - }), // 60 s ceiling — see openai-client.ts for the rationale. - signal: AbortSignal.timeout(60_000), - }); + { timeoutMs: 60_000 }, + ); if (!res.ok) { // Mirror the openai-client body-capture so 4xx/5xx upstream incidents diff --git a/src/lib/ai/local-client.ts b/src/lib/ai/local-client.ts index d967a4162..df03ff991 100644 --- a/src/lib/ai/local-client.ts +++ b/src/lib/ai/local-client.ts @@ -1,3 +1,4 @@ +import { safeFetch } from "@/lib/safe-fetch"; import type { AIProvider, CompletionParams, CompletionResult } from "./types"; interface LocalClientConfig { @@ -38,23 +39,36 @@ export class LocalOpenAICompatibleClient implements AIProvider { const userPrompt = `Return strict JSON only, no markdown, no commentary.\n\n${params.userPrompt}`; - const res = await fetch(url, { - method: "POST", - headers, - body: JSON.stringify({ - model: this.config.model, - messages: [ - { role: "system", content: params.systemPrompt }, - { role: "user", content: userPrompt }, - ], - temperature: params.temperature ?? 0.3, - max_tokens: params.maxTokens ?? 1000, - }), - // 60 s ceiling — a misbehaving local endpoint (Ollama on a stalled - // GPU, a tar-pit proxy) shouldn't be able to pin a worker - // indefinitely. Real local completions land well inside this. - signal: AbortSignal.timeout(60_000), - }); + // safeFetch defaults: no redirect-follow (the local endpoint is the + // most exploitable on this surface — a user-controlled baseUrl that + // 302s to 169.254.169.254 would otherwise leak the bearer on the + // redirected hop into the visible error envelope via `bodyExcerpt`) + // and an explicit 60 s ceiling so a stalled GPU or tar-pit proxy + // can't pin a worker indefinitely. `requirePublicHost` gates the + // baseUrl against the input-time SSRF allowlist; the dispatcher + // pin (issue #217) extends the same flag with a connect-time + // resolved-IP check to also defeat DNS rebinding. Operators who + // legitimately point at a self-hosted Ollama / LM Studio on an + // RFC1918 address opt out via `ALLOW_LOCAL_AI_PRIVATE_HOSTS=true` + // (also enforced at write-time in /api/user/ai-provider). + const allowPrivate = process.env.ALLOW_LOCAL_AI_PRIVATE_HOSTS === "true"; + const res = await safeFetch( + url, + { + method: "POST", + headers, + body: JSON.stringify({ + model: this.config.model, + messages: [ + { role: "system", content: params.systemPrompt }, + { role: "user", content: userPrompt }, + ], + temperature: params.temperature ?? 0.3, + max_tokens: params.maxTokens ?? 1000, + }), + }, + { timeoutMs: 60_000, requirePublicHost: !allowPrivate }, + ); if (!res.ok) { // Mirror the openai-client structured-error pattern. Local endpoints diff --git a/src/lib/ai/openai-client.ts b/src/lib/ai/openai-client.ts index f4b17d1e3..06a3b3a00 100644 --- a/src/lib/ai/openai-client.ts +++ b/src/lib/ai/openai-client.ts @@ -1,3 +1,4 @@ +import { safeFetch } from "@/lib/safe-fetch"; import type { AIProvider, CompletionParams, CompletionResult } from "./types"; interface OpenAIClientConfig { @@ -30,27 +31,29 @@ export class OpenAIClient implements AIProvider { ): Promise { const url = `${this.config.baseUrl.replace(/\/$/, "")}/chat/completions`; - const res = await fetch(url, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: `Bearer ${this.config.apiKey}`, + const res = await safeFetch( + url, + { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${this.config.apiKey}`, + }, + body: JSON.stringify({ + model: this.config.model, + messages: [ + { role: "system", content: params.systemPrompt }, + { role: "user", content: params.userPrompt }, + ], + temperature: params.temperature ?? 0.3, + max_tokens: params.maxTokens ?? 1000, + response_format: { type: "json_object" }, + }), }, - body: JSON.stringify({ - model: this.config.model, - messages: [ - { role: "system", content: params.systemPrompt }, - { role: "user", content: params.userPrompt }, - ], - temperature: params.temperature ?? 0.3, - max_tokens: params.maxTokens ?? 1000, - response_format: { type: "json_object" }, - }), - // Cap any single completion at 60 s so a tar-pit upstream cannot - // pin a worker indefinitely. Real completions land well inside this - // budget — see `getEvent()?.addExternalCall` timings in production. - signal: AbortSignal.timeout(60_000), - }); + // 60 s ceiling so a tar-pit upstream cannot pin a worker + // indefinitely. Real completions land well inside this budget. + { timeoutMs: 60_000 }, + ); if (!res.ok) { // Pull as much of the body as we can so upstream incidents (OpenAI 5xx, diff --git a/src/lib/feedback/publish-github.ts b/src/lib/feedback/publish-github.ts index d31d56e9d..948b692a0 100644 --- a/src/lib/feedback/publish-github.ts +++ b/src/lib/feedback/publish-github.ts @@ -9,6 +9,7 @@ import { prisma } from "@/lib/db"; import { decrypt } from "@/lib/crypto"; import { getEvent } from "@/lib/logging/context"; +import { safeFetch } from "@/lib/safe-fetch"; export interface FeedbackInput { category: string; @@ -135,7 +136,7 @@ export async function publishFeedbackToGithub( const title = `[${categoryLabel}] ${safeSubject || `Feedback – ${dateStr}`}`; - const res = await fetch( + const res = await safeFetch( `https://api.github.com/repos/${config.repo}/issues`, { method: "POST", diff --git a/src/lib/geo.ts b/src/lib/geo.ts index 887217682..e9b5b4358 100644 --- a/src/lib/geo.ts +++ b/src/lib/geo.ts @@ -65,6 +65,7 @@ import type { CityResponse, } from "mmdb-lib/lib/reader/response"; import { getEvent } from "@/lib/logging/context"; +import { safeFetch } from "@/lib/safe-fetch"; interface IpwhoIsResponse { success?: boolean; @@ -284,20 +285,23 @@ async function readUtf8Json(res: Response): Promise { async function lookupIpLocationOnline(ip: string): Promise { if (process.env.IP_GEO_LOOKUP_DISABLED === "1") return null; try { - const res = await fetch(buildLookupUrl(ip), { - signal: AbortSignal.timeout(3000), - headers: { - // ipwho.is and ip-api both honour Accept-Language for the - // city field. Without the hint, ip-api falls back to the - // English ASCII fold ("Nuremberg" instead of "Nürnberg"). - // German first because the user-base is DACH-skewed; English - // as a graceful fallback for cities that don't have a German - // name. Quality 0.5 on the en fallback keeps providers from - // tie-breaking the wrong way. - "Accept-Language": "de, en;q=0.5", - Accept: "application/json", + const res = await safeFetch( + buildLookupUrl(ip), + { + headers: { + // ipwho.is and ip-api both honour Accept-Language for the + // city field. Without the hint, ip-api falls back to the + // English ASCII fold ("Nuremberg" instead of "Nürnberg"). + // German first because the user-base is DACH-skewed; English + // as a graceful fallback for cities that don't have a German + // name. Quality 0.5 on the en fallback keeps providers from + // tie-breaking the wrong way. + "Accept-Language": "de, en;q=0.5", + Accept: "application/json", + }, }, - }); + { timeoutMs: 3_000 }, + ); if (!res.ok) return null; const data = (await readUtf8Json(res)) as GeoResponse; diff --git a/src/lib/logging/transports.ts b/src/lib/logging/transports.ts index 732fd0a07..15a360f91 100644 --- a/src/lib/logging/transports.ts +++ b/src/lib/logging/transports.ts @@ -2,6 +2,7 @@ import type { WideEvent } from "./types"; import { getLoggingConfig } from "./config"; import { shouldEmit } from "./sampler"; import { appendLogEvent } from "./in-memory-buffer"; +import { safeFetch } from "@/lib/safe-fetch"; /** Event auf stdout als einzelne JSON-Zeile schreiben */ function emitToStdout(event: WideEvent): void { @@ -64,12 +65,15 @@ async function flushLokiBuffer(): Promise { } try { - await fetch(`${config.lokiEndpoint}/loki/api/v1/push`, { - method: "POST", - headers, - body: JSON.stringify({ streams }), - signal: AbortSignal.timeout(10_000), - }); + await safeFetch( + `${config.lokiEndpoint}/loki/api/v1/push`, + { + method: "POST", + headers, + body: JSON.stringify({ streams }), + }, + { timeoutMs: 10_000 }, + ); } catch { // Events gehen verloren — akzeptabel fuer Logging } diff --git a/src/lib/monitoring/glitchtip.ts b/src/lib/monitoring/glitchtip.ts index a6a5efe7f..e14f37af8 100644 --- a/src/lib/monitoring/glitchtip.ts +++ b/src/lib/monitoring/glitchtip.ts @@ -1,3 +1,5 @@ +import { safeFetch } from "@/lib/safe-fetch"; + interface ParsedGlitchtipDsn { dsn: string; publicKey: string; @@ -121,7 +123,7 @@ async function tryEnvelope( config: ParsedGlitchtipDsn, body: string, ): Promise { - const response = await fetch(config.envelopeUrl, { + const response = await safeFetch(config.envelopeUrl, { method: "POST", headers: { "content-type": "application/x-sentry-envelope", @@ -150,7 +152,7 @@ async function tryStoreQuery( target.searchParams.set("sentry_version", "7"); target.searchParams.set("sentry_client", "healthlog/1.0"); - const response = await fetch(target, { + const response = await safeFetch(target, { method: "POST", headers: { "content-type": "application/json", @@ -175,7 +177,7 @@ async function tryStoreHeader( payload: Record, ): Promise { const sentryAuth = `Sentry sentry_version=7, sentry_key=${config.publicKey}, sentry_client=healthlog/1.0`; - const response = await fetch(config.storeUrl, { + const response = await safeFetch(config.storeUrl, { method: "POST", headers: { "content-type": "application/json", diff --git a/src/lib/moodlog/push.ts b/src/lib/moodlog/push.ts index fe0409ad9..a4c2382a7 100644 --- a/src/lib/moodlog/push.ts +++ b/src/lib/moodlog/push.ts @@ -2,6 +2,7 @@ import { prisma } from "@/lib/db"; import { decrypt } from "@/lib/crypto"; import { getEvent } from "@/lib/logging/context"; import { isPublicUrl } from "@/lib/validations/notifications"; +import { safeFetch } from "@/lib/safe-fetch"; /** * v1.4.50 — reverse-sync push: HealthLog → MoodLog. @@ -110,24 +111,22 @@ export async function pushMoodEntriesToMoodLog( })), }; - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), 10_000); - const start = performance.now(); let response: Response; try { - response = await fetch(url.toString(), { - method: "POST", - headers: { - Authorization: `Bearer ${apiKey}`, - "Content-Type": "application/json", + response = await safeFetch( + url.toString(), + { + method: "POST", + headers: { + Authorization: `Bearer ${apiKey}`, + "Content-Type": "application/json", + }, + body: JSON.stringify(body), }, - body: JSON.stringify(body), - signal: controller.signal, - redirect: "manual", - }); + { timeoutMs: 10_000 }, + ); } catch (err) { - clearTimeout(timeout); const message = err instanceof Error ? err.message : String(err); getEvent()?.addExternalCall({ service: "moodlog", @@ -138,8 +137,6 @@ export async function pushMoodEntriesToMoodLog( `moodlog push failed for ${userId} (fetch): ${message}`, ); return { pushed: 0, skipped: candidates.length, status: "failed" }; - } finally { - clearTimeout(timeout); } getEvent()?.addExternalCall({ diff --git a/src/lib/moodlog/sync.ts b/src/lib/moodlog/sync.ts index b549c33d5..ccd238057 100644 --- a/src/lib/moodlog/sync.ts +++ b/src/lib/moodlog/sync.ts @@ -2,6 +2,7 @@ import { prisma } from "@/lib/db"; import { decrypt } from "@/lib/crypto"; import { getEvent } from "@/lib/logging/context"; import { isPublicUrl } from "@/lib/validations/notifications"; +import { safeFetch } from "@/lib/safe-fetch"; import { isReauthRequired, recordSyncFailure, @@ -87,23 +88,19 @@ export async function syncMoodLogEntries( url.searchParams.set("from", from); url.searchParams.set("to", to); - const controller = new AbortController(); - const timeoutMs = opts?.fullSync ? 60000 : 15000; - const timeout = setTimeout(() => controller.abort(), timeoutMs); - + // safeFetch keeps the no-redirect-follow guard (the apiKey would + // otherwise leak on an attacker-controlled 302 hop) and adds a hard + // upper bound via AbortSignal.timeout. + const timeoutMs = opts?.fullSync ? 60_000 : 15_000; let response: Response; const fetchStart = performance.now(); try { - response = await fetch(url.toString(), { - headers: { Authorization: `Bearer ${apiKey}` }, - signal: controller.signal, - // SSRF defence-in-depth: do NOT follow redirects. A public - // host that 302s to an RFC1918 target would otherwise leak - // the apiKey to the internal hop. - redirect: "manual", - }); + response = await safeFetch( + url.toString(), + { headers: { Authorization: `Bearer ${apiKey}` } }, + { timeoutMs }, + ); } catch (err) { - clearTimeout(timeout); getEvent()?.addExternalCall({ service: "moodlog", method: "syncMoodLogEntries", @@ -118,8 +115,6 @@ export async function syncMoodLogEntries( errorCode: "fetch_failed", }); return 0; - } finally { - clearTimeout(timeout); } getEvent()?.addExternalCall({ service: "moodlog", diff --git a/src/lib/notifications/senders/ntfy.ts b/src/lib/notifications/senders/ntfy.ts index 79927d94a..608cb8c43 100644 --- a/src/lib/notifications/senders/ntfy.ts +++ b/src/lib/notifications/senders/ntfy.ts @@ -6,6 +6,7 @@ import type { SendOutcome } from "@/lib/notifications/retry-policy"; import { classifyHttpStatus } from "@/lib/notifications/retry-policy"; import { getEvent } from "@/lib/logging/context"; import { recordPushAttempt } from "@/lib/notifications/senders/push-attempt-record"; +import { safeFetch } from "@/lib/safe-fetch"; /** * Send notification via ntfy (simple HTTP POST). @@ -38,12 +39,15 @@ export async function sendViaNtfy( // Strip HTML tags for ntfy (plain text only) const body = payload.message.replace(/<[^>]*>/g, ""); - const res = await fetch(url, { - method: "POST", - headers, - body, - signal: AbortSignal.timeout(5000), - }); + const res = await safeFetch( + url, + { + method: "POST", + headers, + body, + }, + { timeoutMs: 5_000 }, + ); getEvent()?.addExternalCall({ service: "ntfy", diff --git a/src/lib/safe-fetch.ts b/src/lib/safe-fetch.ts new file mode 100644 index 000000000..86b0754b5 --- /dev/null +++ b/src/lib/safe-fetch.ts @@ -0,0 +1,139 @@ +import { isPublicUrl } from "@/lib/validations/notifications"; + +/** + * Defaults applied to every `safeFetch` call. Centralised so the values + * stay greppable and a future audit can verify the convention in one + * place rather than spread across ~20 outbound call sites. + */ +const DEFAULT_TIMEOUT_MS = 15_000; + +export type SafeFetchErrorKind = "private_host" | "timeout" | "network"; + +export interface SafeFetchOptions { + /** + * Follow 3xx responses. Defaults to `false`, i.e. `init.redirect` is + * pinned to `"manual"`. The opt-in exists so a caller with a legitimate + * redirect-following need (e.g. resolving a CDN's URL chain) can ask for + * the WHATWG default; flipping the convention from "remembered to opt + * in to manual" to "remembered to opt out" was the whole point of + * issue #218. + */ + followRedirects?: boolean; + /** + * Override the default 15 000 ms timeout. The timeout is composed with + * any caller-supplied `init.signal` via `AbortSignal.any`, so whichever + * fires first wins. + */ + timeoutMs?: number; + /** + * When true, run `isPublicUrl(url)` before dispatching. Throws + * `SafeFetchError({ kind: "private_host" })` if the URL points at an + * internal range, the cloud metadata endpoint, or any of the IPv4 / + * IPv6 alt-notation classes `isPublicUrl` catches. The DNS-rebinding + * follow-up (issue #217) extends this same flag with a connect-time + * IP pin via a dedicated undici dispatcher. + */ + requirePublicHost?: boolean; + /** + * Optional caller-owned signal. Composed with the timeout signal so a + * caller-side abort still wins even when the timeout has not yet + * elapsed. + */ + signal?: AbortSignal; +} + +/** + * Typed wrapper around outbound fetch failures so callers can switch + * on `err.kind` instead of doing brittle string matching against + * `AbortError` / `TypeError`. Native fetch errors surface as + * `kind: "network"`; timeouts become `kind: "timeout"`; input-time + * private-host rejections become `kind: "private_host"`. + */ +export class SafeFetchError extends Error { + readonly kind: SafeFetchErrorKind; + readonly cause?: unknown; + + constructor(message: string, kind: SafeFetchErrorKind, cause?: unknown) { + super(message); + this.name = "SafeFetchError"; + this.kind = kind; + if (cause !== undefined) this.cause = cause; + } +} + +function composeSignals( + caller: AbortSignal | undefined, + timeoutMs: number, +): AbortSignal { + const timeout = AbortSignal.timeout(timeoutMs); + if (!caller) return timeout; + return AbortSignal.any([caller, timeout]); +} + +/** + * Default-safe outbound fetch wrapper. + * + * - `init.redirect` defaults to `"manual"` so a public host that 302s to + * `169.254.169.254` (cloud metadata) or an RFC1918 admin panel cannot + * coerce the client into leaking the request's `Authorization` header + * on the redirected hop. Callers that explicitly set `init.redirect` + * keep their choice; callers that pass `opts.followRedirects: true` + * get the WHATWG default of `"follow"`. + * - `init.signal` defaults to `AbortSignal.timeout(15_000)` so a tar-pit + * upstream cannot pin a worker indefinitely. Composes with any + * caller-owned signal via `AbortSignal.any`. + * - When `opts.requirePublicHost` is true, runs `isPublicUrl` first and + * throws `SafeFetchError({ kind: "private_host" })` on rejection. The + * DNS-rebinding hardening (issue #217) extends this flag with a + * connect-time IP pin via the pinned dispatcher. + * + * This is the documented egress entry point for every outbound call that + * does NOT target a hard-coded host (Anthropic / OpenAI / Apple / + * Telegram / Withings / GitHub — constant strings, no rebinding surface + * by construction). The four hard-coded hosts also route through it for + * convention consistency. + */ +export async function safeFetch( + url: string | URL, + init: RequestInit = {}, + opts: SafeFetchOptions = {}, +): Promise { + const target = url.toString(); + + if (opts.requirePublicHost && !isPublicUrl(target)) { + throw new SafeFetchError( + `safeFetch refused private or non-public host: ${target}`, + "private_host", + ); + } + + const redirect: RequestRedirect = + init.redirect ?? (opts.followRedirects ? "follow" : "manual"); + + const signal = composeSignals( + init.signal ?? opts.signal ?? undefined, + opts.timeoutMs ?? DEFAULT_TIMEOUT_MS, + ); + + try { + return await fetch(target, { ...init, redirect, signal }); + } catch (err) { + // `AbortSignal.timeout` aborts with `TimeoutError` (`DOMException` + // named "TimeoutError"); a caller-side abort surfaces as + // `AbortError`. Both manifest as the same `err.name` check. + if (err instanceof DOMException) { + if (err.name === "TimeoutError" || err.name === "AbortError") { + throw new SafeFetchError( + `safeFetch aborted (${err.name}): ${target}`, + "timeout", + err, + ); + } + } + throw new SafeFetchError( + `safeFetch network error: ${err instanceof Error ? err.message : String(err)}`, + "network", + err, + ); + } +} diff --git a/src/lib/telegram.ts b/src/lib/telegram.ts index 156661410..a9729de9e 100644 --- a/src/lib/telegram.ts +++ b/src/lib/telegram.ts @@ -3,6 +3,7 @@ * Uses the HTTP API directly — no library needed. */ import { getEvent } from "@/lib/logging/context"; +import { safeFetch } from "@/lib/safe-fetch"; interface TelegramResponse { ok: boolean; @@ -31,14 +32,14 @@ async function telegramApiRequest( ): Promise { const start = performance.now(); try { - const res = await fetch( + const res = await safeFetch( `https://api.telegram.org/bot${botToken}/${method}`, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify(payload), - signal: AbortSignal.timeout(10_000), }, + { timeoutMs: 10_000 }, ); const data = (await res.json()) as TelegramResponse; getEvent()?.addExternalCall({