From be7187cea7ed771e7461eb2bc41a3310ecb9b4ff Mon Sep 17 00:00:00 2001 From: daikaginza Date: Mon, 21 Sep 2026 10:02:19 +0700 Subject: [PATCH] fix(server): bound the terrain-heights cache and its request rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every distinct 5-decimal point was a permanent in-memory entry, a permanent line in an uncapped `.gev-cache/terrain-heights.json`, and a live upstream call. Nothing limited the rate, and coordinates were checked for being finite rather than for being on Earth, so an out-of-range pair was still a valid cache key. Four bounds, each following a pattern this repo already uses: - WGS-84 range is enforced in `parseTerrainPoints`, at the edge, so no later stage has to. Poles and the antimeridian still parse. - The point cache holds at most 50,000 entries and evicts oldest-first, the shape `trackBackfillProxies` uses. Sized from measurement: this checkout's cache file held 6,189 entries in 1.0 MB of real use, so the ceiling is eight times that working set. Terrain does not move, so an evicted point costs one refetch and never a wrong answer. - A cache file above 24 MB is not read back, mirroring `rocketLaunchesProxy`. An unmeasurable file is not treated as an oversized one: without stat, readFile decides, exactly as before. - The route carries an always-on limiter of 90/min per client, 270 global — the `/api/overpass` band, and an order of magnitude above measured demand (a full test:track run drove 10 requests, peak 8/min). The ceilings are constructor options so they can be exercised as behaviour rather than asserted as constants; production passes none. They are destructured in the body because `proxyErrorResponses.test.mjs` slices these proxies to the first column-zero closing brace. --- CHANGELOG.md | 11 ++ docs/CURRENT-STATE.md | 11 ++ scripts/format-scope.json | 3 +- scripts/package-boundaries.json | 4 +- server/providers/terrain.js | 93 +++++++++++- src/data/terrainHeightsProxy.js | 8 ++ src/proxyErrorResponses.test.mjs | 4 + src/terrainHeightsBounds.test.mjs | 232 ++++++++++++++++++++++++++++++ 8 files changed, 360 insertions(+), 6 deletions(-) create mode 100644 src/terrainHeightsBounds.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 29dbedfaa..f59c0c97a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +- Bound what a caller can grow through `/api/terrain/heights`. Every distinct + 5-decimal point was a permanent in-memory entry, a permanent line in an + uncapped `.gev-cache/terrain-heights.json`, and a live upstream call, with + nothing limiting the rate and only a finite-number check on the coordinates. + Coordinates outside WGS-84 range are now rejected with `400`, the point cache + holds at most 50,000 entries and evicts oldest-first, a cache file above + 24 MB is not read back, and the route is limited to 90 requests/minute per + client (270 global). Evicting a point costs one refetch, never a wrong + answer, since terrain does not move. Ordinary use is unaffected: a full + `test:track` run drives 10 requests, peaking at 8/min. + - CCTV cameras whose bearing is a guess now say so. Packs mark bearings derived from a hash of the camera id as `headingConfidence: 'low'`, but nothing read the flag, so roughly 70% of a default catalog rendered like surveyed facings. The HUD diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index fdb42fc0c..adf2e7995 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -2064,6 +2064,17 @@ down | auth-failed` (plus the unchanged `missing-key`/`unsupported`) with > tunings. Ambient overlap includes each sprite's own rendered extent. The > gated AIS, CCTV, and satellite passes retain an active-emphasis count so a > settled dim contact always completes its release after tracking ends. +> - **Terrain-height bounds:** because every distinct 5-decimal point is its own +> cache key, its own line in `.gev-cache/terrain-heights.json`, and its own +> upstream call, four limits bound what a caller can grow. Coordinates outside +> WGS-84 range are rejected with `400` at parse (finite is not the same as on +> Earth); the point cache holds at most 50,000 entries, evicting oldest-first +> — terrain does not move, so an evicted point costs one refetch and never a +> wrong answer; a cache file above 24 MB is not read back into memory (the +> ceiling `rocketLaunchesProxy` already applies); and the route carries an +> always-on limiter of 90 requests/minute/client, 270 global. That cap is an +> order of magnitude above measured demand — a full `npm run test:track` run +> drove 10 requests, peaking at 8/min. > - **Terrain-height resilience:** `/api/terrain/heights` caches canonical > 5-decimal points individually, reconstructs reordered/overlapping batches > in exact request order, and refreshes only missing or stale points. Upstream diff --git a/scripts/format-scope.json b/scripts/format-scope.json index eee546dd5..ca43cbbb3 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -839,5 +839,6 @@ "src/ui/imageryBoxTool.test.mjs", "src/ui/recentImagery.test.mjs", "src/layers/recentImagery/testDoubles.mjs", - "scripts/qa-recent-imagery.mjs" + "scripts/qa-recent-imagery.mjs", + "src/terrainHeightsBounds.test.mjs" ] diff --git a/scripts/package-boundaries.json b/scripts/package-boundaries.json index 189192f10..d12fc42ea 100644 --- a/scripts/package-boundaries.json +++ b/scripts/package-boundaries.json @@ -101,7 +101,9 @@ "exports": ["./server/providers/terrain"], "modules": [ "server/providers/terrain.js", - "src/data/terrainHeightsProxy.js" + "src/data/terrainHeightsProxy.js", + "server/providers/common/rate-limit.js", + "src/sources/rateLimit.js" ], "external": [] }, diff --git a/server/providers/terrain.js b/server/providers/terrain.js index e58a63ea5..ca9442db1 100644 --- a/server/providers/terrain.js +++ b/server/providers/terrain.js @@ -8,6 +8,7 @@ import { terrainPointKey, validTerrainResult, } from '../../src/data/terrainHeightsProxy.js'; +import { makeRateLimiter, clientKey } from './common/rate-limit.js'; /** * Re:Earth terrain point-height proxy: batched lon/lat → ellipsoidal height @@ -20,11 +21,27 @@ import { * Only missing/stale points go upstream; the response is rebuilt in exact * request order. Larger requests are chunked sequentially, and one failing * chunk does not discard the chunks that resolved. + * + * Bounds, because every distinct 5dp point is its own key: coordinates must be + * in WGS-84 range (rejected at parse), the point cache has an entry ceiling + * with oldest-first eviction, an over-large cache file is not read back, and + * the route carries an always-on per-client limiter. */ -export function terrainHeightsProxy() { +export function terrainHeightsProxy(options = {}) { + // Destructured in the body, not the signature: `proxyErrorResponses.test.mjs` + // extracts these proxies by slicing to the first closing brace in column + // zero, so a multi-line parameter list would cut the function in half. + // Overridable so the ceilings can be exercised as behaviour rather than + // asserted as constants; production constructs this with no arguments. + const { + cacheMaxEntries = 50_000, + cacheMaxDiskBytes = 24 * 1024 * 1024, + cachePath, + } = options; const TTL_MS = 30 * 24 * 3600_000; - const CACHE_DIR = path.join(process.cwd(), '.gev-cache'); - const CACHE_PATH = path.join(CACHE_DIR, 'terrain-heights.json'); + const CACHE_PATH = + cachePath || path.join(process.cwd(), '.gev-cache', 'terrain-heights.json'); + const CACHE_DIR = path.dirname(CACHE_PATH); // Sized against measured upstream latency, not the documented page cap. // Re:Earth serves 87-186 ms/point depending on load (observed 2026-09-12, // a 2x swing within one hour). 256 points therefore costs 22-48 s and blows @@ -34,9 +51,50 @@ export function terrainHeightsProxy() { // that range. const UPSTREAM_CHUNK = 64; const MAX_POINTS = 2000; + /** + * Entry ceiling for the point cache, and with it the file written from it. + * + * Every distinct 5dp point is a separate key, so without a ceiling the map + * — and the JSON dumped from it verbatim — grow for as long as a caller + * supplies new coordinates. Measured against real use rather than guessed: + * this checkout's `.gev-cache/terrain-heights.json` held 6,189 entries in + * 1.0 MB after ordinary sessions, i.e. ~168 bytes/entry. 50,000 entries is + * eight times that working set and ~8 MB on disk, comfortably inside the + * 24 MB ceiling `rocketLaunchesProxy` already applies to its own cache file. + */ + const CACHE_MAX_ENTRIES = cacheMaxEntries; + /** Refuse to load a cache file larger than this (mirrors rocketLaunchesProxy). */ + const CACHE_MAX_DISK_BYTES = cacheMaxDiskBytes; + /** + * Requests/min/client. Terrain is keyless and free, so this guards the point + * cache and the upstream community service rather than a bill. Sized from + * measurement: across a full `npm run test:track` run — the heaviest + * exercise this app has — the route served 10 requests, peaking at 8/min. + * 90/min is an order of magnitude above that, and the band `/api/overpass` + * and `/api/military-installations` already use. + */ + const RATE_PER_MIN = 90; + const allow = makeRateLimiter({ + windowMs: 60_000, + max: RATE_PER_MIN, + globalMax: RATE_PER_MIN * 3, + }); /** @type {Map} keyed by canonical 5dp lon/lat. */ const mem = new Map(); + + /** + * Drop the oldest entries until the map is back inside its ceiling. + * Oldest-first by fetch time, the same shape `trackBackfillProxies` uses. + * Terrain does not move, so evicting an old point costs one refetch, never + * a wrong answer. + */ + function evictOverflow() { + if (mem.size <= CACHE_MAX_ENTRIES) return; + const byAge = [...mem.entries()].sort((a, b) => a[1].at - b[1].at); + for (let i = 0; i < byAge.length && mem.size > CACHE_MAX_ENTRIES; i += 1) + mem.delete(byAge[i][0]); + } /** @type {Map>>} single-flight per missing-point subset. */ const inflight = new Map(); let diskLoaded = false; @@ -47,6 +105,21 @@ export function terrainHeightsProxy() { if (diskLoaded) return; diskLoaded = true; try { + // A cache file past the ceiling is not read back into memory: loading it + // would reinstate exactly the unbounded map the ceiling exists to stop. + // The next flush rewrites a bounded one. + // + // "Cannot measure it" is not "too large": if stat is unavailable, fall + // through and let readFile decide, so an absent file behaves exactly as + // it did before this guard existed. + let sizeBytes = null; + try { + sizeBytes = (await fsp.stat(CACHE_PATH)).size; + } catch { + sizeBytes = null; + } + if (sizeBytes !== null && sizeBytes > CACHE_MAX_DISK_BYTES) + throw new Error('cache file too large'); const parsed = JSON.parse(await fsp.readFile(CACHE_PATH, 'utf8')); const pointEntries = parsed?.version === 2 && @@ -87,6 +160,7 @@ export function terrainHeightsProxy() { } diskDirty = mem.size > 0; } + evictOverflow(); } catch { /* no disk cache yet */ } @@ -163,6 +237,14 @@ export function terrainHeightsProxy() { res.end(JSON.stringify(bodyObj)); }; try { + if (!allow(clientKey(req))) { + res.writeHead(429, { + 'Content-Type': 'application/json', + 'Retry-After': '10', + }); + res.end(JSON.stringify({ error: 'Rate limit exceeded' })); + return; + } await loadDiskOnce(); const parsedUrl = new URL(req.url || '', 'http://internal'); const rawPoints = parsedUrl.searchParams.get('points'); @@ -187,7 +269,10 @@ export function terrainHeightsProxy() { fetchMissing: fetchMissingSingleFlight, ttlMs: TTL_MS, }); - if (outcome.cacheChanged) diskDirty = true; + if (outcome.cacheChanged) { + evictOverflow(); + diskDirty = true; + } if (outcome.upstreamError) { console.warn( '[terrain-heights-proxy] refresh incomplete' + diff --git a/src/data/terrainHeightsProxy.js b/src/data/terrainHeightsProxy.js index bfb058693..11b8e0037 100644 --- a/src/data/terrainHeightsProxy.js +++ b/src/data/terrainHeightsProxy.js @@ -18,6 +18,13 @@ function defaultSleep(ms) { /** * Parse the proxy's `points=lon,lat;...` query parameter. + * + * Finite is not the same as on Earth. A coordinate outside WGS-84 range can + * never resolve to a height, but it is still a distinct 5dp cache key, so + * accepting it meant an out-of-range point became a permanent cache entry and + * a live upstream call. Range is checked here, at the edge, so no later stage + * has to. + * * @param {string|null|undefined} raw * @returns {Array<[number, number]>|null} */ @@ -36,6 +43,7 @@ export function parseTerrainPoints(raw) { const lon = Number(parts[0]); const lat = Number(parts[1]); if (!Number.isFinite(lon) || !Number.isFinite(lat)) return null; + if (lat < -90 || lat > 90 || lon < -180 || lon > 180) return null; points.push([lon, lat]); } return points; diff --git a/src/proxyErrorResponses.test.mjs b/src/proxyErrorResponses.test.mjs index 2b45a3b9e..9cc1ff9c7 100644 --- a/src/proxyErrorResponses.test.mjs +++ b/src/proxyErrorResponses.test.mjs @@ -3,6 +3,7 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import path from 'node:path'; import { readResponseTextCapped, coalesceProxyRequest } from './sources/httpBody.js'; +import { makeRateLimiter, clientKey } from '../server/providers/common/rate-limit.js'; const source = ['local.js', 'common/http.js', 'aircraft/enrichment.js', 'terrain.js', 'space/celestrak.js', 'space/launch-library.js', '../../src/data/spaceProviderRequests.js'] .map(file => readFileSync(new URL(`../server/providers/${file}`, import.meta.url), 'utf8')) @@ -22,6 +23,9 @@ function fixture(name, overrides = {}, preview = false) { const logs = []; const deps = { readResponseTextCapped, coalesceProxyRequest, + // The rate-limited proxies reach for these; give them the real ones so a + // fixture request is throttled exactly as production would be. + makeRateLimiter, clientKey, path, process: { cwd: () => '/fixture', env: {} }, fsp: { readFile: async () => { throw new Error('cache absent'); }, diff --git a/src/terrainHeightsBounds.test.mjs b/src/terrainHeightsBounds.test.mjs new file mode 100644 index 000000000..1a0c80290 --- /dev/null +++ b/src/terrainHeightsBounds.test.mjs @@ -0,0 +1,232 @@ +// TERRAIN HEIGHTS — the bounds on a cache keyed by caller-chosen coordinates. +// +// Every distinct 5dp lon/lat is its own cache key, its own permanent line in +// `.gev-cache/terrain-heights.json`, and its own upstream call. Four things +// bound that: WGS-84 range at parse, an entry ceiling on the map, a size +// ceiling on the file that is read back, and a per-client request limiter. +// +// These cases drive the real exported route handler. `fetch` is stubbed per +// test and restored after, so a case that reaches upstream fails loudly. +// +// Run with: npm test +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { parseTerrainPoints } from './data/terrainHeightsProxy.js'; +import { terrainHeightsProxy } from '../server/providers/terrain.js'; + +function install(plugin) { + const routes = new Map(); + plugin.configureServer({ + middlewares: { + use(path, handler) { + routes.set(path, handler); + }, + }, + }); + return routes; +} + +function request(handler, { url = '/', remoteAddress = '127.0.0.1' } = {}) { + return new Promise((resolve, reject) => { + const headers = new Map(); + const req = { method: 'GET', url, headers: {}, socket: { remoteAddress } }; + const res = { + statusCode: 200, + headersSent: false, + setHeader(name, value) { + headers.set(String(name).toLowerCase(), String(value)); + }, + writeHead(status, hdrs = {}) { + this.statusCode = status; + this.headersSent = true; + for (const [name, value] of Object.entries(hdrs)) + headers.set(String(name).toLowerCase(), String(value)); + }, + end(body = '') { + let parsed = null; + try { + parsed = body ? JSON.parse(String(body)) : null; + } catch { + parsed = String(body); + } + resolve({ + statusCode: this.statusCode, + headers: Object.fromEntries(headers), + body: parsed, + }); + }, + }; + Promise.resolve(handler(req, res)).catch(reject); + }); +} + +function stubFetch(t, impl) { + const original = globalThis.fetch; + globalThis.fetch = impl; + t.after(() => { + globalThis.fetch = original; + }); +} + +/** + * Answer like Re:Earth does: one `{ellipsoid}` per requested point, in order. + * Records which point strings were actually asked for, so a test can assert on + * cache misses rather than on internal state. + */ +function stubTerrainUpstream(t) { + const asked = []; + stubFetch(t, async (url) => { + const points = decodeURIComponent( + new URL(String(url)).searchParams.get('points') || '', + ) + .split(';') + .filter(Boolean); + asked.push(...points); + return Response.json({ results: points.map(() => ({ ellipsoid: 150 })) }); + }); + return asked; +} + +test('a coordinate off the globe is not a cache key', () => { + // Finite but not on Earth — the old guard accepted every one of these. + for (const raw of [ + '0,91', + '0,-91', + '181,0', + '-181,0', + '1e9,1e9', + '0,90.00001', + ]) + assert.equal(parseTerrainPoints(raw), null, raw); + + // The poles and the antimeridian are real places and must still parse. + for (const raw of ['0,90', '0,-90', '180,0', '-180,0', '-97.74,30.26']) + assert.ok(parseTerrainPoints(raw), raw); +}); + +test('an out-of-range point is refused before it reaches upstream or the cache', async (t) => { + let upstreamCalls = 0; + stubFetch(t, async () => { + upstreamCalls += 1; + return new Response('[]', { status: 200 }); + }); + const route = install(terrainHeightsProxy()).get('/api/terrain/heights'); + + const refused = await request(route, { url: '/?points=0,91' }); + assert.equal(refused.statusCode, 400); + assert.equal( + upstreamCalls, + 0, + 'an impossible coordinate must not cost an upstream call', + ); +}); + +test('one out-of-range point rejects the batch it is hidden in', async (t) => { + stubFetch(t, async () => new Response('[]', { status: 200 })); + const route = install(terrainHeightsProxy()).get('/api/terrain/heights'); + const mixed = await request(route, { + url: '/?points=-97.74,30.26;0,999;-97.75,30.27', + }); + assert.equal(mixed.statusCode, 400); +}); + +test('the point cache stops growing, and an evicted point is simply refetched', async (t) => { + const asked = stubTerrainUpstream(t); + // A ceiling of 3 exercises the same code path 50,000 does, in four requests. + const route = install(terrainHeightsProxy({ cacheMaxEntries: 3 })).get( + '/api/terrain/heights', + ); + + const point = (i) => `-97.7${i}000,30.26000`; + for (let i = 1; i <= 4; i += 1) + await request(route, { url: `/?points=${point(i)}` }); + assert.equal(asked.length, 4, 'four distinct points, four upstream lookups'); + + // The newest point is still cached... + await request(route, { url: `/?points=${point(4)}` }); + assert.equal(asked.length, 4, 'a cached point costs nothing'); + + // ...and the oldest was evicted, so it costs one refetch — never a wrong + // answer, because terrain does not move. + await request(route, { url: `/?points=${point(1)}` }); + assert.equal(asked.length, 5, 'the evicted point was asked for again'); + assert.equal(asked[4], point(1)); +}); + +test('an over-large cache file is not read back into memory', async (t) => { + const asked = stubTerrainUpstream(t); + const point = '-97.74000,30.26000'; + const dir = await mkdtemp(path.join(tmpdir(), 'gev-terrain-')); + const cachePath = path.join(dir, 'terrain-heights.json'); + t.after(() => rm(dir, { recursive: true, force: true })); + + // A cache file holding the answer, but larger than the ceiling allows. + const padded = { + version: 2, + points: { [point]: { at: Date.now(), result: { ellipsoid: 150 } } }, + _pad: 'x'.repeat(4096), + }; + await writeFile(cachePath, JSON.stringify(padded), 'utf8'); + + const route = install( + terrainHeightsProxy({ cachePath, cacheMaxDiskBytes: 1024 }), + ).get('/api/terrain/heights'); + await request(route, { url: `/?points=${point}` }); + assert.deepEqual( + asked, + [point], + 'the oversized file was skipped, so the point was a miss', + ); + + // The same file under a ceiling that admits it IS used, which is what proves + // the previous assertion came from the size guard and not a broken path. + const admitted = install( + terrainHeightsProxy({ cachePath, cacheMaxDiskBytes: 1024 * 1024 }), + ).get('/api/terrain/heights'); + await request(admitted, { url: `/?points=${point}` }); + assert.deepEqual(asked, [point], 'a cache within the ceiling is still read'); +}); + +test('the route refuses a flood of requests from one client', async (t) => { + stubTerrainUpstream(t); + const route = install(terrainHeightsProxy()).get('/api/terrain/heights'); + + let refusedAt = 0; + for (let i = 1; i <= 95; i += 1) { + // A different point each time: distinct keys are the growth path. + const lon = (-97.7 - i / 1000).toFixed(5); + const answer = await request(route, { url: `/?points=${lon},30.26` }); + if (answer.statusCode === 429) { + refusedAt = i; + break; + } + } + assert.equal(refusedAt, 91, 'the 91st request in a minute is refused'); +}); + +test('a second client keeps its own quota', async (t) => { + stubFetch(t, async () => + Response.json([{ lng: -97.74, lat: 30.26, height: 150 }]), + ); + const route = install(terrainHeightsProxy()).get('/api/terrain/heights'); + + for (let i = 0; i < 90; i += 1) + await request(route, { + url: `/?points=${(-98 - i / 1000).toFixed(5)},30.26`, + remoteAddress: '198.51.100.7', + }); + const exhausted = await request(route, { + url: '/?points=-99.5,30.26', + remoteAddress: '198.51.100.7', + }); + assert.equal(exhausted.statusCode, 429); + + const neighbour = await request(route, { + url: '/?points=-99.6,30.26', + remoteAddress: '198.51.100.8', + }); + assert.notEqual(neighbour.statusCode, 429); +});