From a2d00a33fc83c4c6cf91bb80468feb9695eca436 Mon Sep 17 00:00:00 2001 From: Anna Effort Date: Fri, 21 Aug 2026 17:55:47 -0700 Subject: [PATCH 1/2] fix: keep the /api prefix when proxying /api/logs/* MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The catch-all proxy strips its own /api before forwarding, on the assumption that mcpgateway mounts every router at the root. log_search declares prefix="/api/logs", so /api/logs/activity was forwarded to /logs/activity and 404d. It is the only router in mcpgateway mounted under /api; the other 29 sit at the root. No caller had exercised an /api/logs/* route through the BFF before — the other five log routes exist only as generated URL builders with no live callers — so this surfaced as soon as the activity feed asked for real data. Signed-off-by: Anna Effort --- server/src/routes/proxy/catch-all.ts | 18 +++++++++++++++--- server/test/proxy.test.ts | 25 +++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/server/src/routes/proxy/catch-all.ts b/server/src/routes/proxy/catch-all.ts index 0cc0312..bfa7e04 100644 --- a/server/src/routes/proxy/catch-all.ts +++ b/server/src/routes/proxy/catch-all.ts @@ -26,6 +26,19 @@ 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}`; +} + // 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 @@ -125,10 +138,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..f8e2202 100644 --- a/server/test/proxy.test.ts +++ b/server/test/proxy.test.ts @@ -111,6 +111,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); From 0fa007b2373823881cf3d0569d49cdc4581f4bd4 Mon Sep 17 00:00:00 2001 From: Anna Effort Date: Mon, 24 Aug 2026 09:57:03 -0700 Subject: [PATCH 2/2] fix: keep redirect Location rewrite inverse to the upstream path Keeping /api on logs requests left rewriteUpstreamLocation() prepending /api to a Location that already carried it, so a 307 on a logs path resolved to /api/api/logs/*. toBrowserPath() mirrors toUpstreamPath() off the same constant. Signed-off-by: Anna Effort --- server/src/routes/proxy/catch-all.ts | 9 ++++++++- server/test/proxy.test.ts | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/server/src/routes/proxy/catch-all.ts b/server/src/routes/proxy/catch-all.ts index bfa7e04..488a35e 100644 --- a/server/src/routes/proxy/catch-all.ts +++ b/server/src/routes/proxy/catch-all.ts @@ -39,6 +39,13 @@ function toUpstreamPath(wildcard: string): string { : `/${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 @@ -90,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 diff --git a/server/test/proxy.test.ts b/server/test/proxy.test.ts index f8e2202..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") { @@ -217,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);