Skip to content

feat(mcp): trust private-network server certificates on first use - #60041

Open
karup wants to merge 3 commits into
anomalyco:devfrom
karup:mcp-trusted-hosts
Open

karup wants to merge 3 commits into
anomalyco:devfrom
karup:mcp-trusted-hosts

Conversation

@karup

@karup karup commented Oct 28, 2026 •

Copy link
Copy Markdown

Issue for this PR

Part of #40111 — trust-on-first-use for private-network MCP servers.

Stacked on #60039 (per-server tls config); this diff includes that PR until it lands, then shrinks to the trust-on-first-use portion.

What does this PR do?

For a remote MCP server whose URL is a private IP address (RFC 1918, loopback, link-local) with no tls config, the first connect fails with the presented certificate's fingerprint, subject, and the exact command to trust it. opencode mcp trust <url> [--force] shows the certificate, asks for confirmation, and pins it for the project (machine-local store, per project). Later connects verify the server presents exactly that certificate; a rotated certificate fails until re-pinned with --force. Hostnames and public IPs never auto-trust; an explicit tls config takes precedence.

Adds the mcp_trusted_host table and migration, the mcp.trust.inspect / mcp.trust.pin endpoints with regenerated client, and the CLI command.

How did you verify your code works?

  • 33 new core tests (IP classifier, per-project store, first-use fail → trust → reconnect, rotation + --force, hostname/public-IP rejection, TOCTOU guard), 2 end-to-end server tests over real HTTP, CLI command tests
  • Typecheck (tsc) across all touched packages, root lint, protocol/client codegen clean

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Added by the opencode bot (AI-generated).

Related: #40111, #57012

Public API: adds the opencode mcp trust <url> [--force] CLI command, the experimental /api/experimental/mcp/trust endpoints (client.experimental.mcp.trust.inspect / .pin), Mcp.TrustedHost/TrustInspect/TrustResult schemas and the mcp_trusted_host table (backwards compatible)

karup added 2 commits October 28, 2026 14:54
Remote MCP servers can now carry a tls config with a CA file, inline CA PEM, or a pinned SHA256 certificate fingerprint, scoping trust to that server only. System verification is unchanged for everything else.
Remote MCP servers at private IP addresses can now be trusted on first use instead of requiring a tls config: the first connect fails with the presented certificate's fingerprint and the command to trust it; opencode mcp trust <url> [--force] pins the certificate for the project; later connects verify the pinned certificate, and rotations fail until re-pinned with --force. Hostnames and public IPs never auto-trust; an explicit tls config takes precedence.

Adds the mcp_trusted_host table (per project, machine-local), the mcp.trust.inspect and mcp.trust.pin endpoints, and the CLI command. Builds on the per-server tls config from the previous pull request.
@github-actions github-actions Bot added the needs:compliance This means the issue will auto-close after 2 hours. label Oct 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This PR doesn't fully meet our contributing guidelines and PR template.

What needs to be fixed:

  • PR description is missing required template sections. Please use the PR template.

Please edit this PR description to address the above within 72 hours, or it will be automatically closed.

If you believe this was flagged incorrectly, please let a maintainer know.

@opencodejr opencodejr Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Trust-on-first-use works for the PR's own test certificate, but not for the certificates it is meant for: trusting a typical appliance certificate still leaves the server unable to connect. I ran the core flow against real TLS servers, including one with a self-signed CN=OPNsense certificate without IP subject alternative names (SANs). I also ran the new core, server and CLI tests (all pass) and tsc in core, server and cli (clean). I reviewed only the trust-on-first-use commit and left the tls config part to #60039.

Public API (all additions, backwards compatible):

  • CLI command opencode mcp trust <url> [--force].
  • HTTP endpoints POST /api/experimental/mcp/trust/inspect and POST /api/experimental/mcp/trust, in the OpenAPI spec.
  • Client methods mcp.trust.inspect and mcp.trust.pin.
  • Schemas Mcp.TrustedHost, Mcp.TrustInspect and Mcp.TrustResult.
  • New database table mcp_trusted_host with a migration.

