Skip to content

Commit 526cba0

Browse files
committed
fix(places): gate OSM snapping by item identity + manifest-driven scheme dispatch
Two related problems were letting unrelated OSM POI metadata leak onto data- source items in the place panel: 1. lookupByOsmFilters snapped to the nearest amenity match regardless of identity. A Dott bike station co-located with an optician kiosk would inherit the optician's website, opening hours, wheelchair status, and social tags. Bike-sharing, car-sharing, ev-charging, fuel, and parking detail mappers now carry an OsmIdentity (ref / operator / network / brand) on DataSourceDetail; the resolver passes it to lookupByOsmFilters which only accepts candidates that match by at least one identity field. Match is normalized + token-subset so "Shell" matches "Shell Deutschland Oil GmbH" but not the optician. 2. The /api/places/:id route fell through to a generic name+coord lookup for any scheme without a registered resolver, which produced the same leak whenever an integration's setup() failed to register. The fallback is now gated on isIntegrationScheme(): schemes owned by an installed manifest must resolve via a registered resolver (404 otherwise); non-integration freeform schemes (saved, label, stylePoi, streetView) still get the name+coord lookup. The gate is data-driven from the manifest registry — no allowlist to maintain.
1 parent fe0e631 commit 526cba0

13 files changed

Lines changed: 311 additions & 8 deletions

File tree

apps/api/src/integration-host.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,6 +1130,20 @@ export function getAllIntegrations(): LoadedIntegration[] {
11301130
return Array.from(integrations.values());
11311131
}
11321132

