fix: exclude cluster/replication topology operations from ambient user context

server/serverHelpers/serverUtilities.ts

@@ -59,6 +59,34 @@ const GLOBAL_SCHEMA_UPDATE_OPERATIONS_ENUM = {
 	[terms.OPERATIONS_ENUM.DROP_SCHEMA]: true,
 };
 
+// Cluster/replication topology operations must NOT run under the request's ambient user context
+// (the #1591 attribution wrap below). Two reasons:
+//
+//   1. Their writes are internal node-registry bookkeeping (hdb_nodes, certificates), not
+//      user-authored data — they should not be stamped with a user principal.
+//   2. Critically, they start long-lived, non-awaited replication subscription work (a peer
+//      connection's connect/handshake/reconnect lifecycle) that outlives the handler. Because that
+//      work is constructed while this handler's AsyncLocalStorage scope is active, it keeps
+//      observing the ambient { user } for its ENTIRE life, not just the request — which corrupts
+//      the subscription's identity and produces a WebSocket 1006 reconnect storm that never meshes
+//      (regression from #1591/#1592; the pre-#1591 world gave this background work no ambient user).
+//
+// The durable fix is to detach that background work from the ambient context where it is spawned
+// (harper-pro's replication component); this core-side exclusion resolves the regression for the
+// operations that trigger it without that background work ever inheriting a user. The names span
+// core and harper-pro's replication component (registered via server.registerOperation), so this
+// set must be kept in sync if new topology-management operations are added.
+const OPERATIONS_EXCLUDED_FROM_AMBIENT_USER = new Set<string>([
+	terms.OPERATIONS_ENUM.ADD_NODE, // 'add_node'
+	terms.OPERATIONS_ENUM.UPDATE_NODE, // 'update_node'
+	terms.OPERATIONS_ENUM.SET_NODE_REPLICATION, // 'set_node_replication'
+	'add_node_back',
+	'set_node',
+	'remove_node',
+	'remove_node_back',
+	'configure_cluster',
+]);
+
 import { OperationFunctionObject } from './OperationFunctionObject.ts';
 
 type ValueOf<T> = T[keyof T];
