Merge branch 'main' into fix/geo-distance-dateline

changelog.d/1057-windows-npm-dep0190.md

@@ -0,0 +1,4 @@
+- On Windows, `npm run doctor` and the Pinokio install no longer print Node's
+  DEP0190 warning ("Passing args to a child process with shell option true can
+  lead to security vulnerabilities"): npm.cmd's fixed subcommand is now part of
+  the shell command string instead of an argument array (quanticsoul4772, #1057).

changelog.d/1117-google-viewer-lifecycle.md

@@ -0,0 +1,3 @@
+- Release superseded Google Street View open waits and suppress metadata
+  requests whose viewer was unmounted while the service loaded (rudycelekli,
+  #1117).

changelog.d/1172-cctv-missing-numbers.md

@@ -0,0 +1,13 @@
+- Keep missing CCTV numbers unknown instead of reading them as zero. `Number`
+  turns null, blank strings and booleans into 0 or 1, and the camera source
+  list sends an unknown number as JSON null, so a source-pack camera that left
+  out its pose reached the browser as zeros clamped to each field's floor —
+  facing north, pitch −2°, FOV 20°, range 120 m, mount 6 m and ground 0 m —
+  instead of the defaults (id-hash heading, −17°, 74°, 700 m, 24 m and the
+  city's ground); a null or blank position placed the camera at 0,0. The
+  server's `toFiniteNumber` and the browser's `safeNumber` now treat absent,
+  blank and boolean values as missing. The same conversion also stops a
+  Tallinn row with a null heading reading as a confident north-facing camera,
+  a Caltrans camera with a blank elevation losing its 150 m ground prior, and
+  a TfL or Catalonia camera with a null or blank coordinate being kept at 0,0
+  or on the zero meridian (omerbberk, #1172).

docs/CURRENT-STATE.md

@@ -1352,6 +1352,19 @@ Node-only package entries expose both provider factories. Routes, payloads,
 fallback behavior and existing dev/preview hook registration are preserved;
 browser rendering is unchanged.
 
+## CCTV missing numeric fields
+
+CCTV numbers are read as missing when they are absent, null, blank or boolean,
+on the server (`toFiniteNumber`, every pack and the file/env source packs) and
+in the browser (`safeNumber`). `/api/cctv/sources` sends an unknown number as
+JSON null; the browser then uses the bundled seed camera's value when one
+shares the id, else the field's default — id-hash heading, pitch −17°, FOV 74°
+(which a matched datasheet can still replace), range 700 m, mount 24 m, the
+city's ground elevation — rather than reading zero and clamping it to the
+field's floor. A camera with a missing latitude or longitude and no seed
+position is skipped by the browser catalog instead of being placed at 0,0.
+Real zeros, including `"0"`, are kept.
+
 ## Terrain, traffic, fire and bike-share provider modules
 
 Local composition now imports separate Node modules for Re:Earth heights,
@@ -3469,7 +3482,10 @@ controls (the address goes in the caption); a pose goes out at once and
 again when the panorama's month, photographer and address arrive (one
 successful lookup per panorama per session, seeded by the nearest lookup;
 a details lookup that fails or takes over ten seconds waits five seconds
-before a view event can retry, and an unmount drops lookups in flight). A key
+before a view event can retry, and an unmount drops lookups in flight). Opening
+another panorama settles the previous open before registering its wait; closing
+then releases the current wait immediately. A details lookup released while its
+service loads makes no panorama request after that service becomes available. A key
 Google refuses (`gm_authFailure`: the API not enabled, a referrer it does
 not allow, billing off) reads KEY REJECTED and fails a pending open at
 once; a lookup or open Google never answers fails after 10 or 20 s. In CI

scripts/format-scope.json

@@ -117,6 +117,7 @@
   "server/providers/cctv/constants.js",
   "server/providers/cctv/media.js",
   "server/providers/cctv/normalize.js",
+  "src/data/cctvMissingNumbers.test.mjs",
   "server/providers/cctv/sources.js",
   "server/providers/radio.js",
   "server/providers/radio/catalog.js",

scripts/pinokio-install.mjs

@@ -24,8 +24,8 @@ export function runChecked(command, args, { shell = false } = {}) {
 export function installPinokioDependencies() {
   applyPinokioEnvironment();
   rmSync(READY_FILE, { force: true });
-  const npm = npmProcessSpec();
-  runChecked(npm.command, ['ci'], { shell: npm.shell });
+  const npm = npmProcessSpec(['ci']);
+  runChecked(npm.command, npm.args, { shell: npm.shell });
 
   // Pinokio starts Vite directly and loads only its ENVIRONMENT file plus the
   // normal dotenv ladder. Unlike dev-fresh.sh, it does not import macOS

scripts/setup-doctor.mjs

@@ -76,10 +76,15 @@ export function hasRequiredDependencies(rootDir = ROOT) {
   }
 }
 
-/** Return the npm command and spawn mode required by the target platform. */
-export function npmProcessSpec(platform = process.platform) {
-  const windows = platform === 'win32';
-  return { command: windows ? 'npm.cmd' : 'npm', shell: windows };
+/**
+ * Return the npm command, arguments and spawn mode for the target platform.
+ * Windows must run npm.cmd through a shell, and Node 24 deprecates passing an
+ * argument array with `shell: true` (DEP0190), so there the arguments, which
+ * are fixed npm subcommands, join the command string.
+ */
+export function npmProcessSpec(args, platform = process.platform) {
+  if (platform !== 'win32') return { command: 'npm', args, shell: false };
+  return { command: ['npm.cmd', ...args].join(' '), args: [], shell: true };
 }
 
 /** Read one key from Vite's dotenv file ladder without depending on Vite. */
@@ -190,8 +195,8 @@ export function buildCapabilitySummary(
 
 export function inspectSetup({ includeKeychain = true, authoritativeEnvironment = false, rootDir = ROOT } = {}) {
   const node = classifyNodeVersion();
-  const npm = npmProcessSpec();
-  const npmResult = spawnSync(npm.command, ['--version'], {
+  const npm = npmProcessSpec(['--version']);
+  const npmResult = spawnSync(npm.command, npm.args, {
     encoding: 'utf8',
     shell: npm.shell,
   });

server/providers/cctv/normalize.js

@@ -39,11 +39,21 @@ export function escapeXml(text) {
 /**
  * Coerce a value to a finite number, returning fallback if NaN/Infinity.
  *
+ * Absent, blank and boolean values take the fallback too: `Number` reads
+ * null, '' and false as 0, which would turn a missing coordinate, heading or
+ * elevation into a real zero.
+ *
  * @param {*} value
  * @param {number} [fallback=NaN]
  * @returns {number}
  */
 export function toFiniteNumber(value, fallback = NaN) {
+  if (
+    value == null ||
+    typeof value === 'boolean' ||
+    (typeof value === 'string' && !value.trim())
+  )
+    return fallback;
   const num = Number(value);
   return Number.isFinite(num) ? num : fallback;
 }

src/data/cctvMissingNumbers.test.mjs

@@ -0,0 +1,252 @@
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import path from 'node:path';
+import {
+  fallbackHeadingFromId,
+  isLikelyCataloniaCoordinate,
+  toFiniteNumber,
+} from '../../server/providers/cctv/normalize.js';
+import {
+  loadCaltransSourcesFromOpenData,
+  loadTallinnSourcesFromCatalog,
+  loadTflSourcesFromOpenData,
+  parseCataloniaXml,
+} from '../../server/providers/cctv/sources.js';
+import { cctvProxy } from '../../server/providers/cctv.js';
+import { createCatalog } from '../layers/cctv/catalog.js';
+import { createModel } from '../layers/cctv/model.js';
+
+/** Values `Number` reads as 0 (or 1) although they carry no number. */
+const ABSENT = [null, undefined, '', ' ', false, true];
+
+/** Set env vars for one test and restore them afterwards. */
+function withEnv(t, env) {
+  for (const [name, value] of Object.entries(env)) {
+    const previous = process.env[name];
+    t.after(() => {
+      if (previous === undefined) delete process.env[name];
+      else process.env[name] = previous;
+    });
+    if (value === undefined) delete process.env[name];
+    else process.env[name] = value;
+  }
+}
+
+/** Silence the loaders' progress and failure logging. */
+function quiet(t) {
+  t.mock.method(console, 'log', () => {});
+  t.mock.method(console, 'warn', () => {});
+}
+
+/** The production browser model; only the catalog's pose pass is stubbed. */
+function browserModel() {
+  const parts = {};
+  parts.model = createModel({
+    state: {},
+    services: { focus: {} },
+    parts,
+    source: {},
+  });
+  return parts.model;
+}
+
+/** The browser catalog over the production model. */
+function browserCatalog(model, cityPois = {}) {
+  return createCatalog({
+    state: {},
+    services: { locations: { CITY_POIS: cityPois } },
+    parts: { model: { ...model, ensureCameraPose: () => {} } },
+    source: {},
+  });
+}
+
+/** GET /api/cctv/sources through the real proxy middleware. */
+async function requestSources(plugin) {
+  let handler;
+  plugin.configureServer({
+    middlewares: {
+      use(_route, fn) {
+        handler = fn;
+      },
+    },
+  });
+  const res = {
+    writeHead(status, headers) {
+      Object.assign(this, { status, headers });
+    },
+    end(body) {
+      this.body = body;
+    },
+  };
+  await handler({ url: '/sources', method: 'GET' }, res);
+  assert.equal(res.status, 200);
+  return JSON.parse(res.body);
+}
+
+test('absent, blank and boolean values take the fallback on both sides of the source list', () => {
+  const { safeNumber } = browserModel();
+  for (const value of ABSENT) {
+    assert.ok(Number.isNaN(toFiniteNumber(value)), `server ${String(value)}`);
+    assert.equal(toFiniteNumber(value, 15), 15, `server ${String(value)}`);
+    assert.ok(Number.isNaN(safeNumber(value)), `browser ${String(value)}`);
+    assert.equal(safeNumber(value, 74), 74, `browser ${String(value)}`);
+  }
+  for (const [value, expected] of [
+    [0, 0],
+    ['0', 0],
+    [-17, -17],
+    [' 12.5 ', 12.5],
+  ]) {
+    assert.equal(toFiniteNumber(value, 99), expected);
+    assert.equal(safeNumber(value, 99), expected);
+  }
+});
+
+test('a source-pack camera that omits or nulls its pose gets the browser defaults, not the clamp floors', async (t) => {
+  quiet(t);
+  const pose = {
+    headingDeg: null,
+    pitchDeg: null,
+    fovDeg: null,
+    rangeM: null,
+    mountHeightM: null,
+    groundElevationM: null,
+  };
+  const at = { cityId: 'austin', lat: 30.2672, lon: -97.7431 };
+  withEnv(t, {
+    CCTV_SOURCES_FILE: path.join(tmpdir(), 'gev-no-such-pack.json'),
+    CCTV_SOURCES_JSON: JSON.stringify([
+      { id: 'pack-pose-omitted', ...at, feedType: 'image' },
+      { id: 'pack-pose-null', ...at, ...pose, feedType: 'image' },
+      { id: 'pack-coords-null', lat: null, lon: null, feedType: 'image' },
+      { id: 'pack-coords-blank', lat: '', lon: ' ', feedType: 'image' },
+    ]),
+  });
+
+  const body = await requestSources(cctvProxy({ sourceRoot: tmpdir() }));
+  const model = browserModel();
+  const cameras = browserCatalog(model, {
+    austin: { name: 'Austin', groundElevation: 150 },
+  }).buildCatalogFromSources(body.sources);
+  const byId = new Map(cameras.map((camera) => [camera.id, camera]));
+
+  for (const id of ['pack-pose-omitted', 'pack-pose-null']) {
+    const camera = byId.get(id);
+    assert.ok(camera, id);
+    assert.equal(camera.headingDeg, model.headingFromId(id), id);
+    assert.equal(camera.pitchDeg, -17, id);
+    assert.equal(camera.fovDeg, 74, id);
+    assert.equal(camera.rangeM, 700, id);
+    assert.equal(camera.mountHeightM, 24, id);
+    assert.equal(camera.groundElevationM, 150, id);
+  }
+  // No position is not a camera in the Gulf of Guinea.
+  assert.equal(byId.has('pack-coords-null'), false);
+  assert.equal(byId.has('pack-coords-blank'), false);
+});
+
+test('Tallinn rows with a null heading or ground keep the low-confidence prior', (t) => {
+  quiet(t);
+  const root = mkdtempSync(path.join(tmpdir(), 'gev-tallinn-'));
+  t.after(() => rmSync(root, { recursive: true, force: true }));
+  const file = path.join(root, 'tallinn.json');
+  writeFileSync(
+    file,
+    JSON.stringify([
+      {
+        id: 'tln-null-heading',
+        lat: 59.437,
+        lon: 24.753,
+        headingDeg: null,
+        groundElevationM: null,
+        url: 'https://ristmikud.tallinn.ee/last/cam001.jpg',
+      },
+    ]),
+  );
+  withEnv(t, { CCTV_TALLINN_SOURCES_FILE: file });
+
+  const [camera] = loadTallinnSourcesFromCatalog({ sourceRoot: root });
+  assert.equal(camera.headingConfidence, 'low');
+  assert.equal(camera.headingDeg, fallbackHeadingFromId('tln-null-heading'));
+  assert.equal(camera.groundElevationM, 15);
+});
+
+test('TfL JamCams with null coordinates are dropped instead of placed at 0,0', async (t) => {
+  quiet(t);
+  const jamCam = (id, lat, lon) => ({
+    id: `JamCams_${id}`,
+    commonName: `JamCam ${id}`,
+    lat,
+    lon,
+    additionalProperties: [
+      { key: 'available', value: 'true' },
+      {
+        key: 'imageUrl',
+        value: `https://s3-eu-west-1.amazonaws.com/jamcams.tfl.gov.uk/${id}.jpg`,
+      },
+    ],
+  });
+  t.mock.method(globalThis, 'fetch', async () =>
+    Response.json([
+      jamCam('00001.00001', 51.5072, -0.1276),
+      jamCam('00001.00002', null, null),
+      jamCam('00001.00003', 51.5072, ''),
+    ]),
+  );
+
+  const cameras = await loadTflSourcesFromOpenData();
+  assert.deepEqual(
+    cameras.map((camera) => camera.id),
+    ['tfl-00001.00001'],
+  );
+});
+
+test('a Catalonia feature with a blank longitude is not read as the zero meridian', () => {
+  const feature = (coordinates) => `<gml:featureMember>
+    <cite:cameres fid="cameres.1">
+      <gml:coordinates>${coordinates}</gml:coordinates>
+      <cite:link>https://example.org/cam.jpg</cite:link>
+    </cite:cameres>
+  </gml:featureMember>`;
+  const [blank, valid] = parseCataloniaXml(
+    feature(',41.5') + feature('2.17,41.39'),
+  );
+  // 0 °E at 41.5 °N is inside the Catalonia box, so a zero would pass it.
+  assert.ok(Number.isNaN(blank.lon));
+  assert.equal(isLikelyCataloniaCoordinate(blank.lat, blank.lon), false);
+  assert.equal(isLikelyCataloniaCoordinate(valid.lat, valid.lon), true);
+});
+
+test('a Caltrans camera with a blank elevation keeps the 150 m ground prior', async (t) => {
+  quiet(t);
+  withEnv(t, { CCTV_CALTRANS_DISTRICTS: '4' });
+  t.mock.method(globalThis, 'fetch', async () =>
+    Response.json({
+      data: [
+        {
+          cctv: {
+            inService: 'true',
+            location: {
+              latitude: '37.8044',
+              longitude: '-122.2712',
+              elevation: '',
+              locationName: 'TV101 -- I-880 : Test Ave',
+            },
+            imageData: {
+              static: {
+                currentImageURL:
+                  'https://cwwp2.dot.ca.gov/data/d4/cctv/image/tv101/tv101.jpg',
+              },
+            },
+          },
+        },
+      ],
+    }),
+  );
+
+  const [camera] = await loadCaltransSourcesFromOpenData();
+  assert.ok(camera);
+  assert.equal(camera.groundElevationM, 150);
+});

src/layers/cctv/model.js

@@ -97,12 +97,23 @@ export function createModel({ state: layerState, services, parts, source }) {
 
   /**
    * Coerces a value to a finite number or returns the fallback.
+   *
+   * Absent, blank and boolean values return the fallback as well. The source
+   * list serializes an unknown number (NaN) as JSON null, and `Number(null)`
+   * is 0, so without this an unknown pose field arrived as zero and was
+   * clamped to the field's floor instead of taking its default.
    * @param {*} value
    * @param {number} [fallback=NaN]
    * @returns {number}
    */
 
   function safeNumber(value, fallback = NaN) {
+    if (
+      value == null ||
+      typeof value === 'boolean' ||
+      (typeof value === 'string' && !value.trim())
+    )
+      return fallback;
     const n = Number(value);
     return Number.isFinite(n) ? n : fallback;
   }

src/layers/streetLevel/providers/google/viewer.js

@@ -97,6 +97,8 @@ export function createGoogleViewer({ loader, getService, render } = {}) {
           service ||= getService
             ? await getService()
             : new (await ensureLibrary()).StreetViewService();
+          if (!current())
+            throw new Error('Street View details lookup was released');
           return service.getPanorama({ pano: panoId });
         })().then(resolve, reject);
       });
@@ -236,6 +238,7 @@ export function createGoogleViewer({ loader, getService, render } = {}) {
       const panoId = String(imageId);
       if (!host) throw new Error('Street View is not mounted');
       if (loader.authFailed()) throw new Error(KEY_REJECTED_MESSAGE);
+      cancelShown?.();
       pendingOpen = panoId;
       const instance = await ensurePanorama(host);
       if (pendingOpen !== panoId) return;

src/layers/streetLevel/providers/google/viewer.test.mjs

@@ -403,3 +403,69 @@ test('a remount looks up again a panorama whose lookup hung', async () => {
   await settle();
   assert.equal(poses.length, count);
 });
+
+test('superseded panorama waits cannot steal the current close cleanup', async (t) => {
+  t.mock.timers.enable({ apis: ['setTimeout'] });
+  const { OPEN_TIMEOUT_MS } = await import('./policy.js');
+  const flush = async () => {
+    for (let i = 0; i < 10; i++) await Promise.resolve();
+  };
+  const { fake, loader, viewer, host } = setup();
+  await viewer.mount(host);
+  const [panorama] = fake.built;
+  panorama.setPano = (id) => {
+    panorama.pano = id;
+  };
+  const first = viewer.open('pano-a');
+  await flush();
+  const second = viewer.open('pano-b');
+  await flush();
+  panorama.fire('position_changed'); // new panorama has not reached OK
+  await flush();
+  viewer.close();
+  const remaining = loader.listenerCount();
+  t.mock.timers.tick(OPEN_TIMEOUT_MS);
+  await Promise.allSettled([first, second]);
+  assert.equal(
+    remaining,
+    0,
+    'close disposes every pending open without waiting 20 seconds',
+  );
+  assert.equal(panorama.visible, false);
+});
+
+test('unmount during details-service acquisition makes no later billed lookup', async () => {
+  const fake = fakeStreetViewLibrary({ panoramas: PANORAMAS });
+  const loader = fakeMapsLoader(fake.library);
+  const { createGoogleProvider } = await import('./index.js');
+  let release;
+  const acquiring = new Promise((resolve) => {
+    release = resolve;
+  });
+  const importLibrary = loader.importLibrary.bind(loader);
+  let imports = 0;
+  loader.importLibrary = async (name) => {
+    if (++imports > 1) await acquiring;
+    return importLibrary(name);
+  };
+  const provider = createGoogleProvider({
+    getApiKey: () => 'fixture-key',
+    loader,
+  });
+  const instance = provider.create({
+    notify() {},
+    actions: { reportError() {} },
+  });
+  const viewer = instance.viewer;
+  await viewer.mount(fakeHost());
+  await viewer.open('pano-a');
+  viewer.unmount();
+  release();
+  for (let i = 0; i < 10; i++) await Promise.resolve();
+  assert.equal(
+    fake.lookups.length,
+    0,
+    'teardown invalidates the request before calling getPanorama',
+  );
+  instance.destroy();
+});

src/setupDoctor.test.mjs

@@ -69,12 +69,22 @@ test('placeholder values are never counted as configured credentials', () => {
 });
 
 test('doctor selects a Windows-safe npm process without changing Unix behavior', () => {
-  assert.deepEqual(npmProcessSpec('win32'), {
-    command: 'npm.cmd',
+  // Windows passes no argument array with shell: true (Node DEP0190).
+  assert.deepEqual(npmProcessSpec(['ci'], 'win32'), {
+    command: 'npm.cmd ci',
+    args: [],
     shell: true,
   });
-  assert.deepEqual(npmProcessSpec('darwin'), { command: 'npm', shell: false });
-  assert.deepEqual(npmProcessSpec('linux'), { command: 'npm', shell: false });
+  assert.deepEqual(npmProcessSpec(['--version'], 'darwin'), {
+    command: 'npm',
+    args: ['--version'],
+    shell: false,
+  });
+  assert.deepEqual(npmProcessSpec(['ci'], 'linux'), {
+    command: 'npm',
+    args: ['ci'],
+    shell: false,
+  });
 });
 
 test('doctor recognizes every OpenSky OAuth keychain alias used by dev-fresh', () => {