google-gemini / google-gemini/gemini-cli
Corrupt MCP enablement config silently re-enables servers, then disable() erases it
- Dominant language
- TypeScript
- Stars
- 107k
- Forks
- 14.6k
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 45
Description
### What happened?
`readConfig()` in `packages/cli/src/config/mcp/mcpServerEnablement.ts` collapses a JSON parse failure into the same empty object it uses for "file does not exist":
```ts
try {
const content = await fs.readFile(this.configFilePath, 'utf-8');
return JSON.parse(content) as McpServerEnablementConfig;
} catch (error) {
if (error instanceof Error && 'code' in error && error.code === 'ENOENT') {
return {};
}
coreEvents.emitFeedback('error', 'Failed to read MCP server enablement config.', error);
return {};
}
```
A `SyntaxError` from a hand-edit, a truncated write, or disk corruption is indistinguishable downstream from an absent file. Two consequences:
`isFileEnabled` defaults to enabled when a server has no entry:
```ts
const state = config[normalizeServerId(serverName)];
return state?.enabled ?? true;
```
With `{}`, every server the user deliberately disabled reports enabled, so it gets connected and its tools are exposed to the model.
Then `disable()` writes that same `{}` back with one key added:
```ts
async disable(serverName: string): Promise {
const config = await this.readConfig();
config[normalizeServerId(serverName)] = { enabled: false };
await this.writeConfig(config);
}
```
Every other entry in the file is gone. (`enable()` is guarded by `normalizedName in config`, so it is a no-op on `{}` and does not overwrite.)
### What did you expect to happen?
A malformed config is distinguished from a missing one. Disabled servers stay disabled, and the file is not overwritten while its contents cannot be read.
### Client information
Client Information
Source-level report, no `/about` output. Verified against:
```console
repository checkout: 4238b0b
package version: 0.56.0-nightly.20260806.g761f604c1
published CLI: 0.52.0
node: v24.10.0
OS: macOS 26.5.1
```
### Anything else we need to know?
Found by reading, not from a runtime failure, so no user report backs it. The path is straightforward though: `readConfig` is private and the only reader, and nothing validates upstream.
The security-relevant part is that this fails open across a trust boundary. A user who disabled an MCP server has it silently reconnected.
Fix direction: separate `ENOENT` from a parse failure. On parse failure, either fail closed or refuse to write until the user resolves it, and preserve the existing file rather than overwriting it.
Contributor guide
Research direction
Start in packages/cli/src/config/mcp/mcpServerEnablement.ts, tracing readConfig() into isFileEnabled() and disable(). Reproduce the malformed-JSON and missing-file cases, then verify that parse failures preserve the existing config and that disabled servers remain disabled without overwriting unreadable contents.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- cli, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 72/100