From ddb4670340fb907823c09800f5cd8c89bd22c0b5 Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Tue, 4 Aug 2026 11:29:33 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20unsafe=20notification=20?= =?UTF-8?q?HTML?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ref https://github.com/TryGhost/Ghost/pull/29746 Notification messages are rendered as trusted HTML in Admin. Sanitising on read and write protects both new and previously stored records while preserving semantic update-service content and links. --- .../notifications/notification-email.ts | 4 +- .../services/notifications/notifications.js | 25 +++++++- ...-html.ts => sanitize-notification-html.ts} | 9 ++- .../sanitize-email-html.test.ts.snap | 17 ------ .../notifications/notifications.test.js | 59 +++++++++++++++++++ .../notifications/sanitize-email-html.test.ts | 22 ------- .../sanitize-notification-html.test.ts | 58 ++++++++++++++++++ 7 files changed, 146 insertions(+), 48 deletions(-) rename ghost/core/core/server/services/notifications/{sanitize-email-html.ts => sanitize-notification-html.ts} (67%) delete mode 100644 ghost/core/test/unit/server/services/notifications/__snapshots__/sanitize-email-html.test.ts.snap delete mode 100644 ghost/core/test/unit/server/services/notifications/sanitize-email-html.test.ts create mode 100644 ghost/core/test/unit/server/services/notifications/sanitize-notification-html.test.ts diff --git a/ghost/core/core/server/services/notifications/notification-email.ts b/ghost/core/core/server/services/notifications/notification-email.ts index 1b7a830c595..32f53274337 100644 --- a/ghost/core/core/server/services/notifications/notification-email.ts +++ b/ghost/core/core/server/services/notifications/notification-email.ts @@ -1,4 +1,4 @@ -import {sanitizeEmailHtml} from './sanitize-email-html'; +import {sanitizeNotificationHtml} from './sanitize-notification-html'; interface Mailer { send(options: {to: string; subject: string; html: string; text?: string}): Promise; @@ -48,7 +48,7 @@ export class NotificationEmailService { if (!to.length) { return; } - const message = sanitizeEmailHtml(content); + const message = sanitizeNotificationHtml(content); const siteUrl = this.getSiteUrl(); for (const recipient of to) { const {html, text} = await this.generateEmailContent({ diff --git a/ghost/core/core/server/services/notifications/notifications.js b/ghost/core/core/server/services/notifications/notifications.js index 2647b042f40..59338e68030 100644 --- a/ghost/core/core/server/services/notifications/notifications.js +++ b/ghost/core/core/server/services/notifications/notifications.js @@ -5,10 +5,20 @@ const errors = require('@tryghost/errors'); const ghostVersion = require('@tryghost/version'); const tpl = require('@tryghost/tpl'); const ObjectId = require('bson-objectid').default; +const {sanitizeNotificationHtml} = require('./sanitize-notification-html'); const messages = { noPermissionToDismissNotif: 'You do not have permission to dismiss this notification.', - notificationDoesNotExist: 'Notification does not exist.' + notificationDoesNotExist: 'Notification does not exist.', + invalidNotificationMessage: 'Notification message must be a string.' +}; + +const sanitizeMessage = (message) => { + if (typeof message !== 'string') { + return ''; + } + + return sanitizeNotificationHtml(message); }; class Notifications { @@ -42,6 +52,9 @@ class Notifications { allNotifications.forEach((notification) => { notification.addedAt = moment(notification.addedAt).toDate(); + // Sanitising on read protects installs with notifications that were + // persisted before write-side sanitisation was introduced. + notification.message = sanitizeMessage(notification.message); }); return allNotifications; @@ -145,7 +158,15 @@ class Notifications { }); if (!isDuplicate) { - notificationsToAdd.push(Object.assign({}, defaults, notification, overrides)); + if (typeof notification.message !== 'string') { + throw new errors.ValidationError({ + message: tpl(messages.invalidNotificationMessage) + }); + } + + const notificationToAdd = Object.assign({}, defaults, notification, overrides); + notificationToAdd.message = sanitizeMessage(notificationToAdd.message); + notificationsToAdd.push(notificationToAdd); } }); diff --git a/ghost/core/core/server/services/notifications/sanitize-email-html.ts b/ghost/core/core/server/services/notifications/sanitize-notification-html.ts similarity index 67% rename from ghost/core/core/server/services/notifications/sanitize-email-html.ts rename to ghost/core/core/server/services/notifications/sanitize-notification-html.ts index d2db0fa4279..37a5875955a 100644 --- a/ghost/core/core/server/services/notifications/sanitize-email-html.ts +++ b/ghost/core/core/server/services/notifications/sanitize-notification-html.ts @@ -1,9 +1,8 @@ import sanitizeHtml from 'sanitize-html'; -// Even though the upstream feed is operated by Ghost org, the resulting HTML -// is rendered into admin inboxes on the receiving install. A compromised or -// malformed feed entry must not be able to ship scripts, event handlers, or -// non-http(s) URLs to recipients via this path. +// Notification bodies are rendered as HTML in Ghost Admin and notification +// emails. Keep the content semantic and exclude executable markup, event +// handlers, unsafe URLs, images and inline styles. const ALLOWED_TAGS = [ 'p', 'br', 'hr', 'strong', 'b', 'em', 'i', 'u', 'code', @@ -30,6 +29,6 @@ const SANITIZE_OPTIONS: sanitizeHtml.IOptions = { } }; -export function sanitizeEmailHtml(html: string): string { +export function sanitizeNotificationHtml(html: string): string { return sanitizeHtml(html, SANITIZE_OPTIONS); } diff --git a/ghost/core/test/unit/server/services/notifications/__snapshots__/sanitize-email-html.test.ts.snap b/ghost/core/test/unit/server/services/notifications/__snapshots__/sanitize-email-html.test.ts.snap deleted file mode 100644 index 0ddd160f316..00000000000 --- a/ghost/core/test/unit/server/services/notifications/__snapshots__/sanitize-email-html.test.ts.snap +++ /dev/null @@ -1,17 +0,0 @@ -// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing - -exports[`sanitizeEmailHtml preserves safe formatting and neutralises dangerous content 1 1`] = ` -Object { - "output": " -

