mirror of
https://github.com/hyperdxio/hyperdx
synced 2026-04-21 13:37:15 +00:00
fix: Hide potentially-sensitive alert errors (#2136)
## Summary This PR updates the recent alert runner error persistence + display (#2132) to hardcode webhook and unknown-type errors. The raw error messages could contain potentially sensitive information, so we won't persist them or show them in the UI. <img width="664" height="183" alt="Screenshot 2026-04-20 at 7 13 57 AM" src="https://github.com/user-attachments/assets/0f4f600b-2cdd-47e5-ba72-cec4dbc40423" /> ### How to test locally or on Vercel This can be tested locally by running an alert with an invalid webhook destination.
This commit is contained in:
parent
f086842f3c
commit
00222da68f
2 changed files with 90 additions and 10 deletions
|
|
@ -2813,8 +2813,8 @@ describe('checkAlerts', () => {
|
||||||
expect(updated!.executionErrors![0].type).toBe(
|
expect(updated!.executionErrors![0].type).toBe(
|
||||||
AlertErrorType.WEBHOOK_ERROR,
|
AlertErrorType.WEBHOOK_ERROR,
|
||||||
);
|
);
|
||||||
expect(updated!.executionErrors![0].message).toContain(
|
expect(updated!.executionErrors![0].message).toBe(
|
||||||
'webhook exploded',
|
'Failed to send webhook notification. Check the webhook configuration and destination.',
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
@ -3108,9 +3108,18 @@ describe('checkAlerts', () => {
|
||||||
e => e.type === AlertErrorType.WEBHOOK_ERROR,
|
e => e.type === AlertErrorType.WEBHOOK_ERROR,
|
||||||
),
|
),
|
||||||
).toBe(true);
|
).toBe(true);
|
||||||
|
// Webhook error messages are hardcoded for security; the raw upstream
|
||||||
|
// error ("group webhook failed") must not leak into the stored message.
|
||||||
expect(
|
expect(
|
||||||
updated!.executionErrors!.every(e =>
|
updated!.executionErrors!.every(
|
||||||
e.message.includes('group webhook failed'),
|
e => !e.message.includes('group webhook failed'),
|
||||||
|
),
|
||||||
|
).toBe(true);
|
||||||
|
expect(
|
||||||
|
updated!.executionErrors!.every(
|
||||||
|
e =>
|
||||||
|
e.message ===
|
||||||
|
'Failed to send webhook notification. Check the webhook configuration and destination.',
|
||||||
),
|
),
|
||||||
).toBe(true);
|
).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
@ -3179,21 +3188,80 @@ describe('checkAlerts', () => {
|
||||||
await AlertHistory.countDocuments({ alert: details.alert.id }),
|
await AlertHistory.countDocuments({ alert: details.alert.id }),
|
||||||
).toBe(1);
|
).toBe(1);
|
||||||
|
|
||||||
// A descriptive WEBHOOK_ERROR should be recorded so the user can debug
|
// A WEBHOOK_ERROR should be recorded. The message is hardcoded for
|
||||||
|
// security — the raw internal error ("Webhook not found ... deleted")
|
||||||
|
// must not leak into the stored message.
|
||||||
expect(updated!.executionErrors).toBeDefined();
|
expect(updated!.executionErrors).toBeDefined();
|
||||||
expect(updated!.executionErrors!.length).toBe(1);
|
expect(updated!.executionErrors!.length).toBe(1);
|
||||||
expect(updated!.executionErrors![0].type).toBe(
|
expect(updated!.executionErrors![0].type).toBe(
|
||||||
AlertErrorType.WEBHOOK_ERROR,
|
AlertErrorType.WEBHOOK_ERROR,
|
||||||
);
|
);
|
||||||
expect(updated!.executionErrors![0].message).toContain(
|
expect(updated!.executionErrors![0].message).toBe(
|
||||||
'Webhook not found',
|
'Failed to send webhook notification. Check the webhook configuration and destination.',
|
||||||
);
|
);
|
||||||
// Hint the user on what to do about it
|
|
||||||
expect(updated!.executionErrors![0].message).toMatch(/deleted|update/);
|
|
||||||
|
|
||||||
// No actual network call should have been attempted
|
// No actual network call should have been attempted
|
||||||
expect(slack.postMessageToWebhook).not.toHaveBeenCalled();
|
expect(slack.postMessageToWebhook).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('records an UNKNOWN error with a hardcoded message when an unexpected error is thrown', async () => {
|
||||||
|
const {
|
||||||
|
team,
|
||||||
|
webhook,
|
||||||
|
connection,
|
||||||
|
source,
|
||||||
|
savedSearch,
|
||||||
|
teamWebhooksById,
|
||||||
|
clickhouseClient,
|
||||||
|
} = await setupSavedSearchAlertTest();
|
||||||
|
|
||||||
|
const details = await createAlertDetails(
|
||||||
|
team,
|
||||||
|
source,
|
||||||
|
{
|
||||||
|
source: AlertSource.SAVED_SEARCH,
|
||||||
|
channel: {
|
||||||
|
type: 'webhook',
|
||||||
|
webhookId: webhook._id.toString(),
|
||||||
|
},
|
||||||
|
interval: '5m',
|
||||||
|
thresholdType: AlertThresholdType.ABOVE,
|
||||||
|
threshold: 1,
|
||||||
|
savedSearchId: savedSearch.id,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
taskType: AlertTaskType.SAVED_SEARCH,
|
||||||
|
savedSearch,
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
// Force an unexpected (non-InvalidAlert) failure deep in processAlert
|
||||||
|
// by making updateAlertState reject. This should end up as UNKNOWN.
|
||||||
|
const updateAlertStateSpy = jest
|
||||||
|
.spyOn(alertProvider, 'updateAlertState')
|
||||||
|
.mockImplementationOnce(() => {
|
||||||
|
throw new Error('secret internal detail');
|
||||||
|
});
|
||||||
|
|
||||||
|
await processAlertAtTime(
|
||||||
|
new Date('2023-11-16T22:12:00.000Z'),
|
||||||
|
details,
|
||||||
|
clickhouseClient,
|
||||||
|
connection.id,
|
||||||
|
alertProvider,
|
||||||
|
teamWebhooksById,
|
||||||
|
);
|
||||||
|
|
||||||
|
const updated = await Alert.findById(details.alert.id);
|
||||||
|
expect(updated!.executionErrors).toBeDefined();
|
||||||
|
expect(updated!.executionErrors!.length).toBe(1);
|
||||||
|
expect(updated!.executionErrors![0].type).toBe(AlertErrorType.UNKNOWN);
|
||||||
|
expect(updated!.executionErrors![0].message).toBe(
|
||||||
|
'An unknown error occurred while processing the alert.',
|
||||||
|
);
|
||||||
|
|
||||||
|
updateAlertStateSpy.mockRestore();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it('TILE alert (events) - generic webhook', async () => {
|
it('TILE alert (events) - generic webhook', async () => {
|
||||||
|
|
|
||||||
|
|
@ -145,13 +145,25 @@ export class InvalidAlertError extends Error {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// For security, we do not surface raw error messages for webhook or unknown
|
||||||
|
// failures — they may leak URLs, response bodies, or other sensitive detail
|
||||||
|
// from upstream systems. QUERY_ERROR and INVALID_ALERT messages are authored
|
||||||
|
// by us (ClickHouse errors or our own validation) and are safe to display.
|
||||||
|
const HARDCODED_ALERT_ERROR_MESSAGES: Partial<Record<AlertErrorType, string>> =
|
||||||
|
{
|
||||||
|
[AlertErrorType.WEBHOOK_ERROR]:
|
||||||
|
'Failed to send webhook notification. Check the webhook configuration and destination.',
|
||||||
|
[AlertErrorType.UNKNOWN]:
|
||||||
|
'An unknown error occurred while processing the alert.',
|
||||||
|
};
|
||||||
|
|
||||||
const makeAlertError = (
|
const makeAlertError = (
|
||||||
type: AlertErrorType,
|
type: AlertErrorType,
|
||||||
message: string,
|
message: string,
|
||||||
): IAlertError => ({
|
): IAlertError => ({
|
||||||
timestamp: new Date(),
|
timestamp: new Date(),
|
||||||
type,
|
type,
|
||||||
message: message.slice(0, 10000),
|
message: (HARDCODED_ALERT_ERROR_MESSAGES[type] ?? message).slice(0, 10000),
|
||||||
});
|
});
|
||||||
|
|
||||||
const getErrorMessage = (e: unknown): string => {
|
const getErrorMessage = (e: unknown): string => {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue