Store an IPv6 allowed host the way the endpoint check spells it

CHANGELOG.md

@@ -8,6 +8,16 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+### An IPv6 address in `AGENT_ENDPOINT_ALLOWED_HOSTS` now matches however it is written
+
+The endpoint check compares the list against the address as the URL parser spells it, compressed
+and lower-case, while the list kept each IPv6 entry as the operator wrote it. `[0:0:0:0:0:0:0:1]:8443`
+was therefore a line that silently never matched, the failure the list's other refusals exist to
+prevent. Stripping the brackets on both sides also folded two different names into one, so naming
+`[fd00::1:8443]`, an address, admitted `[fd00::1]:8443`, another address on a port, and the other way
+round. Bracketed entries are now stored in the parser's spelling, with the port kept as written, and
+compared with their brackets on; an entry the parser does not read as an address is refused at boot,
+naming the entry, as a URL or a wildcard already was. Names and IPv4 entries are unaffected.
 ### A Bot's own decline is only recorded against a Bot the caller may reach
 
 A Bot reports that it declined a request through the person's session, and the audit row says

server/src/agents/endpoint.ts

@@ -48,7 +48,10 @@ function namedAsAllowed(
   } catch {
     return false;
   }
-  const hostname = url.hostname.toLowerCase().replace(/^\[|\]$/g, "");
+  // As the parser spells them, brackets included: `[::1]:8443` names an address and a port and
+  // `[::1:8443]` names an address, and with the brackets gone the two read as one. The list is
+  // stored in the same spelling (see `normalizeAllowedHost` in config.ts).
+  const hostname = url.hostname.toLowerCase();
   const host = url.port ? `${hostname}:${url.port}` : hostname;
   return allowedHosts.has(host) || allowedHosts.has(hostname);
 }

server/src/config.ts

@@ -668,23 +668,43 @@ function agentEndpointAllowedHosts(
         `AGENT_ENDPOINT_ALLOWED_HOSTS entry "${entry}" must name one host. Patterns are not accepted: list each address instead.`,
       );
     }
-    hosts.add(normalizeAllowedHost(host));
+    hosts.add(normalizeAllowedHost(entry, host));
   }
   return hosts;
 }
 
