refactor(gateway): hard-break plugin wildcard http handlers

2026-06-07 22:09:57 +00:00 · 2026-03-02 16:22:31 +00:00
parent b13d48987c
commit 2fd8264ab0
31 changed files with 347 additions and 174 deletions
--- a/src/gateway/server/tests/test-utils.ts
+++ b/src/gateway/server/tests/test-utils.ts
@@ -5,7 +5,6 @@ export const createTestRegistry = (overrides: Partial<PluginRegistry> = {}): Plu
  return {
    ...merged,
    gatewayHandlers: merged.gatewayHandlers ?? {},
-    httpHandlers: merged.httpHandlers ?? [],
    httpRoutes: merged.httpRoutes ?? [],
  };
 };
--- a/src/gateway/server/plugins-http.test.ts
+++ b/src/gateway/server/plugins-http.test.ts
@@ -17,11 +17,15 @@ function createPluginLog(): PluginHandlerLog {
 function createRoute(params: {
  path: string;
  pluginId?: string;
-  handler?: (req: IncomingMessage, res: ServerResponse) => void | Promise<void>;
+  auth?: "gateway" | "plugin";
+  match?: "exact" | "prefix";
+  handler?: (req: IncomingMessage, res: ServerResponse) => boolean | void | Promise<boolean | void>;
 }) {
  return {
    pluginId: params.pluginId ?? "route",
    path: params.path,
+    auth: params.auth ?? "gateway",
+    match: params.match ?? "exact",
    handler: params.handler ?? (() => {}),
    source: params.pluginId ?? "route",
  };
@@ -36,7 +40,7 @@ function buildRepeatedEncodedSlash(depth: number): string {
 }

 describe("createGatewayPluginRequestHandler", () => {
-  it("returns false when no handlers are registered", async () => {
+  it("returns false when no routes are registered", async () => {
    const log = createPluginLog();
    const handler = createGatewayPluginRequestHandler({
      registry: createTestRegistry(),
@@ -47,35 +51,13 @@ describe("createGatewayPluginRequestHandler", () => {
    expect(handled).toBe(false);
  });

-  it("continues until a handler reports it handled the request", async () => {
-    const first = vi.fn(async () => false);
-    const second = vi.fn(async () => true);
-    const handler = createGatewayPluginRequestHandler({
-      registry: createTestRegistry({
-        httpHandlers: [
-          { pluginId: "first", handler: first, source: "first" },
-          { pluginId: "second", handler: second, source: "second" },
-        ],
-      }),
-      log: createPluginLog(),
-    });
-
-    const { res } = makeMockHttpResponse();
-    const handled = await handler({} as IncomingMessage, res);
-    expect(handled).toBe(true);
-    expect(first).toHaveBeenCalledTimes(1);
-    expect(second).toHaveBeenCalledTimes(1);
-  });
-
-  it("handles registered http routes before generic handlers", async () => {
+  it("handles exact route matches", async () => {
    const routeHandler = vi.fn(async (_req, res: ServerResponse) => {
      res.statusCode = 200;
    });
-    const fallback = vi.fn(async () => true);
    const handler = createGatewayPluginRequestHandler({
      registry: createTestRegistry({
        httpRoutes: [createRoute({ path: "/demo", handler: routeHandler })],
-        httpHandlers: [{ pluginId: "fallback", handler: fallback, source: "fallback" }],
      }),
      log: createPluginLog(),
    });
@@ -84,18 +66,57 @@ describe("createGatewayPluginRequestHandler", () => {
    const handled = await handler({ url: "/demo" } as IncomingMessage, res);
    expect(handled).toBe(true);
    expect(routeHandler).toHaveBeenCalledTimes(1);
-    expect(fallback).not.toHaveBeenCalled();
  });

-  it("matches canonicalized route variants before generic handlers", async () => {
+  it("prefers exact matches before prefix matches", async () => {
+    const exactHandler = vi.fn(async (_req, res: ServerResponse) => {
+      res.statusCode = 200;
+    });
+    const prefixHandler = vi.fn(async () => true);
+    const handler = createGatewayPluginRequestHandler({
+      registry: createTestRegistry({
+        httpRoutes: [
+          createRoute({ path: "/api", match: "prefix", handler: prefixHandler }),
+          createRoute({ path: "/api/demo", match: "exact", handler: exactHandler }),
+        ],
+      }),
+      log: createPluginLog(),
+    });
+
+    const { res } = makeMockHttpResponse();
+    const handled = await handler({ url: "/api/demo" } as IncomingMessage, res);
+    expect(handled).toBe(true);
+    expect(exactHandler).toHaveBeenCalledTimes(1);
+    expect(prefixHandler).not.toHaveBeenCalled();
+  });
+
+  it("supports route fallthrough when handler returns false", async () => {
+    const first = vi.fn(async () => false);
+    const second = vi.fn(async () => true);
+    const handler = createGatewayPluginRequestHandler({
+      registry: createTestRegistry({
+        httpRoutes: [
+          createRoute({ path: "/hook", match: "exact", handler: first }),
+          createRoute({ path: "/hook", match: "prefix", handler: second }),
+        ],
+      }),
+      log: createPluginLog(),
+    });
+
+    const { res } = makeMockHttpResponse();
+    const handled = await handler({ url: "/hook" } as IncomingMessage, res);
+    expect(handled).toBe(true);
+    expect(first).toHaveBeenCalledTimes(1);
+    expect(second).toHaveBeenCalledTimes(1);
+  });
+
+  it("matches canonicalized route variants", async () => {
    const routeHandler = vi.fn(async (_req, res: ServerResponse) => {
      res.statusCode = 200;
    });
-    const fallback = vi.fn(async () => true);
    const handler = createGatewayPluginRequestHandler({
      registry: createTestRegistry({
        httpRoutes: [createRoute({ path: "/api/demo", handler: routeHandler })],
-        httpHandlers: [{ pluginId: "fallback", handler: fallback, source: "fallback" }],
      }),
      log: createPluginLog(),
    });
@@ -104,28 +125,26 @@ describe("createGatewayPluginRequestHandler", () => {
    const handled = await handler({ url: "/API//demo" } as IncomingMessage, res);
    expect(handled).toBe(true);
    expect(routeHandler).toHaveBeenCalledTimes(1);
-    expect(fallback).not.toHaveBeenCalled();
  });

-  it("logs and responds with 500 when a handler throws", async () => {
+  it("logs and responds with 500 when a route throws", async () => {
    const log = createPluginLog();
    const handler = createGatewayPluginRequestHandler({
      registry: createTestRegistry({
-        httpHandlers: [
-          {
-            pluginId: "boom",
+        httpRoutes: [
+          createRoute({
+            path: "/boom",
            handler: async () => {
              throw new Error("boom");
            },
-            source: "boom",
-          },
+          }),
        ],
      }),
      log,
    });

    const { res, setHeader, end } = makeMockHttpResponse();
-    const handled = await handler({} as IncomingMessage, res);
+    const handled = await handler({ url: "/boom" } as IncomingMessage, res);
    expect(handled).toBe(true);
    expect(log.warn).toHaveBeenCalledWith(expect.stringContaining("boom"));
    expect(res.statusCode).toBe(500);
@@ -134,7 +153,7 @@ describe("createGatewayPluginRequestHandler", () => {
  });
 });

-describe("plugin HTTP registry helpers", () => {
+describe("plugin HTTP route auth checks", () => {
  const deeplyEncodedChannelPath =
    "/api%2525252fchannels%2525252fnostr%2525252fdefault%2525252fprofile";
  const decodeOverflowPublicPath = `/googlechat${buildRepeatedEncodedSlash(40)}public`;
@@ -156,11 +175,15 @@ describe("plugin HTTP registry helpers", () => {
    expect(isRegisteredPluginHttpRoutePath(registry, "/api/%2564emo")).toBe(true);
  });

-  it("enforces auth for protected and registered plugin routes", () => {
+  it("enforces auth for protected and gateway-auth routes", () => {
    const registry = createTestRegistry({
-      httpRoutes: [createRoute({ path: "/api/demo" })],
+      httpRoutes: [
+        createRoute({ path: "/googlechat", match: "prefix", auth: "plugin" }),
+        createRoute({ path: "/api/demo", auth: "gateway" }),
+      ],
    });
    expect(shouldEnforceGatewayAuthForPluginPath(registry, "/api//demo")).toBe(true);
+    expect(shouldEnforceGatewayAuthForPluginPath(registry, "/googlechat/public")).toBe(false);
    expect(shouldEnforceGatewayAuthForPluginPath(registry, "/api/channels/status")).toBe(true);
    expect(shouldEnforceGatewayAuthForPluginPath(registry, deeplyEncodedChannelPath)).toBe(true);
    expect(shouldEnforceGatewayAuthForPluginPath(registry, decodeOverflowPublicPath)).toBe(true);
--- a/src/gateway/server/plugins-http.ts
+++ b/src/gateway/server/plugins-http.ts
@@ -1,31 +1,117 @@
 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";
+import {
+  PROTECTED_PLUGIN_ROUTE_PREFIXES,
+  canonicalizePathForSecurity,
+  canonicalizePathVariant,
+} from "../security-path.js";

 type SubsystemLogger = ReturnType<typeof createSubsystemLogger>;

+export type PluginRoutePathContext = {
+  pathname: string;
+  canonicalPath: string;
+  candidates: string[];
+  malformedEncoding: boolean;
+  decodePassLimitReached: boolean;
+  rawNormalizedPath: string;
+};
+
 export type PluginHttpRequestHandler = (
  req: IncomingMessage,
  res: ServerResponse,
+  pathContext?: PluginRoutePathContext,
 ) => Promise<boolean>;

 type PluginHttpRouteEntry = NonNullable<PluginRegistry["httpRoutes"]>[number];

+function normalizeProtectedPrefix(prefix: string): string {
+  const collapsed = prefix.toLowerCase().replace(/\/{2,}/g, "/");
+  if (collapsed.length <= 1) {
+    return collapsed || "/";
+  }
+  return collapsed.replace(/\/+$/, "");
+}
+
+function prefixMatch(pathname: string, prefix: string): boolean {
+  return (
+    pathname === prefix || pathname.startsWith(`${prefix}/`) || pathname.startsWith(`${prefix}%`)
+  );
+}
+
+const NORMALIZED_PROTECTED_PLUGIN_ROUTE_PREFIXES =
+  PROTECTED_PLUGIN_ROUTE_PREFIXES.map(normalizeProtectedPrefix);
+
+export function isProtectedPluginRoutePathFromContext(context: PluginRoutePathContext): boolean {
+  if (
+    context.candidates.some((candidate) =>
+      NORMALIZED_PROTECTED_PLUGIN_ROUTE_PREFIXES.some((prefix) => prefixMatch(candidate, prefix)),
+    )
+  ) {
+    return true;
+  }
+  if (!context.malformedEncoding) {
+    return false;
+  }
+  return NORMALIZED_PROTECTED_PLUGIN_ROUTE_PREFIXES.some((prefix) =>
+    prefixMatch(context.rawNormalizedPath, prefix),
+  );
+}
+
+export function resolvePluginRoutePathContext(pathname: string): PluginRoutePathContext {
+  const canonical = canonicalizePathForSecurity(pathname);
+  return {
+    pathname,
+    canonicalPath: canonical.canonicalPath,
+    candidates: canonical.candidates,
+    malformedEncoding: canonical.malformedEncoding,
+    decodePassLimitReached: canonical.decodePassLimitReached,
+    rawNormalizedPath: canonical.rawNormalizedPath,
+  };
+}
+
+function doesRouteMatchPath(route: PluginHttpRouteEntry, context: PluginRoutePathContext): boolean {
+  const routeCanonicalPath = canonicalizePathVariant(route.path);
+  if (route.match === "prefix") {
+    return context.candidates.some((candidate) => prefixMatch(candidate, routeCanonicalPath));
+  }
+  return context.candidates.some((candidate) => candidate === routeCanonicalPath);
+}
+
+function findMatchingPluginHttpRoutes(
+  registry: PluginRegistry,
+  context: PluginRoutePathContext,
+): PluginHttpRouteEntry[] {
+  const routes = registry.httpRoutes ?? [];
+  if (routes.length === 0) {
+    return [];
+  }
+  const exactMatches: PluginHttpRouteEntry[] = [];
+  const prefixMatches: PluginHttpRouteEntry[] = [];
+  for (const route of routes) {
+    if (!doesRouteMatchPath(route, context)) {
+      continue;
+    }
+    if (route.match === "prefix") {
+      prefixMatches.push(route);
+    } else {
+      exactMatches.push(route);
+    }
+  }
+  exactMatches.sort((a, b) => b.path.length - a.path.length);
+  prefixMatches.sort((a, b) => b.path.length - a.path.length);
+  return [...exactMatches, ...prefixMatches];
+}
+
 export function findRegisteredPluginHttpRoute(
  registry: PluginRegistry,
  pathname: string,
 ): PluginHttpRouteEntry | undefined {
-  const canonicalPath = canonicalizePathVariant(pathname);
-  const routes = registry.httpRoutes ?? [];
-  return routes.find((entry) => canonicalizePathVariant(entry.path) === canonicalPath);
+  const pathContext = resolvePluginRoutePathContext(pathname);
+  return findMatchingPluginHttpRoutes(registry, pathContext)[0];
 }

-// Only checks specific routes registered via registerHttpRoute, not wildcard handlers
-// registered via registerHttpHandler. Wildcard handlers (e.g., webhooks) implement
-// their own signature-based auth and are handled separately in the auth enforcement logic.
 export function isRegisteredPluginHttpRoutePath(
  registry: PluginRegistry,
  pathname: string,
@@ -35,13 +121,23 @@ export function isRegisteredPluginHttpRoutePath(

 export function shouldEnforceGatewayAuthForPluginPath(
  registry: PluginRegistry,
-  pathname: string,
+  pathnameOrContext: string | PluginRoutePathContext,
 ): boolean {
-  return (
-    hasSecurityPathCanonicalizationAnomaly(pathname) ||
-    isProtectedPluginRoutePath(pathname) ||
-    isRegisteredPluginHttpRoutePath(registry, pathname)
-  );
+  const pathContext =
+    typeof pathnameOrContext === "string"
+      ? resolvePluginRoutePathContext(pathnameOrContext)
+      : pathnameOrContext;
+  if (pathContext.malformedEncoding || pathContext.decodePassLimitReached) {
+    return true;
+  }
+  if (isProtectedPluginRoutePathFromContext(pathContext)) {
+    return true;
+  }
+  const route = findMatchingPluginHttpRoutes(registry, pathContext)[0];
+  if (!route) {
+    return false;
+  }
+  return route.auth === "gateway";
 }

 export function createGatewayPluginRequestHandler(params: {
@@ -49,40 +145,31 @@ export function createGatewayPluginRequestHandler(params: {
  log: SubsystemLogger;
 }): PluginHttpRequestHandler {
  const { registry, log } = params;
-  return async (req, res) => {
+  return async (req, res, providedPathContext) => {
    const routes = registry.httpRoutes ?? [];
-    const handlers = registry.httpHandlers ?? [];
-    if (routes.length === 0 && handlers.length === 0) {
+    if (routes.length === 0) {
      return false;
    }

-    if (routes.length > 0) {
-      const url = new URL(req.url ?? "/", "http://localhost");
-      const route = findRegisteredPluginHttpRoute(registry, url.pathname);
-      if (route) {
-        try {
-          await route.handler(req, res);
-          return true;
-        } catch (err) {
-          log.warn(`plugin http route failed (${route.pluginId ?? "unknown"}): ${String(err)}`);
-          if (!res.headersSent) {
-            res.statusCode = 500;
-            res.setHeader("Content-Type", "text/plain; charset=utf-8");
-            res.end("Internal Server Error");
-          }
-          return true;
-        }
-      }
+    const pathContext =
+      providedPathContext ??
+      (() => {
+        const url = new URL(req.url ?? "/", "http://localhost");
+        return resolvePluginRoutePathContext(url.pathname);
+      })();
+    const matchedRoutes = findMatchingPluginHttpRoutes(registry, pathContext);
+    if (matchedRoutes.length === 0) {
+      return false;
    }

-    for (const entry of handlers) {
+    for (const route of matchedRoutes) {
      try {
-        const handled = await entry.handler(req, res);
-        if (handled) {
+        const handled = await route.handler(req, res);
+        if (handled !== false) {
          return true;
        }
      } catch (err) {
-        log.warn(`plugin http handler failed (${entry.pluginId}): ${String(err)}`);
+        log.warn(`plugin http route failed (${route.pluginId ?? "unknown"}): ${String(err)}`);
        if (!res.headersSent) {
          res.statusCode = 500;
          res.setHeader("Content-Type", "text/plain; charset=utf-8");