From 87b4f34113c52bf0b1771fcf8587e73ac9ba5ba0 Mon Sep 17 00:00:00 2001
From: "Brian M. Carlson" <brian.m.carlson@gmail.com>
Date: Fri, 10 Jan 2020 15:19:08 -0600
Subject: [PATCH 1/5] 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.
---
 packages/pg-pool/index.js                     | 53 ++++++++++++-------
 packages/pg/lib/client.js                     | 16 +++++-
 packages/pg/lib/connection-parameters.js      | 13 ++++-
 packages/pg/lib/native/client.js              | 18 +++++--
 .../test/integration/gh-issues/2064-tests.js  | 32 +++++++++++
 5 files changed, 108 insertions(+), 24 deletions(-)
 create mode 100644 packages/pg/test/integration/gh-issues/2064-tests.js

diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js
index 1c7faf210..33bd1fe49 100644
--- a/packages/pg-pool/index.js
+++ b/packages/pg-pool/index.js
@@ -12,7 +12,7 @@ const removeWhere = (list, predicate) => {
 }
 
 class IdleItem {
-  constructor (client, idleListener, timeoutId) {
+  constructor(client, idleListener, timeoutId) {
     this.client = client
     this.idleListener = idleListener
     this.timeoutId = timeoutId
@@ -20,12 +20,12 @@ class IdleItem {
 }
 
 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
   }
 }
diff --git a/packages/pg/lib/client.js b/packages/pg/lib/client.js
index 93807e48c..3f72e291a 100644
--- a/packages/pg/lib/client.js
+++ b/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 || {}
diff --git a/packages/pg/lib/connection-parameters.js b/packages/pg/lib/connection-parameters.js
index 0d5e0376d..396e95d2e 100644
--- a/packages/pg/lib/connection-parameters.js
+++ b/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)
diff --git a/packages/pg/lib/native/client.js b/packages/pg/lib/native/client.js
index 6859bc2cc..d9c781f5c 100644
--- a/packages/pg/lib/native/client.js
+++ b/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)
   }
diff --git a/packages/pg/test/integration/gh-issues/2064-tests.js b/packages/pg/test/integration/gh-issues/2064-tests.js
new file mode 100644
index 000000000..64c150bd0
--- /dev/null
+++ b/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);
+})

From 10f3a41838e47f26fa928d595ebf09663d584ccf Mon Sep 17 00:00:00 2001
From: "Brian M. Carlson" <brian.m.carlson@gmail.com>
Date: Mon, 13 Jan 2020 09:03:58 -0600
Subject: [PATCH 2/5] Implement feedback

---
 .gitignore                               |  1 +
 packages/pg-pool/index.js                | 54 +++++++++++-------------
 packages/pg/lib/client.js                | 10 ++---
 packages/pg/lib/connection-parameters.js |  5 +--
 4 files changed, 30 insertions(+), 40 deletions(-)

diff --git a/.gitignore b/.gitignore
index df95fda07..bae2a20a1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,3 +7,4 @@ package-lock.json
 *.swp
 dist
 .DS_Store
