fix(gateway): keep plugin routes cold on core paths (#117858)

Co-authored-by: Peter Steinberger <steipete@macos.shared>
This commit is contained in:
Peter Steinberger
2026-08-01 23:21:19 -07:00
committed by GitHub
parent 7ded64d4b6
commit 72d9019184
2 changed files with 102 additions and 6 deletions
+83
View File
@@ -3,6 +3,7 @@
*/
import { connect } from "node:net";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { WebSocket } from "ws";
import { createEmptyPluginRegistry } from "../plugins/registry.js";
import { resetPluginRuntimeStateForTest } from "../plugins/runtime.js";
import { createGatewayRuntimeStateForTest } from "./test-helpers.server-runtime-state.js";
@@ -12,6 +13,7 @@ const mocks = vi.hoisted(() => ({
async (_params: { bindHost: string; port?: number; retryEaddrinuse?: boolean }) => {},
),
resolveGatewayListenHosts: vi.fn(async (_bindHost: string) => ["127.0.0.1"]),
pluginsHttpModuleLoaded: vi.fn(),
}));
vi.mock("./server/http-listen.js", () => ({
@@ -23,6 +25,11 @@ vi.mock("./net.js", async (importOriginal) => {
return { ...actual, resolveGatewayListenHosts: mocks.resolveGatewayListenHosts };
});
vi.mock("./server/plugins-http.js", async (importOriginal) => {
mocks.pluginsHttpModuleLoaded();
return await importOriginal<typeof import("./server/plugins-http.js")>();
});
async function requestPluginUpgrade(port: number, path: string): Promise<string> {
return await new Promise<string>((resolve, reject) => {
const socket = connect({ host: "127.0.0.1", port });
@@ -60,12 +67,88 @@ describe("createGatewayRuntimeState", () => {
mocks.listenGatewayHttpServer.mockResolvedValue(undefined);
mocks.resolveGatewayListenHosts.mockReset();
mocks.resolveGatewayListenHosts.mockResolvedValue(["127.0.0.1"]);
mocks.pluginsHttpModuleLoaded.mockClear();
});
afterEach(() => {
resetPluginRuntimeStateForTest();
});
it("keeps unrelated plugin HTTP routes cold for core HTTP and WebSocket requests", async () => {
const registry = createEmptyPluginRegistry();
registry.httpRoutes.push({
path: "/unrelated",
auth: "plugin",
match: "exact",
handler: () => false,
pluginId: "unrelated",
source: "test",
});
const pluginUpgrade = vi.fn<NonNullable<(typeof registry.httpRoutes)[number]["handleUpgrade"]>>(
(_req, socket) => {
socket.end(
"HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: demo\r\n\r\n",
);
return true;
},
);
registry.httpRoutes.push({
path: "/plugin",
auth: "plugin",
match: "exact",
handler: () => false,
handleUpgrade: pluginUpgrade,
pluginId: "plugin",
source: "test",
});
const getGatewayRequestContext = vi.fn();
const runtimeState = await createGatewayRuntimeStateForTest(registry, {
getGatewayRequestContext,
});
runtimeState.wss.once("connection", (socket) => socket.close());
const server = runtimeState.httpServers[0];
if (!server) {
throw new Error("expected gateway HTTP server");
}
await new Promise<void>((resolve) => {
server.listen(0, "127.0.0.1", resolve);
});
const address = server.address();
if (!address || typeof address === "string") {
throw new Error("expected TCP gateway address");
}
let gatewaySocket: WebSocket | undefined;
try {
await expect(fetch(`http://127.0.0.1:${address.port}/missing`)).resolves.toMatchObject({
status: 404,
});
expect(mocks.pluginsHttpModuleLoaded).not.toHaveBeenCalled();
expect(getGatewayRequestContext).not.toHaveBeenCalled();
gatewaySocket = new WebSocket(`ws://127.0.0.1:${address.port}/`, {
handshakeTimeout: 2_000,
});
await new Promise<void>((resolve, reject) => {
gatewaySocket?.once("open", resolve);
gatewaySocket?.once("error", reject);
});
expect(mocks.pluginsHttpModuleLoaded).not.toHaveBeenCalled();
expect(getGatewayRequestContext).not.toHaveBeenCalled();
expect(pluginUpgrade).not.toHaveBeenCalled();
await expect(requestPluginUpgrade(address.port, "/plugin")).resolves.toContain(
"101 Switching Protocols",
);
expect(mocks.pluginsHttpModuleLoaded).toHaveBeenCalledTimes(1);
expect(pluginUpgrade).toHaveBeenCalledTimes(1);
} finally {
gatewaySocket?.terminate();
await new Promise<void>((resolve, reject) => {
server.close((error) => (error ? reject(error) : resolve()));
});
}
});
it("delegates directly after lazily loading the plugin HTTP handler", async () => {
const registry = createEmptyPluginRegistry();
const routes = registry.httpRoutes;
+19 -6
View File
@@ -51,6 +51,7 @@ import { runWithGatewayHttpWorkAdmission } from "./server/http-work-admission.js
import type { PluginRoutePathContext } from "./server/plugins-http/path-context.js";
import { shouldEnforceGatewayAuthForPluginPath } from "./server/plugins-http/route-auth.js";
import { findMatchingPluginNodeCapabilityRoute } from "./server/plugins-http/route-capability.js";
import { findMatchingPluginHttpRoutes } from "./server/plugins-http/route-match.js";
import {
createPreauthConnectionBudget,
type PreauthConnectionBudget,
@@ -85,6 +86,20 @@ type GatewayPluginUpgradeHandler = (
const loadGatewayPluginsHttpModule = async () => await import("./server/plugins-http.js");
function hasMatchingGatewayPluginRoute(
registry: PluginRegistry,
pathContext: PluginRoutePathContext | undefined,
requiresUpgrade: boolean,
): boolean {
if (!pathContext) {
return (registry.httpRoutes ?? []).length > 0;
}
const matchingRoutes = findMatchingPluginHttpRoutes(registry, pathContext);
return requiresUpgrade
? matchingRoutes.some((route) => typeof route.handleUpgrade === "function")
: matchingRoutes.length > 0;
}
/** Creates the HTTP/WebSocket runtime state for one gateway start. */
export async function createGatewayRuntimeState(params: {
cfg: import("../config/config.js").OpenClawConfig;
@@ -208,11 +223,10 @@ export async function createGatewayRuntimeState(params: {
return await loadedPluginRequestHandler(req, res, pathContext, dispatchContext);
}
const registry = resolvePluginRouteRegistry();
if ((registry.httpRoutes ?? []).length === 0) {
if (!hasMatchingGatewayPluginRoute(registry, pathContext, false)) {
return false;
}
// The loaded handler owns dynamic root-registry lookup; this wrapper only avoids
// importing it for route-free gateways.
// Keep unrelated core HTTP paths cold; the loaded handler still owns dynamic registry lookup.
const { createGatewayPluginRequestHandler } = await loadGatewayPluginsHttpModule();
loadedPluginRequestHandler = createGatewayPluginRequestHandler({
registry: params.pluginRegistry,
@@ -233,11 +247,10 @@ export async function createGatewayRuntimeState(params: {
return await loadedPluginUpgradeHandler(req, socket, head, pathContext, dispatchContext);
}
const registry = resolvePluginRouteRegistry();
if ((registry.httpRoutes ?? []).length === 0) {
if (!hasMatchingGatewayPluginRoute(registry, pathContext, true)) {
return false;
}
// WebSocket upgrades share the loaded handler's dynamic route registry, so reloads still
// follow the active snapshot without a duplicate wrapper lookup on every upgrade.
// Keep core WebSocket upgrades cold while plugin upgrades follow the current route registry.
const { createGatewayPluginUpgradeHandler } = await loadGatewayPluginsHttpModule();
loadedPluginUpgradeHandler = createGatewayPluginUpgradeHandler({
registry: params.pluginRegistry,