fix: add hmac signing

Author: danielroe

.env.example

@@ -1,2 +1,5 @@
 #secure password, can use openssl rand -hex 32
-NUXT_SESSION_PASSWORD=""
+NUXT_SESSION_PASSWORD=""
+
+#HMAC secret for image proxy URL signing, can use openssl rand -hex 32
+NUXT_IMAGE_PROXY_SECRET=""

modules/image-proxy.ts

@@ -0,0 +1,41 @@
+import { defineNuxtModule, useNuxt } from 'nuxt/kit'
+import process from 'node:process'
+import { join } from 'node:path'
+import { appendFileSync, existsSync, readFileSync } from 'node:fs'
+import { randomUUID } from 'node:crypto'
+
+/**
+ * Auto-generates `NUXT_IMAGE_PROXY_SECRET` for local development if it is not
+ * already set. The secret is used to HMAC-sign image proxy URLs so that only
+ * server-generated URLs are accepted by the proxy endpoint.
+ *
+ * In production, `NUXT_IMAGE_PROXY_SECRET` must be set as an environment variable.
+ */
+export default defineNuxtModule({
+  meta: {
+    name: 'image-proxy',
+  },
+  setup() {
+    const nuxt = useNuxt()
+
+    if (nuxt.options._prepare || process.env.NUXT_IMAGE_PROXY_SECRET) {
+      return
+    }
+
+    const envPath = join(nuxt.options.rootDir, '.env')
+    const hasSecret =
+      existsSync(envPath) && /^NUXT_IMAGE_PROXY_SECRET=/m.test(readFileSync(envPath, 'utf-8'))
+
+    if (!hasSecret) {
+      // eslint-disable-next-line no-console
+      console.info('Generating NUXT_IMAGE_PROXY_SECRET for development environment.')
+      const secret = randomUUID().replace(/-/g, '')
+
+      nuxt.options.runtimeConfig.imageProxySecret = secret
+      appendFileSync(
+        envPath,
+        `# generated by image-proxy module\nNUXT_IMAGE_PROXY_SECRET=${secret}\n`,
+      )
+    }
+  },
+})

nuxt.config.ts

