diff --git a/CHANGELOG.md b/CHANGELOG.md index a56f0a5ca..340862ab1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ Docs: https://docs.openclaw.ai ### Fixes -- Security/Plugin channel HTTP auth: normalize protected `/api/channels` path checks against canonicalized request paths (case + percent-decoding) so unauthenticated alternate-path variants cannot bypass gateway auth. +- Security/Plugin channel HTTP auth: normalize protected `/api/channels` path checks against canonicalized request paths (case + percent-decoding + slash normalization), and fail closed on malformed `%`-encoded channel prefixes so alternate-path variants cannot bypass gateway auth. - Security/Exec approvals forwarding: prefer turn-source channel/account/thread metadata when resolving approval delivery targets so stale session routes do not misroute approval prompts. - Auto-reply/Streaming: suppress only exact `NO_REPLY` final replies while still filtering streaming partial sentinel fragments (`NO_`, `NO_RE`, `HEARTBEAT_...`) so substantive replies ending with `NO_REPLY` are delivered and partial silent tokens do not leak during streaming. (#19576) Thanks @aldoeliacim. - Doctor/State integrity: ignore metadata-only slash routing sessions when checking recent missing transcripts so `openclaw doctor` no longer reports false-positive transcript-missing warnings for `*:slash:*` keys. (#27375) thanks @gumadeiras. diff --git a/src/gateway/server-http.ts b/src/gateway/server-http.ts index a89e1df08..2de935e25 100644 --- a/src/gateway/server-http.ts +++ b/src/gateway/server-http.ts @@ -88,17 +88,50 @@ function isCanvasPath(pathname: string): boolean { ); } -function decodePathnameOnce(pathname: string): string { - try { - return decodeURIComponent(pathname); - } catch { - return pathname; +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 normalized = decodePathnameOnce(pathname).toLowerCase(); - return normalized === "/api/channels" || normalized.startsWith("/api/channels/"); + 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 { diff --git a/src/gateway/server.plugin-http-auth.test.ts b/src/gateway/server.plugin-http-auth.test.ts index 79d9070f9..6a931ff50 100644 --- a/src/gateway/server.plugin-http-auth.test.ts +++ b/src/gateway/server.plugin-http-auth.test.ts @@ -86,6 +86,20 @@ function createHooksConfig(): HooksConfigResolved { }; } +function canonicalizePluginPath(pathname: string): string { + let decoded = pathname; + try { + decoded = decodeURIComponent(pathname); + } catch { + decoded = pathname; + } + const collapsed = decoded.toLowerCase().replace(/\/{2,}/g, "/"); + if (collapsed.length <= 1) { + return collapsed; + } + return collapsed.replace(/\/+$/, ""); +} + describe("gateway plugin HTTP auth boundary", () => { test("applies default security headers and optional strict transport security", async () => { const resolvedAuth: ResolvedGatewayAuth = { @@ -256,7 +270,7 @@ describe("gateway plugin HTTP auth boundary", () => { run: async () => { const handlePluginRequest = vi.fn(async (req: IncomingMessage, res: ServerResponse) => { const pathname = new URL(req.url ?? "/", "http://localhost").pathname; - const canonicalPath = decodeURIComponent(pathname).toLowerCase(); + const canonicalPath = canonicalizePluginPath(pathname); if (canonicalPath === "/api/channels/nostr/default/profile") { res.statusCode = 200; res.setHeader("Content-Type", "application/json; charset=utf-8"); @@ -278,38 +292,42 @@ describe("gateway plugin HTTP auth boundary", () => { resolvedAuth, }); - const unauthenticatedCaseVariant = createResponse(); - await dispatchRequest( - server, - createRequest({ path: "/API/channels/nostr/default/profile" }), - unauthenticatedCaseVariant.res, - ); - expect(unauthenticatedCaseVariant.res.statusCode).toBe(401); - expect(unauthenticatedCaseVariant.getBody()).toContain("Unauthorized"); + 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"); + } expect(handlePluginRequest).not.toHaveBeenCalled(); - const unauthenticatedEncodedSlash = createResponse(); - await dispatchRequest( - server, - createRequest({ path: "/api/channels%2Fnostr%2Fdefault%2Fprofile" }), - unauthenticatedEncodedSlash.res, - ); - expect(unauthenticatedEncodedSlash.res.statusCode).toBe(401); - expect(unauthenticatedEncodedSlash.getBody()).toContain("Unauthorized"); - expect(handlePluginRequest).not.toHaveBeenCalled(); - - const authenticatedCaseVariant = createResponse(); - await dispatchRequest( - server, - createRequest({ - path: "/API/channels/nostr/default/profile", - authorization: "Bearer test-token", - }), - authenticatedCaseVariant.res, - ); - expect(authenticatedCaseVariant.res.statusCode).toBe(200); - expect(authenticatedCaseVariant.getBody()).toContain('"route":"channel-canonicalized"'); - expect(handlePluginRequest).toHaveBeenCalledTimes(1); + const authenticatedVariants = [ + "/API/channels/nostr/default/profile", + "/api/%63hannels/nostr/default/profile", + "/api/channels//nostr/default/profile/", + ]; + for (const path of authenticatedVariants) { + const response = createResponse(); + await dispatchRequest( + server, + createRequest({ + path, + authorization: "Bearer test-token", + }), + response.res, + ); + expect(response.res.statusCode).toBe(200); + expect(response.getBody()).toContain('"route":"channel-canonicalized"'); + } + expect(handlePluginRequest).toHaveBeenCalledTimes(authenticatedVariants.length); }, }); });