From 97ebe23d3bd916157a3f682fbdfd8116f6566cf7 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 23 Aug 2026 09:47:22 +0200 Subject: [PATCH] fix(server): bound every outbound fetch The rule is a timeout, a size cap and boundary validation on anything that leaves the process, and several call sites had only the timeout. A link preview, a maps URL resolve, a wiki fallback and a holiday lookup all read the whole body into memory, so a caller who picks the URL picks how much memory the server spends - a few concurrent multi-hundred-megabyte responses is the whole heap. Two of them also fetched more than their name suggests. resolve-url fell through to a full page GET for any host once it found no coordinates, which makes the server a general-purpose outbound proxy wearing its own IP. The weather URLs took lat/lng straight from the caller into the query string. readCapped/exceedsDeclaredLength collect the pattern transit.service.ts and the plugin installer had each grown separately, so there is one of it now. Callers that only scan the head of a document (og: tags, a coordinate regex) keep the truncated bytes and find fewer fields; callers that must have the whole thing reject it. --- server/src/nest/admin/admin.service.ts | 9 +- server/src/nest/help/wiki.ts | 19 ++- server/src/nest/maps/maps.service.ts | 22 +++- .../notifications/transports/ntfy.service.ts | 6 +- server/src/nest/unsplash/unsplash.service.ts | 15 ++- server/src/nest/vacay/vacay.service.ts | 25 +++- server/src/nest/weather/weather.impl.ts | 53 ++++++-- server/src/utils/cappedFetch.ts | 78 +++++++++++ server/tests/unit/nest/admin.service.test.ts | 25 ++++ server/tests/unit/nest/maps.service.test.ts | 30 +++++ .../unit/nest/notification-transports.test.ts | 27 ++++ .../nest/unsplash/unsplash.service.test.ts | 55 ++++++++ server/tests/unit/nest/vacay.service.test.ts | 35 +++++ .../unit/nest/weather/weather.impl.test.ts | 38 ++++++ server/tests/unit/nest/wiki.test.ts | 21 +++ server/tests/unit/utils/cappedFetch.test.ts | 121 ++++++++++++++++++ 16 files changed, 551 insertions(+), 28 deletions(-) create mode 100644 server/src/utils/cappedFetch.ts create mode 100644 server/tests/unit/utils/cappedFetch.test.ts diff --git a/server/src/nest/admin/admin.service.ts b/server/src/nest/admin/admin.service.ts index 340a841f6..9cb35f9ab 100644 --- a/server/src/nest/admin/admin.service.ts +++ b/server/src/nest/admin/admin.service.ts @@ -421,9 +421,12 @@ export class AdminService { } async getGithubReleases(perPage: string = '10', page: string = '1') { - const data = await this.fetchGithub( - `https://api.github.com/repos/liketrek/TREK/releases?per_page=${perPage}&page=${page}`, - ); + // The query arrives as raw strings from the admin UI; clamp to what the + // GitHub API accepts instead of interpolating them into the URL as given. + const per = Math.min(Math.max(Number.parseInt(perPage, 10) || 10, 1), 100); + const pg = Math.max(Number.parseInt(page, 10) || 1, 1); + const qs = new URLSearchParams({ per_page: String(per), page: String(pg) }); + const data = await this.fetchGithub(`https://api.github.com/repos/liketrek/TREK/releases?${qs}`); return Array.isArray(data) ? data : []; } diff --git a/server/src/nest/help/wiki.ts b/server/src/nest/help/wiki.ts index 757f25905..d35465dba 100644 --- a/server/src/nest/help/wiki.ts +++ b/server/src/nest/help/wiki.ts @@ -1,6 +1,7 @@ import { existsSync, promises as fs } from 'fs'; import path from 'path'; import { readEnv } from '../../app-config'; +import { exceedsDeclaredLength, readCapped, readCappedText } from '../../utils/cappedFetch'; /** * In-app Help/Wiki content, sourced from the `wiki/**` directory that ships with @@ -17,6 +18,12 @@ import { readEnv } from '../../app-config'; const REPO = 'liketrek/TREK'; const RAW_BASE = `https://raw.githubusercontent.com/${REPO}/main/wiki`; const TTL_MS = 60 * 60 * 1000; // remote fallback only: refresh from GitHub at most hourly +// Remote fallback only: raw.githubusercontent.com is a third party on the +// request path, so it gets a deadline and a size budget like every other +// outbound client. A page or a screenshot over the budget falls through to the +// stale-cache path instead of being buffered whole. +const WIKI_TIMEOUT_MS = 8000; +const WIKI_MAX_BYTES = 2 * 1024 * 1024; const SLUG_RE = /^[A-Za-z0-9][A-Za-z0-9._-]*$/; /** @@ -78,9 +85,11 @@ async function fetchText(file: string): Promise { try { const res = await fetch(`${RAW_BASE}/${encodeURIComponent(file)}`, { headers: { 'User-Agent': 'TREK-help', Accept: 'text/plain' }, + signal: AbortSignal.timeout(WIKI_TIMEOUT_MS), }); - if (res.ok) { - const text = await res.text(); + if (res.ok && !exceedsDeclaredLength(res, WIKI_MAX_BYTES)) { + const { text, truncated } = await readCappedText(res, WIKI_MAX_BYTES); + if (truncated) throw new Error('wiki page exceeds size limit'); textCache.set(file, { data: text, ts: Date.now() }); return text; } @@ -245,9 +254,11 @@ export async function getWikiAsset(assetPath: string): Promise<{ buf: Buffer; ty try { const res = await fetch(`${RAW_BASE}/${assetPath.split('/').map(encodeURIComponent).join('/')}`, { headers: { 'User-Agent': 'TREK-help' }, + signal: AbortSignal.timeout(WIKI_TIMEOUT_MS), }); - if (res.ok) { - const buf = Buffer.from(await res.arrayBuffer()); + if (res.ok && !exceedsDeclaredLength(res, WIKI_MAX_BYTES)) { + const { bytes: buf, truncated } = await readCapped(res, WIKI_MAX_BYTES); + if (truncated) throw new Error('wiki asset exceeds size limit'); assetCache.set(assetPath, { buf, type, ts: Date.now() }); return { buf, type }; } diff --git a/server/src/nest/maps/maps.service.ts b/server/src/nest/maps/maps.service.ts index 10362a3a9..36f656020 100644 --- a/server/src/nest/maps/maps.service.ts +++ b/server/src/nest/maps/maps.service.ts @@ -9,6 +9,7 @@ import type { } from '@trek/shared'; import { readEnv, getAppUrl } from '../../app-config'; import { safeFetchFollow, SsrfBlockedError } from '../../utils/ssrfGuard'; +import { exceedsDeclaredLength, readCappedText } from '../../utils/cappedFetch'; import { resolveApiKey, type ApiKeySource } from '../settings/instance-api-keys'; // ── Photo cache (disk-backed) ──────────────────────────────────────────────── import { PlacePhotoCacheService } from '../place-photos/place-photo-cache.service'; @@ -336,6 +337,11 @@ interface GooglePlaceDetails extends GooglePlaceResult { // Wikimedia is normally well under a second, but a cold TLS handshake from a // fresh container has been seen at eight. Enrichment answers a live dialog, so // a slow provider is dropped rather than waited out. +// A Google Maps place page is a few hundred KB; the coordinates sit in the +// embedded map data near the top, so two megabytes is plenty and keeps an +// unbounded body out of memory. +const MAX_MAPS_PAGE_BYTES = 2_000_000; + const WIKI_TIMEOUT_MS = 6000; // Tighter than the wiki calls, because this one sits at the FRONT of a chain: @@ -2043,14 +2049,24 @@ export class MapsService { // Still nothing (e.g. a cid page whose final URL lacks coordinates): fetch the // page body once and parse the coordinates out of the embedded map data. - if (!coords) { + // Only Google's own pages get read; the resolved host is what counts, so a + // short link that lands on maps.google.com still qualifies. + let resolvedHost = ''; + try { resolvedHost = new URL(resolvedUrl).hostname; } catch { /* keep the empty host, the branch is skipped */ } + if (!coords && GOOGLE_MAPS_HOSTS.includes(resolvedHost)) { try { const pageRes = await followRedirects(resolvedUrl, { headers: { 'User-Agent': UA }, }); - coords = extractCoords(await pageRes.text()); + if (!exceedsDeclaredLength(pageRes, MAX_MAPS_PAGE_BYTES)) { + // The map data sits near the top of the document, so a truncated read + // still finds the coordinates; an oversized page degrades to the same + // 400 an unparseable one already produced. + const { text } = await readCappedText(pageRes, MAX_MAPS_PAGE_BYTES); + coords = extractCoords(text); + } } catch (err) { - if ((err as { status?: number })?.status === 403) throw err; // SSRF block — surface it + if ((err as { status?: number })?.status === 403) throw err; // SSRF block, surface it // Otherwise fall through to the not-found error below. } } diff --git a/server/src/nest/notifications/transports/ntfy.service.ts b/server/src/nest/notifications/transports/ntfy.service.ts index 95255259b..33905595f 100644 --- a/server/src/nest/notifications/transports/ntfy.service.ts +++ b/server/src/nest/notifications/transports/ntfy.service.ts @@ -51,7 +51,11 @@ export function resolveAdminNtfyUrl(adminCfg: NtfyConfig): string | null { function encodeHeaderValue(value: string): string { for (let i = 0; i < value.length; i++) { - if (value.charCodeAt(i) > 0x7f) { + const code = value.charCodeAt(i); + // Control characters go through the same RFC 2047 wrapper as non-ASCII: a + // trip title with a stray newline in it would otherwise be rejected by + // undici as an invalid header value and drop the notification silently. + if (code > 0x7f || code < 0x20 || code === 0x7f) { return `=?UTF-8?B?${Buffer.from(value, 'utf8').toString('base64')}?=`; } } diff --git a/server/src/nest/unsplash/unsplash.service.ts b/server/src/nest/unsplash/unsplash.service.ts index f3426c1eb..d4864e520 100644 --- a/server/src/nest/unsplash/unsplash.service.ts +++ b/server/src/nest/unsplash/unsplash.service.ts @@ -2,6 +2,7 @@ import { Readable } from 'node:stream'; import { v4 as uuidv4 } from 'uuid'; import { Injectable } from '@nestjs/common'; import { safeFetch } from '../../utils/ssrfGuard'; +import { exceedsDeclaredLength, readCapped } from '../../utils/cappedFetch'; import { resolveApiKey } from '../settings/instance-api-keys'; import { DatabaseService } from '../database/database.service'; import { RuntimeEnvService } from '../app-config/runtime-env.service'; @@ -31,6 +32,9 @@ export interface UnsplashPhoto { const UNSPLASH_IMAGE_HOST = 'images.unsplash.com'; const MAX_COVER_BYTES = 15 * 1024 * 1024; +// Unsplash is a third party on the request path; safeFetch adds no deadline of +// its own, so both the search and the cover download carry one. +const UNSPLASH_TIMEOUT_MS = 10_000; const COVER_EXT_BY_TYPE: Record = { 'image/jpeg': '.jpg', 'image/png': '.png', @@ -86,8 +90,10 @@ export class UnsplashService { Authorization: `Client-ID ${accessKey}`, 'Accept-Version': 'v1', }, + signal: AbortSignal.timeout(UNSPLASH_TIMEOUT_MS), }) : await fetch(`https://unsplash.com/napi/search/photos?${params.toString()}`, { + signal: AbortSignal.timeout(UNSPLASH_TIMEOUT_MS), headers: { 'User-Agent': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:152.0) Gecko/20100101 Firefox/152.0', Accept: '*/*', @@ -144,13 +150,16 @@ export class UnsplashService { */ async saveUnsplashCover(url: string): Promise { if (!this.isUnsplashCoverUrl(url)) throw new Error('Not an Unsplash image URL'); - const res = await safeFetch(url); + const res = await safeFetch(url, { signal: AbortSignal.timeout(UNSPLASH_TIMEOUT_MS) }); if (!res.ok) throw new Error(`Unsplash image download failed (HTTP ${res.status})`); const type = (res.headers.get('content-type') || '').split(';')[0].trim().toLowerCase(); const ext = COVER_EXT_BY_TYPE[type]; if (!ext) throw new Error(`Unsupported cover image type: ${type || 'unknown'}`); - const buf = Buffer.from(await res.arrayBuffer()); - if (buf.byteLength > MAX_COVER_BYTES) throw new Error('Cover image too large'); + // Reject on the declared length and again while streaming: buffering the whole + // body first put the 15MB limit behind the allocation it was meant to prevent. + if (exceedsDeclaredLength(res, MAX_COVER_BYTES)) throw new Error('Cover image too large'); + const { bytes: buf, truncated } = await readCapped(res, MAX_COVER_BYTES); + if (truncated) throw new Error('Cover image too large'); const filename = `${uuidv4()}${ext}`; await this.storage.put('covers', filename, Readable.from(buf)); return filename; diff --git a/server/src/nest/vacay/vacay.service.ts b/server/src/nest/vacay/vacay.service.ts index 5619e8e6b..9d6f8f26d 100644 --- a/server/src/nest/vacay/vacay.service.ts +++ b/server/src/nest/vacay/vacay.service.ts @@ -2,6 +2,7 @@ import { Injectable } from '@nestjs/common'; import { RealtimeService } from '../realtime/realtime.service'; import { DatabaseService } from '../database/database.service'; import { NotificationsService } from '../notifications/notifications.service'; +import { exceedsDeclaredLength } from '../../utils/cappedFetch'; // --------------------------------------------------------------------------- // Types @@ -97,6 +98,13 @@ export interface VacayShare { const CACHE_TTL = 24 * 60 * 60 * 1000; const FETCH_TIMEOUT_MS = 10_000; +// A country's holidays for one year are a few kilobytes; two megabytes means the +// provider is misbehaving and we would rather report the usual error than buffer it. +const MAX_HOLIDAY_BYTES = 2 * 1024 * 1024; +// Both providers key on a 4-digit year and an ISO 3166-1 alpha-2 code. Anything +// else has no business reaching the URL path. +const YEAR_RE = /^\d{4}$/; +const COUNTRY_RE = /^[A-Za-z]{2}$/; // --------------------------------------------------------------------------- // Color palette for auto-assign @@ -455,8 +463,9 @@ export class VacayService { const cached = this.holidayCache.get(cacheKey); let holidays = cached && Date.now() - cached.time < CACHE_TTL ? cached.data as Holiday[] : undefined; if (!holidays) { + if (!COUNTRY_RE.test(country)) continue; const resp = await fetch(`https://date.nager.at/api/v3/PublicHolidays/${year}/${country}`, { signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) }); - if (!resp.ok) continue; + if (!resp.ok || exceedsDeclaredLength(resp, MAX_HOLIDAY_BYTES)) continue; holidays = await resp.json() as Holiday[]; this.holidayCache.set(cacheKey, { data: holidays, time: Date.now() }); } @@ -1222,7 +1231,7 @@ export class VacayService { if (cached && Date.now() - cached.time < CACHE_TTL) return { data: cached.data }; try { const resp = await fetch('https://date.nager.at/api/v3/AvailableCountries', { signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) }); - if (!resp.ok) return { error: 'Failed to fetch countries' }; + if (!resp.ok || exceedsDeclaredLength(resp, MAX_HOLIDAY_BYTES)) return { error: 'Failed to fetch countries' }; const data = await resp.json(); this.holidayCache.set(cacheKey, { data, time: Date.now() }); return { data }; @@ -1232,12 +1241,15 @@ export class VacayService { } async getHolidays(year: string, country: string): Promise<{ data?: unknown; error?: string }> { + // Both segments land in the URL path, so they are checked before the cache + // lookup rather than after it. + if (!YEAR_RE.test(year) || !COUNTRY_RE.test(country)) return { error: 'Failed to fetch holidays' }; const cacheKey = `${year}-${country}`; const cached = this.holidayCache.get(cacheKey); if (cached && Date.now() - cached.time < CACHE_TTL) return { data: cached.data }; try { const resp = await fetch(`https://date.nager.at/api/v3/PublicHolidays/${year}/${country}`, { signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) }); - if (!resp.ok) return { error: 'Failed to fetch holidays' }; + if (!resp.ok || exceedsDeclaredLength(resp, MAX_HOLIDAY_BYTES)) return { error: 'Failed to fetch holidays' }; const data = await resp.json(); this.holidayCache.set(cacheKey, { data, time: Date.now() }); return { data }; @@ -1247,6 +1259,7 @@ export class VacayService { } async getSchoolHolidayRegions(country: string, language = 'EN'): Promise<{ data?: unknown; error?: string }> { + if (!COUNTRY_RE.test(country)) return { error: 'Failed to fetch school holiday regions' }; const normalizedLanguage = String(language || 'EN').slice(0, 2).toUpperCase(); const cacheKey = `school-regions-${country}-${normalizedLanguage}`; const cached = this.holidayCache.get(cacheKey); @@ -1257,6 +1270,9 @@ export class VacayService { fetch(`https://openholidaysapi.org/Subdivisions?countryIsoCode=${country}&languageIsoCode=${normalizedLanguage}`, { headers: { accept: 'text/json' }, signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) }), ]); if (!groupsResp.ok || !subdivisionsResp.ok) return { error: 'Failed to fetch school holiday regions' }; + if (exceedsDeclaredLength(groupsResp, MAX_HOLIDAY_BYTES) || exceedsDeclaredLength(subdivisionsResp, MAX_HOLIDAY_BYTES)) { + return { error: 'Failed to fetch school holiday regions' }; + } const data = { groups: await groupsResp.json(), subdivisions: await subdivisionsResp.json(), @@ -1269,6 +1285,7 @@ export class VacayService { } async getSchoolHolidays(year: string, country: string, subdivision?: string | null, language = 'EN', group?: string | null): Promise<{ data?: unknown; error?: string }> { + if (!YEAR_RE.test(year) || !COUNTRY_RE.test(country)) return { error: 'Failed to fetch school holidays' }; const normalizedLanguage = String(language || 'EN').slice(0, 2).toUpperCase(); const normalizedSubdivision = subdivision || ''; const normalizedGroup = group || ''; @@ -1288,7 +1305,7 @@ export class VacayService { headers: { accept: 'text/json' }, signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), }); - if (!resp.ok) return { error: 'Failed to fetch school holidays' }; + if (!resp.ok || exceedsDeclaredLength(resp, MAX_HOLIDAY_BYTES)) return { error: 'Failed to fetch school holidays' }; const data = await resp.json(); this.holidayCache.set(cacheKey, { data, time: Date.now() }); return { data }; diff --git a/server/src/nest/weather/weather.impl.ts b/server/src/nest/weather/weather.impl.ts index 6499dd3a9..c9a98f57d 100644 --- a/server/src/nest/weather/weather.impl.ts +++ b/server/src/nest/weather/weather.impl.ts @@ -1,3 +1,4 @@ +import { exceedsDeclaredLength } from '../../utils/cappedFetch'; // Open-Meteo sits on the request path as a third party: without a deadline a hung // connection holds the caller until the socket gives up on its own. Every other @@ -19,6 +20,34 @@ function hourIndexFromTime(time?: string): number | null { return Number.isInteger(hour) && hour >= 0 && hour <= 23 ? hour : null; } +// A day of hourly series is a few kilobytes; a megabyte means the provider is +// misbehaving, not that the trip got longer. +const MAX_WEATHER_BYTES = 1024 * 1024; + +/** Open-Meteo GET with the shared deadline and the declared-size cap. */ +async function fetchOpenMeteo(url: string): Promise { + const response = await fetch(url, { signal: AbortSignal.timeout(WEATHER_TIMEOUT_MS) }); + if (exceedsDeclaredLength(response, MAX_WEATHER_BYTES)) { + throw new ApiError(502, 'Open-Meteo API error'); + } + return response; +} + +/** + * lat/lng arrive as raw query strings and get interpolated straight into the + * Open-Meteo URL. The MCP tool already coerces them with z.number(); the REST + * route has no contract, so the boundary check lives here and covers both. + */ +function coord(value: string, max: number, label: string): number { + // Number('') and Number(' ') are 0, which would silently geolocate the Gulf + // of Guinea instead of failing, so an empty value is rejected outright. + const n = String(value ?? '').trim() === '' ? NaN : Number(value); + if (!Number.isFinite(n) || Math.abs(n) > max) { + throw new ApiError(400, `Invalid ${label}`); + } + return n; +} + // ── Interfaces ────────────────────────────────────────────────────────── export interface WeatherResult { @@ -212,7 +241,7 @@ async function _getWeatherImpl( // Forecast range (-1 .. +16 days) if (diffDays >= -1 && diffDays <= 16) { const url = `https://api.open-meteo.com/v1/forecast?latitude=${lat}&longitude=${lng}&daily=temperature_2m_max,temperature_2m_min,weathercode&timezone=auto&forecast_days=16`; - const response = await fetch(url, { signal: AbortSignal.timeout(WEATHER_TIMEOUT_MS) }); + const response = await fetchOpenMeteo(url); const data = await response.json() as OpenMeteoForecast; if (!response.ok || data.error) { @@ -250,7 +279,7 @@ async function _getWeatherImpl( ? '&hourly=temperature_2m,weathercode' : ''; const url = `https://archive-api.open-meteo.com/v1/archive?latitude=${lat}&longitude=${lng}&start_date=${dateStr}&end_date=${dateStr}&daily=temperature_2m_max,temperature_2m_min,weathercode,precipitation_sum${hourParam}&timezone=auto`; - const response = await fetch(url, { signal: AbortSignal.timeout(WEATHER_TIMEOUT_MS) }); + const response = await fetchOpenMeteo(url); const data = await response.json() as OpenMeteoForecast; if (!response.ok || data.error) { @@ -307,7 +336,7 @@ async function _getWeatherImpl( const endStr = endDate.toISOString().slice(0, 10); const url = `https://archive-api.open-meteo.com/v1/archive?latitude=${lat}&longitude=${lng}&start_date=${startStr}&end_date=${endStr}&daily=temperature_2m_max,temperature_2m_min,precipitation_sum&timezone=auto`; - const response = await fetch(url, { signal: AbortSignal.timeout(WEATHER_TIMEOUT_MS) }); + const response = await fetchOpenMeteo(url); const data = await response.json() as OpenMeteoForecast; if (!response.ok || data.error) { @@ -360,7 +389,7 @@ async function _getWeatherImpl( if (cached) return cached; const url = `https://api.open-meteo.com/v1/forecast?latitude=${lat}&longitude=${lng}¤t=temperature_2m,weathercode&timezone=auto`; - const response = await fetch(url, { signal: AbortSignal.timeout(WEATHER_TIMEOUT_MS) }); + const response = await fetchOpenMeteo(url); const data = await response.json() as OpenMeteoForecast; if (!response.ok || data.error) { @@ -382,13 +411,15 @@ async function _getWeatherImpl( } export async function getWeather( - lat: string, - lng: string, + rawLat: string, + rawLng: string, date: string | undefined, lang: string, /** HH:MM of the moment being asked about. Only the archive path can use it. */ time?: string, ): Promise { + const lat = String(coord(rawLat, 90, 'latitude')); + const lng = String(coord(rawLng, 180, 'longitude')); const ck = cacheKey(lat, lng, date ? `${date}T${time ?? ''}` : date); const cached = getCached(ck); if (cached) return cached; @@ -433,7 +464,7 @@ async function _getDetailedWeatherImpl( + `&hourly=temperature_2m,precipitation,weathercode,windspeed_10m,relativehumidity_2m` + `&daily=temperature_2m_max,temperature_2m_min,weathercode,precipitation_sum,windspeed_10m_max,sunrise,sunset` + `&timezone=auto`; - const response = await fetch(url, { signal: AbortSignal.timeout(WEATHER_TIMEOUT_MS) }); + const response = await fetchOpenMeteo(url); const data = await response.json() as OpenMeteoForecast; if (!response.ok || data.error) { @@ -496,7 +527,7 @@ async function _getDetailedWeatherImpl( + `&daily=temperature_2m_max,temperature_2m_min,weathercode,sunrise,sunset,precipitation_probability_max,precipitation_sum,windspeed_10m_max` + `&timezone=auto&start_date=${dateStr}&end_date=${dateStr}`; - const response = await fetch(url, { signal: AbortSignal.timeout(WEATHER_TIMEOUT_MS) }); + const response = await fetchOpenMeteo(url); const data = await response.json() as OpenMeteoForecast; if (!response.ok || data.error) { @@ -555,11 +586,13 @@ async function _getDetailedWeatherImpl( } export async function getDetailedWeather( - lat: string, - lng: string, + rawLat: string, + rawLng: string, date: string, lang: string, ): Promise { + const lat = String(coord(rawLat, 90, 'latitude')); + const lng = String(coord(rawLng, 180, 'longitude')); const ck = `detailed_${cacheKey(lat, lng, date)}`; const cached = getCached(ck); if (cached) return cached; diff --git a/server/src/utils/cappedFetch.ts b/server/src/utils/cappedFetch.ts new file mode 100644 index 000000000..89ff4653c --- /dev/null +++ b/server/src/utils/cappedFetch.ts @@ -0,0 +1,78 @@ +/** + * Response size caps for outbound fetches. + * + * Every outbound request has to bound what it buffers, and there are two + * things worth bounding: the length the provider declares (cheap, but a + * chunked response omits it) and the bytes that actually arrive. The + * declared-length check is the pattern transit.service.ts established; the + * streaming read is the one the plugin installer needed for codeload + * archives, lifted here so the other fetch sites can share it. + */ + +/** Minimal shape of the bits of Response these helpers touch (test stubs are partial). */ +interface CappedReader { + read(): Promise<{ done?: boolean; value?: Uint8Array }>; + cancel(): Promise; +} +interface CappedResponse { + headers?: { get(name: string): string | null } | null; + body?: { getReader(): CappedReader } | null; + arrayBuffer?(): Promise; + text?(): Promise; +} + +/** True when the provider declares a body larger than the budget. */ +export function exceedsDeclaredLength(res: CappedResponse, maxBytes: number): boolean { + return Number(res.headers?.get('content-length') ?? 0) > maxBytes; +} + +/** + * Read a body, stopping once maxBytes have arrived. Callers that must reject + * an oversized payload look at `truncated`; callers that only scan the head of + * the document (og: tags, a coordinate regex) can use the partial bytes and + * degrade to whatever they find. Responses without a stream fall back to + * arrayBuffer(), which is post-checked instead. + */ +export async function readCapped(res: CappedResponse, maxBytes: number): Promise<{ bytes: Buffer; truncated: boolean }> { + const reader = res.body?.getReader(); + if (!reader) { + const raw = res.arrayBuffer ? Buffer.from(await res.arrayBuffer()) : Buffer.from(res.text ? await res.text() : ''); + return raw.length > maxBytes + ? { bytes: raw.subarray(0, maxBytes), truncated: true } + : { bytes: raw, truncated: false }; + } + + const chunks: Buffer[] = []; + let received = 0; + let truncated = false; + for (;;) { + const { done, value } = await reader.read(); + if (done || !value) break; + chunks.push(Buffer.from(value)); + received += value.length; + if (received > maxBytes) { + truncated = true; + await reader.cancel().catch(() => {}); + break; + } + } + const bytes = Buffer.concat(chunks); + return { bytes: truncated ? bytes.subarray(0, maxBytes) : bytes, truncated }; +} + +/** + * readCapped for text bodies. Decodes as UTF-8, so a chopped trailing sequence + * comes back as a replacement character rather than as a throw. Without a + * stream it reads text() directly instead of going through readCapped, which + * prefers arrayBuffer(). + */ +export async function readCappedText(res: CappedResponse, maxBytes: number): Promise<{ text: string; truncated: boolean }> { + if (!res.body?.getReader && res.text) { + const raw = Buffer.from(await res.text(), 'utf8'); + return raw.length > maxBytes + ? { text: new TextDecoder('utf-8').decode(raw.subarray(0, maxBytes)), truncated: true } + : { text: raw.toString('utf8'), truncated: false }; + } + const { bytes, truncated } = await readCapped(res, maxBytes); + return { text: new TextDecoder('utf-8').decode(bytes), truncated }; +} diff --git a/server/tests/unit/nest/admin.service.test.ts b/server/tests/unit/nest/admin.service.test.ts index fe84dd6d6..631f6a08f 100644 --- a/server/tests/unit/nest/admin.service.test.ts +++ b/server/tests/unit/nest/admin.service.test.ts @@ -395,6 +395,31 @@ describe('getGithubReleases', () => { expect(result).toHaveLength(2); expect((result as any[])[0].tag_name).toBe('v3.0.0'); }); + + it('ADMIN-SVC-053a — clamps the paging query instead of interpolating it raw', async () => { + const fetchMock = vi.fn().mockResolvedValue({ ok: true, text: async () => '[]' }); + vi.stubGlobal('fetch', fetchMock); + + await getGithubReleases('9999', '0'); + expect(fetchMock.mock.calls[0][0]).toBe( + 'https://api.github.com/repos/liketrek/TREK/releases?per_page=100&page=1', + ); + + await getGithubReleases('10&per_page=999', 'abc'); + expect(fetchMock.mock.calls[1][0]).toBe( + 'https://api.github.com/repos/liketrek/TREK/releases?per_page=10&page=1', + ); + }); + + it('ADMIN-SVC-053b — round-trips the paging the admin UI actually sends', async () => { + const fetchMock = vi.fn().mockResolvedValue({ ok: true, text: async () => '[]' }); + vi.stubGlobal('fetch', fetchMock); + + await getGithubReleases('20', '2'); + expect(fetchMock.mock.calls[0][0]).toBe( + 'https://api.github.com/repos/liketrek/TREK/releases?per_page=20&page=2', + ); + }); }); // ── checkVersion ────────────────────────────────────────────────────────────── diff --git a/server/tests/unit/nest/maps.service.test.ts b/server/tests/unit/nest/maps.service.test.ts index fecd1ab6c..474a2324f 100644 --- a/server/tests/unit/nest/maps.service.test.ts +++ b/server/tests/unit/nest/maps.service.test.ts @@ -616,6 +616,36 @@ describe('resolveGoogleMapsUrl coordinate extraction (ReDoS guards)', () => { expect(result.lng).toBeCloseTo(-74.0445, 3); }); + it('MAPS-CID-003: does not read the page body of a non-Google resolved URL', async () => { + // The body branch used to run for any host, turning resolve-url into an + // authenticated outbound GET primitive. + const fetchMock = vi.fn(async (u: string) => { + if (u.includes('nominatim')) { + return { ok: true, json: async () => ({ display_name: 'x', name: null, address: {} }) }; + } + if (u.includes('goo.gl')) return { url: 'https://internal.example.com/status' }; + return { url: 'https://internal.example.com/status', text: async () => 'x!3d40.6892!4d-74.0445y' }; + }); + vi.stubGlobal('fetch', fetchMock); + + await expect(svc.resolveGoogleMapsUrl('https://goo.gl/maps/abc123')).rejects.toMatchObject({ status: 400 }); + // Only the redirect-following call went out; the page body was never fetched. + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('MAPS-CID-004: skips a page body that declares more than the size cap', async () => { + const fetchMock = vi.fn(async (u: string) => { + if (u.includes('cid=')) return { url: 'https://www.google.com/maps/place/Somewhere' }; + return { + url: 'https://www.google.com/maps/place/Somewhere', + headers: { get: (h: string) => (h === 'content-length' ? String(50_000_000) : null) }, + text: async () => 'x!3d40.6892!4d-74.0445y', + }; + }); + vi.stubGlobal('fetch', fetchMock); + await expect(svc.resolveGoogleMapsUrl('https://www.google.com/maps?cid=999')).rejects.toMatchObject({ status: 400 }); + }); + it('MAPS-024 (ReDoS): /@(-?\\d+\\.?\\d*),(-?\\d+\\.?\\d*)/ on adversarial input < 500ms of CPU', () => { const adversarial = '/@' + '1'.repeat(10000) + '.'; expect(cpuMillis(() => { adversarial.match(/@(-?\d+\.?\d*),(-?\d+\.?\d*)/); })).toBeLessThan(500); diff --git a/server/tests/unit/nest/notification-transports.test.ts b/server/tests/unit/nest/notification-transports.test.ts index 501782542..ca08abb14 100644 --- a/server/tests/unit/nest/notification-transports.test.ts +++ b/server/tests/unit/nest/notification-transports.test.ts @@ -486,6 +486,33 @@ describe('sendNtfy', () => { expect(calledOpts.headers['Tags']).toBeUndefined(); // empty tags = no header }); + it('NTFY-009a — a title carrying CR/LF is RFC 2047 encoded instead of reaching the header raw', async () => { + // undici rejects a header value with a control character, so a trip titled + // with a stray newline used to drop the notification for every recipient. + const mockFetch = globalThis.fetch as unknown as ReturnType; + mockFetch.mockResolvedValueOnce({ ok: true, text: async () => '' } as never); + + const result = await sendNtfy(ntfyUrl, null, { ...payload, title: 'Trip\r\nX-Injected: 1' }); + + expect(result).toBe(true); + const [, calledOpts] = mockFetch.mock.calls[0]; + const encoded = calledOpts.headers['Title'] as string; + expect(encoded).toMatch(/^=\?UTF-8\?B\?/); + expect(encoded).not.toContain('\n'); + const b64 = encoded.replace(/^=\?UTF-8\?B\?/, '').replace(/\?=$/, ''); + expect(Buffer.from(b64, 'base64').toString('utf8')).toBe('Trip\r\nX-Injected: 1'); + }); + + it('NTFY-009b — a plain ASCII title is still sent verbatim', async () => { + const mockFetch = globalThis.fetch as unknown as ReturnType; + mockFetch.mockResolvedValueOnce({ ok: true, text: async () => '' } as never); + + await sendNtfy(ntfyUrl, null, { ...payload, title: 'Rome 2026 - day 3' }); + + const [, calledOpts] = mockFetch.mock.calls[0]; + expect(calledOpts.headers['Title']).toBe('Rome 2026 - day 3'); + }); + it('NTFY-009 — title with non-Latin-1 chars is RFC 2047 encoded', async () => { const mockFetch = globalThis.fetch as unknown as ReturnType; mockFetch.mockResolvedValueOnce({ ok: true, text: async () => '' } as never); diff --git a/server/tests/unit/nest/unsplash/unsplash.service.test.ts b/server/tests/unit/nest/unsplash/unsplash.service.test.ts index ed6a15fb7..f75b6e511 100644 --- a/server/tests/unit/nest/unsplash/unsplash.service.test.ts +++ b/server/tests/unit/nest/unsplash/unsplash.service.test.ts @@ -78,6 +78,14 @@ describe('unsplashService.searchUnsplashPhotos', () => { expect(await searchUnsplashPhotos(' ')).toEqual({ error: 'Search query is required', status: 400 }); }); + it('UNSPLASH-002a: gives the search request a deadline so a hung upstream cannot pin the caller', async () => { + const fetchMock = vi.fn().mockResolvedValue(fakeRes({ ok: true, type: 'application/json', json: { results: [] } })); + vi.stubGlobal('fetch', fetchMock); + + await searchUnsplashPhotos('paris'); + expect(fetchMock.mock.calls[0][1].signal).toBeInstanceOf(AbortSignal); + }); + it('UNSPLASH-003: maps a non-ok response to an error', async () => { vi.stubGlobal('fetch', vi.fn().mockResolvedValue(fakeRes({ ok: false, status: 429, type: 'application/json', json: { errors: ['Rate limited'] } }))); expect(await searchUnsplashPhotos('paris')).toEqual({ error: 'Rate limited', status: 429 }); @@ -217,6 +225,53 @@ describe('unsplashService.saveUnsplashCover', () => { expect(writtenCovers()).toEqual([]); }); + it('UNSPLASH-008a: rejects on the declared length before the body is buffered', async () => { + const arrayBuffer = vi.fn(async () => new ArrayBuffer(8)); + safeFetch.mockResolvedValue({ + ok: true, + status: 200, + headers: { + get: (h: string) => (h.toLowerCase() === 'content-type' ? 'image/png' : String(20 * 1024 * 1024)), + }, + arrayBuffer, + } as unknown as Response); + + await expect(saveUnsplashCover('https://images.unsplash.com/photo-1')).rejects.toThrow('Cover image too large'); + expect(arrayBuffer).not.toHaveBeenCalled(); + expect(writtenCovers()).toEqual([]); + }); + + it('UNSPLASH-008b: stops a chunked oversized body mid-stream instead of buffering it', async () => { + let cancelled = false; + let served = 0; + safeFetch.mockResolvedValue({ + ok: true, + status: 200, + headers: { get: (h: string) => (h.toLowerCase() === 'content-type' ? 'image/png' : null) }, + body: { + getReader: () => ({ + read: async () => { + served++; + return { done: false, value: new Uint8Array(4 * 1024 * 1024) }; + }, + cancel: async () => { cancelled = true; }, + }), + }, + } as unknown as Response); + + await expect(saveUnsplashCover('https://images.unsplash.com/photo-1')).rejects.toThrow('Cover image too large'); + expect(cancelled).toBe(true); + // 15MB budget, 4MB chunks: the read stops on the fourth, not after an endless body. + expect(served).toBe(4); + expect(writtenCovers()).toEqual([]); + }); + + it('UNSPLASH-008c: passes a deadline through safeFetch', async () => { + safeFetch.mockResolvedValue(fakeRes({ ok: true, type: 'image/jpeg', bytes: 32 })); + await saveUnsplashCover('https://images.unsplash.com/photo-1'); + expect(safeFetch.mock.calls[0][1]).toEqual(expect.objectContaining({ signal: expect.any(AbortSignal) })); + }); + it('UNSPLASH-009: throws when the download fails', async () => { safeFetch.mockResolvedValue(fakeRes({ ok: false, status: 404 })); await expect(saveUnsplashCover('https://images.unsplash.com/photo-1')).rejects.toThrow(/HTTP 404/); diff --git a/server/tests/unit/nest/vacay.service.test.ts b/server/tests/unit/nest/vacay.service.test.ts index 9b963c2f7..2c037c490 100644 --- a/server/tests/unit/nest/vacay.service.test.ts +++ b/server/tests/unit/nest/vacay.service.test.ts @@ -1518,6 +1518,41 @@ describe('quirk fixes', () => { expect(fetchMock).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ signal: expect.any(AbortSignal) })); }); + it('VACAY-SVC-070a: getHolidays refuses a year or country that is not a plain code', async () => { + const fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + const fresh = new VacayService(new DatabaseService(testDb), new RealtimeService(), notificationsStub()); + + for (const [year, country] of [['../../..', 'DE'], ['2026', 'DE/../../x'], ['20xx', 'DE'], ['2026', 'DEU']]) { + expect(await fresh.getHolidays(year, country)).toEqual({ error: 'Failed to fetch holidays' }); + } + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('VACAY-SVC-070b: getSchoolHolidayRegions refuses a country that is not an alpha-2 code', async () => { + const fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + const fresh = new VacayService(new DatabaseService(testDb), new RealtimeService(), notificationsStub()); + + expect(await fresh.getSchoolHolidayRegions('DE&countryIsoCode=FR')).toEqual({ + error: 'Failed to fetch school holiday regions', + }); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('VACAY-SVC-070c: a provider body over the size cap reads as the usual fetch error', async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + headers: { get: (h: string) => (h === 'content-length' ? String(50 * 1024 * 1024) : null) }, + json: async () => [{ date: '2026-01-01' }], + }); + vi.stubGlobal('fetch', fetchMock); + const fresh = new VacayService(new DatabaseService(testDb), new RealtimeService(), notificationsStub()); + + expect(await fresh.getCountries()).toEqual({ error: 'Failed to fetch countries' }); + expect(await fresh.getHolidays('2026', 'DE')).toEqual({ error: 'Failed to fetch holidays' }); + }); + it('VACAY-SVC-071: applyHolidayCalendars honors the cache TTL', async () => { const { plan } = setupUserWithPlan(); testDb.prepare('UPDATE vacay_plans SET holidays_enabled = 1 WHERE id = ?').run(plan.id); diff --git a/server/tests/unit/nest/weather/weather.impl.test.ts b/server/tests/unit/nest/weather/weather.impl.test.ts index eb25cc27d..f3f8eb0f7 100644 --- a/server/tests/unit/nest/weather/weather.impl.test.ts +++ b/server/tests/unit/nest/weather/weather.impl.test.ts @@ -830,3 +830,41 @@ describe('getWeather error paths', () => { expect(fetch).toHaveBeenCalledTimes(2); }); }); + +// ── coordinate boundary validation ─────────────────────────────────────────── + +describe('coordinate validation', () => { + beforeEach(() => { + vi.mocked(fetch).mockReset(); + }); + + it.each([ + ['not-a-number', '10'], + ['', '10'], + ['91', '10'], + ['10', '181'], + ['10', '&forecast_days=16&latitude=0'], + ])('WEATHER-COORD-001: getWeather rejects lat=%s lng=%s with a 400 before any fetch', async (lat, lng) => { + await expect(getWeather(lat, lng, undefined, 'en')).rejects.toMatchObject({ status: 400 }); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('WEATHER-COORD-002: getDetailedWeather rejects an out-of-range latitude', async () => { + await expect(getDetailedWeather('120', '10', dateOffset(2), 'en')).rejects.toMatchObject({ status: 400 }); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('WEATHER-COORD-003: a body over the declared size cap is a 502, not a parsed answer', async () => { + const date = dateOffset(5); + const body = { + daily: { time: [date], temperature_2m_max: [20], temperature_2m_min: [10], weathercode: [0] }, + }; + const oversized = { + ...mockResponse(body), + headers: { get: (h: string) => (h === 'content-length' ? String(5 * 1024 * 1024) : null) }, + } as unknown as Response; + vi.mocked(fetch).mockResolvedValue(oversized); + + await expect(getWeather('42.40', '9.41', date, 'en')).rejects.toMatchObject({ status: 502 }); + }); +}); diff --git a/server/tests/unit/nest/wiki.test.ts b/server/tests/unit/nest/wiki.test.ts index 99b212130..bd16f0aa2 100644 --- a/server/tests/unit/nest/wiki.test.ts +++ b/server/tests/unit/nest/wiki.test.ts @@ -175,6 +175,27 @@ describe('wiki — GitHub fallback when no wiki is on disk', () => { expect(after.title).toBe(before.title); }); + it('passes a deadline to every GitHub request', async () => { + const wiki = await loadWiki(MISSING_WIKI); + fetchSpy.mockResolvedValue(ok('# Timed')); + + await wiki.getWikiPage('Home'); + + expect(fetchSpy.mock.calls[0][1].signal).toBeInstanceOf(AbortSignal); + }); + + it('falls through to WikiNotFound when GitHub declares a body over the cap', async () => { + const wiki = await loadWiki(MISSING_WIKI); + fetchSpy.mockResolvedValue({ + ok: true, + status: 200, + headers: { get: (h: string) => (h === 'content-length' ? String(5 * 1024 * 1024) : null) }, + text: async () => '# Huge', + } as unknown as Response); + + await expect(wiki.getWikiPage('Home')).rejects.toBeInstanceOf(wiki.WikiNotFound); + }); + it('404s when GitHub is unreachable and nothing is cached', async () => { const wiki = await loadWiki(MISSING_WIKI); fetchSpy.mockRejectedValue(new Error('network down')); diff --git a/server/tests/unit/utils/cappedFetch.test.ts b/server/tests/unit/utils/cappedFetch.test.ts new file mode 100644 index 000000000..e6a2c77ec --- /dev/null +++ b/server/tests/unit/utils/cappedFetch.test.ts @@ -0,0 +1,121 @@ +import { describe, it, expect } from 'vitest'; +import { exceedsDeclaredLength, readCapped, readCappedText } from '../../../src/utils/cappedFetch'; + +/** Response stub that streams `chunks` through a reader, tracking cancellation. */ +function streamed(chunks: string[]) { + let i = 0; + const state = { cancelled: false }; + return { + state, + res: { + headers: { get: () => null }, + body: { + getReader: () => ({ + read: async () => + i < chunks.length + ? { done: false, value: new TextEncoder().encode(chunks[i++]) } + : { done: true, value: undefined as unknown as Uint8Array }, + cancel: async () => { state.cancelled = true; }, + }), + }, + }, + }; +} + +describe('exceedsDeclaredLength', () => { + it('CAP-001: is true when content-length is over the budget', () => { + const res = { headers: { get: () => '2048' } }; + expect(exceedsDeclaredLength(res, 1024)).toBe(true); + }); + + it('CAP-002: is false when content-length is missing or under the budget', () => { + expect(exceedsDeclaredLength({ headers: { get: () => '512' } }, 1024)).toBe(false); + expect(exceedsDeclaredLength({ headers: { get: () => null } }, 1024)).toBe(false); + expect(exceedsDeclaredLength({}, 1024)).toBe(false); + }); +}); + +describe('readCapped', () => { + it('CAP-003: returns the whole body when it fits', async () => { + const { res, state } = streamed(['hello ', 'world']); + const { bytes, truncated } = await readCapped(res, 1024); + expect(bytes.toString('utf8')).toBe('hello world'); + expect(truncated).toBe(false); + expect(state.cancelled).toBe(false); + }); + + it('CAP-004: stops at the budget and cancels the reader when the body is oversized', async () => { + const { res, state } = streamed(['aaaa', 'bbbb', 'cccc']); + const { bytes, truncated } = await readCapped(res, 6); + expect(bytes.length).toBe(6); + expect(bytes.toString('utf8')).toBe('aaaabb'); + expect(truncated).toBe(true); + expect(state.cancelled).toBe(true); + }); + + it('CAP-005: survives a reader whose cancel rejects', async () => { + const { res } = streamed(['aaaa', 'bbbb']); + res.body.getReader = ((orig) => () => ({ ...orig(), cancel: async () => { throw new Error('nope'); } }))( + res.body.getReader, + ); + const { truncated } = await readCapped(res, 2); + expect(truncated).toBe(true); + }); + + it('CAP-006: falls back to arrayBuffer when the response exposes no stream', async () => { + const res = { arrayBuffer: async () => new TextEncoder().encode('body').buffer }; + const { bytes, truncated } = await readCapped(res, 1024); + expect(bytes.toString('utf8')).toBe('body'); + expect(truncated).toBe(false); + }); + + it('CAP-007: truncates the arrayBuffer fallback too', async () => { + const res = { arrayBuffer: async () => new TextEncoder().encode('abcdef').buffer }; + const { bytes, truncated } = await readCapped(res, 3); + expect(bytes.toString('utf8')).toBe('abc'); + expect(truncated).toBe(true); + }); + + it('CAP-008: falls back to text() when there is neither a stream nor arrayBuffer', async () => { + const { bytes } = await readCapped({ text: async () => 'from text' }, 1024); + expect(bytes.toString('utf8')).toBe('from text'); + }); + + it('CAP-009: yields an empty body for a stub with no reader at all', async () => { + const { bytes, truncated } = await readCapped({}, 1024); + expect(bytes.length).toBe(0); + expect(truncated).toBe(false); + }); +}); + +describe('readCappedText', () => { + it('CAP-010: decodes the collected bytes as UTF-8', async () => { + const { res } = streamed(['', 'Hallo Grüße', '']); + const { text, truncated } = await readCappedText(res, 1024); + expect(text).toBe('Hallo Grüße'); + expect(truncated).toBe(false); + }); + + it('CAP-011a: prefers text() over arrayBuffer() when there is no stream', async () => { + const res = { + text: async () => '# markdown', + arrayBuffer: async () => new ArrayBuffer(4), + }; + const { text } = await readCappedText(res, 1024); + expect(text).toBe('# markdown'); + }); + + it('CAP-011b: truncates the text() fallback at the budget', async () => { + const { text, truncated } = await readCappedText({ text: async () => 'abcdef' }, 3); + expect(text).toBe('abc'); + expect(truncated).toBe(true); + }); + + it('CAP-011: reports truncation and still returns the head', async () => { + const { res } = streamed(['x', 'z'.repeat(100)]); + const { text, truncated } = await readCappedText(res, 20); + expect(truncated).toBe(true); + expect(text.startsWith('x')).toBe(true); + expect(text.length).toBe(20); + }); +});