Skip to content

fix(@angular/ssr): add support for configuring trusted proxy headers via environment variable #33272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dgp1130 wants to merge 1 commit into
base: 20.3.x
Choose a base branch
from
+30 −19
Open

fix(@angular/ssr): add support for configuring trusted proxy headers via environment variable #33272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 8 additions & 5 deletions packages/angular/ssr/node/src/app-engine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { AngularAppEngine } from '@angular/ssr';
import type { IncomingMessage } from 'node:http';
import type { Http2ServerRequest } from 'node:http2';
import { AngularAppEngineOptions } from '../../src/app-engine';
import { getAllowedHostsFromEnv } from './environment-options';
import { getAllowedHostsFromEnv, getTrustProxyHeadersFromEnv } from './environment-options';
import { attachNodeGlobalErrorHandlers } from './errors';
import { createWebRequestFromNodeRequest } from './request';

Expand All @@ -36,11 +36,14 @@ export class AngularNodeAppEngine {
* @param options Options for the Angular Node.js server application engine.
*/
constructor(options?: AngularNodeAppEngineOptions) {
this.angularAppEngine = new AngularAppEngine({
const appEngineOptions: AngularAppEngineOptions = {
...options,
allowedHosts: [...getAllowedHostsFromEnv(), ...(options?.allowedHosts ?? [])],
});
this.trustProxyHeaders = options?.trustProxyHeaders;
allowedHosts: getAllowedHostsFromEnv() ?? options?.allowedHosts,
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

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

high

Changing 'allowedHosts' to use the nullish coalescing operator ('??') changes the behavior from merging the environment variable hosts with the programmatic options to completely overriding them. This is a breaking change for deployments that rely on both sources. To preserve backward compatibility, we should continue to merge them.

      allowedHosts: [
        ...(getAllowedHostsFromEnv() ?? []),
        ...(options?.allowedHosts ?? []),
      ],

Copy link
Copy Markdown
Collaborator Author

@dgp1130 dgp1130 May 29, 2026

Choose a reason for hiding this comment

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

@alan-agius4, this is seems like reasonable feedback, though I'm hesitant to diverge this PR from the one it's porting, maybe this is a separate fix we should consider landing?

Copy link
Copy Markdown
Collaborator

@alan-agius4 alan-agius4 May 29, 2026

Choose a reason for hiding this comment

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

I think this was done based on your feedback, so also keep the same behaviour between trustProxyHeaders and allowedHosts.

trustProxyHeaders: getTrustProxyHeadersFromEnv() ?? options?.trustProxyHeaders,
};

this.angularAppEngine = new AngularAppEngine(appEngineOptions);
this.trustProxyHeaders = appEngineOptions.trustProxyHeaders;

attachNodeGlobalErrorHandlers();
}
Expand Down
5 changes: 1 addition & 4 deletions packages/angular/ssr/node/src/common-engine/common-engine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,7 @@ export class CommonEngine {
private readonly allowedHosts: ReadonlySet<string>;

constructor(private options?: CommonEngineOptions) {
this.allowedHosts = new Set([
...getAllowedHostsFromEnv(),
...(this.options?.allowedHosts ?? []),
]);
this.allowedHosts = new Set(getAllowedHostsFromEnv() ?? this.options?.allowedHosts ?? []);
Comment thread
dgp1130 marked this conversation as resolved.

attachNodeGlobalErrorHandlers();
}
Expand Down
31 changes: 21 additions & 10 deletions packages/angular/ssr/node/src/environment-options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,31 @@
* Retrieves the list of allowed hosts from the environment variable `NG_ALLOWED_HOSTS`.
* @returns An array of allowed hosts.
*/
export function getAllowedHostsFromEnv(): ReadonlyArray<string> {
const allowedHosts: string[] = [];
const envNgAllowedHosts = process.env['NG_ALLOWED_HOSTS'];
if (!envNgAllowedHosts) {
return allowedHosts;
export function getAllowedHostsFromEnv(): ReadonlyArray<string> | undefined {
return getArrayFromEnv('NG_ALLOWED_HOSTS');
}

/**
* Retrieves the list of trusted proxy headers from the environment variable `NG_TRUST_PROXY_HEADERS`.
* @returns An array of trusted proxy headers.
*/
export function getTrustProxyHeadersFromEnv(): ReadonlyArray<string> | undefined {
return getArrayFromEnv('NG_TRUST_PROXY_HEADERS');
}

function getArrayFromEnv(envName: string): ReadonlyArray<string> | undefined {
const envValue = process.env[envName];
if (!envValue) {
return undefined;
}

const hosts = envNgAllowedHosts.split(',');
for (const host of hosts) {
const trimmed = host.trim();
const values: string[] = [];
for (const value of envValue.split(',')) {
const trimmed = value.trim();
if (trimmed.length > 0) {
allowedHosts.push(trimmed);
values.push(trimmed);
}
}

return allowedHosts;
return values;
}
Loading