Skip to content

Commit eda19a2

Browse files
johnstcnjohnstcn
committed
fix: SSHConfig: check for multiple start/end blocks
1 parent 31b8b33 commit eda19a2

File tree

3 files changed

+221
-3
lines changed

3 files changed

+221
-3
lines changed

‎src/sshConfig.test.ts‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,189 @@ Host coder-vscode.dev.coder.com--*
249249
})
250250
})
251251

252+
it("throws an error if there is a mismatched start and end block count", async () => {
253+
// The below config contains two start blocks and one end block.
254+
// This is a malformed config and should throw an error.
255+
// Previously were were simply taking the first occurrences of the start and
256+
// end blocks, which would potentially lead to loss of any content between the
257+
// missing end block and the next start block.
258+
const existentSSHConfig = `Host beforeconfig
259+
HostName before.config.tld
260+
User before
261+
262+
# --- START CODER VSCODE dev.coder.com ---
263+
Host coder-vscode.dev.coder.com--*
264+
ConnectTimeout 0
265+
LogLevel ERROR
266+
ProxyCommand some-command-here
267+
StrictHostKeyChecking no
268+
UserKnownHostsFile /dev/null
269+
# missing END CODER VSCODE dev.coder.com ---
270+
271+
Host donotdelete
272+
HostName dont.delete.me
273+
User please
274+
275+
# --- START CODER VSCODE dev.coder.com ---
276+
Host coder-vscode.dev.coder.com--*
277+
ConnectTimeout 0
278+
LogLevel ERROR
279+
ProxyCommand some-command-here
280+
StrictHostKeyChecking no
281+
UserKnownHostsFile /dev/null
282+
# --- END CODER VSCODE dev.coder.com ---
283+
284+
Host afterconfig
285+
HostName after.config.tld
286+
User after`
287+
288+
const sshConfig = new SSHConfig(sshFilePath, mockFileSystem)
289+
mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig)
290+
await sshConfig.load()
291+
292+
// When we try to update the config, it should throw an error.
293+
await expect(
294+
sshConfig.update("dev.coder.com", {
295+
Host: "coder-vscode.dev.coder.com--*",
296+
ProxyCommand: "some-command-here",
297+
ConnectTimeout: "0",
298+
StrictHostKeyChecking: "no",
299+
UserKnownHostsFile: "/dev/null",
300+
LogLevel: "ERROR",
301+
}),
302+
).rejects.toThrow(
303+
`Malformed config: ssh config has 2 dev.coder.com START comments but 1 dev.coder.com END comments. Each START must have a matching END.`,
304+
)
305+
})
306+
307+
it("throws an error if there are more than one sections with the same label", async () => {
308+
const existentSSHConfig = `Host beforeconfig
309+
HostName before.config.tld
310+
User before
311+
312+
# --- START CODER VSCODE dev.coder.com ---
313+
Host coder-vscode.dev.coder.com--*
314+
ConnectTimeout 0
315+
LogLevel ERROR
316+
ProxyCommand some-command-here
317+
StrictHostKeyChecking no
318+
UserKnownHostsFile /dev/null
319+
# --- END CODER VSCODE dev.coder.com ---
320+
321+
Host donotdelete
322+
HostName dont.delete.me
323+
User please
324+
325+
# --- START CODER VSCODE dev.coder.com ---
326+
Host coder-vscode.dev.coder.com--*
327+
ConnectTimeout 0
328+
LogLevel ERROR
329+
ProxyCommand some-command-here
330+
StrictHostKeyChecking no
331+
UserKnownHostsFile /dev/null
332+
# --- END CODER VSCODE dev.coder.com ---
333+
334+
Host afterconfig
335+
HostName after.config.tld
336+
User after`
337+
338+
const sshConfig = new SSHConfig(sshFilePath, mockFileSystem)
339+
mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig)
340+
await sshConfig.load()
341+
342+
// When we try to update the config, it should throw an error.
343+
await expect(
344+
sshConfig.update("dev.coder.com", {
345+
Host: "coder-vscode.dev.coder.com--*",
346+
ProxyCommand: "some-command-here",
347+
ConnectTimeout: "0",
348+
StrictHostKeyChecking: "no",
349+
UserKnownHostsFile: "/dev/null",
350+
LogLevel: "ERROR",
351+
}),
352+
).rejects.toThrow(`Malformed config: ssh config has 2 dev.coder.com sections, please remove all but one.`)
353+
})
354+
355+
it("correctly handles interspersed blocks with and without label", async () => {
356+
const existentSSHConfig = `Host beforeconfig
357+
HostName before.config.tld
358+
User before
359+
360+
# --- START CODER VSCODE ---
361+
Host coder-vscode.dev.coder.com--*
362+
ConnectTimeout 0
363+
LogLevel ERROR
364+
ProxyCommand some-command-here
365+
StrictHostKeyChecking no
366+
UserKnownHostsFile /dev/null
367+
# --- END CODER VSCODE ---
368+
369+
Host donotdelete
370+
HostName dont.delete.me
371+
User please
372+
373+
# --- START CODER VSCODE dev.coder.com ---
374+
Host coder-vscode.dev.coder.com--*
375+
ConnectTimeout 0
376+
LogLevel ERROR
377+
ProxyCommand some-command-here
378+
StrictHostKeyChecking no
379+
UserKnownHostsFile /dev/null
380+
# --- END CODER VSCODE dev.coder.com ---
381+
382+
Host afterconfig
383+
HostName after.config.tld
384+
User after`
385+
386+
const sshConfig = new SSHConfig(sshFilePath, mockFileSystem)
387+
mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig)
388+
await sshConfig.load()
389+
390+
const expectedOutput = `Host beforeconfig
391+
HostName before.config.tld
392+
User before
393+
394+
# --- START CODER VSCODE ---
395+
Host coder-vscode.dev.coder.com--*
396+
ConnectTimeout 0
397+
LogLevel ERROR
398+
ProxyCommand some-command-here
399+
StrictHostKeyChecking no
400+
UserKnownHostsFile /dev/null
401+
# --- END CODER VSCODE ---
402+
403+
Host donotdelete
404+
HostName dont.delete.me
405+
User please
406+
407+
# --- START CODER VSCODE dev.coder.com ---
408+
Host coder-vscode.dev.coder.com--*
409+
ConnectTimeout 0
410+
LogLevel ERROR
411+
ProxyCommand some-command-here
412+
StrictHostKeyChecking no
413+
UserKnownHostsFile /dev/null
414+
# --- END CODER VSCODE dev.coder.com ---
415+
416+
Host afterconfig
417+
HostName after.config.tld
418+
User after`
419+
420+
await sshConfig.update("dev.coder.com", {
421+
Host: "coder-vscode.dev.coder.com--*",
422+
ProxyCommand: "some-command-here",
423+
ConnectTimeout: "0",
424+
StrictHostKeyChecking: "no",
425+
UserKnownHostsFile: "/dev/null",
426+
LogLevel: "ERROR",
427+
})
428+
429+
expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath, expectedOutput, {
430+
encoding: "utf-8",
431+
mode: 384,
432+
})
433+
})
434+
252435
it("override values", async () => {
253436
mockFileSystem.readFile.mockRejectedValueOnce("No file found")
254437
const sshConfig = new SSHConfig(sshFilePath, mockFileSystem)

‎src/sshConfig.ts‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { mkdir, readFile, writeFile } from "fs/promises"
22
import path from "path"
3+
import { countSubstring } from "./util"
34

45
class SSHConfigBadFormat extends Error {}
56

@@ -123,14 +124,29 @@ export class SSHConfig {
123124
*/
124125
private getBlock(label: string): Block | undefined {
125126
const raw = this.getRaw()
126-
const startBlockIndex = raw.indexOf(this.startBlockComment(label))
127-
const endBlockIndex = raw.indexOf(this.endBlockComment(label))
127+
const startBlock = this.startBlockComment(label)
128+
const endBlock = this.endBlockComment(label)
129+
const startBlockCount = countSubstring(startBlock, raw)
130+
const endBlockCount = countSubstring(endBlock, raw)
131+
const startBlockIndex = raw.indexOf(startBlock)
132+
const endBlockIndex = raw.indexOf(endBlock)
128133
const hasBlock = startBlockIndex > -1 && endBlockIndex > -1
129134

130135
if (!hasBlock) {
131136
return
132137
}
133138

139+
if (startBlockCount !== endBlockCount) {
140+
throw new SSHConfigBadFormat(
141+
`Malformed config: ssh config has ${startBlockCount} ${label} START comments but ${endBlockCount} ${label} END comments. Each START must have a matching END.`,
142+
)
143+
}
144+
if (startBlockCount > 1 || endBlockCount > 1) {
145+
throw new SSHConfigBadFormat(
146+
`Malformed config: ssh config has ${startBlockCount} ${label} sections, please remove all but one.`,
147+
)
148+
}
149+
134150
if (startBlockIndex === -1) {
135151
throw new SSHConfigBadFormat("Start block not found")
136152
}
@@ -144,7 +160,7 @@ export class SSHConfig {
144160
}
145161

146162
return {
147-
raw: raw.substring(startBlockIndex, endBlockIndex + this.endBlockComment(label).length),
163+
raw: raw.substring(startBlockIndex, endBlockIndex + endBlock.length),
148164
}
149165
}
150166

‎src/util.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,3 +120,22 @@ export function expandPath(input: string): string {
120120
const userHome = os.homedir()
121121
return input.replace(/\${userHome}/g, userHome)
122122
}
123+
124+
/**
125+
* Return the number of times a substring appears in a string.
126+
* @param needle string
127+
* @param haystack string
128+
* @returns number
129+
*/
130+
export function countSubstring(needle: string, haystack: string): number {
131+
if (needle.length < 1 || haystack.length < 1) {
132+
return 0
133+
}
134+
let count = 0
135+
let pos = haystack.indexOf(needle)
136+
while (pos !== -1) {
137+
count++
138+
pos = haystack.indexOf(needle, pos + needle.length)
139+
}
140+
return count
141+
}

0 commit comments

Comments
 (0)