Add untrusted mode: MCP servers that never possess credentials#5914
Draft
JAORMX wants to merge 9 commits into
Draft
Add untrusted mode: MCP servers that never possess credentials#5914JAORMX wants to merge 9 commits into
JAORMX wants to merge 9 commits into
Conversation
rdimitrov
marked this pull request as ready for review
July 22, 2026 15:10
rdimitrov
requested review from
ChrisJBurns,
amirejaz,
aponcedeleonch,
blkt,
jerm-dro,
jhrozek,
rdimitrov,
reyortiz3 and
tgrunnagle
as code owners
July 22, 2026 15:10
JAORMX
marked this pull request as draft
July 22, 2026 15:11
JAORMX
force-pushed
the
feat/untrusted-mcp-egress-broker
branch
from
July 22, 2026 15:11
a77cc62 to
4ab4ade
Compare
Lays the security groundwork for the untrusted-MCP-server egress credential broker (ADR-0001): four independent hardening mechanisms that close gaps the broker design depends on. Read-side binding validation: upstream token storage now validates caller-asserted UserID/ClientID before releasing tokens, making the documented ErrInvalidBinding contract real. Binding is checked before expiry so a mismatched row never releases refresh material; bulk reads exclude mismatched rows per-provider. Strict mode added for untrusted workloads to fail closed on legacy rows. Operator untrusted-env gate: annotated MCPServer workloads (toolhive.stacklok.dev/untrusted=true, interim until the spec field lands) are rejected at reconcile time when the backend container would receive credentials via env valueFrom (Secret/ConfigMap), envFrom, or Secret/ConfigMap/projected/CSI volumes. Terminal error surfacing with latch-clearing on recovery so warnings re-arm. Token encryption at rest: upstream OAuth tokens in Redis are now AES-256-GCM envelope-encrypted with per-record DEKs, KEK keyring with rotation (lazy re-seal on read), and ciphertext bound to the Redis key via AAD to defeat cut-and-paste row swaps. Legacy plaintext rows read through unchanged; encrypted fleet without a keyring fails loudly. Operator RBAC: staged networkpolicies permissions for the Wave 3 egress lockdown (deploy-time pre-staging rationale in ADR).
Implements the untrusted-MCP-server architecture (ADR-0001) across three waves: an MCP server flagged untrusted never possesses a credential — the operator forbids secret-sourced env, injects sentinel literals, and every upstream call egresses through a per-pod Envoy sidecar that terminates TLS and injects the user's own OAuth credential at the boundary. Wave 1 - CRD surface: MCPServer.spec.untrusted and spec.egressPolicy (provider-keyed host/method/path constraints) with CEL admission rules: untrusted requires egress policy, group membership, no secrets, no podTemplateSpec, ClientIP affinity, no backendReplicas. Sentinel env injection with forgery/collision validation. Wave 2 - single-tenant lifecycle: vMCP session layer provisions one backend pod per (user, MCPServer) via a PodLifecycle that clones the operator-built StatefulSet template (fail-closed re-verification of secret material), deterministic pod names for cross-replica restore, Redis-ledger DoS admission (per-user quota, per-server and global caps, fail closed), and a reaper for idle/timeout/zombie GC. Session restore recreates missing pods with an extended budget. Wave 3 - egress broker: Envoy sidecar per untrusted pod with SDS TLS-bump (per-tenant ECDSA CA, generation-named rotation) and a Go ext_authz service that resolves pod-to-user from annotations and injects credentials only after destination binding (D5), never following redirects, with per-dial IP allowlisting (D7). Operator manages CA Secrets, policy ConfigMap, and default-deny NetworkPolicy (loopback + cluster DNS + resolved destinations only). Review-hardened: quota ledger verified, bootstrap SDS wiring proven, CA rotation made race-free, DNS egress pinned to cluster DNS, transient/terminal error split, contract package shared across the pod boundary.
When a user has not consented an upstream provider, all three surfaces that can fail now produce an actionable response carrying the provider name and authorize endpoint, completing the ADR-0001 D8 requirement in both subsystems. Proxy path: the upstreamswap 401 keeps its RFC 6750 challenge and gains a JSON body (error, provider, authorize_url) clients can render as a connect-your-account prompt. vMCP path: upstream_inject returns a typed ConsentRequiredError that unwraps to ErrUpstreamTokenNotFound (classification preserved, errors.Join keeps ErrAuthenticationFailed), fails fast before any backend dispatch, and surfaces to the agent as a tool-result error with a machine-readable UPSTREAM_CONSENT_REQUIRED marker plus JSON payload. Post-consent retry is client-driven: the next request's per-request identity enrichment picks up the new token, no server-side watch. The sidecar's 403 deny text deliberately carries no URL — consent guidance never traverses the untrusted server.
Supply chain: Envoy sidecar pinned to envoyproxy/envoy:v1.36.2 with a real sha256 digest; broker image pinned without :latest; image overrides (THV_UNTRUSTED_ENVOY_IMAGE/BROKER_IMAGE) resolve once at the composition root, with non-digest overrides warned and :latest overrides fatal. Pod hardening: sidecar containers get bounded resource requests/limits (multiplier overrides NaN/Inf/>100 rejected), and the broker exposes a loopback-only /healthz (Redis reachable, policy loaded, CA within validity) driving readiness/liveness probes. Operations: lifecycle tunables (idle TTL, per-user quota, per-server cap, global ratio, readiness timeout) surface as startup-validated THV_UNTRUSTED_* env vars. Token encryption: VirtualMCPServer authServerConfig gains a tokenEncryption field (activeKeyId + keySecretRef, CEL-validated to require redis storage and a non-empty ref). The operator resolves all data keys in the referenced Secret, renders one SecretKeyRef env per key so rotation keeps retired keys decryptable, watches the Secret for rotation rollout, and warns when tokenEncryption is set with Sentinel storage (unsupported). The full key set flows to the egress broker so its keyring tracks the auth server's, closing the key-ID drift that would have denied all injections on rotation. Also guards reserved env prefixes (THV_UNTRUSTED_*, TOOLHIVE_AUTHSERVER_*) against user overrides via vMCP podTemplateSpec.
Closes the last credential channel (ADR-0001 D6c): an upstream response that echoes the injected credential back to the untrusted server is now suppressed. An Envoy ext_proc filter sends response headers and buffered bodies to the broker, which scans for the exact and base64 forms of the token it injected (correlated via Envoy-generated x-request-id through a Lua metadata filter) and replaces matches with a generic 502. Fail-open by default with a THV_EGRESSBROKER_SCAN_FAIL_CLOSED opt-in that also covers unknown request IDs and oversize bodies in-band. Byte-exact scanning is a tripwire, not a proof: encoding transforms evade it, and the ADR now says so — the destination allowlist remains the real boundary. The broker now emits structured audit events for every injection, denial (with reason), and suppressed leak — sub-hashed, never token material — plus low-cardinality OTel counters for scan results, denials, and injections (D11). Review-hardened: correlation metadata actually substituted (Lua filter, not literal %REQ% route metadata), fail-closed posture wired in-band, graceful-stop bounded with a hard-stop fallback, explicit OVERWRITE_IF_EXISTS_OR_ADD on credential injection, deny-reason split asserted, TokenMap race-tested under flood and same-ID contention, and the session-Redis password plumbed to the broker as a SecretKeyRef.
Consolidate the private-IP classifier: the egress broker's dial allowlist now uses networking.IsPrivateIP instead of a second local table, gaining NAT64 embedded-IPv4 decoding (64:ff9b::/96, 64:ff9b:1::/48) so a crafted DNS answer can no longer smuggle the cloud metadata endpoint past D7 on NAT64 networks. The shared classifier absorbs the two benchmarking prefixes the egress table had, making the merged set a strict superset. Render the Envoy bootstrap as data: all config-derived values (vhosts, routes, ext_authz address, scan posture) go through yaml.Marshal instead of string substitution, matching the operator side and removing the reliance on cross-package validation alone. TokenMap rebuilt on hashicorp/golang-lru/v2 (already a dependency), keeping consume-on-read semantics via Peek+Remove under the map mutex; the hand-rolled container/list machinery is gone. Converge tokenenc on pkg/secrets/aes: new EncryptWithAAD/ DecryptWithAAD helpers carry the nonce|ct|tag convention and size bound; tokenenc deletes its duplicated GCM boilerplate and calls them, keeping the redis-key AAD cut-and-paste defense. Also: slices.Contains/Sort/ContainsFunc in the policy hot path, and cross-referenced drift-guard comments between the operator and vMCP secret-material gate test suites.
End-to-end testing against a real cluster surfaced a chain of defects in the untrusted pod data plane that unit tests could not reach. All were reproduced, fixed, and re-verified live: the pod now comes up 3/3 and a malicious-server attack playbook is fully blocked (sentinel env, direct-egress 403, proxy-to-evil denied, echo attack suppressed, DNS exfil refused). Health/probe contract: the broker health listener bound 127.0.0.1 but kubelet probes reach the pod IP, so every probe failed and liveness killed the broker at the threshold before it could initialize. Bind all interfaces, and split /livez (dependency-free, served from process start) from /healthz (readiness gated on CA/policy/Redis) so a slow Redis/CA init can no longer kill a healthy broker mid-startup. Envoy bootstrap: three render bugs — the typed bootstrapDoc dropped the SDS-required node id/cluster on re-marshal, terminate_connect is a v1.37 field the pinned Envoy v1.36 rejects, and with_request_body max_request_bytes 0 fails proto validation. Also removed the Envoy/ broker startup race by making Envoy wait for the rendered bootstrap. Token store reachability: NetworkPolicy egress is evaluated against the post-DNAT pod IP, not the Service ClusterIP, so the generated policy never matched the in-cluster token store and the broker could not reach Redis. Resolve the Redis Service's EndpointSlice pod IPs into the egress allowlist (external Redis falls back to DNS), add the discovery.k8s.io/endpointslices RBAC, and route the broker's token-store dial around the D7 destination guard (it is broker infrastructure, not credential egress). Also: ca-seed init no longer needs a shell (the image is distroless); the broker gains a seed-ca mode. Operator ClusterRole gains pods create/delete so the untrusted lifecycle can be granted to vMCP. Deterministic fix for the pre-existing TestTokenMap_ConcurrencyFlood flake (racy oldest-eviction assertion under a concurrent flood).
Codespell: redeclared (not re-declared) and unparsable (not unparseable) across the untrusted-mode packages and docs; add userA to the codespell ignore list (false positive on a test identifier). The untrusted egress tests were flaky under parallel runs: reconcileOnce installed the package-level DNS stub per call via untrustedDNSTestLock, whose re-entry check treats any installed stub (including a parallel sibling's) as "held" and skips the mutex. A multi-reconcile test could then overwrite another test's stub mid-flight, and a cleared stub sent the next reconcile to real DNS (api.github.com fails offline). Install the fixture stub exactly once per test, holding the mutex for the test's whole lifetime; parallel siblings block until cleanup. Green at -count=8 (previously failed at -count=3).
JAORMX
force-pushed
the
feat/untrusted-mcp-egress-broker
branch
from
July 23, 2026 06:06
92636af to
c6d4513
Compare
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
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
You want to run a third-party MCP server you don't trust. Today, that means handing it real credentials — in its environment, in files, or in request headers — and hoping. This PR adds untrusted mode: an MCP server that never possesses a credential at all. Not a long-lived one, not a short-lived one, not even for a millisecond.
The experience, by persona:
The end user notices almost nothing. You ask your agent to use the GitHub MCP server. Behind the scenes, ToolHive spins up a backend pod just for you, and your GitHub token is injected into your API calls at the cluster's egress boundary — the server running your request never sees it. First time using a provider? The tool result comes back with a "connect your account" link instead of a cryptic failure; you click, consent, retry, done.
The cluster operator gets a declarative, admission-guarded API. Two blocks on the MCPServer and the platform handles the rest:
Invalid configurations (secrets on the backend, custom pod templates, missing group) are rejected at apply time, not discovered at 2am. The operator provisions and rotates the TLS CA, generates NetworkPolicies (default-deny egress: loopback, cluster DNS, and the allowlisted destinations only), and exposes four alert-worthy signals: quota-denied admissions, pod-fan-out approaching caps, credential-echo suppression events, and consent-flow breakage.
The attacker running inside the compromised MCP server finds a pod with a fake token (
GITHUB_TOKEN=thv-untrusted-sentinel:github), no neighbors (one user per pod), no network (everything but the sidecar is denied), and no way to name another user's session — the token-selection key never crosses their process in any form. Calls to non-allowlisted hosts are denied before any credential is loaded. Responses that echo the injected credential back are replaced with a generic 502. Every injected and denied call lands in a structured audit log.The MCP server author changes nothing: the token env var contains a sentinel, make the API call anyway, the platform injects the real credential.
What this defeats: credential theft, replay, cross-user use, and post-hoc use. What it does not defeat (documented loudly in the ADR and user docs): authority abuse within the granted scopes for the current call, data exfiltration through MCP responses, and encoding-transform evasion of the byte-exact response scanner. Least-privilege OAuth scopes, method/path ACLs, and the audit trail are the mitigations.
Type of change
Test plan
task test) — full suite with-racegreen, including new packages (pkg/egressbroker,pkg/vmcp/session/untrusted)task lint-fix) — cleancmd/thv-operator/test-integration/mcp-server-untrusted,mcp-external-auth;pkg/vmcp/session/untrustedintegration-tagged suite) — greenManual: matlatl docs link-graph check clean. Not yet run: the chainsaw e2e scenarios (
test/e2e/chainsaw/operator/untrusted-egress,validation/mcpserver-untrusted) and a live-Envoy end-to-end through the TLS-bump path — both need a Kind cluster. These are wired into CI config but are the known verification gap before merge; flagged for reviewers.API Compatibility
v1beta1APIAll CRD changes are additive-with-defaults:
MCPServer.spec.untrusted(default false),spec.egressPolicy(optional),AuthServerStorageConfig.tokenEncryption(optional). CEL rules gate onlyuntrusted=trueworkloads; trusted workloads are byte-identical in behavior.Changes
cmd/thv-operator/api/v1beta1untrusted+egressPolicyfields with CEL R1–R6;tokenEncryptionon auth-server storagecmd/thv-operator/controllerspkg/vmcp/session/untrustedpkg/egressbroker+cmd/thv-egressbrokerpkg/authserver/storage,pkg/authserver/tokenencpkg/auth/upstreamswap,pkg/vmcp/authUPSTREAM_CONSENT_REQUIREDerrors with authorize URL in both subsystemsdocs/arch/15-untrusted-mode.md,docs/operator/untrusted-mode.md,docs/arch/adr/0001Does this introduce a user-facing change?
Yes — a new opt-in mode for running untrusted MCP servers on Kubernetes. Cluster operators mark an MCPServer
untrusted: truewith anegressPolicy; end users keep their upstream OAuth tokens entirely out of the MCP server's reach, with a guided consent flow when a provider isn't connected yet. Seedocs/operator/untrusted-mode.md.Special notes for reviewers
pkg/egressbroker/injector.go); (3) token selection never influenced by anything the server emits (single-tenant pods + pod-annotation identity); (4) fail-closed everywhere except the documented scanner fail-open default.TestTokenMap_ConcurrencyFloodflake (racy assertion, fails on baseline too — follow-up candidate).