Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitca32d99

Browse files
authored
fix: SSHConfig: atomically write ssh config (#511)
1 parente65ee21 commitca32d99

File tree

2 files changed

+150
-22
lines changed

2 files changed

+150
-22
lines changed

‎src/sshConfig.test.ts

Lines changed: 108 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,17 @@
22
import{it,afterEach,vi,expect}from"vitest"
33
import{SSHConfig}from"./sshConfig"
44

5-
constsshFilePath="~/.config/ssh"
5+
// This is not the usual path to ~/.ssh/config, but
6+
// setting it to a different path makes it easier to test
7+
// and makes mistakes abundantly clear.
8+
constsshFilePath="/Path/To/UserHomeDir/.sshConfigDir/sshConfigFile"
9+
constsshTempFilePathExpr=`^/Path/To/UserHomeDir/\\.sshConfigDir/\\.sshConfigFile\\.vscode-coder-tmp\\.[a-z0-9]+$`
610

711
constmockFileSystem={
8-
readFile:vi.fn(),
912
mkdir:vi.fn(),
13+
readFile:vi.fn(),
14+
rename:vi.fn(),
15+
stat:vi.fn(),
1016
writeFile:vi.fn(),
1117
}
1218

@@ -16,6 +22,7 @@ afterEach(() => {
1622

1723
it("creates a new file and adds config with empty label",async()=>{
1824
mockFileSystem.readFile.mockRejectedValueOnce("No file found")
25+
mockFileSystem.stat.mockRejectedValueOnce({code:"ENOENT"})
1926

2027
constsshConfig=newSSHConfig(sshFilePath,mockFileSystem)
2128
awaitsshConfig.load()
@@ -38,11 +45,20 @@ Host coder-vscode--*
3845
# --- END CODER VSCODE ---`
3946

4047
expect(mockFileSystem.readFile).toBeCalledWith(sshFilePath,expect.anything())
41-
expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath,expectedOutput,expect.anything())
48+
expect(mockFileSystem.writeFile).toBeCalledWith(
49+
expect.stringMatching(sshTempFilePathExpr),
50+
expectedOutput,
51+
expect.objectContaining({
52+
encoding:"utf-8",
53+
mode:0o600,// Default mode for new files.
54+
}),
55+
)
56+
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr),sshFilePath)
4257
})
4358

4459
it("creates a new file and adds the config",async()=>{
4560
mockFileSystem.readFile.mockRejectedValueOnce("No file found")
61+
mockFileSystem.stat.mockRejectedValueOnce({code:"ENOENT"})
4662

4763
constsshConfig=newSSHConfig(sshFilePath,mockFileSystem)
4864
awaitsshConfig.load()
@@ -65,7 +81,15 @@ Host coder-vscode.dev.coder.com--*
6581
# --- END CODER VSCODE dev.coder.com ---`
6682

6783
expect(mockFileSystem.readFile).toBeCalledWith(sshFilePath,expect.anything())
68-
expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath,expectedOutput,expect.anything())
84+
expect(mockFileSystem.writeFile).toBeCalledWith(
85+
expect.stringMatching(sshTempFilePathExpr),
86+
expectedOutput,
87+
expect.objectContaining({
88+
encoding:"utf-8",
89+
mode:0o600,// Default mode for new files.
90+
}),
91+
)
92+
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr),sshFilePath)
6993
})
7094

7195
it("adds a new coder config in an existent SSH configuration",async()=>{
@@ -77,6 +101,7 @@ it("adds a new coder config in an existent SSH configuration", async () => {
77101
StrictHostKeyChecking=no
78102
UserKnownHostsFile=/dev/null`
79103
mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig)
104+
mockFileSystem.stat.mockResolvedValueOnce({mode:0o644})
80105

81106
constsshConfig=newSSHConfig(sshFilePath,mockFileSystem)
82107
awaitsshConfig.load()
@@ -100,10 +125,11 @@ Host coder-vscode.dev.coder.com--*
100125
UserKnownHostsFile /dev/null
101126
# --- END CODER VSCODE dev.coder.com ---`
102127

103-
expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath,expectedOutput,{
128+
expect(mockFileSystem.writeFile).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr),expectedOutput,{
104129
encoding:"utf-8",
105-
mode:384,
130+
mode:0o644,
106131
})
132+
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr),sshFilePath)
107133
})
108134

