From b13d48987ccab4f7bf6007b2bbedeb650f6d4531 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 2 Mar 2026 16:17:31 +0000 Subject: [PATCH] refactor(gateway): unify control-ui and plugin webhook routing --- extensions/bluebubbles/src/monitor.ts | 20 +- .../src/monitor.webhook-route.test.ts | 44 +++ extensions/googlechat/src/monitor.ts | 20 +- .../src/monitor.webhook-routing.test.ts | 45 ++- extensions/zalo/src/monitor.ts | 20 +- extensions/zalo/src/monitor.webhook.test.ts | 24 ++ extensions/zalo/src/monitor.webhook.ts | 11 +- src/gateway/control-ui-http-utils.ts | 19 ++ src/gateway/control-ui-routing.test.ts | 54 ++++ src/gateway/control-ui-routing.ts | 45 +++ src/gateway/control-ui.ts | 115 ++++---- src/gateway/server-http.test-harness.ts | 268 +++++++++++++++++ src/gateway/server-http.ts | 237 +++++++++------ src/gateway/server.plugin-http-auth.test.ts | 274 +----------------- src/plugin-sdk/index.ts | 2 +- src/plugin-sdk/webhook-targets.test.ts | 53 ++++ src/plugin-sdk/webhook-targets.ts | 44 +++ 17 files changed, 870 insertions(+), 425 deletions(-) create mode 100644 extensions/bluebubbles/src/monitor.webhook-route.test.ts create mode 100644 src/gateway/control-ui-http-utils.ts create mode 100644 src/gateway/control-ui-routing.test.ts create mode 100644 src/gateway/control-ui-routing.ts create mode 100644 src/gateway/server-http.test-harness.ts diff --git a/extensions/bluebubbles/src/monitor.ts b/extensions/bluebubbles/src/monitor.ts index fa148e5dd20..af2d9a3faa0 100644 --- a/extensions/bluebubbles/src/monitor.ts +++ b/extensions/bluebubbles/src/monitor.ts @@ -4,6 +4,7 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk"; import { isRequestBodyLimitError, readRequestBodyWithLimit, + registerPluginHttpRoute, registerWebhookTarget, rejectNonPostWebhookRequest, requestBodyErrorToText, @@ -235,7 +236,24 @@ function removeDebouncer(target: WebhookTarget): void { } export function registerBlueBubblesWebhookTarget(target: WebhookTarget): () => void { - const registered = registerWebhookTarget(webhookTargets, target); + const registered = registerWebhookTarget(webhookTargets, target, { + onFirstPathTarget: ({ path }) => + registerPluginHttpRoute({ + path, + pluginId: "bluebubbles", + source: "bluebubbles-webhook", + accountId: target.account.accountId, + log: target.runtime.log, + handler: async (req, res) => { + const handled = await handleBlueBubblesWebhookRequest(req, res); + if (!handled && !res.headersSent) { + res.statusCode = 404; + res.setHeader("Content-Type", "text/plain; charset=utf-8"); + res.end("Not Found"); + } + }, + }), + }); return () => { registered.unregister(); // Clean up debouncer when target is unregistered diff --git a/extensions/bluebubbles/src/monitor.webhook-route.test.ts b/extensions/bluebubbles/src/monitor.webhook-route.test.ts new file mode 100644 index 00000000000..8499ea56b3d --- /dev/null +++ b/extensions/bluebubbles/src/monitor.webhook-route.test.ts @@ -0,0 +1,44 @@ +import type { OpenClawConfig } from "openclaw/plugin-sdk"; +import { afterEach, describe, expect, it } from "vitest"; +import { createEmptyPluginRegistry } from "../../../src/plugins/registry.js"; +import { setActivePluginRegistry } from "../../../src/plugins/runtime.js"; +import type { WebhookTarget } from "./monitor-shared.js"; +import { registerBlueBubblesWebhookTarget } from "./monitor.js"; + +function createTarget(): WebhookTarget { + return { + account: { accountId: "default" } as WebhookTarget["account"], + config: {} as OpenClawConfig, + runtime: {}, + core: {} as WebhookTarget["core"], + path: "/bluebubbles-webhook", + }; +} + +describe("registerBlueBubblesWebhookTarget", () => { + afterEach(() => { + setActivePluginRegistry(createEmptyPluginRegistry()); + }); + + it("registers and unregisters plugin HTTP route at path boundaries", () => { + const registry = createEmptyPluginRegistry(); + setActivePluginRegistry(registry); + + const unregisterA = registerBlueBubblesWebhookTarget(createTarget()); + const unregisterB = registerBlueBubblesWebhookTarget(createTarget()); + + expect(registry.httpRoutes).toHaveLength(1); + expect(registry.httpRoutes[0]).toEqual( + expect.objectContaining({ + pluginId: "bluebubbles", + path: "/bluebubbles-webhook", + source: "bluebubbles-webhook", + }), + ); + + unregisterA(); + expect(registry.httpRoutes).toHaveLength(1); + unregisterB(); + expect(registry.httpRoutes).toHaveLength(0); + }); +}); diff --git a/extensions/googlechat/src/monitor.ts b/extensions/googlechat/src/monitor.ts index e5249774de5..35f7eb6325e 100644 --- a/extensions/googlechat/src/monitor.ts +++ b/extensions/googlechat/src/monitor.ts @@ -5,6 +5,7 @@ import { createScopedPairingAccess, createReplyPrefixOptions, readJsonBodyWithLimit, + registerPluginHttpRoute, registerWebhookTarget, rejectNonPostWebhookRequest, isDangerousNameMatchingEnabled, @@ -100,7 +101,24 @@ function warnDeprecatedUsersEmailEntries( } export function registerGoogleChatWebhookTarget(target: WebhookTarget): () => void { - return registerWebhookTarget(webhookTargets, target).unregister; + return registerWebhookTarget(webhookTargets, target, { + onFirstPathTarget: ({ path }) => + registerPluginHttpRoute({ + path, + pluginId: "googlechat", + source: "googlechat-webhook", + accountId: target.account.accountId, + log: target.runtime.log, + handler: async (req, res) => { + const handled = await handleGoogleChatWebhookRequest(req, res); + if (!handled && !res.headersSent) { + res.statusCode = 404; + res.setHeader("Content-Type", "text/plain; charset=utf-8"); + res.end("Not Found"); + } + }, + }), + }).unregister; } function normalizeAudienceType(value?: string | null): GoogleChatAudienceType | undefined { diff --git a/extensions/googlechat/src/monitor.webhook-routing.test.ts b/extensions/googlechat/src/monitor.webhook-routing.test.ts index adf21bf98b3..f25d55c13b5 100644 --- a/extensions/googlechat/src/monitor.webhook-routing.test.ts +++ b/extensions/googlechat/src/monitor.webhook-routing.test.ts @@ -1,7 +1,9 @@ import { EventEmitter } from "node:events"; import type { IncomingMessage } from "node:http"; import type { OpenClawConfig, PluginRuntime } from "openclaw/plugin-sdk"; -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createEmptyPluginRegistry } from "../../../src/plugins/registry.js"; +import { setActivePluginRegistry } from "../../../src/plugins/runtime.js"; import { createMockServerResponse } from "../../../src/test-utils/mock-http-response.js"; import type { ResolvedGoogleChatAccount } from "./accounts.js"; import { verifyGoogleChatRequest } from "./auth.js"; @@ -86,6 +88,47 @@ function registerTwoTargets() { } describe("Google Chat webhook routing", () => { + afterEach(() => { + setActivePluginRegistry(createEmptyPluginRegistry()); + }); + + it("registers and unregisters plugin HTTP route at path boundaries", () => { + const registry = createEmptyPluginRegistry(); + setActivePluginRegistry(registry); + const unregisterA = registerGoogleChatWebhookTarget({ + account: baseAccount("A"), + config: {} as OpenClawConfig, + runtime: {}, + core: {} as PluginRuntime, + path: "/googlechat", + statusSink: vi.fn(), + mediaMaxMb: 5, + }); + const unregisterB = registerGoogleChatWebhookTarget({ + account: baseAccount("B"), + config: {} as OpenClawConfig, + runtime: {}, + core: {} as PluginRuntime, + path: "/googlechat", + statusSink: vi.fn(), + mediaMaxMb: 5, + }); + + expect(registry.httpRoutes).toHaveLength(1); + expect(registry.httpRoutes[0]).toEqual( + expect.objectContaining({ + pluginId: "googlechat", + path: "/googlechat", + source: "googlechat-webhook", + }), + ); + + unregisterA(); + expect(registry.httpRoutes).toHaveLength(1); + unregisterB(); + expect(registry.httpRoutes).toHaveLength(0); + }); + it("rejects ambiguous routing when multiple targets on the same path verify successfully", async () => { vi.mocked(verifyGoogleChatRequest).mockResolvedValue({ ok: true }); diff --git a/extensions/zalo/src/monitor.ts b/extensions/zalo/src/monitor.ts index 8d257508983..0332286ac9c 100644 --- a/extensions/zalo/src/monitor.ts +++ b/extensions/zalo/src/monitor.ts @@ -3,6 +3,7 @@ import type { MarkdownTableMode, OpenClawConfig, OutboundReplyPayload } from "op import { createScopedPairingAccess, createReplyPrefixOptions, + registerPluginHttpRoute, resolveDirectDmAuthorizationOutcome, resolveSenderCommandAuthorizationWithRuntime, resolveOutboundMediaUrls, @@ -75,7 +76,24 @@ function logVerbose(core: ZaloCoreRuntime, runtime: ZaloRuntimeEnv, message: str } export function registerZaloWebhookTarget(target: ZaloWebhookTarget): () => void { - return registerZaloWebhookTargetInternal(target); + return registerZaloWebhookTargetInternal(target, { + onFirstPathTarget: ({ path }) => + registerPluginHttpRoute({ + path, + pluginId: "zalo", + source: "zalo-webhook", + accountId: target.account.accountId, + log: target.runtime.log, + handler: async (req, res) => { + const handled = await handleZaloWebhookRequest(req, res); + if (!handled && !res.headersSent) { + res.statusCode = 404; + res.setHeader("Content-Type", "text/plain; charset=utf-8"); + res.end("Not Found"); + } + }, + }), + }); } export { diff --git a/extensions/zalo/src/monitor.webhook.test.ts b/extensions/zalo/src/monitor.webhook.test.ts index 33d509d5255..2a297e3a722 100644 --- a/extensions/zalo/src/monitor.webhook.test.ts +++ b/extensions/zalo/src/monitor.webhook.test.ts @@ -2,6 +2,8 @@ import { createServer, type RequestListener } from "node:http"; import type { AddressInfo } from "node:net"; import type { OpenClawConfig, PluginRuntime } from "openclaw/plugin-sdk"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { createEmptyPluginRegistry } from "../../../src/plugins/registry.js"; +import { setActivePluginRegistry } from "../../../src/plugins/runtime.js"; import { clearZaloWebhookSecurityStateForTest, getZaloWebhookRateLimitStateSizeForTest, @@ -95,6 +97,28 @@ function createPairingAuthCore(params?: { storeAllowFrom?: string[]; pairingCrea describe("handleZaloWebhookRequest", () => { afterEach(() => { clearZaloWebhookSecurityStateForTest(); + setActivePluginRegistry(createEmptyPluginRegistry()); + }); + + it("registers and unregisters plugin HTTP route at path boundaries", () => { + const registry = createEmptyPluginRegistry(); + setActivePluginRegistry(registry); + const unregisterA = registerTarget({ path: "/hook" }); + const unregisterB = registerTarget({ path: "/hook" }); + + expect(registry.httpRoutes).toHaveLength(1); + expect(registry.httpRoutes[0]).toEqual( + expect.objectContaining({ + pluginId: "zalo", + path: "/hook", + source: "zalo-webhook", + }), + ); + + unregisterA(); + expect(registry.httpRoutes).toHaveLength(1); + unregisterB(); + expect(registry.httpRoutes).toHaveLength(0); }); it("returns 400 for non-object payloads", async () => { diff --git a/extensions/zalo/src/monitor.webhook.ts b/extensions/zalo/src/monitor.webhook.ts index 8214e388427..269a97ee4e6 100644 --- a/extensions/zalo/src/monitor.webhook.ts +++ b/extensions/zalo/src/monitor.webhook.ts @@ -7,6 +7,7 @@ import { createWebhookAnomalyTracker, readJsonWebhookBodyOrReject, applyBasicWebhookRequestGuards, + type RegisterWebhookTargetOptions, registerWebhookTarget, resolveSingleWebhookTarget, resolveWebhookTargets, @@ -106,8 +107,14 @@ function recordWebhookStatus( }); } -export function registerZaloWebhookTarget(target: ZaloWebhookTarget): () => void { - return registerWebhookTarget(webhookTargets, target).unregister; +export function registerZaloWebhookTarget( + target: ZaloWebhookTarget, + opts?: Pick< + RegisterWebhookTargetOptions, + "onFirstPathTarget" | "onLastPathTargetRemoved" + >, +): () => void { + return registerWebhookTarget(webhookTargets, target, opts).unregister; } export async function handleZaloWebhookRequest( diff --git a/src/gateway/control-ui-http-utils.ts b/src/gateway/control-ui-http-utils.ts new file mode 100644 index 00000000000..d88cd32fe40 --- /dev/null +++ b/src/gateway/control-ui-http-utils.ts @@ -0,0 +1,19 @@ +import type { ServerResponse } from "node:http"; + +export function isReadHttpMethod(method: string | undefined): boolean { + return method === "GET" || method === "HEAD"; +} + +export function respondPlainText(res: ServerResponse, statusCode: number, body: string): void { + res.statusCode = statusCode; + res.setHeader("Content-Type", "text/plain; charset=utf-8"); + res.end(body); +} + +export function respondNotFound(res: ServerResponse): void { + respondPlainText(res, 404, "Not Found"); +} + +export function respondMethodNotAllowed(res: ServerResponse): void { + respondPlainText(res, 405, "Method Not Allowed"); +} diff --git a/src/gateway/control-ui-routing.test.ts b/src/gateway/control-ui-routing.test.ts new file mode 100644 index 00000000000..73710f1a822 --- /dev/null +++ b/src/gateway/control-ui-routing.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vitest"; +import { classifyControlUiRequest } from "./control-ui-routing.js"; + +describe("classifyControlUiRequest", () => { + it("falls through non-read root requests for plugin webhooks", () => { + const classified = classifyControlUiRequest({ + basePath: "", + pathname: "/bluebubbles-webhook", + search: "", + method: "POST", + }); + expect(classified).toEqual({ kind: "not-control-ui" }); + }); + + it("returns not-found for legacy /ui routes when root-mounted", () => { + const classified = classifyControlUiRequest({ + basePath: "", + pathname: "/ui/settings", + search: "", + method: "GET", + }); + expect(classified).toEqual({ kind: "not-found" }); + }); + + it("returns method-not-allowed for basePath non-read methods", () => { + const classified = classifyControlUiRequest({ + basePath: "/openclaw", + pathname: "/openclaw", + search: "", + method: "POST", + }); + expect(classified).toEqual({ kind: "method-not-allowed" }); + }); + + it("returns redirect for basePath entrypoint GET", () => { + const classified = classifyControlUiRequest({ + basePath: "/openclaw", + pathname: "/openclaw", + search: "?foo=1", + method: "GET", + }); + expect(classified).toEqual({ kind: "redirect", location: "/openclaw/?foo=1" }); + }); + + it("classifies basePath subroutes as control ui", () => { + const classified = classifyControlUiRequest({ + basePath: "/openclaw", + pathname: "/openclaw/chat", + search: "", + method: "HEAD", + }); + expect(classified).toEqual({ kind: "serve" }); + }); +}); diff --git a/src/gateway/control-ui-routing.ts b/src/gateway/control-ui-routing.ts new file mode 100644 index 00000000000..44635e92e1d --- /dev/null +++ b/src/gateway/control-ui-routing.ts @@ -0,0 +1,45 @@ +import { isReadHttpMethod } from "./control-ui-http-utils.js"; + +export type ControlUiRequestClassification = + | { kind: "not-control-ui" } + | { kind: "not-found" } + | { kind: "method-not-allowed" } + | { kind: "redirect"; location: string } + | { kind: "serve" }; + +export function classifyControlUiRequest(params: { + basePath: string; + pathname: string; + search: string; + method: string | undefined; +}): ControlUiRequestClassification { + const { basePath, pathname, search, method } = params; + if (!basePath) { + if (pathname === "/ui" || pathname.startsWith("/ui/")) { + return { kind: "not-found" }; + } + // Keep plugin-owned HTTP routes outside the root-mounted Control UI SPA + // fallback so untrusted plugins cannot claim arbitrary UI paths. + if (pathname === "/plugins" || pathname.startsWith("/plugins/")) { + return { kind: "not-control-ui" }; + } + if (pathname === "/api" || pathname.startsWith("/api/")) { + return { kind: "not-control-ui" }; + } + if (!isReadHttpMethod(method)) { + return { kind: "not-control-ui" }; + } + return { kind: "serve" }; + } + + if (!pathname.startsWith(`${basePath}/`) && pathname !== basePath) { + return { kind: "not-control-ui" }; + } + if (!isReadHttpMethod(method)) { + return { kind: "method-not-allowed" }; + } + if (pathname === basePath) { + return { kind: "redirect", location: `${basePath}/${search}` }; + } + return { kind: "serve" }; +} diff --git a/src/gateway/control-ui.ts b/src/gateway/control-ui.ts index 4858a6e5004..fc1ad4633ec 100644 --- a/src/gateway/control-ui.ts +++ b/src/gateway/control-ui.ts @@ -13,6 +13,13 @@ import { type ControlUiBootstrapConfig, } from "./control-ui-contract.js"; import { buildControlUiCspHeader } from "./control-ui-csp.js"; +import { + isReadHttpMethod, + respondMethodNotAllowed, + respondNotFound as respondControlUiNotFound, + respondPlainText, +} from "./control-ui-http-utils.js"; +import { classifyControlUiRequest } from "./control-ui-routing.js"; import { buildControlUiAvatarUrl, CONTROL_UI_AVATAR_PREFIX, @@ -124,7 +131,7 @@ export function handleControlUiAvatarRequest( if (!urlRaw) { return false; } - if (req.method !== "GET" && req.method !== "HEAD") { + if (!isReadHttpMethod(req.method)) { return false; } @@ -143,7 +150,7 @@ export function handleControlUiAvatarRequest( const agentIdParts = pathname.slice(pathWithBase.length).split("/").filter(Boolean); const agentId = agentIdParts[0] ?? ""; if (agentIdParts.length !== 1 || !agentId || !isValidAgentId(agentId)) { - respondNotFound(res); + respondControlUiNotFound(res); return true; } @@ -161,13 +168,13 @@ export function handleControlUiAvatarRequest( const resolved = opts.resolveAvatar(agentId); if (resolved.kind !== "local") { - respondNotFound(res); + respondControlUiNotFound(res); return true; } const safeAvatar = resolveSafeAvatarFile(resolved.filePath); if (!safeAvatar) { - respondNotFound(res); + respondControlUiNotFound(res); return true; } try { @@ -186,12 +193,6 @@ export function handleControlUiAvatarRequest( } } -function respondNotFound(res: ServerResponse) { - res.statusCode = 404; - res.setHeader("Content-Type", "text/plain; charset=utf-8"); - res.end("Not Found"); -} - function setStaticFileHeaders(res: ServerResponse, filePath: string) { const ext = path.extname(filePath).toLowerCase(); res.setHeader("Content-Type", contentTypeForExt(ext)); @@ -278,46 +279,30 @@ export function handleControlUiHttpRequest( const url = new URL(urlRaw, "http://localhost"); const basePath = normalizeControlUiBasePath(opts?.basePath); const pathname = url.pathname; - - if (!basePath) { - if (pathname === "/ui" || pathname.startsWith("/ui/")) { - applyControlUiSecurityHeaders(res); - respondNotFound(res); - return true; - } - // Keep plugin-owned HTTP routes outside the root-mounted Control UI SPA - // fallback so untrusted plugins cannot claim arbitrary UI paths. - if (pathname === "/plugins" || pathname.startsWith("/plugins/")) { - return false; - } - if (pathname === "/api" || pathname.startsWith("/api/")) { - return false; - } - // Root-mounted SPA: non-GET/HEAD may be destined for plugin HTTP handlers - // (e.g. BlueBubbles webhook POST) that run after Control UI in the chain. - if (req.method !== "GET" && req.method !== "HEAD") { - return false; - } + const route = classifyControlUiRequest({ + basePath, + pathname, + search: url.search, + method: req.method, + }); + if (route.kind === "not-control-ui") { + return false; } - - if (basePath) { - if (!pathname.startsWith(`${basePath}/`) && pathname !== basePath) { - return false; - } - // Requests under a configured basePath are always Control UI traffic. - if (req.method !== "GET" && req.method !== "HEAD") { - res.statusCode = 405; - res.setHeader("Content-Type", "text/plain; charset=utf-8"); - res.end("Method Not Allowed"); - return true; - } - if (pathname === basePath) { - applyControlUiSecurityHeaders(res); - res.statusCode = 302; - res.setHeader("Location", `${basePath}/${url.search}`); - res.end(); - return true; - } + if (route.kind === "not-found") { + applyControlUiSecurityHeaders(res); + respondControlUiNotFound(res); + return true; + } + if (route.kind === "method-not-allowed") { + respondMethodNotAllowed(res); + return true; + } + if (route.kind === "redirect") { + applyControlUiSecurityHeaders(res); + res.statusCode = 302; + res.setHeader("Location", route.location); + res.end(); + return true; } applyControlUiSecurityHeaders(res); @@ -353,17 +338,17 @@ export function handleControlUiHttpRequest( const rootState = opts?.root; if (rootState?.kind === "invalid") { - res.statusCode = 503; - res.setHeader("Content-Type", "text/plain; charset=utf-8"); - res.end( + respondPlainText( + res, + 503, `Control UI assets not found at ${rootState.path}. Build them with \`pnpm ui:build\` (auto-installs UI deps), or update gateway.controlUi.root.`, ); return true; } if (rootState?.kind === "missing") { - res.statusCode = 503; - res.setHeader("Content-Type", "text/plain; charset=utf-8"); - res.end( + respondPlainText( + res, + 503, "Control UI assets not found. Build them with `pnpm ui:build` (auto-installs UI deps), or run `pnpm ui:dev` during development.", ); return true; @@ -378,9 +363,9 @@ export function handleControlUiHttpRequest( cwd: process.cwd(), }); if (!root) { - res.statusCode = 503; - res.setHeader("Content-Type", "text/plain; charset=utf-8"); - res.end( + respondPlainText( + res, + 503, "Control UI assets not found. Build them with `pnpm ui:build` (auto-installs UI deps), or run `pnpm ui:dev` during development.", ); return true; @@ -397,9 +382,9 @@ export function handleControlUiHttpRequest( } })(); if (!rootReal) { - res.statusCode = 503; - res.setHeader("Content-Type", "text/plain; charset=utf-8"); - res.end( + respondPlainText( + res, + 503, "Control UI assets not found. Build them with `pnpm ui:build` (auto-installs UI deps), or run `pnpm ui:dev` during development.", ); return true; @@ -420,13 +405,13 @@ export function handleControlUiHttpRequest( const requested = rel && !rel.endsWith("/") ? rel : `${rel}index.html`; const fileRel = requested || "index.html"; if (!isSafeRelativePath(fileRel)) { - respondNotFound(res); + respondControlUiNotFound(res); return true; } const filePath = path.resolve(root, fileRel); if (!isWithinDir(root, filePath)) { - respondNotFound(res); + respondControlUiNotFound(res); return true; } @@ -456,7 +441,7 @@ export function handleControlUiHttpRequest( // that dotted SPA routes (e.g. /user/jane.doe, /v2.0) still get the // client-side router fallback. if (STATIC_ASSET_EXTENSIONS.has(path.extname(fileRel).toLowerCase())) { - respondNotFound(res); + respondControlUiNotFound(res); return true; } @@ -478,6 +463,6 @@ export function handleControlUiHttpRequest( } } - respondNotFound(res); + respondControlUiNotFound(res); return true; } diff --git a/src/gateway/server-http.test-harness.ts b/src/gateway/server-http.test-harness.ts new file mode 100644 index 00000000000..09c7510b8d1 --- /dev/null +++ b/src/gateway/server-http.test-harness.ts @@ -0,0 +1,268 @@ +import type { IncomingMessage, ServerResponse } from "node:http"; +import { expect, vi } from "vitest"; +import type { createSubsystemLogger } from "../logging/subsystem.js"; +import type { ResolvedGatewayAuth } from "./auth.js"; +import { createGatewayRequest, createHooksConfig } from "./hooks-test-helpers.js"; +import { canonicalizePathVariant, isProtectedPluginRoutePath } from "./security-path.js"; +import { createGatewayHttpServer, createHooksRequestHandler } from "./server-http.js"; +import { withTempConfig } from "./test-temp-config.js"; + +export type GatewayHttpServer = ReturnType; +export type GatewayServerOptions = Partial[0]>; + +export const AUTH_NONE: ResolvedGatewayAuth = { + mode: "none", + token: undefined, + password: undefined, + allowTailscale: false, +}; + +export const AUTH_TOKEN: ResolvedGatewayAuth = { + mode: "token", + token: "test-token", + password: undefined, + allowTailscale: false, +}; + +export function createRequest(params: { + path: string; + authorization?: string; + method?: string; +}): IncomingMessage { + return createGatewayRequest({ + path: params.path, + authorization: params.authorization, + method: params.method, + }); +} + +export function createResponse(): { + res: ServerResponse; + setHeader: ReturnType; + end: ReturnType; + getBody: () => string; +} { + const setHeader = vi.fn(); + let body = ""; + const end = vi.fn((chunk?: unknown) => { + if (typeof chunk === "string") { + body = chunk; + return; + } + if (chunk == null) { + body = ""; + return; + } + body = JSON.stringify(chunk); + }); + const res = { + headersSent: false, + statusCode: 200, + setHeader, + end, + } as unknown as ServerResponse; + return { + res, + setHeader, + end, + getBody: () => body, + }; +} + +export async function dispatchRequest( + server: GatewayHttpServer, + req: IncomingMessage, + res: ServerResponse, +): Promise { + server.emit("request", req, res); + await new Promise((resolve) => setImmediate(resolve)); +} + +export async function withGatewayTempConfig( + prefix: string, + run: () => Promise, +): Promise { + await withTempConfig({ + cfg: { gateway: { trustedProxies: [] } }, + prefix, + run, + }); +} + +export function createTestGatewayServer(options: { + resolvedAuth: ResolvedGatewayAuth; + overrides?: GatewayServerOptions; +}): GatewayHttpServer { + return createGatewayHttpServer({ + canvasHost: null, + clients: new Set(), + controlUiEnabled: false, + controlUiBasePath: "/__control__", + openAiChatCompletionsEnabled: false, + openResponsesEnabled: false, + handleHooksRequest: async () => false, + ...options.overrides, + resolvedAuth: options.resolvedAuth, + }); +} + +export async function withGatewayServer(params: { + prefix: string; + resolvedAuth: ResolvedGatewayAuth; + overrides?: GatewayServerOptions; + run: (server: GatewayHttpServer) => Promise; +}): Promise { + await withGatewayTempConfig(params.prefix, async () => { + const server = createTestGatewayServer({ + resolvedAuth: params.resolvedAuth, + overrides: params.overrides, + }); + await params.run(server); + }); +} + +export async function sendRequest( + server: GatewayHttpServer, + params: { + path: string; + authorization?: string; + method?: string; + }, +) { + const response = createResponse(); + await dispatchRequest(server, createRequest(params), response.res); + return response; +} + +export function expectUnauthorizedResponse( + response: ReturnType, + label?: string, +): void { + expect(response.res.statusCode, label).toBe(401); + expect(response.getBody(), label).toContain("Unauthorized"); +} + +export function createCanonicalizedChannelPluginHandler() { + return vi.fn(async (req: IncomingMessage, res: ServerResponse) => { + const pathname = new URL(req.url ?? "/", "http://localhost").pathname; + const canonicalPath = canonicalizePathVariant(pathname); + if (canonicalPath !== "/api/channels/nostr/default/profile") { + return false; + } + res.statusCode = 200; + res.setHeader("Content-Type", "application/json; charset=utf-8"); + res.end(JSON.stringify({ ok: true, route: "channel-canonicalized" })); + return true; + }); +} + +export function createHooksHandler(bindHost: string) { + return createHooksRequestHandler({ + getHooksConfig: () => createHooksConfig(), + bindHost, + port: 18789, + logHooks: { + warn: vi.fn(), + debug: vi.fn(), + info: vi.fn(), + error: vi.fn(), + } as unknown as ReturnType, + dispatchWakeHook: () => {}, + dispatchAgentHook: () => "run-1", + }); +} + +export type RouteVariant = { + label: string; + path: string; +}; + +export const CANONICAL_UNAUTH_VARIANTS: RouteVariant[] = [ + { label: "case-variant", path: "/API/channels/nostr/default/profile" }, + { label: "encoded-slash", path: "/api/channels%2Fnostr%2Fdefault%2Fprofile" }, + { + label: "encoded-slash-4x", + path: "/api%2525252fchannels%2525252fnostr%2525252fdefault%2525252fprofile", + }, + { label: "encoded-segment", path: "/api/%63hannels/nostr/default/profile" }, + { label: "dot-traversal-encoded-slash", path: "/api/foo/..%2fchannels/nostr/default/profile" }, + { + label: "dot-traversal-encoded-dotdot-slash", + path: "/api/foo/%2e%2e%2fchannels/nostr/default/profile", + }, + { + label: "dot-traversal-double-encoded", + path: "/api/foo/%252e%252e%252fchannels/nostr/default/profile", + }, + { label: "duplicate-slashes", path: "/api/channels//nostr/default/profile" }, + { label: "trailing-slash", path: "/api/channels/nostr/default/profile/" }, + { label: "malformed-short-percent", path: "/api/channels%2" }, + { label: "malformed-double-slash-short-percent", path: "/api//channels%2" }, +]; + +export const CANONICAL_AUTH_VARIANTS: RouteVariant[] = [ + { label: "auth-case-variant", path: "/API/channels/nostr/default/profile" }, + { + label: "auth-encoded-slash-4x", + path: "/api%2525252fchannels%2525252fnostr%2525252fdefault%2525252fprofile", + }, + { label: "auth-encoded-segment", path: "/api/%63hannels/nostr/default/profile" }, + { label: "auth-duplicate-trailing-slash", path: "/api/channels//nostr/default/profile/" }, + { + label: "auth-dot-traversal-encoded-slash", + path: "/api/foo/..%2fchannels/nostr/default/profile", + }, + { + label: "auth-dot-traversal-double-encoded", + path: "/api/foo/%252e%252e%252fchannels/nostr/default/profile", + }, +]; + +export function buildChannelPathFuzzCorpus(): RouteVariant[] { + const variants = [ + "/api/channels/nostr/default/profile", + "/API/channels/nostr/default/profile", + "/api/foo/..%2fchannels/nostr/default/profile", + "/api/foo/%2e%2e%2fchannels/nostr/default/profile", + "/api/foo/%252e%252e%252fchannels/nostr/default/profile", + "/api/channels//nostr/default/profile/", + "/api/channels%2Fnostr%2Fdefault%2Fprofile", + "/api/channels%252Fnostr%252Fdefault%252Fprofile", + "/api%2525252fchannels%2525252fnostr%2525252fdefault%2525252fprofile", + "/api//channels/nostr/default/profile", + "/api/channels%2", + "/api/channels%zz", + "/api//channels%2", + "/api//channels%zz", + ]; + return variants.map((path) => ({ label: `fuzz:${path}`, path })); +} + +export async function expectUnauthorizedVariants(params: { + server: GatewayHttpServer; + variants: RouteVariant[]; +}) { + for (const variant of params.variants) { + const response = await sendRequest(params.server, { path: variant.path }); + expectUnauthorizedResponse(response, variant.label); + } +} + +export async function expectAuthorizedVariants(params: { + server: GatewayHttpServer; + variants: RouteVariant[]; + authorization: string; +}) { + for (const variant of params.variants) { + const response = await sendRequest(params.server, { + path: variant.path, + authorization: params.authorization, + }); + expect(response.res.statusCode, variant.label).toBe(200); + expect(response.getBody(), variant.label).toContain('"route":"channel-canonicalized"'); + } +} + +export function defaultProtectedPluginRoutePath(pathname: string): boolean { + return isProtectedPluginRoutePath(pathname); +} diff --git a/src/gateway/server-http.ts b/src/gateway/server-http.ts index 22d70bb0041..64947b3b34b 100644 --- a/src/gateway/server-http.ts +++ b/src/gateway/server-http.ts @@ -146,6 +146,22 @@ function writeUpgradeAuthFailure( export type HooksRequestHandler = (req: IncomingMessage, res: ServerResponse) => Promise; +type GatewayHttpRequestStage = { + name: string; + run: () => Promise | boolean; +}; + +async function runGatewayHttpRequestStages( + stages: readonly GatewayHttpRequestStage[], +): Promise { + for (const stage of stages) { + if (await stage.run()) { + return true; + } + } + return false; +} + export function createHooksRequestHandler( opts: { getHooksConfig: () => HooksConfigResolved | null; @@ -429,113 +445,144 @@ export function createGatewayHttpServer(opts: { req.url = scopedCanvas.rewrittenUrl; } const requestPath = new URL(req.url ?? "/", "http://localhost").pathname; - if (await handleHooksRequest(req, res)) { - return; - } - if ( - await handleToolsInvokeHttpRequest(req, res, { - auth: resolvedAuth, - trustedProxies, - allowRealIpFallback, - rateLimiter, - }) - ) { - return; - } - if (await handleSlackHttpRequest(req, res)) { - return; - } + + const requestStages: GatewayHttpRequestStage[] = [ + { + name: "hooks", + run: () => handleHooksRequest(req, res), + }, + { + name: "tools-invoke", + run: () => + handleToolsInvokeHttpRequest(req, res, { + auth: resolvedAuth, + trustedProxies, + allowRealIpFallback, + rateLimiter, + }), + }, + { + name: "slack", + run: () => handleSlackHttpRequest(req, res), + }, + ]; + if (openResponsesEnabled) { - if ( - await handleOpenResponsesHttpRequest(req, res, { - auth: resolvedAuth, - config: openResponsesConfig, - trustedProxies, - allowRealIpFallback, - rateLimiter, - }) - ) { - return; - } + requestStages.push({ + name: "openresponses", + run: () => + handleOpenResponsesHttpRequest(req, res, { + auth: resolvedAuth, + config: openResponsesConfig, + trustedProxies, + allowRealIpFallback, + rateLimiter, + }), + }); } if (openAiChatCompletionsEnabled) { - if ( - await handleOpenAiHttpRequest(req, res, { - auth: resolvedAuth, - trustedProxies, - allowRealIpFallback, - rateLimiter, - }) - ) { - return; - } + requestStages.push({ + name: "openai", + run: () => + handleOpenAiHttpRequest(req, res, { + auth: resolvedAuth, + trustedProxies, + allowRealIpFallback, + rateLimiter, + }), + }); } if (canvasHost) { - if (isCanvasPath(requestPath)) { - const ok = await authorizeCanvasRequest({ - req, - auth: resolvedAuth, - trustedProxies, - allowRealIpFallback, - clients, - canvasCapability: scopedCanvas.capability, - malformedScopedPath: scopedCanvas.malformedScopedPath, - rateLimiter, - }); - if (!ok.ok) { - sendGatewayAuthFailure(res, ok); - return; - } - } - if (await handleA2uiHttpRequest(req, res)) { - return; - } - if (await canvasHost.handleHttpRequest(req, res)) { - return; - } + requestStages.push({ + name: "canvas-auth", + run: async () => { + if (!isCanvasPath(requestPath)) { + return false; + } + const ok = await authorizeCanvasRequest({ + req, + auth: resolvedAuth, + trustedProxies, + allowRealIpFallback, + clients, + canvasCapability: scopedCanvas.capability, + malformedScopedPath: scopedCanvas.malformedScopedPath, + rateLimiter, + }); + if (!ok.ok) { + sendGatewayAuthFailure(res, ok); + return true; + } + return false; + }, + }); + requestStages.push({ + name: "a2ui", + run: () => handleA2uiHttpRequest(req, res), + }); + requestStages.push({ + name: "canvas-http", + run: () => canvasHost.handleHttpRequest(req, res), + }); } if (controlUiEnabled) { - if ( - handleControlUiAvatarRequest(req, res, { - basePath: controlUiBasePath, - resolveAvatar: (agentId) => resolveAgentAvatar(configSnapshot, agentId), - }) - ) { - return; - } - if ( - handleControlUiHttpRequest(req, res, { - basePath: controlUiBasePath, - config: configSnapshot, - root: controlUiRoot, - }) - ) { - return; - } + requestStages.push({ + name: "control-ui-avatar", + run: () => + handleControlUiAvatarRequest(req, res, { + basePath: controlUiBasePath, + resolveAvatar: (agentId) => resolveAgentAvatar(configSnapshot, agentId), + }), + }); + requestStages.push({ + name: "control-ui-http", + run: () => + handleControlUiHttpRequest(req, res, { + basePath: controlUiBasePath, + config: configSnapshot, + root: controlUiRoot, + }), + }); } // Plugins run after built-in gateway routes so core surfaces keep // precedence on overlapping paths. if (handlePluginRequest) { - if ( - (shouldEnforcePluginGatewayAuth ?? shouldEnforceDefaultPluginGatewayAuth)(requestPath) - ) { - const pluginAuthOk = await enforcePluginRouteGatewayAuth({ - req, - res, - auth: resolvedAuth, - trustedProxies, - allowRealIpFallback, - rateLimiter, - }); - if (!pluginAuthOk) { - return; - } - } - if (await handlePluginRequest(req, res)) { - return; - } + requestStages.push({ + name: "plugin-auth", + run: async () => { + if ( + !(shouldEnforcePluginGatewayAuth ?? shouldEnforceDefaultPluginGatewayAuth)( + requestPath, + ) + ) { + return false; + } + const pluginAuthOk = await enforcePluginRouteGatewayAuth({ + req, + res, + auth: resolvedAuth, + trustedProxies, + allowRealIpFallback, + rateLimiter, + }); + if (!pluginAuthOk) { + return true; + } + return false; + }, + }); + requestStages.push({ + name: "plugin-http", + run: () => handlePluginRequest(req, res), + }); } - if (handleGatewayProbeRequest(req, res, requestPath)) { + + requestStages.push({ + name: "gateway-probes", + run: () => handleGatewayProbeRequest(req, res, requestPath), + }); + + if (await runGatewayHttpRequestStages(requestStages)) { return; } diff --git a/src/gateway/server.plugin-http-auth.test.ts b/src/gateway/server.plugin-http-auth.test.ts index c43130df45e..544bb80d333 100644 --- a/src/gateway/server.plugin-http-auth.test.ts +++ b/src/gateway/server.plugin-http-auth.test.ts @@ -1,269 +1,27 @@ import type { IncomingMessage, ServerResponse } from "node:http"; import { describe, expect, test, vi } from "vitest"; -import type { createSubsystemLogger } from "../logging/subsystem.js"; -import type { ResolvedGatewayAuth } from "./auth.js"; -import { createGatewayRequest, createHooksConfig } from "./hooks-test-helpers.js"; import { canonicalizePathVariant, isProtectedPluginRoutePath } from "./security-path.js"; -import { createGatewayHttpServer, createHooksRequestHandler } from "./server-http.js"; -import { withTempConfig } from "./test-temp-config.js"; - -type GatewayHttpServer = ReturnType; -type GatewayServerOptions = Partial[0]>; - -const AUTH_NONE: ResolvedGatewayAuth = { - mode: "none", - token: undefined, - password: undefined, - allowTailscale: false, -}; - -const AUTH_TOKEN: ResolvedGatewayAuth = { - mode: "token", - token: "test-token", - password: undefined, - allowTailscale: false, -}; - -function createRequest(params: { - path: string; - authorization?: string; - method?: string; -}): IncomingMessage { - return createGatewayRequest({ - path: params.path, - authorization: params.authorization, - method: params.method, - }); -} - -function createResponse(): { - res: ServerResponse; - setHeader: ReturnType; - end: ReturnType; - getBody: () => string; -} { - const setHeader = vi.fn(); - let body = ""; - const end = vi.fn((chunk?: unknown) => { - if (typeof chunk === "string") { - body = chunk; - return; - } - if (chunk == null) { - body = ""; - return; - } - body = JSON.stringify(chunk); - }); - const res = { - headersSent: false, - statusCode: 200, - setHeader, - end, - } as unknown as ServerResponse; - return { - res, - setHeader, - end, - getBody: () => body, - }; -} - -async function dispatchRequest( - server: GatewayHttpServer, - req: IncomingMessage, - res: ServerResponse, -): Promise { - server.emit("request", req, res); - await new Promise((resolve) => setImmediate(resolve)); -} - -async function withGatewayTempConfig(prefix: string, run: () => Promise): Promise { - await withTempConfig({ - cfg: { gateway: { trustedProxies: [] } }, - prefix, - run, - }); -} - -function createTestGatewayServer(options: { - resolvedAuth: ResolvedGatewayAuth; - overrides?: GatewayServerOptions; -}): GatewayHttpServer { - return createGatewayHttpServer({ - canvasHost: null, - clients: new Set(), - controlUiEnabled: false, - controlUiBasePath: "/__control__", - openAiChatCompletionsEnabled: false, - openResponsesEnabled: false, - handleHooksRequest: async () => false, - ...options.overrides, - resolvedAuth: options.resolvedAuth, - }); -} - -async function withGatewayServer(params: { - prefix: string; - resolvedAuth: ResolvedGatewayAuth; - overrides?: GatewayServerOptions; - run: (server: GatewayHttpServer) => Promise; -}): Promise { - await withGatewayTempConfig(params.prefix, async () => { - const server = createTestGatewayServer({ - resolvedAuth: params.resolvedAuth, - overrides: params.overrides, - }); - await params.run(server); - }); -} - -async function sendRequest( - server: GatewayHttpServer, - params: { - path: string; - authorization?: string; - method?: string; - }, -) { - const response = createResponse(); - await dispatchRequest(server, createRequest(params), response.res); - return response; -} - -function expectUnauthorizedResponse( - response: ReturnType, - label?: string, -): void { - expect(response.res.statusCode, label).toBe(401); - expect(response.getBody(), label).toContain("Unauthorized"); -} +import { + AUTH_NONE, + AUTH_TOKEN, + buildChannelPathFuzzCorpus, + CANONICAL_AUTH_VARIANTS, + CANONICAL_UNAUTH_VARIANTS, + createCanonicalizedChannelPluginHandler, + createHooksHandler, + createTestGatewayServer, + expectAuthorizedVariants, + expectUnauthorizedResponse, + expectUnauthorizedVariants, + sendRequest, + withGatewayServer, + withGatewayTempConfig, +} from "./server-http.test-harness.js"; function canonicalizePluginPath(pathname: string): string { return canonicalizePathVariant(pathname); } -function createCanonicalizedChannelPluginHandler() { - return vi.fn(async (req: IncomingMessage, res: ServerResponse) => { - const pathname = new URL(req.url ?? "/", "http://localhost").pathname; - const canonicalPath = canonicalizePluginPath(pathname); - if (canonicalPath !== "/api/channels/nostr/default/profile") { - return false; - } - res.statusCode = 200; - res.setHeader("Content-Type", "application/json; charset=utf-8"); - res.end(JSON.stringify({ ok: true, route: "channel-canonicalized" })); - return true; - }); -} - -function createHooksHandler(bindHost: string) { - return createHooksRequestHandler({ - getHooksConfig: () => createHooksConfig(), - bindHost, - port: 18789, - logHooks: { - warn: vi.fn(), - debug: vi.fn(), - info: vi.fn(), - error: vi.fn(), - } as unknown as ReturnType, - dispatchWakeHook: () => {}, - dispatchAgentHook: () => "run-1", - }); -} - -type RouteVariant = { - label: string; - path: string; -}; - -const CANONICAL_UNAUTH_VARIANTS: RouteVariant[] = [ - { label: "case-variant", path: "/API/channels/nostr/default/profile" }, - { label: "encoded-slash", path: "/api/channels%2Fnostr%2Fdefault%2Fprofile" }, - { - label: "encoded-slash-4x", - path: "/api%2525252fchannels%2525252fnostr%2525252fdefault%2525252fprofile", - }, - { label: "encoded-segment", path: "/api/%63hannels/nostr/default/profile" }, - { label: "dot-traversal-encoded-slash", path: "/api/foo/..%2fchannels/nostr/default/profile" }, - { - label: "dot-traversal-encoded-dotdot-slash", - path: "/api/foo/%2e%2e%2fchannels/nostr/default/profile", - }, - { - label: "dot-traversal-double-encoded", - path: "/api/foo/%252e%252e%252fchannels/nostr/default/profile", - }, - { label: "duplicate-slashes", path: "/api/channels//nostr/default/profile" }, - { label: "trailing-slash", path: "/api/channels/nostr/default/profile/" }, - { label: "malformed-short-percent", path: "/api/channels%2" }, - { label: "malformed-double-slash-short-percent", path: "/api//channels%2" }, -]; - -const CANONICAL_AUTH_VARIANTS: RouteVariant[] = [ - { label: "auth-case-variant", path: "/API/channels/nostr/default/profile" }, - { - label: "auth-encoded-slash-4x", - path: "/api%2525252fchannels%2525252fnostr%2525252fdefault%2525252fprofile", - }, - { label: "auth-encoded-segment", path: "/api/%63hannels/nostr/default/profile" }, - { label: "auth-duplicate-trailing-slash", path: "/api/channels//nostr/default/profile/" }, - { - label: "auth-dot-traversal-encoded-slash", - path: "/api/foo/..%2fchannels/nostr/default/profile", - }, - { - label: "auth-dot-traversal-double-encoded", - path: "/api/foo/%252e%252e%252fchannels/nostr/default/profile", - }, -]; - -function buildChannelPathFuzzCorpus(): RouteVariant[] { - const variants = [ - "/api/channels/nostr/default/profile", - "/API/channels/nostr/default/profile", - "/api/foo/..%2fchannels/nostr/default/profile", - "/api/foo/%2e%2e%2fchannels/nostr/default/profile", - "/api/foo/%252e%252e%252fchannels/nostr/default/profile", - "/api/channels//nostr/default/profile/", - "/api/channels%2Fnostr%2Fdefault%2Fprofile", - "/api/channels%252Fnostr%252Fdefault%252Fprofile", - "/api%2525252fchannels%2525252fnostr%2525252fdefault%2525252fprofile", - "/api//channels/nostr/default/profile", - "/api/channels%2", - "/api/channels%zz", - "/api//channels%2", - "/api//channels%zz", - ]; - return variants.map((path) => ({ label: `fuzz:${path}`, path })); -} - -async function expectUnauthorizedVariants(params: { - server: GatewayHttpServer; - variants: RouteVariant[]; -}) { - for (const variant of params.variants) { - const response = await sendRequest(params.server, { path: variant.path }); - expectUnauthorizedResponse(response, variant.label); - } -} - -async function expectAuthorizedVariants(params: { - server: GatewayHttpServer; - variants: RouteVariant[]; - authorization: string; -}) { - for (const variant of params.variants) { - const response = await sendRequest(params.server, { - path: variant.path, - authorization: params.authorization, - }); - expect(response.res.statusCode, variant.label).toBe(200); - expect(response.getBody(), variant.label).toContain('"route":"channel-canonicalized"'); - } -} - describe("gateway plugin HTTP auth boundary", () => { test("applies default security headers and optional strict transport security", async () => { await withGatewayTempConfig("openclaw-plugin-http-security-headers-test-", async () => { diff --git a/src/plugin-sdk/index.ts b/src/plugin-sdk/index.ts index 60f74ecb542..840f697c292 100644 --- a/src/plugin-sdk/index.ts +++ b/src/plugin-sdk/index.ts @@ -128,7 +128,7 @@ export { resolveSingleWebhookTargetAsync, resolveWebhookTargets, } from "./webhook-targets.js"; -export type { WebhookTargetMatchResult } from "./webhook-targets.js"; +export type { RegisterWebhookTargetOptions, WebhookTargetMatchResult } from "./webhook-targets.js"; export { applyBasicWebhookRequestGuards, isJsonContentType, diff --git a/src/plugin-sdk/webhook-targets.test.ts b/src/plugin-sdk/webhook-targets.test.ts index 753e0ddc186..ab56d3fccfc 100644 --- a/src/plugin-sdk/webhook-targets.test.ts +++ b/src/plugin-sdk/webhook-targets.test.ts @@ -31,6 +31,59 @@ describe("registerWebhookTarget", () => { registered.unregister(); expect(targets.has("/hook")).toBe(false); }); + + it("runs first/last path lifecycle hooks only at path boundaries", () => { + const targets = new Map>(); + const teardown = vi.fn(); + const onFirstPathTarget = vi.fn(() => teardown); + const onLastPathTargetRemoved = vi.fn(); + + const registeredA = registerWebhookTarget( + targets, + { path: "hook", id: "A" }, + { onFirstPathTarget, onLastPathTargetRemoved }, + ); + const registeredB = registerWebhookTarget( + targets, + { path: "/hook", id: "B" }, + { onFirstPathTarget, onLastPathTargetRemoved }, + ); + + expect(onFirstPathTarget).toHaveBeenCalledTimes(1); + expect(onFirstPathTarget).toHaveBeenCalledWith({ + path: "/hook", + target: expect.objectContaining({ id: "A", path: "/hook" }), + }); + + registeredB.unregister(); + expect(teardown).not.toHaveBeenCalled(); + expect(onLastPathTargetRemoved).not.toHaveBeenCalled(); + + registeredA.unregister(); + expect(teardown).toHaveBeenCalledTimes(1); + expect(onLastPathTargetRemoved).toHaveBeenCalledTimes(1); + expect(onLastPathTargetRemoved).toHaveBeenCalledWith({ path: "/hook" }); + + registeredA.unregister(); + expect(teardown).toHaveBeenCalledTimes(1); + expect(onLastPathTargetRemoved).toHaveBeenCalledTimes(1); + }); + + it("does not register target when first-path hook throws", () => { + const targets = new Map>(); + expect(() => + registerWebhookTarget( + targets, + { path: "/hook", id: "A" }, + { + onFirstPathTarget: () => { + throw new Error("boom"); + }, + }, + ), + ).toThrow("boom"); + expect(targets.has("/hook")).toBe(false); + }); }); describe("resolveWebhookTargets", () => { diff --git a/src/plugin-sdk/webhook-targets.ts b/src/plugin-sdk/webhook-targets.ts index 1a7cd40accf..0f15cca1517 100644 --- a/src/plugin-sdk/webhook-targets.ts +++ b/src/plugin-sdk/webhook-targets.ts @@ -6,21 +6,65 @@ export type RegisteredWebhookTarget = { unregister: () => void; }; +export type RegisterWebhookTargetOptions = { + onFirstPathTarget?: (params: { path: string; target: T }) => void | (() => void); + onLastPathTargetRemoved?: (params: { path: string }) => void; +}; + +const pathTeardownByTargetMap = new WeakMap, Map void>>(); + +function getPathTeardownMap(targetsByPath: Map): Map void> { + const mapKey = targetsByPath as unknown as Map; + const existing = pathTeardownByTargetMap.get(mapKey); + if (existing) { + return existing; + } + const created = new Map void>(); + pathTeardownByTargetMap.set(mapKey, created); + return created; +} + export function registerWebhookTarget( targetsByPath: Map, target: T, + opts?: RegisterWebhookTargetOptions, ): RegisteredWebhookTarget { const key = normalizeWebhookPath(target.path); const normalizedTarget = { ...target, path: key }; const existing = targetsByPath.get(key) ?? []; + + if (existing.length === 0) { + const onFirstPathResult = opts?.onFirstPathTarget?.({ + path: key, + target: normalizedTarget, + }); + if (typeof onFirstPathResult === "function") { + getPathTeardownMap(targetsByPath).set(key, onFirstPathResult); + } + } + targetsByPath.set(key, [...existing, normalizedTarget]); + + let isActive = true; const unregister = () => { + if (!isActive) { + return; + } + isActive = false; + const updated = (targetsByPath.get(key) ?? []).filter((entry) => entry !== normalizedTarget); if (updated.length > 0) { targetsByPath.set(key, updated); return; } targetsByPath.delete(key); + + const teardown = getPathTeardownMap(targetsByPath).get(key); + if (teardown) { + getPathTeardownMap(targetsByPath).delete(key); + teardown(); + } + opts?.onLastPathTargetRemoved?.({ path: key }); }; return { target: normalizedTarget, unregister }; }