refactor(security): extract protected-route path policy helpers

2026-03-21 16:41:56 +00:00 · 2026-02-26 13:01:12 +01:00
parent 39b5ffdaa6
commit 6632fd1ea9
4 changed files with 270 additions and 93 deletions
--- a/src/gateway/security-path.test.ts
+++ b/src/gateway/security-path.test.ts
@@ -0,0 +1,48 @@
+import { describe, expect, it } from "vitest";
+import {
+  PROTECTED_PLUGIN_ROUTE_PREFIXES,
+  canonicalizePathForSecurity,
+  isPathProtectedByPrefixes,
+  isProtectedPluginRoutePath,
+} from "./security-path.js";
+
+describe("security-path canonicalization", () => {
+  it("canonicalizes decoded case/slash variants", () => {
+    expect(canonicalizePathForSecurity("/API/channels//nostr/default/profile/")).toEqual({
+      path: "/api/channels/nostr/default/profile",
+      malformedEncoding: false,
+      rawNormalizedPath: "/api/channels/nostr/default/profile",
+    });
+    expect(canonicalizePathForSecurity("/api/%63hannels%2Fnostr%2Fdefault%2Fprofile").path).toBe(
+      "/api/channels/nostr/default/profile",
+    );
+  });
+
+  it("marks malformed encoding", () => {
+    expect(canonicalizePathForSecurity("/api/channels%2").malformedEncoding).toBe(true);
+    expect(canonicalizePathForSecurity("/api/channels%zz").malformedEncoding).toBe(true);
+  });
+});
+
+describe("security-path protected-prefix matching", () => {
+  const channelVariants = [
+    "/API/channels/nostr/default/profile",
+    "/api/channels%2Fnostr%2Fdefault%2Fprofile",
+    "/api/%63hannels/nostr/default/profile",
+    "/api/channels%2",
+    "/api/channels%zz",
+  ];
+
+  for (const path of channelVariants) {
+    it(`protects plugin channel path variant: ${path}`, () => {
+      expect(isProtectedPluginRoutePath(path)).toBe(true);
+      expect(isPathProtectedByPrefixes(path, PROTECTED_PLUGIN_ROUTE_PREFIXES)).toBe(true);
+    });
+  }
+
+  it("does not protect unrelated paths", () => {
+    expect(isProtectedPluginRoutePath("/plugin/public")).toBe(false);
+    expect(isProtectedPluginRoutePath("/api/channels-public")).toBe(false);
+    expect(isProtectedPluginRoutePath("/api/channel")).toBe(false);
+  });
+});
--- a/src/gateway/security-path.ts
+++ b/src/gateway/security-path.ts
@@ -0,0 +1,59 @@
+export type SecurityPathCanonicalization = {
+  path: string;
+  malformedEncoding: boolean;
+  rawNormalizedPath: string;
+};
+
+function normalizePathSeparators(pathname: string): string {
+  const collapsed = pathname.replace(/\/{2,}/g, "/");
+  if (collapsed.length <= 1) {
+    return collapsed;
+  }
+  return collapsed.replace(/\/+$/, "");
+}
+
+function normalizeProtectedPrefix(prefix: string): string {
+  return normalizePathSeparators(prefix.toLowerCase()) || "/";
+}
+
+function prefixMatch(pathname: string, prefix: string): boolean {
+  return (
+    pathname === prefix ||
+    pathname.startsWith(`${prefix}/`) ||
+    // Fail closed when malformed %-encoding follows the protected prefix.
+    pathname.startsWith(`${prefix}%`)
+  );
+}
+
+export function canonicalizePathForSecurity(pathname: string): SecurityPathCanonicalization {
+  let decoded = pathname;
+  let malformedEncoding = false;
+  try {
+    decoded = decodeURIComponent(pathname);
+  } catch {
+    malformedEncoding = true;
+  }
+  return {
+    path: normalizePathSeparators(decoded.toLowerCase()) || "/",
+    malformedEncoding,
+    rawNormalizedPath: normalizePathSeparators(pathname.toLowerCase()) || "/",
+  };
+}
+
+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))) {
+    return true;
+  }
+  if (!canonical.malformedEncoding) {
+    return false;
+  }
+  return normalizedPrefixes.some((prefix) => prefixMatch(canonical.rawNormalizedPath, prefix));
+}
+
+export const PROTECTED_PLUGIN_ROUTE_PREFIXES = ["/api/channels"] as const;
+
+export function isProtectedPluginRoutePath(pathname: string): boolean {
+  return isPathProtectedByPrefixes(pathname, PROTECTED_PLUGIN_ROUTE_PREFIXES);
+}
--- a/src/gateway/server-http.ts
+++ b/src/gateway/server-http.ts
@@ -59,6 +59,7 @@ import { getBearerToken } from "./http-utils.js";
 import { handleOpenAiHttpRequest } from "./openai-http.js";
 import { handleOpenResponsesHttpRequest } from "./openresponses-http.js";
 import { GATEWAY_CLIENT_MODES, normalizeGatewayClientMode } from "./protocol/client-info.js";