Ghost 6.50.0 is now available

-

This release includes a critical fix for an authentication issue. Please update as soon as possible.

- -

Contact security if you have questions.

- -

do not click

-", -} -`; diff --git a/ghost/core/test/unit/server/services/notifications/notifications.test.js b/ghost/core/test/unit/server/services/notifications/notifications.test.js index 75efd962bd6..8b6e01fd045 100644 --- a/ghost/core/test/unit/server/services/notifications/notifications.test.js +++ b/ghost/core/test/unit/server/services/notifications/notifications.test.js @@ -47,9 +47,68 @@ describe('Notifications Service', function () { assert.equal(createdNotification.message, 'Hello test world!'); assert.equal(createdNotification.createdAtVersion, '4.1.0'); }); + + it('sanitizes notification HTML before returning it for storage', function () { + const settingsCache = { + get: sinon.fake.returns([]) + }; + + const notificationsSvc = new Notifications({settingsCache}); + const {notificationsToAdd} = notificationsSvc.add({ + notifications: [{ + message: '

Keep formatting and safe links.

' + }] + }); + + assert.equal(notificationsToAdd[0].message, '

Keep formatting and safe links.

'); + }); + + it('rejects non-string messages', function () { + const settingsCache = { + get: sinon.fake.returns([]) + }; + + const notificationsSvc = new Notifications({settingsCache}); + + assert.throws(() => notificationsSvc.add({ + notifications: [{message: {invalid: true}}] + }), { + message: 'Notification message must be a string.' + }); + }); }); describe('browse', function () { + it('sanitizes notifications that were stored before write sanitization', function () { + const settingsCache = { + get: sinon.fake.returns([{ + custom: true, + message: '

Keep formatting.

', + addedAt: '2021-03-17T01:41:20.906Z' + }]) + }; + + const notificationSvc = new Notifications({settingsCache}); + const notifications = notificationSvc.browse({user: owner}); + + assert.equal(notifications[0].message, '

Keep formatting.

