From 94bdea26e64e9ff09e96a30f862176b65261e801 Mon Sep 17 00:00:00 2001
From: Cursor Agent <cursoragent@cursor.com>
Date: Tue, 1 Sep 2026 08:34:54 +0000
Subject: [PATCH 01/13] fix: preserve Connection host path when proxying PromQL

new URL(absolutePath, base) replaces the base pathname, so a
VictoriaMetrics cluster host like
http://vmselect:8481/select/0/prometheus dropped /select/0/prometheus
and hit /api/v1/query_range instead.

Join the existing pathname with the Prometheus API path. Covers
query_range, query, query_exemplars, and label values.

Fixes hyperdxio/hyperdx#3046
---
 .changeset/promql-proxy-preserve-host-path.md | 11 ++++
 .../routers/api/__tests__/prometheus.test.ts  | 58 +++++++++++++++++++
 packages/api/src/routers/api/prometheus.ts    | 25 +++++++-
 3 files changed, 93 insertions(+), 1 deletion(-)
 create mode 100644 .changeset/promql-proxy-preserve-host-path.md

diff --git a/.changeset/promql-proxy-preserve-host-path.md b/.changeset/promql-proxy-preserve-host-path.md
new file mode 100644
index 0000000000..164308a9db
--- /dev/null
+++ b/.changeset/promql-proxy-preserve-host-path.md
@@ -0,0 +1,11 @@
+---
+'@hyperdx/api': patch
+---
+
+fix: preserve a Connection host path prefix when proxying PromQL.
+`proxyToPrometheus` joined absolute Prometheus paths (`/api/v1/query_range`,
+`/api/v1/query`, `/api/v1/query_exemplars`, `/api/v1/label/.../values`) with
+`new URL(path, host)`, which replaces the host pathname instead of appending to
+it. VictoriaMetrics cluster `vmselect` URLs such as
+`http://vmselect:8481/select/0/prometheus` were rewritten to
+`/api/v1/query_range` and rejected. The join now keeps the existing pathname.
diff --git a/packages/api/src/routers/api/__tests__/prometheus.test.ts b/packages/api/src/routers/api/__tests__/prometheus.test.ts
index 4fa1848083..f3915c9042 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.test.ts
@@ -12,6 +12,7 @@ import {
   formatMatrixResponse,
   formatVectorResponse,
   isClientDisconnect,
+  joinPrometheusUpstreamUrl,
   parseDuration,
   parseTimestamp,
   recordProxyOutcome,
@@ -257,3 +258,60 @@ describe('isClientDisconnect', () => {
     expect(isClientDisconnect(undefined)).toBe(false);
   });
 });
+
+describe('joinPrometheusUpstreamUrl', () => {
+  it('keeps a root-mounted Prometheus host working', () => {
+    expect(
+      joinPrometheusUpstreamUrl(
+        'http://prometheus:9090',
+        '/api/v1/query_range',
+      ).toString(),
+    ).toBe('http://prometheus:9090/api/v1/query_range');
+  });
+
+  it('keeps a trailing slash on a root-mounted host from doubling the path', () => {
+    expect(
+      joinPrometheusUpstreamUrl(
+        'http://prometheus:9090/',
+        '/api/v1/query_range',
+      ).toString(),
+    ).toBe('http://prometheus:9090/api/v1/query_range');
+  });
+
+  it('preserves a VictoriaMetrics cluster tenant prefix', () => {
+    expect(
+      joinPrometheusUpstreamUrl(
+        'http://vmselect:8481/select/0/prometheus',
+        '/api/v1/query_range',
+      ).toString(),
+    ).toBe('http://vmselect:8481/select/0/prometheus/api/v1/query_range');
+  });
+
+  it('strips a trailing slash on the tenant prefix before joining', () => {
+    expect(
+      joinPrometheusUpstreamUrl(
+        'http://vmselect:8481/select/0/prometheus/',
+        '/api/v1/label/__name__/values',
+      ).toString(),
+    ).toBe(
+      'http://vmselect:8481/select/0/prometheus/api/v1/label/__name__/values',
+    );
+  });
+
+  it('preserves userinfo and existing query params on the connection host', () => {
+    expect(
+      joinPrometheusUpstreamUrl(
+        'http://user:pw@vmselect:8481/select/0/prometheus?extra=1',
+        '/api/v1/query',
+      ).toString(),
+    ).toBe(
+      'http://user:pw@vmselect:8481/select/0/prometheus/api/v1/query?extra=1',
+    );
+  });
+
+  it('throws on an invalid connection host, matching the proxy 400 path', () => {
+    expect(() =>
+      joinPrometheusUpstreamUrl('not-a-url', '/api/v1/query_range'),
+    ).toThrow();
+  });
+});
diff --git a/packages/api/src/routers/api/prometheus.ts b/packages/api/src/routers/api/prometheus.ts
index f1bc3b3069..b8eebab0d5 100644
--- a/packages/api/src/routers/api/prometheus.ts
+++ b/packages/api/src/routers/api/prometheus.ts
@@ -193,6 +193,29 @@ export function isClientDisconnect(err: unknown): boolean {
 // Prometheus's native response shape (`{status, data}` / `{status, errorType,
 // error}`) is already what HyperDX clients expect, so we forward the status code
 // as-is — but never the content-type, which is always relabelled (see below).
+
+/**
+ * Join a Connection host with an absolute Prometheus API path.
+ *
+ * `new URL('/api/v1/query_range', 'http://host:8481/select/0/prometheus')`
+ * discards `/select/0/prometheus` because an absolute path replaces the base
+ * pathname. VictoriaMetrics cluster (and any Prometheus-compatible server
+ * mounted under a prefix) needs that prefix kept. Host userinfo, query, and
+ * hash are left untouched.
+ *
+ * @see https://github.com/hyperdxio/hyperdx/issues/3046
+ */
+export function joinPrometheusUpstreamUrl(
+  upstreamHost: string,
+  path: string,
+): URL {
+  const url = new URL(upstreamHost);
+  const basePath = url.pathname.replace(/\/$/, '');
+  const suffix = path.startsWith('/') ? path : `/${path}`;
+  url.pathname = `${basePath}${suffix}`;
+  return url;
+}
+
 async function proxyToPrometheus(
   upstreamHost: string,
   path: string,
@@ -201,7 +224,7 @@ async function proxyToPrometheus(
 ): Promise<number> {
   let url: URL;
   try {
-    url = new URL(path, upstreamHost);
+    url = joinPrometheusUpstreamUrl(upstreamHost, path);
   } catch {
     res.status(400).json({
       status: 'error',

From 70b886dbb5c07b1c5d89d7f48c67fcf102c479e8 Mon Sep 17 00:00:00 2001
From: Cursor Agent <cursoragent@cursor.com>
Date: Tue, 1 Sep 2026 08:49:53 +0000
Subject: [PATCH 02/13] fix: address PromQL proxy review comments

Move joinPrometheusUpstreamUrl above the proxyToPrometheus doc block,
keep operator-pinned host query params from being overwritten, and pin
the fetch call site with a VictoriaMetrics cluster integration test.
Also document that Connection hosts with a stray UI path (e.g. /graph)
will 404 after this change.
---
 .changeset/promql-proxy-preserve-host-path.md |  8 +++
 .../api/__tests__/prometheus.int.test.ts      | 64 +++++++++++++++++++
 .../routers/api/__tests__/prometheus.test.ts  |  9 +++
 packages/api/src/routers/api/prometheus.ts    | 27 ++++----
 4 files changed, 96 insertions(+), 12 deletions(-)

diff --git a/.changeset/promql-proxy-preserve-host-path.md b/.changeset/promql-proxy-preserve-host-path.md
index 164308a9db..a077afed60 100644
--- a/.changeset/promql-proxy-preserve-host-path.md
+++ b/.changeset/promql-proxy-preserve-host-path.md
@@ -9,3 +9,11 @@ fix: preserve a Connection host path prefix when proxying PromQL.
 it. VictoriaMetrics cluster `vmselect` URLs such as
 `http://vmselect:8481/select/0/prometheus` were rewritten to
 `/api/v1/query_range` and rejected. The join now keeps the existing pathname.
+
+This is a behavior change for Connections whose host already included a path
+that was never meant as a Prometheus API prefix — for example
+`http://prom:9090/graph` copied from the Prometheus UI. That previously happened
+to work because the absolute API path replaced `/graph`; requests now go to
+`/graph/api/v1/query_range` and will 404. Trim stray paths from existing
+Connection hosts before upgrading. Root-mounted hosts (`http://prom:9090` or
+`http://prom:9090/`) are unchanged.
diff --git a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
index f25cc6cab8..bb47454c03 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
@@ -303,6 +303,70 @@ describe('prometheus router', () => {
       expect(calledUrl).not.toContain('connectionId');
     });
 
+    // Pins the call site, not just the helper: reverting
+    // `url = new URL(path, upstreamHost)` would drop `/select/0/prometheus`.
+    it('keeps a VictoriaMetrics cluster path prefix on the upstream URL', async () => {
+      const { agent, team } = await getLoggedInAgent(server);
+      const conn = await seedPrometheusConnection(
+        team._id,
+        'http://vmselect:8481/select/0/prometheus',
+      );
+
+      mockFetch.mockResolvedValueOnce(
+        fakeUpstreamResponse({
+          status: 'success',
+          data: { resultType: 'matrix', result: [] },
+        }),
+      );
+
+      await agent
+        .get('/v1/prometheus/query_range')
+        .query({
+          query: 'up',
+          start: '1700000000',
+          end: '1700000060',
+          step: '15s',
+          connectionId: conn._id.toString(),
+        })
+        .expect(200);
+
+      expect(mockFetch).toHaveBeenCalledTimes(1);
+      expect(mockFetch.mock.calls[0][0] as string).toMatch(
+        /^http:\/\/vmselect:8481\/select\/0\/prometheus\/api\/v1\/query_range/,
+      );
+    });
+
+    it('does not let request query params override params pinned on the connection host', async () => {
+      const { agent, team } = await getLoggedInAgent(server);
+      const conn = await seedPrometheusConnection(
+        team._id,
+        'http://vmselect:8481/select/0/prometheus?extra_label=namespace%3Dprod',
+      );
+
+      mockFetch.mockResolvedValueOnce(
+        fakeUpstreamResponse({
+          status: 'success',
+          data: { resultType: 'matrix', result: [] },
+        }),
+      );
+
+      await agent
+        .get('/v1/prometheus/query_range')
+        .query({
+          query: 'up',
+          start: '1700000000',
+          end: '1700000060',
+          step: '15s',
+          extra_label: 'namespace=other',
+          connectionId: conn._id.toString(),
+        })
+        .expect(200);
+
+      const requested = new URL(mockFetch.mock.calls[0][0] as string);
+      expect(requested.searchParams.get('extra_label')).toBe('namespace=prod');
+      expect(requested.searchParams.get('query')).toBe('up');
+    });
+
     it('does NOT proxy to Prometheus when connection is not isPrometheusEndpoint', async () => {
       const { agent, team } = await getLoggedInAgent(server);
       const conn = await seedClickHouseConnection(team._id);
diff --git a/packages/api/src/routers/api/__tests__/prometheus.test.ts b/packages/api/src/routers/api/__tests__/prometheus.test.ts
index f3915c9042..79aeeefd04 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.test.ts
@@ -309,6 +309,15 @@ describe('joinPrometheusUpstreamUrl', () => {
     );
   });
 
+  it('prepends a slash when the path has none', () => {
+    expect(
+      joinPrometheusUpstreamUrl(
+        'http://prometheus:9090',
+        'api/v1/query',
+      ).toString(),
+    ).toBe('http://prometheus:9090/api/v1/query');
+  });
+
   it('throws on an invalid connection host, matching the proxy 400 path', () => {
     expect(() =>
       joinPrometheusUpstreamUrl('not-a-url', '/api/v1/query_range'),
diff --git a/packages/api/src/routers/api/prometheus.ts b/packages/api/src/routers/api/prometheus.ts
index b8eebab0d5..754185fdef 100644
--- a/packages/api/src/routers/api/prometheus.ts
+++ b/packages/api/src/routers/api/prometheus.ts
@@ -183,17 +183,6 @@ export function isClientDisconnect(err: unknown): boolean {
   );
 }
 
-// Forwards the response straight from the upstream Prometheus to the
-// HyperDX client. Returns the HTTP status it wrote, so callers can record an
-// error metric: this helper handles its own failures by writing 400/502/504 and
-// returning normally, so a caller's `catch` never sees an upstream outage and
-// would otherwise report zero errors while still recording duration. The response can be multi-megabyte (e.g. `/label/__name__/
-// values` on a large Prometheus), so we avoid `await resp.json()` +
-// `res.json(...)` which would parse + re-serialize the whole body in memory.
-// Prometheus's native response shape (`{status, data}` / `{status, errorType,
-// error}`) is already what HyperDX clients expect, so we forward the status code
-// as-is — but never the content-type, which is always relabelled (see below).
-
 /**
  * Join a Connection host with an absolute Prometheus API path.
  *
@@ -216,6 +205,16 @@ export function joinPrometheusUpstreamUrl(
   return url;
 }
 
+// Forwards the response straight from the upstream Prometheus to the
+// HyperDX client. Returns the HTTP status it wrote, so callers can record an
+// error metric: this helper handles its own failures by writing 400/502/504 and
+// returning normally, so a caller's `catch` never sees an upstream outage and
+// would otherwise report zero errors while still recording duration. The response can be multi-megabyte (e.g. `/label/__name__/
+// values` on a large Prometheus), so we avoid `await resp.json()` +
+// `res.json(...)` which would parse + re-serialize the whole body in memory.
+// Prometheus's native response shape (`{status, data}` / `{status, errorType,
+// error}`) is already what HyperDX clients expect, so we forward the status code
+// as-is — but never the content-type, which is always relabelled (see below).
 async function proxyToPrometheus(
   upstreamHost: string,
   path: string,
@@ -233,9 +232,13 @@ async function proxyToPrometheus(
     });
     return 400;
   }
+  // Params already on the Connection host are operator-pinned (e.g.
+  // `?extra_label=namespace%3Dprod` on a VictoriaMetrics tenant URL). Do not
+  // let the incoming request overwrite them.
+  const hostPinnedKeys = new Set(url.searchParams.keys());
   for (const [k, v] of Object.entries(params)) {
     if (['connectionId', 'database', 'table'].includes(k)) continue;
-    if (v == null) continue;
+    if (v == null || hostPinnedKeys.has(k)) continue;
     // A repeatable param (`match[]`) appends every value
     if (Array.isArray(v)) {
       for (const item of v) url.searchParams.append(k, item);

From 5c2e40e467a330695481b745789f74cbf392092f Mon Sep 17 00:00:00 2001
From: Cursor Agent <cursoragent@cursor.com>
Date: Wed, 2 Sep 2026 06:51:58 +0000
Subject: [PATCH 03/13] fix: address remaining PromQL proxy review comments

Reject non-http(s) Connection hosts in joinPrometheusUpstreamUrl so a
scheme-less host 400s instead of 502ing. Keep start/end/step under the
proxy so exemplar clamping and chart resolution still win over host-pinned
params, and record both behavior changes in a minor changeset.
---
 .changeset/promql-proxy-preserve-host-path.md | 10 ++-
 .../api/__tests__/prometheus.int.test.ts      | 81 +++++++++++++++++++
 .../routers/api/__tests__/prometheus.test.ts  | 12 +++
 packages/api/src/routers/api/prometheus.ts    | 20 ++++-
 4 files changed, 119 insertions(+), 4 deletions(-)

diff --git a/.changeset/promql-proxy-preserve-host-path.md b/.changeset/promql-proxy-preserve-host-path.md
index a077afed60..ba134be904 100644
--- a/.changeset/promql-proxy-preserve-host-path.md
+++ b/.changeset/promql-proxy-preserve-host-path.md
@@ -1,5 +1,5 @@
 ---
-'@hyperdx/api': patch
+'@hyperdx/api': minor
 ---
 
 fix: preserve a Connection host path prefix when proxying PromQL.
@@ -17,3 +17,11 @@ to work because the absolute API path replaced `/graph`; requests now go to
 `/graph/api/v1/query_range` and will 404. Trim stray paths from existing
 Connection hosts before upgrading. Root-mounted hosts (`http://prom:9090` or
 `http://prom:9090/`) are unchanged.
+
+Query parameters already on the Connection host are treated as operator-pinned
+(for example `?extra_label=namespace%3Dprod` on a VictoriaMetrics tenant URL)
+and are no longer overwritten by the same-named request parameter. Incoming
+values for those keys are dropped, including repeatable ones such as `match[]`.
+Exception: `start`, `end`, and `step` stay under the proxy's control so the
+`/query_exemplars` window clamp and chart resolution cannot be defeated by a
+host that already carries those keys.
diff --git a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
index bb47454c03..544542e245 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
@@ -273,6 +273,29 @@ describe('prometheus router', () => {
       expect(res.headers['x-content-type-options']).toBe('nosniff');
     });
 
+    it('returns 400 for a scheme-less connection host instead of 502', async () => {
+      const { agent, team } = await getLoggedInAgent(server);
+      const conn = await seedPrometheusConnection(team._id, 'prometheus:9090');
+
+      const res = await agent
+        .get('/v1/prometheus/query_range')
+        .query({
+          query: 'up',
+          start: '1700000000',
+          end: '1700000060',
+          step: '15s',
+          connectionId: conn._id.toString(),
+        })
+        .expect(400);
+
+      expect(res.body).toMatchObject({
+        status: 'error',
+        errorType: 'bad_data',
+        error: expect.stringContaining('prometheus:9090'),
+      });
+      expect(mockFetch).not.toHaveBeenCalled();
+    });
+
     it('proxies to upstream Prometheus when connection isPrometheusEndpoint', async () => {
       const { agent, team } = await getLoggedInAgent(server);
       const conn = await seedPrometheusConnection(team._id);
@@ -367,6 +390,35 @@ describe('prometheus router', () => {
       expect(requested.searchParams.get('query')).toBe('up');
     });
 
+    it('lets the request step override a step pinned on the connection host', async () => {
+      const { agent, team } = await getLoggedInAgent(server);
+      const conn = await seedPrometheusConnection(
+        team._id,
+        'http://prom.example.com?step=5m',
+      );
+
+      mockFetch.mockResolvedValueOnce(
+        fakeUpstreamResponse({
+          status: 'success',
+          data: { resultType: 'matrix', result: [] },
+        }),
+      );
+
+      await agent
+        .get('/v1/prometheus/query_range')
+        .query({
+          query: 'up',
+          start: '1700000000',
+          end: '1700000060',
+          step: '15s',
+          connectionId: conn._id.toString(),
+        })
+        .expect(200);
+
+      const requested = new URL(mockFetch.mock.calls[0][0] as string);
+      expect(requested.searchParams.get('step')).toBe('15s');
+    });
+
     it('does NOT proxy to Prometheus when connection is not isPrometheusEndpoint', async () => {
       const { agent, team } = await getLoggedInAgent(server);
       const conn = await seedClickHouseConnection(team._id);
@@ -964,6 +1016,35 @@ describe('prometheus router', () => {
       expect(end - sentStart).toBe(PROMETHEUS_MAX_EXEMPLAR_WINDOW_SEC);
     });
 
+    it('lets the clamped exemplar window override start/end pinned on the host', async () => {
+      const { agent, team } = await getLoggedInAgent(server);
+      const end = 1700000000;
+      const thirtyDays = 30 * 24 * 60 * 60;
+      const conn = await seedPrometheusConnection(
+        team._id,
+        `http://prom.example.com?start=${end - thirtyDays}&end=${end}`,
+      );
+
+      mockFetch.mockResolvedValueOnce(
+        fakeUpstreamResponse({ status: 'success', data: [] }),
+      );
+
+      await agent
+        .get('/v1/prometheus/query_exemplars')
+        .query({
+          query: 'up',
+          start: String(end - thirtyDays),
+          end: String(end),
+          connectionId: conn._id.toString(),
+        })
+        .expect(200);
+
+      const requested = new URL(mockFetch.mock.calls[0][0] as string);
+      const sentStart = Number(requested.searchParams.get('start'));
+      expect(Number(requested.searchParams.get('end'))).toBe(end);
+      expect(end - sentStart).toBe(PROMETHEUS_MAX_EXEMPLAR_WINDOW_SEC);
+    });
+
     it('returns an empty result for ClickHouse-backed connections (no native exemplar table function)', async () => {
       const { agent, team } = await getLoggedInAgent(server);
       const conn = await seedClickHouseConnection(team._id);
diff --git a/packages/api/src/routers/api/__tests__/prometheus.test.ts b/packages/api/src/routers/api/__tests__/prometheus.test.ts
index 79aeeefd04..ad08492b50 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.test.ts
@@ -323,4 +323,16 @@ describe('joinPrometheusUpstreamUrl', () => {
       joinPrometheusUpstreamUrl('not-a-url', '/api/v1/query_range'),
     ).toThrow();
   });
+
+  // `new URL('prometheus:9090')` succeeds (scheme `prometheus:`, opaque path).
+  // Without the http(s) guard the helper would return the host unchanged and
+  // the proxy would 502 instead of 400.
+  it('throws on a scheme-less host that URL parses as an opaque path', () => {
+    expect(() =>
+      joinPrometheusUpstreamUrl('prometheus:9090', '/api/v1/query_range'),
+    ).toThrow(/http\(s\)/);
+    expect(() =>
+      joinPrometheusUpstreamUrl('localhost:9090', '/api/v1/query_range'),
+    ).toThrow(/http\(s\)/);
+  });
 });
diff --git a/packages/api/src/routers/api/prometheus.ts b/packages/api/src/routers/api/prometheus.ts
index 754185fdef..efd4042b70 100644
--- a/packages/api/src/routers/api/prometheus.ts
+++ b/packages/api/src/routers/api/prometheus.ts
@@ -199,6 +199,14 @@ export function joinPrometheusUpstreamUrl(
   path: string,
 ): URL {
   const url = new URL(upstreamHost);
+  // `new URL('prometheus:9090')` succeeds with an opaque path (`prometheus:`
+  // scheme). The pathname setter is a no-op there, so without this guard the
+  // helper would return the host unchanged, `fetch` would fail, and the proxy
+  // would 502 / increment query_errors for a user misconfiguration. Same check
+  // as clickhouseProxy.ts.
+  if (url.protocol !== 'http:' && url.protocol !== 'https:') {
+    throw new TypeError('Connection host must be http(s)');
+  }
   const basePath = url.pathname.replace(/\/$/, '');
   const suffix = path.startsWith('/') ? path : `/${path}`;
   url.pathname = `${basePath}${suffix}`;
@@ -233,9 +241,15 @@ async function proxyToPrometheus(
     return 400;
   }
   // Params already on the Connection host are operator-pinned (e.g.
-  // `?extra_label=namespace%3Dprod` on a VictoriaMetrics tenant URL). Do not
-  // let the incoming request overwrite them.
-  const hostPinnedKeys = new Set(url.searchParams.keys());
+  // `?extra_label=namespace%3Dprod` on a VictoriaMetrics tenant URL). Skip the
+  // incoming value for those keys rather than appending — including repeatable
+  // ones such as `match[]`. Exception: `start`/`end`/`step` are owned by this
+  // proxy (exemplar window clamp, chart resolution) and still overwrite a host
+  // value so a Connection cannot defeat those bounds.
+  const PROXY_OWNED_PARAM_KEYS = new Set(['start', 'end', 'step']);
+  const hostPinnedKeys = new Set(
+    [...url.searchParams.keys()].filter(k => !PROXY_OWNED_PARAM_KEYS.has(k)),
+  );
   for (const [k, v] of Object.entries(params)) {
     if (['connectionId', 'database', 'table'].includes(k)) continue;
     if (v == null || hostPinnedKeys.has(k)) continue;

From ff0fd281c5dff36b19c187e84afebd7c70d4e4a4 Mon Sep 17 00:00:00 2001
From: milansanjeev <12941259+milansanjeev@users.noreply.github.com>
Date: Thu, 3 Sep 2026 09:03:01 +0530
Subject: [PATCH 04/13] fix(api): address remaining PromQL proxy review
 findings

- joinPrometheusUpstreamUrl: strip ALL trailing slashes from the host
  pathname (was only stripping one), so a host saved with a doubled
  trailing slash doesn't leave a `//` in the joined path.
- Drop the dead relative-path branch (every call site passes an
  absolute path); document the contract instead. Removes the unit
  test that only exercised that branch.
- Invert query-param precedence in proxyToPrometheus: every param the
  request supplies now always wins over a same-named one pinned on the
  Connection host -- including repeatable params like match[], via an
  explicit delete before append so host and request values don't end
  up coexisting. A host-only param the request never references (e.g.
  a VictoriaMetrics extra_label tenant scope) is left untouched. This
  replaces the previous PROXY_OWNED_PARAM_KEYS allowlist, which let a
  host silently override caller-owned params like query/match[]/limit.
- proxyToPrometheus's invalid-host 400 branch: redact userinfo from
  the raw host before interpolating it into the response, and surface
  the caught error's own message instead of one generic string, so a
  wrong-scheme host and an unparseable host get distinct, actionable
  400 bodies.
- Rewrite the host-pinned-param test that asserted the OLD (now
  reverted) precedence; add a test for the new override behavior, a
  match[]-specific regression test (replace, not append-alongside),
  and fix the exemplar-window test that pinned the same `end` value on
  both host and request (making the assertion pass regardless of
  which side actually won).
- Update the changeset to describe the corrected param precedence.
---
 .changeset/promql-proxy-preserve-host-path.md | 12 ++-
 .../api/__tests__/prometheus.int.test.ts      | 74 ++++++++++++++++++-
 .../routers/api/__tests__/prometheus.test.ts  |  8 +-
 packages/api/src/routers/api/prometheus.ts    | 44 ++++++-----
 4 files changed, 107 insertions(+), 31 deletions(-)

diff --git a/.changeset/promql-proxy-preserve-host-path.md b/.changeset/promql-proxy-preserve-host-path.md
index ba134be904..9275b536bf 100644
--- a/.changeset/promql-proxy-preserve-host-path.md
+++ b/.changeset/promql-proxy-preserve-host-path.md
@@ -18,10 +18,8 @@ to work because the absolute API path replaced `/graph`; requests now go to
 Connection hosts before upgrading. Root-mounted hosts (`http://prom:9090` or
 `http://prom:9090/`) are unchanged.
 
-Query parameters already on the Connection host are treated as operator-pinned
-(for example `?extra_label=namespace%3Dprod` on a VictoriaMetrics tenant URL)
-and are no longer overwritten by the same-named request parameter. Incoming
-values for those keys are dropped, including repeatable ones such as `match[]`.
-Exception: `start`, `end`, and `step` stay under the proxy's control so the
-`/query_exemplars` window clamp and chart resolution cannot be defeated by a
-host that already carries those keys.
+Query parameters on the Connection host are now only a fallback: any param the
+request supplies (including repeatable ones such as `match[]`) always wins and
+replaces a same-named host value outright, rather than being dropped. A
+param the request never mentions -- for example `?extra_label=namespace%3Dprod`
+pinning a VictoriaMetrics tenant scope -- is left as-is.
diff --git a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
index 544542e245..e711f68866 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
@@ -359,7 +359,7 @@ describe('prometheus router', () => {
       );
     });
 
-    it('does not let request query params override params pinned on the connection host', async () => {
+    it('leaves a host-pinned param untouched when the request never references it', async () => {
       const { agent, team } = await getLoggedInAgent(server);
       const conn = await seedPrometheusConnection(
         team._id,
@@ -380,7 +380,6 @@ describe('prometheus router', () => {
           start: '1700000000',
           end: '1700000060',
           step: '15s',
-          extra_label: 'namespace=other',
           connectionId: conn._id.toString(),
         })
         .expect(200);
@@ -390,6 +389,44 @@ describe('prometheus router', () => {
       expect(requested.searchParams.get('query')).toBe('up');
     });
 
+    // A host-pinned param the request DOES supply a value for must not
+    // silently win: a Connection host could otherwise override `query`,
+    // `match[]`, or `limit` and the caller would get a wrong answer with no
+    // error (e.g. a pinned `query=up` making every chart on that connection
+    // return the same series regardless of what was actually requested).
+    it('lets a request query param override a same-named one pinned on the connection host', async () => {
+      const { agent, team } = await getLoggedInAgent(server);
+      const conn = await seedPrometheusConnection(
+        team._id,
+        'http://vmselect:8481/select/0/prometheus?extra_label=namespace%3Dprod',
+      );
+
+      mockFetch.mockResolvedValueOnce(
+        fakeUpstreamResponse({
+          status: 'success',
+          data: { resultType: 'matrix', result: [] },
+        }),
+      );
+
+      await agent
+        .get('/v1/prometheus/query_range')
+        .query({
+          query: 'up',
+          start: '1700000000',
+          end: '1700000060',
+          step: '15s',
+          extra_label: 'namespace=other',
+          connectionId: conn._id.toString(),
+        })
+        .expect(200);
+
+      const requested = new URL(mockFetch.mock.calls[0][0] as string);
+      expect(requested.searchParams.get('extra_label')).toBe(
+        'namespace=other',
+      );
+      expect(requested.searchParams.get('query')).toBe('up');
+    });
+
     it('lets the request step override a step pinned on the connection host', async () => {
       const { agent, team } = await getLoggedInAgent(server);
       const conn = await seedPrometheusConnection(
@@ -771,6 +808,33 @@ describe('prometheus router', () => {
       expect(requested.searchParams.has('match[]')).toBe(false);
     });
 
+    // A repeatable param needs its own regression test: a naive fix could
+    // append the request's values alongside a host-pinned one instead of
+    // replacing it, silently narrowing what upstream is asked for (e.g. a
+    // host pinning `match[]` would otherwise make label-value autocomplete
+    // return the wrong series list).
+    it('replaces a host-pinned match[] with the request values, not appends alongside them', async () => {
+      const { agent, team } = await getLoggedInAgent(server);
+      const conn = await seedPrometheusConnection(
+        team._id,
+        'http://prom.example.com?match%5B%5D=host-pinned-series',
+      );
+
+      await agent
+        .get('/v1/prometheus/label/job/values')
+        .query({
+          connectionId: conn._id.toString(),
+          match: ['requested-series-1', 'requested-series-2'],
+        })
+        .expect(200);
+
+      const requested = new URL(String(mockFetch.mock.calls[0][0]));
+      expect(requested.searchParams.getAll('match[]')).toEqual([
+        'requested-series-1',
+        'requested-series-2',
+      ]);
+    });
+
     // queryLabelValues' own bounds/limit/fallback behaviour is covered in
     // controllers/__tests__/timeseriesEngine.int.test.ts. What only the route
     // can show is that query-string params reach it intact — bounds in unix
@@ -1020,9 +1084,13 @@ describe('prometheus router', () => {
       const { agent, team } = await getLoggedInAgent(server);
       const end = 1700000000;
       const thirtyDays = 30 * 24 * 60 * 60;
+      // Pinned an hour earlier than the request's `end` so the assertion
+      // below can only pass if the request's value actually won -- if the
+      // host's pinned value were used instead, `end` wouldn't match.
+      const hostEnd = end - 3600;
       const conn = await seedPrometheusConnection(
         team._id,
-        `http://prom.example.com?start=${end - thirtyDays}&end=${end}`,
+        `http://prom.example.com?start=${hostEnd - thirtyDays}&end=${hostEnd}`,
       );
 
       mockFetch.mockResolvedValueOnce(
diff --git a/packages/api/src/routers/api/__tests__/prometheus.test.ts b/packages/api/src/routers/api/__tests__/prometheus.test.ts
index ad08492b50..dbf11461bf 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.test.ts
@@ -309,13 +309,13 @@ describe('joinPrometheusUpstreamUrl', () => {
     );
   });
 
-  it('prepends a slash when the path has none', () => {
+  it('strips every trailing slash, not just one, so the join never leaves a double slash', () => {
     expect(
       joinPrometheusUpstreamUrl(
-        'http://prometheus:9090',
-        'api/v1/query',
+        'http://prometheus:9090//',
+        '/api/v1/query_range',
       ).toString(),
-    ).toBe('http://prometheus:9090/api/v1/query');
+    ).toBe('http://prometheus:9090/api/v1/query_range');
   });
 
   it('throws on an invalid connection host, matching the proxy 400 path', () => {
diff --git a/packages/api/src/routers/api/prometheus.ts b/packages/api/src/routers/api/prometheus.ts
index efd4042b70..9d0b1e47fd 100644
--- a/packages/api/src/routers/api/prometheus.ts
+++ b/packages/api/src/routers/api/prometheus.ts
@@ -192,6 +192,9 @@ export function isClientDisconnect(err: unknown): boolean {
  * mounted under a prefix) needs that prefix kept. Host userinfo, query, and
  * hash are left untouched.
  *
+ * `path` must be an absolute path (every call site passes a literal starting
+ * with `/`) -- this is not a general-purpose URL joiner.
+ *
  * @see https://github.com/hyperdxio/hyperdx/issues/3046
  */
 export function joinPrometheusUpstreamUrl(
@@ -207,9 +210,11 @@ export function joinPrometheusUpstreamUrl(
   if (url.protocol !== 'http:' && url.protocol !== 'https:') {
     throw new TypeError('Connection host must be http(s)');
   }
-  const basePath = url.pathname.replace(/\/$/, '');
-  const suffix = path.startsWith('/') ? path : `/${path}`;
-  url.pathname = `${basePath}${suffix}`;
+  // Strip ALL trailing slashes, not just one -- a host saved with a doubled
+  // trailing slash (e.g. `http://prom:9090//`) would otherwise leave a `//`
+  // in the joined path, which most servers treat as a distinct (404) path.
+  const basePath = url.pathname.replace(/\/+$/, '');
+  url.pathname = `${basePath}${path}`;
   return url;
 }
 
@@ -232,28 +237,33 @@ async function proxyToPrometheus(
   let url: URL;
   try {
     url = joinPrometheusUpstreamUrl(upstreamHost, path);
-  } catch {
+  } catch (err) {
+    // Redact userinfo the same way `redactedTarget` does below -- this branch
+    // runs before `url` exists, so it can't reuse that helper, but the raw
+    // host is still shown to the browser and may carry `user:pw@`. Surface
+    // the caught error's own message (e.g. the scheme guard's "Connection
+    // host must be http(s)") instead of a single generic string, so a wrong
+    // scheme isn't reported identically to a host that fails to parse at all.
+    const redactedHost = upstreamHost.replace(/:\/\/[^/@]*@/, '://');
     res.status(400).json({
       status: 'error',
       errorType: 'bad_data',
-      error: `Connection host is not a valid URL: ${JSON.stringify(upstreamHost)}`,
+      error: `Invalid Connection host ${JSON.stringify(redactedHost)}: ${err instanceof Error ? err.message : String(err)}`,
     });
     return 400;
   }
-  // Params already on the Connection host are operator-pinned (e.g.
-  // `?extra_label=namespace%3Dprod` on a VictoriaMetrics tenant URL). Skip the
-  // incoming value for those keys rather than appending — including repeatable
-  // ones such as `match[]`. Exception: `start`/`end`/`step` are owned by this
-  // proxy (exemplar window clamp, chart resolution) and still overwrite a host
-  // value so a Connection cannot defeat those bounds.
-  const PROXY_OWNED_PARAM_KEYS = new Set(['start', 'end', 'step']);
-  const hostPinnedKeys = new Set(
-    [...url.searchParams.keys()].filter(k => !PROXY_OWNED_PARAM_KEYS.has(k)),
-  );
+  // Every param the request supplies wins outright, including repeatable
+  // ones like `match[]` -- so a Connection host can never silently override
+  // (or, for `match[]`, narrow) a value the caller or this proxy explicitly
+  // sets. A host-only param the request never mentions (e.g. VictoriaMetrics
+  // `?extra_label=namespace%3Dprod` pinning a tenant scope) is left as-is.
   for (const [k, v] of Object.entries(params)) {
     if (['connectionId', 'database', 'table'].includes(k)) continue;
-    if (v == null || hostPinnedKeys.has(k)) continue;
-    // A repeatable param (`match[]`) appends every value
+    if (v == null) continue;
+    // Clear any host-pinned value at this key first: for a repeatable param
+    // this prevents an `append` from leaving the host's value(s) alongside
+    // the request's rather than replacing them.
+    url.searchParams.delete(k);
     if (Array.isArray(v)) {
       for (const item of v) url.searchParams.append(k, item);
     } else {

From 1141556ee036d96dc3e6c7704880beecc860263b Mon Sep 17 00:00:00 2001
From: milansanjeev <12941259+milansanjeev@users.noreply.github.com>
Date: Thu, 3 Sep 2026 09:13:30 +0530
Subject: [PATCH 05/13] fix(api): userinfo redaction regex missed scheme-less
 hosts, leaking passwords

The 400 branch's ad-hoc redaction (`replace(/:\/\/[^/@]*@/, '://')`) only
strips credentials when the host contains `://`. That's exactly the case
the http(s) scheme guard exists to catch: a host saved as `user:pw@prom:9090`
(no scheme) parses with scheme `user:` and an opaque path, never contains
`://`, and would echo the raw password straight back into the browser-visible
error body -- the opposite of the redaction's own stated intent.

Extracted a shared `redactHostUserinfo` helper (anchored, optional scheme and
`//`) used by both the 400 branch and the success-path `redactedTarget`,
replacing the URL-object-based approach there with the same string helper so
there's one redaction implementation instead of two that could drift. Added
unit tests for the scheme-less case, the normal case, a no-userinfo host, and
an "@" appearing later in a query value (must not be touched).
---
 .../routers/api/__tests__/prometheus.test.ts  | 27 +++++++++++++++
 packages/api/src/routers/api/prometheus.ts    | 33 +++++++++++--------
 2 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/packages/api/src/routers/api/__tests__/prometheus.test.ts b/packages/api/src/routers/api/__tests__/prometheus.test.ts
index dbf11461bf..3dfc7849b7 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.test.ts
@@ -16,6 +16,7 @@ import {
   parseDuration,
   parseTimestamp,
   recordProxyOutcome,
+  redactHostUserinfo,
   resolveExemplarWindow,
 } from '@/routers/api/prometheus';
 
@@ -336,3 +337,29 @@ describe('joinPrometheusUpstreamUrl', () => {
     ).toThrow(/http\(s\)/);
   });
 });
+
+describe('redactHostUserinfo', () => {
+  it('strips userinfo from a normal http(s) URL', () => {
+    expect(redactHostUserinfo('http://user:pw@prom:9090/graph')).toBe(
+      'http://prom:9090/graph',
+    );
+  });
+
+  it('leaves a URL with no userinfo unchanged', () => {
+    expect(redactHostUserinfo('http://prom:9090')).toBe('http://prom:9090');
+  });
+
+  // This is the case the http(s) guard in joinPrometheusUpstreamUrl exists to
+  // catch: `new URL('user:pw@prom:9090')` parses scheme `user:` with an
+  // opaque path, so there is no `://` to anchor on -- a redaction regex that
+  // required one would echo the password straight back into the browser.
+  it('strips userinfo from a scheme-less host with no "//"', () => {
+    expect(redactHostUserinfo('user:pw@prom:9090')).not.toContain('pw');
+  });
+
+  it('does not touch an "@" that appears after the host (e.g. in a query value)', () => {
+    expect(redactHostUserinfo('http://prom:9090/path?to=a@b.com')).toBe(
+      'http://prom:9090/path?to=a@b.com',
+    );
+  });
+});
diff --git a/packages/api/src/routers/api/prometheus.ts b/packages/api/src/routers/api/prometheus.ts
index 9d0b1e47fd..8dc3f41373 100644
--- a/packages/api/src/routers/api/prometheus.ts
+++ b/packages/api/src/routers/api/prometheus.ts
@@ -183,6 +183,19 @@ export function isClientDisconnect(err: unknown): boolean {
   );
 }
 
+/**
+ * Strip userinfo (`user:pw@`) from a Connection host/URL string for safe
+ * display in an error response. Operates on the raw string rather than a
+ * parsed `URL`, because it also has to redact hosts that fail to parse, or
+ * whose userinfo lands in an opaque path instead of `URL.username`/
+ * `.password` -- a scheme-less host like `user:pw@prom:9090` parses with
+ * scheme `user:` and never has a `://`, so a check that required one would
+ * leave the password in the response for exactly that case.
+ */
+export function redactHostUserinfo(host: string): string {
+  return host.replace(/^([a-z][a-z0-9+.-]*:)?(\/\/)?[^/@]*@/i, '$1$2');
+}
+
 /**
  * Join a Connection host with an absolute Prometheus API path.
  *
@@ -238,17 +251,14 @@ async function proxyToPrometheus(
   try {
     url = joinPrometheusUpstreamUrl(upstreamHost, path);
   } catch (err) {
-    // Redact userinfo the same way `redactedTarget` does below -- this branch
-    // runs before `url` exists, so it can't reuse that helper, but the raw
-    // host is still shown to the browser and may carry `user:pw@`. Surface
-    // the caught error's own message (e.g. the scheme guard's "Connection
-    // host must be http(s)") instead of a single generic string, so a wrong
-    // scheme isn't reported identically to a host that fails to parse at all.
-    const redactedHost = upstreamHost.replace(/:\/\/[^/@]*@/, '://');
+    // The raw host is shown to the browser and may carry credentials.
+    // Surface the caught error's own message (e.g. "Connection host must be
+    // http(s)") instead of a single generic string, so a wrong scheme isn't
+    // reported identically to a host that fails to parse at all.
     res.status(400).json({
       status: 'error',
       errorType: 'bad_data',
-      error: `Invalid Connection host ${JSON.stringify(redactedHost)}: ${err instanceof Error ? err.message : String(err)}`,
+      error: `Invalid Connection host ${JSON.stringify(redactHostUserinfo(upstreamHost))}: ${err instanceof Error ? err.message : String(err)}`,
     });
     return 400;
   }
@@ -275,12 +285,7 @@ async function proxyToPrometheus(
   // A connection host may carry basic-auth credentials (`http://user:pw@host`),
   // and the error bodies below are shown in the browser. Strip them for display
   // only — `target` itself still needs them to authenticate.
-  const redactedTarget = (() => {
-    const safe = new URL(url);
-    safe.username = '';
-    safe.password = '';
-    return safe.toString();
-  })();
+  const redactedTarget = redactHostUserinfo(target);
 
   let upstreamResp: Response;
   try {

From e79d4d68e2c53d00dc2309e23b7f9cf8295b289e Mon Sep 17 00:00:00 2001
From: milansanjeev <12941259+milansanjeev@users.noreply.github.com>
Date: Thu, 3 Sep 2026 09:22:44 +0530
Subject: [PATCH 06/13] test(api): pin credential redaction at the proxy call
 site, not just the helper

Only redactHostUserinfo itself was unit-tested; reverting the 400
branch to echo the raw Connection host verbatim (pre-redaction-fix
behavior) would still pass the whole suite. Add an integration case
seeding a scheme-less host with embedded credentials and asserting
the 400 body omits the password while still naming the host.
---
 .../api/__tests__/prometheus.int.test.ts      | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
index e711f68866..09fffce510 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
@@ -296,6 +296,33 @@ describe('prometheus router', () => {
       expect(mockFetch).not.toHaveBeenCalled();
     });
 
+    // Only the pure `redactHostUserinfo` helper is unit-tested for this --
+    // this pins the actual call site, so reverting the 400 branch to echo
+    // the raw host verbatim (as it did before the userinfo-redaction fix)
+    // would fail the suite.
+    it('redacts credentials from a scheme-less host in the 400 body', async () => {
+      const { agent, team } = await getLoggedInAgent(server);
+      const conn = await seedPrometheusConnection(
+        team._id,
+        'user:secret-password@prometheus:9090',
+      );
+
+      const res = await agent
+        .get('/v1/prometheus/query_range')
+        .query({
+          query: 'up',
+          start: '1700000000',
+          end: '1700000060',
+          step: '15s',
+          connectionId: conn._id.toString(),
+        })
+        .expect(400);
+
+      expect(res.body.error).not.toContain('secret-password');
+      expect(res.body.error).toContain('prometheus:9090');
+      expect(mockFetch).not.toHaveBeenCalled();
+    });
+
     it('proxies to upstream Prometheus when connection isPrometheusEndpoint', async () => {
       const { agent, team } = await getLoggedInAgent(server);
       const conn = await seedPrometheusConnection(team._id);

From 13770abc1221d69b4c6eb6429020ca33fbf3bb8a Mon Sep 17 00:00:00 2001
From: milansanjeev <12941259+milansanjeev@users.noreply.github.com>
Date: Thu, 3 Sep 2026 09:24:40 +0530
Subject: [PATCH 07/13] fix(api): strip a password containing "@" fully, not
 just up to its first occurrence

redactHostUserinfo's userinfo-matching group excluded "@" from what it
consumed, so it stopped at the first "@" in the userinfo segment --
a password containing a literal "@" left its suffix in the redacted
output. Match through the last "@" before the first path separator
instead (still bounded by "/" so query-string "@"s are untouched).

Also documents the known, accepted limitation that a scheme-less host
like `user:pw@prom:9090` (which parses with scheme `user:`, ambiguous
with real userinfo) only guarantees the password is stripped, not the
username, and that the changeset should tell operators to trim a
stray host query string too, not just a stray path.
---
 .changeset/promql-proxy-preserve-host-path.md |  6 +++-
 .../routers/api/__tests__/prometheus.test.ts  | 10 +++++++
 packages/api/src/routers/api/prometheus.ts    | 29 ++++++++++++++-----
 3 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/.changeset/promql-proxy-preserve-host-path.md b/.changeset/promql-proxy-preserve-host-path.md
index 9275b536bf..d7a38dc301 100644
--- a/.changeset/promql-proxy-preserve-host-path.md
+++ b/.changeset/promql-proxy-preserve-host-path.md
@@ -22,4 +22,8 @@ Query parameters on the Connection host are now only a fallback: any param the
 request supplies (including repeatable ones such as `match[]`) always wins and
 replaces a same-named host value outright, rather than being dropped. A
 param the request never mentions -- for example `?extra_label=namespace%3Dprod`
-pinning a VictoriaMetrics tenant scope -- is left as-is.
+pinning a VictoriaMetrics tenant scope -- is left as-is. This also means a host
+copied with a stray query string (not just a stray path) now forwards that
+query string upstream as a fallback on every request unless the same key is
+part of the request itself -- trim those too if they weren't intended as
+Prometheus API params.
diff --git a/packages/api/src/routers/api/__tests__/prometheus.test.ts b/packages/api/src/routers/api/__tests__/prometheus.test.ts
index 3dfc7849b7..5070c8424c 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.test.ts
@@ -362,4 +362,14 @@ describe('redactHostUserinfo', () => {
       'http://prom:9090/path?to=a@b.com',
     );
   });
+
+  // A password containing a literal "@" must be fully stripped, not just up
+  // to its first occurrence -- otherwise the suffix after the first "@"
+  // (part of the actual secret) would leak into the redacted output.
+  it('fully strips a password that itself contains "@"', () => {
+    const redacted = redactHostUserinfo('http://user:p@ss@prom:9090/graph');
+    expect(redacted).toBe('http://prom:9090/graph');
+    expect(redacted).not.toContain('p@ss');
+    expect(redacted).not.toContain('ss@');
+  });
 });
diff --git a/packages/api/src/routers/api/prometheus.ts b/packages/api/src/routers/api/prometheus.ts
index 8dc3f41373..24d648f834 100644
--- a/packages/api/src/routers/api/prometheus.ts
+++ b/packages/api/src/routers/api/prometheus.ts
@@ -184,16 +184,29 @@ export function isClientDisconnect(err: unknown): boolean {
 }
 
 /**
- * Strip userinfo (`user:pw@`) from a Connection host/URL string for safe
- * display in an error response. Operates on the raw string rather than a
- * parsed `URL`, because it also has to redact hosts that fail to parse, or
- * whose userinfo lands in an opaque path instead of `URL.username`/
- * `.password` -- a scheme-less host like `user:pw@prom:9090` parses with
- * scheme `user:` and never has a `://`, so a check that required one would
- * leave the password in the response for exactly that case.
+ * Strip userinfo (`user:pw@`) from a Connection host/URL string, or a fully
+ * serialized target URL (host + path + query), for safe display in an error
+ * response -- assumes a host or host+path is present, not a bare host+query
+ * (an `@` in a query value with no path before it would be misread as
+ * userinfo, though the "no path, no `/`" shape doesn't occur for this
+ * proxy's own targets).
+ *
+ * Operates on the raw string rather than a parsed `URL`, because it also has
+ * to redact hosts that fail to parse, or whose userinfo lands in an opaque
+ * path instead of `URL.username`/`.password` -- a scheme-less host like
+ * `user:pw@prom:9090` parses with scheme `user:` and never has a `://`, so a
+ * check that required one would leave the password in the response for
+ * exactly that case. In that shape the leading `user:` reads as the scheme
+ * and survives redaction (only the password after it is stripped) -- there
+ * is no way to tell it apart from a real scheme without a fixed allowlist,
+ * so this only guarantees the password is removed, not the username.
+ *
+ * Matches through the *last* `@` before the first path separator, not the
+ * first, so a password that itself contains `@` is fully stripped rather
+ * than leaving its suffix in the output.
  */
 export function redactHostUserinfo(host: string): string {
-  return host.replace(/^([a-z][a-z0-9+.-]*:)?(\/\/)?[^/@]*@/i, '$1$2');
+  return host.replace(/^([a-z][a-z0-9+.-]*:)?(\/\/)?[^/]*@/i, '$1$2');
 }
 
 /**

From 9e0400ecd4d756255ed1961b8bb27e838ec4e05f Mon Sep 17 00:00:00 2001
From: milansanjeev <12941259+milansanjeev@users.noreply.github.com>
Date: Thu, 3 Sep 2026 09:41:44 +0530
Subject: [PATCH 08/13] fix(api): restrict caller-overridable PromQL params to
 a real allowlist; fix limit=0 loss

Three issues from the latest review round:

1. getParams() spreads the entire req.query/req.body with no allowlist, so
   the "request always wins" merge loop let a request override ANY
   same-named key on the Connection host -- including non-Prometheus keys
   like VictoriaMetrics's extra_label, which a host may pin as a
   tenant-isolation scope. Restrict the merge to a fixed set of real
   Prometheus API params (query, time, start, end, step, match, match[],
   limit); anything else the request supplies is now ignored, so a
   caller can never un-pin a host-only param just by naming it.

2. The /label/:name/values route dropped `limit=0` before it ever reached
   proxyToPrometheus (`limit ? {...} : {}` treats 0 as absent), so a
   request explicitly asking for "unlimited" silently lost to a
   host-pinned limit. Forward it based on presence (`limit != null`), and
   have the merge loop turn a request `limit=0` into deleting any
   host-pinned limit rather than sending a literal "0" upstream --
   preserving the existing "0 forwards as absent" contract while still
   letting the request's 0 clear a host value.

3. The 400 branch's redacted-host echo is fundamentally fragile: a
   regex-based redactor can always be wrong for some malformed shape (this
   round found one: a "/" preceding credentials defeats the last-fix's
   [^/]*@ boundary). Stop echoing the host at all -- err.message alone is
   always safe, since it's either this function's own fixed string or
   URL's fixed "Invalid URL" (verified against Node, never echoes input).

Rewrote the extra_label test to demonstrate the new (correct) "host-only
param survives" behavior instead of the old "request overrides it"
behavior it was asserting; added tests for query overriding a host-pinned
query, limit=0 clearing a host-pinned limit, and a happy-path fetch-target
assertion that real credentials still reach the actual upstream call (only
the display-redaction path had coverage before).
---
 .../api/__tests__/prometheus.int.test.ts      | 117 ++++++++++++++++--
 packages/api/src/routers/api/prometheus.ts    |  57 +++++++--
 2 files changed, 148 insertions(+), 26 deletions(-)

diff --git a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
index 09fffce510..c10259bed7 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
@@ -291,16 +291,16 @@ describe('prometheus router', () => {
       expect(res.body).toMatchObject({
         status: 'error',
         errorType: 'bad_data',
-        error: expect.stringContaining('prometheus:9090'),
+        error: expect.stringContaining('http(s)'),
       });
       expect(mockFetch).not.toHaveBeenCalled();
     });
 
-    // Only the pure `redactHostUserinfo` helper is unit-tested for this --
-    // this pins the actual call site, so reverting the 400 branch to echo
-    // the raw host verbatim (as it did before the userinfo-redaction fix)
-    // would fail the suite.
-    it('redacts credentials from a scheme-less host in the 400 body', async () => {
+    // The 400 branch never echoes the Connection host at all (rather than
+    // trying to redact it), so this pins the actual call site: reverting to
+    // interpolate the raw host back in would fail the suite even though only
+    // the pure `redactHostUserinfo` helper is otherwise unit-tested.
+    it('never echoes the connection host (or any credentials in it) in the 400 body', async () => {
       const { agent, team } = await getLoggedInAgent(server);
       const conn = await seedPrometheusConnection(
         team._id,
@@ -319,7 +319,8 @@ describe('prometheus router', () => {
         .expect(400);
 
       expect(res.body.error).not.toContain('secret-password');
-      expect(res.body.error).toContain('prometheus:9090');
+      expect(res.body.error).not.toContain('prometheus:9090');
+      expect(res.body.error).toContain('http(s)');
       expect(mockFetch).not.toHaveBeenCalled();
     });
 
@@ -386,6 +387,38 @@ describe('prometheus router', () => {
       );
     });
 
+    // Every other userinfo test asserts what's redacted for *display* (the
+    // error bodies below). The actual outgoing `fetch` target must still
+    // carry real credentials, or authenticated Prometheus connections would
+    // silently stop authenticating.
+    it('still sends real credentials to the actual upstream fetch call', async () => {
+      const { agent, team } = await getLoggedInAgent(server);
+      const conn = await seedPrometheusConnection(
+        team._id,
+        'http://user:pw@prom.example.com',
+      );
+
+      mockFetch.mockResolvedValueOnce(
+        fakeUpstreamResponse({
+          status: 'success',
+          data: { resultType: 'matrix', result: [] },
+        }),
+      );
+
+      await agent
+        .get('/v1/prometheus/query_range')
+        .query({
+          query: 'up',
+          start: '1700000000',
+          end: '1700000060',
+          step: '15s',
+          connectionId: conn._id.toString(),
+        })
+        .expect(200);
+
+      expect(mockFetch.mock.calls[0][0] as string).toContain('user:pw@');
+    });
+
     it('leaves a host-pinned param untouched when the request never references it', async () => {
       const { agent, team } = await getLoggedInAgent(server);
       const conn = await seedPrometheusConnection(
@@ -416,12 +449,48 @@ describe('prometheus router', () => {
       expect(requested.searchParams.get('query')).toBe('up');
     });
 
-    // A host-pinned param the request DOES supply a value for must not
-    // silently win: a Connection host could otherwise override `query`,
-    // `match[]`, or `limit` and the caller would get a wrong answer with no
-    // error (e.g. a pinned `query=up` making every chart on that connection
-    // return the same series regardless of what was actually requested).
+    // A host-pinned Prometheus-native param the request DOES supply a value
+    // for must not silently win: a Connection host could otherwise override
+    // `query`, `match[]`, or `limit` and the caller would get a wrong answer
+    // with no error (e.g. a pinned `query=up` making every chart on that
+    // connection return the same series regardless of what was requested).
     it('lets a request query param override a same-named one pinned on the connection host', async () => {
+      const { agent, team } = await getLoggedInAgent(server);
+      const conn = await seedPrometheusConnection(
+        team._id,
+        'http://vmselect:8481/select/0/prometheus?query=host_pinned_query',
+      );
+
+      mockFetch.mockResolvedValueOnce(
+        fakeUpstreamResponse({
+          status: 'success',
+          data: { resultType: 'matrix', result: [] },
+        }),
+      );
+
+      await agent
+        .get('/v1/prometheus/query_range')
+        .query({
+          query: 'up',
+          start: '1700000000',
+          end: '1700000060',
+          step: '15s',
+          connectionId: conn._id.toString(),
+        })
+        .expect(200);
+
+      const requested = new URL(mockFetch.mock.calls[0][0] as string);
+      expect(requested.searchParams.get('query')).toBe('up');
+    });
+
+    // Unlike a real Prometheus API param, an arbitrary key the request
+    // happens to also send (here VictoriaMetrics's own `extra_label`, which
+    // a Connection host may pin as a tenant-isolation scope) must NOT be
+    // caller-overridable: `getParams` spreads the entire incoming
+    // query/body with no allowlist, so without restricting the merge to
+    // real Prometheus params, any request could un-pin a host's scoping
+    // param just by naming it.
+    it('does not let a request override a non-Prometheus param pinned on the connection host', async () => {
       const { agent, team } = await getLoggedInAgent(server);
       const conn = await seedPrometheusConnection(
         team._id,
@@ -449,7 +518,7 @@ describe('prometheus router', () => {
 
       const requested = new URL(mockFetch.mock.calls[0][0] as string);
       expect(requested.searchParams.get('extra_label')).toBe(
-        'namespace=other',
+        'namespace=prod',
       );
       expect(requested.searchParams.get('query')).toBe('up');
     });
@@ -717,6 +786,28 @@ describe('prometheus router', () => {
       expect(requested.searchParams.has('limit')).toBe(false);
     });
 
+    // `limit: '0'` is falsy, so a naive `limit ? {...} : {}` guard on the
+    // route drops it before it ever reaches proxyToPrometheus's merge loop --
+    // a host-pinned limit would then silently survive even though the
+    // request explicitly asked for "unlimited". Pin a non-zero limit on the
+    // host and confirm the request's 0 still clears it (rather than 0 itself
+    // reaching upstream, which the previous test guards separately).
+    it('lets a request limit of 0 clear a non-zero limit pinned on the connection host', async () => {
+      const { agent, team } = await getLoggedInAgent(server);
+      const conn = await seedPrometheusConnection(
+        team._id,
+        'http://prom.example.com?limit=100',
+      );
+
+      await agent
+        .get('/v1/prometheus/label/job/values')
+        .query({ connectionId: conn._id.toString(), limit: '0' })
+        .expect(200);
+
+      const requested = new URL(String(mockFetch.mock.calls[0][0]));
+      expect(requested.searchParams.has('limit')).toBe(false);
+    });
+
     it('rejects a negative limit', async () => {
       const { agent, team } = await getLoggedInAgent(server);
       const conn = await seedPrometheusConnection(team._id);
diff --git a/packages/api/src/routers/api/prometheus.ts b/packages/api/src/routers/api/prometheus.ts
index 24d648f834..5092e72e4d 100644
--- a/packages/api/src/routers/api/prometheus.ts
+++ b/packages/api/src/routers/api/prometheus.ts
@@ -264,29 +264,55 @@ async function proxyToPrometheus(
   try {
     url = joinPrometheusUpstreamUrl(upstreamHost, path);
   } catch (err) {
-    // The raw host is shown to the browser and may carry credentials.
-    // Surface the caught error's own message (e.g. "Connection host must be
-    // http(s)") instead of a single generic string, so a wrong scheme isn't
-    // reported identically to a host that fails to parse at all.
+    // Not echoing `upstreamHost` at all -- a redaction regex on an arbitrary
+    // (possibly malformed, since this is the failure branch) string can
+    // always be wrong for some shape (e.g. a `/` preceding the credentials).
+    // `err.message` is always safe to show: it's either this function's own
+    // fixed "Connection host must be http(s)", or `URL`'s parse-failure
+    // message, which is the fixed string "Invalid URL" and never echoes the
+    // input (verified against Node's URL implementation).
     res.status(400).json({
       status: 'error',
       errorType: 'bad_data',
-      error: `Invalid Connection host ${JSON.stringify(redactHostUserinfo(upstreamHost))}: ${err instanceof Error ? err.message : String(err)}`,
+      error: `Invalid Connection host: ${err instanceof Error ? err.message : String(err)}`,
     });
     return 400;
   }
-  // Every param the request supplies wins outright, including repeatable
-  // ones like `match[]` -- so a Connection host can never silently override
-  // (or, for `match[]`, narrow) a value the caller or this proxy explicitly
-  // sets. A host-only param the request never mentions (e.g. VictoriaMetrics
-  // `?extra_label=namespace%3Dprod` pinning a tenant scope) is left as-is.
+  // Only real Prometheus API params are ever caller-settable. `params` is
+  // built upstream by spreading the *entire* `req.query`/`req.body` with no
+  // allowlist (see `getParams`), so without this, a request could supply an
+  // arbitrary key -- e.g. VictoriaMetrics's `extra_label`, which a Connection
+  // host may pin as a tenant-isolation scope -- and un-pin or override it,
+  // even though no legitimate caller ever sends that key.
+  //
+  // For a key in this set, the request always wins outright, including
+  // repeatable ones like `match[]` -- so a Connection host can never silently
+  // override (or, for `match[]`, narrow) a value the caller or this proxy
+  // explicitly sets. A host-only param outside this set (e.g. the
+  // `extra_label` example above) is left as-is.
+  const CALLER_SETTABLE_PARAM_KEYS = new Set([
+    'query',
+    'time',
+    'start',
+    'end',
+    'step',
+    'match',
+    'match[]',
+    'limit',
+  ]);
   for (const [k, v] of Object.entries(params)) {
-    if (['connectionId', 'database', 'table'].includes(k)) continue;
+    if (!CALLER_SETTABLE_PARAM_KEYS.has(k)) continue;
     if (v == null) continue;
     // Clear any host-pinned value at this key first: for a repeatable param
     // this prevents an `append` from leaving the host's value(s) alongside
-    // the request's rather than replacing them.
+    // the request's rather than replacing them. This also means a request's
+    // `limit=0` still clears a host-pinned `limit` (see below) even though
+    // it isn't itself re-set.
     url.searchParams.delete(k);
+    // Prometheus reads a limit of 0 as "unlimited", the same as omitting the
+    // key entirely -- forward that by omission too, rather than a literal
+    // "0", so upstream sees the same request shape it always has.
+    if (k === 'limit' && v === '0') continue;
     if (Array.isArray(v)) {
       for (const item of v) url.searchParams.append(k, item);
     } else {
@@ -921,7 +947,12 @@ router.get('/label/:name/values', async (req, res) => {
         {
           ...(start != null ? { start: String(start) } : {}),
           ...(end != null ? { end: String(end) } : {}),
-          ...(limit ? { limit: String(limit) } : {}),
+          // `limit` is 0 or absent when validation passes (see the schema
+          // above); 0 means "unlimited" to Prometheus and must still be
+          // forwarded (`v == null` is the merge loop's own absence check),
+          // otherwise a request explicitly asking for unlimited results
+          // silently loses to a host-pinned `limit`.
+          ...(limit != null ? { limit: String(limit) } : {}),
           // Restored under the name Prometheus expects
           ...(match != null ? { 'match[]': match } : {}),
         },

From 9af65f4c6e1c2f739b22b7ed11974d8b8496e3a6 Mon Sep 17 00:00:00 2001
From: milansanjeev <12941259+milansanjeev@users.noreply.github.com>
Date: Thu, 3 Sep 2026 09:52:40 +0530
Subject: [PATCH 09/13] fix(api): drop query/hash from the displayed upstream
 target, not just userinfo

Preserving the Connection host's path prefix (this PR's whole point) also
now preserves its query string on the joined URL. A secret pinned there
(e.g. VictoriaMetrics `?authKey=...`) flows into `target` and, since
`redactHostUserinfo` only ever stripped userinfo, would be echoed
verbatim in the 502/504 error bodies shown in the browser -- a new
exposure this PR created (the pre-fix `new URL(path, host)` discarded the
host's query entirely, so this case couldn't previously occur).

Build the displayed target from `url.origin` + `url.pathname` only,
dropping the query/hash outright rather than trying to redact only the
secret-shaped parts of it (there's no way to tell a legitimate host query
value apart from a pinned secret once merged into one URL). `url.origin`
never includes userinfo per the URL spec, which also makes it this
function's only remaining caller -- the 400 branch stopped echoing the
host entirely in the previous commit -- so redactHostUserinfo has no
production call site left. Removed it and its dedicated unit tests rather
than leave unused code around.

Added an integration test asserting a host-query-string secret is absent
from a 502 body (mirrors the existing basic-auth-userinfo 502 test).
---
 .../api/__tests__/prometheus.int.test.ts      | 33 +++++++++++++++
 .../routers/api/__tests__/prometheus.test.ts  | 37 -----------------
 packages/api/src/routers/api/prometheus.ts    | 40 +++++--------------
 3 files changed, 43 insertions(+), 67 deletions(-)

diff --git a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
index c10259bed7..b98aeab1cb 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
@@ -254,6 +254,39 @@ describe('prometheus router', () => {
       expect(res.body.error).not.toContain('s3cr3t');
     });
 
+    // Unlike basic-auth userinfo, a secret pinned in the host's own query
+    // string (e.g. VictoriaMetrics `?authKey=...`) is preserved verbatim on
+    // the upstream URL by the param merge -- `redactedTarget` must drop the
+    // whole query string for display, not try to redact only the
+    // secret-shaped parts of it, or this leaks into the 502 body.
+    it('does not leak a secret pinned in the host query string into the 502 message', async () => {
+      const { agent, team } = await getLoggedInAgent(server);
+      const conn = await seedPrometheusConnection(
+        team._id,
+        'http://prom.example.com?authKey=super-secret-token',
+      );
+
+      mockFetch.mockRejectedValueOnce(
+        Object.assign(new Error('fetch failed'), {
+          cause: { code: 'ECONNREFUSED' },
+        }),
+      );
+
+      const res = await agent
+        .get('/v1/prometheus/query_exemplars')
+        .query({
+          query: 'up',
+          start: '1700000000',
+          end: '1700000060',
+          connectionId: conn._id.toString(),
+        })
+        .expect(502);
+
+      expect(res.body.error).toContain('ECONNREFUSED');
+      expect(res.body.error).not.toContain('super-secret-token');
+      expect(res.body.error).toContain('prom.example.com');
+    });
+
     // nosniff is set by router middleware, so the helper's own error bodies —
     // which echo the caller-supplied host — carry it too.
     it('sends nosniff on its own error responses', async () => {
diff --git a/packages/api/src/routers/api/__tests__/prometheus.test.ts b/packages/api/src/routers/api/__tests__/prometheus.test.ts
index 5070c8424c..dbf11461bf 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.test.ts
@@ -16,7 +16,6 @@ import {
   parseDuration,
   parseTimestamp,
   recordProxyOutcome,
-  redactHostUserinfo,
   resolveExemplarWindow,
 } from '@/routers/api/prometheus';
 
@@ -337,39 +336,3 @@ describe('joinPrometheusUpstreamUrl', () => {
     ).toThrow(/http\(s\)/);
   });
 });
-
-describe('redactHostUserinfo', () => {
-  it('strips userinfo from a normal http(s) URL', () => {
-    expect(redactHostUserinfo('http://user:pw@prom:9090/graph')).toBe(
-      'http://prom:9090/graph',
-    );
-  });
-
-  it('leaves a URL with no userinfo unchanged', () => {
-    expect(redactHostUserinfo('http://prom:9090')).toBe('http://prom:9090');
-  });
-
-  // This is the case the http(s) guard in joinPrometheusUpstreamUrl exists to
-  // catch: `new URL('user:pw@prom:9090')` parses scheme `user:` with an
-  // opaque path, so there is no `://` to anchor on -- a redaction regex that
-  // required one would echo the password straight back into the browser.
-  it('strips userinfo from a scheme-less host with no "//"', () => {
-    expect(redactHostUserinfo('user:pw@prom:9090')).not.toContain('pw');
-  });
-
-  it('does not touch an "@" that appears after the host (e.g. in a query value)', () => {
-    expect(redactHostUserinfo('http://prom:9090/path?to=a@b.com')).toBe(
-      'http://prom:9090/path?to=a@b.com',
-    );
-  });
-
-  // A password containing a literal "@" must be fully stripped, not just up
-  // to its first occurrence -- otherwise the suffix after the first "@"
-  // (part of the actual secret) would leak into the redacted output.
-  it('fully strips a password that itself contains "@"', () => {
-    const redacted = redactHostUserinfo('http://user:p@ss@prom:9090/graph');
-    expect(redacted).toBe('http://prom:9090/graph');
-    expect(redacted).not.toContain('p@ss');
-    expect(redacted).not.toContain('ss@');
-  });
-});
diff --git a/packages/api/src/routers/api/prometheus.ts b/packages/api/src/routers/api/prometheus.ts
index 5092e72e4d..b250870c99 100644
--- a/packages/api/src/routers/api/prometheus.ts
+++ b/packages/api/src/routers/api/prometheus.ts
@@ -183,32 +183,6 @@ export function isClientDisconnect(err: unknown): boolean {
   );
 }
 
-/**
- * Strip userinfo (`user:pw@`) from a Connection host/URL string, or a fully
- * serialized target URL (host + path + query), for safe display in an error
- * response -- assumes a host or host+path is present, not a bare host+query
- * (an `@` in a query value with no path before it would be misread as
- * userinfo, though the "no path, no `/`" shape doesn't occur for this
- * proxy's own targets).
- *
- * Operates on the raw string rather than a parsed `URL`, because it also has
- * to redact hosts that fail to parse, or whose userinfo lands in an opaque
- * path instead of `URL.username`/`.password` -- a scheme-less host like
- * `user:pw@prom:9090` parses with scheme `user:` and never has a `://`, so a
- * check that required one would leave the password in the response for
- * exactly that case. In that shape the leading `user:` reads as the scheme
- * and survives redaction (only the password after it is stripped) -- there
- * is no way to tell it apart from a real scheme without a fixed allowlist,
- * so this only guarantees the password is removed, not the username.
- *
- * Matches through the *last* `@` before the first path separator, not the
- * first, so a password that itself contains `@` is fully stripped rather
- * than leaving its suffix in the output.
- */
-export function redactHostUserinfo(host: string): string {
-  return host.replace(/^([a-z][a-z0-9+.-]*:)?(\/\/)?[^/]*@/i, '$1$2');
-}
-
 /**
  * Join a Connection host with an absolute Prometheus API path.
  *
@@ -321,10 +295,16 @@ async function proxyToPrometheus(
   }
   const target = url.toString();
 
-  // A connection host may carry basic-auth credentials (`http://user:pw@host`),
-  // and the error bodies below are shown in the browser. Strip them for display
-  // only — `target` itself still needs them to authenticate.
-  const redactedTarget = redactHostUserinfo(target);
+  // A connection host may carry basic-auth credentials (`http://user:pw@host`)
+  // or a secret pinned in its own query string (e.g. VictoriaMetrics
+  // `?authKey=...`, preserved by the merge above for any key outside
+  // `CALLER_SETTABLE_PARAM_KEYS`), and the error bodies below are shown in
+  // the browser. `url.origin` never includes userinfo (WHATWG URL spec), and
+  // dropping the query/hash entirely -- rather than trying to redact only the
+  // secret-shaped parts of it -- means there's no query key to ever miss.
+  // `target` itself is unaffected and still carries everything needed to
+  // authenticate.
+  const redactedTarget = `${url.origin}${url.pathname}`;
 
   let upstreamResp: Response;
   try {

From b84ef5fecf244c4773b5a0d9e0ffc8aa03d62416 Mon Sep 17 00:00:00 2001
From: milansanjeev <12941259+milansanjeev@users.noreply.github.com>
Date: Thu, 3 Sep 2026 10:00:19 +0530
Subject: [PATCH 10/13] chore(api): address final PromQL proxy review nitpicks

- Hoist CALLER_SETTABLE_PARAM_KEYS to module scope instead of
  reallocating it on every proxyToPrometheus call.
- Fix a stale test comment referencing the now-removed
  redactHostUserinfo helper.
- Add a 504-timeout-branch test mirroring the existing 502 one,
  pinning that it also drops both userinfo and a host query-string
  secret from the error message (previously only the 502 path had
  direct coverage).
---
 .../api/__tests__/prometheus.int.test.ts      | 35 ++++++++++++++-
 packages/api/src/routers/api/prometheus.ts    | 45 ++++++++++---------
 2 files changed, 56 insertions(+), 24 deletions(-)

diff --git a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
index b98aeab1cb..f27610e847 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
@@ -287,6 +287,38 @@ describe('prometheus router', () => {
       expect(res.body.error).toContain('prom.example.com');
     });
 
+    // The 504 timeout branch builds its message from the same `redactedTarget`
+    // as the 502 branch, but had no test of its own -- pin it separately so a
+    // regression isolated to this branch (e.g. someone reintroducing the raw
+    // `target` here specifically) would still be caught.
+    it('does not leak credentials or a host query-string secret into the 504 message', async () => {
+      const { agent, team } = await getLoggedInAgent(server);
+      const conn = await seedPrometheusConnection(
+        team._id,
+        'http://user:s3cr3t@prom.example.com?authKey=super-secret-token',
+      );
+
+      mockFetch.mockRejectedValueOnce(
+        Object.assign(new Error('The operation was aborted'), {
+          name: 'TimeoutError',
+        }),
+      );
+
+      const res = await agent
+        .get('/v1/prometheus/query_exemplars')
+        .query({
+          query: 'up',
+          start: '1700000000',
+          end: '1700000060',
+          connectionId: conn._id.toString(),
+        })
+        .expect(504);
+
+      expect(res.body.error).not.toContain('s3cr3t');
+      expect(res.body.error).not.toContain('super-secret-token');
+      expect(res.body.error).toContain('prom.example.com');
+    });
+
     // nosniff is set by router middleware, so the helper's own error bodies —
     // which echo the caller-supplied host — carry it too.
     it('sends nosniff on its own error responses', async () => {
@@ -331,8 +363,7 @@ describe('prometheus router', () => {
 
     // The 400 branch never echoes the Connection host at all (rather than
     // trying to redact it), so this pins the actual call site: reverting to
-    // interpolate the raw host back in would fail the suite even though only
-    // the pure `redactHostUserinfo` helper is otherwise unit-tested.
+    // interpolate the raw host back in would fail the suite.
     it('never echoes the connection host (or any credentials in it) in the 400 body', async () => {
       const { agent, team } = await getLoggedInAgent(server);
       const conn = await seedPrometheusConnection(
diff --git a/packages/api/src/routers/api/prometheus.ts b/packages/api/src/routers/api/prometheus.ts
index b250870c99..e2b6ccc1d2 100644
--- a/packages/api/src/routers/api/prometheus.ts
+++ b/packages/api/src/routers/api/prometheus.ts
@@ -218,6 +218,24 @@ export function joinPrometheusUpstreamUrl(
   return url;
 }
 
+// Only real Prometheus API params are ever caller-settable in
+// proxyToPrometheus's query merge below. `params` there is built upstream by
+// spreading the *entire* `req.query`/`req.body` with no allowlist (see
+// `getParams`), so without this, a request could supply an arbitrary key --
+// e.g. VictoriaMetrics's `extra_label`, which a Connection host may pin as a
+// tenant-isolation scope -- and un-pin or override it, even though no
+// legitimate caller ever sends that key.
+const CALLER_SETTABLE_PARAM_KEYS = new Set([
+  'query',
+  'time',
+  'start',
+  'end',
+  'step',
+  'match',
+  'match[]',
+  'limit',
+]);
+
 // Forwards the response straight from the upstream Prometheus to the
 // HyperDX client. Returns the HTTP status it wrote, so callers can record an
 // error metric: this helper handles its own failures by writing 400/502/504 and
@@ -252,28 +270,11 @@ async function proxyToPrometheus(
     });
     return 400;
   }
-  // Only real Prometheus API params are ever caller-settable. `params` is
-  // built upstream by spreading the *entire* `req.query`/`req.body` with no
-  // allowlist (see `getParams`), so without this, a request could supply an
-  // arbitrary key -- e.g. VictoriaMetrics's `extra_label`, which a Connection
-  // host may pin as a tenant-isolation scope -- and un-pin or override it,
-  // even though no legitimate caller ever sends that key.
-  //
-  // For a key in this set, the request always wins outright, including
-  // repeatable ones like `match[]` -- so a Connection host can never silently
-  // override (or, for `match[]`, narrow) a value the caller or this proxy
-  // explicitly sets. A host-only param outside this set (e.g. the
-  // `extra_label` example above) is left as-is.
-  const CALLER_SETTABLE_PARAM_KEYS = new Set([
-    'query',
-    'time',
-    'start',
-    'end',
-    'step',
-    'match',
-    'match[]',
-    'limit',
-  ]);
+  // For a key in CALLER_SETTABLE_PARAM_KEYS, the request always wins
+  // outright, including repeatable ones like `match[]` -- so a Connection
+  // host can never silently override (or, for `match[]`, narrow) a value
+  // the caller or this proxy explicitly sets. A host-only param outside
+  // that set (e.g. VictoriaMetrics's `extra_label`) is left as-is.
   for (const [k, v] of Object.entries(params)) {
     if (!CALLER_SETTABLE_PARAM_KEYS.has(k)) continue;
     if (v == null) continue;

From 03dbb89966a4ae592dd564c823fa63960fa9284d Mon Sep 17 00:00:00 2001
From: milansanjeev <12941259+milansanjeev@users.noreply.github.com>
Date: Thu, 3 Sep 2026 10:02:32 +0530
Subject: [PATCH 11/13] fix(api): allowlist was silently dropping the real
 timeout/stats params

CALLER_SETTABLE_PARAM_KEYS omitted timeout and stats -- both real
Prometheus API params on /api/v1/query and /api/v1/query_range. Under
the old denylist a request's ?timeout=5s was forwarded; with the new
allowlist it was silently dropped instead, and the query would run to
this proxy's own PROMETHEUS_PROXY_TIMEOUT_MS. Added both keys and a
regression test forwarding a request timeout that overrides one pinned
on the host.

Also corrected the changeset, which claimed "any param the request
supplies always wins" -- no longer true once the allowlist exists.
---
 .changeset/promql-proxy-preserve-host-path.md | 22 +++++++-----
 .../api/__tests__/prometheus.int.test.ts      | 35 +++++++++++++++++++
 packages/api/src/routers/api/prometheus.ts    |  2 ++
 3 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/.changeset/promql-proxy-preserve-host-path.md b/.changeset/promql-proxy-preserve-host-path.md
index d7a38dc301..2b0d091831 100644
--- a/.changeset/promql-proxy-preserve-host-path.md
+++ b/.changeset/promql-proxy-preserve-host-path.md
@@ -18,12 +18,16 @@ to work because the absolute API path replaced `/graph`; requests now go to
 Connection hosts before upgrading. Root-mounted hosts (`http://prom:9090` or
 `http://prom:9090/`) are unchanged.
 
-Query parameters on the Connection host are now only a fallback: any param the
-request supplies (including repeatable ones such as `match[]`) always wins and
-replaces a same-named host value outright, rather than being dropped. A
-param the request never mentions -- for example `?extra_label=namespace%3Dprod`
-pinning a VictoriaMetrics tenant scope -- is left as-is. This also means a host
-copied with a stray query string (not just a stray path) now forwards that
-query string upstream as a fallback on every request unless the same key is
-part of the request itself -- trim those too if they weren't intended as
-Prometheus API params.
+Query parameters on the Connection host are now only a fallback for a fixed
+set of real Prometheus API params (`query`, `time`, `start`, `end`, `step`,
+`match`/`match[]`, `limit`, `timeout`, `stats`): a request value for one of
+these (including repeatable ones such as `match[]`) always wins and replaces
+a same-named host value outright, rather than being dropped. Any other host
+query key the request never mentions -- for example
+`?extra_label=namespace%3Dprod` pinning a VictoriaMetrics tenant scope -- is
+left as-is and is never overridable by the request, since a param name
+outside that fixed set is not forwarded at all regardless of what the host
+carries. This also means a host copied with a stray query string (not just a
+stray path) now forwards its non-Prometheus keys upstream as a fallback on
+every request -- trim those too if they weren't intended as Prometheus API
+params.
diff --git a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
index f27610e847..080f245aa1 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
@@ -547,6 +547,41 @@ describe('prometheus router', () => {
       expect(requested.searchParams.get('query')).toBe('up');
     });
 
+    // `timeout` and `stats` are real Prometheus API params on
+    // /api/v1/query(_range) -- omitting them from the allowlist (as an
+    // earlier version of this fix did) would silently drop a request's
+    // value rather than forward it, contradicting the "request always wins"
+    // guarantee for every param this list claims to cover.
+    it('forwards a request timeout, overriding one pinned on the connection host', async () => {
+      const { agent, team } = await getLoggedInAgent(server);
+      const conn = await seedPrometheusConnection(
+        team._id,
+        'http://prom.example.com?timeout=5s',
+      );
+
+      mockFetch.mockResolvedValueOnce(
+        fakeUpstreamResponse({
+          status: 'success',
+          data: { resultType: 'matrix', result: [] },
+        }),
+      );
+
+      await agent
+        .get('/v1/prometheus/query_range')
+        .query({
+          query: 'up',
+          start: '1700000000',
+          end: '1700000060',
+          step: '15s',
+          timeout: '30s',
+          connectionId: conn._id.toString(),
+        })
+        .expect(200);
+
+      const requested = new URL(mockFetch.mock.calls[0][0] as string);
+      expect(requested.searchParams.get('timeout')).toBe('30s');
+    });
+
     // Unlike a real Prometheus API param, an arbitrary key the request
     // happens to also send (here VictoriaMetrics's own `extra_label`, which
     // a Connection host may pin as a tenant-isolation scope) must NOT be
diff --git a/packages/api/src/routers/api/prometheus.ts b/packages/api/src/routers/api/prometheus.ts
index e2b6ccc1d2..0f4a6d0e0c 100644
--- a/packages/api/src/routers/api/prometheus.ts
+++ b/packages/api/src/routers/api/prometheus.ts
@@ -234,6 +234,8 @@ const CALLER_SETTABLE_PARAM_KEYS = new Set([
   'match',
   'match[]',
   'limit',
+  'timeout',
+  'stats',
 ]);
 
 // Forwards the response straight from the upstream Prometheus to the

From 858928b09c4f8d1b0c26a94a221a06a8a35f689e Mon Sep 17 00:00:00 2001
From: milansanjeev <12941259+milansanjeev@users.noreply.github.com>
Date: Thu, 3 Sep 2026 10:11:30 +0530
Subject: [PATCH 12/13] test(api): add symmetric stats-override test; clarify
 changeset drop behavior

stats and timeout were added to the allowlist together but only timeout
had a request-overrides-host regression test, so a regression dropping
just stats would still pass. Add the same test shape for stats.

Also make the changeset explicit that a direct API caller's own
non-Prometheus query param (not just a host-pinned one) is now silently
dropped rather than forwarded, since the allowlist applies regardless of
who supplied the param name.
---
 .changeset/promql-proxy-preserve-host-path.md |  6 ++++
 .../api/__tests__/prometheus.int.test.ts      | 33 +++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/.changeset/promql-proxy-preserve-host-path.md b/.changeset/promql-proxy-preserve-host-path.md
index 2b0d091831..df056c114c 100644
--- a/.changeset/promql-proxy-preserve-host-path.md
+++ b/.changeset/promql-proxy-preserve-host-path.md
@@ -31,3 +31,9 @@ carries. This also means a host copied with a stray query string (not just a
 stray path) now forwards its non-Prometheus keys upstream as a fallback on
 every request -- trim those too if they weren't intended as Prometheus API
 params.
+
+This is also a behavior change for a direct API caller (e.g. curl or
+Terraform) that previously relied on sending an arbitrary, non-Prometheus
+query param through this endpoint: that param is now silently dropped rather
+than forwarded, regardless of whether the Connection host carries anything
+under the same name.
diff --git a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
index 080f245aa1..60adbf0a34 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
@@ -582,6 +582,39 @@ describe('prometheus router', () => {
       expect(requested.searchParams.get('timeout')).toBe('30s');
     });
 
+    // `stats` is `timeout`'s sibling in the allowlist -- covered separately
+    // so a regression dropping just `stats` (and not `timeout`) would still
+    // be caught.
+    it('forwards a request stats value, overriding one pinned on the connection host', async () => {
+      const { agent, team } = await getLoggedInAgent(server);
+      const conn = await seedPrometheusConnection(
+        team._id,
+        'http://prom.example.com?stats=all',
+      );
+
+      mockFetch.mockResolvedValueOnce(
+        fakeUpstreamResponse({
+          status: 'success',
+          data: { resultType: 'matrix', result: [] },
+        }),
+      );
+
+      await agent
+        .get('/v1/prometheus/query_range')
+        .query({
+          query: 'up',
+          start: '1700000000',
+          end: '1700000060',
+          step: '15s',
+          stats: 'none',
+          connectionId: conn._id.toString(),
+        })
+        .expect(200);
+
+      const requested = new URL(mockFetch.mock.calls[0][0] as string);
+      expect(requested.searchParams.get('stats')).toBe('none');
+    });
+
     // Unlike a real Prometheus API param, an arbitrary key the request
     // happens to also send (here VictoriaMetrics's own `extra_label`, which
     // a Connection host may pin as a tenant-isolation scope) must NOT be

From de4026b482becb554e06b968f21b4177b4cf1a2b Mon Sep 17 00:00:00 2001
From: milansanjeev <12941259+milansanjeev@users.noreply.github.com>
Date: Fri, 4 Sep 2026 08:53:57 +0530
Subject: [PATCH 13/13] style(api): fix prettier formatting in the integration
 test file

The only real lint failure on this PR -- a multi-line expect() call
that prettier wants collapsed to one line, plus the changeset's prose
wrap. No behavior change.
---
 .changeset/promql-proxy-preserve-host-path.md | 32 +++++++++----------
 .../api/__tests__/prometheus.int.test.ts      |  4 +--
 2 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/.changeset/promql-proxy-preserve-host-path.md b/.changeset/promql-proxy-preserve-host-path.md
index df056c114c..1d549c25e2 100644
--- a/.changeset/promql-proxy-preserve-host-path.md
+++ b/.changeset/promql-proxy-preserve-host-path.md
@@ -18,22 +18,20 @@ to work because the absolute API path replaced `/graph`; requests now go to
 Connection hosts before upgrading. Root-mounted hosts (`http://prom:9090` or
 `http://prom:9090/`) are unchanged.
 
-Query parameters on the Connection host are now only a fallback for a fixed
-set of real Prometheus API params (`query`, `time`, `start`, `end`, `step`,
+Query parameters on the Connection host are now only a fallback for a fixed set
+of real Prometheus API params (`query`, `time`, `start`, `end`, `step`,
 `match`/`match[]`, `limit`, `timeout`, `stats`): a request value for one of
-these (including repeatable ones such as `match[]`) always wins and replaces
-a same-named host value outright, rather than being dropped. Any other host
-query key the request never mentions -- for example
-`?extra_label=namespace%3Dprod` pinning a VictoriaMetrics tenant scope -- is
-left as-is and is never overridable by the request, since a param name
-outside that fixed set is not forwarded at all regardless of what the host
-carries. This also means a host copied with a stray query string (not just a
-stray path) now forwards its non-Prometheus keys upstream as a fallback on
-every request -- trim those too if they weren't intended as Prometheus API
-params.
+these (including repeatable ones such as `match[]`) always wins and replaces a
+same-named host value outright, rather than being dropped. Any other host query
+key the request never mentions -- for example `?extra_label=namespace%3Dprod`
+pinning a VictoriaMetrics tenant scope -- is left as-is and is never overridable
+by the request, since a param name outside that fixed set is not forwarded at
+all regardless of what the host carries. This also means a host copied with a
+stray query string (not just a stray path) now forwards its non-Prometheus keys
+upstream as a fallback on every request -- trim those too if they weren't
+intended as Prometheus API params.
 
-This is also a behavior change for a direct API caller (e.g. curl or
-Terraform) that previously relied on sending an arbitrary, non-Prometheus
-query param through this endpoint: that param is now silently dropped rather
-than forwarded, regardless of whether the Connection host carries anything
-under the same name.
+This is also a behavior change for a direct API caller (e.g. curl or Terraform)
+that previously relied on sending an arbitrary, non-Prometheus query param
+through this endpoint: that param is now silently dropped rather than forwarded,
+regardless of whether the Connection host carries anything under the same name.
diff --git a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
index 3da113beb6..8dc75637d6 100644
--- a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
+++ b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts
@@ -649,9 +649,7 @@ describe('prometheus router', () => {
         .expect(200);
 
       const requested = new URL(mockFetch.mock.calls[0][0] as string);
-      expect(requested.searchParams.get('extra_label')).toBe(
-        'namespace=prod',
-      );
+      expect(requested.searchParams.get('extra_label')).toBe('namespace=prod');
       expect(requested.searchParams.get('query')).toBe('up');
     });