fix(embed): resolve highlight ids against the layer instead of trusting them

Explicit `featureId`/`featureIds` were passed through unchecked, so a
`highlightFeature` naming an id no feature carries — a typo, or any layer
whose features live in its MapLibre source rather than `layer.geojson` —
selected a phantom id, drew no highlight, and still answered `ack {ok:
true}`. Resolve ids against the layer's features under the same
`String(feature.id ?? index)` convention the map controller uses, and
reject a request that names features but resolves to none, leaving the
user's existing selection untouched. A request naming nothing is still
the documented "clear the highlight" form.

Also fix the docs claim that outbound messages are never addressed to
`*` (they are, before the handshake, when the `*` wildcard is
configured), type the null-geometry test fixtures as `Feature<null>[]`
(`Feature` defaults its geometry parameter to `Geometry`), and match the
surrounding assertion style in one test.

Author: giswqs

apps/geolibre-desktop/src/hooks/useEmbedApi.ts

@@ -125,6 +125,19 @@ export function useEmbedApi(mapControllerRef: RefObject<MapController | null>):
       const layer = state.layers.find((item) => item.id === target.layerId);
       if (!layer) throw new Error(`No layer with id "${target.layerId}"`);
       const ids = resolveHighlightIds(layer.geojson?.features ?? [], target);
+      // A request that names nothing is the documented "clear the highlight"
+      // form. A request that DOES name features but resolves to none is a
+      // failure, not a clear: the id may be a typo, or the layer may keep its
+      // features in its MapLibre source rather than `geojson` (vector tiles,
+      // source-owned layers), where nothing can be resolved here. Report that
+      // instead of wiping the user's selection behind an `ok` ack.
+      const askedForFeatures = target.featureIds.length > 0 || target.filter !== null;
+      if (askedForFeatures && ids.length === 0) {
+        throw new Error(
+          `highlightFeature matched no features in layer "${target.layerId}"` +
+            (layer.geojson ? "" : " (the layer's features are not readable from the store)"),
+        );
+      }
       // Drive the store as well as the map so the Attribute table and Layers
       // panel agree with what the host highlighted, exactly as a map click does.
       state.selectLayer(target.layerId);

apps/geolibre-desktop/src/lib/embed-api.ts

@@ -12,7 +12,7 @@
 // GeoLibre does not control — so it is off unless the deployment names the
 // origins it trusts, and every message is checked against that list.
 
-import type { Feature } from "geojson";
+import type { Feature, Geometry } from "geojson";
 
 /** Protocol version carried by every message in both directions. */
 export const EMBED_API_VERSION = 1;
