Merge pull request #20750 from overleaf/jpa-issue-20743

[logger] sanitize sentry exception context

GitOrigin-RevId: 7f141ad93fc8df807acad9bcf691027efe799150

libraries/logger/sentry-manager.js

@@ -1,4 +1,4 @@
-const OError = require('@overleaf/o-error')
+const Serializers = require('./serializers')
 const RATE_LIMIT_MAX_ERRORS = 5
 const RATE_LIMIT_INTERVAL_MS = 60000
 
@@ -42,7 +42,7 @@ class SentryManager {
     }
 
     // extract any error object
-    let error = attributes.err || attributes.error
+    let error = Serializers.err(attributes.err || attributes.error)
 
     // avoid reporting errors twice
     for (const key in attributes) {
@@ -67,45 +67,21 @@ class SentryManager {
       const tags = {}
       const extra = {}
       for (const key in attributes) {
-        const value = attributes[key]
+        let value = attributes[key]
+        if (Serializers[key]) {
+          value = Serializers[key](value)
+        }
         if (key.match(/_id/) && typeof value === 'string') {
           tags[key] = value
         }
         extra[key] = value
       }
 
-      // capture req object if available
-      const { req } = attributes
-      if (req != null) {
-        extra.req = {
-          method: req.method,
-          url: req.originalUrl,
-          query: req.query,
-          headers: req.headers,
-          ip: req.ip,
-        }
-      }
-
-      // recreate error objects that have been converted to a normal object
-      if (!(error instanceof Error) && typeof error === 'object') {
-        const newError = new Error(error.message)
-        for (const key of Object.keys(error || {})) {
-          const value = error[key]
-          newError[key] = value
-        }
-        error = newError
-      }
-
       // OError integration
-      extra.info = OError.getFullInfo(error)
+      extra.info = error.info
+      delete error.info
 
-      // filter paths from the message to avoid duplicate errors in sentry
-      // (e.g. errors from `fs` methods which have a path attribute)
       try {
-        if (error.path) {
-          error.message = error.message.replace(` '${error.path}'`, '')
-        }
-
         // send the error to sentry
         this.Sentry.captureException(error, { tags, extra, level })
 

libraries/logger/serializers.js

@@ -4,13 +4,20 @@ function errSerializer(err) {
   if (!err) {
     return err
   }
+  let message = err.message
+  if (err.path) {
+    // filter paths from the message to avoid duplicate errors with different path in message
+    // (e.g. errors from `fs` methods which have a path attribute)
+    message = message.replace(` '${err.path}'`, '')
+  }
   return {
-    message: err.message,
+    message,
     name: err.name,
     stack: OError.getFullStack(err),
     info: OError.getFullInfo(err),
     code: err.code,
     signal: err.signal,
+    path: err.path,
   }
 }
 

libraries/logger/test/unit/sentry-manager-tests.js

@@ -104,6 +104,116 @@ describe('SentryManager', function () {
       )
     })
 
+    it('should sanitize error', function () {
+      const err = {
+        name: 'CustomError',
+        message: 'hello',
+        _oErrorTags: [{ stack: 'here:1', info: { one: 1 } }],
+        stack: 'here:0',
+        info: { key: 'value' },
+        code: 42,
+        signal: 9,
+        path: '/foo',
+      }
+      this.sentryManager.captureException({ err }, 'message', 'error')
+      const expectedErr = {
+        name: 'CustomError',
+        message: 'hello',
+        stack: 'here:0\nhere:1',
+        code: 42,
+        signal: 9,
+        path: '/foo',
+      }
+      expect(this.Sentry.captureException).to.have.been.calledWith(
+        sinon.match(expectedErr),
+        sinon.match({
+          tags: sinon.match.any,
+          level: sinon.match.any,
+          extra: {
+            description: 'message',
+            info: sinon.match({
+              one: 1,
+              key: 'value',
+            }),
+          },
+        })
+      )
+      expect(this.Sentry.captureException.args[0][0]).to.deep.equal(expectedErr)
+    })
+    it('should sanitize request', function () {
+      const req = {
+        ip: '1.2.3.4',
+        method: 'GET',
+        url: '/foo',
+        headers: {
+          referer: 'abc',
+          'content-length': 1337,
+          'user-agent': 'curl',
+          authorization: '42',
+        },
+      }
+      this.sentryManager.captureException({ req }, 'message', 'error')
+      const expectedReq = {
+        remoteAddress: '1.2.3.4',
+        method: 'GET',
+        url: '/foo',
+        headers: {
+          referer: 'abc',
+          'content-length': 1337,
+          'user-agent': 'curl',
+        },
+      }
+      expect(this.Sentry.captureException).to.have.been.calledWith(
+        sinon.match({
+          message: 'message',
+        }),
+        sinon.match({
+          tags: sinon.match.any,
+          level: sinon.match.any,
+          extra: {
+            info: sinon.match.any,
+            req: expectedReq,
+          },
+        })
+      )
+      expect(this.Sentry.captureException.args[0][1].extra.req).to.deep.equal(
+        expectedReq
+      )
+    })
+    it('should sanitize response', function () {
+      const res = {
+        statusCode: 417,
+        body: Buffer.from('foo'),
+        getHeader(key) {
+          expect(key).to.be.oneOf(['content-length'])
+          if (key === 'content-length') return 1337
+        },
+      }
+      this.sentryManager.captureException({ res }, 'message', 'error')
+      const expectedRes = {
+        statusCode: 417,
+        headers: {
+          'content-length': 1337,
+        },
+      }
+      expect(this.Sentry.captureException).to.have.been.calledWith(
+        sinon.match({
+          message: 'message',
+        }),
+        sinon.match({
+          tags: sinon.match.any,
+          level: sinon.match.any,
+          extra: {
+            info: sinon.match.any,
+            res: expectedRes,
+          },
+        })
+      )
+      expect(this.Sentry.captureException.args[0][1].extra.res).to.deep.equal(
+        expectedRes
+      )
+    })
+
     describe('reportedToSentry', function () {
       it('should mark the error as reported to sentry', function () {
         const err = new Error()