diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..79f6bbf --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,37 @@ +# Project overview +The ./README.md provides an overview of this project. + +@README.md + +## Potentially out-dated README.md contents + +** IMPORTANT ** +We are actively implementing new features in this project and have not updated all the docs yet. +Therefore the contents of the README.md as provided above are just for reference to provide +a good foundational understanding of the conceptual functionality of this project (or as the +project was a day or two ago). + +** Double-Check ** +For any work you may do, you must NOT consider the README.md contents as fully accurate, +up-to-date, or authoratative. + +Double-check and verify actual state before doing any work. + +You can also look at recent git commits. + +And also use claude-mem tool to get an excellent understanding of recent work, developments & plans. + +# Build Path + +`cd /path/to/mcp-debug` <---- replace this with actual path to the project. +(if already in the project root directory, the above command is unnecessary) + +go build -o ./bin/mcp-debug . + +## Build to ./bin/ + +Do not build/publish to ./ + +Only build/publish to ./bin/ + +(our MCP client is pointed at `./bin/mcp-debug`, so that is where you must create the build). diff --git a/DRAFT_ENV_INHERITANCE.md b/DRAFT_ENV_INHERITANCE.md new file mode 100644 index 0000000..7dc3230 --- /dev/null +++ b/DRAFT_ENV_INHERITANCE.md @@ -0,0 +1,1263 @@ +# Environment Variable Inheritance (DRAFT) + +> **⚠️ DRAFT DOCUMENTATION**: This feature is fully implemented and tested but has not yet been validated with real-world MCP servers. Documentation may be updated based on real-world testing feedback. + +**Status**: Feature implemented in commit 49f5581 +**Branch**: feature/env-selective-inheritance +**Last Updated**: 2026-01-13 + +--- + +## Overview + +mcp-debug implements a **tier-based environment variable inheritance system** that provides fine-grained control over which environment variables are passed from the parent process to MCP server child processes. + +This feature addresses a critical security concern: preventing accidental leakage of sensitive environment variables (credentials, tokens, SSH agent sockets, API keys) to upstream MCP servers, especially experimental or third-party servers that may not have been thoroughly vetted. + +### Why This Matters + +When running MCP servers as child processes, the default behavior in most systems is to inherit all environment variables from the parent process. This can inadvertently expose: + +- **Cloud provider credentials** (AWS_ACCESS_KEY_ID, AZURE_CLIENT_SECRET, etc.) +- **Authentication tokens** (GITHUB_TOKEN, ANTHROPIC_API_KEY, etc.) +- **SSH agent sockets** (SSH_AUTH_SOCK) +- **Development secrets** (.env file variables) +- **Corporate credentials** loaded into your shell + +With selective inheritance, you explicitly control what gets shared, following the principle of least privilege. + +--- + +## Security Rationale + +### The Problem + +The traditional "inherit everything" approach using `os.Environ()` is convenient but dangerous: + +```yaml +servers: + - name: experimental-server + command: python3 + args: ["-m", "untrusted_mcp_server"] + # This server now has access to ALL your environment variables! +``` + +### The Solution + +Tier-based inheritance with explicit control: + +```yaml +servers: + - name: experimental-server + command: python3 + args: ["-m", "untrusted_mcp_server"] + inherit: + mode: tier1 # Only baseline variables (default) + extra: ["PYTHONPATH"] # Explicitly add what's needed + deny: ["SSH_AUTH_SOCK"] # Explicitly block sensitive vars +``` + +### Security Benefits + +1. **Default-secure**: By default, only Tier 1 baseline variables are inherited +2. **Tier 1 baseline always available**: Prevents broken environments (PATH, HOME always available unless explicitly denied) +3. **Explicit opt-in for additional vars**: Variables beyond Tier 1 must be explicitly requested via `extra` or `prefix` +4. **Auditable**: Configuration files show exactly what each server receives +5. **Defense in depth**: Multiple layers (tiers, deny lists, implicit blocks) +6. **httpoxy mitigation**: HTTP_PROXY (uppercase) blocked by default to prevent httpoxy attacks +7. **No mode to inherit everything**: Even `mode: all` only inherits Tier 1 + Tier 2, not all parent variables + +--- + +## Tier System + +The inheritance system is organized into two tiers plus an implicit denylist. + +### Tier 1: Baseline Variables (Always Inherited) + +These are minimal essential variables that most programs need to function correctly. **Always inherited unless explicitly denied.** + +| Variable | Purpose | +|----------|---------| +| `PATH` | Executable search path | +| `HOME` | User home directory | +| `USER` | Current username | +| `SHELL` | User's shell | +| `LANG` | Primary locale setting | +| `LC_ALL` | Locale override | +| `TZ` | Timezone | +| `TMPDIR` | Temporary directory (Unix) | +| `TEMP` | Temporary directory (Windows) | +| `TMP` | Temporary directory (Windows) | + +**Rationale**: These variables are required for basic process functionality and rarely contain secrets. Excluding them would break most servers. By making Tier 1 the guaranteed baseline, we prevent accidentally creating broken environments while still maintaining security. + +**Blocking Tier 1 variables**: If you need to block a Tier 1 variable (e.g., for maximum isolation testing), use the `deny:` list. + +### Tier 2: Network and TLS Variables + +These are helpful for servers that make network connections or need TLS certificate validation. Inherited when `mode: tier1+tier2` or `mode: all` is set. + +| Variable | Purpose | +|----------|---------| +| `SSL_CERT_FILE` | Path to TLS certificate bundle | +| `SSL_CERT_DIR` | Directory containing TLS certificates | +| `REQUESTS_CA_BUNDLE` | Python requests library CA bundle | +| `CURL_CA_BUNDLE` | curl CA bundle path | +| `NODE_EXTRA_CA_CERTS` | Node.js additional CA certificates | + +**Rationale**: Enterprise environments often require custom CA bundles for TLS inspection/interception. These variables enable servers to validate certificates in corporate networks. + +**Note**: Proxy variables (http_proxy, https_proxy) are in the **implicit denylist** by default due to security concerns (see below). + +### Implicit Denylist + +These variables are **blocked by default** and require explicit opt-in via `extra` + `allow_denied_if_explicit: true`. + +| Variable | Reason | +|----------|--------| +| `HTTP_PROXY` | httpoxy vulnerability (uppercase variant) | +| `HTTPS_PROXY` | httpoxy vulnerability (uppercase variant) | +| `http_proxy` | Potential security risk | +| `https_proxy` | Potential security risk | +| `NO_PROXY` | Can leak internal network topology | +| `no_proxy` | Can leak internal network topology | + +**httpoxy vulnerability**: The uppercase `HTTP_PROXY` variable can be set by attackers via HTTP headers in CGI-like environments, causing the application to proxy requests through attacker-controlled servers. See [httpoxy.org](https://httpoxy.org/) for details. + +**Overriding the denylist**: If you genuinely need proxy variables, you must explicitly request them: + +```yaml +inherit: + mode: tier1+tier2 + extra: ["http_proxy", "https_proxy"] + allow_denied_if_explicit: true +``` + +--- + +## Inheritance Modes + +The `mode` setting controls the base set of variables to inherit. **All modes inherit at least Tier 1 baseline variables** (unless explicitly denied). + +### `mode: none` or `mode: tier1` (DEFAULT) + +**Inherit Tier 1 baseline variables only**, plus any variables from `extra` and `prefix`. + +```yaml +servers: + - name: python-server + command: python3 + args: ["-m", "my_server"] + inherit: + mode: tier1 # Can be omitted (it's the default) + extra: ["PYTHONPATH", "VIRTUAL_ENV"] +``` + +**Inherited**: PATH, HOME, USER, SHELL, LANG, LC_ALL, TZ, TMPDIR, TEMP, TMP + +**Use cases**: +- Default for most servers +- Good balance of functionality and security +- Prevents most secret leakage + +**Note**: `mode: none` and `mode: tier1` behave identically - both inherit Tier 1 baseline. The "none" naming is kept for backward compatibility but is somewhat misleading. To achieve true isolation (no automatic inheritance), use `mode: tier1` with an explicit `deny:` list containing all Tier 1 variables. + +### `mode: tier1+tier2` + +**Inherit Tier 1 + Tier 2 variables**, plus any from `extra` and `prefix`. + +```yaml +servers: + - name: api-server + command: node + args: ["server.js"] + inherit: + mode: tier1+tier2 + extra: ["NODE_ENV"] +``` + +**Inherited**: All Tier 1 variables + SSL_CERT_FILE, SSL_CERT_DIR, REQUESTS_CA_BUNDLE, CURL_CA_BUNDLE, NODE_EXTRA_CA_CERTS + +**Use cases**: +- Servers making HTTPS requests +- Enterprise environments with custom CA bundles +- Servers needing TLS certificate validation + +### `mode: all` + +**Same as `mode: tier1+tier2`**. Inherits Tier 1 + Tier 2 variables, plus any from `extra` and `prefix`. + +```yaml +servers: + - name: trusted-server + command: ./my-trusted-app + inherit: + mode: all + extra: ["MY_APP_CONFIG", "DATABASE_URL"] +``` + +**Inherited**: Tier 1 + Tier 2 variables, plus anything in `extra` or matching `prefix` patterns + +**Use cases**: +- Servers needing network/TLS capabilities +- When you want a recognizable name that implies "maximum compatibility" +- Backward compatibility with configurations expecting "all" to mean "Tier 1 + Tier 2" + +**⚠️ Important**: Despite the name, `mode: all` does **NOT** inherit all parent environment variables. It's equivalent to `tier1+tier2`. This is a security-first design decision to prevent accidental secret leakage. To inherit additional variables, you must explicitly list them in `extra:` or `prefix:`. + +--- + +## Configuration Options + +### Complete Schema + +```yaml +# Proxy-level defaults (optional) +proxy: + healthCheckInterval: "30s" + connectionTimeout: "10s" + +inherit: # Applied to all servers unless overridden + mode: "tier1" # none | tier1 | tier1+tier2 | all + extra: [] # Additional variable names + prefix: [] # Variable name prefixes to match + deny: [] # Variables to block + allow_denied_if_explicit: false # Allow denied vars if in 'extra' + +servers: + - name: my-server + transport: stdio + command: python3 + args: ["-m", "my_mcp_server"] + + # Server-specific inheritance (overrides proxy defaults) + inherit: + mode: "tier1+tier2" + extra: ["PYTHONPATH", "VIRTUAL_ENV"] + prefix: ["MY_APP_", "DATTO_"] + deny: ["SSH_AUTH_SOCK"] + allow_denied_if_explicit: true + + # Explicit overrides (always applied, never denied) + env: + MY_CONFIG: "production" + API_KEY: "${MY_API_KEY}" # Expanded from parent env +``` + +### Field Reference + +#### `mode` (string) + +Controls base inheritance behavior. + +- **Type**: String enum +- **Values**: `none`, `tier1`, `tier1+tier2`, `all` +- **Default**: `tier1` (if not specified) +- **Example**: `mode: "tier1+tier2"` +- **Note**: `none` and `tier1` are equivalent; `all` is equivalent to `tier1+tier2` + +#### `extra` (array of strings) + +Additional variable names to inherit beyond the tier definition. + +- **Type**: Array of strings +- **Default**: Empty array +- **Case-sensitive**: Variable names are matched case-sensitively on Unix, case-insensitively on Windows +- **Example**: `extra: ["PYTHONPATH", "VIRTUAL_ENV", "NODE_ENV"]` + +Variables listed in `extra` can bypass the implicit denylist if `allow_denied_if_explicit: true` is set. + +#### `prefix` (array of strings) + +Inherit all variables whose names start with these prefixes. + +- **Type**: Array of strings +- **Default**: Empty array +- **Case-sensitive**: Prefix matching follows platform conventions +- **Example**: `prefix: ["MY_APP_", "DATTO_", "CUSTOM_"]` + +Useful for inheriting groups of related variables (e.g., all configuration for a specific application). + +#### `deny` (array of strings) + +Variables to explicitly block from inheritance, including Tier 1 baseline variables. + +- **Type**: Array of strings +- **Default**: Empty array +- **Combines with implicit denylist**: Both are applied +- **Example**: `deny: ["SSH_AUTH_SOCK", "AWS_SECRET_ACCESS_KEY", "PATH"]` + +Use this to block sensitive variables or to achieve maximum isolation by denying Tier 1 variables. + +#### `allow_denied_if_explicit` (boolean) + +Allow variables from the implicit denylist (and explicit `deny` list) if they're in `extra`. + +- **Type**: Boolean +- **Default**: `false` +- **Example**: `allow_denied_if_explicit: true` + +When `false`: Denied variables are always blocked, even if in `extra`. +When `true`: Variables in `extra` bypass both implicit and explicit deny lists. + +**Security note**: Only enable this if you understand the risks (e.g., httpoxy). + +--- + +## Configuration Examples + +### Example 1: Basic Python Server (Default - Most Secure) + +**Scenario**: Python MCP server needs Python-specific variables. + +```yaml +servers: + - name: python-mcp + transport: stdio + command: python3 + args: ["-m", "my_python_server"] + inherit: + mode: tier1 # Optional - this is the default + extra: ["PYTHONPATH", "VIRTUAL_ENV", "PYTHONHOME"] +``` + +**Inherited**: +- Tier 1: PATH, HOME, USER, SHELL, LANG, LC_ALL, TZ, TMPDIR +- Extra: PYTHONPATH, VIRTUAL_ENV, PYTHONHOME + +### Example 2: Node.js Server with TLS + +**Scenario**: Node.js server making HTTPS API calls in corporate environment. + +```yaml +servers: + - name: node-api-server + transport: stdio + command: node + args: ["server.js"] + inherit: + mode: tier1+tier2 + extra: ["NODE_ENV", "NODE_OPTIONS"] +``` + +**Inherited**: +- Tier 1: PATH, HOME, USER, SHELL, LANG, LC_ALL, TZ, TMPDIR +- Tier 2: SSL_CERT_FILE, SSL_CERT_DIR, REQUESTS_CA_BUNDLE, CURL_CA_BUNDLE, NODE_EXTRA_CA_CERTS +- Extra: NODE_ENV, NODE_OPTIONS + +### Example 3: Application-Specific Variables + +**Scenario**: Server needs all variables prefixed with `DATTO_` and `RMM_`. + +```yaml +servers: + - name: datto-rmm + transport: stdio + command: python3 + args: ["-m", "datto_rmm.server"] + inherit: + mode: tier1 + prefix: ["DATTO_", "RMM_"] + extra: ["PYTHONPATH"] +``` + +**Inherited**: +- Tier 1: PATH, HOME, USER, SHELL, etc. +- All variables starting with `DATTO_` (e.g., DATTO_API_KEY, DATTO_URL) +- All variables starting with `RMM_` +- PYTHONPATH + +### Example 4: Maximum Security (Untrusted Server) + +**Scenario**: Untrusted experimental server, minimal exposure. + +```yaml +servers: + - name: experimental + transport: stdio + command: python3 + args: ["-m", "untrusted_server"] + inherit: + mode: tier1 # Only baseline variables + deny: ["SSH_AUTH_SOCK"] # Extra paranoia (though not in Tier 1 anyway) +``` + +**Inherited**: Only Tier 1 baseline variables (PATH, HOME, USER, SHELL, LANG, LC_ALL, TZ, TMPDIR, TEMP, TMP) + +### Example 5: Maximum Isolation (Testing) + +**Scenario**: Complete isolation for testing, no automatic inheritance. + +```yaml +servers: + - name: isolated-test + transport: stdio + command: python3 + args: ["-m", "test_server"] + inherit: + mode: tier1 + deny: ["PATH", "HOME", "USER", "SHELL", "LANG", "LC_ALL", "TZ", "TMPDIR", "TEMP", "TMP"] + env: + # Only these exact variables will be set + PYTHONPATH: "/opt/myapp" + MY_CONFIG: "test" +``` + +**Inherited**: None (all Tier 1 variables explicitly denied) +**Set**: Only PYTHONPATH and MY_CONFIG from `env:` block + +**Note**: This will likely break most servers. Only use for controlled testing. + +### Example 6: Proxy with Corporate Proxy Variables + +**Scenario**: Enterprise environment requiring lowercase proxy variables. + +```yaml +servers: + - name: enterprise-server + transport: stdio + command: node + args: ["server.js"] + inherit: + mode: tier1+tier2 + extra: ["http_proxy", "https_proxy", "no_proxy"] + allow_denied_if_explicit: true # Override implicit denylist +``` + +**Inherited**: +- Tier 1 + Tier 2 variables +- http_proxy, https_proxy, no_proxy (despite being in implicit denylist) + +**⚠️ Security Note**: Only use this configuration if you control the server code and understand the httpoxy risk. + +### Example 7: "All" Mode (Tier 1 + Tier 2 + Extras) + +**Scenario**: Server needing network capabilities plus custom variables. + +```yaml +servers: + - name: feature-rich-server + transport: stdio + command: ./my-server + inherit: + mode: all # Same as tier1+tier2 + extra: ["DATABASE_URL", "REDIS_URL", "API_TOKEN"] + deny: ["AWS_SECRET_ACCESS_KEY", "GITHUB_TOKEN"] +``` + +**Inherited**: +- Tier 1 + Tier 2 variables +- DATABASE_URL, REDIS_URL, API_TOKEN + +**Blocked**: +- AWS_SECRET_ACCESS_KEY, GITHUB_TOKEN (explicitly denied) +- All other parent environment variables not in Tier 1, Tier 2, or `extra` list + +### Example 8: Proxy-Level Defaults + +**Scenario**: Set defaults for all servers, override for specific ones. + +```yaml +# Proxy-level defaults +proxy: + healthCheckInterval: "30s" + +inherit: + mode: tier1 + extra: ["LANG", "LC_ALL"] + +servers: + - name: basic-server + transport: stdio + command: python3 + args: ["-m", "basic_server"] + # Inherits proxy defaults (tier1 + LANG + LC_ALL) + + - name: special-server + transport: stdio + command: python3 + args: ["-m", "special_server"] + inherit: + mode: tier1+tier2 # Override: needs TLS + extra: ["PYTHONPATH", "API_KEY"] +``` + +--- + +## Default Behavior + +If you don't specify any inheritance configuration, the system uses secure defaults. + +### When No `inherit` Block Exists + +```yaml +servers: + - name: my-server + transport: stdio + command: python3 + args: ["-m", "server"] + # No inherit block specified +``` + +**Behavior**: Defaults to `mode: tier1` with no extras, prefixes, or deny rules. + +**Inherited**: PATH, HOME, USER, SHELL, LANG, LC_ALL, TZ, TMPDIR, TEMP, TMP (Tier 1 baseline) + +### Explicit Overrides Always Win + +Even with `mode: tier1` and `deny` lists, explicit overrides in the `env:` block are always applied: + +```yaml +servers: + - name: my-server + transport: stdio + command: python3 + args: ["-m", "server"] + inherit: + mode: tier1 + deny: ["PATH"] # Deny PATH from inheritance + env: + PATH: "/custom/path" # Always set (explicit override) + CUSTOM_VAR: "value" # Always set + API_KEY: "${PARENT_API_KEY}" # Expanded from parent +``` + +**Result**: PATH is set to "/custom/path" (not inherited from parent), CUSTOM_VAR is set, API_KEY is expanded from parent environment. + +--- + +## Resolution Order + +Understanding precedence is crucial for debugging configuration issues. + +### Inheritance Resolution (Processing Order) + +1. **Implicit denylist** - HTTP_PROXY, HTTPS_PROXY, etc. blocked by default +2. **Explicit deny lists** - Variables in `deny:` arrays (proxy and server level) +3. **Tier 1 variables** - Always inherited unless denied +4. **Tier 2 variables** - Inherited if mode includes tier2 +5. **Extra variables** - From `extra:` lists (proxy and server level) +6. **Prefix-matched variables** - Variables matching `prefix:` patterns +7. **Explicit `env:` overrides** - Always applied, never denied + +### Deny Resolution + +A variable is blocked if: +- It's in the **implicit denylist** AND not in `extra` with `allow_denied_if_explicit: true` +- It's in the **proxy-level deny list** AND not in `extra` with `allow_denied_if_explicit: true` +- It's in the **server-level deny list** AND not in `extra` with `allow_denied_if_explicit: true` + +Variables in the `env:` block are NEVER blocked, regardless of deny lists. + +### Example Resolution + +```yaml +proxy: + inherit: + mode: tier1 + extra: ["PROXY_VAR"] + deny: ["BLOCKED_VAR"] + +servers: + - name: my-server + inherit: + mode: tier1+tier2 + extra: ["SERVER_VAR"] + # deny: [] (not specified, so proxy deny list applies) + env: + EXPLICIT_VAR: "value" + BLOCKED_VAR: "allowed-via-override" +``` + +**Resolution**: +1. Implicit denylist: HTTP_PROXY, HTTPS_PROXY, etc. blocked +2. Proxy deny: BLOCKED_VAR blocked (from inheritance) +3. Tier 1: PATH, HOME, USER, SHELL, etc. inherited +4. Tier 2: SSL_CERT_FILE, etc. inherited (server mode is tier1+tier2) +5. Proxy extra: PROXY_VAR inherited +6. Server extra: SERVER_VAR inherited +7. Explicit env: EXPLICIT_VAR set, BLOCKED_VAR set (overrides deny) + +--- + +## Implicit Denylist Details + +### Why Block HTTP_PROXY? + +The `HTTP_PROXY` environment variable (uppercase) is vulnerable to the **httpoxy** attack in CGI-like environments: + +1. Attacker sends HTTP request with `Proxy: evil.com:8080` header +2. CGI environment sets `HTTP_PROXY=evil.com:8080` +3. Application uses `HTTP_PROXY` to proxy all outbound requests +4. Attacker intercepts all traffic, stealing credentials and data + +**References**: +- [httpoxy.org](https://httpoxy.org/) +- [CVE-2016-5385](https://nvd.nist.gov/vuln/detail/CVE-2016-5385) + +### Lowercase vs Uppercase + +- **Lowercase** (`http_proxy`, `https_proxy`) - Standard libcurl convention, generally safer +- **Uppercase** (`HTTP_PROXY`, `HTTPS_PROXY`) - Vulnerable to httpoxy in CGI environments + +The implicit denylist blocks **both** out of an abundance of caution. If you need proxy support: + +```yaml +inherit: + extra: ["http_proxy", "https_proxy"] # Use lowercase variants + allow_denied_if_explicit: true +``` + +### Full Implicit Denylist + +``` +HTTP_PROXY +HTTPS_PROXY +http_proxy +https_proxy +NO_PROXY +no_proxy +``` + +--- + +## Troubleshooting + +### Server Can't Find Executables + +**Symptom**: Server fails with "command not found" errors. + +**Cause**: `PATH` denied (only possible with explicit `deny: ["PATH"]`). + +**Solution**: Don't deny PATH, or set it explicitly: + +```yaml +inherit: + mode: tier1 # PATH included by default +# OR if you denied it: +env: + PATH: "/usr/local/bin:/usr/bin:/bin" +``` + +### Server Can't Validate TLS Certificates + +**Symptom**: SSL certificate verification failures in corporate environment. + +**Cause**: Missing CA bundle environment variables. + +**Solution**: Use `tier1+tier2` mode or add SSL variables explicitly: + +```yaml +inherit: + mode: tier1+tier2 # Includes SSL_CERT_FILE, etc. +# OR +inherit: + mode: tier1 + extra: ["SSL_CERT_FILE", "REQUESTS_CA_BUNDLE"] +``` + +### Proxy Variables Not Working + +**Symptom**: Server can't connect through corporate proxy. + +**Cause**: Proxy variables in implicit denylist. + +**Solution**: Explicitly allow lowercase proxy variables: + +```yaml +inherit: + extra: ["http_proxy", "https_proxy", "no_proxy"] + allow_denied_if_explicit: true +``` + +### Server Missing Application-Specific Variables + +**Symptom**: Server errors about missing configuration. + +**Cause**: Variables not in tier definitions. + +**Solution**: Use `extra` or `prefix`: + +```yaml +inherit: + mode: tier1 + extra: ["MY_API_KEY", "MY_CONFIG"] +# OR +inherit: + mode: tier1 + prefix: ["MY_APP_"] # Inherits MY_APP_KEY, MY_APP_URL, etc. +``` + +### "Mode All" Doesn't Inherit Everything + +**Symptom**: Variables are missing despite using `mode: all`. + +**Cause**: `mode: all` is equivalent to `tier1+tier2`, not "inherit everything". + +**Solution**: This is intentional for security. Explicitly add needed variables: + +```yaml +inherit: + mode: all # tier1+tier2 + extra: ["MY_VAR", "ANOTHER_VAR", "SECRET_KEY"] +``` + +### Need True Isolation (No Tier 1) + +**Symptom**: Want to block all automatic inheritance, including Tier 1. + +**Cause**: Tier 1 is always inherited unless explicitly denied. + +**Solution**: Explicitly deny all Tier 1 variables: + +```yaml +inherit: + mode: tier1 + deny: ["PATH", "HOME", "USER", "SHELL", "LANG", "LC_ALL", "TZ", "TMPDIR", "TEMP", "TMP"] +env: + # Only set what you need + CUSTOM_VAR: "value" +``` + +**Warning**: This will likely break most servers. Only use for controlled testing. + +### Case Sensitivity Issues (Windows) + +**Symptom**: Environment variables not being inherited on Windows. + +**Cause**: Case mismatch between config and actual variable names. + +**Solution**: On Windows, environment variables are case-insensitive. The system will normalize keys automatically, but for consistency, match the case used in your environment: + +```yaml +# Windows: both work +inherit: + extra: ["PATH"] # Standard + extra: ["Path"] # Also works +``` + +### Checking What's Actually Inherited + +**Debug technique**: Create a test server that prints its environment: + +```yaml +servers: + - name: env-test + transport: stdio + command: /usr/bin/env # Unix + # command: cmd # Windows + # args: ["/c", "set"] # Windows + inherit: + mode: tier1 + extra: ["TEST_VAR"] +``` + +Run discovery or proxy mode and examine the output to see exactly what variables were inherited. + +--- + +## Validation + +mcp-debug validates inheritance configuration at startup. + +### Valid Modes + +```yaml +inherit: + mode: "none" # ✓ Valid (same as tier1) + mode: "tier1" # ✓ Valid + mode: "tier1+tier2" # ✓ Valid + mode: "all" # ✓ Valid (same as tier1+tier2) + mode: "" # ✓ Valid (defaults to tier1) +``` + +### Invalid Modes + +```yaml +inherit: + mode: "tier2" # ✗ Invalid (no tier2-only mode) + mode: "tier1,tier2" # ✗ Invalid (use "tier1+tier2") + mode: "everything" # ✗ Invalid (not a defined mode) +``` + +### Validation Errors + +**Invalid mode**: +``` +Error: server 'my-server': inherit: invalid mode "tier2": must be one of: none, tier1, tier1+tier2, all +``` + +**Solution**: Fix the mode value in your configuration. + +### Running Validation + +```bash +# Validate config file +uvx mcp-debug config validate --config config.yaml + +# Test server startup +uvx mcp-debug --proxy --config config.yaml --log /tmp/debug.log +``` + +Check the log file for validation errors or warnings. + +--- + +## Platform Differences + +### Windows vs Unix + +**Environment Variable Names**: +- **Unix/Linux/macOS**: Case-sensitive (`PATH` ≠ `path`) +- **Windows**: Case-insensitive (`PATH` = `Path` = `path`) + +**Temporary Directories**: +- **Unix**: `TMPDIR` +- **Windows**: `TEMP`, `TMP` + +The tier system includes all variants for cross-platform compatibility. + +**File Paths in Values**: +- **Unix**: `/home/user/.config` +- **Windows**: `C:\Users\User\AppData\Roaming` + +Environment variable expansion respects platform path conventions. + +### XDG Base Directory Specification + +On Unix-like systems, Tier 1 includes XDG Base Directory variables per the [freedesktop.org specification](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html): + +- `XDG_CONFIG_HOME` - User configuration files +- `XDG_CACHE_HOME` - User cache data +- `XDG_DATA_HOME` - User data files +- `XDG_STATE_HOME` - User state data +- `XDG_RUNTIME_DIR` - Runtime files and sockets + +These are not applicable on Windows. + +--- + +## Advanced Use Cases + +### Multi-Tenant Isolation + +**Scenario**: Running multiple MCP servers for different customers, ensuring complete isolation. + +```yaml +servers: + - name: customer-a + transport: stdio + command: python3 + args: ["-m", "mcp_server"] + inherit: + mode: tier1 + deny: ["HOME", "USER"] # Block even Tier 1 vars for extra isolation + env: + CUSTOMER_ID: "customer-a" + DB_URL: "postgresql://db-a/data" + HOME: "/var/lib/customer-a" # Custom HOME + + - name: customer-b + transport: stdio + command: python3 + args: ["-m", "mcp_server"] + inherit: + mode: tier1 + deny: ["HOME", "USER"] + env: + CUSTOMER_ID: "customer-b" + DB_URL: "postgresql://db-b/data" + HOME: "/var/lib/customer-b" # Custom HOME +``` + +Each server has isolated environment with custom values. + +### Development vs Production + +**Scenario**: Different inheritance rules for dev and production. + +```yaml +# development-config.yaml +proxy: + inherit: + mode: tier1+tier2 # More permissive for development + extra: ["DEBUG", "DEV_TOKEN"] + +servers: + - name: dev-server + transport: stdio + command: ./dev-server +``` + +```yaml +# production-config.yaml +proxy: + inherit: + mode: tier1 # Strict for production + deny: ["SSH_AUTH_SOCK", "AWS_SESSION_TOKEN"] + +servers: + - name: prod-server + transport: stdio + command: ./prod-server +``` + +### Dynamic Environment Variables + +**Scenario**: Pass current timestamp or dynamic values to servers. + +```bash +# Set variable in parent shell +export BUILD_ID="$(date +%s)" +export DEPLOY_VERSION="v1.2.3" +``` + +```yaml +servers: + - name: my-server + transport: stdio + command: python3 + args: ["-m", "server"] + inherit: + mode: tier1 + extra: ["BUILD_ID", "DEPLOY_VERSION"] +``` + +The server receives the current values of these variables. + +### Testing Different Inheritance Modes + +**Scenario**: A/B testing different inheritance configurations. + +Create multiple config files and test: + +```bash +# Test tier1 (minimal) +uvx mcp-debug --proxy --config config-tier1.yaml + +# Test tier1+tier2 (with network) +uvx mcp-debug --proxy --config config-tier2.yaml + +# Test with extras +uvx mcp-debug --proxy --config config-extras.yaml +``` + +Compare behavior and choose the most secure option that works. + +--- + +## Migration Guide + +### From No Configuration + +If you previously ran mcp-debug without any inheritance configuration: + +**Old behavior**: Depended on implementation details (may have varied) + +**New behavior**: Defaults to `mode: tier1` (secure by default) + +**Migration path**: +1. Test with default `tier1` mode +2. If servers break, identify missing variables via logs +3. Add missing variables to `extra` list +4. OR switch to `mode: tier1+tier2` if TLS variables are needed + +### From Expecting "All Variables" + +If you expected all parent environment variables to be inherited: + +**Old expectation**: All variables from parent process + +**New behavior**: Only Tier 1 (or Tier 1 + Tier 2 with `mode: all`) + +**Migration path**: + +**Step 1**: Identify which variables your server actually needs: + +```yaml +inherit: + mode: tier1 # Start minimal + # Server will fail with clear errors about missing vars +``` + +**Step 2**: Add needed variables explicitly: + +```yaml +inherit: + mode: tier1+tier2 # If TLS needed + extra: ["DATABASE_URL", "API_KEY", "CUSTOM_CONFIG"] +``` + +**Step 3**: Test thoroughly and add missing variables as needed. + +### Adding to Existing Servers + +If you have existing server configurations without inheritance: + +**Before**: +```yaml +servers: + - name: my-server + transport: stdio + command: python3 + args: ["-m", "server"] +``` + +**After (explicit tier1)**: +```yaml +servers: + - name: my-server + transport: stdio + command: python3 + args: ["-m", "server"] + inherit: + mode: tier1 # Make it explicit + extra: ["PYTHONPATH"] +``` + +The default behavior (tier1) is applied automatically if you don't add an `inherit` block. + +--- + +## Testing Your Configuration + +### Manual Testing + +**Step 1**: Create a test server that prints its environment: + +```yaml +# test-config.yaml +servers: + - name: env-dump + transport: stdio + command: /usr/bin/env + inherit: + mode: tier1 + extra: ["TEST_VAR"] +``` + +**Step 2**: Set test variables: + +```bash +export TEST_VAR="test-value" +export SECRET_VAR="should-not-appear" +``` + +**Step 3**: Run mcp-debug: + +```bash +uvx mcp-debug --proxy --config test-config.yaml +``` + +**Step 4**: Check output - `TEST_VAR` should appear, `SECRET_VAR` should not. + +### Automated Testing + +Create a test script: + +```bash +#!/bin/bash + +export HOME="/home/testuser" # Tier 1 - should inherit +export SSL_CERT_FILE="/etc/ssl/cert.pem" # Tier 2 - only with tier2 mode +export CUSTOM_VAR="test" # Only with extra +export SECRET_VAR="secret" # Should NOT inherit + +# Test tier1 mode +echo "Testing tier1 mode..." +uvx mcp-debug --proxy --config test-tier1.yaml 2>&1 | grep -q "HOME=" +if [ $? -eq 0 ]; then echo "✓ Tier1 works"; else echo "✗ Tier1 failed"; fi + +# Test tier1+tier2 mode +echo "Testing tier1+tier2 mode..." +uvx mcp-debug --proxy --config test-tier2.yaml 2>&1 | grep -q "SSL_CERT_FILE=" +if [ $? -eq 0 ]; then echo "✓ Tier2 works"; else echo "✗ Tier2 failed"; fi + +# Verify secret not leaked +echo "Testing secret isolation..." +uvx mcp-debug --proxy --config test-tier1.yaml 2>&1 | grep -q "SECRET_VAR=" +if [ $? -eq 1 ]; then echo "✓ Secret blocked"; else echo "✗ Secret leaked!"; fi +``` + +### Integration Testing + +Test with real MCP servers: + +```yaml +servers: + - name: real-server + transport: stdio + command: python3 + args: ["-m", "my_real_server"] + inherit: + mode: tier1 + extra: ["PYTHONPATH"] +``` + +Run through normal workflows and verify: +1. Server starts successfully +2. Tools work as expected +3. No errors about missing environment variables +4. Sensitive variables are not accessible to the server + +--- + +## FAQ + +### Q: What happens if I don't specify an `inherit` block? + +**A**: Defaults to `mode: tier1` (secure by default). Only Tier 1 baseline variables are inherited. + +### Q: What's the difference between `mode: none` and `mode: tier1`? + +**A**: They're identical in the current implementation. Both inherit Tier 1 baseline variables. The "none" naming is kept for backward compatibility. + +### Q: How do I achieve true isolation (no automatic inheritance)? + +**A**: Use `mode: tier1` with an explicit `deny:` list containing all Tier 1 variables: + +```yaml +inherit: + mode: tier1 + deny: ["PATH", "HOME", "USER", "SHELL", "LANG", "LC_ALL", "TZ", "TMPDIR", "TEMP", "TMP"] +env: + # Only set what you need + CUSTOM_VAR: "value" +``` + +### Q: Why doesn't `mode: all` inherit ALL variables? + +**A**: Security-first design. `mode: all` is equivalent to `tier1+tier2`. Inheriting all parent variables would risk leaking credentials and secrets. To inherit additional variables, use the `extra:` list. + +### Q: Can I use `mode: tier2` without `tier1`? + +**A**: No. The only modes are `none` (=tier1), `tier1`, `tier1+tier2`, and `all` (=tier1+tier2). Tier 2 always includes Tier 1. + +### Q: Why are proxy variables blocked by default? + +**A**: To prevent httpoxy attacks. The uppercase `HTTP_PROXY` variable can be set by attackers via HTTP headers in CGI environments. We block both uppercase and lowercase variants out of caution. + +### Q: How do I enable proxy variables safely? + +**A**: Use lowercase variants with explicit opt-in: + +```yaml +inherit: + mode: tier1+tier2 + extra: ["http_proxy", "https_proxy", "no_proxy"] + allow_denied_if_explicit: true +``` + +### Q: Does `env:` override the deny list? + +**A**: Yes. Explicit `env:` overrides are ALWAYS applied, regardless of deny lists. + +```yaml +inherit: + deny: ["BLOCKED_VAR"] +env: + BLOCKED_VAR: "this-will-be-set" # Always wins +``` + +### Q: Can I mix `prefix` and `extra`? + +**A**: Yes. They're additive: + +```yaml +inherit: + extra: ["CUSTOM1", "CUSTOM2"] + prefix: ["MY_APP_", "CONFIG_"] +``` + +This inherits: Tier 1 + CUSTOM1 + CUSTOM2 + any variables starting with MY_APP_ or CONFIG_. + +### Q: Are environment variable names case-sensitive? + +**A**: On Unix: yes (`PATH` ≠ `path`). On Windows: no (`PATH` = `Path` = `path`). + +### Q: Can I set proxy-level defaults and override per-server? + +**A**: Yes: + +```yaml +proxy: + inherit: + mode: tier1 # Default for all servers + +servers: + - name: server1 + inherit: + mode: tier1+tier2 # Override for this server +``` + +### Q: What if a variable is in both `extra` and `deny`? + +**A**: Depends on `allow_denied_if_explicit`: +- `false` (default): Variable is blocked +- `true`: Variable is allowed (because it's in `extra`) + +### Q: How do I debug what's being inherited? + +**A**: Use a test server that prints its environment: + +```yaml +servers: + - name: env-test + command: /usr/bin/env + inherit: + mode: tier1 +``` + +### Q: Can I use environment variable expansion in the `inherit` block? + +**A**: Yes, for the lists: + +```yaml +inherit: + extra: ["${MY_VAR_NAME}"] # Expands at config load time +``` + +But this is rarely useful. More commonly, you'd use expansion in `env:`: + +```yaml +env: + API_KEY: "${PARENT_API_KEY}" # Gets value from parent +``` + +### Q: Does this work with SSE (Server-Sent Events) transport? + +**A**: The inheritance system only applies to `stdio` transport (local child processes). SSE and HTTP transports don't have environment inheritance because they're remote connections. + +### Q: Why is Tier 1 always inherited? + +**A**: Security AND functionality. Tier 1 variables (PATH, HOME, etc.) are required for basic process operation and rarely contain secrets. Making them the guaranteed baseline prevents accidentally creating broken environments while maintaining security. You can still block them with `deny:` if needed for testing. + +--- + +## See Also + +- [Main README](README.md) - mcp-debug overview +- [Configuration Examples](examples/) - Sample config files +- [MCP Specification](https://modelcontextprotocol.io/) - Model Context Protocol docs +- [httpoxy.org](https://httpoxy.org/) - httpoxy vulnerability details +- [XDG Base Directory](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html) - XDG spec + +--- + +## Feedback and Updates + +This is DRAFT documentation for a newly implemented feature. As we gather real-world usage data, we may: + +- Adjust tier definitions based on common requirements +- Add new configuration options +- Update security recommendations +- Add more examples and troubleshooting guidance + +**Report issues or suggest improvements**: +- GitHub Issues: [mcp-debug issues](https://github.com/standardbeagle/mcp-debug/issues) +- Discussion: Include "[env-inheritance]" in the title + +**Last Updated**: 2026-01-13 +**Implementation Commit**: 49f5581 +**Branch**: feature/env-selective-inheritance diff --git a/README.md b/README.md index 300f61e..5bc25a2 100644 --- a/README.md +++ b/README.md @@ -20,9 +20,13 @@ MCP Debug enables rapid development and testing of MCP servers with hot-swapping ### Session Recording & Playback - Record JSON-RPC traffic for debugging and documentation +- Records all tool calls (static and dynamic servers) +- Records management operations (server_add, etc.) +- Tool responses include recording metadata when active - Playback client mode - replay requests to test servers - Playback server mode - replay responses to test clients - Regression testing with recorded sessions +- **[📖 Recording Documentation](docs/RECORDING.md)** - Complete recording guide ### Development Proxy - Multi-server aggregation with tool prefixing @@ -76,7 +80,7 @@ uvx mcp-debug --proxy --config config.yaml --log /tmp/debug.log - `server_add` - Add a server: `{name: "fs", command: "npx -y @mcp/filesystem /path"}` - `server_remove` - Remove server completely - `server_disconnect` - Disconnect server (tools return errors) -- `server_reconnect` - Reconnect with new command +- `server_reconnect` - Reconnect with optional new command (preserves config if omitted) - `server_list` - Show all servers and status ### Playback Modes @@ -89,6 +93,8 @@ uvx mcp-debug --playback-client session.jsonl | ./your-mcp-server mcp-tui uvx mcp-debug --playback-server session.jsonl ``` +**See [Recording Documentation](docs/RECORDING.md) for detailed recording format, workflows, and examples.** + ## Configuration ```yaml @@ -116,8 +122,70 @@ MCP_RECORD_FILE="session.jsonl" # Auto-record sessions MCP_CONFIG_PATH="./config.yaml" # Default config ``` +## Environment Variable Inheritance (DRAFT) + +> **⚠️ DRAFT**: This feature is fully implemented but not yet validated with real-world MCP servers. See [DRAFT_ENV_INHERITANCE.md](DRAFT_ENV_INHERITANCE.md) for complete documentation. + +mcp-debug implements a tier-based environment variable inheritance system to prevent accidental leakage of sensitive values (credentials, tokens, SSH agent sockets) to upstream MCP servers. + +### Security-First Design + +By default, only **Tier 1 baseline variables** (PATH, HOME, USER, SHELL, locale, TMPDIR) are inherited. This prevents inadvertent exposure of: +- Cloud provider credentials (AWS_ACCESS_KEY_ID, AZURE_CLIENT_SECRET) +- Authentication tokens (GITHUB_TOKEN, ANTHROPIC_API_KEY) +- SSH agent sockets (SSH_AUTH_SOCK) +- Development secrets and corporate credentials + +You can control inheritance behavior per-server or set proxy-wide defaults. + +### Quick Example + +```yaml +# Secure by default - only baseline variables +servers: + - name: untrusted-server + transport: stdio + command: python3 + args: ["-m", "experimental_server"] + # No inherit block = tier1 mode (secure default) + +# Explicit control for trusted servers + - name: trusted-server + transport: stdio + command: python3 + args: ["-m", "my_server"] + inherit: + mode: tier1+tier2 # Add network/TLS variables + extra: ["PYTHONPATH", "VIRTUAL_ENV"] # Explicitly add needed vars + prefix: ["MY_APP_"] # Include all MY_APP_* variables + deny: ["SSH_AUTH_SOCK"] # Block specific variables +``` + +### Inheritance Modes + +| Mode | Description | Use Case | +|------|-------------|----------| +| `none` | No inheritance (only explicit `env:` values) | Maximum isolation | +| `tier1` | Baseline variables only (DEFAULT) | Most servers | +| `tier1+tier2` | Baseline + network/TLS variables | Servers making HTTPS requests | +| `all` | Inherit everything (with optional deny list) | Fully trusted servers | + +### Tier Definitions + +**Tier 1 (Baseline)**: PATH, HOME, USER, SHELL, LANG, LC_ALL, TZ, TMPDIR, TEMP, TMP + +**Tier 2 (Network/TLS)**: SSL_CERT_FILE, SSL_CERT_DIR, REQUESTS_CA_BUNDLE, CURL_CA_BUNDLE, NODE_EXTRA_CA_CERTS + +**Implicit Denylist**: HTTP_PROXY, HTTPS_PROXY, http_proxy, https_proxy, NO_PROXY, no_proxy (httpoxy mitigation) + +### Complete Documentation + +For complete documentation including all configuration options, security rationale, troubleshooting, and advanced use cases, see [DRAFT_ENV_INHERITANCE.md](DRAFT_ENV_INHERITANCE.md). + ## Development Workflow +### Dynamic Servers + ```bash # 1. Start with empty config mcp-tui uvx mcp-debug --proxy --config empty-config.yaml @@ -137,6 +205,45 @@ server_reconnect: {name: myserver, command: ./my-server-v2} # 6. Same tools work immediately with new implementation! ``` +### Static Servers (from config.yaml) + +**NEW:** Static servers can now be hot-swapped while preserving all configuration including environment variables! + +```bash +# config.yaml +servers: + - name: "my-server" + command: "python3" + args: ["-m", "my_server"] + env: + API_KEY: "${MY_API_KEY}" + API_SECRET: "${MY_SECRET}" + timeout: "300s" + +# 1. Start proxy with config +mcp-tui uvx mcp-debug --proxy --config config.yaml + +# 2. Make changes to server code + +# 3. Hot-swap WITHOUT exposing secrets +server_disconnect: {name: "my-server"} +server_reconnect: {name: "my-server"} # No command = uses stored config + +# 4. Server reconnects with ALL env vars preserved! +``` + +**Key Benefits:** +- Environment variables (including secrets) never exposed to MCP client +- Inheritance settings preserved +- Timeout and other config preserved +- Same tools work immediately with new implementation + +**Alternative:** Provide new command to replace configuration: +```bash +server_reconnect: {name: "my-server", command: "./new-server"} +# Warning: This loses env vars and other config from config.yaml +``` + ## CLI Commands ```bash diff --git a/client/env.go b/client/env.go new file mode 100644 index 0000000..4c4b049 --- /dev/null +++ b/client/env.go @@ -0,0 +1,72 @@ +package client + +import ( + "os" + "runtime" + "strings" +) + +// MergeEnvironment merges the parent process environment with overrides. +// The parent environment is obtained via os.Environ() and then override +// values are applied on top. On Windows, environment variable names are +// case-insensitive and normalized to uppercase for proper deduplication. +// +// Returns a []string in "KEY=value" format suitable for exec.Cmd.Env. +func MergeEnvironment(overrides map[string]string) []string { + isWindows := runtime.GOOS == "windows" + + // Maps for tracking environment variables + // envMap: normalized_key -> value + // keyMap: normalized_key -> original_key (preserves casing) + envMap := make(map[string]string) + keyMap := make(map[string]string) + + // Load parent environment + for _, entry := range os.Environ() { + key, value := splitEnvEntry(entry) + if key == "" { + continue // Skip malformed entries + } + + lookupKey := normalizeKey(key, isWindows) + envMap[lookupKey] = value + keyMap[lookupKey] = key + } + + // Apply overrides (last wins) + for key, value := range overrides { + lookupKey := normalizeKey(key, isWindows) + envMap[lookupKey] = value + keyMap[lookupKey] = key // Use override's exact casing + } + + // Build result slice + result := make([]string, 0, len(envMap)) + for lookupKey, value := range envMap { + originalKey := keyMap[lookupKey] + result = append(result, originalKey+"="+value) + } + + return result +} + +// normalizeKey normalizes environment variable keys for comparison. +// On Windows, converts to uppercase for case-insensitive comparison. +// On other platforms, returns the key unchanged. +func normalizeKey(key string, isWindows bool) string { + if isWindows { + return strings.ToUpper(key) + } + return key +} + +// splitEnvEntry splits an environment entry "KEY=value" into key and value. +// Handles values containing "=" by only splitting on the first occurrence. +// Returns empty string for key if the entry is malformed. +func splitEnvEntry(entry string) (key, value string) { + idx := strings.Index(entry, "=") + if idx <= 0 { + return "", "" // Malformed entry + } + return entry[:idx], entry[idx+1:] +} diff --git a/client/env_test.go b/client/env_test.go new file mode 100644 index 0000000..d75534a --- /dev/null +++ b/client/env_test.go @@ -0,0 +1,238 @@ +package client + +import ( + "os" + "runtime" + "testing" +) + +// TestMergeEnvironment tests the MergeEnvironment function with various scenarios +func TestMergeEnvironment(t *testing.T) { + tests := []struct { + name string + parent []string + overrides map[string]string + expected map[string]string + }{ + { + name: "empty overrides", + parent: []string{"PATH=/usr/bin", "HOME=/home/user"}, + overrides: map[string]string{}, + expected: map[string]string{ + "PATH": "/usr/bin", + "HOME": "/home/user", + }, + }, + { + name: "override existing variable", + parent: []string{"PATH=/usr/bin", "HOME=/home/user"}, + overrides: map[string]string{ + "PATH": "/custom/bin", + }, + expected: map[string]string{ + "PATH": "/custom/bin", + "HOME": "/home/user", + }, + }, + { + name: "add new variable", + parent: []string{"PATH=/usr/bin"}, + overrides: map[string]string{ + "DEBUG": "true", + }, + expected: map[string]string{ + "PATH": "/usr/bin", + "DEBUG": "true", + }, + }, + { + name: "mixed override and add", + parent: []string{"PATH=/usr/bin", "HOME=/home/user"}, + overrides: map[string]string{ + "PATH": "/custom/bin", + "DEBUG": "true", + }, + expected: map[string]string{ + "PATH": "/custom/bin", + "HOME": "/home/user", + "DEBUG": "true", + }, + }, + { + name: "empty string value", + parent: []string{"PATH=/usr/bin"}, + overrides: map[string]string{ + "PATH": "", + }, + expected: map[string]string{ + "PATH": "", + }, + }, + { + name: "nil parent", + parent: nil, + overrides: map[string]string{ + "DEBUG": "true", + }, + expected: map[string]string{ + "DEBUG": "true", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Temporarily set system env to match parent for testing + if tt.parent != nil { + // Clear env and set to parent values for this test + oldEnv := os.Environ() + defer func() { + // Restore original env + os.Clearenv() + for _, entry := range oldEnv { + key, value := splitEnvEntry(entry) + if key != "" { + os.Setenv(key, value) + } + } + }() + + os.Clearenv() + for _, entry := range tt.parent { + key, value := splitEnvEntry(entry) + if key != "" { + os.Setenv(key, value) + } + } + } + + result := MergeEnvironment(tt.overrides) + resultMap := sliceToMap(result) + + // If parent is nil, merge with system environment + if tt.parent == nil { + // Just check that our override is present + if val, ok := resultMap["DEBUG"]; !ok || val != "true" { + t.Errorf("Expected DEBUG=true in result, got %v", resultMap) + } + return + } + + // Check all expected variables are present with correct values + for key, expectedVal := range tt.expected { + if actualVal, ok := resultMap[key]; !ok { + t.Errorf("Expected key %q not found in result", key) + } else if actualVal != expectedVal { + t.Errorf("For key %q: expected %q, got %q", key, expectedVal, actualVal) + } + } + + // Check no unexpected variables (only for non-nil parent) + if len(resultMap) != len(tt.expected) { + t.Errorf("Expected %d variables, got %d: %v", len(tt.expected), len(resultMap), resultMap) + } + }) + } +} + +// TestMergeEnvironmentWindows tests Windows-specific case-insensitive behavior +func TestMergeEnvironmentWindows(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("Skipping Windows-specific test") + } + + // Save and restore environment + defer restoreEnv("PATH")() + + // Set up test environment + oldEnv := os.Environ() + defer func() { + // Restore original env + os.Clearenv() + for _, entry := range oldEnv { + key, value := splitEnvEntry(entry) + if key != "" { + os.Setenv(key, value) + } + } + }() + + os.Clearenv() + os.Setenv("PATH", "C:\\Windows\\System32") + os.Setenv("HOME", "C:\\Users\\test") + + overrides := map[string]string{ + "Path": "C:\\Custom\\Path", // Different case + } + + result := MergeEnvironment(overrides) + resultMap := sliceToMap(result) + + // On Windows, PATH and Path should be treated as the same variable + pathValue := "" + for key, val := range resultMap { + if key == "PATH" || key == "Path" || key == "path" { + pathValue = val + break + } + } + + if pathValue != "C:\\Custom\\Path" { + t.Errorf("Expected PATH to be overridden (case-insensitive), got: %v", resultMap) + } + + // HOME should still be present + if home, ok := resultMap["HOME"]; !ok || home != "C:\\Users\\test" { + t.Errorf("Expected HOME=C:\\Users\\test, got: %v", resultMap) + } +} + +// restoreEnv saves the current environment variable and returns a function to restore it +func restoreEnv(key string) func() { + original, exists := os.LookupEnv(key) + return func() { + if exists { + os.Setenv(key, original) + } else { + os.Unsetenv(key) + } + } +} + +// TestSplitEnvEntry tests parsing of environment variable entries +func TestSplitEnvEntry(t *testing.T) { + tests := []struct { + name string + entry string + wantKey string + wantValue string + }{ + {"normal entry", "PATH=/usr/bin", "PATH", "/usr/bin"}, + {"value with equals", "URL=http://example.com?foo=bar", "URL", "http://example.com?foo=bar"}, + {"empty value", "EMPTY=", "EMPTY", ""}, + {"no equals", "INVALID", "", ""}, + {"empty key", "=value", "", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + key, value := splitEnvEntry(tt.entry) + if key != tt.wantKey || value != tt.wantValue { + t.Errorf("splitEnvEntry(%q) = (%q, %q), want (%q, %q)", + tt.entry, key, value, tt.wantKey, tt.wantValue) + } + }) + } +} + +// sliceToMap converts []string environment to map[string]string for easier testing +func sliceToMap(env []string) map[string]string { + result := make(map[string]string) + for _, entry := range env { + key, value := splitEnvEntry(entry) + if key != "" { + result[key] = value + } + } + return result +} diff --git a/client/envbuilder.go b/client/envbuilder.go new file mode 100644 index 0000000..655ca9a --- /dev/null +++ b/client/envbuilder.go @@ -0,0 +1,249 @@ +package client + +import ( + "os" + "runtime" + "strings" + + "mcp-debug/config" +) + +// Tier1Vars are baseline environment variables that most processes need. +// These are always inherited unless explicitly denied. +var Tier1Vars = []string{ + "PATH", + "HOME", + "USER", + "SHELL", + "LANG", + "LC_ALL", + "TZ", + "TMPDIR", + "TEMP", + "TMP", +} + +// Tier2Vars are network and TLS-related variables. +// These are inherited when TLS inheritance is enabled. +var Tier2Vars = []string{ + "SSL_CERT_FILE", + "SSL_CERT_DIR", + "REQUESTS_CA_BUNDLE", + "CURL_CA_BUNDLE", + "NODE_EXTRA_CA_CERTS", +} + +// ImplicitDenylist contains variables that should never be inherited +// without explicit configuration, as they can cause unexpected behavior. +var ImplicitDenylist = []string{ + "HTTP_PROXY", + "HTTPS_PROXY", + "http_proxy", + "https_proxy", + "NO_PROXY", + "no_proxy", +} + +// BuildEnvironment constructs the environment for an MCP server based on +// tier-based inheritance rules and configuration overrides. +// +// Inheritance tiers: +// - Tier 1 (baseline): Always inherited unless explicitly denied +// - Tier 2 (network/TLS): Inherited when TLS inheritance enabled +// - Implicit denylist: Blocked by default (e.g., HTTP_PROXY) +// - Extra variables: Additional variables specified in config +// - Prefix matching: Variables matching configured prefixes +// +// Configuration precedence (highest to lowest): +// 1. Explicit env overrides in server config +// 2. Explicit deny rules (server and proxy level) +// 3. Tier 1 variables (unless denied) +// 4. Tier 2 variables (if TLS enabled, unless denied) +// 5. Extra variables from config (unless denied) +// 6. Prefix-matched variables (unless denied) +// +// Parameters: +// - serverConfig: The server configuration containing env overrides and inheritance rules +// - proxyInherit: Proxy-level inheritance configuration (may be nil) +// +// Returns: +// - []string: Environment in "KEY=value" format for exec.Cmd.Env +func BuildEnvironment(serverConfig *config.ServerConfig, proxyInherit *config.InheritConfig) []string { + isWindows := runtime.GOOS == "windows" + + // Build combined deny map (normalized keys) + denyMap := buildDenyMap(serverConfig, proxyInherit, isWindows) + + // Build parent environment map (normalized lookup keys) + parentMap := buildParentMap() + + // Result map: normalized_key -> (original_key, value) + envMap := make(map[string]struct { + key string + value string + }) + + // Helper to add variable if not denied + // explicitExtra indicates if this is from the Extra list (bypasses implicit deny) + addVar := func(key string, explicitExtra bool) { + lookupKey := normalizeKey(key, isWindows) + + // Check if denied + if denyMap[lookupKey] { + // If this is from Extra list and AllowDeniedIfExplicit is true, allow it + if explicitExtra { + if serverConfig.Inherit != nil && serverConfig.Inherit.AllowDeniedIfExplicit { + // Allow this variable even though it's denied + } else if proxyInherit != nil && proxyInherit.AllowDeniedIfExplicit { + // Allow this variable even though it's denied + } else { + return // Denied and not explicitly allowed + } + } else { + return // Explicitly denied + } + } + + if val, exists := parentMap[lookupKey]; exists { + envMap[lookupKey] = struct { + key string + value string + }{key, val} + } + } + + // Step 1: Add Tier 1 (baseline) variables + for _, key := range Tier1Vars { + addVar(key, false) + } + + // Step 2: Add Tier 2 (network/TLS) variables if tier1+tier2 or all mode enabled + tier2Enabled := false + if serverConfig.Inherit != nil { + if serverConfig.Inherit.Mode == config.InheritTier1Tier2 || serverConfig.Inherit.Mode == config.InheritAll { + tier2Enabled = true + } + } + if !tier2Enabled && proxyInherit != nil { + if proxyInherit.Mode == config.InheritTier1Tier2 || proxyInherit.Mode == config.InheritAll { + tier2Enabled = true + } + } + if tier2Enabled { + for _, key := range Tier2Vars { + addVar(key, false) + } + } + + // Step 3: Add extra variables from config (server level, then proxy level) + if serverConfig.Inherit != nil { + for _, key := range serverConfig.Inherit.Extra { + addVar(key, true) // Mark as explicit extra + } + } + if proxyInherit != nil { + for _, key := range proxyInherit.Extra { + addVar(key, true) // Mark as explicit extra + } + } + + // Step 4: Add prefix-matched variables (server level, then proxy level) + prefixes := []string{} + if serverConfig.Inherit != nil { + prefixes = append(prefixes, serverConfig.Inherit.Prefix...) + } + if proxyInherit != nil { + prefixes = append(prefixes, proxyInherit.Prefix...) + } + + for lookupKey, val := range parentMap { + if denyMap[lookupKey] { + continue // Already denied + } + // Check if any prefix matches + for _, prefix := range prefixes { + normalizedPrefix := normalizeKey(prefix, isWindows) + if strings.HasPrefix(lookupKey, normalizedPrefix) { + // Find original key from parent environment + originalKey := "" + for _, entry := range os.Environ() { + k, v := splitEnvEntry(entry) + if normalizeKey(k, isWindows) == lookupKey && v == val { + originalKey = k + break + } + } + if originalKey != "" { + envMap[lookupKey] = struct { + key string + value string + }{originalKey, val} + } + break + } + } + } + + // Step 5: Apply explicit environment overrides from server config + // These override everything and ignore deny rules + for key, value := range serverConfig.Env { + lookupKey := normalizeKey(key, isWindows) + envMap[lookupKey] = struct { + key string + value string + }{key, value} + } + + // Build final result + result := make([]string, 0, len(envMap)) + for _, entry := range envMap { + result = append(result, entry.key+"="+entry.value) + } + + return result +} + +// buildDenyMap creates a normalized map of denied variable names. +// Includes implicit denylist plus any explicit deny rules from config. +func buildDenyMap(serverConfig *config.ServerConfig, proxyInherit *config.InheritConfig, isWindows bool) map[string]bool { + denyMap := make(map[string]bool) + + // Add implicit denylist + for _, key := range ImplicitDenylist { + denyMap[normalizeKey(key, isWindows)] = true + } + + // Add server-level deny rules + if serverConfig.Inherit != nil { + for _, key := range serverConfig.Inherit.Deny { + denyMap[normalizeKey(key, isWindows)] = true + } + } + + // Add proxy-level deny rules + if proxyInherit != nil { + for _, key := range proxyInherit.Deny { + denyMap[normalizeKey(key, isWindows)] = true + } + } + + return denyMap +} + +// buildParentMap creates a normalized map of parent environment variables. +// Returns: map[normalized_key]value +func buildParentMap() map[string]string { + isWindows := runtime.GOOS == "windows" + parentMap := make(map[string]string) + + for _, entry := range os.Environ() { + key, value := splitEnvEntry(entry) + if key == "" { + continue + } + lookupKey := normalizeKey(key, isWindows) + parentMap[lookupKey] = value + } + + return parentMap +} diff --git a/client/envbuilder_test.go b/client/envbuilder_test.go new file mode 100644 index 0000000..dff9159 --- /dev/null +++ b/client/envbuilder_test.go @@ -0,0 +1,573 @@ +package client + +import ( + "os" + "runtime" + "strings" + "testing" + + "mcp-debug/config" +) + +// TestBuildEnvironment_ModeNone tests that only explicit overrides are included +func TestBuildEnvironment_ModeNone(t *testing.T) { + // Save and restore environment + oldEnv := os.Environ() + defer restoreEnvironment(oldEnv) + + // Set up test environment + os.Clearenv() + os.Setenv("HOME", "/home/user") + os.Setenv("PATH", "/usr/bin") + os.Setenv("SECRET_KEY", "should-not-inherit") + + // Create server config with mode=none and explicit overrides + serverCfg := &config.ServerConfig{ + Inherit: &config.InheritConfig{ + Mode: config.InheritNone, + }, + Env: map[string]string{ + "CUSTOM": "value", + }, + } + + // Build environment + result := BuildEnvironment(serverCfg, nil) + resultMap := sliceToMap(result) + + // Verify override is present + if resultMap["CUSTOM"] != "value" { + t.Error("CUSTOM override should be present") + } + + // Verify Tier1 vars ARE inherited (Tier1 is baseline, always inherited unless denied) + if _, ok := resultMap["HOME"]; !ok { + t.Error("HOME should be inherited (Tier1 baseline)") + } + if _, ok := resultMap["PATH"]; !ok { + t.Error("PATH should be inherited (Tier1 baseline)") + } + + // Verify non-tier1 var is NOT present + if _, ok := resultMap["SECRET_KEY"]; ok { + t.Error("SECRET_KEY should NOT be inherited in mode=none") + } +} + +// TestBuildEnvironment_ModeTier1 tests that only Tier1 vars are inherited +func TestBuildEnvironment_ModeTier1(t *testing.T) { + // Save and restore environment + oldEnv := os.Environ() + defer restoreEnvironment(oldEnv) + + // Set up test environment + os.Clearenv() + os.Setenv("HOME", "/home/user") + os.Setenv("PATH", "/usr/bin") + os.Setenv("USER", "testuser") + os.Setenv("SHELL", "/bin/bash") + os.Setenv("SECRET_KEY", "should-not-inherit") + os.Setenv("SSH_AUTH_SOCK", "/tmp/ssh-agent") + + // Create server config with tier1 mode and explicit overrides + serverCfg := &config.ServerConfig{ + Inherit: &config.InheritConfig{ + Mode: config.InheritTier1, + }, + Env: map[string]string{ + "CUSTOM": "value", + }, + } + + // Build environment + result := BuildEnvironment(serverCfg, nil) + resultMap := sliceToMap(result) + + // Verify tier1 vars are present + tier1Expected := []string{"HOME", "PATH", "USER", "SHELL"} + for _, varName := range tier1Expected { + if _, ok := resultMap[varName]; !ok { + t.Errorf("%s should be inherited (tier1 var)", varName) + } + } + + // Verify non-tier1 vars are NOT present + if _, ok := resultMap["SECRET_KEY"]; ok { + t.Error("SECRET_KEY should NOT be inherited (not in tier1)") + } + if _, ok := resultMap["SSH_AUTH_SOCK"]; ok { + t.Error("SSH_AUTH_SOCK should NOT be inherited (not in tier1)") + } + + // Verify explicit override is present + if resultMap["CUSTOM"] != "value" { + t.Error("CUSTOM override should be present") + } +} + +// TestBuildEnvironment_ModeTier1Tier2 tests that Tier1 + Tier2 vars are inherited +func TestBuildEnvironment_ModeTier1Tier2(t *testing.T) { + // Save and restore environment + oldEnv := os.Environ() + defer restoreEnvironment(oldEnv) + + // Set up test environment + os.Clearenv() + os.Setenv("HOME", "/home/user") + os.Setenv("PATH", "/usr/bin") + os.Setenv("SSL_CERT_FILE", "/etc/ssl/cert.pem") + os.Setenv("CURL_CA_BUNDLE", "/etc/ssl/ca-bundle.crt") + os.Setenv("SECRET_KEY", "should-not-inherit") + + // Create server config with tier1+tier2 mode + serverCfg := &config.ServerConfig{ + Inherit: &config.InheritConfig{ + Mode: config.InheritTier1Tier2, + }, + } + + // Build environment + result := BuildEnvironment(serverCfg, nil) + resultMap := sliceToMap(result) + + // Verify tier1 vars are present + if _, ok := resultMap["HOME"]; !ok { + t.Error("HOME should be inherited (tier1 var)") + } + if _, ok := resultMap["PATH"]; !ok { + t.Error("PATH should be inherited (tier1 var)") + } + + // Verify tier2 vars are present + if _, ok := resultMap["SSL_CERT_FILE"]; !ok { + t.Error("SSL_CERT_FILE should be inherited (tier2 var)") + } + if _, ok := resultMap["CURL_CA_BUNDLE"]; !ok { + t.Error("CURL_CA_BUNDLE should be inherited (tier2 var)") + } + + // Verify non-tier vars are NOT present + if _, ok := resultMap["SECRET_KEY"]; ok { + t.Error("SECRET_KEY should NOT be inherited (not in any tier)") + } +} + +// TestBuildEnvironment_ModeAll tests that mode=all includes Tier1+Tier2 and can use Extra for additional vars +func TestBuildEnvironment_ModeAll(t *testing.T) { + // Save and restore environment + oldEnv := os.Environ() + defer restoreEnvironment(oldEnv) + + // Set up test environment + os.Clearenv() + os.Setenv("HOME", "/home/user") + os.Setenv("PATH", "/usr/bin") + os.Setenv("SSL_CERT_FILE", "/etc/ssl/cert.pem") + os.Setenv("CUSTOM_VAR", "custom-value") + os.Setenv("SECRET_KEY", "secret123") + + // Create server config with mode=all + extra vars + // Note: mode=all means Tier1+Tier2, not "inherit everything" + // To inherit additional vars, use Extra list + serverCfg := &config.ServerConfig{ + Inherit: &config.InheritConfig{ + Mode: config.InheritAll, + Extra: []string{"CUSTOM_VAR", "SECRET_KEY"}, + }, + } + + // Build environment + result := BuildEnvironment(serverCfg, nil) + resultMap := sliceToMap(result) + + // Verify Tier1 vars are present + if _, ok := resultMap["HOME"]; !ok { + t.Error("HOME should be inherited (tier1 var)") + } + if _, ok := resultMap["PATH"]; !ok { + t.Error("PATH should be inherited (tier1 var)") + } + + // Verify Tier2 vars are present + if _, ok := resultMap["SSL_CERT_FILE"]; !ok { + t.Error("SSL_CERT_FILE should be inherited (tier2 var)") + } + + // Verify extra vars are present + if resultMap["CUSTOM_VAR"] != "custom-value" { + t.Error("CUSTOM_VAR should be inherited (extra var)") + } + if resultMap["SECRET_KEY"] != "secret123" { + t.Error("SECRET_KEY should be inherited (extra var)") + } +} + +// TestBuildEnvironment_ExtraVariables tests that explicitly requested vars are added +func TestBuildEnvironment_ExtraVariables(t *testing.T) { + // Save and restore environment + oldEnv := os.Environ() + defer restoreEnvironment(oldEnv) + + // Set up test environment + os.Clearenv() + os.Setenv("HOME", "/home/user") + os.Setenv("PATH", "/usr/bin") + os.Setenv("PYTHONPATH", "/opt/python/lib") + os.Setenv("VIRTUAL_ENV", "/opt/venv") + + // Create server config with tier1 + extras + serverCfg := &config.ServerConfig{ + Inherit: &config.InheritConfig{ + Mode: config.InheritTier1, + Extra: []string{"PYTHONPATH", "VIRTUAL_ENV"}, + }, + } + + // Build environment + result := BuildEnvironment(serverCfg, nil) + resultMap := sliceToMap(result) + + // Verify tier1 vars are present + if _, ok := resultMap["HOME"]; !ok { + t.Error("HOME should be inherited (tier1 var)") + } + + // Verify extra vars are present + if resultMap["PYTHONPATH"] != "/opt/python/lib" { + t.Error("PYTHONPATH should be inherited (extra var)") + } + if resultMap["VIRTUAL_ENV"] != "/opt/venv" { + t.Error("VIRTUAL_ENV should be inherited (extra var)") + } +} + +// TestBuildEnvironment_PrefixMatching tests that variables matching prefixes are added +func TestBuildEnvironment_PrefixMatching(t *testing.T) { + // Save and restore environment + oldEnv := os.Environ() + defer restoreEnvironment(oldEnv) + + // Set up test environment + os.Clearenv() + os.Setenv("HOME", "/home/user") + os.Setenv("DATTO_API_KEY", "key123") + os.Setenv("DATTO_API_URL", "https://api.datto.com") + os.Setenv("DATTO_DEBUG", "true") + os.Setenv("OTHER_VAR", "should-not-match") + + // Create server config with tier1 + DATTO_ prefix + serverCfg := &config.ServerConfig{ + Inherit: &config.InheritConfig{ + Mode: config.InheritTier1, + Prefix: []string{"DATTO_"}, + }, + } + + // Build environment + result := BuildEnvironment(serverCfg, nil) + resultMap := sliceToMap(result) + + // Verify tier1 vars are present + if _, ok := resultMap["HOME"]; !ok { + t.Error("HOME should be inherited (tier1 var)") + } + + // Verify all DATTO_ prefixed vars are present + if resultMap["DATTO_API_KEY"] != "key123" { + t.Error("DATTO_API_KEY should be inherited (prefix match)") + } + if resultMap["DATTO_API_URL"] != "https://api.datto.com" { + t.Error("DATTO_API_URL should be inherited (prefix match)") + } + if resultMap["DATTO_DEBUG"] != "true" { + t.Error("DATTO_DEBUG should be inherited (prefix match)") + } + + // Verify non-matching vars are NOT present + if _, ok := resultMap["OTHER_VAR"]; ok { + t.Error("OTHER_VAR should NOT be inherited (no prefix match)") + } +} + +// TestBuildEnvironment_Denylist tests that denied vars are blocked +func TestBuildEnvironment_Denylist(t *testing.T) { + // Save and restore environment + oldEnv := os.Environ() + defer restoreEnvironment(oldEnv) + + // Set up test environment + os.Clearenv() + os.Setenv("HOME", "/home/user") + os.Setenv("PATH", "/usr/bin") + os.Setenv("SSH_AUTH_SOCK", "/tmp/ssh-agent") + + // Create server config with mode=all + deny SSH_AUTH_SOCK + serverCfg := &config.ServerConfig{ + Inherit: &config.InheritConfig{ + Mode: config.InheritAll, + Deny: []string{"SSH_AUTH_SOCK"}, + }, + } + + // Build environment + result := BuildEnvironment(serverCfg, nil) + resultMap := sliceToMap(result) + + // Verify allowed vars are present + if _, ok := resultMap["HOME"]; !ok { + t.Error("HOME should be inherited") + } + if _, ok := resultMap["PATH"]; !ok { + t.Error("PATH should be inherited") + } + + // Verify denied var is NOT present + if _, ok := resultMap["SSH_AUTH_SOCK"]; ok { + t.Error("SSH_AUTH_SOCK should be denied") + } +} + +// TestBuildEnvironment_AllowDeniedIfExplicit tests that explicitly requested vars override denylist +func TestBuildEnvironment_AllowDeniedIfExplicit(t *testing.T) { + // Save and restore environment + oldEnv := os.Environ() + defer restoreEnvironment(oldEnv) + + // Set up test environment + os.Clearenv() + os.Setenv("HOME", "/home/user") + os.Setenv("SSH_AUTH_SOCK", "/tmp/ssh-agent") + os.Setenv("HTTP_PROXY", "http://proxy:8080") + + // Create server config with deny + allow_denied_if_explicit + extra + serverCfg := &config.ServerConfig{ + Inherit: &config.InheritConfig{ + Mode: config.InheritTier1, + Extra: []string{"SSH_AUTH_SOCK", "HTTP_PROXY"}, + Deny: []string{"SSH_AUTH_SOCK"}, + AllowDeniedIfExplicit: true, + }, + } + + // Build environment + result := BuildEnvironment(serverCfg, nil) + resultMap := sliceToMap(result) + + // Verify denied but explicitly requested var IS present (because allow_denied_if_explicit=true) + if resultMap["SSH_AUTH_SOCK"] != "/tmp/ssh-agent" { + t.Error("SSH_AUTH_SOCK should be allowed (explicitly requested + allow_denied_if_explicit)") + } + + // Verify HTTP_PROXY is also allowed (in implicit denylist + Extra list + allow_denied_if_explicit) + if resultMap["HTTP_PROXY"] != "http://proxy:8080" { + t.Error("HTTP_PROXY should be allowed (implicit denylist but in Extra + allow_denied_if_explicit)") + } +} + +// TestBuildEnvironment_HTTPProxyBlocked tests that uppercase HTTP_PROXY is blocked by default +func TestBuildEnvironment_HTTPProxyBlocked(t *testing.T) { + // Save and restore environment + oldEnv := os.Environ() + defer restoreEnvironment(oldEnv) + + // Set up test environment + os.Clearenv() + os.Setenv("HOME", "/home/user") + os.Setenv("HTTP_PROXY", "http://proxy:8080") + os.Setenv("HTTPS_PROXY", "https://proxy:8080") + os.Setenv("http_proxy", "http://proxy:8080") + os.Setenv("https_proxy", "https://proxy:8080") + + // Create server config with mode=all (should still block HTTP_PROXY variants) + serverCfg := &config.ServerConfig{ + Inherit: &config.InheritConfig{ + Mode: config.InheritAll, + }, + } + + // Build environment + result := BuildEnvironment(serverCfg, nil) + resultMap := sliceToMap(result) + + // Verify HTTP_PROXY variants are blocked + proxyVars := []string{"HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"} + for _, varName := range proxyVars { + if _, ok := resultMap[varName]; ok { + t.Errorf("%s should be blocked (implicit denylist)", varName) + } + } + + // Verify other vars still work + if _, ok := resultMap["HOME"]; !ok { + t.Error("HOME should be inherited") + } +} + +// TestBuildEnvironment_OverridePrecedence tests that explicit overrides win over inherited +func TestBuildEnvironment_OverridePrecedence(t *testing.T) { + // Save and restore environment + oldEnv := os.Environ() + defer restoreEnvironment(oldEnv) + + // Set up test environment + os.Clearenv() + os.Setenv("HOME", "/home/user") + os.Setenv("PATH", "/usr/bin") + os.Setenv("CUSTOM", "parent-value") + + // Create server config with tier1 mode and explicit overrides + serverCfg := &config.ServerConfig{ + Inherit: &config.InheritConfig{ + Mode: config.InheritTier1, + }, + Env: map[string]string{ + "PATH": "/custom/bin", + "CUSTOM": "override-value", + }, + } + + // Build environment + result := BuildEnvironment(serverCfg, nil) + resultMap := sliceToMap(result) + + // Verify overrides win + if resultMap["PATH"] != "/custom/bin" { + t.Errorf("PATH override should win, got %q", resultMap["PATH"]) + } + if resultMap["CUSTOM"] != "override-value" { + t.Errorf("CUSTOM override should win, got %q", resultMap["CUSTOM"]) + } + + // Verify overrides bypass denylist + os.Setenv("HTTP_PROXY", "http://parent:8080") + serverCfg.Env["HTTP_PROXY"] = "http://override:8080" + + result = BuildEnvironment(serverCfg, nil) + resultMap = sliceToMap(result) + + if resultMap["HTTP_PROXY"] != "http://override:8080" { + t.Errorf("HTTP_PROXY override should bypass denylist, got %q", resultMap["HTTP_PROXY"]) + } +} + +// TestBuildEnvironment_Windows tests Windows case-insensitive behavior +func TestBuildEnvironment_Windows(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("Skipping Windows-specific test") + } + + // Save and restore environment + oldEnv := os.Environ() + defer restoreEnvironment(oldEnv) + + // Set up test environment + os.Clearenv() + os.Setenv("PATH", "C:\\Windows\\System32") + os.Setenv("HOME", "C:\\Users\\test") + + // Create server config with case-variant override + serverCfg := &config.ServerConfig{ + Inherit: &config.InheritConfig{ + Mode: config.InheritTier1, + }, + Env: map[string]string{ + "Path": "C:\\Custom\\Path", // Different case + }, + } + + // Build environment + result := BuildEnvironment(serverCfg, nil) + resultMap := sliceToMap(result) + + // On Windows, PATH and Path should be treated as the same variable + pathValue := "" + pathKey := "" + for key, val := range resultMap { + if strings.ToUpper(key) == "PATH" { + pathValue = val + pathKey = key + break + } + } + + if pathValue != "C:\\Custom\\Path" { + t.Errorf("Expected PATH to be overridden (case-insensitive), got key=%q value=%q", pathKey, pathValue) + } + + // Verify only one PATH variant exists + pathCount := 0 + for key := range resultMap { + if strings.ToUpper(key) == "PATH" { + pathCount++ + } + } + if pathCount != 1 { + t.Errorf("Expected exactly 1 PATH variant, got %d: %v", pathCount, resultMap) + } + + // HOME should still be present + if _, ok := resultMap["HOME"]; !ok { + t.Error("HOME should be inherited") + } +} + +// TestBuildEnvironment_LocaleVariables tests that LC_* locale vars can be inherited via prefix +func TestBuildEnvironment_LocaleVariables(t *testing.T) { + // Save and restore environment + oldEnv := os.Environ() + defer restoreEnvironment(oldEnv) + + // Set up test environment with various LC_* vars + os.Clearenv() + os.Setenv("HOME", "/home/user") + os.Setenv("LANG", "en_US.UTF-8") + os.Setenv("LC_ALL", "en_US.UTF-8") + os.Setenv("LC_TIME", "en_GB.UTF-8") + os.Setenv("LC_NUMERIC", "de_DE.UTF-8") + os.Setenv("LC_MONETARY", "fr_FR.UTF-8") + os.Setenv("OTHER_VAR", "should-not-match") + + // Create server config with tier1 mode + LC_ prefix to capture all locale vars + serverCfg := &config.ServerConfig{ + Inherit: &config.InheritConfig{ + Mode: config.InheritTier1, + Prefix: []string{"LC_"}, + }, + } + + // Build environment + result := BuildEnvironment(serverCfg, nil) + resultMap := sliceToMap(result) + + // Verify base locale vars are present (LANG and LC_ALL are in Tier1) + if resultMap["LANG"] != "en_US.UTF-8" { + t.Error("LANG should be inherited (tier1 var)") + } + if resultMap["LC_ALL"] != "en_US.UTF-8" { + t.Error("LC_ALL should be inherited (tier1 var)") + } + + // Verify all LC_* vars are present (via prefix matching) + localeVars := []string{"LC_TIME", "LC_NUMERIC", "LC_MONETARY"} + for _, varName := range localeVars { + if _, ok := resultMap[varName]; !ok { + t.Errorf("%s should be inherited (LC_* prefix match)", varName) + } + } + + // Verify non-LC_* var is NOT present + if _, ok := resultMap["OTHER_VAR"]; ok { + t.Error("OTHER_VAR should NOT be inherited") + } +} + +// restoreEnvironment restores the environment to a previous state +func restoreEnvironment(oldEnv []string) { + os.Clearenv() + for _, entry := range oldEnv { + key, value := splitEnvEntry(entry) + if key != "" { + os.Setenv(key, value) + } + } +} diff --git a/client/stdio_client.go b/client/stdio_client.go index fd40dd2..a21a79b 100644 --- a/client/stdio_client.go +++ b/client/stdio_client.go @@ -6,9 +6,12 @@ import ( "encoding/json" "fmt" "io" + "log" "os/exec" "sync" "time" + + "mcp-debug/config" ) // StdioClient implements MCPClient using stdio transport @@ -17,15 +20,17 @@ type StdioClient struct { command string args []string env []string - + inheritCfg *config.InheritConfig // NEW: inheritance configuration + cmd *exec.Cmd stdin io.WriteCloser stdout io.ReadCloser reader *bufio.Reader idGen *RequestIDGenerator - + connected bool mu sync.Mutex + requestMu sync.Mutex // Serialize all I/O operations } // NewStdioClient creates a new stdio-based MCP client @@ -43,6 +48,11 @@ func (c *StdioClient) SetEnvironment(env []string) { c.env = env } +// SetInheritConfig sets the inheritance configuration for environment variables +func (c *StdioClient) SetInheritConfig(cfg *config.InheritConfig) { + c.inheritCfg = cfg +} + // Connect establishes connection to the MCP server func (c *StdioClient) Connect(ctx context.Context) error { c.mu.Lock() @@ -54,9 +64,28 @@ func (c *StdioClient) Connect(ctx context.Context) error { // Create command c.cmd = exec.CommandContext(ctx, c.command, c.args...) - if c.env != nil { - c.cmd.Env = c.env + if c.env != nil || c.inheritCfg != nil { + // Convert []string env to map[string]string for overrides + overrides := make(map[string]string) + if c.env != nil { + for _, entry := range c.env { + key, value := splitEnvEntry(entry) + if key != "" { + overrides[key] = value + } + } + } + + // Build a minimal ServerConfig with environment overrides and inheritance config + serverConfig := &config.ServerConfig{ + Env: overrides, + Inherit: c.inheritCfg, + } + + // BuildEnvironment handles defaulting to tier1 if Inherit is nil + c.cmd.Env = BuildEnvironment(serverConfig, nil) } + // Note: When both c.env and c.inheritCfg are nil, c.cmd.Env stays nil (Go's default) // Create pipes stdin, err := c.cmd.StdinPipe() @@ -79,14 +108,20 @@ func (c *StdioClient) Connect(ctx context.Context) error { stdout.Close() return fmt.Errorf("failed to start MCP server: %w", err) } - + c.connected = true + log.Printf("[DEBUG] StdioClient.Connect() SUCCESS: %s - connected=%v", c.serverName, c.connected) return nil } // Initialize performs MCP protocol handshake func (c *StdioClient) Initialize(ctx context.Context) (*InitializeResult, error) { - if !c.connected { + // Check connected state with proper mutex + c.mu.Lock() + connected := c.connected + c.mu.Unlock() + + if !connected { return nil, fmt.Errorf("client not connected") } @@ -110,7 +145,12 @@ func (c *StdioClient) Initialize(ctx context.Context) (*InitializeResult, error) // ListTools discovers available tools from the server func (c *StdioClient) ListTools(ctx context.Context) ([]ToolInfo, error) { - if !c.connected { + // Check connected state with proper mutex + c.mu.Lock() + connected := c.connected + c.mu.Unlock() + + if !connected { return nil, fmt.Errorf("client not connected") } @@ -136,7 +176,15 @@ func (c *StdioClient) ListTools(ctx context.Context) ([]ToolInfo, error) { // CallTool invokes a specific tool with arguments func (c *StdioClient) CallTool(ctx context.Context, name string, args map[string]interface{}) (*CallToolResult, error) { - if !c.connected { + // Check connected state with proper mutex + c.mu.Lock() + connected := c.connected + c.mu.Unlock() + + log.Printf("[DEBUG] CallTool(%s, %s): connected=%v", c.serverName, name, connected) + + if !connected { + log.Printf("[DEBUG] CallTool(%s, %s): FAILED - client not connected", c.serverName, name) return nil, fmt.Errorf("client not connected") } @@ -193,13 +241,14 @@ func (c *StdioClient) Close() error { // Process kill is expected to cause exit error, so ignore } } - + c.connected = false - + log.Printf("[DEBUG] StdioClient.Close(): %s - connected=%v", c.serverName, c.connected) + if len(errs) > 0 { return fmt.Errorf("errors during close: %v", errs) } - + return nil } @@ -217,57 +266,50 @@ func (c *StdioClient) IsConnected() bool { // sendRequest sends a JSON-RPC request and waits for response func (c *StdioClient) sendRequest(ctx context.Context, request *JSONRPCRequest) (*JSONRPCResponse, error) { + // Check connected state with proper mutex + c.mu.Lock() + connected := c.connected + c.mu.Unlock() + + if !connected { + return nil, fmt.Errorf("client not connected") + } + + // Serialize all I/O operations + c.requestMu.Lock() + defer c.requestMu.Unlock() + // Set timeout for the request ctx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() - + // Serialize request requestBytes, err := json.Marshal(request) if err != nil { return nil, fmt.Errorf("failed to marshal request: %w", err) } - - // Send request + + // Send request - now protected by mutex requestLine := append(requestBytes, '\n') if _, err := c.stdin.Write(requestLine); err != nil { return nil, fmt.Errorf("failed to write request: %w", err) } - - // Read response with timeout - responseChan := make(chan *JSONRPCResponse, 1) - errorChan := make(chan error, 1) - - go func() { - // Read response line - responseLine, err := c.reader.ReadBytes('\n') - if err != nil { - errorChan <- fmt.Errorf("failed to read response: %w", err) - return - } - - // Parse response - var response JSONRPCResponse - if err := json.Unmarshal(responseLine, &response); err != nil { - errorChan <- fmt.Errorf("failed to unmarshal response: %w", err) - return - } - - // Verify response ID matches request ID - if response.ID != request.ID { - errorChan <- fmt.Errorf("response ID %d does not match request ID %d", response.ID, request.ID) - return - } - - responseChan <- &response - }() - - // Wait for response or timeout - select { - case response := <-responseChan: - return response, nil - case err := <-errorChan: - return nil, err - case <-ctx.Done(): - return nil, fmt.Errorf("request timeout: %w", ctx.Err()) + + // Read response - now protected by mutex + responseLine, err := c.reader.ReadBytes('\n') + if err != nil { + return nil, fmt.Errorf("failed to read response: %w", err) + } + + // Parse and validate response + var response JSONRPCResponse + if err := json.Unmarshal(responseLine, &response); err != nil { + return nil, fmt.Errorf("failed to unmarshal response: %w", err) } + + if response.ID != request.ID { + return nil, fmt.Errorf("response ID mismatch: expected %d, got %d", request.ID, response.ID) + } + + return &response, nil } \ No newline at end of file diff --git a/config/types.go b/config/types.go index 815b7a4..590811d 100644 --- a/config/types.go +++ b/config/types.go @@ -7,23 +7,44 @@ import ( "time" ) +// InheritMode defines how environment variables are inherited +type InheritMode string + +const ( + InheritNone InheritMode = "none" + InheritTier1 InheritMode = "tier1" + InheritTier1Tier2 InheritMode = "tier1+tier2" + InheritAll InheritMode = "all" +) + +// InheritConfig controls which environment variables are inherited +type InheritConfig struct { + Mode InheritMode `yaml:"mode,omitempty"` + Extra []string `yaml:"extra,omitempty"` + Prefix []string `yaml:"prefix,omitempty"` + Deny []string `yaml:"deny,omitempty"` + AllowDeniedIfExplicit bool `yaml:"allow_denied_if_explicit,omitempty"` +} + // ProxyConfig represents the main configuration for the proxy server type ProxyConfig struct { Servers []ServerConfig `yaml:"servers"` Proxy ProxySettings `yaml:"proxy"` + Inherit *InheritConfig `yaml:"inherit,omitempty"` // NEW: proxy-level defaults } // ServerConfig represents configuration for a remote MCP server type ServerConfig struct { - Name string `yaml:"name"` - Prefix string `yaml:"prefix"` - Transport string `yaml:"transport"` - Command string `yaml:"command,omitempty"` - Args []string `yaml:"args,omitempty"` + Name string `yaml:"name"` + Prefix string `yaml:"prefix"` + Transport string `yaml:"transport"` + Command string `yaml:"command,omitempty"` + Args []string `yaml:"args,omitempty"` Env map[string]string `yaml:"env,omitempty"` - URL string `yaml:"url,omitempty"` - Auth *AuthConfig `yaml:"auth,omitempty"` - Timeout string `yaml:"timeout,omitempty"` + Inherit *InheritConfig `yaml:"inherit,omitempty"` // NEW: per-server inheritance + URL string `yaml:"url,omitempty"` + Auth *AuthConfig `yaml:"auth,omitempty"` + Timeout string `yaml:"timeout,omitempty"` } // AuthConfig represents authentication configuration @@ -93,8 +114,15 @@ func (c *ProxyConfig) Validate() error { return fmt.Errorf("server %s: invalid timeout format: %w", server.Name, err) } } + + // Validate server-level inherit config + if server.Inherit != nil { + if err := server.Inherit.Validate(); err != nil { + return fmt.Errorf("server %s: inherit: %w", server.Name, err) + } + } } - + // Validate proxy settings if c.Proxy.HealthCheckInterval != "" { if _, err := time.ParseDuration(c.Proxy.HealthCheckInterval); err != nil { @@ -107,37 +135,69 @@ func (c *ProxyConfig) Validate() error { return fmt.Errorf("invalid connectionTimeout format: %w", err) } } - + + // Validate proxy-level inherit config + if c.Inherit != nil { + if err := c.Inherit.Validate(); err != nil { + return fmt.Errorf("proxy.inherit: %w", err) + } + } + return nil } // ExpandEnvVars expands environment variables in configuration values func (c *ProxyConfig) ExpandEnvVars() { + // Expand proxy-level inheritance config + expandInheritConfig(c.Inherit) + for i := range c.Servers { server := &c.Servers[i] - + // Expand command server.Command = expandEnvVar(server.Command) - + // Expand args for j := range server.Args { server.Args[j] = expandEnvVar(server.Args[j]) } - + // Expand environment variables for key, value := range server.Env { server.Env[key] = expandEnvVar(value) } - + // Expand URL server.URL = expandEnvVar(server.URL) - + // Expand auth fields if server.Auth != nil { server.Auth.Token = expandEnvVar(server.Auth.Token) server.Auth.Username = expandEnvVar(server.Auth.Username) server.Auth.Password = expandEnvVar(server.Auth.Password) } + + // Expand server-level inheritance config + expandInheritConfig(server.Inherit) + } +} + +// expandInheritConfig expands environment variables in InheritConfig fields +func expandInheritConfig(ic *InheritConfig) { + if ic == nil { + return + } + + for i := range ic.Extra { + ic.Extra[i] = expandEnvVar(ic.Extra[i]) + } + + for i := range ic.Prefix { + ic.Prefix[i] = expandEnvVar(ic.Prefix[i]) + } + + for i := range ic.Deny { + ic.Deny[i] = expandEnvVar(ic.Deny[i]) } } @@ -172,7 +232,7 @@ func (s *ServerConfig) GetServerTimeout() time.Duration { // GetProxySettings returns proxy settings with defaults func (c *ProxyConfig) GetProxySettings() ProxySettings { settings := c.Proxy - + // Apply defaults if settings.HealthCheckInterval == "" { settings.HealthCheckInterval = "30s" @@ -183,6 +243,36 @@ func (c *ProxyConfig) GetProxySettings() ProxySettings { if settings.MaxRetries == 0 { settings.MaxRetries = 3 } - + return settings +} + +// ResolveInheritConfig returns the effective inheritance config for a server. +// Server-level config overrides proxy-level defaults. +func (s *ServerConfig) ResolveInheritConfig(proxyDefault *InheritConfig) *InheritConfig { + if s.Inherit != nil { + return s.Inherit + } + if proxyDefault != nil { + return proxyDefault + } + // Hardcoded default: tier1 mode + return &InheritConfig{ + Mode: InheritTier1, + } +} + +// Validate checks that the inheritance configuration is valid +func (ic *InheritConfig) Validate() error { + // Validate mode + switch ic.Mode { + case "", InheritNone, InheritTier1, InheritTier1Tier2, InheritAll: + // Valid modes (empty defaults to tier1) + default: + return fmt.Errorf("invalid mode %q: must be one of: none, tier1, tier1+tier2, all", ic.Mode) + } + + // Note: mode=none with extras/prefix is valid (inherit nothing except explicitly requested vars) + + return nil } \ No newline at end of file diff --git a/discovery/discoverer.go b/discovery/discoverer.go index 3a56610..bfcc6f1 100644 --- a/discovery/discoverer.go +++ b/discovery/discoverer.go @@ -126,7 +126,11 @@ func (d *Discoverer) discoverServer(ctx context.Context, serverConfig config.Ser // createStdioClient creates a stdio-based MCP client func (d *Discoverer) createStdioClient(serverConfig config.ServerConfig) (client.MCPClient, error) { stdioClient := client.NewStdioClient(serverConfig.Name, serverConfig.Command, serverConfig.Args) - + + // Set inheritance config + inheritCfg := serverConfig.ResolveInheritConfig(d.config.Inherit) + stdioClient.SetInheritConfig(inheritCfg) + // Set environment variables if specified if len(serverConfig.Env) > 0 { var env []string diff --git a/docs/RECORDING.md b/docs/RECORDING.md new file mode 100644 index 0000000..39f3ec7 --- /dev/null +++ b/docs/RECORDING.md @@ -0,0 +1,416 @@ +# Session Recording & Playback + +MCP Debug provides comprehensive recording of JSON-RPC traffic in proxy mode, enabling debugging, documentation, and regression testing workflows. + +## Overview + +When running in proxy mode with the `--record` flag, mcp-debug captures: +- All tool call requests and responses +- Management tool operations (server_add, server_remove, etc.) +- Tool calls to all servers (both static and dynamically added) +- Error responses and connection failures +- Complete JSON-RPC message payloads + +## Quick Start + +```bash +# Start proxy with recording enabled +mcp-debug --proxy --config config.yaml --record session.jsonl + +# Use the proxy normally - all traffic is recorded +# Stop the proxy when done (Ctrl+C) + +# Examine the recording +cat session.jsonl +``` + +## Recording Metadata in Tool Responses + +When recording is enabled, mcp-debug automatically adds metadata to all tool responses informing you that the session is being recorded: + +``` +📹 Recording: session.jsonl + Full path: /mnt/c/dev/projects/github/mcp-debug/session.jsonl + Purpose: JSON-RPC message log for debugging and playback testing +``` + +This metadata: +- Appears in all tool responses (static, dynamic, and management tools) +- Includes both the relative path (as specified) and absolute path +- Is added to the MCP response Content array (protocol-compliant) +- Only appears when `--record` flag is used +- Does not affect recording format or playback functionality + +The metadata is purely informational and can be safely ignored by automation tools. + +## Recording Format + +Recordings use **JSONL** (JSON Lines) format - one JSON object per line: + +```jsonl +# MCP Recording Session +# Started: 2026-01-12T23:44:33-07:00 +{"start_time":"2026-01-12T23:44:33.862903809-07:00","server_info":"Dynamic MCP Proxy v1.0.0","messages":[]} +{"timestamp":"2026-01-12T23:45:42.940680618-07:00","direction":"request","message_type":"tool_call","tool_name":"fs_read_file","server_name":"filesystem","message":{...}} +{"timestamp":"2026-01-12T23:45:43.123456789-07:00","direction":"response","message_type":"tool_call","tool_name":"fs_read_file","server_name":"filesystem","message":{...}} +``` + +### File Structure + +1. **Comment Lines** (lines 1-2): Human-readable session metadata +2. **Session Header** (line 3): JSON object with session information +3. **Message Lines** (line 4+): One JSON object per message + +### Session Header + +```json +{ + "start_time": "2026-01-12T23:44:33.862903809-07:00", + "server_info": "Dynamic MCP Proxy v1.0.0", + "messages": [] +} +``` + +Fields: +- `start_time`: ISO 8601 timestamp when recording started +- `server_info`: Proxy version information +- `messages`: Always empty array (messages stored as separate lines) + +### Message Format + +Each recorded message is a JSON object with these fields: + +```json +{ + "timestamp": "2026-01-12T23:45:42.940680618-07:00", + "direction": "request", + "message_type": "tool_call", + "tool_name": "fs_read_file", + "server_name": "filesystem", + "message": { + "method": "tools/call", + "params": { + "name": "fs_read_file", + "arguments": { + "path": "/etc/hosts" + } + } + } +} +``` + +Fields: +- `timestamp`: ISO 8601 timestamp when message was captured +- `direction`: Either `"request"` or `"response"` +- `message_type`: Type of message (currently always `"tool_call"`) +- `tool_name`: Prefixed tool name (e.g., `fs_read_file`, `math_calculate`) +- `server_name`: Name of the upstream MCP server +- `message`: Complete JSON-RPC message payload + +## What Gets Recorded + +### Static Servers (from config.yaml) + +All tool calls to servers defined in your configuration are recorded: + +```yaml +servers: + - name: "filesystem" + prefix: "fs" + command: "npx -y @mcp/filesystem /path" +``` + +Tool calls like `fs_read_file`, `fs_write_file`, etc. are captured. + +### Dynamic Servers (via server_add) + +Servers added at runtime are also recorded: + +```json +// server_add request is recorded +{ + "direction": "request", + "tool_name": "server_add", + "server_name": "proxy", + "message": { + "name": "database", + "command": "python3 db_server.py" + } +} + +// Subsequent tool calls to the new server are recorded +{ + "direction": "request", + "tool_name": "database_query", + "server_name": "database", + ... +} +``` + +### Management Tools + +All proxy management operations are recorded: +- `server_add` - Adding new servers +- `server_remove` - Removing servers +- `server_disconnect` - Disconnecting servers +- `server_reconnect` - Reconnecting with new commands +- `server_list` - Listing server status + +### Error Responses + +Failed requests and error responses are recorded: + +```json +{ + "direction": "response", + "tool_name": "fs_read_file", + "message": { + "content": [{ + "type": "text", + "text": "Error: File not found" + }], + "isError": true + } +} +``` + +## Playback Modes + +### Client Mode + +Replay recorded requests to test a server: + +```bash +# Replay requests from recording to a server +mcp-debug --playback-client session.jsonl | ./your-mcp-server + +# The server receives the recorded requests and can respond +``` + +**Use Cases**: +- Testing server implementations against real traffic +- Regression testing (compare responses to recorded ones) +- Debugging server behavior with specific requests + +### Server Mode + +Replay recorded responses to test a client: + +```bash +# Start playback server (use with mcp-tui or other clients) +mcp-tui mcp-debug --playback-server session.jsonl +``` + +**Use Cases**: +- Testing client behavior with known responses +- Simulating server responses without running real servers +- UI/integration testing + +## Common Workflows + +### Debugging Tool Calls + +1. Record a session where you encounter an issue: + ```bash + mcp-debug --proxy --config config.yaml --record debug-session.jsonl + ``` + +2. Examine the recording to see exact requests/responses: + ```bash + # Pretty-print messages + grep '"direction":"request"' debug-session.jsonl | jq . + + # See all tool names used + grep -o '"tool_name":"[^"]*"' debug-session.jsonl | sort | uniq + + # Extract specific tool's messages + grep '"tool_name":"fs_read_file"' debug-session.jsonl | jq . + ``` + +3. Identify the problematic request and inspect the payload + +### Regression Testing + +1. Record a "golden" session with correct behavior: + ```bash + mcp-debug --proxy --config config.yaml --record golden.jsonl + ``` + +2. After making changes, record a new session: + ```bash + mcp-debug --proxy --config config.yaml --record test.jsonl + ``` + +3. Compare recordings: + ```bash + # Extract and compare responses + grep '"direction":"response"' golden.jsonl > golden-responses.jsonl + grep '"direction":"response"' test.jsonl > test-responses.jsonl + diff golden-responses.jsonl test-responses.jsonl + ``` + +### Documentation & Examples + +1. Record typical usage patterns: + ```bash + mcp-debug --proxy --config config.yaml --record examples.jsonl + ``` + +2. Extract example requests for documentation: + ```bash + # Get all unique tool calls + jq -r 'select(.direction=="request") | .tool_name' examples.jsonl | sort | uniq + + # Extract example for specific tool + jq 'select(.tool_name=="fs_read_file" and .direction=="request") | .message.params.arguments' examples.jsonl | head -1 + ``` + +## Tips & Best Practices + +### File Naming + +Use descriptive names for recordings: +```bash +--record 2026-01-12-filesystem-operations.jsonl +--record user-authentication-flow.jsonl +--record error-case-missing-file.jsonl +``` + +### Filtering Messages + +Use `jq` to filter and analyze recordings: + +```bash +# Count messages by direction +jq -s 'group_by(.direction) | map({direction: .[0].direction, count: length})' session.jsonl + +# List all servers used +jq -r '.server_name' session.jsonl | sort | uniq + +# Find slow operations (>1 second between request/response) +# (requires custom scripting to correlate timestamps) + +# Extract error responses +jq 'select(.message.isError == true)' session.jsonl +``` + +### Recording Size + +Recordings can grow large with many messages or large payloads: + +```bash +# Check recording size +ls -lh session.jsonl + +# Count messages +grep -c '"direction":' session.jsonl + +# Compress old recordings +gzip session-2026-01-01.jsonl +``` + +### Sensitive Data + +⚠️ **Warning**: Recordings contain complete message payloads including: +- File paths and contents +- API keys or credentials passed as arguments +- Database query results +- Any data transmitted through the proxy + +**Best Practices**: +- Don't commit recordings to version control unless sanitized +- Use `.gitignore` to exclude `*.jsonl` files +- Redact sensitive data before sharing recordings +- Store recordings securely + +### Continuous Recording + +Enable automatic recording with environment variable: + +```bash +export MCP_RECORD_FILE="session.jsonl" +mcp-debug --proxy --config config.yaml +``` + +This records all sessions to the specified file (overwrites on each run). + +## Limitations + +### Current Limitations + +- **MCP Protocol Messages**: Only tool calls are recorded. MCP protocol messages (initialize, tools/list, ping) are not yet captured. +- **Multiple Formats**: Playback client/server modes have separate implementations and may not support all recorded message types. +- **No Filtering**: All messages are recorded; cannot exclude specific tools or servers. +- **Overwrite Mode**: Recording always overwrites the target file; no append mode. + +### Future Enhancements + +Planned improvements: +- Record full MCP protocol layer (initialize, notifications, etc.) +- Selective recording (filter by server, tool, or pattern) +- Append mode for long-running sessions +- Recording analysis tools (stats, timeline, visualization) +- Built-in diff/comparison tools + +## Troubleshooting + +### No Messages Recorded + +If recording file is created but contains no messages: + +1. **Verify recording is enabled**: + ```bash + # Should see "Recording enabled to: session.jsonl" in logs + mcp-debug --proxy --config config.yaml --record session.jsonl --log /dev/stdout + ``` + +2. **Check if tools are registered**: + ```bash + # Should see "Registered tool: server_tool_name" for each tool + ``` + +3. **Verify tool calls are being made**: + - Connect with mcp-tui and try calling tools + - Check proxy logs for tool call activity + +### Invalid JSON in Recording + +If parsing fails: + +```bash +# Find invalid lines +jq . session.jsonl 2>&1 | grep -A2 "parse error" + +# Validate each line +awk 'NR > 3' session.jsonl | while read line; do echo "$line" | jq . >/dev/null || echo "Line $NR invalid"; done +``` + +### Playback Failures + +If playback doesn't work as expected: + +1. **Verify recording format**: + ```bash + # Header should be single-line JSON + sed -n '3p' session.jsonl | jq . + + # Messages should have required fields + sed -n '4p' session.jsonl | jq '{direction, tool_name, message}' + ``` + +2. **Check server compatibility**: + - Ensure server expects JSON-RPC format + - Verify tool names match server's registered tools + +## Related Documentation + +- [README.md](../README.md) - Main project documentation +- [Configuration Guide](../README.md#configuration) - Server configuration +- [Environment Variables](../README.md#environment-variables) - Recording environment settings + +## Support + +For issues with recording: +1. Check the [GitHub Issues](https://github.com/ExactDoug/mcp-debug/issues) +2. Enable debug logging: `MCP_DEBUG=1 mcp-debug ...` +3. Include recording file samples (with sensitive data redacted) when reporting issues diff --git a/docs/debugging-plan.md b/docs/debugging-plan.md new file mode 100644 index 0000000..bea7a84 --- /dev/null +++ b/docs/debugging-plan.md @@ -0,0 +1,255 @@ +# Debugging Plan: "client not connected" After server_reconnect + +## Problem Statement + +After calling `server_reconnect`, the proxy says "Server now connected and tools updated", but subsequent tool calls fail with "[datto-rmm] client not connected" error. + +## Evidence from Logs + +**Timeline from mcp-session.jsonl**: +``` +04:14:32.190 - server_reconnect REQUEST +04:14:33.083 - server_reconnect RESPONSE: "Reconnected... Server now connected" +04:14:36.557 - drmm_account_devices REQUEST (3 seconds later) +04:14:36.562 - drmm_account_devices RESPONSE: ERROR "[datto-rmm] client not connected" +``` + +**Timeline from mcp-debug.log**: +``` +04:14:32.196 - Reconnecting server 'datto-rmm' with STORED configuration +04:14:33.082 - Added client 'datto-rmm' to proxy server's client list +04:14:33.083 - Server 'datto-rmm' marked as connected +``` + +## Root Cause Hypothesis + +The `server_reconnect` handler says the server is connected, but the StdioClient's internal `c.connected` flag is `false`. This suggests: + +1. **Race condition**: The `c.connected` flag is being set/read without proper synchronization +2. **State inconsistency**: `serverInfo.IsConnected` (wrapper) is true but `client.connected` (internal) is false +3. **Client replacement issue**: The new client isn't properly initialized + +## Bug Found: CallTool Doesn't Hold Mutex + +**Location**: `client/stdio_client.go:169` + +```go +func (c *StdioClient) CallTool(...) (*CallToolResult, error) { + if !c.connected { // ❌ NO MUTEX - reading without lock! + return nil, fmt.Errorf("client not connected") + } + ... +} +``` + +**Problem**: `CallTool` checks `c.connected` without holding `c.mu`, so it can read stale/incorrect values. + +**Compare with Initialize()** (line 117): +```go +func (c *StdioClient) Initialize(...) (*InitializeResult, error) { + if !c.connected { // ❌ ALSO NO MUTEX + return nil, fmt.Errorf("client not connected") + } + ... +} +``` + +Both `CallTool` and `Initialize` check `connected` without the mutex, while: +- `Connect()` sets `connected=true` under `c.mu.Lock()` (line 111) +- `Close()` sets `connected=false` under `c.mu.Lock()` (line 235) +- `sendRequest()` checks `connected` under `c.mu.Lock()` (lines 249-253) + +## Implementation Plan + +### Step 1: Add Debug Logging (IN PROGRESS) + +Add logging to track `connected` flag state transitions: + +**File**: `client/stdio_client.go` + +1. ✅ Log when Connect() sets connected=true (line 111) +2. ✅ Log when CallTool checks connected (line 169) +3. ⏳ Log when Close() sets connected=false (line 235) + +### Step 2: Fix CallTool Mutex Bug (✅ DONE) + +Fixed CallTool to read `connected` under mutex protection (matching the pattern in sendRequest). + +### Step 3: Fix Initialize Mutex Bug (PENDING) + +Need to also fix Initialize() to read `connected` under mutex: + +```go +func (c *StdioClient) Initialize(ctx context.Context) (*InitializeResult, error) { + // Check connected state with proper mutex + c.mu.Lock() + connected := c.connected + c.mu.Unlock() + + if !connected { + return nil, fmt.Errorf("client not connected") + } + ... +} +``` + +### Step 4: Fix ListTools Mutex Bug (PENDING) + +Check if ListTools also has the same issue (line 141): + +```go +func (c *StdioClient) ListTools(ctx context.Context) ([]ToolInfo, error) { + if !c.connected { // ❌ PROBABLY NO MUTEX + return nil, fmt.Errorf("client not connected") + } + ... +} +``` + +## Testing Plan + +### Test 1: Add Debug Logging and Rebuild + +```bash +# Build with debug logging +go build -o ./bin/mcp-debug . + +# Kill existing instances +pkill -f mcp-debug + +# Start new instance (Claude Code will auto-restart) +# Then test disconnect/reconnect cycle + +# Watch logs in real-time +tail -f /mnt/c/dev/projects/github/datto_rmm_smart_mcp/mcp-debug.log +``` + +**Expected output** from debug logs: +``` +[DEBUG] StdioClient.Connect() SUCCESS: datto-rmm - connected=true +[DEBUG] CallTool(datto-rmm, drmm_account_devices): connected=true +``` + +**If bug is confirmed**, we'll see: +``` +[DEBUG] StdioClient.Connect() SUCCESS: datto-rmm - connected=true +[DEBUG] CallTool(datto-rmm, drmm_account_devices): connected=false ← WRONG! +``` + +### Test 2: After Fixing Mutexes + +```bash +# Test disconnect/reconnect cycle +1. Make tool call - should work +2. server_disconnect +3. server_reconnect +4. Make tool call - should work WITHOUT errors +5. Repeat 10 times +``` + +### Test 3: Race Detection + +```bash +# Run with race detector +go test -race ./... +``` + +## Success Criteria + +- ✅ No "[datto-rmm] client not connected" errors after successful reconnect +- ✅ Debug logs show connected=true when CallTool is invoked +- ✅ No data races detected by race detector +- ✅ Disconnect/reconnect cycle works reliably + +## Files to Modify + +1. `client/stdio_client.go`: + - ✅ Add mutex protection to CallTool (line 169) + - ⏳ Add mutex protection to Initialize (line 117) + - ⏳ Add mutex protection to ListTools (line 141) + - ⏳ Add debug logging to Connect, CallTool, Close + +## Root Cause Analysis + +The fundamental issue is **inconsistent mutex usage** for the `connected` field: + +- **Protected** (correct): Connect(), Close(), sendRequest() +- **Unprotected** (BUG): CallTool(), Initialize(), ListTools() + +This creates race conditions where: +1. Thread A: Connect() sets connected=true, releases mutex +2. Thread B: CallTool() reads connected (no mutex) → may read stale false value +3. Tool call fails even though client is actually connected + +The fix is to make ALL accesses to `connected` use the mutex, matching the pattern already used in sendRequest(). + +## UPDATE: Mutex Fixes Completed But Issue Persists + +**Status**: All mutex protection and debug logging has been implemented (commit 81c9f04), but tool calls still fail after server_reconnect. + +**New Debug Evidence** (from user testing after mutex fixes): +``` +04:33:54 - [DEBUG] StdioClient.Connect() SUCCESS: datto-rmm - connected=true ← NEW CLIENT +04:34:57 - [DEBUG] CallTool(datto-rmm, account_devices): connected=false ← OLD CLIENT +``` + +**Critical Discovery**: The debug logs prove that CallTool() is being invoked on the OLD disconnected client, not the NEW connected client created by server_reconnect. + +## Actual Root Cause: Handler Closure Captures Stale Client Reference + +The mutex hypothesis was incorrect. The real bug is in how tool handlers are registered: + +### Static Servers (BROKEN) +Static servers from config.yaml use `proxy.CreateProxyHandler()` which captures the mcpClient reference in a closure at initialization time: + +**File**: `integration/proxy_server.go` lines 102-115 +```go +for _, tool := range result.Tools { + // Create proxy handler - CAPTURES mcpClient IN CLOSURE + handler := proxy.CreateProxyHandler(mcpClient, tool, ...) + p.mcpServer.AddTool(mcpTool, handler) +} +``` + +**File**: `proxy/handler.go` line 20 +```go +func CreateProxyHandler(mcpClient client.MCPClient, ...) { + return func(...) { + // mcpClient is CAPTURED in closure - IMMUTABLE + result, err := mcpClient.CallTool(...) + } +} +``` + +When server_reconnect creates a new client, the handler closure still references the old captured client. + +### Dynamic Servers (WORKING) +Dynamic servers from server_add use `createDynamicProxyHandler()` which looks up the current client at call time: + +**File**: `integration/dynamic_wrapper.go` lines 768-780 +```go +func (w *DynamicWrapper) createDynamicProxyHandler(serverName, originalToolName string) { + return func(...) { + // Looks up CURRENT client at call time + w.mu.RLock() + serverInfo := w.dynamicServers[serverName] + client := serverInfo.Client // Gets current client + w.mu.RUnlock() + + client.CallTool(...) // Uses current client + } +} +``` + +This pattern doesn't capture the client, so reconnect automatically provides the new client. + +## Solution + +Make static servers use the dynamic handler pattern: +1. Remove static handler creation from proxy_server.go (lines 102-115) +2. Add dynamic handler creation for all tools in dynamic_wrapper.go Start() +3. All handlers will look up current client from dynamicServers map at call time + +This eliminates closure-captured client references and enables hot-swapping for all servers. + +**Next Commit**: Implement handler pattern fix (see plan in .claude/plans/quizzical-questing-phoenix.md) diff --git a/examples/config-inheritance-advanced.yaml b/examples/config-inheritance-advanced.yaml new file mode 100644 index 0000000..be76fe0 --- /dev/null +++ b/examples/config-inheritance-advanced.yaml @@ -0,0 +1,178 @@ +# Advanced Environment Inheritance Example +# DRAFT - Not yet tested with real-world MCP servers +# +# This example demonstrates advanced features including: +# - mode: all (inherit everything with deny lists) +# - allow_denied_if_explicit (override implicit denylist) +# - Proxy-level defaults with per-server overrides +# - HTTP_PROXY override (httpoxy mitigation bypass) +# +# See ../DRAFT_ENV_INHERITANCE.md for complete documentation. + +# Proxy-level defaults +proxy: + healthCheckInterval: "30s" + connectionTimeout: "10s" + maxRetries: 3 + + # Default inheritance for all servers + inherit: + mode: tier1 # Conservative default + deny: + # Block these variables for ALL servers by default + - SSH_AUTH_SOCK + - AWS_SESSION_TOKEN + # Servers can override this entire config + +servers: + # Example 1: Trusted in-house server with mode: all + - name: trusted-internal-server + transport: stdio + command: ./internal-server + inherit: + mode: all # Inherit everything from parent + deny: + # Even in mode:all, explicitly block these secrets + - AWS_SECRET_ACCESS_KEY + - AZURE_CLIENT_SECRET + - GITHUB_TOKEN + - ANTHROPIC_API_KEY + - OPENAI_API_KEY + # + # Inherited variables: + # - Everything in parent environment + # - EXCEPT the five denied credentials above + # - EXCEPT proxy-level denies (SSH_AUTH_SOCK, AWS_SESSION_TOKEN) + # - EXCEPT implicit denylist (HTTP_PROXY, https_proxy, etc.) + # + # Use case: Fully trusted internal server that needs broad environment + # access but should never see production API keys + + # Example 2: Corporate proxy server with httpoxy override + - name: corporate-proxy-client + transport: stdio + command: python3 + args: ["-m", "proxy_aware_server"] + inherit: + mode: tier1+tier2 + extra: + # Request lowercase proxy variables (safer than uppercase) + - http_proxy + - https_proxy + - no_proxy + allow_denied_if_explicit: true # Allow proxy vars from implicit denylist + # + # Inherited variables: + # - All tier1 + tier2 variables + # - http_proxy, https_proxy, no_proxy (overriding implicit denylist) + # + # Security note: We explicitly request lowercase variants (safer) and + # set allow_denied_if_explicit to bypass the implicit denylist. + # Only do this if you trust the server and need proxy support. + # + # The uppercase variants (HTTP_PROXY, HTTPS_PROXY) remain blocked + # unless explicitly added to 'extra' list. + # + # Use case: Corporate environment requiring HTTP proxy for outbound + # connections, with trusted MCP server + + # Example 3: Multi-tenant server with complete isolation + - name: tenant-a-server + transport: stdio + command: python3 + args: ["-m", "multitenant_server"] + inherit: + mode: none # No inheritance at all + env: + # Only these explicitly defined variables are set + TENANT_ID: "tenant-a" + DB_HOST: "tenant-a-db.internal" + DB_NAME: "tenant_a_production" + DB_USER: "tenant_a_user" + DB_PASSWORD: "${TENANT_A_DB_PASSWORD}" # Expanded from parent + ISOLATION_LEVEL: "maximum" + # + # Inherited variables: + # - NONE from parent environment + # - Only the env: block values above + # + # Use case: Multi-tenant architecture requiring complete environment + # isolation between tenants + + # Example 4: Development server with relaxed inheritance + - name: dev-server + transport: stdio + command: python3 + args: ["-m", "dev_server"] + inherit: + mode: all # Inherit everything (convenient for development) + deny: [] # Don't deny anything (override proxy defaults) + allow_denied_if_explicit: true + env: + ENVIRONMENT: "development" + DEBUG: "true" + # + # Inherited variables: + # - Everything in parent environment + # - Including SSH_AUTH_SOCK (proxy deny overridden) + # - Including proxy variables (implicit denylist overridden) + # + # Use case: Local development where you want maximum convenience + # and aren't worried about secret leakage + # + # WARNING: Never use this configuration for untrusted/experimental + # servers or in production environments + + # Example 5: Minimal server using proxy defaults + - name: default-behavior-server + transport: stdio + command: python3 + args: ["-m", "basic_server"] + # No inherit block = uses proxy.inherit defaults + # + # Inherited variables: + # - Tier1 (from proxy default mode: tier1) + # - NOT SSH_AUTH_SOCK (from proxy deny list) + # - NOT AWS_SESSION_TOKEN (from proxy deny list) + # + # Use case: Standard server using secure defaults + + # Example 6: Override proxy defaults for specific server + - name: override-proxy-defaults + transport: stdio + command: node + args: ["special-server.js"] + inherit: + mode: tier1+tier2 # Override proxy's tier1 + extra: + - NODE_ENV + - SSH_AUTH_SOCK # Include despite proxy deny + deny: + - GITHUB_TOKEN # Add additional denial + allow_denied_if_explicit: true # Allow SSH_AUTH_SOCK from extra + # + # Inherited variables: + # - Tier1 + tier2 (overriding proxy default) + # - SSH_AUTH_SOCK (explicitly allowed despite proxy deny) + # - NODE_ENV + # - NOT GITHUB_TOKEN (explicitly denied) + # - NOT AWS_SESSION_TOKEN (still from proxy deny) + # + # Use case: Server needs SSH for git operations but should never + # see GitHub token (use SSH key instead) + + # Example 7: Testing/debugging server that prints environment + - name: env-inspector + transport: stdio + command: /usr/bin/env # Prints all environment variables + inherit: + mode: tier1 + extra: ["TEST_VAR1", "TEST_VAR2"] + # + # Use this server to verify what variables are actually inherited. + # The /usr/bin/env command prints all environment variables it receives. + # + # Run: uvx mcp-debug --proxy --config this-file.yaml + # Then use server_list or server tools to see output + # + # Use case: Debugging inheritance configuration diff --git a/examples/config-inheritance-basic.yaml b/examples/config-inheritance-basic.yaml new file mode 100644 index 0000000..393820a --- /dev/null +++ b/examples/config-inheritance-basic.yaml @@ -0,0 +1,65 @@ +# Basic Environment Inheritance Example +# DRAFT - Not yet tested with real-world MCP servers +# +# This example demonstrates the default tier1 mode, which inherits only +# baseline environment variables necessary for most programs to function. +# +# Security: This is the most secure default mode, preventing leakage of +# credentials, tokens, and other sensitive environment variables. +# +# See ../DRAFT_ENV_INHERITANCE.md for complete documentation. + +servers: + # Example 1: Default behavior (tier1 implicit) + - name: basic-python-server + transport: stdio + command: python3 + args: ["-m", "my_python_mcp_server"] + # No inherit block specified = tier1 mode (default) + # + # Inherited variables: + # - PATH, HOME, USER, SHELL (system basics) + # - LANG, LC_ALL, TZ (locale/timezone) + # - TMPDIR, TEMP, TMP (temporary directories) + # + # NOT inherited: + # - AWS_ACCESS_KEY_ID, GITHUB_TOKEN (credentials) + # - SSH_AUTH_SOCK (SSH agent) + # - Any custom application variables + + # Example 2: Explicit tier1 mode with minimal extras + - name: python-with-path + transport: stdio + command: python3 + args: ["-m", "another_server"] + inherit: + mode: tier1 # Explicit (same as default) + extra: ["PYTHONPATH"] # Add one additional variable + # + # Inherited variables: + # - All tier1 variables (PATH, HOME, USER, etc.) + # - PYTHONPATH (explicitly added) + # + # Use case: Python server needs custom module path + + # Example 3: Explicit environment overrides + - name: configured-server + transport: stdio + command: node + args: ["server.js"] + inherit: + mode: tier1 + env: + # These explicit overrides are ALWAYS set, regardless of inheritance mode + NODE_ENV: "production" + LOG_LEVEL: "info" + # Expand from parent environment + API_KEY: "${MY_API_KEY}" + # + # Note: env: overrides are never blocked by deny lists + # They always win over inherited variables + +proxy: + healthCheckInterval: "30s" + connectionTimeout: "10s" + maxRetries: 3 diff --git a/examples/config-inheritance-none.yaml b/examples/config-inheritance-none.yaml new file mode 100644 index 0000000..6bea6a8 --- /dev/null +++ b/examples/config-inheritance-none.yaml @@ -0,0 +1,171 @@ +# Minimal Environment Inheritance Example (mode: none) +# DRAFT - Not yet tested with real-world MCP servers +# +# This example demonstrates mode: none, which provides maximum isolation +# by inheriting no environment variables automatically. Only explicitly +# defined variables in the env: block are passed to the server. +# +# Security: This is the most secure mode, providing complete control over +# what environment the server sees. Use for untrusted servers or when you +# need guaranteed isolation. +# +# See ../DRAFT_ENV_INHERITANCE.md for complete documentation. + +servers: + # Example 1: Complete isolation with explicit configuration + - name: isolated-server + transport: stdio + command: python3 + args: ["-m", "untrusted_mcp_server"] + inherit: + mode: none # No automatic inheritance + env: + # Only these variables will be set in the server environment + PATH: "/usr/local/bin:/usr/bin:/bin" + HOME: "/tmp/isolated-home" + PYTHONPATH: "/opt/mcp-servers/lib" + LOG_LEVEL: "warning" + ENVIRONMENT: "isolated" + # + # Inherited variables: + # - ONLY the five variables defined in env: above + # - Nothing from parent environment + # + # Use case: Maximum security for untrusted or experimental servers + + # Example 2: Minimal with explicit extras (still isolated from most vars) + - name: semi-isolated-server + transport: stdio + command: node + args: ["server.js"] + inherit: + mode: none + extra: + # These specific variables will be inherited IF they exist + - PATH + - HOME + - NODE_ENV + env: + # Explicit overrides (always set) + SERVICE_NAME: "isolated-node-server" + PORT: "3000" + # + # Inherited variables: + # - PATH, HOME, NODE_ENV (if they exist in parent) + # - SERVICE_NAME, PORT (explicit overrides) + # - Nothing else from parent environment + # + # Note: This is similar to tier1, but you have explicit control + # over exactly which variables are allowed + + # Example 3: Multi-tenant isolation + - name: tenant-a + transport: stdio + command: python3 + args: ["-m", "tenant_server", "--tenant=a"] + inherit: + mode: none + env: + TENANT_ID: "a" + DB_URL: "postgresql://tenant-a-db/production" + STORAGE_PATH: "/data/tenant-a" + API_KEY: "${TENANT_A_API_KEY}" # Expanded from parent + MAX_CONNECTIONS: "10" + # + # Inherited variables: + # - Only the env: block above + # - TENANT_A_API_KEY is expanded from parent but not directly inherited + # + # Use case: Multi-tenant architecture where tenants must be completely + # isolated from each other's configuration + + - name: tenant-b + transport: stdio + command: python3 + args: ["-m", "tenant_server", "--tenant=b"] + inherit: + mode: none + env: + TENANT_ID: "b" + DB_URL: "postgresql://tenant-b-db/production" + STORAGE_PATH: "/data/tenant-b" + API_KEY: "${TENANT_B_API_KEY}" + MAX_CONNECTIONS: "10" + # + # Tenant B has identical structure but different configuration. + # Complete isolation ensures tenant A cannot see tenant B's credentials. + + # Example 4: Testing server with controlled environment + - name: test-server + transport: stdio + command: python3 + args: ["-m", "pytest", "-v"] + inherit: + mode: none + env: + # Minimal testing environment + PATH: "/usr/local/bin:/usr/bin:/bin" + HOME: "/tmp/test-home" + PYTHONPATH: "/workspace/tests" + PYTEST_CURRENT_TEST: "true" + TEST_ENVIRONMENT: "isolated" + # No access to production credentials or secrets + # + # Use case: Running tests in isolated environment to ensure they + # don't depend on local environment variables + + # Example 5: Containerized server simulation + - name: container-like-server + transport: stdio + command: python3 + args: ["-m", "containerized_server"] + inherit: + mode: none + extra: + # Minimal set similar to container defaults + - PATH + - HOME + env: + # Container-like environment + HOSTNAME: "mcp-server-container" + ENVIRONMENT: "production" + LOG_FORMAT: "json" + TZ: "UTC" + # + # Inherited variables: + # - PATH, HOME (if present) + # - HOSTNAME, ENVIRONMENT, LOG_FORMAT, TZ (explicit) + # + # Use case: Simulating a containerized environment where the server + # gets a minimal, controlled environment + + # Example 6: Mode none but allow denied if explicit + - name: isolated-with-escape-hatch + transport: stdio + command: python3 + args: ["-m", "special_server"] + inherit: + mode: none + extra: + - PATH + - HOME + - http_proxy # Normally in implicit denylist + - https_proxy + allow_denied_if_explicit: true # Allow proxy vars despite denylist + env: + CONFIG_FILE: "/etc/special-server/config.yaml" + # + # Inherited variables: + # - PATH, HOME (if present) + # - http_proxy, https_proxy (if present, despite denylist) + # - CONFIG_FILE (explicit) + # + # Use case: Isolated server that needs proxy access but nothing else + +proxy: + healthCheckInterval: "30s" + connectionTimeout: "10s" + maxRetries: 3 + +# Note: No proxy.inherit block specified. In mode: none, this doesn't +# matter since servers explicitly disable all inheritance anyway. diff --git a/examples/config-inheritance-tier2.yaml b/examples/config-inheritance-tier2.yaml new file mode 100644 index 0000000..f6ab881 --- /dev/null +++ b/examples/config-inheritance-tier2.yaml @@ -0,0 +1,103 @@ +# Tier1+Tier2 Environment Inheritance Example +# DRAFT - Not yet tested with real-world MCP servers +# +# This example demonstrates tier1+tier2 mode, which adds network and TLS +# variables to the baseline. Useful for servers making HTTPS requests, +# especially in enterprise environments with custom CA certificates. +# +# See ../DRAFT_ENV_INHERITANCE.md for complete documentation. + +servers: + # Example 1: API server with TLS support + - name: api-client-server + transport: stdio + command: python3 + args: ["-m", "api_mcp_server"] + inherit: + mode: tier1+tier2 # Baseline + network/TLS variables + extra: + - PYTHONPATH + - VIRTUAL_ENV + # + # Inherited variables: + # Tier 1: + # - PATH, HOME, USER, SHELL + # - LANG, LC_ALL, TZ + # - TMPDIR, TEMP, TMP + # Tier 2: + # - SSL_CERT_FILE, SSL_CERT_DIR (system TLS certs) + # - REQUESTS_CA_BUNDLE (Python requests library) + # - CURL_CA_BUNDLE (curl) + # - NODE_EXTRA_CA_CERTS (Node.js) + # Extra: + # - PYTHONPATH, VIRTUAL_ENV + # + # Use case: Python server making HTTPS API calls in corporate network + # with custom CA certificates for TLS inspection + + # Example 2: Application-specific variables with prefix matching + - name: datto-rmm-server + transport: stdio + command: python3 + args: ["-m", "datto_rmm_mcp.server"] + inherit: + mode: tier1+tier2 + prefix: + - DATTO_ # Inherit all DATTO_* variables + - RMM_ # Inherit all RMM_* variables + extra: + - PYTHONPATH + deny: + - SSH_AUTH_SOCK # Block SSH even if somehow matched + # + # Inherited variables: + # - All tier1 + tier2 variables + # - All variables starting with DATTO_ (e.g., DATTO_API_KEY, DATTO_URL) + # - All variables starting with RMM_ + # - PYTHONPATH + # - NOT SSH_AUTH_SOCK (explicitly denied) + # + # Use case: Application with multiple related environment variables + # that share a common prefix + + # Example 3: Node.js server in enterprise environment + - name: node-https-server + transport: stdio + command: node + args: ["enterprise-server.js"] + inherit: + mode: tier1+tier2 + extra: + - NODE_ENV + - NODE_OPTIONS + - NPM_CONFIG_REGISTRY # Corporate npm registry + deny: + - AWS_ACCESS_KEY_ID # Prevent cloud credential leakage + - AWS_SECRET_ACCESS_KEY + - GITHUB_TOKEN + env: + # Explicit configuration + SERVICE_NAME: "mcp-node-server" + LOG_LEVEL: "info" + # + # Inherited variables: + # - All tier1 + tier2 (including NODE_EXTRA_CA_CERTS) + # - NODE_ENV, NODE_OPTIONS, NPM_CONFIG_REGISTRY + # - NOT AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, GITHUB_TOKEN + # - SERVICE_NAME, LOG_LEVEL (explicit overrides) + # + # Use case: Enterprise Node.js server with custom CA certificates, + # corporate npm registry, but blocking cloud credentials + +# Proxy-level defaults (applied to all servers unless overridden) +proxy: + healthCheckInterval: "30s" + connectionTimeout: "10s" + maxRetries: 3 + + # Optional: Set default inheritance for all servers + # inherit: + # mode: tier1 + # extra: [] + # + # Individual servers can override these defaults diff --git a/integration/dynamic_proxy_server.go b/integration/dynamic_proxy_server.go index c434d50..6bcbef7 100644 --- a/integration/dynamic_proxy_server.go +++ b/integration/dynamic_proxy_server.go @@ -27,11 +27,12 @@ type DynamicProxyServer struct { clients map[string]client.MCPClient // server name -> client serverConfigs map[string]config.ServerConfig // server name -> config toolRegistry map[string][]string // server name -> list of tool names + config *config.ProxyConfig // Full proxy config including Inherit settings mu sync.RWMutex } // NewDynamicProxyServer creates a new dynamic proxy server -func NewDynamicProxyServer(cfg *config.ProxySettings) *DynamicProxyServer { +func NewDynamicProxyServer(cfg *config.ProxyConfig) *DynamicProxyServer { // Create MCP server with stdio transport mcpServer := mcp_golang.NewServer( stdio.NewStdioServerTransport(), @@ -45,6 +46,7 @@ func NewDynamicProxyServer(cfg *config.ProxySettings) *DynamicProxyServer { clients: make(map[string]client.MCPClient), serverConfigs: make(map[string]config.ServerConfig), toolRegistry: make(map[string][]string), + config: cfg, } } @@ -327,6 +329,11 @@ func (p *DynamicProxyServer) createAndConnectClient(ctx context.Context, serverC switch serverConfig.Transport { case "stdio": stdioClient := client.NewStdioClient(serverConfig.Name, serverConfig.Command, serverConfig.Args) + + // Set inheritance config + inheritCfg := serverConfig.ResolveInheritConfig(p.config.Inherit) + stdioClient.SetInheritConfig(inheritCfg) + if serverConfig.Env != nil { // Convert map[string]string to []string envSlice := make([]string, 0, len(serverConfig.Env)) diff --git a/integration/dynamic_wrapper.go b/integration/dynamic_wrapper.go index d205059..e4b8e02 100644 --- a/integration/dynamic_wrapper.go +++ b/integration/dynamic_wrapper.go @@ -6,13 +6,14 @@ import ( "fmt" "log" "os" + "path/filepath" "strings" "sync" "time" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" - + "mcp-debug/client" "mcp-debug/config" "mcp-debug/discovery" @@ -26,9 +27,10 @@ type DynamicWrapper struct { mu sync.RWMutex // Recording functionality - recordFile *os.File - recordEnabled bool - recordMu sync.Mutex + recordFile *os.File + recordEnabled bool + recordMu sync.Mutex + recordFilename string // Path to the recording file (for metadata) } type DynamicServerInfo struct { @@ -86,7 +88,8 @@ func NewDynamicWrapper(cfg *config.ProxyConfig) *DynamicWrapper { func (w *DynamicWrapper) EnableRecording(filename string) error { w.recordMu.Lock() defer w.recordMu.Unlock() - + + if w.recordEnabled { return fmt.Errorf("recording already enabled") } @@ -95,21 +98,26 @@ func (w *DynamicWrapper) EnableRecording(filename string) error { if err != nil { return fmt.Errorf("failed to create recording file: %w", err) } - + w.recordFile = file + w.recordFilename = filename w.recordEnabled = true - + // Write session header session := RecordingSession{ StartTime: time.Now(), ServerInfo: "Dynamic MCP Proxy v1.0.0", Messages: []RecordedMessage{}, } - - headerBytes, _ := json.MarshalIndent(session, "", " ") - fmt.Fprintf(file, "# MCP Recording Session\n# Started: %s\n%s\n", + + headerBytes, _ := json.Marshal(session) + fmt.Fprintf(file, "# MCP Recording Session\n# Started: %s\n%s\n", session.StartTime.Format(time.RFC3339), string(headerBytes)) - + + // Inject recorder and metadata function into proxy server for static server recording + w.proxyServer.recorderFunc = w.recordMessage + w.proxyServer.metadataFunc = w.addRecordingMetadata + log.Printf("Recording enabled to: %s", filename) return nil } @@ -148,6 +156,51 @@ func (w *DynamicWrapper) recordMessage(direction, messageType, toolName, serverN w.recordFile.Sync() // Ensure immediate write } +// addRecordingMetadata adds recording file information to tool results when recording is active +func (w *DynamicWrapper) addRecordingMetadata(result *mcp.CallToolResult) *mcp.CallToolResult { + if !w.recordEnabled { + return result + } + + w.recordMu.Lock() + filename := w.recordFilename + w.recordMu.Unlock() + + if filename == "" { + return result + } + + // Compute absolute path + absPath, err := filepath.Abs(filename) + if err != nil { + absPath = filename // fallback to original if abs fails + } + + // Build metadata text + metadataText := fmt.Sprintf( + "📹 Recording: %s\n Full path: %s\n Purpose: JSON-RPC message log for debugging and playback testing", + filename, + absPath, + ) + + // Create metadata content item + metadataItem := mcp.NewTextContent(metadataText) + + // Copy-on-write to avoid mutating input + newResult := &mcp.CallToolResult{ + Content: make([]mcp.Content, len(result.Content), len(result.Content)+1), + IsError: result.IsError, + } + + // Copy existing content + copy(newResult.Content, result.Content) + + // Append metadata to NEW slice + newResult.Content = append(newResult.Content, metadataItem) + + return newResult +} + func (w *DynamicWrapper) registerManagementTools() { // server_add tool addTool := mcp.NewTool("server_add", @@ -195,14 +248,13 @@ func (w *DynamicWrapper) registerManagementTools() { // server_reconnect tool reconnectTool := mcp.NewTool("server_reconnect", - mcp.WithDescription("Reconnect a server with new command (use after server_disconnect)"), + mcp.WithDescription("Reconnect a server with optional new command (use after server_disconnect)"), mcp.WithString("name", mcp.Required(), mcp.Description("Name of the server to reconnect"), ), mcp.WithString("command", - mcp.Required(), - mcp.Description("New command to run (e.g., 'npx -y @modelcontextprotocol/filesystem /path')"), + mcp.Description("New command to run. If omitted, uses stored configuration from config.yaml."), ), ) @@ -216,13 +268,15 @@ func (w *DynamicWrapper) handleServerAdd(ctx context.Context, request mcp.CallTo name, err := request.RequireString("name") if err != nil { result := mcp.NewToolResultError("name is required") + result = w.addRecordingMetadata(result) w.recordMessage("response", "tool_call", "server_add", "proxy", result) return result, nil } - + command, err := request.RequireString("command") if err != nil { result := mcp.NewToolResultError("command is required") + result = w.addRecordingMetadata(result) w.recordMessage("response", "tool_call", "server_add", "proxy", result) return result, nil } @@ -232,13 +286,19 @@ func (w *DynamicWrapper) handleServerAdd(ctx context.Context, request mcp.CallTo // Check if already exists if _, exists := w.dynamicServers[name]; exists { - return mcp.NewToolResultError(fmt.Sprintf("Server '%s' already exists", name)), nil + result := mcp.NewToolResultError(fmt.Sprintf("Server '%s' already exists", name)) + result = w.addRecordingMetadata(result) + w.recordMessage("response", "tool_call", "server_add", "proxy", result) + return result, nil } - + // Parse command parts := strings.Fields(command) if len(parts) == 0 { - return mcp.NewToolResultError("Invalid command"), nil + result := mcp.NewToolResultError("Invalid command") + result = w.addRecordingMetadata(result) + w.recordMessage("response", "tool_call", "server_add", "proxy", result) + return result, nil } // Create server config @@ -253,20 +313,34 @@ func (w *DynamicWrapper) handleServerAdd(ctx context.Context, request mcp.CallTo // Create and connect client stdioClient := client.NewStdioClient(name, serverConfig.Command, serverConfig.Args) + + // Use default inheritance (tier1 or proxy defaults) + inheritCfg := serverConfig.ResolveInheritConfig(w.proxyServer.config.Inherit) + stdioClient.SetInheritConfig(inheritCfg) + if err := stdioClient.Connect(ctx); err != nil { - return mcp.NewToolResultError(fmt.Sprintf("Failed to connect: %v", err)), nil + result := mcp.NewToolResultError(fmt.Sprintf("Failed to connect: %v", err)) + result = w.addRecordingMetadata(result) + w.recordMessage("response", "tool_call", "server_add", "proxy", result) + return result, nil } - + if _, err := stdioClient.Initialize(ctx); err != nil { stdioClient.Close() - return mcp.NewToolResultError(fmt.Sprintf("Failed to initialize: %v", err)), nil + result := mcp.NewToolResultError(fmt.Sprintf("Failed to initialize: %v", err)) + result = w.addRecordingMetadata(result) + w.recordMessage("response", "tool_call", "server_add", "proxy", result) + return result, nil } - + // List tools tools, err := stdioClient.ListTools(ctx) if err != nil { stdioClient.Close() - return mcp.NewToolResultError(fmt.Sprintf("Failed to list tools: %v", err)), nil + result := mcp.NewToolResultError(fmt.Sprintf("Failed to list tools: %v", err)) + result = w.addRecordingMetadata(result) + w.recordMessage("response", "tool_call", "server_add", "proxy", result) + return result, nil } // Store server info @@ -315,24 +389,34 @@ func (w *DynamicWrapper) handleServerAdd(ctx context.Context, request mcp.CallTo result := fmt.Sprintf("Added server '%s' with command: %s %s\nRegistered %d tools successfully.", name, serverConfig.Command, strings.Join(serverConfig.Args, " "), registeredCount) - + toolResult := mcp.NewToolResultText(result) + toolResult = w.addRecordingMetadata(toolResult) w.recordMessage("response", "tool_call", "server_add", "proxy", toolResult) return toolResult, nil } func (w *DynamicWrapper) handleServerRemove(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + // Record the request + w.recordMessage("request", "tool_call", "server_remove", "proxy", request) + name, err := request.RequireString("name") if err != nil { - return mcp.NewToolResultError("name is required"), nil + result := mcp.NewToolResultError("name is required") + result = w.addRecordingMetadata(result) + w.recordMessage("response", "tool_call", "server_remove", "proxy", result) + return result, nil } - + w.mu.Lock() defer w.mu.Unlock() - + serverInfo, exists := w.dynamicServers[name] if !exists { - return mcp.NewToolResultError(fmt.Sprintf("Server '%s' not found", name)), nil + result := mcp.NewToolResultError(fmt.Sprintf("Server '%s' not found", name)) + result = w.addRecordingMetadata(result) + w.recordMessage("response", "tool_call", "server_remove", "proxy", result) + return result, nil } // Note: We can't actually remove tools from mark3labs/mcp-go at runtime @@ -357,11 +441,17 @@ func (w *DynamicWrapper) handleServerRemove(ctx context.Context, request mcp.Cal result := fmt.Sprintf("Removed server '%s'. Note: %d tools remain registered but are now unavailable.", name, len(serverInfo.Tools)) - - return mcp.NewToolResultText(result), nil + + toolResult := mcp.NewToolResultText(result) + toolResult = w.addRecordingMetadata(toolResult) + w.recordMessage("response", "tool_call", "server_remove", "proxy", toolResult) + return toolResult, nil } func (w *DynamicWrapper) handleServerList(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + // Record the request + w.recordMessage("request", "tool_call", "server_list", "proxy", request) + w.mu.RLock() defer w.mu.RUnlock() @@ -411,26 +501,41 @@ func (w *DynamicWrapper) handleServerList(ctx context.Context, request mcp.CallT totalServers := staticCount + len(w.dynamicServers) result.WriteString(fmt.Sprintf("\nTotal servers: %d (static: %d, dynamic: %d)\n", totalServers, staticCount, len(w.dynamicServers))) - - return mcp.NewToolResultText(result.String()), nil + + toolResult := mcp.NewToolResultText(result.String()) + toolResult = w.addRecordingMetadata(toolResult) + w.recordMessage("response", "tool_call", "server_list", "proxy", toolResult) + return toolResult, nil } func (w *DynamicWrapper) handleServerDisconnect(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + // Record the request + w.recordMessage("request", "tool_call", "server_disconnect", "proxy", request) + name, err := request.RequireString("name") if err != nil { - return mcp.NewToolResultError("name is required"), nil + result := mcp.NewToolResultError("name is required") + result = w.addRecordingMetadata(result) + w.recordMessage("response", "tool_call", "server_disconnect", "proxy", result) + return result, nil } - + w.mu.Lock() defer w.mu.Unlock() - + serverInfo, exists := w.dynamicServers[name] if !exists { - return mcp.NewToolResultError(fmt.Sprintf("Server '%s' not found", name)), nil + result := mcp.NewToolResultError(fmt.Sprintf("Server '%s' not found", name)) + result = w.addRecordingMetadata(result) + w.recordMessage("response", "tool_call", "server_disconnect", "proxy", result) + return result, nil } if !serverInfo.IsConnected { - return mcp.NewToolResultText(fmt.Sprintf("Server '%s' is already disconnected", name)), nil + toolResult := mcp.NewToolResultText(fmt.Sprintf("Server '%s' is already disconnected", name)) + toolResult = w.addRecordingMetadata(toolResult) + w.recordMessage("response", "tool_call", "server_disconnect", "proxy", toolResult) + return toolResult, nil } log.Printf("Disconnecting server '%s'", name) @@ -441,76 +546,141 @@ func (w *DynamicWrapper) handleServerDisconnect(ctx context.Context, request mcp if err := serverInfo.Client.Close(); err != nil { log.Printf("Error closing client %s: %v", name, err) } + + // Remove from proxy server's client list to prevent stale references + w.proxyServer.mu.Lock() + newClients := make([]client.MCPClient, 0, len(w.proxyServer.clients)-1) + for _, c := range w.proxyServer.clients { + if c.ServerName() != name { + newClients = append(newClients, c) + } + } + w.proxyServer.clients = newClients + w.proxyServer.mu.Unlock() + log.Printf("Removed client '%s' from proxy server's client list", name) } - + // Mark as disconnected but keep tools registered serverInfo.IsConnected = false serverInfo.ErrorMessage = "Server disconnected by user" serverInfo.Client = nil result := fmt.Sprintf("Disconnected server '%s'. Tools remain registered but will return errors.\\nUse server_reconnect to restore with new binary/command.", name) - return mcp.NewToolResultText(result), nil + toolResult := mcp.NewToolResultText(result) + toolResult = w.addRecordingMetadata(toolResult) + w.recordMessage("response", "tool_call", "server_disconnect", "proxy", toolResult) + return toolResult, nil } func (w *DynamicWrapper) handleServerReconnect(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + // Record the request + w.recordMessage("request", "tool_call", "server_reconnect", "proxy", request) + name, err := request.RequireString("name") if err != nil { - return mcp.NewToolResultError("name is required"), nil - } - - command, err := request.RequireString("command") - if err != nil { - return mcp.NewToolResultError("command is required"), nil + result := mcp.NewToolResultError("name is required") + result = w.addRecordingMetadata(result) + w.recordMessage("response", "tool_call", "server_reconnect", "proxy", result) + return result, nil } - + + // Get command (optional now) + commandStr := request.GetString("command", "") + w.mu.Lock() defer w.mu.Unlock() - + serverInfo, exists := w.dynamicServers[name] if !exists { - return mcp.NewToolResultError(fmt.Sprintf("Server '%s' not found", name)), nil + result := mcp.NewToolResultError(fmt.Sprintf("Server '%s' not found", name)) + result = w.addRecordingMetadata(result) + w.recordMessage("response", "tool_call", "server_reconnect", "proxy", result) + return result, nil } - + if serverInfo.IsConnected { - return mcp.NewToolResultError(fmt.Sprintf("Server '%s' is still connected. Use server_disconnect first.", name)), nil + toolResult := mcp.NewToolResultError(fmt.Sprintf("Server '%s' is still connected. Use server_disconnect first.", name)) + toolResult = w.addRecordingMetadata(toolResult) + w.recordMessage("response", "tool_call", "server_reconnect", "proxy", toolResult) + return toolResult, nil } - - log.Printf("Reconnecting server '%s' with new command: %s", name, command) - - // Parse new command - parts := strings.Fields(command) - if len(parts) == 0 { - return mcp.NewToolResultError("Invalid command"), nil - } - - // Update server config - serverConfig := config.ServerConfig{ - Name: name, - Prefix: name, - Transport: "stdio", - Command: parts[0], - Args: parts[1:], - Timeout: "30s", + + var serverConfig config.ServerConfig + + if commandStr != "" { + // Command provided: parse and create new config + log.Printf("Reconnecting server '%s' with NEW command: %s", name, commandStr) + + parts := strings.Fields(commandStr) + if len(parts) == 0 { + result := mcp.NewToolResultError("Invalid command") + result = w.addRecordingMetadata(result) + w.recordMessage("response", "tool_call", "server_reconnect", "proxy", result) + return result, nil + } + + // Create new config (preserves name/prefix, but loses env vars) + serverConfig = config.ServerConfig{ + Name: name, + Prefix: serverInfo.Config.Prefix, + Transport: "stdio", + Command: parts[0], + Args: parts[1:], + Timeout: "30s", + } + } else { + // Command omitted: use stored config + log.Printf("Reconnecting server '%s' with STORED configuration", name) + + if serverInfo.Config.Command == "" { + toolResult := mcp.NewToolResultError("Stored config has no command. Please provide command parameter.") + toolResult = w.addRecordingMetadata(toolResult) + w.recordMessage("response", "tool_call", "server_reconnect", "proxy", toolResult) + return toolResult, nil + } + + // Use stored config as-is (preserves env, inherit, timeout, etc.) + serverConfig = serverInfo.Config } - + // Create and connect new client - stdioClient := client.NewStdioClient(name, serverConfig.Command, serverConfig.Args) + stdioClient := client.NewStdioClient(serverConfig.Name, serverConfig.Command, serverConfig.Args) + + // Apply inheritance config from stored ServerConfig + inheritCfg := serverConfig.ResolveInheritConfig(w.proxyServer.config.Inherit) + stdioClient.SetInheritConfig(inheritCfg) + + // Apply environment variables from stored ServerConfig + if len(serverConfig.Env) > 0 { + var env []string + for key, value := range serverConfig.Env { + env = append(env, fmt.Sprintf("%s=%s", key, value)) + } + stdioClient.SetEnvironment(env) + } + if err := stdioClient.Connect(ctx); err != nil { // Mark as disconnected but keep tools registered serverInfo.IsConnected = false serverInfo.ErrorMessage = fmt.Sprintf("Failed to connect: %v", err) serverInfo.Config = serverConfig - return mcp.NewToolResultError(fmt.Sprintf("Failed to connect: %v", err)), nil + toolResult := mcp.NewToolResultError(fmt.Sprintf("Failed to connect: %v", err)) + toolResult = w.addRecordingMetadata(toolResult) + w.recordMessage("response", "tool_call", "server_reconnect", "proxy", toolResult) + return toolResult, nil } - + if _, err := stdioClient.Initialize(ctx); err != nil { stdioClient.Close() serverInfo.IsConnected = false serverInfo.ErrorMessage = fmt.Sprintf("Failed to initialize: %v", err) serverInfo.Config = serverConfig - return mcp.NewToolResultError(fmt.Sprintf("Failed to initialize: %v", err)), nil + toolResult := mcp.NewToolResultError(fmt.Sprintf("Failed to initialize: %v", err)) + toolResult = w.addRecordingMetadata(toolResult) + w.recordMessage("response", "tool_call", "server_reconnect", "proxy", toolResult) + return toolResult, nil } - + // List tools from new server tools, err := stdioClient.ListTools(ctx) if err != nil { @@ -518,27 +688,40 @@ func (w *DynamicWrapper) handleServerReconnect(ctx context.Context, request mcp. serverInfo.IsConnected = false serverInfo.ErrorMessage = fmt.Sprintf("Failed to list tools: %v", err) serverInfo.Config = serverConfig - return mcp.NewToolResultError(fmt.Sprintf("Failed to list tools: %v", err)), nil + toolResult := mcp.NewToolResultError(fmt.Sprintf("Failed to list tools: %v", err)) + toolResult = w.addRecordingMetadata(toolResult) + w.recordMessage("response", "tool_call", "server_reconnect", "proxy", toolResult) + return toolResult, nil } - // Update server info + // Update server info (but NOT IsConnected yet - defer until all state updated) serverInfo.Client = stdioClient serverInfo.Config = serverConfig - serverInfo.IsConnected = true serverInfo.ErrorMessage = "" - - // Update proxy server's client list + + // Update proxy server's client list with proper mutex protection + w.proxyServer.mu.Lock() + clientFound := false for i, c := range w.proxyServer.clients { if c.ServerName() == name { w.proxyServer.clients[i] = stdioClient + clientFound = true break } } - + if !clientFound { + // Client not in list (was removed by disconnect), append it + w.proxyServer.clients = append(w.proxyServer.clients, stdioClient) + log.Printf("Added client '%s' to proxy server's client list", name) + } else { + log.Printf("Updated client '%s' in proxy server's client list", name) + } + w.proxyServer.mu.Unlock() + // Update registry with new client (tools keep same names) for _, tool := range tools { prefixedName := fmt.Sprintf("%s_%s", name, tool.Name) - + // Check if this tool name exists in our registered tools found := false for _, registeredTool := range serverInfo.Tools { @@ -547,7 +730,7 @@ func (w *DynamicWrapper) handleServerReconnect(ctx context.Context, request mcp. break } } - + if found { // Update registry with new client discoveredTool := discovery.RemoteTool{ @@ -561,11 +744,24 @@ func (w *DynamicWrapper) handleServerReconnect(ctx context.Context, request mcp. log.Printf("Updated tool registration: %s", prefixedName) } } - - result := fmt.Sprintf("Reconnected server '%s' with command: %s %s\\nServer now connected and tools updated.", - name, serverConfig.Command, strings.Join(serverConfig.Args, " ")) - - return mcp.NewToolResultText(result), nil + + // NOW mark as connected (atomic state transition after all updates complete) + serverInfo.IsConnected = true + log.Printf("Server '%s' marked as connected", name) + + // Build result message based on how we reconnected + var resultMsg string + if commandStr != "" { + resultMsg = fmt.Sprintf("Reconnected server '%s' with NEW command: %s %s\nServer now connected and tools updated.", + name, serverConfig.Command, strings.Join(serverConfig.Args, " ")) + } else { + resultMsg = fmt.Sprintf("Reconnected server '%s' using STORED configuration\nServer now connected and tools updated.", name) + } + + toolResult := mcp.NewToolResultText(resultMsg) + toolResult = w.addRecordingMetadata(toolResult) + w.recordMessage("response", "tool_call", "server_reconnect", "proxy", toolResult) + return toolResult, nil } // createDynamicProxyHandler creates a handler that checks connection status @@ -574,36 +770,46 @@ func (w *DynamicWrapper) createDynamicProxyHandler(serverName, originalToolName // Record the tool call request prefixedToolName := fmt.Sprintf("%s_%s", serverName, originalToolName) w.recordMessage("request", "tool_call", prefixedToolName, serverName, request) + + // Copy client reference while holding lock to prevent use-after-free w.mu.RLock() serverInfo, exists := w.dynamicServers[serverName] + var client client.MCPClient + if exists && serverInfo.IsConnected { + client = serverInfo.Client // Copy reference + } w.mu.RUnlock() - + if !exists { result := mcp.NewToolResultError(fmt.Sprintf("Server '%s' not found", serverName)) + result = w.addRecordingMetadata(result) w.recordMessage("response", "tool_call", prefixedToolName, serverName, result) return result, nil } - - if !serverInfo.IsConnected { + + if client == nil { + // Server disconnected errorMsg := fmt.Sprintf("Server '%s' is disconnected", serverName) if serverInfo.ErrorMessage != "" { errorMsg += fmt.Sprintf(": %s", serverInfo.ErrorMessage) } errorMsg += "\nUse server_reconnect to restore connection." result := mcp.NewToolResultError(errorMsg) + result = w.addRecordingMetadata(result) w.recordMessage("response", "tool_call", prefixedToolName, serverName, result) return result, nil } - + // Extract arguments from the request args := request.GetArguments() argsMap := make(map[string]interface{}) for key, value := range args { argsMap[key] = value } - - // Forward the call to the remote server - result, err := serverInfo.Client.CallTool(ctx, originalToolName, argsMap) + + // Forward the call to the remote server using copied client reference + // (safe from concurrent disconnect) + result, err := client.CallTool(ctx, originalToolName, argsMap) if err != nil { // Mark server as disconnected on connection errors if isConnectionError(err) { @@ -611,9 +817,10 @@ func (w *DynamicWrapper) createDynamicProxyHandler(serverName, originalToolName serverInfo.IsConnected = false serverInfo.ErrorMessage = err.Error() w.mu.Unlock() - + errorMsg := fmt.Sprintf("Server '%s' connection failed: %v\nUse server_reconnect to restore connection.", serverName, err) result := mcp.NewToolResultError(errorMsg) + result = w.addRecordingMetadata(result) w.recordMessage("response", "tool_call", prefixedToolName, serverName, result) return result, nil } @@ -621,6 +828,7 @@ func (w *DynamicWrapper) createDynamicProxyHandler(serverName, originalToolName // Wrap error with server context errorMsg := fmt.Sprintf("[%s] %v", serverName, err) result := mcp.NewToolResultError(errorMsg) + result = w.addRecordingMetadata(result) w.recordMessage("response", "tool_call", prefixedToolName, serverName, result) return result, nil } @@ -648,7 +856,8 @@ func (w *DynamicWrapper) createDynamicProxyHandler(serverName, originalToolName finalResult = mcp.NewToolResultText("Tool executed successfully") } } - + + finalResult = w.addRecordingMetadata(finalResult) w.recordMessage("response", "tool_call", prefixedToolName, serverName, finalResult) return finalResult, nil } @@ -667,7 +876,113 @@ func isConnectionError(err error) bool { // Initialize initializes the proxy with static servers func (w *DynamicWrapper) Initialize(ctx context.Context) error { // Initialize the proxy server with static servers - return w.proxyServer.Initialize(ctx) + if err := w.proxyServer.Initialize(ctx); err != nil { + return err + } + + // Populate dynamicServers map with static servers + if err := w.populateStaticServers(); err != nil { + return err + } + + // Create dynamic handlers for ALL tools (including static servers) + // This allows hot-swapping to work correctly for all servers + w.createHandlersForAllTools() + + return nil +} + +// populateStaticServers adds static servers from config to dynamicServers map +func (w *DynamicWrapper) populateStaticServers() error { + w.mu.Lock() + defer w.mu.Unlock() + + // Iterate through static server configs + for _, serverConfig := range w.proxyServer.config.Servers { + // Find matching client by ServerName + var matchingClient client.MCPClient + for _, c := range w.proxyServer.clients { + if c.ServerName() == serverConfig.Name { + matchingClient = c + break + } + } + + if matchingClient != nil { + // SUCCESS: Server connected, add with tools + allTools := w.proxyServer.registry.GetAllTools() + var serverTools []string + for _, tool := range allTools { + if tool.ServerName == serverConfig.Name { + serverTools = append(serverTools, tool.PrefixedName) + } + } + + serverInfo := &DynamicServerInfo{ + Name: serverConfig.Name, + Client: matchingClient, + Config: serverConfig, + Tools: serverTools, + IsConnected: true, + ErrorMessage: "", + } + w.dynamicServers[serverConfig.Name] = serverInfo + log.Printf("Added static server '%s' to dynamic management with %d tools", + serverConfig.Name, len(serverTools)) + } else { + // FAILED: No client, but still add to enable reconnect + var errorMsg string + for _, result := range w.proxyServer.discoveryResults { + if result.ServerName == serverConfig.Name && result.Error != nil { + errorMsg = result.Error.Error() + break + } + } + if errorMsg == "" { + errorMsg = "Failed to connect during initialization" + } + + serverInfo := &DynamicServerInfo{ + Name: serverConfig.Name, + Client: nil, + Config: serverConfig, // Store config for reconnect + Tools: []string{}, + IsConnected: false, + ErrorMessage: errorMsg, + } + w.dynamicServers[serverConfig.Name] = serverInfo + log.Printf("Added static server '%s' to dynamic management (disconnected: %s)", + serverConfig.Name, errorMsg) + } + } + + return nil +} + +// createHandlersForAllTools creates dynamic handlers for all registered tools +// This allows both static and dynamic servers to use the same handler pattern +// that looks up the current client at call time, enabling hot-swapping +func (w *DynamicWrapper) createHandlersForAllTools() { + allTools := w.proxyServer.registry.GetAllTools() + + for _, tool := range allTools { + // Create MCP tool definition, preserving upstream inputSchema + description := fmt.Sprintf("[%s] %s", tool.ServerName, tool.Description) + var mcpTool mcp.Tool + if len(tool.InputSchema) > 0 { + mcpTool = mcp.NewToolWithRawSchema(tool.PrefixedName, description, tool.InputSchema) + } else { + mcpTool = mcp.NewTool(tool.PrefixedName, mcp.WithDescription(description)) + } + + // Create dynamic handler that looks up client at call time + handler := w.createDynamicProxyHandler(tool.ServerName, tool.OriginalName) + + // Register with MCP server + w.baseServer.AddTool(mcpTool, handler) + + log.Printf("Registered tool with dynamic handler: %s", tool.PrefixedName) + } } // Start starts the MCP server diff --git a/integration/proxy_server.go b/integration/proxy_server.go index 389fc32..62256f6 100644 --- a/integration/proxy_server.go +++ b/integration/proxy_server.go @@ -17,12 +17,15 @@ import ( // ProxyServer manages the complete MCP proxy server type ProxyServer struct { - config *config.ProxyConfig - mcpServer *server.MCPServer - registry *proxy.ToolRegistry - clients []client.MCPClient - discoverer *discovery.Discoverer - + config *config.ProxyConfig + mcpServer *server.MCPServer + registry *proxy.ToolRegistry + clients []client.MCPClient + discoverer *discovery.Discoverer + discoveryResults []*discovery.DiscoveryResult // Store for populateStaticServers access + recorderFunc proxy.RecorderFunc // Optional recorder for tool call traffic + metadataFunc func(*mcp.CallToolResult) *mcp.CallToolResult // Optional metadata injector + mu sync.RWMutex initialized bool } @@ -47,13 +50,16 @@ func (p *ProxyServer) Initialize(ctx context.Context) error { } log.Println("Initializing Dynamic MCP Proxy Server...") - - // Create MCP server instance - p.mcpServer = server.NewMCPServer( - "Dynamic MCP Proxy", - "1.0.0", - server.WithToolCapabilities(true), - ) + + // Create MCP server instance ONLY if one doesn't exist + // (DynamicWrapper pre-assigns this before calling Initialize) + if p.mcpServer == nil { + p.mcpServer = server.NewMCPServer( + "Dynamic MCP Proxy", + "1.0.0", + server.WithToolCapabilities(true), + ) + } // Discover tools from all configured servers log.Println("Discovering tools from remote servers...") @@ -61,7 +67,10 @@ func (p *ProxyServer) Initialize(ctx context.Context) error { if err != nil { return fmt.Errorf("failed to discover tools: %w", err) } - + + // Store results for populateStaticServers to access + p.discoveryResults = results + // Process discovery results successfulResults := discovery.GetSuccessfulResults(results) failedResults := discovery.GetFailedResults(results) @@ -89,20 +98,13 @@ func (p *ProxyServer) Initialize(ctx context.Context) error { p.clients = append(p.clients, mcpClient) - // Register tools and create handlers + // Register tools in registry for _, tool := range result.Tools { p.registry.RegisterTool(tool, mcpClient) - - // Create MCP tool definition - mcpTool := p.createMCPTool(tool) - - // Create proxy handler - handler := proxy.CreateProxyHandler(mcpClient, tool) - - // Register with MCP server - p.mcpServer.AddTool(mcpTool, handler) - - log.Printf("Registered tool: %s", tool.PrefixedName) + + // Note: Handlers will be created by DynamicWrapper using dynamic lookup pattern + // This allows hot-swapping to work correctly for static servers + log.Printf("Registered tool in registry (handler to be created by wrapper): %s", tool.PrefixedName) } } @@ -177,7 +179,11 @@ func (p *ProxyServer) createAndConnectClient(ctx context.Context, serverName str switch serverConfig.Transport { case "stdio": stdioClient := client.NewStdioClient(serverConfig.Name, serverConfig.Command, serverConfig.Args) - + + // Set inheritance config + inheritCfg := serverConfig.ResolveInheritConfig(p.config.Inherit) + stdioClient.SetInheritConfig(inheritCfg) + // Set environment variables if specified if len(serverConfig.Env) > 0 { var env []string @@ -186,7 +192,7 @@ func (p *ProxyServer) createAndConnectClient(ctx context.Context, serverName str } stdioClient.SetEnvironment(env) } - + mcpClient = stdioClient default: return nil, fmt.Errorf("unsupported transport: %s", serverConfig.Transport) @@ -207,11 +213,14 @@ func (p *ProxyServer) createAndConnectClient(ctx context.Context, serverName str // createMCPTool creates an mcp.Tool from a RemoteTool func (p *ProxyServer) createMCPTool(remoteTool discovery.RemoteTool) mcp.Tool { - // For now, create a simple tool with basic parameters - // In a full implementation, we would parse the InputSchema to create proper parameter definitions - + description := fmt.Sprintf("[%s] %s", remoteTool.ServerName, remoteTool.Description) + + if len(remoteTool.InputSchema) > 0 { + return mcp.NewToolWithRawSchema(remoteTool.PrefixedName, description, remoteTool.InputSchema) + } + return mcp.NewTool(remoteTool.PrefixedName, - mcp.WithDescription(fmt.Sprintf("[%s] %s", remoteTool.ServerName, remoteTool.Description)), + mcp.WithDescription(description), ) } diff --git a/main.go b/main.go index fb2b5fd..95c7596 100644 --- a/main.go +++ b/main.go @@ -186,19 +186,19 @@ func helloHandler(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallTo // runDynamicProxyWithManagement runs the proxy with dynamic management tools func runDynamicProxyWithManagement(configPath, recordFile string) error { ctx := context.Background() - + // Load configuration log.Printf("Loading configuration from: %s", configPath) cfg, err := config.LoadConfig(configPath) if err != nil { return fmt.Errorf("failed to load configuration: %w", err) } - + log.Printf("Configuration loaded: %d servers configured", len(cfg.Servers)) - - // Create dynamic wrapper + + // Create dynamic wrapper (uses mark3labs/mcp-go which works with stdio) wrapper := integration.NewDynamicWrapper(cfg) - + // Enable recording if specified if recordFile != "" { log.Printf("Recording JSON-RPC traffic to: %s", recordFile) @@ -206,7 +206,7 @@ func runDynamicProxyWithManagement(configPath, recordFile string) error { return fmt.Errorf("failed to enable recording: %w", err) } } - + // Initialize with static servers log.Println("Initializing proxy server...") if err := wrapper.Initialize(ctx); err != nil { @@ -216,7 +216,7 @@ func runDynamicProxyWithManagement(configPath, recordFile string) error { } log.Println("Starting with no initial servers - use server_add to add servers dynamically") } - + // Start the server return wrapper.Start() } @@ -232,9 +232,9 @@ func runDynamicProxyServer(configPath string) error { } log.Printf("Configuration loaded: %d servers configured", len(cfg.Servers)) - + // Create dynamic proxy server - proxyServer := integration.NewDynamicProxyServer(&cfg.Proxy) + proxyServer := integration.NewDynamicProxyServer(cfg) // Set up graceful shutdown ctx, cancel := context.WithCancel(context.Background()) diff --git a/mcp-debug b/mcp-debug deleted file mode 100755 index b607c17..0000000 Binary files a/mcp-debug and /dev/null differ diff --git a/proxy/handler.go b/proxy/handler.go index b932c38..05020ab 100644 --- a/proxy/handler.go +++ b/proxy/handler.go @@ -3,33 +3,69 @@ package proxy import ( "context" "fmt" - + "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" - + "mcp-debug/client" "mcp-debug/discovery" ) +// RecorderFunc is a function that records JSON-RPC messages with metadata +type RecorderFunc func(direction, messageType, toolName, serverName string, message interface{}) + // CreateProxyHandler creates a handler that forwards tool calls to remote servers -func CreateProxyHandler(mcpClient client.MCPClient, remoteTool discovery.RemoteTool) server.ToolHandlerFunc { +// The optional recorder function enables recording of tool call traffic +// The optional metadataFunc injects metadata into tool results (e.g., recording info) +func CreateProxyHandler(mcpClient client.MCPClient, remoteTool discovery.RemoteTool, recorder RecorderFunc, metadataFunc func(*mcp.CallToolResult) *mcp.CallToolResult) server.ToolHandlerFunc { return func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + // Record the request if recorder is provided + if recorder != nil { + recorder("request", "tool_call", remoteTool.PrefixedName, remoteTool.ServerName, request) + } // Extract arguments from the request args, err := extractArguments(request) if err != nil { - return mcp.NewToolResultError(fmt.Sprintf("Failed to extract arguments: %v", err)), nil + errResult := mcp.NewToolResultError(fmt.Sprintf("Failed to extract arguments: %v", err)) + // Inject metadata if function provided + if metadataFunc != nil { + errResult = metadataFunc(errResult) + } + if recorder != nil { + recorder("response", "tool_call", remoteTool.PrefixedName, remoteTool.ServerName, errResult) + } + return errResult, nil } - + // Forward the call to the remote server using the original tool name result, err := mcpClient.CallTool(ctx, remoteTool.OriginalName, args) if err != nil { // Wrap error with server context errorMsg := fmt.Sprintf("[%s] %v", remoteTool.ServerName, err) - return mcp.NewToolResultError(errorMsg), nil + errResult := mcp.NewToolResultError(errorMsg) + // Inject metadata if function provided + if metadataFunc != nil { + errResult = metadataFunc(errResult) + } + if recorder != nil { + recorder("response", "tool_call", remoteTool.PrefixedName, remoteTool.ServerName, errResult) + } + return errResult, nil } // Transform the result back to MCP format mcpResult := transformResult(result) + + // Inject metadata if function provided + if metadataFunc != nil { + mcpResult = metadataFunc(mcpResult) + } + + // Record the response if recorder is provided + if recorder != nil { + recorder("response", "tool_call", remoteTool.PrefixedName, remoteTool.ServerName, mcpResult) + } + return mcpResult, nil } } @@ -117,19 +153,19 @@ func (r *ToolRegistry) GetAllTools() []discovery.RemoteTool { } // CreateHandlerForTool creates a proxy handler for a specific tool -func (r *ToolRegistry) CreateHandlerForTool(prefixedToolName string) (server.ToolHandlerFunc, error) { +func (r *ToolRegistry) CreateHandlerForTool(prefixedToolName string, recorder RecorderFunc, metadataFunc func(*mcp.CallToolResult) *mcp.CallToolResult) (server.ToolHandlerFunc, error) { // Get tool metadata tool, exists := r.GetTool(prefixedToolName) if !exists { return nil, fmt.Errorf("tool not found: %s", prefixedToolName) } - + // Get associated client mcpClient, exists := r.GetClient(tool.ServerName) if !exists { return nil, fmt.Errorf("client not found for server: %s", tool.ServerName) } - - // Create and return the handler - return CreateProxyHandler(mcpClient, tool), nil + + // Create and return the handler with optional recorder and metadata function + return CreateProxyHandler(mcpClient, tool, recorder, metadataFunc), nil } \ No newline at end of file diff --git a/tests/config-fixtures/test-inherit-advanced.yaml b/tests/config-fixtures/test-inherit-advanced.yaml new file mode 100644 index 0000000..1655215 --- /dev/null +++ b/tests/config-fixtures/test-inherit-advanced.yaml @@ -0,0 +1,78 @@ +# DRAFT TEST FIXTURE - Environment Inheritance +# Status: Not yet validated with real-world MCP servers +# Purpose: Test proxy-level defaults with server-level overrides +# Expected behavior: Verify configuration resolution chain: +# 1. Proxy defaults apply to servers without inherit config +# 2. Servers can override proxy defaults +# 3. mode: none provides maximum isolation +# Created: 2026-01-12 +# Feature: Selective Environment Inheritance (commit 49f5581) + +proxy: + healthCheckInterval: "30s" + connectionTimeout: "10s" + maxRetries: 3 + + # Proxy-level inheritance defaults + inherit: + mode: tier1 # Default for all servers + deny: + - SSH_AUTH_SOCK # Block SSH agent by default + - AWS_SESSION_TOKEN + +servers: + # Server 1: Uses proxy defaults (no inherit block) + - name: "test-inherit-proxy-defaults" + prefix: "default" + transport: "stdio" + command: "/usr/bin/env" + timeout: "10s" + # No inherit block = uses proxy.inherit defaults + # + # Expected inherited variables: + # - Tier1 (from proxy default mode: tier1) + # - NOT SSH_AUTH_SOCK (from proxy deny list) + # - NOT AWS_SESSION_TOKEN (from proxy deny list) + # + # Use case: Verify proxy defaults are applied correctly + + # Server 2: Overrides proxy defaults with tier2 + extras + - name: "test-inherit-override-tier2" + prefix: "override" + transport: "stdio" + command: "/usr/bin/env" + timeout: "10s" + inherit: + mode: tier1+tier2 # Override proxy's tier1 + extra: + - PYTHONPATH + - NODE_ENV + deny: + - GITHUB_TOKEN # Add server-specific denial + # + # Expected inherited variables: + # - Tier1 + tier2 (overriding proxy default) + # - PYTHONPATH, NODE_ENV (extras) + # - NOT GITHUB_TOKEN (server-level deny) + # - NOT SSH_AUTH_SOCK, AWS_SESSION_TOKEN (proxy-level deny still applies) + # + # Use case: Verify server config overrides proxy defaults + + # Server 3: Maximum isolation with mode: none + - name: "test-inherit-isolation" + prefix: "isolated" + transport: "stdio" + command: "/usr/bin/env" + timeout: "10s" + inherit: + mode: none # No inheritance at all + env: + # Only these explicit variables are set + ISOLATION_TEST: "true" + SERVER_NAME: "test-inherit-isolation" + # + # Expected inherited variables: + # - NONE from parent environment + # - Only env: block values (ISOLATION_TEST, SERVER_NAME) + # + # Use case: Verify mode: none provides complete isolation diff --git a/tests/config-fixtures/test-inherit-denylist-override.yaml b/tests/config-fixtures/test-inherit-denylist-override.yaml new file mode 100644 index 0000000..0c2ebf6 --- /dev/null +++ b/tests/config-fixtures/test-inherit-denylist-override.yaml @@ -0,0 +1,65 @@ +# DRAFT TEST FIXTURE - Environment Inheritance +# Status: Not yet validated with real-world MCP servers +# Purpose: Test allow_denied_if_explicit flag +# Expected behavior: Explicitly requested variables in 'extra' can override +# deny list when allow_denied_if_explicit is true +# Created: 2026-01-12 +# Feature: Selective Environment Inheritance (commit 49f5581) + +servers: + # Test allow_denied_if_explicit flag with explicit override + - name: "test-inherit-allow-denied" + prefix: "allowdeny" + transport: "stdio" + command: "/usr/bin/env" # Prints environment to verify inheritance + timeout: "10s" + inherit: + mode: tier1 # Baseline variables only + extra: + - HTTP_PROXY # Normally in implicit denylist + - CUSTOM_SECRET # Also in explicit deny list below + deny: + - CUSTOM_SECRET # Explicitly deny this variable + - AWS_ACCESS_KEY_ID + allow_denied_if_explicit: true # Allow extras to override denies + # + # Expected inherited variables: + # - All tier1 variables (PATH, HOME, USER, etc.) + # - HTTP_PROXY (overriding implicit denylist via allow_denied_if_explicit) + # - CUSTOM_SECRET (overriding explicit deny via allow_denied_if_explicit) + # - NOT AWS_ACCESS_KEY_ID (denied but not in extra list) + # + # Security note: This flag should only be used when you trust the server + # and need specific variables from the deny lists (like proxy vars). + # + # Use case: Corporate proxy server that needs HTTP_PROXY but should + # otherwise follow security best practices + + # Control test: Same config without allow_denied_if_explicit + - name: "test-inherit-deny-enforced" + prefix: "denyenforce" + transport: "stdio" + command: "/usr/bin/env" + timeout: "10s" + inherit: + mode: tier1 + extra: + - HTTP_PROXY # Normally in implicit denylist + - CUSTOM_SECRET # Also in explicit deny list + deny: + - CUSTOM_SECRET + - AWS_ACCESS_KEY_ID + # allow_denied_if_explicit: false (default) + # + # Expected inherited variables: + # - All tier1 variables (PATH, HOME, USER, etc.) + # - NOT HTTP_PROXY (implicit denylist blocks it) + # - NOT CUSTOM_SECRET (explicit deny blocks it) + # - NOT AWS_ACCESS_KEY_ID (explicitly denied) + # + # Use case: Verify that without the flag, deny lists are enforced + +proxy: + healthCheckInterval: "30s" + connectionTimeout: "10s" + maxRetries: 3 diff --git a/tests/config-fixtures/test-inherit-tier1.yaml b/tests/config-fixtures/test-inherit-tier1.yaml new file mode 100644 index 0000000..d56a402 --- /dev/null +++ b/tests/config-fixtures/test-inherit-tier1.yaml @@ -0,0 +1,50 @@ +# DRAFT TEST FIXTURE - Environment Inheritance +# Status: Not yet validated with real-world MCP servers +# Purpose: Test basic tier1 mode (default behavior) +# Expected behavior: Server should inherit only baseline tier1 variables +# - PATH, HOME, USER, SHELL, LOGNAME +# - LANG, LC_ALL, LC_*, TZ +# - TMPDIR, TEMP, TMP +# Created: 2026-01-12 +# Feature: Selective Environment Inheritance (commit 49f5581) + +servers: + # Test tier1 default inheritance (implicit mode) + - name: "test-inherit-tier1-implicit" + prefix: "tier1" + transport: "stdio" + command: "/usr/bin/env" # Prints environment to verify inheritance + timeout: "10s" + # No inherit block specified = tier1 mode (default) + # + # Expected inherited variables: + # - System basics: PATH, HOME, USER, SHELL, LOGNAME + # - Locale/timezone: LANG, LC_ALL, LC_*, TZ + # - Temp directories: TMPDIR, TEMP, TMP + # + # Expected NOT inherited: + # - Credentials: AWS_*, GITHUB_TOKEN, ANTHROPIC_API_KEY, etc. + # - SSH: SSH_AUTH_SOCK, SSH_AGENT_PID + # - Network: HTTP_PROXY, http_proxy, no_proxy + # - Custom application variables + + # Test explicit tier1 mode with single extra variable + - name: "test-inherit-tier1-explicit" + prefix: "tier1ex" + transport: "stdio" + command: "/usr/bin/env" + timeout: "10s" + inherit: + mode: tier1 # Explicit specification + extra: ["PYTHONPATH"] # Add one additional variable + # + # Expected inherited variables: + # - All tier1 variables (same as above) + # - PYTHONPATH (if set in parent environment) + # + # Use case: Verify explicit tier1 mode works identically to implicit + +proxy: + healthCheckInterval: "30s" + connectionTimeout: "10s" + maxRetries: 3 diff --git a/tests/config-fixtures/test-inherit-tier2.yaml b/tests/config-fixtures/test-inherit-tier2.yaml new file mode 100644 index 0000000..2770f7a --- /dev/null +++ b/tests/config-fixtures/test-inherit-tier2.yaml @@ -0,0 +1,51 @@ +# DRAFT TEST FIXTURE - Environment Inheritance +# Status: Not yet validated with real-world MCP servers +# Purpose: Test tier1+tier2 mode with extras, prefix, and deny list +# Expected behavior: Server should inherit tier1 + tier2 variables, plus: +# - Extra variables: PYTHONPATH, VIRTUAL_ENV +# - Variables matching prefix: TEST_* +# - Except denied: SSH_AUTH_SOCK +# Created: 2026-01-12 +# Feature: Selective Environment Inheritance (commit 49f5581) + +servers: + # Test tier1+tier2 with comprehensive configuration + - name: "test-inherit-tier2-comprehensive" + prefix: "tier2" + transport: "stdio" + command: "/usr/bin/env" # Prints environment to verify inheritance + timeout: "10s" + inherit: + mode: tier1+tier2 # Baseline + network/TLS variables + extra: + - PYTHONPATH # Python module path + - VIRTUAL_ENV # Virtual environment + prefix: + - TEST_ # All TEST_* variables (for testing) + deny: + - SSH_AUTH_SOCK # Block SSH agent even if somehow matched + # + # Expected inherited variables: + # Tier 1 (baseline): + # - PATH, HOME, USER, SHELL, LOGNAME + # - LANG, LC_ALL, LC_*, TZ + # - TMPDIR, TEMP, TMP + # Tier 2 (network/TLS): + # - SSL_CERT_FILE, SSL_CERT_DIR (system TLS certificates) + # - REQUESTS_CA_BUNDLE (Python requests library) + # - CURL_CA_BUNDLE (curl) + # - NODE_EXTRA_CA_CERTS (Node.js) + # Extra: + # - PYTHONPATH (if set in parent) + # - VIRTUAL_ENV (if set in parent) + # Prefix: + # - All TEST_* variables (e.g., TEST_VAR1, TEST_CONFIG, etc.) + # Denied: + # - NOT SSH_AUTH_SOCK (explicitly blocked) + # + # Use case: Comprehensive test of tier2, extras, prefix matching, and deny list + +proxy: + healthCheckInterval: "30s" + connectionTimeout: "10s" + maxRetries: 3