fix(ssh): repair managed Windows SSH config permissions (#1110)

Windows files inherit permissions from their parent directory, so a Coder-generated SSH config file could end up readable by other accounts. OpenSSH rejects files like that with "Bad owner or permissions" and aborts the whole config, which blocks every Coder host, not just one. This PR locks down the managed SSH directory before each write so files inherit clean permissions, using only built-in Windows tools with no elevation required.

- Repair permissions on the managed SSH directory and its config files on Windows, fixing existing bad fragments on upgrade
- Handle a race between two editor windows sharing the same SSH directory
- Add Windows ARM64 test coverage in CI
- Update CI caching to avoid unnecessary cache misses

Fixes #1108

Author: EhabY

.gitattributes

@@ -1 +1,3 @@
 * text=auto eol=lf
+pnpm-lock.yaml linguist-generated=true
+flake.lock linguist-generated=true

.github/actions/setup/action.yml

@@ -10,11 +10,15 @@ runs:
       run: git config --global url."https://github.com/".insteadOf "git@github.com:"
 
     - uses: pnpm/action-setup@ea17c68df8912ef543352723c149a84f56e3d413 # v6.1.0
+      with:
+        # Caches the pnpm store, keyed by lockfile hash with a prefix fallback,
+        # so a dependency bump reuses the rest of the store. setup-node's own
+        # pnpm cache restores on an exact hash only.
+        cache: true
 
     - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
       with:
         node-version: "22"
-        cache: "pnpm"
 
     - name: Install dependencies
       shell: bash

.github/workflows/ci.yaml

@@ -48,6 +48,11 @@ jobs:
               name: Windows,
               electron-version: "latest",
             }
+          - {
+              os: windows-11-arm,
+              name: Windows ARM64,
+              electron-version: "latest",
+            }
           - { os: macos-15, name: macOS, electron-version: "latest" }
 
     steps:
@@ -62,6 +67,7 @@ jobs:
         shell: bash
         env:
           CI: true
+          EXPECTED_ARCH: ${{ runner.arch }}
 
   test-integration:
     name: Integration Test (${{ matrix.name }}, VS Code ${{ matrix.vscode-version }})

.vscodeignore

@@ -62,4 +62,4 @@ AGENTS.md
 # Storybook
 .storybook/**
 storybook-static/**
-**/*.stories.*
+**/*.stories.*

CHANGELOG.md

@@ -21,6 +21,10 @@
   replay the buffered connection logs instead. Close codes never reached the
   reconnect logic, so these closes retried forever. Server-initiated normal
   closes (`1000`/`1001`) keep reconnecting.
+- Repair permissions on the Windows SSH config files the extension generates,
+  so connections stop failing with "Bad owner or permissions". Only you,
+  SYSTEM, and Administrators keep access to them. The extension leaves your own
+  SSH config untouched and needs no admin rights.
 
 ## [v1.16.3](https://github.com/coder/vscode-coder/releases/tag/v1.16.3) 2026-09-14
 

CONTRIBUTING.md

@@ -87,6 +87,41 @@ command.
 Coder Remote periodically reads the `network-info-dir + "/" + matchingSSHPID`
 file to display network information.
 
+### Windows SSH config permissions
+
+Windows files inherit their permissions from the directory they live in, so a
+config the extension generates under `%APPDATA%\coder.coder-remote\ssh` can end
+up readable by other accounts. OpenSSH rejects such a file with "Bad owner or
+permissions" and skips the whole `Include`, which blocks every Coder host, not
+just the one it came from.
+
+Before each managed write, `src/remote/windowsAcl.ts` locks the directory down
+and lets its files inherit from it:
+
+| Step                                                     | Command                                              |
+| -------------------------------------------------------- | ---------------------------------------------------- |
+| Read the current user's SID                              | `whoami.exe /user /fo csv /nh`                       |
+| Clear the directory's own grants                         | `icacls.exe <dir> /reset`                            |
+| Grant that user, SYSTEM, and Administrators full control | `icacls.exe <dir> /inheritance:r /grant:r <trustee>` |
+| Clear each `*.conf` file so it inherits the directory    | `icacls.exe <file> /reset`                           |
+
+Resetting every `*.conf` file, not only the one being written, also repairs
+files left behind by other deployments and editors.
+
+Worth knowing:
+
+- Like VS Code, the code checks exit codes but never reads ACLs back. It needs
+  no script, native module, ownership change, or elevation, and it leaves the
+  user's own SSH config alone.
+- Links and non-files are rejected before the repair, because inheritable
+  grants reach children even without `/T`. That stops mistakes, not an attacker
+  racing the check.
+- The repair is not atomic: a failure after `/reset` can leave the directory
+  with its parent's grants.
+
+`windowsAcl.native.test.ts` drives the real `icacls.exe`, `whoami.exe`, and
+OpenSSH. Run it unelevated as well as in CI to catch privilege assumptions.
+
 ## Other features
 
 The extension provides several sidebar panels:
@@ -289,7 +324,8 @@ When updating the minimum Node.js version, update these files:
 
 Some dependencies are not directly used in the source but are required anyway.
 
-- `bufferutil` and `utf-8-validate` are peer dependencies of `ws`.
+- `bufferutil` and `utf-8-validate` are peer dependencies of `ws`. Their source
+  builds are off, so Windows on ARM64 uses their JavaScript fallback.
 - `ua-parser-js` and `dayjs` are used by the Coder API client.
 
 The coder client is vendored from coder/coder. Pin it to a release tag in

pnpm-workspace.yaml

@@ -32,13 +32,14 @@ dedupePeers: true
 
 allowBuilds:
   "@vscode/vsce-sign": true
-  bufferutil: true
+  # Only win32-arm64 lacks a prebuild, and its JavaScript fallback is fine.
+  bufferutil: false
   electron: true
   esbuild: true
   keytar: false
   odiff-bin: true
   unrs-resolver: true
-  utf-8-validate: true
+  utf-8-validate: false
 
 overrides:
   "@vscode-elements/elements": ^2.5.1

src/remote/remote.ts

@@ -67,6 +67,7 @@ import {
 	sshSupportsSetEnv,
 	type SshProperties,
 } from "./sshSupport";
+import { createManagedPermissions } from "./windowsAcl";
 import { WorkspaceStateMachine } from "./workspaceStateMachine";
 
 import type { Api } from "coder/site/src/api/api";
@@ -929,6 +930,8 @@ export class Remote {
 		const coderConfig = new SshConfig(
 			this.pathResolver.getSshConfigPath(safeHostname, hostEditorId(sshHost)),
 			this.logger,
+			undefined,
+			createManagedPermissions(),
 		);
 
 		// Options the user set themselves win the merge below, so they are exempt

src/remote/sshConfig.ts

@@ -1,6 +1,7 @@
 import {
 	mkdir,
 	readFile,
+	readdir,
 	rename,
 	stat,
 	unlink,
@@ -34,10 +35,20 @@ export interface SshValues {
 	SetEnv?: string;
 }
 
+/**
+ * Restricts the Coder-managed config directory and the files it generates.
+ * A config without one is not Coder-managed, so it is written untouched.
+ */
+export interface ManagedPermissions {
+	prepareDirectory(directory: string): Promise<void>;
+	secure(filePath: string): Promise<void>;
+}
+
 /** Injectable for tests. */
 export interface FileSystem {
 	mkdir: typeof mkdir;
 	readFile: typeof readFile;
+	readdir: typeof readdir;
 	rename: typeof rename;
 	stat: typeof stat;
 	unlink: typeof unlink;
@@ -47,6 +58,7 @@ export interface FileSystem {
 const defaultFileSystem: FileSystem = {
 	mkdir,
 	readFile,
+	readdir,
 	rename,
 	stat,
 	unlink,
@@ -299,15 +311,19 @@ export class SshConfig {
 	private readonly fileSystem: FileSystem;
 	private readonly logger: Logger;
 	private raw: string | undefined;
+	/** Marks this file as Coder-managed; absent for the user's own config. */
+	private readonly permissions: ManagedPermissions | undefined;
 
 	constructor(
 		filePath: string,
 		logger: Logger,
 		fileSystem: FileSystem = defaultFileSystem,
+		permissions?: ManagedPermissions,
 	) {
 		this.filePath = filePath;
 		this.logger = logger;
 		this.fileSystem = fileSystem;
+		this.permissions = permissions;
 	}
 
 	async load() {
@@ -442,39 +458,99 @@ export class SshConfig {
 
 	/** Atomically write raw via a temp file. */
 	private async save(): Promise<void> {
-		// Preserve the existing file mode.
-		const existingMode = await this.fileSystem
-			.stat(this.filePath)
-			.then((stat) => stat.mode)
-			.catch((ex: NodeJS.ErrnoException) => {
-				if (ex.code === "ENOENT") {
-					return 0o600;
-				}
-				throw ex;
-			});
-		await this.fileSystem.mkdir(path.dirname(this.filePath), {
+		const existingMode = await this.getFileMode();
+		const fileName = path.basename(this.filePath);
+		const dirName = path.dirname(this.filePath);
+		await this.fileSystem.mkdir(dirName, {
 			mode: 0o700,
 			recursive: true,
 		});
-		const fileName = path.basename(this.filePath);
-		const dirName = path.dirname(this.filePath);
+		// Must come before any file reset or temporary write in this directory.
+		await this.permissions?.prepareDirectory(dirName);
+		await this.repairIncludedFiles(dirName);
 		const tempPath = tempFilePath(
 			`${dirName}/.${fileName}`,
 			"vscode-coder-tmp",
 		);
+		await this.writeTemp(tempPath, existingMode);
+		await this.repairPermissions(tempPath);
+		await this.replaceWithTemp(tempPath);
+	}
+
+	/** Preserve the existing file mode, defaulting to owner-only access. */
+	private async getFileMode(): Promise<number> {
+		try {
+			return (await this.fileSystem.stat(this.filePath)).mode;
+		} catch (error) {
+			if ((error as NodeJS.ErrnoException).code === "ENOENT") {
+				return 0o600;
+			}
+			throw error;
+		}
+	}
+
+	/** Repair every direct Include match; one unsafe sibling blocks every host. */
+	private async repairIncludedFiles(dirName: string): Promise<void> {
+		if (!this.permissions) return;
+		const entries = await this.fileSystem
+			.readdir(dirName, { withFileTypes: true })
+			.catch((error: unknown) => {
+				this.logger.warn(
+					"Failed to enumerate Coder-managed SSH config files",
+					error,
+				);
+				return [];
+			});
+		for (const entry of entries) {
+			if (!entry.name.toLowerCase().endsWith(SSH_CONFIG_EXT)) continue;
+			const filePath = path.join(dirName, entry.name);
+			// On Windows, fopen fails on a directory, so OpenSSH aborts the whole
+			// Include. No ACL change fixes that, so report it instead.
+			if (!entry.isFile()) {
+				throw new Error(
+					`SSH config entry ${filePath} is not a regular file. Move or rename it so it no longer matches *.conf, then reconnect.`,
+				);
+			}
+			await this.repairPermissions(filePath);
+		}
+	}
+
+	/** Create the temporary file exclusively, leaving any preexisting path alone. */
+	private async writeTemp(tempPath: string, mode: number): Promise<void> {
 		try {
 			await this.fileSystem.writeFile(tempPath, this.getRaw(), {
-				mode: existingMode,
 				encoding: "utf-8",
+				flag: "wx",
+				mode,
 			});
 		} catch (err) {
+			// On EEXIST this write did not create the path, so it must not delete it.
+			if ((err as NodeJS.ErrnoException).code !== "EEXIST") {
+				await this.discardTemp(tempPath);
+			}
 			throw new Error(
 				`Failed to write temporary SSH config file at ${tempPath}: ${err instanceof Error ? err.message : String(err)}. ` +
 					`Please check your disk space, permissions, and that the directory exists.`,
 				{ cause: err },
 			);
 		}
+	}
+
+	/** Log a repair failure without preventing an SSH connection attempt. */
+	private async repairPermissions(filePath: string): Promise<void> {
+		try {
+			await this.permissions?.secure(filePath);
+		} catch (error) {
+			this.logger.warn(
+				"Failed to repair SSH config permissions",
+				filePath,
+				error,
+			);
+		}
+	}
 
+	/** Replace the destination atomically, cleaning up if the rename fails. */
+	private async replaceWithTemp(tempPath: string): Promise<void> {
 		try {
 			await renameWithRetry(
 				(src, dest) => this.fileSystem.rename(src, dest),
@@ -493,6 +569,7 @@ export class SshConfig {
 		}
 	}
 
+	/** Attempt cleanup without hiding the original write or rename failure. */
 	private async discardTemp(tempPath: string): Promise<void> {
 		try {
 			await this.fileSystem.unlink(tempPath);