fix(integrations): type the Site Manager transport boundary and guard it
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
cc4641afd7
commit
bfbba5782a
3 files changed
+59
-7
No files matched your search
+48
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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<Response>;
|
||||
|
||||
export class UnifiSiteManagerApi {
|
||||
|
||||
@@ -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<typeof platformFetch>[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 = <T>(result: PromiseSettledResult<T>, 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<TestingResult> {
|
||||
try {
|
||||
await this.createApi(fetchCloudAsync).getSitesAsync();
|
||||
|
||||
Reference in new issue
Block a user