stacklok / stacklok/toolhive

ValidateRemoteURL misses empty-host, trailing-dot, zone-ID, and non-standard IP forms

Open
#6,197 5 comments 0 reactions 1 assignee View on GitHub

@premctl is already working on this.

Since Aug 24, 2026.

operator p2 security
Dominant language
Go
Stars
2.2k
Forks
300
Avg merge
1d 15h
Merged PRs (30d)
184

Description

Summary

ValidateRemoteURL (cmd/thv-operator/pkg/validation/url_validation.go) blocks internal/private endpoints by matching the parsed hostname against a CIDR list (when net.ParseIP succeeds) or a hostname blocklist (otherwise). Several host forms fall through net.ParseIP into the hostname path and match nothing there, bypassing the blocklist entirely. All of these are pre-existing and independent of the new allowPrivateEndpoint flag (#5784 / #6195) — they are accepted in both flag states:

Form Example Why it slips through
Empty hostname http://:8080/ u.Host is ":8080" (non-empty, passes the host check) but u.Hostname() is "", which matches no blocklist entry. Go's HTTP client dials loopback for an empty host.
Trailing dot (FQDN root) http://localhost./ "localhost." fails the exact match against "localhost" and the "."+blocked suffix match; resolvers treat it as localhost. Applies to every blocked name (metadata.google.internal., etc.).
IPv6 zone ID http://[fe80::1%25eth0]/ net.ParseIP rejects zone IDs, so a link-local literal drops into the hostname path and is never checked against fe80::/10.
Non-standard IP forms http://2130706433/, http://0177.0.0.1/ Decimal/octal forms fail net.ParseIP and match no blocked hostname; whether they reach loopback depends on the consuming HTTP stack/resolver.

These were identified during the review of #6195 and deliberately not fixed there: that PR's contract was flag-off behavior byte-for-byte identical to main, and closing these holes changes validation outcomes in both flag states, so it warrants its own change and test matrix.

Suggested direction

  • Reject an empty u.Hostname() explicitly (the current u.Host == "" check does not cover http://:port).
  • Normalize (or reject) a single trailing dot before blocklist matching.
  • Parse host literals with netip.ParseAddr, which accepts zone IDs — check the address part against the CIDR sets, or reject zoned literals outright.
  • Consider rejecting any host that is neither a valid DNS name nor an address netip.ParseAddr accepts — a shape allowlist also disposes of the decimal/octal forms without enumerating them.

Unit tests should cover each form in both AllowPrivateEndpoint states (all should be rejected in both).

Context: validateHostNotInternal and the blocklists live in cmd/thv-operator/pkg/validation/url_validation.go; the validator intentionally performs no DNS resolution, and nothing here requires adding any.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.