fix: harden plugin route auth path canonicalization

This commit is contained in:
Peter Steinberger
2026-02-26 17:01:41 +01:00
parent 37a138c554
commit 258d615c4d
4 changed files with 104 additions and 13 deletions

View File

@@ -26,7 +26,7 @@ Docs: https://docs.openclaw.ai
- Security/Node exec approvals: bind `system.run` approvals to canonicalized env overrides (`envHash`/`envKeys`) and fail closed on env-binding mismatches/missing bindings, while adding `GIT_EXTERNAL_DIFF` to blocked host env keys. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting.
- Security/Microsoft Teams media fetch: route Graph message/hosted-content/attachment fetches and auth-scope fallback attachment downloads through shared SSRF-guarded fetch paths, and centralize hostname-suffix allowlist policy helpers in the plugin SDK to remove channel/plugin drift. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting.
- Microsoft Teams/File uploads: acknowledge `fileConsent/invoke` immediately (`invokeResponse` before upload + file card send) so Teams no longer shows false "Something went wrong" timeout banners while upload completion continues asynchronously; includes updated async regression coverage. Landed from contributor PR #27641 by @scz2011.
- 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/Plugin channel HTTP auth: normalize protected `/api/channels` path checks against canonicalized request paths (case + percent-decoding + slash normalization), resolve encoded dot-segment traversal variants, and fail closed on malformed `%`-encoded channel prefixes so alternate-path variants cannot bypass gateway auth. This ships in the next npm release (`2026.2.26`). Thanks @zpbrent for reporting.
- 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.
- Queue/Drain/Cron reliability: harden lane draining with guaranteed `draining` flag reset on synchronous pump failures, reject new queue enqueues during gateway restart drain windows (instead of silently killing accepted tasks), add `/stop` queued-backlog cutoff metadata with stale-message skipping (while avoiding cross-session native-stop cutoff bleed), and raise isolated cron `agentTurn` outer safety timeout to avoid false 10-minute timeout races against longer agent session timeouts. (#27407, #27332, #27427)
- Gateway shared-auth scopes: preserve requested operator scopes for shared-token clients when device identity is unavailable, instead of clearing scopes during auth handling. Landed from contributor PR #27498 by @kevinWangSheng. (#27494)

View File

@@ -10,12 +10,23 @@ describe("security-path canonicalization", () => {
it("canonicalizes decoded case/slash variants", () => {
expect(canonicalizePathForSecurity("/API/channels//nostr/default/profile/")).toEqual({
path: "/api/channels/nostr/default/profile",
candidates: ["/api/channels/nostr/default/profile"],
malformedEncoding: false,
rawNormalizedPath: "/api/channels/nostr/default/profile",
});
expect(canonicalizePathForSecurity("/api/%63hannels%2Fnostr%2Fdefault%2Fprofile").path).toBe(
const encoded = canonicalizePathForSecurity("/api/%63hannels%2Fnostr%2Fdefault%2Fprofile");
expect(encoded.path).toBe("/api/channels/nostr/default/profile");
expect(encoded.candidates).toContain("/api/%63hannels%2fnostr%2fdefault%2fprofile");
expect(encoded.candidates).toContain("/api/channels/nostr/default/profile");
});
it("resolves traversal after repeated decoding", () => {
expect(canonicalizePathForSecurity("/api/foo/..%2fchannels/nostr/default/profile").path).toBe(
"/api/channels/nostr/default/profile",
);
expect(
canonicalizePathForSecurity("/api/foo/%252e%252e%252fchannels/nostr/default/profile").path,
).toBe("/api/channels/nostr/default/profile");
});
it("marks malformed encoding", () => {
@@ -29,6 +40,9 @@ describe("security-path protected-prefix matching", () => {
"/API/channels/nostr/default/profile",
"/api/channels%2Fnostr%2Fdefault%2Fprofile",
"/api/%63hannels/nostr/default/profile",
"/api/foo/..%2fchannels/nostr/default/profile",
"/api/foo/%2e%2e%2fchannels/nostr/default/profile",
"/api/foo/%252e%252e%252fchannels/nostr/default/profile",
"/api/channels%2",
"/api/channels%zz",
];
@@ -43,6 +57,7 @@ describe("security-path protected-prefix matching", () => {
it("does not protect unrelated paths", () => {
expect(isProtectedPluginRoutePath("/plugin/public")).toBe(false);
expect(isProtectedPluginRoutePath("/api/channels-public")).toBe(false);
expect(isProtectedPluginRoutePath("/api/foo/..%2fchannels-public")).toBe(false);
expect(isProtectedPluginRoutePath("/api/channel")).toBe(false);
});
});

View File

@@ -1,9 +1,12 @@
export type SecurityPathCanonicalization = {
path: string;
candidates: string[];
malformedEncoding: boolean;
rawNormalizedPath: string;
};
const MAX_PATH_DECODE_PASSES = 3;
function normalizePathSeparators(pathname: string): string {
const collapsed = pathname.replace(/\/{2,}/g, "/");
if (collapsed.length <= 1) {
@@ -16,6 +19,18 @@ function normalizeProtectedPrefix(prefix: string): string {
return normalizePathSeparators(prefix.toLowerCase()) || "/";
}
function resolveDotSegments(pathname: string): string {
try {
return new URL(pathname, "http://localhost").pathname;
} catch {
return pathname;
}
}
function normalizePathForSecurity(pathname: string): string {
return normalizePathSeparators(resolveDotSegments(pathname).toLowerCase()) || "/";
}
function prefixMatch(pathname: string, prefix: string): boolean {
return (
pathname === prefix ||
@@ -26,15 +41,39 @@ function prefixMatch(pathname: string, prefix: string): boolean {
}
export function canonicalizePathForSecurity(pathname: string): SecurityPathCanonicalization {
const candidates: string[] = [];
const seen = new Set<string>();
const pushCandidate = (value: string) => {
const normalized = normalizePathForSecurity(value);
if (seen.has(normalized)) {
return;
}
seen.add(normalized);
candidates.push(normalized);
};
pushCandidate(pathname);
let decoded = pathname;
let malformedEncoding = false;
try {
decoded = decodeURIComponent(pathname);
} catch {
malformedEncoding = true;
for (let pass = 0; pass < MAX_PATH_DECODE_PASSES; pass++) {
let nextDecoded = decoded;
try {
nextDecoded = decodeURIComponent(decoded);
} catch {
malformedEncoding = true;
break;
}
if (nextDecoded === decoded) {
break;
}
decoded = nextDecoded;
pushCandidate(decoded);
}
return {
path: normalizePathSeparators(decoded.toLowerCase()) || "/",
path: candidates[candidates.length - 1] ?? "/",
candidates,
malformedEncoding,
rawNormalizedPath: normalizePathSeparators(pathname.toLowerCase()) || "/",
};
@@ -43,7 +82,11 @@ export function canonicalizePathForSecurity(pathname: string): SecurityPathCanon
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))) {
if (
canonical.candidates.some((candidate) =>
normalizedPrefixes.some((prefix) => prefixMatch(candidate, prefix)),
)
) {
return true;
}
if (!canonical.malformedEncoding) {

View File

@@ -88,12 +88,25 @@ function createHooksConfig(): HooksConfigResolved {
function canonicalizePluginPath(pathname: string): string {
let decoded = pathname;
try {
decoded = decodeURIComponent(pathname);
} catch {
decoded = pathname;
for (let pass = 0; pass < 3; pass++) {
let nextDecoded = decoded;
try {
nextDecoded = decodeURIComponent(decoded);
} catch {
break;
}
if (nextDecoded === decoded) {
break;
}
decoded = nextDecoded;
}
const collapsed = decoded.toLowerCase().replace(/\/{2,}/g, "/");
let resolved = decoded;
try {
resolved = new URL(decoded, "http://localhost").pathname;
} catch {
resolved = decoded;
}
const collapsed = resolved.toLowerCase().replace(/\/{2,}/g, "/");
if (collapsed.length <= 1) {
return collapsed;
}
@@ -109,6 +122,15 @@ 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: "dot-traversal-encoded-slash", path: "/api/foo/..%2fchannels/nostr/default/profile" },
{
label: "dot-traversal-encoded-dotdot-slash",
path: "/api/foo/%2e%2e%2fchannels/nostr/default/profile",
},
{
label: "dot-traversal-double-encoded",
path: "/api/foo/%252e%252e%252fchannels/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" },
@@ -119,12 +141,23 @@ 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/" },
{
label: "auth-dot-traversal-encoded-slash",
path: "/api/foo/..%2fchannels/nostr/default/profile",
},
{
label: "auth-dot-traversal-double-encoded",
path: "/api/foo/%252e%252e%252fchannels/nostr/default/profile",
},
];
function buildChannelPathFuzzCorpus(): RouteVariant[] {
const variants = [
"/api/channels/nostr/default/profile",
"/API/channels/nostr/default/profile",
"/api/foo/..%2fchannels/nostr/default/profile",
"/api/foo/%2e%2e%2fchannels/nostr/default/profile",
"/api/foo/%252e%252e%252fchannels/nostr/default/profile",
"/api/channels//nostr/default/profile/",
"/api/channels%2Fnostr%2Fdefault%2Fprofile",
"/api/channels%252Fnostr%252Fdefault%252Fprofile",