Skip to content

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

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

API-285: One-step MCP install (nansen mcp install <client>)#487
gulshngill wants to merge 3 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 #2bdce77

📝 2 findings

Review completed. Please address the findings below.

Findings by Severity

Severity Count
🟡 Medium 2

Review effort: 4/5 (Complex)

Summary

This is a well-engineered new command. The security decisions are sound and explicitly documented: merge-only writes, atomic rename, backup before write, key never printed, closed client-name validation, hardcoded HTTPS server URL, and mcp-remote pinned exact. Test coverage is thorough (27 tests, real temp dirs, symlink path, corrupt-JSON refusal, dry-run, file/dir modes). Two medium-severity issues are noted below.

Findings

src/commands/mcp.js — Medium: uninstall writes no backup before modifying the config

Severity: medium

Description: The install path copies the existing config to .bak before calling writeConfig. The uninstall path calls writeConfig directly without a backup. While writeConfig is atomic (temp-file + rename, so no truncation on crash), a user who accidentally uninstalls the wrong entry has no .bak to recover from. Given that install explicitly documents and advertises the backup behaviour, the asymmetry may surprise users.

Suggested fix: Add a copyFileSync + chmodSync(backupPath, 0o600) call in the uninstall branch, mirroring the install branch, before calling writeConfig. The relevant location is src/commands/mcp.js lines 207–208:

// before writeConfig(configPath, updated):
const backupPath = `${configPath}.bak`;
fsx.copyFileSync(configPath, backupPath);
try { fsx.chmodSync(backupPath, 0o600); } catch { /* best-effort */ }
log(`Backed up existing config to ${backupPath}`);

src/commands/mcp.js — Medium: buildMcpCommands log dep is not wired to output in the production runCLI path

Severity: medium

Description: buildMcpCommands extracts log from its deps argument with a fallback to console.log. However, runCLI (src/cli.js:1871) passes the raw deps object it received from the caller — and that object contains output, not log. So in production (node src/index.js mcp install …), the mcp command silently falls back to bare console.log, bypassing any output override. Every other command builder (buildAlertsCommands, buildAgentCommands) follows exactly the same pattern, so this is not a new regression, but the mcp command is the first operational command added since that pattern was established — and the tests pass log directly so the gap is invisible there.

Suggested fix: Either (a) pass log: deps.log || deps.output || console.log when calling buildMcpCommands in runCLI, or (b) in buildMcpCommands destructure const { log = console.log, output } = deps; const emit = log ?? output ?? console.log; to honour whichever key the caller provides. Option (a) keeps the fix local to cli.js and doesn't affect tests. Alternatively, open a follow-up issue to normalise the log/output naming across all build*Commands helpers.


Token usage: 3,996 input, 6,133 output, 874,615 cache read, 45,089 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

Comment thread src/commands/mcp.js
const writeConfig = (configPath, config) => {
const dir = path.dirname(configPath);
if (!fsx.existsSync(dir)) fsx.mkdirSync(dir, { recursive: true, mode: 0o700 });
const tmp = path.join(dir, `.${path.basename(configPath)}.tmp-${process.pid}`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential file inclusion attack via reading file - medium severity
If an attacker can control the input leading into the ReadFile function, they might be able to read sensitive files and launch further attacks with that information.

Show fix

Remediation: Ignore this issue only after you've verified or sanitized the input going into this function. This issue is only relevant in the backend, not in the frontend!

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

@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).

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