1133+
/**
1134+
* True when `scheme` matches the id of an installed integration manifest —
1135+
* regardless of whether its `setup()` succeeded. The places route uses this
1136+
* to decide whether an unregistered resolver should 404 (the integration
1137+
* owns the scheme but didn't boot) or fall through to the freeform name+
1138+
* coord lookup (the scheme is a UI-side convention like `saved`/`stylePoi`,
1139+
* not an integration). Manifest discovery runs before `setup()`, so this
1140+
* stays accurate even when an integration crashes during boot — which is
1141+
* the failure mode this gate exists to catch.
1142+
*/
1143+
export function isIntegrationScheme(scheme: string): boolean {
1144+
return integrations.has(scheme);
1145+
}
1146+
11331147
export function getIntegrationsByDomain(domain: string): LoadedIntegration[] {
11341148
return Array.from(integrations.values()).filter(
11351149
(i) => i.enabled && i.manifest.domains.includes(domain),

apps/api/src/routes/__tests__/places.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,10 @@ vi.mock("@integrations/reviews/orchestrator", () => ({
8585
fetchAggregate: vi.fn().mockResolvedValue(null),
8686
}));
8787

88+
const mockIsIntegrationScheme = vi.fn().mockReturnValue(false);
8889
vi.mock("../../integration-host.js", () => ({
8990
getAllIntegrations: vi.fn().mockReturnValue([]),
91+
isIntegrationScheme: (scheme: string) => mockIsIntegrationScheme(scheme),
9092
}));
9193

9294
// Mock DB RIS service
@@ -455,6 +457,53 @@ describe("GET /places/:id", () => {
455457
expect(mockLookupByCoords).toHaveBeenCalled();
456458
});
457459

460+
it("returns 404 for an integration scheme whose resolver didn't register", async () => {
461+
// Mirrors the failure mode that produced the original leak: a data-
462+
// source integration (here scooter-sharing) is installed — its manifest
463+
// is on disk so `isIntegrationScheme` returns true — but its `setup()`
464+
// threw at boot, so no resolver was registered. Without this gate the
465+
// route would fall through to lookupByCoords and substitute the
466+
// nearest OSM POI's tags onto the scooter.
467+
mockIsIntegrationScheme.mockImplementation((scheme) => scheme === "scooter-sharing");
468+
469+
const res = await app.inject({
470+
method: "GET",
471+
url: `/places/${encodeURIComponent("scooter-sharing:dott-123")}?${qs({
472+
lat: "50.7764",
473+
lng: "6.0889",
474+
name: "Dott E-Scooter",
475+
})}`,
476+
});
477+
478+
expect(res.statusCode).toBe(404);
479+
expect(mockLookupByNameAndCoords).not.toHaveBeenCalled();
480+
expect(mockLookupByCoords).not.toHaveBeenCalled();
481+
482+
mockIsIntegrationScheme.mockReturnValue(false);
483+
});
484+
485+
it("allows coord-fallback for a non-integration freeform scheme (stylePoi)", async () => {
486+
// `stylePoi` is emitted by the web client when the user clicks a basemap
487+
// POI symbol. It corresponds to no integration manifest, so the route
488+
// should let the name+coord lookup run as today — that's how we get the
489+
// OSM POI's full enrichment when the user genuinely asked for it.
490+
mockLookupByNameAndCoords.mockResolvedValue(MOCK_PLACE);
491+
mockGetPlaceKnowledge.mockResolvedValue({ externalIds: {} });
492+
mockBuildReviewLinks.mockReturnValue([]);
493+
494+
const res = await app.inject({
495+
method: "GET",
496+
url: `/places/${encodeURIComponent("stylePoi:abc")}?${qs({
497+
lat: "52.52",
498+
lng: "13.37",
499+
name: "Some POI",
500+
})}`,
501+
});
502+
503+
expect(res.statusCode).toBe(200);
504+
expect(mockLookupByNameAndCoords).toHaveBeenCalled();
505+
});
506+
458507
it("returns 404 when neither lookup returns a result", async () => {
459508
mockLookupByNameAndCoords.mockResolvedValue(null);
460509
mockLookupByCoords.mockResolvedValue(null);

apps/api/src/routes/places.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import {
3131
type PlaceResolverContext,
3232
} from "@openmapx/place-ids";
3333
import type { FastifyPluginAsync } from "fastify";
34-
import { getAllIntegrations } from "../integration-host.js";
34+
import { getAllIntegrations, isIntegrationScheme } from "../integration-host.js";
3535
import { getPlaceKnowledge } from "../services/knowledge/index";
3636
import { buildReviewLinks } from "../services/review-links";
3737
import { TTL, withCache } from "../utils/cache.js";
@@ -384,6 +384,25 @@ export const placesRoute: FastifyPluginAsync = async (fastify) => {
384384
}
385385
return enrichPlace(resolved, lang);
386386
}
387+
// No resolver registered for this scheme. The coord-fallback
388+
// below would happily snap to the nearest OSM POI — fine for
389+
// freeform UI schemes (saved labels, basemap POI clicks,
390+
// Street View drops) that aren't backed by any integration,
391+
// dangerous for an integration whose `setup()` failed and
392+
// never got to register its resolver. The manifest registry
393+
// tells us which is which: any scheme matching an installed
394+
// integration id is strict; everything else is freeform.
395+
if (isIntegrationScheme(parsedId.scheme)) {
396+
fastify.log.warn(
397+
{ scheme: parsedId.scheme, rawId },
398+
"places: integration scheme has no resolver; refusing coord-fallback",
399+
);
400+
const err: CacheableError = {
401+
statusCode: 404,
402+
message: `No resolver for scheme '${parsedId.scheme}'`,
403+
};
404+
throw err;
405+
}
387406
}
388407

389408
// Coord-fallback path for schemes without a registered resolver

integrations/data-source/resolver.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,26 @@
1515
*/
1616

1717
import { createPlace } from "@openmapx/core";
18-
import type { MobilityDataSourceProvider } from "@openmapx/integration-framework";
18+
import type { MobilityDataSourceProvider, OsmIdentity } from "@openmapx/integration-framework";
1919
import {
2020
lookupAddressByCoords,
2121
lookupByOsmFilters,
2222
} from "@openmapx/integration-geocoding/place-lookup";
2323
import { VEHICLE_ID_PREFIX } from "@openmapx/mobility-core/mapper";
2424
import type { PlaceResolver } from "@openmapx/place-ids";
2525

26+
async function fetchIdentity(
27+
provider: MobilityDataSourceProvider,
28+
itemId: string,
29+
): Promise<OsmIdentity | undefined> {
30+
try {
31+
const detail = await provider.getDetail(itemId);
32+
return detail.data?.identity;
33+
} catch {
34+
return undefined;
35+
}
36+
}
37+
2638
export function createDataSourceResolver(provider: MobilityDataSourceProvider): PlaceResolver {
2739
const scheme = provider.id;
2840
const osmFilters = provider.meta.osmFilters;
@@ -35,13 +47,20 @@ export function createDataSourceResolver(provider: MobilityDataSourceProvider):
3547

3648
const isVehicle = value.startsWith(VEHICLE_ID_PREFIX);
3749

50+
// When the provider supplies an identity for the item (operator / ref /
51+
// network / brand), constrain the OSM snap to candidates that match it.
52+
// Providers that don't yet expose `identity` on their detail get the
53+
// legacy nearest-match behaviour — the gate inside `lookupByOsmFilters`
54+
// ignores identity when none is passed.
55+
const identity = osmFilters && !isVehicle ? await fetchIdentity(provider, value) : undefined;
56+
3857
// Providers without an OSM equivalent (webcams, scooters, …) skip the
3958
// Overpass lookup and return an address-only Place so the panel can
4059
// still show something sensible. Free-floating vehicles take the same
4160
// path even when the provider's stations would normally snap to OSM.
4261
let place =
4362
osmFilters && !isVehicle
44-
? await lookupByOsmFilters(lat, lng, osmFilters, `${scheme}:${value}`)
63+
? await lookupByOsmFilters(lat, lng, osmFilters, `${scheme}:${value}`, identity)
4564
: null;
4665

4766
// Skip the reverse-geocode fallback when the caller already has an

integrations/ev-charging/providers/station-mapper.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,26 @@
1-
import type { DataSourceDetail, DataSourceDetailSection, DataSourceResult } from "@openmapx/core";
1+
import type {
2+
DataSourceDetail,
3+
DataSourceDetailSection,
4+
DataSourceResult,
5+
OsmIdentity,
6+
} from "@openmapx/core";
27
import type { EvChargingConnector, EvChargingStation } from "@openmapx/mobility-core/ev-charging";
38

9+
function stationIdentity(station: EvChargingStation): OsmIdentity | undefined {
10+
const operator = station.operator?.name;
11+
const legal = station.operator?.legalName;
12+
if (!operator && !legal) return undefined;
13+
const identity: OsmIdentity = {};
14+
if (operator) {
15+
identity.operator = operator;
16+
identity.brand = operator;
17+
}
18+
if (legal && legal !== operator) {
19+
identity.network = legal;
20+
}
21+
return identity;
22+
}
23+
424
function getMaxPower(station: EvChargingStation): number {
525
return station.connectors.reduce((max, connector) => {
626
if (connector.powerKw && connector.powerKw > max) return connector.powerKw;
@@ -138,6 +158,7 @@ export function mapStationToDetail(station: EvChargingStation): DataSourceDetail
138158
sources: station.sources,
139159
name: station.name,
140160
coordinates: station.coordinates,
161+
identity: stationIdentity(station),
141162
attributions: station.attributions,
142163
address: station.address,
143164
operator: station.operator,

integrations/fuel/providers/mapper.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,17 @@
1-
import type { DataSourceDetail, DataSourceDetailSection, DataSourceResult } from "@openmapx/core";
1+
import type {
2+
DataSourceDetail,
3+
DataSourceDetailSection,
4+
DataSourceResult,
5+
OsmIdentity,
6+
} from "@openmapx/core";
27
import type { FuelStation } from "@openmapx/mobility-core/fuel";
38
import opening_hours from "opening_hours";
49

10+
function stationIdentity(station: FuelStation): OsmIdentity | undefined {
11+
if (!station.brand) return undefined;
12+
return { brand: station.brand, operator: station.brand };
13+
}
14+
515
interface OpeningTime {
616
text: string;
717
start: string;
@@ -103,6 +113,7 @@ export function mapFuelStationToDetail(station: FuelStation): DataSourceDetail {
103113
sources: [extractSourcePrefix(station.id)],
104114
name: station.name,
105115
coordinates: station.coordinates,
116+
identity: stationIdentity(station),
106117
address: station.address ? { line1: station.address } : undefined,
107118
operator: station.brand ? { name: station.brand } : undefined,
108119
sections,
@@ -185,6 +196,7 @@ export function buildTankerkoenigDetail(
185196
sources: ["tankerkoenig"],
186197
name: station.name,
187198
coordinates: station.coordinates,
199+
identity: stationIdentity(station),
188200
address: station.address ? { line1: station.address } : undefined,
189201
operator: station.brand ? { name: station.brand } : undefined,
190202
openingHours: tankerkoenigToOsmHours(tankerkoenigRaw),

0 commit comments

Comments
 (0)