+import { isProtectedPluginRoutePath } from "./security-path.js";
 import type { GatewayWsClient } from "./server/ws-types.js";
 import { handleToolsInvokeHttpRequest } from "./tools-invoke-http.js";

@@ -88,52 +89,6 @@ function isCanvasPath(pathname: string): boolean {
  );
 }

-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 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 {
  if (client.connect.role === "node") {
    return true;
@@ -215,6 +170,34 @@ async function authorizeCanvasRequest(params: {
  return lastAuthFailure ?? { ok: false, reason: "unauthorized" };
 }

+async function enforcePluginRouteGatewayAuth(params: {
+  requestPath: string;
+  req: IncomingMessage;
+  res: ServerResponse;
+  auth: ResolvedGatewayAuth;
+  trustedProxies: string[];
+  allowRealIpFallback: boolean;
+  rateLimiter?: AuthRateLimiter;
+}): Promise<boolean> {
+  if (!isProtectedPluginRoutePath(params.requestPath)) {
+    return true;
+  }
+  const token = getBearerToken(params.req);
+  const authResult = await authorizeHttpGatewayConnect({
+    auth: params.auth,
+    connectAuth: token ? { token, password: token } : null,
+    req: params.req,
+    trustedProxies: params.trustedProxies,
+    allowRealIpFallback: params.allowRealIpFallback,
+    rateLimiter: params.rateLimiter,
+  });
+  if (!authResult.ok) {
+    sendGatewayAuthFailure(params.res, authResult);
+    return false;
+  }
+  return true;
+}
+
 function writeUpgradeAuthFailure(
  socket: { write: (chunk: string) => void },
  auth: GatewayAuthResult,
@@ -536,23 +519,20 @@ export function createGatewayHttpServer(opts: {
        return;
      }
      if (handlePluginRequest) {
-        // Channel HTTP endpoints are gateway-auth protected by default.
-        // Non-channel plugin routes remain plugin-owned and must enforce
+        // Protected plugin route prefixes are gateway-auth protected by default.
+        // Non-protected plugin routes remain plugin-owned and must enforce
        // their own auth when exposing sensitive functionality.
-        if (isProtectedPluginChannelPath(requestPath)) {
-          const token = getBearerToken(req);
-          const authResult = await authorizeHttpGatewayConnect({
-            auth: resolvedAuth,
-            connectAuth: token ? { token, password: token } : null,
-            req,
-            trustedProxies,
-            allowRealIpFallback,
-            rateLimiter,
-          });
-          if (!authResult.ok) {
-            sendGatewayAuthFailure(res, authResult);
-            return;
-          }
+        const pluginAuthOk = await enforcePluginRouteGatewayAuth({
+          requestPath,
+          req,
+          res,
+          auth: resolvedAuth,
+          trustedProxies,
+          allowRealIpFallback,
+          rateLimiter,
+        });
+        if (!pluginAuthOk) {
+          return;
        }
        if (await handlePluginRequest(req, res)) {
          return;
--- a/src/gateway/server.plugin-http-auth.test.ts
+++ b/src/gateway/server.plugin-http-auth.test.ts
@@ -100,6 +100,75 @@ function canonicalizePluginPath(pathname: string): string {
  return collapsed.replace(/\/+$/, "");
 }

+type RouteVariant = {
+  label: string;
+  path: string;
+};
+
+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: "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" },
+  { label: "malformed-double-slash-short-percent", path: "/api//channels%2" },
+];
+
+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/" },
+];
+
+function buildChannelPathFuzzCorpus(): RouteVariant[] {
+  const variants = [
+    "/api/channels/nostr/default/profile",
+    "/API/channels/nostr/default/profile",
+    "/api/channels//nostr/default/profile/",
+    "/api/channels%2Fnostr%2Fdefault%2Fprofile",
+    "/api/channels%252Fnostr%252Fdefault%252Fprofile",
+    "/api//channels/nostr/default/profile",
+    "/api/channels%2",
+    "/api/channels%zz",
+    "/api//channels%2",
+    "/api//channels%zz",
+  ];
+  return variants.map((path) => ({ label: `fuzz:${path}`, path }));
+}
+
+async function expectUnauthorizedVariants(params: {
+  server: ReturnType<typeof createGatewayHttpServer>;
+  variants: RouteVariant[];
+}) {
+  for (const variant of params.variants) {
+    const response = createResponse();
+    await dispatchRequest(params.server, createRequest({ path: variant.path }), response.res);
+    expect(response.res.statusCode, variant.label).toBe(401);
+    expect(response.getBody(), variant.label).toContain("Unauthorized");
+  }
+}
+
+async function expectAuthorizedVariants(params: {
+  server: ReturnType<typeof createGatewayHttpServer>;
+  variants: RouteVariant[];
+  authorization: string;
+}) {
+  for (const variant of params.variants) {
+    const response = createResponse();
+    await dispatchRequest(
+      params.server,
+      createRequest({
+        path: variant.path,
+        authorization: params.authorization,
+      }),
+      response.res,
+    );
+    expect(response.res.statusCode, variant.label).toBe(200);
+    expect(response.getBody(), variant.label).toContain('"route":"channel-canonicalized"');
+  }
+}
+
 describe("gateway plugin HTTP auth boundary", () => {
  test("applies default security headers and optional strict transport security", async () => {
    const resolvedAuth: ResolvedGatewayAuth = {
@@ -292,42 +361,63 @@ describe("gateway plugin HTTP auth boundary", () => {
          resolvedAuth,
        });

-        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");
-        }
+        await expectUnauthorizedVariants({ server, variants: CANONICAL_UNAUTH_VARIANTS });
        expect(handlePluginRequest).not.toHaveBeenCalled();

-        const authenticatedVariants = [
-          "/API/channels/nostr/default/profile",
-          "/api/%63hannels/nostr/default/profile",
-          "/api/channels//nostr/default/profile/",
-        ];
-        for (const path of authenticatedVariants) {
+        await expectAuthorizedVariants({
+          server,
+          variants: CANONICAL_AUTH_VARIANTS,
+          authorization: "Bearer test-token",
+        });
+        expect(handlePluginRequest).toHaveBeenCalledTimes(CANONICAL_AUTH_VARIANTS.length);
+      },
+    });
+  });
+
+  test("rejects unauthenticated plugin-channel fuzz corpus variants", async () => {
+    const resolvedAuth: ResolvedGatewayAuth = {
+      mode: "token",
+      token: "test-token",
+      password: undefined,
+      allowTailscale: false,
+    };
+
+    await withTempConfig({
+      cfg: { gateway: { trustedProxies: [] } },
+      prefix: "openclaw-plugin-http-auth-fuzz-corpus-test-",
+      run: async () => {
+        const handlePluginRequest = vi.fn(async (req: IncomingMessage, res: ServerResponse) => {
+          const pathname = new URL(req.url ?? "/", "http://localhost").pathname;
+          const canonicalPath = canonicalizePluginPath(pathname);
+          if (canonicalPath === "/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-canonicalized" }));
+            return true;
+          }
+          return false;
+        });
+
+        const server = createGatewayHttpServer({
+          canvasHost: null,
+          clients: new Set(),
+          controlUiEnabled: false,
+          controlUiBasePath: "/__control__",
+          openAiChatCompletionsEnabled: false,
+          openResponsesEnabled: false,
+          handleHooksRequest: async () => false,
+          handlePluginRequest,
+          resolvedAuth,
+        });
+
+        for (const variant of buildChannelPathFuzzCorpus()) {
          const response = createResponse();
-          await dispatchRequest(
-            server,
-            createRequest({
-              path,
-              authorization: "Bearer test-token",
-            }),
-            response.res,
+          await dispatchRequest(server, createRequest({ path: variant.path }), response.res);
+          expect(response.res.statusCode, variant.label).not.toBe(200);
+          expect(response.getBody(), variant.label).not.toContain(
+            '"route":"channel-canonicalized"',
          );
-          expect(response.res.statusCode).toBe(200);
-          expect(response.getBody()).toContain('"route":"channel-canonicalized"');
        }
-        expect(handlePluginRequest).toHaveBeenCalledTimes(authenticatedVariants.length);
      },
    });
  });