Skip to content

Harden environment isolation and error handling#2

Open
pandysp wants to merge 2 commits intoandroidStern-personal:mainfrom
pandysp:security/harden-env-and-error-handling
Open

Harden environment isolation and error handling#2
pandysp wants to merge 2 commits intoandroidStern-personal:mainfrom
pandysp:security/harden-env-and-error-handling

Conversation

@pandysp
Copy link
Copy Markdown

@pandysp pandysp commented Feb 9, 2026

Summary

Security hardening PR addressing environment leakage, silent failures, and reconnection bugs:

  • Stop leaking process.env to MCP subprocesses — the StdioClientTransport was receiving the gateway's entire environment (Claude setup tokens, Tailscale keys, etc.). The MCP SDK already merges a safe base env (HOME, PATH, SHELL, USER, TERM, LOGNAME) via getDefaultEnvironment() before user-provided env, so we just stop passing process.env.
  • Warn on undefined ${VAR} interpolation — typos like ${MY_API_KYE} now log a warning instead of silently sending empty auth headers.
  • Log transport errors and reconnectsonerror, onclose, and reconnect() were silent; failures are now observable in logs.
  • Expand reconnectable errors — add ETIMEDOUT, ECONNRESET, ENOTFOUND, EHOSTUNREACH to the set that triggers automatic reconnection.
  • Fix stale client after failed reconnect — delete the map entry before reconnecting and guard against undefined after, preventing a TypeError crash on the non-null assertion.

Breaking change

MCP servers that silently relied on inherited environment variables (e.g., NODE_OPTIONS, ANTHROPIC_API_KEY, custom LD_LIBRARY_PATH) will need those vars added explicitly to their env config block. This is intentional — the gateway's full environment should never leak into subprocess sandboxes.

What's NOT in this PR

Filed separately as issues to keep this PR focused:

  • SSRF URL validation for HTTP transports
  • Tool lifecycle (unregister on disconnect)
  • Input validation against tool JSON schemas

Test plan

  • npm install succeeds
  • TypeScript compiles without new errors (npx tsc --noEmit — the pre-existing index.ts:37 .map type error is unchanged)
  • Diff review: no functional changes beyond the 5 security fixes
  • Confirmed MCP SDK v1.26.0 has the internal getDefaultEnvironment() merge

- Stop spreading process.env into MCP subprocesses; the SDK's
  StdioClientTransport already merges a safe base environment
  (HOME, PATH, SHELL, etc.) before user-provided env vars.
  **Breaking:** servers that relied on inherited env vars must now
  declare them explicitly in their env config block.
- Warn when ${VAR} interpolation references an undefined env var
  instead of silently substituting an empty string.
- Log transport errors, disconnects, and reconnect attempts so
  failures are observable instead of swallowed silently.
- Expand reconnectable error set with ETIMEDOUT, ECONNRESET,
  ENOTFOUND, and EHOSTUNREACH.
- Delete stale client map entry before reconnect and guard against
  missing entry after reconnect to prevent TypeError on failed
  reconnection.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Block HTTP transport URLs pointing to private/reserved addresses
  (127.0.0.0/8, 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16,
  169.254.0.0/16, localhost, .local, IPv6 loopback/ULA).
  Opt out per-server with allowPrivateUrls: true.
- Validate tool arguments against inputSchema before forwarding to
  MCP servers: warn on missing required fields and type mismatches.
  Warn-only (does not block calls) to avoid breaking tools with
  loose schemas.

Co-Authored-By: Claude Opus 4.6 <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