Skip to content

Allow opt-in private endpoints for remote URLs - #6195

Merged
amirejaz merged 1 commit into
stacklok:mainfrom
premctl:allow-private-endpoint
Aug 20, 2026
Merged

amirejaz merged 1 commit into
stacklok:mainfrom
premctl:allow-private-endpoint

Conversation

@premctl

@premctl premctl commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

ValidateRemoteURL unconditionally rejects private/loopback IPs and internal hostnames (cluster.local, kubernetes.default*, localhost, metadata.google.internal) with no opt-in. That blocks a legitimate topology: a Virtual MCP reaching a co-located in-cluster backend in-mesh, where going through an external address would strip the Istio/SPIFFE workload identity the backend's authorization policy keys on. The design (per-resource opt-in field + exact carve-out below) was agreed with @aponcedeleonch in #5784.

  • Add allowPrivateEndpoint (bool, default false) to both MCPServerEntry and MCPRemoteProxy specs, and change ValidateRemoteURL to take a ValidateRemoteURLOptions carrying the flag, threaded from both controllers.
  • With the flag on, only the private-network allowance widens: RFC-1918 (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16), IPv6 ULA (fc00::/7), and hostnames ending in cluster.local (.svc names were never blocked, since no DNS resolution is performed).
  • The SSRF protections stay unconditional regardless of the flag: link-local (169.254.0.0/16, fe80::/10), loopback/unspecified (127.0.0.0/8, ::1/128, ::/128, 0.0.0.0/8), localhost, metadata.google.internal, and all kubernetes.default* variants — including kubernetes.default.svc.cluster.local, which is checked before the cluster.local relaxation can apply. IPv4-mapped IPv6 forms are normalized before matching on both the blocked and relaxed sides.
  • With the flag off (the default), behavior is byte-for-byte identical to today.

Fixes #5784

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe):

Test plan

  • Unit tests (task test, task operator-test) — table-driven coverage of every carve-out category with the flag on and off, including the never-relaxed set staying blocked with the flag on and IPv4-mapped IPv6 forms; controller tests for both MCPServerEntry and MCPRemoteProxy proving the CR field flows through to validation (private URL rejected with flag unset, accepted with flag true)
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)

API Compatibility

  • This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Additive optional field only (allowPrivateEndpoint, defaults to false).

Does this introduce a user-facing change?

Yes. MCPServerEntry and MCPRemoteProxy gain an optional spec.allowPrivateEndpoint field (default false). When set, the remote URL may point at a private/in-cluster endpoint (RFC-1918, IPv6 ULA, cluster.local/.svc hostnames) — intended for reaching a co-located in-cluster backend in-mesh so workload-identity authorization still applies. Loopback, link-local, cloud-metadata, and kubernetes.default* endpoints remain blocked regardless of the flag.

Implementation plan

