Skip to content

API-285: One-step MCP install (nansen mcp install <client>) - #487

Open
gulshngill wants to merge 4 commits into
mainfrom
feat/api-285-mcp-install
Open

API-285: One-step MCP install (nansen mcp install <client>)#487
gulshngill wants to merge 4 commits into
mainfrom
feat/api-285-mcp-install

Conversation

@gulshngill

Copy link
Copy Markdown
Contributor

Summary

Implements API-285 — one-step install of the hosted Nansen MCP server into local MCP clients:

nansen mcp install claude-code | claude-desktop | cursor   [--dry-run]
nansen mcp uninstall <client>

The command writes a nansen entry into the client's own config file using the API key from nansen login / NANSEN_API_KEY. No network calls, no shelling out — pure fs operations.

Provider research (public sources)

Surveyed one-step-install mechanisms from Nansen's own MCP docs, Claude Code (claude mcp add, .mcp.json), Claude Desktop, Cursor (~/.cursor/mcp.json + deeplinks), VS Code (servers key, code --add-mcp), Codex, Gemini CLI, and vendor installers (Sentry wizard, Smithery CLI, Stripe, GitHub MCP badges). Key facts driving the design:

  • Nansen's server is hosted streamable HTTP at https://mcp.nansen.ai/ra/mcp, auth via NANSEN-API-KEY header (docs) — so entries are remote-URL, no local server process.
  • Claude Code ~/.claude.json entries require "type": "http" next to url; Cursor infers from url; Claude Desktop's config is stdio-only, so it bridges via npx mcp-remote (pinned mcp-remote@0.1.38, header arg written without a space after the colon to dodge Claude Desktop's arg-splitting bug).
  • Industry best practices adopted: merge-only JSON writes, backup before write, idempotent re-install (also the key-rotation path), --dry-run, uninstall support.

