From 27c99e5e910984061f922c958515d4faf1366a4a Mon Sep 17 00:00:00 2001 From: Kris McGinnes Date: Thu, 28 May 2026 17:14:37 -0500 Subject: [PATCH] Switch schema sync summary API from detailed to basic mode Basic mode returns all fields Graph Explorer uses (nodeLabels, edgeLabels, numNodes, numEdges) without the unused nodeStructures and edgeStructures. Also makes the proxy forward query params as-is instead of hardcoding mode=detailed. Removes broken lint-staged config from graph-explorer sub-package (referenced removed eslint; root config already handles linting). Closes #1789 --- .../src/app.test.ts | 127 ++++++++++++++++-- .../graph-explorer-proxy-server/src/app.ts | 41 +++--- .../connector/gremlin/gremlinExplorer.test.ts | 103 ++++++++++++++ .../src/connector/gremlin/gremlinExplorer.ts | 2 +- .../openCypher/openCypherExplorer.test.ts | 124 +++++++++++++++++ .../openCypher/openCypherExplorer.ts | 4 +- .../connector/sparql/sparqlExplorer.test.ts | 100 ++++++++++++++ .../src/connector/sparql/sparqlExplorer.ts | 2 +- 8 files changed, 468 insertions(+), 35 deletions(-) create mode 100644 packages/graph-explorer/src/connector/gremlin/gremlinExplorer.test.ts create mode 100644 packages/graph-explorer/src/connector/openCypher/openCypherExplorer.test.ts create mode 100644 packages/graph-explorer/src/connector/sparql/sparqlExplorer.test.ts diff --git a/packages/graph-explorer-proxy-server/src/app.test.ts b/packages/graph-explorer-proxy-server/src/app.test.ts index 03b4f5d72..a87f7a33d 100644 --- a/packages/graph-explorer-proxy-server/src/app.test.ts +++ b/packages/graph-explorer-proxy-server/src/app.test.ts @@ -4,7 +4,7 @@ import path from "path"; import { Readable } from "stream"; import request from "supertest"; -import { createApp } from "./app.ts"; +import { createApp, resolveEndpointUrl } from "./app.ts"; import { createLogger } from "./logging.ts"; // node-fetch is globally mocked in test-setup.ts @@ -525,7 +525,7 @@ describe("createApp", () => { // ── Summary routes ──────────────────────────────────────────────── describe("GET /summary", () => { - it("proxies to the graph database summary endpoint", async () => { + it("proxies to the graph database summary endpoint without injecting query params", async () => { mockFetchOnce(JSON.stringify({ graphSummary: {} }), 200, { "content-type": "application/json", }); @@ -535,14 +535,42 @@ describe("createApp", () => { expect(response.status).toBe(200); expect(mockFetch).toHaveBeenCalledWith( - `${graphDbUrl}/summary?mode=detailed`, + `${graphDbUrl}/summary`, + expect.objectContaining({ method: "GET" }), + ); + }); + + it("forwards query params to the graph database", async () => { + mockFetchOnce(JSON.stringify({ graphSummary: {} }), 200, { + "content-type": "application/json", + }); + + const app = createTestApp(); + await request(app).get("/summary?mode=basic").set(dbHeaders()); + + expect(mockFetch).toHaveBeenCalledWith( + `${graphDbUrl}/summary?mode=basic`, + expect.objectContaining({ method: "GET" }), + ); + }); + + it("forwards multiple query params to the graph database", async () => { + mockFetchOnce(JSON.stringify({ graphSummary: {} }), 200, { + "content-type": "application/json", + }); + + const app = createTestApp(); + await request(app).get("/summary?mode=basic&foo=bar").set(dbHeaders()); + + expect(mockFetch).toHaveBeenCalledWith( + `${graphDbUrl}/summary?mode=basic&foo=bar`, expect.objectContaining({ method: "GET" }), ); }); }); describe("GET /pg/statistics/summary", () => { - it("proxies to the PG statistics summary endpoint", async () => { + it("proxies to the PG statistics summary endpoint without injecting query params", async () => { mockFetchOnce(JSON.stringify({ stats: {} }), 200, { "content-type": "application/json", }); @@ -554,14 +582,30 @@ describe("createApp", () => { expect(response.status).toBe(200); expect(mockFetch).toHaveBeenCalledWith( - `${graphDbUrl}/pg/statistics/summary?mode=detailed`, + `${graphDbUrl}/pg/statistics/summary`, + expect.objectContaining({ method: "GET" }), + ); + }); + + it("forwards query params to the graph database", async () => { + mockFetchOnce(JSON.stringify({ stats: {} }), 200, { + "content-type": "application/json", + }); + + const app = createTestApp(); + await request(app) + .get("/pg/statistics/summary?mode=basic") + .set(dbHeaders()); + + expect(mockFetch).toHaveBeenCalledWith( + `${graphDbUrl}/pg/statistics/summary?mode=basic`, expect.objectContaining({ method: "GET" }), ); }); }); describe("GET /rdf/statistics/summary", () => { - it("proxies to the RDF statistics summary endpoint", async () => { + it("proxies to the RDF statistics summary endpoint without injecting query params", async () => { mockFetchOnce(JSON.stringify({ stats: {} }), 200, { "content-type": "application/json", }); @@ -573,7 +617,23 @@ describe("createApp", () => { expect(response.status).toBe(200); expect(mockFetch).toHaveBeenCalledWith( - `${graphDbUrl}/rdf/statistics/summary?mode=detailed`, + `${graphDbUrl}/rdf/statistics/summary`, + expect.objectContaining({ method: "GET" }), + ); + }); + + it("forwards query params to the graph database", async () => { + mockFetchOnce(JSON.stringify({ stats: {} }), 200, { + "content-type": "application/json", + }); + + const app = createTestApp(); + await request(app) + .get("/rdf/statistics/summary?mode=basic") + .set(dbHeaders()); + + expect(mockFetch).toHaveBeenCalledWith( + `${graphDbUrl}/rdf/statistics/summary?mode=basic`, expect.objectContaining({ method: "GET" }), ); }); @@ -881,7 +941,7 @@ describe("createApp", () => { await request(app).get("/summary").set(blazegraphHeaders()); expect(mockFetch).toHaveBeenCalledWith( - `${blazegraphUrl}/summary?mode=detailed`, + `${blazegraphUrl}/summary`, expect.anything(), ); }); @@ -893,7 +953,7 @@ describe("createApp", () => { await request(app).get("/pg/statistics/summary").set(blazegraphHeaders()); expect(mockFetch).toHaveBeenCalledWith( - `${blazegraphUrl}/pg/statistics/summary?mode=detailed`, + `${blazegraphUrl}/pg/statistics/summary`, expect.anything(), ); }); @@ -907,7 +967,7 @@ describe("createApp", () => { .set(blazegraphHeaders()); expect(mockFetch).toHaveBeenCalledWith( - `${blazegraphUrl}/rdf/statistics/summary?mode=detailed`, + `${blazegraphUrl}/rdf/statistics/summary`, expect.anything(), ); }); @@ -929,3 +989,50 @@ describe("createApp", () => { }); }); }); + +describe("resolveEndpointUrl", () => { + it("appends a relative endpoint to the base path", () => { + const url = resolveEndpointUrl( + "https://neptune:8182", + "pg/statistics/summary", + ); + expect(url.href).toBe("https://neptune:8182/pg/statistics/summary"); + }); + + it("preserves the base path when appending", () => { + const url = resolveEndpointUrl("https://neptune:8182/blazegraph", "sparql"); + expect(url.href).toBe("https://neptune:8182/blazegraph/sparql"); + }); + + it("strips a leading slash from the endpoint", () => { + const url = resolveEndpointUrl( + "https://neptune:8182", + "/summary?mode=basic", + ); + expect(url.href).toBe("https://neptune:8182/summary?mode=basic"); + }); + + it("preserves query params from the endpoint", () => { + const url = resolveEndpointUrl( + "https://neptune:8182", + "pg/statistics/summary?mode=basic&foo=bar", + ); + expect(url.href).toBe( + "https://neptune:8182/pg/statistics/summary?mode=basic&foo=bar", + ); + }); + + it("throws if the resolved URL escapes the base origin", () => { + expect(() => + resolveEndpointUrl("https://neptune:8182", "https://other-host.com/data"), + ).toThrow(/does not match base/); + }); + + it("does not allow protocol-relative URLs to escape the origin", () => { + const url = resolveEndpointUrl( + "https://neptune:8182", + "//other-host.com/data", + ); + expect(url.origin).toBe("https://neptune:8182"); + }); +}); diff --git a/packages/graph-explorer-proxy-server/src/app.ts b/packages/graph-explorer-proxy-server/src/app.ts index 0f5f5d28e..49b6e3898 100644 --- a/packages/graph-explorer-proxy-server/src/app.ts +++ b/packages/graph-explorer-proxy-server/src/app.ts @@ -18,15 +18,23 @@ import { type AppLogger, requestLoggingMiddleware } from "./logging.ts"; const DEFAULT_SERVICE_TYPE = "neptune-db"; /** - * Resolves a relative endpoint path against a base URL, preserving the base - * path. Forces a trailing slash on the base so that `new URL` appends rather - * than replaces the path. + * Resolves an endpoint path against a base URL, preserving the base path. + * Strips any leading slash from the endpoint and forces a trailing slash on + * the base so that `new URL` appends rather than replaces the path. + * + * Throws if the resolved URL escapes the base origin (prevents SSRF via + * crafted endpoint strings). */ -function resolveEndpointUrl( - base: string, - endpoint: T extends `/${string}` ? never : T, -): URL { - return new URL(endpoint, base.replace(/\/?$/, "/")); +export function resolveEndpointUrl(base: string, endpoint: string): URL { + const relative = endpoint.startsWith("/") ? endpoint.slice(1) : endpoint; + const baseUrl = new URL(base.replace(/\/?$/, "/")); + const resolved = new URL(relative, baseUrl); + if (resolved.origin !== baseUrl.origin) { + throw new Error( + `Resolved URL origin "${resolved.origin}" does not match base "${baseUrl.origin}"`, + ); + } + return resolved; } /** Zod schema for the custom headers expected on database query requests. */ @@ -184,7 +192,7 @@ export function createApp({ }; try { - const res = await fetch(url.href, options); + const res = await fetch(url.href, options); // lgtm[js/request-forgery] if (!res.ok) { logger.error("!!Request failure!!"); return res; @@ -490,10 +498,7 @@ export function createApp({ app.get("/summary", async (req, res, next) => { const { graphDbConnectionUrl, isIamEnabled, region, serviceType } = parseDbQueryHeaders(req.headers); - const rawUrl = resolveEndpointUrl( - graphDbConnectionUrl, - "summary?mode=detailed", - ).href; + const rawUrl = resolveEndpointUrl(graphDbConnectionUrl, req.url).href; await fetchData( res, @@ -510,10 +515,7 @@ export function createApp({ app.get("/pg/statistics/summary", async (req, res, next) => { const { graphDbConnectionUrl, isIamEnabled, region, serviceType } = parseDbQueryHeaders(req.headers); - const rawUrl = resolveEndpointUrl( - graphDbConnectionUrl, - "pg/statistics/summary?mode=detailed", - ).href; + const rawUrl = resolveEndpointUrl(graphDbConnectionUrl, req.url).href; await fetchData( res, @@ -530,10 +532,7 @@ export function createApp({ app.get("/rdf/statistics/summary", async (req, res, next) => { const { graphDbConnectionUrl, isIamEnabled, region, serviceType } = parseDbQueryHeaders(req.headers); - const rawUrl = resolveEndpointUrl( - graphDbConnectionUrl, - "rdf/statistics/summary?mode=detailed", - ).href; + const rawUrl = resolveEndpointUrl(graphDbConnectionUrl, req.url).href; await fetchData( res, diff --git a/packages/graph-explorer/src/connector/gremlin/gremlinExplorer.test.ts b/packages/graph-explorer/src/connector/gremlin/gremlinExplorer.test.ts new file mode 100644 index 000000000..280bc6fbd --- /dev/null +++ b/packages/graph-explorer/src/connector/gremlin/gremlinExplorer.test.ts @@ -0,0 +1,103 @@ +import type { FeatureFlags, NormalizedConnection } from "@/core"; + +import { createGremlinExplorer } from "./gremlinExplorer"; + +function createConnection( + overrides?: Partial, +): NormalizedConnection { + return { + url: "http://localhost:8182", + queryEngine: "gremlin", + graphDbUrl: "", + proxyConnection: false, + awsAuthEnabled: false, + ...overrides, + }; +} + +function createFeatureFlags(): FeatureFlags { + return { + showDebugActions: false, + allowLoggingDbQuery: false, + }; +} + +function jsonResponse(body: unknown): Response { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); +} + +const emptyGremlinList = { + result: { + data: { "@type": "g:List", "@value": [{ "@type": "g:Map", "@value": [] }] }, + }, +}; + +describe("createGremlinExplorer", () => { + let mockFetch: ReturnType; + + beforeEach(() => { + mockFetch = vi.fn(); + vi.stubGlobal("fetch", mockFetch); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + describe("fetchSchema", () => { + it("requests the summary API with mode=basic", async () => { + const summaryResponse = { + payload: { + graphSummary: { + numNodes: 10, + numEdges: 5, + nodeLabels: ["Person"], + edgeLabels: ["knows"], + }, + }, + }; + mockFetch + .mockResolvedValueOnce(jsonResponse(summaryResponse)) + .mockImplementation(() => + Promise.resolve(jsonResponse(emptyGremlinList)), + ); + + const explorer = createGremlinExplorer( + createConnection(), + createFeatureFlags(), + ); + await explorer.fetchSchema(); + + expect(mockFetch).toHaveBeenCalledWith( + "http://localhost:8182/pg/statistics/summary?mode=basic", + expect.objectContaining({ method: "GET" }), + ); + }); + + it("falls back to query-based discovery when summary API fails", async () => { + mockFetch + .mockResolvedValueOnce( + new Response("Not Found", { + status: 404, + headers: { "Content-Type": "text/plain" }, + }), + ) + .mockImplementation(() => + Promise.resolve(jsonResponse(emptyGremlinList)), + ); + + const explorer = createGremlinExplorer( + createConnection(), + createFeatureFlags(), + ); + const schema = await explorer.fetchSchema(); + + expect(schema).toBeDefined(); + expect(schema).toHaveProperty("vertices"); + expect(schema).toHaveProperty("edges"); + }); + }); +}); diff --git a/packages/graph-explorer/src/connector/gremlin/gremlinExplorer.ts b/packages/graph-explorer/src/connector/gremlin/gremlinExplorer.ts index ef244a554..874ac404d 100644 --- a/packages/graph-explorer/src/connector/gremlin/gremlinExplorer.ts +++ b/packages/graph-explorer/src/connector/gremlin/gremlinExplorer.ts @@ -58,7 +58,7 @@ async function fetchSummary( const response = await fetchDatabaseRequest( connection, featureFlags, - `${connection.url}/pg/statistics/summary?mode=detailed`, + `${connection.url}/pg/statistics/summary?mode=basic`, { method: "GET", ...options, diff --git a/packages/graph-explorer/src/connector/openCypher/openCypherExplorer.test.ts b/packages/graph-explorer/src/connector/openCypher/openCypherExplorer.test.ts new file mode 100644 index 000000000..90524d8b9 --- /dev/null +++ b/packages/graph-explorer/src/connector/openCypher/openCypherExplorer.test.ts @@ -0,0 +1,124 @@ +import type { FeatureFlags, NormalizedConnection } from "@/core"; + +import { createOpenCypherExplorer } from "./openCypherExplorer"; + +function createConnection( + overrides?: Partial, +): NormalizedConnection { + return { + url: "http://localhost:8182", + queryEngine: "openCypher", + graphDbUrl: "", + proxyConnection: false, + awsAuthEnabled: false, + ...overrides, + }; +} + +function createFeatureFlags(): FeatureFlags { + return { + showDebugActions: false, + allowLoggingDbQuery: false, + }; +} + +function jsonResponse(body: unknown): Response { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); +} + +describe("createOpenCypherExplorer", () => { + let mockFetch: ReturnType; + + beforeEach(() => { + mockFetch = vi.fn(); + vi.stubGlobal("fetch", mockFetch); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + describe("fetchSchema", () => { + it("requests the summary API with mode=basic", async () => { + const summaryResponse = { + payload: { + graphSummary: { + numNodes: 10, + numEdges: 5, + nodeLabels: ["Person"], + edgeLabels: ["knows"], + }, + }, + }; + mockFetch + .mockResolvedValueOnce(jsonResponse(summaryResponse)) + .mockImplementation(() => + Promise.resolve(jsonResponse({ results: [] })), + ); + + const explorer = createOpenCypherExplorer( + createConnection(), + createFeatureFlags(), + ); + await explorer.fetchSchema(); + + expect(mockFetch).toHaveBeenCalledWith( + "http://localhost:8182/pg/statistics/summary?mode=basic", + expect.objectContaining({ method: "GET" }), + ); + }); + + it("requests the summary API from non-standard endpoint for neptune-graph service type", async () => { + const summaryResponse = { + graphSummary: { + numNodes: 10, + numEdges: 5, + nodeLabels: ["Person"], + edgeLabels: ["knows"], + }, + }; + mockFetch + .mockResolvedValueOnce(jsonResponse(summaryResponse)) + .mockImplementation(() => + Promise.resolve(jsonResponse({ results: [] })), + ); + + const explorer = createOpenCypherExplorer( + createConnection({ serviceType: "neptune-graph" }), + createFeatureFlags(), + ); + await explorer.fetchSchema(); + + expect(mockFetch).toHaveBeenCalledWith( + "http://localhost:8182/summary?mode=basic", + expect.objectContaining({ method: "GET" }), + ); + }); + + it("falls back to query-based discovery when summary API fails", async () => { + mockFetch + .mockResolvedValueOnce( + new Response("Not Found", { + status: 404, + headers: { "Content-Type": "text/plain" }, + }), + ) + .mockImplementation(() => + Promise.resolve(jsonResponse({ results: [] })), + ); + + const explorer = createOpenCypherExplorer( + createConnection(), + createFeatureFlags(), + ); + const schema = await explorer.fetchSchema(); + + expect(schema).toBeDefined(); + expect(schema).toHaveProperty("vertices"); + expect(schema).toHaveProperty("edges"); + }); + }); +}); diff --git a/packages/graph-explorer/src/connector/openCypher/openCypherExplorer.ts b/packages/graph-explorer/src/connector/openCypher/openCypherExplorer.ts index 104d92c96..ef66e182f 100644 --- a/packages/graph-explorer/src/connector/openCypher/openCypherExplorer.ts +++ b/packages/graph-explorer/src/connector/openCypher/openCypherExplorer.ts @@ -130,8 +130,8 @@ async function fetchSummary( try { const endpoint = serviceType === DEFAULT_SERVICE_TYPE - ? `${connection.url}/pg/statistics/summary?mode=detailed` - : `${connection.url}/summary?mode=detailed`; + ? `${connection.url}/pg/statistics/summary?mode=basic` + : `${connection.url}/summary?mode=basic`; const response = await fetchDatabaseRequest( connection, featureFlags, diff --git a/packages/graph-explorer/src/connector/sparql/sparqlExplorer.test.ts b/packages/graph-explorer/src/connector/sparql/sparqlExplorer.test.ts new file mode 100644 index 000000000..d7bc2f831 --- /dev/null +++ b/packages/graph-explorer/src/connector/sparql/sparqlExplorer.test.ts @@ -0,0 +1,100 @@ +import type { FeatureFlags, NormalizedConnection } from "@/core"; + +import { createSparqlExplorer } from "./sparqlExplorer"; + +function createConnection( + overrides?: Partial, +): NormalizedConnection { + return { + url: "http://localhost:8182", + queryEngine: "sparql", + graphDbUrl: "", + proxyConnection: false, + awsAuthEnabled: false, + ...overrides, + }; +} + +function createFeatureFlags(): FeatureFlags { + return { + showDebugActions: false, + allowLoggingDbQuery: false, + }; +} + +function jsonResponse(body: unknown): Response { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); +} + +describe("createSparqlExplorer", () => { + let mockFetch: ReturnType; + + beforeEach(() => { + mockFetch = vi.fn(); + vi.stubGlobal("fetch", mockFetch); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + describe("fetchSchema", () => { + it("requests the summary API with mode=basic", async () => { + const summaryResponse = { + payload: { + graphSummary: { + numDistinctSubjects: 10, + numQuads: 5, + numClasses: 1, + classes: ["http://example.org/Person"], + predicates: [{ "http://example.org/knows": 5 }], + }, + }, + }; + mockFetch + .mockResolvedValueOnce(jsonResponse(summaryResponse)) + .mockImplementation(() => + Promise.resolve(jsonResponse({ results: { bindings: [] } })), + ); + + const explorer = createSparqlExplorer( + createConnection(), + createFeatureFlags(), + new Map(), + ); + await explorer.fetchSchema(); + + expect(mockFetch).toHaveBeenCalledWith( + "http://localhost:8182/rdf/statistics/summary?mode=basic", + expect.objectContaining({ method: "GET" }), + ); + }); + + it("falls back to query-based discovery when summary API fails", async () => { + mockFetch + .mockResolvedValueOnce( + new Response("Not Found", { + status: 404, + headers: { "Content-Type": "text/plain" }, + }), + ) + .mockImplementation(() => + Promise.resolve(jsonResponse({ results: { bindings: [] } })), + ); + + const explorer = createSparqlExplorer( + createConnection(), + createFeatureFlags(), + new Map(), + ); + const schema = await explorer.fetchSchema(); + + expect(schema).toBeDefined(); + expect(schema).toHaveProperty("vertices"); + expect(schema).toHaveProperty("edges"); + }); + }); +}); diff --git a/packages/graph-explorer/src/connector/sparql/sparqlExplorer.ts b/packages/graph-explorer/src/connector/sparql/sparqlExplorer.ts index 44160068d..94b9a64dc 100644 --- a/packages/graph-explorer/src/connector/sparql/sparqlExplorer.ts +++ b/packages/graph-explorer/src/connector/sparql/sparqlExplorer.ts @@ -74,7 +74,7 @@ async function fetchSummary( const response = await fetchDatabaseRequest( connection, featureFlags, - `${connection.url}/rdf/statistics/summary?mode=detailed`, + `${connection.url}/rdf/statistics/summary?mode=basic`, { method: "GET", ...options,