preserve existing mode on config file if present

johnstcn · johnstcn · commit 192556784455 · 2025-05-21T18:03:38.000+01:00
diff --git a/src/sshConfig.test.ts b/src/sshConfig.test.ts
@@ -5,10 +5,11 @@ import { SSHConfig } from "./sshConfig"
 const sshFilePath = "~/.config/ssh"
 
 const mockFileSystem = {
-  readFile: vi.fn(),
   mkdir: vi.fn(),
-  writeFile: vi.fn(),
+  readFile: vi.fn(),
   rename: vi.fn(),
+  stat: vi.fn(),
+  writeFile: vi.fn(),
 }
 
 afterEach(() => {
@@ -17,6 +18,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()
@@ -49,6 +51,7 @@ Host coder-vscode--*
 
 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()
@@ -88,6 +91,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()
@@ -113,7 +117,7 @@ Host coder-vscode.dev.coder.com--*
 
   expect(mockFileSystem.writeFile).toBeCalledWith(expect.stringContaining(sshFilePath), expectedOutput, {
     encoding: "utf-8",
-    mode: 384,
+    mode: 0o644,
   })
   expect(mockFileSystem.rename).toBeCalledWith(expect.stringContaining(sshFilePath + "."), sshFilePath)
 })
@@ -150,6 +154,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()
@@ -178,7 +183,7 @@ Host *
 
   expect(mockFileSystem.writeFile).toBeCalledWith(expect.stringContaining(sshFilePath), expectedOutput, {
     encoding: "utf-8",
-    mode: 384,
+    mode: 0o644,
   })
   expect(mockFileSystem.rename).toBeCalledWith(expect.stringContaining(sshFilePath + "."), sshFilePath)
 })
@@ -199,6 +204,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()
@@ -224,7 +230,7 @@ Host coder-vscode.dev.coder.com--*
 
   expect(mockFileSystem.writeFile).toBeCalledWith(expect.stringContaining(sshFilePath), expectedOutput, {
     encoding: "utf-8",
-    mode: 384,
+    mode: 0o644,
   })
   expect(mockFileSystem.rename).toBeCalledWith(expect.stringContaining(sshFilePath + "."), sshFilePath)
 })
@@ -233,6 +239,7 @@ it("it does not remove a user-added block that only matches the host of an old c
   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()
@@ -259,7 +266,7 @@ Host coder-vscode.dev.coder.com--*
 
   expect(mockFileSystem.writeFile).toBeCalledWith(expect.stringContaining(sshFilePath), expectedOutput, {
     encoding: "utf-8",
-    mode: 384,
+    mode: 0o644,
   })
   expect(mockFileSystem.rename).toBeCalledWith(expect.stringContaining(sshFilePath + "."), sshFilePath)
 })
@@ -491,6 +498,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
@@ -534,13 +542,15 @@ Host afterconfig
 
   expect(mockFileSystem.writeFile).toBeCalledWith(expect.stringContaining(sshFilePath), expectedOutput, {
     encoding: "utf-8",
-    mode: 384,
+    mode: 0o644,
   })
   expect(mockFileSystem.rename).toBeCalledWith(expect.stringContaining(sshFilePath + "."), 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(
diff --git a/src/sshConfig.ts b/src/sshConfig.ts
@@ -1,4 +1,4 @@
-import { mkdir, readFile, writeFile, rename } from "fs/promises"
+import { mkdir, readFile, rename, stat, writeFile } from "fs/promises"
 import path from "path"
 import { countSubstring } from "./util"
 
@@ -20,17 +20,19 @@ export interface SSHValues {
 
 // Interface for the file system to make it easier to test
 export interface FileSystem {
-  readFile: typeof readFile
   mkdir: typeof mkdir
-  writeFile: typeof writeFile
+  readFile: typeof readFile
   rename: typeof rename
+  stat: typeof stat
+  writeFile: typeof writeFile
 }
 
 const defaultFileSystem: FileSystem = {
-  readFile,
   mkdir,
-  writeFile,
+  readFile,
   rename,
+  stat,
+  writeFile,
 }
 
 // mergeSSHConfigValues will take a given ssh config and merge it with the overrides
@@ -222,14 +224,24 @@ 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,
     })
     const randSuffix = Math.random().toString(36).substring(8)
     const tempFilePath = `${this.filePath}.${randSuffix}`
     await this.fileSystem.writeFile(tempFilePath, this.getRaw(), {
-      mode: 0o600, // owner rw
+      mode: existingMode,
       encoding: "utf-8",
     })
     await this.fileSystem.rename(tempFilePath, this.filePath)
