inspector icon indicating copy to clipboard operation
inspector copied to clipboard

npx @modelcontextprotocol/inspector not using current PATH

Open bolinfest opened this issue 8 months ago • 1 comments

Describe the bug npx @modelcontextprotocol/inspector is not using PATH from current environment.

To Reproduce When I run:

export PATH=/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin
npx @modelcontextprotocol/inspector /bin/true

I see:

Query parameters: [Object: null prototype] {
  command: 'cargo',
  args: 'run',
  env: '{"HOME":"/Users/mbolin","LOGNAME":"mbolin","PATH":"/Users/mbolin/.npm/_npx...

i.e., there is extra stuff. This is a problem because I can't make a screen recording without leaking sensitive info.

Expected behavior I should see the value that corresponds to export PATH

Logs sorry, nope

Additional context Not sure if npx is to blame here?

bolinfest avatar May 05 '25 00:05 bolinfest

Hi, was looking into this and my understanding is that there are a couple different things going on:

  1. The default env variables decided by the sdk are overriding the process env variables
  2. Either way, its not a good idea to log the env variables by default

olaservo avatar May 19 '25 15:05 olaservo

@claude please review this issue and its comments and related open PRs, and help make a recommendation on how and what to resolve vs keep as-is.

olaservo avatar Aug 18 '25 13:08 olaservo

Claude finished @olaservo's task —— View job


Analysis Complete: Issue #375 PATH Environment Variable Handling

Todo List:

  • [x] Review codebase for environment variable handling
  • [x] Check for related open PRs
  • [x] Analyze security/privacy implications of logging env variables
  • [x] Provide recommendations on what to resolve vs keep as-is

Root Cause Analysis

I've identified the core issues causing the reported problems:

  1. Default Environment Override: In server/src/index.ts:32-35, the server creates a defaultEnvironment that merges MCP SDK defaults with process.env.MCP_ENV_VARS. This gets merged with user environment variables, potentially overriding user-specified PATH values.

  2. Sensitive Data Logging: In server/src/index.ts:179, the server logs ALL query parameters including the complete environment object:

    console.log("Query parameters:", JSON.stringify(query));
    
  3. npx PATH Pollution: When using npx @modelcontextprotocol/inspector, npx modifies PATH before execution, so the logged PATH includes npx's additions rather than the user's clean PATH setting.

Security & Privacy Issues 🚨

Critical Security Risk: The current logging implementation exposes:

  • Complete environment variables (including potentially sensitive tokens/secrets)
  • User's full PATH containing personal directory structures
  • Any credentials or API keys that might be in the environment

This is a significant privacy and security concern that should be addressed immediately.

Recommendations by Priority

🔥 Immediate (Security Critical)
  1. Remove/sanitize environment logging: The blanket JSON.stringify(query) logging at server/src/index.ts:179 must be removed or modified to exclude environment data
  2. Implement filtered logging: If debugging info is needed, log only safe, non-sensitive parameters
⚡ High Priority (Functionality)
  1. Fix environment precedence: User-specified env vars should override defaults, not vice versa (fix precedence in server/src/index.ts:187)
  2. Add clean environment option: Provide flag to use only user-specified variables without SDK defaults
📋 Medium Priority (Improvements)
  1. Environment variable validation: Whitelist known safe environment variables
  2. Structured logging: Replace console.log with proper logging framework with configurable levels

Impact Assessment

  • Security: HIGH - Sensitive data exposure in logs
  • Privacy: HIGH - Personal paths and potentially secrets logged
  • Functionality: MEDIUM - PATH not behaving as user expects
  • User Experience: MEDIUM - Cannot make clean screen recordings due to sensitive info in logs

Recommendation: RESOLVE

This issue should definitely be resolved rather than kept as-is because:

  1. Security risk of logging sensitive environment data
  2. Privacy concerns for users making recordings/documentation
  3. Functional issue where user's PATH exports aren't respected
  4. Low implementation complexity - the fixes are straightforward

The core problems are both fixable and represent genuine issues rather than acceptable design trade-offs.


claude[bot] avatar Aug 18 '25 13:08 claude[bot]

The path order issue has been addressed in this PR: https://github.com/modelcontextprotocol/inspector/pull/421

Keeping this open to address the env logging concerns for sensitive environment settings.

olaservo avatar Aug 18 '25 14:08 olaservo

Closing since the PATH issue was resolved, created a follow-up (linked above) to revisit what should be logged as a separate issue.

olaservo avatar Oct 08 '25 13:10 olaservo