mirror of
https://github.com/mattermost/mattermost.git
synced 2026-09-24 16:05:00 +08:00
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 <vishal.kr.singh2021@gmail.com> * 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 <vishal.kr.singh2021@gmail.com> * fix: explain desktop notification path * Avoid title fallback for notification tags Signed-off-by: Vishal Kumar Singh <vishal.kr.singh2021@gmail.com> * test: align notification action payload expectations Signed-off-by: Vishal Kumar Singh <vishal.kr.singh2021@gmail.com> * test: align notification tag e2e expectation --------- Signed-off-by: Vishal Kumar Singh <vishal.kr.singh2021@gmail.com> Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
co-authored by
Mattermost Build
parent
dd69d06dc6
commit
befdb0175a
@@ -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().
|
||||
|
||||
@@ -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),
|
||||
});
|
||||
|
||||
@@ -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<NotificationResult> {
|
||||
export function notifyMe(title: string, body: string, channelId: string, teamId: string, silent: boolean, soundName: string, url: string, postId: string): ActionFuncAsync<NotificationResult> {
|
||||
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: () => {
|
||||
|
||||
@@ -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');
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user