Merge pull request #1280 from bilawalsidhu/codex/fix-tool-display-text

fix(tools): bound untrusted display text at output boundaries

changelog.d/1280-text-issue-fix.md

@@ -0,0 +1,3 @@
+- Keep outside text in voice and MCP tool results to one line of 180
+  characters: ALPR tags, station names and other display fields (asturksever,
+  #1280).

docs/CURRENT-STATE.md

@@ -129,6 +129,22 @@ search keeps a successful geocode, with no search or view-based region recovery.
 
 Manager-owned periodic network updates pause while the document is hidden. Returning to a visible tab runs one overdue update per enabled layer without overlapping an active refresh and restarts that layer's polling schedule from the return, so the next poll follows one interval later whether or not the catch-up update succeeds; explicit refresh requests remain available. Transit also defers camera-driven feed admission while hidden and rechecks coverage on return. Visibility listeners are released with their polling intervals.
 
+Overpass reserves each normalized query before its disk-cache lookup. The
+initial disk lookup, upstream quota admission, fetching and memory publication
+are coalesced under that reservation. A caller that joined a reservation
+rejected by another client's quota retries admission with its own quota.
+Stale fallback runs per response after the reservation is released, and disk
+writes finish asynchronously. Settled and rejected reservations are removed
+only while they still own their map entry.
+
+External ALPR tags, surveillance-tool fields, voice context text and compact
+analyst strings become one line of at most 180 characters (scene labels: 120).
+Empty ALPR values stay null, and the cap never splits a surrogate pair.
+Canonical analyst identifiers such as `id` and `routeId` stay unchanged, because
+voice follow-ups match on them and bikeshare uses the raw station name as its
+ID. The text is still untrusted: normalization does not prevent prompt
+injection.
+
 Terrain height requests accept at most 64 points, matching the browser service's
 existing batch size and one bounded upstream chunk. Larger requests return HTTP
 413 before disk-cache loading or upstream work; malformed coordinates remain 400.

scripts/package-boundaries.json

@@ -2115,6 +2115,7 @@
       "src/layers/alpr/overlay.js",
       "src/layers/alpr/source.js",
       "src/data/inputOwnership.js",
+      "src/data/displayText.js",
       "src/layers/alpr/records.js",
       "src/data/tomtomTiles.js",
       "src/data/tileBudget.js",
@@ -3023,6 +3024,7 @@
       "src/voice/chatPreferences.js",
       "src/voice/panelLayout.js",
       "src/voice/voiceMiniPreview.js",
+      "src/data/displayText.js",
       "src/voice/gevActions.js",
       "src/agent/worldPacket.js",
       "src/voice/speech.js",
@@ -3871,6 +3873,7 @@
       "src/search/coordinateParser.js",
       "src/search/presetGeocoder.js",
       "src/app/localGeojsonServices.js",
+      "src/data/displayText.js",
       "src/layers/alpr/records.js",
       "src/layers/earthquakes/records.js",
       "src/app/layers/perimeters.js",
@@ -4547,6 +4550,7 @@
     "exports": ["./layers/alpr/source"],
     "modules": [
       "src/layers/alpr/policy.js",
+      "src/data/displayText.js",
       "src/layers/alpr/records.js",
       "src/layers/alpr/source.js",
       "src/data/tomtomTiles.js",
@@ -5246,6 +5250,7 @@
       "src/layers/submarineCables/bundledSource.js",
       "src/layers/alpr/source.js",
       "src/layers/alpr/policy.js",
+      "src/data/displayText.js",
       "src/layers/alpr/records.js",
       "src/layers/alpr/tileRecords.js",
       "src/sources/infrastructureData.js",

src/data/alprCameras.test.mjs

@@ -1518,3 +1518,24 @@ test('a repeated move inside an outstanding query keeps the displayed marker set
     h.restore();
   }
 });
+
+test('ALPR display tags collapse controls and cap final text without erasing Unicode', () => {
+  const record = normalizeAlprNode({
+    type: 'node',
+    id: 1,
+    lat: 30,
+    lon: -97,
+    tags: {
+      'surveillance:type': 'ALPR',
+      operator: '  Şehir\n\u0000 PD  ',
+      manufacturer: '😀'.repeat(200),
+      'camera:type': null,
+      'surveillance:zone': '\t traffic   zone ',
+    },
+  });
+  assert.equal(record.operator, 'Şehir PD');
+  assert.equal(record.manufacturer.length, 180);
+  assert.equal(record.manufacturer, '😀'.repeat(90));
+  assert.equal(record.cameraType, null);
+  assert.equal(record.zone, 'traffic zone');
+});

