-
Notifications
You must be signed in to change notification settings - Fork 15
feat: autogenerate default allowedHosts and trustProxyHeaders #427
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
63c4b60
d723b43
a41c8f4
5228f75
a772d28
aa3369d
a87042c
f17a9b0
2d4a587
0fed716
a4c1f7e
49f5442
8ee6796
1967b06
43517b0
e47da59
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,16 +119,16 @@ If you are using Angular 20 or Angular 19 with App Engine Developer Preview: | |
| ```ts | ||
| import { AngularAppEngine, createRequestHandler } from '@angular/ssr' | ||
| import { getContext } from '@netlify/angular-runtime/context.js' | ||
| import type { Context } from '@netlify/edge-functions' | ||
|
|
||
| const angularAppEngine = new AngularAppEngine() | ||
|
|
||
| export async function netlifyAppEngineHandler(request: Request): Promise<Response> { | ||
| const context = getContext() | ||
| const context: Context = getContext() | ||
|
|
||
| // Example API endpoints can be defined here. | ||
| // Uncomment and define endpoints as necessary. | ||
| // const pathname = new URL(request.url).pathname; | ||
| // if (pathname === '/api/hello') { | ||
| // if (context.url.pathname === '/api/hello') { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Search for Context usage patterns and URL pathname extraction
echo "=== Checking Context type definition and usage ==="
rg -n -A5 -B5 'Context.*url|context\.url' -g '*.{ts,js,d.ts}'
echo "=== Checking Request URL usage ==="
rg -n 'request\.url|new URL\(request' -g '*.{ts,js}'Repository: netlify/angular-runtime Length of output: 599 🏁 Script executed: #!/bin/bash
# Check the actual README content at lines 131 and 157
echo "=== README.md lines around 131 ==="
sed -n '125,140p' README.md
echo ""
echo "=== README.md lines around 157 ==="
sed -n '150,165p' README.md
echo ""
echo "=== Check serverModuleHelpers.js for context/URL handling ==="
cat -n src/helpers/serverModuleHelpers.js | head -50Repository: netlify/angular-runtime Length of output: 3489 🏁 Script executed: #!/bin/bash
# Check for Context type imports and definitions
echo "=== Search for Context type definition ==="
rg -n "interface Context|type Context|from '@netlify" -g '*.{ts,d.ts}' -A 3
echo ""
echo "=== Check what getContext returns ==="
rg -n "export.*getContext|function getContext" -g '*.{ts,js}' -A 5Repository: netlify/angular-runtime Length of output: 1053 🏁 Script executed: #!/bin/bash
# Find all actual usages of context properties to understand the API
echo "=== All context property accesses ==="
rg -n 'context\.' -g '*.{ts,js}' | grep -v "// " | head -20
echo ""
echo "=== Check Netlify context API in any type definitions ==="
find . -name "*.d.ts" -exec grep -l "Netlify\|Context" {} \;Repository: netlify/angular-runtime Length of output: 462 🏁 Script executed: #!/bin/bash
# Search more specifically for context property usage
echo "=== Look for context?.ip or context property accesses ==="
rg -n 'context\?\.' -g '*.ts'
echo ""
echo "=== Check demo files for context usage ==="
cat -n demo/server.ts
echo ""
echo "=== Check dashboard component context usage ==="
sed -n '20,30p' demo/src/app/dashboard/dashboard.component.tsRepository: netlify/angular-runtime Length of output: 1637 Fix README examples to use the correct URL parsing approach. The README shows 🤖 Prompt for AI Agents |
||
| // return Response.json({ message: 'Hello from the API' }); | ||
| // } | ||
|
|
||
|
|
@@ -147,14 +147,14 @@ If you are using Angular 19 and did not opt into the App Engine Developer Previe | |
| ```ts | ||
| import { CommonEngine } from '@angular/ssr/node' | ||
| import { render } from '@netlify/angular-runtime/common-engine.js' | ||
| import type { Context } from '@netlify/edge-functions' | ||
|
|
||
| const commonEngine = new CommonEngine() | ||
|
|
||
| export async function netlifyCommonEngineHandler(request: Request, context: any): Promise<Response> { | ||
| export async function netlifyCommonEngineHandler(request: Request, context: Context): Promise<Response> { | ||
| // Example API endpoints can be defined here. | ||
| // Uncomment and define endpoints as necessary. | ||
| // const pathname = new URL(request.url).pathname; | ||
| // if (pathname === '/api/hello') { | ||
| // if (context.url.pathname === '/api/hello') { | ||
| // return Response.json({ message: 'Hello from the API' }); | ||
| // } | ||
|
|
||
|
|
@@ -183,8 +183,6 @@ The [`server.ts` file](https://angular.dev/guide/ssr#configure-server-side-rende | |
|
|
||
| To use the Angular Runtime while building and deploying with the CLI, you need to have `netlify-cli v26.0.0` installed (or a later version). | ||
|
|
||
| Please also make sure to use `ntl deploy --build` (rather than `ntl build && ntl deploy`). | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While this as-is seems outdated because |
||
|
|
||
| ## Getting Help | ||
|
|
||
| We love to hear from you so if you have questions, comments or find a bug in the | ||
|
|
||
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| export declare function getAllowedHosts(): string[] | ||
| export declare function getTrustProxyHeaders(): string[] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import { env } from 'node:process' | ||
|
|
||
| export function getAllowedHosts() { | ||
| const allowedHosts = [] | ||
|
|
||
| if (!env.DEPLOY_ID || !env.DEPLOY_PRIME_URL || !env.DEPLOY_URL || !env.SITE_ID || !env.SITE_NAME || !env.URL) { | ||
| console.warn('Missing Netlify-specific environment variable(s). `allowedHosts` config might be incomplete.') | ||
| return allowedHosts | ||
| } | ||
|
Comment on lines
+6
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: When AngularAppEngine/AngularNodeAppEngine is configured with allowedHosts: [], it treats that as “no allowed hosts are configured”. As a result, requests whose request URL hostname is not recognized as allowed will not be treated as allowed; Angular will return a 400 Bad Request (and fall back to client-side rendering only when allowedHosts is non-empty / properly authorized). This behavior is described in Angular’s SSR API docs: AngularAppEngine verifies the hostname of request.url against an authorized-host list; if the hostname is not recognized and allowedHosts is not empty, the engine returns a Client-Side Rendered version, otherwise it returns 400 Bad Request. Therefore, with allowedHosts set to the empty array, the “allowedHosts is not empty” condition is false, so the 400 path applies for disallowed/unknown hosts.[1] Angular’s option type shows allowedHosts is an optional readonly string[]; passing an empty array explicitly configures the allowlist as empty.[2] Citations:
Consider the fallback behavior when required environment variables are missing. When environment variables are missing, this function returns an empty 🤖 Prompt for AI Agents |
||
|
|
||
| const deployPrimeUrlHostname = new URL(env.DEPLOY_PRIME_URL).hostname | ||
|
|
||
| // <subdomain>.netlify.app OR <custom-domain> | ||
| // www handling is not required as Netlify will auto-redirect | ||
| allowedHosts.push(new URL(env.URL).hostname) | ||
| // <deploy-id>--<subdomain>.netlify.app | ||
| allowedHosts.push(new URL(env.DEPLOY_URL).hostname) | ||
| // <branch-name>--<subdomain>.netlify.app or <dp-#>--<subdomain>.netlify.app (supports ADS) | ||
| allowedHosts.push(deployPrimeUrlHostname) | ||
| // <subdomain>.netlify.app | ||
| // this will be duplicated for sites without custom domain | ||
| // but it's important to have in case a site's custom domain is removed after a deploy | ||
| allowedHosts.push(`${env.SITE_NAME}.netlify.app`) | ||
| // <site-id>.netlify.app | ||
| allowedHosts.push(`${env.SITE_ID}.netlify.app`) | ||
| // <deploy-id>--<site-id>.netlify.app | ||
| allowedHosts.push(`${env.DEPLOY_ID}--${env.SITE_ID}.netlify.app`) | ||
|
|
||
| // we need to extract the branch name or the deploy preview number | ||
| // so we can add the subdomain as well as site-id specific URLs | ||
| // this would be required for sites using ADS so that | ||
| // we can add netlify.app URLs as well | ||
| if (deployPrimeUrlHostname.includes('--')) { | ||
| const [branchNameOrDpNumber] = deployPrimeUrlHostname.split('--') | ||
| allowedHosts.push(`${branchNameOrDpNumber}--${env.SITE_NAME}.netlify.app`) | ||
| allowedHosts.push(`${branchNameOrDpNumber}--${env.SITE_ID}.netlify.app`) | ||
| } | ||
|
|
||
| return allowedHosts | ||
| } | ||
|
|
||
| export function getTrustProxyHeaders() { | ||
| return ['x-forwarded-for'] | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 does
getContextnot automatically getContexttype? If so - that probably should be fixed sogetContext()is typed instead of having helper in angular-runtime and type from different package?