Future-proof redaction options

src/Transport.ts

@@ -74,8 +74,7 @@ import {
   kJsonContentType,
   kNdjsonContentType,
   kAcceptHeader,
-  kRedactConnection,
-  kAdditionalRedactionKeys
+  kRedaction
 } from './symbols'
 
 const { version: clientVersion } = require('../package.json') // eslint-disable-line
@@ -112,8 +111,7 @@ export interface TransportOptions {
     ndjsonContentType?: string
     accept?: string
   }
-  redactConnection?: boolean
-  additionalRedactionKeys?: string[]
+  redaction?: RedactionOptions
 }
 
 export interface TransportRequestParams {
@@ -160,8 +158,7 @@ export interface TransportRequestOptions {
     * ```
     */
   meta?: boolean
-  redactConnection?: boolean
-  additionalRedactionKeys?: string[]
+  redaction?: RedactionOptions
 }
 
 export interface TransportRequestOptionsWithMeta extends TransportRequestOptions {
@@ -183,6 +180,11 @@ export interface SniffOptions {
   context: any
 }
 
+export interface RedactionOptions {
+  type: 'off' | 'replace' | 'remove'
+  additionalKeys?: string[]
+}
+
 export default class Transport {
   [kNodeFilter]: nodeFilterFn
   [kNodeSelector]: nodeSelectorFn
@@ -209,8 +211,7 @@ export default class Transport {
   [kJsonContentType]: string
   [kNdjsonContentType]: string
   [kAcceptHeader]: string
-  [kRedactConnection]: boolean
-  [kAdditionalRedactionKeys]: string[]
+  [kRedaction]: RedactionOptions
 
   static sniffReasons = {
     SNIFF_ON_START: 'sniff-on-start',
@@ -270,8 +271,7 @@ export default class Transport {
     this[kJsonContentType] = opts.vendoredHeaders?.jsonContentType ?? 'application/json'
     this[kNdjsonContentType] = opts.vendoredHeaders?.ndjsonContentType ?? 'application/x-ndjson'
     this[kAcceptHeader] = opts.vendoredHeaders?.accept ?? 'application/json, text/plain'
-    this[kRedactConnection] = typeof opts.redactConnection === 'boolean' ? opts.redactConnection : false
-    this[kAdditionalRedactionKeys] = Array.isArray(opts.additionalRedactionKeys) ? opts.additionalRedactionKeys : []
+    this[kRedaction] = opts.redaction ?? { type: 'replace', additionalKeys: [] }
 
     if (opts.sniffOnStart === true) {
       this.sniff({
@@ -373,8 +373,7 @@ export default class Transport {
     const maxCompressedResponseSize = options.maxCompressedResponseSize ?? this[kMaxCompressedResponseSize]
 
     const errorOptions: ErrorOptions = {
-      redactConnection: typeof options.redactConnection === 'boolean' ? options.redactConnection : this[kRedactConnection],
-      additionalRedactionKeys: Array.isArray(options.additionalRedactionKeys) ? options.additionalRedactionKeys : this[kAdditionalRedactionKeys]
+      redaction: typeof options.redaction === 'object' ? options.redaction : this[kRedaction]
     }
 
     this[kDiagnostic].emit('serialization', null, result)

src/errors.ts

@@ -19,11 +19,11 @@
 
 import * as http from 'http'
 import { DiagnosticResult } from './types'
-import { redactObject } from './security'
+import { RedactionOptions } from './Transport'
+import { redactDiagnostic } from './security'
 
 export interface ErrorOptions {
-  redactConnection: boolean
-  additionalRedactionKeys: string[]
+  redaction: RedactionOptions
 }
 
 export class ElasticsearchClientError extends Error {
@@ -33,12 +33,14 @@ export class ElasticsearchClientError extends Error {
     this.name = 'ElasticsearchClientError'
 
     this.options = {
-      redactConnection: false,
-      additionalRedactionKeys: []
+      redaction: {
+        type: 'replace',
+        additionalKeys: []
+      }
     }
 
     if (isObject(options)) {
-      this.options = { ...this.options, ...options }
+      this.options.redaction = { ...this.options.redaction, ...options.redaction }
     }
   }
 }
@@ -51,13 +53,7 @@ export class TimeoutError extends ElasticsearchClientError {
     this.name = 'TimeoutError'
     this.message = message ?? 'Timeout Error'
 
-    if (isObject(meta)) {
-      if (this.options.redactConnection) {
-        meta.meta.connection = null
-      }
-
-      meta = redactObject(meta, this.options.additionalRedactionKeys) as DiagnosticResult
-    }
+    if (isObject(meta)) meta = redactDiagnostic(meta, this.options.redaction)
     this.meta = meta
   }
 }
@@ -70,13 +66,7 @@ export class ConnectionError extends ElasticsearchClientError {
     this.name = 'ConnectionError'
     this.message = message ?? 'Connection Error'
 
-    if (isObject(meta)) {
-      if (this.options.redactConnection) {
-        meta.meta.connection = null
-      }
-
-      meta = redactObject(meta, this.options.additionalRedactionKeys ?? []) as DiagnosticResult
-    }
+    if (isObject(meta)) meta = redactDiagnostic(meta, this.options.redaction)
     this.meta = meta
   }
 }
@@ -89,12 +79,7 @@ export class NoLivingConnectionsError extends ElasticsearchClientError {
     this.name = 'NoLivingConnectionsError'
     this.message = message ?? 'Given the configuration, the ConnectionPool was not able to find a usable Connection for this request.'
 
-    if (this.options.redactConnection) {
-      meta.meta.connection = null
-    }
-
-    meta = redactObject(meta, this.options.additionalRedactionKeys ?? []) as DiagnosticResult
-    this.meta = meta
+    this.meta = redactDiagnostic(meta, this.options.redaction)
   }
 }
 
@@ -166,12 +151,7 @@ export class ResponseError extends ElasticsearchClientError {
       this.message = meta.body as string ?? 'Response Error'
     }
 
-    if (this.options.redactConnection) {
-      meta.meta.connection = null
-    }
-
-    meta = redactObject(meta, this.options.additionalRedactionKeys ?? []) as DiagnosticResult
-    this.meta = meta
+    this.meta = redactDiagnostic(meta, this.options.redaction)
   }
 
   get body (): any | undefined {
@@ -198,13 +178,7 @@ export class RequestAbortedError extends ElasticsearchClientError {
     this.name = 'RequestAbortedError'
     this.message = message ?? 'Request aborted'
 
-    if (isObject(meta)) {
-      if (this.options.redactConnection) {
-        meta.meta.connection = null
-      }
-
-      meta = redactObject(meta, this.options.additionalRedactionKeys ?? []) as DiagnosticResult
-    }
+    if (isObject(meta)) meta = redactDiagnostic(meta, this.options.redaction)
     this.meta = meta
   }
 }
@@ -217,13 +191,7 @@ export class ProductNotSupportedError extends ElasticsearchClientError {
     this.name = 'ProductNotSupportedError'
     this.message = `The client noticed that the server is not ${product} and we do not support this unknown product.`
 
-    if (isObject(meta)) {
-      if (this.options.redactConnection) {
-        meta.meta.connection = null
-      }
-
-      meta = redactObject(meta, this.options.additionalRedactionKeys ?? []) as DiagnosticResult
-    }
+    if (isObject(meta)) meta = redactDiagnostic(meta, this.options.redaction)
     this.meta = meta
   }
 }

src/security.ts

@@ -16,6 +16,8 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+import { DiagnosticResult } from './types'
+import { RedactionOptions } from './Transport'
 
 const secretKeys = [
   'authorization',
@@ -57,13 +59,36 @@ export function redactObject (obj: Record<string, any>, additionalKeys: string[]
 
       // check if redaction is needed for this key
       if (toRedact.includes(key.toLowerCase())) {
-        // Object.defineProperty getter ensures the property can be directly accessed just as before,
-        // but hides it from all serialization methods. Cheers to @delvedor for the idea.
-        Object.defineProperty(newObj, key, { get: () => value })
+        newObj[key] = '[redacted]'
       } else {
         newObj[key] = value
       }
     })
     return newObj
   }
 }
+
+/**
+ * Redacts a DiagnosticResult object using the provided options.
+ * - 'off' does nothing
+ * - 'remove' removes most optional properties, replaces non-optional properties with the simplest possible alternative
+ * - 'replace' runs `redactObject`, which replaces secret keys with `[redacted]`
+ */
+export function redactDiagnostic (diag: DiagnosticResult, options: RedactionOptions): DiagnosticResult {
+  switch (options.type) {
+    case 'off':
+      break
+    case 'remove':
+      delete diag.headers
+      delete diag.meta.sniff
+      delete diag.meta.request.params.headers
+      diag.meta.request.options = {}
+      diag.meta.connection = null
+      break
+    case 'replace':
+      diag = redactObject(diag, options.additionalKeys ?? []) as DiagnosticResult
+      break
+  }
+
+  return diag
+}

src/symbols.ts

@@ -46,5 +46,4 @@ export const kMaxCompressedResponseSize = Symbol('max compressed response size')
 export const kJsonContentType = Symbol('json content type')
 export const kNdjsonContentType = Symbol('ndjson content type')
 export const kAcceptHeader = Symbol('accept header')
-export const kRedactConnection = Symbol('redact connection')
-export const kAdditionalRedactionKeys = Symbol('additional redaction keys')
+export const kRedaction = Symbol('redaction')

test/unit/errors.test.ts

@@ -98,12 +98,11 @@ function errFactory (message: string, meta: DiagnosticResult, options: errors.Er
   ]
 }
 
-test('redact sensitive data when logging errors', t => {
+test('replace sensitive data for redaction type "replace"', t => {
   const diags = makeDiagnostics()
 
-  const errorOptions = {
-    redactConnection: false,
-    additionalRedactionKeys: []
+  const errorOptions: errors.ErrorOptions = {
+    redaction: { type: 'replace' }
   }
 
   diags.forEach(diag => {
@@ -118,18 +117,21 @@ test('redact sensitive data when logging errors', t => {
   t.end()
 })
 
-test('redact entire connection if redactConnection is true', t => {
+test('strip most optional properties when redaction type is "remove"', t => {
   const diags = makeDiagnostics()
 
-  const errorOptions = {
-    redactConnection: true,
-    additionalRedactionKeys: []
+  const errorOptions: errors.ErrorOptions = {
+    redaction: { type: 'remove' }
   }
 
   diags.forEach(diag => {
     errFactory('err', diag, errorOptions).forEach(err => {
       if (err.meta !== undefined) {
-        t.equal(err.meta.meta.connection, null, `${err.name} should redact the connection object`)
+        t.equal(typeof err.meta.headers, 'undefined', `${err.name} should remove meta.headers`)
+        t.equal(typeof err.meta.meta.sniff, 'undefined', `${err.name} should remove meta.meta.sniff`)
+        t.equal(typeof err.meta.meta.request.params.headers, 'undefined', `${err.name} should remove meta.meta.request.params.headers`)
+        t.same(err.meta.meta.request.options, {}, `${err.name} should remove meta.meta.request.options`)
+        t.equal(err.meta.meta.connection, null, `${err.name} should remove the connection object`)
       } else {
         t.fail('should not be called')
       }
@@ -142,9 +144,11 @@ test('redact entire connection if redactConnection is true', t => {
 test('redact extra keys when passed', t => {
   const diags = makeDiagnostics()
 
-  const errorOptions = {
-    redactConnection: false,
-    additionalRedactionKeys: ['X-Another-Header']
+  const errorOptions: errors.ErrorOptions = {
+    redaction: {
+      type: 'replace',
+      additionalKeys: ['X-Another-Header']
+    }
   }
 
   diags.forEach(diag => {

test/unit/security.test.ts

@@ -56,18 +56,6 @@ test('redactObject', t => {
     t.notMatch(inspect(result.foo.bar.baz.bat.bit.but.biz.fiz), "'x-elastic-app-auth': 'abcd1234'")
   })
 
-
-  t.test('Object.keys does not expose secret', t => {
-    const result = redactObject({
-      authorization: 'secret1',
-      password: 'secret2'
-    })
-
-    t.notOk(Object.keys(result).includes('authorization'))
-    t.notOk(Object.keys(result).includes('password'))
-    t.end()
-  })
-
   t.test('Object.values does not expose secret', t => {
     const result = redactObject({
       authorization: 'secret1',
@@ -86,34 +74,12 @@ test('redactObject', t => {
     })
 
     Object.entries(result).forEach(([key, value]) => {
-      t.not(key, 'apiKey')
-      t.not(key, 'x-elastic-app-auth')
       t.not(value, 'secret1')
       t.not(value, 'secret2')
     })
     t.end()
   })
 
-  t.test('for..in loop does not expose secret', t => {
-    const result = redactObject({
-      authorization: 'secret-a',
-      password: 'secret-b',
-    })
-
-    for (const key in result) {
-      t.not(key, 'authorization')
-      t.not(key, 'password')
-    }
-    t.end()
-  })
-
-  t.test('keeps actual values accessible', t => {
-    t.plan(2)
-    const result = redactObject({ password: 'secret' })
-    t.equal(JSON.stringify(result), '{}')
-    t.equal(result.password, 'secret')
-  })
-
   t.test('does not redact keys that do not match', t => {
     t.plan(4)
     const result = redactObject({