API-320: Standardize MCP client credential handling (opt-in --env-ref, no key echoes) - #507
Open
gulshngill wants to merge 2 commits into
Open
Conversation
…tall mode (API-320)
Define the safest practical credential path for each supported MCP client
and stop leaking keys through generated configs, backups, failure
messages, and shell history:
- New opt-in `nansen mcp install <client> --env-ref` writes an
environment-variable reference instead of the key literal
(claude-code: ${NANSEN_API_KEY} header, cursor: ${env:NANSEN_API_KEY}
header, claude-desktop: env block omitted so the pinned mcp-remote
bridge inherits NANSEN_API_KEY from the OS environment). No login
needed — the CLI never touches the key in this mode.
- Default stays the inline literal (some client versions fail to expand
references in HTTP headers and would send them literally); the
plaintext note now points at --env-ref. Per-client patterns and
ceilings documented in README and `nansen mcp` usage.
- verify resolves exact reference forms from the current shell and fails
actionably when the variable is unset; near-miss references (any
${...} value that isn't the exact supported form) are refused, since
the client would send a credential verify never tested.
- Replacing an existing nansen entry now redacts its credential slots in
the .bak backup — an --env-ref migration no longer strands the old key
on disk. Other entries are preserved verbatim.
- login help leads with history-safe methods (--human, secret-manager
command substitution) and flags that literal --api-key values and
inline env assignments are recorded in shell history; the login
verification failure message redacts the key.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…API-320 review) Three minor findings from the independent review of #507: - redactNansenEntryCredential now also redacts the inline `--header NANSEN-API-KEY:<key>` argv form a hand-written desktop entry can carry, and returns whether it actually redacted anything. A `${...}` reference is not a secret, so it is preserved instead of blanked. - install only logs "Nansen credential redacted" when a credential was redacted — reinstalling over an env-ref entry no longer claims it. - verify names an unsupported credential reference instead of reporting a URL/transport mismatch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #502 (which stacks on #487) — review only the top commit.
What
Defines the safest practical credential path per supported MCP client and closes the remaining key-leak paths in generated configs, backups, failure messages, and shell history.
Opt-in
nansen mcp install <client> --env-refWrites an environment-variable reference instead of the key literal, so the generated config never holds the key. No login required — the CLI never reads the key in this mode.
--env-refheaders"NANSEN-API-KEY": "${NANSEN_API_KEY}"headers"NANSEN-API-KEY": "${env:NANSEN_API_KEY}"envblock (argv already env-referenced)envblock omitted; the pinned mcp-remote bridge inheritsNANSEN_API_KEYfrom the OS environmentIndirection is opt-in, not the default, because both claude-code and cursor have documented-but-buggy header expansion: a config the client fails to expand silently sends the literal
${...}string as the API key. The default inline install now points at--env-refin its plaintext note; README documents each pattern and its ceiling.verify understands references
nansen mcp verify <client>resolves the exact reference forms from the current shell and fails actionably (without echoing anything) whenNANSEN_API_KEYis unset. Near-miss references — any${...}value that isn't the exact supported form — are refused, because the client could expand them to a credential verify never tested. Scope is documented honestly: verify proves config shape + current-shell resolution, not that the client itself expands the reference.No more key copies in backups
Replacing an existing nansen entry used to byte-copy the old config (key included) into
<config>.bak, so an--env-refmigration stranded the old key on disk. The backup now redacts the nansen entry's credential slots (headers in any casing, desktopenvblock) while preserving every other entry verbatim; the nansen entry is regenerable via re-install.Login hardening
nansen loginhelp now leads with history-safe methods (--human, secret-manager command substitution) and states plainly that literal--api-keyvalues and inlineNANSEN_API_KEY=<key>assignments are recorded in shell history. The login verification failure message redacts the key before relaying upstream error text.Tests
npm test: 2016 passed, 2 skipped (54 files);npm run lintclean.--env-refinstall in both flag positions; verify resolving from env and rejecting unset refs without a network call; backup redaction on migration; login help ordering and failure-message redaction; no-echo assertions throughout.--env-refinstall logged-out → config holds only the reference;verifywith the variable unset → actionable error; with it set → authenticated data call succeeds.🤖 Generated with Claude Code