fix(security): harden assertNotPrivateUrl with ipaddr.js + host normalization

Replaces the regex blocklist in assertNotPrivateUrl with ipaddr.js range
classification and normalizes the host before checking it. Consolidates two
community proposals (#930 ipaddr.js parsing, #912 trailing-dot normalization)
into one validator so the SSRF-critical path lives in-house with full tests.

- Classify literal IPs by range (loopback / linkLocal / unspecified) via
  ipaddr.js instead of a hand-maintained regex list, which also catches
  alternate IPv4 encodings and avoids over-blocking mapped public IPs (the old
  `::ffff:` regex blocked every mapped address, including public ones). IPv4-
  mapped IPv6 is reduced to its embedded IPv4 before classification.
- Strip a trailing root dot from the host so `localhost.` / `127.0.0.1.` can't
  bypass the checks (they resolve to the same target as the dotless form, #911).
- Strip IPv6 brackets and lowercase for the localhost comparison.
- RFC1918, bare LAN hostnames (e.g. `nomad3`), and external FQDNs remain
  allowed — LAN appliances need them, and DNS rebinding is a fetch-time concern
  outside this guard's scope.

Adds a consolidated unit spec covering loopback/link-local/unspecified literals,
alternate encodings, IPv4-mapped v6, mixed-case + trailing-dot localhost, and
the allowed LAN/FQDN/mapped-public cases.

Resolves #922. Supersedes #930 and #912 (thanks @Gujiassh and @luyua9).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

admin/app/validators/common.ts

@@ -14,25 +14,43 @@ import ipaddr from 'ipaddr.js'
  */
 export function assertNotPrivateUrl(urlString: string): void {
   const parsed = new URL(urlString)
-  const hostname = parsed.hostname.toLowerCase()
 
-  // `URL.hostname` strips the surrounding brackets from IPv6 literals
-  // (e.g. `http://[::1]/` → hostname `::1`), so IPv6 patterns must match
-  // the unbracketed form.
-  const blockedPatterns = [
-    /^localhost$/,
-    /^127\.\d+\.\d+\.\d+$/,
-    /^0\.0\.0\.0$/,
-    /^169\.254\.\d+\.\d+$/, // Link-local / cloud metadata
-    /^::1$/, // IPv6 loopback
-    /^fe80:/i, // IPv6 link-local
-    /^::ffff:/i, // IPv4-mapped IPv6 (e.g. ::ffff:7f00:1 = 127.0.0.1)
-    /^::$/, // IPv6 all-zeros (equivalent to 0.0.0.0)
-  ]
+  // Normalize the host before classifying it:
+  //  - lowercase for the `localhost` comparison
+  //  - strip the surrounding brackets `URL.hostname` leaves on IPv6 literals
+  //    (`http://[::1]/` → `::1`)
+  //  - strip any trailing root dot(s): `localhost.` and `127.0.0.1.` resolve to
+  //    the same target as the dotless form, so they must not slip past the
+  //    checks below (#911).
+  const hostname = parsed.hostname
+    .toLowerCase()
+    .replace(/^\[|\]$/g, '')
+    .replace(/\.+$/, '')
 
-  if (blockedPatterns.some((re) => re.test(hostname))) {
+  if (hostname === 'localhost') {
     throw new Error(`Download URL must not point to a loopback or link-local address: ${hostname}`)
   }
+
+  // Anything that isn't a literal IP (DNS names, bare LAN hostnames like
+  // `nomad3`, external FQDNs) is allowed — LAN appliances need them, and DNS
+  // rebinding is a fetch-time concern outside this guard's scope. Classifying
+  // literal addresses with ipaddr.js (rather than a regex list) catches
+  // alternate encodings and normalizes address ranges correctly (#922).
+  if (!ipaddr.isValid(hostname)) return
+
+  let addr = ipaddr.parse(hostname)
+  if (addr.kind() === 'ipv6' && (addr as ipaddr.IPv6).isIPv4MappedAddress()) {
+    // e.g. ::ffff:127.0.0.1 — classify by the embedded IPv4 so a mapped
+    // loopback/link-local is blocked while a mapped public IP is allowed.
+    addr = (addr as ipaddr.IPv6).toIPv4Address()
+  }
+
+  const range = addr.range()
+  if (range === 'loopback' || range === 'linkLocal' || range === 'unspecified') {
+    throw new Error(
+      `Download URL must not point to a loopback or link-local address: ${addr.toNormalizedString()}`
+    )
+  }
 }
 
 /**

admin/tests/unit/private_url.spec.ts

@@ -0,0 +1,53 @@
+import * as assert from 'node:assert/strict'
+import { test } from 'node:test'
+
+import { assertNotPrivateUrl } from '../../app/validators/common.js'
+
+const expectBlocked = (url: string) => {
+  assert.throws(() => assertNotPrivateUrl(url), /loopback or link-local/)
+}
+
+const expectAllowed = (url: string) => {
+  assert.doesNotThrow(() => assertNotPrivateUrl(url))
+}
+
+test('blocks loopback and unspecified IPv4 literals', () => {
+  expectBlocked('http://127.0.0.1/file.zim')
+  expectBlocked('http://127.1.2.3/file.zim')
+  expectBlocked('http://0.0.0.0/file.zim')
+  // Alternate encodings normalized to 127.0.0.1 / 0.0.0.0 by the WHATWG URL parser
+  expectBlocked('http://0177.0.0.1/file.zim')
+  expectBlocked('http://2130706433/file.zim')
+  expectBlocked('http://0/file.zim')
+})
+
+test('blocks link-local IPv4 literals including cloud metadata', () => {
+  expectBlocked('http://169.254.169.254/latest/meta-data/')
+  expectBlocked('http://169.254.1.1/file.zim')
+})
+
+test('blocks IPv6 loopback, link-local, unspecified, and IPv4-mapped loopback', () => {
+  expectBlocked('http://[::1]/file.zim')
+  expectBlocked('http://[fe80::1]/file.zim')
+  expectBlocked('http://[::]/file.zim')
+  expectBlocked('http://[::ffff:127.0.0.1]/file.zim')
+  expectBlocked('http://[::ffff:7f00:1]/file.zim')
+})
+
+test('blocks localhost, including mixed-case and trailing-root-dot forms (#911)', () => {
+  expectBlocked('http://localhost/file.zim')
+  expectBlocked('http://LOCALHOST/file.zim')
+  expectBlocked('http://localhost./file.zim')
+  expectBlocked('http://LocalHost./file.zim')
+})
+
+test('allows RFC1918 LAN literals, bare LAN hostnames, and public FQDNs', () => {
+  expectAllowed('http://10.0.0.2/file.zim')
+  expectAllowed('http://172.16.0.2/file.zim')
+  expectAllowed('http://192.168.1.10/file.zim')
+  expectAllowed('http://nomad3/file.zim')
+  expectAllowed('http://my-nas.local/file.zim')
+  expectAllowed('https://downloads.example.com/file.zim')
+  // A mapped *public* IP must not be blocked
+  expectAllowed('http://[::ffff:8.8.8.8]/file.zim')
+})