Skip to content

Commit 6b228f2

Browse files
EhabYEhabY
committed
fix: retry rename on Windows EPERM/EACCES/EBUSY during SSH config save
On Windows, antivirus, Search Indexer, cloud sync, or concurrent processes can briefly lock files, causing fs.rename() to fail with EPERM, EACCES, or EBUSY when atomically replacing ~/.ssh/config or credential files. Add renameWithRetry() utility matching the strategy used by VS Code (pfs.ts) and graceful-fs: 60s wall-clock timeout with linear backoff (10ms increments, capped at 100ms). Only applies on Windows; other platforms call rename directly.
1 parent f0df04e commit 6b228f2

File tree

5 files changed

+221
-36
lines changed

5 files changed

+221
-36
lines changed

‎src/core/cliCredentialManager.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import * as semver from "semver";
77
import { isKeyringEnabled } from "../cliConfig";
88
import { featureSetForVersion } from "../featureSet";
99
import { getHeaderArgs } from "../headers";
10-
import { tempFilePath, toSafeHost } from "../util";
10+
import { renameWithRetry, tempFilePath, toSafeHost } from "../util";
1111

1212
import * as cliUtils from "./cliUtils";
1313

@@ -256,7 +256,7 @@ export class CliCredentialManager {
256256
const tempPath = tempFilePath(filePath, "temp");
257257
try {
258258
await fs.writeFile(tempPath, content, { mode: 0o600 });
259-
await fs.rename(tempPath, filePath);
259+
await renameWithRetry(fs.rename, tempPath, filePath);
260260
} catch (err) {
261261
await fs.rm(tempPath, { force: true }).catch((rmErr) => {
262262
this.logger.warn("Failed to delete temp file", tempPath, rmErr);

‎src/remote/sshConfig.ts‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
1-
import { mkdir, readFile, rename, stat, writeFile } from "node:fs/promises";
1+
import {
2+
mkdir,
3+
readFile,
4+
rename,
5+
stat,
6+
unlink,
7+
writeFile,
8+
} from "node:fs/promises";
29
import path from "node:path";
310

4-
import { countSubstring, tempFilePath } from "../util";
11+
import { countSubstring, renameWithRetry, tempFilePath } from "../util";
512

613
class SSHConfigBadFormat extends Error {}
714

@@ -25,6 +32,7 @@ export interface FileSystem {
2532
readFile: typeof readFile;
2633
rename: typeof rename;
2734
stat: typeof stat;
35+
unlink: typeof unlink;
2836
writeFile: typeof writeFile;
2937
}
3038

@@ -33,6 +41,7 @@ const defaultFileSystem: FileSystem = {
3341
readFile,
3442
rename,
3543
stat,
44+
unlink,
3645
writeFile,
3746
};
3847

@@ -357,8 +366,13 @@ export class SSHConfig {
357366
}
358367

359368
try {
360-
await this.fileSystem.rename(tempPath, this.filePath);
369+
await renameWithRetry(
370+
(src, dest) => this.fileSystem.rename(src, dest),
371+
tempPath,
372+
this.filePath,
373+
);
361374
} catch (err) {
375+
await this.fileSystem.unlink(tempPath).catch(() => undefined);
362376
throw new Error(
363377
`Failed to rename temporary SSH config file at ${tempPath} to ${this.filePath}: ${
364378
err instanceof Error ? err.message : String(err)

‎src/util.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,51 @@ export function countSubstring(needle: string, haystack: string): number {
150150
return count;
151151
}
152152

153+
const transientRenameCodes: ReadonlySet<string> = new Set([
154+
"EPERM",
155+
"EACCES",
156+
"EBUSY",
157+
]);
158+
159+
/**
160+
* Rename with retry for transient Windows filesystem errors (EPERM, EACCES,
161+
* EBUSY). On Windows, antivirus, Search Indexer, cloud sync, or concurrent
162+
* processes can briefly lock files causing renames to fail.
163+
*
164+
* On non-Windows platforms, calls renameFn directly with no retry.
165+
*
166+
* Matches the strategy used by VS Code (pfs.ts) and graceful-fs: 60s
167+
* wall-clock timeout with linear backoff (10ms increments) capped at 100ms.
168+
*/
169+
export async function renameWithRetry(
170+
renameFn: (src: string, dest: string) => Promise<void>,
171+
source: string,
172+
destination: string,
173+
timeoutMs = 60_000,
174+
delayCapMs = 100,
175+
): Promise<void> {
176+
if (process.platform !== "win32") {
177+
return renameFn(source, destination);
178+
}
179+
const startTime = Date.now();
180+
for (let attempt = 1; ; attempt++) {
181+
try {
182+
return await renameFn(source, destination);
183+
} catch (err) {
184+
const code = (err as NodeJS.ErrnoException).code;
185+
if (
186+
!code ||
187+
!transientRenameCodes.has(code) ||
188+
Date.now() - startTime >= timeoutMs
189+
) {
190+
throw err;
191+
}
192+
const delay = Math.min(delayCapMs, attempt * 10);
193+
await new Promise((resolve) => setTimeout(resolve, delay));
194+
}
195+
}
196+
}
197+
153198
export function escapeCommandArg(arg: string): string {
154199
const escapedString = arg.replaceAll('"', String.raw`\"`);
155200
return `"${escapedString}"`;

‎test/unit/remote/sshConfig.test.ts‎

Lines changed: 75 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const mockFileSystem = {
2121
readFile: vi.fn(),
2222
rename: vi.fn(),
2323
stat: vi.fn(),
24+
unlink: vi.fn().mockResolvedValue(undefined),
2425
writeFile: vi.fn(),
2526
};
2627

@@ -53,19 +54,19 @@ Host coder-vscode--*
5354
UserKnownHostsFile /dev/null
5455
# --- END CODER VSCODE ---`;
5556

56-
expect(mockFileSystem.readFile).toBeCalledWith(
57+
expect(mockFileSystem.readFile).toHaveBeenCalledWith(
5758
sshFilePath,
5859
expect.anything(),
5960
);
60-
expect(mockFileSystem.writeFile).toBeCalledWith(
61+
expect(mockFileSystem.writeFile).toHaveBeenCalledWith(
6162
expect.stringContaining(sshTempFilePrefix),
6263
expectedOutput,
6364
expect.objectContaining({
6465
encoding: "utf-8",
6566
mode: 0o600, // Default mode for new files.
6667
}),
6768
);
68-
expect(mockFileSystem.rename).toBeCalledWith(
69+
expect(mockFileSystem.rename).toHaveBeenCalledWith(
6970
expect.stringContaining(sshTempFilePrefix),
7071
sshFilePath,
7172
);
@@ -96,19 +97,19 @@ Host coder-vscode.dev.coder.com--*
9697
UserKnownHostsFile /dev/null
9798
# --- END CODER VSCODE dev.coder.com ---`;
9899

99-
expect(mockFileSystem.readFile).toBeCalledWith(
100+
expect(mockFileSystem.readFile).toHaveBeenCalledWith(
100101
sshFilePath,
101102
expect.anything(),
102103
);
103-
expect(mockFileSystem.writeFile).toBeCalledWith(
104+
expect(mockFileSystem.writeFile).toHaveBeenCalledWith(
104105
expect.stringContaining(sshTempFilePrefix),
105106
expectedOutput,
106107
expect.objectContaining({
107108
encoding: "utf-8",
108109
mode: 0o600, // Default mode for new files.
109110
}),
110111
);
111-
expect(mockFileSystem.rename).toBeCalledWith(
112+
expect(mockFileSystem.rename).toHaveBeenCalledWith(
112113
expect.stringContaining(sshTempFilePrefix),
113114
sshFilePath,
114115
);
@@ -148,15 +149,15 @@ Host coder-vscode.dev.coder.com--*
148149
UserKnownHostsFile /dev/null
149150
# --- END CODER VSCODE dev.coder.com ---`;
150151

151-
expect(mockFileSystem.writeFile).toBeCalledWith(
152+
expect(mockFileSystem.writeFile).toHaveBeenCalledWith(
152153
expect.stringContaining(sshTempFilePrefix),
153154
expectedOutput,
154155
{
155156
encoding: "utf-8",
156157
mode: 0o644,
157158
},
158159
);
159-
expect(mockFileSystem.rename).toBeCalledWith(
160+
expect(mockFileSystem.rename).toHaveBeenCalledWith(
160161
expect.stringContaining(sshTempFilePrefix),
161162
sshFilePath,
162163
);
@@ -222,15 +223,15 @@ Host coder-vscode.dev-updated.coder.com--*
222223
Host *
223224
SetEnv TEST=1`;
224225

225-
expect(mockFileSystem.writeFile).toBeCalledWith(
226+
expect(mockFileSystem.writeFile).toHaveBeenCalledWith(
226227
expect.stringContaining(sshTempFilePrefix),
227228
expectedOutput,
228229
{
229230
encoding: "utf-8",
230231
mode: 0o644,
231232
},
232233
);
233-
expect(mockFileSystem.rename).toBeCalledWith(
234+
expect(mockFileSystem.rename).toHaveBeenCalledWith(
234235
expect.stringContaining(sshTempFilePrefix),
235236
sshFilePath,
236237
);
@@ -277,15 +278,15 @@ Host coder-vscode.dev.coder.com--*
277278
UserKnownHostsFile /dev/null
278279
# --- END CODER VSCODE dev.coder.com ---`;
279280

280-
expect(mockFileSystem.writeFile).toBeCalledWith(
281+
expect(mockFileSystem.writeFile).toHaveBeenCalledWith(
281282
expect.stringContaining(sshTempFilePrefix),
282283
expectedOutput,
283284
{
284285
encoding: "utf-8",
285286
mode: 0o644,
286287
},
287288
);
288-
expect(mockFileSystem.rename).toBeCalledWith(
289+
expect(mockFileSystem.rename).toHaveBeenCalledWith(
289290
expect.stringContaining(sshTempFilePrefix),
290291
sshFilePath,
291292
);
@@ -321,15 +322,15 @@ Host coder-vscode.dev.coder.com--*
321322
UserKnownHostsFile /dev/null
322323
# --- END CODER VSCODE dev.coder.com ---`;
323324

324-
expect(mockFileSystem.writeFile).toBeCalledWith(
325+
expect(mockFileSystem.writeFile).toHaveBeenCalledWith(
325326
expect.stringContaining(sshTempFilePrefix),
326327
expectedOutput,
327328
{
328329
encoding: "utf-8",
329330
mode: 0o644,
330331
},
331332
);
332-
expect(mockFileSystem.rename).toBeCalledWith(
333+
expect(mockFileSystem.rename).toHaveBeenCalledWith(
333334
expect.stringContaining(sshTempFilePrefix),
334335
sshFilePath,
335336
);
@@ -605,15 +606,15 @@ Host afterconfig
605606
LogLevel: "ERROR",
606607
});
607608

608-
expect(mockFileSystem.writeFile).toBeCalledWith(
609+
expect(mockFileSystem.writeFile).toHaveBeenCalledWith(
609610
expect.stringContaining(sshTempFilePrefix),
610611
expectedOutput,
611612
{
612613
encoding: "utf-8",
613614
mode: 0o644,
614615
},
615616
);
616-
expect(mockFileSystem.rename).toBeCalledWith(
617+
expect(mockFileSystem.rename).toHaveBeenCalledWith(
617618
expect.stringContaining(sshTempFilePrefix),
618619
sshFilePath,
619620
);
@@ -659,19 +660,19 @@ Host coder-vscode.dev.coder.com--*
659660
loglevel DEBUG
660661
# --- END CODER VSCODE dev.coder.com ---`;
661662

662-
expect(mockFileSystem.readFile).toBeCalledWith(
663+
expect(mockFileSystem.readFile).toHaveBeenCalledWith(
663664
sshFilePath,
664665
expect.anything(),
665666
);
666-
expect(mockFileSystem.writeFile).toBeCalledWith(
667+
expect(mockFileSystem.writeFile).toHaveBeenCalledWith(
667668
expect.stringContaining(sshTempFilePrefix),
668669
expectedOutput,
669670
expect.objectContaining({
670671
encoding: "utf-8",
671672
mode: 0o600, // Default mode for new files.
672673
}),
673674
);
674-
expect(mockFileSystem.rename).toBeCalledWith(
675+
expect(mockFileSystem.rename).toHaveBeenCalledWith(
675676
expect.stringContaining(sshTempFilePrefix),
676677
sshFilePath,
677678
);
@@ -689,7 +690,7 @@ it("fails if we are unable to write the temporary file", async () => {
689690

690691
await sshConfig.load();
691692

692-
expect(mockFileSystem.readFile).toBeCalledWith(
693+
expect(mockFileSystem.readFile).toHaveBeenCalledWith(
693694
sshFilePath,
694695
expect.anything(),
695696
);
@@ -705,28 +706,72 @@ it("fails if we are unable to write the temporary file", async () => {
705706
).rejects.toThrow(/Failed to write temporary SSH config file.*EACCES/);
706707
});
707708

708-
it("fails if we are unable to rename the temporary file", async () => {
709-
const existentSSHConfig = `Host beforeconfig
710-
HostName before.config.tld
711-
User before`;
712-
713-
const sshConfig = new SSHConfig(sshFilePath, mockFileSystem);
714-
mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig);
709+
it("cleans up temp file when rename fails", async () => {
710+
mockFileSystem.readFile.mockResolvedValueOnce("Host existing\n HostName x");
715711
mockFileSystem.stat.mockResolvedValueOnce({ mode: 0o600 });
716712
mockFileSystem.writeFile.mockResolvedValueOnce("");
717-
mockFileSystem.rename.mockRejectedValueOnce(new Error("EACCES"));
713+
const err = new Error("EXDEV");
714+
(err as NodeJS.ErrnoException).code = "EXDEV";
715+
mockFileSystem.rename.mockRejectedValueOnce(err);
718716

717+
const sshConfig = new SSHConfig(sshFilePath, mockFileSystem);
719718
await sshConfig.load();
720719
await expect(
721720
sshConfig.update("dev.coder.com", {
722721
Host: "coder-vscode.dev.coder.com--*",
723-
ProxyCommand: "some-command-here",
722+
ProxyCommand: "cmd",
724723
ConnectTimeout: "0",
725724
StrictHostKeyChecking: "no",
726725
UserKnownHostsFile: "/dev/null",
727726
LogLevel: "ERROR",
728727
}),
729-
).rejects.toThrow(/Failed to rename temporary SSH config file.*EACCES/);
728+
).rejects.toThrow(/Failed to rename temporary SSH config file/);
729+
expect(mockFileSystem.unlink).toHaveBeenCalledWith(
730+
expect.stringContaining(sshTempFilePrefix),
731+
);
732+
});
733+
734+
describe("rename retry on Windows", () => {
735+
const realPlatform = process.platform;
736+
737+
beforeEach(() => {
738+
Object.defineProperty(process, "platform", { value: "win32" });
739+
vi.useFakeTimers();
740+
});
741+
afterEach(() => {
742+
vi.useRealTimers();
743+
Object.defineProperty(process, "platform", { value: realPlatform });
744+
});
745+
746+
it("retries on transient EPERM and succeeds", async () => {
747+
mockFileSystem.readFile.mockResolvedValueOnce(
748+
"Host existing\n HostName x",
749+
);
750+
mockFileSystem.stat.mockResolvedValueOnce({ mode: 0o600 });
751+
mockFileSystem.writeFile.mockResolvedValueOnce("");
752+
const err = new Error("EPERM");
753+
(err as NodeJS.ErrnoException).code = "EPERM";
754+
mockFileSystem.rename
755+
.mockRejectedValueOnce(err)
756+
.mockResolvedValueOnce(undefined);
757+
758+
const sshConfig = new SSHConfig(sshFilePath, mockFileSystem);
759+
await sshConfig.load();
760+
const promise = sshConfig.update("dev.coder.com", {
761+
Host: "coder-vscode.dev.coder.com--*",
762+
ProxyCommand: "cmd",
763+
ConnectTimeout: "0",
764+
StrictHostKeyChecking: "no",
765+
UserKnownHostsFile: "/dev/null",
766+
LogLevel: "ERROR",
767+
});
768+
769+
await vi.advanceTimersByTimeAsync(100);
770+
await promise;
771+
772+
expect(mockFileSystem.rename).toHaveBeenCalledTimes(2);
773+
expect(mockFileSystem.unlink).not.toHaveBeenCalled();
774+
});
730775
});
731776

732777
describe("parseSshConfig", () => {

0 commit comments

Comments
 (0)