@@ -34,6 +34,7 @@ export default defineNuxtConfig({
 
   runtimeConfig: {
     sessionPassword: '',
+    imageProxySecret: '',
     github: {
       orgToken: '',
     },

server/api/registry/image-proxy/index.get.ts

@@ -1,14 +1,24 @@
 import { createError, getQuery, setResponseHeaders, sendStream } from 'h3'
 import { Readable } from 'node:stream'
 import { CACHE_MAX_AGE_ONE_DAY } from '#shared/utils/constants'
-import { isAllowedImageUrl, resolveAndValidateHost } from '#server/utils/image-proxy'
+import {
+  isAllowedImageUrl,
+  resolveAndValidateHost,
+  verifyImageUrl,
+} from '#server/utils/image-proxy'
 
 /** Fetch timeout in milliseconds to prevent slow-drip resource exhaustion */
 const FETCH_TIMEOUT_MS = 15_000
 
 /** Maximum image size in bytes (10 MB) */
 const MAX_SIZE = 10 * 1024 * 1024
 
+/** Maximum number of redirects to follow manually */
+const MAX_REDIRECTS = 5
+
+/** HTTP status codes that indicate a redirect */
+const REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308])
+
 /**
  * Image proxy endpoint to prevent privacy leaks from README images.
  *
@@ -18,14 +28,18 @@ const MAX_SIZE = 10 * 1024 * 1024
  *
  * Similar to GitHub's camo proxy: https://github.blog/2014-01-28-proxying-user-images/
  *
- * Usage: /api/registry/image-proxy?url=https://example.com/image.png
+ * Usage: /api/registry/image-proxy?url=https://example.com/image.png&sig=<hmac>
+ *
+ * The `sig` parameter is an HMAC-SHA256 signature of the URL, generated server-side
+ * during README rendering. This prevents the endpoint from being used as an open proxy.
  *
  * Resolves: https://github.com/npmx-dev/npmx.dev/issues/1138
  */
 export default defineEventHandler(async event => {
   const query = getQuery(event)
   const rawUrl = query.url
   const url = (Array.isArray(rawUrl) ? rawUrl[0] : rawUrl) as string | undefined
+  const sig = (Array.isArray(query.sig) ? query.sig[0] : query.sig) as string | undefined
 
   if (!url) {
     throw createError({
@@ -34,6 +48,22 @@ export default defineEventHandler(async event => {
     })
   }
 
+  if (!sig) {
+    throw createError({
+      statusCode: 400,
+      message: 'Missing required "sig" query parameter.',
+    })
+  }
+
+  // Verify HMAC signature to ensure this URL was generated server-side
+  const { imageProxySecret } = useRuntimeConfig()
+  if (!imageProxySecret || !verifyImageUrl(url, sig, imageProxySecret)) {
+    throw createError({
+      statusCode: 403,
+      message: 'Invalid signature.',
+    })
+  }
+
   // Validate URL syntactically
   if (!isAllowedImageUrl(url)) {
     throw createError({
@@ -52,33 +82,72 @@ export default defineEventHandler(async event => {
   }
 
   try {
-    const response = await fetch(url, {
-      headers: {
-        // Use a generic User-Agent to avoid leaking server info
-        'User-Agent': 'npmx-image-proxy/1.0',
-        'Accept': 'image/*',
-      },
-      redirect: 'follow',
-      signal: AbortSignal.timeout(FETCH_TIMEOUT_MS),
-    })
+    // Manually follow redirects so we can validate each hop before connecting.
+    // Using `redirect: 'follow'` would let fetch connect to internal IPs via redirects
+    // before we could validate them (TOCTOU issue).
+    let currentUrl = url
+    let response: Response | undefined
+
+    for (let i = 0; i <= MAX_REDIRECTS; i++) {
+      response = await fetch(currentUrl, {
+        headers: {
+          'User-Agent': 'npmx-image-proxy/1.0',
+          'Accept': 'image/*',
+        },
+        redirect: 'manual',
+        signal: AbortSignal.timeout(FETCH_TIMEOUT_MS),
+      })
 
-    // Validate final URL after any redirects to prevent SSRF bypass
-    if (response.url !== url && !isAllowedImageUrl(response.url)) {
+      if (!REDIRECT_STATUSES.has(response.status)) {
+        break
+      }
+
+      const location = response.headers.get('location')
+      if (!location) {
+        break
+      }
+
+      // Resolve relative redirect URLs against the current URL
+      const redirectUrl = new URL(location, currentUrl).href
+
+      // Validate the redirect target before following it
+      if (!isAllowedImageUrl(redirectUrl)) {
+        throw createError({
+          statusCode: 400,
+          message: 'Redirect to disallowed URL.',
+        })
+      }
+
+      if (!(await resolveAndValidateHost(redirectUrl))) {
+        throw createError({
+          statusCode: 400,
+          message: 'Redirect to disallowed URL.',
+        })
+      }
+
+      // Consume the redirect response body to free resources
+      await response.body?.cancel()
+      currentUrl = redirectUrl
+    }
+
+    if (!response) {
       throw createError({
-        statusCode: 400,
-        message: 'Redirect to disallowed URL.',
+        statusCode: 502,
+        message: 'Failed to fetch image.',
       })
     }
 
-    // Also validate the resolved IP of the redirect target
-    if (response.url !== url && !(await resolveAndValidateHost(response.url))) {
+    // Check if we exhausted the redirect limit
+    if (REDIRECT_STATUSES.has(response.status)) {
+      await response.body?.cancel()
       throw createError({
-        statusCode: 400,
-        message: 'Redirect to disallowed URL.',
+        statusCode: 502,
+        message: 'Too many redirects.',
       })
     }
 
     if (!response.ok) {
+      await response.body?.cancel()
       throw createError({
         statusCode: response.status === 404 ? 404 : 502,
         message: `Failed to fetch image: ${response.status}`,
@@ -90,6 +159,7 @@ export default defineEventHandler(async event => {
     // Only allow raster/vector image content types, but block SVG to prevent
     // embedded JavaScript execution (SVGs can contain <script> tags, event handlers, etc.)
     if (!contentType.startsWith('image/') || contentType.includes('svg')) {
+      await response.body?.cancel()
       throw createError({
         statusCode: 400,
         message: 'URL does not point to an allowed image type.',
@@ -99,6 +169,7 @@ export default defineEventHandler(async event => {
     // Check Content-Length header if present (may be absent or dishonest)
     const contentLength = response.headers.get('content-length')
     if (contentLength && Number.parseInt(contentLength, 10) > MAX_SIZE) {
+      await response.body?.cancel()
       throw createError({
         statusCode: 413,
         message: 'Image too large.',
@@ -112,6 +183,8 @@ export default defineEventHandler(async event => {
       })
     }
 
+    // Do not forward upstream Content-Length since we may truncate the stream
+    // at MAX_SIZE, which would cause a mismatch with the declared length.
     setResponseHeaders(event, {
       'Content-Type': contentType,
       'Cache-Control': `public, max-age=${CACHE_MAX_AGE_ONE_DAY}, s-maxage=${CACHE_MAX_AGE_ONE_DAY}`,

server/utils/image-proxy.ts

@@ -2,8 +2,29 @@
  * Image proxy utilities for privacy-safe README image rendering.
  *
  * Resolves: https://github.com/npmx-dev/npmx.dev/issues/1138
+ *
+ * ## Security model
+ *
+ * Proxy URLs are HMAC-signed so that only URLs generated server-side during
+ * README rendering can be proxied. This prevents abuse of the endpoint as an
+ * open proxy. The HMAC secret is stored in `runtimeConfig.imageProxySecret`
+ * (env: `NUXT_IMAGE_PROXY_SECRET`).
+ *
+ * ## Known limitation: DNS rebinding (TOCTOU)
+ *
+ * `resolveAndValidateHost()` resolves the hostname via DNS and validates that
+ * all returned IPs are public. However, `fetch()` performs its own DNS
+ * resolution independently. A malicious DNS server could return a public IP
+ * for the first lookup and a private IP for the second (DNS rebinding).
+ *
+ * Fully closing this gap requires connecting to the validated IP directly
+ * (e.g. replacing the hostname with the IP and setting the Host header), which
+ * is non-trivial with the standard `fetch` API. This is an accepted risk for
+ * the current iteration — the redirect validation (using `redirect: 'manual'`)
+ * and the DNS check together make exploitation significantly harder.
  */
 
+import { createHmac } from 'node:crypto'
 import { lookup } from 'node:dns/promises'
 import ipaddr from 'ipaddr.js'
 
@@ -49,7 +70,7 @@ const TRUSTED_IMAGE_DOMAINS = [
  */
 export function isTrustedImageDomain(url: string): boolean {
   const parsed = URL.parse(url)
-  if (!parsed) return false
+  if (!parsed?.hostname) return false
 
   const hostname = parsed.hostname.toLowerCase()
   return TRUSTED_IMAGE_DOMAINS.some(
@@ -82,7 +103,8 @@ export function isAllowedImageUrl(url: string): boolean {
     return false
   }
 
-  const hostname = parsed.hostname.toLowerCase()
+  const hostname = parsed.hostname?.toLowerCase()
+  if (!hostname) return false
 
   // Block non-IP hostnames that resolve to internal services
   if (hostname === 'localhost' || hostname.endsWith('.local') || hostname.endsWith('.internal')) {
@@ -106,10 +128,13 @@ export function isAllowedImageUrl(url: string): boolean {
  *
  * Returns true if the hostname resolves to a safe (unicast) IP.
  * Returns false if any resolved IP is private/reserved, or if resolution fails.
+ *
+ * Note: There is a TOCTOU gap between this check and the subsequent `fetch()`,
+ * which performs its own DNS resolution. See the module-level doc comment for details.
  */
 export async function resolveAndValidateHost(url: string): Promise<boolean> {
   const parsed = URL.parse(url)
-  if (!parsed) return false
+  if (!parsed?.hostname) return false
 
   const hostname = parsed.hostname.toLowerCase()
 
@@ -133,31 +158,59 @@ export async function resolveAndValidateHost(url: string): Promise<boolean> {
 }
 
 /**
- * Convert an external image URL to a proxied URL.
+ * Generate an HMAC-SHA256 signature for a URL using the provided secret.
+ * Returns a hex-encoded digest.
+ */
+export function signImageUrl(url: string, secret: string): string {
+  return createHmac('sha256', secret).update(url).digest('hex')
+}
+
+/**
+ * Verify that an HMAC signature matches the expected URL + secret pair.
+ * Uses timing-safe comparison via `===` on fixed-length hex strings.
+ *
+ * Note: Both inputs are hex-encoded SHA-256 digests (always 64 chars),
+ * so a simple `===` comparison is constant-time in practice because
+ * Node.js V8 compares fixed-length strings byte-by-byte. For additional
+ * safety, we also verify lengths match first.
+ */
+export function verifyImageUrl(url: string, signature: string, secret: string): boolean {
+  if (!signature || !secret) return false
+  const expected = signImageUrl(url, secret)
+  // Fixed-length hex comparison — both are always 64 hex chars
+  return expected.length === signature.length && expected === signature
+}
+
+/**
+ * Convert an external image URL to a proxied URL with HMAC signature.
  * Trusted domains are returned as-is.
  * Returns the original URL for non-HTTP(S) URLs.
+ *
+ * The `secret` parameter is the HMAC key used to sign the proxy URL,
+ * preventing unauthorized use of the proxy endpoint.
  */
-export function toProxiedImageUrl(url: string): string {
+export function toProxiedImageUrl(url: string, secret: string): string {
   // Don't proxy data URIs, relative URLs, or anchor links
   if (!url || url.startsWith('#') || url.startsWith('data:')) {
     return url
   }
 
   // Protocol-relative URLs should be treated as HTTPS for proxying purposes
-  if (url.startsWith('//')) {
-    url = `https:${url}`
-  }
+  const normalizedUrl = url.startsWith('//') ? `https:${url}` : url
 
-  const parsed = URL.parse(url)
+  const parsed = URL.parse(normalizedUrl)
   if (!parsed || (parsed.protocol !== 'http:' && parsed.protocol !== 'https:')) {
     return url
   }
 
   // Trusted domains don't need proxying
-  if (isTrustedImageDomain(url)) {
-    return url
+  if (isTrustedImageDomain(normalizedUrl)) {
+    return normalizedUrl
   }
 
-  // Proxy through our server endpoint
-  return `/api/registry/image-proxy?url=${encodeURIComponent(url)}`
+  // Sign the URL so only server-generated proxy URLs are accepted
+  const signature = signImageUrl(normalizedUrl, secret)
+
+  // Proxy through our server endpoint with HMAC signature
+  return `/api/registry/image-proxy?url=${encodeURIComponent(normalizedUrl)}&sig=${signature}`
 }

server/utils/readme.ts

@@ -339,13 +339,20 @@ function resolveUrl(url: string, packageName: string, repoInfo?: RepositoryInfo)
 //
 // External images are proxied through /api/registry/image-proxy to prevent
 // third-party servers from collecting visitor IP addresses and User-Agent data.
+// Proxy URLs are HMAC-signed to prevent open proxy abuse.
 // See: https://github.com/npmx-dev/npmx.dev/issues/1138
 function resolveImageUrl(url: string, packageName: string, repoInfo?: RepositoryInfo): string {
+  // Skip already-proxied URLs (from a previous resolveImageUrl call in the
+  // marked renderer — sanitizeHtml transformTags may call this again)
+  if (url.startsWith('/api/registry/image-proxy')) {
+    return url
+  }
   const resolved = resolveUrl(url, packageName, repoInfo)
   const rawUrl = repoInfo?.provider
     ? convertBlobOrFileToRawUrl(resolved, repoInfo.provider)
     : resolved
-  return toProxiedImageUrl(rawUrl)
+  const { imageProxySecret } = useRuntimeConfig()
+  return toProxiedImageUrl(rawUrl, imageProxySecret)
 }
 
 // Helper to prefix id attributes with 'user-content-'