From 3128de60b145230f18439522400e9738293cea60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Emre=20Kabakc=C4=B1?= Date: Mon, 18 May 2026 02:54:03 +0100 Subject: [PATCH] fix(server): remove unauthenticated GET /internal/connections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /internal/connections was registered without auth middleware, letting any unauthenticated caller enumerate every chat connection in the system plus its agentId association — a tenant-boundary leak. The route had no internal callers in the codebase (the 'server-to-server connection listing' comment was aspirational), so the smallest safe fix is to remove the route outright rather than gate it. The authenticated /api/v1/connections endpoint already covers the legitimate listing use case. Updates the regression test in rest-api-hardening to assert 404 instead of the previous documented-200 gap. Fixes #687 --- .../__tests__/rest-api-hardening.test.ts | 23 +++++-------------- .../src/gateway/routes/public/connections.ts | 11 --------- 2 files changed, 6 insertions(+), 28 deletions(-) diff --git a/packages/server/src/gateway/__tests__/rest-api-hardening.test.ts b/packages/server/src/gateway/__tests__/rest-api-hardening.test.ts index 2aa02927d..21e9a13b7 100644 --- a/packages/server/src/gateway/__tests__/rest-api-hardening.test.ts +++ b/packages/server/src/gateway/__tests__/rest-api-hardening.test.ts @@ -714,28 +714,17 @@ describe("connection routes: access control", () => { }); /** - * SUSPECTED BUG: GET /internal/connections has no authentication. - * - * The createConnectionCrudRoutes function registers: - * app.get("/internal/connections", listAllConnections) - * with no auth middleware. Any unauthenticated caller can enumerate all - * platform connections and their agentId associations. - * - * This test documents the current (insecure) behavior. - * The endpoint should either be removed, moved behind auth, or restricted - * to same-process callers only (e.g., localhost-only binding). + * Regression: GET /internal/connections was previously registered with no + * auth middleware, enabling unauthenticated tenant enumeration. The route + * had no internal callers (the "Internal endpoint" comment was aspirational) + * so it was removed outright. This test pins the 404 to prevent re-introduction. */ - test("[KNOWN GAP] GET /internal/connections requires no auth — documents unauthenticated access", async () => { + test("GET /internal/connections is not exposed (route removed)", async () => { // No session set — completely unauthenticated const response = await orgContext.run({ organizationId: ORG_A }, () => buildConnectionApp().request("/internal/connections") ); - // Current behavior: 200 with connection data, no auth required. - // This is a security gap — internal routes should not be reachable without auth. - expect(response.status).toBe(200); - const data = (await response.json()) as any; - // Data leaks connection details to unauthenticated callers: - expect(Array.isArray(data.connections)).toBe(true); + expect(response.status).toBe(404); }); }); diff --git a/packages/server/src/gateway/routes/public/connections.ts b/packages/server/src/gateway/routes/public/connections.ts index 37b76b400..9d592efcb 100644 --- a/packages/server/src/gateway/routes/public/connections.ts +++ b/packages/server/src/gateway/routes/public/connections.ts @@ -458,17 +458,6 @@ export function createConnectionCrudRoutes( app.get("/internal/connections/platforms", listLocalTestPlatforms); app.get("/internal/connections/test-targets", listLocalTestTargets); - // Internal endpoint for server-to-server connection listing (no auth required) - const listAllConnections = async (c: any) => { - const { platform, agentId } = c.req.query(); - const connections = await manager.listConnections({ - platform: platform || undefined, - agentId: agentId || undefined, - }); - return c.json({ connections }); - }; - app.get("/internal/connections", listAllConnections); - app.openapi(ListConnectionsRoute, async (c): Promise => { const session = await verifySettingsSession(c); if (!session) {