From befdb0175a882cecb722cc03d961044ddfe489c0 Mon Sep 17 00:00:00 2001 From: Vishal Kumar Singh Date: Thu, 9 Jul 2026 17:00:26 +0530 Subject: [PATCH] Stop leaking message body via the Notifications API tag (#36364) * Stop leaking message body via the Notifications API tag showNotification was passing the rendered chat body as the Web Notifications API tag option. On Chromium-based browsers (Chrome, Edge, Brave), the tag is serialised into the notification-activation command line via the --notification-launch-id argument, where endpoint detection tooling such as CrowdStrike Falcon FDR, Microsoft Defender for Endpoint, and Sysmon Event ID 1 captures the full process-start command line and forwards it to the customer's SIEM. That meant private message content (including incident-response messages, credentials accidentally pasted into chat, and customer PII) was being copied into telemetry pipelines that were never in scope to receive it. Use the title - which already carries only the sender / channel context - as the tag instead. As a side benefit this is closer to the spec-intended use of tag: subsequent notifications from the same conversation now replace the prior one rather than stacking. Add a regression test covering the leak: the test pushes a body with a plausible secret pattern (token=AKIA-...) and asserts the tag never echoes any of it. Signed-off-by: Vishal Kumar Singh * Allow callers to pass an explicit notification tag Threads channelId through dispatchNotification so per-conversation notifications coalesce by a stable opaque id rather than the user-visible title. The title remains as a safe fallback when callers do not supply a tag, preserving the existing behaviour for the session-expired notification emitted from login.tsx where no channel context exists. Signed-off-by: Vishal Kumar Singh * fix: explain desktop notification path * Avoid title fallback for notification tags Signed-off-by: Vishal Kumar Singh * test: align notification action payload expectations Signed-off-by: Vishal Kumar Singh * test: align notification tag e2e expectation --------- Signed-off-by: Vishal Kumar Singh Co-authored-by: Mattermost Build --- .../notifications/at_mentions_spec.js | 19 +++--- .../src/actions/notification_actions.test.js | 3 + .../src/actions/notification_actions.tsx | 8 ++- .../channels/src/utils/notifications.test.ts | 58 ++++++++++++++++++- webapp/channels/src/utils/notifications.ts | 19 +++++- 5 files changed, 93 insertions(+), 14 deletions(-) diff --git a/e2e-tests/cypress/tests/integration/channels/notifications/at_mentions_spec.js b/e2e-tests/cypress/tests/integration/channels/notifications/at_mentions_spec.js index 752fbec386c..c11367021d9 100644 --- a/e2e-tests/cypress/tests/integration/channels/notifications/at_mentions_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/notifications/at_mentions_spec.js @@ -57,19 +57,20 @@ describe('Notifications', () => { const message = `@${receiver.username} I'm messaging you! ${Date.now()}`; // # Use another account to post a message @-mentioning our receiver - cy.postMessageAs({sender, message, channelId: otherChannel.id}); + cy.postMessageAs({sender, message, channelId: otherChannel.id}).then(({id: postId}) => { + const body = `@${sender.username}: ${message}`; - const body = `@${sender.username}: ${message}`; + cy.get('@notifySpy').should('have.been.calledWithMatch', otherChannel.display_name, (args) => { + expect(args.body, `Notification body: "${args.body}" should match: "${body}"`).to.equal(body); + expect(args.tag, `Notification tag: "${args.tag}" should match the post id`).to.equal(postId); + expect(args.tag, `Notification tag: "${args.tag}" should not contain notification text`).not.to.equal(body); + return true; + }); - cy.get('@notifySpy').should('have.been.calledWithMatch', otherChannel.display_name, (args) => { - expect(args.body, `Notification body: "${args.body}" should match: "${body}"`).to.equal(body); - expect(args.tag, `Notification tag: "${args.tag}" should match: "${body}"`).to.equal(body); - return true; + cy.get('@notifySpy').should('have.been.calledWithMatch', + otherChannel.display_name, {body, tag: postId, requireInteraction: false, silent: false}); }); - cy.get('@notifySpy').should('have.been.calledWithMatch', - otherChannel.display_name, {body, tag: body, requireInteraction: false, silent: false}); - // * Verify unread mentions badge cy.get(`#sidebarItem_${otherChannel.name}`). scrollIntoView(). diff --git a/webapp/channels/src/actions/notification_actions.test.js b/webapp/channels/src/actions/notification_actions.test.js index f4765528d0c..d5b6a8aae22 100644 --- a/webapp/channels/src/actions/notification_actions.test.js +++ b/webapp/channels/src/actions/notification_actions.test.js @@ -211,6 +211,7 @@ describe('notification_actions', () => { body: '@username: Where is Jessica Hyde?', requireInteraction: false, silent: false, + tag: 'post_id', title: 'Utopia', onClick: expect.any(Function), }); @@ -358,6 +359,7 @@ describe('notification_actions', () => { body: '@username: Where is Jessica Hyde?', requireInteraction: false, silent: false, + tag: 'post_id', title: 'Muted Channel', onClick: expect.any(Function), }); @@ -459,6 +461,7 @@ describe('notification_actions', () => { body: '@username: Where is Jessica Hyde?', requireInteraction: false, silent: false, + tag: 'post_id', title: 'Reply in Utopia', onClick: expect.any(Function), }); diff --git a/webapp/channels/src/actions/notification_actions.tsx b/webapp/channels/src/actions/notification_actions.tsx index c06e32c04ed..be48aa804e0 100644 --- a/webapp/channels/src/actions/notification_actions.tsx +++ b/webapp/channels/src/actions/notification_actions.tsx @@ -163,7 +163,7 @@ export function sendDesktopNotification(post: Post, msgProps: NewPostMessageProp return {data: {status: 'not_sent', reason: 'desktop_notification_hook', data: String(hookResult)}}; } - const result = dispatch(notifyMe(argsAfterHooks.title, argsAfterHooks.body, channel.id, teamId, argsAfterHooks.silent, argsAfterHooks.soundName, argsAfterHooks.url)); + const result = dispatch(notifyMe(argsAfterHooks.title, argsAfterHooks.body, channel.id, teamId, argsAfterHooks.silent, argsAfterHooks.soundName, argsAfterHooks.url, post.id)); //Don't add extra sounds on native desktop clients if (desktopSoundEnabled && !isDesktopApp() && !isMobile()) { @@ -420,10 +420,12 @@ function shouldSkipNotification( return undefined; } -export function notifyMe(title: string, body: string, channelId: string, teamId: string, silent: boolean, soundName: string, url: string): ActionFuncAsync { +export function notifyMe(title: string, body: string, channelId: string, teamId: string, silent: boolean, soundName: string, url: string, postId: string): ActionFuncAsync { return async (dispatch) => { // handle notifications in desktop app if (isDesktopApp()) { + // The notification-tag leak only affects Chromium-based browser notifications, + // so the desktop app path does not need the opaque post id. const result = await DesktopApp.dispatchNotification(title, body, channelId, teamId, silent, soundName, url); return {data: result}; } @@ -432,6 +434,8 @@ export function notifyMe(title: string, body: string, channelId: string, teamId: const result = await dispatch(showNotification({ title, body, + + tag: postId, requireInteraction: false, silent, onClick: () => { diff --git a/webapp/channels/src/utils/notifications.test.ts b/webapp/channels/src/utils/notifications.test.ts index 7228db77293..26f63c8e0f7 100644 --- a/webapp/channels/src/utils/notifications.test.ts +++ b/webapp/channels/src/utils/notifications.test.ts @@ -93,7 +93,7 @@ describe('Notifications.showNotification', () => { const call = window.Notification.mock.calls[0]; expect(call[1]).toEqual({ body: 'body', - tag: 'body', + tag: '', icon: '', requireInteraction: true, silent: false, @@ -119,13 +119,67 @@ describe('Notifications.showNotification', () => { const call = window.Notification.mock.calls[0]; expect(call[1]).toEqual({ body: 'body', - tag: 'body', + tag: '', icon: '', requireInteraction: true, silent: false, }); }); + it('should not leak notification text via the Notifications API tag when no tag is provided', async () => { + // The Notifications API tag is serialised into the activation command line on Chromium + // and captured by EDR / SIEM tooling. The tag must therefore never carry message content. + window.Notification.permission = 'granted'; + jest.resetModules(); + Notifications = require('utils/notifications'); + + const sensitiveBody = '@alice: token=AKIA-SECRET-VALUE confidential incident details'; + const visibleTitle = '@alice posted in Town Square'; + + await store.dispatch(Notifications.showNotification({ + title: visibleTitle, + body: sensitiveBody, + requireInteraction: false, + silent: false, + })); + + expect(window.Notification).toHaveBeenCalledTimes(1); + const options = window.Notification.mock.calls[0][1]; + expect(options.body).toBe(sensitiveBody); + expect(options.tag).not.toContain('AKIA-SECRET-VALUE'); + expect(options.tag).not.toContain('token='); + expect(options.tag).not.toContain(visibleTitle); + expect(options.tag).toBe(''); + }); + + it('should use the explicit tag identifier when the caller provides one', async () => { + // When a stable opaque id is passed, it must be used verbatim so that + // subsequent updates to the same message replace the previous notification, and + // so that no user-visible text leaks into the tag field at all. + window.Notification.permission = 'granted'; + jest.resetModules(); + Notifications = require('utils/notifications'); + + const sensitiveBody = '@bob: AWS_SECRET_ACCESS_KEY=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY'; + const visibleTitle = '@bob posted in #incident-response'; + const postId = 'post-9bg7p4dyitggimxtxctt7gwp4y'; + + await store.dispatch(Notifications.showNotification({ + title: visibleTitle, + body: sensitiveBody, + tag: postId, + requireInteraction: false, + silent: false, + })); + + expect(window.Notification).toHaveBeenCalledTimes(1); + const options = window.Notification.mock.calls[0][1]; + expect(options.tag).toBe(postId); + expect(options.tag).not.toContain('AWS_SECRET_ACCESS_KEY'); + expect(options.tag).not.toContain(visibleTitle); + expect(options.body).toBe(sensitiveBody); + }); + it('should do nothing if permissions previously requested but not granted', async () => { window.Notification.requestPermission.mockResolvedValue('denied'); diff --git a/webapp/channels/src/utils/notifications.ts b/webapp/channels/src/utils/notifications.ts index a6f2b2fe107..f2beb121010 100644 --- a/webapp/channels/src/utils/notifications.ts +++ b/webapp/channels/src/utils/notifications.ts @@ -23,6 +23,14 @@ let requestedNotificationPermission = Boolean('Notification' in window && Notifi export interface ShowNotificationParams { title: string; body: string; + + /** + * Opaque, non-content identifier used as the Web Notifications API tag. + * Callers may pass a stable id when they need replacement semantics. When + * omitted, the tag is left empty so no user-visible notification text reaches + * the tag field (see #36297 / MM-68537). + */ + tag?: string; requireInteraction: boolean; silent: boolean; onClick?: (this: Notification, e: Event) => any | null; @@ -32,6 +40,7 @@ export function showNotification( { title, body, + tag, requireInteraction, silent, onClick, @@ -71,7 +80,15 @@ export function showNotification( const notification = new Notification(title, { body, - tag: body, + + // Use the explicit opaque tag when the caller provides one; otherwise keep it empty. + // Notification text must never reach the tag field: + // Chromium-based browsers serialise tag into the notification + // activation command line via --notification-launch-id + // (https://notifications.spec.whatwg.org/#dom-notification-tag), where endpoint + // detection tools log it and ship it to customer SIEM pipelines that were never in + // scope to receive chat content. See #36297 / MM-68537. + tag: tag ?? '', icon: icon50, requireInteraction, silent,