Skip to content

fix(@angular/ssr): validate host headers to prevent header-based SSRF#32516

Open
alan-agius4 wants to merge 2 commits intoangular:mainfrom
alan-agius4:security-validate-main
Open

fix(@angular/ssr): validate host headers to prevent header-based SSRF#32516
alan-agius4 wants to merge 2 commits intoangular:mainfrom
alan-agius4:security-validate-main

Conversation

@alan-agius4
Copy link
Collaborator

@alan-agius4 alan-agius4 commented Feb 19, 2026

This change introduces strict validation for Host, X-Forwarded-Host, X-Forwarded-Proto, and X-Forwarded-Port headers in the Angular SSR request handling pipeline, including CommonEngine and AngularAppEngine.

Previously, the application engine constructed the base URL for server-side rendering using these headers without validation. This could allow an attacker to manipulate the headers to steer relative HttpClient requests to arbitrary internal or external hosts (SSRF).

The validation rules are:

  • Host and X-Forwarded-Host headers are validated against an authorized allowlist.
  • Host and X-Forwarded-Host headers cannot contain path separators.
  • X-Forwarded-Port header must be numeric.
  • X-Forwarded-Proto header must be http or https.

Requests with invalid or disallowed headers will now log an error and fallback to Client-Side Rendering (CSR). In a future major version, these requests will be rejected with a 400 Bad Request.

Note: Most cloud providers and CDNs already validate these headers before the request reaches the text application, but this change adds an essential layer of defense-in-depth.

AngularAppEngine Users:
To exclude safe hosts from validation, configure the allowedHosts option in angular.json to include all domain names where your application is deployed.

Example configuration in angular.json:

"architect": {
  "build": {
    "options": {
      "security": {
        "allowedHosts": ["example.com", "*.trusted-example.com"]
      }
    }
  }
}

or

const appEngine = new AngularAppEngine({
  allowedHosts: ["example.com", "*.trusted-example.com"]
})

const appEngine = new AngularNodeAppEngine({
  allowedHosts: ["example.com", "*.trusted-example.com"]
})

CommonEngine Users:
If you are using CommonEngine, you must now provide the allowedHosts option when initializing or rendering your application.

Example:

const commonEngine = new CommonEngine({
  allowedHosts: ["example.com", "*.trusted-example.com"]
});

The application also respects NG_ALLOWED_HOSTS (comma-separated list) and HOSTNAME environment variables for authorizing hosts.

@alan-agius4 alan-agius4 added the target: minor This PR is targeted for the next minor release label Feb 19, 2026
@alan-agius4 alan-agius4 added target: rc This PR is targeted for the next release-candidate and removed target: minor This PR is targeted for the next minor release area: @angular/ssr labels Feb 19, 2026
@alan-agius4 alan-agius4 force-pushed the security-validate-main branch from a44aef7 to 8778533 Compare February 19, 2026 11:26
@alan-agius4 alan-agius4 force-pushed the security-validate-main branch 5 times, most recently from 757306a to de79ce3 Compare February 19, 2026 12:15
@alan-agius4 alan-agius4 requested a review from dgp1130 February 19, 2026 13:15
@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer labels Feb 19, 2026
@alan-agius4 alan-agius4 marked this pull request as ready for review February 19, 2026 13:15
@alan-agius4 alan-agius4 force-pushed the security-validate-main branch 4 times, most recently from faf18cf to aa6ee92 Compare February 19, 2026 14:49
Copy link
Collaborator

@dgp1130 dgp1130 left a comment

Choose a reason for hiding this comment

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

Left some general suggestions, but nothing major. Feel free to ignore or come back to any of these later rather than treating them as blocking this PR. The two main things to confirm are really just:

  1. Should we commit to ${HOSTNAME} / ${NG_ALLOWED_HOSTS} right now, or come back later?
  2. Should we error when allowedHosts is configured and we receive a request for a disallowed host, to be up front about a misconfiguration rather than silently deoptimizing?

