Store an IPv6 allowed host the way the endpoint check spells it - #383
Merged
davidmckayv merged 2 commits intoSep 5, 2026
Merged
Conversation
namedAsAllowed compares the list against URL.hostname, which the parser canonicalises: compressed, lower-case, in brackets. The list kept an IPv6 entry as written, so [0:0:0:0:0:0:0:1]:8443 silently never matched, which is the failure the list's URL and wildcard refusals exist to prevent. Both sides also stripped the brackets, which folded an address and an address-with-port into one name: [fd00::1:8443] and [fd00::1]:8443 both became fd00::1:8443, so naming either admitted the other. Canonicalise a bracketed entry through the URL parser at boot, keep the port as written, refuse an entry the parser does not read as an address the way a URL is refused, and compare with the brackets on.
kevin9327
requested review from
MikeRyanDev,
davidmckayv,
guidovizoso and
tylerslaton
as code owners
September 5, 2026 12:12
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
# Conflicts: # CHANGELOG.md
davidmckayv
approved these changes
Sep 5, 2026
davidmckayv
left a comment
Contributor
There was a problem hiding this comment.
Reviewed diff against current main; resolved CHANGELOG keep-both; validated locally (format + composed test run). CI green.
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.
What this changes
AGENT_ENDPOINT_ALLOWED_HOSTSandnamedAsAllowed(agents/endpoint.ts) disagree about how an IPv6 address is spelled.url.hostname, which the WHATWG parser canonicalises:[0:0:0:0:0:0:0:1]becomes[::1],[FE80::1]becomes[fe80::1]. It then strips the brackets.normalizeAllowedHost) keeps the address as the operator wrote it, lower-cased, brackets stripped.So an operator who writes
[0:0:0:0:0:0:0:1]:8443gets a set holding0:0:0:0:0:0:0:1:8443, the endpoint check looks for::1:8443, and the entry silently never matches. The comment aboveagentEndpointAllowedHostssays a mistake in this list is "worth catching here rather than at the first registration that silently never matches", and refuses URLs and wildcards for exactly that reason; this was the same failure for a valid address.Stripping the brackets on both sides has a second effect: it is not injective.
[fd00::1:8443]is an address on the private network and[fd00::1]:8443is a different address with a port, and both becamefd00::1:8443. Naming either admitted the other. ([::1:8443]is the same shape, but the target floor reads it as public, so the test uses ULA addresses.)Now:
normalizeAllowedHostruns a bracketed entry's address throughnew URL()and storeshostnameas the parser spells it, brackets on. The port is kept as written rather than taken from the parser, because the parser drops a scheme's default port and an operator who wrote:80meant that port. A bracketed entry the parser refuses, or one followed by anything but:port, is refused at boot with the existing "must be a host" message naming the entry, the way a URL or a wildcard already is.namedAsAllowedno longer strips the brackets, so the two sides compare the same string.Names and IPv4 entries take the early return and are unchanged.
Where it runs
Boundary and audit
the metadata address cannot be named back instill passes).Changelog
A line under
Unreleased.Proof
config.test.ts:[0:0:0:0:0:0:0:1]:8443, [FE80::1], [::1:8443]is stored as[::1]:8443,[fe80::1],[::1:8443];[not-an-address]and[::1]junkare refused naming the entry.agent-endpoint.test.ts:http://[0:0:0:0:0:0:0:1]:8443/ag-uiandhttp://[::1]:8443/ag-uiare both allowed by a list holding[::1]:8443; a list holding[fd00::1:8443]does not admithttp://[fd00::1]:8443/, a list holding[fd00::1]:8443does not admithttp://[fd00::1:8443]/, and each admits its own. Before the change the second test fails on both refusals (each side stripped tofd00::1:8443), and the config test fails on the stored spelling.bunx tsc --noEmitinserverandbunx biome checkon the four files are clean.