Skip to content

Commit 5adc925

Browse files
authored
fix(@angular/ssr): enforce explicit opt-in for proxy headers
This commit introduces a secure-by-default model for trusting proxy headers (`X-Forwarded-*`) in the `@angular/ssr` package. Previously, the engine relied on complex lazy header patching and regex filters to guard against spoofed headers. However, implicit decoding behaviors by URL constructors can render naive regex filtering ineffective against certain percent-encoded payloads.
1 parent 48eab1f commit 5adc925

10 files changed

Lines changed: 288 additions & 320 deletions

File tree

goldens/public-api/angular/ssr/index.api.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export class AngularAppEngine {
2222
// @public
2323
export interface AngularAppEngineOptions {
2424
allowedHosts?: readonly string[];
25+
trustProxyHeaders?: boolean | readonly string[];
2526
}
2627

2728
// @public

goldens/public-api/angular/ssr/node/index.api.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export interface CommonEngineRenderOptions {
5555
export function createNodeRequestHandler<T extends NodeRequestHandlerFunction>(handler: T): T;
5656

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

6060
// @public
6161
export function isMainModule(url: string): boolean;

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export interface AngularNodeAppEngineOptions extends AngularAppEngineOptions {}
2929
*/
3030
export class AngularNodeAppEngine {
3131
private readonly angularAppEngine: AngularAppEngine;
32+
private readonly trustProxyHeaders?: boolean | readonly string[];
3233

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

4345
attachNodeGlobalErrorHandlers();
4446
}
@@ -75,7 +77,9 @@ export class AngularNodeAppEngine {
7577
requestContext?: unknown,
7678
): Promise<Response | null> {
7779
const webRequest =
78-
request instanceof Request ? request : createWebRequestFromNodeRequest(request);
80+
request instanceof Request
81+
? request
82+
: createWebRequestFromNodeRequest(request, this.trustProxyHeaders);
7983

8084
return this.angularAppEngine.handle(webRequest, requestContext);
8185
}

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

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@
88

99
import type { IncomingHttpHeaders, IncomingMessage } from 'node:http';
1010
import type { Http2ServerRequest } from 'node:http2';
11-
import { getFirstHeaderValue } from '../../src/utils/validation';
11+
import {
12+
getFirstHeaderValue,
13+
isProxyHeaderAllowed,
14+
normalizeTrustProxyHeaders,
15+
} from '../../src/utils/validation';
1216

1317
/**
1418
* A set containing all the pseudo-headers defined in the HTTP/2 specification.
@@ -17,7 +21,13 @@ import { getFirstHeaderValue } from '../../src/utils/validation';
1721
* as they are not allowed to be set directly using the `Node.js` Undici API or
1822
* the web `Headers` API.
1923
*/
20-
const HTTP2_PSEUDO_HEADERS = new Set([':method', ':scheme', ':authority', ':path', ':status']);
24+
const HTTP2_PSEUDO_HEADERS: ReadonlySet<string> = new Set([
25+
':method',
26+
':scheme',
27+
':authority',
28+
':path',
29+
':status',
30+
]);
2131

2232
/**
2333
* Converts a Node.js `IncomingMessage` or `Http2ServerRequest` into a
@@ -27,16 +37,25 @@ const HTTP2_PSEUDO_HEADERS = new Set([':method', ':scheme', ':authority', ':path
2737
* be used by web platform APIs.
2838
*
2939
* @param nodeRequest - The Node.js request object (`IncomingMessage` or `Http2ServerRequest`) to convert.
40+
* @param trustProxyHeaders - A boolean or an array of proxy headers to trust when constructing the request URL.
41+
*
42+
* @remarks
43+
* When `trustProxyHeaders` is enabled, headers such as `X-Forwarded-Host` and
44+
* `X-Forwarded-Prefix` should ideally be strictly validated at a higher infrastructure
45+
* level (e.g., at the reverse proxy or API gateway) before reaching the application.
46+
*
3047
* @returns A Web Standard `Request` object.
3148
*/
3249
export function createWebRequestFromNodeRequest(
3350
nodeRequest: IncomingMessage | Http2ServerRequest,
51+
trustProxyHeaders?: boolean | readonly string[],
3452
): Request {
53+
const trustProxyHeadersNormalized = normalizeTrustProxyHeaders(trustProxyHeaders);
3554
const { headers, method = 'GET' } = nodeRequest;
3655
const withBody = method !== 'GET' && method !== 'HEAD';
3756
const referrer = headers.referer && URL.canParse(headers.referer) ? headers.referer : undefined;
3857

39-
return new Request(createRequestUrl(nodeRequest), {
58+
return new Request(createRequestUrl(nodeRequest, trustProxyHeadersNormalized), {
4059
method,
4160
headers: createRequestHeaders(headers),
4261
body: withBody ? nodeRequest : undefined,
@@ -75,32 +94,64 @@ function createRequestHeaders(nodeHeaders: IncomingHttpHeaders): Headers {
7594
* Creates a `URL` object from a Node.js `IncomingMessage`, taking into account the protocol, host, and port.
7695
*
7796
* @param nodeRequest - The Node.js `IncomingMessage` or `Http2ServerRequest` object to extract URL information from.
97+
* @param trustProxyHeaders - A set of allowed proxy headers.
98+
*
99+
* @remarks
100+
* When `trustProxyHeaders` is enabled, headers such as `X-Forwarded-Host` and
101+
* `X-Forwarded-Prefix` should ideally be strictly validated at a higher infrastructure
102+
* level (e.g., at the reverse proxy or API gateway) before reaching the application.
103+
*
78104
* @returns A `URL` object representing the request URL.
79105
*/
80-
export function createRequestUrl(nodeRequest: IncomingMessage | Http2ServerRequest): URL {
106+
export function createRequestUrl(
107+
nodeRequest: IncomingMessage | Http2ServerRequest,
108+
trustProxyHeaders: ReadonlySet<string>,
109+
): URL {
81110
const {
82111
headers,
83112
socket,
84113
url = '',
85114
originalUrl,
86115
} = nodeRequest as IncomingMessage & { originalUrl?: string };
116+
87117
const protocol =
88-
getFirstHeaderValue(headers['x-forwarded-proto']) ??
118+
getAllowedProxyHeaderValue(headers, 'x-forwarded-proto', trustProxyHeaders) ??
89119
('encrypted' in socket && socket.encrypted ? 'https' : 'http');
120+
90121
const hostname =
91-
getFirstHeaderValue(headers['x-forwarded-host']) ?? headers.host ?? headers[':authority'];
122+
getAllowedProxyHeaderValue(headers, 'x-forwarded-host', trustProxyHeaders) ??
123+
headers.host ??
124+
headers[':authority'];
92125

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

97130
let hostnameWithPort = hostname;
98131
if (!hostname?.includes(':')) {
99-
const port = getFirstHeaderValue(headers['x-forwarded-port']);
132+
const port = getAllowedProxyHeaderValue(headers, 'x-forwarded-port', trustProxyHeaders);
100133
if (port) {
101134
hostnameWithPort += `:${port}`;
102135
}
103136
}
104137

105138
return new URL(`${protocol}://${hostnameWithPort}${originalUrl ?? url}`);
106139
}
140+
141+
/**
142+
* Gets the first value of an allowed proxy header.
143+
*
144+
* @param headers - The Node.js incoming HTTP headers.
145+
* @param headerName - The name of the proxy header to retrieve.
146+
* @param trustProxyHeaders - A set of allowed proxy headers.
147+
* @returns The value of the allowed proxy header, or `undefined` if not allowed or not present.
148+
*/
149+
function getAllowedProxyHeaderValue(
150+
headers: IncomingHttpHeaders,
151+
headerName: string,
152+
trustProxyHeaders: ReadonlySet<string>,
153+
): string | undefined {
154+
return isProxyHeaderAllowed(headerName, trustProxyHeaders)
155+
? getFirstHeaderValue(headers[headerName])
156+
: undefined;
157+
}

packages/angular/ssr/node/test/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ ts_project(
77
srcs = glob(["**/*_spec.ts"]),
88
deps = [
99
"//:node_modules/@types/node",
10+
"//packages/angular/ssr",
1011
"//packages/angular/ssr/node",
1112
],
1213
)

packages/angular/ssr/node/test/request_spec.ts

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
*/
88

99
import { IncomingMessage } from 'node:http';
10-
import { Http2ServerRequest } from 'node:http2';
1110
import { Socket } from 'node:net';
11+
import { normalizeTrustProxyHeaders } from '../../src/utils/validation';
1212
import { createRequestUrl } from '../src/request';
1313

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

29-
// Helper to create a mock Http2ServerRequest object for testing.
30-
function createHttp2Request(details: {
31-
headers: Record<string, string | string[] | undefined>;
32-
url?: string;
33-
}): Http2ServerRequest {
34-
return {
35-
headers: details.headers,
36-
socket: new Socket(),
37-
url: details.url,
38-
} as Http2ServerRequest;
39-
}
40-
4129
describe('createRequestUrl', () => {
4230
it('should create a http URL with hostname and port from the host header', () => {
4331
const url = createRequestUrl(
4432
createRequest({
4533
headers: { host: 'localhost:8080' },
4634
url: '/test',
4735
}),
36+
new Set(),
4837
);
4938
expect(url.href).toBe('http://localhost:8080/test');
5039
});
@@ -56,6 +45,7 @@ describe('createRequestUrl', () => {
5645
encryptedSocket: true,
5746
url: '/test',
5847
}),
48+
new Set(),
5949
);
6050
expect(url.href).toBe('https://example.com/test');
6151
});
@@ -67,6 +57,7 @@ describe('createRequestUrl', () => {
6757
encryptedSocket: true,
6858
url: '',
6959
}),
60+
new Set(),
7061
);
7162
expect(url.href).toBe('https://example.com/');
7263
});
@@ -78,6 +69,7 @@ describe('createRequestUrl', () => {
7869
encryptedSocket: true,
7970
url: '/test?a=1',
8071
}),
72+
new Set(),
8173
);
8274
expect(url.href).toBe('https://example.com/test?a=1');
8375
});
@@ -90,6 +82,7 @@ describe('createRequestUrl', () => {
9082
url: '/test',
9183
originalUrl: '/original',
9284
}),
85+
new Set(),
9386
);
9487
expect(url.href).toBe('https://example.com/original');
9588
});
@@ -102,6 +95,7 @@ describe('createRequestUrl', () => {
10295
url: undefined,
10396
originalUrl: undefined,
10497
}),
98+
new Set(),
10599
);
106100
expect(url.href).toBe('https://example.com/');
107101
});
@@ -112,6 +106,7 @@ describe('createRequestUrl', () => {
112106
headers: { host: 'localhost:8080' },
113107
url: '//example.com/test',
114108
}),
109+
new Set(),
115110
);
116111
expect(url.href).toBe('http://localhost:8080//example.com/test');
117112
});
@@ -123,6 +118,7 @@ describe('createRequestUrl', () => {
123118
url: '/test',
124119
originalUrl: '//example.com/original',
125120
}),
121+
new Set(),
126122
);
127123
expect(url.href).toBe('http://localhost:8080//example.com/original');
128124
});
@@ -137,6 +133,7 @@ describe('createRequestUrl', () => {
137133
},
138134
url: '/test',
139135
}),
136+
normalizeTrustProxyHeaders(true),
140137
);
141138
expect(url.href).toBe('https://example.com/test');
142139
});
@@ -152,6 +149,7 @@ describe('createRequestUrl', () => {
152149
},
153150
url: '/test',
154151
}),
152+
normalizeTrustProxyHeaders(true),
155153
);
156154
expect(url.href).toBe('https://example.com:8443/test');
157155
});

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

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@ import { getPotentialLocaleIdFromUrl, getPreferredLocale } from './i18n';
1212
import { EntryPointExports, getAngularAppEngineManifest } from './manifest';
1313
import { createRedirectResponse } from './utils/redirect';
1414
import { joinUrlParts } from './utils/url';
15-
import { cloneRequestAndPatchHeaders, validateRequest } from './utils/validation';
15+
import {
16+
normalizeTrustProxyHeaders,
17+
sanitizeRequestHeaders,
18+
validateRequest,
19+
} from './utils/validation';
1620

