From ad69ec1d81a7d99c87c8876ee24cc46b3f326cd4 Mon Sep 17 00:00:00 2001 From: Florian Date: Fri, 30 Oct 2026 10:19:03 +0100 Subject: [PATCH 1/4] feat(health): probe platform backends where the deployment keeps them Health checks can name a platform backend as ${endpoint:}, resolved through the same resolver its client queries with: the self-hosted service, then the operator's URL, then the public default. The Overpass client uses that resolver, and the three Overpass health checks probe ${endpoint:overpass}/api/status instead of always pinging the public instance. The contract lists the endpoint ids and rejects others. The host now sends URL credentials as Basic auth for every probe and keeps them out of the displayed URL and error messages, and trusts the operator's own instance like a self-hosted service. --- .changeset/health-check-platform-endpoints.md | 6 + .../__tests__/integration-health.test.ts | 103 +++++++++++++++++- apps/api/src/services/integration-health.ts | 91 ++++++++++++++-- docs/docs/developer/integration-system.md | 26 ++++- integrations/overlay-hiking/manifest.json | 2 +- integrations/poi-overpass/manifest.json | 4 +- integrations/transit-overpass/manifest.json | 4 +- packages/core/src/utils/index.ts | 7 ++ packages/core/src/utils/overpass/client.ts | 32 +++--- .../core/src/utils/platform-endpoints.test.ts | 37 +++++++ packages/core/src/utils/platform-endpoints.ts | 61 +++++++++++ .../core/src/utils/url-credentials.test.ts | 22 ++++ packages/core/src/utils/url-credentials.ts | 41 +++++++ packages/extension-contracts/src/index.ts | 14 ++- .../extension-contracts/src/integration.ts | 38 ++++++- .../manifest-health-endpoints.test.ts | 41 +++++++ 16 files changed, 487 insertions(+), 42 deletions(-) create mode 100644 .changeset/health-check-platform-endpoints.md create mode 100644 packages/core/src/utils/platform-endpoints.test.ts create mode 100644 packages/core/src/utils/platform-endpoints.ts create mode 100644 packages/core/src/utils/url-credentials.test.ts create mode 100644 packages/core/src/utils/url-credentials.ts create mode 100644 packages/integration-framework/src/__tests__/manifest-health-endpoints.test.ts diff --git a/.changeset/health-check-platform-endpoints.md b/.changeset/health-check-platform-endpoints.md new file mode 100644 index 000000000..af2787611 --- /dev/null +++ b/.changeset/health-check-platform-endpoints.md @@ -0,0 +1,6 @@ +--- +"@openmapx/extension-contracts": minor +"@openmapx/extension-cli": patch +--- + +Accept `${endpoint:}` in integration health-check URL templates and headers: the instance of a platform backend that the platform's own client queries (the self-hosted service, then the operator's URL, then the public default). `HEALTH_CHECK_ENDPOINT_IDS` lists the ids, currently `overpass`, and validation rejects any other. Rebuild the CLI against the updated portable contracts. diff --git a/apps/api/src/services/__tests__/integration-health.test.ts b/apps/api/src/services/__tests__/integration-health.test.ts index f8c7f49b2..f3ee0e142 100644 --- a/apps/api/src/services/__tests__/integration-health.test.ts +++ b/apps/api/src/services/__tests__/integration-health.test.ts @@ -10,6 +10,7 @@ const coreServerMocks = vi.hoisted(() => ({ safeFetchStatus: vi.fn(), })); const dnsMocks = vi.hoisted(() => ({ lookup: vi.fn() })); +const serviceUrls = vi.hoisted(() => new Map()); const netMocks = vi.hoisted(() => ({ createConnection: vi.fn() })); vi.mock("@openmapx/core/server", () => coreServerMocks); @@ -23,7 +24,7 @@ vi.mock("node:dns/promises", () => dnsMocks); vi.mock("../service-registry", () => ({ serviceUrl: (id: string) => { if (id === "valhalla") return "http://valhalla:8002"; - return undefined; + return serviceUrls.get(id); }, })); @@ -553,6 +554,106 @@ describe("integration-health placeholder substitution", () => { }); }); +// biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax +describe("integration-health ${endpoint:...} placeholders", () => { + let fetchSpy: ReturnType; + + beforeEach(() => { + fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 200 })); + }); + + afterEach(() => { + serviceUrls.clear(); + vi.unstubAllEnvs(); + vi.restoreAllMocks(); + }); + + function manifestIntegration(id: string): LoadedIntegration { + const manifest = JSON.parse( + readFileSync(resolve(REPO_ROOT, `integrations/${id}/manifest.json`), "utf8"), + ); + return { ...makeIntegration(manifest.healthCheck), id, manifest }; + } + + it.each([ + ["poi-overpass", "poi-overpass"], + ["transit-overpass", "transit-overpass"], + ["overlay-hiking", "overlay-hiking:OpenStreetMap Overpass"], + ])("%s probes the self-hosted Overpass when it is enabled", async (id, statusId) => { + vi.stubEnv("OVERPASS_URL", "https://overpass.example.org"); + serviceUrls.set("overpass", "http://overpass:80"); + + const statuses = await executeIntegrationHealthCheck(manifestIntegration(id)); + + expect(statuses.find((status) => status.id === statusId)).toMatchObject({ + status: "up", + url: "http://overpass:80/api/status", + }); + expect(coreServerMocks.safeFetchStatus).toHaveBeenCalledWith( + "http://overpass:80/api/status", + expect.objectContaining({ allowPrivateHosts: ["overpass"] }), + ); + expect(fetchSpy.mock.calls.map((call: unknown[]) => String(call[0]))).not.toContain( + "https://overpass-api.de/api/status", + ); + }); + + it("probes an operator's private instance, sending its URL credentials as Basic auth", async () => { + vi.stubEnv("OVERPASS_URL", "http://mapper:s3cret-pass@overpass.lan:8080/api/interpreter"); + + const [status] = await executeIntegrationHealthCheck(manifestIntegration("poi-overpass")); + + expect(status).toMatchObject({ status: "up", url: "http://overpass.lan:8080/api/status" }); + expect(coreServerMocks.safeFetchStatus).toHaveBeenCalledWith( + "http://overpass.lan:8080/api/status", + expect.objectContaining({ + allowPrivateHosts: ["overpass.lan"], + allowedRedirectOrigin: "http://overpass.lan:8080", + headers: expect.objectContaining({ + Authorization: `Basic ${Buffer.from("mapper:s3cret-pass").toString("base64")}`, + }), + }), + ); + }); + + it("never shows URL credentials, even in an error message that repeats them", async () => { + vi.stubEnv("OVERPASS_URL", "https://mapper:s3cret-pass@overpass.example.org"); + coreServerMocks.safeFetchStatus.mockRejectedValue( + new Error("request to https://mapper:s3cret-pass@overpass.example.org failed"), + ); + + const [status] = await executeIntegrationHealthCheck(manifestIntegration("poi-overpass")); + + expect(status?.status).toBe("down"); + expect(JSON.stringify(status)).not.toMatch(/s3cret-pass|mapper/); + }); + + it("probes the public default, as a public host, when nothing is configured", async () => { + vi.stubEnv("OVERPASS_URL", ""); + + const [status] = await executeIntegrationHealthCheck(manifestIntegration("transit-overpass")); + + expect(status?.url).toBe("https://overpass-api.de/api/status"); + expect(coreServerMocks.safeFetchStatus).toHaveBeenCalledWith( + "https://overpass-api.de/api/status", + expect.objectContaining({ allowPrivateHosts: [] }), + ); + }); + + it("reports an endpoint id it does not know as unconfigured, without a request", async () => { + const [status] = await executeIntegrationHealthCheck( + // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax + makeIntegration({ type: "http", urlTemplate: "${endpoint:nominatim}/status" }), + ); + + expect(status).toMatchObject({ + status: "unconfigured", + url: "nominatim endpoint not configured", + }); + expect(coreServerMocks.safeFetchStatus).not.toHaveBeenCalled(); + }); +}); + describe("integration-health secret redaction", () => { let fetchSpy: ReturnType; diff --git a/apps/api/src/services/integration-health.ts b/apps/api/src/services/integration-health.ts index 5eb4d346a..4cfda4437 100644 --- a/apps/api/src/services/integration-health.ts +++ b/apps/api/src/services/integration-health.ts @@ -1,5 +1,12 @@ import { createConnection } from "node:net"; -import { USER_AGENT, validatePublicUrl } from "@openmapx/core"; +import { + isHealthCheckEndpointId, + platformEndpointServiceId, + resolvePlatformEndpoint, + USER_AGENT, + urlCredentialsAsBasicAuth, + validatePublicUrl, +} from "@openmapx/core"; import { resolvePublicConnectionAddresses, safeFetchStatus } from "@openmapx/core/server"; import { type LoadedIntegration, toIntegrationMeta } from "@openmapx/integration-framework"; import { getSecretFields } from "../utils/validate-config-body"; @@ -123,6 +130,23 @@ function secretRedactor(integration: LoadedIntegration): (text: string) => strin }; } +/** `redact`, also masking `values` (each at least the redactable length). */ +function withRedactedValues( + redact: (text: string) => string, + values: string[], +): (text: string) => string { + const patterns = [...new Set(values)] + .filter((value) => value.length >= MIN_REDACTABLE_SECRET_LENGTH) + .sort((a, b) => b.length - a.length) + .map(encodedSecretPattern); + if (patterns.length === 0) return redact; + return (text) => { + let out = redact(text); + for (const pattern of patterns) out = out.replace(pattern, "***"); + return out; + }; +} + async function executeSingleHealthCheck( integration: LoadedIntegration, hc: { @@ -145,7 +169,7 @@ async function executeSingleHealthCheck( const name = hc.name ? `${toIntegrationMeta(integration).name} — ${hc.name}` : toIntegrationMeta(integration).name; - const redactSecrets = secretRedactor(integration); + let redactSecrets = secretRedactor(integration); // Skip the probe when any required config key is unresolved (no value from // defaults / DB / vault / env). The cascade is populated at integration @@ -258,7 +282,7 @@ async function executeSingleHealthCheck( } // URL template interpolation (fall back to static url if template produces a broken URL). - // Two placeholder syntaxes: + // Three placeholder syntaxes: // - `${configKey}` — resolves against the integration's resolved config // (same source used at request time). // - `${service:id}` — resolves to the URL of a self-hosted service in the @@ -266,10 +290,35 @@ async function executeSingleHealthCheck( // "unconfigured" rather than falling through to the static `url` (which // is typically the public-fallback endpoint a self-hosted operator // doesn't actually use). + // - `${endpoint:id}` — resolves to the instance of a platform backend its + // clients query: the self-hosted service, the operator's URL or the + // public default (`resolvePlatformEndpoint`). let checkUrl: string; let unsatisfiedService: string | null = null; + let unsatisfiedEndpoint: string | null = null; + // Hosts the deployment chose (a registered service or the operator's own + // instance URL), which may be internal; manifest-literal and config-derived + // hosts must be public. const serviceHosts = new Set(); function resolvePlaceholder(key: string): string { + if (key.startsWith("endpoint:")) { + const endpointId = key.slice("endpoint:".length); + const endpoint = isHealthCheckEndpointId(endpointId) + ? resolvePlatformEndpoint(endpointId, serviceUrl(platformEndpointServiceId(endpointId))) + : null; + if (!endpoint) { + unsatisfiedEndpoint = unsatisfiedEndpoint ?? endpointId; + return ""; + } + if (endpoint.source !== "default") { + try { + serviceHosts.add(new URL(endpoint.url).host); + } catch { + // An unparsable operator URL fails the probe's own URL handling below. + } + } + return endpoint.url; + } if (key.startsWith("service:")) { const serviceId = key.slice("service:".length); const url = serviceUrl(serviceId); @@ -299,6 +348,15 @@ async function executeSingleHealthCheck( status: "unconfigured", }; } + if (unsatisfiedEndpoint) { + return { + id, + name, + category, + url: `${unsatisfiedEndpoint} endpoint not configured`, + status: "unconfigured", + }; + } const hasHost = /^https?:\/\/[^/]/.test(resolved); checkUrl = hasHost ? resolved : ((hc.url as string | undefined) ?? resolved); } else if (hc.url) { @@ -307,6 +365,13 @@ async function executeSingleHealthCheck( return null; } + // URL credentials (an operator's `https://user:pass@host`) go as Basic auth + // and never reach the display URL or an error message. + const withoutCredentials = urlCredentialsAsBasicAuth(checkUrl); + checkUrl = withoutCredentials.url; + const credentialHeader = withoutCredentials.authorization; + redactSecrets = withRedactedValues(redactSecrets, withoutCredentials.secrets); + // Mask sensitive data in the display URL. The name-based pass keeps a // recognizable `key=***` shape for the common query-parameter form; the // value-based pass catches every other placement (path segment, userinfo, @@ -315,9 +380,9 @@ async function executeSingleHealthCheck( checkUrl.replace(/([?&](api_?key|key|apikey|app_key|token|access_token))=[^&]+/gi, "$1=***"), ); - // Interpolate `${configKey}` and `${service:id}` placeholders in headers - // (same resolution rules as urlTemplate — resolved config or service - // registry, not raw process.env). + // Interpolate `${configKey}`, `${service:id}` and `${endpoint:id}` + // placeholders in headers (same resolution rules as urlTemplate — resolved + // config, service registry or platform endpoint, not raw process.env). const rawHeaders = (hc.headers as Record) ?? {}; const interpolatedHeaders: Record = {}; for (const [k, v] of Object.entries(rawHeaders)) { @@ -325,11 +390,19 @@ async function executeSingleHealthCheck( resolvePlaceholder(key), ); } + // A manifest's own Authorization header takes precedence over URL credentials. + if ( + credentialHeader && + !Object.keys(interpolatedHeaders).some((key) => key.toLowerCase() === "authorization") + ) { + interpolatedHeaders.Authorization = credentialHeader; + } // SSRF guard: a manifest-controlled health-check URL must not target an - // internal/loopback/link-local host. Registered `service:` hosts are exempt - // (they are SUPPOSED to be internal); everything else (manifest-literal or - // ${config}-derived) is validated. Comparing the FINAL host against the set + // internal/loopback/link-local host. Registered `service:` hosts and the + // self-hosted or operator instance an `endpoint:` names are exempt (they + // are the deployment's own choice and may be internal); everything else + // (manifest-literal, ${config}-derived or a public default) is validated. Comparing the FINAL host against the set // of service-substituted hosts prevents smuggling an internal IP past the // guard by also referencing a service elsewhere in the template. let hostExempt = false; diff --git a/docs/docs/developer/integration-system.md b/docs/docs/developer/integration-system.md index 166b735ef..420c41f4b 100644 --- a/docs/docs/developer/integration-system.md +++ b/docs/docs/developer/integration-system.md @@ -256,18 +256,32 @@ interface HealthCheck { name?: string; type: "http" | "ping" | "tcp" | "custom"; url?: string; - urlTemplate?: string; // ${configKey} placeholders + urlTemplate?: string; // ${configKey}, ${service:}, ${endpoint:} headers?: Record; requiredConfigKeys?: string[]; // unconfigured if any is unset category?: string; } ``` -A `healthCheck` makes the integration probeable. `urlTemplate` and `headers` -interpolate `${configKey}` placeholders resolved through the config cascade, so a -probe can carry an API key without hard-coding it. If a key listed in -`requiredConfigKeys` is unset, the probe reports **unconfigured** rather than -attempting a request without the required configuration. +A `healthCheck` makes the integration probeable. Probe the instance the +integration actually queries, not a fixed public URL. `urlTemplate` and `headers` +interpolate three placeholders: + +| Placeholder | Resolves to | When it cannot resolve | +| ------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------- | +| `${configKey}` | The value from the config cascade, so a probe can carry an API key without hard-coding it. | Empty; list the key in `requiredConfigKeys` to report **unconfigured** instead. | +| `${service:}` | The URL of the enabled self-hosted service. | **Unconfigured**: the service is not enabled. | +| `${endpoint:}` | The instance of a platform backend that the platform's own client queries: the self-hosted service when enabled, then the operator's URL, then the public default. | **Unconfigured**: no instance is set and there is no public default. | + +Endpoint ids are fixed by the contract (`HEALTH_CHECK_ENDPOINT_IDS`), and +validation rejects any other. Today the only one is `overpass`: the self-hosted +`overpass` service, then `OVERPASS_URL`, then `overpass-api.de`. The Overpass +integrations probe `${endpoint:overpass}/api/status`. + +Self-hosted services and an operator's instance may be internal hosts. Every +other probe URL must be public. Credentials in a URL, such as an operator's +`https://user:pass@host`, are sent as Basic auth and never shown in the status +URL or an error. A manifest's own `Authorization` header takes precedence. Use cheap, static endpoints for third-party reachability checks. Set `healthCheckIntervalMs` to a considerate background cadence (Mangrove uses 900000 ms, or 15 minutes). The scheduler reuses cached results between probes diff --git a/integrations/overlay-hiking/manifest.json b/integrations/overlay-hiking/manifest.json index b89a62f1d..d8227821a 100644 --- a/integrations/overlay-hiking/manifest.json +++ b/integrations/overlay-hiking/manifest.json @@ -58,7 +58,7 @@ { "name": "OpenStreetMap Overpass", "type": "http", - "url": "https://overpass-api.de/api/status" + "urlTemplate": "${endpoint:overpass}/api/status" } ], "quality": "built-in", diff --git a/integrations/poi-overpass/manifest.json b/integrations/poi-overpass/manifest.json index e78113263..485755a41 100644 --- a/integrations/poi-overpass/manifest.json +++ b/integrations/poi-overpass/manifest.json @@ -27,8 +27,8 @@ } }, "healthCheck": { - "type": "ping", - "url": "https://overpass-api.de/api/status" + "type": "http", + "urlTemplate": "${endpoint:overpass}/api/status" }, "quality": "built-in", "category": "Geocoding", diff --git a/integrations/transit-overpass/manifest.json b/integrations/transit-overpass/manifest.json index 915efd70c..656d9253a 100644 --- a/integrations/transit-overpass/manifest.json +++ b/integrations/transit-overpass/manifest.json @@ -24,8 +24,8 @@ "routes": false }, "healthCheck": { - "type": "ping", - "url": "https://overpass-api.de/api/status" + "type": "http", + "urlTemplate": "${endpoint:overpass}/api/status" }, "dataSources": [ { diff --git a/packages/core/src/utils/index.ts b/packages/core/src/utils/index.ts index 6c8b13308..8ccbddaad 100644 --- a/packages/core/src/utils/index.ts +++ b/packages/core/src/utils/index.ts @@ -278,6 +278,12 @@ export { } from "./parseCoordinates"; export { formatPaymentMethodLabel } from "./paymentMethods"; export { withPhotoContributors } from "./photo-contributors"; +export { + isHealthCheckEndpointId, + platformEndpointServiceId, + type ResolvedPlatformEndpoint, + resolvePlatformEndpoint, +} from "./platform-endpoints"; export { computePlusCode, decodePlusCode, @@ -435,6 +441,7 @@ export { TRAFFIC_BAND_COLORS, type TrafficBand, } from "./trafficSeverity"; +export { type UrlWithoutCredentials, urlCredentialsAsBasicAuth } from "./url-credentials"; export { contactDomain, USER_AGENT, diff --git a/packages/core/src/utils/overpass/client.ts b/packages/core/src/utils/overpass/client.ts index 1b90d15c7..f1f2eda48 100644 --- a/packages/core/src/utils/overpass/client.ts +++ b/packages/core/src/utils/overpass/client.ts @@ -1,11 +1,12 @@ import { type Attribution, OPENSTREETMAP_ATTRIBUTION } from "@openmapx/mobility-core/attribution"; import { snapshotContributors } from "@openmapx/mobility-core/contributors"; +import { type ResolvedPlatformEndpoint, resolvePlatformEndpoint } from "../platform-endpoints"; import { USER_AGENT } from "../userAgent"; import type { OverpassResponse } from "./types"; -const DEFAULT_OVERPASS_URL = "https://overpass-api.de/api/interpreter"; const OVERPASS_FALLBACK_URL = "https://overpass.kumi.systems/api/interpreter"; -let configuredOverpassUrl: string | null = null; +/** The enabled self-hosted service's URL, which the integrations set when they activate. */ +let selfHostedOverpassUrl: string | null = null; let captureSupplier: (url: string) => readonly Attribution[] = () => [OPENSTREETMAP_ATTRIBUTION]; @@ -16,20 +17,15 @@ export function setOverpassContributorResolver( captureSupplier = resolve ?? (() => [OPENSTREETMAP_ATTRIBUTION]); } -function normalizeOverpassUrl(url: string): string { - const trimmed = url.replace(/\/$/, ""); - return trimmed.endsWith("/api/interpreter") ? trimmed : `${trimmed}/api/interpreter`; -} - -function currentOverpassUrl(): string { - if (configuredOverpassUrl) return configuredOverpassUrl; - const envUrl = process.env.OVERPASS_URL?.trim(); - return envUrl ? normalizeOverpassUrl(envUrl) : DEFAULT_OVERPASS_URL; +/** The instance queries go to, resolved as `${endpoint:overpass}` health checks resolve it. */ +function currentOverpassEndpoint(): ResolvedPlatformEndpoint { + const endpoint = resolvePlatformEndpoint("overpass", selfHostedOverpassUrl); + if (!endpoint) throw new Error("Overpass has a public default endpoint"); + return endpoint; } export function setOverpassUrl(url: string | undefined): void { - const trimmed = url?.trim(); - configuredOverpassUrl = trimmed && trimmed.length > 0 ? normalizeOverpassUrl(trimmed) : null; + selfHostedOverpassUrl = url?.trim() || null; } export class OverpassRateLimitError extends Error { @@ -185,16 +181,16 @@ export function isOverpassRuntimeLimit(remark: string | undefined): boolean { * When a custom OVERPASS_URL is set (e.g. local instance), the fallback is skipped. */ export async function overpassQuery(query: string): Promise { + const endpoint = currentOverpassEndpoint(); try { - return await fetchOverpass(currentOverpassUrl(), query); + return await fetchOverpass(`${endpoint.url}/api/interpreter`, query); } catch (err) { // Only fall back to the public mirror when using the default public endpoint. - // A custom OVERPASS_URL means a local/private instance — no point falling back - // to a different public server. - const usingCustomUrl = configuredOverpassUrl !== null || !!process.env.OVERPASS_URL; + // A self-hosted or operator instance is the deployment's choice — no point + // falling back to a different public server. const isFallbackable = err instanceof OverpassRateLimitError || err instanceof OverpassUnavailableError; - if (!usingCustomUrl && isFallbackable) { + if (endpoint.source === "default" && isFallbackable) { return fetchOverpass(OVERPASS_FALLBACK_URL, query); } throw err; diff --git a/packages/core/src/utils/platform-endpoints.test.ts b/packages/core/src/utils/platform-endpoints.test.ts new file mode 100644 index 000000000..d090c156b --- /dev/null +++ b/packages/core/src/utils/platform-endpoints.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "vitest"; +import { + isHealthCheckEndpointId, + platformEndpointServiceId, + resolvePlatformEndpoint, +} from "./platform-endpoints"; + +describe("resolvePlatformEndpoint", () => { + it("takes the enabled self-hosted service ahead of the operator's URL", () => { + expect( + resolvePlatformEndpoint("overpass", "http://overpass:80", { + OVERPASS_URL: "https://overpass.example.org", + }), + ).toEqual({ url: "http://overpass:80", source: "service" }); + }); + + it("takes the operator's URL without its API route or trailing slash", () => { + expect( + resolvePlatformEndpoint("overpass", null, { + OVERPASS_URL: " https://overpass.example.org/api/interpreter/ ", + }), + ).toEqual({ url: "https://overpass.example.org", source: "operator" }); + }); + + it("falls back to the public instance when nothing is configured", () => { + expect(resolvePlatformEndpoint("overpass", undefined, { OVERPASS_URL: " " })).toEqual({ + url: "https://overpass-api.de", + source: "default", + }); + }); + + it("knows only the endpoints the contract names", () => { + expect(isHealthCheckEndpointId("overpass")).toBe(true); + expect(isHealthCheckEndpointId("nominatim")).toBe(false); + expect(platformEndpointServiceId("overpass")).toBe("overpass"); + }); +}); diff --git a/packages/core/src/utils/platform-endpoints.ts b/packages/core/src/utils/platform-endpoints.ts new file mode 100644 index 000000000..4dce93494 --- /dev/null +++ b/packages/core/src/utils/platform-endpoints.ts @@ -0,0 +1,61 @@ +import type { HealthCheckEndpointId } from "@openmapx/extension-contracts"; + +export { isHealthCheckEndpointId } from "@openmapx/extension-contracts"; + +/** + * A platform backend that API-side clients query wherever the deployment + * keeps it: its self-hosted service, an operator's instance, or a public one. + */ +interface PlatformEndpointDefinition { + /** The built-in service that self-hosts it. */ + serviceId: string; + /** The operator's URL for an instance elsewhere. */ + envVar: string; + /** The public instance used when neither is set; null when there is none. */ + publicDefault: string | null; + /** An API route an operator's URL may name, which is not part of the instance's base URL. */ + routeSuffix?: string; +} + +const PLATFORM_ENDPOINTS: Readonly> = { + overpass: { + serviceId: "overpass", + envVar: "OVERPASS_URL", + publicDefault: "https://overpass-api.de", + routeSuffix: "/api/interpreter", + }, +}; + +export interface ResolvedPlatformEndpoint { + /** The instance's base URL, without a trailing slash or API route. */ + url: string; + source: "service" | "operator" | "default"; +} + +/** The built-in service that self-hosts `id`. */ +export function platformEndpointServiceId(id: HealthCheckEndpointId): string { + return PLATFORM_ENDPOINTS[id].serviceId; +} + +/** + * The instance requests for `id` go to: the self-hosted service when it is + * enabled (`selfHostedUrl`), then the operator's URL, then the public default. + * The platform's clients and health checks both resolve through this, so a + * health check always probes the instance actually queried. + */ +export function resolvePlatformEndpoint( + id: HealthCheckEndpointId, + selfHostedUrl: string | null | undefined, + env: Record = process.env, +): ResolvedPlatformEndpoint | null { + const definition = PLATFORM_ENDPOINTS[id]; + const base = (url: string) => { + const trimmed = url.trim().replace(/\/+$/, ""); + const suffix = definition.routeSuffix; + return suffix && trimmed.endsWith(suffix) ? trimmed.slice(0, -suffix.length) : trimmed; + }; + if (selfHostedUrl?.trim()) return { url: base(selfHostedUrl), source: "service" }; + const operatorUrl = env[definition.envVar]?.trim(); + if (operatorUrl) return { url: base(operatorUrl), source: "operator" }; + return definition.publicDefault ? { url: definition.publicDefault, source: "default" } : null; +} diff --git a/packages/core/src/utils/url-credentials.test.ts b/packages/core/src/utils/url-credentials.test.ts new file mode 100644 index 000000000..7fa44a474 --- /dev/null +++ b/packages/core/src/utils/url-credentials.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from "vitest"; +import { urlCredentialsAsBasicAuth } from "./url-credentials"; + +describe("urlCredentialsAsBasicAuth", () => { + it("moves user info into a Basic header, decoding it as UTF-8", () => { + expect( + urlCredentialsAsBasicAuth("https://m%C3%BCller:p%40ss@overpass.example.org/api"), + ).toEqual({ + url: "https://overpass.example.org/api", + authorization: `Basic ${Buffer.from("müller:p@ss", "utf8").toString("base64")}`, + secrets: ["müller", "p@ss", "m%C3%BCller", "p%40ss"], + }); + }); + + it("leaves a URL without credentials, and a non-URL, unchanged", () => { + expect(urlCredentialsAsBasicAuth("http://overpass:80/api/status")).toEqual({ + url: "http://overpass:80/api/status", + secrets: [], + }); + expect(urlCredentialsAsBasicAuth("/api/status")).toEqual({ url: "/api/status", secrets: [] }); + }); +}); diff --git a/packages/core/src/utils/url-credentials.ts b/packages/core/src/utils/url-credentials.ts new file mode 100644 index 000000000..72984ae3a --- /dev/null +++ b/packages/core/src/utils/url-credentials.ts @@ -0,0 +1,41 @@ +export interface UrlWithoutCredentials { + /** The URL without its user info. */ + url: string; + /** A Basic `Authorization` header value for the removed credentials. */ + authorization?: string; + /** The removed values, decoded and as written, for redacting them from text. */ + secrets: string[]; +} + +/** Base64 of the text's UTF-8 bytes, as Basic authorization encodes credentials. */ +function base64Utf8(text: string): string { + let binary = ""; + for (const byte of new TextEncoder().encode(text)) binary += String.fromCharCode(byte); + return btoa(binary); +} + +/** + * Move a URL's user info (`https://user:pass@host`) into a Basic + * authorization header. `fetch` refuses a URL carrying credentials and names + * the whole URL, password included, in its error. A string that is not a URL + * is returned unchanged. + */ +export function urlCredentialsAsBasicAuth(url: string): UrlWithoutCredentials { + let parsed: URL; + try { + parsed = new URL(url); + } catch { + return { url, secrets: [] }; + } + if (!parsed.username && !parsed.password) return { url, secrets: [] }; + const user = decodeURIComponent(parsed.username); + const password = decodeURIComponent(parsed.password); + const secrets = [user, password, parsed.username, parsed.password].filter(Boolean); + parsed.username = ""; + parsed.password = ""; + return { + url: parsed.href, + authorization: `Basic ${base64Utf8(`${user}:${password}`)}`, + secrets: [...new Set(secrets)], + }; +} diff --git a/packages/extension-contracts/src/index.ts b/packages/extension-contracts/src/index.ts index 7f4146d59..7f8628a60 100644 --- a/packages/extension-contracts/src/index.ts +++ b/packages/extension-contracts/src/index.ts @@ -13,8 +13,18 @@ export { extensionManifestSchema, validateExtensionManifest, } from "./extension"; -export type { IntegrationDataSource, IntegrationManifest } from "./integration"; -export { integrationManifestSchema, validateDataSource, validateManifest } from "./integration"; +export type { + HealthCheckEndpointId, + IntegrationDataSource, + IntegrationManifest, +} from "./integration"; +export { + HEALTH_CHECK_ENDPOINT_IDS, + integrationManifestSchema, + isHealthCheckEndpointId, + validateDataSource, + validateManifest, +} from "./integration"; export { PLATFORM_VERSION, satisfiesPlatformVersion } from "./platform"; export type { ManifestProvenance, ServiceManifest } from "./service"; export { serviceManifestSchema, validateServiceManifest } from "./service"; diff --git a/packages/extension-contracts/src/integration.ts b/packages/extension-contracts/src/integration.ts index ae27590cf..d6dbfecb4 100644 --- a/packages/extension-contracts/src/integration.ts +++ b/packages/extension-contracts/src/integration.ts @@ -170,6 +170,26 @@ export function readCredentialSetup(propertyDef: unknown): CredentialSetup | und return parsed.success ? parsed.data : undefined; } +/** + * Platform backends a health check can probe as `${endpoint:}`. Each + * resolves to the instance the platform's own client for it queries: the + * self-hosted service when it is enabled, then the operator's URL, then the + * public default. + */ +export const HEALTH_CHECK_ENDPOINT_IDS = ["overpass"] as const; +export type HealthCheckEndpointId = (typeof HEALTH_CHECK_ENDPOINT_IDS)[number]; + +export function isHealthCheckEndpointId(value: string): value is HealthCheckEndpointId { + return (HEALTH_CHECK_ENDPOINT_IDS as readonly string[]).includes(value); +} + +/** `${endpoint:}` placeholders in `text` naming no known platform endpoint. */ +function unknownEndpointPlaceholders(text: string): string[] { + return [...text.matchAll(/\$\{endpoint:([^}]*)\}/g)] + .map((match) => match[1] ?? "") + .filter((id) => !isHealthCheckEndpointId(id)); +} + const healthCheckSchema = z.object({ name: z.string().optional(), type: z.enum(["http", "ping", "tcp", "custom"]), @@ -178,7 +198,9 @@ const healthCheckSchema = z.object({ * Template with `${configKey}` placeholders resolved from the integration's * configSchema cascade (defaults → database → vault → env). Use this when * the probe needs an API key or similar; the placeholder name must match a - * key declared in `configSchema.properties`. + * key declared in `configSchema.properties`. `${service:}` is a + * self-hosted service's URL, and `${endpoint:}` the instance of a + * platform backend in use (`HEALTH_CHECK_ENDPOINT_IDS`). */ urlTemplate: z.string().optional(), /** Same substitution rules as `urlTemplate`. */ @@ -464,6 +486,20 @@ export function validateManifest(raw: unknown): ManifestValidationResult { "manifest.healthCheck is required for integrations with infrastructure dependencies (requires)", ); } + const healthChecks = manifest.healthCheck + ? Array.isArray(manifest.healthCheck) + ? manifest.healthCheck + : [manifest.healthCheck] + : []; + for (const check of healthChecks) { + for (const text of [check.urlTemplate ?? "", ...Object.values(check.headers ?? {})]) { + for (const id of unknownEndpointPlaceholders(text)) { + errors.push( + `manifest.healthCheck: unknown endpoint "${id}" in \${endpoint:${id}} (known: ${HEALTH_CHECK_ENDPOINT_IDS.join(", ")})`, + ); + } + } + } if (manifest.runtimeDataSources && manifest.dataSources?.length) { errors.push( "manifest.dataSources must be empty when runtimeDataSources is set — the integration supplies its sources through ctx.setDataSources()", diff --git a/packages/integration-framework/src/__tests__/manifest-health-endpoints.test.ts b/packages/integration-framework/src/__tests__/manifest-health-endpoints.test.ts new file mode 100644 index 000000000..fc5394f99 --- /dev/null +++ b/packages/integration-framework/src/__tests__/manifest-health-endpoints.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from "vitest"; +import { validateManifest } from "../manifest"; + +function withHealthCheck(healthCheck: unknown) { + return validateManifest({ + id: "poi-overpass", + name: "Overpass POI Search", + domains: ["poi-search"], + healthCheck, + }); +} + +describe("integration manifest health-check endpoints", () => { + it("accepts a known platform endpoint in a URL template", () => { + expect( + // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax + withHealthCheck({ type: "http", urlTemplate: "${endpoint:overpass}/api/status" }).valid, + ).toBe(true); + }); + + it("rejects an unknown endpoint in a URL template or a header, naming the known ones", () => { + const result = withHealthCheck([ + // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax + { name: "a", type: "http", urlTemplate: "${endpoint:nominatim}/status" }, + { + name: "b", + type: "http", + url: "https://example.org/status", + // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax + headers: { "X-Upstream": "${endpoint:}" }, + }, + ]); + expect(result.valid).toBe(false); + expect(result.errors).toEqual([ + // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax + 'manifest.healthCheck: unknown endpoint "nominatim" in ${endpoint:nominatim} (known: overpass)', + // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax + 'manifest.healthCheck: unknown endpoint "" in ${endpoint:} (known: overpass)', + ]); + }); +}); From a56628975fea1542d7594ef33785d7296c812487 Mon Sep 17 00:00:00 2001 From: Florian Date: Fri, 30 Oct 2026 10:37:51 +0100 Subject: [PATCH 2/4] fix(health): keep a platform endpoint where the probe goes An endpoint can be an internal host carrying the operator's credentials. Allow ${endpoint:} only as the whole URL prefix of a built-in manifest's urlTemplate, never in headers; strip the operator's credentials before the endpoint enters the template, and exempt the host and send them as Basic auth only when the final URL is on the endpoint's own origin. --- .changeset/health-check-platform-endpoints.md | 2 +- .../__tests__/integration-health.test.ts | 57 +++++++++++- apps/api/src/services/integration-health.ts | 88 ++++++++++++++++--- docs/docs/developer/integration-system.md | 15 +++- packages/core/src/utils/index.ts | 1 + packages/core/src/utils/platform-endpoints.ts | 2 +- packages/extension-contracts/src/index.ts | 1 + .../extension-contracts/src/integration.ts | 32 +++++-- .../manifest-health-endpoints.test.ts | 46 +++++++--- 9 files changed, 201 insertions(+), 43 deletions(-) diff --git a/.changeset/health-check-platform-endpoints.md b/.changeset/health-check-platform-endpoints.md index af2787611..0d8a37fd0 100644 --- a/.changeset/health-check-platform-endpoints.md +++ b/.changeset/health-check-platform-endpoints.md @@ -3,4 +3,4 @@ "@openmapx/extension-cli": patch --- -Accept `${endpoint:}` in integration health-check URL templates and headers: the instance of a platform backend that the platform's own client queries (the self-hosted service, then the operator's URL, then the public default). `HEALTH_CHECK_ENDPOINT_IDS` lists the ids, currently `overpass`, and validation rejects any other. Rebuild the CLI against the updated portable contracts. +Accept `${endpoint:}` in integration health-check URL templates and headers: the instance of a platform backend that the platform's own client queries (the self-hosted service, then the operator's URL, then the public default). `HEALTH_CHECK_ENDPOINT_IDS` lists the ids, currently `overpass`. Validation rejects any other id, and an endpoint anywhere but as the whole URL prefix of `urlTemplate`, including in headers. Rebuild the CLI against the updated portable contracts. diff --git a/apps/api/src/services/__tests__/integration-health.test.ts b/apps/api/src/services/__tests__/integration-health.test.ts index f3ee0e142..d950f8923 100644 --- a/apps/api/src/services/__tests__/integration-health.test.ts +++ b/apps/api/src/services/__tests__/integration-health.test.ts @@ -572,9 +572,57 @@ describe("integration-health ${endpoint:...} placeholders", () => { const manifest = JSON.parse( readFileSync(resolve(REPO_ROOT, `integrations/${id}/manifest.json`), "utf8"), ); - return { ...makeIntegration(manifest.healthCheck), id, manifest }; + return { ...makeIntegration(manifest.healthCheck), id, manifest, isBuiltIn: true }; } + it.each([ + [ + "elsewhere in the URL", + // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax + { type: "http", urlTemplate: "https://collector.example/?u=${endpoint:overpass}" }, + ], + [ + "as a host prefix", + // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax + { type: "http", urlTemplate: "${endpoint:overpass}@collector.example/status" }, + ], + [ + "in a header", + { + type: "http", + url: "https://collector.example/status", + // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax + headers: { "X-Upstream": "${endpoint:overpass}" }, + }, + ], + ])("never sends the operator's endpoint %s", async (_where, healthCheck) => { + vi.stubEnv("OVERPASS_URL", "https://mapper:s3cret-pass@overpass.example.org"); + + const [status] = await executeIntegrationHealthCheck({ + ...makeIntegration(healthCheck), + isBuiltIn: true, + }); + + expect(status).toMatchObject({ status: "down", url: "health check misconfigured" }); + expect(coreServerMocks.safeFetchStatus).not.toHaveBeenCalled(); + expect(JSON.stringify(status)).not.toMatch(/s3cret-pass|mapper/); + }); + + it("does not resolve an endpoint for a community integration", async () => { + serviceUrls.set("overpass", "http://overpass:80"); + + const [status] = await executeIntegrationHealthCheck( + // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax + makeIntegration({ type: "http", urlTemplate: "${endpoint:overpass}/api/status" }), + ); + + expect(status).toMatchObject({ + status: "down", + error: "endpoint placeholders are reserved for built-in integrations", + }); + expect(coreServerMocks.safeFetchStatus).not.toHaveBeenCalled(); + }); + it.each([ ["poi-overpass", "poi-overpass"], ["transit-overpass", "transit-overpass"], @@ -641,10 +689,11 @@ describe("integration-health ${endpoint:...} placeholders", () => { }); it("reports an endpoint id it does not know as unconfigured, without a request", async () => { - const [status] = await executeIntegrationHealthCheck( + const [status] = await executeIntegrationHealthCheck({ // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax - makeIntegration({ type: "http", urlTemplate: "${endpoint:nominatim}/status" }), - ); + ...makeIntegration({ type: "http", urlTemplate: "${endpoint:nominatim}/status" }), + isBuiltIn: true, + }); expect(status).toMatchObject({ status: "unconfigured", diff --git a/apps/api/src/services/integration-health.ts b/apps/api/src/services/integration-health.ts index 4cfda4437..3f6c82a45 100644 --- a/apps/api/src/services/integration-health.ts +++ b/apps/api/src/services/integration-health.ts @@ -1,5 +1,6 @@ import { createConnection } from "node:net"; import { + ENDPOINT_URL_TEMPLATE, isHealthCheckEndpointId, platformEndpointServiceId, resolvePlatformEndpoint, @@ -130,6 +131,15 @@ function secretRedactor(integration: LoadedIntegration): (text: string) => strin }; } +/** The endpoint a probe resolved: its origin, and the operator's credentials for it. */ +interface ProbeEndpoint { + origin: string; + /** The self-hosted or operator instance, which may be internal. */ + trusted: boolean; + authorization?: string; + secrets: string[]; +} + /** `redact`, also masking `values` (each at least the redactable length). */ function withRedactedValues( redact: (text: string) => string, @@ -296,13 +306,27 @@ async function executeSingleHealthCheck( let checkUrl: string; let unsatisfiedService: string | null = null; let unsatisfiedEndpoint: string | null = null; + // `${endpoint:id}` names the deployment's own instance, possibly internal and + // with the operator's credentials. A built-in manifest may use it only as the + // whole URL prefix; its credentials never enter the template and go only to + // that origin, as Basic auth. + let endpointMisuse: string | null = null; + let resolvedEndpoint: ProbeEndpoint | null = null; // Hosts the deployment chose (a registered service or the operator's own // instance URL), which may be internal; manifest-literal and config-derived // hosts must be public. const serviceHosts = new Set(); - function resolvePlaceholder(key: string): string { + function resolvePlaceholder(key: string, inHeader = false): string { if (key.startsWith("endpoint:")) { const endpointId = key.slice("endpoint:".length); + if (!integration.isBuiltIn) { + endpointMisuse = "endpoint placeholders are reserved for built-in integrations"; + return ""; + } + if (inHeader) { + endpointMisuse = "an endpoint placeholder may not appear in a header"; + return ""; + } const endpoint = isHealthCheckEndpointId(endpointId) ? resolvePlatformEndpoint(endpointId, serviceUrl(platformEndpointServiceId(endpointId))) : null; @@ -310,14 +334,20 @@ async function executeSingleHealthCheck( unsatisfiedEndpoint = unsatisfiedEndpoint ?? endpointId; return ""; } - if (endpoint.source !== "default") { - try { - serviceHosts.add(new URL(endpoint.url).host); - } catch { - // An unparsable operator URL fails the probe's own URL handling below. - } + const withoutCredentials = urlCredentialsAsBasicAuth(endpoint.url); + const base = withoutCredentials.url.replace(/\/$/, ""); + try { + resolvedEndpoint = { + origin: new URL(base).origin, + trusted: endpoint.source !== "default", + authorization: withoutCredentials.authorization, + secrets: withoutCredentials.secrets, + }; + } catch { + endpointMisuse = `the ${endpointId} endpoint URL is not a valid URL`; + return ""; } - return endpoint.url; + return base; } if (key.startsWith("service:")) { const serviceId = key.slice("service:".length); @@ -336,7 +366,11 @@ async function executeSingleHealthCheck( return resolveConfigValue(integration, key) ?? ""; } if (hc.urlTemplate) { - const resolved = (hc.urlTemplate as string).replace(/\$\{([\w:-]+)\}/g, (_, key: string) => + const template = hc.urlTemplate as string; + if (template.includes("${endpoint:") && !ENDPOINT_URL_TEMPLATE.test(template)) { + endpointMisuse = "an endpoint placeholder must be the whole URL prefix"; + } + const resolved = template.replace(/\$\{([\w:-]+)\}/g, (_, key: string) => resolvePlaceholder(key), ); if (unsatisfiedService) { @@ -365,12 +399,30 @@ async function executeSingleHealthCheck( return null; } - // URL credentials (an operator's `https://user:pass@host`) go as Basic auth - // and never reach the display URL or an error message. + // URL credentials go as Basic auth and never reach the display URL or an + // error message: a manifest's own `https://user:pass@host`, or the operator's + // endpoint credentials, which only ever go to that endpoint's origin. const withoutCredentials = urlCredentialsAsBasicAuth(checkUrl); checkUrl = withoutCredentials.url; - const credentialHeader = withoutCredentials.authorization; + let credentialHeader = withoutCredentials.authorization; redactSecrets = withRedactedValues(redactSecrets, withoutCredentials.secrets); + // Assigned inside resolvePlaceholder, which TypeScript does not follow. + const endpoint = resolvedEndpoint as ProbeEndpoint | null; + if (endpoint) { + redactSecrets = withRedactedValues(redactSecrets, endpoint.secrets); + let origin: string | null = null; + try { + origin = new URL(checkUrl).origin; + } catch { + origin = null; + } + if (origin !== endpoint.origin) { + endpointMisuse = endpointMisuse ?? "the probe URL leaves the endpoint's origin"; + } else { + credentialHeader = endpoint.authorization ?? credentialHeader; + if (endpoint.trusted) serviceHosts.add(new URL(checkUrl).host); + } + } // Mask sensitive data in the display URL. The name-based pass keeps a // recognizable `key=***` shape for the common query-parameter form; the @@ -387,9 +439,19 @@ async function executeSingleHealthCheck( const interpolatedHeaders: Record = {}; for (const [k, v] of Object.entries(rawHeaders)) { interpolatedHeaders[k] = v.replace(/\$\{([\w:-]+)\}/g, (_, key: string) => - resolvePlaceholder(key), + resolvePlaceholder(key, true), ); } + if (endpointMisuse) { + return { + id, + name, + category, + url: "health check misconfigured", + status: "down", + error: endpointMisuse, + }; + } // A manifest's own Authorization header takes precedence over URL credentials. if ( credentialHeader && diff --git a/docs/docs/developer/integration-system.md b/docs/docs/developer/integration-system.md index 420c41f4b..1b8545523 100644 --- a/docs/docs/developer/integration-system.md +++ b/docs/docs/developer/integration-system.md @@ -273,10 +273,17 @@ interpolate three placeholders: | `${service:}` | The URL of the enabled self-hosted service. | **Unconfigured**: the service is not enabled. | | `${endpoint:}` | The instance of a platform backend that the platform's own client queries: the self-hosted service when enabled, then the operator's URL, then the public default. | **Unconfigured**: no instance is set and there is no public default. | -Endpoint ids are fixed by the contract (`HEALTH_CHECK_ENDPOINT_IDS`), and -validation rejects any other. Today the only one is `overpass`: the self-hosted -`overpass` service, then `OVERPASS_URL`, then `overpass-api.de`. The Overpass -integrations probe `${endpoint:overpass}/api/status`. +Endpoint ids are fixed by the contract (`HEALTH_CHECK_ENDPOINT_IDS`); today the +only one is `overpass`: the self-hosted `overpass` service, then `OVERPASS_URL`, +then `overpass-api.de`. The Overpass integrations probe +`${endpoint:overpass}/api/status`. An endpoint can be an internal host carrying +the operator's credentials, so it is only ever where a probe goes: + +- it must be the whole URL prefix of `urlTemplate`, used once, followed by a + path or nothing; validation rejects it anywhere else, including headers; +- only built-in integrations resolve it; a community manifest's probe is marked + down; +- the operator's credentials go only to the endpoint's own origin. Self-hosted services and an operator's instance may be internal hosts. Every other probe URL must be public. Credentials in a URL, such as an operator's diff --git a/packages/core/src/utils/index.ts b/packages/core/src/utils/index.ts index 8ccbddaad..65a184d49 100644 --- a/packages/core/src/utils/index.ts +++ b/packages/core/src/utils/index.ts @@ -279,6 +279,7 @@ export { export { formatPaymentMethodLabel } from "./paymentMethods"; export { withPhotoContributors } from "./photo-contributors"; export { + ENDPOINT_URL_TEMPLATE, isHealthCheckEndpointId, platformEndpointServiceId, type ResolvedPlatformEndpoint, diff --git a/packages/core/src/utils/platform-endpoints.ts b/packages/core/src/utils/platform-endpoints.ts index 4dce93494..bf3862890 100644 --- a/packages/core/src/utils/platform-endpoints.ts +++ b/packages/core/src/utils/platform-endpoints.ts @@ -1,6 +1,6 @@ import type { HealthCheckEndpointId } from "@openmapx/extension-contracts"; -export { isHealthCheckEndpointId } from "@openmapx/extension-contracts"; +export { ENDPOINT_URL_TEMPLATE, isHealthCheckEndpointId } from "@openmapx/extension-contracts"; /** * A platform backend that API-side clients query wherever the deployment diff --git a/packages/extension-contracts/src/index.ts b/packages/extension-contracts/src/index.ts index 7f8628a60..b0abe43eb 100644 --- a/packages/extension-contracts/src/index.ts +++ b/packages/extension-contracts/src/index.ts @@ -19,6 +19,7 @@ export type { IntegrationManifest, } from "./integration"; export { + ENDPOINT_URL_TEMPLATE, HEALTH_CHECK_ENDPOINT_IDS, integrationManifestSchema, isHealthCheckEndpointId, diff --git a/packages/extension-contracts/src/integration.ts b/packages/extension-contracts/src/integration.ts index d6dbfecb4..df4f47534 100644 --- a/packages/extension-contracts/src/integration.ts +++ b/packages/extension-contracts/src/integration.ts @@ -183,6 +183,12 @@ export function isHealthCheckEndpointId(value: string): value is HealthCheckEndp return (HEALTH_CHECK_ENDPOINT_IDS as readonly string[]).includes(value); } +/** + * A URL template using `${endpoint:}` starts with it, followed by a path or + * nothing, and uses it once, so the probe stays on the endpoint's origin. + */ +export const ENDPOINT_URL_TEMPLATE = /^\$\{endpoint:[\w-]+\}(?:\/(?![\s\S]*\$\{endpoint:)|$)/; + /** `${endpoint:}` placeholders in `text` naming no known platform endpoint. */ function unknownEndpointPlaceholders(text: string): string[] { return [...text.matchAll(/\$\{endpoint:([^}]*)\}/g)] @@ -200,7 +206,8 @@ const healthCheckSchema = z.object({ * the probe needs an API key or similar; the placeholder name must match a * key declared in `configSchema.properties`. `${service:}` is a * self-hosted service's URL, and `${endpoint:}` the instance of a - * platform backend in use (`HEALTH_CHECK_ENDPOINT_IDS`). + * platform backend in use (`HEALTH_CHECK_ENDPOINT_IDS`): only as the whole + * URL prefix, never in headers, and only in built-in manifests. */ urlTemplate: z.string().optional(), /** Same substitution rules as `urlTemplate`. */ @@ -492,12 +499,23 @@ export function validateManifest(raw: unknown): ManifestValidationResult { : [manifest.healthCheck] : []; for (const check of healthChecks) { - for (const text of [check.urlTemplate ?? "", ...Object.values(check.headers ?? {})]) { - for (const id of unknownEndpointPlaceholders(text)) { - errors.push( - `manifest.healthCheck: unknown endpoint "${id}" in \${endpoint:${id}} (known: ${HEALTH_CHECK_ENDPOINT_IDS.join(", ")})`, - ); - } + const template = check.urlTemplate ?? ""; + for (const id of unknownEndpointPlaceholders(template)) { + errors.push( + `manifest.healthCheck: unknown endpoint "${id}" in \${endpoint:${id}} (known: ${HEALTH_CHECK_ENDPOINT_IDS.join(", ")})`, + ); + } + // The endpoint can be an internal host with the operator's credentials: + // it may only be where the probe goes, never data sent elsewhere. + if (template.includes("${endpoint:") && !ENDPOINT_URL_TEMPLATE.test(template)) { + errors.push( + // biome-ignore lint/suspicious/noTemplateCurlyInString: the message names the manifest syntax + "manifest.healthCheck: ${endpoint:} must be the whole URL prefix of urlTemplate, used once, followed by a path or nothing", + ); + } + if (Object.values(check.headers ?? {}).some((value) => value.includes("${endpoint:"))) { + // biome-ignore lint/suspicious/noTemplateCurlyInString: the message names the manifest syntax + errors.push("manifest.healthCheck: ${endpoint:} may not appear in headers"); } } if (manifest.runtimeDataSources && manifest.dataSources?.length) { diff --git a/packages/integration-framework/src/__tests__/manifest-health-endpoints.test.ts b/packages/integration-framework/src/__tests__/manifest-health-endpoints.test.ts index fc5394f99..19f74be38 100644 --- a/packages/integration-framework/src/__tests__/manifest-health-endpoints.test.ts +++ b/packages/integration-framework/src/__tests__/manifest-health-endpoints.test.ts @@ -18,24 +18,44 @@ describe("integration manifest health-check endpoints", () => { ).toBe(true); }); - it("rejects an unknown endpoint in a URL template or a header, naming the known ones", () => { - const result = withHealthCheck([ - // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax - { name: "a", type: "http", urlTemplate: "${endpoint:nominatim}/status" }, - { - name: "b", - type: "http", - url: "https://example.org/status", - // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax - headers: { "X-Upstream": "${endpoint:}" }, - }, - ]); + it("rejects an unknown endpoint, naming the known ones", () => { + // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax + const result = withHealthCheck({ type: "http", urlTemplate: "${endpoint:nominatim}/status" }); expect(result.valid).toBe(false); expect(result.errors).toEqual([ // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax 'manifest.healthCheck: unknown endpoint "nominatim" in ${endpoint:nominatim} (known: overpass)', + ]); + }); + + it.each([ + // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax + "https://collector.example/?u=${endpoint:overpass}", + // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax + "${endpoint:overpass}@collector.example/status", + // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax + "${endpoint:overpass}.collector.example/status", + // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax + "${endpoint:overpass}/status?u=${endpoint:overpass}", + ])("rejects an endpoint that is not the whole URL prefix: %s", (urlTemplate) => { + const result = withHealthCheck({ type: "http", urlTemplate }); + expect(result.valid).toBe(false); + expect(result.errors).toContain( + // biome-ignore lint/suspicious/noTemplateCurlyInString: error text names the syntax + "manifest.healthCheck: ${endpoint:} must be the whole URL prefix of urlTemplate, used once, followed by a path or nothing", + ); + }); + + it("rejects an endpoint in a header", () => { + const result = withHealthCheck({ + type: "http", + url: "https://collector.example/status", // biome-ignore lint/suspicious/noTemplateCurlyInString: manifest template syntax - 'manifest.healthCheck: unknown endpoint "" in ${endpoint:} (known: overpass)', + headers: { "X-Upstream": "${endpoint:overpass}" }, + }); + expect(result.errors).toEqual([ + // biome-ignore lint/suspicious/noTemplateCurlyInString: error text names the syntax + "manifest.healthCheck: ${endpoint:} may not appear in headers", ]); }); }); From cde0563983574095ef3e9bb0df4debba060a18de Mon Sep 17 00:00:00 2001 From: Florian Date: Fri, 30 Oct 2026 10:49:14 +0100 Subject: [PATCH 3/4] fix(core): trim endpoint slashes without a quadratic regex CodeQL flagged /\/+$/ on an operator URL as polynomial: strip trailing slashes with a loop. --- packages/core/src/utils/platform-endpoints.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/core/src/utils/platform-endpoints.ts b/packages/core/src/utils/platform-endpoints.ts index bf3862890..ba2add812 100644 --- a/packages/core/src/utils/platform-endpoints.ts +++ b/packages/core/src/utils/platform-endpoints.ts @@ -50,7 +50,9 @@ export function resolvePlatformEndpoint( ): ResolvedPlatformEndpoint | null { const definition = PLATFORM_ENDPOINTS[id]; const base = (url: string) => { - const trimmed = url.trim().replace(/\/+$/, ""); + let trimmed = url.trim(); + // A loop, not /\/+$/: that regex is quadratic on a long run of slashes. + while (trimmed.endsWith("/")) trimmed = trimmed.slice(0, -1); const suffix = definition.routeSuffix; return suffix && trimmed.endsWith(suffix) ? trimmed.slice(0, -suffix.length) : trimmed; }; From c24575640e2b763b0bad53398eb817d5d3aa0de5 Mon Sep 17 00:00:00 2001 From: Florian Date: Fri, 30 Oct 2026 11:00:47 +0100 Subject: [PATCH 4/4] fix(overpass): send OVERPASS_URL credentials as Basic auth (#686) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What changed and why Stacked on #685, whose core helper it reuses; the diff against #685's branch is this one commit. An operator can point OpenMapX at a hosted Overpass with `OVERPASS_URL`, including one that needs a login (`https://user:pass@host`). Every query to such an instance failed. Node's `fetch` refuses a URL that carries credentials, throwing `Request cannot be constructed from a URL that includes credentials: https://user:pass@host/…`, so the password also ended up in the error. The Overpass client now sends such credentials as a Basic `Authorization` header and queries the URL without them, through the same `urlCredentialsAsBasicAuth` helper the health checks use since #685. Attribution keeps resolving from the host, now taken from the URL without credentials. Instances without credentials are unaffected. ## Verification - **Query test:** a query to an `OVERPASS_URL` with credentials goes to `https://overpass.example.org/api/interpreter` with `Authorization: Basic …` for those credentials. The existing Overpass client tests still pass, including the public-mirror fallback. - **Node behaviour:** I confirmed that `fetch("https://u:p@example.com")` throws the error quoted above. - **Repository checks:** `pnpm lint`, `pnpm check-types` and `pnpm check:policy` pass. The full Vitest suite passes (1,953 files, 24,087 tests) except `motis-probe.test.ts`'s socket-retry test. That test is flaky: it failed 1 of 5 isolated reruns on this same code, and it drives a real socket through `node:http` in the data manager, which this change doesn't touch. - **Not run:** a query against a real credential-protected Overpass instance; none is configured anywhere I can reach. ## Release and upgrade impact Product impact: ordinary No package release: the change is limited to the Overpass client in the private `@openmapx/core` package; neither published package's source changes in this commit. --- packages/core/src/utils/overpass/client.test.ts | 15 +++++++++++++++ packages/core/src/utils/overpass/client.ts | 9 +++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/core/src/utils/overpass/client.test.ts b/packages/core/src/utils/overpass/client.test.ts index 5d27e0e40..227f90923 100644 --- a/packages/core/src/utils/overpass/client.test.ts +++ b/packages/core/src/utils/overpass/client.test.ts @@ -20,6 +20,21 @@ afterEach(() => { }); describe("overpassQuery", () => { + it("sends OVERPASS_URL credentials as Basic auth, which fetch refuses in the URL", async () => { + vi.stubEnv("OVERPASS_URL", "https://mapper:s3cret@overpass.example.org"); + const fetchMock = vi.fn( + async (_url: string, _init?: RequestInit) => + new Response(JSON.stringify({ elements: [] }), { status: 200 }), + ); + vi.stubGlobal("fetch", fetchMock); + + await expect(overpassQuery(QUERY)).resolves.toMatchObject({ elements: [] }); + expect(fetchMock.mock.calls[0]?.[0]).toBe("https://overpass.example.org/api/interpreter"); + expect(new Headers(fetchMock.mock.calls[0]?.[1]?.headers).get("Authorization")).toBe( + `Basic ${Buffer.from("mapper:s3cret").toString("base64")}`, + ); + }); + it("reports a server that does not answer in time as unavailable, not as a 500", async () => { setOverpassUrl("http://overpass.test"); vi.stubGlobal( diff --git a/packages/core/src/utils/overpass/client.ts b/packages/core/src/utils/overpass/client.ts index f1f2eda48..22b605497 100644 --- a/packages/core/src/utils/overpass/client.ts +++ b/packages/core/src/utils/overpass/client.ts @@ -1,6 +1,7 @@ import { type Attribution, OPENSTREETMAP_ATTRIBUTION } from "@openmapx/mobility-core/attribution"; import { snapshotContributors } from "@openmapx/mobility-core/contributors"; import { type ResolvedPlatformEndpoint, resolvePlatformEndpoint } from "../platform-endpoints"; +import { urlCredentialsAsBasicAuth } from "../url-credentials"; import { USER_AGENT } from "../userAgent"; import type { OverpassResponse } from "./types"; @@ -127,13 +128,17 @@ function overpassErrorPageError(text: string): Error { const OVERPASS_REQUEST_TIMEOUT_MS = 35_000; async function fetchOverpass(url: string, query: string): Promise { - const contributors = snapshotContributors(captureSupplier(url)); + // An operator's `https://user:pass@host` goes as Basic auth: fetch refuses + // a URL carrying credentials and names the whole URL in its error. + const target = urlCredentialsAsBasicAuth(url); + const contributors = snapshotContributors(captureSupplier(target.url)); try { - const res = await fetch(url, { + const res = await fetch(target.url, { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded", "User-Agent": USER_AGENT, + ...(target.authorization ? { Authorization: target.authorization } : {}), }, body: `data=${encodeURIComponent(query)}`, signal: AbortSignal.timeout(OVERPASS_REQUEST_TIMEOUT_MS),