google-gemini / google-gemini/gemini-cli
bug: file-based credential store races concurrent writers and truncation permanently locks out saved credentials
- Dominant language
- TypeScript
- Stars
- 107k
- Forks
- 14.6k
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 45
Description
## What happened?
The file-based credential store implements every operation as a whole-file **read-modify-write with no locking and no atomic write**:
1. `setPassword()` / `deleteCredentials()` load the entire JSON, mutate one entry, and rewrite the whole file. Two concurrent writers (e.g., OAuth token refresh racing a login in another terminal/process) both read the same base snapshot; the second write silently erases the first's entry — a lost credential.
2. `saveData()` truncates-and-overwrites in place (`fs.writeFile` directly onto the live path). A crash or disk-full mid-write leaves a truncated file; since decryption of a truncated ciphertext always throws, `loadData()` then raises `"Corrupted credentials file detected..."` on every subsequent operation, locking the user out of **all** stored credentials until they manually delete the file.
## Affected code
`packages/core/src/services/fileKeychain.ts:119-135`:
```ts
async setPassword(service, account, password): Promise {
const data = await this.loadData(); // full read
if (!data[service]) { data[service] = {}; }
data[service][account] = password;
await this.saveData(data); // full write
}
```
`packages/core/src/services/fileKeychain.ts:110-117`:
```ts
const encrypted = this.encrypt(json);
await fs.writeFile(this.tokenFilePath, encrypted, { mode: 0o600 }); // not atomic
```
and the permanent-lockout path at ~95-105 (`Corrupted credentials file detected ... Please delete or rename this file`).
## How can this be reproduced?
- Race: run two processes performing `setPassword`/token saves simultaneously; one entry disappears.
- Truncation: interrupt the process (or fill the disk) during `saveData`; every later `getPassword` throws the corruption error.
## What did you expect to happen?
- Writes go to a temp file + atomic rename.
- Mutations are serialized (lockfile) or merged against a re-read snapshot.
## Impact
Silent credential loss and, in the truncation case, total lockout requiring manual intervention.
## Suggested direction
Temp-file + `fs.rename` for atomicity; a simple exclusive lockfile (with stale-lock recovery) around read-modify-write; optionally keep one `.bak` generation so a truncated file can self-heal.
---
*Found by source audit on current `main` (commit `5411f113c`); platform-independent. No open issue/PR covering this was found (searched: credentials file corrupted keychain).*
Contributor guide
Research direction
Start with packages/core/src/services/fileKeychain.ts, especially setPassword(), deleteCredentials(), saveData(), and loadData() around the cited lines. Reproduce concurrent credential updates and an interrupted save, then determine how atomic replacement, serialized read-modify-write operations, and recovery should be verified. Done means concurrent entries are preserved and interrupted writes no longer permanently prevent credential loading.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- authentication, cli, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100