Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions goldens/public-api/angular/ssr/index.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export class AngularAppEngine {
// @public
export interface AngularAppEngineOptions {
allowedHosts?: readonly string[];
trustProxyHeaders?: boolean | readonly string[];
}

// @public
Expand Down
2 changes: 1 addition & 1 deletion goldens/public-api/angular/ssr/node/index.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export interface CommonEngineRenderOptions {
export function createNodeRequestHandler<T extends NodeRequestHandlerFunction>(handler: T): T;

// @public
export function createWebRequestFromNodeRequest(nodeRequest: IncomingMessage | Http2ServerRequest): Request;
export function createWebRequestFromNodeRequest(nodeRequest: IncomingMessage | Http2ServerRequest, trustProxyHeaders?: boolean | readonly string[]): Request;

// @public
export function isMainModule(url: string): boolean;
Expand Down
6 changes: 5 additions & 1 deletion packages/angular/ssr/node/src/app-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export interface AngularNodeAppEngineOptions extends AngularAppEngineOptions {}
*/
export class AngularNodeAppEngine {
private readonly angularAppEngine: AngularAppEngine;
private readonly trustProxyHeaders?: boolean | readonly string[];

/**
* Creates a new instance of the Angular Node.js server application engine.
Expand All @@ -39,6 +40,7 @@ export class AngularNodeAppEngine {
...options,
allowedHosts: [...getAllowedHostsFromEnv(), ...(options?.allowedHosts ?? [])],
});
this.trustProxyHeaders = options?.trustProxyHeaders;

attachNodeGlobalErrorHandlers();
}
Expand Down Expand Up @@ -75,7 +77,9 @@ export class AngularNodeAppEngine {
requestContext?: unknown,
): Promise<Response | null> {
const webRequest =
request instanceof Request ? request : createWebRequestFromNodeRequest(request);
request instanceof Request
? request
: createWebRequestFromNodeRequest(request, this.trustProxyHeaders);

return this.angularAppEngine.handle(webRequest, requestContext);
}
Expand Down
65 changes: 58 additions & 7 deletions packages/angular/ssr/node/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -17,7 +21,13 @@ import { getFirstHeaderValue } from '../../src/utils/validation';
* as they are not allowed to be set directly using the `Node.js` Undici API or
* the web `Headers` API.
*/
const HTTP2_PSEUDO_HEADERS = new Set([':method', ':scheme', ':authority', ':path', ':status']);
const HTTP2_PSEUDO_HEADERS: ReadonlySet<string> = new Set([
':method',
':scheme',
':authority',
':path',
':status',
]);

/**
* Converts a Node.js `IncomingMessage` or `Http2ServerRequest` into a
Expand All @@ -27,16 +37,25 @@ const HTTP2_PSEUDO_HEADERS = new Set([':method', ':scheme', ':authority', ':path
* 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 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.
*/
export function createWebRequestFromNodeRequest(
nodeRequest: IncomingMessage | Http2ServerRequest,
trustProxyHeaders?: boolean | readonly string[],
): Request {
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), {
return new Request(createRequestUrl(nodeRequest, trustProxyHeadersNormalized), {
method,
headers: createRequestHeaders(headers),
body: withBody ? nodeRequest : undefined,
Expand Down Expand Up @@ -75,32 +94,64 @@ function createRequestHeaders(nodeHeaders: IncomingHttpHeaders): Headers {
* 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 set of allowed proxy headers.
*
* @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 `URL` object representing the request URL.
*/
export function createRequestUrl(nodeRequest: IncomingMessage | Http2ServerRequest): URL {
export function createRequestUrl(
nodeRequest: IncomingMessage | Http2ServerRequest,
trustProxyHeaders: ReadonlySet<string>,
): URL {
const {
headers,
socket,
url = '',
originalUrl,
} = nodeRequest as IncomingMessage & { originalUrl?: string };

const protocol =
getFirstHeaderValue(headers['x-forwarded-proto']) ??
getAllowedProxyHeaderValue(headers, 'x-forwarded-proto', trustProxyHeaders) ??
('encrypted' in socket && socket.encrypted ? 'https' : 'http');

const hostname =
getFirstHeaderValue(headers['x-forwarded-host']) ?? headers.host ?? headers[':authority'];
getAllowedProxyHeaderValue(headers, 'x-forwarded-host', trustProxyHeaders) ??
headers.host ??
headers[':authority'];

if (Array.isArray(hostname)) {
throw new Error('host value cannot be an array.');
}

let hostnameWithPort = hostname;
if (!hostname?.includes(':')) {
const port = getFirstHeaderValue(headers['x-forwarded-port']);
const port = getAllowedProxyHeaderValue(headers, 'x-forwarded-port', trustProxyHeaders);
if (port) {
hostnameWithPort += `:${port}`;
}
}

return new URL(`${protocol}://${hostnameWithPort}${originalUrl ?? url}`);
}

/**
* Gets the first value of an allowed proxy header.
*
* @param headers - The Node.js incoming HTTP headers.
* @param headerName - The name of the proxy header to retrieve.
* @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: ReadonlySet<string>,
): string | undefined {
return isProxyHeaderAllowed(headerName, trustProxyHeaders)
? getFirstHeaderValue(headers[headerName])
: undefined;
}
24 changes: 11 additions & 13 deletions packages/angular/ssr/node/test/request_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
*/

import { IncomingMessage } from 'node:http';
import { Http2ServerRequest } from 'node:http2';
import { Socket } from 'node:net';
import { createRequestUrl } from '../src/request';
import { normalizeTrustProxyHeaders } from '../../src/utils/validation';

Check failure on line 12 in packages/angular/ssr/node/test/request_spec.ts

View workflow job for this annotation

GitHub Actions / lint

`../../src/utils/validation` import should occur before import of `../src/request`

// Helper to create a mock request object for testing.
function createRequest(details: {
Expand All @@ -26,25 +26,14 @@
} as unknown as IncomingMessage;
}

// Helper to create a mock Http2ServerRequest object for testing.
function createHttp2Request(details: {
headers: Record<string, string | string[] | undefined>;
url?: string;
}): Http2ServerRequest {
return {
headers: details.headers,
socket: new Socket(),
url: details.url,
} as Http2ServerRequest;
}

describe('createRequestUrl', () => {
it('should create a http URL with hostname and port from the host header', () => {
const url = createRequestUrl(
createRequest({
headers: { host: 'localhost:8080' },
url: '/test',
}),
new Set(),
);
expect(url.href).toBe('http://localhost:8080/test');
});
Expand All @@ -56,6 +45,7 @@
encryptedSocket: true,
url: '/test',
}),
new Set(),
);
expect(url.href).toBe('https://example.com/test');
});
Expand All @@ -67,6 +57,7 @@
encryptedSocket: true,
url: '',
}),
new Set(),
);
expect(url.href).toBe('https://example.com/');
});
Expand All @@ -78,6 +69,7 @@
encryptedSocket: true,
url: '/test?a=1',
}),
new Set(),
);
expect(url.href).toBe('https://example.com/test?a=1');
});
Expand All @@ -90,6 +82,7 @@
url: '/test',
originalUrl: '/original',
}),
new Set(),
);
expect(url.href).toBe('https://example.com/original');
});
Expand All @@ -102,6 +95,7 @@
url: undefined,
originalUrl: undefined,
}),
new Set(),
);
expect(url.href).toBe('https://example.com/');
});
Expand All @@ -112,6 +106,7 @@
headers: { host: 'localhost:8080' },
url: '//example.com/test',
}),
new Set(),
);
expect(url.href).toBe('http://localhost:8080//example.com/test');
});
Expand All @@ -123,6 +118,7 @@
url: '/test',
originalUrl: '//example.com/original',
}),
new Set(),
);
expect(url.href).toBe('http://localhost:8080//example.com/original');
});
Expand All @@ -137,6 +133,7 @@
},
url: '/test',
}),
normalizeTrustProxyHeaders(true),
);
expect(url.href).toBe('https://example.com/test');
});
Expand All @@ -152,6 +149,7 @@
},
url: '/test',
}),
normalizeTrustProxyHeaders(true),
);
expect(url.href).toBe('https://example.com:8443/test');
});
Expand Down
55 changes: 34 additions & 21 deletions packages/angular/ssr/src/app-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 { cloneRequestAndPatchHeaders, validateRequest } from './utils/validation';
import {
normalizeTrustProxyHeaders,
sanitizeRequestHeaders,
validateRequest,
} from './utils/validation';