@@ -97,14 +125,29 @@ export async function processLocalTransaction(req: OperationRequest, operationFu
 	// differs (or is absent), the ambient context is merged rather than replaced: other ambient
 	// properties (open transaction, signal, caches) are preserved so atomicity is unaffected and
 	// only the user is swapped for attribution; the outer context object itself is never mutated.
+	// Cluster/replication topology operations are excluded (see OPERATIONS_EXCLUDED_FROM_AMBIENT_USER):
+	// they spawn long-lived background work that would otherwise inherit this ambient user for its
+	// whole life.
 	const hdbUser = req.body.hdb_user;
 	const currentStore = contextStorage.getStore();
 	const callOperationFunction = () =>
 		operationFunctionCaller.callOperationFunctionAsAwait(operationFunction, req.body, null);
-	let data =
-		hdbUser && currentStore?.user !== hdbUser
-			? await contextStorage.run({ ...currentStore, user: hdbUser }, callOperationFunction)
+	let data;
+	if (OPERATIONS_EXCLUDED_FROM_AMBIENT_USER.has(req.body.operation)) {
+		// Excluded operations must run with no ambient user principal. Strip any ambient user —
+		// whether it would come from this request's hdb_user or from an enclosing ambient context
+		// (e.g. a nested server.operation() call from within an authenticated handler) — so the
+		// long-lived background work they spawn never inherits a user. Other ambient state is
+		// preserved, and the outer context object is never mutated.
+		data = currentStore
+			? await contextStorage.run({ ...currentStore, user: undefined }, callOperationFunction)
 			: await callOperationFunction();
+	} else {
+		data =
+			hdbUser && currentStore?.user !== hdbUser
+				? await contextStorage.run({ ...currentStore, user: hdbUser }, callOperationFunction)
+				: await callOperationFunction();
+	}
 
 	if (typeof data !== 'object') {
 		data = { message: data };

unitTests/server/serverHelpers/serverUtilities.test.js

@@ -563,5 +563,105 @@ describe('Test serverUtilities.js module ', () => {
 			);
 			assert.equal(observedStore, undefined);
 		});
+
+		// Regression for the #1591/#1592 follow-up: cluster/replication topology operations must not run
+		// under the ambient user context, because they spawn long-lived, non-awaited background work (a
+		// replication subscription connection's connect/handshake/reconnect lifecycle) that outlives the
+		// handler. Under AsyncLocalStorage that background work would otherwise keep observing the ambient
+		// { user } for its entire life — corrupting the subscription's identity and producing a WebSocket
+		// 1006 reconnect storm that never meshes.
+		describe('cluster/replication topology operations are excluded from the ambient user context', function () {
+			const TOPOLOGY_OPERATIONS = [
+				'add_node',
+				'add_node_back',
+				'update_node',
+				'set_node',
+				'set_node_replication',
+				'remove_node',
+				'remove_node_back',
+				'configure_cluster',
+			];
+
+			for (const operation of TOPOLOGY_OPERATIONS) {
+				it(`does not establish an ambient user context for '${operation}'`, async function () {
+					op_func_caller_stub.callThrough();
+					let observedStore;
+					await serverUtilities.processLocalTransaction(
+						{ body: { operation, hdb_user: { username: 'admin' } } },
+						async () => {
+							observedStore = contextStorage.getStore();
+							return test_func_data;
+						}
+					);
+					assert.equal(observedStore, undefined, `${operation} handler must run with no ambient user`);
+				});
+			}
+
+			it('background work spawned by a topology handler does not observe the request user after the handler returns', async function () {
+				op_func_caller_stub.callThrough();
+				let backgroundStore = 'unset';
+				let resolveBackground;
+				const backgroundRead = new Promise((resolve) => {
+					resolveBackground = resolve;
+				});
+				await serverUtilities.processLocalTransaction(
+					{ body: { operation: 'add_node', hdb_user: { username: 'admin' } } },
+					async () => {
+						// simulate the handler kicking off non-awaited long-lived work (e.g. a subscription
+						// connection) that reads the ambient context after the handler has returned
+						setTimeout(() => {
+							backgroundStore = contextStorage.getStore();
+							resolveBackground();
+						}, 5);
+						return test_func_data;
+					}
+				);
+				await backgroundRead;
+				assert.equal(
+					backgroundStore,
+					undefined,
+					'detached background work must not inherit the request user for its ongoing lifetime'
+				);
+			});
+
+			it('strips an inherited ambient user when a topology op runs inside an existing context (nested server.operation)', async function () {
+				// A topology op invoked via server.operation() from within an authenticated request would
+				// otherwise fall through and run under the enclosing context's user, re-leaking it into the
+				// background work it spawns. The exclusion must strip that inherited user too, while
+				// preserving other ambient state and not mutating the outer context object.
+				op_func_caller_stub.callThrough();
+				const outerUser = { username: 'outer_user' };
+				const outerTransaction = { open: true };
+				const outerContext = { user: outerUser, transaction: outerTransaction, someRequestState: true };
+				let observedStore;
+				await contextStorage.run(outerContext, () =>
+					serverUtilities.processLocalTransaction(
+						{ body: { operation: 'add_node', hdb_user: outerUser } },
+						async () => {
+							observedStore = contextStorage.getStore();
+							return test_func_data;
+						}
+					)
+				);
+				assert.equal(observedStore.user, undefined, 'inherited ambient user must be stripped for topology ops');
+				assert.equal(observedStore.transaction, outerTransaction, 'other ambient state must be preserved');
+				assert.equal(observedStore.someRequestState, true, 'other ambient state must be preserved');
+				assert.equal(outerContext.user, outerUser, 'outer context object must not be mutated');
+			});
+
+			it('still establishes the ambient user for a non-topology operation (does not over-exclude)', async function () {
+				op_func_caller_stub.callThrough();
+				const hdbUser = { username: 'data_user' };
+				let observedStore;
+				await serverUtilities.processLocalTransaction(
+					{ body: { operation: 'insert', hdb_user: hdbUser } },
+					async () => {
+						observedStore = contextStorage.getStore();
+						return test_func_data;
+					}
+				);
+				assert.equal(observedStore?.user, hdbUser, 'data operations must keep #1591 audit attribution');
+			});
+		});
 	});
 });