+.vscode/
diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js
index 33bd1fe49..42731d3c6 100644
--- a/packages/pg-pool/index.js
+++ b/packages/pg-pool/index.js
@@ -12,7 +12,7 @@ const removeWhere = (list, predicate) => {
 }
 
 class IdleItem {
-  constructor(client, idleListener, timeoutId) {
+  constructor (client, idleListener, timeoutId) {
     this.client = client
     this.idleListener = idleListener
     this.timeoutId = timeoutId
@@ -20,12 +20,12 @@ class IdleItem {
 }
 
 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,24 +57,18 @@ function makeIdleListener(pool, client) {
 }
 
 class Pool extends EventEmitter {
-  constructor(options, Client) {
+  constructor (options, Client) {
     super()
-
+    this.options = Object.assign({}, options)
+    const password = this.options.password
     // "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', {
+    Object.defineProperty(this.options, 'password', {
+      configurable: true,
       enumerable: false,
-      get() {
-        return password
-      },
-      set(value) {
-        password = value
-      }
+      value: password,
+      writable: true
     })
-
-    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
@@ -92,11 +86,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')
@@ -139,7 +133,7 @@ class Pool extends EventEmitter {
     throw new Error('unexpected condition')
   }
 
-  _remove(client) {
+  _remove (client) {
     const removed = removeWhere(
       this._idle,
       item => item.client === client
@@ -154,7 +148,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)
@@ -200,7 +194,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)
@@ -248,7 +242,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)
     }
@@ -292,7 +286,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) {
@@ -314,7 +308,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)
@@ -367,7 +361,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')
@@ -380,15 +374,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
   }
 }
diff --git a/packages/pg/lib/client.js b/packages/pg/lib/client.js
index 3f72e291a..fe1ce8baf 100644
--- a/packages/pg/lib/client.js
+++ b/packages/pg/lib/client.js
@@ -33,16 +33,12 @@ var Client = function (config) {
 
   // "hiding" the password so it doesn't show up in stack traces
   // or if the client is console.logged
-  let password = this.connectionParameters.password
+  const password = this.connectionParameters.password
   Object.defineProperty(this, 'password', {
     enumerable: false,
     configurable: false,
-    get() {
-      return password
-    },
-    set(value) {
-      password = value
-    }
+    writable: true,
+    value: password
   })
 
   this.replication = this.connectionParameters.replication
diff --git a/packages/pg/lib/connection-parameters.js b/packages/pg/lib/connection-parameters.js
index 396e95d2e..b00d3f1a3 100644
--- a/packages/pg/lib/connection-parameters.js
+++ b/packages/pg/lib/connection-parameters.js
@@ -61,9 +61,8 @@ var ConnectionParameters = function (config) {
   Object.defineProperty(this, 'password', {
     enumerable: false,
     configurable: false,
-    get() {
-      return password
-    }
+    writable: false,
+    value: password
   })
 
   this.binary = val('binary', config)

From 040b77eac343c0276ec6a6794a002abe58280b82 Mon Sep 17 00:00:00 2001
From: "Brian M. Carlson" <brian.m.carlson@gmail.com>
Date: Mon, 13 Jan 2020 09:05:54 -0600
Subject: [PATCH 3/5] Fix more whitespace the autoformatter changed

---
 packages/pg/lib/native/client.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/pg/lib/native/client.js b/packages/pg/lib/native/client.js
index d9c781f5c..9e1c8baf7 100644
--- a/packages/pg/lib/native/client.js
+++ b/packages/pg/lib/native/client.js
@@ -199,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)
@@ -292,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)
   }

From 26e3a755417ca194fb01d5eeeb48f4185cefd0cd Mon Sep 17 00:00:00 2001
From: "Brian M. Carlson" <brian.m.carlson@gmail.com>
Date: Mon, 13 Jan 2020 09:59:39 -0600
Subject: [PATCH 4/5] Simplify code a bit

---
 packages/pg/lib/native/client.js | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/packages/pg/lib/native/client.js b/packages/pg/lib/native/client.js
index 9e1c8baf7..d06166573 100644
--- a/packages/pg/lib/native/client.js
+++ b/packages/pg/lib/native/client.js
@@ -46,15 +46,11 @@ var Client = module.exports = function (config) {
 
   // "hiding" the password so it doesn't show up in stack traces
   // or if the client is console.logged
-  let hiddenPassword = cp.password
+  const hiddenPassword = cp.password
   Object.defineProperty(this, 'password', {
     enumerable: false,
-    get() {
-      return hiddenPassword
-    },
-    set(value) {
-      hiddenPassword = value
-    }
+    writable: true,
+    value: hiddenPassword
   })
   this.database = cp.database
   this.host = cp.host

From 1091d7c46084a3d858c84a86d799ab884c5ce19f Mon Sep 17 00:00:00 2001
From: Charmander <~@charmander.me>
Date: Mon, 13 Jan 2020 11:44:37 -0800
Subject: [PATCH 5/5] Remove password from stringified outputs (#2070)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Keep ConnectionParameters’s password property writable

`Client` writes to it when `password` is a function.

* Avoid creating password property on pool options

when it didn’t exist previously.

* Allow password option to be non-enumerable

to avoid breaking uses like `new Pool(existingPool.options)`.

* Make password property definitions consistent

in formatting and configurability.
---
 packages/pg-pool/index.js                | 21 ++++++++++++---------
 packages/pg/lib/client.js                |  5 ++---
 packages/pg/lib/connection-parameters.js |  7 +++----
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js
index 42731d3c6..dd0d478d2 100644
--- a/packages/pg-pool/index.js
+++ b/packages/pg-pool/index.js
@@ -60,15 +60,18 @@ class Pool extends EventEmitter {
   constructor (options, Client) {
     super()
     this.options = Object.assign({}, options)
-    const password = this.options.password
-    // "hiding" the password so it doesn't show up in stack traces
-    // or if the client is console.logged
-    Object.defineProperty(this.options, 'password', {
-      configurable: true,
-      enumerable: false,
-      value: password,
-      writable: true
-    })
+
+    if (options != null && 'password' in options) {
+      // "hiding" the password so it doesn't show up in stack traces
+      // or if the client is console.logged
+      Object.defineProperty(this.options, 'password', {
+        configurable: true,
+        enumerable: false,
+        writable: true,
+        value: options.password
+      })
+    }
+
     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
diff --git a/packages/pg/lib/client.js b/packages/pg/lib/client.js
index fe1ce8baf..c929d26f3 100644
--- a/packages/pg/lib/client.js
+++ b/packages/pg/lib/client.js
@@ -33,12 +33,11 @@ var Client = function (config) {
 
   // "hiding" the password so it doesn't show up in stack traces
   // or if the client is console.logged
-  const password = this.connectionParameters.password
   Object.defineProperty(this, 'password', {
+    configurable: true,
     enumerable: false,
-    configurable: false,
     writable: true,
-    value: password
+    value: this.connectionParameters.password
   })
 
   this.replication = this.connectionParameters.replication
diff --git a/packages/pg/lib/connection-parameters.js b/packages/pg/lib/connection-parameters.js
index b00d3f1a3..c0f8498eb 100644
--- a/packages/pg/lib/connection-parameters.js
+++ b/packages/pg/lib/connection-parameters.js
@@ -57,12 +57,11 @@ var ConnectionParameters = function (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', {
+    configurable: true,
     enumerable: false,
-    configurable: false,
-    writable: false,
-    value: password
+    writable: true,
+    value: val('password', config)
   })
 
   this.binary = val('binary', config)