Skip to content

Commit 4a896a4

Browse files
committed
fix(ssh): repair managed Windows SSH config permissions
Repair included fragments with built-in Windows tools and retain best-effort SSH setup. Inject permission repair as a collaborator so the config writer stays platform agnostic, skip files whose permissions already verify, and drive the real Windows tools from the native tests. Fixes #1108 Generated by Coder Agents.
1 parent 96dd908 commit 4a896a4

21 files changed

Lines changed: 1330 additions & 17 deletions

.gitattributes

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
11
* text=auto eol=lf
2+
pnpm-lock.yaml linguist-generated=true
3+
flake.lock linguist-generated=true

.github/workflows/ci.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ jobs:
4848
name: Windows,
4949
electron-version: "latest",
5050
}
51+
- {
52+
os: windows-11-arm,
53+
name: Windows ARM64,
54+
electron-version: "latest",
55+
}
5156
- { os: macos-15, name: macOS, electron-version: "latest" }
5257

5358
steps:
@@ -62,6 +67,7 @@ jobs:
6267
shell: bash
6368
env:
6469
CI: true
70+
EXPECTED_ARCH: ${{ runner.arch }}
6571

6672
test-integration:
6773
name: Integration Test (${{ matrix.name }}, VS Code ${{ matrix.vscode-version }})

