diff --git a/CHANGELOG.md b/CHANGELOG.md index b1110eeb925..17c5727c822 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -119,6 +119,7 @@ Docs: https://docs.openclaw.ai - Runtime/tool-state stability: recover from dangling Anthropic `tool_use` after compaction, serialize long-running Discord handler runs without blocking new inbound events, and prevent stale busy snapshots from suppressing stuck-channel recovery. (from #33630, #33583) Thanks @kevinWangSheng and @theotarr. - ACP/Discord startup hardening: clean up stuck ACP worker children on gateway restart, unbind stale ACP thread bindings during Discord startup reconciliation, and add per-thread listener watchdog timeouts so wedged turns cannot block later messages. (#33699) Thanks @dutifulbob. - Extensions/media local-root propagation: consistently forward `mediaLocalRoots` through extension `sendMedia` adapters (Google Chat, Slack, iMessage, Signal, WhatsApp), preserving non-local media behavior while restoring local attachment resolution from configured roots. Synthesis of #33581, #33545, #33540, #33536, #33528. Thanks @bmendonca3. +- Gateway/plugin HTTP auth hardening: require gateway auth when any overlapping matched route needs it, block mixed-auth fallthrough at dispatch, and reject mixed-auth exact/prefix route overlaps during plugin registration. - Feishu/video media send contract: keep mp4-like outbound payloads on `msg_type: "media"` (including reply and reply-in-thread paths) so videos render as media instead of degrading to file-link behavior, while preserving existing non-video file subtype handling. (from #33720, #33808, #33678) Thanks @polooooo, @dingjianrui, and @kevinWangSheng. - Gateway/security default response headers: add `Permissions-Policy: camera=(), microphone=(), geolocation=()` to baseline gateway HTTP security headers for all responses. (#30186) thanks @habakan. - Plugins/startup loading: lazily initialize plugin runtime, split startup-critical plugin SDK imports into `openclaw/plugin-sdk/core` and `openclaw/plugin-sdk/telegram`, and preserve `api.runtime` reflection semantics for plugin compatibility. (#28620) thanks @hmemcpy. diff --git a/docs/tools/plugin.md b/docs/tools/plugin.md index d709f9227c8..77666b7ac11 100644 --- a/docs/tools/plugin.md +++ b/docs/tools/plugin.md @@ -141,6 +141,7 @@ Notes: - `api.registerHttpHandler(...)` is obsolete. Use `api.registerHttpRoute(...)`. - Plugin routes must declare `auth` explicitly. - Exact `path + match` conflicts are rejected unless `replaceExisting: true`, and one plugin cannot replace another plugin's route. +- Overlapping routes with different `auth` levels are rejected. Keep `exact`/`prefix` fallthrough chains on the same auth level only. ## Plugin SDK import paths diff --git a/src/gateway/server-http.ts b/src/gateway/server-http.ts index e5d2a5fa0e1..89db12bc24e 100644 --- a/src/gateway/server-http.ts +++ b/src/gateway/server-http.ts @@ -298,6 +298,7 @@ function buildPluginRequestStages(params: { if (!params.handlePluginRequest) { return []; } + let pluginGatewayAuthSatisfied = false; return [ { name: "plugin-auth", @@ -325,6 +326,7 @@ function buildPluginRequestStages(params: { if (!pluginAuthOk) { return true; } + pluginGatewayAuthSatisfied = true; return false; }, }, @@ -333,7 +335,11 @@ function buildPluginRequestStages(params: { run: () => { const pathContext = params.pluginPathContext ?? resolvePluginRoutePathContext(params.requestPath); - return params.handlePluginRequest?.(params.req, params.res, pathContext) ?? false; + return ( + params.handlePluginRequest?.(params.req, params.res, pathContext, { + gatewayAuthSatisfied: pluginGatewayAuthSatisfied, + }) ?? false + ); }, }, ]; diff --git a/src/gateway/server/plugins-http.test.ts b/src/gateway/server/plugins-http.test.ts index 0610798a7df..391792b0022 100644 --- a/src/gateway/server/plugins-http.test.ts +++ b/src/gateway/server/plugins-http.test.ts @@ -110,6 +110,80 @@ describe("createGatewayPluginRequestHandler", () => { expect(second).toHaveBeenCalledTimes(1); }); + it("fails closed when a matched gateway route reaches dispatch without auth", async () => { + const exactPluginHandler = vi.fn(async () => false); + const prefixGatewayHandler = vi.fn(async () => true); + const handler = createGatewayPluginRequestHandler({ + registry: createTestRegistry({ + httpRoutes: [ + createRoute({ + path: "/plugin/secure/report", + match: "exact", + auth: "plugin", + handler: exactPluginHandler, + }), + createRoute({ + path: "/plugin/secure", + match: "prefix", + auth: "gateway", + handler: prefixGatewayHandler, + }), + ], + }), + log: createPluginLog(), + }); + + const { res } = makeMockHttpResponse(); + const handled = await handler( + { url: "/plugin/secure/report" } as IncomingMessage, + res, + undefined, + { + gatewayAuthSatisfied: false, + }, + ); + expect(handled).toBe(false); + expect(exactPluginHandler).not.toHaveBeenCalled(); + expect(prefixGatewayHandler).not.toHaveBeenCalled(); + }); + + it("allows gateway route fallthrough only after gateway auth succeeds", async () => { + const exactPluginHandler = vi.fn(async () => false); + const prefixGatewayHandler = vi.fn(async () => true); + const handler = createGatewayPluginRequestHandler({ + registry: createTestRegistry({ + httpRoutes: [ + createRoute({ + path: "/plugin/secure/report", + match: "exact", + auth: "plugin", + handler: exactPluginHandler, + }), + createRoute({ + path: "/plugin/secure", + match: "prefix", + auth: "gateway", + handler: prefixGatewayHandler, + }), + ], + }), + log: createPluginLog(), + }); + + const { res } = makeMockHttpResponse(); + const handled = await handler( + { url: "/plugin/secure/report" } as IncomingMessage, + res, + undefined, + { + gatewayAuthSatisfied: true, + }, + ); + expect(handled).toBe(true); + expect(exactPluginHandler).toHaveBeenCalledTimes(1); + expect(prefixGatewayHandler).toHaveBeenCalledTimes(1); + }); + it("matches canonicalized route variants", async () => { const routeHandler = vi.fn(async (_req, res: ServerResponse) => { res.statusCode = 200; @@ -189,4 +263,14 @@ describe("plugin HTTP route auth checks", () => { expect(shouldEnforceGatewayAuthForPluginPath(registry, decodeOverflowPublicPath)).toBe(true); expect(shouldEnforceGatewayAuthForPluginPath(registry, "/not-plugin")).toBe(false); }); + + it("enforces auth when any overlapping matched route requires gateway auth", () => { + const registry = createTestRegistry({ + httpRoutes: [ + createRoute({ path: "/plugin/secure/report", match: "exact", auth: "plugin" }), + createRoute({ path: "/plugin/secure", match: "prefix", auth: "gateway" }), + ], + }); + expect(shouldEnforceGatewayAuthForPluginPath(registry, "/plugin/secure/report")).toBe(true); + }); }); diff --git a/src/gateway/server/plugins-http.ts b/src/gateway/server/plugins-http.ts index 2fd0554bf10..50114a33af6 100644 --- a/src/gateway/server/plugins-http.ts +++ b/src/gateway/server/plugins-http.ts @@ -5,6 +5,7 @@ import { resolvePluginRoutePathContext, type PluginRoutePathContext, } from "./plugins-http/path-context.js"; +import { matchedPluginRoutesRequireGatewayAuth } from "./plugins-http/route-auth.js"; import { findMatchingPluginHttpRoutes } from "./plugins-http/route-match.js"; export { @@ -24,6 +25,7 @@ export type PluginHttpRequestHandler = ( req: IncomingMessage, res: ServerResponse, pathContext?: PluginRoutePathContext, + dispatchContext?: { gatewayAuthSatisfied?: boolean }, ) => Promise; export function createGatewayPluginRequestHandler(params: { @@ -31,7 +33,7 @@ export function createGatewayPluginRequestHandler(params: { log: SubsystemLogger; }): PluginHttpRequestHandler { const { registry, log } = params; - return async (req, res, providedPathContext) => { + return async (req, res, providedPathContext, dispatchContext) => { const routes = registry.httpRoutes ?? []; if (routes.length === 0) { return false; @@ -47,6 +49,13 @@ export function createGatewayPluginRequestHandler(params: { if (matchedRoutes.length === 0) { return false; } + if ( + matchedPluginRoutesRequireGatewayAuth(matchedRoutes) && + dispatchContext?.gatewayAuthSatisfied === false + ) { + log.warn(`plugin http route blocked without gateway auth (${pathContext.canonicalPath})`); + return false; + } for (const route of matchedRoutes) { try { diff --git a/src/gateway/server/plugins-http/route-auth.ts b/src/gateway/server/plugins-http/route-auth.ts index 7549bde34b3..577a0babdfb 100644 --- a/src/gateway/server/plugins-http/route-auth.ts +++ b/src/gateway/server/plugins-http/route-auth.ts @@ -6,6 +6,12 @@ import { } from "./path-context.js"; import { findMatchingPluginHttpRoutes } from "./route-match.js"; +export function matchedPluginRoutesRequireGatewayAuth( + routes: readonly Pick[number], "auth">[], +): boolean { + return routes.some((route) => route.auth === "gateway"); +} + export function shouldEnforceGatewayAuthForPluginPath( registry: PluginRegistry, pathnameOrContext: string | PluginRoutePathContext, @@ -20,9 +26,5 @@ export function shouldEnforceGatewayAuthForPluginPath( if (isProtectedPluginRoutePathFromContext(pathContext)) { return true; } - const route = findMatchingPluginHttpRoutes(registry, pathContext)[0]; - if (!route) { - return false; - } - return route.auth === "gateway"; + return matchedPluginRoutesRequireGatewayAuth(findMatchingPluginHttpRoutes(registry, pathContext)); } diff --git a/src/plugins/http-registry.test.ts b/src/plugins/http-registry.test.ts index 179ddadac5e..9993c7cb39d 100644 --- a/src/plugins/http-registry.test.ts +++ b/src/plugins/http-registry.test.ts @@ -131,4 +131,37 @@ describe("registerPluginHttpRoute", () => { expectedLogFragment: "route replacement denied", }); }); + + it("rejects mixed-auth overlapping routes", () => { + const registry = createEmptyPluginRegistry(); + const logs: string[] = []; + + registerPluginHttpRoute({ + path: "/plugin/secure", + auth: "gateway", + match: "prefix", + handler: vi.fn(), + registry, + pluginId: "demo-gateway", + source: "demo-gateway-src", + log: (msg) => logs.push(msg), + }); + + const unregister = registerPluginHttpRoute({ + path: "/plugin/secure/report", + auth: "plugin", + match: "exact", + handler: vi.fn(), + registry, + pluginId: "demo-plugin", + source: "demo-plugin-src", + log: (msg) => logs.push(msg), + }); + + expect(registry.httpRoutes).toHaveLength(1); + expect(logs.at(-1)).toContain("route overlap denied"); + + unregister(); + expect(registry.httpRoutes).toHaveLength(1); + }); }); diff --git a/src/plugins/http-registry.ts b/src/plugins/http-registry.ts index a1af2cf9fc4..bf45f1b076a 100644 --- a/src/plugins/http-registry.ts +++ b/src/plugins/http-registry.ts @@ -1,5 +1,6 @@ import type { IncomingMessage, ServerResponse } from "node:http"; import { normalizePluginHttpPath } from "./http-path.js"; +import { findOverlappingPluginHttpRoute } from "./http-route-overlap.js"; import type { PluginHttpRouteRegistration, PluginRegistry } from "./registry.js"; import { requireActivePluginRegistry } from "./runtime.js"; @@ -33,6 +34,18 @@ export function registerPluginHttpRoute(params: { } const routeMatch = params.match ?? "exact"; + const overlappingRoute = findOverlappingPluginHttpRoute(routes, { + path: normalizedPath, + match: routeMatch, + }); + if (overlappingRoute && overlappingRoute.auth !== params.auth) { + params.log?.( + `plugin: route overlap denied at ${normalizedPath} (${routeMatch}, ${params.auth})${suffix}; ` + + `overlaps ${overlappingRoute.path} (${overlappingRoute.match}, ${overlappingRoute.auth}) ` + + `owned by ${overlappingRoute.pluginId ?? "unknown-plugin"} (${overlappingRoute.source ?? "unknown-source"})`, + ); + return () => {}; + } const existingIndex = routes.findIndex( (entry) => entry.path === normalizedPath && entry.match === routeMatch, ); diff --git a/src/plugins/http-route-overlap.ts b/src/plugins/http-route-overlap.ts new file mode 100644 index 00000000000..fa2c46cc185 --- /dev/null +++ b/src/plugins/http-route-overlap.ts @@ -0,0 +1,44 @@ +import { canonicalizePathVariant } from "../gateway/security-path.js"; +import type { OpenClawPluginHttpRouteMatch } from "./types.js"; + +type PluginHttpRouteLike = { + path: string; + match: OpenClawPluginHttpRouteMatch; +}; + +function prefixMatchPath(pathname: string, prefix: string): boolean { + return ( + pathname === prefix || pathname.startsWith(`${prefix}/`) || pathname.startsWith(`${prefix}%`) + ); +} + +export function doPluginHttpRoutesOverlap( + a: Pick, + b: Pick, +): boolean { + const aPath = canonicalizePathVariant(a.path); + const bPath = canonicalizePathVariant(b.path); + + if (a.match === "exact" && b.match === "exact") { + return aPath === bPath; + } + if (a.match === "prefix" && b.match === "prefix") { + return prefixMatchPath(aPath, bPath) || prefixMatchPath(bPath, aPath); + } + + const prefixRoute = a.match === "prefix" ? a : b; + const exactRoute = a.match === "exact" ? a : b; + return prefixMatchPath( + canonicalizePathVariant(exactRoute.path), + canonicalizePathVariant(prefixRoute.path), + ); +} + +export function findOverlappingPluginHttpRoute< + T extends { + path: string; + match: OpenClawPluginHttpRouteMatch; + }, +>(routes: readonly T[], candidate: PluginHttpRouteLike): T | undefined { + return routes.find((route) => doPluginHttpRoutesOverlap(route, candidate)); +} diff --git a/src/plugins/loader.test.ts b/src/plugins/loader.test.ts index cdd23edbfa8..a4aea837757 100644 --- a/src/plugins/loader.test.ts +++ b/src/plugins/loader.test.ts @@ -731,6 +731,59 @@ describe("loadOpenClawPlugins", () => { ).toBe(true); }); + it("rejects mixed-auth overlapping http routes", () => { + useNoBundledPlugins(); + const plugin = writePlugin({ + id: "http-route-overlap", + filename: "http-route-overlap.cjs", + body: `module.exports = { id: "http-route-overlap", register(api) { + api.registerHttpRoute({ path: "/plugin/secure", auth: "gateway", match: "prefix", handler: async () => true }); + api.registerHttpRoute({ path: "/plugin/secure/report", auth: "plugin", match: "exact", handler: async () => true }); +} };`, + }); + + const registry = loadRegistryFromSinglePlugin({ + plugin, + pluginConfig: { + allow: ["http-route-overlap"], + }, + }); + + const routes = registry.httpRoutes.filter((entry) => entry.pluginId === "http-route-overlap"); + expect(routes).toHaveLength(1); + expect(routes[0]?.path).toBe("/plugin/secure"); + expect( + registry.diagnostics.some((diag) => + String(diag.message).includes("http route overlap rejected"), + ), + ).toBe(true); + }); + + it("allows same-auth overlapping http routes", () => { + useNoBundledPlugins(); + const plugin = writePlugin({ + id: "http-route-overlap-same-auth", + filename: "http-route-overlap-same-auth.cjs", + body: `module.exports = { id: "http-route-overlap-same-auth", register(api) { + api.registerHttpRoute({ path: "/plugin/public", auth: "plugin", match: "prefix", handler: async () => true }); + api.registerHttpRoute({ path: "/plugin/public/report", auth: "plugin", match: "exact", handler: async () => true }); +} };`, + }); + + const registry = loadRegistryFromSinglePlugin({ + plugin, + pluginConfig: { + allow: ["http-route-overlap-same-auth"], + }, + }); + + const routes = registry.httpRoutes.filter( + (entry) => entry.pluginId === "http-route-overlap-same-auth", + ); + expect(routes).toHaveLength(2); + expect(registry.diagnostics).toEqual([]); + }); + it("respects explicit disable in config", () => { process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = "/nonexistent/bundled/plugins"; const plugin = writePlugin({ diff --git a/src/plugins/registry.ts b/src/plugins/registry.ts index 9fc797ab235..37947fce707 100644 --- a/src/plugins/registry.ts +++ b/src/plugins/registry.ts @@ -12,6 +12,7 @@ import type { HookEntry } from "../hooks/types.js"; import { resolveUserPath } from "../utils.js"; import { registerPluginCommand } from "./commands.js"; import { normalizePluginHttpPath } from "./http-path.js"; +import { findOverlappingPluginHttpRoute } from "./http-route-overlap.js"; import type { PluginRuntime } from "./runtime/types.js"; import { isPluginHookName, @@ -335,6 +336,22 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { return; } const match = params.match ?? "exact"; + const overlappingRoute = findOverlappingPluginHttpRoute(registry.httpRoutes, { + path: normalizedPath, + match, + }); + if (overlappingRoute && overlappingRoute.auth !== params.auth) { + pushDiagnostic({ + level: "error", + pluginId: record.id, + source: record.source, + message: + `http route overlap rejected: ${normalizedPath} (${match}, ${params.auth}) ` + + `overlaps ${overlappingRoute.path} (${overlappingRoute.match}, ${overlappingRoute.auth}) ` + + `owned by ${describeHttpRouteOwner(overlappingRoute)}`, + }); + return; + } const existingIndex = registry.httpRoutes.findIndex( (entry) => entry.path === normalizedPath && entry.match === match, );