diff --git a/server/src/routes/proxy/catch-all.ts b/server/src/routes/proxy/catch-all.ts index 0cc0312..488a35e 100644 --- a/server/src/routes/proxy/catch-all.ts +++ b/server/src/routes/proxy/catch-all.ts @@ -26,6 +26,26 @@ import { upstreamAuthHeader } from "../../lib/upstream-auth.js"; const SAFE_METHODS = new Set(["GET", "HEAD", "OPTIONS", "TRACE"]); +// mcpgateway mounts every router at the root except log_search, which declares +// `prefix="/api/logs"`. Stripping this route's own `/api` for those paths would +// forward /api/logs/* to /logs/* upstream, which 404s. Keep the prefix instead. +// Verified against mcpgateway: /api/logs is the only such router. +const UPSTREAM_API_PREFIXES = ["logs/"]; + +/** Browser `/api/` -> the path FastAPI actually serves. */ +function toUpstreamPath(wildcard: string): string { + return UPSTREAM_API_PREFIXES.some((prefix) => wildcard.startsWith(prefix)) + ? `/api/${wildcard}` + : `/${wildcard}`; +} + +/** Inverse of toUpstreamPath: upstream path -> browser `/api/*` path. */ +function toBrowserPath(upstreamPath: string): string { + return UPSTREAM_API_PREFIXES.some((prefix) => upstreamPath.startsWith(`/api/${prefix}`)) + ? upstreamPath + : `/api${upstreamPath}`; +} + // Inbound headers that must never reach upstream verbatim: bff_sid/bff_csrf // (Cookie) are BFF-only secrets; the rest are infra/auth headers mcpgateway // trusts for request-URL construction (Forwarded/X-Forwarded-*, including @@ -77,7 +97,7 @@ function rewriteUpstreamLocation( return rest; } const upstreamPath = location.slice(config.contextforgeUrl.length); - return { ...rest, location: `/api${upstreamPath}` }; + return { ...rest, location: toBrowserPath(upstreamPath) }; } // fastify.csrfProtection is callback-style (request, reply, done), not @@ -125,10 +145,9 @@ export default async function catchAllProxyRoute(fastify: FastifyInstance): Prom "/api/*", { preHandler: [fastify.sessionAuth, csrfIfUnsafe] }, async (request: FastifyRequest, reply: FastifyReply) => { - // Wildcard capture excludes the leading '/api/'; FastAPI routes are - // mounted at root, so reattach a single leading slash. + // Wildcard capture excludes the leading '/api/'; see toUpstreamPath. const wildcard = (request.params as Record)["*"] ?? ""; - const upstreamPath = `/${wildcard}`; + const upstreamPath = toUpstreamPath(wildcard); const bearerToken = request.session!.bearerToken; const sessionId = request.session!.sessionId; diff --git a/server/test/proxy.test.ts b/server/test/proxy.test.ts index ed4ed3d..43e6d63 100644 --- a/server/test/proxy.test.ts +++ b/server/test/proxy.test.ts @@ -35,6 +35,13 @@ beforeAll(async () => { res.end(); return; } + // Same redirect on an /api-mounted route, where the Location already + // carries the prefix. + if (req.url === "/api/logs/activity/") { + res.writeHead(307, { location: `${upstreamOrigin}/api/logs/activity` }); + res.end(); + return; + } // Simulates an expired/invalid bearer token — FastAPI's real // rbac middleware rejects with 401 here. if (req.url === "/expired") { @@ -111,6 +118,31 @@ describe("ALL /api/*", () => { expect(lastRequest?.authorization).toBe("Bearer test-bearer-token"); }); + it("keeps the /api prefix for /api/logs/*, which mcpgateway mounts under /api", async () => { + const app = await buildApp(); + const { cookie } = await seedSession(app); + + const response = await app.fastify.inject({ + method: "GET", + url: "/api/logs/activity?limit=100", + headers: { cookie }, + }); + + expect(response.statusCode).toBe(200); + // Stripping the prefix here would forward /logs/activity, which 404s. + expect(lastRequest?.path).toBe("/api/logs/activity?limit=100"); + expect(lastRequest?.authorization).toBe("Bearer test-bearer-token"); + }); + + it("does not treat a non-logs path beginning with the same letters as /api-mounted", async () => { + const app = await buildApp(); + const { cookie } = await seedSession(app); + + await app.fastify.inject({ method: "GET", url: "/api/logsearch", headers: { cookie } }); + + expect(lastRequest?.path).toBe("/logsearch"); + }); + it("never lets the browser override the injected Authorization header", async () => { const app = await buildApp(); const { cookie } = await seedSession(app); @@ -192,6 +224,22 @@ describe("ALL /api/*", () => { expect(response.headers.location).toBe("/api/teams/"); }); + it("does not double the prefix rewriting a redirect on an /api-mounted route", async () => { + const app = await buildApp(); + const { cookie } = await seedSession(app); + + const response = await app.fastify.inject({ + method: "GET", + url: "/api/logs/activity/", + headers: { cookie }, + }); + + expect(response.statusCode).toBe(307); + // Prepending unconditionally here would send the browser to + // /api/api/logs/activity, which 404s. + expect(response.headers.location).toBe("/api/logs/activity"); + }); + it("revokes the BFF session when upstream returns 401 (expired/invalid bearer token)", async () => { const app = await buildApp(); const { cookie } = await seedSession(app);