diff --git a/server/serverHelpers/serverUtilities.ts b/server/serverHelpers/serverUtilities.ts index a36be9d267..ad3f530fd9 100644 --- a/server/serverHelpers/serverUtilities.ts +++ b/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([ + 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[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 }; diff --git a/unitTests/server/serverHelpers/serverUtilities.test.js b/unitTests/server/serverHelpers/serverUtilities.test.js index 2fad7f7172..753d99b2ea 100644 --- a/unitTests/server/serverHelpers/serverUtilities.test.js +++ b/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'); + }); + }); }); });