From dcbd233bb52d40e7a44c3555965f24be310d7908 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Wed, 8 Jul 2026 18:11:23 -0600 Subject: [PATCH 1/2] fix: exclude cluster/replication topology operations from ambient user context The #1591/#1592 ambient-user wrap in processLocalTransaction runs every operation handler carrying hdb_user inside contextStorage.run({ ...store, user }). Cluster/replication topology operations (add_node, add_node_back, set_node, update_node, set_node_replication, remove_node, ...) start long-lived, non-awaited replication subscription work whose connect/handshake/ reconnect lifecycle outlives the handler. Because that background work is constructed while the handler's AsyncLocalStorage scope is active, it keeps observing the ambient { user } for its entire life, corrupting the subscription's identity and producing a WebSocket 1006 reconnect storm that never meshes. Exclude those operations from the ambient-user wrap: their writes are internal node-registry bookkeeping (not user-authored), and the background work they spawn must have no ambient user (matching pre-#1591 behavior). Co-Authored-By: Claude Opus 4.8 --- server/serverHelpers/serverUtilities.ts | 33 +++++++- .../serverHelpers/serverUtilities.test.js | 75 +++++++++++++++++++ 2 files changed, 107 insertions(+), 1 deletion(-) diff --git a/server/serverHelpers/serverUtilities.ts b/server/serverHelpers/serverUtilities.ts index a36be9d267..44607f8339 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,12 +125,15 @@ 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 + hdbUser && currentStore?.user !== hdbUser && !OPERATIONS_EXCLUDED_FROM_AMBIENT_USER.has(req.body.operation) ? await contextStorage.run({ ...currentStore, user: hdbUser }, callOperationFunction) : await callOperationFunction(); diff --git a/unitTests/server/serverHelpers/serverUtilities.test.js b/unitTests/server/serverHelpers/serverUtilities.test.js index 2fad7f7172..56baac326f 100644 --- a/unitTests/server/serverHelpers/serverUtilities.test.js +++ b/unitTests/server/serverHelpers/serverUtilities.test.js @@ -563,5 +563,80 @@ 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('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'); + }); + }); }); }); From d89cccafb94f1ba3ef85054af19b782fe25ef72a Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Wed, 8 Jul 2026 18:17:22 -0600 Subject: [PATCH 2/2] fix: strip inherited ambient user for excluded topology ops too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cross-model review (Codex) noted the exclusion only skipped adding a new ambient user, so a topology op invoked while an ambient context already exists (e.g. a nested server.operation() from an authenticated handler) would still run under — and re-leak — the enclosing context's user. Strip any inherited ambient user for excluded operations as well, preserving other ambient state and never mutating the outer context object. Co-Authored-By: Claude Opus 4.8 --- server/serverHelpers/serverUtilities.ts | 18 ++++++++++--- .../serverHelpers/serverUtilities.test.js | 25 +++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/server/serverHelpers/serverUtilities.ts b/server/serverHelpers/serverUtilities.ts index 44607f8339..ad3f530fd9 100644 --- a/server/serverHelpers/serverUtilities.ts +++ b/server/serverHelpers/serverUtilities.ts @@ -132,10 +132,22 @@ export async function processLocalTransaction(req: OperationRequest, operationFu const currentStore = contextStorage.getStore(); const callOperationFunction = () => operationFunctionCaller.callOperationFunctionAsAwait(operationFunction, req.body, null); - let data = - hdbUser && currentStore?.user !== hdbUser && !OPERATIONS_EXCLUDED_FROM_AMBIENT_USER.has(req.body.operation) - ? 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 56baac326f..753d99b2ea 100644 --- a/unitTests/server/serverHelpers/serverUtilities.test.js +++ b/unitTests/server/serverHelpers/serverUtilities.test.js @@ -624,6 +624,31 @@ describe('Test serverUtilities.js module ', () => { ); }); + 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' };