From 2b5dcb53740daa93f9650a60225781f05b39b7e8 Mon Sep 17 00:00:00 2001 From: walker Date: Mon, 24 Aug 2026 18:57:39 +0800 Subject: [PATCH] fix(cli): isolate concurrent credential temp files --- packages/cli/src/credentials.test.ts | 26 ++++++++++++++++++++++++-- packages/cli/src/credentials.ts | 11 ++++++++--- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/credentials.test.ts b/packages/cli/src/credentials.test.ts index 5b467ab9..eabf1f7b 100644 --- a/packages/cli/src/credentials.test.ts +++ b/packages/cli/src/credentials.test.ts @@ -1,7 +1,8 @@ -import { homedir } from 'node:os'; +import { mkdtemp, readFile, rm } from 'node:fs/promises'; +import { homedir, tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; -import { configDir } from './credentials.js'; +import { configDir, credentialsPath, writeCredentials } from './credentials.js'; const ORIGINAL_XDG_CONFIG_HOME = process.env.XDG_CONFIG_HOME; const ORIGINAL_HOME = process.env.HOME; @@ -29,3 +30,24 @@ describe('configDir', () => { expect(configDir()).toBe(join(homedir() || '.', '.config', 'sh1pt')); }); }); + +describe('writeCredentials', () => { + it('supports concurrent atomic writes without sharing a temporary file', async () => { + const root = await mkdtemp(join(tmpdir(), 'sh1pt-credentials-')); + process.env.XDG_CONFIG_HOME = root; + + try { + const credentials = Array.from({ length: 20 }, (_, index) => ({ + access_token: `access-${index}`, + refresh_token: `refresh-${index}`, + })); + + await expect(Promise.all(credentials.map(writeCredentials))).resolves.toHaveLength(20); + + const saved = JSON.parse(await readFile(credentialsPath(), 'utf8')); + expect(credentials).toContainEqual(saved); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/cli/src/credentials.ts b/packages/cli/src/credentials.ts index 823a2476..80137204 100644 --- a/packages/cli/src/credentials.ts +++ b/packages/cli/src/credentials.ts @@ -50,9 +50,14 @@ export async function writeCredentials(creds: Credentials): Promise { // Atomic write: write to a tmp file then rename, so a crash mid-write // never leaves credentials.json truncated/corrupt. Mirrors writeVault() in // local-vault.ts which holds equally sensitive data. - const tmp = `${path}.tmp`; - await fs.writeFile(tmp, JSON.stringify(creds, null, 2) + '\n', { encoding: 'utf8', mode: 0o600 }); - await fs.rename(tmp, path); + const tmp = `${path}.${process.pid}.${Math.random().toString(36).slice(2)}.tmp`; + try { + await fs.writeFile(tmp, JSON.stringify(creds, null, 2) + '\n', { encoding: 'utf8', mode: 0o600 }); + await fs.rename(tmp, path); + } catch (error) { + await fs.unlink(tmp).catch(() => {}); + throw error; + } // rename(2) preserves the source mode, but if the destination pre-existed // at a looser mode (e.g. 0644 from an older sh1pt build), the resulting // file keeps that loose mode. Explicitly tighten after rename.