From 6632fd1ea94775536a0fadc428fa313122ac86d4 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 26 Feb 2026 13:01:12 +0100 Subject: [PATCH] refactor(security): extract protected-route path policy helpers --- src/gateway/security-path.test.ts | 48 +++++++ src/gateway/security-path.ts | 59 ++++++++ src/gateway/server-http.ts | 104 ++++++-------- src/gateway/server.plugin-http-auth.test.ts | 152 ++++++++++++++++---- 4 files changed, 270 insertions(+), 93 deletions(-) create mode 100644 src/gateway/security-path.test.ts create mode 100644 src/gateway/security-path.ts diff --git a/src/gateway/security-path.test.ts b/src/gateway/security-path.test.ts new file mode 100644 index 00000000000..371cbf10d30 --- /dev/null +++ b/src/gateway/security-path.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from "vitest"; +import { + PROTECTED_PLUGIN_ROUTE_PREFIXES, + canonicalizePathForSecurity, + isPathProtectedByPrefixes, + isProtectedPluginRoutePath, +} from "./security-path.js"; + +describe("security-path canonicalization", () => { + it("canonicalizes decoded case/slash variants", () => { + expect(canonicalizePathForSecurity("/API/channels//nostr/default/profile/")).toEqual({ + path: "/api/channels/nostr/default/profile", + malformedEncoding: false, + rawNormalizedPath: "/api/channels/nostr/default/profile", + }); + expect(canonicalizePathForSecurity("/api/%63hannels%2Fnostr%2Fdefault%2Fprofile").path).toBe( + "/api/channels/nostr/default/profile", + ); + }); + + it("marks malformed encoding", () => { + expect(canonicalizePathForSecurity("/api/channels%2").malformedEncoding).toBe(true); + expect(canonicalizePathForSecurity("/api/channels%zz").malformedEncoding).toBe(true); + }); +}); + +describe("security-path protected-prefix matching", () => { + const channelVariants = [ + "/API/channels/nostr/default/profile", + "/api/channels%2Fnostr%2Fdefault%2Fprofile", + "/api/%63hannels/nostr/default/profile", + "/api/channels%2", + "/api/channels%zz", + ]; + + for (const path of channelVariants) { + it(`protects plugin channel path variant: ${path}`, () => { + expect(isProtectedPluginRoutePath(path)).toBe(true); + expect(isPathProtectedByPrefixes(path, PROTECTED_PLUGIN_ROUTE_PREFIXES)).toBe(true); + }); + } + + it("does not protect unrelated paths", () => { + expect(isProtectedPluginRoutePath("/plugin/public")).toBe(false); + expect(isProtectedPluginRoutePath("/api/channels-public")).toBe(false); + expect(isProtectedPluginRoutePath("/api/channel")).toBe(false); + }); +}); diff --git a/src/gateway/security-path.ts b/src/gateway/security-path.ts new file mode 100644 index 00000000000..a797c7b9478 --- /dev/null +++ b/src/gateway/security-path.ts @@ -0,0 +1,59 @@ +export type SecurityPathCanonicalization = { + path: string; + malformedEncoding: boolean; + rawNormalizedPath: string; +}; + +function normalizePathSeparators(pathname: string): string { + const collapsed = pathname.replace(/\/{2,}/g, "/"); + if (collapsed.length <= 1) { + return collapsed; + } + return collapsed.replace(/\/+$/, ""); +} + +function normalizeProtectedPrefix(prefix: string): string { + return normalizePathSeparators(prefix.toLowerCase()) || "/"; +} + +function prefixMatch(pathname: string, prefix: string): boolean { + return ( + pathname === prefix || + pathname.startsWith(`${prefix}/`) || + // Fail closed when malformed %-encoding follows the protected prefix. + pathname.startsWith(`${prefix}%`) + ); +} + +export function canonicalizePathForSecurity(pathname: string): SecurityPathCanonicalization { + let decoded = pathname; + let malformedEncoding = false; + try { + decoded = decodeURIComponent(pathname); + } catch { + malformedEncoding = true; + } + return { + path: normalizePathSeparators(decoded.toLowerCase()) || "/", + malformedEncoding, + rawNormalizedPath: normalizePathSeparators(pathname.toLowerCase()) || "/", + }; +} + +export function isPathProtectedByPrefixes(pathname: string, prefixes: readonly string[]): boolean { + const canonical = canonicalizePathForSecurity(pathname); + const normalizedPrefixes = prefixes.map(normalizeProtectedPrefix); + if (normalizedPrefixes.some((prefix) => prefixMatch(canonical.path, prefix))) { + return true; + } + if (!canonical.malformedEncoding) { + return false; + } + return normalizedPrefixes.some((prefix) => prefixMatch(canonical.rawNormalizedPath, prefix)); +} + +export const PROTECTED_PLUGIN_ROUTE_PREFIXES = ["/api/channels"] as const; + +export function isProtectedPluginRoutePath(pathname: string): boolean { + return isPathProtectedByPrefixes(pathname, PROTECTED_PLUGIN_ROUTE_PREFIXES); +} diff --git a/src/gateway/server-http.ts b/src/gateway/server-http.ts index 2de935e2502..92ac0c7ebfa 100644 --- a/src/gateway/server-http.ts +++ b/src/gateway/server-http.ts @@ -59,6 +59,7 @@ import { getBearerToken } from "./http-utils.js"; import { handleOpenAiHttpRequest } from "./openai-http.js"; import { handleOpenResponsesHttpRequest } from "./openresponses-http.js"; import { GATEWAY_CLIENT_MODES, normalizeGatewayClientMode } from "./protocol/client-info.js"; +import { isProtectedPluginRoutePath } from "./security-path.js"; import type { GatewayWsClient } from "./server/ws-types.js"; import { handleToolsInvokeHttpRequest } from "./tools-invoke-http.js"; @@ -88,52 +89,6 @@ function isCanvasPath(pathname: string): boolean { ); } -function normalizeSecurityPath(pathname: string): string { - const collapsed = pathname.replace(/\/{2,}/g, "/"); - if (collapsed.length <= 1) { - return collapsed; - } - return collapsed.replace(/\/+$/, ""); -} - -function canonicalizePathForSecurity(pathname: string): { - path: string; - malformedEncoding: boolean; -} { - let decoded = pathname; - let malformedEncoding = false; - try { - decoded = decodeURIComponent(pathname); - } catch { - malformedEncoding = true; - } - return { - path: normalizeSecurityPath(decoded.toLowerCase()) || "/", - malformedEncoding, - }; -} - -function hasProtectedPluginChannelPrefix(pathname: string): boolean { - return ( - pathname === "/api/channels" || - pathname.startsWith("/api/channels/") || - pathname.startsWith("/api/channels%") - ); -} - -function isProtectedPluginChannelPath(pathname: string): boolean { - const canonicalPath = canonicalizePathForSecurity(pathname); - if (hasProtectedPluginChannelPrefix(canonicalPath.path)) { - return true; - } - if (!canonicalPath.malformedEncoding) { - return false; - } - // Fail closed on bad %-encoding. Keep channel-prefix paths protected even - // when URL decoding fails. - return hasProtectedPluginChannelPrefix(normalizeSecurityPath(pathname.toLowerCase())); -} - function isNodeWsClient(client: GatewayWsClient): boolean { if (client.connect.role === "node") { return true; @@ -215,6 +170,34 @@ async function authorizeCanvasRequest(params: { return lastAuthFailure ?? { ok: false, reason: "unauthorized" }; } +async function enforcePluginRouteGatewayAuth(params: { + requestPath: string; + req: IncomingMessage; + res: ServerResponse; + auth: ResolvedGatewayAuth; + trustedProxies: string[]; + allowRealIpFallback: boolean; + rateLimiter?: AuthRateLimiter; +}): Promise { + if (!isProtectedPluginRoutePath(params.requestPath)) { + return true; + } + const token = getBearerToken(params.req); + const authResult = await authorizeHttpGatewayConnect({ + auth: params.auth, + connectAuth: token ? { token, password: token } : null, + req: params.req, + trustedProxies: params.trustedProxies, + allowRealIpFallback: params.allowRealIpFallback, + rateLimiter: params.rateLimiter, + }); + if (!authResult.ok) { + sendGatewayAuthFailure(params.res, authResult); + return false; + } + return true; +} + function writeUpgradeAuthFailure( socket: { write: (chunk: string) => void }, auth: GatewayAuthResult, @@ -536,23 +519,20 @@ export function createGatewayHttpServer(opts: { return; } if (handlePluginRequest) { - // Channel HTTP endpoints are gateway-auth protected by default. - // Non-channel plugin routes remain plugin-owned and must enforce + // Protected plugin route prefixes are gateway-auth protected by default. + // Non-protected plugin routes remain plugin-owned and must enforce // their own auth when exposing sensitive functionality. - if (isProtectedPluginChannelPath(requestPath)) { - const token = getBearerToken(req); - const authResult = await authorizeHttpGatewayConnect({ - auth: resolvedAuth, - connectAuth: token ? { token, password: token } : null, - req, - trustedProxies, - allowRealIpFallback, - rateLimiter, - }); - if (!authResult.ok) { - sendGatewayAuthFailure(res, authResult); - return; - } + const pluginAuthOk = await enforcePluginRouteGatewayAuth({ + requestPath, + req, + res, + auth: resolvedAuth, + trustedProxies, + allowRealIpFallback, + rateLimiter, + }); + if (!pluginAuthOk) { + return; } if (await handlePluginRequest(req, res)) { return; diff --git a/src/gateway/server.plugin-http-auth.test.ts b/src/gateway/server.plugin-http-auth.test.ts index 6a931ff505e..66852604088 100644 --- a/src/gateway/server.plugin-http-auth.test.ts +++ b/src/gateway/server.plugin-http-auth.test.ts @@ -100,6 +100,75 @@ function canonicalizePluginPath(pathname: string): string { return collapsed.replace(/\/+$/, ""); } +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-segment", path: "/api/%63hannels/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-segment", path: "/api/%63hannels/nostr/default/profile" }, + { label: "auth-duplicate-trailing-slash", path: "/api/channels//nostr/default/profile/" }, +]; + +function buildChannelPathFuzzCorpus(): RouteVariant[] { + const variants = [ + "/api/channels/nostr/default/profile", + "/API/channels/nostr/default/profile", + "/api/channels//nostr/default/profile/", + "/api/channels%2Fnostr%2Fdefault%2Fprofile", + "/api/channels%252Fnostr%252Fdefault%252Fprofile", + "/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: ReturnType; + variants: RouteVariant[]; +}) { + for (const variant of params.variants) { + const response = createResponse(); + await dispatchRequest(params.server, createRequest({ path: variant.path }), response.res); + expect(response.res.statusCode, variant.label).toBe(401); + expect(response.getBody(), variant.label).toContain("Unauthorized"); + } +} + +async function expectAuthorizedVariants(params: { + server: ReturnType; + variants: RouteVariant[]; + authorization: string; +}) { + for (const variant of params.variants) { + const response = createResponse(); + await dispatchRequest( + params.server, + createRequest({ + path: variant.path, + authorization: params.authorization, + }), + response.res, + ); + 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 () => { const resolvedAuth: ResolvedGatewayAuth = { @@ -292,42 +361,63 @@ describe("gateway plugin HTTP auth boundary", () => { resolvedAuth, }); - const unauthenticatedVariants = [ - "/API/channels/nostr/default/profile", - "/api/channels%2Fnostr%2Fdefault%2Fprofile", - "/api/%63hannels/nostr/default/profile", - "/api/channels//nostr/default/profile", - "/api/channels/nostr/default/profile/", - "/api/channels%2", - "/api//channels%2", - ]; - for (const path of unauthenticatedVariants) { - const response = createResponse(); - await dispatchRequest(server, createRequest({ path }), response.res); - expect(response.res.statusCode).toBe(401); - expect(response.getBody()).toContain("Unauthorized"); - } + await expectUnauthorizedVariants({ server, variants: CANONICAL_UNAUTH_VARIANTS }); expect(handlePluginRequest).not.toHaveBeenCalled(); - const authenticatedVariants = [ - "/API/channels/nostr/default/profile", - "/api/%63hannels/nostr/default/profile", - "/api/channels//nostr/default/profile/", - ]; - for (const path of authenticatedVariants) { + await expectAuthorizedVariants({ + server, + variants: CANONICAL_AUTH_VARIANTS, + authorization: "Bearer test-token", + }); + expect(handlePluginRequest).toHaveBeenCalledTimes(CANONICAL_AUTH_VARIANTS.length); + }, + }); + }); + + test("rejects unauthenticated plugin-channel fuzz corpus variants", async () => { + const resolvedAuth: ResolvedGatewayAuth = { + mode: "token", + token: "test-token", + password: undefined, + allowTailscale: false, + }; + + await withTempConfig({ + cfg: { gateway: { trustedProxies: [] } }, + prefix: "openclaw-plugin-http-auth-fuzz-corpus-test-", + run: async () => { + const handlePluginRequest = 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") { + res.statusCode = 200; + res.setHeader("Content-Type", "application/json; charset=utf-8"); + res.end(JSON.stringify({ ok: true, route: "channel-canonicalized" })); + return true; + } + return false; + }); + + const server = createGatewayHttpServer({ + canvasHost: null, + clients: new Set(), + controlUiEnabled: false, + controlUiBasePath: "/__control__", + openAiChatCompletionsEnabled: false, + openResponsesEnabled: false, + handleHooksRequest: async () => false, + handlePluginRequest, + resolvedAuth, + }); + + for (const variant of buildChannelPathFuzzCorpus()) { const response = createResponse(); - await dispatchRequest( - server, - createRequest({ - path, - authorization: "Bearer test-token", - }), - response.res, + await dispatchRequest(server, createRequest({ path: variant.path }), response.res); + expect(response.res.statusCode, variant.label).not.toBe(200); + expect(response.getBody(), variant.label).not.toContain( + '"route":"channel-canonicalized"', ); - expect(response.res.statusCode).toBe(200); - expect(response.getBody()).toContain('"route":"channel-canonicalized"'); } - expect(handlePluginRequest).toHaveBeenCalledTimes(authenticatedVariants.length); }, }); });