diff --git a/charts/trek/templates/configmap.yaml b/charts/trek/templates/configmap.yaml index 62693a5a..5cecc9d8 100644 --- a/charts/trek/templates/configmap.yaml +++ b/charts/trek/templates/configmap.yaml @@ -70,3 +70,9 @@ data: {{- if .Values.env.MCP_RATE_LIMIT }} MCP_RATE_LIMIT: {{ .Values.env.MCP_RATE_LIMIT | quote }} {{- end }} + {{- if .Values.env.OVERPASS_URL }} + OVERPASS_URL: {{ .Values.env.OVERPASS_URL | quote }} + {{- end }} + {{- if .Values.env.OVERPASS_TIMEOUT_MS }} + OVERPASS_TIMEOUT_MS: {{ .Values.env.OVERPASS_TIMEOUT_MS | quote }} + {{- end }} diff --git a/charts/trek/values.yaml b/charts/trek/values.yaml index 3143899f..a87a4665 100644 --- a/charts/trek/values.yaml +++ b/charts/trek/values.yaml @@ -67,6 +67,12 @@ env: # Max MCP API requests per user per minute. Defaults to 300. # MCP_MAX_SESSION_PER_USER: "20" # Max concurrent MCP sessions per user. Defaults to 20. + # OVERPASS_URL: "" + # Custom Overpass endpoint(s) for the map POI "explore" search, comma-separated. When set, REPLACES the bundled + # public mirrors — point it at an internal/self-hosted Overpass instance when the public mirrors are unreachable + # from the cluster (e.g. locked-down egress). Non-http(s) entries are ignored. + # OVERPASS_TIMEOUT_MS: "12000" + # Per-endpoint timeout (ms) for Overpass POI requests. Raise it for a slow self-hosted Overpass instance. Defaults to 12000. # Secret environment variables stored in a Kubernetes Secret. diff --git a/server/.env.example b/server/.env.example index c31a2956..06921fbb 100644 --- a/server/.env.example +++ b/server/.env.example @@ -40,6 +40,9 @@ DEMO_MODE=false # Demo mode - resets data hourly # MCP_RATE_LIMIT=300 # Max MCP API requests per user per minute (default: 300) # MCP_MAX_SESSION_PER_USER=20 # Max concurrent MCP sessions per user (default: 20) +# OVERPASS_URL= # Custom Overpass endpoint(s) for the map POI "explore" search, comma-separated. When set, REPLACES the bundled public mirrors — point it at an internal/self-hosted Overpass instance when the public ones are unreachable from your network. Non-http(s) entries are ignored. If you don't self-host Overpass but the public mirrors throttle you, setting APP_URL also gives outbound requests a unique User-Agent the mirrors rate-limit less. +# OVERPASS_TIMEOUT_MS=12000 # Per-endpoint timeout (ms) for Overpass POI requests; slower endpoints are abandoned so a faster mirror wins. Raise it for a slow self-hosted instance. (default: 12000) + # Initial admin account — only used on first boot when no users exist yet. # If both are set the admin account is created with these credentials. # If either is omitted a random password is generated and printed to the server log. diff --git a/server/src/services/mapsService.ts b/server/src/services/mapsService.ts index d28998fc..f050fc6e 100644 --- a/server/src/services/mapsService.ts +++ b/server/src/services/mapsService.ts @@ -68,17 +68,17 @@ interface GooglePlaceDetails extends GooglePlaceResult { // ── Constants ──────────────────────────────────────────────────────────────── -const USER_AGENT = (() => { +// Overpass, Nominatim and Wikimedia all ask that requests carry a User-Agent that +// uniquely identifies the deploying instance — a shared, generic UA gets rate-limited +// and throttled harder (see #1309). When the instance URL is configured we append it; +// getAppUrl()'s bare http://localhost fallback isn't a useful identifier, so we drop it. +export function buildUserAgent(instanceUrl: string | undefined): string { const base = 'TREK Travel Planner (https://github.com/mauriceboe/TREK)'; - const hasInstanceUrl = Boolean(process.env.APP_URL || process.env.ALLOWED_ORIGINS); - if (!hasInstanceUrl) return base; - try { - const url = getAppUrl(); - if (url && !url.startsWith('http://localhost')) return `${base}; ${url}`; - } catch { /* ignore */ } + if (instanceUrl && !instanceUrl.startsWith('http://localhost')) return `${base}; ${instanceUrl}`; return base; -})(); -function userAgent(): string { return USER_AGENT; } +} +// Computed once at load — getAppUrl() reads only env vars, which don't change at runtime. +const UA = buildUserAgent(getAppUrl()); // TREK's internal language codes mostly coincide with valid BCP-47 codes, but a // couple don't: 'br' is Brazilian Portuguese here (BCP-47 'pt-BR'; bare 'br' is @@ -163,7 +163,7 @@ export async function searchNominatim(query: string, lang?: string) { 'accept-language': toApiLang(lang), }); const response = await fetch(`https://nominatim.openstreetmap.org/search?${params}`, { - headers: { 'User-Agent': userAgent() }, + headers: { 'User-Agent': UA }, }); if (!response.ok) { const text = await response.text().catch(() => ''); @@ -198,7 +198,7 @@ export async function lookupNominatim(osmType: string, osmId: string, lang?: str }); try { const res = await fetch(`https://nominatim.openstreetmap.org/lookup?${params}`, { - headers: { 'User-Agent': userAgent() }, + headers: { 'User-Agent': UA }, }); if (!res.ok) return null; const data = await res.json() as NominatimResult[]; @@ -223,7 +223,7 @@ export async function fetchOverpassDetails(osmType: string, osmId: string): Prom try { const res = await fetch('https://overpass-api.de/api/interpreter', { method: 'POST', - headers: { 'User-Agent': userAgent(), 'Content-Type': 'application/x-www-form-urlencoded' }, + headers: { 'User-Agent': UA, 'Content-Type': 'application/x-www-form-urlencoded' }, body: `data=${encodeURIComponent(query)}`, }); if (!res.ok) return null; @@ -292,15 +292,39 @@ interface PoiSearchResult { // frequently overloaded (504s) and some community mirrors are unreachable from // certain networks. Racing them means whichever mirror is fastest-reachable for // this user answers, and an overloaded or blocked one never blocks the others. -const OVERPASS_MIRRORS = [ +const DEFAULT_OVERPASS_MIRRORS = [ 'https://overpass-api.de/api/interpreter', 'https://maps.mail.ru/osm/tools/overpass/api/interpreter', 'https://overpass.kumi.systems/api/interpreter', 'https://overpass.private.coffee/api/interpreter', ]; -// Per-mirror cap. Because mirrors race in parallel this is also the worst-case -// total wait before every mirror is given up on and a 502 is returned. -const OVERPASS_TIMEOUT_MS = 30000; + +// Operators behind locked-down egress — or running their own Overpass — can point TREK +// at one or more custom endpoints via OVERPASS_URL (comma-separated). When set it +// REPLACES the public mirrors, so a firewalled cluster never reaches out to them and a +// self-hosted instance is used exclusively (see #1309). Non-http(s) entries are dropped. +export function resolveOverpassEndpoints(raw: string | undefined = process.env.OVERPASS_URL): string[] { + const custom = (raw ?? '') + .split(',') + .map(s => s.trim()) + .filter(s => { + try { const u = new URL(s); return u.protocol === 'http:' || u.protocol === 'https:'; } + catch { return false; } + }); + return custom.length ? custom : DEFAULT_OVERPASS_MIRRORS; +} +const OVERPASS_MIRRORS = resolveOverpassEndpoints(); +// Per-mirror fetch cap. Because mirrors race in parallel this is also the worst-case +// wait before every mirror is given up on and a 502 is returned. Public mirrors answer +// in 1–2s when reachable, so the cap mainly bounds dead/blocked ones; operators with a +// slow self-hosted endpoint can raise it via OVERPASS_TIMEOUT_MS. A non-positive or +// non-numeric value falls back to the default — a 0/negative cap would abort every +// request immediately and 502 the search. +export function resolveOverpassTimeoutMs(raw: string | undefined = process.env.OVERPASS_TIMEOUT_MS): number { + const n = Number(raw); + return Number.isFinite(n) && n > 0 ? n : 12000; +} +const OVERPASS_TIMEOUT_MS = resolveOverpassTimeoutMs(); // Largest viewport side we send to Overpass. A country/continent-sized bbox makes // Overpass scan millions of elements and time out; clamping to a centred window // keeps the query cheap so the explore pill returns fast at ANY zoom level. @@ -329,7 +353,7 @@ async function overpassFetch(query: string): Promise { try { const res = await fetch(url, { method: 'POST', - headers: { 'User-Agent': userAgent(), 'Content-Type': 'application/x-www-form-urlencoded' }, + headers: { 'User-Agent': UA, 'Content-Type': 'application/x-www-form-urlencoded' }, body, signal: ctrl.signal, }); @@ -352,8 +376,15 @@ async function overpassFetch(query: string): Promise { // Promise.any resolves with the first mirror to return valid JSON, and only // rejects (AggregateError) once every mirror has failed. return await Promise.any(OVERPASS_MIRRORS.map(attempt)); - } catch { - throw Object.assign(new Error('Overpass request failed'), { status: 502 }); + } catch (err) { + // Log WHY every endpoint failed (connection refused, aborted/timed out, non-OSM + // body, …) so an operator can tell blocked egress / a firewall from a transiently + // overloaded mirror — otherwise this is a bare 502 with no breadcrumb (see #1309). + const reasons = err instanceof AggregateError + ? err.errors.map(e => (e instanceof Error ? e.message : String(e))).join(' | ') + : (err instanceof Error ? err.message : String(err)); + console.error(`[Overpass] all ${OVERPASS_MIRRORS.length} endpoint(s) failed — ${reasons}`); + throw Object.assign(new Error('Could not reach any Overpass endpoint'), { status: 502 }); } finally { // Cancel the slower/losing requests — we already have (or have given up on) a result. controllers.forEach(c => { try { c.abort(); } catch { /* noop */ } }); @@ -525,7 +556,7 @@ export async function fetchWikimediaPhoto(lat: number, lng: number, name?: strin pilimit: '1', redirects: '1', }); - const res = await fetch(`https://en.wikipedia.org/w/api.php?${searchParams}`, { headers: { 'User-Agent': userAgent() } }); + const res = await fetch(`https://en.wikipedia.org/w/api.php?${searchParams}`, { headers: { 'User-Agent': UA } }); if (res.ok) { const data = await res.json() as { query?: { pages?: Record } }; const pages = data.query?.pages; @@ -554,7 +585,7 @@ export async function fetchWikimediaPhoto(lat: number, lng: number, name?: strin iiurlwidth: '400', }); try { - const res = await fetch(`https://commons.wikimedia.org/w/api.php?${params}`, { headers: { 'User-Agent': userAgent() } }); + const res = await fetch(`https://commons.wikimedia.org/w/api.php?${params}`, { headers: { 'User-Agent': UA } }); if (!res.ok) return null; const data = await res.json() as { query?: { pages?: Record } }; const pages = data.query?.pages; @@ -1003,7 +1034,7 @@ export async function reverseGeocode(lat: string, lng: string, lang?: string): P 'accept-language': toApiLang(lang), }); const response = await fetch(`https://nominatim.openstreetmap.org/reverse?${params}`, { - headers: { 'User-Agent': userAgent() }, + headers: { 'User-Agent': UA }, }); if (!response.ok) return { name: null, address: null }; const data = await response.json() as { name?: string; display_name?: string; address?: Record }; diff --git a/server/tests/unit/services/mapsService.test.ts b/server/tests/unit/services/mapsService.test.ts index 1ae65e34..74902e6d 100644 --- a/server/tests/unit/services/mapsService.test.ts +++ b/server/tests/unit/services/mapsService.test.ts @@ -74,6 +74,10 @@ import { buildOsmDetails, getMapsKey, googleFtidFromMapsUrl, + buildUserAgent, + resolveOverpassEndpoints, + resolveOverpassTimeoutMs, + searchOverpassPois, } from '../../../src/services/mapsService'; afterEach(() => { @@ -1496,3 +1500,96 @@ describe('googleFtidFromMapsUrl', () => { expect(googleFtidFromMapsUrl(null)).toBeNull(); }); }); + +// ── buildUserAgent (instance-specific UA, #1309) ────────────────────────────── + +describe('buildUserAgent', () => { + const base = 'TREK Travel Planner (https://github.com/mauriceboe/TREK)'; + + it('MAPS-094: returns the bare base UA when no instance URL is configured', () => { + expect(buildUserAgent(undefined)).toBe(base); + expect(buildUserAgent('')).toBe(base); + }); + + it('MAPS-095: appends a configured https instance URL so the deployment is identifiable', () => { + expect(buildUserAgent('https://trek.example.org')).toBe(`${base}; https://trek.example.org`); + }); + + it('MAPS-096: drops the http://localhost fallback — it is not a unique identifier', () => { + expect(buildUserAgent('http://localhost:3001')).toBe(base); + }); +}); + +// ── resolveOverpassEndpoints (OVERPASS_URL override, #1309) ──────────────────── + +describe('resolveOverpassEndpoints', () => { + it('MAPS-097: falls back to the public mirrors when OVERPASS_URL is unset/empty', () => { + expect(resolveOverpassEndpoints(undefined).length).toBeGreaterThan(1); + expect(resolveOverpassEndpoints('').length).toBeGreaterThan(1); + expect(resolveOverpassEndpoints(undefined)[0]).toContain('overpass-api.de'); + }); + + it('MAPS-098: a single custom endpoint REPLACES the public mirrors (locked-down egress)', () => { + expect(resolveOverpassEndpoints('https://overpass.internal/api/interpreter')) + .toEqual(['https://overpass.internal/api/interpreter']); + }); + + it('MAPS-099: parses a comma-separated list and trims whitespace', () => { + expect(resolveOverpassEndpoints(' https://a.test/api , http://b.test/api ')) + .toEqual(['https://a.test/api', 'http://b.test/api']); + }); + + it('MAPS-100: drops non-http(s) / malformed entries, keeping the valid ones', () => { + expect(resolveOverpassEndpoints('https://ok.test/api, ftp://no.test, not a url')) + .toEqual(['https://ok.test/api']); + }); + + it('MAPS-101: falls back to the defaults when every custom entry is invalid', () => { + expect(resolveOverpassEndpoints('not a url, ftp://no.test').length).toBeGreaterThan(1); + }); +}); + +// ── resolveOverpassTimeoutMs (OVERPASS_TIMEOUT_MS override, #1309) ───────────── + +describe('resolveOverpassTimeoutMs', () => { + it('MAPS-104: falls back to the 12s default for unset / empty / non-numeric values', () => { + expect(resolveOverpassTimeoutMs(undefined)).toBe(12000); + expect(resolveOverpassTimeoutMs('')).toBe(12000); + expect(resolveOverpassTimeoutMs('abc')).toBe(12000); + }); + + it('MAPS-105: honours a positive numeric override', () => { + expect(resolveOverpassTimeoutMs('30000')).toBe(30000); + }); + + it('MAPS-106: rejects 0, negative and Infinity — a non-positive cap would 502 every search', () => { + expect(resolveOverpassTimeoutMs('0')).toBe(12000); + expect(resolveOverpassTimeoutMs('-5')).toBe(12000); + expect(resolveOverpassTimeoutMs('Infinity')).toBe(12000); + }); +}); + +// ── searchOverpassPois error path (all endpoints down, #1309) ────────────────── + +describe('searchOverpassPois all-endpoints-down', () => { + const bbox = { south: -41.2, west: 146.31, north: -41.16, east: 146.37 }; + + it('MAPS-102: surfaces a 502 with a clear message when every Overpass endpoint fails', async () => { + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('connect ECONNREFUSED'))); + await expect(searchOverpassPois('restaurant', bbox)).rejects.toMatchObject({ + status: 502, + message: 'Could not reach any Overpass endpoint', + }); + errSpy.mockRestore(); + }); + + it('MAPS-103: logs each endpoint failure so an operator can diagnose blocked egress', async () => { + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('connect ECONNREFUSED'))); + await expect(searchOverpassPois('bar', bbox)).rejects.toThrow(); + expect(errSpy).toHaveBeenCalledWith(expect.stringContaining('[Overpass] all')); + expect(errSpy).toHaveBeenCalledWith(expect.stringContaining('ECONNREFUSED')); + errSpy.mockRestore(); + }); +}); diff --git a/wiki/Environment-Variables.md b/wiki/Environment-Variables.md index 32729e16..5c933fc0 100644 --- a/wiki/Environment-Variables.md +++ b/wiki/Environment-Variables.md @@ -214,6 +214,8 @@ when the binary is found, and the Import button in the Reservations panel is hid | Variable | Description | Default | |---------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------------| | `IDEMPOTENCY_TTL_SECONDS` | How long (in seconds) stored idempotency keys are kept before garbage collection. The offline client replays queued mutations with their `X-Idempotency-Key` on reconnect, so this must exceed the longest expected offline window or a replay could create a duplicate. Invalid values silently fall back to the default. | `2592000` (30 days) | +| `OVERPASS_URL` | Custom [Overpass API](https://wiki.openstreetmap.org/wiki/Overpass_API) endpoint(s) used by the map's POI "explore" search, comma-separated. When set it **replaces** the bundled public mirrors — point it at an internal or self-hosted Overpass instance when the public mirrors are unreachable from your network (e.g. firewalled/locked-down egress in a Kubernetes cluster). Entries that aren't valid `http(s)` URLs are ignored. If you don't run your own Overpass but the public mirrors throttle TREK, first make sure `APP_URL` (or `ALLOWED_ORIGINS`) is set: that alone gives outbound Overpass/Nominatim requests a unique User-Agent, which the public mirrors rate-limit far less. | bundled public mirrors | +| `OVERPASS_TIMEOUT_MS` | Per-endpoint timeout (in milliseconds) for Overpass POI requests. Endpoints race in parallel and one that hasn't answered within this window is abandoned so a faster mirror can win. Raise it if you run a slow self-hosted Overpass instance. Invalid values fall back to the default. | `12000` | ---