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

fix: SSHConfig: atomically write ssh config#511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 108 additions & 15 deletionssrc/sshConfig.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,11 +2,17 @@
import { it, afterEach, vi, expect } from "vitest"
import { SSHConfig } from "./sshConfig"

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

const mockFileSystem = {
readFile: vi.fn(),
mkdir: vi.fn(),
readFile: vi.fn(),
rename: vi.fn(),
stat: vi.fn(),
writeFile: vi.fn(),
}

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

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

const sshConfig = new SSHConfig(sshFilePath, mockFileSystem)
await sshConfig.load()
Expand All@@ -38,11 +45,20 @@ Host coder-vscode--*
# --- END CODER VSCODE ---`

expect(mockFileSystem.readFile).toBeCalledWith(sshFilePath, expect.anything())
expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath, expectedOutput, expect.anything())
expect(mockFileSystem.writeFile).toBeCalledWith(
expect.stringMatching(sshTempFilePathExpr),
expectedOutput,
expect.objectContaining({
encoding: "utf-8",
mode: 0o600, // Default mode for new files.
}),
)
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr), sshFilePath)
})

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

const sshConfig = new SSHConfig(sshFilePath, mockFileSystem)
await sshConfig.load()
Expand All@@ -65,7 +81,15 @@ Host coder-vscode.dev.coder.com--*
# --- END CODER VSCODE dev.coder.com ---`

expect(mockFileSystem.readFile).toBeCalledWith(sshFilePath, expect.anything())
expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath, expectedOutput, expect.anything())
expect(mockFileSystem.writeFile).toBeCalledWith(
expect.stringMatching(sshTempFilePathExpr),
expectedOutput,
expect.objectContaining({
encoding: "utf-8",
mode: 0o600, // Default mode for new files.
}),
)
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr), sshFilePath)
})

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

const sshConfig = new SSHConfig(sshFilePath, mockFileSystem)
await sshConfig.load()
Expand All@@ -100,10 +125,11 @@ Host coder-vscode.dev.coder.com--*
UserKnownHostsFile /dev/null
# --- END CODER VSCODE dev.coder.com ---`

expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath, expectedOutput, {
expect(mockFileSystem.writeFile).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr), expectedOutput, {
encoding: "utf-8",
mode:384,
mode:0o644,
})
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr), sshFilePath)
})

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

const sshConfig = new SSHConfig(sshFilePath, mockFileSystem)
await sshConfig.load()
Expand All@@ -164,10 +191,11 @@ Host coder-vscode.dev-updated.coder.com--*
Host *
SetEnv TEST=1`

expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath, expectedOutput, {
expect(mockFileSystem.writeFile).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr), expectedOutput, {
encoding: "utf-8",
mode:384,
mode:0o644,
})
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr), sshFilePath)
})

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

const sshConfig = new SSHConfig(sshFilePath, mockFileSystem)
await sshConfig.load()
Expand All@@ -209,16 +238,18 @@ Host coder-vscode.dev.coder.com--*
UserKnownHostsFile /dev/null
# --- END CODER VSCODE dev.coder.com ---`

expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath, expectedOutput, {
expect(mockFileSystem.writeFile).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr), expectedOutput, {
encoding: "utf-8",
mode:384,
mode:0o644,
})
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr), sshFilePath)
})

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

const sshConfig = new SSHConfig(sshFilePath, mockFileSystem)
await sshConfig.load()
Expand All@@ -243,10 +274,11 @@ Host coder-vscode.dev.coder.com--*
UserKnownHostsFile /dev/null
# --- END CODER VSCODE dev.coder.com ---`

expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath, expectedOutput, {
expect(mockFileSystem.writeFile).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr), expectedOutput, {
encoding: "utf-8",
mode:384,
mode:0o644,
})
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr), sshFilePath)
})

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

const sshConfig = new SSHConfig(sshFilePath, mockFileSystem)
mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig)
mockFileSystem.stat.mockResolvedValueOnce({ mode: 0o644 })
await sshConfig.load()

const expectedOutput = `Host beforeconfig
Expand DownExpand Up@@ -517,14 +550,17 @@ Host afterconfig
LogLevel: "ERROR",
})

expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath, expectedOutput, {
expect(mockFileSystem.writeFile).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr), expectedOutput, {
encoding: "utf-8",
mode:384,
mode:0o644,
})
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr), sshFilePath)
})

it("override values", async () => {
mockFileSystem.readFile.mockRejectedValueOnce("No file found")
mockFileSystem.stat.mockRejectedValueOnce({ code: "ENOENT" })

const sshConfig = new SSHConfig(sshFilePath, mockFileSystem)
await sshConfig.load()
await sshConfig.update(
Expand DownExpand Up@@ -561,5 +597,62 @@ Host coder-vscode.dev.coder.com--*
# --- END CODER VSCODE dev.coder.com ---`

