From 28ae1f5912e5a6bb44b51c19bb52735ce14832f9 Mon Sep 17 00:00:00 2001 From: Karl Power Date: Mon, 2 Nov 2026 11:36:11 +0100 Subject: [PATCH 1/2] fix: Reject alerts on PromQL dashboard tiles validateAlertInput only checked raw SQL tiles, so an alert on a PromQL tile was saved even though checkAlerts skips PromQL configs and never evaluates it. Reject it with a 400. This covers the app, the external API and MCP save_alert. --- .changeset/reject-promql-tile-alerts.md | 5 +++ packages/api/src/controllers/alerts.ts | 10 ++++- .../routers/api/__tests__/alerts.int.test.ts | 39 +++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 .changeset/reject-promql-tile-alerts.md diff --git a/.changeset/reject-promql-tile-alerts.md b/.changeset/reject-promql-tile-alerts.md new file mode 100644 index 0000000000..17b1de0956 --- /dev/null +++ b/.changeset/reject-promql-tile-alerts.md @@ -0,0 +1,5 @@ +--- +'@hyperdx/api': patch +--- + +fix: reject alerts on PromQL dashboard tiles, which the alert checker never evaluates diff --git a/packages/api/src/controllers/alerts.ts b/packages/api/src/controllers/alerts.ts index 0e8762e48c..60f461dd47 100644 --- a/packages/api/src/controllers/alerts.ts +++ b/packages/api/src/controllers/alerts.ts @@ -4,7 +4,10 @@ import { isFormulaSourceKind, validateRawSqlForAlert, } from '@hyperdx/common-utils/dist/core/utils'; -import { isRawSqlSavedChartConfig } from '@hyperdx/common-utils/dist/guards'; +import { + isPromqlSavedChartConfig, + isRawSqlSavedChartConfig, +} from '@hyperdx/common-utils/dist/guards'; import { isRangeThresholdType } from '@hyperdx/common-utils/dist/types'; import { groupBy } from 'lodash'; import { Types } from 'mongoose'; @@ -91,6 +94,11 @@ export const validateAlertInput = async ( throw new Api400Error('Tile not found'); } + // The alert checker skips PromQL tiles, so such an alert would never run. + if (tile.config != null && isPromqlSavedChartConfig(tile.config)) { + throw new Api400Error('Alerts are not supported on PromQL tiles'); + } + if (tile.config != null && isRawSqlSavedChartConfig(tile.config)) { if (!displayTypeSupportsRawSqlAlerts(tile.config.displayType)) { throw new Api400Error( diff --git a/packages/api/src/routers/api/__tests__/alerts.int.test.ts b/packages/api/src/routers/api/__tests__/alerts.int.test.ts index 8d81390593..424ce89051 100644 --- a/packages/api/src/routers/api/__tests__/alerts.int.test.ts +++ b/packages/api/src/routers/api/__tests__/alerts.int.test.ts @@ -1142,6 +1142,45 @@ describe('alerts router', () => { .expect(400); }); + it('rejects creating an alert on a PromQL tile', async () => { + const promqlTileId = randomMongoId(); + const dashboard = await Dashboard.create({ + name: 'PromQL Dashboard', + team: team._id, + tiles: [ + { + id: promqlTileId, + x: 0, + y: 0, + w: 6, + h: 3, + config: { + configType: 'promql', + name: 'Up', + displayType: DisplayType.Line, + source: randomMongoId(), + promqlExpression: [{ expression: 'up' }], + }, + }, + ], + }); + + const res = await agent + .post('/alerts') + .send( + makeAlertInput({ + dashboardId: dashboard._id.toString(), + tileId: promqlTileId, + webhookId: webhook._id.toString(), + }), + ) + .expect(400); + expect(JSON.stringify(res.body)).toContain( + 'Alerts are not supported on PromQL tiles', + ); + expect(await Alert.countDocuments({ team: team._id })).toBe(0); + }); + it('rejects creating an alert on a raw SQL tile without interval params', async () => { const rawSqlTile = makeRawSqlTile({ sqlTemplate: 'SELECT count() FROM otel_logs', From 548d29e5e162d7621a475d62a5c83672251f61e6 Mon Sep 17 00:00:00 2001 From: Karl Power Date: Mon, 2 Nov 2026 12:25:10 +0100 Subject: [PATCH 2/2] fix: apply tile alert support rule to dashboard saves --- .changeset/reject-promql-tile-alerts.md | 5 +- packages/api/src/controllers/alerts.ts | 21 ++++---- packages/api/src/controllers/dashboard.ts | 16 ++++-- .../routers/api/__tests__/alerts.int.test.ts | 23 ++++++++ .../api/__tests__/dashboard.int.test.ts | 53 +++++++++++++++++++ .../external-api/v2/utils/dashboards.ts | 20 ++----- packages/common-utils/src/core/utils.ts | 22 +++++++- 7 files changed, 129 insertions(+), 31 deletions(-) diff --git a/.changeset/reject-promql-tile-alerts.md b/.changeset/reject-promql-tile-alerts.md index 17b1de0956..03c8dc66f9 100644 --- a/.changeset/reject-promql-tile-alerts.md +++ b/.changeset/reject-promql-tile-alerts.md @@ -1,5 +1,8 @@ --- '@hyperdx/api': patch +'@hyperdx/common-utils': patch --- -fix: reject alerts on PromQL dashboard tiles, which the alert checker never evaluates +fix: reject alerts on dashboard tiles the alert checker never evaluates (PromQL +tiles, and builder tiles with unsupported display types), and drop them when a +dashboard is saved diff --git a/packages/api/src/controllers/alerts.ts b/packages/api/src/controllers/alerts.ts index 60f461dd47..9e040bacf2 100644 --- a/packages/api/src/controllers/alerts.ts +++ b/packages/api/src/controllers/alerts.ts @@ -2,6 +2,7 @@ import { displayTypeSupportsBuilderAlerts, displayTypeSupportsRawSqlAlerts, isFormulaSourceKind, + savedChartConfigSupportsAlerts, validateRawSqlForAlert, } from '@hyperdx/common-utils/dist/core/utils'; import { @@ -94,18 +95,20 @@ export const validateAlertInput = async ( throw new Api400Error('Tile not found'); } - // The alert checker skips PromQL tiles, so such an alert would never run. - if (tile.config != null && isPromqlSavedChartConfig(tile.config)) { - throw new Api400Error('Alerts are not supported on PromQL tiles'); + // The alert checker cannot evaluate these tiles, so the alert would never run. + if (tile.config != null && !savedChartConfigSupportsAlerts(tile.config)) { + if (isPromqlSavedChartConfig(tile.config)) { + throw new Api400Error('Alerts are not supported on PromQL tiles'); + } + const tileKind = isRawSqlSavedChartConfig(tile.config) + ? 'Raw SQL tiles' + : 'dashboard tiles'; + throw new Api400Error( + `Alerts on ${tileKind} are only supported for Line, Stacked Bar, or Number display types`, + ); } if (tile.config != null && isRawSqlSavedChartConfig(tile.config)) { - if (!displayTypeSupportsRawSqlAlerts(tile.config.displayType)) { - throw new Api400Error( - 'Alerts on Raw SQL tiles are only supported for Line, Stacked Bar, or Number display types', - ); - } - const { errors } = validateRawSqlForAlert(tile.config); if (errors.length > 0) { throw new Api400Error( diff --git a/packages/api/src/controllers/dashboard.ts b/packages/api/src/controllers/dashboard.ts index c342f20f4a..4518f54ca7 100644 --- a/packages/api/src/controllers/dashboard.ts +++ b/packages/api/src/controllers/dashboard.ts @@ -1,3 +1,4 @@ +import { savedChartConfigSupportsAlerts } from '@hyperdx/common-utils/dist/core/utils'; import { DashboardWithoutIdSchema, resolveChartPaletteToken, @@ -20,10 +21,19 @@ import type { AlertDocument, IAlert } from '@/models/alert'; import Dashboard, { IDashboard } from '@/models/dashboard'; import { resolveAlertDisplayFields } from '@/utils/alerts'; +// Drop alerts on tiles the alert checker cannot evaluate (e.g. PromQL), so +// they are never stored and any existing ones are deleted on save. +function getSupportedTileAlert(tile: Tile) { + return savedChartConfigSupportsAlerts(tile.config) + ? tile.config.alert + : undefined; +} + function pickAlertsByTile(tiles: Tile[]) { return tiles.reduce((acc, tile) => { - if (tile.config.alert) { - acc[tile.id] = tile.config.alert; + const alert = getSupportedTileAlert(tile); + if (alert) { + acc[tile.id] = alert; } return acc; }, {}); @@ -94,7 +104,7 @@ async function syncDashboardAlerts( const newTilesForAlertSync: TileForAlertSync[] = newTiles.map(t => ({ id: t.id, - config: { alert: t.config.alert }, + config: { alert: getSupportedTileAlert(t) }, })); const { tileIds: newTileIds, tileIdsWithAlerts: newTileIdsWithAlerts } = extractTileAlertData(newTilesForAlertSync); diff --git a/packages/api/src/routers/api/__tests__/alerts.int.test.ts b/packages/api/src/routers/api/__tests__/alerts.int.test.ts index 424ce89051..e8da9d0563 100644 --- a/packages/api/src/routers/api/__tests__/alerts.int.test.ts +++ b/packages/api/src/routers/api/__tests__/alerts.int.test.ts @@ -1142,6 +1142,29 @@ describe('alerts router', () => { .expect(400); }); + it('rejects creating an alert on a builder table tile', async () => { + const tile = makeTile(); + tile.config = { ...tile.config, displayType: DisplayType.Table }; + const dashboard = await agent + .post('/dashboards') + .send({ name: 'Test Dashboard', tiles: [tile], tags: [] }) + .expect(200); + + const res = await agent + .post('/alerts') + .send( + makeAlertInput({ + dashboardId: dashboard.body.id, + tileId: tile.id, + webhookId: webhook._id.toString(), + }), + ) + .expect(400); + expect(JSON.stringify(res.body)).toContain( + 'Alerts on dashboard tiles are only supported', + ); + }); + it('rejects creating an alert on a PromQL tile', async () => { const promqlTileId = randomMongoId(); const dashboard = await Dashboard.create({ diff --git a/packages/api/src/routers/api/__tests__/dashboard.int.test.ts b/packages/api/src/routers/api/__tests__/dashboard.int.test.ts index 8a02026a0d..8a6d185739 100644 --- a/packages/api/src/routers/api/__tests__/dashboard.int.test.ts +++ b/packages/api/src/routers/api/__tests__/dashboard.int.test.ts @@ -1,5 +1,6 @@ import { AlertThresholdType, + DisplayType, MetricsDataType, PresetDashboard, SourceKind, @@ -35,6 +36,15 @@ const makeMockAlert = (webhookId: string) => ({ thresholdType: AlertThresholdType.ABOVE, }); +const makePromqlConfig = (alert?: ReturnType) => ({ + configType: 'promql' as const, + name: 'Up', + displayType: DisplayType.Line, + source: 'test-source', + promqlExpression: [{ expression: 'up' }], + alert, +}); + describe('dashboard router', () => { const server = getServer(); let agent: Awaited>['agent']; @@ -508,6 +518,49 @@ describe('dashboard router', () => { expect(alerts.body.data).toEqual([]); }); + it('does not create alerts on PromQL tiles when creating a dashboard', async () => { + await agent + .post('/dashboards') + .send({ + name: 'Test Dashboard', + tiles: [ + { + ...makeTile(), + config: makePromqlConfig(makeMockAlert(webhook._id.toString())), + }, + ], + tags: [], + }) + .expect(200); + + expect(await Alert.countDocuments({ team: team._id })).toBe(0); + }); + + it('deletes the alert when a tile is switched to PromQL', async () => { + const mockAlert = makeMockAlert(webhook._id.toString()); + const dashboard = await agent + .post('/dashboards') + .send({ + name: 'Test Dashboard', + tiles: [makeTile({ alert: mockAlert })], + tags: [], + }) + .expect(200); + expect(await Alert.countDocuments({ team: team._id })).toBe(1); + + await agent + .patch(`/dashboards/${dashboard.body.id}`) + .send({ + ...dashboard.body, + tiles: [ + { ...dashboard.body.tiles[0], config: makePromqlConfig(mockAlert) }, + ], + }) + .expect(200); + + expect(await Alert.countDocuments({ team: team._id })).toBe(0); + }); + it('alerts are updated when updating dashboard (updating tile alert settings)', async () => { const mockAlert = makeMockAlert(webhook._id.toString()); const dashboard = await agent diff --git a/packages/api/src/routers/external-api/v2/utils/dashboards.ts b/packages/api/src/routers/external-api/v2/utils/dashboards.ts index 8ffa9564b3..db481009ff 100644 --- a/packages/api/src/routers/external-api/v2/utils/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/utils/dashboards.ts @@ -1,9 +1,8 @@ import { getHeatmapMode } from '@hyperdx/common-utils/dist/core/heatmap'; import { getPromqlSeries } from '@hyperdx/common-utils/dist/core/promql'; import { - displayTypeSupportsBuilderAlerts, - displayTypeSupportsRawSqlAlerts, isFormulaSourceKind, + savedChartConfigSupportsAlerts, TIME_SERIES_DISPLAY_TYPE_BY_NAME, } from '@hyperdx/common-utils/dist/core/utils'; import { @@ -1863,22 +1862,9 @@ export async function cleanupDashboardAlerts({ }) { const newTileIdSet = new Set(internalTiles.map(t => t.id)); const tileIdsToDeleteAlerts = [ - // Tiles whose config no longer supports alerts. PromQL tiles cannot be - // alerted on (HDX-4636), so an alert left on a tile switched to PromQL - // would never be checked. + // Tiles whose config no longer supports alerts (e.g. switched to PromQL). ...internalTiles - .filter(tile => { - if (isPromqlSavedChartConfig(tile.config)) { - return true; - } - if (isRawSqlSavedChartConfig(tile.config)) { - return !displayTypeSupportsRawSqlAlerts(tile.config.displayType); - } - if (isBuilderSavedChartConfig(tile.config)) { - return !displayTypeSupportsBuilderAlerts(tile.config.displayType); - } - return false; - }) + .filter(tile => !savedChartConfigSupportsAlerts(tile.config)) .map(tile => tile.id), // Tiles that were completely removed. ...[...existingTileIds].filter(id => !newTileIdSet.has(id)), diff --git a/packages/common-utils/src/core/utils.ts b/packages/common-utils/src/core/utils.ts index df92f006a1..972a298ed9 100644 --- a/packages/common-utils/src/core/utils.ts +++ b/packages/common-utils/src/core/utils.ts @@ -11,7 +11,11 @@ import { isQueryExpressionFilter, isStaticListFilter, } from '@/filters'; -import { isBuilderSavedChartConfig, isRawSqlSavedChartConfig } from '@/guards'; +import { + isBuilderSavedChartConfig, + isPromqlSavedChartConfig, + isRawSqlSavedChartConfig, +} from '@/guards'; import { MacroExpansionError, MalformedMacroArgsError } from '@/macroErrors'; import { getSourceDependentMacrosUsed, @@ -1675,6 +1679,22 @@ export function displayTypeSupportsPromQLAlerts( return displayType ? false : false; } +/** + * Whether the alert checker can evaluate an alert on a tile with this config. + * Shared by every save path so alerts are never stored where they cannot run. + */ +export function savedChartConfigSupportsAlerts( + config: SavedChartConfig, +): boolean { + if (isPromqlSavedChartConfig(config)) { + return displayTypeSupportsPromQLAlerts(config.displayType); + } + if (isRawSqlSavedChartConfig(config)) { + return displayTypeSupportsRawSqlAlerts(config.displayType); + } + return displayTypeSupportsBuilderAlerts(config.displayType); +} + /** Expand the chart's macros, returning failures instead of throwing. */ function resolveRawSqlMacros( chartConfig: RawSqlChartConfig,