Merge pull request #937 from daikaginza/fix/overpass-cache-tracks-upstream

fix(overpass): do not serve cached answers from a different upstream (#932)

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Changelog
 
+- Overpass and military-installation disk caches no longer serve answers from a
+  previously configured `OVERPASS_UPSTREAMS` instance after it is repointed.
+
 - A Radio Browser mirror answering 204, 205 or 304, or a status outside
   200–599, no longer crashes the local dev/preview server. Null-body statuses
   get an empty body; anything else that cannot become a response fails that

docs/CURRENT-STATE.md

@@ -4132,6 +4132,10 @@ for, resolved lazily after env loading. It replaces the empty default list;
 there is no built-in fallback. Local/private instances are allowed. The stable
 `gods-eye-view/…` User-Agent is unchanged. URLs never appear in response headers
 or upstream errors, including old cached endpoint metadata.
+Disk cache entries for both routes carry a SHA-256 tag of the configured
+instance set (order does not matter) and are served only while it matches, so
+repointing `OVERPASS_UPSTREAMS` refetches instead of serving another instance's
+answers; untagged entries stop serving once an instance is configured.
 
 The shared transport guards `/api/overpass` and `/api/military-installations`.
 With no configured upstream, each route serves existing cache data (including

server/providers/military-installations.js

@@ -9,7 +9,10 @@ import {
   fetchOverpassPayload,
   overpassNotConfigured,
 } from './overpass/transport.js';
-import { resolveOverpassUpstreams } from './overpass/constants.js';
+import {
+  resolveOverpassUpstreams,
+  resolveOverpassUpstreamTag,
+} from './overpass/constants.js';
 import {
   _militaryInstallationCache,
   trimMilitaryInstallationCache,
@@ -74,7 +77,13 @@ function militaryInstallationsProxy() {
       retrievedAt: new Date().toISOString(),
       status: 'ready',
     };
-    const entry = { payload, cachedAt: Date.now() };
+    // Tagged with the configured upstream set so the disk layer does not serve
+    // this answer back after the operator repoints (see overpass/cache.js).
+    const entry = {
+      payload,
+      cachedAt: Date.now(),
+      upstreams: resolveOverpassUpstreamTag(),
+    };
     _militaryInstallationCache.set(key, entry);
     trimMilitaryInstallationCache();
     writeMilitaryInstallationDisk(key, entry);

server/providers/military-installations/cache.js

@@ -5,6 +5,7 @@ import {
   MILITARY_INSTALLATION_DISK_DIR,
   MILITARY_INSTALLATION_MAX_CACHE,
 } from './constants.js';
+import { cachedByConfiguredUpstream } from '../overpass/constants.js';
 import path from 'node:path';
 import { createHash } from 'node:crypto';
 import { promises as fsp } from 'node:fs';
@@ -114,6 +115,9 @@ async function readMilitaryInstallationDisk(
     const entry = JSON.parse(
       await fsp.readFile(militaryInstallationDiskPath(cacheKey, dir), 'utf8'),
     );
+    // Same reason as the Overpass disk cache: the key is the bbox alone, so an
+    // entry written against a different OVERPASS_UPSTREAMS is otherwise a hit.
+    if (!cachedByConfiguredUpstream(entry)) return null;
     if (!militaryInstallationDiskFresh(entry, maxAgeMs)) return null;
     return migrateMilitaryInstallationEntry(entry);
   } catch {

server/providers/overpass/cache.js

@@ -5,6 +5,7 @@ import {
   OVERPASS_DISK_DIR,
   OVERPASS_CACHE_MS,
   OVERPASS_CACHE_MAX_ENTRIES,
+  cachedByConfiguredUpstream,
 } from './constants.js';
 import path from 'node:path';
 import { createHash } from 'node:crypto';
@@ -47,6 +48,11 @@ async function readOverpassDisk(cacheKey, maxAgeMs) {
     // Older versions persisted 4xx refusals with normal data TTLs. Ignore
     // them on both fresh and stale reads so an upgrade can recover immediately.
     if (!overpassPayloadIsData(payload)) return null;
+    // The key is the query alone, so a file written by a different upstream —
+    // including the public instances that were the default before #742 — is a
+    // hit for the same query. Refuse it: the operator's configured instance
+    // must be what answers, on the stale path too.
+    if (!cachedByConfiguredUpstream(payload)) return null;
     if (Date.now() - payload.cachedAt > maxAgeMs) return null;
     return payload;
   } catch {

server/providers/overpass/constants.js

@@ -1,4 +1,5 @@
 import path from 'node:path';
+import { createHash } from 'node:crypto';
 
 // ---------------------------------------------------------------------------
 // Overpass API proxy constants and cache state
@@ -27,16 +28,68 @@ function parseOverpassUpstreams(raw) {
   return endpoints.slice(0, 8);
 }
 
-let upstreamMemo = { raw: null, endpoints: [] };
+let upstreamMemo = { raw: null, endpoints: [], tag: 'none' };
+
+/**
+ * Short, stable identifier for the configured upstream set. Cached responses
+ * carry it so an answer produced by a different instance is not served after
+ * the operator repoints `OVERPASS_UPSTREAMS` — the disk cache outlives the
+ * configuration change by 7 days, a month for boundary queries, and forever on
+ * the serve-stale path. It is a hash, never the URLs, and it is only ever
+ * compared on read: it is not sent to clients and does not appear in errors.
+ */
+function upstreamTag(endpoints) {
+  if (!endpoints.length) return 'none';
+  // Sorted, so reordering the same instances for failover keeps the cache.
+  return createHash('sha256')
+    .update([...endpoints].sort().join('\n'))
+    .digest('hex');
+}
 
 /** Resolve after environment loading. Public Overpass instances are not used by default. */
 function resolveOverpassUpstreams() {
-  const raw = process.env.OVERPASS_UPSTREAMS || '';
-  if (upstreamMemo.raw !== raw)
-    upstreamMemo = { raw, endpoints: parseOverpassUpstreams(raw) };
+  readUpstreamMemo();
   return [...upstreamMemo.endpoints];
 }
 
+/** The configured-upstream tag stored alongside, and checked against, cache entries. */
+function resolveOverpassUpstreamTag() {
+  return readUpstreamMemo().tag;
+}
+
+let reportedForeignCache = false;
+
+/**
+ * Was this cached entry produced by the upstream set configured now?
+ *
+ * With nothing configured there is no instance to contradict, and serving
+ * last-good data at any age is the point of the cache (the proxy's
+ * not-configured path depends on it), so any entry is accepted. Once an
+ * operator does configure an instance, only its own answers are served: an
+ * entry with no tag, or one from a different instance, is refused rather than
+ * matched on the query alone. Shared by both caches fed by these upstreams.
+ */
+function cachedByConfiguredUpstream(entry) {
+  const tag = resolveOverpassUpstreamTag();
+  if (tag === 'none' || entry?.upstreams === tag) return true;
+  if (!reportedForeignCache) {
+    reportedForeignCache = true;
+    console.warn(
+      '[Overpass] ignoring cached responses produced by a different OVERPASS_UPSTREAMS; they will be refetched from the configured instance.',
+    );
+  }
+  return false;
+}
+
+function readUpstreamMemo() {
+  const raw = process.env.OVERPASS_UPSTREAMS || '';
+  if (upstreamMemo.raw !== raw) {
+    const endpoints = parseOverpassUpstreams(raw);
+    upstreamMemo = { raw, endpoints, tag: upstreamTag(endpoints) };
+  }
+  return upstreamMemo;
+}
+
 /**
  * TTL for FRESH cached Overpass responses (ms). Road geometry is static for
  * months — the original 45 s TTL forced a public-mirror round-trip on nearly
@@ -153,6 +206,8 @@ export {
   OVERPASS_MAX_RESPONSE_BYTES,
   parseOverpassUpstreams,
   resolveOverpassUpstreams,
+  resolveOverpassUpstreamTag,
+  cachedByConfiguredUpstream,
   OVERPASS_USER_AGENT,
   OVERPASS_TIMEOUT_MS,
 };

server/providers/overpass/transport.js

@@ -1,6 +1,7 @@
 import {
   OVERPASS_MAX_RESPONSE_BYTES,
   resolveOverpassUpstreams,
+  resolveOverpassUpstreamTag,
   OVERPASS_USER_AGENT,
   OVERPASS_TIMEOUT_MS,
 } from './constants.js';
@@ -187,6 +188,9 @@ async function fetchOverpassPayload(
         contentType: 'application/json',
         // Never retain a secret-bearing endpoint in cache or response metadata.
         endpoint: 'configured',
+        // Which configured instance produced this, as a hash rather than a URL,
+        // so a cached answer is not reused after the operator repoints.
+        upstreams: resolveOverpassUpstreamTag(),
         rateLimited: false,
       };
     } catch {

src/data/installationProxy.test.mjs

@@ -26,6 +26,7 @@ import {
   validMilitaryInstallationBox,
   writeMilitaryInstallationDisk,
 } from '../../vite.config.js';
+import { resolveOverpassUpstreamTag } from '../../server/providers/overpass/constants.js';
 
 const DAY_MS = 86_400_000;
 test('installation failure reasons disclose no raw upstream error and do not guess overload', () => {
@@ -274,3 +275,30 @@ test('an unwritable cache directory degrades to a miss instead of throwing', asy
     await fsp.rm(dir, { recursive: true, force: true });
   }
 });
+
+test('an installation entry is not served after OVERPASS_UPSTREAMS is repointed', async (t) => {
+  const prior = process.env.OVERPASS_UPSTREAMS;
+  t.after(() => {
+    if (prior === undefined) delete process.env.OVERPASS_UPSTREAMS;
+    else process.env.OVERPASS_UPSTREAMS = prior;
+  });
+  const dir = tempCacheDir();
+  const key = '30.250,-97.750,30.300,-97.700';
+  try {
+    process.env.OVERPASS_UPSTREAMS = 'https://a.example/api/interpreter,https://b.example/api/interpreter';
+    const tag = resolveOverpassUpstreamTag();
+    await writeMilitaryInstallationDisk(key, { ...entry(Date.now()), upstreams: tag }, dir);
+    assert.ok(await readMilitaryInstallationDisk(key, TTL_MS, dir), 'its own entry is a hit');
+    process.env.OVERPASS_UPSTREAMS = 'https://b.example/api/interpreter,https://a.example/api/interpreter';
+    assert.equal(resolveOverpassUpstreamTag(), tag, 'reordering the same instances keeps the tag');
+    process.env.OVERPASS_UPSTREAMS = 'https://c.example/api/interpreter';
+    assert.equal(await readMilitaryInstallationDisk(key, TTL_MS, dir), null, 'fresh read refused');
+    assert.equal(await readMilitaryInstallationDisk(key, Infinity, dir), null, 'stale read refused');
+    await writeMilitaryInstallationDisk('legacy', entry(Date.now()), dir);
+    assert.equal(await readMilitaryInstallationDisk('legacy', Infinity, dir), null, 'untagged entry refused');
+    delete process.env.OVERPASS_UPSTREAMS;
+    assert.ok(await readMilitaryInstallationDisk(key, Infinity, dir), 'nothing configured accepts any entry');
+  } finally {
+    fs.rmSync(dir, { recursive: true, force: true });
+  }
+});

src/overpassProxy.test.mjs

@@ -12,6 +12,7 @@ import { createHash, randomUUID } from 'node:crypto';
 import path from 'node:path';
 import { Readable } from 'node:stream';
 import createViteConfig, { fetchOverpassPayload, overpassPayloadIsData, readOverpassDisk } from '../vite.config.js';
+import { resolveOverpassUpstreamTag } from '../server/providers/overpass/constants.js';
 
 const DATA = { status: 200, body: '{"elements":[]}' };
 
@@ -155,7 +156,13 @@ test('coalesced outage callers both receive last-good data, never a cached refus
     const directory = path.join(process.cwd(), '.gev-cache', 'overpass');
     const file = path.join(directory, `${createHash('sha1').update(body).digest('hex')}.json`);
     await mkdir(directory, { recursive: true });
-    const stale = { ...DATA, cachedAt: Date.now() - 40 * 86400000 };
+    // Last-good data was fetched from the instance configured above, so it
+    // carries that instance's tag the way the transport now writes it.
+    const stale = {
+      ...DATA,
+      cachedAt: Date.now() - 40 * 86400000,
+      upstreams: resolveOverpassUpstreamTag(),
+    };
     await writeFile(file, JSON.stringify(stale));
     const entered = Promise.withResolvers();
     const release = Promise.withResolvers();
@@ -188,3 +195,46 @@ test('coalesced outage callers both receive last-good data, never a cached refus
     }
   }
 });
+
+test('a disk entry is not served after OVERPASS_UPSTREAMS is repointed', async (t) => {
+  const prior = process.env.OVERPASS_UPSTREAMS;
+  t.after(() => {
+    if (prior === undefined) delete process.env.OVERPASS_UPSTREAMS;
+    else process.env.OVERPASS_UPSTREAMS = prior;
+  });
+  const key = `overpass-upstream-switch-${randomUUID()}`;
+  const directory = path.join(process.cwd(), '.gev-cache', 'overpass');
+  const file = path.join(
+    directory,
+    `${createHash('sha1').update(key).digest('hex')}.json`,
+  );
+  await mkdir(directory, { recursive: true });
+  try {
+    // Written while one instance was configured...
+    process.env.OVERPASS_UPSTREAMS = 'https://first.example/api/interpreter';
+    const first = resolveOverpassUpstreamTag();
+    await writeFile(
+      file,
+      JSON.stringify({ ...DATA, cachedAt: Date.now(), upstreams: first }),
+    );
+    assert.ok(await readOverpassDisk(key, 60000), 'its own entry is a hit');
+
+    // ...is not an answer once the operator points at a different one, on the
+    // stale path either: the key is the query alone, so it would otherwise hit.
+    process.env.OVERPASS_UPSTREAMS = 'https://second.example/api/interpreter';
+    assert.notEqual(resolveOverpassUpstreamTag(), first, 'tag follows the env');
+    assert.equal(await readOverpassDisk(key, 60000), null, 'fresh read refused');
+    assert.equal(
+      await readOverpassDisk(key, Infinity),
+      null,
+      'stale read refused',
+    );
+
+    // With nothing configured there is no instance to contradict, and serving
+    // last-good data is what the not-configured path exists for.
+    delete process.env.OVERPASS_UPSTREAMS;
+    assert.ok(await readOverpassDisk(key, Infinity), 'unconfigured still serves');
+  } finally {
+    await unlink(file);
+  }
+});