/**
* Options for the Angular server application engine.
Expand All @@ -22,6 +26,25 @@ export interface AngularAppEngineOptions {
* A set of allowed hostnames for the server application.
*/
allowedHosts?: readonly string[];

/**
* Extends the scope of trusted proxy headers (`X-Forwarded-*`).
*
* @remarks
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Just for my own curiosity, is there a particular benefit to using @remarks? Does our tooling do something with that? I've never used that myself.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, @remarks are added as Usage notes in the docs.

* **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.
* If `false` or not provided, proxy headers are ignored.
*
* @default false
*/
trustProxyHeaders?: boolean | readonly string[];
Comment thread
alan-agius4 marked this conversation as resolved.
Comment thread
alan-agius4 marked this conversation as resolved.
}

/**
Expand Down Expand Up @@ -78,6 +101,11 @@ export class AngularAppEngine {
this.manifest.supportedLocales,
);

/**
* The normalized allowed proxy headers.
*/
private readonly trustProxyHeaders: ReadonlySet<string>;

/**
* A cache that holds entry points, keyed by their potential locale string.
*/
Expand All @@ -89,6 +117,7 @@ export class AngularAppEngine {
*/
constructor(options?: AngularAppEngineOptions) {
this.allowedHosts = this.getAllowedHosts(options);
this.trustProxyHeaders = normalizeTrustProxyHeaders(options?.trustProxyHeaders);
}

private getAllowedHosts(options: AngularAppEngineOptions | undefined): ReadonlySet<string> {
Expand Down Expand Up @@ -131,33 +160,17 @@ export class AngularAppEngine {
*/
async handle(request: Request, requestContext?: unknown): Promise<Response | null> {
const allowedHost = this.allowedHosts;
const disableAllowedHostsCheck = AngularAppEngine.ɵdisableAllowedHostsCheck;
const securedRequest = sanitizeRequestHeaders(request, this.trustProxyHeaders);

try {
validateRequest(request, allowedHost, disableAllowedHostsCheck);
validateRequest(securedRequest, allowedHost, AngularAppEngine.ɵdisableAllowedHostsCheck);
} catch (error) {
return this.handleValidationError(request.url, error as Error);
return this.handleValidationError(securedRequest.url, error as Error);
}

// Clone request with patched headers to prevent unallowed host header access.
const { request: securedRequest, onError: onHeaderValidationError } = disableAllowedHostsCheck
? { request, onError: null }
: cloneRequestAndPatchHeaders(request, allowedHost);

const serverApp = await this.getAngularServerAppForRequest(securedRequest);
if (serverApp) {
const promises: Promise<Response | null>[] = [];
if (onHeaderValidationError) {
promises.push(
onHeaderValidationError.then((error) =>
this.handleValidationError(securedRequest.url, error),
),
);
}

promises.push(serverApp.handle(securedRequest, requestContext));

return Promise.race(promises);
return serverApp.handle(securedRequest, requestContext);
}

if (this.supportedLocales.length > 1) {
Expand Down
Loading
Loading