Skip to content

Commit d821dac

Browse files
fix(auth): lock credential profile mutations (#272)
* fix(auth): lock credential profile mutations * fix(auth): verify credential lock ownership
1 parent 251b593 commit d821dac

2 files changed

Lines changed: 175 additions & 13 deletions

File tree

src/lib/credentials.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,19 @@ describe('writeProfile', () => {
157157
expect(file.dev).toEqual({ apiKey: 'sk-dev', apiUrl: 'https://dev' });
158158
});
159159

160+
it('reclaims stale mutation locks and removes the lock after writing', () => {
161+
const lockPath = `${credentialsPath}.lock`;
162+
writeFileSync(
163+
lockPath,
164+
`${JSON.stringify({ pid: process.pid, createdAt: Date.now() - 60_000, token: 'stale' })}\n`,
165+
);
166+
167+
writeProfile('default', { apiKey: 'sk-new' }, { path: credentialsPath });
168+
169+
expect(readProfile('default', { path: credentialsPath })).toEqual({ apiKey: 'sk-new' });
170+
expect(existsSync(lockPath)).toBe(false);
171+
});
172+
160173
it('does not leak the api key into the on-disk file format aside from the value itself', () => {
161174
writeProfile('default', { apiKey: 'sk-secret-12345' }, { path: credentialsPath });
162175
const onDisk = readFileSync(credentialsPath, 'utf-8');
@@ -168,6 +181,7 @@ describe('writeProfile', () => {
168181
describe('deleteProfile', () => {
169182
it('returns false when the profile is missing', () => {
170183
expect(deleteProfile('nope', { path: credentialsPath })).toBe(false);
184+
expect(existsSync(credentialsPath)).toBe(false);
171185
});
172186

173187
it('removes the named profile and leaves others intact', () => {

src/lib/credentials.ts

Lines changed: 161 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ import {
55
readFileSync,
66
renameSync,
77
statSync,
8+
unlinkSync,
89
writeFileSync,
910
} from 'node:fs';
1011
import { spawnSync, type SpawnSyncReturns } from 'node:child_process';
12+
import { randomUUID } from 'node:crypto';
1113
import { homedir } from 'node:os';
1214
import { dirname, join } from 'node:path';
1315
import { localValidationError } from './errors.js';
@@ -62,6 +64,17 @@ export interface CredentialsOptions {
6264
path?: string;
6365
}
6466

67+
interface CredentialsLockInfo {
68+
pid?: number;
69+
createdAt?: number;
70+
token?: string;
71+
}
72+
73+
interface CredentialsLock {
74+
assertHeld: () => void;
75+
release: () => void;
76+
}
77+
6578
interface RestrictiveModeOptions {
6679
platform?: NodeJS.Platform;
6780
env?: NodeJS.ProcessEnv;
@@ -83,6 +96,10 @@ const FIELD_TO_FILE_KEY: Record<keyof ProfileEntry, string> = {
8396
apiUrl: 'api_url',
8497
};
8598

99+
const CREDENTIALS_LOCK_RETRY_MS = 25;
100+
const CREDENTIALS_LOCK_WAIT_MS = 5_000;
101+
const CREDENTIALS_LOCK_STALE_MS = 30_000;
102+
86103
export function parseCredentials(content: string): CredentialsFile {
87104
const result: CredentialsFile = {};
88105
let currentEntry: ProfileEntry | null = null;
@@ -165,23 +182,24 @@ export function writeProfile(
165182
): void {
166183
assertValidProfileName(profile);
167184
const path = resolvePath(options);
168-
const file = readCredentialsFile(options);
169-
file[profile] = { ...file[profile], ...entry };
170-
writeCredentialsAtomic(path, file);
185+
mutateCredentialsFile(path, file => {
186+
file[profile] = { ...file[profile], ...entry };
187+
return file;
188+
});
171189
}
172190

173191
export function deleteProfile(profile: string, options: CredentialsOptions = {}): boolean {
174192
assertValidProfileName(profile);
175193
const path = resolvePath(options);
176-
const file = readCredentialsFile(options);
177-
if (!(profile in file)) return false;
178-
delete file[profile];
179-
if (Object.keys(file).length === 0) {
180-
writeCredentialsAtomic(path, {});
181-
} else {
182-
writeCredentialsAtomic(path, file);
183-
}
184-
return true;
194+
if (!existsSync(path)) return false;
195+
let removed = false;
196+
mutateCredentialsFile(path, file => {
197+
if (!(profile in file)) return undefined;
198+
removed = true;
199+
delete file[profile];
200+
return file;
201+
});
202+
return removed;
185203
}
186204

187205
/**
@@ -244,10 +262,140 @@ function resolvePath(options: CredentialsOptions): string {
244262
return options.path ?? defaultCredentialsPath();
245263
}
246264

247-
function writeCredentialsAtomic(path: string, file: CredentialsFile): void {
265+
function mutateCredentialsFile(
266+
path: string,
267+
mutate: (file: CredentialsFile) => CredentialsFile | undefined,
268+
): void {
248269
mkdirSync(dirname(path), { recursive: true, mode: 0o700 });
270+
const lock = acquireCredentialsLock(path);
271+
try {
272+
const file = readCredentialsFile({ path });
273+
const nextFile = mutate(file);
274+
if (nextFile === undefined) return;
275+
lock.assertHeld();
276+
writeCredentialsAtomic(path, nextFile);
277+
} finally {
278+
lock.release();
279+
}
280+
}
281+
282+
function writeCredentialsAtomic(path: string, file: CredentialsFile): void {
249283
const tmp = `${path}.tmp.${process.pid}`;
250284
writeFileSync(tmp, serializeCredentials(file), { mode: 0o600, encoding: 'utf8' });
251285
renameSync(tmp, path);
252286
ensureRestrictiveMode(path);
253287
}
288+
289+
function acquireCredentialsLock(path: string): CredentialsLock {
290+
const lockPath = `${path}.lock`;
291+
const deadline = Date.now() + CREDENTIALS_LOCK_WAIT_MS;
292+
const token = `${process.pid}:${Date.now()}:${randomUUID()}`;
293+
const lockInfo: Required<CredentialsLockInfo> = {
294+
pid: process.pid,
295+
createdAt: Date.now(),
296+
token,
297+
};
298+
299+
while (true) {
300+
try {
301+
writeFileSync(lockPath, `${JSON.stringify(lockInfo)}\n`, {
302+
encoding: 'utf8',
303+
flag: 'wx',
304+
mode: 0o600,
305+
});
306+
return {
307+
assertHeld: () => assertCredentialsLockHeld(lockPath, token),
308+
release: () => releaseCredentialsLock(lockPath, token),
309+
};
310+
} catch (error) {
311+
if (!isErrnoException(error) || error.code !== 'EEXIST') {
312+
throw error;
313+
}
314+
315+
reclaimStaleCredentialsLock(lockPath);
316+
if (Date.now() >= deadline) {
317+
throw localValidationError(
318+
'credentialsLock',
319+
'timed out waiting for another credential update to finish; retry the command',
320+
undefined,
321+
'field',
322+
);
323+
}
324+
sleepSync(CREDENTIALS_LOCK_RETRY_MS);
325+
}
326+
}
327+
}
328+
329+
function reclaimStaleCredentialsLock(lockPath: string): void {
330+
let lockInfo: CredentialsLockInfo | undefined;
331+
try {
332+
lockInfo = JSON.parse(readFileSync(lockPath, 'utf-8')) as CredentialsLockInfo;
333+
} catch {
334+
lockInfo = undefined;
335+
}
336+
337+
const createdAt = typeof lockInfo?.createdAt === 'number' ? lockInfo.createdAt : undefined;
338+
const ageMs = createdAt === undefined ? Number.POSITIVE_INFINITY : Date.now() - createdAt;
339+
const pid = typeof lockInfo?.pid === 'number' ? lockInfo.pid : undefined;
340+
if (ageMs <= CREDENTIALS_LOCK_STALE_MS && (pid === undefined || isProcessAlive(pid))) {
341+
return;
342+
}
343+
344+
try {
345+
unlinkSync(lockPath);
346+
} catch (error) {
347+
if (!isErrnoException(error) || error.code !== 'ENOENT') {
348+
throw error;
349+
}
350+
}
351+
}
352+
353+
function assertCredentialsLockHeld(lockPath: string, token: string): void {
354+
const lockInfo = readCredentialsLockInfo(lockPath);
355+
if (lockInfo?.token === token) return;
356+
throw localValidationError(
357+
'credentialsLock',
358+
'lost ownership of the credential update lock; retry the command',
359+
undefined,
360+
'field',
361+
);
362+
}
363+
364+
function releaseCredentialsLock(lockPath: string, token: string): void {
365+
const lockInfo = readCredentialsLockInfo(lockPath);
366+
if (lockInfo?.token !== token) return;
367+
try {
368+
unlinkSync(lockPath);
369+
} catch (error) {
370+
if (!isErrnoException(error) || error.code !== 'ENOENT') {
371+
throw error;
372+
}
373+
}
374+
}
375+
376+
function readCredentialsLockInfo(lockPath: string): CredentialsLockInfo | undefined {
377+
try {
378+
return JSON.parse(readFileSync(lockPath, 'utf-8')) as CredentialsLockInfo;
379+
} catch {
380+
return undefined;
381+
}
382+
}
383+
384+
function isProcessAlive(pid: number): boolean {
385+
if (pid === process.pid) return true;
386+
try {
387+
process.kill(pid, 0);
388+
return true;
389+
} catch (error) {
390+
if (isErrnoException(error) && error.code === 'ESRCH') return false;
391+
return true;
392+
}
393+
}
394+
395+
function isErrnoException(error: unknown): error is NodeJS.ErrnoException {
396+
return error instanceof Error && 'code' in error;
397+
}
398+
399+
function sleepSync(ms: number): void {
400+
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms);
401+
}

0 commit comments

Comments
 (0)