Skip to content

feat: support secret remote MCP URLs and headers#416

Merged
penso merged 6 commits intomainfrom
issue-119
Mar 12, 2026
Merged

feat: support secret remote MCP URLs and headers#416
penso merged 6 commits intomainfrom
issue-119

Conversation

@penso
Copy link
Copy Markdown
Collaborator

@penso penso commented Mar 11, 2026

Summary

  • support remote MCP URL query-string keys and request headers as secret-aware config
  • sanitize remote MCP status/UI output, hide pending OAuth URLs, and avoid storing raw remote URLs in OAuth registration logs/keys
  • add remote header add/edit/clear UI, docs, and targeted Rust plus Playwright coverage

Closes #119
Closes #140

Validation

Completed

  • just format
  • biome check --write crates/web/src/assets/js/page-mcp.js crates/web/ui/e2e/specs/mcp.spec.js
  • just build-css
  • CARGO_HOME=/tmp/cargo-home-issue119 CARGO_TARGET_DIR=target/issue-119-validate cargo test -p moltis-mcp --lib
  • CARGO_HOME=/tmp/cargo-home-issue119 CARGO_TARGET_DIR=target/issue-119-validate cargo test -p moltis-oauth registration_store --lib
  • CARGO_HOME=/tmp/cargo-home-issue119 CARGO_TARGET_DIR=target/issue-119-validate cargo test -p moltis-gateway --lib mcp_service::tests -- --nocapture
  • CARGO_HOME=/tmp/cargo-home-issue119 CARGO_TARGET_DIR=target/issue-119-validate cargo +nightly-2025-11-30 clippy -p moltis-mcp -p moltis-gateway --lib --tests -- -D warnings
  • cargo +nightly-2025-11-30 clippy -Z unstable-options --workspace --all-targets --timings -- -D warnings
  • CARGO_HOME=/tmp/cargo-home-issue119 CARGO_TARGET_DIR=target/issue-119-validate cargo test -p moltis check_mcp_servers -- --nocapture
  • cd crates/web/ui && npm run e2e -- e2e/specs/mcp.spec.js

Remaining

  • ./scripts/local-validate.sh 416

Manual QA

  • add a remote SSE server with a URL like https://mcp.example.com/mcp?api_key=$THEKEY and confirm the saved/editable UI shows a redacted URL only
  • add request headers in the MCP settings page, then edit the server and confirm values stay hidden while header names remain visible
  • use Clear stored headers in the edit flow and confirm the server reconnects without the previous headers
  • trigger OAuth for a remote MCP server and confirm the UI requests a fresh auth URL instead of exposing it in the server list payload

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 11, 2026

Greptile Summary

This PR adds secret-aware handling for remote MCP server URLs and request headers: query-string values and header values are wrapped in Secret<String> throughout the stack, OAuth registration keys are hashed with SHA-256 to avoid persisting raw secret-bearing URLs, all status/log/error paths use a sanitized display projection, and the web UI gains header add/edit/clear controls with redacted display.

Key changes:

  • New crates/mcp/src/remote.rs module: ResolvedRemoteConfig centralises URL + header resolution (env-placeholder substitution, HeaderMap construction, reserved-header guard), sanitize_url_for_display redacts query-string values while preserving $NAME/${NAME} placeholders
  • McpServerConfig.url promoted to Option<Secret<String>> and new headers: HashMap<String, Secret<String>> field added with custom serde helpers that expose raw values only during disk serialization
  • McpManager.status_all() now returns a sanitized URL and header names only; auth_url is removed from the status payload entirely
  • RegistrationStore now keys entries by sha256:<hex> of the server URL, with a load fallback for legacy raw-URL keys
  • MCP server startup moved to after runtime env overrides are available, ensuring URL/header placeholders resolve on first boot
  • substitute_env_placeholders processes bytes individually and casts each non-$ byte to char via as char, which corrupts multi-byte UTF-8 sequences; typical ASCII URLs are unaffected but this is a latent correctness bug
  • decode_display_escapes contains a duplicate ("%24", "$") entry (digits have no case so the second replacement is always a no-op)
  • merge_env_overrides silently gives config-file values priority over credential-store (UI) values via entry().or_insert() with no comment explaining the chosen priority order
  • safeRemoteUrlText in the JS front-end probes several property names (sanitized_url, url_display, display_url, safe_url) that are not emitted by the backend, obscuring the real API contract

