From d35458f78b2d166bbf53e3ccf8700e11cf12c8f1 Mon Sep 17 00:00:00 2001 From: ZacharyZcR Date: Tue, 25 Aug 2026 00:55:36 +0800 Subject: [PATCH] fix: allow approved private notification hosts (#1330) --- src/backend/automations/http.ts | 14 +-- .../database/routes/user-settings-routes.ts | 88 +++++++++++++++++++ .../tests/utils/notification-egress.test.ts | 17 ++++ .../tests/utils/safe-outbound-fetch.test.ts | 18 +++- src/backend/utils/discord-sender.ts | 4 +- src/backend/utils/notification-egress.ts | 25 ++++++ src/backend/utils/notification-sender.ts | 4 +- src/backend/utils/safe-outbound-fetch.ts | 18 +++- src/ui/api/ai-api.ts | 21 +++++ src/ui/locales/en.json | 3 + src/ui/sidebar/AdminSettingsPanel.tsx | 26 ++++++ src/ui/sidebar/AdminSettingsSections.tsx | 25 ++++++ 12 files changed, 250 insertions(+), 13 deletions(-) create mode 100644 src/backend/tests/utils/notification-egress.test.ts create mode 100644 src/backend/utils/notification-egress.ts diff --git a/src/backend/automations/http.ts b/src/backend/automations/http.ts index db06951b..569addf4 100644 --- a/src/backend/automations/http.ts +++ b/src/backend/automations/http.ts @@ -1,13 +1,14 @@ import { safeOutboundFetch } from "../utils/safe-outbound-fetch.js"; +import { readNotificationPrivateAllowlist } from "../utils/notification-egress.js"; /** * Outbound HTTP for automation steps and notification channels. * * safeOutboundFetch refuses private and loopback addresses, which is the right * default against SSRF but also blocks the self-hosted ntfy or Gotify sitting - * on a LAN that many installs actually use. Rather than weaken the guard - * globally, a destination can opt in explicitly; everything else about the - * guard (scheme, embedded credentials, no redirects) still applies. + * on a LAN that many installs actually use. Private delivery therefore needs + * both a channel opt-in and an exact host in the administrator allowlist; + * scheme validation, DNS pinning and redirect refusal remain in force. */ export interface AutomationFetchOptions { method?: string; @@ -52,8 +53,8 @@ export async function automationFetch( } /** - * The opt-in path. Keeps the parts of the guard that are always right and - * drops only the address blocklist. + * The opt-in path still goes through the guarded resolver. Only an exact host + * authorized by an administrator may resolve to a private address. */ async function privateNetworkFetch( rawUrl: string, @@ -73,5 +74,6 @@ async function privateNetworkFetch( throw new Error("URLs with embedded credentials are not allowed"); } - return fetch(rawUrl, { ...init, redirect: "error" }); + const allowlist = await readNotificationPrivateAllowlist(); + return safeOutboundFetch(rawUrl, init, allowlist); } diff --git a/src/backend/database/routes/user-settings-routes.ts b/src/backend/database/routes/user-settings-routes.ts index 47f8a2cc..40df928b 100644 --- a/src/backend/database/routes/user-settings-routes.ts +++ b/src/backend/database/routes/user-settings-routes.ts @@ -14,6 +14,10 @@ import { } from "../../utils/audit-forwarder.js"; import { getTelemetryEnvOverride } from "../../utils/analytics.js"; import { AI_PRIVATE_ALLOWLIST_KEY, parseAllowlist } from "../../ai/egress.js"; +import { + NOTIFICATION_PRIVATE_ALLOWLIST_KEY, + parseNotificationAllowlist, +} from "../../utils/notification-egress.js"; import { createCurrentSettingsRepository, createCurrentUserRepository, @@ -1078,6 +1082,90 @@ export function registerUserSettingsRoutes( } }); + router.get( + "/notification-private-endpoints", + authenticateJWT, + async (req, res) => { + const userId = (req as AuthenticatedRequest).userId; + try { + if (!(await getAdminActor(userId))) { + return res.status(403).json({ error: "Not authorized" }); + } + const raw = await createCurrentSettingsRepository().get( + NOTIFICATION_PRIVATE_ALLOWLIST_KEY, + ); + res.json({ hosts: parseNotificationAllowlist(raw) }); + } catch (err) { + authLogger.error("Failed to get notification endpoint allowlist", err); + res.status(500).json({ error: "Failed to get the allowlist" }); + } + }, + ); + + router.patch( + "/notification-private-endpoints", + authenticateJWT, + async (req, res) => { + const userId = (req as AuthenticatedRequest).userId; + try { + const actor = await getAdminActor(userId); + if (!actor) { + return res.status(403).json({ error: "Not authorized" }); + } + + const { hosts } = req.body; + if (!Array.isArray(hosts)) { + return res.status(400).json({ error: "hosts must be an array" }); + } + if (hosts.length > 50) { + return res + .status(400) + .json({ error: "At most 50 hosts are allowed" }); + } + + const cleaned: string[] = []; + for (const entry of hosts) { + if (typeof entry !== "string") { + return res + .status(400) + .json({ error: "Each host must be a string" }); + } + const host = entry.trim().toLowerCase(); + if (!host) continue; + if (!/^[a-z0-9._:-]+$/.test(host)) { + return res + .status(400) + .json({ error: `${entry} is not a valid hostname` }); + } + if (!cleaned.includes(host)) cleaned.push(host); + } + + await createCurrentSettingsRepository().set( + NOTIFICATION_PRIVATE_ALLOWLIST_KEY, + JSON.stringify(cleaned), + ); + const { ipAddress, userAgent } = getRequestMeta(req); + await logAudit({ + userId, + username: actor.username ?? userId, + action: "update_notification_private_endpoints", + resourceType: "setting", + details: JSON.stringify({ hosts: cleaned }), + ipAddress, + userAgent, + success: true, + }); + res.json({ hosts: cleaned }); + } catch (err) { + authLogger.error( + "Failed to update notification endpoint allowlist", + err, + ); + res.status(500).json({ error: "Failed to update the allowlist" }); + } + }, + ); + /** * @openapi * /users/host-defaults: diff --git a/src/backend/tests/utils/notification-egress.test.ts b/src/backend/tests/utils/notification-egress.test.ts new file mode 100644 index 00000000..ac38cfeb --- /dev/null +++ b/src/backend/tests/utils/notification-egress.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from "vitest"; +import { parseNotificationAllowlist } from "../../utils/notification-egress.js"; + +describe("parseNotificationAllowlist", () => { + it("defaults to an empty list", () => { + expect(parseNotificationAllowlist(null)).toEqual([]); + expect(parseNotificationAllowlist("invalid")).toEqual([]); + }); + + it("normalizes configured hosts", () => { + expect( + parseNotificationAllowlist( + JSON.stringify([" NTFY.Internal ", "192.168.1.20", 42, ""]), + ), + ).toEqual(["ntfy.internal", "192.168.1.20"]); + }); +}); diff --git a/src/backend/tests/utils/safe-outbound-fetch.test.ts b/src/backend/tests/utils/safe-outbound-fetch.test.ts index e2fc1b20..fe54d398 100644 --- a/src/backend/tests/utils/safe-outbound-fetch.test.ts +++ b/src/backend/tests/utils/safe-outbound-fetch.test.ts @@ -51,6 +51,7 @@ function runHook( addresses: LookupAddress[] | string | undefined, error: NodeJS.ErrnoException | null = null, lookupOptions: LookupOptions = { all: true }, + allowPrivate = false, ) { const fakeLookup = vi.fn( ( @@ -66,7 +67,7 @@ function runHook( }, ); - const hook = createDnsLookupHook(fakeLookup); + const hook = createDnsLookupHook(fakeLookup, allowPrivate); const callback = vi.fn(); hook("example.invalid", lookupOptions, callback); @@ -92,6 +93,21 @@ const publicAddresses: LookupAddress[] = [ ]; describe("createDnsLookupHook", () => { + it("permits private results only for an explicitly authorized host", () => { + const { callback } = runHook( + [{ address: "192.168.1.20", family: 4 }], + null, + { all: true }, + true, + ); + + expect(callback).toHaveBeenCalledWith( + null, + [{ address: "192.168.1.20", family: 4 }], + 0, + ); + }); + it("allows a public IPv4 address through", () => { const { callback } = runHook([ { diff --git a/src/backend/utils/discord-sender.ts b/src/backend/utils/discord-sender.ts index 817ff5b2..0a416256 100644 --- a/src/backend/utils/discord-sender.ts +++ b/src/backend/utils/discord-sender.ts @@ -1,5 +1,6 @@ import { safeOutboundFetch } from "./safe-outbound-fetch.js"; import { statsLogger } from "./logger.js"; +import { readNotificationPrivateAllowlist } from "./notification-egress.js"; export interface DiscordConfig { url: string; @@ -14,7 +15,8 @@ async function fetchWithRetry( options: RequestInit, ): Promise { const attempt = async () => { - const res = await safeOutboundFetch(url, options); + const allowlist = await readNotificationPrivateAllowlist(); + const res = await safeOutboundFetch(url, options, allowlist); if (!res.ok) { let body = ""; try { diff --git a/src/backend/utils/notification-egress.ts b/src/backend/utils/notification-egress.ts new file mode 100644 index 00000000..fbfa8be3 --- /dev/null +++ b/src/backend/utils/notification-egress.ts @@ -0,0 +1,25 @@ +import { createCurrentSettingsRepository } from "../database/repositories/factory.js"; + +export const NOTIFICATION_PRIVATE_ALLOWLIST_KEY = + "notification_private_endpoint_allowlist"; + +export function parseNotificationAllowlist(raw: string | null): string[] { + if (!raw) return []; + try { + const value = JSON.parse(raw); + if (!Array.isArray(value)) return []; + return value + .filter((entry): entry is string => typeof entry === "string") + .map((entry) => entry.trim().toLowerCase()) + .filter(Boolean); + } catch { + return []; + } +} + +export async function readNotificationPrivateAllowlist(): Promise { + const raw = await createCurrentSettingsRepository().get( + NOTIFICATION_PRIVATE_ALLOWLIST_KEY, + ); + return parseNotificationAllowlist(raw); +} diff --git a/src/backend/utils/notification-sender.ts b/src/backend/utils/notification-sender.ts index 768bef2d..4c050036 100644 --- a/src/backend/utils/notification-sender.ts +++ b/src/backend/utils/notification-sender.ts @@ -1,5 +1,6 @@ import { statsLogger } from "./logger.js"; import { safeOutboundFetch } from "./safe-outbound-fetch.js"; +import { readNotificationPrivateAllowlist } from "./notification-egress.js"; export interface AlertPayload { hostName: string; @@ -37,7 +38,8 @@ async function fetchWithRetry( options: RequestInit, ): Promise { const attempt = async () => { - const res = await safeOutboundFetch(url, options); + const allowlist = await readNotificationPrivateAllowlist(); + const res = await safeOutboundFetch(url, options, allowlist); if (!res.ok) { throw new Error(`HTTP ${res.status}: ${res.statusText}`); } diff --git a/src/backend/utils/safe-outbound-fetch.ts b/src/backend/utils/safe-outbound-fetch.ts index 5eaf78a9..0ebf4262 100644 --- a/src/backend/utils/safe-outbound-fetch.ts +++ b/src/backend/utils/safe-outbound-fetch.ts @@ -71,7 +71,10 @@ export function isBlockedAddress(address: string): boolean { // fake DNS resolver, instead of only through a real fetch()/Agent call — // the actual bug here lived entirely in this callback, several layers // below where undici's own "fetch failed" wrapping would otherwise hide it. -export function createDnsLookupHook(dnsLookup: DnsLookupFn = lookup) { +export function createDnsLookupHook( + dnsLookup: DnsLookupFn = lookup, + allowPrivate = false, +) { return function lookupHook( host: string, lookupOptions: LookupOptions, @@ -110,7 +113,10 @@ export function createDnsLookupHook(dnsLookup: DnsLookupFn = lookup) { ); } - if (addrs.some(({ address }) => isBlockedAddress(address))) { + if ( + !allowPrivate && + addrs.some(({ address }) => isBlockedAddress(address)) + ) { return callback( new Error("Private destinations are not allowed"), "", @@ -144,6 +150,7 @@ export function createDnsLookupHook(dnsLookup: DnsLookupFn = lookup) { export async function safeOutboundFetch( rawUrl: string, options: RequestInit, + allowedPrivateHosts: readonly string[] = [], ): Promise { const url = new URL(rawUrl); if ( @@ -155,13 +162,16 @@ export async function safeOutboundFetch( } const hostname = url.hostname.replace(/^\[|\]$/g, ""); - if (isIP(hostname) && isBlockedAddress(hostname)) { + const allowPrivate = allowedPrivateHosts.some( + (host) => host.trim().toLowerCase() === hostname.toLowerCase(), + ); + if (!allowPrivate && isIP(hostname) && isBlockedAddress(hostname)) { throw new Error("Private destinations are not allowed"); } const dispatcher = new Agent({ connect: { - lookup: createDnsLookupHook(lookup), + lookup: createDnsLookupHook(lookup, allowPrivate), }, }); diff --git a/src/ui/api/ai-api.ts b/src/ui/api/ai-api.ts index b634e439..d28609fd 100644 --- a/src/ui/api/ai-api.ts +++ b/src/ui/api/ai-api.ts @@ -216,3 +216,24 @@ export async function setAiPrivateEndpoints( throw handleApiError(error, "update AI endpoint allowlist"); } } + +export async function getNotificationPrivateEndpoints(): Promise { + try { + return (await authApi.get("/users/notification-private-endpoints")).data + .hosts; + } catch (error) { + throw handleApiError(error, "get notification endpoint allowlist"); + } +} + +export async function setNotificationPrivateEndpoints( + hosts: string[], +): Promise { + try { + return ( + await authApi.patch("/users/notification-private-endpoints", { hosts }) + ).data.hosts; + } catch (error) { + throw handleApiError(error, "update notification endpoint allowlist"); + } +} diff --git a/src/ui/locales/en.json b/src/ui/locales/en.json index 6b115c54..b525712a 100644 --- a/src/ui/locales/en.json +++ b/src/ui/locales/en.json @@ -3595,6 +3595,9 @@ "aiGloballyEnabledDesc": "Let users turn on the AI assistant. While this is off, the assistant is hidden and blocked for everyone.", "aiPrivateEndpoints": "Allowed private AI hosts", "aiPrivateEndpointsDesc": "Hosts on your private network that users may point a provider at, such as a self-hosted Ollama. Separate them with commas.", + "notificationPrivateEndpoints": "Allowed private notification hosts", + "notificationPrivateEndpointsDesc": "Exact private hosts that notification channels may contact. Separate them with commas.", + "updateNotificationEndpointsFailed": "Failed to update notification endpoint allowlist", "updateAiEnabledFailed": "Could not update the AI setting", "updateAiEndpointsFailed": "Could not update the allowed hosts" }, diff --git a/src/ui/sidebar/AdminSettingsPanel.tsx b/src/ui/sidebar/AdminSettingsPanel.tsx index d54a0aef..06d5c843 100644 --- a/src/ui/sidebar/AdminSettingsPanel.tsx +++ b/src/ui/sidebar/AdminSettingsPanel.tsx @@ -4,8 +4,10 @@ import { notifyAiStatusChanged } from "@/hooks/use-ai-availability"; import { getAiGloballyEnabled, getAiPrivateEndpoints, + getNotificationPrivateEndpoints, setAiGloballyEnabled as setAiGloballyEnabledApi, setAiPrivateEndpoints as setAiPrivateEndpointsApi, + setNotificationPrivateEndpoints as setNotificationPrivateEndpointsApi, } from "@/api/ai-api"; import { getUserList, @@ -169,6 +171,8 @@ export function AdminSettingsPanel({ useState(true); const [aiGloballyEnabled, setAiGloballyEnabled] = useState(false); const [aiPrivateEndpoints, setAiPrivateEndpoints] = useState([]); + const [notificationPrivateEndpoints, setNotificationPrivateEndpoints] = + useState([]); const [hostDefaults, setHostDefaults] = useState({}); const [touchInputSettings, setTouchInputSettings] = useState({ ...TOUCH_INPUT_DEFAULTS }); @@ -365,6 +369,7 @@ export function AdminSettingsPanel({ touchInput, aiEnabled, aiEndpoints, + notificationEndpoints, imageStorage, ] = await Promise.allSettled([ getRegistrationAllowed(), @@ -383,6 +388,7 @@ export function AdminSettingsPanel({ getTouchInputSettings(), getAiGloballyEnabled(), getAiPrivateEndpoints(), + getNotificationPrivateEndpoints(), getTerminalImageStorageSettings(), ]); @@ -435,6 +441,9 @@ export function AdminSettingsPanel({ if (aiEndpoints.status === "fulfilled") { setAiPrivateEndpoints(aiEndpoints.value); } + if (notificationEndpoints.status === "fulfilled") { + setNotificationPrivateEndpoints(notificationEndpoints.value); + } if (imageStorage.status === "fulfilled") { setImageStorageSettings(imageStorage.value); } @@ -594,6 +603,19 @@ export function AdminSettingsPanel({ } } + async function handleSaveNotificationPrivateEndpoints(hosts: string[]) { + const previous = notificationPrivateEndpoints; + setNotificationPrivateEndpoints(hosts); + try { + setNotificationPrivateEndpoints( + await setNotificationPrivateEndpointsApi(hosts), + ); + } catch { + setNotificationPrivateEndpoints(previous); + toast.error(t("admin.updateNotificationEndpointsFailed")); + } + } + async function saveTouchInputSettings(settings = touchInputSettings) { try { const saved = await updateTouchInputSettings(settings); @@ -1119,6 +1141,10 @@ export function AdminSettingsPanel({ onToggleAiGloballyEnabled={handleToggleAiGloballyEnabled} aiPrivateEndpoints={aiPrivateEndpoints} onSaveAiPrivateEndpoints={handleSaveAiPrivateEndpoints} + notificationPrivateEndpoints={notificationPrivateEndpoints} + onSaveNotificationPrivateEndpoints={ + handleSaveNotificationPrivateEndpoints + } handleToggleSessionSharingGloballyEnabled={ handleToggleSessionSharingGloballyEnabled } diff --git a/src/ui/sidebar/AdminSettingsSections.tsx b/src/ui/sidebar/AdminSettingsSections.tsx index fb98f503..d4ec6e08 100644 --- a/src/ui/sidebar/AdminSettingsSections.tsx +++ b/src/ui/sidebar/AdminSettingsSections.tsx @@ -31,6 +31,8 @@ type GeneralSettingsSectionProps = { onToggleAiGloballyEnabled: () => void; aiPrivateEndpoints: string[]; onSaveAiPrivateEndpoints: (hosts: string[]) => void; + notificationPrivateEndpoints: string[]; + onSaveNotificationPrivateEndpoints: (hosts: string[]) => void; handleToggleSessionSharingGloballyEnabled: () => void; allowRegistration: boolean; handleToggleRegistration: () => void; @@ -80,6 +82,8 @@ export function AdminGeneralSettingsSection({ onToggleAiGloballyEnabled, aiPrivateEndpoints, onSaveAiPrivateEndpoints, + notificationPrivateEndpoints, + onSaveNotificationPrivateEndpoints, handleToggleSessionSharingGloballyEnabled, allowRegistration, handleToggleRegistration, @@ -184,6 +188,27 @@ export function AdminGeneralSettingsSection({ /> )} +
+ + {t("admin.notificationPrivateEndpoints")} + + + {t("admin.notificationPrivateEndpointsDesc")} + + + onSaveNotificationPrivateEndpoints( + event.target.value + .split(",") + .map((entry) => entry.trim()) + .filter(Boolean), + ) + } + /> +