-
Notifications
You must be signed in to change notification settings - Fork 7.3k
fix(notify): address 5 post-merge review findings #2517
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -698,6 +698,21 @@ export function renderPreferences(host: PreferencesHost): PreferencesResult { | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| reloadNotifSection(); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // When a new channel is linked, auto-update the rule's channels list | ||||||||||||||||||||||
| // so it includes the new channel without requiring a manual toggle. | ||||||||||||||||||||||
| function saveRuleWithNewChannel(newChannel: ChannelType): void { | ||||||||||||||||||||||
| const enabledEl = container.querySelector<HTMLInputElement>('#usNotifEnabled'); | ||||||||||||||||||||||
| const sensitivityEl = container.querySelector<HTMLSelectElement>('#usNotifSensitivity'); | ||||||||||||||||||||||
| if (!enabledEl) return; | ||||||||||||||||||||||
| const enabled = enabledEl.checked; | ||||||||||||||||||||||
| const sensitivity = (sensitivityEl?.value ?? 'all') as 'all' | 'high' | 'critical'; | ||||||||||||||||||||||
| const existing = Array.from(container.querySelectorAll<HTMLElement>('[data-channel-type]')) | ||||||||||||||||||||||
| .filter(el => el.querySelector('.us-notif-disconnect')) | ||||||||||||||||||||||
| .map(el => el.dataset.channelType as ChannelType); | ||||||||||||||||||||||
| const channels = [...new Set([...existing, newChannel])]; | ||||||||||||||||||||||
| void saveAlertRules({ variant: SITE_VARIANT, enabled, eventTypes: [], sensitivity, channels }); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| let alertRuleDebounceTimer: ReturnType<typeof setTimeout> | null = null; | ||||||||||||||||||||||
| signal.addEventListener('abort', () => { | ||||||||||||||||||||||
| if (alertRuleDebounceTimer !== null) { | ||||||||||||||||||||||
|
|
@@ -758,6 +773,7 @@ export function renderPreferences(host: PreferencesHost): PreferencesResult { | |||||||||||||||||||||
| getChannelsData().then((data) => { | ||||||||||||||||||||||
| const tg = data.channels.find(c => c.channelType === 'telegram'); | ||||||||||||||||||||||
| if (tg?.verified || expired) { | ||||||||||||||||||||||
| if (tg?.verified) saveRuleWithNewChannel('telegram'); | ||||||||||||||||||||||
| reloadNotifSection(); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
773
to
778
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Clearing the poll on the success path prevents the repeated calls:
Suggested change
|
||||||||||||||||||||||
| }).catch(() => { | ||||||||||||||||||||||
|
|
@@ -780,7 +796,7 @@ export function renderPreferences(host: PreferencesHost): PreferencesResult { | |||||||||||||||||||||
| return; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| setEmailChannel(email).then(() => { | ||||||||||||||||||||||
| if (!signal.aborted) reloadNotifSection(); | ||||||||||||||||||||||
| if (!signal.aborted) { saveRuleWithNewChannel('email'); reloadNotifSection(); } | ||||||||||||||||||||||
| }).catch(() => {}); | ||||||||||||||||||||||
| return; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
@@ -799,7 +815,7 @@ export function renderPreferences(host: PreferencesHost): PreferencesResult { | |||||||||||||||||||||
| return; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| setSlackChannel(url).then(() => { | ||||||||||||||||||||||
| if (!signal.aborted) reloadNotifSection(); | ||||||||||||||||||||||
| if (!signal.aborted) { saveRuleWithNewChannel('slack'); reloadNotifSection(); } | ||||||||||||||||||||||
| }).catch(() => {}); | ||||||||||||||||||||||
| return; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
encryptSlackWebhookdecodes the base64 key and imports it without checking it is exactly 32 bytes. The relay'scrypto.cjshas an explicit guard, so if an operator accidentally sets a 16-byte or 24-byte key on Vercel, the edge function will happily encrypt with AES-128/192-GCM while the relay will throw on every decrypt attempt. The result is silent Slack delivery failure for all users, with no error surfaced on the write path.Adding the same check here surfaces the misconfiguration at link-time (a 503 back to the client) rather than silently at alert delivery time: