From 56bf4fc2ff917e7ea327091837b5e4f7c1671b3d Mon Sep 17 00:00:00 2001
From: Ravjot Brar <83892020+ravjotbrar@users.noreply.github.com>
Date: Thu, 17 Sep 2026 15:33:23 -0700
Subject: [PATCH 1/3] Fix K8s sidecar registration and stale cluster topology
 broadcast (#512)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Fix K8s sidecar registration and stale cluster topology broadcast

Two coupled defects broke the Kubernetes metrics path and left the
Cluster Topology view stale:

1. Sidecar registration (regressed in #379): the register handler
   required a pre-existing metricsServerMap entry, but in K8s the
   orchestrator never spawns collectors, so externally-managed sidecars
   were always rejected (401/404). Registration is now allowed when the
   nodeId belongs to the discovered cluster topology, and the sidecar's
   entry is created on first register. Scoped to K8s so the strict
   no-entry gate is preserved for spawned (Web/Electron) collectors.

   Sidecars authenticate with a shared ORCHESTRATOR_KEY provisioned to
   both the orchestrator and every sidecar via a Kubernetes Secret;
   resolveCollectorKey falls back to that key only in K8s mode.

2. Stale topology broadcast (regressed in #279): refreshAllClusterRegistries
   broadcast the registry every 30s but no longer re-discovered topology,
   so clients kept the connect-time snapshot. It now re-discovers each
   tracked cluster through a live client belonging to that cluster before
   broadcasting.

Adds tests covering registry refresh on topology change and that
reconcile acts on the refreshed topology. Updates the K8s example
manifests (Secret + env wiring) and deployment docs.

Signed-off-by: ravjotb <ravjot.brar@improving.com>

* Reuse the per-cluster connection grouping in refreshAllClusterRegistries

Build connectionIdsByCluster once and use it both to pick a client for
topology re-discovery and to target the broadcast, instead of making a
second pass over `clients` for a separate clientByCluster map. The client
is reached via clients.get(connectionId).client, keyed by the same
connectionIds already grouped for the broadcast.

No behavior change.

Signed-off-by: ravjotb <ravjot.brar@improving.com>

* Don't ship a placeholder orchestrator Secret in app.yaml

app.yaml defined a Secret named valkey-admin-orchestrator-key with a
committed placeholder value. Applying it after creating a real random
Secret of the same name would overwrite the real key with the public
placeholder, leaving the orchestrator and every sidecar signing with a
known key (CWE-798). An attacker able to reach the orchestrator could
then register a known cluster node with an attacker-controlled metrics
URI.

Remove the Secret definition from app.yaml — the manifest now only
references the Secret by name. The key is created out-of-band with a
user-supplied random value (kubectl create secret ...), so apply can
never clobber it. Docs updated to state the Secret is created first and
is intentionally not defined in the manifest.

Signed-off-by: ravjotb <ravjot.brar@improving.com>

* Address review feedback on topology refresh and K8s registration

- Carry per-cluster connection metadata forward on refresh instead of
  reverting to initialConnectionDetails: updateClusterNodeRegistry now
  takes a required NodeInfo, and the refresh loop sources it from an
  existing node of that cluster. Narrow discoverCluster to NodeInfo and
  drop the redundant getClusterTopology wrapper.
- Re-discover preconfigured clusters (K8s sidecars, headless Web) through
  the initial client each cycle, since they have no entry in `clients`;
  otherwise a node scaled in after boot never registers until a UI
  session opens. Await the boot-time K8s discovery.
- Bound each cluster's re-discovery with a timeout so one hung CLUSTER
  SLOTS can't stall the whole broadcast loop.
- examples/k8s/valkey-statefulset.yaml: give the inline metrics sidecar
  the ORCHESTRATOR_KEY it needs, and document that the Secret must exist
  and the cluster must be formed before the sidecars start.
- Document that the shared orchestrator key is a cluster-scoped
  credential and that the K8s metrics host is not pinned to loopback.
- Rework the topology-refresh test to exercise real discovery via a fake
  CLUSTER SLOTS client; drop a tautological reconcile test.

Signed-off-by: ravjotb <ravjot.brar@improving.com>

* Overwrite the known cluster entry on refresh instead of orphaning it

updateClusterNodeRegistry keyed the registry on the clusterId derived
from CLUSTER SLOTS (the first slot range's primary). That id changes on
failover/resharding, so re-discovering an existing cluster could write a
new entry under a different id and leave the old one orphaned in the map.

Add an optional clusterId override: the refresh loop passes the cluster's
existing id so the entry is overwritten in place, while boot-time first
discovery still falls back to the derived id. Adds a test covering the
changed-derived-id case.

Signed-off-by: ravjotb <ravjot.brar@improving.com>

* Add coverage for the K8s register and topology-refresh fixes

The branch's new behavior was largely untested, and the K8s-gated paths
are unreachable from the default test suite because isKubernetes is a
module-load constant.

- orchestrator-k8s-register.test.ts sets DEPLOYMENT_MODE=K8 before import
  to cover the shared-key path: resolveCollectorKey falls back to
  ORCHESTRATOR_KEY (and still prefers a minted per-node key), and
  handleRegister admits a signed sidecar for a known cluster node with no
  pre-existing entry while rejecting unknown nodes and wrong-key signatures.
- Extract resolveClusterRefreshTarget (the refresh client/nodeInfo choice)
  so it is unit-testable: a user-connected cluster carries its own node
  metadata forward; a preconfigured/headless cluster falls back to the
  initial client + initialConnectionDetails. Covered in metrics-orchestrator
  (user path / skip) and topology-refresh-preconfigured (fallback path).
  getInitialClient is routed via internals so the fallback is mockable;
  no behavior change.

Signed-off-by: ravjotb <ravjot.brar@improving.com>

* Use the initial client only for the preconfigured cluster on refresh

resolveClusterRefreshTarget fell back to the initial client for any
client-less cluster when preConfiguredConnection was set. In a mixed
deployment, an inactive cluster could then be rediscovered through a
different preconfigured cluster's client, storing that topology under the
inactive cluster's id.

Track the preconfigured clusterId (recorded at boot from the Web and K8s
discovery paths; updateClusterNodeRegistry now returns the written id) and
only use the initial client when the cluster being refreshed matches it.
Other client-less clusters are left unchanged. A user-connected cluster
still refreshes through its own client, so the preconfigured cluster is
covered by that path when a session is open.

Signed-off-by: ravjotb <ravjot.brar@improving.com>

---------

Signed-off-by: ravjotb <ravjot.brar@improving.com>
(cherry picked from commit 9836fb49fd62282c342d6a22386ef61ef4045a13)
---
 .../__tests__/metrics-orchestrator.test.ts    | 107 +++++++++++++++-
 .../orchestrator-k8s-register.test.ts         | 114 ++++++++++++++++++
 .../topology-refresh-preconfigured.test.ts    |  70 +++++++++++
 apps/server/src/connection.ts                 |   5 +-
 apps/server/src/index.ts                      |  40 +++++-
 apps/server/src/metrics-orchestrator.ts       |  96 ++++++++++++---
 .../src/content/docs/deployment/kubernetes.md |  36 +++++-
 examples/k8s/app.yaml                         |  17 +++
 .../k8s/valkey-statefulset-sidecar-patch.yaml |   5 +
 examples/k8s/valkey-statefulset.yaml          |  20 +++
 10 files changed, 476 insertions(+), 34 deletions(-)
 create mode 100644 apps/server/src/__tests__/orchestrator-k8s-register.test.ts
 create mode 100644 apps/server/src/__tests__/topology-refresh-preconfigured.test.ts

diff --git a/apps/server/src/__tests__/metrics-orchestrator.test.ts b/apps/server/src/__tests__/metrics-orchestrator.test.ts
index a2b615ad..3cc6e4a0 100644
--- a/apps/server/src/__tests__/metrics-orchestrator.test.ts
+++ b/apps/server/src/__tests__/metrics-orchestrator.test.ts
@@ -6,6 +6,8 @@ import {
   metricsServerMap,
   stopAllMetricsServers,
   reconcileClusterMetricsServers,
+  updateClusterNodeRegistry,
+  resolveClusterRefreshTarget,
   clients,
   clusterNodesRegistry,
   __test__,
@@ -229,12 +231,6 @@ describe("metrics-orchestrator", () => {
 
       // Mock all side-effectful internal functions
       mock.method(__test__, "createClient", async () => ({}))
-      mock.method(__test__, "getClusterTopology", async () => ({
-        clusterNodes: {
-          node1: { host: "127.0.0.1", port: "6379", tls: false, verifyTlsCertificate: false },
-        },
-        clusterId: "cluster-1",
-      }))
       mock.method(__test__, "updateClusterNodeRegistry", async () => mockClusterNodesRegistry)
       mock.method(__test__, "updateMetricsServers", async () => {})
       mock.method(__test__, "findDiff", async () => ({ nodesToAdd: {}, nodesToRemove: [] }))
@@ -271,4 +267,103 @@ describe("metrics-orchestrator", () => {
       assert.strictEqual(updateMetricsServers.mock.callCount(), 0)
     })
   })
+
+  describe("topology refresh", () => {
+    beforeEach(() => {
+      mock.restoreAll()
+      clients.clear()
+      metricsServerMap.clear()
+      clusterNodesRegistry.clear()
+    })
+    afterEach(() => {
+      mock.restoreAll()
+      clients.clear()
+      metricsServerMap.clear()
+      clusterNodesRegistry.clear()
+    })
+
+    it("updateClusterNodeRegistry replaces stale topology with the freshly discovered one", async () => {
+      // Seed a stale registry: cluster "node-1" knows only one node.
+      clusterNodesRegistry.set("node-1", {
+        "192-168-1-1-6379": { host: "192.168.1.1", port: 6379, tls: false, verifyTlsCertificate: false },
+      })
+
+      // A live client for that cluster whose CLUSTER SLOTS now reports three
+      // primaries. (SLOTS parsing itself is covered in connection.test.ts.)
+      const client = {
+        customCommand: async (args: string[]) =>
+          args[0] === "CLUSTER" && args[1] === "SLOTS"
+            ? [
+              [0, 5460, ["192.168.1.1", 6379, "node-1"]],
+              [5461, 10922, ["192.168.1.3", 6379, "node-2"]],
+              [10923, 16383, ["192.168.1.4", 6379, "node-3"]],
+            ]
+            : [],
+      } as never
+
+      const sampleNode = Object.values(clusterNodesRegistry.get("node-1") ?? {})[0]
+      await updateClusterNodeRegistry(client, sampleNode)
+
+      assert.deepStrictEqual(
+        Object.keys(clusterNodesRegistry.get("node-1") ?? {}).sort(),
+        ["192-168-1-1-6379", "192-168-1-3-6379", "192-168-1-4-6379"],
+        "registry should reflect the newly discovered topology, not the stale snapshot",
+      )
+    })
+
+    it("overwrites the existing entry when a known clusterId is passed, even if the derived id changed", async () => {
+      // Existing cluster tracked under "orig-id".
+      clusterNodesRegistry.set("orig-id", {
+        "192-168-1-1-6379": { host: "192.168.1.1", port: 6379, tls: false, verifyTlsCertificate: false },
+      })
+
+      // After a failover, CLUSTER SLOTS now lists a different first primary, so the
+      // derived clusterId would be "new-first-primary" — which would orphan "orig-id".
+      const client = {
+        customCommand: async (args: string[]) =>
+          args[0] === "CLUSTER" && args[1] === "SLOTS"
+            ? [[0, 16383, ["192.168.1.9", 6379, "new-first-primary"]]]
+            : [],
+      } as never
+
+      const sampleNode = Object.values(clusterNodesRegistry.get("orig-id") ?? {})[0]
+      await updateClusterNodeRegistry(client, sampleNode, "orig-id")
+
+      assert.deepStrictEqual(
+        [...clusterNodesRegistry.keys()],
+        ["orig-id"],
+        "the known clusterId should be overwritten in place, leaving no orphaned entry",
+      )
+      assert.deepStrictEqual(
+        Object.keys(clusterNodesRegistry.get("orig-id") ?? {}),
+        ["192-168-1-9-6379"],
+        "the entry should hold the freshly discovered node",
+      )
+    })
+
+    it("resolveClusterRefreshTarget uses the user client and carries node metadata forward", async () => {
+      const userClient = { id: "user-client" } as never
+      const clusterNodes = {
+        node1: { host: "10.0.0.1", port: 6379, tls: true, verifyTlsCertificate: false, username: "admin", authType: "iam" as const },
+      }
+
+      const target = await resolveClusterRefreshTarget("cluster-1", clusterNodes, userClient)
+
+      assert.strictEqual(target?.client, userClient, "should refresh with the cluster's own live client")
+      // nodeInfo must be a node from THIS cluster (preserving its tls/username/auth),
+      // not initialConnectionDetails.
+      assert.deepStrictEqual(target?.nodeInfo, clusterNodes.node1, "should carry the cluster's own node metadata forward")
+    })
+
+    it("resolveClusterRefreshTarget skips a cluster with no live client when not preconfigured", async () => {
+      // DEPLOYMENT_MODE is unset in this file, so preConfiguredConnection is falsy
+      // and there is no initial client to fall back to.
+      const target = await resolveClusterRefreshTarget(
+        "cluster-1",
+        { node1: { host: "10.0.0.1", port: 6379, tls: false, verifyTlsCertificate: false } },
+        undefined,
+      )
+      assert.strictEqual(target, undefined, "a cluster with no live client and no preconfigured fallback is skipped")
+    })
+  })
 })
diff --git a/apps/server/src/__tests__/orchestrator-k8s-register.test.ts b/apps/server/src/__tests__/orchestrator-k8s-register.test.ts
new file mode 100644
index 00000000..3512fabf
--- /dev/null
+++ b/apps/server/src/__tests__/orchestrator-k8s-register.test.ts
@@ -0,0 +1,114 @@
+import { describe, it, afterEach } from "node:test"
+import assert from "node:assert"
+import {
+  ORCHESTRATOR_AUTH_DOMAIN,
+  ORCHESTRATOR_AUTH_HEADER,
+  createOrchestratorAuthCredential
+} from "valkey-common"
+import type { Request, Response } from "express"
+
+// These tests exercise the Kubernetes-only branches of the orchestrator:
+//   - resolveCollectorKey falls back to the shared ORCHESTRATOR_KEY, and
+//   - handleRegister admits a sidecar whose nodeId is part of the discovered
+//     cluster topology even though the orchestrator never spawned it.
+// isKubernetes is a module-load constant, so DEPLOYMENT_MODE and the shared key
+// must be set BEFORE importing the module.
+process.env.DEPLOYMENT_MODE = "K8"
+process.env.ORCHESTRATOR_KEY = "k8s-shared-secret"
+
+const {
+  metricsServerMap,
+  clusterNodesRegistry,
+  resolveCollectorKey,
+  __test__,
+} = await import("../metrics-orchestrator")
+
+const NODE_ID = "valkey-6-valkey-headless-valkey-svc-cluster-local-6379"
+const URI = "http://10.42.0.32:3000"
+const SHARED_KEY = "k8s-shared-secret"
+
+const makeRes = () => {
+  const captured: { statusCode: number; body?: unknown } = { statusCode: 200 }
+  const res = {
+    status(code: number) { captured.statusCode = code; return res },
+    send(body?: unknown) { captured.body = body; return res },
+    sendStatus(code: number) { captured.statusCode = code; return res },
+  }
+  return { res: res as unknown as Response, captured }
+}
+
+const makeReq = (body: unknown, credential?: string) => ({
+  body,
+  headers: credential === undefined ? {} : { [ORCHESTRATOR_AUTH_HEADER]: credential },
+}) as unknown as Request
+
+const signRegister = (fields: { nodeId?: string; metricsServerUri?: string; timestamp?: number } = {}, key = SHARED_KEY) =>
+  createOrchestratorAuthCredential(key, ORCHESTRATOR_AUTH_DOMAIN.REGISTER, {
+    nodeId: NODE_ID,
+    metricsServerUri: URI,
+    timestamp: Date.now(),
+    ...fields,
+  }) as string
+
+// Register a node into the discovered topology so it is a known cluster member.
+const seedTopology = (nodeId = NODE_ID) => {
+  clusterNodesRegistry.set("cluster-1", {
+    [nodeId]: { host: "10.42.0.32", port: 6379, tls: false, verifyTlsCertificate: false },
+  })
+}
+
+describe("K8s shared-key registration", () => {
+  afterEach(() => {
+    metricsServerMap.clear()
+    clusterNodesRegistry.clear()
+    __test__.collectorKeys.clear()
+  })
+
+  describe("resolveCollectorKey", () => {
+    it("falls back to the shared ORCHESTRATOR_KEY for a node the orchestrator never spawned", () => {
+      assert.strictEqual(resolveCollectorKey("never-spawned-node"), SHARED_KEY)
+    })
+
+    it("still prefers a per-node minted key when one exists", () => {
+      __test__.collectorKeys.set("spawned-node", "per-node-key")
+      assert.strictEqual(resolveCollectorKey("spawned-node"), "per-node-key")
+    })
+  })
+
+  describe("handleRegister topology-membership gate", () => {
+    it("admits a correctly signed sidecar for a known cluster node with no pre-existing entry", () => {
+      seedTopology()
+      assert.strictEqual(metricsServerMap.has(NODE_ID), false, "precondition: no orchestrator-spawned entry")
+
+      const { res, captured } = makeRes()
+      __test__.handleRegister(makeReq({ nodeId: NODE_ID, metricsServerUri: URI, timestamp: Date.now() }, signRegister()), res)
+
+      assert.strictEqual(captured.statusCode, 200, "a known cluster node should be allowed to register")
+      assert.strictEqual(metricsServerMap.get(NODE_ID)?.metricsURI, URI, "the sidecar's entry should be created on first register")
+    })
+
+    it("rejects a correctly signed sidecar whose nodeId is not in the discovered topology", () => {
+      // Topology known, but for a different node.
+      seedTopology("some-other-node-6379")
+
+      const { res, captured } = makeRes()
+      __test__.handleRegister(makeReq({ nodeId: NODE_ID, metricsServerUri: URI, timestamp: Date.now() }, signRegister()), res)
+
+      assert.strictEqual(captured.statusCode, 401, "an unknown node must be rejected even with a valid signature")
+      assert.strictEqual(metricsServerMap.has(NODE_ID), false)
+    })
+
+    it("rejects a known cluster node when the signature uses the wrong key", () => {
+      seedTopology()
+
+      const { res, captured } = makeRes()
+      __test__.handleRegister(
+        makeReq({ nodeId: NODE_ID, metricsServerUri: URI, timestamp: Date.now() }, signRegister({}, "wrong-key")),
+        res,
+      )
+
+      assert.strictEqual(captured.statusCode, 401, "a bad signature must be rejected before the membership check")
+      assert.strictEqual(metricsServerMap.has(NODE_ID), false)
+    })
+  })
+})
diff --git a/apps/server/src/__tests__/topology-refresh-preconfigured.test.ts b/apps/server/src/__tests__/topology-refresh-preconfigured.test.ts
new file mode 100644
index 00000000..83ee5fa0
--- /dev/null
+++ b/apps/server/src/__tests__/topology-refresh-preconfigured.test.ts
@@ -0,0 +1,70 @@
+import { describe, it, afterEach, mock } from "node:test"
+import assert from "node:assert"
+
+// preConfiguredConnection is a module-load constant derived from VALKEY_HOST /
+// VALKEY_PORT, so it must be set BEFORE importing the module to exercise the
+// headless-refresh fallback (no live user client → initial client).
+process.env.VALKEY_HOST = "valkey-0.example"
+process.env.VALKEY_PORT = "6379"
+
+const {
+  resolveClusterRefreshTarget,
+  setPreconfiguredClusterId,
+  initialConnectionDetails,
+  __test__,
+} = await import("../metrics-orchestrator")
+
+const PRECONFIGURED_ID = "preconfigured-cluster"
+
+describe("resolveClusterRefreshTarget (preconfigured / headless)", () => {
+  afterEach(() => {
+    mock.restoreAll()
+    setPreconfiguredClusterId(undefined)
+  })
+
+  it("refreshes the preconfigured cluster via the initial client + initialConnectionDetails", async () => {
+    setPreconfiguredClusterId(PRECONFIGURED_ID)
+    const initialClient = { id: "initial-client" } as never
+    mock.method(__test__, "getInitialClient", async () => initialClient)
+
+    const target = await resolveClusterRefreshTarget(
+      PRECONFIGURED_ID,
+      { node1: { host: "10.0.0.1", port: 6379, tls: false, verifyTlsCertificate: false } },
+      undefined,
+    )
+
+    assert.strictEqual(target?.client, initialClient, "the preconfigured cluster should refresh via the initial client")
+    assert.strictEqual(
+      target?.nodeInfo,
+      initialConnectionDetails,
+      "the preconfigured cluster decorates with initialConnectionDetails",
+    )
+  })
+
+  it("does NOT use the initial client for a different client-less cluster", async () => {
+    // Preconfigured cluster is A; refreshing a stale, client-less cluster B must
+    // not rediscover A's topology through the initial client.
+    setPreconfiguredClusterId(PRECONFIGURED_ID)
+    const getInitial = mock.method(__test__, "getInitialClient", async () => ({ id: "initial-client" } as never))
+
+    const target = await resolveClusterRefreshTarget(
+      "some-other-cluster",
+      { node1: { host: "10.9.9.9", port: 6379, tls: false, verifyTlsCertificate: false } },
+      undefined,
+    )
+
+    assert.strictEqual(target, undefined, "an unrelated client-less cluster must be left unchanged")
+    assert.strictEqual(getInitial.mock.callCount(), 0, "the initial client must not be consulted for other clusters")
+  })
+
+  it("still prefers a user client over the initial client when one is present", async () => {
+    const userClient = { id: "user-client" } as never
+    mock.method(__test__, "getInitialClient", async () => ({ id: "initial-client" } as never))
+
+    const clusterNodes = { node1: { host: "10.0.0.1", port: 6379, tls: true, verifyTlsCertificate: false } }
+    const target = await resolveClusterRefreshTarget("any-cluster", clusterNodes, userClient)
+
+    assert.strictEqual(target?.client, userClient)
+    assert.deepStrictEqual(target?.nodeInfo, clusterNodes.node1)
+  })
+})
diff --git a/apps/server/src/connection.ts b/apps/server/src/connection.ts
index 191853e9..7587490d 100644
--- a/apps/server/src/connection.ts
+++ b/apps/server/src/connection.ts
@@ -22,7 +22,8 @@ import {
   reconcileClusterMetricsServers, 
   isKubernetes, 
   forgetCollectorKey,
-  ClusterNodeMap } from "./metrics-orchestrator"
+  ClusterNodeMap,
+  type NodeInfo } from "./metrics-orchestrator"
 import { subscribe } from "./node-watchers"
 import { clearCpuSamples } from "./node-utilization"
 import { createClusterValkeyClient, createStandaloneValkeyClient } from "./valkey-client"
@@ -595,7 +596,7 @@ function sendStandaloneConnectFulfilled(ws: WebSocket, payload: StandaloneConnec
 
 export async function discoverCluster(
   client: GlideClient | GlideClusterClient, 
-  payload: { connectionDetails: ConnectionDetails, connectionId?: string;},
+  payload: { connectionDetails: NodeInfo, connectionId?: string;},
 )  {
   try {
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
diff --git a/apps/server/src/index.ts b/apps/server/src/index.ts
index b12006ae..149cdb9f 100644
--- a/apps/server/src/index.ts
+++ b/apps/server/src/index.ts
@@ -47,7 +47,9 @@ import {
   isKubernetes,
   preConfiguredConnection,
   getInitialClient,
-  updateClusterNodeRegistry
+  updateClusterNodeRegistry,
+  resolveClusterRefreshTarget,
+  setPreconfiguredClusterId
 } from "./metrics-orchestrator"
 import { isAllowedWebSocketOrigin } from "./websocket-origin"
 import { ensureSession, hasAuthorizedSession, isConnectionAuthorized, setSessionExpiryListener } from "./session"
@@ -143,7 +145,14 @@ const wss = new WebSocketServer({ noServer: true })
 
 const delay = (ms: number) => new Promise((res) => setTimeout(res, ms))
 
-function refreshAllClusterRegistries() {
+// Upper bound on a single cluster's topology re-discovery. Without it, one hung
+// CLUSTER SLOTS would leave the awaited refresh unsettled and stall the broadcast
+// for every cluster, not just the unresponsive one.
+const TOPOLOGY_REDISCOVERY_TIMEOUT_MS = 10_000
+
+async function refreshAllClusterRegistries() {
+  // Group live connections by cluster once; used both to pick a client to
+  // re-discover each cluster's topology and to target the broadcast below.
   const connectionIdsByCluster = new Map<string, string[]>()
   for (const [connectionId, entry] of clients) {
     if (!entry.clusterId) continue
@@ -152,6 +161,28 @@ function refreshAllClusterRegistries() {
     connectionIdsByCluster.set(entry.clusterId, ids)
   }
 
+  // Re-discover each tracked cluster before broadcasting. User-connected clusters
+  // use their own client + node metadata; a preconfigured cluster (K8s / headless
+  // Web) has no entry in `clients`, so it falls back to the initial client so
+  // scaled-in nodes are picked up without a UI session. Time-bounded so one hung
+  // node can't stall the whole loop.
+  await Promise.all(
+    [...clusterNodesRegistry.entries()].map(async ([clusterId, clusterNodes]) => {
+      const connectionId = connectionIdsByCluster.get(clusterId)?.[0]
+      const userClient = connectionId ? clients.get(connectionId)?.client : undefined
+
+      const target = await resolveClusterRefreshTarget(clusterId, clusterNodes, userClient)
+      if (!target) return
+
+      await Promise.race([
+        updateClusterNodeRegistry(target.client, target.nodeInfo, clusterId),
+        delay(TOPOLOGY_REDISCOVERY_TIMEOUT_MS).then(() =>
+          console.warn(`Topology re-discovery for cluster ${clusterId} timed out; broadcasting last known nodes.`),
+        ),
+      ])
+    }),
+  )
+
   for (const [clusterId, clusterNodes] of clusterNodesRegistry) {
     const connectionIds = connectionIdsByCluster.get(clusterId)
     if (!connectionIds) continue
@@ -172,7 +203,7 @@ function refreshAllClusterRegistries() {
 async function refreshAllClusterRegistriesLoop() {
   while (true) {
     try {
-      refreshAllClusterRegistries()
+      await refreshAllClusterRegistries()
     } catch (err) {
       console.warn("Unable to refresh cluster topologies. ", err)
     } finally {
@@ -184,7 +215,8 @@ async function refreshAllClusterRegistriesLoop() {
 
 async function updateRegistryforK8() {
   const client = await getInitialClient()
-  updateClusterNodeRegistry(client, initialConnectionDetails)
+  const clusterId = await updateClusterNodeRegistry(client, initialConnectionDetails)
+  setPreconfiguredClusterId(clusterId)
 }
 
 // Electron: bind to localhost only — Origin headers are forgeable by non-browser clients,
diff --git a/apps/server/src/metrics-orchestrator.ts b/apps/server/src/metrics-orchestrator.ts
index 4d88a414..b71792cb 100644
--- a/apps/server/src/metrics-orchestrator.ts
+++ b/apps/server/src/metrics-orchestrator.ts
@@ -43,7 +43,7 @@ export type MetricsServerMap = Map<string,
   }
 >
 
-type NodeInfo = {
+export type NodeInfo = {
   host: string;
   port: number | string;
   username?: string;
@@ -64,6 +64,11 @@ export const clusterNodesRegistry: Map<string, ClusterNodeMap> = new Map()
 
 export const clusterCredentials: Map<string, string | undefined> = new Map()
 
+let preconfiguredClusterId: string | undefined
+export function setPreconfiguredClusterId(clusterId: string | undefined) {
+  preconfiguredClusterId = clusterId
+}
+
 export const metricsServerMap: MetricsServerMap = new Map()
 
 /**
@@ -79,7 +84,14 @@ const collectorKeys: Map<string, string> = new Map()
  * replayed against another.
  */
 export function resolveCollectorKey(nodeId: string): string | undefined {
-  return collectorKeys.get(nodeId)
+  const spawnedKey = collectorKeys.get(nodeId)
+  if (spawnedKey) return spawnedKey
+  // K8s collectors are external sidecars the orchestrator never spawns, so there
+  // is no per-node minted key. They authenticate with a shared key provisioned to
+  // both the orchestrator and every sidecar via a Kubernetes Secret. Scoped to K8s
+  // so the per-node property is preserved for spawned (Web/Electron) collectors.
+  if (isKubernetes) return process.env[ORCHESTRATOR_AUTH_KEY_ENV]
+  return undefined
 }
 
 /**
@@ -225,6 +237,18 @@ function isPinnedMetricsHost(uri: string): boolean {
   }
 }
 
+/**
+ * True when `nodeId` is part of a discovered cluster topology. In K8s the
+ * orchestrator only tracks topology (it never spawns collectors), so this is
+ * how a sidecar's registration is authorized as belonging to the cluster.
+ */
+function isKnownClusterNode(nodeId: string): boolean {
+  for (const clusterNodes of clusterNodesRegistry.values()) {
+    if (flattenClusterNodeMap(clusterNodes)[nodeId]) return true
+  }
+  return false
+}
+
 /**
  * `POST /orchestrator/register` — a collector advertising where it can be
  * reached.
@@ -269,14 +293,22 @@ function handleRegister(req: Request, res: Response): void {
     return
   }
 
-  const entry = metricsServerMap.get(nodeId)
-  if (!entry) {
-    // Key material without an entry means the spawn did not complete.
+  // Allowed if we already have an entry (orchestrator-spawned) or, in K8s,
+  // the node is part of the discovered cluster (sidecars are external, so
+  // they create their entry on first register).
+  const allowed = metricsServerMap.has(nodeId) || (isKubernetes && isKnownClusterNode(nodeId))
+  if (!allowed) {
     console.warn(`Rejected metrics registration for ${nodeId}: no metrics server entry`)
     res.status(401).send("Unauthorized")
     return
   }
 
+  let entry = metricsServerMap.get(nodeId)
+  if (!entry) {
+    entry = { metricsURI: "", pid: undefined, lastSeen: Date.now() }
+    metricsServerMap.set(nodeId, entry)
+  }
+
   entry.metricsURI = metricsServerUri
   entry.lastSeen = Date.now()
   console.log(`Metrics server registered for ${nodeId} at ${metricsServerUri}`)
@@ -359,18 +391,21 @@ async function createClient(connectionDetails: ConnectionDetails) {
   return await createOrchestratorValkeyClient({ addresses, credentials, useTLS: tls, verifyTlsCertificate, databaseId: db })
 }
 
-async function getClusterTopology(client: GlideClusterClient | GlideClient | null, node: ConnectionDetails) {
-  if (!client) client = await createClient(node)
-
-  const { discoveredClusterNodes, clusterId } = await discoverCluster(client, { connectionDetails: node })
-
-  return { discoveredClusterNodes, clusterId }
-}
-
-export async function updateClusterNodeRegistry(client: GlideClusterClient | GlideClient | null, connectionDetails = initialConnectionDetails) {
+export async function updateClusterNodeRegistry(
+  client: GlideClusterClient | GlideClient,
+  nodeInfo: NodeInfo,
+  clusterId?: string,
+) {
   try {
-    const { discoveredClusterNodes, clusterId } = await getClusterTopology(client, connectionDetails)
-    if (clusterId && discoveredClusterNodes) clusterNodesRegistry.set(clusterId, discoveredClusterNodes)
+    const { discoveredClusterNodes, clusterId: discoveredClusterId } = await discoverCluster(client, { connectionDetails: nodeInfo })
+    // Prefer the caller's known clusterId when refreshing an existing cluster: the
+    // discovered id is derived from the first primary in CLUSTER SLOTS, which can
+    // change on failover/resharding and would otherwise orphan the old entry.
+    const key = clusterId ?? discoveredClusterId
+    if (key && discoveredClusterNodes) {
+      clusterNodesRegistry.set(key, discoveredClusterNodes)
+      return key
+    }
   }
   catch (err) {
     if (err instanceof ConnectionError) {
@@ -378,7 +413,29 @@ export async function updateClusterNodeRegistry(client: GlideClusterClient | Gli
     }
     console.error(err)
   }
-  return clusterNodesRegistry
+  return undefined
+}
+
+/**
+ * Decide how to refresh one tracked cluster. A user-connected cluster is refreshed
+ * with its own client and metadata carried forward from an existing node (so a
+ * refresh doesn't revert TLS/auth to server defaults). A preconfigured cluster
+ * (K8s / headless Web) has no live user client, so it falls back to the initial
+ * client with initialConnectionDetails. Returns undefined when the cluster can't
+ * be refreshed (no client available).
+ */
+export async function resolveClusterRefreshTarget(
+  clusterId: string,
+  clusterNodes: ClusterNodeMap,
+  userClient: GlideClusterClient | GlideClient | undefined,
+): Promise<{ client: GlideClusterClient | GlideClient; nodeInfo: NodeInfo } | undefined> {
+  if (userClient) return { client: userClient, nodeInfo: Object.values(clusterNodes)[0] }
+
+  // Only the preconfigured cluster may be refreshed via the initial client.
+  if (preConfiguredConnection && clusterId === preconfiguredClusterId) {
+    return { client: await internals.getInitialClient(), nodeInfo: initialConnectionDetails }
+  }
+  return undefined
 }
 
 async function findDiff(metricsServerMap: MetricsServerMap, clusterNodeMap: ClusterNodeMap) {
@@ -582,9 +639,10 @@ export async function startPreconfiguredMetricsServers() {
   const client = await getInitialClient()
   if (await belongsToCluster(client)) {
     if (isWebMode) {
-      const { discoveredClusterNodes, clusterId } = await internals.getClusterTopology(client, initialConnectionDetails)
+      const { discoveredClusterNodes, clusterId } = await discoverCluster(client, { connectionDetails: initialConnectionDetails })
       if (clusterId && discoveredClusterNodes) {
         clusterNodesRegistry.set(clusterId, discoveredClusterNodes)
+        setPreconfiguredClusterId(clusterId)
         if (!clusterCredentials.has(clusterId)) clusterCredentials.set(clusterId, initialConnectionDetails.password)
       }
       runReconcileLoop()
@@ -616,7 +674,7 @@ export function cleanupOrchestratorResources() {
 const internals =  {
   startMetricsServers,
   createClient,
-  getClusterTopology,
+  getInitialClient,
   updateClusterNodeRegistry,
   findDiff,
   flattenClusterNodeMap,
diff --git a/docs-site/src/content/docs/deployment/kubernetes.md b/docs-site/src/content/docs/deployment/kubernetes.md
index 74444ca5..4e503e90 100644
--- a/docs-site/src/content/docs/deployment/kubernetes.md
+++ b/docs-site/src/content/docs/deployment/kubernetes.md
@@ -55,6 +55,21 @@ For a non-local deployment, use published images from a container registry and u
 
 ### 3. Deploy the app server
 
+The metrics sidecars authenticate to the orchestrator's `/orchestrator/register`
+and `/orchestrator/ping` endpoints with a shared key. Because sidecars are external
+(the orchestrator does not spawn them in Kubernetes), the same key must be
+provisioned to both the app server and every sidecar via a Secret. Create it first
+with your own random value:
+
+```bash
+kubectl create secret generic valkey-admin-orchestrator-key -n valkey \
+  --from-literal=ORCHESTRATOR_KEY="$(openssl rand -hex 32)"
+```
+
+`app.yaml` intentionally does **not** define this Secret — it only references it —
+so `kubectl apply` can never overwrite your generated key with a committed
+placeholder. Create the Secret first (above), then deploy the app server:
+
 ```bash
 kubectl apply -f examples/k8s/app.yaml
 kubectl rollout status deployment/valkey-admin-app -n valkey
@@ -181,15 +196,30 @@ kubectl logs -n valkey valkey-0 -c metrics
 
 You want to see `Register success` in the metrics sidecar log.
 
-:::caution[Sidecar registration is not yet functional]
-Sidecar registration does not currently succeed in `DEPLOYMENT_MODE=K8`. The orchestrator only tracks collectors it spawned itself, and it does not spawn sidecars, so a sidecar's registration is rejected and the sidecar exits after 30 attempts:
+:::caution[Sidecar registration requires the shared key]
+Registration and ping are authenticated. Each sidecar signs its requests with the
+shared `ORCHESTRATOR_KEY`, and the orchestrator verifies against the same key, so
+the Secret from step 3 must be present on **both** the app Deployment and the
+sidecar. If the key is missing or does not match, the sidecar exits after 30
+attempts:
 
 ```text
 Register failed: 401 Unauthorized
 Failed to register with server after 30 attempts. Shutting down.
 ```
 
-Support  is in progress. Until then the Kubernetes sidecar topology on this page will deploy but will not populate metrics panels. Use the [Docker deployment](/deployment/docker/) for a working metrics path.
+If you see this, confirm the `valkey-admin-orchestrator-key` Secret exists in the
+`valkey` namespace and that both pods reference it (`kubectl get pod ... -o yaml |
+grep -A3 ORCHESTRATOR_KEY`).
+
+The key is shared cluster-wide rather than per-node, so it authenticates a sidecar
+as *a* member of the cluster, not as one specific node. Treat it as a
+cluster-scoped credential: restrict read access on the Secret, and rotate it by
+recreating the Secret and restarting the app Deployment and the sidecars. Note that
+in `DEPLOYMENT_MODE=K8` the registered metrics host is not pinned to loopback, so
+any holder of the key can point a node's metrics URI at an arbitrary host that the
+server will then fetch from — keep the key tightly scoped and the namespace's
+network egress constrained accordingly.
 :::
 
 ### Charts Empty in the UI
diff --git a/examples/k8s/app.yaml b/examples/k8s/app.yaml
index f06dc692..a3718145 100644
--- a/examples/k8s/app.yaml
+++ b/examples/k8s/app.yaml
@@ -1,3 +1,15 @@
+# NOTE: the metrics control plane (/orchestrator/register, /orchestrator/ping)
+# authenticates sidecars with a shared key read from the ORCHESTRATOR_KEY env
+# var below, sourced from the `valkey-admin-orchestrator-key` Secret. Create that
+# Secret with your OWN random value before applying this manifest — it is
+# intentionally NOT defined here so `kubectl apply` can never overwrite a real
+# key with a committed placeholder:
+#
+#   kubectl create secret generic valkey-admin-orchestrator-key -n valkey \
+#     --from-literal=ORCHESTRATOR_KEY="$(openssl rand -hex 32)"
+#
+# The same Secret is referenced by every metrics sidecar
+# (see valkey-statefulset-sidecar-patch.yaml).
 apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -28,6 +40,11 @@ spec:
               value: valkey-0.valkey-headless.valkey.svc.cluster.local
             - name: VALKEY_PORT
               value: "6379"
+            - name: ORCHESTRATOR_KEY
+              valueFrom:
+                secretKeyRef:
+                  name: valkey-admin-orchestrator-key
+                  key: ORCHESTRATOR_KEY
           ports:
             - containerPort: 8080
               name: http
diff --git a/examples/k8s/valkey-statefulset-sidecar-patch.yaml b/examples/k8s/valkey-statefulset-sidecar-patch.yaml
index 6d84ae4b..eb7428d4 100644
--- a/examples/k8s/valkey-statefulset-sidecar-patch.yaml
+++ b/examples/k8s/valkey-statefulset-sidecar-patch.yaml
@@ -36,6 +36,11 @@ spec:
                   fieldPath: status.podIP
             - name: METRICS_ADVERTISE_PORT
               value: "3000"
+            - name: ORCHESTRATOR_KEY
+              valueFrom:
+                secretKeyRef:
+                  name: valkey-admin-orchestrator-key
+                  key: ORCHESTRATOR_KEY
           ports:
             - containerPort: 3000
               name: metrics
diff --git a/examples/k8s/valkey-statefulset.yaml b/examples/k8s/valkey-statefulset.yaml
index a5ddc47d..e188bc6d 100644
--- a/examples/k8s/valkey-statefulset.yaml
+++ b/examples/k8s/valkey-statefulset.yaml
@@ -1,3 +1,18 @@
+# Example Valkey cluster StatefulSet with an inline metrics sidecar.
+#
+# PREREQUISITES — the metrics sidecars will crash-loop (and, because StatefulSet
+# rollout is ordered, block the whole rollout at valkey-0) unless both are met:
+#
+#   1. Create the shared orchestrator key Secret first (the sidecar exits with
+#      "Missing ORCHESTRATOR_KEY ... Shutting down" without it):
+#        kubectl create secret generic valkey-admin-orchestrator-key -n valkey \
+#          --from-literal=ORCHESTRATOR_KEY="$(openssl rand -hex 32)"
+#
+#   2. Form the cluster after the pods are up. These nodes start with
+#      --cluster-enabled yes but do NOT self-form, so CLUSTER SLOTS is empty and
+#      the sidecar dies on "Cluster(No topology views found)". Assign slots once:
+#        valkey-cli --cluster create <pod IPs>:6379 --cluster-replicas <n>
+#
 apiVersion: v1
 kind: Service
 metadata:
@@ -107,6 +122,11 @@ spec:
                   fieldPath: status.podIP
             - name: METRICS_ADVERTISE_PORT
               value: "3000"
+            - name: ORCHESTRATOR_KEY
+              valueFrom:
+                secretKeyRef:
+                  name: valkey-admin-orchestrator-key
+                  key: ORCHESTRATOR_KEY
           ports:
             - containerPort: 3000
               name: metrics

From 3544791013336f29c356a5b4f681ece1d85b4c86 Mon Sep 17 00:00:00 2001
From: Ravjot Brar <83892020+ravjotbrar@users.noreply.github.com>
Date: Fri, 18 Sep 2026 12:42:12 -0700
Subject: [PATCH 2/3] Default the server bind to loopback for Web mode (#521)

Web mode has no authentication, yet the server bound to all interfaces
(0.0.0.0) by default, silently exposing an unauthenticated admin tool to
the network on any host/VM deployment.

Introduce SERVER_BIND_HOST (mirroring the existing METRICS_BIND_HOST
convention). It defaults to 127.0.0.1, so external exposure is now an
explicit operator decision:
- index.ts binds SERVER_BIND_HOST ?? (K8 ? 0.0.0.0 : 127.0.0.1)
- Dockerfile.app sets SERVER_BIND_HOST=0.0.0.0 so published ports work;
  docs show binding the published port to loopback to keep it local
- examples/k8s/app.yaml sets it explicitly
- Docker and AWS ElastiCache quick-starts warn that publishing 8080
  exposes an unauthenticated service
- documented SERVER_BIND_HOST in the server configuration reference

Also flips the Docker image build to attach provenance + SBOM
attestations (provenance: true, sbom: true) in docker-build.yml.

Signed-off-by: ravjotb <ravjot.brar@improving.com>
(cherry picked from commit 80c442e28f587532ee4ce1b8d9d02cba4dee27ed)
---
 .github/workflows/docker-build.yml                   |  3 ++-
 apps/server/src/index.ts                             | 11 ++++++-----
 docker/Dockerfile.app                                |  4 ++++
 docs-site/src/content/docs/configuration/server.md   | 12 ++++++++++++
 .../src/content/docs/deployment/aws-elasticache.md   |  9 +++++++++
 docs-site/src/content/docs/deployment/docker.md      | 12 ++++++++++++
 examples/k8s/app.yaml                                |  3 +++
 7 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml
index 0460c8bc..125b0e41 100644
--- a/.github/workflows/docker-build.yml
+++ b/.github/workflows/docker-build.yml
@@ -112,7 +112,8 @@ jobs:
           platforms: linux/amd64,linux/arm64
           cache-from: type=gha
           cache-to: type=gha,mode=max
-          provenance: false
+          provenance: true
+          sbom: true
 
       - name: Update Docker Hub description
         if: env.PUBLISH_DOCKERHUB == 'true'
diff --git a/apps/server/src/index.ts b/apps/server/src/index.ts
index 149cdb9f..8fae6514 100644
--- a/apps/server/src/index.ts
+++ b/apps/server/src/index.ts
@@ -34,7 +34,6 @@ import { memoryUsageRequested } from "./actions/memoryUsage"
 import { monitorRequested } from "./actions/monitorAction"
 import { unsubscribeAll, getWatcherCount } from "./node-watchers"
 import { teardownConnection } from "./connection"
-import { isElectron } from "./metrics-orchestrator"
 import { Handler, ReduxAction, safeSend, unknownHandler, type WsActionMessage } from "./actions/utils"
 import {
   createMetricsOrchestratorRouter,
@@ -219,10 +218,12 @@ async function updateRegistryforK8() {
   setPreconfiguredClusterId(clusterId)
 }
 
-// Electron: bind to localhost only — Origin headers are forgeable by non-browser clients,
-// so network-level isolation is the only reliable gate for a desktop app.
-server.listen(port, isElectron ? "127.0.0.1" : undefined, () => {
-  console.log(`Server running at http://localhost:${port}`)
+// Default to loopback so an unauthenticated Web server isn't reachable off-host;
+// containers (Docker/K8s) set SERVER_BIND_HOST=0.0.0.0 explicitly.
+const bindHost =
+  process.env.SERVER_BIND_HOST ?? (isKubernetes ? "0.0.0.0" : "127.0.0.1")
+server.listen(port, bindHost, () => {
+  console.log(`Server running at http://${bindHost}:${port}`)
   if (process.send) { // Check if process.send is available (i.e., if forked)
     process.send({ type: "websocket-ready" }) // Send a ready message to the parent process
   }
diff --git a/docker/Dockerfile.app b/docker/Dockerfile.app
index 101e6959..ffeccc74 100644
--- a/docker/Dockerfile.app
+++ b/docker/Dockerfile.app
@@ -23,6 +23,10 @@ WORKDIR /app
 
 ENV NODE_ENV=production
 ENV DEPLOYMENT_MODE=Web
+# Bind all interfaces inside the container so published ports (-p) are reachable.
+# The server itself defaults to loopback off-container; this is the explicit
+# container opt-in to external exposure.
+ENV SERVER_BIND_HOST=0.0.0.0
 
 # Copy root workspace files
 COPY package.json package-lock.json ./
diff --git a/docs-site/src/content/docs/configuration/server.md b/docs-site/src/content/docs/configuration/server.md
index b03aa830..c91384f2 100644
--- a/docs-site/src/content/docs/configuration/server.md
+++ b/docs-site/src/content/docs/configuration/server.md
@@ -38,6 +38,18 @@ The TCP port the Express + WebSocket server listens on. The same port serves the
 PORT=9090
 ```
 
+### `SERVER_BIND_HOST`
+
+Network interface the server binds to. Defaults to loopback (`127.0.0.1`) so an unauthenticated Web server is not reachable off-host unless you opt in. Web mode has no authentication, so only bind a routable interface when access is otherwise restricted (authenticating proxy, security groups, etc.).
+
+- **Default:** `127.0.0.1` (loopback); `0.0.0.0` when `DEPLOYMENT_MODE=K8`
+- **Read in:** `apps/server/src/index.ts`
+- **Note:** The Docker image sets `SERVER_BIND_HOST=0.0.0.0` so published ports (`-p`) are reachable; bind the published port to loopback (`-p 127.0.0.1:8080:8080`) to keep it local.
+
+```bash
+SERVER_BIND_HOST=0.0.0.0
+```
+
 ## Mode & Orchestrator
 
 ### `DEPLOYMENT_MODE`
diff --git a/docs-site/src/content/docs/deployment/aws-elasticache.md b/docs-site/src/content/docs/deployment/aws-elasticache.md
index 080b9d78..3bc1886c 100644
--- a/docs-site/src/content/docs/deployment/aws-elasticache.md
+++ b/docs-site/src/content/docs/deployment/aws-elasticache.md
@@ -19,6 +19,15 @@ docker run -d --name valkey-admin \
 
 Open `http://<your-instance-ip>:8080` and add a connection to your ElastiCache endpoint through the UI.
 
+:::caution[This publishes an unauthenticated service on a routable interface]
+Web mode has **no authentication**, and `-p 8080:8080` publishes it on all
+interfaces — anyone who can reach `<your-instance-ip>:8080` can access your
+ElastiCache cluster through it. For anything beyond a throwaway test, use the
+**Production Deployment** below (HTTPS + Cognito), keep the port bound to loopback
+(`-p 127.0.0.1:8080:8080`), and restrict access with security groups. Treat
+external exposure as a deliberate decision.
+:::
+
 For IAM authentication, ensure the instance's IAM role has `elasticache:Connect` permission scoped to your replication group and user.
 
 ---
diff --git a/docs-site/src/content/docs/deployment/docker.md b/docs-site/src/content/docs/deployment/docker.md
index 7f80469c..685cdd5a 100644
--- a/docs-site/src/content/docs/deployment/docker.md
+++ b/docs-site/src/content/docs/deployment/docker.md
@@ -42,6 +42,18 @@ docker compose up -d
 
 Open `http://localhost:8080` and add a connection to your Valkey instance through the UI.
 
+:::caution[Publishing `8080` exposes an unauthenticated service]
+Valkey Admin Web mode has **no authentication** — anyone who can reach the published
+port can use it to access your Valkey instances. `-p 8080:8080` (and the compose
+`ports: "8080:8080"`) publishes it on **all** host interfaces.
+
+- Keep it local: bind the published port to loopback with `-p 127.0.0.1:8080:8080`
+  (or `ports: "127.0.0.1:8080:8080"`).
+- Only publish on a routable interface if you have placed an authenticating proxy
+  in front of it or otherwise restricted access. Treat external exposure as a
+  deliberate decision.
+:::
+
 ## With Pre-configured Connection
 
 To auto-start metrics collection on startup, provide connection details as environment variables. This works for both **cluster** and **standalone** Valkey instances — Valkey Admin detects the topology automatically:
diff --git a/examples/k8s/app.yaml b/examples/k8s/app.yaml
index a3718145..8dabe5d9 100644
--- a/examples/k8s/app.yaml
+++ b/examples/k8s/app.yaml
@@ -36,6 +36,9 @@ spec:
               value: "8080"
             - name: DEPLOYMENT_MODE
               value: "K8"
+            # Bind all interfaces so the Service/pod network can reach the server.
+            - name: SERVER_BIND_HOST
+              value: "0.0.0.0"
             - name: VALKEY_HOST
               value: valkey-0.valkey-headless.valkey.svc.cluster.local
             - name: VALKEY_PORT

From 5f8607c083b488dbd42db1ac0b2342d1a14a9075 Mon Sep 17 00:00:00 2001
From: Ravjot Brar <83892020+ravjotbrar@users.noreply.github.com>
Date: Fri, 18 Sep 2026 12:42:27 -0700
Subject: [PATCH 3/3] Require a per-launch token for the Electron WebSocket
 handshake (#516)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In Electron mode the WebSocket handshake was accepted for any request
presenting an Origin of "null" or "file://". That opaque origin is not
unique to the desktop renderer — a sandboxed iframe (or data:/blob:
document) in an ordinary browser produces the same "null" origin, and the
backend listens on loopback, so any visited web page could open a socket
to it and drive the user's Valkey connections with no credentials.

Gate the non-web / loopback renderer origins behind a per-launch token:
- electron.main.js mints a random 256-bit token each launch, passes it to
  the backend (fork env ELECTRON_WS_TOKEN) and to the renderer
  (webPreferences.additionalArguments).
- preload.js exposes it to the page via contextBridge.
- wsEpics.ts appends it to the Electron WS URL (?token=...).
- websocket-origin.ts requires a matching token (crypto.timingSafeEqual)
  alongside the local origin, and fails closed if no token is provisioned.

Web mode is unchanged (strict same-origin + configured allowlist). Tests
updated to require the token and cover missing/wrong/absent-token cases.

Signed-off-by: ravjotb <ravjot.brar@improving.com>
(cherry picked from commit bfaa57969e0581293f53658f423ad701db380f61)
---
 apps/frontend/electron.main.js                |  7 ++
 apps/frontend/preload.js                      |  8 +++
 apps/frontend/src/state/epics/wsEpics.ts      |  3 +-
 apps/frontend/src/types/electron.d.ts         |  5 ++
 .../src/__tests__/websocket-origin.test.ts    | 67 +++++++++++++++----
 apps/server/src/websocket-origin.ts           | 42 ++++++++++--
 6 files changed, 113 insertions(+), 19 deletions(-)

diff --git a/apps/frontend/electron.main.js b/apps/frontend/electron.main.js
index 544a3151..cb84b328 100644
--- a/apps/frontend/electron.main.js
+++ b/apps/frontend/electron.main.js
@@ -2,10 +2,14 @@
 const { app, BrowserWindow, ipcMain, safeStorage, shell, powerMonitor, session } = require("electron")
 const path = require("path")
 const { fork } = require("child_process")
+const crypto = require("crypto")
 const { createApplicationMenu } = require("./menu")
 
 let serverProcess
 const ELECTRON = "Electron"
+// Per-launch secret shared with the backend so only this app's renderer can open
+// the WebSocket. Regenerated every launch; never persisted.
+const wsToken = crypto.randomBytes(32).toString("hex")
 function startServer() {
   if (app.isPackaged) {
     const serverPath = path.join(process.resourcesPath, "server-backend.cjs")
@@ -14,6 +18,7 @@ function startServer() {
       env: {
         ...process.env,
         DEPLOYMENT_MODE: ELECTRON,
+        ELECTRON_WS_TOKEN: wsToken,
         PROCESS_RESOURCES_PATH: process.resourcesPath,
         DATA_DIR: path.join(app.getPath("userData"), "metrics-data"),
       },
@@ -39,6 +44,8 @@ function createWindow() {
       nodeIntegration: false,
       contextIsolation: true,
       preload: path.join(__dirname, "preload.js"),
+      // Hand the per-launch token to the preload (readable via process.argv).
+      additionalArguments: [`--valkey-admin-ws-token=${wsToken}`],
     },
   })
 
diff --git a/apps/frontend/preload.js b/apps/frontend/preload.js
index c081778d..c640c79c 100644
--- a/apps/frontend/preload.js
+++ b/apps/frontend/preload.js
@@ -1,6 +1,14 @@
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 const { contextBridge, ipcRenderer } = require("electron")
 
+// The main process passes the per-launch WebSocket token via additionalArguments.
+const wsTokenArg = process.argv.find((arg) => arg.startsWith("--valkey-admin-ws-token="))
+const wsToken = wsTokenArg ? wsTokenArg.slice("--valkey-admin-ws-token=".length) : ""
+
+contextBridge.exposeInMainWorld("valkeyAdminRuntime", {
+  wsToken,
+})
+
 contextBridge.exposeInMainWorld("secureStorage", {
   encrypt: (password) => ipcRenderer.invoke("secure-storage:encrypt", password),
   decrypt: (encrypted) => ipcRenderer.invoke("secure-storage:decrypt", encrypted),
diff --git a/apps/frontend/src/state/epics/wsEpics.ts b/apps/frontend/src/state/epics/wsEpics.ts
index faef520b..937d4b4b 100644
--- a/apps/frontend/src/state/epics/wsEpics.ts
+++ b/apps/frontend/src/state/epics/wsEpics.ts
@@ -29,7 +29,8 @@ let socket$: WebSocketSubject<PayloadAction> | null = null
 const getWebsocketURL = () => {
   // If it's an Electron deployment
   if (window.location.protocol === "file:") {
-    return "ws://localhost:8080"
+    const token = window.valkeyAdminRuntime?.wsToken
+    return token ? `ws://localhost:8080?token=${encodeURIComponent(token)}` : "ws://localhost:8080"
   }
   
   const protocol = window.location.protocol === "https:" ? "wss" : "ws"
diff --git a/apps/frontend/src/types/electron.d.ts b/apps/frontend/src/types/electron.d.ts
index dbbac7a1..401edc8e 100644
--- a/apps/frontend/src/types/electron.d.ts
+++ b/apps/frontend/src/types/electron.d.ts
@@ -2,8 +2,13 @@ export interface ElectronNavigation {
   onNavigate: (callback: (route: string) => void) => void
 }
 
+export interface ValkeyAdminRuntime {
+  wsToken: string
+}
+
 declare global {
   interface Window {
     electronNavigation: ElectronNavigation
+    valkeyAdminRuntime?: ValkeyAdminRuntime
   }
 }
diff --git a/apps/server/src/__tests__/websocket-origin.test.ts b/apps/server/src/__tests__/websocket-origin.test.ts
index 5f7d1b9f..be945ec1 100644
--- a/apps/server/src/__tests__/websocket-origin.test.ts
+++ b/apps/server/src/__tests__/websocket-origin.test.ts
@@ -4,16 +4,18 @@ import { DEPLOYMENT_TYPE } from "valkey-common"
 import { isAllowedWebSocketOrigin } from "../websocket-origin"
 import type { IncomingMessage } from "http"
 
-const makeRequest = (headers: Record<string, string | undefined>) =>
-  ({ headers }) as IncomingMessage
+const makeRequest = ({ url, ...headers }: Record<string, string | undefined>) =>
+  ({ headers, url }) as IncomingMessage
 
 describe("isAllowedWebSocketOrigin", () => {
   const originalDeploymentMode = process.env.DEPLOYMENT_MODE
   const originalAllowedOrigins = process.env.VALKEY_ADMIN_ALLOWED_WS_ORIGINS
+  const originalWsToken = process.env.ELECTRON_WS_TOKEN
 
   afterEach(() => {
     process.env.DEPLOYMENT_MODE = originalDeploymentMode
     process.env.VALKEY_ADMIN_ALLOWED_WS_ORIGINS = originalAllowedOrigins
+    process.env.ELECTRON_WS_TOKEN = originalWsToken
   })
 
   it("rejects requests without an origin header", () => {
@@ -22,28 +24,69 @@ describe("isAllowedWebSocketOrigin", () => {
     assert.strictEqual(isAllowedWebSocketOrigin(makeRequest({ host: "localhost:8080" })), false)
   })
 
-  it("allows packaged Electron origins", () => {
+  it("allows packaged Electron origins only with a valid per-launch token", () => {
     process.env.DEPLOYMENT_MODE = DEPLOYMENT_TYPE.ELECTRON
+    process.env.ELECTRON_WS_TOKEN = "secret-token"
+
+    for (const origin of ["file://", "null"]) {
+      assert.strictEqual(
+        isAllowedWebSocketOrigin(makeRequest({ origin, host: "localhost:8080", url: "/?token=secret-token" })),
+        true,
+        `${origin} with valid token should be allowed`,
+      )
+    }
+  })
+
+  it("rejects Electron non-web origins when the token is missing or wrong", () => {
+    process.env.DEPLOYMENT_MODE = DEPLOYMENT_TYPE.ELECTRON
+    process.env.ELECTRON_WS_TOKEN = "secret-token"
+
+    for (const origin of ["file://", "null"]) {
+      // No token
+      assert.strictEqual(
+        isAllowedWebSocketOrigin(makeRequest({ origin, host: "localhost:8080", url: "/" })),
+        false,
+        `${origin} without a token must be rejected`,
+      )
+      // Wrong token
+      assert.strictEqual(
+        isAllowedWebSocketOrigin(makeRequest({ origin, host: "localhost:8080", url: "/?token=nope" })),
+        false,
+        `${origin} with a wrong token must be rejected`,
+      )
+    }
+  })
+
+  it("fails closed when no server-side token is provisioned", () => {
+    process.env.DEPLOYMENT_MODE = DEPLOYMENT_TYPE.ELECTRON
+    delete process.env.ELECTRON_WS_TOKEN
 
     assert.strictEqual(
-      isAllowedWebSocketOrigin(makeRequest({ origin: "file://", host: "localhost:8080" })),
-      true,
-    )
-    assert.strictEqual(
-      isAllowedWebSocketOrigin(makeRequest({ origin: "null", host: "localhost:8080" })),
-      true,
+      isAllowedWebSocketOrigin(makeRequest({ origin: "null", host: "localhost:8080", url: "/?token=anything" })),
+      false,
     )
   })
 
-  it("allows loopback origins in Electron mode and blocks remote origins", () => {
+  it("allows loopback origins in Electron mode only with a token and blocks remote origins", () => {
     process.env.DEPLOYMENT_MODE = DEPLOYMENT_TYPE.ELECTRON
+    process.env.ELECTRON_WS_TOKEN = "secret-token"
 
     assert.strictEqual(
-      isAllowedWebSocketOrigin(makeRequest({ origin: "http://localhost:5173", host: "localhost:8080" })),
+      isAllowedWebSocketOrigin(
+        makeRequest({ origin: "http://localhost:5173", host: "localhost:8080", url: "/?token=secret-token" }),
+      ),
       true,
     )
+    // Loopback origin but no token
+    assert.strictEqual(
+      isAllowedWebSocketOrigin(makeRequest({ origin: "http://localhost:5173", host: "localhost:8080", url: "/" })),
+      false,
+    )
+    // Remote origin is rejected regardless of token
     assert.strictEqual(
-      isAllowedWebSocketOrigin(makeRequest({ origin: "https://evil.example", host: "localhost:8080" })),
+      isAllowedWebSocketOrigin(
+        makeRequest({ origin: "https://evil.example", host: "localhost:8080", url: "/?token=secret-token" }),
+      ),
       false,
     )
   })
diff --git a/apps/server/src/websocket-origin.ts b/apps/server/src/websocket-origin.ts
index 8a8172e5..d93ededa 100644
--- a/apps/server/src/websocket-origin.ts
+++ b/apps/server/src/websocket-origin.ts
@@ -1,10 +1,12 @@
 import { DEPLOYMENT_TYPE } from "valkey-common"
+import { timingSafeEqual } from "crypto"
 import type { IncomingMessage } from "http"
 
 const LOCALHOST_HOSTNAMES = new Set(["localhost", "127.0.0.1", "::1", "[::1]"])
 const LOCAL_PROTOCOLS = new Set(["http:", "https:"])
-// Electron file:// renderers may send Origin as "file://" or "null" on the WebSocket handshake.
-const ELECTRON_ORIGINS = new Set(["null", "file://"])
+// Non-web origins a file:// renderer may present; accepted only with a valid token.
+const ELECTRON_NONWEB_ORIGINS = new Set(["null", "file://"])
+const ELECTRON_WS_TOKEN_ENV = "ELECTRON_WS_TOKEN"
 
 const normalizeHost = (hostname: string) => hostname.replace(/^\[|]$/g, "").toLowerCase()
 
@@ -34,6 +36,29 @@ const isSameOrigin = (origin: URL, req: IncomingMessage) => {
   return normalizeOrigin(origin.origin) === `${origin.protocol}//${hostHeader.toLowerCase()}`
 }
 
+// Length-independent comparison so the token can't be recovered by timing.
+const tokensMatch = (a: string, b: string) => {
+  const ab = Buffer.from(a)
+  const bb = Buffer.from(b)
+  // timingSafeEqual requires equal lengths; the token is fixed-length so this
+  // guard leaks nothing useful.
+  return ab.length === bb.length && timingSafeEqual(ab, bb)
+}
+
+// The renderer appends the per-launch token as `?token=...` on the WS URL.
+const hasValidElectronToken = (req: IncomingMessage) => {
+  const expected = process.env[ELECTRON_WS_TOKEN_ENV]
+  // Fail closed: if no token was provisioned, the token gate cannot be satisfied.
+  if (!expected) return false
+  try {
+    const url = new URL(req.url ?? "", "http://localhost")
+    const provided = url.searchParams.get("token")
+    return provided != null && tokensMatch(provided, expected)
+  } catch {
+    return false
+  }
+}
+
 export const isAllowedWebSocketOrigin = (req: IncomingMessage) => {
   // Browsers send Origin on WebSocket handshakes, so we can reject cross-site pages before accepting the upgrade.
   const originHeader = req.headers.origin
@@ -51,11 +76,16 @@ export const isAllowedWebSocketOrigin = (req: IncomingMessage) => {
   }
 
   if (deploymentMode === DEPLOYMENT_TYPE.ELECTRON) {
-    try {
-      return ELECTRON_ORIGINS.has(normalizedOrigin) || isLoopbackOrigin(new URL(normalizedOrigin))
-    } catch { // new URL can technically throw
-      return false
+    // Require a valid per-launch token alongside the local renderer origin.
+    let originLooksLocal = ELECTRON_NONWEB_ORIGINS.has(normalizedOrigin)
+    if (!originLooksLocal) {
+      try {
+        originLooksLocal = isLoopbackOrigin(new URL(normalizedOrigin))
+      } catch { // new URL can technically throw
+        originLooksLocal = false
+      }
     }
+    return originLooksLocal && hasValidElectronToken(req)
   }
 
   try { // for Web deployment — only same origin is allowed