Confidence Score: 4/5

  • Safe to merge; all security-sensitive paths are well-protected, with one latent correctness bug (non-ASCII byte corruption) that is extremely unlikely to affect real-world MCP URLs or headers.
  • The security design is solid: secrets are wrapped in Secret<String>, never exposed in logs or the status API, OAuth registration keys are hashed, and the SSE transport correctly gates custom Authorization headers against OAuth. The main issue is substitute_env_placeholders iterating bytes and casting with as char, which silently corrupts non-ASCII UTF-8 — a real bug that poses no practical risk for typical ASCII MCP URLs but should be fixed. The two other findings (duplicate dead code, undocumented env-override priority) are minor. Test coverage is thorough across Rust unit tests and Playwright E2E specs.
  • crates/mcp/src/remote.rs — the substitute_env_placeholders byte-iteration bug and duplicate decode_display_escapes entry should be addressed before wider deployment.

Important Files Changed

Filename Overview
crates/mcp/src/remote.rs New module encapsulating secret-aware remote config resolution, URL sanitization, and env-placeholder substitution. Contains a real correctness bug: non-ASCII UTF-8 bytes are corrupted during substitution (byte-cast to char). Also has a duplicate %24 dead-code entry in decode_display_escapes.
crates/gateway/src/mcp_service.rs Refactored to use Secret<String> for URLs and headers, adds credential-store env refresh before every MCP operation. merge_env_overrides silently gives config values priority over DB values with no comment explaining the intent.
crates/mcp/src/registry.rs Migrates url field to Option<Secret<String>> and adds headers: HashMap<String, Secret<String>> with custom serde helpers that expose raw values only during serialization. Round-trip test confirms secrets survive serialize/deserialize.
crates/oauth/src/registration_store.rs Keys are now SHA-256 hashes of the raw server URL (sha256:<hex>), preventing secrets in query strings from leaking into the registration file. load and delete both fall back to raw-URL keys for backward compatibility with existing entries.
crates/mcp/src/manager.rs Status payload now exposes only sanitized URLs and header names (not values), removes auth_url from status, and clears env for SSE servers. New tests confirm sanitization and that pending OAuth URLs are hidden.
crates/mcp/src/sse_transport.rs Replaces bare url: String with request_url: Secret<String> + display_url + default_headers, applies custom headers on every POST/GET/DELETE, and skips the custom Authorization header when OAuth auth is present. All log/error paths use display_url.
crates/mcp/src/auth.rs Wraps server_url in Secret<String>, adds a pre-computed server_url_display for all logs and error messages, and removes the OAuth authorization URL from info-level logs. Clean and correct.
crates/web/src/assets/js/page-mcp.js Adds header add/edit/clear UI for SSE servers, refactors save logic into buildSseEditPayload/buildStdioEditPayload, and forces OAuth to always start fresh (passing null). safeRemoteUrlText probes several phantom property names that don't exist in the API.
crates/config/src/schema.rs Adds headers: HashMap<String, String> to McpServerEntry with #[serde(default)]. Straightforward schema extension.
crates/gateway/src/server.rs Moves MCP server startup to after runtime env overrides are wired (so URL/header placeholders resolve on boot), sets the credential store on LiveMcpService, and wraps the config URL in Secret::new. Correct sequencing fix.

Sequence Diagram

sequenceDiagram
    participant UI as Web UI (page-mcp.js)
    participant GW as Gateway (mcp_service.rs)
    participant MGR as McpManager
    participant REM as remote.rs
    participant SSE as SseTransport
    participant MCP as Remote MCP Server

    UI->>GW: mcp.add / mcp.update {url, headers}
    GW->>GW: parse_server_config() → Secret<url>, Secret<headers>
    GW->>GW: refresh_manager_env_overrides() (config + credential store)
    GW->>MGR: add_server / update_server(McpServerConfig)
    MGR->>REM: ResolvedRemoteConfig::from_server_config(config, env_overrides)
    REM->>REM: substitute_env_placeholders(url)
    REM->>REM: build_header_map(headers, env_overrides)
    REM-->>MGR: ResolvedRemoteConfig {request_url: Secret, display_url, headers: HeaderMap}
    MGR->>SSE: SseTransport::new_with_remote(remote)
    SSE->>MCP: POST (with custom headers + optional Bearer token)
    MCP-->>SSE: response
    SSE-->>MGR: McpClient
    MGR-->>GW: Ok
    GW->>MGR: status_all()
    MGR-->>GW: ServerStatus {url: sanitized, header_names: [...], auth_url: None}
    GW-->>UI: sanitized status payload