.prettierrc.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"overrides": [
33
{
4-
"files": "*.jsonc",
4+
"files": ["*.jsonc", "assets/wsh/acl.js"],
55
"options": {
66
"trailingComma": "none"
77
}

.vscodeignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ coverage/**
55

66
# Development files
77
src/**
8+
assets/wsh/tsconfig.json
89
test/**
910
scripts/**
1011
**/*.ts

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,16 @@
55
from published versions since it shows up in the VS Code extension changelog
66
tab and is confusing to users. Add it back between releases if needed. -->
77

8+
## Unreleased
9+
10+
### Fixed
11+
12+
- Repair permissions on Coder-managed Windows SSH config files, including other
13+
deployments' files matched by the shared Include. This fixes connections blocked
14+
by inherited permissions or stale account access in an unrelated config. The
15+
main SSH config and directory permissions are left unchanged. If repair fails,
16+
log a warning and still attempt the SSH connection.
17+
818
## [v1.16.3](https://github.com/coder/vscode-coder/releases/tag/v1.16.3) 2026-09-14
919

1020
### Changed

CONTRIBUTING.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,26 @@ Alternatively:
213213
4. If your change is something users ought to be aware of, add an entry in the
214214
changelog.
215215

216+
### Windows SSH config permissions
217+
218+
Repair requires Windows Script Host, JScript, and ADSI to be allowed by policy.
219+
The extension does not request elevation or bypass policy. A standard user can
220+
repair files when they have access to read and change the DACL (`READ_CONTROL`
221+
and `WRITE_DAC`). If repair fails, it logs a warning and still attempts the SSH
222+
connection, which OpenSSH may then reject. Write and rename failures still stop
223+
setup. If the script fails after inheritance is disabled, copied grants remain
224+
until a successful retry; access is not reset to the parent's permissions.
225+
226+
`assets/wsh/acl.js` ships as source in the universal VSIX and runs under Windows
227+
Script Host, so it uses ES3 syntax rather than Node.js. Its sibling
228+
`tsconfig.json` and `globals.d.ts` keep the WScript and ADSI types out of the
229+
extension's Node.js environment; `pnpm typecheck` covers both. Typechecking does
230+
not transpile the asset, so keep indexed loops: `for...of` fails the ES3 lint
231+
check. A typecheck is not a runtime compatibility check, so `acl.native.test.ts`
232+
drives the real `icacls.exe`, `cscript.exe`, and `ssh.exe` instead of mocks. It
233+
runs whenever the tests run on Windows, including x64 and ARM64 in CI, and needs
234+
the OpenSSH client installed.
235+
216236
## Node.js Version
217237

218238
This extension targets the Node.js version bundled with VS Code's Electron:

assets/wsh/acl.js

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/** Windows Script Host runs this source directly using ES3-compatible JScript. */
2+
/**
3+
* Treat the target as a filesystem path, not a registry key or share.
4+
* @see https://learn.microsoft.com/en-us/windows/win32/api/iads/ne-iads-ads_pathtype_enum
5+
*/
6+
var ADS_PATH_FILE = 1;
7+
/**
8+
* Use the ADSI descriptor object so its DACL can be replaced.
9+
* @see https://learn.microsoft.com/en-us/windows/win32/api/iads/ne-iads-ads_sd_format_enum
10+
*/
11+
var ADS_SD_FORMAT_IID = 1;
12+
/**
13+
* Read and write only the DACL, preserving ownership and audit settings.
14+
* @see https://learn.microsoft.com/en-us/windows/win32/api/iads/ne-iads-ads_security_info_enum
15+
*/
16+
var ADS_SECURITY_INFO_DACL = 4;
17+
/**
18+
* Revision 2 supports the ordinary, non-object ACEs used for this file.
19+
* @see https://learn.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-acl
20+
*/
21+
var ACL_REVISION = 2;
22+
/**
23+
* Grant full file access to the three permitted trustees.
24+
* @see https://learn.microsoft.com/en-us/windows/win32/fileio/file-access-rights-constants
25+
*/
26+
var FILE_ALL_ACCESS = 0x1f01ff;
27+
/**
28+
* Numeric SID syntax avoids localized account-name resolution.
29+
* @see https://learn.microsoft.com/en-us/windows/win32/secauthz/sid-components
30+
*/
31+
var USER_SID_PATTERN = /^S-\d+(?:-\d+)+$/;
32+
33+
try {
34+
if (WScript.Arguments.length !== 2) {
35+
throw new Error("Expected a file path and user SID");
36+
}
37+
var target = WScript.Arguments.Item(0);
38+
var sid = WScript.Arguments.Item(1);
39+
if (!USER_SID_PATTERN.test(sid)) {
40+
throw new Error("Invalid Windows user SID");
41+
}
42+
43+
// Replace the file DACL through built-in ADSI without changing its owner.
44+
// https://learn.microsoft.com/en-us/windows/win32/api/iads/nn-iads-iadssecurityutility
45+
var security = new ActiveXObject("ADsSecurityUtility");
46+
security.SecurityMask = ADS_SECURITY_INFO_DACL;
47+
var descriptor = security.GetSecurityDescriptor(
48+
target,
49+
ADS_PATH_FILE,
50+
ADS_SD_FORMAT_IID
51+
);
52+
var dacl = new ActiveXObject("AccessControlList");
53+
dacl.AclRevision = ACL_REVISION;
54+
// Permit the current user, SYSTEM, and the local Administrators group.
55+
// https://learn.microsoft.com/en-us/windows/win32/secauthz/well-known-sids
56+
var trustees = [sid, "S-1-5-18", "S-1-5-32-544"];
57+
// Keep indexed iteration: this source is not transpiled, and ES3 has no for...of.
58+
for (var index = 0; index < trustees.length; index++) {
59+
var ace = new ActiveXObject("AccessControlEntry");
60+
ace.Trustee = trustees[index];
61+
ace.AccessMask = FILE_ALL_ACCESS;
62+
// An allow ACE with no inheritance flags applies only to this file.
63+
// https://learn.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-ace_header
64+
ace.AceType = 0;
65+
ace.AceFlags = 0;
66+
dacl.AddAce(ace);
67+
}
68+
descriptor.DiscretionaryAcl = dacl;
69+
security.SetSecurityDescriptor(
70+
target,
71+
ADS_PATH_FILE,
72+
descriptor,
73+
ADS_SD_FORMAT_IID
74+
);
75+
} catch (error) {
76+
/**
77+
* Automation errors carry `description` rather than `message`.
78+
* @see https://learn.microsoft.com/en-us/openspecs/ie_standards/ms-es3ex/a4f75a6b-dda5-40e1-85d0-9f95afb24fba
79+
* @type {any}
80+
*/
81+
var thrown = error;
82+
WScript.StdErr.WriteLine(
83+
"Windows ACL repair failed: " +
84+
((thrown && (thrown.message || thrown.description)) || thrown)
85+
);
86+
WScript.Quit(1);
87+
}

assets/wsh/globals.d.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/** Globals provided by Windows Script Host, not by Node.js. */
2+
declare var WScript: WScriptHost;
3+
/** Construct only the ADSI Automation objects used by the ACL setter. */
4+
declare var ActiveXObject: ActiveXObjectConstructor;
5+
6+
interface WScriptHost {
7+
/** Positional command-line arguments after the script path. */
8+
Arguments: {
9+
length: number;
10+
Item(index: number): string;
11+
};
12+
/** Diagnostic output captured by the extension. */
13+
StdErr: {
14+
WriteLine(message: string): void;
15+
};
16+
Quit(exitCode: number): void;
17+
}
18+
19+
interface ActiveXObjectConstructor {
20+
new (programId: "ADsSecurityUtility"): ADsSecurityUtility;
21+
new (programId: "AccessControlList"): AccessControlList;
22+
new (programId: "AccessControlEntry"): AccessControlEntry;
23+
}
24+
25+
interface ADsSecurityUtility {
26+
/** Select which security descriptor sections to read and write. */
27+
SecurityMask: number;
28+
GetSecurityDescriptor(
29+
path: string,
30+
pathType: number,
31+
format: number,
32+
): SecurityDescriptor;
33+
/** Persist only the sections selected by SecurityMask. */
34+
SetSecurityDescriptor(
35+
path: string,
36+
pathType: number,
37+
securityDescriptor: SecurityDescriptor,
38+
format: number,
39+
): void;
40+
}
41+
42+
interface SecurityDescriptor {
43+
/** File access rules, separate from ownership and audit settings. */
44+
DiscretionaryAcl: AccessControlList;
45+
}
46+
47+
interface AccessControlList {
48+
/** Set to 2 for ordinary file ACEs. */
49+
AclRevision: number;
50+
AddAce(accessControlEntry: AccessControlEntry): void;
51+
}
52+
53+
interface AccessControlEntry {
54+
/** SID or account name receiving this rule. */
55+
Trustee: string;
56+
AccessMask: number;
57+
/** 0 is access allowed. */
58+
AceType: number;
59+
/** 0 limits the rule to this file. */
60+
AceFlags: number;
61+
}

assets/wsh/tsconfig.json

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"compilerOptions": {
3+
"allowJs": true,
4+
"checkJs": true,
5+
"noEmit": true,
6+
"strict": true,
7+
"ignoreDeprecations": "6.0",
8+
"target": "ES5",
9+
"lib": ["ES5"],
10+
"types": []
11+
},
12+
"files": ["acl.js", "globals.d.ts"]
13+
}

eslint.config.mjs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,19 @@ export default defineConfig(
179179
},
180180
},
181181

182+
// Windows Script Host runs JScript with ES3 syntax and globals.
183+
{
184+
files: ["assets/wsh/acl.js"],
185+
languageOptions: {
186+
ecmaVersion: 3,
187+
sourceType: "script",
188+
globals: {
189+
ActiveXObject: "readonly",
190+
WScript: "readonly",
191+
},
192+
},
193+
},
194+
182195
// Build config - ESM with Node globals
183196
{
184197
files: ["esbuild.mjs", "scripts/*.mjs", ".storybook/themes/*.{mjs,cjs}"],

0 commit comments

Comments
 (0)