fix: avoid redaction of response headers (#3427)

Make sure that redaction of captured response headers
does not impact the actual headers object in user code.

Refs: #3382
Co-authored-by: Trent Mick <trent.mick@elastic.co>

Author: david-luna

CHANGELOG.asciidoc

@@ -41,6 +41,8 @@ Notes:
 
 * Add support for 'knex' version v1 and v2. ({pull}3355[#3355])
 * Add `tedious@16.x` support. ({pull}3366[#3366])
+* Add `apm.setGlobalLabel()` to dynamically extend the `globalLabels` set in the initial config.
+  Refer to <<apm-set-global-label>> for details. ({pull}3337[#3337])
 
 [float]
 ===== Bug fixes
@@ -60,6 +62,9 @@ code (`import 'elastic-apm-node/start.js'` or `require('elastic-apm-node/start.j
 in a Worker will *not* start the APM agent. Instead, one must use:
 `require('elastic-apm-node').start()` or equivalent.
 
+* Avoid redaction of response headers while extracting `transaction.context.response`
+  data from the HTTP response. Contributed by @lytc. ({pull}3427[#3427])
+
 [float]
 ===== Chores
 
@@ -100,7 +105,6 @@ in a Worker will *not* start the APM agent. Instead, one must use:
   and the APM agent will automatically add a MetricReader to ship metrics to
   APM server. See the <<opentelemetry-bridge>> for details. ({pull}3152[#3152])
 
-* Add `apm.setGlobalLabel()` to dynamically extend the `globalLabels` set in the initial config. Refer to <<apm-set-global-label>> for details. ({pull}3337[#3337])
 
 [float]
 ===== Chores

lib/filters/sanitize-field-names.js

@@ -15,6 +15,11 @@ const REDACTED = require('../constants').REDACTED
  *
  * Express provides multiple body parser middlewares with x-www-form-urlencoded
  * handling.  See http://expressjs.com/en/resources/middleware/body-parser.html
+ *
+ * @param {Object | String} body
+ * @param {Object} requestHeaders
+ * @param {Array<RegExp>} regexes
+ * @returns {Object | String} a copy of the body with the redacted fields
  */
 function redactKeysFromPostedFormVariables (body, requestHeaders, regexes) {
   // only redact from application/x-www-form-urlencoded
@@ -30,31 +35,31 @@ function redactKeysFromPostedFormVariables (body, requestHeaders, regexes) {
   // if body is a string, use querystring to create object,
   // pass to redactKeysFromObject, and reserialize as string
   if (typeof body === 'string') {
-    const objBody = querystring.parse(body)
-    redactKeysFromObject(objBody, regexes)
+    const objBody = redactKeysFromObject(querystring.parse(body), regexes)
     return querystring.stringify(objBody)
   }
 
   return body
 }
 
-function redactKeyFromObject (obj, regex) {
-  for (const [key] of Object.entries(obj)) {
-    if (regex.test(key)) {
-      obj[key] = REDACTED
-    }
-  }
-  return obj
-}
-
+/**
+ * Returns a copy of the provided object. Each entry of the copy will have
+ * its value REDACTEd if the key matches any of the regexes
+ *
+ * @param {Object} obj The source object be copied with redacted fields
+ * @param {Array<RegExp>} regexes RegExps to check if the entry value needd to be redacted
+ * @returns {Object} Copy of the source object with REDACTED entries or the original if falsy or regexes is not an array
+ */
 function redactKeysFromObject (obj, regexes) {
   if (!obj || !Array.isArray(regexes)) {
     return obj
   }
-  for (const [, regex] of regexes.entries()) {
-    redactKeyFromObject(obj, regex)
+  const result = {}
+  for (const key of Object.keys(obj)) {
+    const shouldRedact = regexes.some((regex) => regex.test(key))
+    result[key] = shouldRedact ? REDACTED : obj[key]
   }
-  return obj
+  return result
 }
 
 module.exports = {

lib/parsers.js

@@ -56,10 +56,7 @@ function getContextFromRequest (req, conf, type) {
   }
 
   if (conf.captureHeaders) {
-    context.headers = redactKeysFromObject(
-      Object.assign({}, req.headers),
-      conf.sanitizeFieldNamesRegExp
-    )
+    context.headers = redactKeysFromObject(req.headers, conf.sanitizeFieldNamesRegExp)
 
     // TODO: remove filterHttpHeaders option in next major release
     // https://github.com/elastic/apm-agent-nodejs/issues/3332
@@ -102,6 +99,25 @@ function getContextFromRequest (req, conf, type) {
   return context
 }
 
+/**
+ * Extract appropriate `{transaction,error}.context.response` from an HTTP
+ * response object. This handles header redaction according to the agent config.
+ *
+ * @param {Object} res - Typically `res` is a Node.js `http.OutgoingMessage`
+ *    (https://nodejs.org/api/http.html#class-httpoutgoingmessage).
+ *    However, some cases (e.g. Lambda and Azure Functions instrumentation)
+ *    create a pseudo-res object that matches well enough for this function.
+ *    Some relevant fields: (TODO: document all used fields)
+ *    - `statusCode` - Required. A number.
+ *    - `headers` - An object.
+ *    - `headersSent` - Boolean indicating if the headers have been sent
+ *       (https://nodejs.org/api/http.html#outgoingmessageheaderssent)
+ *    - `finished` - Boolean indicating if `response.end()` has been called
+ *       (https://nodejs.org/api/http.html#responsefinished)
+ * @param {Object} conf - The full agent configuration.
+ * @param {Boolean} isError - Indicates if this response contains an error and
+ *    some extra fields should be added to the context
+ */
 function getContextFromResponse (res, conf, isError) {
   var context = {
     status_code: res.statusCode,

test/parsers.test.js

@@ -87,6 +87,36 @@ test('#getContextFromResponse()', function (t) {
       res.end()
     })
   })
+
+  // Some instrumentations set a pseudo response object, e.g. serverless
+  // functions where the "response" isn't a core Node.js `http.OutgoingMessage`.
+  t.test('for pseudo-res', function (t) {
+    const testCases = [
+      {
+        res: { statusCode: 500 },
+        conf: { captureHeaders: true },
+        isError: false,
+        expectedContext: { status_code: 500, headers: {} }
+      },
+      {
+        res: { statusCode: 500 },
+        conf: { captureHeaders: true },
+        isError: true,
+        expectedContext: { status_code: 500, headers: {}, headers_sent: undefined, finished: undefined }
+      },
+      {
+        res: { statusCode: 200, headers: { 'content-type': 'application/json' } },
+        conf: { captureHeaders: true },
+        isError: false,
+        expectedContext: { status_code: 200, headers: { 'content-type': 'application/json' } }
+      }
+    ]
+    testCases.forEach((tc, idx) => {
+      const context = parsers.getContextFromResponse(tc.res, tc.conf, tc.isError)
+      t.deepEqual(context, tc.expectedContext, `pseudo-res testCase ${idx}`)
+    })
+    t.end()
+  })
 })
 
 test('#getContextFromRequest()', function (t) {

test/sanitize-field-names/main.test.js

@@ -21,20 +21,25 @@ test('redactKeysFromObject tests', function (t) {
     three: 'four',
     five: 'six'
   }
-  redactKeysFromObject(obj1, [/th.*ee/])
-  t.equals(obj1.three, '[REDACTED]', 'key three redacted')
-  t.equals(obj1.one, 'two', 'key one remains in ibject')
-  t.equals(obj1.five, 'six', 'key five remains in object')
+  const redactedObj1 = redactKeysFromObject(obj1, [/th.*ee/])
+  t.equals(redactedObj1.three, '[REDACTED]', 'key three redacted')
+  t.equals(redactedObj1.one, 'two', 'key one remains in ibject')
+  t.equals(redactedObj1.five, 'six', 'key five remains in object')
+  t.ok(obj1 !== redactedObj1, 'redacted object is a copy')
+  t.equals(obj1.three, 'four', 'original object is not redacted')
 
   const obj2 = {
     one: 'two',
     three: 'four',
     five: 'six'
   }
-  redactKeysFromObject(obj2, [/th.*ee/, /three/, /.*five/])
-  t.equals(obj2.three, '[REDACTED]', 'key three redacted')
-  t.equals(obj2.one, 'two', 'key one remains in ibject')
-  t.equals(obj2.five, '[REDACTED]', 'key five redacted')
+  const redactedObj2 = redactKeysFromObject(obj2, [/th.*ee/, /three/, /.*five/])
+  t.equals(redactedObj2.three, '[REDACTED]', 'key three redacted')
+  t.equals(redactedObj2.one, 'two', 'key one remains in ibject')
+  t.equals(redactedObj2.five, '[REDACTED]', 'key five redacted')
+  t.ok(obj2 !== redactedObj2, 'redacted object is a copy')
+  t.equals(obj2.three, 'four', 'original object is not redacted')
+  t.equals(obj2.five, 'six', 'original object is not redacted')
 
   t.end()
 })