Concerns, see the inline comments:

  1. A pinned certificate without the IP in its subject alternative names still fails to connect (ERR_TLS_CERT_ALTNAME_INVALID), and most appliance certificates are like that.
  2. Unrelated connection failures on private IPs are reported as "certificate not trusted", which hides the real error.
  3. --force also skips the confirmation, including when re-pinning a changed certificate.

A test with a certificate that has no IP SAN would cover the main case. The existing tests only use a certificate with IP:127.0.0.1.

Related: #40111, #57012

`TLS certificate for ${host}:${McpTls.port(url)} changed since it was last trusted: expected ${stored.fingerprint}, got ${cert.fingerprint}. If the appliance rotated its certificate, re-pin it with: opencode mcp trust ${config.url} --force`,
),
)
return { ...config, tls: { ca_pem: cert.pem } }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using the pinned certificate as a ca_pem trust anchor only works if it lists the server's IP in its subject alternative names. Bun's fetch still checks that the names match. I tried a self-signed CN=OPNsense certificate without SANs, the kind most appliances ship: mcp.trust succeeds, but the reconnect fails with ERR_TLS_CERT_ALTNAME_INVALID. explainFailure then returns the raw error, since the stored fingerprint matches. The fixture certificates include IP:127.0.0.1, so the tests don't catch this. You have already checked the exact fingerprint, so the name check could be replaced by an exact-fingerprint comparison on this connection, for example a checkServerIdentity that compares fingerprint256. That would also remove the gap between the probe and the real connection. #60039's fingerprint mode has the same issue.

This rewrites every failed connect to a private-IP https server without a stored pin into "TLS certificate … is not trusted", whatever the actual error was. With the CA trusted through NODE_EXTRA_CA_CERTS and a server that returns HTTP 500, the status says the certificate isn't trusted and suggests mcp trust. The real error is lost. It also opens an extra TLS probe on every failure. Could you only rewrite the message when the error is a certificate verification failure (e.g. DEPTH_ZERO_SELF_SIGNED_CERT, SELF_SIGNED_CERT_IN_CHAIN, UNABLE_TO_VERIFY_LEAF_SIGNATURE, ERR_TLS_CERT_ALTNAME_INVALID), and keep the original message otherwise?

}
log.warn(`The certificate changed: the previously trusted fingerprint was ${stored.fingerprint}`)
}
if (!force) yield* requireInteractive("Pass --force when trusting a certificate without an interactive terminal")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

--force does two things here: it allows re-pinning a changed certificate, and it skips the confirmation, even in an interactive terminal. A changed certificate is exactly when a tampered connection is most likely, yet that is the case that never asks. The description and docs say the command asks for confirmation. Consider still prompting when a terminal is available and only skipping the prompt when there isn't one, or keeping 're-pin' and 'skip the prompt' as separate flags.

# Conflicts:
#	packages/client/src/effect/api/api.ts
#	packages/client/src/effect/generated/client.ts
#	packages/client/src/promise/generated/client.ts
#	packages/client/src/promise/generated/types.ts
#	packages/core/schema.json
#	packages/core/src/database/migration.gen.ts

@opencodejr opencodejr Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The new push only merges dev, so neither earlier thread is addressed yet: a pinned appliance certificate without the IP in its SANs still fails to connect, unrelated errors are still reported as untrusted certificates, and --force still skips the confirmation. The trust tests in core, server and protocol pass, and tsc is clean in core, server, cli and client.

What the merge changes: after #59072, the regenerated client lists these endpoints under client.experimental.mcp.trust.inspect / .pin, and client.mcp.trust.* is generated only as a deprecated alias, "Removed in the next minor". packages/cli/src/commands/handlers/mcp/trust.ts still calls client.mcp.trust.inspect and client.mcp.trust.pin, so opencode mcp trust will break when the aliases are removed. Please switch it to client.experimental.mcp.trust.*.

Public API (additions, backwards compatible): the opencode mcp trust <url> [--force] CLI command; POST /api/experimental/mcp/trust/inspect and POST /api/experimental/mcp/trust, which the SDK now exposes as client.experimental.mcp.trust.inspect / .pin; the Mcp.TrustedHost, Mcp.TrustInspect and Mcp.TrustResult schemas; and the mcp_trusted_host table.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs:compliance This means the issue will auto-close after 2 hours.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant