Skip to content

Commit 2135513

Browse files
dennishenrypacotedev
andauthored
fix: smarter git ssh override (#194)
npm will no longer manually set `GIT_ASKPASS` or `GIT_SSH_COMMAND` if it finds those values already defined in the user's git config. ## References npm/cli#2891 #193 #129 --------- Co-authored-by: pacotedev <[email protected]>
1 parent 9afd0d4 commit 2135513

3 files changed

Lines changed: 200 additions & 25 deletions

File tree

lib/opts.js

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,57 @@
1+
const fs = require('node:fs')
2+
const os = require('node:os')
3+
const path = require('node:path')
4+
const ini = require('ini')
5+
6+
const gitConfigPath = path.join(os.homedir(), '.gitconfig')
7+
8+
let cachedConfig = null
9+
10+
// Function to load and cache the git config
11+
const loadGitConfig = () => {
12+
if (cachedConfig === null) {
13+
try {
14+
cachedConfig = {}
15+
if (fs.existsSync(gitConfigPath)) {
16+
const configContent = fs.readFileSync(gitConfigPath, 'utf-8')
17+
cachedConfig = ini.parse(configContent)
18+
}
19+
} catch (error) {
20+
cachedConfig = {}
21+
}
22+
}
23+
return cachedConfig
24+
}
25+
26+
const checkGitConfigs = () => {
27+
const config = loadGitConfig()
28+
return {
29+
sshCommandSetInConfig: config?.core?.sshCommand !== undefined,
30+
askPassSetInConfig: config?.core?.askpass !== undefined,
31+
}
32+
}
33+
34+
const sshCommandSetInEnv = process.env.GIT_SSH_COMMAND !== undefined
35+
const askPassSetInEnv = process.env.GIT_ASKPASS !== undefined
36+
const { sshCommandSetInConfig, askPassSetInConfig } = checkGitConfigs()
37+
138
// Values we want to set if they're not already defined by the end user
239
// This defaults to accepting new ssh host key fingerprints
3-
const gitEnv = {
4-
GIT_ASKPASS: 'echo',
5-
GIT_SSH_COMMAND: 'ssh -oStrictHostKeyChecking=accept-new',
40+
const finalGitEnv = {
41+
...(askPassSetInEnv || askPassSetInConfig ? {} : {
42+
GIT_ASKPASS: 'echo',
43+
}),
44+
...(sshCommandSetInEnv || sshCommandSetInConfig ? {} : {
45+
GIT_SSH_COMMAND: 'ssh -oStrictHostKeyChecking=accept-new',
46+
}),
647
}
48+
749
module.exports = (opts = {}) => ({
850
stdioString: true,
951
...opts,
1052
shell: false,
11-
env: opts.env || { ...gitEnv, ...process.env },
53+
env: opts.env || { ...finalGitEnv, ...process.env },
1254
})
55+
56+
// Export the loadGitConfig function for testing
57+
module.exports.loadGitConfig = loadGitConfig

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
},
3939
"dependencies": {
4040
"@npmcli/promise-spawn": "^7.0.0",
41+
"ini": "^4.1.3",
4142
"lru-cache": "^10.0.1",
4243
"npm-pick-manifest": "^9.0.0",
4344
"proc-log": "^4.0.0",

test/opts.js

Lines changed: 150 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,94 @@
11
const t = require('tap')
2-
const gitOpts = require('../lib/opts.js')
2+
const ini = require('ini')
3+
let [GIT_ASKPASS, GIT_SSH_COMMAND] = ['', '']
4+
5+
const mockFs = {
6+
existsSync: () => false,
7+
readFileSync: () => '',
8+
}
9+
10+
const gitOpts = t.mock('../lib/opts.js', {
11+
'node:fs': mockFs,
12+
})
13+
14+
t.beforeEach(() => {
15+
backupEnv()
16+
})
17+
18+
t.afterEach(() => {
19+
restoreEnv()
20+
})
321

422
t.test('defaults', t => {
5-
const { GIT_ASKPASS, GIT_SSH_COMMAND } = process.env
6-
t.teardown(() => {
7-
process.env.GIT_ASKPASS = GIT_ASKPASS
8-
process.env.GIT_SSH_COMMAND = GIT_SSH_COMMAND
9-
})
10-
delete process.env.GIT_ASKPASS
11-
delete process.env.GIT_SSH_COMMAND
12-
t.match(gitOpts().env, {
13-
GIT_ASKPASS: 'echo',
14-
GIT_SSH_COMMAND: 'ssh -oStrictHostKeyChecking=accept-new',
23+
t.match(gitOpts(), {
24+
env: {
25+
GIT_ASKPASS: 'echo',
26+
GIT_SSH_COMMAND: 'ssh -oStrictHostKeyChecking=accept-new',
27+
},
28+
shell: false,
1529
}, 'got the git defaults we want')
16-
t.equal(gitOpts().shell, false, 'shell defaults to false')
17-
t.equal(gitOpts({ shell: '/bin/bash' }).shell, false, 'shell cannot be overridden')
30+
1831
t.end()
1932
})
2033

21-
t.test('does not override', t => {
22-
const { GIT_ASKPASS, GIT_SSH_COMMAND } = process.env
23-
t.teardown(() => {
24-
process.env.GIT_ASKPASS = GIT_ASKPASS
25-
process.env.GIT_SSH_COMMAND = GIT_SSH_COMMAND
34+
t.test('handle case when fs.existsSync throws an error', t => {
35+
const gitOptsWithMockFs = t.mock('../lib/opts.js', {
36+
'node:fs': {
37+
...mockFs,
38+
existsSync: () => {
39+
throw new Error('Mocked error')
40+
},
41+
},
2642
})
43+
44+
t.match(gitOptsWithMockFs(), {
45+
env: {
46+
GIT_ASKPASS: 'echo',
47+
GIT_SSH_COMMAND: 'ssh -oStrictHostKeyChecking=accept-new',
48+
},
49+
shell: false,
50+
}, 'should apply defaults when fs.existsSync throws an error')
51+
52+
t.end()
53+
})
54+
55+
t.test('handle case when git config does not exist', t => {
56+
const gitOptsWithMockFs = t.mock('../lib/opts.js', {
57+
'node:fs': {
58+
...mockFs,
59+
existsSync: () => false,
60+
},
61+
})
62+
63+
t.match(gitOptsWithMockFs(), {
64+
env: {
65+
GIT_ASKPASS: 'echo',
66+
GIT_SSH_COMMAND: 'ssh -oStrictHostKeyChecking=accept-new',
67+
},
68+
shell: false,
69+
}, 'should apply defaults when git config does not exist')
70+
71+
t.end()
72+
})
73+
74+
t.test('does not override when sshCommand is set in env', t => {
2775
process.env.GIT_ASKPASS = 'test_askpass'
2876
process.env.GIT_SSH_COMMAND = 'test_ssh_command'
29-
t.match(gitOpts().env, {
30-
GIT_ASKPASS: 'test_askpass',
31-
GIT_SSH_COMMAND: 'test_ssh_command',
77+
78+
t.match(gitOpts(), {
79+
env: {
80+
GIT_ASKPASS: 'test_askpass',
81+
GIT_SSH_COMMAND: 'test_ssh_command',
82+
},
83+
shell: false,
3284
}, 'values already in process.env remain')
85+
3386
t.end()
3487
})
3588

3689
t.test('as non-root', t => {
3790
process.getuid = () => 999
91+
3892
t.match(gitOpts({
3993
foo: 'bar',
4094
env: { override: 'for some reason' },
@@ -49,5 +103,80 @@ t.test('as non-root', t => {
49103
gid: undefined,
50104
abc: undefined,
51105
}, 'do not set uid/gid as non-root')
106+
107+
t.end()
108+
})
109+
110+
t.test('does not override when sshCommand is set in git config', t => {
111+
const gitConfigContent = `[core]
112+
askpass = echo
113+
sshCommand = custom_ssh_command
114+
`
115+
const gitOptsWithMockFs = t.mock('../lib/opts.js', {
116+
'node:fs': {
117+
...mockFs,
118+
existsSync: () => true,
119+
readFileSync: () => gitConfigContent,
120+
},
121+
})
122+
123+
t.match(gitOptsWithMockFs(), {
124+
env: {
125+
GIT_ASKPASS: null,
126+
GIT_SSH_COMMAND: null,
127+
},
128+
shell: false,
129+
}, 'sshCommand in git config remains')
130+
52131
t.end()
53132
})
133+
134+
t.test('does not override when sshCommand is set in git config', t => {
135+
const gitConfigContent = `[core]
136+
askpass = echo
137+
sshCommand = custom_ssh_command
138+
`
139+
140+
const { loadGitConfig } = t.mock('../lib/opts.js', {
141+
'node:fs': {
142+
...mockFs,
143+
existsSync: () => true,
144+
readFileSync: () => gitConfigContent,
145+
},
146+
})
147+
148+
t.match(loadGitConfig(),
149+
ini.parse(gitConfigContent),
150+
'cachedConfig should be populated with git config'
151+
)
152+
153+
const gitOptsWithMockFs = t.mock('../lib/opts.js', {
154+
'node:fs': {
155+
...mockFs,
156+
existsSync: () => true,
157+
readFileSync: () => gitConfigContent,
158+
},
159+
})
160+
161+
t.match(gitOptsWithMockFs(), {
162+
env: {
163+
GIT_ASKPASS: null,
164+
GIT_SSH_COMMAND: null,
165+
},
166+
shell: false,
167+
}, 'sshCommand in git config remains')
168+
169+
t.end()
170+
})
171+
172+
function backupEnv () {
173+
GIT_ASKPASS = process.env.GIT_ASKPASS
174+
GIT_SSH_COMMAND = process.env.GIT_SSH_COMMAND
175+
delete process.env.GIT_ASKPASS
176+
delete process.env.GIT_SSH_COMMAND
177+
}
178+
179+
function restoreEnv () {
180+
process.env.GIT_ASKPASS = GIT_ASKPASS
181+
process.env.GIT_SSH_COMMAND = GIT_SSH_COMMAND
182+
}

0 commit comments

Comments
 (0)