Loading

Comments Outside Diff (3)

  1. crates/mcp/src/remote.rs, line 1358-1418 (link)

    Non-ASCII UTF-8 bytes are corrupted during substitution

    substitute_env_placeholders iterates over raw bytes and pushes each non-`

    Greptile Summary

This PR adds secret-aware handling for remote MCP server URLs and request headers: query-string values and header values are wrapped in Secret<String> throughout the stack, OAuth registration keys are hashed with SHA-256 to avoid persisting raw secret-bearing URLs, all status/log/error paths use a sanitized display projection, and the web UI gains header add/edit/clear controls with redacted display.

Key changes:

  • New crates/mcp/src/remote.rs module: ResolvedRemoteConfig centralises URL + header resolution (env-placeholder substitution, HeaderMap construction, reserved-header guard), sanitize_url_for_display redacts query-string values while preserving $NAME/${NAME} placeholders
  • McpServerConfig.url promoted to Option<Secret<String>> and new headers: HashMap<String, Secret<String>> field added with custom serde helpers that expose raw values only during disk serialization
  • McpManager.status_all() now returns a sanitized URL and header names only; auth_url is removed from the status payload entirely
  • RegistrationStore now keys entries by sha256:<hex> of the server URL, with a load fallback for legacy raw-URL keys
  • MCP server startup moved to after runtime env overrides are available, ensuring URL/header placeholders resolve on first boot
  • substitute_env_placeholders processes bytes individually and casts each non-$ byte to char via as char, which corrupts multi-byte UTF-8 sequences; typical ASCII URLs are unaffected but this is a latent correctness bug
  • decode_display_escapes contains a duplicate ("%24", "$") entry (digits have no case so the second replacement is always a no-op)
  • merge_env_overrides silently gives config-file values priority over credential-store (UI) values via entry().or_insert() with no comment explaining the chosen priority order
  • safeRemoteUrlText in the JS front-end probes several property names (sanitized_url, url_display, display_url, safe_url) that are not emitted by the backend, obscuring the real API contract

Confidence Score: 4/5

  • Safe to merge; all security-sensitive paths are well-protected, with one latent correctness bug (non-ASCII byte corruption) that is extremely unlikely to affect real-world MCP URLs or headers.
  • The security design is solid: secrets are wrapped in Secret<String>, never exposed in logs or the status API, OAuth registration keys are hashed, and the SSE transport correctly gates custom Authorization headers against OAuth. The main issue is substitute_env_placeholders iterating bytes and casting with as char, which silently corrupts non-ASCII UTF-8 — a real bug that poses no practical risk for typical ASCII MCP URLs but should be fixed. The two other findings (duplicate dead code, undocumented env-override priority) are minor. Test coverage is thorough across Rust unit tests and Playwright E2E specs.
  • crates/mcp/src/remote.rs — the substitute_env_placeholders byte-iteration bug and duplicate decode_display_escapes entry should be addressed before wider deployment.

Important Files Changed

Filename Overview
crates/mcp/src/remote.rs New module encapsulating secret-aware remote config resolution, URL sanitization, and env-placeholder substitution. Contains a real correctness bug: non-ASCII UTF-8 bytes are corrupted during substitution (byte-cast to char). Also has a duplicate %24 dead-code entry in decode_display_escapes.
crates/gateway/src/mcp_service.rs Refactored to use Secret<String> for URLs and headers, adds credential-store env refresh before every MCP operation. merge_env_overrides silently gives config values priority over DB values with no comment explaining the intent.
crates/mcp/src/registry.rs Migrates url field to Option<Secret<String>> and adds headers: HashMap<String, Secret<String>> with custom serde helpers that expose raw values only during serialization. Round-trip test confirms secrets survive serialize/deserialize.
crates/oauth/src/registration_store.rs Keys are now SHA-256 hashes of the raw server URL (sha256:<hex>), preventing secrets in query strings from leaking into the registration file. load and delete both fall back to raw-URL keys for backward compatibility with existing entries.
crates/mcp/src/manager.rs Status payload now exposes only sanitized URLs and header names (not values), removes auth_url from status, and clears env for SSE servers. New tests confirm sanitization and that pending OAuth URLs are hidden.
crates/mcp/src/sse_transport.rs Replaces bare url: String with request_url: Secret<String> + display_url + default_headers, applies custom headers on every POST/GET/DELETE, and skips the custom Authorization header when OAuth auth is present. All log/error paths use display_url.
crates/mcp/src/auth.rs Wraps server_url in Secret<String>, adds a pre-computed server_url_display for all logs and error messages, and removes the OAuth authorization URL from info-level logs. Clean and correct.
crates/web/src/assets/js/page-mcp.js Adds header add/edit/clear UI for SSE servers, refactors save logic into buildSseEditPayload/buildStdioEditPayload, and forces OAuth to always start fresh (passing null). safeRemoteUrlText probes several phantom property names that don't exist in the API.
crates/config/src/schema.rs Adds headers: HashMap<String, String> to McpServerEntry with #[serde(default)]. Straightforward schema extension.
crates/gateway/src/server.rs Moves MCP server startup to after runtime env overrides are wired (so URL/header placeholders resolve on boot), sets the credential store on LiveMcpService, and wraps the config URL in Secret::new. Correct sequencing fix.

Sequence Diagram

sequenceDiagram
    participant UI as Web UI (page-mcp.js)
    participant GW as Gateway (mcp_service.rs)
    participant MGR as McpManager
    participant REM as remote.rs
    participant SSE as SseTransport
    participant MCP as Remote MCP Server

    UI->>GW: mcp.add / mcp.update {url, headers}
    GW->>GW: parse_server_config() → Secret<url>, Secret<headers>
    GW->>GW: refresh_manager_env_overrides() (config + credential store)
    GW->>MGR: add_server / update_server(McpServerConfig)
    MGR->>REM: ResolvedRemoteConfig::from_server_config(config, env_overrides)
    REM->>REM: substitute_env_placeholders(url)
    REM->>REM: build_header_map(headers, env_overrides)
    REM-->>MGR: ResolvedRemoteConfig {request_url: Secret, display_url, headers: HeaderMap}
    MGR->>SSE: SseTransport::new_with_remote(remote)
    SSE->>MCP: POST (with custom headers + optional Bearer token)
    MCP-->>SSE: response
    SSE-->>MGR: McpClient
    MGR-->>GW: Ok
    GW->>MGR: status_all()
    MGR-->>GW: ServerStatus {url: sanitized, header_names: [...], auth_url: None}
    GW-->>UI: sanitized status payload
Loading

byte as a char via bytes[idx] as char. For valid UTF-8 strings that contain non-ASCII characters (any code point > 127), multi-byte sequences are split: e.g., the two bytes 0xC3 0xA9 that encode "é" are pushed as the unrelated codepoints '\u{00C3}' ("Ã") and '\u{00A9}' ("©").

While HTTP header values and MCP URLs are typically ASCII, reqwest::header::HeaderValue::from_str does accept Latin-1 supplement bytes (0x80–0xFF), so a crafted header value with such bytes would be silently mangled before being substituted.

The safe fix is to copy non-`

Greptile Summary

This PR adds secret-aware handling for remote MCP server URLs and request headers: query-string values and header values are wrapped in Secret<String> throughout the stack, OAuth registration keys are hashed with SHA-256 to avoid persisting raw secret-bearing URLs, all status/log/error paths use a sanitized display projection, and the web UI gains header add/edit/clear controls with redacted display.

Key changes:

  • New crates/mcp/src/remote.rs module: ResolvedRemoteConfig centralises URL + header resolution (env-placeholder substitution, HeaderMap construction, reserved-header guard), sanitize_url_for_display redacts query-string values while preserving $NAME/${NAME} placeholders
  • McpServerConfig.url promoted to Option<Secret<String>> and new headers: HashMap<String, Secret<String>> field added with custom serde helpers that expose raw values only during disk serialization
  • McpManager.status_all() now returns a sanitized URL and header names only; auth_url is removed from the status payload entirely
  • RegistrationStore now keys entries by sha256:<hex> of the server URL, with a load fallback for legacy raw-URL keys
  • MCP server startup moved to after runtime env overrides are available, ensuring URL/header placeholders resolve on first boot
  • substitute_env_placeholders processes bytes individually and casts each non-$ byte to char via as char, which corrupts multi-byte UTF-8 sequences; typical ASCII URLs are unaffected but this is a latent correctness bug
  • decode_display_escapes contains a duplicate ("%24", "$") entry (digits have no case so the second replacement is always a no-op)
  • merge_env_overrides silently gives config-file values priority over credential-store (UI) values via entry().or_insert() with no comment explaining the chosen priority order
  • safeRemoteUrlText in the JS front-end probes several property names (sanitized_url, url_display, display_url, safe_url) that are not emitted by the backend, obscuring the real API contract

Confidence Score: 4/5

  • Safe to merge; all security-sensitive paths are well-protected, with one latent correctness bug (non-ASCII byte corruption) that is extremely unlikely to affect real-world MCP URLs or headers.
  • The security design is solid: secrets are wrapped in Secret<String>, never exposed in logs or the status API, OAuth registration keys are hashed, and the SSE transport correctly gates custom Authorization headers against OAuth. The main issue is substitute_env_placeholders iterating bytes and casting with as char, which silently corrupts non-ASCII UTF-8 — a real bug that poses no practical risk for typical ASCII MCP URLs but should be fixed. The two other findings (duplicate dead code, undocumented env-override priority) are minor. Test coverage is thorough across Rust unit tests and Playwright E2E specs.
  • crates/mcp/src/remote.rs — the substitute_env_placeholders byte-iteration bug and duplicate decode_display_escapes entry should be addressed before wider deployment.

Important Files Changed

Filename Overview
crates/mcp/src/remote.rs New module encapsulating secret-aware remote config resolution, URL sanitization, and env-placeholder substitution. Contains a real correctness bug: non-ASCII UTF-8 bytes are corrupted during substitution (byte-cast to char). Also has a duplicate %24 dead-code entry in decode_display_escapes.
crates/gateway/src/mcp_service.rs Refactored to use Secret<String> for URLs and headers, adds credential-store env refresh before every MCP operation. merge_env_overrides silently gives config values priority over DB values with no comment explaining the intent.
crates/mcp/src/registry.rs Migrates url field to Option<Secret<String>> and adds headers: HashMap<String, Secret<String>> with custom serde helpers that expose raw values only during serialization. Round-trip test confirms secrets survive serialize/deserialize.
crates/oauth/src/registration_store.rs Keys are now SHA-256 hashes of the raw server URL (sha256:<hex>), preventing secrets in query strings from leaking into the registration file. load and delete both fall back to raw-URL keys for backward compatibility with existing entries.
crates/mcp/src/manager.rs Status payload now exposes only sanitized URLs and header names (not values), removes auth_url from status, and clears env for SSE servers. New tests confirm sanitization and that pending OAuth URLs are hidden.
crates/mcp/src/sse_transport.rs Replaces bare url: String with request_url: Secret<String> + display_url + default_headers, applies custom headers on every POST/GET/DELETE, and skips the custom Authorization header when OAuth auth is present. All log/error paths use display_url.
crates/mcp/src/auth.rs Wraps server_url in Secret<String>, adds a pre-computed server_url_display for all logs and error messages, and removes the OAuth authorization URL from info-level logs. Clean and correct.
crates/web/src/assets/js/page-mcp.js Adds header add/edit/clear UI for SSE servers, refactors save logic into buildSseEditPayload/buildStdioEditPayload, and forces OAuth to always start fresh (passing null). safeRemoteUrlText probes several phantom property names that don't exist in the API.
crates/config/src/schema.rs Adds headers: HashMap<String, String> to McpServerEntry with #[serde(default)]. Straightforward schema extension.
crates/gateway/src/server.rs Moves MCP server startup to after runtime env overrides are wired (so URL/header placeholders resolve on boot), sets the credential store on LiveMcpService, and wraps the config URL in Secret::new. Correct sequencing fix.

Sequence Diagram

sequenceDiagram
    participant UI as Web UI (page-mcp.js)
    participant GW as Gateway (mcp_service.rs)
    participant MGR as McpManager
    participant REM as remote.rs
    participant SSE as SseTransport
    participant MCP as Remote MCP Server

    UI->>GW: mcp.add / mcp.update {url, headers}
    GW->>GW: parse_server_config() → Secret<url>, Secret<headers>
    GW->>GW: refresh_manager_env_overrides() (config + credential store)
    GW->>MGR: add_server / update_server(McpServerConfig)
    MGR->>REM: ResolvedRemoteConfig::from_server_config(config, env_overrides)
    REM->>REM: substitute_env_placeholders(url)
    REM->>REM: build_header_map(headers, env_overrides)
    REM-->>MGR: ResolvedRemoteConfig {request_url: Secret, display_url, headers: HeaderMap}
    MGR->>SSE: SseTransport::new_with_remote(remote)
    SSE->>MCP: POST (with custom headers + optional Bearer token)
    MCP-->>SSE: response
    SSE-->>MGR: McpClient
    MGR-->>GW: Ok
    GW->>MGR: status_all()
    MGR-->>GW: ServerStatus {url: sanitized, header_names: [...], auth_url: None}
    GW-->>UI: sanitized status payload
Loading

bytes as contiguous slices from the original &str rather than byte-cast them individually:

// instead of:
out.push(bytes[idx] as char);
// prefer:
out.push_str(&input[idx..idx + 1]);  // valid only for ASCII; for full safety copy up to next '

<sub>Last reviewed commit: 2a672ba</sub>

A robust implementation would locate the next `

Greptile Summary

This PR adds secret-aware handling for remote MCP server URLs and request headers: query-string values and header values are wrapped in Secret<String> throughout the stack, OAuth registration keys are hashed with SHA-256 to avoid persisting raw secret-bearing URLs, all status/log/error paths use a sanitized display projection, and the web UI gains header add/edit/clear controls with redacted display.

Key changes:

  • New crates/mcp/src/remote.rs module: ResolvedRemoteConfig centralises URL + header resolution (env-placeholder substitution, HeaderMap construction, reserved-header guard), sanitize_url_for_display redacts query-string values while preserving $NAME/${NAME} placeholders
  • McpServerConfig.url promoted to Option<Secret<String>> and new headers: HashMap<String, Secret<String>> field added with custom serde helpers that expose raw values only during disk serialization
  • McpManager.status_all() now returns a sanitized URL and header names only; auth_url is removed from the status payload entirely
  • RegistrationStore now keys entries by sha256:<hex> of the server URL, with a load fallback for legacy raw-URL keys
  • MCP server startup moved to after runtime env overrides are available, ensuring URL/header placeholders resolve on first boot
  • substitute_env_placeholders processes bytes individually and casts each non-$ byte to char via as char, which corrupts multi-byte UTF-8 sequences; typical ASCII URLs are unaffected but this is a latent correctness bug
  • decode_display_escapes contains a duplicate ("%24", "$") entry (digits have no case so the second replacement is always a no-op)
  • merge_env_overrides silently gives config-file values priority over credential-store (UI) values via entry().or_insert() with no comment explaining the chosen priority order
  • safeRemoteUrlText in the JS front-end probes several property names (sanitized_url, url_display, display_url, safe_url) that are not emitted by the backend, obscuring the real API contract

Confidence Score: 4/5

  • Safe to merge; all security-sensitive paths are well-protected, with one latent correctness bug (non-ASCII byte corruption) that is extremely unlikely to affect real-world MCP URLs or headers.
  • The security design is solid: secrets are wrapped in Secret<String>, never exposed in logs or the status API, OAuth registration keys are hashed, and the SSE transport correctly gates custom Authorization headers against OAuth. The main issue is substitute_env_placeholders iterating bytes and casting with as char, which silently corrupts non-ASCII UTF-8 — a real bug that poses no practical risk for typical ASCII MCP URLs but should be fixed. The two other findings (duplicate dead code, undocumented env-override priority) are minor. Test coverage is thorough across Rust unit tests and Playwright E2E specs.
  • crates/mcp/src/remote.rs — the substitute_env_placeholders byte-iteration bug and duplicate decode_display_escapes entry should be addressed before wider deployment.

Important Files Changed

Filename Overview
crates/mcp/src/remote.rs New module encapsulating secret-aware remote config resolution, URL sanitization, and env-placeholder substitution. Contains a real correctness bug: non-ASCII UTF-8 bytes are corrupted during substitution (byte-cast to char). Also has a duplicate %24 dead-code entry in decode_display_escapes.
crates/gateway/src/mcp_service.rs Refactored to use Secret<String> for URLs and headers, adds credential-store env refresh before every MCP operation. merge_env_overrides silently gives config values priority over DB values with no comment explaining the intent.
crates/mcp/src/registry.rs Migrates url field to Option<Secret<String>> and adds headers: HashMap<String, Secret<String>> with custom serde helpers that expose raw values only during serialization. Round-trip test confirms secrets survive serialize/deserialize.
crates/oauth/src/registration_store.rs Keys are now SHA-256 hashes of the raw server URL (sha256:<hex>), preventing secrets in query strings from leaking into the registration file. load and delete both fall back to raw-URL keys for backward compatibility with existing entries.
crates/mcp/src/manager.rs Status payload now exposes only sanitized URLs and header names (not values), removes auth_url from status, and clears env for SSE servers. New tests confirm sanitization and that pending OAuth URLs are hidden.
crates/mcp/src/sse_transport.rs Replaces bare url: String with request_url: Secret<String> + display_url + default_headers, applies custom headers on every POST/GET/DELETE, and skips the custom Authorization header when OAuth auth is present. All log/error paths use display_url.
crates/mcp/src/auth.rs Wraps server_url in Secret<String>, adds a pre-computed server_url_display for all logs and error messages, and removes the OAuth authorization URL from info-level logs. Clean and correct.
crates/web/src/assets/js/page-mcp.js Adds header add/edit/clear UI for SSE servers, refactors save logic into buildSseEditPayload/buildStdioEditPayload, and forces OAuth to always start fresh (passing null). safeRemoteUrlText probes several phantom property names that don't exist in the API.
crates/config/src/schema.rs Adds headers: HashMap<String, String> to McpServerEntry with #[serde(default)]. Straightforward schema extension.
crates/gateway/src/server.rs Moves MCP server startup to after runtime env overrides are wired (so URL/header placeholders resolve on boot), sets the credential store on LiveMcpService, and wraps the config URL in Secret::new. Correct sequencing fix.

Sequence Diagram

sequenceDiagram
    participant UI as Web UI (page-mcp.js)
    participant GW as Gateway (mcp_service.rs)
    participant MGR as McpManager
    participant REM as remote.rs
    participant SSE as SseTransport
    participant MCP as Remote MCP Server

    UI->>GW: mcp.add / mcp.update {url, headers}
    GW->>GW: parse_server_config() → Secret<url>, Secret<headers>
    GW->>GW: refresh_manager_env_overrides() (config + credential store)
    GW->>MGR: add_server / update_server(McpServerConfig)
    MGR->>REM: ResolvedRemoteConfig::from_server_config(config, env_overrides)
    REM->>REM: substitute_env_placeholders(url)
    REM->>REM: build_header_map(headers, env_overrides)
    REM-->>MGR: ResolvedRemoteConfig {request_url: Secret, display_url, headers: HeaderMap}
    MGR->>SSE: SseTransport::new_with_remote(remote)
    SSE->>MCP: POST (with custom headers + optional Bearer token)
    MCP-->>SSE: response
    SSE-->>MGR: McpClient
    MGR-->>GW: Ok
    GW->>MGR: status_all()
    MGR-->>GW: ServerStatus {url: sanitized, header_names: [...], auth_url: None}
    GW-->>UI: sanitized status payload
Loading

byte with memchr (or `input.find('

Last reviewed commit: 2a672ba)) and push_str` the slice between them in one step, avoiding the cast entirely.

  1. crates/mcp/src/remote.rs, line 1482-1498 (link)

    Duplicate %24 entry is dead code

    The decode_display_escapes array contains two identical ("%24", "$") tuples — one in the "uppercase" group and one in the "lowercase" group. Because the code point 24 contains only decimal digits, there is no case difference and the second replacement is a guaranteed no-op after the first has already replaced every %24.

        [
            ("%5B", "["),
            ("%5D", "]"),
            ("%24", "$"),
            ("%7B", "{"),
            ("%7D", "}"),
            ("%5b", "["),
            ("%5d", "]"),
            ("%7b", "{"),
            ("%7d", "}"),
        ]
    
    
    
  2. crates/web/src/assets/js/page-mcp.js, line 2184-2189 (link)

    safeRemoteUrlText probes several non-existent API properties

    The fallback chain checks server.sanitized_url, server.url_display, server.display_url, and server.safe_url — none of which are emitted by the backend. The ServerStatus struct serialises the sanitised URL as url, so the only meaningful entry in the chain is server.url.

    Keeping phantom property names in the loop has two downsides:

    1. It obscures the actual API contract for future readers.
    2. If a future backend accidentally starts emitting one of those names with a raw (unsanitised) URL, this function would silently surface it.

    Consider simplifying to just the real field:

Last reviewed commit: 2a672ba

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a672ba651

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq bot commented Mar 11, 2026

Merging this PR will not alter performance

✅ 39 untouched benchmarks
⏩ 5 skipped benchmarks1


Comparing issue-119 (033b52c) with main (564b9a7)

Open in CodSpeed

Footnotes

  1. 5 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 11, 2026

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7a096e4268

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +159 to +160
if self.auth.is_some() && name == reqwest::header::AUTHORIZATION {
continue;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve custom Authorization header when OAuth token missing

Skipping Authorization whenever self.auth.is_some() drops user-configured remote headers even when the auth provider has no bearer token to inject. After any prior 401 that creates an auth provider (or before OAuth is completed), requests to servers that rely on a static Authorization header (for example API-key style auth) are sent without that header and will keep failing with 401, making the server effectively unrecoverable until config/auth state is reset.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3ea453e98

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +160 to +162
let auth_provider = existing_auth.unwrap_or_else(|| {
Self::build_auth_provider(name, &remote, config.oauth.as_ref())
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Rebuild cached OAuth provider when resolved URL changes

This path reuses an existing auth provider purely by server name, but the commit now resolves SSE URLs from mutable env_overrides before connecting. If a placeholder-backed URL changes (for example after updating Settings environment variables), the transport uses the new URL while OAuth discovery/refresh in the reused provider still targets the old server URL/resource it was created with, which can cause persistent 401s during restart/reauth. Recreate or invalidate the cached provider when the resolved remote URL (or OAuth override inputs) changes.

Useful? React with 👍 / 👎.

Comment on lines +86 to +87
let value = if value.is_empty() || is_entire_env_placeholder_syntax(&value) {
value.into_owned()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Redact $NAME query values unless placeholders are verified

The sanitizer preserves any query value that syntactically looks like $NAME or ${NAME}. A literal secret token with that shape (for example token=$ABCD123) will therefore be shown unredacted in status/UI, because this branch treats it as a placeholder. That leaks secrets for dollar-prefixed tokens; values should only be preserved when they are confirmed placeholders and otherwise be redacted.

Useful? React with 👍 / 👎.

@penso penso merged commit 742bce9 into main Mar 12, 2026
35 of 36 checks passed
@penso penso deleted the issue-119 branch March 12, 2026 11:14
penso added a commit that referenced this pull request Mar 23, 2026
* feat(mcp): support secret remote URLs and headers

* fix(cli): add missing MCP header defaults in doctor tests

* fix(mcp): address review feedback

* test(onboarding): make LLM step navigation resilient

* fix(mcp): stabilize remote header validation order
jmikedupont2 pushed a commit to meta-introspector/moltis that referenced this pull request Mar 23, 2026
* feat(mcp): support secret remote URLs and headers

* fix(cli): add missing MCP header defaults in doctor tests

* fix(mcp): address review feedback

* test(onboarding): make LLM step navigation resilient

* fix(mcp): stabilize remote header validation order
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Support remote MCP with key in the URL [Feature]: Add headers config to MCP's HTTP/SSE transport

1 participant