From c404a5b7dfee9ef3fbe4f8679006a809bb2b8124 Mon Sep 17 00:00:00 2001 From: nopp Date: Thu, 23 Jul 2026 01:47:56 +0700 Subject: [PATCH 1/2] fix(auth): lock credential profile mutations --- src/lib/credentials.test.ts | 14 ++++ src/lib/credentials.ts | 149 +++++++++++++++++++++++++++++++++--- 2 files changed, 151 insertions(+), 12 deletions(-) diff --git a/src/lib/credentials.test.ts b/src/lib/credentials.test.ts index 194c1ff..447d9d3 100644 --- a/src/lib/credentials.test.ts +++ b/src/lib/credentials.test.ts @@ -157,6 +157,19 @@ describe('writeProfile', () => { expect(file.dev).toEqual({ apiKey: 'sk-dev', apiUrl: 'https://dev' }); }); + it('reclaims stale mutation locks and removes the lock after writing', () => { + const lockPath = `${credentialsPath}.lock`; + writeFileSync( + lockPath, + `${JSON.stringify({ pid: process.pid, createdAt: Date.now() - 60_000, token: 'stale' })}\n`, + ); + + writeProfile('default', { apiKey: 'sk-new' }, { path: credentialsPath }); + + expect(readProfile('default', { path: credentialsPath })).toEqual({ apiKey: 'sk-new' }); + expect(existsSync(lockPath)).toBe(false); + }); + it('does not leak the api key into the on-disk file format aside from the value itself', () => { writeProfile('default', { apiKey: 'sk-secret-12345' }, { path: credentialsPath }); const onDisk = readFileSync(credentialsPath, 'utf-8'); @@ -168,6 +181,7 @@ describe('writeProfile', () => { describe('deleteProfile', () => { it('returns false when the profile is missing', () => { expect(deleteProfile('nope', { path: credentialsPath })).toBe(false); + expect(existsSync(credentialsPath)).toBe(false); }); it('removes the named profile and leaves others intact', () => { diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index 6c20b1f..15f8c5a 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -5,6 +5,7 @@ import { readFileSync, renameSync, statSync, + unlinkSync, writeFileSync, } from 'node:fs'; import { spawnSync, type SpawnSyncReturns } from 'node:child_process'; @@ -62,6 +63,12 @@ export interface CredentialsOptions { path?: string; } +interface CredentialsLockInfo { + pid?: number; + createdAt?: number; + token?: string; +} + interface RestrictiveModeOptions { platform?: NodeJS.Platform; env?: NodeJS.ProcessEnv; @@ -83,6 +90,10 @@ const FIELD_TO_FILE_KEY: Record = { apiUrl: 'api_url', }; +const CREDENTIALS_LOCK_RETRY_MS = 25; +const CREDENTIALS_LOCK_WAIT_MS = 5_000; +const CREDENTIALS_LOCK_STALE_MS = 30_000; + export function parseCredentials(content: string): CredentialsFile { const result: CredentialsFile = {}; let currentEntry: ProfileEntry | null = null; @@ -165,23 +176,24 @@ export function writeProfile( ): void { assertValidProfileName(profile); const path = resolvePath(options); - const file = readCredentialsFile(options); - file[profile] = { ...file[profile], ...entry }; - writeCredentialsAtomic(path, file); + mutateCredentialsFile(path, file => { + file[profile] = { ...file[profile], ...entry }; + return file; + }); } export function deleteProfile(profile: string, options: CredentialsOptions = {}): boolean { assertValidProfileName(profile); const path = resolvePath(options); - const file = readCredentialsFile(options); - if (!(profile in file)) return false; - delete file[profile]; - if (Object.keys(file).length === 0) { - writeCredentialsAtomic(path, {}); - } else { - writeCredentialsAtomic(path, file); - } - return true; + if (!existsSync(path)) return false; + let removed = false; + mutateCredentialsFile(path, file => { + if (!(profile in file)) return undefined; + removed = true; + delete file[profile]; + return file; + }); + return removed; } /** @@ -244,6 +256,22 @@ function resolvePath(options: CredentialsOptions): string { return options.path ?? defaultCredentialsPath(); } +function mutateCredentialsFile( + path: string, + mutate: (file: CredentialsFile) => CredentialsFile | undefined, +): void { + mkdirSync(dirname(path), { recursive: true, mode: 0o700 }); + const releaseLock = acquireCredentialsLock(path); + try { + const file = readCredentialsFile({ path }); + const nextFile = mutate(file); + if (nextFile === undefined) return; + writeCredentialsAtomic(path, nextFile); + } finally { + releaseLock(); + } +} + function writeCredentialsAtomic(path: string, file: CredentialsFile): void { mkdirSync(dirname(path), { recursive: true, mode: 0o700 }); const tmp = `${path}.tmp.${process.pid}`; @@ -251,3 +279,100 @@ function writeCredentialsAtomic(path: string, file: CredentialsFile): void { renameSync(tmp, path); ensureRestrictiveMode(path); } + +function acquireCredentialsLock(path: string): () => void { + const lockPath = `${path}.lock`; + const deadline = Date.now() + CREDENTIALS_LOCK_WAIT_MS; + const token = `${process.pid}:${Date.now()}:${Math.random().toString(16).slice(2)}`; + const lockInfo: Required = { + pid: process.pid, + createdAt: Date.now(), + token, + }; + + while (true) { + try { + writeFileSync(lockPath, `${JSON.stringify(lockInfo)}\n`, { + encoding: 'utf8', + flag: 'wx', + mode: 0o600, + }); + return () => releaseCredentialsLock(lockPath, token); + } catch (error) { + if (!isErrnoException(error) || error.code !== 'EEXIST') { + throw error; + } + + reclaimStaleCredentialsLock(lockPath); + if (Date.now() >= deadline) { + throw localValidationError( + 'credentialsLock', + 'timed out waiting for another credential update to finish; retry the command', + undefined, + 'field', + ); + } + sleepSync(CREDENTIALS_LOCK_RETRY_MS); + } + } +} + +function reclaimStaleCredentialsLock(lockPath: string): void { + let lockInfo: CredentialsLockInfo | undefined; + try { + lockInfo = JSON.parse(readFileSync(lockPath, 'utf-8')) as CredentialsLockInfo; + } catch { + lockInfo = undefined; + } + + const createdAt = typeof lockInfo?.createdAt === 'number' ? lockInfo.createdAt : undefined; + const ageMs = createdAt === undefined ? Number.POSITIVE_INFINITY : Date.now() - createdAt; + const pid = typeof lockInfo?.pid === 'number' ? lockInfo.pid : undefined; + if (ageMs <= CREDENTIALS_LOCK_STALE_MS && (pid === undefined || isProcessAlive(pid))) { + return; + } + + try { + unlinkSync(lockPath); + } catch (error) { + if (!isErrnoException(error) || error.code !== 'ENOENT') { + throw error; + } + } +} + +function releaseCredentialsLock(lockPath: string, token: string): void { + let lockInfo: CredentialsLockInfo | undefined; + try { + lockInfo = JSON.parse(readFileSync(lockPath, 'utf-8')) as CredentialsLockInfo; + } catch { + return; + } + if (lockInfo.token !== token) return; + try { + unlinkSync(lockPath); + } catch (error) { + if (!isErrnoException(error) || error.code !== 'ENOENT') { + throw error; + } + } +} + +function isProcessAlive(pid: number): boolean { + if (pid === process.pid) return true; + try { + process.kill(pid, 0); + return true; + } catch (error) { + if (isErrnoException(error) && error.code === 'ESRCH') return false; + return true; + } +} + +function isErrnoException(error: unknown): error is NodeJS.ErrnoException { + return error instanceof Error && 'code' in error; +} + +function sleepSync(ms: number): void { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms); +} From 92f7630a960f4a2cf66469c03285c82359a5a6b6 Mon Sep 17 00:00:00 2001 From: nopp Date: Thu, 23 Jul 2026 02:04:00 +0700 Subject: [PATCH 2/2] fix(auth): verify credential lock ownership --- src/lib/credentials.ts | 49 +++++++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index 15f8c5a..c67944b 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -9,6 +9,7 @@ import { writeFileSync, } from 'node:fs'; import { spawnSync, type SpawnSyncReturns } from 'node:child_process'; +import { randomUUID } from 'node:crypto'; import { homedir } from 'node:os'; import { dirname, join } from 'node:path'; import { localValidationError } from './errors.js'; @@ -69,6 +70,11 @@ interface CredentialsLockInfo { token?: string; } +interface CredentialsLock { + assertHeld: () => void; + release: () => void; +} + interface RestrictiveModeOptions { platform?: NodeJS.Platform; env?: NodeJS.ProcessEnv; @@ -261,29 +267,29 @@ function mutateCredentialsFile( mutate: (file: CredentialsFile) => CredentialsFile | undefined, ): void { mkdirSync(dirname(path), { recursive: true, mode: 0o700 }); - const releaseLock = acquireCredentialsLock(path); + const lock = acquireCredentialsLock(path); try { const file = readCredentialsFile({ path }); const nextFile = mutate(file); if (nextFile === undefined) return; + lock.assertHeld(); writeCredentialsAtomic(path, nextFile); } finally { - releaseLock(); + lock.release(); } } function writeCredentialsAtomic(path: string, file: CredentialsFile): void { - mkdirSync(dirname(path), { recursive: true, mode: 0o700 }); const tmp = `${path}.tmp.${process.pid}`; writeFileSync(tmp, serializeCredentials(file), { mode: 0o600, encoding: 'utf8' }); renameSync(tmp, path); ensureRestrictiveMode(path); } -function acquireCredentialsLock(path: string): () => void { +function acquireCredentialsLock(path: string): CredentialsLock { const lockPath = `${path}.lock`; const deadline = Date.now() + CREDENTIALS_LOCK_WAIT_MS; - const token = `${process.pid}:${Date.now()}:${Math.random().toString(16).slice(2)}`; + const token = `${process.pid}:${Date.now()}:${randomUUID()}`; const lockInfo: Required = { pid: process.pid, createdAt: Date.now(), @@ -297,7 +303,10 @@ function acquireCredentialsLock(path: string): () => void { flag: 'wx', mode: 0o600, }); - return () => releaseCredentialsLock(lockPath, token); + return { + assertHeld: () => assertCredentialsLockHeld(lockPath, token), + release: () => releaseCredentialsLock(lockPath, token), + }; } catch (error) { if (!isErrnoException(error) || error.code !== 'EEXIST') { throw error; @@ -341,14 +350,20 @@ function reclaimStaleCredentialsLock(lockPath: string): void { } } +function assertCredentialsLockHeld(lockPath: string, token: string): void { + const lockInfo = readCredentialsLockInfo(lockPath); + if (lockInfo?.token === token) return; + throw localValidationError( + 'credentialsLock', + 'lost ownership of the credential update lock; retry the command', + undefined, + 'field', + ); +} + function releaseCredentialsLock(lockPath: string, token: string): void { - let lockInfo: CredentialsLockInfo | undefined; - try { - lockInfo = JSON.parse(readFileSync(lockPath, 'utf-8')) as CredentialsLockInfo; - } catch { - return; - } - if (lockInfo.token !== token) return; + const lockInfo = readCredentialsLockInfo(lockPath); + if (lockInfo?.token !== token) return; try { unlinkSync(lockPath); } catch (error) { @@ -358,6 +373,14 @@ function releaseCredentialsLock(lockPath: string, token: string): void { } } +function readCredentialsLockInfo(lockPath: string): CredentialsLockInfo | undefined { + try { + return JSON.parse(readFileSync(lockPath, 'utf-8')) as CredentialsLockInfo; + } catch { + return undefined; + } +} + function isProcessAlive(pid: number): boolean { if (pid === process.pid) return true; try {