diff --git a/CHANGELOG.md b/CHANGELOG.md index 58cbce1585e..6e6d7b2d97d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ Docs: https://docs.openclaw.ai - Zalo/Pairing auth tests: add webhook regression coverage asserting DM pairing-store reads/writes remain account-scoped, preventing cross-account authorization bleed in multi-account setups. (#26121) Thanks @bmendonca3. - Zalouser/Pairing auth tests: add account-scoped DM pairing-store regression coverage (`monitor.account-scope.test.ts`) to prevent cross-account allowlist bleed in multi-account setups. (#26672) Thanks @bmendonca3. - Security/Web tools SSRF guard: keep DNS pinning for untrusted `web_fetch` and citation-redirect URL checks when proxy env vars are set, and require explicit dangerous opt-in before env-proxy routing can bypass pinned dispatch for trusted/operator-controlled endpoints. Thanks @tdjackey for reporting. +- Gateway/Security canonicalization hardening: decode plugin route path variants to canonical fixpoint (with bounded depth), fail closed on canonicalization anomalies, and enforce gateway auth for deeply encoded `/api/channels/*` variants to prevent alternate-path auth bypass through plugin handlers. Thanks @tdjackey for reporting. - Agents/Sessions list transcript paths: handle missing/non-string/relative `sessions.list.path` values and per-agent `{agentId}` templates when deriving `transcriptPath`, so cross-agent session listings resolve to concrete agent session files instead of workspace-relative paths. (#24775) Thanks @martinfrancois. - macOS/PeekabooBridge: add compatibility socket symlinks for legacy `clawdbot`, `clawdis`, and `moltbot` Application Support socket paths so pre-rename clients can still connect. (#6033) Thanks @lumpinif and @vincentkoc. - Webchat/Feishu session continuation: preserve routable `OriginatingChannel`/`OriginatingTo` metadata from session delivery context in `chat.send`, and prefer provider-normalized channel when deciding cross-channel route dispatch so Webchat replies continue on the selected Feishu session instead of falling back to main/internal session routing. (#31573) diff --git a/src/gateway/security-path.test.ts b/src/gateway/security-path.test.ts index f665efbfb35..366fd2237e2 100644 --- a/src/gateway/security-path.test.ts +++ b/src/gateway/security-path.test.ts @@ -1,23 +1,38 @@ import { describe, expect, it } from "vitest"; import { PROTECTED_PLUGIN_ROUTE_PREFIXES, + buildCanonicalPathCandidates, canonicalizePathForSecurity, isPathProtectedByPrefixes, isProtectedPluginRoutePath, } from "./security-path.js"; +function buildRepeatedEncodedSlashPath(depth: number): string { + let encodedSlash = "%2f"; + for (let i = 1; i < depth; i++) { + encodedSlash = encodedSlash.replace(/%/g, "%25"); + } + return `/api${encodedSlash}channels${encodedSlash}nostr${encodedSlash}default${encodedSlash}profile`; +} + describe("security-path canonicalization", () => { it("canonicalizes decoded case/slash variants", () => { - expect(canonicalizePathForSecurity("/API/channels//nostr/default/profile/")).toEqual({ - canonicalPath: "/api/channels/nostr/default/profile", - candidates: ["/api/channels/nostr/default/profile"], - malformedEncoding: false, - rawNormalizedPath: "/api/channels/nostr/default/profile", - }); + expect(canonicalizePathForSecurity("/API/channels//nostr/default/profile/")).toEqual( + expect.objectContaining({ + canonicalPath: "/api/channels/nostr/default/profile", + candidates: ["/api/channels/nostr/default/profile"], + malformedEncoding: false, + decodePasses: 0, + decodePassLimitReached: false, + rawNormalizedPath: "/api/channels/nostr/default/profile", + }), + ); const encoded = canonicalizePathForSecurity("/api/%63hannels%2Fnostr%2Fdefault%2Fprofile"); expect(encoded.canonicalPath).toBe("/api/channels/nostr/default/profile"); expect(encoded.candidates).toContain("/api/%63hannels%2fnostr%2fdefault%2fprofile"); expect(encoded.candidates).toContain("/api/channels/nostr/default/profile"); + expect(encoded.decodePasses).toBeGreaterThan(0); + expect(encoded.decodePassLimitReached).toBe(false); }); it("resolves traversal after repeated decoding", () => { @@ -34,6 +49,22 @@ describe("security-path canonicalization", () => { expect(canonicalizePathForSecurity("/api/channels%2").malformedEncoding).toBe(true); expect(canonicalizePathForSecurity("/api/channels%zz").malformedEncoding).toBe(true); }); + + it("resolves 4x encoded slash path variants to protected channel routes", () => { + const deeplyEncoded = "/api%2525252fchannels%2525252fnostr%2525252fdefault%2525252fprofile"; + const canonical = canonicalizePathForSecurity(deeplyEncoded); + expect(canonical.canonicalPath).toBe("/api/channels/nostr/default/profile"); + expect(canonical.decodePasses).toBeGreaterThanOrEqual(4); + expect(isProtectedPluginRoutePath(deeplyEncoded)).toBe(true); + }); + + it("flags decode depth overflow and fails closed for protected prefix checks", () => { + const excessiveDepthPath = buildRepeatedEncodedSlashPath(40); + const candidates = buildCanonicalPathCandidates(excessiveDepthPath, 32); + expect(candidates.decodePassLimitReached).toBe(true); + expect(candidates.malformedEncoding).toBe(false); + expect(isProtectedPluginRoutePath(excessiveDepthPath)).toBe(true); + }); }); describe("security-path protected-prefix matching", () => { @@ -44,6 +75,7 @@ describe("security-path protected-prefix matching", () => { "/api/foo/..%2fchannels/nostr/default/profile", "/api/foo/%2e%2e%2fchannels/nostr/default/profile", "/api/foo/%252e%252e%252fchannels/nostr/default/profile", + "/api%2525252fchannels%2525252fnostr%2525252fdefault%2525252fprofile", "/api/channels%2", "/api/channels%zz", ]; diff --git a/src/gateway/security-path.ts b/src/gateway/security-path.ts index 7b9fa493aac..f1e9857fd33 100644 --- a/src/gateway/security-path.ts +++ b/src/gateway/security-path.ts @@ -1,11 +1,13 @@ export type SecurityPathCanonicalization = { canonicalPath: string; candidates: string[]; + decodePasses: number; + decodePassLimitReached: boolean; malformedEncoding: boolean; rawNormalizedPath: string; }; -const MAX_PATH_DECODE_PASSES = 3; +const MAX_PATH_DECODE_PASSES = 32; function normalizePathSeparators(pathname: string): string { const collapsed = pathname.replace(/\/{2,}/g, "/"); @@ -43,13 +45,19 @@ function pushNormalizedCandidate(candidates: string[], seen: Set, value: export function buildCanonicalPathCandidates( pathname: string, maxDecodePasses = MAX_PATH_DECODE_PASSES, -): { candidates: string[]; malformedEncoding: boolean } { +): { + candidates: string[]; + decodePasses: number; + decodePassLimitReached: boolean; + malformedEncoding: boolean; +} { const candidates: string[] = []; const seen = new Set(); pushNormalizedCandidate(candidates, seen, pathname); let decoded = pathname; let malformedEncoding = false; + let decodePasses = 0; for (let pass = 0; pass < maxDecodePasses; pass++) { let nextDecoded = decoded; try { @@ -61,10 +69,24 @@ export function buildCanonicalPathCandidates( if (nextDecoded === decoded) { break; } + decodePasses += 1; decoded = nextDecoded; pushNormalizedCandidate(candidates, seen, decoded); } - return { candidates, malformedEncoding }; + let decodePassLimitReached = false; + if (!malformedEncoding) { + try { + decodePassLimitReached = decodeURIComponent(decoded) !== decoded; + } catch { + malformedEncoding = true; + } + } + return { + candidates, + decodePasses, + decodePassLimitReached, + malformedEncoding, + }; } export function canonicalizePathVariant(pathname: string): string { @@ -82,16 +104,24 @@ function prefixMatch(pathname: string, prefix: string): boolean { } export function canonicalizePathForSecurity(pathname: string): SecurityPathCanonicalization { - const { candidates, malformedEncoding } = buildCanonicalPathCandidates(pathname); + const { candidates, decodePasses, decodePassLimitReached, malformedEncoding } = + buildCanonicalPathCandidates(pathname); return { canonicalPath: candidates[candidates.length - 1] ?? "/", candidates, + decodePasses, + decodePassLimitReached, malformedEncoding, rawNormalizedPath: normalizePathSeparators(pathname.toLowerCase()) || "/", }; } +export function hasSecurityPathCanonicalizationAnomaly(pathname: string): boolean { + const canonical = canonicalizePathForSecurity(pathname); + return canonical.malformedEncoding || canonical.decodePassLimitReached; +} + const normalizedPrefixesCache = new WeakMap(); function getNormalizedPrefixes(prefixes: readonly string[]): readonly string[] { @@ -114,6 +144,10 @@ export function isPathProtectedByPrefixes(pathname: string, prefixes: readonly s ) { return true; } + // Fail closed when canonicalization depth cannot be fully resolved. + if (canonical.decodePassLimitReached) { + return true; + } if (!canonical.malformedEncoding) { return false; } diff --git a/src/gateway/server-http.ts b/src/gateway/server-http.ts index fb27a615539..22d70bb0041 100644 --- a/src/gateway/server-http.ts +++ b/src/gateway/server-http.ts @@ -48,6 +48,7 @@ import { import { sendGatewayAuthFailure, setDefaultSecurityHeaders } from "./http-common.js"; import { handleOpenAiHttpRequest } from "./openai-http.js"; import { handleOpenResponsesHttpRequest } from "./openresponses-http.js"; +import { hasSecurityPathCanonicalizationAnomaly } from "./security-path.js"; import { isProtectedPluginRoutePath } from "./security-path.js"; import { authorizeCanvasRequest, @@ -80,6 +81,10 @@ const GATEWAY_PROBE_STATUS_BY_PATH = new Map([ ["/readyz", "ready"], ]); +function shouldEnforceDefaultPluginGatewayAuth(pathname: string): boolean { + return hasSecurityPathCanonicalizationAnomaly(pathname) || isProtectedPluginRoutePath(pathname); +} + function handleGatewayProbeRequest( req: IncomingMessage, res: ServerResponse, @@ -511,7 +516,9 @@ export function createGatewayHttpServer(opts: { // Plugins run after built-in gateway routes so core surfaces keep // precedence on overlapping paths. if (handlePluginRequest) { - if ((shouldEnforcePluginGatewayAuth ?? isProtectedPluginRoutePath)(requestPath)) { + if ( + (shouldEnforcePluginGatewayAuth ?? shouldEnforceDefaultPluginGatewayAuth)(requestPath) + ) { const pluginAuthOk = await enforcePluginRouteGatewayAuth({ req, res, diff --git a/src/gateway/server.plugin-http-auth.test.ts b/src/gateway/server.plugin-http-auth.test.ts index 4db9e04329f..bfe0b06ebfa 100644 --- a/src/gateway/server.plugin-http-auth.test.ts +++ b/src/gateway/server.plugin-http-auth.test.ts @@ -181,6 +181,10 @@ type RouteVariant = { 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" }, { @@ -199,6 +203,10 @@ const CANONICAL_UNAUTH_VARIANTS: RouteVariant[] = [ 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/" }, { @@ -221,6 +229,7 @@ function buildChannelPathFuzzCorpus(): RouteVariant[] { "/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", @@ -454,7 +463,7 @@ describe("gateway plugin HTTP auth boundary", () => { test("uses /api/channels auth by default while keeping wildcard handlers ungated with no predicate", async () => { const handlePluginRequest = vi.fn(async (req: IncomingMessage, res: ServerResponse) => { const pathname = new URL(req.url ?? "/", "http://localhost").pathname; - if (pathname === "/api/channels/nostr/default/profile") { + if (canonicalizePluginPath(pathname) === "/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-default" })); @@ -483,6 +492,11 @@ describe("gateway plugin HTTP auth boundary", () => { }); expectUnauthorizedResponse(unauthenticatedChannel); + const unauthenticatedDeepEncodedChannel = await sendRequest(server, { + path: "/api%2525252fchannels%2525252fnostr%2525252fdefault%2525252fprofile", + }); + expectUnauthorizedResponse(unauthenticatedDeepEncodedChannel); + const authenticated = await sendRequest(server, { path: "/googlechat", authorization: "Bearer test-token", @@ -496,6 +510,13 @@ describe("gateway plugin HTTP auth boundary", () => { }); expect(authenticatedChannel.res.statusCode).toBe(200); expect(authenticatedChannel.getBody()).toContain('"route":"channel-default"'); + + const authenticatedDeepEncodedChannel = await sendRequest(server, { + path: "/api%2525252fchannels%2525252fnostr%2525252fdefault%2525252fprofile", + authorization: "Bearer test-token", + }); + expect(authenticatedDeepEncodedChannel.res.statusCode).toBe(200); + expect(authenticatedDeepEncodedChannel.getBody()).toContain('"route":"channel-default"'); }, }); }); diff --git a/src/gateway/server/plugins-http.test.ts b/src/gateway/server/plugins-http.test.ts index 535067dcaaa..4fe07c05fd1 100644 --- a/src/gateway/server/plugins-http.test.ts +++ b/src/gateway/server/plugins-http.test.ts @@ -27,6 +27,14 @@ function createRoute(params: { }; } +function buildRepeatedEncodedSlash(depth: number): string { + let encodedSlash = "%2f"; + for (let i = 1; i < depth; i++) { + encodedSlash = encodedSlash.replace(/%/g, "%25"); + } + return encodedSlash; +} + describe("createGatewayPluginRequestHandler", () => { it("returns false when no handlers are registered", async () => { const log = createPluginLog(); @@ -127,6 +135,10 @@ describe("createGatewayPluginRequestHandler", () => { }); describe("plugin HTTP registry helpers", () => { + const deeplyEncodedChannelPath = + "/api%2525252fchannels%2525252fnostr%2525252fdefault%2525252fprofile"; + const decodeOverflowPublicPath = `/googlechat${buildRepeatedEncodedSlash(40)}public`; + it("detects registered route paths", () => { const registry = createTestRegistry({ httpRoutes: [createRoute({ path: "/demo" })], @@ -150,6 +162,8 @@ describe("plugin HTTP registry helpers", () => { }); expect(shouldEnforceGatewayAuthForPluginPath(registry, "/api//demo")).toBe(true); expect(shouldEnforceGatewayAuthForPluginPath(registry, "/api/channels/status")).toBe(true); + expect(shouldEnforceGatewayAuthForPluginPath(registry, deeplyEncodedChannelPath)).toBe(true); + expect(shouldEnforceGatewayAuthForPluginPath(registry, decodeOverflowPublicPath)).toBe(true); expect(shouldEnforceGatewayAuthForPluginPath(registry, "/not-plugin")).toBe(false); }); }); diff --git a/src/gateway/server/plugins-http.ts b/src/gateway/server/plugins-http.ts index 793fc332d6a..b1d21afa40a 100644 --- a/src/gateway/server/plugins-http.ts +++ b/src/gateway/server/plugins-http.ts @@ -2,6 +2,7 @@ import type { IncomingMessage, ServerResponse } from "node:http"; import type { createSubsystemLogger } from "../../logging/subsystem.js"; import type { PluginRegistry } from "../../plugins/registry.js"; import { canonicalizePathVariant } from "../security-path.js"; +import { hasSecurityPathCanonicalizationAnomaly } from "../security-path.js"; import { isProtectedPluginRoutePath } from "../security-path.js"; type SubsystemLogger = ReturnType; @@ -37,7 +38,9 @@ export function shouldEnforceGatewayAuthForPluginPath( pathname: string, ): boolean { return ( - isProtectedPluginRoutePath(pathname) || isRegisteredPluginHttpRoutePath(registry, pathname) + hasSecurityPathCanonicalizationAnomaly(pathname) || + isProtectedPluginRoutePath(pathname) || + isRegisteredPluginHttpRoutePath(registry, pathname) ); }