fix(@angular/ssr): introduce trustProxyHeaders option to safely vali…

[alan-agius4]

…date and sanitize proxy headers

This commit adds the trustProxyHeaders option to AngularAppEngineOptions and AngularNodeAppEngineOptions to configure, validate, and sanitize X-Forwarded-* headers.

• When trustProxyHeaders is undefined (default):
  • Allows X-Forwarded-Host and X-Forwarded-Proto.
  • Intercepts X-Forwarded-Prefix and triggers a dynamic CSR deoptimization to skip SSR if present.
  • Logs an informative message when receiving any other X-Forwarded-* headers.
• When false:
  • Ignores and strips all proxy headers from the request.
• When true:
  • Trusts all proxy headers.
• When a string array:
  • Allows only the proxy headers provided inside the array.

[gemini-code-assist]

Code Review

This pull request introduces the "trustProxyHeaders" option to the Angular SSR engine, enabling explicit control over trusted "X-Forwarded-*" headers. It replaces header-patching validation with a sanitization step that removes untrusted headers and can trigger CSR deoptimization. Feedback highlights a potential performance regression from over-aggressive CSR deoptimization on common headers and a breaking change in path prefix validation that now excludes dots. An optimization was also suggested to improve the efficiency of request sanitization by avoiding redundant cloning.

[gemini-code-assist]

The current implementation triggers CSR deoptimization ('deoptToCSR = true') for any 'X-Forwarded-*' header that is not explicitly allowed. By default, this includes common headers like 'X-Forwarded-For' and 'X-Forwarded-Port'. This will cause a significant performance regression for most applications running behind a proxy, as they will fall back to client-side rendering whenever these headers are present.

Consider only triggering 'deoptToCSR' for headers that fundamentally change the rendering context in a way that cannot be safely handled if untrusted (like 'X-Forwarded-Prefix'), while simply stripping other untrusted headers to maintain security without sacrificing SSR performance.

    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',
      );
      if (lowerKey === 'x-forwarded-prefix') {
        deoptToCSR = true;
      }
      keysToDelete.push(key);
    }

[gemini-code-assist]

The new 'VALID_PREFIX_REGEX' is significantly more restrictive than the previous validation logic and excludes dots ('.'). This is a breaking change for applications that use dots in their path prefixes (e.g., '/api/v1.2' or '.well-known').

While preventing directory traversal ('..') is critical, valid path segments often contain dots. Consider updating the regex to allow dots while still ensuring that segments like '.' and '..' are rejected.

[gemini-code-assist]

Using 'request.clone()' followed by 'new Request(...)' is inefficient as it may cause the request body to be buffered or processed twice. Since the original request is being replaced by the sanitized version, you can pass the original request directly to the 'Request' constructor along with the modified headers. This is more efficient and avoids unnecessary cloning of the body.

Suggested change

	const clonedReq = new Request(request.clone(), {
	signal: request.signal,
	});
	const headers = clonedReq.headers;
	const originalGet = headers.get;
	// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
	(headers.get as typeof originalGet) = function (name) {
	const value = originalGet.call(headers, name);
	if (!value) {
	return value;
	}
	validateHeader(name, value, allowedHosts, onError);
	return value;
	};
	const originalValues = headers.values;
	// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
	(headers.values as typeof originalValues) = function () {
	for (const name of HOST_HEADERS_TO_VALIDATE) {
	validateHeader(name, originalGet.call(headers, name), allowedHosts, onError);
	}
	return originalValues.call(headers);
	};
	const originalEntries = headers.entries;
	// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
	(headers.entries as typeof originalEntries) = function () {
	const iterator = originalEntries.call(headers);
	return {
	next() {
	const result = iterator.next();
	if (!result.done) {
	const [key, value] = result.value;
	validateHeader(key, value, allowedHosts, onError);
	}
	return result;
	},
	[Symbol.iterator]() {
	return this;
	},
	};
	};
	// Ensure for...of loops use the new patched entries
	(headers[Symbol.iterator] as typeof originalEntries) = headers.entries;
	return { request: clonedReq, onError: onErrorPromise };
	}
	/**
	* Validates a specific header value against the allowed hosts.
	* @param name - The name of the header to validate.
	* @param value - The value of the header to validate.
	* @param allowedHosts - A set of allowed hostnames.
	* @param onError - A callback function to call if the header value is invalid.
	* @throws Error if the header value is invalid.
	*/
	function validateHeader(
	name: string,
	value: string | null,
	allowedHosts: ReadonlySet<string>,
	onError: (value: Error) => void,
	): void {
	if (!value) {
	return;
	for (const key of keysToDelete) {
	headers.delete(key);
	}
	if (!HOST_HEADERS_TO_VALIDATE.has(name.toLowerCase())) {
	return;
	}
	try {
	verifyHostAllowed(name, value, allowedHosts);
	} catch (error) {
	onError(error as Error);
	throw error;
	}
	return { request: clonedReq, deoptToCSR };
	const headers = new Headers(request.headers);
	for (const key of keysToDelete) {
	headers.delete(key);
	}
	return {
	request: new Request(request, { headers, signal: request.signal }),
	deoptToCSR,
	};