mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-18 08:27:26 +00:00
fix(gateway): fail closed plugin auth path canonicalization
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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",
|
||||
];
|
||||
|
||||
@@ -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<string>, 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<string>();
|
||||
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<readonly string[], readonly string[]>();
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
@@ -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<string, "live" | "ready">([
|
||||
["/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,
|
||||
|
||||
@@ -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"');
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<typeof createSubsystemLogger>;
|
||||
@@ -37,7 +38,9 @@ export function shouldEnforceGatewayAuthForPluginPath(
|
||||
pathname: string,
|
||||
): boolean {
|
||||
return (
|
||||
isProtectedPluginRoutePath(pathname) || isRegisteredPluginHttpRoutePath(registry, pathname)
|
||||
hasSecurityPathCanonicalizationAnomaly(pathname) ||
|
||||
isProtectedPluginRoutePath(pathname) ||
|
||||
isRegisteredPluginHttpRoutePath(registry, pathname)
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user