src/data/displayText.js

@@ -0,0 +1,11 @@
+/** One-line text capped at `maxLength` without splitting a surrogate pair. Still untrusted. */
+export function normalizeDisplayText(value, maxLength = 180) {
+  if (value == null || typeof value === 'object') return '';
+  return String(value)
+    .replace(/[\u0000-\u001f\u007f-\u009f]/g, ' ')
+    .replace(/\s+/g, ' ')
+    .trim()
+    .slice(0, maxLength)
+    .replace(/[\uD800-\uDBFF]$/, '')
+    .trim();
+}

src/layers/alpr/records.js

@@ -1,7 +1,8 @@
+import { normalizeDisplayText } from '../../data/displayText.js';
 import { QUERY_LIMIT, QUERY_SNAP_DEGREES } from './policy.js';
 
 function textTag(value) {
-  const t = String(value ?? '').trim();
+  const t = normalizeDisplayText(value);
   return t || null;
 }
 

src/voice/gevActions.js

@@ -1,4 +1,5 @@
 import { interruptGlobeRotation } from '../cameraVerbs.js';
+import { normalizeDisplayText } from '../data/displayText.js';
 import {
   layerSnapshot,
   feedProvenanceEnvelope,
@@ -5068,13 +5069,7 @@ function uniqueStrings(values) {
  * @returns {string} Sanitized single-line label (max 120 chars).
  */
 function sanitizeLabel(value) {
-  const text = String(value || '');
-  let out = '';
-  for (const ch of text) {
-    const code = ch.codePointAt(0);
-    out += code < 0x20 || code === 0x7f ? ' ' : ch; // drop control chars incl. newlines
-  }
-  return out.replace(/\s+/g, ' ').trim().slice(0, 120);
+  return normalizeDisplayText(value, 120);
 }
 
 function cachePrecision(service) {
@@ -5191,10 +5186,8 @@ function compactProperties(props) {
 }
 
 function cleanText(value) {
-  if (value == null || typeof value === 'object') return '';
-  const text = String(value).trim();
-  if (!text || text === 'undefined' || text === 'null') return '';
-  return text.length > 180 ? `${text.slice(0, 177)}...` : text;
+  const text = normalizeDisplayText(value);
+  return text === 'undefined' || text === 'null' ? '' : text;
 }
 
 function layerTitle(layerId) {
@@ -5359,6 +5352,22 @@ const ANALYST_IDENTITY_FIELDS = Object.freeze([
   'noradId',
 ]);
 
+/**
+ * Keys that identify an analyst record. The referent registry, track_entity,
+ * locateLayerEntity and follow-up filters match on them, so they skip
+ * display-text normalization.
+ */
+const ANALYST_CANONICAL_FIELDS = new Set([
+  'layerKey',
+  'id',
+  'icao24',
+  'mmsi',
+  'noradId',
+  'registration',
+  'callsign',
+  'routeId',
+]);
+
 /**
  * Compact one analyst record for the voice payload: identity, position (so a
  * follow-up can fly to it), distance, and that layer's manifest fields with
@@ -5396,6 +5405,11 @@ function compactAnalystItem(r) {
     } else compact[key] = r[key];
   }
   if (present(r.ageHours)) compact.ageHours = r.ageHours;
+  // Canonical keys stay raw: bikeshare's id is the raw station name.
+  for (const key of Object.keys(compact)) {
+    if (typeof compact[key] === 'string' && !ANALYST_CANONICAL_FIELDS.has(key))
+      compact[key] = normalizeDisplayText(compact[key]);
+  }
   return compact;
 }
 

src/voice/gevActions.test.mjs

@@ -6548,6 +6548,84 @@ test('analyst-only lookup distinguishes duplicate names and uses canonical aircr
   assert.equal(plane.latitude, 36, 'canonical ID resolves the current position, not old coordinates');
 });
 
+test('analyst item display strings are single-line and bounded for voice', async () => {
+  globalThis.window = globalThis.window || { clearTimeout, setTimeout, requestIdleCallback: null };
+  const rows = [{ id: 'stable-id', name: 'Şehir\n' + 'x'.repeat(3000), label: 'Town\tStation', operator: 'City\nPD', lat: 30, lon: -97 }];
+  const { runner } = manifestRunner({ bikeshare: { getStats: () => ({ count: 1, lastUpdate: Date.now() }), getAnalystRecords: () => rows } });
+  const result = await runner('analyst_query', { layers: ['bikeshare'], scope: { kind: 'anywhere' }, limit: 1 });
+  assert.equal(result.ok, true);
+  assert.equal(result.items[0].name.length, 180);
+  assert.equal(result.items[0].label, 'Town Station');
+  assert.equal(result.items[0].id, 'stable-id');
+  for (const value of Object.values(result.items[0])) if (typeof value === 'string') assert.doesNotMatch(value, /[\r\n\t]/);
+});
+
+test('analyst_query keeps canonical IDs raw so numbered references still fly to the station', async () => {
+  globalThis.window = globalThis.window || { clearTimeout, setTimeout, requestIdleCallback: null };
+  const { viewer, styleManager } = createVoiceNavigationHarness();
+  const referents = createReferentRegistry();
+  const longName = 'Station ' + 'é'.repeat(200);
+  const spaced = 'Congress  Ave   & 2nd';
+  const stations = [
+    { id: spaced, name: spaced, lat: 30.2672, lon: -97.7431 },
+    { id: longName, name: longName, lat: 30.2711, lon: -97.7437 },
+  ];
+  const bikeshare = { getStats: () => ({ count: stations.length, lastUpdate: Date.now() }), getAnalystRecords: () => stations };
+  const runner = createActionRunner({
+    viewer, styleManager,
+    dataManager: {
+      layers: new Map([['bikeshare', { module: bikeshare }]]),
+      isEnabled: () => true,
+      getAll: () => [{ id: 'bikeshare', name: 'bikeshare', enabled: true, stats: bikeshare.getStats() }],
+    },
+    deixis: { referents },
+  });
+  const listed = await runner('analyst_query', { layers: ['bikeshare'], scope: { kind: 'anywhere' }, limit: 2 });
+  assert.equal(listed.ok, true);
+  const bySpaced = listed.items.find((item) => item.id === spaced);
+  const byLong = listed.items.find((item) => item.id === longName);
+  assert.ok(bySpaced && byLong, 'compact items keep the raw station-name IDs');
+  assert.equal(bySpaced.name, 'Congress Ave & 2nd', 'display name is still normalized');
+  assert.equal(byLong.name.length, 180, 'display name is still bounded');
+  assert.equal(byLong.id.length, longName.length);
+  const registered = referents.list();
+  assert.equal(registered.length, 2, 'the analyst result became the numbered list');
+  for (const ref of registered) {
+    const station = stations.find((row) => row.id === ref.id);
+    assert.ok(station, `referent ${ref.n} keeps the canonical ID`);
+    const flown = await runner('fly_to_location', { referent: ref.n });
+    assert.equal(flown.ok, true, `referent ${ref.n} resolves instead of "no longer loaded"`);
+    assert.equal(flown.latitude, station.lat);
+    assert.equal(flown.longitude, station.lon);
+  }
+});
+
+test('analyst_query keeps transit route IDs raw so a follow-up filter still matches', async () => {
+  globalThis.window = globalThis.window || { clearTimeout, setTimeout, requestIdleCallback: null };
+  for (const routeId of ['Route  17', 'R'.repeat(181)]) {
+    const { viewer, styleManager } = createVoiceNavigationHarness();
+    const vehicles = [{ id: 'bus-1', routeId, lat: 30, lon: -97 }];
+    const transit = { getStats: () => ({ count: 1, lastUpdate: Date.now() }), getAnalystRecords: () => vehicles };
+    const runner = createActionRunner({
+      viewer, styleManager,
+      dataManager: {
+        layers: new Map([['transit', { module: transit }]]),
+        isEnabled: () => true,
+        getAll: () => [{ id: 'transit', name: 'transit', enabled: true, stats: transit.getStats() }],
+      },
+    });
+    const listed = await runner('analyst_query', { layers: ['transit'], scope: { kind: 'anywhere' }, limit: 1 });
+    assert.equal(listed.ok, true);
+    assert.equal(listed.items[0].routeId, routeId);
+    const followUp = await runner('analyst_query', {
+      followUp: true,
+      filters: [{ field: 'routeId', op: 'eq', value: listed.items[0].routeId }],
+    });
+    assert.equal(followUp.ok, true);
+    assert.equal(followUp.count, 1, 'the returned route ID matches the same vehicle');
+  }
+});
+
 test('set_panel_open accepts the Authoring name for the scene panel', async () => {
   const calls = [];
   const runner = createGevActionRunner({