109135
it("updates an existent coder config",async()=>{
@@ -138,6 +164,7 @@ Host coder-vscode.dev.coder.com--*
138164
Host *
139165
SetEnv TEST=1`
140166
mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig)
167+
mockFileSystem.stat.mockResolvedValueOnce({mode:0o644})
141168

142169
constsshConfig=newSSHConfig(sshFilePath,mockFileSystem)
143170
awaitsshConfig.load()
@@ -164,10 +191,11 @@ Host coder-vscode.dev-updated.coder.com--*
164191
Host *
165192
SetEnv TEST=1`
166193

167-
expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath,expectedOutput,{
194+
expect(mockFileSystem.writeFile).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr),expectedOutput,{
168195
encoding:"utf-8",
169-
mode:384,
196+
mode:0o644,
170197
})
198+
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr),sshFilePath)
171199
})
172200

173201
it("does not remove deployment-unaware SSH config and adds the new one",async()=>{
@@ -186,6 +214,7 @@ Host coder-vscode--*
186214
UserKnownHostsFile=/dev/null
187215
# --- END CODER VSCODE ---`
188216
mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig)
217+
mockFileSystem.stat.mockResolvedValueOnce({mode:0o644})
189218

190219
constsshConfig=newSSHConfig(sshFilePath,mockFileSystem)
191220
awaitsshConfig.load()
@@ -209,16 +238,18 @@ Host coder-vscode.dev.coder.com--*
209238
UserKnownHostsFile /dev/null
210239
# --- END CODER VSCODE dev.coder.com ---`
211240

212-
expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath,expectedOutput,{
241+
expect(mockFileSystem.writeFile).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr),expectedOutput,{
213242
encoding:"utf-8",
214-
mode:384,
243+
mode:0o644,
215244
})
245+
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr),sshFilePath)
216246
})
217247

218248
it("it does not remove a user-added block that only matches the host of an old coder SSH config",async()=>{
219249
constexistentSSHConfig=`Host coder-vscode--*
220250
ForwardAgent=yes`
221251
mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig)
252+
mockFileSystem.stat.mockResolvedValueOnce({mode:0o644})
222253

223254
constsshConfig=newSSHConfig(sshFilePath,mockFileSystem)
224255
awaitsshConfig.load()
@@ -243,10 +274,11 @@ Host coder-vscode.dev.coder.com--*
243274
UserKnownHostsFile /dev/null
244275
# --- END CODER VSCODE dev.coder.com ---`
245276

246-
expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath,expectedOutput,{
277+
expect(mockFileSystem.writeFile).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr),expectedOutput,{
247278
encoding:"utf-8",
248-
mode:384,
279+
mode:0o644,
249280
})
281+
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr),sshFilePath)
250282
})
251283

252284
it("throws an error if there is a missing end block",async()=>{
@@ -476,6 +508,7 @@ Host afterconfig
476508

477509
constsshConfig=newSSHConfig(sshFilePath,mockFileSystem)
478510
mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig)
511+
mockFileSystem.stat.mockResolvedValueOnce({mode:0o644})
479512
awaitsshConfig.load()
480513

481514
constexpectedOutput=`Host beforeconfig
@@ -517,14 +550,17 @@ Host afterconfig
517550
LogLevel:"ERROR",
518551
})
519552

520-
expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath,expectedOutput,{
553+
expect(mockFileSystem.writeFile).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr),expectedOutput,{
521554
encoding:"utf-8",
522-
mode:384,
555+
mode:0o644,
523556
})
557+
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr),sshFilePath)
524558
})
525559

526560
it("override values",async()=>{
527561
mockFileSystem.readFile.mockRejectedValueOnce("No file found")
562+
mockFileSystem.stat.mockRejectedValueOnce({code:"ENOENT"})
563+
528564
constsshConfig=newSSHConfig(sshFilePath,mockFileSystem)
529565
awaitsshConfig.load()
530566
awaitsshConfig.update(
@@ -561,5 +597,62 @@ Host coder-vscode.dev.coder.com--*
561597
# --- END CODER VSCODE dev.coder.com ---`
562598

563599
expect(mockFileSystem.readFile).toBeCalledWith(sshFilePath,expect.anything())
564-
expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath,expectedOutput,expect.anything())
600+
expect(mockFileSystem.writeFile).toBeCalledWith(
601+
expect.stringMatching(sshTempFilePathExpr),
602+
expectedOutput,
603+
expect.objectContaining({
604+
encoding:"utf-8",
605+
mode:0o600,// Default mode for new files.
606+
}),
607+
)
608+
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr),sshFilePath)
609+
})
610+
611+
it("fails if we are unable to write the temporary file",async()=>{
612+
constexistentSSHConfig=`Host beforeconfig
613+
HostName before.config.tld
614+
User before`
615+
616+
constsshConfig=newSSHConfig(sshFilePath,mockFileSystem)
617+
mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig)
618+
mockFileSystem.stat.mockResolvedValueOnce({mode:0o600})
619+
mockFileSystem.writeFile.mockRejectedValueOnce(newError("EACCES"))
620+
621+
awaitsshConfig.load()
622+
623+
expect(mockFileSystem.readFile).toBeCalledWith(sshFilePath,expect.anything())
624+
awaitexpect(
625+
sshConfig.update("dev.coder.com",{
626+
Host:"coder-vscode.dev.coder.com--*",
627+
ProxyCommand:"some-command-here",
628+
ConnectTimeout:"0",
629+
StrictHostKeyChecking:"no",
630+
UserKnownHostsFile:"/dev/null",
631+
LogLevel:"ERROR",
632+
}),
633+
).rejects.toThrow(/FailedtowritetemporarySSHconfigfile.*EACCES/)
634+
})
635+
636+
it("fails if we are unable to rename the temporary file",async()=>{
637+
constexistentSSHConfig=`Host beforeconfig
638+
HostName before.config.tld
639+
User before`
640+
641+
constsshConfig=newSSHConfig(sshFilePath,mockFileSystem)
642+
mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig)
643+
mockFileSystem.stat.mockResolvedValueOnce({mode:0o600})
644+
mockFileSystem.writeFile.mockResolvedValueOnce("")
645+
mockFileSystem.rename.mockRejectedValueOnce(newError("EACCES"))
646+
647+
awaitsshConfig.load()
648+
awaitexpect(
649+
sshConfig.update("dev.coder.com",{
650+
Host:"coder-vscode.dev.coder.com--*",
651+
ProxyCommand:"some-command-here",
652+
ConnectTimeout:"0",
653+
StrictHostKeyChecking:"no",
654+
UserKnownHostsFile:"/dev/null",
655+
LogLevel:"ERROR",
656+
}),
657+
).rejects.toThrow(/FailedtorenametemporarySSHconfigfile.*EACCES/)
565658
})