1721
/**
1822
* Options for the Angular server application engine.
@@ -22,6 +26,25 @@ export interface AngularAppEngineOptions {
2226
* A set of allowed hostnames for the server application.
2327
*/
2428
allowedHosts?: readonly string[];
29+
30+
/**
31+
* Extends the scope of trusted proxy headers (`X-Forwarded-*`).
32+
*
33+
* @remarks
34+
* **This is a security-sensitive option!**
35+
*
36+
* When `trustProxyHeaders` is enabled, request headers such as `X-Forwarded-Host` and
37+
* `X-Forwarded-Prefix` are trusted by the server and used for routing. These
38+
* headers must be strictly validated and provided by a trusted client (e.g., at a reverse proxy, load
39+
* balancer, or API gateway) and must *not* be provided by untrusted end users.
40+
*
41+
* If a `string[]` is provided, only those proxy headers are allowed.
42+
* If `true`, all proxy headers are allowed.
43+
* If `false` or not provided, proxy headers are ignored.
44+
*
45+
* @default false
46+
*/
47+
trustProxyHeaders?: boolean | readonly string[];
2548
}
2649

2750
/**
@@ -78,6 +101,11 @@ export class AngularAppEngine {
78101
this.manifest.supportedLocales,
79102
);
80103

104+
/**
105+
* The normalized allowed proxy headers.
106+
*/
107+
private readonly trustProxyHeaders: ReadonlySet<string>;
108+
81109
/**
82110
* A cache that holds entry points, keyed by their potential locale string.
83111
*/
@@ -89,6 +117,7 @@ export class AngularAppEngine {
89117
*/
90118
constructor(options?: AngularAppEngineOptions) {
91119
this.allowedHosts = this.getAllowedHosts(options);
120+
this.trustProxyHeaders = normalizeTrustProxyHeaders(options?.trustProxyHeaders);
92121
}
93122

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

136165
try {
137-
validateRequest(request, allowedHost, disableAllowedHostsCheck);
166+
validateRequest(securedRequest, allowedHost, AngularAppEngine.ɵdisableAllowedHostsCheck);
138167
} catch (error) {
139-
return this.handleValidationError(request.url, error as Error);
168+
return this.handleValidationError(securedRequest.url, error as Error);
140169
}
141170

142-
// Clone request with patched headers to prevent unallowed host header access.
143-
const { request: securedRequest, onError: onHeaderValidationError } = disableAllowedHostsCheck
144-
? { request, onError: null }
145-
: cloneRequestAndPatchHeaders(request, allowedHost);
146-
147171
const serverApp = await this.getAngularServerAppForRequest(securedRequest);
148172
if (serverApp) {
149-
const promises: Promise<Response | null>[] = [];
150-
if (onHeaderValidationError) {
151-
promises.push(
152-
onHeaderValidationError.then((error) =>
153-
this.handleValidationError(securedRequest.url, error),
154-
),
155-
);
156-
}
157-
158-
promises.push(serverApp.handle(securedRequest, requestContext));
159-
160-
return Promise.race(promises);
173+
return serverApp.handle(securedRequest, requestContext);
161174
}
162175

163176
if (this.supportedLocales.length > 1) {

0 commit comments

Comments
 (0)