From 57810cd34ec8ef0dcdc9c2200599eb81e992a1ce Mon Sep 17 00:00:00 2001 From: Michael Barrett Date: Tue, 23 Jan 2024 08:22:57 +0000 Subject: [PATCH] Added allowlist for Sentry transactions (#19538) refs [ARCH-41](https://linear.app/tryghost/issue/ARCH-41/add-allowlist-for-sentry-transactions) Added allowlist for Sentry transactions so that we can better control the data we are putting into Sentry --- ghost/core/core/shared/sentry.js | 25 +++++++++++++++++++++- ghost/core/test/unit/shared/sentry.test.js | 22 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/ghost/core/core/shared/sentry.js b/ghost/core/core/shared/sentry.js index abf2ad27eb..24f58228e6 100644 --- a/ghost/core/core/shared/sentry.js +++ b/ghost/core/core/shared/sentry.js @@ -57,6 +57,27 @@ const beforeSend = function (event, hint) { } }; +const ALLOWED_HTTP_TRANSACTIONS = [ + '/ghost/api', + '/members/api' +].map((path) => { + // Sentry names HTTP transactions like: " " i.e. "GET /ghost/api/content/settings" + return new RegExp(`^(GET|POST|PUT|DELETE)\\s(?${path}\\/.+)`); +}); + +const beforeSendTransaction = function (event) { + // Drop transactions that are not in the allowed list + for (const transaction of ALLOWED_HTTP_TRANSACTIONS) { + const match = event.transaction.match(transaction); + + if (match?.groups?.path) { + return event; + } + } + + return null; +}; + if (sentryConfig && !sentryConfig.disabled) { const Sentry = require('@sentry/node'); const version = require('@tryghost/version').full; @@ -72,7 +93,8 @@ if (sentryConfig && !sentryConfig.disabled) { environment: environment, maxValueLength: 1000, integrations: [], - beforeSend + beforeSend, + beforeSendTransaction }; // Enable tracing if sentry.tracing.enabled is true @@ -117,6 +139,7 @@ if (sentryConfig && !sentryConfig.disabled) { captureException: Sentry.captureException, captureMessage: Sentry.captureMessage, beforeSend: beforeSend, + beforeSendTransaction: beforeSendTransaction, initQueryTracing: (knex) => { if (sentryConfig.tracing?.enabled === true) { const integration = new SentryKnexTracingIntegration(knex); diff --git a/ghost/core/test/unit/shared/sentry.test.js b/ghost/core/test/unit/shared/sentry.test.js index 05047685a0..84464d3fc7 100644 --- a/ghost/core/test/unit/shared/sentry.test.js +++ b/ghost/core/test/unit/shared/sentry.test.js @@ -155,4 +155,26 @@ describe('UNIT: sentry', function () { assert.deepEqual(result, expected); }); }); + + describe('beforeTransaction', function () { + it('filters transactions based on an allow list', function () { + sentry = require('../../../core/shared/sentry'); + + const beforeSendTransaction = sentry. beforeSendTransaction; + + const allowedTransactions = [ + {transaction: 'GET /ghost/api/settings'}, + {transaction: 'PUT /members/api/member'}, + {transaction: 'POST /ghost/api/tiers'}, + {transaction: 'DELETE /members/api/member'} + ]; + + allowedTransactions.forEach((transaction) => { + assert.equal(beforeSendTransaction(transaction), transaction); + }); + + assert.equal(beforeSendTransaction({transaction: 'GET /foo/bar'}), null); + assert.equal(beforeSendTransaction({transaction: 'Some other transaction'}), null); + }); + }); });