Remove password from stringified outputs

Theres a security concern where if you're not careful and you include your client or pool instance in console.log or stack traces it might include the database password.  To widen the pit of success I'm making that field non-enumerable.  You can still get at it...it just wont show up "by accident" when you're logging things now.

The backwards compatiblity impact of this is very small, but it is still technically somewhat an API change so...8.0.

Author: brianc

packages/pg-pool/index.js

@@ -12,20 +12,20 @@ const removeWhere = (list, predicate) => {
 }
 
 class IdleItem {
-  constructor (client, idleListener, timeoutId) {
+  constructor(client, idleListener, timeoutId) {
     this.client = client
     this.idleListener = idleListener
     this.timeoutId = timeoutId
   }
 }
 
 class PendingItem {
-  constructor (callback) {
+  constructor(callback) {
     this.callback = callback
   }
 }
 
-function promisify (Promise, callback) {
+function promisify(Promise, callback) {
   if (callback) {
     return { callback: callback, result: undefined }
   }
@@ -41,8 +41,8 @@ function promisify (Promise, callback) {
   return { callback: cb, result: result }
 }
 
-function makeIdleListener (pool, client) {
-  return function idleListener (err) {
+function makeIdleListener(pool, client) {
+  return function idleListener(err) {
     err.client = client
 
     client.removeListener('error', idleListener)
@@ -57,9 +57,24 @@ function makeIdleListener (pool, client) {
 }
 
 class Pool extends EventEmitter {
-  constructor (options, Client) {
+  constructor(options, Client) {
     super()
-    this.options = Object.assign({}, options)
+
+    // "hiding" the password so it doesn't show up in stack traces
+    // or if the client is console.logged
+    const optionsWithPasswordHidden = Object.assign({}, options)
+    let password = optionsWithPasswordHidden.password
+    Object.defineProperty(optionsWithPasswordHidden, 'password', {
+      enumerable: false,
+      get() {
+        return password
+      },
+      set(value) {
+        password = value
+      }
+    })
+
+    this.options = optionsWithPasswordHidden
     this.options.max = this.options.max || this.options.poolSize || 10
     this.log = this.options.log || function () { }
     this.Client = this.options.Client || Client || require('pg').Client
@@ -77,11 +92,11 @@ class Pool extends EventEmitter {
     this.ended = false
   }
 
-  _isFull () {
+  _isFull() {
     return this._clients.length >= this.options.max
   }
 
-  _pulseQueue () {
+  _pulseQueue() {
     this.log('pulse queue')
     if (this.ended) {
       this.log('pulse queue ended')
@@ -124,7 +139,7 @@ class Pool extends EventEmitter {
     throw new Error('unexpected condition')
   }
 
-  _remove (client) {
+  _remove(client) {
     const removed = removeWhere(
       this._idle,
       item => item.client === client
@@ -139,7 +154,7 @@ class Pool extends EventEmitter {
     this.emit('remove', client)
   }
 
-  connect (cb) {
+  connect(cb) {
     if (this.ending) {
       const err = new Error('Cannot use a pool after calling end on the pool')
       return cb ? cb(err) : this.Promise.reject(err)
@@ -185,7 +200,7 @@ class Pool extends EventEmitter {
     return result
   }
 
-  newClient (pendingItem) {
+  newClient(pendingItem) {
     const client = new this.Client(this.options)
     this._clients.push(client)
     const idleListener = makeIdleListener(this, client)
@@ -233,7 +248,7 @@ class Pool extends EventEmitter {
   }
 
   // acquire a client for a pending work item
-  _acquireClient (client, pendingItem, idleListener, isNew) {
+  _acquireClient(client, pendingItem, idleListener, isNew) {
     if (isNew) {
       this.emit('connect', client)
     }
@@ -277,7 +292,7 @@ class Pool extends EventEmitter {
 
   // release a client back to the poll, include an error
   // to remove it from the pool
-  _release (client, idleListener, err) {
+  _release(client, idleListener, err) {
     client.on('error', idleListener)
 
     if (err || this.ending) {
@@ -299,7 +314,7 @@ class Pool extends EventEmitter {
     this._pulseQueue()
   }
 
-  query (text, values, cb) {
+  query(text, values, cb) {
     // guard clause against passing a function as the first parameter
     if (typeof text === 'function') {
       const response = promisify(this.Promise, text)
@@ -352,7 +367,7 @@ class Pool extends EventEmitter {
     return response.result
   }
 
-  end (cb) {
+  end(cb) {
     this.log('ending')
     if (this.ending) {
       const err = new Error('Called end on pool more than once')
@@ -365,15 +380,15 @@ class Pool extends EventEmitter {
     return promised.result
   }
 
-  get waitingCount () {
+  get waitingCount() {
     return this._pendingQueue.length
   }
 
-  get idleCount () {
+  get idleCount() {
     return this._idle.length
   }
 
-  get totalCount () {
+  get totalCount() {
     return this._clients.length
   }
 }

packages/pg/lib/client.js

@@ -30,7 +30,21 @@ var Client = function (config) {
   this.database = this.connectionParameters.database
   this.port = this.connectionParameters.port
   this.host = this.connectionParameters.host
-  this.password = this.connectionParameters.password
+
+  // "hiding" the password so it doesn't show up in stack traces
+  // or if the client is console.logged
+  let password = this.connectionParameters.password
+  Object.defineProperty(this, 'password', {
+    enumerable: false,
+    configurable: false,
+    get() {
+      return password
+    },
+    set(value) {
+      password = value
+    }
+  })
+
   this.replication = this.connectionParameters.replication
 
   var c = config || {}

packages/pg/lib/connection-parameters.js

@@ -54,7 +54,18 @@ var ConnectionParameters = function (config) {
   this.database = val('database', config)
   this.port = parseInt(val('port', config), 10)
   this.host = val('host', config)
-  this.password = val('password', config)
+
+  // "hiding" the password so it doesn't show up in stack traces
+  // or if the client is console.logged
+  const password = val('password', config)
+  Object.defineProperty(this, 'password', {
+    enumerable: false,
+    configurable: false,
+    get() {
+      return password
+    }
+  })
+
   this.binary = val('binary', config)
   this.ssl = typeof config.ssl === 'undefined' ? useSsl() : config.ssl
   this.client_encoding = val('client_encoding', config)

packages/pg/lib/native/client.js

@@ -43,7 +43,19 @@ var Client = module.exports = function (config) {
   // for the time being. TODO: deprecate all this jazz
   var cp = this.connectionParameters = new ConnectionParameters(config)
   this.user = cp.user
-  this.password = cp.password
+
+  // "hiding" the password so it doesn't show up in stack traces
+  // or if the client is console.logged
+  let hiddenPassword = cp.password
+  Object.defineProperty(this, 'password', {
+    enumerable: false,
+    get() {
+      return hiddenPassword
+    },
+    set(value) {
+      hiddenPassword = value
+    }
+  })
   this.database = cp.database
   this.host = cp.host
   this.port = cp.port
@@ -187,7 +199,7 @@ Client.prototype.query = function (config, values, callback) {
 
       // we already returned an error,
       // just do nothing if query completes
-      query.callback = () => {}
+      query.callback = () => { }
 
       // Remove from queue
       var index = this._queryQueue.indexOf(query)
@@ -280,7 +292,7 @@ Client.prototype._pulseQueryQueue = function (initialConnection) {
 // attempt to cancel an in-progress query
 Client.prototype.cancel = function (query) {
   if (this._activeQuery === query) {
-    this.native.cancel(function () {})
+    this.native.cancel(function () { })
   } else if (this._queryQueue.indexOf(query) !== -1) {
     this._queryQueue.splice(this._queryQueue.indexOf(query), 1)
   }

packages/pg/test/integration/gh-issues/2064-tests.js

@@ -0,0 +1,32 @@
+
+"use strict"
+const helper = require('./../test-helper')
+const assert = require('assert')
+const util = require('util')
+
+const suite = new helper.Suite()
+
+const password = 'FAIL THIS TEST'
+
+suite.test('Password should not exist in toString() output', () => {
+  const pool = new helper.pg.Pool({ password })
+  const client = new helper.pg.Client({ password })
+  assert(pool.toString().indexOf(password) === -1);
+  assert(client.toString().indexOf(password) === -1);
+})
+
+suite.test('Password should not exist in util.inspect output', () => {
+  const pool = new helper.pg.Pool({ password })
+  const client = new helper.pg.Client({ password })
+  const depth = 20;
+  assert(util.inspect(pool, { depth }).indexOf(password) === -1);
+  assert(util.inspect(client, { depth }).indexOf(password) === -1);
+})
+
+suite.test('Password should not exist in json.stringfy output', () => {
+  const pool = new helper.pg.Pool({ password })
+  const client = new helper.pg.Client({ password })
+  const depth = 20;
+  assert(JSON.stringify(pool).indexOf(password) === -1);
+  assert(JSON.stringify(client).indexOf(password) === -1);
+})