diff --git a/src/lib/credentials.test.ts b/src/lib/credentials.test.ts index ac038f3..c8df383 100644 --- a/src/lib/credentials.test.ts +++ b/src/lib/credentials.test.ts @@ -1,7 +1,11 @@ +import { execSync } from 'node:child_process'; +import type { ChildProcess } from 'node:child_process'; +import { spawn } from 'node:child_process'; import { mkdtempSync, statSync, readFileSync, writeFileSync, existsSync, mkdirSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { fileURLToPath } from 'node:url'; +import { afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest'; import { DEFAULT_PROFILE, assertValidProfileName, @@ -171,6 +175,65 @@ describe('defaultCredentialsPath', () => { }); }); +const projectRoot = fileURLToPath(new URL('../..', import.meta.url)); +const credentialsWriteChild = join(projectRoot, 'test/helpers/credentials-write-child.mjs'); + +function waitForChild(child: ChildProcess): Promise { + return new Promise((resolve, reject) => { + let stderr = ''; + child.stderr?.on('data', chunk => { + stderr += String(chunk); + }); + child.on('error', reject); + child.on('close', code => { + if (code === 0) resolve(); + else reject(new Error(`child exited with code ${code}: ${stderr}`)); + }); + }); +} + +function spawnCredentialsWriter(profile: string, apiKey: string, path: string): ChildProcess { + return spawn(process.execPath, [credentialsWriteChild], { + env: { + ...process.env, + CRED_PROFILE: profile, + CRED_PATH: path, + CRED_API_KEY: apiKey, + }, + stdio: ['ignore', 'ignore', 'pipe'], + }); +} + +describe('credentials write lock', () => { + beforeAll(() => { + execSync('npm run build', { cwd: projectRoot, stdio: 'pipe' }); + }); + + it('serializes cross-process writes so concurrent profile updates are not lost', async () => { + const children = [ + spawnCredentialsWriter('dev', 'sk-dev', credentialsPath), + spawnCredentialsWriter('staging', 'sk-staging', credentialsPath), + ]; + await Promise.all(children.map(waitForChild)); + + const file = readCredentialsFile({ path: credentialsPath }); + expect(file.dev).toEqual({ apiKey: 'sk-dev' }); + expect(file.staging).toEqual({ apiKey: 'sk-staging' }); + }); + + it('removes the lock file after writeProfile completes', () => { + writeProfile('default', { apiKey: 'sk-lock-cleanup' }, { path: credentialsPath }); + expect(existsSync(`${credentialsPath}.lock`)).toBe(false); + }); + + it('removes the lock file after deleteProfile completes', () => { + writeProfile('default', { apiKey: 'sk-d' }, { path: credentialsPath }); + writeProfile('dev', { apiKey: 'sk-dev' }, { path: credentialsPath }); + expect(deleteProfile('dev', { path: credentialsPath })).toBe(true); + expect(existsSync(`${credentialsPath}.lock`)).toBe(false); + }); +}); + describe('assertValidProfileName / profile-name guard', () => { it('accepts conventional profile names', () => { for (const name of ['default', 'dev', 'prod', 'ci-staging', 'team.qa', 'a_b', 'P1']) { diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index c885c66..07db962 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -1,10 +1,13 @@ import { chmodSync, + closeSync, existsSync, mkdirSync, + openSync, readFileSync, renameSync, statSync, + unlinkSync, writeFileSync, } from 'node:fs'; import { homedir } from 'node:os'; @@ -145,23 +148,27 @@ export function writeProfile( ): void { assertValidProfileName(profile); const path = resolvePath(options); - const file = readCredentialsFile(options); - file[profile] = { ...file[profile], ...entry }; - writeCredentialsAtomic(path, file); + withCredentialsLock(path, () => { + const file = readCredentialsFile(options); + file[profile] = { ...file[profile], ...entry }; + writeCredentialsAtomic(path, 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; + return withCredentialsLock(path, () => { + 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; + }); } export function ensureRestrictiveMode(path: string): void { @@ -181,3 +188,94 @@ function writeCredentialsAtomic(path: string, file: CredentialsFile): void { renameSync(tmp, path); ensureRestrictiveMode(path); } + +/** Max wall-clock wait when another process holds the credentials lock. */ +const CREDENTIALS_LOCK_MAX_WAIT_MS = 10_000; +/** Back-off between lock attempts. */ +const CREDENTIALS_LOCK_RETRY_MS = 25; +/** Reclaim a lock file when the holder pid is gone or the file is older than this. */ +const CREDENTIALS_LOCK_STALE_MS = 30_000; + +function credentialsLockPath(credentialsPath: string): string { + return `${credentialsPath}.lock`; +} + +/** + * Serialize read-modify-write on the credentials file across processes. + * `writeCredentialsAtomic` only makes the final rename atomic; without this + * lock, concurrent `writeProfile` / `deleteProfile` calls can each read the + * same snapshot and the last rename wins — silently dropping the other update. + */ +function withCredentialsLock(credentialsPath: string, fn: () => T): T { + acquireCredentialsLock(credentialsPath); + try { + return fn(); + } finally { + releaseCredentialsLock(credentialsPath); + } +} + +function acquireCredentialsLock(credentialsPath: string): void { + const lockPath = credentialsLockPath(credentialsPath); + // Ensure the credentials directory exists before creating the lock file. + // writeCredentialsAtomic also mkdirs, but only after the lock is held. + mkdirSync(dirname(lockPath), { recursive: true, mode: 0o700 }); + const deadline = Date.now() + CREDENTIALS_LOCK_MAX_WAIT_MS; + while (Date.now() < deadline) { + try { + const fd = openSync(lockPath, 'wx'); + try { + writeFileSync(fd, `${process.pid}\n${Date.now()}\n`, 'utf8'); + } finally { + closeSync(fd); + } + return; + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code !== 'EEXIST') throw err; + if (isStaleCredentialsLock(lockPath)) { + try { + unlinkSync(lockPath); + } catch { + // Another waiter may have claimed or released the lock. + } + continue; + } + syncSleep(CREDENTIALS_LOCK_RETRY_MS); + } + } + throw new Error(`Timed out acquiring credentials lock: ${lockPath}`); +} + +function releaseCredentialsLock(credentialsPath: string): void { + try { + unlinkSync(credentialsLockPath(credentialsPath)); + } catch { + // Lock already released or never acquired — teardown must not mask errors. + } +} + +function isStaleCredentialsLock(lockPath: string): boolean { + try { + const stat = statSync(lockPath); + if (Date.now() - stat.mtimeMs > CREDENTIALS_LOCK_STALE_MS) return true; + const firstLine = readFileSync(lockPath, 'utf8').split('\n')[0] ?? ''; + const pid = Number.parseInt(firstLine, 10); + if (!Number.isFinite(pid) || pid <= 0) return true; + try { + process.kill(pid, 0); + return false; + } catch { + return true; + } + } catch { + return false; + } +} + +function syncSleep(ms: number): void { + const until = Date.now() + ms; + while (Date.now() < until) { + // Busy-wait: credentials I/O is sync-only; sub-ms precision is unnecessary. + } +} diff --git a/test/helpers/credentials-write-child.mjs b/test/helpers/credentials-write-child.mjs new file mode 100644 index 0000000..c5b561b --- /dev/null +++ b/test/helpers/credentials-write-child.mjs @@ -0,0 +1,12 @@ +import { writeProfile } from '../../dist/lib/credentials.js'; + +const profile = process.env.CRED_PROFILE; +const credentialsPath = process.env.CRED_PATH; +const apiKey = process.env.CRED_API_KEY; + +if (!profile || !credentialsPath || !apiKey) { + console.error('CRED_PROFILE, CRED_PATH, and CRED_API_KEY are required'); + process.exit(1); +} + +writeProfile(profile, { apiKey }, { path: credentialsPath });