Comment on lines 91 to 94
let document = opts.document;
if (!document && opts.documentFilePath) {
document = await this.getDocument(opts.documentFilePath);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question: What's the purpose of reading the document here? Why do we need this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Document in this is case is the index.html which we need to serve it to render CSR.

Comment on lines 225 to 234
const envNgAllowedHosts = processEnv['NG_ALLOWED_HOSTS'];
if (envNgAllowedHosts) {
const hosts = envNgAllowedHosts.split(',');
for (const host of hosts) {
const hostTrimmed = host.trim();
if (hostTrimmed) {
allowedHosts.add(hostTrimmed);
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider: Do you want to tackle this now, as something we'll probably have to support for a few versions, or leave it off and require allowedHosts to be specified in the build options in the short term until we come back to this in v22+?

I agree this is probably a good direction, but I'm wondering if it might benefit from more discussion, and I don't think we need to have it right now. Would it make sense to hold off before committing to a new public API here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My reason for this is that you'd be able to provide the allowedHost at runtime where using env variables is not an option. For instance in worker context.

If you feel we should hold this off, I am fine with this. Please let me know.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm ok either way and happy to defer to you on the decision. I'm just highlighting the option to defer "providing allowedHosts at runtime" to v22 if you don't feel it's necessary here. Ultimately up to you, no objection to landing it now if you think that's better.


const key = name.toLowerCase();
if (HOST_HEADERS_TO_VALIDATE.has(key)) {
verifyHostAllowed(key, value, allowedHosts);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Observation: I could see this failing some apps who iterate through headers like:

for (const [name, value] of Object.entries(req.headers)) {
  if (!name.startsWith('x-myapp-*')) continue;

  doSomethingWith(name, value);
}

In this case, they're not really using host or x-forwarded-host, but they will read the values and probably trigger the CSR deopt. I don't think there's anything we can meaningfully do about this, but pointing out the case of iterating through all the headers which isn't too uncommon.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They will actually not trigger the CSR deopt. in this case they will not be validated.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why wouldn't this be validated? Wouldn't reading req.headers['host'] (even through Object.entries or another iterator) trigger validation and then deopt?

Copy link
Collaborator Author

@alan-agius4 alan-agius4 Feb 20, 2026

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh... does that imply that we should patch these iteration functions? entries, entriesList, rawValues, toSortedArray, Symbol.iterator? Otherwise you could circumvent this check a basic req.headers.entriesList().find(([header]) => header === 'host').

(That example is obviously intentionally working around the safety check, but anything just iterating all headers has the same risk.)

}
}

const xForwardedPort = getFirstHeaderValue(headers.get('x-forwarded-port'));

Choose a reason for hiding this comment

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

Quick question: do we normalize headers and lowercase the names earlier in the code?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No we do not normalize headers names, the headers.get() is case insensitive.

This change introduces strict validation for `Host`, `X-Forwarded-Host`, `X-Forwarded-Proto`, and `X-Forwarded-Port` headers in the Angular SSR request handling pipeline, including `CommonEngine` and `AngularAppEngine`.

Previously, the application engine constructed the base URL for server-side rendering using these headers without validation. This could allow an attacker to manipulate the headers to steer relative `HttpClient` requests to arbitrary internal or external hosts (SSRF).

The validation rules are:

- `Host` and `X-Forwarded-Host` headers are validated against a strict allowlist.
- `Host` and `X-Forwarded-Host` headers cannot contain path separators.
- `X-Forwarded-Port` header must be numeric.
- `X-Forwarded-Proto` header must be `http` or `https`.

Requests with invalid or disallowed headers will now log an error and fallback to Client-Side Rendering (CSR). In a future major version, these requests will be rejected with a 400 Bad Request.

Note: Most cloud providers and CDNs already validate these headers before the request reaches the text application, but this change adds an essential layer of defense-in-depth.

**AngularAppEngine Users:**
To exclude safe hosts from validation, configure the `allowedHosts` option in `angular.json` to include all domain names where your application is deployed.

Example configuration in `angular.json`:

```json
"architect": {
  "build": {
    "options": {
      "security": {
        "allowedHosts": ["example.com", "*.trusted-example.com"]
      }
    }
  }
}
```

or

```ts
const appEngine = new AngularAppEngine({
  allowedHosts: ["example.com", "*.trusted-example.com"]
})

const appEngine = new AngularNodeAppEngine({
  allowedHosts: ["example.com", "*.trusted-example.com"]
})
```

**CommonEngine Users:**
If you are using `CommonEngine`, you must now provide the `allowedHosts` option when initializing or rendering your application.

Example:
```typescript
const commonEngine = new CommonEngine({
  allowedHosts: ["example.com", "*.trusted-example.com"]
});
```

The application also respects `NG_ALLOWED_HOSTS` (comma-separated list) environment variable for authorizing hosts.
@alan-agius4 alan-agius4 force-pushed the security-validate-main branch 6 times, most recently from da7a6aa to 301a633 Compare February 20, 2026 11:17
@alan-agius4
Copy link
Collaborator Author

Left some general suggestions, but nothing major. Feel free to ignore or come back to any of these later rather than treating them as blocking this PR. The two main things to confirm are really just:

Should we commit to ${HOSTNAME} / ${NG_ALLOWED_HOSTS} right now, or come back later?

I removed ${HOSTNAME}, I think ${NG_ALLOWED_HOSTS} can be useful to support this dynamically in environments that support Node.js

Should we error when allowedHosts is configured and we receive a request for a disallowed host, to be up front about a misconfiguration rather than silently deoptimizing?

Updated the logic to do this.

@alan-agius4 alan-agius4 force-pushed the security-validate-main branch 2 times, most recently from 50ff773 to fe3fbba Compare February 20, 2026 14:10
@alan-agius4 alan-agius4 requested a review from dgp1130 February 20, 2026 15:08
@alan-agius4 alan-agius4 force-pushed the security-validate-main branch from fe3fbba to e38778a Compare February 20, 2026 15:53
@alan-agius4 alan-agius4 force-pushed the security-validate-main branch from e38778a to a9a660f Compare February 20, 2026 15:54
Copy link
Collaborator

@dgp1130 dgp1130 left a comment

Choose a reason for hiding this comment

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

Like I mentioned previously, feel free to ignore or come back to these comments later, I don't think any of them are blocking.

'Please provide a list of allowed hosts in the "allowedHosts" option in the "CommonEngine" constructor.',
isAllowedHostConfigured
? ''
: '\nFallbacking to client side rendering. This will become a 400 Bad Request in a future major version.',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo: "Falling back"

return allowedHosts;
}

const hosts = envNgAllowedHosts.split(',');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider: Is it worth validating that the provided names are valid hostnames (letters, numbers, dashes only I think)?

Less of a security thing and more just DX to check for typos.

}

// Clone request with patched headers to prevent unallowed host header access.
const { request: securedRequest, onError: onHeaderValidationError } =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider: Rename request to insecureRequest and then rename securedRequest to request.

This way avoids future changes accidentally using request when they really wanted to use the secured one.

* @returns An object containing the cloned request and a promise that resolves to an error
* if any of the validated headers contain invalid values.
*/
export function cloneRequestAndPatchHeaders(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider: The functionality purist it me likes the idea of cloning the request, but I'm wondering if there's a risk of the original request leaking into user code somehow in a way which might circumvent this check? Are there any server frameworks which offer a kind of getActiveRequest() function which might retrieve the original? Would it be safer to mutate the original object?

Maybe this is fine as a temporary mitigation since we'll eventually move to a hard 400 error up front, so it might not be too big a deal in the short term.

} catch (error) {
onError(error as Error);

return null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider: Would it be safer to throw in user space rather than letting them assume a given header doesn't exist?

We probably still need onError as we wouldn't want the user to catch and suppress this error, but I'm thinking user code might behave strangely if we pretend the header doesn't exist at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: review The PR is still awaiting reviews from at least one requested reviewer area: @angular/ssr target: rc This PR is targeted for the next release-candidate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments