From 4b7b7c5d1cab856a210755a1f8fd0d92cf449e26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zachary=20Gu=C3=A9not?= Date: Thu, 10 Sep 2026 10:48:30 +0200 Subject: [PATCH] fix(integrations): type the Site Manager transport boundary and guard it Co-Authored-By: Claude Opus 5 --- .../unifi-site-manager-integration.spec.ts | 48 +++++++++++++++++++ .../unifi-site-manager-api.ts | 6 +++ .../unifi-site-manager-integration.ts | 12 ++--- 3 files changed, 59 insertions(+), 7 deletions(-) diff --git a/packages/integrations/src/unifi-site-manager/test/unifi-site-manager-integration.spec.ts b/packages/integrations/src/unifi-site-manager/test/unifi-site-manager-integration.spec.ts index d99da37f8..e65b02754 100644 --- a/packages/integrations/src/unifi-site-manager/test/unifi-site-manager-integration.spec.ts +++ b/packages/integrations/src/unifi-site-manager/test/unifi-site-manager-integration.spec.ts @@ -7,8 +7,10 @@ vi.hoisted(() => { process.env.SECRET_ENCRYPTION_KEY = "ff3f4f7ce30e870c9630de9e5d244ffa81101a24ed0dfe5f064beb53a7e684f1"; }); +import { fetchWithTrustedCertificatesAsync } from "@homarr/core/infrastructure/http"; import * as undici from "undici"; +import type { IntegrationTestingInput } from "../../base/integration"; import type { IntegrationSecret } from "../../base/types"; import { UnifiSiteManagerIntegration } from "../unifi-site-manager-integration"; @@ -17,7 +19,16 @@ vi.mock("undici", async (importOriginal) => ({ fetch: vi.fn(), })); +// Mocked separately from `undici` on purpose: `fetchWithTrustedCertificatesAsync` +// also calls undici's `fetch` underneath, so mocking `undici` alone would +// intercept both transports and hide a regression that routes the Site +// Manager key through the trusted-certificate (admin CA bundle) transport. +vi.mock("@homarr/core/infrastructure/http", () => ({ + fetchWithTrustedCertificatesAsync: vi.fn(), +})); + const mockFetch = vi.mocked(undici.fetch); +const mockTrustedFetch = vi.mocked(fetchWithTrustedCertificatesAsync); const jsonResponse = (body: unknown, status = 200) => new Response(JSON.stringify(body), { status, headers: { "content-type": "application/json" } }) as never; @@ -72,6 +83,7 @@ const createIntegration = () => beforeEach(() => { mockFetch.mockReset(); + mockTrustedFetch.mockReset(); }); describe("UnifiSiteManagerIntegration getSiteOverviewAsync", () => { @@ -140,3 +152,39 @@ describe("UnifiSiteManagerIntegration getSiteOverviewAsync", () => { await expect(createIntegration().getSiteOverviewAsync()).resolves.toEqual({ sites: [] }); }); }); + +describe("UnifiSiteManagerIntegration cloud-only transport", () => { + type TestingFn = (input: IntegrationTestingInput) => Promise<{ success: boolean }>; + + /** + * `testingAsync` is protected; this bypasses that at runtime the same way + * the wud/gluetun specs do, so the test can call it directly instead of + * going through the DB-backed certificate lookup in `testConnectionAsync`. + */ + const invokeTesting = (integration: UnifiSiteManagerIntegration, input: IntegrationTestingInput) => { + const testing = (integration as unknown as { testingAsync: TestingFn }).testingAsync.bind(integration); + return testing(input); + }; + + /** + * Regression guard for the vulnerability this integration exists to avoid: + * if a future edit makes `testingAsync` destructure and use `fetchAsync` + * from its input (as the sibling `unifi-os` integration legitimately does), + * that `fetchAsync` is built from Homarr's trusted-certificate transport — + * the one that trusts every admin-uploaded CA — and this test catches it. + */ + test("never routes the Site Manager key through the trusted-certificate transport", async () => { + mockFetch.mockImplementation(async (url) => route(String(url))); + + await createIntegration().getSiteOverviewAsync(); + + // Stand-in for the real `input.fetchAsync`, which in production is built + // on top of `fetchWithTrustedCertificatesAsync`. If `testingAsync` used + // it, this mock would record the call. + const input = { fetchAsync: mockTrustedFetch } as unknown as IntegrationTestingInput; + const result = await invokeTesting(createIntegration(), input); + + expect(result.success).toBe(true); + expect(mockTrustedFetch).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/integrations/src/unifi-site-manager/unifi-site-manager-api.ts b/packages/integrations/src/unifi-site-manager/unifi-site-manager-api.ts index 21987f30b..068759798 100644 --- a/packages/integrations/src/unifi-site-manager/unifi-site-manager-api.ts +++ b/packages/integrations/src/unifi-site-manager/unifi-site-manager-api.ts @@ -1,4 +1,5 @@ import { ResponseError } from "@homarr/common/server"; +import type { RequestInit, Response } from "undici"; import type { z } from "zod/v4"; import { parseSiteManagerAsync } from "./unifi-site-manager-parse"; @@ -23,6 +24,11 @@ const REQUEST_TIMEOUT_MS = 15_000; */ export const MAX_SD_WAN_CONFIGS = 5; +/** + * Déclaré sur les types d'undici, pas sur ceux du DOM : le dépôt n'utilise que + * `undici` côté serveur. Aligner les deux évite les `as unknown as` qui + * désactivaient le contrôle de type à la frontière du transport. + */ export type SiteManagerFetch = (url: string, init?: RequestInit & { timeout?: number }) => Promise; export class UnifiSiteManagerApi { diff --git a/packages/integrations/src/unifi-site-manager/unifi-site-manager-integration.ts b/packages/integrations/src/unifi-site-manager/unifi-site-manager-integration.ts index e281bc482..1b9b25d52 100644 --- a/packages/integrations/src/unifi-site-manager/unifi-site-manager-integration.ts +++ b/packages/integrations/src/unifi-site-manager/unifi-site-manager-integration.ts @@ -24,15 +24,10 @@ import { buildSdWanIndex, mapSites } from "./unifi-site-manager-types"; */ const fetchCloudAsync: SiteManagerFetch = async (url, init) => { const { timeout, ...rest } = init ?? {}; - const response = await platformFetch(url, { + return await platformFetch(url, { ...rest, signal: timeout === undefined ? undefined : AbortSignal.timeout(timeout), - } as Parameters[1]); - // `SiteManagerFetch` (task 3) types its return as the ambient DOM `Response` - // (this file's lib includes `dom`); undici's own `Response` class is - // structurally close but not identical (its Headers iterator differs), so a - // type-only bridge is needed here. No behavior change: same object, same call. - return response as unknown as Response; + }); }; const settled = (result: PromiseSettledResult, fallback: T): T => @@ -43,6 +38,9 @@ export class UnifiSiteManagerIntegration extends Integration implements UnifiSit return await this.buildOverviewAsync(fetchCloudAsync); } + // `input.fetchAsync` is ignored on purpose: it carries the administrator's + // trusted-certificate bundle, and using it here would let that CA vouch for + // `api.ui.com` — see the module-level comment on `fetchCloudAsync`. protected async testingAsync(_input: IntegrationTestingInput): Promise { try { await this.createApi(fetchCloudAsync).getSitesAsync();