Repository: knex/knex
Pull request #5610 (2023-06-27 10:01:52)

Bump pg-connection-string from 2.5.0 to 2.6.1
Bumps [pg-connection-string](https://github.com/brianc/node-postgres/tree/HEAD/packages/pg-connection-string) from 2.5.0 to 2.6.1.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/brianc/node-postgres/blob/master/CHANGELOG.md">pg-connection-string's changelog</a>.</em></p>
<blockquote>
<p>All major and minor releases are briefly explained below.</p>
<p>For richer information consult the commit log on github with referenced pull requests.</p>
<p>We do not include break-fix version release in this file.</p>
<h2>pg-pool@8.10.0</h2>
<ul>
<li>Emit <code>release</code> event when client is returned to <a href="https://redirect.github.com/brianc/node-postgres/pull/2845">the pool</a>.</li>
</ul>
<h2>pg@8.9.0</h2>
<ul>
<li>Add support for <a href="https://redirect.github.com/brianc/node-postgres/pull/2898">stream factory</a>.</li>
<li><a href="https://redirect.github.com/brianc/node-postgres/pull/2901">Better errors</a> for SASL authentication.</li>
<li><a href="https://redirect.github.com/brianc/node-postgres/pull/2815">Use native crypto module</a> for SASL authentication.</li>
</ul>
<h2>pg@8.8.0</h2>
<ul>
<li>Bump minimum required version of <a href="https://redirect.github.com/brianc/node-postgres/pull/2787">native bindings</a>.</li>
<li>Catch previously uncatchable errors thrown in <a href="https://redirect.github.com/brianc/node-postgres/pull/2569"><code>pool.query</code></a>.</li>
<li>Prevent the pool from blocking the event loop if all clients are <a href="https://redirect.github.com/brianc/node-postgres/pull/2721">idle</a> (and <code>allowExitOnIdle</code> is enabled).</li>
<li>Support <code>lock_timeout</code> in <a href="https://redirect.github.com/brianc/node-postgres/pull/2779">client config</a>.</li>
<li>Fix errors thrown in callbacks from <a href="https://redirect.github.com/brianc/node-postgres/pull/2753">interfering with cleanup</a>.</li>
</ul>
<h3>pg-pool@3.5.0</h3>
<ul>
<li>Add connection <a href="https://redirect.github.com/brianc/node-postgres/pull/2698">lifetime limit</a> config option.</li>
</ul>
<h3>pg@8.7.0</h3>
<ul>
<li>Add optional config to <a href="https://redirect.github.com/brianc/node-postgres/pull/2568">pool</a> to allow process to exit if pool is idle.</li>
</ul>
<h3>pg-cursor@2.7.0</h3>
<ul>
<li>Convert to <a href="https://redirect.github.com/brianc/node-postgres/pull/2553">es6 class</a></li>
<li>Add support for promises <a href="https://redirect.github.com/brianc/node-postgres/pull/2554">to cursor methods</a></li>
</ul>
<h3>pg@8.6.0</h3>
<ul>
<li>Better <a href="https://redirect.github.com/brianc/node-postgres/pull/2436">SASL</a> error messages &amp; more validation on bad configuration.</li>
<li>Export <a href="https://redirect.github.com/brianc/node-postgres/pull/2445">DatabaseError</a>.</li>
<li>Add <a href="https://redirect.github.com/brianc/node-postgres/pull/2464">ParameterDescription</a> support to protocol parsing.</li>
<li>Fix typescript <a href="https://redirect.github.com/brianc/node-postgres/pull/2490">typedefs</a> with <code>--isolatedModules</code>.</li>
</ul>
<h3>pg-query-stream@4.0.0</h3>
<ul>
<li>Library has been <a href="https://redirect.github.com/brianc/node-postgres/pull/2376">converted</a> to Typescript. The behavior is identical, but there could be subtle breaking changes due to class names changing or other small inconsistencies introduced by the conversion.</li>
</ul>
<h3>pg@8.5.0</h3>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/brianc/node-postgres/commit/eaafac36dc8f4a13f1fecc9e3420d35559fd8e2b"><code>eaafac3</code></a> Publish</li>
<li><a href="https://github.com/brianc/node-postgres/commit/c38ecf3405d70a06885ff2f6632bb99a53bbb295"><code>c38ecf3</code></a> Fix connection string parsing for overriden hosts (<a href="https://github.com/brianc/node-postgres/tree/HEAD/packages/pg-connection-string/issues/2977">#2977</a>)</li>
<li><a href="https://github.com/brianc/node-postgres/commit/14b840e96e57fc0617b5c4758f6318f774148ee4"><code>14b840e</code></a> Publish</li>
<li><a href="https://github.com/brianc/node-postgres/commit/f305419676afe79a91dc7863289f7f97ac6db3d5"><code>f305419</code></a> Use <code>URL</code> rather than <code>url.parse()</code> in pg-connection-string</li>
<li>See full diff in <a href="https://github.com/brianc/node-postgres/commits/pg-connection-string@2.6.1/packages/pg-connection-string">compare view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=pg-connection-string&package-manager=npm_and_yarn&previous-version=2.5.0&new-version=2.6.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)


</details>

coveralls: 
[![Coverage Status](https://coveralls.io/builds/60983369/badge)](https://coveralls.io/builds/60983369)

coverage: 92.774%. remained the same when pulling **543bf3f758a80daefc555eba654e47fa03ff8d49 on dependabot/npm_and_yarn/pg-connection-string-2.6.1** into **2ed6988d80ac3602cf47a293389f0194f798c912 on master**.


Patch:
diff --git a/lib/client.js b/lib/client.js
index 1a8ec2f5..00ee2893 100644
--- a/lib/client.js
+++ b/lib/client.js
@@ -31,6 +31,7 @@ const { POOL_CONFIG_OPTIONS } = require('./constants');
 const ViewBuilder = require('./schema/viewbuilder.js');
 const ViewCompiler = require('./schema/viewcompiler.js');
 const isPlainObject = require('lodash/isPlainObject');
+const { setHiddenProperty } = require('./util/security.js');
 
 const debug = require('debug')('knex:client');
 
@@ -43,6 +44,10 @@ class Client extends EventEmitter {
     this.config = config;
     this.logger = new Logger(config);
 
+    if (this.config.connection && this.config.connection.password) {
+      setHiddenProperty(this.config.connection);
+    }
+
     //Client is a required field, so throw error if it's not supplied.
     //If 'this.dialect' is set, then this is a 'super()' call, in which case
     //'client' does not have to be set as it's already assigned on the client prototype.
@@ -69,6 +74,9 @@ class Client extends EventEmitter {
       this.connectionConfigExpirationChecker = () => true; // causes the provider to be called on first use
     } else {
       this.connectionSettings = cloneDeep(config.connection || {});
+      if (config.connection && config.connection.password) {
+        setHiddenProperty(this.connectionSettings, config.connection);
+      }
       this.connectionConfigExpirationChecker = null;
     }
     if (this.driverName && config.connection) {
@@ -305,6 +313,18 @@ class Client extends EventEmitter {
     try {
       const connection = await this.pool.acquire().promise;
       debug('acquired connection from pool: %s', connection.__knexUid);
+      if (connection.config) {
+        if (connection.config.password) {
+          setHiddenProperty(connection.config);
+        }
+        if (
+          connection.config.authentication &&
+          connection.config.authentication.options &&
+          connection.config.authentication.options.password
+        ) {
+          setHiddenProperty(connection.config.authentication.options);
+        }
+      }
       return connection;
     } catch (error) {
       let convertedError = error;
diff --git a/lib/dialects/mssql/index.js b/lib/dialects/mssql/index.js
index 27654fd8..67fba028 100644
--- a/lib/dialects/mssql/index.js
+++ b/lib/dialects/mssql/index.js
@@ -12,6 +12,7 @@ const TableCompiler = require('./schema/mssql-tablecompiler');
 const ViewCompiler = require('./schema/mssql-viewcompiler');
 const ColumnCompiler = require('./schema/mssql-columncompiler');
 const QueryBuilder = require('../../query/querybuilder');
+const { setHiddenProperty } = require('../../util/security');
 
 const debug = require('debug')('knex:mssql');
 
@@ -66,6 +67,10 @@ class Client_MSSQL extends Client {
       },
     };
 
+    if (cfg.authentication.options.password) {
+      setHiddenProperty(cfg.authentication.options);
+    }
+
     // tedious always connect via tcp when port is specified
     if (cfg.options.instanceName) delete cfg.options.port;
 
diff --git a/lib/knex-builder/make-knex.js b/lib/knex-builder/make-knex.js
index 55f9acd3..7345708c 100644
--- a/lib/knex-builder/make-knex.js
+++ b/lib/knex-builder/make-knex.js
@@ -7,6 +7,7 @@ const QueryInterface = require('../query/method-constants');
 const merge = require('lodash/merge');
 const batchInsert = require('../execution/batch-insert');
 const { isObject } = require('../util/is');
+const { setHiddenProperty } = require('../util/security');
 
 // Javascript does not officially support "callable objects".  Instead,
 // you must create a regular Function and inject properties/methods
@@ -208,6 +209,10 @@ function initContext(knexFn) {
         knexClone.client = Object.create(this.client.constructor.prototype); // Clone client to avoid leaking listeners that are set on it
         merge(knexClone.client, this.client);
         knexClone.client.config = Object.assign({}, this.client.config); // Clone client config to make sure they can be modified independently
+
+        if (this.client.config.password) {
+          setHiddenProperty(knexClone.client.config, this.client.config);
+        }
       }
 
       redefineProperties(knexClone, knexClone.client);
diff --git a/lib/util/security.js b/lib/util/security.js
new file mode 100644
index 00000000..d4b4edb9
--- /dev/null
+++ b/lib/util/security.js
@@ -0,0 +1,26 @@
+/**
+ * Sets a hidden (non-enumerable) property on the `target` object, copying it
+ * from `source`.
+ *
+ * This is useful when we want to protect certain data from being accidentally
+ * leaked through logs, also when the property is non-enumerable on the `source`
+ * object and we want to ensure that it is properly copied.
+ *
+ * @param {object} target
+ * @param {object} source - default: target
+ * @param {string} propertyName - default: 'password'
+ */
+function setHiddenProperty(target, source, propertyName = 'password') {
+  if (!source) {
+    source = target;
+  }
+
+  Object.defineProperty(target, propertyName, {
+    enumerable: false,
+    value: source[propertyName],
+  });
+}
+
+module.exports = {
+  setHiddenProperty,
+};
diff --git a/package.json b/package.json
index b814701c..fbb6ab50 100644
--- a/package.json
+++ b/package.json
@@ -70,7 +70,7 @@
     "getopts": "2.3.0",
     "interpret": "^2.2.0",
     "lodash": "^4.17.21",
-    "pg-connection-string": "2.5.0",
+    "pg-connection-string": "2.6.1",
     "rechoir": "^0.8.0",
     "resolve-from": "^5.0.0",
     "tarn": "^3.0.2",
diff --git a/types/index.d.ts b/types/index.d.ts
index 5bd7ea5c..d17d1122 100644
--- a/types/index.d.ts
+++ b/types/index.d.ts
@@ -2647,6 +2647,7 @@ export declare namespace Knex {
       constraintName?: string
     ): ColumnBuilder;
     checkRegex(regex: string, constraintName?: string): ColumnBuilder;
+    collate(collation: string): ColumnBuilder;
   }
 
   interface ForeignConstraintBuilder {


Test patch:
diff --git a/test/db-less-test-suite.js b/test/db-less-test-suite.js
index 1100a293..647d83af 100644
--- a/test/db-less-test-suite.js
+++ b/test/db-less-test-suite.js
@@ -10,6 +10,7 @@ describe('Util Tests', function () {
   require('./unit/util/nanoid');
   require('./unit/util/save-async-stack');
   require('./unit/util/comma-no-paren-regex');
+  require('./unit/util/security');
 });
 
 describe('Query Building Tests', function () {
diff --git a/test/integration/connection-config-provider.js b/test/integration/connection-config-provider.js
index 37006e90..0725fe13 100644
--- a/test/integration/connection-config-provider.js
+++ b/test/integration/connection-config-provider.js
@@ -4,6 +4,7 @@ const { expect } = require('chai');
 
 const _ = require('lodash');
 const makeKnex = require('../../knex');
+const { setHiddenProperty } = require('../../lib/util/security');
 
 module.exports = function (config) {
   describe('Connection configuration provider', function () {
@@ -13,6 +14,9 @@ module.exports = function (config) {
 
     this.beforeEach(() => {
       configWorkingCopy = _.cloneDeep(config);
+      if (config.connection && config.connection.password) {
+        setHiddenProperty(configWorkingCopy.connection, config.connection);
+      }
       configWorkingCopy.pool.min = 1;
       configWorkingCopy.pool.max = 2;
       providerInvocationCount = 0;
diff --git a/test/integration2/query/misc/additional.spec.js b/test/integration2/query/misc/additional.spec.js
index 73d9d74d..f05d4d2a 100644
--- a/test/integration2/query/misc/additional.spec.js
+++ b/test/integration2/query/misc/additional.spec.js
@@ -1,6 +1,7 @@
 /*eslint no-var:0, max-len:0 */
 'use strict';
 
+const util = require('util');
 const chai = require('chai');
 chai.use(require('chai-as-promised'));
 chai.use(require('sinon-chai'));
@@ -34,6 +35,7 @@ const {
 const logger = require('../../../integration/logger');
 const { insertAccounts } = require('../../../util/dataInsertHelper');
 const sinon = require('sinon');
+const { setHiddenProperty } = require('../../../../lib/util/security');
 
 describe('Additional', function () {
   getAllDbs().forEach((db) => {
@@ -1006,6 +1008,16 @@ describe('Additional', function () {
           // To make this test easier, I'm changing the pool settings to max 1.
           // Also setting acquireTimeoutMillis to lower as not to wait the default time
           const knexConfig = _.cloneDeep(knex.client.config);
+          if (
+            knex.client.config.connection &&
+            knex.client.config.connection.password
+          ) {
+            setHiddenProperty(
+              knexConfig.connection,
+              knex.client.config.connection
+            );
+          }
+
           knexConfig.pool.min = 0;
           knexConfig.pool.max = 1;
           knexConfig.pool.acquireTimeoutMillis = 100;
@@ -1123,6 +1135,16 @@ describe('Additional', function () {
           // To make this test easier, I'm changing the pool settings to max 1.
           // Also setting acquireTimeoutMillis to lower as not to wait the default time
           const knexConfig = _.cloneDeep(knex.client.config);
+          if (
+            knex.client.config.connection &&
+            knex.client.config.connection.password
+          ) {
+            setHiddenProperty(
+              knexConfig.connection,
+              knex.client.config.connection
+            );
+          }
+
           knexConfig.pool.min = 0;
           knexConfig.pool.max = 2;
           knexConfig.pool.acquireTimeoutMillis = 100;
@@ -1420,6 +1442,17 @@ describe('Additional', function () {
             expect(knexDb.client.pool.destroyed).to.equal(false);
           });
         });
+
+        it('should not leak passwords when logged', async function () {
+          const query = knex('test_table_two').select('*');
+          const fakeLog = util.inspect(query, { depth: null });
+
+          // These passwords come from `scripts/docker-compose.yml`
+          const passwordMatches = fakeLog.match(
+            /(knextest|testpassword|S0meVeryHardPassword|testrootpassword)/g
+          );
+          expect(passwordMatches).to.be.null;
+        });
       });
     });
   });
diff --git a/test/unit/util/security.js b/test/unit/util/security.js
new file mode 100644
index 00000000..c2846b12
--- /dev/null
+++ b/test/unit/util/security.js
@@ -0,0 +1,39 @@
+const { expect } = require('chai');
+const { setHiddenProperty } = require('../../../lib/util/security');
+
+describe('setHiddenProperty', () => {
+  it('hides password property when only target object is passed', () => {
+    const target = { password: 'xyz' };
+    expect(Object.keys(target)).to.contain('password'); // sanity check
+
+    setHiddenProperty(target);
+    expect(Object.keys(target)).to.not.contain('password');
+
+    // Although not enumerable, it is still present
+    expect(target).to.have.property('password');
+    expect(target.password).to.equal('xyz');
+  });
+
+  it('sets property from source even when it is not enumerable', () => {
+    const source = Object.create(
+      {},
+      {
+        password: {
+          enumerable: false,
+          value: 'xyz',
+        },
+      }
+    );
+
+    expect(Object.keys(source)).to.not.contain('password'); // sanity check
+    expect(source).to.have.property('password'); // sanity check
+    expect(source.password).to.equal('xyz'); // sanity check
+
+    const target = {};
+    setHiddenProperty(target, source);
+
+    expect(Object.keys(target)).to.not.contain('password');
+    expect(target).to.have.property('password');
+    expect(target.password).to.equal('xyz');
+  });
+});