google-gemini / google-gemini/gemini-cli

bug: file-based credential store races concurrent writers and truncation permanently locks out saved credentials

Open
#29,054 0 comments 0 reactions 0 assignees View on GitHub
area/security status/need-triage
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.