API-285: One-step MCP install (nansen mcp install <client>) - #487
API-285: One-step MCP install (nansen mcp install <client>)#487gulshngill wants to merge 4 commits into
Found 1 finding within acceptable thresholds
Review Status
✅ Passed
Findings
| Severity | Count |
|---|---|
| 🟡 Medium | 1 |
Review effort: 3/5 (Moderate)
Details
This is a well-engineered feature addition. The security threat-modelling is thorough — merge-only atomic writes, backup-before-write, key never printed, no shell injection, hardcoded HTTPS URL, pinned mcp-remote version. The test suite is comprehensive and all fs operations correctly flow through the injected fsx override. One medium-severity UX/logic issue found.
Findings
src/commands/mcp.js — medium
install --dry-run requires login, defeating its purpose
The login check at line 224–227 runs before the --dry-run branch at line 229:
// install
const apiKey = apiInstance?.apiKey;
if (!apiKey) {
throw new CommandError('Not logged in. Run: nansen login', 'NOT_LOGGED_IN');
}
if (flags['dry-run']) {
const redacted = buildServerEntry(client, '<redacted>');
...
}A user running nansen mcp install cursor --dry-run to preview the config path and entry shape before they log in will get Not logged in. Run: nansen login instead of the intended preview. This is especially jarring because the dry-run output explicitly redacts the key — there is no reason to require a real key for it.
Fix: Move the --dry-run branch above the apiKey guard:
// install
if (flags['dry-run']) {
const redacted = buildServerEntry(client, '<redacted>');
log(`Would write "${SERVER_KEY}" entry to ${configPath}:`);
log(JSON.stringify({ mcpServers: { [SERVER_KEY]: redacted } }, null, 2));
return undefined;
}
const apiKey = apiInstance?.apiKey;
if (!apiKey) {
throw new CommandError('Not logged in. Run: nansen login', 'NOT_LOGGED_IN');
}Add a test: run(['install', 'cursor'], { flags: { 'dry-run': true }, apiInstance: { apiKey: null } }) should resolve (not reject) and produce the redacted preview.
Note: Claude suggested: APPROVE_WITH_COMMENTS. Final status determined by severity thresholds.