Skip to content

Commit 06b450e

Browse files
fix(auth): tighten Windows credentials ACL
1 parent 60d55e4 commit 06b450e

2 files changed

Lines changed: 94 additions & 2 deletions

File tree

src/lib/credentials.test.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { mkdtempSync, statSync, readFileSync, writeFileSync, existsSync, mkdirSync } from 'node:fs';
22
import { homedir, tmpdir } from 'node:os';
33
import { join } from 'node:path';
4-
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
4+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
55
import {
66
DEFAULT_PROFILE,
77
assertValidProfileName,
@@ -191,6 +191,45 @@ describe('ensureRestrictiveMode', () => {
191191
expect(() => ensureRestrictiveMode(credentialsPath)).not.toThrow();
192192
});
193193

194+
it('tightens the Windows ACL with icacls instead of POSIX chmod', () => {
195+
mkdirSync(tmpRoot, { recursive: true });
196+
writeFileSync(credentialsPath, 'data', { mode: 0o666 });
197+
const spawn = vi.fn(() => ({ status: 0, signal: null, output: [], pid: 123 })) as never;
198+
199+
ensureRestrictiveMode(credentialsPath, {
200+
platform: 'win32',
201+
env: { USERNAME: 'alice' } as NodeJS.ProcessEnv,
202+
spawnSync: spawn,
203+
});
204+
205+
expect(spawn).toHaveBeenCalledWith(
206+
'icacls',
207+
[credentialsPath, '/inheritance:r', '/grant:r', 'alice:F'],
208+
{
209+
shell: false,
210+
stdio: 'ignore',
211+
windowsHide: true,
212+
},
213+
);
214+
});
215+
216+
it('warns on Windows when credentials ACL tightening cannot run', () => {
217+
mkdirSync(tmpRoot, { recursive: true });
218+
writeFileSync(credentialsPath, 'data');
219+
const warnings: string[] = [];
220+
const spawn = vi.fn(() => ({ status: 0, signal: null, output: [], pid: 123 })) as never;
221+
222+
ensureRestrictiveMode(credentialsPath, {
223+
platform: 'win32',
224+
env: {} as NodeJS.ProcessEnv,
225+
spawnSync: spawn,
226+
warn: line => warnings.push(line),
227+
});
228+
229+
expect(spawn).not.toHaveBeenCalled();
230+
expect(warnings.join('\n')).toContain('credentials file permissions were not tightened');
231+
});
232+
194233
// POSIX-only premise: Windows has no 0644/0600 distinction to downgrade.
195234
it.skipIf(process.platform === 'win32')('downgrades over-permissive modes', () => {
196235
mkdirSync(tmpRoot, { recursive: true });

src/lib/credentials.ts

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
statSync,
88
writeFileSync,
99
} from 'node:fs';
10+
import { spawnSync, type SpawnSyncReturns } from 'node:child_process';
1011
import { homedir } from 'node:os';
1112
import { dirname, join } from 'node:path';
1213
import { localValidationError } from './errors.js';
@@ -61,6 +62,17 @@ export interface CredentialsOptions {
6162
path?: string;
6263
}
6364

65+
interface RestrictiveModeOptions {
66+
platform?: NodeJS.Platform;
67+
env?: NodeJS.ProcessEnv;
68+
spawnSync?: (
69+
command: string,
70+
args: readonly string[],
71+
options: { shell: false; stdio: 'ignore'; windowsHide: true },
72+
) => SpawnSyncReturns<Buffer>;
73+
warn?: (line: string) => void;
74+
}
75+
6476
const FILE_KEY_TO_FIELD: Record<string, keyof ProfileEntry> = {
6577
api_key: 'apiKey',
6678
api_url: 'apiUrl',
@@ -172,12 +184,53 @@ export function deleteProfile(profile: string, options: CredentialsOptions = {})
172184
return true;
173185
}
174186

175-
export function ensureRestrictiveMode(path: string): void {
187+
export function ensureRestrictiveMode(path: string, options: RestrictiveModeOptions = {}): void {
176188
if (!existsSync(path)) return;
189+
if ((options.platform ?? process.platform) === 'win32') {
190+
ensureWindowsRestrictiveAcl(path, options);
191+
return;
192+
}
177193
const overpermissive = (statSync(path).mode & 0o077) !== 0;
178194
if (overpermissive) chmodSync(path, 0o600);
179195
}
180196

197+
function ensureWindowsRestrictiveAcl(path: string, options: RestrictiveModeOptions): void {
198+
const username = (options.env ?? process.env).USERNAME?.trim();
199+
if (!username) {
200+
warnWindowsAcl(
201+
'could not determine the Windows username; credentials file permissions were not tightened',
202+
options,
203+
);
204+
return;
205+
}
206+
207+
const run = options.spawnSync ?? spawnSync;
208+
const result = run('icacls', [path, '/inheritance:r', '/grant:r', `${username}:F`], {
209+
shell: false,
210+
stdio: 'ignore',
211+
windowsHide: true,
212+
});
213+
214+
if (result.error) {
215+
warnWindowsAcl(
216+
`icacls failed while tightening credentials file permissions: ${result.error.message}`,
217+
options,
218+
);
219+
return;
220+
}
221+
if (result.status !== 0) {
222+
warnWindowsAcl(
223+
`icacls exited with status ${result.status ?? 'unknown'}; credentials file permissions may be too broad`,
224+
options,
225+
);
226+
}
227+
}
228+
229+
function warnWindowsAcl(message: string, options: RestrictiveModeOptions): void {
230+
const warn = options.warn ?? ((line: string) => process.stderr.write(`${line}\n`));
231+
warn(`[warning] ${message}`);
232+
}
233+
181234
function resolvePath(options: CredentialsOptions): string {
182235
return options.path ?? defaultCredentialsPath();
183236
}

0 commit comments

Comments
 (0)