fix: smarter git ssh override

lib/opts.js

@@ -9,6 +9,7 @@ let cachedConfig = null
 
 // Function to load and cache the git config
 const loadGitConfig = () => {
+  /* istanbul ignore next */
   if (cachedConfig === null) {
     try {
       if (fs.existsSync(gitConfigPath)) {
@@ -24,37 +25,36 @@ const loadGitConfig = () => {
   return cachedConfig
 }
 
-const isGitSshCommandSetInConfig = () => {
+const checkGitConfigs = () => {
   const config = loadGitConfig()
-  return config.core && config.core.sshCommand !== undefined
+  return {
+    sshCommandSetInConfig: config.core?.sshCommand !== undefined,
+    askPassSetInConfig: config.core?.askpass !== undefined,
+  }
 }
 
-const isGitAskPassSetInConfig = () => {
-  const config = loadGitConfig()
-  return config.core && config.core.askpass !== undefined
+const sshCommandSetInEnv = process.env.GIT_SSH_COMMAND !== undefined
+const askPassSetInEnv = process.env.GIT_ASKPASS !== undefined
+const { sshCommandSetInConfig, askPassSetInConfig } = checkGitConfigs()
+
+// 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 finalGitEnv = {
+  ...(askPassSetInEnv || askPassSetInConfig ? {} : {
+    GIT_ASKPASS: 'echo',
+  }),
+  ...(sshCommandSetInEnv || sshCommandSetInConfig ? {} : {
+    GIT_SSH_COMMAND: 'ssh -oStrictHostKeyChecking=accept-new',
+  }),
 }
 
-module.exports = (opts = {}) => {
-  const sshCommandSetInEnv = process.env.GIT_SSH_COMMAND !== undefined
-  const sshCommandSetInConfig = isGitSshCommandSetInConfig()
-  const askPassSetInEnv = process.env.GIT_ASKPASS !== undefined
-  const askPassSetInConfig = isGitAskPassSetInConfig()
-
-  // 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 finalGitEnv = {
-    ...(askPassSetInEnv || askPassSetInConfig ? {} : {
-      GIT_ASKPASS: 'echo',
-    }),
-    ...(sshCommandSetInEnv || sshCommandSetInConfig ? {} : {
-      GIT_SSH_COMMAND: 'ssh -oStrictHostKeyChecking=accept-new',
-    }),
-  }
+module.exports = (opts = {}) => ({
+  stdioString: true,
+  ...opts,
+  shell: false,
+  env: opts.env || { ...finalGitEnv, ...process.env },
+})
 
-  return {
-    stdioString: true,
-    ...opts,
-    shell: false,
-    env: opts.env || { ...finalGitEnv, ...process.env },
-  }
+module.exports._resetCachedConfig = () => {
+  cachedConfig = null
 }

package.json

@@ -34,8 +34,7 @@
     "@npmcli/template-oss": "4.22.0",
     "npm-package-arg": "^11.0.0",
     "slash": "^3.0.0",
-    "tap": "^16.0.1",
-    "proxyquire": "^2.1.3"
+    "tap": "^16.0.1"
   },
   "dependencies": {
     "@npmcli/promise-spawn": "^7.0.0",

test/opts.js

@@ -1,49 +1,38 @@
 const t = require('tap')
-const proxyquire = require('proxyquire')
-const gitOpts = require('../lib/opts.js')
-const fs = require('node:fs')
-const os = require('node:os')
-const path = require('node:path')
-
-const gitConfigPath = path.join(os.homedir(), '.gitconfig')
+let [GIT_ASKPASS, GIT_SSH_COMMAND] = ['', '']
 
 const mockFs = {
   existsSync: () => false,
   readFileSync: () => '',
 }
 
-// Utility function to backup and restore gitconfig
-const backupGitConfig = () => {
-  const backupPath = `${gitConfigPath}.backup`
-  if (fs.existsSync(gitConfigPath)) {
-    fs.copyFileSync(gitConfigPath, backupPath)
-    fs.unlinkSync(gitConfigPath)
-  }
-  return backupPath
-}
+const gitOpts = t.mock('../lib/opts.js', {
+  'node:fs': mockFs,
+})
 
-const restoreGitConfig = (backupPath) => {
-  if (fs.existsSync(backupPath)) {
-    fs.copyFileSync(backupPath, gitConfigPath)
-    fs.unlinkSync(backupPath)
-  } else if (fs.existsSync(gitConfigPath)) {
-    fs.unlinkSync(gitConfigPath)
-  }
-}
+t.beforeEach(() => {
+  gitOpts._resetCachedConfig()
+  backupEnv()
+})
 
-const writeGitConfig = (content) => {
-  fs.writeFileSync(gitConfigPath, content)
-}
+t.afterEach(() => {
+  restoreEnv()
+})
 
-t.test('handle case when fs.existsSync throws an error', 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
-  })
+t.test('defaults', t => {
+  t.match(gitOpts(), {
+    env: {
+      GIT_ASKPASS: 'echo',
+      GIT_SSH_COMMAND: 'ssh -oStrictHostKeyChecking=accept-new',
+    },
+    shell: false,
+  }, 'got the git defaults we want')
+
+  t.end()
+})
 
-  // Mocking fs.existsSync to throw an error
-  const gitOptsWithMockFs = proxyquire('../lib/opts.js', {
+t.test('handle case when fs.existsSync throws an error', t => {
+  const gitOptsWithMockFs = t.mock('../lib/opts.js', {
     'node:fs': {
       ...mockFs,
       existsSync: () => {
@@ -63,38 +52,26 @@ t.test('handle case when fs.existsSync throws an error', t => {
   t.end()
 })
 
-t.test('defaults', t => {
-  const backupPath = backupGitConfig()
-  const { GIT_ASKPASS, GIT_SSH_COMMAND } = process.env
-  t.teardown(() => {
-    restoreGitConfig(backupPath)
-    process.env.GIT_ASKPASS = GIT_ASKPASS
-    process.env.GIT_SSH_COMMAND = GIT_SSH_COMMAND
+t.test('handle case when git config does not exist', t => {
+  const gitOptsWithMockFs = t.mock('../lib/opts.js', {
+    'node:fs': {
+      ...mockFs,
+      existsSync: () => false,
+    },
   })
 
-  delete process.env.GIT_ASKPASS
-  delete process.env.GIT_SSH_COMMAND
-
-  t.match(gitOpts(), {
+  t.match(gitOptsWithMockFs(), {
     env: {
       GIT_ASKPASS: 'echo',
       GIT_SSH_COMMAND: 'ssh -oStrictHostKeyChecking=accept-new',
     },
     shell: false,
-  }, 'got the git defaults we want')
+  }, 'should apply defaults when git config does not exist')
 
   t.end()
 })
 
 t.test('does not override when sshCommand is set in env', t => {
-  const backupPath = backupGitConfig()
-  const { GIT_ASKPASS, GIT_SSH_COMMAND } = process.env
-  t.teardown(() => {
-    restoreGitConfig(backupPath)
-    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'
 
@@ -110,14 +87,6 @@ t.test('does not override when sshCommand is set in env', t => {
 })
 
 t.test('as non-root', t => {
-  const backupPath = backupGitConfig()
-  const { GIT_ASKPASS, GIT_SSH_COMMAND } = process.env
-  t.teardown(() => {
-    restoreGitConfig(backupPath)
-    process.env.GIT_ASKPASS = GIT_ASKPASS
-    process.env.GIT_SSH_COMMAND = GIT_SSH_COMMAND
-  })
-
   process.getuid = () => 999
 
   t.match(gitOpts({
@@ -139,27 +108,37 @@ t.test('as non-root', t => {
 })
 
 t.test('does not override when sshCommand is set in git config', t => {
-  const backupPath = backupGitConfig()
-  const { GIT_ASKPASS, GIT_SSH_COMMAND } = process.env
-  t.teardown(() => {
-    restoreGitConfig(backupPath)
-    process.env.GIT_ASKPASS = GIT_ASKPASS
-    process.env.GIT_SSH_COMMAND = GIT_SSH_COMMAND
-  })
-
-  writeGitConfig(`
-[core]
+  const gitConfigContent = `[core]
   askpass = echo
   sshCommand = custom_ssh_command
-`)
+`
+  const gitOptsWithMockFs = t.mock('../lib/opts.js', {
+    'node:fs': {
+      ...mockFs,
+      existsSync: () => true,
+      readFileSync: () => gitConfigContent,
+    },
+  })
 
-  t.match(gitOpts(), {
+  t.match(gitOptsWithMockFs(), {
     env: {
-      GIT_ASKPASS: 'undefined',
-      GIT_SSH_COMMAND: 'undefined',
+      GIT_ASKPASS: null,
+      GIT_SSH_COMMAND: null,
     },
     shell: false,
   }, 'sshCommand in git config remains')
 
   t.end()
 })
+
+function backupEnv () {
+  GIT_ASKPASS = process.env.GIT_ASKPASS
+  GIT_SSH_COMMAND = process.env.GIT_SSH_COMMAND
+  delete process.env.GIT_ASKPASS
+  delete process.env.GIT_SSH_COMMAND
+}
+
+function restoreEnv () {
+  process.env.GIT_ASKPASS = GIT_ASKPASS
+  process.env.GIT_SSH_COMMAND = GIT_SSH_COMMAND
+}