Security decisions (threat-modeled independently)

  • No config clobbering (highest practical risk — these files hold users' other MCP servers): only mcpServers.nansen is ever assigned; all sibling servers and unrelated keys pass through. Unparseable JSON → refuse with the file path, never repair/overwrite. .bak copy before every install write. Atomic temp-file + rename so a crash can't truncate the config. Type guard on mcpServers.
  • Key exposure: the key is never printed — not in output, --dry-run (redacted), or errors. New dirs 0700, files 0600, backup 0600, existing target chmod'd 0600 post-write (best-effort). Explicit plaintext + settings-sync warnings on install. Telemetry already sends flag names only, so no key material can leak there.
  • No injection surface: no shelling out (claude mcp add etc. deliberately not exec'd); client name validated against a closed set before any path math; no user-supplied paths.
  • Supply chain: server URL is a hardcoded HTTPS constant (no --url/env override — a redirectable URL would exfiltrate the key); mcp-remote pinned exact; the docs' --allow-http flag deliberately dropped (URL is HTTPS).
  • Symlinked configs (dotfile managers) are followed via realpathSync so the rename edits the real file instead of replacing the link. TOCTOU judged not realistic (same-user home dir).

Tests

New src/__tests__/mcp.test.js (27 tests): per-platform path resolution incl. claude-desktop-on-Linux error, per-client entry shapes (pinned version, no --allow-http, no-space header), merge/remove purity + non-object mcpServers guards, and handler tests against real temp dirs — file/dir modes, backup content + mode, idempotent re-install, corrupt-JSON refusal (file untouched), not-logged-in (no writes), --dry-run (no writes, key never printed), key-never-in-output, uninstall (incl. no-key and no-entry paths), symlink follow, schema.json registration, and runCLI routing / --dry-run boolean-flag parsing.

  • npm test: 52 files, 1913 passed / 2 skipped ✅
  • npm run lint: clean ✅
  • Manual smoke: install/uninstall round-trip against a fake $HOME, help paths, nansen schema mcp.

Limitations / follow-up

  • v1 clients: claude-code, claude-desktop, cursor. VS Code/Windsurf/Codex/Gemini punted (VS Code configs are JSONC; Codex is TOML) — docs link covers manual setup.
  • ~/.claude.json is also rewritten by live Claude Code sessions — a session saving state after our write can drop the entry (last-writer-wins, not corruption). Output tells the user to restart; if it bites, fallback is execFile('claude', ['mcp','add',...]).
  • mcp-remote pin (0.1.38) trades missed upstream security fixes for protection against compromised future releases; bumping is a one-constant change.
  • Windows file-permission hardening relies on default per-user profile ACLs (no chmod equivalent applied).
  • Note: scripts/postinstall.js already distributes agent skills — MCP install is a second, parallel distribution channel; worth a docs pass later on when to use which.

🤖 Generated with Claude Code

@nansen-pr-reviewer

nansen-pr-reviewer Bot commented Aug 13, 2026

Copy link
Copy Markdown

pr-reviewer Summary for #12d05f5

📝 1 finding

Review completed. Please address the findings below.

Findings by Severity

Severity Count
🟡 Medium 1

Review effort: 3/5 (Moderate)

Summary

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.


Token usage: 9,461 input, 4,562 output, 740,873 cache read, 44,728 cache write | Usage Guide

New pushes are reviewed automatically with a 10-minute cooldown between reviews. To request a review at any time, comment @nansen-pr-reviewer re-review.

Add `nansen mcp install/uninstall <client>` to write the hosted Nansen MCP
server (https://mcp.nansen.ai/ra/mcp) into Claude Code, Claude Desktop, or
Cursor configs. Merge-only atomic writes with backup, key never printed,
--dry-run supported.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@gulshngill
gulshngill force-pushed the feat/api-285-mcp-install branch from a49f962 to e2bc3b3 Compare August 13, 2026 16:59
@gulshngill

Copy link
Copy Markdown
Contributor Author

🤖 Automated Branch Sync (claude-code) — 2026-08-13T17:02:07Z

Action: Rebased feat/api-285-mcp-install onto main (was 9 commits behind)
Status: mergeable ✅ — a49f962e2bc3b3, all CI green

Conflict resolved (1 file): src/cli.js — the parseArgs boolean-flag list. Both sides appended a flag to the same line: main added offline (#486 doctor/auth), this PR added dry-run. Kept both.

Auto-merged cleanly, both intents verified intact:

  • src/cli.jsbuildMcpCommands import + spread alongside doctor.js import and isOfflineCommand tracking
  • src/cli.js help banner — mcp, auth and doctor lines all present
  • src/schema.jsonmcp, auth, doctor all present, parses clean
  • README.md — MCP section + main's doctor/auth status troubleshooting rows

Verified: full suite 1985 passed / 2 skipped / 0 failed, eslint clean locally; CI lint + test (20/22/24) + Aikido + pr-reviewer all pass.

⚠️ mergeStateStatus is BLOCKED solely on REVIEW_REQUIRED — no conflicts, no failing checks.

Left for a human (not changed during sync): main introduced isOfflineCommand, which suppresses the background update-check, cost-map refresh and telemetry for commands promising zero network. mcp install/uninstall only writes local config files, so it arguably belongs in that set — but adding it is a behaviour decision, not a conflict resolution, so it was deliberately left alone.

@gulshngill

Copy link
Copy Markdown
Contributor Author

🤖 Automated Branch Sync (claude-code) — 2026-08-13T18:56:53Z

Action: No rebase needed — feat/api-285-mcp-install is 0 commits behind main (merge base 01b887bd == main tip). Branch left untouched; no force-push.
Status: mergeable ✅ (MERGEABLE)

Merge state is BLOCKED only because reviewDecision: REVIEW_REQUIRED — all checks pass (lint, test 20/22/24, Aikido, pr-reviewer). Needs a human approval, not a sync.

@gulshngill

Copy link
Copy Markdown
Contributor Author

🤖 Automated Branch Sync (claude-code) — 2026-08-14T00:58:01Z

Action: No rebase needed — feat/api-285-mcp-install is already up to date with main (merge-base 01b887bd == main tip). 0 commits behind, 1 ahead.
Status: mergeable ✅ (MERGEABLE, no conflicts)

mergeStateStatus: BLOCKED is due to REVIEW_REQUIRED — all CI checks pass (lint, test 20/22/24, Aikido, pr-reviewer). Needs a human approval, not a sync.

@gulshngill

Copy link
Copy Markdown
Contributor Author

🤖 Automated Branch Sync (claude-code) — 2026-08-14T02:56:49Z

Action: No rebase needed — feat/api-285-mcp-install (e2bc3b3) already contains all of main (01b887b); 0 commits behind, 1 ahead.
Status: mergeable ✅ (MERGEABLE, no conflicts)

Merge is gated only by REVIEW_REQUIRED — all checks green (lint, test 20/22/24, Aikido, pr-reviewer). Needs a human approval.

@gulshngill

Copy link
Copy Markdown
Contributor Author

🤖 Automated Branch Sync (claude-code) — 2026-08-14T04:57:08Z

Action: No rebase needed — feat/api-285-mcp-install (e2bc3b3) is already up to date with main (01b887b); 0 commits behind.
Status: mergeable ✅ (mergeStateStatus: BLOCKED — awaiting review approval, not conflicts; all CI checks green)

@gulshngill

Copy link
Copy Markdown
Contributor Author

🤖 Automated Branch Sync (claude-code) — 2026-08-14T14:57:24Z

Action: No rebase needed — feat/api-285-mcp-install is already up to date with main (merge-base = 01b887b, 0 commits behind).
Status: mergeable ✅ — all checks green (lint, test 20/22/24, Aikido, pr-reviewer). mergeStateStatus: BLOCKED is solely REVIEW_REQUIRED (awaiting human approval), not a merge conflict.

@gulshngill

Copy link
Copy Markdown
Contributor Author

🤖 Automated Branch Sync (claude-code) — 2026-08-17T05:50:35Z

Action: No rebase needed — feat/api-285-mcp-install already contains all of main (01b887b); 1 commit ahead, 0 behind.
Status: mergeable ✅ (no conflicts, all CI checks green)

Merge is still gated on REVIEW_REQUIRED — human approval pending. No merge/close performed.

@gulshngill

Copy link
Copy Markdown
Contributor Author

🤖 Automated PR Review (codex) — 2026-08-17T06:04:44Z

Reviewed: MCP install/uninstall behavior, client config formats, merge/backup/atomic-write paths, error handling, key exposure, CLI/schema/docs integration, and test coverage
Fixed: distinguish config read errors from JSON parse errors; reject non-object config roots; remove secret-bearing temp files after failed atomic renames
Tests: passed — 54 files, 1,988 passed / 2 skipped; ESLint clean; production dependency audit found 0 vulnerabilities

@gulshngill

Copy link
Copy Markdown
Contributor Author

🤖 Automated Branch Sync (claude-code) — 2026-08-17T08:57:58Z

Action: No rebase needed — feat/api-285-mcp-install (b914d97) is already up to date with main (01b887b): 0 commits behind, 2 ahead. No force-push performed.
Status: mergeable ✅ (MERGEABLE, no conflicts)

ℹ️ Merge state is BLOCKED solely because REVIEW_REQUIRED — all CI checks (lint, test 20/22/24, Aikido, pr-reviewer) passed. Needs a human approval, not a sync.

@gulshngill

Copy link
Copy Markdown
Contributor Author

🤖 Automated PR Review (codex) — 2026-08-17T09:02:56Z

Reviewed: MCP client config formats, install/uninstall behavior, atomic writes, backups, error handling, API-key exposure, CLI/schema/docs integration, and test coverage
Fixed: uninstall --dry-run now previews without changing config; Claude Desktop now passes the API key through mcp-remote environment expansion instead of exposing it in process arguments
Tests: passed — 54 files, 1,989 passed / 2 skipped; ESLint clean; production dependency audit found 0 vulnerabilities

@gulshngill

Copy link
Copy Markdown
Contributor Author

🤖 Automated Branch Sync (claude-code) — 2026-08-17T12:59:06Z

Action: Checked feat/api-285-mcp-install against main — already up to date (0 commits behind, 3 ahead). No rebase or force-push needed.
Status: mergeable ✅ (mergeable: MERGEABLE, no conflicts)

Merge is currently gated on REVIEW_REQUIRED — all CI checks pass (lint, test 20/22/24, Aikido, pr-reviewer).

Addresses both pr-reviewer findings on #487:

- `mcp uninstall` now copies the config to `.bak` (best-effort chmod 0600)
  before writing, mirroring `install`. Only runs when there is an entry to
  remove and not under `--dry-run`, so a missing config or a no-op uninstall
  still writes nothing. copyFileSync failures surface as-is, like install.
  Adds a notice that the backup retains the API key just removed.
- `runCLI` passes `log: deps.log ?? output` to `buildMcpCommands`, so mcp
  output honours the caller's stdout sink instead of falling back to bare
  console.log. An explicit `log` dep still wins, preserving test injection.

Regression tests: uninstall backup content/mode/log, no backup when there is
nothing to remove (incl. dry-run), failed backup copy leaves the config
untouched, and runCLI routing mcp output through `output` with console.log
unused. README + changeset wording updated to cover uninstall.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant