feat(opencode): Allow per-MCP-server trust configuration - #40125
Open
karup wants to merge 26 commits into
Open
Conversation
Add per-MCP-server TLS trust configuration supporting custom CA files, embedded CA certificates, and certificate fingerprint pinning. - Add Tls config schema (caFile, caPem, fingerprint) to V1 and V2 MCP configs - Build TLS CA from config and inject via custom fetch into transports - Validate PEM certs, fingerprint format, and file type before use - Pre-flight TLS connection for fingerprint verification with cert pinning - TLS trust is scoped per-fetch-instance; does not affect global TLS
…w findings Extract duplicated TLS config assembly (caPem validation + caFile read + fingerprint verify) into a single buildTlsCa() async helper, eliminating 3 identical ~20-line blocks across connectRemote, startAuth, and McpDebugCommand. Also address code review findings: - Fix test any types with proper RequestInit intersection - Deduplicate fingerprint normalization (validate once, pass through) - Add parseInt radix (use Number() instead) - Add derToPem and buildTlsCa unit tests - Pre-compute tls options object in createTlsFetch to reduce per-request allocations - Export derToPem for testability - Add security doc comment noting custom CA follows redirects
Use openSync + fstatSync + readFileSync(fd) to stat and read the same inode, eliminating the TOCTOU gap between existsSync/statSync and readFileSync where a file swap could occur. Also: - Add security doc comment to resolveFilePath about path traversal being by design - Replace for loop with Uint8Array.from in derToPem test
Contributor
|
Hey! Your PR title Please update it to start with one of:
Where See CONTRIBUTING.md for details. |
Add TLS Trust section with three options (caFile, caPem, fingerprint) and examples for each. Users can now discover per-server TLS trust from the docs.
readCaFile trims trailing whitespace, so match against trimmed cert in expectations. Fix derToPem body line extraction to use lines.slice(1, -2). Fix concatenation test to use valid PEM for caPem instead of plain text.
karup
changed the title
karup
changed the title
6 tasks
6 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
1 participant
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.
Issue for this PR
Closes #40111
Also partially addresses
#23506
Rather than disabling verification globally, fingerprint pinning achieves the same goal — trusting a specific self-signed cert — without insecure: true. caFile handles private CAs cleanly without a blanket bypass.
#14696
#26862
The createTlsFetch() wrapper and per-server TLS config injection proves the transport-layer plumbing needed for mTLS. Adding cert/key fields to the tls config block follows the same pattern — caFile validates PEM; cert+key would too.
#1694
Provides a portable alternative. Instead of relying on OS-specific trust stores, caFile: "~/ca.pem" lets users point to their corporate CA explicitly, working identically across macOS/Linux/Windows without NODE_EXTRA_CA_CERTS.
Type of change
What does this PR do?
Adds per-MCP-server TLS trust configuration for remote MCP servers using self-signed certificates or private CAs. Three options:
caFile(path to CA cert),caPem(inline CA cert), andfingerprint(SHA256 certificate pinning, like SSH host keys).Config schema is added to both V1 and V2 MCP configs. A
buildTlsCa()helper reads files, validates PEM content, and verifies fingerprints, thencreateTlsFetch()injects the trusted CA into a custom fetch wrapper scoped to that transport only — no global TLS changes.Fingerprint verification uses a one-time pre-flight TLS socket to retrieve the server cert, compares it against the user-provided fingerprint, and promotes the verified cert to a trusted CA. Trust applies only to that transport instance and does not leak across MCP servers.
How did you verify your code works?
buildTlsCaintegrationif (mcp.tls)prevents any new allocations or yields when TLS is not configured)Screenshots / recordings
Not a UI change.
Checklist