fixup! fix(@angular/ssr): enforce explicit opt-in for proxy headers

Author: alan-agius4

packages/angular/ssr/node/src/request.ts

@@ -8,7 +8,11 @@
 
 import type { IncomingHttpHeaders, IncomingMessage } from 'node:http';
 import type { Http2ServerRequest } from 'node:http2';
-import { getFirstHeaderValue } from '../../src/utils/validation';
+import {
+  getFirstHeaderValue,
+  isProxyHeaderAllowed,
+  normalizeTrustProxyHeaders,
+} from '../../src/utils/validation';
 
 /**
  * A set containing all the pseudo-headers defined in the HTTP/2 specification.
@@ -33,33 +37,27 @@ const HTTP2_PSEUDO_HEADERS: ReadonlySet<string> = new Set([
  * be used by web platform APIs.
  *
  * @param nodeRequest - The Node.js request object (`IncomingMessage` or `Http2ServerRequest`) to convert.
- * @param trustProxyHeaders - A boolean or an array of allowed proxy headers.
+ * @param trustProxyHeaders - A boolean or an array of proxy headers to trust when constructing the request URL.
  *
  * @remarks
  * When `trustProxyHeaders` is enabled, headers such as `X-Forwarded-Host` and
  * `X-Forwarded-Prefix` should ideally be strictly validated at a higher infrastructure
  * level (e.g., at the reverse proxy or API gateway) before reaching the application.
  *
  * @returns A Web Standard `Request` object.
- *
- * @private
  */
 export function createWebRequestFromNodeRequest(
   nodeRequest: IncomingMessage | Http2ServerRequest,
   trustProxyHeaders?: boolean | readonly string[],
 ): Request {
-  const trustProxyHeadersNormalized =
-    trustProxyHeaders && typeof trustProxyHeaders !== 'boolean'
-      ? new Set(trustProxyHeaders.map((h) => h.toLowerCase()))
-      : trustProxyHeaders;
-
+  const trustProxyHeadersNormalized = normalizeTrustProxyHeaders(trustProxyHeaders);
   const { headers, method = 'GET' } = nodeRequest;
   const withBody = method !== 'GET' && method !== 'HEAD';
   const referrer = headers.referer && URL.canParse(headers.referer) ? headers.referer : undefined;
 
   return new Request(createRequestUrl(nodeRequest, trustProxyHeadersNormalized), {
     method,
-    headers: createRequestHeaders(headers, trustProxyHeadersNormalized),
+    headers: createRequestHeaders(headers),
     body: withBody ? nodeRequest : undefined,
     duplex: withBody ? 'half' : undefined,
     referrer,
@@ -70,27 +68,16 @@ export function createWebRequestFromNodeRequest(
  * Creates a `Headers` object from Node.js `IncomingHttpHeaders`.
  *
  * @param nodeHeaders - The Node.js `IncomingHttpHeaders` object to convert.
- * @param trustProxyHeaders - A boolean or a set of allowed proxy headers.
  * @returns A `Headers` object containing the converted headers.
  */
-function createRequestHeaders(
-  nodeHeaders: IncomingHttpHeaders,
-  trustProxyHeaders: boolean | ReadonlySet<string> | undefined,
-): Headers {
+function createRequestHeaders(nodeHeaders: IncomingHttpHeaders): Headers {
   const headers = new Headers();
 
   for (const [name, value] of Object.entries(nodeHeaders)) {
     if (HTTP2_PSEUDO_HEADERS.has(name)) {
       continue;
     }
 
-    if (
-      name.toLowerCase().startsWith('x-forwarded-') &&
-      !isProxyHeaderAllowed(name, trustProxyHeaders)
-    ) {
-      continue;
-    }
-
     if (typeof value === 'string') {
       headers.append(name, value);
     } else if (Array.isArray(value)) {
@@ -107,7 +94,7 @@ function createRequestHeaders(
  * Creates a `URL` object from a Node.js `IncomingMessage`, taking into account the protocol, host, and port.
  *
  * @param nodeRequest - The Node.js `IncomingMessage` or `Http2ServerRequest` object to extract URL information from.
- * @param trustProxyHeaders - A boolean or a set of allowed proxy headers.
+ * @param trustProxyHeaders - A set of allowed proxy headers.
  *
  * @remarks
  * When `trustProxyHeaders` is enabled, headers such as `X-Forwarded-Host` and
@@ -118,7 +105,7 @@ function createRequestHeaders(
  */
 export function createRequestUrl(
   nodeRequest: IncomingMessage | Http2ServerRequest,
-  trustProxyHeaders?: boolean | ReadonlySet<string>,
+  trustProxyHeaders: ReadonlySet<string>,
 ): URL {
   const {
     headers,
@@ -156,37 +143,15 @@ export function createRequestUrl(
  *
  * @param headers - The Node.js incoming HTTP headers.
  * @param headerName - The name of the proxy header to retrieve.
- * @param trustProxyHeaders - A boolean or a set of allowed proxy headers.
+ * @param trustProxyHeaders - A set of allowed proxy headers.
  * @returns The value of the allowed proxy header, or `undefined` if not allowed or not present.
  */
 function getAllowedProxyHeaderValue(
   headers: IncomingHttpHeaders,
   headerName: string,
-  trustProxyHeaders: boolean | ReadonlySet<string> | undefined,
+  trustProxyHeaders: ReadonlySet<string>,
 ): string | undefined {
   return isProxyHeaderAllowed(headerName, trustProxyHeaders)
     ? getFirstHeaderValue(headers[headerName])
     : undefined;
 }
-
-/**
- * Checks if a specific proxy header is allowed.
- *
- * @param headerName - The name of the proxy header to check.
- * @param trustProxyHeaders - A boolean or a set of allowed proxy headers.
- * @returns `true` if the header is allowed, `false` otherwise.
- */
-function isProxyHeaderAllowed(
-  headerName: string,
-  trustProxyHeaders: boolean | ReadonlySet<string> | undefined,
-): boolean {
-  if (!trustProxyHeaders) {
-    return false;
-  }
-
-  if (trustProxyHeaders === true) {
-    return true;
-  }
-
-  return trustProxyHeaders.has(headerName.toLowerCase());
-}

packages/angular/ssr/src/app-engine.ts

@@ -12,7 +12,11 @@ import { getPotentialLocaleIdFromUrl, getPreferredLocale } from './i18n';
 import { EntryPointExports, getAngularAppEngineManifest } from './manifest';
 import { createRedirectResponse } from './utils/redirect';
 import { joinUrlParts } from './utils/url';
-import { sanitizeRequestHeaders, validateRequest } from './utils/validation';
+import {
+  normalizeTrustProxyHeaders,
+  sanitizeRequestHeaders,
+  validateRequest,
+} from './utils/validation';
 
 /**
  * Options for the Angular server application engine.
@@ -27,9 +31,12 @@ export interface AngularAppEngineOptions {
    * Extends the scope of trusted proxy headers (`X-Forwarded-*`).
    *
    * @remarks
-   * When `trustProxyHeaders` is enabled, headers such as `X-Forwarded-Host` and
-   * `X-Forwarded-Prefix` should ideally be strictly validated at a higher infrastructure
-   * level (e.g., at the reverse proxy or API gateway) before reaching the application.
+   * **This is a security-sensitive option!**
+   *
+   * When `trustProxyHeaders` is enabled, request headers such as `X-Forwarded-Host` and
+   * `X-Forwarded-Prefix` are trusted by the server and used for routing. These
+   * headers must be strictly validated and provided by a trusted client (e.g., at a reverse proxy, load
+   * balancer, or API gateway) and must *not* be provided by untrusted end users.
    *
    * If a `string[]` is provided, only those proxy headers are allowed.
    * If `true`, all proxy headers are allowed.
@@ -97,7 +104,7 @@ export class AngularAppEngine {
   /**
    * The normalized allowed proxy headers.
    */
-  private readonly trustProxyHeaders: ReadonlySet<string> | boolean;
+  private readonly trustProxyHeaders: ReadonlySet<string>;
 
   /**
    * A cache that holds entry points, keyed by their potential locale string.
@@ -110,12 +117,7 @@ export class AngularAppEngine {
    */
   constructor(options?: AngularAppEngineOptions) {
     this.allowedHosts = this.getAllowedHosts(options);
-
-    const trustProxyHeaders = options?.trustProxyHeaders ?? false;
-    this.trustProxyHeaders =
-      typeof trustProxyHeaders === 'boolean'
-        ? trustProxyHeaders
-        : new Set(trustProxyHeaders.map((h) => h.toLowerCase()));
+    this.trustProxyHeaders = normalizeTrustProxyHeaders(options?.trustProxyHeaders);
   }
 
   private getAllowedHosts(options: AngularAppEngineOptions | undefined): ReadonlySet<string> {

packages/angular/ssr/src/utils/validation.ts

@@ -6,6 +6,17 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
+/**
+ * Common X-Forwarded-* headers.
+ */
+const X_FORWARDED_HEADERS = new Set([
+  'x-forwarded-for',
+  'x-forwarded-host',
+  'x-forwarded-port',
+  'x-forwarded-proto',
+  'x-forwarded-prefix',
+]);
+
 /**
  * The set of headers that should be validated for host header injection attacks.
  */
@@ -22,7 +33,8 @@ const VALID_PORT_REGEX = /^\d+$/;
 const VALID_PROTO_REGEX = /^https?$/i;
 
 /**
- * Regular expression to validate that the prefix is valid.
+ * Regular expression to validate that the prefix is valid. Validates that the prefix is a valid path prefix and nothing else.
+ * It validates that the prefix starts with a forward slash and contains only letters, numbers, hyphens, underscores and forward slashes.
  */
 const VALID_PREFIX_REGEX = /^\/([a-z0-9_-]+\/)*[a-z0-9_-]*$/i;
 
@@ -85,42 +97,36 @@ export function validateUrl(url: URL, allowedHosts: ReadonlySet<string>): void {
  * If no headers need to be removed, the original request is returned without cloning.
  *
  * @param request - The incoming `Request` object to sanitize.
- * @param trustProxyHeaders - A set of allowed proxy headers or a boolean indicating whether all are allowed.
+ * @param trustProxyHeaders - A set of allowed proxy headers.
  * @returns The sanitized request, or the original request if no changes were needed.
  */
 export function sanitizeRequestHeaders(
   request: Request,
-  trustProxyHeaders?: boolean | ReadonlySet<string>,
+  trustProxyHeaders: ReadonlySet<string>,
 ): Request {
-  if (trustProxyHeaders === true) {
-    return request;
-  }
+  let headersDeleted = false;
+  const headers = new Headers();
 
-  const keysToDelete: string[] = [];
-  for (const [key] of request.headers) {
+  for (const [key, value] of request.headers) {
     const lowerKey = key.toLowerCase();
-    if (
-      lowerKey.startsWith('x-forwarded-') &&
-      (!trustProxyHeaders || !trustProxyHeaders.has(lowerKey))
-    ) {
-      keysToDelete.push(key);
+    if (lowerKey.startsWith('x-forwarded-') && !isProxyHeaderAllowed(lowerKey, trustProxyHeaders)) {
+      // eslint-disable-next-line no-console
+      console.warn(
+        `Received "${key}" header but "trustProxyHeaders" was not set up to allow it.\n` +
+          `For more information, see https://angular.dev/best-practices/security#configuring-trusted-proxy-headers`,
+      );
+      headersDeleted = true;
+    } else {
+      headers.set(key, value);
     }
   }
 
-  if (keysToDelete.length === 0) {
-    return request;
-  }
-
-  const clonedReq = new Request(request.clone(), {
-    signal: request.signal,
-  });
-
-  const headers = clonedReq.headers;
-  for (const key of keysToDelete) {
-    headers.delete(key);
-  }
-
-  return clonedReq;
+  return headersDeleted
+    ? new Request(request.clone(), {
+        signal: request.signal,
+        headers,
+      })
+    : request;
 }
 
 /**
@@ -217,3 +223,36 @@ function validateHeaders(
     );
   }
 }
+
+/**
+ * Checks if a specific proxy header is allowed.
+ *
+ * @param headerName - The name of the proxy header to check.
+ * @param trustProxyHeaders - A set of allowed proxy headers.
+ * @returns `true` if the header is allowed, `false` otherwise.
+ */
+export function isProxyHeaderAllowed(
+  headerName: string,
+  trustProxyHeaders: ReadonlySet<string>,
+): boolean {
+  return trustProxyHeaders.has(headerName.toLowerCase());
+}
+
+/**
+ * Normalizes the `trustProxyHeaders` option to a consistent representation.
+ * @param trustProxyHeaders The input `trustProxyHeaders` value.
+ * @returns A `Set<string>` of normalized header names.
+ */
+export function normalizeTrustProxyHeaders(
+  trustProxyHeaders: boolean | readonly string[] | undefined,
+): ReadonlySet<string> {
+  if (!trustProxyHeaders) {
+    return new Set();
+  }
+
+  if (trustProxyHeaders === true) {
+    return X_FORWARDED_HEADERS;
+  }
+
+  return new Set(trustProxyHeaders.map((h) => h.toLowerCase()));
+}

packages/angular/ssr/test/utils/validation_spec.ts

@@ -224,7 +224,7 @@ describe('Validation Utils', () => {
           'x-forwarded-proto': 'https',
         },
       });
-      const secured = sanitizeRequestHeaders(req);
+      const secured = sanitizeRequestHeaders(req, new Set());
 
       expect(secured.headers.get('host')).toBe('example.com');
       expect(secured.headers.has('x-forwarded-host')).toBeFalse();
@@ -255,7 +255,10 @@ describe('Validation Utils', () => {
           'x-forwarded-proto': 'https',
         },
       });
-      const secured = sanitizeRequestHeaders(req, true);
+      const secured = sanitizeRequestHeaders(
+        req,
+        new Set(['x-forwarded-host', 'x-forwarded-proto']),
+      );
 
       expect(secured.headers.get('host')).toBe('example.com');
       expect(secured.headers.get('x-forwarded-host')).toBe('proxy.com');
@@ -266,7 +269,7 @@ describe('Validation Utils', () => {
       const req = new Request('http://example.com', {
         headers: { 'accept': 'application/json' },
       });
-      const secured = sanitizeRequestHeaders(req);
+      const secured = sanitizeRequestHeaders(req, new Set());
 
       expect(secured).toBe(req);
       expect(secured.headers.get('accept')).toBe('application/json');