From a1de53710c629d699a2cdbdf97fb656e2ce36b28 Mon Sep 17 00:00:00 2001 From: Gar Date: Fri, 9 Apr 2021 10:52:45 -0700 Subject: [PATCH] fix(env): stop filtering process.env We should not be removing GIT_ values from the environment if the user has set them. All we now do is set two defaults if they were not already set by the end user. --- lib/env.js | 34 ---------------------------------- lib/opts.js | 9 +++++++-- package-lock.json | 37 +++---------------------------------- package.json | 1 - test/env.js | 11 ----------- test/opts.js | 22 ++++++++++++++++++++-- 6 files changed, 30 insertions(+), 84 deletions(-) delete mode 100644 lib/env.js delete mode 100644 test/env.js diff --git a/lib/env.js b/lib/env.js deleted file mode 100644 index 2a5693f..0000000 --- a/lib/env.js +++ /dev/null @@ -1,34 +0,0 @@ -const uniqueFilename = require('unique-filename') -const { join } = require('path') -const {tmpdir} = require('os') - -const goodEnvVars = new Set([ - 'GIT_ASKPASS', - 'GIT_EXEC_PATH', - 'GIT_PROXY_COMMAND', - 'GIT_SSH', - 'GIT_SSH_COMMAND', - 'GIT_SSL_CAINFO', - 'GIT_SSL_NO_VERIFY' -]) - -// memoize -let gitEnv - -module.exports = () => { - if (gitEnv) - return gitEnv - - // we set the template dir to an empty folder to give git less to do - const tmpDir = join(tmpdir(), 'npmcli-git-template-tmp') - const tmpName = uniqueFilename(tmpDir, 'git-clone') - return gitEnv = Object.keys(process.env).reduce((gitEnv, k) => { - if (goodEnvVars.has(k) || !k.startsWith('GIT_')) - gitEnv[k] = process.env[k] - return gitEnv - }, { - GIT_ASKPASS: 'echo', - GIT_TEMPLATE_DIR: tmpName, - GIT_SSH_COMMAND: 'ssh -oStrictHostKeyChecking=accept-new' - }) -} diff --git a/lib/opts.js b/lib/opts.js index 7da4801..6db9e9a 100644 --- a/lib/opts.js +++ b/lib/opts.js @@ -1,6 +1,11 @@ -const gitEnv = require('./env.js') +// Values we want to set if they're not already defined by the end user +// This defaults to accepting new ssh host key fingerprints +const gitEnv = { + GIT_ASKPASS: 'echo', + GIT_SSH_COMMAND: 'ssh -oStrictHostKeyChecking=accept-new' +} module.exports = (opts = {}) => ({ stdioString: true, ...opts, - env: opts.env || gitEnv(), + env: opts.env || { ...gitEnv, ...process.env } }) diff --git a/package-lock.json b/package-lock.json index a0bc6be..e9bafcb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,7 +16,6 @@ "promise-inflight": "^1.0.1", "promise-retry": "^2.0.1", "semver": "^7.3.5", - "unique-filename": "^1.1.1", "which": "^2.0.2" }, "devDependencies": { @@ -1044,6 +1043,7 @@ "version": "0.1.4", "resolved": "https://registry.npmjs.org/imurmurhash/-/imurmurhash-0.1.4.tgz", "integrity": "sha1-khi5srkoojixPcT7a21XbyMUU+o=", + "dev": true, "engines": { "node": ">=0.8.19" } @@ -4367,22 +4367,6 @@ "node": ">=0.10.0" } }, - "node_modules/unique-filename": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/unique-filename/-/unique-filename-1.1.1.tgz", - "integrity": "sha512-Vmp0jIp2ln35UTXuryvjzkjGdRyf9b2lTXuSYUiPmzRcl3FDtYqAwOnTJkAngD9SWhnoJzDbTKwaOrZ+STtxNQ==", - "dependencies": { - "unique-slug": "^2.0.0" - } - }, - "node_modules/unique-slug": { - "version": "2.0.2", - "resolved": "https://registry.npmjs.org/unique-slug/-/unique-slug-2.0.2.tgz", - "integrity": "sha512-zoWr9ObaxALD3DOPfjPSqxt4fnZiWblxHIgeWqW8x7UqDzEtHEQLzji2cuJYQFCU6KmoJikOYAZlrTHHebjx2w==", - "dependencies": { - "imurmurhash": "^0.1.4" - } - }, "node_modules/uri-js": { "version": "4.2.2", "resolved": "https://registry.npmjs.org/uri-js/-/uri-js-4.2.2.tgz", @@ -5531,7 +5515,8 @@ "imurmurhash": { "version": "0.1.4", "resolved": "https://registry.npmjs.org/imurmurhash/-/imurmurhash-0.1.4.tgz", - "integrity": "sha1-khi5srkoojixPcT7a21XbyMUU+o=" + "integrity": "sha1-khi5srkoojixPcT7a21XbyMUU+o=", + "dev": true }, "infer-owner": { "version": "1.0.4", @@ -7992,22 +7977,6 @@ } } }, - "unique-filename": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/unique-filename/-/unique-filename-1.1.1.tgz", - "integrity": "sha512-Vmp0jIp2ln35UTXuryvjzkjGdRyf9b2lTXuSYUiPmzRcl3FDtYqAwOnTJkAngD9SWhnoJzDbTKwaOrZ+STtxNQ==", - "requires": { - "unique-slug": "^2.0.0" - } - }, - "unique-slug": { - "version": "2.0.2", - "resolved": "https://registry.npmjs.org/unique-slug/-/unique-slug-2.0.2.tgz", - "integrity": "sha512-zoWr9ObaxALD3DOPfjPSqxt4fnZiWblxHIgeWqW8x7UqDzEtHEQLzji2cuJYQFCU6KmoJikOYAZlrTHHebjx2w==", - "requires": { - "imurmurhash": "^0.1.4" - } - }, "uri-js": { "version": "4.2.2", "resolved": "https://registry.npmjs.org/uri-js/-/uri-js-4.2.2.tgz", diff --git a/package.json b/package.json index 3493093..0fad13e 100644 --- a/package.json +++ b/package.json @@ -35,7 +35,6 @@ "promise-inflight": "^1.0.1", "promise-retry": "^2.0.1", "semver": "^7.3.5", - "unique-filename": "^1.1.1", "which": "^2.0.2" } } diff --git a/test/env.js b/test/env.js deleted file mode 100644 index a1838aa..0000000 --- a/test/env.js +++ /dev/null @@ -1,11 +0,0 @@ -const gitEnv = require('../lib/env.js') -const t = require('tap') - -process.env.GIT_UNKNOWN_THINGIE = 'this should not be copied' -const ga = process.env.GIT_ASKPASS -process.env.GIT_ASKPASS = 'foo' -const ge = gitEnv() -t.equal(ge.GIT_ASKPASS, 'foo', 'askpass makes it through') -t.equal(ge.GIT_UNKNOWN_THINGIE, undefined, 'unknown thing filtered out') -t.equal(ge.GIT_SSH_COMMAND, 'ssh -oStrictHostKeyChecking=accept-new') -t.equal(gitEnv(), ge, 'memoized') diff --git a/test/opts.js b/test/opts.js index c6d24db..a709d7d 100644 --- a/test/opts.js +++ b/test/opts.js @@ -1,8 +1,26 @@ const t = require('tap') const gitOpts = require('../lib/opts.js') -const gitEnv = require('../lib/env.js') +const gitEnv = { + GIT_ASKPASS: 'echo', + GIT_SSH_COMMAND: 'ssh -oStrictHostKeyChecking=accept-new' +} -t.match(gitOpts().env, gitEnv(), 'got the git env by default') +t.match(gitOpts().env, gitEnv, 'got the git defaults we want') + +t.test('does not override', t => { + const { GIT_ASKPASS, GIT_SSH_COMMAND } = process.env + t.teardown(() => { + process.env.GIT_ASKPASS = GIT_ASKPASS + process.env.GIT_SSH_COMMAND = GIT_SSH_COMMAND + }) + process.env.GIT_ASKPASS = 'test_askpass' + process.env.GIT_SSH_COMMAND = 'test_ssh_command' + t.match(gitOpts().env, { + GIT_ASKPASS: 'test_askpass', + GIT_SSH_COMMAND: 'test_ssh_command', + }, 'values already in process.env remain') + t.end() +}) t.test('as non-root', t => { process.getuid = () => 999