Skip to content

Commit 11911fc

Browse files
authored
fix: retry rename on Windows EPERM/EACCES/EBUSY during SSH config save (#841)
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. Closes #840
1 parent f0df04e commit 11911fc

File tree

7 files changed

+278
-63
lines changed

7 files changed

+278
-63
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22

33
## Unreleased
44

5+
### Fixed
6+
7+
- Fixed SSH config writes failing on Windows when antivirus, cloud sync software,
8+
or another process briefly locks the file.
9+
510
### Added
611

712
- Automatically set `reconnectionGraceTime`, `serverShutdownTimeout`, and `maxReconnectionAttempts`

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/remote.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ import { vscodeProposed } from "../vscodeProposed";
5151
import { WorkspaceMonitor } from "../workspace/workspaceMonitor";
5252

5353
import {
54-
SSHConfig,
54+
SshConfig,
5555
type SSHValues,
5656
mergeSshConfigValues,
5757
parseCoderSshOptions,
@@ -766,7 +766,7 @@ export class Remote {
766766
sshConfigFile = path.join(os.homedir(), sshConfigFile.slice(1));
767767
}
768768

769-
const sshConfig = new SSHConfig(sshConfigFile);
769+
const sshConfig = new SshConfig(sshConfigFile, this.logger);
770770
await sshConfig.load();
771771

772772
// Merge SSH config from three sources (highest to lowest priority):

src/remote/sshConfig.ts

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
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

6-
class SSHConfigBadFormat extends Error {}
13+
import type { Logger } from "../logging/logger";
14+
15+
class SshConfigBadFormat extends Error {}
716

817
interface Block {
918
raw: string;
@@ -25,6 +34,7 @@ export interface FileSystem {
2534
readFile: typeof readFile;
2635
rename: typeof rename;
2736
stat: typeof stat;
37+
unlink: typeof unlink;
2838
writeFile: typeof writeFile;
2939
}
3040

@@ -33,6 +43,7 @@ const defaultFileSystem: FileSystem = {
3343
readFile,
3444
rename,
3545
stat,
46+
unlink,
3647
writeFile,
3748
};
3849

@@ -163,9 +174,10 @@ export function mergeSshConfigValues(
163174
return merged;
164175
}
165176

166-
export class SSHConfig {
177+
export class SshConfig {
167178
private readonly filePath: string;
168179
private readonly fileSystem: FileSystem;
180+
private readonly logger: Logger;
169181
private raw: string | undefined;
170182

171183
private startBlockComment(safeHostname: string): string {
@@ -179,16 +191,25 @@ export class SSHConfig {
179191
: `# --- END CODER VSCODE ---`;
180192
}
181193

182-
constructor(filePath: string, fileSystem: FileSystem = defaultFileSystem) {
194+
constructor(
195+
filePath: string,
196+
logger: Logger,
197+
fileSystem: FileSystem = defaultFileSystem,
198+
) {
183199
this.filePath = filePath;
200+
this.logger = logger;
184201
this.fileSystem = fileSystem;
185202
}
186203

187204
async load() {
188205
try {
189206
this.raw = await this.fileSystem.readFile(this.filePath, "utf-8");
207+
this.logger.debug("Loaded SSH config", this.filePath);
190208
} catch {
191-
// Probably just doesn't exist!
209+
this.logger.debug(
210+
"SSH config file not found, starting fresh",
211+
this.filePath,
212+
);
192213
this.raw = "";
193214
}
194215
}
@@ -204,8 +225,10 @@ export class SSHConfig {
204225
const block = this.getBlock(safeHostname);
205226
const newBlock = this.buildBlock(safeHostname, values, overrides);
206227
if (block) {
228+
this.logger.debug("Replacing SSH config block", safeHostname);
207229
this.replaceBlock(block, newBlock);
208230
} else {
231+
this.logger.debug("Appending new SSH config block", safeHostname);
209232
this.appendBlock(newBlock);
210233
}
211234
await this.save();
@@ -222,13 +245,13 @@ export class SSHConfig {
222245
const startBlockCount = countSubstring(startBlock, raw);
223246
const endBlockCount = countSubstring(endBlock, raw);
224247
if (startBlockCount !== endBlockCount) {
225-
throw new SSHConfigBadFormat(
248+
throw new SshConfigBadFormat(
226249
`Malformed config: ${this.filePath} has an unterminated START CODER VSCODE ${safeHostname ? safeHostname + " " : ""}block. Each START block must have an END block.`,
227250
);
228251
}
229252

230253
if (startBlockCount > 1 || endBlockCount > 1) {
231-
throw new SSHConfigBadFormat(
254+
throw new SshConfigBadFormat(
232255
`Malformed config: ${this.filePath} has ${startBlockCount} START CODER VSCODE ${safeHostname ? safeHostname + " " : ""}sections. Please remove all but one.`,
233256
);
234257
}
@@ -241,15 +264,15 @@ export class SSHConfig {
241264
}
242265

243266
if (startBlockIndex === -1) {
244-
throw new SSHConfigBadFormat("Start block not found");
267+
throw new SshConfigBadFormat("Start block not found");
245268
}
246269

247270
if (startBlockIndex === -1) {
248-
throw new SSHConfigBadFormat("End block not found");
271+
throw new SshConfigBadFormat("End block not found");
249272
}
250273

251274
if (endBlockIndex < startBlockIndex) {
252-
throw new SSHConfigBadFormat(
275+
throw new SshConfigBadFormat(
253276
"Malformed config, end block is before start block",
254277
);
255278
}
@@ -357,8 +380,20 @@ export class SSHConfig {
357380
}
358381

359382
try {
360-
await this.fileSystem.rename(tempPath, this.filePath);
383+
await renameWithRetry(
384+
(src, dest) => this.fileSystem.rename(src, dest),
385+
tempPath,
386+
this.filePath,
387+
);
388+
this.logger.debug("Saved SSH config", this.filePath);
361389
} catch (err) {
390+
await this.fileSystem.unlink(tempPath).catch((unlinkErr: unknown) => {
391+
this.logger.warn(
392+
"Failed to clean up temp SSH config file",
393+
tempPath,
394+
unlinkErr,
395+
);
396+
});
362397
throw new Error(
363398
`Failed to rename temporary SSH config file at ${tempPath} to ${this.filePath}: ${
364399
err instanceof Error ? err.message : String(err)
@@ -370,7 +405,7 @@ export class SSHConfig {
370405

371406
public getRaw() {
372407
if (this.raw === undefined) {
373-
throw new Error("SSHConfig is not loaded. Try sshConfig.load()");
408+
throw new Error("SshConfig is not loaded. Try sshConfig.load()");
374409
}
375410

376411
return this.raw;

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}"`;

0 commit comments

Comments
 (0)