fix(security): harden canonical auth matching for plugin channel routes

This commit is contained in:
Peter Steinberger
2026-02-26 12:55:23 +01:00
parent 0231cac957
commit 0ed675b1df
3 changed files with 90 additions and 39 deletions

View File

@@ -13,7 +13,7 @@ Docs: https://docs.openclaw.ai
### Fixes ### 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. - 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. - 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. - 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.

View File

@@ -88,17 +88,50 @@ function isCanvasPath(pathname: string): boolean {
); );
} }
function decodePathnameOnce(pathname: string): string { function normalizeSecurityPath(pathname: string): string {
try { const collapsed = pathname.replace(/\/{2,}/g, "/");
return decodeURIComponent(pathname); if (collapsed.length <= 1) {
} catch { return collapsed;
return pathname;
} }
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 { function isProtectedPluginChannelPath(pathname: string): boolean {
const normalized = decodePathnameOnce(pathname).toLowerCase(); const canonicalPath = canonicalizePathForSecurity(pathname);
return normalized === "/api/channels" || normalized.startsWith("/api/channels/"); 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 { function isNodeWsClient(client: GatewayWsClient): boolean {

View File

@@ -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", () => { describe("gateway plugin HTTP auth boundary", () => {
test("applies default security headers and optional strict transport security", async () => { test("applies default security headers and optional strict transport security", async () => {
const resolvedAuth: ResolvedGatewayAuth = { const resolvedAuth: ResolvedGatewayAuth = {
@@ -256,7 +270,7 @@ describe("gateway plugin HTTP auth boundary", () => {
run: async () => { run: async () => {
const handlePluginRequest = vi.fn(async (req: IncomingMessage, res: ServerResponse) => { const handlePluginRequest = vi.fn(async (req: IncomingMessage, res: ServerResponse) => {
const pathname = new URL(req.url ?? "/", "http://localhost").pathname; 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") { if (canonicalPath === "/api/channels/nostr/default/profile") {
res.statusCode = 200; res.statusCode = 200;
res.setHeader("Content-Type", "application/json; charset=utf-8"); res.setHeader("Content-Type", "application/json; charset=utf-8");
@@ -278,38 +292,42 @@ describe("gateway plugin HTTP auth boundary", () => {
resolvedAuth, resolvedAuth,
}); });
const unauthenticatedCaseVariant = createResponse(); const unauthenticatedVariants = [
await dispatchRequest( "/API/channels/nostr/default/profile",
server, "/api/channels%2Fnostr%2Fdefault%2Fprofile",
createRequest({ path: "/API/channels/nostr/default/profile" }), "/api/%63hannels/nostr/default/profile",
unauthenticatedCaseVariant.res, "/api/channels//nostr/default/profile",
); "/api/channels/nostr/default/profile/",
expect(unauthenticatedCaseVariant.res.statusCode).toBe(401); "/api/channels%2",
expect(unauthenticatedCaseVariant.getBody()).toContain("Unauthorized"); "/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(); expect(handlePluginRequest).not.toHaveBeenCalled();
const unauthenticatedEncodedSlash = createResponse(); const authenticatedVariants = [
await dispatchRequest( "/API/channels/nostr/default/profile",
server, "/api/%63hannels/nostr/default/profile",
createRequest({ path: "/api/channels%2Fnostr%2Fdefault%2Fprofile" }), "/api/channels//nostr/default/profile/",
unauthenticatedEncodedSlash.res, ];
); for (const path of authenticatedVariants) {
expect(unauthenticatedEncodedSlash.res.statusCode).toBe(401); const response = createResponse();
expect(unauthenticatedEncodedSlash.getBody()).toContain("Unauthorized"); await dispatchRequest(
expect(handlePluginRequest).not.toHaveBeenCalled(); server,
createRequest({
const authenticatedCaseVariant = createResponse(); path,
await dispatchRequest( authorization: "Bearer test-token",
server, }),
createRequest({ response.res,
path: "/API/channels/nostr/default/profile", );
authorization: "Bearer test-token", expect(response.res.statusCode).toBe(200);
}), expect(response.getBody()).toContain('"route":"channel-canonicalized"');
authenticatedCaseVariant.res, }
); expect(handlePluginRequest).toHaveBeenCalledTimes(authenticatedVariants.length);
expect(authenticatedCaseVariant.res.statusCode).toBe(200);
expect(authenticatedCaseVariant.getBody()).toContain('"route":"channel-canonicalized"');
expect(handlePluginRequest).toHaveBeenCalledTimes(1);
}, },
}); });
}); });