-function normalizeAllowedHost(host: string): string {
-  // IPv6 is bracketed as [host] or [host]:port. Strip the brackets and keep the port.
-  if (host.startsWith("[")) {
-    const close = host.indexOf("]");
-    if (close === -1) return host.replace(/^\[/, "").replace(/\]$/, "");
-    const ipv6 = host.slice(1, close).toLowerCase();
-    const rest = host.slice(close + 1);
-    if (!rest) return ipv6;
-    if (rest.startsWith(":")) return `${ipv6}${rest.toLowerCase()}`;
-    return `${ipv6}${rest.toLowerCase()}`;
-  }
-  return host;
+/**
+ * An IPv6 entry, spelled the way the endpoint check will see it.
+ *
+ * `namedAsAllowed` compares against `URL.hostname`, which the parser canonicalises: compressed,
+ * lower-case, in brackets. An entry kept as written matched only when the operator happened to
+ * write it that way, so `[0:0:0:0:0:0:0:1]:8443` was a line that silently never matched, which is
+ * the failure the URL and wildcard refusals above exist to prevent. Stripping the brackets instead
+ * folded two different names into one: `[::1]:8443`, an address and a port, and `[::1:8443]`, an
+ * address, both became `::1:8443`, so naming either admitted the other.
+ *
+ * The address goes through the URL parser rather than a hand-written normaliser, so the spelling
+ * here is the parser's own and cannot drift from it. The port is kept as written, since the parser
+ * drops a scheme's default port and an operator who wrote `:80` meant that port. A bracketed entry
+ * the parser refuses is not an address, and is refused the way a URL is: at boot, naming the entry.
+ */
+function normalizeAllowedHost(entry: string, host: string): string {
+  if (!host.startsWith("[")) return host;
+  const close = host.indexOf("]");
+  const address = close === -1 ? host : host.slice(0, close + 1);
+  const port = close === -1 ? "" : host.slice(close + 1);
+  const refusal = () =>
+    new Error(
+      `AGENT_ENDPOINT_ALLOWED_HOSTS entry "${entry}" must be a host, optionally with a port, and not a URL.`,
+    );
+  if (port && !/^:\d{1,5}$/.test(port)) throw refusal();
+  let hostname: string;
+  try {
+    hostname = new URL(`http://${address}`).hostname;
+  } catch {
+    throw refusal();
+  }
+  return `${hostname}${port}`;
 }
 
 function privateHostsAllowed(environment: Environment): boolean {

server/tests/agent-endpoint.test.ts

@@ -634,6 +634,42 @@ describe("private addresses named one at a time", () => {
     ).toBeFalse();
   });
 
+  test("an IPv6 address matches however the endpoint spells it", () => {
+    // The list holds the parser's spelling (config.ts canonicalises it); the endpoint may arrive
+    // uncompressed or upper-case and the parser folds both to the same name.
+    for (const address of [
+      "http://[0:0:0:0:0:0:0:1]:8443/ag-ui",
+      "http://[::1]:8443/ag-ui",
+      "http://[::1]:8443/ag-ui".toUpperCase().replace("HTTP", "http"),
+    ]) {
+      expect(
+        checkAgentEndpoint(address, { allowedHosts: named("[::1]:8443") })
+          .allowed,
+      ).toBeTrue();
+    }
+  });
+
+  test("an IPv6 address is not confused with an address and a port", () => {
+    // `[fd00::1:8443]` is an address on the private network, and `[fd00::1]:8443` is another one
+    // with a port. Naming either must not admit the other, which is what stripping the brackets
+    // from both did: each became `fd00::1:8443`.
+    expect(
+      checkAgentEndpoint("http://[fd00::1]:8443/ag-ui", {
+        allowedHosts: named("[fd00::1:8443]"),
+      }).allowed,
+    ).toBeFalse();
+    expect(
+      checkAgentEndpoint("http://[fd00::1:8443]/ag-ui", {
+        allowedHosts: named("[fd00::1]:8443"),
+      }).allowed,
+    ).toBeFalse();
+    expect(
+      checkAgentEndpoint("http://[fd00::1:8443]/ag-ui", {
+        allowedHosts: named("[fd00::1:8443]"),
+      }).allowed,
+    ).toBeTrue();
+  });
+
   test("the metadata address cannot be named back in", () => {
     /*
      * The property that makes this safe to ship. The never-allowed list is checked before the

server/tests/config.test.ts

@@ -818,6 +818,33 @@ describe("AGENT_ENDPOINT_ALLOWED_HOSTS", () => {
     expect([...hosts].sort()).toEqual(["10.0.0.42:9000", "agents.internal"]);
   });
 
+  test("an IPv6 address is stored the way the endpoint check spells it", () => {
+    // The check compares against `URL.hostname`: compressed, lower-case, in brackets. An entry kept
+    // as the operator wrote it was a line that silently never matched.
+    const hosts = loadConfig({
+      ...base(),
+      AGENT_ENDPOINT_ALLOWED_HOSTS:
+        "[0:0:0:0:0:0:0:1]:8443, [FE80::1], [::1:8443]",
+    }).agentEndpointAllowedHosts;
+    expect([...hosts].sort()).toEqual([
+      "[::1:8443]",
+      "[::1]:8443",
+      "[fe80::1]",
+    ]);
+  });
+
+  test("a bracketed entry that is not an address is refused, naming the entry", () => {
+    expect(() =>
+      loadConfig({
+        ...base(),
+        AGENT_ENDPOINT_ALLOWED_HOSTS: "[not-an-address]",
+      }),
+    ).toThrow(/must be a host/);
+    expect(() =>
+      loadConfig({ ...base(), AGENT_ENDPOINT_ALLOWED_HOSTS: "[::1]junk" }),
+    ).toThrow(/must be a host/);
+  });
+
   test("a URL is refused, naming the entry", () => {
     expect(() =>
       loadConfig({