fix: validate notification link when posting

fixes an issue that allows posting notifications containing links with
cross-site scripting attack

Signed-off-by: Heikki Hellgren <heikki.hellgren@op.fi>

.changeset/rotten-crabs-hear.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-notifications-backend': patch
+---
+
+Validate notification link when new notification is created

plugins/notifications-backend/src/plugin.ts

@@ -67,6 +67,7 @@ export const notificationsPlugin = createBackendPlugin({
         database: coreServices.database,
         discovery: coreServices.discovery,
         signals: signalsServiceRef,
+        config: coreServices.rootConfig,
       },
       async init({
         auth,
@@ -77,6 +78,7 @@ export const notificationsPlugin = createBackendPlugin({
         database,
         discovery,
         signals,
+        config,
       }) {
         httpRouter.use(
           await createRouter({
@@ -84,6 +86,7 @@ export const notificationsPlugin = createBackendPlugin({
             httpAuth,
             userInfo,
             logger,
+            config,
             database,
             discovery,
             signals,

plugins/notifications-backend/src/service/router.test.ts

@@ -23,7 +23,8 @@ import request from 'supertest';
 import { createRouter } from './router';
 import { ConfigReader } from '@backstage/config';
 import { SignalsService } from '@backstage/plugin-signals-node';
-import { mockServices } from '@backstage/backend-test-utils';
+import { mockCredentials, mockServices } from '@backstage/backend-test-utils';
+import { NotificationSendOptions } from '@backstage/plugin-notifications-node';
 
 function createDatabase(): PluginDatabaseManager {
   return DatabaseManager.fromConfig(
@@ -47,8 +48,13 @@ describe('createRouter', () => {
 
   const discovery = mockServices.discovery();
   const userInfo = mockServices.userInfo();
-  const httpAuth = mockServices.httpAuth();
+  const httpAuth = mockServices.httpAuth({
+    defaultCredentials: mockCredentials.service(),
+  });
   const auth = mockServices.auth();
+  const config = mockServices.rootConfig({
+    data: { app: { baseUrl: 'http://localhost' } },
+  });
 
   beforeAll(async () => {
     const router = await createRouter({
@@ -57,6 +63,7 @@ describe('createRouter', () => {
       discovery,
       signals: signalService,
       userInfo,
+      config,
       httpAuth,
       auth,
     });
@@ -75,4 +82,80 @@ describe('createRouter', () => {
       expect(response.body).toEqual({ status: 'ok' });
     });
   });
+
+  describe('POST /', () => {
+    const sendNotification = async (data: NotificationSendOptions) =>
+      request(app)
+        .post('/')
+        .send(data)
+        .set('Content-Type', 'application/json')
+        .set('Accept', 'application/json');
+
+    it('returns error on invalid link', async () => {
+      const javascriptXSS = await sendNotification({
+        recipients: {
+          type: 'broadcast',
+        },
+        payload: {
+          title: 'test notification',
+          // eslint-disable-next-line no-script-url
+          link: 'javascript:alert(document.domain)',
+        },
+      });
+
+      expect(javascriptXSS.status).toEqual(400);
+
+      const ftpLink = await sendNotification({
+        recipients: {
+          type: 'broadcast',
+        },
+        payload: {
+          title: 'test notification',
+          link: 'ftp://example.com',
+        },
+      });
+
+      expect(ftpLink.status).toEqual(400);
+    });
+
+    it('should accept absolute http links', async () => {
+      const httpLink = await sendNotification({
+        recipients: {
+          type: 'broadcast',
+        },
+        payload: {
+          title: 'test notification',
+          link: 'http://localhost/test',
+        },
+      });
+
+      expect(httpLink.status).toEqual(200);
+
+      const httpsLink = await sendNotification({
+        recipients: {
+          type: 'broadcast',
+        },
+        payload: {
+          title: 'test notification',
+          link: 'https://example.com',
+        },
+      });
+
+      expect(httpsLink.status).toEqual(200);
+    });
+
+    it('should accept relative links', async () => {
+      const catalogLink = await sendNotification({
+        recipients: {
+          type: 'broadcast',
+        },
+        payload: {
+          title: 'test notification',
+          link: '/catalog',
+        },
+      });
+
+      expect(catalogLink.status).toEqual(200);
+    });
+  });
 });

plugins/notifications-backend/src/service/router.ts

@@ -46,10 +46,12 @@ import {
 } from '@backstage/plugin-notifications-common';
 import { parseEntityOrderFieldParams } from './parseEntityOrderFieldParams';
 import { getUsersForEntityRef } from './getUsersForEntityRef';
+import { Config } from '@backstage/config';
 
 /** @internal */
 export interface RouterOptions {
   logger: LoggerService;
+  config: Config;
   database: PluginDatabaseManager;
   discovery: DiscoveryService;
   auth: AuthService;
@@ -65,6 +67,7 @@ export async function createRouter(
   options: RouterOptions,
 ): Promise<express.Router> {
   const {
+    config,
     logger,
     database,
     auth,
@@ -79,6 +82,7 @@ export async function createRouter(
   const catalogClient =
     catalog ?? new CatalogClient({ discoveryApi: discovery });
   const store = await DatabaseNotificationsStore.create({ database });
+  const frontendBaseUrl = config.getString('app.baseUrl');
 
   const getUser = async (req: Request<unknown>) => {
     const credentials = await httpAuth.credentials(req, { allow: ['user'] });
@@ -177,6 +181,18 @@ export async function createRouter(
     }
   };
 
+  const validateLink = (link: string) => {
+    const stripLeadingSlash = (s: string) => s.replace(/^\//, '');
+    const ensureTrailingSlash = (s: string) => s.replace(/\/?$/, '/');
+    const url = new URL(
+      stripLeadingSlash(link),
+      ensureTrailingSlash(frontendBaseUrl),
+    );
+    if (url.protocol !== 'https:' && url.protocol !== 'http:') {
+      throw new Error('Only HTTP/HTTPS links are allowed');
+    }
+  };
+
   // TODO: Move to use OpenAPI router instead
   const router = Router();
   router.use(express.json());
@@ -419,13 +435,21 @@ export async function createRouter(
         allow: ['service'],
       });
 
-      const { title } = payload;
+      const { title, link } = payload;
 
       if (!recipients || !title) {
         logger.error(`Invalid notification request received`);
         throw new InputError(`Invalid notification request received`);
       }
 
+      if (link) {
+        try {
+          validateLink(link);
+        } catch (e) {
+          throw new InputError('Invalid link provided', e);
+        }
+      }
+
       const origin = credentials.principal.subject;
       const baseNotification = {
         payload: {