Approved implementation plan (agreed in #5784)
  • Add allowPrivateEndpoint bool (default false) to both MCPServerEntry and MCPRemoteProxy CRD specs.
  • Change ValidateRemoteURL(rawURL string) error to take an options parameter (rather than the bare string) carrying this flag, and thread it through from both mcpserverentry_controller.go and mcpremoteproxy_controller.go.
  • Exact carve-out:
    • Relaxable when allowPrivateEndpoint: true: RFC-1918 (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16), IPv6 ULA (fc00::/7), hostnames ending in cluster.local or .svc.
    • Never relaxed, regardless of the flag: link-local (169.254.0.0/16, fe80::/10), loopback/unspecified (127.0.0.0/8, ::1/128, 0.0.0.0/8), localhost, metadata.google.internal, kubernetes.default*. Loopback staying blocked is explicitly the point that matters most — a pod reaching loopback hits itself, which is never a legitimate co-located backend.
  • Regenerate CRD schema/deepcopy/YAML and docs.

Special notes for reviewers

  • Hostname check ordering is load-bearing: the always-blocked names (including kubernetes.default.svc.cluster.local) are evaluated before the cluster.local relaxation, so opting in cannot expose the API server's DNS names.
  • One inherent consequence of the agreed carve-out worth stating explicitly: with the flag on, the kubernetes.default* hostname block cannot protect the API server from being addressed by its ClusterIP directly (e.g. 10.96.0.1 is an RFC-1918 address the flag allows). The hostname block should not be read as API-server protection once a resource opts in — the flag is a deliberate, per-resource trust statement about the private network.
  • .svc-suffixed hostnames (other than kubernetes.default.svc) were already allowed before this change (the validator does no DNS resolution), so the flag does not change their handling; the "relaxable" list documents intent rather than adding a new block.
  • zz_generated.deepcopy.go is unchanged — a plain bool needs no generated deepcopy code. CRD YAML and docs/operator/crd-api.md are regenerated.
  • A stale comment in test/e2e/thv-operator/virtualmcp/mcpremoteproxy_scaling_test.go claiming MCPRemoteProxy "never" targets in-cluster servers is updated (comment-only).

Generated with Claude Code

ValidateRemoteURL unconditionally rejected private and in-cluster
endpoints, blocking the legitimate case of a vMCP reaching a
co-located in-cluster backend in-mesh, where routing through the
mesh preserves Istio/SPIFFE workload identity for the backend's
authorization policy.

Add an allowPrivateEndpoint field to MCPServerEntry and
MCPRemoteProxy that relaxes only the private-network checks:
RFC 1918 ranges, IPv6 unique-local addresses, and hostnames
ending in cluster.local. Loopback, link-local, cloud-metadata,
localhost, and kubernetes.default endpoints stay blocked
regardless of the flag, and behavior with the flag unset is
unchanged.

Signed-off-by: Prem Kumar Sompura <prem_sompura@hotmail.com>
@premctl
premctl force-pushed the allow-private-endpoint branch from 6b7ffbc to 3e53d21 Compare August 5, 2026 11:35

@amirejaz amirejaz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed with a security focus and cross-checked against the carve-out agreed in #5784 — the never-relaxed set, the ordering (kubernetes.default* blocked before the cluster.local relaxation), the IPv4-mapped IPv6 normalization, and the byte-for-byte-unchanged flag-off behavior all check out. Looks good, approving.

One check for @aponcedeleonch before merge: you'd listed .svc as relaxable (blocked unless opted in), but the PR leaves bare .svc allowed in both flag states (documented as never-blocked since no DNS resolution happens). The reasoning is sound — gating it would break the flag-off-unchanged contract and the existing .svc e2e workaround — but are you good with that, or do you want .svc genuinely behind the flag?

@github-actions github-actions Bot added the size/M Medium PR: 300-599 lines changed label Aug 5, 2026
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.54839% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.46%. Comparing base (aa3f5b3) to head (3e53d21).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
cmd/thv-operator/pkg/validation/url_validation.go 92.59% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6195      +/-   ##
==========================================
- Coverage   72.51%   72.46%   -0.06%     
==========================================
  Files         739      739              
  Lines       76719    76722       +3     
==========================================
- Hits        55634    55595      -39     
- Misses      17107    17161      +54     
+ Partials     3978     3966      -12     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@premctl

premctl commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

@aponcedeleonch — thanks @amirejaz for the review. The one open call before merge is whether bare .svc stays never-blocked (its current behavior — allowed in both flag states) or gets gated behind allowPrivateEndpoint.

My recommendation is to leave it never-blocked:

  • Backward-compat: bare .svc was never on the blocklist to begin with — only kubernetes.default.svc is — so it's already allowed today. Gating it would newly block it when the flag is off, breaking the "flag-off ⇒ byte-for-byte unchanged" contract.
  • It wouldn't add real security: validation does no DNS resolution, so an in-cluster service reachable via foo.ns.svc is equally reachable via foo.ns (same ClusterIP). Gating .svc blocks one spelling of the same target, not the target — you'd just use the shorter name.
  • It keeps the existing .svc e2e workaround working.

The never-relaxed set (localhost, kubernetes.default*, metadata, loopback/link-local) stays hard-blocked in both states, so nothing SSRF-critical changes.

That said, if you'd rather have .svc genuinely behind the flag, it's a small change — happy to push it. Just let me know which way you want it.

@amirejaz
amirejaz merged commit 92be1bf into stacklok:main Aug 20, 2026
46 checks passed
@github-actions github-actions Bot mentioned this pull request Aug 26, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow opt-in internal/private remoteUrl hosts for co-located MCP backends (operator ValidateRemoteURL has no allowlist)

3 participants