'); + }); + + it('neutralizes non-string messages that were stored previously', function () { + const settingsCache = { + get: sinon.fake.returns([{ + custom: true, + message: {invalid: true}, + addedAt: '2021-03-17T01:41:20.906Z' + }]) + }; + + const notificationSvc = new Notifications({settingsCache}); + const notifications = notificationSvc.browse({user: owner}); + + assert.equal(notifications[0].message, ''); + }); + it('can browse non-major version upgrade notifications', function () { const settingsCache = { get: sinon.fake.returns([{ diff --git a/ghost/core/test/unit/server/services/notifications/sanitize-email-html.test.ts b/ghost/core/test/unit/server/services/notifications/sanitize-email-html.test.ts deleted file mode 100644 index 74673b37c27..00000000000 --- a/ghost/core/test/unit/server/services/notifications/sanitize-email-html.test.ts +++ /dev/null @@ -1,22 +0,0 @@ -const {assertMatchSnapshot} = require('../../../../utils/assertions'); -const {sanitizeEmailHtml} = require('../../../../../core/server/services/notifications/sanitize-email-html'); - -// The script tag, on* handler, and javascript: URL are deliberate negative -// cases — the snapshot must show them stripped. -const FIXTURE_MESSAGE_HTML = ` -

Ghost 6.50.0 is now available

-

This release includes a critical fix for an authentication issue. Please update as soon as possible.

- -

Contact security if you have questions.

- -

do not click

-`; - -describe('sanitizeEmailHtml', function () { - it('preserves safe formatting and neutralises dangerous content', function () { - assertMatchSnapshot({output: sanitizeEmailHtml(FIXTURE_MESSAGE_HTML)}); - }); -}); diff --git a/ghost/core/test/unit/server/services/notifications/sanitize-notification-html.test.ts b/ghost/core/test/unit/server/services/notifications/sanitize-notification-html.test.ts new file mode 100644 index 00000000000..d241bafa3e8 --- /dev/null +++ b/ghost/core/test/unit/server/services/notifications/sanitize-notification-html.test.ts @@ -0,0 +1,58 @@ +import assert from 'node:assert/strict'; +import {sanitizeNotificationHtml} from '../../../../../core/server/services/notifications/sanitize-notification-html'; + +const RELEASE_NOTIFICATION = 'Ghost v6.55.0 has been released, click here to upgrade.'; + +const CRITICAL_NOTIFICATION = ` +
+
+
+ Ghost +
+

Critical Ghost security update

+

Hi there,

+

A critical security update for Ghost has been released that patches recently reported vulnerabilities. Please update your Ghost install to the latest version as soon as possible, and consider resetting your authentication credentials:

+

ghost.org/help/auth-reset

+

Full security advisories are published here:

+

github.com/TryGhost/Ghost/security/advisories

+

How to update: Docker or Ghost-CLI.

+

Sent to administrators of Ghost sites.

+
+
+`; + +describe('sanitizeNotificationHtml', function () { + it('preserves release notification links', function () { + const output = sanitizeNotificationHtml(RELEASE_NOTIFICATION); + + assert.equal(output, 'Ghost v6.55.0 has been released, click here to upgrade.'); + }); + + it('preserves the semantic content and links used by critical notifications', function () { + const output = sanitizeNotificationHtml(CRITICAL_NOTIFICATION); + + assert.equal( + output.replace(/\s+/g, ' ').trim(), + '

Critical Ghost security update

Hi there,

A critical security update for Ghost has been released that patches recently reported vulnerabilities. Please update your Ghost install to the latest version as soon as possible, and consider resetting your authentication credentials:

ghost.org/help/auth-reset

Full security advisories are published here:

github.com/TryGhost/Ghost/security/advisories

How to update: Docker or Ghost-CLI.

Sent to administrators of Ghost sites.

' + ); + }); + + it('removes executable markup, unsafe URLs, images and inline styles', function () { + const output = sanitizeNotificationHtml(` + + + Unsafe link + `); + + assert.doesNotMatch(output, /script|onerror|onclick|javascript:|style|tracker\.png|Unsafe link<\/a>/); + }); + + it('is idempotent for malformed namespaced markup', function () { + const input = '

Keep me

Unsafe linkMath text'; + const output = sanitizeNotificationHtml(input); + + assert.equal(output, '

Keep me

Unsafe linkMath text'); + assert.equal(sanitizeNotificationHtml(output), output); + }); +});