refactor(gateway): harden plugin http route contracts

This commit is contained in:
Peter Steinberger
2026-03-02 16:47:51 +00:00
parent 33e76db12a
commit 7a7eee920a
23 changed files with 642 additions and 270 deletions

View File

@@ -9,6 +9,7 @@ describe("registerPluginHttpRoute", () => {
const unregister = registerPluginHttpRoute({
path: "/plugins/demo",
auth: "plugin",
handler,
registry,
});
@@ -16,7 +17,7 @@ describe("registerPluginHttpRoute", () => {
expect(registry.httpRoutes).toHaveLength(1);
expect(registry.httpRoutes[0]?.path).toBe("/plugins/demo");
expect(registry.httpRoutes[0]?.handler).toBe(handler);
expect(registry.httpRoutes[0]?.auth).toBe("gateway");
expect(registry.httpRoutes[0]?.auth).toBe("plugin");
expect(registry.httpRoutes[0]?.match).toBe("exact");
unregister();
@@ -28,6 +29,7 @@ describe("registerPluginHttpRoute", () => {
const logs: string[] = [];
const unregister = registerPluginHttpRoute({
path: "",
auth: "plugin",
handler: vi.fn(),
registry,
accountId: "default",
@@ -39,7 +41,7 @@ describe("registerPluginHttpRoute", () => {
expect(() => unregister()).not.toThrow();
});
it("replaces stale route on same path and keeps latest registration", () => {
it("replaces stale route on same path when replaceExisting=true", () => {
const registry = createEmptyPluginRegistry();
const logs: string[] = [];
const firstHandler = vi.fn();
@@ -47,6 +49,7 @@ describe("registerPluginHttpRoute", () => {
const unregisterFirst = registerPluginHttpRoute({
path: "/plugins/synology",
auth: "plugin",
handler: firstHandler,
registry,
accountId: "default",
@@ -56,6 +59,8 @@ describe("registerPluginHttpRoute", () => {
const unregisterSecond = registerPluginHttpRoute({
path: "/plugins/synology",
auth: "plugin",
replaceExisting: true,
handler: secondHandler,
registry,
accountId: "default",
@@ -77,4 +82,67 @@ describe("registerPluginHttpRoute", () => {
unregisterSecond();
expect(registry.httpRoutes).toHaveLength(0);
});
it("rejects conflicting route registrations without replaceExisting", () => {
const registry = createEmptyPluginRegistry();
const logs: string[] = [];
registerPluginHttpRoute({
path: "/plugins/demo",
auth: "plugin",
handler: vi.fn(),
registry,
pluginId: "demo-a",
source: "demo-a-src",
log: (msg) => logs.push(msg),
});
const unregister = registerPluginHttpRoute({
path: "/plugins/demo",
auth: "plugin",
handler: vi.fn(),
registry,
pluginId: "demo-b",
source: "demo-b-src",
log: (msg) => logs.push(msg),
});
expect(registry.httpRoutes).toHaveLength(1);
expect(logs.at(-1)).toContain("route conflict");
unregister();
expect(registry.httpRoutes).toHaveLength(1);
});
it("rejects route replacement when a different plugin owns the route", () => {
const registry = createEmptyPluginRegistry();
const logs: string[] = [];
registerPluginHttpRoute({
path: "/plugins/demo",
auth: "plugin",
handler: vi.fn(),
registry,
pluginId: "demo-a",
source: "demo-a-src",
log: (msg) => logs.push(msg),
});
const unregister = registerPluginHttpRoute({
path: "/plugins/demo",
auth: "plugin",
replaceExisting: true,
handler: vi.fn(),
registry,
pluginId: "demo-b",
source: "demo-b-src",
log: (msg) => logs.push(msg),
});
expect(registry.httpRoutes).toHaveLength(1);
expect(logs.at(-1)).toContain("route replacement denied");
unregister();
expect(registry.httpRoutes).toHaveLength(1);
});
});

View File

@@ -12,8 +12,9 @@ export function registerPluginHttpRoute(params: {
path?: string | null;
fallbackPath?: string | null;
handler: PluginHttpRouteHandler;
auth?: PluginHttpRouteRegistration["auth"];
auth: PluginHttpRouteRegistration["auth"];
match?: PluginHttpRouteRegistration["match"];
replaceExisting?: boolean;
pluginId?: string;
source?: string;
accountId?: string;
@@ -36,6 +37,22 @@ export function registerPluginHttpRoute(params: {
(entry) => entry.path === normalizedPath && entry.match === routeMatch,
);
if (existingIndex >= 0) {
const existing = routes[existingIndex];
if (!existing) {
return () => {};
}
if (!params.replaceExisting) {
params.log?.(
`plugin: route conflict at ${normalizedPath} (${routeMatch})${suffix}; owned by ${existing.pluginId ?? "unknown-plugin"} (${existing.source ?? "unknown-source"})`,
);
return () => {};
}
if (existing.pluginId && params.pluginId && existing.pluginId !== params.pluginId) {
params.log?.(
`plugin: route replacement denied for ${normalizedPath} (${routeMatch})${suffix}; owned by ${existing.pluginId}`,
);
return () => {};
}
const pluginHint = params.pluginId ? ` (${params.pluginId})` : "";
params.log?.(
`plugin: replacing stale webhook path ${normalizedPath} (${routeMatch})${suffix}${pluginHint}`,
@@ -46,7 +63,7 @@ export function registerPluginHttpRoute(params: {
const entry: PluginHttpRouteRegistration = {
path: normalizedPath,
handler: params.handler,
auth: params.auth ?? "gateway",
auth: params.auth,
match: routeMatch,
pluginId: params.pluginId,
source: params.source,

View File

@@ -548,7 +548,7 @@ describe("loadOpenClawPlugins", () => {
id: "http-route-demo",
filename: "http-route-demo.cjs",
body: `module.exports = { id: "http-route-demo", register(api) {
api.registerHttpRoute({ path: "/demo", handler: async (_req, res) => { res.statusCode = 200; res.end("ok"); } });
api.registerHttpRoute({ path: "/demo", auth: "gateway", handler: async (_req, res) => { res.statusCode = 200; res.end("ok"); } });
} };`,
});
@@ -568,6 +568,95 @@ describe("loadOpenClawPlugins", () => {
expect(httpPlugin?.httpRoutes).toBe(1);
});
it("rejects plugin http routes missing explicit auth", () => {
useNoBundledPlugins();
const plugin = writePlugin({
id: "http-route-missing-auth",
filename: "http-route-missing-auth.cjs",
body: `module.exports = { id: "http-route-missing-auth", register(api) {
api.registerHttpRoute({ path: "/demo", handler: async () => true });
} };`,
});
const registry = loadRegistryFromSinglePlugin({
plugin,
pluginConfig: {
allow: ["http-route-missing-auth"],
},
});
expect(registry.httpRoutes.find((entry) => entry.pluginId === "http-route-missing-auth")).toBe(
undefined,
);
expect(
registry.diagnostics.some((diag) =>
String(diag.message).includes("http route registration missing or invalid auth"),
),
).toBe(true);
});
it("allows explicit replaceExisting for same-plugin http route overrides", () => {
useNoBundledPlugins();
const plugin = writePlugin({
id: "http-route-replace-self",
filename: "http-route-replace-self.cjs",
body: `module.exports = { id: "http-route-replace-self", register(api) {
api.registerHttpRoute({ path: "/demo", auth: "plugin", handler: async () => false });
api.registerHttpRoute({ path: "/demo", auth: "plugin", replaceExisting: true, handler: async () => true });
} };`,
});
const registry = loadRegistryFromSinglePlugin({
plugin,
pluginConfig: {
allow: ["http-route-replace-self"],
},
});
const routes = registry.httpRoutes.filter(
(entry) => entry.pluginId === "http-route-replace-self",
);
expect(routes).toHaveLength(1);
expect(routes[0]?.path).toBe("/demo");
expect(registry.diagnostics).toEqual([]);
});
it("rejects http route replacement when another plugin owns the route", () => {
useNoBundledPlugins();
const first = writePlugin({
id: "http-route-owner-a",
filename: "http-route-owner-a.cjs",
body: `module.exports = { id: "http-route-owner-a", register(api) {
api.registerHttpRoute({ path: "/demo", auth: "plugin", handler: async () => false });
} };`,
});
const second = writePlugin({
id: "http-route-owner-b",
filename: "http-route-owner-b.cjs",
body: `module.exports = { id: "http-route-owner-b", register(api) {
api.registerHttpRoute({ path: "/demo", auth: "plugin", replaceExisting: true, handler: async () => true });
} };`,
});
const registry = loadOpenClawPlugins({
cache: false,
config: {
plugins: {
load: { paths: [first.file, second.file] },
allow: ["http-route-owner-a", "http-route-owner-b"],
},
},
});
const route = registry.httpRoutes.find((entry) => entry.path === "/demo");
expect(route?.pluginId).toBe("http-route-owner-a");
expect(
registry.diagnostics.some((diag) =>
String(diag.message).includes("http route replacement rejected"),
),
).toBe(true);
});
it("respects explicit disable in config", () => {
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = "/nonexistent/bundled/plugins";
const plugin = writePlugin({

View File

@@ -284,6 +284,12 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
record.gatewayMethods.push(trimmed);
};
const describeHttpRouteOwner = (entry: PluginHttpRouteRegistration): string => {
const plugin = entry.pluginId?.trim() || "unknown-plugin";
const source = entry.source?.trim() || "unknown-source";
return `${plugin} (${source})`;
};
const registerHttpRoute = (record: PluginRecord, params: OpenClawPluginHttpRouteParams) => {
const normalizedPath = normalizePluginHttpPath(params.path);
if (!normalizedPath) {
@@ -295,24 +301,58 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
});
return;
}
const match = params.match ?? "exact";
if (
registry.httpRoutes.some((entry) => entry.path === normalizedPath && entry.match === match)
) {
if (params.auth !== "gateway" && params.auth !== "plugin") {
pushDiagnostic({
level: "error",
pluginId: record.id,
source: record.source,
message: `http route already registered: ${normalizedPath} (${match})`,
message: `http route registration missing or invalid auth: ${normalizedPath}`,
});
return;
}
const match = params.match ?? "exact";
const existingIndex = registry.httpRoutes.findIndex(
(entry) => entry.path === normalizedPath && entry.match === match,
);
if (existingIndex >= 0) {
const existing = registry.httpRoutes[existingIndex];
if (!existing) {
return;
}
if (!params.replaceExisting) {
pushDiagnostic({
level: "error",
pluginId: record.id,
source: record.source,
message: `http route already registered: ${normalizedPath} (${match}) by ${describeHttpRouteOwner(existing)}`,
});
return;
}
if (existing.pluginId && existing.pluginId !== record.id) {
pushDiagnostic({
level: "error",
pluginId: record.id,
source: record.source,
message: `http route replacement rejected: ${normalizedPath} (${match}) owned by ${describeHttpRouteOwner(existing)}`,
});
return;
}
registry.httpRoutes[existingIndex] = {
pluginId: record.id,
path: normalizedPath,
handler: params.handler,
auth: params.auth,
match,
source: record.source,
};
return;
}
record.httpRoutes += 1;
registry.httpRoutes.push({
pluginId: record.id,
path: normalizedPath,
handler: params.handler,
auth: params.auth ?? "gateway",
auth: params.auth,
match,
source: record.source,
});

View File

@@ -205,8 +205,9 @@ export type OpenClawPluginHttpRouteHandler = (
export type OpenClawPluginHttpRouteParams = {
path: string;
handler: OpenClawPluginHttpRouteHandler;
auth?: OpenClawPluginHttpRouteAuth;
auth: OpenClawPluginHttpRouteAuth;
match?: OpenClawPluginHttpRouteMatch;
replaceExisting?: boolean;
};
export type OpenClawPluginCliContext = {