@@ -324,16 +324,24 @@ export function parseEmbedRequest(
 /**
  * Resolve the feature ids a highlight target names within a layer's features.
  *
- * Explicit ids are kept as sent (they are matched against the same
- * `String(feature.id ?? index)` convention the map controller uses); a `filter`
- * selects every feature whose properties equal all of the filter's pairs.
+ * Ids are resolved against the layer, not taken on trust: an explicit id is kept
+ * only when a feature carries it under the same `String(feature.id ?? index)`
+ * convention the map controller uses, and a `filter` selects every feature whose
+ * properties equal all of the filter's pairs. An id that names nothing is
+ * dropped here so the caller can tell a real match from a typo (or from a layer
+ * whose features are not readable at all) rather than selecting a phantom.
  *
- * @param features - The layer's features, in map order.
+ * @param features - The layer's features, in map order. Only ids and properties
+ *   are read, so a null-geometry feature (an attribute-only table) is accepted.
  * @param target - The parsed highlight target.
  * @returns Feature ids to highlight, in feature order for filter matches.
  */
-export function resolveHighlightIds(features: Feature[], target: EmbedHighlightTarget): string[] {
-  const ids = [...target.featureIds];
+export function resolveHighlightIds(
+  features: Feature<Geometry | null>[],
+  target: EmbedHighlightTarget,
+): string[] {
+  const known = new Set(features.map((feature, index) => String(feature.id ?? index)));
+  const ids = target.featureIds.filter((id) => known.has(id));
   if (!target.filter) return ids;
   const pairs = Object.entries(target.filter);
   features.forEach((feature, index) => {

docs/user-guide/embedding.md

@@ -102,7 +102,9 @@ For a static build, bake it in instead:
 Entries are origins (`scheme://host[:port]`); a trailing path is ignored. `*`
 allows any origin and is only appropriate on a private network. The allowlist is
 enforced in both directions: a message from an unlisted origin is ignored, and
-every message the app sends is addressed to a listed origin, never `*`.
+every message the app sends is addressed to a listed origin. (With `*`
+configured, outbound messages are addressed to `*` until the host's first
+message identifies it, which is one more reason to name your origins.)
 
 Setting the allowlist also narrows the `?embed=1` project/scripting bridges (used
 by the [Python package](../python.md)) to the same origins. As extra hardening
@@ -129,9 +131,15 @@ them out of the other `postMessage` traffic on your page.
 | `highlightFeature` | `{ layerId, featureId \| featureIds \| filter, fit }`      | Selects and highlights features; `filter` matches properties. `fit` zooms to them.  |
 | `openTool`         | `{ id, params }`                                           | Opens the Processing dialog on a tool, pre-filling `params`. Runtime twin of `?tool=`. |
 
-Send `{ layerId }` alone to `highlightFeature` to clear the highlight. Add a
-`requestId` to any message and the app answers with an `ack` (below) reporting
-whether it worked.
+Send `{ layerId }` alone to `highlightFeature` to clear the highlight. A request
+that names features (or a filter) but matches none is rejected rather than
+treated as a clear, so a mistyped id does not silently wipe the user's selection.
+Highlighting reads the layer's features from the project, so it applies to vector
+layers GeoLibre holds as GeoJSON, not to ones whose features live only in a tile
+source.
+
+Add a `requestId` to any message and the app answers with an `ack` (below)
+reporting whether it worked.
 
 ### GeoLibre to host
 

tests/embed-api.test.ts

@@ -267,23 +267,42 @@ describe("parseEmbedRequest: openTool", () => {
   });
 
   it("requires an id", () => {
-    assert.ok(parseEmbedRequest(message("openTool", {}))?.hasOwnProperty("error"));
+    const parsed = parseEmbedRequest(message("openTool", {}));
+    assert.ok(parsed && "error" in parsed);
   });
 });
 
 describe("resolveHighlightIds", () => {
-  const features: Feature[] = [
+  // Geometry is irrelevant here (only ids and properties are read), so these
+  // fixtures declare it as null and type accordingly.
+  const features: Feature<null>[] = [
     { type: "Feature", id: "f1", properties: { parcel: "A-1", area: 12 }, geometry: null },
     { type: "Feature", properties: { parcel: "A-2", area: 34 }, geometry: null },
     { type: "Feature", id: "f3", properties: { parcel: "A-1", area: 56 }, geometry: null },
   ];
 
-  it("returns explicit ids untouched when no filter is given", () => {
+  it("keeps an explicit id that the layer actually carries", () => {
     assert.deepEqual(resolveHighlightIds(features, highlightTarget({ featureIds: ["f3"] })), [
       "f3",
     ]);
   });
 
+  it("resolves an explicit id against the index convention for an id-less feature", () => {
+    assert.deepEqual(resolveHighlightIds(features, highlightTarget({ featureIds: ["1"] })), ["1"]);
+  });
+
+  it("drops an explicit id no feature carries, rather than selecting a phantom", () => {
+    assert.deepEqual(resolveHighlightIds(features, highlightTarget({ featureIds: ["NOPE"] })), []);
+    assert.deepEqual(
+      resolveHighlightIds(features, highlightTarget({ featureIds: ["f1", "NOPE"] })),
+      ["f1"],
+    );
+  });
+
+  it("resolves nothing when the layer has no readable features", () => {
+    assert.deepEqual(resolveHighlightIds([], highlightTarget({ featureIds: ["f1"] })), []);
+  });
+
   it("matches every feature satisfying the filter, in feature order", () => {
     const ids = resolveHighlightIds(features, highlightTarget({ filter: { parcel: "A-1" } }));
     assert.deepEqual(ids, ["f1", "f3"]);