expect(mockFileSystem.readFile).toBeCalledWith(sshFilePath, expect.anything())
expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath, expectedOutput, expect.anything())
expect(mockFileSystem.writeFile).toBeCalledWith(
expect.stringMatching(sshTempFilePathExpr),
expectedOutput,
expect.objectContaining({
encoding: "utf-8",
mode: 0o600, // Default mode for new files.
}),
)
expect(mockFileSystem.rename).toBeCalledWith(expect.stringMatching(sshTempFilePathExpr), sshFilePath)
})

it("fails if we are unable to write the temporary file", async () => {
const existentSSHConfig = `Host beforeconfig
HostName before.config.tld
User before`

const sshConfig = new SSHConfig(sshFilePath, mockFileSystem)
mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig)
mockFileSystem.stat.mockResolvedValueOnce({ mode: 0o600 })
mockFileSystem.writeFile.mockRejectedValueOnce(new Error("EACCES"))

await sshConfig.load()

expect(mockFileSystem.readFile).toBeCalledWith(sshFilePath, expect.anything())
await expect(
sshConfig.update("dev.coder.com", {
Host: "coder-vscode.dev.coder.com--*",
ProxyCommand: "some-command-here",
ConnectTimeout: "0",
StrictHostKeyChecking: "no",
UserKnownHostsFile: "/dev/null",
LogLevel: "ERROR",
}),
).rejects.toThrow(/Failed to write temporary SSH config file.*EACCES/)
})

it("fails if we are unable to rename the temporary file", async () => {
const existentSSHConfig = `Host beforeconfig
HostName before.config.tld
User before`

const sshConfig = new SSHConfig(sshFilePath, mockFileSystem)
mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig)
mockFileSystem.stat.mockResolvedValueOnce({ mode: 0o600 })
mockFileSystem.writeFile.mockResolvedValueOnce("")
mockFileSystem.rename.mockRejectedValueOnce(new Error("EACCES"))

await sshConfig.load()
await expect(
sshConfig.update("dev.coder.com", {
Host: "coder-vscode.dev.coder.com--*",
ProxyCommand: "some-command-here",
ConnectTimeout: "0",
StrictHostKeyChecking: "no",
UserKnownHostsFile: "/dev/null",
LogLevel: "ERROR",
}),
).rejects.toThrow(/Failed to rename temporary SSH config file.*EACCES/)
})
49 changes: 42 additions & 7 deletionssrc/sshConfig.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
import { mkdir, readFile, writeFile } from "fs/promises"
import { mkdir, readFile,rename, stat,writeFile } from "fs/promises"
import path from "path"
import { countSubstring } from "./util"

Expand All@@ -20,14 +20,18 @@ export interface SSHValues {

// Interface for the file system to make it easier to test
export interface FileSystem {
readFile: typeof readFile
mkdir: typeof mkdir
readFile: typeof readFile
rename: typeof rename
stat: typeof stat
writeFile: typeof writeFile
}

const defaultFileSystem: FileSystem = {
readFile,
mkdir,
readFile,
rename,
stat,
writeFile,
}

Expand DownExpand Up@@ -220,14 +224,45 @@ export class SSHConfig {
}

private async save() {
// We want to preserve the original file mode.
const existingMode = await this.fileSystem
.stat(this.filePath)
.then((stat) => stat.mode)
.catch((ex) => {
if (ex.code && ex.code === "ENOENT") {
return 0o600 // default to 0600 if file does not exist
}
throw ex // Any other error is unexpected
})
await this.fileSystem.mkdir(path.dirname(this.filePath), {
mode: 0o700, // only owner has rwx permission, not group or everyone.
recursive: true,
})
return this.fileSystem.writeFile(this.filePath, this.getRaw(), {
mode: 0o600, // owner rw
encoding: "utf-8",
})
const randSuffix = Math.random().toString(36).substring(8)
const fileName = path.basename(this.filePath)
const dirName = path.dirname(this.filePath)
const tempFilePath = `${dirName}/.${fileName}.vscode-coder-tmp.${randSuffix}`
try {
await this.fileSystem.writeFile(tempFilePath, this.getRaw(), {
mode: existingMode,
encoding: "utf-8",
})
} catch (err) {
throw new Error(
`Failed to write temporary SSH config file at ${tempFilePath}: ${err instanceof Error ? err.message : String(err)}. ` +
`Please check your disk space, permissions, and that the directory exists.`,
)
}

try {
await this.fileSystem.rename(tempFilePath, this.filePath)
} catch (err) {
throw new Error(
`Failed to rename temporary SSH config file at ${tempFilePath} to ${this.filePath}: ${
err instanceof Error ? err.message : String(err)
}. Please check your disk space, permissions, and that the directory exists.`,
)
}
}

public getRaw() {
Expand Down
[8]ページ先頭
©2009-2025 Movatter.jp