‎src/sshConfig.ts

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import{mkdir,readFile,writeFile}from"fs/promises"
1+
import{mkdir,readFile,rename,stat,writeFile}from"fs/promises"
22
importpathfrom"path"
33
import{countSubstring}from"./util"
44

@@ -20,14 +20,18 @@ export interface SSHValues {
2020

2121
// Interface for the file system to make it easier to test
2222
exportinterfaceFileSystem{
23-
readFile:typeofreadFile
2423
mkdir:typeofmkdir
24+
readFile:typeofreadFile
25+
rename:typeofrename
26+
stat:typeofstat
2527
writeFile:typeofwriteFile
2628
}
2729

2830
constdefaultFileSystem:FileSystem={
29-
readFile,
3031
mkdir,
32+
readFile,
33+
rename,
34+
stat,
3135
writeFile,
3236
}
3337

@@ -220,14 +224,45 @@ export class SSHConfig {
220224
}
221225

222226
privateasyncsave(){
227+
// We want to preserve the original file mode.
228+
constexistingMode=awaitthis.fileSystem
229+
.stat(this.filePath)
230+
.then((stat)=>stat.mode)
231+
.catch((ex)=>{
232+
if(ex.code&&ex.code==="ENOENT"){
233+
return0o600// default to 0600 if file does not exist
234+
}
235+
throwex// Any other error is unexpected
236+
})
223237
awaitthis.fileSystem.mkdir(path.dirname(this.filePath),{
224238
mode:0o700,// only owner has rwx permission, not group or everyone.
225239
recursive:true,
226240
})
227-
returnthis.fileSystem.writeFile(this.filePath,this.getRaw(),{
228-
mode:0o600,// owner rw
229-
encoding:"utf-8",
230-
})
241+
constrandSuffix=Math.random().toString(36).substring(8)
242+
constfileName=path.basename(this.filePath)
243+
constdirName=path.dirname(this.filePath)
244+
consttempFilePath=`${dirName}/.${fileName}.vscode-coder-tmp.${randSuffix}`
245+
try{
246+
awaitthis.fileSystem.writeFile(tempFilePath,this.getRaw(),{
247+
mode:existingMode,
248+
encoding:"utf-8",
249+
})
250+
}catch(err){
251+
thrownewError(
252+
`Failed to write temporary SSH config file at${tempFilePath}:${errinstanceofError ?err.message :String(err)}. `+
253+
`Please check your disk space, permissions, and that the directory exists.`,
254+
)
255+
}
256+
257+
try{
258+
awaitthis.fileSystem.rename(tempFilePath,this.filePath)
259+
}catch(err){
260+
thrownewError(
261+
`Failed to rename temporary SSH config file at${tempFilePath} to${this.filePath}:${
262+
errinstanceofError ?err.message :String(err)
263+
}. Please check your disk space, permissions, and that the directory exists.`,
264+
)
265+
}
231266
}
232267

233268
publicgetRaw(){

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp