feat: add TLS configuration support for MCP HTTP/SSE client connections - #3779
Merged
Merged
Conversation
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (9)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds optional TLS configuration for HTTP/SSE MCP clients: schema and storage, TLS-enabled http.Client builder and injection into MCP transports, DB migration/persistence, OpenAPI/docs, redaction/merge updates, config-hash inclusion, and frontend create/edit UI changes. ChangesMCP TLS Configuration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
BearTS
marked this pull request as ready for review
May 26, 2026 21:28
BearTS
changed the title
insecure_skip_verify, ca_cert_pem) for HTTP and SSE MCP connections
Contributor
Confidence Score: 4/5Safe to merge for API/UI users, but file-based config.json consumers cannot use tls_config until the schema is updated. The Go implementation is complete and correct; the one concrete gap is that transports/config.schema.json — needs Important Files Changed
Reviews (8): Last reviewed commit: "feat: add custom ssl support in mcp" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx (1)
228-230: ⚡ Quick winInconsistent TLS payload normalization between create and edit forms.
mcpClientForm.tsxusesbuildTLSConfigPayloadto strip emptyca_cert_pemwhen onlyinsecure_skip_verifyis set, but this inline logic sends the entiredata.tls_configobject. This could result in the edit form sending{ insecure_skip_verify: true, ca_cert_pem: { value: "", env_var: "", from_env: false } }while the create form sends{ insecure_skip_verify: true }.Consider extracting
buildTLSConfigPayloadto a shared utility (e.g.,@/lib/utils/mcp.ts) and reusing it here.♻️ Proposed fix
Create a shared utility:
// `@/lib/utils/mcp.ts` import { MCPTLSConfig } from "`@/lib/types/mcp`"; /** Strips empty TLS config so we don't send `{}` to the server. */ export function buildTLSConfigPayload(tls: MCPTLSConfig | undefined): MCPTLSConfig | undefined { if (!tls) return undefined; const hasSkipVerify = tls.insecure_skip_verify === true; const hasCACert = tls.ca_cert_pem?.value || tls.ca_cert_pem?.from_env; if (!hasSkipVerify && !hasCACert) return undefined; return { insecure_skip_verify: tls.insecure_skip_verify, ca_cert_pem: hasCACert ? tls.ca_cert_pem : undefined }; }Then update this file:
+import { buildTLSConfigPayload } from "`@/lib/utils/mcp`"; // ... - tls_config: data.tls_config?.insecure_skip_verify || data.tls_config?.ca_cert_pem?.value || data.tls_config?.ca_cert_pem?.from_env - ? data.tls_config - : undefined, + tls_config: buildTLSConfigPayload(data.tls_config),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx` around lines 228 - 230, The edit form sends the raw data.tls_config instead of normalizing it like the create form; extract the existing buildTLSConfigPayload (used in mcpClientForm.tsx) into a shared utility (e.g., `@/lib/utils/mcp.ts`) that accepts MCPTLSConfig | undefined and returns MCPTLSConfig | undefined, then replace the inline logic in mcpClientSheet.tsx to call buildTLSConfigPayload(data.tls_config) (and import it) so empty ca_cert_pem objects are stripped and both create and edit flows send the same normalized TLS payload.framework/configstore/tables/mcp.go (1)
83-92: 💤 Low valueConsider encrypting
TLSConfigJSONwhen it contains CA certificate PEM data.
HeadersJSONis encrypted whenencrypt.IsEnabled()because it may contain secrets.TLSConfigJSONcan containca_cert_pemwhich, while not a secret key, represents infrastructure configuration that some organizations treat as sensitive. The current implementation stores it in plaintext.If this is intentional (CA certs are public trust anchors, not secrets), no change is needed. Otherwise, consider adding encryption logic similar to
HeadersJSONin lines 191-197.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@framework/configstore/tables/mcp.go` around lines 83 - 92, The TLSConfigJSON field is being stored in plaintext even though TLSConfig may include ca_cert_pem; if your org wants this treated as sensitive, mirror the HeadersJSON encryption pattern: when encrypt.IsEnabled() is true, encrypt the marshaled TLSConfig bytes before assigning TLSConfigJSON (use the same encrypt.Encrypt/Decrypt helpers and the same conditional branch used for HeadersJSON), otherwise continue storing plaintext; update the code paths that set TLSConfigJSON in the TLSConfig handling (references: TLSConfig, TLSConfigJSON, HeadersJSON, encrypt.IsEnabled()) so both fields follow consistent encryption behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/openapi/schemas/management/mcp.yaml`:
- Around line 216-225: MCPClientUpdateRequest contains duplicate tls_config
mappings; remove the duplicate entry (or merge their properties) so tls_config
appears only once under the MCPClientUpdateRequest schema; specifically, keep a
single tls_config object definition with properties insecure_skip_verify and
ca_cert_pem and delete the redundant tls_config block referenced in the diff
(also apply the same de-duplication to the duplicate at lines 285-301).
In `@framework/configstore/rdb.go`:
- Around line 1825-1827: The current update code only sets
updates["tls_config_json"] when tlsConfigJSON != nil so you cannot clear an
existing TLS config; change the logic to explicitly include the key in updates
even when tlsConfigJSON == nil (e.g., set updates["tls_config_json"] = nil or a
SQL NULL-equivalent) for normal MCP client updates so the column is set to NULL
in the DB; modify the branch that currently checks tlsConfigJSON != nil to
instead distinguish "config_hash" updates from normal updates and ensure
updates["tls_config_json"] is present (with nil) when the caller intended to
clear the value, referencing the tlsConfigJSON variable and the updates map in
rdb.go.
In `@transports/bifrost-http/handlers/mcp.go`:
- Around line 1194-1203: The TLS preservation logic only covers redacted
CACertPEM but misses the case where incoming.TLSConfig is nil (omitted) so the
existing TLS config gets cleared; modify the merge to first check if
incoming.TLSConfig == nil and if so set merged.TLSConfig = oldRaw.TLSConfig
(preserving current config), and still keep the existing redacted-CA handling
that replaces incoming.TLSConfig.CACertPEM with oldRaw.TLSConfig.CACertPEM when
incoming.TLSConfig.CACertPEM.IsRedacted() &&
incoming.TLSConfig.CACertPEM.Equals(oldRedacted.TLSConfig.CACertPEM); update
code around incoming.TLSConfig / oldRaw / oldRedacted / merged.TLSConfig to
match the pattern used for IsPingAvailable/AllowedExtraHeaders/Headers.
---
Nitpick comments:
In `@framework/configstore/tables/mcp.go`:
- Around line 83-92: The TLSConfigJSON field is being stored in plaintext even
though TLSConfig may include ca_cert_pem; if your org wants this treated as
sensitive, mirror the HeadersJSON encryption pattern: when encrypt.IsEnabled()
is true, encrypt the marshaled TLSConfig bytes before assigning TLSConfigJSON
(use the same encrypt.Encrypt/Decrypt helpers and the same conditional branch
used for HeadersJSON), otherwise continue storing plaintext; update the code
paths that set TLSConfigJSON in the TLSConfig handling (references: TLSConfig,
TLSConfigJSON, HeadersJSON, encrypt.IsEnabled()) so both fields follow
consistent encryption behavior.
In `@ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx`:
- Around line 228-230: The edit form sends the raw data.tls_config instead of
normalizing it like the create form; extract the existing buildTLSConfigPayload
(used in mcpClientForm.tsx) into a shared utility (e.g., `@/lib/utils/mcp.ts`)
that accepts MCPTLSConfig | undefined and returns MCPTLSConfig | undefined, then
replace the inline logic in mcpClientSheet.tsx to call
buildTLSConfigPayload(data.tls_config) (and import it) so empty ca_cert_pem
objects are stripped and both create and edit flows send the same normalized TLS
payload.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 54c787b5-3d9c-4924-a59c-17f7f717df97
📒 Files selected for processing (12)
core/mcp/clientmanager.gocore/schemas/mcp.godocs/openapi/schemas/management/mcp.yamlframework/configstore/clientconfig.goframework/configstore/rdb.goframework/configstore/tables/mcp.gotransports/bifrost-http/handlers/mcp.gotransports/bifrost-http/lib/config.goui/app/workspace/mcp-registry/views/mcpClientForm.tsxui/app/workspace/mcp-registry/views/mcpClientSheet.tsxui/lib/types/mcp.tsui/lib/types/schemas.ts
BearTS
force-pushed
the
05-27-feat_add_custom_ssl_support_in_mcp
branch
from
May 26, 2026 21:58
ace91fb to
397ed4a
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
transports/bifrost-http/handlers/mcp.go (1)
587-604:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTLSConfig is not propagated in the create flow.
The
schemasConfigfor newly created MCP clients does not includeTLSConfigfrom the request. While the update path (line 861) correctly includesreq.TLSConfig, all three creation paths (non-OAuth here, OAuth at lines 524-541, and per-user OAuth at lines 432-449) omit it. The frontend sendstls_configin creation requests, but it won't be persisted or used.🐛 Proposed fix for non-OAuth path
schemasConfig := &schemas.MCPClientConfig{ ID: req.ClientID, Name: req.Name, IsCodeModeClient: req.IsCodeModeClient, ConnectionType: schemas.MCPConnectionType(req.ConnectionType), ConnectionString: req.ConnectionString, StdioConfig: req.StdioConfig, + TLSConfig: req.TLSConfig, ToolsToExecute: req.ToolsToExecute, ToolsToAutoExecute: req.ToolsToAutoExecute, Headers: req.Headers, AllowedExtraHeaders: req.AllowedExtraHeaders, AuthType: schemas.MCPAuthType(req.AuthType), OauthConfigID: req.OauthConfigID, IsPingAvailable: req.IsPingAvailable, ToolSyncInterval: toolSyncInterval, ToolPricing: req.ToolPricing, AllowOnAllVirtualKeys: req.AllowOnAllVirtualKeys, }Apply the same fix to the
pendingConfigstructs in the OAuth paths (lines 432-449 and 524-541).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@transports/bifrost-http/handlers/mcp.go` around lines 587 - 604, The MCP client creation paths are not propagating TLSConfig from the request so tls_config sent by the frontend isn't persisted; update the non-OAuth creation block that builds schemasConfig (the schemas.MCPClientConfig literal) to set TLSConfig: req.TLSConfig, and apply the same addition to the pendingConfig structs used in both OAuth creation branches (the pendingConfig variables around the per-user OAuth and OAuth paths) so all three creation flows include req.TLSConfig (the update path already does this).
🧹 Nitpick comments (2)
docs/openapi/schemas/management/mcp.yaml (1)
137-153: ⚡ Quick winExtract a shared
MCPTLSConfigschema and reference it.
tls_configis duplicated inline; centralizing it avoids schema drift across create/update variants.♻️ Proposed refactor
+MCPTLSConfig: + type: object + description: | + TLS configuration for HTTP and SSE connections. + Not applicable to stdio or inprocess connection types. + properties: + insecure_skip_verify: + type: boolean + description: | + Disable TLS certificate verification. Takes priority over ca_cert_pem when both are set. + Use only in development or trusted isolated environments. Not recommended for production. + ca_cert_pem: + type: string + description: | + PEM-encoded CA certificate to trust for MCP server connections. + Use when the MCP server uses a self-signed or private CA certificate. + Supports env.VAR_NAME syntax to read the certificate from an environment variable. + MCPClientCreateRequestBase: type: object properties: ... tls_config: - type: object - ... + $ref: '`#/MCPTLSConfig`' ... MCPClientUpdateRequest: type: object properties: ... tls_config: - type: object - ... + $ref: '`#/MCPTLSConfig`'Based on learnings: "In OpenAPI schemas, keep definitions modular ... use local references in the form $ref: '
#/SchemaName'."Also applies to: 275-291
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/schemas/management/mcp.yaml` around lines 137 - 153, Extract the inline tls_config object into a reusable schema named MCPTLSConfig and replace all inline occurrences (e.g., the tls_config property in the existing schema and the duplicate at the other occurrence) with local $ref references to '`#/components/schemas/MCPTLSConfig`' (or '`#/SchemaName`' pattern used in this project); ensure MCPTLSConfig preserves fields insecure_skip_verify and ca_cert_pem with their descriptions and types, and update any create/update/other MCP schemas that currently inline tls_config to reference MCPTLSConfig instead.core/mcp/clientmanager.go (1)
1262-1266: ⚡ Quick winCustom Transport loses
http.DefaultTransportsettings.The new
http.Transportonly setsTLSClientConfig, losing thehttp.DefaultTransportdefaults including:
Proxy: http.ProxyFromEnvironment— users behind corporate proxies (HTTP_PROXY/HTTPS_PROXY) will have connections failTLSHandshakeTimeout: 10 * time.Second— TLS handshakes can hang before context timeouts applyForceAttemptHTTP2: true— HTTP/2 won't be negotiatedConsider cloning the default transport and overriding only the TLS config:
♻️ Proposed fix to preserve DefaultTransport settings
+// Clone default transport to preserve proxy, timeouts, and HTTP/2 support +func cloneDefaultTransport() *http.Transport { + return &http.Transport{ + Proxy: http.ProxyFromEnvironment, + ForceAttemptHTTP2: true, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + } +} + func (m *MCPManager) buildTLSHTTPClient(tlsCfg *schemas.MCPTLSConfig) (*http.Client, error) { if tlsCfg == nil { return nil, nil } tlsConfig := &tls.Config{MinVersion: tls.VersionTLS12} if tlsCfg.InsecureSkipVerify { m.logger.Warn("MCP client: skipping TLS verification — do not use in production") tlsConfig.InsecureSkipVerify = true } else if tlsCfg.CACertPEM != nil { caPEM := tlsCfg.CACertPEM.GetValue() if caPEM != "" { rootCAs, err := x509.SystemCertPool() if err != nil { rootCAs = x509.NewCertPool() } if !rootCAs.AppendCertsFromPEM([]byte(caPEM)) { return nil, fmt.Errorf("failed to parse MCP CA certificate PEM") } tlsConfig.RootCAs = rootCAs } } + transport := cloneDefaultTransport() + transport.TLSClientConfig = tlsConfig return &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: tlsConfig, - }, + Transport: transport, }, nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/mcp/clientmanager.go` around lines 1262 - 1266, The returned http.Client currently builds a new http.Transport with only TLSClientConfig, dropping defaults like Proxy, TLSHandshakeTimeout, and ForceAttemptHTTP2; instead, obtain and clone the default transport (http.DefaultTransport) into a *http.Transport (safely type-assert/handle non-*http.Transport fallback), set its TLSClientConfig to tlsConfig, and use that cloned transport as the Client.Transport so only TLS is overridden while preserving Proxy, TLSHandshakeTimeout, ForceAttemptHTTP2, etc.; locate the return site that constructs &http.Client{Transport: &http.Transport{TLSClientConfig: tlsConfig}} in clientmanager.go and replace it with cloning logic for http.DefaultTransport.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/openapi/schemas/management/mcp.yaml`:
- Around line 341-350: Update the OpenAPI schema description for
MCPClientConfig.tls_config.ca_cert_pem to indicate that the field accepts a PEM
string for input but is redacted in responses; specifically mention that while
clients may provide a PEM (supports env.VAR_NAME syntax), API responses will
return a redacted placeholder (not the raw PEM) to avoid implying the private
value is returned. Modify the property description for ca_cert_pem in the
tls_config object to state both input format and redaction behavior so the
contract accurately reflects runtime behavior.
In `@ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx`:
- Around line 228-230: The current conditional omits tls_config entirely when
its subfields are falsy, which loses explicit user intent to clear or set
values; update the patching logic in mcpClientSheet (where tls_config is built)
to always include tls_config when the user touched/edited TLS settings (e.g.,
detect an edit flag or presence of data.tls_config object) and send explicit
values for insecure_skip_verify and ca_cert_pem (including null/empty to
indicate clearing) instead of omitting the whole tls_config; reference the
tls_config object and its properties insecure_skip_verify and ca_cert_pem in
your change so the server receives explicit updates rather than relying on
truthy checks.
---
Outside diff comments:
In `@transports/bifrost-http/handlers/mcp.go`:
- Around line 587-604: The MCP client creation paths are not propagating
TLSConfig from the request so tls_config sent by the frontend isn't persisted;
update the non-OAuth creation block that builds schemasConfig (the
schemas.MCPClientConfig literal) to set TLSConfig: req.TLSConfig, and apply the
same addition to the pendingConfig structs used in both OAuth creation branches
(the pendingConfig variables around the per-user OAuth and OAuth paths) so all
three creation flows include req.TLSConfig (the update path already does this).
---
Nitpick comments:
In `@core/mcp/clientmanager.go`:
- Around line 1262-1266: The returned http.Client currently builds a new
http.Transport with only TLSClientConfig, dropping defaults like Proxy,
TLSHandshakeTimeout, and ForceAttemptHTTP2; instead, obtain and clone the
default transport (http.DefaultTransport) into a *http.Transport (safely
type-assert/handle non-*http.Transport fallback), set its TLSClientConfig to
tlsConfig, and use that cloned transport as the Client.Transport so only TLS is
overridden while preserving Proxy, TLSHandshakeTimeout, ForceAttemptHTTP2, etc.;
locate the return site that constructs &http.Client{Transport:
&http.Transport{TLSClientConfig: tlsConfig}} in clientmanager.go and replace it
with cloning logic for http.DefaultTransport.
In `@docs/openapi/schemas/management/mcp.yaml`:
- Around line 137-153: Extract the inline tls_config object into a reusable
schema named MCPTLSConfig and replace all inline occurrences (e.g., the
tls_config property in the existing schema and the duplicate at the other
occurrence) with local $ref references to '`#/components/schemas/MCPTLSConfig`'
(or '`#/SchemaName`' pattern used in this project); ensure MCPTLSConfig preserves
fields insecure_skip_verify and ca_cert_pem with their descriptions and types,
and update any create/update/other MCP schemas that currently inline tls_config
to reference MCPTLSConfig instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 49f34480-7744-442b-a6ed-2fa13d1552c7
📒 Files selected for processing (12)
core/mcp/clientmanager.gocore/schemas/mcp.godocs/openapi/schemas/management/mcp.yamlframework/configstore/clientconfig.goframework/configstore/rdb.goframework/configstore/tables/mcp.gotransports/bifrost-http/handlers/mcp.gotransports/bifrost-http/lib/config.goui/app/workspace/mcp-registry/views/mcpClientForm.tsxui/app/workspace/mcp-registry/views/mcpClientSheet.tsxui/lib/types/mcp.tsui/lib/types/schemas.ts
BearTS
force-pushed
the
05-27-feat_add_custom_ssl_support_in_mcp
branch
from
May 26, 2026 22:13
397ed4a to
57e7ad9
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@framework/configstore/migrations.go`:
- Line 8729: Update the stale comment above migrationDropAzureAPIVersionColumn
to accurately describe its behavior: replace the incorrect note about adding
created_by_user_id to governance_virtual_keys with a brief description that this
migration drops the azure_api_version column from the config_keys table
(mentioning any relevant behavior such as whether it uses DROP COLUMN and any
constraints handled), so the comment matches the actual implementation in
migrationDropAzureAPIVersionColumn.
In `@transports/bifrost-http/handlers/mcp.go`:
- Line 861: The create paths are missing propagation of TLSConfig—update the
addMCPClient create handler to include TLSConfig: req.TLSConfig in the
schemasConfig struct literal (matching the update path), and likewise add
TLSConfig: req.TLSConfig to the pending config objects created in the OAuth
flows (the pending config creation blocks referenced in the OAuth paths),
ensuring all places that build a new MCP client config (schemasConfig / pending
config structs) include TLSConfig set from req.TLSConfig.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d8a6f2d1-b9a6-491f-bff4-0e5cb6a1d1a8
📒 Files selected for processing (13)
core/mcp/clientmanager.gocore/schemas/mcp.godocs/openapi/schemas/management/mcp.yamlframework/configstore/clientconfig.goframework/configstore/migrations.goframework/configstore/rdb.goframework/configstore/tables/mcp.gotransports/bifrost-http/handlers/mcp.gotransports/bifrost-http/lib/config.goui/app/workspace/mcp-registry/views/mcpClientForm.tsxui/app/workspace/mcp-registry/views/mcpClientSheet.tsxui/lib/types/mcp.tsui/lib/types/schemas.ts
BearTS
force-pushed
the
05-27-feat_add_custom_ssl_support_in_mcp
branch
from
May 26, 2026 22:29
57e7ad9 to
464b549
Compare
BearTS
force-pushed
the
05-27-feat_add_custom_ssl_support_in_mcp
branch
from
May 26, 2026 22:44
464b549 to
6ae7faf
Compare
BearTS
changed the title
insecure_skip_verify, ca_cert_pem) for HTTP and SSE MCP connections
17 tasks
BearTS
force-pushed
the
05-27-feat_add_custom_ssl_support_in_mcp
branch
2 times, most recently
from
May 27, 2026 20:20
71ea8a2 to
cdea6c3
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Collaborator
Merge activity
|
Pratham-Mishra04
force-pushed
the
05-27-feat_add_custom_ssl_support_in_mcp
branch
from
May 28, 2026 09:32
cdea6c3 to
2918807
Compare
akshaydeo
pushed a commit
that referenced
this pull request
May 29, 2026
…ns (#3779) ## Summary Adds TLS configuration support for HTTP and SSE MCP client connections, allowing users to connect to MCP servers that use self-signed or private CA certificates, or to disable TLS verification in development/testing environments. ## Changes - Introduced `MCPTLSConfig` schema with `InsecureSkipVerify` and `CACertPEM` fields. `CACertPEM` supports `env.VAR_NAME` syntax for reading certificates from environment variables. `InsecureSkipVerify` takes priority over `CACertPEM` when both are set. - Added `buildTLSHTTPClient` helper on `MCPManager` that constructs a custom `*http.Client` with the appropriate TLS configuration. Returns `nil` when no TLS config is provided so the library default is used. - Applied the custom HTTP client to all three connection paths: StreamableHTTP, SSE, and the OAuth verification flow. - `CACertPEM` is treated as a sensitive value and is redacted in API responses, with redaction-aware merge logic to preserve the raw value on update when the incoming value matches the previously redacted one. - `TLSConfig` is persisted as a JSON column (`tls_config_json`) in the database via a new migration and included in the MCP client config hash for change detection. - Added TLS configuration UI section to both the create form and the edit sheet, visible only for HTTP and SSE connection types. Includes a toggle for skipping TLS verification and an `EnvVarInput` textarea for the CA certificate PEM. - Updated OpenAPI schema documentation for `MCPClientCreateRequestBase`, `MCPClientUpdateRequest`, and `MCPClientConfig`. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [x] Docs ## How to test ```sh # Core/Transports go test ./core/mcp/... ./framework/configstore/... # UI cd ui pnpm i pnpm build ``` **HTTP connection with a self-signed CA:** 1. Create an MCP client with `connection_type: http` and set `tls_config.ca_cert_pem` to a PEM-encoded CA certificate (or `env.MY_CA_CERT`). 2. Verify the client connects successfully to an MCP server using a certificate signed by that CA. **Insecure skip verify (development only):** 1. Create an MCP client with `connection_type: http` or `sse` and set `tls_config.insecure_skip_verify: true`. 2. Verify the client connects to an MCP server with an untrusted certificate and that a warning is logged. **Redaction:** 1. Create a client with a `ca_cert_pem` value. 2. Fetch the client via GET and confirm the PEM value is redacted. 3. Submit an update without changing the CA cert and confirm the original value is preserved. ## Breaking changes - [ ] Yes - [x] No ## Security considerations - `InsecureSkipVerify` disables TLS certificate verification entirely. A warning is logged when this option is used. It is documented as development/testing only and not recommended for production. - `CACertPEM` is treated as a sensitive credential: it is redacted in API responses and handled through the existing `EnvVar` redaction pipeline. - A minimum TLS version of TLS 1.2 is enforced on all custom TLS clients constructed by `buildTLSHTTPClient`. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
17 tasks
akshaydeo
added a commit
that referenced
this pull request
May 29, 2026
## Summary This PR releases **core v1.5.14**, **framework v1.3.14**, **transports v1.5.6**, and bumps all dependent plugins to their respective `.14` patch versions. It delivers a broad set of new capabilities across MCP authentication, key rotation, OTel metrics, Bedrock/Anthropic compatibility, and UI improvements, alongside a number of targeted bug fixes and refactors. ## Changes - **Direct API Key Header** — Providers can now receive an API key passed directly via a request header (#3817) - **MCP Per-User Auth** — Introduced `MCPCredentialStore` abstraction, per-user MCP credential reconciliation, and a new per-user header auth type with lazy-auth submission flow (#3656, #3702, #3703, #3704, #3705) - **MCP TLS Configuration** — Added configurable TLS (`insecureSkipVerify`, `caCertPem`) for HTTP/SSE MCP client connections (#3779, #3783) - **MCP Sessions Management** — Filter, search, and pagination on the MCP sessions list API and table, plus a `can_reauth` identity gate (#3823, #3824, #3825) - **Key Rotation** — Keys now rotate on 401/402/403 responses; returns `502 upstream_credentials_exhausted` when all keys are permanently exhausted. Added `triggered_rotation` to `KeyAttemptRecord` and tightened `bifrost_key_rotation_events_total` semantics (#3430, #3491) - **OTel Metrics** — Added OTel spec-compatible metrics (backward compatible) with provider cache and semantic cache attributes in metrics export (#3865, #3816) - **Opus 4.8 Support** — System message handling and general compatibility for Opus 4.8 (#3868, #3878) - **Dimension Rankings** — New `GetDimensionRankings` API and dashboard tabs for team, customer, BU, and user rankings (#3766) - **Model Pricing Attributes** — `additional_attributes` field on model pricing rows with management API and UI editor (#3829) - **Prompt Cache Retention** — Added prompt cache retention parameter on responses requests (#3810) - **Tool Call Execution UI** — Inline tool-call execution, stop streaming, bulk execute/submit, and a redesigned tool-call UI (#3837, #3843) - **Sheet Navigation** — Prev/next keyboard navigation and URL state across virtual key, MCP client, and routing rule sheets (#3739, #3740, #3744, #3745) - **Bedrock Tool Name Truncation** — Truncate Bedrock function/tool names to the provider length limit - **Bedrock Guardrails** — Set guardrail config in Bedrock requests built from responses (#3862) - **Anthropic Tool Use** — Default `tool_use` input to `{}` when arguments are absent (#3880) - **Responses Streaming** — Fixed responses stream events (#3838) - **Compat Flow** — Fixed missing parameter parsing on the compat flow (#3881) - **Passthrough API Version** — Set a default API version in passthrough requests as a fallback (#3853) - **Virtual Key Updates** — Avoid overriding optional fields during virtual key update (#3855) - **User-Mode Flows** — Gate user-mode flows on caller `user_id`, skip temp token mint, and unify flow/credential kind filtering for pending flows (#3841, #3859) - **Partial Tool Calls** — Handle partial tool call execution failures and return successful results (#3849) - **URL Query Escaping** — Support escaped characters in URL query parameters (#3826) - **MCP Auth Errors** — Inline banner and retry support for MCP auth-required errors (#3856) - **Renamed Resolvers** — `staticHeadersResolver`/`serverOAuthResolver` renamed to `sharedHeadersResolver`/`sharedOAuthResolver` (#3840) - **Starlark Nested Tool Calls** — Exposed `RunWithPluginPipeline` on `ClientManager` and routed Starlark nested tool calls through the canonical plugin gate (#3794) - **Deferred-Fill OAuth Removed** — Removed deferred-fill user-mode OAuth flow support (#3839) - **Go 1.26.3** — Upgraded toolchain to Go 1.26.3 (#3782) ## Type of change - [x] Bug fix - [x] Feature - [x] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go version # should report go1.26.3 go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` - Validate MCP per-user auth by configuring a per-user header auth type and confirming credentials are stored and reconciled on virtual key and MCP client changes. - Validate key rotation by triggering a 401/402/403 from an upstream provider and confirming rotation occurs; exhaust all keys and confirm a `502 upstream_credentials_exhausted` is returned. - Validate OTel metrics output includes `provider_cache` and `semantic_cache` attributes. - Validate Bedrock requests with tool names exceeding the provider limit are truncated correctly. - Validate Opus 4.8 system message handling by sending a request with a system message to an Opus 4.8 endpoint. ## Breaking changes - [x] Yes - [ ] No The deferred-fill user-mode OAuth flow has been removed (#3839). Any integrations relying on that flow must migrate to the new per-user credential store approach. The `staticHeadersResolver` and `serverOAuthResolver` identifiers have been renamed to `sharedHeadersResolver` and `sharedOAuthResolver` respectively (#3840); any direct references must be updated. ## Related issues #3817, #3656, #3702, #3703, #3704, #3705, #3779, #3783, #3823, #3824, #3825, #3430, #3491, #3865, #3816, #3868, #3878, #3766, #3829, #3810, #3837, #3843, #3739, #3740, #3744, #3745, #3862, #3880, #3838, #3881, #3853, #3855, #3841, #3859, #3849, #3826, #3856, #3840, #3794, #3839, #3782, #3724, #3814, #3836, #3869, #3886 ## Security considerations - MCP per-user credentials are stored via the new `MCPCredentialStore` abstraction; ensure the backing store is appropriately access-controlled and that credential values are encrypted at rest. - The direct API key header feature passes provider secrets via HTTP headers; ensure TLS is enforced on all ingress paths and that headers are not logged in plaintext. - User-mode flows are now gated on `caller user_id` and temp token minting is skipped where appropriate, reducing the surface for privilege escalation. - TLS configuration for MCP HTTP/SSE connections supports `insecureSkipVerify`; this should only be enabled in controlled environments. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
Merged
akshaydeo
added a commit
that referenced
this pull request
May 29, 2026
## ✨ Features - **Direct API Key Header** - Pass a provider API key directly via request header (#3817) - **MCP Per-User Authentication** - New per-user header auth type with credential storage and lazy-auth submission flow (#3703, #3704, #3705) - **MCP TLS Configuration** - Configurable TLS (insecureSkipVerify, caCertPem) for HTTP/SSE MCP client connections (#3779, #3783) - **MCP Sessions Management** - Filter, search, and pagination on the MCP sessions list API and table, plus a can_reauth identity gate (#3823, #3824, #3825) - **Tool Call Execution UI** - Inline tool-call execution, stop streaming, bulk execute/submit, and a redesigned tool-call UI (#3837, #3843) - **Dimension Rankings Dashboard** - New dashboard tabs for team, customer, BU, and user rankings, backed by a GetDimensionRankings API (#3766) - **Model Pricing Attributes** - additional_attributes on model pricing rows with management API and UI editor (#3829) - **Prompt Cache Retention** - Prompt cache retention parameter on responses requests (#3810) - **Opus 4.8 Support** - System message handling and compatibility for Opus 4.8 (#3878, #3868) - **Key Rotation** - Rotate keys on 401/402/403 and return 502 upstream_credentials_exhausted when all keys are permanently dead (#3491) - **OTel Metrics** - OTel spec compatible metrics plus provider and semantic cache attributes in metrics export (#3865, #3816) - **Sheet Navigation** - Prev/next keyboard navigation and URL state across virtual key, MCP client, and routing rule sheets (#3739, #3740, #3744, #3745) - **Go 1.26.3** - Upgraded toolchain to Go 1.26.3 (#3782) ## 🐞 Fixed - **Bedrock Tool Names** - Truncate Bedrock function/tool names to the provider length limit - **Bedrock Guardrails** - Set guardrail config in Bedrock request built from responses (#3862) - **Anthropic Tool Use** - Default Anthropic tool_use input to {} when arguments are absent (#3880) - **Responses Streaming** - Fixed responses stream events (#3838) - **Compat Flow** - Fixed missing parameter parsing on the compat flow (#3881) - **Passthrough API Version** - Set a default API version in passthrough requests as a fallback (#3853) - **Virtual Key Updates** - Avoid overriding optional fields during virtual key update (#3855) - **User-Mode Flows** - Gate user-mode flows on caller user_id, skip temp token mint, and unify flow/credential kind filtering for pending flows (#3841, #3859) - **Partial Tool Calls** - Handle partial tool call execution failures and return successful results (#3849) - **URL Query Escaping** - Support escaped characters in URL query parameters (#3826) - **MCP Auth Errors** - Inline banner and retry support for MCP auth-required errors (#3856) - **JSON Editor Height** - Cap JSON editor max height at 400px in message views (#3842)
18 tasks
18 tasks
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds TLS configuration support for HTTP and SSE MCP client connections, allowing users to connect to MCP servers that use self-signed or private CA certificates, or to disable TLS verification in development/testing environments.
Changes
MCPTLSConfigschema withInsecureSkipVerifyandCACertPEMfields.CACertPEMsupportsenv.VAR_NAMEsyntax for reading certificates from environment variables.InsecureSkipVerifytakes priority overCACertPEMwhen both are set.buildTLSHTTPClienthelper onMCPManagerthat constructs a custom*http.Clientwith the appropriate TLS configuration. Returnsnilwhen no TLS config is provided so the library default is used.CACertPEMis treated as a sensitive value and is redacted in API responses, with redaction-aware merge logic to preserve the raw value on update when the incoming value matches the previously redacted one.TLSConfigis persisted as a JSON column (tls_config_json) in the database via a new migration and included in the MCP client config hash for change detection.EnvVarInputtextarea for the CA certificate PEM.MCPClientCreateRequestBase,MCPClientUpdateRequest, andMCPClientConfig.Type of change
Affected areas
How to test
HTTP connection with a self-signed CA:
connection_type: httpand settls_config.ca_cert_pemto a PEM-encoded CA certificate (orenv.MY_CA_CERT).Insecure skip verify (development only):
connection_type: httporsseand settls_config.insecure_skip_verify: true.Redaction:
ca_cert_pemvalue.Breaking changes
Security considerations
InsecureSkipVerifydisables TLS certificate verification entirely. A warning is logged when this option is used. It is documented as development/testing only and not recommended for production.CACertPEMis treated as a sensitive credential: it is redacted in API responses and handled through the existingEnvVarredaction pipeline.buildTLSHTTPClient.Checklist
docs/contributing/README.mdand followed the guidelines