From 29fec8bb9f0b909ceecb232569c562ec599be8cb Mon Sep 17 00:00:00 2001 From: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> Date: Sat, 14 Mar 2026 21:58:28 -0500 Subject: [PATCH] fix(gateway): harden health monitor account gating (#46749) * gateway: harden health monitor account gating * gateway: tighten health monitor account-id guard --- src/gateway/server-channels.test.ts | 52 +++++++++++++++-- src/gateway/server-channels.ts | 88 +++++++++++++++++++---------- 2 files changed, 106 insertions(+), 34 deletions(-) diff --git a/src/gateway/server-channels.test.ts b/src/gateway/server-channels.test.ts index b6e8f556123..d3820c294b9 100644 --- a/src/gateway/server-channels.test.ts +++ b/src/gateway/server-channels.test.ts @@ -259,15 +259,12 @@ describe("server-channels auto restart", () => { expect(manager.isHealthMonitorEnabled("discord", DEFAULT_ACCOUNT_ID)).toBe(false); }); - it("uses wrapped account config health monitor overrides", () => { + it("uses raw account config overrides when resolvers omit health monitor fields", () => { installTestRegistry( createTestPlugin({ resolveAccount: () => ({ enabled: true, configured: true, - config: { - healthMonitor: { enabled: false }, - }, }), }), ); @@ -276,7 +273,11 @@ describe("server-channels auto restart", () => { loadConfig: () => ({ channels: { discord: { - healthMonitor: { enabled: true }, + accounts: { + [DEFAULT_ACCOUNT_ID]: { + healthMonitor: { enabled: false }, + }, + }, }, }, }), @@ -284,4 +285,45 @@ describe("server-channels auto restart", () => { expect(manager.isHealthMonitorEnabled("discord", DEFAULT_ACCOUNT_ID)).toBe(false); }); + + it("fails closed when account resolution throws during health monitor gating", () => { + installTestRegistry( + createTestPlugin({ + resolveAccount: () => { + throw new Error("unresolved SecretRef"); + }, + }), + ); + + const manager = createManager(); + + expect(manager.isHealthMonitorEnabled("discord", DEFAULT_ACCOUNT_ID)).toBe(false); + }); + + it("does not treat an empty account id as the default account when matching raw overrides", () => { + installTestRegistry( + createTestPlugin({ + resolveAccount: () => ({ + enabled: true, + configured: true, + }), + }), + ); + + const manager = createManager({ + loadConfig: () => ({ + channels: { + discord: { + accounts: { + default: { + healthMonitor: { enabled: false }, + }, + }, + }, + }, + }), + }); + + expect(manager.isHealthMonitorEnabled("discord", "")).toBe(true); + }); }); diff --git a/src/gateway/server-channels.ts b/src/gateway/server-channels.ts index 5595b946884..075fac382a3 100644 --- a/src/gateway/server-channels.ts +++ b/src/gateway/server-channels.ts @@ -7,7 +7,12 @@ import { formatErrorMessage } from "../infra/errors.js"; import { resetDirectoryCache } from "../infra/outbound/target-resolver.js"; import type { createSubsystemLogger } from "../logging/subsystem.js"; import type { PluginRuntime } from "../plugins/runtime/types.js"; -import { DEFAULT_ACCOUNT_ID } from "../routing/session-key.js"; +import { resolveAccountEntry } from "../routing/account-lookup.js"; +import { + DEFAULT_ACCOUNT_ID, + normalizeAccountId, + normalizeOptionalAccountId, +} from "../routing/session-key.js"; import type { RuntimeEnv } from "../runtime.js"; const CHANNEL_RESTART_POLICY: BackoffPolicy = { @@ -31,6 +36,16 @@ type ChannelRuntimeStore = { runtimes: Map; }; +type HealthMonitorConfig = { + healthMonitor?: { + enabled?: boolean; + }; +}; + +type ChannelHealthMonitorConfig = HealthMonitorConfig & { + accounts?: Record; +}; + function createRuntimeStore(): ChannelRuntimeStore { return { aborts: new Map(), @@ -120,45 +135,60 @@ export function createChannelManager(opts: ChannelManagerOptions): ChannelManage const restartKey = (channelId: ChannelId, accountId: string) => `${channelId}:${accountId}`; + const resolveAccountHealthMonitorOverride = ( + channelConfig: ChannelHealthMonitorConfig | undefined, + accountId: string, + ): boolean | undefined => { + if (!channelConfig?.accounts) { + return undefined; + } + const direct = resolveAccountEntry(channelConfig.accounts, accountId); + if (typeof direct?.healthMonitor?.enabled === "boolean") { + return direct.healthMonitor.enabled; + } + + const normalizedAccountId = normalizeOptionalAccountId(accountId); + if (!normalizedAccountId) { + return undefined; + } + const matchKey = Object.keys(channelConfig.accounts).find( + (key) => normalizeAccountId(key) === normalizedAccountId, + ); + if (!matchKey) { + return undefined; + } + return channelConfig.accounts[matchKey]?.healthMonitor?.enabled; + }; + const isHealthMonitorEnabled = (channelId: ChannelId, accountId: string): boolean => { const cfg = loadConfig(); - const plugin = getChannelPlugin(channelId); - const resolvedAccount = plugin?.config.resolveAccount(cfg, accountId) as - | { - healthMonitor?: { - enabled?: boolean; - }; - config?: { - healthMonitor?: { - enabled?: boolean; - }; - }; - } - | undefined; - const accountOverride = resolvedAccount?.healthMonitor?.enabled; - const wrappedAccountOverride = resolvedAccount?.config?.healthMonitor?.enabled; - const channelOverride = ( - cfg.channels?.[channelId] as - | { - healthMonitor?: { - enabled?: boolean; - }; - } - | undefined - )?.healthMonitor?.enabled; + const channelConfig = cfg.channels?.[channelId] as ChannelHealthMonitorConfig | undefined; + const accountOverride = resolveAccountHealthMonitorOverride(channelConfig, accountId); + const channelOverride = channelConfig?.healthMonitor?.enabled; if (typeof accountOverride === "boolean") { return accountOverride; } - if (typeof wrappedAccountOverride === "boolean") { - return wrappedAccountOverride; - } - if (typeof channelOverride === "boolean") { return channelOverride; } + const plugin = getChannelPlugin(channelId); + if (!plugin) { + return true; + } + try { + // Probe only: health-monitor config is read directly from raw channel config above. + // This call exists solely to fail closed if resolver-side config loading is broken. + plugin.config.resolveAccount(cfg, accountId); + } catch (err) { + channelLogs[channelId].warn?.( + `[${channelId}:${accountId}] health-monitor: failed to resolve account; skipping monitor (${formatErrorMessage(err)})`, + ); + return false; + } + return true; };